Fix BanUnbanForm floating promises and add submit guards
This commit is contained in:
@@ -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.
|
||||
|
||||
|
||||
@@ -29,54 +29,59 @@ export function BanUnbanForm({ jailNames, onBan, onUnban }: BanUnbanFormProps):
|
||||
const [unbanJail, setUnbanJail] = useState("");
|
||||
const [formError, setFormError] = useState<string | null>(null);
|
||||
const [formSuccess, setFormSuccess] = useState<string | null>(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<void> => {
|
||||
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}.`);
|
||||
|
||||
setIsBanning(true);
|
||||
try {
|
||||
const ip = banIpVal.trim();
|
||||
await onBan(banJail, ip);
|
||||
setFormSuccess(`${ip} 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);
|
||||
});
|
||||
} catch (err: unknown) {
|
||||
setFormError(formatErrorMessage(err));
|
||||
} finally {
|
||||
setIsBanning(false);
|
||||
}
|
||||
};
|
||||
|
||||
const handleUnban = (fromAllJails: boolean): void => {
|
||||
const handleUnban = async (fromAllJails: boolean): Promise<void> => {
|
||||
setFormError(null);
|
||||
setFormSuccess(null);
|
||||
if (!unbanIpVal.trim()) {
|
||||
setFormError("IP address is required.");
|
||||
return;
|
||||
}
|
||||
|
||||
setIsUnbanning(true);
|
||||
try {
|
||||
const ip = unbanIpVal.trim();
|
||||
const jail = fromAllJails ? undefined : unbanJail || undefined;
|
||||
onUnban(unbanIpVal.trim(), jail)
|
||||
.then(() => {
|
||||
await onUnban(ip, jail);
|
||||
const scope = jail ?? "all jails";
|
||||
setFormSuccess(`${unbanIpVal.trim()} unbanned from ${scope}.`);
|
||||
setFormSuccess(`${ip} 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);
|
||||
});
|
||||
} catch (err: unknown) {
|
||||
setFormError(formatErrorMessage(err));
|
||||
} finally {
|
||||
setIsUnbanning(false);
|
||||
}
|
||||
};
|
||||
|
||||
return (
|
||||
@@ -130,7 +135,9 @@ export function BanUnbanForm({ jailNames, onBan, onUnban }: BanUnbanFormProps):
|
||||
</Select>
|
||||
</Field>
|
||||
</div>
|
||||
<Button appearance="primary" icon={<LockClosedRegular />} onClick={handleBan}>
|
||||
<Button appearance="primary" icon={<LockClosedRegular />} onClick={() => {
|
||||
void handleBan();
|
||||
}} disabled={isBanning}>
|
||||
Ban
|
||||
</Button>
|
||||
</div>
|
||||
@@ -167,10 +174,14 @@ export function BanUnbanForm({ jailNames, onBan, onUnban }: BanUnbanFormProps):
|
||||
</Select>
|
||||
</Field>
|
||||
</div>
|
||||
<Button appearance="secondary" icon={<LockOpenRegular />} onClick={() => { handleUnban(false); }}>
|
||||
<Button appearance="secondary" icon={<LockOpenRegular />} onClick={() => {
|
||||
void handleUnban(false);
|
||||
}} disabled={isUnbanning}>
|
||||
Unban
|
||||
</Button>
|
||||
<Button appearance="outline" icon={<LockOpenRegular />} onClick={() => { handleUnban(true); }}>
|
||||
<Button appearance="outline" icon={<LockOpenRegular />} onClick={() => {
|
||||
void handleUnban(true);
|
||||
}} disabled={isUnbanning}>
|
||||
Unban from All Jails
|
||||
</Button>
|
||||
</div>
|
||||
|
||||
80
frontend/src/pages/jails/__tests__/BanUnbanForm.test.tsx
Normal file
80
frontend/src/pages/jails/__tests__/BanUnbanForm.test.tsx
Normal file
@@ -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 = <T extends Element>(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<void>((resolve) => {
|
||||
resolveBan = resolve;
|
||||
}),
|
||||
);
|
||||
const onUnban = vi.fn().mockResolvedValue(undefined);
|
||||
const user = userEvent.setup();
|
||||
|
||||
render(<BanUnbanForm jailNames={["sshd"]} onBan={onBan} onUnban={onUnban} />);
|
||||
|
||||
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<void>((_resolve, reject) => {
|
||||
rejectUnban = reject;
|
||||
}),
|
||||
);
|
||||
const user = userEvent.setup();
|
||||
|
||||
render(<BanUnbanForm jailNames={["sshd"]} onBan={onBan} onUnban={onUnban} />);
|
||||
|
||||
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();
|
||||
});
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user