Skip to content

refactor: optimize roles#3324

Merged
chronark merged 15 commits intomainfrom
refactor-optimize-roles
Jul 7, 2025
Merged

refactor: optimize roles#3324
chronark merged 15 commits intomainfrom
refactor-optimize-roles

Conversation

@ogzhanolguncu
Copy link
Contributor

@ogzhanolguncu ogzhanolguncu commented Jun 13, 2025

What does this PR do?

This PR optimizes the way we load roles table views. Before this PR, we were fetching everything related to roles, like connected keys and key counts. Same for permissions. And it was making the roles quite slow when there were so many keys attached to roles. Right now, this PR allows us to fetch connected keys and perms separately for each roleId. Thanks to tRPC batching and our virtual tables, it loads much faster.

Btw, the reason for using sql syntax instead of drizzle syntax in some cases is that I noticed some latency improvements that way. I couldn't translate the sql syntax to drizzle without losing the latency benefits.

Fixes # (issue)

If there is not an issue for this, please create one first. This is used to tracking purposes and also helps use understand why this PR exists

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Chore (refactoring code, technical debt, workflow improvements)
  • Enhancement (small improvements)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How should this be tested?

  • /authorization/roles should not regress. So it should match the prod
  • Role's edit should work. Make sure key updates and permission updates work
  • Verify that if you have more than 50 keys or 50 permissions attached UI shouldn't let you update it through UI. We might change that 50 in the future.
  • Make sure no data is lost when updating roles with more than 50 keys and permissions.

Checklist

Required

  • Filled out the "How to test" section in this PR
  • Read Contributing Guide
  • Self-reviewed my own code
  • Commented on my code in hard-to-understand areas
  • Ran pnpm build
  • Ran pnpm fmt
  • Checked for warnings, there are none
  • Removed all console.logs
  • Merged the latest changes from main onto my branch with git pull origin main
  • My changes don't cause any responsiveness issues

Appreciated

  • If a UI change was made: Added a screen recording or screenshots to this PR
  • Updated the Unkey Docs if changes were necessary

Summary by CodeRabbit

  • New Features

    • Introduced role-based assignment limits with warnings for keys and permissions when creating or editing roles.
    • Added summary counts for assigned keys, permissions, and roles in tables, replacing detailed item listings.
    • Added new warning callout component to inform users when assignment limits are reached.
  • Enhancements

    • Improved table and form components to show concise assignment summaries and limit warnings.
    • Updated loading skeletons and animations for a more polished UI experience.
    • Simplified role and permission data models for faster loading and easier navigation.
    • Expanded cache refresh logic to ensure up-to-date data after changes.
  • Bug Fixes

    • Improved null safety and error handling in permission assignment and display.
  • Chores

    • Updated internal data structures and queries for better performance and maintainability.

@changeset-bot
Copy link

changeset-bot bot commented Jun 13, 2025

⚠️ No Changeset found

Latest commit: e0cd6cc

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@vercel
Copy link

vercel bot commented Jun 13, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
dashboard 🛑 Canceled (Inspect) Jul 3, 2025 11:34am
1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
engineering ⬜️ Ignored (Inspect) Visit Preview Jul 3, 2025 11:34am

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jun 13, 2025

📝 Walkthrough

Walkthrough

This change refactors the handling of role and permission assignments in the dashboard application. It simplifies role and permission queries to return only basic data, introduces new endpoints for counting assigned keys and permissions, enforces assignment limits in the UI, and updates table components to display summarized assignment counts instead of item lists. Related hooks, schemas, and cache invalidation logic are also updated.

Changes

File(s) / Path(s) Change Summary
.../authorization/roles/components/table/components/actions/components/delete-role.tsx
.../edit-role.tsx
.../role-info.tsx
.../keys-table-action.popover.constants.tsx
.../utils/get-row-class.ts
Changed prop and parameter types from Roles to RoleBasic in DeleteRole, EditRole, RoleInfo, RolesTableActions, getRolesTableActionItems, and getRowClassName.
.../authorization/roles/components/table/components/actions/components/hooks/use-fetch-connected-keys-and-perms.ts Renamed hook to useFetchConnectedKeysAndPermsData, updated to use useRoleLimits, changed return structure, and conditional query enabling.
.../authorization/roles/components/table/components/assigned-items-cell.tsx Refactored to fetch assigned items internally via TRPC, changed props to use roleId and kind, and now displays summarized assignment counts with new animations.
.../authorization/roles/components/table/hooks/use-role-limits.ts Introduced new hook useRoleLimits to manage and enforce assignment limits for keys and permissions.
.../authorization/roles/components/table/hooks/use-roles-list-query.ts Updated internal typing from Roles to RoleBasic; no signature changes.
.../authorization/roles/components/table/roles-list.tsx Updated to use RoleBasic, simplified assigned items columns to use new AssignedItemsCell props, updated headers.
.../authorization/roles/components/table/components/skeletons.tsx Added animation classes, removed duplicate skeleton items, updated pulse bar widths.
.../authorization/permissions/components/table/components/assigned-items-cell.tsx Refactored to use value and kind props, added icon and label helpers, now displays summarized count and label for assignments.
.../authorization/permissions/components/table/components/skeletons.tsx Changed icons and structure for skeleton components, added new icon for keys.
.../authorization/permissions/components/table/permissions-list.tsx Simplified columns to use new AssignedItemsCell props, removed badge and status styling logic.
.../authorization/roles/components/upsert-role/components/assign-key/key-field.tsx
.../assign-permission/permissions-field.tsx
Integrated useRoleLimits for enforcing assignment limits, conditionally disables inputs and shows warnings.
.../authorization/roles/components/upsert-role/components/warning-callout.tsx Added new RoleWarningCallout component for displaying assignment limit warnings.
.../authorization/roles/components/upsert-role/hooks/use-upsert-role.ts
.../permissions/components/upsert-permission/hooks/use-upsert-permission.ts
Expanded cache invalidation logic after mutations to include related roles and assignments.
.../authorization/roles/components/upsert-role/index.tsx Integrated useRoleLimits into role upsert dialog, conditionally omits assignments in mutation if limits are exceeded.
.../authorization/roles/components/upsert-role/upsert-role.schema.ts Simplified and clarified optionality for keyIds and permissionIds schemas.
.../lib/trpc/routers/authorization/roles/keys/connected-keys.ts
.../permissions/connected-permissions.ts
Added new TRPC procedures to count assigned keys and permissions for a role.
.../lib/trpc/routers/authorization/roles/query.ts Refactored roles query to return only basic role data, removed aggregation of assigned keys/permissions.
.../lib/trpc/routers/authorization/roles/upsert.ts Updated upsert logic to conditionally update assignments only if arrays are provided, improved error handling.
.../lib/trpc/routers/index.ts Registered new endpoints for connected keys and permissions in the roles router.
.../lib/trpc/routers/authorization/permissions/query.ts Simplified permissions query to return only total connected roles, removed role name aggregation.

Sequence Diagram(s)

sequenceDiagram
    participant UI
    participant UpsertRoleDialog
    participant useRoleLimits
    participant TRPC
    participant DB

    UI->>UpsertRoleDialog: Submit role form (with keyIds/permissionIds)
    UpsertRoleDialog->>useRoleLimits: calculateLimits(keyIds, permissionIds)
    useRoleLimits-->>UpsertRoleDialog: { hasKeyWarning, hasPermWarning }
    UpsertRoleDialog->>TRPC: upsertRole({ keyIds, permissionIds }) (omit if warning)
    TRPC->>DB: Insert/Update role, key-role, role-permission associations
    DB-->>TRPC: Success/Failure
    TRPC-->>UpsertRoleDialog: Mutation result
    UpsertRoleDialog->>TRPC: Invalidate related caches (roles, permissions, assignments)
    TRPC-->>UpsertRoleDialog: Cache invalidated
Loading
sequenceDiagram
    participant UI
    participant AssignedItemsCell
    participant TRPC
    participant DB

    UI->>AssignedItemsCell: Render with roleId and kind
    AssignedItemsCell->>TRPC: Fetch assigned keys/permissions count (by kind)
    TRPC->>DB: Query count for roleId
    DB-->>TRPC: { totalCount }
    TRPC-->>AssignedItemsCell: { totalCount }
    AssignedItemsCell-->>UI: Render summary (e.g., "3 Keys")
Loading

Suggested labels

🕹️ oss.gg, :joystick: 750 points

Suggested reviewers

  • perkinsjr
  • chronark
  • mcstepp
✨ Finishing Touches
  • 📝 Generate Docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@github-actions
Copy link
Contributor

github-actions bot commented Jun 13, 2025

