From 6073a006b8fb6232f34eaadb111a5cf42365cf29 Mon Sep 17 00:00:00 2001 From: npeter83 Date: Tue, 6 Oct 2026 03:19:32 +0200 Subject: [PATCH] fix(downloads): an upload's cancel, progress, failure and notice speak of an upload - Cancelling an upload asks "Delete upload?" and says the uploaded file is removed from the server with it. - The phone-safe conversion's real ffmpeg % shows as a bar ("Converting for phones"), not an indeterminate pulse. - A failed upload is titled "The upload isn't ready", with its own reasons (convert_failed, retryable; upload_missing) instead of download copy. - The inbox says "Upload ready to share" for a converted upload. --- .../DownloadCenter.confirm.dom.test.tsx | 17 ++++++++++++ .../NotificationsPanel.menu.dom.test.tsx | 16 +++++++++++ .../src/components/NotificationsPanel.tsx | 12 ++++++--- frontend/src/components/downloadModals.tsx | 6 ++--- .../src/components/downloadRow.dom.test.tsx | 24 +++++++++++++++++ frontend/src/components/downloadRow.tsx | 10 ++++--- .../src/components/useDownloadActions.tsx | 27 ++++++++++++++----- frontend/src/i18n/locales/en/downloads.json | 15 ++++++++++- frontend/src/i18n/locales/en/inbox.json | 3 +++ frontend/src/i18n/locales/hu/downloads.json | 15 ++++++++++- frontend/src/i18n/locales/hu/inbox.json | 3 +++ frontend/src/lib/api/downloads.ts | 2 +- frontend/src/lib/downloadFailure.test.ts | 21 ++++++++++++++- frontend/src/lib/downloadFailure.ts | 7 +++++ 14 files changed, 157 insertions(+), 21 deletions(-) diff --git a/frontend/src/components/DownloadCenter.confirm.dom.test.tsx b/frontend/src/components/DownloadCenter.confirm.dom.test.tsx index f4adee4c..46deb263 100644 --- a/frontend/src/components/DownloadCenter.confirm.dom.test.tsx +++ b/frontend/src/components/DownloadCenter.confirm.dom.test.tsx @@ -124,4 +124,21 @@ describe("DownloadCenter confirm dialogs", () => { ["downloads.confirm.deleteTitle", "downloads.actions.delete"], ]); }); + + it("cancelling an upload says the uploaded file goes with it", async () => { + // Nothing to fetch again: the cancel deletes the original from the server. + mocks.jobs = [{ ...job(3, "queued"), job_kind: "upload" }]; + render( + + + , + ); + await pick("Clip 3", "downloads.actions.cancel"); + await waitFor(() => expect(mocks.confirm).toHaveBeenCalledTimes(1)); + const [opts] = mocks.confirm.mock.calls[0]!; + expect([opts.title, opts.confirmLabel]).toEqual([ + "downloads.confirm.cancelUploadTitle", + "downloads.confirm.cancelUploadAction", + ]); + }); }); diff --git a/frontend/src/components/NotificationsPanel.menu.dom.test.tsx b/frontend/src/components/NotificationsPanel.menu.dom.test.tsx index d450ec12..5cb4ef94 100644 --- a/frontend/src/components/NotificationsPanel.menu.dom.test.tsx +++ b/frontend/src/components/NotificationsPanel.menu.dom.test.tsx @@ -364,3 +364,19 @@ describe("a download-lifecycle notice", () => { expect(screen.queryByText("download")).toBeNull(); }); }); + +describe("NotificationsPanel · a finished job's title", () => { + it("names what finished: a download, a clip, an upload", () => { + h.server = [ + server(1), + server(2, { data: { title: "Ready 2", kind: "edit" } }), + server(3, { data: { title: "Ready 3", kind: "upload" } }), + ]; + h.client = []; + mount(); + const titleOf = (text: string) => rowOf(text).textContent ?? ""; + expect(titleOf("Ready 1")).toContain("inbox.downloadDone.title"); + expect(titleOf("Ready 2")).toContain("inbox.editDone.title"); + expect(titleOf("Ready 3")).toContain("inbox.uploadDone.title"); + }); +}); diff --git a/frontend/src/components/NotificationsPanel.tsx b/frontend/src/components/NotificationsPanel.tsx index 9ea0cab7..134a5cfb 100644 --- a/frontend/src/components/NotificationsPanel.tsx +++ b/frontend/src/components/NotificationsPanel.tsx @@ -446,6 +446,11 @@ const DOWNLOAD_LIFECYCLE: Record = { download_expired: "downloadExpired", download_evicted: "downloadEvicted", }; +// A `download_done` notice's title by the producer's `kind`; any other kind reads as a download. +const DONE_TITLE: Record = { + edit: "inbox.editDone.title", + upload: "inbox.uploadDone.title", +}; function serverKind(n: AppNotification): "scheduler" | "download" | null { if (n.type === "scheduler" && typeof n.data?.job_id === "string") return "scheduler"; @@ -552,10 +557,11 @@ function NotificationRow({ title = t(`inbox.${lifecycle}.title`, { from: d.from ?? "" }); body = d.title || null; } else if (isDownloadDone) { - // "Download ready" / "Edit ready" per the producer's kind, with the video's title as the body - // (shipped in data.title). Same notification type; the copy differs so an edit isn't mislabelled. + // "Download ready" / "Edit ready" / "Upload ready" per the producer's kind, with the video's title + // as the body (shipped in data.title). Same notification type; the copy differs so an edit or an + // upload isn't called a download. const d = n.data as { title?: string | null; kind?: string }; - title = t(d.kind === "edit" ? "inbox.editDone.title" : "inbox.downloadDone.title"); + title = t(DONE_TITLE[d.kind ?? ""] ?? "inbox.downloadDone.title"); body = d.title || n.body; } return ( diff --git a/frontend/src/components/downloadModals.tsx b/frontend/src/components/downloadModals.tsx index 9de5d747..c7bb47ab 100644 --- a/frontend/src/components/downloadModals.tsx +++ b/frontend/src/components/downloadModals.tsx @@ -7,7 +7,7 @@ import Modal, { ModalFooter } from "./ui/Modal"; import { api, type DownloadJob } from "../lib/api"; import { Button } from "./ui/Button"; import { IconBtn } from "./downloadRow"; -import { failCopyKeys } from "../lib/downloadFailure"; +import { failCopyKeys, failTitleKey } from "../lib/downloadFailure"; import { Input } from "./ui/Input"; // Per-item user modals split out of DownloadCenter (self-contained, prop-driven): rename/metadata @@ -112,7 +112,7 @@ export function MetaEditModal({ job, onClose }: { job: DownloadJob; onClose: () // retry control regardless. `unknown` is deliberately NOT here: an unclassified failure is most often an // auth wall we couldn't parse (a login-walled Facebook post reads as a bare "no format"), so a // prominent Retry would mislead; force-retrying from the row stays available for genuine transients. -const RETRYABLE_ERRORS = ["postprocess", "network"]; +const RETRYABLE_ERRORS = ["postprocess", "network", "convert_failed"]; // A readable "why did this fail" window for a failed job: a category headline + plain explanation, // the raw yt-dlp/ffmpeg text behind a disclosure, and a Retry when the failure may be transient. @@ -130,7 +130,7 @@ export function ErrorModal({ const [showRaw, setShowRaw] = useState(false); const retryable = RETRYABLE_ERRORS.includes(code); return ( - +
{t(failCopyKeys(job, "short"))}

{t(failCopyKeys(job, "body"))}

diff --git a/frontend/src/components/downloadRow.dom.test.tsx b/frontend/src/components/downloadRow.dom.test.tsx index 7ef2ba2e..39df6c0c 100644 --- a/frontend/src/components/downloadRow.dom.test.tsx +++ b/frontend/src/components/downloadRow.dom.test.tsx @@ -292,3 +292,27 @@ describe("DownloadRow · no picture", () => { expect(container.querySelector("svg.lucide-film")).not.toBeNull(); }); }); + +// The phone-safe conversion reports ffmpeg's real progress, so it gets the % bar a download has; +// a yt-dlp post-step reports none and keeps the pulse. +describe("DownloadRow · a running conversion", () => { + const running = (phase: string): DownloadJob => ({ + ...baseJob, + status: "running", + progress: 40, + phase, + }); + + it("shows its phase and its %", () => { + const { container } = render(); + expect(screen.getByText("downloads.phase.converting")).toBeTruthy(); + expect(screen.getByText("40%")).toBeTruthy(); + expect(container.querySelector(".animate-pulse")).toBeNull(); + }); + + it("a post-step with no % keeps the pulse", () => { + const { container } = render(); + expect(screen.queryByText("40%")).toBeNull(); + expect(container.querySelector(".animate-pulse")).not.toBeNull(); + }); +}); diff --git a/frontend/src/components/downloadRow.tsx b/frontend/src/components/downloadRow.tsx index 9809a525..926e9c3d 100644 --- a/frontend/src/components/downloadRow.tsx +++ b/frontend/src/components/downloadRow.tsx @@ -14,7 +14,7 @@ import { relativeTime, } from "../lib/format"; import { navItemAttrs } from "../lib/listNav"; -import { failCopyKeys } from "../lib/downloadFailure"; +import { failCopyKeys, failTitleKey } from "../lib/downloadFailure"; import { displayUrl, hasOwnTitle, jobTitle } from "./downloadTitle"; import { IconButton } from "./ui/IconButton"; import { Pressable } from "./ui/Pressable"; @@ -34,12 +34,14 @@ const STATUS_CLS: Record = { }; // Byte-progress phases (show a % bar) vs ffmpeg post-steps (no %, indeterminate pulse). -// "editing" (the phase-2 editor) reports a real % from ffmpeg's out_time, so it gets a bar. -const DOWNLOAD_PHASES = ["video", "audio", "editing"]; +// "editing" (the phase-2 editor) and "converting" (the phone-safe conversion of a download or an +// upload) report a real % from ffmpeg's out_time, so they get a bar. +const DOWNLOAD_PHASES = ["video", "audio", "editing", "converting"]; const PHASE_KEYS = [ "video", "audio", "editing", + "converting", "merging", "audio_extract", "thumbnail", @@ -313,7 +315,7 @@ export function DownloadRow({ onShowError?.(job)} className="text-danger inline-block leading-6 min-h-6 touch-target pointer-coarse:leading-(--touch-target)" - title={t("downloads.errors.fail.title")} + title={t(failTitleKey(job))} > · {t(failCopyKeys(job, "short"))}{" "} diff --git a/frontend/src/components/useDownloadActions.tsx b/frontend/src/components/useDownloadActions.tsx index aa549678..1731c101 100644 --- a/frontend/src/components/useDownloadActions.tsx +++ b/frontend/src/components/useDownloadActions.tsx @@ -35,6 +35,18 @@ import { menuGroups } from "./ui/menuGroups"; export type DownloadTab = "queue" | "done" | "shared"; export type JobOp = "pause" | "resume" | "cancel" | "delete" | "redownload"; +// The Cancel confirmation of a queued job; an upload's names what it deletes. +const CANCEL_DOWNLOAD = { + title: "downloads.confirm.cancelTitle", + body: "downloads.confirm.cancelBody", + action: "downloads.confirm.cancelAction", +}; +const CANCEL_UPLOAD = { + title: "downloads.confirm.cancelUploadTitle", + body: "downloads.confirm.cancelUploadBody", + action: "downloads.confirm.cancelUploadAction", +}; + export function useDownloadActions({ onPlay, onEdit, @@ -147,13 +159,14 @@ export function useDownloadActions({ label: t("downloads.actions.cancel"), icon: icon(Ban), danger: true, - onSelect: () => - void confirmThen( - t("downloads.confirm.cancelTitle"), - t("downloads.confirm.cancelBody"), - t("downloads.confirm.cancelAction"), - () => act.mutate({ id: job.id, op: "cancel" }), - ), + // 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; + void confirmThen(t(copy.title), t(copy.body), t(copy.action), () => + act.mutate({ id: job.id, op: "cancel" }), + ); + }, }, ], ); diff --git a/frontend/src/i18n/locales/en/downloads.json b/frontend/src/i18n/locales/en/downloads.json index 95684f65..3130bb73 100644 --- a/frontend/src/i18n/locales/en/downloads.json +++ b/frontend/src/i18n/locales/en/downloads.json @@ -63,7 +63,8 @@ "sponsorblock": "Removing sponsors", "metadata": "Writing metadata", "processing": "Processing", - "editing": "Editing" + "editing": "Editing", + "converting": "Converting for phones" }, "actions": { "pause": "Pause", @@ -151,6 +152,9 @@ "cancelTitle": "Cancel download?", "cancelBody": "This stops the download and removes it from your queue.", "cancelAction": "Stop download", + "cancelUploadTitle": "Delete upload?", + "cancelUploadBody": "The file you uploaded is removed from the server along with it.", + "cancelUploadAction": "Delete upload", "deleteTitle": "Delete download?", "deleteBody": "This deletes it from your downloads. The file may stay cached for other users until it expires.", "removeSharedTitle": "Remove from your list?", @@ -226,6 +230,7 @@ }, "fail": { "title": "Couldn't download", + "titleUpload": "The upload isn't ready", "detailsLink": "details", "detailsToggle": "Technical details", "cat": { @@ -257,6 +262,14 @@ "short": "Couldn't process the video", "body": "The video downloaded but a processing step (merge or convert) failed. A retry often fixes this." }, + "convert_failed": { + "short": "Couldn't convert the upload", + "body": "Your file couldn't be converted into the format every phone plays. It may be damaged, or in a format the server can't read. The technical details below may help." + }, + "upload_missing": { + "short": "Uploaded file missing", + "body": "The file you uploaded is no longer on the server. Upload it again." + }, "source_missing": { "short": "Source unavailable", "body": "The original download this clip is based on is no longer available." diff --git a/frontend/src/i18n/locales/en/inbox.json b/frontend/src/i18n/locales/en/inbox.json index c4eb9bf9..c615c6ba 100644 --- a/frontend/src/i18n/locales/en/inbox.json +++ b/frontend/src/i18n/locales/en/inbox.json @@ -22,6 +22,9 @@ "editDone": { "title": "Edit ready" }, + "uploadDone": { + "title": "Upload ready to share" + }, "maintenance": { "title": "Videos removed", "body_one": "{{count}} saved or playlisted video was removed because it's no longer available on YouTube.", diff --git a/frontend/src/i18n/locales/hu/downloads.json b/frontend/src/i18n/locales/hu/downloads.json index 8371a869..7dd76372 100644 --- a/frontend/src/i18n/locales/hu/downloads.json +++ b/frontend/src/i18n/locales/hu/downloads.json @@ -63,7 +63,8 @@ "sponsorblock": "Szponzorok eltávolítása", "metadata": "Metaadat írása", "processing": "Feldolgozás", - "editing": "Szerkesztés" + "editing": "Szerkesztés", + "converting": "Átalakítás telefonra" }, "actions": { "pause": "Szünet", @@ -151,6 +152,9 @@ "cancelTitle": "Megszakítod a letöltést?", "cancelBody": "Ez leállítja a letöltést és eltávolítja a sorból.", "cancelAction": "Letöltés leállítása", + "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", "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.", "removeSharedTitle": "Eltávolítod a listádból?", @@ -226,6 +230,7 @@ }, "fail": { "title": "Nem sikerült letölteni", + "titleUpload": "A feltöltés nem készült el", "detailsLink": "részletek", "detailsToggle": "Technikai részletek", "cat": { @@ -257,6 +262,14 @@ "short": "Nem sikerült feldolgozni a videót", "body": "A videó letöltődött, de egy feldolgozási lépés (összefűzés vagy konvertálás) meghiúsult. Újrapróbálásra gyakran megjavul." }, + "convert_failed": { + "short": "Nem sikerült átalakítani", + "body": "A fájlt nem sikerült olyan formátumra alakítani, amit minden telefon lejátszik. Lehet, hogy sérült, vagy a szerver nem ismeri a formátumát. Az alábbi technikai részletek segíthetnek." + }, + "upload_missing": { + "short": "Hiányzik a feltöltött fájl", + "body": "A feltöltött fájl már nincs a szerveren. Töltsd fel újra." + }, "source_missing": { "short": "A forrás nem elérhető", "body": "Az eredeti letöltés, amelyből ez a vágás készült, már nem érhető el." diff --git a/frontend/src/i18n/locales/hu/inbox.json b/frontend/src/i18n/locales/hu/inbox.json index 8f16fdba..204501d7 100644 --- a/frontend/src/i18n/locales/hu/inbox.json +++ b/frontend/src/i18n/locales/hu/inbox.json @@ -22,6 +22,9 @@ "editDone": { "title": "Vágás kész" }, + "uploadDone": { + "title": "A feltöltés megosztható" + }, "maintenance": { "title": "Videók eltávolítva", "body_one": "{{count}} mentett vagy lejátszási listás videó eltávolítva, mert már nem elérhető a YouTube-on.", diff --git a/frontend/src/lib/api/downloads.ts b/frontend/src/lib/api/downloads.ts index fcac578d..231b66a8 100644 --- a/frontend/src/lib/api/downloads.ts +++ b/frontend/src/lib/api/downloads.ts @@ -89,7 +89,7 @@ export interface DownloadJob { display_uploader: string | null; // user override of the channel name display_uploader_url: string | null; // user override of the channel link extra_links: string[]; // extra reference URLs (beyond source_url) - job_kind?: DownloadJobKind; // "download" (default) | "edit" (a per-user trim/crop derivative) + job_kind?: DownloadJobKind; // see DownloadJobKind; absent reads as "download" source_job_id?: number | null; // parent download an edit was cut from edit_spec?: EditSpec | null; can_download: boolean; diff --git a/frontend/src/lib/downloadFailure.test.ts b/frontend/src/lib/downloadFailure.test.ts index d4e0dcb9..579c333c 100644 --- a/frontend/src/lib/downloadFailure.test.ts +++ b/frontend/src/lib/downloadFailure.test.ts @@ -6,7 +6,7 @@ // must still say the category, not print a key. import { describe, expect, it } from "vitest"; import i18n from "../i18n"; -import { failCopyKeys } from "./downloadFailure"; +import { failCopyKeys, failTitleKey } from "./downloadFailure"; describe("failCopyKeys", () => { it("puts a catalog entry before its category, and is the category alone without one", () => { @@ -47,3 +47,22 @@ describe("failCopyKeys", () => { ); }); }); + +describe("failTitleKey", () => { + it("titles an upload's failure as a conversion, anything else as a download", 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({})).toBe("downloads.errors.fail.title"); + }); + + it("an upload's own failures read as an upload's", async () => { + await i18n.changeLanguage("en"); + for (const code of ["convert_failed", "upload_missing"]) { + const short = i18n.t(failCopyKeys({ error_code: code, error_detail: null }, "short")); + expect(short).not.toBe(i18n.t("downloads.errors.fail.cat.unknown.short")); + } + }); +}); diff --git a/frontend/src/lib/downloadFailure.ts b/frontend/src/lib/downloadFailure.ts index 2b55c329..612f4918 100644 --- a/frontend/src/lib/downloadFailure.ts +++ b/frontend/src/lib/downloadFailure.ts @@ -18,3 +18,10 @@ export function failCopyKeys( ...(code === "unknown" ? [] : [category("unknown")]), ]; } + +/** The title over a failed job's explanation: an upload's failure is a conversion, not a download. */ +export function failTitleKey(job: Pick): string { + return job.job_kind === "upload" + ? "downloads.errors.fail.titleUpload" + : "downloads.errors.fail.title"; +}