mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-08 12:32:43 +00:00
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.
This commit is contained in:
1 parent
274923a060
commit
dec5732687
4 files changed
+265
-18
No files matched your search
@@ -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<string, boolean>();
|
||||
|
||||
vi.mock("@/hooks/use-auth", () => ({
|
||||
useAccountOnceMounted: () => currentUser,
|
||||
}));
|
||||
|
||||
vi.mock("@/lib/queries/use-dashboard-snapshot", () => ({
|
||||
useDashboardSnapshot: () => ({
|
||||
data: { tiles: { summaries: { weight: { count: 12 } } } },
|
||||
}),
|
||||
}));
|
||||
|
||||
const RESPONSES: Record<string, unknown> = {
|
||||
'["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<typeof import("@tanstack/react-query")>();
|
||||
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> = {}): 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(
|
||||
<I18nProvider initialLocale="en">
|
||||
<GettingStartedChecklist />
|
||||
</I18nProvider>,
|
||||
);
|
||||
}
|
||||
|
||||
describe("<GettingStartedChecklist> — 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);
|
||||
});
|
||||
});
|
||||
@@ -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<Array<{ id: string }>>({
|
||||
queryKey: queryKeys.medications(),
|
||||
|
||||
@@ -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));
|
||||
});
|
||||
});
|
||||
@@ -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);
|
||||
|
||||
Reference in new issue
Block a user