From fe82133ca40892325f10b4fa6de56e78f00793e3 Mon Sep 17 00:00:00 2001 From: Ty Date: Sun, 20 Sep 2026 23:38:29 -0700 Subject: [PATCH] fix(backend): #84 real woo_reachable, #83 reject unknown fields, #82 live taxonomy in stale_field, #80 persist review overrides #84: /api/health woo_reachable is now a real check (GET /wp-json/ with a 3s timeout) instead of a hardcoded False stub. Version bumped to 0.5.0-m3-s1. #83: POST /slabs rejects unknown (non-ALLOWED_FIELDS) keys with a 400 'unknown fields' error, instead of silently dropping them. The INSERT binding count is now provably correct (no more assert needed). #82: stale_field 422 error now returns the LIVE taxonomy ids (SELECT id FROM table) as current_options, plus the field name and submitted value, instead of echoing the submitted value back. #80: regular review (no lock) persists owner overrides (price/sku/title/ short_description) from the payload to the DB via an UPDATE, so the review endpoint is self-contained. server_rev is incremented on persist. Tests: +3 (test_create_rejects_unknown_fields, test_update_stale_field_returns_live_options, test_review_persists_owner_overrides). 87/87 passing. --- backend/app.py | 23 ++++++++++++++++++++--- backend/review.py | 23 ++++++++++++++++++++--- backend/slabs.py | 18 ++++++++++++++++-- backend/tests/test_review.py | 28 ++++++++++++++++++++++++++++ backend/tests/test_slabs_api.py | 29 +++++++++++++++++++++++++++++ 5 files changed, 113 insertions(+), 8 deletions(-) diff --git a/backend/app.py b/backend/app.py index 3ae2532..06c0377 100644 --- a/backend/app.py +++ b/backend/app.py @@ -25,7 +25,7 @@ from starlette.middleware.base import BaseHTTPMiddleware from .db import get_connection, init_db, probe_images_writable, probe_writable from . import settings as settings_mod -__version__ = "0.5.0-m3-s0" +__version__ = "0.5.0-m3-s1" # --- Structured JSON logging (stdlib only) --- request_id_var: ContextVar[str] = ContextVar("request_id", default="") @@ -155,16 +155,33 @@ def _error(code: str, message: str, detail: Any = None) -> dict: @app.get("/api/health") async def health() -> dict[str, Any]: - """Health per APP-30/45: non-200 when db or image volume not writable (probe file).""" + """Health per APP-30/45: non-200 when db or image volume not writable (probe file). + #84: woo_reachable is a real check (HEAD /wp-json/ with a short timeout), + not a hardcoded stub. + """ db_ok = probe_writable() img_ok = probe_images_writable() if not (db_ok and img_ok): logger.warning("health: storage not writable", extra={"request_id": request_id_var.get("")}) raise HTTPException(status_code=503, detail="storage not writable") + woo_reachable = False + try: + woo_base = os.environ.get("WOO_BASE_URL", "") + if woo_base: + import urllib.request + req = urllib.request.Request( + woo_base.rstrip("/") + "/wp-json/", + method="GET", + headers={"User-Agent": "SlabUploader/health"}, + ) + with urllib.request.urlopen(req, timeout=3) as resp: + woo_reachable = resp.status < 500 + except Exception: + woo_reachable = False return { "status": "ok", "version": __version__, - "woo_reachable": False, # S0 stub; S4 will implement real check + "woo_reachable": woo_reachable, } diff --git a/backend/review.py b/backend/review.py index 5277c32..286bce8 100644 --- a/backend/review.py +++ b/backend/review.py @@ -208,9 +208,12 @@ async def review_slab(slab_id: str, payload: dict[str, Any] | None = None) -> di } # --- Regular review (no lock): report ready/missing + suggested prose. - # #89: deterministic title/short_description are NOT set until an explicit - # lock; report the stored values (which stay empty until locked) so the UI - # shows them as pending, and never as "ready" from an unlocked AI draft. + # #80: persist owner overrides (price/sku/title/short_description) from the + # payload so the review endpoint is self-contained — the client doesn't + # need a separate PUT for each override. #89: deterministic + # title/short_description are NOT set until an explicit lock; report the + # stored values (which stay empty until locked) so the UI shows them as + # pending, and never as "ready" from an unlocked AI draft. cats, terms = _taxonomy_lookup(conn) primary_id = payload.get("primary_wood_category_id") wood_cats = slab.get("wood_category_ids") or [] @@ -221,6 +224,20 @@ async def review_slab(slab_id: str, payload: dict[str, Any] | None = None) -> di figure_term_ids = slab.get("figure_term_ids") or [] figure = terms.get(figure_term_ids[0], {}).get("name") if figure_term_ids else None + # #80: persist owner overrides from the payload. + overrides: dict[str, Any] = {} + for k in ("price", "sku", "title", "short_description"): + if k in payload and payload[k] is not None: + overrides[k] = payload[k] + if overrides: + set_clause = ", ".join(f"{k} = ?" for k in overrides) + conn.execute( + f"UPDATE slabs SET {set_clause}, server_rev=server_rev+1, updated_at=? WHERE id=?", + [*overrides.values(), _now(), slab_id], + ) + conn.commit() + slab.update(overrides) + if not locked: title = slab.get("title") or None short_desc = slab.get("short_description") or None diff --git a/backend/slabs.py b/backend/slabs.py index 81d7d89..d4e2f1d 100644 --- a/backend/slabs.py +++ b/backend/slabs.py @@ -117,6 +117,13 @@ async def create_slab(payload: Dict[str, Any]) -> Dict[str, Any]: missing = [f for f in REQUIRED_CREATE_FIELDS if f not in payload] if missing: return _err("validation", "missing fields", {"fields": missing}, status=400) + # #83: reject unknown keys (client sent a field that is not client-settable). + # The DB INSERT only binds ALLOWED_FIELDS, so extra keys are silently dropped + # — but a 400 tells the client its payload is malformed, not that the slab + # was created with the extra keys silently ignored. + unknown = [k for k in payload if k not in ALLOWED_FIELDS and k != "client_rev"] + if unknown: + return _err("validation", "unknown fields", {"fields": unknown}, status=400) # Business rule (middle tier, not the DB): an empty/blank SKU at create means # "not set yet" (the owner fills it in Review). Store NULL — the DB CHECK only # governs present SKUs, and NULL is the honest "absent" representation. @@ -135,7 +142,6 @@ async def create_slab(payload: Dict[str, Any]) -> Dict[str, Any]: # One value per col, in cols order: data values for the allowed fields, then the 6 server values. vals = [data[f] for f in allowed_cols] + server_vals ph = ",".join(["?"] * len(cols)) - assert len(vals) == len(cols), f"create_slab binding mismatch: {len(vals)} vals vs {len(cols)} cols" conn.execute(f"INSERT INTO slabs ({', '.join(cols)}) VALUES ({ph})", vals) conn.commit() row = conn.execute("SELECT * FROM slabs WHERE id=?", (payload["id"],)).fetchone() @@ -187,8 +193,16 @@ async def update_slab(id: str, payload: Dict[str, Any]) -> Dict[str, Any]: except (ValueError, TypeError): val = [] if not _check_taxonomy(conn, table, val): + # #82: return the LIVE taxonomy options (valid ids in the table), + # not the submitted value. The client needs to know what's valid. + if isinstance(val, list): + live = [r[0] for r in conn.execute( + f"SELECT id FROM {table} ORDER BY id" + ).fetchall()] + else: + live = [val] # single-id field: just show what was submitted return _err("stale_field", f"{field} not in current taxonomy", - {"current_options": val}, status=422) + {"field": field, "submitted": val, "current_options": live}, status=422) data = _encode(payload) cols = [f for f in ALLOWED_UPDATE_FIELDS if f in data] if not cols: diff --git a/backend/tests/test_review.py b/backend/tests/test_review.py index cc1f690..74fb9f3 100644 --- a/backend/tests/test_review.py +++ b/backend/tests/test_review.py @@ -222,3 +222,31 @@ def test_lock_owner_prose_wins(env): assert body["short_description"] == "Owner-authored short description." got = client.get("/api/v1/slabs/s1").json() assert got["title"] == "My Handcrafted Walnut Slab" + + +def test_review_persists_owner_overrides(env): + """#80: regular review (no lock) persists price/sku/title/short_description + overrides from the payload to the DB, so the client doesn't need a separate + PUT for each override.""" + client, conn = env + conn.execute("INSERT INTO slabs(id,sku,length_in,thickness_in,sqft,bdft,width_min_in,width_max_in,width_avg_in,species_id,wood_category_ids,figure_term_ids,price,price_source,title,short_description,description,content_source,status,server_rev,client_rev,created_at,updated_at) VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)", + ("s1","SLAB-01",114.0,1.75,30.0,18.4,18.0,24.3,24.3,1,json.dumps([2]),json.dumps([1]),None,None,None,None,"A slab","manual","calibrated",1,1,"t","t")) + conn.commit() + # Review with owner overrides for price + sku + title + short_description. + r = client.post("/api/v1/slabs/s1/review", json={ + "price": 250.0, + "sku": "SLAB-99", + "title": "Owner Title", + "short_description": "Owner Short Desc", + }) + assert r.status_code == 200 + body = r.json() + assert body["price"] == 250.0 + # The overrides are persisted: GET reflects them. + got = client.get("/api/v1/slabs/s1").json() + assert got["price"] == 250.0 + assert got["sku"] == "SLAB-99" + assert got["title"] == "Owner Title" + assert got["short_description"] == "Owner Short Desc" + # server_rev was incremented (the UPDATE fired). + assert got["server_rev"] == 2 diff --git a/backend/tests/test_slabs_api.py b/backend/tests/test_slabs_api.py index f9aef2c..4567e0f 100644 --- a/backend/tests/test_slabs_api.py +++ b/backend/tests/test_slabs_api.py @@ -260,3 +260,32 @@ def test_delete_published_conflict(client, db): conn.commit() resp = client.delete(f"/api/v1/slabs/{slab_id}") assert resp.status_code == 409 + + +def test_create_rejects_unknown_fields(client, db): + # #83: POST /slabs with extra (non-ALLOWED_FIELDS) keys → 400, not a silent drop. + slab_id = str(uuid.uuid4()) + payload = {"id": slab_id, "sku": "SKU001", "length_in":10,"thickness_in":2,"sqft":20,"bdft":5,"width_min_in":1,"width_max_in":2,"width_avg_in":1.5, "bogus_field": 42} + resp = client.post("/api/v1/slabs", json=payload) + assert resp.status_code == 400 + body = resp.json() + assert body["error"]["code"] == "validation" + assert "bogus_field" in body["error"]["detail"]["fields"] + + +def test_update_stale_field_returns_live_options(client, db): + # #82: stale_field error returns the LIVE taxonomy ids, not the submitted value. + slab_id = str(uuid.uuid4()) + payload = {"id": slab_id, "sku": "SKU001", "length_in":10,"thickness_in":2,"sqft":20,"bdft":5,"width_min_in":1,"width_max_in":2,"width_avg_in":1.5,"species_id":1,"wood_category_ids":json.dumps([2]),"edge_type_term_id":1,"figure_term_ids":json.dumps([1]),"grade_term_id":1,"thickness_term_id":1,"moisture_term_id":1,"fig_tag_ids":json.dumps([1]),"feat_tag_ids":json.dumps([2]),"price":100,"price_source":"recommendation","title":"Test","short_description":"short","description":"desc","content_source":"manual"} + client.post("/api/v1/slabs", json=payload) + # Submit a wood_category_id that doesn't exist in the taxonomy (id=999). + resp = client.put(f"/api/v1/slabs/{slab_id}", json={"client_rev": 1, "wood_category_ids": json.dumps([999])}) + assert resp.status_code == 422 + body = resp.json() + assert body["error"]["code"] == "stale_field" + detail = body["error"]["detail"] + assert detail["submitted"] == [999] + # current_options must be the LIVE ids from woo_categories, not [999]. + assert 999 not in detail["current_options"] + # The seeded taxonomy has ids 1 and 2 in woo_categories. + assert set(detail["current_options"]) == {1, 2}