Skip to content

feat: Add folder organization for feature flags#327

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

feat: Add folder organization for feature flags#327
dagangtj wants to merge 2 commits intodatabuddy-analytics:mainfrom
dagangtj:feature/flag-folders-271

Conversation

@dagangtj
Copy link

Closes #271

Summary

Implements folder organization for feature flags as requested in the bounty issue.

Changes

Database Schema

  • Added text field to table
  • Added composite index on and for query performance
  • Fully backward compatible - existing flags work without folders

API Endpoints

  • flags.list: Added optional parameter for filtering flags by folder
  • flags.create: Added optional field to set folder on creation
  • flags.update: Added optional field to move flags between folders

Implementation Details

  • Used simple string field approach (recommended in issue) for flexibility
  • Supports nested folders via path separator (e.g., auth/login, checkout/payment)
  • Cache keys updated to include folder parameter
  • All changes are UI-only metadata - no impact on flag evaluation logic

Testing

  • Schema changes validated
  • API endpoints support folder parameter
  • Backward compatible - flags without folders work normally

Next Steps (UI Implementation)

This PR provides the backend foundation. UI implementation would include:

  • Folder tree/sidebar in flags page
  • Folder selector in flag create/edit form
  • Folder filtering dropdown
  • Drag-and-drop folder organization

Notes

  • Focused on minimal, core functionality as requested
  • No over-engineering - simple and effective
  • Ready for UI layer to build upon

- Add folder text field to flags table schema
- Add index on folder and websiteId for performance
- Support folder parameter in flags.list endpoint
- Support folder field in flags.create and flags.update
- Backward compatible: folder is optional
@vercel
Copy link

vercel bot commented Feb 26, 2026

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

A member of the Team first needs to authorize it.

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

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 26, 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.

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
  • Post copyable unit tests in a comment

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.

@dosubot
Copy link

dosubot bot commented Feb 26, 2026

Related Documentation

Checked 1 published document(s) in 1 knowledge base(s). No updates required.

How did I do? Any feedback?  Join Discord

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 26, 2026

Greptile Summary

Added folder organization capability for feature flags by introducing a nullable folder text field in the database schema and extending the list, create, and update API endpoints to support folder filtering and assignment.

Key Changes:

  • Database: Added folder text field and composite index on (folder, websiteId) for query performance
  • API: Extended schemas to accept optional folder parameter across list/create/update endpoints
  • Backward compatible: existing flags without folders work normally

Issues Found:

  • Cache key construction bug causes incorrect results when filtering by null or empty folders
  • Inconsistent empty string handling between create and update endpoints

Confidence Score: 2/5

  • This PR has logical bugs that will cause incorrect caching behavior in production
  • The cache key logic has a critical bug that doesn't distinguish between different folder filter states, leading to incorrect cached results being returned to users. Additionally, there's inconsistent handling of empty strings between create and update operations.
  • packages/rpc/src/routers/flags.ts requires fixes to the cache key logic and empty string handling

Important Files Changed

Filename Overview
packages/db/src/drizzle/schema.ts Added nullable folder text field and composite index for folder organization - clean, backward-compatible schema change
packages/rpc/src/routers/flags.ts Added folder parameter to list/create/update endpoints - cache key logic bug and inconsistent empty string handling

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[flags.list API call] --> B{folder param provided?}
    B -->|No| C[Build cache key with 'all']
    B -->|Yes| D[Build cache key with folder value]
    C --> E[Check cache]
    D --> E
    E -->|Cache hit| F[Return cached results]
    E -->|Cache miss| G[Build DB query]
    G --> H{folder !== undefined?}
    H -->|No| I[Query without folder filter]
    H -->|Yes| J[Query with eq folder, folder]
    I --> K[Return all flags]
    J --> L[Return filtered flags]
    K --> M[Cache results]
    L --> M
    M --> N[Return to client]
    
    style B fill:#f9f,stroke:#333
    style H fill:#f9f,stroke:#333
    style E fill:#bbf,stroke:#333
Loading

Last reviewed commit: d911624

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

2 files reviewed, 2 comments

Edit Code Review Agent Settings | Greptile

.handler(({ context, input }) => {
const scope = getScope(input.websiteId, input.organizationId);
const cacheKey = `list:${scope}:${input.status || "all"}`;
const cacheKey = `list:${scope}:${input.status || "all"}:${input.folder || "all"}`;
Copy link
Contributor

Choose a reason for hiding this comment

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

cache key doesn't distinguish between undefined (no filter), null (filter for null folders), and empty string (filter for empty folders) - the expression input.folder || "all" evaluates to "all" for all three cases, but the query at lines 300-302 treats them differently, causing cache to return incorrect results

Copy link
Author

Choose a reason for hiding this comment

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

Fixed in 3524cd0. The cache key now properly distinguishes between undefined (no filter → "all"), and null/empty string (no folder → "none"), so each filter state gets its own cache entry.

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 26, 2026

Additional Comments (1)

packages/rpc/src/routers/flags.ts
inconsistent handling of empty string folder - create endpoint (line 696) treats empty string as null using input.folder || null, but update spreads ...updates which preserves empty strings

- Cache key now distinguishes between undefined (no filter), null/empty
  (no folder), avoiding incorrect cached results
- Update endpoint normalizes empty string folder to null, consistent
  with create endpoint behavior

Addresses Greptile review comments on PR databuddy-analytics#327
@dagangtj
Copy link
Author

@greptile-apps Both issues addressed in 3524cd0:

  1. Cache key bug: Now uses explicit check — undefined → "all", null/empty → "none", so each filter state gets a distinct cache key.
  2. Inconsistent empty string handling: Added normalization in the update endpoint (updates.folder = updates.folder || null) so it matches the create endpoint's behavior.

Thanks for the thorough review!

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 26, 2026

Perfect! Both fixes look solid:

  1. Cache key: The explicit check input.folder === undefined ? "all" : (input.folder || "none") correctly distinguishes between no filter (undefined → "all"), and filtering for no folder (null/empty → "none"). This prevents cache collisions.

  2. Empty string normalization: The updates.folder = updates.folder || null in the update endpoint now matches the create endpoint's behavior, ensuring consistency across both operations.

The implementation is clean and the comment makes the intent clear. Thanks for the quick turnaround!

@dagangtj
Copy link
Author

Hi, just checking in - is there anything else needed for this PR? Happy to make any changes.

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