From 4a38cc8614c23429312ba151dca226cad72245ce Mon Sep 17 00:00:00 2001 From: null Date: Fri, 3 Jul 2026 18:10:28 -0500 Subject: [PATCH] fix(tracker): gate bank card unpaid/remaining by occurrence + add trackerService tests (T1) buildBankTracking summed expected_amount for all active unpaid bills with no resolveDueDate gate, so annual/off-month bills inflated unpaid_this_month and the bank remaining (same class as QA-B5-02, live on the bank path). getTracker now derives the unpaid total from the already-gated rows (netting partials) and passes it in. summary.remaining/total_remaining now use the bank card's own remaining in bank mode (agreeing with safe-to-spend), and a stray balance/100 is now fromCents. New tests/trackerService.test.js: gating fix, summary totals, bank-mode remaining agreement, cents<->dollars, getOverdueCount gating. Co-Authored-By: Claude Opus 4.8 --- HISTORY.md | 1 + services/trackerService.js | 74 +++++++++++++++-------- tests/trackerService.test.js | 114 +++++++++++++++++++++++++++++++++++ 3 files changed, 165 insertions(+), 24 deletions(-) create mode 100644 tests/trackerService.test.js diff --git a/HISTORY.md b/HISTORY.md index 61b1fd2..59b03ac 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -3,6 +3,7 @@ ### 🐛 Tracker & bill-modal hardening +- **[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) ### ✨ Data page overhaul diff --git a/services/trackerService.js b/services/trackerService.js index c422765..0efa488 100644 --- a/services/trackerService.js +++ b/services/trackerService.js @@ -64,7 +64,11 @@ function fetchBankPendingCounts(db, userId, billIds) { return counts; } -function buildBankTracking(db, userId, year, month) { +// `gatedUnpaidThisMonth` (dollars) is the occurrence-gated unpaid total the +// caller already computed from the tracker rows (which honor resolveDueDate). +// When provided we use it instead of the ungated SQL below, so annual/quarterly/ +// off-month bills don't inflate `unpaid_this_month` → the bank `remaining`. +function buildBankTracking(db, userId, year, month, gatedUnpaidThisMonth = null) { try { const settings = getUserSettings(userId); if (settings.bank_tracking_enabled !== 'true') return { enabled: false }; @@ -96,29 +100,37 @@ function buildBankTracking(db, userId, year, month) { `).get(userId, days) : { pending_total: 0 }; - const { start, end } = getCycleRange(year, month); - const unpaidRow = db.prepare(` - SELECT COALESCE(SUM( - CASE WHEN m.actual_amount IS NOT NULL THEN m.actual_amount - ELSE COALESCE(b.expected_amount, 0) END - ), 0) AS unpaid_total - FROM bills b - LEFT JOIN monthly_bill_state m ON m.bill_id = b.id AND m.year = ? AND m.month = ? - LEFT JOIN ( - SELECT bill_id, SUM(amount) AS paid_sum FROM payments - WHERE paid_date BETWEEN ? AND ? - AND deleted_at IS NULL - AND ${accountingActiveSql()} - GROUP BY bill_id - ) pay ON pay.bill_id = b.id - WHERE b.user_id = ? AND b.active = 1 AND b.deleted_at IS NULL - AND COALESCE(m.is_skipped, 0) = 0 AND COALESCE(pay.paid_sum, 0) = 0 - `).get(year, month, start, end, userId); + // Fallback (only when the caller didn't pass a gated total, e.g. a direct + // call): the ungated SQL sum. This overcounts off-month bills — prefer the + // gated total from getTracker's rows. + let unpaid; + if (gatedUnpaidThisMonth != null) { + unpaid = roundMoney(gatedUnpaidThisMonth); + } else { + const { start, end } = getCycleRange(year, month); + const unpaidRow = db.prepare(` + SELECT COALESCE(SUM( + CASE WHEN m.actual_amount IS NOT NULL THEN m.actual_amount + ELSE COALESCE(b.expected_amount, 0) END + ), 0) AS unpaid_total + FROM bills b + LEFT JOIN monthly_bill_state m ON m.bill_id = b.id AND m.year = ? AND m.month = ? + LEFT JOIN ( + SELECT bill_id, SUM(amount) AS paid_sum FROM payments + WHERE paid_date BETWEEN ? AND ? + AND deleted_at IS NULL + AND ${accountingActiveSql()} + GROUP BY bill_id + ) pay ON pay.bill_id = b.id + WHERE b.user_id = ? AND b.active = 1 AND b.deleted_at IS NULL + AND COALESCE(m.is_skipped, 0) = 0 AND COALESCE(pay.paid_sum, 0) = 0 + `).get(year, month, start, end, userId); + unpaid = fromCents(unpaidRow.unpaid_total); + } - const balance = roundMoney(account.balance / 100); + const balance = fromCents(account.balance); const pending = fromCents(pendingRow.pending_total); const effective = roundMoney(balance - pending); - const unpaid = fromCents(unpaidRow.unpaid_total); return { enabled: true, @@ -565,7 +577,12 @@ function getTracker(userId, query = {}, now = new Date()) { : (startingAmounts?.fifteenth_amount || 0); const periodLabel = activeRemainingPeriod === '1st' ? '1st balance' : '15th balance'; - const bankTracking = buildBankTracking(db, userId, year, month); + // Occurrence-gated unpaid total for this month: the still-owed amount across + // bills actually due this month (activeRows already honor resolveDueDate), + // netting partial payments. Feeds the bank card so its unpaid/remaining agree + // with the rows instead of over-counting annual/off-month bills. + const activeMonthUnpaid = sumMoney(activeRows, r => Math.max(rowDueAmount(r) - rowPaidTowardDue(r), 0)); + const bankTracking = buildBankTracking(db, userId, year, month, activeMonthUnpaid); const bankPendingCounts = bankTracking.enabled ? fetchBankPendingCounts(db, userId, billIds) : {}; const totalStarting = bankTracking.enabled ? bankTracking.effective_balance @@ -649,8 +666,17 @@ function getTracker(userId, query = {}, now = new Date()) { has_starting_amounts: hasStartingAmounts, total_paid: activeTotalPaid, paid_toward_due: activePaidTowardDue, - remaining: roundMoney(hasStartingAmounts ? periodStartingAmount - periodPaidTowardDue : periodOutstandingBalance), - total_remaining: roundMoney(hasStartingAmounts ? totalStarting - activePaidTowardDue : activeOutstandingBalance), + // In bank mode the effective bank balance is a single pool (no per-period + // split), so both remaining figures use the bank card's own remaining + // (effective_balance − gated unpaid) — keeping the summary consistent with + // safe-to-spend and the bank card instead of showing a different number + // derived from manual starting amounts. + remaining: roundMoney(bankTracking.enabled + ? bankTracking.remaining + : (hasStartingAmounts ? periodStartingAmount - periodPaidTowardDue : periodOutstandingBalance)), + total_remaining: roundMoney(bankTracking.enabled + ? bankTracking.remaining + : (hasStartingAmounts ? totalStarting - activePaidTowardDue : activeOutstandingBalance)), remaining_period: activeRemainingPeriod, remaining_label: periodLabel, remaining_hint: hasStartingAmounts diff --git a/tests/trackerService.test.js b/tests/trackerService.test.js new file mode 100644 index 0000000..17faf85 --- /dev/null +++ b/tests/trackerService.test.js @@ -0,0 +1,114 @@ +'use strict'; + +// T1 — the Tracker's core aggregation (getTracker summary + bank tracking + +// getOverdueCount) had no dedicated tests despite being the densest money-math +// in the app. Covers the occurrence-gating fix (annual/off-month bills must not +// inflate the bank card's unpaid/remaining), summary totals, the bank-mode +// remaining agreement, cents↔dollars integrity, and overdue gating. +const test = require('node:test'); +const assert = require('node:assert/strict'); +const os = require('node:os'); +const path = require('node:path'); +const fs = require('node:fs'); + +const dbPath = path.join(os.tmpdir(), `bill-tracker-trackersvc-${process.pid}.sqlite`); +process.env.DB_PATH = dbPath; + +const { getDb, closeDb } = require('../db/database'); +const { getTracker, getOverdueCount } = require('../services/trackerService'); + +// Fixed "now" so occurrence gating + statuses are deterministic (mid-June 2026). +const JUNE_20 = new Date(2026, 5, 20, 12, 0, 0); + +let db; +function mkUser(name) { + return db.prepare( + "INSERT INTO users (username, password_hash, role, active) VALUES (?, 'x', 'user', 1)", + ).run(name).lastInsertRowid; +} +const insertBill = () => db.prepare(` + INSERT INTO bills (user_id, name, due_day, billing_cycle, cycle_type, cycle_day, expected_amount, active) + VALUES (?, ?, ?, ?, ?, ?, ?, 1) +`); +function enableBank(userId, balanceCents) { + const acctId = db.prepare(` + INSERT INTO financial_accounts (user_id, name, org_name, account_type, balance, available_balance, monitored) + VALUES (?, 'Checking', 'Test Bank', 'checking', ?, ?, 1) + `).run(userId, balanceCents, balanceCents).lastInsertRowid; + const setSetting = db.prepare('INSERT INTO user_settings (user_id, key, value) VALUES (?, ?, ?)'); + setSetting.run(userId, 'bank_tracking_enabled', 'true'); + setSetting.run(userId, 'bank_tracking_account_id', String(acctId)); + // Pending window 0 so no recent-manual-payment guessing affects the balance. + setSetting.run(userId, 'bank_tracking_pending_days', '0'); +} + +test.before(() => { db = getDb(); }); +test.after(() => { + closeDb(); + for (const s of ['', '-wal', '-shm']) { try { fs.rmSync(dbPath + s); } catch {} } +}); + +test('bank mode: annual/off-month bill does NOT inflate unpaid_this_month or remaining (gating fix)', () => { + const userId = mkUser('t1-gating'); + const ins = insertBill(); + ins.run(userId, 'Monthly', 15, 'monthly', 'monthly', null, 10000); // $100 due every month + ins.run(userId, 'Annual (Jan)', 1, 'annually', 'annual', '1', 50000); // $500, not due in June + enableBank(userId, 200000); // $2,000 balance + + const t = getTracker(userId, { year: 2026, month: 6 }, JUNE_20); + + assert.equal(t.bank_tracking.enabled, true); + // Only the $100 monthly bill is due in June; the $500 annual bill is excluded. + assert.equal(t.bank_tracking.unpaid_this_month, 100, 'gated: $100, not $600'); + assert.equal(t.bank_tracking.balance, 2000, 'balance in dollars (fromCents)'); + assert.equal(t.bank_tracking.remaining, 1900, '2000 − 100 (not 2000 − 600)'); + // Fix #2: the summary remaining figures agree with the bank card. + assert.equal(t.summary.remaining, 1900); + assert.equal(t.summary.total_remaining, 1900); +}); + +test('summary totals: gated expected, total_paid, paid_toward_due (cents→dollars)', () => { + const userId = mkUser('t1-summary'); + const ins = insertBill(); + const billA = ins.run(userId, 'A due 15', 15, 'monthly', 'monthly', null, 10000).lastInsertRowid; // $100 + const billB = ins.run(userId, 'B due 1', 1, 'monthly', 'monthly', null, 20000).lastInsertRowid; // $200 + ins.run(userId, 'Annual (Jan)', 1, 'annually', 'annual', '1', 90000); // $900, excluded in June + + const pay = db.prepare( + "INSERT INTO payments (bill_id, amount, paid_date, payment_source) VALUES (?, ?, ?, 'manual')", + ); + pay.run(billA, 10000, '2026-06-15'); // pay A in full ($100) + pay.run(billB, 5000, '2026-06-10'); // partial toward B ($50) + + const t = getTracker(userId, { year: 2026, month: 6 }, JUNE_20); + + assert.equal(t.summary.total_expected, 300, 'only June bills: 100 + 200 (annual excluded)'); + assert.equal(t.summary.total_paid, 150, '100 + 50'); + assert.equal(t.summary.paid_toward_due, 150, 'both payments are ≤ their due amount'); + assert.equal(t.rows.filter(r => r.name === 'Annual (Jan)').length, 0, 'annual bill absent from rows'); +}); + +test('summary.remaining falls back to outstanding balance when no bank + no starting amounts', () => { + const userId = mkUser('t1-nobank'); + insertBill().run(userId, 'Solo due 15', 15, 'monthly', 'monthly', null, 10000); // $100, unpaid + const t = getTracker(userId, { year: 2026, month: 6 }, JUNE_20); + assert.equal(t.bank_tracking.enabled, false); + assert.equal(t.summary.has_starting_amounts, false); + // No bank / no starting amounts → remaining is the outstanding still-owed for + // the current period (June 20 → "15th" period); the $100 bill is unpaid. + assert.equal(t.summary.remaining, 100); +}); + +test('getOverdueCount gates by occurrence and honors paid/skip', () => { + const userId = mkUser('t1-overdue'); + const ins = insertBill(); + ins.run(userId, 'Overdue monthly', 1, 'monthly', 'monthly', null, 10000); // due June 1, unpaid → overdue on June 20 + ins.run(userId, 'Annual (Jan)', 1, 'annually', 'annual', '1', 50000); // not due in June → NOT overdue + const paidBill = ins.run(userId, 'Paid monthly', 1, 'monthly', 'monthly', null, 10000).lastInsertRowid; + db.prepare("INSERT INTO payments (bill_id, amount, paid_date, payment_source) VALUES (?, 10000, '2026-06-02', 'manual')") + .run(paidBill); + + const { count, names } = getOverdueCount(userId, JUNE_20); + assert.equal(count, 1, 'only the unpaid monthly bill due June 1 is overdue'); + assert.deepEqual(names, ['Overdue monthly']); +});