Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
📝 WalkthroughWalkthroughThis change set introduces a major refactor and feature expansion for the API settings page in the dashboard application. Several new React components are added for managing API settings, including API name updates, API ID copying, default configuration, IP whitelist management, delete protection, and deletion. The settings UI is consolidated into a single Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SettingsClient
participant SubComponent as [Settings Subcomponents]
participant API
User->>SettingsClient: Visit API Settings Page
SettingsClient->>SubComponent: Render settings sections (API name, ID, defaults, IP whitelist, delete protection, delete)
User->>SubComponent: Interact with forms/buttons (e.g., update name, copy ID, toggle protection)
SubComponent->>API: Submit changes via mutation (e.g., update, delete, toggle)
API-->>SubComponent: Respond with success/error
SubComponent-->>User: Show toast/feedback, refresh UI as needed
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (18)
✨ 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:
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! 🙏 |
…yed/unkey into new-ratelimit-settings-page
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
apps/dashboard/app/(app)/apis/[apiId]/settings/components/delete-api.tsx (4)
74-74: Remove console.error in production code.The error logging to the console should be handled through a proper logging service rather than using
console.errorin production code.- console.error(err); + // Use a proper logging service or remove this line
116-117: Improve button width for better responsiveness.The current button width is very large on mobile (24rem) and much smaller on desktop (8rem), which seems counterintuitive since mobile screens are typically smaller.
- className="rounded-lg justify-end items-end px-3 text-error-9 font-medium text-[13px] leading-6 w-[24rem] lg:w-[8rem]" + className="rounded-lg justify-end items-end px-3 text-error-9 font-medium text-[13px] leading-6 w-full sm:w-[12rem] lg:w-[8rem]"
128-128: Fix typo in className.There's a typo in the className where there's no space between
items-centerandw-full.- <div className="flex flex-row justify-between w-full h-16 px-6 py-4 items-centerw-full"> + <div className="flex flex-row justify-between w-full h-16 px-6 py-4 items-center w-full">
181-196: Consider using a more semantic color for the delete button.While using error-9 for the delete button color is appropriate, consider defining a specific semantic color for destructive actions in your design system for better consistency across the application.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
apps/dashboard/app/(app)/apis/[apiId]/settings/components/copy-api-id.tsx(1 hunks)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/app/(app)/apis/[apiId]/settings/components/delete-api.tsx(1 hunks)apps/dashboard/app/(app)/apis/[apiId]/settings/components/delete-protection.tsx(1 hunks)apps/dashboard/app/(app)/apis/[apiId]/settings/components/update-api-name.tsx(1 hunks)apps/dashboard/app/(app)/apis/[apiId]/settings/components/update-ip-whitelist.tsx(1 hunks)apps/dashboard/components/navigation/sidebar/app-sidebar/index.tsx(2 hunks)apps/dashboard/components/settings-card.tsx(2 hunks)internal/icons/src/index.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (8)
- apps/dashboard/app/(app)/apis/[apiId]/settings/components/default-prefix.tsx
- apps/dashboard/components/settings-card.tsx
- apps/dashboard/app/(app)/apis/[apiId]/settings/components/default-bytes.tsx
- apps/dashboard/app/(app)/apis/[apiId]/settings/components/copy-api-id.tsx
- apps/dashboard/app/(app)/apis/[apiId]/settings/components/update-ip-whitelist.tsx
- apps/dashboard/app/(app)/apis/[apiId]/settings/components/update-api-name.tsx
- internal/icons/src/index.ts
- apps/dashboard/app/(app)/apis/[apiId]/settings/components/delete-protection.tsx
🧰 Additional context used
🧬 Code Graph Analysis (1)
apps/dashboard/app/(app)/apis/[apiId]/settings/components/delete-api.tsx (6)
apps/dashboard/lib/trpc/routers/index.ts (1)
router(84-216)apps/dashboard/lib/trpc/routers/api/delete.ts (1)
deleteApi(7-117)apps/dashboard/components/settings-card.tsx (1)
SettingCard(12-51)apps/dashboard/app/(app)/apis/[apiId]/settings/components/status-badge.tsx (1)
StatusBadge(28-35)internal/icons/src/icons/lock.tsx (1)
Lock(15-61)internal/icons/src/icons/xmark.tsx (1)
XMark(15-42)
⏰ Context from checks skipped due to timeout of 90000ms (19)
- GitHub Check: Test Go API Local / Test (Shard 8/8)
- GitHub Check: Test Go API Local / Test (Shard 7/8)
- GitHub Check: Test Go API Local / Test (Shard 4/8)
- GitHub Check: Test Go API Local / Test (Shard 3/8)
- GitHub Check: Test Go API Local / Test (Shard 6/8)
- GitHub Check: Test Packages / Test ./apps/dashboard
- GitHub Check: Test Packages / Test ./packages/hono
- GitHub Check: Test Go API Local / Test (Shard 5/8)
- GitHub Check: Test Packages / Test ./packages/rbac
- GitHub Check: Test Go API Local / Test (Shard 2/8)
- GitHub Check: Test Packages / Test ./internal/clickhouse
- GitHub Check: Test Go API Local / Test (Shard 1/8)
- GitHub Check: Test Packages / Test ./internal/keys
- GitHub Check: Test Packages / Test ./internal/billing
- GitHub Check: Test API / API Test Local
- GitHub Check: Build / Build
- GitHub Check: Test Agent Local / test_agent_local
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: autofix
🔇 Additional comments (7)
apps/dashboard/components/navigation/sidebar/app-sidebar/index.tsx (3)
20-21: Appropriate import updates.The import statements have been properly updated to include
useRouterfromnext/navigationanduseEffectfrom React, which are necessary for the new workspace change detection functionality.
33-33: LGTM!Properly initializing the router using the Next.js
useRouterhook.
35-42:Details
✅ Verification successful
Consider a more targeted refresh approach.
While the implementation correctly refreshes the sidebar when the workspace changes, the comment on line 40 mentions using "a more targeted approach" but still uses the generic
router.refresh(). This might cause unnecessary re-renders of other components.Consider investigating if a more granular refresh method is available that would only update the sidebar components affected by workspace changes, rather than triggering a full router refresh. This would be especially beneficial for performance in larger applications.
🏁 Script executed:
#!/bin/bash # Check if there are any custom hooks or context providers that might offer more targeted refresh rg -A 3 -B 3 "createContext|useContext|refresh" --glob "*.tsx" apps/dashboard/Length of output: 61660
No granular refresh available — retaining router.refresh()
I searched the dashboard code for any existing hook or context that would allow a sidebar‑only update and found none. As it stands,
router.refresh()is the only mechanism to re-fetch/sidebar state when the workspace changes.– File:
apps/dashboard/components/navigation/sidebar/app-sidebar/index.tsx
– Lines: 35–42apps/dashboard/app/(app)/apis/[apiId]/settings/components/delete-api.tsx (4)
46-84: Well-implemented deletion confirmation flow.The component correctly implements a robust deletion confirmation process with appropriate validations:
- Requires the user to type the exact API name
- Requires confirming intent by typing a dynamically generated message that includes the number of keys
- Disables the button until both validations pass
- Handles API deletion through a proper mutation with success/error handling
- Redirects to the API listing page after successful deletion
The approach ensures users are fully aware of the consequences before proceeding with deletion.
90-109: Good use of the SettingCard component with proper UX indicators.The implementation effectively uses SettingCard to maintain consistency with other settings components and provides clear visual indication when delete protection is enabled through the Lock badge. The description accurately explains the delete protection feature's purpose.
124-199: Well-structured confirmation dialog with clear warnings.The confirmation dialog is well-structured with:
- Clear title and closing mechanism
- Detailed warning about the consequences of deletion
- Two separate validation inputs with clear instructions
- Disabled submit button until validations pass
- Loading state during submission
- Final warning about the permanent nature of the action
This multi-step confirmation process helps prevent accidental deletions while providing clear feedback to users.
49-50: Good use of dynamic confirmation text.The dynamic generation of the confirmation intent string based on the number of keys is well implemented with proper singular/plural handling.
apps/dashboard/app/(app)/apis/[apiId]/settings/components/delete-api.tsx
Show resolved
Hide resolved
apps/dashboard/app/(app)/apis/[apiId]/settings/components/delete-protection.tsx
Outdated
Show resolved
Hide resolved
apps/dashboard/app/(app)/apis/[apiId]/settings/components/delete-api.tsx
Outdated
Show resolved
Hide resolved
apps/dashboard/app/(app)/apis/[apiId]/settings/components/update-ip-whitelist.tsx
Outdated
Show resolved
Hide resolved
apps/dashboard/app/(app)/apis/[apiId]/settings/components/update-ip-whitelist.tsx
Outdated
Show resolved
Hide resolved
chronark
left a comment
chronark
left a comment
There was a problem hiding this comment.
clicking on the background during this dialog does not close it, I don't know why but it works in main
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
internal/ui/src/components/button.tsx (1)
153-153: Fix incorrect comment for the info variant section.The comment incorrectly identifies the following section as "Success" when it should be "Info" since it defines the info variant styles.
- // Success + // Info
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
apps/dashboard/app/(app)/apis/[apiId]/settings/components/copy-api-id.tsx(1 hunks)apps/dashboard/app/(app)/apis/[apiId]/settings/components/default-bytes.tsx(2 hunks)apps/dashboard/app/(app)/apis/[apiId]/settings/components/default-prefix.tsx(3 hunks)apps/dashboard/app/(app)/apis/[apiId]/settings/components/delete-api.tsx(1 hunks)apps/dashboard/app/(app)/apis/[apiId]/settings/components/delete-protection.tsx(1 hunks)apps/dashboard/app/(app)/apis/[apiId]/settings/components/settings-client.tsx(1 hunks)apps/dashboard/app/(app)/apis/[apiId]/settings/components/update-api-name.tsx(1 hunks)apps/dashboard/app/(app)/apis/[apiId]/settings/components/update-ip-whitelist.tsx(1 hunks)apps/dashboard/app/(app)/apis/[apiId]/settings/page.tsx(2 hunks)apps/dashboard/app/(app)/apis/[apiId]/settings/update-api-name.tsx(0 hunks)apps/dashboard/app/(app)/apis/[apiId]/settings/update-ip-whitelist.tsx(0 hunks)internal/ui/src/components/button.tsx(2 hunks)
💤 Files with no reviewable changes (2)
- apps/dashboard/app/(app)/apis/[apiId]/settings/update-api-name.tsx
- apps/dashboard/app/(app)/apis/[apiId]/settings/update-ip-whitelist.tsx
🚧 Files skipped from review as they are similar to previous changes (9)
- apps/dashboard/app/(app)/apis/[apiId]/settings/page.tsx
- apps/dashboard/app/(app)/apis/[apiId]/settings/components/settings-client.tsx
- apps/dashboard/app/(app)/apis/[apiId]/settings/components/default-prefix.tsx
- apps/dashboard/app/(app)/apis/[apiId]/settings/components/default-bytes.tsx
- apps/dashboard/app/(app)/apis/[apiId]/settings/components/copy-api-id.tsx
- apps/dashboard/app/(app)/apis/[apiId]/settings/components/update-api-name.tsx
- apps/dashboard/app/(app)/apis/[apiId]/settings/components/delete-api.tsx
- apps/dashboard/app/(app)/apis/[apiId]/settings/components/update-ip-whitelist.tsx
- apps/dashboard/app/(app)/apis/[apiId]/settings/components/delete-protection.tsx
⏰ Context from checks skipped due to timeout of 90000ms (11)
- GitHub Check: Test Go API Local / Test (Shard 1/8)
- GitHub Check: Test Go API Local / Test (Shard 8/8)
- GitHub Check: Test Packages / Test ./internal/resend
- GitHub Check: Test Packages / Test ./internal/encryption
- GitHub Check: Test Packages / Test ./internal/billing
- GitHub Check: Build / Build
- GitHub Check: Test API / API Test Local
- GitHub Check: Test Agent Local / test_agent_local
- GitHub Check: Docs
- GitHub Check: autofix
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
internal/ui/src/components/button.tsx (1)
42-42: LGTM on adding the info color variant.Good addition of the "info" color variant which follows the pattern of existing color variants. This enhances the button component's flexibility for the API settings page update.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
chronark
left a comment
chronark
left a comment
There was a problem hiding this comment.
Something broke I think, I'm pretty sure the delete api button was disabled before when protection is on, but now it's not the case anymore
The check in tRPC still catches it but, this is a regression I think?
https://share.cleanshot.com/GYtt3n0V
apps/dashboard/app/(app)/apis/[apiId]/settings/components/default-bytes.tsx
Show resolved
Hide resolved
apps/dashboard/app/(app)/apis/[apiId]/settings/components/default-bytes.tsx
Outdated
Show resolved
Hide resolved
|
yeah this worked in the past |
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
Enhancements
Bug Fixes
Refactor
Chores