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.
|
📝 WalkthroughWalkthroughAdds a client-driven identities UI (search, controls, virtualized infinite table, per-row actions, skeletons, create-identity dialog) and backend support (identity query/search enhancements, totalCount, ClickHouse latest-verification endpoint, SQL LIKE escaping). Removes legacy list/search/row components. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Browser
participant TRPC as tRPC Client
participant Backend
participant DB as Prisma
participant CH as ClickHouse
User->>Browser: Open identities page
Browser->>TRPC: identity.query (useInfiniteQuery, limit=50, optional search)
TRPC->>Backend: request identities + totalCount
Backend->>DB: SELECT identities (+ keys, ratelimits) and COUNT with filters
DB-->>Backend: identities[], totalCount, nextCursor
Backend-->>TRPC: response
TRPC-->>Browser: render virtualized table (skeletons while loading)
sequenceDiagram
participant User
participant Browser
participant TRPC as tRPC Client
participant Backend
participant CH as ClickHouse
User->>Browser: Hover/inspect "Last used" cell
Browser->>TRPC: latestVerification({ identityId })
TRPC->>Backend: request latest verification
Backend->>CH: SELECT MAX(last_used) FROM key_verifications_raw_v2 WHERE workspace_id AND identity_id
CH-->>Backend: last_used
Backend-->>TRPC: { lastVerificationTime }
TRPC-->>Browser: render timestamp or "Never used"
sequenceDiagram
participant User
participant Browser
participant Form as react-hook-form
participant TRPC as tRPC Client
participant Backend
participant DB as Prisma
User->>Browser: Click "Create Identity"
Browser->>User: Show dialog (form)
User->>Form: Fill externalId + metadata
Form->>Browser: Validate via zod (length, JSON, size)
alt Valid
Browser->>TRPC: mutation identity.create({ externalId, meta })
TRPC->>Backend: create identity
Backend->>DB: INSERT identity
alt Duplicate (409)
DB-->>Backend: constraint error
Backend-->>TRPC: TRPCError CONFLICT
TRPC-->>Browser: set externalId field error
else Success
DB-->>Backend: created identity
Backend-->>TRPC: success
TRPC-->>Browser: success -> toast, invalidate identity queries
end
else Invalid
Browser->>User: show validation errors
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Areas needing extra attention:
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/create-identity-dialog.tsx (1)
86-92: Consider defensive error handling for JSON parsing.While the zod schema validation ensures
metais valid JSON before reaching this point, wrappingJSON.parsein a try-catch would provide defense-in-depth against edge cases (e.g., if validation is bypassed or the schema changes).Apply this diff to add defensive error handling:
const onSubmit = (data: FormValues) => { - const meta = data.meta?.trim() ? JSON.parse(data.meta) : null; + let meta = null; + if (data.meta?.trim()) { + try { + meta = JSON.parse(data.meta); + } catch (error) { + console.error("Failed to parse metadata JSON:", error); + setError("meta", { message: "Invalid JSON format" }); + return; + } + } createIdentity.mutate({ externalId: data.externalId, meta, }); };apps/dashboard/lib/trpc/routers/identity/latestVerification.ts (1)
37-52: Redundant nested error handling.The
TRPCErrorthrown at line 38-41 is immediately caught by the outercatchblock at line 47, which logs and re-throws a newTRPCErrorwith the same code and similar message. This adds unnecessary overhead and duplicates error messages in logs.- if (result.err) { - throw new TRPCError({ - code: "INTERNAL_SERVER_ERROR", - message: "Something went wrong when fetching data from ClickHouse.", - }); - } - - return { - lastVerificationTime: result.val.length > 0 ? result.val[0].last_used : null, - }; - } catch (error) { - console.error("Error querying last verification for identity:", error); - throw new TRPCError({ - code: "INTERNAL_SERVER_ERROR", - message: "Something went wrong when fetching data from ClickHouse.", - }); - } + if (result.err) { + console.error("Error querying last verification for identity:", result.err); + throw new TRPCError({ + code: "INTERNAL_SERVER_ERROR", + message: "Something went wrong when fetching data from ClickHouse.", + }); + } + + return { + lastVerificationTime: result.val.length > 0 ? result.val[0].last_used : null, + };apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/table/identities-list.tsx (1)
83-93: Map is fully rebuilt on each data update.The
useEffectcreates a newMapfrom scratch on everyidentitiesDatachange. For incremental pagination, consider merging new pages into the existing map to preserve references and avoid unnecessary re-renders.useEffect(() => { if (identitiesData) { - const newMap = new Map<string, Identity>(); - identitiesData.pages.forEach((page) => { - page.identities.forEach((identity) => { - newMap.set(identity.id, identity); + setIdentitiesMap((prevMap) => { + const newMap = new Map(prevMap); + identitiesData.pages.forEach((page) => { + page.identities.forEach((identity) => { + newMap.set(identity.id, identity); + }); }); + return newMap; }); - setIdentitiesMap(newMap); } }, [identitiesData]);apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/table/skeletons.tsx (2)
36-45: Consider using a non-interactive element for skeleton button.Using a
<button>element for a loading skeleton means screen readers may announce an interactive element that does nothing. A<div>with appropriate styling would be semantically cleaner.-export const ActionColumnSkeleton = () => ( - <button - type="button" +export const ActionColumnSkeleton = () => ( + <div + role="presentation" className={cn( "group size-5 p-0 rounded m-0 items-center flex justify-center animate-pulse", "border border-gray-6", )} > <Dots className="text-gray-11" iconSize="sm-regular" /> - </button> + </div> );
16-18: Remove unusedExternalIdColumnSkeletonexport.The export at line 16 is never imported or used anywhere in the codebase. Removing it will reduce dead code.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/controls/identities-search.tsx(1 hunks)apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/controls/index.tsx(1 hunks)apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/create-identity-dialog.tsx(1 hunks)apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/identities-client.tsx(1 hunks)apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/table/identities-list.tsx(1 hunks)apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/table/identity-table-actions.tsx(1 hunks)apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/table/last-used.tsx(1 hunks)apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/table/skeletons.tsx(1 hunks)apps/dashboard/app/(app)/[workspaceSlug]/identities/components/results.tsx(0 hunks)apps/dashboard/app/(app)/[workspaceSlug]/identities/filter.tsx(0 hunks)apps/dashboard/app/(app)/[workspaceSlug]/identities/navigation.tsx(2 hunks)apps/dashboard/app/(app)/[workspaceSlug]/identities/page.tsx(2 hunks)apps/dashboard/app/(app)/[workspaceSlug]/identities/row.tsx(0 hunks)apps/dashboard/lib/trpc/routers/identity/latestVerification.ts(1 hunks)apps/dashboard/lib/trpc/routers/identity/query.ts(4 hunks)apps/dashboard/lib/trpc/routers/identity/search.ts(2 hunks)apps/dashboard/lib/trpc/routers/index.ts(2 hunks)
💤 Files with no reviewable changes (3)
- apps/dashboard/app/(app)/[workspaceSlug]/identities/components/results.tsx
- apps/dashboard/app/(app)/[workspaceSlug]/identities/filter.tsx
- apps/dashboard/app/(app)/[workspaceSlug]/identities/row.tsx
🧰 Additional context used
🧠 Learnings (14)
📓 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.
📚 Learning: 2025-07-28T19:42:37.047Z
Learnt from: mcstepp
Repo: unkeyed/unkey PR: 3662
File: apps/dashboard/app/(app)/projects/page.tsx:74-81
Timestamp: 2025-07-28T19:42:37.047Z
Learning: In apps/dashboard/app/(app)/projects/page.tsx, the user mcstepp prefers to keep placeholder functions like generateSlug inline during POC/demonstration phases rather than extracting them to utility modules, with plans to refactor later when the feature matures beyond the proof-of-concept stage.
Applied to files:
apps/dashboard/app/(app)/[workspaceSlug]/identities/page.tsxapps/dashboard/app/(app)/[workspaceSlug]/identities/_components/table/skeletons.tsx
📚 Learning: 2025-08-18T10:28:47.391Z
Learnt from: ogzhanolguncu
Repo: unkeyed/unkey PR: 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)/[workspaceSlug]/identities/page.tsxapps/dashboard/app/(app)/[workspaceSlug]/identities/_components/identities-client.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/page.tsxapps/dashboard/app/(app)/[workspaceSlug]/identities/navigation.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/_components/identities-client.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-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/index.ts
📚 Learning: 2025-07-28T20:36:36.865Z
Learnt from: mcstepp
Repo: unkeyed/unkey PR: 3662
File: apps/dashboard/lib/trpc/routers/branch/getByName.ts:0-0
Timestamp: 2025-07-28T20:36:36.865Z
Learning: In apps/dashboard/lib/trpc/routers/branch/getByName.ts, mcstepp prefers to keep mock data (gitCommitMessage, buildDuration, lastCommitAuthor, etc.) in the branch procedure during POC phases to demonstrate what the UI would look like with proper schema changes, rather than returning null/undefined values.
Applied to files:
apps/dashboard/lib/trpc/routers/identity/latestVerification.ts
📚 Learning: 2024-12-03T14:17:08.016Z
Learnt from: ogzhanolguncu
Repo: unkeyed/unkey PR: 2143
File: apps/dashboard/app/(app)/logs/logs-page.tsx:77-83
Timestamp: 2024-12-03T14:17:08.016Z
Learning: The `<LogsTable />` component already implements virtualization to handle large datasets efficiently.
Applied to files:
apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/table/identities-list.tsx
📚 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/create-identity-dialog.tsx
📚 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]/identities/_components/controls/index.tsx
📚 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/lib/trpc/routers/identity/query.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.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/trpc/routers/identity/search.ts
🧬 Code graph analysis (11)
apps/dashboard/app/(app)/[workspaceSlug]/identities/page.tsx (1)
apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/identities-client.tsx (1)
IdentitiesClient(6-13)
apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/identities-client.tsx (2)
apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/controls/index.tsx (1)
IdentitiesListControls(4-12)apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/table/identities-list.tsx (1)
IdentitiesList(47-320)
apps/dashboard/app/(app)/[workspaceSlug]/identities/navigation.tsx (1)
apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/create-identity-dialog.tsx (1)
CreateIdentityDialog(44-155)
apps/dashboard/lib/trpc/routers/index.ts (1)
apps/dashboard/lib/trpc/routers/identity/latestVerification.ts (1)
identityLastVerificationTime(10-54)
apps/dashboard/lib/trpc/routers/identity/latestVerification.ts (1)
apps/dashboard/lib/trpc/trpc.ts (3)
t(8-8)requireUser(10-21)requireWorkspace(23-36)
apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/controls/identities-search.tsx (1)
internal/ui/src/components/llm-search/index.tsx (1)
LLMSearch(176-176)
apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/table/identities-list.tsx (7)
apps/dashboard/lib/trpc/routers/identity/query.ts (1)
IdentityResponseSchema(12-22)apps/dashboard/app/(app)/[workspaceSlug]/identities/page.tsx (1)
dynamic(6-6)internal/ui/src/components/buttons/copy-button.tsx (1)
CopyButton(27-81)apps/dashboard/lib/shorten-id.ts (1)
shortenId(5-54)apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/table/last-used.tsx (1)
LastUsedCell(6-60)apps/dashboard/components/virtual-table/index.tsx (1)
VirtualTable(38-407)apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/table/skeletons.tsx (5)
IdentityColumnSkeleton(4-14)CountColumnSkeleton(20-22)CreatedColumnSkeleton(24-26)LastUsedColumnSkeleton(28-34)ActionColumnSkeleton(36-46)
apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/create-identity-dialog.tsx (6)
apps/dashboard/app/(app)/[workspaceSlug]/apis/[apiId]/_components/create-key/create-key.schema.ts (1)
FormValues(351-351)internal/ui/src/components/toaster.tsx (1)
toast(29-29)apps/dashboard/lib/collections/index.ts (1)
reset(76-84)internal/ui/src/components/dialog/dialog-container.tsx (1)
DialogContainer(66-66)internal/ui/src/components/buttons/button.tsx (1)
Button(439-439)internal/ui/src/components/form/form-input.tsx (1)
FormInput(56-56)
apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/controls/index.tsx (2)
apps/dashboard/components/logs/controls-container.tsx (2)
ControlsContainer(1-7)ControlsLeft(9-11)apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/controls/identities-search.tsx (1)
IdentitiesSearch(6-31)
apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/table/identity-table-actions.tsx (2)
apps/dashboard/lib/trpc/routers/identity/query.ts (1)
IdentityResponseSchema(12-22)apps/dashboard/components/logs/table-action.popover.tsx (2)
MenuItem(19-29)TableActionPopover(36-169)
apps/dashboard/lib/trpc/routers/identity/query.ts (1)
internal/rbac/src/queries.ts (2)
or(46-50)and(52-56)
⏰ 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 (20)
apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/create-identity-dialog.tsx (3)
12-40: LGTM: Form validation schema is well-structured.The validation covers all requirements from the PR objectives:
- External ID: 3-255 characters with whitespace trimming and validation
- Metadata: Optional JSON with proper syntax validation and 1MB size limit
63-84: LGTM: Error handling correctly distinguishes between conflicts and generic errors.The success flow properly invalidates queries to refresh the identities list, and the error handling appropriately sets inline form errors for duplicate external IDs while showing toast messages for other failures.
94-154: LGTM: Dialog structure and form submission flow are well-implemented.The use of the
formattribute on the submit button to connect it with the form ID is the correct pattern for forms with external submit buttons. Loading states and validation feedback are properly handled.apps/dashboard/app/(app)/[workspaceSlug]/identities/navigation.tsx (1)
6-6: LGTM: CreateIdentityDialog correctly integrated into navigation.The integration follows the established pattern for navbar actions in the dashboard.
Also applies to: 18-20
apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/controls/index.tsx (1)
1-12: LGTM: Clean composition following established patterns.The component correctly reuses the existing controls container components and integrates the IdentitiesSearch component.
apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/controls/identities-search.tsx (1)
1-31: LGTM: LLMSearch integration is correctly implemented.The hardcoded
isLoading={false}is appropriate since the search appears to be client-side filtering based on the query state management pattern. The example queries are helpful for user guidance.apps/dashboard/lib/trpc/routers/index.ts (1)
56-56: LGTM: latestVerification endpoint correctly added to identity router.The addition follows the established pattern for extending TRPC routers in this codebase.
Also applies to: 342-342
apps/dashboard/app/(app)/[workspaceSlug]/identities/page.tsx (1)
1-21: LGTM: Page simplification improves maintainability.The new structure delegates identity listing concerns to the
IdentitiesClientcomponent, resulting in cleaner separation of concerns while maintaining the necessary beta feature guard.apps/dashboard/lib/trpc/routers/identity/search.ts (1)
31-65: LGTM: Relational data fetching enhances identity responses.The addition of related keys and ratelimits aligns with the expanded identity data model. The selective column fetching (
idonly) for related entities is efficient given the small result set (limit of 5).apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/table/identity-table-actions.tsx (1)
10-50: LGTM: Clean implementation with proper error handling.The component correctly uses the Clipboard API with promise-based error handling and user feedback via toasts. The memoization with appropriate dependencies prevents unnecessary re-renders.
apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/identities-client.tsx (1)
1-13: LGTM!Clean composition component that properly combines controls and list. The "use client" directive is correctly placed, and the flex layout is appropriate for stacking these components.
apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/table/last-used.tsx (2)
22-58: Well-structured loading and error states.The component handles all three states (loading, error, success) appropriately with good UX patterns. The controlled tooltip integration with
TimestampInfois clean.
7-18: No changes needed — the pattern is established and pragmatic.The
skipBatch: trueapproach with per-row queries is a standard pattern throughout the codebase and is already mitigated by virtualization. The identities table usesVirtualTablewith a 50-item limit and infinite scroll, which prevents excessive concurrent queries. Given the team's stated preference to implement optimizations only when they become necessary, this suggestion constitutes premature optimization. Remove the performance concern unless there's evidence of actual performance issues.apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/table/identities-list.tsx (3)
47-105: Good implementation of search, navigation, and state management.The query state integration with
nuqs, infinite query setup, and navigation handling are well-structured. Using a Map for identity lookups is appropriate for quick access patterns.
107-230: Well-designed column configuration with proper UI patterns.External ID truncation at 50 chars,
shortenIdfor identity IDs, tooltips with copy functionality, and proper loading states follow the PR requirements. The dynamic import for actions with a loading placeholder is a nice touch for code splitting.
11-11: The import path@unkey/ui/src/lib/utilsis the correct and intended way to use thecnutility. The@unkey/uipackage does not exportcnfrom its public entry point (@unkey/ui), keeping it as an internal utility. This import pattern is consistently used across 30+ files in the codebase and is the only available method to access thecnfunction. No changes are needed.apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/table/skeletons.tsx (1)
1-14: Consistent and well-structured skeleton components.The skeletons appropriately mirror the structure of their corresponding data cells, providing good visual loading feedback. Consistent use of
grayA-3andanimate-pulsemaintains UI coherence.Also applies to: 20-34
apps/dashboard/lib/trpc/routers/identity/query.ts (3)
6-10: LGTM on schema extension.The addition of the optional
searchparameter aligns with the PR objective to support search-based filtering for identities.
60-71: LGTM on relational data fetching.The
withclause correctly fetches only theidcolumn for relatedkeysandratelimits, minimizing data transfer while providing the counts needed for the UI.
82-92: LGTM on response transformation.The transformed identities correctly include the related
keysandratelimitsarrays, matching the updatedIdentityResponseSchema.
apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/table/identities-list.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/dashboard/app/integrations/vercel/callback/client.tsx (1)
1-6: Remove the unrelated change to this deprecated file or clarify its purpose.Line 23 (commented
WorkspaceSwitcherimport) appears incidental to the commit message "fix: escape LIKE" and unrelated to the PR's stated objectives (identities list styling and Create Identity flow). Since the file is explicitly marked as deprecated and "hidden for now" (lines 1-4), modifying it—even to clean up unused imports—introduces unnecessary changes. Either remove this change or include it in a separate PR focused on deprecation cleanup with clear justification in the commit message.
🧹 Nitpick comments (25)
apps/dashboard/app/(app)/[workspaceSlug]/settings/team/client.tsx (1)
85-102: Admin header block wiring looks good; consider dropping unnecessarykeypropsThe new admin-only header (tab
Select+InviteButton) is correctly gated byisAdmin, wired to the existingtabstate, and safe with respect touser/organizationthanks to the earlier null guard.The
keyprops onSelectandInviteButtonare redundant here because these components are not rendered via an array/map; removing them would slightly reduce noise without changing behavior.- <div className="flex flex-row justify-end w-full gap-4"> - <div> - <Select key="tab-select" value={tab} onValueChange={(value: Tab) => setTab(value)}> + <div className="flex flex-row justify-end w-full gap-4"> + <div> + <Select value={tab} onValueChange={(value: Tab) => setTab(value)}> @@ - </div> - <InviteButton key="invite-button" user={user} organization={organization} /> + </div> + <InviteButton user={user} organization={organization} />go/apps/api/openapi/openapi-generated.yaml (1)
5449-5450: Clarify “always returns 200” to match new 403 exception.Great addition explaining 403 vs 200/NOT_FOUND. For consistency, adjust the earlier claim that this endpoint “always returns HTTP 200” (Line 5436) to reflect the permission exception you document here.
Suggested tweak:
-**Important**: Always returns HTTP 200. Check the `valid` field in response data to determine if the key is authorized. +**Important**: Returns HTTP 200 for verification outcomes. Authentication/permission errors may return 4xx (e.g., 403 when the root key has no verify permissions). Check the `valid` field in response data to determine if the key is authorized.Also applies to: 5436-5436
go/apps/api/routes/v2_keys_verify_key/403_test.go (2)
35-57: 403 scenario is well-covered; consider asserting error code and minimizing detail couplingThis subtest nicely checks the 403 path and ensures the error detail hints at both wildcard and specific verify permissions, but you might also assert the structured error code (e.g.,
FORBIDDEN) to make the contract stronger and rely a bit less on the exact phrasing ofError.Detail, which can be more brittle over time.
84-122: VALID-path subtests are solid; table-driven grouping could reduce repetitionBoth VALID cases (wildcard vs specific API permission) are clearly expressed and assert the important bits (
Status,Code,Valid), but they share almost all setup and assertions; if you touch this again, you could fold the four scenarios into a small table-driven test to cut duplication and align with the repo’s direction toward table-based tests. Based on learnings, there’s already an issue open to track that broader refactor.apps/dashboard/app/(app)/[workspaceSlug]/settings/team/members.tsx (2)
26-28: ConfirmPopover anchoring and state wiring look good; consider clearing state on closeThe shared
ConfirmPopoveranchored viaanchorRefand driven byisConfirmPopoverOpenis a nice improvement over per-row confirms. One small cleanup would be to resetcurrentMembershipandanchorElwhen the popover is closed, so you don’t retain stale references after cancel/close:const handleConfirmPopoverOpenChange = (open: boolean) => { setIsConfirmPopoverOpen(open); if (!open) { setCurrentMembership(null); setAnchorEl(null); } }; // … <ConfirmPopover isOpen={isConfirmPopoverOpen} onOpenChange={handleConfirmPopoverOpenChange} triggerRef={anchorRef} // ... />This keeps the state model tightly aligned with the popover’s visibility and avoids holding onto an old DOM node unnecessarily.
Also applies to: 34-34, 75-97
14-14: Align loading state UI with existing Empty-based patterns (optional)You’re importing both
EmptyandLoadingfrom@unkey/ui, and already useEmptyfor the “no team members” state. In other parts of this repo,Emptyis sometimes used as the container even for loading states (wrapping a loader) for a consistent look and feel (based on learnings). You could optionally refactor theisLoadingbranch to reuse<Empty>as the wrapper instead of a custom bordered div, to keep visual parity with other settings views.This is stylistic only; current code is functionally fine.
Also applies to: 47-52, 64-71
apps/dashboard/app/api/webhooks/stripe/route.ts (2)
33-92: Automated renewal detector is conservative; watch multi‑item subs and Stripe semanticsThe renewal detection logic is intentionally conservative (falls back to “manual change” unless it’s clearly just period dates + items/latest_invoice), which is a good safety bias. Two things to keep in mind:
- It only inspects
items.data[0], so if you ever support multi‑item subscriptions, this could misclassify some changes and might need to be generalized.- Correctness relies on how Stripe populates
event.data.previous_attributesforcustomer.subscription.updated(especially for auto‑renew vs scheduled upgrades/downgrades). It’s worth double‑checking a few real webhook samples in staging to ensure the key set/shape matches these assumptions.No blocking issues, but I’d validate this against live/staging events before relying on it heavily.
181-187: Early return for automated renewals looks correct; consider status code/messageSkipping DB/quota updates and notifications for detected automated renewals matches the helper’s intent and avoids noisy Slack alerts. The only minor nit is returning
"Skip"with HTTP 201 (Created) — a 200 with a more conventional body (e.g."OK") might be clearer for logs/metrics, but Stripe will treat any 2xx as success either way.go/pkg/db/queries/identity_list.sql (1)
11-29: Cursor conditioni.id >= sqlc.arg(id_cursor)can duplicate rows across pagesWith
AND i.id >= sqlc.arg(id_cursor), if the caller uses the last seenidas the next page’s cursor (typical pattern), that row will reappear on the following page. If the intended behavior is “start after this id”, consider switching to>(or adjusting how the cursor value is computed) to avoid duplicates:AND i.id > sqlc.arg(id_cursor)Please double‑check how the TRPC/API layer is populating
id_cursorto ensure the pagination semantics are what you expect.deployment/docker-compose.yaml (1)
83-99: Vault / ACME Vault config in docker-compose is coherent for dev useThe UNKEY_VAULT_* (bucket
vault) and UNKEY_ACME_VAULT_* (bucketacme-vault) settings are consistent across apiv2, gw, agent, and ctrl and line up with the MinIO bucket name.Since this compose file is dev‑only, hardcoded master keys and API keys are acceptable here, but make sure these exact values are not reused in any staging or production environment.
Also applies to: 135-143, 276-287, 377-387
go/pkg/db/identity_list.sql_generated.go (1)
8-11: Generated ListIdentities row/scan logic matches the SQL, but Ratelimits typing is very looseThe generated SQL string, params, and scan order all line up with the updated
identity_list.sql, including the ratelimits JSON column and theid_cursorfilter. From a generation standpoint this looks correct.Two follow‑ups to consider (in
identity_list.sql/ sqlc config rather than editing this file directly):
- Cursor semantics: As noted on the SQL file,
AND i.id >= ?will re‑include the boundary row if callers pass the last seen id as the next cursor. If you intend “start after cursor”, switch to>(and re‑generate).- Ratelimits type:
Ratelimits interface{}will typically come back as[]bytefrom MySQL JSON. If callers always treat this as opaque JSON, consider tightening it to[]byteorjson.RawMessagevia sqlc type overrides so you get compile‑time guarantees.Also applies to: 13-43, 52-62, 95-119
go/k8s/manifests/dashboard.yaml (1)
26-29: Agent dependency wait and env wiring look correct; ensure token is overridden outside devUpdating the init container to wait on both
planetscale:3900andagent:8080is consistent with the new agent dependency, and wiringAGENT_URL/AGENT_TOKENinto the dashboard container matches that integration.Treat
"agent-auth-secret"likeCTRL_API_KEYand ensure it’s overridden via Kubernetes secrets or per‑env values in non‑local deployments so real credentials aren’t baked into manifests.Also applies to: 55-59
go/apps/ctrl/config.go (1)
128-135: Consider adding basic validation for new Vault config fieldsThe new Vault and ACME Vault fields on
Configare wired into the struct but aren’t yet validated inValidate(). If/when Vault becomes a required dependency (for env vars or ACME certificates), it would be safer to assert at startup that:
VaultMasterKeys(andAcmeVaultMasterKeys, if enabled) are non‑empty.VaultS3/AcmeVaultS3have URL, bucket, and credentials populated.That keeps misconfiguration from surfacing only at deploy/cert runtime.
go/pkg/db/schema.sql (1)
319-332: Consider indexes for common query patterns.The
environment_variablestable structure is well-designed with appropriate constraints. However, if queries frequently filter byworkspace_idorenvironment_idalone (withoutkey), consider adding indexes for these columns in the source schema.Since this is a generated file, any index additions should be made in
internal/db/src/schemaand regenerated.apps/dashboard/lib/trpc/routers/deploy/env-vars/update.ts (2)
66-73: Consider adding workspaceId to the UPDATE WHERE clause.The update only filters by
id, but the initial query already validated workspace ownership. While functionally safe (the query at lines 28-38 ensures authorization), addingworkspaceIdto the WHERE clause provides defense-in-depth against TOCTOU issues if the env var is reassigned between the check and update.await db .update(schema.environmentVariables) .set({ key: input.key ?? envVar.key, value: encrypted, type: input.type, }) - .where(eq(schema.environmentVariables.id, input.envVarId)); + .where( + and( + eq(schema.environmentVariables.id, input.envVarId), + eq(schema.environmentVariables.workspaceId, ctx.workspace.id), + ), + );
26-84: Mutation lacks a return value.The mutation completes without returning anything. Consider returning
{ success: true }or the updated env var id to confirm the operation and enable client-side optimistic updates.apps/dashboard/lib/trpc/routers/deploy/env-vars/create.ts (2)
50-69: Duplicate key conflicts will surface as generic INTERNAL_SERVER_ERROR.The DB has a unique index on
(environmentId, key), but if a duplicate key is submitted, the insert will fail and be caught by the generic error handler at lines 70-79, returning "Failed to create environment variables" without indicating which key conflicted.Consider catching the duplicate key error specifically and returning a more descriptive message:
} catch (error) { if (error instanceof TRPCError) { throw error; } // Check for duplicate key constraint violation if (error instanceof Error && error.message.includes("Duplicate entry")) { throw new TRPCError({ code: "CONFLICT", message: "One or more environment variable keys already exist", }); } throw new TRPCError({ code: "INTERNAL_SERVER_ERROR", message: "Failed to create environment variables", }); }
31-80: Mutation lacks a return value.Similar to
updateEnvVar, this mutation doesn't return anything. Consider returning the created env var IDs to enable client-side cache updates.await db.insert(schema.environmentVariables).values(encryptedVars); + + return { ids: encryptedVars.map((v) => v.id) }; } catch (error) {internal/db/src/schema/environment_variables.ts (1)
41-44: Relation namedprojectbut referencesenvironmentstable.The relation name is misleading - it should be
environmentto match what it actually references.- project: one(environments, { + environment: one(environments, { fields: [environmentVariables.environmentId], references: [environments.id], }),go/k8s/manifests/agent.yaml (1)
18-54: Add securityContext to prevent privilege escalation.Static analysis flagged missing security hardening. Add a
securityContextto run as non-root and prevent privilege escalation.spec: + securityContext: + runAsNonRoot: true + runAsUser: 1000 containers: - name: agent image: unkey-agent:latest imagePullPolicy: Never + securityContext: + allowPrivilegeEscalation: false + readOnlyRootFilesystem: true + capabilities: + drop: + - ALL ports:apps/dashboard/lib/trpc/routers/deploy/env-vars/decrypt.ts (1)
27-27: Consider using.query()instead of.mutation()for read-only operation.This procedure only reads and decrypts data without modifying state. Semantically,
.query()would be more appropriate than.mutation(), and it enables caching on the client side.apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/details/env-variables-section/components/env-var-form.tsx (1)
22-32: UnusedprojectIdparameter.The
projectIdprop is accepted but aliased to_projectIdand never used. If it's not needed for the update mutation, consider removing it from the props interface to avoid confusion.type EnvVarFormProps = { envVarId: string; initialData: EnvVarFormData; - projectId: string; getExistingEnvVar: (key: string, excludeId?: string) => EnvVar | undefined; onSuccess: () => void; onCancel: () => void; excludeId?: string; autoFocus?: boolean; className?: string; }; export function EnvVarForm({ envVarId, initialData, - projectId: _projectId, getExistingEnvVar,apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/details/env-variables-section/components/env-var-inputs.tsx (1)
25-35: Consider callingkeyRegister.onChangeto maintain react-hook-form's internal tracking.When spreading
keyRegisterbut overridingonChange, the originalonChangefromregister()is not invoked. WhilesetValueupdates the form value, react-hook-form's internal event tracking (like touched state on change) may not work as expected.onChange={(e) => { if (keyDisabled) { return; } + // Call the original onChange to maintain form tracking + keyRegister.onChange(e); // Auto-uppercase the key and replace spaces with underscores - // nothing else should be valid in an env var... setValue("key", e.target.value.toUpperCase().replace(/ /g, "_"), { shouldValidate: true, }); }}apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/details/env-variables-section/add-env-vars.tsx (1)
206-209: Empty catch block silently swallows errors.While
toast.promisehandles displaying the error to the user, having an empty catch block is a code smell. Consider adding a comment explaining the intent or removing the try-catch if it's not needed.try { await mutation; onSuccess(); - } catch {} + } catch { + // Error already displayed via toast.promise + } };apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/details/env-variables-section/env-var-row.tsx (1)
119-122: Consider clearingdecryptedValueon cancel for consistency.When canceling an edit,
decryptedValueis not cleared (unlike on success). This means if the user reveals → edits → cancels → reveals again, the cached decrypted value may be stale if another user modified it.onCancel={() => { setIsEditing(false); setIsRevealed(false); + setDecryptedValue(undefined); }}
...app/(app)/[workspaceSlug]/projects/[projectId]/details/env-variables-section/env-var-row.tsx
Show resolved
Hide resolved
mcstepp
force-pushed
the
ENG-2268-identities-list-view
branch
from
22cc6e1 to
0e8124f
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
apps/dashboard/lib/trpc/routers/identity/query.ts (1)
5-11: Search, relations, and totalCount logic looks good; consider optimizing count queryOverall this refactor hangs together well:
- Optional
searchin the payload, plus the sharedbuildFilterConditions(workspace + not-deleted + optional LIKE onexternalId/idwithescapeLike), gives predictable search behavior and correctly reuses conditions for both the list and the count. Based on learnings, centralizing LIKE-escaping like this is the right move for avoiding wildcard surprises while keeping Drizzle’s parameterization intact.- Loading
keysandratelimitsrelations with onlyidselected, and threading them throughIdentityResponseSchema, matches the new UI needs without overfetching.- Cursor-based pagination (
lt(identity.id, cursor)andlimit + 1withhasMore/nextCursor) is implemented in the standard pattern and looks correct.One thing to improve (previously raised) is the
totalCountimplementation:
totalCountis computed viadb.query.identities.findMany(...).length, which loads all matching rows’ IDs into memory just to count them. For large workspaces this becomes unnecessarily expensive.Consider switching to a dedicated COUNT query (or Drizzle’s count helper, if available for your version) that returns only the aggregate:
- const countQuery = await db.query.identities.findMany({ - where: buildFilterConditions, - columns: { - id: true, - }, - }); - - const totalCount = countQuery.length; + const { identity } = await import("@unkey/db/src/schema"); + const countResult = await db + .select({ count: sql<number>`count(*)` }) + .from(identity) + .where((table, helpers) => buildFilterConditions(table, helpers)); + + const totalCount = countResult[0]?.count ?? 0;You can adapt this to whatever count utility your Drizzle version supports.
Drizzle ORM "relational query" or "db.query.*" API: what is the recommended way to perform an efficient COUNT(*) with the same where conditions, instead of fetching all rows and using `.length`?Also applies to: 21-23, 39-40, 61-69, 71-83, 84-95, 106-116, 120-125
🧹 Nitpick comments (3)
apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/table/identity-table-actions.tsx (1)
17-27: Optional: DRY up clipboard copy logicThe two menu items duplicate the same
navigator.clipboard.writeText+ toast + error-handling flow. Consider extracting a small helper to reduce repetition and keep messages consistent:+const copyWithToast = async (value: string, success: string) => { + try { + await navigator.clipboard.writeText(value); + toast.success(success); + } catch (error) { + console.error("Failed to copy to clipboard:", error); + toast.error("Failed to copy to clipboard"); + } +}; + export const IdentityTableActions = ({ identity }: { identity: Identity }) => { const menuItems: MenuItem[] = useMemo( () => [ { id: "copy-identity-id", label: "Copy identity ID", icon: <Clone iconSize="md-medium" />, - onClick: () => { - navigator.clipboard - .writeText(identity.id) - .then(() => { - toast.success("Identity ID copied to clipboard"); - }) - .catch((error) => { - console.error("Failed to copy to clipboard:", error); - toast.error("Failed to copy to clipboard"); - }); - }, + onClick: () => void copyWithToast(identity.id, "Identity ID copied to clipboard"), }, { id: "copy-external-id", label: "Copy external ID", icon: <Clone iconSize="md-medium" />, - onClick: () => { - navigator.clipboard - .writeText(identity.externalId) - .then(() => { - toast.success("External ID copied to clipboard"); - }) - .catch((error) => { - console.error("Failed to copy to clipboard:", error); - toast.error("Failed to copy to clipboard"); - }); - }, + onClick: () => void copyWithToast(identity.externalId, "External ID copied to clipboard"), }, ], [identity.id, identity.externalId], );Also applies to: 33-43
apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/controls/identities-search.tsx (1)
1-31: LLMSearch integration withuseQueryStatelooks solidHooking
LLMSearchinto the"search"query param viauseQueryStatematches the table’s usage and should keep list + URL in sync. If you want to avoid queries that differ only by surrounding whitespace, you could optionallysetSearch(query.trim())inonSearch, but it’s not required for correctness.apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/identities-client.tsx (1)
1-13: Simple composition of controls + list looks goodThe
IdentitiesClientwrapper cleanly composes controls and table into a vertical layout; there’s no functional risk here. IfIdentitiesListControls/IdentitiesListare already client components and this is only used under the clientPage, you could consider dropping the local"use client"directive to avoid an extra client boundary, but that’s purely optional.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/controls/identities-search.tsx(1 hunks)apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/controls/index.tsx(1 hunks)apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/create-identity-dialog.tsx(1 hunks)apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/identities-client.tsx(1 hunks)apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/table/identities-list.tsx(1 hunks)apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/table/identity-table-actions.tsx(1 hunks)apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/table/last-used.tsx(1 hunks)apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/table/skeletons.tsx(1 hunks)apps/dashboard/app/(app)/[workspaceSlug]/identities/components/results.tsx(0 hunks)apps/dashboard/app/(app)/[workspaceSlug]/identities/filter.tsx(0 hunks)apps/dashboard/app/(app)/[workspaceSlug]/identities/navigation.tsx(2 hunks)apps/dashboard/app/(app)/[workspaceSlug]/identities/page.tsx(2 hunks)apps/dashboard/app/(app)/[workspaceSlug]/identities/row.tsx(0 hunks)apps/dashboard/lib/trpc/routers/identity/latestVerification.ts(1 hunks)apps/dashboard/lib/trpc/routers/identity/query.ts(5 hunks)apps/dashboard/lib/trpc/routers/identity/search.ts(2 hunks)apps/dashboard/lib/trpc/routers/index.ts(2 hunks)apps/dashboard/lib/trpc/routers/utils/sql.ts(1 hunks)
💤 Files with no reviewable changes (3)
- apps/dashboard/app/(app)/[workspaceSlug]/identities/row.tsx
- apps/dashboard/app/(app)/[workspaceSlug]/identities/filter.tsx
- apps/dashboard/app/(app)/[workspaceSlug]/identities/components/results.tsx
🚧 Files skipped from review as they are similar to previous changes (7)
- apps/dashboard/lib/trpc/routers/identity/search.ts
- apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/table/last-used.tsx
- apps/dashboard/lib/trpc/routers/identity/latestVerification.ts
- apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/controls/index.tsx
- apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/table/identities-list.tsx
- apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/table/skeletons.tsx
- apps/dashboard/app/(app)/[workspaceSlug]/identities/navigation.tsx
🧰 Additional context used
🧠 Learnings (12)
📓 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.
📚 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/_components/identities-client.tsx
📚 Learning: 2025-08-18T10:28:47.391Z
Learnt from: ogzhanolguncu
Repo: unkeyed/unkey PR: 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)/[workspaceSlug]/identities/_components/identities-client.tsxapps/dashboard/app/(app)/[workspaceSlug]/identities/page.tsx
📚 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/create-identity-dialog.tsx
📚 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.ts
📚 Learning: 2025-02-27T14:35:15.251Z
Learnt from: ogzhanolguncu
Repo: unkeyed/unkey PR: 2918
File: apps/dashboard/lib/trpc/routers/api/overview-api-search.ts:0-0
Timestamp: 2025-02-27T14:35:15.251Z
Learning: When using Drizzle ORM with LIKE queries, use the sql`` tagged template syntax (e.g., sql`${table.name} LIKE ${`%${input.query}%`}`) instead of direct string interpolation to prevent SQL injection attacks.
Applied to files:
apps/dashboard/lib/trpc/routers/identity/query.tsapps/dashboard/lib/trpc/routers/utils/sql.ts
📚 Learning: 2025-06-25T19:51:15.995Z
Learnt from: ogzhanolguncu
Repo: unkeyed/unkey PR: 3369
File: apps/dashboard/lib/trpc/routers/settings/root-keys/query.ts:67-67
Timestamp: 2025-06-25T19:51:15.995Z
Learning: Drizzle ORM automatically handles SQL parameterization and escaping for queries, including LIKE queries with direct string interpolation (e.g., `like(schema.keys.name, \`%${filter.value}%\`)`), making them safe from SQL injection attacks without requiring explicit escaping or sql`` tagged template syntax.
Applied to files:
apps/dashboard/lib/trpc/routers/identity/query.tsapps/dashboard/lib/trpc/routers/utils/sql.ts
📚 Learning: 2025-02-10T14:12:17.261Z
Learnt from: ogzhanolguncu
Repo: unkeyed/unkey PR: 2883
File: apps/dashboard/app/(app)/logs/components/controls/components/logs-search/index.tsx:29-31
Timestamp: 2025-02-10T14:12:17.261Z
Learning: In the logs search component's error handling, error messages are deliberately wrapped in single quotes within template literals for visual distinction (e.g. "Unable to process your search request 'some error message'").
Applied to files:
apps/dashboard/lib/trpc/routers/identity/query.ts
📚 Learning: 2025-04-08T09:34:24.576Z
Learnt from: ogzhanolguncu
Repo: unkeyed/unkey PR: 2872
File: apps/dashboard/lib/trpc/routers/ratelimit/createNamespace.ts:36-39
Timestamp: 2025-04-08T09:34:24.576Z
Learning: In the Unkey dashboard, when making database queries involving workspaces, use `ctx.workspace.id` directly instead of fetching the workspace separately for better performance and security.
Applied to files:
apps/dashboard/lib/trpc/routers/identity/query.ts
📚 Learning: 2025-12-05T12:05:33.143Z
Learnt from: Flo4604
Repo: unkeyed/unkey PR: 4484
File: go/pkg/db/queries/identity_list.sql:11-24
Timestamp: 2025-12-05T12:05:33.143Z
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.ts
📚 Learning: 2025-07-28T19:42:37.047Z
Learnt from: mcstepp
Repo: unkeyed/unkey PR: 3662
File: apps/dashboard/app/(app)/projects/page.tsx:74-81
Timestamp: 2025-07-28T19:42:37.047Z
Learning: In apps/dashboard/app/(app)/projects/page.tsx, the user mcstepp prefers to keep placeholder functions like generateSlug inline during POC/demonstration phases rather than extracting them to utility modules, with plans to refactor later when the feature matures beyond the proof-of-concept stage.
Applied to files:
apps/dashboard/app/(app)/[workspaceSlug]/identities/page.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/page.tsx
🧬 Code graph analysis (7)
apps/dashboard/lib/trpc/routers/index.ts (1)
apps/dashboard/lib/trpc/routers/identity/latestVerification.ts (1)
identityLastVerificationTime(10-54)
apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/identities-client.tsx (2)
apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/controls/index.tsx (1)
IdentitiesListControls(4-12)apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/table/identities-list.tsx (1)
IdentitiesList(47-326)
apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/create-identity-dialog.tsx (5)
apps/dashboard/app/(app)/[workspaceSlug]/apis/[apiId]/_components/create-key/create-key.schema.ts (1)
FormValues(351-351)internal/ui/src/components/toaster.tsx (1)
toast(29-29)apps/dashboard/lib/collections/index.ts (1)
reset(76-84)internal/ui/src/components/dialog/dialog-container.tsx (1)
DialogContainer(66-66)internal/ui/src/components/buttons/button.tsx (1)
Button(439-439)
apps/dashboard/lib/trpc/routers/identity/query.ts (3)
apps/dashboard/lib/trpc/routers/utils/sql.ts (1)
escapeLike(15-17)internal/rbac/src/queries.ts (2)
or(46-50)and(52-56)apps/dashboard/lib/db.ts (1)
db(5-26)
apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/controls/identities-search.tsx (1)
internal/ui/src/components/llm-search/index.tsx (1)
LLMSearch(176-176)
apps/dashboard/app/(app)/[workspaceSlug]/identities/page.tsx (1)
apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/identities-client.tsx (1)
IdentitiesClient(6-13)
apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/table/identity-table-actions.tsx (2)
apps/dashboard/lib/trpc/routers/identity/query.ts (1)
IdentityResponseSchema(13-23)apps/dashboard/components/logs/table-action.popover.tsx (2)
MenuItem(19-29)TableActionPopover(36-169)
🪛 GitHub Actions: autofix.ci
apps/dashboard/lib/trpc/routers/identity/query.ts
[error] 44-44: lint/correctness/noUnusedVariables: This variable is unused.
⏰ 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 (6)
apps/dashboard/lib/trpc/routers/utils/sql.ts (1)
1-17: escapeLike implementation and docs look correctThe escape order and example output align, and this is a good centralization of LIKE-escape logic for search. No changes needed.
apps/dashboard/lib/trpc/routers/index.ts (1)
60-60: Wiring identity.latestVerification matches existing patternsImporting
identityLastVerificationTimeand exposing it asidentity.latestVerificationis consistent with howapi.keys.latestVerificationis wired and keeps the router surface coherent.Also applies to: 340-347
apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/table/identity-table-actions.tsx (1)
10-49: Overall component structure and popover integration look goodAside from the import/type fix above, the
IdentityTableActionscomponent cleanly wiresMenuItems intoTableActionPopover, usesuseMemowith appropriate deps (identity.id,identity.externalId), and provides clear success/error toasts for both identity ID and external ID copy actions.apps/dashboard/app/(app)/[workspaceSlug]/identities/page.tsx (1)
1-20: Identities page wiring and beta gate look correctThe page-level client boundary, identities beta feature check, and composition of
Navigation+IdentitiesClientalign with the described flow and should behave as expected with dynamic data.apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/create-identity-dialog.tsx (2)
63-92: CreateIdentity mutation flow and JSON meta handling align with requirementsThe mutation wiring matches the spec: success toast, identities list invalidation via
utils.identity.query.invalidate(), dialog close, and form reset, plus 409 → field error for duplicate external IDs. Parsingmetato JSON (or sendingnullon empty/whitespace) is consistent with the zodmetarefinements that already enforce valid JSON and<1MB. Overall this flow looks good.
94-155: Dialog UX and form integration look consistent with dashboard patternsThe
NavbarActionButtonentry point,DialogContainerwith a single primary submit button wired viaform="create-identity-form", field-level error display forexternalIdandmeta, and helper copy about associating identities with keys/ratelimits all read well and fulfill the described “Create Identity” flow. No functional issues stand out here.
apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/create-identity-dialog.tsx
Show resolved
Hide resolved
.../dashboard/app/(app)/[workspaceSlug]/identities/_components/table/identity-table-actions.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/dashboard/lib/trpc/routers/identity/query.ts (1)
1-1: Drop unusedlthelper frombuildFilterConditions(and import) to fix lintIn
buildFilterConditions,ltis destructured but never used, which matches thenoUnusedVariableslint failure:- const buildFilterConditions = (identity: any, { and, eq, or, like, lt }: any) => { + const buildFilterConditions = (identity: any, { and, eq, or, like }: any) => {Since you already get
ltfromhelpersinside thewherecallback:const { and, lt } = helpers;the top‑level
ltimport is redundant and can be removed as well:-import { and, count, db, eq, like, lt, or, schema } from "@/lib/db"; +import { and, count, db, eq, like, or, schema } from "@/lib/db";This keeps the helper generic, removes the unused variable, and avoids future lint errors on the unused import.
After applying this, run the linter again to ensure all
noUnusedVariableswarnings aroundltare cleared.Also applies to: 66-83, 86-97
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/dashboard/lib/trpc/routers/identity/latestVerification.ts(1 hunks)apps/dashboard/lib/trpc/routers/identity/query.ts(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/dashboard/lib/trpc/routers/identity/latestVerification.ts
🧰 Additional context used
🧠 Learnings (10)
📓 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.
📚 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.ts
📚 Learning: 2025-02-27T14:35:15.251Z
Learnt from: ogzhanolguncu
Repo: unkeyed/unkey PR: 2918
File: apps/dashboard/lib/trpc/routers/api/overview-api-search.ts:0-0
Timestamp: 2025-02-27T14:35:15.251Z
Learning: When using Drizzle ORM with LIKE queries, use the sql`` tagged template syntax (e.g., sql`${table.name} LIKE ${`%${input.query}%`}`) instead of direct string interpolation to prevent SQL injection attacks.
Applied to files:
apps/dashboard/lib/trpc/routers/identity/query.ts
📚 Learning: 2025-06-25T19:51:15.995Z
Learnt from: ogzhanolguncu
Repo: unkeyed/unkey PR: 3369
File: apps/dashboard/lib/trpc/routers/settings/root-keys/query.ts:67-67
Timestamp: 2025-06-25T19:51:15.995Z
Learning: Drizzle ORM automatically handles SQL parameterization and escaping for queries, including LIKE queries with direct string interpolation (e.g., `like(schema.keys.name, \`%${filter.value}%\`)`), making them safe from SQL injection attacks without requiring explicit escaping or sql`` tagged template syntax.
Applied to files:
apps/dashboard/lib/trpc/routers/identity/query.ts
📚 Learning: 2025-02-10T14:12:17.261Z
Learnt from: ogzhanolguncu
Repo: unkeyed/unkey PR: 2883
File: apps/dashboard/app/(app)/logs/components/controls/components/logs-search/index.tsx:29-31
Timestamp: 2025-02-10T14:12:17.261Z
Learning: In the logs search component's error handling, error messages are deliberately wrapped in single quotes within template literals for visual distinction (e.g. "Unable to process your search request 'some error message'").
Applied to files:
apps/dashboard/lib/trpc/routers/identity/query.ts
📚 Learning: 2025-04-08T09:34:24.576Z
Learnt from: ogzhanolguncu
Repo: unkeyed/unkey PR: 2872
File: apps/dashboard/lib/trpc/routers/ratelimit/createNamespace.ts:36-39
Timestamp: 2025-04-08T09:34:24.576Z
Learning: In the Unkey dashboard, when making database queries involving workspaces, use `ctx.workspace.id` directly instead of fetching the workspace separately for better performance and security.
Applied to files:
apps/dashboard/lib/trpc/routers/identity/query.ts
📚 Learning: 2025-12-05T12:05:33.143Z
Learnt from: Flo4604
Repo: unkeyed/unkey PR: 4484
File: go/pkg/db/queries/identity_list.sql:11-24
Timestamp: 2025-12-05T12:05:33.143Z
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.ts
📚 Learning: 2025-06-24T13:29:10.129Z
Learnt from: ogzhanolguncu
Repo: unkeyed/unkey PR: 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/lib/trpc/routers/identity/query.ts
📚 Learning: 2025-05-15T15:59:20.955Z
Learnt from: mcstepp
Repo: unkeyed/unkey PR: 3242
File: apps/dashboard/app/(app)/apis/[apiId]/api-id-navbar.tsx:230-266
Timestamp: 2025-05-15T15:59:20.955Z
Learning: Avoid using `any` type in TypeScript code as it defeats the purpose of type safety and will cause linter issues in the future. Instead, create proper interfaces or utilize existing type definitions, especially for complex nested objects.
Applied to files:
apps/dashboard/lib/trpc/routers/identity/query.ts
📚 Learning: 2024-10-23T16:21:47.395Z
Learnt from: p6l-richard
Repo: unkeyed/unkey PR: 2085
File: apps/www/components/glossary/search.tsx:16-20
Timestamp: 2024-10-23T16:21:47.395Z
Learning: For the `FilterableCommand` component in `apps/www/components/glossary/search.tsx`, refactoring type definitions into an interface is not necessary at this time.
Applied to files:
apps/dashboard/lib/trpc/routers/identity/query.ts
🧬 Code graph analysis (1)
apps/dashboard/lib/trpc/routers/identity/query.ts (4)
internal/db/src/index.ts (1)
schema(8-8)apps/dashboard/lib/trpc/routers/utils/sql.ts (1)
escapeLike(15-17)internal/rbac/src/queries.ts (2)
or(46-50)and(52-56)apps/dashboard/lib/db.ts (1)
db(5-26)
🪛 GitHub Actions: autofix.ci
apps/dashboard/lib/trpc/routers/identity/query.ts
[error] 51-54: lint/style/noNonNullAssertion: Forbidden non-null assertion.
[error] 68-68: lint/correctness/noUnusedVariables: This variable is unused.
⏰ 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). (1)
- GitHub Check: Test Dashboard / Test Dashboard
🔇 Additional comments (1)
apps/dashboard/lib/trpc/routers/identity/query.ts (1)
13-23: Identity schema, relations, and totalCount wiring look consistentThe extensions to
IdentityResponseSchema(keys,ratelimits) andIdentitiesResponse(totalCount) line up with thefindManywithrelations and thetransformedIdentities/return shape. No obvious type or shape mismatches here.Also applies to: 25-30, 98-108, 120-130, 134-139
mcstepp
force-pushed
the
ENG-2268-identities-list-view
branch
from
6f7b92e to
b570061
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (2)
apps/dashboard/lib/trpc/routers/identity/query.ts (2)
66-83: Remove unusedltparameter to fix pipeline failure.The
lthelper is destructured on Line 68 but never used withinbuildFilterConditions, triggering thenoUnusedVariableslint error. Theltoperator is only needed in the main query (line 87, 93) for cursor pagination, not in this helper function.Apply this diff:
// Helper function to build filter conditions for query API // biome-ignore lint/suspicious/noExplicitAny: Leave it as is for now - const buildFilterConditions = (identity: any, { and, eq, or, like, lt }: any) => { + const buildFilterConditions = (identity: any, { and, eq, or, like }: any) => { const conditions = [eq(identity.workspaceId, workspaceId), eq(identity.deleted, false)];
48-56: Remove non-null assertion to fix pipeline failure.The
or(...)!on Line 54 triggers thenoNonNullAssertionlint error. The file already demonstrates the correct pattern in lines 73-79 where the result is checked before pushing.Apply this diff to use the safe pattern consistently:
if (search) { const escapedSearch = escapeLike(search); - baseConditions.push( - or( - like(schema.identities.externalId, `%${escapedSearch}%`), - like(schema.identities.id, `%${escapedSearch}%`), - )!, - ); + const searchCondition = or( + like(schema.identities.externalId, `%${escapedSearch}%`), + like(schema.identities.id, `%${escapedSearch}%`), + ); + if (searchCondition) { + baseConditions.push(searchCondition); + } }
🧹 Nitpick comments (3)
apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/controls/identities-search.tsx (2)
18-24: Example queries may mislead users.Some example queries suggest capabilities that the backend doesn't support:
"Find identity with ID 'user_123'"- backend only searchesexternalId, not identityid"Show identities created in the last week"- backend doesn't support date filteringConsider aligning examples with actual functionality:
exampleQueries={[ - "Find identity with ID 'user_123'", + "user_123", "Show identities with external ID containing 'test'", - "Find identities with external ID 'john@example.com'", - "Show identities created in the last week", + "john@example.com", + "test-identity", ]}
24-24: Consider wiringisLoadingto actual search state.Hardcoding
isLoading={false}provides no feedback during search operations. If the parent component or a query hook tracks loading state, passing it here would improve UX.apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/table/skeletons.tsx (1)
28-34: Simplify nested animation classes.The container on Line 29 applies
animate-pulse, making the individualanimate-pulseclasses on Lines 30-32 redundant. The pulsing animation will cascade to children.Apply this diff to remove redundant classes:
export const LastUsedColumnSkeleton = () => ( <div className="px-1.5 rounded-md flex gap-2 items-center w-[140px] h-[22px] bg-grayA-3 animate-pulse"> - <div className="h-2 w-2 bg-grayA-3 rounded-full animate-pulse" /> - <div className="h-2 w-12 bg-grayA-3 rounded animate-pulse" /> - <div className="h-2 w-12 bg-grayA-3 rounded animate-pulse" /> + <div className="h-2 w-2 bg-gray-6 rounded-full" /> + <div className="h-2 w-12 bg-gray-6 rounded" /> + <div className="h-2 w-12 bg-gray-6 rounded" /> </div> );Note: Changed to
bg-gray-6for visual contrast against thebg-grayA-3container, creating a more realistic skeleton appearance.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/controls/identities-search.tsx(1 hunks)apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/controls/index.tsx(1 hunks)apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/create-identity-dialog.tsx(1 hunks)apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/identities-client.tsx(1 hunks)apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/table/identities-list.tsx(1 hunks)apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/table/identity-table-actions.tsx(1 hunks)apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/table/last-used.tsx(1 hunks)apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/table/skeletons.tsx(1 hunks)apps/dashboard/app/(app)/[workspaceSlug]/identities/components/results.tsx(0 hunks)apps/dashboard/app/(app)/[workspaceSlug]/identities/filter.tsx(0 hunks)apps/dashboard/app/(app)/[workspaceSlug]/identities/navigation.tsx(2 hunks)apps/dashboard/app/(app)/[workspaceSlug]/identities/page.tsx(2 hunks)apps/dashboard/app/(app)/[workspaceSlug]/identities/row.tsx(0 hunks)apps/dashboard/lib/trpc/routers/identity/latestVerification.ts(1 hunks)apps/dashboard/lib/trpc/routers/identity/query.ts(5 hunks)apps/dashboard/lib/trpc/routers/identity/search.ts(2 hunks)apps/dashboard/lib/trpc/routers/index.ts(2 hunks)apps/dashboard/lib/trpc/routers/utils/sql.ts(1 hunks)
💤 Files with no reviewable changes (3)
- apps/dashboard/app/(app)/[workspaceSlug]/identities/row.tsx
- apps/dashboard/app/(app)/[workspaceSlug]/identities/filter.tsx
- apps/dashboard/app/(app)/[workspaceSlug]/identities/components/results.tsx
🚧 Files skipped from review as they are similar to previous changes (6)
- apps/dashboard/lib/trpc/routers/index.ts
- apps/dashboard/lib/trpc/routers/identity/latestVerification.ts
- apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/table/last-used.tsx
- apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/create-identity-dialog.tsx
- apps/dashboard/app/(app)/[workspaceSlug]/identities/navigation.tsx
- apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/table/identity-table-actions.tsx
🧰 Additional context used
🧠 Learnings (22)
📓 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.
📚 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/_components/identities-client.tsx
📚 Learning: 2025-08-18T10:28:47.391Z
Learnt from: ogzhanolguncu
Repo: unkeyed/unkey PR: 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)/[workspaceSlug]/identities/_components/identities-client.tsxapps/dashboard/app/(app)/[workspaceSlug]/identities/page.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/trpc/routers/identity/search.ts
📚 Learning: 2025-09-25T18:49:11.451Z
Learnt from: mcstepp
Repo: unkeyed/unkey PR: 4010
File: apps/dashboard/lib/trpc/routers/deploy/deployment/rollback.ts:39-44
Timestamp: 2025-09-25T18:49:11.451Z
Learning: In apps/dashboard/lib/trpc/routers/deploy/deployment/rollback.ts and similar files, mcstepp prefers to keep the demo API key authentication simple without additional validation complexity, since it's temporary code that will be replaced after the demo phase.
Applied to files:
apps/dashboard/lib/trpc/routers/identity/search.ts
📚 Learning: 2024-11-29T15:15:47.308Z
Learnt from: chronark
Repo: unkeyed/unkey PR: 2693
File: apps/api/src/routes/v1_keys_updateKey.ts:350-368
Timestamp: 2024-11-29T15:15:47.308Z
Learning: In `apps/api/src/routes/v1_keys_updateKey.ts`, the code intentionally handles `externalId` and `ownerId` separately for clarity. The `ownerId` field will be removed in the future, simplifying the code.
Applied to files:
apps/dashboard/lib/trpc/routers/identity/search.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-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/lib/trpc/routers/identity/search.ts
📚 Learning: 2025-12-05T12:05:33.143Z
Learnt from: Flo4604
Repo: unkeyed/unkey PR: 4484
File: go/pkg/db/queries/identity_list.sql:11-24
Timestamp: 2025-12-05T12:05:33.143Z
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/search.tsapps/dashboard/lib/trpc/routers/identity/query.ts
📚 Learning: 2025-07-28T19:42:37.047Z
Learnt from: mcstepp
Repo: unkeyed/unkey PR: 3662
File: apps/dashboard/app/(app)/projects/page.tsx:74-81
Timestamp: 2025-07-28T19:42:37.047Z
Learning: In apps/dashboard/app/(app)/projects/page.tsx, the user mcstepp prefers to keep placeholder functions like generateSlug inline during POC/demonstration phases rather than extracting them to utility modules, with plans to refactor later when the feature matures beyond the proof-of-concept stage.
Applied to files:
apps/dashboard/app/(app)/[workspaceSlug]/identities/page.tsxapps/dashboard/app/(app)/[workspaceSlug]/identities/_components/table/skeletons.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/page.tsx
📚 Learning: 2024-12-03T14:17:08.016Z
Learnt from: ogzhanolguncu
Repo: unkeyed/unkey PR: 2143
File: apps/dashboard/app/(app)/logs/logs-page.tsx:77-83
Timestamp: 2024-12-03T14:17:08.016Z
Learning: The `<LogsTable />` component already implements virtualization to handle large datasets efficiently.
Applied to files:
apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/table/identities-list.tsx
📚 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.ts
📚 Learning: 2025-02-27T14:35:15.251Z
Learnt from: ogzhanolguncu
Repo: unkeyed/unkey PR: 2918
File: apps/dashboard/lib/trpc/routers/api/overview-api-search.ts:0-0
Timestamp: 2025-02-27T14:35:15.251Z
Learning: When using Drizzle ORM with LIKE queries, use the sql`` tagged template syntax (e.g., sql`${table.name} LIKE ${`%${input.query}%`}`) instead of direct string interpolation to prevent SQL injection attacks.
Applied to files:
apps/dashboard/lib/trpc/routers/identity/query.tsapps/dashboard/lib/trpc/routers/utils/sql.ts
📚 Learning: 2025-06-25T19:51:15.995Z
Learnt from: ogzhanolguncu
Repo: unkeyed/unkey PR: 3369
File: apps/dashboard/lib/trpc/routers/settings/root-keys/query.ts:67-67
Timestamp: 2025-06-25T19:51:15.995Z
Learning: Drizzle ORM automatically handles SQL parameterization and escaping for queries, including LIKE queries with direct string interpolation (e.g., `like(schema.keys.name, \`%${filter.value}%\`)`), making them safe from SQL injection attacks without requiring explicit escaping or sql`` tagged template syntax.
Applied to files:
apps/dashboard/lib/trpc/routers/identity/query.tsapps/dashboard/lib/trpc/routers/utils/sql.ts
📚 Learning: 2025-02-10T14:12:17.261Z
Learnt from: ogzhanolguncu
Repo: unkeyed/unkey PR: 2883
File: apps/dashboard/app/(app)/logs/components/controls/components/logs-search/index.tsx:29-31
Timestamp: 2025-02-10T14:12:17.261Z
Learning: In the logs search component's error handling, error messages are deliberately wrapped in single quotes within template literals for visual distinction (e.g. "Unable to process your search request 'some error message'").
Applied to files:
apps/dashboard/lib/trpc/routers/identity/query.ts
📚 Learning: 2025-04-08T09:34:24.576Z
Learnt from: ogzhanolguncu
Repo: unkeyed/unkey PR: 2872
File: apps/dashboard/lib/trpc/routers/ratelimit/createNamespace.ts:36-39
Timestamp: 2025-04-08T09:34:24.576Z
Learning: In the Unkey dashboard, when making database queries involving workspaces, use `ctx.workspace.id` directly instead of fetching the workspace separately for better performance and security.
Applied to files:
apps/dashboard/lib/trpc/routers/identity/query.ts
📚 Learning: 2025-06-24T13:29:10.129Z
Learnt from: ogzhanolguncu
Repo: unkeyed/unkey PR: 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/lib/trpc/routers/identity/query.ts
📚 Learning: 2025-05-15T15:59:20.955Z
Learnt from: mcstepp
Repo: unkeyed/unkey PR: 3242
File: apps/dashboard/app/(app)/apis/[apiId]/api-id-navbar.tsx:230-266
Timestamp: 2025-05-15T15:59:20.955Z
Learning: Avoid using `any` type in TypeScript code as it defeats the purpose of type safety and will cause linter issues in the future. Instead, create proper interfaces or utilize existing type definitions, especially for complex nested objects.
Applied to files:
apps/dashboard/lib/trpc/routers/identity/query.ts
📚 Learning: 2024-10-23T16:21:47.395Z
Learnt from: p6l-richard
Repo: unkeyed/unkey PR: 2085
File: apps/www/components/glossary/search.tsx:16-20
Timestamp: 2024-10-23T16:21:47.395Z
Learning: For the `FilterableCommand` component in `apps/www/components/glossary/search.tsx`, refactoring type definitions into an interface is not necessary at this time.
Applied to files:
apps/dashboard/lib/trpc/routers/identity/query.ts
📚 Learning: 2025-05-15T15:57:02.128Z
Learnt from: mcstepp
Repo: unkeyed/unkey PR: 3242
File: apps/dashboard/app/(app)/apis/[apiId]/api-id-navbar.tsx:47-50
Timestamp: 2025-05-15T15:57:02.128Z
Learning: When reviewing code for Unkey, prefer using `Boolean()` over the double negation (`!!`) operator for boolean coercion, as their linter rules favor this pattern.
Applied to files:
apps/dashboard/lib/trpc/routers/identity/query.ts
📚 Learning: 2024-10-23T16:19:42.049Z
Learnt from: p6l-richard
Repo: unkeyed/unkey PR: 2085
File: apps/www/components/glossary/search.tsx:41-57
Timestamp: 2024-10-23T16:19:42.049Z
Learning: For the `FilterableCommand` component in `apps/www/components/glossary/search.tsx`, adding error handling and loading states to the results list is not necessary.
Applied to files:
apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/controls/identities-search.tsx
🧬 Code graph analysis (4)
apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/identities-client.tsx (2)
apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/controls/index.tsx (1)
IdentitiesListControls(4-12)apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/table/identities-list.tsx (1)
IdentitiesList(47-326)
apps/dashboard/app/(app)/[workspaceSlug]/identities/page.tsx (1)
apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/identities-client.tsx (1)
IdentitiesClient(6-13)
apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/controls/index.tsx (2)
apps/dashboard/components/logs/controls-container.tsx (1)
ControlsContainer(1-7)apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/controls/identities-search.tsx (1)
IdentitiesSearch(6-31)
apps/dashboard/lib/trpc/routers/identity/query.ts (4)
internal/db/src/index.ts (1)
schema(8-8)apps/dashboard/lib/trpc/routers/utils/sql.ts (1)
escapeLike(15-17)internal/rbac/src/queries.ts (2)
or(46-50)and(52-56)apps/dashboard/lib/db.ts (1)
db(5-26)
🪛 GitHub Actions: autofix.ci
apps/dashboard/lib/trpc/routers/identity/query.ts
[error] 51-54: lint/style/noNonNullAssertion: Forbidden non-null assertion.
[error] 66-68: lint/correctness/noUnusedVariables: This variable is unused.
⏰ 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 (7)
apps/dashboard/lib/trpc/routers/identity/search.ts (1)
39-50: Good addition of relation preloading.Eagerly loading
keysandratelimitswith minimal column projection (idonly) is efficient for count display while avoiding N+1 queries.apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/controls/index.tsx (1)
1-12: LGTM!Clean composition following the established patterns from the logs controls. The structure leaves room for adding right-side controls (e.g., Create Identity button) if needed.
apps/dashboard/app/(app)/[workspaceSlug]/identities/page.tsx (1)
8-20: Simplified page composition looks good.The refactor to delegate to
IdentitiesClientwhile preserving the beta feature gate is clean. The client component structure works well with theuseWorkspaceNavigationhook requirement.apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/identities-client.tsx (1)
1-13: LGTM! Clean composition pattern.The component serves as an appropriate client boundary for the identities feature, composing controls and list components in a straightforward layout. The "use client" directive is correctly placed at the file level.
apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/table/identities-list.tsx (3)
29-45: LGTM! Effective code splitting for actions popover.The dynamic import with
ssr: falseand loading placeholder is well-implemented, optimizing initial bundle size while providing a seamless loading experience.
76-81: Verify aggressive cache strategy is intentional.The query is configured with
staleTime: Number.POSITIVE_INFINITYand disabled refetching, meaning users won't see new identities created by others (or themselves in other tabs) until they manually refresh the page or trigger a new search.If this is intentional for performance, consider adding a manual refresh action or reducing the stale time. Otherwise, consider enabling refetch on window focus for better UX:
{ getNextPageParam: (lastPage) => lastPage.nextCursor, - staleTime: Number.POSITIVE_INFINITY, - refetchOnMount: false, - refetchOnWindowFocus: false, + staleTime: 5 * 60 * 1000, // 5 minutes + refetchOnMount: false, + refetchOnWindowFocus: true, },
236-325: LGTM! Well-structured virtual table implementation.The VirtualTable configuration correctly handles loading states, pagination, empty states, and skeleton rendering. The countInfoText now properly displays loaded count vs. total count (addressing previous feedback), and the empty state messaging adapts appropriately to search context.
apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/table/identities-list.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/table/identities-list.tsx (1)
157-166: Add click guard to prevent multiple navigation attempts.The link remains clickable even when
isNavigatingis true, which can cause multiplehandleLinkClickcalls and duplicate state updates.Add an early return in the onClick handler:
<Link className="font-mono group-hover:underline decoration-dotted text-accent-9" href={`/${workspace.slug}/identities/${identity.id}`} aria-disabled={isNavigating} - onClick={() => { + onClick={(e) => { + if (isNavigating) { + e.preventDefault(); + return; + } handleLinkClick(identity.id); }} >Alternatively, add
pointer-events-noneto the className when navigating:<Link - className="font-mono group-hover:underline decoration-dotted text-accent-9" + className={cn( + "font-mono group-hover:underline decoration-dotted text-accent-9", + isNavigating && "pointer-events-none opacity-50" + )} href={`/${workspace.slug}/identities/${identity.id}`} aria-disabled={isNavigating} onClick={() => { handleLinkClick(identity.id); }} >
🧹 Nitpick comments (1)
apps/dashboard/lib/trpc/routers/identity/query.ts (1)
42-84: Extract shared filter logic to reduce duplication.The filter conditions (workspace, deleted, search) are duplicated in
baseConditions(lines 42–57) andbuildFilterConditions(lines 67–84). While these serve different Drizzle APIs, the underlying filter logic is identical and must be kept in sync manually.Consider extracting the filter criteria into a shared helper:
// Extract filter criteria const getFilterCriteria = (search: string | undefined) => { return { workspaceId, deleted: false, search: search ? escapeLike(search) : undefined, }; }; const criteria = getFilterCriteria(search); // For SQL builder API (count query) const baseConditions = [ eq(schema.identities.workspaceId, criteria.workspaceId), eq(schema.identities.deleted, criteria.deleted), ]; if (criteria.search) { const searchCondition = or( like(schema.identities.externalId, `%${criteria.search}%`), like(schema.identities.id, `%${criteria.search}%`), ); if (searchCondition) { baseConditions.push(searchCondition); } } // For query API const buildFilterConditions = (identity: any, { and, eq, or, like }: any) => { const conditions = [ eq(identity.workspaceId, criteria.workspaceId), eq(identity.deleted, criteria.deleted), ]; if (criteria.search) { const searchCondition = or( like(identity.externalId, `%${criteria.search}%`), like(identity.id, `%${criteria.search}%`), ); if (searchCondition) { conditions.push(searchCondition); } } return and(...conditions); };This ensures filter logic stays synchronized and reduces maintenance burden.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/table/identities-list.tsx(1 hunks)apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/table/identity-table-actions.tsx(1 hunks)apps/dashboard/lib/trpc/routers/identity/query.ts(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/table/identity-table-actions.tsx
🧰 Additional context used
🧠 Learnings (14)
📓 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.
📚 Learning: 2024-12-03T14:07:45.173Z
Learnt from: ogzhanolguncu
Repo: unkeyed/unkey PR: 2143
File: apps/dashboard/components/ui/group-button.tsx:21-31
Timestamp: 2024-12-03T14:07:45.173Z
Learning: In the `ButtonGroup` component (`apps/dashboard/components/ui/group-button.tsx`), avoid suggesting the use of `role="group"` in ARIA attributes.
Applied to files:
apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/table/identities-list.tsx
📚 Learning: 2025-05-15T16:26:08.666Z
Learnt from: ogzhanolguncu
Repo: unkeyed/unkey PR: 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)/[workspaceSlug]/identities/_components/table/identities-list.tsx
📚 Learning: 2024-10-23T16:25:33.113Z
Learnt from: p6l-richard
Repo: unkeyed/unkey PR: 2085
File: apps/www/components/glossary/terms-stepper-mobile.tsx:16-20
Timestamp: 2024-10-23T16:25:33.113Z
Learning: In the `apps/www/components/glossary/terms-stepper-mobile.tsx` file, avoid suggesting to extract the term navigation logic into a custom hook, as the user prefers to keep the component straightforward.
Applied to files:
apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/table/identities-list.tsx
📚 Learning: 2024-12-03T14:17:08.016Z
Learnt from: ogzhanolguncu
Repo: unkeyed/unkey PR: 2143
File: apps/dashboard/app/(app)/logs/logs-page.tsx:77-83
Timestamp: 2024-12-03T14:17:08.016Z
Learning: The `<LogsTable />` component already implements virtualization to handle large datasets efficiently.
Applied to files:
apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/table/identities-list.tsx
📚 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.ts
📚 Learning: 2025-02-27T14:35:15.251Z
Learnt from: ogzhanolguncu
Repo: unkeyed/unkey PR: 2918
File: apps/dashboard/lib/trpc/routers/api/overview-api-search.ts:0-0
Timestamp: 2025-02-27T14:35:15.251Z
Learning: When using Drizzle ORM with LIKE queries, use the sql`` tagged template syntax (e.g., sql`${table.name} LIKE ${`%${input.query}%`}`) instead of direct string interpolation to prevent SQL injection attacks.
Applied to files:
apps/dashboard/lib/trpc/routers/identity/query.ts
📚 Learning: 2025-06-25T19:51:15.995Z
Learnt from: ogzhanolguncu
Repo: unkeyed/unkey PR: 3369
File: apps/dashboard/lib/trpc/routers/settings/root-keys/query.ts:67-67
Timestamp: 2025-06-25T19:51:15.995Z
Learning: Drizzle ORM automatically handles SQL parameterization and escaping for queries, including LIKE queries with direct string interpolation (e.g., `like(schema.keys.name, \`%${filter.value}%\`)`), making them safe from SQL injection attacks without requiring explicit escaping or sql`` tagged template syntax.
Applied to files:
apps/dashboard/lib/trpc/routers/identity/query.ts
📚 Learning: 2025-02-10T14:12:17.261Z
Learnt from: ogzhanolguncu
Repo: unkeyed/unkey PR: 2883
File: apps/dashboard/app/(app)/logs/components/controls/components/logs-search/index.tsx:29-31
Timestamp: 2025-02-10T14:12:17.261Z
Learning: In the logs search component's error handling, error messages are deliberately wrapped in single quotes within template literals for visual distinction (e.g. "Unable to process your search request 'some error message'").
Applied to files:
apps/dashboard/lib/trpc/routers/identity/query.ts
📚 Learning: 2025-04-08T09:34:24.576Z
Learnt from: ogzhanolguncu
Repo: unkeyed/unkey PR: 2872
File: apps/dashboard/lib/trpc/routers/ratelimit/createNamespace.ts:36-39
Timestamp: 2025-04-08T09:34:24.576Z
Learning: In the Unkey dashboard, when making database queries involving workspaces, use `ctx.workspace.id` directly instead of fetching the workspace separately for better performance and security.
Applied to files:
apps/dashboard/lib/trpc/routers/identity/query.ts
📚 Learning: 2025-12-05T12:05:33.143Z
Learnt from: Flo4604
Repo: unkeyed/unkey PR: 4484
File: go/pkg/db/queries/identity_list.sql:11-24
Timestamp: 2025-12-05T12:05:33.143Z
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.ts
📚 Learning: 2025-06-24T13:29:10.129Z
Learnt from: ogzhanolguncu
Repo: unkeyed/unkey PR: 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/lib/trpc/routers/identity/query.ts
📚 Learning: 2025-05-15T15:57:02.128Z
Learnt from: mcstepp
Repo: unkeyed/unkey PR: 3242
File: apps/dashboard/app/(app)/apis/[apiId]/api-id-navbar.tsx:47-50
Timestamp: 2025-05-15T15:57:02.128Z
Learning: When reviewing code for Unkey, prefer using `Boolean()` over the double negation (`!!`) operator for boolean coercion, as their linter rules favor this pattern.
Applied to files:
apps/dashboard/lib/trpc/routers/identity/query.ts
📚 Learning: 2025-05-15T16:09:49.243Z
Learnt from: mcstepp
Repo: unkeyed/unkey PR: 3242
File: apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/components/controls/components/logs-search/index.tsx:7-43
Timestamp: 2025-05-15T16:09:49.243Z
Learning: For type safety issues involving `any` type assertions, the team prefers to address these systematically with linter updates rather than fixing them individually in code reviews.
Applied to files:
apps/dashboard/lib/trpc/routers/identity/query.ts
🧬 Code graph analysis (1)
apps/dashboard/lib/trpc/routers/identity/query.ts (3)
apps/dashboard/lib/trpc/routers/utils/sql.ts (1)
escapeLike(15-17)internal/rbac/src/queries.ts (2)
or(46-50)and(52-56)apps/dashboard/lib/db.ts (1)
db(5-26)
⏰ 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 (1)
apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/table/identities-list.tsx (1)
78-80: Verify that aggressive caching strategy aligns with UX requirements.The query is configured to never become stale and never refetch on mount or window focus. This means users won't see updates (new identities, deletions, metadata changes) unless they explicitly refresh the page or trigger a refetch action.
While this optimizes performance and reduces server load, it may impact user experience if they expect to see recent changes, especially in collaborative environments or after performing actions in other tabs.
Consider if one of the following adjustments better serves your use case:
Allow refetch on window focus for cross-tab sync:
- refetchOnWindowFocus: false, + refetchOnWindowFocus: true,Set a reasonable staleTime instead of infinity:
- staleTime: Number.POSITIVE_INFINITY, + staleTime: 5 * 60 * 1000, // 5 minutesProvide a manual refresh button in the UI for users to fetch latest data.
If the current behavior is intentional for this feature, consider documenting the rationale.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/table/identities-list.tsx (2)
65-82: Add error handling for the query.The
useInfiniteQueryresult doesn't destructure or handle theerrorstate. If the query fails, users will see an indefinite loading state or an empty state without explanation.Add error handling:
const { data: identitiesData, hasNextPage, fetchNextPage, isFetchingNextPage, isLoading: isLoadingInitial, + error, + isError, } = trpc.identity.query.useInfiniteQuery(Then add an error state to your component (before the VirtualTable return) to display a user-friendly message when
isErroris true.
158-160: Consider usingcn()for className composition.The template literal for className works but is inconsistent with the rest of the codebase's use of the
cn()utility.Apply this diff for consistency:
<Link - className={`font-mono group-hover:underline decoration-dotted text-accent-9 ${ - isNavigating ? "pointer-events-none opacity-50" : "" - }`} + className={cn( + "font-mono group-hover:underline decoration-dotted text-accent-9", + isNavigating && "pointer-events-none opacity-50" + )} href={`/${workspace.slug}/identities/${identity.id}`}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/table/identities-list.tsx(1 hunks)apps/dashboard/app/(app)/[workspaceSlug]/identities/page.tsx(2 hunks)apps/dashboard/lib/trpc/routers/identity/search.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/dashboard/lib/trpc/routers/identity/search.ts
🧰 Additional context used
🧠 Learnings (9)
📓 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.
📚 Learning: 2024-12-03T14:07:45.173Z
Learnt from: ogzhanolguncu
Repo: unkeyed/unkey PR: 2143
File: apps/dashboard/components/ui/group-button.tsx:21-31
Timestamp: 2024-12-03T14:07:45.173Z
Learning: In the `ButtonGroup` component (`apps/dashboard/components/ui/group-button.tsx`), avoid suggesting the use of `role="group"` in ARIA attributes.
Applied to files:
apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/table/identities-list.tsx
📚 Learning: 2025-05-15T16:26:08.666Z
Learnt from: ogzhanolguncu
Repo: unkeyed/unkey PR: 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)/[workspaceSlug]/identities/_components/table/identities-list.tsxapps/dashboard/app/(app)/[workspaceSlug]/identities/page.tsx
📚 Learning: 2024-10-23T16:25:33.113Z
Learnt from: p6l-richard
Repo: unkeyed/unkey PR: 2085
File: apps/www/components/glossary/terms-stepper-mobile.tsx:16-20
Timestamp: 2024-10-23T16:25:33.113Z
Learning: In the `apps/www/components/glossary/terms-stepper-mobile.tsx` file, avoid suggesting to extract the term navigation logic into a custom hook, as the user prefers to keep the component straightforward.
Applied to files:
apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/table/identities-list.tsx
📚 Learning: 2024-12-03T14:17:08.016Z
Learnt from: ogzhanolguncu
Repo: unkeyed/unkey PR: 2143
File: apps/dashboard/app/(app)/logs/logs-page.tsx:77-83
Timestamp: 2024-12-03T14:17:08.016Z
Learning: The `<LogsTable />` component already implements virtualization to handle large datasets efficiently.
Applied to files:
apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/table/identities-list.tsx
📚 Learning: 2025-07-28T19:42:37.047Z
Learnt from: mcstepp
Repo: unkeyed/unkey PR: 3662
File: apps/dashboard/app/(app)/projects/page.tsx:74-81
Timestamp: 2025-07-28T19:42:37.047Z
Learning: In apps/dashboard/app/(app)/projects/page.tsx, the user mcstepp prefers to keep placeholder functions like generateSlug inline during POC/demonstration phases rather than extracting them to utility modules, with plans to refactor later when the feature matures beyond the proof-of-concept stage.
Applied to files:
apps/dashboard/app/(app)/[workspaceSlug]/identities/page.tsx
📚 Learning: 2025-08-18T10:28:47.391Z
Learnt from: ogzhanolguncu
Repo: unkeyed/unkey PR: 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)/[workspaceSlug]/identities/page.tsx
📚 Learning: 2025-06-10T14:21:42.413Z
Learnt from: ogzhanolguncu
Repo: unkeyed/unkey PR: 3311
File: apps/dashboard/components/logs/llm-search/components/search-input.tsx:14-14
Timestamp: 2025-06-10T14:21:42.413Z
Learning: In Next.js applications, importing backend modules into frontend components causes bundling issues and can expose server-side code to the client bundle. For simple constants like limits, it's acceptable to duplicate the value rather than compromise the frontend/backend architectural separation.
Applied to files:
apps/dashboard/app/(app)/[workspaceSlug]/identities/page.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/page.tsx
🧬 Code graph analysis (1)
apps/dashboard/app/(app)/[workspaceSlug]/identities/page.tsx (1)
apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/identities-client.tsx (1)
IdentitiesClient(6-13)
⏰ 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 (5)
apps/dashboard/app/(app)/[workspaceSlug]/identities/page.tsx (1)
1-20: LGTM! Clean simplification.The page component has been appropriately simplified by delegating the complex table logic to
IdentitiesClient. The beta feature gate is preserved, and the previous issue with thedynamicexport has been resolved.apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/table/identities-list.tsx (4)
29-45: Good use of dynamic import for code splitting.The lazy-loaded
IdentityTableActionPopoverwith a matching skeleton reduces the initial bundle size. The loading fallback UI is consistent with the final rendered state.
157-173: Navigation guard properly implemented.The link now includes both a visual/interaction lock (
pointer-events-noneandopacity-50) and an explicit guard in the onClick handler (lines 164-167) that prevents navigation during loading. This addresses the previous review concern.
262-272: Correct use oftotalCountfor pagination display.The count info text now properly displays
identitiesList.length(items loaded) vstotalCount(total available) instead of showing the same value twice. This addresses the previous review feedback.
78-81: This query is properly invalidated on identity mutations.The query configuration with
staleTime: Number.POSITIVE_INFINITYand manual refetch prevention is safe because all identity mutations that affect this list (currently onlyidentity.create) properly invalidateidentity.querythrough theironSuccesscallbacks. Both creation flows callinvalidateQueriesfor this query.
MichaelUnkey
left a comment
MichaelUnkey
left a comment
There was a problem hiding this comment.
Long id makes text centered? only thing i noticed
Interesting! I dug into it and its because its a button under the hood. The text is always centered in the button, and the button width is the size of the cell. I can fix it though. Good catch! |
mcstepp
force-pushed
the
ENG-2268-identities-list-view
branch
from
ee7d5b7 to
30945a4
Compare
Fixed. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (7)
apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/table/skeletons.tsx (1)
36-46: Consider using adivinstead ofbuttonfor the skeleton.The
ActionColumnSkeletonrenders a non-interactive loading placeholder. Using a<button>element is semantically incorrect since it's not an actionable control during loading. This could also cause accessibility issues as screen readers will announce it as a button.-export const ActionColumnSkeleton = () => ( - <button - type="button" - className={cn( - "group size-5 p-0 rounded m-0 items-center flex justify-center animate-pulse", - "border border-gray-6", - )} - > - <Dots className="text-gray-11" iconSize="sm-regular" /> - </button> -); +export const ActionColumnSkeleton = () => ( + <div + className={cn( + "size-5 rounded flex items-center justify-center animate-pulse", + "border border-gray-6", + )} + > + <Dots className="text-gray-11" iconSize="sm-regular" /> + </div> +);apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/create-identity-dialog.tsx (2)
16-18: Redundant refinement check.The third refine (
trimmed !== "") on line 18 is unreachable—iftrimmed.length >= 3passes, the string cannot be empty. This check was likely left over from an earlier iteration.externalId: z .string() .transform((s) => s.trim()) .refine((trimmed) => trimmed.length >= 3, "External ID must be at least 3 characters") - .refine((trimmed) => trimmed.length <= 255, "External ID must be 255 characters or fewer") - .refine((trimmed) => trimmed !== "", "External ID cannot be only whitespace"), + .refine((trimmed) => trimmed.length <= 255, "External ID must be 255 characters or fewer"),
22-39: Consider separating JSON validity and size validation for clearer error messages.The current refine combines two distinct validations (valid JSON and size limit) with a single error message. Users won't know which constraint they violated. However, this is acceptable for an initial implementation.
meta: z .string() .optional() + .refine( + (val) => { + if (!val || val.trim() === "") return true; + try { + JSON.parse(val); + return true; + } catch { + return false; + } + }, + { message: "Must be valid JSON" }, + ) .refine( (val) => { - if (!val || val.trim() === "") { - return true; - } - try { - JSON.parse(val); - // Check size limit (1MB) - const size = new Blob([val]).size; - return size < 1024 * 1024; - } catch { - return false; - } - }, - { - message: "Must be valid JSON and less than 1MB", + if (!val || val.trim() === "") return true; + const size = new Blob([val]).size; + return size < 1024 * 1024; }, + { message: "Metadata must be less than 1MB" }, ),apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/controls/identities-search.tsx (1)
24-24:isLoadingis hardcoded tofalse.The
LLMSearchcomponent accepts anisLoadingprop to display loading feedback during searches, but it's currently hardcoded. The loading state from the identities query exists in theIdentitiesListcomponent, separate from this search control. Consider restructuring to pass the loading state from the list query to this search component.apps/dashboard/lib/trpc/routers/identity/query.ts (1)
39-85: Deduplicate filter construction between count and main queryRight now the workspace/deleted/search filters are implemented twice (
baseConditionsfor the count query andbuildFilterConditionsfor the main query). They’re logically equivalent but will be easy to drift and re-runescapeLikeunnecessarily.Consider building the conditions in a single helper and reusing it for both queries, e.g. by:
- Defining
buildFilterConditionsabove the count query, and- Using it for the count as well, something like:
.where(buildFilterConditions(schema.identities, { and, eq, or, like }))This keeps the search behaviour and workspace scoping in one place.
apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/table/identities-list.tsx (2)
58-61:selectedIdentityis never set; selection styling is effectively dead code
selectedIdentityis only reset tonullinhandleLinkClickand never set to a realIdentity, but it’s passed toVirtualTableand used inrowClassName. That means the “selected row” styling can never actually activate.Either:
- Wire it up in
handleRowClick(or similar) if you want a selected-row affordance:const handleRowClick = useCallback( (identity: Identity) => { setSelectedIdentity(identity); router.push(`/${workspace.slug}/identities/${identity.id}`); }, [router, workspace.slug], );or
- Drop
selectedIdentityand the related props if row selection isn’t part of the UX.This will simplify the component and avoid confusing unused state.
Also applies to: 250-256
274-297: Consider adding a “Create Identity” CTA directly in the empty stateThe empty state currently only surfaces a “Learn about Identities” docs button. Given the PR/issue goals call out a Create Identity CTA in the empty state, you may want to add a secondary action here (e.g. a button that opens the create-identity dialog) so users have an obvious primary action when the table is empty, not just in the header/controls.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/controls/identities-search.tsx(1 hunks)apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/controls/index.tsx(1 hunks)apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/create-identity-dialog.tsx(1 hunks)apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/identities-client.tsx(1 hunks)apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/table/identities-list.tsx(1 hunks)apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/table/identity-table-actions.tsx(1 hunks)apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/table/last-used.tsx(1 hunks)apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/table/skeletons.tsx(1 hunks)apps/dashboard/app/(app)/[workspaceSlug]/identities/components/results.tsx(0 hunks)apps/dashboard/app/(app)/[workspaceSlug]/identities/filter.tsx(0 hunks)apps/dashboard/app/(app)/[workspaceSlug]/identities/navigation.tsx(2 hunks)apps/dashboard/app/(app)/[workspaceSlug]/identities/page.tsx(2 hunks)apps/dashboard/app/(app)/[workspaceSlug]/identities/row.tsx(0 hunks)apps/dashboard/lib/trpc/routers/identity/latestVerification.ts(1 hunks)apps/dashboard/lib/trpc/routers/identity/query.ts(5 hunks)apps/dashboard/lib/trpc/routers/identity/search.ts(3 hunks)apps/dashboard/lib/trpc/routers/index.ts(2 hunks)apps/dashboard/lib/trpc/routers/utils/sql.ts(1 hunks)
💤 Files with no reviewable changes (3)
- apps/dashboard/app/(app)/[workspaceSlug]/identities/components/results.tsx
- apps/dashboard/app/(app)/[workspaceSlug]/identities/filter.tsx
- apps/dashboard/app/(app)/[workspaceSlug]/identities/row.tsx
🚧 Files skipped from review as they are similar to previous changes (5)
- apps/dashboard/lib/trpc/routers/identity/search.ts
- apps/dashboard/lib/trpc/routers/identity/latestVerification.ts
- apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/table/identity-table-actions.tsx
- apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/table/last-used.tsx
- apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/controls/index.tsx
🧰 Additional context used
🧠 Learnings (25)
📓 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.
📚 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.ts
📚 Learning: 2025-02-27T14:35:15.251Z
Learnt from: ogzhanolguncu
Repo: unkeyed/unkey PR: 2918
File: apps/dashboard/lib/trpc/routers/api/overview-api-search.ts:0-0
Timestamp: 2025-02-27T14:35:15.251Z
Learning: When using Drizzle ORM with LIKE queries, use the sql`` tagged template syntax (e.g., sql`${table.name} LIKE ${`%${input.query}%`}`) instead of direct string interpolation to prevent SQL injection attacks.
Applied to files:
apps/dashboard/lib/trpc/routers/identity/query.tsapps/dashboard/lib/trpc/routers/utils/sql.ts
📚 Learning: 2025-06-25T19:51:15.995Z
Learnt from: ogzhanolguncu
Repo: unkeyed/unkey PR: 3369
File: apps/dashboard/lib/trpc/routers/settings/root-keys/query.ts:67-67
Timestamp: 2025-06-25T19:51:15.995Z
Learning: Drizzle ORM automatically handles SQL parameterization and escaping for queries, including LIKE queries with direct string interpolation (e.g., `like(schema.keys.name, \`%${filter.value}%\`)`), making them safe from SQL injection attacks without requiring explicit escaping or sql`` tagged template syntax.
Applied to files:
apps/dashboard/lib/trpc/routers/identity/query.tsapps/dashboard/lib/trpc/routers/utils/sql.ts
📚 Learning: 2025-02-10T14:12:17.261Z
Learnt from: ogzhanolguncu
Repo: unkeyed/unkey PR: 2883
File: apps/dashboard/app/(app)/logs/components/controls/components/logs-search/index.tsx:29-31
Timestamp: 2025-02-10T14:12:17.261Z
Learning: In the logs search component's error handling, error messages are deliberately wrapped in single quotes within template literals for visual distinction (e.g. "Unable to process your search request 'some error message'").
Applied to files:
apps/dashboard/lib/trpc/routers/identity/query.ts
📚 Learning: 2025-04-08T09:34:24.576Z
Learnt from: ogzhanolguncu
Repo: unkeyed/unkey PR: 2872
File: apps/dashboard/lib/trpc/routers/ratelimit/createNamespace.ts:36-39
Timestamp: 2025-04-08T09:34:24.576Z
Learning: In the Unkey dashboard, when making database queries involving workspaces, use `ctx.workspace.id` directly instead of fetching the workspace separately for better performance and security.
Applied to files:
apps/dashboard/lib/trpc/routers/identity/query.ts
📚 Learning: 2025-12-05T12:05:33.143Z
Learnt from: Flo4604
Repo: unkeyed/unkey PR: 4484
File: go/pkg/db/queries/identity_list.sql:11-24
Timestamp: 2025-12-05T12:05:33.143Z
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.ts
📚 Learning: 2025-06-24T13:29:10.129Z
Learnt from: ogzhanolguncu
Repo: unkeyed/unkey PR: 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/lib/trpc/routers/identity/query.ts
📚 Learning: 2024-10-23T16:21:47.395Z
Learnt from: p6l-richard
Repo: unkeyed/unkey PR: 2085
File: apps/www/components/glossary/search.tsx:16-20
Timestamp: 2024-10-23T16:21:47.395Z
Learning: For the `FilterableCommand` component in `apps/www/components/glossary/search.tsx`, refactoring type definitions into an interface is not necessary at this time.
Applied to files:
apps/dashboard/lib/trpc/routers/identity/query.ts
📚 Learning: 2025-05-15T15:57:02.128Z
Learnt from: mcstepp
Repo: unkeyed/unkey PR: 3242
File: apps/dashboard/app/(app)/apis/[apiId]/api-id-navbar.tsx:47-50
Timestamp: 2025-05-15T15:57:02.128Z
Learning: When reviewing code for Unkey, prefer using `Boolean()` over the double negation (`!!`) operator for boolean coercion, as their linter rules favor this pattern.
Applied to files:
apps/dashboard/lib/trpc/routers/identity/query.ts
📚 Learning: 2025-05-15T16:09:49.243Z
Learnt from: mcstepp
Repo: unkeyed/unkey PR: 3242
File: apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/components/controls/components/logs-search/index.tsx:7-43
Timestamp: 2025-05-15T16:09:49.243Z
Learning: For type safety issues involving `any` type assertions, the team prefers to address these systematically with linter updates rather than fixing them individually in code reviews.
Applied to files:
apps/dashboard/lib/trpc/routers/identity/query.ts
📚 Learning: 2025-08-18T10:28:47.391Z
Learnt from: ogzhanolguncu
Repo: unkeyed/unkey PR: 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)/[workspaceSlug]/identities/_components/identities-client.tsxapps/dashboard/app/(app)/[workspaceSlug]/identities/page.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/_components/identities-client.tsx
📚 Learning: 2025-07-28T19:42:37.047Z
Learnt from: mcstepp
Repo: unkeyed/unkey PR: 3662
File: apps/dashboard/app/(app)/projects/page.tsx:74-81
Timestamp: 2025-07-28T19:42:37.047Z
Learning: In apps/dashboard/app/(app)/projects/page.tsx, the user mcstepp prefers to keep placeholder functions like generateSlug inline during POC/demonstration phases rather than extracting them to utility modules, with plans to refactor later when the feature matures beyond the proof-of-concept stage.
Applied to files:
apps/dashboard/app/(app)/[workspaceSlug]/identities/page.tsx
📚 Learning: 2025-06-10T14:21:42.413Z
Learnt from: ogzhanolguncu
Repo: unkeyed/unkey PR: 3311
File: apps/dashboard/components/logs/llm-search/components/search-input.tsx:14-14
Timestamp: 2025-06-10T14:21:42.413Z
Learning: In Next.js applications, importing backend modules into frontend components causes bundling issues and can expose server-side code to the client bundle. For simple constants like limits, it's acceptable to duplicate the value rather than compromise the frontend/backend architectural separation.
Applied to files:
apps/dashboard/app/(app)/[workspaceSlug]/identities/page.tsx
📚 Learning: 2025-05-15T16:26:08.666Z
Learnt from: ogzhanolguncu
Repo: unkeyed/unkey PR: 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)/[workspaceSlug]/identities/page.tsxapps/dashboard/app/(app)/[workspaceSlug]/identities/_components/table/identities-list.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/page.tsxapps/dashboard/app/(app)/[workspaceSlug]/identities/navigation.tsx
📚 Learning: 2024-12-03T14:07:45.173Z
Learnt from: ogzhanolguncu
Repo: unkeyed/unkey PR: 2143
File: apps/dashboard/components/ui/group-button.tsx:21-31
Timestamp: 2024-12-03T14:07:45.173Z
Learning: In the `ButtonGroup` component (`apps/dashboard/components/ui/group-button.tsx`), avoid suggesting the use of `role="group"` in ARIA attributes.
Applied to files:
apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/table/identities-list.tsx
📚 Learning: 2024-12-03T14:17:08.016Z
Learnt from: ogzhanolguncu
Repo: unkeyed/unkey PR: 2143
File: apps/dashboard/app/(app)/logs/logs-page.tsx:77-83
Timestamp: 2024-12-03T14:17:08.016Z
Learning: The `<LogsTable />` component already implements virtualization to handle large datasets efficiently.
Applied to files:
apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/table/identities-list.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-05-16T16:16:02.286Z
Learnt from: mcstepp
Repo: unkeyed/unkey PR: 3258
File: apps/dashboard/components/dashboard/feedback-component.tsx:28-35
Timestamp: 2025-05-16T16:16:02.286Z
Learning: When validating string inputs in forms using Zod, it's best practice to use `.trim()` before length checks to prevent submissions consisting only of whitespace characters, particularly for feedback forms where meaningful content is expected.
Applied to files:
apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/create-identity-dialog.tsx
📚 Learning: 2025-07-16T17:51:57.297Z
Learnt from: chronark
Repo: unkeyed/unkey PR: 3617
File: go/apps/api/openapi/openapi.yaml:3309-3312
Timestamp: 2025-07-16T17:51:57.297Z
Learning: In the Unkey API OpenAPI schema, the permissions query regex for the verifyKey endpoint intentionally allows all whitespace characters (including tabs and newlines) via `\s`. Do not flag this as an error in future reviews.
Applied to files:
apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/create-identity-dialog.tsx
📚 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/app/(app)/[workspaceSlug]/identities/_components/create-identity-dialog.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/app/(app)/[workspaceSlug]/identities/_components/create-identity-dialog.tsx
📚 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/create-identity-dialog.tsx
🧬 Code graph analysis (6)
apps/dashboard/lib/trpc/routers/index.ts (1)
apps/dashboard/lib/trpc/routers/identity/latestVerification.ts (1)
identityLastVerificationTime(10-54)
apps/dashboard/lib/trpc/routers/identity/query.ts (3)
apps/dashboard/lib/trpc/routers/utils/sql.ts (1)
escapeLike(15-17)internal/rbac/src/queries.ts (2)
or(46-50)and(52-56)apps/dashboard/lib/db.ts (1)
db(5-26)
apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/controls/identities-search.tsx (1)
internal/ui/src/components/llm-search/index.tsx (1)
LLMSearch(176-176)
apps/dashboard/app/(app)/[workspaceSlug]/identities/page.tsx (1)
apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/identities-client.tsx (1)
IdentitiesClient(6-13)
apps/dashboard/app/(app)/[workspaceSlug]/identities/navigation.tsx (1)
apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/create-identity-dialog.tsx (1)
CreateIdentityDialog(44-155)
apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/create-identity-dialog.tsx (3)
internal/ui/src/components/toaster.tsx (1)
toast(29-29)internal/ui/src/components/dialog/dialog-container.tsx (1)
DialogContainer(66-66)internal/ui/src/components/buttons/button.tsx (1)
Button(439-439)
⏰ 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 (6)
apps/dashboard/lib/trpc/routers/index.ts (1)
60-60: LGTM!The new
latestVerificationendpoint is correctly wired and follows the established pattern used forapi.keys.latestVerification(line 211). The import and router structure are consistent with the codebase conventions.Also applies to: 346-346
apps/dashboard/app/(app)/[workspaceSlug]/identities/navigation.tsx (1)
18-20: LGTM!The
CreateIdentityDialogis appropriately placed withinNavbar.Actions, following the established navigation patterns. Based on learnings,useWorkspaceNavigationguarantees the workspace exists, so there are no null-safety concerns.apps/dashboard/lib/trpc/routers/utils/sql.ts (1)
15-17: LGTM!The escape sequence order is correct—backslashes must be escaped first to prevent double-escaping from subsequent replacements. The documentation is clear with a practical example.
apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/identities-client.tsx (1)
6-13: LGTM!Clean composition component that establishes the client boundary for the identities feature. The child components will inherit the client context as per Next.js App Router conventions.
apps/dashboard/app/(app)/[workspaceSlug]/identities/_components/create-identity-dialog.tsx (1)
63-84: LGTM!The mutation handlers are well-implemented:
- Success path correctly invalidates queries, shows a toast, closes the dialog, and resets the form.
- Error handling appropriately distinguishes between CONFLICT (duplicate external ID) and other errors.
apps/dashboard/app/(app)/[workspaceSlug]/identities/page.tsx (1)
4-18: IdentitiesClient-based page composition looks correctThe page now cleanly delegates all list UI to
IdentitiesClientwhile preserving the identities beta gate and existingNavigation. No behavioural or routing issues stand out here.
3 participants
What does this PR do?
Styled identities list to match keys page:
Added Create Identity feature:
Note: More to be added in future tickets. This is just feature parity with existing identities functionality, but with the updated styling to match the rest of the dashboard.
Fixes #4457
Type of change
How should this be tested?
Key checks:
Checklist
Required
pnpm buildpnpm fmtmake fmton/godirectoryconsole.logsgit pull origin mainAppreciated