fix(tracker): quick-pay duplicate guard + atomic balance write (X1)
POST /payments/quick had no dedupe and non-atomic INSERT+applyBalanceDelta, unlike /payments/bulk. A double-click/retry/two-tab pay created a second payment and dropped the balance twice; a mid-way failure left a payment with no balance adjustment. Now checks the bill_id+paid_date+amount composite key (idempotent 200) and wraps the write in db.transaction. Test: tests/paymentsQuickRoute.test.js Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
parent
836dbdb9ae
commit
d689ff6e68
|
|
@ -1,6 +1,10 @@
|
||||||
# Bill Tracker — Changelog
|
# Bill Tracker — Changelog
|
||||||
## v0.41.0
|
## 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 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).
|
- **[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).
|
||||||
|
|
|
||||||
|
|
@ -241,15 +241,32 @@ router.post('/quick', (req, res) => {
|
||||||
const payDate = paymentValidation.normalized.paid_date;
|
const payDate = paymentValidation.normalized.paid_date;
|
||||||
const paySource = paymentValidation.normalized.payment_source;
|
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 balCalc = computeBalanceDelta(bill, payAmount);
|
||||||
|
|
||||||
const result = db.prepare(
|
// Atomic: the INSERT and the balance update must both land or neither, so a
|
||||||
'INSERT INTO payments (bill_id, amount, paid_date, method, notes, balance_delta, interest_delta, payment_source) VALUES (?, ?, ?, ?, ?, ?, ?, ?)'
|
// mid-way failure never leaves a payment without its balance adjustment.
|
||||||
).run(bill.id, payAmount, payDate, method || null, notes || null, balCalc?.balance_delta ?? null, balCalc?.interest_delta ?? null, paySource);
|
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(paymentId)));
|
||||||
|
|
||||||
res.status(201).json(serializePayment(db.prepare('SELECT * FROM payments WHERE id = ?').get(result.lastInsertRowid)));
|
|
||||||
});
|
});
|
||||||
|
|
||||||
// POST /api/payments/autopay-suggestions/:billId/confirm
|
// POST /api/payments/autopay-suggestions/:billId/confirm
|
||||||
|
|
|
||||||
|
|
@ -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)');
|
||||||
|
});
|
||||||
Loading…
Reference in New Issue