From 044fdeba9ba4274450b0eb159bde20f89b1386d6 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 4 Oct 2026 06:59:02 +0000 Subject: [PATCH 1/3] fix: re-find expired Prowlarr releases the way the book's release search does When a queued release's indexer link has expired, the handler re-searches Prowlarr for the same release. That search was built only from the download's title, so a book with a subtitle ("Good Omens: The Nice and Accurate Prophecies of Agnes Nutter, Witch") searched an indexer that ANDs its terms with the whole thing and found nothing. The download then failed with "the release could not be refreshed", although a search from the book's page found the same release straight away. The refresh now searches like the book's own release search: - First with the metadata provider's record of the book (from the download's book_key), with its looser search title and its ISBNs. - Then the same, widened. - Last, a book built from the download, searched by its title without the subtitle and its first author. It stops at the first search that finds the exact same release, and still never substitutes a different one. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01V88DbAixT45r3AJsyFJchY --- shelfmark/release_sources/prowlarr/handler.py | 118 ++++++++++++++---- tests/prowlarr/test_handler.py | 117 +++++++++++++++++ 2 files changed, 208 insertions(+), 27 deletions(-) diff --git a/shelfmark/release_sources/prowlarr/handler.py b/shelfmark/release_sources/prowlarr/handler.py index 57c4fa822..b0f305664 100644 --- a/shelfmark/release_sources/prowlarr/handler.py +++ b/shelfmark/release_sources/prowlarr/handler.py @@ -7,6 +7,7 @@ from shelfmark.core.config import config from shelfmark.core.logger import setup_logger +from shelfmark.core.naming import derive_primary_title from shelfmark.core.request_helpers import normalize_optional_text from shelfmark.core.search_plan import build_release_search_plan from shelfmark.core.utils import normalize_http_url @@ -306,7 +307,15 @@ def _resolve_download( ) def _refresh_release(self, task: DownloadTask) -> dict[str, Any] | None: - """Re-query Prowlarr and cache the exact original release if it still exists.""" + """Re-query Prowlarr and cache the exact original release if it still exists. + + Searched the way the book's own release search does first: with the metadata + provider's record for the book (its looser search title and its ISBNs), then + widened. A search built only from the download's title can find nothing on an + indexer that ANDs its terms ("Good Omens: The Nice and Accurate Prophecies of + Agnes Nutter, Witch") although the release is still there, which left the user + to search again by hand for the same result. + """ title = normalize_optional_text(task.title) if title is None: return None @@ -314,38 +323,93 @@ def _refresh_release(self, task: DownloadTask) -> dict[str, Any] | None: context = getattr(task, "retry_source_context", None) if not isinstance(context, dict): context = {} - indexer = normalize_optional_text(context.get("indexer")) - book = BookMetadata( - provider="shelfmark", - provider_id=task.task_id, - title=title, - authors=[task.author] if task.author else [], - search_title=title, - search_author=task.author, - ) - # No language default here on purpose: this re-finds one exact release by its - # guid, and Prowlarr does not filter on plan.languages anyway. - plan = build_release_search_plan( - book, - indexers=[indexer] if indexer is not None else None, - ) + content_type = task.content_type or "ebook" source = ProwlarrSource() - results = source.search(book, plan, content_type=task.content_type or "ebook") - for release in results: - raw_release = get_release(release.source_id) - if raw_release is None: - continue - if not self._raw_release_matches_task(raw_release, task.task_id): - continue - - cache_release(task.task_id, raw_release) - logger.info("Refreshed Prowlarr release: %s", task.task_id) - return raw_release + for book, expand_search in self._refresh_searches(task, title): + # No language default here on purpose: this re-finds one exact release by + # its guid, and Prowlarr does not filter on plan.languages anyway. + plan = build_release_search_plan( + book, + indexers=[indexer] if indexer is not None else None, + ) + results = source.search( + book, plan, expand_search=expand_search, content_type=content_type + ) + for release in results: + raw_release = get_release(release.source_id) + if raw_release is None: + continue + if not self._raw_release_matches_task(raw_release, task.task_id): + continue + + cache_release(task.task_id, raw_release) + logger.info("Refreshed Prowlarr release: %s", task.task_id) + return raw_release return None + def _refresh_searches(self, task: DownloadTask, title: str) -> list[tuple[BookMetadata, bool]]: + """The searches that re-find a release, most like the user's own first. + + Each is (book, expand_search). The metadata provider's book when the download + recorded one, as the book's release search uses it, then widened; then a book + built from the download itself (its title without the subtitle). + """ + searches: list[tuple[BookMetadata, bool]] = [] + provider_book = self._provider_book(task) + if provider_book is not None: + searches.extend([(provider_book, False), (provider_book, True)]) + + authors = [a.strip() for a in (task.author or "").split(",") if a.strip()] + primary_title = derive_primary_title(title, task.subtitle) or title + searches.append( + ( + BookMetadata( + provider="shelfmark", + provider_id=task.task_id, + title=title, + authors=authors, + search_title=primary_title, + search_author=authors[0] if authors else None, + ), + False, + ) + ) + return searches + + @staticmethod + def _provider_book(task: DownloadTask) -> BookMetadata | None: + """The metadata provider's record of the book the download was for, if known.""" + book_key = normalize_optional_text(getattr(task, "book_key", None)) + if book_key is None or ":" not in book_key: + return None + provider, _, provider_id = book_key.partition(":") + if not provider or not provider_id or provider in {"manual", "id"}: + return None + + from shelfmark.metadata_providers import ( + get_provider, + get_provider_kwargs, + is_provider_registered, + ) + + if not is_provider_registered(provider): + return None + try: + return get_provider(provider, **get_provider_kwargs(provider)).get_book(provider_id) + except ( + OSError, + RuntimeError, + ValueError, + KeyError, + TypeError, + requests.exceptions.RequestException, + ) as exc: + logger.debug("Could not load %s to refresh %s: %s", book_key, task.task_id, exc) + return None + @staticmethod def _raw_release_matches_task(raw_release: dict[str, Any], task_id: str) -> bool: wanted = normalize_optional_text(task_id) diff --git a/tests/prowlarr/test_handler.py b/tests/prowlarr/test_handler.py index f70a1a108..40d23ea22 100644 --- a/tests/prowlarr/test_handler.py +++ b/tests/prowlarr/test_handler.py @@ -1763,3 +1763,120 @@ def test_matches_a_qualified_id_built_from_a_guidless_release(self): built = build_source_id(raw) assert ProwlarrHandler._raw_release_matches_task(raw, built) + + +class TestRefreshSearches: + """An expired link is re-found the way the book's own release search finds it.""" + + TASK_ID = "12:https://www.myanonamouse.net/t/168537" + + def _task(self, **fields): + return DownloadTask( + task_id=self.TASK_ID, + source="prowlarr", + title="Good Omens: The Nice and Accurate Prophecies of Agnes Nutter, Witch", + subtitle="The Nice and Accurate Prophecies of Agnes Nutter, Witch", + author="Neil Gaiman, Terry Pratchett", + content_type="audiobook", + retry_source_context={"indexer": "MyAnonamouse"}, + **fields, + ) + + def _search_finding_on(self, found_on_call: int, searched: list): + """A Prowlarr search that finds the release only on call number `found_on_call`.""" + + def search(book, plan, *, expand_search=False, content_type="ebook"): + searched.append((book.provider, book.search_title, expand_search, content_type)) + if len(searched) != found_on_call: + return [] + raw = {"guid": "https://www.myanonamouse.net/t/168537", "indexerId": 12} + cache_release(self.TASK_ID + ":fresh", {**raw, "protocol": "torrent"}) + return [ + Release( + source="prowlarr", + source_id=self.TASK_ID + ":fresh", + title="Good Omens", + protocol=ReleaseProtocol.TORRENT, + ) + ] + + return search + + def _provider(self, book): + provider = MagicMock() + provider.get_book.return_value = book + return provider + + def test_searches_with_the_provider_book_first(self): + from shelfmark.metadata_providers import BookMetadata + + provider_book = BookMetadata( + provider="hardcover", + provider_id="42", + title="Good Omens", + authors=["Neil Gaiman", "Terry Pratchett"], + search_title="Good Omens", + ) + searched: list = [] + remove_release(self.TASK_ID) + try: + with ( + patch( + "shelfmark.release_sources.prowlarr.handler.ProwlarrSource.search", + side_effect=self._search_finding_on(2, searched), + ), + patch("shelfmark.metadata_providers.is_provider_registered", return_value=True), + patch("shelfmark.metadata_providers.get_provider_kwargs", return_value={}), + patch( + "shelfmark.metadata_providers.get_provider", + return_value=self._provider(provider_book), + ), + ): + raw = ProwlarrHandler()._refresh_release(self._task(book_key="hardcover:42")) + finally: + remove_release(self.TASK_ID) + remove_release(self.TASK_ID + ":fresh") + + assert raw is not None + # The provider's book as the release search uses it, then widened: found there. + assert searched == [ + ("hardcover", "Good Omens", False, "audiobook"), + ("hardcover", "Good Omens", True, "audiobook"), + ] + + def test_without_a_provider_book_searches_the_title_without_its_subtitle(self): + searched: list = [] + try: + with patch( + "shelfmark.release_sources.prowlarr.handler.ProwlarrSource.search", + side_effect=self._search_finding_on(1, searched), + ): + raw = ProwlarrHandler()._refresh_release(self._task()) + finally: + remove_release(self.TASK_ID) + remove_release(self.TASK_ID + ":fresh") + + assert raw is not None + assert searched == [("shelfmark", "Good Omens", False, "audiobook")] + + def test_a_provider_that_fails_falls_back_to_the_download_title(self): + provider = MagicMock() + provider.get_book.side_effect = RuntimeError("Hardcover is down") + searched: list = [] + try: + with ( + patch( + "shelfmark.release_sources.prowlarr.handler.ProwlarrSource.search", + side_effect=self._search_finding_on(1, searched), + ), + patch("shelfmark.metadata_providers.is_provider_registered", return_value=True), + patch("shelfmark.metadata_providers.get_provider_kwargs", return_value={}), + patch("shelfmark.metadata_providers.get_provider", return_value=provider), + ): + raw = ProwlarrHandler()._refresh_release(self._task(book_key="hardcover:42")) + finally: + remove_release(self.TASK_ID) + remove_release(self.TASK_ID + ":fresh") + + assert raw is not None + assert searched == [("shelfmark", "Good Omens", False, "audiobook")] From 98d1f81335018df9c0699424dd196777504b67b0 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 4 Oct 2026 07:01:30 +0000 Subject: [PATCH 2/3] fix: also re-find an expired release by title and author A short, common title ("Eric", by Terry Pratchett) searched alone brings back so many other books ("Eric Brown - Helix") that the release being refreshed isn't among the results. Searching by hand for "Eric Terry Pratchett" found it at once. The refresh now adds that search: the search title and first author as one query. It runs after the book's own release search and before the search without content-type categories, which only widens the categories and doesn't help here. Without a provider book: the download's title without its subtitle, then title and author. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01V88DbAixT45r3AJsyFJchY --- shelfmark/release_sources/prowlarr/handler.py | 82 ++++++++++++------- tests/prowlarr/test_handler.py | 42 ++++++++-- 2 files changed, 86 insertions(+), 38 deletions(-) diff --git a/shelfmark/release_sources/prowlarr/handler.py b/shelfmark/release_sources/prowlarr/handler.py index b0f305664..7c095c2d8 100644 --- a/shelfmark/release_sources/prowlarr/handler.py +++ b/shelfmark/release_sources/prowlarr/handler.py @@ -1,5 +1,6 @@ """Prowlarr download handler - resolves releases and delegates lifecycle to shared clients.""" +from dataclasses import dataclass from typing import TYPE_CHECKING, Any from urllib.parse import urlparse @@ -9,7 +10,7 @@ from shelfmark.core.logger import setup_logger from shelfmark.core.naming import derive_primary_title from shelfmark.core.request_helpers import normalize_optional_text -from shelfmark.core.search_plan import build_release_search_plan +from shelfmark.core.search_plan import build_release_search_plan, pick_search_author from shelfmark.core.utils import normalize_http_url from shelfmark.download.clients import ( DownloadClient, @@ -90,6 +91,15 @@ def _coerce_positive_minutes(raw_minutes: object) -> int | None: return minutes if minutes > 0 else None +@dataclass(frozen=True) +class _RefreshSearch: + """One Prowlarr search for re-finding an expired release.""" + + book: BookMetadata + query: str | None = None # Searched as typed, like a manual search + expand: bool = False # Without the content-type categories + + @register_handler("prowlarr") class ProwlarrHandler(ExternalClientHandler): """Handler for Prowlarr downloads via configured torrent or usenet client.""" @@ -309,12 +319,11 @@ def _resolve_download( def _refresh_release(self, task: DownloadTask) -> dict[str, Any] | None: """Re-query Prowlarr and cache the exact original release if it still exists. - Searched the way the book's own release search does first: with the metadata - provider's record for the book (its looser search title and its ISBNs), then - widened. A search built only from the download's title can find nothing on an - indexer that ANDs its terms ("Good Omens: The Nice and Accurate Prophecies of - Agnes Nutter, Witch") although the release is still there, which left the user - to search again by hand for the same result. + Searched the way the user would find it again (see `_refresh_searches`). A + search built only from the download's title can miss a release that is still + there: an indexer that ANDs its terms finds nothing for a long title with its + subtitle, and a short title ("Eric") brings back so many other books that the + release isn't among the results. That left the user to search by hand. """ title = normalize_optional_text(task.title) if title is None: @@ -327,15 +336,16 @@ def _refresh_release(self, task: DownloadTask) -> dict[str, Any] | None: content_type = task.content_type or "ebook" source = ProwlarrSource() - for book, expand_search in self._refresh_searches(task, title): + for search in self._refresh_searches(task, title): # No language default here on purpose: this re-finds one exact release by # its guid, and Prowlarr does not filter on plan.languages anyway. plan = build_release_search_plan( - book, + search.book, + manual_query=search.query, indexers=[indexer] if indexer is not None else None, ) results = source.search( - book, plan, expand_search=expand_search, content_type=content_type + search.book, plan, expand_search=search.expand, content_type=content_type ) for release in results: raw_release = get_release(release.source_id) @@ -350,34 +360,44 @@ def _refresh_release(self, task: DownloadTask) -> dict[str, Any] | None: return None - def _refresh_searches(self, task: DownloadTask, title: str) -> list[tuple[BookMetadata, bool]]: - """The searches that re-find a release, most like the user's own first. + def _refresh_searches(self, task: DownloadTask, title: str) -> list[_RefreshSearch]: + """The searches that re-find a release, in order, stopping at the first hit. - Each is (book, expand_search). The metadata provider's book when the download - recorded one, as the book's release search uses it, then widened; then a book - built from the download itself (its title without the subtitle). + With the metadata provider's record of the book (when the download noted it): + the book's own release search, then its title and author as one query (for a + title so common the release is lost among other books), then without the + content-type categories. Without it: the download's title without its + subtitle, then title and author. """ - searches: list[tuple[BookMetadata, bool]] = [] provider_book = self._provider_book(task) if provider_book is not None: - searches.extend([(provider_book, False), (provider_book, True)]) + query = self._title_author_query(provider_book) + return [ + _RefreshSearch(provider_book), + *([_RefreshSearch(provider_book, query=query)] if query else []), + _RefreshSearch(provider_book, expand=True), + ] authors = [a.strip() for a in (task.author or "").split(",") if a.strip()] - primary_title = derive_primary_title(title, task.subtitle) or title - searches.append( - ( - BookMetadata( - provider="shelfmark", - provider_id=task.task_id, - title=title, - authors=authors, - search_title=primary_title, - search_author=authors[0] if authors else None, - ), - False, - ) + book = BookMetadata( + provider="shelfmark", + provider_id=task.task_id, + title=title, + authors=authors, + search_title=derive_primary_title(title, task.subtitle) or title, + search_author=authors[0] if authors else None, ) - return searches + query = self._title_author_query(book) + return [_RefreshSearch(book), *([_RefreshSearch(book, query=query)] if query else [])] + + @staticmethod + def _title_author_query(book: BookMetadata) -> str | None: + """ "Eric Terry Pratchett": the search title and first author, or None without one.""" + author = pick_search_author(book) + search_title = book.search_title or book.title + if not author or not search_title: + return None + return f"{search_title} {author}" @staticmethod def _provider_book(task: DownloadTask) -> BookMetadata | None: diff --git a/tests/prowlarr/test_handler.py b/tests/prowlarr/test_handler.py index 40d23ea22..b8e6787a7 100644 --- a/tests/prowlarr/test_handler.py +++ b/tests/prowlarr/test_handler.py @@ -1786,7 +1786,7 @@ def _search_finding_on(self, found_on_call: int, searched: list): """A Prowlarr search that finds the release only on call number `found_on_call`.""" def search(book, plan, *, expand_search=False, content_type="ebook"): - searched.append((book.provider, book.search_title, expand_search, content_type)) + searched.append((book.provider, book.search_title, expand_search, plan.manual_query)) if len(searched) != found_on_call: return [] raw = {"guid": "https://www.myanonamouse.net/t/168537", "indexerId": 12} @@ -1823,7 +1823,7 @@ def test_searches_with_the_provider_book_first(self): with ( patch( "shelfmark.release_sources.prowlarr.handler.ProwlarrSource.search", - side_effect=self._search_finding_on(2, searched), + side_effect=self._search_finding_on(3, searched), ), patch("shelfmark.metadata_providers.is_provider_registered", return_value=True), patch("shelfmark.metadata_providers.get_provider_kwargs", return_value={}), @@ -1838,10 +1838,12 @@ def test_searches_with_the_provider_book_first(self): remove_release(self.TASK_ID + ":fresh") assert raw is not None - # The provider's book as the release search uses it, then widened: found there. + # The provider's book as the release search uses it, then title and author, + # then without categories: found there. assert searched == [ - ("hardcover", "Good Omens", False, "audiobook"), - ("hardcover", "Good Omens", True, "audiobook"), + ("hardcover", "Good Omens", False, None), + ("hardcover", "Good Omens", False, "Good Omens Neil Gaiman"), + ("hardcover", "Good Omens", True, None), ] def test_without_a_provider_book_searches_the_title_without_its_subtitle(self): @@ -1857,7 +1859,33 @@ def test_without_a_provider_book_searches_the_title_without_its_subtitle(self): remove_release(self.TASK_ID + ":fresh") assert raw is not None - assert searched == [("shelfmark", "Good Omens", False, "audiobook")] + assert searched == [("shelfmark", "Good Omens", False, None)] + + def test_a_common_title_is_searched_with_its_author(self): + # "Eric" alone brings back other books ("Eric Brown - Helix") and not this one. + searched: list = [] + task = DownloadTask( + task_id=self.TASK_ID, + source="prowlarr", + title="Eric", + author="Terry Pratchett", + content_type="audiobook", + ) + try: + with patch( + "shelfmark.release_sources.prowlarr.handler.ProwlarrSource.search", + side_effect=self._search_finding_on(2, searched), + ): + raw = ProwlarrHandler()._refresh_release(task) + finally: + remove_release(self.TASK_ID) + remove_release(self.TASK_ID + ":fresh") + + assert raw is not None + assert searched == [ + ("shelfmark", "Eric", False, None), + ("shelfmark", "Eric", False, "Eric Terry Pratchett"), + ] def test_a_provider_that_fails_falls_back_to_the_download_title(self): provider = MagicMock() @@ -1879,4 +1907,4 @@ def test_a_provider_that_fails_falls_back_to_the_download_title(self): remove_release(self.TASK_ID + ":fresh") assert raw is not None - assert searched == [("shelfmark", "Good Omens", False, "audiobook")] + assert searched == [("shelfmark", "Good Omens", False, None)] From 0d4979685448036a4436120d69428a753dc7ec70 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 4 Oct 2026 07:18:42 +0000 Subject: [PATCH 3/3] feat: simpler Queued and Wanted tables MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Queued (waiting for room) - No "Queue for download" checkbox: Remove already takes an item out, and an unticked box quietly stopped it downloading. - "Waiting for" shows what the automatic check found ("2 unsatisfied slots", "Freeleech or ratio 2.00: …"), "Last checked" when it last looked, and "Next check" counts down to the next run. GET /api/saved now says when the next check is due (next_check_at); the page reads the list again just after it, so the countdown moves on to the following one. - Releases are listed one per line: "Ebook, EPUB", "Audiobook, Tony Robinson". Wanted - One Releases column with "Choose releases", which opens the release picker. Picks saved there put the book in the queue. No Automatic download column. - Wanted items that already have releases picked move into the queue themselves. One whose last automatic download partly failed stays, with its error, "Choose again" and "+ Get", so it isn't retried blind. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01V88DbAixT45r3AJsyFJchY --- shelfmark/core/saved_autoget.py | 18 ++ shelfmark/core/saved_routes.py | 14 +- shelfmark/main.py | 1 + src/frontend/src/App.tsx | 18 +- .../src/components/activity/SavedPanel.tsx | 286 ++++++++++-------- .../src/contexts/SavedItemsContext.tsx | 38 ++- src/frontend/src/services/api.ts | 12 +- src/frontend/src/tests/savedItems.test.ts | 43 +++ src/frontend/src/utils/savedItems.ts | 31 ++ tests/core/test_saved_autoget.py | 26 ++ tests/core/test_saved_items.py | 15 + 11 files changed, 361 insertions(+), 141 deletions(-) diff --git a/shelfmark/core/saved_autoget.py b/shelfmark/core/saved_autoget.py index db8cf1d5c..b8564fa0d 100644 --- a/shelfmark/core/saved_autoget.py +++ b/shelfmark/core/saved_autoget.py @@ -24,6 +24,7 @@ import copy import math import threading +import time from dataclasses import dataclass, field from typing import TYPE_CHECKING, Any @@ -239,6 +240,10 @@ def __init__(self, service: SavedItemsService, hooks: AutoGetHooks) -> None: self._thread: threading.Thread | None = None self._thread_lock = threading.Lock() self._soon: threading.Timer | None = None + # When the scheduled check and a pending quick check (`check_soon`) run, in epoch + # seconds, for the Queued page. + self._next_check_at: float | None = None + self._soon_at: float | None = None def run_check(self) -> RunReport: """Check every item marked for automatic download, oldest first.""" @@ -302,6 +307,7 @@ def _check_item(self, item: dict[str, Any], run: _RunContext, report: RunReport) def _loop(self) -> None: delay: float = _STARTUP_DELAY_SECONDS while True: + self._next_check_at = time.time() + delay self._wake.wait(delay) self._wake.clear() try: @@ -310,6 +316,15 @@ def _loop(self) -> None: logger.exception("Saved items automatic download check failed") delay = interval_minutes() * 60 + def next_check_at(self) -> float | None: + """When the next check is due (epoch seconds), or None when none is scheduled.""" + if not enabled(): + return None + with self._thread_lock: + running = self._thread is not None and self._thread.is_alive() + due = [at for at in (self._next_check_at, self._soon_at) if at is not None] + return min(due) if running and due else None + def start(self) -> None: """Start the background schedule (idempotent).""" with self._thread_lock: @@ -325,11 +340,14 @@ def check_soon(self) -> None: with self._thread_lock: if self._soon is not None and self._soon.is_alive(): return + self._soon_at = time.time() + _SOON_DELAY_SECONDS self._soon = threading.Timer(_SOON_DELAY_SECONDS, self._run_soon) self._soon.daemon = True self._soon.start() def _run_soon(self) -> None: + with self._thread_lock: + self._soon_at = None # Running now: the schedule says when the next one is try: self.run_check() except Exception: # a background check must not raise diff --git a/shelfmark/core/saved_routes.py b/shelfmark/core/saved_routes.py index 438640191..b6876e7da 100644 --- a/shelfmark/core/saved_routes.py +++ b/shelfmark/core/saved_routes.py @@ -46,8 +46,13 @@ def register_saved_routes( login_required: Callable[..., Any], resolve_auth_mode: Callable[[], str], on_auto_get: Callable[[], None] | None = None, + next_check_at: Callable[[], float | None] | None = None, ) -> None: - """Register the saved items API; `on_auto_get` runs when an item is marked.""" + """Register the saved items API. + + `on_auto_get` runs when an item is marked; `next_check_at` says when the automatic + check next runs (epoch seconds), shown beside the queued items. + """ @app.route("/api/saved", methods=["GET"]) @login_required @@ -55,7 +60,12 @@ def api_saved_list() -> Response | tuple[Response, int]: owner = _owner(resolve_auth_mode) if owner is None: return _no_owner() - return jsonify({"items": service.list_items(owner)}) + return jsonify( + { + "items": service.list_items(owner), + "next_check_at": next_check_at() if next_check_at is not None else None, + } + ) @app.route("/api/saved", methods=["POST"]) @login_required diff --git a/shelfmark/main.py b/shelfmark/main.py index d9fa0e651..91f76f0bf 100644 --- a/shelfmark/main.py +++ b/shelfmark/main.py @@ -1248,6 +1248,7 @@ def _saved_notify(item: dict[str, Any], owner: SavedOwner) -> None: login_required, get_auth_mode, on_auto_get=saved_autogetter.check_soon, + next_check_at=saved_autogetter.next_check_at, ) saved_autogetter.start() diff --git a/src/frontend/src/App.tsx b/src/frontend/src/App.tsx index f83d70fde..1876ee7b1 100644 --- a/src/frontend/src/App.tsx +++ b/src/frontend/src/App.tsx @@ -2064,25 +2064,39 @@ function App() { } }; + // Wanted: pick releases for the book; saving them there moves it into the queue. + const handleSavedChoose = async (item: SavedItem): Promise => { + try { + if (effectiveSearchMode === 'universal') { + await handleGetReleases(item.book); + } else { + await handleDownload(item.book); + } + } catch (error) { + console.warn('Could not open releases for saved item:', error); + } + }; + const savedPanelFor = (stage: SavedStage) => ( savedStore.remove(item)} onRefresh={savedStore.refresh} - onAutoGet={savedStore.setAutoGet} onOpenDetails={(item) => { setDetailsNotice(null); void showBookDetails(item.book, item.book.id); }} + onChooseReleases={handleSavedChoose} // Queued: numbered on from the downloads up next, which go first. onReorder={stage === 'queued' ? savedStore.reorder : undefined} positionOffset={stage === 'queued' ? statusCounts.queued : 0} + nextCheckAt={stage === 'queued' ? savedStore.nextCheckAt : null} /> ); + const savedCounts: Record = { queued: savedStore.items.filter((item) => savedStage(item) === 'queued').length, later: savedStore.items.filter((item) => savedStage(item) === 'later').length, diff --git a/src/frontend/src/components/activity/SavedPanel.tsx b/src/frontend/src/components/activity/SavedPanel.tsx index dadbd6d8e..f50ac1ef6 100644 --- a/src/frontend/src/components/activity/SavedPanel.tsx +++ b/src/frontend/src/components/activity/SavedPanel.tsx @@ -1,15 +1,17 @@ import { useState } from 'react'; -import { useMountEffect } from '../../hooks/useMountEffect'; +import { useDependencyEffect, useMountEffect } from '../../hooks/useMountEffect'; import { useRowDrag } from '../../hooks/useRowDrag'; import { withBasePath } from '../../utils/basePath'; +import { formatCountdown } from '../../utils/mamAccount'; import { formatDateTime, isoTimeAgo } from '../../utils/relativeTime'; import { SAVED_STAGE_LABELS, describeSavedPick, - hasMamPick, queueOrder, + savedPickLines, savedStage, + waitingFor, type SavedItem, type SavedStage, } from '../../utils/savedItems'; @@ -36,19 +38,21 @@ import { interface SavedPanelProps { items: SavedItem[]; loaded: boolean; - // Whether items can be marked to download on their own (the global setting). - autoGetAvailable: boolean; + // Downloads the picked releases now. onGet: (item: SavedItem) => Promise; onRemove: (item: SavedItem) => Promise; // Re-reads the list, for what the background checks found since. onRefresh: () => Promise; - onAutoGet: (item: SavedItem, changes: { auto_get: boolean }) => Promise; // Opens the book's details, as clicking a book does on the other pages. onOpenDetails?: (item: SavedItem) => void; + // Wanted: opens the release picker; picks saved there move the book to the queue. + onChooseReleases?: (item: SavedItem) => Promise; // Queued only: put the items in this order (ids, first claim on room first). onReorder?: (ids: number[]) => Promise; // Queued only: how many downloads are ahead of these, to number on from. positionOffset?: number; + // Queued only: when the automatic check next runs (ms since epoch), or null. + nextCheckAt?: number | null; } // A queued row's place in line and its move buttons. @@ -62,55 +66,10 @@ interface RowQueue { row?: ReturnType['rowProps']>; } -const AutoGetControls = ({ - item, - onAutoGet, -}: { - item: SavedItem; - onAutoGet: SavedPanelProps['onAutoGet']; -}) => { - const [busy, setBusy] = useState(false); - const toggle = async (autoGet: boolean) => { - setBusy(true); - try { - await onAutoGet(item, { auto_get: autoGet }); - } finally { - setBusy(false); - } - }; - const checkedAgo = item.auto_checked_at ? isoTimeAgo(item.auto_checked_at) : null; - let status = 'Waiting for the next check'; - if (item.auto_status) { - status = checkedAgo ? `${item.auto_status} · checked ${checkedAgo}` : item.auto_status; - } - - return ( -
- - {item.auto_get && ( -

- {status} -

- )} -
- ); -}; +type RowProps = Omit< + SavedPanelProps, + 'items' | 'loaded' | 'onRefresh' | 'onReorder' | 'positionOffset' +> & { item: SavedItem; queue?: RowQueue }; const coverUrl = (preview: string | undefined): string | null => { if (!preview) return null; @@ -126,31 +85,66 @@ const savedSortValue = (item: SavedItem, key: SavedSortKey): string | number | u return Number.isNaN(saved) ? undefined : saved; }; +const smallButton = + 'rounded-full px-2.5 py-1 text-xs font-medium whitespace-nowrap transition-colors disabled:opacity-40'; +const greenButton = `${smallButton} bg-emerald-600 text-white hover:bg-emerald-700`; +const quietButton = `${smallButton} opacity-70 hover:bg-(--hover-surface) hover:opacity-100`; + +/** "in 4:32" to the next automatic check, ticking; "Checking now" once it's due. */ +const NextCheck = ({ at }: { at: number }) => { + const [now, setNow] = useState(() => Date.now()); + useDependencyEffect(() => { + const timer = window.setInterval(() => setNow(Date.now()), 1000); + return () => window.clearInterval(timer); + }, [at]); + const seconds = (at - now) / 1000; + return ( + + {seconds > 0 ? `in ${formatCountdown(seconds)}` : 'Checking now'} + + ); +}; + +const ReleaseLines = ({ item }: { item: SavedItem }) => { + const lines = savedPickLines(item); + const full = savedPickLines(item, { full: true }); + return ( +
    + {lines.map((line, index) => ( + // Lines can repeat ("Audiobook" twice): the index tells them apart. + // eslint-disable-next-line react/no-array-index-key +
  • + {line} +
    • + ))} +
+ ); +}; + const SavedRow = ({ item, - autoGetAvailable, onGet, onRemove, - onAutoGet, onOpenDetails, + onChooseReleases, + nextCheckAt, queue, -}: { item: SavedItem; queue?: RowQueue } & Omit< - SavedPanelProps, - 'items' | 'loaded' | 'onRefresh' | 'onReorder' | 'positionOffset' ->) => { - const [busy, setBusy] = useState<'get' | 'remove' | null>(null); - const run = async (action: 'get' | 'remove') => { +}: RowProps) => { + const [busy, setBusy] = useState<'get' | 'choose' | 'remove' | null>(null); + const run = async (action: 'get' | 'choose' | 'remove') => { setBusy(action); try { - await (action === 'get' ? onGet(item) : onRemove(item)); + if (action === 'get') await onGet(item); + else if (action === 'choose') await onChooseReleases?.(item); + else await onRemove(item); } finally { setBusy(null); } }; - const picks = describeSavedPick(item); - const allPicks = describeSavedPick(item, { full: true }); const saved = Date.parse(item.created_at); const openDetails = onOpenDetails ? () => onOpenDetails(item) : undefined; + const hasPicks = item.kind !== 'book' && item.releases.length > 0; + const reason = waitingFor(item); return ( )} - -

