BanGUI/Docs/Tasks.md

## [E2E-3] Ban records appear in UI after simulated failed logins

**Where found:**
`Docker/simulate_failed_logins.sh` exists and is used in the `make dev-ban-test` target, but there is no automated test that verifies the resulting bans are surfaced correctly in the BanGUI frontend (History page, Dashboard, or Jails detail page).

**Why this is needed:**
The ban pipeline is the core feature of the product. A regression anywhere in the chain (fail2ban log parsing → fail2ban banning → backend polling → database write → API response → frontend rendering) would go undetected until a user reports it.

**Goal:**
After running `simulate_failed_logins.sh` with a known IP, the ban record for that IP must appear in the BanGUI UI within a reasonable timeout.

**What to do:**
1. Create `e2e/tests/02_ban_records.robot`.
2. Suite Setup: `Login As Admin`.
3. Test teardown: unban the test IP using `Docker/check_ban_status.sh --unban 192.168.100.99` via `Run Process`.
4. Test case:
   ```robot
   *** Test Cases ***
   Simulated Failed Logins Appear As Ban Records
       [Teardown]    Run Process    bash    ${EXECDIR}/Docker/check_ban_status.sh
       ...    --unban    192.168.100.99
       # Step 1 — write failure lines
       ${result}=    Run Process
       ...    bash    ${EXECDIR}/Docker/simulate_failed_logins.sh    5    192.168.100.99
       ...    timeout=15s
       Should Be Equal As Integers    ${result.rc}    0
       # Step 2 — wait for fail2ban to process and backend to pick up the ban
       Sleep    15s
       # Step 3 — check History page
       Go To    ${FRONTEND_URL}/history
       Wait For Elements State    css=table,tbody    visible    timeout=20s
       Get Text    body    contains    192.168.100.99
       # Step 4 — confirm jail name is shown
       Get Text    body    contains    manual-Jail
   ```
5. Optionally add a direct API assertion before the UI check to isolate UI vs. backend failures:
   ```robot
   ${resp}=    GET    ${BACKEND_URL}/api/history    expected_status=200
   Should Contain    ${resp.text}    192.168.100.99
   ```

**Possible traps and issues:**
- The default `COUNT` for `simulate_failed_logins.sh` is 5 and the fail2ban `maxretry` for `manual-Jail` must be ≤ 5 for the ban to trigger. If the jail config has been changed locally the test may pass the script step but produce no ban.
- `simulate_failed_logins.sh` writes to `Docker/logs/auth.log`. The fail2ban container reads from `/remotelogs/bangui/auth.log` (mapped volume). If the file mapping differs the lines will never be detected.
- The backend polls fail2ban on a schedule (APScheduler). The 15 s sleep may not be enough if the polling interval is longer. Read the scheduler interval from the config before hardcoding the wait.
- `check_ban_status.sh` uses `docker exec` directly. In a Podman environment the container runtime may be `podman`, making the unban teardown fail and leaving the test IP permanently banned until manual cleanup.
- The History page paginates results. If the test IP is not on the first page the `contains` assertion will fail. Assert via the API or set a large page size query parameter in the URL.
- Running the test suite multiple times in the same session accumulates lines in `auth.log`. Old lines do not re-trigger bans, but the counter inside fail2ban resets on container restart. Add a `truncate -s 0 Docker/logs/auth.log` step before writing new lines if idempotency is needed.

**Docs changes needed:**
- Add a note in [Testing-Requirements.md](Testing-Requirements.md) explaining the ban pipeline timing expectations and why a sleep is needed in this test.
- Document the `manual-Jail` maxretry value and log path in [CONFIGURATION.md](CONFIGURATION.md) so it is clear what the E2E test depends on.

**Doc references:**
- [CONFIGURATION.md](CONFIGURATION.md)
- [Testing-Requirements.md](Testing-Requirements.md)
- `Docker/simulate_failed_logins.sh`
- `Docker/check_ban_status.sh`
- `Docker/fail2ban-dev-config/` — jail configuration

---

## [E2E-4] Blocklist import executes and is reflected in the UI

**Where found:**
The blocklist import endpoint (`POST /api/blocklists/import`) is implemented in `backend/app/routers/blocklist.py` and has unit tests in `backend/tests/test_routers/test_blocklist.py` and `backend/tests/test_tasks/test_blocklist_import.py`. The `/blocklists` frontend page exists but there is no E2E test verifying the manual import button works end-to-end.

**Why this is needed:**
The import flow is asynchronous and involves DNS validation, an external HTTP fetch, and a database write. Unit tests mock all external calls. An E2E test is the only way to verify that the full import pipeline — including the network call to the external source — completes and updates the UI.

**Goal:**
Clicking the manual import button on the `/blocklists` page triggers an import run, the UI reflects completion (no error banner), and the import log table shows a new entry.

**What to do:**
1. Create `e2e/tests/03_blocklist_import.robot`.
2. Suite Setup: `Login As Admin`.
3. Pre-condition: at least one blocklist source must be configured. Add a keyword `Ensure Blocklist Source Exists` that:
   - Calls `GET /api/blocklists` to check if any sources are defined.
   - If none: calls `POST /api/blocklists` to add a known-good source (e.g., a small, stable public list).