Thank you for following the naming conventions for pull request titles! 🙏

@vercel vercel bot temporarily deployed to Preview – dashboard June 13, 2025 13:55 Inactive
@vercel vercel bot temporarily deployed to Preview – dashboard June 13, 2025 15:31 Inactive
@ogzhanolguncu ogzhanolguncu marked this pull request as ready for review June 13, 2025 15:34
@vercel vercel bot temporarily deployed to Preview – dashboard June 13, 2025 15:36 Inactive
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 17

🔭 Outside diff range comments (5)
apps/dashboard/app/(app)/authorization/roles/components/table/components/actions/components/edit-role.tsx (1)

16-23: 🧹 Nitpick (assertive)

Pass loading state down or guard against premature render

keys.map / permissions.map run before data is fetched (they default to [], so safe) but UpsertRoleDialog may still need to know that the data is in-flight to show spinners or disable save. Consider forwarding isLoading from useFetchConnectedKeysAndPermsData.

-  const { permissions, keys, error } = useFetchConnectedKeysAndPermsData(role.roleId);
+  const { permissions, keys, isLoading, error } =
+    useFetchConnectedKeysAndPermsData(role.roleId);

and then:

<UpsertRoleDialog
  /* existing props */
  isLoadingInitial={isLoading}
/>
apps/dashboard/app/(app)/authorization/roles/components/table/hooks/use-roles-list-query.ts (1)

55-72: 🧹 Nitpick (assertive)

Minor: avoid rebuilding Map every fetch for better incremental updates

Current logic replaces the entire rolesMap, forcing downstream consumers to re-render all rows. When paginating this defeats virtualization benefits.

Refactor to mutate incrementally:

-      const newMap = new Map<string, RoleBasic>();
+      const newMap = new Map(rolesMap); // keep existing

       rolesData.pages.forEach((page) => {
         page.roles.forEach((role) => {
           newMap.set(role.roleId, role);
         });
       });

and update state only when newMap.size !== rolesMap.size or entries differ.

apps/dashboard/lib/trpc/routers/index.ts (1)

168-183: 🧹 Nitpick (assertive)

Check router nesting for the new “connected” endpoints*

connectedKeys / connectedPerms sit directly under authorization.roles, while similar data-access routes (keys.query, permissions.query) live inside their own sub-routers.
If this divergence is accidental, consider moving them under:

authorization.roles.keys.connected
authorization.roles.permissions.connected

to keep the public API surface orthogonal and predictable.

No functional issue, but tightening the route hierarchy early avoids future breaking moves.

apps/dashboard/app/(app)/authorization/roles/components/table/roles-list.tsx (1)

104-122: 🧹 Nitpick (assertive)

Minor optimisation – avoid re-creating inline components per render

AssignedItemsCell is recreated for every row render which is unavoidable, but the anonymous function wrapper isn’t.
Extracting it to a useCallback with stable deps removes an extra function allocation per cell:

const renderAssignedKeys = useCallback(
  (r: RoleBasic) => (
    <AssignedItemsCell
      roleId={r.roleId}
      isSelected={r.roleId === selectedRole?.roleId}
      type="keys"
    />
  ),
  [selectedRole?.roleId],
);

Not critical, but helps when the table scales.

apps/dashboard/app/(app)/authorization/roles/components/table/components/assigned-items-cell.tsx (1)

39-44: 🧹 Nitpick (assertive)

Reuse-of-element pattern may confuse React diffing – expose an Icon component instead

icon is stored as a single React element instance and then injected multiple times ({icon}) inside a list.
React treats an element object as immutable – reusing it across siblings may lead to reconciliation quirks and fails strict-mode warnings.

Refactor to hold a component (or factory) rather than an element:

-const icon =
-  type === "keys" ? (
-    <Key2 size="md-regular" />
-  ) : (
-    <HandHoldingKey className="size-3" size="md-regular" />
-  );
+const Icon = type === "keys" ? Key2 : HandHoldingKey;

and consume it with:

<Icon size="md-regular" className="size-3" />

(The skeleton clone can add opacity via className={cn("opacity-50", props.className)}.)

This guarantees each render path receives its own element instance.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 70cc9c2 and 90694c3.

📒 Files selected for processing (22)
  • apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/keys-table-action.popover.tsx (1 hunks)
  • apps/dashboard/app/(app)/authorization/roles/components/table/components/actions/components/delete-role.tsx (2 hunks)
  • apps/dashboard/app/(app)/authorization/roles/components/table/components/actions/components/edit-role.tsx (1 hunks)
  • apps/dashboard/app/(app)/authorization/roles/components/table/components/actions/components/hooks/use-fetch-connected-keys-and-perms.ts (1 hunks)
  • apps/dashboard/app/(app)/authorization/roles/components/table/components/actions/components/role-info.tsx (1 hunks)
  • apps/dashboard/app/(app)/authorization/roles/components/table/components/actions/keys-table-action.popover.constants.tsx (1 hunks)
  • apps/dashboard/app/(app)/authorization/roles/components/table/components/assigned-items-cell.tsx (3 hunks)
  • apps/dashboard/app/(app)/authorization/roles/components/table/hooks/use-role-limits.ts (1 hunks)
  • apps/dashboard/app/(app)/authorization/roles/components/table/hooks/use-roles-list-query.ts (2 hunks)
  • apps/dashboard/app/(app)/authorization/roles/components/table/roles-list.tsx (5 hunks)
  • apps/dashboard/app/(app)/authorization/roles/components/table/utils/get-row-class.ts (2 hunks)
  • apps/dashboard/app/(app)/authorization/roles/components/upsert-role/components/assign-key/key-field.tsx (3 hunks)
  • apps/dashboard/app/(app)/authorization/roles/components/upsert-role/components/assign-permission/permissions-field.tsx (5 hunks)
  • apps/dashboard/app/(app)/authorization/roles/components/upsert-role/components/warning-callout.tsx (1 hunks)
  • apps/dashboard/app/(app)/authorization/roles/components/upsert-role/hooks/use-upsert-role.ts (1 hunks)
  • apps/dashboard/app/(app)/authorization/roles/components/upsert-role/index.tsx (6 hunks)
  • apps/dashboard/app/(app)/authorization/roles/components/upsert-role/upsert-role.schema.ts (1 hunks)
  • apps/dashboard/lib/trpc/routers/authorization/roles/keys/connected-keys.ts (1 hunks)
  • apps/dashboard/lib/trpc/routers/authorization/roles/permissions/connected-permissions.ts (1 hunks)
  • apps/dashboard/lib/trpc/routers/authorization/roles/query.ts (3 hunks)
  • apps/dashboard/lib/trpc/routers/authorization/roles/upsert.ts (4 hunks)
  • apps/dashboard/lib/trpc/routers/index.ts (2 hunks)
🧰 Additional context used
🧠 Learnings (1)
apps/dashboard/app/(app)/authorization/roles/components/upsert-role/hooks/use-upsert-role.ts (1)
Learnt from: AkshayBandi027
PR: unkeyed/unkey#2215
File: apps/dashboard/app/(app)/@breadcrumb/authorization/roles/[roleId]/page.tsx:28-29
Timestamp: 2024-10-08T15:33:04.290Z
Learning: In `authorization/roles/[roleId]/update-role.tsx`, the tag `role-${role.id}` is revalidated after updating a role to ensure that the caching mechanism is properly handled for roles.
🧬 Code Graph Analysis (15)
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/keys-table-action.popover.tsx (2)
apps/dashboard/lib/utils.ts (1)
  • cn (5-7)
internal/icons/src/icons/dots.tsx (1)
  • Dots (15-60)
apps/dashboard/app/(app)/authorization/roles/components/table/components/actions/components/role-info.tsx (1)
apps/dashboard/lib/trpc/routers/authorization/roles/query.ts (1)
  • RoleBasic (16-16)
apps/dashboard/app/(app)/authorization/roles/components/table/components/actions/components/delete-role.tsx (2)
apps/dashboard/lib/trpc/routers/authorization/roles/query.ts (1)
  • RoleBasic (16-16)
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/keys-table-action.popover.tsx (1)
  • ActionComponentProps (6-9)
apps/dashboard/app/(app)/authorization/roles/components/table/components/actions/components/edit-role.tsx (2)
apps/dashboard/lib/trpc/routers/authorization/roles/query.ts (1)
  • RoleBasic (16-16)
apps/dashboard/app/(app)/authorization/roles/components/table/components/actions/components/hooks/use-fetch-connected-keys-and-perms.ts (1)
  • useFetchConnectedKeysAndPermsData (4-24)
