Conversation
mcstepp
requested review from
MichaelUnkey,
chronark,
ogzhanolguncu and
perkinsjr
as code owners
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
📝 WalkthroughWalkthroughAdds identity-level ratelimit management: new CreateIdentityDialog and EditRatelimitDialog, centralized ratelimit schema and RatelimitSetup refactor to support keys and identities, TRPC routes to create and update identity ratelimits, and import/hook updates to useEditRatelimits across the dashboard. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Table as Identity Table
participant Dialog as EditRatelimitDialog
participant Form as react-hook-form
participant RatelimitSetup as RatelimitSetup UI
participant Hook as useEditRatelimits("identity")
participant TRPC as identity.update.ratelimit
participant Server as Server (handler)
participant DB as Database
User->>Table: Click "Edit ratelimit"
Table->>Dialog: open(identity)
Dialog->>Form: init with identity.ratelimits
Form->>RatelimitSetup: render controls
User->>RatelimitSetup: edit/add/remove rules
User->>Dialog: submit
Dialog->>Form: validate
alt validation fails
Form->>Dialog: show inline errors
else validation passes
Dialog->>Hook: useEditRatelimits("identity").mutateAsync(ratelimitPayload)
Hook->>TRPC: call mutation with payload
TRPC->>Server: auth & input validation
Server->>DB: begin transaction
Server->>DB: delete removed rules / update existing / insert new
DB-->>Server: commit
Server->>Server: insert audit logs
Server-->>TRPC: return success
TRPC-->>Hook: resolve
Hook->>Dialog: onSuccess (toast, invalidate queries)
Dialog->>Table: refresh view
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
mcstepp
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/dialogs/create-identity-dialog.tsx (1)
62-72: Consider wrapping JSON.parse in try-catch for defensive error handling.If
metadata.datacontains invalid JSON (despite schema validation),JSON.parsewill throw. While the schema should prevent this, adding a try-catch provides defense-in-depth.const onSubmit = (data: FormValues) => { - const meta = - data.metadata?.enabled && data.metadata.data ? JSON.parse(data.metadata.data) : null; + let meta = null; + if (data.metadata?.enabled && data.metadata.data) { + try { + meta = JSON.parse(data.metadata.data); + } catch { + toast.error("Invalid metadata", { description: "Please ensure metadata is valid JSON" }); + return; + } + } const ratelimits = data.ratelimit?.enabled && data.ratelimit.data ? data.ratelimit.data : undefined;apps/dashboard/lib/trpc/routers/identity/updateRatelimit.ts (2)
74-79: Consider batching deletes for better performance.Sequential deletes in a loop can be slow with many ratelimits. Consider using a single delete with
inArrayfor better performance.+import { inArray } from "@/lib/db"; + // Delete ratelimits that exist in DB but are not in the input (they were removed) -for (const existing of existingRatelimits) { - if (!inputRatelimitIds.has(existing.id)) { - await tx.delete(schema.ratelimits).where(eq(schema.ratelimits.id, existing.id)); - } -} +const idsToDelete = existingRatelimits + .filter((existing) => !inputRatelimitIds.has(existing.id)) + .map((r) => r.id); +if (idsToDelete.length > 0) { + await tx.delete(schema.ratelimits).where(inArray(schema.ratelimits.id, idsToDelete)); +}
110-112: Throw TRPCError instead of generic Error for consistent error handling.This
throw new Error(...)will be caught and re-thrown asINTERNAL_SERVER_ERROR, but throwing aTRPCErrorwithBAD_REQUESTwould be more semantically correct for this validation failure.} else if (input.ratelimit.enabled) { // Rate limiting is enabled but no rules provided (edge case). Should not happen in v2. - throw new Error("Rate limiting is enabled but no rules were provided"); + throw new TRPCError({ + code: "BAD_REQUEST", + message: "Rate limiting is enabled but no rules were provided", + }); } else {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
apps/dashboard/app/(app)/[workspaceSlug]/apis/[apiId]/_components/create-key/create-key.constants.tsx(1 hunks)apps/dashboard/app/(app)/[workspaceSlug]/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/edit-ratelimits/index.tsx(3 hunks)apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/create-identity-dialog.tsx(0 hunks)apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/dialogs/create-identity-dialog.tsx(1 hunks)apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/dialogs/create-identity.constants.tsx(1 hunks)apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/dialogs/create-identity.schema.ts(1 hunks)apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/dialogs/edit-ratelimit-dialog.tsx(1 hunks)apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/dialogs/general-setup.tsx(1 hunks)apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/table/identity-table-actions.tsx(2 hunks)apps/dashboard/app/(app)/[workspaceSlug]/identities/navigation.tsx(1 hunks)apps/dashboard/app/new/hooks/use-key-creation-step.tsx(1 hunks)apps/dashboard/components/dashboard/ratelimits/ratelimit-setup.tsx(6 hunks)apps/dashboard/hooks/use-edit-ratelimits.ts(2 hunks)apps/dashboard/lib/schemas/ratelimit.ts(1 hunks)apps/dashboard/lib/trpc/routers/identity/create.ts(4 hunks)apps/dashboard/lib/trpc/routers/identity/query.ts(2 hunks)apps/dashboard/lib/trpc/routers/identity/search.ts(1 hunks)apps/dashboard/lib/trpc/routers/identity/updateRatelimit.ts(1 hunks)apps/dashboard/lib/trpc/routers/index.ts(2 hunks)apps/dashboard/lib/trpc/routers/key/updateRatelimit.ts(1 hunks)
💤 Files with no reviewable changes (1)
- apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/create-identity-dialog.tsx
🧰 Additional context used
🧠 Learnings (28)
📓 Common learnings
Learnt from: Flo4604
Repo: unkeyed/unkey PR: 4190
File: go/internal/services/keys/verifier.go:51-53
Timestamp: 2025-10-30T15:10:52.743Z
Learning: PR #4190 for unkeyed/unkey is focused solely on database schema and query changes for identity-based credits. It adds IdentityCredits and KeyCredits fields to structs and queries, but does not implement the priority enforcement logic in the usagelimiter. The logic implementation is intentionally deferred to a later PR in the stack.
Learnt from: Flo4604
Repo: unkeyed/unkey PR: 4484
File: go/pkg/db/queries/identity_list.sql:11-24
Timestamp: 2025-12-05T12:05:33.166Z
Learning: In go/pkg/db/queries/identity_list.sql and similar identity ratelimit queries, the RatelimitInfo struct's key_id and identity_id fields can be omitted from JSON_OBJECT construction because: (1) these are identity-level ratelimits where key_id doesn't apply, and (2) identity_id is redundant since it's already known from the parent identity context. Zero values for these fields are acceptable and intentional.
📚 Learning: 2025-06-19T13:01:55.338Z
Learnt from: ogzhanolguncu
Repo: unkeyed/unkey PR: 3315
File: apps/dashboard/app/(app)/apis/[apiId]/_components/create-key/components/general-setup.tsx:40-50
Timestamp: 2025-06-19T13:01:55.338Z
Learning: In the create-key form's GeneralSetup component, the Controller is intentionally bound to "identityId" as the primary field while "externalId" is set explicitly via setValue. The ExternalIdField component has been designed to handle this pattern where it receives identityId as its value prop but manages both identityId and externalId through its onChange callback.
Applied to files:
apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/dialogs/general-setup.tsx
📚 Learning: 2024-10-04T17:27:09.821Z
Learnt from: chronark
Repo: unkeyed/unkey PR: 2146
File: apps/dashboard/app/(app)/apis/[apiId]/settings/default-prefix.tsx:74-75
Timestamp: 2024-10-04T17:27:09.821Z
Learning: In `apps/dashboard/app/(app)/apis/[apiId]/settings/default-prefix.tsx`, the hidden `<input>` elements for `workspaceId` and `keyAuthId` work correctly without being registered with React Hook Form.
Applied to files:
apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/dialogs/general-setup.tsxapps/dashboard/app/new/hooks/use-key-creation-step.tsxapps/dashboard/app/(app)/[workspaceSlug]/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/edit-ratelimits/index.tsxapps/dashboard/components/dashboard/ratelimits/ratelimit-setup.tsx
📚 Learning: 2025-04-22T11:49:06.167Z
Learnt from: ogzhanolguncu
Repo: unkeyed/unkey PR: 3156
File: apps/dashboard/app/(app)/apis/[apiId]/_components/create-key/index.tsx:112-0
Timestamp: 2025-04-22T11:49:06.167Z
Learning: In the CreateKeyDialog component of Unkey, section navigation is designed to always allow progression even if validation fails, as visual indicators display the validation state of each section.
Applied to files:
apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/dialogs/create-identity-dialog.tsx
📚 Learning: 2025-07-28T20:38:53.244Z
Learnt from: mcstepp
Repo: unkeyed/unkey PR: 3662
File: apps/dashboard/app/(app)/projects/[projectId]/diff/[...compare]/components/client.tsx:322-341
Timestamp: 2025-07-28T20:38:53.244Z
Learning: In apps/dashboard/app/(app)/projects/[projectId]/diff/[...compare]/components/client.tsx, mcstepp prefers to keep hardcoded endpoint logic in the getDiffType function during POC phases for demonstrating diff functionality, rather than implementing a generic diff algorithm. This follows the pattern of keeping simplified implementations for demonstration purposes.
Applied to files:
apps/dashboard/app/(app)/[workspaceSlug]/identities/navigation.tsx
📚 Learning: 2025-06-02T11:08:56.397Z
Learnt from: ogzhanolguncu
Repo: unkeyed/unkey PR: 3292
File: apps/dashboard/lib/vault.ts:80-97
Timestamp: 2025-06-02T11:08:56.397Z
Learning: The vault.ts file in apps/dashboard/lib/vault.ts is a duplicate of the vault package from the `api` directory and should be kept consistent with that original implementation.
Applied to files:
apps/dashboard/app/(app)/[workspaceSlug]/identities/navigation.tsx
📚 Learning: 2025-09-23T17:39:59.820Z
Learnt from: perkinsjr
Repo: unkeyed/unkey PR: 4009
File: apps/dashboard/app/(app)/[workspace]/apis/[apiId]/_overview/components/table/components/override-indicator.tsx:88-97
Timestamp: 2025-09-23T17:39:59.820Z
Learning: The useWorkspaceNavigation hook in the Unkey dashboard guarantees that a workspace exists. If no workspace is found, the hook redirects the user to create a new workspace. Users cannot be logged in without a workspace, and new users must create one to continue. Therefore, workspace will never be null when using this hook.
Applied to files:
apps/dashboard/app/(app)/[workspaceSlug]/identities/navigation.tsx
📚 Learning: 2025-09-23T17:40:44.944Z
Learnt from: perkinsjr
Repo: unkeyed/unkey PR: 4009
File: apps/dashboard/app/(app)/[workspace]/authorization/roles/navigation.tsx:26-40
Timestamp: 2025-09-23T17:40:44.944Z
Learning: In the Unkey dashboard authorization section navigation components, the maintainer prefers to wrap entire navbars in Suspense rather than scoping Suspense to individual components, even if it blocks the whole navigation during loading.
Applied to files:
apps/dashboard/app/(app)/[workspaceSlug]/identities/navigation.tsx
📚 Learning: 2024-10-23T12:05:31.121Z
Learnt from: chronark
Repo: unkeyed/unkey PR: 2544
File: apps/api/src/pkg/env.ts:4-6
Timestamp: 2024-10-23T12:05:31.121Z
Learning: The `cloudflareRatelimiter` type definition in `apps/api/src/pkg/env.ts` should not have its interface changed; it should keep the `limit` method returning `Promise<{ success: boolean }>` without additional error properties.
Applied to files:
apps/dashboard/lib/schemas/ratelimit.tsapps/dashboard/hooks/use-edit-ratelimits.tsapps/dashboard/lib/trpc/routers/identity/query.tsapps/dashboard/lib/trpc/routers/key/updateRatelimit.tsapps/dashboard/lib/trpc/routers/index.tsapps/dashboard/lib/trpc/routers/identity/updateRatelimit.tsapps/dashboard/lib/trpc/routers/identity/search.tsapps/dashboard/app/(app)/[workspaceSlug]/apis/[apiId]/_components/create-key/create-key.constants.tsxapps/dashboard/app/new/hooks/use-key-creation-step.tsxapps/dashboard/components/dashboard/ratelimits/ratelimit-setup.tsx
📚 Learning: 2024-10-20T07:05:55.471Z
Learnt from: chronark
Repo: unkeyed/unkey PR: 2294
File: apps/api/src/pkg/keys/service.ts:268-271
Timestamp: 2024-10-20T07:05:55.471Z
Learning: In `apps/api/src/pkg/keys/service.ts`, `ratelimitAsync` is a table relation, not a column selection. When querying, ensure that table relations are included appropriately, not as columns.
Applied to files:
apps/dashboard/lib/schemas/ratelimit.tsapps/dashboard/hooks/use-edit-ratelimits.tsapps/dashboard/lib/trpc/routers/identity/query.tsapps/dashboard/lib/trpc/routers/key/updateRatelimit.tsapps/dashboard/lib/trpc/routers/identity/create.tsapps/dashboard/lib/trpc/routers/identity/updateRatelimit.tsapps/dashboard/lib/trpc/routers/identity/search.tsapps/dashboard/app/(app)/[workspaceSlug]/apis/[apiId]/_components/create-key/create-key.constants.tsxapps/dashboard/app/(app)/[workspaceSlug]/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/edit-ratelimits/index.tsx
📚 Learning: 2025-04-22T11:48:39.670Z
Learnt from: ogzhanolguncu
Repo: unkeyed/unkey PR: 3156
File: apps/dashboard/app/(app)/apis/[apiId]/_components/create-key/components/ratelimit-setup.tsx:36-47
Timestamp: 2025-04-22T11:48:39.670Z
Learning: The Unkey dashboard's form validation for numeric values like rate limits is handled through the Zod schema validation (with `.positive()` validators and additional checks in `superRefine`), rather than HTML input attributes like `min`.
Applied to files:
apps/dashboard/lib/schemas/ratelimit.tsapps/dashboard/app/(app)/[workspaceSlug]/identities/_components/dialogs/create-identity.schema.tsapps/dashboard/lib/trpc/routers/key/updateRatelimit.ts
📚 Learning: 2025-04-22T11:47:24.733Z
Learnt from: ogzhanolguncu
Repo: unkeyed/unkey PR: 3156
File: apps/dashboard/app/(app)/apis/[apiId]/_components/create-key/components/ratelimit-setup.tsx:48-59
Timestamp: 2025-04-22T11:47:24.733Z
Learning: For form inputs in the Unkey dashboard, validation constraints are handled at the schema level using Zod rather than using HTML attributes like min/step on numeric inputs.
Applied to files:
apps/dashboard/lib/schemas/ratelimit.tsapps/dashboard/app/(app)/[workspaceSlug]/identities/_components/dialogs/create-identity.schema.ts
📚 Learning: 2025-10-30T15:10:52.743Z
Learnt from: Flo4604
Repo: unkeyed/unkey PR: 4190
File: go/internal/services/keys/verifier.go:51-53
Timestamp: 2025-10-30T15:10:52.743Z
Learning: PR #4190 for unkeyed/unkey is focused solely on database schema and query changes for identity-based credits. It adds IdentityCredits and KeyCredits fields to structs and queries, but does not implement the priority enforcement logic in the usagelimiter. The logic implementation is intentionally deferred to a later PR in the stack.
Applied to files:
apps/dashboard/hooks/use-edit-ratelimits.ts
📚 Learning: 2025-08-25T12:56:59.310Z
Learnt from: ogzhanolguncu
Repo: unkeyed/unkey PR: 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/identity/query.tsapps/dashboard/lib/trpc/routers/identity/search.ts
📚 Learning: 2025-12-05T12:05:33.166Z
Learnt from: Flo4604
Repo: unkeyed/unkey PR: 4484
File: go/pkg/db/queries/identity_list.sql:11-24
Timestamp: 2025-12-05T12:05:33.166Z
Learning: In go/pkg/db/queries/identity_list.sql and similar identity ratelimit queries, the RatelimitInfo struct's key_id and identity_id fields can be omitted from JSON_OBJECT construction because: (1) these are identity-level ratelimits where key_id doesn't apply, and (2) identity_id is redundant since it's already known from the parent identity context. Zero values for these fields are acceptable and intentional.
Applied to files:
apps/dashboard/lib/trpc/routers/identity/query.tsapps/dashboard/lib/trpc/routers/identity/create.tsapps/dashboard/lib/trpc/routers/identity/updateRatelimit.ts
📚 Learning: 2025-08-22T12:49:20.668Z
Learnt from: perkinsjr
Repo: unkeyed/unkey PR: 3775
File: apps/dashboard/lib/trpc/routers/api/setDefaultBytes.ts:1-1
Timestamp: 2025-08-22T12:49:20.668Z
Learning: The team at unkeyed/unkey is comfortable with keeping TRPC schema imports from app route folders as temporary technical debt during their migration from TRPC to calling their v2 API directly, since this code will be replaced rather than maintained long-term.
Applied to files:
apps/dashboard/lib/trpc/routers/key/updateRatelimit.ts
📚 Learning: 2025-08-22T12:50:07.024Z
Learnt from: perkinsjr
Repo: unkeyed/unkey PR: 3775
File: apps/dashboard/lib/trpc/routers/api/keys/query-overview-timeseries/index.ts:1-2
Timestamp: 2025-08-22T12:50:07.024Z
Learning: The team at Unkey is comfortable accepting TRPC schema imports from app route folders in server-side TRPC routers as technical debt, since they're planning to migrate away from TRPC to calling their v2 API directly, making these imports temporary and not worth refactoring.
Applied to files:
apps/dashboard/lib/trpc/routers/key/updateRatelimit.ts
📚 Learning: 2025-08-22T12:48:58.289Z
Learnt from: perkinsjr
Repo: unkeyed/unkey PR: 3775
File: apps/dashboard/lib/trpc/routers/api/keys/query-key-usage-timeseries/index.ts:1-1
Timestamp: 2025-08-22T12:48:58.289Z
Learning: The team at Unkey is planning to move from TRPC to calling their v2 API directly. They're comfortable keeping TRPC input schemas imported from app route folders as technical debt since this code will be replaced, rather than refactoring to move schemas to lib/schemas.
Applied to files:
apps/dashboard/lib/trpc/routers/key/updateRatelimit.ts
📚 Learning: 2025-03-19T09:25:59.751Z
Learnt from: Flo4604
Repo: unkeyed/unkey PR: 2955
File: go/apps/api/routes/v2_identities_create_identity/handler.go:162-202
Timestamp: 2025-03-19T09:25:59.751Z
Learning: In the Unkey codebase, input validation for API endpoints is primarily handled through OpenAPI schema validation, which occurs before requests reach the handler code. For example, in the identities.createIdentity endpoint, minimum values for ratelimit duration and limit are defined in the OpenAPI schema rather than duplicating these checks in the handler.
Applied to files:
apps/dashboard/lib/trpc/routers/key/updateRatelimit.ts
📚 Learning: 2025-08-22T12:45:07.187Z
Learnt from: perkinsjr
Repo: unkeyed/unkey PR: 3775
File: apps/dashboard/lib/trpc/routers/workspace/onboarding.ts:1-1
Timestamp: 2025-08-22T12:45:07.187Z
Learning: The team at Unkey is planning to move from TRPC to calling their v2 API directly, making current TRPC schema organization temporary. They're comfortable with keeping server input schemas imported from app route folders as technical debt since this code will be replaced.
Applied to files:
apps/dashboard/lib/trpc/routers/key/updateRatelimit.ts
📚 Learning: 2024-12-05T13:27:55.555Z
Learnt from: ogzhanolguncu
Repo: unkeyed/unkey PR: 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/identity/updateRatelimit.ts
📚 Learning: 2025-08-21T15:54:45.198Z
Learnt from: chronark
Repo: unkeyed/unkey PR: 3825
File: go/internal/services/usagelimiter/limit.go:38-0
Timestamp: 2025-08-21T15:54:45.198Z
Learning: In go/internal/services/usagelimiter/limit.go, the UpdateKeyCreditsDecrement operation cannot be safely wrapped with db.WithRetry due to the lack of idempotency mechanisms in the current tech stack. Retrying this non-idempotent write operation risks double-charging users if the first attempt commits but the client sees a transient error.
Applied to files:
apps/dashboard/lib/trpc/routers/identity/updateRatelimit.ts
📚 Learning: 2025-06-19T11:48:05.070Z
Learnt from: ogzhanolguncu
Repo: unkeyed/unkey PR: 3324
File: apps/dashboard/app/(app)/authorization/roles/components/table/components/actions/keys-table-action.popover.constants.tsx:17-18
Timestamp: 2025-06-19T11:48:05.070Z
Learning: In the authorization roles refactor, the RoleBasic type uses `roleId` as the property name for the role identifier, not `id`. This is consistent throughout the codebase in apps/dashboard/lib/trpc/routers/authorization/roles/query.ts.
Applied to files:
apps/dashboard/lib/trpc/routers/identity/search.ts
📚 Learning: 2025-07-25T19:11:00.208Z
Learnt from: mcstepp
Repo: unkeyed/unkey PR: 3662
File: apps/dashboard/lib/trpc/routers/deployment/getOpenApiDiff.ts:110-147
Timestamp: 2025-07-25T19:11:00.208Z
Learning: In apps/dashboard/lib/trpc/routers/deployment/getOpenApiDiff.ts, the user mcstepp prefers to keep mock data fallbacks in POC/demonstration code for simplicity, even if it wouldn't be production-ready. This aligns with the PR being work-in-progress for demonstration purposes.
Applied to files:
apps/dashboard/lib/trpc/routers/identity/search.ts
📚 Learning: 2025-07-25T19:09:43.284Z
Learnt from: mcstepp
Repo: unkeyed/unkey PR: 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/identity/search.ts
📚 Learning: 2025-08-25T13:46:34.441Z
Learnt from: ogzhanolguncu
Repo: unkeyed/unkey PR: 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)/[workspaceSlug]/apis/[apiId]/_components/create-key/create-key.constants.tsxapps/dashboard/app/new/hooks/use-key-creation-step.tsxapps/dashboard/app/(app)/[workspaceSlug]/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/edit-ratelimits/index.tsx
📚 Learning: 2025-08-25T13:46:08.303Z
Learnt from: ogzhanolguncu
Repo: unkeyed/unkey PR: 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/new/hooks/use-key-creation-step.tsxapps/dashboard/app/(app)/[workspaceSlug]/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/edit-ratelimits/index.tsx
📚 Learning: 2025-01-22T16:52:29.277Z
Learnt from: MichaelUnkey
Repo: unkeyed/unkey PR: 2810
File: internal/ui/src/components/date-time/components/time-split.tsx:0-0
Timestamp: 2025-01-22T16:52:29.277Z
Learning: For time input fields in React, use pattern="[0-9]*" for HTML5 validation and handle specific range validation (hours: 0-23, minutes/seconds: 0-59) in the onChange handler to ensure better user experience and consistent validation behavior.
Applied to files:
apps/dashboard/components/dashboard/ratelimits/ratelimit-setup.tsx
🧬 Code graph analysis (10)
apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/dialogs/general-setup.tsx (1)
apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/dialogs/create-identity.schema.ts (1)
FormValues(17-17)
apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/dialogs/create-identity-dialog.tsx (3)
apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/dialogs/create-identity.schema.ts (3)
FormValues(17-17)formSchema(5-15)getDefaultValues(19-35)apps/dashboard/lib/trpc/routers/identity/create.ts (1)
createIdentity(20-155)apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/dialogs/create-identity.constants.tsx (1)
SECTIONS(7-26)
apps/dashboard/lib/schemas/ratelimit.ts (1)
apps/dashboard/lib/schemas/metadata.ts (1)
createConditionalSchema(4-23)
apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/dialogs/edit-ratelimit-dialog.tsx (5)
apps/dashboard/lib/trpc/routers/identity/query.ts (1)
IdentityResponseSchema(13-31)apps/dashboard/lib/schemas/ratelimit.ts (2)
RatelimitFormValues(49-49)ratelimitSchema(33-45)apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/dialogs/create-identity.schema.ts (1)
getDefaultValues(19-35)apps/dashboard/hooks/use-edit-ratelimits.ts (1)
useEditRatelimits(15-121)apps/dashboard/components/dashboard/ratelimits/ratelimit-setup.tsx (1)
RatelimitSetup(10-217)
apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/dialogs/create-identity.schema.ts (2)
apps/dashboard/lib/schemas/metadata.ts (1)
metadataSchema(52-56)apps/dashboard/lib/schemas/ratelimit.ts (1)
ratelimitSchema(33-45)
apps/dashboard/lib/trpc/routers/identity/create.ts (2)
internal/id/src/generate.ts (1)
newId(36-55)internal/db/src/index.ts (1)
schema(8-8)
apps/dashboard/lib/trpc/routers/index.ts (1)
apps/dashboard/lib/trpc/routers/identity/updateRatelimit.ts (1)
updateIdentityRatelimit(17-47)
apps/dashboard/lib/trpc/routers/identity/updateRatelimit.ts (5)
apps/dashboard/lib/schemas/ratelimit.ts (1)
ratelimitSchema(33-45)apps/dashboard/lib/trpc/trpc.ts (3)
t(8-8)requireUser(10-21)requireWorkspace(23-36)apps/dashboard/lib/db.ts (1)
db(5-26)internal/id/src/generate.ts (1)
newId(36-55)apps/dashboard/lib/audit.ts (2)
UnkeyAuditLog(59-101)insertAuditLogs(103-158)
apps/dashboard/app/(app)/[workspaceSlug]/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/edit-ratelimits/index.tsx (1)
apps/dashboard/hooks/use-edit-ratelimits.ts (1)
useEditRatelimits(15-121)
apps/dashboard/components/dashboard/ratelimits/ratelimit-setup.tsx (1)
apps/dashboard/lib/schemas/ratelimit.ts (2)
RatelimitFormContextValues(52-57)RatelimitItem(48-48)
⏰ 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). (2)
- GitHub Check: Test Dashboard / Test Dashboard
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (32)
apps/dashboard/app/(app)/[workspaceSlug]/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/edit-ratelimits/index.tsx (2)
1-6: LGTM on import centralization.The imports are correctly updated to use the centralized schema and component paths (
@/lib/schemas/ratelimitand@/components/dashboard/ratelimits/ratelimit-setup), aligning with the PR's refactoring goals.
50-66: LGTM on hook usage and payload simplification.The hook is correctly invoked with
"key"entity type, and the payload simplification toratelimit: data.ratelimitis cleaner. The catch block appropriately suppresses console noise while letting the hook handle toast notifications.apps/dashboard/hooks/use-edit-ratelimits.ts (3)
5-15: Well-structured TypeScript overloads for entity type discrimination.The overload pattern correctly provides type-safe return values based on
entityType. When callers use"key", they get the key mutation type; when using"identity", they get the identity mutation type. This enables proper type inference at call sites.
69-118: LGTM on identity mutation handler.The implementation correctly mirrors the key mutation pattern with appropriate identity-specific messaging and cache invalidation (
trpcUtils.identity.query.invalidate()). Error handling is comprehensive with specific messages for NOT_FOUND and INTERNAL_SERVER_ERROR cases.
120-121: Conditional return correctly handles entity type.Both mutations must be instantiated unconditionally per React hooks rules, and the conditional return correctly selects the appropriate one. The TypeScript overloads ensure callers receive the correct return type.
apps/dashboard/app/(app)/[workspaceSlug]/apis/[apiId]/_components/create-key/create-key.constants.tsx (1)
6-6: LGTM on import path update.The import is correctly updated to use the centralized component path, consistent with other files in this PR.
apps/dashboard/app/new/hooks/use-key-creation-step.tsx (1)
13-13: LGTM on import path update.The import is correctly updated to the centralized component path.
apps/dashboard/lib/trpc/routers/index.ts (2)
65-65: LGTM on import addition.The import correctly references the new identity ratelimit update procedure.
349-352: LGTM on identity ratelimit route registration.The route is correctly registered under
identity.update.ratelimit, maintaining structural consistency withkey.update.ratelimit. This aligns with the hook's expectation inuse-edit-ratelimits.tswhich callstrpc.identity.update.ratelimit.useMutation().apps/dashboard/lib/trpc/routers/key/updateRatelimit.ts (1)
3-3: LGTM! Clean refactoring to centralized schema.The import path update to use the centralized ratelimit schema improves maintainability and consistency across the codebase.
apps/dashboard/app/(app)/[workspaceSlug]/identities/navigation.tsx (1)
6-6: LGTM! Import path correctly updated.The import path reflects the new component location in the dialogs subdirectory.
apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/table/identity-table-actions.tsx (1)
5-5: LGTM! Edit ratelimit feature correctly integrated.The implementation correctly adds the "Edit ratelimit..." menu item with proper state management and dialog integration, matching the requirements from issue #4459.
Also applies to: 9-9, 16-16, 20-27, 76-80
apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/dialogs/general-setup.tsx (1)
1-26: LGTM! Clean form component implementation.The GeneralSetup component correctly integrates with react-hook-form and provides appropriate validation feedback for the externalId field.
apps/dashboard/lib/trpc/routers/identity/search.ts (1)
50-53: LGTM! Ratelimit data properly expanded.The additional columns (name, limit, duration, autoApply) provide the necessary data for displaying and editing identity ratelimits in the UI.
apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/dialogs/create-identity.schema.ts (1)
5-35: LGTM! Schema validation correctly implemented.The form schema properly validates the externalId field with appropriate length and whitespace constraints, and correctly merges metadata and ratelimit schemas. Default values are sensible for identity creation.
apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/dialogs/edit-ratelimit-dialog.tsx (1)
23-151: LGTM! Edit ratelimit dialog well-structured.The dialog correctly initializes from existing identity ratelimits, handles form submission with proper error handling, and provides a clean UI with identity information display and the shared RatelimitSetup component.
apps/dashboard/lib/trpc/routers/identity/create.ts (1)
102-103: LGTM! Audit log metadata correctly updated.The audit log properly tracks whether ratelimits were included during identity creation and their count.
apps/dashboard/components/dashboard/ratelimits/ratelimit-setup.tsx (3)
10-15: Well-structured props for entity-type flexibility.The addition of
entityTypewith a sensible default maintains backward compatibility for existing key-based usage while enabling identity-specific behavior.
26-40: Robust error accessor for conditional schema validation.The
getFieldErrorhelper safely navigates the nested error structure fromreact-hook-formwhen using conditional/dynamic schemas. The defensive checks handle edge cases well.
78-81: Dynamic description correctly reflects entity context.The description text appropriately varies based on
entityType, providing clear guidance for both key and identity ratelimit configurations.apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/dialogs/create-identity-dialog.tsx (3)
39-60: Solid error handling with field-level CONFLICT feedback.The mutation properly handles the CONFLICT error by setting a form-level error on
externalId, giving users clear feedback about duplicate external IDs.
81-130: Clean NavigableDialog integration with proper form wiring.The dialog correctly wraps the form with
FormProvider, uses a separateformattribute to connect the submit button outside the form element, and properly manages the open/close state.
117-118: No changes needed. The project uses TanStack Query v4.36.1, whereisLoadingis the correct property for mutations. This is only a concern when upgrading to v5, which renamesisLoadingtoisPending.apps/dashboard/lib/trpc/routers/identity/updateRatelimit.ts (1)
17-47: Well-structured TRPC procedure with proper middleware chain.The procedure correctly uses
requireUserandrequireWorkspacemiddleware, validates input against the combined schema, and handles the identity lookup with appropriate error responses.apps/dashboard/lib/trpc/routers/identity/query.ts (2)
22-30: Schema correctly expanded to support ratelimit editing.The extended
ratelimitsschema includes all fields needed for the edit ratelimit dialog (name,limit,duration,autoApply). This aligns well with the PR objectives.
113-120: Query projection matches the updated response schema.The
with.ratelimits.columnsselection correctly fetches all fields defined inIdentityResponseSchema, ensuring data consistency between the API contract and database query.apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/dialogs/create-identity.constants.tsx (2)
7-26: Clean section definitions with proper entity context.The
SECTIONSconstant follows the established NavigableDialog pattern, correctly passesentityType="identity"to bothRatelimitSetupandMetadataSetup, and usesas constfor proper type inference withStepNamesFrom.
28-28: Type-safe section name derivation.Using
StepNamesFrom<typeof SECTIONS>ensuresDialogSectionNamestays in sync with theSECTIONSarray, preventing typos and enabling autocomplete.apps/dashboard/lib/schemas/ratelimit.ts (4)
1-2: LGTM!The imports are correct and necessary for the schema definitions.
28-31: LGTM!The validation schema correctly enforces that when enabled, at least one rate limit must be present.
33-45: LGTM!The conditional schema pattern with default values provides good UX when ratelimits are disabled, and the default rate limit configuration (10 requests per 1000ms) is reasonable.
47-57: LGTM!The type exports are correctly defined. The manual
RatelimitFormContextValuestype is a pragmatic solution to avoid TypeScript union issues with conditional schemas when using form libraries.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/dashboard/lib/schemas/ratelimit.ts (2)
17-23: Enforce integer constraint for limit field.The PR acceptance criteria specify "limit positive integer", but the current schema allows decimal values. Since rate limits are inherently discrete counts, add
.int()to ensure only integers are accepted.Apply this diff:
limit: z.coerce .number({ errorMap: (issue, { defaultError }) => ({ message: issue.code === "invalid_type" ? "Limit must be a valid number" : defaultError, }), }) - .positive({ message: "Limit must be greater than 0" }), + .int({ message: "Limit must be a whole number" }) + .positive({ message: "Limit must be greater than 0" }),
10-16: Enforce integer constraint for refillInterval field.Milliseconds are inherently integers, and allowing decimal values could lead to unexpected behavior or confusion. Add
.int()to ensure only whole milliseconds are accepted.Apply this diff:
refillInterval: z.coerce .number({ errorMap: (issue, { defaultError }) => ({ message: issue.code === "invalid_type" ? "Duration must be a valid number" : defaultError, }), }) - .min(1000, { message: "Refill interval must be at least 1 second (1000ms)" }), + .int({ message: "Refill interval must be a whole number" }) + .min(1000, { message: "Refill interval must be at least 1 second (1000ms)" }),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/dashboard/lib/schemas/ratelimit.ts(1 hunks)apps/dashboard/lib/trpc/routers/identity/create.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/dashboard/lib/trpc/routers/identity/create.ts
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: mcstepp
Repo: unkeyed/unkey PR: 4508
File: apps/dashboard/lib/trpc/routers/identity/updateRatelimit.ts:141-147
Timestamp: 2025-12-16T16:52:48.628Z
Learning: In apps/dashboard/lib/trpc/routers/identity/updateRatelimit.ts, audit log resources for identity ratelimit updates intentionally use type "ratelimit" with id set to identity.id (rather than type "identity"). This design pattern shows that the ratelimit operations belong to the identity entity.
Learnt from: Flo4604
Repo: unkeyed/unkey PR: 4190
File: go/internal/services/keys/verifier.go:51-53
Timestamp: 2025-10-30T15:10:52.743Z
Learning: PR #4190 for unkeyed/unkey is focused solely on database schema and query changes for identity-based credits. It adds IdentityCredits and KeyCredits fields to structs and queries, but does not implement the priority enforcement logic in the usagelimiter. The logic implementation is intentionally deferred to a later PR in the stack.
Learnt from: Flo4604
Repo: unkeyed/unkey PR: 4484
File: go/pkg/db/queries/identity_list.sql:11-24
Timestamp: 2025-12-05T12:05:33.166Z
Learning: In go/pkg/db/queries/identity_list.sql and similar identity ratelimit queries, the RatelimitInfo struct's key_id and identity_id fields can be omitted from JSON_OBJECT construction because: (1) these are identity-level ratelimits where key_id doesn't apply, and (2) identity_id is redundant since it's already known from the parent identity context. Zero values for these fields are acceptable and intentional.
📚 Learning: 2025-12-16T16:52:48.628Z
Learnt from: mcstepp
Repo: unkeyed/unkey PR: 4508
File: apps/dashboard/lib/trpc/routers/identity/updateRatelimit.ts:141-147
Timestamp: 2025-12-16T16:52:48.628Z
Learning: In apps/dashboard/lib/trpc/routers/identity/updateRatelimit.ts, audit log resources for identity ratelimit updates intentionally use type "ratelimit" with id set to identity.id (rather than type "identity"). This design pattern shows that the ratelimit operations belong to the identity entity.
Applied to files:
apps/dashboard/lib/schemas/ratelimit.ts
📚 Learning: 2024-10-23T12:05:31.121Z
Learnt from: chronark
Repo: unkeyed/unkey PR: 2544
File: apps/api/src/pkg/env.ts:4-6
Timestamp: 2024-10-23T12:05:31.121Z
Learning: The `cloudflareRatelimiter` type definition in `apps/api/src/pkg/env.ts` should not have its interface changed; it should keep the `limit` method returning `Promise<{ success: boolean }>` without additional error properties.
Applied to files:
apps/dashboard/lib/schemas/ratelimit.ts
📚 Learning: 2024-10-20T07:05:55.471Z
Learnt from: chronark
Repo: unkeyed/unkey PR: 2294
File: apps/api/src/pkg/keys/service.ts:268-271
Timestamp: 2024-10-20T07:05:55.471Z
Learning: In `apps/api/src/pkg/keys/service.ts`, `ratelimitAsync` is a table relation, not a column selection. When querying, ensure that table relations are included appropriately, not as columns.
Applied to files:
apps/dashboard/lib/schemas/ratelimit.ts
📚 Learning: 2025-04-22T11:48:39.670Z
Learnt from: ogzhanolguncu
Repo: unkeyed/unkey PR: 3156
File: apps/dashboard/app/(app)/apis/[apiId]/_components/create-key/components/ratelimit-setup.tsx:36-47
Timestamp: 2025-04-22T11:48:39.670Z
Learning: The Unkey dashboard's form validation for numeric values like rate limits is handled through the Zod schema validation (with `.positive()` validators and additional checks in `superRefine`), rather than HTML input attributes like `min`.
Applied to files:
apps/dashboard/lib/schemas/ratelimit.ts
📚 Learning: 2025-03-19T09:25:59.751Z
Learnt from: Flo4604
Repo: unkeyed/unkey PR: 2955
File: go/apps/api/routes/v2_identities_create_identity/handler.go:162-202
Timestamp: 2025-03-19T09:25:59.751Z
Learning: In the Unkey codebase, input validation for API endpoints is primarily handled through OpenAPI schema validation, which occurs before requests reach the handler code. For example, in the identities.createIdentity endpoint, minimum values for ratelimit duration and limit are defined in the OpenAPI schema rather than duplicating these checks in the handler.
Applied to files:
apps/dashboard/lib/schemas/ratelimit.ts
📚 Learning: 2025-04-22T11:47:24.733Z
Learnt from: ogzhanolguncu
Repo: unkeyed/unkey PR: 3156
File: apps/dashboard/app/(app)/apis/[apiId]/_components/create-key/components/ratelimit-setup.tsx:48-59
Timestamp: 2025-04-22T11:47:24.733Z
Learning: For form inputs in the Unkey dashboard, validation constraints are handled at the schema level using Zod rather than using HTML attributes like min/step on numeric inputs.
Applied to files:
apps/dashboard/lib/schemas/ratelimit.ts
🔇 Additional comments (3)
apps/dashboard/lib/schemas/ratelimit.ts (3)
27-46: LGTM! Name uniqueness validation correctly implemented.The
superRefineimplementation properly enforces unique ratelimit names using a Set for efficient tracking and correctly reports validation errors with the appropriate path. This addresses the acceptance criteria requiring "non-empty unique names."
48-60: LGTM! Conditional schema and defaults are well-structured.The use of
createConditionalSchemawith sensible defaults (enabled: false, with a default ratelimit entry) provides a good user experience and allows flexible ratelimit configuration.
62-72: LGTM! Type exports are well-defined.The type exports provide good type safety, and the manual
RatelimitFormContextValuestype is a pragmatic solution to avoid conditional schema union issues in form contexts.
2 participants
What does this PR do?
Added edit ratelimits functionality for identities, bringing feature parity with keys.
Refactoring:
Backend:
Fixes #4459
Type of change
How should this be tested?
Setup: workspace is opted into identities feature
Edit existing identity ratelimits:
Create identity with ratelimits:
Verify key ratelimits still work:
Checklist
Required
pnpm buildpnpm fmtmake fmton/godirectoryconsole.logsgit pull origin mainAppreciated