From f6eb9e352d04e26190edc305b308c7ac48616e91 Mon Sep 17 00:00:00 2001 From: Ty Date: Mon, 21 Sep 2026 12:49:53 -0700 Subject: [PATCH] fix(m3): Option A must-fix #79 #82 #83 #84 #89 #98 - #79 publish SKU/create via WooClient (User-Agent + typed 403 unrecoverable) - #82 stale_field current_options from live taxonomy + owner labels - #83 create allowlist/extra-key safe; taxonomy+enum 4xx on create - #84 health woo_reachable real probe (non-failing); version 0.5.0-m3 - #89 review persists assembled title/short after taxonomy lock; UI applies - #98 live-edge from synced term; attrs resolved by slug then name --- backend/app.py | 26 +++++- backend/publish.py | 71 ++++++++------- backend/review.py | 96 +++++++++++++++++---- backend/slabs.py | 105 +++++++++++++++++++---- backend/tests/test_publish.py | 60 +++++++++++++ backend/tests/test_slabs_api.py | 44 ++++++++++ backend/woo.py | 51 ++++++++--- frontend/src/create/review/Review.svelte | 49 +++++++---- 8 files changed, 408 insertions(+), 94 deletions(-) diff --git a/backend/app.py b/backend/app.py index 3ae2532..cb35196 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" # --- Structured JSON logging (stdlib only) --- request_id_var: ContextVar[str] = ContextVar("request_id", default="") @@ -155,7 +155,10 @@ 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). + + woo_reachable is a *signal* only — a store outage does not fail health (#84). + """ db_ok = probe_writable() img_ok = probe_images_writable() if not (db_ok and img_ok): @@ -164,10 +167,27 @@ async def health() -> dict[str, Any]: return { "status": "ok", "version": __version__, - "woo_reachable": False, # S0 stub; S4 will implement real check + "woo_reachable": _probe_woo_reachable(), } +def _probe_woo_reachable() -> bool: + """Cheap non-failing Woo reachability probe for health (SETTINGS-40 style). + + Storage health never depends on this result. Missing creds → False. + """ + try: + base, username, enc_password = _woo_credentials() + if not base or not username or not enc_password: + return False + client = WooClient.from_settings(base, username, enc_password) + # One lightweight authenticated call (categories page 1). + code, _h, _b = client._get("/products/categories?per_page=1&page=1") + return code == 200 + except Exception: + return False + + @app.get("/api/v1/meta") async def meta() -> dict[str, Any]: """Per API.md §3.""" diff --git a/backend/publish.py b/backend/publish.py index 6232f49..8a2be6a 100644 --- a/backend/publish.py +++ b/backend/publish.py @@ -16,7 +16,14 @@ from fastapi import APIRouter from fastapi.responses import JSONResponse from .db import get_connection -from .woo import WooAuthError, WooClient, WooConnectionError, WooCredentialsMissing, WooError +from .woo import ( + WooAuthError, + WooClient, + WooConnectionError, + WooCredentialsMissing, + WooError, + WooForbiddenError, +) from .review import compute_ready, _slab_dict router = APIRouter(prefix="/api/v1/slabs") @@ -72,21 +79,24 @@ def _build_payload(slab: dict, conn, status: str) -> dict: tags = [t for t in (slab.get("fig_tag_ids") or [])]+[t for t in (slab.get("feat_tag_ids") or [])] if tags: payload["tags"] = [{"id": t} for t in tags] - # Five locked attributes by synced term name. + # Five locked attributes: resolve the attribute row from the synced cache by + # slug first (METADATA-5 / QUALITY-105 — never hardcode live Woo attribute ids), + # then fall back to case-insensitive name. Term *options* are always the + # names of the selected synced terms. terms = {r["id"]: dict(r) for r in conn.execute("SELECT * FROM woo_attribute_terms").fetchall()} attrs = {r["id"]: dict(r) for r in conn.execute("SELECT * FROM woo_attributes").fetchall()} built_attrs = [] - for field, attr_name, pos in ( - ("edge_type_term_id", "Edge Type", 0), - ("figure_term_ids", "Figure", 1), - ("grade_term_id", "Grade", 2), - ("thickness_term_id", "Thickness", 3), - ("moisture_term_id", "Moisture", 4), + for field, attr_slug, attr_name, pos in ( + ("edge_type_term_id", "edge-type", "Edge Type", 0), + ("figure_term_ids", "pa_figure", "Figure", 1), + ("grade_term_id", "pa_grade", "Grade", 2), + ("thickness_term_id", "pa_thickness", "Thickness", 3), + ("moisture_term_id", "pa_moisture", "Moisture", 4), ): raw = slab.get(field) term_ids = raw if isinstance(raw, list) else ([raw] if raw else []) options = [terms[t]["name"] for t in term_ids if t in terms] - attr_id = _attr_id_by_name(attrs, attr_name) + attr_id = _attr_id_from_synced(attrs, attr_slug, attr_name) if attr_id and options: built_attrs.append({"id": attr_id, "position": pos, "visible": True, "variation": False, "options": options}) if built_attrs: @@ -94,19 +104,35 @@ def _build_payload(slab: dict, conn, status: str) -> dict: return payload -def _attr_id_by_name(attrs: dict, name: str) -> int | None: +def _attr_id_from_synced(attrs: dict, slug: str, name: str) -> int | None: + """Match a locked attribute against the synced Woo set by slug, then name.""" + slug_l = slug.lower().lstrip() + # Accept both "figure" and "pa_figure" style slugs from Woo. + slug_alts = {slug_l, slug_l.removeprefix("pa_"), f"pa_{slug_l.removeprefix('pa_')}"} for a in attrs.values(): - if a.get("name", "").lower() == name.lower(): + s = (a.get("slug") or "").lower() + if s in slug_alts: + return a["id"] + name_l = name.lower() + for a in attrs.values(): + if (a.get("name") or "").lower() == name_l: return a["id"] return None def _classify(e: Exception) -> tuple[str, str, bool]: - """Return (code, plain message, recoverable).""" + """Return (code, plain message, recoverable). PUBLISH-36/37/38.""" if isinstance(e, WooCredentialsMissing): return "woo_unconfigured", "Woo is not configured. Set the store credentials in Settings.", False if isinstance(e, WooAuthError): return "woo_auth", "The store rejected the credentials. Check the app password.", False + if isinstance(e, WooForbiddenError): + # WAF / security plugin block — retrying the same request will not help. + return ( + "woo_forbidden", + "The store blocked this request (security filter). Check the store firewall or User-Agent allowlist — retrying will not help.", + False, + ) if isinstance(e, WooConnectionError): return "woo_unreachable", "The store is unreachable right now. You can retry.", True return "woo_failed", "The store create failed. You can retry.", True @@ -202,26 +228,13 @@ def _create_status(conn) -> str: def _sku_exists(client: WooClient, sku: str) -> bool: - code, _h, body = client._request( - f"{client.base_url}/wp-json/wc/v3/products?sku={sku}", - "GET", None, - {"Authorization": f"Basic {client._auth}", "Accept": "application/json"}, - ) - if code != 200: - raise WooError(f"sku lookup returned {code}") - return len(json.loads(body)) > 0 + """SKU lookup via WooClient so User-Agent and 403 typing apply (#79).""" + return len(client.get_products_by_sku(sku)) > 0 def _create_product(client: WooClient, payload: dict) -> int: - body = json.dumps(payload).encode("utf-8") - code, _h, resp = client._request( - f"{client.base_url}/wp-json/wc/v3/products", - "POST", body, - {"Authorization": f"Basic {client._auth}", "Content-Type": "application/json", "Accept": "application/json"}, - ) - if code not in (200, 201): - raise WooError(f"product create returned {code}") - return int(json.loads(resp)["id"]) + """Product create via WooClient so User-Agent and 403 typing apply (#79).""" + return client.create_product(payload) def _log(conn, slab_id: str, woo_product_id: int | None, status: str, message: str) -> None: diff --git a/backend/review.py b/backend/review.py index 02da487..dcee400 100644 --- a/backend/review.py +++ b/backend/review.py @@ -5,6 +5,7 @@ METADATA-80/85/90, SETTINGS-35. import json import re +from datetime import datetime, timezone from typing import Any from fastapi import APIRouter @@ -66,14 +67,16 @@ def _dims(length_in: float, width_avg_in: float, thickness_in: float) -> str: return f"{_fmt1(length_in)} x {_fmt1(width_avg_in)} x {_fmt2(thickness_in)} in" -def build_title(slab: dict, figure: str | None, species_name: str, primary_leaf: dict) -> str: +def build_title(slab: dict, figure: str | None, species_name: str, primary_leaf: dict, terms: dict | None = None) -> str: pattern, noun, carries_dims = _LEAF_PATTERNS.get( primary_leaf.get("slug"), ("{figure}{species} Slab", "Slab", False) ) figure = figure.strip() if figure else "" live_edge = "" - if primary_leaf["slug"] in ("dimensional-wood", "bowl-blanks"): - live_edge = "Live Edge " if slab.get("edge_type_term_id") else "" + if primary_leaf.get("slug") in ("dimensional-wood", "bowl-blanks"): + # "Live Edge" is a product-form flag driven by the *synced* edge term + # (METADATA-5 / #98) — never treat any non-null edge id as Live Edge. + live_edge = "Live Edge " if _edge_is_live(slab.get("edge_type_term_id"), terms or {}) else "" title = pattern.format( figure=figure + " " if figure else "", species=species_name, @@ -84,8 +87,17 @@ def build_title(slab: dict, figure: str | None, species_name: str, primary_leaf: return re.sub(r"\s+", " ", title).strip() -def build_short_description(slab: dict, figure: str | None, species_name: str, primary_leaf: dict) -> str: - title = build_title(slab, figure, species_name, primary_leaf) +def _edge_is_live(edge_term_id: Any, terms: dict) -> bool: + """True when the synced edge term name/slug indicates live edge.""" + if not edge_term_id: + return False + term = terms.get(edge_term_id) or {} + blob = f"{term.get('name', '')} {term.get('slug', '')}".lower() + return "live" in blob + + +def build_short_description(slab: dict, figure: str | None, species_name: str, primary_leaf: dict, terms: dict | None = None) -> str: + title = build_title(slab, figure, species_name, primary_leaf, terms) ft, rem = _split_length(slab["length_in"]) length_str = f"{ft}ft {_fmt1(rem)}in" bdft = round(slab["bdft"], 1) @@ -142,6 +154,13 @@ def price_suggestion(conn, slab: dict) -> float | None: @router.post("/{slab_id}/review") async def review_slab(slab_id: str, payload: dict[str, Any] | None = None) -> dict: + """Compute ready + deterministic title/short_description from *locked* taxonomy. + + METADATA-80 / REVIEW-10 / #89: title and short_description are assembled only + from the confirmed (persisted) listing facts. Owner-supplied title/short_desc + win (METADATA-50). Assembled values are written back to the slab so the lock + sticks for publish. + """ payload = payload or {} conn = get_connection() row = conn.execute( @@ -151,7 +170,16 @@ async def review_slab(slab_id: str, payload: dict[str, Any] | None = None) -> di return JSONResponse(status_code=404, content=_error("not_found", "Slab not found")) slab = _slab_dict(row) # Apply any review edits (primary category declaration + prose). - editable = {"primary_wood_category_id", "title", "short_description", "description", "price", "price_source", "sku", "content_source"} + editable = { + "primary_wood_category_id", + "title", + "short_description", + "description", + "price", + "price_source", + "sku", + "content_source", + } for k in editable: if k in payload and payload[k] is not None: if k == "primary_wood_category_id": @@ -163,18 +191,30 @@ async def review_slab(slab_id: str, payload: dict[str, Any] | None = None) -> di wood_cats = slab.get("wood_category_ids") or [] if not primary_id: primary_id = wood_cats[0] if wood_cats else None - primary_leaf = cats.get(primary_id, {}) - species_name = cats.get(slab.get("species_id"), {}).get("name", "") + primary_leaf = cats.get(primary_id, {}) if primary_id else {} + species_name = cats.get(slab.get("species_id"), {}).get("name", "") if slab.get("species_id") else "" figure_term_ids = slab.get("figure_term_ids") or [] figure = terms.get(figure_term_ids[0], {}).get("name") if figure_term_ids else None - # Deterministic copy (only if not already overridden by owner). - if not payload.get("title"): - if primary_leaf and species_name: - slab["title"] = build_title(slab, figure, species_name, primary_leaf) - if not payload.get("short_description"): - if primary_leaf and species_name: - slab["short_description"] = build_short_description(slab, figure, species_name, primary_leaf) + # Taxonomy lock gate (#89): only assemble title/short_desc when the owner has + # confirmed the facts that patterns need (species + primary wood category). + taxonomy_locked = bool(primary_leaf and species_name and primary_leaf.get("slug")) + + # Owner override: non-empty title/short_description in the payload wins. + # Empty/absent means "assemble from locked taxonomy". + raw_title = payload.get("title") + raw_short = payload.get("short_description") + want_title = not (isinstance(raw_title, str) and raw_title.strip()) + want_short = not (isinstance(raw_short, str) and raw_short.strip()) + + if taxonomy_locked: + if want_title: + slab["title"] = build_title(slab, figure, species_name, primary_leaf, terms) + if want_short: + slab["short_description"] = build_short_description( + slab, figure, species_name, primary_leaf, terms + ) + # Price suggestion (REVIEW-25): bdft * species rate, empty if no rate; owner override sticks. suggested = price_suggestion(conn, slab) if payload.get("price") is not None: @@ -183,7 +223,32 @@ async def review_slab(slab_id: str, payload: dict[str, Any] | None = None) -> di price = slab["price"] else: price = suggested + if price is not None: + slab["price"] = price + if not slab.get("price_source"): + slab["price_source"] = "recommendation" + + # Persist lock: write assembled title/short_desc/price onto the slab so publish + # and subsequent loads see the confirmed set (#89). + now = datetime.now(timezone.utc).replace(microsecond=0, tzinfo=None).isoformat() + conn.execute( + "UPDATE slabs SET title=?, short_description=?, price=?, price_source=COALESCE(price_source, ?), " + "updated_at=? WHERE id=?", + ( + slab.get("title"), + slab.get("short_description"), + price, + "recommendation", + now, + slab_id, + ), + ) + # Server-derived ready status on the row when the full set is present. ready, missing = compute_ready({**slab, "price": price}) + if ready: + conn.execute("UPDATE slabs SET status='ready', updated_at=? WHERE id=? AND status NOT IN ('published','failed')", (now, slab_id)) + conn.commit() + return { "ready": ready, "missing": missing, @@ -192,4 +257,5 @@ async def review_slab(slab_id: str, payload: dict[str, Any] | None = None) -> di "price": price, "price_suggestion": suggested, "primary_wood_category_id": primary_id, + "taxonomy_locked": taxonomy_locked, } diff --git a/backend/slabs.py b/backend/slabs.py index 4669a64..2b41823 100644 --- a/backend/slabs.py +++ b/backend/slabs.py @@ -42,6 +42,20 @@ TAXONOMY_TABLES = { "fig_tag_ids": "woo_tags", "feat_tag_ids": "woo_tags", } +# Owner-facing labels for stale_field messages (QUALITY-120 — never raw column names). +FIELD_LABELS = { + "species_id": "Species", + "wood_category_ids": "Wood category", + "edge_type_term_id": "Edge type", + "figure_term_ids": "Figure", + "grade_term_id": "Grade", + "thickness_term_id": "Thickness", + "moisture_term_id": "Moisture", + "fig_tag_ids": "Figure tags", + "feat_tag_ids": "Feature tags", +} +PRICE_SOURCES = {"recommendation", "override", "manual"} +INFERENCE_STATUSES = {"idle", "inferring", "done", "failed", "skipped"} def _now_iso() -> str: @@ -95,6 +109,58 @@ def _check_taxonomy(conn, table: str, val: Any) -> bool: return conn.execute(f"SELECT 1 FROM {table} WHERE id=?", (val,)).fetchone() is not None +def _current_options(conn, table: str) -> list[dict[str, Any]]: + """Live taxonomy options for a stale_field response (PUBLISH-20 / #82).""" + rows = conn.execute(f"SELECT id, name FROM {table} ORDER BY name, id").fetchall() + return [{"id": r["id"], "name": r["name"]} for r in rows] + + +def _normalize_tax_val(val: Any) -> Any: + if isinstance(val, str): + try: + return json.loads(val) + except (ValueError, TypeError): + return [] + return val + + +def _validate_taxonomy_fields(conn, payload: Dict[str, Any]) -> JSONResponse | None: + """Return a 422 stale_field response, or None if all present taxonomy ids are live.""" + for field, table in TAXONOMY_TABLES.items(): + if field not in payload or payload[field] is None: + continue + val = _normalize_tax_val(payload[field]) + if not _check_taxonomy(conn, table, val): + label = FIELD_LABELS.get(field, field) + return _err( + "stale_field", + f"{label} is no longer in the store taxonomy", + {"field": field, "current_options": _current_options(conn, table)}, + status=422, + ) + return None + + +def _validate_enums(payload: Dict[str, Any]) -> JSONResponse | None: + if "price_source" in payload and payload["price_source"] is not None: + if payload["price_source"] not in PRICE_SOURCES: + return _err( + "validation", + "price_source is not a known value", + {"field": "price_source", "allowed": sorted(PRICE_SOURCES)}, + status=400, + ) + if "inference_status" in payload and payload["inference_status"] is not None: + if payload["inference_status"] not in INFERENCE_STATUSES: + return _err( + "validation", + "inference_status is not a known value", + {"field": "inference_status", "allowed": sorted(INFERENCE_STATUSES)}, + status=400, + ) + return None + + # Capture-only fields required to create a slab. The Review screen fills the # taxonomy/content fields after create (CREATE.md §1: "every other field stays # editable until create"), so create must accept a capture-math payload. @@ -110,14 +176,22 @@ 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) + # Drop unknown keys (QUALITY-120 / #83) — never let client keys expand the INSERT. + payload = {k: v for k, v in payload.items() if k in ALLOWED_FIELDS} # 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. - sku = payload["sku"] + sku = payload.get("sku") if isinstance(sku, str) and sku.strip() == "": payload = {**payload, "sku": None} + enum_err = _validate_enums(payload) + if enum_err is not None: + return enum_err conn = get_connection() - if payload["sku"] is not None and _sku_taken(conn, payload["sku"]): + tax_err = _validate_taxonomy_fields(conn, payload) + if tax_err is not None: + return tax_err + if payload.get("sku") is not None and _sku_taken(conn, payload["sku"]): return _err("duplicate_sku", "SKU already exists", {"sku": payload["sku"]}, status=409) now = _now_iso() data = _encode(payload) @@ -129,8 +203,12 @@ async def create_slab(payload: Dict[str, Any]) -> Dict[str, Any]: 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() + try: + conn.execute(f"INSERT INTO slabs ({', '.join(cols)}) VALUES ({ph})", vals) + conn.commit() + except Exception: + # QUALITY-120: never surface a driver stack to the owner. + return _err("validation", "Could not create the slab. Check the fields and try again.", status=400) row = conn.execute("SELECT * FROM slabs WHERE id=?", (payload["id"],)).fetchone() return _row_to_slab(row) @@ -155,6 +233,9 @@ async def get_slab(id: str) -> Dict[str, Any]: async def update_slab(id: str, payload: Dict[str, Any]) -> Dict[str, Any]: if "client_rev" not in payload: return _err("validation", "client_rev required", status=400) + enum_err = _validate_enums(payload) + if enum_err is not None: + return enum_err conn = get_connection() current = conn.execute("SELECT * FROM slabs WHERE id=? AND deleted_at IS NULL", (id,)).fetchone() if current is None: @@ -164,19 +245,11 @@ async def update_slab(id: str, payload: Dict[str, Any]) -> Dict[str, Any]: return _err("revision_conflict", "client_rev mismatch", {"current_server_rev": current["server_rev"]}, status=409) if "sku" in payload and payload["sku"] != current["sku"]: - if _sku_taken(conn, payload["sku"], exclude_id=id): + if payload["sku"] is not None and _sku_taken(conn, payload["sku"], exclude_id=id): return _err("duplicate_sku", "SKU already exists", {"sku": payload["sku"]}, status=409) - for field, table in TAXONOMY_TABLES.items(): - if field in payload and payload[field] is not None: - val = payload[field] - if isinstance(val, str): - try: - val = json.loads(val) - except (ValueError, TypeError): - val = [] - if not _check_taxonomy(conn, table, val): - return _err("stale_field", f"{field} not in current taxonomy", - {"current_options": val}, status=422) + tax_err = _validate_taxonomy_fields(conn, payload) + if tax_err is not None: + return tax_err data = _encode(payload) cols = [f for f in ALLOWED_UPDATE_FIELDS if f in data] if not cols: diff --git a/backend/tests/test_publish.py b/backend/tests/test_publish.py index c9b7459..cf0b08c 100644 --- a/backend/tests/test_publish.py +++ b/backend/tests/test_publish.py @@ -182,3 +182,63 @@ def test_partial_media_failure_502(env, monkeypatch): # sync_log written, status failed assert conn.execute("SELECT status FROM slabs WHERE id='s1'").fetchone()["status"] == "failed" assert conn.execute("SELECT COUNT(*) c FROM sync_log WHERE status='failed'").fetchone()["c"] >= 1 + + +def test_publish_sends_user_agent_on_sku_and_create(env, monkeypatch): + """#79: every publish Woo hop carries User-Agent (same transport as client).""" + client, conn = env + s = _full_slab() + _insert_slab(conn, s) + conn.commit() + seen = [] + + def fake_request(url, method, data, headers): + seen.append({"url": url, "method": method, "headers": dict(headers or {})}) + if "/products?sku=" in url: + return 200, {}, b"[]" + if url.endswith("/wp-json/wc/v3/products") and method == "POST": + return 201, {}, json.dumps({"id": 42}).encode() + return 200, {}, b"[]" + + from backend.woo import WooClient, _USER_AGENT + + orig_init = WooClient.__init__ + + def patched_init(self, *a, **k): + orig_init(self, *a, **k) + self._request = fake_request + + monkeypatch.setattr(WooClient, "__init__", patched_init) + r = client.post("/api/v1/slabs/s1/publish") + assert r.status_code == 200, r.text + sku_hops = [h for h in seen if "/products?sku=" in h["url"]] + create_hops = [h for h in seen if h["url"].endswith("/wp-json/wc/v3/products") and h["method"] == "POST"] + assert sku_hops and create_hops + for hop in sku_hops + create_hops: + assert hop["headers"].get("User-Agent") == _USER_AGENT + + +def test_publish_403_is_unrecoverable(env, monkeypatch): + """#79: WAF 403 on SKU lookup is unrecoverable — do not invite retry.""" + client, conn = env + s = _full_slab() + _insert_slab(conn, s) + conn.commit() + from backend.woo import WooClient, WooForbiddenError + + def fake_request(url, method, data, headers): + raise WooForbiddenError("request forbidden (403)") + + orig_init = WooClient.__init__ + + def patched_init(self, *a, **k): + orig_init(self, *a, **k) + self._request = fake_request + + monkeypatch.setattr(WooClient, "__init__", patched_init) + r = client.post("/api/v1/slabs/s1/publish") + assert r.status_code == 502 + body = r.json() + assert body["error"]["code"] == "woo_forbidden" + assert body["error"]["detail"]["recoverable"] is False + assert "retry" not in body["error"]["message"].lower() or "will not help" in body["error"]["message"].lower() diff --git a/backend/tests/test_slabs_api.py b/backend/tests/test_slabs_api.py index f9aef2c..2feb65e 100644 --- a/backend/tests/test_slabs_api.py +++ b/backend/tests/test_slabs_api.py @@ -260,3 +260,47 @@ 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_stale_field_returns_live_options_not_submitted_value(client, db): + """#82 / PUBLISH-20: current_options is the live taxonomy list, not the bad id.""" + slab_id = str(uuid.uuid4()) + payload = { + "id": slab_id, "sku": "SKU-STALE", "length_in": 10, "thickness_in": 2, "sqft": 20, "bdft": 5, + "width_min_in": 1, "width_max_in": 2, "width_avg_in": 1.5, + } + assert client.post("/api/v1/slabs", json=payload).status_code == 201 + resp = client.put(f"/api/v1/slabs/{slab_id}", json={"client_rev": 1, "edge_type_term_id": 999}) + assert resp.status_code == 422 + body = resp.json() + assert body["error"]["code"] == "stale_field" + opts = body["error"]["detail"]["current_options"] + assert isinstance(opts, list) and opts + assert all("id" in o and "name" in o for o in opts) + assert 999 not in [o["id"] for o in opts] + assert "Edge type" in body["error"]["message"] or "edge" in body["error"]["message"].lower() + + +def test_create_extra_key_and_bad_taxonomy_are_4xx(client, db): + """#83: extra keys and bad taxonomy ids must not 500.""" + slab_id = str(uuid.uuid4()) + base = { + "id": slab_id, "sku": "SKU-XTRA", "length_in": 10, "thickness_in": 2, "sqft": 20, "bdft": 5, + "width_min_in": 1, "width_max_in": 2, "width_avg_in": 1.5, + "status": "published", # server-managed; must be dropped, not bind-break + } + r = client.post("/api/v1/slabs", json=base) + assert r.status_code == 201, r.text + assert r.json()["status"] == "calibrated" + # Bad taxonomy on create + r2 = client.post("/api/v1/slabs", json={ + **base, "id": str(uuid.uuid4()), "sku": "SKU-BADTAX", "species_id": 999, + }) + assert r2.status_code == 422 + assert r2.json()["error"]["code"] == "stale_field" + # Bad enum + r3 = client.post("/api/v1/slabs", json={ + **base, "id": str(uuid.uuid4()), "sku": "SKU-BADENUM", "price_source": "not-a-source", + }) + assert r3.status_code == 400 + assert r3.json()["error"]["code"] == "validation" diff --git a/backend/woo.py b/backend/woo.py index d1eb557..921cc53 100644 --- a/backend/woo.py +++ b/backend/woo.py @@ -98,14 +98,7 @@ class WooClient: def _get(self, path: str) -> tuple[int, dict, bytes]: url = f"{self.base_url}/wp-json/wc/v3{path}" - return self._request( - url, "GET", None, - { - "Authorization": f"Basic {self._auth}", - "Accept": "application/json", - "User-Agent": _USER_AGENT, - }, - ) + return self._request(url, "GET", None, self._auth_headers()) def _fetch_all(self, path: str) -> list[dict[str, Any]]: page = 1 @@ -132,6 +125,42 @@ class WooClient: def get_attribute_terms(self, attr_id: int) -> list[dict[str, Any]]: return self._fetch_all(f"/products/attributes/{attr_id}/terms") + def _auth_headers(self, extra: dict[str, str] | None = None) -> dict[str, str]: + """Shared headers for every outbound Woo/WP request (User-Agent + auth).""" + h = { + "Authorization": f"Basic {self._auth}", + "Accept": "application/json", + "User-Agent": _USER_AGENT, + } + if extra: + h.update(extra) + return h + + def get_products_by_sku(self, sku: str) -> list[dict[str, Any]]: + """GET products filtered by SKU. Raises typed Woo* errors on non-200.""" + code, _h, body = self._request( + f"{self.base_url}/wp-json/wc/v3/products?sku={sku}", + "GET", + None, + self._auth_headers(), + ) + if code != 200: + raise WooServerError(f"sku lookup returned {code}") + return list(json.loads(body)) + + def create_product(self, payload: dict[str, Any]) -> int: + """POST /products. Raises typed Woo* errors on failure; returns product id.""" + body = json.dumps(payload).encode("utf-8") + code, _h, resp = self._request( + f"{self.base_url}/wp-json/wc/v3/products", + "POST", + body, + self._auth_headers({"Content-Type": "application/json"}), + ) + if code not in (200, 201): + raise WooServerError(f"product create returned {code}") + return int(json.loads(resp)["id"]) + def upload_media(self, png_bytes: bytes, filename: str) -> int: """POST a PNG to WordPress core media; return the attachment id.""" boundary = "----slabuploader-media-boundary" @@ -143,11 +172,7 @@ class WooClient: url = f"{self.base_url}/wp-json/wp/v2/media" code, _headers, resp = self._request( url, "POST", body, - { - "Authorization": f"Basic {self._auth}", - "Content-Type": f"multipart/form-data; boundary={boundary}", - "User-Agent": _USER_AGENT, - }, + self._auth_headers({"Content-Type": f"multipart/form-data; boundary={boundary}"}), ) if code not in (200, 201): raise WooServerError(f"media upload returned {code}") diff --git a/frontend/src/create/review/Review.svelte b/frontend/src/create/review/Review.svelte index fd46375..7033496 100644 --- a/frontend/src/create/review/Review.svelte +++ b/frontend/src/create/review/Review.svelte @@ -186,9 +186,11 @@ const aiIn = (ids: number[]) => (id: number) => ids.includes(id); - // Persist taxonomy/price/sku edits to the slab (PUT), then recompute ready. - async function commitReview(): Promise { + // Persist taxonomy/price/sku edits to the slab (PUT), then lock + assemble + // title/short_description from the confirmed set (POST /review) — #89. + async function commitReview(opts: { lockTaxonomy?: boolean } = {}): Promise { if (!slab) return; + const lockTaxonomy = opts.lockTaxonomy !== false; const myToken = ++commitToken; reviewBusy = true; reviewError = ''; @@ -203,8 +205,10 @@ moisture_term_id: moistureId === '' ? null : Number(moistureId), fig_tag_ids: figTagIds, feat_tag_ids: featTagIds, - title: title || null, - short_description: shortDescription || null, + // When locking taxonomy for title assembly, omit empty title/short so + // the server builds them from the confirmed facts (METADATA-80). + title: title.trim() ? title : null, + short_description: shortDescription.trim() ? shortDescription : null, description: description || null, content_source: contentSource || null, price: price === '' ? null : Number(price), @@ -215,20 +219,29 @@ const fresh = await updateSlab(slabId, patch); if (myToken !== commitToken) return; // superseded; don't clobber the latest slab = fresh; - if (primaryCatId !== '') { - const res = await reviewSlab(slabId, { - primary_wood_category_id: Number(primaryCatId), - title: title || undefined, - short_description: shortDescription || undefined, - price: price === '' ? undefined : Number(price), - sku: sku || undefined, - }); - if (myToken !== commitToken) return; - review = res; - if (res.primary_wood_category_id != null) primaryCatId = res.primary_wood_category_id; - } else { - review = await reviewSlab(slabId); - if (myToken !== commitToken) return; + const reviewPayload: Record = {}; + if (primaryCatId !== '') reviewPayload.primary_wood_category_id = Number(primaryCatId); + // Owner overrides only when non-empty — empty means "assemble after lock". + if (title.trim()) reviewPayload.title = title; + if (shortDescription.trim()) reviewPayload.short_description = shortDescription; + if (price !== '') reviewPayload.price = Number(price); + if (sku) reviewPayload.sku = sku; + const res = await reviewSlab(slabId, reviewPayload); + if (myToken !== commitToken) return; + review = res; + if (res.primary_wood_category_id != null) primaryCatId = res.primary_wood_category_id; + // Apply server-assembled title/short when local fields were empty (#89 lock). + if (lockTaxonomy) { + if (!title.trim() && res.title) title = res.title; + if (!shortDescription.trim() && res.short_description) shortDescription = res.short_description; + if (price === '' && res.price != null) { + price = String(res.price); + priceSource = 'recommendation'; + } + // Refresh slab so server_rev and persisted title match. + try { + slab = await getSlab(slabId); + } catch { /* keep fresh from PUT */ } } } catch (e) { if (myToken !== commitToken) return;