diff --git a/HISTORY.md b/HISTORY.md index e4ac013..a1046c7 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -3,6 +3,7 @@ ### πŸ› Tracker & bill-modal hardening +- **[Notifications] "Reminder days before" was a dead setting β€” the notifier ignored it** β€” every bill has a `reminder_days_before` column (default 3) and the bill modal exposed a "Reminder Days" control for subscriptions, but `services/notificationService.js` used a hard-coded schedule (early reminder always at exactly 3 days out) and never read the column. A user who set "remind me 7 days before" still only got the fixed 3-day/1-day/today reminders. The early reminder now fires at the bill's own `reminder_days_before` lead (only when β‰₯ 2 days, so it never collides with the 1-day/same-day reminders), and the email subject + body say "due in N days" using that value. The lead-time selection was pulled into a pure, exported `reminderTypeFor(bill, diffDays)` so it's unit-tested directly (`tests/notificationLeadTime.test.js`) β€” default 3 stays backwards-compatible. The **"Reminder Days Before" control now shows for every bill** (not just subscriptions), and saving a non-subscription bill no longer clobbers the column back to 3. (Tracker BM3) - **[Bill modal/SimpleFIN] Importing bank payments didn't refresh the payment list or the Tracker** β€” the two flows in the bill modal that *create* payments β€” **Sync** (`syncBillSimplefinPayments`) and a **merchant-rule historical import** (`onRulesChanged` β†’ `importHistoricalPayments`) β€” only reloaded the linked-transactions list, unlike the unmatch handlers which correctly reload payments *and* linked transactions *and* call `onSave`. So after importing, say, 3 payments from bank history, the modal's Payment History stayed stale and the Tracker row behind it kept showing "due/overdue" even though the bill was now covered β€” until you closed and reopened. Both paths now `await Promise.all([loadPayments(), loadLinkedTransactions()])` then `onSave?.()`, matching the unmatch pattern, so imported payments appear immediately and the Tracker updates live. (The SimpleFIN *search/preview/candidate* flow was already correct.) (Tracker BM4) - **[Tracker/SimpleFIN] Bank card's "unpaid this month" and "remaining" over-counted off-month bills** β€” `buildBankTracking` (`services/trackerService.js`) summed `expected_amount` for *all* active unpaid bills via SQL with no occurrence gate, so an annual or off-month quarterly bill inflated `unpaid_this_month` (and therefore the bank `remaining`) even though the Tracker rows beside it correctly excluded it β€” the same class of bug as QA-B5-02, still live on the bank path. `getTracker` now derives the unpaid total from the already-gated rows (via `resolveDueDate`), netting partial payments, and passes it into `buildBankTracking`. Also made `summary.remaining` / `total_remaining` use the bank card's own remaining when bank tracking is on (they previously used manual starting-amount math even in bank mode, disagreeing with safe-to-spend), and switched a stray `balance / 100` to `fromCents`. New test file `tests/trackerService.test.js` covers the gating fix, summary totals, the bank-mode remaining agreement, cents↔dollars integrity, and `getOverdueCount` gating β€” the dense Tracker aggregation had no dedicated tests before. (Tracker T1) - **[Payments] Quick-pay could create duplicate payments and double-drop the balance** β€” `POST /api/payments/quick` (the one-click "pay" behind every Tracker row) had **no duplicate guard** and its INSERT + balance update weren't atomic, unlike `POST /api/payments/bulk`. A double-click, a retry, or two open tabs made a *second* payment for the same bill/date/amount and applied the balance drop twice; a failure between the INSERT and the balance write left a payment with no balance adjustment. Quick-pay now checks the same `bill_id + paid_date + amount` composite key (returning the existing payment idempotently, HTTP 200) and wraps the INSERT + `applyBalanceDelta` in a single `db.transaction`. A different amount on the same day is still a legitimate new payment. Test: `tests/paymentsQuickRoute.test.js`. (Tracker X1) diff --git a/client/components/BillModal.jsx b/client/components/BillModal.jsx index 3cda120..d64864b 100644 --- a/client/components/BillModal.jsx +++ b/client/components/BillModal.jsx @@ -559,7 +559,7 @@ export default function BillModal({ bill, initialBill, categories, onClose, onSa auto_mark_paid: canAutoMarkPaid && autoMarkPaid, is_subscription: isSubscription, subscription_type: isSubscription ? subscriptionType : null, - reminder_days_before: isSubscription ? parseInt(reminderDaysBefore || '3', 10) : 3, + reminder_days_before: parseInt(reminderDaysBefore || '3', 10), subscription_source: sourceBill?.subscription_source || 'manual', subscription_detected_at: sourceBill?.subscription_detected_at, has_2fa: has2fa, @@ -788,8 +788,8 @@ export default function BillModal({ bill, initialBill, categories, onClose, onSa {isSubscription && ( -
-
+
+
-
- - setReminderDaysBefore(e.target.value)} - /> -

