feat: Feature Flag Folders for Organization#340
feat: Feature Flag Folders for Organization#340cd333c wants to merge 2 commits intodatabuddy-analytics:mainfrom
Conversation
- Add 'folder' text field to flags table schema with index - Add folder parameter to list/create/update API endpoints - Add folder field to shared flagFormSchema - Create FolderTree sidebar component with: - Hierarchical tree view (nested via path separator '/') - All Flags / Uncategorized quick filters - Inline create, rename (double-click), delete with confirmation - Flag count per folder (including nested) - Mobile-responsive collapsible sidebar - Add folder grouping to FlagsList with collapsible sections - Add FolderBadge to flag rows showing assigned folder - Add folder selector with autocomplete to FlagSheet (create/edit) - Integrate FolderTree into FlagsPage with sidebar layout - Folder rename/delete batch-updates flags via existing API - UI-only organization: no changes to flag evaluation or SDK behavior Closes databuddy-analytics#271
|
|
|
@cd333c 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 implements a folder/organization system for feature flags by adding an optional Key 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[Empty State]
B -- Yes --> D[Render FolderTree sidebar + FlagsList]
D --> E[FolderTree sidebar]
E --> F[All Flags — null]
E --> G[Uncategorized — empty string]
E --> H[Named folder node]
F --> I[selectedFolder = null]
G --> J[selectedFolder = empty string]
H --> K[selectedFolder = path string]
I --> L[filteredFlags = all active flags]
J --> M[filteredFlags = flags where folder is null]
K --> N["filteredFlags = flags where folder === selectedFolder\n⚠️ does NOT include nested sub-folder flags"]
H --> O{Rename folder?}
O -- Yes --> P["batch update: flags.update\nfor each affected flag"]
P --> Q[invalidateQueries]
H --> R{Delete folder?}
R -- Yes --> S["batch update: folder = null\nfor each affected flag"]
S --> Q
D --> T[FlagSheet open?]
T -- Yes --> U["Folder input with autocomplete\n(derived from existing flagsList)"]
U --> V["flags.create / flags.update\nwith folder field"]
V --> Q
Last reviewed commit: d1f831b |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const treeContent = ( | ||
| <FolderTreeContent | ||
| flags={flags} | ||
| onDeleteFolderAction={onDeleteFolderAction} | ||
| onRenameFolderAction={onRenameFolderAction} | ||
| onSelectFolderAction={onSelectFolderAction} | ||
| selectedFolder={selectedFolder} | ||
| websiteId={websiteId} | ||
| /> | ||
| ); |
There was a problem hiding this comment.
Undefined variable websiteId passed to FolderTreeContent
The FolderTree function destructures the websiteId prop as websiteId: _websiteId (aliased to suppress the unused variable warning). However, on line 491 the JSX still references websiteId — a name that is no longer in scope — rather than _websiteId. This will cause a ReferenceError at runtime and is also a TypeScript compile error.
Additionally, FolderTreeContent is typed as Omit<FolderTreeProps, "websiteId">, so it does not even accept a websiteId prop. The prop should simply be removed from the treeContent JSX entirely.
| const treeContent = ( | |
| <FolderTreeContent | |
| flags={flags} | |
| onDeleteFolderAction={onDeleteFolderAction} | |
| onRenameFolderAction={onRenameFolderAction} | |
| onSelectFolderAction={onSelectFolderAction} | |
| selectedFolder={selectedFolder} | |
| websiteId={websiteId} | |
| /> | |
| ); | |
| const treeContent = ( | |
| <FolderTreeContent | |
| flags={flags} | |
| onDeleteFolderAction={onDeleteFolderAction} | |
| onRenameFolderAction={onRenameFolderAction} | |
| onSelectFolderAction={onSelectFolderAction} | |
| selectedFolder={selectedFolder} | |
| /> | |
| ); |
| const filteredFlags = useMemo(() => { | ||
| if (selectedFolder === null) { | ||
| return activeFlags; | ||
| } | ||
|
|
||
| if (selectedFolder === "") { | ||
| return activeFlags.filter((flag) => !flag.folder); | ||
| } | ||
|
|
||
| return activeFlags.filter((flag) => flag.folder === selectedFolder); | ||
| }, [activeFlags, selectedFolder]); |
There was a problem hiding this comment.
Selected parent folder doesn't show flags in sub-folders
When a user clicks a parent folder (e.g. "auth") in the sidebar, filteredFlags uses an exact equality check:
return activeFlags.filter((flag) => flag.folder === selectedFolder);This only returns flags assigned directly to "auth", not flags in "auth/login" or any other nested path. Meanwhile, the sidebar's countFlagsInFolder helper includes nested flags in its count via a startsWith check. The result is a confusing mismatch — the sidebar badge may show 5 flags but the main list only renders 2.
To be consistent with the sidebar counts, the filter should include nested children:
| const filteredFlags = useMemo(() => { | |
| if (selectedFolder === null) { | |
| return activeFlags; | |
| } | |
| if (selectedFolder === "") { | |
| return activeFlags.filter((flag) => !flag.folder); | |
| } | |
| return activeFlags.filter((flag) => flag.folder === selectedFolder); | |
| }, [activeFlags, selectedFolder]); | |
| const filteredFlags = useMemo(() => { | |
| if (selectedFolder === null) { | |
| return activeFlags; | |
| } | |
| if (selectedFolder === "") { | |
| return activeFlags.filter((flag) => !flag.folder); | |
| } | |
| return activeFlags.filter( | |
| (flag) => | |
| flag.folder === selectedFolder || | |
| flag.folder?.startsWith(`${selectedFolder}/`), | |
| ); | |
| }, [activeFlags, selectedFolder]); |
| if (input.folder !== undefined) { | ||
| if (input.folder === null || input.folder === "") { | ||
| conditions.push(isNull(flags.folder)); | ||
| } else { | ||
| conditions.push(eq(flags.folder, input.folder)); | ||
| } |
There was a problem hiding this comment.
Unreachable null branch in folder filter
listFlagsSchema defines folder as z.string().optional(), meaning validated input can only ever be string | undefined — it can never be null. The input.folder === null branch at line 301 is therefore dead code and will never execute.
If the intent is to allow callers to explicitly filter for uncategorized flags (no folder), the schema should be updated to accept null:
| if (input.folder !== undefined) { | |
| if (input.folder === null || input.folder === "") { | |
| conditions.push(isNull(flags.folder)); | |
| } else { | |
| conditions.push(eq(flags.folder, input.folder)); | |
| } | |
| if (input.folder !== undefined) { | |
| if (input.folder === "") { | |
| conditions.push(isNull(flags.folder)); | |
| } else { | |
| conditions.push(eq(flags.folder, input.folder)); | |
| } | |
| } |
Or, if null filtering is desired via the API, change listFlagsSchema to folder: z.string().nullable().optional().
| const label = | ||
| folderPath === null | ||
| ? "Uncategorized" | ||
| : folderPath.includes("/") | ||
| ? folderPath | ||
| : folderPath; | ||
|
|
There was a problem hiding this comment.
Dead branch in label ternary
Both branches of the inner ternary evaluate to the same value (folderPath), making the folderPath.includes("/") check completely redundant:
const label =
folderPath === null
? "Uncategorized"
: folderPath.includes("/")
? folderPath // same as ↓
: folderPath; // same as ↑This can be simplified:
| const label = | |
| folderPath === null | |
| ? "Uncategorized" | |
| : folderPath.includes("/") | |
| ? folderPath | |
| : folderPath; | |
| const label = folderPath === null ? "Uncategorized" : folderPath; |
2 participants
🎯 Feature Flag Folders for Organization
Implements #271 — adds a folder system to organize feature flags in the dashboard UI.
Implementation: Option A (Simple String Field)
Added an optional
foldertext field to theflagstable, storing folder paths as strings (e.g.,"auth/login","checkout/payment"). This allows nested folders via path separator without requiring a separate table.Changes
Backend
packages/db/src/drizzle/schema.ts): Addedfoldertext column with btree indexpackages/rpc/src/routers/flags.ts): AddedfoldertolistFlagsSchema(filter),createFlagSchema, andupdateFlagSchemapackages/shared/src/flags/index.ts): AddedfoldertoflagFormSchemaDashboard UI
New: Folder Tree Sidebar (
folder-tree.tsx)Updated: Flags List (
flags-list.tsx)Updated: Flag Sheet (
flag-sheet.tsx)Updated: Flags Page (
page.tsx)Design Decisions
/separator for hierarchy (e.g.,auth/login)flags.updateendpointfolderfield has zero impact on flag evaluation, SDK behavior, or API responses for external consumersTechnical Notes
Testing
flags.listworks with cache key differentiationAfter merge
Run
bun run db:pushto apply the schema change.Closes #271