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.
This commit is contained in:
Marc-André Bombeck committed 2026-09-10 11:57:11 +02:00
1 parent 07f1b6ce92
commit b10a3ef22e
5 files changed
+82 -5

No files matched your search

+4 -2
View File
@@ -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
+28
View File
@@ -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: {
+29 -2
View File
@@ -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;
}
+1 -1
View File
@@ -376,7 +376,7 @@ export const onboardingPaths: NonNullable<ZodOpenApiObject["paths"]> = {
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 } },
@@ -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.