4. Test case:
   ```robot
   *** Test Cases ***
   Manual Blocklist Import Completes Without Error
       Ensure Blocklist Source Exists
       Go To    ${FRONTEND_URL}/blocklists
       Wait For Elements State    css=[aria-label*="Import"],button    visible    timeout=15s
       # record the current log entry count
       ${resp_before}=    GET    ${BACKEND_URL}/api/blocklists/log    expected_status=200
       # trigger the import
       Click    css=[aria-label*="Import"],button
       # wait for import to finish (spinner gone or success toast)
       Wait For Elements State    css=[aria-label*="Import"],button    enabled    timeout=45s
       Get Text    body    not contains    error
       # verify the log has a new entry
       ${resp_after}=    GET    ${BACKEND_URL}/api/blocklists/log    expected_status=200
       Should Not Be Equal    ${resp_before.text}    ${resp_after.text}
   ```
5. If the environment has no internet access, mock the external fetch via a local HTTP server (use Python's `http.server` in a `Run Process` background task and point the source URL at `http://localhost:<port>/test.txt`).

**Possible traps and issues:**
- The import endpoint has a rate limit (`RATE_LIMIT_BLOCKLIST_IMPORT_REQUESTS`). Running the suite more than once in the same hour may result in a 429. Either reset the rate limiter between runs or use a unique client IP via a custom `X-Forwarded-For` header.
- The external network may be unavailable in CI. The test must either skip gracefully (`Skip If    ${no_internet}`) or use a local mock server.
- The import button selector is unknown until inspected in a running browser. The selector `css=[aria-label*="Import"],button` is a best guess — it must be verified against the actual rendered DOM and updated if wrong.
- If a blocklist source URL returns an invalid or empty file, the import will complete but with 0 bans added. The test must distinguish "import ran successfully" from "bans were added" — these are separate assertions.
- The `POST /api/blocklists/import` can take several seconds per source. The 45 s timeout may be insufficient for large lists.

**Docs changes needed:**
- Add the rate limit value and reset mechanism to [CONFIGURATION.md](CONFIGURATION.md).
- Document the E2E dependency on network access (or local mock) in [Testing-Requirements.md](Testing-Requirements.md).

**Doc references:**
- [CONFIGURATION.md](CONFIGURATION.md)
- [Testing-Requirements.md](Testing-Requirements.md)
- [Features.md](Features.md) — blocklist feature description
- `backend/app/routers/blocklist.py` — endpoint and rate limit implementation
- `backend/tests/test_tasks/test_blocklist_import.py`

---

## [E2E-5] Config edit saves and persists after page reload

**Where found:**
The `/config` page allows editing jail settings, filter definitions, and server-level options. It is covered by unit tests (`frontend/src/pages/__tests__/ConfigPage.test.tsx`, `backend/tests/test_routers/test_config.py`) but no E2E test verifies that a change made in the UI actually persists through the backend, survives a page reload, and reflects the new value.

**Why this is needed:**
The config page uses an auto-save mechanism (`useAutoSave`) that debounces writes. A regression in the debounce logic, the PATCH endpoint, or the GET-on-mount rehydration would silently discard user edits. Only a full round-trip test can catch this.

**Goal:**
Change a config field value via the UI, wait for the auto-save indicator to confirm the save, reload the page, and assert the new value is still present.

**What to do:**
1. Create `e2e/tests/04_config_edit.robot`.
2. Suite Setup: `Login As Admin`.
3. Choose a safe, low-risk config field to edit — e.g., the `[DEFAULT]` `bantime` value, or a per-jail `maxretry` setting.
4. Record the original value before editing so it can be restored in teardown.
5. Test case:
   ```robot
   *** Settings ***
   Library    Browser
   Resource   ../resources/auth.resource
   Test Teardown    Restore Original Config Value

   *** Test Cases ***
   Config Field Edit Persists After Reload
       Login As Admin
       Go To    ${FRONTEND_URL}/config
       Wait For Elements State    css=[role="tablist"]    visible    timeout=15s
       # Read current value for teardown
       ${original}=    Get Text    css=[data-field="bantime"]
       Set Suite Variable    ${ORIGINAL_BANTIME}    ${original}
       # Edit the field
       Fill Text    css=[data-field="bantime"]    7200
       # Wait for auto-save indicator to show "Saved"
       Wait For Elements State    css=[data-autosave="saved"]    visible    timeout=15s
       # Reload and verify persistence
       Reload
       Wait For Elements State    css=[data-field="bantime"]    visible    timeout=15s
       Get Text    css=[data-field="bantime"]    ==    7200

   *** Keywords ***
   Restore Original Config Value
       Go To    ${FRONTEND_URL}/config
       Fill Text    css=[data-field="bantime"]    ${ORIGINAL_BANTIME}
       Wait For Elements State    css=[data-autosave="saved"]    visible    timeout=15s
   ```
6. Also verify via the API that the value was actually written:
   ```robot
   ${resp}=    GET    ${BACKEND_URL}/api/config    expected_status=200
   Should Contain    ${resp.text}    7200
   ```

**Possible traps and issues:**
- The config page auto-save uses a debounce delay. The test must wait for the "Saved" indicator rather than a fixed `Sleep`, otherwise the reload may happen before the PATCH request fires.
- The selectors `[data-field="bantime"]` and `[data-autosave="saved"]` do not exist in the current frontend components (no `data-*` attributes on production elements). These must be added to the components before the test can work. See [E2E-6] for the prerequisite task.
- Config fields are rendered inside a tab panel. The correct tab must be activated before the target field is interactable. The test must click the right tab first.
- If the backend validates the new value and rejects it (e.g., bantime must be a positive integer), the test will fail at the API assertion. Use a value that is guaranteed to be valid.
- Editing config files on disk via the API may restart the fail2ban service inside the container, causing a brief health-check failure and destabilising subsequent tests in the suite. Run config edit tests last or use a test-only jail that is isolated from the main config.
- Teardown must restore the original value even if the test fails mid-way. Ensure `Test Teardown` is set, not just a final keyword call.

**Docs changes needed:**
- Document the auto-save debounce behaviour and the "Saved" indicator semantics in [Web-Development.md](Web-Development.md) so E2E test authors know what to wait for.
- Note in [Testing-Requirements.md](Testing-Requirements.md) that config edit tests must restore state in teardown.

**Doc references:**
- [Web-Development.md](Web-Development.md)
- [CONFIGURATION.md](CONFIGURATION.md)
- [Testing-Requirements.md](Testing-Requirements.md)
- `frontend/src/components/config/__tests__/AutoSaveIndicator.test.tsx`
- `frontend/src/hooks/__tests__/useAutoSave.test.ts`
- `backend/tests/test_routers/test_config.py`

---

## [E2E-6] Add `data-testid` / `data-*` attributes to production frontend components

**Where found:**
Inspecting the frontend source, `data-testid` attributes appear only in test mock files (e.g., `MapPage.test.tsx` line 57: `<div data-testid="world-map" />`). The production components in `frontend/src/components/` and `frontend/src/pages/` have no `data-testid` or `data-*` attributes. E2E tests [E2E-2], [E2E-4], and [E2E-5] all require stable selectors that survive CSS and class-name refactors.

**Why this is needed:**
CSS class selectors and `aria-label` text are brittle — they break when styles change or text is translated. `data-testid` attributes are the idiomatic, refactor-safe way to locate elements in E2E tests. Without them, every UI change risks breaking the E2E suite for reasons unrelated to correctness.

**Goal:**
Key interactive and landmark elements across all pages and the config form have `data-testid` (or semantic `data-*`) attributes that the Robot Framework E2E suite can rely on.

**What to do:**
1. Identify the minimum set of elements needed by the four E2E suites:
   - `data-testid="page-error-boundary"` on the `PageErrorBoundary` fallback render.
   - `data-testid="dashboard"`, `data-testid="map-page"`, `data-testid="jails-page"`, `data-testid="history-page"`, `data-testid="blocklists-page"`, `data-testid="config-page"` on each page's root element.
   - `data-testid="history-table"` on the History page table body.
   - `data-testid="blocklist-import-button"` on the manual import trigger.
   - `data-testid="autosave-status"` on the auto-save indicator, with `data-status="saved" | "saving" | "error"`.
   - `data-field="<fieldname>"` on config input fields that E2E tests will edit.
2. Add the attributes directly to the JSX — no wrappers, no extra elements.
3. Do not add `data-testid` to elements that already have stable semantic roles (e.g., `<button type="submit">` with unique text, landmark `<main>`, `<nav>`).
4. Update the existing vitest component tests to use the new `data-testid` selectors instead of text queries where appropriate.

**Possible traps and issues:**
- `data-testid` attributes are visible in production HTML. This is standard practice and not a security concern, but some teams prefer to strip them in production builds via a Babel/Vite plugin. Decide on a policy before adding them.
- Adding attributes to components that are also tested by vitest may require updating those unit tests if they query by `data-testid`.
- The `PageErrorBoundary` component wraps lazy-loaded pages. The fallback element must carry the `data-testid` on the rendered fallback JSX, not on the wrapper itself.
- Config fields are rendered dynamically from API data. The `data-field` value must be derived from the field's API key name, not a hardcoded index.

**Docs changes needed:**
- Add a "Selector conventions" section to [Web-Development.md](Web-Development.md) documenting when to use `data-testid` vs. semantic selectors vs. ARIA roles, and listing all reserved `data-testid` values used by the E2E suite.

**Doc references:**
- [Web-Development.md](Web-Development.md)
- [Testing-Requirements.md](Testing-Requirements.md)
- `frontend/src/components/ErrorBoundary.tsx`
- `frontend/src/components/config/__tests__/AutoSaveIndicator.test.tsx`