diff --git a/tests/test_speaker_edit_submit.py b/tests/test_speaker_edit_submit.py new file mode 100644 index 0000000..70dfb7e --- /dev/null +++ b/tests/test_speaker_edit_submit.py @@ -0,0 +1,95 @@ +"""Guards on the speaker edit form's submit behaviour. + +Regression: the voice-queue work emptied ``queueVoiceRegenOnSave`` but left +``onsubmit="return queueVoiceRegenOnSave(...)"`` in the edit form. A handler +that falls off the end returns ``undefined``, which is falsy, so the browser +cancelled the submit and every speaker edit was silently discarded - the save +never reached the server, so nothing looked broken and nothing errored. + +An ``onsubmit`` that returns a value must return a boolean deliberately. +""" +from __future__ import annotations + +import re +from pathlib import Path + +TEMPLATES = Path(__file__).resolve().parents[1] / "umbraaudio" / "web" / "templates" + +# onsubmit="return someFn(...)" - the cancelling shape +ONSUBMIT_RETURN = re.compile(r'onsubmit="return\s+([A-Za-z_$][\w$]*)\s*\(') +FUNC_DEF = re.compile( + r"function\s+([A-Za-z_$][\w$]*)\s*\([^)]*\)\s*\{", re.MULTILINE +) +# Comments quote these patterns when documenting the hazard, so strip them +# first or the scanner reports the documentation as if it were live markup. +COMMENTS = re.compile(r"\{#.*?#\}|", re.DOTALL) + + +def _strip_comments(text: str) -> str: + return COMMENTS.sub("", text) + + +def _all_template_text() -> str: + return _strip_comments( + "\n".join(p.read_text() for p in TEMPLATES.rglob("*.html")) + ) + + +def _function_bodies(text: str) -> dict[str, str]: + """Map function name -> body text, brace-matched.""" + bodies: dict[str, str] = {} + for m in FUNC_DEF.finditer(text): + start = m.end() + depth = 1 + i = start + while i < len(text) and depth: + if text[i] == "{": + depth += 1 + elif text[i] == "}": + depth -= 1 + i += 1 + bodies[m.group(1)] = text[start : i - 1] + return bodies + + +def test_no_onsubmit_handler_can_cancel_the_speaker_edit() -> None: + """The edit form must never gate its own submission on a JS return value.""" + row = _strip_comments((TEMPLATES / "_speaker_row.html").read_text()) + assert 'onsubmit="return' not in row, ( + "_speaker_row.html uses onsubmit=\"return ...\"; if that handler ever " + "returns a falsy value the browser drops the edit without any error" + ) + + +def test_every_onsubmit_return_handler_returns_a_boolean() -> None: + """Repo-wide: any onsubmit returning a value must decide explicitly.""" + text = _all_template_text() + bodies = _function_bodies(text) + for name in sorted(set(ONSUBMIT_RETURN.findall(text))): + assert name in bodies, f"onsubmit returns {name}() but it is not defined" + body = bodies[name] + returns_bool = re.search(r"return\s+(true|false)\b", body) + assert returns_bool, ( + f"onsubmit=\"return {name}(...)\" but {name}() never returns an " + "explicit true/false - falling off the end yields undefined, which " + "cancels the submit and silently discards the user's edit" + ) + + +def test_voice_regen_on_save_is_not_empty() -> None: + """The specific function that was gutted.""" + bodies = _function_bodies((TEMPLATES / "speakers.html").read_text()) + assert "queueVoiceRegenOnSave" in bodies, "handler is missing entirely" + body = bodies["queueVoiceRegenOnSave"].strip() + assert body, "queueVoiceRegenOnSave has an empty body" + assert "enqueueVoice" in body, "it must actually queue a regeneration" + + +def test_edit_form_queues_regen_after_a_successful_save() -> None: + """Re-voice must run after the new profile is persisted, not before.""" + row = _strip_comments((TEMPLATES / "_speaker_row.html").read_text()) + assert "queueVoiceRegenOnSave" in row + assert "hx-on::after-request" in row, ( + "regeneration should be hooked to after-request so the saved profile " + "is on disk before the voice is rebuilt from it" + ) diff --git a/umbraaudio/web/templates/_speaker_row.html b/umbraaudio/web/templates/_speaker_row.html index 30bca91..8001ed3 100644 --- a/umbraaudio/web/templates/_speaker_row.html +++ b/umbraaudio/web/templates/_speaker_row.html @@ -171,7 +171,13 @@
+ hx-on::after-request="if(event.detail.successful){ queueVoiceRegenOnSave({{ speaker_idx }}); }"> + {# Re-voice is hooked to after-request, NOT onsubmit. An + onsubmit="return fn(...)" whose fn falls through returns + undefined, which is falsy, so the browser cancels the whole + submit and silently discards every edit. Queueing here also + guarantees the saved profile is on disk before the voice is + regenerated from it. #}
diff --git a/umbraaudio/web/templates/speakers.html b/umbraaudio/web/templates/speakers.html index b194c8d..d05778c 100644 --- a/umbraaudio/web/templates/speakers.html +++ b/umbraaudio/web/templates/speakers.html @@ -33,7 +33,18 @@ var _regenPoll = null; var _regenIdx = null; -function queueVoiceRegenOnSave(idx, name) { +// Runs after a successful PUT, so speakers.json already holds the new profile. +// The name is re-read from the swapped row because the reviewer may have just +// renamed the speaker, and regenerating under the old name would voice a +// speaker that no longer exists. +function queueVoiceRegenOnSave(idx) { + var row = document.getElementById('speaker-row-' + idx); + var label = row ? row.querySelector('strong') : null; + var name = label ? label.textContent.trim() : ''; + if (!name) { + return; + } + enqueueVoice(name, idx); } // Voice regeneration is queue-driven (see enqueueVoice below); the old