- {picks} -

+ + {hasPicks && } {item.last_error && (

{item.last_error}

)} + {!queue && ( +
+ + {hasPicks && ( + + )} +
+ )} - {autoGetAvailable && ( - - {item.releases.length > 0 ? ( - - ) : ( - Pick a release first - )} + {queue ? ( + <> + + {reason ?? First check pending} + + + {item.auto_checked_at ? isoTimeAgo(item.auto_checked_at) : 'Not yet'} + + + {typeof nextCheckAt === 'number' ? : '—'} + + + ) : ( + + {isoTimeAgo(item.created_at)} )} - - {isoTimeAgo(item.created_at)} -
- + {queue && ( + + )} @@ -247,30 +272,25 @@ const SavedRow = ({ const EMPTY: Record = { queued: { title: 'Nothing queued.', - hint: "Pick a release and save it (or tick Queue for download on a saved pick) and it downloads on its own once there's room.", + hint: "Pick a release and save it, and it downloads on its own once there's room.", }, later: { title: 'Nothing saved for later.', - hint: "Use the bookmark on a book or release, or Save for later when you're out of room, to keep it here.", + hint: "Use the bookmark on a book, or Save for later when you're out of room, to keep it here.", }, }; /** * One of the two saved tabs: Queued (releases picked, downloading on their own once - * there's room) or Saved for later (to get by hand). + * there's room) or Wanted (books to choose releases for). */ export const SavedPanel = ({ stage, items, loaded, - autoGetAvailable, - onGet, - onRemove, onRefresh, - onAutoGet, - onOpenDetails, - onReorder, positionOffset = 0, + ...rest }: SavedPanelProps & { stage: SavedStage }) => { useMountEffect(() => { void onRefresh(); @@ -294,28 +314,36 @@ export const SavedPanel = ({ ); }; +// How long after a check is due to read the list again: what it found, and the next one. +const AFTER_CHECK_MS = 4000; + const SavedTable = ({ stage, items, - autoGetAvailable, - onGet, - onRemove, - onAutoGet, - onOpenDetails, onReorder, positionOffset = 0, -}: Omit & { stage: SavedStage }) => { + onCheckDue, + ...rowHandlers +}: Omit & { + stage: SavedStage; + onCheckDue: () => Promise; +}) => { + const { nextCheckAt } = rowHandlers; + useDependencyEffect(() => { + if (stage !== 'queued' || typeof nextCheckAt !== 'number') return undefined; + const timer = window.setTimeout( + () => void onCheckDue(), + Math.max(0, nextCheckAt - Date.now()) + AFTER_CHECK_MS, + ); + return () => window.clearTimeout(timer); + }, [stage, nextCheckAt]); const [sort, setSort] = useState>({ key: 'saved', direction: 'desc' }); const [moving, setMoving] = useState(false); // Queued items are in line: shown in the order they get room, not sortable. @@ -339,9 +367,9 @@ const SavedTable = ({ return (

- {stage === 'queued' + {inLine ? "Each downloads on its own once there's room, checked in this order: one that doesn't fit yet doesn't hold up the ones after it." - : 'Get one with + Get whenever you want it.'} + : 'Choose releases for a book and it joins the queue, to download once there’s room.'}

@@ -355,26 +383,24 @@ const SavedTable = ({ <> + + + ) : ( <> + )} - {autoGetAvailable && } - {inLine ? ( - - ) : ( - - )} @@ -400,11 +426,7 @@ const SavedTable = ({ } : undefined } - autoGetAvailable={autoGetAvailable} - onGet={onGet} - onRemove={onRemove} - onAutoGet={onAutoGet} - onOpenDetails={onOpenDetails} + {...rowHandlers} /> ))} diff --git a/src/frontend/src/contexts/SavedItemsContext.tsx b/src/frontend/src/contexts/SavedItemsContext.tsx index 2649415b7..64ccf8290 100644 --- a/src/frontend/src/contexts/SavedItemsContext.tsx +++ b/src/frontend/src/contexts/SavedItemsContext.tsx @@ -34,6 +34,8 @@ export interface SavedItemsContextValue { remove: (item: SavedItem, options?: { quiet?: boolean }) => Promise; /** Turn automatic downloading on or off. */ setAutoGet: (item: SavedItem, changes: { auto_get: boolean }) => Promise; + /** When the automatic check next runs (ms since epoch), or null when none is due. */ + nextCheckAt: number | null; /** Put the queued items in this order (ids, first claim on room first). */ reorder: (ids: number[]) => Promise; refresh: () => Promise; @@ -64,12 +66,16 @@ export const useSavedItemsStore = ({ }): SavedItemsContextValue => { const [items, setItems] = useState([]); const [loaded, setLoaded] = useState(false); + const [nextCheckAt, setNextCheckAt] = useState(null); + // Wanted items already sent to the queue, so a failed move isn't retried each refresh. + const promoted = useRef(new Set()); // What the last refresh saw, to tell when an automatic download took an item. const lastSeen = useRef(null); const refresh = useCallback(async () => { try { - const next = await getSavedItems(); + const { items: next, next_check_at } = await getSavedItems(); + setNextCheckAt(next_check_at === null ? null : next_check_at * 1000); const previous = lastSeen.current; if (previous) { const remaining = new Set(next.map((item) => item.id)); @@ -79,12 +85,37 @@ export const useSavedItemsStore = ({ } lastSeen.current = next; setItems(next); + if (autoGetAvailable) { + // Picked releases belong in the queue: Wanted is for books still to pick for. + // One whose last automatic download failed stays, so its error is seen first. + next + .filter( + (item) => + savedStage(item) === 'later' && + item.releases.length > 0 && + !item.last_error && + !promoted.current.has(item.id), + ) + .forEach((item) => { + promoted.current.add(item.id); + void updateSavedItem(item.id, { + auto_get: true, + payloads: savedPayloads(item.book, item.releases), + }) + .then((updated) => + setItems((current) => + current.map((existing) => (existing.id === updated.id ? updated : existing)), + ), + ) + .catch((error: unknown) => console.warn('Could not queue saved item:', error)); + }); + } } catch (error) { console.warn('Could not load saved items:', error); } finally { setLoaded(true); } - }, [onShowToast]); + }, [autoGetAvailable, onShowToast]); const replaceItem = useCallback((saved: SavedItem) => { const apply = (current: SavedItem[]) => [ @@ -196,10 +227,11 @@ export const useSavedItemsStore = ({ savePicks: store, remove, setAutoGet, + nextCheckAt, reorder, refresh, }; - }, [contentType, items, loaded, refresh, remove, reorder, setAutoGet, store]); + }, [contentType, items, loaded, nextCheckAt, refresh, remove, reorder, setAutoGet, store]); }; // Re-read now and then, so items got automatically leave the list (with a toast). diff --git a/src/frontend/src/services/api.ts b/src/frontend/src/services/api.ts index 0b701bb45..9286dc635 100644 --- a/src/frontend/src/services/api.ts +++ b/src/frontend/src/services/api.ts @@ -715,8 +715,16 @@ export const getMamRatio = async (): Promise => export const getBookActivity = async (): Promise => (await fetchJSON<{ downloads: BookDownloadEntry[] }>(`${API_BASE}/activity/books`)).downloads; -export const getSavedItems = async (): Promise => - (await fetchJSON<{ items: SavedItem[] }>(`${API_BASE}/saved`)).items; +/** The saved list, and when the automatic check next runs (epoch seconds, or null). */ +export const getSavedItems = async (): Promise<{ + items: SavedItem[]; + next_check_at: number | null; +}> => { + const body = await fetchJSON<{ items: SavedItem[]; next_check_at?: number | null }>( + `${API_BASE}/saved`, + ); + return { items: body.items, next_check_at: body.next_check_at ?? null }; +}; export const saveForLater = async (payload: { book: Book; diff --git a/src/frontend/src/tests/savedItems.test.ts b/src/frontend/src/tests/savedItems.test.ts index d9fcde990..a007dd7b7 100644 --- a/src/frontend/src/tests/savedItems.test.ts +++ b/src/frontend/src/tests/savedItems.test.ts @@ -7,7 +7,9 @@ import { hasMamPick, savedBookKey, savedPayloads, + savedPickLines, savedStage, + waitingFor, } from '../utils/savedItems'; const release = (extra: Partial): Release => ({ @@ -138,3 +140,44 @@ describe('savedStage', () => { expect(savedStage({ releases: [], auto_get: false })).toBe('later'); }); }); + +describe('savedPickLines', () => { + it('puts each release on its own line', () => { + const narrated = (...names: string[]) => release({ extra: { narrators: names } }); + expect( + savedPickLines({ + kind: 'combined', + releases: [ + { content_type: 'ebook', release: release({ format: 'epub' }) }, + { content_type: 'audiobook', release: narrated('Tony Robinson') }, + { content_type: 'audiobook', release: narrated('Bill Nighy', 'A', 'B') }, + { content_type: 'audiobook', release: narrated('Nigel Planer') }, + ], + }), + ).toEqual([ + 'Ebook, EPUB', + 'Audiobook, Tony Robinson', + 'Audiobook, Bill Nighy +2', + 'Audiobook, Nigel Planer', + ]); + }); + + it('has no lines for a book with nothing picked', () => { + expect(savedPickLines({ kind: 'book', releases: [] })).toEqual([]); + }); +}); + +describe('waitingFor', () => { + it('says what a queued item waits for, from the check status', () => { + expect(waitingFor({ auto_status: 'Waiting for 2 unsatisfied slots' })).toBe( + '2 unsatisfied slots', + ); + expect(waitingFor({ auto_status: 'Waiting for buffer: 1.20 GB short' })).toBe( + 'Buffer: 1.20 GB short', + ); + expect(waitingFor({ auto_status: "Couldn't queue: no client" })).toBe( + "Couldn't queue: no client", + ); + expect(waitingFor({ auto_status: null })).toBeNull(); + }); +}); diff --git a/src/frontend/src/utils/savedItems.ts b/src/frontend/src/utils/savedItems.ts index e9e75b45a..cc9036854 100644 --- a/src/frontend/src/utils/savedItems.ts +++ b/src/frontend/src/utils/savedItems.ts @@ -125,6 +125,37 @@ export const pickLabel = (pick: SavedPick, { full = false }: { full?: boolean } return formatLabel(pick.release) || 'Ebook'; }; +/** + * What a saved item will download, one line per release: "Ebook, EPUB", "Audiobook, + * Narrator Name +2"; `full` names every narrator. Empty for a book with none picked. + */ +export const savedPickLines = ( + item: Pick, + { full = false }: { full?: boolean } = {}, +): string[] => { + if (item.kind === 'book') return []; + return item.releases.map((pick) => { + if (pick.content_type === 'audiobook') { + const names = narratorsOf(pick.release); + if (names.length === 0) return 'Audiobook'; + return `Audiobook, ${full ? names.join(', ') : shortNarrators(names)}`; + } + const format = formatLabel(pick.release); + return format ? `Ebook, ${format}` : 'Ebook'; + }); +}; + +/** + * What a queued item waits for, from the last automatic check's status: "2 unsatisfied + * slots", "Freeleech or ratio 2.00: …". Null before its first check. + */ +export const waitingFor = (item: Pick): string | null => { + const status = item.auto_status?.trim(); + if (!status) return null; + const reason = status.replace(/^waiting for\s+/i, ''); + return reason.charAt(0).toUpperCase() + reason.slice(1); +}; + /** * What a saved item will download: "Ebook EPUB + audiobook, Narrator Name +2", or "Book * only"; `full` names every narrator. diff --git a/tests/core/test_saved_autoget.py b/tests/core/test_saved_autoget.py index 25845a11b..ee9e6236a 100644 --- a/tests/core/test_saved_autoget.py +++ b/tests/core/test_saved_autoget.py @@ -316,3 +316,29 @@ def get_torrent(self, _torrent_id): client.item = None with pytest.raises(MamError): mam_account.torrent_is_freeleech(1, vip=True) + + +def test_next_check_time(service, mam, monkeypatch): + import threading + import time as time_module + + getter = SavedAutoGetter(service, Recorder().hooks()) + assert getter.next_check_at() is None # Not started + + # A running schedule reports when it wakes next; a marked item brings that forward. + monkeypatch.setattr(getter, "_thread", threading.Thread(target=lambda: None)) + monkeypatch.setattr(getter._thread, "is_alive", lambda: True) + getter._next_check_at = time_module.time() + 600 + assert getter.next_check_at() == pytest.approx(time_module.time() + 600, abs=2) + monkeypatch.setattr(saved_autoget, "_SOON_DELAY_SECONDS", 3600) # Keep the timer idle + getter._next_check_at = time_module.time() + 7200 + getter.check_soon() + assert getter.next_check_at() == pytest.approx(time_module.time() + 3600, abs=2) + getter._soon.cancel() + # Once the quick check has run, the schedule's own time is next again. + monkeypatch.setattr(getter, "run_check", lambda: None) + getter._run_soon() + assert getter.next_check_at() == pytest.approx(time_module.time() + 7200, abs=2) + + monkeypatch.setattr(saved_autoget, "enabled", lambda: False) + assert getter.next_check_at() is None diff --git a/tests/core/test_saved_items.py b/tests/core/test_saved_items.py index 2fce020bf..92562930c 100644 --- a/tests/core/test_saved_items.py +++ b/tests/core/test_saved_items.py @@ -334,3 +334,18 @@ def test_route(self, service): assert self._order(service) == [b, a] assert client.post("/api/saved/reorder", json={"ids": "x"}).status_code == 400 assert client.post("/api/saved/reorder", json={"ids": [999]}).status_code == 400 + + +def test_the_list_says_when_the_next_check_is(service): + app = Flask(__name__) + app.secret_key = "test" + register_saved_routes( + app, + service, + lambda f: f, + lambda: "none", + next_check_at=lambda: 1_800_000_000.0, + ) + body = app.test_client().get("/api/saved").get_json() + assert body["items"] == [] + assert body["next_check_at"] == 1_800_000_000.0
Actions