From 73631ab8127cf28cc37114567f02d74293e82b45 Mon Sep 17 00:00:00 2001 From: null Date: Fri, 3 Jul 2026 18:21:28 -0500 Subject: [PATCH] fix(tracker): route error handling + autopay write atomicity (T2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit routes/tracker.js had no try/catch and returned a plain {error} shape; the three GET handlers now wrap in try/catch + standardizeError (including the invalid-month path). applyAutopaySuggestions ran INSERT + applyBalanceDelta as two un-transactional steps on a GET — wrapped both in one db.transaction. Tests: autopay creates one payment + drops balance (idempotent), route returns standardized error. Co-Authored-By: Claude Opus 4.8 --- HISTORY.md | 1 + routes/tracker.js | 28 +++++++++++++++++++---- services/trackerService.js | 41 ++++++++++++++++++--------------- tests/trackerService.test.js | 44 ++++++++++++++++++++++++++++++++++++ 4 files changed, 91 insertions(+), 23 deletions(-) diff --git a/HISTORY.md b/HISTORY.md index 148eb5d..2ac6395 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -3,6 +3,7 @@ ### 🐛 Tracker & bill-modal hardening +- **[Tracker] Route error handling + autopay write atomicity** — `routes/tracker.js` had no try/catch and returned a plain `{error}` shape unlike the rest of the API; its three GET handlers now wrap in try/catch and return `standardizeError` (`{error, message, field, code}`), including the invalid-month validation path. Separately, `applyAutopaySuggestions` (which runs on a `GET /tracker`) inserted the auto-mark-paid payment and applied the balance delta as two un-transactional steps — a mid-way failure could leave a payment without its balance adjustment. Wrapped the INSERT + `applyBalanceDelta` in a single `db.transaction`. Covered by new cases in `tests/trackerService.test.js` (auto-mark creates one payment + drops the balance, idempotent on re-load; route returns the standardized error). (Tracker T2) - **[Tracker] Sidebar overdue badge (and drift/bills) went stale after row actions** — the app never called `queryClient.invalidateQueries`; a Tracker mutation only `refetch()`'d the single tracker query passed down as `refresh`. So after paying/skipping/editing a bill, the sidebar **overdue badge** (`['overdue-count']`, 2-minute staleTime) kept its old number for up to two minutes — you could clear your last overdue bill and still see "3" — and the drift report / bills list didn't update either. Added a `useInvalidateTrackerData()` hook (invalidates `tracker` + `overdue-count` + `drift-report` + `bills`) and wired it in place of the bare `refetch` handed to the rows, `BillModal.onSave`, bank-sync, reorder, and the payment/late-attribution handlers, so the whole shell updates live. (Tracker X3) - **[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) diff --git a/routes/tracker.js b/routes/tracker.js index c5af6fc..2879c0e 100644 --- a/routes/tracker.js +++ b/routes/tracker.js @@ -1,22 +1,40 @@ const express = require('express'); const router = express.Router(); const { getTracker, getUpcomingBills, getOverdueCount } = require('../services/trackerService'); +const { standardizeError } = require('../middleware/errorFormatter'); // GET /api/tracker/overdue-count — lightweight count for sidebar badge router.get('/overdue-count', (req, res) => { - res.json(getOverdueCount(req.user.id)); + try { + res.json(getOverdueCount(req.user.id)); + } catch (err) { + console.error('[tracker/overdue-count]', err.message); + res.status(500).json(standardizeError('Failed to load overdue count', 'INTERNAL_ERROR')); + } }); // GET /api/tracker?year=2026&month=5 router.get('/', (req, res) => { - const result = getTracker(req.user.id, req.query); - if (result.error) return res.status(result.status || 400).json({ error: result.error }); - res.json(result); + try { + const result = getTracker(req.user.id, req.query); + if (result.error) { + return res.status(result.status || 400).json(standardizeError(result.error, 'VALIDATION_ERROR')); + } + res.json(result); + } catch (err) { + console.error('[tracker]', err.message); + res.status(500).json(standardizeError('Failed to load tracker data', 'INTERNAL_ERROR')); + } }); // GET /api/tracker/upcoming?days=30 router.get('/upcoming', (req, res) => { - res.json(getUpcomingBills(req.user.id, req.query)); + try { + res.json(getUpcomingBills(req.user.id, req.query)); + } catch (err) { + console.error('[tracker/upcoming]', err.message); + res.status(500).json(standardizeError('Failed to load upcoming bills', 'INTERNAL_ERROR')); + } }); module.exports = router; diff --git a/services/trackerService.js b/services/trackerService.js index 0efa488..bedab3b 100644 --- a/services/trackerService.js +++ b/services/trackerService.js @@ -412,29 +412,34 @@ function applyAutopaySuggestions(db, bill, payments, mbs, year, month, todayStr, } const balCalc = computeBalanceDelta(bill, suggestedAmount); - const result = db.prepare(` - INSERT INTO payments (bill_id, amount, paid_date, method, notes, balance_delta, interest_delta, payment_source) - VALUES (?, ?, ?, ?, ?, ?, ?, ?) - `).run( - bill.id, - suggestedAmount, - dueDate, - 'autopay', - 'Auto-marked paid on due date', - balCalc?.balance_delta ?? null, - balCalc?.interest_delta ?? null, - 'manual', - ); + // Atomic: the auto-mark payment INSERT and its balance update must both land + // or neither. This runs on a GET /tracker, so a mid-way failure would + // otherwise leave a payment without its balance adjustment (or vice versa). + const insertAutoPayment = db.transaction(() => { + const r = db.prepare(` + INSERT INTO payments (bill_id, amount, paid_date, method, notes, balance_delta, interest_delta, payment_source) + VALUES (?, ?, ?, ?, ?, ?, ?, ?) + `).run( + bill.id, + suggestedAmount, + dueDate, + 'autopay', + 'Auto-marked paid on due date', + balCalc?.balance_delta ?? null, + balCalc?.interest_delta ?? null, + 'manual', + ); + if (balCalc) applyBalanceDelta(db, bill.id, balCalc); + return r.lastInsertRowid; + }); + const paymentId = insertAutoPayment(); - if (balCalc) { - applyBalanceDelta(db, bill.id, balCalc); - bill.current_balance = balCalc.new_balance; - } + if (balCalc) bill.current_balance = balCalc.new_balance; payments.push(db.prepare(` SELECT bill_id, id, amount, paid_date, method, notes, payment_source, transaction_id, created_at, updated_at FROM payments WHERE id = ? - `).get(result.lastInsertRowid)); + `).get(paymentId)); return null; } diff --git a/tests/trackerService.test.js b/tests/trackerService.test.js index 17faf85..5dcd637 100644 --- a/tests/trackerService.test.js +++ b/tests/trackerService.test.js @@ -99,6 +99,50 @@ test('summary.remaining falls back to outstanding balance when no bank + no star assert.equal(t.summary.remaining, 100); }); +test('auto_mark_paid bill creates an autopay payment and drops the balance atomically', () => { + const userId = mkUser('t1-autopay'); + // Eligible: autopay on, assumed_paid, due June 1 (past on June 20), auto_mark_paid. + db.prepare(` + INSERT INTO bills (user_id, name, due_day, billing_cycle, cycle_type, cycle_day, + expected_amount, current_balance, autopay_enabled, autodraft_status, auto_mark_paid, active) + VALUES (?, 'Autodraft', 1, 'monthly', 'monthly', NULL, 10000, 50000, 1, 'assumed_paid', 1, 1) + `).run(userId); + + const t = getTracker(userId, { year: 2026, month: 6 }, JUNE_20); + + const bill = db.prepare("SELECT id, current_balance FROM bills WHERE user_id = ?").get(userId); + const pays = db.prepare("SELECT amount, method, payment_source FROM payments WHERE bill_id = ? AND deleted_at IS NULL").all(bill.id); + assert.equal(pays.length, 1, 'exactly one auto-mark payment created'); + assert.equal(pays[0].amount, 10000, '$100 in cents'); + assert.equal(pays[0].method, 'autopay'); + assert.equal(bill.current_balance, 40000, '500.00 − 100.00 = 400.00 (balance applied)'); + // The row should read as done (paid/autodraft), and running again must not double-charge. + const t2 = getTracker(userId, { year: 2026, month: 6 }, JUNE_20); + const paysAfter = db.prepare("SELECT COUNT(*) c FROM payments WHERE bill_id = ? AND deleted_at IS NULL").get(bill.id).c; + assert.equal(paysAfter, 1, 'second load does not create a duplicate autopay payment'); + assert.ok(t.summary.count_paid + t.summary.count_autodraft >= 1); + assert.ok(t2); +}); + +test('GET /tracker returns a standardized error for an invalid month', async () => { + const router = require('../routes/tracker'); + const layer = router.stack.find(l => l.route?.path === '/' && l.route.methods.get); + const handler = layer.route.stack[layer.route.stack.length - 1].handle; + const userId = mkUser('t1-route'); + const result = await new Promise((resolve) => { + const req = { query: { year: '2026', month: '13' }, user: { id: userId, role: 'user' } }; + const res = { + statusCode: 200, + status(c) { this.statusCode = c; return this; }, + json(d) { resolve({ status: this.statusCode, data: d }); }, + }; + handler(req, res); + }); + assert.equal(result.status, 400); + assert.equal(result.data.code, 'VALIDATION_ERROR', 'standardized shape, not a plain {error}'); + assert.match(result.data.message, /month/); +}); + test('getOverdueCount gates by occurrence and honors paid/skip', () => { const userId = mkUser('t1-overdue'); const ins = insertBill();