feat(settings): save feedback — success/error toasts, disabled buttons; fix scan-form raw-JSON swap
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.
This commit is contained in:
John O'Keefe committed 2026-10-28 00:22:00 -04:00
1 parent 7b9e8e4fc2
commit 9f39e2efd8
7 files changed
+357 -24

No files matched your search

+1
View File
@@ -29,6 +29,7 @@ import "./profile-modal";
import "./register";
import "./search";
import "./series";
import "./settings";
import "./setup";
import "./stats";
import "./storage";
+60
View File
@@ -0,0 +1,60 @@
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)");
});
});
+103
View File
@@ -0,0 +1,103 @@
// Admin settings page: save feedback.
//
// The page's three save surfaces (Base URL form, scan settings form,
// per-row tunable forms) are htmx-driven; before this module none of
// them gave visible confirmation — the Base URL form re-rendered into
// an identical-looking form, and the scan form swapped raw JSON text
// over its card (fixed server-side by hx-swap="none" + this toast).
// Toasts are the sanctioned client-local island, so the notification
// lives here; no inline scripts (CSP hash discipline).
import { showToast } from "./toast";
// Map a settings save form to its success-toast label. Returns null for
// elements that are not settings save forms, so the delegated listener
// can filter in one call. Keyed by the data-settings-save attribute
// stamped on each form in templates/admin_settings.templ.
export function saveLabelFor(elt: EventTarget | null | undefined): string | null {
const form = elt instanceof Element ? elt : null;
const kind = form?.getAttribute("data-settings-save");
switch (kind) {
case "settings":
return "Settings saved";
case "scan":
return "Scan settings saved";
case "tunable":
return "Setting saved";
default:
return null;
}
}
// Pull a human-readable error out of a failed save response. The config
// endpoints return JSON `{"error": "..."}`; the tunable endpoint returns
// an HTML span on 400 — markup must never reach a toast, so non-JSON
// bodies degrade to the status line.
export function extractSaveError(xhr: XMLHttpRequest | undefined): string {
const text = (xhr?.responseText || "").trim();
if (text) {
try {
const parsed = JSON.parse(text) as { error?: string; message?: string };
if (parsed.error) return parsed.error;
if (parsed.message) return parsed.message;
} catch {
// HTML or plain-text body — fall through.
}
}
return `Save failed (HTTP ${xhr?.status || 0})`;
}
// The element htmx issued the request FROM. htmx 2.x dispatches request
// lifecycle events with detail.elt set to the element the event was
// dispatched on — for FORM submissions that is an ancestor wrapper, and
// the real requesting element lives on detail.requestConfig.elt
// (verified empirically: afterRequest on the settings forms reports
// elt=DIV wrapper, requestConfig.elt=FORM). Element-triggered requests
// (htmx.trigger on the search input) set both to the same element, so
// prefer requestConfig.elt and fall back to elt.
function requestingElement(
detail:
| {
requestConfig?: { elt?: EventTarget };
elt?: EventTarget;
}
| null
| undefined,
): Element | null {
const rc = detail?.requestConfig?.elt;
if (rc instanceof Element) return rc;
const elt = detail?.elt;
return elt instanceof Element ? elt : null;
}
function wireSettingsSaveFeedback(): void {
// htmx fires lifecycle events on the REQUESTING element; body-level
// delegation covers all three forms (and rows swapped in later).
document.body.addEventListener("htmx:afterRequest", (evt) => {
const e = evt as CustomEvent<{
elt: EventTarget;
requestConfig?: { elt?: EventTarget };
successful: boolean;
xhr?: XMLHttpRequest;
}>;
const label = saveLabelFor(requestingElement(e.detail));
if (!label) return;
if (e.detail.successful) {
showToast(label, "success");
} else {
showToast(`Save failed: ${extractSaveError(e.detail.xhr)}`, "error");
}
});
// A request that never gets a response (network down, server restart)
// fires sendError, not afterRequest — without this the button just
// re-enables silently and the user can't tell the save never landed.
document.body.addEventListener("htmx:sendError", (evt) => {
const e = evt as CustomEvent<{ elt: EventTarget; requestConfig?: { elt?: EventTarget } }>;
if (!saveLabelFor(requestingElement(e.detail))) return;
showToast("Save failed: network error", "error");
});
}
// The bundle is a deferred module script: the DOM is parsed when this
// runs, so direct wiring matches the other modules (admin.ts etc.).
wireSettingsSaveFeedback();