From 4b6e118a88976c74e26541572445c52b05ceb304 Mon Sep 17 00:00:00 2001
From: Lukas <lukas.pupkalipinski@lpl-mind.de>
Date: Sat, 14 Mar 2026 19:50:55 +0100
Subject: [PATCH] Fix ActivateJailDialog blocking logic and mypy false positive

Two frontend bugs and one mypy false positive fixed:

- ActivateJailDialog: Activate button was never disabled when
  blockingIssues.length > 0 (missing condition in disabled prop).
- ActivateJailDialog: handleConfirm called onActivated() even when
  the backend returned active=false (blocked activation). Dialog now
  stays open and shows result.message instead.
- config.py: Settings() call flagged by mypy --strict because
  pydantic-settings loads required fields from env vars at runtime;
  suppressed with a targeted type: ignore[call-arg] comment.

Tests: added ActivateJailDialog.test.tsx (5 tests covering button state,
backend-rejection handling, success path, and crash detection callback).
---
 Docs/Tasks.md                                 | 131 ++++++++++
 backend/app/config.py                         |   2 +-
 .../components/config/ActivateJailDialog.tsx  |   9 +-
 .../__tests__/ActivateJailDialog.test.tsx     | 229 ++++++++++++++++++
 4 files changed, 369 insertions(+), 2 deletions(-)
 create mode 100644 frontend/src/components/config/__tests__/ActivateJailDialog.test.tsx

diff --git a/Docs/Tasks.md b/Docs/Tasks.md
index 32e7007..e3eb49e 100644
--- a/Docs/Tasks.md
+++ b/Docs/Tasks.md
@@ -331,3 +331,134 @@ The `deactivate_jail` endpoint in `backend/app/routers/config.py` is inconsisten
 3. **Manual test with running server:**
    - Go to `/config`, find an active jail and click "Deactivate".
    - Immediately navigate to the Dashboard — "Active Jails" count should already reflect the reduced count without any delay.
+
+---
+
+## Task 7 — Fix ActivateJailDialog not honouring backend rejection and mypy false positive
+
+**Status:** in progress
+
+### Problem description
+
+Two independent bugs were introduced during Tasks 5–6:
+
+**Bug 1 — "Activate" button is never disabled on validation errors (frontend)**
+
+In `frontend/src/components/config/ActivateJailDialog.tsx`, Task 5 Part B set:
+
+```tsx
+const blockingIssues = validationIssues; // all issues block activation
+```
+
+but the "Activate" `<Button>` `disabled` prop was never updated to include `blockingIssues.length > 0`:
+
+```tsx
+disabled={submitting || validating}   // BUG: missing `|| blockingIssues.length > 0`
+```
+
+The pre-validation error message renders correctly, but the button stays clickable. A user can press "Activate" despite seeing a red error — the backend will refuse (returning `active=false`) but the UX is broken and confusing.
+
+**Bug 2 — Dialog closes and fires `onActivated()` even when backend rejects activation (frontend)**
+
+`handleConfirm`'s `.then()` handler never inspects `result.active`. When the backend blocks activation and returns `{ active: false, message: "...", validation_warnings: [...] }`, the frontend still:
+
+1. Calls `setValidationWarnings(result.validation_warnings)` — sets warnings in state.
+2. Immediately calls `resetForm()` — which **clears** the newly-set warnings.
+3. Calls `onActivated()` — which triggers the parent to refresh the jail list (and may close the dialog).
+
+The user sees the dialog briefly appear to succeed, the parent refreshes, but the jail never activated.
+
+**Bug 3 — mypy strict false positive in `config.py`**
+
+`get_settings()` calls `Settings()` without arguments. mypy strict mode flags this as:
+```
+backend/app/config.py:88: error: Missing named argument "session_secret" for "Settings"  [call-arg]
+```
+This is a known pydantic-settings limitation: the library loads required fields from environment variables at runtime, which mypy cannot see statically. A targeted suppression with an explanatory comment is the correct fix.
+
+### What to do
+
+#### Part A — Disable "Activate" button when blocking issues are present (frontend)
+
+**File:** `frontend/src/components/config/ActivateJailDialog.tsx`
+
+Find the "Activate" `<Button>` near the bottom of the returned JSX and change its `disabled` prop:
+
+```tsx
+// Before:
+disabled={submitting || validating}
+
+// After:
+disabled={submitting || validating || blockingIssues.length > 0}
+```
+
+#### Part B — Handle `active=false` response from backend (frontend)
+
+**File:** `frontend/src/components/config/ActivateJailDialog.tsx`
+
+In `handleConfirm`'s `.then()` callback, add a check for `result.active` before calling `resetForm()` and `onActivated()`:
+
+```tsx
+.then((result) => {
+  if (!result.active) {
+    // Backend rejected the activation (e.g. missing logpath).
+    // Show the server's message and keep the dialog open.
+    setError(result.message);
+    return;
+  }
+  if (result.validation_warnings.length > 0) {
+    setValidationWarnings(result.validation_warnings);
+  }
+  resetForm();
+  if (!result.fail2ban_running) {
+    onCrashDetected?.();
+  }
+  onActivated();
+})
+```
+
+#### Part C — Fix mypy false positive (backend)
+
+**File:** `backend/app/config.py`
+
+Add a targeted `# type: ignore[call-arg]` with an explanatory comment to the `Settings()` call in `get_settings()`:
+
+```python
+return Settings()  # type: ignore[call-arg]  # pydantic-settings populates required fields from env vars
+```
+
+### Tests to add or update
+
+**File:** `frontend/src/components/config/__tests__/ActivateJailDialog.test.tsx` (new file)
+
+Write tests covering:
+
+1. **`test_activate_button_disabled_when_blocking_issues`** — render the dialog with mocked `validateJailConfig` returning an issue with `field="logpath"`. Assert the "Activate" button is disabled.
+
+2. **`test_activate_button_enabled_when_no_issues`** — render the dialog with mocked `validateJailConfig` returning no issues. Assert the "Activate" button is enabled after validation completes.
+
+3. **`test_dialog_stays_open_when_backend_returns_active_false`** — mock `activateJail` to return `{ active: false, message: "Jail cannot be activated", validation_warnings: [], fail2ban_running: true, name: "test" }`. Click "Activate". Assert: (a) `onActivated` is NOT called; (b) the error message text appears.
+
+4. **`test_dialog_calls_on_activated_when_backend_returns_active_true`** — mock `activateJail` to return `{ active: true, message: "ok", validation_warnings: [], fail2ban_running: true, name: "test" }`. Click "Activate". Assert `onActivated` is called once.
+
+5. **`test_crash_detected_callback_fires_when_fail2ban_not_running`** — mock `activateJail` to return `active: true, fail2ban_running: false`. Assert `onCrashDetected` is called.
+
+### Verification
+
+1. Run frontend type check and lint:
+   ```bash
+   cd frontend && npx tsc --noEmit && npx eslint src/components/config/ActivateJailDialog.tsx
+   ```
+   Zero errors and zero warnings.
+
+2. Run frontend tests:
+   ```bash
+   cd frontend && npx vitest run src/components/config/__tests__/ActivateJailDialog
+   ```
+   All 5 new tests pass.
+
+3. Run mypy:
+   ```bash
+   .venv/bin/mypy backend/app/ --strict
+   ```
+   Zero errors.
diff --git a/backend/app/config.py b/backend/app/config.py
index e5e60e5..0f73ce5 100644
--- a/backend/app/config.py
+++ b/backend/app/config.py
@@ -85,4 +85,4 @@ def get_settings() -> Settings:
         A validated :class:`Settings` object. Raises :class:`pydantic.ValidationError`
         if required keys are absent or values fail validation.
     """
-    return Settings()
+    return Settings()  # type: ignore[call-arg]  # pydantic-settings populates required fields from env vars
diff --git a/frontend/src/components/config/ActivateJailDialog.tsx b/frontend/src/components/config/ActivateJailDialog.tsx
index 8f77ae0..8dfd71e 100644
--- a/frontend/src/components/config/ActivateJailDialog.tsx
+++ b/frontend/src/components/config/ActivateJailDialog.tsx
@@ -152,6 +152,13 @@ export function ActivateJailDialog({
 
     activateJail(jail.name, overrides)
       .then((result) => {
+        if (!result.active) {
+          // Backend rejected the activation (e.g. missing logpath or filter).
+          // Show the server's message and keep the dialog open so the user
+          // can read the explanation without the dialog disappearing.
+          setError(result.message);
+          return;
+        }
         if (result.validation_warnings.length > 0) {
           setValidationWarnings(result.validation_warnings);
         }
@@ -336,7 +343,7 @@ export function ActivateJailDialog({
             <Button
               appearance="primary"
               onClick={handleConfirm}
-              disabled={submitting || validating}
+              disabled={submitting || validating || blockingIssues.length > 0}
               icon={submitting ? <Spinner size="tiny" /> : undefined}
             >
               {submitting ? "Activating and verifying…" : "Activate"}
diff --git a/frontend/src/components/config/__tests__/ActivateJailDialog.test.tsx b/frontend/src/components/config/__tests__/ActivateJailDialog.test.tsx
new file mode 100644
index 0000000..ef4ddf5
--- /dev/null
+++ b/frontend/src/components/config/__tests__/ActivateJailDialog.test.tsx
@@ -0,0 +1,229 @@
+/**
+ * Tests for ActivateJailDialog (Task 7).
+ *
+ * Covers:
+ *  - "Activate" button is disabled when pre-validation returns blocking issues.
+ *  - "Activate" button is enabled when validation passes.
+ *  - Dialog stays open and shows an error when the backend returns active=false.
+ *  - `onActivated` is called and dialog closes when backend returns active=true.
+ *  - `onCrashDetected` is called when fail2ban_running is false after activation.
+ */
+
+import { describe, it, expect, vi, beforeEach } from "vitest";
+import { render, screen, waitFor } from "@testing-library/react";
+import userEvent from "@testing-library/user-event";
+import { FluentProvider, webLightTheme } from "@fluentui/react-components";
+import { ActivateJailDialog } from "../ActivateJailDialog";
+import type { InactiveJail, JailActivationResponse, JailValidationResult } from "../../../types/config";
+
+// ---------------------------------------------------------------------------
+// Mocks
+// ---------------------------------------------------------------------------
+
+vi.mock("../../../api/config", () => ({
+  activateJail: vi.fn(),
+  validateJailConfig: vi.fn(),
+}));
+
+import { activateJail, validateJailConfig } from "../../../api/config";
+
+const mockActivateJail = vi.mocked(activateJail);
+const mockValidateJailConfig = vi.mocked(validateJailConfig);
+
+// ---------------------------------------------------------------------------
+// Fixtures
+// ---------------------------------------------------------------------------
+
+const baseJail: InactiveJail = {
+  name: "airsonic-auth",
+  filter: "airsonic-auth",
+  actions: ["iptables-multiport"],
+  port: "8080",
+  logpath: ["/var/log/airsonic/airsonic.log"],
+  bantime: "10m",
+  findtime: "10m",
+  maxretry: 5,
+  ban_time_seconds: 600,
+  find_time_seconds: 600,
+  log_encoding: "auto",
+  backend: "auto",
+  date_pattern: null,
+  use_dns: "warn",
+  prefregex: "",
+  fail_regex: ["Failed login.*from <HOST>"],
+  ignore_regex: [],
+  bantime_escalation: null,
+  source_file: "/config/fail2ban/jail.d/airsonic-auth.conf",
+  enabled: false,
+};
+
+/** Successful activation response. */
+const successResponse: JailActivationResponse = {
+  name: "airsonic-auth",
+  active: true,
+  message: "Jail activated successfully.",
+  fail2ban_running: true,
+  validation_warnings: [],
+};
+
+/** Response when backend blocks activation (e.g. missing logpath). */
+const blockedResponse: JailActivationResponse = {
+  name: "airsonic-auth",
+  active: false,
+  message: "Jail 'airsonic-auth' cannot be activated: logpath does not exist.",
+  fail2ban_running: true,
+  validation_warnings: ["logpath: /var/log/airsonic/airsonic.log does not exist"],
+};
+
+/** Validation result with a logpath issue (should block the button). */
+const validationWithLogpathIssue: JailValidationResult = {
+  jail_name: "airsonic-auth",
+  valid: false,
+  issues: [{ field: "logpath", message: "/var/log/airsonic/airsonic.log does not exist" }],
+};
+
+/** Validation result with no issues. */
+const validationPassed: JailValidationResult = {
+  jail_name: "airsonic-auth",
+  valid: true,
+  issues: [],
+};
+
+// ---------------------------------------------------------------------------
+// Helpers
+// ---------------------------------------------------------------------------
+
+interface DialogProps {
+  jail?: InactiveJail | null;
+  open?: boolean;
+  onClose?: () => void;
+  onActivated?: () => void;
+  onCrashDetected?: () => void;
+}
+
+function renderDialog({
+  jail = baseJail,
+  open = true,
+  onClose = vi.fn(),
+  onActivated = vi.fn(),
+  onCrashDetected = vi.fn(),
+}: DialogProps = {}) {
+  return render(
+    <FluentProvider theme={webLightTheme}>
+      <ActivateJailDialog
+        jail={jail}
+        open={open}
+        onClose={onClose}
+        onActivated={onActivated}
+        onCrashDetected={onCrashDetected}
+      />
+    </FluentProvider>,
+  );
+}
+
+// ---------------------------------------------------------------------------
+// Tests
+// ---------------------------------------------------------------------------
+
+describe("ActivateJailDialog", () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+  });
+
+  it("disables the Activate button when pre-validation returns blocking issues", async () => {
+    mockValidateJailConfig.mockResolvedValue(validationWithLogpathIssue);
+
+    renderDialog();
+
+    // Wait for validation to complete and the error message to appear.
+    await waitFor(() => {
+      expect(screen.getByText(/configuration errors detected/i)).toBeInTheDocument();
+    });
+
+    const activateBtn = screen.getByRole("button", { name: /^activate$/i });
+    expect(activateBtn).toBeDisabled();
+  });
+
+  it("enables the Activate button when validation passes", async () => {
+    mockValidateJailConfig.mockResolvedValue(validationPassed);
+
+    renderDialog();
+
+    // Wait for validation spinner to disappear.
+    await waitFor(() => {
+      expect(screen.queryByText(/validating configuration/i)).not.toBeInTheDocument();
+    });
+
+    const activateBtn = screen.getByRole("button", { name: /^activate$/i });
+    expect(activateBtn).not.toBeDisabled();
+  });
+
+  it("keeps the dialog open and shows an error when backend returns active=false", async () => {
+    mockValidateJailConfig.mockResolvedValue(validationPassed);
+    mockActivateJail.mockResolvedValue(blockedResponse);
+
+    const onActivated = vi.fn();
+    renderDialog({ onActivated });
+
+    // Wait for validation to finish.
+    await waitFor(() => {
+      expect(screen.queryByText(/validating configuration/i)).not.toBeInTheDocument();
+    });
+
+    const activateBtn = screen.getByRole("button", { name: /^activate$/i });
+    await userEvent.click(activateBtn);
+
+    // The server's error message should appear.
+    await waitFor(() => {
+      expect(
+        screen.getByText(/cannot be activated/i),
+      ).toBeInTheDocument();
+    });
+
+    // onActivated must NOT have been called.
+    expect(onActivated).not.toHaveBeenCalled();
+  });
+
+  it("calls onActivated when backend returns active=true", async () => {
+    mockValidateJailConfig.mockResolvedValue(validationPassed);
+    mockActivateJail.mockResolvedValue(successResponse);
+
+    const onActivated = vi.fn();
+    renderDialog({ onActivated });
+
+    await waitFor(() => {
+      expect(screen.queryByText(/validating configuration/i)).not.toBeInTheDocument();
+    });
+
+    const activateBtn = screen.getByRole("button", { name: /^activate$/i });
+    await userEvent.click(activateBtn);
+
+    await waitFor(() => {
+      expect(onActivated).toHaveBeenCalledOnce();
+    });
+  });
+
+  it("calls onCrashDetected when fail2ban_running is false after activation", async () => {
+    mockValidateJailConfig.mockResolvedValue(validationPassed);
+    mockActivateJail.mockResolvedValue({
+      ...successResponse,
+      fail2ban_running: false,
+    });
+
+    const onActivated = vi.fn();
+    const onCrashDetected = vi.fn();
+    renderDialog({ onActivated, onCrashDetected });
+
+    await waitFor(() => {
+      expect(screen.queryByText(/validating configuration/i)).not.toBeInTheDocument();
+    });
+
+    const activateBtn = screen.getByRole("button", { name: /^activate$/i });
+    await userEvent.click(activateBtn);
+
+    await waitFor(() => {
+      expect(onCrashDetected).toHaveBeenCalledOnce();
+    });
+    expect(onActivated).toHaveBeenCalledOnce();
+  });
+});