apps/dashboard/app/(app)/authorization/roles/components/table/utils/get-row-class.ts (1)
apps/dashboard/lib/trpc/routers/authorization/roles/query.ts (1)
  • RoleBasic (16-16)
apps/dashboard/app/(app)/authorization/roles/components/table/roles-list.tsx (2)
apps/dashboard/lib/trpc/routers/authorization/roles/query.ts (1)
  • RoleBasic (16-16)
apps/dashboard/components/virtual-table/types.ts (1)
  • Column (13-24)
apps/dashboard/app/(app)/authorization/roles/components/upsert-role/components/warning-callout.tsx (1)
internal/ui/src/components/inline-link.tsx (1)
  • InlineLink (32-56)
apps/dashboard/lib/trpc/routers/index.ts (2)
apps/dashboard/lib/trpc/routers/authorization/roles/keys/connected-keys.ts (1)
  • queryRoleKeys (10-61)
apps/dashboard/lib/trpc/routers/authorization/roles/permissions/connected-permissions.ts (1)
  • queryRolePermissions (10-58)
apps/dashboard/lib/trpc/routers/authorization/roles/query.ts (1)
apps/dashboard/lib/db.ts (1)
  • db (5-26)
apps/dashboard/app/(app)/authorization/roles/components/table/components/assigned-items-cell.tsx (2)
apps/dashboard/lib/trpc/server.ts (1)
  • trpc (7-14)
apps/dashboard/lib/utils.ts (1)
  • cn (5-7)
apps/dashboard/app/(app)/authorization/roles/components/table/hooks/use-role-limits.ts (1)
apps/dashboard/lib/trpc/server.ts (1)
  • trpc (7-14)
apps/dashboard/lib/trpc/routers/authorization/roles/keys/connected-keys.ts (2)
apps/dashboard/lib/trpc/trpc.ts (4)
  • t (8-8)
  • requireUser (10-21)
  • requireWorkspace (23-36)
  • withRatelimit (113-128)
apps/dashboard/lib/db.ts (1)
  • db (5-26)
apps/dashboard/lib/trpc/routers/authorization/roles/upsert.ts (1)
apps/dashboard/lib/audit.ts (1)
  • insertAuditLogs (99-174)
apps/dashboard/app/(app)/authorization/roles/components/table/hooks/use-roles-list-query.ts (1)
apps/dashboard/lib/trpc/routers/authorization/roles/query.ts (1)
  • RoleBasic (16-16)
apps/dashboard/app/(app)/authorization/roles/components/table/components/actions/keys-table-action.popover.constants.tsx (3)
apps/dashboard/lib/trpc/routers/authorization/roles/query.ts (1)
  • RoleBasic (16-16)
apps/dashboard/app/(app)/authorization/roles/components/table/hooks/use-role-limits.ts (1)
  • useRoleLimits (18-94)
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/keys-table-action.popover.tsx (1)
  • MenuItem (11-21)
⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: Test Packages / Test ./internal/clickhouse
🔇 Additional comments (12)
apps/dashboard/app/(app)/authorization/roles/components/table/components/actions/components/role-info.tsx (1)

1-25: Type update looks correct

RoleBasic replacement is consistent with the new query contract; component logic unchanged and safe-guarded with fallbacks. No further issues spotted.

apps/dashboard/app/(app)/authorization/roles/components/table/components/actions/components/delete-role.tsx (1)

67-73: roleIds param type might expect array – verify

useDeleteRole.mutateAsync is called with roleIds: roleDetails.roleId (a string). If the backend expects string[], this will 400. Double-check the zod schema.

-      await deleteRole.mutateAsync({
-        roleIds: roleDetails.roleId,
-      });
+      await deleteRole.mutateAsync({
+        roleIds: [roleDetails.roleId],
+      });
apps/dashboard/app/(app)/authorization/roles/components/upsert-role/index.tsx (3)

67-68: Good integration of useRoleLimits

Pulling calculateLimits up to the dialog level keeps validation logic centralised.
No action needed.


132-142: Verify backend accepts undefined for omitted arrays

When warnings are present you send

keyIds: undefined,
permissionIds: undefined,

The schema marks these fields as optional, so the object is type-safe, but make sure the tRPC mutation treats undefined as “do not update” and not “clear the list”.
Please confirm or guard in the router.


228-234: Passing roleId down keeps child components stateless – nice

Handing the ID to KeyField / PermissionField lets them self-determine limits.

apps/dashboard/app/(app)/authorization/roles/components/table/components/actions/keys-table-action.popover.constants.tsx (1)

31-39: Prefetch function must return a promise

KeysTableActionPopover awaits prefetch to control hover-prefetch timing.
prefetchIfAllowed already returns Promise<void>, so 👍.

apps/dashboard/app/(app)/authorization/roles/components/table/components/actions/components/hooks/use-fetch-connected-keys-and-perms.ts (1)

4-14: Conditional query enabling looks solid

Gating the TRPC query with shouldPrefetch && roleId prevents wasted network calls for large roles – nice optimisation.

apps/dashboard/app/(app)/authorization/roles/components/upsert-role/upsert-role.schema.ts (1)

21-32: Changing defaults removes guaranteed array shape

keyIds / permissionIds are now optional.
Down-stream code that assumes an array (e.g., data.keyIds.map) must handle undefined. Most callers initialise with [], but double-check any direct usages outside the dialog.

apps/dashboard/lib/trpc/routers/authorization/roles/keys/connected-keys.ts (1)

16-18: Unbounded limit can hammer the db

z.number().default(3) allows callers to pass any positive number.
Add an upper bound (e.g. .min(1).max(100)) or enforce it server-side to stop a single request from returning an unpaginated key list.

-roleId: z.string(),
-limit: z.number().default(3),
+roleId: z.string(),
+limit: z.number().min(1).max(100).default(3),
apps/dashboard/lib/trpc/routers/authorization/roles/permissions/connected-permissions.ts (1)

16-18: Same limit hard-cap recommendation

Mirror the safeguard applied to the connected-keys endpoint to keep behaviour consistent and protect the database.

-roleId: z.string(),
-limit: z.number().default(3),
+roleId: z.string(),
+limit: z.number().min(1).max(100).default(3),
apps/dashboard/lib/trpc/routers/authorization/roles/query.ts (1)

87-91: Empty description collapsed to empty string – loses “unset” semantics

Converting NULL to "" prevents clients from distinguishing “no description provided” vs “description is an empty string”.
Unless there’s a strict UI requirement, return null instead:

-description: row.description || "",
+description: row.description ?? null,
apps/dashboard/app/(app)/authorization/roles/components/upsert-role/components/assign-permission/permissions-field.tsx (1)

82-84: Verify that permission.roles is always hydrated

Filtering out already-assigned permissions relies on permission.roles?.some(...).
If roles is omitted from the payload returned by useFetchPermissions()/useSearchPermissions, the check will silently pass and allow duplicates to be picked.

Please confirm the permissions endpoints include roles when roleId is supplied, or guard with an explicit comment explaining why duplicates are impossible.

@ogzhanolguncu ogzhanolguncu mentioned this pull request Jun 18, 2025
18 tasks
@chronark chronark self-assigned this Jun 19, 2025
Copy link
Collaborator

@chronark chronark left a comment

Choose a reason for hiding this comment

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

just some small stuff

one thing to consider is the layout shift though. I didn't realise it until I saw it, but because we show 3 keys + "more", the row may expand when transitioning from loading to data state. same for permissions I suppose
https://share.cleanshot.com/vcVx354W

Copy link
Contributor Author

I added some animation for it probably its causing that. I'll either fix it or get rid of it 😄

@vercel vercel bot temporarily deployed to Preview – engineering June 19, 2025 08:14 Inactive
@vercel vercel bot temporarily deployed to Preview – dashboard June 19, 2025 08:16 Inactive
Copy link
Contributor Author

one thing to consider is the layout shift though. I didn't realise it until I saw it, but because we show 3 keys + "more", the row may expand when transitioning from loading to data state. same for permissions I suppose
btw I just got it. you mean we add a proper transition from loading state to multiple items state to improve UX

Copy link
Contributor Author

🫡

Copy link
Collaborator

well or we just don’t display multiple rows of keys/permissions inside a role-row 😅

Copy link
Contributor Author

I mean you might be right seeing 3 role then .... more doesn't bring any benefit to user atm. When I first working on it it felt good, but now I'm confused too

@vercel vercel bot temporarily deployed to Preview – dashboard July 3, 2025 11:34 Inactive
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

♻️ Duplicate comments (11)
apps/dashboard/app/(app)/authorization/roles/components/table/components/actions/keys-table-action.popover.constants.tsx (1)

66-75: Clipboard API capability check still missing (already flagged earlier)

