feat: Add folder system for feature flags (#271)#361
feat: Add folder system for feature flags (#271)#361FraktalDeFiDAO wants to merge 2 commits intodatabuddy-analytics:mainfrom
Conversation
- Add folder field to flags table with database index - Update ORPC API endpoints to support folder operations - Add folder grouping to flags list with expand/collapse - Add folder field to flag create/edit sheet - Support nested folders via path separator (e.g., auth/login) - Add folder selector component with create capability - Purely UI organization (no backend business logic changes) - TypeScript strict types throughout - Accessibility improvements (semantic button elements) Implements databuddy-analytics#271 Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
- Add folder creation/edit/delete UI modal - Update flag sheet to use FolderSelector component - Add folder management button to flags page - Group feature flags by folder in list view (existing) - Add folder icons and visual indicators - Support nested folders via path separator - Move flags to Uncategorized when folder deleted Closes databuddy-analytics#271 /claim databuddy-analytics#271 Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
|
|
@FraktalDeFiDAO is attempting to deploy a commit to the Databuddy OSS Team on Vercel. A member of the Team first needs to authorize it. |
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 adds a UI-only folder system for organizing feature flags, introducing new components ( Issues found:
Confidence Score: 2/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[User opens Flags Page] --> B{Flags exist?}
B -- No --> C[EmptyState]
B -- Yes --> D[Folder Management Toolbar]
D --> E[FlagsList grouped by folder]
E --> F[FolderSection - top-level folders]
F --> G[NestedFolderSection - child folders]
F --> H[FlagRow - direct flags]
E --> I[Uncategorized flags - no folder]
D --> J[Manage Folders button]
J --> K[FolderManagementModal]
K --> L[Create folder toast only]
K --> M[Rename folder - updateFlagMutation loop]
K --> N[Delete folder - updateFlagMutation loop\n⚠️ passes null, fails schema]
O[Flag Create/Edit Sheet] --> P[FolderSelector combobox]
P --> Q[Select existing folder]
P --> R[Create new folder from search\n⚠️ CommandEmpty never shows]
M --> S[(flags DB\nnew folder column)]
N --> S
O --> S
|
| dependencies: z.array(z.string()).optional(), | ||
| environment: z.string().optional(), | ||
| targetGroupIds: z.array(z.string()).optional(), | ||
| folder: z.string().max(200, "Folder path too long").optional(), |
There was a problem hiding this comment.
null folder value fails schema validation — delete-folder broken
updateFlagSchema declares folder as z.string().optional(), which accepts string | undefined but not null. However, handleDeleteFolder in page.tsx calls:
await updateFlagMutation.mutateAsync({ id: flag.id, folder: null });This will be rejected by Zod validation every time, meaning "Delete folder" silently fails (the error is caught and shows a toast, but flags are never moved to Uncategorized).
There is also a secondary issue in the update handler itself (line 577):
folder: input.folder ?? existingFlag[0].folder,The ?? operator treats null as a missing value, so even if null somehow slipped past the schema, the folder would not be cleared — it would fall back to the existing folder.
Fix both problems together:
| folder: z.string().max(200, "Folder path too long").optional(), | |
| folder: z.string().max(200, "Folder path too long").nullable().optional(), |
And update the handler (line 577):
folder: input.folder !== undefined ? input.folder : existingFlag[0].folder,The same nullable() fix is needed in flagFormSchema in packages/shared/src/flags/index.ts (line 65) so that clearing a folder via the form also works.
| </Button> | ||
| </PopoverTrigger> | ||
| <PopoverContent align="start" className="w-[300px] p-0"> | ||
| <Command shouldFilter={false}> |
There was a problem hiding this comment.
shouldFilter={false} disables search — create-from-search broken
Setting shouldFilter={false} tells cmdk to never hide any item, so no matter what the user types, all folder options remain visible and CommandEmpty never renders (it only renders when there are zero visible items). This means:
- Typing in the search box has no visible effect on the list.
- The "Create" button (rendered inside
CommandEmpty) is permanently inaccessible when any folder already exists, breaking the create-new-folder-from-search flow.
To fix this, manually filter folderOptions by searchValue:
| <Command shouldFilter={false}> | |
| <Command shouldFilter={false}> |
Keep shouldFilter={false} but also filter the displayed options and conditionally render the "Create" item:
// inside the component, derive filtered options
const filteredOptions = folderOptions.filter((o) =>
o.label.toLowerCase().includes(searchValue.toLowerCase())
);
const showCreate =
searchValue.trim() &&
onCreateFolder &&
!folderOptions.some((o) => o.value === searchValue.trim());Then render filteredOptions instead of folderOptions in the CommandGroup, and show the Create item whenever showCreate is true (not just inside CommandEmpty).
| "use client"; | ||
|
|
||
| import { | ||
| CaretDownIcon, | ||
| CaretRightIcon, | ||
| Folder, | ||
| FolderOpen, | ||
| } from "@phosphor-icons/react/dist/ssr"; | ||
| import { useState } from "react"; | ||
| import { cn } from "@/lib/utils"; |
There was a problem hiding this comment.
Dead code —
FolderTree component is not imported anywhere
folder-tree.tsx exports a FolderTree component that is never imported by any other file in the PR. It also contains the same useState misuse for building the folder tree (line ~47) that was flagged in folder-management-modal.tsx.
Consider removing this file if it is not yet needed, to keep the bundle clean and avoid confusion.
| @@ -687,6 +688,10 @@ export const flags = pgTable( | |||
| "btree", | |||
| table.createdBy.asc().nullsLast().op("text_ops") | |||
| ), | |||
| index("idx_flags_folder").using( | |||
| "btree", | |||
| table.folder.asc().nullsLast().op("text_ops") | |||
| ), | |||
| foreignKey({ | |||
There was a problem hiding this comment.
Missing database migration for new
folder column
A new folder column and idx_flags_folder index are added to the flags table, but no corresponding migration file appears in the PR diff. If Drizzle migrations are managed manually (rather than auto-applied), deploying this without the migration will cause runtime errors when any code tries to read or write the folder column.
Please add a Drizzle migration (e.g., drizzle-kit generate) and include it in this PR.
What Changed
Features
Technical Details
Closes #271
/claim #271