From 8633386f0056574f47cda09e56e682783fa47aa8 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Tue, 9 Jun 2026 10:55:45 -0700 Subject: [PATCH] Wishlist: manual "add to wishlist" now skips already-owned tracks (#825) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The manual album "Add to Wishlist" modal had NO ownership check at any layer — the album view opened the modal without ownership info, the modal added every track, and the backend (add_album_track_to_wishlist) added each unconditionally. So adding an album you (partially) own dumped the owned tracks straight into the wishlist (carlosjfcasero #825) — and the auto-cleanup doesn't reliably remove them. The bulk discography path already dedups (full missing-track analysis); this path didn't. Backend (the reliable seam): add_album_track_to_wishlist now skips a track that already exists in the library, gated on the same wishlist.allow_duplicate_tracks toggle the watchlist scan + cleanup use — OFF → skip owned (returns {success, skipped:true}), ON → add anyway. Default is ON, so default users are unaffected; the quality re-download flow uses a different endpoint, so it's untouched. Frontend: handleAddToWishlist + addModalTracksToWishlist count skipped tracks separately so the toast is honest ("Added 3 (5 already owned)" / "All N already in your library") instead of falsely claiming owned tracks were added. Tests: skips owned when duplicates off, adds missing when off, adds owned when on (and doesn't even run the check then). 205 wishlist tests pass. --- core/wishlist/routes.py | 27 ++++++++++++++++++ tests/wishlist/test_routes.py | 51 ++++++++++++++++++++++++++++++++++ webui/static/wishlist-tools.js | 35 +++++++++++++++-------- 3 files changed, 101 insertions(+), 12 deletions(-) diff --git a/core/wishlist/routes.py b/core/wishlist/routes.py index 53a36af2..c05a626c 100644 --- a/core/wishlist/routes.py +++ b/core/wishlist/routes.py @@ -423,6 +423,33 @@ def add_album_track_to_wishlist( track_data = _build_track_data(track, album) + # #825: don't add a track that's already in the library, unless the user + # has opted into duplicates. The manual album "add to wishlist" modal + # otherwise dumped owned tracks straight into the wishlist with no check + # (carlosjfcasero) — and the auto-cleanup may not reliably remove them. + # Respects the same wishlist.allow_duplicate_tracks toggle the watchlist + # scan + cleanup use: OFF → skip owned, ON → add anyway. (The quality + # re-download flow uses a different endpoint, so it's unaffected.) + try: + from config.settings import config_manager as _cfg + if not _cfg.get('wishlist.allow_duplicate_tracks', True): + _db = runtime.get_music_database() + _existing, _conf = _db.check_track_exists( + track.get('name', ''), artist.get('name', ''), + confidence_threshold=0.7, + server_source=runtime.active_server, + album=album.get('name', ''), + ) + if _existing and _conf >= 0.7: + runtime.logger.info( + "[Wishlist Add] skipping '%s' by '%s' — already in library " + "(allow_duplicate_tracks is off)", + track.get('name'), artist.get('name')) + return {"success": True, "skipped": True, + "message": f"'{track.get('name')}' is already in your library"}, 200 + except Exception as _own_err: + runtime.logger.debug("Wishlist add ownership check failed (adding anyway): %s", _own_err) + enhanced_source_context = { **(source_context or {}), "artist_id": artist.get("id"), diff --git a/tests/wishlist/test_routes.py b/tests/wishlist/test_routes.py index 8292aaba..43c34943 100644 --- a/tests/wishlist/test_routes.py +++ b/tests/wishlist/test_routes.py @@ -561,3 +561,54 @@ def test_add_album_track_to_wishlist_requires_required_fields(): "success": False, "error": "Missing required fields: track, artist, album", } + + +# ── #825: don't add already-owned tracks to the wishlist (respects the toggle) ── + +def _own_track_args(): + return dict(track={"id": "t", "name": "Song", "artists": [{"name": "A"}]}, + artist={"id": "a1", "name": "A"}, + album={"id": "al1", "name": "Album"}, source_type="album") + + +def test_add_album_track_skips_owned_when_duplicates_off(monkeypatch): + from config.settings import config_manager + monkeypatch.setattr(config_manager, 'get', + lambda key, default=None: False if key == 'wishlist.allow_duplicate_tracks' else default) + runtime, service, db, _logger, _ = _build_runtime() + db.check_track_exists = lambda *a, **k: (object(), 0.95) # already owned + + payload, status = add_album_track_to_wishlist(runtime, **_own_track_args()) + + assert status == 200 + assert payload.get("skipped") is True + assert service.add_calls == [] # nothing added + + +def test_add_album_track_adds_missing_when_duplicates_off(monkeypatch): + from config.settings import config_manager + monkeypatch.setattr(config_manager, 'get', + lambda key, default=None: False if key == 'wishlist.allow_duplicate_tracks' else default) + runtime, service, db, _logger, _ = _build_runtime() + db.check_track_exists = lambda *a, **k: (None, 0.0) # not in library + + payload, status = add_album_track_to_wishlist(runtime, **_own_track_args()) + + assert status == 200 + assert not payload.get("skipped") + assert len(service.add_calls) == 1 # added + + +def test_add_album_track_adds_owned_when_duplicates_on(monkeypatch): + from config.settings import config_manager + monkeypatch.setattr(config_manager, 'get', + lambda key, default=None: True if key == 'wishlist.allow_duplicate_tracks' else default) + runtime, service, db, _logger, _ = _build_runtime() + called = [] + db.check_track_exists = lambda *a, **k: called.append(1) or (object(), 0.99) + + payload, status = add_album_track_to_wishlist(runtime, **_own_track_args()) + + assert status == 200 + assert len(service.add_calls) == 1 # added anyway (user wants dupes) + assert called == [] # ownership check skipped entirely diff --git a/webui/static/wishlist-tools.js b/webui/static/wishlist-tools.js index cbc23c52..1842e72a 100644 --- a/webui/static/wishlist-tools.js +++ b/webui/static/wishlist-tools.js @@ -953,6 +953,7 @@ async function handleAddToWishlist() { let successCount = 0; let errorCount = 0; + let skippedCount = 0; // already-in-library tracks the backend declined to add (#825) // Add each track to wishlist individually for (const track of tracks) { @@ -1026,7 +1027,10 @@ async function handleAddToWishlist() { const result = await response.json(); - if (result.success) { + if (result.success && result.skipped) { + skippedCount++; // already in library — not added (#825) + console.log(`⏭️ "${track.name}" already in library — skipped`); + } else if (result.success) { successCount++; console.log(`✅ Added "${track.name}" to wishlist`); } else { @@ -1040,12 +1044,15 @@ async function handleAddToWishlist() { } } - // Show completion message - if (successCount > 0) { - const message = errorCount > 0 - ? `Added ${successCount}/${tracks.length} tracks to wishlist (${errorCount} failed)` - : `Added ${successCount} tracks to wishlist`; - showToast(message, successCount === tracks.length ? 'success' : 'warning'); + // Show completion message (#825: report already-owned tracks honestly + // instead of counting them as "added"). + if (successCount === 0 && skippedCount > 0 && errorCount === 0) { + showToast(`All ${skippedCount} track${skippedCount !== 1 ? 's' : ''} already in your library`, 'success'); + } else if (successCount > 0) { + let message = `Added ${successCount} track${successCount !== 1 ? 's' : ''} to wishlist`; + if (skippedCount > 0) message += ` (${skippedCount} already owned)`; + if (errorCount > 0) message += ` — ${errorCount} failed`; + showToast(message, errorCount > 0 ? 'warning' : 'success'); } else { showToast('Failed to add any tracks to wishlist', 'error'); } @@ -1365,6 +1372,7 @@ async function addModalTracksToWishlist(playlistId) { try { let successCount = 0; let errorCount = 0; + let skippedCount = 0; // already-in-library tracks the backend declined to add (#825) // Add each track to wishlist individually let wingItSkipped = 0; @@ -1479,11 +1487,14 @@ async function addModalTracksToWishlist(playlistId) { } } - // Show result toast - if (successCount > 0) { - let message = errorCount > 0 - ? `Added ${successCount}/${tracks.length} tracks to wishlist (${errorCount} failed)` - : `Added ${successCount} tracks to wishlist`; + // Show result toast (#825: report already-owned skips honestly). + if (successCount === 0 && skippedCount > 0 && errorCount === 0 && wingItSkipped === 0) { + showToast(`All ${skippedCount} track${skippedCount !== 1 ? 's' : ''} already in your library`, 'success'); + await closeDownloadMissingModal(playlistId); + } else if (successCount > 0) { + let message = `Added ${successCount} track${successCount !== 1 ? 's' : ''} to wishlist`; + if (skippedCount > 0) message += ` (${skippedCount} already owned)`; + if (errorCount > 0) message += ` — ${errorCount} failed`; if (wingItSkipped > 0) message += ` (${wingItSkipped} wing-it skipped)`; showToast(message, 'success');