fix: 🎯 Bounty: Feature Flag Folders for Organization#322
fix: 🎯 Bounty: Feature Flag Folders for Organization#322makaiachildress-web wants to merge 1 commit intodatabuddy-analytics:mainfrom
Conversation
) Add a folder system to organize feature flags in the dashboard UI. This is purely for UI organization and does not affect flag evaluation, dependencies, API responses, or SDK behavior. Changes: - Add optional `folder` text field to flags table with index - Extend list/create/update API endpoints with folder support - Add folder validation (alphanumeric, hyphens, underscores, slashes, spaces) - Create FolderSidebar component with tree view, create/rename/delete - Add collapsible folder sections to FlagsList with flag counts - Add folder selector with quick-pick chips to flag create/edit sheet - Add Jotai atoms for folder selection and collapse state - Add 49 tests for folder schema validation and API contracts Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Makaia Taye Childress seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
|
Someone 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 You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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 |
|
Related Documentation Checked 1 published document(s) in 1 knowledge base(s). No updates required. How did I do? Any feedback? |
Greptile SummaryAdds feature flag folder organization — flags can be assigned to hierarchical folder paths (e.g.,
Confidence Score: 3/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[FlagsPage] -->|flags list| B[FolderSidebar]
A -->|filtered flags| C[FlagsList]
A -->|flag edit/create| D[FlagSheet]
B -->|selectedFolderAtom| E[Jotai Store]
B -->|collapsedFoldersAtom| E
C -->|collapsedFoldersAtom| E
B -->|onMoveFlagsToRoot| F[updateFlagMutation]
B -->|onRenameFolderFlags| F
F -->|mutate folder: null| G[flags.update RPC]
F -->|mutate folder: newPath| G
D -->|create/update with folder| H[flags.create / flags.update RPC]
G --> I[(DB: flags.folder column)]
H --> I
C -->|hasFolders?| J{Group by folder}
J -->|yes| K[FolderSection per group]
J -->|no| L[Flat FlagRow list]
K --> M[FlagRow per flag]
Last reviewed commit: 6d4bffc |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const handleMoveFlagsToRoot = useCallback( | ||
| (folderPath: string) => { | ||
| const flagsInFolder = activeFlags.filter( | ||
| (f) => | ||
| (f as Flag).folder === folderPath || | ||
| (f as Flag).folder?.startsWith(`${folderPath}/`) | ||
| ); | ||
| for (const flag of flagsInFolder) { | ||
| updateFlagMutation.mutate({ | ||
| id: flag.id, | ||
| folder: null, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Batch mutations have no error handling
handleMoveFlagsToRoot calls updateFlagMutation.mutate() in a synchronous loop for every flag in the folder. If any individual mutation fails (e.g., network error, validation failure), there's no error handling — the failure is silently swallowed and some flags may be moved while others aren't, leaving the folder in a partial state. The same issue applies to handleRenameFolderFlags below.
Consider using mutateAsync with Promise.allSettled (or a sequential await loop) so failures can be surfaced to the user:
const handleMoveFlagsToRoot = useCallback(
async (folderPath: string) => {
const flagsInFolder = activeFlags.filter(
(f) =>
(f as Flag).folder === folderPath ||
(f as Flag).folder?.startsWith(`${folderPath}/`)
);
const results = await Promise.allSettled(
flagsInFolder.map((flag) =>
updateFlagMutation.mutateAsync({ id: flag.id, folder: null })
)
);
const failures = results.filter((r) => r.status === "rejected");
if (failures.length > 0) {
toast.error(`Failed to move ${failures.length} flag(s)`);
}
},
[activeFlags, updateFlagMutation]
);
| const handleDeleteFolder = useCallback( | ||
| (path: string) => { | ||
| onMoveFlagsToRoot(path); | ||
| if (selectedFolder === path) { | ||
| setSelectedFolder(null); | ||
| } | ||
| toast.success(`Folder "${path}" deleted — flags moved to root`); | ||
| }, | ||
| [onMoveFlagsToRoot, selectedFolder, setSelectedFolder] | ||
| ); |
There was a problem hiding this comment.
Destructive action without confirmation dialog
handleDeleteFolder immediately moves all flags to root and shows a success toast without asking the user to confirm. Per the project's UI guidelines, destructive or irreversible actions must use an AlertDialog. A folder with many nested flags could be unintentionally destroyed with a single click — and there's no undo mechanism.
Consider wrapping this in a confirmation dialog similar to the DeleteDialog already used for flag deletion on the same page.
Context Used: Context from dashboard - .cursor/rules/ui-guidelines.mdc (source)
| <Button | ||
| className="size-6" | ||
| onClick={() => setIsCreating(true)} | ||
| size="icon" | ||
| variant="ghost" | ||
| > | ||
| <FolderPlusIcon className="size-3.5" weight="duotone" /> | ||
| </Button> |
There was a problem hiding this comment.
Missing aria-label on icon-only button
This icon-only button has no accessible label. Per the project's UI and accessibility guidelines, icon-only buttons must include an aria-label.
| <Button | |
| className="size-6" | |
| onClick={() => setIsCreating(true)} | |
| size="icon" | |
| variant="ghost" | |
| > | |
| <FolderPlusIcon className="size-3.5" weight="duotone" /> | |
| </Button> | |
| <Button | |
| aria-label="Create folder" | |
| className="size-6" | |
| onClick={() => setIsCreating(true)} | |
| size="icon" | |
| variant="ghost" | |
| > | |
| <FolderPlusIcon className="size-3.5" weight="duotone" /> | |
| </Button> |
Context Used: Context from dashboard - .cursor/rules/ui-guidelines.mdc (source)
|
|
||
| const FOLDER_NAME_REGEX = /^[a-zA-Z0-9_\-/ ]*$/; | ||
|
|
There was a problem hiding this comment.
Constant declared between import groups
FOLDER_NAME_REGEX is declared between two import blocks (line 12, sandwiched between the phosphor-icons import and framer-motion import). This breaks the standard convention of grouping all imports together at the top of the file before any declarations.
| const FOLDER_NAME_REGEX = /^[a-zA-Z0-9_\-/ ]*$/; | |
| import { AnimatePresence, motion } from "framer-motion"; |
Move the constant after all imports:
import type { Flag } from "./types";
const FOLDER_NAME_REGEX = /^[a-zA-Z0-9_\-/ ]*$/;Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
3 participants
Summary\n\nFixes #271\n\n## Changes\n\nImplemented fix for the issue described above.\n\n/claim #271