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 <noreply@anthropic.com>

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;

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);

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 monthOptions = useMemo(() => buildMonthOptions(i18n.language), [i18n.language]);
-    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 { dateFrom, dateTo } = computeDateRange(period, state.customDateFrom, state.customDateTo);
 

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 monthOptions = useMemo(() => buildMonthOptions(i18n.language), [i18n.language]);
-    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 hasCategories = ["byCategory", "overTime"].includes(state.tab) && filterCategories.length > 0;
   const showFilterPanel = hasCategories || (state.tab === "trends" && sources.length > 1);

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) };
+  });
+}