Conversation
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 21
🧹 Nitpick comments (67)
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/settings/update-key-ratelimit.tsx (2)
32-55: Consider updating form schema to include ratelimitType.While the
ratelimitTypeis hardcoded in theonSubmitfunction, it might be more maintainable to include it in the form schema for clarity and to ensure type safety.const formSchema = z.object({ keyId: z.string(), workspaceId: z.string(), enabled: z.boolean(), ratelimitAsync: z.boolean().optional().default(false), + ratelimitType: z.literal("v1").default("v1"), ratelimitLimit: z.coerce .number({ errorMap: (issue, { defaultError }) => ({ message: issue.code === "invalid_type" ? "Amount must be greater than 0" : defaultError, }), }) .positive({ message: "This refill limit must be a positive number." }) .int() .optional(), ratelimitDuration: z.coerce // ... rest of the schema });
91-93: Consider using a more structured error handling approach.Currently, errors are logged to the console and shown as a toast message. Consider capturing and categorizing errors more systematically.
onError(err) { - console.error(err); - toast.error(err.message); + // Log the error with context for better debugging + console.error("Failed to update ratelimit:", err); + + // Provide more specific error messages to the user + if (err.data?.code === "UNAUTHORIZED") { + toast.error("You don't have permission to update this key's ratelimit."); + } else if (err.data?.code === "BAD_REQUEST") { + toast.error("Invalid ratelimit configuration. Please check your inputs."); + } else { + toast.error(err.message || "Failed to update ratelimit. Please try again."); + } },apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/settings/update-key-owner-id.tsx (2)
60-61: Remove console error logging.There's a
console.errorstatement in the error handler, which should be removed or replaced with proper error logging for production code.onError(err) { - console.error(err); toast.error(err.message); },
32-38: Consider renaming component and props for clarity.Since the codebase is transitioning from
ownerIdtoidentityId, consider renaming this component and its props to reflect the new ownership model. The current nameUpdateKeyOwnerIdand the prop structure still reference the legacyownerIdfield.apps/dashboard/app/(app)/apis/[apiId]/_components/create-key/components/ratelimit-setup.tsx (1)
21-22: Minor cleanup - removed inline comment.The removal of the inline comment about
as constis a minor cleanup that doesn't affect functionality.apps/dashboard/app/(app)/apis/[apiId]/_components/create-key/components/general-setup.tsx (1)
51-71: Consider adding initial value handling for edit scenarios.The combobox implementation works well for creating new keys, but doesn't handle synchronizing with existing form values when editing a key.
Consider initializing the selected identity state from existing form values:
- const [selectedIdentityId, setSelectedIdentityId] = useState<string | null>(null); + const { watch } = useFormContext<FormValues>(); + const externalIdValue = watch("externalId"); + const [selectedIdentityId, setSelectedIdentityId] = useState<string | null>(externalIdValue || null);This ensures the component correctly displays the current value when editing existing keys.
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/edit-ratelimits/utils.ts (1)
5-14: Consider extracting duplicate default rate limit configuration.The same default rate limit configuration appears both here and in
getDefaultValues(). This creates a maintenance burden if defaults need to change.Consider extracting this to a shared constant:
+// In a shared constants file or at the top of this file +export const DEFAULT_RATELIMIT = { + name: "Default", + limit: 10, + refillInterval: 1000, +}; export const getKeyRatelimitsDefaults = (keyDetails: KeyDetails) => { const defaultRatelimits = keyDetails.key.ratelimits.items.length > 0 ? keyDetails.key.ratelimits.items - : (getDefaultValues().ratelimit?.data ?? [ - { - name: "Default", - limit: 10, - refillInterval: 1000, - }, - ]); + : (getDefaultValues().ratelimit?.data ?? [DEFAULT_RATELIMIT]);This would make future changes to defaults easier to maintain.
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/hooks/use-edit-metadata.ts (1)
4-53: Consider exposing loading state for better UI feedback.While the hook handles success and error states well, it doesn't expose the loading state that could be used to disable UI elements or show spinners during mutation.
Consider exposing the mutation state in the return value:
export const useEditMetadata = (onSuccess?: () => void) => { const trpcUtils = trpc.useUtils(); - const updateKeyMetadata = trpc.key.update.metadata.useMutation({ + const mutation = trpc.key.update.metadata.useMutation({ // existing handlers }); - return updateKeyMetadata; + return { + updateKeyMetadata: mutation.mutate, + updateKeyMetadataAsync: mutation.mutateAsync, + isLoading: mutation.isPending, + isError: mutation.isError, + error: mutation.error, + }; };This allows consumers to show loading indicators and disable UI elements while updates are in progress.
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/hooks/use-update-key-status.tsx (2)
25-34: Improve error message consistencyThe endpoint reference in the error message differs from other similar hooks. The contact URL ("support.unkey.dev") doesn't match the email format ("support@unkey.dev") used in other error messages in the codebase.
- "We encountered an issue while updating your key. Please try again later or contact support at support.unkey.dev", + "We encountered an issue while updating your key. Please try again later or contact support@unkey.dev",
39-40: Inconsistent support link implementationThis uses
window.opento navigate to "https://support.unkey.dev" whereas other similar hooks use "mailto:support@unkey.dev". Consider standardizing the support contact method across all hooks.- onClick: () => window.open("https://support.unkey.dev", "_blank"), + onClick: () => window.open("mailto:support@unkey.dev", "_blank"),apps/dashboard/app/(app)/apis/[apiId]/_components/create-key/hooks/use-fetch-identities/index.ts (2)
54-58: Add loading state check toloadMorefunctionThe
loadMorefunction checks if it's not already fetching the next page and if there is a next page, but it doesn't verify that the initial data loading is complete. This could lead to issues if called before the initial data is loaded.const loadMore = () => { - if (!isFetchingNextPage && hasNextPage) { + if (!isLoading && !isFetchingNextPage && hasNextPage) { fetchNextPage(); } };
47-52: Consider adding a loading placeholderWhen data is loading, the hook returns an empty array. Consider providing a loading placeholder or skeleton state to improve user experience during the initial load.
internal/ui/src/components/button.tsx (1)
363-372: Consider clarifying the purpose of maintaining two refsThe component now has both a forwarded
refand an internalbuttonRef. While they serve different purposes (forwarded for external use, internal for width measurement), this could be confusing. Consider adding a comment explaining why both are needed.apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/hooks/use-edit-expiration.ts (2)
9-13: Improve date formatting for better readabilityThe current implementation uses
toLocaleString()for formatting the expiration date, which includes both date and time with default formatting. Consider using a more specific date format that matches the application's date standards.- } has been updated to expire on ${variables.expiration.data.toLocaleString()}`; + } has been updated to expire on ${variables.expiration.data.toLocaleDateString(undefined, { + year: 'numeric', + month: 'long', + day: 'numeric', + hour: '2-digit', + minute: '2-digit' + })}`;
21-25: Consider adding a timeout before calling onSuccessWhen working with UI components that might need to close or transition after a successful operation, it can be helpful to add a small delay before calling the
onSuccesscallback. This allows the success message to be visible briefly before any UI changes occur.trpcUtils.api.keys.list.invalidate(); if (onSuccess) { - onSuccess(); + // Small delay to allow the toast to be visible before potential UI changes + setTimeout(onSuccess, 300); }apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/keys-table-action.popover.constants.tsx (2)
38-48: Remove console.error in production code.The console.error in the clipboard error handler could leak sensitive information in production. Consider using a structured logging approach or removing it.
- console.error("Failed to copy to clipboard:", error); + // Silent failure is acceptable for clipboard operations
26-57: Consider grouping related actions with comments.The menu items are organized with dividers, but the grouping logic isn't immediately clear. Adding comments to indicate logical groups would improve maintainability.
return [ + // Key information actions { id: "override", label: "Edit key name...", icon: <PenWriting3 size="md-regular" />, ActionComponent: (props) => <EditKeyName {...props} keyDetails={key} />, }, { id: "copy", label: "Copy key ID", className: "mt-1", icon: <Clone size="md-regular" />, onClick: () => { navigator.clipboard .writeText(key.id) .then(() => { toast.success("Key ID copied to clipboard"); }) .catch((error) => { console.error("Failed to copy to clipboard:", error); toast.error("Failed to copy to clipboard"); }); }, divider: true, }, + // Identity and status actions { id: "edit-external-id", label: "Edit External ID...", icon: <ArrowOppositeDirectionY size="md-regular" />, ActionComponent: (props) => <EditExternalId {...props} keyDetails={key} />, divider: true, },apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/settings/update-key-remaining.tsx (1)
135-168: Apply DRY principle to mutation calls.The mutation calls in both branches are nearly identical except for the
enabledflag and additional data. Consider using a single mutation call with conditional object construction.- if (values.limitEnabled) { - // When limit is enabled, construct the full limit object - await updateRemaining.mutateAsync({ - keyId: values.keyId, - limit: { - enabled: true, - data: { - remaining: values.remaining!, - refill: ... - }, - }, - }); - } else { - await updateRemaining.mutateAsync({ - keyId: values.keyId, - limit: { - enabled: false, - }, - }); - } + const limitConfig = values.limitEnabled + ? { + enabled: true, + data: { + remaining: values.remaining ?? 0, + refill: getRefillConfig(), + }, + } + : { enabled: false }; + + await updateRemaining.mutateAsync({ + keyId: values.keyId, + limit: limitConfig, + });apps/dashboard/app/(app)/apis/[apiId]/_components/create-key/hooks/use-fetch-identities/create-identity-options.tsx (4)
13-21: Improve type checking in isMetaEmpty function.The
isMetaEmptyfunction has a potential issue with its type checking. It checks ifmetais falsy (which includesnull), but then only verifies if it's an object type without explicitly checking fornullagain.const isMetaEmpty = (meta: unknown) => { if (!meta) { return true; } - if (typeof meta !== "object") { + if (meta === null || typeof meta !== "object") { return false; } return Object.keys(meta).length === 0; };
34-46: Consider using responsive width values.The component uses hardcoded width values like
w-[120px]andw-[200px]which might not be responsive across different screen sizes. Consider using responsive design patterns.- <span className="text-accent-12 font-medium text-xs w-[120px] truncate text-left"> + <span className="text-accent-12 font-medium text-xs max-w-[120px] sm:max-w-[160px] md:max-w-[200px] truncate text-left">Also applies to: 104-114
48-99: Extract complex tooltip content into a separate component.The JSX structure for the tooltip content is complex and deeply nested. For better maintainability, consider extracting it into a separate component.
+ // In a separate file or above this function + const IdentityMetadataTooltip = ({ identity }: { identity: Omit<Identity, "deleted"> }) => { + const hasMetadata = !isMetaEmpty(identity.meta); + + return ( + <div className="flex flex-col h-full"> + {/* Header - Always shown */} + <div className="px-4 py-2 border-b border-grayA-4 text-gray-10 text-xs font-medium bg-grayA-2"> + Metadata + </div> + {/* Content - Different based on metadata presence */} + {hasMetadata ? ( + // ... rest of the content + ) : ( + // ... empty metadata content + )} + </div> + ); + }; // Then in your tooltip <TooltipContent side="right" align="start" sideOffset={30} className="drop-shadow-2xl border border-gray-4 overflow-hidden rounded-[10px] p-0 bg-white dark:bg-black w-[320px] z-[100]" > - <div className="flex flex-col h-full"> - {/* ... all the complex nested JSX ... */} - </div> + <IdentityMetadataTooltip identity={identity} /> </TooltipContent>
119-139: Document the special "load_more" value.The "Load more" button uses a special value
"__load_more__"which is not a real identity ID. This should be documented to prevent confusion.if (hasNextPage) { options.push({ label: ( <Button type="button" variant="ghost" loading={isFetchingNextPage} onClick={(e) => { e.preventDefault(); e.stopPropagation(); loadMore(); }} className="text-xs text-accent-12 px-2 py-0.5 hover:bg-grayA-3 rounded w-full bg-transparent hover:bg-transparent focus:ring-0 font-medium" > Load more... </Button> ), + // Special sentinel value to identify the "Load more" option + // This should be filtered out before using the selected value value: "__load_more__", selectedLabel: <></>, }); }apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/status-cell/components/status-badge.tsx (3)
17-17: Clarify the purpose of badgeRef.The
badgeRefis created but only used in the Badge component without any apparent DOM manipulation. If it's not being used for any specific functionality, consider removing it.- const badgeRef = useRef<HTMLDivElement>(null); // And then below: - <Badge - ref={badgeRef} + <Badge
23-67: Simplify conditional rendering logic.The conditional rendering structure is complex with nested conditions. Consider extracting the rendering logic into separate components or functions.
+ // Extract Badge rendering to separate functions + const renderPrimaryBadge = () => { + if (isDisabled) { + return ( + <Badge + className={cn( + "px-1.5 rounded-md flex gap-2 items-center max-w-min h-[22px] border-none cursor-pointer", + isSelected ? STATUS_STYLES.badge.selected : STATUS_STYLES.badge.default, + )} + > + {primary.icon && <span>{primary.icon}</span>} + <span>{primary.label}</span> + </Badge> + ); + } + + return ( + <div + className={cn( + primary.color, + "px-1.5 py-1 flex items-center justify-center gap-2 h-[22px]", + count > 0 ? "rounded-l-md" : "rounded-md", + )} + > + {primary.icon && <span>{primary.icon}</span>} + <span>{primary.label}</span> + </div> + ); + }; + + const renderCountBadge = () => { + if (!count) return null; + + if (isDisabled) { + return ( + <Badge + className={cn( + "rounded-r-md px-1.5 py-1 flex items-center justify-center h-[22px] border-none", + primary.color, + )} + > + +{count} + </Badge> + ); + } + + return ( + <div + className={cn( + primary.color, + "rounded-r-md px-1.5 py-1 flex items-center justify-center h-[22px]", + )} + > + +{count} + </div> + ); + }; return ( <div className="flex items-center justify-start gap-0.5 text-xs"> - {isDisabled ? ( - // Use Badge component only for "Disabled" label - <Badge - ref={badgeRef} - className={cn( - "px-1.5 rounded-md flex gap-2 items-center max-w-min h-[22px] border-none cursor-pointer", - isSelected ? STATUS_STYLES.badge.selected : STATUS_STYLES.badge.default, - )} - > - {primary.icon && <span>{primary.icon}</span>} - <span>{primary.label}</span> - </Badge> - ) : ( - <div - className={cn( - primary.color, - "px-1.5 py-1 flex items-center justify-center gap-2 h-[22px]", - count > 0 ? "rounded-l-md" : "rounded-md", - )} - > - {primary.icon && <span>{primary.icon}</span>} - <span>{primary.label}</span> - </div> - )} - - {count > 0 && - (isDisabled ? ( - <Badge - className={cn( - "rounded-r-md px-1.5 py-1 flex items-center justify-center h-[22px] border-none", - primary.color, - )} - > - +{count} - </Badge> - ) : ( - <div - className={cn( - primary.color, - "rounded-r-md px-1.5 py-1 flex items-center justify-center h-[22px]", - )} - > - +{count} - </div> - ))} + {renderPrimaryBadge()} + {renderCountBadge()} </div> );
13-14: Document the purpose of the isSelected prop.The new
isSelectedprop has been added with a default value, but its purpose is not clearly documented. Consider adding a JSDoc comment to explain its use.type StatusBadgeProps = { primary: { label: string; color: string; icon: React.ReactNode; }; count: number; + /** + * When true, applies selected state styling to the badge. + * Used when the badge is in a selected/active state (e.g., in dropdown or focused row). + */ isSelected?: boolean; };apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/hooks/use-edit-ratelimits.ts (1)
57-60: Add protocol to support URL for better user experience.The support URL is missing the protocol (https://), which may prevent it from being clickable in some contexts.
- "We encountered an issue while updating your key. Please try again later or contact support at support.unkey.dev", + "We encountered an issue while updating your key. Please try again later or contact support at https://support.unkey.dev",apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/disable-key.tsx (2)
36-36: Consider defaulting confirmation to false for destructive actionsSetting
confirmStatusChangetotrueby default might skip an important safety step for users when disabling keys. Consider defaulting this tofalsefor the disabling action to ensure users must take an explicit action before confirming.- confirmStatusChange: true, + confirmStatusChange: isEnabling ? true : false,
74-87: Enhance error handling specificityWhile the comment explains that
useUpdateKeyStatushandles toast notifications, the catch block silently swallows all errors. Consider adding more specific error handling or at least logging for debugging purposes.} catch (error) { // `useUpdateKeyStatus` already shows a toast, but we still need to // prevent unhandled‐rejection noise in the console. + console.error("Error updating key status:", error); } finally {apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/edit-metadata/index.tsx (2)
59-81: Simplify conditional metadata handlingThe current implementation has two separate mutation calls with overlapping logic. Consider simplifying to reduce duplication.
const onSubmit = async (data: MetadataFormValues) => { try { - if (data.metadata.enabled && data.metadata.data) { - await updateMetadata.mutateAsync({ - keyId: keyDetails.id, - metadata: { - enabled: data.metadata.enabled, - data: data.metadata.data, - }, - }); - } else { - await updateMetadata.mutateAsync({ - keyId: keyDetails.id, - metadata: { - enabled: false, - }, - }); - } + await updateMetadata.mutateAsync({ + keyId: keyDetails.id, + metadata: { + enabled: data.metadata.enabled && !!data.metadata.data, + data: data.metadata.enabled ? data.metadata.data : undefined, + }, + }); } catch { // useEditMetadata already shows a toast, but we still need to // prevent unhandled rejection noise in the console. + console.error("Error updating metadata"); } };
115-117: Consider adding more specific styling structureUsing
[&>*:first-child]:p-0is a bit indirect. Consider a more explicit class structure for MetadataSetup to make the styling relationships clearer and more maintainable.- <div className="[&>*:first-child]:p-0"> + <div className="metadata-setup-container"> <MetadataSetup /> </div>This would require adding the appropriate styles for
.metadata-setup-containerin your CSS.apps/dashboard/components/ui/command.tsx (1)
45-45: Avoid using hardcoded size with!importantUsing
!size-[14px]with an important flag (!) could make future maintenance challenging. Consider using the icon's built-in size props or a more standard Tailwind class.- <Magnifier className="mr-2 shrink-0 text-accent-9 !size-[14px]" size="sm-regular" /> + <Magnifier className="mr-2 shrink-0 text-accent-9" size="sm" />apps/engineering/content/design/components/form/form-checkbox.variants.tsx (1)
1-206: Consider adding interactive examplesWhile the current examples are comprehensive in showing various states and variants, they're all static. Consider adding some examples showing state changes on interaction (e.g., toggling a checkbox affecting others or showing/hiding content).
This would require adding some simple state management, for example:
export const InteractiveFormCheckboxVariant = () => { const [isChecked, setIsChecked] = useState(false); return ( <RenderComponentWithSnippet> <div className="space-y-4"> <FormCheckbox label="Show advanced options" description="Toggle to view additional settings" onCheckedChange={(checked) => setIsChecked(checked)} /> {isChecked && ( <div className="pl-6 p-3 border rounded-md bg-gray-1"> <p>Advanced options appear here</p> </div> )} </div> </RenderComponentWithSnippet> ); };apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/edit-ratelimits/index.tsx (2)
77-77: Form ID doesn't match component purposeThe form ID "edit-remaining-uses-form" doesn't match this component's purpose of editing rate limits, suggesting it might have been copied from another component.
- <form id="edit-remaining-uses-form" onSubmit={handleSubmit(onSubmit)}> + <form id="edit-ratelimits-form" onSubmit={handleSubmit(onSubmit)}>In related code, you should also update line 90 to match:
- form="edit-remaining-uses-form" + form="edit-ratelimits-form"
69-72: Minimal error handling in onSubmitThe catch block silences errors without providing any specific feedback to users beyond what the mutation hook might provide.
Consider enhancing error handling to provide more context-specific feedback beyond what the hook provides:
try { await key.mutateAsync({ keyId: keyDetails.id, ratelimitType: "v2", ratelimit: { enabled: data.ratelimit.enabled, data: data.ratelimit.data, }, }); - } catch { + } catch (error) { // `useEditRatelimits` already shows a toast, but we still need to // prevent unhandled rejection noise in the console. + console.error("Failed to update ratelimits:", error); + // Optional: Add additional UI feedback specific to this component }apps/dashboard/components/ui/form-combobox.tsx (3)
89-117: Simplify complex conditional rendering logicThe nested ternary expressions for variant-based styling can be difficult to read and maintain.
Extract the variant-based logic into a separate function or use a more readable approach:
- <output - id={descriptionId} - className={cn( - "text-gray-9 flex gap-2 items-start", - inputVariant === "success" - ? "text-success-11" - : inputVariant === "warning" - ? "text-warning-11" - : "", - )} - > + <output + id={descriptionId} + className={cn( + "text-gray-9 flex gap-2 items-start", + { + "text-success-11": inputVariant === "success", + "text-warning-11": inputVariant === "warning", + } + )} + >
100-114: Extract icon rendering into a helper functionThe conditional icon rendering logic can be extracted to improve readability.
+ const DescriptionIcon = () => { + if (inputVariant === "warning") { + return ( + <TriangleWarning2 + size="md-regular" + className="flex-shrink-0 mt-[3px]" + aria-hidden="true" + /> + ); + } + return ( + <CircleInfo + size="md-regular" + className="flex-shrink-0 mt-[3px]" + aria-hidden="true" + /> + ); + }; <div className="size-[14px]"> - {inputVariant === "warning" ? ( - <TriangleWarning2 - size="md-regular" - className="flex-shrink-0 mt-[3px]" - aria-hidden="true" - /> - ) : ( - <CircleInfo - size="md-regular" - className="flex-shrink-0 mt-[3px]" - aria-hidden="true" - /> - )} + <DescriptionIcon /> </div>
36-56: Add defensive programming for unexpected variant valuesThe component doesn't verify that the variant value is valid, which could lead to unexpected styling issues.
Consider adding validation for the variant prop:
export const FormCombobox = React.forwardRef<HTMLDivElement, FormComboboxProps>( ( { label, description, error, required, optional, className, wrapperClassName, variant, id: propId, ...props }, ref, ) => { + // Ensure variant is one of the expected values + const validVariants = ["default", "error", "success", "warning"]; + const normalizedVariant = variant && validVariants.includes(variant) ? variant : "default"; + - const inputVariant = error ? "error" : variant; + const inputVariant = error ? "error" : normalizedVariant; const inputId = propId || React.useId(); const descriptionId = `${inputId}-helper`; const errorId = `${inputId}-error`;apps/dashboard/lib/trpc/routers/key/updateExpiration.ts (2)
65-87: DRY up repetitive error handlingThe error handling pattern is repeated in several places with identical messages, which violates the DRY principle.
Consider extracting the error handling into a reusable function:
+ const handleKeyUpdateError = (operation: string) => { + return (err: unknown) => { + console.error(`Error ${operation} key:`, err); + throw new TRPCError({ + code: "INTERNAL_SERVER_ERROR", + message: `We were unable to update expiration on this key. Please try again or contact support@unkey.dev`, + }); + }; + }; await db .transaction(async (tx) => { await tx .update(schema.keys) .set({ expires, }) .where(eq(schema.keys.id, key.id)) - .catch((_err) => { - throw new TRPCError({ - code: "INTERNAL_SERVER_ERROR", - message: - "We were unable to update expiration on this key. Please try again or contact support@unkey.dev", - }); - }); + .catch(handleKeyUpdateError("updating")); // ... audit logs ... }) - .catch((_err) => { - throw new TRPCError({ - code: "INTERNAL_SERVER_ERROR", - message: - "We were unable to update expiration on this key. Please try again or contact support@unkey.dev", - }); - }); + .catch(handleKeyUpdateError("updating in transaction"));
72-75: Use template literals consistentlyThe audit log description uses string concatenation for some values, which is inconsistent with the use of template literals elsewhere in the codebase.
description: - input.expiration?.enabled && input.expiration.data - ? `Changed expiration of ${key.id} to ${input.expiration.data.toUTCString()}` - : `Disabled expiration for ${key.id}`, + input.expiration?.enabled && input.expiration.data + ? `Changed expiration of ${key.id} to ${input.expiration.data.toUTCString()}` + : `Disabled expiration for ${key.id}`,The code already uses template literals, so this comment is mainly to remind about consistently using them throughout the codebase.
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/edit-key-name.tsx (3)
14-31: Refine schema validation for key name changesThe current validation only checks if names are different when normalized. However, the error message isn't clear about whitespace trimming.
Improve the validation and error message for clarity:
const editNameFormSchema = z .object({ name: nameSchema, //Hidden field. Required for comparison originalName: z.string().optional().default(""), }) .superRefine((data, ctx) => { const normalizedNewName = (data.name || "").trim(); const normalizedOriginalName = (data.originalName || "").trim(); if (normalizedNewName === normalizedOriginalName && normalizedNewName !== "") { ctx.addIssue({ code: z.ZodIssueCode.custom, - message: "New name must be different from the current name", + message: "New name must be different from the current name (ignoring whitespace)", path: ["name"], }); } + + // Additional validation could be added here + // For example, check minimum meaningful change (not just whitespace difference) + if (normalizedNewName !== "" && normalizedNewName.replace(/\s+/g, "") === normalizedOriginalName.replace(/\s+/g, "")) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: "New name must have substantive changes beyond whitespace", + path: ["name"], + }); + } });
80-87: Enhance error handling in onSubmitSimilar to the issue in the EditRatelimits component, error handling here is minimal and doesn't provide context-specific feedback.
const onSubmit = async (data: EditNameFormValues) => { try { await key.mutateAsync({ ...data, keyId: keyDetails.id }); - } catch { + } catch (error) { // `useEditKeyName` already shows a toast, but we still need to // prevent unhandled‐rejection noise in the console. + console.error("Failed to update key name:", error); + + // You could also add component-specific error handling here + // For example, to highlight specific form fields based on the error } };
111-111: Inconsistent button text handling during submission stateThe button text changes based on isSubmitting state, but this pattern is not used consistently across similar components.
For consistency with other components like EditRatelimits, consider using the loading prop instead of conditional text:
<Button type="submit" form="edit-key-name-form" variant="primary" size="xlg" className="w-full rounded-lg" disabled={!isValid || isSubmitting} loading={key.isLoading} > - {isSubmitting ? "Saving..." : "Update key name"} + Update key name </Button>apps/dashboard/components/confirmation-popover.tsx (3)
6-6: Avoid deep-linking into package internals
@unkey/ui/src/lib/utilsis an internal path that is not part of the public API surface of the package. Importing from it risks breakage on every package upgrade. Re-exportcnfrom@unkey/ui(or a local helper) and import from there instead.-import { cn } from "@unkey/ui/src/lib/utils"; +import { cn } from "@unkey/ui";
104-104: Removeas anycast by tightening theButtonprop typeCasting to
anyhides type errors.Buttonalready narrows acceptedcolors; extendConfirmVariantto match those values or widen theButtontype:const { iconBg, iconColor, buttonColor, icon: Icon } = VARIANT_STYLES[variant]; ... -<Button color={buttonColor as any} onClick={handleConfirm} className="px-4"> +<Button color={buttonColor} onClick={handleConfirm} className="px-4">
buttonColor’s type can be inferred with:type ButtonColor = typeof VARIANT_STYLES[keyof typeof VARIANT_STYLES]['buttonColor'];
66-69: Support async confirmation handlersIf
onConfirmreturns a promise (e.g., server call) the popover closes immediately and any rejection is swallowed. Consider awaiting the promise and only closing on success (or surfacing errors):-const handleConfirm = () => { - onConfirm(); - onOpenChange(false); -}; +const handleConfirm = async () => { + try { + await onConfirm(); + onOpenChange(false); + } catch (err) { + // surface error feedback here + console.error(err); + } +};apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/edit-credits/index.tsx (5)
58-93: Consider simplifying the conditional logic in the submit handler.The current nested conditional structure is complex and makes the flow harder to follow. Consider flattening the logic for better readability.
const onSubmit = async (data: CreditsFormValues) => { try { - if (data.limit) { - if (data.limit.enabled === true) { - if (data.limit.data) { - await key.mutateAsync({ - keyId: keyDetails.id, - limit: { - enabled: true, - data: data.limit.data, - }, - }); - } else { - // Shouldn't happen - toast.error("Failed to Update Key Limits", { - description: "An unexpected error occurred. Please try again later.", - action: { - label: "Contact Support", - onClick: () => window.open("https://support.unkey.dev", "_blank"), - }, - }); - } - } else { - await key.mutateAsync({ - keyId: keyDetails.id, - limit: { - enabled: false, - }, - }); - } - } + // Case 1: No limit data provided + if (!data.limit) { + return; + } + + // Case 2: Limits disabled + if (!data.limit.enabled) { + await key.mutateAsync({ + keyId: keyDetails.id, + limit: { + enabled: false, + }, + }); + return; + } + + // Case 3: Limits enabled but no data (error case) + if (!data.limit.data) { + toast.error("Failed to Update Key Limits", { + description: "An unexpected error occurred. Please try again later.", + action: { + label: "Contact Support", + onClick: () => window.open("https://support.unkey.dev", "_blank"), + }, + }); + return; + } + + // Case 4: Limits enabled with data (happy path) + await key.mutateAsync({ + keyId: keyDetails.id, + limit: { + enabled: true, + data: data.limit.data, + }, + }); } catch { // `useEditKeyRemainingUses` already shows a toast, but we still need to // prevent unhandled‐rejection noise in the console. } };
89-92: Add specific error logging.While the comment explains why errors are caught silently, it would be better to log the error for debugging purposes, even if you're not displaying it to the user.
} catch (error) { // `useEditKeyRemainingUses` already shows a toast, but we still need to // prevent unhandled‐rejection noise in the console. + console.error("Error in edit credits submission:", error); }
90-90: Incorrect hook name in comment.The comment refers to
useEditKeyRemainingUsesbut the hook being used isuseEditCredits.- // `useEditKeyRemainingUses` already shows a toast, but we still need to + // `useEditCredits` already shows a toast, but we still need to
97-97: Form ID mismatch.The form has ID "edit-remaining-uses-form" but this component is for editing credits, not remaining uses.
- <form id="edit-remaining-uses-form" onSubmit={handleSubmit(onSubmit)}> + <form id="edit-credits-form" onSubmit={handleSubmit(onSubmit)}>Also update the button reference on line 110:
- form="edit-remaining-uses-form" + form="edit-credits-form"
52-56: Improve hook naming clarity.The variable name
keyfor the hook result is confusing as it could be mistaken for the API key object, when it's actually the mutation hook result.- const key = useEditCredits(() => { + const editCreditsMutation = useEditCredits(() => { reset(getKeyLimitDefaults(keyDetails)); clearPersistedData(); onClose(); });Also update related references on lines 63, 81, and 115.
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/keys-table-action.popover.tsx (1)
33-41: Improve focus management on popover open.The current implementation has duplicate focus logic in both the useEffect and the onOpenAutoFocus handler, which could lead to inconsistent behavior. Consider consolidating this logic.
useEffect(() => { - if (open) { - const firstEnabledIndex = items.findIndex((item) => !item.disabled); - setFocusIndex(firstEnabledIndex >= 0 ? firstEnabledIndex : 0); - if (firstEnabledIndex >= 0) { - menuItems.current[firstEnabledIndex]?.focus(); - } - } + // Only reset focus index when popover opens/closes, but don't focus elements + // (let onOpenAutoFocus handle that) + if (open) { + const firstEnabledIndex = items.findIndex((item) => !item.disabled); + setFocusIndex(firstEnabledIndex >= 0 ? firstEnabledIndex : 0); + } }, [open, items]);apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/edit-external-id/index.tsx (4)
58-69: Simplify dialog open change handler logic.The nested conditional logic is overly complex. The function can be simplified while maintaining the same behavior.
const handleDialogOpenChange = (open: boolean) => { - if (isConfirmPopoverOpen) { - // If confirm popover is active don't let this trigger outer popover - if (!open) { - return; - } - } else { - if (!open) { - onClose(); - } - } + // If trying to close when confirm popover is active, prevent closure + if (!open && isConfirmPopoverOpen) { + return; + } + + // Otherwise, handle normal close + if (!open) { + onClose(); + } };
71-80: Ensure atomicity in state updates during async operations.The clearSelection function updates state and then performs an async operation. This could lead to race conditions if the component unmounts or the function is called multiple times.
const clearSelection = async () => { - setSelectedIdentityId(null); - await updateKeyOwner.mutateAsync({ + try { + await updateKeyOwner.mutateAsync({ + keyId: keyDetails.id, + ownerType: "v2", + identity: { + id: null, + }, + }); + setSelectedIdentityId(null); + } catch (error) { + console.error("Error clearing external ID:", error); + } - keyId: keyDetails.id, - ownerType: "v2", - identity: { - id: null, - }, - }); };
133-137: Improve combobox onChange handler.The current implementation searches through all identities on every value change. This could be inefficient for large lists. Consider optimizing this handler.
onValueChange={(val) => { - const identity = identities.find((id) => id.id === val); - setSelectedIdentityId(identity?.id || null); + // Only update if the value has changed + if (val !== selectedIdentityId) { + setSelectedIdentityId(val || null); + } }}
150-157: Consider adding loading state to the confirm popover.The
clearSelectionfunction is asynchronous, but the ConfirmPopover doesn't show a loading state during the operation.Create a loading state variable and pass it to the ConfirmPopover:
const [isConfirmPopoverOpen, setIsConfirmPopoverOpen] = useState(false); + const [isClearingSelection, setIsClearingSelection] = useState(false); const clearSelection = async () => { + setIsClearingSelection(true); try { await updateKeyOwner.mutateAsync({ keyId: keyDetails.id, ownerType: "v2", identity: { id: null, }, }); setSelectedIdentityId(null); + setIsConfirmPopoverOpen(false); } catch (error) { console.error("Error clearing external ID:", error); } finally { + setIsClearingSelection(false); } }; // ... later in JSX ... <ConfirmPopover isOpen={isConfirmPopoverOpen} onOpenChange={setIsConfirmPopoverOpen} onConfirm={clearSelection} triggerRef={clearButtonRef} title="Confirm removing external ID" description="This will remove the external ID association from this key. Any tracking or analytics related to this ID will no longer be associated with this key." confirmButtonText="Remove external ID" cancelButtonText="Cancel" variant="danger" + isLoading={isClearingSelection} />Note: This assumes the ConfirmPopover component accepts an
isLoadingprop. If it doesn't, you may need to modify that component.internal/ui/src/components/checkbox.tsx (3)
170-173: Consider expanding variant map for better coverage.The VARIANT_MAP only includes "default" and "destructive" variants. Consider adding more common variants for consistency with other components.
const VARIANT_MAP: Record<string, { variant: CheckboxVariant; color?: CheckboxColor }> = { default: { variant: "primary" }, destructive: { variant: "primary", color: "danger" }, + success: { variant: "primary", color: "success" }, + warning: { variant: "primary", color: "warning" }, + info: { variant: "primary", color: "info" }, + outline: { variant: "outline" }, + ghost: { variant: "ghost" }, };
168-168: Enhance checkmarkVariants with proper variants.The
checkmarkVariantscva doesn't define any variants, making it less useful for styling customization.-const checkmarkVariants = cva("flex items-center justify-center"); +const checkmarkVariants = cva("flex items-center justify-center", { + variants: { + color: { + white: "text-white", + black: "text-black", + primary: "text-accent-12", + success: "text-success-12", + warning: "text-warning-12", + danger: "text-error-12", + info: "text-info-12", + }, + size: { + sm: "scale-75", + md: "scale-100", + lg: "scale-125", + xlg: "scale-150", + }, + }, + defaultVariants: { + color: "white", + size: "md", + }, +});Then update the usage:
- <CheckboxPrimitive.Indicator className={cn(checkmarkVariants(), checkmarkColor)}> + <CheckboxPrimitive.Indicator + className={cn( + checkmarkVariants({ + size, + color: mappedColor === "default" && mappedVariant === "primary" + ? "white" + : "white" + }) + )} + >
275-278: Improve checkmark color logic.The current checkmark color logic is hard to read and doesn't account for all possible variant combinations.
- const checkmarkColor = - mappedColor === "default" && mappedVariant === "primary" - ? "text-white dark:text-black" - : "text-white"; + // Determine checkmark color based on variant and color + let checkmarkColor = "text-white"; + + // For primary variant with default color, use white (or black in dark mode) + if (mappedVariant === "primary" && mappedColor === "default") { + checkmarkColor = "text-white dark:text-black"; + } + // For outline and ghost variants, use the theme color + else if (mappedVariant === "outline" || mappedVariant === "ghost") { + switch (mappedColor) { + case "success": + checkmarkColor = "text-success-11"; + break; + case "warning": + checkmarkColor = "text-warning-11"; + break; + case "danger": + checkmarkColor = "text-error-11"; + break; + case "info": + checkmarkColor = "text-info-11"; + break; + default: + checkmarkColor = "text-gray-12"; + } + }internal/ui/src/components/form/form-checkbox.tsx (1)
73-111: Consider simplifying the nested conditional styling logicThe conditional text color logic uses nested ternary operators which can become harder to maintain as more conditions are added.
Consider extracting this conditional logic into a helper function or using a mapping object:
- <output - id={descriptionId} - className={cn( - "text-gray-9 flex gap-2 items-start", - variant === "primary" && color === "success" - ? "text-success-11" - : variant === "primary" && color === "warning" - ? "text-warning-11" - : "", - )} - > + <output + id={descriptionId} + className={cn( + "text-gray-9 flex gap-2 items-start", + getTextColorClass(variant, color), + )} + >Where you could define:
const getTextColorClass = (variant: CheckboxProps["variant"], color: CheckboxProps["color"]) => { if (variant === "primary") { if (color === "success") return "text-success-11"; if (color === "warning") return "text-warning-11"; } return ""; };apps/engineering/content/design/components/form-chekbox.mdx (1)
85-85: Minor grammar improvement neededThere's a missing article in this sentence.
-A subtler alternative to the primary variant with transparent background when checked. +A subtler alternative to the primary variant with a transparent background when checked.🧰 Tools
🪛 LanguageTool
[uncategorized] ~85-~85: You might be missing the article “a” here.
Context: ...alternative to the primary variant with transparent background when checked. <OutlineFormC...(AI_EN_LECTOR_MISSING_DETERMINER_A)
apps/engineering/content/design/components/checkbox.mdx (1)
487-489: Minor grammar improvement neededThere's a missing article in this sentence.
-Checkbox components can be combined with labels for better user experience. +Checkbox components can be combined with labels for a better user experience.🧰 Tools
🪛 LanguageTool
[uncategorized] ~489-~489: You might be missing the article “a” here.
Context: ...ponents can be combined with labels for better user experience. <RenderComponentWithS...(AI_EN_LECTOR_MISSING_DETERMINER_A)
apps/dashboard/lib/trpc/routers/api/keys/query-api-keys/schema.ts (2)
18-23:creditsSchemalacks conditional rules — enabled = true withnullfields is nonsensicalWhen
enabledistrue,remaining,refillAmount, andrefillDaymust be numbers.
You can express this with arefineor by splitting the schema:export const creditsSchema = z.discriminatedUnion("enabled", [ z.object({ enabled: z.literal(false), remaining: z.null(), refillAmount: z.null(), refillDay: z.null(), }), z.object({ enabled: z.literal(true), remaining: z.number().int().nonnegative(), refillAmount: z.number().int().positive(), refillDay: z.number().int().min(1).max(31), }), ]);
27-33: Default empty array & minimum length guard forratelimits.items
itemsis required even whenenabledisfalse, which means callers must send an (often useless) empty array.
Either default it or add a conditional guard similar to credits:export const ratelimitsSchema = z.discriminatedUnion("enabled", [ z.object({ enabled: z.literal(false), items: z.array(ratelimitItemSchema).default([]) }), z.object({ enabled: z.literal(true), items: z.array(ratelimitItemSchema).min(1, "At least one rule is required"), }), ]);apps/dashboard/components/ui/combobox.tsx (1)
148-176: Leaking option labels into search index can confuse users
CommandInputfilters byvaluenotlabel, so iflabelcontains rich JSX (icons, secondary text) search may fail.
Consider adding a hiddenkeywordsprop to options or filtering againstoption.valueas well.This is a UX nit but easy to improve.
apps/dashboard/lib/trpc/routers/key/updateRatelimit.ts (3)
25-28:discriminatedUnion().and()can narrow but also masks exhaustivenessCombining the union with
.and(base)produces an intersection that TypeScript often widens, makingratelimitTypechecks less safe (e.g., forgetting to handle future"v3").Consider extracting the discriminant into a top-level union:
export const ratelimitInputSchema = z.union([ baseRatelimitInputSchema.merge(ratelimitValidationV1), baseRatelimitInputSchema.merge(ratelimitValidationV2), ]);This keeps type-narrowing predictable.
171-176: Sequential DB writes inside a loop – usePromise.allfor latency, or batch
for…ofupdates/inserts each rule one by one.
Although wrapped in a transaction, round-trips still happen sequentially and will hurt performance when editing 10-20 rules.Either collect updates and execute them with
Promise.all([...])or use a single bulkinsert … on conflict updatequery where supported.
224-244: Audit log meta key explosion riskStoring each rule as
ratelimit.rule.${index}.*can exceed row size limits and makes querying painful.
Consider JSON-encoding the wholeinput.ratelimit.dataarray into a singleratelimit.rulesfield or limiting to a subset of fields.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (78)
apps/dashboard/app/(app)/apis/[apiId]/_components/create-key/components/credits-setup.tsx(1 hunks)apps/dashboard/app/(app)/apis/[apiId]/_components/create-key/components/general-setup.tsx(2 hunks)apps/dashboard/app/(app)/apis/[apiId]/_components/create-key/components/key-created-success-dialog.tsx(4 hunks)apps/dashboard/app/(app)/apis/[apiId]/_components/create-key/components/ratelimit-setup.tsx(2 hunks)apps/dashboard/app/(app)/apis/[apiId]/_components/create-key/create-key.constants.tsx(1 hunks)apps/dashboard/app/(app)/apis/[apiId]/_components/create-key/create-key.schema.ts(4 hunks)apps/dashboard/app/(app)/apis/[apiId]/_components/create-key/hooks/use-fetch-identities/create-identity-options.tsx(1 hunks)apps/dashboard/app/(app)/apis/[apiId]/_components/create-key/hooks/use-fetch-identities/index.ts(1 hunks)apps/dashboard/app/(app)/apis/[apiId]/_components/create-key/index.tsx(2 hunks)apps/dashboard/app/(app)/apis/[apiId]/api-id-navbar.tsx(1 hunks)apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/settings/update-key-expiration.tsx(1 hunks)apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/settings/update-key-metadata.tsx(1 hunks)apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/settings/update-key-owner-id.tsx(1 hunks)apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/settings/update-key-ratelimit.tsx(1 hunks)apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/settings/update-key-remaining.tsx(1 hunks)apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/delete-key.tsx(1 hunks)apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/disable-key.tsx(1 hunks)apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/edit-credits/index.tsx(1 hunks)apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/edit-credits/utils.ts(1 hunks)apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/edit-expiration/index.tsx(1 hunks)apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/edit-expiration/utils.ts(1 hunks)apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/edit-external-id/index.tsx(1 hunks)apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/edit-key-name.tsx(1 hunks)apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/edit-metadata/index.tsx(1 hunks)apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/edit-metadata/utils.ts(1 hunks)apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/edit-ratelimits/index.tsx(1 hunks)apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/edit-ratelimits/utils.ts(1 hunks)apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/hooks/use-delete-key.ts(1 hunks)apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/hooks/use-edit-credits.ts(1 hunks)apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/hooks/use-edit-expiration.ts(1 hunks)apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/hooks/use-edit-external-id.ts(1 hunks)apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/hooks/use-edit-key.tsx(1 hunks)apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/hooks/use-edit-metadata.ts(1 hunks)apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/hooks/use-edit-ratelimits.ts(1 hunks)apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/hooks/use-update-key-status.tsx(1 hunks)apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/key-info.tsx(1 hunks)apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/keys-table-action.popover.constants.tsx(1 hunks)apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/keys-table-action.popover.tsx(1 hunks)apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/skeletons.tsx(2 hunks)apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/status-cell/components/status-badge.tsx(1 hunks)apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/status-cell/index.tsx(3 hunks)apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/status-cell/use-key-status.ts(1 hunks)apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/keys-list.tsx(3 hunks)apps/dashboard/app/(app)/apis/[apiId]/page.tsx(1 hunks)apps/dashboard/app/(app)/apis/loading.tsx(2 hunks)apps/dashboard/app/react-query-provider.tsx(2 hunks)apps/dashboard/components/confirmation-popover.tsx(1 hunks)apps/dashboard/components/navigation/action-button.tsx(1 hunks)apps/dashboard/components/navigation/copyable-id-button.tsx(1 hunks)apps/dashboard/components/ui/combobox.tsx(1 hunks)apps/dashboard/components/ui/command.tsx(9 hunks)apps/dashboard/components/ui/form-combobox.tsx(1 hunks)apps/dashboard/components/ui/popover.tsx(2 hunks)apps/dashboard/components/ui/select.tsx(2 hunks)apps/dashboard/lib/audit.ts(2 hunks)apps/dashboard/lib/trpc/routers/api/keys/query-api-keys/get-all-keys.ts(3 hunks)apps/dashboard/lib/trpc/routers/api/keys/query-api-keys/schema.ts(2 hunks)apps/dashboard/lib/trpc/routers/index.ts(2 hunks)apps/dashboard/lib/trpc/routers/key/create.ts(1 hunks)apps/dashboard/lib/trpc/routers/key/query-identities.ts(1 hunks)apps/dashboard/lib/trpc/routers/key/updateEnabled.ts(2 hunks)apps/dashboard/lib/trpc/routers/key/updateExpiration.ts(4 hunks)apps/dashboard/lib/trpc/routers/key/updateMetadata.ts(4 hunks)apps/dashboard/lib/trpc/routers/key/updateName.ts(3 hunks)apps/dashboard/lib/trpc/routers/key/updateOwnerId.ts(3 hunks)apps/dashboard/lib/trpc/routers/key/updateRatelimit.ts(2 hunks)apps/dashboard/lib/trpc/routers/key/updateRemaining.ts(3 hunks)apps/engineering/content/design/components/checkbox.mdx(1 hunks)apps/engineering/content/design/components/form-chekbox.mdx(1 hunks)apps/engineering/content/design/components/form/form-checkbox.variants.tsx(1 hunks)internal/icons/src/icons/user.tsx(1 hunks)internal/icons/src/index.ts(1 hunks)internal/ui/package.json(1 hunks)internal/ui/src/components/button.tsx(1 hunks)internal/ui/src/components/checkbox.tsx(1 hunks)internal/ui/src/components/form/form-checkbox.tsx(1 hunks)internal/ui/src/components/form/index.tsx(1 hunks)internal/ui/src/index.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
apps/dashboard/lib/trpc/routers/key/create.ts (1)
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.
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/settings/update-key-owner-id.tsx (1)
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.
🧬 Code Graph Analysis (27)
apps/dashboard/app/(app)/apis/loading.tsx (2)
apps/dashboard/components/navigation/action-button.tsx (1)
NavbarActionButton(22-53)internal/icons/src/icons/plus.tsx (1)
Plus(16-52)
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/skeletons.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)/apis/[apiId]/_components/create-key/hooks/use-fetch-identities/index.ts (1)
apps/dashboard/lib/trpc/server.ts (1)
trpc(7-14)
internal/icons/src/icons/user.tsx (1)
internal/icons/src/props.ts (2)
IconProps(30-35)sizeMap(7-28)
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/edit-metadata/utils.ts (1)
apps/dashboard/lib/trpc/routers/api/keys/query-api-keys/schema.ts (1)
KeyDetails(51-51)
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/status-cell/index.tsx (1)
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/status-cell/components/status-badge.tsx (1)
StatusBadge(16-70)
apps/dashboard/app/(app)/apis/[apiId]/api-id-navbar.tsx (1)
apps/dashboard/app/(app)/apis/[apiId]/_components/create-key/index.tsx (1)
CreateKeyDialog(32-209)
apps/dashboard/app/react-query-provider.tsx (2)
apps/dashboard/lib/trpc/server.ts (1)
trpc(7-14)apps/dashboard/lib/utils.ts (1)
getBaseUrl(172-185)
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/edit-ratelimits/utils.ts (2)
apps/dashboard/lib/trpc/routers/api/keys/query-api-keys/schema.ts (1)
KeyDetails(51-51)apps/dashboard/app/(app)/apis/[apiId]/_components/create-key/create-key.utils.ts (1)
getDefaultValues(88-120)
internal/ui/src/components/button.tsx (1)
apps/dashboard/lib/utils.ts (1)
cn(5-7)
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/keys-list.tsx (4)
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/status-cell/index.tsx (1)
StatusDisplay(16-144)apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/keys-table-action.popover.tsx (1)
KeysTableActionPopover(27-130)apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/keys-table-action.popover.constants.tsx (1)
getKeysTableActionItems(25-97)apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/skeletons.tsx (1)
ActionColumnSkeleton(72-82)
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/hooks/use-delete-key.ts (1)
apps/dashboard/lib/trpc/server.ts (1)
trpc(7-14)
apps/dashboard/lib/trpc/routers/index.ts (3)
apps/dashboard/lib/trpc/trpc.ts (1)
t(8-8)apps/dashboard/lib/trpc/routers/key/query-identities.ts (1)
queryIdentities(27-82)apps/dashboard/lib/trpc/routers/key/updateOwnerId.ts (1)
updateKeyOwner(29-70)
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/edit-metadata/index.tsx (6)
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/keys-table-action.popover.tsx (1)
ActionComponentProps(6-9)apps/dashboard/hooks/use-persisted-form.tsx (1)
usePersistedForm(22-90)apps/dashboard/app/(app)/apis/[apiId]/_components/create-key/create-key.schema.ts (2)
MetadataFormValues(364-364)metadataSchema(223-227)apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/edit-metadata/utils.ts (1)
getKeyMetadataDefaults(3-10)apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/hooks/use-edit-metadata.ts (1)
useEditMetadata(4-53)apps/dashboard/app/(app)/apis/[apiId]/_components/create-key/components/metadata-setup.tsx (1)
MetadataSetup(18-113)
apps/dashboard/app/(app)/apis/[apiId]/_components/create-key/index.tsx (3)
apps/dashboard/components/navigation/action-button.tsx (1)
NavbarActionButton(22-53)internal/icons/src/icons/plus.tsx (1)
Plus(16-52)apps/dashboard/components/navigation/copyable-id-button.tsx (1)
CopyableIDButton(10-80)
apps/dashboard/lib/trpc/routers/key/updateMetadata.ts (3)
apps/dashboard/lib/trpc/trpc.ts (3)
t(8-8)requireUser(10-21)requireWorkspace(23-36)apps/dashboard/app/(app)/apis/[apiId]/_components/create-key/create-key.schema.ts (1)
metadataSchema(223-227)apps/dashboard/lib/audit.ts (1)
insertAuditLogs(99-174)
internal/ui/src/components/form/form-checkbox.tsx (4)
internal/ui/src/components/checkbox.tsx (2)
CheckboxProps(249-254)Checkbox(303-303)apps/dashboard/lib/utils.ts (1)
cn(5-7)internal/ui/src/components/form/form-textarea.tsx (2)
RequiredTag(128-142)OptionalTag(115-126)internal/icons/src/icons/circle-info.tsx (1)
CircleInfo(15-56)
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/hooks/use-edit-key.tsx (2)
apps/dashboard/lib/trpc/server.ts (1)
trpc(7-14)apps/dashboard/app/(app)/apis/[apiId]/_components/create-key/create-key.constants.tsx (1)
UNNAMED_KEY(11-11)
apps/dashboard/components/ui/command.tsx (1)
apps/dashboard/lib/utils.ts (1)
cn(5-7)
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/edit-expiration/utils.ts (2)
apps/dashboard/lib/trpc/routers/api/keys/query-api-keys/schema.ts (1)
KeyDetails(51-51)apps/dashboard/app/(app)/apis/[apiId]/_components/create-key/create-key.utils.ts (1)
getDefaultValues(88-120)
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/edit-credits/utils.ts (3)
apps/dashboard/app/(app)/apis/[apiId]/_components/create-key/create-key.schema.ts (1)
refillSchema(70-110)apps/dashboard/lib/trpc/routers/api/keys/query-api-keys/schema.ts (1)
KeyDetails(51-51)apps/dashboard/app/(app)/apis/[apiId]/_components/create-key/create-key.utils.ts (1)
getDefaultValues(88-120)
apps/dashboard/lib/trpc/routers/api/keys/query-api-keys/get-all-keys.ts (1)
apps/dashboard/lib/trpc/routers/api/keys/query-api-keys/schema.ts (1)
KeyDetails(51-51)
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/keys-table-action.popover.constants.tsx (2)
apps/dashboard/lib/trpc/routers/api/keys/query-api-keys/schema.ts (1)
KeyDetails(51-51)apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/keys-table-action.popover.tsx (1)
MenuItem(11-20)
apps/dashboard/lib/trpc/routers/key/updateRemaining.ts (3)
apps/dashboard/lib/trpc/trpc.ts (3)
t(8-8)requireUser(10-21)requireWorkspace(23-36)apps/dashboard/lib/trpc/routers/api/keys/query-api-keys/schema.ts (1)
creditsSchema(18-23)apps/dashboard/lib/audit.ts (1)
insertAuditLogs(99-174)
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/keys-table-action.popover.tsx (3)
apps/dashboard/components/ui/popover.tsx (3)
Popover(31-31)PopoverTrigger(31-31)PopoverContent(31-31)apps/dashboard/lib/utils.ts (1)
cn(5-7)internal/icons/src/icons/dots.tsx (1)
Dots(15-60)
apps/dashboard/lib/trpc/routers/api/keys/query-api-keys/schema.ts (1)
apps/dashboard/app/(app)/apis/[apiId]/_components/create-key/create-key.schema.ts (3)
ratelimitItemSchema(111-132)RatelimitItem(332-332)creditsSchema(229-241)
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/hooks/use-update-key-status.tsx (2)
apps/dashboard/lib/trpc/server.ts (1)
trpc(7-14)apps/dashboard/lib/trpc/routers/key/updateEnabled.ts (1)
updateKeyEnabled(7-85)
🪛 LanguageTool
apps/engineering/content/design/components/form-chekbox.mdx
[uncategorized] ~85-~85: You might be missing the article “a” here.
Context: ...alternative to the primary variant with transparent background when checked. <OutlineFormC...
(AI_EN_LECTOR_MISSING_DETERMINER_A)
apps/engineering/content/design/components/checkbox.mdx
[uncategorized] ~489-~489: You might be missing the article “a” here.
Context: ...ponents can be combined with labels for better user experience. <RenderComponentWithS...
(AI_EN_LECTOR_MISSING_DETERMINER_A)
[uncategorized] ~604-~604: You might be missing the article “the” here.
Context: ...me additional features not shown in all examples above: ### Legacy Variant Support For...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
🔇 Additional comments (123)
apps/dashboard/components/navigation/copyable-id-button.tsx (1)
62-62: Height adjustment looks good.The height change from
h-8toh-7appears to be a deliberate UI refinement for better visual consistency. According to the summary, this component is now used in the updated API key creation UI, so the height adjustment likely ensures it aligns properly with neighboring elements.internal/ui/package.json (1)
19-19: New checkbox dependency is appropriate.Adding the Radix UI checkbox component is consistent with the existing pattern of using Radix UI components in this codebase (you're already using react-slot, react-tooltip, etc.). This addition supports the new Checkbox component implementation mentioned in the summary.
apps/dashboard/app/(app)/apis/[apiId]/_components/create-key/create-key.constants.tsx (1)
11-11: Good extraction of string constant.Extracting "Unnamed Key" into a named constant follows the DRY principle and improves maintainability. According to the summary, this constant replaces hardcoded fallback strings for unnamed keys in related components, ensuring consistent naming across the UI.
apps/dashboard/app/(app)/apis/[apiId]/page.tsx (1)
1-4: Import path standardization looks good.Switching from relative to absolute imports improves code maintainability and readability. This change aligns with the broader refactoring effort in the
[apiId]directory mentioned in the summary, while maintaining the same functionality.internal/ui/src/index.ts (1)
10-10: Export for new Checkbox component added successfully.The Checkbox component export is correctly added to the barrel file, making it available for use throughout the application.
internal/ui/src/components/form/index.tsx (1)
3-3: FormCheckbox component is now properly exported.The FormCheckbox component is now available through the form components barrel file, following the established pattern for other form components.
internal/icons/src/index.ts (1)
96-96: User icon component export added correctly.The new User icon export follows the consistent pattern used for other icon exports in this file.
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/settings/update-key-ratelimit.tsx (1)
96-96:Details
✅ Verification successful
Correctly updated mutation call to include ratelimit type.
The
ratelimitType: "v1"parameter has been added to the mutation call to align with the backend API changes that now support multiple ratelimit versions through a discriminated union.Run the following to verify that the backend expects this parameter:
🏁 Script executed:
#!/bin/bash # Verify the backend mutation schema requires ratelimitType rg -A 10 "key\.update\.ratelimit" --glob "*.ts"Length of output: 2052
Mutation call correctly includes
ratelimitType: "v1"The backend’s
key.update.ratelimitmutation is a discriminated union onratelimitType, and other hooks (e.g. inuse-edit-ratelimits.ts) already branch onvariables.ratelimitType. This addition aligns with the expected schema:• File:
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/settings/update-key-ratelimit.tsx
Line 96await updateRatelimit.mutateAsync({ ...values, ratelimitType: "v1" });apps/dashboard/lib/trpc/routers/key/updateEnabled.ts (1)
84-84: Return object structure improvement.The mutation now returns a structured object with both the updated
enabledstatus and thekeyIdinstead of just a boolean value. This provides more context to the caller and aligns with the pattern established in other key update mutations.apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/settings/update-key-owner-id.tsx (1)
66-66: Added owner type discriminator to mutation call.The mutation call now explicitly includes
ownerType: "v1"to target the legacy owner update path, which aligns with the refactored backend that supports both legacy (v1) and identity-based (v2) ownership models.apps/dashboard/app/(app)/apis/loading.tsx (1)
1-1: Improved UI consistency with NavbarActionButton.Replacing the custom button implementation with the reusable
NavbarActionButtoncomponent improves code maintainability and ensures UI consistency across the application. The new implementation is cleaner and includes the appropriate Plus icon.Also applies to: 33-36
apps/dashboard/app/(app)/apis/[apiId]/_components/create-key/components/ratelimit-setup.tsx (1)
116-121: Support for ratelimit ID persistence added.The addition of a hidden input field for storing the
idproperty of each ratelimit item is essential for maintaining identity during update operations. This ensures existing ratelimits can be correctly identified and updated rather than recreated.apps/dashboard/app/(app)/apis/[apiId]/api-id-navbar.tsx (1)
98-98: Simplified navbar structure by removing API ID display.The navbar has been simplified by directly rendering the
CreateKeyDialogcomponent inside the<Navbar>, removing the previous<Navbar.Actions>container that displayed the API ID badge and copy button. This change coordinates with the enhanced key creation UI in the corresponding file that now includes the API ID copy functionality.apps/dashboard/app/(app)/apis/[apiId]/_components/create-key/index.tsx (2)
10-16: Added imports for enhanced nav bar components.Imported specialized navigation components to support the improved UI for the key creation trigger and API ID display.
139-145: Enhanced UI for key creation with improved navigation actions.Replaced the simple button with a more comprehensive navigation bar action area that includes:
- A
NavbarActionButtonwith a "Plus" icon for creating new keys- A
CopyableIDButtonfor displaying and copying the API IDThis enhancement provides a more intuitive and accessible UI by relocating the API ID copy functionality from the main navbar and providing clearer visual cues for actions.
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/status-cell/use-key-status.ts (1)
118-120: Updated access pattern for credit-related data.The data access pattern has been modified to retrieve
remainingandrefillAmountfrom the nestedcreditsobject withinkeyData.keyinstead of directly fromkeyData.key. This change ensures compatibility with the restructured data model, where credit information is organized in a dedicated nested object.apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/status-cell/index.tsx (3)
13-13: Good addition of the selection state propAdding the
isSelectedboolean prop to theStatusDisplayPropsinterface allows for proper visual feedback when a key is selected in the table, enhancing the user experience.
16-16: Correctly implemented selection state propagationThe
isSelectedprop is properly received in the component signature and correctly passed down to theStatusBadgecomponent, ensuring the selection state is properly visualized.Also applies to: 65-65
36-36: Improved styling consistency with alpha-based colorUpdating from
bg-gray-3tobg-grayA-3for the loading state container maintains a consistent styling approach using alpha-based colors across the application.apps/dashboard/components/navigation/action-button.tsx (1)
25-27: Enhanced default styling for better visual hierarchyThe changes to default props improve the component's appearance and usability:
- Changing from "outline" to "primary" variant makes action buttons more prominent
- Reducing size from "md" to "sm" creates a more compact, modern UI
- Simplifying the className to just padding allows the variant to control the appearance
These changes align well with modern UI practices and create consistency across the application.
apps/dashboard/lib/trpc/routers/index.ts (2)
28-28: Well-structured addition of identities query endpointAdding the identities query functionality follows the established pattern in the codebase. The implementation properly supports pagination and provides a clean API for retrieving identity data.
Also applies to: 89-91
33-33: Improved key owner update handler namingRenaming from
updateKeyOwnerIdtoupdateKeyOwnerbetter reflects the enhanced functionality that now supports multiple owner types ("v1" and "v2"), making the code more self-documenting.Also applies to: 97-97
apps/dashboard/components/ui/popover.tsx (2)
3-3: Logical reordering of importsMoving the
cnutility import above the Radix UI imports improves the organization of the code, with local/project utilities before external dependencies.
20-22: Enhanced popover styling with improved readabilityThe refactored styling improves both the component's appearance and code readability:
- Split class strings into logical groups for better maintainability
- Updated border radius from
rounded-mdtorounded-lgfor a softer look- Added
overflow-hiddento prevent content from breaking the popover's shape- Replaced generic colors with explicit gray scale colors for consistency
These changes align with the broader UI styling updates across the application.
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/settings/update-key-expiration.tsx (1)
82-85: API payload structure updated correctlyThe change to the payload structure follows the new pattern of using nested objects with
enabledanddataproperties for key updates. This aligns with the backend schema changes mentioned in the summary.apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/settings/update-key-metadata.tsx (1)
61-67: API payload structure updated correctlyThe mutation payload has been properly restructured to use a nested
metadataobject withenabledanddataproperties, consistent with the pattern used in other key update components. TheBoolean(values.metadata)expression correctly determines if metadata is enabled based on whether the string is non-empty.apps/dashboard/components/ui/select.tsx (3)
10-10: Improved color contrast with alpha transparencyChanging from solid gray colors to alpha-based gray colors (
text-grayA-8andtext-grayA-12) improves visual consistency across different background contexts.
15-15: Added dark mode supportThe addition of
dark:bg-blackfallback backgrounds for all variant styles ensures proper appearance in dark mode environments.Also applies to: 19-19, 23-23, 27-27
41-41: Consistent use of alpha transparencyUpdated to use
text-grayA-12instead oftext-gray-12for consistent use of alpha-based colors in text elements.internal/icons/src/icons/user.tsx (1)
1-48: Well-structured User icon componentThe new User icon component follows best practices for SVG icons in React:
- Properly handles size variations using the standardized
sizeMap- Uses
currentColorfor color inheritance- Includes appropriate licensing information
- Sets accessible SVG attributes
This will integrate well with the existing icon system based on the props interface.
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/key-info.tsx (1)
1-26: New KeyInfo component looks good!This well-designed component displays key information with appropriate styling and handles both key name display and fallback cases correctly. The use of the tooltip wrapper and monospace font for the ID enhances usability.
apps/dashboard/app/react-query-provider.tsx (3)
5-5: Good enhancement with additional tRPC client importsThe additional imports from
@trpc/clientsupport the new selective batching functionality implemented below.
26-47: Well-configured Query Client with appropriate default settingsThe explicit QueryClient configuration with documented settings for staleTime, cacheTime, retry behavior, etc. improves the application's data fetching strategy. These are sensible defaults that will enhance user experience by showing stale data during refetches and having appropriate cache invalidation periods.
52-65: Excellent implementation of selective batchingUsing
splitLinkto conditionally disable batching based on theskipBatchcontext flag is a good approach. This provides flexibility to opt out of batching for specific requests that require immediate processing while preserving batching benefits for most requests.apps/dashboard/app/(app)/apis/[apiId]/_components/create-key/components/credits-setup.tsx (2)
38-57: Improved default value handling when enabling usage limitsThe refactored code now sets up the entire refill structure with appropriate defaults in a single operation, which is cleaner and more maintainable than updating individual fields.
62-96: Cleaner refill interval change handling with consolidated object updatesThe refactored approach of setting the entire refill object at once based on the interval type improves code consistency and readability. Each interval type now has a clear set of associated default values.
apps/dashboard/lib/audit.ts (2)
80-80: LGTM: Added "ratelimit" as a resource typeAdding the "ratelimit" resource type to the audit log will properly track rate limit-related operations in the audit trail, which aligns with the enhanced rate limit functionality being introduced.
91-91: LGTM: Updated meta property type in resourcesAllowing
undefinedvalues in the meta property makes the type definition more flexible and better accommodates cases where metadata might be absent for certain resources.apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/hooks/use-delete-key.ts (1)
1-49: LGTM: Well-implemented delete key hookThe hook follows best practices for React hooks and TRPC mutations:
- Proper toast notifications for success and error states
- Comprehensive error handling with specific messages for different error types
- Cache invalidation on successful deletion
- Support for an optional callback
The error messages are clear and provide appropriate guidance to users when issues occur.
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/keys-list.tsx (4)
17-19: LGTM: Clean import organizationConsolidated React imports and added the new components needed for the action popover functionality.
188-195: LGTM: Updated StatusDisplay component with selection stateAdded the
isSelectedprop to enable visual feedback for selected keys in the status cell.
197-205: LGTM: Added action column with popover menuThe new action column integrates the key management actions via the KeysTableActionPopover component, providing a clean way to access key operations.
280-281: LGTM: Updated skeleton rendering for action columnAdded ActionColumnSkeleton for the new action column and updated the condition to exclude it from the default skeleton rendering.
apps/dashboard/app/(app)/apis/[apiId]/_components/create-key/components/general-setup.tsx (3)
1-8: New imports support the enhanced External ID selection.The additional imports are appropriately added to support the new combobox functionality, paginated identity fetching, and state management.
14-14: LGTM: Form utilities appropriately extended.The destructured
setValuefunction is necessary for programmatically updating form values when identities are selected.
17-26: Well-structured identity selection state management.The implementation appropriately:
- Maintains local state for the selected identity
- Fetches paginated identities
- Creates formatted option objects for the combobox
This allows for efficient selection from potentially large lists of identities.
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/edit-ratelimits/utils.ts (1)
1-3: LGTM: Appropriate imports.The imports bring in the necessary types and utility functions needed for default value generation.
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/hooks/use-edit-metadata.ts (3)
1-6: LGTM: Hook setup and dependencies.The imports and initial hook structure are appropriate for a mutation hook.
7-23: Well-implemented success handling with clear user feedback.The success handler:
- Provides context-aware success messages based on whether metadata was enabled or disabled
- Invalidates the keys list cache to ensure UI consistency
- Supports an optional callback for additional success handling
This gives users clear feedback about what changed and ensures the UI reflects the latest data.
24-51: Comprehensive error handling with specific user feedback.The error handler provides:
- Specific error messages based on error codes
- Clear instructions for users on how to proceed
- A support contact option for generic errors
This gives users actionable information when issues occur.
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/skeletons.tsx (2)
1-1: LGTM: Icon import for action skeleton.The Dots icon is appropriately imported to maintain visual consistency with the actual action button.
5-5: Width change improves responsive layout.The change from a fixed
w-[236px]tow-autoallows the skeleton to better adapt to different viewport sizes and content widths.apps/dashboard/app/(app)/apis/[apiId]/_components/create-key/create-key.schema.ts (4)
46-50: Well-structured schema extraction for name validation.Creating a reusable
nameSchemais a good practice that eliminates duplication and ensures consistent validation across different parts of the form.
61-61: Nice implementation of the extracted nameSchema.Using the extracted schema here maintains consistency and makes future changes easier.
112-112: Good addition for supporting update operations.Adding the
idfield asnullishtoratelimitItemSchemaallows for identification of existing rate limit items during updates while still supporting new item creation scenarios.
322-322: Consistent use of nameSchema.Applying the same schema here ensures validation consistency between UI forms and API inputs.
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/edit-credits/utils.ts (2)
1-6: Clear imports with appropriate types.The imports are well-organized, importing the necessary types and utilities from their respective modules.
7-44: Well-implemented utility for deriving default credit limit settings.The
getKeyLimitDefaultsfunction effectively maps key details to a structured form format, with appropriate fallback values. The logic for determining refill configuration based on the presence ofrefillDayandrefillAmountis clear and handles all possible cases.A few minor observations:
- Good use of comments to indicate different refill types (monthly, daily, none)
- Appropriate fallback to defaults when values are missing
- Well-structured return object that matches the form schema
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/hooks/use-edit-credits.ts (1)
1-44: Well-structured mutation hook with comprehensive error handling.This hook provides a clean interface for updating key credits with excellent error handling and user feedback:
- The success message is detailed and includes the key ID and remaining uses information
- Error handling is thorough with specific messages for different error cases
- The invalidation of the keys list query ensures UI stays in sync with data changes
- The optional success callback provides flexibility for parent components
The implementation follows best practices for mutation hooks in React and aligns well with the other similar hooks in the codebase.
apps/dashboard/lib/trpc/routers/api/keys/query-api-keys/get-all-keys.ts (3)
206-213: Good addition of ratelimits data to the query.Including the related ratelimits data with specific field selection improves the API by providing all necessary data in a single query, reducing the need for additional requests.
229-229: Type safety improvements and metadata exposure.The explicit type annotation on
transformedKeysenhances type safety, and exposing the key's metadata directly improves API usability.Also applies to: 245-245
247-262: Well-structured data transformation for credits and ratelimits.The restructuring of key data into a nested format with
creditsandratelimitsobjects improves data organization and API clarity:
- The credits object with
enabled,remaining,refillAmount, andrefillDayfields provides a comprehensive view of credit configuration- The ratelimits object with
enabledflag and items array clearly represents the state and configuration of rate limits- Mapping rate limit items with consistent property names facilitates frontend consumption
This structured approach aligns well with the form schemas and UI component structure, creating a consistent data model throughout the application.
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/hooks/use-update-key-status.tsx (1)
4-47: Well-structured hook with proper error handling and feedbackThe
useUpdateKeyStatushook follows good patterns for React hooks and tRPC mutations with:
- Clear success handling with appropriate messaging based on the enabled state
- Comprehensive error handling with specific messages for different error codes
- Cache invalidation to ensure UI consistency after mutations
- Optional callback support for component coordination
apps/dashboard/app/(app)/apis/[apiId]/_components/create-key/hooks/use-fetch-identities/index.ts (1)
7-72: Well-structured hook for paginated data fetchingThe
useFetchIdentitieshook implements a clean pattern for infinite queries with:
- Typed tRPC integration with proper pagination
- Comprehensive error handling with specific messages for different error codes
- Memoized data transformation to avoid unnecessary re-computations
- Helper functions for pagination control and data refreshing
internal/ui/src/components/button.tsx (1)
330-442: Excellent refactor to support ref forwardingThe Button component has been properly refactored to use
React.forwardRef, enabling it to accept and forward refs to the underlying DOM element. This improves composability with other components and libraries that require direct DOM access.apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/hooks/use-edit-expiration.ts (1)
4-55: Well-implemented hook for managing key expirationThe
useEditExpirationhook follows a consistent pattern with other update hooks in the codebase, providing:
- Proper interaction with the tRPC mutation
- Contextual success messaging based on the expiration state
- Comprehensive error handling with specific error codes
- Cache invalidation to ensure UI consistency
apps/dashboard/lib/trpc/routers/key/updateName.ts (3)
1-1: Good use of shared validation schemaUsing the imported
nameSchemaensures consistent validation rules across key creation and updates.Also applies to: 14-14
43-53: Nice addition of validation logicThe added validation logic properly normalizes both the current and new names before comparison, preventing unnecessary database operations when there's no actual change.
99-104: Enhanced return type provides better contextThe improved return object with
keyId,updatedflag, and both previous and new names provides more context for client-side handling, which improves the user experience when displaying success messages.apps/dashboard/app/(app)/apis/[apiId]/_components/create-key/components/key-created-success-dialog.tsx (3)
4-4: Good use of reusable components and constantsImporting the reusable
ConfirmPopovercomponent and theUNNAMED_KEYconstant promotes code consistency across the application.Also applies to: 11-11
129-129: Consistent handling of unnamed keysUsing the
UNNAMED_KEYconstant ensures consistent display of unnamed keys throughout the application.
228-242: Excellent refactoring to use the reusable confirmation popoverReplacing custom confirmation popover implementation with the reusable
ConfirmPopovercomponent improves code maintainability and reduces duplication.apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/hooks/use-edit-external-id.ts (2)
7-66: Well-implemented mutation hook with comprehensive error handlingThe hook provides differentiated success messages based on owner type and action, with detailed error handling for different error codes. The invalidation of the cached keys list ensures data consistency.
32-34: Good null check for onSuccess callbackChecking if the
onSuccesscallback exists before calling it prevents potential runtime errors.apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/hooks/use-edit-key.tsx (2)
10-18: Dynamic success message improves user feedbackThe success message that includes the previous and new names provides clear feedback to users about what changed.
23-51: Comprehensive error handling with specific messagesThe detailed error handling with specific messages for different error codes enhances the user experience by providing clear guidance on issues and next steps.
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/keys-table-action.popover.constants.tsx (1)
1-97: Comprehensive action menu implementation looks good.The implementation provides a well-structured set of actions for key management, with appropriate icons, labels, and components. The code handles clipboard operations with proper error handling and toast notifications.
apps/dashboard/app/(app)/apis/[apiId]/_components/create-key/hooks/use-fetch-identities/create-identity-options.tsx (1)
1-142: Well-implemented identity options with rich UI features.The implementation provides a comprehensive solution for displaying identities with tooltips for metadata and pagination support. The structure is clean and the component handles different scenarios appropriately.
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/status-cell/components/status-badge.tsx (1)
1-70: Good implementation of conditional badge rendering.The implementation appropriately handles different status types with conditional rendering, applying proper styling for both the primary badge and count indicator. The addition of the
isSelectedprop adds flexibility for different UI states.apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/edit-expiration/index.tsx (1)
1-113: Well-structured component with proper form management!This component effectively implements a dialog for editing key expiration settings with several good practices:
- Uses persisted form state keyed by API key ID
- Properly loads saved values when dialog opens
- Handles form state reset and cleanup
- Provides clear visual feedback during submission
- Prevents form submission when invalid or during loading states
- Has appropriate error handling that prevents unhandled promise rejections
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/hooks/use-edit-ratelimits.ts (1)
76-98: Clean implementation of time formatting utility.The
formatIntervalfunction is well-implemented with appropriate handling of different time ranges, resulting in user-friendly duration formats.apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/delete-key.tsx (1)
1-159: Excellent implementation of a destructive action with double-confirmation.This component demonstrates best practices for implementing destructive actions:
- Uses a checkbox confirmation to prevent accidental clicks
- Implements a second confirmation via popover
- Provides clear warnings about the irreversible nature of the action
- Manages loading states properly through the entire flow
- Has proper error handling with toast notifications
- Prevents dialog close during confirmation to avoid accidental dismissal
apps/dashboard/lib/trpc/routers/key/updateMetadata.ts (4)
1-1: Good modularization of schema validation.Importing and extending a shared schema improves code organization and reusability.
Also applies to: 12-13
19-30: Improved metadata parsing logic.The updated metadata parsing logic is more explicit about when to parse JSON, making the code safer and more readable.
80-83: Enhanced audit logging with context-aware descriptions.The audit log descriptions now properly distinguish between updating and removing metadata, improving clarity in audit trails.
105-108: Better API response structure.Returning an object with both the
keyIdandsuccessstatus provides more context to API consumers compared to a simple boolean.apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/disable-key.tsx (3)
13-17: Well-structured schema validationThe Zod schema effectively enforces confirmation with a clear error message, ensuring the user explicitly acknowledges their action before disabling a key.
52-63: Well-handled dialog and popover interactionThe code effectively prevents unintended dialog closures when the confirmation popover is active, maintaining a good user experience.
149-161: Good double-confirmation pattern for destructive actionsThe conditional rendering of the
ConfirmPopoveronly for disabling keys is an excellent UX practice, providing additional protection against accidental destructive actions.apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/edit-metadata/index.tsx (2)
24-35: Effective form persistence implementationUsing
usePersistedFormwith a key-specific storage key is a good approach to maintain form state between sessions. The "memory" storage type is appropriate for this temporary edit form.
89-92: Good form state management during dialog closeSaving the current values before closing the dialog ensures that user input isn't lost if they accidentally close the dialog, improving the user experience.
apps/dashboard/components/ui/command.tsx (3)
9-9: Icon replacement improves UI consistencyReplacing the Search icon with the Magnifier icon from
@unkey/iconsenhances UI consistency across the application, maintaining the design system's visual language.Also applies to: 44-45
18-18: Improved UI aesthetics with rounded cornersUpdating the command container to use
rounded-lgimproves the visual aesthetics and aligns with modern UI design practices.
117-118: Improved spacing and selection statesThe updates to item spacing (
mt-3 first-of-type:mt-0) and selection states (aria-selected:bg-grayA-3 aria-selected:text-grayA-12) improve both visual aesthetics and accessibility.apps/engineering/content/design/components/form/form-checkbox.variants.tsx (3)
1-13: Well-structured default checkbox variantThe default FormCheckbox variant includes appropriate label and description, providing a good base example of the component usage.
16-39: Good demonstration of required state and error handlingThese examples effectively demonstrate both the required state alone and with an error message, which are critical patterns for form validation.
162-205: Excellent complex checkbox hierarchy exampleThe complex example effectively demonstrates a common pattern of parent/child checkboxes with different states, sizes, and variants. This provides great reference for implementing hierarchical checkbox selections.
apps/dashboard/lib/trpc/routers/key/query-identities.ts (1)
41-43: Cursor comparison may break with UUIDs or non-monotonic IDs
lt(identity.id, cursor)only works if your primary keys are monotonically decreasing strings (e.g., Snowflake IDs).
If you ever switch to UUIDs the<operator becomes meaningless. Safer options:
- Use an integer/BigInt cursor (
createdAttimestamp, numeric ID).- When IDs are strings, switch to keyset pagination on
(createdAt, id)or fall back to offset pagination.Review the long-term ID strategy before this ships.
apps/dashboard/lib/trpc/routers/key/updateRemaining.ts (1)
11-15: Schema/implementation drift
creditsSchema(per snippet inquery-api-keys/schema.ts) defines{ enabled, remaining, refillAmount, refillDay }, whereas this code expectsinput.limit.enabledand nestedrefill.
Either the schema snippet is outdated or this mutation will never validate. Please ensure the Zod schema and runtime usage are aligned.internal/ui/src/components/checkbox.tsx (1)
1-304: Great implementation of an accessible, themeable checkbox component!The component is well-structured with comprehensive variants, proper accessibility support, and excellent type definitions with JSDoc comments. The use of Radix UI primitives ensures good accessibility, and the variant system provides flexible styling options.
internal/ui/src/components/form/form-checkbox.tsx (6)
1-6: Well-structured imports and dependenciesThe component imports the necessary dependencies and reuses existing components like OptionalTag and RequiredTag, promoting code reuse.
7-17: Good use of type documentation for API generationThe DocumentedFormCheckboxProps type provides a clean way to generate API documentation while maintaining proper type definitions. This approach helps maintain consistency between the code and documentation.
19-21: Clear type compositionThe FormCheckboxProps type properly extends the base CheckboxProps while omitting specific props to enforce defaults, which is a good practice for component composition.
22-44: Strong accessibility implementationThe component generates unique IDs for accessibility and properly sets up the relationships between form elements. The use of React.useId() for generating identifiers is a modern and recommended approach.
45-72: Well-structured fieldset and form layoutThe component follows best practices by using fieldset for grouping and proper label associations. The flexbox layout provides a clean and consistent appearance.
117-118: Properly defined display nameSetting the displayName property is good for debugging and dev tools identification.
apps/engineering/content/design/components/form-chekbox.mdx (5)
1-4: Correct metadata structure for documentationThe frontmatter includes appropriate title and description which helps with documentation navigation and searchability.
5-20: Well-organized variant importsThe imports are comprehensive and clearly named, making it easy to understand which variant examples are being included.
21-106: Comprehensive documentation of component variantsThe documentation thoroughly covers all variants and states of the FormCheckbox component with clear explanations and visual examples.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~85-~85: You might be missing the article “a” here.
Context: ...alternative to the primary variant with transparent background when checked. <OutlineFormC...(AI_EN_LECTOR_MISSING_DETERMINER_A)
107-115: Good use of auto-generated props tableUsing AutoTypeTable to generate the props documentation ensures that it stays in sync with the actual component implementation.
116-150: Strong focus on accessibility and best practicesThe documentation includes detailed sections on accessibility considerations and best practices, which is essential for building inclusive UI components.
apps/engineering/content/design/components/checkbox.mdx (7)
1-14: Well-structured component documentation headerThe documentation starts with a clear title, description, and necessary imports, providing a good foundation for the rest of the content.
15-110: Thorough coverage of basic variantsThe documentation comprehensively covers the primary, outline, and ghost variants with examples in both light and dark themes.
111-458: Extensive color variant documentationThe documentation thoroughly demonstrates all color variants (danger, warning, success, info) with each basic variant type, providing a complete reference for developers.
459-486: Clear size variant demonstrationThe documentation effectively shows the different size options available for the checkbox component.
490-601: Practical usage examplesThe documentation includes practical examples of checkboxes with labels and grouped checkboxes, which helps developers understand real-world implementation patterns.
602-614: Helpful additional feature documentationThe documentation explains legacy variant support and customization options, which is valuable for developers migrating from older versions or needing custom implementations.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~604-~604: You might be missing the article “the” here.
Context: ...me additional features not shown in all examples above: ### Legacy Variant Support For...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
615-669: Comprehensive implementation details and guidelinesThe documentation provides detailed implementation information and thorough usage guidelines covering semantics, accessibility, interaction design, content, form design, and responsive design considerations.
apps/dashboard/lib/trpc/routers/key/updateOwnerId.ts (5)
7-26: Well-designed discriminated union schemaThe schema uses a discriminated union pattern with the
ownerTypefield to clearly distinguish between v1 and v2 owner types. This provides good type safety and clear validation rules.
27-33: Clear type definition and procedure setupThe type definition and procedure setup use the discriminated union schema effectively. Renaming the procedure from
updateKeyOwnerIdtoupdateKeyOwnerbetter reflects its broadened functionality.
58-70: Clean routing logic based on owner typeThe procedure elegantly routes to the appropriate update function based on the owner type, passing all necessary context.
72-137: Well-implemented v1 owner update logicThe
updateOwnerV1function includes proper type checking, transaction handling, and audit logging. The function correctly validates the input type before processing, which prevents type errors.
139-206: Consistent v2 owner update implementationThe
updateOwnerV2function follows the same pattern as v1, maintaining consistency. The explicit setting ofownerIdto null ensures data integrity when migrating between owner types.// Line 164 includes an important data consistency practice ownerId: null, // Set ownerId to null to maintain consistencyThis approach prevents potential confusion by making sure only one owner reference type is active at a time.
...keyAuthId]/_components/components/table/components/actions/components/edit-metadata/utils.ts
Show resolved
Hide resolved
...yAuthId]/_components/components/table/components/actions/components/edit-expiration/utils.ts
Show resolved
Hide resolved
...yAuthId]/_components/components/table/components/actions/components/edit-ratelimits/utils.ts
Show resolved
Hide resolved
...pp/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/skeletons.tsx
Show resolved
Hide resolved
...ys/[keyAuthId]/_components/components/table/components/actions/keys-table-action.popover.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/dashboard/components/ui/combobox.tsx (1)
123-146:⚠️ Potential issueCombobox still missing
aria-controls/aria-labelledby– breaks a11y contract.Previous review flagged this; the trigger now has an
idbut the required relationships are still absent.
Add:- role="combobox" - aria-expanded={open} + role="combobox" + aria-expanded={open} + aria-controls={`${id}-listbox`} + aria-labelledby={`${id}-label`}and give
<CommandList>id={${id}-listbox}plus wrap the visible label in
<span id={${id}-label}>{selectedOption?.selectedLabel ?? placeholder}</span>.
🧹 Nitpick comments (10)
apps/dashboard/app/(app)/apis/[apiId]/_components/create-key/hooks/use-create-identity.ts (1)
1-47: Well-structured hook with comprehensive error handlingThe implementation provides good user feedback through toast notifications and properly invalidates the query cache on successful mutations. The error handling is robust, distinguishing between different error types with appropriate messages.
There's one potential improvement for the error handler:
- toast.error("Failed to Create Identity", { - description: - err.message || - "An unexpected error occurred. Please try again or contact support@unkey.dev", + toast.error("Failed to Create Identity", { + description: "An unexpected error occurred. Please try again or contact support@unkey.dev",Consider using only the generic message for unexpected errors instead of potentially exposing internal error messages to users.
apps/dashboard/app/(app)/apis/[apiId]/_components/create-key/components/external-id-field.tsx (1)
16-86: Well-implemented form field with search and creation capabilitiesThe component properly integrates with form state, handles search functionality, and offers identity creation when needed. The UI includes appropriate placeholders, error states, and loading indicators.
Consider these minor improvements:
The component resets its internal state when the value changes (using
key={value}), which might cause unexpected re-renders. A better approach would be to use auseEffectto reset the popover state.The error might need better accessibility:
- error={error} + error={error} + aria-invalid={!!error} + aria-errormessage={error ? `external-id-error-${Math.random().toString(36).substring(2, 9)}` : undefined}apps/dashboard/lib/trpc/routers/identity/query.ts (2)
27-82: Well-implemented paginated query procedureThe TRPC procedure follows best practices with proper input validation, middleware, error handling, and cursor-based pagination. The transformation of the database results to match the output schema is cleanly implemented.
Consider adding more detailed logging to help with debugging:
- console.error("Error retrieving identities:", error); + console.error("Error retrieving identities:", { + error, + workspaceId: ctx.workspace.id, + cursor: input.cursor, + limit: input.limit + });This additional context would make troubleshooting easier if errors occur in production.
57-65: Consider using optional chaining for nullable fieldsFor safer transformation of database results, consider using optional chaining:
const transformedIdentities = identities.map((identity) => ({ id: identity.id, externalId: identity.externalId, workspaceId: identity.workspaceId, environment: identity.environment, meta: identity.meta, createdAt: identity.createdAt, - updatedAt: identity.updatedAt ? identity.updatedAt : null, + updatedAt: identity.updatedAt ?? null, }));The nullish coalescing operator (
??) is more semantically appropriate here as it specifically handlesnullandundefinedvalues.apps/dashboard/lib/trpc/routers/identity/create.ts (4)
47-56:metaJSON validation ignores circular structures and size limits.
JSON.stringifywill throw for circular references, but large or deeply-nested objects can still sneak through and later bloat the column / exceed row-size limits.
Consider:• Adding a
zodschema that caps depth/size (e.g. viaz.record(z.any()).passthrough().refine(v => JSON.stringify(v).length < 8_000, "Meta is too large")).
• Persistingmetaasjsonbto exploit native validation & indexing.
58-66:updatedAtinitial value set tonullmay break ordering/TTL queries.If downstream queries sort by
updatedAtor filter withCOALESCE(updatedAt, createdAt), anullcan yield unexpected results.
Populate it withDate.now()on creation, then update on future mutations.- createdAt: Date.now(), - updatedAt: null, + createdAt: Date.now(), + updatedAt: Date.now(),
60-66: Hard-coded emptyenvironmentmay be semantically incorrect.If
environmentis meant to distinguish “prod | dev | staging”, persisting""hides intent and requires null-checks later.
Usenull(or a valid enum value) and document the default.- environment: "", + environment: null, // or "prod"
98-101: Return shape omits server-generated fields that the client often needs.Clients usually need at least
createdAtand possiblymeta; returning them avoids an immediate follow-up query.-return { identityId, externalId }; +return { identityId, externalId, createdAt: Date.now(), meta: input.meta };apps/dashboard/components/ui/combobox.tsx (2)
150-178:onSelecthandler should callevent.preventDefault()to keep focus logic consistent.Radix
CommandItemfires a custom select event that bubbles. WithoutpreventDefault()the popover may close twice, occasionally producing a React state warning in concurrent mode.onSelect={() => { - onSelect(option.value); - setOpen(false); + onSelect(option.value); + setOpen(false); + // prevent Radix default close logic from racing + event.preventDefault?.(); }}
108-112:useMemodependency should includeoptionslength or a stable reference.If the parent component regenerates an equal - by - value array each render,
optionsreference changes and defeats memoisation.
Consider acceptingoptionsfromuseMemoupstream or comparing viavalueset.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
apps/dashboard/app/(app)/apis/[apiId]/_components/create-key/components/external-id-field.tsx(1 hunks)apps/dashboard/app/(app)/apis/[apiId]/_components/create-key/components/general-setup.tsx(2 hunks)apps/dashboard/app/(app)/apis/[apiId]/_components/create-key/hooks/use-create-identity.ts(1 hunks)apps/dashboard/app/(app)/apis/[apiId]/_components/create-key/hooks/use-fetch-identities/index.ts(1 hunks)apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/edit-external-id/index.tsx(1 hunks)apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/hooks/use-edit-external-id.ts(1 hunks)apps/dashboard/app/(app)/identities/[identityId]/page.tsx(1 hunks)apps/dashboard/components/ui/combobox.tsx(1 hunks)apps/dashboard/lib/trpc/routers/identity/create.ts(1 hunks)apps/dashboard/lib/trpc/routers/identity/query.ts(1 hunks)apps/dashboard/lib/trpc/routers/index.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- apps/dashboard/app/(app)/apis/[apiId]/_components/create-key/components/general-setup.tsx
- apps/dashboard/lib/trpc/routers/index.ts
- apps/dashboard/app/(app)/apis/[apiId]/_components/create-key/hooks/use-fetch-identities/index.ts
- apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/hooks/use-edit-external-id.ts
- apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/edit-external-id/index.tsx
🧰 Additional context used
🧬 Code Graph Analysis (1)
apps/dashboard/app/(app)/apis/[apiId]/_components/create-key/hooks/use-create-identity.ts (2)
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/hooks/use-edit-external-id.ts (1)
onSuccess(8-35)apps/dashboard/lib/trpc/server.ts (1)
trpc(7-14)
🔇 Additional comments (2)
apps/dashboard/app/(app)/identities/[identityId]/page.tsx (1)
183-183: Good addition: Added query result limit.Adding the
limit: 50parameter to theclickhouse.verifications.latestcall is a sensible improvement that prevents potentially unbounded query results. This improves performance and resource utilization.apps/dashboard/app/(app)/apis/[apiId]/_components/create-key/components/external-id-field.tsx (1)
54-58: Verify state updates for search value and selected identityThe current implementation uses separate handlers for input changes and selection, which could lead to inconsistent state:
- When typing in the search box, only
searchValueis updated (not the external value)- When selecting an item, only the external value is updated (via
onChange)This approach works but depends on the
FormComboboxcomponent's internal behavior. Ensure that the search value is properly reset when an item is selected.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
apps/dashboard/components/ui/combobox.tsx (2)
123-126:⚠️ Potential issueMissing accessibility attributes for combobox
The combobox is still missing required ARIA attributes for proper screen reader support.
As mentioned in a previous review, a combobox needs
aria-controls(pointing to the listbox) andaria-labelledby(pointing to a visible label). Apply this diff:// biome-ignore lint/a11y/useSemanticElements: <explanation> role="combobox" aria-expanded={open} + aria-controls={`${id || "combobox"}-listbox`} + aria-labelledby={`${id || "combobox"}-label`}And add the corresponding id to the CommandList:
- <CommandList className="max-h-[300px] overflow-y-auto overflow-x-hidden"> + <CommandList id={`${id || "combobox"}-listbox`} className="max-h-[300px] overflow-y-auto overflow-x-hidden">You'll also need to ensure there's a properly associated label element with the matching ID.
141-147: 🛠️ Refactor suggestionAdd visible label for screen readers
For proper accessibility, there should be a visible label that is referenced by
aria-labelledby.{selectedOption ? ( <div className="py-0 w-full"> + <span id={`${id || "combobox"}-label`}> {selectedOption.selectedLabel || selectedOption.label} + </span> </div> ) : ( + <span id={`${id || "combobox"}-label`}> {placeholder} + </span> )}
🧹 Nitpick comments (6)
apps/dashboard/components/ui/combobox.tsx (6)
123-123: Add meaningful explanation for lint rule ignoreThe biome-ignore comment doesn't provide a meaningful explanation for why this accessibility rule is being ignored.
- // biome-ignore lint/a11y/useSemanticElements: <explanation> + // biome-ignore lint/a11y/useSemanticElements: Using role="combobox" on a button is intentional for this custom dropdown implementation
152-187: Set min-width directly on Command componentThe PopoverContent width calculations might not always work as expected with dynamic content.
Setting the width directly on the Command component as well can help ensure proper sizing:
<PopoverContent className="p-0 w-full min-w-[var(--radix-popover-trigger-width)] rounded-lg border border-grayA-4 bg-white dark:bg-black shadow-md z-50"> - <Command> + <Command className="w-full">
164-165: Prevent empty state flash during searchWhen typing quickly in the search box, users might briefly see the empty state message before results filter.
Consider adding a small delay before showing the empty state:
<CommandList id={`${id || "combobox"}-listbox`} className="max-h-[300px] overflow-y-auto overflow-x-hidden"> - <CommandEmpty>{emptyMessage}</CommandEmpty> + <CommandEmpty className="py-2 px-3 text-sm text-grayA-11">{emptyMessage}</CommandEmpty>Also consider adding a slight transition effect to improve the user experience.
154-163: Improve keyboard handling for screenreader usersThe current keyboard handling stops propagation for most keys, which may interfere with some assistive technologies.
Consider refining the keyboard handler to only prevent propagation for arrow keys:
<CommandInput onInput={onChange} onKeyDown={(e) => { - if (e.key !== "Enter" && e.key !== " ") { + // Only stop propagation for navigation keys to prevent interfering with screen readers + if (["ArrowUp", "ArrowDown", "Home", "End", "PageUp", "PageDown"].includes(e.key)) { e.stopPropagation(); } }} placeholder={searchPlaceholder} className="text-xs placeholder:text-xs placeholder:text-accent-8" />
115-151: Add keyboard shortcut to open/close comboboxFor better keyboard accessibility, users should be able to open/close the combobox with keyboard shortcuts.
Add a keyboard handler to the button:
<Button variant="outline" // biome-ignore lint/a11y/useSemanticElements: Using role="combobox" on a button is intentional for this custom dropdown implementation role="combobox" aria-expanded={open} disabled={disabled} id={id} aria-describedby={ariaDescribedby} aria-invalid={ariaInvalid} aria-required={ariaRequired} + onKeyDown={(e) => { + // Open dropdown on Alt+ArrowDown, which is a common pattern + if (e.altKey && e.key === "ArrowDown") { + e.preventDefault(); + setOpen(true); + } + // Close dropdown on Escape + else if (e.key === "Escape" && open) { + e.preventDefault(); + setOpen(false); + } + }} className={cn(
169-175: Enhance command item to support keyboard focus indicationCommand items should visually indicate when they are focused via keyboard for better accessibility.
<CommandItem key={option.value} value={option.searchValue || option.value} onSelect={() => { onSelect(option.value); setOpen(false); }} - className="flex items-center py-0.5" + className="flex items-center py-0.5 data-[focus=true]:bg-accent-3 outline-none" >
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (5)
apps/dashboard/app/(app)/apis/[apiId]/_components/create-key/components/external-id-field.tsx(1 hunks)apps/dashboard/app/(app)/apis/[apiId]/_components/create-key/hooks/use-fetch-identities/create-identity-options.tsx(1 hunks)apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/edit-external-id/index.tsx(1 hunks)apps/dashboard/components/ui/combobox.tsx(1 hunks)apps/dashboard/components/ui/command.tsx(9 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/dashboard/app/(app)/apis/[apiId]/_components/create-key/hooks/use-fetch-identities/create-identity-options.tsx
- apps/dashboard/app/(app)/apis/[apiId]/_components/create-key/components/external-id-field.tsx
- apps/dashboard/components/ui/command.tsx
- apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/edit-external-id/index.tsx
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: Test Go API Local / Test (Shard 5/8)
- GitHub Check: Test Go API Local / Test (Shard 4/8)
- GitHub Check: Test Go API Local / Test (Shard 3/8)
- GitHub Check: Build / Build
- GitHub Check: Test Go API Local / Test (Shard 2/8)
- GitHub Check: Test Go API Local / Test (Shard 1/8)
- GitHub Check: Test API / API Test Local
🔇 Additional comments (1)
apps/dashboard/components/ui/combobox.tsx (1)
157-159: Consider behavior when Enter is pressed in the search inputCurrently, if a user presses Enter in the search input, the event propagates which might lead to unexpected behavior.
Verify the desired behavior when Enter is pressed in the search input. Typically, this would select the first item in the filtered list.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
internal/ui/src/components/checkbox.tsx (4)
10-183: Well-structured styling system with comprehensive variantsThe styling system is well-organized with clear variant definitions covering different visual styles, colors, and sizes. The compound variants appropriately handle the complex styling needs for different combinations and states.
However, consider extracting these extensive style definitions to a separate file to improve maintainability as the component library grows.
276-287: Consider simplifying variant mapping logicThe current variant mapping logic is somewhat complex. Consider simplifying it to improve readability and maintainability.
- let mappedVariant: CheckboxVariant = "primary"; - let mappedColor: CheckboxColor = color; - - if (variant === null || variant === undefined) { - mappedVariant = "primary"; - } else if (VARIANT_MAP[variant as keyof typeof VARIANT_MAP]) { - const mapping = VARIANT_MAP[variant as keyof typeof VARIANT_MAP]; - mappedVariant = mapping.variant; - if (mapping.color) { - mappedColor = mapping.color; - } - } else { - mappedVariant = variant as CheckboxVariant; - } + // Default to primary variant + let mappedVariant: CheckboxVariant = "primary"; + // Start with provided color + let mappedColor: CheckboxColor = color; + + // If a valid variant is provided + if (variant && typeof variant === 'string') { + // Check if it's a special mapping + const mapping = VARIANT_MAP[variant as keyof typeof VARIANT_MAP]; + if (mapping) { + mappedVariant = mapping.variant; + // Override color if specified in mapping + mappedColor = mapping.color || mappedColor; + } else { + // Use the provided variant directly + mappedVariant = variant as CheckboxVariant; + } + }
290-293: Consider moving checkmark color to compound variantsThe checkmark color logic makes assumptions about the background color based on variant and color. This might be better handled through compound variants to keep styling concerns centralized.
You could add compound variants for the checkmark color and then simply reference them here, making the component more maintainable if the visual design changes.
1-318: Consider adding unit tests for this componentThis is a foundational UI component that will likely be used across many features. I recommend adding comprehensive unit tests to ensure it behaves correctly across all variants and states.
Tests should cover:
- Rendering in different variants, colors, and sizes
- Proper state changes when clicked
- Accessibility features like keyboard navigation
- Proper ref forwarding
- Event handler invocation
Would you like me to provide a template for unit tests for this component?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/keys-table-action.popover.tsx(1 hunks)apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/skeletons.tsx(2 hunks)internal/ui/src/components/checkbox.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/keys-table-action.popover.tsx
- apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/skeletons.tsx
🧰 Additional context used
🧬 Code Graph Analysis (1)
internal/ui/src/components/checkbox.tsx (3)
internal/icons/src/props.ts (1)
IconProps(30-35)apps/dashboard/lib/utils.ts (1)
cn(5-7)internal/icons/src/icons/check.tsx (1)
Check(15-38)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: Test Go API Local / Test (Shard 5/8)
- GitHub Check: Test Go API Local / Test (Shard 4/8)
- GitHub Check: Test Go API Local / Test (Shard 3/8)
- GitHub Check: Test API / API Test Local
- GitHub Check: Test Go API Local / Test (Shard 2/8)
- GitHub Check: Build / Build
- GitHub Check: Test Go API Local / Test (Shard 1/8)
🔇 Additional comments (6)
internal/ui/src/components/checkbox.tsx (6)
1-8: LGTM: Strong foundation with proper importsGood choice using Radix UI's CheckboxPrimitive for accessibility and cva for styling variants. The client directive ensures this component works correctly in a Next.js application.
185-188: Potential type inconsistency in VARIANT_MAPThe
VARIANT_MAPincludes 'default' and 'destructive' keys, but these aren't explicitly defined in theCheckboxVarianttype. This could lead to type safety issues.- const VARIANT_MAP: Record<string, { variant: CheckboxVariant; color?: CheckboxColor }> = { + type VariantMapKey = 'default' | 'destructive'; + const VARIANT_MAP: Record<VariantMapKey, { variant: CheckboxVariant; color?: CheckboxColor }> = { default: { variant: "primary" }, destructive: { variant: "primary", color: "danger" }, };
190-203: Good utility function for mapping sizesThe
getIconSizefunction properly maps checkbox sizes to icon sizes, improving maintainability by centralizing this logic.
205-258: Excellent prop documentation with JSDoc commentsThe detailed TypeScript types with JSDoc comments provide clear documentation for component consumers. This is essential for a reusable UI component.
295-312: Well implemented component with proper ref forwardingThe component implementation correctly forwards refs and properly maps all the variant properties. Good use of the
cnutility for class composition.
316-316: Good practice setting display nameSetting the component's display name to match the Radix primitive improves debugging experience in React DevTools.
2 participants
What does this PR do?
Fixes # (issue)
If there is not an issue for this, please create one first. This is used to tracking purposes and also helps use understand why this PR exists
Type of change
How should this be tested?
Checklist
Required
pnpm buildpnpm fmtconsole.logsgit pull origin mainAppreciated
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Documentation
Chores