From 8d81ac07089f58bbea484fe805412be5a4cdcf51 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Fri, 11 Sep 2026 09:22:18 +0200 Subject: [PATCH] fix(onboarding): a connected source is acknowledged, not offered `chooseFirstResultTask` read the answers alone, so an account whose WHOOP had been feeding the dashboard for months was sent to go and connect WHOOP. It takes the delivering sources now; the server still picks from the answers, having no status in hand, and the screen re-picks once the envelope resolves. The fall-through stops where there is nowhere to fall to: a person whose one answer is an already-connected source keeps the step and is told what was found, rather than ending the flow on no result at all. --- .../__tests__/first-result-screen.test.tsx | 59 +++++++++++++++++-- .../onboarding/first-result-screen.tsx | 21 ++++++- .../__tests__/first-result-config.test.ts | 37 ++++++++++++ .../onboarding/__tests__/wizard-steps.test.ts | 40 +++++++++++++ src/lib/onboarding/first-result-config.ts | 32 +++++++++- src/lib/onboarding/wizard-steps.ts | 14 ++++- 6 files changed, 194 insertions(+), 9 deletions(-) diff --git a/src/components/onboarding/__tests__/first-result-screen.test.tsx b/src/components/onboarding/__tests__/first-result-screen.test.tsx index af21c21d9..3fead3a5b 100644 --- a/src/components/onboarding/__tests__/first-result-screen.test.tsx +++ b/src/components/onboarding/__tests__/first-result-screen.test.tsx @@ -19,6 +19,7 @@ */ import { describe, expect, it, vi } from "vitest"; import { renderToStaticMarkup } from "react-dom/server"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; import { I18nProvider } from "@/lib/i18n/context"; import type { SyncVerdict } from "@/lib/integrations/sync-verdict"; @@ -101,25 +102,33 @@ function statuses(overrides: StatusOverrides | null) { }; } -function state(): OnboardingStateDto { +function state( + needs: Partial = {}, +): OnboardingStateDto { return { steps: defaultOnboardingSteps(), needs: { ...emptyOnboardingNeeds(), recordTarget: "me", sources: ["whoop"], + ...needs, }, completedAt: null, firstResult: null, }; } -function render(overrides: StatusOverrides | null) { +function render( + overrides: StatusOverrides | null, + needs: Partial = {}, +) { statuses(overrides); return renderToStaticMarkup( - - - , + + + + + , ); } @@ -191,4 +200,44 @@ describe(" — the connect arm", () => { expect(html).not.toContain("data-connect-slot="); expect(html).toContain('data-slot="onboarding-task-checking"'); }); + + /** + * v1.38.19 (wave B / I1) — the heading is chosen from the answers alone on + * the server, because no status is in hand there. Once the envelope has + * resolved the screen re-picks: a source that is already delivering is not + * a task, and "Connect WHOOP" over a WHOOP that has been feeding the + * dashboard for months is the flow talking past the account it can see. + */ + it("stops offering a source that is already delivering", () => { + const html = render( + { + connected: true, + verdict: "fresh", + lastSuccessAt: "2026-09-10T06:35:00.000Z", + }, + { medication: "yes" }, + ); + expect(html).toContain('data-task="add-medication"'); + expect(html).not.toContain("Connect WHOOP"); + }); + + it("acknowledges the connection when there is nothing to fall through to", () => { + // The whole answer is one already-connected source. Dropping the step + // would end the flow on nothing at all, so the screen keeps it and says + // what it found — and that is the result it records. + const html = render({ + connected: true, + verdict: "fresh", + lastSuccessAt: "2026-09-10T06:35:00.000Z", + }); + expect(html).toContain('data-task="connect-source"'); + expect(html).toContain('data-connect-state="fresh"'); + expect(html).toContain("WHOOP is already connected"); + }); + + it("keeps the answer's heading while the envelope is in flight", () => { + const html = render(null, { medication: "yes" }); + expect(html).toContain('data-task="connect-source"'); + expect(html).toContain("Connect WHOOP"); + }); }); diff --git a/src/components/onboarding/first-result-screen.tsx b/src/components/onboarding/first-result-screen.tsx index d203b0dcf..a3bd92ff8 100644 --- a/src/components/onboarding/first-result-screen.tsx +++ b/src/components/onboarding/first-result-screen.tsx @@ -1,6 +1,6 @@ "use client"; -import { useEffect, useRef, useState } from "react"; +import { useEffect, useMemo, useRef, useState } from "react"; import Link from "next/link"; import { useRouter } from "next/navigation"; import { useQuery } from "@tanstack/react-query"; @@ -34,6 +34,7 @@ import { AREA_PAGE_HREF, AREA_READING_TARGETS, connectSourceView, + deliveringSources, SOURCE_INTEGRATION, type ConnectSourceSlot, } from "@/lib/onboarding/first-result-config"; @@ -70,7 +71,23 @@ export function FirstResultScreen({ state }: { state: OnboardingStateDto }) { const { t } = useTranslations(); const router = useRouter(); const answer = useOnboardingAnswer(); - const offer = chooseFirstResultTask(state.needs); + // The server picked the order from the answers alone — it had no status in + // hand. Re-pick here once the envelope resolves: a source that is already + // delivering is not a task, and offering to connect it is the flow talking + // past an account it can see. The same query key the task body reads, so + // this costs no second request. + const statuses = useIntegrationStatuses(true); + const delivering = useMemo( + () => deliveringSources(statuses.data?.integrations), + [statuses.data], + ); + // …but only where there is somewhere to fall to. A person whose one answer + // is a source that is already delivering still gets an end to the flow: the + // screen acknowledges the connection it found rather than dropping the step + // and ending on nothing. + const offer = + chooseFirstResultTask(state.needs, delivering) ?? + chooseFirstResultTask(state.needs); const back = previousScreen(state, "first-result"); // Completed only when the LEDGER says so as well: a restart puts the step diff --git a/src/lib/onboarding/__tests__/first-result-config.test.ts b/src/lib/onboarding/__tests__/first-result-config.test.ts index 4ee104c1a..ca6160843 100644 --- a/src/lib/onboarding/__tests__/first-result-config.test.ts +++ b/src/lib/onboarding/__tests__/first-result-config.test.ts @@ -15,6 +15,7 @@ import { AREA_PAGE_HREF, AREA_READING_TARGETS, connectSourceView, + deliveringSources, SOURCE_INTEGRATION, } from "../first-result-config"; import { BROWSER_CONNECTABLE_SOURCES } from "../wizard-steps"; @@ -219,3 +220,39 @@ describe("connect-source state copy", () => { }, ); }); + +/** + * v1.38.19 (wave B / I1) — which sources the flow must stop offering. + * + * Only a connection that is actually delivering, or one whose data has merely + * gone quiet: both are real connections the person made, and offering to make + * them again is the wizard talking past the account it can see. Everything + * else — never connected, first sync running, needs repairing — stays in the + * priority, because there is still something to do or something to say. + */ +describe("deliveringSources", () => { + function entry(integration: string, verdict: string) { + return { + integration, + connected: verdict !== "disconnected", + configured: true, + syncHealth: { verdict, since: null }, + lastSuccessAt: null, + }; + } + + it("names the sources that are delivering or merely quiet", () => { + const set = deliveringSources([ + entry("whoop", "fresh"), + entry("oura", "stale"), + entry("polar", "pending_first_sync"), + entry("fitbit", "reauth_required"), + entry("withings", "disconnected"), + ] as never); + expect([...set].sort()).toEqual(["oura", "whoop"]); + }); + + it("names nothing while the envelope has not resolved", () => { + expect(deliveringSources(undefined).size).toBe(0); + }); +}); diff --git a/src/lib/onboarding/__tests__/wizard-steps.test.ts b/src/lib/onboarding/__tests__/wizard-steps.test.ts index bbb82e687..740852a6f 100644 --- a/src/lib/onboarding/__tests__/wizard-steps.test.ts +++ b/src/lib/onboarding/__tests__/wizard-steps.test.ts @@ -167,6 +167,46 @@ describe("chooseFirstResultTask", () => { ).toEqual({ task: "log-reading", target: "weight-body" }); expect(chooseFirstResultTask(emptyOnboardingNeeds())).toBeNull(); }); + + /** + * v1.38.19 (wave B / I1) — the choice used to read the answers alone, so an + * account whose WHOOP had been delivering for months was told to go and + * connect WHOOP. A source that is already delivering is not a task; the + * priority falls through to the next one the person's answers name. + */ + it("passes over a source that is already delivering", () => { + const needs: OnboardingNeeds = { + ...emptyOnboardingNeeds(), + sources: ["whoop", "oura"], + medication: "yes", + areas: ["sleep"], + }; + expect(chooseFirstResultTask(needs, new Set(["whoop"]))).toEqual({ + task: "connect-source", + target: "oura", + }); + expect(chooseFirstResultTask(needs, new Set(["whoop", "oura"]))).toEqual({ + task: "add-medication", + target: null, + }); + expect( + chooseFirstResultTask( + { ...needs, medication: "no" }, + new Set(["whoop", "oura"]), + ), + ).toEqual({ task: "log-reading", target: "sleep" }); + }); + + it("is unchanged when nothing is known about the connections", () => { + const needs: OnboardingNeeds = { + ...emptyOnboardingNeeds(), + sources: ["whoop"], + medication: "yes", + }; + expect(chooseFirstResultTask(needs)).toEqual( + chooseFirstResultTask(needs, new Set()), + ); + }); }); describe("firstResultApplies", () => { diff --git a/src/lib/onboarding/first-result-config.ts b/src/lib/onboarding/first-result-config.ts index d320e7c6d..0f2eb588b 100644 --- a/src/lib/onboarding/first-result-config.ts +++ b/src/lib/onboarding/first-result-config.ts @@ -10,7 +10,10 @@ import type { SyncVerdict } from "@/lib/integrations/sync-verdict"; import type { OnboardingAreaKey } from "@/lib/modules/registry"; -import type { BrowserConnectableSource } from "./wizard-steps"; +import { + BROWSER_CONNECTABLE_SOURCES, + type BrowserConnectableSource, +} from "./wizard-steps"; export interface AreaReadingTarget { /** The `defaultType` the measurement form opens on. */ @@ -176,3 +179,30 @@ function canStartConnection( return true; } } + +/** + * The sources the flow must stop offering to connect: the ones whose + * connection is already delivering (`fresh`), and the ones whose data has + * merely gone quiet (`stale`) — both are connections the person made, and + * both would be "connect it" a second time. + * + * `pending_first_sync` is deliberately NOT in the set: that is a connection + * made moments ago, and it is exactly the result this step exists to show. + * Neither is anything on the attention arm — there is still something to say. + */ +export function deliveringSources( + entries: + ReadonlyArray | undefined, +): ReadonlySet { + const delivering = new Set(); + if (!entries) return delivering; + for (const source of BROWSER_CONNECTABLE_SOURCES) { + const entry = entries.find( + (candidate) => + candidate.integration === SOURCE_INTEGRATION[source].statusKey, + ); + const verdict = connectSourceView(source, entry)?.verdict; + if (verdict === "fresh" || verdict === "stale") delivering.add(source); + } + return delivering; +} diff --git a/src/lib/onboarding/wizard-steps.ts b/src/lib/onboarding/wizard-steps.ts index 71c5912a7..b5055dccc 100644 --- a/src/lib/onboarding/wizard-steps.ts +++ b/src/lib/onboarding/wizard-steps.ts @@ -93,6 +93,8 @@ export function questionScreens(state: FlowState): OnboardingStepId[] { : [...FIXED_QUESTIONS]; } +const EMPTY_CONNECTED: ReadonlySet = new Set(); + export interface FirstResultOffer { task: OnboardingFirstResultTask; /** A source key for a connection, an area key for a reading, else null. */ @@ -104,11 +106,21 @@ export interface FirstResultOffer { * the browser can complete → the first medication with a reminder, if the * answer to Q3 was a daily schedule → one reading for the first Q2 area. * Everything the priority passes over goes to the checklist instead. + * + * `connected` names the sources whose connection is already delivering. It is + * optional because the server picks the order from the answers alone, before + * any status is in hand; the screen re-picks once the envelope resolves. A + * source in the set is not a task — an account whose WHOOP had been feeding + * the dashboard for months was told to go and connect WHOOP, which is the + * wizard talking past the account it can already see. */ export function chooseFirstResultTask( needs: OnboardingNeeds, + connected: ReadonlySet = EMPTY_CONNECTED, ): FirstResultOffer | null { - const connectable = needs.sources.find(isBrowserConnectableSource); + const connectable = needs.sources.find( + (source) => isBrowserConnectableSource(source) && !connected.has(source), + ); if (connectable) return { task: "connect-source", target: connectable }; if (needs.medication === "yes") return { task: "add-medication", target: null };