From 4a93c60ea7b539327ecf51ae32f903037d702b57 Mon Sep 17 00:00:00 2001 From: le king fu Date: Sat, 11 Jul 2026 15:15:37 -0400 Subject: [PATCH] feat(reports): plumb accountIds[] account filter into 7 report services MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 and wire it into Trends/Compare/Budget). Resolves #273 Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.fr.md | 1 + CHANGELOG.md | 1 + src/services/budgetService.test.ts | 84 +++++++++++ src/services/budgetService.ts | 25 +++- src/services/dashboardService.test.ts | 65 +++++++++ src/services/dashboardService.ts | 12 +- src/services/reportService.cartes.test.ts | 46 ++++++ src/services/reportService.test.ts | 169 +++++++++++++++++++++- src/services/reportService.ts | 86 +++++++---- src/utils/sqlFilters.test.ts | 27 ++++ src/utils/sqlFilters.ts | 16 ++ 11 files changed, 487 insertions(+), 45 deletions(-) create mode 100644 src/services/budgetService.test.ts create mode 100644 src/services/dashboardService.test.ts create mode 100644 src/utils/sqlFilters.test.ts create mode 100644 src/utils/sqlFilters.ts diff --git a/CHANGELOG.fr.md b/CHANGELOG.fr.md index be402ba..59f869f 100644 --- a/CHANGELOG.fr.md +++ b/CHANGELOG.fr.md @@ -7,6 +7,7 @@ - Rapports → Comparaison : les deux rapports comparables hiérarchiques (réel vs réel et réel vs budget) permettent désormais de **replier ou déplier les sous-catégories de chaque catégorie parente**. Un chevron sur chaque catégorie de premier niveau masque son détail tout en gardant sa ligne de sous-total visible, et un bouton « Tout déplier / Tout replier » les bascule ensemble. Les groupes s'ouvrent **repliés** pour un aperçu compact au niveau des sous-totaux ; ce que vous dépliez est mémorisé par rapport (#254). - Rapports → Tendances → par catégorie (vue tableau) : le tableau d'analyse de résultat est désormais **hiérarchique et repliable**, à l'image du rapport de comparaison. Les catégories parentes apparaissent en groupes indentés au-dessus de leurs sous-catégories, chacune avec son propre sous-total, et un chevron (plus un bouton « Tout déplier / Tout replier ») replie un groupe jusqu'à ce seul sous-total. Les groupes s'ouvrent **repliés**, et vos choix sont mémorisés séparément des tableaux comparables. La ligne **Résultat avant transferts** se place maintenant entre les sections de dépenses et la section des transferts au lieu du bas, et replier un groupe ne déplace aucun sous-total ni résultat — les chiffres sont toujours calculés sur l'ensemble des données (#265). - Rapports : pose les fondations d'un futur filtre de comptes pour les rapports — le hook de période partagé suit désormais aussi une sélection multi-comptes via l'URL (mémorisable comme la plage de dates, et validée contre les URL malformées ou modifiées à la main). Purement interne pour l'instant : aucun contrôle de filtre visible encore, le sélecteur de comptes et le branchement par rapport arrivent dans des issues de suivi (#272). +- Rapports : les sept services de rapports (tendances, catégories dans le temps, comparaison réel-vs-réel, budget-vs-réel, dépenses par catégorie, et le tableau de bord Cartes) acceptent désormais un filtre multi-comptes optionnel, comparé à une liste paramétrée de sources d'import. Plomberie interne seulement — omettre le filtre partout donne des résultats identiques au byte près, et aucune page de rapport n'expose encore de contrôle de filtre ; cela arrive dans des issues de suivi (#273). ### Corrigé diff --git a/CHANGELOG.md b/CHANGELOG.md index 9e96d05..203c3af 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ - Reports → Compare: the two hierarchical comparable reports (real-vs-real and real-vs-budget) now let you **collapse or expand each parent category's sub-categories**. A chevron on every top-level category folds its breakdown away while keeping the category's subtotal row in view, and an "Expand all / Collapse all" button toggles them together. Groups start **collapsed** so the report opens on a compact, subtotal-level overview; whatever you expand is remembered per report (#254). - Reports → Trends → by category (table view): the income-statement table is now **hierarchical and collapsible**, matching the compare report. Parent categories appear as indented groups above their sub-categories, each with its own subtotal, and a chevron (plus an "Expand all / Collapse all" button) folds a group down to just that subtotal. Groups start **collapsed**, and your choices are remembered separately from the comparable tables. The **Result before transfers** line now sits between the expense sections and the transfers section instead of at the bottom, and folding a group never moves any subtotal or result — the figures are always computed from the full data (#265). - Reports: laid the groundwork for an upcoming account filter across the report pages — the shared period hook now also tracks a URL-backed, multi-account selection (bookmarkable like the date range, and validated against malformed/hand-edited URLs). Internal only for now: no filter control is visible yet, the account picker and per-report wiring land in follow-up issues (#272). +- Reports: all seven report services (trends, by-category-over-time, real-vs-real compare, budget-vs-actual, expenses-by-category, and the Cartes dashboard) now accept an optional multi-account filter, matched against a parameterized list of import sources. Backend plumbing only — leaving every filter out still returns byte-identical results, and no report page exposes a filter control yet; that lands in follow-up issues (#273). ### Fixed diff --git a/src/services/budgetService.test.ts b/src/services/budgetService.test.ts new file mode 100644 index 0000000..178ff43 --- /dev/null +++ b/src/services/budgetService.test.ts @@ -0,0 +1,84 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { getBudgetVsActualData } from "./budgetService"; + +vi.mock("./db", () => { + const getDb = vi.fn(); + return { getDb }; +}); + +import { getDb } from "./db"; + +const mockSelect = vi.fn(); +const mockDb = { select: mockSelect }; + +beforeEach(() => { + vi.mocked(getDb).mockResolvedValue(mockDb as never); + mockSelect.mockReset(); +}); + +// getBudgetVsActualData fans out to 4 parallel selects (getAllActiveCategories, +// getBudgetEntriesForYear, and getActualsByCategoryRange for the month + YTD +// windows). Only the two `getActualsByCategoryRange` calls query `transactions` +// and are eligible for the accountIds filter — budgets are not tied to an +// account, so the categories/entries queries must never carry the clause. +describe("getBudgetVsActualData — accountIds filter (Issue #273)", () => { + it("without accountIds, the actuals queries carry no source_id clause (regression)", async () => { + mockSelect.mockImplementation(() => Promise.resolve([])); + + await getBudgetVsActualData(2026, 3); + + const actualsCalls = mockSelect.mock.calls.filter(([sql]) => + (sql as string).includes("FROM transactions"), + ); + expect(actualsCalls.length).toBe(2); // month + YTD + for (const [sql, params] of actualsCalls) { + expect(sql as string).not.toContain("source_id"); + expect(params as unknown[]).toHaveLength(2); // dateFrom, dateTo only + } + }); + + it("an empty accountIds array behaves exactly like no filter (regression)", async () => { + mockSelect.mockImplementation(() => Promise.resolve([])); + + await getBudgetVsActualData(2026, 3, []); + + const actualsCalls = mockSelect.mock.calls.filter(([sql]) => + (sql as string).includes("FROM transactions"), + ); + for (const [sql, params] of actualsCalls) { + expect(sql as string).not.toContain("source_id"); + expect(params as unknown[]).toHaveLength(2); + } + }); + + it("applies a parameterized accountIds IN clause to both the month and YTD actuals queries", async () => { + mockSelect.mockImplementation(() => Promise.resolve([])); + + await getBudgetVsActualData(2026, 3, [4, 9]); + + const actualsCalls = mockSelect.mock.calls.filter(([sql]) => + (sql as string).includes("FROM transactions"), + ); + expect(actualsCalls.length).toBe(2); + for (const [sql, params] of actualsCalls) { + expect(sql as string).toContain("source_id IN ($3, $4)"); + expect(sql as string).not.toContain("4,9"); + expect(params as unknown[]).toEqual(expect.arrayContaining([4, 9])); + } + }); + + it("never adds the filter to the categories/budget-entries queries (a budget is not tied to an account)", async () => { + mockSelect.mockImplementation(() => Promise.resolve([])); + + await getBudgetVsActualData(2026, 3, [4]); + + const categoriesCall = mockSelect.mock.calls.find(([sql]) => + (sql as string).includes("FROM categories"), + )!; + const budgetEntriesCall = mockSelect.mock.calls.find(([sql]) => + (sql as string).includes("FROM budget_entries"), + )!; + expect(categoriesCall[0]).not.toContain("source_id"); + expect(budgetEntriesCall[0]).not.toContain("source_id"); + }); +}); diff --git a/src/services/budgetService.ts b/src/services/budgetService.ts index 8b2bdb3..8954f6f 100644 --- a/src/services/budgetService.ts +++ b/src/services/budgetService.ts @@ -1,4 +1,5 @@ import { getDb } from "./db"; +import { inPlaceholders } from "../utils/sqlFilters"; import type { Category, BudgetEntry, @@ -192,23 +193,35 @@ export async function getActualTotalsForYear( async function getActualsByCategoryRange( dateFrom: string, - dateTo: string + dateTo: string, + accountIds?: number[] ): Promise> { const db = await getDb(); + const params: unknown[] = [dateFrom, dateTo]; + const accountPlaceholders = inPlaceholders(accountIds, params.length + 1); + const accountFilter = accountPlaceholders ? ` AND source_id IN (${accountPlaceholders})` : ""; + if (accountPlaceholders) params.push(...accountIds!); return db.select>( `SELECT category_id, COALESCE(SUM(amount), 0) AS actual FROM transactions - WHERE date BETWEEN $1 AND $2 + WHERE date BETWEEN $1 AND $2${accountFilter} GROUP BY category_id`, - [dateFrom, dateTo] + params ); } const TYPE_ORDER: Record = { expense: 0, income: 1, transfer: 2 }; +/** + * Budget vs actual, by category, for the reference month plus YTD. `accountIds` + * (Issue #273) optionally scopes both the month and YTD actuals to a subset of + * import sources (`transactions.source_id`) — budgeted amounts are unaffected + * by the filter (a budget is not tied to any account). + */ export async function getBudgetVsActualData( year: number, - month: number + month: number, + accountIds?: number[] ): Promise { // Date ranges const { dateFrom: monthFrom, dateTo: monthTo } = computeMonthDateRange(year, month); @@ -219,8 +232,8 @@ export async function getBudgetVsActualData( const [allCategories, yearEntries, monthActuals, ytdActuals] = await Promise.all([ getAllActiveCategories(), getBudgetEntriesForYear(year), - getActualsByCategoryRange(monthFrom, monthTo), - getActualsByCategoryRange(ytdFrom, ytdTo), + getActualsByCategoryRange(monthFrom, monthTo, accountIds), + getActualsByCategoryRange(ytdFrom, ytdTo, accountIds), ]); // Build maps diff --git a/src/services/dashboardService.test.ts b/src/services/dashboardService.test.ts new file mode 100644 index 0000000..e00060b --- /dev/null +++ b/src/services/dashboardService.test.ts @@ -0,0 +1,65 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { getExpensesByCategory } from "./dashboardService"; + +vi.mock("./db", () => { + const getDb = vi.fn(); + return { getDb }; +}); + +import { getDb } from "./db"; + +const mockSelect = vi.fn(); +const mockDb = { select: mockSelect }; + +beforeEach(() => { + vi.mocked(getDb).mockResolvedValue(mockDb as never); + mockSelect.mockReset(); +}); + +describe("getExpensesByCategory — accountIds filter (Issue #273)", () => { + it("without accountIds, the query carries no source_id clause (regression)", async () => { + mockSelect.mockResolvedValueOnce([]); + + await getExpensesByCategory("2025-01-01", "2025-12-31"); + + const sql = mockSelect.mock.calls[0][0] as string; + const params = mockSelect.mock.calls[0][1] as unknown[]; + expect(sql).not.toContain("source_id"); + expect(params).toEqual(["2025-01-01", "2025-12-31"]); + }); + + it("an empty accountIds array behaves exactly like no filter (regression)", async () => { + mockSelect.mockResolvedValueOnce([]); + + await getExpensesByCategory("2025-01-01", "2025-12-31", []); + + const sql = mockSelect.mock.calls[0][0] as string; + const params = mockSelect.mock.calls[0][1] as unknown[]; + expect(sql).not.toContain("source_id"); + expect(params).toEqual(["2025-01-01", "2025-12-31"]); + }); + + it("applies a parameterized accountIds IN clause, one placeholder per id", async () => { + mockSelect.mockResolvedValueOnce([]); + + await getExpensesByCategory("2025-01-01", "2025-12-31", [6, 11]); + + const sql = mockSelect.mock.calls[0][0] as string; + const params = mockSelect.mock.calls[0][1] as unknown[]; + expect(sql).toContain("t.source_id IN ($3, $4)"); + expect(sql).not.toContain("6,11"); + expect(params).toEqual(["2025-01-01", "2025-12-31", 6, 11]); + }); + + it("applies the accountIds filter alone (no dates) alongside the fixed expense-type clause", async () => { + mockSelect.mockResolvedValueOnce([]); + + await getExpensesByCategory(undefined, undefined, [2]); + + const sql = mockSelect.mock.calls[0][0] as string; + const params = mockSelect.mock.calls[0][1] as unknown[]; + expect(sql).toContain("t.source_id IN ($1)"); + expect(sql).toContain("COALESCE(c.type, 'expense') = 'expense'"); + expect(params).toEqual([2]); + }); +}); diff --git a/src/services/dashboardService.ts b/src/services/dashboardService.ts index 8fed529..41f45ca 100644 --- a/src/services/dashboardService.ts +++ b/src/services/dashboardService.ts @@ -1,4 +1,5 @@ import { getDb } from "./db"; +import { inPlaceholders } from "../utils/sqlFilters"; import type { DashboardSummary, CategoryBreakdownItem, @@ -54,7 +55,7 @@ export async function getDashboardSummary( export async function getExpensesByCategory( dateFrom?: string, dateTo?: string, - sourceId?: number, + accountIds?: number[], ): Promise { const db = await getDb(); @@ -72,10 +73,11 @@ export async function getExpensesByCategory( params.push(dateTo); paramIndex++; } - if (sourceId != null) { - whereClauses.push(`t.source_id = $${paramIndex}`); - params.push(sourceId); - paramIndex++; + const accountPlaceholders = inPlaceholders(accountIds, paramIndex); + if (accountPlaceholders) { + whereClauses.push(`t.source_id IN (${accountPlaceholders})`); + params.push(...accountIds!); + paramIndex += accountIds!.length; } const whereSQL = `WHERE ${whereClauses.join(" AND ")}`; diff --git a/src/services/reportService.cartes.test.ts b/src/services/reportService.cartes.test.ts index ab39758..6e32db2 100644 --- a/src/services/reportService.cartes.test.ts +++ b/src/services/reportService.cartes.test.ts @@ -469,4 +469,50 @@ describe("getCartesSnapshot", () => { expect(worst.overrunAbs).toBe(150); expect(worst.overrunPct).toBeCloseTo(75, 5); }); + + // --- accountIds filter (Issue #273) --- + // + // getCartesSnapshot must forward accountIds to its two sub-reports that + // support it (getCompareMonthOverMonth for top movers, getBudgetVsActualData + // for budget adherence) — the review caveat that flagged this: omitting it + // would let this dashboard silently ignore an active account filter even + // though its own building blocks respect it. + + it("forwards accountIds to the compare (top movers) sub-report", async () => { + mockSelect.mockImplementation(() => Promise.resolve([])); + + await getCartesSnapshot(2026, 3, "month", [3, 8]); + + const momCall = mockSelect.mock.calls.find(([sql]) => + (sql as string).includes("ORDER BY ABS(month_current_total - month_previous_total) DESC"), + )!; + expect(momCall[0]).toContain("AND t.source_id IN ($9, $10)"); + expect(momCall[1]).toEqual(expect.arrayContaining([3, 8])); + }); + + it("forwards accountIds to the budget-vs-actual (budget adherence) sub-report", async () => { + mockSelect.mockImplementation(() => Promise.resolve([])); + + await getCartesSnapshot(2026, 3, "month", [3, 8]); + + const actualsCalls = mockSelect.mock.calls.filter(([sql]) => + (sql as string).includes("FROM transactions\n WHERE date BETWEEN"), + ); + // Both the month and YTD actuals queries (getBudgetVsActualData) receive it. + expect(actualsCalls.length).toBeGreaterThanOrEqual(2); + for (const [sql, params] of actualsCalls) { + expect(sql as string).toContain("source_id IN ($3, $4)"); + expect(params as unknown[]).toEqual(expect.arrayContaining([3, 8])); + } + }); + + it("without accountIds, no sub-report SQL carries a source_id clause (regression)", async () => { + mockSelect.mockImplementation(() => Promise.resolve([])); + + await getCartesSnapshot(2026, 3); + + for (const [sql] of mockSelect.mock.calls) { + expect(sql as string).not.toContain("source_id"); + } + }); }); diff --git a/src/services/reportService.test.ts b/src/services/reportService.test.ts index 7258375..a35735d 100644 --- a/src/services/reportService.test.ts +++ b/src/services/reportService.test.ts @@ -1,5 +1,6 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; import { + getMonthlyTrends, getCategoryOverTime, getHighlights, getCompareMonthOverMonth, @@ -29,6 +30,66 @@ beforeEach(() => { mockSelect.mockReset(); }); +describe("getMonthlyTrends", () => { + it("builds query without WHERE clause when no filters are provided (regression: byte-identical to pre-#273)", async () => { + mockSelect.mockResolvedValueOnce([]); + + await getMonthlyTrends(); + + const sql = mockSelect.mock.calls[0][0] as string; + const params = mockSelect.mock.calls[0][1] as unknown[]; + expect(sql).not.toContain("WHERE"); + expect(params).toEqual([]); + }); + + it("applies date range filters without an accountIds argument (regression)", async () => { + mockSelect.mockResolvedValueOnce([]); + + await getMonthlyTrends("2025-01-01", "2025-12-31"); + + const sql = mockSelect.mock.calls[0][0] as string; + const params = mockSelect.mock.calls[0][1] as unknown[]; + expect(sql).toContain("date >= $1"); + expect(sql).toContain("date <= $2"); + expect(sql).not.toContain("source_id"); + expect(params).toEqual(["2025-01-01", "2025-12-31"]); + }); + + it("an empty accountIds array behaves exactly like no filter", async () => { + mockSelect.mockResolvedValueOnce([]); + + await getMonthlyTrends("2025-01-01", "2025-12-31", []); + + const sql = mockSelect.mock.calls[0][0] as string; + const params = mockSelect.mock.calls[0][1] as unknown[]; + expect(sql).not.toContain("source_id"); + expect(params).toEqual(["2025-01-01", "2025-12-31"]); + }); + + it("applies a parameterized accountIds IN clause, one placeholder per id", async () => { + mockSelect.mockResolvedValueOnce([]); + + await getMonthlyTrends("2025-01-01", "2025-12-31", [3, 7]); + + const sql = mockSelect.mock.calls[0][0] as string; + const params = mockSelect.mock.calls[0][1] as unknown[]; + expect(sql).toContain("source_id IN ($3, $4)"); + expect(sql).not.toContain("3,7"); + expect(params).toEqual(["2025-01-01", "2025-12-31", 3, 7]); + }); + + it("applies the accountIds filter alone (no dates)", async () => { + mockSelect.mockResolvedValueOnce([]); + + await getMonthlyTrends(undefined, undefined, [5]); + + const sql = mockSelect.mock.calls[0][0] as string; + const params = mockSelect.mock.calls[0][1] as unknown[]; + expect(sql).toContain("WHERE source_id IN ($1)"); + expect(params).toEqual([5]); + }); +}); + describe("getCategoryOverTime", () => { it("builds query without WHERE clause when no filters are provided", async () => { // First call: top categories, second call: monthly breakdown @@ -87,32 +148,60 @@ describe("getCategoryOverTime", () => { expect(topCatParams).toEqual(["2025-01-01", "2025-12-31", 50]); }); - it("applies sourceId filter", async () => { + it("applies accountIds filter (single id)", async () => { mockSelect .mockResolvedValueOnce([]) // topCategories .mockResolvedValueOnce([]); // monthlyRows - await getCategoryOverTime(undefined, undefined, 50, 3); + await getCategoryOverTime(undefined, undefined, 50, [3]); const topCatSQL = mockSelect.mock.calls[0][0] as string; const topCatParams = mockSelect.mock.calls[0][1] as unknown[]; - expect(topCatSQL).toContain("t.source_id"); + expect(topCatSQL).toContain("t.source_id IN ($1)"); expect(topCatParams[0]).toBe(3); }); - it("combines typeFilter, date range, and sourceId", async () => { + it("applies accountIds filter with one bound placeholder per id (never joined into the SQL string)", async () => { mockSelect .mockResolvedValueOnce([]) // topCategories .mockResolvedValueOnce([]); // monthlyRows - await getCategoryOverTime("2025-01-01", "2025-06-30", 10, 2, "expense"); + await getCategoryOverTime(undefined, undefined, 50, [3, 7, 12]); + + const topCatSQL = mockSelect.mock.calls[0][0] as string; + const topCatParams = mockSelect.mock.calls[0][1] as unknown[]; + expect(topCatSQL).toContain("t.source_id IN ($1, $2, $3)"); + expect(topCatSQL).not.toContain("3,7,12"); + // topN ($4) follows the 3 account placeholders. + expect(topCatParams).toEqual([3, 7, 12, 50]); + }); + + it("an empty accountIds array behaves exactly like no filter (no clause, no orphan param)", async () => { + mockSelect + .mockResolvedValueOnce([]) // topCategories + .mockResolvedValueOnce([]); // monthlyRows + + await getCategoryOverTime("2025-01-01", "2025-12-31", 50, []); + + const topCatSQL = mockSelect.mock.calls[0][0] as string; + const topCatParams = mockSelect.mock.calls[0][1] as unknown[]; + expect(topCatSQL).not.toContain("source_id"); + expect(topCatParams).toEqual(["2025-01-01", "2025-12-31", 50]); + }); + + it("combines typeFilter, date range, and accountIds", async () => { + mockSelect + .mockResolvedValueOnce([]) // topCategories + .mockResolvedValueOnce([]); // monthlyRows + + await getCategoryOverTime("2025-01-01", "2025-06-30", 10, [2], "expense"); const topCatSQL = mockSelect.mock.calls[0][0] as string; const topCatParams = mockSelect.mock.calls[0][1] as unknown[]; expect(topCatSQL).toContain("COALESCE(c.type, 'expense') = $1"); expect(topCatSQL).toContain("t.date >= $2"); expect(topCatSQL).toContain("t.date <= $3"); - expect(topCatSQL).toContain("t.source_id = $4"); + expect(topCatSQL).toContain("t.source_id IN ($4)"); expect(topCatParams).toEqual(["expense", "2025-01-01", "2025-06-30", 2, 10]); }); @@ -492,6 +581,46 @@ describe("getCompareMonthOverMonth", () => { expect(result[0].deltaAbs).toBe(50); expect(result[0].cumulativeDeltaAbs).toBe(80); }); + + it("without accountIds, the delta SQL/params stay byte-identical to pre-#273 (regression)", async () => { + mockSelect.mockResolvedValueOnce([]); + + await getCompareMonthOverMonth(2026, 4); + + const sql = mockSelect.mock.calls[0][0] as string; + const params = mockSelect.mock.calls[0][1] as unknown[]; + expect(sql).not.toContain("source_id"); + expect(params).toHaveLength(8); + }); + + it("an empty accountIds array behaves exactly like no filter (regression)", async () => { + mockSelect.mockResolvedValueOnce([]); + + await getCompareMonthOverMonth(2026, 4, []); + + const sql = mockSelect.mock.calls[0][0] as string; + const params = mockSelect.mock.calls[0][1] as unknown[]; + expect(sql).not.toContain("source_id"); + expect(params).toHaveLength(8); + }); + + it("applies a parameterized accountIds IN clause starting at $9 (after the 8 date bounds)", async () => { + mockSelect.mockResolvedValueOnce([]); + + await getCompareMonthOverMonth(2026, 4, [3, 7]); + + const sql = mockSelect.mock.calls[0][0] as string; + const params = mockSelect.mock.calls[0][1] as unknown[]; + expect(sql).toContain("AND t.source_id IN ($9, $10)"); + expect(sql).not.toContain("3,7"); + expect(params).toEqual([ + "2026-04-01", "2026-04-30", + "2026-03-01", "2026-03-31", + "2026-01-01", "2026-04-30", + "2026-01-01", "2026-03-31", + 3, 7, + ]); + }); }); describe("getCompareYearOverYear", () => { @@ -549,6 +678,34 @@ describe("getCompareYearOverYear", () => { expect(result[0].deltaPct).toBeCloseTo(50, 4); expect(result[0].cumulativeDeltaPct).toBeCloseTo((900 / 1600) * 100, 4); }); + + it("without accountIds, the delta SQL/params stay byte-identical to pre-#273 (regression)", async () => { + mockSelect.mockResolvedValueOnce([]); + + await getCompareYearOverYear(2026, 4); + + const sql = mockSelect.mock.calls[0][0] as string; + const params = mockSelect.mock.calls[0][1] as unknown[]; + expect(sql).not.toContain("source_id"); + expect(params).toHaveLength(8); + }); + + it("applies a parameterized accountIds IN clause starting at $9 (after the 8 date bounds)", async () => { + mockSelect.mockResolvedValueOnce([]); + + await getCompareYearOverYear(2026, 4, [9]); + + const sql = mockSelect.mock.calls[0][0] as string; + const params = mockSelect.mock.calls[0][1] as unknown[]; + expect(sql).toContain("AND t.source_id IN ($9)"); + expect(params).toEqual([ + "2026-04-01", "2026-04-30", + "2025-04-01", "2025-04-30", + "2026-01-01", "2026-04-30", + "2025-01-01", "2025-04-30", + 9, + ]); + }); }); describe("real-vs-real compare — transfer netting (Issue #243)", () => { diff --git a/src/services/reportService.ts b/src/services/reportService.ts index 93355fc..7acebc7 100644 --- a/src/services/reportService.ts +++ b/src/services/reportService.ts @@ -1,5 +1,6 @@ import { getDb } from "./db"; import { getBudgetVsActualData } from "./budgetService"; +import { inPlaceholders } from "../utils/sqlFilters"; import type { MonthlyTrendItem, CategoryBreakdownItem, @@ -29,7 +30,7 @@ import type { export async function getMonthlyTrends( dateFrom?: string, dateTo?: string, - sourceId?: number, + accountIds?: number[], ): Promise { const db = await getDb(); @@ -47,10 +48,11 @@ export async function getMonthlyTrends( params.push(dateTo); paramIndex++; } - if (sourceId != null) { - whereClauses.push(`source_id = $${paramIndex}`); - params.push(sourceId); - paramIndex++; + const accountPlaceholders = inPlaceholders(accountIds, paramIndex); + if (accountPlaceholders) { + whereClauses.push(`source_id IN (${accountPlaceholders})`); + params.push(...accountIds!); + paramIndex += accountIds!.length; } const whereSQL = @@ -73,7 +75,7 @@ export async function getCategoryOverTime( dateFrom?: string, dateTo?: string, topN: number = 50, - sourceId?: number, + accountIds?: number[], typeFilter?: "expense" | "income" | "transfer", ): Promise { const db = await getDb(); @@ -98,10 +100,11 @@ export async function getCategoryOverTime( params.push(dateTo); paramIndex++; } - if (sourceId != null) { - whereClauses.push(`t.source_id = $${paramIndex}`); - params.push(sourceId); - paramIndex++; + const accountPlaceholders = inPlaceholders(accountIds, paramIndex); + if (accountPlaceholders) { + whereClauses.push(`t.source_id IN (${accountPlaceholders})`); + params.push(...accountIds!); + paramIndex += accountIds!.length; } const whereSQL = whereClauses.length > 0 ? `WHERE ${whereClauses.join(" AND ")}` : ""; @@ -883,8 +886,16 @@ function previousMonth(year: number, month: number): { year: number; month: numb * rows still contribute only their outflows, byte-identical to before. The two * result lines (before/after transfers) are computed in ComparePeriodTable from * the per-type subtotals, not here. Uncategorized rows (c.type NULL) → 'expense'. + * + * `accountIds` (Issue #273) adds an optional `t.source_id IN (...)` clause — + * one bound placeholder per id, starting at $9 (the 8 date bounds occupy + * $1-$8). Omitted/empty → no clause at all, so the query text and results stay + * byte-identical to the pre-#273 behavior. */ -const COMPARE_DELTA_SQL = `SELECT +function compareDeltaSql(accountIds?: number[]): string { + const accountPlaceholders = inPlaceholders(accountIds, 9); + const accountFilter = accountPlaceholders ? ` AND t.source_id IN (${accountPlaceholders})` : ""; + return `SELECT t.category_id, COALESCE(c.name, 'Uncategorized') AS category_name, COALESCE(c.color, '#9ca3af') AS category_color, @@ -900,20 +911,23 @@ const COMPARE_DELTA_SQL = `SELECT OR (t.date >= $3 AND t.date <= $4) OR (t.date >= $5 AND t.date <= $6) OR (t.date >= $7 AND t.date <= $8) - ) + )${accountFilter} GROUP BY t.category_id, category_name, category_color ORDER BY ABS(month_current_total - month_previous_total) DESC`; +} /** * Month-over-month expense delta by category. Returns both a monthly view * (reference month vs immediately-previous month) and a cumulative YTD view * (Jan→refMonth of refYear vs Jan→prevMonth of refYear — i.e. "cumulative * progress through end of last month" vs "cumulative progress through end of - * this month"). All SQL parameterised. + * this month"). All SQL parameterised. `accountIds` (Issue #273) optionally + * scopes the whole report to a subset of import sources (`transactions.source_id`). */ export async function getCompareMonthOverMonth( year: number, month: number, + accountIds?: number[], ): Promise { const db = await getDb(); const { start: curStart, end: curEnd } = monthBoundaries(year, month); @@ -931,16 +945,19 @@ export async function getCompareMonthOverMonth( const cumPreviousStart = `${prev.year}-01-01`; const cumPreviousEnd = prevEnd; + const deltaParams: unknown[] = [ + curStart, curEnd, + prevStart, prevEnd, + cumCurrentStart, cumCurrentEnd, + cumPreviousStart, cumPreviousEnd, + ]; + if (accountIds && accountIds.length > 0) deltaParams.push(...accountIds); + // Delta select stays first so its params/SQL remain `mock.calls[0]`; the // category metadata (for the hierarchy) is fetched alongside. `?? []` guards // under-specified mocks — db.select never returns undefined in production. const [rows, cats] = await Promise.all([ - db.select(COMPARE_DELTA_SQL, [ - curStart, curEnd, - prevStart, prevEnd, - cumCurrentStart, cumCurrentEnd, - cumPreviousStart, cumPreviousEnd, - ]), + db.select(compareDeltaSql(accountIds), deltaParams), db.select(COMPARE_CATEGORIES_SQL), ]); return buildCompareTree(rowsToDeltas(rows), cats ?? []); @@ -952,11 +969,13 @@ export async function getCompareMonthOverMonth( * view (Jan→refMonth of refYear vs Jan→refMonth of refYear - 1). Uses the * reference year's December as the "current month" when no explicit * reference month is provided; callers typically pass the user's chosen - * reference month. All SQL parameterised. + * reference month. All SQL parameterised. `accountIds` (Issue #273) optionally + * scopes the whole report to a subset of import sources (`transactions.source_id`). */ export async function getCompareYearOverYear( year: number, month: number = 12, + accountIds?: number[], ): Promise { const db = await getDb(); const { start: curMonthStart, end: curMonthEnd } = monthBoundaries(year, month); @@ -967,14 +986,17 @@ export async function getCompareYearOverYear( const cumPreviousStart = `${year - 1}-01-01`; const cumPreviousEnd = prevMonthEnd; + const deltaParams: unknown[] = [ + curMonthStart, curMonthEnd, + prevMonthStart, prevMonthEnd, + cumCurrentStart, cumCurrentEnd, + cumPreviousStart, cumPreviousEnd, + ]; + if (accountIds && accountIds.length > 0) deltaParams.push(...accountIds); + // See getCompareMonthOverMonth: delta select first, categories alongside. const [rows, cats] = await Promise.all([ - db.select(COMPARE_DELTA_SQL, [ - curMonthStart, curMonthEnd, - prevMonthStart, prevMonthEnd, - cumCurrentStart, cumCurrentEnd, - cumPreviousStart, cumPreviousEnd, - ]), + db.select(compareDeltaSql(accountIds), deltaParams), db.select(COMPARE_CATEGORIES_SQL), ]); return buildCompareTree(rowsToDeltas(rows), cats ?? []); @@ -1217,11 +1239,19 @@ async function fetchSeasonality( * via the monthly series instead of re-querying. * 4. Budget vs actual for the reference month. * 5. Seasonality: same calendar month across the two prior years. + * + * `accountIds` (Issue #273) is forwarded to the two sub-reports that already + * support it (`getCompareMonthOverMonth` for top movers, `getBudgetVsActualData` + * for budget adherence) — omitting it here would let this dashboard silently + * ignore an active account filter even though its building blocks respect it. + * The KPI/sparkline/seasonality series (`fetchMonthlyFlows`, `fetchSeasonality`) + * do not take an account filter yet; that is out of scope for this issue. */ export async function getCartesSnapshot( referenceYear: number, referenceMonth: number, mode: CartesKpiPeriodMode = "month", + accountIds?: number[], ): Promise { // Date window: 25 months back from the reference to cover YoY + a 13-month // sparkline. Start = 24 months before ref = (ref - 24 months) = month offset -24. @@ -1233,8 +1263,8 @@ export async function getCartesSnapshot( const [seasonalityRows, flowRows, momRows, budgetRows] = await Promise.all([ fetchSeasonality(referenceMonth, referenceYear - 2, referenceYear - 1), fetchMonthlyFlows(windowStartIso, refEnd), - getCompareMonthOverMonth(referenceYear, referenceMonth), - getBudgetVsActualData(referenceYear, referenceMonth), + getCompareMonthOverMonth(referenceYear, referenceMonth, accountIds), + getBudgetVsActualData(referenceYear, referenceMonth, accountIds), ]); // Index the flow rows by month for O(1) lookup, then fill missing months diff --git a/src/utils/sqlFilters.test.ts b/src/utils/sqlFilters.test.ts new file mode 100644 index 0000000..d8b18ec --- /dev/null +++ b/src/utils/sqlFilters.test.ts @@ -0,0 +1,27 @@ +import { describe, it, expect } from "vitest"; +import { inPlaceholders } from "./sqlFilters"; + +describe("inPlaceholders", () => { + it("returns null for undefined ids (no filter)", () => { + expect(inPlaceholders(undefined, 1)).toBeNull(); + }); + + it("returns null for an empty array (no filter)", () => { + expect(inPlaceholders([], 1)).toBeNull(); + }); + + it("builds a single placeholder for one id", () => { + expect(inPlaceholders([7], 1)).toBe("$1"); + }); + + it("builds one placeholder per id, starting at startIndex", () => { + expect(inPlaceholders([3, 7, 12], 4)).toBe("$4, $5, $6"); + }); + + it("never joins the raw id values into the string (CWE-89 guard)", () => { + const result = inPlaceholders([3, 7], 1); + expect(result).not.toContain("3"); + expect(result).not.toContain("7"); + expect(result).toBe("$1, $2"); + }); +}); diff --git a/src/utils/sqlFilters.ts b/src/utils/sqlFilters.ts new file mode 100644 index 0000000..ecc3b1c --- /dev/null +++ b/src/utils/sqlFilters.ts @@ -0,0 +1,16 @@ +/** + * Shared helper for building a parameterized SQL `IN (...)` clause — one + * bound placeholder per id, never a joined/interpolated value list (CWE-89). + * First introduced for the report services' optional account + * (`transactions.source_id`) filter (Issue #273); reused by every service + * that accepts an `accountIds?: number[]` filter so the placeholder + * bookkeeping (start index, one `$N` per id) lives in exactly one place. + * + * Returns `null` when `ids` is empty/undefined so callers can skip adding the + * clause entirely — that is the "no filter" case, and the query must stay + * byte-identical to how it read before the filter existed. + */ +export function inPlaceholders(ids: number[] | undefined, startIndex: number): string | null { + if (!ids || ids.length === 0) return null; + return ids.map((_, i) => `$${startIndex + i}`).join(", "); +}