From 8e31124ca8b3a72ae9c5be6341dba36abfc6fb86 Mon Sep 17 00:00:00 2001 From: "russell@unturf.com" Date: Sat, 16 May 2026 10:16:37 -0400 Subject: [PATCH] =?UTF-8?q?fix:=20MPS-24=20Phase=202.8d=20=E2=80=94=20prox?= =?UTF-8?q?y-proof=20AJAX=20signal=20(the=20actual=20root=20cause)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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). --- CLAUDE.md | 9 +- docs/tickets/mps-24.md | 31 ++++++ make_post_sell/static/js/product_tags.js | 1 + make_post_sell/static/js/tag_bulk.js | 127 ++++++++++++++++++----- make_post_sell/tests/test_functional.py | 48 +++++++++ make_post_sell/views/__init__.py | 19 +++- 6 files changed, 204 insertions(+), 31 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index ae827ac..d116b6a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -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 diff --git a/docs/tickets/mps-24.md b/docs/tickets/mps-24.md index 1161a7f..5a7a3d6 100644 --- a/docs/tickets/mps-24.md +++ b/docs/tickets/mps-24.md @@ -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=` 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` diff --git a/make_post_sell/static/js/product_tags.js b/make_post_sell/static/js/product_tags.js index f6e8552..1c829be 100644 --- a/make_post_sell/static/js/product_tags.js +++ b/make_post_sell/static/js/product_tags.js @@ -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, diff --git a/make_post_sell/static/js/tag_bulk.js b/make_post_sell/static/js/tag_bulk.js index e2e3aca..4b2ec83 100644 --- a/make_post_sell/static/js/tag_bulk.js +++ b/make_post_sell/static/js/tag_bulk.js @@ -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(); } })(); diff --git a/make_post_sell/tests/test_functional.py b/make_post_sell/tests/test_functional.py index 8fa2d24..50287dd 100644 --- a/make_post_sell/tests/test_functional.py +++ b/make_post_sell/tests/test_functional.py @@ -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 diff --git a/make_post_sell/views/__init__.py b/make_post_sell/views/__init__.py index af3c608..24716a3 100644 --- a/make_post_sell/views/__init__.py +++ b/make_post_sell/views/__init__.py @@ -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.