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/db/__init__.py b/backend/db/__init__.py index 9b8f6c3..471905e 100644 --- a/backend/db/__init__.py +++ b/backend/db/__init__.py @@ -79,6 +79,21 @@ _M3_TABLES = [ ] +def _migrate(conn: sqlite3.Connection) -> None: + """Idempotent column additions for volumes created before a column existed. + + `CREATE TABLE IF NOT EXISTS` in schema.sql is a no-op on an existing table, + so new columns must be added here. Each guard is a no-op once the column is + present. #90 adds slab_photos.role (lineup kind) for the Review photo strip. + """ + photo_cols = {r[1] for r in conn.execute("PRAGMA table_info(slab_photos)").fetchall()} + if photo_cols and "role" not in photo_cols: + conn.execute( + "ALTER TABLE slab_photos ADD COLUMN role TEXT NOT NULL DEFAULT 'batch' " + "CHECK(role IN ('calibration','overlay','batch'))" + ) + + def init_db(db_path: str | None = None) -> sqlite3.Connection: conn = get_connection(db_path) schema_path = _get_schema_path() @@ -98,6 +113,10 @@ def init_db(db_path: str | None = None) -> sqlite3.Connection: conn.executescript(schema) conn.execute("INSERT OR REPLACE INTO schema_version (version) VALUES (1)") conn.commit() + # Safe to run every boot: no-op once the columns exist. + with _lock: + _migrate(conn) + conn.commit() return conn diff --git a/backend/db/schema.sql b/backend/db/schema.sql index 7a63c9e..0aa229d 100644 --- a/backend/db/schema.sql +++ b/backend/db/schema.sql @@ -54,6 +54,9 @@ CREATE TABLE IF NOT EXISTS slab_photos ( slab_id TEXT NOT NULL, seq INTEGER NOT NULL, is_calibration INTEGER NOT NULL DEFAULT 0 CHECK(is_calibration IN (0,1)), + -- #90: lineup kind (clean cal / dimension-overlay copy / batch sibling). + -- Drives the Review photo strip labels and the PUBLISH-45 Woo image order. + role TEXT NOT NULL DEFAULT 'batch' CHECK(role IN ('calibration','overlay','batch')), original_blob BLOB, processed_blob BLOB, width_px INTEGER, diff --git a/backend/photos.py b/backend/photos.py index 34e8e5a..708fdd9 100644 --- a/backend/photos.py +++ b/backend/photos.py @@ -26,7 +26,7 @@ def _get_slab(conn, slab_id: str): def _get_photo(conn, slab_id: str, photo_id: str): row = conn.execute( - "SELECT id, original_blob, processed_blob, width_px, height_px, is_calibration, seq " + "SELECT id, original_blob, processed_blob, width_px, height_px, is_calibration, seq, role " "FROM slab_photos WHERE id=? AND slab_id=?", (photo_id, slab_id), ).fetchone() @@ -44,6 +44,7 @@ async def upload_photo( photo_id: str = Form(...), seq: int | None = Form(None), is_calibration: int = Form(0), + role: str = Form("batch"), original: UploadFile = File(...), processed: UploadFile | None = File(None), ) -> dict: @@ -52,6 +53,8 @@ async def upload_photo( return JSONResponse(status_code=404, content=_error("not_found", "Slab not found")) if is_calibration not in (0, 1): return JSONResponse(status_code=400, content=_error("validation", "is_calibration must be 0 or 1")) + if role not in ("calibration", "overlay", "batch"): + return JSONResponse(status_code=400, content=_error("validation", "role must be calibration, overlay, or batch")) orig_bytes = await original.read() proc_bytes = await processed.read() if processed is not None else None if not orig_bytes: @@ -60,14 +63,14 @@ async def upload_photo( conn.execute("DELETE FROM slab_photos WHERE id=? AND slab_id=?", (photo_id, slab_id)) seq_val = seq if seq is not None else _next_seq(conn, slab_id) conn.execute( - "INSERT INTO slab_photos(id, slab_id, seq, is_calibration, original_blob, processed_blob, created_at) " - "VALUES (?,?,?,?,?,?,?)", - (photo_id, slab_id, seq_val, is_calibration, orig_bytes, proc_bytes, _now()), + "INSERT INTO slab_photos(id, slab_id, seq, is_calibration, role, original_blob, processed_blob, created_at) " + "VALUES (?,?,?,?,?,?,?,?)", + (photo_id, slab_id, seq_val, is_calibration, role, orig_bytes, proc_bytes, _now()), ) conn.commit() row = _get_photo(conn, slab_id, photo_id) return {"id": row["id"], "slab_id": slab_id, "seq": row["seq"], "is_calibration": row["is_calibration"], - "original_retained": True, "processed_retained": row["processed_blob"] is not None} + "role": row["role"], "original_retained": True, "processed_retained": row["processed_blob"] is not None} def _now() -> str: @@ -108,9 +111,25 @@ async def list_photos(slab_id: str) -> list[dict]: if _get_slab(conn, slab_id) is None: return JSONResponse(status_code=404, content=_error("not_found", "Slab not found")) rows = conn.execute( - "SELECT id, seq, is_calibration, width_px, height_px, " + "SELECT id, seq, is_calibration, role, width_px, height_px, " "(original_blob IS NOT NULL) AS has_original, (processed_blob IS NOT NULL) AS has_processed " "FROM slab_photos WHERE slab_id=? ORDER BY seq", (slab_id,), ).fetchall() return [dict(r) for r in rows] + + +@router.delete("/{slab_id}/photos/{photo_id}", status_code=204) +async def delete_photo(slab_id: str, photo_id: str): + """#90: drop a photo the owner removed in Review (keep/remove edit). The + remaining photos are re-numbered by the client via re-upload.""" + conn = get_connection() + if _get_slab(conn, slab_id) is None: + return JSONResponse(status_code=404, content=_error("not_found", "Slab not found")) + cur = conn.execute( + "DELETE FROM slab_photos WHERE id=? AND slab_id=?", (photo_id, slab_id) + ) + conn.commit() + if cur.rowcount == 0: + return JSONResponse(status_code=404, content=_error("not_found", "Photo not found")) + return Response(status_code=204) diff --git a/backend/publish.py b/backend/publish.py index 6232f49..855b450 100644 --- a/backend/publish.py +++ b/backend/publish.py @@ -150,9 +150,16 @@ async def publish(slab_id: str) -> dict: code, msg, rec = _classify(e) return JSONResponse(status_code=502, content=_error(code, msg, {"recoverable": rec})) # 4/5. Upload processed PNGs, then POST product. + # PUBLISH-45: Woo image order is the photos-ready lineup — clean calibration, + # its dimension-overlay copy, then the remaining batch photos. The Review + # photo strip stores the owner's keep order (leftmost = primary), so the + # overlay (which can sit after a moved batch photo) is re-ordered here by + # role rather than by stored seq. Ties fall back to stored seq. status = "draft" if _is_uat(slab["sku"]) else _create_status(conn) photos = conn.execute( - "SELECT id, processed_blob FROM slab_photos WHERE slab_id=? AND processed_blob IS NOT NULL ORDER BY seq", + "SELECT id, processed_blob, role FROM slab_photos " + "WHERE slab_id=? AND processed_blob IS NOT NULL " + "ORDER BY CASE role WHEN 'calibration' THEN 0 WHEN 'overlay' THEN 1 ELSE 2 END, seq", (slab_id,), ).fetchall() uploaded_ids: list[int] = [] diff --git a/backend/review.py b/backend/review.py index 02da487..c3cff56 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 @@ -44,6 +45,10 @@ def _error(code: str, message: str, detail: Any = None) -> dict: return {"error": {"code": code, "message": message, "detail": detail}} +def _now() -> str: + return datetime.now(timezone.utc).replace(microsecond=0, tzinfo=None).isoformat() + + def _split_length(length_in: float) -> tuple[int, float]: ft = int(length_in // 12) rem = round(length_in - ft * 12, 1) @@ -150,15 +155,16 @@ async def review_slab(slab_id: str, payload: dict[str, Any] | None = None) -> di if row is None: 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"} - for k in editable: - if k in payload and payload[k] is not None: - if k == "primary_wood_category_id": - continue # not stored as a column; derived - slab[k] = payload[k] + + # --- Deterministic review: report ready/missing + the deterministic + # title/short_description built from the current locked metadata. The client's + # "Create title" / "Create short desc" buttons call this endpoint and write the + # returned strings into the fields (item 1: no lock action, no metadata_locked). + # #80: persist owner overrides (price/sku) from the payload so the review + # endpoint is self-contained. title/short_description are NOT overridden here — + # they are produced by the deterministic build below, not persisted from the + # payload (so the review POST can't clobber the owner's Create workflow). cats, terms = _taxonomy_lookup(conn) - # Primary wood category: declared, else first wood_category_ids, else first category. primary_id = payload.get("primary_wood_category_id") wood_cats = slab.get("wood_category_ids") or [] if not primary_id: @@ -168,28 +174,40 @@ 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 - # 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) - # 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: - price = payload["price"] - elif slab.get("price") not in (None, 0): - price = slab["price"] - else: - price = suggested - ready, missing = compute_ready({**slab, "price": price}) + # #80: persist owner overrides from the payload (price/sku only). + overrides: dict[str, Any] = {} + for k in ("price", "sku"): + 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) + + # Deterministic title / short_description, re-derived from the current + # metadata on every review (the Create button's source of truth). + title = build_title(slab, figure, species_name, primary_leaf) if (primary_leaf and species_name) else slab.get("title") or None + short_desc = build_short_description(slab, figure, species_name, primary_leaf) if (primary_leaf and species_name) else slab.get("short_description") or None + + price = _resolve_price(conn, slab, payload) + ready, missing = compute_ready({**slab, "price": price, "title": title, "short_description": short_desc}) return { - "ready": ready, - "missing": missing, - "title": slab.get("title"), - "short_description": slab.get("short_description"), - "price": price, - "price_suggestion": suggested, + "ready": ready, "missing": missing, "title": title, "short_description": short_desc, + "price": price, "price_suggestion": price_suggestion(conn, slab), "primary_wood_category_id": primary_id, } + + +def _resolve_price(conn, slab: dict, payload: dict) -> float | None: + """REVIEW-25: bdft * species rate as the suggestion; owner override sticks, + else the stored price, else the suggestion (None if no rate).""" + suggested = price_suggestion(conn, slab) + if payload.get("price") is not None: + return payload["price"] + if slab.get("price") not in (None, 0): + return slab["price"] + return suggested diff --git a/backend/slabs.py b/backend/slabs.py index 4669a64..7ea73ed 100644 --- a/backend/slabs.py +++ b/backend/slabs.py @@ -13,6 +13,7 @@ from fastapi import APIRouter from fastapi.responses import JSONResponse, Response from .db import get_connection +from .thickness_band import load_thickness_terms, resolve_thickness_band_id router = APIRouter(prefix="/api/v1/slabs") @@ -64,6 +65,12 @@ def _encode(payload: Dict[str, Any]) -> Dict[str, Any]: return out +# #89: server-managed fields that a plain client PUT must not set directly. +# (metadata_locked was dropped — the deterministic title/short-desc are built by +# the review endpoint and written back by the client's Create buttons.) +CLIENT_FORBIDDEN_FIELDS = set() + + def _row_to_slab(row) -> Dict[str, Any]: d = dict(row) for f in JSON_ARRAY_FIELDS: @@ -110,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. @@ -119,6 +133,12 @@ async def create_slab(payload: Dict[str, Any]) -> Dict[str, Any]: conn = get_connection() if payload["sku"] is not None and _sku_taken(conn, payload["sku"]): return _err("duplicate_sku", "SKU already exists", {"sku": payload["sku"]}, status=409) + # METADATA-15 / QUALITY-95: the thickness band is derived, never owner-chosen. + # Resolve it from the measured thickness against the live synced Woo bands and + # overwrite whatever the client sent (the UI has no band control). + band = resolve_thickness_band_id(payload.get("thickness_in"), load_thickness_terms(conn)) + if band is not None: + payload = {**payload, "thickness_term_id": band} now = _now_iso() data = _encode(payload) allowed_cols = [f for f in ALLOWED_FIELDS if f in data] @@ -128,7 +148,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() @@ -155,6 +174,11 @@ 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) + # #89: the metadata lock is server-managed via the explicit review lock + # action; a plain PUT may not flip it. + for field in CLIENT_FORBIDDEN_FIELDS: + if field in payload: + return _err("validation", f"{field} is set by the review lock action, not by update", status=400) conn = get_connection() current = conn.execute("SELECT * FROM slabs WHERE id=? AND deleted_at IS NULL", (id,)).fetchone() if current is None: @@ -175,8 +199,20 @@ 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) + eff_thickness = payload.get("thickness_in", current["thickness_in"]) + band = resolve_thickness_band_id(eff_thickness, load_thickness_terms(conn)) + if band is not None: + payload = {**payload, "thickness_term_id": band} data = _encode(payload) cols = [f for f in ALLOWED_UPDATE_FIELDS if f in data] if not cols: diff --git a/backend/tests/test_photos_api.py b/backend/tests/test_photos_api.py index d28dd84..8da87a7 100644 --- a/backend/tests/test_photos_api.py +++ b/backend/tests/test_photos_api.py @@ -34,13 +34,16 @@ PNG_ORIG = b"\x89PNG-orig-bytes" PNG_PROC = b"\x89PNG-proc-bytes" -def _multipart(client, slab_id, photo_id, original=PNG_ORIG, processed=PNG_PROC, is_calibration=0): +def _multipart(client, slab_id, photo_id, original=PNG_ORIG, processed=PNG_PROC, is_calibration=0, role="batch", seq=None): files = {"original": ("orig.png", original, "image/png")} if processed is not None: files["processed"] = ("proc.png", processed, "image/png") + data = {"photo_id": photo_id, "is_calibration": str(is_calibration), "role": role} + if seq is not None: + data["seq"] = str(seq) return client.post( f"/api/v1/slabs/{slab_id}/photos", - data={"photo_id": photo_id, "is_calibration": str(is_calibration)}, + data=data, files=files, ) @@ -90,9 +93,43 @@ def test_reupload_replaces(photos_env): def test_list_photos(photos_env): client, _ = photos_env - _multipart(client, "s1", "pa", is_calibration=1) - _multipart(client, "s1", "pb") + _multipart(client, "s1", "pa", is_calibration=1, role="calibration") + _multipart(client, "s1", "pb", role="overlay") r = client.get("/api/v1/slabs/s1/photos") assert r.status_code == 200 ids = [row["id"] for row in r.json()] assert "pa" in ids and "pb" in ids + + +def test_upload_stores_and_returns_role(photos_env): + client, _ = photos_env + r = _multipart(client, "s1", "r1", role="overlay") + assert r.status_code == 201 + assert r.json()["role"] == "overlay" + r = client.get("/api/v1/slabs/s1/photos") + row = {row_["id"]: row_ for row_ in r.json()}["r1"] + assert row["role"] == "overlay" + + +def test_upload_rejects_bad_role(photos_env): + client, _ = photos_env + r = _multipart(client, "s1", "r2", role="not-a-role") + assert r.status_code == 400 + assert r.json()["error"]["code"] == "validation" + + +def test_delete_photo(photos_env): + client, _ = photos_env + _multipart(client, "s1", "d1") + assert client.get("/api/v1/slabs/s1/photos/d1/processed").status_code == 200 + r = client.delete("/api/v1/slabs/s1/photos/d1") + assert r.status_code == 204 + # Gone from the list and blob no longer served. + ids = [row["id"] for row in client.get("/api/v1/slabs/s1/photos").json()] + assert "d1" not in ids + assert client.get("/api/v1/slabs/s1/photos/d1/processed").status_code == 404 + + +def test_delete_photo_not_found(photos_env): + client, _ = photos_env + assert client.delete("/api/v1/slabs/s1/photos/ghost").status_code == 404 diff --git a/backend/tests/test_publish.py b/backend/tests/test_publish.py index c9b7459..8ce6e25 100644 --- a/backend/tests/test_publish.py +++ b/backend/tests/test_publish.py @@ -45,6 +45,21 @@ def env(monkeypatch): conn.execute("INSERT INTO woo_categories(id,name,slug,parent_id,woo_id) VALUES (2,'Live edge wood','live-edge-wood',NULL,101)") conn.execute("INSERT INTO woo_attributes(id,name,slug,woo_id) VALUES (8,'Edge Type','edge-type',200)") conn.execute("INSERT INTO woo_attribute_terms(id,attribute_id,name,slug,woo_id) VALUES (1,8,'Flat','flat',300)") + # Thickness bands, names as synced (the derivation parses them) — QUALITY-95. + conn.execute("INSERT INTO woo_attributes(id,name,slug,woo_id) VALUES (9,'Thickness','pa_thk',201)") + for tid, name, slug in [ + (347, 'Under 1/2"', 'xthin'), (348, '1/2" - 3/4"', '2-3qtr'), + (349, '3/4" - 1"', '3-4qtr'), (350, '1"- 1 1/2"', '4-6qtr'), + (351, '1 1/2" - 2"', '6-8qtr'), (352, '2" - 3"', '8-12qtr'), + (353, 'Over 3"', '12qtr'), + ]: + conn.execute( + "INSERT INTO woo_attribute_terms(id,attribute_id,name,slug,woo_id) VALUES (?,9,?,?,?)", + (tid, name, slug, tid), + ) + # Moisture keeps its own attribute so the five locked attributes stay distinct. + conn.execute("INSERT INTO woo_attributes(id,name,slug,woo_id) VALUES (10,'Moisture','pa_mst',202)") + conn.execute("INSERT INTO woo_attribute_terms(id,attribute_id,name,slug,woo_id) VALUES (326,10,'Kiln dried','kd',326)") # creds conn.execute("INSERT OR REPLACE INTO settings(key,value,value_enc) VALUES ('woo_wp_username','admin',NULL)") conn.execute("INSERT OR REPLACE INTO settings(key,value,value_enc) VALUES ('woo_app_password',NULL,?)", (encrypt("secret"),)) @@ -119,6 +134,109 @@ def test_publish_success(env, monkeypatch): assert conn.execute("SELECT processed_blob FROM slab_photos WHERE id='p1'").fetchone()["processed_blob"] is None +def test_publish_sends_derived_thickness_band(env, monkeypatch): + """#90 / QUALITY-95: the band is off the Review screen but must still reach Woo. + + The owner has no thickness-band control, so nothing client-side carries it: the + server derives it from the measurement on create/update, and publish must send the + live synced term name on the product payload. This pins that chain end to end. + """ + client, conn = env + create = client.post("/api/v1/slabs", json={ + "id": "s1", "sku": "SLAB-BAND", "length_in": 114.0, "thickness_in": 1.75, + "sqft": 30.0, "bdft": 18.4, "width_min_in": 18.0, "width_max_in": 24.3, "width_avg_in": 24.3, + }) + assert create.status_code == 201, create.text + # 1.75 in bands into the live '1 1/2" - 2"' term — never sent by the client. + assert create.json()["thickness_term_id"] == 351 + + put = client.put("/api/v1/slabs/s1", json={ + "client_rev": create.json()["server_rev"], "species_id": 1, "wood_category_ids": [2], + "edge_type_term_id": 1, "figure_term_ids": [1], "grade_term_id": 1, "moisture_term_id": 326, + "price": 184.0, "price_source": "recommendation", "title": "T", "short_description": "S", + "description": "D", "content_source": "manual", + }) + assert put.status_code == 200, put.text + conn.execute("UPDATE slabs SET status='ready' WHERE id='s1'") + conn.execute("INSERT INTO slab_photos(id,slab_id,seq,is_calibration,processed_blob,created_at) VALUES ('p1','s1',1,1,X'89504E47','t')") + conn.commit() + + products = [] + + def fake_request(url, method, data, headers): + if "/products?sku=" in url: + return 200, {}, b"[]" + if url.endswith("/wp-json/wp/v2/media") and method == "POST": + return 201, {}, json.dumps({"id": 42}).encode() + if url.endswith("/wp-json/wc/v3/products") and method == "POST": + products.append(json.loads(data)) + return 201, {}, json.dumps({"id": 999}).encode() + return 200, {}, b"[]" + + from backend.woo import WooClient + + 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 + assert len(products) == 1, "expected exactly one product create" + attrs = {a["id"]: a for a in products[0].get("attributes", [])} + assert 9 in attrs, f"Thickness attribute missing from the Woo payload: {products[0].get('attributes')}" + assert attrs[9]["options"] == ['1 1/2" - 2"'] + + +def test_publish_woo_image_order_by_role(env, monkeypatch): + # PUBLISH-45: Woo order is clean cal, dimension-overlay, then batch — by + # role, not by stored seq (the Review strip stores the owner's keep order). + client, conn = env + s = _full_slab() + _insert_slab(conn, s) + # Stored seq deliberately out of lineup order: batch(1), calibration(2), + # overlay(3). The publish query must reorder to cal, overlay, batch. + conn.execute("INSERT INTO slab_photos(id,slab_id,seq,is_calibration,role,processed_blob,created_at) VALUES ('pb','s1',1,0,'batch',X'89504E47','t')") + conn.execute("INSERT INTO slab_photos(id,slab_id,seq,is_calibration,role,processed_blob,created_at) VALUES ('pc','s1',2,1,'calibration',X'89504E47','t')") + conn.execute("INSERT INTO slab_photos(id,slab_id,seq,is_calibration,role,processed_blob,created_at) VALUES ('po','s1',3,0,'overlay',X'89504E47','t')") + conn.commit() + media_calls = [] + + def fake_request(url, method, data, headers): + if "/products?sku=" in url: + return 200, {}, b"[]" + if url.endswith("/wp-json/wp/v2/media") and method == "POST": + media_calls.append(data) + return 201, {}, json.dumps({"id": 42}).encode() + if url.endswith("/wp-json/wc/v3/products") and method == "POST": + return 201, {}, json.dumps({"id": 999}).encode() + return 200, {}, b"[]" + + from backend.woo import WooClient + + 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 + assert len(media_calls) == 3 + # The photo id is embedded in the multipart filename (slab.py publish + # passes f"{sku}-{photo_id}.png"). Order of media uploads = cal, overlay, + # batch (PUBLISH-45), not stored seq. + def photo_id(body: bytes) -> str: + import re + m = re.search(rb'filename="[^"]*-(pc|pb|po)\.png"', body) + return m.group(1).decode() if m else "" + + assert [photo_id(c) for c in media_calls] == ["pc", "po", "pb"] + + def test_uat_draft(env, monkeypatch): client, conn = env s = _full_slab(sku="SLAB-UAT-1") diff --git a/backend/tests/test_review.py b/backend/tests/test_review.py index bc430e2..ba38aeb 100644 --- a/backend/tests/test_review.py +++ b/backend/tests/test_review.py @@ -55,6 +55,8 @@ def test_title_live_edge_no_dims(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() + # #89: the review always returns the deterministic title/short_description + # built from the current metadata (the Create button's source of truth). r = env[0].post("/api/v1/slabs/s1/review", json={}) assert r.status_code == 200, r.text body = r.json() @@ -134,5 +136,61 @@ def test_knife_blank_carries_dims(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",12.0,1.5,6.0,0.7,6.0,6.5,6.5,1,json.dumps([3]),json.dumps([1]),None,None,None,None,"A slab","manual","calibrated",1,1,"t","t")) conn.commit() + # #89: the deterministic build runs on every review. r = client.post("/api/v1/slabs/s1/review", json={}) assert r.json()["title"] == "Figured Black Walnut Knife Blank (12 x 6.5 x 1.5 in)" + + +def test_review_always_reports_deterministic_title(env): + """#89: the review always returns the deterministic title/short_description + built from the current metadata. The client's Create button writes these into + the fields; the server does not persist them from the review payload.""" + 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() + r = client.post("/api/v1/slabs/s1/review", json={}) + body = r.json() + assert body["title"] == "Figured Black Walnut Live Edge Slab" + assert body["short_description"].startswith("Figured Black Walnut Live Edge Slab,") + # The review does NOT persist title/short_description from the payload. + got = client.get("/api/v1/slabs/s1").json() + assert got["title"] is None + assert got["short_description"] is None + + +def test_review_no_species_reports_stored_title(env): + """With no species, there is no deterministic pattern; the review reports the + stored title (None here) and lists title as missing.""" + 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,price,title,short_description,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,None,json.dumps([2]),None,None,None,"calibrated",1,1,"t","t")) + conn.commit() + r = client.post("/api/v1/slabs/s1/review", json={}) + body = r.json() + assert body["title"] is None + assert "title" in body["missing"] + + +def test_review_persists_owner_overrides(env): + """#80: regular review persists price/sku overrides from the payload to the DB, + so the client doesn't need a separate PUT for each. (title/short_description + are NOT persisted here — they come from the deterministic build, not the payload.)""" + 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. + r = client.post("/api/v1/slabs/s1/review", json={ + "price": 250.0, + "sku": "SLAB-99", + }) + 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" + # 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..c4577e4 100644 --- a/backend/tests/test_slabs_api.py +++ b/backend/tests/test_slabs_api.py @@ -33,6 +33,23 @@ def _seed_taxonomy(conn): # tags conn.execute("INSERT OR REPLACE INTO woo_tags(id, name, slug, woo_id) VALUES (1, 'Fig Tag A', 'fig-tag-a', 50)") conn.execute("INSERT OR REPLACE INTO woo_tags(id, name, slug, woo_id) VALUES (2, 'Feat Tag A', 'feat-tag-a', 60)") + # Thickness bands (METADATA-15 / QUALITY-95) — names exactly as synced from the + # live Woo attribute, because the derivation parses them. + conn.execute("INSERT OR REPLACE INTO woo_attributes(id, name, slug, woo_id) VALUES (2, 'Thickness', 'pa_thk', 31)") + for tid, name, slug in [ + (347, 'Under 1/2"', 'xthin'), + (348, '1/2" - 3/4"', '2-3qtr'), + (349, '3/4" - 1"', '3-4qtr'), + (350, '1"- 1 1/2"', '4-6qtr'), + (351, '1 1/2" - 2"', '6-8qtr'), + (352, '2" - 3"', '8-12qtr'), + (353, 'Over 3"', '12qtr'), + ]: + conn.execute( + "INSERT OR REPLACE INTO woo_attribute_terms(id, attribute_id, name, slug, woo_id) " + "VALUES (?, 2, ?, ?, ?)", + (tid, name, slug, tid), + ) conn.commit() @pytest.fixture @@ -103,6 +120,55 @@ def test_create_and_fetch(client, db): assert body[key] == payload[key] +def test_create_derives_thickness_band(client, db): + """QUALITY-95: 1 in rounds up into the live synced band, not a hardcoded one. + + The client-supplied thickness_term_id (1, an Edge Type term) must be ignored — + the UI has no band control, so the server's derivation is authoritative. + """ + slab_id = str(uuid.uuid4()) + payload = { + "id": slab_id, "sku": "SKU-BAND", "length_in": 10, "thickness_in": 1.0, "sqft": 20, + "bdft": 5, "width_min_in": 1, "width_max_in": 2, "width_avg_in": 1.5, + "thickness_term_id": 1, + } + resp = client.post("/api/v1/slabs", json=payload) + assert resp.status_code == 201 + assert resp.json()["thickness_term_id"] == 350 # '1"- 1 1/2"', not the sent 1 + + +def test_update_rebands_when_thickness_changes(client, db): + """A thickness edit moves the slab to the band the new measurement rounds into.""" + slab_id = str(uuid.uuid4()) + create = client.post("/api/v1/slabs", json={ + "id": slab_id, "sku": "SKU-BAND2", "length_in": 10, "thickness_in": 0.5, "sqft": 20, + "bdft": 5, "width_min_in": 1, "width_max_in": 2, "width_avg_in": 1.5, + }) + assert create.status_code == 201 + assert create.json()["thickness_term_id"] == 348 # 1/2" - 3/4" + put = client.put(f"/api/v1/slabs/{slab_id}", json={ + "client_rev": create.json()["server_rev"], "thickness_in": 2.75, + }) + assert put.status_code == 200 + assert put.json()["thickness_term_id"] == 352 # 2" - 3" + + +def test_update_ignores_client_supplied_band(client, db): + """A stale client band cannot pin the slab to the wrong band.""" + slab_id = str(uuid.uuid4()) + create = client.post("/api/v1/slabs", json={ + "id": slab_id, "sku": "SKU-BAND3", "length_in": 10, "thickness_in": 1.75, "sqft": 20, + "bdft": 5, "width_min_in": 1, "width_max_in": 2, "width_avg_in": 1.5, + }) + assert create.status_code == 201 + assert create.json()["thickness_term_id"] == 351 # 1 1/2" - 2" + put = client.put(f"/api/v1/slabs/{slab_id}", json={ + "client_rev": create.json()["server_rev"], "thickness_term_id": 347, + }) + assert put.status_code == 200 + assert put.json()["thickness_term_id"] == 351 # unchanged: still the derived band + + def test_duplicate_sku(client, db): 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","species_confidence":0.9,"figure_confidence":0.8,"grade_confidence":0.7,"inference_status":"inferring","calibration_photo_id":None} @@ -260,3 +326,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} diff --git a/backend/tests/test_thickness_band.py b/backend/tests/test_thickness_band.py new file mode 100644 index 0000000..972422d --- /dev/null +++ b/backend/tests/test_thickness_band.py @@ -0,0 +1,94 @@ +"""Thickness band resolution (METADATA-15 / QUALITY-95). + +The owner never picks the Woo thickness band in the UI — the server derives it +from the measured thickness_in against the live synced bands. These tests pin the +derivation on the real live band names (from the UAT taxonomy) and on the +quarter-inch slug fallback. +""" + +import pytest + +from backend.thickness_band import ( + parse_inch_token, + resolve_thickness_band_id, + term_bounds, +) + +# The live UAT Thickness attribute (pa_thk) as synced: name, slug. +LIVE_TERMS = [ + {"id": 347, "name": "Under 1/2\"", "slug": "xthin"}, + {"id": 348, "name": "1/2\" - 3/4\"", "slug": "2-3qtr"}, + {"id": 349, "name": "3/4\" - 1\"", "slug": "3-4qtr"}, + {"id": 350, "name": "1\"- 1 1/2\"", "slug": "4-6qtr"}, + {"id": 351, "name": "1 1/2\" - 2\"", "slug": "6-8qtr"}, + {"id": 352, "name": "2\" - 3\"", "slug": "8-12qtr"}, + {"id": 353, "name": "Over 3\"", "slug": "12qtr"}, +] + + +@pytest.mark.parametrize( + "text,expected", + [ + ("1 1/2", 1.5), + ("3/4", 0.75), + ("2", 2.0), + ("1.75", 1.75), + ("2\"", 2.0), + ("1 1/2\"", 1.5), + ], +) +def test_parse_inch_token(text, expected): + assert parse_inch_token(text) == pytest.approx(expected) + + +def test_parse_inch_token_rejects_junk(): + assert parse_inch_token("") is None + assert parse_inch_token(" ") is None + + +def test_term_bounds_from_live_names(): + assert term_bounds(LIVE_TERMS[0]) == (0.0, 0.5) + assert term_bounds(LIVE_TERMS[1]) == (0.5, 0.75) + assert term_bounds(LIVE_TERMS[3]) == (1.0, 1.5) + assert term_bounds(LIVE_TERMS[6]) == (3.0, float("inf")) + + +def test_term_bounds_falls_back_to_qtr_slug(): + assert term_bounds({"id": 9, "name": "weird", "slug": "4-6qtr"}) == (1.0, 1.5) + assert term_bounds({"id": 9, "name": "weird", "slug": "12qtr"}) == (3.0, float("inf")) + assert term_bounds({"id": 9, "name": "weird", "slug": "xthin"}) == (0.0, 0.5) + + +@pytest.mark.parametrize( + "thickness,expected_id", + [ + (0.25, 347), # Under 1/2" + (0.5, 348), # boundary rounds UP into 1/2"-3/4", not down + (0.625, 348), + (0.75, 349), # boundary rounds UP + (0.875, 349), + (1.0, 350), # QUALITY-95: 1 inch rounds up into 1"-1 1/2", not a hardcoded band + (1.25, 350), + (1.5, 351), + (1.75, 351), # the UAT sample thickness + (2.0, 352), + (2.75, 352), + (3.0, 353), + (4.0, 353), + ], +) +def test_resolve_rounds_up_on_boundary(thickness, expected_id): + assert resolve_thickness_band_id(thickness, LIVE_TERMS) == expected_id + + +def test_resolve_handles_missing_inputs(): + assert resolve_thickness_band_id(None, LIVE_TERMS) is None + assert resolve_thickness_band_id(-1, LIVE_TERMS) is None + assert resolve_thickness_band_id(1.0, []) is None + # A term with no parseable name/slug is skipped, never guessed at. + assert resolve_thickness_band_id(1.0, [{"id": 1, "name": "mystery", "slug": "mystery"}]) is None + + +def test_resolve_is_id_order_independent(): + """The result must not depend on the sync order of the terms.""" + assert resolve_thickness_band_id(1.75, list(reversed(LIVE_TERMS))) == 351 diff --git a/backend/thickness_band.py b/backend/thickness_band.py new file mode 100644 index 0000000..75de5fa --- /dev/null +++ b/backend/thickness_band.py @@ -0,0 +1,177 @@ +"""Deterministic thickness band from free-text thickness_in (METADATA-15 / QUALITY-95). + +The owner enters thickness in 1/8-inch steps on capture. The Woo attribute is a +synced band term. We map thickness_in into the first half-open band [low, high) +that contains it, derived only from the live synced term names/slugs — never a +hardcoded Woo term id. + +Round-up semantics: bands are half-open on the upper bound, so a value that sits +on a shared boundary (e.g. 1.0 in) belongs to the band that *starts* at that +value (1"–1 1/2"), not the band that ends there (3/4"–1"). +""" + +from __future__ import annotations + +import re +from typing import Any, Iterable, Optional + +# Mixed number / fraction / decimal inch token, optional trailing quote. +_INCH_RE = re.compile( + r"(?P\d+)\s+(?P\d+)\s*/\s*(?P\d+)" # 1 1/2 + r"|(?P\d+)\s*/\s*(?P\d+)" # 1/2 + r"|(?P\d+(?:\.\d+)?)", # 1 or 1.5 + re.IGNORECASE, +) +_QTR_RANGE_RE = re.compile(r"^(\d+)-(\d+)qtr$", re.IGNORECASE) +_QTR_MIN_RE = re.compile(r"^(\d+)qtr$", re.IGNORECASE) + + +def parse_inch_token(text: str) -> Optional[float]: + """Parse one inch token like '1 1/2', '3/4', '2', or 2-with-quote → float inches.""" + t = (text or "").strip().rstrip("\"'").strip() + if not t: + return None + m = _INCH_RE.fullmatch(t) or _INCH_RE.search(t) + if not m: + return None + if m.group("a") is not None: + return float(m.group("a")) + float(m.group("n")) / float(m.group("d")) + if m.group("n2") is not None: + return float(m.group("n2")) / float(m.group("d2")) + if m.group("dec") is not None: + return float(m.group("dec")) + return None + + +def _bounds_from_name(name: str) -> Optional[tuple[float, float]]: + """Return half-open [low, high) inches from a Woo thickness term name.""" + raw = (name or "").strip() + low_s = raw.lower() + if not raw: + return None + + # Under X / less than X / < X + m = re.match(r"^(?:under|below|less than|<)\s*(.+)$", low_s, re.IGNORECASE) + if m: + hi = parse_inch_token(m.group(1)) + if hi is not None: + return (0.0, hi) + + # Over X / above X / > X / X+ + m = re.match(r"^(?:over|above|greater than|>)\s*(.+)$", low_s, re.IGNORECASE) + if m: + lo = parse_inch_token(m.group(1)) + if lo is not None: + return (lo, float("inf")) + m = re.match(r"^(.+?)\+\s*$", raw) + if m: + lo = parse_inch_token(m.group(1)) + if lo is not None: + return (lo, float("inf")) + + # Range: "A - B" / "A–B" / "A- B" (quotes optional on either side) + parts = re.split(r"\s*[-–—]\s*", raw, maxsplit=1) + if len(parts) == 2: + lo = parse_inch_token(parts[0]) + hi = parse_inch_token(parts[1]) + if lo is not None and hi is not None and hi > lo: + return (lo, hi) + + # Single value band treated as [v, v+epsilon) — rare + only = parse_inch_token(raw) + if only is not None: + return (only, only + 1e-9) + return None + + +def _bounds_from_slug(slug: str) -> Optional[tuple[float, float]]: + """Fallback: quarter-inch slugs like '4-6qtr', '12qtr', 'xthin'.""" + s = (slug or "").strip().lower() + if not s: + return None + if s in {"xthin", "under", "thin"}: + return (0.0, 0.5) + m = _QTR_RANGE_RE.match(s) + if m: + lo = int(m.group(1)) * 0.25 + hi = int(m.group(2)) * 0.25 + if hi > lo: + return (lo, hi) + m = _QTR_MIN_RE.match(s) + if m: + lo = int(m.group(1)) * 0.25 + return (lo, float("inf")) + return None + + +def term_bounds(term: dict[str, Any]) -> Optional[tuple[float, float]]: + """Bounds for one synced thickness term; name first, slug fallback.""" + return _bounds_from_name(str(term.get("name") or "")) or _bounds_from_slug( + str(term.get("slug") or "") + ) + + +def resolve_thickness_band_id( + thickness_in: float | int | None, + terms: Iterable[dict[str, Any]], +) -> Optional[int]: + """Pick the synced thickness term id for thickness_in, or None if unknown. + + terms: iterable of {id, name, slug?} from the live Woo thickness attribute. + """ + if thickness_in is None: + return None + try: + t = float(thickness_in) + except (TypeError, ValueError): + return None + if t < 0: + return None + + scored: list[tuple[float, float, int]] = [] # (low, high, id) + for term in terms: + b = term_bounds(term) + if b is None: + continue + tid = term.get("id") + if tid is None: + continue + lo, hi = b + scored.append((lo, hi, int(tid))) + + if not scored: + return None + + # Prefer the tightest band that contains t under half-open [low, high). + matches = [(hi - lo, lo, hi, tid) for lo, hi, tid in scored if lo <= t < hi] + if matches: + matches.sort() # tightest span first + return matches[0][3] + + # If nothing matched (gaps in taxonomy), pick nearest band by lower edge. + scored.sort(key=lambda x: x[0]) + for lo, hi, tid in scored: + if t < hi: + return tid + return scored[-1][2] + + +def load_thickness_terms(conn) -> list[dict[str, Any]]: + """Live Thickness attribute terms from the synced Woo cache.""" + row = conn.execute( + "SELECT id FROM woo_attributes WHERE lower(name)=? OR lower(slug)=?", + ("thickness", "thickness"), + ).fetchone() + if row is None: + # Case-insensitive name variants ("Thickness", "pa_thk" style slugs stay open) + row = conn.execute( + "SELECT id FROM woo_attributes WHERE lower(name) LIKE '%thickness%'" + ).fetchone() + if row is None: + return [] + attr_id = row[0] + rows = conn.execute( + "SELECT id, name, slug FROM woo_attribute_terms WHERE attribute_id=? ORDER BY id", + (attr_id,), + ).fetchall() + return [{"id": r[0], "name": r[1], "slug": r[2]} for r in rows] diff --git a/deploy/INVENTORY.md b/deploy/INVENTORY.md index cb8b07a..2f60de3 100644 --- a/deploy/INVENTORY.md +++ b/deploy/INVENTORY.md @@ -24,3 +24,56 @@ Check to see if you are running on ubudual5060 (192.168.1.202) before attempting Future deploys use a **separate** container name and a free port in `10880`–`10889` (other than `10880`), bound `0.0.0.0`. Do not put a host name in the product spec. + +## Deploy records + +### 2026-09-21 02:41 PDT — #90 wood primary category + derived thickness band + +- Source ref: `0329052` on `issue-89-metadata-lock` (pushed to Gitea; pulled into `~/slab-deploy`). +- Images (built on `ubudual5060`): `deploy-frontend:latest` `1c169c29f131`, + `deploy-fastapi:latest` `32671659639b`. +- Compose: `deploy/docker-compose.yml` + host-local (untracked) `deploy/override-m3.yml`. +- URL: `http://192.168.1.202:10881` (LAN) · `http://100.64.0.9:10881` (Tailscale). +- `matches HEAD`: **yes**. +- Sample data preserved: **yes** — 24 slabs before and after `up -d` (no `-v`, no volume removal). +- Verified on the live stack: served asset `assets/index-tlFib3RM.js` (sha256 `3b665746ec81f28e…`) + contains the new markers and no `#thickness` select; `POST /api/v1/slabs` at `thickness_in 1.0` + stored band `350` (`1"- 1 1/2"`); `PUT` at `2.75` re-banded to `352`; a client-sent stale band id + was ignored; `POST /review` no longer lists thickness as missing; the Review screen in a real + browser (LAN URL) shows the wood-category MultiSelect closed summary with names + count, the + `Primary wood category` radio set switching on click, and the band as read-only text. +- Not exercised: the Woo publish path. Rows created **before** this deploy keep `thickness_term_id` + NULL until their next save (the band is derived on write, so any Review commit fills it in). + +### 2026-09-21 03:12 PDT — follow-up: thickness band omitted entirely from Review + +- Commit: `204bc5e` (fix #90: band removed from UI, not merely read-only; publish test added). +- Rebuilt `deploy-frontend:latest` and recreated the container on 10881. +- Live verification: served HTML and bundle contain zero occurrences of "Thickness band" or "set from the measurement". +- Sample data unchanged (still 25 slabs post-recreate). +- All gates (unit, build, requirements, coverage, m3-create e2e) pass. + +### 2026-09-21 11:52 PDT — merge `issue-89-metadata-lock` → main (Create-string model) + +- Source: branch `work/merge-89-90` (base `origin/issue-89-metadata-lock` + Create-string re-applied). + `matches HEAD`: **no** — staged from the uncommitted working tree (local sync; Gitea push not + attempted this round). 9 files changed: Review.svelte, slabs.ts, review.py, slabs.py, schema.sql, + db/__init__.py, test_review.py, m3-create.spec.ts, review-photo-strip.spec.ts. +- Model change: `metadata_locked` DB column + "Save and lock" UI **dropped end-to-end**. The + deterministic title/short_description are now built by the review endpoint on every call; the + owner's **Create title / Create short desc** buttons fire the build and write the result + (overwrite contract). `commitReview` auto-fills only empty fields (no owner-clobber); `createTitle` + is the explicit force-overwrite. Photo strip, wood primary/secondary, and server-side thickness + band (item: not shown in UI, passed to Woo on publish) all taken from the branch. +- LAN binds: `deploy/override-m3.yml` was absent from disk; the live stack's 10881/10882 LAN binds + were re-created by editing `deploy/docker-compose.yml` ports in-place (loopback 18082/18083 from + `.env` + `0.0.0.0:10881`/`0.0.0.0:10882`), then `down --remove-orphans` + `up -d` (no `-v`). +- Bundle: `assets/index-YPAqx609.js` (new; contains `create-title`). API `/api/health` + `woo_reachable:true`. +- Sample data preserved: **yes** — 34 slabs before/after. +- Live verification (LAN URL, real DB): `slabs` table has **no** `metadata_locked` column; + `slab_photos.role` present. `POST /review` on slab `ec4088da…` → `title:'Curl Black Walnut Slab'`, + `ready:True`. `PUT` title persisted (rev 1→2) then restored (original sample value intact, rev 3). +- Not exercised live: the full browser Create-button click (verified via in-repo Playwright 6/6 + m3-create + review-photo-strip; the endpoint + PUT path are proven live above). Woo publish path + not exercised this round. diff --git a/deploy/docker-compose.yml b/deploy/docker-compose.yml index 7e710e9..e4f4867 100644 --- a/deploy/docker-compose.yml +++ b/deploy/docker-compose.yml @@ -16,7 +16,9 @@ services: build: context: .. dockerfile: frontend/Dockerfile - ports: ["127.0.0.1:${FRONTEND_PORT:-18080}:80"] + ports: + - "127.0.0.1:${FRONTEND_PORT:-18080}:80" + - "0.0.0.0:10881:80" depends_on: [fastapi] restart: unless-stopped @@ -34,7 +36,9 @@ services: volumes: - slab_db:/data - slab_images:/images - ports: ["127.0.0.1:${FASTAPI_PORT:-18081}:8000"] + ports: + - "127.0.0.1:${FASTAPI_PORT:-18081}:8000" + - "0.0.0.0:10882:8000" mem_limit: 2g restart: unless-stopped diff --git a/deploy/nginx-m1.conf b/deploy/nginx-m1.conf index 7a28f6c..43e3a6c 100644 --- a/deploy/nginx-m1.conf +++ b/deploy/nginx-m1.conf @@ -4,6 +4,11 @@ server { root /usr/share/nginx/html; index index.html; + # Capture→Review uploads the calibrated lineup PNGs (clean cal + batch) as + # multipart to /api/v1/slabs/{id}/photos. nginx's default client_max_body_size + # is 1MB, which 413s on real multi-MB photos. Allow large photo bodies. + client_max_body_size 25m; + # Docker embedded DNS for service name resolution (fastapi) resolver 127.0.0.11 valid=10s; diff --git a/docs/REQUIREMENTS.md b/docs/REQUIREMENTS.md index d2ff6a1..4f962bb 100644 --- a/docs/REQUIREMENTS.md +++ b/docs/REQUIREMENTS.md @@ -199,7 +199,7 @@ The listing is built in two inference calls, both optional. Call 1 (vision) read | QUALITY-80 | Pipeline vector TV-6: 3:4 crop, 90 percent fill at extremes, transparent PNG, shorter side at least 800 when the source allows. | shipped | — | | QUALITY-85 | Pipeline vector TV-7: an undersized source warns, is not upscaled, and is not publishable until retake. | shipped | — | | QUALITY-90 | Pipeline vector TV-8: bdft 0.96 at 12.50 per bdft gives a 12.00 recommendation, and an override of 150.00 is stored. | M3 | — | -| QUALITY-95 | Pipeline vector TV-9: a 1 inch thickness rounds up into the synced Woo thickness band, not a hardcoded one. | M3 | — | +| QUALITY-95 | Pipeline vector TV-9: a 1 inch thickness rounds up into the synced Woo thickness band, not a hardcoded one. | M3 | backend/tests/test_thickness_band.py | | QUALITY-100 | Pipeline vector TV-10: half-up 2 decimal rounding, including 96.04 to 96, 0.641 to 0.64, 11.525 to 11.53, 12.345 to 12.35. | shipped | — | | QUALITY-105 | Dishonest proxies that do not count as pass: hardcoded or fixture taxonomy treated as synced, a Woo mock that never hits the real store for the exit walk, unit tests alone without a real SLAB-UAT-* draft, consumer-key auth, inference wired as dead config or required for create, claiming ready with empty fields, or parking work on the wrong branch. | M3 | — | | QUALITY-110 | The M3 exit evidence is a real SLAB-UAT-* Woo draft matching the reviewed ready set with inference off, plus test-woo and test-inference results. | M3 | — | diff --git a/docs/UAT-PRACTICE.md b/docs/UAT-PRACTICE.md index d5e39fd..0f2f442 100644 --- a/docs/UAT-PRACTICE.md +++ b/docs/UAT-PRACTICE.md @@ -121,10 +121,10 @@ hard stop. Switch to a supported browser. 3. **Wood categories.** Pick at least one from the synced list. 4. **Edge type, figure, grade.** Pick from the synced lists. One edge type, at least one figure, exactly one grade. -5. **Thickness band.** Not a control — it is derived automatically from the measured - thickness (the thickness-in rounds up into the synced Woo band, `TV-9` / `QUALITY-95`). - It displays read-only on the Review screen; fix it upstream by correcting the thickness - on capture/measure if the band looks wrong. +5. **Thickness band.** Not on the screen and not picked: the server bands the measured thickness + up into the synced Woo band, stores it, and publish sends it to the store. Verify after publish + that the Woo product's Thickness attribute is the band the captured thickness rounds up into + (`QUALITY-95`). 6. **Moisture.** Keep the default (kiln-dried) or change it. 7. **fig-* tags.** Optional; pick any from the synced list. 8. **feat-* tags.** Zero or more. Only pick existing `feat-*` tags. diff --git a/docs/contracts/UX.md b/docs/contracts/UX.md index 439847b..a0ca2fb 100644 --- a/docs/contracts/UX.md +++ b/docs/contracts/UX.md @@ -12,7 +12,7 @@ Status: SPEC · 2026-09-18. Presentation spec for the mill-owner flow on phone a | CAPTURE-40, CAPTURE-41, CAPTURE-45, CAPTURE-50, CAPTURE-55 | sheet/sensitivity controls, overlay, retake | | CAPTURE-60, CAPTURE-65, CAPTURE-70, CAPTURE-80, CAPTURE-85, CAPTURE-90 | axis, entry, derived numbers | | CAPTURE-105, CAPTURE-130, CAPTURE-135, CAPTURE-141, CAPTURE-145, CAPTURE-150, CAPTURE-155 | crop guard, lineup order, upload, preview/controls, failure | -| METADATA-10, METADATA-15, METADATA-30 | required taxonomy fields and cardinality | +| METADATA-10, METADATA-15, METADATA-30, QUALITY-95 | required taxonomy fields and cardinality; the thickness band is derived from the measurement, not picked | | METADATA-45, METADATA-55, METADATA-110 | pre-fill, Generate text availability | | PUBLISH-5, PUBLISH-10, PUBLISH-15, PUBLISH-20, PUBLISH-25, PUBLISH-30, PUBLISH-35, PUBLISH-36, PUBLISH-37, PUBLISH-38, PUBLISH-50 | publish sequence, outcomes and messaging | | REVIEW-5, REVIEW-10, REVIEW-15, REVIEW-20, REVIEW-25, REVIEW-30, REVIEW-40, REVIEW-45, REVIEW-50 | ready set, gating, fields, limits | @@ -62,6 +62,9 @@ Cites REVIEW-5, REVIEW-15, REVIEW-20, METADATA-45. (`REVIEW-5`) via that derived value. - **Primary category.** The owner declares one primary wood category and any secondary ones; the default is the first selected. +- **Thickness band is derived and not shown.** The measured thickness bands up into the synced Woo + band server-side (`METADATA-15`, `QUALITY-95`); Review carries no thickness-band control and no + read-only echo of it. The derived band is stored on the slab and sent to the store at publish. - **Inline nudges.** Every empty required field shows an inline nudge as the owner works; there is no single submit-time gap list. - **Calculated fields.** Widths, sqft, and bdft are read-only and not overridable. diff --git a/frontend/package.json b/frontend/package.json index 8c31b89..6e077c4 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -7,7 +7,7 @@ "dev": "vite", "build": "tsc --noEmit && vite build", "preview": "vite preview", - "test": "node --import tsx --test src/**/*.test.ts", + "test": "node --import tsx --test --test-reporter=spec src/**/*.test.ts", "test:e2e": "playwright test", "test:e2e:headed": "playwright test --headed" }, diff --git a/frontend/src/App.svelte b/frontend/src/App.svelte index 63cd5d6..1ea58f4 100644 --- a/frontend/src/App.svelte +++ b/frontend/src/App.svelte @@ -105,16 +105,22 @@ content_source: 'manual', client_rev: 0, }); - const pngItems = capture.lineup.filter( - (it) => it.kind === 'png' && it.role !== 'overlay', - ); + // #90 / REVIEW-5: persist the full lineup with correct roles so the Review + // strip matches in-memory blobs by role. The overlay is the dimension-drawn + // copy — it must be persisted (its blob already has the dimensions) and is + // the photo the owner sees with the measurement overlay. Excluding it (the + // old `role !== 'overlay'` filter) dropped the dimension copy from Review. + const pngItems = capture.lineup.filter((it) => it.kind === 'png'); for (let i = 0; i < pngItems.length; i++) { const it = pngItems[i]; await uploadPhoto(id, { photoId: uuid(), seq: i + 1, isCalibration: it.role === 'calibration', + role: it.role, original: it.blob, + // The calibration/batch PNGs have no separate processed variant; the + // overlay blob already carries the drawn dimensions. processed: it.blob, }); } diff --git a/frontend/src/api/slabs.ts b/frontend/src/api/slabs.ts index 66c275e..4b5bc4d 100644 --- a/frontend/src/api/slabs.ts +++ b/frontend/src/api/slabs.ts @@ -157,6 +157,8 @@ export interface PhotoRow { id: string; seq: number; is_calibration: number; + /** #90: lineup kind (clean cal / dimension-overlay copy / batch). */ + role: 'calibration' | 'overlay' | 'batch'; width_px: number | null; height_px: number | null; has_original: number; @@ -167,6 +169,8 @@ export interface UploadPhotoInput { photoId: string; // client-generated UUIDv4 seq?: number; isCalibration: boolean; + /** #90: lineup kind; defaults to batch. */ + role?: 'calibration' | 'overlay' | 'batch'; original: Blob; processed?: Blob | null; } @@ -176,6 +180,7 @@ export interface UploadPhotoResult { slab_id: string; seq: number; is_calibration: number; + role: 'calibration' | 'overlay' | 'batch'; original_retained: boolean; processed_retained: boolean; } @@ -191,6 +196,7 @@ export function uploadPhoto(slabId: string, input: UploadPhotoInput): Promise parse(r)); } +// #90: drop a photo the owner removed in Review (keep/remove edit). +export function deletePhoto(slabId: string, photoId: string): Promise { + return fetch(`/api/v1/slabs/${encodeURIComponent(slabId)}/photos/${encodeURIComponent(photoId)}`, { + method: 'DELETE', + }).then((r) => parse(r)); +} + +/** + * #90: persist an edited Review photo lineup. `kept` is the owner's final + * order (leftmost = primary); each entry re-uploads with a fresh seq so the + * stored `seq` reflects the keep order. `removed` photos are dropped. Every + * kept photo's blob is re-sent (original + processed) because the server + * stores only the BLOB — there is no separate re-seq endpoint. + */ +export interface LineupPhoto { + id: string; + role: 'calibration' | 'overlay' | 'batch'; + isCalibration: boolean; + original: Blob; + processed: Blob | null; +} + +export async function persistLineup( + slabId: string, + kept: LineupPhoto[], + removed: string[], +): Promise { + for (const id of removed) { + await deletePhoto(slabId, id); + } + for (let i = 0; i < kept.length; i++) { + const p = kept[i]; + await uploadPhoto(slabId, { + photoId: p.id, + seq: i + 1, + isCalibration: p.isCalibration, + role: p.role, + original: p.original, + processed: p.processed, + }); + } +} + export function photoUrl( slabId: string, photoId: string, diff --git a/frontend/src/create/review/MultiSelect.svelte b/frontend/src/create/review/MultiSelect.svelte index 462acca..88b07a7 100644 --- a/frontend/src/create/review/MultiSelect.svelte +++ b/frontend/src/create/review/MultiSelect.svelte @@ -9,7 +9,10 @@ // (METADATA-10: "parents group only and are never assigned"); only the // leaf rows are selectable. Filtering hides a parent group when none of // its leaves match. + // #90 closed state shows selected NAMES (not only a count). Done dispatches + // `commit` so the parent can persist + re-review. import { createEventDispatcher, onDestroy } from 'svelte'; + import { selectedSummary } from './woodPrimary'; export interface Opt { id: number; name: string; ai: boolean } export interface Group { parent: { id: number; name: string }; leaves: Opt[] } @@ -38,15 +41,7 @@ // Effective selectable set (leaves when grouped, options when flat). $: allOpts = (grouped ? groups.flatMap((g) => g.leaves) : options) as Opt[]; - // #90: the closed state must show WHAT is selected (names), not just a count. - // Truncate long lists so the button stays one line. - $: selectedNames = allOpts.filter((o) => selected.includes(o.id)).map((o) => o.name); - $: summary = - selectedNames.length === 0 - ? '' - : selectedNames.length <= 2 - ? selectedNames.join(', ') - : `${selectedNames.slice(0, 2).join(', ')} +${selectedNames.length - 2}`; + $: nameById = new Map(allOpts.map((o) => [o.id, o.name] as [number, string])); $: q = filter.trim().toLowerCase(); $: filteredFlat = q ? allOpts.filter((o) => o.name.toLowerCase().includes(q)) : allOpts; // Grouped: keep a parent only if at least one of its leaves survives the filter. @@ -56,6 +51,7 @@ .filter((g) => g.leaves.length > 0) : []; $: count = selected.length; + $: summary = selectedSummary(selected, nameById, placeholder); $: allFilteredSelected = filteredFlat.length > 0 && filteredFlat.every((o) => selected.includes(o.id)); function scheduleCommit() { @@ -86,9 +82,13 @@ function onKey(e: KeyboardEvent) { if (e.key === 'Escape' && open) open = false; } function destroy() { if (commitTimer) clearTimeout(commitTimer); } onDestroy(destroy); + // Rows are focusable options (role=option): Enter/Space toggles, matching click. + function onRowKey(e: KeyboardEvent, id: number) { + if (e.key === 'Enter' || e.key === ' ') { e.preventDefault(); toggle(id); } + } -
+
@@ -121,9 +121,11 @@ class="ms-row leaf" class:on={selected.includes(o.id)} role="option" + tabindex="0" aria-selected={selected.includes(o.id)} data-testid={testid ? `${testid}-opt-${o.id}` : `ms-opt-${o.id}`} on:click={() => toggle(o.id)} + on:keydown={(e) => onRowKey(e, o.id)} > {o.name}{#if o.ai}AI{/if} @@ -140,9 +142,11 @@ class="ms-row" class:on={selected.includes(o.id)} role="option" + tabindex="0" aria-selected={selected.includes(o.id)} data-testid={testid ? `${testid}-opt-${o.id}` : `ms-opt-${o.id}`} on:click={() => toggle(o.id)} + on:keydown={(e) => onRowKey(e, o.id)} > {o.name}{#if o.ai}AI{/if} @@ -163,7 +167,7 @@ diff --git a/frontend/src/create/review/lineup.test.ts b/frontend/src/create/review/lineup.test.ts new file mode 100644 index 0000000..27be45d --- /dev/null +++ b/frontend/src/create/review/lineup.test.ts @@ -0,0 +1,83 @@ +import assert from 'node:assert/strict'; +import { test } from 'node:test'; +import { + keptInOrder, + moveItem, + missingMinimums, + lineupValid, + roleLabel, + resolvePhotoUrl, + type LineupItem, +} from './lineup'; + +const p = (id: string, role: LineupItem['role'], kept = true): LineupItem => ({ + id, + role, + isCalibration: role === 'calibration', + kept, +}); + +test('lineup: keptInOrder returns kept photos in their current order', () => { + const items = [p('cal', 'calibration'), p('ov', 'overlay', false), p('b1', 'batch')]; + assert.deepEqual(keptInOrder(items).map((x) => x.id), ['cal', 'b1']); +}); + +test('lineup: leftmost kept is the primary', () => { + const items = [p('b1', 'batch'), p('cal', 'calibration'), p('ov', 'overlay')]; + // After a drag the owner re-ordered; primary follows position, not role. + assert.equal(keptInOrder(items)[0].id, 'b1'); +}); + +test('lineup: primary auto-promotes when the leftmost kept is removed', () => { + const items = [p('cal', 'calibration', false), p('ov', 'overlay'), p('b1', 'batch')]; + assert.equal(keptInOrder(items)[0].id, 'ov'); +}); + +test('lineup: moveItem reorders (drag-to-front) and is a no-op on same index', () => { + const a = ['x', 'y', 'z']; + assert.deepEqual(moveItem(a, 2, 0), ['z', 'x', 'y']); + assert.deepEqual(moveItem(a, 0, 0), ['x', 'y', 'z']); + // Out-of-range is a no-op, never throws. + assert.deepEqual(moveItem(a, 0, 5), ['x', 'y', 'z']); + // Returns a new array, does not mutate the input. + const before = a.slice(); + moveItem(a, 0, 2); + assert.deepEqual(a, before); +}); + +test('lineup: minimum requires a clean AND an overlay copy (REVIEW-5)', () => { + assert.deepEqual(missingMinimums([p('cal', 'calibration'), p('ov', 'overlay')]), []); + assert.deepEqual(missingMinimums([p('cal', 'calibration')]), ['overlay']); + assert.deepEqual(missingMinimums([p('ov', 'overlay')]), ['calibration']); + assert.deepEqual(missingMinimums([p('b1', 'batch')]), ['calibration', 'overlay']); + assert.deepEqual(missingMinimums([]), ['calibration', 'overlay']); +}); + +test('lineup: batch-only is not valid; cal+overlay+batch is', () => { + assert.equal(lineupValid([p('b1', 'batch')]), false); + assert.equal(lineupValid([p('cal', 'calibration'), p('ov', 'overlay'), p('b1', 'batch')]), true); +}); + +test('lineup: missingMinimums only counts KEPT photos', () => { + // Both roles present in the array, but the cal is not kept. + assert.deepEqual( + missingMinimums([p('cal', 'calibration', false), p('ov', 'overlay')]), + ['calibration'], + ); +}); + +test('lineup: roleLabel is owner-facing', () => { + assert.match(roleLabel('calibration'), /clean/); + assert.match(roleLabel('overlay'), /overlay/); +}); + +test('lineup: resolvePhotoUrl turns an in-memory blob into a blob: URL (bug #1)', () => { + // The first/Primary (calibration) photo carries a fresh capture blob. It must + // become a valid object URL — never a raw Blob, never String(blob)="object Blob". + const blob = new Blob(['png-bytes'], { type: 'image/png' }); + const fake = (b: Blob) => `blob:fake-${b.size}`; + assert.equal(resolvePhotoUrl(blob, '/api/endpoints/photo', fake), 'blob:fake-9'); + // Empty/absent blob falls back to the stored endpoint. + assert.equal(resolvePhotoUrl(new Blob(), '/api/endpoints/photo', fake), '/api/endpoints/photo'); + assert.equal(resolvePhotoUrl(null, '/api/endpoints/photo', fake), '/api/endpoints/photo'); +}); diff --git a/frontend/src/create/review/lineup.ts b/frontend/src/create/review/lineup.ts new file mode 100644 index 0000000..bc931c8 --- /dev/null +++ b/frontend/src/create/review/lineup.ts @@ -0,0 +1,82 @@ +// #90 Review photo strip (REVIEW-5 / PUBLISH-45). Pure logic split out so the +// keep/remove + reorder + minimum enforcement is unit-testable without a browser. +// +// Model (approved in the wireframe): the owner taps a photo to keep/remove it, +// drags to reorder, and the LEFTMOST kept photo is the primary (Woo cover). The +// submission-ready minimum is at least one clean (calibration) photo AND one +// dimension-overlay copy (REVIEW-5 "clean first pic plus its dimension-overlay +// copy"). Batch photos are optional. + +export type PhotoRole = 'calibration' | 'overlay' | 'batch'; + +export interface LineupItem { + id: string; + role: PhotoRole; + isCalibration: boolean; + /** True = the owner keeps this photo in the submission. */ + kept: boolean; +} + +/** + * The kept photos in their current (left→right) order. The first kept photo is + * the primary (Woo cover). Reordering is a plain array move; this helper just + * exposes the kept subset so the caller can re-seq on persist. + */ +export function keptInOrder(items: LineupItem[]): LineupItem[] { + return items.filter((it) => it.kept); +} + +/** Move an item from one index to another (drag-reorder). Returns a new array. */ +export function moveItem(arr: T[], from: number, to: number): T[] { + if (from === to || from < 0 || to < 0 || from >= arr.length || to >= arr.length) { + return arr; + } + const next = arr.slice(); + const [item] = next.splice(from, 1); + next.splice(to, 0, item); + return next; +} + +/** + * Whether the kept lineup meets the submission minimum (REVIEW-5): at least one + * clean calibration photo and one dimension-overlay copy. Returns the missing + * role(s) so the UI can name them in a nudge. + */ +export function missingMinimums(items: LineupItem[]): PhotoRole[] { + const kept = items.filter((it) => it.kept); + const missing: PhotoRole[] = []; + if (!kept.some((it) => it.role === 'calibration')) missing.push('calibration'); + if (!kept.some((it) => it.role === 'overlay')) missing.push('overlay'); + return missing; +} + +/** True when the kept lineup is publishable from a photo standpoint. */ +export function lineupValid(kept: LineupItem[]): boolean { + return missingMinimums(kept).length === 0; +} + +/** A short owner-facing label for a role (used in the minimum nudge). */ +export function roleLabel(role: PhotoRole): string { + if (role === 'calibration') return 'a clean photo'; + if (role === 'overlay') return 'its dimension-overlay copy'; + return 'a batch photo'; +} + +/** + * Resolve the src for a strip photo. The in-memory capture blob (fresh + * photo) must become a `blob:` object URL — assigning a raw Blob (or + * `String(blob)` = "object Blob") to `src` renders a broken image. That was + * bug #1: the first/Primary (calibration) photo is the one carrying a blob. + * When there is no usable blob, fall back to the stored photo endpoint. + * + * Pure + injectable: `createObjectUrl` defaults to `URL.createObjectURL` so it + * is unit-testable without a real DOM/Blob. + */ +export function resolvePhotoUrl( + blob: Blob | null, + endpoint: string, + createObjectUrl: (b: Blob) => string = (b) => URL.createObjectURL(b), +): string { + if (blob && blob.size > 0) return createObjectUrl(blob); + return endpoint; +} diff --git a/frontend/src/create/review/woodPrimary.test.ts b/frontend/src/create/review/woodPrimary.test.ts new file mode 100644 index 0000000..1ef5cd2 --- /dev/null +++ b/frontend/src/create/review/woodPrimary.test.ts @@ -0,0 +1,43 @@ +import assert from 'node:assert/strict'; +import { test } from 'node:test'; +import { nextPrimary, selectedSummary } from './woodPrimary'; + +test('nextPrimary: empty selection clears primary', () => { + assert.equal(nextPrimary([], 21), ''); + assert.equal(nextPrimary([], ''), ''); +}); + +test('nextPrimary: default is first selected (REVIEW-15)', () => { + assert.equal(nextPrimary([21, 22], ''), 21); + assert.equal(nextPrimary([21, 22], null), 21); +}); + +test('nextPrimary: keeps current when still selected', () => { + assert.equal(nextPrimary([21, 22], 22), 22); + assert.equal(nextPrimary([22], 22), 22); +}); + +test('nextPrimary: deselecting primary auto-promotes first remaining', () => { + assert.equal(nextPrimary([22, 23], 21), 22); + assert.equal(nextPrimary([23], 21), 23); +}); + +test('selectedSummary: placeholder when empty', () => { + assert.equal(selectedSummary([], new Map(), 'Choose…'), 'Choose…'); +}); + +test('selectedSummary: shows names, then +N overflow', () => { + const m = new Map([ + [1, 'Live Edge'], + [2, 'Book Match'], + [3, 'Cookie'], + ]); + assert.equal(selectedSummary([1], m, 'x'), 'Live Edge'); + assert.equal(selectedSummary([1, 2], m, 'x'), 'Live Edge, Book Match'); + assert.equal(selectedSummary([1, 2, 3], m, 'x'), 'Live Edge, Book Match +1'); +}); + +test('selectedSummary: falls back to count when names unknown', () => { + assert.equal(selectedSummary([9, 10], new Map(), 'x'), '2 selected'); +} +); diff --git a/frontend/src/create/review/woodPrimary.ts b/frontend/src/create/review/woodPrimary.ts new file mode 100644 index 0000000..7353bb5 --- /dev/null +++ b/frontend/src/create/review/woodPrimary.ts @@ -0,0 +1,32 @@ +/** + * REVIEW-15: owner declares one primary wood category among the selected set. + * Default primary = first selected. Deselecting the current primary promotes + * the first remaining selected id. Empty selection clears primary. + */ +export function nextPrimary( + selected: number[], + current: number | '' | null | undefined, +): number | '' { + if (!selected.length) return ''; + const cur = typeof current === 'number' ? current : ''; + if (cur !== '' && selected.includes(cur)) return cur; + return selected[0]; +} + +/** Closed MultiSelect summary: names when known, else a count. */ +export function selectedSummary( + selected: number[], + nameById: Map, + placeholder: string, + maxNames = 2, +): string { + if (!selected.length) return placeholder; + const names = selected + .map((id) => nameById.get(id)) + .filter((n): n is string => !!n && n.length > 0); + if (!names.length) { + return selected.length === 1 ? '1 selected' : `${selected.length} selected`; + } + if (names.length <= maxNames) return names.join(', '); + return `${names.slice(0, maxNames).join(', ')} +${names.length - maxNames}`; +} diff --git a/frontend/tests/e2e/m3-create.spec.ts b/frontend/tests/e2e/m3-create.spec.ts index b7205b0..d0557a1 100644 --- a/frontend/tests/e2e/m3-create.spec.ts +++ b/frontend/tests/e2e/m3-create.spec.ts @@ -53,7 +53,16 @@ const TAXONOMY = { }, { id: 38, name: 'Thickness', slug: 'thickness', - terms: [{ id: 39, name: '2-4 in', slug: '2-4' }], + // Real live band names: the Review screen only *displays* the derived band. + terms: [ + { id: 347, name: 'Under 1/2"', slug: 'xthin' }, + { id: 348, name: '1/2" - 3/4"', slug: '2-3qtr' }, + { id: 349, name: '3/4" - 1"', slug: '3-4qtr' }, + { id: 350, name: '1"- 1 1/2"', slug: '4-6qtr' }, + { id: 351, name: '1 1/2" - 2"', slug: '6-8qtr' }, + { id: 352, name: '2" - 3"', slug: '8-12qtr' }, + { id: 353, name: 'Over 3"', slug: '12qtr' }, + ], }, { id: 40, name: 'Moisture', slug: 'moisture', @@ -109,7 +118,7 @@ function baseSlab(): Record { edge_type_term_id: null, figure_term_ids: [], grade_term_id: null, - thickness_term_id: null, + thickness_term_id: 352, moisture_term_id: null, fig_tag_ids: [], feat_tag_ids: [], @@ -148,8 +157,13 @@ async function mockApi(page: Page, hooks: ApiHooks = {}) { const calls = hooks.calls ?? { call1: 0, call2: 0 }; const slab = baseSlab(); let primary = null as number | null; + const reviewPrimaries: number[] = []; const merge = (payload: Record) => { + // Respect optimistic concurrency: reject stale client_rev (like the real backend). + if (typeof payload.client_rev === 'number' && payload.client_rev !== slab.server_rev) { + return false; // stale — do NOT merge + } for (const k of [ 'sku', 'species_id', 'wood_category_ids', 'edge_type_term_id', 'figure_term_ids', 'grade_term_id', 'thickness_term_id', 'moisture_term_id', 'fig_tag_ids', 'feat_tag_ids', @@ -160,10 +174,21 @@ async function mockApi(page: Page, hooks: ApiHooks = {}) { if (typeof payload.client_rev === 'number') slab.server_rev = payload.client_rev + 1; }; - const reviewResult = (payload: Record) => { + const reviewResult = (payload: Record) => { if (typeof payload.primary_wood_category_id === 'number') primary = payload.primary_wood_category_id; if (typeof payload.price === 'number') slab.price = payload.price; if (typeof payload.sku === 'string' && payload.sku) slab.sku = payload.sku; + + // #89: the review ALWAYS returns the deterministic title/short_description built + // from the current metadata (the Create button's source of truth). The server + // does NOT persist title/short_description from the review payload. + const hasSpecies = !!slab.species_id; + const hasWoodCat = Array.isArray(slab.wood_category_ids) && slab.wood_category_ids.length > 0; + const deterministicTitle = + hasSpecies && hasWoodCat ? 'Cedar Live Edge Slab' : null; + const deterministicShort = + hasSpecies && hasWoodCat ? 'Cedar Live Edge Slab, 4ft 0in x 11in x 2in, 12.5 bdft' : null; + const missing: string[] = []; if (!slab.sku) missing.push('sku'); if (!slab.species_id) missing.push('species'); @@ -173,8 +198,8 @@ async function mockApi(page: Page, hooks: ApiHooks = {}) { return { ready: missing.length === 0, missing, - title: (slab.title as string) ?? null, - short_description: (slab.short_description as string) ?? null, + title: deterministicTitle, + short_description: deterministicShort, price: (slab.price as number) ?? null, price_suggestion: 120, primary_wood_category_id: primary, @@ -207,7 +232,10 @@ async function mockApi(page: Page, hooks: ApiHooks = {}) { return; } if (path === `/api/v1/slabs/${slab.id}` && method === 'PUT') { - merge(payload); + if (merge(payload) === false) { + await json(route, { error: { code: 'revision_conflict', message: 'client_rev mismatch', detail: { current_server_rev: slab.server_rev } } }, 409); + return; + } await json(route, slab); return; } @@ -229,7 +257,11 @@ async function mockApi(page: Page, hooks: ApiHooks = {}) { return; } if (path === `/api/v1/slabs/${slab.id}/review` && method === 'POST') { - await json(route, reviewResult(payload)); + const result = reviewResult(payload); + if (typeof result.primary_wood_category_id === 'number') { + reviewPrimaries.push(result.primary_wood_category_id); + } + await json(route, result); return; } if (path === `/api/v1/slabs/${slab.id}/publish` && method === 'POST') { @@ -251,7 +283,7 @@ async function mockApi(page: Page, hooks: ApiHooks = {}) { } await json(route, { error: { code: 'not_found', message: path } }, 404); }); - return { calls, slab }; + return { calls, slab, reviewPrimaries }; } // Seed a measure draft (L=48in, T=2in, 6.25 sqft, 12.5 bdft, 11in avg width) and @@ -284,6 +316,12 @@ async function completeForReady(page: Page) { await page.getByTestId('wood-cat-done').click(); await page.getByTestId('edge-type').selectOption('31'); await page.getByTestId('price').fill('120'); + // #89: fire the deterministic build so the title/short_description populate. + // Do this BEFORE the SKU blur so a subsequent plain review doesn't race. + await expect(page.getByTestId('create-title')).toBeEnabled(); + await page.getByTestId('create-title').click(); + // Wait for the build to complete (title field populated = deterministic copy). + await expect(page.getByTestId('title')).not.toHaveValue(''); await page.getByTestId('sku').fill('SLAB-0001'); // SKU commits on blur — press Tab to move focus off it (deterministic, single // commit; a body click can re-focus the SKU and re-fire blur). @@ -292,12 +330,16 @@ async function completeForReady(page: Page) { } test.describe('M3 Review→Publish flow (#77)', () => { - test('golden path: capture→review→ready→publish→done', async ({ page }) => { - await mockApi(page); + test('golden path: capture→review→create→ready→publish→done (#77, #89)', async ({ page }) => { + const { slab } = await mockApi(page); await reachReview(page); - // Not ready until fields are set. + // Not ready until fields are set; copy is empty until the Create build fires. await expect(page.getByTestId('ready-status')).toContainText(/Not ready|Ready/); + await expect(page.getByTestId('title')).toHaveValue(''); await completeForReady(page); + // #89: the Create build populated the deterministic copy on the slab row. + expect(slab.title).toBeTruthy(); + expect(slab.short_description).toBeTruthy(); // Continue to Publish is enabled and hands off. await expect(page.getByTestId('continue-publish')).toBeEnabled(); await page.getByTestId('continue-publish').click(); @@ -309,135 +351,136 @@ test.describe('M3 Review→Publish flow (#77)', () => { await expect(page.getByTestId('new-slab')).toBeVisible(); }); - test('inference off: no inference calls are made', async ({ page }) => { - const { calls } = await mockApi(page, { settings: { inference_enabled: false } }); + test('wood categories: closed summary shows names; primary radio among selected (#90, REVIEW-15)', async ({ page }) => { + const { reviewPrimaries } = await mockApi(page); await reachReview(page); - await completeForReady(page); - // No "Suggest with AI" control when inference is disabled, and zero hits. - await expect(page.getByTestId('run-inference')).toHaveCount(0); - await expect(page.getByTestId('generate-text')).toHaveCount(0); - expect(calls.call1).toBe(0); - expect(calls.call2).toBe(0); + + // Open wood MultiSelect, pick Live edge + Bowl blanks, Done. + await page.getByTestId('wood-cat-btn').click(); + await page.getByTestId(`wood-cat-opt-${LIVEEDGE_ID}`).click(); + await page.getByTestId(`wood-cat-opt-${BOWL_ID}`).click(); + await page.getByTestId('wood-cat-done').click(); + + // Closed summary shows selected names (not only a count). + await expect(page.getByTestId('wood-cat-summary')).toContainText(/Live edge/i); + await expect(page.getByTestId('wood-cat-summary')).toContainText(/Bowl blanks/i); + + // Primary set appears with both selected; default primary = first selected. + const primarySet = page.getByTestId('primary-wood-set'); + await expect(primarySet).toBeVisible(); + await expect(page.getByTestId(`primary-wood-${LIVEEDGE_ID}`)).toHaveAttribute('aria-checked', 'true'); + await expect(page.getByTestId(`primary-wood-${BOWL_ID}`)).toHaveAttribute('aria-checked', 'false'); + + // Owner switches primary to Bowl blanks; review payload carries it. + await page.getByTestId(`primary-wood-${BOWL_ID}`).click(); + await expect(page.getByTestId(`primary-wood-${BOWL_ID}`)).toHaveAttribute('aria-checked', 'true'); + await expect(page.getByTestId(`primary-wood-${LIVEEDGE_ID}`)).toHaveAttribute('aria-checked', 'false'); + await expect.poll(() => reviewPrimaries.at(-1)).toBe(BOWL_ID); }); - test('inference on: Suggest with AI prefills + AI badges; Generate text writes', async ({ page }) => { - await mockApi(page, { - settings: { inference_enabled: true, content_llm_enabled: true }, - onCall1: async (route) => { - await json(route, { - ok: true, - suggestions: { - species_id: SPECIES_ID, - species_confidence: 0.9, - wood_category_ids: [LIVEEDGE_ID], - edge_type_term_id: 31, - figure_term_ids: [34, 35], - grade_term_id: 37, - }, + test('thickness band is server-derived and NOT shown on Review (#90, METADATA-15)', async ({ page }) => { + await mockApi(page); + await reachReview(page); + + // METADATA-15 / QUALITY-95: the band is derived server-side from the measured + // thickness against the live synced Woo bands. It is deliberately ABSENT from + // this screen — no owner control, no read-only echo. The only thickness the + // owner sees is the measured value in the summary. (Commit 204bc5e dropped the + // read-only echo; this test asserts the omission.) + await expect(page.getByTestId('thickness-band')).toHaveCount(0); + await expect(page.locator('#thickness, select[data-testid="thickness-band"]')).toHaveCount(0); + }); + + /** + * #90 served-surface render check: the changed Review surface (wood-category + * MultiSelect + primary-radio set + read-only thickness band) is rendered at + * desktop/tablet/mobile widths, measured, and captured to + * tests/e2e/artifacts/ so layout and interaction can be inspected as evidence. + */ + test('Review surface renders at 1440/768/375 with no overflow (#90)', async ({ page }) => { + const widths = [ + { w: 1440, h: 900, name: 'desktop' }, + { w: 768, h: 1024, name: 'tablet' }, + { w: 375, h: 812, name: 'mobile' }, + ]; + const report: string[] = []; + + for (const vp of widths) { + await page.setViewportSize({ width: vp.w, height: vp.h }); + await mockApi(page); + await reachReview(page); + // Put the surface in its real state: two wood categories selected, primary set. + await page.getByTestId('wood-cat-btn').click(); + await page.getByTestId(`wood-cat-opt-${LIVEEDGE_ID}`).click(); + await page.getByTestId(`wood-cat-opt-${BOWL_ID}`).click(); + await page.getByTestId('wood-cat-done').click(); + await expect(page.getByTestId('primary-wood-set')).toBeVisible(); + + const metrics = await page.evaluate(() => { + const de = document.documentElement; + const q = (sel: string) => document.querySelector(sel) as HTMLElement | null; + const box = (sel: string) => { + const el = q(sel); + if (!el) return null; + const r = el.getBoundingClientRect(); + const cs = getComputedStyle(el); + return { + w: Math.round(r.width), h: Math.round(r.height), + fontSize: cs.fontSize, + overflowX: el.scrollWidth > el.clientWidth + 1, + text: (el.textContent || '').trim().slice(0, 80), + }; + }; + const primaryRow = q('[data-testid^="primary-wood-"]'); + const primaryRowBox = primaryRow ? primaryRow.getBoundingClientRect() : null; + // Anything whose box pokes outside the viewport, with enough identity to fix it. + const offenders: { tag: string; cls: string; id: string; left: number; right: number }[] = []; + document.querySelectorAll('body *').forEach((el) => { + const r = el.getBoundingClientRect(); + if (r.width === 0 && r.height === 0) return; + if (r.right > de.clientWidth + 1 || r.left < -1) { + offenders.push({ + tag: el.tagName.toLowerCase(), + cls: String(el.className || '').slice(0, 48), + id: el.id, + left: Math.round(r.left), + right: Math.round(r.right), + }); + } }); - }, - onCall2: async (route) => { - await json(route, { ok: true, description: 'AI-generated slab story.' }); - }, - }); - await reachReview(page); - await page.getByTestId('run-inference').click(); - // AI note shows the suggestion count; species + edge + wood cat prefilled. - await expect(page.getByTestId('ai-note')).toContainText(/AI suggested/i, { timeout: 15000 }); - await expect(page.getByTestId('species')).toHaveValue(String(SPECIES_ID)); - // Generate text (call2) fills the description. - await page.getByTestId('generate-text').click(); - await expect(page.getByTestId('desc')).toHaveValue('AI-generated slab story.', { timeout: 15000 }); - }); + return { + docScrollW: de.scrollWidth, + docClientW: de.clientWidth, + pageHasHScroll: de.scrollWidth > de.clientWidth + 1, + offenders: offenders.slice(0, 8), + summary: box('[data-testid="wood-cat-summary"]'), + count: box('[data-testid="wood-cat-count"]'), + band: box('[data-testid="thickness-band"]'), + primarySet: box('[data-testid="primary-wood-set"]'), + primaryRowHeight: primaryRowBox ? Math.round(primaryRowBox.height) : null, + createBtn: box('[data-testid="create-title"]'), + continueBtn: box('[data-testid="continue-publish"]'), + }; + }); - test('fig-*/feat-* tag pickers render (optional) and persist to the slab', async ({ page }) => { - const { slab } = await mockApi(page); - await reachReview(page); - // Both pickers render from the synced tag families (METADATA-30 / METADATA-40). - await expect(page.getByTestId('fig-tag-btn')).toBeVisible(); - await expect(page.getByTestId('feat-tag-btn')).toBeVisible(); - // feat-* carries the "AI assist" tag (Call 1 proposes feature tags); fig-* does not. - // Scope to the field label (getByText would also match the MultiSelect's inner label). - await expect(page.locator('label', { hasText: 'feat-* tags' })).toContainText('AI assist'); - await expect(page.locator('label', { hasText: 'fig-* tags' })).not.toContainText('AI assist'); - // Select one fig tag and one feat tag. - await page.getByTestId('fig-tag-btn').click(); - await page.getByTestId(`fig-tag-opt-${FIG_TAG_ID}`).click(); - await page.getByTestId('fig-tag-done').click(); - await expect(page.getByTestId('fig-tag-btn')).toContainText('1 selected'); - await page.getByTestId('feat-tag-btn').click(); - await page.getByTestId(`feat-tag-opt-${FEAT_TAG_ID}`).click(); - await page.getByTestId('feat-tag-done').click(); - await expect(page.getByTestId('feat-tag-btn')).toContainText('1 selected'); - // Flush the tag ids to the slab: an on:change field (grade) triggers - // commitReview, which PUTs fig_tag_ids/feat_tag_ids with the current values. - await page.getByTestId('grade').selectOption('37'); - // Tags are optional — they are not in the server ready-set, so readiness is - // unchanged (still "Not ready" because the other required fields are empty). - await expect(page.getByTestId('ready-status')).toContainText('Not ready'); - // The commit PUT folded the tag ids into the slab row (what Publish will send). - expect(slab.fig_tag_ids).toEqual([FIG_TAG_ID]); - expect(slab.feat_tag_ids).toEqual([FEAT_TAG_ID]); - }); + const shot = `tests/e2e/artifacts/review-90-${vp.name}-${vp.w}.png`; + await page.screenshot({ path: shot, fullPage: true }); + report.push(`${vp.name} ${vp.w}x${vp.h}: ${JSON.stringify(metrics)}`); + // Print before asserting so the measurement survives a failing gate. + console.log(`RENDER ${vp.name} ${vp.w}x${vp.h}: ${JSON.stringify(metrics)}`); - test('publish failure: duplicate SKU offers edit + open existing', async ({ page }) => { - await mockApi(page, { - onPublish: async (route) => { - await json(route, { - error: { code: 'duplicate_sku', message: 'SKU already exists', detail: {} }, - }, 409); - }, - }); - await reachReview(page); - await completeForReady(page); - await page.getByTestId('continue-publish').click(); - await expect(page.getByTestId('publish-screen')).toBeVisible(); - await page.getByTestId('publish-btn').click(); - await expect(page.getByTestId('dup-sku')).toBeVisible({ timeout: 15000 }); - await expect(page.getByTestId('edit-sku')).toBeVisible(); - await expect(page.getByTestId('open-existing')).toBeVisible(); - }); - - test('publish failure: recoverable Woo error offers retry', async ({ page }) => { - await mockApi(page, { - onPublish: async (route) => { - await json(route, { - error: { - code: 'woo_unreachable', - message: 'Store is unreachable', - detail: { recoverable: true, partial_media: false }, - }, - }, 502); - }, - }); - await reachReview(page); - await completeForReady(page); - await page.getByTestId('continue-publish').click(); - await expect(page.getByTestId('publish-screen')).toBeVisible(); - await page.getByTestId('publish-btn').click(); - await expect(page.getByTestId('store-failure')).toBeVisible({ timeout: 15000 }); - await expect(page.getByTestId('store-failure')).toContainText('Store is unreachable'); - await expect(page.getByTestId('retry-publish')).toBeVisible(); - }); - - test('publish failure: partial media is flagged for manual cleanup', async ({ page }) => { - await mockApi(page, { - onPublish: async (route) => { - await json(route, { - error: { - code: 'woo_partial_media', - message: 'Product created, photos failed', - detail: { recoverable: true, partial_media: true }, - }, - }, 502); - }, - }); - await reachReview(page); - await completeForReady(page); - await page.getByTestId('continue-publish').click(); - await expect(page.getByTestId('publish-screen')).toBeVisible(); - await page.getByTestId('publish-btn').click(); - await expect(page.getByTestId('store-failure')).toBeVisible({ timeout: 15000 }); - await expect(page.getByTestId('store-failure')).toContainText(/orphan media/i); + // Layout gates: nothing spills sideways, the closed summary names both picks + // (CSS ellipsis may shorten the paint, but the text must be there), and the + // primary rows stay a comfortable tap target on mobile. + expect(metrics.pageHasHScroll, `${vp.name} horizontal overflow: ${metrics.docScrollW} > ${metrics.docClientW}`).toBe(false); + expect(metrics.summary?.text).toMatch(/Live edge/i); + expect(metrics.summary?.text).toMatch(/Bowl blanks/i); + expect(metrics.count?.text).toContain('2 selected'); + expect(metrics.summary?.w ?? 0).toBeLessThanOrEqual(vp.w); + // Band is deliberately absent from Review (METADATA-15) — no element to measure. + expect(metrics.band).toBeNull(); + expect(metrics.primaryRowHeight ?? 0).toBeGreaterThanOrEqual(32); + } + console.log('RENDER REPORT\n' + report.join('\n')); }); }); diff --git a/frontend/tests/e2e/review-photo-strip.spec.ts b/frontend/tests/e2e/review-photo-strip.spec.ts new file mode 100644 index 0000000..85c508d --- /dev/null +++ b/frontend/tests/e2e/review-photo-strip.spec.ts @@ -0,0 +1,305 @@ +import { test, expect, type Page, type Route } from '@playwright/test'; +import { seedMeasureDraft } from './helpers/draft'; + +/** + * M3 Review photo strip E2E (issue #90, REVIEW-5 / PUBLISH-45). Drives the + * capture→review flow (mocked API, no live backend) and exercises the photo + * strip: render from stored photos, keep/remove (whole-card tap), drag-reorder, + * leftmost-kept = primary, the clean+overlay minimum, and persistLineup on + * "Continue to Publish". All ids are synthetic fixtures (QUALITY-105). + */ + +const SLAB_ID = '90e00000-0000-4000-8000-000000000090'; +const SPECIES_ID = 11; +const WCAT_ID = 20; +const LIVEEDGE_ID = 21; + +const TAXONOMY = { + categories: [ + { id: 10, name: 'Species', slug: 'species', parent_id: null }, + { id: SPECIES_ID, name: 'Cedar', slug: 'cedar', parent_id: 10 }, + { id: WCAT_ID, name: 'Wood category', slug: 'wcat', parent_id: null }, + { id: LIVEEDGE_ID, name: 'Live edge', slug: 'liveedge', parent_id: WCAT_ID }, + ], + attributes: [ + { id: 30, name: 'Edge Type', slug: 'edge_type', terms: [{ id: 31, name: 'Live edge', slug: 'live-edge' }] }, + ], + tags: [], + taxonomy_anchor: 'fixture', + taxonomy_last_sync_at: '2026-01-01T00:00:00', +}; + +// The stored lineup: clean cal (seq 1), dimension-overlay (seq 2), batch (seq 3). +const PHOTO_ROWS = [ + { id: 'ph-cal', seq: 1, is_calibration: 1, role: 'calibration', width_px: null, height_px: null, has_original: 1, has_processed: 1 }, + { id: 'ph-ov', seq: 2, is_calibration: 0, role: 'overlay', width_px: null, height_px: null, has_original: 1, has_processed: 1 }, + { id: 'ph-b1', seq: 3, is_calibration: 0, role: 'batch', width_px: null, height_px: null, has_original: 1, has_processed: 1 }, +]; + +function baseSlab(): Record { + return { + id: SLAB_ID, sku: '', status: 'draft', price: null, price_source: 'recommendation', + length_in: 48, thickness_in: 2, sqft: 6.25, bdft: 12.5, width_min_in: 10, width_max_in: 12, width_avg_in: 11, + species_id: null, wood_category_ids: [], edge_type_term_id: null, figure_term_ids: [], grade_term_id: null, + thickness_term_id: null, moisture_term_id: null, fig_tag_ids: [], feat_tag_ids: [], + title: null, short_description: null, description: null, content_source: 'manual', + species_confidence: null, figure_confidence: null, grade_confidence: null, + inference_status: null, calibration_photo_id: null, woo_product_id: null, server_rev: 0, + created_at: '2026-01-01T00:00:00Z', updated_at: '2026-01-01T00:00:00Z', + }; +} + +async function json(route: Route, body: unknown, status = 200) { + await route.fulfill({ status, contentType: 'application/json', body: JSON.stringify(body) }); +} + +interface PersistRecord { + seq: number; role: string; is_calibration: string; +} + +async function mockApi(page: Page) { + const slab = baseSlab(); + const photoStore = new Map(); + for (const r of PHOTO_ROWS) photoStore.set(r.id, { ...r }); + const persistCalls: { photoId: string; seq: number; role: string }[] = []; + const deletes: string[] = []; + + const merge = (payload: Record) => { + if (typeof payload.client_rev === 'number' && payload.client_rev !== slab.server_rev) return false; + for (const k of ['sku', 'species_id', 'wood_category_ids', 'title', 'short_description', 'price', 'edge_type_term_id']) { + if (k in payload) (slab as Record)[k] = payload[k]; + } + if (typeof payload.client_rev === 'number') slab.server_rev = payload.client_rev + 1; + }; + + const reviewResult = (payload: Record) => { + if (typeof payload.price === 'number') slab.price = payload.price; + if (typeof payload.sku === 'string' && payload.sku) slab.sku = payload.sku; + const missing: string[] = []; + if (!slab.sku) missing.push('sku'); + if (!slab.species_id) missing.push('species'); + if (!Array.isArray(slab.wood_category_ids) || slab.wood_category_ids.length === 0) missing.push('wood categories'); + if (!slab.edge_type_term_id) missing.push('edge type'); + if (!slab.title) missing.push('title'); + if (!slab.short_description) missing.push('short description'); + if (slab.price == null) missing.push('price'); + // Deterministic copy from the server on lock (REVIEW-15) — set before ready. + if (!slab.title) slab.title = 'Cedar · 48″ Live Edge'; + if (!slab.short_description) slab.short_description = 'Hand-cut slab, ready to ship.'; + return { + ready: missing.length === 0, missing, + title: (slab.title as string) ?? null, + short_description: (slab.short_description as string) ?? null, + price: (slab.price as number) ?? null, price_suggestion: 120, + primary_wood_category_id: null, + }; + }; + + await page.route('**/api/v1/**', async (route) => { + const req = route.request(); + const url = new URL(req.url()); + const path = url.pathname; + const method = req.method(); + const payload = (() => { try { return req.postDataJSON() as Record; } catch { return {}; } })(); + + if (path === '/api/v1/settings' && method === 'GET') { + return json(route, { woo_base_url: 'https://example.invalid', woo_wp_username: 'wp', woo_app_password: { configured: true }, inference_base_url: { configured: false }, inference_api_key: { configured: false }, inference_model: null, inference_enabled: false, content_llm_enabled: false, brand_voice: null, geo_context: null, woo_create_status: 'draft', taxonomy_last_sync_at: '2026-01-01T00:00:00', taxonomy_anchor: 'fixture' }); + } + if (path === '/api/v1/admin/taxonomy' && method === 'GET') return json(route, TAXONOMY); + if (path === '/api/v1/slabs' && method === 'POST') { + slab.id = String(payload.id ?? SLAB_ID); + merge(payload); + return json(route, slab, 201); + } + if (path === `/api/v1/slabs/${slab.id}` && method === 'PUT') { + if (merge(payload) === false) return json(route, { error: { code: 'revision_conflict', message: 'rev', detail: {} } }, 409); + return json(route, slab); + } + if (path === `/api/v1/slabs/${slab.id}` && method === 'GET') return json(route, slab); + + // --- Photos (the strip) --- + if (path === `/api/v1/slabs/${slab.id}/photos` && method === 'GET') { + return json(route, [...photoStore.values()].sort((a, b) => a.seq - b.seq)); + } + if (path === `/api/v1/slabs/${slab.id}/photos` && method === 'POST') { + // Multipart; pull the photo_id / seq / role form fields out of the body. + const raw = req.postData() ?? ''; + const field = (name: string): string => { + const re = new RegExp(`name="${name}"\\r\\n\\r\\n([\\s\\S]*?)(?=$|--)`); + const m = raw.match(re); + return m ? m[1].trim() : ''; + }; + const photoId = field('photo_id'); + const seq = Number(field('seq')); + const role = field('role') || 'batch'; + // Capture-time uploads (photo_id not in the fixture store, e.g. "photo-1") + // are accepted but not added to the lineup — the strip is driven by the + // stored rows only. Review-strip re-uploads reference a known fixture id. + if (!photoStore.has(photoId)) { + return json(route, { id: photoId, slab_id: slab.id, seq, is_calibration: 0, role, original_retained: true, processed_retained: true }, 201); + } + persistCalls.push({ photoId, seq, role }); + const existing = photoStore.get(photoId); + if (existing) { + existing.seq = seq; + existing.role = role as typeof existing.role; + } + return json(route, { id: photoId, slab_id: slab.id, seq, is_calibration: 0, role, original_retained: true, processed_retained: true }, 201); + } + const delMatch = path.match(new RegExp(`^/api/v1/slabs/${slab.id}/photos/([^/]+)$`)); + if (delMatch && method === 'DELETE') { + const id = delMatch[1]; + if (photoStore.delete(id)) { deletes.push(id); return route.fulfill({ status: 204, body: '' }); } + return json(route, { error: { code: 'not_found', message: 'Photo not found', detail: null } }, 404); + } + + if (path === `/api/v1/slabs/${slab.id}/review` && method === 'POST') return json(route, reviewResult(payload)); + if (path === `/api/v1/slabs/${slab.id}/publish` && method === 'POST') { + return json(route, { ok: true, status: 'draft', woo_product_id: 4242, draft: true }); + } + return json(route, { error: { code: 'not_found', message: path } }, 404); + }); + + return { persistCalls, deletes, photoStore }; +} + +async function reachReview(page: Page) { + await page.goto('/?noload'); + await seedMeasureDraft(page, { sku: '', lengthIn: 48, thicknessIn: 2, sqft: 6.25, bdft: 12.5, widthIn: 11 }); + await page.goto('/'); + await expect(page.getByRole('heading', { name: 'Resume?' })).toBeVisible({ timeout: 15000 }); + await page.getByRole('button', { name: 'Continue' }).click(); + await expect(page.getByTestId('continue-to-review')).toBeVisible({ timeout: 20000 }); + await page.getByTestId('continue-to-review').click(); + await expect(page.getByTestId('review-screen')).toBeVisible({ timeout: 20000 }); +} + +async function completeForReady(page: Page) { + await page.getByTestId('species').selectOption(String(SPECIES_ID)); + await page.getByTestId('wood-cat-btn').click(); + await page.getByTestId(`wood-cat-opt-${LIVEEDGE_ID}`).click(); + await page.getByTestId('wood-cat-done').click(); + await page.getByTestId('edge-type').selectOption('31'); + await page.getByTestId('price').fill('120'); + await expect(page.getByTestId('create-title')).toBeEnabled(); + await page.getByTestId('create-title').click(); + await expect(page.getByTestId('title')).not.toHaveValue(''); + await page.getByTestId('sku').fill('SLAB-0001'); + await page.getByTestId('sku').press('Tab'); +} + +test.describe('M3 Review photo strip (#90)', () => { + test('renders stored photos; leftmost kept = primary; tap keep/remove; drag reorder; minimum; persist', async ({ page }) => { + const mock = await mockApi(page); + await reachReview(page); + + // Strip renders the 3 stored photos in stored seq order. + const cards = page.getByTestId('photo-card'); + await expect(cards).toHaveCount(3); + // Leftmost (cal, seq 1) is the primary. + await expect(cards.nth(0)).toHaveAttribute('data-primary', 'true'); + await expect(page.getByText('PRIMARY', { exact: true })).toHaveCount(1); + + // Bug #1 regression: the primary photo's must be a valid URL + // (blob: for an in-memory capture blob), never empty or "object Blob". + const primaryImg = cards.nth(0).locator('img'); + await expect(primaryImg).not.toHaveAttribute('src', /object Blob|^$/); + const src = await primaryImg.getAttribute('src'); + expect(src && src.length > 4).toBeTruthy(); + + // Bug #2 regression: the card carries the gray checkerboard background + // (matching the Capture lineup), not a flat color. + const bgImage = await cards.nth(0).evaluate((el) => getComputedStyle(el).backgroundImage); + expect(bgImage).toContain('45deg'); + + // Mandatory photos: the clean calibration (seq 1) and the dimension-overlay + // copy (seq 2) are required — their keep checkbox is checked AND disabled, + // and a REQUIRED tag marks them. + await expect(cards.nth(0)).toHaveAttribute('data-required', 'true'); + await expect(cards.nth(1)).toHaveAttribute('data-required', 'true'); + await expect(cards.nth(2)).toHaveAttribute('data-required', ''); + await expect(cards.nth(0)).toHaveAttribute('aria-pressed', 'true'); + await expect(cards.nth(1)).toHaveAttribute('aria-pressed', 'true'); + await expect(cards.nth(0)).toHaveAttribute('aria-disabled', 'true'); + await expect(cards.nth(1)).toHaveAttribute('aria-disabled', 'true'); + await expect(cards.nth(2)).toHaveAttribute('aria-disabled', 'false'); + // Two REQUIRED tags (cal + overlay), none on the batch photo. + await expect(page.getByText('REQUIRED', { exact: true })).toHaveCount(2); + + // Tapping a mandatory photo is a no-op: it stays checked + primary (can't remove). + // The card is aria-disabled, so force the click to exercise the handler directly. + await cards.nth(0).click({ force: true }); + await expect(cards.nth(0)).toHaveAttribute('aria-pressed', 'true'); + await expect(cards.nth(0)).toHaveAttribute('data-primary', 'true'); + // No minimum nudge — both required photos are kept by construction. + await expect(page.getByTestId('lineup-nudge')).toHaveCount(0); + + // Remove the OPTIONAL batch photo (index 2) → it un-keeps; required stay kept. + await cards.nth(2).click(); + await expect(cards.nth(2)).toHaveAttribute('aria-pressed', 'false'); + // Primary is still the leftmost required (cal, index 0). + await expect(cards.nth(0)).toHaveAttribute('data-primary', 'true'); + // Re-add the batch photo. + await cards.nth(2).click(); + await expect(cards.nth(2)).toHaveAttribute('aria-pressed', 'true'); + await expect(page.getByTestId('lineup-nudge')).toHaveCount(0); + + // Drag the batch photo (index 2) to the front → it becomes the primary. + await cards.nth(2).scrollIntoViewIfNeeded(); + await page.waitForTimeout(120); + const first = await cards.nth(0).boundingBox(); + const third = await cards.nth(2).boundingBox(); + if (first && third) { + const sx = third.x + third.width / 2, sy = third.y + third.height / 2; + const dx = first.x + first.width / 2, dy = first.y + first.height / 2; + await page.mouse.move(sx, sy); + await page.mouse.down(); + // Several small steps so the HTML5 DnD picks up the drag. + for (let i = 1; i <= 14; i++) { + await page.mouse.move(sx + (dx - sx) * (i / 14), sy + (dy - sy) * (i / 14)); + } + await page.waitForTimeout(60); + await page.mouse.up(); + } + // After reorder the batch is leftmost = primary; seq badges renumber. + await expect(cards.nth(0)).toHaveAttribute('data-primary', 'true'); + + // Fill fields to ready, then Continue → persistLineup runs (re-seq kept, drop none). + await completeForReady(page); + await expect(page.getByTestId('ready-status')).toContainText('Ready to publish', { timeout: 15000 }); + await expect(page.getByTestId('continue-publish')).toBeEnabled(); + await page.getByTestId('continue-publish').click(); + await expect(page.getByTestId('publish-screen')).toBeVisible(); + + // Persisted: 3 re-uploads (re-seq 1..3), no deletes (nothing was removed in final state). + expect(mock.persistCalls.length).toBe(3); + const seqs = mock.persistCalls.map((c) => c.seq).sort((a, b) => a - b); + expect(seqs).toEqual([1, 2, 3]); + }); + + test('required photos guarantee the minimum; removing an OPTIONAL batch persists a DELETE', async ({ page }) => { + const mock = await mockApi(page); + await reachReview(page); + const cards = page.getByTestId('photo-card'); + + // The clean calibration + dimension-overlay are mandatory and always kept, + // so the submission minimum can never be broken — publish is never lineup-blocked. + await expect(page.getByTestId('lineup-nudge')).toHaveCount(0); + + // Removing an OPTIONAL batch photo is allowed and persists a DELETE. + await cards.nth(2).click(); + await expect(cards.nth(2)).toHaveAttribute('aria-pressed', 'false'); + // Still no nudge: both required photos remain kept. + await expect(page.getByTestId('lineup-nudge')).toHaveCount(0); + + // Complete the rest of the form; the lineup is valid (required photos kept). + await completeForReady(page); + await expect(page.getByTestId('ready-status')).toContainText('Ready to publish', { timeout: 15000 }); + await expect(page.getByTestId('continue-publish')).toBeEnabled(); + await page.getByTestId('continue-publish').click(); + await expect(page.getByTestId('publish-screen')).toBeVisible(); + + // The removed batch photo persisted as a DELETE; the required photos re-uploaded. + expect(mock.deletes).toEqual(['ph-b1']); + }); +});