fix(mappings): move sku/codmat from path to query — slash in keys 404'd
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) <noreply@anthropic.com>
This commit is contained in:
@@ -95,16 +95,18 @@ async def create_mapping(data: MappingCreate):
|
|||||||
except Exception as e:
|
except Exception as e:
|
||||||
return {"success": False, "error": str(e)}
|
return {"success": False, "error": str(e)}
|
||||||
|
|
||||||
@router.put("/api/mappings/{sku}/{codmat}")
|
# sku/codmat in query string (not path) — codurile ROA contin legitim '/' si '"'
|
||||||
def update_mapping(sku: str, codmat: str, data: MappingUpdate):
|
# (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:
|
try:
|
||||||
updated = mapping_service.update_mapping(sku, codmat, data.cantitate_roa, data.activ)
|
updated = mapping_service.update_mapping(sku, codmat, data.cantitate_roa, data.activ)
|
||||||
return {"success": updated}
|
return {"success": updated}
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
return {"success": False, "error": str(e)}
|
return {"success": False, "error": str(e)}
|
||||||
|
|
||||||
@router.put("/api/mappings/{sku}/{codmat}/edit")
|
@router.put("/api/mappings/edit")
|
||||||
def edit_mapping(sku: str, codmat: str, data: MappingEdit):
|
def edit_mapping(data: MappingEdit, sku: str = Query(...), codmat: str = Query(...)):
|
||||||
try:
|
try:
|
||||||
result = mapping_service.edit_mapping(sku, codmat, data.new_sku, data.new_codmat,
|
result = mapping_service.edit_mapping(sku, codmat, data.new_sku, data.new_codmat,
|
||||||
data.cantitate_roa)
|
data.cantitate_roa)
|
||||||
@@ -112,16 +114,16 @@ def edit_mapping(sku: str, codmat: str, data: MappingEdit):
|
|||||||
except Exception as e:
|
except Exception as e:
|
||||||
return {"success": False, "error": str(e)}
|
return {"success": False, "error": str(e)}
|
||||||
|
|
||||||
@router.delete("/api/mappings/{sku}/{codmat}")
|
@router.delete("/api/mappings/delete")
|
||||||
def delete_mapping(sku: str, codmat: str):
|
def delete_mapping(sku: str = Query(...), codmat: str = Query(...)):
|
||||||
try:
|
try:
|
||||||
deleted = mapping_service.delete_mapping(sku, codmat)
|
deleted = mapping_service.delete_mapping(sku, codmat)
|
||||||
return {"success": deleted}
|
return {"success": deleted}
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
return {"success": False, "error": str(e)}
|
return {"success": False, "error": str(e)}
|
||||||
|
|
||||||
@router.post("/api/mappings/{sku}/{codmat}/restore")
|
@router.post("/api/mappings/restore")
|
||||||
def restore_mapping(sku: str, codmat: str):
|
def restore_mapping(sku: str = Query(...), codmat: str = Query(...)):
|
||||||
try:
|
try:
|
||||||
restored = mapping_service.restore_mapping(sku, codmat)
|
restored = mapping_service.restore_mapping(sku, codmat)
|
||||||
return {"success": restored}
|
return {"success": restored}
|
||||||
|
|||||||
@@ -8,6 +8,12 @@ let editingMapping = null; // {sku, codmat} when editing
|
|||||||
|
|
||||||
const kitPriceCache = new Map();
|
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
|
// Load on page ready
|
||||||
document.addEventListener('DOMContentLoaded', () => {
|
document.addEventListener('DOMContentLoaded', () => {
|
||||||
loadMappings();
|
loadMappings();
|
||||||
@@ -242,7 +248,7 @@ function editFlatValue(span, sku, codmat, field, currentValue) {
|
|||||||
try {
|
try {
|
||||||
const body = {};
|
const body = {};
|
||||||
body[field] = newValue;
|
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',
|
method: 'PUT',
|
||||||
headers: { 'Content-Type': 'application/json' },
|
headers: { 'Content-Type': 'application/json' },
|
||||||
body: JSON.stringify(body)
|
body: JSON.stringify(body)
|
||||||
@@ -432,7 +438,7 @@ async function saveMapping() {
|
|||||||
if (editingMapping) {
|
if (editingMapping) {
|
||||||
if (mappings.length === 1) {
|
if (mappings.length === 1) {
|
||||||
// Single CODMAT edit: use existing PUT endpoint
|
// 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',
|
method: 'PUT',
|
||||||
headers: { 'Content-Type': 'application/json' },
|
headers: { 'Content-Type': 'application/json' },
|
||||||
body: JSON.stringify({
|
body: JSON.stringify({
|
||||||
@@ -450,7 +456,7 @@ async function saveMapping() {
|
|||||||
|
|
||||||
// Delete each existing CODMAT for old SKU
|
// Delete each existing CODMAT for old SKU
|
||||||
for (const m of existing) {
|
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'
|
method: 'DELETE'
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
@@ -588,7 +594,7 @@ function cancelInlineAdd() {
|
|||||||
async function toggleActive(sku, codmat, currentActive) {
|
async function toggleActive(sku, codmat, currentActive) {
|
||||||
const newActive = currentActive ? 0 : 1;
|
const newActive = currentActive ? 0 : 1;
|
||||||
try {
|
try {
|
||||||
const res = await fetch(`/api/mappings/${encodeURIComponent(sku)}/${encodeURIComponent(codmat)}`, {
|
const res = await fetch(`/api/mappings/update?${mapQS(sku, codmat)}`, {
|
||||||
method: 'PUT',
|
method: 'PUT',
|
||||||
headers: { 'Content-Type': 'application/json' },
|
headers: { 'Content-Type': 'application/json' },
|
||||||
body: JSON.stringify({ activ: newActive })
|
body: JSON.stringify({ activ: newActive })
|
||||||
@@ -601,7 +607,7 @@ async function toggleActive(sku, codmat, currentActive) {
|
|||||||
// Show toast with undo
|
// Show toast with undo
|
||||||
const action = newActive ? 'activata' : 'dezactivata';
|
const action = newActive ? 'activata' : 'dezactivata';
|
||||||
showUndoToast(`Mapare ${sku} \u2192 ${codmat} ${action}.`, () => {
|
showUndoToast(`Mapare ${sku} \u2192 ${codmat} ${action}.`, () => {
|
||||||
fetch(`/api/mappings/${encodeURIComponent(sku)}/${encodeURIComponent(codmat)}`, {
|
fetch(`/api/mappings/update?${mapQS(sku, codmat)}`, {
|
||||||
method: 'PUT',
|
method: 'PUT',
|
||||||
headers: { 'Content-Type': 'application/json' },
|
headers: { 'Content-Type': 'application/json' },
|
||||||
body: JSON.stringify({ activ: currentActive })
|
body: JSON.stringify({ activ: currentActive })
|
||||||
@@ -643,7 +649,7 @@ function initDeleteModal() {
|
|||||||
if (!pendingDelete) return;
|
if (!pendingDelete) return;
|
||||||
const { sku, codmat } = pendingDelete;
|
const { sku, codmat } = pendingDelete;
|
||||||
try {
|
try {
|
||||||
const res = await fetch(`/api/mappings/${encodeURIComponent(sku)}/${encodeURIComponent(codmat)}`, {
|
const res = await fetch(`/api/mappings/delete?${mapQS(sku, codmat)}`, {
|
||||||
method: 'DELETE'
|
method: 'DELETE'
|
||||||
});
|
});
|
||||||
const data = await res.json();
|
const data = await res.json();
|
||||||
@@ -669,7 +675,7 @@ function deleteMappingConfirm(sku, codmat) {
|
|||||||
|
|
||||||
async function restoreMapping(sku, codmat) {
|
async function restoreMapping(sku, codmat) {
|
||||||
try {
|
try {
|
||||||
const res = await fetch(`/api/mappings/${encodeURIComponent(sku)}/${encodeURIComponent(codmat)}/restore`, {
|
const res = await fetch(`/api/mappings/restore?${mapQS(sku, codmat)}`, {
|
||||||
method: 'POST'
|
method: 'POST'
|
||||||
});
|
});
|
||||||
const data = await res.json();
|
const data = await res.json();
|
||||||
@@ -721,7 +727,7 @@ function handleMappingConflict(data) {
|
|||||||
const sku = (document.getElementById('inlineSku') || document.getElementById('inputSku'))?.value?.trim();
|
const sku = (document.getElementById('inlineSku') || document.getElementById('inputSku'))?.value?.trim();
|
||||||
const codmat = (document.getElementById('inlineCodmat') || document.querySelector('.cl-codmat'))?.value?.trim();
|
const codmat = (document.getElementById('inlineCodmat') || document.querySelector('.cl-codmat'))?.value?.trim();
|
||||||
if (sku && codmat) {
|
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(r => r.json())
|
||||||
.then(d => {
|
.then(d => {
|
||||||
if (d.success) { cancelInlineAdd(); loadMappings(); }
|
if (d.success) { cancelInlineAdd(); loadMappings(); }
|
||||||
|
|||||||
@@ -159,5 +159,5 @@
|
|||||||
{% endblock %}
|
{% endblock %}
|
||||||
|
|
||||||
{% block scripts %}
|
{% block scripts %}
|
||||||
<script src="{{ request.scope.get('root_path', '') }}/static/js/mappings.js?v=18"></script>
|
<script src="{{ request.scope.get('root_path', '') }}/static/js/mappings.js?v=19"></script>
|
||||||
{% endblock %}
|
{% endblock %}
|
||||||
|
|||||||
78
api/tests/e2e/test_mappings_slash_delete.py
Normal file
78
api/tests/e2e/test_mappings_slash_delete.py
Normal file
@@ -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}"
|
||||||
@@ -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"GET {path} returned {resp.status_code}, expected one of {expected_codes}. "
|
||||||
f"Body: {resp.text[:300]}"
|
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}"
|
||||||
|
)
|
||||||
|
|||||||
@@ -126,16 +126,17 @@ def test_mappings_list_after_create(client, real_codmat, test_sku):
|
|||||||
|
|
||||||
|
|
||||||
def test_mappings_update(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={
|
resp = client.put("/api/mappings/update",
|
||||||
"cantitate_roa": 3.0,
|
params={"sku": test_sku, "codmat": real_codmat},
|
||||||
})
|
json={"cantitate_roa": 3.0})
|
||||||
assert resp.status_code == 200
|
assert resp.status_code == 200
|
||||||
body = resp.json()
|
body = resp.json()
|
||||||
assert body.get("success") is True, f"update returned: {body}"
|
assert body.get("success") is True, f"update returned: {body}"
|
||||||
|
|
||||||
|
|
||||||
def test_mappings_delete(client, real_codmat, test_sku):
|
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
|
assert resp.status_code == 200
|
||||||
body = resp.json()
|
body = resp.json()
|
||||||
assert body.get("success") is True, f"delete returned: {body}"
|
assert body.get("success") is True, f"delete returned: {body}"
|
||||||
|
|||||||
Reference in New Issue
Block a user