feat(reports): shared account filter state in useReportsPeriod (#272) #280

Closed

maximus wants to merge 0 commits from issue-272-report-filters-hook into main

maximus commented

2026-07-11 19:02:22 +00:00

Owner

Resolves #272

Generated autonomously by /autopilot run of 2026-07-11

Extends the shared useReportsPeriod hook additively with accountIds: number[] (URL-backed via a new sources query param, comma-separated, default [] = no filter) and adds the ReportFilters { period, accountIds } type in shared/types. URL parsing validates each token as a finite integer (strict regex + Number.isSafeInteger), dropping invalid tokens individually rather than discarding the whole selection.

Foundation only (issue I1 of milestone overnight-2026-07-08-filtres-fondation): no UI exposes the filter yet and no service reads accountIds yet — both land in follow-up issues (#273+). The hook shape is purely extended (no rename, no removed fields), so all 6 existing consumers (useTrends, useCompare, useCategoryZoom, ReportsCategoryPage, ReportsPage, ReportsComparePage) compile unchanged, confirmed by a green npm run build.

Tests: 728 -> 745 (17 added, 0 regressions). Build (tsc+vite) green. No DB migration.

Reviewer caveat: none blocking. One judgment call worth a second look: invalid tokens in the sources param are dropped individually (e.g. sources=3,abc,7 -> [3,7]) rather than invalidating the whole list — see parseAccountIds's docstring/tests in src/hooks/useReportsPeriod.ts / .test.ts if you want to confirm that matches your intent for a hand-edited/corrupted URL.

Resolves #272 Generated autonomously by /autopilot run of 2026-07-11 Extends the shared `useReportsPeriod` hook additively with `accountIds: number[]` (URL-backed via a new `sources` query param, comma-separated, default `[]` = no filter) and adds the `ReportFilters { period, accountIds }` type in `shared/types`. URL parsing validates each token as a finite integer (strict regex + `Number.isSafeInteger`), dropping invalid tokens individually rather than discarding the whole selection. Foundation only (issue I1 of milestone `overnight-2026-07-08-filtres-fondation`): no UI exposes the filter yet and no service reads `accountIds` yet — both land in follow-up issues (#273+). The hook shape is purely extended (no rename, no removed fields), so all 6 existing consumers (`useTrends`, `useCompare`, `useCategoryZoom`, `ReportsCategoryPage`, `ReportsPage`, `ReportsComparePage`) compile unchanged, confirmed by a green `npm run build`. Tests: 728 -> 745 (17 added, 0 regressions). Build (tsc+vite) green. No DB migration. Reviewer caveat: none blocking. One judgment call worth a second look: invalid tokens in the `sources` param are dropped individually (e.g. `sources=3,abc,7` -> `[3,7]`) rather than invalidating the whole list — see `parseAccountIds`'s docstring/tests in `src/hooks/useReportsPeriod.ts` / `.test.ts` if you want to confirm that matches your intent for a hand-edited/corrupted URL.

maximus added 1 commit 2026-07-11 19:02:23 +00:00

feat(reports): extend useReportsPeriod with account filter foundation

PR Check / rust (pull_request) Successful in 21m28s

Details

PR Check / frontend (pull_request) Successful in 2m22s

Details

65be6c7482

Add accountIds: number[] to the shared useReportsPeriod hook, URL-backed
via a dedicated `sources` query param (comma-separated, bookmarkable like
the existing period/from/to). Purely additive: the hook's existing shape
is unchanged, so all 6 consumers (useTrends, useCompare, useCategoryZoom,
ReportsCategoryPage, ReportsPage, ReportsComparePage) keep compiling as-is.

Also adds the shared ReportFilters { period, accountIds } type for
follow-up issues to consume, and exports pure parseAccountIds/
serializeAccountIds helpers (same hookless-testability pattern as
resolveReportsPeriod). URL parsing validates each token as a finite
integer via a strict regex + Number.isSafeInteger, dropping invalid
tokens individually rather than discarding the whole selection.

This is foundation only: no UI control exposes the filter yet, and no
service reads accountIds yet (both land in follow-up issues of the
"rapports uniformes" epic).

Resolves #272

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

maximus added the

autopilot:pending-human

label 2026-07-11 19:02:39 +00:00

maximus commented

2026-07-11 20:10:41 +00:00

Author

Owner

/pr-review — APPROVE

Résumé : fondation propre et purement additive. Le hook useReportsPeriod gagne accountIds (adossé au param d'URL sources) plus deux fonctions pures parseAccountIds/serializeAccountIds, validées de façon défensive et bien testées (17 tests). Changement rétrocompatible : les 7 consommateurs existants ne déstructurent que des champs préexistants → aucune régression possible. Aucune migration DB.

Points vérifiés

Correction du parsing : filtre en deux temps (regex ^-?\d+$ puis Number.isSafeInteger) — c'est bien la 2e passe qui rejette 99999999999999999999 (tous chiffres → passe la regex mais déborde isSafeInteger). Notation hex/exponentielle, décimales, tokens vides et espaces sont tous couverts. Aucun chemin d'exception (toutes les opérations sont totales sur des strings).
Rétrocompat : UseReportsPeriodResult est étendu, jamais réduit. Les 4 pages + useCompare/useTrends/useCategoryZoom ne lisent que period/setPeriod/from/to/setCustomDates → build vert crédible.
Cohérence : setAccountIds suit à l'identique le pattern setPeriod/setCustomDates (updater fonctionnel new URLSearchParams(prev), { replace: true }), et supprime sources quand la sélection est vide — miroir de la suppression de from/to pour une période non-custom.
Sémantique du type : ReportFilters.accountIds = liste de ImportSource.id (bien number), distinct de balance_accounts — le docstring le note explicitement. Bon réflexe.
Sécurité : validation stricte d'un param d'URL au bon endroit (frontière). Changement net-positif. Pas de secret, pas d'injection.
Docs/CHANGELOG : architecture.md (table des hooks) + CHANGELOG.md/.fr.md sous [Unreleased] mis à jour. Conforme.

Suggestions (non bloquantes, pour les follow-ups #273+)

ReportFilters défini mais non consommé — normal pour une slice « fondation ». À surveiller : le hook expose period+accountIds à plat alors que ReportFilters les regroupe ; décider au moment du câblage services quelle forme fait foi, pour éviter deux modèles parallèles.
ids négatifs / zéro acceptés — import_sources.id est un autoincrement ≥ 1, donc ^-?\d+$ laisse passer -3/0 qui ne matcheront jamais un compte réel (inoffensif : no-op en IN (...) SQL comme dans un picker). Resserrer en ^\d+$ (+ n > 0) serait plus fidèle au domaine.
Pas de déduplication — parseAccountIds("3,3") → [3,3]. Sans effet en SQL IN, mais si la valeur alimente un état de cases à cocher, un Set/dedup au parse éviterait un double-comptage.

Aucun de ces points ne bloque le merge.

## /pr-review — APPROVE **Résumé** : fondation propre et purement additive. Le hook `useReportsPeriod` gagne `accountIds` (adossé au param d'URL `sources`) plus deux fonctions pures `parseAccountIds`/`serializeAccountIds`, validées de façon défensive et bien testées (17 tests). Changement rétrocompatible : les 7 consommateurs existants ne déstructurent que des champs préexistants → aucune régression possible. Aucune migration DB. ### Points vérifiés - **Correction du parsing** : filtre en deux temps (regex `^-?\d+$` *puis* `Number.isSafeInteger`) — c'est bien la 2e passe qui rejette `99999999999999999999` (tous chiffres → passe la regex mais déborde `isSafeInteger`). Notation hex/exponentielle, décimales, tokens vides et espaces sont tous couverts. Aucun chemin d'exception (toutes les opérations sont totales sur des strings). - **Rétrocompat** : `UseReportsPeriodResult` est étendu, jamais réduit. Les 4 pages + `useCompare`/`useTrends`/`useCategoryZoom` ne lisent que `period/setPeriod/from/to/setCustomDates` → build vert crédible. - **Cohérence** : `setAccountIds` suit à l'identique le pattern `setPeriod`/`setCustomDates` (updater fonctionnel `new URLSearchParams(prev)`, `{ replace: true }`), et supprime `sources` quand la sélection est vide — miroir de la suppression de `from`/`to` pour une période non-custom. - **Sémantique du type** : `ReportFilters.accountIds` = liste de `ImportSource.id` (bien `number`), distinct de `balance_accounts` — le docstring le note explicitement. Bon réflexe. - **Sécurité** : validation stricte d'un param d'URL au bon endroit (frontière). Changement net-positif. Pas de secret, pas d'injection. - **Docs/CHANGELOG** : `architecture.md` (table des hooks) + `CHANGELOG.md`/`.fr.md` sous `[Unreleased]` mis à jour. Conforme. ### Suggestions (non bloquantes, pour les follow-ups #273+) 1. **`ReportFilters` défini mais non consommé** — normal pour une slice « fondation ». À surveiller : le hook expose `period`+`accountIds` à plat alors que `ReportFilters` les regroupe ; décider au moment du câblage services quelle forme fait foi, pour éviter deux modèles parallèles. 2. **ids négatifs / zéro acceptés** — `import_sources.id` est un autoincrement ≥ 1, donc `^-?\d+$` laisse passer `-3`/`0` qui ne matcheront jamais un compte réel (inoffensif : no-op en `IN (...)` SQL comme dans un picker). Resserrer en `^\d+$` (+ `n > 0`) serait plus fidèle au domaine. 3. **Pas de déduplication** — `parseAccountIds("3,3")` → `[3,3]`. Sans effet en SQL `IN`, mais si la valeur alimente un état de cases à cocher, un `Set`/dedup au parse éviterait un double-comptage. Aucun de ces points ne bloque le merge.

maximus commented

2026-07-11 20:39:45 +00:00

Author

Owner

Merged into main via local fast-forward (commit fe9ae01) as a linear stack. Forgejo doesn't flag non-API merges as merged; closing manually.

Merged into `main` via local fast-forward (commit fe9ae01) as a linear stack. Forgejo doesn't flag non-API merges as merged; closing manually.

maximus closed this pull request

2026-07-11 20:39:45 +00:00

PR Check / rust (pull_request) Successful in 21m28s

Details

PR Check / frontend (pull_request) Successful in 2m22s

Details