diff --git a/packages/app/src/onboarding/internal/model.test.ts b/packages/app/src/onboarding/internal/model.test.ts index ee3a8b00c9..5838876e91 100644 --- a/packages/app/src/onboarding/internal/model.test.ts +++ b/packages/app/src/onboarding/internal/model.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, it } from "vitest"; +import { describe, expect, it, vi } from "vitest"; import { createOnboardingModel, type OnboardingCompletionStorage, @@ -134,7 +134,6 @@ describe("onboarding model", () => { advance(model); advance(model); advance(model); - advance(model); expect(model.getState()).toMatchObject({ stepIndex: 3 }); model.goTo("providers"); expect(model.getState()).toMatchObject({ stepIndex: 1 }); @@ -162,6 +161,25 @@ describe("onboarding model", () => { expect(model.getState()).toMatchObject({ status: "open", stepIndex: 2 }); }); + it("moving on from the last step finishes setup, unless the user left that step first", async () => { + const storage = new MemoryCompletionStorage(); + const model = await openModel(storage); + model.applySetupHost(FRESH_HOST, { includePhoneStep: false }); + model.goTo("orchestration"); + + // Finish is applying choices when the user goes back to Projects. + const finishingFrom = currentVisit(model); + model.back(); + model.next(finishingFrom); + expect(model.getState()).toMatchObject({ status: "open", stepIndex: 2 }); + expect(storage.completed).toBe(false); + + advance(model); + advance(model); + await vi.waitFor(() => expect(storage.completed).toBe(true)); + expect(model.getState()).toEqual({ status: "hidden" }); + }); + it("records completion when setup closes, so it does not open again", async () => { const storage = new MemoryCompletionStorage(); const model = await openModel(storage); diff --git a/packages/app/src/onboarding/internal/model.ts b/packages/app/src/onboarding/internal/model.ts index 1ac2f01f4a..f8aaf46d4f 100644 --- a/packages/app/src/onboarding/internal/model.ts +++ b/packages/app/src/onboarding/internal/model.ts @@ -46,8 +46,9 @@ export interface OnboardingModel { applySetupHost(host: OnboardingSetupHost | null, options: OnboardingStepOptions): void; reopen(serverId: string, options: OnboardingStepOptions): void; /** - * Moves on from the step visit the caller saw. A no-op once that visit ended, so an action that - * finishes late never moves a step the user left or a setup they closed and reopened. + * Moves on from the step visit the caller saw; from the last step, that finishes setup. A no-op + * once that visit ended, so an action that finishes late never moves a step the user left or + * closes a setup they went back in, or closed and reopened. */ next(visit: number): void; back(): void; @@ -129,6 +130,12 @@ export function createOnboardingModel(input: { const ready = loadCompletion(); + async function complete() { + completion = "completed"; + setState({ status: "hidden" }); + await input.storage.markCompleted(); + } + return { ready, getState: () => state, @@ -146,7 +153,9 @@ export function createOnboardingModel(input: { setState(openState(serverId, options)); }, next(visit) { - if (state.status === "open" && state.visit === visit) moveTo(state.stepIndex + 1); + if (state.status !== "open" || state.visit !== visit) return; + if (state.stepIndex === state.steps.length - 1) void complete(); + else moveTo(state.stepIndex + 1); }, back() { if (state.status === "open") moveTo(state.stepIndex - 1); @@ -156,10 +165,6 @@ export function createOnboardingModel(input: { const stepIndex = state.steps.indexOf(step); if (stepIndex >= 0) moveTo(stepIndex); }, - async complete() { - completion = "completed"; - setState({ status: "hidden" }); - await input.storage.markCompleted(); - }, + complete, }; } diff --git a/packages/app/src/onboarding/internal/onboarding-modal.tsx b/packages/app/src/onboarding/internal/onboarding-modal.tsx index c36c4855f1..0eb0418185 100644 --- a/packages/app/src/onboarding/internal/onboarding-modal.tsx +++ b/packages/app/src/onboarding/internal/onboarding-modal.tsx @@ -42,11 +42,8 @@ export function OnboardingModal({ model, state }: { model: OnboardingModel; stat }, [model]); const handleBack = useCallback(() => model.back(), [model]); const { visit } = state; - // Moving on from the last step finishes setup; any step's own work runs before that. - const handleAdvance = useCallback(() => { - if (isLast) void model.complete(); - else model.next(visit); - }, [isLast, model, visit]); + // A step's own work runs before it moves on; from the last step, moving on finishes setup. + const handleAdvance = useCallback(() => model.next(visit), [model, visit]); const handleAddSuggestions = useCallback(() => { void (async () => { if (await addChecked()) handleAdvance();