diff --git a/frontend/css/app.css b/frontend/css/app.css index e793e78..8480aad 100644 --- a/frontend/css/app.css +++ b/frontend/css/app.css @@ -154,3 +154,18 @@ a.link:hover { text-decoration: underline; } overflow: auto; white-space: pre-wrap; } + +/* Albums: two-pane proposal review — folder list left, evidence/edit right. */ +.two-pane { display: grid; grid-template-columns: minmax(200px, 320px) 1fr; gap: 16px; align-items: start; } +.album-list { list-style: none; margin: 0; padding: 0; display: grid; gap: 4px; } +.album-list li { margin: 0; } +.album-list a { display: block; padding: 6px 8px; border-radius: 8px; border: 1px solid transparent; } +.album-list a[aria-current="true"] { border-color: var(--border); background: var(--surface-2); } +.badge.stale { color: var(--warn); border-color: var(--warn); } +.badge.approved { color: var(--ok); border-color: var(--ok); } +.badge.error { color: var(--danger); border-color: var(--danger); } + +/* Narrow screens: stack the panes so blockers and actions stay reachable. */ +@media (max-width: 720px) { + .two-pane { grid-template-columns: 1fr; } +} diff --git a/frontend/index.html b/frontend/index.html index 2f2f2a7..0cde89c 100644 --- a/frontend/index.html +++ b/frontend/index.html @@ -16,6 +16,7 @@ Safety Library Analyze + Albums Stats diff --git a/frontend/js/api.js b/frontend/js/api.js index e65f099..cadd2a6 100644 --- a/frontend/js/api.js +++ b/frontend/js/api.js @@ -79,4 +79,24 @@ export const api = { libraryAssets: (params = {}, opts = {}) => request("/library/assets?" + new URLSearchParams(params).toString(), opts), libraryStats: (opts = {}) => request("/library/stats", opts), + + // ── Albums: evidence and naming proposals ─────────────────────────────── + albumEvidence: (params = {}, opts = {}) => + request("/albums/evidence?" + new URLSearchParams(params).toString(), opts), + albumProposals: (params = {}, opts = {}) => + request("/albums/proposals?" + new URLSearchParams(params).toString(), opts), + generateProposals: (payload = {}, opts = {}) => + request("/albums/proposals", { method: "POST", body: JSON.stringify(payload), ...opts }), + editProposal: (album, payload, opts = {}) => + request(`/albums/proposals/${encodeURIComponent(album)}/edit`, { + method: "POST", + body: JSON.stringify(payload), + ...opts, + }), + approveProposal: (album, payload, opts = {}) => + request(`/albums/proposals/${encodeURIComponent(album)}/approve`, { + method: "POST", + body: JSON.stringify(payload), + ...opts, + }), }; diff --git a/frontend/js/app.js b/frontend/js/app.js index f999c57..0a2ea4e 100644 --- a/frontend/js/app.js +++ b/frontend/js/app.js @@ -1,6 +1,7 @@ import { api } from "./api.js"; import { navigate, onRouteChange, parseHash } from "./router.js"; import { + renderAlbums, renderAnalyze, renderLibrary, renderSafety, @@ -360,6 +361,7 @@ function render() { else if (path === "/safety") renderSafety(root, params); else if (path === "/library") renderLibrary(root, params); else if (path === "/analyze") renderAnalyze(root, params); + else if (path === "/albums") renderAlbums(root, params); else if (path === "/stats") renderStats(root, params); else show(errorBanner("Unknown view")); } diff --git a/frontend/js/views.js b/frontend/js/views.js index a7de115..9049b67 100644 --- a/frontend/js/views.js +++ b/frontend/js/views.js @@ -321,6 +321,227 @@ export async function renderStats(root) { ); } +// ── Albums: evidence + naming proposals ────────────────────────────────────── +// Two panes (concept §7): albums on the left with proposal state, the selected +// album's evidence, rationale, confidence, and editable final name on the right. +// Approving never renames anything — it records the approved name only. +export async function renderAlbums(root, params = {}) { + setActiveNav("albums"); + let evidence, proposals; + try { + [evidence, proposals] = await Promise.all([ + api.albumEvidence({ limit: 500 }), + api.albumProposals(), + ]); + } catch (error) { + root.replaceChildren(errorBanner(`Failed to load albums: ${error.message}`)); + return; + } + + const byAlbum = new Map(proposals.items.map((item) => [item.album, item])); + const folders = evidence.folders; + const selected = params.album || (folders[0] && folders[0].album) || null; + + const list = el( + "ul", + { class: "album-list", "data-testid": "album-list", "aria-label": "Albums" }, + ...folders.map((folder) => { + const proposal = byAlbum.get(folder.album); + const state = proposal ? (proposal.stale ? "stale" : proposal.status) : "none"; + return el( + "li", + {}, + el( + "a", + { + class: "link", + href: `#/albums?album=${encodeURIComponent(folder.album)}`, + "data-testid": "album-row", + "data-album": folder.album, + "aria-current": folder.album === selected ? "true" : false, + }, + folder.album, + " ", + el("span", { class: `badge ${state}`, "data-testid": "album-state" }, state), + " ", + el("span", { class: "muted" }, `${folder.asset_count} photos`) + ) + ); + }) + ); + + const detail = selected + ? albumDetail( + root, + folders.find((folder) => folder.album === selected), + byAlbum.get(selected), + params + ) + : el("p", { class: "muted" }, "No albums with evidence yet."); + + root.replaceChildren( + el("h1", {}, "Albums"), + el( + "div", + { class: "toolbar" }, + el( + "button", + { + class: "primary", + "data-testid": "generate-proposals", + onclick: async () => { + try { + await api.generateProposals({}); + render(); + } catch (error) { + alert(`Could not generate proposals: ${error.message}`); + } + }, + }, + "Generate proposals" + ) + ), + el("div", { class: "two-pane" }, el("div", { class: "pane-left" }, list), el("div", { class: "pane-right" }, detail)) + ); +} + +function albumDetail(root, folder, proposal, params) { + if (!folder) return el("p", { class: "muted" }, "Album not found."); + + const evidenceBlock = el( + "div", + { "data-testid": "album-evidence" }, + el("h2", {}, "Evidence"), + el( + "dl", + {}, + el("dt", {}, "Source album"), + el("dd", { "data-testid": "source-album" }, folder.album), + el("dt", {}, "Photos affected"), + el("dd", { "data-testid": "affected-count" }, String(folder.asset_count)), + el("dt", {}, "Analyzed"), + el("dd", {}, `${folder.analyzed_count} (${folder.nsfw_count} NSFW, ${folder.pending_count} pending)`), + el("dt", {}, "Year"), + el( + "dd", + {}, + folder.dominant_year != null + ? String(folder.dominant_year) + : folder.year_conflict + ? "conflicting" + : "unknown" + ), + el("dt", {}, "Locations"), + el("dd", {}, folder.locations.map((l) => l.value).join(", ") || "—") + ), + facetBlock("Top tags", folder.tags.slice(0, 12)) + ); + + if (!proposal) { + return el( + "div", + {}, + el("h2", {}, folder.album), + el("p", { class: "muted", "data-testid": "no-proposal" }, "No proposal yet — generate one."), + evidenceBlock + ); + } + + const issues = el("div", { class: "alert", role: "alert", "data-testid": "name-issues", hidden: "hidden" }); + const input = el("input", { + type: "text", + id: "final-name", + value: proposal.name || "", + "aria-label": "Final album name", + "data-testid": "final-name", + // Prompt validation: mirror the server's naming policy so a bad name is + // flagged as it is typed, before any request is sent. + oninput: () => showIssues(issues, input.value), + }); + + const save = el( + "button", + { + "data-testid": "save-name", + onclick: () => mutate(root, () => api.editProposal(proposal.album, { name: input.value, expected_version: proposal.version }), params), + }, + "Save name" + ); + + const approve = el( + "button", + { + class: "primary", + "data-testid": "approve", + disabled: proposal.stale || proposal.status === "error" ? "disabled" : false, + title: proposal.stale ? "evidence changed — regenerate first" : false, + onclick: () => mutate(root, () => api.approveProposal(proposal.album, { expected_version: proposal.version }), params), + }, + proposal.status === "approved" ? "Approved" : "Approve name" + ); + + const nodes = [ + el("h2", {}, folder.album), + el( + "div", + { class: "decision-bar" }, + el("span", { class: `badge ${proposal.status}`, "data-testid": "proposal-status" }, proposal.status), + proposal.confidence != null + ? el("span", { class: "badge", "data-testid": "confidence" }, `confidence ${proposal.confidence}`) + : null, + proposal.stale ? el("span", { class: "badge attention", "data-testid": "stale" }, "stale — regenerate") : null + ), + ]; + if (params.conflict) { + nodes.push( + el( + "div", + { class: "alert", role: "alert", "data-testid": "conflict" }, + "This proposal changed since you loaded it. Nothing was modified; showing the latest state." + ) + ); + } + if (proposal.status === "error") { + nodes.push( + el("div", { class: "alert", role: "alert", "data-testid": "proposal-error" }, `Generation failed: ${proposal.error_code}`) + ); + } + nodes.push( + el("p", { class: "muted", "data-testid": "suggested-name" }, `Suggested: ${proposal.proposed_name || "—"}`), + el("p", { "data-testid": "rationale" }, proposal.rationale || "No rationale given."), + el("div", { class: "toolbar" }, el("label", { for: "final-name" }, "Final name "), input, save, approve), + issues, + evidenceBlock + ); + return el("div", {}, ...nodes); +} + +// Mirrors photo_pipeline/services/naming.py so typing feedback matches the server. +const FORBIDDEN_RE = /[/\\:*?"<>|]/; + +function showIssues(node, value) { + const problems = []; + if (FORBIDDEN_RE.test(value)) problems.push('a name cannot contain / \\ : * ? " < > |'); + if (!value.replace(/[\s.]/g, "")) problems.push("a name cannot be empty"); + node.textContent = problems.join("; "); + node.hidden = problems.length === 0; +} + +async function mutate(root, action, params) { + try { + await action(); + render(); + } catch (error) { + if (error.status === 409) { + // Stale browser state: show the conflict and the server's latest truth. + navigate("/albums", { ...params, conflict: "1" }); + render(); + } else { + alert(`Failed: ${error.message}`); + } + } +} + // ── shared bits ─────────────────────────────────────────────────────────────── function stat(label, value) { return el("div", { class: "stat card" }, el("div", { class: "stat-value" }, String(value ?? 0)), el("div", { class: "muted" }, label)); diff --git a/tests/e2e/test_albums_ui.py b/tests/e2e/test_albums_ui.py new file mode 100644 index 0000000..62b4fc2 --- /dev/null +++ b/tests/e2e/test_albums_ui.py @@ -0,0 +1,222 @@ +"""Browser journeys for the Albums proposal view (US03-04). + +Covers evidence display, editing with prompt validation, collision/invalid-name +guidance, explicit approval, visible stale-conflict handling, keyboard operation, +and the safety property that approving a name renames nothing on disk. +""" + +from __future__ import annotations + +import uuid +from datetime import datetime, timezone + +import httpx +import pytest +from playwright.sync_api import expect + +from tests.e2e._pipeline_harness import Server, image, seed_library + +NOW = datetime(2026, 1, 1, tzinfo=timezone.utc) +TIMEOUT = 10 + + +def _seed(tmp_path): + """A library with one album ("rome") whose two photos are SFW and analysed.""" + seeded = seed_library(tmp_path, {}, {}) # empty scan; add the album below + album = seeded.lib / "rome" + album.mkdir() + image(album / "a.jpg", 1) + image(album / "b.jpg", 2) + + from sqlalchemy import select + + from photo_pipeline.config import Config + from photo_pipeline.db import create_db_engine, create_session_factory, run_migrations + from photo_pipeline.models import AnalysisResult, Asset, SafetyReview + from photo_pipeline.services.inventory import InventoryService + + config = Config.from_env( + { + "PHOTO_PIPELINE_DATA_DIR": str(seeded.data), + "PHOTO_PIPELINE_LIBRARY_ROOTS": str(seeded.lib), + } + ) + run_migrations(config.database_url) + engine = create_db_engine(config.database_url) + sf = create_session_factory(engine) + InventoryService(sf).scan(seeded.lib) + with sf() as session: + rows = list(session.execute(select(Asset.id, Asset.current_path)).all()) + for asset_id, path in rows: + session.add( + SafetyReview( + id=str(uuid.uuid4()), asset_id=asset_id, decision="sfw", created_at=NOW + ) + ) + session.add( + AnalysisResult( + asset_id=asset_id, + status="analyzed", + description=f"a view of {path}", + tags='["ruins", "city"]', + approx_year=2019, + location_hint="Rome", + ) + ) + session.commit() + engine.dispose() + seeded.asset_ids.update({p.rsplit("/", 1)[-1]: a for a, p in rows}) + return seeded + + +@pytest.fixture +def server(tmp_path): + seeded = _seed(tmp_path) + running = Server(seeded).start() + running.seeded = seeded + try: + yield running + finally: + running.stop() + + +def _generate(base): + httpx.post(f"{base}/api/v1/albums/proposals", json={}, timeout=TIMEOUT).raise_for_status() + + +def test_view_shows_album_evidence_rationale_and_affected_count(page, server): + errors = [] + page.on("console", lambda m: errors.append(m.text) if m.type == "error" else None) + _generate(server.base) + + page.goto(f"{server.base}/app/#/albums") + page.get_by_test_id("album-row").first.wait_for() + + expect(page.get_by_test_id("source-album")).to_have_text("rome") + expect(page.get_by_test_id("affected-count")).to_have_text("2") + # Suggested name, rationale and confidence are all visible before approving. + assert "2019" in page.get_by_test_id("suggested-name").inner_text() + assert page.get_by_test_id("rationale").inner_text().strip() != "" + expect(page.get_by_test_id("proposal-status")).to_have_text("proposed") + assert errors == [], f"console errors: {errors}" + + +def test_album_without_a_proposal_is_explained(page, server): + page.goto(f"{server.base}/app/#/albums") + expect(page.get_by_test_id("no-proposal")).to_be_visible() + + +def test_edit_persists_across_reload(page, server): + _generate(server.base) + page.goto(f"{server.base}/app/#/albums") + + field = page.get_by_test_id("final-name") + field.wait_for() + field.fill("2019 Rome Holiday") + page.get_by_test_id("save-name").click() + expect(page.get_by_test_id("proposal-status")).to_have_text("edited") + + page.reload() + expect(page.get_by_test_id("final-name")).to_have_value("2019 Rome Holiday") + + +def test_invalid_name_is_flagged_promptly_while_typing(page, server): + _generate(server.base) + page.goto(f"{server.base}/app/#/albums") + + field = page.get_by_test_id("final-name") + field.wait_for() + field.fill("2019/Rome") + # Validation is immediate — no request needed to learn the name is unusable. + issues = page.get_by_test_id("name-issues") + expect(issues).to_be_visible() + assert "cannot contain" in issues.inner_text() + + field.fill("2019 Rome") + expect(issues).to_be_hidden() + + +def test_approval_is_explicit_and_changes_no_file_paths(page, server): + _generate(server.base) + before = { + row["current_path"] + for row in httpx.get(f"{server.base}/api/v1/inventory/assets", timeout=TIMEOUT).json()[ + "items" + ] + } + + page.goto(f"{server.base}/app/#/albums") + approve = page.get_by_test_id("approve") + approve.wait_for() + expect(page.get_by_test_id("proposal-status")).to_have_text("proposed") + + approve.click() # approval only happens on this explicit action + expect(page.get_by_test_id("proposal-status")).to_have_text("approved") + + after = { + row["current_path"] + for row in httpx.get(f"{server.base}/api/v1/inventory/assets", timeout=TIMEOUT).json()[ + "items" + ] + } + assert after == before, "approving a proposal must not rename anything" + + page.reload() + expect(page.get_by_test_id("proposal-status")).to_have_text("approved") + + +def test_stale_evidence_blocks_approval_visibly(page, server): + _generate(server.base) + # Change the album's evidence behind the UI's back: a new analysed photo. + image(server.seeded.lib / "rome" / "c.jpg", 3) + httpx.post(f"{server.base}/api/v1/inventory/scan", timeout=TIMEOUT).raise_for_status() + + page.goto(f"{server.base}/app/#/albums") + page.get_by_test_id("album-row").first.wait_for() + expect(page.get_by_test_id("stale")).to_be_visible() + expect(page.get_by_test_id("approve")).to_be_disabled() + + +def test_version_conflict_is_reported_without_mutating(page, server): + _generate(server.base) + page.goto(f"{server.base}/app/#/albums") + field = page.get_by_test_id("final-name") + field.wait_for() + + # Another client edits first, so the page's expected_version goes stale. + current = httpx.get(f"{server.base}/api/v1/albums/proposals/rome", timeout=TIMEOUT).json() + httpx.post( + f"{server.base}/api/v1/albums/proposals/rome/edit", + json={"name": "Someone Else", "expected_version": current["version"]}, + timeout=TIMEOUT, + ).raise_for_status() + + field.fill("My Name") + page.get_by_test_id("save-name").click() + + expect(page.get_by_test_id("conflict")).to_be_visible() + # The other client's value survived; this page's edit was not applied. + expect(page.get_by_test_id("final-name")).to_have_value("Someone Else") + + +def test_album_list_and_editing_work_from_the_keyboard(page, server): + _generate(server.base) + page.goto(f"{server.base}/app/#/albums") + page.get_by_test_id("album-row").first.wait_for() + + # Follow the album link with the keyboard — no mouse involved. Selecting it + # re-renders, so wait for the selection to be reflected before typing; + # otherwise focus lands on a node the re-render is about to replace. + page.get_by_test_id("album-row").first.focus() + page.keyboard.press("Enter") + expect(page.get_by_test_id("album-row").first).to_have_attribute("aria-current", "true") + expect(page.get_by_test_id("final-name")).to_be_visible() + + field = page.get_by_test_id("final-name") + field.focus() + page.keyboard.press("ControlOrMeta+a") + page.keyboard.type("Keyboard Named Album") + page.keyboard.press("Tab") + page.keyboard.press("Enter") # focus is now the Save button + expect(page.get_by_test_id("proposal-status")).to_have_text("edited") + expect(page.get_by_test_id("final-name")).to_have_value("Keyboard Named Album") diff --git a/tests/story_traceability.json b/tests/story_traceability.json index 5bfe5b3..a98fb0d 100644 --- a/tests/story_traceability.json +++ b/tests/story_traceability.json @@ -78,6 +78,9 @@ ], "US03-03": [ "tests/integration/test_album_proposals.py" + ], + "US03-04": [ + "tests/e2e/test_albums_ui.py" ] } }