chore: use same schema for prefix and bytes #3284
Conversation
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
📝 Walkthrough""" WalkthroughValidation schemas for default bytes and default prefix settings were refactored to use centralized, reusable schemas. Error handling in both the frontend components and backend procedures was enhanced, providing more specific user feedback and centralizing error management logic. No changes were made to public entity signatures. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI_Component as DefaultBytes/DefaultPrefix Component
participant API as TRPC API Procedure
participant DB as Database
User->>UI_Component: Submit new defaultBytes/defaultPrefix
UI_Component->>UI_Component: Validate input with reusable schema
UI_Component->>API: Send mutation request
API->>API: Validate input with reusable schema
API->>DB: Update keyAuth record
alt Success
API-->>UI_Component: Return success
UI_Component-->>User: Show success toast with details
else Error (NOT_FOUND/INTERNAL_SERVER_ERROR/BAD_REQUEST)
API-->>UI_Component: Return error with code
UI_Component-->>User: Show specific error toast
else Other Error
API-->>UI_Component: Return generic error
UI_Component-->>User: Show fallback error toast with support link
end
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms (5)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Thank you for following the naming conventions for pull request titles! 🙏 |
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
apps/dashboard/lib/trpc/routers/api/setDefaultPrefix.ts (1)
18-84: 🛠️ Refactor suggestionInconsistent error handling pattern with setDefaultBytes.ts.
This procedure still uses the old
.catch()pattern for error handling, whilesetDefaultBytes.tshas been refactored to use a cleanertry-catchapproach. Consider updating this for consistency.Apply this refactor to match the pattern in
setDefaultBytes.ts:.mutation(async ({ ctx, input }) => { - const keyAuth = await db.query.keyAuth - .findFirst({ - where: (table, { eq, and, isNull }) => - and( - eq(table.workspaceId, ctx.workspace.id), - eq(table.id, input.keyAuthId), - isNull(table.deletedAtM), - ), - }) - .catch((_err) => { - throw new TRPCError({ - code: "INTERNAL_SERVER_ERROR", - message: - "We were unable to update the key auth. Please try again or contact support@unkey.dev", - }); - }); + const keyAuth = await db.query.keyAuth.findFirst({ + where: (table, { eq, and, isNull }) => + and( + eq(table.workspaceId, ctx.workspace.id), + eq(table.id, input.keyAuthId), + isNull(table.deletedAtM), + ), + }); + if (!keyAuth) { throw new TRPCError({ code: "NOT_FOUND", message: "We are unable to find the correct key auth. Please try again or contact support@unkey.dev.", }); } - await db - .transaction(async (tx) => { + try { + await db.transaction(async (tx) => { await tx .update(schema.keyAuth) .set({ defaultPrefix: input.defaultPrefix, }) - .where(eq(schema.keyAuth.id, keyAuth.id)) - .catch((_err) => { - throw new TRPCError({ - code: "INTERNAL_SERVER_ERROR", - message: - "We were unable to update the API default prefix. Please try again or contact support@unkey.dev.", - }); - }); + .where(eq(schema.keyAuth.id, keyAuth.id)); + await insertAuditLogs(tx, { workspaceId: ctx.workspace.id, actor: { type: "user", id: ctx.user.id, }, event: "api.update", description: `Changed ${keyAuth.id} default prefix from ${keyAuth.defaultPrefix} to ${input.defaultPrefix}`, resources: [ { type: "keyAuth", id: keyAuth.id, }, ], context: { location: ctx.audit.location, userAgent: ctx.audit.userAgent, }, }); - }) - .catch((_err) => { + }); + } catch (err) { + console.error(err); throw new TRPCError({ code: "INTERNAL_SERVER_ERROR", message: "We were unable to update the default prefix. Please try again or contact support@unkey.dev.", }); - }); + } });
🧹 Nitpick comments (1)
apps/dashboard/app/(app)/apis/[apiId]/settings/components/default-prefix.tsx (1)
51-69: Consider adding BAD_REQUEST error handling for consistency.The error handling is good, but it's missing the
BAD_REQUESTcase that's handled in thedefault-bytes.tsxcomponent. Consider adding it for consistency.Add this error case between the
INTERNAL_SERVER_ERRORand fallback cases:} else if (err.data?.code === "INTERNAL_SERVER_ERROR") { toast.error("Server Error", { description: "We encountered an issue while updating the default prefix. Please try again later or contact support at support@unkey.dev", }); + } else if (err.data?.code === "BAD_REQUEST") { + toast.error("Invalid Configuration", { + description: `Please check your prefix settings. ${err.message || ""}`, + }); } else {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/dashboard/app/(app)/apis/[apiId]/settings/components/default-bytes.tsx(2 hunks)apps/dashboard/app/(app)/apis/[apiId]/settings/components/default-prefix.tsx(2 hunks)apps/dashboard/lib/trpc/routers/api/setDefaultBytes.ts(2 hunks)apps/dashboard/lib/trpc/routers/api/setDefaultPrefix.ts(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
apps/dashboard/lib/trpc/routers/api/setDefaultPrefix.ts (1)
apps/dashboard/app/(app)/apis/[apiId]/_components/create-key/create-key.schema.ts (1)
keyPrefixSchema(27-37)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: Test Packages / Test ./packages/nextjs
- GitHub Check: Test Packages / Test ./packages/cache
- GitHub Check: Test Packages / Test ./packages/api
- GitHub Check: Test Packages / Test ./internal/clickhouse
- GitHub Check: autofix
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (13)
apps/dashboard/lib/trpc/routers/api/setDefaultPrefix.ts (2)
4-4: LGTM: Schema centralization improves maintainability.Good move to import the centralized
keyPrefixSchemato maintain consistency across the application.
14-14:❌ Incorrect review comment
Verify the
.pipe(z.string())transformation.The
keyPrefixSchemais defined as optional (.optional()in the schema), but piping it toz.string()makes it required. Please confirm this is the intended behavior, as it changes the validation contract.
🏁 Script executed:
#!/bin/bash # Description: Verify how keyPrefixSchema is used across the codebase to ensure consistency # Search for other usages of keyPrefixSchema to check if .pipe(z.string()) is used consistently rg -A 2 -B 2 "keyPrefixSchema" --type tsLength of output: 3538
Ignore the optionalness concern.
The
keyPrefixSchemais defined as a requiredz.string().max(8)(no.optional()), so.pipe(z.string())does not alter its requiredness or change the validation contract. This usage is consistent in:
create-key.schema.ts(defineskeyPrefixSchema)default-prefix.tsx(client-side form schema)setDefaultPrefix.ts(TRPC input schema)No changes are needed.
Likely an incorrect or invalid review comment.
apps/dashboard/app/(app)/apis/[apiId]/settings/components/default-bytes.tsx (4)
11-11: LGTM: Schema centralization implemented correctly.Good use of the centralized
keyBytesSchemato maintain consistency across the application.
15-15: LGTM: Clean schema usage.The direct usage of
keyBytesSchemais correct and simpler than the prefix component's.pipe(z.string())approach.
43-45: LGTM: Enhanced user feedback.The success toast now provides clear, descriptive feedback with both title and description, improving the user experience.
51-73: Excellent error handling implementation.The comprehensive error handling with specific error codes provides much better user feedback compared to generic error messages. The contact support action is a nice touch for unhandled errors.
apps/dashboard/app/(app)/apis/[apiId]/settings/components/default-prefix.tsx (3)
11-11: LGTM: Consistent schema centralization.Good implementation of the centralized
keyPrefixSchemaimport.
15-15: Consistent with backend implementation.The
.pipe(z.string())usage matches the backend procedure, maintaining consistency between frontend and backend validation.
43-45: LGTM: Improved success feedback.The enhanced success toast provides clear, descriptive feedback improving user experience.
apps/dashboard/lib/trpc/routers/api/setDefaultBytes.ts (4)
1-1: LGTM: Schema centralization implemented.Good use of the centralized
keyBytesSchemafor consistent validation.
13-13: LGTM: Clean schema usage.Direct usage of
keyBytesSchemais straightforward and maintains consistency.
18-25: LGTM: Simplified database query.Removing the
.catchblock allows errors to propagate naturally to the outer error handling, which is cleaner.
35-71: Excellent error handling refactor.The refactored error handling with explicit
try-catchis much cleaner than the previous nested.catch()approach. The error logging will help with debugging, and the centralized error message provides consistent user feedback.
What does this PR do?
Improves error handling and makes it more user friendly for bytes and prefix API settings updates. And, uses SSOT for schemas.
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