-
Notifications
You must be signed in to change notification settings - Fork 539
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add rewrites and upserts for gateway.new #2779
Conversation
|
📝 WalkthroughWalkthroughThis pull request introduces enhancements to the dashboard application, focusing on a new Changes
Sequence DiagramsequenceDiagram
participant User
participant Browser
participant Middleware
participant GatewayNewPage
participant Database
User->>Browser: Access gateway.new
Browser->>Middleware: Request routing
Middleware->>GatewayNewPage: Route request
GatewayNewPage->>Database: Check existing workspace
alt No workspace exists
GatewayNewPage->>Database: Create new workspace
end
GatewayNewPage->>Browser: Redirect to API creation page
Possibly related PRs
Suggested Labels
Suggested Reviewers
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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! 🙏 |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (4)
apps/dashboard/app/(app)/apis/create-api-button.tsx (1)
42-42
: State initialization with defaultOpen is clear.
This ensures the dialog’s open state is controlled effectively upon component mount.Consider adding a simple memo or callback if additional logic depends on “open” to avoid unnecessary re-renders, though not essential here.
apps/dashboard/app/(app)/gateway-new/page.tsx (2)
24-32
: Consider transaction safety or upsert for concurrent requests.
If two requests arrive at nearly the same time, each not finding an existing workspace, you might create duplicates. A database-level lock or an upsert approach would help ensure exactly one workspace is created.Example snippet (PostgreSQL-like syntax):
INSERT INTO workspaces (id, name, tenantId, betaFeatures, features) VALUES (:id, 'Personal Workspace', :tenantId, '{}', '{}') ON CONFLICT (tenantId) DO NOTHING;
34-34
: “new=true” query param might be misleading if no new workspace was created.
If the workspace already exists, the user is still redirected to “/apis?new=true”. Consider informing the user differently when no new workspace was created.apps/dashboard/next.config.js (1)
53-63
: Path segment is ignored in rewrite.
Right now, any path under gateway.new redirects to “/gateway-new”, discarding the user’s original path. If you want to preserve or reference that path, consider adjusting the destination to “/gateway-new/:path”.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/dashboard/app/(app)/apis/create-api-button.tsx
(2 hunks)apps/dashboard/app/(app)/apis/page.tsx
(2 hunks)apps/dashboard/app/(app)/gateway-new/page.tsx
(1 hunks)apps/dashboard/middleware.ts
(1 hunks)apps/dashboard/next.config.js
(1 hunks)
🔇 Additional comments (10)
apps/dashboard/app/(app)/apis/create-api-button.tsx (4)
22-22
: Good addition of useState import.
This is necessary to handle local dialog state.
30-32
: Optional prop is relevant for dialog state initialization.
The new type definition clarifies usage for parent components, making the component interface more robust.
34-37
: Prop merge in function signature is straightforward and clear.
Combining button attributes with the new prop ensures backward compatibility and extensibility.
62-62
: Properly binding the dialog open state.
You handle the dialog’s visibility cleanly by assigning it to the local open state and the onOpenChange callback.
apps/dashboard/app/(app)/apis/page.tsx (3)
15-17
: Clear type definition for incoming search parameters.
Defining the optional “new” boolean in searchParams
improves readability and ensures type safety.
19-19
: Props-based function signature.
This clarifies that ApisOverviewPage
now depends on external search parameters, which can help future maintainers.
55-58
: Conditional dialog opening.
Using the condition (apis.length === 0 || props.searchParams.new)
effectively handles first-time or on-demand dialog display.
apps/dashboard/app/(app)/gateway-new/page.tsx (2)
19-22
: Workspace lookup logic is correct.
Looks good. The query constraints (no deletedAt, matching tenantId) make sense to identify a single active workspace.
16-18
: Handle missing tenantId more gracefully.
If getTenantId() ever returns null or undefined, the subsequent logic could fail silently. Consider validating the tenantId and possibly redirecting or throwing an error if it's not set.
apps/dashboard/middleware.ts (1)
76-76
: Included “/gateway-new” in the middleware matcher.
This is consistent with the new route. The addition ensures the same auth and workspace checks apply to the new path.
Summary by CodeRabbit
gateway.new
domain.CreateApiButton
component with a customizable default open state.gateway.new
domain.