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