Add country-specific companion table filtering for map page
This commit is contained in:
156
Docs/Tasks.md
156
Docs/Tasks.md
@@ -8,71 +8,123 @@ Reference: `Docs/Refactoring.md` for full analysis of each issue.
|
||||
|
||||
## Open Issues
|
||||
|
||||
### Backend Architecture
|
||||
---
|
||||
|
||||
- **Replace the single shared SQLite connection.**
|
||||
- Current startup code opens one `aiosqlite.Connection` and reuses it for every request.
|
||||
- This should be replaced with either a connection pool or request-scoped connections to avoid concurrency and locking issues.
|
||||
- Update request dependencies, application lifecycle, and tests to use the new pattern.
|
||||
### TASK-001 — WorldMap: filter companion table by selected country (server-side)
|
||||
|
||||
- **Refactor dependency wiring and shared resource management.**
|
||||
- Remove hidden module-level import coupling between routers, services, and shared utilities.
|
||||
- Introduce explicit factories or providers for shared resources such as DB, HTTP client session, scheduler, and settings.
|
||||
- Ensure routers depend on injected providers rather than global state or dynamic imports.
|
||||
**Status:** Done
|
||||
**Priority:** Medium
|
||||
**Domain:** Full-stack (backend + frontend)
|
||||
**References:** `Docs/Features.md §4`, `Docs/Web-Development.md`
|
||||
|
||||
- **Harden fail2ban integration.**
|
||||
- Remove the `sys.path` hack that locates `fail2ban-master` at runtime.
|
||||
- Replace it with a deterministic packaging or configuration model so the backend does not depend on repository layout.
|
||||
- Refactor `Fail2BanClient` so concurrency control is instance-based and not backed by hidden module globals.
|
||||
#### Background
|
||||
|
||||
- **Improve startup / setup guard behavior.**
|
||||
- Convert `SetupRedirectMiddleware` from an on-demand DB check into a startup/initialisation guard where possible.
|
||||
- Cache setup completion in a safe way and provide an explicit invalidation path if the application state changes.
|
||||
- Reduce middleware responsibility and avoid DB access during normal request dispatch.
|
||||
The `GET /api/dashboard/bans/by-country` endpoint always returns the **200 most recent** ban rows in `bans` (constant `_MAX_COMPANION_BANS = 200` in `backend/app/services/ban_service.py`). `MapPage.tsx` stores a `selectedCountry` state and filters the returned rows client-side via `visibleBans`. This means the companion table can only show the fraction of a country's bans that fall within the global top-200 window. If the selected time range has, say, 1 500 bans and 300 are from China, but China's bans are not all in the top 200 overall, the table will silently display fewer than 300 rows.
|
||||
|
||||
- **Make deployment configuration explicit.**
|
||||
- Move hard-coded environment assumptions such as CORS origins into settings.
|
||||
- Ensure `fail2ban_socket`, `fail2ban_config_dir`, and startup commands are fully configurable via `Settings`.
|
||||
- Document production-ready defaults separately from development defaults.
|
||||
When a country is selected the companion table **must** return the complete set of bans for that country so the user sees an accurate picture.
|
||||
|
||||
### Reliability and Resilience
|
||||
#### Desired behaviour
|
||||
|
||||
- **Add backend lifecycle tests for resource cleanup.**
|
||||
- Verify startup opens and initialises DB, HTTP session, scheduler, and geo cache correctly.
|
||||
- Verify shutdown closes those resources cleanly.
|
||||
- No country selected → companion table shows the 200 most recent bans across all countries (existing behaviour, no change).
|
||||
- Country selected → the server returns **all** ban entries for that country in the selected time window; no client-side row-count cap applies.
|
||||
- Deselecting a country (clicking the same country again, or the "Clear filter" button) reverts to the default 200-row unfiltered view.
|
||||
- The existing `visibleBans` client-side filter in `MapPage.tsx` can remain as a defensive guard but must not be the only filter.
|
||||
|
||||
- **Add concurrency/regression coverage for DB and fail2ban socket use.**
|
||||
- Add tests that simulate multiple concurrent requests using the same DB dependency.
|
||||
- Add tests around fail2ban socket retries, protocol errors, and rate limiting.
|
||||
#### Implementation steps
|
||||
|
||||
- **Improve state caching and invalidation.**
|
||||
- Add tests for session cache invalidation on logout.
|
||||
- Add tests for setup completion caching so stale state is never served.
|
||||
1. **Backend — router** (`backend/app/routers/dashboard.py`)
|
||||
- Add `country_code: str | None = Query(default=None, description="ISO alpha-2 country code to filter companion rows.")` to `get_bans_by_country`.
|
||||
- Pass it to `ban_service.bans_by_country(..., country_code=country_code)`.
|
||||
|
||||
### Backend Feature Work
|
||||
2. **Backend — service** (`backend/app/services/ban_service.py`)
|
||||
- Add `country_code: str | None = None` keyword argument to `bans_by_country`.
|
||||
- After `geo_map` is built (existing geo-resolution step), collect IPs whose resolved country matches `country_code`.
|
||||
- For the **fail2ban source**: call `fail2ban_db_repo.get_currently_banned` with `ip_filter=matched_ips` and no `limit` (remove the `_MAX_COMPANION_BANS` cap for filtered queries).
|
||||
- For the **archive source**: filter `all_rows` to those whose IP is in `matched_ips` and return all of them (skip the `page_size=_MAX_COMPANION_BANS` call).
|
||||
- When `country_code` is `None`, behaviour is identical to today.
|
||||
|
||||
- **Document and implement backend-safe environment-driven CORS.**
|
||||
- Add support for production and local development origins through configuration.
|
||||
- Avoid a hardcoded Vite origin in the core app factory.
|
||||
3. **Backend — repository** (`backend/app/repositories/fail2ban_db_repo.py`)
|
||||
- Add `ip_filter: list[str] | None = None` to `get_currently_banned`.
|
||||
- When provided and non-empty, append `AND ip IN ({placeholders})` to the SQL `WHERE` clause, parameterised safely (never interpolated as a string).
|
||||
|
||||
- **Centralise scheduler job registration.**
|
||||
- Refactor APScheduler registration so background tasks are registered through a common lifecycle helper.
|
||||
- Ensure jobs can be discovered, replaced, and tested without requiring implicit `app.state` side effects.
|
||||
4. **Backend — repository (archive)** (`backend/app/repositories/history_archive_repo.py`)
|
||||
- Similarly add optional `ip_filter` to the archive companion-rows query used from `bans_by_country`.
|
||||
|
||||
- **Strengthen fail2ban error handling and reporting.**
|
||||
- Standardise `502` responses for connection/protocol failures across all endpoints.
|
||||
- Add structured logging for retries and fatal socket failures.
|
||||
- Ensure the UI can distinguish offline fail2ban from internal backend failures.
|
||||
5. **Frontend — API client** (`frontend/src/api/map.ts`)
|
||||
- Add optional `countryCode?: string` parameter to `fetchBansByCountry`.
|
||||
- When set, append `country_code=<value>` to the query string.
|
||||
|
||||
- **Improve documentation of backend responsibilities.**
|
||||
- Keep `Docs/Tasks.md` aligned with the backend architecture review.
|
||||
- Add references to the backend modules, resource lifecycle, and dependency model in the documentation.
|
||||
6. **Frontend — hook** (`frontend/src/hooks/useMapData.ts`)
|
||||
- Add `countryCode?: string` to the function signature.
|
||||
- Include it in the `useCallback` dependency array and pass it to `fetchBansByCountry`.
|
||||
|
||||
7. **Frontend — page** (`frontend/src/pages/MapPage.tsx`)
|
||||
- Pass `selectedCountry ?? undefined` as `countryCode` to `useMapData`.
|
||||
- The hook's effect will re-fetch automatically when `selectedCountry` changes; the existing `useEffect` that resets `page` to 1 already covers this.
|
||||
|
||||
#### Testing guidance
|
||||
|
||||
- Select a country that has > 200 bans in the chosen time window; confirm the companion table shows more than the previous cap would allow.
|
||||
- With no country selected, confirm only 200 rows are returned (no regression).
|
||||
- Deselect the country; confirm the unfiltered 200-row view is restored.
|
||||
- Test with the archive source as well as the fail2ban live source.
|
||||
- Verify the `ip_filter` SQL clause is parameterised and cannot be injected.
|
||||
|
||||
---
|
||||
|
||||
### TASK-002 — WorldMap: sticky table header and sticky pagination bar
|
||||
|
||||
**Priority:** Low
|
||||
**Domain:** Frontend only
|
||||
**References:** `Docs/Features.md §4`, `Docs/Web-Design.md`, `Docs/Web-Development.md`
|
||||
|
||||
#### Background
|
||||
|
||||
The companion ban table in `MapPage.tsx` is wrapped in `tableWrapper` (CSS `overflow: auto; maxHeight: 420px`). Both the Fluent UI `TableHeader` row and the `.pagination` div inside `tableWrapper` scroll with the content. Once the user scrolls more than a few rows, the column header labels disappear and the pagination controls become unreachable without scrolling back to the top or bottom.
|
||||
|
||||
#### Desired behaviour
|
||||
|
||||
- The column header row (`TableHeader →TableRow → TableHeaderCell × 6`) must remain fixed at the **top** of the scrollable container at all times.
|
||||
- The pagination / page-size bar (`.pagination` div at the bottom of `tableWrapper`) must remain fixed at the **bottom** of the scrollable container at all times.
|
||||
- Rows in `TableBody` scroll normally between the two fixed ends.
|
||||
- No changes to the container height, overall layout, or other pages.
|
||||
|
||||
#### Implementation steps
|
||||
|
||||
All changes are in `frontend/src/pages/MapPage.tsx`.
|
||||
|
||||
1. **Sticky table header cells**
|
||||
- In `useStyles` (`makeStyles`), add a new class:
|
||||
```ts
|
||||
stickyHeaderCell: {
|
||||
position: "sticky",
|
||||
top: 0,
|
||||
zIndex: 1,
|
||||
backgroundColor: tokens.colorNeutralBackground1,
|
||||
boxShadow: `0 1px 0 ${tokens.colorNeutralStroke2}`,
|
||||
},
|
||||
```
|
||||
- Apply `className={styles.stickyHeaderCell}` to **each** `TableHeaderCell` in the header row.
|
||||
- Note: `position: sticky` on `<tr>` elements is unreliable across browsers for table layouts; apply it to each `<th>` (`TableHeaderCell`) instead.
|
||||
|
||||
2. **Sticky pagination bar**
|
||||
- In the existing `pagination` entry in `useStyles`, add:
|
||||
```ts
|
||||
position: "sticky",
|
||||
bottom: 0,
|
||||
zIndex: 1,
|
||||
```
|
||||
- The existing `backgroundColor: tokens.colorNeutralBackground2` already prevents table rows from bleeding through.
|
||||
|
||||
3. **No other changes** — do not alter `tableWrapper`, its height, or anything outside `MapPage.tsx`.
|
||||
|
||||
#### Testing guidance
|
||||
|
||||
- Load the Map page with a time range that produces > 25 bans (enough to overflow the `420px` container).
|
||||
- Scroll down through the table and confirm the column headers remain visible at the top.
|
||||
- Scroll down and confirm the pagination bar remains visible at the bottom.
|
||||
- Verify no visual artefacts (table body rows must not overlap or bleed through the sticky elements).
|
||||
- Run `tsc --noEmit` — zero type errors expected.
|
||||
- Run existing frontend tests: `vitest run` — no regressions.
|
||||
|
||||
### Priority Execution Plan
|
||||
|
||||
1. Fix the global SQLite connection pattern and tests.
|
||||
2. Refactor dependency injection / explicit shared resources.
|
||||
3. Harden fail2ban client concurrency and packaging.
|
||||
4. Convert setup guard to a safer startup-driven model.
|
||||
5. Add deployment-safe configuration and production-ready CORS.
|
||||
6. Add lifecycle and concurrency regression tests.
|
||||
|
||||
Reference in New Issue
Block a user