From 65815ef2e067c6ded7437150be1413f86ead2de3 Mon Sep 17 00:00:00 2001 From: medic-bot Date: Tue, 10 Mar 2026 18:53:31 -0400 Subject: [PATCH] fix: address reviewer feedback (#31) - Remove non-null assertions on bYear/bMonth in useDashboard fetchData by making parameters required - Extract shared computeDateRange and buildMonthOptions into src/utils/dateRange.ts to eliminate duplication across useDashboard, useReports, DashboardPage and ReportsPage Co-Authored-By: Claude Opus 4.6 --- src/hooks/useDashboard.ts | 55 +++++-------------------------- src/hooks/useReports.ts | 45 +------------------------ src/pages/DashboardPage.tsx | 43 ++---------------------- src/pages/ReportsPage.tsx | 43 ++---------------------- src/utils/dateRange.ts | 66 +++++++++++++++++++++++++++++++++++++ 5 files changed, 82 insertions(+), 170 deletions(-) create mode 100644 src/utils/dateRange.ts diff --git a/src/hooks/useDashboard.ts b/src/hooks/useDashboard.ts index f74759c..1611d98 100644 --- a/src/hooks/useDashboard.ts +++ b/src/hooks/useDashboard.ts @@ -12,6 +12,7 @@ import { } from "../services/dashboardService"; import { getCategoryOverTime } from "../services/reportService"; import { getBudgetVsActualData } from "../services/budgetService"; +import { computeDateRange } from "../utils/dateRange"; interface DashboardState { summary: DashboardSummary; @@ -87,55 +88,17 @@ function reducer(state: DashboardState, action: DashboardAction): DashboardState } } -function computeDateRange( - period: DashboardPeriod, - customDateFrom?: string, - customDateTo?: string, -): { dateFrom?: string; dateTo?: string } { - if (period === "all") return {}; - if (period === "custom" && customDateFrom && customDateTo) { - return { dateFrom: customDateFrom, dateTo: customDateTo }; - } - - const now = new Date(); - const year = now.getFullYear(); - const month = now.getMonth(); - const day = now.getDate(); - - const dateTo = `${year}-${String(month + 1).padStart(2, "0")}-${String(day).padStart(2, "0")}`; - - let from: Date; - switch (period) { - case "month": - from = new Date(year, month, 1); - break; - case "3months": - from = new Date(year, month - 2, 1); - break; - case "6months": - from = new Date(year, month - 5, 1); - break; - case "year": - from = new Date(year, 0, 1); - break; - case "12months": - from = new Date(year, month - 11, 1); - break; - default: - from = new Date(year, month, 1); - break; - } - - const dateFrom = `${from.getFullYear()}-${String(from.getMonth() + 1).padStart(2, "0")}-${String(from.getDate()).padStart(2, "0")}`; - - return { dateFrom, dateTo }; -} - export function useDashboard() { const [state, dispatch] = useReducer(reducer, initialState); const fetchIdRef = useRef(0); - const fetchData = useCallback(async (period: DashboardPeriod, customFrom?: string, customTo?: string, bYear?: number, bMonth?: number) => { + const fetchData = useCallback(async ( + period: DashboardPeriod, + customFrom: string | undefined, + customTo: string | undefined, + bYear: number, + bMonth: number, + ) => { const fetchId = ++fetchIdRef.current; dispatch({ type: "SET_LOADING", payload: true }); dispatch({ type: "SET_ERROR", payload: null }); @@ -146,7 +109,7 @@ export function useDashboard() { getDashboardSummary(dateFrom, dateTo), getExpensesByCategory(dateFrom, dateTo), getCategoryOverTime(dateFrom, dateTo), - getBudgetVsActualData(bYear!, bMonth!), + getBudgetVsActualData(bYear, bMonth), ]); if (fetchId !== fetchIdRef.current) return; diff --git a/src/hooks/useReports.ts b/src/hooks/useReports.ts index 0cbfc5a..b13f101 100644 --- a/src/hooks/useReports.ts +++ b/src/hooks/useReports.ts @@ -12,6 +12,7 @@ import type { import { getMonthlyTrends, getCategoryOverTime, getDynamicReportData } from "../services/reportService"; import { getExpensesByCategory } from "../services/dashboardService"; import { getBudgetVsActualData } from "../services/budgetService"; +import { computeDateRange } from "../utils/dateRange"; interface ReportsState { tab: ReportTab; @@ -101,50 +102,6 @@ function reducer(state: ReportsState, action: ReportsAction): ReportsState { } } -function computeDateRange( - period: DashboardPeriod, - customDateFrom?: string, - customDateTo?: string, -): { dateFrom?: string; dateTo?: string } { - if (period === "all") return {}; - if (period === "custom" && customDateFrom && customDateTo) { - return { dateFrom: customDateFrom, dateTo: customDateTo }; - } - - const now = new Date(); - const year = now.getFullYear(); - const month = now.getMonth(); - const day = now.getDate(); - - const dateTo = `${year}-${String(month + 1).padStart(2, "0")}-${String(day).padStart(2, "0")}`; - - let from: Date; - switch (period) { - case "month": - from = new Date(year, month, 1); - break; - case "3months": - from = new Date(year, month - 2, 1); - break; - case "6months": - from = new Date(year, month - 5, 1); - break; - case "year": - from = new Date(year, 0, 1); - break; - case "12months": - from = new Date(year, month - 11, 1); - break; - default: - from = new Date(year, month, 1); - break; - } - - const dateFrom = `${from.getFullYear()}-${String(from.getMonth() + 1).padStart(2, "0")}-${String(from.getDate()).padStart(2, "0")}`; - - return { dateFrom, dateTo }; -} - export function useReports() { const [state, dispatch] = useReducer(reducer, initialState); const fetchIdRef = useRef(0); diff --git a/src/pages/DashboardPage.tsx b/src/pages/DashboardPage.tsx index 6b556c0..e5aae8c 100644 --- a/src/pages/DashboardPage.tsx +++ b/src/pages/DashboardPage.tsx @@ -8,37 +8,11 @@ import CategoryPieChart from "../components/dashboard/CategoryPieChart"; import CategoryOverTimeChart from "../components/reports/CategoryOverTimeChart"; import BudgetVsActualTable from "../components/reports/BudgetVsActualTable"; import TransactionDetailModal from "../components/shared/TransactionDetailModal"; -import type { CategoryBreakdownItem, DashboardPeriod } from "../shared/types"; +import type { CategoryBreakdownItem } from "../shared/types"; +import { computeDateRange, buildMonthOptions } from "../utils/dateRange"; const fmt = new Intl.NumberFormat("en-CA", { style: "currency", currency: "CAD" }); -function computeDateRange( - period: DashboardPeriod, - customDateFrom?: string, - customDateTo?: string, -): { dateFrom?: string; dateTo?: string } { - if (period === "all") return {}; - if (period === "custom" && customDateFrom && customDateTo) { - return { dateFrom: customDateFrom, dateTo: customDateTo }; - } - const now = new Date(); - const year = now.getFullYear(); - const month = now.getMonth(); - const day = now.getDate(); - const dateTo = `${year}-${String(month + 1).padStart(2, "0")}-${String(day).padStart(2, "0")}`; - let from: Date; - switch (period) { - case "month": from = new Date(year, month, 1); break; - case "3months": from = new Date(year, month - 2, 1); break; - case "6months": from = new Date(year, month - 5, 1); break; - case "year": from = new Date(year, 0, 1); break; - case "12months": from = new Date(year, month - 11, 1); break; - default: from = new Date(year, month, 1); break; - } - const dateFrom = `${from.getFullYear()}-${String(from.getMonth() + 1).padStart(2, "0")}-${String(from.getDate()).padStart(2, "0")}`; - return { dateFrom, dateTo }; -} - export default function DashboardPage() { const { t, i18n } = useTranslation(); const { state, setPeriod, setCustomDates, setBudgetMonth } = useDashboard(); @@ -91,18 +65,7 @@ export default function DashboardPage() { }, ]; - const monthOptions = useMemo(() => { - const now = new Date(); - const currentMonth = now.getMonth(); - const currentYear = now.getFullYear(); - return Array.from({ length: 24 }, (_, i) => { - const d = new Date(currentYear, currentMonth - i, 1); - const y = d.getFullYear(); - const m = d.getMonth() + 1; - const label = new Intl.DateTimeFormat(i18n.language, { month: "long", year: "numeric" }).format(d); - return { key: `${y}-${m}`, value: `${y}-${m}`, label: label.charAt(0).toUpperCase() + label.slice(1) }; - }); - }, [i18n.language]); + const monthOptions = useMemo(() => buildMonthOptions(i18n.language), [i18n.language]); const { dateFrom, dateTo } = computeDateRange(period, state.customDateFrom, state.customDateTo); diff --git a/src/pages/ReportsPage.tsx b/src/pages/ReportsPage.tsx index e7f3b20..da975bd 100644 --- a/src/pages/ReportsPage.tsx +++ b/src/pages/ReportsPage.tsx @@ -3,7 +3,7 @@ import { useTranslation } from "react-i18next"; import { Hash, Table, BarChart3 } from "lucide-react"; import { useReports } from "../hooks/useReports"; import { PageHelp } from "../components/shared/PageHelp"; -import type { ReportTab, CategoryBreakdownItem, DashboardPeriod, ImportSource } from "../shared/types"; +import type { ReportTab, CategoryBreakdownItem, ImportSource } from "../shared/types"; import { getAllSources } from "../services/importSourceService"; import PeriodSelector from "../components/dashboard/PeriodSelector"; import MonthlyTrendsChart from "../components/reports/MonthlyTrendsChart"; @@ -16,36 +16,10 @@ import BudgetVsActualTable from "../components/reports/BudgetVsActualTable"; import DynamicReport from "../components/reports/DynamicReport"; import ReportFilterPanel from "../components/reports/ReportFilterPanel"; import TransactionDetailModal from "../components/shared/TransactionDetailModal"; +import { computeDateRange, buildMonthOptions } from "../utils/dateRange"; const TABS: ReportTab[] = ["trends", "byCategory", "overTime", "budgetVsActual", "dynamic"]; -function computeDateRange( - period: DashboardPeriod, - customDateFrom?: string, - customDateTo?: string, -): { dateFrom?: string; dateTo?: string } { - if (period === "all") return {}; - if (period === "custom" && customDateFrom && customDateTo) { - return { dateFrom: customDateFrom, dateTo: customDateTo }; - } - const now = new Date(); - const year = now.getFullYear(); - const month = now.getMonth(); - const day = now.getDate(); - const dateTo = `${year}-${String(month + 1).padStart(2, "0")}-${String(day).padStart(2, "0")}`; - let from: Date; - switch (period) { - case "month": from = new Date(year, month, 1); break; - case "3months": from = new Date(year, month - 2, 1); break; - case "6months": from = new Date(year, month - 5, 1); break; - case "year": from = new Date(year, 0, 1); break; - case "12months": from = new Date(year, month - 11, 1); break; - default: from = new Date(year, month, 1); break; - } - const dateFrom = `${from.getFullYear()}-${String(from.getMonth() + 1).padStart(2, "0")}-${String(from.getDate()).padStart(2, "0")}`; - return { dateFrom, dateTo }; -} - export default function ReportsPage() { const { t, i18n } = useTranslation(); const { state, setTab, setPeriod, setCustomDates, setBudgetMonth, setPivotConfig, setSourceId } = useReports(); @@ -92,18 +66,7 @@ export default function ReportsPage() { return []; }, [state.tab, state.categorySpending, state.categoryOverTime]); - const monthOptions = useMemo(() => { - const now = new Date(); - const currentMonth = now.getMonth(); - const currentYear = now.getFullYear(); - return Array.from({ length: 24 }, (_, i) => { - const d = new Date(currentYear, currentMonth - i, 1); - const y = d.getFullYear(); - const m = d.getMonth() + 1; - const label = new Intl.DateTimeFormat(i18n.language, { month: "long", year: "numeric" }).format(d); - return { key: `${y}-${m}`, value: `${y}-${m}`, label: label.charAt(0).toUpperCase() + label.slice(1) }; - }); - }, [i18n.language]); + const monthOptions = useMemo(() => buildMonthOptions(i18n.language), [i18n.language]); const hasCategories = ["byCategory", "overTime"].includes(state.tab) && filterCategories.length > 0; const showFilterPanel = hasCategories || (state.tab === "trends" && sources.length > 1); diff --git a/src/utils/dateRange.ts b/src/utils/dateRange.ts new file mode 100644 index 0000000..23ba65d --- /dev/null +++ b/src/utils/dateRange.ts @@ -0,0 +1,66 @@ +import type { DashboardPeriod } from "../shared/types"; + +/** + * Compute a date range (dateFrom / dateTo) based on the selected period. + * Shared between useDashboard, useReports, DashboardPage and ReportsPage. + */ +export function computeDateRange( + period: DashboardPeriod, + customDateFrom?: string, + customDateTo?: string, +): { dateFrom?: string; dateTo?: string } { + if (period === "all") return {}; + if (period === "custom" && customDateFrom && customDateTo) { + return { dateFrom: customDateFrom, dateTo: customDateTo }; + } + + const now = new Date(); + const year = now.getFullYear(); + const month = now.getMonth(); + const day = now.getDate(); + + const dateTo = `${year}-${String(month + 1).padStart(2, "0")}-${String(day).padStart(2, "0")}`; + + let from: Date; + switch (period) { + case "month": + from = new Date(year, month, 1); + break; + case "3months": + from = new Date(year, month - 2, 1); + break; + case "6months": + from = new Date(year, month - 5, 1); + break; + case "year": + from = new Date(year, 0, 1); + break; + case "12months": + from = new Date(year, month - 11, 1); + break; + default: + from = new Date(year, month, 1); + break; + } + + const dateFrom = `${from.getFullYear()}-${String(from.getMonth() + 1).padStart(2, "0")}-${String(from.getDate()).padStart(2, "0")}`; + + return { dateFrom, dateTo }; +} + +/** + * Build an array of month options for the budget month dropdown. + * Returns the last 24 months with localized labels. + */ +export function buildMonthOptions(language: string): Array<{ key: string; value: string; label: string }> { + const now = new Date(); + const currentMonth = now.getMonth(); + const currentYear = now.getFullYear(); + return Array.from({ length: 24 }, (_, i) => { + const d = new Date(currentYear, currentMonth - i, 1); + const y = d.getFullYear(); + const m = d.getMonth() + 1; + const label = new Intl.DateTimeFormat(language, { month: "long", year: "numeric" }).format(d); + return { key: `${y}-${m}`, value: `${y}-${m}`, label: label.charAt(0).toUpperCase() + label.slice(1) }; + }); +}