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);