fix(api,web): movie delete always removes files
Drop the delete_files choice: removing a movie now always unlinks
its §7.4 library folder. Removes the query flag from DELETE
/api/movies/{id} and the OpenAPI params, and the toggle from the UI
confirmation panel — the confirmation itself stays. Path-safety and
torrent-untouched behaviour are unchanged.
Closes #110
This commit is contained in:
@@ -23,8 +23,7 @@ use utoipa_scalar::{Scalar, Servable};
|
||||
|
||||
pub use health::{Check, Health, HealthReport, Status};
|
||||
pub use movies::{
|
||||
Accepted, AttentionQueues, CreateMovie, DeleteMovieQuery, ErrorBody, Movie, MovieFile, Release,
|
||||
UpdateMovie,
|
||||
Accepted, AttentionQueues, CreateMovie, ErrorBody, Movie, MovieFile, Release, UpdateMovie,
|
||||
};
|
||||
pub use owners::{CreateOwner, Owner, UpdateOwner};
|
||||
pub use roots::Root;
|
||||
|
||||
@@ -343,18 +343,9 @@ pub async fn update(
|
||||
Ok(Json(load_movie(&state, id).await?))
|
||||
}
|
||||
|
||||
#[derive(Debug, Deserialize, IntoParams)]
|
||||
pub struct DeleteMovieQuery {
|
||||
/// Also remove the title's folder under its root (§7.4). Off unless the
|
||||
/// caller says otherwise: dropping the row is reversible, unlinking
|
||||
/// 40 GB is not.
|
||||
#[serde(default)]
|
||||
pub delete_files: bool,
|
||||
}
|
||||
|
||||
#[utoipa::path(
|
||||
delete, path = "/api/movies/{movie_id}", tag = "movies",
|
||||
params(("movie_id" = i64, Path, description = "Movie row id"), DeleteMovieQuery),
|
||||
params(("movie_id" = i64, Path, description = "Movie row id")),
|
||||
responses(
|
||||
(status = 204),
|
||||
(status = 404, body = ErrorBody),
|
||||
@@ -366,14 +357,11 @@ pub struct DeleteMovieQuery {
|
||||
pub async fn delete(
|
||||
State(state): State<AppState>,
|
||||
Path(id): Path<i64>,
|
||||
Query(query): Query<DeleteMovieQuery>,
|
||||
) -> Result<StatusCode, ApiError> {
|
||||
// The row is loaded first so a missing movie is 404 before anything
|
||||
// touches the disk.
|
||||
load_movie(&state, id).await?;
|
||||
if query.delete_files {
|
||||
remove_library_files(&state, id).await?;
|
||||
}
|
||||
remove_library_files(&state, id).await?;
|
||||
// `media_files.path` is UNIQUE and the owner is polymorphic, so nothing
|
||||
// cascades: leaving the rows behind would block re-importing the same
|
||||
// path after a re-add. Owner tags go with the title they tagged.
|
||||
@@ -838,7 +826,7 @@ mod tests {
|
||||
/// The §7.4 folder is the unit of deletion, so the sidecar goes with the
|
||||
/// feature — and the root itself is never touched.
|
||||
#[tokio::test]
|
||||
async fn delete_files_removes_the_whole_title_folder() {
|
||||
async fn deleting_a_movie_removes_the_whole_title_folder() {
|
||||
let (_dir, state, base) = application().await;
|
||||
let movie = add_movie(&base, 693_134, 1).await;
|
||||
let id = movie["id"].as_i64().expect("id");
|
||||
@@ -846,7 +834,7 @@ mod tests {
|
||||
let folder = library_on_disk(&state, id, root.path()).await;
|
||||
|
||||
let response = reqwest::Client::new()
|
||||
.delete(format!("{base}/api/movies/{id}?delete_files=true"))
|
||||
.delete(format!("{base}/api/movies/{id}"))
|
||||
.send()
|
||||
.await
|
||||
.expect("delete");
|
||||
@@ -867,32 +855,6 @@ mod tests {
|
||||
assert_eq!(orphans, 0, "the file rows go with the files");
|
||||
}
|
||||
|
||||
/// Default off (§7.4 in the issue): removing a title from the library is
|
||||
/// not the same decision as unlinking 40 GB.
|
||||
#[tokio::test]
|
||||
async fn delete_without_the_flag_leaves_the_files_on_disk() {
|
||||
let (_dir, state, base) = application().await;
|
||||
let movie = add_movie(&base, 693_134, 1).await;
|
||||
let id = movie["id"].as_i64().expect("id");
|
||||
let root = tempfile::tempdir().expect("root");
|
||||
let folder = library_on_disk(&state, id, root.path()).await;
|
||||
|
||||
let response = reqwest::Client::new()
|
||||
.delete(format!("{base}/api/movies/{id}"))
|
||||
.send()
|
||||
.await
|
||||
.expect("delete");
|
||||
assert_eq!(response.status(), StatusCode::NO_CONTENT);
|
||||
assert!(folder.exists(), "the files stay until asked for");
|
||||
assert_eq!(
|
||||
reqwest::get(format!("{base}/api/movies/{id}"))
|
||||
.await
|
||||
.expect("get deleted")
|
||||
.status(),
|
||||
StatusCode::NOT_FOUND
|
||||
);
|
||||
}
|
||||
|
||||
/// The guard that keeps a delete inside the library: a path that is not
|
||||
/// under the title's root is left alone, whatever the row says.
|
||||
#[tokio::test]
|
||||
@@ -921,7 +883,7 @@ mod tests {
|
||||
.expect("media file");
|
||||
|
||||
let response = reqwest::Client::new()
|
||||
.delete(format!("{base}/api/movies/{id}?delete_files=true"))
|
||||
.delete(format!("{base}/api/movies/{id}"))
|
||||
.send()
|
||||
.await
|
||||
.expect("delete");
|
||||
|
||||
+13
-39
@@ -1026,7 +1026,8 @@ function releasesMain(): ReleasesView {
|
||||
removeWrap.replaceChildren(panel);
|
||||
removeWrap.hidden = false;
|
||||
remove.setAttribute("aria-expanded", "true");
|
||||
panel.querySelector<HTMLElement>(".remove-files")?.focus();
|
||||
// cancel takes focus, not the destructive action: no toggle is left to arm first
|
||||
panel.querySelector<HTMLElement>(".remove-actions .control:last-child")?.focus();
|
||||
}
|
||||
|
||||
remove.addEventListener("click", () => {
|
||||
@@ -1133,8 +1134,8 @@ interface RemoveActions {
|
||||
}
|
||||
|
||||
/**
|
||||
* The removal confirmation (issue 104). Two decisions, not one: the library
|
||||
* entry always goes, the files only when asked, and the toggle starts off.
|
||||
* The removal confirmation (issue 104, simplified by 110): removing a title
|
||||
* always unlinks its §7.4 folder, so there is one decision, not two.
|
||||
*
|
||||
* It names the §7.4 folder it would unlink rather than promising in the
|
||||
* abstract — the service knows only what it wrote (§2), so the file list is
|
||||
@@ -1147,22 +1148,8 @@ function removePanel(movie: LibraryMovie, actions: RemoveActions): HTMLElement {
|
||||
panel.setAttribute("role", "group");
|
||||
panel.setAttribute("aria-label", `remove ${movie.title}`);
|
||||
|
||||
let deleteFiles = false;
|
||||
let files: MovieFile[] | null = null;
|
||||
|
||||
const toggle = document.createElement("button");
|
||||
toggle.type = "button";
|
||||
toggle.className = "remove-files";
|
||||
toggle.disabled = true;
|
||||
toggle.setAttribute("aria-pressed", "false");
|
||||
const toggleName = document.createElement("span");
|
||||
toggleName.className = "remove-files-name";
|
||||
toggleName.textContent = "delete files from disk";
|
||||
const toggleWord = document.createElement("span");
|
||||
toggleWord.className = "remove-files-word readout";
|
||||
toggleWord.textContent = "off";
|
||||
toggle.append(toggleName, toggleWord);
|
||||
|
||||
const evidence = document.createElement("p");
|
||||
evidence.className = "remove-evidence readout dim";
|
||||
evidence.textContent = "reading files…";
|
||||
@@ -1179,6 +1166,7 @@ function removePanel(movie: LibraryMovie, actions: RemoveActions): HTMLElement {
|
||||
const confirm = document.createElement("button");
|
||||
confirm.type = "button";
|
||||
confirm.className = "control";
|
||||
confirm.textContent = "remove and delete files";
|
||||
confirm.setAttribute("aria-describedby", note.id);
|
||||
|
||||
const cancel = document.createElement("button");
|
||||
@@ -1188,42 +1176,30 @@ function removePanel(movie: LibraryMovie, actions: RemoveActions): HTMLElement {
|
||||
cancel.addEventListener("click", actions.cancel);
|
||||
|
||||
function paint() {
|
||||
toggle.setAttribute("aria-pressed", String(deleteFiles));
|
||||
toggleWord.textContent = deleteFiles ? "on" : "off";
|
||||
panel.dataset.armed = String(deleteFiles);
|
||||
confirm.textContent = deleteFiles ? "remove and delete files" : "remove from library";
|
||||
if (deleteFiles) {
|
||||
const hasFiles = files !== null && files.length > 0;
|
||||
panel.dataset.armed = String(hasFiles);
|
||||
if (hasFiles) {
|
||||
note.textContent =
|
||||
"deletes the folder above. the torrent keeps seeding until its tracker rule clears.";
|
||||
note.dataset.tone = "warn";
|
||||
return;
|
||||
}
|
||||
delete note.dataset.tone;
|
||||
note.textContent =
|
||||
files !== null && files.length === 0
|
||||
? "the library entry goes. nothing was imported for this title."
|
||||
: "the library entry goes. files stay on disk.";
|
||||
note.textContent = "the library entry goes. nothing was imported for this title.";
|
||||
}
|
||||
|
||||
toggle.addEventListener("click", () => {
|
||||
deleteFiles = !deleteFiles;
|
||||
paint();
|
||||
});
|
||||
|
||||
confirm.addEventListener("click", () => {
|
||||
confirm.disabled = true;
|
||||
cancel.disabled = true;
|
||||
toggle.disabled = true;
|
||||
delete note.dataset.tone;
|
||||
note.textContent = deleteFiles ? "removing title and files…" : "removing…";
|
||||
void removeMovie(movie.id, deleteFiles).then((outcome) => {
|
||||
note.textContent = "removing title and files…";
|
||||
void removeMovie(movie.id).then((outcome) => {
|
||||
if (outcome.kind === "done") {
|
||||
actions.removed();
|
||||
return;
|
||||
}
|
||||
confirm.disabled = false;
|
||||
cancel.disabled = false;
|
||||
toggle.disabled = files !== null && files.length > 0;
|
||||
// the row is still there on a failed unlink, so retrying is the fix
|
||||
note.textContent = `remove failed — ${outcome.detail}`;
|
||||
note.dataset.tone = "fault";
|
||||
@@ -1232,7 +1208,6 @@ function removePanel(movie: LibraryMovie, actions: RemoveActions): HTMLElement {
|
||||
|
||||
void movieFiles(movie.id).then((outcome) => {
|
||||
if (outcome.kind === "error") {
|
||||
// never offer to delete what cannot be named
|
||||
evidence.textContent = `files unreadable — ${outcome.detail}`;
|
||||
paint();
|
||||
return;
|
||||
@@ -1248,7 +1223,6 @@ function removePanel(movie: LibraryMovie, actions: RemoveActions): HTMLElement {
|
||||
const folder = libraryFolder(files);
|
||||
path.hidden = false;
|
||||
path.textContent = folder ?? files.map((file) => file.path).join("\n");
|
||||
toggle.disabled = false;
|
||||
paint();
|
||||
});
|
||||
|
||||
@@ -1256,8 +1230,8 @@ function removePanel(movie: LibraryMovie, actions: RemoveActions): HTMLElement {
|
||||
controls.className = "remove-actions";
|
||||
controls.append(confirm, cancel);
|
||||
paint();
|
||||
// evidence, then the decision, then what the decision costs
|
||||
panel.append(evidence, path, toggle, note, controls);
|
||||
// evidence, then what removing it costs
|
||||
panel.append(evidence, path, note, controls);
|
||||
return panel;
|
||||
}
|
||||
|
||||
|
||||
+5
-6
@@ -147,14 +147,13 @@ export function totalSize(files: MovieFile[]): number {
|
||||
export type ActionOutcome = { kind: "done" } | { kind: "error"; detail: string };
|
||||
|
||||
/**
|
||||
* Remove the title from the library. `deleteFiles` is the operator's second,
|
||||
* separate decision (§7.4): the row always goes, the folder only on request.
|
||||
* Either way the torrent keeps seeding — the reaper owns that lifecycle
|
||||
* (§7.3) and a hardlinked file loses only its library name.
|
||||
* Remove the title from the library. Both the row and its §7.4 folder go.
|
||||
* The torrent keeps seeding — the reaper owns that lifecycle (§7.3) and a
|
||||
* hardlinked file loses only its library name.
|
||||
*/
|
||||
export async function removeMovie(movieId: number, deleteFiles: boolean): Promise<ActionOutcome> {
|
||||
export async function removeMovie(movieId: number): Promise<ActionOutcome> {
|
||||
try {
|
||||
const response = await fetch(`/api/movies/${movieId}?delete_files=${String(deleteFiles)}`, {
|
||||
const response = await fetch(`/api/movies/${movieId}`, {
|
||||
method: "DELETE",
|
||||
});
|
||||
if (!response.ok) {
|
||||
|
||||
+2
-55
@@ -854,65 +854,12 @@ body {
|
||||
box-shadow: inset 0 1px var(--highlight);
|
||||
}
|
||||
|
||||
/* armed is a state, so it is amber and it is also a word: the toggle reads
|
||||
"on" with all colour removed */
|
||||
/* armed is a state, not a toggle: it is amber whenever files are on disk to
|
||||
delete */
|
||||
.remove-body[data-armed="true"] {
|
||||
border-color: oklch(from var(--signal-warn) l c h / 55%);
|
||||
}
|
||||
|
||||
/* a control, not a form field: it stops at its own label */
|
||||
.remove-files {
|
||||
display: flex;
|
||||
align-items: center;
|
||||
justify-self: start;
|
||||
max-width: 100%;
|
||||
gap: var(--space-3);
|
||||
min-height: 2.75rem;
|
||||
padding: 0 var(--space-3);
|
||||
font: inherit;
|
||||
font-size: var(--text-sm);
|
||||
text-align: left;
|
||||
color: var(--ink);
|
||||
background: var(--panel-raised);
|
||||
border: 1px solid var(--line);
|
||||
border-radius: var(--radius);
|
||||
box-shadow: inset 0 1px var(--highlight);
|
||||
cursor: pointer;
|
||||
transition:
|
||||
border-color 150ms var(--ease-out),
|
||||
color 150ms var(--ease-out);
|
||||
}
|
||||
|
||||
.remove-files:hover:not(:disabled) {
|
||||
border-color: var(--line-strong);
|
||||
}
|
||||
|
||||
.remove-files:disabled {
|
||||
color: var(--ink-faint);
|
||||
cursor: default;
|
||||
}
|
||||
|
||||
.remove-files-name {
|
||||
flex: 1;
|
||||
min-width: 0;
|
||||
}
|
||||
|
||||
.remove-files-word {
|
||||
flex: none;
|
||||
font-size: var(--text-xs);
|
||||
text-transform: uppercase;
|
||||
letter-spacing: 0.08em;
|
||||
color: var(--ink-faint);
|
||||
}
|
||||
|
||||
.remove-files[aria-pressed="true"] {
|
||||
border-color: oklch(from var(--signal-warn) l c h / 55%);
|
||||
}
|
||||
|
||||
.remove-files[aria-pressed="true"] .remove-files-word {
|
||||
color: var(--signal-warn);
|
||||
}
|
||||
|
||||
.remove-evidence,
|
||||
.remove-path,
|
||||
.remove-note {
|
||||
|
||||
Reference in New Issue
Block a user