diff --git a/src/lib/onboarding/__tests__/checklist.test.ts b/src/lib/onboarding/__tests__/checklist.test.ts index c765877f8..1a807a769 100644 --- a/src/lib/onboarding/__tests__/checklist.test.ts +++ b/src/lib/onboarding/__tests__/checklist.test.ts @@ -2,13 +2,38 @@ import { describe, expect, it } from "vitest"; import { buildChecklist, + checklistOrderFromNeeds, checklistProgress, + CHECKLIST_ITEM_IDS, isProfileComplete, shouldShowChecklist, trendHintFor, visibleChecklist, type ChecklistItemId, } from "../checklist"; +import { + defaultOnboardingSteps, + emptyOnboardingNeeds, + type OnboardingNeeds, + type OnboardingStateDto, +} from "../needs"; + +/** A setup state the flow really produced: Q1 answered, confirm reached. */ +function onboardingState( + overrides: Partial = {}, + needs: Partial = {}, +): OnboardingStateDto { + return { + steps: defaultOnboardingSteps().map((step) => ({ + ...step, + status: "done" as const, + })), + needs: { ...emptyOnboardingNeeds(), recordTarget: "me", ...needs }, + completedAt: "2026-09-10T08:00:00.000Z", + firstResult: null, + ...overrides, + }; +} const completeProfile = { heightCm: 175, @@ -247,3 +272,224 @@ describe("trendHintFor", () => { expect(trendHintFor(99)).toEqual({ kind: "hidden" }); }); }); + +describe("checklistOrderFromNeeds", () => { + function order(needs: Partial) { + return checklistOrderFromNeeds({ ...emptyOnboardingNeeds(), ...needs }); + } + + it("keeps profile first whatever the answers say", () => { + for (const needs of [ + {}, + { medication: "yes" as const }, + { sources: ["withings" as const] }, + { areas: ["sleep" as const] }, + ]) { + expect(order(needs)[0]).toBe("profile"); + } + }); + + it("puts medication second for somebody who takes one on a schedule", () => { + expect(order({ medication: "yes" })[1]).toBe("medication"); + expect(order({ medication: "sometimes" })[1]).toBe("medication"); + expect(order({ medication: "no" })[1]).not.toBe("medication"); + }); + + it("promotes the data source only for a source that is a connection", () => { + expect(order({ sources: ["withings"] }).indexOf("dataSource")).toBe(1); + // Typing readings in and uploading a file are not connections, so they + // say nothing about that row. + expect(order({ sources: ["manual", "file"] }).indexOf("dataSource")).toBe( + 3, + ); + }); + + it("promotes the reading row once an area was chosen", () => { + expect(order({ areas: ["blood-pressure"] })[1]).toBe("measurement"); + }); + + it("orders medication ahead of the data source when both are answered", () => { + const ids = order({ medication: "yes", sources: ["oura"] }); + expect(ids.indexOf("medication")).toBeLessThan(ids.indexOf("dataSource")); + }); + + it("never drops or duplicates a row, for any answer combination", () => { + const combinations: Partial[] = [ + {}, + { medication: "yes" }, + { sources: ["manual"] }, + { sources: ["fitbit"] }, + { areas: ["mood", "labs"] }, + { medication: "sometimes", sources: ["nightscout"], areas: ["glucose"] }, + ]; + for (const needs of combinations) { + const ids = order(needs); + expect([...ids].sort()).toEqual([...CHECKLIST_ITEM_IDS].sort()); + } + }); +}); + +describe("buildChecklist, ordered by the setup answers", () => { + it("keeps the fixed order for a record that never entered the flow", () => { + expect(buildChecklist(inputs()).map((i) => i.id)).toEqual([ + ...CHECKLIST_ITEM_IDS, + ]); + }); + + it("keeps the fixed order while the answers are still being given", () => { + const items = buildChecklist( + inputs({ + onboarding: onboardingState( + { completedAt: null }, + { medication: "yes" }, + ), + }), + ); + // Re-ordering the dashboard under somebody mid-question would be movement + // they did not ask for. + expect(items.map((i) => i.id)).toEqual([...CHECKLIST_ITEM_IDS]); + }); + + it("orders the rows from the answers once the flow is confirmed", () => { + const items = buildChecklist( + inputs({ + onboarding: onboardingState( + {}, + { medication: "yes", sources: ["withings"] }, + ), + }), + ); + expect(items.map((i) => i.id).slice(0, 3)).toEqual([ + "profile", + "medication", + "dataSource", + ]); + expect([...items.map((i) => i.id)].sort()).toEqual( + [...CHECKLIST_ITEM_IDS].sort(), + ); + }); + + it("carries each row's own state through the reordering", () => { + const items = buildChecklist( + inputs({ + medicationCount: 2, + dismissedIds: new Set(["insights"]), + onboarding: onboardingState({}, { medication: "yes" }), + }), + ); + expect(items.find((i) => i.id === "medication")?.done).toBe(true); + expect(items.find((i) => i.id === "insights")?.dismissed).toBe(true); + }); +}); + +describe("shouldShowChecklist and the setup flow", () => { + const doneItems = () => + buildChecklist( + inputs({ + measurementCount: 3, + medicationCount: 1, + dataSourceConnected: true, + notificationsConfigured: true, + insightsConfigured: true, + }), + ); + + it("no longer disappears at five readings for a record that ran the flow", () => { + const items = buildChecklist(inputs({ measurementCount: 10 })); + expect( + shouldShowChecklist({ + onboardingCompletedAt: "2026-01-01T00:00:00Z", + measurementCount: 10, + dismissedAll: false, + items, + onboarding: onboardingState(), + }), + ).toBe(true); + }); + + it("goes when its rows are done, even for a record that ran the flow", () => { + expect( + shouldShowChecklist({ + onboardingCompletedAt: "2026-01-01T00:00:00Z", + measurementCount: 10, + dismissedAll: false, + items: doneItems(), + onboarding: onboardingState(), + }), + ).toBe(false); + }); + + it("goes when the person hides the whole list", () => { + expect( + shouldShowChecklist({ + onboardingCompletedAt: null, + measurementCount: 0, + dismissedAll: true, + items: buildChecklist(inputs()), + onboarding: onboardingState(), + }), + ).toBe(false); + }); + + it("stays while the flow is unfinished, even with every row done", () => { + // The one task the flow offered is not one of the six rows, so "every row + // done" is not the same statement as "the setup finished". + expect( + shouldShowChecklist({ + onboardingCompletedAt: "2026-01-01T00:00:00Z", + measurementCount: 10, + dismissedAll: false, + items: doneItems(), + onboarding: onboardingState({ + firstResult: { + task: "log-reading", + target: null, + completedAt: null, + }, + }), + }), + ).toBe(true); + }); + + it("does not stay pinned by a first-result task the person passed", () => { + const state = onboardingState({ + firstResult: { task: "log-reading", target: null, completedAt: null }, + }); + expect( + shouldShowChecklist({ + onboardingCompletedAt: "2026-01-01T00:00:00Z", + measurementCount: 10, + dismissedAll: false, + items: doneItems(), + onboarding: { + ...state, + steps: state.steps.map((step) => + step.id === "first-result" + ? { ...step, status: "skipped" as const } + : step, + ), + }, + }), + ).toBe(false); + }); + + it("leaves an account that predates the flow on the rule it already had", () => { + // The payload publishes the field for every record, so "nine pending + // steps" must not read as an unfinished setup. + const untouched: OnboardingStateDto = { + steps: defaultOnboardingSteps(), + needs: emptyOnboardingNeeds(), + completedAt: null, + firstResult: null, + }; + expect( + shouldShowChecklist({ + onboardingCompletedAt: "2026-01-01T00:00:00Z", + measurementCount: 10, + dismissedAll: false, + items: buildChecklist(inputs({ measurementCount: 10 })), + onboarding: untouched, + }), + ).toBe(false); + }); +}); diff --git a/src/lib/onboarding/__tests__/needs.test.ts b/src/lib/onboarding/__tests__/needs.test.ts index 7af629791..02ad9b7d1 100644 --- a/src/lib/onboarding/__tests__/needs.test.ts +++ b/src/lib/onboarding/__tests__/needs.test.ts @@ -17,6 +17,7 @@ import { accountHoldsBothUnits, defaultOnboardingSteps, everyOnboardingQuestionSettled, + hasEnteredOnboardingFlow, ONBOARDING_QUESTION_STEP_IDS, emptyOnboardingNeeds, isOnboardingSettled, @@ -224,7 +225,8 @@ describe("applyOnboardingAnswer", () => { describe("isOnboardingSettled", () => { const settled: OnboardingStateDto = { steps: ONBOARDING_STEP_IDS.map((id) => ({ id, status: "done" as const })), - needs: emptyOnboardingNeeds(), + // Q1 answered: the mark of a record that actually entered the flow. + needs: { ...emptyOnboardingNeeds(), recordTarget: "me" }, completedAt: "2026-09-10T08:00:00.000Z", firstResult: { task: "log-reading", @@ -264,6 +266,81 @@ describe("isOnboardingSettled", () => { }), ).toBe(false); }); + + it("settles when the offered task was deliberately passed", () => { + // The task was offered — `firstResult` names it, with no completion — and + // the person passed. Asking only "did it produce anything" answers no + // forever, which pins the checklist open with no way out but dismissing + // rows one by one. + expect( + isOnboardingSettled({ + ...settled, + steps: settled.steps.map((s) => + s.id === "first-result" ? { ...s, status: "skipped" as const } : s, + ), + firstResult: { task: "log-reading", target: null, completedAt: null }, + }), + ).toBe(true); + }); + + it("settles a record that never entered the flow, however it is published", () => { + // The payload carries the field for EVERY record. Without this arm, an + // account that predates the flow reads as permanently unfinished. + expect( + isOnboardingSettled({ + steps: defaultOnboardingSteps(), + needs: emptyOnboardingNeeds(), + completedAt: null, + firstResult: null, + }), + ).toBe(true); + }); +}); + +describe("hasEnteredOnboardingFlow", () => { + const blankState: OnboardingStateDto = { + steps: defaultOnboardingSteps(), + needs: emptyOnboardingNeeds(), + completedAt: null, + firstResult: null, + }; + + it("is false for a record the flow never touched", () => { + expect(hasEnteredOnboardingFlow(null)).toBe(false); + expect(hasEnteredOnboardingFlow(blankState)).toBe(false); + }); + + it("is true once the one required question is answered", () => { + expect( + hasEnteredOnboardingFlow({ + ...blankState, + needs: { ...emptyOnboardingNeeds(), recordTarget: "someone-else" }, + }), + ).toBe(true); + }); + + it("is true for a flow that was completed", () => { + expect( + hasEnteredOnboardingFlow({ + ...blankState, + completedAt: "2026-09-10T08:00:00.000Z", + }), + ).toBe(true); + }); + + it("is not fooled by the units step resolving from the account's columns", () => { + // `resolveOnboardingSteps` marks `units` done for an account that already + // holds both preferences. That record has still never seen a question. + expect( + hasEnteredOnboardingFlow({ + ...blankState, + steps: resolveOnboardingSteps(defaultOnboardingSteps(), { + glucoseUnit: "mg/dL", + unitPreference: "metric", + }), + }), + ).toBe(false); + }); }); describe("the unit preferences the account already holds", () => { diff --git a/src/lib/onboarding/checklist.ts b/src/lib/onboarding/checklist.ts index 9ec9e4496..b67450e7d 100644 --- a/src/lib/onboarding/checklist.ts +++ b/src/lib/onboarding/checklist.ts @@ -7,6 +7,7 @@ */ import { + hasEnteredOnboardingFlow, isOnboardingSettled, type OnboardingNeeds, type OnboardingStateDto, @@ -208,12 +209,18 @@ export function checklistProgress(items: ChecklistItem[]): ChecklistProgress { /** * Should the dashboard render the checklist at all? * - * Visible while the user is still in the setup phase: - * - `onboardingCompletedAt` is null OR - * - they have fewer than 5 measurements + * Never when the whole list is dismissed, and never when there is no + * non-dismissed row left to show. * - * AND the user has not dismissed the entire checklist - * AND there is at least one undone, non-dismissed item. + * For a record that ran the setup flow the design spec retires the old + * five-reading rule: the list goes when its rows are done or the person hides + * it. Five readings were never evidence that anything was set up. A record + * that never entered the flow keeps the pre-v1.39 rule unchanged — first-run + * wizard unfinished, or fewer than five readings. + * + * A list whose rows are all done still stays while the flow itself is + * unfinished, because the one task the flow offered is not one of the six + * rows: "every row done" and "the setup finished" are different statements. */ export function shouldShowChecklist(args: { onboardingCompletedAt: string | null; @@ -221,25 +228,26 @@ export function shouldShowChecklist(args: { dismissedAll: boolean; items: ChecklistItem[]; /** - * v1.39 (C1) — the needs-based setup state. While it is unsettled — a step - * still pending, the flow never confirmed, or a first-result task offered - * and never produced — the checklist stays whatever the reading count says. - * That is the design spec's "it no longer disappears at five measurements": - * five readings are not evidence that the setup finished. - * - * Optional, and absence means nothing is outstanding: a record with no - * onboarding row has no unfinished flow to keep the list open for. + * v1.39 (C1) — the needs-based setup state for this record, or null before + * the account payload resolves. A record that never entered the flow reads + * as empty answers with nine pending steps and is treated as having no + * unfinished setup, which is what keeps every account that predates the flow + * on the rule it already had. */ onboarding?: OnboardingStateDto | null; }): boolean { if (args.dismissedAll) return false; - const stillInSetup = - args.onboardingCompletedAt == null || - args.measurementCount < 5 || - !isOnboardingSettled(args.onboarding); - if (!stillInSetup) return false; + const onboarding = args.onboarding ?? null; const visible = visibleChecklist(args.items); if (visible.length === 0) return false; + + const stillInSetup = + hasEnteredOnboardingFlow(onboarding) || + args.onboardingCompletedAt == null || + args.measurementCount < 5; + if (!stillInSetup) return false; + + if (!isOnboardingSettled(onboarding)) return true; return visible.some((item) => !item.done); } diff --git a/src/lib/onboarding/needs.ts b/src/lib/onboarding/needs.ts index 97a1282ef..c271088eb 100644 --- a/src/lib/onboarding/needs.ts +++ b/src/lib/onboarding/needs.ts @@ -334,6 +334,26 @@ export function everyOnboardingStepSettled( return steps.every((step) => step.status !== "pending"); } +/** + * Did this record ever enter the setup flow? + * + * Q1 is the one question with no skip arm, so an answer to it is the mark of + * having started, and the flow's own completion stamp covers a row whose + * answers a later release stopped recognising. + * + * Deliberately NOT "any step is not pending": `units` resolves to `done` from + * the account's own unit columns, so a record that never saw a question can + * carry a settled step. The published state is additive — every record has an + * `onboarding` field, and a record that predates the flow reads as empty + * answers with nine pending steps — so this is what tells the two apart. + */ +export function hasEnteredOnboardingFlow( + state: OnboardingStateDto | null | undefined, +): boolean { + if (!state) return false; + return state.completedAt !== null || state.needs.recordTarget !== null; +} + /** * Has the person finished setting up? * @@ -343,15 +363,26 @@ export function everyOnboardingStepSettled( * point of the whole flow — it ends on one completed task — so an offer with * no result is not a finished setup. * - * `null` (a record that never entered the flow) settles: there is no - * unfinished flow to keep anything open for. + * Two things settle it outright. `null`, and a record that never entered the + * flow: there is no unfinished setup to keep anything open for, and since the + * payload publishes the field for every record, without that arm every account + * that predates the flow would read as permanently unfinished. + * + * And a first-result step the person deliberately passed. The task is offered + * (`firstResult` records which one, with no completion) and then skipped; the + * offer is still on the row, so asking only "did the offered task produce + * anything" answers "no" forever. A deliberate pass is an answer, not an + * unfinished task. */ export function isOnboardingSettled( state: OnboardingStateDto | null | undefined, ): boolean { if (!state) return true; + if (!hasEnteredOnboardingFlow(state)) return true; if (!everyOnboardingStepSettled(state.steps)) return false; if (state.completedAt === null) return false; + const offered = state.steps.find((step) => step.id === "first-result"); + if (offered?.status === "skipped") return true; return state.firstResult === null || state.firstResult.completedAt !== null; }