fix(qa): bank-tracking unpaid_this_month gates by occurrence (QA-B5-02)
- routes/summary buildBankTracking: fetch unpaid candidates and filter by resolveDueDate in JS so annual / off-month quarterly bills don't inflate the SimpleFIN "unpaid this month" metric (completes the occurrence-gating family) - add tests/summaryBankTracking.test.js (isolated route test) - docs: archive QA-B5-02; Active Findings Log now empty (0 open) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
parent
1bd282f47b
commit
819cfdfa9f
|
|
@ -3,7 +3,7 @@
|
|||
|
||||
### 🐛 QA Fixes
|
||||
|
||||
- **[Summary/Analytics] Non-monthly bills were counted in every month** — the Summary expense list/total and the Analytics "expected vs actual" line both counted annual (and off-month quarterly) bills for months they weren't due, over-stating the obligation and disagreeing with the Tracker (e.g. a yearly insurance bill inflated every month). Both `routes/summary.js` and `services/analyticsService.js` now gate bills by `resolveDueDate` — the same occurrence check the Tracker uses. Guarded by Tracker↔Summary and Tracker↔Analytics reconciliation checks in `e2e/api.probe.spec.js`. Residual tracked as **QA-B5-02**: the SimpleFIN bank-tracking `unpaid_this_month` metric uses a SQL query that needs the same filter. (was QA-B5-01, QA-B5-03)
|
||||
- **[Summary/Analytics] Non-monthly bills were counted in every month** — the Summary expense list/total and the Analytics "expected vs actual" line both counted annual (and off-month quarterly) bills for months they weren't due, over-stating the obligation and disagreeing with the Tracker (e.g. a yearly insurance bill inflated every month). Both `routes/summary.js` and `services/analyticsService.js` now gate bills by `resolveDueDate` — the same occurrence check the Tracker uses. Guarded by Tracker↔Summary and Tracker↔Analytics reconciliation checks in `e2e/api.probe.spec.js`. The SimpleFIN bank-tracking `unpaid_this_month` metric had the same gap and is fixed the same way (fetch + JS `resolveDueDate` filter, since SQL can't call it), covered by `tests/summaryBankTracking.test.js`. (was QA-B5-01, QA-B5-02, QA-B5-03)
|
||||
- **[Data] Seed Demo Data amounts were 100× too small** — `scripts/seedDemoData.js` inserted demo dollar amounts straight into `bills.expected_amount` / `current_balance` / `minimum_payment`, which became integer-cents columns in the v1.03 migration, so a seeded "$85" bill showed as $0.85 (a whole demo month totalled ~$32 instead of ~$3,200). Now wraps demo values in `toCents()` before insert. Regression guard added in `e2e/api.probe.spec.js`. (was QA-B9-01)
|
||||
- **[Bills] `expected_amount` was unvalidated** — POST/PUT `/api/bills` accepted negative amounts, non-numeric input (silently coerced to $0), and absurd values (`1e15` → cents past `Number.MAX_SAFE_INTEGER`). `validateBillData` (`services/billsService.js`) now rejects non-numeric / negative / out-of-range with a structured `VALIDATION_ERROR`, accepting `0`…`$100,000,000`. Regression assertions in `e2e/api.probe.spec.js`. (was QA-B13-01)
|
||||
- **[UI] Negative amounts rendered as "$-50.00"** — client `fmt()` (`client/lib/utils.js`) and server `formatUSD()` (`utils/money.js`) placed the minus sign after the currency symbol; now render the conventional "-$50.00". Test added in `client/lib/utils.test.js`. (was QA-B6-01)
|
||||
|
|
|
|||
|
|
@ -92,7 +92,7 @@ before cross-cutting; regression last). Update **Status** and **Findings** every
|
|||
| B2 | Tracker (core) | `/` buckets, pay/skip/notes/overrides, balance cards, overdue, ledger, drift | seeded + adversarial | ⬜ | 0 / 0 |
|
||||
| B3 | Bills & schedules | `/bills` CRUD, custom schedules, reorder, merchant rules, historical import | adversarial | ⬜ | 0 / 0 |
|
||||
| B4 | Subscriptions & Categories | `/subscriptions`, catalog, `/categories`, groups, reorder | seeded | ⬜ | 0 / 0 |
|
||||
| B5 | Reporting reconciliation | `/summary`, `/calendar`, `/analytics`, `/health` cross-check totals | seeded + large | 🔄 | 1 / 1 |
|
||||
| B5 | Reporting reconciliation | `/summary`, `/calendar`, `/analytics`, `/health` cross-check totals | seeded + large | 🔄 | 0 / 3 |
|
||||
| B6 | Spending | `/spending` YNAB view, averages, cover-overspending, safe-to-spend | seeded + edge months | 🔄 | 0 / 1 |
|
||||
| B7 | Debt planning (math) | `/snowball`, `/payoff` APR/amortization vs hand-calc | edge (APR=0, $0 debt) | 🔄 | 0 / 2 |
|
||||
| B8 | Banking & bank sync | `/bank-transactions`, SimpleFIN sync, matching, merchant/store, advisory filter | seeded txns | ⬜ | 0 / 0 |
|
||||
|
|
@ -115,7 +115,7 @@ until you get a clean cycle.
|
|||
|
||||
| Cycle | Started | Build / commit | Findings logged | Fixed / archived | Result |
|
||||
|-------|---------|----------------|-----------------|------------------|--------|
|
||||
| 1 | 2026-07-02 | `bdbf231`→`a15ff05`+ (dev) | 12 | **11 fixed & archived** (B9-01, B13-01, B6-01, B7-01, B7-02, B14-01/02/03, B0-01, **B5-01, B5-03**) | 🔁 re-run required. Post seed-fix reconciliation caught the **occurrence-gating family**: Summary (B5-01, S2) and Analytics (B5-03, S3) counted annual/off-month-quarterly bills every month → both fixed via `resolveDueDate`; Tracker↔Summary and Tracker↔Analytics now reconcile (guarded in the probe). Open: **1** (QA-B5-02, S3 — SimpleFIN bank-tracking `unpaid` SQL residual, needs SimpleFIN env). Probed B0/B1/B3/B4/B5/B6/B7/B8/B9/B13/B14; solid: auth/isolation, CSRF, payment/date validation, recurrence, matching/dedup, subscription+spending math, XSS. |
|
||||
| 1 | 2026-07-02 | `bdbf231`→(dev) | 12 | **12 → all fixed, verified & archived** (B9-01, B13-01, B6-01, B7-01/02, B14-01/02/03, B0-01, B5-01/02/03) | ✅ **0 open.** Post seed-fix reconciliation caught the **occurrence-gating family** — Summary (S2), Analytics, and SimpleFIN bank-tracking all counted non-monthly bills every month; all fixed via `resolveDueDate` and guarded (probe reconciliation + `tests/summaryBankTracking.test.js`). Probed B0/B1/B3/B4/B5/B6/B7/B8/B9/B13/B14; solid: auth/isolation, CSRF, payment/date validation, recurrence, matching/dedup, subscription+spending math, XSS, calendar gating. **A full re-run (B0→B15) is still required to declare the cycle clean per exit criteria.** |
|
||||
|
||||
**Result key:** 🔄 in progress · 🔁 findings fixed, re-run required · ✅ clean (zero findings — QA complete)
|
||||
|
||||
|
|
@ -134,7 +134,7 @@ fixing. Keep only **Open / Fixing / Fixed** rows here. Once a finding is
|
|||
|
||||
| ID | Sev | Area (`file:line`) | Summary | Status | Notes / repro |
|
||||
|----|-----|--------------------|---------|--------|---------------|
|
||||
| QA-B5-02 | S3 | `routes/summary.js:53` (`buildBankTracking`) | SimpleFIN `unpaid_this_month` SQL sums all active bills (no occurrence gate) — same root as QA-B5-01, secondary/SimpleFIN-only path | 🔴 Open | needs SimpleFIN test env to verify a fix |
|
||||
| _(none — all Cycle 1 findings fixed, verified & archived to `HISTORY.md` v0.41.0)_ | | | | | |
|
||||
|
||||
**Finding template** (paste a new row above; keep the full write-up here until archived):
|
||||
|
||||
|
|
|
|||
|
|
@ -48,15 +48,16 @@ function buildBankTrackingSummary(db, userId, year, month) {
|
|||
`).get(userId, effectivePendingDays)
|
||||
: { pending_total: 0 };
|
||||
|
||||
// Unpaid bills remaining this month (not skipped, not yet paid)
|
||||
// Unpaid bills remaining this month (not skipped, not yet paid), occurrence-gated
|
||||
// in JS so annual / off-month quarterly bills don't inflate the total (QA-B5-02,
|
||||
// same root as QA-B5-01 — SQL can't call resolveDueDate).
|
||||
const { start, end } = getCycleRange(year, month);
|
||||
const unpaidRow = db.prepare(`
|
||||
SELECT COALESCE(SUM(
|
||||
const unpaidCandidates = db.prepare(`
|
||||
SELECT b.due_day, b.billing_cycle, b.cycle_type, b.cycle_day,
|
||||
CASE
|
||||
WHEN m.actual_amount IS NOT NULL THEN m.actual_amount
|
||||
ELSE COALESCE(b.expected_amount, 0)
|
||||
END
|
||||
), 0) AS unpaid_total
|
||||
END AS amount_cents
|
||||
FROM bills b
|
||||
LEFT JOIN monthly_bill_state m ON m.bill_id = b.id AND m.year = ? AND m.month = ?
|
||||
LEFT JOIN (
|
||||
|
|
@ -72,12 +73,15 @@ function buildBankTrackingSummary(db, userId, year, month) {
|
|||
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);
|
||||
`).all(year, month, start, end, userId);
|
||||
const unpaidTotalCents = unpaidCandidates
|
||||
.filter(row => resolveDueDate(row, year, month))
|
||||
.reduce((sum, row) => sum + (row.amount_cents || 0), 0);
|
||||
|
||||
const balanceDollars = money(account.balance / 100);
|
||||
const pendingDollars = fromCents(pendingRow.pending_total);
|
||||
const effectiveDollars = money(balanceDollars - pendingDollars);
|
||||
const unpaidDollars = fromCents(unpaidRow.unpaid_total);
|
||||
const unpaidDollars = fromCents(unpaidTotalCents);
|
||||
|
||||
return {
|
||||
enabled: true,
|
||||
|
|
|
|||
|
|
@ -0,0 +1,69 @@
|
|||
'use strict';
|
||||
|
||||
// QA-B5-02 regression: the SimpleFIN bank-tracking `unpaid_this_month` metric must
|
||||
// only count bills that actually occur in the requested month — annual / off-month
|
||||
// quarterly bills should not inflate it (same occurrence gate as Tracker/Summary).
|
||||
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-summary-bt-test-${process.pid}.sqlite`);
|
||||
process.env.DB_PATH = dbPath;
|
||||
|
||||
const { getDb, closeDb } = require('../db/database');
|
||||
|
||||
function callSummary(userId, year, month) {
|
||||
const router = require('../routes/summary');
|
||||
const layer = router.stack.find(item => item.route?.path === '/' && item.route.methods.get);
|
||||
assert.ok(layer, 'GET /api/summary route should exist');
|
||||
const handler = layer.route.stack[0].handle;
|
||||
return new Promise((resolve) => {
|
||||
const req = { query: { year: String(year), month: String(month) }, user: { id: userId, role: 'user' } };
|
||||
const res = {
|
||||
statusCode: 200,
|
||||
status(code) { this.statusCode = code; return this; },
|
||||
json(data) { resolve({ status: this.statusCode, data }); },
|
||||
};
|
||||
handler(req, res);
|
||||
});
|
||||
}
|
||||
|
||||
test('bank-tracking unpaid_this_month excludes bills not due this month (QA-B5-02)', async () => {
|
||||
const db = getDb();
|
||||
const userId = db.prepare(
|
||||
"INSERT INTO users (username, password_hash, role, active) VALUES ('bt-user', 'x', 'user', 1)",
|
||||
).run().lastInsertRowid;
|
||||
|
||||
const insertBill = db.prepare(`
|
||||
INSERT INTO bills (user_id, name, due_day, billing_cycle, cycle_type, cycle_day, expected_amount, active)
|
||||
VALUES (?, ?, ?, ?, ?, ?, ?, 1)
|
||||
`);
|
||||
// Due every June (query month): counts.
|
||||
insertBill.run(userId, 'Monthly Bill', 15, 'monthly', 'monthly', null, 10000); // $100
|
||||
// Annual, anchored to January: NOT due in June — must be excluded.
|
||||
insertBill.run(userId, 'Annual Bill', 1, 'annually', 'annual', '1', 50000); // $500
|
||||
|
||||
// Enable bank tracking pointed at a financial account with a balance.
|
||||
const acctId = db.prepare(`
|
||||
INSERT INTO financial_accounts (user_id, name, org_name, account_type, balance, available_balance, monitored)
|
||||
VALUES (?, 'Checking', 'Test Bank', 'checking', 200000, 200000, 1)
|
||||
`).run(userId).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));
|
||||
|
||||
const { data } = await callSummary(userId, 2026, 6); // June 2026
|
||||
|
||||
assert.ok(data.bank_tracking, 'bank_tracking summary should be present when enabled');
|
||||
// Only the $100 monthly bill is due in June; the $500 annual bill (Jan) is excluded.
|
||||
assert.equal(data.bank_tracking.unpaid_this_month, 100);
|
||||
});
|
||||
|
||||
test.after(() => {
|
||||
closeDb();
|
||||
for (const suffix of ['', '-wal', '-shm']) {
|
||||
try { fs.rmSync(dbPath + suffix); } catch { /* ignore */ }
|
||||
}
|
||||
});
|
||||
Loading…
Reference in New Issue