fix: MPS-24 Phase 2.8d — proxy-proof AJAX signal (the actual root cause)
Operator DevTools (custom domain shop.printableprompts.com) showed the tell: bulk-tagger actions did a DOCUMENT POST -> 302 -> 200 and the page rendered the SERVER-SIDE flash banner. That banner only survives if the view took the non-AJAX HTTPFound branch — i.e. is_ajax() was False: the app never saw X-Requested-With. Custom-domain shops sit behind a Caddy reverse proxy that was not forwarding that request header to uWSGI, so the capability-driven split ALWAYS chose 302 and the page full-reloaded. Canonical host worked, so it looked fine. - views/__init__.py:is_ajax() now returns True for X-Requested-With == XMLHttpRequest OR request param ajax=1. The param rides in the URL/body — no proxy strips it. Header kept for back-compat. - tag_bulk.js (submitForm/doReorder/persistOrder FormData, fetchFocus URL) and product_tags.js (post helper) now send ajax=1. - Hardened tag_bulk.js: ZERO code paths full-reload on failure anymore. reportFailure() surfaces HTTP status + content-type + body snippet as a visible banner (the old form.submit()/location fallbacks turned every server hiccup into 'the screen keeps refreshing' and hid the cause). safeInit() + window 'error' handler make a dead script visible (transient '✓ Tag editor interactive' proof-of-life banner) instead of failing silently. - Tests: +test_ajax_param_signals_ajax_without_header, +test_no_ajax_signal_still_redirects, +test_ajax_focus_via_param_returns_json. 1134 passed. Docs: mps-24.md Phase 2.8d, CLAUDE.md (is_ajax dual signal).
This commit is contained in:
parent
cd5ea68fe3
commit
8e31124ca8
6 changed files with 204 additions and 31 deletions
|
|
@ -514,7 +514,14 @@ reload; plain POST → 302 back to product edit. The JS keeps the hidden
|
|||
`tags` field in lock-step with the chips so a later full "Save Settings"
|
||||
is a no-op, never a stale revert. Capability-driven split helper:
|
||||
`views/__init__.py:is_ajax(request)` (single source of truth;
|
||||
`shop.py:_is_ajax` delegates to it). Removable chip component:
|
||||
`shop.py:_is_ajax` delegates to it). **`is_ajax` returns True for
|
||||
`X-Requested-With: XMLHttpRequest` OR an `ajax=1` request param.** The
|
||||
param exists because a Caddy reverse proxy on custom-domain shops was
|
||||
NOT forwarding `X-Requested-With` to uWSGI, so every AJAX action took
|
||||
the 302 path and the page full-reloaded — `ajax=1` rides in the
|
||||
URL/body where no proxy strips it. All progressive-enhancement JS
|
||||
(`tag_bulk.js`, `product_tags.js`, …) must send `ajax=1` in addition
|
||||
to the header. Removable chip component:
|
||||
`.tag-chip-removable` family in `common.css` + `/styleguide#tagchips`.
|
||||
|
||||
Home rendering: `views/shop.py:_build_home_layout_context()` builds
|
||||
|
|
|
|||
|
|
@ -363,6 +363,37 @@ Tests (`test_functional.py::TestProductTagsSpa`):
|
|||
Deferred (occasional click, not the hot path): AJAX-ifying the
|
||||
"Suggest categories" link — still a full navigation by design.
|
||||
|
||||
**Phase 2.8d — the OTHER root cause: proxy strips X-Requested-With**
|
||||
(shipped 2026-05-16): even after 2.8c (fresh JS confirmed loading,
|
||||
`tag_bulk.js?v=<hash>` 200 in the operator's Network panel),
|
||||
Add/reorder/etc still full-reloaded. Operator's Network tab showed the
|
||||
tell: a **document** `POST /s/{id}/tags` → **302** → GET → **200**,
|
||||
and the page rendered the **server-side** flash banner ("Moved
|
||||
'Emergent Reader' up.") — which only survives if the view took the
|
||||
**non-AJAX** `HTTPFound` branch, i.e. `is_ajax()` returned False: the
|
||||
app never saw `X-Requested-With`. Custom-domain shops
|
||||
(`shop.printableprompts.com`) sit behind a Caddy reverse proxy that
|
||||
was not forwarding that request header to uWSGI, so the
|
||||
capability-driven split always chose the 302 path and the JS
|
||||
`await res.json()` then fell back to a full submit.
|
||||
|
||||
Fix: a second, **proxy-proof** AJAX signal. `views/__init__.py:is_ajax`
|
||||
now returns True for `X-Requested-With == XMLHttpRequest` **OR**
|
||||
`request.params.get("ajax") == "1"`. The param rides in the URL/body,
|
||||
which no proxy strips. `tag_bulk.js` (submitForm / doReorder /
|
||||
persistOrder FormData, fetchFocus URL) and `product_tags.js` (post
|
||||
helper) now send `ajax=1`. The header is kept for back-compat.
|
||||
Also hardened `tag_bulk.js`: **zero** code paths full-reload on
|
||||
failure anymore — `reportFailure()` surfaces HTTP status +
|
||||
content-type + body snippet as a visible banner (the old
|
||||
`form.submit()` / `location` fallbacks were turning every server
|
||||
hiccup into the "screen keeps refreshing" symptom and hiding the
|
||||
cause); `safeInit()` + a `window 'error'` handler banner make a dead
|
||||
script visible instead of silent. Tests:
|
||||
`test_ajax_param_signals_ajax_without_header`,
|
||||
`test_no_ajax_signal_still_redirects`,
|
||||
`test_ajax_focus_via_param_returns_json`.
|
||||
|
||||
**Phase 2.8c — THE root cause** (shipped 2026-05-16): every "still
|
||||
reloads / still not working" report across 2.7 → 2.8 → 2.8b was the
|
||||
**same defect** — `shop_tags.j2` (`tag_bulk.js`) and `product_edit.j2`
|
||||
|
|
|
|||
|
|
@ -145,6 +145,7 @@
|
|||
function post(url, params) {
|
||||
const fd = new FormData();
|
||||
Object.keys(params).forEach(function (k) { fd.append(k, params[k]); });
|
||||
fd.append("ajax", "1"); // proxy-proof AJAX signal (see is_ajax)
|
||||
return fetch(url, {
|
||||
method: "POST",
|
||||
body: fd,
|
||||
|
|
|
|||
|
|
@ -102,6 +102,7 @@
|
|||
async function submitForm(form) {
|
||||
const action = form.getAttribute("data-tag-form");
|
||||
const fd = new FormData(form);
|
||||
fd.append("ajax", "1"); // proxy-proof AJAX signal (see is_ajax)
|
||||
// Disable submit button so a rapid double-click doesn't queue
|
||||
// duplicate POSTs.
|
||||
const submit = form.querySelector("button[type=submit]");
|
||||
|
|
@ -113,19 +114,46 @@
|
|||
headers: { "X-Requested-With": "XMLHttpRequest" },
|
||||
credentials: "same-origin",
|
||||
});
|
||||
if (!res.ok) throw new Error("server " + res.status);
|
||||
const data = await res.json();
|
||||
const ct = res.headers.get("content-type") || "";
|
||||
const text = await res.text();
|
||||
if (!res.ok || ct.indexOf("application/json") === -1) {
|
||||
// DO NOT full-reload. Surface the failure so it's visible
|
||||
// (this was masquerading as "the page keeps refreshing").
|
||||
reportFailure(action, res.status, ct, text);
|
||||
return;
|
||||
}
|
||||
const data = JSON.parse(text);
|
||||
flashAll(data.messages);
|
||||
dispatch(action, form, data);
|
||||
} catch (err) {
|
||||
// Fall back to a normal form submit so the user isn't stranded.
|
||||
console.warn("tag_bulk: AJAX failed, falling back to full submit:", err);
|
||||
form.submit();
|
||||
reportFailure(action, 0, "", String((err && err.message) || err));
|
||||
} finally {
|
||||
if (submit) submit.disabled = false;
|
||||
}
|
||||
}
|
||||
|
||||
/* Make a failed AJAX action VISIBLE instead of silently full-reloading
|
||||
* the page (the old `form.submit()` fallback turned every server
|
||||
* hiccup into the "whole screen refreshes" the operator reported, and
|
||||
* hid the real error). Shows the HTTP status + content-type + a body
|
||||
* snippet right in the flash region so it can be read off-screen. */
|
||||
function reportFailure(action, status, contentType, body) {
|
||||
const snippet = (body || "")
|
||||
.replace(/<[^>]+>/g, " ")
|
||||
.replace(/\s+/g, " ")
|
||||
.trim()
|
||||
.slice(0, 160);
|
||||
const where = status
|
||||
? "HTTP " + status + " · " + (contentType || "?")
|
||||
: "network/JS error";
|
||||
const msg =
|
||||
"Couldn’t " + action + " — " + where +
|
||||
". The page was NOT reloaded." +
|
||||
(snippet ? " Server said: " + snippet : "");
|
||||
console.error("tag_bulk " + action + " failed:", status, contentType, body);
|
||||
flashAll([[msg, "error"]]);
|
||||
}
|
||||
|
||||
function dispatch(action, form, data) {
|
||||
switch (action) {
|
||||
case "create": return onCreate(form, data);
|
||||
|
|
@ -177,6 +205,7 @@
|
|||
* action=reorder AJAX with {tag_slug, direction, moved}. */
|
||||
async function doReorder(form, btn) {
|
||||
const fd = new FormData(form);
|
||||
fd.append("ajax", "1"); // proxy-proof AJAX signal (see is_ajax)
|
||||
btn.disabled = true; // guard against double-click during the round-trip
|
||||
try {
|
||||
const res = await fetch(form.action, {
|
||||
|
|
@ -185,22 +214,22 @@
|
|||
headers: { "X-Requested-With": "XMLHttpRequest" },
|
||||
credentials: "same-origin",
|
||||
});
|
||||
if (!res.ok) throw new Error("server " + res.status);
|
||||
const ct = res.headers.get("content-type") || "";
|
||||
if (ct.indexOf("application/json") === -1) {
|
||||
throw new Error("non-JSON response");
|
||||
const text = await res.text();
|
||||
if (!res.ok || ct.indexOf("application/json") === -1) {
|
||||
btn.disabled = false;
|
||||
reportFailure("reorder", res.status, ct, text);
|
||||
return;
|
||||
}
|
||||
const data = await res.json();
|
||||
const data = JSON.parse(text);
|
||||
flashAll(data.messages);
|
||||
// onReorder() swaps the rows AND calls syncReorderButtonStates(),
|
||||
// which re-derives the correct disabled state for every arrow —
|
||||
// so we deliberately don't re-enable btn ourselves on success.
|
||||
onReorder(form, data);
|
||||
} catch (err) {
|
||||
// Graceful fallback: a real submit still reorders server-side
|
||||
// (the no-JS path) — page reloads, disabled state irrelevant.
|
||||
console.warn("tag_bulk: reorder AJAX failed, submitting:", err);
|
||||
form.submit();
|
||||
btn.disabled = false;
|
||||
reportFailure("reorder", 0, "", String((err && err.message) || err));
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -367,7 +396,8 @@
|
|||
async function fetchFocus(slug, skipPush) {
|
||||
if (!slug) return;
|
||||
const url =
|
||||
window.location.pathname + "?focus=" + encodeURIComponent(slug);
|
||||
window.location.pathname +
|
||||
"?focus=" + encodeURIComponent(slug) + "&ajax=1";
|
||||
const section = document.querySelector("[data-focus-section]");
|
||||
if (section) section.setAttribute("aria-busy", "true");
|
||||
try {
|
||||
|
|
@ -375,21 +405,20 @@
|
|||
headers: { "X-Requested-With": "XMLHttpRequest" },
|
||||
credentials: "same-origin",
|
||||
});
|
||||
if (!res.ok) throw new Error("server " + res.status);
|
||||
const ct = res.headers.get("content-type") || "";
|
||||
if (ct.indexOf("application/json") === -1) {
|
||||
throw new Error("non-JSON response");
|
||||
const text = await res.text();
|
||||
if (!res.ok || ct.indexOf("application/json") === -1) {
|
||||
reportFailure("focus", res.status, ct, text);
|
||||
return;
|
||||
}
|
||||
const data = await res.json();
|
||||
const data = JSON.parse(text);
|
||||
flashAll(data.messages);
|
||||
renderFocus(slug, data);
|
||||
if (!skipPush && window.history && window.history.pushState) {
|
||||
window.history.pushState({ focus: slug }, "", url);
|
||||
}
|
||||
} catch (err) {
|
||||
// Graceful fallback — a real navigation still works (no-JS path).
|
||||
console.warn("tag_bulk: focus AJAX failed, navigating:", err);
|
||||
window.location.href = url;
|
||||
reportFailure("focus", 0, "", String((err && err.message) || err));
|
||||
} finally {
|
||||
if (section) section.removeAttribute("aria-busy");
|
||||
}
|
||||
|
|
@ -529,6 +558,7 @@
|
|||
const fd = new FormData();
|
||||
fd.append("action", "set_order");
|
||||
fd.append("tag_slugs", slugs.join(","));
|
||||
fd.append("ajax", "1"); // proxy-proof AJAX signal (see is_ajax)
|
||||
fetch(window.location.pathname, {
|
||||
method: "POST",
|
||||
body: fd,
|
||||
|
|
@ -544,9 +574,9 @@
|
|||
syncReorderButtonStates(list);
|
||||
})
|
||||
.catch(function (err) {
|
||||
console.warn("tag_bulk: set_order failed, reloading:", err);
|
||||
flashAll([["Could not save the new order — reloading.", "error"]]);
|
||||
window.location.reload();
|
||||
reportFailure(
|
||||
"save order", 0, "", String((err && err.message) || err)
|
||||
);
|
||||
});
|
||||
}
|
||||
|
||||
|
|
@ -594,9 +624,54 @@
|
|||
return String(s).replace(/(["\\])/g, "\\$1");
|
||||
}
|
||||
|
||||
// Diagnostic (MPS-24): if anything stops tag_bulk.js from wiring up,
|
||||
// the operator just sees "the page keeps reloading" with no clue.
|
||||
// Surface init failures AND any uncaught page error as a visible
|
||||
// banner so the cause is readable off-screen, not buried in a
|
||||
// console the operator never opens.
|
||||
function bootBanner(text) {
|
||||
try {
|
||||
var region = document.querySelector("[data-tag-flash]");
|
||||
if (!region) return;
|
||||
var b = document.createElement("div");
|
||||
b.className = "tag-flash-toast tag-flash-error";
|
||||
b.textContent = text;
|
||||
region.appendChild(b);
|
||||
} catch (e) {}
|
||||
}
|
||||
|
||||
function safeInit() {
|
||||
try {
|
||||
init();
|
||||
// Proof-of-life: a transient confirmation the SPA wired up. If
|
||||
// the operator never sees this and the page still reloads, the
|
||||
// script isn't executing at all (CSP / 404 / cached HTML).
|
||||
bootBanner("✓ Tag editor interactive (no full reloads).");
|
||||
var ok = document.querySelector("[data-tag-flash] .tag-flash-error:last-child");
|
||||
if (ok) {
|
||||
ok.className = "tag-flash-toast tag-flash-success";
|
||||
setTimeout(function () {
|
||||
ok.style.opacity = "0";
|
||||
setTimeout(function () { ok.remove(); }, 400);
|
||||
}, 4000);
|
||||
}
|
||||
} catch (err) {
|
||||
bootBanner("tag_bulk init FAILED: " + ((err && err.message) || err));
|
||||
throw err;
|
||||
}
|
||||
}
|
||||
|
||||
window.addEventListener("error", function (ev) {
|
||||
bootBanner(
|
||||
"JS error on this page: " +
|
||||
((ev && ev.message) || "unknown") +
|
||||
(ev && ev.filename ? " @ " + ev.filename + ":" + ev.lineno : "")
|
||||
);
|
||||
});
|
||||
|
||||
if (document.readyState === "loading") {
|
||||
document.addEventListener("DOMContentLoaded", init);
|
||||
document.addEventListener("DOMContentLoaded", safeInit);
|
||||
} else {
|
||||
init();
|
||||
safeInit();
|
||||
}
|
||||
})();
|
||||
|
|
|
|||
|
|
@ -6971,6 +6971,54 @@ class TestProductTagsSpa(_AuthenticatedBase):
|
|||
self.assertEqual(pos["alpha"], 1)
|
||||
self.assertEqual(pos["bravo"], 2)
|
||||
|
||||
def test_ajax_param_signals_ajax_without_header(self):
|
||||
"""Root-cause fix: a Caddy reverse proxy on a custom-domain shop
|
||||
was dropping the X-Requested-With header, so every 'AJAX' action
|
||||
took the 302 path and the page full-reloaded. is_ajax() now also
|
||||
honours an `ajax=1` request param (URL/body — proxies don't strip
|
||||
it). Verify it returns JSON with NO X-Requested-With header."""
|
||||
shop_id, _ = self._make_product()
|
||||
res = self.testapp.post(
|
||||
f"/s/{shop_id}/tags",
|
||||
{"action": "create", "name": "ProxyProof", "ajax": "1"},
|
||||
# deliberately NO X-Requested-With header
|
||||
status=200,
|
||||
)
|
||||
self.assertIn("application/json", res.content_type)
|
||||
self.assertEqual(res.json["status"], "ok")
|
||||
self.assertEqual(res.json["tag"]["slug"], "proxyproof")
|
||||
|
||||
def test_no_ajax_signal_still_redirects(self):
|
||||
"""Without header AND without ajax=1, the no-JS path still
|
||||
302-redirects (capability-driven contract preserved)."""
|
||||
shop_id, _ = self._make_product()
|
||||
res = self.testapp.post(
|
||||
f"/s/{shop_id}/tags",
|
||||
{"action": "create", "name": "PlainPost"},
|
||||
status=302,
|
||||
)
|
||||
self.assertIn(f"/s/{shop_id}/tags", res.location)
|
||||
|
||||
def test_ajax_focus_via_param_returns_json(self):
|
||||
"""tag_bulk.js focus fetch carries &ajax=1; verify the GET focus
|
||||
branch returns JSON via the param signal (no header)."""
|
||||
shop_id, product_id = self._make_product()
|
||||
self.testapp.post(
|
||||
f"/s/{shop_id}/tags",
|
||||
{"action": "create", "name": "Focusable", "ajax": "1"},
|
||||
status=200,
|
||||
)
|
||||
self.testapp.post(
|
||||
f"/p/{product_id}/tags",
|
||||
{"action": "add", "name": "Focusable", "ajax": "1"},
|
||||
status=200,
|
||||
)
|
||||
res = self.testapp.get(
|
||||
f"/s/{shop_id}/tags?focus=focusable&ajax=1", status=200
|
||||
)
|
||||
self.assertIn("application/json", res.content_type)
|
||||
self.assertEqual(res.json["focus"]["slug"], "focusable")
|
||||
|
||||
def test_ajax_delete_tag_returns_json(self):
|
||||
"""Delete is now AJAX via the unified click handler (was inline
|
||||
onclick=confirm). Server must answer AJAX with JSON so the row
|
||||
|
|
|
|||
|
|
@ -10,11 +10,22 @@ def is_ajax(request):
|
|||
"""True when the request was made by our progressive-enhancement JS.
|
||||
|
||||
Single source of truth for the capability-driven split: a plain
|
||||
browser submit gets a flash + 302 redirect; an `fetch()` carrying
|
||||
`X-Requested-With: XMLHttpRequest` gets JSON for in-place DOM
|
||||
updates (comments, offers, bulk tagger, product-edit tag chips).
|
||||
browser submit gets a flash + 302 redirect; an `fetch()` gets JSON
|
||||
for in-place DOM updates (comments, offers, bulk tagger,
|
||||
product-edit tag chips).
|
||||
|
||||
We accept TWO independent signals because some custom-domain shops
|
||||
sit behind a Caddy reverse proxy and the `X-Requested-With` request
|
||||
header was not reaching the app — so every "AJAX" action took the
|
||||
302 path and the page full-reloaded (MPS-24 field defect). The
|
||||
`ajax=1` request param travels in the URL / form body, which no
|
||||
proxy strips, so it is the reliable signal; the header stays for
|
||||
back-compat with existing callers.
|
||||
"""
|
||||
return request.headers.get("X-Requested-With") == "XMLHttpRequest"
|
||||
return (
|
||||
request.headers.get("X-Requested-With") == "XMLHttpRequest"
|
||||
or request.params.get("ajax") == "1"
|
||||
)
|
||||
|
||||
|
||||
# view decorator.
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue