refactor(onboarding): remove the legacy /signup free-trial wizard - #3568
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
💤 Files with no reviewable changes (4)
📝 WalkthroughWalkthroughChangesThe authentication flow now uses a local six-digit Authentication flow
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Not re-flagging anything new. The registerWithEmailPassword issue I raised in my prior review (waiting on a verification-code screen that PasswordAuth never renders) has been fixed by commit 8e17bb8, which drops the verify() call and documents why password-signup needs no OTP step — I confirmed PasswordAuth's signup mutation indeed just does signup() -> checkSession() -> navigateBack() with no verify screen. Given the size of this refactor (35 files, ~3.4k LOC removed) and that it touches auth-adjacent code, I'd still like a human to give it a final pass rather than rely solely on automated review.
Extended reasoning...
This run's bug hunter found no new issues. My own check confirms the fix commit (8e17bb8) correctly resolves the previously-flagged E2E bug: PasswordAuth.tsx's signInOrSignUp mutation for the signup case only calls authService.signup(), checkSession(), and navigateBack() — there is no verify-code UI in that path, so removing the emailVerification.verify() wait (and the dead startFreeTrial() call) is correct. The rest of the PR is a large, mostly mechanical deletion of the legacy /signup wizard (OnboardingContainer/View/Stepper, step components, VerifyEmailPage, associated routes and specs) plus a straightforward relocation of VerificationCodeInput and small reference cleanups (UrlService.onboarding(), ONBOARDING_STEP_KEY, AuthService constructor param). No new security-sensitive logic is introduced; it is net deletion of an already-superseded flow. Given the XL size and that it touches auth service/tests even though changes there are now correct, I'm deferring to a human for final sign-off rather than auto-approving, mainly out of caution around the size/scope rather than a specific unresolved concern.
b33b72d to
963b86c
Compare
8e17bb8 to
e7bf443
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3568 +/- ##
==========================================
- Coverage 76.13% 75.52% -0.62%
==========================================
Files 1157 1044 -113
Lines 30057 27248 -2809
Branches 7440 6896 -544
==========================================
- Hits 22885 20579 -2306
+ Misses 6338 5872 -466
+ Partials 834 797 -37
*This pull request uses carry forward flags. Click here to find out more.
🚀 New features to boost your workflow:
|
The multi-step Free Trial onboarding wizard served at /signup (and its /user/verify-email redirect) has been fully replaced by the onboarding picker at /onboarding, reached through the RequireOnboarding gate, which auto-starts the trial and routes first deploys through the configure page. Delete the wizard end-to-end: OnboardingPage/Container/View/Stepper, all step components, VerifyEmailPage, and the /signup and /user/verify-email routes. Relocate the still-shared VerificationCodeInput to the auth EmailCodeVerify component that now owns email verification, and drop the now-orphaned UrlService.onboarding() builder and ONBOARDING_STEP_KEY.
…signup registerWithEmailPassword drove the PasswordAuth signup form and then waited for the passwordless "Verification code digit N" screen, which PasswordAuth never renders: password-signup sets the session server-side and PasswordAuth redirects straight off /login. Remove the verify() call so the helper matches the actual password flow.
e7bf443 to
b0cb3fd
Compare
2 participants
Why
The multi-step "Free Trial" onboarding wizard served at
/signup(and its/user/verify-emailredirect helper) has been fully replaced by the onboarding picker at/onboarding, reached through theRequireOnboardinggate. The picker auto-starts the trial (useEnsureTrialStarted) and routes first deploys through/new-deployment/configure, and email verification is now owned by the authEmailCodeVerifycomponent on/login. The old wizard was dead weight — its only remaining in-app entry point wasVerifyEmailPage.Stacked on #3567 (managed-only wallet context + boot gate), which stripped the trial buttons and flags this wizard depended on.
Part of the onboarding redesign cleanup.
What
Removes the legacy
/signupfree-trial wizard end-to-end (net −3.4k LOC):OnboardingPage,OnboardingContainer,OnboardingView,OnboardingStepper, allsteps/*(FreeTrial, EmailVerification, PaymentMethod, Welcome,TrialStartButton, …),VerifyEmailPage, and the/signup+/user/verify-emailroute files (deleted outright — no redirect stubs).VerificationCodeInput(+ spec) intocomponents/auth/EmailCodeVerify/, its sole remaining consumer.UrlService.onboarding()builder, deletedservices/storage/keys.ts(ONBOARDING_STEP_KEY), and removed the deadlocalStoragelogout cleanup + unused constructor param fromAuthService.startFreeTrial()step fromregisterWithEmailPasswordand slimmed theOnboardingPagepage-object to theisCurrentPage()check still used bypasswordless-login.spec.ts.Kept (out of scope): the
/onboardingpicker,RequireOnboardinggate, onboarding hooks,newSignup()(→/login?tab=signup), and the separate/get-startedtutorial.Verification
tsc: no new errors (verified by delta against the existing spec-typing baseline).npm run test:unit: 36 pass across the relocatedVerificationCodeInput/EmailCodeVerifyandauth.servicespecs.eslint: 0 errors on touched files.Note
The email/password E2E flow (
registerWithEmailPassword) now relies on the newEmailCodeVerifyverify screen rendering on/loginright after signup (mirroring the passwordless path). That path runs against a live env rather than in pre-push checks — worth a real run before merge.Summary by CodeRabbit