Compare commits
1 Commits
us/US05-01
...
us/US04-05
| Author | SHA1 | Date | |
|---|---|---|---|
| 9a7810b936 |
@@ -165,6 +165,11 @@ a.link:hover { text-decoration: underline; }
|
||||
.badge.approved { color: var(--ok); border-color: var(--ok); }
|
||||
.badge.error { color: var(--danger); border-color: var(--danger); }
|
||||
|
||||
/* Renames: the preview table must show whole paths, so let them wrap rather than
|
||||
truncate — a hidden path segment is exactly what makes a move a surprise. */
|
||||
.grid th, .grid td { text-align: left; padding: 6px 8px; border-bottom: 1px solid var(--border); vertical-align: top; }
|
||||
.path { font-family: ui-monospace, monospace; font-size: 0.85em; word-break: break-all; }
|
||||
|
||||
/* Narrow screens: stack the panes so blockers and actions stay reachable. */
|
||||
@media (max-width: 720px) {
|
||||
.two-pane { grid-template-columns: 1fr; }
|
||||
|
||||
@@ -17,6 +17,7 @@
|
||||
<a href="#/library" data-nav="library">Library</a>
|
||||
<a href="#/analyze" data-nav="analyze">Analyze</a>
|
||||
<a href="#/albums" data-nav="albums">Albums</a>
|
||||
<a href="#/renames" data-nav="renames">Renames</a>
|
||||
<a href="#/stats" data-nav="stats">Stats</a>
|
||||
</nav>
|
||||
</header>
|
||||
|
||||
@@ -99,4 +99,19 @@ export const api = {
|
||||
body: JSON.stringify(payload),
|
||||
...opts,
|
||||
}),
|
||||
|
||||
// ── Renames: plan, confirm, apply, recover ──────────────────────────────
|
||||
listPlans: (opts = {}) => request("/rename-plans", opts),
|
||||
buildPlan: (opts = {}) => request("/rename-plans", { method: "POST", ...opts }),
|
||||
getPlan: (id, opts = {}) => request(`/rename-plans/${encodeURIComponent(id)}`, opts),
|
||||
applyPlan: (id, payload, opts = {}) =>
|
||||
request(`/rename-plans/${encodeURIComponent(id)}/apply`, {
|
||||
method: "POST",
|
||||
body: JSON.stringify(payload),
|
||||
...opts,
|
||||
}),
|
||||
rollbackPlan: (id, opts = {}) =>
|
||||
request(`/rename-plans/${encodeURIComponent(id)}/rollback`, { method: "POST", ...opts }),
|
||||
renameRecovery: (opts = {}) => request("/rename-recovery", opts),
|
||||
resolveRecovery: (opts = {}) => request("/rename-recovery/resolve", { method: "POST", ...opts }),
|
||||
};
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import { api } from "./api.js";
|
||||
import { navigate, onRouteChange, parseHash } from "./router.js";
|
||||
import { renderRenames, setRenamesRender } from "./renames.js";
|
||||
import {
|
||||
renderAlbums,
|
||||
renderAnalyze,
|
||||
@@ -362,11 +363,13 @@ function render() {
|
||||
else if (path === "/library") renderLibrary(root, params);
|
||||
else if (path === "/analyze") renderAnalyze(root, params);
|
||||
else if (path === "/albums") renderAlbums(root, params);
|
||||
else if (path === "/renames") renderRenames(root, params);
|
||||
else if (path === "/stats") renderStats(root, params);
|
||||
else show(errorBanner("Unknown view"));
|
||||
}
|
||||
|
||||
// Let views re-render the current route after a mutation.
|
||||
setRender(render);
|
||||
setRenamesRender(render);
|
||||
onRouteChange(render);
|
||||
render();
|
||||
|
||||
357
frontend/js/renames.js
Normal file
357
frontend/js/renames.js
Normal file
@@ -0,0 +1,357 @@
|
||||
// Renames view (US04-05): preview a rename plan, confirm it with the server-issued
|
||||
// token, watch it run, and resolve anything an interruption left behind.
|
||||
//
|
||||
// Nothing here decides what is safe. Blocking issues, plan applicability, and
|
||||
// recovery classifications all come from the server; the view only shows them and
|
||||
// refuses to offer an action the server would reject.
|
||||
import { api } from "./api.js";
|
||||
import { el, errorBanner, setActiveNav } from "./dom.js";
|
||||
|
||||
// What the last confirm did, for this tab only. Deliberately not persisted: after a
|
||||
// reload the page must show what the journal says, not what this page remembers.
|
||||
let outcome = null;
|
||||
|
||||
let render = () => {};
|
||||
export function setRenamesRender(fn) {
|
||||
render = fn;
|
||||
}
|
||||
|
||||
const BLOCKED_TITLE = "an unresolved rename must be resolved first";
|
||||
|
||||
export async function renderRenames(root, params = {}) {
|
||||
setActiveNav("renames");
|
||||
let plans, recovery;
|
||||
try {
|
||||
[plans, recovery] = await Promise.all([api.listPlans(), api.renameRecovery()]);
|
||||
} catch (error) {
|
||||
root.replaceChildren(errorBanner(`Failed to load renames: ${error.message}`));
|
||||
return;
|
||||
}
|
||||
|
||||
const selectedId = params.plan || (plans.items[0] && plans.items[0].id) || null;
|
||||
let plan = null;
|
||||
if (selectedId) {
|
||||
try {
|
||||
plan = await api.getPlan(selectedId);
|
||||
} catch (error) {
|
||||
root.replaceChildren(errorBanner(`Failed to load plan: ${error.message}`));
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
const blocked = recovery.blocks_mutation;
|
||||
root.replaceChildren(
|
||||
el("h1", {}, "Renames"),
|
||||
recoveryPanel(recovery),
|
||||
el(
|
||||
"div",
|
||||
{ class: "toolbar" },
|
||||
el(
|
||||
"button",
|
||||
{
|
||||
class: "primary",
|
||||
"data-testid": "build-plan",
|
||||
disabled: blocked ? "disabled" : false,
|
||||
title: blocked ? BLOCKED_TITLE : false,
|
||||
onclick: () => run(() => api.buildPlan(), "Planning…"),
|
||||
},
|
||||
"Build plan from approved names"
|
||||
)
|
||||
),
|
||||
outcomeBanner(),
|
||||
plan
|
||||
? planSection(plan, blocked)
|
||||
: el(
|
||||
"p",
|
||||
{ class: "muted", "data-testid": "no-plan" },
|
||||
"No rename plan yet — approve album names, then build a plan to preview the moves."
|
||||
)
|
||||
);
|
||||
}
|
||||
|
||||
// ── preview ──────────────────────────────────────────────────────────────────
|
||||
function planSection(plan, blocked) {
|
||||
const blockingIssues = plan.operations.flatMap((operation) =>
|
||||
operation.issues.filter((issue) => issue.blocking)
|
||||
);
|
||||
|
||||
return el(
|
||||
"div",
|
||||
{ "data-testid": "plan" },
|
||||
el(
|
||||
"div",
|
||||
{ class: "decision-bar" },
|
||||
el("span", { class: `badge ${plan.state}`, "data-testid": "plan-state" }, plan.state),
|
||||
el(
|
||||
"span",
|
||||
{ class: "badge", "data-testid": "operation-count" },
|
||||
`${plan.operation_count} folder${plan.operation_count === 1 ? "" : "s"}`
|
||||
),
|
||||
el(
|
||||
"span",
|
||||
{ class: "badge", "data-testid": "affected-assets" },
|
||||
`${plan.operations.reduce((sum, o) => sum + o.asset_count, 0)} photos`
|
||||
),
|
||||
blockingIssues.length
|
||||
? el(
|
||||
"span",
|
||||
{ class: "badge attention", "data-testid": "blocking-count" },
|
||||
`${blockingIssues.length} blocking issue${blockingIssues.length === 1 ? "" : "s"}`
|
||||
)
|
||||
: null
|
||||
),
|
||||
plan.blockers.length
|
||||
? el(
|
||||
"div",
|
||||
{ class: "alert", role: "alert", "data-testid": "plan-blockers" },
|
||||
`This plan cannot run: ${plan.blockers.join("; ")}`
|
||||
)
|
||||
: null,
|
||||
operationsTable(plan),
|
||||
confirmBlock(plan, blocked)
|
||||
);
|
||||
}
|
||||
|
||||
function operationType(operation) {
|
||||
if (operation.case_only) return "case-only rename";
|
||||
if (!operation.same_filesystem) return "cross-filesystem move";
|
||||
return "rename";
|
||||
}
|
||||
|
||||
function operationsTable(plan) {
|
||||
if (!plan.operations.length) {
|
||||
return el("p", { class: "muted", "data-testid": "no-operations" }, "This plan moves nothing.");
|
||||
}
|
||||
return el(
|
||||
"table",
|
||||
{ class: "grid", "data-testid": "operations" },
|
||||
el(
|
||||
"thead",
|
||||
{},
|
||||
el(
|
||||
"tr",
|
||||
{},
|
||||
...["#", "Album", "From", "To", "Type", "Photos", "Issues", "State"].map((label) =>
|
||||
el("th", { scope: "col" }, label)
|
||||
)
|
||||
)
|
||||
),
|
||||
el(
|
||||
"tbody",
|
||||
{},
|
||||
...plan.operations.map((operation) =>
|
||||
el(
|
||||
"tr",
|
||||
{ "data-testid": "operation-row", "data-album": operation.album },
|
||||
el("td", {}, String(operation.sequence)),
|
||||
el("td", { "data-testid": "op-album" }, operation.album),
|
||||
// Full paths, not basenames: the whole point of the preview is that no
|
||||
// move is a surprise.
|
||||
el("td", { class: "path", "data-testid": "op-source" }, operation.source_path),
|
||||
el("td", { class: "path", "data-testid": "op-destination" }, operation.destination_path),
|
||||
el("td", { "data-testid": "op-type" }, operationType(operation)),
|
||||
el("td", { "data-testid": "op-count" }, String(operation.asset_count)),
|
||||
el(
|
||||
"td",
|
||||
{ "data-testid": "op-issues" },
|
||||
operation.issues.length
|
||||
? operation.issues.map((issue) =>
|
||||
el(
|
||||
"span",
|
||||
{
|
||||
class: `badge ${issue.blocking ? "attention" : ""}`,
|
||||
"data-testid": "op-issue",
|
||||
"data-code": issue.code,
|
||||
title: issue.message,
|
||||
},
|
||||
`${issue.blocking ? "blocks" : "note"}: ${issue.code}`
|
||||
)
|
||||
)
|
||||
: el("span", { class: "muted" }, "—")
|
||||
),
|
||||
el(
|
||||
"td",
|
||||
{},
|
||||
el("span", { "data-testid": "op-state" }, operation.journal_state),
|
||||
operation.verified_at
|
||||
? el("span", { class: "badge complete", "data-testid": "op-verified" }, "verified")
|
||||
: null,
|
||||
operation.error_code
|
||||
? el(
|
||||
"div",
|
||||
{ class: "blocker", "data-testid": "op-error" },
|
||||
`${operation.error_code}: ${operation.error_message || ""}`
|
||||
)
|
||||
: null
|
||||
)
|
||||
)
|
||||
)
|
||||
)
|
||||
);
|
||||
}
|
||||
|
||||
// ── confirmation ─────────────────────────────────────────────────────────────
|
||||
// Journal states whose content can still be moved back; `complete` is not one of
|
||||
// them (photo_pipeline/services/rename_journal.py: complete is terminal).
|
||||
const REVERSIBLE = new Set(["moved", "database_updated", "verified", "rollback_required"]);
|
||||
|
||||
function rollbackable(plan) {
|
||||
return plan.operations.some((operation) => REVERSIBLE.has(operation.journal_state));
|
||||
}
|
||||
|
||||
function confirmBlock(plan, blocked) {
|
||||
const applyable = plan.applicable && !blocked;
|
||||
return el(
|
||||
"div",
|
||||
{ class: "card", "data-testid": "confirm" },
|
||||
el("h2", {}, "Confirm"),
|
||||
// The token is shown, not just sent: a confirmation the user cannot see is a
|
||||
// confirmation they cannot check against what the preview claimed.
|
||||
el(
|
||||
"p",
|
||||
{ class: "muted", "data-testid": "confirm-token" },
|
||||
`Plan ${plan.id} · version ${plan.version} · checksum ${plan.checksum.slice(0, 12)}`
|
||||
),
|
||||
el(
|
||||
"p",
|
||||
{ "data-testid": "cancel-note" },
|
||||
"Once confirmed, the run cannot be cancelled part-way: each folder is moved, " +
|
||||
"verified, and recorded before the next one starts. An interruption is " +
|
||||
"recoverable from the journal — it is never left half-applied."
|
||||
),
|
||||
el(
|
||||
"div",
|
||||
{ class: "toolbar" },
|
||||
el(
|
||||
"button",
|
||||
{
|
||||
class: "primary",
|
||||
"data-testid": "apply-plan",
|
||||
disabled: applyable ? false : "disabled",
|
||||
title: blocked
|
||||
? BLOCKED_TITLE
|
||||
: plan.applicable
|
||||
? false
|
||||
: `plan is ${plan.state} — it cannot be applied`,
|
||||
onclick: () =>
|
||||
run(
|
||||
() =>
|
||||
api.applyPlan(plan.id, {
|
||||
expected_version: plan.version,
|
||||
expected_checksum: plan.checksum,
|
||||
}),
|
||||
`Applying ${plan.operation_count} rename(s)…`
|
||||
),
|
||||
},
|
||||
`Apply ${plan.operation_count} rename(s)`
|
||||
),
|
||||
// Rollback is recovery, not undo: a completed rename is terminal and stays
|
||||
// that way. The button appears only while operations are still reversible.
|
||||
rollbackable(plan)
|
||||
? el(
|
||||
"button",
|
||||
{
|
||||
"data-testid": "rollback-plan",
|
||||
onclick: () => run(() => api.rollbackPlan(plan.id), "Rolling back…"),
|
||||
},
|
||||
"Roll back the unfinished moves"
|
||||
)
|
||||
: null
|
||||
),
|
||||
el("p", { role: "status", "aria-live": "polite", "data-testid": "apply-progress" }, "")
|
||||
);
|
||||
}
|
||||
|
||||
// Runs a mutating call, showing progress while it is in flight and leaving the
|
||||
// result — including a stale-confirmation conflict — visible afterwards.
|
||||
async function run(action, progressText) {
|
||||
const progress = document.querySelector('[data-testid="apply-progress"]');
|
||||
if (progress) progress.textContent = progressText;
|
||||
try {
|
||||
outcome = { kind: "ok", result: await action() };
|
||||
} catch (error) {
|
||||
outcome = error.status === 409 ? { kind: "conflict" } : { kind: "error", error };
|
||||
}
|
||||
render();
|
||||
}
|
||||
|
||||
function outcomeBanner() {
|
||||
if (!outcome) return null;
|
||||
if (outcome.kind === "conflict") {
|
||||
return el(
|
||||
"div",
|
||||
{ class: "alert", role: "alert", "data-testid": "conflict" },
|
||||
"This plan changed on the server since it was loaded. Nothing was applied; " +
|
||||
"the preview below is the current state — review it and confirm again."
|
||||
);
|
||||
}
|
||||
if (outcome.kind === "error") {
|
||||
return el(
|
||||
"div",
|
||||
{ class: "alert", role: "alert", "data-testid": "apply-error" },
|
||||
`Failed: ${outcome.error.message}`
|
||||
);
|
||||
}
|
||||
const result = outcome.result;
|
||||
if (result.applied === undefined) return null; // a build or recovery call
|
||||
return el(
|
||||
"div",
|
||||
{ class: "alert", role: "status", "data-testid": "apply-result" },
|
||||
`Applied ${result.applied}, failed ${result.failed}, skipped ${result.skipped}. ` +
|
||||
`The plan is now ${result.state}.`
|
||||
);
|
||||
}
|
||||
|
||||
// ── recovery ─────────────────────────────────────────────────────────────────
|
||||
function recoveryPanel(recovery) {
|
||||
if (!recovery.items.length) return null;
|
||||
const recoverable = recovery.items.filter((item) => item.classification !== "manual");
|
||||
return el(
|
||||
"div",
|
||||
{ class: "alert", role: "alert", "data-testid": "recovery-banner" },
|
||||
el(
|
||||
"strong",
|
||||
{},
|
||||
recovery.blocks_mutation
|
||||
? "A rename was interrupted — other changes are paused"
|
||||
: "A rename is unresolved"
|
||||
),
|
||||
el(
|
||||
"p",
|
||||
{ "data-testid": "recovery-explanation" },
|
||||
recovery.blocks_mutation
|
||||
? "Some folders on disk may disagree with the database. Planning, applying, and " +
|
||||
"album-name changes stay blocked until this is resolved; browsing is unaffected."
|
||||
: "These operations never reached a terminal state."
|
||||
),
|
||||
el(
|
||||
"ul",
|
||||
{},
|
||||
...recovery.items.map((item) =>
|
||||
el(
|
||||
"li",
|
||||
{ "data-testid": "recovery-item", "data-classification": item.classification },
|
||||
`${item.album}: ${item.journal_state} — ${item.classification} — ${item.reason} ` +
|
||||
`(${item.source_path} → ${item.destination_path})`
|
||||
)
|
||||
)
|
||||
),
|
||||
// Only evidence-safe work is offered. Ambiguous operations get no button at
|
||||
// all — an action the server would refuse must not look available.
|
||||
recoverable.length
|
||||
? el(
|
||||
"button",
|
||||
{
|
||||
"data-testid": "resolve-recovery",
|
||||
onclick: () => run(() => api.resolveRecovery(), "Resolving…"),
|
||||
},
|
||||
`Resolve ${recoverable.length} recoverable rename(s) from evidence`
|
||||
)
|
||||
: el(
|
||||
"p",
|
||||
{ "data-testid": "manual-only" },
|
||||
"No automatic action is safe here: source and destination both exist, or neither " +
|
||||
"does. Inspect the paths above and resolve them by hand."
|
||||
)
|
||||
);
|
||||
}
|
||||
@@ -327,11 +327,15 @@ export async function renderStats(root) {
|
||||
// Approving never renames anything — it records the approved name only.
|
||||
export async function renderAlbums(root, params = {}) {
|
||||
setActiveNav("albums");
|
||||
let evidence, proposals;
|
||||
let evidence, proposals, recovery;
|
||||
try {
|
||||
[evidence, proposals] = await Promise.all([
|
||||
[evidence, proposals, recovery] = await Promise.all([
|
||||
api.albumEvidence({ limit: 500 }),
|
||||
api.albumProposals(),
|
||||
// An unresolved rename means folder paths on disk may disagree with the
|
||||
// database, so the server refuses name changes until it is resolved. Ask up
|
||||
// front rather than offering a button that is guaranteed to fail.
|
||||
api.renameRecovery(),
|
||||
]);
|
||||
} catch (error) {
|
||||
root.replaceChildren(errorBanner(`Failed to load albums: ${error.message}`));
|
||||
@@ -370,17 +374,27 @@ export async function renderAlbums(root, params = {}) {
|
||||
})
|
||||
);
|
||||
|
||||
const renameBlocked = recovery.blocks_mutation;
|
||||
const detail = selected
|
||||
? albumDetail(
|
||||
root,
|
||||
folders.find((folder) => folder.album === selected),
|
||||
byAlbum.get(selected),
|
||||
params
|
||||
params,
|
||||
renameBlocked
|
||||
)
|
||||
: el("p", { class: "muted" }, "No albums with evidence yet.");
|
||||
|
||||
root.replaceChildren(
|
||||
el("h1", {}, "Albums"),
|
||||
renameBlocked
|
||||
? el(
|
||||
"div",
|
||||
{ class: "alert", role: "alert", "data-testid": "rename-blocked" },
|
||||
"An interrupted rename is unresolved. Name changes are paused until it is " +
|
||||
"resolved on the Renames page; browsing stays available."
|
||||
)
|
||||
: null,
|
||||
el(
|
||||
"div",
|
||||
{ class: "toolbar" },
|
||||
@@ -389,6 +403,8 @@ export async function renderAlbums(root, params = {}) {
|
||||
{
|
||||
class: "primary",
|
||||
"data-testid": "generate-proposals",
|
||||
disabled: renameBlocked ? "disabled" : false,
|
||||
title: renameBlocked ? "an unresolved rename must be resolved first" : false,
|
||||
onclick: async () => {
|
||||
try {
|
||||
await api.generateProposals({});
|
||||
@@ -405,7 +421,7 @@ export async function renderAlbums(root, params = {}) {
|
||||
);
|
||||
}
|
||||
|
||||
function albumDetail(root, folder, proposal, params) {
|
||||
function albumDetail(root, folder, proposal, params, renameBlocked = false) {
|
||||
if (!folder) return el("p", { class: "muted" }, "Album not found.");
|
||||
|
||||
const evidenceBlock = el(
|
||||
@@ -473,8 +489,13 @@ function albumDetail(root, folder, proposal, params) {
|
||||
{
|
||||
class: "primary",
|
||||
"data-testid": "approve",
|
||||
disabled: proposal.stale || proposal.status === "error" ? "disabled" : false,
|
||||
title: proposal.stale ? "evidence changed — regenerate first" : false,
|
||||
disabled:
|
||||
renameBlocked || proposal.stale || proposal.status === "error" ? "disabled" : false,
|
||||
title: renameBlocked
|
||||
? "an unresolved rename must be resolved first"
|
||||
: 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"
|
||||
|
||||
@@ -17,6 +17,7 @@ from photo_pipeline.schemas import (
|
||||
)
|
||||
from photo_pipeline.services.albums import AlbumService
|
||||
from photo_pipeline.services.proposals import ConflictError, ProposalError, ProposalService
|
||||
from photo_pipeline.services.rename_journal import RenameJournal
|
||||
|
||||
router = APIRouter(tags=["albums"])
|
||||
|
||||
@@ -37,6 +38,22 @@ def _error(status: int, code: str, message: str) -> JSONResponse:
|
||||
return JSONResponse(status_code=status, content={"error": {"code": code, "message": message}})
|
||||
|
||||
|
||||
def _unresolved_rename(request: Request) -> JSONResponse | None:
|
||||
"""Refuse work that feeds renaming while the library may be half-renamed.
|
||||
|
||||
An unresolved rename means some folder paths on disk disagree with the database,
|
||||
so evidence gathered now — and any approval based on it — could describe a
|
||||
location that no longer exists. Reading stays available; only mutation stops.
|
||||
"""
|
||||
if RenameJournal(request.app.state.session_factory).blocks_mutation():
|
||||
return _error(
|
||||
409,
|
||||
"rename_recovery_required",
|
||||
"an unresolved rename is in progress; resolve it before changing album names",
|
||||
)
|
||||
return None
|
||||
|
||||
|
||||
@router.get("/albums/evidence")
|
||||
def evidence(
|
||||
request: Request,
|
||||
@@ -47,8 +64,8 @@ def evidence(
|
||||
|
||||
|
||||
@router.post("/albums/proposals")
|
||||
def generate(body: GenerateProposalsRequest, request: Request) -> dict:
|
||||
return _proposals(request).generate(body.albums)
|
||||
def generate(body: GenerateProposalsRequest, request: Request):
|
||||
return _unresolved_rename(request) or _proposals(request).generate(body.albums)
|
||||
|
||||
|
||||
@router.get("/albums/proposals")
|
||||
@@ -76,6 +93,9 @@ def edit_proposal(album: str, body: EditProposalRequest, request: Request):
|
||||
|
||||
@router.post("/albums/proposals/{album:path}/approve")
|
||||
def approve_proposal(album: str, body: ApproveProposalRequest, request: Request):
|
||||
blocked = _unresolved_rename(request)
|
||||
if blocked is not None:
|
||||
return blocked
|
||||
try:
|
||||
return _proposals(request).approve(album, expected_version=body.expected_version)
|
||||
except ConflictError as error:
|
||||
|
||||
@@ -229,6 +229,12 @@ class RenameJournal:
|
||||
classification, reason = _classify(row["journal_state"], source_exists, destination_exists)
|
||||
return {
|
||||
"operation_id": operation_id,
|
||||
"plan_id": row["plan_id"],
|
||||
"album": row["album"],
|
||||
# The paths travel with the verdict so a reviewer can see which folder is
|
||||
# unresolved without correlating an opaque operation id by hand.
|
||||
"source_path": row["source_path"],
|
||||
"destination_path": row["destination_path"],
|
||||
"journal_state": row["journal_state"],
|
||||
"classification": classification,
|
||||
"reason": reason,
|
||||
|
||||
@@ -418,8 +418,17 @@ def _plan_dict(plan: RenamePlan, operations: list[RenameOperation]) -> dict:
|
||||
"asset_count": operation.asset_count,
|
||||
"same_filesystem": operation.same_filesystem,
|
||||
"case_only": operation.case_only,
|
||||
"issues": json.loads(operation.issues or "[]"),
|
||||
# Severity is decided here, not in the browser: a client that has to
|
||||
# keep its own copy of BLOCKING_CODES will eventually disagree with
|
||||
# the validator about whether a plan can run.
|
||||
"issues": [
|
||||
{**issue, "blocking": issue["code"] in BLOCKING_CODES}
|
||||
for issue in json.loads(operation.issues or "[]")
|
||||
],
|
||||
"journal_state": operation.journal_state,
|
||||
"verified_at": operation.verified_at.isoformat() if operation.verified_at else None,
|
||||
"error_code": operation.error_code,
|
||||
"error_message": operation.error_message,
|
||||
}
|
||||
for operation in operations
|
||||
],
|
||||
|
||||
@@ -30,4 +30,5 @@ testpaths = ["tests"]
|
||||
markers = [
|
||||
"phase_b: Phase B end-to-end acceptance (US02-07) — API, worker-recovery, and browser journeys",
|
||||
"phase_c: Phase C end-to-end acceptance (US03-05) — album proposal API and browser journeys",
|
||||
"phase_d: Phase D end-to-end acceptance (US04-06) — guarded rename API, fault, and browser journeys",
|
||||
]
|
||||
|
||||
359
tests/e2e/test_renames_ui.py
Normal file
359
tests/e2e/test_renames_ui.py
Normal file
@@ -0,0 +1,359 @@
|
||||
"""Browser journeys for the rename view (US04-05).
|
||||
|
||||
Covers previewing every affected path, confirming with the server-issued token,
|
||||
applying with visible progress and terminal verification, refusing a stale
|
||||
confirmation, previewing a collision as blocking, and resolving an interrupted
|
||||
rename — including the check that an unresolved rename really does block unrelated
|
||||
mutations at the API, not just in the UI.
|
||||
|
||||
Renames really happen here: the assertions read the filesystem afterwards.
|
||||
"""
|
||||
|
||||
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
|
||||
|
||||
# Part of the Phase D acceptance command (US04-06); mapped to US04-05 for traceability.
|
||||
pytestmark = pytest.mark.phase_d
|
||||
|
||||
NOW = datetime(2026, 1, 1, tzinfo=timezone.utc)
|
||||
TIMEOUT = 10
|
||||
APPROVED = "2019 Rome"
|
||||
|
||||
|
||||
def _seed(tmp_path):
|
||||
"""A library with one album ("rome") whose two photos are SFW and analysed."""
|
||||
seeded = seed_library(tmp_path, {}, {})
|
||||
album = seeded.lib / "rome"
|
||||
album.mkdir()
|
||||
image(album / "a.jpg", 1)
|
||||
image(album / "b.jpg", 2)
|
||||
|
||||
from sqlalchemy import select
|
||||
|
||||
from photo_pipeline.models import AnalysisResult, Asset, SafetyReview
|
||||
from photo_pipeline.services.inventory import InventoryService
|
||||
|
||||
with _factory(seeded) as sf:
|
||||
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()
|
||||
return seeded
|
||||
|
||||
|
||||
class _factory:
|
||||
"""Session factory against the seeded database, for the few things a test has to
|
||||
set up or inspect below the API (journal states, mainly)."""
|
||||
|
||||
def __init__(self, seeded):
|
||||
self._seeded = seeded
|
||||
|
||||
def __enter__(self):
|
||||
from photo_pipeline.config import Config
|
||||
from photo_pipeline.db import create_db_engine, create_session_factory, run_migrations
|
||||
|
||||
config = Config.from_env(
|
||||
{
|
||||
"PHOTO_PIPELINE_DATA_DIR": str(self._seeded.data),
|
||||
"PHOTO_PIPELINE_LIBRARY_ROOTS": str(self._seeded.lib),
|
||||
}
|
||||
)
|
||||
run_migrations(config.database_url)
|
||||
self._engine = create_db_engine(config.database_url)
|
||||
return create_session_factory(self._engine)
|
||||
|
||||
def __exit__(self, *_):
|
||||
self._engine.dispose()
|
||||
return False
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def server(tmp_path):
|
||||
seeded = _seed(tmp_path)
|
||||
running = Server(seeded).start()
|
||||
running.seeded = seeded
|
||||
try:
|
||||
yield running
|
||||
finally:
|
||||
running.stop()
|
||||
|
||||
|
||||
# ── helpers ──────────────────────────────────────────────────────────────────
|
||||
|
||||
|
||||
def _approve(base, *, album="rome", name=APPROVED):
|
||||
"""Generate a proposal, set its final name, and approve it — the state a rename
|
||||
plan is built from."""
|
||||
httpx.post(f"{base}/api/v1/albums/proposals", json={}, timeout=TIMEOUT).raise_for_status()
|
||||
current = httpx.get(f"{base}/api/v1/albums/proposals/{album}", timeout=TIMEOUT).json()
|
||||
httpx.post(
|
||||
f"{base}/api/v1/albums/proposals/{album}/edit",
|
||||
json={"name": name, "expected_version": current["version"]},
|
||||
timeout=TIMEOUT,
|
||||
).raise_for_status()
|
||||
current = httpx.get(f"{base}/api/v1/albums/proposals/{album}", timeout=TIMEOUT).json()
|
||||
httpx.post(
|
||||
f"{base}/api/v1/albums/proposals/{album}/approve",
|
||||
json={"expected_version": current["version"]},
|
||||
timeout=TIMEOUT,
|
||||
).raise_for_status()
|
||||
|
||||
|
||||
def _build(base):
|
||||
response = httpx.post(f"{base}/api/v1/rename-plans", timeout=TIMEOUT)
|
||||
response.raise_for_status()
|
||||
return response.json()
|
||||
|
||||
|
||||
def _open_plan(page, server):
|
||||
"""Approve a name, open the view, and build the plan through the UI."""
|
||||
_approve(server.base)
|
||||
page.goto(f"{server.base}/app/#/renames")
|
||||
page.get_by_test_id("build-plan").click()
|
||||
page.get_by_test_id("operations").wait_for()
|
||||
|
||||
|
||||
def _interrupt(seeded, plan, *, state="moving", make_destination=False):
|
||||
"""Leave an operation in a non-terminal journal state, as a crash mid-apply would.
|
||||
|
||||
``make_destination`` also creates the destination folder, so source and
|
||||
destination both exist and the evidence becomes ambiguous (``manual``).
|
||||
"""
|
||||
from photo_pipeline.services.rename_journal import RenameJournal
|
||||
|
||||
with _factory(seeded) as sf:
|
||||
journal = RenameJournal(sf)
|
||||
operation = journal.operations(plan["id"])[0]
|
||||
journal.begin(operation["id"], worker_id="crashed", fencing_token=1)
|
||||
if state != "moving":
|
||||
journal.transition(operation["id"], state, fencing_token=1)
|
||||
if make_destination:
|
||||
(seeded.lib / APPROVED).mkdir(exist_ok=True)
|
||||
|
||||
|
||||
# ── preview ──────────────────────────────────────────────────────────────────
|
||||
|
||||
|
||||
def test_preview_shows_every_path_operation_type_and_count(page, server):
|
||||
errors = []
|
||||
page.on("console", lambda m: errors.append(m.text) if m.type == "error" else None)
|
||||
_open_plan(page, server)
|
||||
|
||||
expect(page.get_by_test_id("plan-state")).to_have_text("validated")
|
||||
expect(page.get_by_test_id("operation-count")).to_have_text("1 folder")
|
||||
expect(page.get_by_test_id("affected-assets")).to_have_text("2 photos")
|
||||
|
||||
row = page.get_by_test_id("operation-row").first
|
||||
# Whole paths, both sides — nothing about the move is left to be inferred.
|
||||
assert row.get_by_test_id("op-source").inner_text().endswith("/rome")
|
||||
assert row.get_by_test_id("op-destination").inner_text().endswith(f"/{APPROVED}")
|
||||
expect(row.get_by_test_id("op-type")).to_have_text("rename")
|
||||
expect(row.get_by_test_id("op-count")).to_have_text("2")
|
||||
expect(row.get_by_test_id("op-state")).to_have_text("planned")
|
||||
assert errors == [], f"console errors: {errors}"
|
||||
|
||||
|
||||
def test_confirmation_shows_the_server_token_and_the_cancellation_limit(page, server):
|
||||
_open_plan(page, server)
|
||||
plan = httpx.get(f"{server.base}/api/v1/rename-plans", timeout=TIMEOUT).json()["items"][0]
|
||||
|
||||
token = page.get_by_test_id("confirm-token").inner_text()
|
||||
assert plan["id"] in token and f"version {plan['version']}" in token
|
||||
assert plan["checksum"][:12] in token
|
||||
# The limit on cancellation is stated before the button, not discovered after.
|
||||
assert "cannot be cancelled" in page.get_by_test_id("cancel-note").inner_text()
|
||||
|
||||
|
||||
def test_a_collision_is_previewed_as_blocking_and_cannot_be_applied(page, server):
|
||||
_approve(server.base)
|
||||
# Someone else already owns the destination name, with content worth keeping.
|
||||
(server.seeded.lib / APPROVED).mkdir()
|
||||
(server.seeded.lib / APPROVED / "precious.jpg").write_bytes(b"do not lose me")
|
||||
|
||||
page.goto(f"{server.base}/app/#/renames")
|
||||
page.get_by_test_id("build-plan").click()
|
||||
page.get_by_test_id("operations").wait_for()
|
||||
|
||||
expect(page.get_by_test_id("plan-state")).to_have_text("invalid")
|
||||
expect(page.get_by_test_id("plan-blockers")).to_contain_text("destination_exists")
|
||||
issue = page.get_by_test_id("op-issue").first
|
||||
expect(issue).to_have_attribute("data-code", "destination_exists")
|
||||
assert issue.inner_text().startswith("blocks")
|
||||
expect(page.get_by_test_id("apply-plan")).to_be_disabled()
|
||||
|
||||
assert (server.seeded.lib / APPROVED / "precious.jpg").read_bytes() == b"do not lose me"
|
||||
assert (server.seeded.lib / "rome").is_dir()
|
||||
|
||||
|
||||
# ── apply ────────────────────────────────────────────────────────────────────
|
||||
|
||||
|
||||
def test_apply_reports_progress_terminal_verification_and_moves_the_folder(page, server):
|
||||
_open_plan(page, server)
|
||||
# The progress region is live before anything runs, so the status it announces
|
||||
# during the run reaches assistive technology.
|
||||
progress = page.get_by_test_id("apply-progress")
|
||||
expect(progress).to_have_attribute("aria-live", "polite")
|
||||
|
||||
page.get_by_test_id("apply-plan").click()
|
||||
|
||||
expect(page.get_by_test_id("apply-result")).to_contain_text("Applied 1, failed 0, skipped 0")
|
||||
expect(page.get_by_test_id("plan-state")).to_have_text("applied")
|
||||
row = page.get_by_test_id("operation-row").first
|
||||
expect(row.get_by_test_id("op-state")).to_have_text("complete")
|
||||
expect(row.get_by_test_id("op-verified")).to_be_visible()
|
||||
|
||||
assert (server.seeded.lib / APPROVED / "a.jpg").exists()
|
||||
assert not (server.seeded.lib / "rome").exists()
|
||||
|
||||
|
||||
def test_a_stale_confirmation_is_refused_and_explained(page, server):
|
||||
_open_plan(page, server)
|
||||
# Another client applies first, which bumps the plan version this page is holding.
|
||||
plan = httpx.get(f"{server.base}/api/v1/rename-plans", timeout=TIMEOUT).json()["items"][0]
|
||||
httpx.post(
|
||||
f"{server.base}/api/v1/rename-plans/{plan['id']}/apply",
|
||||
json={"expected_version": plan["version"]},
|
||||
timeout=TIMEOUT,
|
||||
).raise_for_status()
|
||||
|
||||
page.get_by_test_id("apply-plan").click()
|
||||
|
||||
expect(page.get_by_test_id("conflict")).to_be_visible()
|
||||
# The refusal is not a second run: the page now shows the server's real state.
|
||||
expect(page.get_by_test_id("plan-state")).to_have_text("applied")
|
||||
expect(page.get_by_test_id("apply-plan")).to_be_disabled()
|
||||
|
||||
|
||||
def test_the_applied_plan_survives_a_reload(page, server):
|
||||
_open_plan(page, server)
|
||||
page.get_by_test_id("apply-plan").click()
|
||||
expect(page.get_by_test_id("plan-state")).to_have_text("applied")
|
||||
|
||||
page.reload()
|
||||
expect(page.get_by_test_id("plan-state")).to_have_text("applied")
|
||||
expect(page.get_by_test_id("operation-row").first.get_by_test_id("op-state")).to_have_text(
|
||||
"complete"
|
||||
)
|
||||
# The result banner is tab state, not server state, so it correctly does not return.
|
||||
expect(page.get_by_test_id("apply-result")).to_have_count(0)
|
||||
|
||||
|
||||
def test_apply_can_be_confirmed_from_the_keyboard(page, server):
|
||||
_open_plan(page, server)
|
||||
page.get_by_test_id("apply-plan").focus()
|
||||
page.keyboard.press("Enter")
|
||||
expect(page.get_by_test_id("plan-state")).to_have_text("applied")
|
||||
assert (server.seeded.lib / APPROVED).is_dir()
|
||||
|
||||
|
||||
def test_a_finished_rename_offers_no_rollback(page, server):
|
||||
"""Rollback is recovery, not undo: once an operation is `complete` the journal
|
||||
treats it as terminal, so the view must not suggest it can be reversed."""
|
||||
_open_plan(page, server)
|
||||
page.get_by_test_id("apply-plan").click()
|
||||
expect(page.get_by_test_id("plan-state")).to_have_text("applied")
|
||||
expect(page.get_by_test_id("rollback-plan")).to_have_count(0)
|
||||
|
||||
|
||||
def test_rollback_returns_an_unfinished_move_to_its_source(page, server):
|
||||
_approve(server.base)
|
||||
plan = _build(server.base)
|
||||
# A crash after the move but before the database caught up: the content sits at
|
||||
# the destination and the operation is still reversible.
|
||||
_interrupt(server.seeded, plan, state="moving")
|
||||
(server.seeded.lib / "rome").rename(server.seeded.lib / APPROVED)
|
||||
with _factory(server.seeded) as sf:
|
||||
from photo_pipeline.services.rename_journal import RenameJournal
|
||||
|
||||
journal = RenameJournal(sf)
|
||||
journal.transition(journal.operations(plan["id"])[0]["id"], "moved", fencing_token=1)
|
||||
|
||||
page.goto(f"{server.base}/app/#/renames")
|
||||
page.get_by_test_id("rollback-plan").click()
|
||||
|
||||
expect(page.get_by_test_id("plan-state")).to_have_text("rolled_back")
|
||||
assert (server.seeded.lib / "rome" / "a.jpg").exists()
|
||||
assert not (server.seeded.lib / APPROVED).exists()
|
||||
|
||||
|
||||
# ── recovery ─────────────────────────────────────────────────────────────────
|
||||
|
||||
|
||||
def test_an_interrupted_rename_is_shown_and_offers_the_safe_action(page, server):
|
||||
_approve(server.base)
|
||||
plan = _build(server.base)
|
||||
_interrupt(server.seeded, plan) # source intact, destination free → resumable
|
||||
|
||||
page.goto(f"{server.base}/app/#/renames")
|
||||
banner = page.get_by_test_id("recovery-banner")
|
||||
banner.wait_for()
|
||||
expect(page.get_by_test_id("recovery-item")).to_have_attribute(
|
||||
"data-classification", "resumable"
|
||||
)
|
||||
# While it is unresolved nothing else may mutate the library.
|
||||
expect(page.get_by_test_id("build-plan")).to_be_disabled()
|
||||
expect(page.get_by_test_id("apply-plan")).to_be_disabled()
|
||||
|
||||
page.get_by_test_id("resolve-recovery").click()
|
||||
expect(banner).to_have_count(0)
|
||||
expect(page.get_by_test_id("build-plan")).to_be_enabled()
|
||||
|
||||
|
||||
def test_ambiguous_evidence_offers_no_automatic_action(page, server):
|
||||
_approve(server.base)
|
||||
plan = _build(server.base)
|
||||
# Both paths exist: nothing can tell which side holds the truth.
|
||||
_interrupt(server.seeded, plan, make_destination=True)
|
||||
|
||||
page.goto(f"{server.base}/app/#/renames")
|
||||
page.get_by_test_id("recovery-banner").wait_for()
|
||||
expect(page.get_by_test_id("recovery-item")).to_have_attribute("data-classification", "manual")
|
||||
expect(page.get_by_test_id("manual-only")).to_be_visible()
|
||||
# No button is offered for work the server would refuse to do.
|
||||
expect(page.get_by_test_id("resolve-recovery")).to_have_count(0)
|
||||
|
||||
|
||||
def test_an_unresolved_rename_blocks_unrelated_name_changes(page, server):
|
||||
_approve(server.base)
|
||||
plan = _build(server.base)
|
||||
_interrupt(server.seeded, plan, make_destination=True)
|
||||
|
||||
page.goto(f"{server.base}/app/#/albums?album=rome")
|
||||
expect(page.get_by_test_id("rename-blocked")).to_be_visible()
|
||||
expect(page.get_by_test_id("approve")).to_be_disabled()
|
||||
expect(page.get_by_test_id("generate-proposals")).to_be_disabled()
|
||||
|
||||
# The disabled buttons are not the only guard: the API refuses too.
|
||||
current = httpx.get(f"{server.base}/api/v1/albums/proposals/rome", timeout=TIMEOUT).json()
|
||||
refused = httpx.post(
|
||||
f"{server.base}/api/v1/albums/proposals/rome/approve",
|
||||
json={"expected_version": current["version"]},
|
||||
timeout=TIMEOUT,
|
||||
)
|
||||
assert refused.status_code == 409
|
||||
assert refused.json()["error"]["code"] == "rename_recovery_required"
|
||||
@@ -97,6 +97,9 @@
|
||||
],
|
||||
"US04-04": [
|
||||
"tests/integration/test_rename_recovery.py"
|
||||
],
|
||||
"US04-05": [
|
||||
"tests/e2e/test_renames_ui.py"
|
||||
]
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user