Docs/Tasks.md

@@ -1,26 +1,3 @@
-### T-17 · `useHistory` is missing abort-signal guards — stale state update bug
-
-**Where found:** `frontend/src/hooks/useHistory.ts` — `.then()`, `.catch()`, `.finally()` callbacks update state without checking `abortRef.current.signal.aborted`
-
-**Why this is needed:** Every other data-fetching hook in the codebase guards all state-update callbacks against aborted signals. `useHistory` does not. If the component unmounts mid-request, `setItems`, `setTotal`, `setLoading` will all fire on an unmounted component. In React 18 this is a no-op but it still indicates a broken invariant and `handleFetchError` could misclassify the abort as a real error (depends on whether `fetch` threw `AbortError` or the API module swallowed it).
-
-**Goal:** All callbacks in `useHistory` check the abort signal before mutating state.
-
-**What to do:**
-1. Capture the controller in a local variable inside `load()` (already done: `abortRef.current = new AbortController()`).
-2. In `.then()`: add `if (abortRef.current.signal.aborted) return;` before `setItems(...)`.
-3. In `.catch()`: add the same guard before `handleFetchError(...)`.
-4. In `.finally()`: add `if (!abortRef.current.signal.aborted)` before `setLoading(false)`.
-
-**Possible traps and issues:**
-- `abortRef.current` may have been replaced by a new controller before the callback fires. Capture the controller in a closure variable at the top of `load()`: `const controller = abortRef.current`.
-
-**Docs changes needed:** None.
-
-**Doc references:** `frontend/src/hooks/useHistory.ts`
-
----
-
 ### T-18 · Merge `useDashboardCountryData` and `useMapData` — near-identical hooks
 
 **Where found:** `frontend/src/hooks/useDashboardCountryData.ts` and `frontend/src/hooks/useMapData.ts`

frontend/src/hooks/__tests__/useBansByCountry.test.ts

@@ -0,0 +1,83 @@
+import { describe, expect, it, vi, beforeEach } from "vitest";
+import { renderHook, act, waitFor } from "@testing-library/react";
+import type { BansByCountryResponse } from "../../types/map";
+import { useBansByCountry } from "../useBansByCountry";
+import * as api from "../../api/map";
+
+vi.mock("../../api/map");
+
+describe("useBansByCountry", () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+  });
+
+  it("fetches data automatically on mount", async () => {
+    const fetchMock = vi.mocked(api.fetchBansByCountry);
+    const response: BansByCountryResponse = {
+      countries: { US: 1, GB: 2 },
+      country_names: { US: "United States", GB: "United Kingdom" },
+      bans: [],
+      total: 3,
+    };
+
+    fetchMock.mockResolvedValueOnce(response);
+
+    const { result } = renderHook(() => useBansByCountry("24h", "all", "fail2ban"));
+
+    expect(result.current.loading).toBe(true);
+
+    await waitFor(() => {
+      expect(result.current.loading).toBe(false);
+    });
+
+    expect(result.current.countries).toEqual({ US: 1, GB: 2 });
+    expect(result.current.countryNames).toEqual({ US: "United States", GB: "United Kingdom" });
+    expect(result.current.total).toBe(3);
+    expect(fetchMock).toHaveBeenCalledTimes(1);
+  });
+
+  it("does not fetch automatically when autoFetch is false", async () => {
+    const fetchMock = vi.mocked(api.fetchBansByCountry);
+    const response: BansByCountryResponse = {
+      countries: { US: 1 },
+      country_names: { US: "United States" },
+      bans: [],
+      total: 1,
+    };
+
+    fetchMock.mockResolvedValueOnce(response);
+
+    const { result } = renderHook(() => useBansByCountry("24h", "all", "fail2ban", undefined, false));
+
+    expect(result.current.loading).toBe(true);
+    expect(fetchMock).not.toHaveBeenCalled();
+
+    act(() => {
+      result.current.refresh();
+    });
+
+    await waitFor(() => {
+      expect(result.current.loading).toBe(false);
+    });
+
+    expect(result.current.countries).toEqual({ US: 1 });
+    expect(fetchMock).toHaveBeenCalledTimes(1);
+  });
+
+  it("handles fetch errors correctly", async () => {
+    const fetchMock = vi.mocked(api.fetchBansByCountry);
+    const error = new Error("Network error");
+
+    fetchMock.mockRejectedValueOnce(error);
+
+    const { result } = renderHook(() => useBansByCountry("24h", "all", "fail2ban"));
+
+    await waitFor(() => {
+      expect(result.current.loading).toBe(false);
+    });
+
+    expect(result.current.error).not.toBeNull();
+    expect(result.current.countries).toEqual({});
+    expect(result.current.total).toBe(0);
+  });
+});

