feat(reports): plumb accountIds[] account filter into 7 report services #281

Closed

maximus wants to merge 2 commits from issue-273-account-filter-services into issue-272-report-filters-hook

maximus commented

2026-07-11 19:15:57 +00:00

Owner

Resolves #273

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

Summary

Gives every report service an optional accountIds?: number[] filter that narrows results to a subset of import sources (transactions.source_id), via a parameterized IN (...) clause — one bound placeholder per id.
New param: getCompareMonthOverMonth, getCompareYearOverYear, getBudgetVsActualData, getCartesSnapshot (forwarded to its getCompareMonthOverMonth + getBudgetVsActualData sub-calls so the Cartes dashboard's top-movers and budget-adherence cards don't silently ignore an active filter).
Signature change scalar→plural: getMonthlyTrends, getCategoryOverTime, getExpensesByCategory (sourceId?: number → accountIds?: number[]). No production caller passed a scalar sourceId today, so only test call-sites needed updating.
New shared inPlaceholders() helper (src/utils/sqlFilters.ts) generates the placeholder list so the “one $N per id” bookkeeping lives in exactly one place — this is the first IN ($n, $n+1, ...) pattern in the codebase.
Backend plumbing only: no report page exposes a filter control yet (follow-up issues #274-#276 add the shared <FilterPanel> and wire it into Trends/Compare/Budget).

SQL-safety note

Every IN (...) clause is built from bound placeholders ($9, $10, …) with the ids pushed into the params array — never accountIds.join(',') interpolated into SQL text (CWE-89). The clause is only added when accountIds.length > 0; omitted/empty accountIds produces the exact same SQL as before this PR, pinned by a regression test per service.

Test plan

npm run build (tsc + vite) green
npm test green: 745 → 773 (28 new tests: sqlFilters.test.ts new, budgetService.test.ts new, dashboardService.test.ts new, extended reportService.test.ts / reportService.cartes.test.ts)
Regression test per changed service proving omitted/empty accountIds is byte-identical to pre-#273 behavior
Grepped all call-sites of the 3 signature-changed services (production + tests); no orphan scalar caller remains

Resolves #273 Generated autonomously by /autopilot run of 2026-07-11 ## Summary - Gives every report service an optional `accountIds?: number[]` filter that narrows results to a subset of import sources (`transactions.source_id`), via a **parameterized** `IN (...)` clause — one bound placeholder per id. - **New param**: `getCompareMonthOverMonth`, `getCompareYearOverYear`, `getBudgetVsActualData`, `getCartesSnapshot` (forwarded to its `getCompareMonthOverMonth` + `getBudgetVsActualData` sub-calls so the Cartes dashboard's top-movers and budget-adherence cards don't silently ignore an active filter). - **Signature change scalar→plural**: `getMonthlyTrends`, `getCategoryOverTime`, `getExpensesByCategory` (`sourceId?: number` → `accountIds?: number[]`). No production caller passed a scalar `sourceId` today, so only test call-sites needed updating. - New shared `inPlaceholders()` helper (`src/utils/sqlFilters.ts`) generates the placeholder list so the “one `$N` per id” bookkeeping lives in exactly one place — this is the first `IN ($n, $n+1, ...)` pattern in the codebase. - Backend plumbing only: no report page exposes a filter control yet (follow-up issues #274-#276 add the shared `<FilterPanel>` and wire it into Trends/Compare/Budget). ## SQL-safety note Every `IN (...)` clause is built from bound placeholders (`$9`, `$10`, …) with the ids pushed into the params array — never `accountIds.join(',')` interpolated into SQL text (CWE-89). The clause is only added when `accountIds.length > 0`; omitted/empty `accountIds` produces the exact same SQL as before this PR, pinned by a regression test per service. ## Test plan - [x] `npm run build` (tsc + vite) green - [x] `npm test` green: 745 → 773 (28 new tests: `sqlFilters.test.ts` new, `budgetService.test.ts` new, `dashboardService.test.ts` new, extended `reportService.test.ts` / `reportService.cartes.test.ts`) - [x] Regression test per changed service proving omitted/empty `accountIds` is byte-identical to pre-#273 behavior - [x] Grepped all call-sites of the 3 signature-changed services (production + tests); no orphan scalar caller remains

maximus added 1 commit 2026-07-11 19:15:58 +00:00

feat(reports): plumb accountIds[] account filter into 7 report services 4a93c60ea7

Give every report service an optional accountIds?: number[] filter that
narrows results to a subset of import sources (transactions.source_id),
matched via a parameterized IN (...) clause — one bound placeholder per
id, never a joined string (CWE-89). New shared inPlaceholders() helper
(src/utils/sqlFilters.ts) generates the placeholder list so the pattern
lives in exactly one place.

New accountIds param: getCompareMonthOverMonth, getCompareYearOverYear,
getBudgetVsActualData, getCartesSnapshot (forwarded to its
getCompareMonthOverMonth + getBudgetVsActualData sub-calls, so the
Cartes dashboard's top-movers and budget-adherence cards respect an
active filter instead of silently ignoring it).

Signature change scalar -> plural: getMonthlyTrends, getCategoryOverTime,
getExpensesByCategory (sourceId?: number -> accountIds?: number[]). No
scalar production caller of these three passed sourceId today, so only
the test call-sites needed updating to the array shape.

Omitted/empty accountIds adds no clause at all, so every service stays
byte-identical to its pre-#273 SQL/results — pinned by a regression test
per service (new budgetService.test.ts / dashboardService.test.ts files,
extended reportService.test.ts / reportService.cartes.test.ts).

This is backend plumbing only: no report page exposes an account filter
control yet (follow-up issues #274-#276 add the shared <FilterPanel> and
wire it into Trends/Compare/Budget).

Resolves #273

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

maximus added the

autopilot:pending-human

label 2026-07-11 19:16:16 +00:00

maximus added 1 commit 2026-07-11 19:17:50 +00:00

docs(reports): fix stale COMPARE_DELTA_SQL comment references b40381fb89

The constant became the compareDeltaSql() function in the previous
commit (Issue #273); three comments still named the old constant.

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

maximus commented

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

Author

Owner

Revue `/pr-review` — APPROVE

Résumé : Plomberie backend propre et sûre. Chaque service de rapport reçoit un filtre accountIds?: number[] optionnel via un IN (...) entièrement paramétré (un placeholder lié par id), factorisé dans un helper partagé inPlaceholders(). Aucun appelant de production ne passe encore le filtre, et l'omettre laisse le SQL byte-identique au comportement pré-#273.

Vérifications

Sécurité (CWE-89) — Aucune injection possible. Toutes les clauses IN (...) sont bâties à partir de placeholders liés ($9, $10, …), les ids poussés dans le tableau de params ; jamais de accountIds.join(',') interpolé dans le texte SQL. Les seuls .join du diff sont " AND " (assemblage de clauses) et ", " sur des chaînes $N déjà sûres. sqlFilters.test.ts asserte explicitement que les valeurs brutes n'apparaissent jamais dans la chaîne.

Correction

Indexation des placeholders correcte partout : getActualsByCategoryRange démarre à $3 (params.length + 1), compareDeltaSql() à $9 (après les 8 bornes de date), les autres suivent paramIndex de façon cohérente.
getCategoryOverTime : le filtre atteint les deux requêtes (top-catégories ET détail mensuel) via un whereSQL/params partagé — pas d'incohérence entre les deux vues. Le LIMIT $paramIndex reste aligné après les placeholders de comptes.
Compare réel/réel + YoY : deltaParams ne pousse accountIds que si non vide ; l'omission garde SQL/params byte-identiques (régression : 8 params attendus).
getCartesSnapshot forwarde bien accountIds à ses deux sous-rapports qui le supportent (getCompareMonthOverMonth + getBudgetVsActualData) — le caveat de review d'origine (dashboard ignorant silencieusement un filtre actif) est couvert par test.
Signature scalaire→pluriel (getMonthlyTrends, getCategoryOverTime, getExpensesByCategory) : aucun appelant de production ne passait sourceId. Vérifié sur main ET sur la branche head — tous les call-sites (useTrends, useDashboard, useCompare, useCartes, CompareBudgetView) restent à 2-3 args. Rien ne casse.

Tests — 28 nouveaux tests ; régression par service (omis/vide → pas de clause source_id, nombre de params inchangé) ; cas single / multi / vide / undefined couverts. Aucun skip/only. sqlFilters.test.ts, budgetService.test.ts, dashboardService.test.ts nouveaux ; reportService.test.ts / .cartes.test.ts étendus.

Data / conventions — Aucune migration, aucun changement de schéma. CHANGELOG FR + EN présents. Commits conventionnels (feat(reports): + docs(reports):, ce dernier corrigeant les références de commentaire COMPARE_DELTA_SQL → compareDeltaSql() — bonne hygiène).

Suggestions (non bloquantes)

Scoping partiel de getCartesSnapshot — top-movers et budget-adherence respectent le filtre, mais les séries KPI/sparkline/seasonality (fetchMonthlyFlows, fetchSeasonality) non. Explicitement documenté hors-scope, et sans UI de filtre encore branchée il n'y a aucune incohérence visible. À surveiller quand le contrôle de filtre débarquera (#274-#276) : le dashboard Cartes montrerait alors un scoping mixte selon la carte.
Deux idiomes de garde coexistent — if (accountPlaceholders) params.push(...) (budget / compareDeltaSql) vs if (accountIds && accountIds.length > 0) … (fonctions compare). Équivalents et corrects, juste légèrement inconsistants. Trivial.

Note pile : #281 cible issue-272-report-filters-hook, donc check.yml (déclenché uniquement vers main) ne l'a pas validée. Rejouer build + vitest + cargo sur le tip cumulé de la pile avant merge.

## Revue `/pr-review` — APPROVE **Résumé** : Plomberie backend propre et sûre. Chaque service de rapport reçoit un filtre `accountIds?: number[]` optionnel via un `IN (...)` entièrement paramétré (un placeholder lié par id), factorisé dans un helper partagé `inPlaceholders()`. Aucun appelant de production ne passe encore le filtre, et l'omettre laisse le SQL byte-identique au comportement pré-#273. ### Vérifications **Sécurité (CWE-89)** — Aucune injection possible. Toutes les clauses `IN (...)` sont bâties à partir de placeholders liés (`$9, $10, …`), les ids poussés dans le tableau de params ; jamais de `accountIds.join(',')` interpolé dans le texte SQL. Les seuls `.join` du diff sont `" AND "` (assemblage de clauses) et `", "` sur des chaînes `$N` déjà sûres. `sqlFilters.test.ts` asserte explicitement que les valeurs brutes n'apparaissent jamais dans la chaîne. **Correction** - Indexation des placeholders correcte partout : `getActualsByCategoryRange` démarre à `$3` (`params.length + 1`), `compareDeltaSql()` à `$9` (après les 8 bornes de date), les autres suivent `paramIndex` de façon cohérente. - `getCategoryOverTime` : le filtre atteint **les deux** requêtes (top-catégories ET détail mensuel) via un `whereSQL`/`params` partagé — pas d'incohérence entre les deux vues. Le `LIMIT $paramIndex` reste aligné après les placeholders de comptes. - Compare réel/réel + YoY : `deltaParams` ne pousse `accountIds` que si non vide ; l'omission garde SQL/params byte-identiques (régression : 8 params attendus). - `getCartesSnapshot` forwarde bien `accountIds` à ses deux sous-rapports qui le supportent (`getCompareMonthOverMonth` + `getBudgetVsActualData`) — le caveat de review d'origine (dashboard ignorant silencieusement un filtre actif) est couvert par test. - Signature scalaire→pluriel (`getMonthlyTrends`, `getCategoryOverTime`, `getExpensesByCategory`) : **aucun appelant de production** ne passait `sourceId`. Vérifié sur `main` ET sur la branche head — tous les call-sites (`useTrends`, `useDashboard`, `useCompare`, `useCartes`, `CompareBudgetView`) restent à 2-3 args. Rien ne casse. **Tests** — 28 nouveaux tests ; régression par service (omis/vide → pas de clause `source_id`, nombre de params inchangé) ; cas single / multi / vide / undefined couverts. Aucun `skip`/`only`. `sqlFilters.test.ts`, `budgetService.test.ts`, `dashboardService.test.ts` nouveaux ; `reportService.test.ts` / `.cartes.test.ts` étendus. **Data / conventions** — Aucune migration, aucun changement de schéma. CHANGELOG FR + EN présents. Commits conventionnels (`feat(reports):` + `docs(reports):`, ce dernier corrigeant les références de commentaire `COMPARE_DELTA_SQL` → `compareDeltaSql()` — bonne hygiène). ### Suggestions (non bloquantes) 1. **Scoping partiel de `getCartesSnapshot`** — top-movers et budget-adherence respectent le filtre, mais les séries KPI/sparkline/seasonality (`fetchMonthlyFlows`, `fetchSeasonality`) non. Explicitement documenté hors-scope, et sans UI de filtre encore branchée il n'y a aucune incohérence visible. À surveiller quand le contrôle de filtre débarquera (#274-#276) : le dashboard Cartes montrerait alors un scoping mixte selon la carte. 2. **Deux idiomes de garde coexistent** — `if (accountPlaceholders) params.push(...)` (budget / `compareDeltaSql`) vs `if (accountIds && accountIds.length > 0) …` (fonctions compare). Équivalents et corrects, juste légèrement inconsistants. Trivial. --- _Note pile : #281 cible `issue-272-report-filters-hook`, donc `check.yml` (déclenché uniquement vers `main`) ne l'a pas validée. Rejouer build + vitest + cargo sur le tip cumulé de la pile avant 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