From dec57326871d5b74a9ddb3d2d5158008dd890aef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Fri, 11 Sep 2026 09:07:41 +0200 Subject: [PATCH] fix(onboarding): checklist queries follow its own setup rule The card's visibility rule gained a "ran the setup flow" disjunct; the component's query gate kept restating the older half of it. A wizard-run account past five readings therefore saw a permanent checklist whose medication, data-source, notification and insight rows were frozen at "not done", because every supporting query was disabled. Both places now read one exported predicate, isStillInSetup, which also keeps the rows live for a restarted run nobody finished. --- .../getting-started-checklist.test.tsx | 139 ++++++++++++++++++ .../onboarding/getting-started-checklist.tsx | 25 +++- .../onboarding/__tests__/checklist.test.ts | 73 +++++++++ src/lib/onboarding/checklist.ts | 46 ++++-- 4 files changed, 265 insertions(+), 18 deletions(-) create mode 100644 src/components/onboarding/__tests__/getting-started-checklist.test.tsx diff --git a/src/components/onboarding/__tests__/getting-started-checklist.test.tsx b/src/components/onboarding/__tests__/getting-started-checklist.test.tsx new file mode 100644 index 000000000..cfe0c20e9 --- /dev/null +++ b/src/components/onboarding/__tests__/getting-started-checklist.test.tsx @@ -0,0 +1,139 @@ +/** + * v1.39 (Wave C, C1) — the checklist's data queries follow the same rule as + * its visibility. + * + * `shouldShowChecklist` gained `hasEnteredOnboardingFlow` as a third + * disjunct; `checklistRelevant` in the component kept restating the older + * half of the rule, so a wizard-run account past five readings still saw the + * card while every supporting query was `enabled: false` — the medication, + * data-source, notification and insights rows read "not done" for things the + * account had already done (research I9). + * + * Rendered SSR-only, per this repo's component-test convention. `useQuery` is + * mocked to honour `enabled` exactly the way tanstack does — a disabled query + * has no data — so what the rows say here is what the dashboard says. + */ +import { renderToStaticMarkup } from "react-dom/server"; +import { describe, expect, it, vi } from "vitest"; + +import { I18nProvider } from "@/lib/i18n/context"; +import type { AuthUser } from "@/hooks/use-auth"; +import { + defaultOnboardingSteps, + emptyOnboardingNeeds, + type OnboardingStateDto, +} from "@/lib/onboarding/needs"; + +let currentUser: AuthUser | null = null; +/** Every `enabled` flag the component asked for, keyed by its query key. */ +const enabledByKey = new Map(); + +vi.mock("@/hooks/use-auth", () => ({ + useAccountOnceMounted: () => currentUser, +})); + +vi.mock("@/lib/queries/use-dashboard-snapshot", () => ({ + useDashboardSnapshot: () => ({ + data: { tiles: { summaries: { weight: { count: 12 } } } }, + }), +})); + +const RESPONSES: Record = { + '["medications"]': [{ id: "m1" }, { id: "m2" }, { id: "m3" }], + '["user","ai-provider"]': { aiAvailable: true }, + '["integrations","status"]': { + integrations: [{ integration: "whoop", connected: true }], + }, + // Deliberately NOT configured: one open row keeps the card on screen, so + // the rows this test is about are actually rendered. + '["notifications","preferences"]': { channels: [{ enabled: false }] }, +}; + +vi.mock("@tanstack/react-query", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + useQuery: (options: { + queryKey: readonly unknown[]; + enabled?: boolean; + }) => { + const key = JSON.stringify(options.queryKey); + const enabled = options.enabled !== false; + enabledByKey.set(key, enabled); + return { data: enabled ? RESPONSES[key] : undefined }; + }, + }; +}); + +const { GettingStartedChecklist } = + await import("../getting-started-checklist"); + +/** A record the flow really finished: Q1 answered, confirmed, task settled. */ +function settledOnboarding(): OnboardingStateDto { + return { + steps: defaultOnboardingSteps().map((step) => ({ + ...step, + status: "done" as const, + })), + needs: { ...emptyOnboardingNeeds(), recordTarget: "me" }, + completedAt: "2026-09-10T08:00:00.000Z", + firstResult: { + task: "log-reading", + target: null, + completedAt: "2026-09-10T08:05:00.000Z", + }, + } as OnboardingStateDto; +} + +function user(overrides: Partial = {}): AuthUser { + return { + id: "u1", + email: "a@b.c", + username: "a", + heightCm: 175, + dateOfBirth: "1990-01-01T00:00:00.000Z", + gender: "MALE", + onboardingCompletedAt: "2026-09-10T08:00:00.000Z", + onboarding: settledOnboarding(), + ...overrides, + } as AuthUser; +} + +function render(): string { + enabledByKey.clear(); + return renderToStaticMarkup( + + + , + ); +} + +describe(" — rows for an established account", () => { + it("reads the real medication, data-source and insights state once the flow is behind the account", () => { + currentUser = user(); + const html = render(); + + // The card is on screen: the notifications row is genuinely open. The + // list itself is collapsed on a server paint, so the progress meter is + // what the rows add up to — profile, measurement, medication, data + // source and insights done, notifications not. + expect(html).toContain('data-testid="onboarding-card"'); + expect(html).toContain("5 of 6 done"); + }); + + it("keeps the supporting queries enabled while the card can render", () => { + currentUser = user(); + render(); + expect(enabledByKey.get('["medications"]')).toBe(true); + expect(enabledByKey.get('["integrations","status"]')).toBe(true); + expect(enabledByKey.get('["notifications","preferences"]')).toBe(true); + expect(enabledByKey.get('["user","ai-provider"]')).toBe(true); + }); + + it("still fetches nothing for a record that never entered the flow and is long past five readings", () => { + currentUser = user({ onboarding: null }); + render(); + expect(enabledByKey.get('["medications"]')).toBe(false); + expect(enabledByKey.get('["integrations","status"]')).toBe(false); + }); +}); diff --git a/src/components/onboarding/getting-started-checklist.tsx b/src/components/onboarding/getting-started-checklist.tsx index 09d678d29..15343b037 100644 --- a/src/components/onboarding/getting-started-checklist.tsx +++ b/src/components/onboarding/getting-started-checklist.tsx @@ -26,6 +26,7 @@ import { queryKeys } from "@/lib/query-keys"; import { buildChecklist, checklistProgress, + isStillInSetup, shouldShowChecklist, upcomingVisitCountFrom, visibleChecklist, @@ -239,17 +240,27 @@ export function GettingStartedChecklist() { }, [snapshotData]); // v1.15.20 — only fetch the checklist's supporting data while the card - // can actually render: the user is still in setup (`shouldShowChecklist`'s - // `onboardingCompletedAt == null || measurementCount < 5` rule) and has - // not dismissed it. Established users used to pay this medications fetch - // (and, pre-v1.5, two more) on every dashboard load for a card that - // never mounts. Gated on the snapshot having resolved so the count is - // real, not the loading-default 0. + // can actually render: the user is still in setup and has not dismissed + // it. Established users used to pay this medications fetch (and, pre-v1.5, + // two more) on every dashboard load for a card that never mounts. Gated on + // the snapshot having resolved so the count is real, not the + // loading-default 0. + // + // v1.39 (Wave C, C1) — the rule is `isStillInSetup`, the same predicate + // `shouldShowChecklist` applies, rather than a restatement of half of it. + // The two drifted apart once already: the visibility rule learned about + // records that ran the setup flow and this gate did not, so the card + // rendered for a wizard-run account past five readings with every + // network-backed row frozen at "not done" (research I9, M-p). const checklistRelevant = !!user && !dismissedAll && snapshotData !== undefined && - (user.onboardingCompletedAt == null || measurementCount < 5); + isStillInSetup({ + onboarding: user.onboarding ?? null, + onboardingCompletedAt: user.onboardingCompletedAt ?? null, + measurementCount, + }); const { data: medsData } = useQuery>({ queryKey: queryKeys.medications(), diff --git a/src/lib/onboarding/__tests__/checklist.test.ts b/src/lib/onboarding/__tests__/checklist.test.ts index 6d19e8fa1..693f336d3 100644 --- a/src/lib/onboarding/__tests__/checklist.test.ts +++ b/src/lib/onboarding/__tests__/checklist.test.ts @@ -9,6 +9,7 @@ import { CHECKLIST_BASE_ITEM_IDS, CHECKLIST_ITEM_IDS, isProfileComplete, + isStillInSetup, shouldShowChecklist, trendHintFor, upcomingVisitCountFrom, @@ -608,3 +609,75 @@ describe("shouldShowChecklist and the setup flow", () => { ).toBe(false); }); }); + +describe("isStillInSetup", () => { + // The one predicate both the visibility rule and the component's query gate + // read, so the card can never render with its supporting queries switched + // off (research I9). + it("is true for a record that ran the flow, whatever its reading count", () => { + expect( + isStillInSetup({ + onboarding: onboardingState(), + onboardingCompletedAt: "2026-01-01T00:00:00Z", + measurementCount: 120, + }), + ).toBe(true); + }); + + it("is true for a restarted run nobody finished (M-p)", () => { + const restarted = onboardingState({ completedAt: null }); + expect( + isStillInSetup({ + onboarding: restarted, + // `restart` deliberately leaves the account stamp alone. + onboardingCompletedAt: "2026-01-01T00:00:00Z", + measurementCount: 120, + }), + ).toBe(true); + }); + + it("keeps the pre-flow rule for a record that never entered the flow", () => { + const untouched: OnboardingStateDto = { + steps: defaultOnboardingSteps(), + needs: emptyOnboardingNeeds(), + completedAt: null, + firstResult: null, + }; + expect( + isStillInSetup({ + onboarding: untouched, + onboardingCompletedAt: "2026-01-01T00:00:00Z", + measurementCount: 10, + }), + ).toBe(false); + expect( + isStillInSetup({ + onboarding: untouched, + onboardingCompletedAt: "2026-01-01T00:00:00Z", + measurementCount: 4, + }), + ).toBe(true); + expect( + isStillInSetup({ + onboarding: null, + onboardingCompletedAt: null, + measurementCount: 10, + }), + ).toBe(true); + }); + + it("is the rule `shouldShowChecklist` itself applies", () => { + const args = { + onboarding: onboardingState(), + onboardingCompletedAt: "2026-01-01T00:00:00Z", + measurementCount: 120, + }; + expect( + shouldShowChecklist({ + ...args, + dismissedAll: false, + items: buildChecklist(inputs({ measurementCount: 120 })), + }), + ).toBe(isStillInSetup(args)); + }); +}); diff --git a/src/lib/onboarding/checklist.ts b/src/lib/onboarding/checklist.ts index c0fda5294..fab022c00 100644 --- a/src/lib/onboarding/checklist.ts +++ b/src/lib/onboarding/checklist.ts @@ -282,11 +282,7 @@ export function checklistProgress(items: ChecklistItem[]): ChecklistProgress { * 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; - measurementCount: number; - dismissedAll: boolean; - items: ChecklistItem[]; +export interface StillInSetupInputs { /** * 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 @@ -295,17 +291,45 @@ export function shouldShowChecklist(args: { * on the rule it already had. */ onboarding?: OnboardingStateDto | null; -}): boolean { + onboardingCompletedAt: string | null; + measurementCount: number; +} + +/** + * v1.39 (Wave C, C1) — "is this record still being set up?", as ONE predicate. + * + * Two places decide this: whether the card renders at all, and whether the + * component fetches the data its rows are about. They were two restatements of + * the same rule and they drifted — the visibility rule gained + * `hasEnteredOnboardingFlow`, the query gate kept the older half, and a + * wizard-run account past five readings got a permanently visible card whose + * every network-backed row read "not done" (research I9). A record whose + * restarted run was abandoned is in the same state (M-p): `restart` clears the + * record's own `completedAt` and deliberately not the account stamp, so + * `hasEnteredOnboardingFlow` is what keeps its rows live. + * + * Exported so the component calls it rather than restating it. + */ +export function isStillInSetup(args: StillInSetupInputs): boolean { + return ( + hasEnteredOnboardingFlow(args.onboarding ?? null) || + args.onboardingCompletedAt == null || + args.measurementCount < 5 + ); +} + +export function shouldShowChecklist( + args: StillInSetupInputs & { + dismissedAll: boolean; + items: ChecklistItem[]; + }, +): boolean { if (args.dismissedAll) 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 (!isStillInSetup(args)) return false; if (!isOnboardingSettled(onboarding)) return true; return visible.some((item) => !item.done);