From 9ee5ad353f8feccd861ab76136a413164f7d332a Mon Sep 17 00:00:00 2001 From: le king fu Date: Sat, 11 Jul 2026 21:16:09 -0400 Subject: [PATCH] fix(reports): thread accountIds into Cartes KPI + seasonality series MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit getCartesSnapshot forwarded the account filter only to the top-movers and budget sub-reports, leaving fetchMonthlyFlows (KPIs, sparklines, 12-month overlay) and fetchSeasonality unfiltered. The Dashboard is the first page to expose the account filter over these series, so its KPI cards showed unfiltered totals while the category bars / trend / top-movers respected the filter — figures that did not reconcile on the same screen. Thread an optional accountIds through both fetchers (parameterized `source_id IN (...)` via inPlaceholders) and pass it from getCartesSnapshot, so the whole dashboard honours the filter — which is what the #279 CHANGELOG entry already promises. No CHANGELOG change: the code now matches the note. Resolves #279 Co-Authored-By: Claude Opus 4.8 (1M context) --- src/services/reportService.cartes.test.ts | 37 +++++++++++++++++++--- src/services/reportService.ts | 38 ++++++++++++++++------- 2 files changed, 58 insertions(+), 17 deletions(-) diff --git a/src/services/reportService.cartes.test.ts b/src/services/reportService.cartes.test.ts index 6e32db2..d6196da 100644 --- a/src/services/reportService.cartes.test.ts +++ b/src/services/reportService.cartes.test.ts @@ -470,13 +470,14 @@ describe("getCartesSnapshot", () => { expect(worst.overrunPct).toBeCloseTo(75, 5); }); - // --- accountIds filter (Issue #273) --- + // --- accountIds filter (Issues #273 / #279) --- // - // getCartesSnapshot must forward accountIds to its two sub-reports that + // getCartesSnapshot must forward accountIds to EVERY sub-query so the whole + // dashboard honours an active account filter: the 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. + // for budget adherence) AND the KPI/sparkline/seasonality series + // (fetchMonthlyFlows, fetchSeasonality). Omitting it anywhere would let the + // dashboard silently mix filtered and unfiltered figures on the same screen. it("forwards accountIds to the compare (top movers) sub-report", async () => { mockSelect.mockImplementation(() => Promise.resolve([])); @@ -506,6 +507,32 @@ describe("getCartesSnapshot", () => { } }); + it("forwards accountIds to the monthly-flows series (KPIs, sparklines, overlay)", async () => { + mockSelect.mockImplementation(() => Promise.resolve([])); + + await getCartesSnapshot(2026, 3, "month", [3, 8]); + + // fetchMonthlyFlows is the only snapshot query selecting a "%Y-%m" month. + const flowCall = mockSelect.mock.calls.find(([sql]) => + (sql as string).includes("strftime('%Y-%m', date) AS month"), + )!; + expect(flowCall[0]).toContain("AND source_id IN ($3, $4)"); + expect(flowCall[1]).toEqual(expect.arrayContaining([3, 8])); + }); + + it("forwards accountIds to the seasonality series", async () => { + mockSelect.mockImplementation(() => Promise.resolve([])); + + await getCartesSnapshot(2026, 3, "month", [3, 8]); + + // fetchSeasonality is the only snapshot query filtering on a "%m" month. + const seasonalityCall = mockSelect.mock.calls.find(([sql]) => + (sql as string).includes("strftime('%m', date) = $1"), + )!; + expect(seasonalityCall[0]).toContain("AND source_id IN ($4, $5)"); + expect(seasonalityCall[1]).toEqual(expect.arrayContaining([3, 8])); + }); + it("without accountIds, no sub-report SQL carries a source_id clause (regression)", async () => { mockSelect.mockImplementation(() => Promise.resolve([])); diff --git a/src/services/reportService.ts b/src/services/reportService.ts index 67dcc19..3b72a17 100644 --- a/src/services/reportService.ts +++ b/src/services/reportService.ts @@ -1186,18 +1186,25 @@ interface RawMonthFlow { async function fetchMonthlyFlows( dateFrom: string, dateTo: string, + accountIds?: number[], ): Promise { const db = await getDb(); + const params: unknown[] = [dateFrom, dateTo]; + const accountPlaceholders = inPlaceholders(accountIds, 3); + const accountClause = accountPlaceholders + ? ` AND source_id IN (${accountPlaceholders})` + : ""; + if (accountPlaceholders) params.push(...accountIds!); return db.select( `SELECT strftime('%Y-%m', date) AS month, COALESCE(SUM(CASE WHEN amount > 0 THEN amount ELSE 0 END), 0) AS income, ABS(COALESCE(SUM(CASE WHEN amount < 0 THEN amount ELSE 0 END), 0)) AS expenses FROM transactions - WHERE date >= $1 AND date <= $2 + WHERE date >= $1 AND date <= $2${accountClause} GROUP BY month ORDER BY month ASC`, - [dateFrom, dateTo], + params, ); } @@ -1210,9 +1217,16 @@ async function fetchSeasonality( month: number, yearFrom: number, yearTo: number, + accountIds?: number[], ): Promise { const db = await getDb(); const mm = String(month).padStart(2, "0"); + const params: unknown[] = [mm, yearFrom, yearTo]; + const accountPlaceholders = inPlaceholders(accountIds, 4); + const accountClause = accountPlaceholders + ? ` AND source_id IN (${accountPlaceholders})` + : ""; + if (accountPlaceholders) params.push(...accountIds!); return db.select( `SELECT CAST(strftime('%Y', date) AS INTEGER) AS year, @@ -1220,10 +1234,10 @@ async function fetchSeasonality( FROM transactions WHERE strftime('%m', date) = $1 AND CAST(strftime('%Y', date) AS INTEGER) >= $2 - AND CAST(strftime('%Y', date) AS INTEGER) <= $3 + AND CAST(strftime('%Y', date) AS INTEGER) <= $3${accountClause} GROUP BY year ORDER BY year DESC`, - [mm, yearFrom, yearTo], + params, ); } @@ -1240,12 +1254,12 @@ async function fetchSeasonality( * 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. + * `accountIds` (Issues #273/#279) is forwarded to every sub-query so the whole + * dashboard honours an active account filter: the top-movers + * (`getCompareMonthOverMonth`) and budget (`getBudgetVsActualData`) sub-reports, + * and the KPI/sparkline/seasonality series (`fetchMonthlyFlows`, + * `fetchSeasonality`). Omitting it anywhere would let the dashboard silently mix + * filtered and unfiltered figures on the same screen. */ export async function getCartesSnapshot( referenceYear: number, @@ -1261,8 +1275,8 @@ export async function getCartesSnapshot( // Seasonality range: previous 2 years for the same calendar month. const [seasonalityRows, flowRows, momRows, budgetRows] = await Promise.all([ - fetchSeasonality(referenceMonth, referenceYear - 2, referenceYear - 1), - fetchMonthlyFlows(windowStartIso, refEnd), + fetchSeasonality(referenceMonth, referenceYear - 2, referenceYear - 1, accountIds), + fetchMonthlyFlows(windowStartIso, refEnd, accountIds), getCompareMonthOverMonth(referenceYear, referenceMonth, accountIds), getBudgetVsActualData(referenceYear, referenceMonth, accountIds), ]);