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] 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 () => {