diff --git a/HISTORY.md b/HISTORY.md index 58988db..61b1fd2 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -1,6 +1,10 @@ # Bill Tracker — Changelog ## v0.41.0 +### 🐛 Tracker & bill-modal hardening + +- **[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 - **[Data] Redesigned the Data page (modern, goal-oriented, 2026)** — replaced the dense tabbed/collapsible-card layout with a settings-style **two-pane** shell: a sticky goal-nav (Bank sync · Transactions · Import · Export & backups) beside one active pane, collapsing to a segmented control on mobile. Adds a **connection hero** with five distinct states (loading / server-disabled / error+retry / not-connected / connected±needs-attention) so a network blip is never mistaken for "not connected" and a server without SimpleFIN never shows a dead Connect button; **`?section=` deep-linking** (URL source of truth, back-button friendly, migrates the old tab key); plain-language titles + icons; a live **"N to review"** badge and **health dot** on the nav; at-a-glance transaction count + "syncs automatically" reassurance; **lazy-loaded** import panes; reduced-motion-aware transitions; and command-palette section links. `/data` is now covered by the authed axe sweep (zero critical/serious). Shell only — every section component (and the SimpleFIN sync buttons) is reused unchanged. `SectionCard` chrome modernized (icon chip, sentence-case titles). diff --git a/routes/payments.js b/routes/payments.js index bb17ad0..7a520a4 100644 --- a/routes/payments.js +++ b/routes/payments.js @@ -241,15 +241,32 @@ router.post('/quick', (req, res) => { const payDate = paymentValidation.normalized.paid_date; const paySource = paymentValidation.normalized.payment_source; + // Guard against a double-clicked / retried "pay" (or two tabs) creating a + // duplicate payment. Mirrors the bulk route's composite-key check + // (bill_id + paid_date + amount). If one already exists, return it idempotently. + const existingDuplicate = db.prepare( + `SELECT p.* FROM payments p + WHERE p.bill_id = ? AND p.paid_date = ? AND p.amount = ? AND p.${SQL_NOT_DELETED} + ORDER BY p.id DESC LIMIT 1` + ).get(bill.id, payDate, payAmount); + if (existingDuplicate) { + return res.status(200).json(serializePayment(existingDuplicate)); + } + const balCalc = computeBalanceDelta(bill, payAmount); - const result = db.prepare( - 'INSERT INTO payments (bill_id, amount, paid_date, method, notes, balance_delta, interest_delta, payment_source) VALUES (?, ?, ?, ?, ?, ?, ?, ?)' - ).run(bill.id, payAmount, payDate, method || null, notes || null, balCalc?.balance_delta ?? null, balCalc?.interest_delta ?? null, paySource); + // Atomic: the INSERT and the balance update must both land or neither, so a + // mid-way failure never leaves a payment without its balance adjustment. + const insertQuickPayment = 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, payAmount, payDate, method || null, notes || null, balCalc?.balance_delta ?? null, balCalc?.interest_delta ?? null, paySource); + applyBalanceDelta(db, bill.id, balCalc); + return r.lastInsertRowid; + }); + const paymentId = insertQuickPayment(); - applyBalanceDelta(db, bill.id, balCalc); - - res.status(201).json(serializePayment(db.prepare('SELECT * FROM payments WHERE id = ?').get(result.lastInsertRowid))); + res.status(201).json(serializePayment(db.prepare('SELECT * FROM payments WHERE id = ?').get(paymentId))); }); // POST /api/payments/autopay-suggestions/:billId/confirm diff --git a/tests/paymentsQuickRoute.test.js b/tests/paymentsQuickRoute.test.js new file mode 100644 index 0000000..4508648 --- /dev/null +++ b/tests/paymentsQuickRoute.test.js @@ -0,0 +1,89 @@ +'use strict'; + +// X1 — POST /payments/quick must be idempotent under a double-clicked / retried +// "pay" and must apply the balance delta atomically with the INSERT. Before the +// fix, a second identical request created a duplicate payment AND double-applied +// the balance drop. +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-quickpay-${process.pid}.sqlite`); +process.env.DB_PATH = dbPath; + +const { getDb, closeDb } = require('../db/database'); +const router = require('../routes/payments'); + +function handler(method, routePath) { + const layer = router.stack.find(l => l.route?.path === routePath && l.route.methods[method]); + assert.ok(layer, `${method.toUpperCase()} ${routePath} exists`); + return layer.route.stack[layer.route.stack.length - 1].handle; +} +function call(method, routePath, { userId, params = {}, body = {} } = {}) { + const h = handler(method, routePath); + return new Promise((resolve) => { + const req = { params, body, query: {}, user: { id: userId, role: 'user' } }; + const res = { + statusCode: 200, + status(c) { this.statusCode = c; return this; }, + json(d) { resolve({ status: this.statusCode, data: d }); }, + }; + h(req, res); + }); +} + +let userId, billId; +test.before(() => { + const db = getDb(); + userId = db.prepare("INSERT INTO users (username, password_hash, role, active) VALUES ('qp-user','x','user',1)").run().lastInsertRowid; + // $100 expected, $1,000 balance so we can observe the balance drop. + billId = db.prepare( + "INSERT INTO bills (user_id, name, due_day, expected_amount, current_balance, minimum_payment, interest_rate, active) VALUES (?, 'Card', 1, 10000, 100000, 5000, 0, 1)", + ).run(userId).lastInsertRowid; +}); +test.after(() => { + closeDb(); + for (const s of ['', '-wal', '-shm']) { try { fs.unlinkSync(dbPath + s); } catch {} } +}); + +test('first quick pay creates the payment (201) and drops the balance once', async () => { + const { status, data } = await call('post', '/quick', { + userId, body: { bill_id: billId, amount: 100, paid_date: '2026-07-01' }, + }); + assert.equal(status, 201); + assert.equal(data.amount, 100, 'payment amount in dollars'); + + const db = getDb(); + const count = db.prepare('SELECT COUNT(*) c FROM payments WHERE bill_id = ? AND deleted_at IS NULL').get(billId).c; + assert.equal(count, 1); + const bal = db.prepare('SELECT current_balance FROM bills WHERE id = ?').get(billId).current_balance; + assert.equal(bal, 90000, '1000.00 − 100.00 = 900.00 (cents)'); +}); + +test('a duplicate quick pay (same bill+date+amount) is idempotent — no second row, balance unchanged', async () => { + const { status, data } = await call('post', '/quick', { + userId, body: { bill_id: billId, amount: 100, paid_date: '2026-07-01' }, + }); + assert.equal(status, 200, 'returns the existing payment, not a new 201'); + assert.equal(data.amount, 100); + + const db = getDb(); + const count = db.prepare('SELECT COUNT(*) c FROM payments WHERE bill_id = ? AND deleted_at IS NULL').get(billId).c; + assert.equal(count, 1, 'still exactly one payment'); + const bal = db.prepare('SELECT current_balance FROM bills WHERE id = ?').get(billId).current_balance; + assert.equal(bal, 90000, 'balance NOT dropped a second time'); +}); + +test('a different amount on the same day is a legitimate new payment', async () => { + const { status } = await call('post', '/quick', { + userId, body: { bill_id: billId, amount: 50, paid_date: '2026-07-01' }, + }); + assert.equal(status, 201); + const db = getDb(); + const count = db.prepare('SELECT COUNT(*) c FROM payments WHERE bill_id = ? AND deleted_at IS NULL').get(billId).c; + assert.equal(count, 2); + const bal = db.prepare('SELECT current_balance FROM bills WHERE id = ?').get(billId).current_balance; + assert.equal(bal, 85000, '900.00 − 50.00 = 850.00 (cents)'); +});