The direct navigator.clipboard.writeText call can throw in non-HTTPS or unsupported contexts. A guard / fallback (or try-catch around a capability check) is still advisable.

This is the same concern raised in a previous review; please resolve or explicitly dismiss.

apps/dashboard/lib/trpc/routers/index.ts (1)

223-224: LGTM: New endpoints enable granular role data fetching.

The connectedKeys and connectedPerms endpoints provide the separate access paths needed for the performance optimization described in the PR objectives, allowing the UI to fetch key and permission counts independently.

apps/dashboard/app/(app)/authorization/roles/components/upsert-role/hooks/use-upsert-role.ts (1)

14-27: LGTM: Comprehensive async cache invalidation addresses past feedback.

The implementation properly uses Promise.all with async/await to ensure all cache invalidations complete before proceeding, addressing the race condition concerns from the previous review. The comprehensive invalidation strategy covering roles, permissions, and the new connected endpoints ensures data consistency.

apps/dashboard/app/(app)/authorization/roles/components/upsert-role/components/warning-callout.tsx (1)

17-17: Add aria-hidden to decorative icon for accessibility

The TriangleWarning icon is purely decorative and should include aria-hidden="true" to prevent screen readers from announcing it unnecessarily.

apps/dashboard/app/(app)/authorization/roles/components/upsert-role/components/assign-key/key-field.tsx (2)

31-32: Consider memoizing the calculateLimits call

The calculateLimits(value) call is executed on every render, which could cause unnecessary computations as the hook logic grows more complex.


159-159: Consider UX impact of disabling the entire combobox

Disabling the whole combobox when hasKeyWarning is true prevents users from searching for keys, not just adding new ones. This could impact user experience.

apps/dashboard/app/(app)/authorization/roles/components/table/hooks/use-role-limits.ts (2)

47-49: Use nullish coalescing to handle zero values correctly

The logical OR operators (||) incorrectly treat 0 as falsy, causing fallback to other values when the count is exactly zero.


71-83: Potential premature prefetch before data availability

The prefetchIfAllowed function calls calculateLimits() before data may be available, potentially triggering unnecessary prefetching.

apps/dashboard/app/(app)/authorization/roles/components/table/components/assigned-items-cell.tsx (3)

14-29: Two queries are instantiated on every render – consider a single conditional query instead

Both queries are created each render with only the enabled flag controlling execution. This creates unnecessary React Query cache entries.

Consider using a single conditional query:

-const { data: keysData, isLoading: keysLoading } =
-  trpc.authorization.roles.connectedKeys.useQuery(
-    { roleId },
-    {
-      enabled: kind === "keys",
-      staleTime: 5 * 60 * 1000,
-    },
-  );
-const { data: permissionsData, isLoading: permissionsLoading } =
-  trpc.authorization.roles.connectedPerms.useQuery(
-    { roleId },
-    {
-      enabled: kind === "permissions",
-      staleTime: 5 * 60 * 1000,
-    },
-  );
+const queryFn = kind === "keys"
+  ? trpc.authorization.roles.connectedKeys.useQuery
+  : trpc.authorization.roles.connectedPerms.useQuery;
+
+const { data, isLoading } = queryFn(
+  { roleId },
+  { staleTime: 5 * 60 * 1000 },
+);

31-33: Consider adding error handling for failed queries.

The component handles loading and success states but doesn't account for query errors.

Add error state handling:

+const { data: keysData, isLoading: keysLoading, error: keysError } =
   trpc.authorization.roles.connectedKeys.useQuery(
     { roleId },
     {
       enabled: kind === "keys",
       staleTime: 5 * 60 * 1000,
     },
   );
+const { data: permissionsData, isLoading: permissionsLoading, error: permissionsError } =
   trpc.authorization.roles.connectedPerms.useQuery(
     { roleId },
     {
       enabled: kind === "permissions",
       staleTime: 5 * 60 * 1000,
     },
   );

 const data = kind === "keys" ? keysData : permissionsData;
 const isLoading = kind === "keys" ? keysLoading : permissionsLoading;
+const error = kind === "keys" ? keysError : permissionsError;
 const totalCount = data?.totalCount;

+if (error) {
+  return (
+    <div className="flex flex-col gap-1 py-2 max-w-[200px]">
+      <div className="text-red-9 text-xs">Failed to load</div>
+    </div>
+  );
+}

75-84: Animations enhance UX but consider layout stability.

The entrance animations could cause layout shifts when transitioning from loading to loaded states.

Consider using consistent heights to minimize layout shifts:

-<div className="flex flex-col gap-1 py-2 max-w-[200px] animate-in fade-in slide-in-from-top-2 duration-300">
+<div className="flex flex-col gap-1 py-2 max-w-[200px] min-h-[38px] animate-in fade-in slide-in-from-top-2 duration-300">
📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 84be909 and e0cd6cc.

📒 Files selected for processing (15)
  • apps/dashboard/app/(app)/authorization/permissions/components/table/components/assigned-items-cell.tsx (1 hunks)
  • apps/dashboard/app/(app)/authorization/permissions/components/table/components/skeletons.tsx (2 hunks)
  • apps/dashboard/app/(app)/authorization/permissions/components/table/permissions-list.tsx (5 hunks)
  • apps/dashboard/app/(app)/authorization/permissions/components/upsert-permission/hooks/use-upsert-permission.ts (1 hunks)
  • apps/dashboard/app/(app)/authorization/roles/components/table/components/actions/components/delete-role.tsx (2 hunks)
  • apps/dashboard/app/(app)/authorization/roles/components/table/components/actions/keys-table-action.popover.constants.tsx (3 hunks)
  • apps/dashboard/app/(app)/authorization/roles/components/table/components/assigned-items-cell.tsx (2 hunks)
  • apps/dashboard/app/(app)/authorization/roles/components/table/components/skeletons.tsx (1 hunks)
  • apps/dashboard/app/(app)/authorization/roles/components/table/hooks/use-role-limits.ts (1 hunks)
  • apps/dashboard/app/(app)/authorization/roles/components/upsert-role/components/assign-key/key-field.tsx (4 hunks)
  • apps/dashboard/app/(app)/authorization/roles/components/upsert-role/components/assign-permission/permissions-field.tsx (5 hunks)
  • apps/dashboard/app/(app)/authorization/roles/components/upsert-role/components/warning-callout.tsx (1 hunks)
  • apps/dashboard/app/(app)/authorization/roles/components/upsert-role/hooks/use-upsert-role.ts (1 hunks)
  • apps/dashboard/lib/trpc/routers/authorization/permissions/query.ts (2 hunks)
  • apps/dashboard/lib/trpc/routers/index.ts (2 hunks)
