mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-08 12:32:43 +00:00
fix(onboarding): tidy the small edges around the setup routes
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.
This commit is contained in:
1 parent
b17f18295e
commit
44b6ea01c6
7 files changed
+51
-9
No files matched your search
@@ -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:
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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 })),
|
||||
|
||||
@@ -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<T>(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 } };
|
||||
|
||||
@@ -285,7 +285,9 @@ export const onboardingPaths: NonNullable<ZodOpenApiObject["paths"]> = {
|
||||
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: {
|
||||
|
||||
@@ -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 () => {
|
||||
|
||||
Reference in new issue
Block a user