From 510d4b1d049f44d50e6dec32008d6439ce77de07 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sat, 3 Oct 2026 23:29:12 +0200 Subject: [PATCH] Write a medication's window and its course in one transaction The PUT wrote startsOn/endsOn on the row and then rewrote the latest course in a second transaction, so a failure between the two left the row and its courses disagreeing. The window now reaches the row only through the course writer, which writes both under one lock. --- .../medications/[id]/__tests__/route.test.ts | 20 +++++++++--- src/app/api/medications/[id]/route.ts | 32 +++++++++++++------ src/lib/medications/courses.ts | 4 +-- 3 files changed, 40 insertions(+), 16 deletions(-) diff --git a/src/app/api/medications/[id]/__tests__/route.test.ts b/src/app/api/medications/[id]/__tests__/route.test.ts index ed1e5b8bf..5545404eb 100644 --- a/src/app/api/medications/[id]/__tests__/route.test.ts +++ b/src/app/api/medications/[id]/__tests__/route.test.ts @@ -80,7 +80,10 @@ vi.mock("next/headers", () => ({ import { PUT } from "../route"; import { prisma } from "@/lib/db"; import { getSession } from "@/lib/auth/session"; -import { resolveCourseFields } from "@/lib/medications/courses"; +import { + resolveCourseFields, + setCurrentWindow, +} from "@/lib/medications/courses"; import { getMedicationCategories, isAssignableCategory, @@ -200,10 +203,16 @@ describe("PUT /api/medications/[id] — v1.5 scheduling primitives", () => { vi.mocked(auditLog).mockResolvedValue(undefined); const res = await PUT(putReq({ endsOn: "2026-12-31" }), ROUTE_CTX); expect(res.status).toBe(200); + // v1.40 (#1024) — the window goes through the course writer, which + // writes the row and the latest course in one transaction; the plain + // row update no longer carries it. const call = lastUpdateCall(); - expect(call.data.endsOn).toBeInstanceOf(Date); - expect((call.data.endsOn as Date).toISOString()).toBe( - new Date("2026-12-31").toISOString(), + expect(call.data).not.toHaveProperty("endsOn"); + expect(setCurrentWindow).toHaveBeenCalledWith( + expect.objectContaining({ + medicationId: "m1", + endsOn: new Date("2026-12-31"), + }), ); }); @@ -1078,7 +1087,8 @@ describe("PUT /api/medications/[id] — shipped client on a record-only medicati expect(prisma.medicationIntakeEvent.updateMany).not.toHaveBeenCalled(); // Everything else in the save applies. expect(data).toMatchObject({ name: "Atorvastatin 20" }); - expect(data).toHaveProperty("startsOn"); + // v1.40 (#1024) — the window rides the course writer, not this update. + expect(setCurrentWindow).toHaveBeenCalled(); // And it is still a record: the stored schedule comes back on record. const body = (await res.json()) as { data: { diff --git a/src/app/api/medications/[id]/route.ts b/src/app/api/medications/[id]/route.ts index fd576ea10..c30e57891 100644 --- a/src/app/api/medications/[id]/route.ts +++ b/src/app/api/medications/[id]/route.ts @@ -21,6 +21,7 @@ import { import { serializeScheduleRecurrence } from "@/lib/medication-schedule"; import { checkCurrentWindow, + CourseWriteError, courseRefusalResponse, resolveCourseFields, setCurrentWindow, @@ -957,8 +958,10 @@ export const PUT = apiHandler( // v1.5 scheduling primitives — pass-through when supplied. // `startsOn` / `endsOn` are `Date | null | undefined` (the // schema lets the user clear them explicitly with null). - ...(startsOn !== undefined && { startsOn }), - ...(normalisedEndsOn !== undefined && { endsOn: normalisedEndsOn }), + // v1.40 (#1024) — `startsOn` / `endsOn` are not written here: the + // course writer below writes them together with the course they + // project, in one transaction, so the row and its courses cannot + // disagree if one of the two writes fails. ...(oneShot !== undefined && { oneShot }), // v1.16.11 — as-needed flag, field-by-field (the invariants above // already guaranteed the medication ends schedule-consistent). @@ -1055,13 +1058,24 @@ export const PUT = apiHandler( // v1.40 (#1024) — write the window onto the latest course and project it // back, then serve the projected row. if (touchesWindow) { - const projected = await setCurrentWindow({ - userId: user.id, - medicationId: id, - timeZone: courseTz, - ...nextWindow, - }); - if (projected) medication = { ...medication, ...projected }; + let projected; + try { + projected = await setCurrentWindow({ + userId: user.id, + medicationId: id, + timeZone: courseTz, + ...nextWindow, + }); + } catch (err) { + // A concurrent course write landed between the check above and this + // transaction. Nothing of the window was written. + if (err instanceof CourseWriteError) { + const r = courseRefusalResponse(err.refusal); + return apiError(r.message, r.status, { errorCode: r.errorCode }); + } + throw err; + } + medication = { ...medication, ...projected }; } // v1.40 (#1041) — an iPhone app build that predates custom categories diff --git a/src/lib/medications/courses.ts b/src/lib/medications/courses.ts index c4646e017..8ad078a2b 100644 --- a/src/lib/medications/courses.ts +++ b/src/lib/medications/courses.ts @@ -328,7 +328,7 @@ function windowAsCourse( */ export async function setCurrentWindow( args: WriteContext & { startsOn: Date | null; endsOn: Date | null }, -): Promise { +): Promise { const todayKey = userDayKey(args.now ?? new Date(), args.timeZone); return prisma.$transaction(async (tx) => { await lockMedication(tx, args.medicationId); @@ -340,7 +340,7 @@ export async function setCurrentWindow( const latest = sortCourses(courses).at(-1); if (args.startsOn === null && args.endsOn === null) { - if (!latest) return null; + if (!latest) return project(tx, args.medicationId); if (courses.length > 1) throw new CourseWriteError("windowRequired"); await tx.medicationCourse.delete({ where: { id: latest.id } }); return project(tx, args.medicationId);