feat(reports): adopt shared FilterPanel on the Trends page #283

Closed

maximus wants to merge 1 commit from issue-275-adopt-trends into issue-274-filter-panel

maximus commented

2026-07-11 19:41:34 +00:00

Owner

Resolves #275

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

Summary

Mounts <FilterPanel> (from #274) on ReportsTrendsPage, next to the period selector, replacing the bare <PeriodSelector>.
Threads accountIds/setAccountIds (from useReportsPeriod, #272) into the panel; useTrends now also reads accountIds from useReportsPeriod and forwards it to getMonthlyTrends/getCategoryOverTime (both accountIds-aware since #273).
The account checkbox list (accounts prop) is sourced via getAllImportSources() from transactionService.ts — the same query the Transactions single-source filter (useTransactions.ts) uses — fetched once on mount inside useTrends (new accounts state + SET_ACCOUNTS action), since architecture convention keeps hook state out of the page component.
Empty accountIds ([], the default) behaves as "no filter", already verified byte-identical at the service layer by the #273 test suite (reportService.test.ts).
Layout: the sub-view/chart-type/view-mode toggle row moved from sharing a row with PeriodSelector to sitting right-aligned below the new FilterPanel block (FilterPanel is a self-contained card, like the other reports toolbars).
CHANGELOG (EN+FR) entry under ## [Unreleased] / ### Added; one-line docs/architecture.md update to the useReportsPeriod row reflecting it's now adopted on the Trends page (Compare/Budget/Dashboard/Hub still pending, #276).

Caveat

No new automated test was added for the useTrends/ReportsTrendsPage wiring itself: this repo has no @testing-library/react/jsdom, so hooks with useReducer/useEffect are only unit-tested via extracted pure helpers (see useCompare.test.ts, FilterPanel.test.tsx's own note on this). There's no new pure branching logic here to extract — it's parameter forwarding into already-tested service functions (reportService.test.ts covers the accountIds filter behavior exhaustively, including the empty-array/no-regression case). Manual/code-review verification only for the wiring itself.

Test plan

npm run build green (tsc + vite)
npm test green — 788/788 (baseline unchanged, no new tests added)

Resolves #275 Generated autonomously by /autopilot run of 2026-07-11 ## Summary - Mounts `<FilterPanel>` (from #274) on `ReportsTrendsPage`, next to the period selector, replacing the bare `<PeriodSelector>`. - Threads `accountIds`/`setAccountIds` (from `useReportsPeriod`, #272) into the panel; `useTrends` now also reads `accountIds` from `useReportsPeriod` and forwards it to `getMonthlyTrends`/`getCategoryOverTime` (both accountIds-aware since #273). - The account checkbox list (`accounts` prop) is sourced via `getAllImportSources()` from `transactionService.ts` — the same query the Transactions single-source filter (`useTransactions.ts`) uses — fetched once on mount inside `useTrends` (new `accounts` state + `SET_ACCOUNTS` action), since architecture convention keeps hook state out of the page component. - Empty `accountIds` (`[]`, the default) behaves as "no filter", already verified byte-identical at the service layer by the #273 test suite (`reportService.test.ts`). - Layout: the sub-view/chart-type/view-mode toggle row moved from sharing a row with `PeriodSelector` to sitting right-aligned below the new `FilterPanel` block (FilterPanel is a self-contained card, like the other reports toolbars). - CHANGELOG (EN+FR) entry under `## [Unreleased]` / `### Added`; one-line `docs/architecture.md` update to the `useReportsPeriod` row reflecting it's now adopted on the Trends page (Compare/Budget/Dashboard/Hub still pending, #276). ## Caveat - No new automated test was added for the `useTrends`/`ReportsTrendsPage` wiring itself: this repo has no `@testing-library/react`/jsdom, so hooks with `useReducer`/`useEffect` are only unit-tested via extracted pure helpers (see `useCompare.test.ts`, `FilterPanel.test.tsx`'s own note on this). There's no new pure branching logic here to extract — it's parameter forwarding into already-tested service functions (`reportService.test.ts` covers the `accountIds` filter behavior exhaustively, including the empty-array/no-regression case). Manual/code-review verification only for the wiring itself. ## Test plan - [x] `npm run build` green (tsc + vite) - [x] `npm test` green — 788/788 (baseline unchanged, no new tests added)

maximus added 1 commit 2026-07-11 19:41:34 +00:00

feat(reports): adopt shared FilterPanel on the Trends page 30682004d3

Mounts <FilterPanel> (from #274) on ReportsTrendsPage next to the period
selector, and threads accountIds from useReportsPeriod through useTrends
into getMonthlyTrends/getCategoryOverTime (both already accountIds-aware
since #273). The account checkbox list reuses the same getAllImportSources
query as the Transactions single-source filter. Empty selection = no
filter, byte-identical to pre-#275 output.

Resolves #275

maximus added the

autopilot:pending-human

label 2026-07-11 19:41:58 +00:00

maximus commented

2026-07-11 20:25:48 +00:00

Author

Owner

/pr-review — PR #283 · `feat(reports): adopt shared FilterPanel on the Trends page`

Verdict : APPROVE ✅

Résumé

Branchement propre et bien scopé du FilterPanel partagé (#274) sur la page Tendances. Le diff head-vs-base se limite à 5 fichiers (2× CHANGELOG, docs/architecture.md, useTrends.ts, ReportsTrendsPage.tsx) — pas de dérive de périmètre. Chaque appel a été vérifié contre le code réel des maillons #272-#274 de la pile.

Vérifications

Props FilterPanel — les 4 props passées (temporalControl, accountIds, onAccountIdsChange, accounts) correspondent exactement à FilterPanelProps, types inclus (number[], (number[]) => void, ImportSource[]).
Signatures services — getMonthlyTrends(from, to, ids) : ids en 3ᵉ position = accountIds. getCategoryOverTime(from, to, undefined, ids) : undefined en 3ᵉ laisse le défaut topN = 50 s'appliquer, ids atterrit bien en 4ᵉ (accountIds). Correct.
accountIds vide = aucun filtre, byte-identique — garanti par inPlaceholders() qui retourne null pour []/undefined, donc aucune clause WHERE source_id IN (...) ajoutée. Couvert exhaustivement par reportService.test.ts (« an empty accountIds array behaves exactly like no filter » sur les deux fonctions).
SQL — accountIds passe par des placeholders paramétrés ($N) + params.push(...), jamais d'interpolation → pas d'injection.
accounts exposé — bien renvoyé par useTrends via ...state ; le chargement de la liste (getAllImportSources) vit dans le hook, pas dans la page (respecte la convention « état hors composant »).
i18n — aucune chaîne en dur ajoutée ; FilterPanel réutilise reports.filters.accounts.*, la page réutilise reports.trends.*.
Docs / CHANGELOG — docs/architecture.md (ligne useReportsPeriod) et CHANGELOG EN+FR sous [Unreleased]/Added à jour et exacts. Pas de migration DB.
Effet de fetch — ids passé en argument (pas de closure sur accountIds), garde fetchIdRef conservée → pas de course sur changement de filtre.

Observations non bloquantes

Couplage du slot error — le 2ᵉ effet (chargement des comptes) partage le slot error du reducer. Un succès du fetch Tendances (SET_TRENDS/SET_CATEGORY_OVER_TIME remet error: null) effacerait une erreur de chargement de comptes, et inversement une erreur comptes pourrait masquer transitoirement. Cohérent avec le pattern reducer existant, purement cosmétique (bannière) — non bloquant.
Tests de wiring — pas de test automatisé sur le câblage useTrends/page lui-même. Justification du PR body valide : pas de jsdom/testing-library dans le repo, et le diff n'introduit aucune logique de branchement pure à extraire (simple forwarding de paramètres vers des services déjà testés à fond). Conforme à la convention du repo.

Note de merge (pile stackée)

PR ciblant issue-274-filter-panel, pas main → check.yml ne tourne pas (statut no-status). Valider le tip cumulé en local avant de merger la pile (tsc && vite build, vitest run, cargo check, cargo test). Le PR body indique build vert + 788/788 vitest.

🤖 Revue générée via /pr-review

## /pr-review — PR #283 · `feat(reports): adopt shared FilterPanel on the Trends page` **Verdict : APPROVE** ✅ ### Résumé Branchement propre et bien scopé du `FilterPanel` partagé (#274) sur la page Tendances. Le diff head-vs-base se limite à 5 fichiers (2× CHANGELOG, `docs/architecture.md`, `useTrends.ts`, `ReportsTrendsPage.tsx`) — pas de dérive de périmètre. Chaque appel a été vérifié contre le code réel des maillons #272-#274 de la pile. ### Vérifications - **Props `FilterPanel`** — les 4 props passées (`temporalControl`, `accountIds`, `onAccountIdsChange`, `accounts`) correspondent exactement à `FilterPanelProps`, types inclus (`number[]`, `(number[]) => void`, `ImportSource[]`). - **Signatures services** — `getMonthlyTrends(from, to, ids)` : `ids` en 3ᵉ position = `accountIds`. `getCategoryOverTime(from, to, undefined, ids)` : `undefined` en 3ᵉ laisse le défaut `topN = 50` s'appliquer, `ids` atterrit bien en 4ᵉ (`accountIds`). Correct. - **`accountIds` vide = aucun filtre, byte-identique** — garanti par `inPlaceholders()` qui retourne `null` pour `[]`/`undefined`, donc aucune clause `WHERE source_id IN (...)` ajoutée. Couvert exhaustivement par `reportService.test.ts` (« an empty accountIds array behaves exactly like no filter » sur les deux fonctions). - **SQL** — `accountIds` passe par des placeholders paramétrés (`$N`) + `params.push(...)`, jamais d'interpolation → pas d'injection. - **`accounts` exposé** — bien renvoyé par `useTrends` via `...state` ; le chargement de la liste (`getAllImportSources`) vit dans le hook, pas dans la page (respecte la convention « état hors composant »). - **i18n** — aucune chaîne en dur ajoutée ; `FilterPanel` réutilise `reports.filters.accounts.*`, la page réutilise `reports.trends.*`. - **Docs / CHANGELOG** — `docs/architecture.md` (ligne `useReportsPeriod`) et CHANGELOG EN+FR sous `[Unreleased]/Added` à jour et exacts. Pas de migration DB. - **Effet de fetch** — `ids` passé en argument (pas de closure sur `accountIds`), garde `fetchIdRef` conservée → pas de course sur changement de filtre. ### Observations non bloquantes 1. **Couplage du slot `error`** — le 2ᵉ effet (chargement des comptes) partage le slot `error` du reducer. Un succès du fetch Tendances (`SET_TRENDS`/`SET_CATEGORY_OVER_TIME` remet `error: null`) effacerait une erreur de chargement de comptes, et inversement une erreur comptes pourrait masquer transitoirement. Cohérent avec le pattern reducer existant, purement cosmétique (bannière) — non bloquant. 2. **Tests de wiring** — pas de test automatisé sur le câblage `useTrends`/page lui-même. Justification du PR body valide : pas de jsdom/testing-library dans le repo, et le diff n'introduit aucune logique de branchement pure à extraire (simple forwarding de paramètres vers des services déjà testés à fond). Conforme à la convention du repo. ### Note de merge (pile stackée) PR ciblant `issue-274-filter-panel`, pas `main` → `check.yml` ne tourne pas (statut `no-status`). Valider le **tip cumulé en local** avant de merger la pile (`tsc && vite build`, `vitest run`, `cargo check`, `cargo test`). Le PR body indique build vert + **788/788** vitest. --- 🤖 Revue générée via `/pr-review`

maximus commented

2026-07-11 20:39:46 +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:46 +00:00