mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-10 05:12:14 +00:00
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.
This commit is contained in:
1 parent
1ba422b87c
commit
24f06884b3
9 files changed
+142
-29
No files matched your search
@@ -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
|
||||
|
||||
@@ -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}`;
|
||||
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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\)/,
|
||||
|
||||
@@ -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"
|
||||
);
|
||||
}
|
||||
@@ -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
|
||||
|
||||
@@ -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"])
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
});
|
||||
Reference in new issue
Block a user