feat(reports): adopt shared FilterPanel on Compare and Budget #284

Closed

maximus wants to merge 1 commit from issue-276-adopt-compare-budget into issue-275-adopt-trends

maximus commented

2026-07-11 19:52:44 +00:00

Owner

Resolves #276

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

Summary

Mounts <FilterPanel> on ReportsComparePage (temporal slot = existing CompareReferenceMonthPicker) and BudgetPage (temporal slot = existing YearNavigator) — temporal model unchanged on both pages.
useCompare now fetches accountIds from useReportsPeriod and threads it into getCompareMonthOverMonth/getCompareYearOverYear; fetches the account checkbox list via getAllImportSources (same pattern as useTrends/#275).
CompareBudgetView (the Compare page's budget sub-tab) accepts a new accountIds prop and threads it into getBudgetVsActualData — this closes the gap flagged in the issue's review caveat, where this sub-tab silently ignored the filter while the rest of the page respected it.
useBudget now fetches accountIds and its own account list the same way, and threads accountIds into every refreshData call site (initial load + all 6 post-mutation refreshes).

Decision (MEDIUM ambiguity) — Budget's data source

The issue's acceptance criterion says: "Budget : accountIds passe a getBudgetVsActualData (via useBudget)". useBudget never calls getBudgetVsActualData — that function is exclusive to CompareBudgetView, the Dashboard, and the Cartes report. useBudget's only real-transactions fetch is getActualTotalsForYear(year - 1), which populates the grid's previous-year reference column.

Decided: extend getActualTotalsForYear with the same optional accountIds?: number[] pass-through pattern #273 already established on the other 7 services (one-line change, forwards to the existing getActualsByCategoryRange helper it already calls internally). This makes the Budget page's actual reference data respect the filter, which is the evident intent, without inventing an unrelated call to a differently-shaped service (BudgetVsActualRow, single-month+YTD) that has no consumer in the Budget grid's BudgetYearRow model.

Rejected alternative: wire a literal getBudgetVsActualData call into useBudget just to match the issue text verbatim — rejected because it would fetch data in an incompatible shape unused by BudgetTable, and would require restructuring the Budget grid's rendering, which the issue explicitly keeps unchanged (temporal model unchanged; the Budget-page restructure to an income-statement grid is separate scope, tracked under epic #260/M2 per STATE.md).

Test plan

npm run build (tsc + vite) green
npm test — 788 -> 791 (3 new regression tests for getActualTotalsForYear's accountIds pass-through, mirroring the existing getBudgetVsActualData tests), zero regressions
No selection (empty accountIds) is byte-identical to pre-#276 behavior on both pages (regression-tested at the service layer; UI wiring mirrors the #275 Trends adoption exactly)

Caveat for reviewer

See the "Decision" section above re: getActualTotalsForYear vs. the issue's literal getBudgetVsActualData wording for the Budget page.

Resolves #276 Generated autonomously by /autopilot run of 2026-07-11 ## Summary - Mounts `<FilterPanel>` on `ReportsComparePage` (temporal slot = existing `CompareReferenceMonthPicker`) and `BudgetPage` (temporal slot = existing `YearNavigator`) — temporal model unchanged on both pages. - `useCompare` now fetches `accountIds` from `useReportsPeriod` and threads it into `getCompareMonthOverMonth`/`getCompareYearOverYear`; fetches the account checkbox list via `getAllImportSources` (same pattern as `useTrends`/#275). - `CompareBudgetView` (the Compare page's budget sub-tab) accepts a new `accountIds` prop and threads it into `getBudgetVsActualData` — this closes the gap flagged in the issue's review caveat, where this sub-tab silently ignored the filter while the rest of the page respected it. - `useBudget` now fetches `accountIds` and its own account list the same way, and threads `accountIds` into every `refreshData` call site (initial load + all 6 post-mutation refreshes). ## Decision (MEDIUM ambiguity) — Budget's data source The issue's acceptance criterion says: "Budget : accountIds passe a getBudgetVsActualData (via useBudget)". `useBudget` never calls `getBudgetVsActualData` — that function is exclusive to `CompareBudgetView`, the Dashboard, and the Cartes report. `useBudget`'s only real-transactions fetch is `getActualTotalsForYear(year - 1)`, which populates the grid's previous-year reference column. Decided: extend `getActualTotalsForYear` with the same optional `accountIds?: number[]` pass-through pattern `#273` already established on the other 7 services (one-line change, forwards to the existing `getActualsByCategoryRange` helper it already calls internally). This makes the Budget page's actual reference data respect the filter, which is the evident intent, without inventing an unrelated call to a differently-shaped service (`BudgetVsActualRow`, single-month+YTD) that has no consumer in the Budget grid's `BudgetYearRow` model. Rejected alternative: wire a literal `getBudgetVsActualData` call into `useBudget` just to match the issue text verbatim — rejected because it would fetch data in an incompatible shape unused by `BudgetTable`, and would require restructuring the Budget grid's rendering, which the issue explicitly keeps unchanged (temporal model unchanged; the Budget-page restructure to an income-statement grid is separate scope, tracked under epic #260/M2 per STATE.md). ## Test plan - [x] `npm run build` (tsc + vite) green - [x] `npm test` — 788 -> 791 (3 new regression tests for `getActualTotalsForYear`'s accountIds pass-through, mirroring the existing `getBudgetVsActualData` tests), zero regressions - [x] No selection (empty `accountIds`) is byte-identical to pre-#276 behavior on both pages (regression-tested at the service layer; UI wiring mirrors the #275 Trends adoption exactly) ## Caveat for reviewer See the "Decision" section above re: `getActualTotalsForYear` vs. the issue's literal `getBudgetVsActualData` wording for the Budget page.

maximus added 1 commit 2026-07-11 19:52:45 +00:00

feat(reports): adopt shared FilterPanel on Compare and Budget fe9ae0118c

Mounts <FilterPanel> on ReportsComparePage (next to the existing
CompareReferenceMonthPicker) and BudgetPage (next to the existing
YearNavigator), keeping each page's temporal control unchanged. Threads
accountIds from useReportsPeriod through useCompare into
getCompareMonthOverMonth/getCompareYearOverYear, and through
CompareBudgetView into getBudgetVsActualData — closing the gap where
that sub-tab silently ignored the filter while the rest of the Compare
page respected it.

Budget: the issue named getBudgetVsActualData as the target for
useBudget, but that hook never calls it (it's exclusive to
CompareBudgetView/Dashboard/Cartes) — its only real actuals fetch is
getActualTotalsForYear(year - 1), the previous-year reference column.
Extended that function with the same optional accountIds pass-through
established by #273, rather than wiring in an unrelated call shape.

Both hooks fetch their account checkbox list via getAllImportSources,
mirroring the useTrends/#275 pattern. Empty selection = no filter,
byte-identical to pre-#276 output (regression-tested).

Resolves #276

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

maximus added the

autopilot:pending-human

label 2026-07-11 19:52:53 +00:00

maximus commented

2026-07-11 20:31:57 +00:00

Author

Owner

`/pr-review` — PR #284 · feat(reports): adopt shared FilterPanel on Compare and Budget

Verdict : APPROVE

Adoption mécanique et fidèle du FilterPanel partagé sur les pages Comparaison et Budget, calquée sur le pattern #275 (Tendances) déjà mergé. accountIds circule correctement depuis l'état URL mémoïsé jusqu'à des effets à dépendances stables (aucune boucle de rendu), le pass-through service préserve le comportement byte-identique sans filtre, et le trou signalé dans le caveat de #275 (CompareBudgetView ignorait le filtre) est refermé.

Vérifié

Pas de boucle d'effet. accountIds est useMemo'd sur le param URL sources → référence stable ; les nouveaux useEffect([…, accountIds]) de useCompare/useBudget ne se relancent que sur un vrai changement de sélection. fetch (useCompare) et refreshData (useBudget) ont des deps [] → stables. Aucun risque de fetch infini.
Pass-through service correct. getActualTotalsForYear(year-1, ids) = une ligne vers getActualsByCategoryRange existant ; inPlaceholders(ids, 3) renvoie null pour undefined/[] → la requête sans filtre reste byte-identique. Placeholders à partir de $3 → cohérent avec les nouveaux tests (source_id IN ($3, $4), params […, 4, 9]). Seul appelant de getActualTotalsForYear = useBudget → param optionnel rétrocompatible.
Tests non cassés. useCompare.test.ts n'exerce que les helpers purs → insensible au nouvel effet getAllImportSources et au 3ᵉ argument service. useBudget n'expose pas refreshData → son changement de signature est purement interne. 3 tests de régression ajoutés (788→791), expectations internes cohérentes avec inPlaceholders.
UI/i18n. Contrat de props FilterPanel respecté sur les 2 sites d'appel ; les 2 rendus de page sont bien formés. La restructuration showActualControls (Comparaison) préserve le comportement antérieur (sélecteur de mois de référence toujours visible ; sous-mode + vue seulement en mode actual). Aucune nouvelle chaîne user-facing (réutilise les clés de #274). CHANGELOG FR+EN sous [Unreleased], architecture.md à jour.
Sécurité/data. SQL paramétré (pas d'injection), pas de secret, aucune migration touchée.

Notes non bloquantes

Portée du filtre Budget. Le FilterPanel couvre visuellement toute la grille, mais seule la colonne de référence « actuals année précédente » réagit (les montants budgétés ne sont pas rattachés à un compte). C'est la sémantique correcte et c'est clairement documenté (corps de PR + CHANGELOG) ; la refonte Budget→état de résultat est déférée à l'épic #260/M2. À revisiter en M2 : le filtre devrait-il paraître plus complet ici.
Slot d'erreur partagé. Dans les 2 hooks, un échec du chargement de la liste des sources (getAllImportSources) dispatch SET_ERROR sur le même champ error que le fetch principal → un hoquet de liste de comptes remonterait en erreur de page même si le rapport a chargé. Faible sévérité, identique au pattern #275/useTrends déjà mergé ; isolable plus tard.
Process (PR stackée). Base = issue-275-adopt-trends, pas main → check.yml n'a jamais tourné sur ce maillon ; le build vert / 791 tests = run local de l'auteur. Conformément à la guidance stacked-PR : valider le tip cumulé en local (tsc && vite build, vitest run, cargo check/test) avant de merger la pile.

La décision d'étendre getActualTotalsForYear plutôt que de câbler un appel littéral à getBudgetVsActualData est le bon choix — elle filtre la seule donnée de transactions réelles que la grille Budget consomme réellement, sans introduire un fetch inutilisé de forme incompatible.

## `/pr-review` — PR #284 · feat(reports): adopt shared FilterPanel on Compare and Budget **Verdict : APPROVE** Adoption mécanique et fidèle du `FilterPanel` partagé sur les pages Comparaison et Budget, calquée sur le pattern #275 (Tendances) déjà mergé. `accountIds` circule correctement depuis l'état URL mémoïsé jusqu'à des effets à dépendances stables (aucune boucle de rendu), le pass-through service préserve le comportement byte-identique sans filtre, et le trou signalé dans le caveat de #275 (`CompareBudgetView` ignorait le filtre) est refermé. ### Vérifié - **Pas de boucle d'effet.** `accountIds` est `useMemo`'d sur le param URL `sources` → référence stable ; les nouveaux `useEffect([…, accountIds])` de `useCompare`/`useBudget` ne se relancent que sur un vrai changement de sélection. `fetch` (useCompare) et `refreshData` (useBudget) ont des deps `[]` → stables. Aucun risque de fetch infini. - **Pass-through service correct.** `getActualTotalsForYear(year-1, ids)` = une ligne vers `getActualsByCategoryRange` existant ; `inPlaceholders(ids, 3)` renvoie `null` pour `undefined`/`[]` → la requête sans filtre reste byte-identique. Placeholders à partir de `$3` → cohérent avec les nouveaux tests (`source_id IN ($3, $4)`, params `[…, 4, 9]`). Seul appelant de `getActualTotalsForYear` = `useBudget` → param optionnel rétrocompatible. - **Tests non cassés.** `useCompare.test.ts` n'exerce que les helpers purs → insensible au nouvel effet `getAllImportSources` et au 3ᵉ argument service. `useBudget` n'expose pas `refreshData` → son changement de signature est purement interne. 3 tests de régression ajoutés (788→791), expectations internes cohérentes avec `inPlaceholders`. - **UI/i18n.** Contrat de props `FilterPanel` respecté sur les 2 sites d'appel ; les 2 rendus de page sont bien formés. La restructuration `showActualControls` (Comparaison) préserve le comportement antérieur (sélecteur de mois de référence toujours visible ; sous-mode + vue seulement en mode `actual`). Aucune nouvelle chaîne user-facing (réutilise les clés de #274). CHANGELOG FR+EN sous [Unreleased], `architecture.md` à jour. - **Sécurité/data.** SQL paramétré (pas d'injection), pas de secret, aucune migration touchée. ### Notes non bloquantes 1. **Portée du filtre Budget.** Le `FilterPanel` couvre visuellement toute la grille, mais seule la colonne de référence « actuals année précédente » réagit (les montants budgétés ne sont pas rattachés à un compte). C'est la sémantique correcte et c'est clairement documenté (corps de PR + CHANGELOG) ; la refonte Budget→état de résultat est déférée à l'épic #260/M2. À revisiter en M2 : le filtre devrait-il paraître plus complet ici. 2. **Slot d'erreur partagé.** Dans les 2 hooks, un échec du chargement de la liste des sources (`getAllImportSources`) dispatch `SET_ERROR` sur le même champ `error` que le fetch principal → un hoquet de liste de comptes remonterait en erreur de page même si le rapport a chargé. Faible sévérité, identique au pattern #275/`useTrends` déjà mergé ; isolable plus tard. 3. **Process (PR stackée).** Base = `issue-275-adopt-trends`, pas `main` → `check.yml` n'a jamais tourné sur ce maillon ; le build vert / 791 tests = run local de l'auteur. Conformément à la guidance stacked-PR : valider le **tip cumulé en local** (`tsc && vite build`, `vitest run`, `cargo check/test`) avant de merger la pile. La décision d'étendre `getActualTotalsForYear` plutôt que de câbler un appel littéral à `getBudgetVsActualData` est le bon choix — elle filtre la seule donnée de transactions réelles que la grille Budget consomme réellement, sans introduire un fetch inutilisé de forme incompatible.

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