Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
|
Contributor
|
Warning Rate limit exceeded@chronark has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 12 minutes and 30 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (29)
📝 WalkthroughWalkthroughReplaces TRPC paginated queries/mutations for ratelimits with a TanStack client-side collections + useLiveQuery layer, adds flat list TRPC endpoints, removes legacy paginated hooks and a migration script, and updates UI components to use collection operations and simplified loading/prop signatures. (≈34 words) Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant UI as Dashboard UI
participant Live as LiveCollection
participant Q as QueryClient
participant TRPC as trpcClient
participant API as TRPC Server
UI->>Live: useLiveQuery() subscribe
Live->>Q: resolve cache / call queryFn
Q->>TRPC: trpcClient.<namespace|override>.list.query()
TRPC->>API: RPC list request
API-->>TRPC: list response [{id,name,...},...]
TRPC-->>Q: return data
Q-->>Live: cache update
Live-->>UI: reactive render update
sequenceDiagram
autonumber
participant UI as Dashboard UI (create/update/delete)
participant Coll as Collection (namespaces/overrides)
participant TRPC as trpcClient
participant API as TRPC Server
rect rgba(220,255,220,0.12)
UI->>Coll: insert/update/delete(record)
Coll->>TRPC: call corresponding mutation (create/update/delete)
TRPC->>API: RPC mutate
API-->>TRPC: mutation result
TRPC-->>Coll: reconcile result
Coll-->>UI: collection reflects change (live)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Pre-merge checks (1 passed, 2 warnings)❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing Touches
🧪 Generate 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. Comment |
chronark
requested review from
Flo4604,
MichaelUnkey,
mcstepp,
ogzhanolguncu and
perkinsjr
as code owners
Collaborator
Author
This stack of pull requests is managed by Graphite. Learn more about stacking. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 20
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/dashboard/app/(app)/ratelimits/_components/create-namespace-button.tsx (1)
91-93: Button loading/disabled state should useisSubmittingTie UI state to form submission to prevent duplicate submits, regardless of backend path.
- disabled={create.isLoading || !isValid || isSubmitting} - loading={create.isLoading || isSubmitting} + disabled={!isValid || isSubmitting} + loading={isSubmitting}If you keep TRPC, you can OR with
create.isLoading.apps/dashboard/app/(app)/ratelimits/[namespaceId]/namespace-navbar.tsx (1)
20-27: Remove unused prop from type
includeOverridesis defined but unused. Drop it to prevent confusion.type NamespaceNavbarProps = { namespaceId: string; - includeOverrides?: boolean; activePage: { href: string; text: string; }; }; export const NamespaceNavbar = ({ namespaceId, - activePage, + activePage, }: NamespaceNavbarProps) => {#!/bin/bash # Check call sites still passing includeOverrides. rg -n '\bincludeOverrides\b' -g '!**/node_modules/**'Also applies to: 29-33
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (15)
apps/dashboard/app/(app)/ratelimits/[namespaceId]/_components/namespace-delete-dialog.tsx(4 hunks)apps/dashboard/app/(app)/ratelimits/[namespaceId]/namespace-navbar.tsx(5 hunks)apps/dashboard/app/(app)/ratelimits/[namespaceId]/settings/components/settings-client.tsx(3 hunks)apps/dashboard/app/(app)/ratelimits/_components/create-namespace-button.tsx(2 hunks)apps/dashboard/app/(app)/ratelimits/_components/list/hooks/use-namespace-list-query.ts(0 hunks)apps/dashboard/app/(app)/ratelimits/_components/list/index.tsx(3 hunks)apps/dashboard/components/navigation/sidebar/app-sidebar/hooks/use-ratelimit-navigation.tsx(3 hunks)apps/dashboard/components/navigation/sidebar/app-sidebar/index.tsx(2 hunks)apps/dashboard/components/navigation/sidebar/team-switcher.tsx(0 hunks)apps/dashboard/lib/collections/index.ts(1 hunks)apps/dashboard/lib/trpc/routers/index.ts(2 hunks)apps/dashboard/lib/trpc/routers/ratelimit/namespaces_list.ts(1 hunks)apps/dashboard/lib/trpc/routers/ratelimit/query-namespaces/index.ts(0 hunks)apps/dashboard/package.json(1 hunks)tools/migrate/main.ts(0 hunks)
💤 Files with no reviewable changes (4)
- apps/dashboard/components/navigation/sidebar/team-switcher.tsx
- apps/dashboard/app/(app)/ratelimits/_components/list/hooks/use-namespace-list-query.ts
- tools/migrate/main.ts
- apps/dashboard/lib/trpc/routers/ratelimit/query-namespaces/index.ts
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2025-08-25T13:46:34.441Z
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3834
File: apps/dashboard/app/(app)/ratelimits/_components/controls/components/namespace-list-refresh.tsx:4-4
Timestamp: 2025-08-25T13:46:34.441Z
Learning: The namespace list refresh component (apps/dashboard/app/(app)/ratelimits/_components/controls/components/namespace-list-refresh.tsx) intentionally uses the overview hook (useFilters from @/app/(app)/ratelimits/[namespaceId]/_overview/hooks/use-filters) rather than a namespace-specific hook. This cross-coupling between namespace list components and overview hooks is an architectural design decision.
Applied to files:
apps/dashboard/app/(app)/ratelimits/[namespaceId]/namespace-navbar.tsxapps/dashboard/app/(app)/ratelimits/_components/list/index.tsxapps/dashboard/lib/trpc/routers/index.tsapps/dashboard/app/(app)/ratelimits/[namespaceId]/settings/components/settings-client.tsxapps/dashboard/app/(app)/ratelimits/_components/create-namespace-button.tsxapps/dashboard/app/(app)/ratelimits/[namespaceId]/_components/namespace-delete-dialog.tsxapps/dashboard/components/navigation/sidebar/app-sidebar/hooks/use-ratelimit-navigation.tsx
📚 Learning: 2025-08-25T13:46:08.303Z
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3834
File: apps/dashboard/app/(app)/ratelimits/_components/controls/components/namespace-list-datetime/index.tsx:1-1
Timestamp: 2025-08-25T13:46:08.303Z
Learning: The NamespaceListDateTime component in apps/dashboard/app/(app)/ratelimits/_components/controls/components/namespace-list-datetime/index.tsx is intentionally designed to use the overview hook (useFilters from @/app/(app)/ratelimits/[namespaceId]/_overview/hooks/use-filters) rather than the namespace list hook, as clarified by ogzhanolguncu. This coupling is by design, not an architectural issue.
Applied to files:
apps/dashboard/app/(app)/ratelimits/[namespaceId]/namespace-navbar.tsxapps/dashboard/app/(app)/ratelimits/_components/list/index.tsxapps/dashboard/app/(app)/ratelimits/[namespaceId]/settings/components/settings-client.tsxapps/dashboard/app/(app)/ratelimits/_components/create-namespace-button.tsxapps/dashboard/components/navigation/sidebar/app-sidebar/hooks/use-ratelimit-navigation.tsx
📚 Learning: 2025-08-25T12:56:59.310Z
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3834
File: apps/dashboard/lib/trpc/routers/ratelimit/query-namespaces/index.ts:59-66
Timestamp: 2025-08-25T12:56:59.310Z
Learning: In the ratelimit namespace query system (apps/dashboard/lib/trpc/routers/ratelimit/query-namespaces/index.ts), the nameQuery filter is designed as an array for future extensibility to support multiple filters, but currently only the first filter (index 0) is processed. This is intentional future-proofing.
Applied to files:
apps/dashboard/lib/trpc/routers/ratelimit/namespaces_list.tsapps/dashboard/app/(app)/ratelimits/_components/list/index.tsxapps/dashboard/lib/trpc/routers/index.tsapps/dashboard/app/(app)/ratelimits/[namespaceId]/settings/components/settings-client.tsx
📚 Learning: 2025-07-25T19:09:43.284Z
Learnt from: mcstepp
PR: unkeyed/unkey#3662
File: apps/dashboard/lib/trpc/routers/deployment/list.ts:11-11
Timestamp: 2025-07-25T19:09:43.284Z
Learning: In apps/dashboard/lib/trpc/routers/deployment/list.ts, the listDeployments procedure intentionally queries the versions table rather than a deployments table. The user mcstepp indicated that renaming the table would require a database migration, which was deferred for the current PR focused on UI features.
Applied to files:
apps/dashboard/lib/trpc/routers/ratelimit/namespaces_list.ts
📚 Learning: 2025-08-18T10:28:47.391Z
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3797
File: apps/dashboard/app/(app)/projects/[projectId]/deployments/components/control-cloud/index.tsx:1-4
Timestamp: 2025-08-18T10:28:47.391Z
Learning: In Next.js App Router, components that use React hooks don't need their own "use client" directive if they are rendered within a client component that already has the directive. The client boundary propagates to child components.
Applied to files:
apps/dashboard/app/(app)/ratelimits/[namespaceId]/_components/namespace-delete-dialog.tsxapps/dashboard/components/navigation/sidebar/app-sidebar/hooks/use-ratelimit-navigation.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Test Go API Local / Test
- GitHub Check: Test API / API Test Local
- GitHub Check: Build / Build
- GitHub Check: Test Packages / Test
🔇 Additional comments (7)
apps/dashboard/components/navigation/sidebar/app-sidebar/index.tsx (1)
87-89: Ratelimit “load more” removal looks consistentloadMore map, deps, and hook shape now exclude ratelimits. No dangling references spotted.
Also applies to: 119-120
apps/dashboard/components/navigation/sidebar/app-sidebar/hooks/use-ratelimit-navigation.tsx (1)
76-89: Verify downstream consumers after return-shape changeThis hook now only returns
{ enhancedNavItems }. Ensure callers no longer rely onisLoading/loadMore.#!/bin/bash # Find all usages of the hook and check for destructuring of non-existent fields. rg -nP 'useRatelimitNavigation\s*\(' -C2 rg -nP '\{[^}]*\benhancedNavItems\b[^}]*\}' -g '!**/node_modules/**' rg -nP '\b(isLoading|loadMore)\b' -g '!**/node_modules/**'apps/dashboard/app/(app)/ratelimits/[namespaceId]/settings/components/settings-client.tsx (1)
45-47: ```shell
#!/bin/bashDisplay the beginning of settings-client.tsx to inspect useLiveQuery usage
cat "apps/dashboard/app/(app)/ratelimits/[namespaceId]/settings/components/settings-client.tsx" | sed -n '1,100p'
</blockquote></details> <details> <summary>apps/dashboard/app/(app)/ratelimits/[namespaceId]/namespace-navbar.tsx (1)</summary><blockquote> `36-41`: **Consistent loading handling across ratelimits views** Other files default `data` to `[]`. Decide on one pattern (undefined vs empty) and apply consistently to avoid subtle runtime differences in skeleton rendering. ```shell #!/bin/bash # Inspect all `useLiveQuery` usages under ratelimits to see loading patterns. rg -nP 'useLiveQuery\s*\(' apps/dashboard/app/\(app\)/ratelimits -C2apps/dashboard/lib/collections/index.ts (3)
14-27: Confirm client-only usage or handle SSR base URL/cookiesThe TRPC client points to relative
/api/trpcwithcredentials: "include". If this module is imported during SSR, cookies/URL resolution may break. Ensure client-only usage or compute an absolute URL and forward cookies in SSR.
45-47: Verify TRPC procedure path
ratelimit.namespace.update.namelooks like a nested router-procedure chain. Confirm the router actually exposes this path (vs. e.g.updateName).Run:
#!/bin/bash # Verify server procedure path(s) rg -nP 'ratelimit.*namespace.*(update\.name|updateName)' --type=ts -C2
2-3: ```shell
#!/bin/bashFind all QueryClient instantiations
rg -n "new QueryClient" -C3 || true
Find React Query providers
rg -n "QueryClientProvider" -C3 || true
Find any existing shared queryClient declarations
rg -n "queryClient\s*=\s*new QueryClient" -C3 || true
</blockquote></details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
apps/dashboard/app/(app)/ratelimits/_components/create-namespace-button.tsx
Outdated
Show resolved
Hide resolved
apps/dashboard/app/(app)/ratelimits/[namespaceId]/_components/namespace-delete-dialog.tsx
Show resolved
Hide resolved
apps/dashboard/app/(app)/ratelimits/[namespaceId]/settings/components/settings-client.tsx
Outdated
Show resolved
Hide resolved
Contributor
|
Thank you for following the naming conventions for pull request titles! 🙏 |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 31
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (10)
apps/dashboard/lib/trpc/routers/ratelimit/createNamespace.ts (1)
50-56: Harden DB error handling and avoid brittle substring check
- Optional-chain
e.body?.messageto prevent defensive crashes.- Prefer matching a stable DB error signal (e.g., code 1062 / ER_DUP_ENTRY) in addition to the message substring to future-proof duplicate detection.
If DatabaseError exposes a code, consider this tweak:
- console.error("Failed to create namespace", e); - if (e instanceof DatabaseError && e.body.message.includes("Duplicate entry")) { + console.error("Failed to create namespace", e?.body?.message ?? e); + if ( + e instanceof DatabaseError && + (e.body?.message?.includes("Duplicate entry") || + // if available in your env: + (e as any).code === "ER_DUP_ENTRY") + ) {apps/dashboard/lib/trpc/routers/ratelimit/createOverride.ts (1)
63-73: Use an atomic upsert to eliminate races and simplify logicSwitching to an insert ... on duplicate key update avoids the check-then-act race and removes the need for the separate read/update branch.
Example (MySQL/PlanetScale):
await tx .insert(schema.ratelimitOverrides) .values({ id, workspaceId: ctx.workspace.id, namespaceId: namespace.id, identifier: input.identifier, limit: input.limit, duration: input.duration, async: false, createdAtM: Date.now(), }) .onDuplicateKeyUpdate({ set: { limit: input.limit, duration: input.duration, updatedAtM: Date.now(), deletedAtM: null, }, });Also consider validating inputs as positive integers to avoid invalid overrides:
.input(z.object({ namespaceId: z.string(), identifier: z.string().min(1), limit: z.number().int().positive(), duration: z.number().int().positive(), }))apps/dashboard/lib/trpc/routers/ratelimit/updateOverride.ts (1)
11-15: Harden input schema (ints, >0, sensible caps)Prevent bad values early (negative/float/overflow).
Example:
.input( z.object({ id: z.string().min(1), limit: z.number().int().positive().max(1_000_000), duration: z.number().int().positive().max(86_400_000), // <= 24h }), )apps/dashboard/app/(app)/ratelimits/[namespaceId]/_components/delete-dialog.tsx (1)
10-14: Remove no-op refine
v === vis always true for strings; this validation does nothing.const formSchema = z.object({ identifier: z.string().min(1, "Please confirm the identifier"), });apps/dashboard/app/(app)/ratelimits/[namespaceId]/overrides/overrides-table.tsx (2)
13-19: Align local type with backend/UI (remove async from Override if deprecated)If
asyncis gone, update the table’sOverridetype accordingly to prevent accidental use.type Override = { id: string; identifier: string; limit: number; duration: number; // async: boolean | null; // remove if no longer used };
116-120: Drop deprecatedasyncflag fromOverridesTableActionprops
Server now enforcesasync = falseand the UI no longer uses this field; remove it to avoid stale API coupling.<OverridesTableAction overrideDetails={{ duration: override.duration, limit: override.limit, - async: override.async, overrideId: override.id, }}apps/dashboard/app/(app)/ratelimits/[namespaceId]/namespace-navbar.tsx (1)
20-27: Remove unused prop from the public type
includeOverridesis no longer consumed; drop it fromNamespaceNavbarPropsto avoid drift.type NamespaceNavbarProps = { namespaceId: string; - includeOverrides?: boolean; activePage: { href: string; text: string; }; };apps/dashboard/app/(app)/ratelimits/[namespaceId]/_components/identifier-dialog.tsx (2)
13-25: Fix validation copy to match the fieldMessage says “Name” but the field is “Identifier.”
identifier: z .string() .trim() - .min(2, "Name is required and should be at least 2 characters") + .min(2, "Identifier is required and should be at least 2 characters") .max(250),
29-36: Drop PropsWithChildren — this dialog doesn’t accept childrenSimplify the prop type.
-type Props = PropsWithChildren<{ +type Props = { isModalOpen: boolean; onOpenChange: (value: boolean) => void; identifier?: string; isLoading?: boolean; namespaceId: string; overrideDetails?: OverrideDetails | null; -}>; +};apps/dashboard/lib/trpc/routers/index.ts (1)
251-255: Naming consistency: expose llm search under a consistent keyElsewhere, llm search endpoints are exposed as
llmSearch. Here the key isratelimitLlmSearchvia shorthand, which is inconsistent and may surprise clients.logs: t.router({ query: queryRatelimitLogs, - ratelimitLlmSearch, + llmSearch: ratelimitLlmSearch, queryRatelimitTimeseries, }),
♻️ Duplicate comments (12)
apps/dashboard/package.json (1)
15-15: Dependency health: TanStack dual-core and Zod major bump risk
TanStack: Adding query-core@^5 alongside react-query@^4 risks shipping two cores and confusing caches/plugins. Either upgrade react-query to v5 (and compatible trpc adapter) or isolate usage to avoid cross-wiring.
Zod: Jumping to "zod": "4.1.5" is a breaking major. Ensure all packages (including any shared validators and @hookform/resolvers usage) are compatible to avoid type/runtime mismatches. If not aligned repo-wide, either revert to v3 now or plan a coordinated upgrade.
Quick checks to run:
#!/bin/bash # Inspect TanStack deps present rg -n "@tanstack/(react-query|query-core|react-db|query-db-collection)" -C2 # Why-resolve to see versions pulled in pnpm why @tanstack/query-core @tanstack/react-query @tanstack/query-db-collection # Find all zod consumers across the monorepo rg -nP '\bzod\b' -g '!**/dist/**' -C2If you decide to align on React Query v5, ensure the TRPC binding matches (or keep v4-bound TRPC fully isolated from v5 query clients).
Also applies to: 38-41, 103-103
apps/dashboard/app/(app)/ratelimits/_components/list/index.tsx (1)
23-30: Fix possible runtime crash: defaultuseLiveQuerydata to []
useLiveQueryreturnsundefinedinitially;namespaces.length/.mapwill throw. Default to an empty array.- const { data: namespaces } = useLiveQuery( + const { data } = useLiveQuery( (q) => q .from({ namespace: collection.ratelimitNamespaces }) .where(({ namespace }) => ilike(namespace.name, `%${nameFilter}%`)) .orderBy(({ namespace }) => namespace.id, "desc"), [nameFilter], ); + const namespaces = data ?? [];Also applies to: 32-61, 66-68
apps/dashboard/lib/trpc/routers/ratelimit/namespaces_list.ts (1)
13-19: Fix soft-delete column name; add deterministic orderingThe filter uses
deletedAtM, which likely should bedeletedAt(ordeletedAtMs). Also add anorderByfor deterministic results to match UI expectations.Apply:
return await db.query.ratelimitNamespaces.findMany({ - where: (table, { eq, and, isNull }) => - and(eq(table.workspaceId, ctx.workspace.id), isNull(table.deletedAtM)), + where: (table, { eq, and, isNull }) => + and(eq(table.workspaceId, ctx.workspace.id), isNull(table.deletedAt)), + orderBy: (table, { desc }) => [desc(table.id)], columns: { id: true, name: true, }, });Run to confirm actual column name:
#!/bin/bash rg -nP 'ratelimitNamespaces.*deletedAtM[s]?|ratelimitNamespaces.*deletedAt\b' -C2apps/dashboard/app/(app)/ratelimits/[namespaceId]/_components/namespace-delete-dialog.tsx (1)
42-47: Await deletion; guard double-submit; wire loading UIDeletion isn’t awaited; navigation can race and failures go silent. Also disable while submitting and show loading.
- const onSubmit = async () => { - collection.ratelimitNamespaces.delete(namespace.id); - router.push("/ratelimits"); - - //await deleteNamespace.mutateAsync({ namespaceId: namespace.id }); - }; + const onSubmit = async () => { + // extra safety: do not proceed unless confirmed + if (watch("name") !== namespace.name) return; + try { + await collection.ratelimitNamespaces.delete(namespace.id); + router.push("/ratelimits"); + } catch (e) { + console.error("Failed to delete namespace", e); + // optionally surface a toast here + } + }; @@ - disabled={!isValid} + disabled={!isValid || isSubmitting} + loading={isSubmitting}Also plumb
isSubmittingfrom react-hook-form:- const { register, handleSubmit, watch } = useForm<FormValues>({ + const { + register, + handleSubmit, + watch, + formState: { isSubmitting }, + } = useForm<FormValues>({Also applies to: 55-65
apps/dashboard/components/navigation/sidebar/app-sidebar/hooks/use-ratelimit-navigation.tsx (2)
18-22: Default data from useLiveQuery to avoid undefined access
datacan be undefined before hydration;data.lengthwill throw.- const { data } = useLiveQuery((q) => + const { data = [] } = useLiveQuery((q) => q .from({ namespace: collection.ratelimitNamespaces }) .orderBy(({ namespace }) => namespace.id, "desc"), );
21-21: Prefer alphabetical ordering for better UXIDs descending are not user-friendly in nav; sort by name asc.
- .orderBy(({ namespace }) => namespace.id, "desc"), + .orderBy(({ namespace }) => namespace.name, "asc"),apps/dashboard/app/(app)/ratelimits/_components/create-namespace-button.tsx (1)
44-57: Await insert, avoid date-based IDs, and keep UI from double-submitting
- onSubmit should be async and await the insert; otherwise users can click multiple times and race the local index/mutation.
- Use crypto.randomUUID() instead of new Date().toISOString() for client-side IDs. This was flagged earlier as well.
- Preserve server error propagation by rethrowing non-DuplicateKeyError.
- const onSubmit = (values: FormValues) => { - try { - collection.ratelimitNamespaces.insert({ - id: new Date().toISOString(), - name: values.name, - }); - reset(); - setIsOpen(false); - } catch (error) { - if (error instanceof DuplicateKeyError) { - setError("name", { type: "custom", message: "Namespace already exists" }); - } - } - }; + const onSubmit = async (values: FormValues) => { + try { + await collection.ratelimitNamespaces.insert({ + id: crypto.randomUUID(), + name: values.name, + }); + reset(); + setIsOpen(false); + } catch (error) { + if (error instanceof DuplicateKeyError) { + setError("name", { type: "custom", message: "Namespace already exists" }); + } else { + throw error; + } + } + };apps/dashboard/app/(app)/ratelimits/[namespaceId]/settings/components/settings-client.tsx (3)
88-103: Disable Save based on trimmed valuePrevent names that differ only by whitespace; align with submit validation.
- disabled={namespaceName === namespace.name || !namespaceName} + disabled={ + (namespaceName ?? "").trim() === "" || + (namespaceName ?? "").trim() === (namespace?.name ?? "") + }
17-24: Guard against undefined data and avoid.at(0)Default
datato an empty array and use index access to prevent runtime errors before first render.- const { data } = useLiveQuery((q) => + const { data = [] } = useLiveQuery((q) => q .from({ namespace: collection.ratelimitNamespaces }) .where(({ namespace }) => eq(namespace.id, namespaceId)), ); - - const namespace = data.at(0); + const namespace = data[0];
32-53: Trim, await persistence, and harden duplicate check
- Reject whitespace/no-op names.
- Compare names case-insensitively on trimmed values.
- Await
updateand surface success/failure toasts.- const handleUpdateName = async () => { + const handleUpdateName = async () => { if (!namespace) { return; } - if (namespaceName === namespace.name || !namespaceName) { - return toast.error("Please provide a different name before saving."); - } - let error = ""; - collection.ratelimitNamespaces.forEach((ns) => { - if (ns.id !== namespaceId && namespaceName === ns.name) { - error = "Another namespace already has this name"; - return; - } - }); - if (error) { - return toast.error(error); - } - - collection.ratelimitNamespaces.update(namespace.id, (draft) => { - draft.name = namespaceName; - }); + const next = (namespaceName ?? "").trim(); + if (!next || next === namespace.name) { + return toast.error("Please provide a different name before saving."); + } + let error = ""; + collection.ratelimitNamespaces.forEach((ns) => { + if (ns.id !== namespaceId && ns.name.trim().toLowerCase() === next.toLowerCase()) { + error = "Another namespace already has this name"; + } + }); + if (error) return toast.error(error); + try { + await collection.ratelimitNamespaces.update(namespace.id, (draft) => { + draft.name = next; + }); + toast.success("Namespace name saved."); + } catch { + toast.error("Failed to save namespace name. Please try again."); + } };apps/dashboard/app/(app)/ratelimits/[namespaceId]/namespace-navbar.tsx (1)
76-81: Sort QuickNav items for usabilityAlphabetize namespaces for predictable navigation.
- items={data.map((ns) => ({ + items={[...data].sort((a, b) => (a.name ?? "").localeCompare(b.name ?? "")).map((ns) => ({ id: ns.id, label: ns.name, href: `/ratelimits/${ns.id}`, }))}apps/dashboard/lib/trpc/routers/index.ts (1)
263-270: Breaking rename: namespace.query → namespace.list (add a back-compat alias)This is a public API rename. To avoid immediate client breakage, keep a temporary alias
querypointing to the same procedure and deprecate it for one release cycle.Apply:
namespace: t.router({ - list: listRatelimitNamespaces, + list: listRatelimitNamespaces, + // DEPRECATED: temporary back-compat, remove after clients migrate. + // Keep for one release cycle to prevent runtime breaks. + query: listRatelimitNamespaces, queryRatelimitLastUsed, create: createNamespace, update: t.router({ name: updateNamespaceName, }), delete: deleteNamespace, }),Verify no lingering usages:
#!/bin/bash # Find client usages that still call the old path rg -nP -C2 '\bratelimit\.namespace\.query\b' rg -nP -C2 '\btrpc\.ratelimit\.namespace\.query\b|\bapi\.ratelimit\.namespace\.query\b'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (25)
apps/dashboard/app/(app)/ratelimits/[namespaceId]/_components/delete-dialog.tsx(4 hunks)apps/dashboard/app/(app)/ratelimits/[namespaceId]/_components/identifier-dialog.tsx(2 hunks)apps/dashboard/app/(app)/ratelimits/[namespaceId]/_components/namespace-delete-dialog.tsx(3 hunks)apps/dashboard/app/(app)/ratelimits/[namespaceId]/namespace-navbar.tsx(5 hunks)apps/dashboard/app/(app)/ratelimits/[namespaceId]/overrides/overrides-table.tsx(2 hunks)apps/dashboard/app/(app)/ratelimits/[namespaceId]/settings/components/settings-client.tsx(4 hunks)apps/dashboard/app/(app)/ratelimits/_components/create-namespace-button.tsx(3 hunks)apps/dashboard/app/(app)/ratelimits/_components/hooks/use-namespace-list-filters.ts(1 hunks)apps/dashboard/app/(app)/ratelimits/_components/list/index.tsx(3 hunks)apps/dashboard/app/(app)/ratelimits/_components/list/namespace-card.tsx(1 hunks)apps/dashboard/components/navigation/sidebar/app-sidebar/hooks/use-ratelimit-navigation.tsx(3 hunks)apps/dashboard/components/navigation/sidebar/app-sidebar/index.tsx(2 hunks)apps/dashboard/components/navigation/sidebar/team-switcher.tsx(2 hunks)apps/dashboard/lib/collections/client.ts(1 hunks)apps/dashboard/lib/collections/index.ts(1 hunks)apps/dashboard/lib/collections/ratelimit_namespaces.ts(1 hunks)apps/dashboard/lib/collections/ratelimit_overrides.ts(1 hunks)apps/dashboard/lib/trpc/routers/index.ts(2 hunks)apps/dashboard/lib/trpc/routers/ratelimit/createNamespace.ts(1 hunks)apps/dashboard/lib/trpc/routers/ratelimit/createOverride.ts(1 hunks)apps/dashboard/lib/trpc/routers/ratelimit/namespace-search.ts(0 hunks)apps/dashboard/lib/trpc/routers/ratelimit/namespaces_list.ts(1 hunks)apps/dashboard/lib/trpc/routers/ratelimit/overrides_list.ts(1 hunks)apps/dashboard/lib/trpc/routers/ratelimit/updateOverride.ts(1 hunks)apps/dashboard/package.json(3 hunks)
💤 Files with no reviewable changes (1)
- apps/dashboard/lib/trpc/routers/ratelimit/namespace-search.ts
🧰 Additional context used
🧠 Learnings (9)
📚 Learning: 2025-08-25T13:46:34.441Z
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3834
File: apps/dashboard/app/(app)/ratelimits/_components/controls/components/namespace-list-refresh.tsx:4-4
Timestamp: 2025-08-25T13:46:34.441Z
Learning: The namespace list refresh component (apps/dashboard/app/(app)/ratelimits/_components/controls/components/namespace-list-refresh.tsx) intentionally uses the overview hook (useFilters from @/app/(app)/ratelimits/[namespaceId]/_overview/hooks/use-filters) rather than a namespace-specific hook. This cross-coupling between namespace list components and overview hooks is an architectural design decision.
Applied to files:
apps/dashboard/app/(app)/ratelimits/_components/hooks/use-namespace-list-filters.tsapps/dashboard/app/(app)/ratelimits/_components/list/namespace-card.tsxapps/dashboard/app/(app)/ratelimits/[namespaceId]/settings/components/settings-client.tsxapps/dashboard/app/(app)/ratelimits/[namespaceId]/overrides/overrides-table.tsxapps/dashboard/app/(app)/ratelimits/[namespaceId]/_components/delete-dialog.tsxapps/dashboard/app/(app)/ratelimits/_components/create-namespace-button.tsxapps/dashboard/app/(app)/ratelimits/[namespaceId]/_components/namespace-delete-dialog.tsxapps/dashboard/app/(app)/ratelimits/[namespaceId]/namespace-navbar.tsxapps/dashboard/components/navigation/sidebar/app-sidebar/hooks/use-ratelimit-navigation.tsxapps/dashboard/lib/trpc/routers/index.tsapps/dashboard/app/(app)/ratelimits/_components/list/index.tsxapps/dashboard/app/(app)/ratelimits/[namespaceId]/_components/identifier-dialog.tsx
📚 Learning: 2025-08-25T13:46:08.303Z
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3834
File: apps/dashboard/app/(app)/ratelimits/_components/controls/components/namespace-list-datetime/index.tsx:1-1
Timestamp: 2025-08-25T13:46:08.303Z
Learning: The NamespaceListDateTime component in apps/dashboard/app/(app)/ratelimits/_components/controls/components/namespace-list-datetime/index.tsx is intentionally designed to use the overview hook (useFilters from @/app/(app)/ratelimits/[namespaceId]/_overview/hooks/use-filters) rather than the namespace list hook, as clarified by ogzhanolguncu. This coupling is by design, not an architectural issue.
Applied to files:
apps/dashboard/app/(app)/ratelimits/_components/hooks/use-namespace-list-filters.tsapps/dashboard/app/(app)/ratelimits/_components/list/namespace-card.tsxapps/dashboard/app/(app)/ratelimits/[namespaceId]/settings/components/settings-client.tsxapps/dashboard/app/(app)/ratelimits/[namespaceId]/overrides/overrides-table.tsxapps/dashboard/app/(app)/ratelimits/[namespaceId]/namespace-navbar.tsxapps/dashboard/components/navigation/sidebar/app-sidebar/hooks/use-ratelimit-navigation.tsxapps/dashboard/app/(app)/ratelimits/_components/list/index.tsxapps/dashboard/app/(app)/ratelimits/[namespaceId]/_components/identifier-dialog.tsx
📚 Learning: 2025-08-25T12:56:59.310Z
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3834
File: apps/dashboard/lib/trpc/routers/ratelimit/query-namespaces/index.ts:59-66
Timestamp: 2025-08-25T12:56:59.310Z
Learning: In the ratelimit namespace query system (apps/dashboard/lib/trpc/routers/ratelimit/query-namespaces/index.ts), the nameQuery filter is designed as an array for future extensibility to support multiple filters, but currently only the first filter (index 0) is processed. This is intentional future-proofing.
Applied to files:
apps/dashboard/app/(app)/ratelimits/_components/hooks/use-namespace-list-filters.tsapps/dashboard/app/(app)/ratelimits/[namespaceId]/settings/components/settings-client.tsxapps/dashboard/lib/trpc/routers/ratelimit/createNamespace.tsapps/dashboard/lib/collections/ratelimit_namespaces.tsapps/dashboard/lib/trpc/routers/ratelimit/namespaces_list.tsapps/dashboard/app/(app)/ratelimits/[namespaceId]/namespace-navbar.tsxapps/dashboard/lib/trpc/routers/index.tsapps/dashboard/app/(app)/ratelimits/_components/list/index.tsx
📚 Learning: 2025-06-24T13:29:10.129Z
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3401
File: apps/dashboard/app/(app)/logs/filters.query-params.ts:10-0
Timestamp: 2025-06-24T13:29:10.129Z
Learning: The `buildQueryParams` function in `apps/dashboard/app/(app)/logs/filters.query-params.ts` calls `useFilters()` hook inside it, but this is valid because the function is only called from within other React hooks, maintaining the Rules of Hooks compliance.
Applied to files:
apps/dashboard/app/(app)/ratelimits/_components/hooks/use-namespace-list-filters.ts
📚 Learning: 2024-12-05T13:27:55.555Z
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#2707
File: apps/dashboard/lib/trpc/routers/ratelimit/createOverride.ts:63-63
Timestamp: 2024-12-05T13:27:55.555Z
Learning: In `apps/dashboard/lib/trpc/routers/ratelimit/createOverride.ts`, when determining the maximum number of rate limit overrides (`max`), the intentional use of `const max = hasWorkspaceAccess("ratelimitOverrides", namespace.workspace) || 5;` allows `max` to fall back to `5` when `hasWorkspaceAccess` returns `0` or `false`. This fallback behavior is expected and intended in the codebase.
Applied to files:
apps/dashboard/lib/trpc/routers/ratelimit/overrides_list.tsapps/dashboard/lib/collections/ratelimit_overrides.ts
📚 Learning: 2024-10-04T20:47:34.791Z
Learnt from: chronark
PR: unkeyed/unkey#2180
File: apps/dashboard/lib/constants/workspace-navigations.tsx:85-88
Timestamp: 2024-10-04T20:47:34.791Z
Learning: In navigation code (e.g., `apps/dashboard/lib/constants/workspace-navigations.tsx`), prefer using `segments.at(0)` over `segments[0]` to handle cases when the `segments` array might be empty.
Applied to files:
apps/dashboard/app/(app)/ratelimits/[namespaceId]/settings/components/settings-client.tsx
📚 Learning: 2025-08-18T10:28:47.391Z
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3797
File: apps/dashboard/app/(app)/projects/[projectId]/deployments/components/control-cloud/index.tsx:1-4
Timestamp: 2025-08-18T10:28:47.391Z
Learning: In Next.js App Router, components that use React hooks don't need their own "use client" directive if they are rendered within a client component that already has the directive. The client boundary propagates to child components.
Applied to files:
apps/dashboard/app/(app)/ratelimits/[namespaceId]/_components/delete-dialog.tsxapps/dashboard/app/(app)/ratelimits/_components/create-namespace-button.tsxapps/dashboard/app/(app)/ratelimits/[namespaceId]/_components/namespace-delete-dialog.tsxapps/dashboard/components/navigation/sidebar/app-sidebar/hooks/use-ratelimit-navigation.tsxapps/dashboard/app/(app)/ratelimits/[namespaceId]/_components/identifier-dialog.tsx
📚 Learning: 2025-05-15T16:26:08.666Z
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3242
File: apps/dashboard/app/(app)/apis/[apiId]/_overview/components/table/components/override-indicator.tsx:50-65
Timestamp: 2025-05-15T16:26:08.666Z
Learning: In the Unkey dashboard, Next.js router (router.push) should be used for client-side navigation instead of window.location.href to preserve client state and enable smoother transitions between pages.
Applied to files:
apps/dashboard/app/(app)/ratelimits/_components/create-namespace-button.tsx
📚 Learning: 2025-07-25T19:09:43.284Z
Learnt from: mcstepp
PR: unkeyed/unkey#3662
File: apps/dashboard/lib/trpc/routers/deployment/list.ts:11-11
Timestamp: 2025-07-25T19:09:43.284Z
Learning: In apps/dashboard/lib/trpc/routers/deployment/list.ts, the listDeployments procedure intentionally queries the versions table rather than a deployments table. The user mcstepp indicated that renaming the table would require a database migration, which was deferred for the current PR focused on UI features.
Applied to files:
apps/dashboard/lib/trpc/routers/ratelimit/namespaces_list.ts
🧬 Code graph analysis (18)
apps/dashboard/lib/trpc/routers/ratelimit/overrides_list.ts (2)
apps/dashboard/lib/trpc/trpc.ts (4)
t(8-8)requireUser(10-21)requireWorkspace(23-36)withRatelimit(122-138)apps/dashboard/lib/db.ts (1)
db(5-26)
apps/dashboard/lib/collections/index.ts (2)
apps/dashboard/lib/collections/ratelimit_namespaces.ts (1)
ratelimitNamespaces(15-70)apps/dashboard/lib/collections/ratelimit_overrides.ts (1)
ratelimitOverrides(17-68)
apps/dashboard/app/(app)/ratelimits/[namespaceId]/settings/components/settings-client.tsx (1)
apps/dashboard/lib/collections/index.ts (1)
collection(6-9)
apps/dashboard/app/(app)/ratelimits/[namespaceId]/overrides/overrides-table.tsx (2)
apps/dashboard/lib/collections/index.ts (1)
collection(6-9)apps/dashboard/components/virtual-table/types.ts (1)
Column(13-24)
apps/dashboard/app/(app)/ratelimits/[namespaceId]/_components/delete-dialog.tsx (1)
apps/dashboard/lib/collections/index.ts (1)
collection(6-9)
apps/dashboard/lib/collections/client.ts (1)
apps/dashboard/lib/trpc/routers/index.ts (1)
Router(334-334)
apps/dashboard/components/navigation/sidebar/team-switcher.tsx (1)
apps/dashboard/lib/collections/index.ts (1)
reset(12-17)
apps/dashboard/lib/collections/ratelimit_overrides.ts (1)
apps/dashboard/lib/collections/client.ts (1)
trpcClient(12-25)
apps/dashboard/lib/collections/ratelimit_namespaces.ts (1)
apps/dashboard/lib/collections/client.ts (1)
trpcClient(12-25)
apps/dashboard/app/(app)/ratelimits/_components/create-namespace-button.tsx (1)
apps/dashboard/lib/collections/index.ts (2)
collection(6-9)reset(12-17)
apps/dashboard/app/(app)/ratelimits/[namespaceId]/_components/namespace-delete-dialog.tsx (2)
apps/dashboard/lib/collections/index.ts (1)
collection(6-9)apps/dashboard/lib/trpc/routers/index.ts (1)
router(123-331)
apps/dashboard/components/navigation/sidebar/app-sidebar/index.tsx (1)
apps/dashboard/components/navigation/sidebar/app-sidebar/hooks/use-ratelimit-navigation.tsx (1)
useRatelimitNavigation(15-95)
apps/dashboard/lib/trpc/routers/ratelimit/namespaces_list.ts (2)
apps/dashboard/lib/trpc/trpc.ts (4)
t(8-8)requireUser(10-21)requireWorkspace(23-36)withRatelimit(122-138)apps/dashboard/lib/db.ts (1)
db(5-26)
apps/dashboard/app/(app)/ratelimits/[namespaceId]/namespace-navbar.tsx (3)
apps/dashboard/lib/collections/index.ts (1)
collection(6-9)apps/dashboard/components/navigation/copyable-id-button.tsx (1)
CopyableIDButton(10-82)apps/dashboard/app/(app)/ratelimits/[namespaceId]/_components/identifier-dialog.tsx (1)
IdentifierDialog(38-159)
apps/dashboard/components/navigation/sidebar/app-sidebar/hooks/use-ratelimit-navigation.tsx (2)
apps/dashboard/lib/collections/index.ts (1)
collection(6-9)apps/dashboard/components/navigation/sidebar/workspace-navigations.tsx (1)
NavItem(16-29)
apps/dashboard/lib/trpc/routers/index.ts (2)
apps/dashboard/lib/trpc/routers/ratelimit/namespaces_list.ts (1)
listRatelimitNamespaces(6-30)apps/dashboard/lib/trpc/routers/ratelimit/overrides_list.ts (1)
listRatelimitOverrides(6-33)
apps/dashboard/app/(app)/ratelimits/_components/list/index.tsx (2)
apps/dashboard/app/(app)/ratelimits/_components/hooks/use-namespace-list-filters.ts (1)
useNamespaceListFilters(20-89)apps/dashboard/lib/collections/index.ts (1)
collection(6-9)
apps/dashboard/app/(app)/ratelimits/[namespaceId]/_components/identifier-dialog.tsx (2)
apps/dashboard/lib/trpc/routers/index.ts (1)
router(123-331)apps/dashboard/lib/collections/index.ts (1)
collection(6-9)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Test Go API Local / Test
- GitHub Check: Test API / API Test Local
- GitHub Check: Build / Build
- GitHub Check: Test Packages / Test
🔇 Additional comments (14)
apps/dashboard/app/(app)/ratelimits/_components/list/namespace-card.tsx (1)
28-28: Prefetch removal aligns with collection-driven data pathDropping the hover prefetch keeps concerns in one place (collections/live queries). Looks good.
apps/dashboard/lib/collections/index.ts (1)
6-9: Collection aggregator export is clearNaming and structure are straightforward for consumers (
collection.ratelimitNamespaces/...).apps/dashboard/components/navigation/sidebar/app-sidebar/index.tsx (2)
87-87: LGTM: load-more removal for ratelimits matches flat/live dataThis aligns with the client-collection navigation source.
118-119: Dependency list updated correctlyHandler now only maps project/api load-more. Clean.
apps/dashboard/lib/trpc/routers/ratelimit/updateOverride.ts (1)
56-60: Verify/drop write to deprecated async columnYou’re unconditionally writing
async: false. If the column is being removed, this will break at runtime. Either keep it intentionally (document why) or remove the write..set({ limit: input.limit, duration: input.duration, updatedAtM: Date.now(), - async: false, })apps/dashboard/app/(app)/ratelimits/[namespaceId]/overrides/overrides-table.tsx (1)
50-54: LGTM: correct live query filter by namespaceQuery shape and filter look right for local-first reads.
apps/dashboard/lib/trpc/routers/ratelimit/overrides_list.ts (1)
6-10: Middleware chain looks goodUser/workspace gating plus read ratelimit is correctly applied.
apps/dashboard/lib/collections/client.ts (1)
1-1: Client-only boundaryGood: explicit "use client" prevents server import of this module-level singleton.
apps/dashboard/components/navigation/sidebar/app-sidebar/hooks/use-ratelimit-navigation.tsx (1)
65-72: Passingicon: nullmatches the declared type; OK to keep
NavItem.iconis typed asReact.ElementType | null; settingnullis valid here.apps/dashboard/lib/collections/ratelimit_namespaces.ts (2)
25-44: Server-generated ID vs client placeholder: prevent ghost rows and ensure consistencyIn onInsert you persist only name; the server generates a new id. The UI inserts with a client-side temp id, so the local row’s key may diverge from the server’s id, causing duplicates or “ghost” entries until reload.
Options:
- Pass the client id through to the server create (if supported).
- Or, after mutate, force a collection refresh to reconcile keys.
- const p = trpcClient.ratelimit.namespace.create.mutate({ name: newNamespace.name }); + // If the API supports it, include the client id to keep keys stable: + const p = trpcClient.ratelimit.namespace.create.mutate({ + // id: newNamespace.id, // ← enable once server supports client-provided IDs + name: newNamespace.name, + }); @@ - await p; + await p; + // Ensure local state matches server (avoids temp-id drift). + await ratelimitNamespaces.preload();Please verify whether
ratelimit.namespace.createaccepts anidinput; if so, enable passingnewNamespace.id, otherwise retain thepreload()reconciliation.
45-58: Verify tRPC procedure exists and fallback on updateConfirm trpcClient.ratelimit.namespace.update.name exists; harden the onUpdate payload to handle missing modified or modified.name by falling back to original.name.
File: apps/dashboard/lib/collections/ratelimit_namespaces.ts — onUpdate handler.
- const p = trpcClient.ratelimit.namespace.update.name.mutate({ - namespaceId: original.id, - name: modified.name, - }); + const p = trpcClient.ratelimit.namespace.update.name.mutate({ + namespaceId: original.id, + name: modified?.name ?? original.name, + });apps/dashboard/app/(app)/ratelimits/[namespaceId]/settings/components/settings-client.tsx (1)
25-31: Initialization logic is soundPopulating
namespaceNameonce the namespace arrives is correct and avoids flicker.apps/dashboard/lib/trpc/routers/index.ts (2)
90-91: LGTM: new list endpoint imports are correctly wiredImports match the new modules and align with the local-first shift. No issues spotted here.
272-276: Additive API: override.list added (client wiring confirmed)
All client calls useratelimit.override.list.query(); nooverride.queryreferences found.
apps/dashboard/app/(app)/ratelimits/_components/create-namespace-button.tsx
Show resolved
Hide resolved
apps/dashboard/app/(app)/ratelimits/_components/create-namespace-button.tsx
Show resolved
Hide resolved
apps/dashboard/app/(app)/ratelimits/_components/hooks/use-namespace-list-filters.ts
Show resolved
Hide resolved
apps/dashboard/app/(app)/ratelimits/[namespaceId]/_components/delete-dialog.tsx
Show resolved
Hide resolved
perkinsjr
requested changes
Sep 9, 2025
Member
perkinsjr
left a comment
perkinsjr
left a comment
There was a problem hiding this comment.
Can we tighten up the zod schemas as well.
I didn't bother marking them because code rabbit got there, but might as well have nice tight coupling.
apps/dashboard/app/(app)/ratelimits/[namespaceId]/settings/components/settings-client.tsx
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
apps/dashboard/components/navigation/sidebar/team-switcher.tsx (1)
15-15: Awaiting reset to avoid stale flash — good fixImporting and awaiting
reset()beforerouter.replace("/")addresses the previously noted UX flash after org switch.Also applies to: 72-75
apps/dashboard/lib/collections/index.ts (1)
11-17: Make reset faster and more resilient: cleanup first, then preload concurrentlyRun both cleanups, then kick off both preloads in parallel and settle them to avoid a partial failure blocking the other collection. This also aligns with previous feedback.
export async function reset() { - for (const c of [ratelimitNamespaces, ratelimitOverrides]) { - await c.cleanup(); - await c.preload(); - } + // Run cleanups first (in parallel), then preload concurrently. + await Promise.allSettled([ + ratelimitNamespaces.cleanup(), + ratelimitOverrides.cleanup(), + ]); + await Promise.allSettled([ + ratelimitNamespaces.preload(), + ratelimitOverrides.preload(), + ]); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
apps/dashboard/components/navigation/sidebar/team-switcher.tsx(2 hunks)apps/dashboard/lib/collections/index.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
apps/dashboard/lib/collections/index.ts (2)
apps/dashboard/lib/collections/ratelimit_namespaces.ts (1)
ratelimitNamespaces(15-70)apps/dashboard/lib/collections/ratelimit_overrides.ts (1)
ratelimitOverrides(17-68)
apps/dashboard/components/navigation/sidebar/team-switcher.tsx (1)
apps/dashboard/lib/collections/index.ts (1)
reset(12-17)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Test Go API Local / Test
- GitHub Check: Test API / API Test Local
- GitHub Check: Build / Build
- GitHub Check: Test Packages / Test
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (11)
apps/dashboard/lib/collections/ratelimit_overrides.ts (6)
8-15: Split row/create/update schemas; enforce UUIDs and integer bounds (prevents onInsert crash when id is absent).Current schema requires
idon insert and allows non-integer/negative values. This will fail for server‑generated ids and permits invalid limits/durations.-const schema = z.object({ - id: z.string(), - namespaceId: z.string(), - identifier: z.string(), - limit: z.number(), - duration: z.number(), -}); -type Schema = z.infer<typeof schema>; +const rowSchema = z.object({ + id: z.string().uuid("Invalid override id"), + namespaceId: z.string().uuid("Invalid namespace id"), + identifier: z.string().trim().min(1).max(200), + limit: z.number().int().nonnegative(), + duration: z.number().int().positive(), +}); +const createSchema = rowSchema.omit({ id: true }); +const updateSchema = z + .object({ + id: z.string().uuid("Invalid override id"), + limit: z.number().int().nonnegative().optional(), + duration: z.number().int().positive().optional(), + }) + .refine((v) => v.limit !== undefined || v.duration !== undefined, { + message: "Provide at least one of limit or duration", + }); +type Schema = z.infer<typeof rowSchema>;
21-25: Align query options: add retry and silence logs in production.queryClient, queryKey: ["ratelimitOverrides"], + retry: 3, queryFn: async () => { - console.info("DB fetching ratelimitOverrides"); + if (process.env.NODE_ENV !== "production") { + // eslint-disable-next-line no-console + console.info("DB fetching ratelimitOverrides"); + } return await trpcClient.ratelimit.override.list.query(); },
44-56: Guard partial updates; keep UX consistent; revalidate after success.onUpdate: async ({ transaction }) => { const { original, modified } = transaction.mutations[0]; - const mutation = trpcClient.ratelimit.override.update.mutate({ - id: original.id, - limit: modified.limit, - duration: modified.duration, - }); - toast.promise(mutation, { + const mutation = trpcClient.ratelimit.override.update.mutate({ + id: original.id, + limit: modified.limit ?? original.limit, + duration: modified.duration ?? original.duration, + }); + toast.promise(mutation, { loading: "Updating override...", success: "Override updated", - error: "Failed to update override", + error: toErr("update"), }); await mutation; + await queryClient.invalidateQueries({ queryKey: ["ratelimitOverrides"] }); },
57-66: Unify delete error handling and revalidate.onDelete: async ({ transaction }) => { const { original } = transaction.mutations[0]; const mutation = trpcClient.ratelimit.override.delete.mutate({ id: original.id }); toast.promise(mutation, { loading: "Deleting override...", success: "Override deleted", - error: "Failed to delete override", + error: toErr("delete"), }); await mutation; + await queryClient.invalidateQueries({ queryKey: ["ratelimitOverrides"] }); },
70-75: Treat identifiers as case-insensitive in the unique index (prevents confusing duplicates).-ratelimitOverrides.createIndex((row) => [row.namespaceId, row.identifier], { +ratelimitOverrides.createIndex((row) => [row.namespaceId, row.identifier.toLowerCase()], { name: "unique_identifier_per_namespace", options: { unique: true, }, });Run to confirm server-side semantics (case-insensitive uniqueness and update payload expectations):
#!/bin/bash # 1) Check TRPC override routes and input schemas rg -nP -C3 '(create|update|delete|list).*override' apps | sed -n '1,200p' rg -nP -C3 'z\.object\(\s*{\s*id:|namespaceId:|identifier:|limit:|duration:' apps # 2) Look for case-insensitive handling in API/DB (lower(), ILIKE, citext, unique index) rg -nP -C2 '(lower\\(identifier\\)|ILIKE|citext|unique.*identifier.*namespace)' --glob '!**/node_modules/**' apps
1-7: Add a shared toast error mapper to surface server messages uniformly.import { toast } from "@unkey/ui"; import { z } from "zod"; import { queryClient, trpcClient } from "./client"; +const toErr = (op: string) => (res: any) => ({ + message: `Failed to ${op} override`, + description: res?.message ?? res?.data?.message ?? "Unknown error", +}); +apps/dashboard/lib/collections/ratelimit_namespaces.ts (3)
8-14: Harden schema (UUID id, trimmed/validated name) and make it reusableTighten input early and expose a reusable name schema for mutations.
const schema = z.object({ - id: z.string(), - name: z.string(), + id: z.string().uuid("Invalid namespace id"), + name: z + .string() + .trim() + .min(1, "Name is required") + .max(50, "Name must be at most 50 characters") + .regex(/^[A-Za-z0-9_.-]+$/, "Allowed: letters, numbers, _, ., -"), }); -type Schema = z.infer<typeof schema>; +const nameSchema = schema.shape.name; + +type Schema = z.infer<typeof schema>;
21-23: Silence fetch log in productionUnconditional console.info is noisy in prod.
- console.info("DB fetching ratelimitNamespaces"); - return await trpcClient.ratelimit.namespace.list.query(); + if (process.env.NODE_ENV !== "production") { + // eslint-disable-next-line no-console + console.info("DB fetching ratelimitNamespaces"); + } + return trpcClient.ratelimit.namespace.list.query();
72-77: Case-insensitive uniqueness (avoid “Foo” vs “foo”)Mirror likely server-side semantics and UX expectations.
-ratelimitNamespaces.createIndex((row) => row.name, { +ratelimitNamespaces.createIndex((row) => row.name.toLowerCase(), { name: "name", options: { unique: true, }, });apps/dashboard/app/(app)/ratelimits/[namespaceId]/settings/components/settings-client.tsx (2)
18-25: Guard against undefined data and avoid .at on possibly-undefinedDefault data to [] and use index access.
- const { data, isLoading } = useLiveQuery((q) => + const { data = [], isLoading } = useLiveQuery((q) => q .from({ namespace: collection.ratelimitNamespaces }) .where(({ namespace }) => eq(namespace.id, namespaceId)), ); - - const namespace = data.at(0); + const namespace = data[0];
102-107: Disable Save based on trimmed value and equalityPrevents saving names that differ only by whitespace.
- value={namespaceName ?? ""} + value={namespaceName ?? ""} @@ - disabled={namespaceName === namespace.name || !namespaceName} + disabled={ + (namespaceName ?? "").trim() === "" || + (namespaceName ?? "").trim() === namespace.name + }Also applies to: 113-114
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
apps/dashboard/app/(app)/ratelimits/[namespaceId]/settings/components/settings-client.tsx(4 hunks)apps/dashboard/lib/collections/ratelimit_namespaces.ts(1 hunks)apps/dashboard/lib/collections/ratelimit_overrides.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (6)
📚 Learning: 2024-12-05T13:27:55.555Z
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#2707
File: apps/dashboard/lib/trpc/routers/ratelimit/createOverride.ts:63-63
Timestamp: 2024-12-05T13:27:55.555Z
Learning: In `apps/dashboard/lib/trpc/routers/ratelimit/createOverride.ts`, when determining the maximum number of rate limit overrides (`max`), the intentional use of `const max = hasWorkspaceAccess("ratelimitOverrides", namespace.workspace) || 5;` allows `max` to fall back to `5` when `hasWorkspaceAccess` returns `0` or `false`. This fallback behavior is expected and intended in the codebase.
Applied to files:
apps/dashboard/lib/collections/ratelimit_overrides.ts
📚 Learning: 2024-11-13T19:06:36.786Z
Learnt from: chronark
PR: unkeyed/unkey#2126
File: apps/api/src/routes/v1_ratelimit_getOverride.happy.test.ts:36-36
Timestamp: 2024-11-13T19:06:36.786Z
Learning: In the rate limit test files (e.g., `apps/api/src/routes/v1_ratelimit_getOverride.happy.test.ts`), URL parameters like `namespaceId` and `identifier` do not need to be URL-encoded in the test code because the values used are always considered safe within the test environment.
Applied to files:
apps/dashboard/lib/collections/ratelimit_overrides.ts
📚 Learning: 2025-08-25T13:46:34.441Z
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3834
File: apps/dashboard/app/(app)/ratelimits/_components/controls/components/namespace-list-refresh.tsx:4-4
Timestamp: 2025-08-25T13:46:34.441Z
Learning: The namespace list refresh component (apps/dashboard/app/(app)/ratelimits/_components/controls/components/namespace-list-refresh.tsx) intentionally uses the overview hook (useFilters from @/app/(app)/ratelimits/[namespaceId]/_overview/hooks/use-filters) rather than a namespace-specific hook. This cross-coupling between namespace list components and overview hooks is an architectural design decision.
Applied to files:
apps/dashboard/app/(app)/ratelimits/[namespaceId]/settings/components/settings-client.tsx
📚 Learning: 2025-08-25T12:56:59.310Z
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3834
File: apps/dashboard/lib/trpc/routers/ratelimit/query-namespaces/index.ts:59-66
Timestamp: 2025-08-25T12:56:59.310Z
Learning: In the ratelimit namespace query system (apps/dashboard/lib/trpc/routers/ratelimit/query-namespaces/index.ts), the nameQuery filter is designed as an array for future extensibility to support multiple filters, but currently only the first filter (index 0) is processed. This is intentional future-proofing.
Applied to files:
apps/dashboard/app/(app)/ratelimits/[namespaceId]/settings/components/settings-client.tsxapps/dashboard/lib/collections/ratelimit_namespaces.ts
📚 Learning: 2025-08-25T13:46:08.303Z
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3834
File: apps/dashboard/app/(app)/ratelimits/_components/controls/components/namespace-list-datetime/index.tsx:1-1
Timestamp: 2025-08-25T13:46:08.303Z
Learning: The NamespaceListDateTime component in apps/dashboard/app/(app)/ratelimits/_components/controls/components/namespace-list-datetime/index.tsx is intentionally designed to use the overview hook (useFilters from @/app/(app)/ratelimits/[namespaceId]/_overview/hooks/use-filters) rather than the namespace list hook, as clarified by ogzhanolguncu. This coupling is by design, not an architectural issue.
Applied to files:
apps/dashboard/app/(app)/ratelimits/[namespaceId]/settings/components/settings-client.tsx
📚 Learning: 2024-10-04T20:47:34.791Z
Learnt from: chronark
PR: unkeyed/unkey#2180
File: apps/dashboard/lib/constants/workspace-navigations.tsx:85-88
Timestamp: 2024-10-04T20:47:34.791Z
Learning: In navigation code (e.g., `apps/dashboard/lib/constants/workspace-navigations.tsx`), prefer using `segments.at(0)` over `segments[0]` to handle cases when the `segments` array might be empty.
Applied to files:
apps/dashboard/app/(app)/ratelimits/[namespaceId]/settings/components/settings-client.tsx
🧬 Code graph analysis (3)
apps/dashboard/lib/collections/ratelimit_overrides.ts (1)
apps/dashboard/lib/collections/client.ts (1)
trpcClient(12-25)
apps/dashboard/app/(app)/ratelimits/[namespaceId]/settings/components/settings-client.tsx (3)
apps/dashboard/lib/collections/index.ts (1)
collection(6-9)apps/dashboard/app/(app)/ratelimits/[namespaceId]/settings/components/skeleton.tsx (1)
SettingsClientSkeleton(1-60)apps/dashboard/app/(app)/ratelimits/_components/create-namespace-button.tsx (1)
CreateNamespaceButton(28-106)
apps/dashboard/lib/collections/ratelimit_namespaces.ts (1)
apps/dashboard/lib/collections/client.ts (1)
trpcClient(12-25)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Test API / API Test Local
- GitHub Check: Test Go API Local / Test
- GitHub Check: Build / Build
- GitHub Check: Test Packages / Test
apps/dashboard/app/(app)/ratelimits/[namespaceId]/settings/components/settings-client.tsx
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/dashboard/package.json (2)
100-101: Fix conflicting TypeScript versions and scope to dev onlyTypeScript appears in dependencies (^5.7.3) and devDependencies (^5.1.3). Keep a single, latest version in devDependencies to avoid toolchain mismatches.
Apply:
- "typescript": "^5.7.3", ... - "typescript": "^5.1.3", + "typescript": "^5.7.3",Also applies to: 117-118
67-67: Deduplicate build-time tooling (keep in devDependencies only)autoprefixer, postcss, and tailwindcss are listed in both deps and devDeps. These are build-time tools; remove them from dependencies.
- "autoprefixer": "^10.4.19", ... - "postcss": "8.4.38", ... - "tailwindcss": "^3.4.3",They’re already present under devDependencies.
Also applies to: 83-83, 97-97, 113-116
♻️ Duplicate comments (5)
apps/dashboard/package.json (1)
38-41: Avoid dual TanStack Query cores (v4 + v5) in the same appYou’re adding @tanstack/query-core@^5 alongside @tanstack/react-query@^4 (which brings query-core@^4). Shipping both cores risks subtle cache/interop issues and bloat. Either upgrade react-query to v5 or avoid depending on v5-only libs here.
Run to confirm duplicates and peer deps:
#!/bin/bash rg -n "@tanstack/(react-query|query-core|react-db|query-db-collection)" apps/dashboard -C2 pnpm -w why @tanstack/query-core @tanstack/react-query @tanstack/query-db-collection @tanstack/react-dbapps/dashboard/lib/collections/index.ts (1)
6-9: Freeze and type the registry to prevent accidental mutationMake the registry immutable and keep precise types.
-export const collection = { - ratelimitNamespaces, - ratelimitOverrides, -}; +export const collection = Object.freeze({ + ratelimitNamespaces, + ratelimitOverrides, +} as const);apps/dashboard/lib/collections/ratelimit_namespaces.ts (3)
15-23: Retry only transient failuresDon’t retry 4xx validation/auth errors.
- retry: 3, + retry: (failureCount, error: any) => { + const status = error?.data?.httpStatus ?? error?.status; + if (status && status >= 400 && status < 500) return false; + return failureCount < 3; + },
71-76: Make name uniqueness case-insensitive (UX-friendly)Avoids “Foo” and “foo” coexisting.
-ratelimitNamespaces.createIndex((row) => row.name, { +ratelimitNamespaces.createIndex((row) => row.name.toLowerCase(), {
8-11: Strengthen schema (UUID ids, trimmed/validated names)Prevents bad local state and aligns with UI/server rules.
-const schema = z.object({ - id: z.string(), - name: z.string(), -}); +const nameSchema = z + .string() + .trim() + .min(1, "Name is required") + .max(50, "Max 50 characters") + .regex(/^[a-zA-Z0-9_.-]+$/, "Only letters, numbers, _ . -"); + +const schema = z.object({ + id: z.string().uuid("Invalid namespace id"), + name: nameSchema, +});
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
apps/dashboard/lib/collections/index.ts(1 hunks)apps/dashboard/lib/collections/ratelimit_namespaces.ts(1 hunks)apps/dashboard/package.json(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-25T12:56:59.310Z
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3834
File: apps/dashboard/lib/trpc/routers/ratelimit/query-namespaces/index.ts:59-66
Timestamp: 2025-08-25T12:56:59.310Z
Learning: In the ratelimit namespace query system (apps/dashboard/lib/trpc/routers/ratelimit/query-namespaces/index.ts), the nameQuery filter is designed as an array for future extensibility to support multiple filters, but currently only the first filter (index 0) is processed. This is intentional future-proofing.
Applied to files:
apps/dashboard/lib/collections/ratelimit_namespaces.ts
🧬 Code graph analysis (1)
apps/dashboard/lib/collections/ratelimit_namespaces.ts (1)
apps/dashboard/lib/collections/client.ts (1)
trpcClient(12-25)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Test Go API Local / Test
- GitHub Check: Test API / API Test Local
- GitHub Check: Build / Build
🔇 Additional comments (2)
apps/dashboard/package.json (1)
15-16: RHF resolvers bump looks good@hookform/resolvers ^3.4.2 matches react-hook-form ^7.51.x usage.
apps/dashboard/lib/collections/ratelimit_namespaces.ts (1)
24-43: Critical: reconcile optimistic insert with server-generated idReturn
{ refetch: true }fromonInsertto replace the temporary record with the server-assigned ID and trim/validatename:- onInsert: async ({ transaction }) => { - const { changes: newNamespace } = transaction.mutations[0]; - if (!newNamespace.name) { - throw new Error("Namespace name is required"); - } - - const mutation = trpcClient.ratelimit.namespace.create.mutate({ name: newNamespace.name }); + onInsert: async ({ transaction }) => { + const { changes: newNamespace } = transaction.mutations[0]; + const name = nameSchema.parse((newNamespace.name ?? "").trim()); + const mutation = trpcClient.ratelimit.namespace.create.mutate({ name }); toast.promise(mutation, { loading: "Creating namespace...", success: "Namespace created", - error: (res) => { - console.error("Failed to create namespace", res); - return { - message: "Failed to create namespace", - description: res.message, - }; - }, + error: (res: any) => ({ + message: "Failed to create namespace", + description: res?.message ?? "Unknown error", + }), }); - await mutation; + await mutation; + return { refetch: true }; },Ensure your local-first collection’s optimistic ID generator produces UUIDs to match the server schema—please verify the
ratelimitNamespacesID strategy.
Comment on lines
+11
to
+19
| // resets all collections data and preloads new | ||
| export async function reset() { | ||
| await Promise.all( | ||
| Object.values(collection).map(async (c) => { | ||
| await c.cleanup(); | ||
| await c.preload(); | ||
| }), | ||
| ); | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Reset order: cleanup all, then preload all (settled) for robustness
Avoid partial failures aborting the whole reset and ensure all cleanups complete before preloading.
-export async function reset() {
- await Promise.all(
- Object.values(collection).map(async (c) => {
- await c.cleanup();
- await c.preload();
- }),
- );
-}
+export async function reset() {
+ const colls = Object.values(collection);
+ await Promise.allSettled(colls.map((c) => c.cleanup()));
+ await Promise.allSettled(colls.map((c) => c.preload()));
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // resets all collections data and preloads new | |
| export async function reset() { | |
| await Promise.all( | |
| Object.values(collection).map(async (c) => { | |
| await c.cleanup(); | |
| await c.preload(); | |
| }), | |
| ); | |
| } | |
| // resets all collections data and preloads new | |
| export async function reset() { | |
| const colls = Object.values(collection); | |
| await Promise.allSettled(colls.map((c) => c.cleanup())); | |
| await Promise.allSettled(colls.map((c) => c.preload())); | |
| } |
🤖 Prompt for AI Agents
In apps/dashboard/lib/collections/index.ts around lines 11 to 19, the current
reset runs cleanup() and preload() per-collection in parallel which can abort on
a single failure and intermix cleanup and preload; change it to first run
cleanup for all collections and wait for that phase to settle (use
Promise.allSettled on Object.values(collection).map(c => c.cleanup())), then run
preload for all collections and wait for that phase to settle (use
Promise.allSettled on Object.values(collection).map(c => c.preload())), and
surface or log any failures from each phase rather than letting a single
rejection abort the whole reset.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (6)
apps/dashboard/lib/collections/index.ts (2)
6-9: Freeze the registry to prevent accidental mutation and keep exact typesPrevents runtime writes and preserves literal types for keys.
-export const collection = { - ratelimitNamespaces, - ratelimitOverrides, -}; +export const collection = Object.freeze({ + ratelimitNamespaces, + ratelimitOverrides, +} as const);
11-19: Make reset tolerant to partial failures and preload concurrentlyUse
allSettledso one failing collection doesn’t block the rest; also separates cleanup and concurrent preload for better UX.-// resets all collections data and preloads new -export async function reset() { - await Promise.all( - Object.values(collection).map(async (c) => { - await c.cleanup(); - await c.preload(); - }), - ); -} +// resets all collections data and preloads new +export async function reset() { + // run cleanups sequentially to release old resources safely + for (const c of Object.values(collection)) { + await c.cleanup(); + } + // then preload concurrently; don't fail all if one rejects + const results = await Promise.allSettled( + Object.values(collection).map((c) => c.preload()) + ); + const rejected = results.filter((r) => r.status === "rejected"); + if (rejected.length) { + // optionally surface a single combined error or log + // console.warn("Some collections failed to preload", rejected); + } +}apps/dashboard/lib/collections/ratelimit_namespaces.ts (4)
19-23: Retry only transient failuresUnconditional
retry: 3will hammer on validation (4xx) errors. Gate retries to network/5xx.- retry: 3, + retry: (failureCount, error: any) => { + const status = error?.data?.httpStatus ?? error?.status; + if (status >= 400 && status < 500) return false; + return failureCount < 3; + },
24-33: Validate and normalize name before create; avoid empty/whitespace namesAlso prefer passing a trimmed value to the server.
- onInsert: async ({ transaction }) => { + onInsert: async ({ transaction }) => { const { changes: newNamespace } = transaction.mutations[0]; - if (!newNamespace.name) { - throw new Error("Namespace name is required"); - } - - const mutation = trpcClient.ratelimit.namespace.create.mutate({ name: newNamespace.name }); + const name = nameSchema.parse((newNamespace.name ?? "").trim()); + const mutation = trpcClient.ratelimit.namespace.create.mutate({ name }); toast.promise(mutation, { loading: "Creating namespace...", success: "Namespace created",
44-57: Validate name on update and refetch to prevent divergenceTrim/validate before sending and refetch after mutate (or reconcile with server response).
- onUpdate: async ({ transaction }) => { + onUpdate: async ({ transaction }) => { const { original, modified } = transaction.mutations[0]; - - const mutation = trpcClient.ratelimit.namespace.update.name.mutate({ + const name = nameSchema.parse((modified.name ?? "").trim()); + const mutation = trpcClient.ratelimit.namespace.update.name.mutate({ namespaceId: original.id, - name: modified.name, + name, }); toast.promise(mutation, { loading: "Updating namespace...", success: "Namespace updated", error: "Failed to update namespace", }); - await mutation; + await mutation; + return { refetch: true }; },
71-76: Make the unique index case-insensitive (UX: prevent Foo vs foo duplicates)Normalize to lower-case for the index key.
-ratelimitNamespaces.createIndex((row) => row.name, { +ratelimitNamespaces.createIndex((row) => row.name.toLowerCase(), { name: "name", options: { unique: true, }, });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
apps/dashboard/lib/collections/index.ts(1 hunks)apps/dashboard/lib/collections/ratelimit_namespaces.ts(1 hunks)apps/dashboard/package.json(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-25T12:56:59.310Z
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3834
File: apps/dashboard/lib/trpc/routers/ratelimit/query-namespaces/index.ts:59-66
Timestamp: 2025-08-25T12:56:59.310Z
Learning: In the ratelimit namespace query system (apps/dashboard/lib/trpc/routers/ratelimit/query-namespaces/index.ts), the nameQuery filter is designed as an array for future extensibility to support multiple filters, but currently only the first filter (index 0) is processed. This is intentional future-proofing.
Applied to files:
apps/dashboard/lib/collections/ratelimit_namespaces.ts
🧬 Code graph analysis (1)
apps/dashboard/lib/collections/ratelimit_namespaces.ts (1)
apps/dashboard/lib/collections/client.ts (1)
trpcClient(12-25)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Test Go API Local / Test
- GitHub Check: Test API / API Test Local
- GitHub Check: Build / Build
🔇 Additional comments (1)
apps/dashboard/package.json (1)
15-16: RHF resolvers bump looks fine
@hookform/resolvers@^3.4.2remains compatible withreact-hook-form@^7. No issues spotted.
* feat: sync drizzle to go/pkg/db/schema.sql * fix: upgrade drizzle-kit to use required flag * fix: autogeneration notice --------- Co-authored-by: Flo <53355483+Flo4604@users.noreply.github.com>
* feat: script to notify users to migrate to v2 * Apply suggestions from code review * [autofix.ci] apply automated fixes * fix: address james' feedback * merge * fix: exclude analytics --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
chronark
force-pushed
the
09-07-feat_local_first_ratelimits
branch
from
9b3e5bc to
dd64beb
Compare
Collaborator
Author
|
@perkinsjr what zod do you mean? I can't seem to find rabbit comments for it |
Flo4604
approved these changes
Sep 10, 2025
This was referenced Sep 16, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Fixes # (issue)
If there is not an issue for this, please create one first. This is used to tracking purposes and also helps use understand why this PR exists
Type of change
How should this be tested?
Checklist
Required
pnpm buildpnpm fmtconsole.logsgit pull origin mainAppreciated
Summary by CodeRabbit
New Features
Refactor
Bug Fixes
Chores