diff --git a/app/features/ytdlp/utils.py b/app/features/ytdlp/utils.py index 745f98fc..50e1ab2f 100644 --- a/app/features/ytdlp/utils.py +++ b/app/features/ytdlp/utils.py @@ -37,9 +37,7 @@ class _DATA: { "quiet": "-q, --quiet", "no_warnings": "--no-warnings", - "skip_download": "--skip-download", "forceprint": "-O, --print", - "simulate": "--simulate", "noprogress": "--no-progress", "wait_for_video": "--wait-for-video", "progress_delta": " --progress-delta", diff --git a/app/library/ItemDTO.py b/app/library/ItemDTO.py index f4ef3d85..91c5b211 100644 --- a/app/library/ItemDTO.py +++ b/app/library/ItemDTO.py @@ -381,6 +381,8 @@ class ItemDTO: """ The archive ID of the item. """ sidecar: dict = field(default_factory=dict) """ Sidecar data associated with the item. """ + download_skipped: bool = False + """ True when yt-dlp intentionally skips the primary media download. """ # yt-dlp injected fields. tmpfilename: str | None = None diff --git a/app/library/downloads/core.py b/app/library/downloads/core.py index 7dcf3907..ef18e746 100644 --- a/app/library/downloads/core.py +++ b/app/library/downloads/core.py @@ -92,9 +92,11 @@ class Download: download using yt-dlp. """ cookie_file: Path | None = None + params: dict[str, Any] = {} + download_skipped = False try: - params: dict = ( + params = ( self.info.get_ytdlp_opts() .add( config={ @@ -116,6 +118,7 @@ class Download: ) .get_all() ) + download_skipped = bool(params.get("skip_download") or params.get("simulate")) params.update( { @@ -212,7 +215,7 @@ class Download: signal.signal(signal.SIGUSR1, mark_cancelled) - self.status_queue.put({"id": self.id, "status": "downloading"}) + self.status_queue.put({"id": self.id, "status": "downloading", "download_skipped": download_skipped}) if isinstance(self.info_dict, dict) and len(self.info_dict) > 1: self.logger.debug(f"Downloading '{self.info.url}' using pre-info.") @@ -232,14 +235,35 @@ class Download: self.logger.debug(f"Downloading using url: {self.info.url}") ret = cls.download(url_list=[self.info.url]) - self.status_queue.put({"id": self.id, "status": "finished" if 0 == ret else "error"}) + self.status_queue.put( + { + "id": self.id, + "status": "finished" if 0 == ret else "error", + "download_skipped": download_skipped, + } + ) except yt_dlp.utils.ExistingVideoReached as exc: self.logger.error(exc) - self.status_queue.put({"id": self.id, "status": "skip", "msg": "Item has already been downloaded."}) + self.status_queue.put( + { + "id": self.id, + "status": "skip", + "msg": "Item has already been downloaded.", + "download_skipped": download_skipped, + } + ) except Exception as exc: self.logger.exception(exc) self.logger.error(exc) - self.status_queue.put({"id": self.id, "status": "error", "msg": str(exc), "error": str(exc)}) + self.status_queue.put( + { + "id": self.id, + "status": "error", + "msg": str(exc), + "error": str(exc), + "download_skipped": download_skipped, + } + ) finally: self.status_queue.put(Terminator()) if cookie_file and cookie_file.exists(): diff --git a/app/library/downloads/status_tracker.py b/app/library/downloads/status_tracker.py index edd248d8..d7a10748 100644 --- a/app/library/downloads/status_tracker.py +++ b/app/library/downloads/status_tracker.py @@ -128,6 +128,8 @@ class StatusTracker: self.tmpfilename = status.get("tmpfilename") self.info.status = status.get("status", self.info.status) + if "download_skipped" in status: + self.info.download_skipped = bool(status.get("download_skipped")) self.info.msg = status.get("msg") self.info.postprocessor = status.get("postprocessor", None) diff --git a/app/tests/test_download.py b/app/tests/test_download.py index f32a7d8a..2faf5193 100644 --- a/app/tests/test_download.py +++ b/app/tests/test_download.py @@ -272,6 +272,58 @@ class TestDownloadStale: class TestDownloadFlow: + def test_download_pushes_download_skipped_flag(self, monkeypatch: pytest.MonkeyPatch) -> None: + class Cfg: + debug = False + ytdlp_debug = False + max_workers = 1 + temp_keep = False + temp_disabled = True + download_info_expires = 3600 + + @staticmethod + def get_instance(): + return Cfg + + monkeypatch.setattr("app.library.downloads.core.Config", Cfg) + + download = Download( + info=make_item(), + info_dict={"id": "test-id", "url": "http://u", "formats": [{"format_id": "18"}]}, + ) + download.status_queue = cast(Any, DummyQueue()) + download._hook_handlers = Mock( + progress_hook=Mock(), + postprocessor_hook=Mock(), + post_hook=Mock(), + ) + download.info.get_ytdlp_opts = Mock( + return_value=Mock( + add=Mock( + return_value=Mock( + get_all=Mock(return_value={"skip_download": True}), + ) + ) + ) + ) + + class FakeYTDLP: + def __init__(self, params): + self.params = params + self._download_retcode = 0 + self._interrupted = False + + def process_ie_result(self, ie_result, download): + return ie_result, download + + monkeypatch.setattr("app.library.downloads.core.YTDLP", FakeYTDLP) + + download._download() + + queue = cast(DummyQueue, download.status_queue) + assert queue.items[0]["download_skipped"] is True + assert queue.items[1]["download_skipped"] is True + @pytest.mark.asyncio async def test_download_flow_inline_process(self, monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: class Cfg: @@ -724,6 +776,14 @@ class TestStatusTracker: await st.process_status_update(status) assert st.info.status == "downloading", "Should update info status" + @pytest.mark.asyncio + async def test_process_status_update_sets_download_skipped(self, mock_config: dict) -> None: + st = StatusTracker(**mock_config) + status = {"id": "test-id", "status": "downloading", "download_skipped": True} + + await st.process_status_update(status) + assert st.info.download_skipped is True, "Should update download_skipped from status queue" + @pytest.mark.asyncio async def test_process_status_update_sets_tmpfilename(self, mock_config: dict) -> None: st = StatusTracker(**mock_config) diff --git a/app/tests/test_itemdto.py b/app/tests/test_itemdto.py index 72a27a9a..0358d57d 100644 --- a/app/tests/test_itemdto.py +++ b/app/tests/test_itemdto.py @@ -110,6 +110,23 @@ class TestItemDTO: assert dto.is_archivable is True assert dto.is_archived is True + @patch("app.library.ItemDTO.get_archive_id") + @patch("app.library.ItemDTO.YTDLPOpts") + @patch("app.library.ItemDTO.archive_read") + def test_post_init_does_not_infer_download_skipped_flag(self, mock_read, mock_opts, mock_get_id): + mock_get_id.return_value = {"archive_id": "arch", "id": "arch", "ie_key": "YT"} + mock_opts.get_instance.return_value.preset.return_value = mock_opts.get_instance.return_value + mock_opts.get_instance.return_value.add_cli.return_value = mock_opts.get_instance.return_value + mock_opts.get_instance.return_value.get_all.return_value = { + "download_archive": "/tmp/a.txt", + "skip_download": True, + } + mock_read.return_value = ["arch"] + + dto = ItemDTO(id="vid", title="t", url="u", folder="f") + + assert dto.download_skipped is False + @patch("app.library.ItemDTO.archive_read") def test_serialize_triggers_archive_status_when_finished(self, mock_read): # Given a finished item with archive info @@ -125,12 +142,32 @@ class TestItemDTO: for key in ItemDTO.removed_fields(): assert key not in data + @patch("app.library.ItemDTO.YTDLPOpts") + def test_serialize_does_not_recompute_download_skipped(self, mock_opts): + mock_opts.get_instance.return_value.preset.return_value = mock_opts.get_instance.return_value + mock_opts.get_instance.return_value.add_cli.return_value = mock_opts.get_instance.return_value + mock_opts.get_instance.return_value.get_all.return_value = {} + + dto = ItemDTO(id="vid", title="t", url="u", folder="f", download_skipped=True) + + mock_opts.get_instance.reset_mock() + + data = dto.serialize() + + assert data["download_skipped"] is True + mock_opts.get_instance.assert_not_called() + @patch("app.library.ItemDTO.YTDLPOpts") def test_get_ytdlp_opts_uses_preset_and_cli(self, mock_opts): mock_opts.get_instance.return_value.preset.return_value = mock_opts.get_instance.return_value mock_opts.get_instance.return_value.add_cli.return_value = mock_opts.get_instance.return_value dto = ItemDTO(id="id", title="t", url="u", folder="f", preset="p", cli="--x") + + mock_opts.get_instance.reset_mock() + mock_opts.get_instance.return_value.preset.reset_mock() + mock_opts.get_instance.return_value.add_cli.reset_mock() + opts = dto.get_ytdlp_opts() mock_opts.get_instance.assert_called_once() diff --git a/app/tests/test_sqlite_store.py b/app/tests/test_sqlite_store.py index 3a52c3d8..263182dd 100644 --- a/app/tests/test_sqlite_store.py +++ b/app/tests/test_sqlite_store.py @@ -17,13 +17,15 @@ async def make_store() -> SqliteStore: return store -def make_item(idx: int, *, status: str = "finished") -> ItemDTO: +def make_item(idx: int, *, status: str = "finished", cli: str = "", download_skipped: bool = False) -> ItemDTO: return ItemDTO( id=f"vid{idx}", title=f"Video {idx}", url=f"https://example.com/{idx}", folder="/downloads", status=status, + cli=cli, + download_skipped=download_skipped, ) @@ -90,6 +92,22 @@ async def test_enqueue_upsert_and_fetch_saved(): await store.close() +@pytest.mark.asyncio +async def test_enqueue_upsert_persists_download_skipped_flag(): + store = await make_store() + item = make_item(2, download_skipped=True) + + await store.enqueue_upsert("history", item) + await store.flush() + + saved = await store.fetch_saved("history") + assert len(saved) == 1 + _, loaded = saved[0] + assert item.download_skipped is True + assert loaded.download_skipped is True + await store.close() + + @pytest.mark.asyncio async def test_delete_and_bulk_delete(): store = await make_store() diff --git a/ui/app/components/History.vue b/ui/app/components/History.vue index 8c3c96dd..0bd3dd7c 100644 --- a/ui/app/components/History.vue +++ b/ui/app/components/History.vue @@ -214,7 +214,7 @@
const getGridImage = (item: StoreItem): string => getImage(config.app.download_path, item) || ''; +const showRetryAction = (item: StoreItem): boolean => !item.filename && !isDownloadSkipped(item); + const historyCardClass = (item: StoreItem): string => { if (item.status === 'error') { return 'border-error'; @@ -1038,6 +1040,9 @@ const clearIncomplete = async () => { const setIcon = (item: StoreItem) => { if ('finished' === item.status) { + if (isDownloadSkipped(item)) { + return 'i-lucide-ban'; + } if (!item.filename) { return 'i-lucide-triangle-alert'; } @@ -1063,6 +1068,9 @@ const setIcon = (item: StoreItem) => { const setIconColor = (item: StoreItem) => { if ('finished' === item.status) { + if (isDownloadSkipped(item)) { + return 'text-info'; + } if (!item.filename) { return 'text-warning'; } @@ -1084,6 +1092,9 @@ const setIconColor = (item: StoreItem) => { const setStatus = (item: StoreItem) => { if ('finished' === item.status) { + if (isDownloadSkipped(item)) { + return 'Download skipped'; + } if (item.extras?.is_premiere) { return 'Premiered'; } diff --git a/ui/app/components/Simple.vue b/ui/app/components/Simple.vue index aae4dc1e..b2401907 100644 --- a/ui/app/components/Simple.vue +++ b/ui/app/components/Simple.vue @@ -445,6 +445,7 @@ import { ag, encodePath, formatTime, + isDownloadSkipped, makeDownload, request, stripPath, @@ -812,6 +813,10 @@ const getStatusLabel = (item: StoreItem): string => { return 'Queued'; } + if (isDownloadSkipped(item)) { + return 'Download skipped'; + } + if (item.status === 'error' && item.filename) { return 'Partial Error'; } @@ -824,6 +829,10 @@ const getStatusColor = (item: StoreItem): 'neutral' | 'info' | 'success' | 'erro return 'neutral'; } + if (isDownloadSkipped(item)) { + return 'info'; + } + if (item.status === 'error' && item.filename) { return 'warning'; } diff --git a/ui/app/types/store.d.ts b/ui/app/types/store.d.ts index 268758fd..ad2f7f18 100644 --- a/ui/app/types/store.d.ts +++ b/ui/app/types/store.d.ts @@ -70,6 +70,8 @@ type StoreItem = { subtitle?: Array; image?: Array; }; + /** If the primary media download was intentionally skipped */ + download_skipped?: boolean; /** Extras for the item */ extras: { /** Which channel the item belongs to */ diff --git a/ui/app/utils/index.ts b/ui/app/utils/index.ts index 548c017c..08b6a4e3 100644 --- a/ui/app/utils/index.ts +++ b/ui/app/utils/index.ts @@ -793,6 +793,10 @@ const shortPath = (path: string, prefix: string = '...'): string => { return `${prefix}/${parts.at(-1)}${hasTrailingSlash ? '/' : ''}`; }; +const isDownloadSkipped = ( + item: Pick | null | undefined, +): boolean => Boolean(item && item.status === 'finished' && item.download_skipped); + /** * Recursively test if a value (including nested objects/arrays) contains a query string. * - Plain queries match keys or values (case-insensitive). @@ -1028,6 +1032,7 @@ export { enableOpacity, stripPath, shortPath, + isDownloadSkipped, deepIncludes, getPath, getImage, diff --git a/ui/tests/utils/index.test.ts b/ui/tests/utils/index.test.ts index c32ab448..3cd931fe 100644 --- a/ui/tests/utils/index.test.ts +++ b/ui/tests/utils/index.test.ts @@ -388,6 +388,16 @@ describe('data conversion helpers', () => { const url = utils.makeDownload({}, { filename: 'playlist' }, 'm3u8'); expect(url).toBe('/base-path/api/player/m3u8/video/playlist.m3u8'); }); + + it('isDownloadSkipped detects finished skipped-download items', () => { + expect(utils.isDownloadSkipped({ status: 'finished', download_skipped: true } as any)).toBe(true); + }); + + it('isDownloadSkipped ignores unfinished or unflagged items', () => { + expect(utils.isDownloadSkipped({ status: 'finished', download_skipped: false } as any)).toBe(false); + expect(utils.isDownloadSkipped({ status: 'downloading', download_skipped: true } as any)).toBe(false); + expect(utils.isDownloadSkipped(undefined as any)).toBe(false); + }); }); describe('dom and browser helpers', () => {