danvics/soulsync
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 1

Wishlist: manual "add to wishlist" now skips already-owned tracks (#825)

Browse source 
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.
This commit is contained in:
BoulderBadgeDad 2026-06-09 10:55:45 -07:00
parent 8d133ecd60
commit 8633386f00
3 changed files with 101 additions and 12 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

27
core/wishlist/routes.py
View file

@ -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"),

51
tests/wishlist/test_routes.py
View file

@ -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

35
webui/static/wishlist-tools.js
View file

@ -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');