John O'Keefe
9f39e2efd8
Release / build-and-push (push) Successful in 4m35s
None of the three save surfaces on the admin settings page gave usable
feedback: the Base URL form re-rendered into an identical-looking form
(its only indicator was the Go-source twin's banner — markup that
diverged from the templ original), the scan form was WORSE — its handler
responds JSON, so htmx's outerHTML swap replaced the whole scan card
with literal JSON text — and the tunable rows showed only a tiny
per-row label.
Client side (toasts are the sanctioned client-local island; no new
inline scripts, CSP unchanged):
- web/src/settings.ts: one delegated htmx:afterRequest/sendError
listener keyed on a data-settings-save attribute stamped on each
form. Success → green toast per surface ("Settings saved" / "Scan
settings saved" / "Setting saved"); failure → error toast carrying
the server's JSON error field, degrading to a status line for the
tunable endpoint's HTML error body (markup must never reach a toast).
Network-level failures (htmx:sendError) toast instead of silently
re-enabling the button.
- htmx 2.x quirk handled: for FORM submissions, afterRequest's
detail.elt is the event-dispatch ancestor, not the form — the
requesting element lives on detail.requestConfig.elt (verified
empirically; saveLabelFor reads the right one).
- hx-disabled-elt="this" on all save buttons: the click is
acknowledged and double-clicks cannot double-save.
Server side:
- Scan form: hx-swap="none" (the inputs already hold the saved values;
nothing needs re-rendering) — kills the JSON-over-the-card swap.
- The Base URL form's save response is a Go-source copy of the templ
form (handlers cannot import templates: templates/utils.go imports
handlers types). The twin now carries the functional attributes
(data-settings-save, hx-disabled-elt) so feedback survives the swap,
with a drift-warning comment; the double-save e2e pins this.
Dev-DB safety: every success save in the spec re-submits the value
already on the page (zero drift), and the spec restores
system_config.updated_by after itself — the save handler stamps the
requesting admin's id there, and that FK (no cascade) otherwise blocks
the integration harness's transient-user sweep (it broke
TestUpdateProfileAdminMode/TestDeleteUserConsolidated on this run's
first pass; pollution cleaned up by hand and now self-restoring).
Tests: web/src/settings.test.ts pins the form→label map and the
error-extraction contract (including the no-markup-in-toast rule);
e2e/settings-feedback.spec.ts drives all three surfaces through the
real admin UI including the error path and the twin-drift double-save.
61 lines
2.5 KiB
TypeScript
61 lines
2.5 KiB
TypeScript
import { describe, expect, it } from "vitest";
|
|
|
|
import { extractSaveError, saveLabelFor } from "./settings";
|
|
|
|
// The settings page's save feedback keys off the data-settings-save
|
|
// attribute stamped on each form in templates/admin_settings.templ. If a
|
|
// form loses the attribute (or a new save surface is added without one),
|
|
// its saves silently regress to "no indicator" — the bug this module
|
|
// exists to fix. These pins make that drift a test failure.
|
|
|
|
function el(attrs: Record<string, string>): Element {
|
|
const node = document.createElement("form");
|
|
for (const [k, v] of Object.entries(attrs)) node.setAttribute(k, v);
|
|
return node;
|
|
}
|
|
|
|
describe("saveLabelFor", () => {
|
|
it("maps every stamped form to its success label", () => {
|
|
expect(saveLabelFor(el({ "data-settings-save": "settings" }))).toBe("Settings saved");
|
|
expect(saveLabelFor(el({ "data-settings-save": "scan" }))).toBe("Scan settings saved");
|
|
expect(saveLabelFor(el({ "data-settings-save": "tunable" }))).toBe("Setting saved");
|
|
});
|
|
|
|
it("returns null for non-settings elements (listener filter)", () => {
|
|
expect(saveLabelFor(document.createElement("div"))).toBeNull();
|
|
expect(saveLabelFor(null)).toBeNull();
|
|
expect(saveLabelFor(undefined)).toBeNull();
|
|
expect(saveLabelFor(el({ "data-settings-save": "unknown-kind" }))).toBeNull();
|
|
});
|
|
});
|
|
|
|
describe("extractSaveError", () => {
|
|
function xhrWith(responseText: string, status: number): XMLHttpRequest {
|
|
const xhr = new XMLHttpRequest();
|
|
Object.defineProperty(xhr, "responseText", { value: responseText });
|
|
Object.defineProperty(xhr, "status", { value: status });
|
|
return xhr;
|
|
}
|
|
|
|
it("surfaces the JSON error field the config endpoints return", () => {
|
|
expect(extractSaveError(xhrWith(`{"error":"invalid timezone"}`, 400))).toBe("invalid timezone");
|
|
});
|
|
|
|
it("falls back to the message field when present", () => {
|
|
expect(extractSaveError(xhrWith(`{"message":"boom"}`, 500))).toBe("boom");
|
|
});
|
|
|
|
it("never puts markup in a toast (tunable endpoint returns an HTML span on 400)", () => {
|
|
const msg = extractSaveError(
|
|
xhrWith(`<span style="color: var(--status-danger);">value rejected</span>`, 400),
|
|
);
|
|
expect(msg).toBe("Save failed (HTTP 400)");
|
|
expect(msg).not.toContain("<");
|
|
});
|
|
|
|
it("degrades to a status line for empty bodies and network failures", () => {
|
|
expect(extractSaveError(xhrWith("", 0))).toBe("Save failed (HTTP 0)");
|
|
expect(extractSaveError(undefined)).toBe("Save failed (HTTP 0)");
|
|
});
|
|
});
|