mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-08 20:42:43 +00:00
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.
This commit is contained in:
1 parent
da58d16642
commit
8d81ac0708
6 files changed
+194
-9
No files matched your search
@@ -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["needs"]> = {},
|
||||
): 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<OnboardingStateDto["needs"]> = {},
|
||||
) {
|
||||
statuses(overrides);
|
||||
return renderToStaticMarkup(
|
||||
<I18nProvider initialLocale="en">
|
||||
<FirstResultScreen state={state()} />
|
||||
</I18nProvider>,
|
||||
<QueryClientProvider client={new QueryClient()}>
|
||||
<I18nProvider initialLocale="en">
|
||||
<FirstResultScreen state={state(needs)} />
|
||||
</I18nProvider>
|
||||
</QueryClientProvider>,
|
||||
);
|
||||
}
|
||||
|
||||
@@ -191,4 +200,44 @@ describe("<FirstResultScreen> — 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");
|
||||
});
|
||||
});
|
||||
@@ -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
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
});
|
||||
@@ -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", () => {
|
||||
|
||||
@@ -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<ConnectSourceStatus & { integration: string }> | undefined,
|
||||
): ReadonlySet<BrowserConnectableSource> {
|
||||
const delivering = new Set<BrowserConnectableSource>();
|
||||
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;
|
||||
}
|
||||
@@ -93,6 +93,8 @@ export function questionScreens(state: FlowState): OnboardingStepId[] {
|
||||
: [...FIXED_QUESTIONS];
|
||||
}
|
||||
|
||||
const EMPTY_CONNECTED: ReadonlySet<string> = new Set<string>();
|
||||
|
||||
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<string> = 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 };
|
||||
|
||||
Reference in new issue
Block a user