diff --git a/Docs/Tasks.md b/Docs/Tasks.md index 078b54a..482967a 100644 --- a/Docs/Tasks.md +++ b/Docs/Tasks.md @@ -264,3 +264,59 @@ Every IP that has a valid geographic mapping should display its country. Failed | Frontend | `frontend/src/pages/MapPage.tsx` | | Tests | `backend/tests/test_services/test_geo_service.py` | | Tests | `backend/tests/test_routers/test_geo.py` | + +--- + +## Task 5 — Blocklist Import Error Badge in Navigation ✅ DONE + +**Completed:** Added `last_run_errors: bool | None` to `ScheduleInfo` model and updated `get_schedule_info()` to derive it from the last import log entry. Frontend: added `last_run_errors` to `ScheduleInfo` type; added `useBlocklistStatus` hook that polls `GET /api/blocklists/schedule` every 60 s; `MainLayout` renders a warning `MessageBar` and an amber badge on the Blocklists nav item when `hasErrors` is `true`. Tests: 3 new service tests + 1 new router test; 433 tests pass. + +### Problem + +[Features.md § 8](Features.md) requires: *"Show a warning badge in the navigation if the most recent import encountered errors"* and *"Notify the user (via the UI status bar) when a scheduled import fails so it does not go unnoticed."* + +Currently `ScheduleInfo` (returned by `GET /api/blocklists/schedule`) contains `last_run_at` but no indicator of whether the last run had errors. The `MainLayout` sidebar only warns about fail2ban being offline; there is no blocklist-import failure indicator anywhere in the shell. + +### Goal + +When the most recent blocklist import run completed with errors, a warning indicator must be visible in the persistent app shell until the condition clears (i.e. a successful import runs). Concretely: + +1. A warning `MessageBar` appears at the top of the main content area (alongside the existing fail2ban-offline bar). +2. A small warning badge is rendered on the **Blocklists** navigation item in the sidebar. + +### Implementation Details + +**Backend — expose error flag in `ScheduleInfo`** + +1. **`app/models/blocklist.py`** — Add `last_run_errors: bool | None = None` to `ScheduleInfo`. `True` means the most recent run's `errors` column was non-null; `None` means no run has ever completed. +2. **`app/services/blocklist_service.py`** — In `get_schedule_info()`, after fetching `last_log`, set `last_run_errors = last_log["errors"] is not None` when `last_log` is not `None`, else leave it as `None`. + +**Frontend — poll and display** + +3. **`frontend/src/types/blocklist.ts`** — Add `last_run_errors: boolean | null` to `ScheduleInfo`. +4. **`frontend/src/hooks/useBlocklist.ts`** — Add a new exported hook `useBlocklistStatus` that polls `GET /api/blocklists/schedule` every 60 seconds (plus on mount) and returns `{ hasErrors: boolean }`. Errors from the poll itself should not surface to the user — silently treat as "unknown". +5. **`frontend/src/layouts/MainLayout.tsx`**: + - Import and call `useBlocklistStatus`. + - When `hasErrors` is `true`, render a second `MessageBar` (intent `"warning"`) in the warning-bar slot describing the blocklist import failure. + - Add a small amber `Badge` (number `!` or just the dot shape) to the Blocklists `NavLink` entry so users see the indicator even when they're on another page. + +**Tests** + +6. **`backend/tests/test_services/test_blocklist_service.py`** — Three new tests in `TestSchedule`: + - `test_get_schedule_info_no_errors_when_log_has_no_errors` — inserts a successful import log entry (errors=None), asserts `info.last_run_errors is False`. + - `test_get_schedule_info_errors_when_log_has_errors` — inserts a log entry with a non-null `errors` string, asserts `info.last_run_errors is True`. + - `test_get_schedule_info_none_when_no_log` — already covered by the existing test; verify it now also asserts `info.last_run_errors is None`. +7. **`backend/tests/test_routers/test_blocklist.py`** — One new test in `TestGetSchedule`: + - `test_schedule_response_includes_last_run_errors` — patches `get_schedule_info` to return a `ScheduleInfo` with `last_run_errors=True`, confirms the JSON field is present and `True`. + +### Files Touched + +| Layer | File | +|-------|------| +| Model | `backend/app/models/blocklist.py` | +| Service | `backend/app/services/blocklist_service.py` | +| Frontend type | `frontend/src/types/blocklist.ts` | +| Frontend hook | `frontend/src/hooks/useBlocklist.ts` | +| Frontend layout | `frontend/src/layouts/MainLayout.tsx` | +| Tests | `backend/tests/test_services/test_blocklist_service.py` | +| Tests | `backend/tests/test_routers/test_blocklist.py` | diff --git a/backend/app/models/blocklist.py b/backend/app/models/blocklist.py index 66e7bd2..946b340 100644 --- a/backend/app/models/blocklist.py +++ b/backend/app/models/blocklist.py @@ -133,6 +133,8 @@ class ScheduleInfo(BaseModel): config: ScheduleConfig next_run_at: str | None last_run_at: str | None + last_run_errors: bool | None = None + """``True`` if the most recent import had errors, ``False`` if clean, ``None`` if never run.""" # --------------------------------------------------------------------------- diff --git a/backend/app/services/blocklist_service.py b/backend/app/services/blocklist_service.py index 33195cc..5df05f0 100644 --- a/backend/app/services/blocklist_service.py +++ b/backend/app/services/blocklist_service.py @@ -480,7 +480,13 @@ async def get_schedule_info( config = await get_schedule(db) last_log = await import_log_repo.get_last_log(db) last_run_at = last_log["timestamp"] if last_log else None - return ScheduleInfo(config=config, next_run_at=next_run_at, last_run_at=last_run_at) + last_run_errors: bool | None = (last_log["errors"] is not None) if last_log else None + return ScheduleInfo( + config=config, + next_run_at=next_run_at, + last_run_at=last_run_at, + last_run_errors=last_run_errors, + ) # --------------------------------------------------------------------------- diff --git a/backend/tests/test_routers/test_blocklist.py b/backend/tests/test_routers/test_blocklist.py index b821562..354bfcc 100644 --- a/backend/tests/test_routers/test_blocklist.py +++ b/backend/tests/test_routers/test_blocklist.py @@ -379,6 +379,31 @@ class TestGetSchedule: assert "next_run_at" in body assert "last_run_at" in body + async def test_schedule_response_includes_last_run_errors( + self, bl_client: AsyncClient + ) -> None: + """GET /api/blocklists/schedule includes last_run_errors field.""" + info_with_errors = ScheduleInfo( + config=ScheduleConfig( + frequency=ScheduleFrequency.daily, + interval_hours=24, + hour=3, + minute=0, + day_of_week=0, + ), + next_run_at=None, + last_run_at="2026-03-01T03:00:00+00:00", + last_run_errors=True, + ) + with patch( + "app.routers.blocklist.blocklist_service.get_schedule_info", + new=AsyncMock(return_value=info_with_errors), + ): + resp = await bl_client.get("/api/blocklists/schedule") + body = resp.json() + assert "last_run_errors" in body + assert body["last_run_errors"] is True + # --------------------------------------------------------------------------- # PUT /api/blocklists/schedule diff --git a/backend/tests/test_services/test_blocklist_service.py b/backend/tests/test_services/test_blocklist_service.py index 633fcfe..728fb3c 100644 --- a/backend/tests/test_services/test_blocklist_service.py +++ b/backend/tests/test_services/test_blocklist_service.py @@ -254,7 +254,42 @@ class TestSchedule: assert loaded.interval_hours == 6 async def test_get_schedule_info_no_log(self, db: aiosqlite.Connection) -> None: - """get_schedule_info returns None for last_run_at when no log exists.""" + """get_schedule_info returns None for last_run_at and last_run_errors when no log exists.""" info = await blocklist_service.get_schedule_info(db, None) assert info.last_run_at is None assert info.next_run_at is None + assert info.last_run_errors is None + + async def test_get_schedule_info_no_errors_when_clean( + self, db: aiosqlite.Connection + ) -> None: + """get_schedule_info returns last_run_errors=False when the last run had no errors.""" + from app.repositories import import_log_repo + + await import_log_repo.add_log( + db, + source_id=None, + source_url="https://example.test/ips.txt", + ips_imported=10, + ips_skipped=0, + errors=None, + ) + info = await blocklist_service.get_schedule_info(db, None) + assert info.last_run_errors is False + + async def test_get_schedule_info_errors_flag_when_failed( + self, db: aiosqlite.Connection + ) -> None: + """get_schedule_info returns last_run_errors=True when the last run had errors.""" + from app.repositories import import_log_repo + + await import_log_repo.add_log( + db, + source_id=None, + source_url="https://example.test/ips.txt", + ips_imported=0, + ips_skipped=0, + errors="Connection timeout", + ) + info = await blocklist_service.get_schedule_info(db, None) + assert info.last_run_errors is True diff --git a/frontend/src/hooks/useBlocklist.ts b/frontend/src/hooks/useBlocklist.ts index a9421b7..9d64f64 100644 --- a/frontend/src/hooks/useBlocklist.ts +++ b/frontend/src/hooks/useBlocklist.ts @@ -235,3 +235,51 @@ export function useRunImport(): UseRunImportReturn { return { running, lastResult, error, runNow }; } + +// --------------------------------------------------------------------------- +// useBlocklistStatus +// --------------------------------------------------------------------------- + +/** How often to re-check the schedule endpoint for import errors (ms). */ +const BLOCKLIST_POLL_INTERVAL_MS = 60_000; + +export interface UseBlocklistStatusReturn { + /** `true` when the most recent import run completed with errors. */ + hasErrors: boolean; +} + +/** + * Poll `GET /api/blocklists/schedule` every 60 seconds to detect whether + * the most recent blocklist import had errors. + * + * Network failures during polling are silently ignored — the indicator + * simply retains its previous value until the next successful poll. + */ +export function useBlocklistStatus(): UseBlocklistStatusReturn { + const [hasErrors, setHasErrors] = useState(false); + + useEffect(() => { + let cancelled = false; + + const poll = (): void => { + fetchSchedule() + .then((info) => { + if (!cancelled) { + setHasErrors(info.last_run_errors === true); + } + }) + .catch(() => { + // Silently swallow network errors — do not change indicator state. + }); + }; + + poll(); + const id = window.setInterval(poll, BLOCKLIST_POLL_INTERVAL_MS); + return (): void => { + cancelled = true; + window.clearInterval(id); + }; + }, []); + + return { hasErrors }; +} diff --git a/frontend/src/layouts/MainLayout.tsx b/frontend/src/layouts/MainLayout.tsx index 6dc4552..894dba1 100644 --- a/frontend/src/layouts/MainLayout.tsx +++ b/frontend/src/layouts/MainLayout.tsx @@ -8,6 +8,7 @@ import { useCallback, useEffect, useState } from "react"; import { + Badge, Button, makeStyles, mergeClasses, @@ -31,6 +32,7 @@ import { import { NavLink, Outlet, useNavigate } from "react-router-dom"; import { useAuth } from "../providers/AuthProvider"; import { useServerStatus } from "../hooks/useServerStatus"; +import { useBlocklistStatus } from "../hooks/useBlocklist"; // --------------------------------------------------------------------------- // Styles @@ -100,6 +102,12 @@ const useStyles = makeStyles({ flexGrow: 1, }, + navLinkContent: { + display: "flex", + alignItems: "center", + gap: tokens.spacingHorizontalS, + flexGrow: 1, + }, navLink: { display: "flex", alignItems: "center", @@ -199,6 +207,7 @@ export function MainLayout(): React.JSX.Element { // with the icon-only sidebar rather than the full-width one. const [collapsed, setCollapsed] = useState(() => window.innerWidth < 640); const { status } = useServerStatus(); + const { hasErrors: blocklistHasErrors } = useBlocklistStatus(); /** True only after the first successful poll and fail2ban is unreachable. */ const serverOffline = status !== null && !status.online; @@ -249,32 +258,45 @@ export function MainLayout(): React.JSX.Element { {/* Nav links */} {/* Footer — Logout */} @@ -313,6 +335,18 @@ export function MainLayout(): React.JSX.Element { )} + {/* Blocklist import error warning — shown when the last scheduled import had errors */} + {blocklistHasErrors && ( +
+ + + Blocklist Import Errors + The most recent blocklist import encountered errors. Check the + Blocklists page for details. + + +
+ )}
diff --git a/frontend/src/types/blocklist.ts b/frontend/src/types/blocklist.ts index 976f31d..c240ef8 100644 --- a/frontend/src/types/blocklist.ts +++ b/frontend/src/types/blocklist.ts @@ -71,6 +71,8 @@ export interface ScheduleInfo { config: ScheduleConfig; next_run_at: string | null; last_run_at: string | null; + /** `true` if the most recent import had errors, `false` if clean, `null` if never run. */ + last_run_errors: boolean | null; } // ---------------------------------------------------------------------------