fix: organization setup redirect if it didn't create correctly#309
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ |
WalkthroughThis pull request refactors two framework instance functions by converting them from asynchronous function declarations to exported constants wrapped with a caching mechanism. Additionally, a new, cached asynchronous function ( Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Layout
participant getOrganization
participant Database
Client->>Layout: Request layout with orgId
Layout->>getOrganization: Validate organization (using cache)
getOrganization->>Database: Query organization data
Database-->>getOrganization: Return data or error
getOrganization-->>Layout: Return organization data
alt Organization valid
Layout->>Client: Render dashboard layout
else Organization invalid or error
Layout->>Client: Redirect to "/setup"
end
Possibly related PRs
Poem
✨ 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/frameworks/page.tsx (1)
25-25: Consider removing console.log statement.Debug logs like this one should be removed before merging to production.
- console.log("Redirect on page.tsx");apps/app/src/app/[locale]/(app)/setup/page.tsx (1)
30-38: Add a comment explaining the redirect condition logic.The condition checks if the active organization isn't set in the session but an organization exists in the database. This logic might not be immediately clear to other developers.
+ // If user has an organization but it's not set as active in their session, + // set it as active and redirect to the organization's frameworks page if (!session?.session?.activeOrganizationId && organization?.id) { await auth.api.setActiveOrganization({ headers: headersList, body: { organizationId: organization.id, }, }); redirect(`/${organization.id}/frameworks`); }apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/layout.tsx (3)
20-26: Type definition improvement.The params type is defined as a Promise, which is unusual for Next.js route parameters. While this works, it would be clearer to define it as a direct object type.
export default async function Layout({ children, params, }: { children: React.ReactNode; - params: Promise<{ orgId: string }>; + params: { orgId: string }; }) {
58-75: Well-implemented cached organization validation function.The
getOrganizationfunction:
- Uses React's cache mechanism to avoid duplicate database queries
- Properly handles database errors with a try/catch block
- Includes appropriate redirects to the setup page when an organization doesn't exist
Consider adding TypeScript return type annotation for better type safety.
-const getOrganization = cache(async (orgId: string) => { +const getOrganization = cache(async (orgId: string): Promise<{ id: string; [key: string]: any }> => {
58-75: Enhanced error handling opportunity.While the error handling redirects users appropriately, logging only displays the error without context. Consider enhancing the error logging to include the orgId for easier debugging.
} catch (error) { - console.error(error); + console.error(`Error fetching organization with ID ${orgId}:`, error); redirect("/setup"); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/frameworks/data/getAllFrameworkInstancesWithControls.ts(1 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/frameworks/data/getSingleFrameworkInstanceWithControls.ts(1 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/frameworks/page.tsx(1 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/layout.tsx(1 hunks)apps/app/src/app/[locale]/(app)/setup/page.tsx(1 hunks)
🧰 Additional context used
🧬 Code Definitions (5)
apps/app/src/app/[locale]/(app)/setup/page.tsx (2)
apps/app/src/app/[locale]/(app)/setup/components/accept-invite.tsx (1)
AcceptInvite(9-55)apps/app/src/components/forms/create-organization-form.tsx (1)
OnboardingClient(33-218)
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/frameworks/page.tsx (4)
apps/app/src/utils/auth.ts (1)
auth(11-86)apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/frameworks/data/getAllFrameworkInstancesWithControls.ts (1)
getAllFrameworkInstancesWithControls(7-34)apps/app/src/components/pages/PageWithBreadcrumb.tsx (1)
PageWithBreadcrumb(42-135)apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/frameworks/components/FrameworksOverview.tsx (1)
FrameworksOverview(9-26)
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/layout.tsx (3)
apps/app/src/utils/auth.ts (1)
auth(11-86)apps/app/src/components/sidebar.tsx (1)
Sidebar(10-56)apps/app/src/components/header.tsx (1)
Header(16-66)
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/frameworks/data/getAllFrameworkInstancesWithControls.ts (1)
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/frameworks/types.ts (1)
FrameworkInstanceWithControls(10-18)
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/frameworks/data/getSingleFrameworkInstanceWithControls.ts (1)
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/frameworks/types.ts (1)
FrameworkInstanceWithControls(10-18)
🔇 Additional comments (10)
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/frameworks/page.tsx (4)
5-5: Good change to use absolute imports.Converting from relative to absolute import path for
PageWithBreadcrumbimproves maintainability and readability.
10-14: Improved formatting of metadata.The consistent indentation in the
generateMetadatafunction enhances code readability.
18-20: Clean formatting of session retrieval.The adjusted indentation makes the session retrieval code more consistent with the rest of the codebase.
33-37: Well-formatted return statement.The adjusted indentation in the return statement improves code readability.
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/frameworks/data/getSingleFrameworkInstanceWithControls.ts (1)
3-3: Great optimization with React cache.Using React's
cachefunction to memoize database queries is an excellent performance optimization. It will prevent redundant database calls when the function is called multiple times with the same parameters across rendering cycles.Also applies to: 7-37
apps/app/src/app/[locale]/(app)/setup/page.tsx (1)
20-28: Good improvement to explicitly fetch the organization.Explicitly fetching the organization from the database rather than relying solely on the session state is a more robust approach. This ensures the organization actually exists in the database before attempting to redirect.
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/frameworks/data/getAllFrameworkInstancesWithControls.ts (1)
3-3: Great performance optimization with React cache.Using React's
cachefunction to memoize database queries is an excellent optimization. It will prevent redundant database calls when this function is called multiple times with the sameorganizationIdparameter across rendering cycles.This is consistent with the same optimization applied to
getSingleFrameworkInstanceWithControls.Also applies to: 7-34
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/layout.tsx (3)
7-11: Good addition of required imports.The new imports for
dbfrom "@comp/db" andcachefrom "react" are correctly added to support the new organization validation functionality.
27-42: Good session validation and organization redirection flow.The code correctly:
- Retrieves the session using auth headers
- Extracts the orgId from params
- Validates the organization exists via
getOrganization- Checks authentication status with proper redirects
This refactored flow ensures users can't access invalid organization pages.
31-32:Details
✅ Verification successful
Verify params unwrapping.
The
paramsobject is being treated as a Promise withawait, but depending on Next.js version and configuration, it might already be a plain object. If it's consistently a Promise, this is correct.
🏁 Script executed:
#!/bin/bash # Look for similar patterns in other route handlers to verify consistency rg -A 2 -B 2 "params.*orgId" --glob "*.tsx" --glob "*.ts" | grep -v "Promise"Length of output: 5653
Review Approved: Params Unwrapping Is Consistent
After reviewing similar route handler files across the codebase (e.g., in layout and page components), it’s clear that using
await paramsis a consistent pattern. Given that Next.js may returnparamsas a Promise in these contexts, the usage in:const orgId = (await params).orgId; await getOrganization(orgId);is valid and aligns with our implementation strategy. No changes are needed.
1 participant
Summary by CodeRabbit
New Features
Refactor