🧰 Additional context used
🧠 Learnings (16)
📓 Common learnings
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#2825
File: apps/dashboard/app/(app)/logs-v2/hooks/use-bookmarked-filters.ts:0-0
Timestamp: 2025-01-30T20:51:44.359Z
Learning: The user (ogzhanolguncu) prefers to handle refactoring suggestions in separate PRs to maintain focus in the current PR.
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3297
File: apps/dashboard/lib/trpc/routers/authorization/roles/query.ts:210-323
Timestamp: 2025-06-04T20:13:12.060Z
Learning: The user ogzhanolguncu prefers explicit, duplicated code over abstracted helper functions when it improves readability, even if it means some duplication in filter building functions in the authorization roles query module.
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#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.
Learnt from: AkshayBandi027
PR: unkeyed/unkey#2215
File: apps/dashboard/app/(app)/@breadcrumb/authorization/roles/[roleId]/page.tsx:28-29
Timestamp: 2024-10-08T15:33:04.290Z
Learning: In `authorization/roles/[roleId]/update-role.tsx`, the tag `role-${role.id}` is revalidated after updating a role to ensure that the caching mechanism is properly handled for roles.
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3375
File: apps/dashboard/app/(app)/settings/root-keys/components/table/hooks/use-root-keys-list-query.ts:0-0
Timestamp: 2025-06-25T20:32:10.471Z
Learning: In the Unkey codebase, ogzhanolguncu prefers strict validation with fail-fast error handling. When validation errors occur that shouldn't happen in normal operation (like invalid operators), throwing errors to crash the page is preferred over graceful error handling or console logging.
apps/dashboard/lib/trpc/routers/index.ts (8)
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#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.
Learnt from: AkshayBandi027
PR: unkeyed/unkey#2215
File: apps/dashboard/app/(app)/@breadcrumb/authorization/roles/[roleId]/page.tsx:28-29
Timestamp: 2024-10-08T15:33:04.290Z
Learning: In `authorization/roles/[roleId]/update-role.tsx`, the tag `role-${role.id}` is revalidated after updating a role to ensure that the caching mechanism is properly handled for roles.
Learnt from: chronark
PR: unkeyed/unkey#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.
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3292
File: apps/dashboard/lib/vault.ts:80-97
Timestamp: 2025-06-02T11:08:56.397Z
Learning: The vault.ts file in apps/dashboard/lib/vault.ts is a duplicate of the vault package from the `api` directory and should be kept consistent with that original implementation.
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3292
File: apps/dashboard/lib/vault.ts:60-78
Timestamp: 2025-06-02T11:09:05.843Z
Learning: The vault implementation in `apps/dashboard/lib/vault.ts` is a duplicate of the vault package from `api` and should be kept consistent with the original implementation.
Learnt from: mcstepp
PR: unkeyed/unkey#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.
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#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.
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#2825
File: apps/dashboard/app/(app)/logs-v2/hooks/use-bookmarked-filters.ts:0-0
Timestamp: 2025-01-30T20:51:44.359Z
Learning: The user (ogzhanolguncu) prefers to handle refactoring suggestions in separate PRs to maintain focus in the current PR.
apps/dashboard/app/(app)/authorization/permissions/components/upsert-permission/hooks/use-upsert-permission.ts (2)
Learnt from: AkshayBandi027
PR: unkeyed/unkey#2215
File: apps/dashboard/app/(app)/@breadcrumb/authorization/roles/[roleId]/page.tsx:28-29
Timestamp: 2024-10-08T15:33:04.290Z
Learning: In `authorization/roles/[roleId]/update-role.tsx`, the tag `role-${role.id}` is revalidated after updating a role to ensure that the caching mechanism is properly handled for roles.
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#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.
apps/dashboard/app/(app)/authorization/roles/components/table/components/actions/components/delete-role.tsx (4)
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#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.
Learnt from: AkshayBandi027
PR: unkeyed/unkey#2215
File: apps/dashboard/app/(app)/@breadcrumb/authorization/roles/[roleId]/page.tsx:28-29
Timestamp: 2024-10-08T15:33:04.290Z
Learning: In `authorization/roles/[roleId]/update-role.tsx`, the tag `role-${role.id}` is revalidated after updating a role to ensure that the caching mechanism is properly handled for roles.
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#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.
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3115
File: apps/dashboard/components/logs/checkbox/filters-popover.tsx:33-55
Timestamp: 2025-04-24T14:34:30.621Z
Learning: In the ShortcutActivator component within filters-popover.tsx, the purpose is to track keys separately for each filter item, providing a registration mechanism for shortcuts passed to it rather than enforcing specific key combinations like option+shift+key.
apps/dashboard/app/(app)/authorization/roles/components/table/components/actions/keys-table-action.popover.constants.tsx (12)
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#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.
Learnt from: AkshayBandi027
PR: unkeyed/unkey#2215
File: apps/dashboard/app/(app)/@breadcrumb/authorization/roles/[roleId]/page.tsx:28-29
Timestamp: 2024-10-08T15:33:04.290Z
Learning: In `authorization/roles/[roleId]/update-role.tsx`, the tag `role-${role.id}` is revalidated after updating a role to ensure that the caching mechanism is properly handled for roles.
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#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.
Learnt from: chronark
PR: unkeyed/unkey#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.
Learnt from: chronark
PR: unkeyed/unkey#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.
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3401
File: apps/dashboard/app/(app)/logs/filters.query-params.ts:10-0
Timestamp: 2025-06-24T13:29:10.129Z
Learning: The `buildQueryParams` function in `apps/dashboard/app/(app)/logs/filters.query-params.ts` calls `useFilters()` hook inside it, but this is valid because the function is only called from within other React hooks, maintaining the Rules of Hooks compliance.
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#2707
File: apps/dashboard/lib/trpc/routers/ratelimit/createOverride.ts:63-63
Timestamp: 2024-12-05T13:27:55.555Z
Learning: In `apps/dashboard/lib/trpc/routers/ratelimit/createOverride.ts`, when determining the maximum number of rate limit overrides (`max`), the intentional use of `const max = hasWorkspaceAccess("ratelimitOverrides", namespace.workspace) || 5;` allows `max` to fall back to `5` when `hasWorkspaceAccess` returns `0` or `false`. This fallback behavior is expected and intended in the codebase.
Learnt from: unrenamed
PR: unkeyed/unkey#2652
File: apps/dashboard/components/dashboard/copy-button.tsx:38-38
Timestamp: 2024-11-08T11:40:17.737Z
Learning: The `copyToClipboardWithMeta` function currently has an unused `_meta` parameter. Consider removing it or utilizing it appropriately, especially when introducing the `useCopyToClipboard` hook.
Learnt from: mcstepp
PR: unkeyed/unkey#3254
File: apps/dashboard/components/logs/hooks/use-bookmarked-filters.ts:183-195
Timestamp: 2025-05-13T14:38:44.134Z
Learning: When working with browser storage (like localStorage) in this codebase, always implement try/catch blocks to handle Safari quirks and other browser-specific issues that users have been experiencing. Safari Private Browsing mode in particular can throw errors when accessing localStorage.
Learnt from: p6l-richard
PR: unkeyed/unkey#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.
Learnt from: p6l-richard
PR: unkeyed/unkey#2085
File: apps/www/components/glossary/terms-rolodex-desktop.tsx:39-50
Timestamp: 2024-10-23T16:19:59.657Z
Learning: Debouncing of the scroll handler is not needed yet in the function `TermsRolodexDesktop` in `apps/www/components/glossary/terms-rolodex-desktop.tsx`.
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3115
File: apps/dashboard/components/logs/checkbox/filters-popover.tsx:33-55
Timestamp: 2025-04-24T14:34:30.621Z
Learning: In the ShortcutActivator component within filters-popover.tsx, the purpose is to track keys separately for each filter item, providing a registration mechanism for shortcuts passed to it rather than enforcing specific key combinations like option+shift+key.
apps/dashboard/app/(app)/authorization/roles/components/upsert-role/hooks/use-upsert-role.ts (3)
Learnt from: AkshayBandi027
PR: unkeyed/unkey#2215
File: apps/dashboard/app/(app)/@breadcrumb/authorization/roles/[roleId]/page.tsx:28-29
Timestamp: 2024-10-08T15:33:04.290Z
Learning: In `authorization/roles/[roleId]/update-role.tsx`, the tag `role-${role.id}` is revalidated after updating a role to ensure that the caching mechanism is properly handled for roles.
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#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.
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3380
File: apps/dashboard/app/(app)/ratelimits/[namespaceId]/namespace-navbar.tsx:40-66
Timestamp: 2025-06-19T10:39:29.388Z
Learning: tRPC has built-in caching that prevents skeleton flashing during component re-renders and navigation, so concerns about `!data || isLoading` conditions causing loading state flashes are generally not needed.
apps/dashboard/app/(app)/authorization/roles/components/table/components/skeletons.tsx (1)
Learnt from: AkshayBandi027
PR: unkeyed/unkey#2215
File: apps/dashboard/app/(app)/@breadcrumb/authorization/roles/[roleId]/page.tsx:28-29
Timestamp: 2024-10-08T15:33:04.290Z
Learning: In `authorization/roles/[roleId]/update-role.tsx`, the tag `role-${role.id}` is revalidated after updating a role to ensure that the caching mechanism is properly handled for roles.
apps/dashboard/lib/trpc/routers/authorization/permissions/query.ts (5)
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#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.
Learnt from: AkshayBandi027
PR: unkeyed/unkey#2215
File: apps/dashboard/app/(app)/@breadcrumb/authorization/roles/[roleId]/page.tsx:28-29
Timestamp: 2024-10-08T15:33:04.290Z
Learning: In `authorization/roles/[roleId]/update-role.tsx`, the tag `role-${role.id}` is revalidated after updating a role to ensure that the caching mechanism is properly handled for roles.
Learnt from: chronark
PR: unkeyed/unkey#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.
Learnt from: chronark
PR: unkeyed/unkey#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.
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#2707
File: apps/dashboard/lib/trpc/routers/ratelimit/createOverride.ts:63-63
Timestamp: 2024-12-05T13:27:55.555Z
Learning: In `apps/dashboard/lib/trpc/routers/ratelimit/createOverride.ts`, when determining the maximum number of rate limit overrides (`max`), the intentional use of `const max = hasWorkspaceAccess("ratelimitOverrides", namespace.workspace) || 5;` allows `max` to fall back to `5` when `hasWorkspaceAccess` returns `0` or `false`. This fallback behavior is expected and intended in the codebase.
apps/dashboard/app/(app)/authorization/roles/components/upsert-role/components/warning-callout.tsx (6)
Learnt from: AkshayBandi027
PR: unkeyed/unkey#2215
File: apps/dashboard/app/(app)/@breadcrumb/authorization/roles/[roleId]/page.tsx:28-29
Timestamp: 2024-10-08T15:33:04.290Z
Learning: In `authorization/roles/[roleId]/update-role.tsx`, the tag `role-${role.id}` is revalidated after updating a role to ensure that the caching mechanism is properly handled for roles.
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#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.
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#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.
Learnt from: p6l-richard
PR: unkeyed/unkey#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.
Learnt from: MichaelUnkey
PR: unkeyed/unkey#2697
File: apps/engineering/content/design/components/id.mdx:34-40
Timestamp: 2024-12-04T17:20:37.426Z
Learning: In component documentation files (e.g., `.mdx` files in `apps/engineering/content/design/components/`), Michael prefers to include a `Props` section but prefers to exclude `Accessibility` and `Best Practices` sections.
Learnt from: mcstepp
PR: unkeyed/unkey#3258
File: apps/dashboard/app/(app)/apis/[apiId]/settings/components/update-ip-whitelist.tsx:87-88
Timestamp: 2025-05-16T16:07:23.395Z
Learning: The `target` prop on `InlineLink` components should be used with a string value like "_blank" rather than as a boolean prop (`target` or `target={true}`).
apps/dashboard/app/(app)/authorization/roles/components/table/hooks/use-role-limits.ts (7)
Learnt from: AkshayBandi027
PR: unkeyed/unkey#2215
File: apps/dashboard/app/(app)/@breadcrumb/authorization/roles/[roleId]/page.tsx:28-29
Timestamp: 2024-10-08T15:33:04.290Z
Learning: In `authorization/roles/[roleId]/update-role.tsx`, the tag `role-${role.id}` is revalidated after updating a role to ensure that the caching mechanism is properly handled for roles.
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#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.
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#2707
File: apps/dashboard/lib/trpc/routers/ratelimit/createOverride.ts:63-63
Timestamp: 2024-12-05T13:27:55.555Z
Learning: In `apps/dashboard/lib/trpc/routers/ratelimit/createOverride.ts`, when determining the maximum number of rate limit overrides (`max`), the intentional use of `const max = hasWorkspaceAccess("ratelimitOverrides", namespace.workspace) || 5;` allows `max` to fall back to `5` when `hasWorkspaceAccess` returns `0` or `false`. This fallback behavior is expected and intended in the codebase.
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3401
File: apps/dashboard/app/(app)/logs/filters.query-params.ts:10-0
Timestamp: 2025-06-24T13:29:10.129Z
Learning: The `buildQueryParams` function in `apps/dashboard/app/(app)/logs/filters.query-params.ts` calls `useFilters()` hook inside it, but this is valid because the function is only called from within other React hooks, maintaining the Rules of Hooks compliance.
Learnt from: chronark
PR: unkeyed/unkey#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.
Learnt from: mcstepp
PR: unkeyed/unkey#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.
Learnt from: chronark
PR: unkeyed/unkey#2143
File: apps/dashboard/app/(app)/logs/components/log-details/components/log-footer.tsx:58-61
Timestamp: 2024-12-03T14:21:19.543Z
Learning: For the "Outcome" field in the `LogFooter` component (`apps/dashboard/app/(app)/logs/components/log-details/components/log-footer.tsx`), default to "N/A" instead of "VALID" when handling null values to avoid confusing customers.
apps/dashboard/app/(app)/authorization/permissions/components/table/permissions-list.tsx (4)
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#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.
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#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.
Learnt from: p6l-richard
PR: unkeyed/unkey#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.
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#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.
apps/dashboard/app/(app)/authorization/roles/components/upsert-role/components/assign-permission/permissions-field.tsx (13)
Learnt from: AkshayBandi027
PR: unkeyed/unkey#2215
File: apps/dashboard/app/(app)/@breadcrumb/authorization/roles/[roleId]/page.tsx:28-29
Timestamp: 2024-10-08T15:33:04.290Z
Learning: In `authorization/roles/[roleId]/update-role.tsx`, the tag `role-${role.id}` is revalidated after updating a role to ensure that the caching mechanism is properly handled for roles.
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#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.
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#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.
Learnt from: MichaelUnkey
PR: unkeyed/unkey#2810
File: internal/ui/src/components/date-time/components/time-split.tsx:10-14
Timestamp: 2025-01-22T16:51:59.978Z
Learning: The DateTime component in internal/ui/src/components/date-time/components/time-split.tsx already includes sufficient validation through handleChange and handleBlur functions, making additional runtime validation unnecessary.
Learnt from: chronark
PR: unkeyed/unkey#2143
File: apps/dashboard/app/(app)/logs/components/log-details/components/log-footer.tsx:58-61
Timestamp: 2024-12-03T14:21:19.543Z
Learning: For the "Outcome" field in the `LogFooter` component (`apps/dashboard/app/(app)/logs/components/log-details/components/log-footer.tsx`), default to "N/A" instead of "VALID" when handling null values to avoid confusing customers.
Learnt from: p6l-richard
PR: unkeyed/unkey#2085
File: apps/www/components/glossary/terms-rolodex-desktop.tsx:26-37
Timestamp: 2024-10-23T16:20:19.324Z
Learning: When reviewing React components in this project, avoid suggesting manual memoization with `useMemo` for performance optimizations, as the team prefers to rely on the React compiler to handle such optimizations.
Learnt from: unrenamed
PR: unkeyed/unkey#2652
File: apps/www/components/particles.tsx:88-90
Timestamp: 2024-11-08T11:44:42.947Z
Learning: In React TypeScript components, if a function memoized with `useCallback` is not called elsewhere in the component, and a `useEffect` is used to invoke it, do not suggest removing the `useEffect` as it is necessary to execute the function.
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3401
File: apps/dashboard/app/(app)/logs/filters.query-params.ts:10-0
Timestamp: 2025-06-24T13:29:10.129Z
Learning: The `buildQueryParams` function in `apps/dashboard/app/(app)/logs/filters.query-params.ts` calls `useFilters()` hook inside it, but this is valid because the function is only called from within other React hooks, maintaining the Rules of Hooks compliance.
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#2143
File: apps/dashboard/app/(app)/logs/components/log-details/resizable-panel.tsx:37-49
Timestamp: 2024-12-03T14:23:07.189Z
Learning: In `apps/dashboard/app/(app)/logs/components/log-details/resizable-panel.tsx`, the resize handler is already debounced.
Learnt from: p6l-richard
PR: unkeyed/unkey#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.
Learnt from: p6l-richard
PR: unkeyed/unkey#2085
File: apps/www/components/glossary/terms-rolodex-desktop.tsx:39-50
Timestamp: 2024-10-23T16:19:59.657Z
Learning: Debouncing of the scroll handler is not needed yet in the function `TermsRolodexDesktop` in `apps/www/components/glossary/terms-rolodex-desktop.tsx`.
Learnt from: unrenamed
PR: unkeyed/unkey#2652
File: apps/dashboard/components/dashboard/copy-button.tsx:38-38
Timestamp: 2024-11-08T11:40:17.737Z
Learning: The `copyToClipboardWithMeta` function currently has an unused `_meta` parameter. Consider removing it or utilizing it appropriately, especially when introducing the `useCopyToClipboard` hook.
Learnt from: unrenamed
PR: unkeyed/unkey#2660
File: apps/play/app/page-bk.tsx:24-24
Timestamp: 2024-11-13T14:58:01.321Z
Learning: In React, refs created with useRef do not change between renders, so including them in the dependency array of useEffect hooks is unnecessary.
apps/dashboard/app/(app)/authorization/roles/components/upsert-role/components/assign-key/key-field.tsx (19)
Learnt from: AkshayBandi027
PR: unkeyed/unkey#2215
File: apps/dashboard/app/(app)/@breadcrumb/authorization/roles/[roleId]/page.tsx:28-29
Timestamp: 2024-10-08T15:33:04.290Z
Learning: In `authorization/roles/[roleId]/update-role.tsx`, the tag `role-${role.id}` is revalidated after updating a role to ensure that the caching mechanism is properly handled for roles.
Learnt from: chronark
PR: unkeyed/unkey#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.
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#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.
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#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.
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3115
File: apps/dashboard/components/logs/checkbox/filters-popover.tsx:33-55
Timestamp: 2025-04-24T14:34:30.621Z
Learning: In the ShortcutActivator component within filters-popover.tsx, the purpose is to track keys separately for each filter item, providing a registration mechanism for shortcuts passed to it rather than enforcing specific key combinations like option+shift+key.
Learnt from: p6l-richard
PR: unkeyed/unkey#2085
File: apps/www/components/glossary/terms-rolodex-desktop.tsx:26-37
Timestamp: 2024-10-23T16:20:19.324Z
Learning: When reviewing React components in this project, avoid suggesting manual memoization with `useMemo` for performance optimizations, as the team prefers to rely on the React compiler to handle such optimizations.
Learnt from: unrenamed
PR: unkeyed/unkey#2652
File: apps/www/components/particles.tsx:88-90
Timestamp: 2024-11-08T11:44:42.947Z
Learning: In React TypeScript components, if a function memoized with `useCallback` is not called elsewhere in the component, and a `useEffect` is used to invoke it, do not suggest removing the `useEffect` as it is necessary to execute the function.
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3401
File: apps/dashboard/app/(app)/logs/filters.query-params.ts:10-0
Timestamp: 2025-06-24T13:29:10.129Z
Learning: The `buildQueryParams` function in `apps/dashboard/app/(app)/logs/filters.query-params.ts` calls `useFilters()` hook inside it, but this is valid because the function is only called from within other React hooks, maintaining the Rules of Hooks compliance.
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#2143
File: apps/dashboard/app/(app)/logs/components/log-details/resizable-panel.tsx:37-49
Timestamp: 2024-12-03T14:23:07.189Z
Learning: In `apps/dashboard/app/(app)/logs/components/log-details/resizable-panel.tsx`, the resize handler is already debounced.
Learnt from: p6l-richard
PR: unkeyed/unkey#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.
Learnt from: p6l-richard
PR: unkeyed/unkey#2085
File: apps/www/components/glossary/terms-rolodex-desktop.tsx:39-50
Timestamp: 2024-10-23T16:19:59.657Z
Learning: Debouncing of the scroll handler is not needed yet in the function `TermsRolodexDesktop` in `apps/www/components/glossary/terms-rolodex-desktop.tsx`.
Learnt from: MichaelUnkey
PR: unkeyed/unkey#2810
File: internal/ui/src/components/date-time/components/time-split.tsx:10-14
Timestamp: 2025-01-22T16:51:59.978Z
Learning: The DateTime component in internal/ui/src/components/date-time/components/time-split.tsx already includes sufficient validation through handleChange and handleBlur functions, making additional runtime validation unnecessary.
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#2707
File: apps/dashboard/lib/trpc/routers/ratelimit/createOverride.ts:63-63
Timestamp: 2024-12-05T13:27:55.555Z
Learning: In `apps/dashboard/lib/trpc/routers/ratelimit/createOverride.ts`, when determining the maximum number of rate limit overrides (`max`), the intentional use of `const max = hasWorkspaceAccess("ratelimitOverrides", namespace.workspace) || 5;` allows `max` to fall back to `5` when `hasWorkspaceAccess` returns `0` or `false`. This fallback behavior is expected and intended in the codebase.
Learnt from: p6l-richard
PR: unkeyed/unkey#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.
Learnt from: p6l-richard
PR: unkeyed/unkey#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.
Learnt from: mcstepp
PR: unkeyed/unkey#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.
Learnt from: chronark
PR: unkeyed/unkey#2146
File: apps/dashboard/app/(app)/apis/[apiId]/settings/default-prefix.tsx:74-75
Timestamp: 2024-10-04T17:27:09.821Z
Learning: In `apps/dashboard/app/(app)/apis/[apiId]/settings/default-prefix.tsx`, the hidden `<input>` elements for `workspaceId` and `keyAuthId` work correctly without being registered with React Hook Form.
Learnt from: unrenamed
PR: unkeyed/unkey#2214
File: apps/www/components/ui/code-editor.tsx:26-26
Timestamp: 2024-10-08T15:33:04.290Z
Learning: Using array index as key in nested mapping is acceptable in React components when the list does not change over time (no additions, removals, or reordering).
Learnt from: unrenamed
PR: unkeyed/unkey#2214
File: apps/www/components/ui/code-editor.tsx:26-26
Timestamp: 2024-10-05T17:36:37.293Z
Learning: Using array index as key in nested mapping is acceptable in React components when the list does not change over time (no additions, removals, or reordering).
apps/dashboard/app/(app)/authorization/roles/components/table/components/assigned-items-cell.tsx (19)
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#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.
Learnt from: AkshayBandi027
PR: unkeyed/unkey#2215
File: apps/dashboard/app/(app)/@breadcrumb/authorization/roles/[roleId]/page.tsx:28-29
Timestamp: 2024-10-08T15:33:04.290Z
Learning: In `authorization/roles/[roleId]/update-role.tsx`, the tag `role-${role.id}` is revalidated after updating a role to ensure that the caching mechanism is properly handled for roles.
Learnt from: chronark
PR: unkeyed/unkey#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.
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#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.
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3401
File: apps/dashboard/app/(app)/logs/filters.query-params.ts:10-0
Timestamp: 2025-06-24T13:29:10.129Z
Learning: The `buildQueryParams` function in `apps/dashboard/app/(app)/logs/filters.query-params.ts` calls `useFilters()` hook inside it, but this is valid because the function is only called from within other React hooks, maintaining the Rules of Hooks compliance.
Learnt from: p6l-richard
PR: unkeyed/unkey#2085
File: apps/www/components/glossary/terms-rolodex-desktop.tsx:26-37
Timestamp: 2024-10-23T16:20:19.324Z
Learning: When reviewing React components in this project, avoid suggesting manual memoization with `useMemo` for performance optimizations, as the team prefers to rely on the React compiler to handle such optimizations.
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#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.
Learnt from: chronark
PR: unkeyed/unkey#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.
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3380
File: apps/dashboard/app/(app)/ratelimits/[namespaceId]/namespace-navbar.tsx:40-66
Timestamp: 2025-06-19T10:39:29.388Z
Learning: tRPC has built-in caching that prevents skeleton flashing during component re-renders and navigation, so concerns about `!data || isLoading` conditions causing loading state flashes are generally not needed.
Learnt from: unrenamed
PR: unkeyed/unkey#2652
File: apps/www/components/particles.tsx:88-90
Timestamp: 2024-11-08T11:44:42.947Z
Learning: In React TypeScript components, if a function memoized with `useCallback` is not called elsewhere in the component, and a `useEffect` is used to invoke it, do not suggest removing the `useEffect` as it is necessary to execute the function.
Learnt from: mcstepp
PR: unkeyed/unkey#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.
Learnt from: mcstepp
PR: unkeyed/unkey#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.
Learnt from: unrenamed
PR: unkeyed/unkey#2214
File: apps/www/components/analytics/analytics-bento.tsx:87-88
Timestamp: 2024-10-05T17:26:57.657Z
Learning: When using `prism-react-renderer`, the `editorLanguage` prop does not support 'typescript' as a value; 'ts' should be used instead.
Learnt from: unrenamed
PR: unkeyed/unkey#2214
File: apps/www/components/analytics/analytics-bento.tsx:87-88
Timestamp: 2024-10-08T15:33:04.290Z
Learning: When using `prism-react-renderer`, the `editorLanguage` prop does not support 'typescript' as a value; 'ts' should be used instead.
Learnt from: p6l-richard
PR: unkeyed/unkey#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.
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#2825
File: apps/dashboard/app/(app)/logs-v2/components/controls/components/logs-datetime/index.tsx:0-0
Timestamp: 2025-01-30T20:38:00.058Z
Learning: In the logs dashboard, keyboard shortcuts that toggle UI elements (like popovers) should be implemented in the component that owns the state being toggled, not in the presentational wrapper components. For example, the 'T' shortcut for toggling the datetime filter is implemented in DatetimePopover, not in LogsDateTime.
Learnt from: p6l-richard
PR: unkeyed/unkey#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.
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3380
File: apps/dashboard/app/(app)/ratelimits/[namespaceId]/namespace-navbar.tsx:35-41
Timestamp: 2025-06-19T10:39:16.254Z
Learning: For critical components like namespace navigation that fetch essential data, error handling may not be necessary if query failures would indicate system-wide infrastructure issues rather than recoverable errors.
Learnt from: Srayash
PR: unkeyed/unkey#2568
File: apps/dashboard/app/auth/sign-up/oauth-signup.tsx:25-25
Timestamp: 2024-10-25T23:53:41.716Z
Learning: In the React component `OAuthSignUp` (`apps/dashboard/app/auth/sign-up/oauth-signup.tsx`), adding a `useEffect` cleanup function to reset the `isLoading` state causes a "something went wrong" popup to appear before redirecting when a user clicks on signup.
apps/dashboard/app/(app)/authorization/permissions/components/table/components/assigned-items-cell.tsx (2)
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#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.
Learnt from: chronark
PR: unkeyed/unkey#2143
File: apps/dashboard/app/(app)/logs/components/log-details/components/log-footer.tsx:58-61
Timestamp: 2024-12-03T14:21:19.543Z
Learning: For the "Outcome" field in the `LogFooter` component (`apps/dashboard/app/(app)/logs/components/log-details/components/log-footer.tsx`), default to "N/A" instead of "VALID" when handling null values to avoid confusing customers.
apps/dashboard/app/(app)/authorization/permissions/components/table/components/skeletons.tsx (2)
Learnt from: chronark
PR: unkeyed/unkey#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.
Learnt from: AkshayBandi027
PR: unkeyed/unkey#2215
File: apps/dashboard/app/(app)/@breadcrumb/authorization/roles/[roleId]/page.tsx:28-29
Timestamp: 2024-10-08T15:33:04.290Z
Learning: In `authorization/roles/[roleId]/update-role.tsx`, the tag `role-${role.id}` is revalidated after updating a role to ensure that the caching mechanism is properly handled for roles.
🧬 Code Graph Analysis (7)
apps/dashboard/lib/trpc/routers/index.ts (2)
apps/dashboard/lib/trpc/routers/authorization/roles/keys/connected-keys.ts (1)
  • queryRoleKeys (10-30)
