[codex] Render deprecated API key scopes#2946
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 17 minutes and 35 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughThis pull request introduces a new Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR makes deprecated API key scopes (the legacy
Confidence Score: 4/5Safe to merge after addressing the category bulk-toggle and scope count display issues. Two P1 findings exist: the 'select all' category action silently adds deprecated scopes to new keys, and the scope count badge in the table under-reports when both a deprecated scope and its newer equivalent are selected. Resolving them is straightforward before merging. src/routes/(console)/project-[region]-[project]/overview/api-keys/scopes.svelte and table.svelte (unchanged in PR) need attention for the onCategoryChange bulk-toggle and getApiKeyScopeCount count inconsistency. Important Files Changed
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/lib/constants.ts (1)
158-163: Makecategorya shared union instead of a free-form string.The picker filters this catalog by category with string equality, so a typo here would just make a scope disappear from the UI with no type error. Reusing a shared
ScopeCategoryunion/const list would let TS validate the catalog and remove the duplicated category list in the component.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/constants.ts` around lines 158 - 163, Change the free-form string "category" on the exported type ScopeDefinition to a shared union type by introducing an exported const array (e.g., ScopeCategories) and deriving a type alias ScopeCategory = typeof ScopeCategories[number], then update ScopeDefinition.category to use ScopeCategory instead of string; update any catalog entries to use one of the allowed values and replace the duplicated category list in the picker component to import and use ScopeCategories/ScopeCategory for filtering and validation (refer to ScopeDefinition, category, ScopeCategories/ScopeCategory).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/routes/`(console)/project-[region]-[project]/overview/api-keys/scopes.svelte:
- Around line 173-180: The Checkbox control (Selector.Checkbox) currently only
renders the scope name/description while the deprecation notice is in a sibling
Badge, so assistive tech won't announce it; update the Checkbox to expose the
deprecated state by either appending " (Deprecated)" to the label or by adding a
description/aria-describedby that includes the deprecation text when
scope.deprecated is true (use scope.scope, scope.description, and
scope.deprecated to build the string), ensuring the Badge remains but is
associated via an id if using aria-describedby so screen readers hear the
deprecated status alongside the label/description; keep activeScopes binding
unchanged.
---
Nitpick comments:
In `@src/lib/constants.ts`:
- Around line 158-163: Change the free-form string "category" on the exported
type ScopeDefinition to a shared union type by introducing an exported const
array (e.g., ScopeCategories) and deriving a type alias ScopeCategory = typeof
ScopeCategories[number], then update ScopeDefinition.category to use
ScopeCategory instead of string; update any catalog entries to use one of the
allowed values and replace the duplicated category list in the picker component
to import and use ScopeCategories/ScopeCategory for filtering and validation
(refer to ScopeDefinition, category, ScopeCategories/ScopeCategory).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 80e69249-55ea-4ee3-96a0-2d0bb4f3f746
📒 Files selected for processing (3)
src/lib/constants.tssrc/routes/(console)/project-[region]-[project]/overview/(components)/keyDetails.sveltesrc/routes/(console)/project-[region]-[project]/overview/api-keys/scopes.svelte
Summary
Render deprecated API key scopes in the scope picker instead of filtering them out, and show a
Deprecatedbadge next to legacy entries.What Changed
ScopeDefinitiontype for API key scope definitionscollections.*,attributes.*, anddocuments.*as deprecatedDeprecatedbadge next to deprecated scope optionsWhy
Deprecated scopes were still valid for existing API keys, but the UI hid them during rendering. That made older scopes invisible in the picker and prevented clearly communicating their status to users.
Impact
Users can now see deprecated scopes during API key creation/editing, with a clear visual indication that those scopes are deprecated.
Validation
bun run lintpassed with existing repo warningsbun run checkfailed due existing repo-wide type errors unrelated to this diffbun run buildfailed due existing missing exports from@appwrite.io/consoleunrelated to this diffbun run testcould not run becausepackage.jsondoes not define atestscriptSummary by CodeRabbit