From dbdf3b61cbe84537f45b28e18953dee41d6ef3d8 Mon Sep 17 00:00:00 2001 From: Daniel Volz Date: Wed, 25 Feb 2026 22:15:35 +0100 Subject: [PATCH] fix: harden reminder scheduler dedupe and boundary timing (#319) * fix: harden reminder scheduler dedupe and boundary timing * fix(ci): align medications route formatting with biome --- backend/src/routes/medications.ts | 8 +- backend/src/services/reminder-scheduler.ts | 295 ++++++++++++--------- backend/src/utils/scheduler-utils.ts | 6 +- 3 files changed, 179 insertions(+), 130 deletions(-) diff --git a/backend/src/routes/medications.ts b/backend/src/routes/medications.ts index 24e7f42..ab6f73b 100644 --- a/backend/src/routes/medications.ts +++ b/backend/src/routes/medications.ts @@ -66,10 +66,10 @@ const medicationSchema = z intakes: z.array(intakeSchema).min(1).max(12).optional(), blisters: z.array(blisterSchema).min(1).max(12).optional(), // Legacy format }) - .refine( - (data) => (data.name && data.name.length > 0) || (data.genericName && data.genericName.length > 0), - { message: "Either 'name' or 'genericName' must be provided", path: ["name"] } - ) + .refine((data) => (data.name && data.name.length > 0) || (data.genericName && data.genericName.length > 0), { + message: "Either 'name' or 'genericName' must be provided", + path: ["name"], + }) .refine((data) => data.intakes || data.blisters, { message: "Either 'intakes' or 'blisters' must be provided" }) .refine( (data) => { diff --git a/backend/src/services/reminder-scheduler.ts b/backend/src/services/reminder-scheduler.ts index 0a7dbb7..e5fad24 100644 --- a/backend/src/services/reminder-scheduler.ts +++ b/backend/src/services/reminder-scheduler.ts @@ -353,7 +353,7 @@ async function getMedicationsNeedingReminder( if (isCritical || isLow) { lowStock.push({ - name: row.name || row.genericName || "", + name: row.name, medsLeft: currentPills, daysLeft, depletionDate, @@ -379,7 +379,7 @@ async function getMedicationsNeedingPrescriptionReminder(userId: number): Promis (row.prescriptionRemainingRefills ?? 0) <= (row.prescriptionLowRefillThreshold ?? 1) ) .map((row) => ({ - name: row.name || row.genericName || "", + name: row.name, remainingRefills: row.prescriptionRemainingRefills ?? 0, lowThreshold: row.prescriptionLowRefillThreshold ?? 1, expiryDate: row.prescriptionExpiryDate ?? null, @@ -724,85 +724,113 @@ async function checkAndSendReminderForUser( ); } else { try { - logger.info( - `[Reminder] User ${settings.userId}: Sending prescription reminder for ${allPrescriptionLow.length} medications...` - ); + // Re-check using fresh state after acquiring lock and pre-mark today as notified. + // This blocks duplicate sends when two reminder checks overlap in time. + const lockedState = loadReminderState(); + const alreadyNotified = lockedState.notifiedMedications.includes(userPrescriptionNotifiedKey); + const shouldSend = !alreadyNotified || settings.repeatDailyReminders; + if (!shouldSend) { + logger.debug( + `[Reminder] User ${settings.userId}: prescription reminder already marked as sent today, skipping` + ); + } - const emptyRx = allPrescriptionLow.filter((m) => m.remainingRefills <= 0); - const lowRx = allPrescriptionLow.filter((m) => m.remainingRefills > 0); - const lines = allPrescriptionLow.map((m) => { - const expirySuffix = m.expiryDate ? t(tr.prescriptionReminder.expiresSuffix, { date: m.expiryDate }) : ""; - if (m.remainingRefills <= 0) { - return `- ${t(tr.prescriptionReminder.lineEmpty, { + const preMarkedNotified = + !shouldSend || alreadyNotified + ? lockedState.notifiedMedications + : [...new Set([...lockedState.notifiedMedications, userPrescriptionNotifiedKey])]; + if (shouldSend && !alreadyNotified) { + saveReminderState({ + lastAutoEmailSent: lockedState.lastAutoEmailSent, + lastAutoEmailDate: lockedState.lastAutoEmailDate, + lastStockSchedulerCheckDate: lockedState.lastStockSchedulerCheckDate, + notifiedMedications: preMarkedNotified, + nextScheduledCheck: lockedState.nextScheduledCheck, + lastNotificationType: lockedState.lastNotificationType, + lastNotificationChannel: lockedState.lastNotificationChannel, + }); + } + + if (shouldSend) { + logger.info( + `[Reminder] User ${settings.userId}: Sending prescription reminder for ${allPrescriptionLow.length} medications...` + ); + + const emptyRx = allPrescriptionLow.filter((m) => m.remainingRefills <= 0); + const lowRx = allPrescriptionLow.filter((m) => m.remainingRefills > 0); + const lines = allPrescriptionLow.map((m) => { + const expirySuffix = m.expiryDate ? t(tr.prescriptionReminder.expiresSuffix, { date: m.expiryDate }) : ""; + if (m.remainingRefills <= 0) { + return `- ${t(tr.prescriptionReminder.lineEmpty, { + name: m.name, + expirySuffix, + })}`; + } + return `- ${t(tr.prescriptionReminder.line, { name: m.name, + refills: m.remainingRefills, expirySuffix, })}`; - } - return `- ${t(tr.prescriptionReminder.line, { - name: m.name, - refills: m.remainingRefills, - expirySuffix, - })}`; - }); + }); - let emailSuccess = false; - let shoutrrrSuccess = false; + let emailSuccess = false; + let shoutrrrSuccess = false; - if (prescriptionEmailEnabled) { - const smtpHost = process.env.SMTP_HOST; - const smtpUser = process.env.SMTP_USER; - const smtpPass = process.env.SMTP_TOKEN || process.env.SMTP_PASS; - const smtpPort = parseInt(process.env.SMTP_PORT ?? "587", 10); - const smtpSecure = process.env.SMTP_SECURE === "true"; - const smtpFrom = process.env.SMTP_FROM ?? smtpUser; + if (prescriptionEmailEnabled) { + const smtpHost = process.env.SMTP_HOST; + const smtpUser = process.env.SMTP_USER; + const smtpPass = process.env.SMTP_TOKEN || process.env.SMTP_PASS; + const smtpPort = parseInt(process.env.SMTP_PORT ?? "587", 10); + const smtpSecure = process.env.SMTP_SECURE === "true"; + const smtpFrom = process.env.SMTP_FROM ?? smtpUser; - if (smtpHost && smtpUser) { - try { - const transporter = nodemailer.createTransport({ - host: smtpHost, - port: smtpPort, - secure: smtpSecure, - auth: { user: smtpUser, pass: smtpPass ?? "" }, - }); + if (smtpHost && smtpUser) { + try { + const transporter = nodemailer.createTransport({ + host: smtpHost, + port: smtpPort, + secure: smtpSecure, + auth: { user: smtpUser, pass: smtpPass ?? "" }, + }); - const subject = - allPrescriptionLow.length === 1 - ? tr.prescriptionReminder.subjectSingle - : t(tr.prescriptionReminder.subjectMultiple, { count: allPrescriptionLow.length }); + const subject = + allPrescriptionLow.length === 1 + ? tr.prescriptionReminder.subjectSingle + : t(tr.prescriptionReminder.subjectMultiple, { count: allPrescriptionLow.length }); - const bodyText = - emptyRx.length > 0 - ? tr.prescriptionReminder.descriptionEmpty - : tr.prescriptionReminder.descriptionLow; - const emptyAlert = - emptyRx.length === 1 - ? tr.prescriptionReminder.alertEmptySingle - : t(tr.prescriptionReminder.alertEmptyMultiple, { count: emptyRx.length }); - const lowAlert = - lowRx.length === 1 - ? tr.prescriptionReminder.alertLowSingle - : t(tr.prescriptionReminder.alertLowMultiple, { count: lowRx.length }); - const alertText = emptyRx.length > 0 ? emptyAlert : lowAlert; + const bodyText = + emptyRx.length > 0 + ? tr.prescriptionReminder.descriptionEmpty + : tr.prescriptionReminder.descriptionLow; + const emptyAlert = + emptyRx.length === 1 + ? tr.prescriptionReminder.alertEmptySingle + : t(tr.prescriptionReminder.alertEmptyMultiple, { count: emptyRx.length }); + const lowAlert = + lowRx.length === 1 + ? tr.prescriptionReminder.alertLowSingle + : t(tr.prescriptionReminder.alertLowMultiple, { count: lowRx.length }); + const alertText = emptyRx.length > 0 ? emptyAlert : lowAlert; - const tableRows = allPrescriptionLow - .map((item) => { - const isEmpty = item.remainingRefills <= 0; - const safeName = escapeHtml(item.name); - const safeRefills = Number(item.remainingRefills) || 0; - const safeThreshold = Number(item.lowThreshold) || 0; - const safeExpiry = item.expiryDate ? escapeHtml(String(item.expiryDate)) : "-"; - const rowBg = isEmpty ? "#fef2f2" : "white"; - return ` + const tableRows = allPrescriptionLow + .map((item) => { + const isEmpty = item.remainingRefills <= 0; + const safeName = escapeHtml(item.name); + const safeRefills = Number(item.remainingRefills) || 0; + const safeThreshold = Number(item.lowThreshold) || 0; + const safeExpiry = item.expiryDate ? escapeHtml(String(item.expiryDate)) : "-"; + const rowBg = isEmpty ? "#fef2f2" : "white"; + return ` ${isEmpty ? "🚨" : "⚠️"} ${safeName} ${safeRefills} ${safeThreshold} ${safeExpiry} `; - }) - .join(""); + }) + .join(""); - const html = ` + const html = `

${emptyRx.length > 0 ? tr.prescriptionReminder.titleEmpty : tr.prescriptionReminder.title}

@@ -842,76 +870,93 @@ async function checkAndSendReminderForUser(
`; - const text = `${emptyRx.length > 0 ? tr.prescriptionReminder.titleEmpty : tr.prescriptionReminder.title}\n\n${bodyText}\n\n${lines.join("\n")}\n\n---\n${getFooterPlain(language)}${settings.repeatDailyReminders ? `\n\n${tr.prescriptionReminder.repeatDailyNote}` : ""}`; + const text = `${emptyRx.length > 0 ? tr.prescriptionReminder.titleEmpty : tr.prescriptionReminder.title}\n\n${bodyText}\n\n${lines.join("\n")}\n\n---\n${getFooterPlain(language)}${settings.repeatDailyReminders ? `\n\n${tr.prescriptionReminder.repeatDailyNote}` : ""}`; - await transporter.sendMail({ - from: smtpFrom, - to: settings.notificationEmail!, - subject, - text, - html, - }); - emailSuccess = true; - } catch (error) { - const errorMessage = error instanceof Error ? error.message : "Unknown error"; - logger.error(`[Reminder] User ${settings.userId}: Failed to send prescription email: ${errorMessage}`); + await transporter.sendMail({ + from: smtpFrom, + to: settings.notificationEmail!, + subject, + text, + html, + }); + emailSuccess = true; + } catch (error) { + const errorMessage = error instanceof Error ? error.message : "Unknown error"; + logger.error( + `[Reminder] User ${settings.userId}: Failed to send prescription email: ${errorMessage}` + ); + } } } - } - if (prescriptionPushEnabled) { - const titleParts: string[] = []; - if (emptyRx.length > 0) - titleParts.push( - `🚨 ${emptyRx.length} ${emptyRx.length === 1 ? tr.prescriptionReminder.pushEmptySingle : tr.prescriptionReminder.pushEmpty}` - ); - if (lowRx.length > 0) - titleParts.push( - `🚨 ${lowRx.length} ${lowRx.length === 1 ? tr.prescriptionReminder.pushLowSingle : tr.prescriptionReminder.pushLow}` - ); - const title = `MedAssist-ng: ${titleParts.join(", ")} - ${tr.prescriptionReminder.pushRenewNow}`; - - const messageParts: string[] = []; - if (emptyRx.length > 0) { - messageParts.push(`🚨 ${tr.prescriptionReminder.pushEmptySection}:`); - for (const m of emptyRx) { - messageParts.push(` • ${m.name}`); - } - } - if (lowRx.length > 0) { - if (emptyRx.length > 0) messageParts.push(""); - messageParts.push(`🚨 ${tr.prescriptionReminder.pushLowSection}:`); - for (const m of lowRx) { - messageParts.push( - ` • ${m.name}: ${t(tr.prescriptionReminder.pushRefillsLeft, { count: m.remainingRefills })}` + if (prescriptionPushEnabled) { + const titleParts: string[] = []; + if (emptyRx.length > 0) + titleParts.push( + `🚨 ${emptyRx.length} ${emptyRx.length === 1 ? tr.prescriptionReminder.pushEmptySingle : tr.prescriptionReminder.pushEmpty}` ); + if (lowRx.length > 0) + titleParts.push( + `🚨 ${lowRx.length} ${lowRx.length === 1 ? tr.prescriptionReminder.pushLowSingle : tr.prescriptionReminder.pushLow}` + ); + const title = `MedAssist-ng: ${titleParts.join(", ")} - ${tr.prescriptionReminder.pushRenewNow}`; + + const messageParts: string[] = []; + if (emptyRx.length > 0) { + messageParts.push(`🚨 ${tr.prescriptionReminder.pushEmptySection}:`); + for (const m of emptyRx) { + messageParts.push(` • ${m.name}`); + } + } + if (lowRx.length > 0) { + if (emptyRx.length > 0) messageParts.push(""); + messageParts.push(`🚨 ${tr.prescriptionReminder.pushLowSection}:`); + for (const m of lowRx) { + messageParts.push( + ` • ${m.name}: ${t(tr.prescriptionReminder.pushRefillsLeft, { count: m.remainingRefills })}` + ); + } + } + const message = `${messageParts.join("\n")}\n\n---\n${getFooterPlain(language)}`; + const result = await sendShoutrrrNotification(settings.shoutrrrUrl!, title, message); + shoutrrrSuccess = result.success; + if (!result.success) { + logger.error(`[Reminder] User ${settings.userId}: Failed to send prescription push: ${result.error}`); } } - const message = `${messageParts.join("\n")}\n\n---\n${getFooterPlain(language)}`; - const result = await sendShoutrrrNotification(settings.shoutrrrUrl!, title, message); - shoutrrrSuccess = result.success; - if (!result.success) { - logger.error(`[Reminder] User ${settings.userId}: Failed to send prescription push: ${result.error}`); + + if (emailSuccess || shoutrrrSuccess) { + const currentState = loadReminderState(); + const singleChannel = emailSuccess ? "email" : "push"; + const channel = emailSuccess && shoutrrrSuccess ? "both" : singleChannel; + saveReminderState({ + lastAutoEmailSent: new Date().toISOString(), + lastAutoEmailDate: today, + lastStockSchedulerCheckDate: currentState.lastStockSchedulerCheckDate, + notifiedMedications: [...new Set([...currentState.notifiedMedications, userPrescriptionNotifiedKey])], + nextScheduledCheck: currentState.nextScheduledCheck, + lastNotificationType: "prescription", + lastNotificationChannel: channel, + }); + + const medNames = allPrescriptionLow.map((m) => m.name).join(", "); + await updateUserReminderSentTime(settings.userId, "prescription", channel, medNames); + } else if (!alreadyNotified) { + // Roll back pre-mark when both channels failed so retries remain possible. + const currentState = loadReminderState(); + saveReminderState({ + lastAutoEmailSent: currentState.lastAutoEmailSent, + lastAutoEmailDate: currentState.lastAutoEmailDate, + lastStockSchedulerCheckDate: currentState.lastStockSchedulerCheckDate, + notifiedMedications: currentState.notifiedMedications.filter( + (key) => key !== userPrescriptionNotifiedKey + ), + nextScheduledCheck: currentState.nextScheduledCheck, + lastNotificationType: currentState.lastNotificationType, + lastNotificationChannel: currentState.lastNotificationChannel, + }); } } - - if (emailSuccess || shoutrrrSuccess) { - const currentState = loadReminderState(); - const singleChannel = emailSuccess ? "email" : "push"; - const channel = emailSuccess && shoutrrrSuccess ? "both" : singleChannel; - saveReminderState({ - lastAutoEmailSent: new Date().toISOString(), - lastAutoEmailDate: today, - lastStockSchedulerCheckDate: currentState.lastStockSchedulerCheckDate, - notifiedMedications: [...new Set([...currentState.notifiedMedications, userPrescriptionNotifiedKey])], - nextScheduledCheck: currentState.nextScheduledCheck, - lastNotificationType: "prescription", - lastNotificationChannel: channel, - }); - - const medNames = allPrescriptionLow.map((m) => m.name).join(", "); - await updateUserReminderSentTime(settings.userId, "prescription", channel, medNames); - } } finally { releaseReminderSendLock(prescriptionSendLock); } diff --git a/backend/src/utils/scheduler-utils.ts b/backend/src/utils/scheduler-utils.ts index 47fe7ad..866e1c5 100644 --- a/backend/src/utils/scheduler-utils.ts +++ b/backend/src/utils/scheduler-utils.ts @@ -122,7 +122,11 @@ export function getNextScheduledTime(reminderHour: number, tz?: string): Date { /** Calculate milliseconds until next check at the given reminder hour */ export function getMsUntilNextCheck(reminderHour: number, tz?: string): number { const next = getNextScheduledTime(reminderHour, tz); - return next.getTime() - Date.now(); + const msUntilNext = next.getTime() - Date.now(); + if (msUntilNext <= 0) { + return msUntilNext + 24 * 60 * 60 * 1000; + } + return msUntilNext; } // =============================================================================