apps/dashboard/lib/trpc/routers/authorization/roles/permissions/connected-permissions.ts (1)
  • queryRolePermissions (10-32)
apps/dashboard/app/(app)/authorization/roles/components/table/components/actions/components/delete-role.tsx (2)
apps/dashboard/lib/trpc/routers/authorization/roles/query.ts (1)
  • RoleBasic (16-16)
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/keys-table-action.popover.tsx (1)
  • ActionComponentProps (6-9)
apps/dashboard/app/(app)/authorization/roles/components/table/components/actions/keys-table-action.popover.constants.tsx (1)
apps/dashboard/lib/trpc/routers/authorization/roles/query.ts (1)
  • RoleBasic (16-16)
apps/dashboard/app/(app)/authorization/roles/components/table/components/skeletons.tsx (1)
internal/icons/src/icons/key-2.tsx (1)
  • Key2 (15-73)
apps/dashboard/app/(app)/authorization/roles/components/table/hooks/use-role-limits.ts (1)
apps/dashboard/lib/trpc/server.ts (1)
  • trpc (7-14)
apps/dashboard/app/(app)/authorization/roles/components/table/components/assigned-items-cell.tsx (4)
apps/dashboard/app/(app)/authorization/permissions/components/table/components/assigned-items-cell.tsx (1)
  • AssignedItemsCell (4-115)
