8d5fab966a
- Delete legacy src/hooks/useReports.ts (the monolithic hook is now fully replaced by the per-domain hooks from #70) - Delete src/components/reports/ReportFilterPanel.tsx (last caller was the pre-refactor ReportsPage; no longer referenced anywhere) - Update docs/architecture.md: reports hook list now lists the 5 per-domain hooks, reports service entry lists every new endpoint, routing section lists the 4 sub-routes, categorizationService entry mentions the new keyword-editing helpers, components folder count + page count updated - Update docs/guide-utilisateur.md section 9: rewrite around hub + 4 sub-reports, explain bookmarkable period via query string, walk through the right-click keyword editing flow, remove stale pivot section - Rewrite in-app docs.reports.* i18n in both FR and EN to match the new UX (hub, sub-reports, contextual keywords) - New ADR docs/adr/0007-reports-hub-refactor.md: context, decision (hub + four routes, per-domain hooks, URL period, security guarantees on the keyword dialog, bounded recursive CTE for category zoom), consequences, alternatives considered - CHANGELOG.md + CHANGELOG.fr.md: Unreleased entries describing the hub, each sub-report, contextual keyword editing, bookmarkable period, view mode persistence, useReports split, pivot removal, and the security posture of AddKeywordDialog / getCategoryZoom Fixes #76 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
95 lines
6.8 KiB
Markdown
95 lines
6.8 KiB
Markdown
# ADR 0007 — Reports hub refactor
|
||
|
||
- Status: Accepted
|
||
- Date: 2026-04-14
|
||
- Milestone: `spec-refonte-rapports`
|
||
|
||
## Context
|
||
|
||
The original `/reports` page exposed five tabs (`trends`, `byCategory`, `overTime`, `budgetVsActual`, `dynamic`) as independent analytic views backed by a single monolithic `useReports` hook. Three problems built up over time:
|
||
|
||
1. **No narrative.** None of the tabs answered "what's important to know about my finances this month?". Users had to navigate several tabs and reconstruct the story themselves.
|
||
2. **Oversized pivot.** The dynamic pivot table (`DynamicReport*`) was powerful but complex. In practice ~90 % of its actual usage boiled down to zooming into a single category. It added visual and cognitive debt without proportional value.
|
||
3. **Disconnected classification.** Keywords could only be edited from `/categories`. Spotting a mis-classified transaction in a report meant leaving the report, editing a rule, and navigating back — a context break that discouraged hygiene.
|
||
|
||
## Decision
|
||
|
||
Refactor `/reports` into a **hub + four dedicated sub-routes**, wired to a shared bookmarkable period and per-domain hooks, with contextual keyword editing via right-click.
|
||
|
||
### Routing
|
||
|
||
```
|
||
/reports → hub (highlights panel + nav cards)
|
||
/reports/highlights → detailed highlights
|
||
/reports/trends → global flow + by-category evolution
|
||
/reports/compare → month vs month / year vs year / actual vs budget
|
||
/reports/category → single-category zoom with rollup
|
||
```
|
||
|
||
All pages share the reporting period through the URL query string (`?from=YYYY-MM-DD&to=YYYY-MM-DD&period=...`), resolved by a pure `resolveReportsPeriod()` helper. Default: current civil year. The query string approach is deliberately **not** a React context — it keeps the URL bookmarkable and stays consistent with the rest of the project, which does not use global React contexts for UI state.
|
||
|
||
### Per-domain hooks
|
||
|
||
The monolithic `useReports` was split into:
|
||
|
||
| Hook | Responsibility |
|
||
|------|----------------|
|
||
| `useReportsPeriod` | Read/write period via `useSearchParams` |
|
||
| `useHighlights` | Fetch highlights snapshot + window-days state |
|
||
| `useTrends` | Fetch global or by-category trends depending on sub-view |
|
||
| `useCompare` | Fetch MoM / YoY; budget mode delegates to `BudgetVsActualTable` |
|
||
| `useCategoryZoom` | Fetch zoom data with rollup toggle |
|
||
|
||
Each page mounts only the hook it needs; no hook carries state for reports the user is not currently viewing.
|
||
|
||
### Dynamic pivot removal
|
||
|
||
Removed outright rather than hidden behind a feature flag. A runtime flag would leave `getDynamicReportData` and its dynamic `FIELD_SQL` in the shipped bundle as a dead-but-live attack surface (OWASP A05:2021). Git history preserves the previous implementation if it ever needs to come back.
|
||
|
||
### Contextual keyword editing
|
||
|
||
Right-clicking a transaction row anywhere transaction-level (category zoom, highlights top transactions, main transactions page) opens an `AddKeywordDialog` that:
|
||
|
||
1. Validates the keyword is 2–64 characters after trim (anti-ReDoS, CWE-1333).
|
||
2. Previews matching transactions via a parameterised `LIKE $1` query, then filters in memory with the existing `buildKeywordRegex` helper (anti-SQL-injection, CWE-89).
|
||
3. Caps the visible preview at 50 rows; an explicit opt-in checkbox lets the user extend the apply to N−50 non-displayed matches.
|
||
4. Runs INSERT (or UPDATE-reassign) + per-transaction UPDATEs inside a single SQL transaction (`BEGIN`/`COMMIT`/`ROLLBACK`), so a crash mid-apply can never leave a keyword orphaned from its transactions (CWE-662).
|
||
5. Renders transaction descriptions as React children — never `dangerouslySetInnerHTML` — with CSS-only truncation (CWE-79).
|
||
6. Recategorises only the rows the user explicitly checked; never retroactive on the entire history.
|
||
|
||
Reassigning an existing keyword across categories requires an explicit confirmation step and leaves the existing keyword's historical matches alone.
|
||
|
||
### Category zoom cycle guard
|
||
|
||
`getCategoryZoom` aggregates via a **bounded** recursive CTE (`WITH RECURSIVE ... WHERE ct.depth < 5`) so a corrupted `parent_id` loop (A → B → A) can never spin forever (CWE-835). A unit test with a canned cyclic fixture asserts termination.
|
||
|
||
## Consequences
|
||
|
||
### Positive
|
||
|
||
- Reports now tell a story ("what moved") before offering analytic depth.
|
||
- Each sub-route is independently code-splittable and testable.
|
||
- Period state is bookmarkable and shareable (copy URL → same view).
|
||
- Keyword hygiene happens inside the report, with a preview that's impossible in the old flow.
|
||
- The dialog's security guarantees are covered by 13 vitest cases (validation boundaries, parameterised LIKE, regex word-boundary filter, BEGIN/COMMIT wrap, ROLLBACK on failure, reassignment policy).
|
||
- The cycle guard is covered by its own test with the depth assertion.
|
||
|
||
### Negative / trade-offs
|
||
|
||
- Adds five new hooks and ~10 new components. Cognitive surface goes up but each piece is smaller and single-purpose.
|
||
- Aggregate tables in the compare and highlights sections intentionally skip the right-click menu (the row represents a category/month, not a transaction, so "add as keyword" is meaningless there). Users looking for consistency may be briefly confused.
|
||
- Right-clicking inside the main transactions page now offers two ways to add a keyword: the existing inline Tag button (no preview) and the new contextual dialog (with preview). Documented as complementary — the inline path is for quick manual classification, the dialog for preview-backed rule authoring.
|
||
|
||
## Alternatives considered
|
||
|
||
- **Keep the five-tab layout and only improve the pivot.** Rejected — it doesn't fix the "no narrative" issue and leaves the oversized pivot problem.
|
||
- **Hide the pivot behind a feature flag.** Rejected — the code stays in the bundle, runtime flag cannot be tree-shaken, and the i18n `reports.pivot.*` keys would have to linger indefinitely. Outright removal with git as the escape hatch was cheaper and cleaner.
|
||
- **React context for the shared period.** Rejected — the project does not use global React contexts for UI state. Query-string persistence is simpler, bookmarkable, and consistent with the rest of the codebase.
|
||
- **A single `ContextMenu` implementation shared across reports and charts.** Chose to generalise the existing `ChartContextMenu` into a `ContextMenu` shell; `ChartContextMenu` now composes the shared shell. Avoids duplicating click-outside + Escape handling.
|
||
|
||
## References
|
||
|
||
- Spec: `spec-refonte-rapports.md`
|
||
- Issues: #69 (foundation), #70 (hooks), #71 (highlights + hub), #72 (trends), #73 (compare), #74 (category zoom + AddKeywordDialog), #75 (right-click propagation), #76 (polish)
|
||
- OWASP A03:2021 (injection), A05:2021 (security misconfiguration)
|
||
- CWE-79 (XSS), CWE-89 (SQL injection), CWE-662 (improper synchronization), CWE-835 (infinite loop), CWE-1333 (ReDoS)
|