frontend/src/hooks/__tests__/useDashboardCountryData.test.ts

@@ -0,0 +1,83 @@
+import { describe, expect, it, vi, beforeEach } from "vitest";
+import { renderHook, act, waitFor } from "@testing-library/react";
+import type { BansByCountryResponse } from "../../types/map";
+import { useDashboardCountryData } from "../useDashboardCountryData";
+import * as api from "../../api/map";
+
+vi.mock("../../api/map");
+
+describe("useDashboardCountryData", () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+  });
+
+  it("fetches data and exposes country data with reload function", async () => {
+    const fetchMock = vi.mocked(api.fetchBansByCountry);
+    const response: BansByCountryResponse = {
+      countries: { US: 1, GB: 2 },
+      country_names: { US: "United States", GB: "United Kingdom" },
+      bans: [
+        {
+          ip: "1.2.3.4",
+          jail: "sshd",
+          banned_at: "2024-01-01T00:00:00Z",
+          service: null,
+          country_code: "US",
+          country_name: "United States",
+          asn: "AS123",
+          org: "Org",
+          ban_count: 1,
+          origin: "selfblock",
+        },
+      ],
+      total: 3,
+    };
+
+    fetchMock.mockResolvedValueOnce(response);
+
+    const { result } = renderHook(() => useDashboardCountryData("24h", "all", "fail2ban"));
+
+    expect(result.current.loading).toBe(true);
+
+    await waitFor(() => {
+      expect(result.current.loading).toBe(false);
+    });
+
+    expect(result.current.countries).toEqual({ US: 1, GB: 2 });
+    expect(result.current.countryNames).toEqual({ US: "United States", GB: "United Kingdom" });
+    expect(result.current.bans).toHaveLength(1);
+    expect(result.current.total).toBe(3);
+    expect(fetchMock).toHaveBeenCalledTimes(1);
+  });
+
+  it("supports the reload function for manual refresh", async () => {
+    const fetchMock = vi.mocked(api.fetchBansByCountry);
+    const response: BansByCountryResponse = {
+      countries: { US: 1 },
+      country_names: { US: "United States" },
+      bans: [],
+      total: 1,
+    };
+
+    fetchMock.mockResolvedValueOnce(response);
+    fetchMock.mockResolvedValueOnce({ ...response, total: 2 });
+
+    const { result } = renderHook(() => useDashboardCountryData("24h", "all", "fail2ban"));
+
+    await waitFor(() => {
+      expect(result.current.loading).toBe(false);
+    });
+
+    expect(result.current.total).toBe(1);
+
+    act(() => {
+      result.current.reload();
+    });
+
+    await waitFor(() => {
+      expect(result.current.total).toBe(2);
+    });
+
+    expect(fetchMock).toHaveBeenCalledTimes(2);
+  });
+});

frontend/src/hooks/useBansByCountry.ts

@@ -0,0 +1,112 @@
+/**
+ * Base hook for fetching ban-by-country aggregates.
+ *
+ * This hook encapsulates the core logic for fetching country-level ban statistics,
+ * maintaining abort-controller semantics, and exposing both the full response and
+ * its individual fields for convenient destructuring.
+ *
+ * For a debounced version, use {@link useMapData}.
+ * For a dashboard-specific wrapper, use {@link useDashboardCountryData}.
+ */
+
+import { useCallback, useEffect, useRef, useState } from "react";
+import { fetchBansByCountry } from "../api/map";
+import { handleFetchError } from "../utils/fetchError";
+import type { BanOriginFilter, TimeRange } from "../types/ban";
+import type { BansByCountryResponse, MapBanItem } from "../types/map";
+
+/**
+ * Return value shape for {@link useBansByCountry}.
+ */
+export interface UseBansByCountryResult {
+  /** Per-country ban counts (ISO alpha-2 → count). */
+  countries: Record<string, number>;
+  /** ISO alpha-2 → country name mapping. */
+  countryNames: Record<string, string>;
+  /** All ban records in the selected window. */
+  bans: MapBanItem[];
+  /** Total ban count. */
+  total: number;
+  /** True while a fetch is in flight. */
+  loading: boolean;
+  /** Error message or null. */
+  error: string | null;
+  /** Trigger a manual re-fetch. */
+  refresh: () => void;
+  /** The full response object (for callers that need raw data). */
+  data: BansByCountryResponse | null;
+}
+
+/**
+ * Fetch and manage ban-by-country data.
+ *
+ * This is the base hook shared by both `useDashboardCountryData` and `useMapData`.
+ * It handles the fetching, abort-controller pattern, and state management.
+ * Callers can add debouncing or other behaviour on top.
+ *
+ * @param timeRange - Time-range preset: `"24h"`, `"7d"`, `"30d"`, or `"365d"`.
+ * @param origin - Origin filter: `"all"`, `"blocklist"`, or `"selfblock"`.
+ * @param source - Data source: `"fail2ban"` or `"archive"`.
+ * @param countryCode - Optional ISO alpha-2 country code to filter by.
+ * @param autoFetch - If false, caller must invoke refresh() manually (default: true).
+ * @returns Country data, loading state, error, and refresh function.
+ */
+export function useBansByCountry(
+  timeRange: TimeRange,
+  origin: BanOriginFilter,
+  source: "fail2ban" | "archive" = "fail2ban",
+  countryCode?: string,
+  autoFetch: boolean = true,
+): UseBansByCountryResult {
+  const [data, setData] = useState<BansByCountryResponse | null>(null);
+  const [loading, setLoading] = useState(true);
+  const [error, setError] = useState<string | null>(null);
+  const abortRef = useRef<AbortController | null>(null);
+
+  const refresh = useCallback((): void => {
+    // Abort any in-flight request from a previous filter selection.
+    abortRef.current?.abort();
+    const controller = new AbortController();
+    abortRef.current = controller;
+
+    setLoading(true);
+    setError(null);
+
+    fetchBansByCountry(timeRange, origin, source, countryCode, controller.signal)
+      .then((resp) => {
+        if (!controller.signal.aborted) {
+          setData(resp);
+        }
+      })
+      .catch((err: unknown) => {
+        if (!controller.signal.aborted) {
+          handleFetchError(err, setError, "Failed to fetch ban-by-country data");
+        }
+      })
+      .finally((): void => {
+        if (!controller.signal.aborted) {
+          setLoading(false);
+        }
+      });
+  }, [timeRange, origin, source, countryCode]);
+
+  useEffect((): (() => void) => {
+    if (autoFetch) {
+      refresh();
+    }
+    return (): void => {
+      abortRef.current?.abort();
+    };
+  }, [refresh, autoFetch]);
+
+  return {
+    countries: data?.countries ?? {},
+    countryNames: data?.country_names ?? {},
+    bans: data?.bans ?? [],
+    total: data?.total ?? 0,
+    loading,
+    error,
+    refresh,
+    data,
+  };
+}

frontend/src/hooks/useDashboardCountryData.ts

@@ -1,17 +1,17 @@
 /**
  * `useDashboardCountryData` hook.
  *
- * Fetches ban-by-country aggregates for dashboard chart components.  Unlike
- * `useMapData`, this hook has no debouncing or map-specific state.
+ * Fetches ban-by-country aggregates for dashboard chart components. This is a thin
+ * wrapper around {@link useBansByCountry} that preserves the original API and naming.
+ *
+ * Unlike `useMapData`, this hook has no debouncing.
  *
  * Re-fetches automatically when `timeRange` or `origin` changes.
  */
 
-import { useCallback, useState } from "react";
-import { fetchBansByCountry } from "../api/map";
-import { useListData } from "./useListData";
 import type { BanOriginFilter, TimeRange } from "../types/ban";
-import type { BansByCountryResponse, MapBanItem } from "../types/map";
+import type { MapBanItem } from "../types/map";
+import { useBansByCountry } from "./useBansByCountry";
 
 /** Return value shape for {@link useDashboardCountryData}. */
 export interface UseDashboardCountryDataResult {
@@ -36,6 +36,7 @@ export interface UseDashboardCountryDataResult {
  *
  * @param timeRange - Time-range preset: `"24h"`, `"7d"`, `"30d"`, or `"365d"`.
  * @param origin    - Origin filter: `"all"`, `"blocklist"`, or `"selfblock"`.
+ * @param source    - Data source: `"fail2ban"` or `"archive"`.
  * @returns Aggregated country data, ban list, loading state, and error.
  */
 export function useDashboardCountryData(
@@ -43,30 +44,8 @@ export function useDashboardCountryData(
   origin: BanOriginFilter,
   source: "fail2ban" | "archive" = "fail2ban",
 ): UseDashboardCountryDataResult {
-  const [countries, setCountries] = useState<Record<string, number>>({});
-  const [countryNames, setCountryNames] = useState<Record<string, string>>({});
-  const [total, setTotal] = useState<number>(0);
-
-  const fetcher = useCallback(
-    (signal: AbortSignal) =>
-      fetchBansByCountry(timeRange, origin, source, undefined, signal),
-    [timeRange, origin, source],
-  );
-
-  const selector = useCallback((response: BansByCountryResponse) => response.bans, []);
-
-  const onSuccess = useCallback((response: BansByCountryResponse) => {
-    setCountries(response.countries);
-    setCountryNames(response.country_names);
-    setTotal(response.total);
-  }, []);
-
-  const { items: bans, loading, error, refresh } = useListData<BansByCountryResponse, MapBanItem>({
-    fetcher,
-    selector,
-    errorMessage: "Failed to fetch dashboard country data",
-    onSuccess,
-  });
+  const { countries, countryNames, bans, total, loading, error, refresh } =
+    useBansByCountry(timeRange, origin, source, undefined);
 
   return { countries, countryNames, bans, total, loading, error, reload: refresh };
 }

frontend/src/hooks/useMapData.ts

@@ -1,12 +1,15 @@
 /**
- * `useMapData` hook — fetches and manages ban-by-country data.
+ * `useMapData` hook — fetches and manages ban-by-country data with debouncing.
+ *
+ * This hook wraps {@link useBansByCountry} and adds a 300ms debounce to reduce
+ * unnecessary fetches when filters change rapidly (e.g., during user interaction
+ * with UI controls).
  */
 
-import { useCallback, useEffect, useRef, useState } from "react";
-import { fetchBansByCountry } from "../api/map";
-import { handleFetchError } from "../utils/fetchError";
-import type { BansByCountryResponse, MapBanItem, TimeRange } from "../types/map";
-import type { BanOriginFilter } from "../types/ban";
+import { useCallback, useEffect, useRef } from "react";
+import { useBansByCountry } from "./useBansByCountry";
+import type { BanOriginFilter, TimeRange } from "../types/ban";
+import type { MapBanItem } from "../types/map";
 
 // ---------------------------------------------------------------------------
 // Constants
@@ -46,38 +49,23 @@ export function useMapData(
   source: "fail2ban" | "archive" = "fail2ban",
   countryCode?: string,
 ): UseMapDataResult {
-  const [data, setData] = useState<BansByCountryResponse | null>(null);
-  const [loading, setLoading] = useState(true);
-  const [error, setError] = useState<string | null>(null);
-  const abortRef = useRef<AbortController | null>(null);
   const debounceRef = useRef<ReturnType<typeof setTimeout> | null>(null);
 
+  const base = useBansByCountry(range, origin, source, countryCode, false);
+
   const load = useCallback((): void => {
     // Cancel any pending debounce timer.
     if (debounceRef.current != null) {
       clearTimeout(debounceRef.current);
     }
-    setError(null);
 
     debounceRef.current = setTimeout((): void => {
-      // Show loading only when the fetch is about to start.
-      setLoading(true);
-      // Abort any in-flight request from a previous filter selection.
-      abortRef.current?.abort();
-      abortRef.current = new AbortController();
-
-      fetchBansByCountry(range, origin, source, countryCode, abortRef.current.signal)
-        .then((resp) => {
-          setData(resp);
-        })
-        .catch((err: unknown) => {
-          handleFetchError(err, setError, "Failed to fetch map data");
-        })
-        .finally((): void => {
-          setLoading(false);
-        });
+      base.refresh();
     }, DEBOUNCE_MS);
-  }, [range, origin, source, countryCode]);
+    // base.refresh is memoized with all relevant deps; including base would cause
+    // unnecessary recreations on every render due to object identity.
+    // eslint-disable-next-line react-hooks/exhaustive-deps
+  }, [base.refresh]);
 
   useEffect((): (() => void) => {
     load();
@@ -85,17 +73,16 @@ export function useMapData(
       if (debounceRef.current != null) {
         clearTimeout(debounceRef.current);
       }
-      abortRef.current?.abort();
     };
   }, [load]);
 
   return {
-    countries: data?.countries ?? {},
-    countryNames: data?.country_names ?? {},
-    bans: data?.bans ?? [],
-    total: data?.total ?? 0,
-    loading,
-    error,
+    countries: base.countries,
+    countryNames: base.countryNames,
+    bans: base.bans,
+    total: base.total,
+    loading: base.loading,
+    error: base.error,
     refresh: load,
   };
 }