feat(onboarding): gate the app behind onboarding and route deploys through configure - #3400
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 (62)
💤 Files with no reviewable changes (7)
✅ Files skipped from review due to trivial changes (5)
🚧 Files skipped from review as they are similar to previous changes (48)
📝 WalkthroughWalkthroughThis PR switches onboarding-related feature-flag checks to ChangesOnboarding redesign rollout
Estimated code review effort: 4 (Complex) | ~60 minutes E2E Test Infrastructure and Onboarding Specs
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested labels: Suggested reviewers: ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/deploy-web/src/components/new-deployment/TemplateList.tsx (1)
90-110: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRoute uploaded SDL through
newDeploymentUrl(...)
apps/deploy-web/src/components/new-deployment/TemplateList.tsx:106still pushesUrlService.newDeployment({ step: RouteStep.editDeployment })directly. The redesign hook sendsedit-deploymentintents to/new-deployment/configure, so this upload path will skip the new screen while the other builder flows do not.🤖 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/new-deployment/TemplateList.tsx` around lines 90 - 110, The file upload flow in onFileSelect still navigates directly with UrlService.newDeployment, bypassing the redesigned configure screen. Update the router.push call in TemplateList so uploaded SDL uses newDeploymentUrl(...) for the edit-deployment intent, matching the other builder flows and routing to /new-deployment/configure instead of going straight to editDeployment.
🧹 Nitpick comments (1)
apps/deploy-web/src/hooks/useEnsureTrialStarted.ts (1)
42-53: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
isStartingTrialduplicates a checkuseManagedWallet.isLoadingalready makes.
useManagedWallet'sisLoadingalready folds inisCreatingManagedWallet = useIsMutating({ mutationKey: ...createMutationKey }) > 0— the exact same global-mutation-cache check the docstring justifies adding here. SinceisLoadingalready gates the effect (line 49:if (isLoading) return;), the seconduseIsMutatingsubscription andisStartingTrialcheck are dead weight: they can never be true whileisLoadingis false. Harmless today, but a second independent computation of the same condition — using a different code path — risks silently diverging if either implementation changes.Consider dropping the duplicate subscription and relying solely on
isLoadingfromuseManagedWallet, or centralizing the mutation-in-flight logic so both sites derive from one source.🤖 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/hooks/useEnsureTrialStarted.ts` around lines 42 - 53, The hook in useEnsureTrialStarted redundantly derives the in-flight create state with useIsMutating via isStartingTrial, but useManagedWallet already exposes the same condition through isLoading. Remove the duplicate subscription and the isStartingTrial guard from useEffect, and rely on useManagedWallet’s isLoading (or centralize the shared mutation state in useManagedWallet) so the effect uses a single source of truth.
🤖 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/new-deployment/RedirectMappableBuilderToConfigure/RedirectMappableBuilderToConfigure.tsx`:
- Around line 23-29: The redirect logic in RedirectMappableBuilderToConfigure is
still treating the CI/CD template as mappable, which causes direct
new-deployment links with templateId to be redirected too early. Update the
isMappable/shouldRedirect check to reuse the same git-intent predicate used
elsewhere, or explicitly exclude CI_CD_TEMPLATE_ID so NewDeploymentContainer can
handle that case correctly.
---
Outside diff comments:
In `@apps/deploy-web/src/components/new-deployment/TemplateList.tsx`:
- Around line 90-110: The file upload flow in onFileSelect still navigates
directly with UrlService.newDeployment, bypassing the redesigned configure
screen. Update the router.push call in TemplateList so uploaded SDL uses
newDeploymentUrl(...) for the edit-deployment intent, matching the other builder
flows and routing to /new-deployment/configure instead of going straight to
editDeployment.
---
Nitpick comments:
In `@apps/deploy-web/src/hooks/useEnsureTrialStarted.ts`:
- Around line 42-53: The hook in useEnsureTrialStarted redundantly derives the
in-flight create state with useIsMutating via isStartingTrial, but
useManagedWallet already exposes the same condition through isLoading. Remove
the duplicate subscription and the isStartingTrial guard from useEffect, and
rely on useManagedWallet’s isLoading (or centralize the shared mutation state in
useManagedWallet) so the effect uses a single source of truth.
🪄 Autofix (Beta)
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: 4a522be9-833e-4c04-95f8-c77104656861
📒 Files selected for processing (62)
apps/api/src/bid-screening/routes/bid-screening.router.tsapps/api/src/billing/services/wallet-initializer/wallet-initializer.service.spec.tsapps/api/src/billing/services/wallet-initializer/wallet-initializer.service.tsapps/api/src/core/services/feature-flags/feature-flags.tsapps/deploy-web/src/components/auth/PasswordAuth/PasswordAuth.spec.tsxapps/deploy-web/src/components/auth/PasswordAuth/PasswordAuth.tsxapps/deploy-web/src/components/auth/PasswordlessAuth/PasswordlessAuth.spec.tsxapps/deploy-web/src/components/auth/PasswordlessAuth/PasswordlessAuth.tsxapps/deploy-web/src/components/deployments/ConfigureDeployment/AutoDeployFlow/AutoDeployFlow.spec.tsxapps/deploy-web/src/components/deployments/ConfigureDeployment/AutoDeployFlow/AutoDeployFlow.tsxapps/deploy-web/src/components/deployments/ConfigureDeployment/ConfigureDeploymentForm/ConfigureDeploymentForm.spec.tsxapps/deploy-web/src/components/deployments/ConfigureDeployment/ConfigureDeploymentForm/ConfigureDeploymentForm.tsxapps/deploy-web/src/components/deployments/ConfigureDeployment/useCreateDeployment/useCreateDeployment.spec.tsapps/deploy-web/src/components/deployments/ConfigureDeployment/useCreateDeployment/useCreateDeployment.tsapps/deploy-web/src/components/deployments/ConfigureDeployment/useDeploymentFlow/useDeploymentFlow.tsapps/deploy-web/src/components/deployments/DeploymentList.tsxapps/deploy-web/src/components/deployments/ReclamationCard/ReclamationCard.spec.tsxapps/deploy-web/src/components/deployments/ReclamationCard/ReclamationCard.tsxapps/deploy-web/src/components/home/AccountHeader.tsxapps/deploy-web/src/components/home/NoDeploymentsState.tsxapps/deploy-web/src/components/layout/Sidebar.tsxapps/deploy-web/src/components/new-deployment/RedirectMappableBuilderToConfigure/RedirectMappableBuilderToConfigure.spec.tsxapps/deploy-web/src/components/new-deployment/RedirectMappableBuilderToConfigure/RedirectMappableBuilderToConfigure.tsxapps/deploy-web/src/components/new-deployment/TemplateList.spec.tsxapps/deploy-web/src/components/new-deployment/TemplateList.tsxapps/deploy-web/src/components/onboarding-picker/OnboardingPickerPage.spec.tsxapps/deploy-web/src/components/onboarding/OnboardingRedirect/OnboardingRedirect.tsxapps/deploy-web/src/components/onboarding/OnboardingRedirectEffect/OnboardingRedirectEffect.spec.tsxapps/deploy-web/src/components/onboarding/OnboardingRedirectEffect/OnboardingRedirectEffect.tsxapps/deploy-web/src/components/onboarding/RequireOnboarding/RequireOnboarding.spec.tsxapps/deploy-web/src/components/onboarding/RequireOnboarding/RequireOnboarding.tsxapps/deploy-web/src/hooks/useAutoDeploymentFlow/useAutoDeploymentFlow.tsapps/deploy-web/src/hooks/useEnsureTrialStarted.spec.tsapps/deploy-web/src/hooks/useEnsureTrialStarted.tsapps/deploy-web/src/hooks/useIsOnboarded.tsapps/deploy-web/src/hooks/useManagedWallet.tsapps/deploy-web/src/hooks/useNewDeploymentUrl/useNewDeploymentUrl.spec.tsapps/deploy-web/src/hooks/useNewDeploymentUrl/useNewDeploymentUrl.tsapps/deploy-web/src/pages/_app.tsxapps/deploy-web/src/pages/deployments/[dseq]/index.tsxapps/deploy-web/src/pages/deployments/index.tsxapps/deploy-web/src/pages/new-deployment/configure/[[...dseq]].tsxapps/deploy-web/src/pages/new-deployment/index.tsxapps/deploy-web/src/pages/onboarding/index.tsxapps/deploy-web/src/types/feature-flags.tsapps/deploy-web/tests/ui/actions/auth.tsapps/deploy-web/tests/ui/actions/feature-flags.tsapps/deploy-web/tests/ui/configure-deployment-flow.spec.tsapps/deploy-web/tests/ui/deploy-from-a-template.spec.tsapps/deploy-web/tests/ui/fixture/base-test.tsapps/deploy-web/tests/ui/fixture/test-env.config.tsapps/deploy-web/tests/ui/managed-wallet-alerts.spec.tsapps/deploy-web/tests/ui/managed-wallet-deployment.spec.tsapps/deploy-web/tests/ui/managed-wallet-onboarding.spec.tsapps/deploy-web/tests/ui/onboarding-gate.spec.tsapps/deploy-web/tests/ui/onboarding-journey.spec.tsapps/deploy-web/tests/ui/onboarding-picker-unlock-trial.spec.tsapps/deploy-web/tests/ui/onboarding-quick-deploy.spec.tsapps/deploy-web/tests/ui/pages/ConfigureDeploymentPage.tsapps/deploy-web/tests/ui/pages/OnboardingPickerPage.tsapps/deploy-web/tests/ui/services/email-verification/auth0-ticket.strategy.tsapps/deploy-web/tests/ui/services/email-verification/index.ts
💤 Files with no reviewable changes (7)
- apps/deploy-web/src/hooks/useIsOnboarded.ts
- apps/deploy-web/tests/ui/services/email-verification/auth0-ticket.strategy.ts
- apps/deploy-web/src/components/onboarding/OnboardingRedirectEffect/OnboardingRedirectEffect.spec.tsx
- apps/deploy-web/tests/ui/onboarding-quick-deploy.spec.ts
- apps/deploy-web/src/components/onboarding/OnboardingRedirectEffect/OnboardingRedirectEffect.tsx
- apps/deploy-web/src/components/onboarding/OnboardingRedirect/OnboardingRedirect.tsx
- apps/deploy-web/tests/ui/fixture/test-env.config.ts
…rough configure Consolidate the onboarding-redesign and bid-screening flags into a single onboarding_redesign_v1 flag (FE + API) and add a blocking onboarding gate: a not-onboarded user is confined to the onboarding + first-deploy path (the onboarding picker, /new-deployment/configure, a single deployment's detail), while onboarded users roam the app freely. "Onboarded" is read client-side from whether the user has ever created a lease. The legacy OnboardingRedirectEffect and useIsOnboarded are removed in favour of the gate. Route deploy entry points through the classic picker: a bare "new deployment" opens the picker, a chosen template or an edit-deployment intent opens the configure screen, and git/redeploy/VM intents keep the classic builder. Make deployment creation wait for the trial wallet: the start-trial request survives navigation (a stable mutation key plus a global in-flight check), and the create is held until the wallet can broadcast rather than firing early. The gate no longer waits on the leases query for allow-list pages, so the trial wallet arriving mid-deploy never remounts an in-progress configure/auto flow.
fd9feea to
91755c2
Compare
Why
With the onboarding redesign we want a single, enforced first-run path: a brand-new user is guided through onboarding and their first deployment, and can't wander into the rest of the app until they've actually deployed something. Before this PR the redesign was split across two flags and had no real gate — a not-onboarded user could reach any page, and the old wallet-based
OnboardingRedirectEffectonly covered part of it. New-deployment entry points also bypassed the new configure screen, and on a fresh trial the deployment could be created before the trial wallet was ready to sign it.Fixes CON-317
What
Flag consolidation
console_onboarding_redesign+bid_screeninginto a singleonboarding_redesign_v1flag across the FE flag type and the APIFeatureFlags(including bid-screening's gate).Onboarding gate (
RequireOnboarding)_app— nothing renders until the user's onboarding state is known (no flash of gated content).useAllLeases); a user whose deployments are all closed still counts./new-deployment/configure*, and a single deployment's detail (/deployments/{dseq}) — and are bounced to/onboardingfrom everywhere else (including the/deploymentslist)./onboarding.OnboardingRedirectEffectanduseIsOnboardedin favor of the single gate.Deploy-entry routing
/new-deploymentdeployment-type/template picker stays the deploy entry: a bare Deploy opens the picker; a chosen template or anedit-deploymentintent opens the configure screen; git, redeploy, and VM (deploy-linux) intents keep the classic builder. Wired viauseNewDeploymentUrl(in-app links) andRedirectMappableBuilderToConfigure(inbound/new-deployment?...links).Deployment creation waits for the trial wallet
useEnsureTrialStarted), so navigating onboarding → configure no longer fires duplicate start-trial calls.useCreateDeployment): it holds the broadcast until the trial wallet can sign, instead of firing prematurely and failing. Both the manual configure flow and the auto-deploy flow funnel through it.Tests
onboarding-gate(the routing/gate contract) andonboarding-journey(new-user template auto-deploy + bring-your-own-image). Registration goes through passwordless email OTP; existing-user login uses email + password.deploy-linux) specs run under both;deploy-from-a-templateandmanaged-wallet-alertsare migrated to the redesign flow.RequireOnboarding,useNewDeploymentUrl,useCreateDeployment,useEnsureTrialStarted, andRedirectMappableBuilderToConfigure.Summary by CodeRabbit