-
Notifications
You must be signed in to change notification settings - Fork 10.8k
feat: admin feature flags v3 + assign/unassign #24428
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
WalkthroughAdds i18n strings feature_assigned_successfully and feature_unassigned_successfully. Introduces AssignFeatureSheet React component to assign or unassign a feature to teams, using debounced search, infinite pagination, and success/error toasts. Refactors FlagAdminList to group flags by type and integrate AssignFeatureSheet via a secondary action. Extends admin TRPC router with getTeamsForFeature (query), assignFeatureToTeam (mutation), and unassignFeatureFromTeam (mutation). Implements corresponding Zod schemas and handlers: getTeamsForFeature paginated search with hasFeature mapping, assignFeatureToTeam upsert on teamFeatures with assignedBy, and unassignFeatureFromTeam deletion via composite key. Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (9)
packages/features/flags/components/AssignFeatureSheet.tsx (4)
60-61
: Fix cache invalidation for infinite query (use broad invalidation)Passing only featureId may not invalidate queries with differing searchTerm/limit/pages. Invalidate all inputs for this procedure.
- utils.viewer.admin.getTeamsForFeature.invalidate({ featureId: flag.slug }); + utils.viewer.admin.getTeamsForFeature.invalidate();- utils.viewer.admin.getTeamsForFeature.invalidate({ featureId: flag.slug }); + utils.viewer.admin.getTeamsForFeature.invalidate();Also applies to: 70-71
102-103
: Localize the title text per TSX i18n guidelineAvoid hardcoded text. Use t().
As per coding guidelines
- <SheetTitle>Assign: {flag.slug}</SheetTitle> + <SheetTitle>{t("assign")}: {flag.slug}</SheetTitle>
99-99
: Pass through onOpenChange to preserve correct open/close semanticsCurrent handler always sets false. Prefer forwarding the boolean to the parent.
- <Sheet open={open} onOpenChange={handleClose}> + <Sheet open={open} onOpenChange={onOpenChange}>
125-131
: Nested interactive controls (button wrapping a checkbox) — a11y concernClickable button containing an interactive control can confuse screen readers/keyboard users. Consider rendering a non-button row (div/li with role="button") or moving the click handler to a non-interactive wrapper and keeping the checkbox as the control that triggers toggle.
Also applies to: 176-181
packages/trpc/server/routers/viewer/admin/unassignFeatureFromTeam.schema.ts (1)
3-6
: Tighten input validation (int/positive, trim/non-empty).Use stricter Zod validators to prevent invalid inputs.
export const ZAdminUnassignFeatureFromTeamSchema = z.object({ - teamId: z.number(), - featureId: z.string(), + teamId: z.number().int().positive(), + featureId: z.string().trim().min(1), });packages/trpc/server/routers/viewer/admin/assignFeatureToTeam.schema.ts (1)
3-6
: Tighten input validation (int/positive, trim/non-empty).Align with stricter constraints.
export const ZAdminAssignFeatureToTeamSchema = z.object({ - teamId: z.number(), - featureId: z.string(), + teamId: z.number().int().positive(), + featureId: z.string().trim().min(1), });packages/trpc/server/routers/viewer/admin/getTeamsForFeature.schema.ts (1)
3-8
: Harden pagination and input constraints.Ensure integer limit/cursor and trimmed search.
export const ZAdminGetTeamsForFeatureSchema = z.object({ featureId: z.string(), - limit: z.number().min(1).max(100).default(10), - cursor: z.number().nullish(), - searchTerm: z.string().optional(), + limit: z.number().int().min(1).max(100).default(10), + cursor: z.number().int().positive().nullish(), + searchTerm: z.string().trim().optional(), });packages/features/flags/components/FlagAdminList.tsx (2)
77-83
: Localize toast message (use t())Replace hard-coded string with a localized key (e.g., t("settings_updated_successfully")).
As per coding guidelines
49-56
: Accessibility/UX: add aria-label to icon button; disable switch while mutating
- Provide an aria-label or tooltip for the icon-only Button.
- Disable the Switch while the mutation is pending to avoid double toggles.
- const mutation = trpc.viewer.admin.toggleFeatureFlag.useMutation({ + const mutation = trpc.viewer.admin.toggleFeatureFlag.useMutation({ onSuccess: () => { - showToast("Flags successfully updated", "success"); + // showToast(t("settings_updated_successfully"), "success"); utils.viewer.features.list.invalidate(); utils.viewer.features.map.invalidate(); }, });- <Switch + <Switch defaultChecked={enabled} - onCheckedChange={(checked) => { + disabled={mutation.isPending} + onCheckedChange={(checked) => { mutation.mutate({ slug, enabled: checked }); }} />- <Button + <Button color="secondary" size="sm" variant="icon" + aria-label="Assign to teams" + tooltip="Assign to teams" onClick={() => handleAssignClick(flag)} StartIcon="users"></Button>As per coding guidelines
Also applies to: 85-91
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (10)
apps/web/public/static/locales/en/common.json
(1 hunks)packages/features/flags/components/AssignFeatureSheet.tsx
(1 hunks)packages/features/flags/components/FlagAdminList.tsx
(1 hunks)packages/trpc/server/routers/viewer/admin/_router.ts
(2 hunks)packages/trpc/server/routers/viewer/admin/assignFeatureToTeam.handler.ts
(1 hunks)packages/trpc/server/routers/viewer/admin/assignFeatureToTeam.schema.ts
(1 hunks)packages/trpc/server/routers/viewer/admin/getTeamsForFeature.handler.ts
(1 hunks)packages/trpc/server/routers/viewer/admin/getTeamsForFeature.schema.ts
(1 hunks)packages/trpc/server/routers/viewer/admin/unassignFeatureFromTeam.handler.ts
(1 hunks)packages/trpc/server/routers/viewer/admin/unassignFeatureFromTeam.schema.ts
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
**/*.ts
: For Prisma queries, only select data you need; never useinclude
, always useselect
Ensure thecredential.key
field is never returned from tRPC endpoints or APIs
Files:
packages/trpc/server/routers/viewer/admin/_router.ts
packages/trpc/server/routers/viewer/admin/assignFeatureToTeam.schema.ts
packages/trpc/server/routers/viewer/admin/unassignFeatureFromTeam.schema.ts
packages/trpc/server/routers/viewer/admin/getTeamsForFeature.schema.ts
packages/trpc/server/routers/viewer/admin/assignFeatureToTeam.handler.ts
packages/trpc/server/routers/viewer/admin/unassignFeatureFromTeam.handler.ts
packages/trpc/server/routers/viewer/admin/getTeamsForFeature.handler.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()
in hot paths like loops
Files:
packages/trpc/server/routers/viewer/admin/_router.ts
packages/trpc/server/routers/viewer/admin/assignFeatureToTeam.schema.ts
packages/trpc/server/routers/viewer/admin/unassignFeatureFromTeam.schema.ts
packages/trpc/server/routers/viewer/admin/getTeamsForFeature.schema.ts
packages/trpc/server/routers/viewer/admin/assignFeatureToTeam.handler.ts
packages/trpc/server/routers/viewer/admin/unassignFeatureFromTeam.handler.ts
packages/trpc/server/routers/viewer/admin/getTeamsForFeature.handler.ts
packages/features/flags/components/FlagAdminList.tsx
packages/features/flags/components/AssignFeatureSheet.tsx
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
packages/trpc/server/routers/viewer/admin/_router.ts
packages/trpc/server/routers/viewer/admin/assignFeatureToTeam.schema.ts
packages/trpc/server/routers/viewer/admin/unassignFeatureFromTeam.schema.ts
packages/trpc/server/routers/viewer/admin/getTeamsForFeature.schema.ts
packages/trpc/server/routers/viewer/admin/assignFeatureToTeam.handler.ts
packages/trpc/server/routers/viewer/admin/unassignFeatureFromTeam.handler.ts
packages/trpc/server/routers/viewer/admin/getTeamsForFeature.handler.ts
packages/features/flags/components/FlagAdminList.tsx
packages/features/flags/components/AssignFeatureSheet.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Always use
t()
for text localization in frontend code; direct text embedding should trigger a warning
Files:
packages/features/flags/components/FlagAdminList.tsx
packages/features/flags/components/AssignFeatureSheet.tsx
🧬 Code graph analysis (6)
packages/trpc/server/routers/viewer/admin/_router.ts (3)
packages/trpc/server/routers/viewer/admin/getTeamsForFeature.schema.ts (1)
ZAdminGetTeamsForFeatureSchema
(3-8)packages/trpc/server/routers/viewer/admin/assignFeatureToTeam.schema.ts (1)
ZAdminAssignFeatureToTeamSchema
(3-6)packages/trpc/server/routers/viewer/admin/unassignFeatureFromTeam.schema.ts (1)
ZAdminUnassignFeatureFromTeamSchema
(3-6)
packages/trpc/server/routers/viewer/admin/assignFeatureToTeam.handler.ts (2)
packages/prisma/index.ts (1)
PrismaClient
(84-84)packages/trpc/server/routers/viewer/admin/assignFeatureToTeam.schema.ts (1)
TAdminAssignFeatureToTeamSchema
(8-8)
packages/trpc/server/routers/viewer/admin/unassignFeatureFromTeam.handler.ts (1)
packages/trpc/server/routers/viewer/admin/unassignFeatureFromTeam.schema.ts (1)
TAdminUnassignFeatureFromTeamSchema
(8-8)
packages/trpc/server/routers/viewer/admin/getTeamsForFeature.handler.ts (2)
packages/prisma/index.ts (1)
PrismaClient
(84-84)packages/trpc/server/routers/viewer/admin/getTeamsForFeature.schema.ts (1)
TAdminGetTeamsForFeatureSchema
(10-10)
packages/features/flags/components/FlagAdminList.tsx (3)
packages/ui/components/list/List.tsx (4)
List
(15-31)ListItem
(37-66)ListItemTitle
(126-140)ListItemText
(142-156)packages/ui/components/button/Button.tsx (1)
Button
(221-349)packages/features/flags/components/AssignFeatureSheet.tsx (1)
AssignFeatureSheet
(31-208)
packages/features/flags/components/AssignFeatureSheet.tsx (3)
packages/trpc/react/trpc.ts (2)
RouterOutputs
(143-143)trpc
(54-138)packages/lib/hooks/useDebounce.ts (1)
useDebounce
(3-17)packages/ui/components/toast/showToast.tsx (1)
showToast
(77-101)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Install dependencies / Yarn install & cache
🔇 Additional comments (3)
packages/trpc/server/routers/viewer/admin/_router.ts (1)
66-83
: Approve admin feature routes: handlers comply with select-only queries and do not expose credential.keyapps/web/public/static/locales/en/common.json (1)
1179-1180
: i18n keys added look goodPlease ensure these keys are added to other locales to avoid fallback/missing strings at runtime.
packages/trpc/server/routers/viewer/admin/assignFeatureToTeam.handler.ts (1)
14-36
: Ensure admin-only protection and use only named export
- Confirm this handler is registered via an adminProcedure in your router to prevent unauthorized access.
- Remove the default export—keep only the named export to align with our named-exports guideline.
What does this PR do?
This PR adds the ability for admins of a instance to assign/unassign feature to specific teams. Plus a v3 redesign of the admin features page
Visual Demo (For contributors especially)
How should this be tested?
Login as admin
View features page
Enable/disable specific features for a team