From 8736f44442c1d9151281645881c46bd232d5906c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 23 Jul 2026 09:15:54 +0200 Subject: [PATCH 1/5] fix(dashboard): stop the score-ring PUT from resending a stale layout The Settings page's instant score-ring toggle PUT the full dashboard layout, built from the client's cached `remote` snapshot. That snapshot can go stale mid-flight: if a normal tile/chart Save committed a newer layout first, the ring request's explicit stale `widgets` (and `comparisonBaseline` / `chartOverlayPrefs`) still won on write and silently reverted the just-saved layout whenever it resolved last. The ring mutation now PUTs only the two ring fields (`buildRingMutationPayload`). `widgets` joins the existing preserve-when-absent merge (`LAYOUT_FIELD_MERGE_DISPOSITION`) so an omitted field is carried forward from whatever is currently stored instead of a client-held copy, closing the race regardless of request order. --- .../dashboard/widgets/__tests__/route.test.ts | 105 ++++++++++++++++++ src/app/api/dashboard/widgets/route.ts | 16 ++- .../dashboard-layout-section.test.tsx | 20 +++- .../settings/dashboard-layout-section.tsx | 34 ++++-- src/lib/__tests__/dashboard-layout.test.ts | 61 +++++++++- src/lib/dashboard-layout.ts | 50 ++++++++- 6 files changed, 262 insertions(+), 24 deletions(-) diff --git a/src/app/api/dashboard/widgets/__tests__/route.test.ts b/src/app/api/dashboard/widgets/__tests__/route.test.ts index 5611f3fc3..258346380 100644 --- a/src/app/api/dashboard/widgets/__tests__/route.test.ts +++ b/src/app/api/dashboard/widgets/__tests__/route.test.ts @@ -62,7 +62,13 @@ import { DASHBOARD_WIDGET_IDS, DASHBOARD_IOS_ONLY_WIDGET_IDS, DASHBOARD_WIDGET_CATALOGUE_IDS, + DEFAULT_DASHBOARD_LAYOUT, serializeDashboardLayout, + // v1.32.1 — the exact payload builder `ringMutation.mutationFn` calls + // (see the settings component). Importing the shipped function rather + // than re-implementing its shape means this test exercises the REAL + // client contract against the REAL route, not a hand-rolled stand-in. + buildRingMutationPayload, type DashboardLayout, } from "@/lib/dashboard-layout"; @@ -636,3 +642,102 @@ describe("dashboard widgets — preserve-when-absent on PUT", () => { } }); }); + +/** + * v1.32.1 — regression for issue #581: dashboard layout changes silently + * overwritten by a concurrent score-ring save. + * + * The scenario from the report: the user edits tile/chart visibility + * (local draft B), then reorders/toggles a hero score ring before hitting + * Save. The ring PUT (`ringMutation`) fires immediately, built from + * whatever layout the client had cached (layout A — the state BEFORE the + * draft edit). The normal Save button then PUTs the full draft (layout B) + * and commits first. If the earlier-fired ring request resolves AFTER + * Save, its body — built from the stale A snapshot — used to carry + * `widgets: A` explicitly, and an explicitly-present field always wins on + * write. Save's B silently reverted to A even though the ring PUT reported + * 200 and only meant to touch the ring selection. + * + * The fix is `buildRingMutationPayload` (imported from the real component + * module, not re-implemented here): it never includes `widgets` at all. + * This test proves the SERVER half of the fix — a PUT shaped exactly like + * the shipped ring mutation's body preserves whatever layout is CURRENTLY + * stored, so the request that resolves last can no longer matter. + */ +describe("dashboard widgets — ring-only PUT cannot race a concurrent full-layout Save (regression #581)", () => { + it("preserves the widgets a concurrent Save already committed, even though the ring request started from a stale snapshot", async () => { + // The state AFTER the normal Save committed layout B — a tile turned + // off that was on in the stale snapshot A the ring mutation started + // from. Starts from the FULL default widget catalogue (rather than a + // single-widget array) so `resolveDashboardLayout`'s auto-upgrade + // append (missing catalogue ids get seeded invisible) is a no-op here + // and the persisted array can be compared for exact equality below. + const savedLayoutB: DashboardLayout = serializeDashboardLayout({ + version: 1, + widgets: DEFAULT_DASHBOARD_LAYOUT.widgets.map((w) => + w.id === "weight" ? { ...w, visible: false, tileVisible: false } : w, + ), + comparisonBaseline: "lastYear", + selectedScoreRings: ["MED_COMPLIANCE"], + heroRingOrder: ["HEALTH_SCORE", "MED_COMPLIANCE"], + }); + vi.mocked(prisma.user.findUnique).mockResolvedValue({ + dashboardWidgetsJson: savedLayoutB, + } as never); + vi.mocked(prisma.user.update).mockResolvedValue({} as never); + + // The ring mutation's real, shipped payload — built without any + // knowledge of layout B. If this were still `{...staleA, ...}` the + // stale `widgets`/`comparisonBaseline` would land here explicitly and + // overwrite B; `buildRingMutationPayload` never carries them. + const ringOnlyBody = buildRingMutationPayload({ + selectedScoreRings: ["READINESS"], + heroRingOrder: ["HEALTH_SCORE", "READINESS"], + }); + + const res = await callPut(makeReq(ringOnlyBody)); + expect(res.status).toBe(200); + + const updateArg = vi.mocked(prisma.user.update).mock + .calls[0]?.[0] as unknown as { + data: { dashboardWidgetsJson: DashboardLayout }; + }; + // B's widgets (and comparisonBaseline) survive — the ring PUT never + // touched them, so nothing it carries can outrace the Save that + // already committed. + expect(updateArg.data.dashboardWidgetsJson.widgets).toEqual( + savedLayoutB.widgets, + ); + expect(updateArg.data.dashboardWidgetsJson.comparisonBaseline).toBe( + "lastYear", + ); + // The ring selection itself DID apply. + expect(updateArg.data.dashboardWidgetsJson.selectedScoreRings).toEqual([ + "READINESS", + ]); + + const body = (await res.json()) as { data: DashboardLayout }; + expect(body.data.widgets[0].visible).toBe(false); + expect(body.data.widgets[0].tileVisible).toBe(false); + expect(body.data.selectedScoreRings).toEqual(["READINESS"]); + }); + + it("still accepts a normal full-layout Save (widgets present + replace semantics unchanged)", async () => { + vi.mocked(prisma.user.findUnique).mockResolvedValue({ + dashboardWidgetsJson: null, + } as never); + vi.mocked(prisma.user.update).mockResolvedValue({} as never); + + const res = await callPut( + makeReq({ + version: 1, + widgets: [ + { id: "weight", visible: false, tileVisible: false, order: 0 }, + ], + }), + ); + expect(res.status).toBe(200); + const body = (await res.json()) as { data: DashboardLayout }; + expect(body.data.widgets[0].visible).toBe(false); + }); +}); diff --git a/src/app/api/dashboard/widgets/route.ts b/src/app/api/dashboard/widgets/route.ts index 790501f5e..d7c8c97f6 100644 --- a/src/app/api/dashboard/widgets/route.ts +++ b/src/app/api/dashboard/widgets/route.ts @@ -84,7 +84,15 @@ const layoutSchema = z.object({ // catalogue (42 ids). The cap only has to clear the catalogue size — // the enum bounds each entry and the id set is closed — so it carries // headroom rather than tracking the count exactly. - .max(50), + .max(50) + // v1.32.1 — optional + preserve-when-absent (issue #581). The + // Settings page's instant score-ring toggle now PUTs ONLY the ring + // fields; omitting `widgets` lets the server carry forward whatever + // is CURRENTLY stored instead of the ring mutation resending a + // client-held snapshot that can go stale mid-flight and race a + // concurrent full-layout Save. A normal Save still always sends + // `widgets` — this only widens what a PARTIAL PUT can leave out. + .optional(), // v1.4.16 phase B8 — comparison baseline (Vormonat / Vorjahr) rides // on the layout blob per research §7 Q3 (no Prisma migration). Optional // so v1.4.15 clients that don't know the field can still PUT. @@ -285,7 +293,11 @@ export const PUT = apiHandler(async (request: NextRequest) => { // visibility / order, while chart prefs are PUT through their own route // (`/api/dashboard/chart-overlay-prefs`), the hero rings come from // Settings, and `comparisonBaseline` is web-only — an omission from any - // one surface must not reset a choice made on another. + // one surface must not reset a choice made on another. `widgets` itself + // joined this set in v1.32.1 (issue #581): the Settings page's instant + // score-ring PUT omits it entirely so it always carries forward + // whatever is CURRENTLY stored, rather than a client-held snapshot that + // could be stale relative to a concurrent full-layout Save. // // The preserve set is not a list maintained here: it is derived from // `LAYOUT_FIELD_MERGE_DISPOSITION`, which the type system forces to cover diff --git a/src/components/settings/__tests__/dashboard-layout-section.test.tsx b/src/components/settings/__tests__/dashboard-layout-section.test.tsx index 1be642928..33eaa1ea4 100644 --- a/src/components/settings/__tests__/dashboard-layout-section.test.tsx +++ b/src/components/settings/__tests__/dashboard-layout-section.test.tsx @@ -69,10 +69,7 @@ vi.mock("@/hooks/use-auth", () => ({ })); import { I18nProvider } from "@/lib/i18n/context"; -import { - DashboardLayoutSection, - reorderWidgets, -} from "../dashboard-layout-section"; +import { DashboardLayoutSection, reorderWidgets } from "../dashboard-layout-section"; function render(node: React.ReactElement, locale: "en" | "de" = "en") { return renderToStaticMarkup( @@ -531,4 +528,19 @@ describe(" — health-score anchor + reorder (v1.27.27)" // The order is the single source of truth: selection is derived from it. expect(src).toMatch(/id !== HEALTH_SCORE_RING_ID/); }); + + it("the ring mutation builds its body via buildRingMutationPayload, not a `...remote` spread (regression #581 source pin)", () => { + const src = readFileSync( + join( + process.cwd(), + "src/components/settings/dashboard-layout-section.tsx", + ), + "utf8", + ); + expect(src).toContain("buildRingMutationPayload(next)"); + // 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,/); + }); }); diff --git a/src/components/settings/dashboard-layout-section.tsx b/src/components/settings/dashboard-layout-section.tsx index 20a4bb507..ddaf3bd5e 100644 --- a/src/components/settings/dashboard-layout-section.tsx +++ b/src/components/settings/dashboard-layout-section.tsx @@ -47,6 +47,7 @@ import { MAX_SELECTED_SCORE_RINGS, HEALTH_SCORE_RING_ID, resolveHeroRingOrder, + buildRingMutationPayload, } from "@/lib/dashboard-layout"; import { Select, @@ -327,23 +328,34 @@ export function DashboardLayoutSection({ id }: { id: string }) { * machine the rest of the section keeps. The first cut routed ring * toggles through the draft, and the combination of an unflushed * draft, the server snapshot cache, and client staleness read as - * "flipping does nothing, rings appear later". The mutation PUTs the - * SERVER layout copy with only `selectedScoreRings` changed (an open - * draft's other edits stay unsent and untouched), optimistically - * updates the widgets query, and invalidates the dashboard snapshot on - * settle so the hero reflects the choice on the next visit. + * "flipping does nothing, rings appear later". The mutation + * optimistically updates the widgets query and invalidates the + * dashboard snapshot on settle so the hero reflects the choice on the + * next visit. + * + * v1.32.1 — the PUT body used to be the FULL server layout copy, + * spreading this component's `remote` query-cache snapshot in ahead of + * the two ring fields. That snapshot can be stale by the time the + * request actually lands: if the normal + * tile/chart Save button committed a newer layout first, this + * request's explicit — present, not omitted — stale `widgets` (and + * `comparisonBaseline` / `chartOverlayPrefs`) still won on write and + * silently reverted the just-saved layout (issue #581). The payload + * now carries ONLY the ring fields; the server's preserve-when-absent + * merge (`LAYOUT_FIELD_MERGE_DISPOSITION` — `widgets` joined it in the + * same release) carries every omitted field forward from whatever is + * CURRENTLY stored, so this request can no longer race a concurrent + * Save no matter which one lands last. */ const ringMutation = useMutation({ mutationFn: async (next: { selectedScoreRings: ScoreRingId[]; heroRingOrder: HeroRingId[]; }) => { - if (!remote) throw new Error("layout not loaded"); - return apiPut("/api/dashboard/widgets", { - ...remote, - selectedScoreRings: next.selectedScoreRings, - heroRingOrder: next.heroRingOrder, - }); + return apiPut( + "/api/dashboard/widgets", + buildRingMutationPayload(next), + ); }, onMutate: async (next) => { await queryClient.cancelQueries({ diff --git a/src/lib/__tests__/dashboard-layout.test.ts b/src/lib/__tests__/dashboard-layout.test.ts index 8379f5c0c..0e091ce05 100644 --- a/src/lib/__tests__/dashboard-layout.test.ts +++ b/src/lib/__tests__/dashboard-layout.test.ts @@ -13,6 +13,7 @@ import { PRESERVED_LAYOUT_FIELDS, layoutNeedsPreserveRead, mergePreservedLayoutFields, + buildRingMutationPayload, type DashboardLayout, } from "@/lib/dashboard-layout"; @@ -1006,20 +1007,24 @@ describe("layout field merge disposition", () => { } }); - it("preserves comparisonBaseline alongside the other client-owned fields", () => { + it("preserves comparisonBaseline and widgets alongside the other client-owned fields", () => { // The regression itself: `comparisonBaseline` must be in the preserve - // set, not merely present in the disposition map. + // set, not merely present in the disposition map. `widgets` joined it in + // v1.32.1 (issue #581) — see the dedicated describe block below. expect([...PRESERVED_LAYOUT_FIELDS].sort()).toEqual([ "chartOverlayPrefs", "comparisonBaseline", "heroRingOrder", "selectedScoreRings", + "widgets", ]); }); - it("replaces the fields the request itself owns", () => { + it("replaces only the field every request must state", () => { + // `version` is the sole `"replace"` field left once `widgets` joined + // the preserve set — a PUT that omits it is rejected by Zod, so there + // is never a stored value to fall back to. expect(LAYOUT_FIELD_MERGE_DISPOSITION.version).toBe("replace"); - expect(LAYOUT_FIELD_MERGE_DISPOSITION.widgets).toBe("replace"); }); it("skips the stored-layout read when the client sent every preserved field", () => { @@ -1086,3 +1091,51 @@ describe("layout field merge disposition", () => { expect(merged.widgets).toEqual(fullyPopulatedLayout.widgets); }); }); + +/** + * v1.32.1 — regression for issue #581 (dashboard layout race). The + * Settings page's instant score-ring PUT used to resend the FULL layout, + * built from the `remote` query-cache snapshot + * (`{...remote, selectedScoreRings, heroRingOrder}`). That snapshot can be + * stale relative to an in-flight or already-committed tile/chart Save; + * because `widgets` was a `"replace"`-disposition field on the server, an + * explicitly-present stale copy always won on write and silently reverted + * the just-saved layout. + * + * `buildRingMutationPayload` is the exact function + * `ringMutation.mutationFn` (in `dashboard-layout-section.tsx`) calls to + * build its request body. Pinning its shape here — no `widgets`, no + * `comparisonBaseline`, no `chartOverlayPrefs` — is what makes the race + * structurally impossible: a payload that never carries those fields + * cannot overwrite them no matter which request lands last. See + * `src/app/api/dashboard/widgets/__tests__/route.test.ts` for the + * server-side half — a PUT shaped like this preserves whatever layout is + * CURRENTLY stored, not a client-held snapshot. + */ +describe("buildRingMutationPayload — instant score-ring PUT payload (regression #581)", () => { + it("carries only version + the two ring fields", () => { + const payload = buildRingMutationPayload({ + selectedScoreRings: ["READINESS"], + heroRingOrder: ["HEALTH_SCORE", "READINESS"], + }); + expect(Object.keys(payload).sort()).toEqual([ + "heroRingOrder", + "selectedScoreRings", + "version", + ]); + expect(payload.version).toBe(1); + expect(payload.selectedScoreRings).toEqual(["READINESS"]); + expect(payload.heroRingOrder).toEqual(["HEALTH_SCORE", "READINESS"]); + }); + + it("never includes widgets, comparisonBaseline, or chartOverlayPrefs — the fields a concurrent Save owns", () => { + const payload = buildRingMutationPayload({ + selectedScoreRings: ["READINESS"], + heroRingOrder: ["HEALTH_SCORE", "READINESS"], + }); + expect(payload.widgets).toBeUndefined(); + expect(payload.comparisonBaseline).toBeUndefined(); + expect(payload.chartOverlayPrefs).toBeUndefined(); + expect("widgets" in payload).toBe(false); + }); +}); diff --git a/src/lib/dashboard-layout.ts b/src/lib/dashboard-layout.ts index 54447cc5d..4c4e790a4 100644 --- a/src/lib/dashboard-layout.ts +++ b/src/lib/dashboard-layout.ts @@ -574,8 +574,8 @@ export interface DashboardLayout { * omits it. * * - `"replace"` — the field is the point of the request. A PUT that omits it - * is either rejected by Zod (`version`, `widgets` are required) or means - * the caller genuinely has nothing to say. + * is rejected by Zod (`version` is required) — the caller always has + * something to say about a `"replace"` field. * - `"preserve"` — the field is owned by a surface OTHER than the one making * this request, so an omission means "I don't know about this", never * "clear it". The route reads the stored layout and carries the existing @@ -591,6 +591,18 @@ export interface DashboardLayout { * clamps a missing value to `"none"`, so every native layout save wiped the * baseline the user had picked on the web. * + * `widgets` moved from `"replace"` to `"preserve"` in v1.32.1 (issue #581). + * The Settings page's instant score-ring toggle used to resend the FULL + * layout — including `widgets` — built from the query cache's `remote` + * snapshot. That snapshot can be stale by the time the ring request lands: + * if a normal tile/chart Save committed a newer layout first, the + * ring request's explicit (present, not omitted) stale `widgets` value + * still won on write and silently reverted the just-saved layout. Making + * `widgets` optional + preserve-when-absent lets the ring mutation omit it + * entirely, so the field it never touches can no longer race a concurrent + * Save — the route always carries forward whatever is CURRENTLY stored, + * not a client-held copy. + * * `satisfies Record` is the load-bearing part. * Adding a field to `DashboardLayout` without adding it here is a TYPE error, * so the next field cannot repeat the `comparisonBaseline` mistake by @@ -598,7 +610,7 @@ export interface DashboardLayout { */ export const LAYOUT_FIELD_MERGE_DISPOSITION = { version: "replace", - widgets: "replace", + widgets: "preserve", comparisonBaseline: "preserve", chartOverlayPrefs: "preserve", selectedScoreRings: "preserve", @@ -647,6 +659,38 @@ export function mergePreservedLayoutFields( return merged; } +/** + * v1.32.1 — pure payload builder for the Settings page's instant score-ring + * PUT (`ringMutation` in `dashboard-layout-section.tsx`). Lives in this + * client-safe module (imported by both the client component and the server + * route already) rather than the "use client" component file, so a + * server-side regression test can import it without pulling the component's + * UI dependency graph across the client/server boundary — see + * `src/app/api/dashboard/widgets/__tests__/route.test.ts`. + * + * Deliberately returns ONLY the ring fields (plus `version`) — no + * `widgets`, `comparisonBaseline`, or `chartOverlayPrefs`. Before v1.32.1 the + * ring mutation resent the FULL layout built from the client's `remote` + * query-cache snapshot, which can be stale by the time the request lands: if + * a normal tile/chart Save committed a newer layout first, the ring + * request's explicit — present, not omitted — stale `widgets` still won on + * write and silently reverted the just-saved layout (issue #581). Those + * three fields are all `"preserve"`-disposition here + * (`LAYOUT_FIELD_MERGE_DISPOSITION`): omitting them means the route always + * carries forward whatever is CURRENTLY stored, so this payload can no + * longer race a concurrent Save no matter which request lands last. + */ +export function buildRingMutationPayload(next: { + selectedScoreRings: ScoreRingId[]; + heroRingOrder: HeroRingId[]; +}): Partial { + return { + version: DASHBOARD_LAYOUT_VERSION, + selectedScoreRings: next.selectedScoreRings, + heroRingOrder: next.heroRingOrder, + }; +} + const DASHBOARD_LAYOUT_VERSION = 1; /** From 1371bb1cf9d33c80d096e02abe082f7c8d077489 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 23 Jul 2026 09:25:16 +0200 Subject: [PATCH 2/5] fix(measurement-reminders): recompute nextDueAt from the field actually set, not a nullish fallback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PATCH /api/measurement-reminders/{id} persisted an explicit null cadence clear correctly, but the immediate nextDueAt recomputation merged fields with `updateValue ?? existingValue` — a JSON null and an omitted field are both nullish, so the merge silently fell back to the OLD cadence for one cycle. This hit interval-to-RRULE and RRULE-to-interval transitions and an explicit anchorDate clear alike. The merge now keys off Object.hasOwn(updateData, field): updateData already encodes exactly which fields are being persisted, including the mutual-exclusivity auto-clear (setting one cadence nulls the other), so recomputation can never diverge from what the row ends up holding. --- .../[id]/__tests__/route.test.ts | 229 ++++++++++++++++++ .../api/measurement-reminders/[id]/route.ts | 35 ++- 2 files changed, 255 insertions(+), 9 deletions(-) create mode 100644 src/app/api/measurement-reminders/[id]/__tests__/route.test.ts diff --git a/src/app/api/measurement-reminders/[id]/__tests__/route.test.ts b/src/app/api/measurement-reminders/[id]/__tests__/route.test.ts new file mode 100644 index 000000000..1a9ac856f --- /dev/null +++ b/src/app/api/measurement-reminders/[id]/__tests__/route.test.ts @@ -0,0 +1,229 @@ +/** + * v1.32.1 — regression for iOS coordination issue #62: the `nextDueAt` + * recomputation on `PATCH /api/measurement-reminders/{id}` used to read an + * explicit `null` cadence clear as "field omitted" (`?? existing.field` + * cannot distinguish the two — both are nullish) and recompute against the + * STALE cadence for one cycle, even though the persisted row itself already + * carried the correct cleared value. + * + * Every test below drives the REAL `computeReminderNextDueAt` (not a mock) + * so the "correct" and "stale" comparison values are genuine recurrence- + * engine output, not hand-picked dates — and asserts the fixtures actually + * differ before trusting the route's answer, so a fixture collision can't + * hide a regression. + */ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { NextRequest } from "next/server"; + +vi.mock("@/lib/api-handler", () => ({ + apiHandler: unknown>(fn: T) => fn, + requireAuth: vi.fn(async () => ({ user: { id: "u1", locale: "en" } })), +})); + +vi.mock("@/lib/auth/audit", () => ({ + auditLog: vi.fn().mockResolvedValue(undefined), +})); + +vi.mock("@/lib/logging/context", () => ({ annotate: vi.fn() })); + +const findFirstMock = vi.fn(); +const updateMock = vi.fn(); +const findUserMock = vi.fn(); +const auditLogCreateMock = vi.fn(); +vi.mock("@/lib/db", () => ({ + prisma: { + measurementReminder: { + findFirst: (...a: unknown[]) => findFirstMock(...a), + update: (...a: unknown[]) => updateMock(...a), + }, + user: { findUnique: (...a: unknown[]) => findUserMock(...a) }, + auditLog: { create: (...a: unknown[]) => auditLogCreateMock(...a) }, + }, +})); + +import { PATCH } from "../route"; +import { computeReminderNextDueAt } from "@/lib/measurement-reminders/scheduling"; + +const BASE_ROW = { + id: "r1", + userId: "u1", + label: "Blutdruck messen", + measurementType: "BLOOD_PRESSURE_SYS", + intervalDays: 30, + rrule: null as string | null, + anchorDate: null as Date | null, + endsOn: null, + origin: "VORSORGE", + notifyHour: 9, + location: null, + nextDueAt: new Date("2026-01-31T09:00:00Z"), + lastSatisfiedAt: null as Date | null, + enabled: true, + createdAt: new Date("2026-01-01T00:00:00Z"), + updatedAt: new Date("2026-01-01T00:00:00Z"), + deletedAt: null, +}; + +const NOW = new Date("2026-06-15T08:00:00.000Z"); +const TZ = "Europe/Berlin"; + +function makeRequest(body: unknown): NextRequest { + return new NextRequest("http://localhost/api/measurement-reminders/r1", { + method: "PATCH", + headers: { "content-type": "application/json" }, + body: JSON.stringify(body), + }); +} + +const params = { params: Promise.resolve({ id: "r1" }) }; + +beforeEach(() => { + vi.clearAllMocks(); + findUserMock.mockResolvedValue({ timezone: TZ }); + updateMock.mockImplementation( + async ({ data }: { data: Record }) => ({ + ...BASE_ROW, + ...data, + }), + ); + vi.useFakeTimers(); + vi.setSystemTime(NOW); +}); + +afterEach(() => { + vi.useRealTimers(); +}); + +describe("PATCH /api/measurement-reminders/[id] — nextDueAt honours explicit-null cadence clears (regression iOS #62)", () => { + it("interval → RRULE: recomputes off the NEW rrule, not the just-cleared interval", async () => { + // Existing rolling reminder: intervalDays=30, rrule=null. + findFirstMock.mockResolvedValue(BASE_ROW); + + const res = await PATCH( + makeRequest({ intervalDays: null, rrule: "FREQ=YEARLY" }), + params, + ); + expect(res.status).toBe(200); + + const persisted = updateMock.mock.calls[0]?.[0]?.data as Record< + string, + unknown + >; + // The explicit-null clear + the new rrule both persist correctly — + // this half was never broken. + expect(persisted.intervalDays).toBeNull(); + expect(persisted.rrule).toBe("FREQ=YEARLY"); + + const correct = computeReminderNextDueAt( + { ...BASE_ROW, intervalDays: null, rrule: "FREQ=YEARLY" }, + TZ, + NOW, + ); + // The bug: `?? existing` reads the cleared intervalDays as "omitted" + // and recomputes against the OLD rolling cadence for one cycle. + const staleBug = computeReminderNextDueAt( + { ...BASE_ROW, intervalDays: 30, rrule: null }, + TZ, + NOW, + ); + // Mutation-check guard: the fixtures must actually diverge, or this + // test can't tell a fix from a no-op. + expect(correct).not.toEqual(staleBug); + + expect(persisted.nextDueAt).toEqual(correct); + expect(persisted.nextDueAt).not.toEqual(staleBug); + }); + + it("RRULE → interval: recomputes off the NEW interval, not the just-cleared rrule (incl. its BYHOUR)", async () => { + findFirstMock.mockResolvedValue({ + ...BASE_ROW, + intervalDays: null, + rrule: "FREQ=DAILY;BYHOUR=7,19", + }); + + const res = await PATCH( + makeRequest({ rrule: null, intervalDays: 14 }), + params, + ); + expect(res.status).toBe(200); + + const persisted = updateMock.mock.calls[0]?.[0]?.data as Record< + string, + unknown + >; + expect(persisted.rrule).toBeNull(); + expect(persisted.intervalDays).toBe(14); + + const correct = computeReminderNextDueAt( + { ...BASE_ROW, intervalDays: 14, rrule: null }, + TZ, + NOW, + ); + const staleBug = computeReminderNextDueAt( + { ...BASE_ROW, intervalDays: null, rrule: "FREQ=DAILY;BYHOUR=7,19" }, + TZ, + NOW, + ); + expect(correct).not.toEqual(staleBug); + + expect(persisted.nextDueAt).toEqual(correct); + expect(persisted.nextDueAt).not.toEqual(staleBug); + }); + + it("explicit anchorDate: null clear recomputes off no-anchor, not the stale anchor", async () => { + findFirstMock.mockResolvedValue({ + ...BASE_ROW, + intervalDays: 30, + anchorDate: new Date("2026-01-15T00:00:00Z"), + }); + + const res = await PATCH(makeRequest({ anchorDate: null }), params); + expect(res.status).toBe(200); + + const persisted = updateMock.mock.calls[0]?.[0]?.data as Record< + string, + unknown + >; + expect(persisted.anchorDate).toBeNull(); + + const correct = computeReminderNextDueAt( + { ...BASE_ROW, intervalDays: 30, anchorDate: null }, + TZ, + NOW, + ); + const staleBug = computeReminderNextDueAt( + { + ...BASE_ROW, + intervalDays: 30, + anchorDate: new Date("2026-01-15T00:00:00Z"), + }, + TZ, + NOW, + ); + expect(correct).not.toEqual(staleBug); + + expect(persisted.nextDueAt).toEqual(correct); + expect(persisted.nextDueAt).not.toEqual(staleBug); + }); + + it("omitted cadence fields on a label-only edit leave the cadence — and nextDueAt — unchanged", async () => { + findFirstMock.mockResolvedValue(BASE_ROW); + + const res = await PATCH(makeRequest({ label: "New label" }), params); + expect(res.status).toBe(200); + + const persisted = updateMock.mock.calls[0]?.[0]?.data as Record< + string, + unknown + >; + expect(persisted.label).toBe("New label"); + // The cadence keys are never written at all on a label-only edit — no + // clear, no touch. + expect("intervalDays" in persisted).toBe(false); + expect("rrule" in persisted).toBe(false); + expect("anchorDate" in persisted).toBe(false); + + const expected = computeReminderNextDueAt(BASE_ROW, TZ, NOW); + expect(persisted.nextDueAt).toEqual(expected); + }); +}); diff --git a/src/app/api/measurement-reminders/[id]/route.ts b/src/app/api/measurement-reminders/[id]/route.ts index b163194a6..aad5e46e0 100644 --- a/src/app/api/measurement-reminders/[id]/route.ts +++ b/src/app/api/measurement-reminders/[id]/route.ts @@ -133,18 +133,35 @@ export const PATCH = apiHandler( // Recompute next-due against the merged cadence. Floor the search at // the last-satisfied instant (or now) so a cadence edit re-anchors // cleanly off the user's last fulfilment. + // + // v1.32.1 — `Object.hasOwn(updateData, field)` replaces a `?? existing` + // fallback here (issue #62). `??` cannot tell an explicit `null` clear + // apart from an omitted field — both are nullish — so a PATCH that + // cleared `intervalDays` to switch a reminder onto an RRULE still + // recomputed `nextDueAt` against the OLD rolling interval for one + // cycle (rolling cadence wins the dispatcher's precedence order, so the + // stale interval silently overrode the just-set RRULE). The same gap + // hit the RRULE → interval direction and an explicit `anchorDate: null` + // clear. `updateData` already encodes the field-by-field truth of what + // is ACTUALLY being persisted — including the mutual-exclusivity + // auto-clear a few lines up — so keying off its own keys (present, even + // when the value is `null`) can never diverge from what the database + // ends up holding. const timezone = await resolveTimezone(user.id); const now = new Date(); const merged: ReminderScheduleInput = { - intervalDays: - (updateData.intervalDays as number | null | undefined) ?? - existing.intervalDays, - rrule: (updateData.rrule as string | null | undefined) ?? existing.rrule, - anchorDate: - (updateData.anchorDate as Date | null | undefined) ?? - existing.anchorDate, - notifyHour: - (updateData.notifyHour as number | undefined) ?? existing.notifyHour, + intervalDays: Object.hasOwn(updateData, "intervalDays") + ? (updateData.intervalDays as number | null) + : existing.intervalDays, + rrule: Object.hasOwn(updateData, "rrule") + ? (updateData.rrule as string | null) + : existing.rrule, + anchorDate: Object.hasOwn(updateData, "anchorDate") + ? (updateData.anchorDate as Date | null) + : existing.anchorDate, + notifyHour: Object.hasOwn(updateData, "notifyHour") + ? (updateData.notifyHour as number) + : existing.notifyHour, lastSatisfiedAt: existing.lastSatisfiedAt, createdAt: existing.createdAt, // v1.18.1 — preserve the course window so an edit of a finite From d750c1827e07b5d93021cd982da31880fdef9a69 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 23 Jul 2026 09:28:39 +0200 Subject: [PATCH 3/5] fix(settings): stop ntfy PUT from clearing the stored auth token on unrelated edits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PUT /api/settings/ntfy reconstructed the entire stored config from the request body. GET never returns the write-only authToken (only hasAuthToken), so clients correctly omit it on any edit that doesn't touch the credential — but the route treated an omitted or empty token as "clear it", silently destroying a working notification credential on routine edits like toggling enabled. An omitted or empty authToken now preserves the currently stored value; only a non-empty value replaces it. Mirrors the identical headerValue preserve-on-empty contract already shipped on the webhook channel route. --- .../__tests__/channel-enabled-routes.test.ts | 118 ++++++++++++++++++ src/app/api/settings/ntfy/route.ts | 23 +++- 2 files changed, 140 insertions(+), 1 deletion(-) diff --git a/src/app/api/settings/__tests__/channel-enabled-routes.test.ts b/src/app/api/settings/__tests__/channel-enabled-routes.test.ts index c5cc73548..c09dc31e9 100644 --- a/src/app/api/settings/__tests__/channel-enabled-routes.test.ts +++ b/src/app/api/settings/__tests__/channel-enabled-routes.test.ts @@ -298,3 +298,121 @@ describe("notification channel enable-only routes", () => { }, ); }); + +/** + * v1.32.1 — regression for iOS coordination issue #63: `PUT + * /api/settings/ntfy` reconstructed the entire stored config from the + * request body. `GET` never returns the write-only `authToken` (only + * `hasAuthToken: true/false`), so both native and web clients correctly + * OMIT — or round-trip an empty string through — the field on any + * unrelated edit. Rebuilding `config` without a preserve fallback + * silently dropped a working token on the next save. + * + * The fix mirrors the pre-existing `headerValue` preserve-on-empty + * contract in `src/app/api/settings/webhook/route.ts` (asserted here too, + * as a parity pin — that route was never broken, but any future drift + * between the two should fail the same suite). Covers exactly the four + * cases the report asks for. + */ +describe("PUT /api/settings/ntfy — write-only authToken preserve-on-omit contract", () => { + function upsertConfig() { + const call = vi.mocked(prisma.notificationChannel.upsert).mock + .calls[0]?.[0] as { + create: { config: string }; + update: { config: string }; + }; + // The route always upserts — assert both arms agree, then decode one. + expect(call.update.config).toBe(call.create.config); + return JSON.parse(call.update.config.replace(/^encrypted:/, "")) as { + serverUrl: string; + topic: string; + authToken?: string; + }; + } + + it("an omitted authToken preserves the stored secret", async () => { + vi.mocked(prisma.notificationChannel.findUnique).mockResolvedValue({ + config: + 'encrypted:{"serverUrl":"https://ntfy.sh","topic":"saved-topic","authToken":"secret"}', + } as never); + + const response = await put("ntfy", { + serverUrl: "https://ntfy.sh", + topic: "saved-topic", + enabled: true, + }); + + expect(response.status).toBe(200); + expect(upsertConfig().authToken).toBe("secret"); + }); + + it("an explicit empty-string authToken preserves the stored secret (matches current clients)", async () => { + vi.mocked(prisma.notificationChannel.findUnique).mockResolvedValue({ + config: + 'encrypted:{"serverUrl":"https://ntfy.sh","topic":"saved-topic","authToken":"secret"}', + } as never); + + const response = await put("ntfy", { + serverUrl: "https://ntfy.sh", + topic: "saved-topic", + authToken: "", + enabled: true, + }); + + expect(response.status).toBe(200); + expect(upsertConfig().authToken).toBe("secret"); + }); + + it("a non-empty authToken replaces the stored secret", async () => { + vi.mocked(prisma.notificationChannel.findUnique).mockResolvedValue({ + config: + 'encrypted:{"serverUrl":"https://ntfy.sh","topic":"saved-topic","authToken":"old-secret"}', + } as never); + + const response = await put("ntfy", { + serverUrl: "https://ntfy.sh", + topic: "saved-topic", + authToken: "new-secret", + enabled: true, + }); + + expect(response.status).toBe(200); + expect(upsertConfig().authToken).toBe("new-secret"); + }); + + it("creating without a token remains valid where anonymous ntfy is allowed", async () => { + vi.mocked(prisma.notificationChannel.findUnique).mockResolvedValue(null); + + const response = await put("ntfy", { + serverUrl: "https://ntfy.sh", + topic: "public-topic", + enabled: false, + }); + + expect(response.status).toBe(200); + const config = upsertConfig(); + expect(config.authToken).toBeUndefined(); + expect(config.topic).toBe("public-topic"); + }); + + it("parity pin: the webhook headerValue preserve-on-empty contract this fix mirrors is intact", async () => { + vi.mocked(prisma.notificationChannel.findUnique).mockResolvedValue({ + config: + 'encrypted:{"url":"https://saved.example/hook","headerName":"Authorization","headerValue":"secret"}', + } as never); + + const response = await put("webhook", { + url: "https://saved.example/hook", + headerName: "Authorization", + enabled: true, + }); + + expect(response.status).toBe(200); + const call = vi.mocked(prisma.notificationChannel.upsert).mock + .calls[0]?.[0] as { update: { config: string } }; + const config = JSON.parse( + call.update.config.replace(/^encrypted:/, ""), + ) as { headerValue?: string }; + expect(config.headerValue).toBe("secret"); + }); +}); diff --git a/src/app/api/settings/ntfy/route.ts b/src/app/api/settings/ntfy/route.ts index 0d916384f..f9dc4f043 100644 --- a/src/app/api/settings/ntfy/route.ts +++ b/src/app/api/settings/ntfy/route.ts @@ -113,10 +113,31 @@ export const PUT = apiHandler(async (request: NextRequest) => { ); } + // v1.32.1 — preserve an existing auth token when the client sends an + // empty one (issue #63). GET never returns the secret (`hasAuthToken` + // only), so both native and web clients correctly omit — or round-trip + // an empty string through — the field on any unrelated edit (e.g. + // toggling `enabled`). Reconstructing `config` from scratch without this + // fallback silently dropped the token on every such save. Mirrors the + // identical `headerValue` preserve-on-empty contract in + // `src/app/api/settings/webhook/route.ts`. A non-empty value replaces it. + let nextAuthToken = authToken || undefined; + if (!nextAuthToken) { + const existing = await prisma.notificationChannel.findUnique({ + where: { userId_type: { userId: user.id, type: "NTFY" } }, + }); + if (existing) { + const prev = JSON.parse(decrypt(existing.config)) as { + authToken?: string; + }; + nextAuthToken = prev.authToken || undefined; + } + } + const config = JSON.stringify({ serverUrl: serverUrl || "https://ntfy.sh", topic: topic || "", - ...(authToken ? { authToken } : {}), + ...(nextAuthToken ? { authToken: nextAuthToken } : {}), }); const encryptedConfig = encrypt(config); From 9ef3207cb65dfe7d10fa0486fa2d22bdcfff4bca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 23 Jul 2026 09:38:24 +0200 Subject: [PATCH 4/5] style: run prettier on the dashboard-layout-section test file --- .../settings/__tests__/dashboard-layout-section.test.tsx | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/components/settings/__tests__/dashboard-layout-section.test.tsx b/src/components/settings/__tests__/dashboard-layout-section.test.tsx index 33eaa1ea4..4594cd1f6 100644 --- a/src/components/settings/__tests__/dashboard-layout-section.test.tsx +++ b/src/components/settings/__tests__/dashboard-layout-section.test.tsx @@ -69,7 +69,10 @@ vi.mock("@/hooks/use-auth", () => ({ })); import { I18nProvider } from "@/lib/i18n/context"; -import { DashboardLayoutSection, reorderWidgets } from "../dashboard-layout-section"; +import { + DashboardLayoutSection, + reorderWidgets, +} from "../dashboard-layout-section"; function render(node: React.ReactElement, locale: "en" | "de" = "en") { return renderToStaticMarkup( From 32edb033e861f577bb5858a77e73c87f04934b44 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 23 Jul 2026 09:39:46 +0200 Subject: [PATCH 5/5] docs: note the three silent-data-loss fixes in the changelog --- CHANGELOG.md | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a15813119..a7b46af27 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,21 @@ ## [Unreleased] +- **Dashboard layout changes can no longer be reverted by a concurrent + score-ring save.** The instant score-ring toggle now sends only the ring + fields instead of resending a cached snapshot of the whole layout, closing + a race where an in-flight ring update could land after a tile/chart Save + and silently restore the older layout. +- **Measurement-reminder cadence edits recompute against the cadence that + was actually saved.** Clearing `intervalDays` or `rrule` to switch a + reminder's schedule no longer leaves the next-due date computed off the + just-replaced value for one cycle. +- **ntfy notification settings keep a saved auth token across unrelated + edits.** Toggling the channel, or editing the server URL or topic, no + longer silently clears a previously configured token. + +No migrations. No breaking changes. + ## [1.32.0] — 2026-07-22 - **Provider ingestion and daily reactions now preserve the correct identity.**