Sync wishlist re-add: build the IDENTICAL payload the auto-add uses
Root cause (the real one): the auto-add passes original_tracks_map[id] — tracks_json run through a specific normalization (album->dict with images/album_type/total_tracks/ release_date, artists->dicts). My re-add hand-rolled a different shape, so the stored spotify_data didn't match and the wishlist's nebula (which reads spotify_data.album. images[0].url) had no cover, plus album/single classification could differ. Fix: extract that normalization into one shared build_original_tracks_map() and use it in BOTH the live sync (core.discovery.sync) and the re-add. The re-add now resolves the track by source_track_id through the same map — byte-identical payload. Verified on a real sync row: re-add payload == live-sync payload, album.images present. (The shared normalizer is also copy-safe, fixing a latent tracks_json mutation in the old inline version.) Fallback (track absent from tracks_json) rebuilds through the same normalizer with the cover seeded from the row's image_url. 10 tests incl. a direct parity assertion.
This commit is contained in:
parent
5cad2c02b0
commit
d0966ec262
3 changed files with 132 additions and 139 deletions
|
|
@ -307,35 +307,11 @@ def run_sync_task(
|
|||
# This avoids needing to re-fetch it from Spotify
|
||||
logger.info("Converting JSON tracks to SpotifyTrack objects...")
|
||||
|
||||
# Store original track data with full album objects (for wishlist with cover art)
|
||||
# Normalize formats for wishlist: album must be dict {'name': ...}, artists must be [{'name': ...}]
|
||||
# Important: copy data — don't mutate tracks_json since SpotifyTrack expects List[str] artists
|
||||
original_tracks_map = {}
|
||||
for t in tracks_json:
|
||||
track_id = t.get('id', '')
|
||||
if track_id:
|
||||
normalized = dict(t)
|
||||
# Normalize album to dict format, preserving images and metadata
|
||||
raw_album = normalized.get('album', '')
|
||||
if isinstance(raw_album, str):
|
||||
normalized['album'] = {
|
||||
'name': raw_album or normalized.get('name', 'Unknown Album'),
|
||||
'images': [], 'album_type': 'single', 'total_tracks': 1, 'release_date': ''
|
||||
}
|
||||
elif not isinstance(raw_album, dict):
|
||||
normalized['album'] = {
|
||||
'name': str(raw_album) if raw_album else normalized.get('name', 'Unknown Album'),
|
||||
'images': [], 'album_type': 'single', 'total_tracks': 1, 'release_date': ''
|
||||
}
|
||||
else:
|
||||
# Dict — ensure required keys exist
|
||||
raw_album.setdefault('name', 'Unknown Album')
|
||||
raw_album.setdefault('images', [])
|
||||
# Normalize artists to list of dicts
|
||||
raw_artists = normalized.get('artists', [])
|
||||
if raw_artists and isinstance(raw_artists[0], str):
|
||||
normalized['artists'] = [{'name': a} for a in raw_artists]
|
||||
original_tracks_map[track_id] = normalized
|
||||
# Store original track data with full album objects (for wishlist with cover art).
|
||||
# Shared with the sync-detail "re-add to wishlist" action so both build the
|
||||
# IDENTICAL payload (album→dict + images, artists→dicts). Copy-safe.
|
||||
from core.sync.wishlist_readd import build_original_tracks_map
|
||||
original_tracks_map = build_original_tracks_map(tracks_json)
|
||||
|
||||
tracks = []
|
||||
for i, t in enumerate(tracks_json):
|
||||
|
|
|
|||
|
|
@ -1,7 +1,9 @@
|
|||
"""Rebuild the track data the sync used to auto-wishlist an unmatched track, so the
|
||||
sync-detail modal can re-add it with the EXACT same context (source_type='playlist'
|
||||
+ the playlist's name/id). Pure — no I/O; the web route supplies the parsed sync
|
||||
entry and calls the wishlist service.
|
||||
"""Build the wishlist-add payload for a synced track — the SINGLE source of truth
|
||||
shared by the live sync (core.discovery.sync) and the sync-detail "re-add to
|
||||
wishlist" action, so a re-add is byte-for-byte the same payload the auto-add used.
|
||||
|
||||
Pure — no I/O. The web route supplies the parsed sync entry and calls the wishlist
|
||||
service; the live sync calls build_original_tracks_map directly.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
|
@ -9,75 +11,88 @@ from __future__ import annotations
|
|||
from typing import Any, Dict, List, Optional
|
||||
|
||||
|
||||
def normalize_wishlist_track(track: Dict[str, Any]) -> Dict[str, Any]:
|
||||
"""Normalize ONE tracks_json track into the wishlist-add shape: album coerced to
|
||||
a dict (preserving images + album_type/total_tracks/release_date), artists to a
|
||||
list of dicts. Copy-safe — never mutates the input."""
|
||||
normalized = dict(track)
|
||||
raw_album = normalized.get('album', '')
|
||||
if isinstance(raw_album, dict):
|
||||
album = dict(raw_album)
|
||||
album.setdefault('name', 'Unknown Album')
|
||||
album.setdefault('images', [])
|
||||
normalized['album'] = album
|
||||
else:
|
||||
name = raw_album if isinstance(raw_album, str) else (str(raw_album) if raw_album else '')
|
||||
normalized['album'] = {
|
||||
'name': name or normalized.get('name', 'Unknown Album'),
|
||||
'images': [], 'album_type': 'single', 'total_tracks': 1, 'release_date': '',
|
||||
}
|
||||
raw_artists = normalized.get('artists', [])
|
||||
if raw_artists and isinstance(raw_artists[0], str):
|
||||
normalized['artists'] = [{'name': a} for a in raw_artists]
|
||||
return normalized
|
||||
|
||||
|
||||
def build_original_tracks_map(tracks_json: Optional[List[Dict[str, Any]]]) -> Dict[str, Dict[str, Any]]:
|
||||
"""``{track_id: normalized_track}`` for a sync's tracks_json — the full-fidelity
|
||||
map the live sync builds before auto-wishlisting unmatched tracks. One source of
|
||||
truth so the re-add matches the auto-add exactly."""
|
||||
out: Dict[str, Dict[str, Any]] = {}
|
||||
for t in tracks_json or []:
|
||||
if not isinstance(t, dict):
|
||||
continue
|
||||
track_id = t.get('id', '')
|
||||
if track_id:
|
||||
out[str(track_id)] = normalize_wishlist_track(t)
|
||||
return out
|
||||
|
||||
|
||||
def reconstruct_sync_track_data(
|
||||
track_results: Optional[List[Dict[str, Any]]],
|
||||
tracks: Optional[List[Dict[str, Any]]],
|
||||
track_index: int,
|
||||
) -> Optional[Dict[str, Any]]:
|
||||
"""Return the ``spotify_track_data`` dict to re-add a synced track to the wishlist.
|
||||
"""Return the wishlist-add ``spotify_track_data`` for re-adding a synced track.
|
||||
|
||||
Prefers the FULL original track from the cached playlist tracks (``tracks``, i.e.
|
||||
tracks_json) — matched by the track_result's ``source_track_id``, then by index —
|
||||
because that carries the full album object + images the original auto-add used.
|
||||
Falls back to a minimal dict rebuilt from the track_result's own fields.
|
||||
Resolves the track the SAME way the auto-add did: the normalized tracks_json
|
||||
entry (``build_original_tracks_map``), looked up by the track_result's
|
||||
``source_track_id`` — so the payload (full album object + images + album_type +
|
||||
total_tracks + artists-as-dicts) is identical to the original auto-add.
|
||||
|
||||
Returns None when the index is out of range, the row isn't a 'wishlist'
|
||||
(unmatched, auto-added) track, or there's no id to key on — so a caller can't
|
||||
re-wishlist a matched/downloaded track or an unidentifiable one.
|
||||
Only 'wishlist' rows are eligible. Falls back to a normalized rebuild from the
|
||||
track_result's own fields (with the album cover from its image_url) when the
|
||||
cached track is missing. None when ineligible or unidentifiable.
|
||||
"""
|
||||
if not track_results or track_index < 0 or track_index >= len(track_results):
|
||||
return None
|
||||
tr = track_results[track_index] or {}
|
||||
# Only rows the sync actually sent to the wishlist are re-addable.
|
||||
if tr.get('download_status') != 'wishlist':
|
||||
return None
|
||||
|
||||
sid = str(tr.get('source_track_id') or '')
|
||||
tracks = tracks or []
|
||||
|
||||
# Base: the FULL original track (best fidelity — full album object, ids, source,
|
||||
# artists) by index if its id matches, else by id search. Copied so we never
|
||||
# mutate the caller's tracks list.
|
||||
base: Optional[Dict[str, Any]] = None
|
||||
if 0 <= track_index < len(tracks):
|
||||
cand = tracks[track_index]
|
||||
if isinstance(cand, dict) and (not sid or str(cand.get('id') or '') == sid):
|
||||
base = dict(cand)
|
||||
if base is None and sid:
|
||||
match = next(
|
||||
(t for t in tracks if isinstance(t, dict) and str(t.get('id') or '') == sid),
|
||||
None,
|
||||
)
|
||||
if match is not None:
|
||||
base = dict(match)
|
||||
# Primary: the exact normalized track the auto-add used.
|
||||
if sid:
|
||||
payload = build_original_tracks_map(tracks).get(sid)
|
||||
if payload:
|
||||
return payload
|
||||
|
||||
# No full track in the cache — rebuild a minimal dict from the track_result.
|
||||
if not (isinstance(base, dict) and base.get('id')):
|
||||
if not sid:
|
||||
return None
|
||||
base = {
|
||||
'id': sid,
|
||||
'name': tr.get('name') or '',
|
||||
'artists': [{'name': tr.get('artist') or ''}],
|
||||
'album': {'name': tr.get('album') or ''},
|
||||
'duration_ms': tr.get('duration_ms') or 0,
|
||||
}
|
||||
|
||||
# Ensure the cover carries through. The wishlist display reads
|
||||
# spotify_data.album.images; tracks_json is sometimes stored WITHOUT images, so
|
||||
# backfill from the track_result's image_url (the album art the sync extracted)
|
||||
# whenever the album has none. This is what makes the re-add reach full parity
|
||||
# with the original auto-add's appearance.
|
||||
album = base.get('album')
|
||||
if not isinstance(album, dict):
|
||||
album = {'name': base.get('name', '')}
|
||||
else:
|
||||
album = dict(album)
|
||||
img = tr.get('image_url')
|
||||
if img and not album.get('images'):
|
||||
album['images'] = [{'url': img}]
|
||||
base['album'] = album
|
||||
return base
|
||||
# Fallback: rebuild from the track_result fields, through the SAME normalizer so
|
||||
# the shape matches, and seed the album cover from the row's image_url.
|
||||
if not sid:
|
||||
return None
|
||||
album: Dict[str, Any] = {'name': tr.get('album') or '', 'album_type': 'single',
|
||||
'total_tracks': 1, 'release_date': ''}
|
||||
if tr.get('image_url'):
|
||||
album['images'] = [{'url': tr['image_url']}]
|
||||
return normalize_wishlist_track({
|
||||
'id': sid,
|
||||
'name': tr.get('name') or '',
|
||||
'artists': [{'name': tr.get('artist') or ''}],
|
||||
'album': album,
|
||||
'duration_ms': tr.get('duration_ms') or 0,
|
||||
})
|
||||
|
||||
|
||||
__all__ = ["reconstruct_sync_track_data"]
|
||||
__all__ = ["normalize_wishlist_track", "build_original_tracks_map", "reconstruct_sync_track_data"]
|
||||
|
|
|
|||
|
|
@ -1,13 +1,17 @@
|
|||
"""Re-add a synced unmatched track to the wishlist with the original context.
|
||||
"""Re-add a synced unmatched track to the wishlist with the EXACT auto-add payload.
|
||||
|
||||
reconstruct_sync_track_data rebuilds the spotify_track_data the sync used. It must
|
||||
prefer the full cached track (with album images), fall back to the track_result
|
||||
fields, and refuse anything that wasn't a 'wishlist' row.
|
||||
The live sync and the sync-detail re-add must build the identical wishlist payload
|
||||
(via build_original_tracks_map), so a re-added track is indistinguishable from the
|
||||
original auto-add — including its album cover, album_type, and artist shape.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from core.sync.wishlist_readd import reconstruct_sync_track_data
|
||||
from core.sync.wishlist_readd import (
|
||||
build_original_tracks_map,
|
||||
normalize_wishlist_track,
|
||||
reconstruct_sync_track_data,
|
||||
)
|
||||
|
||||
|
||||
def _tr(index, sid, status='wishlist', **kw):
|
||||
|
|
@ -18,75 +22,69 @@ def _tr(index, sid, status='wishlist', **kw):
|
|||
|
||||
|
||||
def _full(sid, name="Song", with_images=True):
|
||||
album = {"name": "Album"}
|
||||
album = {"name": "Album", "album_type": "album", "total_tracks": 12}
|
||||
if with_images:
|
||||
album["images"] = [{"url": "http://cdn/cover.jpg"}]
|
||||
album["images"] = [{"url": "http://cdn/cover.jpg", "height": 640, "width": 640}]
|
||||
return {"id": sid, "name": name, "artists": [{"name": "Artist"}], "album": album,
|
||||
"duration_ms": 200000, "popularity": 50}
|
||||
|
||||
|
||||
def test_prefers_full_original_track_by_index():
|
||||
# ── normalize_wishlist_track ──────────────────────────────────────────────────
|
||||
|
||||
def test_normalize_string_album_to_dict():
|
||||
out = normalize_wishlist_track({"id": "a", "name": "T", "album": "My Single", "artists": ["X"]})
|
||||
assert out["album"] == {"name": "My Single", "images": [], "album_type": "single",
|
||||
"total_tracks": 1, "release_date": ""}
|
||||
assert out["artists"] == [{"name": "X"}] # strings -> dicts
|
||||
|
||||
|
||||
def test_normalize_dict_album_preserves_images_and_type():
|
||||
out = normalize_wishlist_track(_full("a"))
|
||||
assert out["album"]["images"][0]["url"] == "http://cdn/cover.jpg"
|
||||
assert out["album"]["album_type"] == "album"
|
||||
assert out["album"]["total_tracks"] == 12
|
||||
|
||||
|
||||
def test_normalize_is_copy_safe():
|
||||
src = {"id": "a", "album": {"name": "A"}, "artists": ["X"]}
|
||||
normalize_wishlist_track(src)
|
||||
assert "images" not in src["album"] # source untouched
|
||||
assert src["artists"] == ["X"]
|
||||
|
||||
|
||||
# ── reconstruct: parity with the live auto-add ────────────────────────────────
|
||||
|
||||
def test_payload_is_identical_to_live_sync_map():
|
||||
# THE PARITY GUARANTEE: re-add payload == what build_original_tracks_map (used by
|
||||
# the live sync) produces for the same track.
|
||||
trs = [_tr(0, "sp_a"), _tr(1, "sp_b")]
|
||||
tracks = [_full("sp_a"), _full("sp_b", name="Other")]
|
||||
out = reconstruct_sync_track_data(trs, tracks, 1)
|
||||
assert out["id"] == "sp_b" # the full original track
|
||||
assert out["album"]["images"][0]["url"] == "http://cdn/cover.jpg" # its own art kept
|
||||
assert out is not tracks[1] # a copy, not the source entry
|
||||
assert out == build_original_tracks_map(tracks)["sp_b"]
|
||||
assert out["album"]["images"][0]["url"] == "http://cdn/cover.jpg" # cover carries through
|
||||
|
||||
|
||||
def test_backfills_album_image_when_full_track_has_none():
|
||||
# THE BUG: tracks_json stored without album.images, but the track_result has the
|
||||
# cover. The re-add must carry the image through (parity with the auto-add).
|
||||
trs = [_tr(0, "sp_a", image_url="http://img/cover250.jpg")]
|
||||
tracks = [_full("sp_a", with_images=False)] # full data but NO images
|
||||
out = reconstruct_sync_track_data(trs, tracks, 0)
|
||||
assert out["id"] == "sp_a"
|
||||
assert out["album"]["images"] == [{"url": "http://img/cover250.jpg"}]
|
||||
|
||||
|
||||
def test_full_track_with_images_is_not_overwritten():
|
||||
# When the cached track already has real album art, keep it (don't downgrade to
|
||||
# the 250px track_result thumb).
|
||||
trs = [_tr(0, "sp_a", image_url="http://img/thumb.jpg")]
|
||||
tracks = [_full("sp_a", with_images=True)]
|
||||
out = reconstruct_sync_track_data(trs, tracks, 0)
|
||||
assert out["album"]["images"][0]["url"] == "http://cdn/cover.jpg" # original kept
|
||||
|
||||
|
||||
def test_does_not_mutate_source_tracks_list():
|
||||
trs = [_tr(0, "sp_a", image_url="http://img/x.jpg")]
|
||||
tracks = [_full("sp_a", with_images=False)]
|
||||
reconstruct_sync_track_data(trs, tracks, 0)
|
||||
assert "images" not in tracks[0]["album"] # source untouched
|
||||
|
||||
|
||||
def test_matches_by_id_when_index_misaligns():
|
||||
# tracks_json in a different order than track_results -> match by source_track_id.
|
||||
def test_resolves_by_source_track_id_not_position():
|
||||
trs = [_tr(0, "sp_a"), _tr(1, "sp_b")]
|
||||
tracks = [_full("sp_b"), _full("sp_a")] # reversed
|
||||
out = reconstruct_sync_track_data(trs, tracks, 0)
|
||||
assert out["id"] == "sp_a" # found by id, not by position
|
||||
tracks = [_full("sp_b"), _full("sp_a")] # tracks_json reversed
|
||||
out = reconstruct_sync_track_data(trs, tracks, 0) # row 0 -> sp_a
|
||||
assert out["id"] == "sp_a"
|
||||
|
||||
|
||||
def test_falls_back_to_track_result_fields_when_no_full_track():
|
||||
def test_fallback_rebuilds_with_cover_when_track_missing():
|
||||
# Track not in tracks_json -> rebuild from the row, seed cover from image_url,
|
||||
# run through the same normalizer (album dict + artists dicts).
|
||||
trs = [_tr(0, "sp_x", name="Real Love Baby", artist="Father John Misty",
|
||||
album="Real Love Baby", image_url="http://img/x.jpg", duration_ms=188000)]
|
||||
out = reconstruct_sync_track_data(trs, [], 0) # no tracks_json
|
||||
out = reconstruct_sync_track_data(trs, [], 0)
|
||||
assert out["id"] == "sp_x"
|
||||
assert out["name"] == "Real Love Baby"
|
||||
assert out["artists"] == [{"name": "Father John Misty"}]
|
||||
assert out["album"]["name"] == "Real Love Baby"
|
||||
assert out["album"]["images"] == [{"url": "http://img/x.jpg"}]
|
||||
assert out["duration_ms"] == 188000
|
||||
|
||||
|
||||
def test_fallback_without_image_omits_images():
|
||||
out = reconstruct_sync_track_data([_tr(0, "sp_x", album="A")], [], 0)
|
||||
assert "images" not in out["album"]
|
||||
assert out["album"]["name"] == "Real Love Baby"
|
||||
|
||||
|
||||
def test_refuses_non_wishlist_row():
|
||||
# A matched/downloaded track must not be re-wishlistable.
|
||||
trs = [_tr(0, "sp_a", status="completed")]
|
||||
assert reconstruct_sync_track_data(trs, [_full("sp_a")], 0) is None
|
||||
|
||||
|
|
@ -99,5 +97,9 @@ def test_refuses_out_of_range_or_empty():
|
|||
|
||||
|
||||
def test_refuses_when_no_id_and_no_full_track():
|
||||
# A wishlist row with no source_track_id and no cached track is unidentifiable.
|
||||
assert reconstruct_sync_track_data([_tr(0, "")], [], 0) is None
|
||||
|
||||
|
||||
def test_build_map_skips_idless_and_non_dicts():
|
||||
m = build_original_tracks_map([_full("a"), {"name": "no id"}, "garbage", None])
|
||||
assert set(m.keys()) == {"a"}
|
||||
|
|
|
|||
Loading…
Reference in a new issue