Revamp onboarding flow #376
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🔭 Outside diff range comments (1)
packages/web/src/app/[domain]/components/resubmitAccountRequestButton.tsx (1)
19-45: Missing router functionality and loading state fix.This component is missing the
useRouterimport androuter.refresh()call that exists in the newsubmitAccountRequestButton.tsx. It also has the same loading state management issue.If this component is being kept, add the missing functionality:
import { useToast } from "@/components/hooks/use-toast" import { createAccountRequest } from "@/actions" import { isServiceError } from "@/lib/utils" +import { useRouter } from "next/navigation" interface SubmitButtonProps { domain: string userId: string } export function SubmitAccountRequestButton({ domain, userId }: SubmitButtonProps) { const { toast } = useToast() + const router = useRouter() const [isSubmitting, setIsSubmitting] = useState(false) const handleSubmit = async () => { setIsSubmitting(true) const result = await createAccountRequest(userId, domain) if (!isServiceError(result)) { if (result.existingRequest) { toast({ title: "Request Already Submitted", description: "Your request to join the organization has already been submitted. Please wait for it to be approved.", variant: "default", }) } else { toast({ title: "Request Submitted", description: "Your request to join the organization has been submitted.", variant: "default", }) } + router.refresh() } else { toast({ title: "Failed to Submit", description: `There was an error submitting your request. Reason: ${result.message}`, variant: "destructive", }) } setIsSubmitting(false) }
🧹 Nitpick comments (15)
docs/docs/configuration/auth/faq.mdx (2)
14-14: Fix typo in the word "doesn't".There's a spelling error that should be corrected for professional documentation.
- Every user must register an account within your Sourcebot deployment. However, this dosn't mean their access + Every user must register an account within your Sourcebot deployment. However, this doesn't mean their access
24-24: Remove redundant preposition for better readability.The phrase "within in the database" contains a redundant preposition.
- This data does not leave your device and is stored within in the database managed by your deployment. If you're + This data does not leave your device and is stored within the database managed by your deployment. If you'repackages/web/src/lib/authProviders.ts (1)
8-18: Clean provider normalization with solid type safety.The implementation correctly handles both function and object providers, providing a consistent interface for consuming components.
Consider adding null checks for enhanced robustness:
export const getAuthProviders = (): AuthProvider[] => { const providers = getProviders(); return providers.map((provider) => { if (typeof provider === "function") { const providerInfo = provider(); - return { id: providerInfo.id, name: providerInfo.name }; + return { id: providerInfo?.id || '', name: providerInfo?.name || '' }; } else { - return { id: provider.id, name: provider.name }; + return { id: provider?.id || '', name: provider?.name || '' }; } }); };docs/docs/configuration/auth/inviting-members.mdx (1)
18-18: Consider using more formal language for documentation.The static analysis tool suggests using "invitation link" instead of "invite link" for formal documentation writing.
-If member approval is required, an owner of the deployment can enable an invite link. When enabled, users +If member approval is required, an owner of the deployment can enable an invitation link. When enabled, userspackages/web/src/app/onboard/components/completeOnboardingButton.tsx (2)
22-30: Refactor duplicated error handling for DRY principle.The error toast notification is duplicated in both the service error handling and catch block. Consider extracting this into a reusable function.
+ const showErrorToast = () => { + toast({ + title: "Error", + description: "Failed to complete onboarding. Please try again.", + variant: "destructive", + }) + setIsLoading(false) + } const handleCompleteOnboarding = async () => { setIsLoading(true) try { const result = await completeOnboarding(SINGLE_TENANT_ORG_DOMAIN) if (isServiceError(result)) { - toast({ - title: "Error", - description: "Failed to complete onboarding. Please try again.", - variant: "destructive", - }) - setIsLoading(false) + showErrorToast() return } router.push("/") } catch (error) { console.error("Error completing onboarding:", error) - toast({ - title: "Error", - description: "Failed to complete onboarding. Please try again.", - variant: "destructive", - }) - setIsLoading(false) + showErrorToast() } }Also applies to: 33-41
34-34: Enhance error logging with more context.Consider adding more context to the error log to aid debugging.
- console.error("Error completing onboarding:", error) + console.error("Error completing onboarding for domain:", SINGLE_TENANT_ORG_DOMAIN, error)packages/web/src/app/invite/components/joinOrganizationButton.tsx (1)
46-54: Consider extracting inline styles to CSS classes.The button uses CSS variables directly in the className. While functional, consider extracting these styles to a CSS class for better maintainability.
- className="w-full h-11 bg-[var(--primary)] hover:bg-[var(--primary)]/90 text-[var(--primary-foreground)] transition-all duration-200 font-medium" + className="w-full h-11 btn-primary font-medium"Then define in your CSS:
.btn-primary { background-color: var(--primary); color: var(--primary-foreground); transition: all 200ms; } .btn-primary:hover { background-color: hsl(from var(--primary) h s l / 0.9); }docs/docs/configuration/auth/providers.mdx (1)
16-25: Fix repetitive sentence structure for better readability.The static analysis tool correctly identified repetitive sentence beginnings with "Email". Consider rewording for better flow.
- Email codes are 6 digit codes sent to a provided email. Email codes are enabled when transactional emails are configured using the following environment variables: + Email codes are 6 digit codes sent to a provided email. These codes are enabled when transactional emails are configured using the following environment variables:packages/web/src/app/[domain]/components/submitAccountRequestButton.tsx (1)
55-55: Remove unnecessary hidden input.The hidden input for domain is redundant since the domain is already available from the component props and URL context.
- <input type="hidden" name="domain" value={domain} />packages/web/src/app/components/providerButton.tsx (1)
8-14: Simplify logo prop structure.The logo prop structure could be simplified since the className is optional and could be handled more elegantly.
interface ProviderButtonProps { name: string; - logo: { src: string, className?: string } | null; + logo?: { src: string; alt?: string; className?: string }; onClick: () => void; className?: string; context: "login" | "signup"; }Then update the usage:
- {logo && <Image src={logo.src} alt={name} className={cn("w-5 h-5 mr-2", logo.className)} />} + {logo && <Image src={logo.src} alt={logo.alt || name} className={cn("w-5 h-5 mr-2", logo.className)} />}packages/web/src/app/[domain]/components/pendingApproval.tsx (2)
48-55: Replace inline animation styles with CSS classes.Inline styles for animation delays aren't ideal for maintainability and theme consistency.
Create CSS classes for the animation delays:
+ <div className="flex items-center justify-center space-x-2 text-sm text-muted-foreground"> + <div className="flex space-x-1"> + <div className="w-2 h-2 bg-[var(--chart-1)] rounded-full animate-pulse [animation-delay:0s]"></div> + <div className="w-2 h-2 bg-[var(--chart-1)]/60 rounded-full animate-pulse [animation-delay:0.2s]"></div> + <div className="w-2 h-2 bg-[var(--chart-1)]/30 rounded-full animate-pulse [animation-delay:0.4s]"></div> - <div className="flex items-center justify-center space-x-2 text-sm text-muted-foreground"> - <div className="flex space-x-1"> - <div className="w-2 h-2 bg-[var(--chart-1)] rounded-full animate-pulse"></div> - <div className="w-2 h-2 bg-[var(--chart-1)]/60 rounded-full animate-pulse" style={{ animationDelay: '0.2s' }}></div> - <div className="w-2 h-2 bg-[var(--chart-1)]/30 rounded-full animate-pulse" style={{ animationDelay: '0.4s' }}></div> </div> <span>Awaiting review</span> </div>Or define proper CSS classes in your stylesheet for better maintainability.
32-36: Replace inline SVG with icon component.The inline SVG should be replaced with a proper icon component for consistency and maintainability.
- <div className="inline-flex items-center justify-center w-16 h-16 rounded-full bg-[var(--chart-4)]/20 mx-auto"> - <svg className="w-8 h-8 text-[var(--chart-4)]" fill="none" stroke="currentColor" viewBox="0 0 24 24"> - <path strokeLinecap="round" strokeLinejoin="round" strokeWidth={2} d="M12 8v4l3 3m6-3a9 9 0 11-18 0 9 9 0 0118 0z" /> - </svg> - </div> + <div className="inline-flex items-center justify-center w-16 h-16 rounded-full bg-[var(--chart-4)]/20 mx-auto"> + <Clock className="w-8 h-8 text-[var(--chart-4)]" /> + </div>Don't forget to import Clock from lucide-react:
+import { Clock } from "lucide-react"packages/web/src/app/components/inviteLinkToggle.tsx (1)
58-58: Remove unnecessary dependency from useEffect.The
toastfunction fromuseToastis stable and doesn't need to be in the dependency array.- }, [toast]) + }, [])packages/web/src/app/onboard/page.tsx (1)
154-183: Add key prop to mapped elementsThe mapped resource cards are missing a
keyprop, which helps React efficiently update the DOM when the list changes.{resourceCards.map((resourceCard) => ( <a + key={resourceCard.href} href={resourceCard.href} target="_blank" rel="noopener" className="p-4 rounded-lg bg-accent hover:bg-accent/80 border border-border hover:border-primary/20 transition-all duration-200 group" >packages/web/src/lib/authUtils.ts (1)
152-245: Comprehensive user organization addition logicThe
addUserToOrganizationfunction excellently handles:
- User and organization validation
- Seat availability checking
- Billing integration when enabled
- Cleanup of related account requests and invites
- Proper transaction management for atomicity
One minor consideration: at line 222,
user.email!uses a non-null assertion. Consider adding validation to ensure the email exists before using it.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
docs/images/invite_link_toggle.pngis excluded by!**/*.pngdocs/images/member_approval_toggle.pngis excluded by!**/*.png
📒 Files selected for processing (58)
docs/docs.json(1 hunks)docs/docs/configuration/audit-logs.mdx(0 hunks)docs/docs/configuration/auth/faq.mdx(1 hunks)docs/docs/configuration/auth/inviting-members.mdx(1 hunks)docs/docs/configuration/auth/overview.mdx(1 hunks)docs/docs/configuration/auth/providers.mdx(1 hunks)docs/docs/configuration/environment-variables.mdx(0 hunks)docs/docs/connections/local-repos.mdx(1 hunks)packages/db/prisma/migrations/20250711192241_add_org_invite_id/migration.sql(1 hunks)packages/db/prisma/migrations/20250712161552_add_member_approval_required_flag/migration.sql(1 hunks)packages/db/prisma/migrations/20250712180758_add_invite_link_enabled_flag/migration.sql(1 hunks)packages/db/prisma/migrations/20250712183440_remove_pending_approval_flag/migration.sql(1 hunks)packages/db/prisma/schema.prisma(1 hunks)packages/shared/src/entitlements.ts(1 hunks)packages/web/src/actions.ts(7 hunks)packages/web/src/app/[domain]/components/onboardGuard.tsx(1 hunks)packages/web/src/app/[domain]/components/pendingApproval.tsx(2 hunks)packages/web/src/app/[domain]/components/resubmitAccountRequestButton.tsx(3 hunks)packages/web/src/app/[domain]/components/submitAccountRequestButton.tsx(1 hunks)packages/web/src/app/[domain]/components/submitJoinRequest.tsx(1 hunks)packages/web/src/app/[domain]/layout.tsx(2 hunks)packages/web/src/app/[domain]/onboard/components/completeOnboarding.tsx(0 hunks)packages/web/src/app/[domain]/onboard/components/connectCodeHost.tsx(0 hunks)packages/web/src/app/[domain]/onboard/components/inviteTeam.tsx(0 hunks)packages/web/src/app/[domain]/onboard/components/onboardBackButton.tsx(0 hunks)packages/web/src/app/[domain]/onboard/page.tsx(0 hunks)packages/web/src/app/[domain]/settings/members/page.tsx(2 hunks)packages/web/src/app/api/(server)/onboarded/route.ts(1 hunks)packages/web/src/app/components/authMethodSelector.tsx(1 hunks)packages/web/src/app/components/authSecurityNotice.tsx(1 hunks)packages/web/src/app/components/dividerSet.tsx(1 hunks)packages/web/src/app/components/inviteLinkToggle.tsx(1 hunks)packages/web/src/app/components/providerButton.tsx(1 hunks)packages/web/src/app/invite/actions.ts(1 hunks)packages/web/src/app/invite/components/joinOrganizationButton.tsx(1 hunks)packages/web/src/app/invite/page.tsx(1 hunks)packages/web/src/app/login/components/credentialsForm.tsx(2 hunks)packages/web/src/app/login/components/loginForm.tsx(3 hunks)packages/web/src/app/login/components/magicLinkForm.tsx(2 hunks)packages/web/src/app/login/page.tsx(2 hunks)packages/web/src/app/onboard/components/completeOnboardingButton.tsx(1 hunks)packages/web/src/app/onboard/components/memberApprovalRequiredToggle.tsx(1 hunks)packages/web/src/app/onboard/components/onboardHeader.tsx(0 hunks)packages/web/src/app/onboard/components/orgCreateForm.tsx(0 hunks)packages/web/src/app/onboard/page.tsx(1 hunks)packages/web/src/app/page.tsx(1 hunks)packages/web/src/app/redeem/page.tsx(2 hunks)packages/web/src/app/signup/page.tsx(2 hunks)packages/web/src/ee/features/publicAccess/publicAccess.tsx(1 hunks)packages/web/src/ee/features/sso/sso.tsx(1 hunks)packages/web/src/env.mjs(0 hunks)packages/web/src/initialize.ts(1 hunks)packages/web/src/lib/authProviders.ts(1 hunks)packages/web/src/lib/authUtils.ts(2 hunks)packages/web/src/lib/errorCodes.ts(1 hunks)packages/web/src/lib/serviceError.ts(1 hunks)packages/web/src/lib/utils.ts(1 hunks)packages/web/src/middleware.ts(1 hunks)
💤 Files with no reviewable changes (10)
- docs/docs/configuration/audit-logs.mdx
- docs/docs/configuration/environment-variables.mdx
- packages/web/src/app/[domain]/onboard/components/completeOnboarding.tsx
- packages/web/src/env.mjs
- packages/web/src/app/[domain]/onboard/components/onboardBackButton.tsx
- packages/web/src/app/onboard/components/onboardHeader.tsx
- packages/web/src/app/[domain]/onboard/components/inviteTeam.tsx
- packages/web/src/app/[domain]/onboard/components/connectCodeHost.tsx
- packages/web/src/app/onboard/components/orgCreateForm.tsx
- packages/web/src/app/[domain]/onboard/page.tsx
🧰 Additional context used
🧬 Code Graph Analysis (15)
packages/web/src/ee/features/publicAccess/publicAccess.tsx (1)
packages/web/src/lib/constants.ts (1)
SOURCEBOT_GUEST_USER_EMAIL(30-30)
packages/web/src/app/redeem/page.tsx (1)
packages/web/src/lib/constants.ts (1)
SINGLE_TENANT_ORG_DOMAIN(32-32)
packages/web/src/app/onboard/components/completeOnboardingButton.tsx (3)
packages/web/src/components/hooks/use-toast.ts (2)
useToast(194-194)toast(194-194)packages/web/src/actions.ts (1)
completeOnboarding(255-288)packages/web/src/lib/constants.ts (1)
SINGLE_TENANT_ORG_DOMAIN(32-32)
packages/web/src/app/[domain]/components/submitAccountRequestButton.tsx (4)
packages/web/src/app/[domain]/components/resubmitAccountRequestButton.tsx (1)
SubmitAccountRequestButton(15-64)packages/web/src/components/hooks/use-toast.ts (2)
useToast(194-194)toast(194-194)packages/web/src/actions.ts (1)
createAccountRequest(1593-1701)packages/web/src/components/ui/button.tsx (1)
Button(56-56)
packages/web/src/initialize.ts (1)
packages/web/src/lib/constants.ts (3)
SINGLE_TENANT_ORG_ID(31-31)SINGLE_TENANT_ORG_NAME(33-33)SINGLE_TENANT_ORG_DOMAIN(32-32)
packages/web/src/app/signup/page.tsx (3)
packages/web/src/lib/constants.ts (1)
SINGLE_TENANT_ORG_DOMAIN(32-32)packages/web/src/lib/authProviders.ts (1)
getAuthProviders(8-18)packages/web/src/app/login/components/loginForm.tsx (1)
LoginForm(23-110)
packages/web/src/app/components/providerButton.tsx (1)
packages/web/src/lib/utils.ts (1)
cn(18-20)
packages/web/src/app/onboard/components/memberApprovalRequiredToggle.tsx (4)
packages/web/src/components/hooks/use-toast.ts (2)
useToast(194-194)toast(194-194)packages/web/src/actions.ts (2)
getMemberApprovalRequired(1703-1709)setMemberApprovalRequired(1711-1724)packages/web/src/lib/constants.ts (1)
SINGLE_TENANT_ORG_DOMAIN(32-32)packages/web/src/components/ui/switch.tsx (1)
Switch(29-29)
packages/web/src/app/api/(server)/onboarded/route.ts (2)
packages/logger/src/index.ts (1)
createLogger(86-86)packages/web/src/lib/constants.ts (1)
SINGLE_TENANT_ORG_DOMAIN(32-32)
packages/web/src/app/page.tsx (2)
packages/web/src/lib/constants.ts (1)
SINGLE_TENANT_ORG_DOMAIN(32-32)packages/web/src/auth.ts (1)
session(188-197)
packages/web/src/lib/authProviders.ts (1)
packages/web/src/auth.ts (1)
getProviders(40-133)
packages/web/src/app/components/inviteLinkToggle.tsx (3)
packages/web/src/components/hooks/use-toast.ts (2)
useToast(194-194)toast(194-194)packages/web/src/actions.ts (3)
getInviteLinkEnabled(1726-1732)getOrgInviteId(1144-1157)setInviteLinkEnabled(1734-1747)packages/web/src/lib/constants.ts (1)
SINGLE_TENANT_ORG_DOMAIN(32-32)
packages/web/src/app/[domain]/components/resubmitAccountRequestButton.tsx (1)
packages/web/src/app/[domain]/components/submitAccountRequestButton.tsx (1)
SubmitAccountRequestButton(16-67)
packages/web/src/app/login/components/loginForm.tsx (2)
packages/web/src/lib/authProviders.ts (1)
AuthProvider(3-6)packages/web/src/app/components/authMethodSelector.tsx (1)
AuthMethodSelector(21-77)
packages/web/src/actions.ts (3)
packages/web/src/lib/serviceError.ts (2)
notFound(91-97)ServiceError(11-11)packages/web/src/lib/authUtils.ts (1)
addUserToOrganization(152-245)packages/web/src/lib/utils.ts (1)
isServiceError(296-302)
🪛 LanguageTool
docs/docs/connections/local-repos.mdx
[style] ~8-~8: To reduce wordiness, try specifying a number or using “many” or “numerous” instead.
Context: ...ful in scenarios where you already have a large number of repos already checked out. Local reposi...
(LARGE_NUMBER_OF)
docs/docs/configuration/auth/inviting-members.mdx
[style] ~18-~18: The noun “invitation” is usually used instead of ‘invite’ in formal writing.
Context: ...n owner of the deployment can enable an invite link. When enabled, users can use this ...
(AN_INVITE)
docs/docs/configuration/auth/providers.mdx
[style] ~18-~18: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...6 digit codes sent to a provided email. Email codes are enabled when transactional em...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🪛 Biome (1.9.4)
packages/web/src/app/onboard/components/memberApprovalRequiredToggle.tsx
[error] 11-12: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
[error] 14-14: Unexpected empty object pattern.
(lint/correctness/noEmptyPattern)
packages/web/src/app/onboard/page.tsx
[error] 42-42: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 155-161: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (56)
packages/db/prisma/migrations/20250711192241_add_org_invite_id/migration.sql (1)
1-2: LGTM: Safe migration adding invite functionality.The migration correctly adds a nullable
inviteIdcolumn to theOrgtable, which is safe for existing data and appropriate for the initial rollout of invite functionality.packages/db/prisma/migrations/20250712161552_add_member_approval_required_flag/migration.sql (1)
1-2: LGTM: Well-designed migration with proper defaults.The migration correctly adds the
memberApprovalRequiredboolean column withNOT NULL DEFAULT false, ensuring backward compatibility while enabling the new organization-level member approval feature.packages/web/src/ee/features/publicAccess/publicAccess.tsx (1)
105-109: LGTM: Correctly removes deprecated pendingApproval field.The removal of
pendingApproval: falsefrom guest user creation aligns with the database schema changes that eliminate user-level pending approval flags. Guest users inherently don't require approval, making this change both necessary and logical.docs/docs/connections/local-repos.mdx (1)
8-8: LGTM: Typo correction improves documentation quality.Good catch fixing "meaing" to "meaning" in the documentation.
packages/db/prisma/migrations/20250712183440_remove_pending_approval_flag/migration.sql (1)
1-8: LGTM: Proper completion of schema transition with data loss warning.The migration correctly removes the deprecated
pendingApprovalcolumn from theUsertable with an appropriate data loss warning. The timing is correct - this runs after the organization-level member approval features are in place, ensuring a clean transition from user-level to organization-level approval management.packages/web/src/lib/utils.ts (1)
122-122: LGTM! Good formatting consistency fix.The indentation of the
iconproperty now aligns with the formatting used in other cases within thegetAuthProviderInfofunction.docs/docs.json (1)
75-78: LGTM! Well-structured documentation expansion.The new authentication documentation pages (providers, inviting-members, roles-and-permissions, faq) properly support the revamped onboarding flow and provide comprehensive coverage of the new member approval and invite link features.
packages/web/src/lib/errorCodes.ts (1)
10-11: LGTM! Improved error handling granularity.Adding specific
USER_NOT_FOUNDandORG_NOT_FOUNDerror codes provides better error handling precision compared to the genericMEMBER_NOT_FOUNDthat was removed. This enables more targeted error responses and user messaging.packages/web/src/app/[domain]/settings/members/page.tsx (2)
15-15: LGTM! Proper component import.The import of
MemberApprovalRequiredToggleis correctly structured and follows the established import patterns in the file.
82-84: LGTM! Proper role-based access control.The conditional rendering ensures that only organization owners can see and interact with the member approval toggle, which is the correct access control pattern. The placement above the invite card is logical from a UI flow perspective.
packages/db/prisma/migrations/20250712180758_add_invite_link_enabled_flag/migration.sql (2)
4-4: Verify data loss impact is acceptable.The migration will drop the
inviteIdcolumn, resulting in data loss. Ensure this is intentional and that any existing invite IDs are either no longer needed or have been migrated elsewhere.
8-10: LGTM! Well-structured invite link schema.The new columns properly support the invite link functionality:
inviteLinkEnabledwithDEFAULT trueensures existing organizations get the feature enabledinviteLinkIdas nullable text provides flexibility for organizations that may not use invite linkspackages/web/src/middleware.ts (1)
16-18: LGTM! Necessary addition for new onboarding flows.The addition of
/inviteand/onboardpaths to the bypass list correctly supports the new centralized onboarding and invitation flows described in the PR objectives.packages/web/src/app/[domain]/components/onboardGuard.tsx (2)
18-18: Redirect path correctly updated for centralized onboarding.The change from domain-prefixed to fixed
/onboardpath aligns with the architectural shift to centralized onboarding flows.
24-24: Dependency array correctly updated.The removal of domain dependency from the useMemo array is consistent with the elimination of domain-specific routing in the onboarding flow.
packages/shared/src/entitlements.ts (1)
113-113: licenseKey declaration remains within function scopeUpon verification,
const licenseKey = getLicenseKey()is on line 113 inside thegetSeatsfunction, so it’s still fetched on-demand. No change to module-load behavior—this refactor is safe.packages/web/src/lib/serviceError.ts (2)
99-105: Well-implemented error function for user not found scenarios.The
userNotFoundfunction follows the established pattern and uses appropriate status code and error code.
107-113: Well-implemented error function for organization not found scenarios.The
orgNotFoundfunction follows the established pattern and uses appropriate status code and error code.packages/web/src/app/redeem/page.tsx (2)
8-9: Appropriate imports for organization onboarding check.The imports are correctly added to support the new organization existence and onboarding status validation.
18-21: Correct implementation of organization onboarding check.The preliminary check ensures that the single-tenant organization exists and is onboarded before processing invite redemption. This aligns with the new centralized onboarding flow and prevents invite redemption when the organization hasn't completed setup.
packages/web/src/app/components/dividerSet.tsx (2)
4-13: Well-designed utility component.The component provides a clean abstraction for rendering elements with separators. The conditional logic correctly avoids adding a separator after the last element.
7-10: Fix potential key collision issue.The Fragment uses
indexas a key, and the TextSeparator inside usesdivider-${index}as a key. Since both keys are based on the same index, this could potentially cause React key conflicts in complex rendering scenarios.Apply this diff to ensure unique keys:
<Fragment key={index}> {child} - {index < elements.length - 1 && <TextSeparator key={`divider-${index}`} />} + {index < elements.length - 1 && <TextSeparator />} </Fragment>The TextSeparator doesn't need its own key since it's the only conditional element within the Fragment.
Likely an incorrect or invalid review comment.
packages/web/src/app/api/(server)/onboarded/route.ts (1)
9-32: Excellent API route implementation.The route handler demonstrates good practices:
- Proper error handling with try-catch
- Appropriate HTTP status codes (404 for not found, 500 for errors)
- Comprehensive logging for debugging
- Clean database query with selective field projection
packages/web/src/app/login/page.tsx (2)
27-30: Proper onboarding flow enforcement.The organization check correctly redirects users to onboarding if the org doesn't exist or isn't onboarded, ensuring the new onboarding flow is properly enforced.
32-32: Good centralization of auth provider logic.Switching from the previous provider retrieval method to
getAuthProviders()centralizes the authentication provider logic and provides better type safety.Also applies to: 39-39
packages/db/prisma/schema.prisma (1)
168-179: Well-designed schema additions for new features.The new organization fields support the member approval and invite link features effectively:
memberApprovalRequireddefaults tofalse(opt-in approach)inviteLinkEnableddefaults totrue(feature enabled by default)inviteLinkIdis nullable (optional identifier)The default values align well with user-friendly onboarding expectations.
packages/web/src/app/signup/page.tsx (2)
26-30: Consistent onboarding flow implementation.The organization check logic matches the login page implementation perfectly, ensuring consistent behavior across authentication entry points.
31-31: Consistent provider handling with login page.The switch to
getAuthProviders()maintains consistency with the login page changes and centralizes authentication provider logic effectively.Also applies to: 38-38
packages/web/src/app/[domain]/layout.tsx (1)
63-74: Excellent refactor from user-level to organization-managed join requests.The migration from checking
user.pendingApprovalto querying theaccountRequesttable represents a cleaner separation of concerns. The logic correctly handles both pending and new join request scenarios with appropriate UI components.packages/web/src/app/invite/actions.ts (1)
10-31: Well-structured server action with proper error handling.The implementation correctly validates organization existence, delegates to the centralized
addUserToOrganizationhelper, and handles both error and success cases appropriately. The use ofwithAuthandsewwrappers ensures proper authentication and error handling.docs/docs/configuration/auth/overview.mdx (1)
7-8: Well-structured documentation refactoring.The condensed overview with dedicated card navigation improves modularity and user experience. The new structure clearly separates concerns into focused documentation pages.
Also applies to: 9-22
packages/web/src/ee/features/sso/sso.tsx (1)
175-175: No remaining references to handleJITProvisioning foundI ran a global search for
handleJITProvisioningand there are no remaining calls or imports in the repo. The JIT provisioning removal is complete—no further updates are needed.packages/web/src/app/onboard/components/completeOnboardingButton.tsx (1)
11-53: Well-implemented component with proper state management.The component correctly handles loading states, error scenarios, and user navigation. The button design and user feedback mechanisms are appropriate for the onboarding flow.
packages/web/src/app/login/components/magicLinkForm.tsx (2)
19-22: Good addition of context prop for UI clarity.The context prop addition enables appropriate labeling for login versus signup flows, improving user experience.
80-80: Conditional button text enhances user clarity.The context-based button labeling clearly distinguishes between login and signup actions, providing better user guidance.
packages/web/src/app/login/components/credentialsForm.tsx (2)
15-18: Consistent context prop addition across auth components.The context prop addition maintains consistency with other authentication form components and enables appropriate UI text for different flows.
84-84: Context-aware button labeling improves user experience.The conditional button text clearly communicates the intended action based on whether the user is logging in or signing up.
packages/web/src/app/page.tsx (2)
7-15: LGTM! Clean migration to single-tenant architecture.The change from user-to-org relationship queries to a direct organization lookup aligns well with the single-tenant model. The conditional redirect logic is clear and appropriate.
17-23: Organization checks precede authentication—no changes neededI verified that both pages consistently validate the organization state before calling
auth():
- packages/web/src/app/invite/page.tsx
• Checksorg.inviteLinkEnabledand invite link ID beforeawait auth()- packages/web/src/app/page.tsx
• Redirects un-onboarded tenants to/onboardbeforeawait auth()This ordering routes invalid invites or un-onboarded orgs appropriately in our single-tenant flow before prompting for login. If this is intentional, no updates are required here.
packages/web/src/app/invite/components/joinOrganizationButton.tsx (1)
17-43: Excellent error handling and user experience patterns.The component demonstrates good practices:
- Proper loading state management
- Comprehensive error handling with user-friendly messages
- Clean async/await usage with try-catch-finally
docs/docs/configuration/auth/providers.mdx (1)
1-105: Comprehensive and well-structured authentication documentation.The documentation provides excellent coverage of all supported authentication providers with clear configuration instructions and proper linking to Auth.js documentation.
packages/web/src/initialize.ts (2)
161-189: Excellent use of database transactions for safe initialization.The transactional approach ensures data consistency during organization setup and the backfill logic handles existing organizations gracefully without requiring database wipes.
176-176: crypto.randomUUID() is cryptographically secure for invite links
crypto.randomUUID() uses cryptographically strong random values (Web Crypto API in browsers or Node.js 16+) to generate v4 UUIDs. It provides sufficient entropy for unguessable inviteLinkId values—no changes needed.• packages/web/src/initialize.ts:176
• packages/web/src/initialize.ts:185packages/web/src/app/[domain]/components/submitJoinRequest.tsx (2)
11-17: Proper session validation with graceful fallback.Good practice to check for session and user ID before rendering the component, returning null for unauthorized users is appropriate.
19-63: Well-structured UI with consistent design patterns.The component demonstrates excellent UI/UX practices:
- Proper semantic HTML structure
- Consistent spacing and typography
- Accessible SVG icons with proper attributes
- Clear user feedback messaging
packages/web/src/app/components/authMethodSelector.tsx (1)
1-77: LGTM! Well-architected authentication component.This component effectively centralizes authentication method rendering with clean separation of concerns, proper TypeScript typing, and good React patterns. The filtering logic for different provider types and conditional rendering is well-implemented.
packages/web/src/app/onboard/components/memberApprovalRequiredToggle.tsx (1)
15-125: Excellent state management and user experience design.The component implementation demonstrates:
- Proper handling of loading states during initialization and updates
- Comprehensive error handling with user-friendly toast notifications
- Smooth UI transitions with conditional rendering of the InviteLinkToggle
- Clean async/await patterns for API calls
The integration with the SINGLE_TENANT_ORG_DOMAIN pattern and proper error boundary handling shows good architectural awareness.
packages/web/src/app/login/components/loginForm.tsx (1)
3-92: Excellent refactoring to centralize authentication logic.The refactoring successfully:
- Delegates authentication method rendering to the new
AuthMethodSelectorcomponent- Maintains all existing functionality including analytics tracking
- Improves type safety with the
AuthProviderinterface- Significantly simplifies the component while enhancing modularity
The
handleProviderClickcallback integration withAuthMethodSelectormaintains the analytics functionality seamlessly.packages/web/src/app/components/authSecurityNotice.tsx (1)
1-98: Well-implemented security notice with proper SSR handling.The component demonstrates excellent practices:
- Prevents hydration errors by checking
hasMountedstate before accessing cookies- Robust cookie handling with proper error catching and logging
- Environment-aware rendering (self-hosted deployments only)
- Clean UX with dismissible functionality and proper ARIA labels
The cookie persistence logic with 1-year expiration and proper encoding/decoding is well-implemented.
packages/web/src/app/invite/page.tsx (1)
20-58: Solid invite flow logic with proper validation.The main
InvitePagecomponent implements a well-structured flow:
- Proper organization validation and onboarding checks
- Secure invite link validation against organization settings
- Clean separation between authenticated and unauthenticated user flows
- Appropriate redirects and membership checking
The async/await patterns and error handling with
notFound()andredirect()are correctly implemented.packages/web/src/actions.ts (3)
1247-1251: Good refactoring to use centralized membership logicThe replacement of manual transaction logic with the centralized
addUserToOrganizationhelper improves code maintainability and ensures consistent seat count management across the codebase.
1703-1747: Well-implemented organization settings API functionsThe new functions for managing
memberApprovalRequiredandinviteLinkEnabledflags are properly implemented with:
- Appropriate OWNER role authorization
- Consistent error handling with
sew()- Clean database update logic
1784-1787: Consistent use of centralized membership logicGood consistency in using the
addUserToOrganizationhelper here as well, maintaining the same error handling and audit patterns.packages/web/src/app/onboard/page.tsx (1)
37-316: Excellent refactor of the onboarding flowThe transformation to a multi-step wizard greatly improves the user experience with:
- Clear visual progress indicators
- Role-based access control ensuring only owners can complete setup
- Helpful resource links for documentation
- Clean, modern UI design
packages/web/src/lib/authUtils.ts (2)
18-119: Well-refactored user creation for single-tenant modeThe simplified
onCreateUserfunction correctly:
- Makes the first user the owner
- Conditionally adds subsequent users based on the
memberApprovalRequiredflag- Maintains proper error handling and audit logging
121-150: Robust seat availability checkingThe
orgHasAvailabilityfunction properly:
- Handles the case when organization doesn't exist
- Excludes GUEST users from seat count
- Supports both limited and unlimited seat configurations
packages/web/src/app/[domain]/components/submitAccountRequestButton.tsx
Outdated
Show resolved
Hide resolved
packages/web/src/app/[domain]/components/resubmitAccountRequestButton.tsx
Show resolved
Hide resolved
packages/web/src/app/onboard/components/memberApprovalRequiredToggle.tsx
Outdated
Show resolved
Hide resolved
packages/db/prisma/migrations/20250711192241_add_org_invite_id/migration.sql
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
docs/docs/configuration/auth/inviting-members.mdx (2)
10-11: Fix the truncated word & misplaced bold delimiterThe word “deploymen**” is missing the trailing ‘t’, and the closing
**occurs before the period, breaking the sentence flow.-**By default, Sourcebot requires new members to be approved by the owner of the deploymen**. This section explains how approvals work and how +**By default, Sourcebot requires new members to be approved by the owner of the deployment.** This section explains how approvals work and how
27-28: Consider using the more formal term “invitation link.”In user-facing docs, “invitation link” is the idiomatic phrase; “invite link” can read as colloquial.
-If member approval is required, an owner of the deployment can enable an invite link. When enabled, users -can use this invite link to register and be automatically added to the organization without approval: +If member approval is required, an owner of the deployment can enable an invitation link. When enabled, users +can use this link to register and be automatically added to the organization without approval:packages/web/src/app/onboard/page.tsx (2)
42-44: Apply optional chaining for safer property access.The current code could throw an error if
orgis null. Use optional chaining for safer access.- if (org && org.isOnboarded) { + if (org?.isOnboarded) { redirect('/'); }
68-81: Consider externalizing hardcoded documentation URLs.The documentation URLs are hardcoded throughout the component. Consider moving them to a configuration file or constants for better maintainability.
Create a configuration object:
const DOCS_URLS = { codeHostConnections: "https://docs.sourcebot.dev/docs/connections/overview", authSystem: "https://docs.sourcebot.dev/docs/configuration/auth", overview: "https://docs.sourcebot.dev/docs/overview", authProviders: "https://docs.sourcebot.dev/docs/configuration/auth/overview", discussions: "https://github.com/sourcebot-dev/sourcebot/discussions" } as const;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
docs/docs/configuration/auth/inviting-members.mdx(1 hunks)packages/db/prisma/migrations/20250713035825_change_org_member_flag_defaults/migration.sql(1 hunks)packages/db/prisma/schema.prisma(1 hunks)packages/web/src/app/onboard/page.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/db/prisma/migrations/20250713035825_change_org_member_flag_defaults/migration.sql
🧰 Additional context used
🪛 LanguageTool
docs/docs/configuration/auth/inviting-members.mdx
[style] ~27-~27: The noun “invitation” is usually used instead of ‘invite’ in formal writing.
Context: ...n owner of the deployment can enable an invite link. When enabled, users can use this ...
(AN_INVITE)
🪛 Biome (1.9.4)
packages/web/src/app/onboard/page.tsx
[error] 42-42: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 155-161: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: build
- GitHub Check: build
- GitHub Check: build
🔇 Additional comments (4)
packages/db/prisma/schema.prisma (1)
168-169: Default flipped totrue– double-check product intent & seed dataNewly-created orgs will now always require member approval unless callers explicitly set the flag to
false.
This can silently break self-serve sign-ups if the UI or seed scripts rely on the previous default.
- Re-verify with product that this is the desired behaviour for all future orgs.
- If yes, ensure the seed/onboarding code explicitly sets
memberApprovalRequiredfor any flows that should stay open.packages/web/src/app/onboard/page.tsx (3)
47-62: Verify the membership role check logic.The logic correctly checks if a user is authenticated and whether they have owner privileges before allowing onboarding access. This is good security practice.
The authentication flow properly:
- Fetches the user's membership from the database
- Verifies the user has OWNER role
- Shows appropriate message for non-owners
- Prevents unauthorized access to onboarding steps
213-240: Excellent step progress indicator implementation.The step progress indicators with connecting lines, checkmarks for completed steps, and dynamic styling provide great UX. The implementation handles edge cases well and uses proper z-index layering.
317-379: Well-implemented access control component.The NonOwnerOnboardingMessage component provides clear messaging and includes the LogoutEscapeHatch for users who need to switch accounts. The UI is informative and actionable.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/db/prisma/migrations/20250713041019_add_onboarding_revamp_changes/migration.sql (1)
8-10: Consider indexingmemberApprovalRequiredfor frequent filtersIf the UI lists only orgs requiring approval (or vice-versa) you’ll incur full‐table scans.
A lightweight boolean index speeds this up:+CREATE INDEX "Org_memberApprovalRequired_idx" + ON "Org"("memberApprovalRequired");Optional but cheap.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/db/prisma/migrations/20250713041019_add_onboarding_revamp_changes/migration.sql(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: build
- GitHub Check: build
- GitHub Check: build
packages/db/prisma/migrations/20250713041019_add_onboarding_revamp_changes/migration.sql
Show resolved
Hide resolved
packages/db/prisma/migrations/20250713041019_add_onboarding_revamp_changes/migration.sql
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
packages/web/src/app/onboard/page.tsx (3)
65-67: Simplify complex step determination logic.The nested ternary operations make this logic difficult to understand and maintain, which was previously flagged in past reviews.
Consider extracting this into a helper function:
+function determineCurrentStep(stepParam: number, isAuthenticated: boolean): number { + if (!isAuthenticated) { + // Unauthenticated users can only access steps 0-1 + return Math.max(0, Math.min(stepParam, 1)); + } + // Authenticated users start from step 2 minimum + return Math.max(2, stepParam); +} - const stepParam = searchParams?.step ? parseInt(searchParams.step) : 0; - const currentStep = session?.user ? Math.max(2, stepParam) : Math.max(0, Math.min(stepParam, 1)); + const stepParam = searchParams?.step ? parseInt(searchParams.step) : 0; + const currentStep = determineCurrentStep(stepParam, !!session?.user);
86-192: Consider extracting step definitions for better maintainability.The step definitions are quite large and embedded within the main component. This was previously identified as a maintainability concern.
Consider moving the step definitions to a separate file or function:
+// utils/onboardingSteps.tsx +export function getOnboardingSteps(providers: any[]): OnboardingStep[] { + return [ + // ... step definitions + ]; +} // In the main component: - const steps: OnboardingStep[] = [ - // ... large step definitions - ] + const steps = getOnboardingSteps(providers);
196-319: Consider component decomposition for better maintainability.The main component remains quite large (~320 lines) and handles multiple responsibilities including layout, step management, and progress indication. This was previously flagged as a maintainability concern.
Consider extracting:
- Progress indicator panel (lines 202-283) into
OnboardingProgressPanel- Content panel (lines 285-313) into
OnboardingContentPanel- Step header section (lines 290-305) into
OnboardingStepHeaderThis would improve testability and make the code more modular.
🧹 Nitpick comments (1)
packages/web/src/app/onboard/page.tsx (1)
43-45: Use optional chaining for safer property access.The current code uses explicit null checking which can be simplified using optional chaining.
- if (org && org.isOnboarded) { + if (org?.isOnboarded) { redirect('/'); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/web/src/app/onboard/page.tsx(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
packages/web/src/app/onboard/page.tsx
[error] 43-43: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: build
- GitHub Check: build
- GitHub Check: build
🔇 Additional comments (3)
packages/web/src/app/onboard/page.tsx (3)
48-63: Well-implemented role-based access control.The ownership verification logic correctly ensures only organization owners can complete onboarding, preventing unauthorized access to configuration steps.
157-186: Resource cards rendering looks good.The key prop issue from previous reviews has been correctly resolved with
key={resourceCard.id}.
321-383: Well-designed fallback component for non-owners.The
NonOwnerOnboardingMessagecomponent provides clear messaging about access restrictions and includes helpful links to documentation.
|
Also changelog reminder |
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
packages/web/src/ee/features/sso/sso.tsx (1)
130-151: Fix automatic user provisioning to enforce seat limits and billingThe current
onCreateUserimplementation inpackages/web/src/lib/authUtils.tscorrectly adds new users to the single-tenant org but bypasses seat-limit checks and billing integration. To ensure parity with the removed JIT provisioning logic, update it to leverage the same availability and billing flows used byaddUserToOrganization.Points to address:
- In the non-first-user branch (around lines 109–117), replace the direct
prisma.userToOrg.createcall with a call toaddUserToOrganization(user.id, SINGLE_TENANT_ORG_ID)so thatorgHasAvailabilityandincrementOrgSeatCountare applied.- Ensure that if billing is enabled,
incrementOrgSeatCountis invoked within the same transaction.- Preserve the existing audit and account-request cleanup logic after successful onboarding.
This will guarantee that SSO-provisioned users are subject to the same organization membership, seat-limit, and billing rules as invite-based users.
♻️ Duplicate comments (2)
packages/web/src/app/onboard/page.tsx (2)
65-67: Extract step determination logic into a separate function.The current step calculation logic is complex and was flagged in previous reviews but remains unaddressed. This nested ternary with Math operations makes the code difficult to understand and maintain.
Consider extracting this into a helper function:
+function determineCurrentStep(stepParam: number, isAuthenticated: boolean): number { + if (!isAuthenticated) { + // Unauthenticated users can only access steps 0-1 + return Math.max(0, Math.min(stepParam, 1)); + } + // Authenticated users start from step 2 minimum + return Math.max(2, stepParam); +} const stepParam = searchParams?.step ? parseInt(searchParams.step) : 0; - const currentStep = session?.user ? Math.max(2, stepParam) : Math.max(0, Math.min(stepParam, 1)); + const currentStep = determineCurrentStep(stepParam, !!session?.user);
38-319: Consider breaking down the large component for better maintainability.This component is 280+ lines and handles multiple responsibilities, which was flagged in previous reviews but hasn't been addressed. The monolithic structure makes it harder to maintain, test, and reuse.
Consider extracting:
- Step progress indicators (left panel) into
OnboardingStepProgresscomponent- Step content (right panel) into
OnboardingStepContentcomponent- Step definitions (lines 86-192) into a separate configuration file
- Resource cards into a separate component
This modularization would improve readability, testability, and maintainability.
🧹 Nitpick comments (2)
packages/web/src/data/org.ts (1)
5-18: Good approach for build-time database unavailability, but consider error specificity.The try-catch implementation effectively handles the build-time scenario where the database is inaccessible. However, the broad catch will intercept all database errors, not just connection issues, which might mask legitimate database problems in production environments.
Consider making the error handling more specific if you need to distinguish between connection errors and other database issues:
} catch (error) { - // During build time we won't be able to access the database, so we catch and return null in this case - // so that we can statically build pages that hit the DB (ex. to check if the org is onboarded) - console.error('Error fetching org from domain:', error); - return null; + // During build time we won't be able to access the database, so we catch and return null in this case + // so that we can statically build pages that hit the DB (ex. to check if the org is onboarded) + console.error('Error fetching org from domain:', error); + + // Consider rethrowing non-connection errors in production if needed + // if (process.env.NODE_ENV === 'production' && !isConnectionError(error)) { + // throw error; + // } + + return null; }Additionally, ensure the function's return type reflects that it can return
nullfor proper TypeScript support.packages/web/src/app/onboard/page.tsx (1)
43-43: Apply optional chaining for safer property access.Static analysis correctly identifies this can be simplified using optional chaining.
- if (org && org.isOnboarded) { + if (org?.isOnboarded) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
packages/web/src/app/[domain]/components/pendingApproval.tsx(2 hunks)packages/web/src/app/[domain]/layout.tsx(2 hunks)packages/web/src/app/invite/page.tsx(1 hunks)packages/web/src/app/onboard/components/memberApprovalRequiredToggle.tsx(1 hunks)packages/web/src/app/onboard/page.tsx(1 hunks)packages/web/src/app/page.tsx(1 hunks)packages/web/src/data/org.ts(1 hunks)packages/web/src/ee/features/sso/sso.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/web/src/app/page.tsx
- packages/web/src/app/[domain]/layout.tsx
- packages/web/src/app/onboard/components/memberApprovalRequiredToggle.tsx
- packages/web/src/app/[domain]/components/pendingApproval.tsx
- packages/web/src/app/invite/page.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
packages/web/src/app/onboard/page.tsx
[error] 43-43: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (4)
packages/web/src/app/onboard/page.tsx (3)
321-383: Well-structured component with good UX design.The
NonOwnerOnboardingMessagecomponent provides clear messaging and appropriate visual feedback for non-owner users. The implementation includes proper accessibility considerations and helpful documentation links.
86-192: Comprehensive step definitions with good UX flow.The step definitions are well-structured with clear progression and appropriate content for each phase. The resource cards provide valuable onboarding guidance with proper external links and visual indicators.
196-283: Excellent responsive design and visual feedback.The left panel implementation with step progress indicators, connecting lines, and dynamic styling provides excellent user feedback. The visual hierarchy and responsive design considerations are well-executed.
packages/web/src/ee/features/sso/sso.tsx (1)
169-169: JIT provisioning removal aligns with centralization effort.The removal of the
handleJITProvisioningfunction is appropriate and consistent with the broader refactor to centralize organization membership logic inauthUtils.ts. This simplifies the SSO provider configuration and eliminates duplicate membership management code.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
packages/web/src/app/onboard/page.tsx (2)
66-67: Add input validation for step parameter.The current implementation could result in
NaNif the step parameter is not a valid number, which could cause unexpected behavior.- const stepParam = searchParams?.step ? parseInt(searchParams.step) : 0; + const stepParam = searchParams?.step ? parseInt(searchParams.step, 10) || 0 : 0;
67-67: Extract step determination logic for better maintainability.The complex nested ternary with Math operations makes the step logic difficult to understand and maintain.
+function determineCurrentStep(stepParam: number, isAuthenticated: boolean): number { + if (!isAuthenticated) { + // Unauthenticated users can only access steps 0-1 + return Math.max(0, Math.min(stepParam, 1)); + } + // Authenticated users start from step 2 minimum + return Math.max(2, stepParam); +} - const currentStep = session?.user ? Math.max(2, stepParam) : Math.max(0, Math.min(stepParam, 1)); + const currentStep = determineCurrentStep(stepParam, !!session?.user);
🧹 Nitpick comments (2)
packages/web/src/app/onboard/page.tsx (2)
43-45: Apply optional chaining for better safety.Static analysis correctly identifies that optional chaining would be safer here.
- if (org && org.isOnboarded) { + if (org?.isOnboarded) { redirect('/'); }
86-192: Consider extracting step definitions to improve maintainability.The inline step definitions make the component quite large and harder to maintain. Consider moving the step configurations to a separate file or function.
// Create a separate file: onboardingSteps.ts export const createOnboardingSteps = (providers: any): OnboardingStep[] => [ { id: "welcome", title: "Welcome to Sourcebot", // ... step definitions }, // ... other steps ];Then import and use in the component:
- const steps: OnboardingStep[] = [ - // ... large step definitions - ] + const steps = createOnboardingSteps(providers);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/web/src/app/[domain]/components/settingsDropdown.tsx(1 hunks)packages/web/src/app/onboard/components/memberApprovalRequiredToggle.tsx(1 hunks)packages/web/src/app/onboard/page.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/web/src/app/onboard/components/memberApprovalRequiredToggle.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
packages/web/src/app/onboard/page.tsx
[error] 43-43: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (5)
packages/web/src/app/[domain]/components/settingsDropdown.tsx (1)
86-96: LGTM! Improved user info display layout.The UI improvements effectively address text overflow issues:
items-startalignment works better for multi-line contentflex-shrink-0prevents avatar distortionbreak-all flex-1 leading-relaxedallows long emails to wrap gracefully instead of being truncatedThese changes enhance readability and provide a better user experience for users with long email addresses.
packages/web/src/app/onboard/page.tsx (4)
157-186: Good fix for React key prop.The resource cards now properly include the
key={resourceCard.id}prop, resolving the previous React list item issue.
196-318: Well-structured multi-step UI implementation.The layout effectively separates progress indicators from content, providing clear visual hierarchy and smooth transitions. The responsive design with left/right panels creates an intuitive onboarding experience.
321-383: Excellent role-based access control implementation.The
NonOwnerOnboardingMessagecomponent provides clear messaging about access restrictions while maintaining a polished UI. The integration withLogoutEscapeHatchensures users have a way to exit if needed.
48-63: Composite primary key provides an index – query performance is already optimal.
Prisma schema confirmsUserToOrguses@@id([orgId, userId]), which creates a composite primary key (and underlying index) onorgId,userId. The lookup will hit that index directly.Still, this check runs on every authenticated page load. To further reduce database load in high-traffic scenarios, consider:
- Caching the membership/role in the user’s session or JWT
- Leveraging a short-lived in-memory cache (e.g. Redis) keyed by userId+orgId
- Batching or deferring this check when possible (e.g. client-side gating)
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/web/src/app/onboard/page.tsx (1)
66-67: Add input validation for step parameter and simplify step logic.The step parameter parsing still lacks proper validation and the step determination logic remains complex as mentioned in previous reviews.
Apply this fix to address both issues:
+const parseStepParam = (step?: string): number => { + if (!step) return 0; + const parsed = parseInt(step, 10); + return isNaN(parsed) ? 0 : parsed; +}; + +const determineCurrentStep = (stepParam: number, isAuthenticated: boolean): number => { + if (!isAuthenticated) { + // Unauthenticated users can only access steps 0-1 + return Math.max(0, Math.min(stepParam, 1)); + } + // Authenticated users start from step 2 minimum + return Math.max(2, stepParam); +}; - const stepParam = searchParams?.step ? parseInt(searchParams.step) : 0; - const currentStep = session?.user ? Math.max(2, stepParam) : Math.max(0, Math.min(stepParam, 1)); + const stepParam = parseStepParam(searchParams?.step); + const currentStep = determineCurrentStep(stepParam, !!session?.user);
🧹 Nitpick comments (2)
packages/web/src/lib/authUtils.ts (1)
109-123: Improve readability of the membership addition logic.The conditional logic for handling users when
memberApprovalRequiredis false could be clearer. Consider extracting this logic into a separate function to improve readability and maintainability.+const addUserAsMemberIfEligible = async (userId: string, orgId: number, hasAvailability: boolean): Promise<void> => { + if (!hasAvailability) { + logger.warn(`onCreateUser: org ${orgId} has reached max capacity. User ${userId} was not added to the org.`); + return; + } + + await prisma.userToOrg.create({ + data: { + userId, + orgId, + role: OrgRole.MEMBER, + } + }); +}; - } else if (!defaultOrg.memberApprovalRequired) { - const hasAvailability = await orgHasAvailability(defaultOrg.domain); - if (!hasAvailability) { - logger.warn(`onCreateUser: org ${SINGLE_TENANT_ORG_ID} has reached max capacity. User ${user.id} was not added to the org.`); - return; - } - - await prisma.userToOrg.create({ - data: { - userId: user.id, - orgId: SINGLE_TENANT_ORG_ID, - role: OrgRole.MEMBER, - } - }); - } + } else if (!defaultOrg.memberApprovalRequired) { + const hasAvailability = await orgHasAvailability(defaultOrg.domain); + await addUserAsMemberIfEligible(user.id, SINGLE_TENANT_ORG_ID, hasAvailability); + }packages/web/src/app/onboard/page.tsx (1)
43-43: Use optional chaining for safer property access.The static analysis tool correctly identifies that optional chaining should be used here for safer property access.
- if (org && org.isOnboarded) { + if (org?.isOnboarded) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
packages/web/src/actions.ts(8 hunks)packages/web/src/app/[domain]/layout.tsx(2 hunks)packages/web/src/app/components/joinOrganizationButton.tsx(1 hunks)packages/web/src/app/components/joinOrganizationCard.tsx(1 hunks)packages/web/src/app/invite/actions.ts(1 hunks)packages/web/src/app/invite/page.tsx(1 hunks)packages/web/src/app/onboard/page.tsx(1 hunks)packages/web/src/lib/authUtils.ts(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/web/src/app/components/joinOrganizationCard.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/web/src/app/[domain]/layout.tsx
- packages/web/src/app/invite/actions.ts
- packages/web/src/actions.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/web/src/app/invite/page.tsx (6)
packages/web/src/data/org.ts (1)
getOrgFromDomain(4-19)packages/web/src/lib/constants.ts (1)
SINGLE_TENANT_ORG_DOMAIN(32-32)packages/web/src/lib/authProviders.ts (1)
getAuthProviders(8-18)packages/web/src/app/components/joinOrganizationCard.tsx (1)
JoinOrganizationCard(5-23)packages/web/src/components/ui/card.tsx (4)
Card(79-79)CardHeader(79-79)CardTitle(79-79)CardContent(79-79)packages/web/src/app/components/authMethodSelector.tsx (1)
AuthMethodSelector(21-77)
🪛 Biome (1.9.4)
packages/web/src/app/onboard/page.tsx
[error] 43-43: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (6)
packages/web/src/lib/authUtils.ts (1)
158-251: Well-implemented centralized user addition logic.The
addUserToOrganizationfunction excellently centralizes user addition logic with comprehensive error handling, seat availability checks, billing integration, and cleanup of existing requests/invites. The transaction handling and service error returns are properly implemented.packages/web/src/app/components/joinOrganizationButton.tsx (1)
1-55: Well-implemented join organization component.The component has excellent error handling, loading state management, and user feedback through toast notifications. The use of
SINGLE_TENANT_ORG_IDis consistent with the single-tenant architecture refactor. The async operation handling and router navigation are properly implemented.packages/web/src/app/invite/page.tsx (2)
19-56: Well-structured invite page with proper validation flow.The invite page correctly validates the invite link, handles different authentication states, and renders appropriate components. The server-side data fetching for organization and membership status is efficient, and the conditional rendering logic is clear and correct.
59-86: Clean welcome card implementation.The
WelcomeCardcomponent provides a good user experience with clear messaging, proper styling, and correct callback URL handling for the authentication flow. The invite link ID is properly passed through the authentication process.packages/web/src/app/onboard/page.tsx (2)
86-192: Well-designed onboarding wizard structure.The multi-step onboarding wizard is well-structured with clear step definitions, proper component separation, and good user experience. The resource cards provide helpful links to documentation, and the progress indicators enhance the onboarding flow.
321-383: Excellent non-owner message component.The
NonOwnerOnboardingMessagecomponent provides clear messaging about owner access requirements with helpful styling and appropriate documentation links. The logout escape hatch is a good UX addition.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/web/src/app/onboard/page.tsx (1)
79-80: Extract complex step determination logic into a helper function.The step calculation logic remains complex and could benefit from extraction as mentioned in previous reviews.
+function determineCurrentStep(stepParam: number, isAuthenticated: boolean): number { + if (!isAuthenticated) { + // Unauthenticated users can only access steps 0-1 + return Math.max(0, Math.min(stepParam, 1)); + } + // Authenticated users start from step 2 minimum + return Math.max(2, stepParam); +} - const stepParam = searchParams?.step ? parseInt(searchParams.step) : 0; - const currentStep = session?.user ? Math.max(2, stepParam) : Math.max(0, Math.min(stepParam, 1)); + const stepParam = searchParams?.step ? parseInt(searchParams.step, 10) || 0 : 0; + const currentStep = determineCurrentStep(stepParam, !!session?.user);This also addresses the input validation concern by adding
|| 0fallback for invalid parseInt results.
🧹 Nitpick comments (1)
packages/web/src/app/onboard/page.tsx (1)
50-50: Apply optional chaining for cleaner code.The static analysis tool correctly identifies that this can be simplified using optional chaining.
- if (org && org.isOnboarded) { + if (org?.isOnboarded) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/web/src/actions.ts(8 hunks)packages/web/src/app/[domain]/settings/members/page.tsx(2 hunks)packages/web/src/app/components/inviteLinkToggle.tsx(1 hunks)packages/web/src/app/onboard/components/memberApprovalRequiredToggle.tsx(1 hunks)packages/web/src/app/onboard/page.tsx(1 hunks)packages/web/src/components/ui/switch.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/web/src/components/ui/switch.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/web/src/app/[domain]/settings/members/page.tsx
- packages/web/src/app/components/inviteLinkToggle.tsx
- packages/web/src/actions.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/web/src/app/onboard/components/memberApprovalRequiredToggle.tsx (5)
packages/web/src/components/hooks/use-toast.ts (2)
useToast(194-194)toast(194-194)packages/web/src/actions.ts (1)
setMemberApprovalRequired(1688-1701)packages/web/src/lib/constants.ts (1)
SINGLE_TENANT_ORG_DOMAIN(32-32)packages/web/src/components/ui/switch.tsx (1)
Switch(29-29)packages/web/src/app/components/inviteLinkToggle.tsx (1)
InviteLinkToggle(17-154)
🪛 Biome (1.9.4)
packages/web/src/app/onboard/page.tsx
[error] 50-50: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (2)
packages/web/src/app/onboard/components/memberApprovalRequiredToggle.tsx (1)
11-89: LGTM! Well-implemented toggle component with proper error handling.The component effectively addresses the previous review concerns and demonstrates excellent React and TypeScript practices:
- Proper interface definition with meaningful props
- Robust async error handling with both service errors and exceptions
- Loading states to prevent concurrent operations
- User feedback through toast notifications
- Smooth conditional rendering with CSS transitions
packages/web/src/app/onboard/page.tsx (1)
41-396: Well-structured onboarding flow with proper authentication controls.The onboarding implementation effectively handles:
- Role-based access control with owner verification
- Step-based navigation with proper URL parameter handling
- SSO entitlement and GCP IAP integration
- Resource cards with external documentation links
- Responsive layout with progress indicators
The component size concern was noted in previous reviews, but the current implementation maintains good readability through clear step definitions and logical organization.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
packages/web/src/app/onboard/page.tsx (2)
78-80: Address the unresolved step determination complexity.The complex step calculation logic and missing parseInt validation flagged in previous reviews remain unaddressed.
Referring to past review suggestions:
- Extract step determination into a separate function for better maintainability
- Add input validation for parseInt to handle NaN cases properly
+function determineCurrentStep(stepParam: number, isAuthenticated: boolean): number { + if (!isAuthenticated) { + return Math.max(0, Math.min(stepParam, 1)); + } + return Math.max(2, stepParam); +} - const stepParam = searchParams?.step ? parseInt(searchParams.step) : 0; - const currentStep = session?.user ? Math.max(2, stepParam) : Math.max(0, Math.min(stepParam, 1)); + const stepParam = searchParams?.step ? parseInt(searchParams.step, 10) || 0 : 0; + const currentStep = determineCurrentStep(stepParam, !!session?.user);
99-332: Component size and complexity continue to grow.The component is now 396 lines (up from 280+ flagged in previous reviews), making the maintainability concerns even more pressing.
The previous suggestions for refactoring remain valid:
- Extract
OnboardingStepProgresscomponent (lines 227-267)- Extract
OnboardingStepContentcomponent for the right panel- Move step definitions (lines 99-205) to a separate configuration file
- Extract resource cards to a separate component
Consider prioritizing this refactoring as the component continues to grow in complexity.
🧹 Nitpick comments (1)
packages/web/src/app/onboard/page.tsx (1)
50-52: Use optional chaining for cleaner conditional logic.The static analysis correctly identifies that this can be simplified using optional chaining.
- if (org && org.isOnboarded) { + if (org?.isOnboarded) { redirect('/'); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/web/src/app/components/joinOrganizationButton.tsx(1 hunks)packages/web/src/app/onboard/components/memberApprovalRequiredToggle.tsx(1 hunks)packages/web/src/app/onboard/page.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/web/src/app/components/joinOrganizationButton.tsx
- packages/web/src/app/onboard/components/memberApprovalRequiredToggle.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
packages/web/src/app/onboard/page.tsx
[error] 50-50: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (3)
packages/web/src/app/onboard/page.tsx (3)
170-172: Good fix for the React key prop issue.The missing key prop from the previous review has been properly addressed using
resourceCard.id.
54-70: Excellent role-based access control implementation.The authentication and membership verification logic properly restricts onboarding to organization owners only, which is a good security practice.
334-396: Well-designed user experience for non-owner access.The
NonOwnerOnboardingMessagecomponent provides clear communication and includes helpful features like the logout escape hatch and documentation links.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
packages/web/src/app/onboard/page.tsx (2)
79-80: Simplify step determination logic and add input validation.The step calculation logic remains complex and the
parseIntoperation lacks validation, which could result inNaNand unexpected behavior.Consider extracting this logic into a separate function with proper validation:
+function determineCurrentStep(stepParam: string | undefined, isAuthenticated: boolean): number { + const parsedStep = stepParam ? parseInt(stepParam, 10) : 0; + const validStep = isNaN(parsedStep) ? 0 : parsedStep; + + if (!isAuthenticated) { + // Unauthenticated users can only access steps 0-1 + return Math.max(0, Math.min(validStep, 1)); + } + // Authenticated users start from step 2 minimum + return Math.max(2, validStep); +} - const stepParam = searchParams?.step ? parseInt(searchParams.step) : 0; - const currentStep = session?.user ? Math.max(2, stepParam) : Math.max(0, Math.min(stepParam, 1)); + const currentStep = determineCurrentStep(searchParams?.step, !!session?.user);
41-332: Consider refactoring the large component for better maintainability.The main Onboarding component is still quite large (290+ lines) and handles multiple responsibilities. While the structure is improved, extracting smaller components would enhance maintainability.
Consider these extractions:
OnboardingStepProgresscomponent for the left panel (lines 215-296)OnboardingStepContentcomponent for the right panel (lines 298-327)- Move step definitions (lines 99-205) to a separate configuration file
- Extract resource cards to a separate component
This would improve readability, testability, and maintainability.
🧹 Nitpick comments (1)
packages/web/src/app/onboard/page.tsx (1)
50-50: Use optional chaining for cleaner code.The conditional check can be simplified using optional chaining as suggested by the static analysis tool.
- if (org && org.isOnboarded) { + if (org?.isOnboarded) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/web/src/app/[domain]/components/pendingApproval.tsx(2 hunks)packages/web/src/app/[domain]/components/submitJoinRequest.tsx(1 hunks)packages/web/src/app/[domain]/settings/members/page.tsx(2 hunks)packages/web/src/app/components/inviteLinkToggle.tsx(1 hunks)packages/web/src/app/onboard/components/memberApprovalRequiredToggle.tsx(1 hunks)packages/web/src/app/onboard/page.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/web/src/app/[domain]/settings/members/page.tsx
- packages/web/src/app/[domain]/components/pendingApproval.tsx
- packages/web/src/app/[domain]/components/submitJoinRequest.tsx
- packages/web/src/app/components/inviteLinkToggle.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
packages/web/src/app/onboard/page.tsx
[error] 50-50: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (2)
packages/web/src/app/onboard/components/memberApprovalRequiredToggle.tsx (1)
11-90: LGTM! The component implementation is well-structured.The previous review concerns about empty interface and function parameters have been properly addressed. The component now has:
- Proper TypeScript interface with defined props
- Correct function parameter destructuring
- Good state management and error handling
- Clean UI with conditional rendering
packages/web/src/app/onboard/page.tsx (1)
334-396: Well-implemented access control component.The
NonOwnerOnboardingMessagecomponent provides clear messaging for non-owner users and includes helpful links to documentation. The UI is polished and accessible.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
packages/web/src/app/onboard/page.tsx (3)
50-50: Simplify optional property access using optional chaining.The condition can be simplified using optional chaining as suggested by the static analysis tool.
- if (org && org.isOnboarded) { + if (org?.isOnboarded) {
46-48: Enhance error handling with proper error UI component.The current error handling returns a basic div with minimal styling and no user guidance. Consider using a proper error component that matches the application's design system.
- if (!org) { - return <div>Error loading organization</div>; - } + if (!org) { + return ( + <div className="min-h-screen bg-background flex items-center justify-center p-6"> + <Card className="max-w-md"> + <CardContent className="p-8 text-center space-y-4"> + <h1 className="text-xl font-semibold text-foreground"> + Organization Not Found + </h1> + <p className="text-muted-foreground"> + Unable to load organization details. Please contact support if this issue persists. + </p> + </CardContent> + </Card> + </div> + ); + }
82-104: Consider extracting resource cards to a separate configuration file.The resource cards configuration could be moved to a separate constant or configuration file to improve maintainability and allow for easier updates to documentation links.
Create a new file
src/config/onboardingResources.ts:export const ONBOARDING_RESOURCE_CARDS = [ { id: "code-host-connections", title: "Code Host Connections", description: "Learn how to index repos across Sourcebot's supported platforms", href: "https://docs.sourcebot.dev/docs/connections/overview", icon: "GitBranchIcon", }, // ... other cards ] as const;Then import and use it in the component to separate configuration from presentation logic.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/web/src/app/onboard/page.tsx(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
packages/web/src/app/onboard/page.tsx
[error] 50-50: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (2)
packages/web/src/app/onboard/page.tsx (2)
179-179: Good implementation of React key prop.The addition of the
key={resourceCard.id}prop correctly addresses the React list rendering requirement and uses a stable, unique identifier.
341-403: Well-implemented component with clear user guidance.The
NonOwnerOnboardingMessagecomponent provides clear feedback to non-owner users with appropriate visual hierarchy, helpful information, and links to documentation. The use of consistent styling and accessibility-friendly SVG icons enhances the user experience.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (4)
packages/web/src/app/onboard/page.tsx (4)
113-219: Extract step definitions to improve maintainability.The step definitions are quite lengthy and embedded within the component. As noted in previous reviews, consider extracting these to a separate configuration file or module.
43-346: Consider breaking down the large component for better maintainability.The main Onboarding component is quite large (280+ lines) and handles multiple responsibilities as noted in previous reviews. This impacts maintainability and testability.
221-221: Add bounds checking for array access.The code directly accesses
steps[currentStep]without verifying bounds, which could cause runtime errors with invalid step parameters as identified in previous reviews.- const currentStepData = steps[currentStep] + const safeCurrentStep = Math.max(0, Math.min(currentStep, steps.length - 1)); + const currentStepData = steps[safeCurrentStep];
86-87: Improve step parameter validation and logic clarity.The step parameter parsing lacks proper validation and the step determination logic is complex, making it error-prone as noted in previous reviews.
+function determineCurrentStep(stepParam: string | undefined, isAuthenticated: boolean): number { + const parsedStep = stepParam ? parseInt(stepParam, 10) || 0 : 0; + if (!isAuthenticated) { + // Unauthenticated users can only access steps 0-1 + return Math.max(0, Math.min(parsedStep, 1)); + } + // Authenticated users start from step 2 minimum + return Math.max(2, parsedStep); +} - const stepParam = searchParams?.step ? parseInt(searchParams.step) : 0; - const currentStep = session?.user ? Math.max(2, stepParam) : Math.max(0, Math.min(stepParam, 1)); + const currentStep = determineCurrentStep(searchParams?.step, !!session?.user);
🧹 Nitpick comments (2)
packages/web/src/app/onboard/page.tsx (2)
57-57: Simplify conditional check using optional chaining.The conditional check can be simplified using optional chaining as suggested by the static analysis tool.
- if (org && org.isOnboarded) { + if (org?.isOnboarded) {
62-77: Consider extracting owner verification logic.The owner verification logic is embedded within the main component, making it harder to test and reuse. Consider extracting this into a utility function.
// In a separate utility file export async function verifyOwnerAccess(session: Session | null, org: Org | null): Promise<boolean> { if (!session?.user || !org) return false; const membership = await prisma.userToOrg.findUnique({ where: { orgId_userId: { orgId: org.id, userId: session.user.id } } }); return membership?.role === OrgRole.OWNER; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/web/src/app/[domain]/settings/members/page.tsx(3 hunks)packages/web/src/app/components/inviteLinkToggle.tsx(1 hunks)packages/web/src/app/onboard/components/memberApprovalRequiredToggle.tsx(1 hunks)packages/web/src/app/onboard/page.tsx(1 hunks)packages/web/src/lib/utils.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/web/src/app/[domain]/settings/members/page.tsx
- packages/web/src/app/components/inviteLinkToggle.tsx
- packages/web/src/lib/utils.ts
- packages/web/src/app/onboard/components/memberApprovalRequiredToggle.tsx
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/web/src/app/onboard/page.tsx (7)
packages/web/src/lib/authProviders.ts (1)
getAuthProviders(8-18)packages/web/src/data/org.ts (1)
getOrgFromDomain(4-19)packages/web/src/lib/constants.ts (1)
SINGLE_TENANT_ORG_DOMAIN(32-32)packages/web/src/lib/utils.ts (2)
getBaseUrl(27-31)createInviteLink(39-41)packages/web/src/app/[domain]/components/gcpIapAuth.tsx (1)
GcpIapAuth(10-26)packages/web/src/app/onboard/components/memberApprovalRequiredToggle.tsx (1)
MemberApprovalRequiredToggle(17-90)packages/web/src/app/onboard/components/completeOnboardingButton.tsx (1)
CompleteOnboardingButton(11-53)
🪛 Biome (1.9.4)
packages/web/src/app/onboard/page.tsx
[error] 57-57: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (4)
packages/web/src/app/onboard/page.tsx (4)
184-217: Resource cards implementation looks good.The resource cards are properly structured with unique keys and appropriate external links. The hover states and accessibility features are well implemented.
241-281: Step progress indicator implementation is well-designed.The step progress indicators with connecting lines, dynamic styling, and visual feedback provide excellent user experience. The implementation properly handles state transitions and accessibility.
348-410: NonOwnerOnboardingMessage component is well-structured.The component provides clear messaging for non-owners with appropriate visual hierarchy, helpful information, and documentation links. The accessibility and UX considerations are well implemented.
80-83: Review GCP IAP Auth Flow ImplementationI searched for GCP IAP environment variable usage and found references in:
- packages/web/src/app/onboard/page.tsx
- packages/web/src/app/[domain]/layout.tsx
- packages/web/src/ee/features/sso/sso.tsx
However, I was unable to locate the source of the
GcpIapAuthcomponent itself. Please manually verify that its implementation:
- Reads and validates
AUTH_EE_GCP_IAP_ENABLEDandAUTH_EE_GCP_IAP_AUDIENCEat startup- Verifies incoming IAP JWTs (audience, issuer, expiry, signature)
- Handles missing or invalid tokens with proper error states or redirects
- Includes tests (unit or e2e) covering edge cases (expired tokens, malformed tokens, missing env vars)
Ensure the flow cannot be bypassed and rejects unauthorized requests.
onCreateUser)Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Documentation
Style
Chores
Database