apps/dashboard/lib/trpc/server.ts (1)
  • trpc (7-14)
internal/icons/src/icons/key-2.tsx (1)
  • Key2 (15-73)
internal/icons/src/icons/page-2.tsx (1)
  • Page2 (15-76)
apps/dashboard/app/(app)/authorization/permissions/components/table/components/skeletons.tsx (2)
internal/icons/src/icons/tag.tsx (1)
  • Tag (15-39)
internal/icons/src/icons/key-2.tsx (1)
  • Key2 (15-73)
⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: Test Packages / Test ./internal/clickhouse
🔇 Additional comments (17)
apps/dashboard/app/(app)/authorization/roles/components/table/components/actions/keys-table-action.popover.constants.tsx (1)

49-58: Verify prefetchInfinite call path – potential mismatch

trpcUtils.authorization.roles.keys.query.prefetchInfinite(...) and the analogous permissions call read oddly: the .query helper is usually used for finite queries, whereas .prefetchInfinite() is intended for an infinite query procedure that is typically exposed as keysinfinite (or similar).
If the router exposes keys.query not as an infinite procedure, these pre-fetches will silently skip the pagination cursor logic and fetch only the first page.

Please double-check the procedure names in authorization.roles and adjust the utils accessors as needed.

apps/dashboard/app/(app)/authorization/roles/components/table/components/actions/components/delete-role.tsx (1)

