Conversation
|
WalkthroughThis PR introduces tracking of the onboarding origin (marketplace vs. dashboard) across the Vercel integration flow. The Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| ) : !fromMarketplaceContext ? ( | ||
| <Button | ||
| variant="tertiary/medium" | ||
| onClick={() => { |
There was a problem hiding this comment.
🔴 Marketplace users with GitHub connected but missing nextUrl have no button to proceed, creating a stuck state
In the github-connection step's FormButtons, the confirmButton ternary evaluates to undefined when fromMarketplaceContext is true and isGitHubConnectedForOnboarding is true but nextUrl is falsy. The first branch requires all three (isGitHubConnectedForOnboarding && fromMarketplaceContext && nextUrl), the second branch requires !fromMarketplaceContext, and the fallback is undefined. No cancelButton is provided either. Since the outer dialog prevents closing for marketplace users (VercelOnboardingModal.tsx:698: if (!open && !fromMarketplaceContext)), and the auto-redirect effect at VercelOnboardingModal.tsx:297-312 also requires nextUrl, the user is permanently stuck with no way to proceed or dismiss the modal.
The old code always had a "Skip for now" fallback that called setState("completed"), allowing the completion effect to close the modal gracefully. The nextUrl can be missing if Vercel doesn't provide a next parameter or if it fails sanitizeVercelNextUrl at vercel.callback.ts:49.
(Refers to lines 1167-1178)
Was this helpful? React with 👍 or 👎 to provide feedback.
| > | ||
| Complete | ||
| </Button> | ||
| ) : ( | ||
| <Button | ||
| variant="tertiary/medium" | ||
| onClick={() => { | ||
| trackOnboarding("vercel onboarding github skipped"); | ||
| setState("completed"); | ||
| if (fromMarketplaceContext && nextUrl) { | ||
| const validUrl = safeRedirectUrl(nextUrl); | ||
| if (validUrl) { | ||
| window.location.href = validUrl; | ||
| } | ||
| } | ||
| }} | ||
| > | ||
| Skip for now | ||
| </Button> | ||
| ) | ||
| } | ||
| cancelButton={ | ||
| isGitHubConnectedForOnboarding && fromMarketplaceContext && nextUrl ? ( | ||
| ) : !fromMarketplaceContext ? ( | ||
| <Button | ||
| variant="tertiary/medium" | ||
| onClick={() => { |
There was a problem hiding this comment.
🚩 Marketplace users without GitHub connected now have no skip/back option — intentional but removes escape hatch
The old code provided a "Skip for now" confirmButton for ALL users on the github-connection step (marketplace or not) when the first condition wasn't met. The new code at lines 1167-1177 restricts the skip button to !fromMarketplaceContext only. For marketplace users who haven't connected GitHub, there are no buttons but they CAN still interact with the "Install GitHub app" or "Connect GitHub repo" UI elements. The dialog also prevents closing for marketplace users. This appears to be an intentional design decision to force GitHub connection in marketplace flows, but it removes the previous escape hatch. If a user can't install the GitHub app (e.g., lacks org permissions), they'd need to use browser navigation to escape.
(Refers to lines 1151-1178)
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
No description provided.