mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-10 05:12:14 +00:00
fix(reminders): send a multi-hour protocol at every hour it names
A reminder with FREQ=DAILY;BYHOUR=7,19 (the morning-and-evening blood pressure course) was only ever sent in the morning. The tick opened its send window at notifyHour alone, so the 19:00 slot stayed due until 07:00 the next day, and the claim was keyed per local day, so a second send that day would have been refused anyway. The window is now the hour of the current slot: the local hour of nextDueAt when it is one of the rule's BYHOUR values, else notifyHour. The early skip for a reminder already sent for its slot uses the same hour, so it stays consistent with the hold-open rule and lastNotifiedAt. A multi-hour reminder claims per slot hour; a single-hour reminder keeps the per-day key.
This commit is contained in:
1 parent
8a03654e8f
commit
888b738546
2 files changed
+176
-7
No files matched your search
@@ -0,0 +1,119 @@
|
||||
/**
|
||||
* A multi-hour protocol (`FREQ=DAILY;BYHOUR=7,19`, the morning-and-evening
|
||||
* blood pressure course) must be sent at both hours. The tick used to gate
|
||||
* on `notifyHour` alone and claim one send per local day, so the evening
|
||||
* slot sat due until the next morning and was never sent in the evening.
|
||||
*
|
||||
* Drives the real tick every 15 minutes across two local days in several
|
||||
* zones, with a claim ledger that behaves like the real one (a key claims
|
||||
* once), and a Prisma double whose updates land on the row the next tick
|
||||
* reads.
|
||||
*/
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
|
||||
const claimed = vi.hoisted(() => new Set<string>());
|
||||
vi.mock("@/lib/notifications/reminder-dedup", async () => {
|
||||
const actual = await vi.importActual<
|
||||
typeof import("@/lib/notifications/reminder-dedup")
|
||||
>("@/lib/notifications/reminder-dedup");
|
||||
return {
|
||||
...actual,
|
||||
claimNotificationEvent: vi.fn(
|
||||
async (_prisma: unknown, args: { dedupKey: string }) => {
|
||||
if (claimed.has(args.dedupKey)) return false;
|
||||
claimed.add(args.dedupKey);
|
||||
return true;
|
||||
},
|
||||
),
|
||||
};
|
||||
});
|
||||
|
||||
import { wallClockInTz, zonedWallClockToUtc } from "@/lib/tz/wall-clock";
|
||||
import { computeReminderNextDueAt } from "@/lib/measurement-reminders/scheduling";
|
||||
|
||||
import { runMeasurementReminderTick } from "../measurement-reminder";
|
||||
|
||||
const ZONES = [
|
||||
"Europe/Berlin",
|
||||
"America/New_York",
|
||||
"America/Los_Angeles",
|
||||
"Asia/Tokyo",
|
||||
"Pacific/Auckland",
|
||||
] as const;
|
||||
|
||||
beforeEach(() => {
|
||||
claimed.clear();
|
||||
});
|
||||
|
||||
function at(tz: string, day: number, hour: number, minute = 0): Date {
|
||||
return zonedWallClockToUtc({ year: 2026, month: 9, day, hour, minute }, tz);
|
||||
}
|
||||
|
||||
describe.each(ZONES)("BYHOUR=7,19 in %s", (tz) => {
|
||||
it("sends at 07:00 and at 19:00 on each day", async () => {
|
||||
const base = {
|
||||
intervalDays: null,
|
||||
rrule: "FREQ=DAILY;BYHOUR=7,19",
|
||||
anchorDate: at(tz, 14, 0),
|
||||
notifyHour: 7,
|
||||
lastSatisfiedAt: null,
|
||||
createdAt: at(tz, 13, 12),
|
||||
};
|
||||
const row: Record<string, unknown> = {
|
||||
id: "r-bp",
|
||||
userId: "u1",
|
||||
origin: "COACH",
|
||||
measurementType: null,
|
||||
label: "BP",
|
||||
location: null,
|
||||
lastSkippedAt: null,
|
||||
lastNotifiedAt: null,
|
||||
enabled: true,
|
||||
endsOn: null,
|
||||
...base,
|
||||
nextDueAt: computeReminderNextDueAt(base, tz, at(tz, 14, 0)),
|
||||
user: { id: "u1", timezone: tz, locale: "en" },
|
||||
};
|
||||
const prisma = {
|
||||
measurementReminder: {
|
||||
findMany: vi.fn(async () =>
|
||||
(row.nextDueAt as Date | null) === null ? [] : [row],
|
||||
),
|
||||
update: vi.fn(async ({ data }: { data: Record<string, unknown> }) => {
|
||||
Object.assign(row, data);
|
||||
return row;
|
||||
}),
|
||||
updateMany: vi.fn(async () => ({ count: 0 })),
|
||||
},
|
||||
measurement: { findFirst: vi.fn(async () => null) },
|
||||
labResult: { findFirst: vi.fn(async () => null) },
|
||||
};
|
||||
const sent: string[] = [];
|
||||
const dispatch = vi.fn(async () => ({
|
||||
dispatched: true,
|
||||
channelsAttempted: 1,
|
||||
channelsSucceeded: 1,
|
||||
}));
|
||||
|
||||
for (
|
||||
let t = at(tz, 14, 0).getTime();
|
||||
t < at(tz, 16, 0).getTime();
|
||||
t += 15 * 60_000
|
||||
) {
|
||||
const now = new Date(t);
|
||||
const before = dispatch.mock.calls.length;
|
||||
await runMeasurementReminderTick(prisma as never, now, {
|
||||
dispatch: dispatch as never,
|
||||
isModuleEnabled: async () => true,
|
||||
});
|
||||
if (dispatch.mock.calls.length > before) {
|
||||
const p = wallClockInTz(now, tz);
|
||||
sent.push(
|
||||
`${p.day} ${String(p.hour).padStart(2, "0")}:${String(p.minute).padStart(2, "0")}`,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
expect(sent).toEqual(["14 07:00", "14 19:00", "15 07:00", "15 19:00"]);
|
||||
});
|
||||
});
|
||||
@@ -50,6 +50,7 @@ import { getEvent } from "@/lib/logging/context";
|
||||
import { isModuleEnabled } from "@/lib/modules/gate";
|
||||
import type { ModuleKey } from "@/lib/modules/registry";
|
||||
import {
|
||||
byHourTimesOfDay,
|
||||
computeReminderNextDueAt,
|
||||
type ReminderScheduleInput,
|
||||
} from "@/lib/measurement-reminders/scheduling";
|
||||
@@ -169,12 +170,51 @@ async function cleanupExpiredCoachReminders(
|
||||
}
|
||||
|
||||
/**
|
||||
* Pure due-predicate: at this instant, is the reminder due AND inside its
|
||||
* local notify-hour window?
|
||||
* The local hours a reminder fires at: every `BYHOUR` of its rule (the
|
||||
* twice-daily BP protocol `FREQ=DAILY;BYHOUR=7,19`), else its `notifyHour`.
|
||||
*/
|
||||
function reminderFireHours(reminder: {
|
||||
notifyHour: number;
|
||||
rrule?: string | null;
|
||||
}): number[] {
|
||||
const byHour = byHourTimesOfDay(reminder.rrule ?? null);
|
||||
return byHour
|
||||
? byHour.map((hhmm) => Number(hhmm.slice(0, 2)))
|
||||
: [reminder.notifyHour];
|
||||
}
|
||||
|
||||
/**
|
||||
* The local hour the reminder's current slot belongs to: the hour of
|
||||
* `nextDueAt` on the user's clock when that is one of its fire hours, else
|
||||
* `notifyHour` (a slot stamped before the hour was edited, or a test
|
||||
* fixture with no real slot). For a single-hour reminder this is always
|
||||
* `notifyHour`; for `BYHOUR=7,19` the evening slot reads 19, so it is sent
|
||||
* in the evening instead of waiting for the next morning.
|
||||
*/
|
||||
export function reminderSlotHour(
|
||||
reminder: {
|
||||
nextDueAt: Date | null;
|
||||
notifyHour: number;
|
||||
rrule?: string | null;
|
||||
},
|
||||
timezone: string,
|
||||
): number {
|
||||
const hours = reminderFireHours(reminder);
|
||||
if (reminder.nextDueAt !== null && hours.length > 1) {
|
||||
const slotHour = wallClockInTz(reminder.nextDueAt, timezone).hour;
|
||||
if (hours.includes(slotHour)) return slotHour;
|
||||
}
|
||||
return reminder.notifyHour;
|
||||
}
|
||||
|
||||
/**
|
||||
* Pure due-predicate: at this instant, is the reminder due AND inside the
|
||||
* local hour of its current slot?
|
||||
*
|
||||
* "Due" = `nextDueAt != null` and `now >= nextDueAt`. The hour gate keeps
|
||||
* a reminder that became overdue overnight from firing at 03:00 — it
|
||||
* waits for the user's chosen `notifyHour` to come round in their local
|
||||
* waits for its slot's hour (`reminderSlotHour`: the `notifyHour`, or the
|
||||
* matching `BYHOUR` of a multi-hour rule) to come round in the user's
|
||||
* timezone. Pulled out so the unit tests can pin the window boundary
|
||||
* (08:59 → no, 09:00 → yes, 09:59 → yes, 10:00 → no) without the DB.
|
||||
*/
|
||||
@@ -183,6 +223,7 @@ export function evaluateMeasurementReminderDue(
|
||||
enabled: boolean;
|
||||
nextDueAt: Date | null;
|
||||
notifyHour: number;
|
||||
rrule?: string | null;
|
||||
},
|
||||
timezone: string,
|
||||
now: Date,
|
||||
@@ -190,9 +231,10 @@ export function evaluateMeasurementReminderDue(
|
||||
if (!reminder.enabled || reminder.nextDueAt === null) {
|
||||
return { fire: false, inHourWindow: false, isDue: false };
|
||||
}
|
||||
const tz = timezone || "Europe/Berlin";
|
||||
const isDue = now.getTime() >= reminder.nextDueAt.getTime();
|
||||
const parts = wallClockInTz(now, timezone || "Europe/Berlin");
|
||||
const inHourWindow = parts.hour === reminder.notifyHour;
|
||||
const parts = wallClockInTz(now, tz);
|
||||
const inHourWindow = parts.hour === reminderSlotHour(reminder, tz);
|
||||
return { fire: isDue && inHourWindow, inHourWindow, isDue };
|
||||
}
|
||||
|
||||
@@ -298,7 +340,8 @@ export async function runMeasurementReminderTick(
|
||||
reminder.lastNotifiedAt != null &&
|
||||
reminder.nextDueAt !== null &&
|
||||
reminder.lastNotifiedAt.getTime() >= reminder.nextDueAt.getTime() &&
|
||||
wallClockInTz(now, timezone).hour !== reminder.notifyHour
|
||||
wallClockInTz(now, timezone).hour !==
|
||||
reminderSlotHour(reminder, timezone)
|
||||
) {
|
||||
summary.skippedOutsideWindow += 1;
|
||||
continue;
|
||||
@@ -338,6 +381,7 @@ export async function runMeasurementReminderTick(
|
||||
enabled: reminder.enabled,
|
||||
nextDueAt: reminder.nextDueAt,
|
||||
notifyHour: reminder.notifyHour,
|
||||
rrule: reminder.rrule,
|
||||
},
|
||||
timezone,
|
||||
now,
|
||||
@@ -409,10 +453,16 @@ export async function runMeasurementReminderTick(
|
||||
const localDate = new Date(now).toLocaleDateString("sv-SE", {
|
||||
timeZone: timezone,
|
||||
});
|
||||
// A multi-hour rule sends more than once a day, so its claim is per
|
||||
// slot hour; a single-hour reminder keeps the per-day key.
|
||||
const slotSuffix =
|
||||
reminderFireHours(reminder).length > 1
|
||||
? `T${String(reminderSlotHour(reminder, timezone)).padStart(2, "0")}`
|
||||
: "";
|
||||
const claimed = await claimNotificationEvent(prisma, {
|
||||
recordUserId: reminder.user.id,
|
||||
eventType: "MEASUREMENT_REMINDER",
|
||||
dedupKey: `measurement:${reminder.id}:${localDate}`,
|
||||
dedupKey: `measurement:${reminder.id}:${localDate}${slotSuffix}`,
|
||||
since: new Date(now.getTime() - REMINDER_DEDUP_LOOKBACK_MS),
|
||||
});
|
||||
if (!claimed) {
|
||||
|
||||
Reference in new issue
Block a user