Uh oh!
There was an error while loading. Please reload this page.
fix(onboarding): persist completion so users aren't trapped in the Get Started loop - #284
Merged
Merged
Conversation
…g sticks handleOnboardingComplete POSTed /api/onboarding/profile to the cross-origin NEXT_PUBLIC_API_URL without credentials:'include', so the browser dropped the sapling_session cookie and the backend's require_self returned 401. The onboarding_completed flag never flipped to True — and because the code never checked res.ok, the 401 was swallowed and users were routed to /dashboard as if it worked, then bounced back to "Get Started" on every subsequent sign-in. Switch to the existing submitOnboardingProfile() helper, which goes through the same-origin lib/api.ts fetchJSON path (API_URL='' proxied via the /api/* rewrite, credentials:'include', and an res.ok check that surfaces failures instead of hiding them). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughIn ChangesOnboarding persistence refactor
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
Deploying with |
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs | frontend-staging | 830555e | Commit Preview URL Branch Preview URL | Jun 30 2026, 01:30 AM |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/app/page.tsx (1)
624-637: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winStop the success flow when onboarding persistence fails.
submitOnboardingProfilenow throws on non-OK responses, but this catch logs the error and then falls through to the completion animation and dashboard redirect. If the profile save returns 401/500,onboarding_completedcan remain false and recreate the loop this PR is fixing.🐛 Proposed fix
} catch (e) { console.error('Failed to save onboarding profile:', e); + return; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/app/page.tsx` around lines 624 - 637, The onboarding save error is being swallowed in the submit path, allowing the success/redirect flow to continue even when submitOnboardingProfile fails. Update the handler around submitOnboardingProfile in page.tsx so that the catch path stops the completion animation and dashboard redirect, and only advances the success flow when the profile persistence call succeeds. Keep the existing error logging, but ensure the branch after the try/catch does not run as if onboarding completed when the request returns a non-OK response.
🧹 Nitpick comments (1)
frontend/src/app/page.tsx (1)
633-633: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAvoid casting unvalidated learning styles into the API contract.
formData.styleis typed asstring, so the assertion can send invalid values despiteOnboardingProfilePayloadrestrictinglearning_styleto a fixed union.♻️ Proposed refactor
+ const learningStyle = formData.style;+ if (+ learningStyle !== 'visual' &&+ learningStyle !== 'reading' &&+ learningStyle !== 'auditory' &&+ learningStyle !== 'hands-on' &&+ learningStyle !== 'mixed'+ ) {+ console.error('Invalid onboarding learning style:', learningStyle);+ return;+ }+ await submitOnboardingProfile({ user_id: userId, first_name: formData.firstName, last_name: formData.lastName, year: formData.year, majors: formData.majors, minors: formData.minors, course_ids: formData.course_ids, - learning_style: formData.style as OnboardingProfilePayload['learning_style'],+ learning_style: learningStyle, });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/app/page.tsx` at line 633, The onboarding payload is casting formData.style directly into OnboardingProfilePayload['learning_style'], which can bypass the union restriction and send invalid values. Update the code in the page.tsx form submission flow to validate formData.style against the allowed learning_style options before building the payload, and only assign a value that is already narrowed to the contract in the onboarding request object.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@frontend/src/app/page.tsx`:
- Around line 624-637: The onboarding save error is being swallowed in the
submit path, allowing the success/redirect flow to continue even when
submitOnboardingProfile fails. Update the handler around submitOnboardingProfile
in page.tsx so that the catch path stops the completion animation and dashboard
redirect, and only advances the success flow when the profile persistence call
succeeds. Keep the existing error logging, but ensure the branch after the
try/catch does not run as if onboarding completed when the request returns a
non-OK response.
---
Nitpick comments:
In `@frontend/src/app/page.tsx`:
- Line 633: The onboarding payload is casting formData.style directly into
OnboardingProfilePayload['learning_style'], which can bypass the union
restriction and send invalid values. Update the code in the page.tsx form
submission flow to validate formData.style against the allowed learning_style
options before building the payload, and only assign a value that is already
narrowed to the contract in the onboarding request object.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
On staging, signing in repeatedly re-shows the "Get Started" / onboarding flow every time, even for an already-onboarded, approved user. Investigation (against the staging DB) showed the user row persists fine — the only thing stuck is the
onboarding_completedboolean, which had beenFalsesince the account was created despite many sign-ins.Root cause
page.tsx:handleOnboardingCompleteposted to/api/onboarding/profileusing the component-localAPI_URL = process.env.NEXT_PUBLIC_API_URL— a cross-origin subdomain (https://api.staging.saplinglearn.com) — withoutcredentials: 'include'. The browser therefore dropped thesapling_sessioncookie, so the backend'srequire_selfreturned 401 and theonboarding_completed = Truewrite (backend/routes/onboarding.py:42) never ran.The failure was invisible because the call used
await fetch(...)with nores.okcheck inside atry/catchthat only catches network errors — so the 401 was swallowed and the user was routed to/dashboardas if it succeeded, then bounced back to onboarding on the next load.Reproduced live:
POST https://api.staging.saplinglearn.com/api/onboarding/profilewith no cookie →401 {"detail":"Not authenticated"}.Fix
Use the existing
submitOnboardingProfile()helper fromlib/api.ts, which goes throughfetchJSON:API_URL = '', proxied to the backend via the/api/*rewrite) → thesapling_sessioncookie is sentcredentials: 'include'res.okand throws on failure, so errors surface instead of being hiddenBlast radius
Audited all other direct
NEXT_PUBLIC_API_URLusages — this was the only authenticated cross-origin fetch. The rest are non-authed (/api/onboarding/coursessearch) or top-level OAuth popup navigations (/api/auth/google), andmiddleware.tssends theCookieheader explicitly server-side.Verification
npm run typecheck(tsc --noEmit) passes with 0 errors.onboarding_completedwas manually set toTrueto unblock immediately; this PR fixes the persistence path so it no longer recurs.🤖 Generated with Claude Code
Summary by CodeRabbit