feat(onboarding): let users permanently skip the onboarding flow - #3525
Open
baktun14 wants to merge 1 commit into
Open
feat(onboarding): let users permanently skip the onboarding flow#3525baktun14 wants to merge 1 commit into
baktun14 wants to merge 1 commit into
Conversation
Adds a permanent, cross-device skip to the new onboarding flow so a no-lease user is no longer trapped in the picker or the stripped auto-deploy screen. Backend: new nullable `onboardingSkippedAt` timestamp on the user settings table (migration 0034), exposed on `/v1/user/me`, and a new idempotent `POST /v1/user/skipOnboarding` that sets it once via a set-if-null update so the original skip time is never overwritten. Frontend: a `useSkipOnboarding` hook that records the event, persists the flag, awaits a session refresh, then routes to the deployments list; the onboarding gate and stripped-chrome hook both treat a skipped user as onboarded; and a shared SkipOnboardingButton on the picker and the minimal nav of the auto-deploy screen. Closes CON-750
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3525 +/- ##
==========================================
- Coverage 74.18% 73.23% -0.96%
==========================================
Files 1142 1054 -88
Lines 29721 27399 -2322
Branches 7447 6990 -457
==========================================
- Hits 22049 20066 -1983
+ Misses 6771 6462 -309
+ Partials 901 871 -30
*This pull request uses carry forward flags. Click here to find out more.
🚀 New features to boost your workflow:
|
Contributor
📝 WalkthroughWalkthroughAdds a persisted onboarding-skip timestamp, authenticated API endpoint, client-side skip controls and mutation handling, analytics tracking, and onboarding gate behavior for skipped users. ChangesOnboarding skip flow
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/api/src/user/services/user/user.service.integration.ts (1)
342-348: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse faker for
usernameinstead ofDate.now().
usernameis unique-constrained; millisecond timestamps can collide across parallel workers or fast consecutive creates.userIdalready usesfaker.string.uuid()— do the same here.♻️ Proposed change
- username: `test-user-${Date.now()}`, + username: `test-user-${faker.string.uuid()}`,Also applies to: 358-364
🤖 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/api/src/user/services/user/user.service.integration.ts` around lines 342 - 348, Replace the Date.now()-based username values in the userRepository.create test fixtures with a faker-generated username, matching the existing faker usage for userId. Apply this change to both create blocks, including the additional occurrence.apps/deploy-web/src/components/onboarding/RequireOnboarding/RequireOnboarding.spec.tsx (1)
156-162: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winKeep the skipped-user fixture type-checked.
The user mock is created with a broad
as nevercast, which can hide mismatches withCustomUserProfile. Usemock<CustomUserProfile>({...})instead so future user-shape changes fail at compile time.🤖 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/onboarding/RequireOnboarding/RequireOnboarding.spec.tsx` around lines 156 - 162, Update the buildDependencies user fixture to construct the user value with mock<CustomUserProfile> instead of the broad as never cast, while preserving the existing logged-out, userId, and onboardingSkippedAt behavior. Ensure the fixture remains type-checked against CustomUserProfile.Source: Coding guidelines
🤖 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/hooks/useSkipOnboarding.ts`:
- Around line 41-42: Prevent useSkipOnboarding from navigating when checkSession
fails: report the refresh error and return before router.push, unless local
skipped state is updated first. Update
apps/deploy-web/src/hooks/useSkipOnboarding.spec.ts lines 37-45 to assert no
navigation occurs after a failed refresh; the hook change is required in
apps/deploy-web/src/hooks/useSkipOnboarding.ts lines 41-42.
---
Nitpick comments:
In `@apps/api/src/user/services/user/user.service.integration.ts`:
- Around line 342-348: Replace the Date.now()-based username values in the
userRepository.create test fixtures with a faker-generated username, matching
the existing faker usage for userId. Apply this change to both create blocks,
including the additional occurrence.
In
`@apps/deploy-web/src/components/onboarding/RequireOnboarding/RequireOnboarding.spec.tsx`:
- Around line 156-162: Update the buildDependencies user fixture to construct
the user value with mock<CustomUserProfile> instead of the broad as never cast,
while preserving the existing logged-out, userId, and onboardingSkippedAt
behavior. Ensure the fixture remains type-checked against CustomUserProfile.
🪄 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: 941167de-19ad-4c97-abe0-bf8892281452
📒 Files selected for processing (27)
apps/api/drizzle/0034_add_onboarding_skipped_at.sqlapps/api/drizzle/meta/0034_snapshot.jsonapps/api/drizzle/meta/_journal.jsonapps/api/src/app/console.tsapps/api/src/core/services/job-queue/job-queue.service.tsapps/api/src/user/controllers/user/user.controller.spec.tsapps/api/src/user/controllers/user/user.controller.tsapps/api/src/user/model-schemas/user/user.schema.tsapps/api/src/user/routes/user-settings/user-settings.router.tsapps/api/src/user/schemas/user.schema.tsapps/api/src/user/services/user/user.service.integration.tsapps/api/src/user/services/user/user.service.spec.tsapps/api/src/user/services/user/user.service.tsapps/api/test/seeders/user.seeder.tsapps/deploy-web/src/components/layout/Nav.tsxapps/deploy-web/src/components/layout/TopNav/TopNav.tsxapps/deploy-web/src/components/onboarding-picker/OnboardingPickerPage.tsxapps/deploy-web/src/components/onboarding-picker/SkipOnboardingButton/SkipOnboardingButton.spec.tsxapps/deploy-web/src/components/onboarding-picker/SkipOnboardingButton/SkipOnboardingButton.tsxapps/deploy-web/src/components/onboarding/RequireOnboarding/RequireOnboarding.spec.tsxapps/deploy-web/src/components/onboarding/RequireOnboarding/RequireOnboarding.tsxapps/deploy-web/src/hooks/useOnboardingChrome.spec.tsapps/deploy-web/src/hooks/useOnboardingChrome.tsapps/deploy-web/src/hooks/useSkipOnboarding.spec.tsapps/deploy-web/src/hooks/useSkipOnboarding.tsapps/deploy-web/src/services/analytics/analytics.service.tsapps/deploy-web/src/types/user.ts
Comment on lines
+41
to
+42
| await checkSession().catch(() => undefined); | ||
| router.push(urlService.deploymentList()); |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Avoid navigation before the skipped state reaches the gate. A failed session refresh leaves RequireOnboarding with onboardingSkippedAt unset, so navigating a lease-less user immediately re-enters onboarding.
apps/deploy-web/src/hooks/useSkipOnboarding.ts#L41-L42: on refresh failure, report the error and return beforerouter.push, or update local user state before navigation.apps/deploy-web/src/hooks/useSkipOnboarding.spec.ts#L37-L45: expect no navigation when refresh fails, unless local skipped state is explicitly updated first.
📍 Affects 2 files
apps/deploy-web/src/hooks/useSkipOnboarding.ts#L41-L42(this comment)apps/deploy-web/src/hooks/useSkipOnboarding.spec.ts#L37-L45
🤖 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/useSkipOnboarding.ts` around lines 41 - 42, Prevent
useSkipOnboarding from navigating when checkSession fails: report the refresh
error and return before router.push, unless local skipped state is updated
first. Update apps/deploy-web/src/hooks/useSkipOnboarding.spec.ts lines 37-45 to
assert no navigation occurs after a failed refresh; the hook change is required
in apps/deploy-web/src/hooks/useSkipOnboarding.ts lines 41-42.
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 free
to 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.
Why
The new onboarding flow has no exit. An authenticated user with a managed wallet but zero leases is force-routed to
/onboarding, and "onboarded" is derived purely from on-chain lease count, so a broken auto-deploy can strand the user on the stripped/new-deployment/configurescreen with only a logout. This adds a permanent, cross-device skip so the gate never routes them back in.Closes CON-750
What
Backend (apps/api)
onboardingSkippedAttimestamp on theuserSettingtable (migration0034), exposed on/v1/user/me.POST /v1/user/skipOnboardingthat sets the flag once via a set-if-null update, so the original skip time is never overwritten by repeat calls.Frontend (apps/deploy-web)
useSkipOnboardinghook: tracksonboarding_skipped, persists the flag, awaits a session refresh, then routes to the deployments list (navigates even if the refresh fails; skips navigation if the persist fails).RequireOnboarding) and the stripped-chrome hook (useOnboardingChrome) both treat a skipped user as onboarded, known as soon as identity is, so a skipper never waits on leases and is never sent back into the funnel.SkipOnboardingButtonon the picker header (source: "picker") and the minimal nav of the auto-deploy screen (source: "auto_deploy").Skipping is pure navigation and touches nothing on-chain: any orphaned in-flight deployment is left intact for the user to close from
/deployments.The legacy
/signupstepper is intentionally untouched (out of scope; slated for removal).Summary by CodeRabbit
New Features
Bug Fixes