Skip to content

feat: Add folder system for feature flags (#271)#361

Open
FraktalDeFiDAO wants to merge 2 commits intodatabuddy-analytics:mainfrom
FraktalDeFiDAO:feature/feature-flag-folders
Open

feat: Add folder system for feature flags (#271)#361
FraktalDeFiDAO wants to merge 2 commits intodatabuddy-analytics:mainfrom
FraktalDeFiDAO:feature/feature-flag-folders

Conversation

@FraktalDeFiDAO
Copy link

What Changed

  • Added complete folder system for organizing feature flags
  • UI-only changes (no backend modifications required)

Features

  • Folder management modal (create, rename, delete)
  • Folder selector in flag create/edit sheet
  • Create new folders directly from flag form
  • Grouped list view by folder
  • Visual folder indicators
  • Support for nested folders
  • Flags moved to Uncategorized when folder deleted

Technical Details

  • Uses existing folder field in database
  • Leverages existing FolderSelector component
  • TypeScript strict types (no any/unknown)
  • Uses Phosphor icons
  • Mobile responsive

Closes #271

/claim #271

FraktalDeFiDAO and others added 2 commits March 23, 2026 18:30
- 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>
@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@vercel
Copy link

vercel bot commented Mar 24, 2026

@FraktalDeFiDAO is attempting to deploy a commit to the Databuddy OSS Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 24, 2026

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: da6c0634-ff5f-466e-a153-6192be858571

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 24, 2026

Greptile Summary

This PR adds a UI-only folder system for organizing feature flags, introducing new components (FolderManagementModal, FolderSelector, FolderTree), a grouped list view in FlagsList, and the backing folder column + index in the database schema. The approach of storing folders as plain strings on each flag (no separate folder entity) is clean and consistent with the existing schema.

Issues found:

  • Stale folder tree in FolderManagementModal — The folder tree is built inside a useState(() => ...) initializer, so it is computed once on mount and never recomputes when folders/flagCounts props change. After a rename or delete, the modal displays stale data. useMemo should be used instead. The same bug exists in the unused folder-tree.tsx.
  • null folder value fails schema validation — "Delete folder" is brokenupdateFlagSchema (and flagFormSchema) use .optional() without .nullable(), so folder: null is rejected by Zod. handleDeleteFolder passes folder: null to the mutation, meaning flags are never actually moved to "Uncategorized". The update handler also uses ?? which treats null as a missing value, so even if null bypassed the schema it still wouldn't clear the field. Both schemas need .nullable() and the handler needs input.folder !== undefined ? input.folder : existingFlag[0].folder.
  • FolderSelector search doesn't filtershouldFilter={false} disables cmdk's built-in filtering, but no manual filtering is applied to folderOptions. Typing in the search box has no effect, and CommandEmpty (which contains the only "Create" button) never renders when any folder exists, making the create-from-search flow inaccessible.
  • Missing database migrationschema.ts adds a folder column and index but no migration file is included. If migrations are managed with Drizzle, this will cause runtime errors on deploy.
  • Dead codefolder-tree.tsx is never imported anywhere and can be removed.

Confidence Score: 2/5

  • Not safe to merge — multiple primary user paths are broken by logic bugs.
  • Three independent P1 bugs break the core feature: (1) stale UI after mutations due to useState misuse, (2) "Delete folder" never succeeds because null is rejected by the Zod schema, and (3) the create-from-search flow in FolderSelector is completely inaccessible. A missing migration is a separate deployment risk. The grouped list view and rename flow work correctly, but the broken delete and search paths make the feature incomplete for production use.
  • folder-management-modal.tsx, folder-selector.tsx, packages/rpc/src/routers/flags.ts, packages/shared/src/flags/index.ts, and packages/db/src/drizzle/schema.ts (missing migration) require attention before merging.

Important Files Changed

Filename Overview
apps/dashboard/app/(main)/websites/[id]/flags/_components/folder-management-modal.tsx New component for folder CRUD; folder tree is built with useState initializer instead of useMemo, so it never reflects updates after rename/delete operations.
apps/dashboard/app/(main)/websites/[id]/flags/_components/folder-selector.tsx New combobox for selecting/creating folders; shouldFilter={false} disables built-in search filtering but no manual filter is applied, making search a no-op and the create-from-search flow inaccessible.
apps/dashboard/app/(main)/websites/[id]/flags/_components/folder-tree.tsx New FolderTree component that is never imported anywhere (dead code); also replicates the useState-initializer bug from folder-management-modal.tsx.
apps/dashboard/app/(main)/websites/[id]/flags/_components/flags-list.tsx Adds grouped folder view with collapsible FolderSection and NestedFolderSection components; countAllFlags helper is correct; minor fix to rule/rules pluralization.
apps/dashboard/app/(main)/websites/[id]/flags/page.tsx Adds folder management toolbar and wires handleRenameFolder/handleDeleteFolder; handleDeleteFolder passes folder: null which fails Zod validation (schema only accepts `string
packages/rpc/src/routers/flags.ts Adds folder field to create/update schemas and handlers; updateFlagSchema uses .optional() without .nullable(), and the update handler uses ?? instead of an explicit !== undefined check, making it impossible to clear a flag's folder via the API.
packages/shared/src/flags/index.ts Adds folder to flagFormSchema; refactors flagScheduleSchema branch order (logic preserved); same .optional() without .nullable() gap as in the RPC schema.
packages/db/src/drizzle/schema.ts Adds folder text column and idx_flags_folder B-tree index to the flags table; no accompanying migration file is included in the PR.

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
Loading

Comments Outside Diff (1)

  1. apps/dashboard/app/(main)/websites/[id]/flags/_components/folder-management-modal.tsx, line 449-487 (link)

    P1 useState misuse — folder tree never updates

    The folder tree is computed inside useState(() => ...) (an initializer function), so it is evaluated exactly once on initial render and frozen forever. When folders or flagCounts props change — e.g., after a rename or delete operation — the displayed tree will still show the stale data from the first mount.

    The exact same pattern appears in folder-tree.tsx (line ~47).

    Switch to useMemo so the tree is recomputed whenever its inputs change:

    Also add useMemo to the import at the top of the file.

Reviews (1): Last reviewed commit: "feat: Add folder system for feature flag..." | Re-trigger Greptile

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(),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 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:

Suggested change
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}>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 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:

  1. Typing in the search box has no visible effect on the list.
  2. 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:

Suggested change
<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).

Comment on lines +1 to +10
"use client";

import {
CaretDownIcon,
CaretRightIcon,
Folder,
FolderOpen,
} from "@phosphor-icons/react/dist/ssr";
import { useState } from "react";
import { cn } from "@/lib/utils";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 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.

Comment on lines 672 to 695
@@ -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({
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

🎯 Bounty: Feature Flag Folders for Organization

2 participants