From cd344047dda9da9ac4e0edd5ef0af25114d66af2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Fri, 24 Jul 2026 11:09:30 +0200 Subject: [PATCH] test(dashboard): cover optimistic-concurrency + the serialised ring/Save writes Server: a write with the current base token succeeds and returns the advanced token; a stale base returns 409 and writes nothing (no unconditional fallback); a malformed base 422s; an omitted base keeps the unconditional write. Client: the ring payload never carries widgets and threads the base token, the shared busy gate covers either write, and the Save/ring wiring is source-pinned. Refs #581 --- .../dashboard/widgets/__tests__/route.test.ts | 100 ++++++++++++++++++ .../dashboard-layout-section.test.tsx | 66 +++++++++++- 2 files changed, 165 insertions(+), 1 deletion(-) diff --git a/src/app/api/dashboard/widgets/__tests__/route.test.ts b/src/app/api/dashboard/widgets/__tests__/route.test.ts index 258346380..9c2cf56a5 100644 --- a/src/app/api/dashboard/widgets/__tests__/route.test.ts +++ b/src/app/api/dashboard/widgets/__tests__/route.test.ts @@ -16,6 +16,8 @@ vi.mock("@/lib/db", () => ({ user: { findUnique: vi.fn(), update: vi.fn(), + // v1.32.16 (issue #581) — the guarded optimistic-concurrency write. + updateMany: vi.fn(), }, auditLog: { create: vi.fn(), @@ -741,3 +743,101 @@ describe("dashboard widgets — ring-only PUT cannot race a concurrent full-layo expect(body.data.widgets[0].visible).toBe(false); }); }); + +/** + * v1.32.16 — optimistic concurrency (issue #581). A Save that carries the + * `baseUpdatedAt` it was made against writes CONDITIONALLY on the stored row + * still carrying that token, so a committed Save can never be silently + * reverted by a later request that started from an older snapshot. A full + * layout with every top-level field present skips the preserve-read, so the + * only `findUnique` here is the post-write token read. + */ +describe("dashboard widgets — optimistic concurrency (issue #581)", () => { + // A full layout carrying every preserved field so `needsPreserveRead` + // is false and the route takes the guarded write path directly. + function fullLayoutBody(baseUpdatedAt?: string) { + return { + version: 1, + widgets: DEFAULT_DASHBOARD_LAYOUT.widgets, + comparisonBaseline: "none", + chartOverlayPrefs: {}, + selectedScoreRings: ["MED_COMPLIANCE"], + heroRingOrder: ["HEALTH_SCORE", "MED_COMPLIANCE"], + ...(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 post-write token read. + vi.mocked(prisma.user.findUnique).mockResolvedValue({ + updatedAt: advanced, + } as never); + + const res = await callPut(makeReq(fullLayoutBody(base.toISOString()))); + expect(res.status).toBe(200); + + // Wrote conditionally on the exact base token — never unconditionally. + 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(); + + // The response echoes the advanced token so the client can rebase. + const body = (await res.json()) as { data: { updatedAt: string } }; + expect(body.data.updatedAt).toBe(advanced.toISOString()); + }); + + it("rejects a stale base token with 409 and writes nothing", async () => { + // The guarded update matches zero rows — someone advanced updatedAt. + vi.mocked(prisma.user.updateMany).mockResolvedValue({ count: 0 } as never); + + const res = await callPut( + makeReq(fullLayoutBody("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("dashboard_layout_conflict"); + + // The conditional write matched no row; there is NO unconditional + // fallback, so the stored layout is left exactly as it was. + 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(fullLayoutBody("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", async () => { + // Backward-compatible path for older web builds / the native client. + vi.mocked(prisma.user.findUnique).mockResolvedValue({ + dashboardWidgetsJson: 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(fullLayoutBody())); + 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/components/settings/__tests__/dashboard-layout-section.test.tsx b/src/components/settings/__tests__/dashboard-layout-section.test.tsx index 4594cd1f6..da6a8362a 100644 --- a/src/components/settings/__tests__/dashboard-layout-section.test.tsx +++ b/src/components/settings/__tests__/dashboard-layout-section.test.tsx @@ -8,6 +8,8 @@ import { DASHBOARD_IOS_ONLY_WIDGET_IDS, DASHBOARD_WIDGET_IDS, IOS_PIN_ONLY_WIDGET_IDS, + buildRingMutationPayload, + widgetWritesBusy, type DashboardLayout, } from "@/lib/dashboard-layout"; @@ -540,10 +542,72 @@ describe(" — health-score anchor + reorder (v1.27.27)" ), "utf8", ); - expect(src).toContain("buildRingMutationPayload(next)"); + // v1.32.16 — the ring body is built via `buildRingMutationPayload`, + // now also threading the optimistic-concurrency base token. + expect(src).toContain("buildRingMutationPayload({ ...next, baseUpdatedAt"); // The pre-fix shape must never come back: spreading a query-cache // snapshot into the ring PUT body is exactly the stale-`widgets` // race issue #581 reported. expect(src).not.toMatch(/\.\.\.remote,/); }); }); + +/** + * v1.32.16 (issue #581) — a committed Save must stay authoritative against a + * concurrent instant hero-ring write. The full DOM interaction (fire a ring + * write, then a Save, assert the disabled/pending gate) can't run under this + * suite's SSR-only convention, so the load-bearing client contract is pinned + * as pure exported units plus source pins for the wiring the JSX depends on. + */ +describe("dashboard layout — Save cannot be clobbered by a ring write (#581)", () => { + it("buildRingMutationPayload never carries widgets and threads the base token", () => { + const payload = buildRingMutationPayload({ + selectedScoreRings: ["READINESS"], + heroRingOrder: ["HEALTH_SCORE", "READINESS"], + baseUpdatedAt: "2026-07-24T10:00:00.000Z", + }); + // None of the fields a stale snapshot could carry and use to overwrite + // a fresher Save. + expect(payload).not.toHaveProperty("widgets"); + expect(payload).not.toHaveProperty("comparisonBaseline"); + expect(payload).not.toHaveProperty("chartOverlayPrefs"); + // The ring selection + the guard token DO ride. + expect(payload.selectedScoreRings).toEqual(["READINESS"]); + expect(payload.baseUpdatedAt).toBe("2026-07-24T10:00:00.000Z"); + }); + + it("buildRingMutationPayload omits the base-token key when none is known", () => { + const payload = buildRingMutationPayload({ + selectedScoreRings: [], + heroRingOrder: ["HEALTH_SCORE"], + }); + expect(payload).not.toHaveProperty("baseUpdatedAt"); + }); + + it("widgetWritesBusy gates off EITHER write being in flight", () => { + expect(widgetWritesBusy(false, false)).toBe(false); + expect(widgetWritesBusy(true, false)).toBe(true); // Save pending + expect(widgetWritesBusy(false, true)).toBe(true); // ring pending + expect(widgetWritesBusy(true, true)).toBe(true); + }); + + it("the Save + ring controls bind the shared busy gate and serialise via one scope (source pin)", () => { + const src = readFileSync( + join( + process.cwd(), + "src/components/settings/dashboard-layout-section.tsx", + ), + "utf8", + ); + // Save button + ring controls both read `widgetWritesBusy(...)` so the + // gate is one source of truth — the button can never disable on + // `saveMutation.isPending` alone (the pre-fix hole). + expect(src).toContain("widgetWritesBusy("); + // Both writes share a mutation scope so they run serially at the client. + expect(src).toContain("scope: { id: DASHBOARD_WIDGETS_MUTATION_SCOPE }"); + // Each write sends the optimistic-concurrency base token. + expect(src).toContain("baseUpdatedAt: readBaseToken()"); + // A 409 is handled (refetch + gentle nudge), not swallowed as a hard error. + expect(src).toContain("err.status === 409"); + }); +});