Fix: manual Find & Add recreated the Jellyfin/Emby playlist (#837)
Automations + auto-sync respect 'append' mode and preserve a server playlist's
description + cover image, but manually matching a missing track ("Find & add")
recreated the whole playlist and wiped them.
Root cause: the add-track endpoint's Jellyfin branch called
`update_playlist(<entire track list>)`, which deletes + recreates the playlist on
Jellyfin/Emby. Switched it to the purpose-built `append_to_playlist([the one
found track])` — the same in-place, dedupe-safe op the 'append' sync mode already
uses — so the playlist (and its description/image) is preserved and only the
missing track is added. append_to_playlist reads `.id` off the track, so the
endpoint now sets it (it previously only set ratingKey).
Plex (in-place addItems) and Navidrome (in-place Subsonic updatePlaylist) were
already non-destructive; Emby routes through the jellyfin branch, so this covers
it too.
Tests: the add-track endpoint appends in place and never calls update_playlist;
a link-to-existing-track touches nothing. 18 tests pass (incl. the existing
append-mode suite).
This commit is contained in:
parent
b36392c62b
commit
1517794e23
2 changed files with 107 additions and 3 deletions
94
tests/test_issue_837_find_add_preserves_playlist.py
Normal file
94
tests/test_issue_837_find_add_preserves_playlist.py
Normal file
|
|
@ -0,0 +1,94 @@
|
|||
"""#837 — manual Find & Add must NOT recreate the Jellyfin/Emby playlist.
|
||||
|
||||
Reporter (carlosjfcasero, Emby/Jellyfin): automations + auto-sync respect the
|
||||
'append' sync mode and preserve the playlist's description/image, but manually
|
||||
matching a missing track ("Find & add") recreated the whole playlist and wiped
|
||||
them. Root cause: the add-track endpoint's Jellyfin branch called
|
||||
`update_playlist(full list)` (delete + recreate) instead of the in-place
|
||||
`append_to_playlist`. These pin that the endpoint now appends in place.
|
||||
|
||||
(Emby routes through the 'jellyfin' branch — no separate emby branch exists.)
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
import tempfile
|
||||
from types import SimpleNamespace
|
||||
|
||||
import pytest
|
||||
|
||||
_TMP = tempfile.mkdtemp(prefix='soulsync-testdb-837-')
|
||||
os.environ['DATABASE_PATH'] = os.path.join(_TMP, 'i837.db')
|
||||
os.environ['SOULSYNC_TEST_DB_READY'] = '1'
|
||||
|
||||
web_server = pytest.importorskip('web_server')
|
||||
|
||||
_GUID = 'aaaaaaaa-bbbb-cccc-dddd-000000000001'
|
||||
|
||||
|
||||
class _FakeJellyfin:
|
||||
def __init__(self, existing=None):
|
||||
self.existing = existing or []
|
||||
self.append_calls = []
|
||||
self.update_calls = []
|
||||
|
||||
def get_playlist_tracks(self, pid):
|
||||
return [SimpleNamespace(ratingKey=str(r)) for r in self.existing]
|
||||
|
||||
def append_to_playlist(self, name, tracks):
|
||||
self.append_calls.append((name, [getattr(t, 'id', None) for t in tracks]))
|
||||
return True
|
||||
|
||||
def update_playlist(self, name, tracks): # the destructive recreate path
|
||||
self.update_calls.append((name, tracks))
|
||||
return True
|
||||
|
||||
|
||||
class _FakeEngine:
|
||||
def __init__(self, jf):
|
||||
self._jf = jf
|
||||
|
||||
def client(self, name):
|
||||
return self._jf if name == 'jellyfin' else None
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def client():
|
||||
return web_server.app.test_client()
|
||||
|
||||
|
||||
def _wire(monkeypatch, jf):
|
||||
monkeypatch.setattr(web_server, 'media_server_engine', _FakeEngine(jf))
|
||||
monkeypatch.setattr(web_server.config_manager, 'get_active_media_server', lambda: 'jellyfin')
|
||||
# the durable source->server match write touches the DB; not under test here
|
||||
monkeypatch.setattr(web_server, '_persist_find_and_add_match', lambda *a, **k: None)
|
||||
|
||||
|
||||
def test_find_and_add_appends_in_place_not_recreate(client, monkeypatch):
|
||||
jf = _FakeJellyfin(existing=[]) # the missing track isn't on the server yet
|
||||
_wire(monkeypatch, jf)
|
||||
|
||||
resp = client.post('/api/server/playlist/PL1/add-track',
|
||||
json={'track_id': _GUID, 'playlist_name': 'Disney'})
|
||||
body = resp.get_json()
|
||||
|
||||
assert body['success'] and body['message'] == 'Track added'
|
||||
assert len(jf.append_calls) == 1, 'should append in place'
|
||||
assert jf.update_calls == [], 'must NOT recreate the playlist (#837)'
|
||||
# append_to_playlist reads `.id` off the track — the endpoint must set it
|
||||
assert jf.append_calls[0][1] == [_GUID]
|
||||
|
||||
|
||||
def test_find_and_add_link_to_existing_track_touches_nothing(client, monkeypatch):
|
||||
# Matching a source to a track already in the playlist is a LINK, not an add.
|
||||
jf = _FakeJellyfin(existing=[_GUID])
|
||||
_wire(monkeypatch, jf)
|
||||
|
||||
resp = client.post('/api/server/playlist/PL1/add-track',
|
||||
json={'track_id': _GUID, 'playlist_name': 'Disney',
|
||||
'source_track_id': 'spotify-xyz'})
|
||||
body = resp.get_json()
|
||||
|
||||
assert body['success'] and body['message'] == 'Track linked'
|
||||
assert jf.append_calls == [] and jf.update_calls == []
|
||||
|
|
@ -19056,14 +19056,24 @@ def server_playlist_add_track(playlist_id):
|
|||
|
||||
elif active_server == 'jellyfin' and media_server_engine.client('jellyfin'):
|
||||
from core.sync.playlist_edit import plan_playlist_add
|
||||
current_tracks = media_server_engine.client('jellyfin').get_playlist_tracks(playlist_id) or []
|
||||
jf = media_server_engine.client('jellyfin')
|
||||
current_tracks = jf.get_playlist_tracks(playlist_id) or []
|
||||
track_ids = [str(t.ratingKey) for t in current_tracks]
|
||||
# Matching an unmatched source to a track already in the playlist
|
||||
# is a LINK, not a second copy — don't duplicate it (#768).
|
||||
plan = plan_playlist_add(track_ids, track_id, is_link=bool(source_track_id), position=position)
|
||||
if plan['should_insert']:
|
||||
new_track_objs = [type('T', (), {'ratingKey': tid, 'title': ''})() for tid in plan['new_ids']]
|
||||
media_server_engine.client('jellyfin').update_playlist(playlist_name, new_track_objs)
|
||||
# #837: append the ONE found track IN PLACE. The old path called
|
||||
# update_playlist(full track list), which on Jellyfin/Emby deletes
|
||||
# and recreates the playlist — wiping its description + cover image.
|
||||
# append_to_playlist adds in place (dedupe-safe), the same
|
||||
# non-destructive op the 'append' sync mode already uses. It reads
|
||||
# `.id` (not ratingKey) off each track, so set both.
|
||||
new_track_obj = type('T', (), {
|
||||
'id': str(track_id), 'ratingKey': str(track_id),
|
||||
'title': server_track_title or '',
|
||||
})()
|
||||
jf.append_to_playlist(playlist_name, [new_track_obj])
|
||||
_persist_find_and_add_match(source_track_id, active_server, track_id, server_track_title, source_title, source_artist, source_provider)
|
||||
return jsonify({"success": True, "message": "Track linked" if not plan['should_insert'] else "Track added"})
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue