diff --git a/admin/src/features/activity/ActivityFilters.test.tsx b/admin/src/features/activity/ActivityFilters.test.tsx new file mode 100644 index 0000000..73bfdd8 --- /dev/null +++ b/admin/src/features/activity/ActivityFilters.test.tsx @@ -0,0 +1,80 @@ +/** + * The filter row's error association. + * + * The timezone is fixed because the only invalid bound reachable in jsdom is a + * wall-clock time inside a spring-forward gap: jsdom applies the + * `datetime-local` value sanitization algorithm, so text that is merely + * incomplete never reaches the component at all — the input hands it back as + * the empty string, which is a bound the operator cleared. + */ + +import { afterAll, expect, test, vi } from "vitest"; +import { fireEvent, render, screen } from "@testing-library/react"; +import ActivityFilters, { NO_FILTERS } from "./ActivityFilters"; + +vi.stubEnv("TZ", "Europe/Paris"); +afterAll(() => vi.unstubAllEnvs()); + +/** 02:30 does not exist on this date in Paris; the clock jumps 02:00 to 03:00. */ +const GAP_WALL_TIME = "2026-03-29T02:30:00"; + +/** The text of the elements an input points at with `aria-describedby`. */ +function describedText(input: HTMLElement): string { + const ids = input.getAttribute("aria-describedby"); + if (ids === null) throw new Error("input has no aria-describedby"); + return ids + .split(/\s+/) + .map((id) => { + const node = document.getElementById(id); + if (node === null) throw new Error(`aria-describedby names missing element ${id}`); + return node.textContent ?? ""; + }) + .join(" "); +} + +function renderFilters() { + const applied: { current: unknown } = { current: null }; + render( + (applied.current = filters)} + onClear={() => (applied.current = null)} + />, + ); + return applied; +} + +test("a rejected bound marks its own input and describes it", () => { + const applied = renderFilters(); + const since = screen.getByLabelText("Since") as HTMLInputElement; + const until = screen.getByLabelText("Until") as HTMLInputElement; + + fireEvent.change(since, { target: { value: GAP_WALL_TIME } }); + fireEvent.click(screen.getByRole("button", { name: "Apply filters" })); + + expect(since.getAttribute("aria-invalid")).toBe("true"); + expect(describedText(since)).toContain("daylight saving"); + // Refused means refused, and only the offending bound carries the mark. + expect(applied.current).toBeNull(); + expect(until.getAttribute("aria-invalid")).toBeNull(); + expect(until.getAttribute("aria-describedby")).toBeNull(); +}); + +test("the mark moves to the other bound, and clears once both parse", () => { + renderFilters(); + const since = screen.getByLabelText("Since") as HTMLInputElement; + const until = screen.getByLabelText("Until") as HTMLInputElement; + + fireEvent.change(until, { target: { value: GAP_WALL_TIME } }); + fireEvent.click(screen.getByRole("button", { name: "Apply filters" })); + + expect(since.getAttribute("aria-invalid")).toBeNull(); + expect(describedText(until)).toContain("daylight saving"); + + fireEvent.change(until, { target: { value: "2026-03-29T04:30:00" } }); + fireEvent.click(screen.getByRole("button", { name: "Apply filters" })); + + expect(until.getAttribute("aria-invalid")).toBeNull(); + expect(until.getAttribute("aria-describedby")).toBeNull(); +}); diff --git a/admin/src/features/activity/ActivityFilters.tsx b/admin/src/features/activity/ActivityFilters.tsx index b6c3f2e..10e3532 100644 --- a/admin/src/features/activity/ActivityFilters.tsx +++ b/admin/src/features/activity/ActivityFilters.tsx @@ -96,10 +96,24 @@ function optionBlocked(value: string): boolean | undefined { return value === "allowed" ? false : undefined; } -function boundError(label: string, reason: "unparseable" | "nonexistent"): string { - return reason === "unparseable" - ? `${label} is not a complete date and time.` - : `${label} names a local time that does not exist — the clock jumps over it for daylight saving.`; +/** The message plus the bound it belongs to, so that input can point at it. */ +interface BoundError { + field: "since" | "until"; + message: string; +} + +/** The editor is rendered once per page, so the message can hold a fixed id. */ +const ERROR_ID = "activity-filter-error"; + +function boundError(field: "since" | "until", reason: "unparseable" | "nonexistent"): BoundError { + const label = field === "since" ? "Since" : "Until"; + return { + field, + message: + reason === "unparseable" + ? `${label} is not a complete date and time.` + : `${label} names a local time that does not exist — the clock jumps over it for daylight saving.`, + }; } interface Props { @@ -115,18 +129,22 @@ export default function ActivityFilters({ applied, isDisabled, onApply, onClear const [blocked, setBlocked] = useState(blockedOption(applied.blocked)); const [since, setSince] = useState(() => datetimeField(applied.since)); const [until, setUntil] = useState(() => datetimeField(applied.until)); - const [error, setError] = useState(null); + const [error, setError] = useState(null); + + /** True for the one bound the current message is about; nothing else is marked. */ + const invalid = (field: BoundError["field"]): true | undefined => + error !== null && error.field === field ? true : undefined; function submit(event: FormEvent) { event.preventDefault(); const sinceValue = resolveDatetimeField(since); if (!sinceValue.ok) { - setError(boundError("Since", sinceValue.reason)); + setError(boundError("since", sinceValue.reason)); return; } const untilValue = resolveDatetimeField(until); if (!untilValue.ok) { - setError(boundError("Until", untilValue.reason)); + setError(boundError("until", untilValue.reason)); return; } setError(null); @@ -186,6 +204,8 @@ export default function ActivityFilters({ applied, isDisabled, onApply, onClear type="datetime-local" step={1} value={since.text} + aria-invalid={invalid("since")} + aria-describedby={invalid("since") && ERROR_ID} disabled={isDisabled} onChange={(event) => setSince(editDatetimeField(since, event.target.value))} {...stylex.props(shared.smallInput, styles.input, shared.focusRing)} @@ -197,6 +217,8 @@ export default function ActivityFilters({ applied, isDisabled, onApply, onClear type="datetime-local" step={1} value={until.text} + aria-invalid={invalid("until")} + aria-describedby={invalid("until") && ERROR_ID} disabled={isDisabled} onChange={(event) => setUntil(editDatetimeField(until, event.target.value))} {...stylex.props(shared.smallInput, styles.input, shared.focusRing)} @@ -221,8 +243,8 @@ export default function ActivityFilters({ applied, isDisabled, onApply, onClear {error !== null && ( -

- {error} +

)} diff --git a/admin/src/features/clients/ClientsPage.test.tsx b/admin/src/features/clients/ClientsPage.test.tsx index 56accd1..7adb288 100644 --- a/admin/src/features/clients/ClientsPage.test.tsx +++ b/admin/src/features/clients/ClientsPage.test.tsx @@ -2,6 +2,20 @@ import { fireEvent, render, screen, waitFor, within } from "@testing-library/rea import { ClientName, type ClientNames } from "./clientNames"; import { BASE, MANAGED_FILE, NEVER, renderClientsPage, setConfigStatus } from "./testFixtures"; +/** The text of the elements an input points at with `aria-describedby`. */ +function describedText(input: HTMLElement): string { + const ids = input.getAttribute("aria-describedby"); + if (ids === null) throw new Error("input has no aria-describedby"); + return ids + .split(/\s+/) + .map((id) => { + const node = document.getElementById(id); + if (node === null) throw new Error(`aria-describedby names missing element ${id}`); + return node.textContent ?? ""; + }) + .join(" "); +} + function clientRow(ip: string): HTMLElement { const row = screen.getByText(ip).closest("tr"); if (row === null) throw new Error(`no client row for ${ip}`); @@ -315,7 +329,9 @@ describe.each([ fireEvent.click(confirm); await waitFor(() => expect( - fetchMock.mock.calls.filter(([input, init]) => init?.method === "DELETE" && String(input).endsWith("/1")), + fetchMock.mock.calls.filter( + ([input, init]) => init?.method === "DELETE" && String(input).endsWith("/1"), + ), ).toHaveLength(1), ); }); @@ -357,3 +373,66 @@ test("a pending config status holds the same line as a failed one (R3-4)", async const observed = clientRow("192.168.1.11"); expect((within(observed).getByRole("button", { name: "Delete" }) as HTMLButtonElement).disabled).toBe(false); }); + +test("a save-time problem marks the input it is about and describes it", async () => { + await renderClientsPage(); + + const range = (await screen.findByLabelText("Range 1")) as HTMLInputElement; + fireEvent.change(range, { target: { value: "" } }); + fireEvent.click(screen.getByRole("button", { name: "Save assignments" })); + + expect(range.getAttribute("aria-invalid")).toBe("true"); + expect(describedText(range)).toBe("Row 1: prefix is required."); + + // Only the offending input is marked; the priority beside it is untouched. + const priority = screen.getByLabelText("Priority for range 1"); + expect(priority.getAttribute("aria-invalid")).toBeNull(); + expect(priority.getAttribute("aria-describedby")).toBeNull(); + + fireEvent.change(range, { target: { value: "10.0.0.0/8" } }); + fireEvent.change(screen.getByLabelText("Priority for range 1"), { target: { value: "abc" } }); + fireEvent.click(screen.getByRole("button", { name: "Save assignments" })); + + expect(range.getAttribute("aria-invalid")).toBeNull(); + expect(describedText(screen.getByLabelText("Priority for range 1"))).toBe( + "Row 1: priority must be a whole number.", + ); +}); + +test("removing a row drops the message rather than moving it to another input", async () => { + await renderClientsPage(); + + // Row 1 is the offending one, so removing it is what would slide the stale + // index onto row 2 — an input that validated cleanly. + const range = (await screen.findByLabelText("Range 1")) as HTMLInputElement; + fireEvent.change(range, { target: { value: "" } }); + fireEvent.click(screen.getByRole("button", { name: "Add range" })); + fireEvent.change(screen.getByLabelText("Range 2"), { target: { value: "10.0.0.0/8" } }); + fireEvent.click(screen.getByRole("button", { name: "Save assignments" })); + + expect(range.getAttribute("aria-invalid")).toBe("true"); + expect(describedText(range)).toBe("Row 1: prefix is required."); + + const section = assignmentsSection(); + fireEvent.click(within(section).getAllByRole("button", { name: "Remove" })[0] as HTMLButtonElement); + + const survivor = screen.getByLabelText("Range 1") as HTMLInputElement; + expect(survivor.value).toBe("10.0.0.0/8"); + expect(survivor.getAttribute("aria-invalid")).toBeNull(); + expect(survivor.getAttribute("aria-describedby")).toBeNull(); + expect(within(section).queryByRole("alert")).toBeNull(); + expect(screen.queryByLabelText("Range 2")).toBeNull(); +}); + +test("editing a row clears the message it was about", async () => { + await renderClientsPage(); + + const range = (await screen.findByLabelText("Range 1")) as HTMLInputElement; + fireEvent.change(range, { target: { value: "" } }); + fireEvent.click(screen.getByRole("button", { name: "Save assignments" })); + expect(range.getAttribute("aria-invalid")).toBe("true"); + + fireEvent.change(range, { target: { value: "10.0.0.0/8" } }); + expect(range.getAttribute("aria-invalid")).toBeNull(); + expect(within(assignmentsSection()).queryByRole("alert")).toBeNull(); +}); diff --git a/admin/src/features/clients/NetworkAssignments.tsx b/admin/src/features/clients/NetworkAssignments.tsx index e5c1645..15cdde6 100644 --- a/admin/src/features/clients/NetworkAssignments.tsx +++ b/admin/src/features/clients/NetworkAssignments.tsx @@ -4,7 +4,15 @@ import * as stylex from "@stylexjs/stylex"; import { clientPrefixesPutMutation } from "@/lib/queries"; import type { ClientPrefix, Group } from "@/lib/types"; import { defaultGroupId } from "@/lib/defaultGroup"; -import { firstProblem, initPrefixEditor, isDirty, prefixEditorReducer, toInputs } from "./prefixEditor"; +import { + firstProblem, + initPrefixEditor, + isDirty, + prefixEditorReducer, + toInputs, + type PrefixEditorAction, + type PrefixProblem, +} from "./prefixEditor"; import InlineError from "@/lib/InlineError"; import AuthorityGate from "@/features/configuration/AuthorityGate"; import Select from "@/ui/Select"; @@ -16,6 +24,9 @@ interface Props { groups: Group[]; } +/** The editor is rendered once per page, so the message can hold a fixed id. */ +const VALIDATION_ID = "network-assignments-validation"; + const styles = stylex.create({ section: { marginTop: "2.5rem", @@ -162,12 +173,24 @@ function AssignmentsTable({ prefixes }: { prefixes: ClientPrefix[] }) { function AssignmentsEditor({ prefixes, groups }: Props) { const queryClient = useQueryClient(); const mutation = useMutation(clientPrefixesPutMutation(queryClient)); - const [state, dispatch] = useReducer(prefixEditorReducer, prefixes, initPrefixEditor); - const [validation, setValidation] = useState(null); + const [state, apply] = useReducer(prefixEditorReducer, prefixes, initPrefixEditor); + const [validation, setValidation] = useState(null); const dirty = isDirty(state); const fallbackGroupId = defaultGroupId(groups); const groupOptions = groups.map((group) => ({ value: String(group.id), label: group.name })); + // A problem names a row by position, and every action here can move, add or + // delete a position. The message describes the rows Save read, so it dies + // with them rather than drifting onto whatever row inherits the index. + const dispatch = (action: PrefixEditorAction) => { + setValidation(null); + apply(action); + }; + + /** True for the one input the current message is about; nothing else is marked. */ + const invalid = (index: number, field: PrefixProblem["field"]): true | undefined => + validation !== null && validation.index === index && validation.field === field ? true : undefined; + const save = () => { const problem = firstProblem(state.rows); setValidation(problem); @@ -188,6 +211,8 @@ function AssignmentsEditor({ prefixes, groups }: Props) { @@ -208,6 +233,8 @@ function AssignmentsEditor({ prefixes, groups }: Props) { type="text" inputMode="numeric" aria-label={`Priority for range ${index + 1}`} + aria-invalid={invalid(index, "priority")} + aria-describedby={invalid(index, "priority") && VALIDATION_ID} placeholder="100" value={row.priority} onChange={(event) => @@ -227,8 +254,8 @@ function AssignmentsEditor({ prefixes, groups }: Props) { )} {validation !== null && ( -

- {validation} +

)} @@ -251,10 +278,7 @@ function AssignmentsEditor({ prefixes, groups }: Props) { {dirty && (