diff --git a/frontend/index.html b/frontend/index.html index 47c23bb..f32e6dd 100644 --- a/frontend/index.html +++ b/frontend/index.html @@ -18,6 +18,7 @@ Analyze Albums Renames + Upload Stats diff --git a/frontend/js/api.js b/frontend/js/api.js index edaa1f0..d75d0ce 100644 --- a/frontend/js/api.js +++ b/frontend/js/api.js @@ -114,4 +114,28 @@ export const api = { request(`/rename-plans/${encodeURIComponent(id)}/rollback`, { method: "POST", ...opts }), renameRecovery: (opts = {}) => request("/rename-recovery", opts), resolveRecovery: (opts = {}) => request("/rename-recovery/resolve", { method: "POST", ...opts }), + + // ── Uploads: preflight, batches, verification ─────────────────────────── + // The API key never travels through here: preflight reports only whether one is + // configured, and every command preview arrives already redacted. + uploadPreflight: (payload = {}, opts = {}) => + request("/upload-preflight", { method: "POST", body: JSON.stringify(payload), ...opts }), + createUploadBatches: (payload, opts = {}) => + request("/upload-batches", { method: "POST", body: JSON.stringify(payload), ...opts }), + listUploadBatches: (opts = {}) => request("/upload-batches", opts), + getUploadBatch: (id, opts = {}) => request(`/upload-batches/${encodeURIComponent(id)}`, opts), + startUploadBatch: (id, opts = {}) => + request(`/upload-batches/${encodeURIComponent(id)}/start`, { method: "POST", ...opts }), + cancelUploadBatch: (id, opts = {}) => + request(`/upload-batches/${encodeURIComponent(id)}/cancel`, { method: "POST", ...opts }), + verifyUploadBatch: (id, opts = {}) => + request(`/upload-batches/${encodeURIComponent(id)}/verify`, { method: "POST", ...opts }), + resolveUploadItem: (id, payload, opts = {}) => + request(`/upload-batches/${encodeURIComponent(id)}/resolve`, { + method: "POST", + body: JSON.stringify(payload), + ...opts, + }), + uploadVerifications: (id, opts = {}) => + request(`/upload-batches/${encodeURIComponent(id)}/verifications`, opts), }; diff --git a/frontend/js/app.js b/frontend/js/app.js index 0d11180..a163fc2 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 { renderRenames, setRenamesRender } from "./renames.js"; +import { renderUploads, setUploadsRender } from "./uploads.js"; import { renderAlbums, renderAnalyze, @@ -364,6 +365,7 @@ function render() { else if (path === "/analyze") renderAnalyze(root, params); else if (path === "/albums") renderAlbums(root, params); else if (path === "/renames") renderRenames(root, params); + else if (path === "/uploads") renderUploads(root, params); else if (path === "/stats") renderStats(root, params); else show(errorBanner("Unknown view")); } @@ -371,5 +373,6 @@ function render() { // Let views re-render the current route after a mutation. setRender(render); setRenamesRender(render); +setUploadsRender(render); onRouteChange(render); render(); diff --git a/frontend/js/uploads.js b/frontend/js/uploads.js new file mode 100644 index 0000000..44636e7 --- /dev/null +++ b/frontend/js/uploads.js @@ -0,0 +1,704 @@ +// Upload view (US05-05): preflight a scope, confirm exactly what will be sent, +// watch the batch run, and resolve whatever the uploader left uncertain. +// +// Two rules shape this file. First, nothing here decides what is safe: blockers, +// the preflight token, and the retry policy all come from the server, and an action +// the server would refuse is not offered at all. Second, the API key never reaches +// the browser — preflight reports only that one is configured, and every command +// preview arrives redacted — so nothing in this view may reconstruct, store, or +// route a secret. +import { api } from "./api.js"; +import { el, errorBanner, setActiveNav } from "./dom.js"; +import { subscribeJob } from "./events.js"; +import { navigate } from "./router.js"; + +// Result of the last command issued from this tab, and the activity of the last +// upload job. Deliberately not persisted: after a reload the page must show what +// the server says happened, not what this page remembers. +let outcome = null; +let activity = []; + +let render = () => {}; +export function setUploadsRender(fn) { + render = fn; +} + +// Report outcomes (services/upload_reports.py) in the words the operator uses. +const OUTCOME_LABEL = [ + ["uploaded", "new"], + ["upgraded", "upgraded"], + ["duplicate", "duplicate"], + ["skipped", "skipped"], + ["failed", "failed"], + ["unknown", "uncertain"], +]; + +export async function renderUploads(root, params = {}) { + setActiveNav("uploads"); + const albums = parseAlbums(params.albums); + const allowPartial = params.partial === "1"; + + let preflight, listed; + try { + [preflight, listed] = await Promise.all([ + api.uploadPreflight({ albums, allow_partial: allowPartial }), + api.listUploadBatches(), + ]); + } catch (error) { + root.replaceChildren(errorBanner(`Failed to load uploads: ${error.message}`)); + return; + } + + const batches = listed.batches; + const selectedId = params.batch || (batches.length ? batches[batches.length - 1].id : null); + let batch = null; + let history = []; + if (selectedId) { + try { + const [detail, verifications] = await Promise.all([ + api.getUploadBatch(selectedId), + api.uploadVerifications(selectedId), + ]); + batch = detail; + history = verifications.verifications; + } catch (error) { + root.replaceChildren(errorBanner(`Failed to load upload batch: ${error.message}`)); + return; + } + } + + root.replaceChildren( + ...[ + el("h1", {}, "Upload"), + configurationCard(preflight), + preflightBlockers(preflight), + scopeSection(preflight, params, albums, allowPartial), + confirmBlock(preflight, params, allowPartial), + outcomeBanner(), + activityLog(), + batchList(batches, selectedId), + batch ? batchDetail(batch, history) : null, + ].filter(Boolean) + ); +} + +// Album names can contain commas, so each one is escaped before the URL joins them. +function parseAlbums(value) { + if (!value) return null; + const names = value.split(",").filter(Boolean).map(decodeURIComponent); + return names.length ? names : null; +} + +function albumsParam(names) { + return names.map(encodeURIComponent).join(","); +} + +// ── configuration ──────────────────────────────────────────────────────────── +// What the upload is aimed at, in the only form the browser is ever given: the +// server URL, whether a key exists, and the uploader's version. +function configurationCard(preflight) { + const credentials = preflight.credentials; + const uploader = preflight.uploader; + return el( + "div", + { class: "card", "data-testid": "upload-config" }, + el("h2", {}, "Configuration"), + el( + "dl", + {}, + el("dt", {}, "Immich server"), + el("dd", { "data-testid": "config-server" }, credentials.server_url || "not configured"), + el("dt", {}, "API key"), + // Presence, never the value — and never a length or prefix either. + el( + "dd", + { "data-testid": "config-key" }, + credentials.api_key_configured ? "configured (never shown)" : "missing" + ), + el("dt", {}, "Reachable"), + el( + "dd", + { "data-testid": "config-reachable" }, + preflight.server.reachable ? "yes" : `no — ${preflight.server.detail || "unknown"}` + ), + el("dt", {}, "Uploader"), + el( + "dd", + { "data-testid": "config-uploader" }, + uploader.installed ? uploader.version || "installed" : `${uploader.binary} is not installed` + ) + ) + ); +} + +function preflightBlockers(preflight) { + if (!preflight.blockers.length) return null; + return el( + "div", + { class: "alert", role: "alert", "data-testid": "preflight-blockers" }, + el("strong", {}, "This scope cannot be uploaded yet"), + el( + "ul", + {}, + ...preflight.blockers.map((blocker) => + el( + "li", + { "data-testid": "preflight-blocker", "data-code": blocker.code }, + `${blocker.code}: ${blocker.message}` + ) + ) + ) + ); +} + +// ── scope ──────────────────────────────────────────────────────────────────── +function scopeSection(preflight, params, albums, allowPartial) { + const selected = new Set(albums || preflight.albums.map((album) => album.album)); + + function toggle(name, checked) { + const next = new Set(selected); + if (checked) next.add(name); + else next.delete(name); + // An empty selection means "everything" again, which is also what an absent + // parameter means — there is no way to preflight nothing. + navigate("/uploads", { ...params, albums: albumsParam([...next]), batch: params.batch }); + } + + const rows = preflight.albums.map((album) => + el( + "tr", + { "data-testid": "album-row", "data-album": album.album }, + el( + "td", + {}, + el("input", { + type: "checkbox", + "data-testid": "album-selected", + "aria-label": `Include ${album.album}`, + checked: selected.has(album.album) ? "checked" : false, + onchange: (event) => toggle(album.album, event.target.checked), + }) + ), + el("td", { "data-testid": "album-name" }, album.album), + // What Immich will call it, which is not always what the folder is called here. + el("td", { "data-testid": "album-immich-name" }, album.album_name), + el("td", { class: "path", "data-testid": "album-folder" }, album.folder), + el("td", { "data-testid": "album-eligible" }, String(album.eligible_count)), + el("td", { "data-testid": "album-blocked" }, String(album.blocked_count)), + el( + "td", + {}, + el("span", { class: `badge ${album.state}`, "data-testid": "album-state" }, album.state), + ...album.blockers.map((blocker) => + el( + "div", + { class: "blocker", "data-testid": "album-blocker", "data-code": blocker.code }, + blocker.message + ) + ) + ), + // The exact invocation, as the server built it. Shown so the upload holds no + // surprises; the key is masked at the source, not here. + el("td", { class: "path", "data-testid": "album-command" }, album.command_preview.join(" ")) + ) + ); + + const totals = preflight.totals; + return el( + "div", + { "data-testid": "upload-scope" }, + el("h2", {}, "Scope"), + el( + "div", + { class: "decision-bar" }, + el("span", { class: "badge", "data-testid": "total-albums" }, `${totals.albums} album(s)`), + el("span", { class: "badge", "data-testid": "total-eligible" }, `${totals.eligible} ready`), + totals.blocked + ? el( + "span", + { class: "badge attention", "data-testid": "total-blocked" }, + `${totals.blocked} blocked` + ) + : null, + el( + "label", + {}, + el("input", { + type: "checkbox", + "data-testid": "allow-partial", + checked: allowPartial ? "checked" : false, + onchange: (event) => + navigate("/uploads", { ...params, partial: event.target.checked ? "1" : "" }), + }), + " Upload ready photos and leave the blocked ones behind" + ) + ), + rows.length + ? el( + "table", + { class: "grid", "data-testid": "albums" }, + el( + "thead", + {}, + el( + "tr", + {}, + ...["", "Album", "Immich album", "Folder", "Ready", "Blocked", "State", "Command"].map( + (label) => el("th", { scope: "col" }, label) + ) + ) + ), + el("tbody", {}, ...rows) + ) + : el("p", { class: "muted", "data-testid": "no-albums" }, "No album is ready to upload.") + ); +} + +// ── confirmation ───────────────────────────────────────────────────────────── +function confirmBlock(preflight, params, allowPartial) { + const ready = preflight.state === "ready"; + const totals = preflight.totals; + const albums = preflight.albums.map((album) => album.album); + return el( + "div", + { class: "card", "data-testid": "confirm" }, + el("h2", {}, "Confirm"), + // The token is shown, not merely sent: a confirmation the user cannot see is a + // confirmation they cannot check against the preview above. + el( + "p", + { class: "muted", "data-testid": "confirm-token" }, + `Preflight ${preflight.token.slice(0, 20)}… · ${allowPartial ? "partial" : "complete"} scope` + ), + el( + "p", + { "data-testid": "upload-note" }, + "Uploading is not reversible from here: Immich decides what to do with each " + + "file, and the app can only record what it reports. One album is sent at a " + + "time and a running album can be stopped." + ), + el( + "div", + { class: "toolbar" }, + el( + "button", + { + class: "primary", + "data-testid": "start-upload", + disabled: ready ? false : "disabled", + title: ready ? false : "resolve the blockers above first", + onclick: () => + run(async () => { + const created = await api.createUploadBatches({ + albums, + token: preflight.token, + allow_partial: allowPartial, + }); + // One lane: the first batch starts now, the rest wait with their own + // start buttons rather than queueing behind a lock that would reject + // them. + const first = created.batches.find((batch) => !batch.retry_blockers.length); + if (!first) return { batches: created.batches.length, started: null }; + const started = await api.startUploadBatch(first.id); + watch(started.job.id, first.id); + return { batches: created.batches.length, started: first.album }; + }), + }, + `Upload ${totals.albums} album(s) · ${totals.eligible} photo(s)` + ) + ) + ); +} + +// ── batches ────────────────────────────────────────────────────────────────── +function batchList(batches, selectedId) { + if (!batches.length) { + return el("p", { class: "muted", "data-testid": "no-batches" }, "No upload has been started yet."); + } + return el( + "div", + { "data-testid": "upload-batches" }, + el("h2", {}, "Batches"), + el( + "table", + { class: "grid", "data-testid": "batches" }, + el( + "thead", + {}, + el( + "tr", + {}, + ...["Album", "State", "Evidence", "Attempts", "Photos"].map((label) => + el("th", { scope: "col" }, label) + ) + ) + ), + el( + "tbody", + {}, + ...batches.map((batch) => + el( + "tr", + { + "data-testid": "batch-row", + "data-album": batch.album, + "aria-current": batch.id === selectedId ? "true" : false, + }, + el( + "td", + {}, + el("a", { class: "link", href: `#/uploads?batch=${encodeURIComponent(batch.id)}` }, batch.album) + ), + el( + "td", + {}, + el("span", { class: `badge ${batch.state}`, "data-testid": "batch-state" }, batch.state) + ), + el("td", { "data-testid": "batch-outcome-state" }, batch.outcome_state || "not parsed"), + el("td", {}, String(batch.attempt_count)), + el("td", {}, String(batch.asset_count)) + ) + ) + ) + ) + ); +} + +function batchDetail(batch, history) { + // Derived from the items, not from the batch's parsed-report summary: verifying + // or resolving an item changes what is true without re-parsing a report, and the + // progress line must show the current answer rather than the uploader's old one. + const counts = {}; + for (const item of batch.items) { + const key = item.outcome || "unknown"; + counts[key] = (counts[key] || 0) + 1; + } + const blockers = batch.retry_blockers; + const uncertain = batch.outcome_state === "requires_verification" || counts.unknown > 0; + + const nodes = [ + el("h2", {}, `${batch.album} — attempt ${batch.attempt_count}`), + el( + "div", + { class: "decision-bar", "data-testid": "batch-progress" }, + el("span", { class: `badge ${batch.state}`, "data-testid": "detail-state" }, batch.state), + ...OUTCOME_LABEL.map(([key, label]) => + el( + "span", + { class: `badge ${key}`, "data-testid": `count-${label}` }, + `${label}: ${counts[key] ?? 0}` + ) + ) + ), + ]; + + if (batch.stale_bytes) { + nodes.push( + el( + "div", + { class: "alert", role: "alert", "data-testid": "stale-bytes" }, + "Files in this batch changed after they were uploaded. Immich still holds the " + + "bytes that were sent; re-approve the album through a fresh preflight rather " + + "than uploading the new bytes over it." + ) + ); + } + if (batch.error_code) { + nodes.push( + el( + "div", + { class: "alert", role: "alert", "data-testid": "batch-error" }, + `${batch.error_code}: ${batch.error_message || ""}` + ) + ); + } + if (uncertain) { + nodes.push( + el( + "div", + { class: "alert", role: "alert", "data-testid": "uncertain" }, + el("strong", {}, "This upload's outcome is not fully known"), + el( + "p", + {}, + "The uploader's report does not account for every file. Retrying could create " + + "a second copy of something Immich already accepted, so verify it first: the " + + "check asks Immich whether it holds the exact bytes that were sent." + ) + ) + ); + } + + nodes.push( + el( + "div", + { class: "toolbar" }, + // A start button exists only when the server would accept one. An uncertain + // outcome and changed bytes therefore offer verification, never a retry. + blockers.length + ? el( + "div", + { class: "blocker", "data-testid": "retry-blocked" }, + blockers.map((blocker) => `${blocker.code}: ${blocker.message}`).join("; ") + ) + : el( + "button", + { + class: "primary", + "data-testid": "start-batch", + onclick: () => + run(async () => { + const started = await api.startUploadBatch(batch.id); + watch(started.job.id, batch.id); + return { started: batch.album }; + }), + }, + batch.attempt_count ? "Run this album again" : "Upload this album" + ), + ["planned", "running"].includes(batch.state) + ? el( + "button", + { + "data-testid": "cancel-batch", + onclick: () => run(() => api.cancelUploadBatch(batch.id)), + }, + batch.state === "running" ? "Stop after the current file" : "Cancel this album" + ) + : null, + // Offered for anything that has run, not only for uncertain outcomes: + // re-checking is read-only and idempotent, and it is how a file edited after + // its upload is discovered. + batch.attempt_count + ? el( + "button", + { + "data-testid": "verify-batch", + onclick: () => run(() => api.verifyUploadBatch(batch.id)), + }, + "Verify against Immich" + ) + : null + ), + itemsTable(batch) + ); + if (history.length) nodes.push(historyList(history)); + return el( + "div", + { class: "card", "data-testid": "batch-detail", "data-batch": batch.id }, + ...nodes + ); +} + +function itemsTable(batch) { + if (!batch.items.length) { + return el("p", { class: "muted", "data-testid": "no-items" }, "This batch has no photos."); + } + return el( + "table", + { class: "grid", "data-testid": "items" }, + el( + "thead", + {}, + el( + "tr", + {}, + ...["Photo", "Outcome", "Evidence", "Verification", "Bytes now", "Resolve"].map((label) => + el("th", { scope: "col" }, label) + ) + ) + ), + el( + "tbody", + {}, + ...batch.items.map((item) => + el( + "tr", + { "data-testid": "item-row", "data-asset-id": item.asset_id }, + el("td", { class: "path", "data-testid": "item-path" }, item.path), + el( + "td", + {}, + el( + "span", + { class: `badge ${item.outcome || ""}`, "data-testid": "item-outcome" }, + item.outcome === "uploaded" ? "new" : item.outcome || "pending" + ) + ), + el("td", { class: "muted", "data-testid": "item-evidence" }, item.evidence || "—"), + el("td", { "data-testid": "item-verification" }, item.verification || "—"), + el( + "td", + {}, + item.changed_after_upload + ? el("span", { class: "badge attention", "data-testid": "item-changed" }, "changed") + : el("span", { class: "muted" }, "unchanged") + ), + el("td", {}, resolveForm(batch, item)) + ) + ) + ) + ); +} + +// Manual resolution is evidence, not permission: the note and the author are +// required by the server, so the form collects both and offers no default. +function resolveForm(batch, item) { + const unresolved = !item.outcome || item.outcome === "unknown" || item.verification === "inconclusive"; + if (!unresolved) return el("span", { class: "muted" }, "—"); + + const outcomeSelect = el( + "select", + { "data-testid": "resolve-outcome", "aria-label": `Outcome for ${item.path}` }, + ...OUTCOME_LABEL.map(([key, label]) => el("option", { value: key }, label)) + ); + const evidence = el("input", { + type: "text", + "data-testid": "resolve-evidence", + "aria-label": `What you checked for ${item.path}`, + placeholder: "What did you check?", + }); + const actor = el("input", { + type: "text", + "data-testid": "resolve-actor", + "aria-label": `Who checked ${item.path}`, + placeholder: "Who are you?", + }); + return el( + "div", + { class: "toolbar" }, + outcomeSelect, + evidence, + actor, + el( + "button", + { + "data-testid": "resolve-item", + onclick: () => + run(() => + api.resolveUploadItem(batch.id, { + asset_id: item.asset_id, + outcome: outcomeSelect.value, + evidence: evidence.value, + actor: actor.value, + }) + ), + }, + "Record" + ) + ); +} + +function historyList(history) { + return el( + "div", + { "data-testid": "verification-history" }, + el("h3", {}, "Verification history"), + el( + "ul", + {}, + ...history.map((entry) => + el( + "li", + { "data-testid": "history-entry", "data-source": entry.source }, + `${entry.created_at || ""} · ${entry.action} · ${entry.source} · ${entry.result} → ` + + `${entry.outcome || "unresolved"} — ${entry.evidence}` + + (entry.actor ? ` (${entry.actor})` : "") + ) + ) + ) + ); +} + +// ── running commands ───────────────────────────────────────────────────────── +async function run(action) { + try { + outcome = { kind: "ok", result: await action() }; + } catch (error) { + outcome = error.status === 409 ? { kind: "conflict", error } : { kind: "error", error }; + } + render(); +} + +// Live job activity. Events are appended to the log node and the running batch's +// panel is refreshed on its own tick — the uploader reports per file, not per job +// event, so waiting for the next event would leave the panel behind. Only that +// panel is rebuilt: a full re-render would re-run preflight, which re-hashes the +// library, so that happens once when the job ends. +const REFRESH_MS = 1000; + +function watch(jobId, batchId) { + activity = [`Started upload job ${jobId}`]; + const tick = setInterval(() => refreshBatch(batchId), REFRESH_MS); + subscribeJob(jobId, { + onEvent: (event) => { + activity.push(`${event.type}${event.message ? ": " + event.message : ""}`); + const log = document.querySelector('[data-testid="upload-activity"]'); + if (log) log.textContent = activity.join("\n"); + }, + onDone: () => { + clearInterval(tick); + activity.push("done"); + render(); + }, + }); +} + +async function refreshBatch(batchId) { + const node = document.querySelector(`[data-testid="batch-detail"][data-batch="${batchId}"]`); + if (!node) return; // the user navigated away from the running batch + try { + const [batch, verifications] = await Promise.all([ + api.getUploadBatch(batchId), + api.uploadVerifications(batchId), + ]); + node.replaceWith(batchDetail(batch, verifications.verifications)); + } catch (_) { + // Transient: the next tick tries again, and the job's end re-renders anyway. + } +} + +function activityLog() { + return el( + "pre", + { + class: "activity-log", + role: "status", + "aria-live": "polite", + "data-testid": "upload-activity", + }, + activity.join("\n") + ); +} + +function outcomeBanner() { + if (!outcome) return null; + if (outcome.kind === "conflict") { + return el( + "div", + { class: "alert", role: "alert", "data-testid": "conflict" }, + `The server refused this: ${outcome.error.message}. Nothing was uploaded; the ` + + "state below is the server's current one — review it and decide again." + ); + } + if (outcome.kind === "error") { + return el( + "div", + { class: "alert", role: "alert", "data-testid": "upload-error" }, + `Failed: ${outcome.error.message}` + ); + } + const result = outcome.result; + if (result && result.batches !== undefined) { + return el( + "div", + { class: "alert", role: "status", "data-testid": "upload-result" }, + `Approved ${result.batches} album batch(es).` + + (result.started ? ` Uploading ${result.started} now.` : " Nothing could be started yet.") + ); + } + return el( + "div", + { class: "alert", role: "status", "data-testid": "upload-result" }, + "Done — the state below is the server's." + ); +} diff --git a/photo_pipeline/services/upload_batches.py b/photo_pipeline/services/upload_batches.py index b048d40..a08af50 100644 --- a/photo_pipeline/services/upload_batches.py +++ b/photo_pipeline/services/upload_batches.py @@ -397,7 +397,7 @@ class UploadBatchService: def _batch_dict(row: UploadBatch, items: list[UploadItem]) -> dict: - return { + batch = { "id": row.id, "album": row.album, "folder": row.folder, @@ -449,3 +449,8 @@ def _batch_dict(row: UploadBatch, items: list[UploadItem]) -> dict: for item in items ], } + # Why this batch may not be (re)started, from the one place that decides it + # (US05-04). Carried in the record so the browser can hide an action the server + # would refuse instead of re-implementing the policy (US05-05). + batch["retry_blockers"] = retry_blockers(batch) + return batch diff --git a/tests/e2e/_pipeline_harness.py b/tests/e2e/_pipeline_harness.py index 144ad26..fa4385c 100644 --- a/tests/e2e/_pipeline_harness.py +++ b/tests/e2e/_pipeline_harness.py @@ -135,15 +135,27 @@ class Server: self.proc = None -def start_worker(seeded: Seeded, *, fake_vision_log: Path) -> subprocess.Popen: - """Launch a real durable worker wired to the recording vision fake.""" +def start_worker( + seeded: Seeded, + *, + fake_vision_log: Path | None = None, + extra_env: dict[str, str] | None = None, +) -> subprocess.Popen: + """Launch a real durable worker wired to the recording vision fake. + + ``extra_env`` carries whatever else the job under test needs — the Immich + credentials and uploader path, for the upload lane. + """ + extra = dict(extra_env or {}) + if fake_vision_log is not None: + extra["PHOTO_PIPELINE_FAKE_VISION_LOG"] = str(fake_vision_log) return subprocess.Popen( [sys.executable, "-m", "photo_pipeline", "worker", "--id", "e2e-worker"], cwd=str(REPO), env=_env( seeded, free_port(), # unused by the worker, but keeps the env shape uniform - extra={"PHOTO_PIPELINE_FAKE_VISION_LOG": str(fake_vision_log)}, + extra=extra, ), stdout=subprocess.PIPE, stderr=subprocess.PIPE, diff --git a/tests/e2e/test_uploads_ui.py b/tests/e2e/test_uploads_ui.py new file mode 100644 index 0000000..4b9ca3e --- /dev/null +++ b/tests/e2e/test_uploads_ui.py @@ -0,0 +1,433 @@ +"""Browser journeys for the upload view (US05-05). + +Covers the preflight preview (scope, redacted configuration, blockers, exact +confirmation), a real upload through the real worker with per-outcome progress, +stopping a running album, and the two ways out of an uncertain outcome — +verification against Immich and a manual resolution that records its evidence. + +Nothing external is mocked inside the browser: the uploader is a real executable +driven by the real worker process, and Immich is a real HTTP server answering the +same ``ping``/``bulk-upload-check`` endpoints the adapter calls in production. The +API key is a sentinel string, so the last test can prove it never reached the page. +""" + +from __future__ import annotations + +import json +import stat +import threading +from http.server import BaseHTTPRequestHandler, HTTPServer +from pathlib import Path + +import httpx +import pytest +from playwright.sync_api import expect + +from tests.e2e._pipeline_harness import ( + Server, + seed_album, + session_factory, + start_worker, + wait_until, +) + +TIMEOUT = 10 +SENTINEL_KEY = "immich-sentinel-9f3a2b" +UPLOADER_VERSION = "immich-go 0.21.0" # a pinned family, so reports are parsable + +# A report the pinned text-v1 grammar understands: one new file, one the server +# already holds. ``$6`` is the folder argument of ``upload from-folder``. +REPORTING_UPLOADER = ( + 'echo "INFO uploaded $6/a.jpg"\n' + 'echo "INFO server has the same file $6/b.jpg"\n' + 'echo "Uploaded 1, duplicates 1"\n' + "exit 0\n" +) +# Exits cleanly but says nothing about any file: the process succeeded, the +# per-file outcome is unknown. +SILENT_UPLOADER = "exit 0\n" + + +# ── fake Immich ────────────────────────────────────────────────────────────── + + +def _handler(state: dict): + class Handler(BaseHTTPRequestHandler): + def do_GET(self): # noqa: N802 (BaseHTTPRequestHandler API) + self._json(200, {"res": "pong"}) + + def do_POST(self): # noqa: N802 + length = int(self.headers.get("Content-Length", 0)) + payload = json.loads(self.rfile.read(length) or b"{}") + if state["mode"] == "broken": + self.send_error(500, "bulk-upload-check is unavailable") + return + reject = state["mode"] == "present" + self._json( + 200, + { + "results": [ + { + "id": asset["id"], + "action": "reject" if reject else "accept", + "reason": "duplicate" if reject else None, + } + for asset in payload.get("assets", []) + ] + }, + ) + + def _json(self, code: int, body: dict) -> None: + raw = json.dumps(body).encode() + self.send_response(code) + self.send_header("Content-Type", "application/json") + self.send_header("Content-Length", str(len(raw))) + self.end_headers() + self.wfile.write(raw) + + def log_message(self, *args): + pass + + return Handler + + +class FakeImmich: + """An Immich that answers ping, and says whether it holds the exact bytes. + + ``mode`` is what the next verification will find: ``present`` (the server + deduplicates them, so it has them), ``absent`` (it would accept them, so it does + not), or ``broken`` (no usable answer at all). + """ + + def __init__(self) -> None: + self.state = {"mode": "present"} + self._server = HTTPServer(("127.0.0.1", 0), _handler(self.state)) + threading.Thread(target=self._server.serve_forever, daemon=True).start() + self.url = f"http://127.0.0.1:{self._server.server_port}" + + def mode(self, mode: str) -> None: + self.state["mode"] = mode + + def stop(self) -> None: + self._server.shutdown() + self._server.server_close() + + +# ── stack ──────────────────────────────────────────────────────────────────── + + +def _uploader(tmp_path: Path, body: str) -> Path: + path = tmp_path / "immich-go" + path.write_text( + f'#!/bin/sh\nif [ "$1" = "--version" ]; then echo "{UPLOADER_VERSION}"; exit 0; fi\n{body}' + ) + path.chmod(path.stat().st_mode | stat.S_IEXEC | stat.S_IXGRP | stat.S_IXOTH) + return path + + +def _mark_upload_ready(seeded, *, unverified: tuple[str, ...] = ()) -> None: + """Give every seeded photo the verified EXIF checkpoints upload requires. + + ``unverified`` names stems whose analysis checkpoint stays incomplete, which is + what makes an album partially blocked. + """ + from datetime import datetime, timezone + + from sqlalchemy import select + + from photo_pipeline.models import AnalysisResult, SafetyReview + + now = datetime(2026, 1, 1, tzinfo=timezone.utc) + blocked = {seeded.asset_ids[stem] for stem in unverified} + with session_factory(seeded) as sf: + with sf() as session: + for review in session.scalars(select(SafetyReview)): + review.exif_verified_at = now + for analysis in session.scalars(select(AnalysisResult)): + analysis.exif_written_at = None if analysis.asset_id in blocked else now + session.commit() + + +class Stack: + """A seeded, upload-ready library plus the server, worker, and fake Immich.""" + + def __init__(self, tmp_path: Path, seeded, immich: FakeImmich) -> None: + self.tmp_path = tmp_path + self.seeded = seeded + self.immich = immich + self.server: Server | None = None + self.worker = None + + def start(self, *, uploader: str = REPORTING_UPLOADER, worker: bool = True) -> "Stack": + env = { + "PHOTO_PIPELINE_IMMICH_SERVER_URL": self.immich.url, + "PHOTO_PIPELINE_IMMICH_API_KEY": SENTINEL_KEY, + "PHOTO_PIPELINE_IMMICH_GO_BINARY": str(_uploader(self.tmp_path, uploader)), + } + self.server = Server(self.seeded, extra_env=env).start() + self.base = self.server.base + if worker: + self.worker = start_worker(self.seeded, extra_env=env) + return self + + def batches(self) -> list[dict]: + return httpx.get(f"{self.base}/api/v1/upload-batches", timeout=TIMEOUT).json()["batches"] + + def stop(self) -> None: + if self.worker is not None: + self.worker.terminate() + self.worker.wait(timeout=10) + if self.server is not None: + self.server.stop() + self.immich.stop() + + +@pytest.fixture +def stack(tmp_path): + seeded = seed_album(tmp_path) + _mark_upload_ready(seeded) + running = Stack(tmp_path, seeded, FakeImmich()) + try: + yield running + finally: + running.stop() + + +def _open(page, stack) -> None: + page.goto(f"{stack.base}/app/#/uploads") + page.get_by_test_id("upload-scope").wait_for() + + +def _upload(page, stack) -> None: + """Confirm the upload and wait for the worker to finish the album.""" + _open(page, stack) + page.get_by_test_id("start-upload").click() + expect(page.get_by_test_id("detail-state")).not_to_have_text("planned", timeout=30_000) + + +# ── preflight ──────────────────────────────────────────────────────────────── + + +def test_the_preview_shows_scope_configuration_and_an_exact_confirmation(page, stack): + errors = [] + page.on("console", lambda m: errors.append(m.text) if m.type == "error" else None) + stack.start(worker=False) + _open(page, stack) + + expect(page.get_by_test_id("config-server")).to_have_text(stack.immich.url) + expect(page.get_by_test_id("config-key")).to_have_text("configured (never shown)") + expect(page.get_by_test_id("config-reachable")).to_have_text("yes") + expect(page.get_by_test_id("config-uploader")).to_have_text(UPLOADER_VERSION) + + row = page.get_by_test_id("album-row").first + expect(row.get_by_test_id("album-name")).to_have_text("rome") + expect(row.get_by_test_id("album-immich-name")).to_have_text("rome") + expect(row.get_by_test_id("album-eligible")).to_have_text("2") + expect(row.get_by_test_id("album-state")).to_have_text("ready") + # The exact invocation is previewed, with the key masked at the source. + command = row.get_by_test_id("album-command").inner_text() + assert "upload from-folder" in command and "--album-name=rome" in command + assert "--api-key=***" in command + + # The confirmation names the scope it is about to send, not just "Upload". + expect(page.get_by_test_id("start-upload")).to_have_text("Upload 1 album(s) · 2 photo(s)") + expect(page.get_by_test_id("start-upload")).to_be_enabled() + assert errors == [], f"console errors: {errors}" + + +def test_an_unfinished_photo_blocks_its_album_and_the_confirmation(page, stack): + _mark_upload_ready(stack.seeded, unverified=("b",)) + stack.start(worker=False) + _open(page, stack) + + expect(page.get_by_test_id("album-state")).to_have_text("blocked") + expect(page.get_by_test_id("album-blocker")).to_have_attribute("data-code", "partial_scope") + expect(page.get_by_test_id("album-eligible")).to_have_text("1") + expect(page.get_by_test_id("album-blocked")).to_have_text("1") + expect(page.get_by_test_id("start-upload")).to_be_disabled() + + # Partial upload exists, but only as a deliberate act: ticking it re-runs the + # preflight under that policy and the confirmation then names the smaller scope. + page.get_by_test_id("allow-partial").check() + expect(page.get_by_test_id("album-state")).to_have_text("ready") + expect(page.get_by_test_id("start-upload")).to_have_text("Upload 1 album(s) · 1 photo(s)") + assert stack.batches() == [], "nothing may be created by previewing" + + +# ── uploading ──────────────────────────────────────────────────────────────── + + +def test_a_confirmed_upload_runs_and_reports_each_outcome(page, stack): + stack.start() + _upload(page, stack) + + expect(page.get_by_test_id("detail-state")).to_have_text("succeeded") + expect(page.get_by_test_id("count-new")).to_have_text("new: 1") + expect(page.get_by_test_id("count-duplicate")).to_have_text("duplicate: 1") + expect(page.get_by_test_id("count-uncertain")).to_have_text("uncertain: 0") + expect(page.get_by_test_id("count-failed")).to_have_text("failed: 0") + expect(page.get_by_test_id("batch-outcome-state")).to_have_text("verified") + + outcomes = sorted(page.get_by_test_id("item-outcome").all_inner_texts()) + assert outcomes == ["duplicate", "new"] + # A finished album offers no restart: the server would refuse one. + expect(page.get_by_test_id("retry-blocked")).to_contain_text("not_runnable") + expect(page.get_by_test_id("start-batch")).to_have_count(0) + + +def test_a_running_album_can_be_stopped(page, stack): + stack.start(uploader='echo "INFO starting"; sleep 20; exit 0\n') + _open(page, stack) + page.get_by_test_id("start-upload").click() + + stop = page.get_by_test_id("cancel-batch") + expect(stop).to_have_text("Stop after the current file", timeout=30_000) + stop.click() + + expect(page.get_by_test_id("detail-state")).to_have_text("cancelled", timeout=30_000) + # A stopped album is a clean boundary, not an uncertain one: it can run again. + expect(page.get_by_test_id("start-batch")).to_be_visible() + + +def test_the_finished_upload_survives_a_reload(page, stack): + stack.start() + _upload(page, stack) + expect(page.get_by_test_id("detail-state")).to_have_text("succeeded") + + page.reload() + + expect(page.get_by_test_id("detail-state")).to_have_text("succeeded") + expect(page.get_by_test_id("count-new")).to_have_text("new: 1") + # The result banner is this tab's memory, not server state, so it stays gone. + expect(page.get_by_test_id("upload-result")).to_have_count(0) + + +# ── uncertainty ────────────────────────────────────────────────────────────── + + +def test_an_uncertain_outcome_offers_verification_and_no_retry(page, stack): + stack.start(uploader=SILENT_UPLOADER) + _upload(page, stack) + + expect(page.get_by_test_id("detail-state")).to_have_text("succeeded") + expect(page.get_by_test_id("batch-outcome-state")).to_have_text("requires_verification") + expect(page.get_by_test_id("count-uncertain")).to_have_text("uncertain: 2") + expect(page.get_by_test_id("uncertain")).to_be_visible() + # The point of the story: verification is offered, a retry is not. + expect(page.get_by_test_id("verify-batch")).to_be_visible() + expect(page.get_by_test_id("start-batch")).to_have_count(0) + + +def test_verification_asks_immich_and_resolves_the_uncertain_items(page, stack): + stack.start(uploader=SILENT_UPLOADER) + stack.immich.mode("present") # Immich holds exactly the bytes that were sent + _upload(page, stack) + + page.get_by_test_id("verify-batch").click() + + expect(page.get_by_test_id("batch-outcome-state")).to_have_text("verified") + expect(page.get_by_test_id("count-new")).to_have_text("new: 2") + expect(page.get_by_test_id("count-uncertain")).to_have_text("uncertain: 0") + expect(page.get_by_test_id("item-verification").first).to_have_text("present") + expect(page.get_by_test_id("history-entry").first).to_have_attribute( + "data-source", "immich_api" + ) + expect(page.get_by_test_id("uncertain")).to_have_count(0) + + +def test_an_unusable_answer_stays_uncertain_until_someone_records_evidence(page, stack): + stack.start(uploader=SILENT_UPLOADER) + stack.immich.mode("broken") # answers, but nothing this adapter will interpret + _upload(page, stack) + + page.get_by_test_id("verify-batch").click() + + # No answer is never "no": the items stay uncertain rather than being called failed. + expect(page.get_by_test_id("item-verification").first).to_have_text("inconclusive") + expect(page.get_by_test_id("uncertain")).to_be_visible() + expect(page.get_by_test_id("start-batch")).to_have_count(0) + + row = page.get_by_test_id("item-row").first + row.get_by_test_id("resolve-outcome").select_option("uploaded") + row.get_by_test_id("resolve-evidence").fill("found it in Immich by checksum") + row.get_by_test_id("resolve-actor").fill("dom") + row.get_by_test_id("resolve-item").click() + + expect(page.get_by_test_id("item-row").first.get_by_test_id("item-outcome")).to_have_text("new") + manual = page.get_by_test_id("history-entry").last + expect(manual).to_have_attribute("data-source", "operator") + expect(manual).to_contain_text("found it in Immich by checksum") + expect(manual).to_contain_text("dom") + + +def test_bytes_changed_after_upload_are_flagged_and_block_another_run(page, stack): + stack.start() + _upload(page, stack) + expect(page.get_by_test_id("detail-state")).to_have_text("succeeded") + # The user edits a photo after it was uploaded; Immich still holds the old bytes. + (stack.seeded.lib / "rome" / "a.jpg").write_bytes(b"edited after the upload") + + page.get_by_test_id("verify-batch").click() + + expect(page.get_by_test_id("stale-bytes")).to_be_visible() + expect(page.get_by_test_id("item-changed")).to_have_count(1) + expect(page.get_by_test_id("retry-blocked")).to_contain_text("changed_after_upload") + expect(page.get_by_test_id("start-batch")).to_have_count(0) + # The preflight agrees: those bytes are no longer approved for any new upload. + expect(page.get_by_test_id("album-state")).to_have_text("blocked") + expect(page.get_by_test_id("album-blocked")).to_have_text("1") + + +# ── privacy ────────────────────────────────────────────────────────────────── + + +def test_the_api_key_never_reaches_the_browser(page, stack): + logs = [] + page.on("console", lambda message: logs.append(message.text)) + stack.start() + _upload(page, stack) + page.get_by_test_id("verify-batch").click() + expect(page.get_by_test_id("item-verification").first).to_have_text("present") + + storage = page.evaluate( + "() => JSON.stringify([{...localStorage}, {...sessionStorage}, document.cookie])" + ) + assert SENTINEL_KEY not in page.content() + assert SENTINEL_KEY not in page.url + assert SENTINEL_KEY not in storage + assert SENTINEL_KEY not in "\n".join(logs) + # The uploader was given the real key even though nothing on the page shows it. + report = wait_until(lambda: sorted((stack.seeded.data / "uploads").glob("*.log")))[0] + assert SENTINEL_KEY not in report.read_text() + + +# ── recovery ───────────────────────────────────────────────────────────────── + + +def test_an_interrupted_attempt_is_shown_as_uncertain_after_a_restart(page, stack): + """A worker that vanished mid-upload leaves a batch whose outcome nobody knows. + Startup recovery marks it uncertain, and the view must not offer to retry it.""" + from photo_pipeline.models import UploadBatch + + stack.start(worker=False) + _open(page, stack) + token = httpx.post(f"{stack.base}/api/v1/upload-preflight", json={}, timeout=TIMEOUT).json()[ + "token" + ] + created = httpx.post( + f"{stack.base}/api/v1/upload-batches", json={"token": token}, timeout=TIMEOUT + ).json()["batches"][0] + with session_factory(stack.seeded) as sf: # what a killed worker leaves behind + with sf() as session: + session.get(UploadBatch, created["id"]).state = "running" + session.commit() + + stack.server.stop() + stack.server.start() # the same port, so recovery runs in a genuinely fresh process + page.reload() + + expect(page.get_by_test_id("detail-state")).to_have_text("unknown_requires_verification") + expect(page.get_by_test_id("batch-error")).to_contain_text("interrupted") + expect(page.get_by_test_id("uncertain")).to_be_visible() + expect(page.get_by_test_id("start-batch")).to_have_count(0) + expect(page.get_by_test_id("retry-blocked")).to_contain_text("requires_verification") diff --git a/tests/story_traceability.json b/tests/story_traceability.json index 24b4c27..88fdc6c 100644 --- a/tests/story_traceability.json +++ b/tests/story_traceability.json @@ -117,6 +117,9 @@ "US05-04": [ "tests/unit/test_immich_bulk_check.py", "tests/integration/test_upload_verification.py" + ], + "US05-05": [ + "tests/e2e/test_uploads_ui.py" ] } }