From 24f06884b3d02c7bb23983f400672b8af51346f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sun, 11 Oct 2026 15:16:54 +0200 Subject: [PATCH] fix(reminders): an ended course reads as not reminding The worker expands no slot past endsOn, so it never reminds for a course that has ended, but remindersActive still read true for it. The reminder gate now includes the course end on the person's clock, so the published value, the worker and the admin check make the same decision. --- docs/api/openapi.yaml | 17 +++--- .../notifications/reminder-check/route.ts | 6 +- src/app/api/medications/[id]/route.ts | 5 +- .../reminder/medication-reminder-check.ts | 4 +- .../__tests__/auto-mark-taken.test.ts | 49 +++++++++++++-- src/lib/medications/auto-mark-taken.ts | 23 +++++-- src/lib/medications/list-read.ts | 2 +- src/lib/openapi/routes/medications/schemas.ts | 4 +- .../medication-auto-mark-taken.test.ts | 61 ++++++++++++++++++- 9 files changed, 142 insertions(+), 29 deletions(-) diff --git a/docs/api/openapi.yaml b/docs/api/openapi.yaml index 4c7f73d4d..1754488a8 100644 --- a/docs/api/openapi.yaml +++ b/docs/api/openapi.yaml @@ -47714,10 +47714,10 @@ components: remindersActive: type: boolean description: "v1.42.2 — whether the server sends dose reminders for this medication: true only when - `notificationsEnabled` is on, the medication is `active`, its intake is tracked (`trackIntake`), it is not - `asNeeded`, and `autoMarkTaken` is off. The same decision gates every server-side reminder send. A client - that arms local reminders arms them from this, not from `notificationsEnabled`. Read-only — computed, not - stored." + `notificationsEnabled` is on, the medication is `active`, its course has not ended (`endsOn` is today or + later on the person's clock, or unset), its intake is tracked (`trackIntake`), it is not `asNeeded`, and + `autoMarkTaken` is off. The same decision gates every server-side reminder send. A client that arms local + reminders arms them from this, not from `notificationsEnabled`. Read-only — computed, not stored." nextDueAt: anyOf: - type: string @@ -83031,10 +83031,11 @@ components: - type: "null" remindersActive: description: "Present on the READ paths only (list and single GET). v1.42.2 — whether the server sends dose reminders - for this medication: true only when `notificationsEnabled` is on, the medication is `active`, its intake is - tracked (`trackIntake`), it is not `asNeeded`, and `autoMarkTaken` is off. The same decision gates every - server-side reminder send. A client that arms local reminders arms them from this, not from - `notificationsEnabled`. Read-only — computed, not stored." + for this medication: true only when `notificationsEnabled` is on, the medication is `active`, its course has + not ended (`endsOn` is today or later on the person's clock, or unset), its intake is tracked + (`trackIntake`), it is not `asNeeded`, and `autoMarkTaken` is off. The same decision gates every server-side + reminder send. A client that arms local reminders arms them from this, not from `notificationsEnabled`. + Read-only — computed, not stored." type: boolean nextDueAt: description: "Present on the READ paths only — the list and single GET compute it; the create / update responses return diff --git a/src/app/api/admin/notifications/reminder-check/route.ts b/src/app/api/admin/notifications/reminder-check/route.ts index efdd13547..b25c36d1d 100644 --- a/src/app/api/admin/notifications/reminder-check/route.ts +++ b/src/app/api/admin/notifications/reminder-check/route.ts @@ -185,7 +185,11 @@ export const POST = apiHandler(async (request: NextRequest) => { // v1.42.2 — a medication whose doses are recorded automatically // is never reminded, here or in the worker. - if (isOverdue && !hasEvent && sendsMedicationReminders(med)) { + if ( + isOverdue && + !hasEvent && + sendsMedicationReminders(med, { now, timeZone: userTz }) + ) { const doseInfo = schedule.dose ?? med.dose; const timeWindow = `${schedule.windowStart}–${schedule.windowEnd}`; diff --git a/src/app/api/medications/[id]/route.ts b/src/app/api/medications/[id]/route.ts index 13417b366..212716afe 100644 --- a/src/app/api/medications/[id]/route.ts +++ b/src/app/api/medications/[id]/route.ts @@ -280,7 +280,10 @@ export const GET = apiHandler( ...courseFields, nextDueAt: display ? display.at.toISOString() : null, // v1.42.2 — the server's own reminder decision, published resolved. - remindersActive: sendsMedicationReminders(medication), + remindersActive: sendsMedicationReminders(medication, { + now, + timeZone: userTz, + }), nextDueOverdue: display?.overdue ?? false, // v1.39.4 (#1040) — see `resolveIntakeActionability`. ...resolveIntakeActionability( diff --git a/src/lib/jobs/reminder/medication-reminder-check.ts b/src/lib/jobs/reminder/medication-reminder-check.ts index 5ae9bbba7..a23ea7a46 100644 --- a/src/lib/jobs/reminder/medication-reminder-check.ts +++ b/src/lib/jobs/reminder/medication-reminder-check.ts @@ -663,7 +663,7 @@ export async function handleReminderCheck( }), }); - if (sendsMedicationReminders(med)) { + if (sendsMedicationReminders(med, { now, timeZone: userTz })) { const claimed = await claimNotificationEvent(prisma, { recordUserId: med.user.id, eventType: "MEDICATION_REMINDER", @@ -727,7 +727,7 @@ export async function handleReminderCheck( // banner, every other channel the user configured still delivers // — so the gate lives in the dispatcher's APNs branch, which is // the only place that knows whether an APNs send was on the table. - if (sendsMedicationReminders(med)) { + if (sendsMedicationReminders(med, { now, timeZone: userTz })) { const recipientLocale = await resolveJobLocale(med.user.locale); const { title, message } = getPhaseMessage( currentPhase, diff --git a/src/lib/medications/__tests__/auto-mark-taken.test.ts b/src/lib/medications/__tests__/auto-mark-taken.test.ts index a77065d7f..0f04c842b 100644 --- a/src/lib/medications/__tests__/auto-mark-taken.test.ts +++ b/src/lib/medications/__tests__/auto-mark-taken.test.ts @@ -10,25 +10,62 @@ import { describe, expect, it } from "vitest"; import { sendsMedicationReminders } from "../auto-mark-taken"; describe("sendsMedicationReminders", () => { - it("reminds only for an active, tracked, scheduled medication with reminders on and no auto-recording", () => { - for (let bits = 0; bits < 32; bits++) { + const at = { + now: new Date("2026-06-10T10:00:00.000Z"), + timeZone: "Europe/Berlin", + }; + + it("reminds only for an active, tracked, scheduled medication with reminders on, no auto-recording and a course not ended", () => { + for (let bits = 0; bits < 64; bits++) { + const ended = (bits & 32) !== 0; const med = { notificationsEnabled: (bits & 1) !== 0, autoMarkTaken: (bits & 2) !== 0, active: (bits & 4) !== 0, trackIntake: (bits & 8) !== 0, asNeeded: (bits & 16) !== 0, + endsOn: ended ? new Date("2026-06-09T00:00:00.000Z") : null, }; - expect(sendsMedicationReminders(med), JSON.stringify(med)).toBe( + expect(sendsMedicationReminders(med, at), JSON.stringify(med)).toBe( med.notificationsEnabled && !med.autoMarkTaken && med.active && med.trackIntake && - !med.asNeeded, + !med.asNeeded && + !ended, ); } }); + it("a course ending today still reminds; one that ended yesterday does not", () => { + const base = { + notificationsEnabled: true, + autoMarkTaken: false, + active: true, + trackIntake: true, + asNeeded: false, + }; + expect( + sendsMedicationReminders( + { ...base, endsOn: new Date("2026-06-10T00:00:00.000Z") }, + at, + ), + ).toBe(true); + expect( + sendsMedicationReminders( + { ...base, endsOn: new Date("2026-06-09T00:00:00.000Z") }, + at, + ), + ).toBe(false); + // Late on the 10th in Berlin is already the 11th there. + expect( + sendsMedicationReminders( + { ...base, endsOn: new Date("2026-06-10T00:00:00.000Z") }, + { ...at, now: new Date("2026-06-10T22:30:00.000Z") }, + ), + ).toBe(false); + }); + it("is the decision at every gate and on every published DTO", () => { const src = (rel: string) => readFileSync(join(process.cwd(), "src", rel), "utf8"); @@ -36,7 +73,9 @@ describe("sendsMedicationReminders", () => { "lib/jobs/reminder/medication-reminder-check.ts", "app/api/admin/notifications/reminder-check/route.ts", ]) { - expect(src(gate), gate).toMatch(/sendsMedicationReminders\(med\)/); + expect(src(gate), gate).toMatch( + /sendsMedicationReminders\(med, \{ now, timeZone: userTz \}\)/, + ); // No send site falls back to the raw preference. expect(src(gate), gate).not.toMatch( /if \([^)]*med\.notificationsEnabled\)/, diff --git a/src/lib/medications/auto-mark-taken.ts b/src/lib/medications/auto-mark-taken.ts index e1dfe55e2..b8644d5a5 100644 --- a/src/lib/medications/auto-mark-taken.ts +++ b/src/lib/medications/auto-mark-taken.ts @@ -7,28 +7,39 @@ * decision instead of re-deriving it from the raw flags. * * A medication reminds only when the person wants reminders - * (`notificationsEnabled`), it is active (not paused or ended), it has doses - * that fall due (intake tracked, not as-needed), and its doses are not - * recorded automatically (`autoMarkTaken`). The person's own reminder - * preference stays stored untouched: switching auto-mark-taken off restores - * whatever they had chosen. + * (`notificationsEnabled`), it is active (not paused or ended), its course + * has not ended (`endsOn` lies before today on the person's clock: the + * worker expands no slot past it), it has doses that fall due (intake + * tracked, not as-needed), and its doses are not recorded automatically + * (`autoMarkTaken`). A course that has not started yet still reminds once + * it does. The person's own reminder preference stays stored untouched: + * switching auto-mark-taken off restores whatever they had chosen. */ +import { courseStatusAt } from "@/lib/medications/scheduling/recurrence"; + export interface ReminderGateMedication { notificationsEnabled: boolean; autoMarkTaken: boolean; active: boolean; trackIntake: boolean; asNeeded: boolean; + endsOn: Date | null; } export function sendsMedicationReminders( medication: ReminderGateMedication, + at: { now: Date; timeZone: string }, ): boolean { return ( medication.notificationsEnabled && !medication.autoMarkTaken && medication.active && medication.trackIntake && - !medication.asNeeded + !medication.asNeeded && + courseStatusAt( + { startsOn: null, endsOn: medication.endsOn }, + at.now, + at.timeZone, + ) !== "ENDED" ); } diff --git a/src/lib/medications/list-read.ts b/src/lib/medications/list-read.ts index 3c4fb67b7..32f6301e8 100644 --- a/src/lib/medications/list-read.ts +++ b/src/lib/medications/list-read.ts @@ -338,7 +338,7 @@ export async function buildMedicationsList( (carriesPriorRollingOccurrence ? 1 : 0), nextDueAt: display ? display.at.toISOString() : null, // v1.42.2 — the server's own reminder decision, published resolved. - remindersActive: sendsMedicationReminders(m), + remindersActive: sendsMedicationReminders(m, { now, timeZone: userTz }), nextDueOverdue: display?.overdue ?? false, nextDueScheduleId: display?.scheduleId ?? null, // v1.39.4 (#1040) — whether the card offers taken / skip today, and diff --git a/src/lib/openapi/routes/medications/schemas.ts b/src/lib/openapi/routes/medications/schemas.ts index 50b5defa7..99e8a6d36 100644 --- a/src/lib/openapi/routes/medications/schemas.ts +++ b/src/lib/openapi/routes/medications/schemas.ts @@ -282,7 +282,7 @@ export const medicationResource = z .boolean() .optional() .describe( - "Present on the READ paths only (list and single GET). v1.42.2 — whether the server sends dose reminders for this medication: true only when `notificationsEnabled` is on, the medication is `active`, its intake is tracked (`trackIntake`), it is not `asNeeded`, and `autoMarkTaken` is off. The same decision gates every server-side reminder send. A client that arms local reminders arms them from this, not from `notificationsEnabled`. Read-only — computed, not stored.", + "Present on the READ paths only (list and single GET). v1.42.2 — whether the server sends dose reminders for this medication: true only when `notificationsEnabled` is on, the medication is `active`, its course has not ended (`endsOn` is today or later on the person's clock, or unset), its intake is tracked (`trackIntake`), it is not `asNeeded`, and `autoMarkTaken` is off. The same decision gates every server-side reminder send. A client that arms local reminders arms them from this, not from `notificationsEnabled`. Read-only — computed, not stored.", ), nextDueAt: z.iso .datetime({ offset: true }) @@ -375,7 +375,7 @@ export const medicationListEntry = medicationResource remindersActive: z .boolean() .describe( - "v1.42.2 — whether the server sends dose reminders for this medication: true only when `notificationsEnabled` is on, the medication is `active`, its intake is tracked (`trackIntake`), it is not `asNeeded`, and `autoMarkTaken` is off. The same decision gates every server-side reminder send. A client that arms local reminders arms them from this, not from `notificationsEnabled`. Read-only — computed, not stored.", + "v1.42.2 — whether the server sends dose reminders for this medication: true only when `notificationsEnabled` is on, the medication is `active`, its course has not ended (`endsOn` is today or later on the person's clock, or unset), its intake is tracked (`trackIntake`), it is not `asNeeded`, and `autoMarkTaken` is off. The same decision gates every server-side reminder send. A client that arms local reminders arms them from this, not from `notificationsEnabled`. Read-only — computed, not stored.", ), courseStatus: z .enum(["UPCOMING", "CURRENT", "ENDED"]) diff --git a/tests/integration/medication-auto-mark-taken.test.ts b/tests/integration/medication-auto-mark-taken.test.ts index cb79cb58f..b30f2e907 100644 --- a/tests/integration/medication-auto-mark-taken.test.ts +++ b/tests/integration/medication-auto-mark-taken.test.ts @@ -584,14 +584,17 @@ describe("remindersActive — the published value is the gate", () => { active: boolean; trackIntake: boolean; asNeeded: boolean; + endsOn: Date | null; }> = []; - for (let bits = 0; bits < 32; bits++) { + for (let bits = 0; bits < 64; bits++) { combos.push({ notificationsEnabled: (bits & 1) !== 0, autoMarkTaken: (bits & 2) !== 0, active: (bits & 4) !== 0, trackIntake: (bits & 8) !== 0, asNeeded: (bits & 16) !== 0, + // A course that ended the day before. + endsOn: (bits & 32) !== 0 ? new Date("2026-06-09T00:00:00.000Z") : null, }); } const ids: string[] = []; @@ -636,7 +639,10 @@ describe("remindersActive — the published value is the gate", () => { const detail = await import("@/app/api/medications/[id]/route"); let trueCount = 0; for (const [i, id] of ids.entries()) { - const expected = sendsMedicationReminders(combos[i]); + const expected = sendsMedicationReminders(combos[i], { + now: new Date(), + timeZone: TZ, + }); if (expected) trueCount++; expect(listed.get(id), `list combo ${i}`).toBe(expected); const res = await (detail.GET as Handler)( @@ -646,7 +652,56 @@ describe("remindersActive — the published value is the gate", () => { const body = (await res.json()) as { data: { remindersActive: boolean } }; expect(body.data.remindersActive, `detail combo ${i}`).toBe(expected); } - // Exactly one combination reminds: on, not auto, active, tracked, scheduled. + // Exactly one combination reminds: on, not auto, active, tracked, + // scheduled, course not ended. expect(trueCount).toBe(1); }); + + it("a course that has ended reads as not reminding, and the worker sends nothing for it", async () => { + const prisma = getPrismaClient(); + const userId = await makeUser(); + const ended = await seedEvening(userId, "Antibiotic", { + autoMarkTakenSince: null, + }); + const running = await seedEvening(userId, "Iron", { + autoMarkTakenSince: null, + }); + await prisma.medication.update({ + where: { id: ended }, + data: { endsOn: new Date("2026-06-09T00:00:00.000Z") }, + }); + for (const tick of [ + "2026-06-10T17:30:00.000Z", + "2026-06-10T19:00:00.000Z", + "2026-06-10T22:00:00.000Z", + ]) { + at(tick); + await handleReminderCheck([]); + } + const dispatchedFor = (medicationId: string) => + vi + .mocked(dispatchNotification) + .mock.calls.filter( + ([arg]) => + (arg as { metadata?: { medicationId?: string } }).metadata + ?.medicationId === medicationId, + ).length; + expect(dispatchedFor(running)).toBeGreaterThan(0); + expect(dispatchedFor(ended)).toBe(0); + + const list = await import("@/app/api/medications/route"); + const res = await (list.GET as Handler)( + new NextRequest("http://localhost/api/medications"), + { params: Promise.resolve({}) }, + ); + const published = new Map( + ( + (await res.json()) as { + data: Array<{ id: string; remindersActive: boolean }>; + } + ).data.map((m) => [m.id, m.remindersActive] as const), + ); + expect(published.get(running)).toBe(true); + expect(published.get(ended)).toBe(false); + }); });