From 40766d662928a3e1c4ad345c31055efa1f507270 Mon Sep 17 00:00:00 2001 From: RyuseiTaniguchi Date: Wed, 2 Sep 2026 11:36:00 +0900 Subject: [PATCH] fix(reports): make the Uncategorized spending drill-down load its transactions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Clicking Uncategorized on the Spending tab opened an empty sheet reading "No transactions found." Two bugs stacked on the same value: 1. DrillDownSheet fetched from an effect keyed on the filter's individual fields. With the sheet closed the filter is null, so every field reads undefined — and an uncategorized drill-down carried categoryId undefined too, making the open state field-for-field identical to the closed one. The effect never re-ran and rows stayed empty. Every real category has a non-undefined id, which is why only Uncategorized broke. Depend on the filter object itself instead; it is fresh state on each click. 2. The null categoryId was flattened to undefined twice on the way down (report-spending and the server action). getTransactions reads null as IS NULL but undefined as "no category filter", so once the fetch did fire it returned every transaction in the range rather than the uncategorized ones. Carry the null through. The footer link had the same hole — it dropped the category param for null, linking to all transactions. Extracted as drillDownTransactionsUrl, which emits the category=uncategorized sentinel the transactions page already parses, with unit tests over the three id states. The chart's rolled-up "Other" slice also carries id null, so with the null now meaningful it would have drilled into Uncategorized. It spans several categories, which drill-down cannot express, so it is flagged synthetic and inert on click; that flag also replaces the fragile id-and-name check used to pick its neutral color. --- src/actions/reports.ts | 5 ++-- src/components/atoms/spending-chart.tsx | 16 +++++++---- src/components/organisms/drill-down-sheet.tsx | 22 +++++++++------ src/components/organisms/report-spending.tsx | 3 +- src/lib/drill-down-url.test.ts | 28 +++++++++++++++++++ src/lib/drill-down-url.ts | 27 ++++++++++++++++++ 6 files changed, 84 insertions(+), 17 deletions(-) create mode 100644 src/lib/drill-down-url.test.ts create mode 100644 src/lib/drill-down-url.ts diff --git a/src/actions/reports.ts b/src/actions/reports.ts index b7d239c9..4f716639 100644 --- a/src/actions/reports.ts +++ b/src/actions/reports.ts @@ -84,7 +84,8 @@ export async function deleteReport( const DRILL_DOWN_LIMIT = 50; export async function getDrillDownTransactions(filters: { - categoryId?: string; + /** A category id, `null` for uncategorized, `undefined` for no category filter. */ + categoryId?: string | null; dateFrom: string; dateTo: string; type?: "income" | "expense"; @@ -93,7 +94,7 @@ export async function getDrillDownTransactions(filters: { const page = await withHousehold(householdId, (tx) => getTransactions(householdId, { - categoryId: filters.categoryId ?? undefined, + categoryId: filters.categoryId, dateFrom: filters.dateFrom, dateTo: filters.dateTo, }, DRILL_DOWN_LIMIT, undefined, tx)); diff --git a/src/components/atoms/spending-chart.tsx b/src/components/atoms/spending-chart.tsx index 10bfeafe..91303cf6 100644 --- a/src/components/atoms/spending-chart.tsx +++ b/src/components/atoms/spending-chart.tsx @@ -8,6 +8,8 @@ export interface SpendingChartItem { id: string | null; name: string; value: number; + /** Set on the rolled-up "Other" slice this chart builds; not a real category. */ + synthetic?: boolean; } interface SpendingChartProps { @@ -18,11 +20,11 @@ interface SpendingChartProps { // The aggregated "Other" row (built below from categories past the top 8) is // not a real category — give it a fixed neutral color instead of cycling back -// into CHART_COLORS, which would collide with an earlier slice's color. -// Real uncategorized spend can also carry a null id under a different name, -// so key off both id and name to avoid recoloring legitimate rows. +// into CHART_COLORS, which would collide with an earlier slice's color. Real +// uncategorized spend also carries a null id, so key off the synthetic flag +// rather than the id to avoid recoloring legitimate rows. function colorAt(item: SpendingChartItem, i: number): string { - if (item.id === null && item.name === "Other") return "var(--chart-neutral)"; + if (item.synthetic) return "var(--chart-neutral)"; return CHART_COLORS[i % CHART_COLORS.length]; } @@ -40,13 +42,15 @@ export function SpendingChart({ data, viewMode, onItemClick }: SpendingChartProp const otherTotal = data.slice(8).reduce((sum, d) => sum + d.value, 0); const chartData: SpendingChartItem[] = otherTotal > 0 - ? [...top8, { id: null, name: "Other", value: otherTotal }] + ? [...top8, { id: null, name: "Other", value: otherTotal, synthetic: true }] : top8; function handleClick(index: number) { if (!onItemClick) return; const item = chartData[index]; - if (item) onItemClick({ id: item.id, name: item.name }); + // "Other" spans several categories, which drill-down can't express — a null + // id there would be read as uncategorized, so leave the slice inert. + if (item && !item.synthetic) onItemClick({ id: item.id, name: item.name }); } if (viewMode === "donut") { diff --git a/src/components/organisms/drill-down-sheet.tsx b/src/components/organisms/drill-down-sheet.tsx index 435e7b4d..82f45d75 100644 --- a/src/components/organisms/drill-down-sheet.tsx +++ b/src/components/organisms/drill-down-sheet.tsx @@ -13,11 +13,13 @@ import { import { Skeleton } from "@/components/ui/skeleton"; import { TransactionListPanel } from "@/components/molecules/transaction-list-panel"; import { getDrillDownTransactions } from "@/actions/reports"; +import { drillDownTransactionsUrl } from "@/lib/drill-down-url"; import { centsToDisplay } from "@/lib/money"; import type { TransactionRow } from "@/queries/transactions"; export interface DrillDownFilter { - categoryId?: string; + /** A category id, `null` for uncategorized, `undefined` for no category filter. */ + categoryId?: string | null; categoryName: string; month?: string; type?: "income" | "expense"; @@ -54,17 +56,21 @@ export function DrillDownSheet({ filter, dateFrom, dateTo, onClose }: DrillDownS setRows(result.rows); setHasMore(result.hasMore); }); - // eslint-disable-next-line react-hooks/exhaustive-deps -- derived values (effectiveDateFrom/To) already capture filter changes - }, [filter?.categoryId, filter?.month, filter?.type, effectiveDateFrom, effectiveDateTo]); + // Depend on the filter object itself, not its fields: an uncategorized + // drill-down carries `categoryId: null` alongside undefined month/type, which + // is field-for-field identical to the closed (null filter) state, so a field + // dependency list would never fire and the sheet would open empty. + }, [filter, effectiveDateFrom, effectiveDateTo]); const totalAmount = rows.reduce((s, r) => s + r.normalizedAmount, 0); const txnPageUrl = filter - ? `/transactions?${new URLSearchParams({ - ...(filter.categoryId ? { category: filter.categoryId } : {}), - from: filter.month ? `${filter.month}-01` : dateFrom, - to: filter.month ? `${filter.month}-31` : dateTo, - }).toString()}` + ? drillDownTransactionsUrl({ + categoryId: filter.categoryId, + month: filter.month, + dateFrom, + dateTo, + }) : "/transactions"; return ( diff --git a/src/components/organisms/report-spending.tsx b/src/components/organisms/report-spending.tsx index 6590736b..2a667ff3 100644 --- a/src/components/organisms/report-spending.tsx +++ b/src/components/organisms/report-spending.tsx @@ -48,8 +48,9 @@ export function ReportSpending({ data, comparisonLabel: compLabel }: ReportSpend ]; function handleDrillDown(item: { id: string | null; name: string }) { + // Keep the null: it means "uncategorized", not "every category". setDrillDown({ - categoryId: item.id ?? undefined, + categoryId: item.id, categoryName: item.name, tabContext: "Spending", }); diff --git a/src/lib/drill-down-url.test.ts b/src/lib/drill-down-url.test.ts new file mode 100644 index 00000000..0a7602bb --- /dev/null +++ b/src/lib/drill-down-url.test.ts @@ -0,0 +1,28 @@ +import { describe, it, expect } from "vitest"; +import { drillDownTransactionsUrl } from "./drill-down-url"; + +describe("drillDownTransactionsUrl", () => { + const range = { dateFrom: "2026-06-01", dateTo: "2026-08-31" }; + + it("passes a real category through as its id", () => { + const url = drillDownTransactionsUrl({ categoryId: "cat-groceries", ...range }); + expect(url).toBe("/transactions?category=cat-groceries&from=2026-06-01&to=2026-08-31"); + }); + + it("encodes a null category as the uncategorized sentinel", () => { + const url = drillDownTransactionsUrl({ categoryId: null, ...range }); + expect(new URLSearchParams(url.split("?")[1]).get("category")).toBe("uncategorized"); + }); + + it("omits the category param entirely when no category is filtered", () => { + const url = drillDownTransactionsUrl(range); + expect(url).toBe("/transactions?from=2026-06-01&to=2026-08-31"); + }); + + it("narrows the range to the month when drilling into one", () => { + const url = drillDownTransactionsUrl({ categoryId: null, month: "2026-07", ...range }); + const params = new URLSearchParams(url.split("?")[1]); + expect(params.get("from")).toBe("2026-07-01"); + expect(params.get("to")).toBe("2026-07-31"); + }); +}); diff --git a/src/lib/drill-down-url.ts b/src/lib/drill-down-url.ts new file mode 100644 index 00000000..fe00a528 --- /dev/null +++ b/src/lib/drill-down-url.ts @@ -0,0 +1,27 @@ +/** + * Build the "View all in Transactions" link behind a report drill-down. + * + * `categoryId` follows the query-layer convention: a string is a real category, + * `null` means "no category assigned", and `undefined` means "don't filter by + * category at all". The transactions page has no null in a URL, so it encodes + * the uncategorized case as the sentinel `category=uncategorized` (see + * `parse-transaction-filters.ts`). + */ +export function drillDownTransactionsUrl(params: { + categoryId?: string | null; + month?: string; + dateFrom: string; + dateTo: string; +}): string { + const { categoryId, month, dateFrom, dateTo } = params; + + const search = new URLSearchParams({ + ...(categoryId === undefined + ? {} + : { category: categoryId ?? "uncategorized" }), + from: month ? `${month}-01` : dateFrom, + to: month ? `${month}-31` : dateTo, + }); + + return `/transactions?${search.toString()}`; +}