From d099678ddf88240ef84d8a2a66ec331bf8a1745b Mon Sep 17 00:00:00 2001 From: escouade-bot Date: Wed, 18 Mar 2026 02:02:41 -0400 Subject: [PATCH] fix: address reviewer feedback (#41) - Default categoryType filter to "expense" so top-N ranking is not skewed by mixing income and expense amounts - Remove redundant showFilterPanel condition (hasCategories already covers the overTime tab) - Add unit tests for getCategoryOverTime query construction and output Co-Authored-By: Claude Opus 4.6 (1M context) --- src/hooks/useReports.ts | 2 +- src/pages/ReportsPage.tsx | 2 +- src/services/reportService.test.ts | 146 +++++++++++++++++++++++++++++ 3 files changed, 148 insertions(+), 2 deletions(-) create mode 100644 src/services/reportService.test.ts diff --git a/src/hooks/useReports.ts b/src/hooks/useReports.ts index a5ec0b5..5abaa64 100644 --- a/src/hooks/useReports.ts +++ b/src/hooks/useReports.ts @@ -61,7 +61,7 @@ const initialState: ReportsState = { customDateFrom: monthStartStr, customDateTo: todayStr, sourceId: null, - categoryType: null, + categoryType: "expense", monthlyTrends: [], categorySpending: [], categoryOverTime: { categories: [], data: [], colors: {}, categoryIds: {} }, diff --git a/src/pages/ReportsPage.tsx b/src/pages/ReportsPage.tsx index 1e34ebc..da4dc90 100644 --- a/src/pages/ReportsPage.tsx +++ b/src/pages/ReportsPage.tsx @@ -69,7 +69,7 @@ export default function ReportsPage() { const monthOptions = useMemo(() => buildMonthOptions(i18n.language), [i18n.language]); const hasCategories = ["byCategory", "overTime"].includes(state.tab) && filterCategories.length > 0; - const showFilterPanel = hasCategories || state.tab === "overTime" || (state.tab === "trends" && sources.length > 1); + const showFilterPanel = hasCategories || (state.tab === "trends" && sources.length > 1); return (
diff --git a/src/services/reportService.test.ts b/src/services/reportService.test.ts new file mode 100644 index 0000000..5b5fa23 --- /dev/null +++ b/src/services/reportService.test.ts @@ -0,0 +1,146 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { getCategoryOverTime } from "./reportService"; + +// Mock the db module +vi.mock("./db", () => ({ + getDb: vi.fn(), +})); + +import { getDb } from "./db"; + +const mockSelect = vi.fn(); +const mockDb = { select: mockSelect }; + +beforeEach(() => { + vi.mocked(getDb).mockResolvedValue(mockDb as never); + mockSelect.mockReset(); +}); + +describe("getCategoryOverTime", () => { + it("builds query without WHERE clause when no filters are provided", async () => { + // First call: top categories, second call: monthly breakdown + mockSelect + .mockResolvedValueOnce([]) // topCategories + .mockResolvedValueOnce([]); // monthlyRows + + await getCategoryOverTime(); + + const topCatSQL = mockSelect.mock.calls[0][0] as string; + // No WHERE clause should appear (no filters at all) + expect(topCatSQL).not.toContain("WHERE"); + }); + + it("applies typeFilter via category type when provided", async () => { + mockSelect + .mockResolvedValueOnce([]) // topCategories + .mockResolvedValueOnce([]); // monthlyRows + + await getCategoryOverTime(undefined, undefined, 50, undefined, "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')"); + expect(topCatSQL).toContain("WHERE"); + expect(topCatParams[0]).toBe("expense"); + }); + + it("applies income typeFilter correctly", async () => { + mockSelect + .mockResolvedValueOnce([]) // topCategories + .mockResolvedValueOnce([]); // monthlyRows + + await getCategoryOverTime("2025-01-01", "2025-06-30", 50, undefined, "income"); + + 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(topCatParams[0]).toBe("income"); + // Date params follow + expect(topCatParams[1]).toBe("2025-01-01"); + expect(topCatParams[2]).toBe("2025-06-30"); + }); + + it("applies date range filters", async () => { + mockSelect + .mockResolvedValueOnce([]) // topCategories + .mockResolvedValueOnce([]); // monthlyRows + + await getCategoryOverTime("2025-01-01", "2025-12-31"); + + const topCatSQL = mockSelect.mock.calls[0][0] as string; + const topCatParams = mockSelect.mock.calls[0][1] as unknown[]; + expect(topCatSQL).toContain("t.date >= $1"); + expect(topCatSQL).toContain("t.date <= $2"); + expect(topCatParams).toEqual(["2025-01-01", "2025-12-31", 50]); + }); + + it("applies sourceId filter", async () => { + mockSelect + .mockResolvedValueOnce([]) // topCategories + .mockResolvedValueOnce([]); // monthlyRows + + 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(topCatParams[0]).toBe(3); + }); + + it("combines typeFilter, date range, and sourceId", 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(topCatParams).toEqual(["expense", "2025-01-01", "2025-06-30", 2, 10]); + }); + + it("returns correct structure with categories, data, colors, and categoryIds", async () => { + mockSelect + .mockResolvedValueOnce([ + { category_id: 1, category_name: "Food", category_color: "#ff0000", total: 500 }, + { category_id: 2, category_name: "Transport", category_color: "#00ff00", total: 200 }, + ]) + .mockResolvedValueOnce([ + { month: "2025-01", category_id: 1, category_name: "Food", total: 300 }, + { month: "2025-01", category_id: 2, category_name: "Transport", total: 100 }, + { month: "2025-02", category_id: 1, category_name: "Food", total: 200 }, + { month: "2025-02", category_id: 2, category_name: "Transport", total: 100 }, + ]); + + const result = await getCategoryOverTime("2025-01-01", "2025-02-28", 50, undefined, "expense"); + + expect(result.categories).toEqual(["Food", "Transport"]); + expect(result.colors).toEqual({ Food: "#ff0000", Transport: "#00ff00" }); + expect(result.categoryIds).toEqual({ Food: 1, Transport: 2 }); + expect(result.data).toHaveLength(2); + expect(result.data[0]).toEqual({ month: "2025-01", Food: 300, Transport: 100 }); + expect(result.data[1]).toEqual({ month: "2025-02", Food: 200, Transport: 100 }); + }); + + it("groups non-top-N categories into Other", async () => { + mockSelect + .mockResolvedValueOnce([ + { category_id: 1, category_name: "Food", category_color: "#ff0000", total: 500 }, + ]) + .mockResolvedValueOnce([ + { month: "2025-01", category_id: 1, category_name: "Food", total: 300 }, + { month: "2025-01", category_id: 2, category_name: "Transport", total: 100 }, + { month: "2025-01", category_id: 3, category_name: "Entertainment", total: 50 }, + ]); + + const result = await getCategoryOverTime("2025-01-01", "2025-01-31", 1, undefined, "expense"); + + expect(result.categories).toEqual(["Food", "Other"]); + expect(result.colors["Other"]).toBe("#9ca3af"); + expect(result.data[0]).toEqual({ month: "2025-01", Food: 300, Other: 150 }); + }); +});