fix(reports): thread accountIds into Cartes KPI + seasonality series
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) <noreply@anthropic.com>
This commit is contained in:
parent
193ef1d9dd
commit
9ee5ad353f
2 changed files with 58 additions and 17 deletions
|
|
@ -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([]));
|
||||
|
||||
|
|
|
|||
|
|
@ -1186,18 +1186,25 @@ interface RawMonthFlow {
|
|||
async function fetchMonthlyFlows(
|
||||
dateFrom: string,
|
||||
dateTo: string,
|
||||
accountIds?: number[],
|
||||
): Promise<RawMonthFlow[]> {
|
||||
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<RawMonthFlow[]>(
|
||||
`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<RawSeasonalityRow[]> {
|
||||
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<RawSeasonalityRow[]>(
|
||||
`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),
|
||||
]);
|
||||
|
|
|
|||
Loading…
Reference in a new issue