From 572f5176bd7410f22d35060676c7c1eb676ecc62 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Fri, 24 Jul 2026 16:39:39 +0200 Subject: [PATCH] test(concurrency): cover the layout and about-me write guards Add per-endpoint unit tests for the insights layout, medications layout, and coach about-me write guards: an interleaved stale-base writer gets a 409 and changes nothing, a tokenless request still writes (older-client compatibility), the fresh token is echoed, and a malformed token 422s. Cover the client token echo, the two 409 dispositions, and the insights read that must keep the token through the resolver. Refresh the CHANGELOG and the generated OpenAPI spec. --- CHANGELOG.md | 27 +++ public/sw.js | 2 +- .../__tests__/route-concurrency.test.ts | 175 ++++++++++++++++++ .../insights/layout/__tests__/route.test.ts | 89 +++++++++ .../layout/__tests__/route.test.ts | 90 +++++++++ .../api/__tests__/optimistic-token.test.ts | 108 +++++++++++ src/lib/openapi/routes/coach.ts | 3 +- src/lib/openapi/routes/dashboard.ts | 3 +- src/lib/openapi/routes/medications/paths.ts | 3 +- src/lib/optimistic-lock.ts | 6 +- .../use-medication-list-layout.test.ts | 134 +++++++++++++- src/lib/queries/use-medication-list-layout.ts | 5 +- 12 files changed, 634 insertions(+), 11 deletions(-) create mode 100644 src/app/api/coach/about-me/__tests__/route-concurrency.test.ts create mode 100644 src/lib/api/__tests__/optimistic-token.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 89172e489..94348e6ac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,33 @@ ## [Unreleased] +## [1.32.21] — 2026-07-24 + +Insights and medications layout saves, and the coach about-me note, are now +protected against a concurrent edit overwriting a newer one. This is the same +protection the dashboard layout already had. If you save a layout in one place +while something else saved a change to the same record a moment earlier, your +save now stops and reloads the current state instead of quietly reverting that +other change. Older clients that do not send the new marker keep working exactly +as before. + +- **Insights layout saves no longer clobber a concurrent edit.** Saving the + overview arrangement and saving the pill order are two separate surfaces that + write the same record. If one lands while the other was based on an older + read, the later save now reloads and asks you to redo it rather than reverting + the first. +- **Medications layout saves get the same guard.** The card/table view toggle + and the manual order both write one record, so a stale toggle can no longer + resurrect an order you just saved, and the reverse holds too. +- **The coach about-me note is guarded as well.** A save based on an older read + is stopped and reloaded instead of overwriting a newer version of the note. +- **Older clients are unaffected.** A request that does not carry the new base + marker keeps the previous save behavior unchanged, so nothing needs updating + to keep working. +- **The dashboard layout contract is now in the published API spec.** It was + missing from the OpenAPI file even though it was the first surface to get this + protection. + ## [1.32.20] — 2026-07-24 The medication-adherence insight now passes through the same safety screen as diff --git a/public/sw.js b/public/sw.js index 2aede105f..8426bc337 100644 --- a/public/sw.js +++ b/public/sw.js @@ -36,7 +36,7 @@ try { // v1.4.38.4 → v1.4.42. Do not hand-edit; bump `package.json` and rebuild. const CACHE_VERSION = (typeof self !== "undefined" && self.__APP_VERSION__) || - /* @sw-version-fallback */ "v1.32.20"; + /* @sw-version-fallback */ "v1.32.21"; const STATIC_CACHE = `healthlog-static-${CACHE_VERSION}`; const PAGE_CACHE = `healthlog-pages-${CACHE_VERSION}`; // v1.18.6 — read-only data cache for a curated allowlist of safe GET `/api/*` diff --git a/src/app/api/coach/about-me/__tests__/route-concurrency.test.ts b/src/app/api/coach/about-me/__tests__/route-concurrency.test.ts new file mode 100644 index 000000000..35106b1b4 --- /dev/null +++ b/src/app/api/coach/about-me/__tests__/route-concurrency.test.ts @@ -0,0 +1,175 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +/** + * v1.32.21 (R5a) — optimistic concurrency on `PUT /api/coach/about-me` + * (issue #581 family). The write guards on `UserHealthProfile.updatedAt`: + * + * - a stale base token → 409 `about_me_conflict`, no write; + * - a matched base token → conditional update, fresh token echoed; + * - a zero-match against a MISSING row → create wins (nothing to clobber); + * - a tokenless request → the prior unconditional upsert (iOS-compat arm). + * + * Mirrors the mock scaffolding of `route-module-gate.test.ts`. + */ + +vi.mock("@/lib/api-handler", () => ({ + apiHandler: unknown>(fn: T) => fn, + requireAuth: vi.fn(async () => ({ user: { id: "u1", locale: "en" } })), +})); + +const { requireModuleEnabled } = vi.hoisted(() => ({ + requireModuleEnabled: vi.fn(async () => ({ enabled: true })), +})); +vi.mock("@/lib/modules/gate", () => ({ requireModuleEnabled })); + +vi.mock("@/lib/api-response", () => ({ + apiError: (error: string, status: number, meta?: unknown) => ({ + data: null, + error, + status, + meta, + }), + apiSuccess: (data: unknown) => ({ data, error: null, status: 200 }), + getClientIp: () => "127.0.0.1", + returnAllZodIssues: (_e: unknown, status: number) => ({ + data: null, + error: "validation", + status, + }), + safeJson: async (req: Request) => ({ data: await req.json(), error: null }), +})); +vi.mock("@/lib/logging/context", () => ({ annotate: vi.fn() })); +vi.mock("@/lib/auth/audit", () => ({ auditLog: vi.fn() })); + +const { profile } = vi.hoisted(() => ({ + profile: { + upsert: vi.fn(), + updateMany: vi.fn(), + findUnique: vi.fn(), + create: vi.fn(), + }, +})); +vi.mock("@/lib/db", () => ({ prisma: { userHealthProfile: profile } })); + +vi.mock("@/lib/rate-limit", () => ({ + checkRateLimit: vi.fn(async () => ({ allowed: true })), + rateLimitHeaders: vi.fn(() => ({})), +})); +vi.mock("@/lib/ai/coach/bytes-codec", () => ({ + encryptToBytes: (s: string) => Buffer.from(s), +})); +vi.mock("@/lib/ai/coach/about-me", () => ({ + getSelfContextForUser: vi.fn(async () => ({ + aboutMe: null, + conditions: null, + allergies: null, + coachFocus: null, + })), + getPendingQuestionsForUser: vi.fn(async () => []), + setPendingQuestionsForUser: vi.fn(), +})); +vi.mock("@/lib/ai/coach/self-context-questions", () => ({ + deriveClarifyingQuestions: vi.fn(async () => ({ + questions: [], + source: "none", + })), +})); + +import { PUT } from "../route"; + +type Envelope = { + data: { updatedAt?: string } | null; + error: string | null; + status: number; + meta?: { errorCode?: string }; +}; +const put = PUT as unknown as (req: Request) => Promise; + +function putReq(baseUpdatedAt?: string): Request { + return new Request("http://localhost/api/coach/about-me", { + method: "PUT", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ + aboutMe: "I run daily", + ...(baseUpdatedAt !== undefined ? { baseUpdatedAt } : {}), + }), + }); +} + +beforeEach(() => { + vi.clearAllMocks(); + requireModuleEnabled.mockResolvedValue({ enabled: true }); +}); + +describe("about-me — optimistic concurrency", () => { + it("guards on the base token and echoes the advanced token", async () => { + const base = new Date("2026-07-24T10:00:00.000Z"); + const advanced = new Date("2026-07-24T10:05:00.000Z"); + profile.updateMany.mockResolvedValue({ count: 1 }); + profile.findUnique.mockResolvedValue({ updatedAt: advanced }); + + const res = await put(putReq(base.toISOString())); + expect(res.status).toBe(200); + + // Conditional write on the exact base token — never the upsert. + expect(profile.updateMany).toHaveBeenCalledTimes(1); + const whereArg = profile.updateMany.mock.calls[0]?.[0]?.where as { + userId: string; + updatedAt: Date; + }; + expect(whereArg.userId).toBe("u1"); + expect((whereArg.updatedAt as Date).toISOString()).toBe(base.toISOString()); + expect(profile.upsert).not.toHaveBeenCalled(); + expect(res.data?.updatedAt).toBe(advanced.toISOString()); + }); + + it("a stale base token 409s and clobbers nothing (existing row advanced)", async () => { + profile.updateMany.mockResolvedValue({ count: 0 }); + // The row exists but its token advanced → real conflict, not a delete. + profile.findUnique.mockResolvedValue({ + updatedAt: new Date("2026-07-24T10:05:00.000Z"), + }); + + const res = await put(putReq("2026-07-24T09:00:00.000Z")); + expect(res.status).toBe(409); + expect(res.data).toBeNull(); + expect(res.meta?.errorCode).toBe("about_me_conflict"); + + // No write happened: neither the upsert nor a create ran. + expect(profile.upsert).not.toHaveBeenCalled(); + expect(profile.create).not.toHaveBeenCalled(); + }); + + it("a zero-match against a MISSING row creates instead of 409ing", async () => { + profile.updateMany.mockResolvedValue({ count: 0 }); + profile.findUnique.mockResolvedValue(null); + profile.create.mockResolvedValue({ + updatedAt: new Date("2026-07-24T12:00:00.000Z"), + }); + + const res = await put(putReq("2026-07-24T09:00:00.000Z")); + expect(res.status).toBe(200); + expect(profile.create).toHaveBeenCalledTimes(1); + expect(res.data?.updatedAt).toBe("2026-07-24T12:00:00.000Z"); + }); + + it("keeps the unconditional upsert when the token is omitted (iOS compat)", async () => { + profile.upsert.mockResolvedValue({ + updatedAt: new Date("2026-07-24T11:00:00.000Z"), + }); + + const res = await put(putReq()); + expect(res.status).toBe(200); + expect(profile.upsert).toHaveBeenCalledTimes(1); + expect(profile.updateMany).not.toHaveBeenCalled(); + expect(res.data?.updatedAt).toBe("2026-07-24T11:00:00.000Z"); + }); + + it("422s a malformed base token without touching the row", async () => { + const res = await put(putReq("not-a-date")); + expect(res.status).toBe(422); + expect(res.meta?.errorCode).toBe("invalid_base_updated_at"); + expect(profile.updateMany).not.toHaveBeenCalled(); + expect(profile.upsert).not.toHaveBeenCalled(); + }); +}); diff --git a/src/app/api/insights/layout/__tests__/route.test.ts b/src/app/api/insights/layout/__tests__/route.test.ts index 9e96a9718..54e529ab9 100644 --- a/src/app/api/insights/layout/__tests__/route.test.ts +++ b/src/app/api/insights/layout/__tests__/route.test.ts @@ -15,6 +15,7 @@ vi.mock("@/lib/db", () => ({ user: { findUnique: vi.fn(), update: vi.fn(), + updateMany: vi.fn(), }, auditLog: { create: vi.fn(), @@ -682,3 +683,91 @@ describe("DELETE /api/insights/layout", () => { expect(call.data.insightsLayoutJson).toBeDefined(); }); }); + +/** + * v1.32.21 (R5a) — optimistic concurrency (issue #581 family). A PUT carrying + * the `baseUpdatedAt` it was based on writes CONDITIONALLY on the stored row + * still carrying that token, so an interleaved edit-mode Save and Settings + * pill-order Save can't clobber one another. A tokenless PUT keeps the prior + * unconditional write — the iOS-compat arm. + */ +describe("PUT /api/insights/layout — optimistic concurrency", () => { + function body(baseUpdatedAt?: string) { + return { + version: 2, + tiles: [{ id: "overview", visible: true, order: 0 }], + ...(baseUpdatedAt !== undefined ? { baseUpdatedAt } : {}), + }; + } + + it("guards the write on the base token and returns the advanced token", async () => { + const base = new Date("2026-07-24T10:00:00.000Z"); + const advanced = new Date("2026-07-24T10:05:00.000Z"); + vi.mocked(prisma.user.updateMany).mockResolvedValue({ count: 1 } as never); + // The merge-read `previous` (no row) then the post-write token read. + vi.mocked(prisma.user.findUnique) + .mockResolvedValueOnce({ insightsLayoutJson: null } as never) + .mockResolvedValueOnce({ updatedAt: advanced } as never); + + const res = await callPut(makeReq(body(base.toISOString()))); + expect(res.status).toBe(200); + + expect(prisma.user.updateMany).toHaveBeenCalledTimes(1); + const whereArg = vi.mocked(prisma.user.updateMany).mock.calls[0]?.[0] + ?.where as { id: string; updatedAt: Date }; + expect(whereArg.id).toBe("user-1"); + expect((whereArg.updatedAt as Date).toISOString()).toBe(base.toISOString()); + expect(prisma.user.update).not.toHaveBeenCalled(); + + const bodyJson = (await res.json()) as { data: { updatedAt: string } }; + expect(bodyJson.data.updatedAt).toBe(advanced.toISOString()); + }); + + it("interleaved writers: the stale-base second writer gets 409 and clobbers nothing", async () => { + vi.mocked(prisma.user.findUnique).mockResolvedValue({ + insightsLayoutJson: null, + } as never); + // Writer B committed first; writer A's guarded write matches zero rows. + vi.mocked(prisma.user.updateMany).mockResolvedValue({ count: 0 } as never); + + const res = await callPut(makeReq(body("2026-07-24T09:00:00.000Z"))); + expect(res.status).toBe(409); + + const bodyJson = (await res.json()) as { + data: null; + error: string; + meta?: { errorCode?: string }; + }; + expect(bodyJson.data).toBeNull(); + expect(bodyJson.meta?.errorCode).toBe("insights_layout_conflict"); + + // Guarded write matched no row; no unconditional fallback → nothing written. + expect(prisma.user.updateMany).toHaveBeenCalledTimes(1); + expect(prisma.user.update).not.toHaveBeenCalled(); + }); + + it("422s a malformed base token without touching the row", async () => { + const res = await callPut(makeReq(body("not-a-date"))); + expect(res.status).toBe(422); + const bodyJson = (await res.json()) as { meta?: { errorCode?: string } }; + expect(bodyJson.meta?.errorCode).toBe("invalid_base_updated_at"); + expect(prisma.user.updateMany).not.toHaveBeenCalled(); + expect(prisma.user.update).not.toHaveBeenCalled(); + }); + + it("keeps the unconditional write when the client omits the token (iOS compat)", async () => { + vi.mocked(prisma.user.findUnique).mockResolvedValue({ + insightsLayoutJson: null, + } as never); + vi.mocked(prisma.user.update).mockResolvedValue({ + updatedAt: new Date("2026-07-24T11:00:00.000Z"), + } as never); + + const res = await callPut(makeReq(body())); + expect(res.status).toBe(200); + expect(prisma.user.update).toHaveBeenCalledTimes(1); + expect(prisma.user.updateMany).not.toHaveBeenCalled(); + const bodyJson = (await res.json()) as { data: { updatedAt: string } }; + expect(bodyJson.data.updatedAt).toBe("2026-07-24T11:00:00.000Z"); + }); +}); diff --git a/src/app/api/medications/layout/__tests__/route.test.ts b/src/app/api/medications/layout/__tests__/route.test.ts index e6c075a20..84d145b7a 100644 --- a/src/app/api/medications/layout/__tests__/route.test.ts +++ b/src/app/api/medications/layout/__tests__/route.test.ts @@ -15,6 +15,7 @@ vi.mock("@/lib/db", () => ({ user: { findUnique: vi.fn(), update: vi.fn(), + updateMany: vi.fn(), }, auditLog: { create: vi.fn(), @@ -152,6 +153,8 @@ describe("PUT /api/medications/layout — preserve-when-absent", () => { version: 1, view: "table", order: ["med-a", "med-b"], + // v1.32.21 — the write now echoes the fresh optimistic-concurrency token. + updatedAt: expect.any(String), }); expect(prisma.user.update).toHaveBeenCalledTimes(1); @@ -275,3 +278,90 @@ describe("DELETE /api/medications/layout", () => { expect(call.data.medicationListLayoutJson).toBeDefined(); }); }); + +/** + * v1.32.21 (R5a) — optimistic concurrency (issue #581 family). A PUT carrying + * the `baseUpdatedAt` it was based on writes CONDITIONALLY on the stored row + * still carrying that token, so a view toggle can never resurrect an order the + * reorder Save just committed (and vice versa). A tokenless PUT keeps the prior + * unconditional write — the iOS-compat arm. + */ +describe("PUT /api/medications/layout — optimistic concurrency", () => { + // A full blob (view + order) so no preserve-read fires; the only findUnique + // is the post-write token read. + function fullBody(baseUpdatedAt?: string) { + return { + version: 1, + view: "table", + order: ["med-a", "med-b"], + ...(baseUpdatedAt !== undefined ? { baseUpdatedAt } : {}), + }; + } + + it("guards the write on the base token and returns the advanced token", async () => { + const base = new Date("2026-07-24T10:00:00.000Z"); + const advanced = new Date("2026-07-24T10:05:00.000Z"); + vi.mocked(prisma.user.updateMany).mockResolvedValue({ count: 1 } as never); + vi.mocked(prisma.user.findUnique).mockResolvedValue({ + updatedAt: advanced, + } as never); + + const res = await callPut(makeReq(fullBody(base.toISOString()))); + expect(res.status).toBe(200); + + // Conditional write on the exact base token — never unconditional. + expect(prisma.user.updateMany).toHaveBeenCalledTimes(1); + const whereArg = vi.mocked(prisma.user.updateMany).mock.calls[0]?.[0] + ?.where as { id: string; updatedAt: Date }; + expect(whereArg.id).toBe("user-1"); + expect((whereArg.updatedAt as Date).toISOString()).toBe(base.toISOString()); + expect(prisma.user.update).not.toHaveBeenCalled(); + + const body = (await res.json()) as { data: { updatedAt: string } }; + expect(body.data.updatedAt).toBe(advanced.toISOString()); + }); + + it("interleaved writers: the stale-base second writer gets 409 and clobbers nothing", async () => { + // Writer B committed first (updatedAt advanced); writer A replays with the + // pre-B token → the conditional write matches zero rows. + vi.mocked(prisma.user.updateMany).mockResolvedValue({ count: 0 } as never); + + const res = await callPut(makeReq(fullBody("2026-07-24T09:00:00.000Z"))); + expect(res.status).toBe(409); + + const body = (await res.json()) as { + data: null; + error: string; + meta?: { errorCode?: string }; + }; + expect(body.data).toBeNull(); + expect(body.meta?.errorCode).toBe("medication_layout_conflict"); + + // The conditional write matched no row; there is NO unconditional + // fallback, so nothing was written — B's value survives. + expect(prisma.user.updateMany).toHaveBeenCalledTimes(1); + expect(prisma.user.update).not.toHaveBeenCalled(); + }); + + it("422s a malformed base token without touching the row", async () => { + const res = await callPut(makeReq(fullBody("not-a-date"))); + expect(res.status).toBe(422); + const body = (await res.json()) as { meta?: { errorCode?: string } }; + expect(body.meta?.errorCode).toBe("invalid_base_updated_at"); + expect(prisma.user.updateMany).not.toHaveBeenCalled(); + expect(prisma.user.update).not.toHaveBeenCalled(); + }); + + it("keeps the unconditional write when the client omits the token (iOS compat)", async () => { + vi.mocked(prisma.user.update).mockResolvedValue({ + updatedAt: new Date("2026-07-24T11:00:00.000Z"), + } as never); + + const res = await callPut(makeReq(fullBody())); + expect(res.status).toBe(200); + expect(prisma.user.update).toHaveBeenCalledTimes(1); + expect(prisma.user.updateMany).not.toHaveBeenCalled(); + const body = (await res.json()) as { data: { updatedAt: string } }; + expect(body.data.updatedAt).toBe("2026-07-24T11:00:00.000Z"); + }); +}); diff --git a/src/lib/api/__tests__/optimistic-token.test.ts b/src/lib/api/__tests__/optimistic-token.test.ts new file mode 100644 index 000000000..4a7ae0ce3 --- /dev/null +++ b/src/lib/api/__tests__/optimistic-token.test.ts @@ -0,0 +1,108 @@ +/** + * v1.32.21 (R5a) — the client-side optimistic-concurrency primitives every + * guarded writer uses (`readUpdatedAtToken`, `withBaseToken`, `isConflict`), + * plus the B-3 token-preservation guarantee for the insights-layout read. + */ +import { describe, it, expect } from "vitest"; +import { QueryClient } from "@tanstack/react-query"; + +import { + readUpdatedAtToken, + withBaseToken, + isConflict, +} from "@/lib/api/optimistic-token"; +import { ApiError } from "@/lib/api/api-fetch"; +import { queryKeys } from "@/lib/query-keys"; +import { + resolveInsightsLayout, + DEFAULT_INSIGHTS_LAYOUT, + type InsightsLayoutWithToken, +} from "@/lib/insights-layout"; + +function makeClient(): QueryClient { + return new QueryClient({ + defaultOptions: { queries: { retry: false, gcTime: Infinity } }, + }); +} + +describe("readUpdatedAtToken", () => { + it("returns the cached updatedAt token at read time", () => { + const qc = makeClient(); + qc.setQueryData(queryKeys.insightsLayout(), { + version: 2, + updatedAt: "2026-07-24T10:00:00.000Z", + }); + expect(readUpdatedAtToken(qc, queryKeys.insightsLayout())).toBe( + "2026-07-24T10:00:00.000Z", + ); + }); + + it("returns undefined when nothing is cached or the token is absent", () => { + const qc = makeClient(); + expect(readUpdatedAtToken(qc, queryKeys.insightsLayout())).toBeUndefined(); + qc.setQueryData(queryKeys.insightsLayout(), { version: 2 }); + expect(readUpdatedAtToken(qc, queryKeys.insightsLayout())).toBeUndefined(); + }); +}); + +describe("withBaseToken", () => { + it("attaches baseUpdatedAt when the token is known", () => { + expect(withBaseToken({ version: 1 }, "2026-07-24T10:00:00.000Z")).toEqual({ + version: 1, + baseUpdatedAt: "2026-07-24T10:00:00.000Z", + }); + }); + + it("omits the field entirely for a tokenless client (backward-compat)", () => { + const out = withBaseToken({ version: 1 }, undefined); + expect(out).toEqual({ version: 1 }); + expect("baseUpdatedAt" in out).toBe(false); + }); +}); + +describe("isConflict", () => { + it("is true only for a 409 ApiError", () => { + expect(isConflict(new ApiError("conflict", 409))).toBe(true); + expect(isConflict(new ApiError("bad", 422))).toBe(false); + expect(isConflict(new Error("boom"))).toBe(false); + expect(isConflict(undefined)).toBe(false); + }); +}); + +/** + * B-3 — the insights-layout GET pipes through `resolveInsightsLayout`, which + * rebuilds a FIXED shape and DROPS `updatedAt`. If the hook did not re-attach + * the token, every insights-layout write would silently fall back to the + * server's compat (unconditional) arm and the guard would be dead on arrival. + */ +describe("B-3 insights-layout token preservation", () => { + it("resolveInsightsLayout strips the server token (why the hook must re-attach)", () => { + const withToken = { + ...DEFAULT_INSIGHTS_LAYOUT, + updatedAt: "2026-07-24T10:00:00.000Z", + }; + const resolved = resolveInsightsLayout(withToken) as { + updatedAt?: string; + }; + expect(resolved.updatedAt).toBeUndefined(); + }); + + it("the hook's re-attach keeps the token, so a later write can echo it", () => { + const raw: InsightsLayoutWithToken = { + ...DEFAULT_INSIGHTS_LAYOUT, + updatedAt: "2026-07-24T10:00:00.000Z", + }; + // Mirrors the hook's queryFn: resolve, then re-attach the token. + const cached: InsightsLayoutWithToken = { + ...resolveInsightsLayout(raw), + updatedAt: raw.updatedAt, + }; + + const qc = makeClient(); + qc.setQueryData(queryKeys.insightsLayout(), cached); + // The token survives into the cache the writers read at mutate time. + expect(readUpdatedAtToken(qc, queryKeys.insightsLayout())).toBe( + "2026-07-24T10:00:00.000Z", + ); + }); +}); diff --git a/src/lib/openapi/routes/coach.ts b/src/lib/openapi/routes/coach.ts index a42b843ab..4a3b04e38 100644 --- a/src/lib/openapi/routes/coach.ts +++ b/src/lib/openapi/routes/coach.ts @@ -954,7 +954,8 @@ export const coachPaths: NonNullable = { "Returns the per-user Insights tile layout (visibility + order) plus the optimistic-concurrency `updatedAt` token. Falls back to the default layout when the user has not customised it. Mirrors the dashboard-widgets contract.", responses: { "200": { - description: "The resolved layout (custom or default) plus its token.", + description: + "The resolved layout (custom or default) plus its token.", content: { "application/json": { schema: dataEnvelope(insightsLayoutResult, "InsightsLayout"), diff --git a/src/lib/openapi/routes/dashboard.ts b/src/lib/openapi/routes/dashboard.ts index f833962d7..4a7c82632 100644 --- a/src/lib/openapi/routes/dashboard.ts +++ b/src/lib/openapi/routes/dashboard.ts @@ -100,7 +100,8 @@ export const dashboardWidgetPaths: NonNullable = { "Returns the resolved effective layout (defaults merged in if the user has not customised it) plus the optimistic-concurrency `updatedAt` token.", responses: { "200": { - description: "The resolved layout (custom or default) plus its token.", + description: + "The resolved layout (custom or default) plus its token.", content: { "application/json": { schema: dataEnvelope(dashboardLayoutResult, "DashboardLayout"), diff --git a/src/lib/openapi/routes/medications/paths.ts b/src/lib/openapi/routes/medications/paths.ts index 09db3026e..5f6e1784a 100644 --- a/src/lib/openapi/routes/medications/paths.ts +++ b/src/lib/openapi/routes/medications/paths.ts @@ -233,7 +233,8 @@ export const medicationPaths: NonNullable = { "Returns the per-user /medications presentation (card/table view + manual order) plus the optimistic-concurrency `updatedAt` token. Falls back to the defaults (cards, empty order) when the user has not customised it. Mirrors the insights-layout contract.", responses: { "200": { - description: "The resolved presentation (custom or default) plus its token.", + description: + "The resolved presentation (custom or default) plus its token.", content: { "application/json": { schema: dataEnvelope( diff --git a/src/lib/optimistic-lock.ts b/src/lib/optimistic-lock.ts index c9d5617d6..924260699 100644 --- a/src/lib/optimistic-lock.ts +++ b/src/lib/optimistic-lock.ts @@ -33,8 +33,7 @@ import { annotate } from "@/lib/logging/context"; /** Result of pulling the `baseUpdatedAt` transport field off a request body. */ export type TakeBaseTokenResult = - | { base: Date | undefined; rest: unknown } - | { invalid: true }; + { base: Date | undefined; rest: unknown } | { invalid: true }; /** * Pull the optimistic-concurrency base token off a raw request body BEFORE @@ -94,8 +93,7 @@ export interface ConflictSpec { /** Either a ready-to-return 409 `Response`, or the fresh token to echo. */ export type GuardedUpdateResult = - | { conflict: Response } - | { updatedAt: string }; + { conflict: Response } | { updatedAt: string }; /** * Generic conditional-update-or-409 core. diff --git a/src/lib/queries/__tests__/use-medication-list-layout.test.ts b/src/lib/queries/__tests__/use-medication-list-layout.test.ts index 0d3c8389c..e11972dd5 100644 --- a/src/lib/queries/__tests__/use-medication-list-layout.test.ts +++ b/src/lib/queries/__tests__/use-medication-list-layout.test.ts @@ -9,12 +9,27 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import { QueryClient } from "@tanstack/react-query"; +// A minimal ApiError stand-in so `@/lib/api/optimistic-token` (which imports +// ApiError from this module and uses it in `isConflict`) resolves a real +// constructor under the mock. Declared via `vi.hoisted` so it exists before +// the hoisted `vi.mock` factory runs. +const { ApiError } = vi.hoisted(() => ({ + ApiError: class ApiError extends Error { + status: number; + constructor(message: string, status: number) { + super(message); + this.name = "ApiError"; + this.status = status; + } + }, +})); vi.mock("@/lib/api/api-fetch", () => ({ apiGet: vi.fn(), apiPut: vi.fn(), + ApiError, })); vi.mock("sonner", () => ({ - toast: { success: vi.fn(), error: vi.fn() }, + toast: { success: vi.fn(), error: vi.fn(), message: vi.fn() }, })); import { apiPut } from "@/lib/api/api-fetch"; @@ -24,7 +39,10 @@ import { runSaveMedicationListOrder, } from "@/lib/queries/use-medication-list-layout"; import { queryKeys } from "@/lib/query-keys"; -import type { MedicationListLayout } from "@/lib/medication-list-layout"; +import type { + MedicationListLayout, + MedicationListLayoutWithToken, +} from "@/lib/medication-list-layout"; const t = (key: string) => key; @@ -146,3 +164,115 @@ describe("runSaveMedicationListOrder", () => { expect(toast.error).toHaveBeenCalledWith("medications.reorderSaveFailed"); }); }); + +/** + * v1.32.21 (R5a) — optimistic-concurrency token echo + 409 handling. + */ +describe("medications layout — optimistic concurrency (client)", () => { + it("echoes the cached base token as baseUpdatedAt on the view PUT", async () => { + const queryClient = makeClient(); + queryClient.setQueryData( + queryKeys.medicationListLayout(), + { + version: 1, + view: "cards", + order: ["med-a"], + updatedAt: "2026-07-24T10:00:00.000Z", + }, + ); + vi.mocked(apiPut).mockResolvedValue({ + version: 1, + view: "table", + order: ["med-a"], + updatedAt: "2026-07-24T10:05:00.000Z", + } as never); + + await runSetMedicationListView({ view: "table", queryClient, t }); + + expect(apiPut).toHaveBeenCalledWith("/api/medications/layout", { + version: 1, + view: "table", + baseUpdatedAt: "2026-07-24T10:00:00.000Z", + }); + }); + + it("echoes the cached base token on the order Save", async () => { + const queryClient = makeClient(); + queryClient.setQueryData( + queryKeys.medicationListLayout(), + { + version: 1, + view: "table", + order: [], + updatedAt: "2026-07-24T10:00:00.000Z", + }, + ); + vi.mocked(apiPut).mockResolvedValue({ + version: 1, + view: "table", + order: ["med-b"], + updatedAt: "2026-07-24T10:05:00.000Z", + } as never); + + await runSaveMedicationListOrder({ order: ["med-b"], queryClient, t }); + + expect(apiPut).toHaveBeenCalledWith("/api/medications/layout", { + version: 1, + order: ["med-b"], + baseUpdatedAt: "2026-07-24T10:00:00.000Z", + }); + }); + + it("view toggle: a 409 rolls back, invalidates, and nudges (not a hard error)", async () => { + const queryClient = makeClient(); + const previous: MedicationListLayoutWithToken = { + version: 1, + view: "cards", + order: ["med-a"], + updatedAt: "2026-07-24T09:00:00.000Z", + }; + queryClient.setQueryData(queryKeys.medicationListLayout(), previous); + const invalidateSpy = vi.spyOn(queryClient, "invalidateQueries"); + vi.mocked(apiPut).mockRejectedValue(new ApiError("conflict", 409)); + + await runSetMedicationListView({ view: "table", queryClient, t }); + + // Optimistic view rolled back to the pre-toggle state. + expect(queryClient.getQueryData(queryKeys.medicationListLayout())).toEqual( + previous, + ); + expect(invalidateSpy).toHaveBeenCalledWith({ + queryKey: queryKeys.medicationListLayout(), + }); + expect(toast.message).toHaveBeenCalledWith("common.conflictReloaded"); + expect(toast.error).not.toHaveBeenCalled(); + }); + + it("order Save: a 409 keeps the dialog open (returns false), invalidates, nudges", async () => { + const queryClient = makeClient(); + queryClient.setQueryData( + queryKeys.medicationListLayout(), + { + version: 1, + view: "table", + order: [], + updatedAt: "2026-07-24T09:00:00.000Z", + }, + ); + const invalidateSpy = vi.spyOn(queryClient, "invalidateQueries"); + vi.mocked(apiPut).mockRejectedValue(new ApiError("conflict", 409)); + + const ok = await runSaveMedicationListOrder({ + order: ["med-b"], + queryClient, + t, + }); + + expect(ok).toBe(false); + expect(invalidateSpy).toHaveBeenCalledWith({ + queryKey: queryKeys.medicationListLayout(), + }); + expect(toast.message).toHaveBeenCalledWith("common.conflictReloaded"); + expect(toast.error).not.toHaveBeenCalled(); + }); +}); diff --git a/src/lib/queries/use-medication-list-layout.ts b/src/lib/queries/use-medication-list-layout.ts index 58bcf8c02..6a2ef6815 100644 --- a/src/lib/queries/use-medication-list-layout.ts +++ b/src/lib/queries/use-medication-list-layout.ts @@ -96,7 +96,10 @@ export async function runSaveMedicationListOrder(deps: { // clobbering a view toggle that committed since the dialog opened. const saved = await apiPut( "/api/medications/layout", - withBaseToken({ version: 1, order }, readUpdatedAtToken(queryClient, key)), + withBaseToken( + { version: 1, order }, + readUpdatedAtToken(queryClient, key), + ), ); if (saved) { queryClient.setQueryData(key, saved);