Skip to content

feat: Feature Flag Folders for Organization (#271)#350

Open
mendarb wants to merge 2 commits intodatabuddy-analytics:mainfrom
mendarb:feat/flag-folders-271
Open

feat: Feature Flag Folders for Organization (#271)#350
mendarb wants to merge 2 commits intodatabuddy-analytics:mainfrom
mendarb:feat/flag-folders-271

Conversation

@mendarb
Copy link

@mendarb mendarb commented Mar 20, 2026

Summary

Adds a folder system to organize feature flags in the dashboard UI. This is purely UI organization - flag evaluation, dependencies, API responses, and SDK behavior remain unchanged.

  • Database: Added optional folder text field to flags table with flags_folder_idx index for query performance
  • API: Extended flags.list with optional folder filter parameter. Added folder to flags.create and flags.update schemas (max 200 chars)
  • Dashboard UI:
    • Folder input field in flag create/edit sheet (with FolderIcon)
    • Flags list groups flags by folder with collapsible headers showing flag count
    • Folder badge displayed on each flag row
    • "Uncategorized" section for flags without folders
    • Fully backward compatible - works without any folders assigned
  • Tests: Validation tests for folder field, filtering, grouping logic

Design Choices

  • Option A (simple string field) as recommended in the issue. Folder paths like auth/login or checkout/payment are stored directly on the flag. Simple, no extra tables needed, supports nested paths via path separator.
  • Folder UI only appears when flags have folders assigned - no visual noise when feature is unused
  • Collapsible folder sections for managing large flag lists
  • Uses Phosphor FolderIcon (duotone weight), existing UI components, rounded classes

Business Logic Unchanged

  • Flag evaluation logic is completely untouched
  • API responses for SDKs include folder as metadata only
  • Dependencies work regardless of folders
  • Flag keys, values, and behavior are unaffected

AI Disclosure

This PR was created with AI assistance (Claude Code). All code has been reviewed and verified.

Closes #271

Implements feature flag folders (databuddy-analytics#271) using Option A (simple string field):

- Database: added optional `folder` text field to `flags` table with index
- API: added `folder` filter to `flags.list`, `folder` field to create/update schemas
- Dashboard: folder input in flag create/edit sheet, folder grouping in flags list
  with collapsible sections, folder badge on flag rows
- Shared: added `folder` to flagFormSchema for form validation
- Tests: validation tests for folder field, grouping logic, filter operations

Backward compatible - existing flags work without folders (shown as "Uncategorized").
Flag evaluation logic is unchanged; folder is UI-only metadata.

Closes databuddy-analytics#271

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@vercel
Copy link

vercel bot commented Mar 20, 2026

@mendarb 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 20, 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: 0b8b9c81-0fa5-42ef-a171-5dabbf834750

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
📝 Coding Plan
  • Generate coding plan for human review comments

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.

Tip

CodeRabbit can generate a title for your PR based on the changes.

Add @coderabbitai placeholder anywhere in the title of your PR and CodeRabbit will replace it with a title based on the changes in the PR. You can change the placeholder by changing the reviews.auto_title_placeholder setting.

@CLAassistant
Copy link

CLAassistant commented Mar 20, 2026

CLA assistant check
All committers have signed the CLA.

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 20, 2026

Greptile Summary

This PR introduces a folder organization system for feature flags — a purely UI/metadata change that adds a nullable folder string column to the flags table, threads it through the RPC layer (list filtering, create, update), and renders collapsible folder sections in the dashboard. The overall design is sound and backward-compatible, but two bugs were introduced:

  • Critical data-loss bug in flag-sheet.tsx: Line 294 has a corrupted property name (tttttfolder instead of folder) combined with targetGroupIds on the same line. This means the folder field is never pre-populated when editing an existing flag, and — more importantly — every save sends folder: null to the API, silently clearing the folder value from the database even when the user made no folder-related changes.
  • Logic gap in the flag restore path (flags.ts): When create is called for a key that matches a soft-deleted flag, the handler runs a restore UPDATE. The new folder field was only added to the fresh INSERT path, not to this restore path, so the provided folder is silently ignored during restore operations.

Confidence Score: 1/5

  • Not safe to merge — contains a critical data-loss bug that silently clears folder values in the database on every flag edit.
  • The tttttfolder typo in flag-sheet.tsx is a critical regression: it causes the folder column to be set to null in the database on every flag update, destroying any folder assignment the user had made. This needs to be fixed before merging. The restore-path omission is a secondary logic bug that also needs fixing.
  • apps/dashboard/app/(main)/websites/[id]/flags/_components/flag-sheet.tsx (line 294 — critical data-loss) and packages/rpc/src/routers/flags.ts (lines 618-640 — missing folder in restore path).

Important Files Changed

Filename Overview
apps/dashboard/app/(main)/websites/[id]/flags/_components/flag-sheet.tsx Contains a critical corruption at line 294 where tttttfolder is used instead of folder, preventing folder population on edit and causing silent folder data loss on every save.
packages/rpc/src/routers/flags.ts Folder field correctly integrated into list (cache key + DB filter), create (insert), and update (spread) paths. However, the soft-delete restore path is missing the folder field, so folder is silently ignored when restoring a previously-deleted flag key.
apps/dashboard/app/(main)/websites/[id]/flags/_components/flags-list.tsx Folder grouping, collapsible sections, and folder badge rendering look correct. Uses __uncategorized sentinel key to avoid naming collisions with actual folders.
packages/db/src/drizzle/schema.ts Adds nullable folder text column and a btree index (flags_folder_idx) on the column. Schema change looks correct; no migration file is present in the repo (project appears to use drizzle-kit push rather than migration files).
packages/rpc/src/routers/flags-folders.test.ts New test file validating folder schema and grouping logic. Tests cover valid paths, null/undefined, max-length rejection, and alphabetical sort — good coverage for the pure-logic parts.
packages/shared/src/flags/index.ts Adds folder (max 200 chars, nullable/optional) to flagFormSchema. Correctly aligned with the DB schema and API schemas.
apps/dashboard/app/(main)/websites/[id]/flags/_components/types.ts Adds `folder?: string

Sequence Diagram

sequenceDiagram
    participant UI as FlagSheet (UI)
    participant RPC as flags.ts (RPC)
    participant DB as PostgreSQL

    Note over UI,DB: Edit existing flag (current broken state)
    UI->>UI: form.reset({ tttttfolder: flag.folder }) ← bug: folder NOT set
    UI->>UI: User submits (data.folder = undefined)
    UI->>RPC: updateMutation({ folder: null })
    RPC->>DB: UPDATE flags SET folder = NULL ← data loss!

    Note over UI,DB: Create flag (folder applies only to fresh insert)
    UI->>RPC: createMutation({ folder: "auth/login" })
    alt fresh insert
        RPC->>DB: INSERT INTO flags (folder) VALUES ('auth/login') ✓
    else restoring soft-deleted flag
        RPC->>DB: UPDATE flags SET name=..., environment=... ← folder missing! ✗
    end

    Note over UI,DB: List flags (correct)
    UI->>RPC: list({ folder: "auth" })
    RPC->>DB: SELECT * FROM flags WHERE folder = 'auth'
    DB-->>RPC: filtered flags
    RPC-->>UI: flags[]
    UI->>UI: groupedFlags by folder → collapsible FolderHeaders
Loading

Comments Outside Diff (1)

  1. packages/rpc/src/routers/flags.ts, line 618-640 (link)

    P1 folder not applied when restoring a soft-deleted flag

    The create handler has two execution paths: a fresh INSERT (line 671+) and an UPDATE that restores a soft-deleted flag with the same key (lines 619-659). The folder: input.folder || null field was only added to the fresh insert path. The restore .set() block does not include folder, so if a user creates a flag with a folder path and there is a previously-deleted flag with the same key, the folder value is silently dropped during the restore.

    Add folder to the restore .set() call alongside the other fields:

    .set({
      name: input.name,
      description: input.description,
      // ...existing fields...
      environment: input.environment,
      folder: input.folder || null,   // ← add this
      deletedAt: null,
      updatedAt: new Date(),
    })

Last reviewed commit: "feat: add folder org..."

dependencies: flag.dependencies ?? [],
environment: flag.environment || undefined,
targetGroupIds: extractTargetGroupIds(),
tttttfolder: flag.folder || undefined, targetGroupIds: extractTargetGroupIds(),
Copy link
Contributor

Choose a reason for hiding this comment

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

P0 Corrupted property name causes folder data loss on edit

Line 294 contains tttttfolder as the property name instead of folder, and both folder and targetGroupIds are merged onto a single line. This is a clear edit corruption.

The consequence is two-fold:

  1. When opening an existing flag for editing, the folder field is never pre-populated (it always shows blank even if the flag has a folder set).
  2. More critically, when the user saves, data.folder is undefined. Line 404 then evaluates data.folder?.trim() || nullnull, so the update mutation is called with folder: null. This silently clears the folder in the database on every save, regardless of whether the user intended to change it.
Suggested change
tttttfolder: flag.folder || undefined, targetGroupIds: extractTargetGroupIds(),
folder: flag.folder || undefined,
targetGroupIds: extractTargetGroupIds(),

- Fix corrupted `tttttfolder` property name to `folder` in flag-sheet.tsx,
  preventing silent data loss that cleared folder on every flag edit
- Add missing `folder` field to soft-delete restore path in flags.ts,
  so folder value is applied when restoring a previously-deleted flag key

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@mendarb
Copy link
Author

mendarb commented Mar 21, 2026

Fixed both Greptile findings in 20eda0d:

  1. tttttfolder typo (flag-sheet.tsx L294) — renamed to folder so the folder value is no longer silently cleared on every flag save.
  2. Missing folder in restore path (flags.ts) — added folder: input.folder || null to the soft-delete restore .set() call so folder assignment survives flag key reuse.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

🎯 Bounty: Feature Flag Folders for Organization

2 participants