Merge remote-tracking branch 'upstream/dev' into fix/album-completeness-fragmented-rows
This commit is contained in:
ragnarlotus
commit
866c8a6400
15 changed files with 382 additions and 65 deletions
|
|
@ -13,9 +13,31 @@ Pure + import-safe: parsing only, no network.
|
|||
from __future__ import annotations
|
||||
|
||||
import re
|
||||
from typing import Any, Optional, Tuple
|
||||
from typing import Any, List, Optional, Tuple
|
||||
from urllib.parse import urlparse
|
||||
|
||||
|
||||
def linked_track_id(track: Any) -> str:
|
||||
"""The source track id stamped on a search result, read from
|
||||
``_source_metadata['track_id']`` — the field every ID-downloadable source
|
||||
(Tidal, Qobuz) records. Empty string when absent. ``TrackResult`` has no
|
||||
top-level ``id``, so callers must NOT use ``getattr(t, 'id')`` (that always
|
||||
missed and left the pasted-link bubble a silent no-op — #932)."""
|
||||
meta = getattr(track, '_source_metadata', None)
|
||||
if not isinstance(meta, dict):
|
||||
return ''
|
||||
return str(meta.get('track_id') or '')
|
||||
|
||||
|
||||
def bubble_linked_track_first(tracks: List[Any], link_track_id: str) -> List[Any]:
|
||||
"""Float the result whose source id matches a pasted link to the top so the
|
||||
user sees the EXACT track they linked, not a fuzzy text-search lookalike
|
||||
(#813/#932). Stable + a graceful no-op when no result carries the id."""
|
||||
if not link_track_id or not tracks:
|
||||
return tracks
|
||||
target = str(link_track_id)
|
||||
return sorted(tracks, key=lambda t: linked_track_id(t) != target)
|
||||
|
||||
# host substring → download source id. Only ID-downloadable streaming sources.
|
||||
_HOSTS = (
|
||||
('tidal.com', 'tidal'),
|
||||
|
|
|
|||
|
|
@ -100,6 +100,7 @@ _ALBUM_ID_COLUMNS = {
|
|||
'deezer': 'deezer_id',
|
||||
'discogs': 'discogs_id',
|
||||
'hydrabase': 'soul_id',
|
||||
'musicbrainz': 'musicbrainz_release_id',
|
||||
}
|
||||
|
||||
# Human-facing label for each source.
|
||||
|
|
|
|||
|
|
@ -211,7 +211,7 @@ def pick_canonical_release(
|
|||
# core.library_reorganize._ALBUM_ID_COLUMNS — a test pins them in sync). A manual
|
||||
# match on any of these should pin/lock the canonical version (#758); a match on
|
||||
# a source the canonical tools don't read (e.g. lastfm) has no version to pin.
|
||||
CANONICAL_ALBUM_SOURCES = frozenset({'spotify', 'itunes', 'deezer', 'discogs', 'hydrabase'})
|
||||
CANONICAL_ALBUM_SOURCES = frozenset({'spotify', 'itunes', 'deezer', 'discogs', 'hydrabase', 'musicbrainz'})
|
||||
|
||||
|
||||
def should_pin_manual_canonical(entity_type: str, source: str) -> bool:
|
||||
|
|
|
|||
|
|
@ -1074,6 +1074,10 @@ class QobuzClient(DownloadSourcePlugin):
|
|||
title=title,
|
||||
album=album_name,
|
||||
track_number=track.get('track_number'),
|
||||
# Stamp the Qobuz track id so a pasted-link manual search can float the
|
||||
# exact track to the top (#932). Without this the bubble never matched
|
||||
# and the linked track stayed buried among fuzzy lookalikes.
|
||||
_source_metadata={'source': 'qobuz', 'track_id': str(track.get('id') or '')},
|
||||
)
|
||||
|
||||
# Stamp real API quality so the global ranker sees actual
|
||||
|
|
|
|||
|
|
@ -185,7 +185,11 @@ def process_watchlist_scan_automatically(automation_id=None, profile_id=None, de
|
|||
'results': [],
|
||||
'summary': {},
|
||||
'error': None,
|
||||
'cancel_requested': False
|
||||
'cancel_requested': False,
|
||||
# #933: stamp these so this scan lands in the History modal too —
|
||||
# the scanner fills scan_track_events; persist_scan_run reads both.
|
||||
'scan_run_id': datetime.now().strftime('%Y%m%d-%H%M%S'),
|
||||
'scan_track_events': [],
|
||||
}
|
||||
|
||||
scan_results = []
|
||||
|
|
@ -294,6 +298,17 @@ def process_watchlist_scan_automatically(automation_id=None, profile_id=None, de
|
|||
total_added_to_wishlist = deps.watchlist_scan_state.get('summary', {}).get('tracks_added_to_wishlist', 0)
|
||||
logger.warning("Automatic watchlist scan cancelled — skipping post-scan steps")
|
||||
|
||||
# #933: record this run in the History ledger — same helper the manual
|
||||
# scan uses, so scheduled scans show up alongside manual ones.
|
||||
try:
|
||||
from core.watchlist.scan_history import persist_scan_run
|
||||
persist_scan_run(
|
||||
database, deps.watchlist_scan_state,
|
||||
profile_id=profile_id, was_cancelled=was_cancelled,
|
||||
)
|
||||
except Exception as _hist_err:
|
||||
logger.error(f"Failed to persist watchlist scan run: {_hist_err}")
|
||||
|
||||
# Post-scan steps — skip if cancelled
|
||||
if not was_cancelled:
|
||||
# Populate discovery pool from similar artists (per-profile)
|
||||
|
|
|
|||
51
core/watchlist/scan_history.py
Normal file
51
core/watchlist/scan_history.py
Normal file
|
|
@ -0,0 +1,51 @@
|
|||
"""Persist a finished watchlist scan to the History ledger (#831 / #933).
|
||||
|
||||
Both the manual scan (``web_server.start_watchlist_scan``) and the automatic
|
||||
scan (``core.watchlist.auto_scan.process_watchlist_scan_automatically``) finish
|
||||
with the same ``watchlist_scan_state`` shape, but only the manual path used to
|
||||
record a history row — so scheduled/nightly scans never showed up in the
|
||||
History modal (#933). This single helper is the shared seam: both paths call it,
|
||||
so they can't drift apart again.
|
||||
|
||||
Pure except for the one ``database.save_watchlist_scan_run`` call — the field
|
||||
extraction is unit-testable with a fake database.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from datetime import datetime
|
||||
from typing import Any, Dict, Optional
|
||||
|
||||
|
||||
def _iso(value: Any) -> Optional[str]:
|
||||
"""ISO-format a datetime; pass through an already-stringified timestamp."""
|
||||
if value is None:
|
||||
return None
|
||||
return value.isoformat() if hasattr(value, 'isoformat') else str(value)
|
||||
|
||||
|
||||
def persist_scan_run(database: Any, state: Dict[str, Any], *,
|
||||
profile_id: Any, was_cancelled: bool) -> bool:
|
||||
"""Record one watchlist scan run + its track ledger from ``state``.
|
||||
|
||||
Reads the counts/timestamps/ledger off the live ``watchlist_scan_state`` the
|
||||
scanner just finished writing, and writes a single history row. ``run_id``
|
||||
comes from ``state['scan_run_id']`` (both paths stamp it); a timestamp
|
||||
fallback keeps it from ever colliding if that's somehow missing. Returns the
|
||||
DB call's truthiness; callers wrap in their own try/except so a history-write
|
||||
failure never breaks the scan.
|
||||
"""
|
||||
summary = state.get('summary') or {}
|
||||
run_id = state.get('scan_run_id') or datetime.now().strftime('%Y%m%d-%H%M%S')
|
||||
return database.save_watchlist_scan_run(
|
||||
run_id=run_id,
|
||||
profile_id=profile_id if profile_id else 1,
|
||||
status='cancelled' if was_cancelled else 'completed',
|
||||
started_at=_iso(state.get('started_at')),
|
||||
completed_at=_iso(state.get('completed_at')) or datetime.now().isoformat(),
|
||||
total_artists=summary.get('total_artists', state.get('total_artists', 0)),
|
||||
artists_scanned=summary.get('successful_scans', 0),
|
||||
tracks_found=state.get('tracks_found_this_scan', 0),
|
||||
tracks_added=state.get('tracks_added_this_scan', 0),
|
||||
track_events=state.get('scan_track_events') or [],
|
||||
)
|
||||
|
|
@ -328,6 +328,22 @@ _ALBUM_QUALIFIER_RE = re.compile(
|
|||
re.IGNORECASE,
|
||||
)
|
||||
|
||||
# A trailing "- ..." clause is stripped ONLY when EVERY token in it is an edition/format
|
||||
# qualifier (+ connectors / a year-ordinal). So "- Single", "- Acoustic Version", "- 2011
|
||||
# Remaster" collapse to the base name, but a real distinguishing subtitle ("- Nos vies en
|
||||
# Lumière", "- Live in Berlin") is kept — the bug was a blanket "- anything$" strip that
|
||||
# erased subtitles and fused different editions (Sokhi: Expedition 33 OST vs Bonus Edition).
|
||||
_DASH_QUALIFIER_WORD = (
|
||||
r'live|acoustic|electric|instrumental|unplugged|mono|stereo|demos?|reissue|'
|
||||
r'remix(?:es)?|edit(?:ed)?|radio|single|ep|lp|version|mix(?:es)?|sessions?|bootleg|'
|
||||
r'covers?|original|redux|deluxe|expanded|remaster(?:ed)?|anniversary|special|'
|
||||
r'edition|bonus|extended|explicit|clean|soundtrack|ost|score'
|
||||
)
|
||||
_TRAILING_DASH_QUALIFIER_RE = re.compile(
|
||||
r'\s*-\s*(?:(?:' + _DASH_QUALIFIER_WORD + r'|the|a|and|&|\+|\d+(?:st|nd|rd|th)?)\b[\s\-]*)+$',
|
||||
re.IGNORECASE,
|
||||
)
|
||||
|
||||
|
||||
def _normalize_album_for_match(name: str) -> str:
|
||||
"""Return a canonical form of an album name suitable for fuzzy comparison.
|
||||
|
|
@ -347,8 +363,9 @@ def _normalize_album_for_match(name: str) -> str:
|
|||
# they're almost always edition or commentary noise, not part of the
|
||||
# album's identifying name.
|
||||
cleaned = re.sub(r'\s*[\(\[][^\)\]]*[\)\]]\s*', ' ', cleaned)
|
||||
# Trailing dash-clauses ("Album - Remastered", "Album - Live")
|
||||
cleaned = re.sub(r'\s*-\s*[^-]+$', '', cleaned)
|
||||
# Trailing dash-clause, but ONLY when it's entirely edition/format qualifiers — a real
|
||||
# subtitle is preserved (see _TRAILING_DASH_QUALIFIER_RE).
|
||||
cleaned = _TRAILING_DASH_QUALIFIER_RE.sub(' ', cleaned)
|
||||
cleaned = re.sub(r'[^a-z0-9 ]+', ' ', cleaned.lower())
|
||||
cleaned = re.sub(r'\s+', ' ', cleaned).strip()
|
||||
return cleaned
|
||||
|
|
@ -382,7 +399,7 @@ def _extract_volume_marker(normalized_name: str):
|
|||
return last.group(1) or last.group(2)
|
||||
|
||||
|
||||
def _albums_likely_match(spotify_album: str, lib_album: str, threshold: float = 0.6) -> bool:
|
||||
def _albums_likely_match(spotify_album: str, lib_album: str, threshold: float = 0.85) -> bool:
|
||||
"""Return True when two album names plausibly identify the same release.
|
||||
|
||||
Designed to swallow naming drift between metadata sources and the
|
||||
|
|
@ -406,11 +423,11 @@ def _albums_likely_match(spotify_album: str, lib_album: str, threshold: float =
|
|||
return False
|
||||
if norm_a == norm_b:
|
||||
return True
|
||||
# After normalization the shorter name often becomes a prefix /
|
||||
# substring of the longer one ("napoleon dynamite" ⊂ "napoleon
|
||||
# dynamite music from the motion picture" before stripping).
|
||||
if norm_a in norm_b or norm_b in norm_a:
|
||||
return True
|
||||
# No loose substring shortcut: after qualifier-stripping, a short name being a
|
||||
# prefix of a longer one is usually a DIFFERENT edition carrying a real subtitle
|
||||
# ("clair obscur expedition 33" ⊂ "clair obscur expedition 33 nos vies en lumiere"),
|
||||
# not naming drift. Genuine drift collapses to an EXACT match above; everything else
|
||||
# must clear a high overall-similarity bar.
|
||||
return SequenceMatcher(None, norm_a, norm_b).ratio() >= threshold
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -78,3 +78,47 @@ def test_payload_non_dict_or_empty():
|
|||
assert q('tidal', None) is None
|
||||
assert q('tidal', {}) is None
|
||||
assert q('qobuz', 'garbage') is None
|
||||
|
||||
|
||||
# ── bubble the pasted-link track to the top (#932) ──
|
||||
|
||||
from types import SimpleNamespace
|
||||
|
||||
from core.downloads.track_link import linked_track_id, bubble_linked_track_first
|
||||
|
||||
|
||||
def _result(track_id=None):
|
||||
"""Mimic a TrackResult: NO top-level `id`, the source id lives in
|
||||
_source_metadata['track_id'] (or absent entirely)."""
|
||||
meta = {'source': 'qobuz', 'track_id': track_id} if track_id is not None else None
|
||||
return SimpleNamespace(_source_metadata=meta, title='t')
|
||||
|
||||
|
||||
def test_linked_track_id_reads_source_metadata():
|
||||
assert linked_track_id(_result('296427754')) == '296427754'
|
||||
|
||||
|
||||
def test_linked_track_id_empty_when_absent():
|
||||
# the exact #932 trap: there is no top-level `id`, so getattr(t,'id') would miss.
|
||||
r = _result()
|
||||
assert not hasattr(r, 'id')
|
||||
assert linked_track_id(r) == ''
|
||||
|
||||
|
||||
def test_bubble_floats_exact_track_to_top():
|
||||
fuzzy_a, exact, fuzzy_b = _result('111'), _result('296427754'), _result('222')
|
||||
out = bubble_linked_track_first([fuzzy_a, exact, fuzzy_b], '296427754')
|
||||
assert out[0] is exact # exact track surfaced first
|
||||
assert out[1:] == [fuzzy_a, fuzzy_b] # stable order for the rest
|
||||
|
||||
|
||||
def test_bubble_handles_int_vs_str_id():
|
||||
out = bubble_linked_track_first([_result('999'), _result('296427754')], 296427754)
|
||||
assert linked_track_id(out[0]) == '296427754'
|
||||
|
||||
|
||||
def test_bubble_noop_when_nothing_matches_or_empty():
|
||||
a, b = _result('1'), _result('2')
|
||||
assert bubble_linked_track_first([a, b], '296427754') == [a, b] # unchanged
|
||||
assert bubble_linked_track_first([], '296427754') == []
|
||||
assert bubble_linked_track_first([a, b], '') == [a, b]
|
||||
|
|
|
|||
|
|
@ -1,4 +1,3 @@
|
|||
import sys
|
||||
import types
|
||||
|
||||
|
||||
|
|
@ -10,35 +9,10 @@ class _DummyConfigManager:
|
|||
return "plex"
|
||||
|
||||
|
||||
if "spotipy" not in sys.modules:
|
||||
spotipy = types.ModuleType("spotipy")
|
||||
|
||||
class _DummySpotify:
|
||||
def __init__(self, *args, **kwargs):
|
||||
pass
|
||||
|
||||
oauth2 = types.ModuleType("spotipy.oauth2")
|
||||
|
||||
class _DummyOAuth:
|
||||
def __init__(self, *args, **kwargs):
|
||||
pass
|
||||
|
||||
spotipy.Spotify = _DummySpotify
|
||||
oauth2.SpotifyOAuth = _DummyOAuth
|
||||
oauth2.SpotifyClientCredentials = _DummyOAuth
|
||||
spotipy.oauth2 = oauth2
|
||||
sys.modules["spotipy"] = spotipy
|
||||
sys.modules["spotipy.oauth2"] = oauth2
|
||||
|
||||
if "config.settings" not in sys.modules:
|
||||
config_pkg = types.ModuleType("config")
|
||||
settings_mod = types.ModuleType("config.settings")
|
||||
|
||||
settings_mod.config_manager = _DummyConfigManager()
|
||||
config_pkg.settings = settings_mod
|
||||
sys.modules["config"] = config_pkg
|
||||
sys.modules["config.settings"] = settings_mod
|
||||
|
||||
# NOTE: deliberately no sys.modules stubbing of spotipy / config.settings here. Both import
|
||||
# fine in the test env, and faking them globally (with no teardown) leaked into other files —
|
||||
# it left a config.settings with no ConfigManager, intermittently breaking
|
||||
# tests/test_config_save_retry depending on collection order.
|
||||
from core.repair_jobs.album_completeness import AlbumCompletenessJob
|
||||
import core.repair_jobs.album_completeness as album_completeness_module
|
||||
|
||||
|
|
|
|||
|
|
@ -24,7 +24,7 @@ from database.music_database import MusicDatabase
|
|||
# should_pin_manual_canonical — pure
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@pytest.mark.parametrize('source', ['spotify', 'itunes', 'deezer', 'discogs', 'hydrabase'])
|
||||
@pytest.mark.parametrize('source', ['spotify', 'itunes', 'deezer', 'discogs', 'hydrabase', 'musicbrainz'])
|
||||
def test_pins_album_on_recognised_source(source):
|
||||
assert should_pin_manual_canonical('album', source) is True
|
||||
|
||||
|
|
@ -34,7 +34,7 @@ def test_does_not_pin_non_album(entity):
|
|||
assert should_pin_manual_canonical(entity, 'spotify') is False
|
||||
|
||||
|
||||
@pytest.mark.parametrize('source', ['lastfm', 'genius', 'musicbrainz', 'audiodb', 'tidal'])
|
||||
@pytest.mark.parametrize('source', ['lastfm', 'genius', 'audiodb', 'tidal'])
|
||||
def test_does_not_pin_source_canonical_cant_read(source):
|
||||
# No album-version data the canonical tools read → nothing to pin.
|
||||
assert should_pin_manual_canonical('album', source) is False
|
||||
|
|
|
|||
|
|
@ -7,7 +7,11 @@ canonical_source/canonical_album_id, and an explicit user source pick
|
|||
|
||||
from __future__ import annotations
|
||||
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
import core.library_reorganize as lr
|
||||
import core.metadata.registry as metadata_registry
|
||||
from core.musicbrainz_search import MusicBrainzSearchClient
|
||||
|
||||
|
||||
def _patch_fetch(monkeypatch, tracklists):
|
||||
|
|
@ -73,3 +77,82 @@ def test_no_canonical_unchanged(monkeypatch):
|
|||
album_data = {"spotify_album_id": "sp1"}
|
||||
source, _, _ = lr._resolve_source(album_data, "spotify")
|
||||
assert source == "spotify"
|
||||
|
||||
|
||||
def test_musicbrainz_release_id_is_used_by_priority_walk(monkeypatch):
|
||||
client = MusicBrainzSearchClient()
|
||||
client._client = MagicMock()
|
||||
client._client.get_release_group.return_value = None
|
||||
client._client.get_release.return_value = {
|
||||
"id": "mb-release-1",
|
||||
"title": "Test Album",
|
||||
"date": "2024-01-01",
|
||||
"artist-credit": [{"name": "Test Artist"}],
|
||||
"release-group": {
|
||||
"id": "mb-group-1",
|
||||
"primary-type": "Album",
|
||||
"secondary-types": [],
|
||||
},
|
||||
"media": [
|
||||
{
|
||||
"position": 1,
|
||||
"tracks": [
|
||||
{
|
||||
"id": "track-1",
|
||||
"number": "1",
|
||||
"position": 1,
|
||||
"length": 180000,
|
||||
"recording": {
|
||||
"id": "recording-1",
|
||||
"title": "Test Track",
|
||||
"artist-credit": [{"name": "Test Artist"}],
|
||||
"length": 180000,
|
||||
},
|
||||
},
|
||||
],
|
||||
},
|
||||
],
|
||||
}
|
||||
|
||||
monkeypatch.setattr(
|
||||
metadata_registry,
|
||||
"get_musicbrainz_client",
|
||||
lambda *args, **kwargs: client,
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
lr,
|
||||
"get_source_priority",
|
||||
lambda primary: ["musicbrainz", "spotify"],
|
||||
)
|
||||
|
||||
album_data = {
|
||||
"musicbrainz_release_id": "mb-release-1",
|
||||
}
|
||||
|
||||
source, api_album, items = lr._resolve_source(
|
||||
album_data,
|
||||
"musicbrainz",
|
||||
)
|
||||
|
||||
assert source == "musicbrainz"
|
||||
assert api_album["id"] == "mb-release-1"
|
||||
assert api_album["name"] == "Test Album"
|
||||
assert items == api_album["tracks"]
|
||||
assert items == [
|
||||
{
|
||||
"id": "recording-1",
|
||||
"name": "Test Track",
|
||||
"artists": [{"name": "Test Artist"}],
|
||||
"duration_ms": 180000,
|
||||
"track_number": 1,
|
||||
"disc_number": 1,
|
||||
},
|
||||
]
|
||||
client._client.get_release_group.assert_any_call(
|
||||
"mb-release-1",
|
||||
includes=["releases", "artist-credits"],
|
||||
)
|
||||
client._client.get_release.assert_any_call(
|
||||
"mb-release-1",
|
||||
includes=["recordings", "artist-credits", "release-groups"],
|
||||
)
|
||||
|
|
|
|||
|
|
@ -156,6 +156,13 @@ def test_compilation_score_explanation() -> None:
|
|||
("Inception (Music From The Motion Picture)", "Inception Soundtrack"),
|
||||
# Substring containment
|
||||
("Random Access Memories", "Random Access Memories (Bonus Edition)"),
|
||||
# Dash-suffixed qualifiers must still collapse — these are the SAME album, so
|
||||
# treating them as different would re-wishlist/redownload forever (the failure the
|
||||
# original blanket strip guarded against; the narrowed strip must keep covering it).
|
||||
("Album Name", "Album Name - Single"),
|
||||
("Album Name", "Album Name - Acoustic Version"),
|
||||
("Hotel California", "Hotel California - 2013 Remaster"),
|
||||
("Album", "Album - The Remixes"),
|
||||
],
|
||||
)
|
||||
def test_likely_match_positive(spotify_name, lib_name) -> None:
|
||||
|
|
@ -176,12 +183,29 @@ def test_likely_match_positive(spotify_name, lib_name) -> None:
|
|||
("Abbey Road", "Sgt. Pepper's Lonely Hearts Club Band"),
|
||||
# Same word in title but different album
|
||||
("Greatest Hits Volume 1", "Greatest Hits Volume 2"),
|
||||
# Sokhi: distinct editions of the same franchise — the OST vs a bonus edition
|
||||
# with a real subtitle. USED to collapse to the same normalized name (the blanket
|
||||
# trailing-dash strip removed "- Nos vies en Lumière"), so the watchlist marked
|
||||
# unowned OST tracks as owned via the bonus edition. Must be DIFFERENT albums.
|
||||
("Clair Obscur: Expedition 33: Original Soundtrack",
|
||||
"Clair Obscur: Expedition 33 - Nos vies en Lumière (Bonus Edition)"),
|
||||
# A real subtitle after a dash must not be stripped down to the base name.
|
||||
("The Album", "The Album - A Whole Different Subtitle"),
|
||||
],
|
||||
)
|
||||
def test_likely_match_negative(spotify_name, lib_name) -> None:
|
||||
assert not _albums_likely_match(spotify_name, lib_name)
|
||||
|
||||
|
||||
def test_real_subtitle_after_dash_is_preserved() -> None:
|
||||
# the regression's root cause: a meaningful subtitle must survive normalization,
|
||||
# while a recognized qualifier after a dash ("- Live", "- 2011") still collapses.
|
||||
assert _normalize_album_for_match("Clair Obscur: Expedition 33 - Nos vies en Lumière") \
|
||||
!= _normalize_album_for_match("Clair Obscur: Expedition 33")
|
||||
assert _normalize_album_for_match("Some Album - Live") == _normalize_album_for_match("Some Album")
|
||||
assert _normalize_album_for_match("Some Album - 2011") == _normalize_album_for_match("Some Album")
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# _albums_likely_match — defensive cases
|
||||
# ---------------------------------------------------------------------------
|
||||
|
|
|
|||
|
|
@ -47,6 +47,62 @@ def test_resave_is_idempotent_on_run_id(db):
|
|||
assert len(runs) == 1 and runs[0]['tracks_added'] == 11
|
||||
|
||||
|
||||
# ── persist_scan_run: the shared seam both scan paths use (#933) ──
|
||||
|
||||
from datetime import datetime
|
||||
|
||||
from core.watchlist.scan_history import persist_scan_run
|
||||
|
||||
|
||||
def _state(**over):
|
||||
"""A watchlist_scan_state as the scanner leaves it when a scan finishes."""
|
||||
s = {
|
||||
'scan_run_id': 'auto-run-1',
|
||||
'started_at': datetime(2026, 6, 26, 2, 0, 0),
|
||||
'completed_at': datetime(2026, 6, 26, 2, 4, 0),
|
||||
'total_artists': 40,
|
||||
'tracks_found_this_scan': 7,
|
||||
'tracks_added_this_scan': 3,
|
||||
'scan_track_events': _events(n_added=3, n_skipped=1),
|
||||
'summary': {'total_artists': 40, 'successful_scans': 40,
|
||||
'new_tracks_found': 7, 'tracks_added_to_wishlist': 3},
|
||||
}
|
||||
s.update(over)
|
||||
return s
|
||||
|
||||
|
||||
def test_persist_scan_run_records_a_history_row(db):
|
||||
# the #933 fix: an automatic (all-profiles → profile_id=None) scan must land in History.
|
||||
assert persist_scan_run(db, _state(), profile_id=None, was_cancelled=False) is True
|
||||
runs = db.get_watchlist_scan_runs()
|
||||
assert len(runs) == 1
|
||||
r = runs[0]
|
||||
assert (r['run_id'], r['status'], r['tracks_found'], r['tracks_added']) == \
|
||||
('auto-run-1', 'completed', 7, 3)
|
||||
assert r['profile_id'] == 1 # None coerced to a concrete profile, never NULL
|
||||
# the per-run ledger came through too
|
||||
assert [e['status'] for e in db.get_watchlist_scan_run_events('auto-run-1')] == \
|
||||
['added', 'added', 'added', 'skipped']
|
||||
|
||||
|
||||
def test_persist_scan_run_cancelled_status(db):
|
||||
persist_scan_run(db, _state(scan_run_id='c1'), profile_id=2, was_cancelled=True)
|
||||
assert db.get_watchlist_scan_runs()[0]['status'] == 'cancelled'
|
||||
|
||||
|
||||
def test_persist_scan_run_accepts_datetime_or_iso_string(db):
|
||||
# state timestamps may be datetime (auto path) or already-iso strings — both must persist.
|
||||
persist_scan_run(db, _state(scan_run_id='dt', completed_at='2026-06-26T02:09:00'),
|
||||
profile_id=1, was_cancelled=False)
|
||||
assert db.get_watchlist_scan_runs()[0]['run_id'] == 'dt'
|
||||
|
||||
|
||||
def test_persist_scan_run_tolerates_sparse_state(db):
|
||||
# a bare/early-finished state must not raise — history-write must never break a scan.
|
||||
assert persist_scan_run(db, {'scan_run_id': 'sparse'}, profile_id=None, was_cancelled=False)
|
||||
assert db.get_watchlist_scan_runs()[0]['tracks_added'] == 0
|
||||
|
||||
|
||||
def test_prune_keeps_most_recent(db):
|
||||
for i in range(1, 8):
|
||||
db.save_watchlist_scan_run(
|
||||
|
|
|
|||
|
|
@ -81,6 +81,11 @@ class _FakeDB:
|
|||
self._watchlist_artists = watchlist_artists or []
|
||||
self._lb_profiles = lb_profiles or []
|
||||
self.database_path = '/tmp/test.db'
|
||||
self.scan_runs_saved = []
|
||||
|
||||
def save_watchlist_scan_run(self, **kwargs):
|
||||
self.scan_runs_saved.append(kwargs)
|
||||
return True
|
||||
|
||||
def get_all_profiles(self):
|
||||
return self._profiles
|
||||
|
|
@ -237,6 +242,39 @@ def test_successful_scan_runs_post_steps(patched_modules):
|
|||
assert deps._state_ref[0]['summary']['new_tracks_found'] == 2
|
||||
|
||||
|
||||
def test_successful_scan_records_history_run(patched_modules):
|
||||
"""#933: the automatic scan must persist a History row (it used to skip this,
|
||||
so only manual scans appeared in History)."""
|
||||
scanner, db = patched_modules
|
||||
deps = _build_deps()
|
||||
|
||||
autosc.process_watchlist_scan_automatically(automation_id='a1', deps=deps)
|
||||
|
||||
assert len(db.scan_runs_saved) == 1 # exactly one row, no double-record
|
||||
saved = db.scan_runs_saved[0]
|
||||
assert saved['status'] == 'completed'
|
||||
assert saved['artists_scanned'] == 1 # one successful _ScanResult
|
||||
assert saved['run_id'] # a run id was stamped
|
||||
|
||||
|
||||
def test_cancelled_scan_still_records_history_run(patched_modules, monkeypatch):
|
||||
"""A cancelled scan is recorded too, with status='cancelled'."""
|
||||
scanner, db = patched_modules
|
||||
deps = _build_deps()
|
||||
# Make the scan observe a cancel request mid-run.
|
||||
orig = scanner.scan_watchlist_artists
|
||||
def _cancel_during(artists, *, scan_state, progress_callback, cancel_check):
|
||||
scan_state['cancel_requested'] = True
|
||||
return orig(artists, scan_state=scan_state, progress_callback=progress_callback,
|
||||
cancel_check=cancel_check)
|
||||
monkeypatch.setattr(scanner, 'scan_watchlist_artists', _cancel_during)
|
||||
|
||||
autosc.process_watchlist_scan_automatically(automation_id='a1', deps=deps)
|
||||
|
||||
assert len(db.scan_runs_saved) == 1
|
||||
assert db.scan_runs_saved[0]['status'] == 'cancelled'
|
||||
|
||||
|
||||
def test_completion_emits_automation_event(patched_modules):
|
||||
"""Successful scan emits 'watchlist_scan_completed' on automation_engine."""
|
||||
scanner, _ = patched_modules
|
||||
|
|
|
|||
|
|
@ -7533,13 +7533,13 @@ def manual_search_for_task(task_id):
|
|||
"error": error,
|
||||
}) + "\n"
|
||||
continue
|
||||
# Pasted-link exact match: bubble the track whose id matches
|
||||
# the link to the top so the user sees the exact version
|
||||
# first (graceful no-op if ids don't line up).
|
||||
# Pasted-link exact match: bubble the track whose source id
|
||||
# matches the link to the top so the user sees the exact version
|
||||
# first. Reads _source_metadata['track_id'] (TrackResult has no
|
||||
# top-level id) — the old getattr(t,'id') always missed (#932).
|
||||
if src_name == link_source and link_track_id and tracks:
|
||||
tracks = sorted(
|
||||
tracks,
|
||||
key=lambda t: str(getattr(t, 'id', '')) != str(link_track_id))
|
||||
from core.downloads.track_link import bubble_linked_track_first
|
||||
tracks = bubble_linked_track_first(tracks, link_track_id)
|
||||
serialized = []
|
||||
for t in tracks:
|
||||
s = _serialize_candidate(t, source_override=src_name)
|
||||
|
|
@ -28223,22 +28223,10 @@ def start_watchlist_scan():
|
|||
# #831 round 2: persist this run + its track ledger so the
|
||||
# Watchlist History modal can show what every past scan did.
|
||||
try:
|
||||
_state = watchlist_scan_state
|
||||
get_database().save_watchlist_scan_run(
|
||||
run_id=_state.get('scan_run_id') or datetime.now().strftime('%Y%m%d-%H%M%S'),
|
||||
profile_id=scan_profile_id,
|
||||
status='cancelled' if was_cancelled else 'completed',
|
||||
started_at=(_state.get('started_at').isoformat()
|
||||
if _state.get('started_at') else None),
|
||||
completed_at=(_state.get('completed_at') or datetime.now()).isoformat()
|
||||
if not isinstance(_state.get('completed_at'), str)
|
||||
else _state.get('completed_at'),
|
||||
total_artists=(_state.get('summary') or {}).get('total_artists',
|
||||
_state.get('total_artists', 0)),
|
||||
artists_scanned=(_state.get('summary') or {}).get('successful_scans', 0),
|
||||
tracks_found=_state.get('tracks_found_this_scan', 0),
|
||||
tracks_added=_state.get('tracks_added_this_scan', 0),
|
||||
track_events=_state.get('scan_track_events') or [],
|
||||
from core.watchlist.scan_history import persist_scan_run
|
||||
persist_scan_run(
|
||||
get_database(), watchlist_scan_state,
|
||||
profile_id=scan_profile_id, was_cancelled=was_cancelled,
|
||||
)
|
||||
except Exception as _hist_err:
|
||||
logger.error(f"Failed to persist watchlist scan run: {_hist_err}")
|
||||
|
|
|
|||
Loading…
Reference in a new issue