0-30 days before renewal.

-
{/* Bank sync button moved to the Transactions tab β†’ Bank Matching Rules section */}
)}
+ {/* Reminder lead time β€” applies to every bill (the notifier honors + reminder_days_before for the early "due soon" reminder). */} +
+
+ + setReminderDaysBefore(e.target.value)} + /> +

+ Get an early reminder this many days before the due date (0-30). Also needs reminders enabled in Settings. +

+
+
+ {/* Debt / Snowball Details β€” collapsible */}
diff --git a/services/notificationService.js b/services/notificationService.js index ad459e1..341a050 100644 --- a/services/notificationService.js +++ b/services/notificationService.js @@ -141,8 +141,32 @@ function senderAddress() { // ── Email templates ─────────────────────────────────────────────────────────── +// Per-bill reminder lead time (days before due) for the early "due soon" +// reminder. Every bill has a reminder_days_before column (default 3); honor it +// so a user who asks for "7 days before" actually gets a 7-day reminder. +function leadDaysOf(bill) { + return Number.isInteger(bill?.reminder_days_before) ? bill.reminder_days_before : 3; +} + +// Which reminder type (if any) applies to a bill that is `diffDays` calendar +// days from its due date. Pure + exported so the lead-time logic is unit-testable +// without invoking the full runner (which uses the real clock + real senders). +// The early reminder fires at the bill's own lead time, and only when that lead +// is β‰₯ 2 so it never collides with the 1-day or same-day reminders. +function reminderTypeFor(bill, diffDays) { + const leadDays = leadDaysOf(bill); + if (diffDays >= 2 && diffDays === leadDays) return 'due_3d'; + if (diffDays === 1) return 'due_1d'; + if (diffDays === 0) return 'due_today'; + if (diffDays < 0) return 'overdue'; + return null; +} + const TYPE_META = { - due_3d: { subject: (b) => `Reminder: ${b.name} due in 3 days`, urgency: 'upcoming' }, + // The `due_3d` key is the early reminder; its lead time is the bill's own + // reminder_days_before (only fires when that is β‰₯ 2 β€” a 1-day lead is covered + // by due_1d and a 0-day lead by due_today). + due_3d: { subject: (b) => `Reminder: ${b.name} due in ${leadDaysOf(b)} days`, urgency: 'upcoming' }, due_1d: { subject: (b) => `Reminder: ${b.name} due tomorrow`, urgency: 'soon' }, due_today: { subject: (b) => `Due today: ${b.name}`, urgency: 'today' }, overdue: { subject: (b) => `Overdue: ${b.name} hasn't been paid`, urgency: 'overdue' }, @@ -169,7 +193,7 @@ function buildEmailHtml(bill, type, dueDate) { // these message strings (previously raw β€” an XSS vector via the bill name). const name = esc(bill.name); const messages = { - due_3d: `${name} is due in 3 days.`, + due_3d: `${name} is due in ${leadDaysOf(bill)} days.`, due_1d: `${name} is due tomorrow.`, due_today: `${name} is due today.`, overdue: `${name} was due on ${fmt(dueDate)} and has not been marked as paid.`, @@ -359,12 +383,9 @@ async function runNotifications() { const dueDay = new Date(due.getFullYear(), due.getMonth(), due.getDate()); const diffDays = Math.round((dueDay - todayDate) / 86400000); - // Determine which type applies today - let type = null; - if (diffDays === 3) type = 'due_3d'; - else if (diffDays === 1) type = 'due_1d'; - else if (diffDays === 0) type = 'due_today'; - else if (diffDays < 0) type = 'overdue'; + // Determine which type applies today. The early reminder fires at the bill's + // own lead time (reminder_days_before, default 3) rather than a fixed 3 days. + const type = reminderTypeFor(bill, diffDays); if (!type) continue; @@ -555,3 +576,6 @@ module.exports = { runNotifications, runDriftNotifications, sendTestEmail, creat // before it, leaving `_push` undefined β†’ "Send test push" always 500'd). module.exports._push = { sendNtfy, sendGotify, sendDiscord, sendTelegram, sendTestPush, sendPushToUser, encryptSecret }; module.exports._email = { buildEmailHtml, esc }; +// Reminder lead-time logic, exposed for unit tests (the full runner uses the +// real clock + real senders, so the pure type selection is tested in isolation). +module.exports._reminders = { reminderTypeFor, leadDaysOf, TYPE_META }; diff --git a/tests/notificationLeadTime.test.js b/tests/notificationLeadTime.test.js new file mode 100644 index 0000000..ce62f8e --- /dev/null +++ b/tests/notificationLeadTime.test.js @@ -0,0 +1,48 @@ +'use strict'; + +// BM3 β€” the reminder notifier used a hard-coded 3-day early reminder and never +// read the bill's reminder_days_before, so the "Reminder days before" control +// was a no-op. These tests pin the now-honored per-bill lead time (pure logic, +// no clock/senders) and the dynamic email wording. +const test = require('node:test'); +const assert = require('node:assert/strict'); + +const { _reminders, _email } = require('../services/notificationService'); +const { reminderTypeFor, leadDaysOf, TYPE_META } = _reminders; + +test('leadDaysOf defaults to 3 and honors an integer reminder_days_before', () => { + assert.equal(leadDaysOf({}), 3); + assert.equal(leadDaysOf({ reminder_days_before: null }), 3); + assert.equal(leadDaysOf({ reminder_days_before: 7 }), 7); + assert.equal(leadDaysOf({ reminder_days_before: 0 }), 0); +}); + +test('a 7-day lead fires the early reminder at 7 days out, NOT at 3', () => { + const bill = { reminder_days_before: 7 }; + assert.equal(reminderTypeFor(bill, 7), 'due_3d', 'early reminder at the chosen lead'); + assert.equal(reminderTypeFor(bill, 3), null, 'no reminder at the old fixed 3 days'); +}); + +test('default (no reminder_days_before) still fires at 3 days β€” backwards compatible', () => { + assert.equal(reminderTypeFor({}, 3), 'due_3d'); + assert.equal(reminderTypeFor({}, 5), null); +}); + +test('1-day and same-day reminders are unaffected and never double up with a small lead', () => { + // lead of 1 β†’ the early reminder is suppressed (due_1d covers it) + assert.equal(reminderTypeFor({ reminder_days_before: 1 }, 1), 'due_1d'); + // lead of 0 β†’ due_today covers it + assert.equal(reminderTypeFor({ reminder_days_before: 0 }, 0), 'due_today'); + // generic day/overdue selection + assert.equal(reminderTypeFor({ reminder_days_before: 5 }, 1), 'due_1d'); + assert.equal(reminderTypeFor({ reminder_days_before: 5 }, 0), 'due_today'); + assert.equal(reminderTypeFor({ reminder_days_before: 5 }, -2), 'overdue'); + assert.equal(reminderTypeFor({ reminder_days_before: 5 }, 4), null); +}); + +test('email subject + body reflect the actual lead days', () => { + const bill = { name: 'Insurance', expected_amount: 12000, reminder_days_before: 7 }; + assert.match(TYPE_META.due_3d.subject(bill), /due in 7 days/); + const html = _email.buildEmailHtml(bill, 'due_3d', '2026-07-10'); + assert.match(html, /is due in 7 days/); +});