70-73: roleIds parameter might expect an array

useDeleteRole().mutateAsync({ roleIds: roleDetails.roleId }) passes a single string to a plural-named field.
If the backend schema defines roleIds as string[], this will break at runtime or require implicit coercion.

Proposed minimal fix:

-      await deleteRole.mutateAsync({
-        roleIds: roleDetails.roleId,
-      });
+      await deleteRole.mutateAsync({
+        roleIds: [roleDetails.roleId],
+      });

Please confirm the expected shape in useDeleteRole before merging.

apps/dashboard/app/(app)/authorization/permissions/components/upsert-permission/hooks/use-upsert-permission.ts (1)

15-15: LGTM: Proper cache invalidation for related data.

Adding roles cache invalidation ensures that role-related data (such as permission counts) stays consistent after permission mutations. This aligns well with the refactored approach of separating role and permission queries.

apps/dashboard/lib/trpc/routers/index.ts (1)

30-30: LGTM: New imports properly added for role optimization.

The queryRoleKeys and queryRolePermissions imports support the performance optimization by enabling separate fetching of connected keys and permissions counts.

Also applies to: 34-34

apps/dashboard/app/(app)/authorization/permissions/components/table/components/skeletons.tsx (3)

2-2: LGTM: Key2 icon import supports skeleton improvements.

The additional import enables the use of appropriate icons in the skeleton components.


30-30: LGTM: Icon change improves semantic consistency.

Changing from Page2 to Tag better represents the assigned keys content in the skeleton state.


37-42: LGTM: Restructured skeleton matches actual component layout.

The updated AssignedToKeysColumnSkeleton with Key2 icon and proper flex layout aligns with the corresponding UI component structure.

apps/dashboard/app/(app)/authorization/roles/components/table/components/skeletons.tsx (2)

23-23: LGTM: Fade-in animations enhance loading UX.

The animate-in fade-in duration-300 classes provide smooth entrance transitions for skeleton components, improving the loading experience.

Also applies to: 32-32


26-26: LGTM: Width adjustment aligns with content changes.

Increasing the pulse bar width from w-16 to w-20 likely matches the actual content width in the updated UI components.

apps/dashboard/app/(app)/authorization/roles/components/upsert-role/components/warning-callout.tsx (1)

10-31: Well-implemented warning component

The component effectively communicates role assignment limits with clear messaging and appropriate styling. The conditional text generation based on type is clean and maintainable.

apps/dashboard/app/(app)/authorization/roles/components/upsert-role/components/assign-key/key-field.tsx (1)

6-7: Excellent integration of role limits functionality

The integration of useRoleLimits hook and conditional rendering of RoleWarningCallout provides clear feedback to users about role assignment limits while maintaining the existing functionality.

Also applies to: 31-32, 169-187

apps/dashboard/app/(app)/authorization/permissions/components/table/permissions-list.tsx (1)

6-6: Clean simplification aligns with performance goals

The removal of detailed item rendering in favor of count-based displays using AssignedItemsCell effectively reduces data complexity while maintaining user understanding. The prop name change from type to kind and use of totalCount fields are consistent with the broader refactoring.

Also applies to: 107-108, 119-121, 129-135

apps/dashboard/app/(app)/authorization/roles/components/table/hooks/use-role-limits.ts (1)

3-10: Well-designed centralized limit management

The hook effectively centralizes role attachment limit logic with clear state management and appropriate separation of concerns. The RoleLimitState type and MAX_ATTACH_LIMIT constant provide good structure for the feature.

Also applies to: 18-69, 85-90

apps/dashboard/lib/trpc/routers/authorization/permissions/query.ts (1)

16-16: Effective query simplification supports performance goals

The removal of detailed role name fetching and parsing in favor of totalConnectedRoles count reduces query complexity and aligns with the performance optimization objectives. This change supports the UI's shift to count-based displays.

Also applies to: 129-129

apps/dashboard/app/(app)/authorization/roles/components/upsert-role/components/assign-permission/permissions-field.tsx (2)

85-85: Good null safety improvement!

The optional chaining for permission.roles?.some() prevents potential runtime errors.


93-118: Well-structured fallback pattern for permission data.

The three-tier fallback approach (assignedPermsDetails → allPermissions → default object) ensures data consistency across different loading states.

apps/dashboard/app/(app)/authorization/permissions/components/table/components/assigned-items-cell.tsx (1)

28-52: Well-structured display text logic with proper error handling.

The fail-fast approach with error throwing aligns with the codebase preferences, and the switch statements handle all valid cases correctly.

@ogzhanolguncu
Copy link
Contributor Author

@chronark This is also good to go but give it another try

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants