diff --git a/Docs/Tasks.md b/Docs/Tasks.md index 10902d5..e99ff34 100644 --- a/Docs/Tasks.md +++ b/Docs/Tasks.md @@ -304,7 +304,9 @@ Issues are grouped by category and ordered roughly by severity. Each entry descr --- -### TASK-016 — Extract generic `useListData` hook to eliminate duplicated fetch-list pattern +### TASK-016 — Extract generic `useListData` hook to eliminate duplicated fetch-list pattern (done) + +**Where fixed:** `frontend/src/hooks/useListData.ts`, `frontend/src/hooks/useActionList.ts`, `frontend/src/hooks/useFilterList.ts`, `frontend/src/hooks/useJailConfigs.ts`, `frontend/src/hooks/useBlocklists.ts`, `frontend/src/api/config.ts`, `frontend/src/api/blocklist.ts`, `frontend/src/hooks/__tests__/useListData.test.ts` **Where found:** `frontend/src/hooks/useActionList.ts`, `useFilterList.ts`, `useJailConfigs.ts`, and `useBlocklists.ts` each contain ~40 lines of nearly identical code: `useState` for data/loading/error, a `useRef`, a `refresh` callback with abort-create-set-loading-fetch-set logic, and a `useEffect` that calls `refresh` on mount. diff --git a/frontend/src/api/blocklist.ts b/frontend/src/api/blocklist.ts index 2e71b24..cb01dc9 100644 --- a/frontend/src/api/blocklist.ts +++ b/frontend/src/api/blocklist.ts @@ -21,8 +21,8 @@ import type { // --------------------------------------------------------------------------- /** Fetch all configured blocklist sources. */ -export async function fetchBlocklists(): Promise { - return get(ENDPOINTS.blocklists); +export async function fetchBlocklists(signal?: AbortSignal): Promise { + return get(ENDPOINTS.blocklists, signal); } /** Create a new blocklist source. */ diff --git a/frontend/src/api/config.ts b/frontend/src/api/config.ts index 587028b..129f88e 100644 --- a/frontend/src/api/config.ts +++ b/frontend/src/api/config.ts @@ -51,8 +51,9 @@ import type { // --------------------------------------------------------------------------- export async function fetchJailConfigs( +signal?: AbortSignal, ): Promise { - return get(ENDPOINTS.configJails); + return get(ENDPOINTS.configJails, signal); } export async function fetchJailConfig( @@ -344,8 +345,8 @@ export async function assignFilterToJail( * * @returns FilterListResponse with all discovered filters and status. */ -export async function fetchFilters(): Promise { - return get(ENDPOINTS.configFilters); +export async function fetchFilters(signal?: AbortSignal): Promise { + return get(ENDPOINTS.configFilters, signal); } /** @@ -385,8 +386,8 @@ export async function updateParsedAction( * * @returns ActionListResponse with all discovered actions and status. */ -export async function fetchActions(): Promise { - return get(ENDPOINTS.configActions); +export async function fetchActions(signal?: AbortSignal): Promise { + return get(ENDPOINTS.configActions, signal); } /** diff --git a/frontend/src/hooks/__tests__/useListData.test.ts b/frontend/src/hooks/__tests__/useListData.test.ts new file mode 100644 index 0000000..1f4c940 --- /dev/null +++ b/frontend/src/hooks/__tests__/useListData.test.ts @@ -0,0 +1,79 @@ +import { describe, it, expect, vi } from "vitest"; +import { renderHook, act } from "@testing-library/react"; +import { useListData } from "../useListData"; + +describe("useListData", () => { + it("loads items and updates loading", async () => { + const fetcher = vi.fn().mockResolvedValue({ items: ["one", "two"] }); + const selector = vi.fn((response: { items: string[] }) => response.items); + + const { result } = renderHook(() => + useListData({ + fetcher, + selector, + errorMessage: "Failed to load", + }) + ); + + expect(result.current.loading).toBe(true); + expect(result.current.items).toEqual([]); + + await act(async () => { + await Promise.resolve(); + }); + + expect(fetcher).toHaveBeenCalledTimes(1); + expect(selector).toHaveBeenCalledWith({ items: ["one", "two"] }); + expect(result.current.items).toEqual(["one", "two"]); + expect(result.current.loading).toBe(false); + }); + + it("sets error when the fetcher rejects", async () => { + const fetcher = vi.fn().mockRejectedValue(new Error("network")); + const selector = vi.fn(); + + const { result } = renderHook(() => + useListData({ + fetcher, + selector, + errorMessage: "Failed to load", + }) + ); + + await act(async () => { + await Promise.resolve(); + }); + + expect(result.current.error).toBe("network"); + expect(result.current.loading).toBe(false); + expect(result.current.items).toEqual([]); + }); + + it("refresh triggers a second fetch", async () => { + const fetcher = vi.fn() + .mockResolvedValueOnce({ items: ["first"] }) + .mockResolvedValueOnce({ items: ["second"] }); + const selector = vi.fn((response: { items: string[] }) => response.items); + + const { result } = renderHook(() => + useListData({ + fetcher, + selector, + errorMessage: "Failed to load", + }) + ); + + await act(async () => { + await Promise.resolve(); + }); + + expect(result.current.items).toEqual(["first"]); + await act(async () => { + result.current.refresh(); + await Promise.resolve(); + }); + + expect(fetcher).toHaveBeenCalledTimes(2); + expect(result.current.items).toEqual(["second"]); + }); +}); diff --git a/frontend/src/hooks/useActionList.ts b/frontend/src/hooks/useActionList.ts index 5704da2..319d3ce 100644 --- a/frontend/src/hooks/useActionList.ts +++ b/frontend/src/hooks/useActionList.ts @@ -1,10 +1,10 @@ /** * React hook for loading action metadata used by the actions tab. */ -import { useCallback, useEffect, useRef, useState } from "react"; +import { useCallback } from "react"; import { fetchActions, removeActionFromJail, createAction, assignActionToJail } from "../api/config"; -import { handleFetchError } from "../utils/fetchError"; -import type { ActionConfig, ActionCreateRequest } from "../types/config"; +import { useListData } from "./useListData"; +import type { ActionConfig, ActionCreateRequest, ActionListResponse } from "../types/config"; export interface UseActionListResult { actions: ActionConfig[]; @@ -20,43 +20,18 @@ export interface UseActionListResult { * Load the action inventory and expose related action operations. */ export function useActionList(): UseActionListResult { - const [actions, setActions] = useState([]); - const [loading, setLoading] = useState(true); - const [error, setError] = useState(null); - const abortRef = useRef(null); + const fetcher = useCallback( + (signal: AbortSignal) => fetchActions(signal), + [], + ); - const refresh = useCallback((): void => { - abortRef.current?.abort(); - const controller = new AbortController(); - abortRef.current = controller; + const selector = useCallback((response: ActionListResponse) => response.actions, []); - setLoading(true); - setError(null); - - fetchActions() - .then((resp) => { - if (!controller.signal.aborted) { - setActions(resp.actions); - } - }) - .catch((err: unknown) => { - if (!controller.signal.aborted) { - handleFetchError(err, setError, "Failed to load actions"); - } - }) - .finally(() => { - if (!controller.signal.aborted) { - setLoading(false); - } - }); - }, []); - - useEffect(() => { - refresh(); - return (): void => { - abortRef.current?.abort(); - }; - }, [refresh]); + const { items: actions, loading, error, refresh } = useListData({ + fetcher, + selector, + errorMessage: "Failed to load actions", + }); const handleRemoveActionFromJail = useCallback( async (jailName: string, actionName: string): Promise => { diff --git a/frontend/src/hooks/useBlocklists.ts b/frontend/src/hooks/useBlocklists.ts index 0305754..b68cd04 100644 --- a/frontend/src/hooks/useBlocklists.ts +++ b/frontend/src/hooks/useBlocklists.ts @@ -2,7 +2,7 @@ * React hook for listing and mutating blocklist sources. */ -import { useCallback, useEffect, useRef, useState } from "react"; +import { useCallback } from "react"; import { createBlocklist, deleteBlocklist, @@ -10,11 +10,12 @@ import { previewBlocklist, updateBlocklist, } from "../api/blocklist"; -import { handleFetchError } from "../utils/fetchError"; +import { useListData } from "./useListData"; import type { BlocklistSource, BlocklistSourceCreate, BlocklistSourceUpdate, + BlocklistListResponse, PreviewResponse, } from "../types/blocklist"; @@ -33,63 +34,44 @@ export interface UseBlocklistsReturn { * Load all blocklist sources and expose CRUD operations. */ export function useBlocklists(): UseBlocklistsReturn { - const [sources, setSources] = useState([]); - const [loading, setLoading] = useState(true); - const [error, setError] = useState(null); - const abortRef = useRef(null); + const fetcher = useCallback( + (signal: AbortSignal) => fetchBlocklists(signal), + [], + ); - const load = useCallback((): void => { - abortRef.current?.abort(); - const ctrl = new AbortController(); - abortRef.current = ctrl; + const selector = useCallback((response: BlocklistListResponse) => response.sources, []); - setLoading(true); - setError(null); - - fetchBlocklists() - .then((data) => { - if (!ctrl.signal.aborted) { - setSources(data.sources); - setLoading(false); - } - }) - .catch((err: unknown) => { - if (!ctrl.signal.aborted) { - handleFetchError(err, setError, "Failed to load blocklists"); - setLoading(false); - } - }); - }, []); - - useEffect(() => { - load(); - return (): void => { - abortRef.current?.abort(); - }; - }, [load]); + const { items: sources, loading, error, refresh } = useListData({ + fetcher, + selector, + errorMessage: "Failed to load blocklists", + }); const createSource = useCallback( async (payload: BlocklistSourceCreate): Promise => { const created = await createBlocklist(payload); - setSources((prev) => [...prev, created]); + refresh(); return created; }, - [], + [refresh], ); const updateSource = useCallback( async (id: number, payload: BlocklistSourceUpdate): Promise => { const updated = await updateBlocklist(id, payload); - setSources((prev) => prev.map((s) => (s.id === id ? updated : s))); + refresh(); return updated; }, - [], + [refresh], ); - const removeSource = useCallback(async (id: number): Promise => { - await deleteBlocklist(id); - setSources((prev) => prev.filter((s) => s.id !== id)); - }, []); + const removeSource = useCallback( + async (id: number): Promise => { + await deleteBlocklist(id); + refresh(); + }, + [refresh], + ); const previewSource = useCallback(async (id: number): Promise => { return previewBlocklist(id); @@ -99,7 +81,7 @@ export function useBlocklists(): UseBlocklistsReturn { sources, loading, error, - refresh: load, + refresh, createSource, updateSource, removeSource, diff --git a/frontend/src/hooks/useFilterList.ts b/frontend/src/hooks/useFilterList.ts index 12477e3..60e80d9 100644 --- a/frontend/src/hooks/useFilterList.ts +++ b/frontend/src/hooks/useFilterList.ts @@ -1,10 +1,10 @@ /** * React hook for loading filter config metadata used by the filter tab. */ -import { useCallback, useEffect, useRef, useState } from "react"; +import { useCallback } from "react"; import { fetchFilters, createFilter, assignFilterToJail } from "../api/config"; -import { handleFetchError } from "../utils/fetchError"; -import type { FilterConfig, FilterCreateRequest } from "../types/config"; +import { useListData } from "./useListData"; +import type { FilterConfig, FilterCreateRequest, FilterListResponse } from "../types/config"; export interface UseFilterListResult { filters: FilterConfig[]; @@ -19,43 +19,18 @@ export interface UseFilterListResult { * Load the filter inventory and expose refresh semantics. */ export function useFilterList(): UseFilterListResult { - const [filters, setFilters] = useState([]); - const [loading, setLoading] = useState(true); - const [error, setError] = useState(null); - const abortRef = useRef(null); + const fetcher = useCallback( + (signal: AbortSignal) => fetchFilters(signal), + [], + ); - const refresh = useCallback((): void => { - abortRef.current?.abort(); - const controller = new AbortController(); - abortRef.current = controller; + const selector = useCallback((response: FilterListResponse) => response.filters, []); - setLoading(true); - setError(null); - - fetchFilters() - .then((resp) => { - if (!controller.signal.aborted) { - setFilters(resp.filters); - } - }) - .catch((err: unknown) => { - if (!controller.signal.aborted) { - handleFetchError(err, setError, "Failed to load filters"); - } - }) - .finally(() => { - if (!controller.signal.aborted) { - setLoading(false); - } - }); - }, []); - - useEffect(() => { - refresh(); - return (): void => { - abortRef.current?.abort(); - }; - }, [refresh]); + const { items: filters, loading, error, refresh } = useListData({ + fetcher, + selector, + errorMessage: "Failed to load filters", + }); const handleCreateFilter = useCallback( async (payload: FilterCreateRequest): Promise => { diff --git a/frontend/src/hooks/useJailConfigs.ts b/frontend/src/hooks/useJailConfigs.ts index 4db45e8..69bc1b3 100644 --- a/frontend/src/hooks/useJailConfigs.ts +++ b/frontend/src/hooks/useJailConfigs.ts @@ -2,10 +2,10 @@ * React hook for loading the jail config inventory. */ -import { useCallback, useEffect, useRef, useState } from "react"; +import { useCallback, useState } from "react"; import { fetchJailConfigs, reloadConfig, updateJailConfig } from "../api/config"; -import { handleFetchError } from "../utils/fetchError"; -import type { JailConfig, JailConfigUpdate } from "../types/config"; +import { useListData } from "./useListData"; +import type { JailConfig, JailConfigUpdate, JailConfigListResponse } from "../types/config"; export interface UseJailConfigsResult { jails: JailConfig[]; @@ -21,57 +21,36 @@ export interface UseJailConfigsResult { * Load all jail configs and expose update controls. */ export function useJailConfigs(): UseJailConfigsResult { - const [jails, setJails] = useState([]); const [total, setTotal] = useState(0); - const [loading, setLoading] = useState(true); - const [error, setError] = useState(null); - const abortRef = useRef(null); - const load = useCallback((): void => { - abortRef.current?.abort(); - const ctrl = new AbortController(); - abortRef.current = ctrl; - setLoading(true); - setError(null); + const fetcher = useCallback( + (signal: AbortSignal) => fetchJailConfigs(signal), + [], + ); - fetchJailConfigs() - .then((resp) => { - if (!ctrl.signal.aborted) { - setJails(resp.jails); - setTotal(resp.total); - } - }) - .catch((err: unknown) => { - if (!ctrl.signal.aborted) { - handleFetchError(err, setError, "Failed to fetch jail configs"); - } - }) - .finally(() => { - if (!abortRef.current?.signal.aborted) { - setLoading(false); - } - }); - }, []); + const selector = useCallback((response: JailConfigListResponse) => response.jails, []); - useEffect(() => { - load(); - return (): void => { - abortRef.current?.abort(); - }; - }, [load]); + const { items: jails, loading, error, refresh } = useListData({ + fetcher, + selector, + errorMessage: "Failed to fetch jail configs", + onSuccess: (response) => { + setTotal(response.total); + }, + }); const updateJail = useCallback( async (name: string, update: JailConfigUpdate): Promise => { await updateJailConfig(name, update); - load(); + refresh(); }, - [load], + [refresh], ); const reloadAll = useCallback(async (): Promise => { await reloadConfig(); - load(); - }, [load]); + refresh(); + }, [refresh]); - return { jails, total, loading, error, refresh: load, updateJail, reloadAll }; + return { jails, total, loading, error, refresh, updateJail, reloadAll }; } diff --git a/frontend/src/hooks/useListData.ts b/frontend/src/hooks/useListData.ts new file mode 100644 index 0000000..54234ec --- /dev/null +++ b/frontend/src/hooks/useListData.ts @@ -0,0 +1,70 @@ +/** + * Generic hook for loading list data from an API endpoint. + */ +import { useCallback, useEffect, useRef, useState } from "react"; +import { handleFetchError } from "../utils/fetchError"; + +export interface UseListDataOptions { + fetcher: (signal: AbortSignal) => Promise; + selector: (response: TResponse) => TItem[]; + errorMessage: string; + onSuccess?: (response: TResponse) => void; + initialItems?: TItem[]; +} + +export interface UseListDataResult { + items: TItem[]; + loading: boolean; + error: string | null; + refresh: () => void; +} + +/** + * Load a list response and expose refresh semantics with abort support. + */ +export function useListData( + options: UseListDataOptions, +): UseListDataResult { + const { fetcher, selector, errorMessage, onSuccess, initialItems } = options; + const [items, setItems] = useState(initialItems ?? []); + const [loading, setLoading] = useState(true); + const [error, setError] = useState(null); + const abortRef = useRef(null); + + const refresh = useCallback((): void => { + abortRef.current?.abort(); + const controller = new AbortController(); + abortRef.current = controller; + + setLoading(true); + setError(null); + + fetcher(controller.signal) + .then((response) => { + if (controller.signal.aborted) return; + setItems(selector(response)); + if (onSuccess) { + onSuccess(response); + } + }) + .catch((err: unknown) => { + if (controller.signal.aborted) return; + handleFetchError(err, setError, errorMessage); + }) + .finally(() => { + if (!controller.signal.aborted) { + setLoading(false); + } + }); + }, [fetcher, selector, errorMessage, onSuccess]); + + useEffect(() => { + refresh(); + + return (): void => { + abortRef.current?.abort(); + }; + }, [refresh]); + + return { items, loading, error, refresh }; +}