refactor(onboarding): remove legacy start-trial flow and endpoint - #3569
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
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 ignored due to path filters (1)
📒 Files selected for processing (11)
💤 Files with no reviewable changes (3)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughChangesThe legacy Legacy trial API removal
Managed wallet state
Passwordless authentication
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested reviewers: ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
apps/api/src/billing/controllers/wallet/wallet.controller.spec.tsESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. apps/api/src/billing/http-schemas/wallet.schema.tsESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox. apps/deploy-web/src/components/auth/PasswordlessAuth/PasswordlessAuth.spec.tsxESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox.
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3569 +/- ##
==========================================
- Coverage 76.37% 75.77% -0.60%
==========================================
Files 1137 1073 -64
Lines 29658 27699 -1959
Branches 7385 6974 -411
==========================================
- Hits 22651 20989 -1662
+ Misses 6174 5909 -265
+ Partials 833 801 -32
*This pull request uses carry forward flags. Click here to find out more.
🚀 New features to boost your workflow:
|
c7c895d to
6ffcae5
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/deploy-web/src/components/wallet/ConnectManagedWalletButton.tsx (1)
29-31: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a click-navigation assertion.
ConnectManagedWalletButton.spec.tsxtests only enabled and disabled states. Add a test that verifiesrouter.pushreceivesUrlService.onboardingPicker(). This protects the changed destination from regression.🤖 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 `@apps/deploy-web/src/components/wallet/ConnectManagedWalletButton.tsx` around lines 29 - 31, Extend ConnectManagedWalletButton.spec.tsx with an enabled-button click test that invokes the handler and asserts router.push receives UrlService.onboardingPicker(). Reuse the existing router mock and button setup while preserving the current enabled and disabled state tests.
🤖 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.
Inline comments:
In `@apps/api/src/billing/controllers/wallet/wallet.controller.spec.ts`:
- Around line 64-68: Update the AuthService mock setup around the rootContainer
registration: create the mock without the inline ability override, then assign
the real createMongoAbility result to its ability property afterward so CASL
internals are not recursively mocked. Ensure authorization assertions use a
throwUnlessCan implementation that delegates to the assigned ability when
needed.
---
Nitpick comments:
In `@apps/deploy-web/src/components/wallet/ConnectManagedWalletButton.tsx`:
- Around line 29-31: Extend ConnectManagedWalletButton.spec.tsx with an
enabled-button click test that invokes the handler and asserts router.push
receives UrlService.onboardingPicker(). Reuse the existing router mock and
button setup while preserving the current enabled and disabled state tests.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 167fa368-db2a-4ca7-ac3d-870ac472a31e
📒 Files selected for processing (55)
apps/api/src/auth/services/ability/ability.service.tsapps/api/src/billing/controllers/wallet/wallet.controller.spec.tsapps/api/src/billing/controllers/wallet/wallet.controller.tsapps/api/src/billing/http-schemas/wallet.schema.tsapps/api/src/billing/routes/index.tsapps/api/src/billing/routes/start-trial/start-trial.router.tsapps/api/src/routers/open-api-handlers.tsapps/api/swagger/openapi.jsonapps/deploy-web/src/components/deployments/DeploymentDetailTopBar/DeploymentDetailTopBar.spec.tsxapps/deploy-web/src/components/deployments/ManifestUpdate/ManifestUpdate.spec.tsxapps/deploy-web/src/components/home/YourAccount/YourAccount.spec.tsxapps/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.spec.tsxapps/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.tsxapps/deploy-web/src/components/onboarding/OnboardingPage.tsxapps/deploy-web/src/components/onboarding/OnboardingStepper/OnboardingStepper.tsxapps/deploy-web/src/components/onboarding/OnboardingView/OnboardingView.tsxapps/deploy-web/src/components/onboarding/VerifyEmailPage/VerifyEmailPage.spec.tsxapps/deploy-web/src/components/onboarding/VerifyEmailPage/VerifyEmailPage.tsxapps/deploy-web/src/components/onboarding/steps/EmailVerificationContainer/EmailVerificationContainer.spec.tsxapps/deploy-web/src/components/onboarding/steps/EmailVerificationContainer/EmailVerificationContainer.tsxapps/deploy-web/src/components/onboarding/steps/EmailVerificationStep/EmailVerificationStep.spec.tsxapps/deploy-web/src/components/onboarding/steps/EmailVerificationStep/EmailVerificationStep.tsxapps/deploy-web/src/components/onboarding/steps/FreeTrialLandingStep/FreeTrialLandingStep.tsxapps/deploy-web/src/components/onboarding/steps/PaymentMethodContainer/PaymentMethodContainer.spec.tsxapps/deploy-web/src/components/onboarding/steps/PaymentMethodContainer/PaymentMethodContainer.tsxapps/deploy-web/src/components/onboarding/steps/PaymentMethodStep/PaymentMethodStep.tsxapps/deploy-web/src/components/onboarding/steps/PaymentMethodsDisplay/EmptyPaymentMethods.tsxapps/deploy-web/src/components/onboarding/steps/PaymentMethodsDisplay/ErrorAlert.tsxapps/deploy-web/src/components/onboarding/steps/PaymentMethodsDisplay/PaymentMethodsDisplay.test.tsxapps/deploy-web/src/components/onboarding/steps/PaymentMethodsDisplay/PaymentMethodsDisplay.tsxapps/deploy-web/src/components/onboarding/steps/PaymentMethodsDisplay/TermsAndConditions.tsxapps/deploy-web/src/components/onboarding/steps/PaymentMethodsDisplay/TrialStartButton.tsxapps/deploy-web/src/components/onboarding/steps/PaymentVerificationCard/PaymentVerificationCard.tsxapps/deploy-web/src/components/onboarding/steps/WelcomeStep/TemplateCard.tsxapps/deploy-web/src/components/onboarding/steps/WelcomeStep/TrialStatusBar.tsxapps/deploy-web/src/components/onboarding/steps/WelcomeStep/WelcomeStep.tsxapps/deploy-web/src/components/wallet/ConnectManagedWalletButton.tsxapps/deploy-web/src/context/WalletProvider/WalletProvider.tsxapps/deploy-web/src/hooks/useManagedWallet.spec.tsxapps/deploy-web/src/hooks/useManagedWallet.tsapps/deploy-web/src/hooks/useOnboardingChrome.spec.tsapps/deploy-web/src/hooks/useOnboardingChrome.tsapps/deploy-web/src/hooks/useReturnTo/useReturnTo.tsapps/deploy-web/src/pages/signup/index.tsxapps/deploy-web/src/queries/queryKeys.tsapps/deploy-web/src/queries/useManagedWalletQuery.spec.tsxapps/deploy-web/src/queries/useManagedWalletQuery.tsapps/deploy-web/src/services/analytics/analytics.service.tsapps/deploy-web/src/services/app-di-container/app-di-container.tsapps/deploy-web/src/utils/urlUtils.tsapps/deploy-web/tests/seeders/wallet.tsapps/deploy-web/tests/ui/pages/HomePage.tsapps/deploy-web/tests/ui/pages/OnboardingPage.tspackages/console-api-types/src/schema.d.tspackages/http-sdk/src/managed-wallet-http/managed-wallet-http.service.ts
💤 Files with no reviewable changes (39)
- apps/deploy-web/src/components/home/YourAccount/YourAccount.spec.tsx
- apps/api/src/billing/routes/index.ts
- apps/deploy-web/src/components/onboarding/steps/WelcomeStep/TrialStatusBar.tsx
- apps/deploy-web/src/components/deployments/ManifestUpdate/ManifestUpdate.spec.tsx
- apps/deploy-web/src/components/onboarding/steps/WelcomeStep/TemplateCard.tsx
- apps/deploy-web/src/components/onboarding/OnboardingPage.tsx
- apps/deploy-web/src/pages/signup/index.tsx
- apps/deploy-web/src/components/onboarding/steps/PaymentMethodsDisplay/ErrorAlert.tsx
- apps/deploy-web/tests/seeders/wallet.ts
- apps/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.spec.tsx
- apps/api/swagger/openapi.json
- apps/deploy-web/src/components/onboarding/steps/PaymentMethodContainer/PaymentMethodContainer.spec.tsx
- apps/deploy-web/src/components/onboarding/steps/PaymentMethodsDisplay/TermsAndConditions.tsx
- apps/api/src/routers/open-api-handlers.ts
- packages/http-sdk/src/managed-wallet-http/managed-wallet-http.service.ts
- apps/deploy-web/src/components/onboarding/steps/EmailVerificationContainer/EmailVerificationContainer.tsx
- apps/deploy-web/src/components/onboarding/steps/PaymentMethodStep/PaymentMethodStep.tsx
- apps/deploy-web/src/components/onboarding/steps/WelcomeStep/WelcomeStep.tsx
- apps/deploy-web/src/components/onboarding/steps/PaymentMethodsDisplay/PaymentMethodsDisplay.tsx
- apps/deploy-web/src/components/onboarding/steps/FreeTrialLandingStep/FreeTrialLandingStep.tsx
- apps/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.tsx
- apps/deploy-web/src/utils/urlUtils.ts
- apps/deploy-web/src/components/onboarding/steps/PaymentMethodContainer/PaymentMethodContainer.tsx
- packages/console-api-types/src/schema.d.ts
- apps/deploy-web/tests/ui/pages/OnboardingPage.ts
- apps/deploy-web/src/components/onboarding/OnboardingStepper/OnboardingStepper.tsx
- apps/deploy-web/src/services/analytics/analytics.service.ts
- apps/deploy-web/src/queries/queryKeys.ts
- apps/deploy-web/src/components/deployments/DeploymentDetailTopBar/DeploymentDetailTopBar.spec.tsx
- apps/deploy-web/src/components/onboarding/steps/PaymentMethodsDisplay/PaymentMethodsDisplay.test.tsx
- apps/deploy-web/src/components/onboarding/steps/PaymentMethodsDisplay/TrialStartButton.tsx
- apps/api/src/billing/routes/start-trial/start-trial.router.ts
- apps/deploy-web/src/components/onboarding/steps/PaymentMethodsDisplay/EmptyPaymentMethods.tsx
- apps/deploy-web/src/components/onboarding/steps/EmailVerificationStep/EmailVerificationStep.spec.tsx
- apps/api/src/billing/http-schemas/wallet.schema.ts
- apps/deploy-web/src/components/onboarding/OnboardingView/OnboardingView.tsx
- apps/deploy-web/src/components/onboarding/steps/EmailVerificationStep/EmailVerificationStep.tsx
- apps/deploy-web/src/components/onboarding/steps/PaymentVerificationCard/PaymentVerificationCard.tsx
- apps/deploy-web/src/components/onboarding/steps/EmailVerificationContainer/EmailVerificationContainer.spec.tsx
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
6ffcae5 to
963921a
Compare
7dafe5b to
aa4fbb9
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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.
Inline comments:
In
`@apps/deploy-web/src/components/auth/PasswordlessAuth/PasswordlessAuth.spec.tsx`:
- Around line 86-98: Update the authenticated test around PasswordlessAuth to
inject a BootLoading mock, retain the existing assertions that EmailCodeStart
and EmailCodeVerify are not called, and add a meaningful assertion that the
BootLoading mock is rendered.
In
`@apps/deploy-web/src/lib/nextjs/api-routes-specs/auth-email-code-verify.spec.ts`:
- Around line 30-39: Update the test around callHandler and the setSession
assertion to also verify subscribedToNewsletter is true alongside userId and
username, ensuring every supplied user setting is preserved in the persisted
session user.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4fb540f3-953c-4892-8545-fd430b396c27
📒 Files selected for processing (4)
apps/deploy-web/src/components/auth/PasswordlessAuth/PasswordlessAuth.spec.tsxapps/deploy-web/src/components/auth/PasswordlessAuth/PasswordlessAuth.tsxapps/deploy-web/src/lib/nextjs/api-routes-specs/auth-email-code-verify.spec.tsapps/deploy-web/src/pages/api/auth/email-code-verify.ts
aa4fbb9 to
5a48071
Compare
5a48071 to
8916424
Compare
Trial wallet provisioning is fully server-side: the wallet is ensured at registration, activation is dispatched off registration/email-verification, and the UI polls until it is ready. The client-triggered POST /v1/start-trial path is now redundant, and the /signup wizard that used to drive it is already gone; its client plumbing was left dead-exposed in the managed wallet context. Remove POST /v1/start-trial (router, controller shim, request/response schemas, the unused create-UserWallet ability) and its dead client: ManagedWalletHttpService.createWallet, useCreateManagedWalletMutation, and the createWallet/isWalletCreating/walletError plumbing in WalletProvider and useManagedWallet (now reporting-only). Drop the trial_started interceptor and event. Swagger and console-api-types regenerated. Also fix the passwordless email verification flow this leaves as the sole entry point. After the code was verified the handler cleared the persisted email, and the anon->authed transition remounts the auth screen from an ancestor provider, re-reading the now-empty email; the "missing email -> entry" guard then fired and bounced back to the email step, clobbering the success navigation. Gate that guard on the anonymous state, drive the post-verify redirect from the authenticated state, and render the boot loader instead of the auth forms while an authenticated visitor is redirected away (removing the login-form flash). Persist the resolved user settings on the verify session so it carries userId like the OAuth callback path.
8916424 to
660b114
Compare
There was a problem hiding this comment.
I reviewed this PR and didn't find any new bugs. Given the size (XL) and that it touches authentication flow logic and removes a public API endpoint, a human look is still worthwhile.
- Checked the
/v1/start-trialremoval end-to-end: route, controller, schemas, OpenAPI/snapshot regeneration, and SDK client — all consistently pruned, no dangling references found. - Checked the
UserWalletability change (createremoved) against the deleted route — no other caller relied on create-ability. - Checked the passwordless auth redirect fix (anonymous-gated guard, boot-loader render, post-verify navigation) against the described remount bug — the fix targets the right condition.
- Prior bug-hunter findings from earlier runs (docs snapshot, dead schema fields, stale E2E helper, dead analytics events) all appear resolved in the current diff.
Extended reasoning...
Overview
This PR decommissions the legacy POST /v1/start-trial endpoint and its client plumbing (route, controller shim, schemas, SDK method, mutation hook, WalletProvider/useManagedWallet creation state, analytics interceptor), and separately fixes a passwordless email-verification bug where a successful login bounced back to the email-entry step due to a remount clearing the persisted email. Changes span apps/api (routing, ability rules, OpenAPI generation) and apps/deploy-web (auth flow, wallet hooks, analytics types), plus generated SDK/OpenAPI artifacts.
Security risks
The AbilityService change narrows UserWallet permissions for REGULAR_USER/REGULAR_PAYING_USER from ["create","read","sign"] to ["read","sign"], consistent with removing the only code path that used create. No new attack surface is introduced; if anything this reduces surface by removing an endpoint and an ability grant. The auth flow fix changes redirect/guard logic based on authenticated state — a subtle misordering here could theoretically strand or misroute a user, but it doesn't create an authorization bypass since it only affects client-side navigation, not server-side access control.
Level of scrutiny
This warrants more than a rubber stamp: it's an XL PR touching route removal (API contract change), ability rules, and authentication UI flow — areas where subtle regressions (e.g. redirect loops, flashing forms) are easy to introduce and hard to catch outside manual/E2E testing. The bug-hunting system already went through several rounds here and the issues raised (CI-breaking snapshot, dead-code E2E helper, dead schema fields, duplicate loading flags) all appear to have been addressed in the current diff. This run found nothing further.
Other factors
Test coverage is substantial and was updated in lockstep with the behavior changes (new tests for the boot-loader render, the authenticated-redirect guard, and the session merge). CodeRabbit also left minor test-assertion suggestions (assert BootLoading renders, assert the full merged settings) which are non-blocking polish. Given the breadth of surface changed and the auth-adjacent nature of the fix, I'm deferring for a human pass rather than approving outright.
2 participants
Why
Trial wallet provisioning is now fully server-side: the wallet is ensured at registration,
activation is dispatched off registration/email-verification, and the UI polls until it is ready.
The client-triggered
POST /v1/start-trialpath — and the/signupwizard that used to drive it(already removed) — are redundant, but their client plumbing was left dead-exposed in the managed
wallet context.
Removing that wizard also makes passwordless email login the sole entry point, which exposed a bug:
after the code was verified the app bounced back to the email step (and flashed the login form)
instead of proceeding, so a fresh sign-in never landed on the app; only a manual refresh recovered.
Closes CON-756
What
Decommission
POST /v1/start-trialUserWalletability.
ManagedWalletHttpService.createWallet,useCreateManagedWalletMutation,and the
createWallet/isWalletCreating/walletErrorplumbing inWalletProvideranduseManagedWallet(now reporting-only).trial_startedresponse interceptor and analytics event.swagger/openapi.jsonandconsole-api-types.API contract change:
POST /v1/start-trialis removed. It has no remaining consumers — trialprovisioning happens server-side at registration.
Fix the passwordless email verification flow
Root cause: the verify handler cleared the persisted email, and the anonymous→authenticated
transition remounts the auth screen (an ancestor provider swaps on login), re-reading the now-empty
email. The "missing email → go back to entry" guard then fired and
router.replace'd back to theemail step, clobbering the post-verify navigation; the login form also flashed during the outgoing
redirect.
deep-links/reloads, not for the post-verify transition).
away — removing the login-form flash and bridging into the destination's loader.
userId, matching theOAuth callback path.
Summary by CodeRabbit