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(); + }); });