fix(downloads): a conversion's commit, a stopped upload, the fast join and a clip's copy hold up

- The fast join gives each ffconcat entry its own media-only format whitelist, so a library file can no longer be opened as a nested concat list (the list's whitelist was handed down to it). - A file the guarded probe cannot read is a download-only file, not a guess from yt-dlp's info. - A conversion's or an ingest's commit that raises is re-read in a fresh session before any file is removed: a commit that landed keeps the file its row now names. - An original that cannot be removed after conversion is logged; the upload stays ready. - An upload whose conversion is cancelled or deleted midway is freed once nothing holds it; a paused one is kept. - A shared link says "preparing" only while its video's job is queued or running. - The admin storage view counts every file on disk ("Files on disk"), not only ready ones. - A clip's failure title, unknown failure and cancel dialog, and a clip's and an upload's delete dialog speak of what the job made; an upload's quota refusal explains that the original is kept. - /watch announces "preparing" as an alert and moves focus to the new Retry after an action.
2026-10-06 21:02:07 +02:00
parent d44d8d6dda
commit ed6facb703
19 changed files with 661 additions and 88 deletions
@@ -10,7 +10,9 @@ import type { DownloadJob } from "../lib/api";

 const mocks = vi.hoisted(() => ({
  jobs: [] as unknown[],
-  confirm: vi.fn(async (_opts: { title: string; confirmLabel?: string }) => false),
+  confirm: vi.fn(
+    async (_opts: { title: string; message?: string; confirmLabel?: string }) => false,
+  ),
 }));

 vi.mock("react-i18next", async (importOriginal) => {
@@ -141,4 +143,41 @@ describe("DownloadCenter confirm dialogs", () => {
      "downloads.confirm.cancelUploadAction",
    ]);
  });
+
+  it("cancelling a clip speaks of the clip", async () => {
+    mocks.jobs = [{ ...job(4, "running"), job_kind: "edit" }];
+    render(
+      <QueryClientProvider client={new QueryClient()}>
+        <DownloadCenter />
+      </QueryClientProvider>,
+    );
+    await pick("Clip 4", "downloads.actions.cancel");
+    await waitFor(() => expect(mocks.confirm).toHaveBeenCalledTimes(1));
+    const [opts] = mocks.confirm.mock.calls[0]!;
+    expect([opts.title, opts.message, opts.confirmLabel]).toEqual([
+      "downloads.confirm.cancelEditTitle",
+      "downloads.confirm.cancelEditBody",
+      "downloads.confirm.cancelEditAction",
+    ]);
+  });
+
+  it.each([
+    ["upload", "downloads.confirm.deleteUploadTitle", "downloads.confirm.deleteUploadBody"],
+    ["edit", "downloads.confirm.deleteEditTitle", "downloads.confirm.deleteEditBody"],
+    ["download", "downloads.confirm.deleteTitle", "downloads.confirm.deleteBody"],
+  ] as const)("deleting a finished %s says what happens to its file", async (kind, title, body) => {
+    // Only a download's file may stay cached for other users; an upload's or a clip's is theirs alone.
+    mocks.jobs = [{ ...job(5, "done"), job_kind: kind }];
+    render(
+      <QueryClientProvider client={new QueryClient()}>
+        <DownloadCenter />
+      </QueryClientProvider>,
+    );
+    fireEvent.click(screen.getByRole("tab", { name: /downloads\.tabs\.done/ }));
+    await screen.findByText("Clip 5");
+    await pick("Clip 5", "downloads.actions.delete");
+    await waitFor(() => expect(mocks.confirm).toHaveBeenCalledTimes(1));
+    const [opts] = mocks.confirm.mock.calls[0]!;
+    expect([opts.title, opts.message]).toEqual([title, body]);
+  });
 });
