diff --git a/CHANGELOG.md b/CHANGELOG.md index 0415f32..ddc483c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,22 @@ alone. ### Fixed +- **`--search` did not start downloads for the first album of each new + artist.** Adding an album was assumed to search it, via Lidarr's + `addOptions.searchForNewAlbum`. That flag is read by + `SearchForRecentlyAdded`, which Lidarr's `ArtistScannedHandler` only + reaches for an artist that has no pending add options — never the + artist the add just created — and the handler clears those options on + its way out, so the flag is stored and dropped + ([Lidarr#5012](https://github.com/Lidarr/Lidarr/issues/5012)). The + second and later albums by an artist were unaffected: Lidarr + pre-creates the discography, so those come back "already added" and + take the flip-to-monitored path, which chartarr searched explicitly. + Every album this run adds or turns on is now named in one `AlbumSearch` + command, which searches unconditionally. Without `--search`, the + summary says how many albums are monitored but idle, instead of looking + like a finished job that downloaded nothing. + - **A skip in the review screen is now honoured on a row that also matched.** The push took the automatic match whenever there was one and only consulted your decision otherwise, so a row that was skipped and diff --git a/README.md b/README.md index 52f2cca..b9038ac 100644 --- a/README.md +++ b/README.md @@ -46,6 +46,10 @@ This runs three stages: Albums already in Lidarr are skipped; albums Lidarr knows but does not monitor are set to monitored. This stage is safe to re-run. + Monitoring an album does not download it — Lidarr picks monitored + albums up on its own schedule. Pass `--search` to have it go looking + straight away; without it, chartarr says how many albums are waiting. + When output is piped or no terminal is available, the progress screens are replaced by plain line output. @@ -63,7 +67,7 @@ push) on sample data without saving or sending anything. --dry-run show what would be pushed without changing anything --yes skip the review stage - --search trigger a Lidarr search for added albums + --search have Lidarr look for the albums and download them --match-only run only the match stage --review-only run only the review stage --push-only run only the push stage diff --git a/chartarr/cli.py b/chartarr/cli.py index e58863e..14909a1 100644 --- a/chartarr/cli.py +++ b/chartarr/cli.py @@ -434,11 +434,10 @@ def events(): try: outcome, album_id = api.add_album( it["rgid"], qp["id"], mp["id"], rf["path"], - search=args.search, also_monitor=siblings) - # only the flipped-to-monitored rows need asking for below: - # a fresh add already carries searchForNewAlbum, and listing - # it here too would have lidarr search the same album twice - if album_id and outcome == "monitored": + also_monitor=siblings) + # everything this run put in or turned on gets searched + # below; adding an album never starts a search by itself + if album_id and outcome in ("added", "monitored"): touched.append(album_id) yield name, outcome, None except lidarr.LidarrError as e: @@ -462,13 +461,18 @@ def events(): line += f" · failed {accent(counts['failed'])}" print(line) if args.search and touched: - # lidarr's per-album searchForNewAlbum flag doesn't fire for albums - # that were only flipped to monitored, so ask for the search here + # the only thing that actually starts a download: one AlbumSearch + # for everything this run added or turned on try: api.search_albums(touched) print(dim(f"asked lidarr to search for {_n(len(touched), 'album')}")) except lidarr.LidarrError as e: print(dim(f"search request failed: {e}")) + elif touched and not args.dry_run: + # without --search the albums sit there monitored and idle, which + # looks like a finished job that downloaded nothing + print(dim(f"{_n(len(touched), 'album')} monitored but not searched — " + f"rerun with --search, or hit Search in lidarr")) for f_ in failures[:8]: print(dim(f" {f_}")) if len(failures) > 8: diff --git a/chartarr/lidarr.py b/chartarr/lidarr.py index 92ed0af..b2f1a4b 100644 --- a/chartarr/lidarr.py +++ b/chartarr/lidarr.py @@ -170,13 +170,19 @@ def lookup(self, rgid: str) -> dict | None: def add_album(self, rgid: str, quality_profile_id: int, metadata_profile_id: int, root_folder: str, - search: bool = False, also_monitor: list[str] | None = None) -> tuple[str, int | None]: """add one release group. returns (outcome, album_id) where outcome is added, monitored or skipped. also_monitor lists other release groups by the same artist that this run will push, so they survive lidarr's post-add scan. + + adding does not search. lidarr's addOptions.searchForNewAlbum is + read by SearchForRecentlyAdded, which ArtistScannedHandler only + reaches for an artist with no AddOptions — never the artist this + add just created — and the handler clears AddOptions on its way + out, so the flag is stored and then dropped (Lidarr#5012). the + caller searches explicitly with search_albums instead. """ existing = self.find_album(rgid) if existing is not None: @@ -208,7 +214,9 @@ def add_album(self, rgid: str, quality_profile_id: int, }) album["artist"] = artist album["monitored"] = True - album["addOptions"] = {"searchForNewAlbum": bool(search)} + # not searchForNewAlbum: see the docstring. leaving it false also + # keeps AddAlbumService from tangling it with searchForMissingAlbums + album["addOptions"] = {"searchForNewAlbum": False} try: created = self._call("album", method="POST", json=album) except LidarrError as e: diff --git a/tests/test_cli.py b/tests/test_cli.py index 908d79c..4d6b7d5 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -375,7 +375,7 @@ def metadata_profiles(self): def root_folders(self): return [{"id": 3, "path": "/music"}] - def add_album(self, rgid, qp, mp, rf, search=False, also_monitor=None): + def add_album(self, rgid, qp, mp, rf, also_monitor=None): return self.outcomes[rgid] def search_albums(self, album_ids): @@ -396,13 +396,15 @@ def _push(monkeypatch, tmp_path, outcomes, **flags): return api -def test_search_does_not_ask_twice_for_a_freshly_added_album( - monkeypatch, tmp_path, capsys): - # add_album already sets addOptions.searchForNewAlbum on the POST, so - # naming the same album in the AlbumSearch command searched it twice +def test_search_asks_for_a_freshly_added_album(monkeypatch, tmp_path, capsys): + # adding does not search: lidarr stores addOptions.searchForNewAlbum + # and drops it unread for an artist it created in the same request + # (Lidarr#5012), so the explicit command is the only thing that starts + # a download. leaving added albums out meant the first album of every + # new artist silently never downloaded. api = _push(monkeypatch, tmp_path, {"rg-new": ("added", 42)}, argv=["--search"]) - assert api.searched == [] + assert api.searched == [42] def test_search_still_asks_for_rows_only_flipped_to_monitored( @@ -421,13 +423,43 @@ def test_search_skips_albums_that_were_already_monitored( assert api.searched == [] -def test_a_mixed_push_searches_only_the_flipped_rows( +def test_a_mixed_push_searches_everything_it_added_or_turned_on( monkeypatch, tmp_path, capsys): + # the realistic shape of a chart push: the first album of an artist + # lands as "added", their others come back 400 "already added" from + # the discography lidarr pre-created and get flipped to "monitored". + # both need searching; only rows already monitored are left alone. api = _push(monkeypatch, tmp_path, {"rg-new": ("added", 1), "rg-old": ("monitored", 2), "rg-there": ("skipped", 3)}, argv=["--search"]) - assert api.searched == [2] + assert api.searched == [1, 2] + + +def test_every_pushed_album_is_searched_exactly_once( + monkeypatch, tmp_path, capsys): + api = _push(monkeypatch, tmp_path, + {f"rg-{i}": ("added" if i % 2 else "monitored", i) + for i in range(1, 7)}, + argv=["--search"]) + assert sorted(api.searched) == [1, 2, 3, 4, 5, 6] + assert len(api.searched) == len(set(api.searched)) + + +def test_without_search_the_summary_says_nothing_was_searched( + monkeypatch, tmp_path, capsys): + # monitored-but-idle looks like a finished job that downloaded nothing + _push(monkeypatch, tmp_path, + {"rg-new": ("added", 1), "rg-old": ("monitored", 2)}) + out = capsys.readouterr().out + assert "2 albums monitored but not searched" in out + assert "--search" in out + + +def test_a_push_with_nothing_to_search_says_nothing( + monkeypatch, tmp_path, capsys): + _push(monkeypatch, tmp_path, {"rg-there": ("skipped", 9)}) + assert "not searched" not in capsys.readouterr().out def test_without_the_search_flag_nothing_is_searched( diff --git a/tests/test_lidarr.py b/tests/test_lidarr.py index 6bbf775..66fd9f2 100644 --- a/tests/test_lidarr.py +++ b/tests/test_lidarr.py @@ -105,13 +105,18 @@ def test_also_monitor_names_every_sibling_once(): @responses.activate -def test_search_flag_stays_on_the_album_not_the_artist(): - # --search must ask for this album only, never the whole discography +def test_adding_never_asks_lidarr_to_search(): + # searchForNewAlbum is read by SearchForRecentlyAdded, which + # ArtistScannedHandler only reaches when the artist has no AddOptions + # — never the artist this add just created — and the handler clears + # AddOptions on the way out, so the flag is stored and dropped + # (Lidarr#5012). searching is the caller's job, via AlbumSearch. stage_lookup() responses.add(responses.POST, API + "/album", json={"id": 42}, status=201) - add(search=True) + add() body = posted_album() - assert body["addOptions"] == {"searchForNewAlbum": True} + assert body["addOptions"] == {"searchForNewAlbum": False} + # and never the artist-wide search, which would grab the discography assert body["artist"]["addOptions"]["searchForMissingAlbums"] is False