diff --git a/HISTORY.md b/HISTORY.md index 36189c5..28f1c76 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -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) diff --git a/docs/QA_PLAN.md b/docs/QA_PLAN.md index 612d317..95a05d7 100644 --- a/docs/QA_PLAN.md +++ b/docs/QA_PLAN.md @@ -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): diff --git a/routes/summary.js b/routes/summary.js index 6ada2c2..1d735b1 100644 --- a/routes/summary.js +++ b/routes/summary.js @@ -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, diff --git a/tests/summaryBankTracking.test.js b/tests/summaryBankTracking.test.js new file mode 100644 index 0000000..0978f06 --- /dev/null +++ b/tests/summaryBankTracking.test.js @@ -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 */ } + } +});