@@ -224,13 +224,33 @@ describe("WatchPage honest failures", () => {
    );
    const page = render(<WatchPage />);
    const retry = await page.findByRole("button", { name: "Try again" });
-    expect(page.getByRole("status").textContent).toContain("This video is being prepared.");
+    expect(page.getByRole("alert").textContent).toContain("This video is being prepared.");
    expect(page.queryByText(GONE)).toBeNull();

    fireEvent.click(retry);
    await waitFor(() => expect(page.container.querySelector("video")).not.toBeNull());
  });

+  it("a Retry that is answered 'still preparing' says so and keeps the focus on Retry", async () => {
+    // The answer replaces the button the viewer was on: announced (an alert, inserted filled), and
+    // the new Retry takes the focus instead of leaving it on <body>.
+    window.history.replaceState({}, "", "/watch/tok-preparing");
+    const preparing = () => res(404, { detail: { code: 404, reason: "video_preparing" } });
+    vi.stubGlobal(
+      "fetch",
+      vi.fn().mockImplementation(() => Promise.resolve(preparing())),
+    );
+    const page = render(<WatchPage />);
+    const first = await page.findByRole("button", { name: "Try again" });
+    expect(document.activeElement).not.toBe(first); // the first load moves no focus
+
+    fireEvent.click(first);
+    await waitFor(() =>
+      expect(document.activeElement).toBe(page.getByRole("button", { name: "Try again" })),
+    );
+    expect(page.getByRole("alert").textContent).toContain("This video is being prepared.");
+  });
+
  async function tryPassword(second: () => Promise<unknown>) {
    vi.stubGlobal(
      "fetch",
@@ -114,6 +114,10 @@ export default function WatchPage() {

  // Bumped by Retry, which asks again.
  const [attempt, setAttempt] = useState(0);
+  // Set once the viewer acted (Retry, the password): the state that answers replaces the button they
+  // were on, so its own Retry takes the focus rather than leaving it on <body>. Not on the first load,
+  // where nothing had the focus to lose.
+  const [acted, setActed] = useState(false);
  useEffect(() => {
    if (!token) return; // the seed above already says "gone"
    send(`/api/public/watch/${encodeURIComponent(token)}`)
@@ -136,6 +140,7 @@ export default function WatchPage() {
      .catch(() => setStatus("error"));
  }, [token, attempt]);
  const retry = () => {
+    setActed(true);
    setStatus("loading");
    setAttempt((n) => n + 1);
  };
@@ -167,6 +172,7 @@ export default function WatchPage() {
  };

  const unlock = async () => {
+    setActed(true);
    setUnlocking(true);
    setError(null);
    try {
@@ -203,11 +209,19 @@ export default function WatchPage() {

        {status === "preparing" && (
          <div className="flex-1 grid place-items-center text-center">
-            {/* Announced like the failure below: it replaces the loading line. */}
-            <div role="status">
+            {/* An alert, like the failure below: it is inserted already filled, which a polite status
+                region is not reliably announced for — and a Retry that answers "still preparing"
+                would otherwise say nothing at all. */}
+            <div role="alert">
              <div className="text-lg font-semibold">{T.preparing}</div>
              <div className="text-sm text-muted mt-1">{T.preparingHint}</div>
-              <Button variant="primary" size="md" onClick={retry} className="mt-4">
+              <Button
+                variant="primary"
+                size="md"
+                onClick={retry}
+                autoFocus={acted}
+                className="mt-4"
+              >
                {T.retry}
              </Button>
            </div>
@@ -220,7 +234,13 @@ export default function WatchPage() {
            <div role="alert">
              <div className="text-lg font-semibold">{T.failed}</div>
              <div className="text-sm text-muted mt-1">{T.failedHint}</div>
-              <Button variant="primary" size="md" onClick={retry} className="mt-4">
+              <Button
+                variant="primary"
+                size="md"
+                onClick={retry}
+                autoFocus={acted}
+                className="mt-4"
+              >
                {T.retry}
              </Button>
            </div>
@@ -186,7 +186,7 @@ export function AdminSystem() {
      {s && (
        <div className="grid grid-cols-2 sm:grid-cols-4 gap-3 mb-5">
          {[
-            [t("downloads.admin.readyFiles"), String(s.ready_files)],
+            [t("downloads.admin.filesOnDisk"), String(s.files_on_disk)],
            [t("downloads.admin.totalSize"), formatBytes(s.total_bytes, lang)],
            [
              t("downloads.admin.cap"),
@@ -16,7 +16,7 @@ import {
  Trash2,
 } from "lucide-react";
 import { qk } from "../lib/queryKeys";
-import { api, type DownloadJob } from "../lib/api";
+import { api, type DownloadJob, type DownloadJobKind } from "../lib/api";
 import { notifyActionError } from "../lib/actionErrors";
 import { invalidateDownloads, quickShareLink } from "../lib/downloads";
 import { libraryRowActions, sharedRowActions } from "../lib/downloadActions";
@@ -46,6 +46,29 @@ const CANCEL_UPLOAD = {
  body: "downloads.confirm.cancelUploadBody",
  action: "downloads.confirm.cancelUploadAction",
 };
+const CANCEL_EDIT = {
+  title: "downloads.confirm.cancelEditTitle",
+  body: "downloads.confirm.cancelEditBody",
+  action: "downloads.confirm.cancelEditAction",
+};
+// Cancelling an upload deletes the file it uploaded (there is nothing to fetch again), so its
+// confirmation says so — "stop the download" would read as safe to undo. A clip is being cut, not
+// downloaded.
+const CANCEL_COPY: Record<DownloadJobKind, typeof CANCEL_DOWNLOAD> = {
+  download: CANCEL_DOWNLOAD,
+  upload: CANCEL_UPLOAD,
+  edit: CANCEL_EDIT,
+};
+// Deleting a download may leave its shared cache file for other holders. An upload and a clip are
+// their owner's alone (a clip's asset is keyed by its user), so deleting one removes the file for good.
+const DELETE_COPY: Record<DownloadJobKind, { title: string; body: string }> = {
+  download: { title: "downloads.confirm.deleteTitle", body: "downloads.confirm.deleteBody" },
+  upload: {
+    title: "downloads.confirm.deleteUploadTitle",
+    body: "downloads.confirm.deleteUploadBody",
+  },
+  edit: { title: "downloads.confirm.deleteEditTitle", body: "downloads.confirm.deleteEditBody" },
+};

 export function useDownloadActions({
  onPlay,
@@ -159,10 +182,8 @@ export function useDownloadActions({
          label: t("downloads.actions.cancel"),
          icon: icon(Ban),
          danger: true,
-          // Cancelling an upload deletes the file it uploaded (there is nothing to fetch again), so
-          // its confirmation says so — "stop the download" would read as safe to undo.
          onSelect: () => {
-            const copy = job.job_kind === "upload" ? CANCEL_UPLOAD : CANCEL_DOWNLOAD;
+            const copy = CANCEL_COPY[job.job_kind ?? "download"];
            void confirmThen(t(copy.title), t(copy.body), t(copy.action), () =>
              act.mutate({ id: job.id, op: "cancel" }),
            );
@@ -233,13 +254,12 @@ export function useDownloadActions({
          label: t("downloads.actions.delete"),
          icon: icon(Trash2),
          danger: true,
-          onSelect: () =>
-            void confirmThen(
-              t("downloads.confirm.deleteTitle"),
-              t("downloads.confirm.deleteBody"),
-              t("downloads.actions.delete"),
-              () => act.mutate({ id: job.id, op: "delete" }),
-            ),
+          onSelect: () => {
+            const copy = DELETE_COPY[job.job_kind ?? "download"];
+            void confirmThen(t(copy.title), t(copy.body), t("downloads.actions.delete"), () =>
+              act.mutate({ id: job.id, op: "delete" }),
+            );
+          },
        },
      ],
    );
@@ -155,8 +155,15 @@
    "cancelUploadTitle": "Delete upload?",
    "cancelUploadBody": "The file you uploaded is removed from the server along with it.",
    "cancelUploadAction": "Delete upload",
+    "cancelEditTitle": "Cancel clip?",
+    "cancelEditBody": "This stops making the clip and removes it from your queue.",
+    "cancelEditAction": "Stop making this clip",
    "deleteTitle": "Delete download?",
    "deleteBody": "This deletes it from your downloads. The file may stay cached for other users until it expires.",
+    "deleteUploadTitle": "Delete upload?",
+    "deleteUploadBody": "The uploaded file is removed from the server. This can't be undone.",
+    "deleteEditTitle": "Delete clip?",
+    "deleteEditBody": "The clip is removed from the server. This can't be undone.",
    "removeSharedTitle": "Remove from your list?",
    "removeSharedBody": "This removes it from your shared list only — it won't delete the owner's file.",
    "redownloadTitle": "Re-download this file?",
@@ -187,7 +194,7 @@
    "deleteConfirm": "Delete the format “{{name}}”?"
  },
  "admin": {
-    "readyFiles": "Ready files",
+    "filesOnDisk": "Files on disk",
    "totalSize": "Total size",
    "cap": "Cache cap",
    "noCap": "no cap",
@@ -231,8 +238,20 @@
    "fail": {
      "title": "Couldn't download",
      "titleUpload": "The upload isn't ready",
+      "titleEdit": "The clip isn't ready",
      "detailsLink": "details",
      "detailsToggle": "Technical details",
+      "upload": {
+        "quota_exceeded": {
+          "body": "The phone-ready version of this upload would not fit in your storage quota. Your original is kept – delete something from your library, then try again."
+        }
+      },
+      "edit": {
+        "unknown": {
+          "short": "Couldn't make the clip",
+          "body": "Something went wrong while making this clip. The technical details below may help."
+        }
+      },
      "cat": {
        "unavailable": {
          "short": "Video unavailable",
@@ -155,8 +155,15 @@
    "cancelUploadTitle": "Törlöd a feltöltést?",
    "cancelUploadBody": "A feltöltött fájl is törlődik a szerverről.",
    "cancelUploadAction": "Feltöltés törlése",
+    "cancelEditTitle": "Megszakítod a klipet?",
+    "cancelEditBody": "Ez leállítja a klip készítését és eltávolítja a sorból.",
+    "cancelEditAction": "Klip készítésének leállítása",
    "deleteTitle": "Törlöd a letöltést?",
    "deleteBody": "Ez törli a letöltéseid közül. A fájl a lejáratáig még a gyorsítótárban maradhat mások számára.",
+    "deleteUploadTitle": "Törlöd a feltöltést?",
+    "deleteUploadBody": "A feltöltött fájl törlődik a szerverről. Ez nem vonható vissza.",
+    "deleteEditTitle": "Törlöd a klipet?",
+    "deleteEditBody": "A klip törlődik a szerverről. Ez nem vonható vissza.",
    "removeSharedTitle": "Eltávolítod a listádból?",
    "removeSharedBody": "Csak a te megosztott listádból tűnik el — a tulajdonos fájlját nem törli.",
    "redownloadTitle": "Újraletöltöd ezt a fájlt?",
@@ -187,7 +194,7 @@
    "deleteConfirm": "Törlöd a(z) „{{name}}” formátumot?"
  },
  "admin": {
-    "readyFiles": "Kész fájlok",
+    "filesOnDisk": "Fájlok a lemezen",
    "totalSize": "Teljes méret",
    "cap": "Gyorsítótár-korlát",
    "noCap": "nincs korlát",
@@ -231,8 +238,20 @@
    "fail": {
      "title": "Nem sikerült letölteni",
      "titleUpload": "A feltöltés nem készült el",
+      "titleEdit": "A klip nem készült el",
      "detailsLink": "részletek",
      "detailsToggle": "Technikai részletek",
+      "upload": {
+        "quota_exceeded": {
+          "body": "A feltöltés telefonra alakított változata nem fér bele a tárhelykeretedbe. Az eredeti megmaradt – törölj valamit a könyvtáradból, majd próbáld újra."
+        }
+      },
+      "edit": {
+        "unknown": {
+          "short": "Nem sikerült elkészíteni a klipet",
+          "body": "Hiba történt a klip készítése közben. Az alábbi technikai részletek segíthetnek."
+        }
+      },
      "cat": {
        "unavailable": {
          "short": "A videó nem elérhető",
@@ -47,7 +47,7 @@ export type DownloadStatus = "queued" | "running" | "paused" | "done" | "error"
 // "download" (default), "edit" (a per-user trim/crop derivative cut from another job), or "upload"
 // (a locally-uploaded file — no re-fetchable source, so re-download is rejected backend-side).
 // Local (not exported): consumers compare `job.job_kind` against literals, never name the type.
-type DownloadJobKind = "download" | "edit" | "upload";
+export type DownloadJobKind = "download" | "edit" | "upload";

 // Phase-2 editor recipe. A single `trim` (one output file) or a `segments` cut-list joined into
 // one file. `accurate` re-encodes for a frame-accurate cut (a crop always re-encodes).
@@ -137,7 +137,7 @@ export interface DownloadUsage {
 }

 export interface AdminStorage {
-  ready_files: number;
+  files_on_disk: number;
  total_bytes: number;
  total_assets: number;
  total_cap_bytes: number;
@@ -49,13 +49,61 @@ describe("failCopyKeys", () => {
 });

 describe("failTitleKey", () => {
-  it("titles an upload's failure as a conversion, anything else as a download", async () => {
+  it("titles a failure by what the job was making", async () => {
    await i18n.changeLanguage("en");
-    // the locale has the copy (a missing key would come back as the key itself)
-    expect(i18n.t(failTitleKey({ job_kind: "upload" }))).not.toMatch(/^downloads\./);
    expect(failTitleKey({ job_kind: "upload" })).toBe("downloads.errors.fail.titleUpload");
-    expect(failTitleKey({ job_kind: "edit" })).toBe("downloads.errors.fail.title");
+    expect(failTitleKey({ job_kind: "edit" })).toBe("downloads.errors.fail.titleEdit");
+    expect(failTitleKey({ job_kind: "download" })).toBe("downloads.errors.fail.title");
    expect(failTitleKey({})).toBe("downloads.errors.fail.title");
+    for (const lng of ["en", "hu"]) {
+      await i18n.changeLanguage(lng);
+      // the locale has the copy (a missing key would come back as the key itself)
+      for (const job_kind of ["upload", "edit"] as const)
+        expect(i18n.t(failTitleKey({ job_kind }))).not.toMatch(/^downloads\./);
+    }
+  });
+
+  it("an upload's quota refusal is worded as its conversion's, in both languages", async () => {
+    // The converted file outgrew the cap; the original stays. Not "this download was stopped because
+    // your quota is already used up" — the download sentence stays for a download.
+    const upload = {
+      error_code: "quota_exceeded",
+      error_detail: null,
+      job_kind: "upload",
+    } as const;
+    const download = { ...upload, job_kind: "download" } as const;
+    for (const lng of ["en", "hu"]) {
+      await i18n.changeLanguage(lng);
+      const body = i18n.t(failCopyKeys(upload, "body"));
+      expect(body).not.toMatch(/^downloads\./);
+      expect(body).not.toBe(i18n.t(failCopyKeys(download, "body")));
+      expect(i18n.t(failCopyKeys(download, "body"))).toBe(
+        i18n.t("downloads.errors.fail.cat.quota_exceeded.body"),
+      );
+    }
+  });
+
+  it("a clip that fails for no known reason says so of a clip, in both languages", async () => {
+    // Not "Couldn't download / Something went wrong with this download" — nothing was downloaded.
+    const clip = { error_code: "unknown", error_detail: null, job_kind: "edit" } as const;
+    const download = { ...clip, job_kind: "download" } as const;
+    for (const lng of ["en", "hu"]) {
+      await i18n.changeLanguage(lng);
+      for (const part of ["short", "body"] as const) {
+        const own = i18n.t(failCopyKeys(clip, part));
+        expect(own).not.toMatch(/^downloads\./);
+        expect(own).not.toBe(i18n.t(failCopyKeys(download, part)));
+      }
+    }
+    // a category with no copy of its own falls back to the clip's unknown sentence, not the download's
+    expect(
+      failCopyKeys({ error_code: "brand_new", error_detail: null, job_kind: "edit" }, "body"),
+    ).toEqual([
+      "downloads.errors.fail.edit.brand_new.body",
+      "downloads.errors.fail.cat.brand_new.body",
+      "downloads.errors.fail.edit.unknown.body",
+      "downloads.errors.fail.cat.unknown.body",
+    ]);
  });

  it("an upload's own failures read as an upload's", async () => {
@@ -1,27 +1,38 @@
-import type { DownloadJob } from "./api";
+import type { DownloadJob, DownloadJobKind } from "./api";

 // The i18n keys that explain a failed download, most specific first — the list form `t()` takes, so
 // the first key a locale has wins. A failure-catalog entry (`error_detail`, e.g. `facebook.login_wall`)
 // names what went wrong on that site and what to do; the category (`error_code`) is the generic
 // sentence, and the fallback when an older client meets a detail it has no copy for; a category it has
-// no copy for falls back to the unknown one, so the row never prints a key. One helper for the queue
-// row's headline and the failure window, so the two can never explain the same job differently.
+// no copy for falls back to the unknown one, so the row never prints a key. A job that is not a
+// download (an upload's conversion, a clip) may word a category its own way, ahead of the shared
+// sentence — "this download was stopped" is not what happened to it — and its own unknown sentence
+// comes before the download's. One helper for the queue row's headline and the failure window, so
+// the two can never explain the same job differently.
 export function failCopyKeys(
-  job: Pick<DownloadJob, "error_code" | "error_detail">,
+  job: Pick<DownloadJob, "error_code" | "error_detail" | "job_kind">,
  part: "short" | "body",
 ): string[] {
-  const category = (code: string) => `downloads.errors.fail.cat.${code}.${part}`;
+  const kind = job.job_kind ?? "download";
+  const category = (code: string) => [
+    ...(kind === "download" ? [] : [`downloads.errors.fail.${kind}.${code}.${part}`]),
+    `downloads.errors.fail.cat.${code}.${part}`,
+  ];
  const code = job.error_code || "unknown";
  return [
    ...(job.error_detail ? [`downloads.errors.fail.catalog.${job.error_detail}.${part}`] : []),
-    category(code),
-    ...(code === "unknown" ? [] : [category("unknown")]),
+    ...category(code),
+    ...(code === "unknown" ? [] : category("unknown")),
  ];
 }

-/** The title over a failed job's explanation: an upload's failure is a conversion, not a download. */
+const TITLE_KEYS: Record<DownloadJobKind, string> = {
+  download: "downloads.errors.fail.title",
+  upload: "downloads.errors.fail.titleUpload", // its failure is a conversion, not a download
+  edit: "downloads.errors.fail.titleEdit", // a clip being cut
+};
+
+/** The title over a failed job's explanation, worded for what the job was making. */
 export function failTitleKey(job: Pick<DownloadJob, "job_kind">): string {
-  return job.job_kind === "upload"
-    ? "downloads.errors.fail.titleUpload"
-    : "downloads.errors.fail.title";
+  return TITLE_KEYS[job.job_kind ?? "download"];
 }