feat(flags): add folder organization for feature flags#385
feat(flags): add folder organization for feature flags#385jaysu66 wants to merge 2 commits intodatabuddy-analytics:stagingfrom
Conversation
|
@jaysu66 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 adds a Confidence Score: 5/5Safe to merge — all findings are P2 style/cleanup suggestions with no correctness or data-integrity impact. The feature is well-scoped and additive: the DB change is a nullable column with an appropriate index, the RPC endpoint is straightforward, cache invalidation for packages/shared/src/flags/index.ts (missing min(1) on folder field) and apps/dashboard/app/(main)/websites/[id]/flags/page.tsx (unused folderCounts Map). Important Files Changed
Sequence DiagramsequenceDiagram
participant U as User
participant UI as FlagsPage
participant FN as FolderNav
participant FS as FlagSheet
participant RPC as flags RPC
participant DB as PostgreSQL
U->>UI: Open Flags Page
UI->>RPC: flags.list({ websiteId })
UI->>RPC: flags.listFolders({ websiteId })
RPC->>DB: SELECT * FROM flags WHERE ...
RPC->>DB: SELECT DISTINCT folder FROM flags WHERE folder IS NOT NULL ORDER BY folder ASC
DB-->>RPC: flags[] + folder[]
RPC-->>UI: flags[] + string[]
UI->>FN: folders, activeFolder, totalCount, uncategorizedCount
FN-->>U: Sidebar with All Flags / named folders / Uncategorized
U->>FN: Click named folder
FN->>UI: setActiveFolder(folder)
UI->>UI: filteredFlags = activeFlags.filter(f => f.folder === folder)
UI-->>U: Filtered flag list
U->>FS: Create / Edit flag with folder value
FS->>RPC: flags.create / flags.update { folder }
RPC->>DB: INSERT / UPDATE flags SET folder = ?
DB-->>RPC: saved flag
RPC-->>FS: success
FS->>UI: invalidate flags.list → onCloseAction()
UI->>UI: handleFlagSheetClose → invalidate flags.listFolders
UI->>RPC: flags.listFolders({ websiteId })
RPC-->>UI: updated string[]
UI->>FN: re-render sidebar
Reviews (1): Last reviewed commit: "feat(flags): add folder organization for..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| }) { | ||
| if (folders.length === 0) return null; | ||
|
|
||
| const folderCounts = new Map<string, number>(); |
There was a problem hiding this comment.
folderCounts variable
folderCounts is allocated but never written to or read — it is dead code. This also leaves a visual inconsistency: the "All Flags" and "Uncategorized" entries show count badges, but named folder buttons do not. Either remove the variable or populate it and render counts per folder.
| const folderCounts = new Map<string, number>(); |
packages/rpc/src/routers/flags.ts
Outdated
| isNotNull(flags.folder) | ||
| ) | ||
| ) | ||
| .orderBy(sql`${flags.folder} asc`); |
There was a problem hiding this comment.
orderBy
sql`${flags.folder} asc` bypasses Drizzle's type-safe ordering helpers. Prefer asc(flags.folder) (import asc from @databuddy/db).
| .orderBy(sql`${flags.folder} asc`); | |
| .orderBy(asc(flags.folder)); |
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!
packages/shared/src/flags/index.ts
Outdated
| .optional(), | ||
| environment: z.string().nullable().optional(), | ||
| targetGroupIds: z.array(z.string()).optional(), | ||
| folder: z.string().max(100, "Folder path too long").nullable().optional(), |
There was a problem hiding this comment.
min(1) on folder string
z.string().max(100) accepts "". The client trims and normalises empty strings to null/undefined, but a direct API caller could persist a blank folder, causing an empty entry to appear in listFolders and FolderNav. Adding .min(1) closes this gap.
| folder: z.string().max(100, "Folder path too long").nullable().optional(), | |
| folder: z.string().min(1).max(100, "Folder path too long").nullable().optional(), |
Adds a `folder` field to feature flags so teams can organize flags into logical groups (e.g. "auth/login", "payments"). Includes: - DB: `folder text` column + composite index on (websiteId, folder) - RPC: `listFolders` endpoint returning distinct folder names; folder propagated through create and update handlers - Shared schema: `folder` field added to `flagFormSchema` - Dashboard: folder input in the create/edit sheet; FolderNav sidebar with per-folder filtering (All / named folder / Uncategorized); folder badge on each row in the flags list Closes databuddy-analytics#271
jaysu66
force-pushed
the
feat/flag-folders
branch
from
1ce4fea to
5ef44f6
Compare
- Add min(1) to folder schema to prevent empty strings passing validation - Replace raw sql`...asc` with drizzle asc() helper for type safety - Clean up unused sql import
2 participants
Summary
Closes #271
Adds a
folderfield to feature flags so teams can organize flags into logical groups (e.g.auth/login,payments,experiments).Changes:
packages/db): newfolder textcolumn onflagstable + composite index on(websiteId, folder)for efficient folder queriespackages/rpc): newlistFoldersendpoint returning distinct folder names per website;folderfield propagated throughcreateandupdatehandlerspackages/shared):folderadded toflagFormSchemawith 100-char maxFolderNavsidebar (All Flags / named folders / Uncategorized) with active folder filtering; sidebar only renders when folders existTest plan
payments) → folder badge appears in list, sidebar showspaymentsentrylistFoldersreturns sorted distinct folder names for the websitebun run db:pushapplies the new column and index without errors