fix: preserve onboarding resume state on reload#1941
Merged
Ajit-Mehrotra merged 3 commits intomainfrom Mar 20, 2026
Merged
Conversation
- Purpose: make onboarding resume logic rely only on persisted step IDs and preserve the saved Activate License step across reloads. - Before this change: the onboarding draft persisted both currentStepId and currentStepIndex, and the modal could remap a saved ACTIVATE_LICENSE step to SUMMARY while activation data was still loading. - Why that was a problem: users who closed and reopened the tab during onboarding could land on the wrong screen even though the draft still pointed at the license step. - What this change accomplishes: onboarding resume now uses only currentStepId, ignores the legacy index field, and keeps the activation step available during the bootstrap loading window so the saved step is not rewritten incorrectly. - How it works: the draft store no longer exposes or restores currentStepIndex, the modal only syncs step IDs, and the onboarding modal tests now cover step-by-step resume behavior plus the activation-loading race.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughSwitched onboarding state from index-based to ID-based: removed Changes
Sequence DiagramsequenceDiagram
participant User
participant OnboardingModal
participant DraftStore as onboardingDraft
participant ActivationStore as activationCodeDataStore
participant StepComponent
User->>OnboardingModal: open or resume onboarding
OnboardingModal->>DraftStore: restore persisted state
DraftStore-->>OnboardingModal: currentStepId
OnboardingModal->>ActivationStore: read loading & activation state
ActivationStore-->>OnboardingModal: loading = true, activationState = null
alt activation data loading
OnboardingModal->>OnboardingModal: shouldKeepResumedActivationStep = true
OnboardingModal->>StepComponent: render ACTIVATE_LICENSE
StepComponent-->>User: show activation step
ActivationStore->>OnboardingModal: loading = false, activationState available
OnboardingModal->>OnboardingModal: recompute visibility
else activation data loaded
OnboardingModal->>DraftStore: setCurrentStep(currentStepId)
DraftStore-->>OnboardingModal: acknowledge
OnboardingModal->>StepComponent: render appropriate step
StepComponent-->>User: display step
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/__test__/components/Onboarding/OnboardingModal.test.ts`:
- Around line 41-44: The test uses mocked storeToRefs which returned plain {
value } objects so mutations don't trigger reactivity; update the test to create
real Vue refs for loading, hasActivationCode and registrationState (import ref
from 'vue' and use ref(false), ref(true), ref<string|null>('ENOKEYFILE')
respectively), ensure registrationState is typed as string | null so assigning
null in the test is allowed, and replace the plain objects with these refs so
changing .value will retrigger the component and exercise the loading→loaded
transition (references: storeToRefs mock, loading, hasActivationCode,
registrationState in OnboardingModal.test.ts).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: fb1b108b-31a3-460b-a069-89cb2714b862
📒 Files selected for processing (5)
web/__test__/components/Onboarding/OnboardingModal.test.tsweb/__test__/components/Onboarding/onboardingStorageCleanup.test.tsweb/__test__/store/onboardingModalVisibility.test.tsweb/src/components/Onboarding/OnboardingModal.vueweb/src/components/Onboarding/store/onboardingDraft.ts
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1941 +/- ##
==========================================
+ Coverage 51.51% 51.55% +0.04%
==========================================
Files 1025 1025
Lines 70460 70460
Branches 7793 7800 +7
==========================================
+ Hits 36297 36329 +32
+ Misses 34040 34008 -32
Partials 123 123 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
- Purpose: keep the onboarding modal test fixtures aligned with the nullable registration state used by the activation store. - Before this change: the new activation-loading resume test assigned to a mock that was inferred as a non-null string. - Why that was a problem: workspace type-check failed even though the runtime behavior and assertions were correct. - What the new change accomplishes: the mock now reflects the real shape and allows the loading-state test to compile cleanly. - How it works: the hoisted fixture is explicitly typed as , which matches the store contract used by the component.
- Purpose: make the onboarding modal loading-transition test behave like the real Pinia store refs used at runtime. - Before this change: the test mutated plain value-holder objects after mount while storeToRefs was mocked to return the store object directly. - Why that was a problem: the test could pass without actually proving that the component reacts to the activation loading-to-loaded transition. - What the new change accomplishes: the mocked activation loading, activation-code presence, and registration state are now real Vue refs during each test run. - How it works: beforeEach recreates those mocked store fields with ref(...) values, which lets post-mount value changes retrigger the component and satisfies lint, type-check, and the review feedback.
Contributor
|
This plugin has been deployed to Cloudflare R2 and is available for testing. |
elibosley
approved these changes
Mar 20, 2026
elibosley
pushed a commit
that referenced
this pull request
Mar 23, 2026
🤖 I have created a release *beep* *boop* --- ## [4.31.0](v4.30.1...v4.31.0) (2026-03-23) ### Features * **api:** support encrypted array start inputs ([#1944](#1944)) ([018a8d5](018a8d5)) * **onboarding:** add shared loading states ([#1945](#1945)) ([776c8cc](776c8cc)) * Serverside state for onboarding display ([#1936](#1936)) ([682d51c](682d51c)) ### Bug Fixes * **api:** reconcile emhttp state without spinning disks ([#1946](#1946)) ([d3e0b95](d3e0b95)) * **onboarding:** auto-open incomplete onboarding on 7.3+ ([#1940](#1940)) ([f0241a8](f0241a8)) * **onboarding:** replace internal boot native selects ([#1942](#1942)) ([d6ea032](d6ea032)) * preserve onboarding resume state on reload ([#1941](#1941)) ([91f7fe9](91f7fe9)) * recover VM availability after reconnect ([#1947](#1947)) ([e064de7](e064de7)) * Unify callback server payloads ([#1938](#1938)) ([f58fcc0](f58fcc0)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
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
2 participants
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.
Summary
currentStepIndexfrom onboarding draft persistence and resume logic so the flow only relies oncurrentStepIdACTIVATE_LICENSEstep while activation bootstrap data is still loading so the modal does not jump forward toSUMMARYon reloadProblem
When a user closed the tab while onboarding was paused on
ACTIVATE_LICENSE, the saved draft still containedcurrentStepId: "ACTIVATE_LICENSE", but reopening the tab could land them onSUMMARYinstead of the license step.This was happening consistently because the modal rebuilt its visible steps before activation state had finished loading. During that window,
ACTIVATE_LICENSEwas temporarily excluded from the step list, the modal fell back to the nearest visible step, and that fallback rewrote the saved draft toSUMMARY.Root Cause
currentStepIdandcurrentStepIndex, even though the ID is the real source of truthACTIVATE_LICENSEappear unavailable, which triggered the remap and overwrote the saved stepChanges
currentStepIndexfrom the onboarding draft store API and persisted payloadcurrentStepIndexfield when deserializing existing onboarding draftsStepIdACTIVATE_LICENSEin the visible step list during activation bootstrap loading when the persisted step being resumed is the license stepTesting
pnpm --dir web test -- --run web/__test__/components/Onboarding/OnboardingModal.test.ts web/__test__/components/Onboarding/onboardingStorageCleanup.test.ts web/__test__/store/onboardingModalVisibility.test.tsSummary by CodeRabbit
Bug Fixes
Refactor
Tests