diff --git a/backend/db/__init__.py b/backend/db/__init__.py index 1d43c39..ff39271 100644 --- a/backend/db/__init__.py +++ b/backend/db/__init__.py @@ -80,6 +80,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() @@ -99,6 +114,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 1c4b6ea..c0fefd3 100644 --- a/backend/db/schema.sql +++ b/backend/db/schema.sql @@ -57,6 +57,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/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..f9e1638 100644 --- a/backend/tests/test_publish.py +++ b/backend/tests/test_publish.py @@ -119,6 +119,53 @@ 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_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/frontend/src/api/slabs.ts b/frontend/src/api/slabs.ts index 757da0d..0e5a7ed 100644 --- a/frontend/src/api/slabs.ts +++ b/frontend/src/api/slabs.ts @@ -159,6 +159,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; @@ -169,6 +171,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; } @@ -178,6 +182,7 @@ export interface UploadPhotoResult { slab_id: string; seq: number; is_calibration: number; + role: 'calibration' | 'overlay' | 'batch'; original_retained: boolean; processed_retained: boolean; } @@ -193,6 +198,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/Review.svelte b/frontend/src/create/review/Review.svelte index 4e38e3a..9911fa1 100644 --- a/frontend/src/create/review/Review.svelte +++ b/frontend/src/create/review/Review.svelte @@ -2,11 +2,15 @@ import { onMount, onDestroy } from 'svelte'; import { getSlab, updateSlab, reviewSlab, lockSlab, getTaxonomy, getSettings, inferTaxon, inferContent, - ApiError, - type Slab, type Taxonomy, type TaxCategory, type TaxTag, type SettingsView, + listPhotos, persistLineup, photoUrl, ApiError, + type Slab, type Taxonomy, type TaxCategory, type TaxTag, type SettingsView, type PhotoRow, } from '../../api/slabs'; import type { CalibratedCapture } from '../flow'; import MultiSelect from './MultiSelect.svelte'; + import { + keptInOrder, moveItem, missingMinimums, roleLabel, + type LineupItem, + } from './lineup'; // Props handed off by App (one-column flow, UX.md §1). export let slabId: string; @@ -132,6 +136,143 @@ const fmt1 = (v: number) => { const s = v.toFixed(1); return s.endsWith('.0') ? s.slice(0, -2) : s; }; const fmt2 = (v: number) => v.toFixed(2); + // --- #90 Photo strip (REVIEW-5 / PUBLISH-45) --- + // The owner edits which photos stay/go and their order. Leftmost kept photo = + // primary (Woo cover). Kept in-memory; persisted via persistLineup() on + // "Continue to Publish". Sources are the STORED photos (listPhotos) so the + // strip survives re-entry into Review; in-memory lineup blobs (from capture) + // are preferred when present. + interface StripPhoto { + id: string; + role: 'calibration' | 'overlay' | 'batch'; + isCalibration: boolean; + kept: boolean; + original: Blob; + processed: Blob | null; + url: string; + } + let photos: StripPhoto[] = []; + let photosLoaded = false; + let lineupDirty = false; + let persistBusy = false; + let persistError = ''; + + // In-memory blobs from the capture lineup, keyed by lineup order. The stored + // photo id matches the capture's upload order only on the first entry; after + // an edit the STORED row (listPhotos) is authoritative and we re-fetch blobs. + const inMemByRole = (): Map<'calibration' | 'overlay' | 'batch', Blob[]> => { + const m = new Map<'calibration' | 'overlay' | 'batch', Blob[]>(); + for (const it of capture?.lineup ?? []) { + if (it.kind !== 'png') continue; + const arr = m.get(it.role) ?? []; + arr.push(it.blob); + m.set(it.role, arr); + } + return m; + }; + + async function loadPhotos() { + try { + const rows: PhotoRow[] = await listPhotos(slabId); + const byRole = inMemByRole(); + const seqByRole: Record = { calibration: 0, overlay: 0, batch: 0 }; + photos = rows.map((r) => { + const role = r.role ?? 'batch'; + const n = seqByRole[role]++; + const blob = (byRole.get(role)?.[n]) ?? null; + const url = + blob ?? + (photoUrl(slabId, r.id, (r.has_processed ? 'processed' : 'original') as 'original' | 'processed')); + return { + id: r.id, + role, + isCalibration: r.is_calibration === 1, + kept: true, + original: blob ?? new Blob(), + processed: blob ?? null, + url: String(url), + }; + }); + photosLoaded = true; + lineupDirty = false; + } catch { + photos = []; + photosLoaded = false; + } + } + + // Whole-card tap = keep/remove toggle. A real drag (>6px) reorders and does NOT + // toggle keep/remove — the threshold lives in the pointer handlers below. + function toggleKeep(id: string) { + photos = photos.map((p) => (p.id === id ? { ...p, kept: !p.kept } : p)); + lineupDirty = true; + } + + function reorder(from: number, to: number) { + photos = moveItem(photos, from, to); + lineupDirty = true; + } + + let dragFrom = -1; + + function onDragStart(e: DragEvent, index: number) { + dragFrom = index; + if (e.dataTransfer) { + e.dataTransfer.effectAllowed = 'move'; + e.dataTransfer.setData('text/plain', String(index)); + } + } + + function onDragOver(e: DragEvent) { + if (e.preventDefault) e.preventDefault(); + } + + function onDrop(e: DragEvent, index: number) { + if (e.preventDefault) e.preventDefault(); + if (dragFrom >= 0 && dragFrom !== index) reorder(dragFrom, index); + dragFrom = -1; + } + + // REVIEW-5: the kept set must contain a clean photo and its overlay copy. Only + // enforced once photos have loaded (photosLoaded); a failed/absent photo list + // (e.g. legacy mock) leaves the lineup minimum neutral rather than blocking. + let lineupMissing: ('calibration' | 'overlay')[] = []; + $: lineupMissing = photosLoaded + ? missingMinimums(keptInOrder(photos as LineupItem[]) as LineupItem[]) + : []; + $: lineupValid = lineupMissing.length === 0; + // Leftmost kept photo id = primary (Woo cover). '' when nothing is kept. + let primaryId = ''; + $: primaryId = photosLoaded + ? ((keptInOrder(photos as LineupItem[]) as StripPhoto[])[0]?.id ?? '') + : ''; + // Persist the edited lineup (re-seq kept + drop removed), then hand to Publish. + async function goPublish() { + if (!lineupValid) return; + persistBusy = true; + persistError = ''; + try { + const kept = keptInOrder(photos as LineupItem[]) as StripPhoto[]; + const removed = photos.filter((p) => !p.kept).map((p) => p.id); + await persistLineup( + slabId, + kept.map((p) => ({ + id: p.id, + role: p.role, + isCalibration: p.isCalibration, + original: p.original, + processed: p.processed, + })), + removed, + ); + onPublish(); + } catch (e) { + persistError = e instanceof ApiError ? e.message : 'Could not save the photos. Try again.'; + } finally { + persistBusy = false; + } + } + function hydrate(s: Slab, t: Taxonomy | null) { speciesId = s.species_id ?? ''; woodCatIds = [...(s.wood_category_ids ?? [])]; @@ -323,6 +464,7 @@ deriveOptions(); hydrate(s, t); canGenerateContent = !!set.content_llm_enabled; + await loadPhotos(); await commitReview(); } catch (e) { loadError = e instanceof ApiError ? e.message : 'Could not load the slab.'; @@ -331,17 +473,9 @@ } }); - function goPublish() { - onPublish(); - } - function goBack() { onBack(); } - - function publish() { - goPublish(); - } {#if loading} @@ -365,6 +499,47 @@
Sqft / Bdft
{fmt2(capture?.sqft ?? slab?.sqft ?? 0)} / {fmt2(capture?.bdft ?? slab?.bdft ?? 0)}
+ + {#if photosLoaded && photos.length} +
+

Tap a photo to keep or remove it · drag to reorder · leftmost photo = primary

+
+ {#each photos as p, i (p.id)} +
toggleKeep(p.id)} + on:keydown={(e) => { if (e.key === 'Enter' || e.key === ' ') { e.preventDefault(); toggleKeep(p.id); } }} + on:dragstart={(e) => onDragStart(e, i)} + on:dragover={onDragOver} + on:drop={(e) => onDrop(e, i)} + > + {roleLabel(p.role)} + {#if p.id === primaryId} + PRIMARY + {/if} + + +
+ {/each} +
+ {#if lineupMissing.length} + + {/if} + {#if persistError}{/if} +
+ {/if} + {#if settings?.inference_enabled}
@@ -594,4 +772,23 @@ .primary:disabled { background: #333; color: #777; } .ghost { padding: 14px 18px; border-radius: 3px; border: 1px solid #444; background: transparent; color: #ccc; } .err { color: #e88; } .busy { color: #aaa; } + + /* #90 Photo strip */ + .photos { margin: 14px 0 4px; } + .ph-hint { color: #8a8a8a; font-size: .72rem; margin: 0 0 8px; } + .ph-grid { display: grid; grid-template-columns: repeat(3, 1fr); gap: 8px; } + @media (max-width: 420px) { .ph-grid { grid-template-columns: repeat(2, 1fr); } } + .pcard { position: relative; aspect-ratio: 1; border: 2px solid #2a2a2a; border-radius: 3px; overflow: hidden; cursor: grab; background: #141414; } + .pcard:active { cursor: grabbing; } + .pcard:focus-visible { outline: 2px solid #00e5ff; outline-offset: 2px; } + .pcard.kept { border-color: #444; } + .pcard.primary { border-color: #00e5ff; } + .pcard:not(.kept) { opacity: .4; } + .ph-thumb { width: 100%; height: 100%; object-fit: cover; display: block; pointer-events: none; } + .ph-pri { position: absolute; top: 6px; right: 6px; background: #00e5ff; color: #002; font-size: .56rem; font-weight: 800; padding: 1px 5px; border-radius: 2px; letter-spacing: .03em; pointer-events: none; } + .ph-seq { position: absolute; bottom: 6px; left: 6px; width: 18px; height: 18px; border-radius: 50%; background: rgba(0,0,0,.7); color: #fff; font-size: .66rem; display: flex; align-items: center; justify-content: center; pointer-events: none; } + .ph-keep { position: absolute; top: 6px; left: 6px; width: 16px; height: 16px; border: 2px solid #fff; border-radius: 3px; background: rgba(0,0,0,.5); pointer-events: none; } + .pcard.kept .ph-keep { background: #00e5ff; border-color: #00e5ff; } + .pcard.kept .ph-keep::after { content: ''; position: absolute; left: 4px; top: 1px; width: 4px; height: 8px; border: solid #002; border-width: 0 2px 2px 0; transform: rotate(45deg); } + .ph-nudge { color: #e8a; font-size: .74rem; margin: 8px 0 0; } diff --git a/frontend/src/create/review/lineup.test.ts b/frontend/src/create/review/lineup.test.ts new file mode 100644 index 0000000..7899d6b --- /dev/null +++ b/frontend/src/create/review/lineup.test.ts @@ -0,0 +1,71 @@ +import assert from 'node:assert/strict'; +import { test } from 'node:test'; +import { + keptInOrder, + moveItem, + missingMinimums, + lineupValid, + roleLabel, + 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/); +}); diff --git a/frontend/src/create/review/lineup.ts b/frontend/src/create/review/lineup.ts new file mode 100644 index 0000000..b4fd348 --- /dev/null +++ b/frontend/src/create/review/lineup.ts @@ -0,0 +1,62 @@ +// #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(kept: LineupItem[]): PhotoRole[] { + 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'; +} 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..6b8b71a --- /dev/null +++ b/frontend/tests/e2e/review-photo-strip.spec.ts @@ -0,0 +1,266 @@ +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', + metadata_locked: false, 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', 'metadata_locked', '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, locked: slab.metadata_locked === true, + }; + }; + + 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('lock-metadata')).toBeEnabled(); + await page.getByTestId('lock-metadata').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); + + // Remove the primary (cal) → primary auto-promotes to the next leftmost (ov). + await cards.nth(0).click(); + await expect(cards.nth(0)).toHaveAttribute('aria-pressed', 'false'); + await expect(cards.nth(1)).toHaveAttribute('data-primary', 'true'); + // Minimum nudge: cal + batch kept, but overlay still present → only cal missing. + await expect(page.getByTestId('lineup-nudge')).toBeVisible(); + await expect(page.getByTestId('lineup-nudge')).toContainText('clean photo'); + + // Re-add the cal (the original re-check bug) → it's back to the left = primary. + await cards.nth(0).click(); + await expect(cards.nth(0)).toHaveAttribute('aria-pressed', 'true'); + await expect(cards.nth(0)).toHaveAttribute('data-primary', '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('removing a photo persists a DELETE and blocks publish when the minimum is broken', async ({ page }) => { + const mock = await mockApi(page); + await reachReview(page); + const cards = page.getByTestId('photo-card'); + + // Remove the overlay (seq 2) → minimum broken (no overlay). + await cards.nth(1).click(); + await expect(cards.nth(1)).toHaveAttribute('aria-pressed', 'false'); + await expect(page.getByTestId('lineup-nudge')).toContainText('overlay'); + + // Complete the rest of the form; the slab fields are ready but the lineup is not. + await completeForReady(page); + await expect(page.getByTestId('ready-status')).toContainText(/Not ready|remaining/); + // Continue is blocked by the lineup minimum even though the fields are set. + await expect(page.getByTestId('continue-publish')).toBeDisabled(); + }); +});