refactoring-backend #3

Merged
lukas.pupkalipinski merged 403 commits from refactoring-backend into main 2026-05-20 20:23:46 +02:00
4 changed files with 234 additions and 213 deletions
Showing only changes of commit 1510dfc851 - Show all commits

View File

@@ -1,46 +1,3 @@
### TASK-BUG-06 — `JailConfigDetail` Form State Never Re-syncs After Background Refresh
**Where found**
`frontend/src/components/config/JailsTab.tsx`. `JailConfigDetail` initialises all 20+ form fields from `jail` prop in `useState` calls (lines 126161). The component uses `key={selectedActiveJail.name}`, which forces remount only when the *selected jail changes*, not when the data for the already-selected jail is refreshed by the parent. If `useJailConfigs` does a background refresh and delivers updated server data for the currently-selected jail, the form continues displaying the stale locally-edited values.
**Goal**
Add a `useEffect` that resets form fields when the incoming `jail` prop changes identity (i.e. when a server refresh delivers a new object for the same jail name). The effect must only run when the user is not mid-edit. The cleanest approach is to track a `lastSavedJail` ref and compare it to the incoming `jail`; if the auto-save has no pending changes and `jail` has changed, reset the fields.
Alternatively, expose a `resetToServer` button that lets the user explicitly pull the latest server state without relying on automatic detection.
**Possible traps and issues**
- Automatically resetting a form a user is actively editing is hostile. The reset must only happen when `autoSave` reports no pending changes and no dirty state.
- Comparing the full `jail` object on every render is expensive; use a ref to track the last-applied server version by comparing a stable property like `jail.name + JSON.stringify(jail)` (hashed or shallow-compared field by field).
- This issue is partially mitigated by `key={selectedActiveJail.name}` forcing remount on jail selection change.
**Docs changes needed**
None required.
**Why this is needed**
If fail2ban reloads externally (e.g. another admin makes a change), the GUI background-refreshes the config but the currently-open form silently shows stale data. A save action would overwrite the external change.
---
### TASK-BUG-07 — `useJails()` Called Twice on `JailsPage` (Double HTTP Request)
**Where found**
`frontend/src/pages/JailsPage.tsx` line 11: `const { jails } = useJails();` — used only to extract `jailNames` for `useIpLookup`. `frontend/src/pages/jails/JailOverviewSection.tsx` line 55: `const { jails, ... } = useJails();` — the full feature hook. Both components are rendered simultaneously on `JailsPage`, causing two parallel `GET /api/jails` requests on every page load.
**Goal**
Remove the `useJails()` call from `JailsPage`. Pass `jailNames` to `JailsPage`'s children as a prop from `JailOverviewSection`, or lift `useJails()` to `JailsPage` and thread `jails` down as a prop to `JailOverviewSection`. Since `JailOverviewSection` already owns all the jail operations, the simplest fix is to accept an optional `onJailNamesLoaded` callback or have `JailsPage` access jail names directly from `JailOverviewSection` via a ref or by reading from the single hook call.
**Possible traps and issues**
- `JailsPage` currently passes `jailNames` to a separate `IpLookupSection` or similar. After consolidating to one `useJails()` call the prop-drilling path needs to be updated.
- `JailOverviewSection` is the authoritative consumer of `useJails`; making `JailsPage` the single call site and passing the result down as props is the cleanest structural change.
**Docs changes needed**
None required.
**Why this is needed**
Every visit to the Jails page sends two identical requests to the backend. At scale with many jails this doubles the serialization and deserialization cost for no benefit.
---
### TASK-BUG-08 — `AssignActionDialog` and `AssignFilterDialog` Call `useJails()` When Closed
**Where found**

View File

@@ -54,9 +54,8 @@ export interface AssignActionDialogProps {
/**
* Confirmation dialog for assigning an action to a jail.
*
* Fetches running jails from the API for the jail selector. The user can
* optionally request a fail2ban reload immediately after the assignment is
* written.
* Wraps AssignActionDialogInner and only renders the inner component when
* the dialog is open, preventing unnecessary API calls when closed.
*
* @param props - Component props.
* @returns JSX element.
@@ -68,6 +67,43 @@ export function AssignActionDialog({
onAssigned,
onAssign,
}: AssignActionDialogProps): React.JSX.Element {
const handleClose = useCallback((): void => {
onClose();
}, [onClose]);
return (
<Dialog open={open} onOpenChange={(_e, data) => { if (!data.open) handleClose(); }}>
<DialogSurface>
{open && (
<AssignActionDialogInner
actionName={actionName}
onClose={handleClose}
onAssigned={onAssigned}
onAssign={onAssign}
/>
)}
</DialogSurface>
</Dialog>
);
}
interface AssignActionDialogInnerProps {
actionName: string | null;
onClose: () => void;
onAssigned: () => void;
onAssign: (jailName: string, payload: AssignActionRequest, reload: boolean) => Promise<void>;
}
/**
* Inner component that contains the dialog body and only renders when open.
* This ensures useJails() is only called when the dialog is actually visible.
*/
function AssignActionDialogInner({
actionName,
onClose,
onAssigned,
onAssign,
}: AssignActionDialogInnerProps): React.JSX.Element {
const { jails, loading: jailsLoading, error: jailsError } = useJails();
const [selectedJail, setSelectedJail] = useState("");
const [reload, setReload] = useState(false);
@@ -77,17 +113,16 @@ export function AssignActionDialog({
const activeJails = jails.filter((j) => j.enabled);
useEffect(() => {
if (!open) return;
setError(null);
setSelectedJail("");
setReload(false);
}, [open]);
}, []);
useEffect(() => {
if (jailsError && open) {
if (jailsError) {
setError(jailsError);
}
}, [jailsError, open]);
}, [jailsError]);
const handleClose = useCallback((): void => {
if (submitting) return;
@@ -119,8 +154,6 @@ export function AssignActionDialog({
const canConfirm = selectedJail !== "" && !submitting && !jailsLoading;
return (
<Dialog open={open} onOpenChange={(_e, data) => { if (!data.open) handleClose(); }}>
<DialogSurface>
<DialogBody>
<DialogTitle>Assign Action to Jail</DialogTitle>
<DialogContent>
@@ -198,7 +231,5 @@ export function AssignActionDialog({
</Button>
</DialogActions>
</DialogBody>
</DialogSurface>
</Dialog>
);
}

View File

@@ -54,9 +54,8 @@ export interface AssignFilterDialogProps {
/**
* Confirmation dialog for assigning a filter to a jail.
*
* Fetches running jails from the API for the jail selector. The user can
* optionally request a fail2ban reload immediately after the assignment is
* written.
* Wraps AssignFilterDialogInner and only renders the inner component when
* the dialog is open, preventing unnecessary API calls when closed.
*
* @param props - Component props.
* @returns JSX element.
@@ -68,6 +67,43 @@ export function AssignFilterDialog({
onAssigned,
onAssign,
}: AssignFilterDialogProps): React.JSX.Element {
const handleClose = useCallback((): void => {
onClose();
}, [onClose]);
return (
<Dialog open={open} onOpenChange={(_e, data) => { if (!data.open) handleClose(); }}>
<DialogSurface>
{open && (
<AssignFilterDialogInner
filterName={filterName}
onClose={handleClose}
onAssigned={onAssigned}
onAssign={onAssign}
/>
)}
</DialogSurface>
</Dialog>
);
}
interface AssignFilterDialogInnerProps {
filterName: string | null;
onClose: () => void;
onAssigned: () => void;
onAssign: (jailName: string, payload: AssignFilterRequest, reload: boolean) => Promise<void>;
}
/**
* Inner component that contains the dialog body and only renders when open.
* This ensures useJails() is only called when the dialog is actually visible.
*/
function AssignFilterDialogInner({
filterName,
onClose,
onAssigned,
onAssign,
}: AssignFilterDialogInnerProps): React.JSX.Element {
const { jails, loading: jailsLoading, error: jailsError } = useJails();
const [selectedJail, setSelectedJail] = useState("");
const [reload, setReload] = useState(false);
@@ -77,17 +113,16 @@ export function AssignFilterDialog({
const activeJails = jails.filter((j) => j.enabled);
useEffect(() => {
if (!open) return;
setError(null);
setSelectedJail("");
setReload(false);
}, [open]);
}, []);
useEffect(() => {
if (jailsError && open) {
if (jailsError) {
setError(jailsError);
}
}, [jailsError, open]);
}, [jailsError]);
const handleClose = useCallback((): void => {
if (submitting) return;
@@ -119,8 +154,6 @@ export function AssignFilterDialog({
const canConfirm = selectedJail !== "" && !submitting && !jailsLoading;
return (
<Dialog open={open} onOpenChange={(_e, data) => { if (!data.open) handleClose(); }}>
<DialogSurface>
<DialogBody>
<DialogTitle>Assign Filter to Jail</DialogTitle>
<DialogContent>
@@ -199,7 +232,5 @@ export function AssignFilterDialog({
</Button>
</DialogActions>
</DialogBody>
</DialogSurface>
</Dialog>
);
}

View File

@@ -73,7 +73,9 @@ function renderDialog(overrides: Partial<React.ComponentProps<typeof AssignFilte
open: true,
onClose: vi.fn(),
onAssigned: vi.fn(),
onAssign: vi.fn(async () => undefined),
onAssign: vi.fn(async (jailName, payload, reload) => {
await assignFilterToJail(jailName, payload, reload);
}),
...overrides,
};
return render(