diff --git a/core/downloads/master.py b/core/downloads/master.py index f63488d8..3769dca3 100644 --- a/core/downloads/master.py +++ b/core/downloads/master.py @@ -373,6 +373,7 @@ def run_full_missing_tracks_process(batch_id, playlist_id, tracks_json, deps: Ma playlist_name=batch_playlist_name, batch_playlist_folder_mode=batch_playlist_folder_mode, profile_id=batch_profile_id, + source=batch_source, ) if effective_playlist_folder_mode and not batch_playlist_folder_mode: with tasks_lock: @@ -1043,10 +1044,12 @@ def run_full_missing_tracks_process(batch_id, playlist_id, tracks_json, deps: Ma wl_source = {} wl_pl_ref = wl_source.get('playlist_id') wl_pl_name = wl_source.get('playlist_name') + wl_pl_source = wl_source.get('source') or 'spotify' if wl_pl_ref and hasattr(db, 'resolve_mirrored_playlist'): wl_mirrored = db.resolve_mirrored_playlist( wl_pl_ref, profile_id=batch_profile_id, + default_source=wl_pl_source, ) if wl_mirrored and wl_mirrored.get('organize_by_playlist'): task_pl_folder_mode = True diff --git a/core/downloads/playlist_folder.py b/core/downloads/playlist_folder.py index 521cc41e..f9ef994e 100644 --- a/core/downloads/playlist_folder.py +++ b/core/downloads/playlist_folder.py @@ -101,6 +101,7 @@ def resolve_playlist_folder_mode_for_batch( playlist_name: str, batch_playlist_folder_mode: bool, profile_id: int = 1, + source: str = 'spotify', ) -> tuple[bool, str]: """Merge batch flag with persisted mirrored-playlist preference.""" if batch_playlist_folder_mode: @@ -109,7 +110,11 @@ def resolve_playlist_folder_mode_for_batch( if not hasattr(db, 'resolve_mirrored_playlist'): return False, playlist_name - mirrored = db.resolve_mirrored_playlist(playlist_id, profile_id=profile_id) + # Pass the batch's source so numeric upstream ids (e.g. Deezer) resolve by + # source instead of colliding with the mirrored-playlists primary key. + mirrored = db.resolve_mirrored_playlist( + playlist_id, profile_id=profile_id, default_source=source or 'spotify' + ) if mirrored and mirrored.get('organize_by_playlist'): return True, mirrored.get('name') or playlist_name return False, playlist_name diff --git a/database/music_database.py b/database/music_database.py index b421adfa..e978df0c 100644 --- a/database/music_database.py +++ b/database/music_database.py @@ -12502,13 +12502,26 @@ class MusicDatabase: *, default_source: str = 'spotify', ) -> Optional[Dict]: - """Resolve a mirrored playlist from numeric id or upstream source id.""" + """Resolve a mirrored playlist from an upstream source id or numeric PK. + + Resolves by ``(source, source_playlist_id)`` FIRST, then falls back to + treating an all-digit ref as the mirrored-playlists primary key. The + order matters: some sources (e.g. Deezer) use all-numeric upstream ids, + and the old PK-first logic mistook those for the PK — so the Deezer + organize-by-playlist toggle resolved the wrong row (or nothing). + """ if playlist_ref is None or playlist_ref == '': return None ref = str(playlist_ref).strip() + if not ref: + return None + if default_source: + row = self.get_mirrored_playlist_by_source(default_source, ref, profile_id) + if row: + return row if ref.isdigit(): return self.get_mirrored_playlist(int(ref)) - return self.get_mirrored_playlist_by_source(default_source, ref, profile_id) + return None def set_mirrored_playlist_organize_by_playlist( self, diff --git a/tests/test_resolve_mirrored_playlist.py b/tests/test_resolve_mirrored_playlist.py new file mode 100644 index 00000000..75d3feea --- /dev/null +++ b/tests/test_resolve_mirrored_playlist.py @@ -0,0 +1,57 @@ +"""Seam tests for resolve_mirrored_playlist source-vs-PK resolution. + +Regression cover for the PR #780 follow-up: numeric *upstream* ids (Deezer +playlist ids are all-digit) must resolve by (source, source_playlist_id), NOT +be mistaken for the mirrored-playlists primary key. The old PK-first logic made +the Deezer organize-by-playlist toggle resolve the wrong row (or nothing). +""" + +from __future__ import annotations + +from database.music_database import MusicDatabase + + +def test_numeric_source_id_resolves_by_source_not_pk(tmp_path): + """A Deezer-style all-numeric upstream id resolves the right row.""" + db = MusicDatabase(str(tmp_path / "m.db")) + pk = db.mirror_playlist(source='deezer', source_playlist_id='908622995', + name='My Deezer Mix', tracks=[], profile_id=1) + assert pk + row = db.resolve_mirrored_playlist('908622995', profile_id=1, default_source='deezer') + assert row is not None + assert row['id'] == pk + assert row['source'] == 'deezer' + # And it must NOT have been a PK lookup: 908622995 is not a valid PK here. + assert db.get_mirrored_playlist(908622995) is None + + +def test_spotify_alphanumeric_resolves_by_source(tmp_path): + db = MusicDatabase(str(tmp_path / "m.db")) + pk = db.mirror_playlist(source='spotify', source_playlist_id='37i9dQZF1DXcBWIGoYBM5M', + name='Top Hits', tracks=[], profile_id=1) + row = db.resolve_mirrored_playlist('37i9dQZF1DXcBWIGoYBM5M', profile_id=1, default_source='spotify') + assert row is not None and row['id'] == pk + + +def test_pk_fallback_when_no_source_match(tmp_path): + """A numeric ref that isn't a known source id still resolves via PK fallback.""" + db = MusicDatabase(str(tmp_path / "m.db")) + pk = db.mirror_playlist(source='spotify', source_playlist_id='abc123XYZ', + name='Sp', tracks=[], profile_id=1) + row = db.resolve_mirrored_playlist(str(pk), profile_id=1, default_source='spotify') + assert row is not None and row['id'] == pk + + +def test_resolution_is_profile_scoped(tmp_path): + db = MusicDatabase(str(tmp_path / "m.db")) + db.mirror_playlist(source='deezer', source_playlist_id='555', + name='D', tracks=[], profile_id=1) + # Another profile must not resolve profile 1's Deezer playlist by source. + assert db.resolve_mirrored_playlist('555', profile_id=2, default_source='deezer') is None + + +def test_empty_refs_return_none(tmp_path): + db = MusicDatabase(str(tmp_path / "m.db")) + assert db.resolve_mirrored_playlist(None) is None + assert db.resolve_mirrored_playlist('') is None + assert db.resolve_mirrored_playlist(' ') is None