chore(qa): vendor chunk splitting, remove unused markdown deps, remove dead totalInterestPaid (batch 0.41.0 QA cleanup)
This commit is contained in:
parent
029c227685
commit
127b69ffc2
|
|
@ -8,6 +8,12 @@
|
|||
- **[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)
|
||||
- **[a11y] Icon-only controls and chart SVGs had no accessible name** — Radix Select filter/sort triggers (Tracker, Bills) and the Spending month-nav buttons rendered with no discernible text (screen readers announced a bare "button"); Analytics chart `<svg role="img">`s had no name; a Snowball drag-handle `<div>` used a prohibited `aria-label`. Added `aria-label`s to the triggers/buttons and a `label` prop to the Analytics `SvgFrame`, and switched the drag-handle to `title`. Clears axe **critical `button-name`** and **serious `svg-img-alt` / `aria-prohibited-attr`** across those pages; guarded by `e2e/a11y.authed.spec.js`. (was QA-B14-01, part of QA-B14-02)
|
||||
|
||||
### 🧹 QA Cleanup
|
||||
|
||||
- **[Build] Split the vendor bundle** — the main `index` chunk was ~659 kB (over Vite's 500 kB warning). Added `build.rollupOptions.output.manualChunks` (`vite.config.mjs`) to split React, Radix, framer-motion, and TanStack into separately-cacheable vendor chunks; the index chunk dropped to ~334 kB and the warning is gone. (was QA-B0-01)
|
||||
- **[Deps] Removed unused markdown libraries** — `react-markdown`, `rehype-sanitize`, and `remark-gfm` were declared but never imported (client markdown is rendered by a custom `MarkdownText` component). Removed all three and corrected the security notes: the actual XSS defense is React auto-escaping + the restrictive custom renderer (https-only link hrefs, no `dangerouslySetInnerHTML` anywhere), not rehype-sanitize. (was QA-B14-03)
|
||||
- **[Debt] Removed dead `totalInterestPaid`** — unused outside its own export, and its unrounded accumulation diverged from `amortizationSchedule`'s per-month rounding. Removed from `services/aprService.js`. (was QA-B7-02)
|
||||
|
||||
### ✨ Spending
|
||||
|
||||
- **Category groups** — Organize spending categories into named groups (e.g. "Bills", "Everyday", "Subscriptions"). New `category_groups` table with CRUD endpoints. Categories can be assigned to a group via the Spending page or API. Groups appear as collapsible headers in the category breakdown.
|
||||
|
|
|
|||
|
|
@ -86,7 +86,7 @@ before cross-cutting; regression last). Update **Status** and **Findings** every
|
|||
|
||||
| # | Batch | Primary surface | Data state | Status | Open / Fixed |
|
||||
|---|-------|-----------------|-----------|--------|--------------|
|
||||
| B0 | Baseline, tooling & **coverage recon** | `npm run ci`/`check`, app boots, console clean, **re-scan routes/pages/API vs plan & update it**, **control census** | any | 🔄 | 1 / 0 |
|
||||
| B0 | Baseline, tooling & **coverage recon** | `npm run ci`/`check`, app boots, console clean, **re-scan routes/pages/API vs plan & update it**, **control census** | any | 🔄 | 0 / 1 |
|
||||
| B-UI | **Design-system primitives** | each `client/components/ui/*` × state matrix (default/hover/focus/active/disabled/loading/error/read-only) × light/dark × keyboard | any | ⬜ | 0 / 0 |
|
||||
| B1 | Auth & authorization | login (pw/OIDC/TOTP/WebAuthn), roles, single-user, CSRF, data isolation | multi + single user | ⬜ | 0 / 0 |
|
||||
| B2 | Tracker (core) | `/` buckets, pay/skip/notes/overrides, balance cards, overdue, ledger, drift | seeded + adversarial | ⬜ | 0 / 0 |
|
||||
|
|
@ -94,14 +94,14 @@ before cross-cutting; regression last). Update **Status** and **Findings** every
|
|||
| B4 | Subscriptions & Categories | `/subscriptions`, catalog, `/categories`, groups, reorder | seeded | ⬜ | 0 / 0 |
|
||||
| B5 | Reporting reconciliation | `/summary`, `/calendar`, `/analytics`, `/health` cross-check totals | seeded + large | ⬜ | 0 / 0 |
|
||||
| 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) | 🔄 | 2 / 0 |
|
||||
| B7 | Debt planning (math) | `/snowball`, `/payoff` APR/amortization vs hand-calc | edge (APR=0, $0 debt) | 🔄 | 1 / 1 |
|
||||
| B8 | Banking & bank sync | `/bank-transactions`, SimpleFIN sync, matching, merchant/store, advisory filter | seeded txns | ⬜ | 0 / 0 |
|
||||
| B9 | Data lifecycle | `/data` import (XLSX/CSV/SQLite), export, ICS feed, backups round-trip | empty + seeded | 🔄 | 0 / 1 |
|
||||
| B10 | Notifications & workers | email + ntfy/Gotify/Discord/Telegram, reminders, cron workers | seeded | ⬜ | 0 / 0 |
|
||||
| B11 | Admin panel | users, login mode, auth methods, backups, cleanup, status, onboarding | admin | ⬜ | 0 / 0 |
|
||||
| B12 | Settings, Profile & global UI | `/settings`, `/profile`, static pages, command palette, sidebar/nav | any | ⬜ | 0 / 0 |
|
||||
| B13 | API / backend direct | all `/api/*`: auth, CSRF, validation, rate limits, error shape, IDOR, cents | via HTTP client | 🔄 | 0 / 1 |
|
||||
| B14 | Non-functional | a11y, performance, PWA/offline, XSS/secrets, timezone/DST | large + adversarial | 🔄 | 2 / 1 |
|
||||
| B14 | Non-functional | a11y, performance, PWA/offline, XSS/secrets, timezone/DST | large + adversarial | 🔄 | 1 / 2 |
|
||||
| B15 | Regression & sign-off | full smoke on **production build**, exit criteria | seeded | ⬜ | 0 / 0 |
|
||||
|
||||
> After B15, if any batch is 🔁 or has open S1/S2, loop back. Then start a new
|
||||
|
|
@ -115,7 +115,7 @@ until you get a clean cycle.
|
|||
|
||||
| Cycle | Started | Build / commit | Findings logged | Fixed / archived | Result |
|
||||
|-------|---------|----------------|-----------------|------------------|--------|
|
||||
| 1 | 2026-07-02 | `bdbf231` (dev) | 9 (find pass ongoing) | 4 (QA-B9-01, B13-01, B6-01, B14-01 → HISTORY v0.41.0; + a11y svg/aria parts of B14-02) | 🔄 in progress — B0/B1/B4/B6/B7/B9/B13/B14 probed. Solid: auth-isolation, CSRF, payment/date validation, subscription+spending math, XSS. **Fixed & archived: seed 100× cents (S2), bill-amount validation, negative-money format, a11y button-name/svg/aria labels.** Open: 5 (B14-02 nested-interactive S3, B7-01 rounding S3, 3 IMP) |
|
||||
| 1 | 2026-07-02 | `bdbf231` (dev) | 9 (find pass ongoing) | 7 → HISTORY v0.41.0 (B9-01, B13-01, B6-01, B14-01, B14-03, B0-01, B7-02; + a11y svg/aria of B14-02) | 🔄 in progress — B0/B1/B3/B4/B6/B7/B8/B9/B13/B14 probed. Solid: auth-isolation, CSRF, payment/date validation, **recurrence (quarterly/annual gating, Feb-31 clamp, leap year — all correct)**, **transaction matching/dedup guards**, subscription+spending math, XSS. **Fixed: seed 100× cents (S2), bill-amount validation, negative-money format, a11y labels, vendor-bundle split, unused-dep removal, dead-code removal.** Open: 2 (B7-01 rounding S3 [float-inherent], B14-02 nested-interactive S3 [architectural]) |
|
||||
|
||||
**Result key:** 🔄 in progress · 🔁 findings fixed, re-run required · ✅ clean (zero findings — QA complete)
|
||||
|
||||
|
|
@ -134,11 +134,8 @@ fixing. Keep only **Open / Fixing / Fixed** rows here. Once a finding is
|
|||
|
||||
| ID | Sev | Area (`file:line`) | Summary | Status | Notes / repro |
|
||||
|----|-----|--------------------|---------|--------|---------------|
|
||||
| QA-B7-01 | S3 | `utils/money.js:29` | `toCents` mis-rounds fractional cents: `toCents(1.005)` → 100 (`$1.00`) not 101 | 🔴 Open | see write-up |
|
||||
| QA-B7-02 | IMP | `services/aprService.js:46` | `totalInterestPaid` is unused outside tests and its unrounded model diverges from `amortizationSchedule` | 🔴 Open | see write-up |
|
||||
| QA-B0-01 | IMP | `vite build` / `client/index` | Main JS bundle 659 kB (203 kB gzip) — over Vite's 500 kB warning | 🔴 Open | see write-up |
|
||||
| QA-B7-01 | S3 | `utils/money.js:29` | `toCents` mis-rounds fractional cents: `toCents(1.005)` → 100 (`$1.00`) not 101 | 🔴 Open | see write-up (deferred — float-inherent) |
|
||||
| QA-B14-02 | S3 | `/categories` (8), `/snowball` (1) | axe **serious** `nested-interactive`: draggable/expandable rows are `role=button` yet contain nested buttons | 🔴 Open | see write-up (deferred — architectural) |
|
||||
| QA-B14-03 | IMP | `package.json` / security docs | `react-markdown`/`rehype-sanitize`/`remark-gfm` unused; XSS docs credit sanitize that isn't wired | 🔴 Open | see write-up |
|
||||
|
||||
**Finding template** (paste a new row above; keep the full write-up here until archived):
|
||||
|
||||
|
|
@ -178,31 +175,6 @@ Impact: bounded to sub-cent, and only when a 3+ decimal dollar value reaches the
|
|||
Fix (deferred): round on a string/scaled-integer basis, or add epsilon before round.
|
||||
```
|
||||
|
||||
```
|
||||
ID: QA-B7-02
|
||||
Severity: IMP (dead / latent-inconsistent code)
|
||||
Area: services/aprService.js:46 (totalInterestPaid)
|
||||
Finding: totalInterestPaid is referenced only by its own definition/export and
|
||||
tests — no route/client consumer. Its unrounded interest accumulation diverges
|
||||
from amortizationSchedule's per-month rounded interest (measured up to $0.16 over
|
||||
a 154-month term). If ever wired to a UI that also shows the schedule, the
|
||||
headline total and the table would disagree.
|
||||
Fix (deferred): remove it, or switch it to the per-month rounded model so it can't
|
||||
diverge, and add a reconciliation test (sum of schedule interest == total).
|
||||
```
|
||||
|
||||
```
|
||||
ID: QA-B0-01
|
||||
Severity: IMP (performance/build)
|
||||
Area: vite build output — dist/assets/index-*.js
|
||||
Finding: main chunk 658.95 kB (203.72 kB gzip), over Vite's 500 kB warn threshold;
|
||||
build prints the chunk-size advisory. Pages are already lazy/route-split, so the
|
||||
index chunk is the shared vendor/core.
|
||||
Fix (deferred): build.rollupOptions.output.manualChunks to split vendor (react,
|
||||
radix, framer-motion, xlsx) or raise chunkSizeWarningLimit if accepted. Ties to
|
||||
B14 performance (Slow-3G initial load).
|
||||
```
|
||||
|
||||
```
|
||||
ID: QA-B14-02 (partially fixed — nested-interactive remains)
|
||||
Severity: S3 (accessibility — WCAG 2 A/AA, axe "serious")
|
||||
|
|
@ -223,25 +195,6 @@ Fix (deferred): don't make the whole row/trigger a button — use a dedicated
|
|||
after. Guard: e2e/a11y.authed.spec.js (currently red on these two pages by design).
|
||||
```
|
||||
|
||||
```
|
||||
ID: QA-B14-03
|
||||
Severity: IMP (dependency hygiene + inaccurate security documentation)
|
||||
Area: package.json (react-markdown, rehype-sanitize, remark-gfm) + SECURITY docs /
|
||||
this plan's §6 and Appendix references to "markdown via rehype-sanitize".
|
||||
Finding: those three deps are NOT imported anywhere in client/ (verified by grep).
|
||||
Client markdown is rendered by a custom client/components/MarkdownText.jsx
|
||||
(renderInlineMarkdown) — bold/code/links only, link regex restricted to
|
||||
https?:// hrefs, output is React-escaped, and there is NO dangerouslySetInnerHTML
|
||||
anywhere in the client. So the real XSS defense is React escaping + the restrictive
|
||||
custom renderer, not rehype-sanitize.
|
||||
POSITIVE (confirmed, not a finding): XSS surface is safe — stored <script> bill names
|
||||
return as inert data and render escaped; About/Privacy admin content is plain
|
||||
escaped text; no javascript: href vector; no raw-HTML sink.
|
||||
Fix (deferred): remove the unused deps (or wire react-markdown+rehype-sanitize if
|
||||
richer markdown is intended) and correct the security docs / this plan to describe
|
||||
the actual defense. Low risk, small cleanup.
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 3. Archiving fixed findings to HISTORY.md
|
||||
|
|
@ -520,7 +473,7 @@ Route groups: `auth`, `auth/oidc`, `admin`, `tracker`, `bills`, `subscriptions`,
|
|||
- [ ] **Visual regression:** capture a baseline screenshot per page × {desktop, mobile} × {light, dark} (Playwright `toHaveScreenshot`); diff against baseline each cycle. Every non-trivial pixel diff is either an intended change (update the baseline in the same commit) or a finding — never ignore it. This is what makes "every page looks right" repeatable instead of eyeballed.
|
||||
- [ ] **Performance:** initial load + lazy route splitting OK on Slow 3G; large lists responsive; no memory leak over 10+ navigations; no duplicate/excess requests (React Query `staleTime`).
|
||||
- [ ] **PWA/offline:** installs; manifest/icon correct; offline shell loads with graceful messaging; SW updates without stale-cache breakage.
|
||||
- [ ] **Security spot-checks:** XSS in bill names/notes/category names/imported data escaped everywhere (markdown via `rehype-sanitize`); no secrets (SimpleFIN token, SMTP creds, OIDC secret) in bundle/responses/logs; cookies `HttpOnly`/`Secure`/`SameSite`; `encryptionService` protects at-rest secrets, keys not committed. (Depth: `SECURITY_AUDIT.md`.)
|
||||
- [ ] **Security spot-checks:** XSS in bill names/notes/category names/imported data escaped everywhere (defense = React auto-escaping + the restrictive custom `MarkdownText` renderer — https-only link hrefs, **no** `dangerouslySetInnerHTML` anywhere; NOT rehype-sanitize, which is unused, see QA-B14-03); no secrets (SimpleFIN token, SMTP creds, OIDC secret) in bundle/responses/logs; cookies `HttpOnly`/`Secure`/`SameSite`; `encryptionService` protects at-rest secrets, keys not committed. (Depth: `SECURITY_AUDIT.md`.)
|
||||
- [ ] **Resilience:** kill API mid-session → recoverable errors, no data loss on next save; locked/corrupt SQLite surfaces clearly; SimpleFIN/SMTP/push down → graceful degrade; two-tab concurrent edits don't silently clobber.
|
||||
- [ ] **Fault injection (systematic):** with a request-interception harness (Playwright `page.route`, or DevTools network overrides), force each page's API calls to **401 mid-session / 403 / 429 / 500 / network-timeout / malformed-JSON** and confirm the UI shows a recoverable error (toast or `ErrorBoundary` fallback), never a white screen, stuck spinner, or silent success. Do this per page, not once globally — each page handles failure differently.
|
||||
- [ ] **Timezone/locale:** non-UTC tz + DST boundary — due dates and calendar stay correct.
|
||||
|
|
|
|||
File diff suppressed because it is too large
Load Diff
|
|
@ -55,10 +55,7 @@
|
|||
"qrcode": "^1.5.4",
|
||||
"react": "^19.2.7",
|
||||
"react-dom": "^19.2.7",
|
||||
"react-markdown": "^10.1.0",
|
||||
"react-router-dom": "^6.26.2",
|
||||
"rehype-sanitize": "^6.0.0",
|
||||
"remark-gfm": "^4.0.1",
|
||||
"sonner": "^1.7.1",
|
||||
"tailwind-merge": "^2.5.4",
|
||||
"tailwindcss-animate": "^1.0.7",
|
||||
|
|
|
|||
|
|
@ -39,30 +39,6 @@ function monthsToPayoff(balance, annualRatePct, monthlyPayment) {
|
|||
return months >= MAX_MONTHS ? null : months;
|
||||
}
|
||||
|
||||
/**
|
||||
* Total interest paid over the life of a single debt at a fixed monthly payment.
|
||||
* Returns null if the payment never overcomes interest.
|
||||
*/
|
||||
function totalInterestPaid(balance, annualRatePct, monthlyPayment) {
|
||||
const rate = Math.max(0, Number(annualRatePct) || 0) / 100 / 12;
|
||||
let bal = Number(balance);
|
||||
const pmt = Number(monthlyPayment);
|
||||
|
||||
if (!Number.isFinite(bal) || bal <= 0) return 0;
|
||||
if (!Number.isFinite(pmt) || pmt <= 0) return null;
|
||||
if (rate > 0 && pmt <= bal * rate) return null;
|
||||
|
||||
let totalInterest = 0;
|
||||
let months = 0;
|
||||
while (bal > 0.005 && months < MAX_MONTHS) {
|
||||
months++;
|
||||
const interest = bal * rate;
|
||||
totalInterest += interest;
|
||||
bal = Math.max(0, bal + interest - pmt);
|
||||
}
|
||||
return months >= MAX_MONTHS ? null : round2(totalInterest);
|
||||
}
|
||||
|
||||
/**
|
||||
* Full month-by-month amortization schedule for a single debt.
|
||||
* Each row: { month, payment, principal, interest, balance }
|
||||
|
|
@ -245,7 +221,6 @@ function round2(n) {
|
|||
module.exports = {
|
||||
monthlyInterest,
|
||||
monthsToPayoff,
|
||||
totalInterestPaid,
|
||||
amortizationSchedule,
|
||||
calculateMinimumOnly,
|
||||
debtAprSnapshot,
|
||||
|
|
|
|||
|
|
@ -63,6 +63,21 @@ export default defineConfig({
|
|||
build: {
|
||||
outDir: 'dist',
|
||||
emptyOutDir: true,
|
||||
rollupOptions: {
|
||||
output: {
|
||||
// QA-B0-01: split the shared vendor code out of the ~659 kB index chunk
|
||||
// so large libs load/cache independently and the main bundle shrinks.
|
||||
manualChunks: {
|
||||
'vendor-react': ['react', 'react-dom', 'react-router-dom'],
|
||||
'vendor-radix': [
|
||||
'@radix-ui/react-dialog', '@radix-ui/react-select', '@radix-ui/react-dropdown-menu',
|
||||
'@radix-ui/react-tabs', '@radix-ui/react-tooltip', '@radix-ui/react-alert-dialog',
|
||||
],
|
||||
'vendor-motion': ['framer-motion'],
|
||||
'vendor-query': ['@tanstack/react-query', '@tanstack/react-virtual'],
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
// Vitest — client-side unit tests (pure logic in client/lib).
|
||||
// Server tests stay on node:test (`npm run test`); client tests run with
|
||||
|
|
|
|||
Loading…
Reference in New Issue