From 550d94ff1687b5ca95c4c00e16feab5c6bd65af1 Mon Sep 17 00:00:00 2001 From: Claude Agent Date: Mon, 8 Jun 2026 13:58:04 +0000 Subject: [PATCH] =?UTF-8?q?fix(mappings):=20move=20sku/codmat=20from=20pat?= =?UTF-8?q?h=20to=20query=20=E2=80=94=20slash=20in=20keys=20404'd?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codurile ROA contin legitim '/' si '"' (ex. RCR1/4"). Rutele FastAPI foloseau parametri de cale (/api/mappings/{sku}/{codmat}...); ASGI decodeaza %2F -> '/' INAINTE de routing, deci o cheie cu slash devine doua segmente si Starlette nu mai potriveste ruta -> 404 la edit/delete/restore. Cand ambii parametri au slash, un convertor {path} nu rezolva (split ambiguu). Solutie: sku/codmat trec din cale in query string. %2F/%22 din query se decodeaza ca parte a valorii, fara sa atinga routing-ul de cale. - backend: 4 rute statice + Query(...) (update/edit/delete/restore); semnaturile mapping_service raman neschimbate - frontend: helper mapQS(); toate cele 8 apeluri path -> query; cache-bust ?v=19 - tests: regresie ci-layer (chei-sentinel cu slash, !=404, toate 4 rutele); test_integration update/delete convertite la query; e2e delete RCR1/4" (slash+ghilimea) prin modalul UI, skip cand Oracle lipseste Co-Authored-By: Claude Opus 4.8 (1M context) --- api/app/routers/mappings.py | 18 ++--- api/app/static/js/mappings.js | 22 +++--- api/app/templates/mappings.html | 2 +- api/tests/e2e/test_mappings_slash_delete.py | 78 +++++++++++++++++++++ api/tests/test_app_basic.py | 22 ++++++ api/tests/test_integration.py | 9 +-- 6 files changed, 130 insertions(+), 21 deletions(-) create mode 100644 api/tests/e2e/test_mappings_slash_delete.py diff --git a/api/app/routers/mappings.py b/api/app/routers/mappings.py index 0f1eb5a..6b60b34 100644 --- a/api/app/routers/mappings.py +++ b/api/app/routers/mappings.py @@ -95,16 +95,18 @@ async def create_mapping(data: MappingCreate): except Exception as e: return {"success": False, "error": str(e)} -@router.put("/api/mappings/{sku}/{codmat}") -def update_mapping(sku: str, codmat: str, data: MappingUpdate): +# sku/codmat in query string (not path) — codurile ROA contin legitim '/' si '"' +# (ex. RCR1/4"); %2F in path e decodat de ASGI inainte de routing -> 404. +@router.put("/api/mappings/update") +def update_mapping(data: MappingUpdate, sku: str = Query(...), codmat: str = Query(...)): try: updated = mapping_service.update_mapping(sku, codmat, data.cantitate_roa, data.activ) return {"success": updated} except Exception as e: return {"success": False, "error": str(e)} -@router.put("/api/mappings/{sku}/{codmat}/edit") -def edit_mapping(sku: str, codmat: str, data: MappingEdit): +@router.put("/api/mappings/edit") +def edit_mapping(data: MappingEdit, sku: str = Query(...), codmat: str = Query(...)): try: result = mapping_service.edit_mapping(sku, codmat, data.new_sku, data.new_codmat, data.cantitate_roa) @@ -112,16 +114,16 @@ def edit_mapping(sku: str, codmat: str, data: MappingEdit): except Exception as e: return {"success": False, "error": str(e)} -@router.delete("/api/mappings/{sku}/{codmat}") -def delete_mapping(sku: str, codmat: str): +@router.delete("/api/mappings/delete") +def delete_mapping(sku: str = Query(...), codmat: str = Query(...)): try: deleted = mapping_service.delete_mapping(sku, codmat) return {"success": deleted} except Exception as e: return {"success": False, "error": str(e)} -@router.post("/api/mappings/{sku}/{codmat}/restore") -def restore_mapping(sku: str, codmat: str): +@router.post("/api/mappings/restore") +def restore_mapping(sku: str = Query(...), codmat: str = Query(...)): try: restored = mapping_service.restore_mapping(sku, codmat) return {"success": restored} diff --git a/api/app/static/js/mappings.js b/api/app/static/js/mappings.js index 86f5ad0..3ef7ae3 100644 --- a/api/app/static/js/mappings.js +++ b/api/app/static/js/mappings.js @@ -8,6 +8,12 @@ let editingMapping = null; // {sku, codmat} when editing const kitPriceCache = new Map(); +// sku/codmat in query string — codurile ROA contin legitim '/' si '"' (ex. RCR1/4"); +// %2F in path e decodat de ASGI inainte de routing -> 404. +function mapQS(sku, codmat) { + return `sku=${encodeURIComponent(sku)}&codmat=${encodeURIComponent(codmat)}`; +} + // Load on page ready document.addEventListener('DOMContentLoaded', () => { loadMappings(); @@ -242,7 +248,7 @@ function editFlatValue(span, sku, codmat, field, currentValue) { try { const body = {}; body[field] = newValue; - const res = await fetch(`/api/mappings/${encodeURIComponent(sku)}/${encodeURIComponent(codmat)}`, { + const res = await fetch(`/api/mappings/update?${mapQS(sku, codmat)}`, { method: 'PUT', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify(body) @@ -432,7 +438,7 @@ async function saveMapping() { if (editingMapping) { if (mappings.length === 1) { // Single CODMAT edit: use existing PUT endpoint - res = await fetch(`/api/mappings/${encodeURIComponent(editingMapping.sku)}/${encodeURIComponent(editingMapping.codmat)}/edit`, { + res = await fetch(`/api/mappings/edit?${mapQS(editingMapping.sku, editingMapping.codmat)}`, { method: 'PUT', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ @@ -450,7 +456,7 @@ async function saveMapping() { // Delete each existing CODMAT for old SKU for (const m of existing) { - await fetch(`/api/mappings/${encodeURIComponent(m.sku)}/${encodeURIComponent(m.codmat)}`, { + await fetch(`/api/mappings/delete?${mapQS(m.sku, m.codmat)}`, { method: 'DELETE' }); } @@ -588,7 +594,7 @@ function cancelInlineAdd() { async function toggleActive(sku, codmat, currentActive) { const newActive = currentActive ? 0 : 1; try { - const res = await fetch(`/api/mappings/${encodeURIComponent(sku)}/${encodeURIComponent(codmat)}`, { + const res = await fetch(`/api/mappings/update?${mapQS(sku, codmat)}`, { method: 'PUT', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ activ: newActive }) @@ -601,7 +607,7 @@ async function toggleActive(sku, codmat, currentActive) { // Show toast with undo const action = newActive ? 'activata' : 'dezactivata'; showUndoToast(`Mapare ${sku} \u2192 ${codmat} ${action}.`, () => { - fetch(`/api/mappings/${encodeURIComponent(sku)}/${encodeURIComponent(codmat)}`, { + fetch(`/api/mappings/update?${mapQS(sku, codmat)}`, { method: 'PUT', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ activ: currentActive }) @@ -643,7 +649,7 @@ function initDeleteModal() { if (!pendingDelete) return; const { sku, codmat } = pendingDelete; try { - const res = await fetch(`/api/mappings/${encodeURIComponent(sku)}/${encodeURIComponent(codmat)}`, { + const res = await fetch(`/api/mappings/delete?${mapQS(sku, codmat)}`, { method: 'DELETE' }); const data = await res.json(); @@ -669,7 +675,7 @@ function deleteMappingConfirm(sku, codmat) { async function restoreMapping(sku, codmat) { try { - const res = await fetch(`/api/mappings/${encodeURIComponent(sku)}/${encodeURIComponent(codmat)}/restore`, { + const res = await fetch(`/api/mappings/restore?${mapQS(sku, codmat)}`, { method: 'POST' }); const data = await res.json(); @@ -721,7 +727,7 @@ function handleMappingConflict(data) { const sku = (document.getElementById('inlineSku') || document.getElementById('inputSku'))?.value?.trim(); const codmat = (document.getElementById('inlineCodmat') || document.querySelector('.cl-codmat'))?.value?.trim(); if (sku && codmat) { - fetch(`/api/mappings/${encodeURIComponent(sku)}/${encodeURIComponent(codmat)}/restore`, { method: 'POST' }) + fetch(`/api/mappings/restore?${mapQS(sku, codmat)}`, { method: 'POST' }) .then(r => r.json()) .then(d => { if (d.success) { cancelInlineAdd(); loadMappings(); } diff --git a/api/app/templates/mappings.html b/api/app/templates/mappings.html index f0d2a72..b01fc4e 100644 --- a/api/app/templates/mappings.html +++ b/api/app/templates/mappings.html @@ -159,5 +159,5 @@ {% endblock %} {% block scripts %} - + {% endblock %} diff --git a/api/tests/e2e/test_mappings_slash_delete.py b/api/tests/e2e/test_mappings_slash_delete.py new file mode 100644 index 0000000..45b84d5 --- /dev/null +++ b/api/tests/e2e/test_mappings_slash_delete.py @@ -0,0 +1,78 @@ +"""E2E regression: delete a mapping whose SKU+CODMAT contain '/' and '"'. + +Covers the full real path: render → onclick="...jsAttrEsc(codmat)..." → +context menu "Sterge" → deleteMappingConfirm → modal confirm → fetch +/api/mappings/delete?sku=&codmat=. Exercises BOTH the slash (routing, query +param fix) and the quote (attribute escaping, commit a530ebf) end-to-end. + +Needs a real Oracle-backed app (seed inserts into ARTICOLE_TERTI with a real +CODMAT from NOM_ARTICOLE). When run against the dummy-Oracle subprocess in the +plain e2e layer, seeding fails and the test skips — it only asserts in the +./test.sh full (or live :5003) layer. +""" +import pytest +from playwright.sync_api import Page, expect + +pytestmark = pytest.mark.e2e + +SLASH_SKU = 'RCR1/4"' + + +def _real_codmat(page: Page, app_url: str): + """Fetch a real CODMAT from the Oracle nomenclator, or None if Oracle is down.""" + for term in ["01", "PH", "CA"]: + resp = page.request.get(f"{app_url}/api/articles/search?q={term}") + if resp.ok: + results = resp.json().get("results", []) + if results: + return results[0]["codmat"] + return None + + +def test_delete_mapping_with_slash_and_quote(page: Page, app_url: str): + codmat = _real_codmat(page, app_url) + if not codmat: + pytest.skip("Oracle unavailable (no real CODMAT) — slash-delete e2e needs the full layer") + + # Seed: create the slash+quote mapping. Restore if a prior run soft-deleted it. + resp = page.request.post(f"{app_url}/api/mappings", + data={"sku": SLASH_SKU, "codmat": codmat, "cantitate_roa": 1}) + body = resp.json() if resp.status in (200, 409) else {} + if not body.get("success"): + if body.get("can_restore"): + r = page.request.post( + f"{app_url}/api/mappings/restore", + params={"sku": SLASH_SKU, "codmat": codmat}) + assert r.ok and r.json().get("success"), f"restore seed failed: {r.status} {r.text()}" + elif resp.status == 503: + pytest.skip("Oracle unavailable (503 on create) — slash-delete e2e needs the full layer") + else: + pytest.fail(f"could not seed mapping: {resp.status} {resp.text()}") + + # Collect any 404 on the mappings API during the UI flow. + not_found = [] + page.on("response", lambda r: not_found.append(r.url) + if r.status == 404 and "/api/mappings/" in r.url else None) + + page.goto(f"{app_url}/mappings") + page.wait_for_load_state("networkidle") + + # Search for the slash+quote SKU. + page.fill("#searchInput", SLASH_SKU) + page.wait_for_timeout(600) # debounce + reload + + # The row's context-menu trigger carries the codmat as a data attribute. + trigger = page.locator(f'.context-menu-trigger[data-sku="{SLASH_SKU}"]').first + expect(trigger).to_be_visible() + trigger.click() + + # Context menu → "Sterge" → confirm modal. + page.locator(".context-menu-item", has_text="Sterge").click() + confirm = page.locator("#confirmDeleteBtn") + expect(confirm).to_be_visible() + confirm.click() + page.wait_for_timeout(600) # delete fetch + reload + + # Row is gone and no 404 was hit on the mappings API. + expect(page.locator(f'.context-menu-trigger[data-sku="{SLASH_SKU}"]')).to_have_count(0) + assert not not_found, f"mappings API returned 404 during delete flow: {not_found}" diff --git a/api/tests/test_app_basic.py b/api/tests/test_app_basic.py index 865977f..b9aee83 100644 --- a/api/tests/test_app_basic.py +++ b/api/tests/test_app_basic.py @@ -112,3 +112,25 @@ def test_route(client, path, expected_codes, is_oracle_route): f"GET {path} returned {resp.status_code}, expected one of {expected_codes}. " f"Body: {resp.text[:300]}" ) + + +def test_mappings_slash_in_keys_routes_ok(client): + """Regression: slash in BOTH sku and codmat used to 404 (path-param routes, + ASGI decoded %2F before routing). Now sku/codmat are query params, so the + route resolves regardless of slashes/quotes. Sentinel keys with a slash can + match no real row, so even if Oracle were up the UPDATE/WHERE is a no-op — + here pool is None so the handler returns 200 success:False without mutation. + Asserting != 404 proves the route resolved.""" + sku, codmat = '__TEST_SKU/X"', '__TEST_CODMAT/Y' + calls = [ + client.delete("/api/mappings/delete", params={"sku": sku, "codmat": codmat}), + client.put("/api/mappings/update", params={"sku": sku, "codmat": codmat}, + json={"cantitate_roa": 1}), + client.put("/api/mappings/edit", params={"sku": sku, "codmat": codmat}, + json={"new_sku": sku, "new_codmat": codmat, "cantitate_roa": 1}), + client.post("/api/mappings/restore", params={"sku": sku, "codmat": codmat}), + ] + for resp in calls: + assert resp.status_code != 404, ( + f"route did not resolve: {resp.status_code} {resp.url}" + ) diff --git a/api/tests/test_integration.py b/api/tests/test_integration.py index c7a54fa..2a8032c 100644 --- a/api/tests/test_integration.py +++ b/api/tests/test_integration.py @@ -126,16 +126,17 @@ def test_mappings_list_after_create(client, real_codmat, test_sku): def test_mappings_update(client, real_codmat, test_sku): - resp = client.put(f"/api/mappings/{test_sku}/{real_codmat}", json={ - "cantitate_roa": 3.0, - }) + resp = client.put("/api/mappings/update", + params={"sku": test_sku, "codmat": real_codmat}, + json={"cantitate_roa": 3.0}) assert resp.status_code == 200 body = resp.json() assert body.get("success") is True, f"update returned: {body}" def test_mappings_delete(client, real_codmat, test_sku): - resp = client.delete(f"/api/mappings/{test_sku}/{real_codmat}") + resp = client.delete("/api/mappings/delete", + params={"sku": test_sku, "codmat": real_codmat}) assert resp.status_code == 200 body = resp.json() assert body.get("success") is True, f"delete returned: {body}"