fix: MPS-24 Phase 2.8b — one unified AJAX click handler for all bulk-tagger actions
Operator: 'same with the delete button. and add' — i.e. Add / Delete (and reorder) still full-reloaded. The generic data-tag-form submit-EVENT interception is unreliable in the field; the explicit click handlers (focus/drag) work. Root fix instead of patching each button: one capture-phase CLICK handler (onTagFormClick) on every submit control inside form[data-tag-form]. - onTagFormClick preventDefault()s so the native submit never starts (no reload, no double-handling), runs the delete confirm via data-confirm, routes reorder -> doReorder (in-place swap), everything else (create/add, delete, attach/detach, apply/dismiss suggestion) -> submitForm. - Removed inline onclick="return confirm()" from shop_tags.j2 AND the JS appendTagRow builder — it fought the interception; now data-confirm. - submit listener kept only as the Enter-key fallback. Standalone wireReorderButtons folded into onTagFormClick. Dead escapeJs removed. - Tests: +test_ajax_delete_tag_returns_json, +test_bulk_tagger_delete_uses_data_confirm_not_inline_onclick, +test_ajax_reorder_arrow_returns_json_and_moves. 1131 passed. Docs: mps-24.md Phase 2.8b.
This commit is contained in:
parent
155f7ff66f
commit
b15c0a0f38
4 changed files with 177 additions and 15 deletions
|
|
@ -363,6 +363,24 @@ 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.8b** (shipped 2026-05-16): the generic `data-tag-form`
|
||||
**`submit`-event** interception proved unreliable in the field —
|
||||
operator reported Add / Delete / reorder *all* still full-reloaded
|
||||
while the explicit click handlers (focus/drag) worked. Root fix:
|
||||
**one unified capture-phase `click` handler** (`onTagFormClick`)
|
||||
on every submit control inside `form[data-tag-form]`. It
|
||||
`preventDefault()`s (native submit never starts → no reload, no
|
||||
double-handling), runs the delete confirm via `data-confirm`
|
||||
(inline `onclick="return confirm()"` removed from `shop_tags.j2`
|
||||
**and** the JS `appendTagRow` builder — it fought the interception),
|
||||
routes `reorder` → `doReorder` (in-place swap) and everything else
|
||||
(create/add, delete, attach/detach, apply/dismiss suggestion) →
|
||||
`submitForm`. The `submit` listener is kept only as the Enter-key
|
||||
fallback; `escapeJs` removed (dead after the onclick→data-confirm
|
||||
switch). Tests: `test_ajax_reorder_arrow_returns_json_and_moves`,
|
||||
`test_ajax_delete_tag_returns_json`,
|
||||
`test_bulk_tagger_delete_uses_data_confirm_not_inline_onclick`.
|
||||
|
||||
### Phase 2.7 — per-product SPA tag chips on product edit (shipped 2026-05-16)
|
||||
|
||||
Operator report: adding/removing a tag on the product edit page
|
||||
|
|
|
|||
|
|
@ -26,18 +26,56 @@
|
|||
// self-gates inside maybeIntercept().
|
||||
document.querySelectorAll("form[data-tag-form]").forEach(attach);
|
||||
|
||||
// Event delegation isn't enough because we re-render rows; we
|
||||
// re-attach to any new forms after each mutation via attachAll().
|
||||
// PRIMARY interception path (MPS-24 Phase 2.8b): a capture-phase
|
||||
// *click* listener on submit controls inside [data-tag-form] forms.
|
||||
// The generic `submit`-event interception below proved unreliable
|
||||
// in the field (every Add / Delete / reorder still full-reloaded
|
||||
// for the operator). A click handler that preventDefault()s stops
|
||||
// the native form submission BEFORE it starts, so the submit event
|
||||
// never fires — no double-handling — and the page never reloads.
|
||||
document.addEventListener("click", onTagFormClick, true);
|
||||
|
||||
// SECONDARY fallback: keep the submit listener so keyboard submits
|
||||
// (Enter in the "Add tag" field) are still AJAX. Mutually exclusive
|
||||
// with the click path (a prevented click never emits `submit`).
|
||||
document.addEventListener("submit", maybeIntercept, true);
|
||||
|
||||
// MPS-24 Phase 2.8: the heavy full-page reload on this page was the
|
||||
// tag-focus navigation (a plain <a href="?focus=">), not the forms.
|
||||
// On a 481-product catalog every tag click reloaded the whole bulk
|
||||
// tagger. Intercept it + wire real drag-to-reorder.
|
||||
// MPS-24 Phase 2.8: the other heavy full-page reload was the
|
||||
// tag-focus navigation (a plain <a href="?focus=">). Intercept it +
|
||||
// wire real drag-to-reorder.
|
||||
wireFocusLinks();
|
||||
wireDragAndDrop();
|
||||
}
|
||||
|
||||
/* The one robust path every button-driven tag action flows through.
|
||||
* Routes reorder to doReorder() (in-place row swap) and everything
|
||||
* else to submitForm(); handles the delete confirm via data-confirm
|
||||
* so there's no inline onclick fighting the interception. */
|
||||
function onTagFormClick(ev) {
|
||||
if (
|
||||
ev.button !== 0 ||
|
||||
ev.metaKey || ev.ctrlKey || ev.shiftKey || ev.altKey
|
||||
) {
|
||||
return; // let the browser do its thing for modified clicks
|
||||
}
|
||||
const btn = ev.target.closest(
|
||||
'form[data-tag-form] button[type="submit"],' +
|
||||
'form[data-tag-form] input[type="submit"]'
|
||||
);
|
||||
if (!btn) return;
|
||||
const form = btn.closest("form[data-tag-form]");
|
||||
if (!form) return;
|
||||
ev.preventDefault(); // kills the native submit + full reload
|
||||
if (btn.disabled) return;
|
||||
const confirmMsg = btn.getAttribute("data-confirm");
|
||||
if (confirmMsg && !window.confirm(confirmMsg)) return;
|
||||
if (form.getAttribute("data-tag-form") === "reorder") {
|
||||
doReorder(form, btn);
|
||||
return;
|
||||
}
|
||||
submitForm(form);
|
||||
}
|
||||
|
||||
function attachAll(root) {
|
||||
(root || document).querySelectorAll("form[data-tag-form]").forEach(attach);
|
||||
}
|
||||
|
|
@ -51,9 +89,12 @@
|
|||
const form = ev.target;
|
||||
if (!(form instanceof HTMLFormElement)) return;
|
||||
if (!form.hasAttribute("data-tag-form")) return;
|
||||
// Confirm dialog is on the delete button via inline onclick; if it
|
||||
// returned false the submit never fires, so we don't need to repeat
|
||||
// the check here.
|
||||
// Keyboard-submit fallback (e.g. Enter in the "Add tag" field).
|
||||
// Button clicks are handled earlier by onTagFormClick(), which
|
||||
// preventDefault()s so this never fires for them. Reorder via
|
||||
// keyboard is degenerate (arrows are buttons), so route the rest
|
||||
// through submitForm(); the delete confirm only matters on the
|
||||
// click path.
|
||||
ev.preventDefault();
|
||||
submitForm(form);
|
||||
}
|
||||
|
|
@ -130,6 +171,39 @@
|
|||
});
|
||||
}
|
||||
|
||||
/* MPS-24 Phase 2.8b: ↑/↓ reorder is AJAX. Routed here from the
|
||||
* unified onTagFormClick() handler (action === "reorder"); reuses
|
||||
* onReorder() for the in-place row swap. The view already answers
|
||||
* action=reorder AJAX with {tag_slug, direction, moved}. */
|
||||
async function doReorder(form, btn) {
|
||||
const fd = new FormData(form);
|
||||
btn.disabled = true; // guard against double-click during the round-trip
|
||||
try {
|
||||
const res = await fetch(form.action, {
|
||||
method: "POST",
|
||||
body: fd,
|
||||
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 data = await res.json();
|
||||
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();
|
||||
}
|
||||
}
|
||||
|
||||
/* ----- handlers --------------------------------------------------- */
|
||||
|
||||
function onCreate(form, data) {
|
||||
|
|
@ -211,7 +285,7 @@
|
|||
'<input type="hidden" name="action" value="delete" />' +
|
||||
'<input type="hidden" name="tag_slug" value="' + escapeHtml(tag.slug) + '" />' +
|
||||
'<button type="submit" class="mps-button mps-button-small mps-button-red" ' +
|
||||
'onclick="return confirm(\'Delete tag ' + escapeJs(tag.name) + '?\');">Delete</button>' +
|
||||
'data-confirm="Delete tag ' + escapeHtml(tag.name) + '?">Delete</button>' +
|
||||
'</form>';
|
||||
list.appendChild(li);
|
||||
attachAll(li);
|
||||
|
|
@ -515,10 +589,6 @@
|
|||
.replace(/'/g, "'");
|
||||
}
|
||||
|
||||
function escapeJs(s) {
|
||||
return String(s).replace(/\\/g, "\\\\").replace(/'/g, "\\'");
|
||||
}
|
||||
|
||||
function cssEscape(s) {
|
||||
if (window.CSS && window.CSS.escape) return window.CSS.escape(s);
|
||||
return String(s).replace(/(["\\])/g, "\\$1");
|
||||
|
|
|
|||
|
|
@ -131,7 +131,7 @@
|
|||
<input type="hidden" name="action" value="delete" />
|
||||
<input type="hidden" name="tag_slug" value="{{ tag.slug }}" />
|
||||
<button type="submit" class="mps-button mps-button-small mps-button-red"
|
||||
onclick="return confirm('Delete tag {{ tag.name }}?');">Delete</button>
|
||||
data-confirm="Delete tag {{ tag.name }}?">Delete</button>
|
||||
</form>
|
||||
</li>
|
||||
{% endfor %}
|
||||
|
|
|
|||
|
|
@ -6971,6 +6971,80 @@ class TestProductTagsSpa(_AuthenticatedBase):
|
|||
self.assertEqual(pos["alpha"], 1)
|
||||
self.assertEqual(pos["bravo"], 2)
|
||||
|
||||
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
|
||||
is removed in place — no full page reload."""
|
||||
shop_id, _ = self._make_product()
|
||||
self.testapp.post(
|
||||
f"/s/{shop_id}/tags",
|
||||
{"action": "create", "name": "Doomed"},
|
||||
headers={"X-Requested-With": "XMLHttpRequest"}, status=200,
|
||||
)
|
||||
res = self.testapp.post(
|
||||
f"/s/{shop_id}/tags",
|
||||
{"action": "delete", "tag_slug": "doomed"},
|
||||
headers={"X-Requested-With": "XMLHttpRequest"},
|
||||
status=200,
|
||||
)
|
||||
self.assertIn("application/json", res.content_type)
|
||||
self.assertEqual(res.json["status"], "ok")
|
||||
self.assertEqual(res.json["deleted_slug"], "doomed")
|
||||
from ..models.tag import get_tag_by_shop_and_slug
|
||||
from ..models.shop import get_shop_by_id
|
||||
self.dbsession.expire_all()
|
||||
shop = get_shop_by_id(self.dbsession, shop_id)
|
||||
self.assertIsNone(
|
||||
get_tag_by_shop_and_slug(self.dbsession, shop, "doomed")
|
||||
)
|
||||
|
||||
def test_bulk_tagger_delete_uses_data_confirm_not_inline_onclick(self):
|
||||
"""Regression: the delete confirm must be data-confirm (owned by
|
||||
the unified click handler), not an inline onclick that fights
|
||||
the interception and lets the page reload."""
|
||||
shop_id, _ = self._make_product()
|
||||
self.testapp.post(
|
||||
f"/s/{shop_id}/tags",
|
||||
{"action": "create", "name": "Checkme"},
|
||||
headers={"X-Requested-With": "XMLHttpRequest"}, status=200,
|
||||
)
|
||||
res = self.testapp.get(f"/s/{shop_id}/tags", status=200)
|
||||
body = res.body.decode()
|
||||
self.assertIn('data-confirm="Delete tag Checkme?"', body)
|
||||
self.assertNotIn("onclick=\"return confirm", body)
|
||||
|
||||
def test_ajax_reorder_arrow_returns_json_and_moves(self):
|
||||
"""The ↑/↓ arrows POST action=reorder&direction=up|down and must
|
||||
answer AJAX with JSON {tag_slug, direction, moved} so tag_bulk.js
|
||||
swaps the row in place — no full page reload."""
|
||||
from ..models.tag import get_tag_by_shop_and_slug
|
||||
from ..models.shop import get_shop_by_id
|
||||
shop_id, _ = self._make_product()
|
||||
for name in ("First", "Second"):
|
||||
self.testapp.post(
|
||||
f"/s/{shop_id}/tags",
|
||||
{"action": "create", "name": name},
|
||||
headers={"X-Requested-With": "XMLHttpRequest"}, status=200,
|
||||
)
|
||||
# Move "Second" up — it should swap ahead of "First".
|
||||
res = self.testapp.post(
|
||||
f"/s/{shop_id}/tags",
|
||||
{"action": "reorder", "tag_slug": "second", "direction": "up"},
|
||||
headers={"X-Requested-With": "XMLHttpRequest"},
|
||||
status=200,
|
||||
)
|
||||
self.assertIn("application/json", res.content_type)
|
||||
data = res.json
|
||||
self.assertEqual(data["status"], "ok")
|
||||
self.assertEqual(data["tag_slug"], "second")
|
||||
self.assertEqual(data["direction"], "up")
|
||||
self.assertTrue(data["moved"])
|
||||
self.dbsession.expire_all()
|
||||
shop = get_shop_by_id(self.dbsession, shop_id)
|
||||
second = get_tag_by_shop_and_slug(self.dbsession, shop, "second")
|
||||
first = get_tag_by_shop_and_slug(self.dbsession, shop, "first")
|
||||
self.assertLess(second.position, first.position)
|
||||
|
||||
def test_bulk_tagger_bare_get_renders_without_products(self):
|
||||
"""Phase 2.8 perf: a bare GET (no focus / no suggestions) must
|
||||
not load+render the whole catalog. Page renders, focus section
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue