From 082dcc7ee1fb2afce23e4eeb42cdae2bebcc265d Mon Sep 17 00:00:00 2001 From: Lukas Date: Sun, 19 Apr 2026 19:42:39 +0200 Subject: [PATCH] Fix BanUnbanForm floating promises and add submit guards --- Docs/Tasks.md | 6 +- frontend/src/pages/jails/BanUnbanForm.tsx | 83 +++++++++++-------- .../jails/__tests__/BanUnbanForm.test.tsx | 80 ++++++++++++++++++ 3 files changed, 132 insertions(+), 37 deletions(-) create mode 100644 frontend/src/pages/jails/__tests__/BanUnbanForm.test.tsx diff --git a/Docs/Tasks.md b/Docs/Tasks.md index 76ef538..2a4d2c7 100644 --- a/Docs/Tasks.md +++ b/Docs/Tasks.md @@ -53,7 +53,11 @@ Issues are grouped by category and ordered roughly by severity. Each entry descr --- -### TASK-003 — `BanUnbanForm` floating promises and no double-submit guard +### TASK-003 — `BanUnbanForm` floating promises and no double-submit guard (done) + +**Where fixed:** `frontend/src/pages/jails/BanUnbanForm.tsx`, `frontend/src/pages/jails/__tests__/BanUnbanForm.test.tsx` + +**Summary:** Converted ban and unban handlers to async functions with separate submit states and disabled submit buttons while requests are in flight. **Where found:** `frontend/src/pages/jails/BanUnbanForm.tsx` — `handleBan` and `handleUnban` are synchronous functions that call `onBan(…).then(…).catch(…)`. The returned promise is not awaited and is not assigned to anything. diff --git a/frontend/src/pages/jails/BanUnbanForm.tsx b/frontend/src/pages/jails/BanUnbanForm.tsx index 856e51f..c128fef 100644 --- a/frontend/src/pages/jails/BanUnbanForm.tsx +++ b/frontend/src/pages/jails/BanUnbanForm.tsx @@ -29,54 +29,59 @@ export function BanUnbanForm({ jailNames, onBan, onUnban }: BanUnbanFormProps): const [unbanJail, setUnbanJail] = useState(""); const [formError, setFormError] = useState(null); const [formSuccess, setFormSuccess] = useState(null); + const [isBanning, setIsBanning] = useState(false); + const [isUnbanning, setIsUnbanning] = useState(false); - const handleBan = (): void => { + const formatErrorMessage = (err: unknown): string => + err instanceof ApiError + ? `${String(err.status)}: ${err.body}` + : err instanceof Error + ? err.message + : String(err); + + const handleBan = async (): Promise => { setFormError(null); setFormSuccess(null); if (!banIpVal.trim() || !banJail) { setFormError("Both IP address and jail are required."); return; } - onBan(banJail, banIpVal.trim()) - .then(() => { - setFormSuccess(`${banIpVal.trim()} banned in ${banJail}.`); - setBanIpVal(""); - }) - .catch((err: unknown) => { - const msg = - err instanceof ApiError - ? `${String(err.status)}: ${err.body}` - : err instanceof Error - ? err.message - : String(err); - setFormError(msg); - }); + + setIsBanning(true); + try { + const ip = banIpVal.trim(); + await onBan(banJail, ip); + setFormSuccess(`${ip} banned in ${banJail}.`); + setBanIpVal(""); + } catch (err: unknown) { + setFormError(formatErrorMessage(err)); + } finally { + setIsBanning(false); + } }; - const handleUnban = (fromAllJails: boolean): void => { + const handleUnban = async (fromAllJails: boolean): Promise => { setFormError(null); setFormSuccess(null); if (!unbanIpVal.trim()) { setFormError("IP address is required."); return; } - const jail = fromAllJails ? undefined : unbanJail || undefined; - onUnban(unbanIpVal.trim(), jail) - .then(() => { - const scope = jail ?? "all jails"; - setFormSuccess(`${unbanIpVal.trim()} unbanned from ${scope}.`); - setUnbanIpVal(""); - setUnbanJail(""); - }) - .catch((err: unknown) => { - const msg = - err instanceof ApiError - ? `${String(err.status)}: ${err.body}` - : err instanceof Error - ? err.message - : String(err); - setFormError(msg); - }); + + setIsUnbanning(true); + try { + const ip = unbanIpVal.trim(); + const jail = fromAllJails ? undefined : unbanJail || undefined; + await onUnban(ip, jail); + const scope = jail ?? "all jails"; + setFormSuccess(`${ip} unbanned from ${scope}.`); + setUnbanIpVal(""); + setUnbanJail(""); + } catch (err: unknown) { + setFormError(formatErrorMessage(err)); + } finally { + setIsUnbanning(false); + } }; return ( @@ -130,7 +135,9 @@ export function BanUnbanForm({ jailNames, onBan, onUnban }: BanUnbanFormProps): - @@ -167,10 +174,14 @@ export function BanUnbanForm({ jailNames, onBan, onUnban }: BanUnbanFormProps): - - diff --git a/frontend/src/pages/jails/__tests__/BanUnbanForm.test.tsx b/frontend/src/pages/jails/__tests__/BanUnbanForm.test.tsx new file mode 100644 index 0000000..104cbfe --- /dev/null +++ b/frontend/src/pages/jails/__tests__/BanUnbanForm.test.tsx @@ -0,0 +1,80 @@ +import { render, screen, waitFor } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { describe, expect, it, vi } from "vitest"; +import { BanUnbanForm } from "../BanUnbanForm"; + +const getRequiredElement = (elements: T[], index: number): T => { + const element = elements[index]; + if (!element) { + throw new Error(`Expected element at index ${String(index)}`); + } + return element; +}; + +describe("BanUnbanForm", () => { + it("disables the ban button while the ban request is pending and shows success", async () => { + let resolveBan: () => void = () => undefined; + const onBan = vi.fn( + () => + new Promise((resolve) => { + resolveBan = resolve; + }), + ); + const onUnban = vi.fn().mockResolvedValue(undefined); + const user = userEvent.setup(); + + render(); + + const banIpInputs = screen.getAllByPlaceholderText("e.g. 192.168.1.100"); + const banIpInput = getRequiredElement(banIpInputs, 0); + await user.type(banIpInput, "192.168.1.100"); + + const selectElements = screen.getAllByRole("combobox"); + const banSelect = getRequiredElement(selectElements, 0); + await user.selectOptions(banSelect, "sshd"); + + const banButton = screen.getByRole("button", { name: /^ban$/i }); + await user.click(banButton); + + expect(onBan).toHaveBeenCalledWith("sshd", "192.168.1.100"); + expect(banButton).toBeDisabled(); + + resolveBan(); + await waitFor(() => { + expect(screen.getByText(/192\.168\.1\.100 banned in sshd\./i)).toBeInTheDocument(); + expect(banButton).not.toBeDisabled(); + }); + }); + + it("shows an error when unban fails and keeps the unban button disabled during the request", async () => { + let rejectUnban: (reason?: unknown) => void = () => undefined; + const onBan = vi.fn().mockResolvedValue(undefined); + const onUnban = vi.fn( + () => + new Promise((_resolve, reject) => { + rejectUnban = reject; + }), + ); + const user = userEvent.setup(); + + render(); + + const unbanIpInputs = screen.getAllByPlaceholderText("e.g. 192.168.1.100"); + const unbanIpInput = getRequiredElement(unbanIpInputs, 1); + await user.type(unbanIpInput, "10.0.0.1"); + + const unbanButton = screen.getByRole("button", { name: /^unban$/i }); + await user.click(unbanButton); + + await waitFor(() => { + expect(unbanButton).toBeDisabled(); + }); + + rejectUnban(new Error("unban failed")); + + await waitFor(() => { + expect(screen.getByText(/unban failed/i)).toBeInTheDocument(); + expect(unbanButton).not.toBeDisabled(); + }); + }); +});