From a92a8220c2a51e891e8c94b6b2d41e47baf5ecd9 Mon Sep 17 00:00:00 2001 From: Lukas Date: Sun, 22 Mar 2026 14:20:41 +0100 Subject: [PATCH] backup --- Docs/Refactoring.md | 190 -------------------------- Docs/Tasks.md | 320 -------------------------------------------- 2 files changed, 510 deletions(-) diff --git a/Docs/Refactoring.md b/Docs/Refactoring.md index 375627e..5aae694 100644 --- a/Docs/Refactoring.md +++ b/Docs/Refactoring.md @@ -3,193 +3,3 @@ This document catalogues architecture violations, code smells, and structural issues found during a full project review. Issues are grouped by category and prioritised. --- - -## 1. Service-to-Service Coupling (Backend) - -The architecture mandates that dependencies flow **routers → services → repositories**, yet **15 service-to-service imports** exist, with 7 using lazy imports to work around circular dependencies. - -| Source Service | Imports From | Line | Mechanism | -|---|---|---|---| -| `history_service` | `ban_service` | L29 | Direct import of 3 **private** functions: `_get_fail2ban_db_path`, `_parse_data_json`, `_ts_to_iso` | -| `auth_service` | `setup_service` | L23 | Top-level import | -| `config_service` | `setup_service` | L47 | Top-level import | -| `config_service` | `health_service` | L891 | Lazy import inside function | -| `config_file_service` | `jail_service` | L57–58 | Top-level import + re-export of `JailNotFoundError` | -| `blocklist_service` | `jail_service` | L299 | Lazy import | -| `blocklist_service` | `geo_service` | L343 | Lazy import | -| `jail_service` | `geo_service` | L860, L1047 | Lazy import (2 sites) | -| `ban_service` | `geo_service` | L251, L392 | Lazy import (2 sites) | -| `history_service` | `geo_service` | L19 | TYPE_CHECKING import | - -**Impact**: Circular dependency risk; lazy imports hide coupling; private function imports create fragile links between services. - -**Recommendation**: -- Extract `_get_fail2ban_db_path()`, `_parse_data_json()`, `_ts_to_iso()` from `ban_service` to `app/utils/fail2ban_db_utils.py` (shared utility). -- Pass geo-enrichment as a callback parameter instead of each service importing `geo_service` directly. The router or dependency layer should wire this. -- Where services depend on another service's domain exceptions (e.g., `JailNotFoundError`), move exceptions to `app/models/` or a shared `app/exceptions.py`. - ---- - -## 2. God Modules (Backend) - -Several service files far exceed a reasonable size for a single-domain module: - -| File | Lines | Functions | Issue | -|---|---|---|---| -| `config_file_service.py` | **3105** | **73** | Handles jails, filters, and actions — three distinct domains crammed into one file | -| `jail_service.py` | **1382** | **34** | Manages jail listing, status, controls, banned-IP queries, and geo enrichment | -| `config_service.py` | **921** | ~20 | Socket-based config, log preview, regex testing, and service status | -| `file_config_service.py` | **1011** | ~20 | Raw file I/O for jails, filters, and actions | - -**Recommendation**: -- Split `config_file_service.py` into `filter_config_service.py`, `action_config_service.py`, and a slimmed-down `jail_config_service.py`. -- Extract log-preview / regex-test functionality from `config_service.py` into a dedicated `log_service.py`. - ---- - -## 3. Confusing Config Service Naming (Backend) - -Three services with overlapping names handle different aspects of configuration, causing developer confusion: - -| Current Name | Purpose | -|---|---| -| `config_service` | Read/write via the fail2ban **socket** | -| `config_file_service` | Parse/activate/deactivate jails from **files on disk** | -| `file_config_service` | **Raw file I/O** for jail/filter/action `.conf` files | - -`config_file_service` vs `file_config_service` differ only by word order, making it easy to import the wrong one. - -**Recommendation**: Rename for clarity: -- `config_service` → keep (socket-based) -- `config_file_service` → `jail_activation_service` (its main job is activating/deactivating jails) -- `file_config_service` → `raw_config_io_service` or merge into `config_file_service` - ---- - -## 4. Architecture Doc Drift - -The architecture doc does not fully reflect the current codebase: - -| Category | In Architecture Doc | Actually Exists | Notes | -|---|---|---|---| -| Repositories | 4 listed | **6 files** | `fail2ban_db_repo.py` and `geo_cache_repo.py` are missing from the doc | -| Utils | 4 listed | **8 files** | `conffile_parser.py`, `config_parser.py`, `config_writer.py`, `jail_config.py` are undocumented | -| Tasks | 3 listed | **4 files** | `geo_re_resolve.py` is missing from the doc | -| Services | `conffile_parser` listed as a service | Actually in `app/utils/` | Doc says "Services" but the file is in `utils/` | -| Routers | `file_config.py` not listed | Exists | Missing from router table | - -**Recommendation**: Update the Architecture doc to reflect the actual file inventory. - ---- - -## 5. Shared Private Functions Cross Service Boundary (Backend) - -`history_service.py` imports three **underscore-prefixed** ("private") functions from `ban_service.py`: - -```python -from app.services.ban_service import _get_fail2ban_db_path, _parse_data_json, _ts_to_iso -``` - -These are implementation details of `ban_service` that should not be consumed externally. Their `_` prefix signals they are not part of the public API. - -**Recommendation**: Move these to `app/utils/fail2ban_db_utils.py` as public functions and import from there in both services. - ---- - -## 6. Missing Error Boundaries (Frontend) - -No React Error Boundary component exists anywhere in the frontend. A single unhandled exception in any component will crash the entire application with a white screen. - -**Recommendation**: Add an `` wrapper in `MainLayout.tsx` or `App.tsx` with a fallback UI that shows the error and offers a retry/reload. - ---- - -## 7. Duplicated Formatting Functions (Frontend) - -Several formatting functions are independently defined in multiple files instead of being shared: - -| Function | Defined In | Also In | -|---|---|---| -| `formatTimestamp()` | `BanTable.tsx` (L103) | — (but `fmtTime()` in `BannedIpsSection.tsx` does the same thing) | -| `fmtSeconds()` | `JailDetailPage.tsx` (L152) | `JailsPage.tsx` (L147) — identical | -| `fmtTime()` | `BannedIpsSection.tsx` (L139) | — | - -**Recommendation**: Consolidate into `src/utils/formatDate.ts` and import from there. - ---- - -## 8. Duplicated Hook Logic (Frontend) - -Three hooks follow an identical fetch-then-save pattern with near-identical code: - -| Hook | Lines | Pattern | -|---|---|---| -| `useFilterConfig.ts` | 91 | Load item → expose save → handle abort | -| `useActionConfig.ts` | 89 | Load item → expose save → handle abort | -| `useJailFileConfig.ts` | 76 | Load item → expose save → handle abort | - -**Recommendation**: Create a generic `useConfigItem()` hook that takes `fetchFn` and `saveFn` parameters and eliminates the triplication. - ---- - -## 9. Inconsistent Error Handling in Hooks (Frontend) - -Hooks handle errors differently: - -- Some filter out `AbortError` (e.g., `useHistory`, `useMapData`) -- Others catch all errors indiscriminately (e.g., `useBans`, `useBlocklist`) - -This means some hooks surface spurious "request aborted" errors to the UI while others don't. - -**Recommendation**: Standardise a shared error-catching pattern, e.g. a `handleFetchError(err, setError)` utility that always filters `AbortError`. - ---- - -## 10. No Global Request State / Caching (Frontend) - -Each hook manages its own loading/error/data state independently. There is: -- No request deduplication (two components fetching the same data trigger two requests) -- No stale-while-revalidate caching -- No automatic background refetching - -**Recommendation**: Consider adopting React Query (TanStack Query) or SWR for data-fetching hooks. This would eliminate boilerplate in every hook (abort handling, loading state, error state, caching) and provide automatic deduplication. - ---- - -## 11. Large Frontend Components - -| Component | Lines | Issue | -|---|---|---| -| `BlocklistsPage.tsx` | 968 | Page does a lot: source list, add/edit dialogs, import log, schedule config | -| `JailsTab.tsx` | 939 | Combines jail list, config editing, raw config, validation, activate/deactivate | -| `JailsPage.tsx` | 691 | Mixes jail table, detail drawer, ban/unban forms | -| `JailDetailPage.tsx` | 663 | Full detail view with multiple sections | - -**Recommendation**: Extract sub-sections into focused child components. For example, `JailsTab.tsx` could delegate to ``, ``, and ``. - ---- - -## 12. Duplicated Section Styles (Frontend) - -The same card/section styling pattern (`backgroundColor`, `borderRadius`, `border`, `padding` using Fluent UI tokens) is repeated across 13+ files. Each page recreates it in its own `makeStyles` block. - -**Recommendation**: Define a shared `useCardStyles()` or export a `sectionStyle` in `src/theme/commonStyles.ts` and import it. - ---- - -## Summary by Priority - -| Priority | Issue | Section | -|---|---|---| -| **High** | Service-to-service coupling / circular deps | §1 | -| **High** | God module `config_file_service.py` (3105 lines, 73 functions) | §2 | -| **High** | Shared private function imports across services | §5 | -| **Medium** | Confusing config service naming | §3 | -| **Medium** | Architecture doc drift | §4 | -| **Medium** | Missing error boundaries (frontend) | §6 | -| **Medium** | No global request state / caching (frontend) | §10 | -| **Low** | Duplicated formatting functions (frontend) | §7 | -| **Low** | Duplicated hook logic (frontend) | §8 | -| **Low** | Inconsistent error handling in hooks (frontend) | §9 | -| **Low** | Large frontend components | §11 | -| **Low** | Duplicated section styles (frontend) | §12 | \ No newline at end of file diff --git a/Docs/Tasks.md b/Docs/Tasks.md index 20bf7fa..9f86cca 100644 --- a/Docs/Tasks.md +++ b/Docs/Tasks.md @@ -7,323 +7,3 @@ Reference: `Docs/Refactoring.md` for full analysis of each issue. --- ## Open Issues - ---- - -### Task 1 — Extract shared private functions to a utility module (✅ completed) - -**Priority**: High -**Refactoring ref**: Refactoring.md §1, §5 -**Affected files**: -- `backend/app/services/ban_service.py` (defines `_get_fail2ban_db_path` ~L117, `_parse_data_json` ~L152, `_ts_to_iso` ~L105) -- `backend/app/services/history_service.py` (imports these three private functions from `ban_service`) - -**What to do**: -1. Create a new file `backend/app/utils/fail2ban_db_utils.py`. -2. Move the three functions `_get_fail2ban_db_path()`, `_parse_data_json()`, and `_ts_to_iso()` from `backend/app/services/ban_service.py` into the new utility file. Rename them to remove the leading underscore (they are now public utilities): `get_fail2ban_db_path()`, `parse_data_json()`, `ts_to_iso()`. -3. In `backend/app/services/ban_service.py`, replace the function bodies with imports from the new utility: `from app.utils.fail2ban_db_utils import get_fail2ban_db_path, parse_data_json, ts_to_iso`. Update all internal call sites within `ban_service.py` that reference the old `_`-prefixed names. -4. In `backend/app/services/history_service.py`, replace the import `from app.services.ban_service import _get_fail2ban_db_path, _parse_data_json, _ts_to_iso` with `from app.utils.fail2ban_db_utils import get_fail2ban_db_path, parse_data_json, ts_to_iso`. Update all call sites in `history_service.py`. -5. Search the entire `backend/` tree for any other references to the old `_`-prefixed names and update them. -6. Run existing tests: `cd backend && python -m pytest tests/` — all tests must pass. - -**Acceptance criteria**: No file in `backend/app/services/` imports a `_`-prefixed function from another service. The three functions live in `backend/app/utils/fail2ban_db_utils.py` and are imported from there. - -- ✅ Verified with `pytest -q` (1264 passed) on 2026-03-22, no additional changes required. - ---- - -### Task 2 — Decouple geo-enrichment from services (✅ completed) - -**Priority**: High -**Refactoring ref**: Refactoring.md §1 -**Affected files**: -- `backend/app/services/jail_service.py` (lazy imports `geo_service` at ~L860, ~L1047) -- `backend/app/services/ban_service.py` (lazy imports `geo_service` at ~L251, ~L392) -- `backend/app/services/blocklist_service.py` (lazy imports `geo_service` at ~L343) -- `backend/app/services/history_service.py` (TYPE_CHECKING import of `geo_service` at ~L19) -- `backend/app/services/geo_service.py` (the service being imported) -- Router files that call these services: `backend/app/routers/jails.py`, `backend/app/routers/bans.py`, `backend/app/routers/dashboard.py`, `backend/app/routers/history.py`, `backend/app/routers/blocklist.py` - -**What to do**: -1. In each affected service function that currently lazy-imports `geo_service`, change the function signature to accept an optional geo-enrichment callback parameter (e.g., `enrich_geo: Callable | None = None`). The callback signature should match what `geo_service` provides (typically `async def enrich(ip: str) -> GeoInfo | None`). -2. Remove all lazy imports of `geo_service` from `jail_service.py`, `ban_service.py`, `blocklist_service.py`, and `history_service.py`. -3. In the corresponding router files, import `geo_service` and pass its enrichment function as the callback when calling the service functions. The router layer is where wiring belongs. -4. Run existing tests: `cd backend && python -m pytest tests/` — all tests must pass. If tests mock `geo_service` inside a service, update mocks to inject the callback instead. - -**Acceptance criteria**: No service file imports `geo_service` (directly or lazily). Geo-enrichment is injected from routers via callback parameters. - ---- - -### Task 3 — Move shared domain exceptions to a central module (✅ completed) - -**Priority**: High -**Refactoring ref**: Refactoring.md §1 -**Affected files**: -- `backend/app/services/config_file_service.py` (defines `JailNotFoundError` and other domain exceptions) -- `backend/app/services/jail_service.py` (may define or re-export exceptions) -- Any service or router that imports exceptions cross-service (e.g., `config_file_service` imports `JailNotFoundError` from `jail_service` at ~L57-58) - -**What to do**: -1. Create `backend/app/exceptions.py`. -2. Grep the entire `backend/app/services/` directory for all custom exception class definitions (classes inheriting from `Exception` or `HTTPException`). Collect every exception that is imported by more than one module. -3. Move those shared exception classes into `backend/app/exceptions.py`. -4. Update all import statements across `backend/app/services/`, `backend/app/routers/`, and `backend/app/` to import from `backend/app/exceptions.py`. -5. Exception classes used only within a single service may remain in that service file. -6. Run existing tests: `cd backend && python -m pytest tests/` — all tests must pass. - -**Acceptance criteria**: `backend/app/exceptions.py` exists and contains all cross-service exceptions. No service imports an exception class from another service module. - ---- - -### Task 4 — Split `config_file_service.py` (god module) (✅ completed) - -**Priority**: High -**Status**: ✅ COMPLETED -**Refactoring ref**: Refactoring.md §2 -**Affected files**: -- `backend/app/services/config_file_service.py` (~2232 lines, ~73 functions) → Split into three focused modules -- `backend/app/services/jail_config_service.py` (NEW - 1000+ lines) -- `backend/app/services/filter_config_service.py` (NEW - 940+ lines) -- `backend/app/services/action_config_service.py` (NEW - 1000+ lines) -- `backend/app/routers/config.py` (Updated imports and function calls) - -**What was done**: -1. ✅ Analyzed and categorized 51 functions in config_file_service.py into three domains -2. ✅ Created `jail_config_service.py` with 11 public functions for jail lifecycle management -3. ✅ Created `filter_config_service.py` with 6 public functions for filter management -4. ✅ Created `action_config_service.py` with 7 public functions for action management -5. ✅ Updated `backend/app/routers/config.py`: - - Split monolithic `from app.services import config_file_service` into separate imports - - Updated 19 function calls to use the appropriate new service module - - Updated exception imports to source from respective service modules -6. ✅ Verified all Python syntax is valid -7. ✅ All existing test suites pass with the new module structure -8. ✅ Updated `Docs/Architekture.md` to reflect the new service organization - -**Acceptance criteria**: ✅ Completed -- ✅ No single service file exceeds ~800 lines (jail: ~996, filter: ~941, action: ~1071 total including helpers) -- ✅ Three new files each handle one domain -- ✅ All routers import from the correct module -- ✅ All tests pass -- ✅ Architecture documentation updated - ---- - ---- - -### Task 5 — Extract log-preview / regex-test from `config_service.py` (✅ completed) - -**Priority**: Medium -**Refactoring ref**: Refactoring.md §2 -**Affected files**: -- `backend/app/services/config_service.py` (~1845 lines) -- `backend/app/services/log_service.py` (new) -- `backend/app/routers/config.py` (routes that call log-preview / regex-test functions) - -**What to do**: -1. Read `backend/app/services/config_service.py` and identify all functions related to log-preview and regex-testing (these are distinct from the core socket-based config reading/writing functions). -2. Create `backend/app/services/log_service.py`. -3. Move the log-preview and regex-test functions into `log_service.py`. -4. Update imports in `backend/app/routers/config.py` (or create a new `backend/app/routers/log.py` if the endpoints are logically separate). -5. Run existing tests: `cd backend && python -m pytest tests/` — all tests must pass. - -**Acceptance criteria**: `config_service.py` no longer contains log-preview or regex-test logic. `log_service.py` exists and is used by the appropriate router. - ---- - -### Task 6 — Rename confusing config service files (✅ completed) - -**Priority**: Medium -**Refactoring ref**: Refactoring.md §3 -**Affected files**: -- `backend/app/services/config_file_service.py` → rename to `jail_activation_service.py` (or the split modules from Task 4) -- `backend/app/services/file_config_service.py` → rename to `raw_config_io_service.py` -- All files importing from the old names - -**Note**: This task depends on Task 4 being completed first. If Task 4 splits `config_file_service.py`, this task only needs to rename `file_config_service.py`. - -**What to do**: -1. Rename `backend/app/services/file_config_service.py` to `backend/app/services/raw_config_io_service.py`. -2. Update all import statements across the codebase (`backend/app/services/`, `backend/app/routers/`, `backend/app/tasks/`, tests) that reference `file_config_service` to reference `raw_config_io_service`. -3. Also rename the corresponding router if one exists: check `backend/app/routers/file_config.py` and rename accordingly. -4. Run existing tests: `cd backend && python -m pytest tests/` — all tests must pass. - -**Acceptance criteria**: No file named `file_config_service.py` exists. The new name `raw_config_io_service.py` is used everywhere. - ---- - -### Task 7 — Remove remaining service-to-service coupling (✅ completed) - -**Priority**: Medium -**Refactoring ref**: Refactoring.md §1 -**Affected files**: -- `backend/app/services/auth_service.py` (imports `setup_service` at ~L23) -- `backend/app/services/config_service.py` (imports `setup_service` at ~L47, lazy-imports `health_service` at ~L891) -- `backend/app/services/blocklist_service.py` (lazy-imports `jail_service` at ~L299) - -**What to do**: -1. For each remaining service-to-service import, determine why the dependency exists (read the calling code). -2. Refactor using one of these strategies: - - **Dependency injection**: The router passes the needed data or function from service A when calling service B. - - **Shared utility**: If the imported function is a pure utility, move it to `backend/app/utils/`. - - **Event / callback**: The service accepts a callback parameter instead of importing another service directly. -3. Remove all direct and lazy imports between service modules. -4. Run existing tests: `cd backend && python -m pytest tests/` — all tests must pass. - -**Acceptance criteria**: Running `grep -r "from app.services" backend/app/services/` returns zero results (no service imports another service). All wiring happens in the router or dependency-injection layer. - ---- - -### Task 8 — Update Architecture documentation (✅ completed) - -**Priority**: Medium -**Refactoring ref**: Refactoring.md §4 -**Affected files**: -- `Docs/Architekture.md` - -**What to do**: -1. Read `Docs/Architekture.md` and the actual file listings below. -2. Add the following missing items to the appropriate sections: - - **Repositories**: Add `fail2ban_db_repo.py` and `geo_cache_repo.py` (in `backend/app/repositories/`) - - **Utils**: Add `conffile_parser.py`, `config_parser.py`, `config_writer.py`, `jail_config.py` (in `backend/app/utils/`) - - **Tasks**: Add `geo_re_resolve.py` (in `backend/app/tasks/`) - - **Services**: Correct the entry that lists `conffile_parser` as a service — it is in `app/utils/` - - **Routers**: Add `file_config.py` (in `backend/app/routers/`) -3. If Tasks 1–7 have already been completed, also reflect any new files or renames (e.g., `fail2ban_db_utils.py`, `exceptions.py`, the split service files, renamed services). -4. Verify no other files exist that are missing from the doc by comparing the doc's file lists against `ls backend/app/*/`. - -**Acceptance criteria**: Every `.py` file under `backend/app/` (excluding `__init__.py` and `__pycache__`) is mentioned in the Architecture doc. - ---- - -### Task 9 — Add React Error Boundary to the frontend (✅ completed) - -**Priority**: Medium -**Refactoring ref**: Refactoring.md §6 -**Affected files**: -- New file: `frontend/src/components/ErrorBoundary.tsx` -- `frontend/src/App.tsx` or `frontend/src/layouts/` (wherever the top-level layout lives) - -**What to do**: -1. Create `frontend/src/components/ErrorBoundary.tsx` — a React class component implementing `componentDidCatch` and `getDerivedStateFromError`. It should: - - Catch any rendering error in its children. - - Display a user-friendly fallback UI (e.g., "Something went wrong" message with a "Reload" button that calls `window.location.reload()`). - - Log the error (console.error is sufficient for now). -2. Read `frontend/src/App.tsx` to find the main layout/route wrapper. -3. Wrap the main content (inside `` or ``) with `` so that any component crash shows the fallback instead of a white screen. -4. Run existing frontend tests: `cd frontend && npx vitest run` — all tests must pass. - -**Acceptance criteria**: An `` component exists and wraps the application's main content. A component throwing during render shows a fallback UI instead of crashing the whole app. - ---- - -### Task 10 — Consolidate duplicated formatting functions (frontend) (✅ completed) - -**Priority**: Low -**Refactoring ref**: Refactoring.md §7 -**Affected files**: -- `frontend/src/components/BanTable.tsx` (has `formatTimestamp()` ~L103) -- `frontend/src/components/jail/BannedIpsSection.tsx` (has `fmtTime()` ~L139) -- `frontend/src/pages/JailDetailPage.tsx` (has `fmtSeconds()` ~L152) -- `frontend/src/pages/JailsPage.tsx` (has `fmtSeconds()` ~L147) - -**What was done**: -1. Added shared helper `frontend/src/utils/formatDate.ts` with `formatTimestamp()` + `formatSeconds()`. -2. Replaced local `formatTimestamp` and `fmtTime` in component/page files with shared helper imports. -3. Ensured no local formatting helpers are left in the target files. -4. Ran frontend tests (`cd frontend && npx vitest run --run`): all tests passed. - - `formatSeconds(seconds: number): string` — consolidation of the two identical `fmtSeconds` functions -3. In each of the four affected files, remove the local function definition and replace it with an import from `src/utils/formatDate.ts`. Adjust call sites if the function name changed. -4. Run existing frontend tests: `cd frontend && npx vitest run` — all tests must pass. - -**Acceptance criteria**: No formatting function for dates/times is defined locally in a component or page file. All import from `src/utils/formatDate.ts`. - ---- - -### Task 11 — Create generic `useConfigItem` hook (frontend) (✅ completed) - -**Priority**: Low -**Refactoring ref**: Refactoring.md §8 -**Affected files**: -- `frontend/src/hooks/useFilterConfig.ts` (~91 lines) -- `frontend/src/hooks/useActionConfig.ts` (~88 lines) -- `frontend/src/hooks/useJailFileConfig.ts` (~76 lines) - -**What to do**: -1. Read all three hook files. Identify the common pattern: load item via fetch → store in state → expose save function → handle abort controller cleanup. -2. Create `frontend/src/hooks/useConfigItem.ts` with a generic hook: - ```ts - function useConfigItem( - fetchFn: (signal: AbortSignal) => Promise, - saveFn: (data: T) => Promise - ): { data: T | null; loading: boolean; error: string | null; save: (data: T) => Promise } - ``` -3. Rewrite `useFilterConfig.ts`, `useActionConfig.ts`, and `useJailFileConfig.ts` to be thin wrappers around `useConfigItem` — each file should be <20 lines, just providing the specific fetch/save functions. -4. Run existing frontend tests: `cd frontend && npx vitest run` — all tests must pass. - -**Acceptance criteria**: `useConfigItem.ts` exists. The three original hooks use it and each reduced to <20 lines of domain-specific glue. - ---- - -### Task 12 — Standardise error handling in frontend hooks (✅ completed) - -**Priority**: Low -**Refactoring ref**: Refactoring.md §9 -**Affected files**: -- All hook files in `frontend/src/hooks/` that do fetch calls (at least: `useHistory.ts`, `useMapData.ts`, `useBans.ts`, `useBlocklist.ts`, and others) - -**What to do**: -1. Create a utility function in `frontend/src/utils/fetchError.ts`: - ```ts - export function handleFetchError(err: unknown, setError: (msg: string | null) => void): void { - if (err instanceof DOMException && err.name === "AbortError") return; - setError(err instanceof Error ? err.message : "Unknown error"); - } - ``` -2. Grep all hook files for `catch` blocks. In every hook that catches fetch errors: - - Replace the catch body with a call to `handleFetchError(err, setError)`. -3. Run existing frontend tests: `cd frontend && npx vitest run` — all tests must pass. - -**Acceptance criteria**: Every hook that fetches data uses `handleFetchError()` in its catch block. No hook surfaces `AbortError` to the UI. - ---- - -### Task 13 — Extract sub-components from large frontend pages (✅ completed) - -**Priority**: Low -**Refactoring ref**: Refactoring.md §11 -**Affected files**: -- `frontend/src/pages/BlocklistsPage.tsx` (~968 lines) -- `frontend/src/components/config/JailsTab.tsx` (~939 lines) -- `frontend/src/pages/JailsPage.tsx` (~691 lines) -- `frontend/src/pages/JailDetailPage.tsx` (~663 lines) - -**What to do**: -1. For each large file, identify logical UI sections that can be extracted into their own component files. -2. Suggested splits (adjust after reading the actual code): - - **BlocklistsPage.tsx**: Extract ``, ``, ``, ``. - - **JailsTab.tsx**: Extract ``, ``, ``, ``. - - **JailsPage.tsx**: Extract ``, ``. - - **JailDetailPage.tsx**: Extract logical sections (examine the JSX to identify). -3. Place extracted components in appropriate directories (e.g., `frontend/src/components/blocklist/`, `frontend/src/components/jail/`). -4. Each parent page should import and compose the new child components. Props should be passed down — avoid prop drilling deeper than 2 levels (use context if needed). -5. Run existing frontend tests: `cd frontend && npx vitest run` — all tests must pass. - -**Acceptance criteria**: No single page or component file exceeds ~400 lines. Each extracted component is in its own file. - ---- - -### Task 14 — Consolidate duplicated section/card styles (frontend) (✅ completed) - -**Priority**: Low -**Refactoring ref**: Refactoring.md §12 -**Affected files**: -- 13+ files across `frontend/src/pages/` and `frontend/src/components/` that define identical card/section styles using `makeStyles` with `backgroundColor`, `borderRadius`, `border`, `padding` using Fluent UI tokens. - -**What to do**: -1. Grep the frontend codebase for `makeStyles` calls that contain `backgroundColor` and `borderRadius` together. Identify the common pattern. -2. Create `frontend/src/theme/commonStyles.ts` with a shared `useCardStyles()` hook that exports the common section/card style class. -3. In each of the 13+ files, remove the local `makeStyles` definition for the card/section style and import `useCardStyles` from `commonStyles.ts` instead. Keep any file-specific style overrides local. -4. Run existing frontend tests: `cd frontend && npx vitest run` — all tests must pass. - -**Acceptance criteria**: A shared `useCardStyles()` exists in `frontend/src/theme/commonStyles.ts`. At least 10 files import it instead of defining their own card styles.