From 036720bcfb08907a2138c4e6b9b4cbd87134ddb0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 10 Sep 2026 09:09:26 +0200 Subject: [PATCH 01/15] docs(changelog): name the modules route's method and the profile refusal as they are The 1.38.15 entry said PUT where the record-settings route answers PATCH, and promised a rejectedFields field on the managed-profile edit that only the two profile routes carry; the edit refuses with a 422 and the issue list. --- CHANGELOG.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b9f390c4c..7ba3240aa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,10 +13,10 @@ on every surface, and four more journeys in the gate. can derive from it. Both arms require the fresh second factor the create and delete routes already do, refuse with 404 rather than 403 for a record the caller does not manage, and the edit is capped at ten an hour per - account. Every field is documented; `rejectedFields` says which value the - server would not take. + account. Every field is documented; a value the server will not take is a + 422 with the issue list. -- **Modules are switched per record.** `GET`/`PUT /api/record-settings/modules` +- **Modules are switched per record.** `GET`/`PATCH /api/record-settings/modules` read and write the module map of the record the browser is acting on, and `GET /api/auth/me` under an active switch reports `modules` and `cycleTrackingEnabled` for that record, not for the caller. Navigation From cc98c561726485434e5599d40b14ed863ff24eb5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 10 Sep 2026 11:03:24 +0200 Subject: [PATCH 02/15] feat(onboarding): store what a record needs, and map it to modules MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The setup questionnaire the design spec asks for needs somewhere to put its answers. `OnboardingRecord` is that place: one row per RECORD rather than per person, so a guardian running a managed profile's setup writes the child's row and not their own, and the three JSON columns hold the answers, the nine ordered steps with their status, and the one task the flow ends on. Nothing here is encrypted, and the schema comment says why: the columns hold closed enum vocabularies, not free text, which is the same class of value as the module map and the goal slugs sitting in plaintext beside them. The needs to modules map lives next to the module keys rather than in the flow, because a module renamed or retired has to break the map at compile time rather than leave a chip that quietly changes nothing. The derivation is total — every directly-owned key gets an explicit value — and the merge beside it is one-directional: what the answers name is switched on, what they do not name is switched off only where the record does not already carry a deliberate on. The two delegated keys are absent by construction: `cycle` is reported separately because its real column is on the cycle profile, and the Coach is not offered by this flow at all. Readers are fail-soft. They sit on the payload that rides every app boot, and a value written by a release that named an area this one retired must not take the shell down with it. Nothing is retired: the four-step wizard, its goal slugs and the one-time tile seed keep working and keep their meaning until the new surface replaces them. The two states are disjoint — the slugs decide tile order, these answers decide which modules are on, and neither reads the other. --- .../0338_onboarding_needs/migration.sql | 22 ++ prisma/schema.prisma | 66 ++++ .../__tests__/onboarding-needs-map.test.ts | 215 ++++++++++++ src/lib/modules/registry.ts | 186 ++++++++++ src/lib/onboarding/__tests__/needs.test.ts | 262 ++++++++++++++ src/lib/onboarding/needs-apply.ts | 126 +++++++ src/lib/onboarding/needs-store.ts | 88 +++++ src/lib/onboarding/needs.ts | 325 ++++++++++++++++++ .../__tests__/onboarding-needs.test.ts | 105 ++++++ src/lib/validations/onboarding-needs.ts | 119 +++++++ 10 files changed, 1514 insertions(+) create mode 100644 prisma/migrations/0338_onboarding_needs/migration.sql create mode 100644 src/lib/modules/__tests__/onboarding-needs-map.test.ts create mode 100644 src/lib/onboarding/__tests__/needs.test.ts create mode 100644 src/lib/onboarding/needs-apply.ts create mode 100644 src/lib/onboarding/needs-store.ts create mode 100644 src/lib/onboarding/needs.ts create mode 100644 src/lib/validations/__tests__/onboarding-needs.test.ts create mode 100644 src/lib/validations/onboarding-needs.ts diff --git a/prisma/migrations/0338_onboarding_needs/migration.sql b/prisma/migrations/0338_onboarding_needs/migration.sql new file mode 100644 index 000000000..244fd1800 --- /dev/null +++ b/prisma/migrations/0338_onboarding_needs/migration.sql @@ -0,0 +1,22 @@ +-- v1.39 (C1) — the needs-based onboarding's per-record state. +-- +-- One row per record (an account or a managed profile), created lazily by the +-- first answered step. Nothing here is encrypted: the three JSON columns hold +-- closed enum vocabularies rather than free text. +CREATE TABLE "onboarding_records" ( + "id" TEXT NOT NULL, + "user_id" TEXT NOT NULL, + "needs_json" JSONB, + "steps_json" JSONB, + "first_result_json" JSONB, + "modules_derived_at" TIMESTAMP(3), + "completed_at" TIMESTAMP(3), + "created_at" TIMESTAMP(3) NOT NULL DEFAULT CURRENT_TIMESTAMP, + "updated_at" TIMESTAMP(3) NOT NULL, + + CONSTRAINT "onboarding_records_pkey" PRIMARY KEY ("id") +); + +CREATE UNIQUE INDEX "onboarding_records_user_id_key" ON "onboarding_records"("user_id"); + +ALTER TABLE "onboarding_records" ADD CONSTRAINT "onboarding_records_user_id_fkey" FOREIGN KEY ("user_id") REFERENCES "users"("id") ON DELETE CASCADE ON UPDATE CASCADE; diff --git a/prisma/schema.prisma b/prisma/schema.prisma index 0c6e0a00a..0c3524930 100644 --- a/prisma/schema.prisma +++ b/prisma/schema.prisma @@ -995,6 +995,12 @@ model User { // seeded catalogue). cycleSymptomsCustom CycleSymptom[] @relation("UserCycleSymptoms") + // v1.39 — the needs-based onboarding's answers and step ledger for THIS + // record. 1:1, and keyed by record rather than by the person at the + // keyboard so a guardian running a managed profile's setup writes the + // child's row, not their own. See `OnboardingRecord`. + onboardingRecord OnboardingRecord? + // v1.15.20 — user-authored "about me" context for the AI surfaces // (1:1, encrypted free text). See `UserHealthProfile`. healthProfile UserHealthProfile? @@ -8298,3 +8304,63 @@ model VaccinationDocumentLink { @@index([userId]) @@map("vaccination_document_links") } + +// ─── Onboarding (needs-based flow) ───────────────────────────── + +/// v1.39 (C1) — what a record answered when it was set up, and how far the +/// flow got. +/// +/// One row per RECORD, not per person: a managed profile is an ordinary `User` +/// row with no credentials, so keying on `userId` is what lets a guardian run +/// a child's setup and have the answers land on the child's record. Created +/// lazily by the first answer; an account that never entered the flow has no +/// row and reads as "nothing answered, nine steps pending". +/// +/// The three JSON columns hold closed enum vocabularies, not free text, so +/// nothing here is encrypted: the encryption convention in this repository +/// covers free text a person wrote (`UserHealthProfile.aboutMeEncrypted`, the +/// Coach transcript, the emergency note) and identifiers (the KVNR), and an +/// answer drawn from nine fixed chips is neither. It is the same class of +/// value as `modulePreferencesJson` and `onboardingGoals`, both of which sit +/// in plaintext beside it. Readers are fail-soft rather than schema-validated +/// at the database edge — see `src/lib/onboarding/needs.ts`. +model OnboardingRecord { + id String @id @default(cuid()) + userId String @unique @map("user_id") + user User @relation(fields: [userId], references: [id], onDelete: Cascade) + + /// The answers as given: `{ recordTarget, areas[], medication, sources[], + /// visit, units }`. Read by `parseOnboardingNeeds`, published on + /// `GET /api/auth/me` as `onboarding.needs`, and the input the module + /// derivation reads once at confirm time. + needsJson Json? @map("needs_json") + + /// The ordered steps as `[{ id, status }]` with `status` one of + /// `pending | done | skipped`. Stored as a status lookup — the ORDER on the + /// wire is always this release's `ONBOARDING_STEP_IDS`, so a step added or + /// retired needs no backfill. + stepsJson Json? @map("steps_json") + + /// The one task the flow offered at the end, and whether it produced its + /// result: `{ task, target, completedAt }`. Null until the first-result + /// step names one. + firstResultJson Json? @map("first_result_json") + + /// When the module map was derived from `needsJson`. The derivation is + /// once-only per run of the flow: `POST /api/onboarding/complete` skips it + /// while this is set, and `POST /api/onboarding/restart` clears it so a + /// re-run may derive again. Without it a repeated confirm would re-apply + /// the answers over decisions taken in Settings since. + modulesDerivedAt DateTime? @map("modules_derived_at") + + /// When the needs flow was completed. Deliberately NOT + /// `User.onboardingCompletedAt`: that column gates the first-run redirect in + /// `src/proxy.ts` and keeps that meaning, while this one may be cleared by + /// "Set up again" without pushing the person back through the redirect. + completedAt DateTime? @map("completed_at") + + createdAt DateTime @default(now()) @map("created_at") + updatedAt DateTime @updatedAt @map("updated_at") + + @@map("onboarding_records") +} diff --git a/src/lib/modules/__tests__/onboarding-needs-map.test.ts b/src/lib/modules/__tests__/onboarding-needs-map.test.ts new file mode 100644 index 000000000..79dbaa825 --- /dev/null +++ b/src/lib/modules/__tests__/onboarding-needs-map.test.ts @@ -0,0 +1,215 @@ +/** + * v1.39 (C1) — the needs → modules mapping, and the merge that protects a + * decision somebody already made. + * + * The mapping is the one piece of the flow with a consequence that outlives + * the screens: a module switched off here disappears from the navigation until + * somebody finds it again in Settings. So the properties below are stated as + * properties over the whole registry rather than as a table of examples — a + * module key added to `MODULE_KEYS` next year is covered by them on the day it + * lands, and an area that stops naming anything fails here rather than + * shipping as a chip with no effect. + */ +import { describe, expect, it } from "vitest"; + +import { + deriveOnboardingModuleDefaults, + isModuleKey, + mergeDerivedModulePreferences, + MODULE_REGISTRY, + ONBOARDING_ALWAYS_ON_MODULES, + ONBOARDING_AREA_KEYS, + ONBOARDING_AREA_MODULES, + OWNED_MODULE_KEYS, + type OnboardingModuleNeeds, +} from "../registry"; + +function needs( + overrides: Partial = {}, +): OnboardingModuleNeeds { + return { + recordTarget: "me", + areas: [], + medication: "no", + visit: "no", + ...overrides, + }; +} + +describe("the area → module map", () => { + it("gives every area at least one module", () => { + expect(ONBOARDING_AREA_KEYS.length).toBe(9); + for (const area of ONBOARDING_AREA_KEYS) { + expect( + ONBOARDING_AREA_MODULES[area]?.length ?? 0, + `area \`${area}\` names no module, so ticking it changes nothing`, + ).toBeGreaterThan(0); + } + }); + + it("names only real module keys", () => { + for (const area of ONBOARDING_AREA_KEYS) { + for (const key of ONBOARDING_AREA_MODULES[area]) { + expect(isModuleKey(key), `\`${key}\` is not a module key`).toBe(true); + } + } + }); + + it("never names the Coach, which the flow deliberately does not offer", () => { + const named = ONBOARDING_AREA_KEYS.flatMap( + (area) => ONBOARDING_AREA_MODULES[area], + ); + expect(named).not.toContain("coach"); + }); +}); + +describe("deriveOnboardingModuleDefaults", () => { + it("answers for every directly-owned module and for none of the delegated ones", () => { + const derived = deriveOnboardingModuleDefaults(needs()); + expect(Object.keys(derived.preferences).sort()).toEqual( + [...OWNED_MODULE_KEYS].sort(), + ); + expect(derived.preferences).not.toHaveProperty("cycle"); + expect(derived.preferences).not.toHaveProperty("coach"); + for (const key of OWNED_MODULE_KEYS) { + expect(MODULE_REGISTRY[key].delegatesTo).toBeUndefined(); + } + }); + + it("keeps the always-on modules on however the questions are answered", () => { + const answers: OnboardingModuleNeeds[] = [ + needs(), + needs({ areas: [...ONBOARDING_AREA_KEYS], medication: "yes" }), + needs({ recordTarget: "someone-else", visit: "within-a-month" }), + needs({ recordTarget: "both", medication: "sometimes", visit: "later" }), + ]; + for (const answer of answers) { + const { preferences } = deriveOnboardingModuleDefaults(answer); + for (const key of ONBOARDING_ALWAYS_ON_MODULES) { + expect( + preferences[key as keyof typeof preferences], + `always-on module \`${key}\` was derived off`, + ).toBe(true); + } + } + }); + + it("switches on exactly what the chosen areas name", () => { + const { preferences, cycleTracking } = deriveOnboardingModuleDefaults( + needs({ areas: ["glucose", "sleep", "mood"] }), + ); + expect(preferences.glucose).toBe(true); + expect(preferences.sleep).toBe(true); + expect(preferences.recovery).toBe(true); + expect(preferences.mood).toBe(true); + expect(preferences.mentalHealth).toBe(true); + expect(preferences.labs).toBe(false); + expect(preferences.workouts).toBe(false); + expect(preferences.illness).toBe(false); + expect(cycleTracking).toBe(false); + }); + + it("reports the cycle area separately, because the module blob does not own it", () => { + expect(deriveOnboardingModuleDefaults(needs({ areas: ["cycle"] }))).toEqual( + expect.objectContaining({ cycleTracking: true }), + ); + expect(deriveOnboardingModuleDefaults(needs()).cycleTracking).toBe(false); + }); + + it("follows the medication answer", () => { + for (const answer of ["yes", "sometimes"] as const) { + expect( + deriveOnboardingModuleDefaults(needs({ medication: answer })) + .preferences.medications, + ).toBe(true); + } + expect( + deriveOnboardingModuleDefaults(needs({ medication: "no" })).preferences + .medications, + ).toBe(false); + }); + + it("switches the doctor report on for any visit that is actually coming", () => { + for (const visit of ["within-a-month", "later"] as const) { + expect( + deriveOnboardingModuleDefaults(needs({ visit })).preferences + .doctorReport, + ).toBe(true); + } + expect( + deriveOnboardingModuleDefaults(needs({ visit: "no" })).preferences + .doctorReport, + ).toBe(false); + }); + + it("switches the immunization log on for a record somebody else runs", () => { + for (const recordTarget of ["someone-else", "both"] as const) { + expect( + deriveOnboardingModuleDefaults(needs({ recordTarget })).preferences + .vaccinations, + ).toBe(true); + } + expect( + deriveOnboardingModuleDefaults(needs({ recordTarget: "me" })).preferences + .vaccinations, + ).toBe(false); + }); + + it("leaves the opt-in surfaces off unless something asks for them", () => { + const { preferences } = deriveOnboardingModuleDefaults( + needs({ areas: [...ONBOARDING_AREA_KEYS] }), + ); + expect(preferences.mcp).toBe(false); + expect(preferences.nutrients).toBe(false); + expect(preferences.environment).toBe(false); + }); +}); + +describe("mergeDerivedModulePreferences", () => { + it("never turns off a module the person switched on by hand", () => { + const { preferences } = deriveOnboardingModuleDefaults(needs()); + expect(preferences.mcp).toBe(false); + expect(preferences.labs).toBe(false); + + const merged = mergeDerivedModulePreferences( + { mcp: true, labs: true }, + preferences, + ); + expect(merged.mcp).toBe(true); + expect(merged.labs).toBe(true); + }); + + it("does turn off a module the record only had by default", () => { + const { preferences } = deriveOnboardingModuleDefaults(needs()); + const merged = mergeDerivedModulePreferences({}, preferences); + expect(merged.labs).toBe(false); + expect(merged.workouts).toBe(false); + }); + + it("switches on what the answers named even where the record had it off", () => { + const { preferences } = deriveOnboardingModuleDefaults( + needs({ areas: ["labs"] }), + ); + const merged = mergeDerivedModulePreferences({ labs: false }, preferences); + expect(merged.labs).toBe(true); + }); + + it("drops a stored key that is not a module", () => { + const { preferences } = deriveOnboardingModuleDefaults(needs()); + const merged = mergeDerivedModulePreferences( + { weight: false, nonsense: true }, + preferences, + ); + expect(merged).not.toHaveProperty("weight"); + expect(merged).not.toHaveProperty("nonsense"); + }); + + it("leaves the two delegated keys alone in both directions", () => { + const { preferences } = deriveOnboardingModuleDefaults( + needs({ areas: ["cycle"] }), + ); + const merged = mergeDerivedModulePreferences({}, preferences); + expect(merged).not.toHaveProperty("cycle"); + expect(merged).not.toHaveProperty("coach"); + }); +}); diff --git a/src/lib/modules/registry.ts b/src/lib/modules/registry.ts index 7025e129f..b2346dc6e 100644 --- a/src/lib/modules/registry.ts +++ b/src/lib/modules/registry.ts @@ -423,3 +423,189 @@ export function moduleDelegatesTo( export function isOptInModule(key: ModuleKey): boolean { return MODULE_REGISTRY[key].optIn === true; } + +/* ─── Needs-based onboarding: the areas a person names → the modules ───────── */ + +/** + * v1.39 (C1) — the nine areas the needs questionnaire offers (design spec + * §"The questions", Q2), as stable slugs. + * + * They live here rather than in the flow because the answer only means + * something as a set of modules, and the mapping below is what gives it that + * meaning. Putting the map next to `MODULE_KEYS` is what keeps a module key + * renamed or retired from leaving a dangling area behind: the record type + * makes it a compile error rather than a silently empty selection. + * + * All nine stay visible, cycle included — it is one chip, and hiding it behind + * recorded sex would make the flow guess at something the person is being + * asked about anyway. + */ +export const ONBOARDING_AREA_KEYS = [ + "blood-pressure", + "weight-body", + "glucose", + "sleep", + "mood", + "cycle", + "activity", + "labs", + "illness", +] as const; + +export type OnboardingAreaKey = (typeof ONBOARDING_AREA_KEYS)[number]; + +/** + * The area → module map. Every area names at least one module; blood pressure + * and weight name only `insights`, because the readings themselves are CORE + * (weight / blood pressure / pulse have no key and can never be switched off) + * and the narrative layer is the only thing an answer can decide there. + * + * `cycle` is in the map and is NOT written through `modulePreferencesJson`: + * that key delegates to `CycleProfile.cycleTrackingEnabled`, so the derivation + * reports it separately (see {@link DerivedModuleDefaults.cycleTracking}) and + * the caller writes the real column. `coach` appears nowhere: the flow never + * talks to an AI provider, and turning the Coach on is a Settings decision + * with a cost attached to it. + */ +export const ONBOARDING_AREA_MODULES: Readonly< + Record +> = Object.freeze({ + "blood-pressure": ["insights"], + "weight-body": ["insights"], + glucose: ["glucose", "insights"], + sleep: ["sleep", "recovery"], + // "Mood and mental wellbeing" is one chip over two modules: the daily mood + // entry and the WHO-5 / PHQ-9 / GAD-7 check-in live on the same surface as + // far as the person answering is concerned. + mood: ["mood", "mentalHealth"], + cycle: ["cycle"], + activity: ["workouts"], + labs: ["labs"], + illness: ["illness"], +}); + +/** + * Modules that stay on whatever the answers say (design spec §"Needs → modules + * → first result"): the measurement engine is CORE and has no key, and these + * three are the surfaces a record is unusable without — the narrative layer, + * the badges, and the document vault. + */ +export const ONBOARDING_ALWAYS_ON_MODULES: readonly ModuleKey[] = [ + "insights", + "achievements", + "inboundDocuments", +]; + +/** + * The answers the module derivation actually reads. The flow stores more than + * this (the sources, the units); nothing else changes which modules are on. + * + * The literal unions are written here rather than imported so this file keeps + * depending on nothing. `src/lib/onboarding/needs.ts` declares the same + * vocabulary with `satisfies`, and the derivation call site is typed against + * this interface — a value that drifts apart fails to compile there. + */ +export interface OnboardingModuleNeeds { + recordTarget: "me" | "someone-else" | "both"; + areas: readonly OnboardingAreaKey[]; + medication: "yes" | "no" | "sometimes"; + visit: "within-a-month" | "later" | "no"; +} + +/** A toggleable module whose state `modulePreferencesJson` really owns. */ +export type OwnedModuleKey = Exclude; + +/** Toggleable modules minus the two delegated keys, in registry order. */ +export const OWNED_MODULE_KEYS: readonly OwnedModuleKey[] = MODULE_KEYS.filter( + (key): key is OwnedModuleKey => + MODULE_REGISTRY[key].delegatesTo === undefined, +); + +export interface DerivedModuleDefaults { + /** + * An explicit value for every directly-owned module: `true` for what the + * answers named, `false` for the rest. Explicit `false` rather than absence + * is the "ordering versus removal" decision from the design spec — the + * stored map is a disabled allowlist, so absence would mean ON. + */ + preferences: Record; + /** + * Whether the answers asked for cycle tracking. Only ever `true` here: the + * gate derives an unanswered cycle from recorded sex, and an area the person + * did not tick is not a statement that they want it switched off. The caller + * writes `CycleProfile.cycleTrackingEnabled` when this is true and leaves + * the column alone when it is not. + */ + cycleTracking: boolean; +} + +/** + * Turn the needs answers into the module map the record starts from. + * + * Pure and total: every directly-owned key gets a value, so the caller never + * has to reason about which keys the answers happened to mention. It says + * nothing about what the record already holds — merging is + * {@link mergeDerivedModulePreferences}, deliberately a second step, because + * "what the answers imply" and "what this person already decided by hand" are + * different questions and only one of them may be overwritten. + */ +export function deriveOnboardingModuleDefaults( + needs: OnboardingModuleNeeds, +): DerivedModuleDefaults { + const on = new Set(ONBOARDING_ALWAYS_ON_MODULES); + + for (const area of needs.areas) { + for (const key of ONBOARDING_AREA_MODULES[area] ?? []) on.add(key); + } + if (needs.medication === "yes" || needs.medication === "sometimes") { + on.add("medications"); + } + // A visit that is coming at all is what the doctor report is for; "no" is + // the only answer that leaves the export surface off. + if (needs.visit === "within-a-month" || needs.visit === "later") { + on.add("doctorReport"); + } + // A record somebody else runs is usually a child's or a parent's, and the + // immunization log is the part of it that matters most there. + if (needs.recordTarget === "someone-else" || needs.recordTarget === "both") { + on.add("vaccinations"); + } + + const preferences = {} as Record; + for (const key of OWNED_MODULE_KEYS) preferences[key] = on.has(key); + + return { preferences, cycleTracking: on.has("cycle") }; +} + +/** + * Merge a derived map into the one the record already holds, without ever + * retracting a decision the person made by hand. + * + * The rule is one-directional: a derived `true` always lands, a derived + * `false` lands only where the stored map does not already carry an explicit + * `true`. Someone who switched the MCP endpoint on, then ran the flow again + * and did not tick anything that implies it, keeps it on — the questionnaire + * orders a record, it does not undo it. + * + * Keys the stored map carries that are not module keys are dropped; the caller + * is expected to have normalised it (`normalisePrefs`), and this is the second + * belt on the same trousers rather than a place to be clever. + */ +export function mergeDerivedModulePreferences( + existing: Readonly>, + derived: Readonly>, +): Record { + const merged: Record = {}; + for (const [key, value] of Object.entries(existing)) { + if (isModuleKey(key)) merged[key] = value; + } + for (const [key, value] of Object.entries(derived)) { + if (value) { + merged[key] = true; + continue; + } + if (merged[key] === true) continue; // switched on by hand — never retracted + merged[key] = false; + } + return merged; +} diff --git a/src/lib/onboarding/__tests__/needs.test.ts b/src/lib/onboarding/__tests__/needs.test.ts new file mode 100644 index 000000000..a070ebda8 --- /dev/null +++ b/src/lib/onboarding/__tests__/needs.test.ts @@ -0,0 +1,262 @@ +/** + * v1.39 (C1) — the stored onboarding state: reading it back, and applying one + * answered step to it. + * + * The two claims worth pinning here are the ones the route promises on the + * wire. `PATCH /api/onboarding/answers` says it is idempotent, and the reason + * it can say so is that the whole computation is this pure function — so the + * property is checkable without a database. The readers say they are + * fail-soft, and the reason that matters is that they sit on the account + * payload that rides every app boot: a column written by a release that named + * an area this one retired must not 500 the shell. + */ +import { describe, expect, it } from "vitest"; + +import { applyOnboardingAnswer } from "../needs-apply"; +import { + defaultOnboardingSteps, + emptyOnboardingNeeds, + isOnboardingSettled, + ONBOARDING_STEP_IDS, + parseOnboardingFirstResult, + parseOnboardingNeeds, + parseOnboardingSteps, + type OnboardingStateDto, +} from "../needs"; + +function blank() { + return { + needs: emptyOnboardingNeeds(), + steps: defaultOnboardingSteps(), + firstResult: null, + }; +} + +const AT = new Date("2026-09-10T08:00:00.000Z"); + +describe("parseOnboardingSteps", () => { + it("returns the full ordered nine whatever the column holds", () => { + expect(parseOnboardingSteps(null).map((s) => s.id)).toEqual([ + ...ONBOARDING_STEP_IDS, + ]); + expect( + parseOnboardingSteps(null).every((s) => s.status === "pending"), + ).toBe(true); + }); + + it("keeps a stored status and ignores an id or status it does not know", () => { + const parsed = parseOnboardingSteps([ + { id: "areas", status: "done" }, + { id: "visit", status: "skipped" }, + { id: "no-such-step", status: "done" }, + { id: "who", status: "half-done" }, + ]); + expect(parsed.find((s) => s.id === "areas")?.status).toBe("done"); + expect(parsed.find((s) => s.id === "visit")?.status).toBe("skipped"); + expect(parsed.find((s) => s.id === "who")?.status).toBe("pending"); + expect(parsed.map((s) => s.id)).toEqual([...ONBOARDING_STEP_IDS]); + }); +}); + +describe("parseOnboardingNeeds", () => { + it("reads nothing out of a corrupt column rather than throwing", () => { + expect(parseOnboardingNeeds("not an object")).toEqual( + emptyOnboardingNeeds(), + ); + expect(parseOnboardingNeeds({ areas: "sleep" }).areas).toEqual([]); + }); + + it("drops unknown answers and de-duplicates the lists", () => { + const parsed = parseOnboardingNeeds({ + recordTarget: "nobody", + areas: ["sleep", "sleep", "astrology"], + medication: "yes", + sources: ["withings", "carrier-pigeon"], + visit: "later", + units: { glucoseUnit: "mmol/L", unitPreference: "furlongs" }, + }); + expect(parsed.recordTarget).toBeNull(); + expect(parsed.areas).toEqual(["sleep"]); + expect(parsed.medication).toBe("yes"); + expect(parsed.sources).toEqual(["withings"]); + expect(parsed.visit).toBe("later"); + expect(parsed.units).toEqual({ + glucoseUnit: "mmol/L", + unitPreference: null, + }); + }); +}); + +describe("parseOnboardingFirstResult", () => { + it("is null without a known task", () => { + expect(parseOnboardingFirstResult(null)).toBeNull(); + expect(parseOnboardingFirstResult({ task: "invent-a-cure" })).toBeNull(); + }); + + it("reads the task, its target and its stamp", () => { + expect( + parseOnboardingFirstResult({ + task: "connect-source", + target: "withings", + completedAt: "2026-09-10T08:00:00.000Z", + }), + ).toEqual({ + task: "connect-source", + target: "withings", + completedAt: "2026-09-10T08:00:00.000Z", + }); + }); +}); + +describe("applyOnboardingAnswer", () => { + it("marks the step done and records the answer", () => { + const next = applyOnboardingAnswer( + blank(), + { step: "areas", areas: ["sleep", "labs"] }, + AT, + ); + expect(next.needs.areas).toEqual(["sleep", "labs"]); + expect(next.steps.find((s) => s.id === "areas")?.status).toBe("done"); + expect(next.steps.find((s) => s.id === "who")?.status).toBe("pending"); + }); + + it("is idempotent — the same body twice is the same state", () => { + const once = applyOnboardingAnswer( + blank(), + { step: "medication", medication: "yes" }, + AT, + ); + const twice = applyOnboardingAnswer( + once, + { step: "medication", medication: "yes" }, + new Date("2026-09-11T09:00:00.000Z"), + ); + expect(twice).toEqual(once); + }); + + it("keeps the first-result stamp across a replay rather than re-taking it", () => { + const first = applyOnboardingAnswer( + blank(), + { + step: "first-result", + firstResult: { task: "log-reading", target: "sleep", completed: true }, + }, + AT, + ); + expect(first.firstResult?.completedAt).toBe(AT.toISOString()); + + const replay = applyOnboardingAnswer( + first, + { + step: "first-result", + firstResult: { task: "log-reading", target: "sleep", completed: true }, + }, + new Date("2026-09-12T10:00:00.000Z"), + ); + expect(replay.firstResult?.completedAt).toBe(AT.toISOString()); + }); + + it("re-offers a DIFFERENT task without inheriting the old stamp", () => { + const first = applyOnboardingAnswer( + blank(), + { + step: "first-result", + firstResult: { task: "log-reading", completed: true }, + }, + AT, + ); + const second = applyOnboardingAnswer( + first, + { + step: "first-result", + firstResult: { task: "add-medication" }, + }, + AT, + ); + expect(second.firstResult).toEqual({ + task: "add-medication", + target: null, + completedAt: null, + }); + }); + + it("records a skip without erasing an answer already given", () => { + const answered = applyOnboardingAnswer( + blank(), + { step: "visit", visit: "within-a-month" }, + AT, + ); + const skipped = applyOnboardingAnswer( + answered, + { step: "visit", status: "skipped" }, + AT, + ); + expect(skipped.steps.find((s) => s.id === "visit")?.status).toBe("skipped"); + expect(skipped.needs.visit).toBe("within-a-month"); + }); + + it("sets one unit without clearing the other", () => { + const both = applyOnboardingAnswer( + blank(), + { + step: "units", + units: { glucoseUnit: "mmol/L", unitPreference: "metric" }, + }, + AT, + ); + const one = applyOnboardingAnswer( + both, + { step: "units", units: { unitPreference: "imperial" } }, + AT, + ); + expect(one.needs.units).toEqual({ + glucoseUnit: "mmol/L", + unitPreference: "imperial", + }); + }); +}); + +describe("isOnboardingSettled", () => { + const settled: OnboardingStateDto = { + steps: ONBOARDING_STEP_IDS.map((id) => ({ id, status: "done" as const })), + needs: emptyOnboardingNeeds(), + completedAt: "2026-09-10T08:00:00.000Z", + firstResult: { + task: "log-reading", + target: null, + completedAt: "2026-09-10T08:01:00.000Z", + }, + }; + + it("settles a record that never entered the flow", () => { + expect(isOnboardingSettled(null)).toBe(true); + }); + + it("settles a finished flow", () => { + expect(isOnboardingSettled(settled)).toBe(true); + }); + + it("does not settle while a step is pending", () => { + expect( + isOnboardingSettled({ + ...settled, + steps: settled.steps.map((s) => + s.id === "sources" ? { ...s, status: "pending" as const } : s, + ), + }), + ).toBe(false); + }); + + it("does not settle without the flow's own completion stamp", () => { + expect(isOnboardingSettled({ ...settled, completedAt: null })).toBe(false); + }); + + it("does not settle while the offered task has produced nothing", () => { + expect( + isOnboardingSettled({ + ...settled, + firstResult: { task: "log-reading", target: null, completedAt: null }, + }), + ).toBe(false); + }); +}); diff --git a/src/lib/onboarding/needs-apply.ts b/src/lib/onboarding/needs-apply.ts new file mode 100644 index 000000000..2c62ef99c --- /dev/null +++ b/src/lib/onboarding/needs-apply.ts @@ -0,0 +1,126 @@ +/** + * v1.39 (C1) — applying one answered step to the stored onboarding state. + * + * Pure, so the idempotence claim is checkable without a database: applying the + * same body twice returns a value equal to applying it once, which is what + * `PATCH /api/onboarding/answers` promises. The route reads the row, calls + * this, and writes the result back; nothing about "what an answer means" lives + * in the route. + * + * Kept beside the vocabulary rather than inside it so `needs.ts` keeps + * depending on nothing but the module registry — this file is the one place + * that knows both the stored shape and the request shape. + */ +import type { OnboardingAnswerInput } from "@/lib/validations/onboarding-needs"; + +import type { + OnboardingFirstResult, + OnboardingNeeds, + OnboardingStepId, + OnboardingStepState, + OnboardingStepStatus, +} from "./needs"; + +export interface OnboardingRecordState { + needs: OnboardingNeeds; + steps: OnboardingStepState[]; + firstResult: OnboardingFirstResult | null; +} + +function withStatus( + steps: readonly OnboardingStepState[], + id: OnboardingStepId, + status: OnboardingStepStatus, +): OnboardingStepState[] { + return steps.map((step) => (step.id === id ? { ...step, status } : step)); +} + +/** + * Apply one answer. + * + * A skip records the pass and leaves the answers exactly as they were: the + * spec's principle is that nothing is lost by a choice, and a person who + * answered Q2, went back and then skipped it should not have their areas + * erased by the skip. `now` is injected so the first-result stamp is the + * server's instant and the test can pin it. + */ +export function applyOnboardingAnswer( + state: OnboardingRecordState, + input: OnboardingAnswerInput, + now: Date, +): OnboardingRecordState { + if ("status" in input && input.status === "skipped") { + return { ...state, steps: withStatus(state.steps, input.step, "skipped") }; + } + + const steps = withStatus(state.steps, input.step, "done"); + + switch (input.step) { + case "who": + return { + ...state, + steps, + needs: { ...state.needs, recordTarget: input.recordTarget }, + }; + case "areas": + return { ...state, steps, needs: { ...state.needs, areas: input.areas } }; + case "medication": + return { + ...state, + steps, + needs: { ...state.needs, medication: input.medication }, + }; + case "sources": + return { + ...state, + steps, + needs: { ...state.needs, sources: input.sources }, + }; + case "visit": + return { ...state, steps, needs: { ...state.needs, visit: input.visit } }; + case "units": + return { + ...state, + steps, + needs: { + ...state.needs, + units: { + // An omitted field leaves the recorded answer alone; an explicit + // `null` clears it. The two cases are different on purpose — the + // screen may set one unit without the other. + glucoseUnit: + input.units.glucoseUnit === undefined + ? state.needs.units.glucoseUnit + : input.units.glucoseUnit, + unitPreference: + input.units.unitPreference === undefined + ? state.needs.units.unitPreference + : input.units.unitPreference, + }, + }, + }; + case "first-result": { + const already = + state.firstResult?.task === input.firstResult.task + ? state.firstResult.completedAt + : null; + return { + ...state, + steps, + firstResult: { + task: input.firstResult.task, + target: input.firstResult.target ?? null, + // Stamped once. A repeat of the same completed task keeps the + // instant it already had, which is what makes the write idempotent + // rather than merely repeatable. + completedAt: input.firstResult.completed + ? (already ?? now.toISOString()) + : already, + }, + }; + } + case "confirm": + case "done": + return { ...state, steps }; + } +} diff --git a/src/lib/onboarding/needs-store.ts b/src/lib/onboarding/needs-store.ts new file mode 100644 index 000000000..4532fab07 --- /dev/null +++ b/src/lib/onboarding/needs-store.ts @@ -0,0 +1,88 @@ +/** + * v1.39 (C1) — reading the needs-onboarding row, for the payload and for the + * three writes. + * + * One place that turns an `OnboardingRecord` row (or its absence) into the + * published DTO, so `GET /api/auth/me` and the write routes cannot disagree + * about what "nothing answered yet" looks like. A record with no row reads as + * empty answers and nine pending steps rather than as a missing object — a + * client then never branches on absence, and the account payload stays + * additive for every account that predates the flow. + */ +import type { PrismaClient } from "@/generated/prisma/client"; + +import { + defaultOnboardingSteps, + emptyOnboardingNeeds, + parseOnboardingFirstResult, + parseOnboardingNeeds, + parseOnboardingSteps, + type OnboardingStateDto, +} from "./needs"; +import type { OnboardingRecordState } from "./needs-apply"; + +/** The columns every reader below needs. */ +export const ONBOARDING_RECORD_SELECT = { + needsJson: true, + stepsJson: true, + firstResultJson: true, + modulesDerivedAt: true, + completedAt: true, +} as const; + +export interface OnboardingRecordRow { + needsJson: unknown; + stepsJson: unknown; + firstResultJson: unknown; + modulesDerivedAt: Date | null; + completedAt: Date | null; +} + +/** The answers / steps / first result, parsed fail-soft. */ +export function readOnboardingRecordState( + row: OnboardingRecordRow | null, +): OnboardingRecordState { + if (!row) { + return { + needs: emptyOnboardingNeeds(), + steps: defaultOnboardingSteps(), + firstResult: null, + }; + } + return { + needs: parseOnboardingNeeds(row.needsJson), + steps: parseOnboardingSteps(row.stepsJson), + firstResult: parseOnboardingFirstResult(row.firstResultJson), + }; +} + +/** The shape published on the account payload. */ +export function toOnboardingStateDto( + row: OnboardingRecordRow | null, +): OnboardingStateDto { + const state = readOnboardingRecordState(row); + return { + steps: state.steps, + needs: state.needs, + completedAt: row?.completedAt?.toISOString() ?? null, + firstResult: state.firstResult, + }; +} + +/** + * Load the published state for one record. + * + * Takes the delegate rather than the whole client, matching the section + * builders next door, so the route's global client and a worker's local one + * share the same read. + */ +export async function loadOnboardingState( + prisma: Pick, + recordId: string, +): Promise { + const row = await prisma.onboardingRecord.findUnique({ + where: { userId: recordId }, + select: ONBOARDING_RECORD_SELECT, + }); + return toOnboardingStateDto(row); +} diff --git a/src/lib/onboarding/needs.ts b/src/lib/onboarding/needs.ts new file mode 100644 index 000000000..20dd0c018 --- /dev/null +++ b/src/lib/onboarding/needs.ts @@ -0,0 +1,325 @@ +/** + * v1.39 (C1) — the needs-based onboarding's server state, as data. + * + * The design spec (`.planning/2026-09-09-onboarding-design-spec.md`) asks five + * questions, saves each answer as it is given, and derives the record's module + * map from the set of answers once. This file is the vocabulary and the + * fail-soft readers for the three JSON columns behind that: the answers, the + * ordered steps, and the one task the flow ended on. + * + * Pure and Prisma-free on purpose — the Zod schemas in + * `src/lib/validations/onboarding-needs.ts` build on it, the OpenAPI generator + * pulls it in without dragging the server graph along, and the browser reads + * the same step ids the server writes. + * + * ── Coexistence with the wizard of today ─────────────────────────────────── + * + * Nothing here retires anything. `User.onboardingStep`, `User.onboardingGoals` + * and the six goal slugs in `./goals.ts` keep their meaning and keep driving + * the four-step wizard and its one-time dashboard seed until C2 replaces that + * surface. The two states are disjoint by construction: the goal slugs decide + * TILE ORDER, these answers decide WHICH MODULES ARE ON, and neither reads the + * other. `User.onboardingCompletedAt` also stays what it was — the first-run + * redirect's gate — which is why the needs flow carries a completion stamp of + * its own rather than borrowing that column: "set up again" has to be able to + * re-ask the questions without pushing the person back through the first-run + * redirect. + */ +import type { + OnboardingAreaKey, + OnboardingModuleNeeds, +} from "@/lib/modules/registry"; +import { ONBOARDING_AREA_KEYS } from "@/lib/modules/registry"; + +/** + * The ordered steps, with the stable ids the spec names. Order is the order + * they are asked in; the ids are the wire contract a client (web now, native + * later) matches on, and a client that meets an id it does not know is + * expected to skip it rather than refuse the list. + */ +export const ONBOARDING_STEP_IDS = [ + "who", + "areas", + "medication", + "sources", + "visit", + "units", + "confirm", + "first-result", + "done", +] as const; + +export type OnboardingStepId = (typeof ONBOARDING_STEP_IDS)[number]; + +export const ONBOARDING_STEP_STATUSES = ["pending", "done", "skipped"] as const; + +export type OnboardingStepStatus = (typeof ONBOARDING_STEP_STATUSES)[number]; + +/** + * The steps a person may pass without answering. `who` is the one required + * question in the spec; `confirm` and `done` are not questions at all, so + * "skipped" would not mean anything for them. + */ +export const ONBOARDING_SKIPPABLE_STEP_IDS = [ + "areas", + "medication", + "sources", + "visit", + "units", + "first-result", +] as const; + +export type OnboardingSkippableStepId = + (typeof ONBOARDING_SKIPPABLE_STEP_IDS)[number]; + +/** + * Q1 — whose record this is. The vocabularies below are pinned against the + * registry's derivation input with `satisfies`, so a value added here that the + * mapping cannot read fails to compile rather than falling through the + * derivation silently. + */ +export const ONBOARDING_RECORD_TARGETS = [ + "me", + "someone-else", + "both", +] as const satisfies readonly OnboardingModuleNeeds["recordTarget"][]; + +export type OnboardingRecordTarget = (typeof ONBOARDING_RECORD_TARGETS)[number]; + +/** Q3 — medication on a schedule. */ +export const ONBOARDING_MEDICATION_ANSWERS = [ + "yes", + "no", + "sometimes", +] as const satisfies readonly OnboardingModuleNeeds["medication"][]; + +export type OnboardingMedicationAnswer = + (typeof ONBOARDING_MEDICATION_ANSWERS)[number]; + +/** Q4 — where the readings come from today. */ +export const ONBOARDING_SOURCE_KEYS = [ + "manual", + "apple-health", + "withings", + "oura", + "whoop", + "polar", + "fitbit", + "strava", + "nightscout", + "file", +] as const; + +export type OnboardingSourceKey = (typeof ONBOARDING_SOURCE_KEYS)[number]; + +/** Q5 — a doctor's visit coming up. */ +export const ONBOARDING_VISIT_ANSWERS = [ + "within-a-month", + "later", + "no", +] as const satisfies readonly OnboardingModuleNeeds["visit"][]; + +export type OnboardingVisitAnswer = (typeof ONBOARDING_VISIT_ANSWERS)[number]; + +/** + * The one task the flow ends on. `target` names what the task is about — a + * source key for a connection, an area key for a reading — and is null for the + * medication task, which has nothing to disambiguate. + */ +export const ONBOARDING_FIRST_RESULT_TASKS = [ + "connect-source", + "add-medication", + "log-reading", +] as const; + +export type OnboardingFirstResultTask = + (typeof ONBOARDING_FIRST_RESULT_TASKS)[number]; + +/** + * Q6 — the unit answers, recorded as given. + * + * These are NOT the source of truth for how a value is displayed: + * `User.glucoseUnit` and `User.unitPreference` are, and the answers step + * writes them. What is kept here is what the flow was TOLD, so a restart can + * show the person the answer they already gave without re-deriving it from a + * display column somebody may have changed in Settings since. + */ +export interface OnboardingUnitAnswers { + glucoseUnit: "mg/dL" | "mmol/L" | null; + unitPreference: "metric" | "imperial" | null; +} + +/** The answers as given. Every field is null / empty until it is answered. */ +export interface OnboardingNeeds { + recordTarget: OnboardingRecordTarget | null; + areas: OnboardingAreaKey[]; + medication: OnboardingMedicationAnswer | null; + sources: OnboardingSourceKey[]; + visit: OnboardingVisitAnswer | null; + units: OnboardingUnitAnswers; +} + +export interface OnboardingStepState { + id: OnboardingStepId; + status: OnboardingStepStatus; +} + +export interface OnboardingFirstResult { + task: OnboardingFirstResultTask; + target: string | null; + /** ISO instant when the offered task produced its result, or null. */ + completedAt: string | null; +} + +/** + * What `GET /api/auth/me` publishes under `onboarding`, and what a client + * reads. Additive: an account that never entered the flow gets the empty + * answers and nine `pending` steps rather than a missing field, so a consumer + * never has to branch on absence. + */ +export interface OnboardingStateDto { + steps: OnboardingStepState[]; + needs: OnboardingNeeds; + completedAt: string | null; + firstResult: OnboardingFirstResult | null; +} + +export function emptyOnboardingNeeds(): OnboardingNeeds { + return { + recordTarget: null, + areas: [], + medication: null, + sources: [], + visit: null, + units: { glucoseUnit: null, unitPreference: null }, + }; +} + +/** Nine steps, all pending, in the spec's order. */ +export function defaultOnboardingSteps(): OnboardingStepState[] { + return ONBOARDING_STEP_IDS.map((id) => ({ id, status: "pending" as const })); +} + +function member( + values: readonly T[], + value: unknown, +): T | null { + return typeof value === "string" && + (values as readonly string[]).includes(value) + ? (value as T) + : null; +} + +function memberList( + values: readonly T[], + value: unknown, +): T[] { + if (!Array.isArray(value)) return []; + const out: T[] = []; + for (const entry of value) { + const parsed = member(values, entry); + if (parsed !== null && !out.includes(parsed)) out.push(parsed); + } + return out; +} + +function asRecord(value: unknown): Record { + return typeof value === "object" && value !== null && !Array.isArray(value) + ? (value as Record) + : {}; +} + +/** + * Read the stored answers, dropping anything that is not a known value. + * + * Fail-soft rather than throwing: a column written by an older release, or by + * a value this release retired, must not 500 the account payload that rides + * every app boot. What cannot be read reads as unanswered, which is the state + * the flow already knows how to handle. + */ +export function parseOnboardingNeeds(value: unknown): OnboardingNeeds { + const raw = asRecord(value); + const units = asRecord(raw.units); + return { + recordTarget: member(ONBOARDING_RECORD_TARGETS, raw.recordTarget), + areas: memberList(ONBOARDING_AREA_KEYS, raw.areas), + medication: member(ONBOARDING_MEDICATION_ANSWERS, raw.medication), + sources: memberList(ONBOARDING_SOURCE_KEYS, raw.sources), + visit: member(ONBOARDING_VISIT_ANSWERS, raw.visit), + units: { + glucoseUnit: member(["mg/dL", "mmol/L"] as const, units.glucoseUnit), + unitPreference: member( + ["metric", "imperial"] as const, + units.unitPreference, + ), + }, + }; +} + +/** + * Read the stored steps as the full ordered nine. + * + * The stored blob is treated as a status lookup, never as the order: the order + * is this release's `ONBOARDING_STEP_IDS`, so a step added to the flow appears + * as `pending` for an account mid-way through rather than vanishing, and a + * step removed from the flow stops being published without a migration. + */ +export function parseOnboardingSteps(value: unknown): OnboardingStepState[] { + const stored = new Map(); + if (Array.isArray(value)) { + for (const entry of value) { + const row = asRecord(entry); + const id = member(ONBOARDING_STEP_IDS, row.id); + const status = member(ONBOARDING_STEP_STATUSES, row.status); + if (id && status) stored.set(id, status); + } + } + return ONBOARDING_STEP_IDS.map((id) => ({ + id, + status: stored.get(id) ?? "pending", + })); +} + +export function parseOnboardingFirstResult( + value: unknown, +): OnboardingFirstResult | null { + const raw = asRecord(value); + const task = member(ONBOARDING_FIRST_RESULT_TASKS, raw.task); + if (!task) return null; + return { + task, + target: typeof raw.target === "string" ? raw.target.slice(0, 64) : null, + completedAt: + typeof raw.completedAt === "string" && raw.completedAt.length > 0 + ? raw.completedAt + : null, + }; +} + +/** True once every step has been answered or deliberately passed. */ +export function everyOnboardingStepSettled( + steps: readonly OnboardingStepState[], +): boolean { + return steps.every((step) => step.status !== "pending"); +} + +/** + * Has the person finished setting up? + * + * Three conditions, and each of them is a way the flow can be left half-done: + * a step still waiting for an answer, a set of answers never confirmed, and a + * first-result task that was offered and never produced. The last one is the + * 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. + */ +export function isOnboardingSettled( + state: OnboardingStateDto | null | undefined, +): boolean { + if (!state) return true; + if (!everyOnboardingStepSettled(state.steps)) return false; + if (state.completedAt === null) return false; + return state.firstResult === null || state.firstResult.completedAt !== null; +} diff --git a/src/lib/validations/__tests__/onboarding-needs.test.ts b/src/lib/validations/__tests__/onboarding-needs.test.ts new file mode 100644 index 000000000..76d8a9b74 --- /dev/null +++ b/src/lib/validations/__tests__/onboarding-needs.test.ts @@ -0,0 +1,105 @@ +/** + * v1.39 (C1) — what `PATCH /api/onboarding/answers` will and will not accept. + * + * The union has an answer arm per step and a skip arm for six of the nine, and + * the split is a decision rather than a shape: `who` is the one required + * question, and `confirm` / `done` are acknowledgements. A skip arm added to + * one of those three would let a client record a pass through a screen that + * does not offer one, so the arms are checked against + * `ONBOARDING_SKIPPABLE_STEP_IDS` rather than against a list written twice. + * + * The other property here is that a skip can never carry an answer. That is + * what makes "skipped, and my areas are …" unrepresentable rather than merely + * discouraged, and it is why the route can trust the parsed body without + * re-checking the combination. + */ +import { describe, expect, it } from "vitest"; + +import { + ONBOARDING_SKIPPABLE_STEP_IDS, + ONBOARDING_STEP_IDS, +} from "@/lib/onboarding/needs"; +import { onboardingAnswerSchema } from "../onboarding-needs"; + +const SKIPPABLE: readonly string[] = ONBOARDING_SKIPPABLE_STEP_IDS; + +describe("onboardingAnswerSchema", () => { + it("accepts a skip for exactly the steps that may be passed", () => { + for (const step of ONBOARDING_STEP_IDS) { + const parsed = onboardingAnswerSchema.safeParse({ + step, + status: "skipped", + }); + expect( + parsed.success, + `\`${step}\` ${SKIPPABLE.includes(step) ? "should" : "should not"} be skippable`, + ).toBe(SKIPPABLE.includes(step)); + } + }); + + it("refuses a skip that carries an answer", () => { + expect( + onboardingAnswerSchema.safeParse({ + step: "areas", + status: "skipped", + areas: ["sleep"], + }).success, + ).toBe(false); + }); + + it("refuses an answer for the wrong step", () => { + expect( + onboardingAnswerSchema.safeParse({ step: "areas", medication: "yes" }) + .success, + ).toBe(false); + }); + + it("refuses a value outside the closed set", () => { + expect( + onboardingAnswerSchema.safeParse({ step: "visit", visit: "someday" }) + .success, + ).toBe(false); + expect( + onboardingAnswerSchema.safeParse({ step: "areas", areas: ["astrology"] }) + .success, + ).toBe(false); + }); + + it("refuses an unknown key alongside a valid answer", () => { + expect( + onboardingAnswerSchema.safeParse({ + step: "who", + recordTarget: "me", + userId: "someone-else", + }).success, + ).toBe(false); + }); + + it("accepts each answered step with its own payload", () => { + const bodies: unknown[] = [ + { step: "who", recordTarget: "both" }, + { step: "areas", areas: ["glucose", "cycle"] }, + { step: "medication", medication: "sometimes" }, + { step: "sources", sources: ["nightscout", "manual"] }, + { step: "visit", visit: "later" }, + { step: "units", units: { glucoseUnit: "mmol/L" } }, + { step: "confirm" }, + { + step: "first-result", + firstResult: { + task: "log-reading", + target: "glucose", + completed: true, + }, + }, + { step: "done" }, + ]; + expect(bodies.length).toBe(ONBOARDING_STEP_IDS.length); + for (const body of bodies) { + expect( + onboardingAnswerSchema.safeParse(body).success, + JSON.stringify(body), + ).toBe(true); + } + }); +}); diff --git a/src/lib/validations/onboarding-needs.ts b/src/lib/validations/onboarding-needs.ts new file mode 100644 index 000000000..043920fac --- /dev/null +++ b/src/lib/validations/onboarding-needs.ts @@ -0,0 +1,119 @@ +/** + * v1.39 (C1) — request schemas for the three needs-onboarding writes. + * + * They live outside the route files for the reason `./onboarding.ts` gives: a + * route module may only export handlers plus the Next.js route config, so the + * OpenAPI registry cannot import a schema declared inside one. Keeping the + * shapes here makes the published contract and the runtime parser the same + * object. + * + * Prisma-free by construction — everything below reads its vocabulary from + * `@/lib/onboarding/needs` and `@/lib/modules/registry`, both pure data. + */ +import { z } from "zod/v4"; + +import { ONBOARDING_AREA_KEYS } from "@/lib/modules/registry"; +import { + ONBOARDING_FIRST_RESULT_TASKS, + ONBOARDING_MEDICATION_ANSWERS, + ONBOARDING_RECORD_TARGETS, + ONBOARDING_SOURCE_KEYS, + ONBOARDING_VISIT_ANSWERS, +} from "@/lib/onboarding/needs"; + +/** + * One answered step. `status` is optional and only ever `"done"` on this arm — + * a skip is the separate arm below, so a body can never claim to skip a step + * and carry an answer for it at the same time. + */ +function answered( + id: Id, + shape: Shape, +) { + return z + .object({ + step: z.literal(id), + status: z.literal("done").optional(), + ...shape, + }) + .strict(); +} + +/** One deliberately passed step. No answer may ride along. */ +function skipped(id: Id) { + return z + .object({ step: z.literal(id), status: z.literal("skipped") }) + .strict(); +} + +/** + * Q6 — the unit answers. Both fields are optional and nullable: the screen + * only appears when glucose or weight is among the areas, and it may set one + * without the other. The route writes the record's real display columns from + * these and records the answer beside them. + */ +export const onboardingUnitsSchema = z + .object({ + glucoseUnit: z.enum(["mg/dL", "mmol/L"]).nullable().optional(), + unitPreference: z.enum(["metric", "imperial"]).nullable().optional(), + }) + .strict(); + +/** + * The task the flow ended on. `completed` is what turns the offer into a + * result — the server stamps the instant rather than taking one from the + * client, so a replay cannot backdate it. + */ +export const onboardingFirstResultSchema = z + .object({ + task: z.enum(ONBOARDING_FIRST_RESULT_TASKS), + target: z.string().trim().min(1).max(64).nullable().optional(), + completed: z.boolean().optional(), + }) + .strict(); + +/** + * `PATCH /api/onboarding/answers` — one step at a time. + * + * A flat union rather than a discriminated one: each step id appears twice + * (answered and skipped) and `z.discriminatedUnion` needs one option per + * literal. The three steps that are not questions — `who` is required, and + * `confirm` / `done` are acknowledgements — have no skip arm, so a client + * cannot record a pass through a screen that does not offer one. + */ +export const onboardingAnswerSchema = z.union([ + answered("who", { recordTarget: z.enum(ONBOARDING_RECORD_TARGETS) }), + answered("areas", { + areas: z + .array(z.enum(ONBOARDING_AREA_KEYS)) + .max(ONBOARDING_AREA_KEYS.length), + }), + answered("medication", { + medication: z.enum(ONBOARDING_MEDICATION_ANSWERS), + }), + answered("sources", { + sources: z + .array(z.enum(ONBOARDING_SOURCE_KEYS)) + .max(ONBOARDING_SOURCE_KEYS.length), + }), + answered("visit", { visit: z.enum(ONBOARDING_VISIT_ANSWERS) }), + answered("units", { units: onboardingUnitsSchema }), + answered("confirm", {}), + answered("first-result", { firstResult: onboardingFirstResultSchema }), + answered("done", {}), + skipped("areas"), + skipped("medication"), + skipped("sources"), + skipped("visit"), + skipped("units"), + skipped("first-result"), +]); + +export type OnboardingAnswerInput = z.infer; + +/** + * `POST /api/onboarding/restart` — no input beyond the session. Strict and + * empty rather than unvalidated, so a client that starts sending a field gets + * a 422 that names it instead of having it silently ignored. + */ +export const onboardingRestartSchema = z.object({}).strict(); From 021ea8f1ff6935fb093577495056d50e10c7637f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 10 Sep 2026 11:03:36 +0200 Subject: [PATCH 03/15] feat(api): let a record answer the setup questions, and act on them Three writes, all on the caller's own record. `PATCH /api/onboarding/answers` saves one step the moment it is answered, so leaving the flow and coming back resumes where it stopped. It is idempotent because the whole computation is a pure function of the row and the body, including the first-result instant, which is kept rather than re-taken. There is no ordering contract at all: a back button has to work. `POST /api/onboarding/complete` keeps everything it did and gains the confirm screen's job. For a record that answered the questions it derives the module map once and applies it, and the once-only latch is the point: a second confirm would re-apply a questionnaire over decisions taken in Settings since. Cycle is switched on through its own column and never off, because the gate already derives an unanswered cycle from recorded sex and an untick is not a request to retract that. `POST /api/onboarding/restart` puts the steps back and clears the latch. It writes no module state whatever: a re-run never turns off something switched on by hand, and the safest way to hold that is for this route to have no opinion about modules at all. It also leaves the first-run redirect's own stamp alone, so asking the questions again does not push anybody back through the old wizard. All three refuse outright while the session is acting on somebody else's record, at every grant level, so a delegate can never write another record's setup. The route into a managed profile's flow belongs with the rest of that record's configuration and is not smuggled in here. The demo allowlist gains the three: the flow has to be completable there with preset answers, and none of them creates health data. --- src/app/api/onboarding/answers/route.ts | 142 ++++++++++++++++ src/app/api/onboarding/complete/route.ts | 127 +++++++++++++- src/app/api/onboarding/restart/route.ts | 109 ++++++++++++ src/lib/openapi/routes/onboarding.ts | 207 ++++++++++++++++++++++- src/proxy.ts | 8 + 5 files changed, 587 insertions(+), 6 deletions(-) create mode 100644 src/app/api/onboarding/answers/route.ts create mode 100644 src/app/api/onboarding/restart/route.ts diff --git a/src/app/api/onboarding/answers/route.ts b/src/app/api/onboarding/answers/route.ts new file mode 100644 index 000000000..25048eea7 --- /dev/null +++ b/src/app/api/onboarding/answers/route.ts @@ -0,0 +1,142 @@ +/** + * v1.39 (C1) — `PATCH /api/onboarding/answers`. + * + * One step at a time, saved the moment it is answered, so leaving the flow and + * coming back resumes where it stopped (design spec §Principles 4). The body + * names the step and either its answer or a deliberate skip; the union in + * `src/lib/validations/onboarding-needs.ts` is what makes "skipped, and here is + * the answer anyway" unrepresentable. + * + * Idempotent: the state is computed by a pure function from the stored row and + * the body, so sending the same step twice writes the same value twice and the + * second write changes nothing a reader can see. The one instant that could + * drift — the first-result completion stamp — is kept from the existing row + * rather than re-taken. + * + * `requireAuth()` rather than `requireRecordAuth`: this route writes the + * CALLER's own record and refuses outright while the session is acting on + * somebody else's (403 `sharing.not_permitted`), at every grant level. The + * storage is keyed by record so a guardian running a managed profile's setup + * has a row to write, but the route into it belongs with the rest of the + * managed-record configuration (#939 / C2), not here — a delegable declaration + * is a reviewed entry in the sharing surface guard, not a default. + */ +import type { NextRequest } from "next/server"; + +import { apiHandler, requireAuth } from "@/lib/api-handler"; +import { + apiError, + apiSuccess, + returnAllZodIssues, + safeJson, +} from "@/lib/api-response"; +import { prisma, toJson } from "@/lib/db"; +import { annotate } from "@/lib/logging/context"; +import { applyOnboardingAnswer } from "@/lib/onboarding/needs-apply"; +import { + ONBOARDING_RECORD_SELECT, + readOnboardingRecordState, + toOnboardingStateDto, +} from "@/lib/onboarding/needs-store"; +import { checkRateLimit } from "@/lib/rate-limit"; +import { onboardingAnswerSchema } from "@/lib/validations/onboarding-needs"; + +export const dynamic = "force-dynamic"; + +const RATE_LIMIT = 120; +const RATE_WINDOW_MS = 10 * 60 * 1000; + +export const PATCH = apiHandler(async (request: NextRequest) => { + const { user } = await requireAuth(); + annotate({ action: { name: "onboarding.answer.save" } }); + + const rl = await checkRateLimit( + `onboarding-answers:${user.id}`, + RATE_LIMIT, + RATE_WINDOW_MS, + ); + if (!rl.allowed) { + annotate({ + action: { name: "onboarding.answer.save" }, + meta: { outcome: "rate_limited" }, + }); + return apiError("Too many onboarding writes, try again later", 429, { + errorCode: "onboarding.answers.rateLimited", + }); + } + + const { data: body, error: jsonError } = await safeJson(request, { + maxBytes: 16 * 1024, + }); + if (jsonError) return jsonError; + + const parsed = onboardingAnswerSchema.safeParse(body); + if (!parsed.success) { + annotate({ + action: { name: "onboarding.answer.save" }, + meta: { outcome: "validation_failed" }, + }); + return returnAllZodIssues(parsed.error, 422, { + errorCode: "onboarding.answers.invalid", + }); + } + const answer = parsed.data; + + const existing = await prisma.onboardingRecord.findUnique({ + where: { userId: user.id }, + select: ONBOARDING_RECORD_SELECT, + }); + const next = applyOnboardingAnswer( + readOnboardingRecordState(existing), + answer, + new Date(), + ); + + // The unit answers are the one step whose consequence lives outside this + // table: `User.glucoseUnit` and `User.unitPreference` are what every display + // surface already reads, so the flow writes them rather than inventing a + // second source of truth. What is kept beside them in `needs.units` is the + // answer as given, so a restart can show it back without re-deriving it from + // a column Settings may have changed since. + if (answer.step === "units" && answer.status !== "skipped") { + const data: { glucoseUnit?: string; unitPreference?: string } = {}; + if (answer.units.glucoseUnit) data.glucoseUnit = answer.units.glucoseUnit; + if (answer.units.unitPreference) { + data.unitPreference = answer.units.unitPreference; + } + if (Object.keys(data).length > 0) { + await prisma.user.update({ where: { id: user.id }, data }); + } + } + + const written = await prisma.onboardingRecord.upsert({ + where: { userId: user.id }, + // Field-by-field, never a spread of the parsed body: the three columns are + // the pure function's output, and nothing the client sent reaches Prisma + // without passing through it. + create: { + userId: user.id, + needsJson: toJson(next.needs), + stepsJson: toJson(next.steps), + firstResultJson: next.firstResult ? toJson(next.firstResult) : undefined, + }, + update: { + needsJson: toJson(next.needs), + stepsJson: toJson(next.steps), + ...(next.firstResult + ? { firstResultJson: toJson(next.firstResult) } + : {}), + }, + select: ONBOARDING_RECORD_SELECT, + }); + + annotate({ + action: { name: "onboarding.answer.save" }, + meta: { + step: answer.step, + status: answer.status === "skipped" ? "skipped" : "done", + }, + }); + + return apiSuccess({ onboarding: toOnboardingStateDto(written) }); +}); diff --git a/src/app/api/onboarding/complete/route.ts b/src/app/api/onboarding/complete/route.ts index 4bfe35382..2246885f4 100644 --- a/src/app/api/onboarding/complete/route.ts +++ b/src/app/api/onboarding/complete/route.ts @@ -1,8 +1,20 @@ -import { prisma } from "@/lib/db"; +import { prisma, toJson } from "@/lib/db"; import { apiHandler, requireAuth } from "@/lib/api-handler"; import { annotate } from "@/lib/logging/context"; import { apiSuccess, apiError, safeJson } from "@/lib/api-response"; import { setOnboardingPendingCookie } from "@/lib/auth/session"; +import { invalidateUserHealthScore } from "@/lib/cache/invalidate"; +import { getOrCreateCycleProfile } from "@/lib/cycle/profile"; +import { normalisePrefs } from "@/lib/modules/gate"; +import { + deriveOnboardingModuleDefaults, + mergeDerivedModulePreferences, +} from "@/lib/modules/registry"; +import { + ONBOARDING_RECORD_SELECT, + readOnboardingRecordState, + toOnboardingStateDto, +} from "@/lib/onboarding/needs-store"; import { NextRequest } from "next/server"; import { z } from "zod/v4"; import { onboardingCompleteSchema } from "@/lib/validations/onboarding"; @@ -10,6 +22,22 @@ import { onboardingCompleteSchema } from "@/lib/validations/onboarding"; /** * Complete the onboarding flow. Saves optional profile data and marks * onboarding as completed. + * + * v1.39 (C1) — this is also the confirm screen's endpoint for the needs-based + * flow, and the two live side by side rather than one replacing the other. + * The legacy half is unchanged: the optional profile fields, the completion + * stamp on `User.onboardingCompletedAt`, and the cleared pending cookie. The + * needs half runs only for a record that actually answered the questions + * (`needs.recordTarget` is set) and is what turns the answers into a module + * map — ONCE, guarded by `OnboardingRecord.modulesDerivedAt`, because a + * re-derivation would re-apply the questionnaire over decisions taken in + * Settings since. `POST /api/onboarding/restart` clears that marker when the + * person asks for the questions again. + * + * The goal slugs of today (`src/lib/onboarding/goals.ts`, the four-step wizard + * at `POST /api/onboarding/step`) are untouched and stay live until C2 + * replaces that surface. They decide TILE ORDER; the answers below decide + * WHICH MODULES ARE ON. Neither reads the other. */ export const POST = apiHandler(async (request: NextRequest) => { const { user } = await requireAuth(); @@ -59,5 +87,100 @@ export const POST = apiHandler(async (request: NextRequest) => { // next navigation drops the /onboarding redirect immediately. await setOnboardingPendingCookie(false); - return apiSuccess({ completed: true }); + const onboarding = await completeNeedsFlow(user.id); + + return apiSuccess({ completed: true, ...(onboarding ? { onboarding } : {}) }); }); + +/** + * The needs half: derive the module map once, mark the confirm step, stamp the + * flow's own completion. + * + * Returns the published state, or `null` for a caller that never entered the + * needs flow — a legacy wizard completion then answers exactly the body it + * always did. + */ +async function completeNeedsFlow(userId: string) { + const record = await prisma.onboardingRecord.findUnique({ + where: { userId }, + select: ONBOARDING_RECORD_SELECT, + }); + if (!record) return null; + + const state = readOnboardingRecordState(record); + if (state.needs.recordTarget === null) { + // The row exists but Q1 was never answered, so there is nothing to derive + // from. Stamping a completion here would claim a setup that did not happen. + annotate({ + action: { name: "onboarding.needs.complete" }, + meta: { outcome: "no_answers" }, + }); + return toOnboardingStateDto(record); + } + + let derived = false; + if (record.modulesDerivedAt === null) { + const defaults = deriveOnboardingModuleDefaults({ + recordTarget: state.needs.recordTarget, + areas: state.needs.areas, + // An unanswered (skipped) question is the conservative answer: no + // medication surface, no doctor report. Both stay one click away in + // Settings, which is the whole ordering-versus-removal decision. + medication: state.needs.medication ?? "no", + visit: state.needs.visit ?? "no", + }); + + const row = await prisma.user.findUnique({ + where: { id: userId }, + select: { modulePreferencesJson: true }, + }); + const merged = mergeDerivedModulePreferences( + normalisePrefs(row?.modulePreferencesJson), + defaults.preferences, + ); + await prisma.user.update({ + where: { id: userId }, + data: { modulePreferencesJson: merged }, + }); + + // `cycle` delegates to `CycleProfile.cycleTrackingEnabled`, so the module + // blob never owns it. Only ever switched ON here: the gate derives an + // unanswered cycle from recorded sex, and an untick is not a request to + // retract that. + if (defaults.cycleTracking) { + await getOrCreateCycleProfile(userId); + await prisma.cycleProfile.update({ + where: { userId }, + data: { cycleTrackingEnabled: true }, + }); + } + + // A module toggle adds or removes a Health Score pillar, so the cached + // composite was computed from a composition that no longer holds. + invalidateUserHealthScore(userId); + derived = true; + } + + const now = new Date(); + const steps = state.steps.map((step) => + step.id === "confirm" && step.status === "pending" + ? { ...step, status: "done" as const } + : step, + ); + const written = await prisma.onboardingRecord.update({ + where: { userId }, + data: { + stepsJson: toJson(steps), + completedAt: now, + ...(derived ? { modulesDerivedAt: now } : {}), + }, + select: ONBOARDING_RECORD_SELECT, + }); + + annotate({ + action: { name: "onboarding.needs.complete" }, + meta: { outcome: derived ? "derived" : "already_derived" }, + }); + + return toOnboardingStateDto(written); +} diff --git a/src/app/api/onboarding/restart/route.ts b/src/app/api/onboarding/restart/route.ts new file mode 100644 index 000000000..0990e898b --- /dev/null +++ b/src/app/api/onboarding/restart/route.ts @@ -0,0 +1,109 @@ +/** + * v1.39 (C1) — `POST /api/onboarding/restart`. + * + * "Set up again", from Settings. It puts the nine steps back to `pending`, + * clears the flow's completion stamp and clears the derivation marker so the + * next confirm may derive a module map again. + * + * What it deliberately does NOT do: + * + * - touch a single module. The design spec's ordering-versus-removal + * decision says a restart never turns off a module the person turned on by + * hand, and the safest way to hold that is for this route to write no + * module state at all. The protection against the SECOND confirm is + * elsewhere, in `mergeDerivedModulePreferences`. + * - clear the answers. They come back as the prefill for the re-run, which + * is what "never re-ask a value the account holds" means here. + * - clear `User.onboardingCompletedAt`. That column gates the first-run + * redirect in `src/proxy.ts`; clearing it would drop the person back into + * the wizard of today rather than into the questions they asked for. + * - forget a first result that really happened. The task produced a reading, + * a medication or a connection; a re-run of the questions does not unmake + * it. + * + * `requireAuth()` for the reason the answers route gives: the caller's own + * record, refused outright under an acting-account switch. + */ +import type { NextRequest } from "next/server"; + +import { apiHandler, requireAuth } from "@/lib/api-handler"; +import { + apiError, + apiSuccess, + getClientIp, + returnAllZodIssues, + safeJson, +} from "@/lib/api-response"; +import { auditLog } from "@/lib/auth/audit"; +import { prisma, toJson } from "@/lib/db"; +import { annotate } from "@/lib/logging/context"; +import { defaultOnboardingSteps } from "@/lib/onboarding/needs"; +import { + ONBOARDING_RECORD_SELECT, + toOnboardingStateDto, +} from "@/lib/onboarding/needs-store"; +import { checkRateLimit } from "@/lib/rate-limit"; +import { onboardingRestartSchema } from "@/lib/validations/onboarding-needs"; + +export const dynamic = "force-dynamic"; + +export const POST = apiHandler(async (request: NextRequest) => { + const { user } = await requireAuth(); + annotate({ action: { name: "onboarding.needs.restart" } }); + + const rl = await checkRateLimit( + `onboarding-restart:${user.id}`, + 10, + 10 * 60 * 1000, + ); + if (!rl.allowed) { + annotate({ + action: { name: "onboarding.needs.restart" }, + meta: { outcome: "rate_limited" }, + }); + return apiError("Too many onboarding writes, try again later", 429, { + errorCode: "onboarding.restart.rateLimited", + }); + } + + const { data: body, error: jsonError } = await safeJson(request, { + maxBytes: 4 * 1024, + }); + if (jsonError) return jsonError; + + const parsed = onboardingRestartSchema.safeParse(body ?? {}); + if (!parsed.success) { + annotate({ + action: { name: "onboarding.needs.restart" }, + meta: { outcome: "validation_failed" }, + }); + return returnAllZodIssues(parsed.error, 422, { + errorCode: "onboarding.restart.invalid", + }); + } + + const steps = defaultOnboardingSteps(); + const written = await prisma.onboardingRecord.upsert({ + where: { userId: user.id }, + create: { userId: user.id, stepsJson: toJson(steps) }, + update: { + stepsJson: toJson(steps), + completedAt: null, + modulesDerivedAt: null, + }, + select: ONBOARDING_RECORD_SELECT, + }); + + await auditLog("onboarding.restart", { + userId: user.id, + ipAddress: getClientIp(request), + details: { steps: steps.length }, + }); + + annotate({ + action: { name: "onboarding.needs.restart" }, + meta: { outcome: "reset" }, + }); + + return apiSuccess({ onboarding: toOnboardingStateDto(written) }); +}); diff --git a/src/lib/openapi/routes/onboarding.ts b/src/lib/openapi/routes/onboarding.ts index 2d7b3b66a..06a81dba6 100644 --- a/src/lib/openapi/routes/onboarding.ts +++ b/src/lib/openapi/routes/onboarding.ts @@ -23,13 +23,131 @@ import { z } from "zod/v4"; import type { ZodOpenApiObject } from "zod-openapi"; +import { + ONBOARDING_FIRST_RESULT_TASKS, + ONBOARDING_MEDICATION_ANSWERS, + ONBOARDING_RECORD_TARGETS, + ONBOARDING_SOURCE_KEYS, + ONBOARDING_STEP_IDS, + ONBOARDING_STEP_STATUSES, + ONBOARDING_VISIT_ANSWERS, +} from "@/lib/onboarding/needs"; import { tourProgressSchema } from "@/lib/onboarding/tour-progress"; +import { ONBOARDING_AREA_KEYS } from "@/lib/modules/registry"; import { onboardingCompleteSchema, onboardingStepSchema, } from "@/lib/validations/onboarding"; +import { + onboardingAnswerSchema, + onboardingRestartSchema, +} from "@/lib/validations/onboarding-needs"; import { dataEnvelope, errorEnvelope, stdResponses } from "./shared"; +/* ── v1.39 (C1): the needs-based flow ─────────────────────────────────────── */ + +const onboardingNeedsResource = z + .object({ + recordTarget: z + .enum(ONBOARDING_RECORD_TARGETS) + .nullable() + .describe("Q1 — whose record this is. Null until answered."), + areas: z + .array(z.enum(ONBOARDING_AREA_KEYS)) + .describe( + "Q2 — the areas to keep an eye on. Each one maps to at least one module through `ONBOARDING_AREA_MODULES`; the map is the server's, and a client must not re-derive it.", + ), + medication: z + .enum(ONBOARDING_MEDICATION_ANSWERS) + .nullable() + .describe("Q3 — medication on a schedule."), + sources: z + .array(z.enum(ONBOARDING_SOURCE_KEYS)) + .describe("Q4 — where the readings come from today."), + visit: z + .enum(ONBOARDING_VISIT_ANSWERS) + .nullable() + .describe("Q5 — a doctor's visit coming up."), + units: z + .object({ + glucoseUnit: z.enum(["mg/dL", "mmol/L"]).nullable(), + unitPreference: z.enum(["metric", "imperial"]).nullable(), + }) + .describe( + "Q6 — the unit answers as given. NOT the source of truth for display: `glucoseUnit` and `unitPreference` on the account payload are, and the answers step writes them. This records what the flow was told.", + ), + }) + .meta({ + id: "OnboardingNeeds", + description: + "The setup answers as given. Every field is null or empty until its question is answered.", + }); + +const onboardingStepResource = z + .object({ + id: z.enum(ONBOARDING_STEP_IDS), + status: z.enum(ONBOARDING_STEP_STATUSES), + }) + .meta({ + id: "OnboardingStepState", + description: + "One step of the setup flow. The ids are stable; a client that meets one it does not know SKIPS it rather than refusing the list, so the flow may grow a screen without a client release.", + }); + +const onboardingFirstResultResource = z + .object({ + task: z.enum(ONBOARDING_FIRST_RESULT_TASKS), + target: z + .string() + .nullable() + .describe( + "What the task is about — a source key for a connection, an area key for a reading. Null for the medication task.", + ), + completedAt: z.iso + .datetime({ offset: true }) + .nullable() + .describe("When the offered task produced its result, or null."), + }) + .meta({ + id: "OnboardingFirstResult", + description: + "The one task the flow offered at the end, and whether it produced its result.", + }); + +export const onboardingStateResource = z + .object({ + steps: z.array(onboardingStepResource), + needs: onboardingNeedsResource, + completedAt: z.iso + .datetime({ offset: true }) + .nullable() + .describe( + "When the needs flow was completed. Distinct from the account payload's `onboardingCompletedAt`, which stays the first-run redirect's gate: `POST /api/onboarding/restart` clears this one and leaves that one alone.", + ), + firstResult: onboardingFirstResultResource.nullable(), + }) + .meta({ + id: "OnboardingState", + description: + "The needs-based setup flow for one record. Always present on the account payload — a record that never entered the flow reads as empty answers with nine `pending` steps, so a client branches on the step statuses rather than on the field's existence.", + }); + +const onboardingStateEnvelope = dataEnvelope( + z.object({ onboarding: onboardingStateResource }), + "OnboardingStateEnvelope", +); + +const onboardingAnswerRequest = onboardingAnswerSchema.meta({ + id: "OnboardingAnswerRequest", + description: + "One step, answered or deliberately passed. The union has an arm per step id plus a skip arm for the six steps that may be passed — `who` is the one required question, and `confirm` / `done` are acknowledgements rather than questions, so none of the three has a skip arm. Every arm is strict: an unknown key is a 422, and a skip may never carry an answer.", +}); + +const onboardingRestartRequest = onboardingRestartSchema.meta({ + id: "OnboardingRestartRequest", + description: "No input beyond the session. Send `{}`.", +}); + const tourProgressResource = tourProgressSchema.meta({ id: "TourProgress", description: @@ -121,6 +239,84 @@ const onboardingCompleteRequest = onboardingCompleteSchema.meta({ }); export const onboardingPaths: NonNullable = { + "/api/onboarding/answers": { + patch: { + tags: ["Onboarding"], + summary: "Save one answer of the needs-based setup flow", + description: + "Persists one step of the setup questionnaire the moment it is answered, so leaving and returning resumes at the same step. The body names the step and carries either its answer or `status: \"skipped\"`; the two can never ride together.\n\nIdempotent. The stored state is computed by a pure function of the row and the body, so sending the same step twice writes the same value and the second write changes nothing a reader can see — including the first-result completion instant, which is kept from the existing row rather than re-taken. There is no ordering contract: steps may be answered, re-answered and passed in any order, which is what makes a back button work.\n\nThe `units` step is the one answer with a consequence outside this state: it writes the record's `glucoseUnit` and `unitPreference`, which every display surface already reads, and records the answer beside them.\n\nCookie or wildcard Bearer, and the CALLER's own record only: a request made while the session is acting on somebody else's record is refused with 403 `sharing.not_permitted` at every grant level, so a delegate can never write another record's setup. Rate-limited to 120 writes per 10 minutes per account.", + requestBody: { + required: true, + content: { "application/json": { schema: onboardingAnswerRequest } }, + }, + responses: { + "200": { + description: "The full setup state after the write.", + content: { + "application/json": { schema: onboardingStateEnvelope }, + }, + }, + "413": { + description: "Body exceeds 16 KiB.", + content: { "application/json": { schema: errorEnvelope } }, + }, + "415": { + description: "`Content-Type` is not `application/json`.", + content: { "application/json": { schema: errorEnvelope } }, + }, + ...stdResponses, + "422": { + description: + "The body did not validate. `meta.errorCode` = `onboarding.answers.invalid`, with every issue on the wire.", + content: { "application/json": { schema: errorEnvelope } }, + }, + "429": { + description: + "More than 120 answer writes in 10 minutes. `meta.errorCode` = `onboarding.answers.rateLimited`.", + content: { "application/json": { schema: errorEnvelope } }, + }, + }, + }, + }, + "/api/onboarding/restart": { + post: { + tags: ["Onboarding"], + summary: "Ask the setup questions again", + description: + "\"Set up again\", from Settings. Puts the nine steps back to `pending`, clears the flow's own completion stamp, and clears the derivation marker so the next `POST /api/onboarding/complete` may derive a module map again.\n\nIt writes no module state at all: the ordering-versus-removal decision says a re-run never turns off a module somebody turned on by hand, and the protection against the second derivation lives in the merge rather than here. It also keeps the answers, as the prefill for the re-run, keeps a first result that really happened, and leaves the account payload's `onboardingCompletedAt` alone so nobody is pushed back through the first-run redirect.\n\nCookie or wildcard Bearer, own record only — same refusal under a switch as the answers route. Rate-limited to 10 restarts per 10 minutes per account.", + requestBody: { + required: true, + content: { "application/json": { schema: onboardingRestartRequest } }, + }, + responses: { + "200": { + description: "The reset setup state.", + content: { + "application/json": { schema: onboardingStateEnvelope }, + }, + }, + "413": { + description: "Body exceeds 4 KiB.", + content: { "application/json": { schema: errorEnvelope } }, + }, + "415": { + description: "`Content-Type` is not `application/json`.", + content: { "application/json": { schema: errorEnvelope } }, + }, + ...stdResponses, + "422": { + description: + "The body carried a field. `meta.errorCode` = `onboarding.restart.invalid`.", + content: { "application/json": { schema: errorEnvelope } }, + }, + "429": { + description: + "More than 10 restarts in 10 minutes. `meta.errorCode` = `onboarding.restart.rateLimited`.", + content: { "application/json": { schema: errorEnvelope } }, + }, + }, + }, + }, "/api/onboarding/step": { post: { tags: ["Onboarding"], @@ -178,9 +374,9 @@ export const onboardingPaths: NonNullable = { "/api/onboarding/complete": { post: { tags: ["Onboarding"], - summary: "Stamp onboarding as complete (legacy single-shot path)", + summary: "Stamp onboarding as complete, and derive the module map", description: - "Marks onboarding finished and saves whatever profile fields came with it, then clears the proxy-readable pending cookie so the next navigation stops redirecting to `/onboarding`.\n\nThe older sibling of `POST /api/onboarding/step`. It is unconditional in a way the step route is not: it stamps the completion whatever the stored step is, it re-stamps on every call rather than refusing a second one, and it writes no audit row and enforces no rate limit of its own. It also does NOT seed the dashboard from the goal selection — that only happens on the step route's completing call.\n\nCalling this mid-wizard makes every remaining `POST /api/onboarding/step` answer 409 `onboarding.step.completed`, because that route's guarded update requires the completion stamp to still be null.\n\nCookie or wildcard Bearer; `userId` is never read from the body.", + "Marks onboarding finished and saves whatever profile fields came with it, then clears the proxy-readable pending cookie so the next navigation stops redirecting to `/onboarding`.\n\nv1.39 — this is also the needs-based flow's confirm endpoint, and the two halves live side by side. For a record that answered the questions (Q1 is set), it derives that record's module map from the answers and applies it, ONCE: the derivation marker is what stops a second confirm re-applying the questionnaire over decisions taken in Settings since, and `POST /api/onboarding/restart` clears the marker when the person asks for the questions again. The merge is one-directional — a module the answers name is switched on, a module they do not name is switched off only where the record does not already carry an explicit on. The `cycle` key is not written here at all: it delegates to the cycle profile, which this route sets to `true` when the cycle area was chosen and never to `false`. The response then carries `onboarding` beside `completed`; a caller that never entered the needs flow gets the fixed acknowledgement it always did.\n\nThe older sibling of `POST /api/onboarding/step`. It is unconditional in a way the step route is not: it stamps the completion whatever the stored step is, it re-stamps on every call rather than refusing a second one, and it writes no audit row and enforces no rate limit of its own. It also does NOT seed the dashboard from the goal selection — that only happens on the step route's completing call.\n\nCalling this mid-wizard makes every remaining `POST /api/onboarding/step` answer 409 `onboarding.step.completed`, because that route's guarded update requires the completion stamp to still be null.\n\nCookie or wildcard Bearer; `userId` is never read from the body.", requestBody: { required: true, content: { "application/json": { schema: onboardingCompleteRequest } }, @@ -188,11 +384,14 @@ export const onboardingPaths: NonNullable = { responses: { "200": { description: - "Onboarding stamped complete. The body is the fixed `{ completed: true }` acknowledgement — it does not echo what was saved.", + "Onboarding stamped complete. `completed` is the fixed acknowledgement — it does not echo the profile fields that were saved. `onboarding` is present only for a record that entered the needs-based flow, and carries the state after the derivation.", content: { "application/json": { schema: dataEnvelope( - z.object({ completed: z.literal(true) }), + z.object({ + completed: z.literal(true), + onboarding: onboardingStateResource.optional(), + }), "OnboardingCompleteEnvelope", ), }, diff --git a/src/proxy.ts b/src/proxy.ts index a43c6cb32..97a86c93a 100644 --- a/src/proxy.ts +++ b/src/proxy.ts @@ -158,6 +158,14 @@ const DEMO_MUTATION_ALLOWLIST: ReadonlyArray<{ path: string; method: string }> = // Chart display-pref toggles above the charts (dashboard + insights): { path: "/api/dashboard/chart-overlay-prefs", method: "PUT" }, { path: "/api/dashboard/widgets", method: "PUT" }, + // v1.39 (C1) — the needs-based setup flow, which the demo has to be able + // to complete with preset answers (design spec §Principles 5). All three + // are user-scoped preference writes: closed enum answers, the step ledger, + // and the module map derived from them. No health data is created by any + // of them, and the demo account's own record is the only thing they touch. + { path: "/api/onboarding/answers", method: "PATCH" }, + { path: "/api/onboarding/complete", method: "POST" }, + { path: "/api/onboarding/restart", method: "POST" }, ]; // Legacy route redirects (German → English) From dbfad0f28e4e76bb87b2dfa16859157e2a4bd978 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 10 Sep 2026 11:03:47 +0200 Subject: [PATCH 04/15] feat(api): publish the setup state on the account payload, and read it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Additive: `onboarding` carries the nine steps with their status, the answers as given, the flow's own completion stamp and the one task it offered. Always present — a record that never entered the flow reads as empty answers with nine pending steps, so a client branches on the statuses rather than on whether the field exists, and an unknown step id is skipped rather than refused. It is resolved for the CALLER's own record rather than for the active one, unlike the two module fields beside it. The three write routes are actor-only, and publishing a setup state the browser cannot write would be a read it could act on and never complete. Both ends land here, because a field with no reader is the failure this repository keeps rediscovering. The getting-started checklist orders its rows from the answers — medication first for somebody who takes one daily, the data-source row first for somebody who named a wearable — and it stays on screen while the flow is unfinished. That is the second half: five readings were never evidence that the setup finished, and now a pending step, an unconfirmed set of answers or an offered task that produced nothing keeps the list where it is. The ordering only applies once the answers are confirmed. Re-ordering the dashboard under somebody who is still mid-question would be movement they did not ask for. --- docs/api/openapi.yaml | 678 +++++++++++++++++- src/app/api/auth/me/route.ts | 87 ++- .../onboarding/getting-started-checklist.tsx | 10 + src/hooks/use-auth.ts | 17 + .../onboarding/__tests__/checklist.test.ts | 6 +- src/lib/onboarding/checklist.ts | 80 ++- src/lib/openapi/routes/account-sharing.ts | 9 + 7 files changed, 852 insertions(+), 35 deletions(-) diff --git a/docs/api/openapi.yaml b/docs/api/openapi.yaml index 0acb08f3d..388d2826b 100644 --- a/docs/api/openapi.yaml +++ b/docs/api/openapi.yaml @@ -16162,6 +16162,125 @@ paths: "401": *a1 "422": *a4 "429": *a5 + /api/onboarding/answers: + patch: + tags: + - Onboarding + summary: Save one answer of the needs-based setup flow + description: >- + Persists one step of the setup questionnaire the moment it is answered, so leaving and returning resumes at the + same step. The body names the step and carries either its answer or `status: "skipped"`; the two can never ride + together. + + + Idempotent. The stored state is computed by a pure function of the row and the body, so sending the same step + twice writes the same value and the second write changes nothing a reader can see — including the first-result + completion instant, which is kept from the existing row rather than re-taken. There is no ordering contract: + steps may be answered, re-answered and passed in any order, which is what makes a back button work. + + + The `units` step is the one answer with a consequence outside this state: it writes the record's `glucoseUnit` + and `unitPreference`, which every display surface already reads, and records the answer beside them. + + + Cookie or wildcard Bearer, and the CALLER's own record only: a request made while the session is acting on + somebody else's record is refused with 403 `sharing.not_permitted` at every grant level, so a delegate can never + write another record's setup. Rate-limited to 120 writes per 10 minutes per account. + requestBody: + required: true + content: + application/json: + schema: + $ref: "#/components/schemas/OnboardingAnswerRequest" + responses: + "200": + description: The full setup state after the write. + content: + application/json: + schema: + $ref: "#/components/schemas/OnboardingStateEnvelope" + "401": *a1 + "413": + description: Body exceeds 16 KiB. + content: + application/json: + schema: + $ref: "#/components/schemas/ErrorEnvelope" + "415": + description: "`Content-Type` is not `application/json`." + content: + application/json: + schema: + $ref: "#/components/schemas/ErrorEnvelope" + "422": + description: The body did not validate. `meta.errorCode` = `onboarding.answers.invalid`, with every issue on the wire. + content: + application/json: + schema: + $ref: "#/components/schemas/ErrorEnvelope" + "429": + description: More than 120 answer writes in 10 minutes. `meta.errorCode` = `onboarding.answers.rateLimited`. + content: + application/json: + schema: + $ref: "#/components/schemas/ErrorEnvelope" + /api/onboarding/restart: + post: + tags: + - Onboarding + summary: Ask the setup questions again + description: >- + "Set up again", from Settings. Puts the nine steps back to `pending`, clears the flow's own completion stamp, + and clears the derivation marker so the next `POST /api/onboarding/complete` may derive a module map again. + + + It writes no module state at all: the ordering-versus-removal decision says a re-run never turns off a module + somebody turned on by hand, and the protection against the second derivation lives in the merge rather than + here. It also keeps the answers, as the prefill for the re-run, keeps a first result that really happened, and + leaves the account payload's `onboardingCompletedAt` alone so nobody is pushed back through the first-run + redirect. + + + Cookie or wildcard Bearer, own record only — same refusal under a switch as the answers route. Rate-limited to + 10 restarts per 10 minutes per account. + requestBody: + required: true + content: + application/json: + schema: + $ref: "#/components/schemas/OnboardingRestartRequest" + responses: + "200": + description: The reset setup state. + content: + application/json: + schema: + $ref: "#/components/schemas/OnboardingStateEnvelope" + "401": *a1 + "413": + description: Body exceeds 4 KiB. + content: + application/json: + schema: + $ref: "#/components/schemas/ErrorEnvelope" + "415": + description: "`Content-Type` is not `application/json`." + content: + application/json: + schema: + $ref: "#/components/schemas/ErrorEnvelope" + "422": + description: The body carried a field. `meta.errorCode` = `onboarding.restart.invalid`. + content: + application/json: + schema: + $ref: "#/components/schemas/ErrorEnvelope" + "429": + description: More than 10 restarts in 10 minutes. `meta.errorCode` = `onboarding.restart.rateLimited`. + content: + application/json: + schema: + $ref: "#/components/schemas/ErrorEnvelope" /api/onboarding/step: post: tags: @@ -16245,12 +16364,23 @@ paths: post: tags: - Onboarding - summary: Stamp onboarding as complete (legacy single-shot path) + summary: Stamp onboarding as complete, and derive the module map description: >- Marks onboarding finished and saves whatever profile fields came with it, then clears the proxy-readable pending cookie so the next navigation stops redirecting to `/onboarding`. + v1.39 — this is also the needs-based flow's confirm endpoint, and the two halves live side by side. For a record + that answered the questions (Q1 is set), it derives that record's module map from the answers and applies it, + ONCE: the derivation marker is what stops a second confirm re-applying the questionnaire over decisions taken in + Settings since, and `POST /api/onboarding/restart` clears the marker when the person asks for the questions + again. The merge is one-directional — a module the answers name is switched on, a module they do not name is + switched off only where the record does not already carry an explicit on. The `cycle` key is not written here at + all: it delegates to the cycle profile, which this route sets to `true` when the cycle area was chosen and never + to `false`. The response then carries `onboarding` beside `completed`; a caller that never entered the needs + flow gets the fixed acknowledgement it always did. + + The older sibling of `POST /api/onboarding/step`. It is unconditional in a way the step route is not: it stamps the completion whatever the stored step is, it re-stamps on every call rather than refusing a second one, and it writes no audit row and enforces no rate limit of its own. It also does NOT seed the dashboard from the goal @@ -16270,8 +16400,9 @@ paths: $ref: "#/components/schemas/OnboardingCompleteRequest" responses: "200": - description: "Onboarding stamped complete. The body is the fixed `{ completed: true }` acknowledgement — it does not - echo what was saved." + description: Onboarding stamped complete. `completed` is the fixed acknowledgement — it does not echo the profile fields + that were saved. `onboarding` is present only for a record that entered the needs-based flow, and carries + the state after the derivation. content: application/json: schema: @@ -27554,6 +27685,282 @@ components: minLength: 1 required: - ids + OnboardingAnswerRequest: + anyOf: + - type: object + properties: + step: + type: string + const: who + status: + type: string + const: done + recordTarget: + type: string + enum: + - me + - someone-else + - both + required: + - step + - recordTarget + additionalProperties: false + - type: object + properties: + step: + type: string + const: areas + status: + type: string + const: done + areas: + maxItems: 9 + type: array + items: + type: string + enum: + - blood-pressure + - weight-body + - glucose + - sleep + - mood + - cycle + - activity + - labs + - illness + required: + - step + - areas + additionalProperties: false + - type: object + properties: + step: + type: string + const: medication + status: + type: string + const: done + medication: + type: string + enum: + - yes + - no + - sometimes + required: + - step + - medication + additionalProperties: false + - type: object + properties: + step: + type: string + const: sources + status: + type: string + const: done + sources: + maxItems: 10 + type: array + items: + type: string + enum: + - manual + - apple-health + - withings + - oura + - whoop + - polar + - fitbit + - strava + - nightscout + - file + required: + - step + - sources + additionalProperties: false + - type: object + properties: + step: + type: string + const: visit + status: + type: string + const: done + visit: + type: string + enum: + - within-a-month + - later + - no + required: + - step + - visit + additionalProperties: false + - type: object + properties: + step: + type: string + const: units + status: + type: string + const: done + units: + type: object + properties: + glucoseUnit: + anyOf: + - type: string + enum: + - mg/dL + - mmol/L + - type: "null" + unitPreference: + anyOf: + - type: string + enum: + - metric + - imperial + - type: "null" + additionalProperties: false + required: + - step + - units + additionalProperties: false + - type: object + properties: + step: + type: string + const: confirm + status: + type: string + const: done + required: + - step + additionalProperties: false + - type: object + properties: + step: + type: string + const: first-result + status: + type: string + const: done + firstResult: + type: object + properties: + task: + type: string + enum: + - connect-source + - add-medication + - log-reading + target: + anyOf: + - type: string + minLength: 1 + maxLength: 64 + - type: "null" + completed: + type: boolean + required: + - task + additionalProperties: false + required: + - step + - firstResult + additionalProperties: false + - type: object + properties: + step: + type: string + const: done + status: + type: string + const: done + required: + - step + additionalProperties: false + - type: object + properties: + step: + type: string + const: areas + status: + type: string + const: skipped + required: + - step + - status + additionalProperties: false + - type: object + properties: + step: + type: string + const: medication + status: + type: string + const: skipped + required: + - step + - status + additionalProperties: false + - type: object + properties: + step: + type: string + const: sources + status: + type: string + const: skipped + required: + - step + - status + additionalProperties: false + - type: object + properties: + step: + type: string + const: visit + status: + type: string + const: skipped + required: + - step + - status + additionalProperties: false + - type: object + properties: + step: + type: string + const: units + status: + type: string + const: skipped + required: + - step + - status + additionalProperties: false + - type: object + properties: + step: + type: string + const: first-result + status: + type: string + const: skipped + required: + - step + - status + additionalProperties: false + description: "One step, answered or deliberately passed. The union has an arm per step id plus a skip arm for the six + steps that may be passed — `who` is the one required question, and `confirm` / `done` are acknowledgements + rather than questions, so none of the three has a skip arm. Every arm is strict: an unknown key is a 422, and a + skip may never carry an answer." + OnboardingRestartRequest: + type: object + properties: {} + additionalProperties: false + description: No input beyond the session. Send `{}`. OnboardingStepRequest: type: object properties: @@ -34603,6 +35010,62 @@ components: - configChangedAt - computedAt additionalProperties: {} + onboardingRecord: + default: null + anyOf: + - type: object + properties: + id: + type: string + minLength: 1 + needs: + default: {} + type: object + propertyNames: + type: string + additionalProperties: {} + steps: + default: [] + type: array + items: + type: object + propertyNames: + type: string + additionalProperties: {} + firstResult: + default: null + anyOf: + - type: object + propertyNames: + type: string + additionalProperties: {} + - type: "null" + modulesDerivedAt: + default: null + anyOf: + - type: string + minLength: 1 + - type: "null" + completedAt: + default: null + anyOf: + - type: string + minLength: 1 + - type: "null" + createdAt: + type: string + minLength: 1 + updatedAt: + type: string + minLength: 1 + required: + - needs + - steps + - firstResult + - modulesDerivedAt + - completedAt + additionalProperties: {} + - type: "null" practitioners: default: [] type: array @@ -35941,6 +36404,7 @@ components: - correlationPatterns - intradayProfiles - healthScoreRecords + - onboardingRecord - practitioners - encounters - encounterDocumentLinks @@ -62667,6 +63131,209 @@ components: - data - error additionalProperties: false + OnboardingStateEnvelope: + type: object + properties: + data: + type: object + properties: + onboarding: + $ref: "#/components/schemas/OnboardingState" + required: + - onboarding + additionalProperties: false + error: + type: "null" + meta: + type: object + properties: + requestId: + type: string + additionalProperties: false + required: + - data + - error + additionalProperties: false + OnboardingState: + type: object + properties: + steps: + type: array + items: + $ref: "#/components/schemas/OnboardingStepState" + needs: + $ref: "#/components/schemas/OnboardingNeeds" + completedAt: + anyOf: + - type: string + format: date-time + pattern: ^(?:(?:\d\d[2468][048]|\d\d[13579][26]|\d\d0[48]|[02468][048]00|[13579][26]00)-02-29|\d{4}-(?:(?:0[13578]|1[02])-(?:0[1-9]|[12]\d|3[01])|(?:0[469]|11)-(?:0[1-9]|[12]\d|30)|(?:02)-(?:0[1-9]|1\d|2[0-8])))T(?:(?:[01]\d|2[0-3]):[0-5]\d(?::[0-5]\d(?:\.\d+)?)?(?:Z|([+-](?:[01]\d|2[0-3]):[0-5]\d)))$ + - type: "null" + description: "When the needs flow was completed. Distinct from the account payload's `onboardingCompletedAt`, which + stays the first-run redirect's gate: `POST /api/onboarding/restart` clears this one and leaves that one + alone." + firstResult: + anyOf: + - $ref: "#/components/schemas/OnboardingFirstResult" + - type: "null" + required: + - steps + - needs + - completedAt + - firstResult + additionalProperties: false + description: The needs-based setup flow for one record. Always present on the account payload — a record that never + entered the flow reads as empty answers with nine `pending` steps, so a client branches on the step statuses + rather than on the field's existence. + OnboardingStepState: + type: object + properties: + id: + type: string + enum: + - who + - areas + - medication + - sources + - visit + - units + - confirm + - first-result + - done + status: + type: string + enum: + - pending + - done + - skipped + required: + - id + - status + additionalProperties: false + description: One step of the setup flow. The ids are stable; a client that meets one it does not know SKIPS it rather + than refusing the list, so the flow may grow a screen without a client release. + OnboardingNeeds: + type: object + properties: + recordTarget: + anyOf: + - type: string + enum: + - me + - someone-else + - both + - type: "null" + description: Q1 — whose record this is. Null until answered. + areas: + type: array + items: + type: string + enum: + - blood-pressure + - weight-body + - glucose + - sleep + - mood + - cycle + - activity + - labs + - illness + description: Q2 — the areas to keep an eye on. Each one maps to at least one module through `ONBOARDING_AREA_MODULES`; + the map is the server's, and a client must not re-derive it. + medication: + anyOf: + - type: string + enum: + - yes + - no + - sometimes + - type: "null" + description: Q3 — medication on a schedule. + sources: + type: array + items: + type: string + enum: + - manual + - apple-health + - withings + - oura + - whoop + - polar + - fitbit + - strava + - nightscout + - file + description: Q4 — where the readings come from today. + visit: + anyOf: + - type: string + enum: + - within-a-month + - later + - no + - type: "null" + description: Q5 — a doctor's visit coming up. + units: + type: object + properties: + glucoseUnit: + anyOf: + - type: string + enum: + - mg/dL + - mmol/L + - type: "null" + unitPreference: + anyOf: + - type: string + enum: + - metric + - imperial + - type: "null" + required: + - glucoseUnit + - unitPreference + additionalProperties: false + description: "Q6 — the unit answers as given. NOT the source of truth for display: `glucoseUnit` and `unitPreference` on + the account payload are, and the answers step writes them. This records what the flow was told." + required: + - recordTarget + - areas + - medication + - sources + - visit + - units + additionalProperties: false + description: The setup answers as given. Every field is null or empty until its question is answered. + OnboardingFirstResult: + type: object + properties: + task: + type: string + enum: + - connect-source + - add-medication + - log-reading + target: + anyOf: + - type: string + - type: "null" + description: What the task is about — a source key for a connection, an area key for a reading. Null for the medication + task. + completedAt: + anyOf: + - type: string + format: date-time + pattern: ^(?:(?:\d\d[2468][048]|\d\d[13579][26]|\d\d0[48]|[02468][048]00|[13579][26]00)-02-29|\d{4}-(?:(?:0[13578]|1[02])-(?:0[1-9]|[12]\d|3[01])|(?:0[469]|11)-(?:0[1-9]|[12]\d|30)|(?:02)-(?:0[1-9]|1\d|2[0-8])))T(?:(?:[01]\d|2[0-3]):[0-5]\d(?::[0-5]\d(?:\.\d+)?)?(?:Z|([+-](?:[01]\d|2[0-3]):[0-5]\d)))$ + - type: "null" + description: When the offered task produced its result, or null. + required: + - task + - target + - completedAt + additionalProperties: false + description: The one task the flow offered at the end, and whether it produced its result. OnboardingStepEnvelope: type: object properties: @@ -62712,6 +63379,8 @@ components: completed: type: boolean const: true + onboarding: + $ref: "#/components/schemas/OnboardingState" required: - completed additionalProperties: false @@ -66362,6 +67031,8 @@ components: required: - medication additionalProperties: {} + onboarding: + $ref: "#/components/schemas/OnboardingState" required: - id - username @@ -66369,6 +67040,7 @@ components: - moduleAccess - recordSession - notificationPrefs + - onboarding additionalProperties: {} description: "The signed-in account: its identity, its preferences, and (since v1.36.0) what account sharing lets it do. Additional properties are the preference fields this spec does not yet enumerate. Under an active switch the diff --git a/src/app/api/auth/me/route.ts b/src/app/api/auth/me/route.ts index c6a64acbe..69a283bb9 100644 --- a/src/app/api/auth/me/route.ts +++ b/src/app/api/auth/me/route.ts @@ -54,6 +54,7 @@ import { resolveModuleMap, getOperatorModuleAvailability, } from "@/lib/modules/gate"; +import { loadOnboardingState } from "@/lib/onboarding/needs-store"; import { parseTourProgress } from "@/lib/onboarding/tour-progress"; import { parseNotificationPrefs } from "@/lib/validations/notification-prefs"; import { resolveAccountAccess } from "@/lib/sharing/account-access"; @@ -107,35 +108,50 @@ export const GET = apiHandler(async () => { // no switch at all, and the reason the own-record payload is untouched. const sections = accountAccess.active?.sections ?? null; - const [record, cycleProfile, resolvedModules, moduleAvailability] = - await Promise.all([ - // The actor's own row is already in hand; a switched session needs the - // record's `gender`, which is the column the cycle gate derives from. - recordId === user.id - ? Promise.resolve({ gender: user.gender }) - : prisma.user.findUnique({ - where: { id: recordId }, - select: { gender: true }, - }), - prisma.cycleProfile.findUnique({ - where: { userId: recordId }, - select: { cycleTrackingEnabled: true }, - }), - // v1.18.0 — resolved module enable/disable map for every toggleable - // module. cycle/coach reflect their real delegated state (the cycle - // gate / disableCoach + operator assistant flag); the rest read the - // disabled-allowlist `modulePreferencesJson`. Default-on. Clients - // hide a whole module surface end-to-end when its key is `false`. - resolveModuleMap(recordId), - // v1.18.0 — operator-layer availability map (server-wide kill-switch). - // The resolved `modules` map above already AND-s this in, so it cannot - // distinguish operator-off from user-off. The Modules hub needs that - // distinction to render an operator-disabled module as a read-only - // "disabled server-wide" row instead of a live toggle that no-ops. - // Server-WIDE, so it is the same map for every record and is not - // re-scoped here. - getOperatorModuleAvailability(), - ]); + const [ + record, + cycleProfile, + resolvedModules, + moduleAvailability, + onboarding, + ] = await Promise.all([ + // The actor's own row is already in hand; a switched session needs the + // record's `gender`, which is the column the cycle gate derives from. + recordId === user.id + ? Promise.resolve({ gender: user.gender }) + : prisma.user.findUnique({ + where: { id: recordId }, + select: { gender: true }, + }), + prisma.cycleProfile.findUnique({ + where: { userId: recordId }, + select: { cycleTrackingEnabled: true }, + }), + // v1.18.0 — resolved module enable/disable map for every toggleable + // module. cycle/coach reflect their real delegated state (the cycle + // gate / disableCoach + operator assistant flag); the rest read the + // disabled-allowlist `modulePreferencesJson`. Default-on. Clients + // hide a whole module surface end-to-end when its key is `false`. + resolveModuleMap(recordId), + // v1.18.0 — operator-layer availability map (server-wide kill-switch). + // The resolved `modules` map above already AND-s this in, so it cannot + // distinguish operator-off from user-off. The Modules hub needs that + // distinction to render an operator-disabled module as a read-only + // "disabled server-wide" row instead of a live toggle that no-ops. + // Server-WIDE, so it is the same map for every record and is not + // re-scoped here. + getOperatorModuleAvailability(), + // v1.39 (C1) — the needs-based setup state, for the ACTOR's own record + // rather than the active one. The two module fields above answer for the + // record because they say what that record tracks; this one is resolved + // the other way because the three write routes are actor-only + // (`requireAuth()` refuses outright under a switch), and publishing a + // setup state the caller cannot write would be a read the browser could + // act on and never complete. A guardian's route into a managed record's + // flow arrives with the rest of that record's configuration (#939 / C2); + // the storage is already keyed by record for it. + loadOnboardingState(prisma, user.id), + ]); // The masking step, and the one place the record scoping is narrowed rather // than resolved. `active` is set for ANY live grant — a READ grant scoped to @@ -186,6 +202,19 @@ export const GET = apiHandler(async () => { timezone: user.timezone, onboardingCompletedAt: user.onboardingCompletedAt, onboardingTourCompleted: user.onboardingTourCompleted, + // v1.39 (C1) — the needs-based setup flow, published additively: + // `steps` (the nine stable ids and their `pending | done | skipped` + // status), `needs` (the answers as given), `completedAt` (the flow's OWN + // completion, distinct from `onboardingCompletedAt` above, which keeps its + // meaning as the first-run redirect's gate) and `firstResult` (the one + // task the flow offered, and whether it produced its result). + // + // Always present, never null: a record that never entered the flow reads + // as empty answers with nine `pending` steps, so a client branches on the + // step statuses rather than on the field's existence. A client that meets + // a step id it does not know skips it — the list is the contract, not the + // client's own copy of it. + onboarding, // v1.18.6 (DISC-02) — one-time medical-disclaimer acknowledgment. Null // when never acknowledged; the onboarding welcome gate reads this to // decide whether to require the acknowledgment before "Get started". diff --git a/src/components/onboarding/getting-started-checklist.tsx b/src/components/onboarding/getting-started-checklist.tsx index 9202fad0f..a4eb66278 100644 --- a/src/components/onboarding/getting-started-checklist.tsx +++ b/src/components/onboarding/getting-started-checklist.tsx @@ -312,11 +312,17 @@ export function GettingStartedChecklist() { notificationsConfigured, insightsConfigured, dismissedIds, + // v1.39 (C1) — the setup answers order these rows: medication first + // for somebody who said they take one daily, the data-source row first + // for somebody who named a wearable. Null until the account payload + // resolves, and for any record that never entered the flow. + onboarding: user?.onboarding ?? null, }), [ user?.heightCm, user?.dateOfBirth, user?.gender, + user?.onboarding, measurementCount, medicationCount, dataSourceConnected, @@ -334,6 +340,10 @@ export function GettingStartedChecklist() { measurementCount, dismissedAll, items, + // v1.39 (C1) — five readings are not evidence the setup finished. While a + // step is still pending, the answers were never confirmed, or the one task + // the flow offered never produced its result, the list stays. + onboarding: user?.onboarding ?? null, }); // v1.4.15 phase-A3 fix #3 — flicker guard. Until BOTH the auth user diff --git a/src/hooks/use-auth.ts b/src/hooks/use-auth.ts index c1f5058a4..aad63b656 100644 --- a/src/hooks/use-auth.ts +++ b/src/hooks/use-auth.ts @@ -23,6 +23,7 @@ import { isDateFormatPreference, storeDateFormat } from "@/lib/date-format"; import { storeTimezone } from "@/lib/timezone-mirror"; import type { ModuleKey } from "@/lib/modules/registry"; import type { ModuleAccessState } from "@/lib/sharing/module-disclosure"; +import type { OnboardingStateDto } from "@/lib/onboarding/needs"; import type { TourProgress } from "@/lib/onboarding/tour-progress"; import { clearOfflineCachesForSessionEnd } from "@/lib/pwa/query-persister"; import { @@ -81,6 +82,18 @@ export interface AuthUser { * `onboardingTourCompleted` boolean, which stays the auto-launch gate. */ onboardingTourProgress: AuthTourProgress | null; + /** + * v1.39 (C1) — the needs-based setup flow's state for this record: the nine + * ordered steps with their `pending | done | skipped` status, the answers as + * given, the flow's own completion stamp (distinct from + * `onboardingCompletedAt` above, which stays the first-run redirect's gate) + * and the one task the flow offered. + * + * Optional in the type so a stale /me payload — an older server image, or a + * test fixture written before the field existed — coerces to "no flow" in + * the mapper below rather than failing the shape. + */ + onboarding?: OnboardingStateDto | null; /** * v1.18.6 (DISC-02) — ISO timestamp of the one-time medical-disclaimer * acknowledgment, or null when never acknowledged. The onboarding welcome @@ -386,6 +399,10 @@ export async function fetchMe(): Promise { // v1.18.6 — coerce against a stale /me payload (older server image // without the field) to null so the tour starts from the top. onboardingTourProgress: data.onboardingTourProgress ?? null, + // v1.39 (C1) — coerce against a stale /me payload (an older server image + // without the field) to null, which every reader treats as "no flow was + // ever run" rather than as an unfinished one. + onboarding: data.onboarding ?? null, // v1.7.0 — coerce against a stale /me payload (older server image // without the field) so the display defaults to metric. unitPreference: data.unitPreference === "imperial" ? "imperial" : "metric", diff --git a/src/lib/onboarding/__tests__/checklist.test.ts b/src/lib/onboarding/__tests__/checklist.test.ts index dc734d5d3..c765877f8 100644 --- a/src/lib/onboarding/__tests__/checklist.test.ts +++ b/src/lib/onboarding/__tests__/checklist.test.ts @@ -16,7 +16,9 @@ const completeProfile = { gender: "MALE", }; -function inputs(overrides: Partial[0]> = {}) { +function inputs( + overrides: Partial[0]> = {}, +): Parameters[0] { return { profile: completeProfile, measurementCount: 0, @@ -25,6 +27,7 @@ function inputs(overrides: Partial[0]> = {}) { notificationsConfigured: false, insightsConfigured: false, dismissedIds: new Set(), + onboarding: null, ...overrides, }; } @@ -145,6 +148,7 @@ describe("visibleChecklist + checklistProgress", () => { it("returns 0% when nothing done", () => { const items = buildChecklist({ profile: { heightCm: null, dateOfBirth: null, gender: null }, + onboarding: null, measurementCount: 0, medicationCount: 0, dataSourceConnected: false, diff --git a/src/lib/onboarding/checklist.ts b/src/lib/onboarding/checklist.ts index a4a828b8c..9ec9e4496 100644 --- a/src/lib/onboarding/checklist.ts +++ b/src/lib/onboarding/checklist.ts @@ -6,6 +6,12 @@ * status + dismissed-ids set) and renders against the result. */ +import { + isOnboardingSettled, + type OnboardingNeeds, + type OnboardingStateDto, +} from "./needs"; + export const CHECKLIST_ITEM_IDS = [ "profile", "measurement", @@ -64,6 +70,55 @@ export interface ChecklistInputs { insightsConfigured: boolean; /** Dismissed item ids (per-item localStorage state). */ dismissedIds: ReadonlySet; + /** + * v1.39 (C1) — the needs-based setup state from `GET /api/auth/me`, or null + * for a record that never entered the flow. + * + * Required rather than optional: the ordering below is the whole reason the + * answers are published, and a caller that stops passing them should fail to + * compile rather than quietly fall back to the fixed order. + */ + onboarding: OnboardingStateDto | null; +} + +/** + * v1.39 (C1) — the order the answers imply. + * + * The rows themselves are fixed; what the questionnaire decides is which of + * them a person sees first. Someone who said they take medication daily should + * not have to scroll past "connect a data source" to find it, and someone who + * named a wearable should meet that row before the manual-entry one. + * + * `profile` stays first whatever the answers say — it is the identity the + * clinical surfaces derive from, not a domain choice — and any row the answers + * do not speak to keeps its position relative to the others. The result is a + * reordering, never a removal: nothing here drops a row. + */ +export function checklistOrderFromNeeds( + needs: OnboardingNeeds, +): ChecklistItemId[] { + const promoted: ChecklistItemId[] = []; + const promote = (id: ChecklistItemId) => { + if (!promoted.includes(id)) promoted.push(id); + }; + + if (needs.medication === "yes" || needs.medication === "sometimes") { + promote("medication"); + } + // "I type them in" and "a file I already have" are not connections, so they + // say nothing about the data-source row. + if (needs.sources.some((s) => s !== "manual" && s !== "file")) { + promote("dataSource"); + } + if (needs.areas.length > 0) promote("measurement"); + + return [ + "profile", + ...promoted, + ...CHECKLIST_ITEM_IDS.filter( + (id) => id !== "profile" && !promoted.includes(id), + ), + ]; } /** @@ -112,7 +167,15 @@ export function buildChecklist(inputs: ChecklistInputs): ChecklistItem[] { dismissed: inputs.dismissedIds.has("insights"), }, ]; - return items; + if (!inputs.onboarding) return items; + // Ordered from the answers only once the flow has been confirmed: before + // that the answers are still being given, and re-ordering the dashboard + // under someone mid-question would be movement they did not ask for. + if (inputs.onboarding.completedAt === null) return items; + const order = checklistOrderFromNeeds(inputs.onboarding.needs); + return order + .map((id) => items.find((item) => item.id === id)) + .filter((item): item is ChecklistItem => item !== undefined); } /** @@ -157,10 +220,23 @@ export function shouldShowChecklist(args: { measurementCount: number; 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. + */ + onboarding?: OnboardingStateDto | null; }): boolean { if (args.dismissedAll) return false; const stillInSetup = - args.onboardingCompletedAt == null || args.measurementCount < 5; + args.onboardingCompletedAt == null || + args.measurementCount < 5 || + !isOnboardingSettled(args.onboarding); if (!stillInSetup) return false; const visible = visibleChecklist(args.items); if (visible.length === 0) return false; diff --git a/src/lib/openapi/routes/account-sharing.ts b/src/lib/openapi/routes/account-sharing.ts index 25a30a8b5..fbfea85c5 100644 --- a/src/lib/openapi/routes/account-sharing.ts +++ b/src/lib/openapi/routes/account-sharing.ts @@ -34,6 +34,7 @@ import { inviteManagedProfileGuardianSchema, updateManagedProfileSchema, } from "@/lib/validations/managed-profiles"; +import { onboardingStateResource } from "./onboarding"; import { moduleAccessMap } from "./profile"; import { dataEnvelope, errorEnvelope, stdResponses } from "./shared"; @@ -462,6 +463,14 @@ const accountPayload = z notificationPrefs: z.looseObject({ medication: z.looseObject({ clientManaged: z.boolean() }), }), + // v1.39 (C1) — enumerated for the same reason `notificationPrefs` is: a + // client DECIDES on it. The getting-started checklist orders its rows from + // `needs` and stays on screen while the flow is unfinished, and a future + // native client renders the steps it knows. Always present, resolved for + // the CALLER's own record rather than the active one — the three write + // routes are actor-only, and publishing a setup state the caller cannot + // write would be a read a browser could act on and never complete. + onboarding: onboardingStateResource, }) .meta({ id: "AccountPayload", From 3f44b882acbfd1ec52751e65ea45871f70a94b49 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 10 Sep 2026 11:03:56 +0200 Subject: [PATCH 05/15] feat(backup): carry the setup answers out of the account and back in MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Nothing recomputes an answer. The module map they produced survives on the account row, but the map says what is switched on, not that the person told the flow they take medication daily and have a visit next month — so a restore without the row opens a blank questionnaire for somebody who already answered it, and leaves the checklist ordering itself from nothing. Both ends live in one file, the arrangement the visits and the score already use, because a reader who greps only the restore route gets a false negative when the route delegates. `modulesDerivedAt` is the field that makes this more than history and it is asserted rather than defaulted: it is the once-only latch on the module derivation, so a restore that let it default to null would hand the next confirm permission to re-apply the questionnaire over every module decision taken since, silently. The round trip seeds it non-null and reads it back. The file's schema validates the three answer objects loosely and the restore narrows them through the same fail-soft readers the account payload uses. Being stricter there would refuse a whole account's restore over one retired chip. The wipe takes the row for the reason the reset already takes the wizard's own columns: the record the answers were about is gone, so the answers describe nothing. --- .../[id]/restore/__tests__/round-trip.test.ts | 4 + .../api/admin/backups/[id]/restore/route.ts | 11 + .../export/__tests__/per-type-routes.test.ts | 1 + .../export/encrypted/__tests__/route.test.ts | 1 + src/lib/data-wipe/wipe-plan.ts | 6 + .../__tests__/full-backup-payload.test.ts | 1 + src/lib/export/backup-plan.ts | 22 ++ src/lib/export/full-backup-payload.ts | 17 ++ src/lib/export/onboarding-backup.ts | 199 ++++++++++++++++++ src/lib/validations/backup.ts | 34 +++ tests/integration/backup-round-trip.test.ts | 80 +++++++ 11 files changed, 376 insertions(+) create mode 100644 src/lib/export/onboarding-backup.ts diff --git a/src/app/api/admin/backups/[id]/restore/__tests__/round-trip.test.ts b/src/app/api/admin/backups/[id]/restore/__tests__/round-trip.test.ts index 224e1148d..c34397817 100644 --- a/src/app/api/admin/backups/[id]/restore/__tests__/round-trip.test.ts +++ b/src/app/api/admin/backups/[id]/restore/__tests__/round-trip.test.ts @@ -259,6 +259,10 @@ function sourceClient() { }, ]), }, + // The setup answers. Absent for this fixture's account, which is the + // ordinary case: the export has to answer for a record that never entered + // the flow without inventing one. + onboardingRecord: { findUnique: vi.fn().mockResolvedValue(null) }, // The score as it was SHOWN on a local day. Not recomputable: today's // number always comes from today's rows, so once the readings behind this // day moved, nothing can reproduce what it said. diff --git a/src/app/api/admin/backups/[id]/restore/route.ts b/src/app/api/admin/backups/[id]/restore/route.ts index 409a5b345..2a2049d47 100644 --- a/src/app/api/admin/backups/[id]/restore/route.ts +++ b/src/app/api/admin/backups/[id]/restore/route.ts @@ -57,6 +57,7 @@ import { import { restoreProfileData } from "@/lib/export/profile-backup"; import { restoreIntradayProfileData } from "@/lib/export/intraday-profile-backup"; import { restoreHealthScoreData } from "@/lib/export/health-score-backup"; +import { restoreOnboardingData } from "@/lib/export/onboarding-backup"; import { restoreVisitsData } from "@/lib/export/visits-backup"; import { restoreVaccinationsData } from "@/lib/export/vaccinations-backup"; import { restoreSensitiveData } from "@/lib/export/sensitive-backup"; @@ -111,6 +112,7 @@ interface RestoreResponse { correlationPatterns: number; intradayProfiles: number; healthScoreRecords: number; + onboardingRecords: number; practitioners: number; encounters: number; encounterLinks: number; @@ -1377,6 +1379,14 @@ const handler = apiHandler( payload, ); + // The needs-based setup answers. Both ends live in + // `src/lib/export/onboarding-backup.ts`, same as the three above. + const onboardingCleared = await restoreOnboardingData( + tx, + ownerId, + payload, + ); + const biomarkerByName = new Map(); const restoredBiomarkerIds = new Set(); for (const biomarker of payload.biomarkers) { @@ -1979,6 +1989,7 @@ const handler = apiHandler( correlationPatterns: profileCleared.correlationPatterns, intradayProfiles: intradayCleared.intradayProfiles, healthScoreRecords: healthScoreCleared.healthScoreRecords, + onboardingRecords: onboardingCleared.onboardingRecords, practitioners: visitsCleared.practitioners, encounters: visitsCleared.encounters, encounterLinks: visitsCleared.encounterLinks, diff --git a/src/app/api/export/__tests__/per-type-routes.test.ts b/src/app/api/export/__tests__/per-type-routes.test.ts index f3ca60494..4f6ea8736 100644 --- a/src/app/api/export/__tests__/per-type-routes.test.ts +++ b/src/app/api/export/__tests__/per-type-routes.test.ts @@ -28,6 +28,7 @@ vi.mock("@/lib/db", () => ({ // `buildIntradayProfileBackupSection`. intradayCumulativeProfile: { findMany: vi.fn().mockResolvedValue([]) }, healthScoreRecord: { findMany: vi.fn().mockResolvedValue([]) }, + onboardingRecord: { findUnique: vi.fn().mockResolvedValue(null) }, // The visit tables. Empty here: this file asserts the sections it seeds, // and an account with no visits is the ordinary case the builder must // still answer for. diff --git a/src/app/api/export/encrypted/__tests__/route.test.ts b/src/app/api/export/encrypted/__tests__/route.test.ts index f24aef437..d98b3ebf5 100644 --- a/src/app/api/export/encrypted/__tests__/route.test.ts +++ b/src/app/api/export/encrypted/__tests__/route.test.ts @@ -26,6 +26,7 @@ vi.mock("@/lib/db", () => ({ // `buildIntradayProfileBackupSection`. intradayCumulativeProfile: { findMany: vi.fn().mockResolvedValue([]) }, healthScoreRecord: { findMany: vi.fn().mockResolvedValue([]) }, + onboardingRecord: { findUnique: vi.fn().mockResolvedValue(null) }, // The visit tables. Empty here: this file asserts the sections it seeds, // and an account with no visits is the ordinary case the builder must // still answer for. diff --git a/src/lib/data-wipe/wipe-plan.ts b/src/lib/data-wipe/wipe-plan.ts index 43fab3294..3d71f5cab 100644 --- a/src/lib/data-wipe/wipe-plan.ts +++ b/src/lib/data-wipe/wipe-plan.ts @@ -213,6 +213,12 @@ export const WIPE_MODELS = [ "AuditLog", "ConsentReceipt", "UserKnownDevice", + // v1.39 — the setup answers and the step ledger. The USER_RESET block below + // already puts the wizard of today back to the start; this is the same + // decision for the needs-based flow, and it belongs on the same side of the + // line for the same reason: the record the answers were about is gone, so + // the answers describe nothing. + "OnboardingRecord", // ── Parents, last ─────────────────────────────────────────────────────── "Medication", diff --git a/src/lib/export/__tests__/full-backup-payload.test.ts b/src/lib/export/__tests__/full-backup-payload.test.ts index 99d035567..93626363b 100644 --- a/src/lib/export/__tests__/full-backup-payload.test.ts +++ b/src/lib/export/__tests__/full-backup-payload.test.ts @@ -373,6 +373,7 @@ function makePrisma() { ]), }, healthScoreRecord: { findMany: vi.fn().mockResolvedValue([]) }, + onboardingRecord: { findUnique: vi.fn().mockResolvedValue(null) }, // The visit tables. Empty here: this file asserts the sections it seeds, // and an account with no visits is the ordinary case the builder must // still answer for. diff --git a/src/lib/export/backup-plan.ts b/src/lib/export/backup-plan.ts index 5f47cadf2..8ed0c9065 100644 --- a/src/lib/export/backup-plan.ts +++ b/src/lib/export/backup-plan.ts @@ -176,6 +176,12 @@ export const BACKED_UP_MODELS = [ // for a restored preference to attach to. "ReminderPhaseConfig", "ConsentReceipt", + // The setup answers. Nothing recomputes them: the module map they produced + // survives on the `User` row, but the map says what is switched on, not that + // the person said they take medication daily and have a visit next month. A + // restore without the row opens a blank questionnaire for somebody who has + // already answered it. + "OnboardingRecord", ] as const; /** @@ -236,6 +242,9 @@ export const BACKUP_WRITER_FILES: readonly string[] = [ // the reason the visits comment gives. "src/lib/export/awards-backup.ts", "src/lib/export/environment-backup.ts", + // The needs-based setup answers, both ends beside each other for the reason + // the visits comment gives. + "src/lib/export/onboarding-backup.ts", // The ECG strips, same arrangement again. The recording reads through its // OWN delegate here rather than riding the measurement it references, // because that reference is a pointer between two independently carried @@ -257,6 +266,7 @@ export const BACKUP_RESTORE_FILES: readonly string[] = [ "src/lib/export/document-filing-backup.ts", "src/lib/export/awards-backup.ts", "src/lib/export/environment-backup.ts", + "src/lib/export/onboarding-backup.ts", "src/lib/export/ecg-backup.ts", "src/lib/cycle/backup.ts", ]; @@ -495,6 +505,18 @@ export const TWO_ENDED_MODELS = [ // theoretical. The reference is resolved against what the restore wrote, // nulled when it cannot be, and reported either way. "EcgRecording", + // The setup answers, carried from the release that introduces them. There is + // no debt entry to write instead: the row is one small object with no + // reference to resolve, so "carried" and "owed" would have cost the same + // afternoon and only one of them returns a record that can say "set up + // again" and mean it. + // + // `modulesDerivedAt` is the field that makes this more than history. It is + // the once-only latch on the module derivation, so a restore that let it + // default to null would hand the next confirm permission to re-apply the + // questionnaire over every module decision taken in Settings since. It is + // carried and asserted, not defaulted. + "OnboardingRecord", ] as const; /** One model claimed to travel both ways. */ diff --git a/src/lib/export/full-backup-payload.ts b/src/lib/export/full-backup-payload.ts index 9aa30beb9..89f63c045 100644 --- a/src/lib/export/full-backup-payload.ts +++ b/src/lib/export/full-backup-payload.ts @@ -112,6 +112,12 @@ import { type IntradayProfileBackupCounts, type IntradayProfileBackupSection, } from "@/lib/export/intraday-profile-backup"; +import { + buildOnboardingBackupSection, + countOnboardingBackupSection, + type OnboardingBackupCounts, + type OnboardingBackupSection, +} from "@/lib/export/onboarding-backup"; export interface FullBackupCounts extends @@ -128,6 +134,7 @@ export interface FullBackupCounts DocumentFilingBackupCounts, AwardsBackupCounts, EnvironmentBackupCounts, + OnboardingBackupCounts, EcgBackupCounts { measurements: number; medications: number; @@ -696,6 +703,7 @@ export async function buildFullBackupPayload( documentFiling, awards, environment, + onboardingRecord, ecg, nutrientDays, ] = await Promise.all([ @@ -860,6 +868,12 @@ export async function buildFullBackupPayload( // `src/lib/export/environment-backup.ts`, and the purpose is absent for // the same reason as the awards above. buildEnvironmentBackupSection(prisma, userId), + // The needs-based setup answers. One small row, and the only copy of what + // the person told the flow. Both ends live in + // `src/lib/export/onboarding-backup.ts` beside each other. + buildOnboardingBackupSection(prisma, userId, { + purpose: disasterRecovery ? "disaster-recovery" : "portable-export", + }), // The ECG strips, their rhythm verdicts and their traces. Both ends live // in `src/lib/export/ecg-backup.ts` beside each other, the same // arrangement as the sections above and for the same reason. @@ -892,6 +906,7 @@ export async function buildFullBackupPayload( const profileSection: ProfileBackupSection = profile; const intradaySection: IntradayProfileBackupSection = intradayProfiles; const healthScoreSection: HealthScoreBackupSection = healthScoreRecords; + const onboardingSection: OnboardingBackupSection = onboardingRecord; const visitsSection: VisitsBackupSection = visits; const vaccinationsSection: VaccinationsBackupSection = vaccinations; const remindersSection: RemindersBackupSection = reminders; @@ -1238,6 +1253,7 @@ export async function buildFullBackupPayload( ...profileSection, ...intradaySection, ...healthScoreSection, + ...onboardingSection, ...visitsSection, ...vaccinationsSection, ...remindersSection, @@ -1306,6 +1322,7 @@ export async function buildFullBackupPayload( ...countProfileBackupSection(profile), ...countIntradayProfileBackupSection(intradayProfiles), ...countHealthScoreBackupSection(healthScoreRecords), + ...countOnboardingBackupSection(onboardingRecord), ...countVisitsBackupSection(visits), ...countVaccinationsBackupSection(vaccinations), ...countRemindersBackupSection(reminders), diff --git a/src/lib/export/onboarding-backup.ts b/src/lib/export/onboarding-backup.ts new file mode 100644 index 000000000..48c3e31af --- /dev/null +++ b/src/lib/export/onboarding-backup.ts @@ -0,0 +1,199 @@ +/** + * The needs-based setup answers, with both backup ends in one file. + * + * The builder and the restore live together deliberately, the way + * `health-score-backup.ts` and `src/lib/cycle/backup.ts` do: a reader asking + * "is this carried at both ends?" answers it here, and a reader who greps only + * the restore ROUTE gets a false negative, because the route delegates. + * + * Why it is `BACKED_UP` rather than `DERIVED`. Nothing recomputes an answer. + * The module map the answers produced survives on the `User` row, but the map + * is not the answers: it says what is switched on, not that the person said + * they take medication daily and have a visit next month. Losing the row would + * make "Set up again" open a blank questionnaire for someone who already + * answered it, and would leave the getting-started checklist ordering itself + * from nothing. It is one small row and it is the only copy. + * + * Nothing here is encrypted, so there is no portable / disaster-recovery split + * on the values: the three JSON columns hold closed enum vocabularies rather + * than free text. The only difference between the two purposes is identity — a + * canonical disaster-recovery payload carries the row id and its timestamps so + * the row comes back as itself. + */ +import type { Prisma, PrismaClient } from "@/generated/prisma/client"; + +import { + parseOnboardingFirstResult, + parseOnboardingNeeds, + parseOnboardingSteps, + type OnboardingFirstResult, + type OnboardingNeeds, + type OnboardingStepState, +} from "@/lib/onboarding/needs"; + +export interface OnboardingBackupOptions { + purpose?: "portable-export" | "disaster-recovery"; +} + +/** The record's setup state, exactly as it was answered. */ +export interface OnboardingBackupEntry { + /** Present in canonical DR payloads so the row keeps a stable identity. */ + id?: string; + /** + * Read through the same fail-soft parsers the account payload uses rather + * than copied out of the column raw. A blob written by a release that named + * an answer this one retired therefore reaches the file as the answers this + * release can honour, and a restore cannot re-import a value its own + * validation would refuse. + */ + needs: OnboardingNeeds; + steps: OnboardingStepState[]; + firstResult: OnboardingFirstResult | null; + /** + * When the module map was derived. Carried, and this is the field that makes + * the row worth carrying rather than defaulting: a restore that dropped it + * would let the next confirm re-derive the whole map over whatever the + * account has changed in Settings since. + */ + modulesDerivedAt: string | null; + completedAt: string | null; + createdAt?: string; + updatedAt?: string; +} + +export interface OnboardingBackupSection { + onboardingRecord: OnboardingBackupEntry | null; +} + +export interface OnboardingBackupCounts { + onboardingRecords: number; +} + +/** + * Build the setup-answers slice of a user's full backup. + * + * Takes the delegate it uses rather than a whole `PrismaClient`, matching the + * other section builders, so the route's global client and the worker's local + * one share one read. + */ +export async function buildOnboardingBackupSection( + prisma: Pick, + userId: string, + options: OnboardingBackupOptions = {}, +): Promise { + const disasterRecovery = options.purpose === "disaster-recovery"; + + const row = await prisma.onboardingRecord.findUnique({ where: { userId } }); + if (!row) return { onboardingRecord: null }; + + return { + onboardingRecord: { + ...(disasterRecovery + ? { + id: row.id, + createdAt: row.createdAt.toISOString(), + updatedAt: row.updatedAt.toISOString(), + } + : {}), + needs: parseOnboardingNeeds(row.needsJson), + steps: parseOnboardingSteps(row.stepsJson), + firstResult: parseOnboardingFirstResult(row.firstResultJson), + modulesDerivedAt: row.modulesDerivedAt?.toISOString() ?? null, + completedAt: row.completedAt?.toISOString() ?? null, + }, + }; +} + +/** Row counts for the audit trail, mirroring the other section counters. */ +export function countOnboardingBackupSection( + section: OnboardingBackupSection, +): OnboardingBackupCounts { + return { onboardingRecords: section.onboardingRecord ? 1 : 0 }; +} + +/** Counts the setup-state restore wiped, for the audit trail. */ +export interface OnboardingRestoreCleared { + onboardingRecords: number; +} + +/** + * One entry as the parsed FILE carries it. + * + * The three answer objects arrive as opaque JSON rather than as the typed + * shapes the builder wrote, and that is deliberate: the file's Zod schema + * validates them loosely so a value this release retired cannot refuse a whole + * account's restore, and the narrowing happens below, through the same + * fail-soft parsers the account payload reads with. A file hand-edited to name + * an area that no longer exists therefore restores without it, rather than + * either failing or writing a value nothing can read. + */ +export interface OnboardingRestoreEntry { + id?: string; + needs: unknown; + steps: unknown; + firstResult: unknown; + modulesDerivedAt: string | null; + completedAt: string | null; + createdAt?: string; + updatedAt?: string; +} + +/** + * The slice of a parsed backup this restore consumes. + * + * Required rather than optional: `BackupPayload` defaults it, so every caller + * already satisfies this, and a caller that stops satisfying it fails to + * compile instead of passing `undefined` into a branch that does nothing and + * reports success. + */ +export interface OnboardingRestoreInput { + onboardingRecord: OnboardingRestoreEntry | null; +} + +/** + * Re-create the account's setup state. + * + * Delete-then-recreate inside the caller's transaction, matching every other + * section. A file carrying no row leaves the account with none, which is the + * honest state for a record that never entered the flow — and the delete still + * runs, because the restore replaces the account's data rather than merging + * into it. + */ +export async function restoreOnboardingData( + tx: Prisma.TransactionClient, + ownerId: string, + payload: OnboardingRestoreInput, +): Promise { + const cleared = await tx.onboardingRecord.deleteMany({ + where: { userId: ownerId }, + }); + + const entry = payload.onboardingRecord; + if (entry) { + const firstResult = parseOnboardingFirstResult(entry.firstResult); + await tx.onboardingRecord.create({ + data: { + ...(entry.id ? { id: entry.id } : {}), + userId: ownerId, + needsJson: parseOnboardingNeeds( + entry.needs, + ) as unknown as Prisma.InputJsonValue, + stepsJson: parseOnboardingSteps( + entry.steps, + ) as unknown as Prisma.InputJsonValue, + firstResultJson: + firstResult === null + ? undefined + : (firstResult as unknown as Prisma.InputJsonValue), + modulesDerivedAt: entry.modulesDerivedAt + ? new Date(entry.modulesDerivedAt) + : null, + completedAt: entry.completedAt ? new Date(entry.completedAt) : null, + ...(entry.createdAt ? { createdAt: new Date(entry.createdAt) } : {}), + ...(entry.updatedAt ? { updatedAt: new Date(entry.updatedAt) } : {}), + }, + }); + } + + return { onboardingRecords: cleared.count }; +} diff --git a/src/lib/validations/backup.ts b/src/lib/validations/backup.ts index 9be51be04..d163e6e36 100644 --- a/src/lib/validations/backup.ts +++ b/src/lib/validations/backup.ts @@ -886,6 +886,33 @@ const healthScoreRecordBackupSchema = z }) .passthrough(); +/** + * v1.39 (C1) — the needs-based setup state. One row per record. + * + * The three answer objects are validated loosely on purpose: the closed + * vocabularies live in `src/lib/onboarding/needs.ts`, and the restore reads + * every one of them back through the same fail-soft parsers the account + * payload uses. Duplicating the enums here would give the file a second, + * quietly divergent opinion about what an answer is, and the failure mode of + * being stricter here is refusing a whole account's restore over one retired + * chip. + */ +const onboardingRecordBackupSchema = z + .object({ + id: z.string().min(1).optional(), + needs: z.record(z.string(), z.unknown()).default({}), + steps: z.array(z.record(z.string(), z.unknown())).default([]), + firstResult: z.record(z.string(), z.unknown()).nullable().default(null), + // The once-only latch on the module derivation. Carried rather than + // defaulted: a null here would hand the next confirm permission to + // re-apply the questionnaire over every module decision made since. + modulesDerivedAt: isoDateTime.nullable().default(null), + completedAt: isoDateTime.nullable().default(null), + createdAt: isoDateTime.optional(), + updatedAt: isoDateTime.optional(), + }) + .passthrough(); + const appSettingsBackupSchema = z .object({ id: z.string().min(1), @@ -1668,6 +1695,10 @@ export const backupPayloadSchema = z // the sections above: a file written before the table existed carries no // key, and an account whose score never resolved writes []. healthScoreRecords: z.array(healthScoreRecordBackupSchema).default([]), + // The needs-based setup answers. Defaulted to null for the same reason as + // the sections above: a file written before the table existed carries no + // key, and a record that never entered the flow has no row. + onboardingRecord: onboardingRecordBackupSchema.nullable().default(null), // Visits, the address book behind them, and the three link tables. // Defaulted for the same reason as the sections above: a file written // before the tables existed carries no key, and an account that has never @@ -1796,6 +1827,8 @@ export interface BackupSummary { intradayProfiles: number; /** Local days whose health score was written down as it was shown. */ healthScoreRecords: number; + /** 1 when the record's setup answers ride the file, 0 otherwise. */ + onboardingRecords: number; /** v1.37.19 (A6-8) — the visit address book. */ practitioners: number; /** v1.37.19 (A6-8) — doctor visits, planned and past. */ @@ -1884,6 +1917,7 @@ export function summarizeBackup(payload: BackupPayload): BackupSummary { correlationPatterns: payload.correlationPatterns.length, intradayProfiles: payload.intradayProfiles.length, healthScoreRecords: payload.healthScoreRecords.length, + onboardingRecords: payload.onboardingRecord ? 1 : 0, // v1.37.19 (A6-8) — the sections restored since 08-01 were written and // restored but absent from this report, so the admin's "what did I just // restore" answer silently under-counted a file that carried visits or diff --git a/tests/integration/backup-round-trip.test.ts b/tests/integration/backup-round-trip.test.ts index 7ecb17c1a..8376beb3e 100644 --- a/tests/integration/backup-round-trip.test.ts +++ b/tests/integration/backup-round-trip.test.ts @@ -239,6 +239,8 @@ const COUNT_BACK: Record< p.medicationEfficacyTarget.count({ where: { medication: { userId } } }), MedicationScheduleRevision: (p, userId) => p.medicationScheduleRevision.count({ where: { medication: { userId } } }), + OnboardingRecord: (p, userId) => + p.onboardingRecord.count({ where: { userId } }), }; /** @@ -743,6 +745,35 @@ async function seedEveryTwoEndedModel(prisma: PrismaClient): Promise { lastComputedAt: AT("2026-07-01T00:00:00.000Z"), }, }); + await prisma.onboardingRecord.create({ + data: { + userId: OWNER_ID, + needsJson: { + recordTarget: "me", + areas: ["blood-pressure", "sleep"], + medication: "yes", + sources: ["withings"], + visit: "within-a-month", + units: { glucoseUnit: "mmol/L", unitPreference: "metric" }, + }, + stepsJson: [ + { id: "who", status: "done" }, + { id: "areas", status: "done" }, + { id: "sources", status: "skipped" }, + ], + firstResultJson: { + task: "connect-source", + target: "withings", + completedAt: "2026-07-01T08:30:00.000Z", + }, + // The once-only latch on the module derivation. Seeded non-null so the + // assertion after the restore can show it came back rather than + // defaulting — a null here would hand the next confirm permission to + // re-apply the questionnaire over every module decision made since. + modulesDerivedAt: AT("2026-07-01T08:00:00.000Z"), + completedAt: AT("2026-07-01T08:00:00.000Z"), + }, + }); await prisma.healthScoreRecord.create({ data: { userId: OWNER_ID, @@ -1413,6 +1444,55 @@ describe("every model the plan claims two-ended survives a real restore", () => { pausedAt: "2026-07-15T08:00:00.000Z", resumedAt: null }, ]); + // The setup answers came back as answers, not as an empty questionnaire. + // + // Asserted column by column rather than by count: the row returning proves + // nothing about `modulesDerivedAt`, and that field is the once-only latch + // on the module derivation — a restore that let it default to null would + // hand the next confirm permission to re-apply the questionnaire over + // every module decision the account has taken since, silently. + const restoredOnboarding = await prisma.onboardingRecord.findUniqueOrThrow({ + where: { userId: OWNER_ID }, + }); + expect({ + needs: restoredOnboarding.needsJson, + steps: restoredOnboarding.stepsJson, + firstResult: restoredOnboarding.firstResultJson, + modulesDerivedAt: restoredOnboarding.modulesDerivedAt?.toISOString(), + completedAt: restoredOnboarding.completedAt?.toISOString(), + }).toEqual({ + needs: { + recordTarget: "me", + areas: ["blood-pressure", "sleep"], + medication: "yes", + sources: ["withings"], + visit: "within-a-month", + units: { glucoseUnit: "mmol/L", unitPreference: "metric" }, + }, + // The stored blob is a status LOOKUP, and the reader answers with the + // full ordered nine — a step the fixture never mentioned comes back + // `pending` rather than missing, which is what lets the flow grow a + // screen without a migration. + steps: [ + { id: "who", status: "done" }, + { id: "areas", status: "done" }, + { id: "medication", status: "pending" }, + { id: "sources", status: "skipped" }, + { id: "visit", status: "pending" }, + { id: "units", status: "pending" }, + { id: "confirm", status: "pending" }, + { id: "first-result", status: "pending" }, + { id: "done", status: "pending" }, + ], + firstResult: { + task: "connect-source", + target: "withings", + completedAt: "2026-07-01T08:30:00.000Z", + }, + modulesDerivedAt: "2026-07-01T08:00:00.000Z", + completedAt: "2026-07-01T08:00:00.000Z", + }); + // The Coach came back able to speak, and the fence held. // // `documentScoped` is asserted per thread rather than in aggregate: a From 3f015effecea5e673db1f79b50d1984eda66808a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 10 Sep 2026 11:17:00 +0200 Subject: [PATCH 06/15] test: prove the setup flow against a real database and a real grant MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four claims the unit suite cannot reach, because each is about what is in the database afterwards: the answers persist and a replay writes nothing new, the module derivation happens once and leaves a later Settings change alone, a restart resets the steps and not the modules, and a delegate with a live accepted grant and a switched session is refused on all three writes — with neither record acquiring a row, which is the quiet failure a fall-back to the caller's own record would produce. The demo guard gains the three paths and, beside them, the case that matters more: admitting one verb on a path must not open a sibling. --- .../proxy-demo-mode-allowlist.test.ts | 25 ++ .../complete/__tests__/route.test.ts | 22 +- tests/integration/onboarding-needs.test.ts | 418 ++++++++++++++++++ 3 files changed, 459 insertions(+), 6 deletions(-) create mode 100644 tests/integration/onboarding-needs.test.ts diff --git a/src/__tests__/proxy-demo-mode-allowlist.test.ts b/src/__tests__/proxy-demo-mode-allowlist.test.ts index 4badffb3a..bf43fe9d2 100644 --- a/src/__tests__/proxy-demo-mode-allowlist.test.ts +++ b/src/__tests__/proxy-demo-mode-allowlist.test.ts @@ -51,6 +51,31 @@ describe("proxy.ts DEMO_MODE mutation allowlist", () => { expect(res.status).not.toBe(403); }); + it("permits the three setup-flow writes so the demo can finish onboarding", () => { + // v1.39 (C1) — the design spec requires the demo to complete the same flow + // with preset answers, and none of the three creates health data: closed + // enum answers, the step ledger, and the module map derived from them. + for (const [path, method] of [ + ["/api/onboarding/answers", "PATCH"], + ["/api/onboarding/complete", "POST"], + ["/api/onboarding/restart", "POST"], + ] as const) { + expect( + proxy(makeRequest(path, method)).status, + `${method} ${path} is blocked in the demo`, + ).not.toBe(403); + } + }); + + it("does not drag in a sibling verb on an allowlisted onboarding path", () => { + // The allowlist pins method per path, so admitting the answers PATCH must + // not open a DELETE somebody adds to that route later. + expect(proxy(makeRequest("/api/onboarding/answers", "DELETE")).status).toBe( + 403, + ); + expect(proxy(makeRequest("/api/onboarding/step", "POST")).status).toBe(403); + }); + it("still blocks a health-data mutation (POST /api/measurements)", () => { const res = proxy(makeRequest("/api/measurements", "POST")); expect(res.status).toBe(403); diff --git a/src/app/api/onboarding/complete/__tests__/route.test.ts b/src/app/api/onboarding/complete/__tests__/route.test.ts index 667d16d7e..ccd333d57 100644 --- a/src/app/api/onboarding/complete/__tests__/route.test.ts +++ b/src/app/api/onboarding/complete/__tests__/route.test.ts @@ -1,12 +1,21 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; import { NextRequest } from "next/server"; -vi.mock("@/lib/db", () => ({ - prisma: { - user: { update: vi.fn() }, - auditLog: { create: vi.fn() }, - }, -})); +vi.mock("@/lib/db", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + prisma: { + user: { update: vi.fn(), findUnique: vi.fn() }, + auditLog: { create: vi.fn() }, + // v1.39 (C1) — the needs half of the route looks for a setup row before + // it does anything. These cases are the LEGACY caller, which has none, + // so the lookup answers null and the route falls straight through to the + // acknowledgement it always returned. + onboardingRecord: { findUnique: vi.fn(), update: vi.fn() }, + }, + }; +}); vi.mock("@/lib/auth/session", async (importOriginal) => { const actual = await importOriginal(); @@ -58,6 +67,7 @@ beforeEach(() => { vi.resetAllMocks(); vi.mocked(prisma.user.update).mockResolvedValue({} as never); vi.mocked(prisma.auditLog.create).mockResolvedValue({} as never); + vi.mocked(prisma.onboardingRecord.findUnique).mockResolvedValue(null); }); describe("POST /api/onboarding/complete", () => { diff --git a/tests/integration/onboarding-needs.test.ts b/tests/integration/onboarding-needs.test.ts new file mode 100644 index 000000000..c7526a0bc --- /dev/null +++ b/tests/integration/onboarding-needs.test.ts @@ -0,0 +1,418 @@ +/** + * The three needs-onboarding writes against real Postgres. + * + * The unit suite proves the pure parts — the area map, the merge that never + * retracts a hand-made decision, the apply function's idempotence. This file + * proves the four things a unit suite cannot, because each is about what is in + * the database afterwards: + * + * * **The answers persist and a replay changes nothing.** Idempotence is a + * claim on the wire, and a route can satisfy the pure function and still + * write a fresh row each time. + * * **The derivation happens once.** The second confirm has to leave the + * module map alone, including a module switched off in Settings between + * the two calls — that is the whole reason the latch exists. + * * **A restart resets the steps and not the modules.** The two are written + * to different tables by different routes, so only a real run can show + * that the second one stays where it was. + * * **A delegate cannot write another record's setup.** Proved with a live + * accepted grant and a switched session, not with a mocked resolver. + */ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +process.env.ENCRYPTION_KEY ??= + "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"; + +import { cookieJar, headerJar } from "./mock-next-headers"; +import { getPrismaClient, truncateAllTables } from "./setup"; + +vi.mock("next/headers", async () => { + const { cookieJar, headerJar } = await import("./mock-next-headers"); + return { + headers: vi.fn(async () => ({ + get: (name: string) => headerJar.get(name.toLowerCase()) ?? null, + })), + cookies: vi.fn(async () => ({ + get: (name: string) => { + const value = cookieJar.get(name); + return value ? { name, value } : undefined; + }, + set: (name: string, value: string) => { + cookieJar.set(name, value); + }, + delete: (name: string) => { + cookieJar.delete(name); + }, + })), + }; +}); + +vi.mock("@/lib/db-compat", () => ({ + ensureDbCompatibility: vi.fn().mockResolvedValue(undefined), +})); + +import { acceptGrant, inviteGrant } from "@/lib/sharing/grants"; +import type { + OnboardingStateDto, + OnboardingStepId, +} from "@/lib/onboarding/needs"; + +let counter = 0; + +async function makeUser(label: string) { + const suffix = `${label}-${counter++}`; + return getPrismaClient().user.create({ + data: { + username: `onb-${suffix}`, + email: `onb-${suffix}@example.test`, + role: "USER", + }, + }); +} + +async function signIn(userId: string) { + const session = await getPrismaClient().session.create({ + data: { userId, expiresAt: new Date(Date.now() + 60_000) }, + }); + cookieJar.set("healthlog_session", session.id); + return session; +} + +/** Attach the record-session assertion the shipped browser client attaches. */ +async function assertCurrentContext(sessionId: string) { + const row = await getPrismaClient().session.findUniqueOrThrow({ + where: { id: sessionId }, + }); + headerJar.set("x-healthlog-record-epoch", String(row.recordEpoch)); + headerJar.set("x-healthlog-record-scope", row.actingAsUserId ?? "self"); +} + +function jsonRequest(path: string, method: string, body: unknown): Request { + return new Request(`http://localhost${path}`, { + method, + headers: { "content-type": "application/json" }, + body: JSON.stringify(body), + }); +} + +async function patchAnswer(body: unknown): Promise { + const { PATCH } = await import("@/app/api/onboarding/answers/route"); + return (PATCH as (r: Request) => Promise)( + jsonRequest("/api/onboarding/answers", "PATCH", body), + ); +} + +async function postComplete(body: unknown = {}): Promise { + const { POST } = await import("@/app/api/onboarding/complete/route"); + return (POST as (r: Request) => Promise)( + jsonRequest("/api/onboarding/complete", "POST", body), + ); +} + +async function postRestart(): Promise { + const { POST } = await import("@/app/api/onboarding/restart/route"); + return (POST as (r: Request) => Promise)( + jsonRequest("/api/onboarding/restart", "POST", {}), + ); +} + +async function readState(res: Response): Promise { + const body = (await res.json()) as { + data: { onboarding: OnboardingStateDto }; + }; + return body.data.onboarding; +} + +function statusOf(state: OnboardingStateDto, id: OnboardingStepId) { + return state.steps.find((step) => step.id === id)?.status; +} + +/** The five answers a "blood pressure and medication" setup gives. */ +async function answerTheQuestions() { + await patchAnswer({ step: "who", recordTarget: "me" }); + await patchAnswer({ step: "areas", areas: ["blood-pressure", "labs"] }); + await patchAnswer({ step: "medication", medication: "yes" }); + await patchAnswer({ step: "sources", status: "skipped" }); + await patchAnswer({ step: "visit", visit: "no" }); +} + +async function modulePrefs(userId: string) { + const row = await getPrismaClient().user.findUniqueOrThrow({ + where: { id: userId }, + select: { modulePreferencesJson: true }, + }); + return (row.modulePreferencesJson ?? {}) as Record; +} + +beforeEach(async () => { + await truncateAllTables(getPrismaClient()); + cookieJar.clear(); + headerJar.clear(); +}); + +afterEach(() => { + vi.restoreAllMocks(); +}); + +describe("PATCH /api/onboarding/answers", () => { + it("saves one step at a time and answers with the full nine", async () => { + const user = await makeUser("answers"); + await signIn(user.id); + + const first = await patchAnswer({ step: "who", recordTarget: "both" }); + expect(first.status).toBe(200); + const state = await readState(first); + expect(state.needs.recordTarget).toBe("both"); + expect(state.steps).toHaveLength(9); + expect(statusOf(state, "who")).toBe("done"); + expect(statusOf(state, "areas")).toBe("pending"); + + const second = await readState( + await patchAnswer({ step: "areas", areas: ["sleep"] }), + ); + expect(second.needs.recordTarget).toBe("both"); + expect(second.needs.areas).toEqual(["sleep"]); + expect(statusOf(second, "areas")).toBe("done"); + }); + + it("is idempotent — a replay leaves one row and the same state", async () => { + const user = await makeUser("replay"); + await signIn(user.id); + + const once = await readState( + await patchAnswer({ step: "medication", medication: "sometimes" }), + ); + const twice = await readState( + await patchAnswer({ step: "medication", medication: "sometimes" }), + ); + expect(twice).toEqual(once); + expect( + await getPrismaClient().onboardingRecord.count({ + where: { userId: user.id }, + }), + ).toBe(1); + }); + + it("records a skip without erasing the answer already given", async () => { + const user = await makeUser("skip"); + await signIn(user.id); + + await patchAnswer({ step: "visit", visit: "within-a-month" }); + const skipped = await readState( + await patchAnswer({ step: "visit", status: "skipped" }), + ); + expect(statusOf(skipped, "visit")).toBe("skipped"); + expect(skipped.needs.visit).toBe("within-a-month"); + }); + + it("writes the record's real unit columns, and records the answer beside them", async () => { + const user = await makeUser("units"); + await signIn(user.id); + + const state = await readState( + await patchAnswer({ + step: "units", + units: { glucoseUnit: "mmol/L", unitPreference: "imperial" }, + }), + ); + expect(state.needs.units).toEqual({ + glucoseUnit: "mmol/L", + unitPreference: "imperial", + }); + const row = await getPrismaClient().user.findUniqueOrThrow({ + where: { id: user.id }, + select: { glucoseUnit: true, unitPreference: true }, + }); + expect(row).toEqual({ glucoseUnit: "mmol/L", unitPreference: "imperial" }); + }); + + it("refuses a body that names a step it does not answer", async () => { + const user = await makeUser("invalid"); + await signIn(user.id); + + const res = await patchAnswer({ step: "areas", medication: "yes" }); + expect(res.status).toBe(422); + const body = (await res.json()) as { meta?: { errorCode?: string } }; + expect(body.meta?.errorCode).toBe("onboarding.answers.invalid"); + expect( + await getPrismaClient().onboardingRecord.count({ + where: { userId: user.id }, + }), + ).toBe(0); + }); +}); + +describe("POST /api/onboarding/complete", () => { + it("derives the module map from the answers, once", async () => { + const user = await makeUser("derive"); + await signIn(user.id); + await answerTheQuestions(); + + const first = await postComplete(); + expect(first.status).toBe(200); + + const derived = await modulePrefs(user.id); + // What the answers named. + expect(derived.medications).toBe(true); + expect(derived.labs).toBe(true); + expect(derived.insights).toBe(true); + expect(derived.achievements).toBe(true); + // What they did not. + expect(derived.workouts).toBe(false); + expect(derived.mood).toBe(false); + expect(derived.doctorReport).toBe(false); + expect(derived.vaccinations).toBe(false); + // The delegated keys are never written into the blob. + expect(derived).not.toHaveProperty("cycle"); + expect(derived).not.toHaveProperty("coach"); + + const state = await readState(first); + expect(statusOf(state, "confirm")).toBe("done"); + expect(state.completedAt).not.toBeNull(); + + // Somebody changes their mind in Settings, then the confirm screen is + // replayed. The second call must leave that decision alone: the latch is + // what stops a questionnaire from re-applying itself over a later choice. + await getPrismaClient().user.update({ + where: { id: user.id }, + data: { modulePreferencesJson: { ...derived, labs: false } }, + }); + const second = await postComplete(); + expect(second.status).toBe(200); + expect((await modulePrefs(user.id)).labs).toBe(false); + }); + + it("never switches off a module the person switched on by hand", async () => { + const user = await makeUser("handmade"); + await signIn(user.id); + // The remote MCP endpoint is off by default and nothing in the flow asks + // for it, so a derivation that ignored the stored map would retract it. + await getPrismaClient().user.update({ + where: { id: user.id }, + data: { modulePreferencesJson: { mcp: true } }, + }); + await answerTheQuestions(); + await postComplete(); + + expect((await modulePrefs(user.id)).mcp).toBe(true); + }); + + it("switches cycle tracking on through its own column when the area is chosen", async () => { + const user = await makeUser("cycle"); + await signIn(user.id); + await patchAnswer({ step: "who", recordTarget: "me" }); + await patchAnswer({ step: "areas", areas: ["cycle"] }); + await postComplete(); + + const profile = await getPrismaClient().cycleProfile.findUniqueOrThrow({ + where: { userId: user.id }, + }); + expect(profile.cycleTrackingEnabled).toBe(true); + }); + + it("leaves a record that never answered the questions exactly as it was", async () => { + const user = await makeUser("legacy"); + await signIn(user.id); + + const res = await postComplete({ heightCm: 180 }); + expect(res.status).toBe(200); + const body = (await res.json()) as { + data: { completed: boolean; onboarding?: unknown }; + }; + expect(body.data.completed).toBe(true); + expect(body.data.onboarding).toBeUndefined(); + + const row = await getPrismaClient().user.findUniqueOrThrow({ + where: { id: user.id }, + select: { modulePreferencesJson: true, onboardingCompletedAt: true }, + }); + expect(row.modulePreferencesJson).toBeNull(); + expect(row.onboardingCompletedAt).not.toBeNull(); + }); +}); + +describe("POST /api/onboarding/restart", () => { + it("resets the steps and leaves the modules where they are", async () => { + const user = await makeUser("restart"); + await signIn(user.id); + await answerTheQuestions(); + await postComplete(); + const derived = await modulePrefs(user.id); + + const res = await postRestart(); + expect(res.status).toBe(200); + const state = await readState(res); + expect(state.steps.every((step) => step.status === "pending")).toBe(true); + expect(state.completedAt).toBeNull(); + // The answers survive as the prefill for the re-run. + expect(state.needs.medication).toBe("yes"); + expect(await modulePrefs(user.id)).toEqual(derived); + + // The latch is cleared, so the next confirm may derive again — and a + // module switched off by hand in between still survives it. + await getPrismaClient().user.update({ + where: { id: user.id }, + data: { modulePreferencesJson: { ...derived, labs: true } }, + }); + await patchAnswer({ step: "who", recordTarget: "me" }); + await patchAnswer({ step: "areas", areas: [] }); + await postComplete(); + expect((await modulePrefs(user.id)).labs).toBe(true); + }); + + it("leaves the first-run redirect's own stamp alone", async () => { + const user = await makeUser("redirect"); + await signIn(user.id); + await answerTheQuestions(); + await postComplete(); + await postRestart(); + + const row = await getPrismaClient().user.findUniqueOrThrow({ + where: { id: user.id }, + select: { onboardingCompletedAt: true }, + }); + expect(row.onboardingCompletedAt).not.toBeNull(); + }); +}); + +describe("a delegate and somebody else's record", () => { + it("refuses every setup write while the session is acting on another record", async () => { + const owner = await makeUser("owner"); + const delegate = await makeUser("delegate"); + const invited = await inviteGrant({ + grantorId: owner.id, + granteeId: delegate.id, + access: "READ", + scope: null, + }); + await acceptGrant({ grantId: invited.id, granteeId: delegate.id }); + + const session = await signIn(delegate.id); + await getPrismaClient().session.update({ + where: { id: session.id }, + data: { actingAsUserId: owner.id }, + }); + await assertCurrentContext(session.id); + + for (const call of [ + () => patchAnswer({ step: "who", recordTarget: "me" }), + () => postComplete(), + () => postRestart(), + ]) { + const res = await call(); + expect(res.status).toBe(403); + const body = (await res.json()) as { meta?: { errorCode?: string } }; + expect(body.meta?.errorCode).toBe("sharing.not_permitted"); + } + + // Neither record acquired a setup row — not the owner's, and not the + // delegate's own, which is the quiet failure a fall-back would produce. + expect(await getPrismaClient().onboardingRecord.count()).toBe(0); + const ownerRow = await getPrismaClient().user.findUniqueOrThrow({ + where: { id: owner.id }, + select: { modulePreferencesJson: true, onboardingCompletedAt: true }, + }); + expect(ownerRow.modulePreferencesJson).toBeNull(); + expect(ownerRow.onboardingCompletedAt).toBeNull(); + }); +}); From 7a1f3fe32dd4377faa63eaf450927eabcc8375c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 10 Sep 2026 11:39:32 +0200 Subject: [PATCH 07/15] fix(onboarding): stop the setup flow asking for units the account already holds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The spec's sixth question is skipped when the account already carries the preferences, and the flow had no way to know: `User.unitPreference` is nullable with null meaning "default metric", and the account payload coerces that away, so nothing downstream could tell a chosen metric from an unset column. The two columns are read raw on the server now, into the same vocabulary the wire publishes, and a record whose account holds both reads its `units` step as done — the question answers itself and is never put again. Read-side only: the stored ledger keeps saying pending, because the person really has not answered it, and an answer given later still lands. The two writes the units answer makes — the display columns and the record of the answer — go in one transaction, so a failure between them can no longer leave the columns changed with no record of what changed them. --- docs/api/openapi.yaml | 10 ++- src/app/api/auth/me/route.ts | 18 ++++- src/app/api/onboarding/answers/route.ts | 32 ++++++-- src/app/api/onboarding/complete/route.ts | 24 ++++-- src/app/api/onboarding/restart/route.ts | 9 ++- src/lib/onboarding/__tests__/needs.test.ts | 94 ++++++++++++++++++++++ src/lib/onboarding/needs-store.ts | 29 +++++-- src/lib/onboarding/needs.ts | 57 +++++++++++++ src/lib/openapi/routes/onboarding.ts | 2 +- tests/integration/onboarding-needs.test.ts | 29 +++++++ 10 files changed, 275 insertions(+), 29 deletions(-) diff --git a/docs/api/openapi.yaml b/docs/api/openapi.yaml index 388d2826b..597841360 100644 --- a/docs/api/openapi.yaml +++ b/docs/api/openapi.yaml @@ -63210,8 +63210,14 @@ components: - id - status additionalProperties: false - description: One step of the setup flow. The ids are stable; a client that meets one it does not know SKIPS it rather - than refusing the list, so the flow may grow a screen without a client release. + description: >- + One step of the setup flow. The ids are stable; a client that meets one it does not know SKIPS it rather than + refusing the list, so the flow may grow a screen without a client release. + + + `units` carries one resolution the others do not: a record whose account already holds BOTH unit preferences + reads as `done` whether or not the question was ever put, which is what stops the flow re-asking a value the + account holds. OnboardingNeeds: type: object properties: diff --git a/src/app/api/auth/me/route.ts b/src/app/api/auth/me/route.ts index 69a283bb9..9c3b71d32 100644 --- a/src/app/api/auth/me/route.ts +++ b/src/app/api/auth/me/route.ts @@ -54,7 +54,11 @@ import { resolveModuleMap, getOperatorModuleAvailability, } from "@/lib/modules/gate"; -import { loadOnboardingState } from "@/lib/onboarding/needs-store"; +import { readHeldUnitPreferences } from "@/lib/onboarding/needs"; +import { + loadOnboardingRecordRow, + toOnboardingStateDto, +} from "@/lib/onboarding/needs-store"; import { parseTourProgress } from "@/lib/onboarding/tour-progress"; import { parseNotificationPrefs } from "@/lib/validations/notification-prefs"; import { resolveAccountAccess } from "@/lib/sharing/account-access"; @@ -113,7 +117,7 @@ export const GET = apiHandler(async () => { cycleProfile, resolvedModules, moduleAvailability, - onboarding, + onboardingRow, ] = await Promise.all([ // The actor's own row is already in hand; a switched session needs the // record's `gender`, which is the column the cycle gate derives from. @@ -150,7 +154,7 @@ export const GET = apiHandler(async () => { // act on and never complete. A guardian's route into a managed record's // flow arrives with the rest of that record's configuration (#939 / C2); // the storage is already keyed by record for it. - loadOnboardingState(prisma, user.id), + loadOnboardingRecordRow(prisma, user.id), ]); // The masking step, and the one place the record scoping is narrowed rather @@ -165,6 +169,14 @@ export const GET = apiHandler(async () => { // exactly as it was, and `moduleAccess` is the same answer with the reason // attached — the record's own switch, the grant's edge, or the operator's. // Every client that only wants "paint it or not" keeps reading the boolean. + // v1.39 (C1) — the published steps carry the Q6 resolution: an account that + // already holds both unit preferences reads `units` as `done`, so the flow + // never re-asks a value the account holds. + const onboarding = toOnboardingStateDto( + onboardingRow, + readHeldUnitPreferences(user), + ); + const { modules, moduleAccess } = buildModuleDisclosure( resolvedModules, moduleAvailability, diff --git a/src/app/api/onboarding/answers/route.ts b/src/app/api/onboarding/answers/route.ts index 25048eea7..fd101d76e 100644 --- a/src/app/api/onboarding/answers/route.ts +++ b/src/app/api/onboarding/answers/route.ts @@ -33,6 +33,7 @@ import { import { prisma, toJson } from "@/lib/db"; import { annotate } from "@/lib/logging/context"; import { applyOnboardingAnswer } from "@/lib/onboarding/needs-apply"; +import { readHeldUnitPreferences } from "@/lib/onboarding/needs"; import { ONBOARDING_RECORD_SELECT, readOnboardingRecordState, @@ -98,18 +99,17 @@ export const PATCH = apiHandler(async (request: NextRequest) => { // second source of truth. What is kept beside them in `needs.units` is the // answer as given, so a restart can show it back without re-deriving it from // a column Settings may have changed since. + const unitData: { glucoseUnit?: string; unitPreference?: string } = {}; if (answer.step === "units" && answer.status !== "skipped") { - const data: { glucoseUnit?: string; unitPreference?: string } = {}; - if (answer.units.glucoseUnit) data.glucoseUnit = answer.units.glucoseUnit; - if (answer.units.unitPreference) { - data.unitPreference = answer.units.unitPreference; + if (answer.units.glucoseUnit) { + unitData.glucoseUnit = answer.units.glucoseUnit; } - if (Object.keys(data).length > 0) { - await prisma.user.update({ where: { id: user.id }, data }); + if (answer.units.unitPreference) { + unitData.unitPreference = answer.units.unitPreference; } } - const written = await prisma.onboardingRecord.upsert({ + const upsert = prisma.onboardingRecord.upsert({ where: { userId: user.id }, // Field-by-field, never a spread of the parsed body: the three columns are // the pure function's output, and nothing the client sent reaches Prisma @@ -130,6 +130,22 @@ export const PATCH = apiHandler(async (request: NextRequest) => { select: ONBOARDING_RECORD_SELECT, }); + // One transaction, because the two writes are one answer: a failure between + // them would leave the display columns changed with no record of the answer + // that changed them. + const [written] = + Object.keys(unitData).length > 0 + ? await prisma.$transaction([ + upsert, + prisma.user.update({ where: { id: user.id }, data: unitData }), + ]) + : await prisma.$transaction([upsert]); + + const held = readHeldUnitPreferences({ + glucoseUnit: unitData.glucoseUnit ?? user.glucoseUnit, + unitPreference: unitData.unitPreference ?? user.unitPreference, + }); + annotate({ action: { name: "onboarding.answer.save" }, meta: { @@ -138,5 +154,5 @@ export const PATCH = apiHandler(async (request: NextRequest) => { }, }); - return apiSuccess({ onboarding: toOnboardingStateDto(written) }); + return apiSuccess({ onboarding: toOnboardingStateDto(written, held) }); }); diff --git a/src/app/api/onboarding/complete/route.ts b/src/app/api/onboarding/complete/route.ts index 2246885f4..e18087449 100644 --- a/src/app/api/onboarding/complete/route.ts +++ b/src/app/api/onboarding/complete/route.ts @@ -10,6 +10,11 @@ import { deriveOnboardingModuleDefaults, mergeDerivedModulePreferences, } from "@/lib/modules/registry"; +import { + readHeldUnitPreferences, + resolveOnboardingSteps, + type HeldUnitPreferences, +} from "@/lib/onboarding/needs"; import { ONBOARDING_RECORD_SELECT, readOnboardingRecordState, @@ -87,7 +92,10 @@ export const POST = apiHandler(async (request: NextRequest) => { // next navigation drops the /onboarding redirect immediately. await setOnboardingPendingCookie(false); - const onboarding = await completeNeedsFlow(user.id); + const onboarding = await completeNeedsFlow( + user.id, + readHeldUnitPreferences(user), + ); return apiSuccess({ completed: true, ...(onboarding ? { onboarding } : {}) }); }); @@ -100,7 +108,7 @@ export const POST = apiHandler(async (request: NextRequest) => { * needs flow — a legacy wizard completion then answers exactly the body it * always did. */ -async function completeNeedsFlow(userId: string) { +async function completeNeedsFlow(userId: string, held: HeldUnitPreferences) { const record = await prisma.onboardingRecord.findUnique({ where: { userId }, select: ONBOARDING_RECORD_SELECT, @@ -108,6 +116,10 @@ async function completeNeedsFlow(userId: string) { if (!record) return null; const state = readOnboardingRecordState(record); + // The units question answers itself for an account that already holds both + // preferences, so the gate and the published state read the same resolved + // ledger rather than the raw one. + const steps = resolveOnboardingSteps(state.steps, held); if (state.needs.recordTarget === null) { // The row exists but Q1 was never answered, so there is nothing to derive // from. Stamping a completion here would claim a setup that did not happen. @@ -115,7 +127,7 @@ async function completeNeedsFlow(userId: string) { action: { name: "onboarding.needs.complete" }, meta: { outcome: "no_answers" }, }); - return toOnboardingStateDto(record); + return toOnboardingStateDto(record, held); } let derived = false; @@ -162,7 +174,7 @@ async function completeNeedsFlow(userId: string) { } const now = new Date(); - const steps = state.steps.map((step) => + const confirmed = steps.map((step) => step.id === "confirm" && step.status === "pending" ? { ...step, status: "done" as const } : step, @@ -170,7 +182,7 @@ async function completeNeedsFlow(userId: string) { const written = await prisma.onboardingRecord.update({ where: { userId }, data: { - stepsJson: toJson(steps), + stepsJson: toJson(confirmed), completedAt: now, ...(derived ? { modulesDerivedAt: now } : {}), }, @@ -182,5 +194,5 @@ async function completeNeedsFlow(userId: string) { meta: { outcome: derived ? "derived" : "already_derived" }, }); - return toOnboardingStateDto(written); + return toOnboardingStateDto(written, held); } diff --git a/src/app/api/onboarding/restart/route.ts b/src/app/api/onboarding/restart/route.ts index 0990e898b..a64a23914 100644 --- a/src/app/api/onboarding/restart/route.ts +++ b/src/app/api/onboarding/restart/route.ts @@ -37,7 +37,10 @@ import { import { auditLog } from "@/lib/auth/audit"; import { prisma, toJson } from "@/lib/db"; import { annotate } from "@/lib/logging/context"; -import { defaultOnboardingSteps } from "@/lib/onboarding/needs"; +import { + defaultOnboardingSteps, + readHeldUnitPreferences, +} from "@/lib/onboarding/needs"; import { ONBOARDING_RECORD_SELECT, toOnboardingStateDto, @@ -105,5 +108,7 @@ export const POST = apiHandler(async (request: NextRequest) => { meta: { outcome: "reset" }, }); - return apiSuccess({ onboarding: toOnboardingStateDto(written) }); + return apiSuccess({ + onboarding: toOnboardingStateDto(written, readHeldUnitPreferences(user)), + }); }); diff --git a/src/lib/onboarding/__tests__/needs.test.ts b/src/lib/onboarding/__tests__/needs.test.ts index a070ebda8..db09724f2 100644 --- a/src/lib/onboarding/__tests__/needs.test.ts +++ b/src/lib/onboarding/__tests__/needs.test.ts @@ -14,6 +14,7 @@ import { describe, expect, it } from "vitest"; import { applyOnboardingAnswer } from "../needs-apply"; import { + accountHoldsBothUnits, defaultOnboardingSteps, emptyOnboardingNeeds, isOnboardingSettled, @@ -21,6 +22,8 @@ import { parseOnboardingFirstResult, parseOnboardingNeeds, parseOnboardingSteps, + readHeldUnitPreferences, + resolveOnboardingSteps, type OnboardingStateDto, } from "../needs"; @@ -260,3 +263,94 @@ describe("isOnboardingSettled", () => { ).toBe(false); }); }); + +describe("the unit preferences the account already holds", () => { + it("reads the two raw columns into the wire vocabulary", () => { + expect( + readHeldUnitPreferences({ + glucoseUnit: "mmol/L", + unitPreference: "imperial", + }), + ).toEqual({ glucoseUnit: "mmol/L", unitPreference: "imperial" }); + }); + + it("answers null for a column that was never chosen", () => { + const held = readHeldUnitPreferences({ + glucoseUnit: null, + unitPreference: null, + }); + expect(held).toEqual({ glucoseUnit: null, unitPreference: null }); + expect(accountHoldsBothUnits(held)).toBe(false); + }); + + it("does not mistake a value it does not know for a chosen one", () => { + expect( + readHeldUnitPreferences({ + glucoseUnit: "stones", + unitPreference: "metrick", + }), + ).toEqual({ glucoseUnit: null, unitPreference: null }); + }); + + it("needs both preferences before the question answers itself", () => { + expect( + accountHoldsBothUnits({ glucoseUnit: "mg/dL", unitPreference: null }), + ).toBe(false); + expect( + accountHoldsBothUnits({ glucoseUnit: null, unitPreference: "metric" }), + ).toBe(false); + expect( + accountHoldsBothUnits({ + glucoseUnit: "mg/dL", + unitPreference: "metric", + }), + ).toBe(true); + }); +}); + +describe("resolveOnboardingSteps", () => { + const held = { + glucoseUnit: "mg/dL", + unitPreference: "metric", + } as const; + + it("marks units done when the account already holds both preferences", () => { + const resolved = resolveOnboardingSteps(defaultOnboardingSteps(), held); + expect(resolved.find((s) => s.id === "units")?.status).toBe("done"); + }); + + it("leaves units pending while either preference is unset", () => { + for (const partial of [ + { glucoseUnit: "mg/dL", unitPreference: null }, + { glucoseUnit: null, unitPreference: "metric" }, + { glucoseUnit: null, unitPreference: null }, + ] as const) { + const resolved = resolveOnboardingSteps( + defaultOnboardingSteps(), + partial, + ); + expect(resolved.find((s) => s.id === "units")?.status).toBe("pending"); + } + }); + + it("never overwrites an answer that was actually given", () => { + const skipped = defaultOnboardingSteps().map((s) => + s.id === "units" ? { ...s, status: "skipped" as const } : s, + ); + expect( + resolveOnboardingSteps(skipped, held).find((s) => s.id === "units") + ?.status, + ).toBe("skipped"); + }); + + it("touches no other step", () => { + const resolved = resolveOnboardingSteps(defaultOnboardingSteps(), held); + expect( + resolved.filter((s) => s.id !== "units").map((s) => s.status), + ).toEqual( + defaultOnboardingSteps() + .filter((s) => s.id !== "units") + .map((s) => s.status), + ); + }); +}); diff --git a/src/lib/onboarding/needs-store.ts b/src/lib/onboarding/needs-store.ts index 4532fab07..64cb3560e 100644 --- a/src/lib/onboarding/needs-store.ts +++ b/src/lib/onboarding/needs-store.ts @@ -17,6 +17,8 @@ import { parseOnboardingFirstResult, parseOnboardingNeeds, parseOnboardingSteps, + resolveOnboardingSteps, + type HeldUnitPreferences, type OnboardingStateDto, } from "./needs"; import type { OnboardingRecordState } from "./needs-apply"; @@ -56,13 +58,21 @@ export function readOnboardingRecordState( }; } -/** The shape published on the account payload. */ +/** + * The shape published on the account payload. + * + * `held` is what the RECORD's account already carries in its two unit columns, + * and it is a required argument rather than an optional one: the Q6 "skipped + * when the account already holds them" rule has to hold for every reader of + * this DTO, and a caller that has not thought about it should fail to compile. + */ export function toOnboardingStateDto( row: OnboardingRecordRow | null, + held: HeldUnitPreferences, ): OnboardingStateDto { const state = readOnboardingRecordState(row); return { - steps: state.steps, + steps: resolveOnboardingSteps(state.steps, held), needs: state.needs, completedAt: row?.completedAt?.toISOString() ?? null, firstResult: state.firstResult, @@ -70,19 +80,24 @@ export function toOnboardingStateDto( } /** - * Load the published state for one record. + * Load one record's stored onboarding row, or `null` when it never entered the + * flow. + * + * The row rather than the DTO, because the DTO needs the record's unit columns + * as well and the caller is the one holding them — the account payload already + * reads that user row for the cycle gate, and a second read of the same row on + * the hottest endpoint in the app would buy nothing. * * Takes the delegate rather than the whole client, matching the section * builders next door, so the route's global client and a worker's local one * share the same read. */ -export async function loadOnboardingState( +export async function loadOnboardingRecordRow( prisma: Pick, recordId: string, -): Promise { - const row = await prisma.onboardingRecord.findUnique({ +): Promise { + return prisma.onboardingRecord.findUnique({ where: { userId: recordId }, select: ONBOARDING_RECORD_SELECT, }); - return toOnboardingStateDto(row); } diff --git a/src/lib/onboarding/needs.ts b/src/lib/onboarding/needs.ts index 20dd0c018..be7606a9b 100644 --- a/src/lib/onboarding/needs.ts +++ b/src/lib/onboarding/needs.ts @@ -323,3 +323,60 @@ export function isOnboardingSettled( if (state.completedAt === null) return false; return state.firstResult === null || state.firstResult.completedAt !== null; } + +/** + * The unit columns an account already holds, read as the vocabulary the wire + * publishes. + * + * `User.unitPreference` is nullable and `null` means "default metric", which + * the account payload coerces away (`unitPreference === "imperial" ? + * "imperial" : "metric"`). That coercion is fine to compare a CHOSEN value + * against and useless for telling a chosen metric from an unset column, so the + * server reads the raw column here and answers `null` for "never chosen". + */ +export interface HeldUnitPreferences { + glucoseUnit: "mg/dL" | "mmol/L" | null; + unitPreference: "metric" | "imperial" | null; +} + +/** Read the two raw columns into the wire vocabulary; anything else is unset. */ +export function readHeldUnitPreferences(row: { + glucoseUnit: string | null; + unitPreference: string | null; +}): HeldUnitPreferences { + return { + glucoseUnit: member(["mg/dL", "mmol/L"] as const, row.glucoseUnit), + unitPreference: member(["metric", "imperial"] as const, row.unitPreference), + }; +} + +/** True once the account carries both unit preferences as deliberate values. */ +export function accountHoldsBothUnits(held: HeldUnitPreferences): boolean { + return held.glucoseUnit !== null && held.unitPreference !== null; +} + +/** + * The steps as they should be READ, given what the account already holds. + * + * The spec's Q6 is "skipped when the account already holds them", and the + * evidence for that is the two unit columns, not the questionnaire's own + * memory of having asked. So a record whose account carries both preferences + * publishes `units` as `done` even though nobody answered it here — which is + * what stops the flow re-asking a value the account holds, and what stops the + * completion gate below waiting for an answer to a question the flow will + * never show. + * + * Read-side only: the stored ledger keeps saying `pending`, because the person + * really has not answered it, and an answer given later still lands. + */ +export function resolveOnboardingSteps( + steps: readonly OnboardingStepState[], + held: HeldUnitPreferences, +): OnboardingStepState[] { + if (!accountHoldsBothUnits(held)) return [...steps]; + return steps.map((step) => + step.id === "units" && step.status === "pending" + ? { ...step, status: "done" as const } + : step, + ); +} diff --git a/src/lib/openapi/routes/onboarding.ts b/src/lib/openapi/routes/onboarding.ts index 06a81dba6..2474b4814 100644 --- a/src/lib/openapi/routes/onboarding.ts +++ b/src/lib/openapi/routes/onboarding.ts @@ -91,7 +91,7 @@ const onboardingStepResource = z .meta({ id: "OnboardingStepState", description: - "One step of the setup flow. The ids are stable; a client that meets one it does not know SKIPS it rather than refusing the list, so the flow may grow a screen without a client release.", + "One step of the setup flow. The ids are stable; a client that meets one it does not know SKIPS it rather than refusing the list, so the flow may grow a screen without a client release.\n\n`units` carries one resolution the others do not: a record whose account already holds BOTH unit preferences reads as `done` whether or not the question was ever put, which is what stops the flow re-asking a value the account holds.", }); const onboardingFirstResultResource = z diff --git a/tests/integration/onboarding-needs.test.ts b/tests/integration/onboarding-needs.test.ts index c7526a0bc..8d9771d55 100644 --- a/tests/integration/onboarding-needs.test.ts +++ b/tests/integration/onboarding-needs.test.ts @@ -226,6 +226,35 @@ describe("PATCH /api/onboarding/answers", () => { expect(row).toEqual({ glucoseUnit: "mmol/L", unitPreference: "imperial" }); }); + it("does not re-ask the units the account already holds", async () => { + const user = await makeUser("units-held"); + await signIn(user.id); + await getPrismaClient().user.update({ + where: { id: user.id }, + data: { glucoseUnit: "mg/dL", unitPreference: "metric" }, + }); + + const state = await readState( + await patchAnswer({ step: "who", recordTarget: "me" }), + ); + // Nobody answered Q6; the account's own columns did. + expect(statusOf(state, "units")).toBe("done"); + expect(state.needs.units).toEqual({ + glucoseUnit: null, + unitPreference: null, + }); + + // Half a preference is not an answer. + await getPrismaClient().user.update({ + where: { id: user.id }, + data: { unitPreference: null }, + }); + const half = await readState( + await patchAnswer({ step: "who", recordTarget: "me" }), + ); + expect(statusOf(half, "units")).toBe("pending"); + }); + it("refuses a body that names a step it does not answer", async () => { const user = await makeUser("invalid"); await signIn(user.id); From cc6e6ca221fb2d12ddc5ed1304b18bb37ba9ab8e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 10 Sep 2026 11:42:38 +0200 Subject: [PATCH 08/15] fix(onboarding): derive the module map only from a questionnaire that was finished The only gate on the derivation was Q1 being set, so one answer was enough to turn the whole questionnaire into a module map. Two ways that landed wrong and they compound. Somebody answers who the record is for, closes the tab, and later finishes the legacy four-step wizard, which posts to this same endpoint and knows nothing about these questions: the five unanswered ones each read as their conservative default, eleven surfaces are written off, and the once-only marker makes it unrepeatable. Or any client patches one answer and posts the confirm straight back. The gate now asks whether every question has an answer or a deliberate pass. A half-answered flow derives nothing, is not stamped complete, and keeps its confirm step pending, so finishing the questions later still does the work. The legacy arm is untouched: a caller with no setup row still answers the fixed acknowledgement it always did. --- docs/api/openapi.yaml | 22 +++++--- src/app/api/onboarding/complete/route.ts | 21 +++++++- src/lib/onboarding/__tests__/needs.test.ts | 60 ++++++++++++++++++++++ src/lib/onboarding/needs.ts | 31 +++++++++++ src/lib/openapi/routes/onboarding.ts | 2 +- tests/integration/onboarding-needs.test.ts | 41 ++++++++++++++- 6 files changed, 165 insertions(+), 12 deletions(-) diff --git a/docs/api/openapi.yaml b/docs/api/openapi.yaml index 597841360..21e04da25 100644 --- a/docs/api/openapi.yaml +++ b/docs/api/openapi.yaml @@ -16371,14 +16371,20 @@ paths: v1.39 — this is also the needs-based flow's confirm endpoint, and the two halves live side by side. For a record - that answered the questions (Q1 is set), it derives that record's module map from the answers and applies it, - ONCE: the derivation marker is what stops a second confirm re-applying the questionnaire over decisions taken in - Settings since, and `POST /api/onboarding/restart` clears the marker when the person asks for the questions - again. The merge is one-directional — a module the answers name is switched on, a module they do not name is - switched off only where the record does not already carry an explicit on. The `cycle` key is not written here at - all: it delegates to the cycle profile, which this route sets to `true` when the cycle area was chosen and never - to `false`. The response then carries `onboarding` beside `completed`; a caller that never entered the needs - flow gets the fixed acknowledgement it always did. + that FINISHED the questions — every one of them answered or deliberately passed — it derives that record's + module map from the answers and applies it, ONCE: the derivation marker is what stops a second confirm + re-applying the questionnaire over decisions taken in Settings since, and `POST /api/onboarding/restart` clears + the marker when the person asks for the questions again. The merge is one-directional — a module the answers + name is switched on, a module they do not name is switched off only where the record does not already carry an + explicit on. The `cycle` key is not written here at all: it delegates to the cycle profile, which this route + sets to `true` when the cycle area was chosen and never to `false`. A half-answered flow derives nothing and is + not stamped as complete: the answers become a module map, and a flow somebody abandoned after the first screen + would have every remaining question read as its conservative default. The same gate is what keeps the LEGACY + wizard's completion out of the derivation, since that wizard never puts these questions. + + + The response then carries `onboarding` beside `completed`; a caller that never entered the needs flow gets the + fixed acknowledgement it always did. The older sibling of `POST /api/onboarding/step`. It is unconditional in a way the step route is not: it stamps diff --git a/src/app/api/onboarding/complete/route.ts b/src/app/api/onboarding/complete/route.ts index e18087449..d34e644f8 100644 --- a/src/app/api/onboarding/complete/route.ts +++ b/src/app/api/onboarding/complete/route.ts @@ -11,6 +11,7 @@ import { mergeDerivedModulePreferences, } from "@/lib/modules/registry"; import { + everyOnboardingQuestionSettled, readHeldUnitPreferences, resolveOnboardingSteps, type HeldUnitPreferences, @@ -32,8 +33,9 @@ import { onboardingCompleteSchema } from "@/lib/validations/onboarding"; * flow, and the two live side by side rather than one replacing the other. * The legacy half is unchanged: the optional profile fields, the completion * stamp on `User.onboardingCompletedAt`, and the cleared pending cookie. The - * needs half runs only for a record that actually answered the questions - * (`needs.recordTarget` is set) and is what turns the answers into a module + * needs half runs only for a record that actually FINISHED the questions — + * every one of them answered or deliberately passed — and is what turns them + * into a module * map — ONCE, guarded by `OnboardingRecord.modulesDerivedAt`, because a * re-derivation would re-apply the questionnaire over decisions taken in * Settings since. `POST /api/onboarding/restart` clears that marker when the @@ -129,6 +131,21 @@ async function completeNeedsFlow(userId: string, held: HeldUnitPreferences) { }); return toOnboardingStateDto(record, held); } + if (!everyOnboardingQuestionSettled(steps)) { + // The confirm screen is the only caller that may derive, and it is reached + // only once every question has an answer or a deliberate pass. Q1 alone is + // not enough: an abandoned flow would have every remaining question read + // as its conservative default, switch off the surfaces that default + // implies, and latch the result so nobody could re-derive it. This route + // is also the LEGACY wizard's endpoint, and that wizard knows nothing + // about these questions — so a person who answered Q1, left, and later + // finished the old wizard must not have a module map derived for them. + annotate({ + action: { name: "onboarding.needs.complete" }, + meta: { outcome: "incomplete" }, + }); + return toOnboardingStateDto(record, held); + } let derived = false; if (record.modulesDerivedAt === null) { diff --git a/src/lib/onboarding/__tests__/needs.test.ts b/src/lib/onboarding/__tests__/needs.test.ts index db09724f2..7af629791 100644 --- a/src/lib/onboarding/__tests__/needs.test.ts +++ b/src/lib/onboarding/__tests__/needs.test.ts @@ -16,6 +16,8 @@ import { applyOnboardingAnswer } from "../needs-apply"; import { accountHoldsBothUnits, defaultOnboardingSteps, + everyOnboardingQuestionSettled, + ONBOARDING_QUESTION_STEP_IDS, emptyOnboardingNeeds, isOnboardingSettled, ONBOARDING_STEP_IDS, @@ -354,3 +356,61 @@ describe("resolveOnboardingSteps", () => { ); }); }); + +describe("everyOnboardingQuestionSettled", () => { + function withStatuses( + status: "pending" | "done" | "skipped", + overrides: Partial> = {}, + ) { + return defaultOnboardingSteps().map((step) => ({ + ...step, + status: overrides[step.id] ?? status, + })); + } + + it("names the six questions and stops before the confirm screen", () => { + expect([...ONBOARDING_QUESTION_STEP_IDS]).toEqual([ + "who", + "areas", + "medication", + "sources", + "visit", + "units", + ]); + }); + + it("refuses a flow that only answered the first question", () => { + expect( + everyOnboardingQuestionSettled(withStatuses("pending", { who: "done" })), + ).toBe(false); + }); + + it("refuses a flow with any one question still waiting", () => { + for (const id of ONBOARDING_QUESTION_STEP_IDS) { + expect( + everyOnboardingQuestionSettled( + withStatuses("done", { [id]: "pending" }), + ), + `\`${id}\` still pending should not read as a finished questionnaire`, + ).toBe(false); + } + }); + + it("accepts a deliberate pass as an answer", () => { + expect( + everyOnboardingQuestionSettled(withStatuses("skipped", { who: "done" })), + ).toBe(true); + }); + + it("ignores what happens after the confirm screen", () => { + expect( + everyOnboardingQuestionSettled( + withStatuses("done", { + confirm: "pending", + "first-result": "pending", + done: "pending", + }), + ), + ).toBe(true); + }); +}); diff --git a/src/lib/onboarding/needs.ts b/src/lib/onboarding/needs.ts index be7606a9b..97a1282ef 100644 --- a/src/lib/onboarding/needs.ts +++ b/src/lib/onboarding/needs.ts @@ -296,6 +296,37 @@ export function parseOnboardingFirstResult( }; } +/** + * The steps that are QUESTIONS — everything the flow puts to the person before + * the confirm screen. + * + * Sliced out of the ordered list rather than written out again, so a question + * added to the flow joins this set on the day it lands and the completion gate + * below cannot fall behind the screens. + */ +export const ONBOARDING_QUESTION_STEP_IDS: readonly OnboardingStepId[] = + ONBOARDING_STEP_IDS.slice(0, ONBOARDING_STEP_IDS.indexOf("confirm")); + +/** + * Has the questionnaire been finished? + * + * Every question answered or deliberately passed — not "Q1 is set". The + * difference matters because the answers become a module map: deriving from a + * flow somebody abandoned after the first screen reads every unanswered + * question as its conservative default and switches off the surfaces that + * default implies, and the derivation latch then makes that unrepeatable. + * + * `who` cannot be passed (the schema gives it no skip arm), so "not pending" + * means "answered" for it and "answered or passed" for the rest. + */ +export function everyOnboardingQuestionSettled( + steps: readonly OnboardingStepState[], +): boolean { + return ONBOARDING_QUESTION_STEP_IDS.every( + (id) => steps.find((step) => step.id === id)?.status !== "pending", + ); +} + /** True once every step has been answered or deliberately passed. */ export function everyOnboardingStepSettled( steps: readonly OnboardingStepState[], diff --git a/src/lib/openapi/routes/onboarding.ts b/src/lib/openapi/routes/onboarding.ts index 2474b4814..01da443e5 100644 --- a/src/lib/openapi/routes/onboarding.ts +++ b/src/lib/openapi/routes/onboarding.ts @@ -376,7 +376,7 @@ export const onboardingPaths: NonNullable = { tags: ["Onboarding"], summary: "Stamp onboarding as complete, and derive the module map", description: - "Marks onboarding finished and saves whatever profile fields came with it, then clears the proxy-readable pending cookie so the next navigation stops redirecting to `/onboarding`.\n\nv1.39 — this is also the needs-based flow's confirm endpoint, and the two halves live side by side. For a record that answered the questions (Q1 is set), it derives that record's module map from the answers and applies it, ONCE: the derivation marker is what stops a second confirm re-applying the questionnaire over decisions taken in Settings since, and `POST /api/onboarding/restart` clears the marker when the person asks for the questions again. The merge is one-directional — a module the answers name is switched on, a module they do not name is switched off only where the record does not already carry an explicit on. The `cycle` key is not written here at all: it delegates to the cycle profile, which this route sets to `true` when the cycle area was chosen and never to `false`. The response then carries `onboarding` beside `completed`; a caller that never entered the needs flow gets the fixed acknowledgement it always did.\n\nThe older sibling of `POST /api/onboarding/step`. It is unconditional in a way the step route is not: it stamps the completion whatever the stored step is, it re-stamps on every call rather than refusing a second one, and it writes no audit row and enforces no rate limit of its own. It also does NOT seed the dashboard from the goal selection — that only happens on the step route's completing call.\n\nCalling this mid-wizard makes every remaining `POST /api/onboarding/step` answer 409 `onboarding.step.completed`, because that route's guarded update requires the completion stamp to still be null.\n\nCookie or wildcard Bearer; `userId` is never read from the body.", + "Marks onboarding finished and saves whatever profile fields came with it, then clears the proxy-readable pending cookie so the next navigation stops redirecting to `/onboarding`.\n\nv1.39 — this is also the needs-based flow's confirm endpoint, and the two halves live side by side. For a record that FINISHED the questions — every one of them answered or deliberately passed — it derives that record's module map from the answers and applies it, ONCE: the derivation marker is what stops a second confirm re-applying the questionnaire over decisions taken in Settings since, and `POST /api/onboarding/restart` clears the marker when the person asks for the questions again. The merge is one-directional — a module the answers name is switched on, a module they do not name is switched off only where the record does not already carry an explicit on. The `cycle` key is not written here at all: it delegates to the cycle profile, which this route sets to `true` when the cycle area was chosen and never to `false`. A half-answered flow derives nothing and is not stamped as complete: the answers become a module map, and a flow somebody abandoned after the first screen would have every remaining question read as its conservative default. The same gate is what keeps the LEGACY wizard's completion out of the derivation, since that wizard never puts these questions.\n\nThe response then carries `onboarding` beside `completed`; a caller that never entered the needs flow gets the fixed acknowledgement it always did.\n\nThe older sibling of `POST /api/onboarding/step`. It is unconditional in a way the step route is not: it stamps the completion whatever the stored step is, it re-stamps on every call rather than refusing a second one, and it writes no audit row and enforces no rate limit of its own. It also does NOT seed the dashboard from the goal selection — that only happens on the step route's completing call.\n\nCalling this mid-wizard makes every remaining `POST /api/onboarding/step` answer 409 `onboarding.step.completed`, because that route's guarded update requires the completion stamp to still be null.\n\nCookie or wildcard Bearer; `userId` is never read from the body.", requestBody: { required: true, content: { "application/json": { schema: onboardingCompleteRequest } }, diff --git a/tests/integration/onboarding-needs.test.ts b/tests/integration/onboarding-needs.test.ts index 8d9771d55..e4277eacc 100644 --- a/tests/integration/onboarding-needs.test.ts +++ b/tests/integration/onboarding-needs.test.ts @@ -127,13 +127,18 @@ function statusOf(state: OnboardingStateDto, id: OnboardingStepId) { return state.steps.find((step) => step.id === id)?.status; } -/** The five answers a "blood pressure and medication" setup gives. */ +/** + * The answers a "blood pressure and medication" setup gives — all six + * questions, because the confirm screen is only reached once every one of them + * has an answer or a deliberate pass. + */ async function answerTheQuestions() { await patchAnswer({ step: "who", recordTarget: "me" }); await patchAnswer({ step: "areas", areas: ["blood-pressure", "labs"] }); await patchAnswer({ step: "medication", medication: "yes" }); await patchAnswer({ step: "sources", status: "skipped" }); await patchAnswer({ step: "visit", visit: "no" }); + await patchAnswer({ step: "units", status: "skipped" }); } async function modulePrefs(userId: string) { @@ -339,6 +344,40 @@ describe("POST /api/onboarding/complete", () => { expect(profile.cycleTrackingEnabled).toBe(true); }); + it("derives nothing from a questionnaire that was never finished", async () => { + const user = await makeUser("half"); + await signIn(user.id); + // Q1 and nothing else — the tab was closed, or the legacy four-step wizard + // posted here afterwards. Either way the remaining five questions have no + // answer, and reading them as their conservative defaults would switch off + // eleven surfaces and latch the result. + await patchAnswer({ step: "who", recordTarget: "me" }); + + const res = await postComplete(); + expect(res.status).toBe(200); + + const row = await getPrismaClient().user.findUniqueOrThrow({ + where: { id: user.id }, + select: { modulePreferencesJson: true }, + }); + expect(row.modulePreferencesJson).toBeNull(); + + const record = await getPrismaClient().onboardingRecord.findUniqueOrThrow({ + where: { userId: user.id }, + }); + expect(record.modulesDerivedAt).toBeNull(); + expect(record.completedAt).toBeNull(); + + const state = await readState(res); + expect(statusOf(state, "confirm")).toBe("pending"); + + // Finishing the questions later still derives, so the gate defers the + // work rather than losing it. + await answerTheQuestions(); + await postComplete(); + expect((await modulePrefs(user.id)).medications).toBe(true); + }); + it("leaves a record that never answered the questions exactly as it was", async () => { const user = await makeUser("legacy"); await signIn(user.id); From 2d2350252f36888dbf056d558d7d33dbd345b123 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 10 Sep 2026 11:48:27 +0200 Subject: [PATCH 09/15] fix(onboarding): never switch a module off on a record that already uses it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The derivation wrote an explicit off for every module the answers did not name, which is right for a record being set up and wrong for one with years of rows behind it. An established self-hoster has, for most modules, no stored preference at all — they were default-on and nobody ever touched the toggle — so the first confirm took away the navigation entry, the dashboard widget and the settings entry for domains full of content. The rows survived; the surfaces did not. A derived off now asks first, and only for the modules it would actually retract. Ownership is not re-answered: the measurement domains come from the same map the MCP reads and the coach snapshot gates off, and each remaining module names the one table its content lives in, exhaustively, so a module added later fails to compile until somebody says where its content is. A module whose domain holds rows keeps whatever the record already had, because absence is what "on" looks like in the stored map. --- src/app/api/onboarding/complete/route.ts | 22 +- .../__tests__/onboarding-needs-map.test.ts | 59 ++++- src/lib/modules/domain-data.ts | 225 ++++++++++++++++++ src/lib/modules/measurement-scope.ts | 17 ++ src/lib/modules/registry.ts | 16 +- tests/integration/onboarding-needs.test.ts | 38 +++ 6 files changed, 372 insertions(+), 5 deletions(-) create mode 100644 src/lib/modules/domain-data.ts diff --git a/src/app/api/onboarding/complete/route.ts b/src/app/api/onboarding/complete/route.ts index d34e644f8..aa9eb6813 100644 --- a/src/app/api/onboarding/complete/route.ts +++ b/src/app/api/onboarding/complete/route.ts @@ -6,9 +6,11 @@ import { setOnboardingPendingCookie } from "@/lib/auth/session"; import { invalidateUserHealthScore } from "@/lib/cache/invalidate"; import { getOrCreateCycleProfile } from "@/lib/cycle/profile"; import { normalisePrefs } from "@/lib/modules/gate"; +import { modulesHoldingRecordData } from "@/lib/modules/domain-data"; import { deriveOnboardingModuleDefaults, mergeDerivedModulePreferences, + OWNED_MODULE_KEYS, } from "@/lib/modules/registry"; import { everyOnboardingQuestionSettled, @@ -148,6 +150,7 @@ async function completeNeedsFlow(userId: string, held: HeldUnitPreferences) { } let derived = false; + let keptForData: string[] = []; if (record.modulesDerivedAt === null) { const defaults = deriveOnboardingModuleDefaults({ recordTarget: state.needs.recordTarget, @@ -163,9 +166,22 @@ async function completeNeedsFlow(userId: string, held: HeldUnitPreferences) { where: { id: userId }, select: { modulePreferencesJson: true }, }); + // Only the modules the answers would switch OFF are worth asking about, + // and a record that already holds rows in one of those domains keeps its + // surface: the flow orders a record, and it cannot order away content + // somebody already has. + const wouldSwitchOff = OWNED_MODULE_KEYS.filter( + (key) => defaults.preferences[key] === false, + ); + const holdsData = await modulesHoldingRecordData( + prisma, + userId, + wouldSwitchOff, + ); const merged = mergeDerivedModulePreferences( normalisePrefs(row?.modulePreferencesJson), defaults.preferences, + holdsData, ); await prisma.user.update({ where: { id: userId }, @@ -187,6 +203,7 @@ async function completeNeedsFlow(userId: string, held: HeldUnitPreferences) { // A module toggle adds or removes a Health Score pillar, so the cached // composite was computed from a composition that no longer holds. invalidateUserHealthScore(userId); + keptForData = [...holdsData]; derived = true; } @@ -208,7 +225,10 @@ async function completeNeedsFlow(userId: string, held: HeldUnitPreferences) { annotate({ action: { name: "onboarding.needs.complete" }, - meta: { outcome: derived ? "derived" : "already_derived" }, + meta: { + outcome: derived ? "derived" : "already_derived", + keptForData: keptForData.length, + }, }); return toOnboardingStateDto(written, held); diff --git a/src/lib/modules/__tests__/onboarding-needs-map.test.ts b/src/lib/modules/__tests__/onboarding-needs-map.test.ts index 79dbaa825..342fc2bd0 100644 --- a/src/lib/modules/__tests__/onboarding-needs-map.test.ts +++ b/src/lib/modules/__tests__/onboarding-needs-map.test.ts @@ -24,6 +24,9 @@ import { type OnboardingModuleNeeds, } from "../registry"; +/** A record with nothing in it — the state a first setup describes. */ +const NO_DATA: ReadonlySet = new Set(); + function needs( overrides: Partial = {}, ): OnboardingModuleNeeds { @@ -174,6 +177,7 @@ describe("mergeDerivedModulePreferences", () => { const merged = mergeDerivedModulePreferences( { mcp: true, labs: true }, preferences, + NO_DATA, ); expect(merged.mcp).toBe(true); expect(merged.labs).toBe(true); @@ -181,7 +185,7 @@ describe("mergeDerivedModulePreferences", () => { it("does turn off a module the record only had by default", () => { const { preferences } = deriveOnboardingModuleDefaults(needs()); - const merged = mergeDerivedModulePreferences({}, preferences); + const merged = mergeDerivedModulePreferences({}, preferences, NO_DATA); expect(merged.labs).toBe(false); expect(merged.workouts).toBe(false); }); @@ -190,7 +194,11 @@ describe("mergeDerivedModulePreferences", () => { const { preferences } = deriveOnboardingModuleDefaults( needs({ areas: ["labs"] }), ); - const merged = mergeDerivedModulePreferences({ labs: false }, preferences); + const merged = mergeDerivedModulePreferences( + { labs: false }, + preferences, + NO_DATA, + ); expect(merged.labs).toBe(true); }); @@ -199,16 +207,61 @@ describe("mergeDerivedModulePreferences", () => { const merged = mergeDerivedModulePreferences( { weight: false, nonsense: true }, preferences, + NO_DATA, ); expect(merged).not.toHaveProperty("weight"); expect(merged).not.toHaveProperty("nonsense"); }); + it("never switches off a domain the record already holds rows in", () => { + const { preferences } = deriveOnboardingModuleDefaults(needs()); + expect(preferences.labs).toBe(false); + expect(preferences.mood).toBe(false); + + // An established record with no stored preference at all: default-on, and + // nobody ever touched the toggle. A derived `false` would take the nav + // entry, the dashboard widget and the settings entry away from a domain + // holding years of rows, so the key is left exactly as it was — and + // absence means on. + const merged = mergeDerivedModulePreferences( + {}, + preferences, + new Set(["labs", "mood"]), + ); + expect(merged).not.toHaveProperty("labs"); + expect(merged).not.toHaveProperty("mood"); + expect(merged.workouts).toBe(false); + }); + + it("keeps a stored explicit off where the domain holds rows", () => { + const { preferences } = deriveOnboardingModuleDefaults(needs()); + // Somebody switched labs off by hand and still has old results. The flow + // must not switch it back on, and must not restate the decision either. + const merged = mergeDerivedModulePreferences( + { labs: false }, + preferences, + new Set(["labs"]), + ); + expect(merged.labs).toBe(false); + }); + + it("still switches a domain ON when the answers name it, rows or not", () => { + const { preferences } = deriveOnboardingModuleDefaults( + needs({ areas: ["labs"] }), + ); + const merged = mergeDerivedModulePreferences( + {}, + preferences, + new Set(["labs"]), + ); + expect(merged.labs).toBe(true); + }); + it("leaves the two delegated keys alone in both directions", () => { const { preferences } = deriveOnboardingModuleDefaults( needs({ areas: ["cycle"] }), ); - const merged = mergeDerivedModulePreferences({}, preferences); + const merged = mergeDerivedModulePreferences({}, preferences, NO_DATA); expect(merged).not.toHaveProperty("cycle"); expect(merged).not.toHaveProperty("coach"); }); diff --git a/src/lib/modules/domain-data.ts b/src/lib/modules/domain-data.ts new file mode 100644 index 000000000..cadb16efd --- /dev/null +++ b/src/lib/modules/domain-data.ts @@ -0,0 +1,225 @@ +/** + * v1.39 (C1) — does a record already hold data in a module's domain? + * + * The setup flow's derivation writes an explicit `false` for every module the + * answers did not name, and that is the right answer for a record that is + * being set up. It is the wrong answer for a record that already has years of + * rows: an established self-hoster upgrading into the flow has, for most + * modules, no stored preference at all — they were default-on and nobody ever + * touched the toggle — so the first confirm would switch off the nav entry, + * the dashboard widget and the settings entry for domains that hold content. + * The rows would survive; the surfaces would not. The design spec's second + * principle is that nothing is lost by a choice, and it says that about a NEW + * record; it says nothing about one with content, so the derivation asks here + * before it retracts anything. + * + * The ownership question is not re-answered here. The measurement-level + * domains come from {@link measurementTypesOwnedBy}, which is the same map the + * MCP reads and the Coach snapshot gate off, so a type that changes hands + * cannot drift out of sync with this. What this file adds is the per-module + * table each domain's non-measurement content lives in, one named entry per + * module, exhaustive over `OwnedModuleKey` so a module added next year fails + * to compile until somebody names where its content lives. + */ +import type { PrismaClient } from "@/generated/prisma/client"; + +import { measurementTypesOwnedBy } from "./measurement-scope"; +import type { OwnedModuleKey } from "./registry"; + +/** The delegates this file reads. Narrow so a worker's client fits too. */ +type DomainDataClient = Pick< + PrismaClient, + | "measurement" + | "moodEntry" + | "workout" + | "labResult" + | "illnessEpisode" + | "userAchievement" + | "insightNarrative" + | "medication" + | "encounter" + | "environmentContext" + | "mcpOAuthConnection" + | "inboundDocument" + | "mentalHealthAssessment" + | "nutrientIntakeDay" + | "vaccinationRecord" +>; + +type DomainProbe = ( + client: DomainDataClient, + recordId: string, +) => Promise; + +/** One indexed existence read; a soft-deleted row is not content. */ +async function anyRow(find: PromiseLike): Promise { + return (await find) !== null; +} + +/** Does the record hold any measurement in the types this module owns? */ +function measurementProbe(key: OwnedModuleKey): DomainProbe { + const types = measurementTypesOwnedBy(key); + return async (client, recordId) => { + if (types.length === 0) return false; + return anyRow( + client.measurement.findFirst({ + where: { userId: recordId, deletedAt: null, type: { in: types } }, + select: { id: true }, + }), + ); + }; +} + +/** Either the module's own table or the measurements it owns. */ +function eitherProbe(a: DomainProbe, b: DomainProbe): DomainProbe { + return async (client, recordId) => + (await a(client, recordId)) || (await b(client, recordId)); +} + +/** + * Where each module's content lives. + * + * Exhaustive by type. Every entry names one table (or one measurement domain) + * whose rows mean "this record uses this module", and nothing here reads a + * preference, a connection state or a cache — the question is content, not + * configuration. + */ +const MODULE_DOMAIN_PROBES: Record = { + mood: eitherProbe( + (client, recordId) => + anyRow( + client.moodEntry.findFirst({ + where: { userId: recordId, deletedAt: null }, + select: { id: true }, + }), + ), + measurementProbe("mood"), + ), + sleep: measurementProbe("sleep"), + glucose: measurementProbe("glucose"), + workouts: eitherProbe( + (client, recordId) => + anyRow( + client.workout.findFirst({ + where: { userId: recordId }, + select: { id: true }, + }), + ), + measurementProbe("workouts"), + ), + recovery: measurementProbe("recovery"), + labs: (client, recordId) => + anyRow( + client.labResult.findFirst({ + where: { userId: recordId, deletedAt: null }, + select: { id: true }, + }), + ), + illness: (client, recordId) => + anyRow( + client.illnessEpisode.findFirst({ + where: { userId: recordId, deletedAt: null }, + select: { id: true }, + }), + ), + achievements: (client, recordId) => + anyRow( + client.userAchievement.findFirst({ + where: { userId: recordId }, + select: { id: true }, + }), + ), + insights: (client, recordId) => + anyRow( + client.insightNarrative.findFirst({ + where: { userId: recordId }, + select: { id: true }, + }), + ), + medications: (client, recordId) => + anyRow( + client.medication.findFirst({ + where: { userId: recordId }, + select: { id: true }, + }), + ), + // The doctor report is built from the visits, so a recorded encounter is + // what "this record uses the report surface" looks like in the data. + doctorReport: (client, recordId) => + anyRow( + client.encounter.findFirst({ + where: { userId: recordId, deletedAt: null }, + select: { id: true }, + }), + ), + environment: eitherProbe( + (client, recordId) => + anyRow( + client.environmentContext.findFirst({ + where: { userId: recordId }, + select: { id: true }, + }), + ), + measurementProbe("environment"), + ), + // The endpoint holds no content of its own; a live authorisation is the + // nearest thing, and switching it off under a connected assistant would + // break a wire somebody set up on purpose. + mcp: (client, recordId) => + anyRow( + client.mcpOAuthConnection.findFirst({ + where: { userId: recordId }, + select: { id: true }, + }), + ), + inboundDocuments: (client, recordId) => + anyRow( + client.inboundDocument.findFirst({ + where: { userId: recordId, deletedAt: null }, + select: { id: true }, + }), + ), + mentalHealth: (client, recordId) => + anyRow( + client.mentalHealthAssessment.findFirst({ + where: { userId: recordId, deletedAt: null }, + select: { id: true }, + }), + ), + nutrients: (client, recordId) => + anyRow( + client.nutrientIntakeDay.findFirst({ + where: { userId: recordId }, + // Keyed on the day and the nutrient, not on an id of its own. + select: { userId: true }, + }), + ), + vaccinations: (client, recordId) => + anyRow( + client.vaccinationRecord.findFirst({ + where: { userId: recordId, deletedAt: null }, + select: { id: true }, + }), + ), +}; + +/** + * Which of `candidates` already hold data for this record. + * + * Only the candidates are read, because the caller only ever asks about the + * modules it is about to switch off — a confirm that names everything reads + * nothing. Each probe is one indexed existence read. + */ +export async function modulesHoldingRecordData( + client: DomainDataClient, + recordId: string, + candidates: readonly OwnedModuleKey[], +): Promise> { + const holding = new Set(); + await Promise.all( + candidates.map(async (key) => { + if (await MODULE_DOMAIN_PROBES[key](client, recordId)) holding.add(key); + }), + ); + return holding; +} diff --git a/src/lib/modules/measurement-scope.ts b/src/lib/modules/measurement-scope.ts index 58d4b58ac..2ff632964 100644 --- a/src/lib/modules/measurement-scope.ts +++ b/src/lib/modules/measurement-scope.ts @@ -224,3 +224,20 @@ export function moduleForMeasurementType( ): ModuleKey | null { return OWNER_BY_MEASUREMENT_TYPE.get(type) ?? null; } + +/** + * The inverse: every measurement type `key` owns, or an empty list for a + * module that owns no measurement-level domain. + * + * Derived from the same map {@link moduleForMeasurementType} answers from, so + * the two can never disagree about who owns what. Read by the setup flow's + * derivation, which asks "does this record already hold data in that domain" + * before it switches a module off. + */ +export function measurementTypesOwnedBy(key: ModuleKey): MeasurementType[] { + const out: MeasurementType[] = []; + for (const [type, owner] of OWNER_BY_MEASUREMENT_TYPE) { + if (owner === key) out.push(type); + } + return out; +} diff --git a/src/lib/modules/registry.ts b/src/lib/modules/registry.ts index b2346dc6e..281944342 100644 --- a/src/lib/modules/registry.ts +++ b/src/lib/modules/registry.ts @@ -579,7 +579,7 @@ export function deriveOnboardingModuleDefaults( /** * Merge a derived map into the one the record already holds, without ever - * retracting a decision the person made by hand. + * retracting a decision the person made by hand or a domain they already use. * * The rule is one-directional: a derived `true` always lands, a derived * `false` lands only where the stored map does not already carry an explicit @@ -587,6 +587,18 @@ export function deriveOnboardingModuleDefaults( * and did not tick anything that implies it, keeps it on — the questionnaire * orders a record, it does not undo it. * + * `holdsData` is the second thing a derived `false` cannot cross, and it is + * about a record with content rather than a record with an opinion. An + * established account upgrading into the setup flow has, for most modules, no + * stored preference at all — they were default-on and nobody ever touched the + * toggle — so without this the first confirm would switch off the navigation + * entry, the dashboard widget and the settings entry for every domain the + * person did not tick, including the ones holding years of rows. The design + * spec's "nothing is lost by a choice" is written about a NEW record; for one + * that already has content the honest answer is to write nothing and leave the + * stored value exactly where it was. Absence means ON, so leaving the key + * alone is what keeps the surface. + * * Keys the stored map carries that are not module keys are dropped; the caller * is expected to have normalised it (`normalisePrefs`), and this is the second * belt on the same trousers rather than a place to be clever. @@ -594,6 +606,7 @@ export function deriveOnboardingModuleDefaults( export function mergeDerivedModulePreferences( existing: Readonly>, derived: Readonly>, + holdsData: ReadonlySet, ): Record { const merged: Record = {}; for (const [key, value] of Object.entries(existing)) { @@ -605,6 +618,7 @@ export function mergeDerivedModulePreferences( continue; } if (merged[key] === true) continue; // switched on by hand — never retracted + if (holdsData.has(key)) continue; // the domain holds rows — never retracted merged[key] = false; } return merged; diff --git a/tests/integration/onboarding-needs.test.ts b/tests/integration/onboarding-needs.test.ts index e4277eacc..01b5ae15f 100644 --- a/tests/integration/onboarding-needs.test.ts +++ b/tests/integration/onboarding-needs.test.ts @@ -331,6 +331,44 @@ describe("POST /api/onboarding/complete", () => { expect((await modulePrefs(user.id)).mcp).toBe(true); }); + it("never switches off a domain the record already holds rows in", async () => { + const user = await makeUser("established"); + await signIn(user.id); + // An account that has been running for years: mood entries and workouts, + // no stored module preference at all, because both were default-on and + // nobody ever touched a toggle. The questions below name neither. + await getPrismaClient().moodEntry.create({ + data: { + userId: user.id, + date: "2026-09-01", + mood: "GUT", + score: 4, + moodLoggedAt: new Date("2026-09-01T18:00:00.000Z"), + }, + }); + await getPrismaClient().workout.create({ + data: { + userId: user.id, + sportType: "RUNNING", + startedAt: new Date("2026-09-01T06:00:00.000Z"), + endedAt: new Date("2026-09-01T06:30:00.000Z"), + durationSec: 1800, + }, + }); + + await answerTheQuestions(); + await postComplete(); + + const prefs = await modulePrefs(user.id); + // Left alone rather than written false: absence is what "on" looks like + // in the stored map, and the surfaces stay. + expect(prefs).not.toHaveProperty("mood"); + expect(prefs).not.toHaveProperty("workouts"); + // A domain with nothing in it is still switched off by the answers. + expect(prefs.illness).toBe(false); + expect(prefs.vaccinations).toBe(false); + }); + it("switches cycle tracking on through its own column when the area is chosen", async () => { const user = await makeUser("cycle"); await signIn(user.id); From 07f1b6ce92b37498643cf15c14957a5b0623d2ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 10 Sep 2026 11:54:19 +0200 Subject: [PATCH 10/15] refactor(modules): one record-keyed write for a record's module state MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The guardian's modules route and the setup flow's confirm both decide a record's modules, and each had its own copy of what that means: write the preference blob, write the delegated cycle column, invalidate the health score. Two copies of a three-step write drift, and the second one was added by this branch. They share one function now, keyed on the record, which is the carrier the managed-profile work needs. Authentication stays at the routes and stays different on purpose — the guardian route acts on a managed record, the setup routes refuse outright under a switch until a guardian's route into a record's setup lands with the rest of that record's configuration. So does the merge: what the answers imply and what somebody just ticked are different questions about what may be overwritten. The actor's own modules route keeps its own write, because that one is guarded on a concurrency token the record-keyed callers do not carry. --- src/app/api/onboarding/complete/route.ts | 30 +++---- src/app/api/record-settings/[family]/route.ts | 38 +++------ src/lib/record-settings/modules.ts | 85 +++++++++++++++++++ 3 files changed, 110 insertions(+), 43 deletions(-) create mode 100644 src/lib/record-settings/modules.ts diff --git a/src/app/api/onboarding/complete/route.ts b/src/app/api/onboarding/complete/route.ts index aa9eb6813..50a4563c4 100644 --- a/src/app/api/onboarding/complete/route.ts +++ b/src/app/api/onboarding/complete/route.ts @@ -3,8 +3,6 @@ import { apiHandler, requireAuth } from "@/lib/api-handler"; import { annotate } from "@/lib/logging/context"; import { apiSuccess, apiError, safeJson } from "@/lib/api-response"; import { setOnboardingPendingCookie } from "@/lib/auth/session"; -import { invalidateUserHealthScore } from "@/lib/cache/invalidate"; -import { getOrCreateCycleProfile } from "@/lib/cycle/profile"; import { normalisePrefs } from "@/lib/modules/gate"; import { modulesHoldingRecordData } from "@/lib/modules/domain-data"; import { @@ -23,6 +21,7 @@ import { readOnboardingRecordState, toOnboardingStateDto, } from "@/lib/onboarding/needs-store"; +import { writeRecordModulePreferences } from "@/lib/record-settings/modules"; import { NextRequest } from "next/server"; import { z } from "zod/v4"; import { onboardingCompleteSchema } from "@/lib/validations/onboarding"; @@ -183,26 +182,19 @@ async function completeNeedsFlow(userId: string, held: HeldUnitPreferences) { defaults.preferences, holdsData, ); - await prisma.user.update({ - where: { id: userId }, - data: { modulePreferencesJson: merged }, - }); - - // `cycle` delegates to `CycleProfile.cycleTrackingEnabled`, so the module - // blob never owns it. Only ever switched ON here: the gate derives an + // The same record-keyed write the guardian's modules route uses, so the + // two surfaces that decide a record's modules cannot drift apart on how + // the decision is stored or what it invalidates. `cycle` delegates to + // `CycleProfile.cycleTrackingEnabled`, so the module blob never owns it, + // and it is only ever switched ON from here: the gate derives an // unanswered cycle from recorded sex, and an untick is not a request to // retract that. - if (defaults.cycleTracking) { - await getOrCreateCycleProfile(userId); - await prisma.cycleProfile.update({ - where: { userId }, - data: { cycleTrackingEnabled: true }, - }); - } + await writeRecordModulePreferences({ + recordId: userId, + modulePreferences: merged, + ...(defaults.cycleTracking ? { cycleTrackingEnabled: true } : {}), + }); - // A module toggle adds or removes a Health Score pillar, so the cached - // composite was computed from a composition that no longer holds. - invalidateUserHealthScore(userId); keptForData = [...holdsData]; derived = true; } diff --git a/src/app/api/record-settings/[family]/route.ts b/src/app/api/record-settings/[family]/route.ts index 089715897..5b91fa3de 100644 --- a/src/app/api/record-settings/[family]/route.ts +++ b/src/app/api/record-settings/[family]/route.ts @@ -14,7 +14,6 @@ import { } from "@/lib/api-response"; import { auditLog } from "@/lib/auth/audit"; import { - invalidateUserHealthScore, invalidateUserInsightsLayout, invalidateUserMedications, invalidateUserProfile, @@ -24,6 +23,7 @@ import { getAllEffectiveRanges, type ThresholdOverridesJson, } from "@/lib/analytics/effective-range"; +import { writeRecordModulePreferences } from "@/lib/record-settings/modules"; import { isManagedRecordSettingsFamily, managedModulePreferencesFrom, @@ -31,7 +31,6 @@ import { resolveGuardianRecordSettingsAccess, } from "@/lib/record-settings"; import { isCycleEnabled } from "@/lib/cycle/gate"; -import { getOrCreateCycleProfile } from "@/lib/cycle/profile"; import { resolveInsightsLayout, serializeInsightsLayout, @@ -251,34 +250,25 @@ export const PATCH = apiHandler( ...managedModulePreferencesFrom(current?.modulePreferencesJson), ...patch.modulePreferences, }; - if (patch.modulePreferences !== undefined) { - await prisma.user.update({ - where: { id: access.recordId }, - data: { modulePreferencesJson: toJson(modulePreferences) }, - }); - } - // The delegated key writes its own column. An explicit boolean is + // One write path, shared with the setup flow's derivation. The + // delegated key writes its own column there: an explicit boolean is // what the cycle gate honours over the sex-derived default, so a // guardian turning Cycle off for a record whose sex says otherwise // is recorded as the deliberate answer it is. - if (patch.cycleTrackingEnabled !== undefined) { - await getOrCreateCycleProfile(access.recordId); - await prisma.cycleProfile.update({ - where: { userId: access.recordId }, - data: { cycleTrackingEnabled: patch.cycleTrackingEnabled }, - }); - } - const cycleProfile = await prisma.cycleProfile.findUnique({ - where: { userId: access.recordId }, - select: { cycleTrackingEnabled: true }, + const written = await writeRecordModulePreferences({ + recordId: access.recordId, + ...(patch.modulePreferences !== undefined + ? { modulePreferences } + : {}), + ...(patch.cycleTrackingEnabled !== undefined + ? { cycleTrackingEnabled: patch.cycleTrackingEnabled } + : {}), }); - invalidateUserHealthScore(access.recordId); settings = { modulePreferences, - cycleTrackingEnabled: isCycleEnabled( - current?.gender ?? null, - cycleProfile, - ), + cycleTrackingEnabled: isCycleEnabled(current?.gender ?? null, { + cycleTrackingEnabled: written.cycleTrackingEnabled, + }), }; // The module keys themselves, not the wrapper field: the record's // trail should say which module moved. `cycleTrackingEnabled` is diff --git a/src/lib/record-settings/modules.ts b/src/lib/record-settings/modules.ts new file mode 100644 index 000000000..13428f2ad --- /dev/null +++ b/src/lib/record-settings/modules.ts @@ -0,0 +1,85 @@ +/** + * v1.39 (C1) — the one write path for a record's module state. + * + * Two surfaces decide a record's modules and they must not disagree about how + * the decision is stored. `PATCH /api/record-settings/modules` (v1.38.15) is a + * guardian switching a managed record's modules by hand; the setup flow's + * confirm screen derives a map from the answers. Both write the same two + * places — the record's `modulePreferencesJson` blob and, for the delegated + * cycle key, the record's own `CycleProfile` column — and both have to + * invalidate the same cache, because a module toggle adds or removes a Health + * Score pillar and the cached composite was computed from a composition that + * no longer holds. That is what lives here, keyed on the RECORD, which is the + * carrier issue #939 point 2 needs. + * + * What deliberately does NOT live here is who may call it. The two callers + * authenticate differently and that is not an oversight: the record-settings + * route is a guardian acting on a managed record (`requireGuardianAuth`), and + * the setup routes are actor-only (`requireAuth()`, which refuses outright + * under an acting-account switch) until a guardian's route into a managed + * record's setup arrives with the rest of that record's configuration. A + * delegable declaration is a reviewed entry in the sharing surface guard, not + * something a shared write helper may hand out. + * + * `PATCH /api/auth/me/modules` is deliberately not routed through here: it is + * the ACTOR's own record, and its write is guarded on an optimistic-concurrency + * base token the two record-keyed callers do not carry. Folding it in would + * mean either dropping that guard or making it a parameter nobody but one + * caller sets. + * + * Nor does the merge: "what the answers imply" and "what a guardian just + * ticked" are different questions with different rules about what may be + * overwritten, so each caller computes its own complete map and this writes + * it. + */ +import { invalidateUserHealthScore } from "@/lib/cache/invalidate"; +import { getOrCreateCycleProfile } from "@/lib/cycle/profile"; +import { prisma, toJson } from "@/lib/db"; + +export interface RecordModuleWrite { + /** The record whose modules these are — never the actor, under a switch. */ + recordId: string; + /** The complete map to persist, or undefined to leave the blob alone. */ + modulePreferences?: Record; + /** + * An explicit answer for the delegated cycle key, or undefined to leave the + * column alone. The blob never owns this key; the profile column does. + */ + cycleTrackingEnabled?: boolean; +} + +/** + * Persist a record's module state and invalidate what it invalidates. + * + * Returns the cycle profile's stored opt-in afterwards (null when the record + * has no profile row), so a caller that publishes the resolved cycle state can + * resolve it against the record's sex without a second read. + */ +export async function writeRecordModulePreferences( + input: RecordModuleWrite, +): Promise<{ cycleTrackingEnabled: boolean | null }> { + if (input.modulePreferences !== undefined) { + await prisma.user.update({ + where: { id: input.recordId }, + data: { modulePreferencesJson: toJson(input.modulePreferences) }, + }); + } + if (input.cycleTrackingEnabled !== undefined) { + await getOrCreateCycleProfile(input.recordId); + await prisma.cycleProfile.update({ + where: { userId: input.recordId }, + data: { cycleTrackingEnabled: input.cycleTrackingEnabled }, + }); + } + + const profile = await prisma.cycleProfile.findUnique({ + where: { userId: input.recordId }, + select: { cycleTrackingEnabled: true }, + }); + + // A module toggle adds or removes a Health Score pillar, so the cached + // composite was computed from a composition that no longer holds. + invalidateUserHealthScore(input.recordId); + + return { cycleTrackingEnabled: profile?.cycleTrackingEnabled ?? null }; +} From b10a3ef22e4521aa207c45ad1573015f3eb8a741 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 10 Sep 2026 11:57:11 +0200 Subject: [PATCH 11/15] fix(onboarding): leave a trail for the three settings columns the flow writes The setup flow writes a record's module map, its glucose unit and its unit preference. All three have a dedicated settings route that records an audit row, and the flow recorded none, so "why did my modules change" had no answer in the activity panel. Restarting the flow did write a row, which made the gap look like an oversight rather than a decision, because it was one. Same event names as the dedicated routes, since it is the same column and the panel should group them. The module row names the keys that actually moved and the domains kept for their content. --- docs/api/openapi.yaml | 6 +++-- src/app/api/onboarding/answers/route.ts | 28 +++++++++++++++++++ src/app/api/onboarding/complete/route.ts | 31 ++++++++++++++++++++-- src/lib/openapi/routes/onboarding.ts | 2 +- tests/integration/onboarding-needs.test.ts | 20 ++++++++++++++ 5 files changed, 82 insertions(+), 5 deletions(-) diff --git a/docs/api/openapi.yaml b/docs/api/openapi.yaml index 21e04da25..c0fdbaf7c 100644 --- a/docs/api/openapi.yaml +++ b/docs/api/openapi.yaml @@ -16389,8 +16389,10 @@ paths: The older sibling of `POST /api/onboarding/step`. It is unconditional in a way the step route is not: it stamps the completion whatever the stored step is, it re-stamps on every call rather than refusing a second one, and it - writes no audit row and enforces no rate limit of its own. It also does NOT seed the dashboard from the goal - selection — that only happens on the step route's completing call. + enforces no rate limit of its own. Its legacy half writes no audit row; the needs half writes one + (`user.modules.update`) when it derives, because that is the same column the dedicated modules route audits. It + also does NOT seed the dashboard from the goal selection — that only happens on the step route's completing + call. Calling this mid-wizard makes every remaining `POST /api/onboarding/step` answer 409 diff --git a/src/app/api/onboarding/answers/route.ts b/src/app/api/onboarding/answers/route.ts index fd101d76e..ac502bea3 100644 --- a/src/app/api/onboarding/answers/route.ts +++ b/src/app/api/onboarding/answers/route.ts @@ -27,9 +27,11 @@ import { apiHandler, requireAuth } from "@/lib/api-handler"; import { apiError, apiSuccess, + getClientIp, returnAllZodIssues, safeJson, } from "@/lib/api-response"; +import { auditLog } from "@/lib/auth/audit"; import { prisma, toJson } from "@/lib/db"; import { annotate } from "@/lib/logging/context"; import { applyOnboardingAnswer } from "@/lib/onboarding/needs-apply"; @@ -146,6 +148,32 @@ export const PATCH = apiHandler(async (request: NextRequest) => { unitPreference: unitData.unitPreference ?? user.unitPreference, }); + // Both columns have a dedicated settings route that writes an audit row, and + // the trail should not depend on which surface the person changed them from. + // Same event names, so the activity panel groups them. + if (unitData.glucoseUnit !== undefined) { + await auditLog("user.glucose-unit.update", { + userId: user.id, + ipAddress: getClientIp(request), + details: { + previous: user.glucoseUnit, + next: unitData.glucoseUnit, + source: "onboarding", + }, + }); + } + if (unitData.unitPreference !== undefined) { + await auditLog("user.unit-preference.update", { + userId: user.id, + ipAddress: getClientIp(request), + details: { + previous: user.unitPreference, + next: unitData.unitPreference, + source: "onboarding", + }, + }); + } + annotate({ action: { name: "onboarding.answer.save" }, meta: { diff --git a/src/app/api/onboarding/complete/route.ts b/src/app/api/onboarding/complete/route.ts index 50a4563c4..d4d378072 100644 --- a/src/app/api/onboarding/complete/route.ts +++ b/src/app/api/onboarding/complete/route.ts @@ -1,7 +1,13 @@ import { prisma, toJson } from "@/lib/db"; import { apiHandler, requireAuth } from "@/lib/api-handler"; import { annotate } from "@/lib/logging/context"; -import { apiSuccess, apiError, safeJson } from "@/lib/api-response"; +import { + apiSuccess, + apiError, + getClientIp, + safeJson, +} from "@/lib/api-response"; +import { auditLog } from "@/lib/auth/audit"; import { setOnboardingPendingCookie } from "@/lib/auth/session"; import { normalisePrefs } from "@/lib/modules/gate"; import { modulesHoldingRecordData } from "@/lib/modules/domain-data"; @@ -98,6 +104,7 @@ export const POST = apiHandler(async (request: NextRequest) => { const onboarding = await completeNeedsFlow( user.id, readHeldUnitPreferences(user), + request, ); return apiSuccess({ completed: true, ...(onboarding ? { onboarding } : {}) }); @@ -111,7 +118,11 @@ export const POST = apiHandler(async (request: NextRequest) => { * needs flow — a legacy wizard completion then answers exactly the body it * always did. */ -async function completeNeedsFlow(userId: string, held: HeldUnitPreferences) { +async function completeNeedsFlow( + userId: string, + held: HeldUnitPreferences, + request: NextRequest, +) { const record = await prisma.onboardingRecord.findUnique({ where: { userId }, select: ONBOARDING_RECORD_SELECT, @@ -195,6 +206,22 @@ async function completeNeedsFlow(userId: string, held: HeldUnitPreferences) { ...(defaults.cycleTracking ? { cycleTrackingEnabled: true } : {}), }); + // The same trail the dedicated modules route writes, under the same event + // name, because it is the same column and the activity panel is where + // somebody goes to ask why their modules changed. The flow was writing + // three audited settings columns and leaving no trail at all, which made + // the answer to that question "nothing did it". + const stored = normalisePrefs(row?.modulePreferencesJson); + const changed = [ + ...Object.keys(merged).filter((key) => merged[key] !== stored[key]), + ...(defaults.cycleTracking ? ["cycleTrackingEnabled"] : []), + ]; + await auditLog("user.modules.update", { + userId, + ipAddress: getClientIp(request), + details: { changed, keptForData: [...holdsData], source: "onboarding" }, + }); + keptForData = [...holdsData]; derived = true; } diff --git a/src/lib/openapi/routes/onboarding.ts b/src/lib/openapi/routes/onboarding.ts index 01da443e5..ff219c68d 100644 --- a/src/lib/openapi/routes/onboarding.ts +++ b/src/lib/openapi/routes/onboarding.ts @@ -376,7 +376,7 @@ export const onboardingPaths: NonNullable = { tags: ["Onboarding"], summary: "Stamp onboarding as complete, and derive the module map", description: - "Marks onboarding finished and saves whatever profile fields came with it, then clears the proxy-readable pending cookie so the next navigation stops redirecting to `/onboarding`.\n\nv1.39 — this is also the needs-based flow's confirm endpoint, and the two halves live side by side. For a record that FINISHED the questions — every one of them answered or deliberately passed — it derives that record's module map from the answers and applies it, ONCE: the derivation marker is what stops a second confirm re-applying the questionnaire over decisions taken in Settings since, and `POST /api/onboarding/restart` clears the marker when the person asks for the questions again. The merge is one-directional — a module the answers name is switched on, a module they do not name is switched off only where the record does not already carry an explicit on. The `cycle` key is not written here at all: it delegates to the cycle profile, which this route sets to `true` when the cycle area was chosen and never to `false`. A half-answered flow derives nothing and is not stamped as complete: the answers become a module map, and a flow somebody abandoned after the first screen would have every remaining question read as its conservative default. The same gate is what keeps the LEGACY wizard's completion out of the derivation, since that wizard never puts these questions.\n\nThe response then carries `onboarding` beside `completed`; a caller that never entered the needs flow gets the fixed acknowledgement it always did.\n\nThe older sibling of `POST /api/onboarding/step`. It is unconditional in a way the step route is not: it stamps the completion whatever the stored step is, it re-stamps on every call rather than refusing a second one, and it writes no audit row and enforces no rate limit of its own. It also does NOT seed the dashboard from the goal selection — that only happens on the step route's completing call.\n\nCalling this mid-wizard makes every remaining `POST /api/onboarding/step` answer 409 `onboarding.step.completed`, because that route's guarded update requires the completion stamp to still be null.\n\nCookie or wildcard Bearer; `userId` is never read from the body.", + "Marks onboarding finished and saves whatever profile fields came with it, then clears the proxy-readable pending cookie so the next navigation stops redirecting to `/onboarding`.\n\nv1.39 — this is also the needs-based flow's confirm endpoint, and the two halves live side by side. For a record that FINISHED the questions — every one of them answered or deliberately passed — it derives that record's module map from the answers and applies it, ONCE: the derivation marker is what stops a second confirm re-applying the questionnaire over decisions taken in Settings since, and `POST /api/onboarding/restart` clears the marker when the person asks for the questions again. The merge is one-directional — a module the answers name is switched on, a module they do not name is switched off only where the record does not already carry an explicit on. The `cycle` key is not written here at all: it delegates to the cycle profile, which this route sets to `true` when the cycle area was chosen and never to `false`. A half-answered flow derives nothing and is not stamped as complete: the answers become a module map, and a flow somebody abandoned after the first screen would have every remaining question read as its conservative default. The same gate is what keeps the LEGACY wizard's completion out of the derivation, since that wizard never puts these questions.\n\nThe response then carries `onboarding` beside `completed`; a caller that never entered the needs flow gets the fixed acknowledgement it always did.\n\nThe older sibling of `POST /api/onboarding/step`. It is unconditional in a way the step route is not: it stamps the completion whatever the stored step is, it re-stamps on every call rather than refusing a second one, and it enforces no rate limit of its own. Its legacy half writes no audit row; the needs half writes one (`user.modules.update`) when it derives, because that is the same column the dedicated modules route audits. It also does NOT seed the dashboard from the goal selection — that only happens on the step route's completing call.\n\nCalling this mid-wizard makes every remaining `POST /api/onboarding/step` answer 409 `onboarding.step.completed`, because that route's guarded update requires the completion stamp to still be null.\n\nCookie or wildcard Bearer; `userId` is never read from the body.", requestBody: { required: true, content: { "application/json": { schema: onboardingCompleteRequest } }, diff --git a/tests/integration/onboarding-needs.test.ts b/tests/integration/onboarding-needs.test.ts index 01b5ae15f..beeceb41a 100644 --- a/tests/integration/onboarding-needs.test.ts +++ b/tests/integration/onboarding-needs.test.ts @@ -229,6 +229,17 @@ describe("PATCH /api/onboarding/answers", () => { select: { glucoseUnit: true, unitPreference: true }, }); expect(row).toEqual({ glucoseUnit: "mmol/L", unitPreference: "imperial" }); + + // Same trail as the two dedicated settings routes, so it does not matter + // which surface the person changed the units from. + const actions = ( + await getPrismaClient().auditLog.findMany({ + where: { userId: user.id }, + select: { action: true }, + }) + ).map((entry) => entry.action); + expect(actions).toContain("user.glucose-unit.update"); + expect(actions).toContain("user.unit-preference.update"); }); it("does not re-ask the units the account already holds", async () => { @@ -304,6 +315,15 @@ describe("POST /api/onboarding/complete", () => { expect(statusOf(state, "confirm")).toBe("done"); expect(state.completedAt).not.toBeNull(); + // The activity panel has to be able to answer "why did my modules change". + const audit = await getPrismaClient().auditLog.findMany({ + where: { userId: user.id, action: "user.modules.update" }, + }); + expect(audit).toHaveLength(1); + expect( + (audit[0].details as { changed?: string[] } | null)?.changed, + ).toContain("medications"); + // Somebody changes their mind in Settings, then the confirm screen is // replayed. The second call must leave that decision alone: the latch is // what stops a questionnaire from re-applying itself over a later choice. From f90307fd228af4746a68d2edf6cefb9f94e90946 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 10 Sep 2026 12:06:02 +0200 Subject: [PATCH 12/15] fix(auth): answer the setup state for the record the payload describes The account payload resolved the setup state for the actor while its module map, its cycle flag and every count the checklist reads beside it answer for the active record. A guardian looking at a child's dashboard therefore got the child's rows ordered by their own answers, and the child's reading count tested against their own flow state. The two halves of that component have to agree about whose record they describe, so the field is record-scoped now, like the module map next to it. Under a grant narrowed to one section the answers come back empty, for the reason the module map is masked: they name the domains the record tracks, and a grant that opens readings does not open that. The shape of the flow still comes through. The three write routes stay actor-only and say why: a delegable declaration is a reviewed entry in the sharing surface guard, and a read that is wider than its write is the honest shape while that entry is still to come. --- docs/api/openapi.yaml | 4 +- src/app/api/auth/me/route.ts | 68 ++++++++++---- .../onboarding/getting-started-checklist.tsx | 6 +- src/hooks/use-auth.ts | 11 ++- src/lib/openapi/routes/account-sharing.ts | 9 +- src/lib/openapi/routes/onboarding.ts | 2 +- tests/integration/onboarding-needs.test.ts | 93 +++++++++++++++++++ 7 files changed, 162 insertions(+), 31 deletions(-) diff --git a/docs/api/openapi.yaml b/docs/api/openapi.yaml index c0fdbaf7c..05f6899fd 100644 --- a/docs/api/openapi.yaml +++ b/docs/api/openapi.yaml @@ -63192,7 +63192,9 @@ components: additionalProperties: false description: The needs-based setup flow for one record. Always present on the account payload — a record that never entered the flow reads as empty answers with nine `pending` steps, so a client branches on the step statuses - rather than on the field's existence. + rather than on the field's existence. On the account payload it describes the ACTIVE record, so a client under + an account switch reads the same record its counts and module map describe; under a scoped grant the answers + come back empty. OnboardingStepState: type: object properties: diff --git a/src/app/api/auth/me/route.ts b/src/app/api/auth/me/route.ts index 9c3b71d32..52dcfc7cf 100644 --- a/src/app/api/auth/me/route.ts +++ b/src/app/api/auth/me/route.ts @@ -54,7 +54,10 @@ import { resolveModuleMap, getOperatorModuleAvailability, } from "@/lib/modules/gate"; -import { readHeldUnitPreferences } from "@/lib/onboarding/needs"; +import { + emptyOnboardingNeeds, + readHeldUnitPreferences, +} from "@/lib/onboarding/needs"; import { loadOnboardingRecordRow, toOnboardingStateDto, @@ -122,10 +125,20 @@ export const GET = apiHandler(async () => { // The actor's own row is already in hand; a switched session needs the // record's `gender`, which is the column the cycle gate derives from. recordId === user.id - ? Promise.resolve({ gender: user.gender }) + ? Promise.resolve({ + gender: user.gender, + glucoseUnit: user.glucoseUnit, + unitPreference: user.unitPreference, + }) : prisma.user.findUnique({ where: { id: recordId }, - select: { gender: true }, + select: { + gender: true, + // v1.39 (C1) — the record's own unit columns, for the setup + // flow's "never re-ask a value the account holds" rule. + glucoseUnit: true, + unitPreference: true, + }, }), prisma.cycleProfile.findUnique({ where: { userId: recordId }, @@ -145,16 +158,19 @@ export const GET = apiHandler(async () => { // Server-WIDE, so it is the same map for every record and is not // re-scoped here. getOperatorModuleAvailability(), - // v1.39 (C1) — the needs-based setup state, for the ACTOR's own record - // rather than the active one. The two module fields above answer for the - // record because they say what that record tracks; this one is resolved - // the other way because the three write routes are actor-only - // (`requireAuth()` refuses outright under a switch), and publishing a - // setup state the caller cannot write would be a read the browser could - // act on and never complete. A guardian's route into a managed record's - // flow arrives with the rest of that record's configuration (#939 / C2); - // the storage is already keyed by record for it. - loadOnboardingRecordRow(prisma, user.id), + // v1.39 (C1) — the needs-based setup state for the RECORD, like the two + // module fields above and for the same reason: it says what that record + // tracks, and its only consumer reads it beside that record's reading + // count, medication count and integration state. Ordering a record's rows + // by somebody else's answers is the one thing this field must not do. + // + // The three WRITE routes stay actor-only (`requireAuth()` refuses outright + // under a switch) until a guardian's route into a managed record's setup + // arrives with the rest of that record's configuration (#939 / C2). A read + // that is wider than the write is the honest shape here: the storage is + // keyed by record already, and a delegate seeing the record's own setup + // state is what makes the dashboard they are looking at coherent. + loadOnboardingRecordRow(prisma, recordId), ]); // The masking step, and the one place the record scoping is narrowed rather @@ -169,13 +185,24 @@ export const GET = apiHandler(async () => { // exactly as it was, and `moduleAccess` is the same answer with the reason // attached — the record's own switch, the grant's edge, or the operator's. // Every client that only wants "paint it or not" keeps reading the boolean. - // v1.39 (C1) — the published steps carry the Q6 resolution: an account that - // already holds both unit preferences reads `units` as `done`, so the flow - // never re-asks a value the account holds. - const onboarding = toOnboardingStateDto( + // v1.39 (C1) — the published steps carry the Q6 resolution: a record whose + // account already holds both unit preferences reads `units` as `done`, so + // the flow never re-asks a value the account holds. + const recordOnboarding = toOnboardingStateDto( onboardingRow, - readHeldUnitPreferences(user), + readHeldUnitPreferences({ + glucoseUnit: record?.glucoseUnit ?? null, + unitPreference: record?.unitPreference ?? null, + }), ); + // The answers name the domains the record tracks, which is exactly what the + // module map below is masked for. A SCOPED grant therefore gets the shape of + // the flow — how far it got, what it ended on — with the answers emptied; + // an unscoped grant, and the record's own session, get them whole. + const onboarding = + sections === null + ? recordOnboarding + : { ...recordOnboarding, needs: emptyOnboardingNeeds() }; const { modules, moduleAccess } = buildModuleDisclosure( resolvedModules, @@ -221,6 +248,11 @@ export const GET = apiHandler(async () => { // meaning as the first-run redirect's gate) and `firstResult` (the one // task the flow offered, and whether it produced its result). // + // Record-scoped, like `modules` and `cycleTrackingEnabled`: it says what + // the record tracks, and the checklist reads it beside that record's + // counts. Under a scoped grant the answers are emptied for the reason the + // module map is masked. + // // Always present, never null: a record that never entered the flow reads // as empty answers with nine `pending` steps, so a client branches on the // step statuses rather than on the field's existence. A client that meets diff --git a/src/components/onboarding/getting-started-checklist.tsx b/src/components/onboarding/getting-started-checklist.tsx index a4eb66278..61ed029e8 100644 --- a/src/components/onboarding/getting-started-checklist.tsx +++ b/src/components/onboarding/getting-started-checklist.tsx @@ -314,8 +314,10 @@ export function GettingStartedChecklist() { dismissedIds, // v1.39 (C1) — the setup answers order these rows: medication first // for somebody who said they take one daily, the data-source row first - // for somebody who named a wearable. Null until the account payload - // resolves, and for any record that never entered the flow. + // for somebody who named a wearable. The payload resolves them for the + // ACTIVE record, which is the same record the counts above describe. + // Null until the payload resolves, and for a record that never entered + // the flow. onboarding: user?.onboarding ?? null, }), [ diff --git a/src/hooks/use-auth.ts b/src/hooks/use-auth.ts index aad63b656..11e67fa72 100644 --- a/src/hooks/use-auth.ts +++ b/src/hooks/use-auth.ts @@ -83,11 +83,12 @@ export interface AuthUser { */ onboardingTourProgress: AuthTourProgress | null; /** - * v1.39 (C1) — the needs-based setup flow's state for this record: the nine - * ordered steps with their `pending | done | skipped` status, the answers as - * given, the flow's own completion stamp (distinct from - * `onboardingCompletedAt` above, which stays the first-run redirect's gate) - * and the one task the flow offered. + * v1.39 (C1) — the needs-based setup flow's state for the record the payload + * describes — the active one under a switch: the nine ordered steps with + * their `pending | done | skipped` status, the answers as given, the flow's + * own completion stamp (distinct from `onboardingCompletedAt` above, which + * stays the first-run redirect's gate) and the one task the flow offered. + * Under a scoped grant the answers come back empty. * * Optional in the type so a stale /me payload — an older server image, or a * test fixture written before the field existed — coerces to "no flow" in diff --git a/src/lib/openapi/routes/account-sharing.ts b/src/lib/openapi/routes/account-sharing.ts index fbfea85c5..a41d6e81e 100644 --- a/src/lib/openapi/routes/account-sharing.ts +++ b/src/lib/openapi/routes/account-sharing.ts @@ -466,10 +466,11 @@ const accountPayload = z // v1.39 (C1) — enumerated for the same reason `notificationPrefs` is: a // client DECIDES on it. The getting-started checklist orders its rows from // `needs` and stays on screen while the flow is unfinished, and a future - // native client renders the steps it knows. Always present, resolved for - // the CALLER's own record rather than the active one — the three write - // routes are actor-only, and publishing a setup state the caller cannot - // write would be a read a browser could act on and never complete. + // native client renders the steps it knows. Always present, and resolved + // for the RECORD the payload describes — the same scoping as `modules`, + // because the checklist reads it beside that record's counts. Under a + // SCOPED grant the answers come back empty, for the reason the module map + // is masked. The three write routes stay actor-only. onboarding: onboardingStateResource, }) .meta({ diff --git a/src/lib/openapi/routes/onboarding.ts b/src/lib/openapi/routes/onboarding.ts index ff219c68d..16f0443df 100644 --- a/src/lib/openapi/routes/onboarding.ts +++ b/src/lib/openapi/routes/onboarding.ts @@ -129,7 +129,7 @@ export const onboardingStateResource = z .meta({ id: "OnboardingState", description: - "The needs-based setup flow for one record. Always present on the account payload — a record that never entered the flow reads as empty answers with nine `pending` steps, so a client branches on the step statuses rather than on the field's existence.", + "The needs-based setup flow for one record. Always present on the account payload — a record that never entered the flow reads as empty answers with nine `pending` steps, so a client branches on the step statuses rather than on the field's existence. On the account payload it describes the ACTIVE record, so a client under an account switch reads the same record its counts and module map describe; under a scoped grant the answers come back empty.", }); const onboardingStateEnvelope = dataEnvelope( diff --git a/tests/integration/onboarding-needs.test.ts b/tests/integration/onboarding-needs.test.ts index beeceb41a..03296d1f4 100644 --- a/tests/integration/onboarding-needs.test.ts +++ b/tests/integration/onboarding-needs.test.ts @@ -52,6 +52,7 @@ vi.mock("@/lib/db-compat", () => ({ })); import { acceptGrant, inviteGrant } from "@/lib/sharing/grants"; +import { emptyOnboardingNeeds } from "@/lib/onboarding/needs"; import type { OnboardingStateDto, OnboardingStepId, @@ -116,6 +117,18 @@ async function postRestart(): Promise { ); } +/** The `onboarding` field the account payload publishes for the caller. */ +async function readMe(): Promise { + const { GET } = await import("@/app/api/auth/me/route"); + const res = await (GET as (r: Request) => Promise)( + new Request("http://localhost/api/auth/me"), + ); + const body = (await res.json()) as { + data: { onboarding: OnboardingStateDto }; + }; + return body.data.onboarding; +} + async function readState(res: Response): Promise { const body = (await res.json()) as { data: { onboarding: OnboardingStateDto }; @@ -482,7 +495,18 @@ describe("POST /api/onboarding/restart", () => { }); await patchAnswer({ step: "who", recordTarget: "me" }); await patchAnswer({ step: "areas", areas: [] }); + await patchAnswer({ step: "medication", medication: "no" }); + await patchAnswer({ step: "sources", status: "skipped" }); + await patchAnswer({ step: "visit", status: "skipped" }); + await patchAnswer({ step: "units", status: "skipped" }); await postComplete(); + // The derivation really did run the second time round, so the survival + // below is the merge protecting a hand-made decision and not the gate + // refusing to look. + const reran = await getPrismaClient().onboardingRecord.findUniqueOrThrow({ + where: { userId: user.id }, + }); + expect(reran.modulesDerivedAt).not.toBeNull(); expect((await modulePrefs(user.id)).labs).toBe(true); }); @@ -541,4 +565,73 @@ describe("a delegate and somebody else's record", () => { expect(ownerRow.modulePreferencesJson).toBeNull(); expect(ownerRow.onboardingCompletedAt).toBeNull(); }); + + it("publishes the RECORD's setup state on the account payload, not the actor's", async () => { + const owner = await makeUser("scoped-owner"); + const delegate = await makeUser("scoped-delegate"); + + // The owner sets up; the delegate never does. + await signIn(owner.id); + await answerTheQuestions(); + await postComplete(); + + const invited = await inviteGrant({ + grantorId: owner.id, + granteeId: delegate.id, + access: "READ", + scope: null, + }); + await acceptGrant({ grantId: invited.id, granteeId: delegate.id }); + + cookieJar.clear(); + headerJar.clear(); + const session = await signIn(delegate.id); + await getPrismaClient().session.update({ + where: { id: session.id }, + data: { actingAsUserId: owner.id }, + }); + await assertCurrentContext(session.id); + + // The checklist reads this beside the record's reading count and + // medication count, both of which answer for the record under a switch. + // Answering it for the actor would order one record's rows by another + // person's answers. + const state = await readMe(); + expect(state.needs.medication).toBe("yes"); + expect(state.needs.areas).toEqual(["blood-pressure", "labs"]); + expect(state.completedAt).not.toBeNull(); + expect(statusOf(state, "confirm")).toBe("done"); + }); + + it("empties the answers under a grant that opens only one section", async () => { + const owner = await makeUser("section-owner"); + const delegate = await makeUser("section-delegate"); + + await signIn(owner.id); + await answerTheQuestions(); + await postComplete(); + + const invited = await inviteGrant({ + grantorId: owner.id, + granteeId: delegate.id, + access: "READ", + scope: ["measurements"], + }); + await acceptGrant({ grantId: invited.id, granteeId: delegate.id }); + + cookieJar.clear(); + headerJar.clear(); + const session = await signIn(delegate.id); + await getPrismaClient().session.update({ + where: { id: session.id }, + data: { actingAsUserId: owner.id }, + }); + await assertCurrentContext(session.id); + + // The answers name the domains the record tracks — the same thing the + // module map is masked for. The shape of the flow still comes through. + const state = await readMe(); + expect(state.needs).toEqual(emptyOnboardingNeeds()); + expect(state.completedAt).not.toBeNull(); + }); }); From b17f18295e8b32e9a083645b05745d728e69cd76 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 10 Sep 2026 12:11:17 +0200 Subject: [PATCH 13/15] fix(onboarding): let the getting-started list go when its rows are done MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three things, all in the same pair. The reader that orders the list from the answers shipped without the proof that it reads correctly: every existing case took the early return, so `checklistOrderFromNeeds` and the two new arms were never executed by a test. They are covered now — medication first for somebody who takes one daily, the named connection ahead of the untouched rows, profile always first, and no answer combination that drops or duplicates a row. A first-result task that was offered and then passed left the setup reading as permanently unfinished, because the check asked only whether the offered task had produced anything. A deliberate pass is an answer. And the published state is additive — every record carries the field — so nine pending steps and empty answers must read as "never entered the flow" rather than as an unfinished one, or every account that predates the flow would look half set up. The list itself now follows the design: for a record that ran the flow it goes when its rows are done or the person hides it, and the five-reading rule stays only for records that never entered. --- .../onboarding/__tests__/checklist.test.ts | 246 ++++++++++++++++++ src/lib/onboarding/__tests__/needs.test.ts | 79 +++++- src/lib/onboarding/checklist.ts | 44 ++-- src/lib/onboarding/needs.ts | 35 ++- 4 files changed, 383 insertions(+), 21 deletions(-) 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; } From 44b6ea01c68d08f100dadd32030fd86063712d41 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 10 Sep 2026 12:14:32 +0200 Subject: [PATCH 14/15] fix(onboarding): tidy the small edges around the setup routes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four of them. The confirm re-stamped its completion instant on every call, so a replay quietly moved the moment the setup finished at while the derivation beside it was carefully latched. The two list answers were stored exactly as sent, duplicates included, so the column could hold something the reader would never return — the reader de-duplicates on the way out and now so does the write. The wide-event name for saving an answer used a different noun from its two siblings, which makes a dashboard filter awkward for one feature. And the contract said the restart body was required while the route accepts none. --- docs/api/openapi.yaml | 2 +- src/app/api/onboarding/answers/route.ts | 8 ++++---- src/app/api/onboarding/complete/route.ts | 5 ++++- src/lib/onboarding/__tests__/needs.test.ts | 21 +++++++++++++++++++++ src/lib/onboarding/needs-apply.ts | 17 +++++++++++++++-- src/lib/openapi/routes/onboarding.ts | 4 +++- tests/integration/onboarding-needs.test.ts | 3 +++ 7 files changed, 51 insertions(+), 9 deletions(-) diff --git a/docs/api/openapi.yaml b/docs/api/openapi.yaml index 05f6899fd..051227ea3 100644 --- a/docs/api/openapi.yaml +++ b/docs/api/openapi.yaml @@ -16244,7 +16244,7 @@ paths: Cookie or wildcard Bearer, own record only — same refusal under a switch as the answers route. Rate-limited to 10 restarts per 10 minutes per account. requestBody: - required: true + required: false content: application/json: schema: diff --git a/src/app/api/onboarding/answers/route.ts b/src/app/api/onboarding/answers/route.ts index ac502bea3..d8cc35898 100644 --- a/src/app/api/onboarding/answers/route.ts +++ b/src/app/api/onboarding/answers/route.ts @@ -51,7 +51,7 @@ const RATE_WINDOW_MS = 10 * 60 * 1000; export const PATCH = apiHandler(async (request: NextRequest) => { const { user } = await requireAuth(); - annotate({ action: { name: "onboarding.answer.save" } }); + annotate({ action: { name: "onboarding.needs.save" } }); const rl = await checkRateLimit( `onboarding-answers:${user.id}`, @@ -60,7 +60,7 @@ export const PATCH = apiHandler(async (request: NextRequest) => { ); if (!rl.allowed) { annotate({ - action: { name: "onboarding.answer.save" }, + action: { name: "onboarding.needs.save" }, meta: { outcome: "rate_limited" }, }); return apiError("Too many onboarding writes, try again later", 429, { @@ -76,7 +76,7 @@ export const PATCH = apiHandler(async (request: NextRequest) => { const parsed = onboardingAnswerSchema.safeParse(body); if (!parsed.success) { annotate({ - action: { name: "onboarding.answer.save" }, + action: { name: "onboarding.needs.save" }, meta: { outcome: "validation_failed" }, }); return returnAllZodIssues(parsed.error, 422, { @@ -175,7 +175,7 @@ export const PATCH = apiHandler(async (request: NextRequest) => { } annotate({ - action: { name: "onboarding.answer.save" }, + action: { name: "onboarding.needs.save" }, meta: { step: answer.step, status: answer.status === "skipped" ? "skipped" : "done", diff --git a/src/app/api/onboarding/complete/route.ts b/src/app/api/onboarding/complete/route.ts index d4d378072..bff695481 100644 --- a/src/app/api/onboarding/complete/route.ts +++ b/src/app/api/onboarding/complete/route.ts @@ -236,7 +236,10 @@ async function completeNeedsFlow( where: { userId }, data: { stepsJson: toJson(confirmed), - completedAt: now, + // Stamped once. The derivation is latched and the stamp should be too, + // or a replay of the confirm quietly moves the instant the setup + // finished at. + completedAt: record.completedAt ?? now, ...(derived ? { modulesDerivedAt: now } : {}), }, select: ONBOARDING_RECORD_SELECT, diff --git a/src/lib/onboarding/__tests__/needs.test.ts b/src/lib/onboarding/__tests__/needs.test.ts index 02ad9b7d1..d3ff3e666 100644 --- a/src/lib/onboarding/__tests__/needs.test.ts +++ b/src/lib/onboarding/__tests__/needs.test.ts @@ -222,6 +222,27 @@ describe("applyOnboardingAnswer", () => { }); }); +describe("applyOnboardingAnswer and the two list answers", () => { + it("stores the list de-duplicated, so the column holds what the reader returns", () => { + const next = applyOnboardingAnswer( + blank(), + { + step: "areas", + areas: ["sleep", "sleep", "labs"], + }, + AT, + ); + expect(next.needs.areas).toEqual(["sleep", "labs"]); + + const sources = applyOnboardingAnswer( + blank(), + { step: "sources", sources: ["oura", "oura", "manual"] }, + AT, + ); + expect(sources.needs.sources).toEqual(["oura", "manual"]); + }); +}); + describe("isOnboardingSettled", () => { const settled: OnboardingStateDto = { steps: ONBOARDING_STEP_IDS.map((id) => ({ id, status: "done" as const })), diff --git a/src/lib/onboarding/needs-apply.ts b/src/lib/onboarding/needs-apply.ts index 2c62ef99c..293448d5c 100644 --- a/src/lib/onboarding/needs-apply.ts +++ b/src/lib/onboarding/needs-apply.ts @@ -27,6 +27,15 @@ export interface OnboardingRecordState { firstResult: OnboardingFirstResult | null; } +/** + * The two list answers are stored as given, and a client that sends the same + * chip twice would otherwise leave the column holding something the reader + * never returns — `parseOnboardingNeeds` de-duplicates on the way out. + */ +function unique(values: readonly T[]): T[] { + return [...new Set(values)]; +} + function withStatus( steps: readonly OnboardingStepState[], id: OnboardingStepId, @@ -63,7 +72,11 @@ export function applyOnboardingAnswer( needs: { ...state.needs, recordTarget: input.recordTarget }, }; case "areas": - return { ...state, steps, needs: { ...state.needs, areas: input.areas } }; + return { + ...state, + steps, + needs: { ...state.needs, areas: unique(input.areas) }, + }; case "medication": return { ...state, @@ -74,7 +87,7 @@ export function applyOnboardingAnswer( return { ...state, steps, - needs: { ...state.needs, sources: input.sources }, + needs: { ...state.needs, sources: unique(input.sources) }, }; case "visit": return { ...state, steps, needs: { ...state.needs, visit: input.visit } }; diff --git a/src/lib/openapi/routes/onboarding.ts b/src/lib/openapi/routes/onboarding.ts index 16f0443df..f023d8be4 100644 --- a/src/lib/openapi/routes/onboarding.ts +++ b/src/lib/openapi/routes/onboarding.ts @@ -285,7 +285,9 @@ export const onboardingPaths: NonNullable = { description: "\"Set up again\", from Settings. Puts the nine steps back to `pending`, clears the flow's own completion stamp, and clears the derivation marker so the next `POST /api/onboarding/complete` may derive a module map again.\n\nIt writes no module state at all: the ordering-versus-removal decision says a re-run never turns off a module somebody turned on by hand, and the protection against the second derivation lives in the merge rather than here. It also keeps the answers, as the prefill for the re-run, keeps a first result that really happened, and leaves the account payload's `onboardingCompletedAt` alone so nobody is pushed back through the first-run redirect.\n\nCookie or wildcard Bearer, own record only — same refusal under a switch as the answers route. Rate-limited to 10 restarts per 10 minutes per account.", requestBody: { - required: true, + // The route accepts an absent body and treats it as `{}`; the schema + // is strict, so a body that carries anything is a 422. + required: false, content: { "application/json": { schema: onboardingRestartRequest } }, }, responses: { diff --git a/tests/integration/onboarding-needs.test.ts b/tests/integration/onboarding-needs.test.ts index 03296d1f4..35c6f37e9 100644 --- a/tests/integration/onboarding-needs.test.ts +++ b/tests/integration/onboarding-needs.test.ts @@ -340,6 +340,8 @@ describe("POST /api/onboarding/complete", () => { // Somebody changes their mind in Settings, then the confirm screen is // replayed. The second call must leave that decision alone: the latch is // what stops a questionnaire from re-applying itself over a later choice. + // The completion instant is latched with it — a replay does not move the + // moment the setup finished at. await getPrismaClient().user.update({ where: { id: user.id }, data: { modulePreferencesJson: { ...derived, labs: false } }, @@ -347,6 +349,7 @@ describe("POST /api/onboarding/complete", () => { const second = await postComplete(); expect(second.status).toBe(200); expect((await modulePrefs(user.id)).labs).toBe(false); + expect((await readState(second)).completedAt).toBe(state.completedAt); }); it("never switches off a module the person switched on by hand", async () => { From f5df6111a17ec3e1615c5317ed7ae6ca946612da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 10 Sep 2026 12:20:50 +0200 Subject: [PATCH 15/15] test(onboarding): put the setup flow's cases through the finished questionnaire Two cases in the integration file answered one or two questions and posted the confirm, which the gate now refuses; they answer or pass all six. The audit assertion reads the details column as the JSON string it is. --- tests/integration/onboarding-needs.test.ts | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/tests/integration/onboarding-needs.test.ts b/tests/integration/onboarding-needs.test.ts index 35c6f37e9..68b1b1280 100644 --- a/tests/integration/onboarding-needs.test.ts +++ b/tests/integration/onboarding-needs.test.ts @@ -333,9 +333,12 @@ describe("POST /api/onboarding/complete", () => { where: { userId: user.id, action: "user.modules.update" }, }); expect(audit).toHaveLength(1); - expect( - (audit[0].details as { changed?: string[] } | null)?.changed, - ).toContain("medications"); + const details = JSON.parse(audit[0].details ?? "{}") as { + changed?: string[]; + source?: string; + }; + expect(details.changed).toContain("medications"); + expect(details.source).toBe("onboarding"); // Somebody changes their mind in Settings, then the confirm screen is // replayed. The second call must leave that decision alone: the latch is @@ -410,6 +413,10 @@ describe("POST /api/onboarding/complete", () => { await signIn(user.id); await patchAnswer({ step: "who", recordTarget: "me" }); await patchAnswer({ step: "areas", areas: ["cycle"] }); + await patchAnswer({ step: "medication", status: "skipped" }); + await patchAnswer({ step: "sources", status: "skipped" }); + await patchAnswer({ step: "visit", status: "skipped" }); + await patchAnswer({ step: "units", status: "skipped" }); await postComplete(); const profile = await getPrismaClient().cycleProfile.findUniqueOrThrow({