diff --git a/core/wishlist/ignore.py b/core/wishlist/ignore.py new file mode 100644 index 00000000..beced2c4 --- /dev/null +++ b/core/wishlist/ignore.py @@ -0,0 +1,162 @@ +"""Wishlist ignore-list — a TTL'd skip-gate for the wishlist (#874). + +When a user removes a track from the wishlist or cancels an in-flight +wishlist download, SoulSync would otherwise re-add it on the next +automatic cycle (watchlist scan, failed-track capture, or the cancel +handler's own re-add), so the same release downloads → fails/cancels → +re-queues forever. The ignore list records the user's "stop +auto-grabbing this" intent, and the wishlist *add* path checks it, +skipping automatic re-adds until the entry ages out. + +It is deliberately softer than the blocklist: + - it **expires** after ``IGNORE_TTL_DAYS`` so the track is re-attempted + again later rather than banned forever, and + - it **never blocks a manual force-download** — only the automatic + re-queue. (Manual downloads don't go through ``add_to_wishlist`` at + all, and an explicit manual *add* both bypasses the gate and clears + any existing ignore for the track.) + +This module is pure decision logic — no database handle and no clock of +its own; the caller passes ``now``. The SQL lives in ``MusicDatabase`` +as a thin wrapper around these helpers, which keeps the TTL / id-matching +rules unit-testable without a database. +""" + +from __future__ import annotations + +from datetime import datetime, timedelta +from typing import Any, Dict, Iterable, List, Optional, Set, Tuple + +IGNORE_TTL_DAYS = 30 + +# Recognised reasons (free-text tolerated; these are the canonical two). +REASON_REMOVED = "removed" +REASON_CANCELLED = "cancelled" + + +def normalize_ignore_id(track_id: Any) -> str: + """Canonical key for a wishlist track id. + + The wishlist stores some ids as a composite ``::`` + (when ``wishlist.allow_duplicate_tracks`` is on). The add-path gate + keys on the bare track id (``spotify_track_data['id']``), so we strip + the ``::album`` suffix here so an ignore recorded from a composite-id + wishlist row still matches the bare-id add attempt, and vice-versa. + Returns ``''`` for falsy/blank input. + """ + s = str(track_id or "").strip() + if not s: + return "" + return s.split("::", 1)[0] + + +def _parse_ts(value: Any) -> Optional[datetime]: + """Best-effort parse of a sqlite TIMESTAMP / ISO string / datetime.""" + if isinstance(value, datetime): + return value + if not value: + return None + s = str(value).strip() + for fmt in ( + "%Y-%m-%d %H:%M:%S", + "%Y-%m-%dT%H:%M:%S", + "%Y-%m-%d %H:%M:%S.%f", + "%Y-%m-%dT%H:%M:%S.%f", + ): + try: + return datetime.strptime(s, fmt) + except ValueError: + continue + try: + return datetime.fromisoformat(s) + except ValueError: + return None + + +def is_expired(created_at: Any, now: datetime, ttl_days: int = IGNORE_TTL_DAYS) -> bool: + """True when an ignore entry created at ``created_at`` has aged past TTL. + + Fail-SAFE in the gate's favour: an unparseable/blank timestamp is + treated as **expired** (returns True) so a corrupt row can never wedge + a track out of the wishlist permanently — the worst case is the + ignore silently lapses and the track becomes eligible again. + """ + created = _parse_ts(created_at) + if created is None: + return True + return now >= created + timedelta(days=ttl_days) + + +def active_ignored_ids( + rows: Iterable[Dict[str, Any]], now: datetime, ttl_days: int = IGNORE_TTL_DAYS +) -> Set[str]: + """Set of normalized track ids whose ignore entry is still within TTL.""" + out: Set[str] = set() + for row in rows or []: + tid = normalize_ignore_id(row.get("track_id")) + if tid and not is_expired(row.get("created_at"), now, ttl_days): + out.add(tid) + return out + + +def is_ignored( + rows: Iterable[Dict[str, Any]], track_id: Any, now: datetime, ttl_days: int = IGNORE_TTL_DAYS +) -> bool: + """Whether ``track_id`` matches an in-TTL entry among ``rows``.""" + key = normalize_ignore_id(track_id) + if not key: + return False + return key in active_ignored_ids(rows, now, ttl_days) + + +def extract_display(data: Any) -> Tuple[str, str]: + """Pull a (track_name, artist_name) pair from a Spotify-shaped dict. + + Tolerates ``artists`` as a list of dicts or bare strings, and missing + fields. Used to give ignore-list rows a human label for the UI. + Returns ``('', '')`` when nothing usable is present. + """ + if not isinstance(data, dict): + return "", "" + name = str(data.get("name") or "").strip() + artist = "" + artists = data.get("artists") or [] + if isinstance(artists, list) and artists: + first = artists[0] + if isinstance(first, dict): + artist = str(first.get("name") or "").strip() + else: + artist = str(first or "").strip() + return name, artist + + +def ignore_wishlist_track( + database: Any, + profile_id: int, + track_id: Any, + reason: str, + spotify_data: Any = None, +) -> bool: + """Record an ignore entry for a wishlist track. Best-effort; never raises. + + Copies the track's display name/artist from ``spotify_data`` when + provided (callers should capture it BEFORE removing the wishlist row, + since the row may be gone afterwards); otherwise leaves them blank. + Returns True when an entry was written. + """ + key = normalize_ignore_id(track_id) + if not key or database is None: + return False + name, artist = extract_display(spotify_data or {}) + try: + return bool( + database.add_to_wishlist_ignore( + key, + track_name=name, + artist_name=artist, + reason=reason or REASON_REMOVED, + profile_id=profile_id, + ) + ) + except Exception: + return False diff --git a/core/wishlist/routes.py b/core/wishlist/routes.py index c05a626c..a467676c 100644 --- a/core/wishlist/routes.py +++ b/core/wishlist/routes.py @@ -310,12 +310,30 @@ def remove_track_from_wishlist( if not spotify_track_id: return {"success": False, "error": "No spotify_track_id provided"}, 400 - success = get_wishlist_service().remove_track_from_wishlist( + service = get_wishlist_service() + _db = getattr(service, "database", None) + # #874: capture the track's display info BEFORE removal (the row is + # gone afterwards) so the ignore-list entry carries a human label. + _ignore_data = None + try: + if _db is not None: + _ignore_data = _db.get_wishlist_spotify_data( + spotify_track_id, profile_id=runtime.profile_id) + except Exception: + _ignore_data = None + + success = service.remove_track_from_wishlist( spotify_track_id, profile_id=runtime.profile_id, ) if success: + # #874: a user-initiated remove means "stop auto-requeuing this". + # Record a TTL'd ignore so the watchlist/auto-processor doesn't + # re-add it. Best-effort — never fails the remove. + from core.wishlist.ignore import ignore_wishlist_track, REASON_REMOVED + ignore_wishlist_track(_db, runtime.profile_id, + spotify_track_id, REASON_REMOVED, spotify_data=_ignore_data) runtime.logger.info("Successfully removed track from wishlist: %s", spotify_track_id) return {"success": True, "message": "Track removed from wishlist"}, 200 @@ -358,13 +376,20 @@ def remove_album_from_wishlist( if matched: spotify_track_id = track.get("track_id") or track.get("spotify_track_id") or track.get("id") if spotify_track_id: - tracks_to_remove.append(spotify_track_id) + # Keep the loaded spotify_data alongside the id so the #874 + # ignore entry can be labelled without a second DB read. + tracks_to_remove.append((spotify_track_id, spotify_data)) + from core.wishlist.ignore import ignore_wishlist_track, REASON_REMOVED + _db = getattr(wishlist_service, "database", None) removed_count = 0 album_remove_pid = runtime.profile_id - for spotify_track_id in tracks_to_remove: + for spotify_track_id, track_spotify_data in tracks_to_remove: if wishlist_service.remove_track_from_wishlist(spotify_track_id, profile_id=album_remove_pid): removed_count += 1 + # #874: user removed the whole album → ignore each track. + ignore_wishlist_track(_db, album_remove_pid, + spotify_track_id, REASON_REMOVED, spotify_data=track_spotify_data) if removed_count > 0: runtime.logger.info("Successfully removed %s tracks from album %s", removed_count, album_id) @@ -390,11 +415,22 @@ def remove_batch_from_wishlist( if not spotify_track_ids or not isinstance(spotify_track_ids, list): return {"success": False, "error": "Missing or invalid spotify_track_ids"}, 400 + from core.wishlist.ignore import ignore_wishlist_track, REASON_REMOVED + service = get_wishlist_service() + _db = getattr(service, "database", None) removed = 0 pid = runtime.profile_id for track_id in spotify_track_ids: - if get_wishlist_service().remove_track_from_wishlist(track_id, profile_id=pid): + # Capture label before the row is deleted (#874). + _data = None + try: + if _db is not None: + _data = _db.get_wishlist_spotify_data(track_id, profile_id=pid) + except Exception: + _data = None + if service.remove_track_from_wishlist(track_id, profile_id=pid): removed += 1 + ignore_wishlist_track(_db, pid, track_id, REASON_REMOVED, spotify_data=_data) runtime.logger.info("Batch removed %s track(s) from wishlist", removed) return { diff --git a/database/music_database.py b/database/music_database.py index 1f6078fb..30d2c2b3 100644 --- a/database/music_database.py +++ b/database/music_database.py @@ -344,7 +344,29 @@ class MusicDatabase: source_info TEXT -- JSON of source context (playlist name, album info, etc.) ) """) - + + # Wishlist ignore-list (#874): a TTL'd skip-gate. When a user + # removes a track from the wishlist or cancels an in-flight + # wishlist download, the track is recorded here so the automatic + # re-add paths (watchlist scan, failed-track capture, cancel + # re-add) skip it until the entry ages out (see core.wishlist. + # ignore.IGNORE_TTL_DAYS). Softer than `blocklist`: it expires + # and never blocks a manual force-download. Keyed on the bare + # track id; unique per (profile, track) so re-ignoring refreshes. + cursor.execute(""" + CREATE TABLE IF NOT EXISTS wishlist_ignore ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + profile_id INTEGER NOT NULL DEFAULT 1, + track_id TEXT NOT NULL, + track_name TEXT DEFAULT '', + artist_name TEXT DEFAULT '', + reason TEXT DEFAULT 'removed', -- 'removed' | 'cancelled' + created_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP, + UNIQUE(profile_id, track_id) + ) + """) + cursor.execute("CREATE INDEX IF NOT EXISTS idx_wishlist_ignore_profile ON wishlist_ignore (profile_id, track_id)") + # Watchlist table for storing artists to monitor for new releases cursor.execute(""" CREATE TABLE IF NOT EXISTS watchlist_artists ( @@ -8851,6 +8873,21 @@ class MusicDatabase: _blocked[0], _blocked[1]) return False + # Ignore-list guard (#874): a user who removed or cancelled this + # track asked us to stop AUTO-requeuing it — every automatic + # re-add funnels through here, so one check covers them all. + # A *manual* add is explicit user intent → bypass the gate AND + # clear any stale ignore so it sticks. Fail-open: any error here + # must never block a legitimate wishlist add. + try: + if source_type == 'manual': + self.remove_from_wishlist_ignore(track_id, profile_id=profile_id) + elif self.is_track_ignored(track_id, profile_id=profile_id): + logger.info("Skipping wishlist add — track is on the ignore-list (#874): %s", track_id) + return False + except Exception as _ignore_exc: + logger.debug("Wishlist ignore-list check skipped (fail-open): %s", _ignore_exc) + from core.library import manual_library_match as _mlm if _mlm.get_match_for_track(self, profile_id, spotify_track_data): logger.info( @@ -8995,7 +9032,147 @@ class MusicDatabase: except Exception as e: logger.error(f"Error removing track from wishlist: {e}") return False - + + # ── Wishlist ignore-list (#874) ────────────────────────────────────── + # A TTL'd skip-gate consulted by add_to_wishlist so user-removed / + # user-cancelled tracks are not auto-re-queued. All methods fail-open + # (an error here must never block a legitimate wishlist add). + + def add_to_wishlist_ignore(self, track_id: str, track_name: str = "", + artist_name: str = "", reason: str = "removed", + profile_id: int = 1) -> bool: + """Record (or refresh) an ignore entry for a wishlist track id. + + Keyed on the bare track id; UNIQUE(profile_id, track_id) means a + repeat ignore replaces the row and so refreshes its TTL clock. + """ + from core.wishlist.ignore import normalize_ignore_id + key = normalize_ignore_id(track_id) + if not key: + return False + try: + with self._get_connection() as conn: + cursor = conn.cursor() + cursor.execute(""" + INSERT OR REPLACE INTO wishlist_ignore + (profile_id, track_id, track_name, artist_name, reason, created_at) + VALUES (?, ?, ?, ?, ?, CURRENT_TIMESTAMP) + """, (profile_id, key, track_name or "", artist_name or "", reason or "removed")) + conn.commit() + logger.info("Added track to wishlist ignore-list (%s): '%s' [%s]", + reason or "removed", track_name or key, key) + return True + except Exception as e: + logger.error("Error adding to wishlist ignore-list: %s", e) + return False + + def is_track_ignored(self, track_id: str, profile_id: int = 1, + ttl_days: Optional[int] = None) -> bool: + """Whether ``track_id`` has a non-expired ignore entry. Fail-open False.""" + from core.wishlist.ignore import normalize_ignore_id, is_expired, IGNORE_TTL_DAYS + ttl = IGNORE_TTL_DAYS if ttl_days is None else ttl_days + key = normalize_ignore_id(track_id) + if not key: + return False + try: + with self._get_connection() as conn: + cursor = conn.cursor() + cursor.execute( + "SELECT created_at FROM wishlist_ignore WHERE profile_id = ? AND track_id = ?", + (profile_id, key)) + row = cursor.fetchone() + if not row: + return False + return not is_expired(row["created_at"], datetime.now(), ttl) + except Exception as e: + logger.debug("is_track_ignored failed open: %s", e) + return False + + def remove_from_wishlist_ignore(self, track_id: str, profile_id: int = 1) -> bool: + """Un-ignore a track (manual override / UI action). Returns True if a row went.""" + from core.wishlist.ignore import normalize_ignore_id + key = normalize_ignore_id(track_id) + if not key: + return False + try: + with self._get_connection() as conn: + cursor = conn.cursor() + cursor.execute( + "DELETE FROM wishlist_ignore WHERE profile_id = ? AND track_id = ?", + (profile_id, key)) + conn.commit() + return cursor.rowcount > 0 + except Exception as e: + logger.error("Error removing from wishlist ignore-list: %s", e) + return False + + def get_wishlist_ignore(self, profile_id: int = 1, + ttl_days: Optional[int] = None) -> List[Dict[str, Any]]: + """Active (non-expired) ignore entries, newest first; purges lapsed rows.""" + from core.wishlist.ignore import is_expired, IGNORE_TTL_DAYS + ttl = IGNORE_TTL_DAYS if ttl_days is None else ttl_days + try: + with self._get_connection() as conn: + cursor = conn.cursor() + cursor.execute( + "SELECT track_id, track_name, artist_name, reason, created_at " + "FROM wishlist_ignore WHERE profile_id = ? ORDER BY created_at DESC", + (profile_id,)) + rows = cursor.fetchall() + now = datetime.now() + active, expired_ids = [], [] + for r in rows: + if is_expired(r["created_at"], now, ttl): + expired_ids.append(r["track_id"]) + else: + active.append({ + "track_id": r["track_id"], + "track_name": r["track_name"] or "", + "artist_name": r["artist_name"] or "", + "reason": r["reason"] or "removed", + "created_at": r["created_at"], + }) + # Opportunistic housekeeping so the table can't grow unbounded. + if expired_ids: + cursor.executemany( + "DELETE FROM wishlist_ignore WHERE profile_id = ? AND track_id = ?", + [(profile_id, tid) for tid in expired_ids]) + conn.commit() + return active + except Exception as e: + logger.error("Error reading wishlist ignore-list: %s", e) + return [] + + def clear_wishlist_ignore(self, profile_id: int = 1) -> int: + """Drop every ignore entry for a profile. Returns rows removed.""" + try: + with self._get_connection() as conn: + cursor = conn.cursor() + cursor.execute("DELETE FROM wishlist_ignore WHERE profile_id = ?", (profile_id,)) + conn.commit() + return cursor.rowcount + except Exception as e: + logger.error("Error clearing wishlist ignore-list: %s", e) + return 0 + + def get_wishlist_spotify_data(self, track_id: str, profile_id: int = 1) -> Dict[str, Any]: + """Parsed ``spotify_data`` for a wishlist row, or {}. Used to label an + ignore entry with the track's name/artist before the row is removed.""" + try: + with self._get_connection() as conn: + cursor = conn.cursor() + cursor.execute( + "SELECT spotify_data FROM wishlist_tracks WHERE spotify_track_id = ? AND profile_id = ?", + (track_id, profile_id)) + row = cursor.fetchone() + if not row or not row["spotify_data"]: + return {} + data = json.loads(row["spotify_data"]) + return data if isinstance(data, dict) else {} + except Exception as e: + logger.debug("get_wishlist_spotify_data failed: %s", e) + return {} + def get_wishlist_tracks(self, limit: Optional[int] = None, profile_id: int = 1, offset: int = 0, category: Optional[str] = None) -> List[Dict[str, Any]]: """Get tracks in the wishlist for the given profile, ordered by date added diff --git a/tests/wishlist/test_wishlist_ignore.py b/tests/wishlist/test_wishlist_ignore.py new file mode 100644 index 00000000..783511f2 --- /dev/null +++ b/tests/wishlist/test_wishlist_ignore.py @@ -0,0 +1,190 @@ +"""#874 — wishlist ignore-list: TTL skip-gate for user-removed/cancelled tracks. + +Two layers: + * pure logic (core.wishlist.ignore) — TTL, id-normalization, display extract + * DB seam (MusicDatabase on a temp db) — add/check/remove/list/clear, the + add_to_wishlist gate, the manual-add bypass, and the regression that the + success-cleanup removal path does NOT ignore. +""" + +from datetime import datetime, timedelta + +import pytest + +from core.wishlist.ignore import ( + IGNORE_TTL_DAYS, + REASON_CANCELLED, + REASON_REMOVED, + active_ignored_ids, + extract_display, + is_expired, + is_ignored, + normalize_ignore_id, +) +from database.music_database import MusicDatabase + + +# ── pure logic ────────────────────────────────────────────────────────── + +def test_normalize_strips_composite_album_suffix(): + assert normalize_ignore_id("track123::album456") == "track123" + assert normalize_ignore_id(" track123 ") == "track123" + assert normalize_ignore_id("") == "" + assert normalize_ignore_id(None) == "" + + +def test_is_expired_true_past_ttl_false_within(): + now = datetime(2026, 6, 15, 12, 0, 0) + fresh = (now - timedelta(days=5)).strftime("%Y-%m-%d %H:%M:%S") + stale = (now - timedelta(days=40)).strftime("%Y-%m-%d %H:%M:%S") + assert is_expired(fresh, now) is False + assert is_expired(stale, now) is True + + +def test_is_expired_unparseable_is_treated_expired_fail_open(): + # Corrupt timestamp must lapse (never wedge a track out of the wishlist). + assert is_expired("not-a-date", datetime(2026, 6, 15)) is True + assert is_expired("", datetime(2026, 6, 15)) is True + assert is_expired(None, datetime(2026, 6, 15)) is True + + +def test_is_ignored_matches_composite_and_bare_ids(): + now = datetime(2026, 6, 15, 12, 0, 0) + created = (now - timedelta(days=1)).strftime("%Y-%m-%d %H:%M:%S") + rows = [{"track_id": "abc", "created_at": created}] + # Stored bare, queried with composite (and vice-versa) — both match. + assert is_ignored(rows, "abc::album9", now) is True + rows2 = [{"track_id": "abc", "created_at": created}] + assert is_ignored(rows2, "abc", now) is True + assert is_ignored(rows2, "different", now) is False + + +def test_active_ignored_ids_drops_expired(): + now = datetime(2026, 6, 15, 12, 0, 0) + fresh = (now - timedelta(days=2)).strftime("%Y-%m-%d %H:%M:%S") + stale = (now - timedelta(days=99)).strftime("%Y-%m-%d %H:%M:%S") + rows = [ + {"track_id": "keep", "created_at": fresh}, + {"track_id": "drop", "created_at": stale}, + ] + assert active_ignored_ids(rows, now) == {"keep"} + + +def test_extract_display_handles_dict_and_string_artists(): + assert extract_display({"name": "Song", "artists": [{"name": "A"}]}) == ("Song", "A") + assert extract_display({"name": "Song", "artists": ["B"]}) == ("Song", "B") + assert extract_display({}) == ("", "") + assert extract_display(None) == ("", "") + + +# ── DB seam (temp database — never the live db) ───────────────────────── + +@pytest.fixture +def db(tmp_path): + return MusicDatabase(str(tmp_path / "m.db")) + + +def _track(track_id="t1", name="Some Song", artist="Some Artist", album_id="alb1"): + return { + "id": track_id, + "name": name, + "artists": [{"name": artist}], + "album": {"id": album_id, "name": "Some Album", "images": []}, + } + + +def test_add_check_remove_roundtrip(db): + assert db.is_track_ignored("t1") is False + assert db.add_to_wishlist_ignore("t1", "Song", "Artist", REASON_REMOVED) is True + assert db.is_track_ignored("t1") is True + # Composite id of the same base track is also considered ignored. + assert db.is_track_ignored("t1::albX") is True + assert db.remove_from_wishlist_ignore("t1") is True + assert db.is_track_ignored("t1") is False + + +def test_get_and_clear_ignore_list(db): + db.add_to_wishlist_ignore("a", "SongA", "ArtA", REASON_REMOVED) + db.add_to_wishlist_ignore("b", "SongB", "ArtB", REASON_CANCELLED) + entries = db.get_wishlist_ignore() + assert {e["track_id"] for e in entries} == {"a", "b"} + assert any(e["reason"] == "cancelled" for e in entries) + assert db.clear_wishlist_ignore() == 2 + assert db.get_wishlist_ignore() == [] + + +def test_expired_entry_is_not_active_and_gets_purged(db): + db.add_to_wishlist_ignore("old", "Old", "Art", REASON_REMOVED) + # Backdate it well past the TTL. + with db._get_connection() as conn: + conn.execute( + "UPDATE wishlist_ignore SET created_at = ? WHERE track_id = ?", + ((datetime.now() - timedelta(days=IGNORE_TTL_DAYS + 5)).strftime("%Y-%m-%d %H:%M:%S"), "old"), + ) + conn.commit() + assert db.is_track_ignored("old") is False # lapsed + assert db.get_wishlist_ignore() == [] # and purged on read + with db._get_connection() as conn: + remaining = conn.execute("SELECT COUNT(*) c FROM wishlist_ignore").fetchone()["c"] + assert remaining == 0 + + +# ── the gate: add_to_wishlist honours the ignore-list ─────────────────── + +def test_gate_blocks_auto_readd_but_manual_bypasses_and_clears(db): + track = _track("t1") + # Auto add works first time. + assert db.add_to_wishlist(track, source_type="playlist") is True + # User removes + ignores it. + db.remove_from_wishlist("t1") + db.add_to_wishlist_ignore("t1", "Some Song", "Some Artist", REASON_REMOVED) + # Auto re-add (watchlist / failed-capture / cancel) is now blocked. + assert db.add_to_wishlist(track, source_type="playlist") is False + assert db.is_track_ignored("t1") is True + # A MANUAL add bypasses the gate AND clears the ignore so it sticks. + assert db.add_to_wishlist(track, source_type="manual") is True + assert db.is_track_ignored("t1") is False + + +def test_gate_failopen_when_ignore_table_errors(db, monkeypatch): + # If the ignore check raises, the add must still succeed (never block). + monkeypatch.setattr(db, "is_track_ignored", lambda *a, **k: (_ for _ in ()).throw(RuntimeError("boom"))) + assert db.add_to_wishlist(_track("t9"), source_type="playlist") is True + + +def test_route_remove_track_records_ignore(db, monkeypatch): + # Pin the route → ignore wiring (not just the DB layer): a user-initiated + # remove via the route function must drop the row AND ignore the track. + import types + from core.wishlist import routes as routes_module + + db.add_to_wishlist(_track("rt1", name="Route Song", artist="Route Artist"), + source_type="playlist") + + class _Svc: + database = db + + def remove_track_from_wishlist(self, tid, profile_id=1): + return db.remove_from_wishlist(tid, profile_id=profile_id) + + monkeypatch.setattr(routes_module, "get_wishlist_service", lambda: _Svc()) + runtime = types.SimpleNamespace(profile_id=1, logger=routes_module.module_logger) + + payload, status = routes_module.remove_track_from_wishlist(runtime, "rt1") + assert status == 200 + assert db.is_track_ignored("rt1") is True + # The ignore carries the captured label. + entry = db.get_wishlist_ignore()[0] + assert entry["track_name"] == "Route Song" + assert entry["reason"] == REASON_REMOVED + + +def test_regression_success_cleanup_does_not_ignore(db): + # The post-download success path calls remove_from_wishlist directly — it + # must NOT add anything to the ignore-list (only user remove/cancel do). + track = _track("t5") + assert db.add_to_wishlist(track, source_type="playlist") is True + db.remove_from_wishlist("t5") # simulate success cleanup + assert db.is_track_ignored("t5") is False # NOT ignored + # And so a later legitimate auto-add still works. + assert db.add_to_wishlist(track, source_type="playlist") is True diff --git a/web_server.py b/web_server.py index 7cfd9387..a873ac7b 100644 --- a/web_server.py +++ b/web_server.py @@ -16819,6 +16819,45 @@ def remove_batch_from_wishlist(): logger.error(f"Error batch removing from wishlist: {e}") return jsonify({"success": False, "error": str(e)}), 500 +@app.route('/api/wishlist/ignore-list', methods=['GET']) +def get_wishlist_ignore_list(): + """#874: active (non-expired) wishlist ignore entries for this profile.""" + try: + runtime = _build_wishlist_route_runtime() + entries = get_wishlist_service().database.get_wishlist_ignore(profile_id=runtime.profile_id) + from core.wishlist.ignore import IGNORE_TTL_DAYS + return jsonify({"success": True, "entries": entries, "ttl_days": IGNORE_TTL_DAYS}) + except Exception as e: + logger.error(f"Error reading wishlist ignore-list: {e}") + return jsonify({"success": False, "error": str(e), "entries": []}), 500 + +@app.route('/api/wishlist/ignore-list/remove', methods=['POST']) +def remove_from_wishlist_ignore_list(): + """#874: un-ignore a track so it can be auto-acquired again.""" + try: + data = request.get_json() or {} + track_id = data.get('track_id') or data.get('spotify_track_id') + if not track_id: + return jsonify({"success": False, "error": "No track_id provided"}), 400 + runtime = _build_wishlist_route_runtime() + ok = get_wishlist_service().database.remove_from_wishlist_ignore( + track_id, profile_id=runtime.profile_id) + return jsonify({"success": True, "removed": ok}) + except Exception as e: + logger.error(f"Error removing from wishlist ignore-list: {e}") + return jsonify({"success": False, "error": str(e)}), 500 + +@app.route('/api/wishlist/ignore-list/clear', methods=['POST']) +def clear_wishlist_ignore_list(): + """#874: clear the entire wishlist ignore-list for this profile.""" + try: + runtime = _build_wishlist_route_runtime() + count = get_wishlist_service().database.clear_wishlist_ignore(profile_id=runtime.profile_id) + return jsonify({"success": True, "cleared": count}) + except Exception as e: + logger.error(f"Error clearing wishlist ignore-list: {e}") + return jsonify({"success": False, "error": str(e)}), 500 + @app.route('/api/add-album-to-wishlist', methods=['POST']) def add_album_track_to_wishlist(): """Endpoint to add a single track from an album to the wishlist.""" @@ -18956,26 +18995,44 @@ def _check_batch_completion_v2(batch_id): def _add_cancelled_task_to_wishlist(task): - """ - Helper function to add cancelled task to wishlist. - Separated for clarity and error isolation. + """Handle a user-cancelled download's wishlist state. + + #874: a manual cancel means "stop auto-retrying this release". The + previous behaviour re-added the cancelled track to the wishlist, which + the auto-processor then re-downloaded → re-cancelled → re-added, + forever. Instead we record a TTL'd ignore entry (so the watchlist / + auto-processor skip it) and drop it from the active wishlist. The user + can still force-download it manually (manual paths bypass the gate), + and the ignore lapses after core.wishlist.ignore.IGNORE_TTL_DAYS so it + is reconsidered later. Error-isolated; never raises into the caller. """ if not task: return - + try: from core.wishlist_service import get_wishlist_service + from core.wishlist.ignore import extract_display, REASON_CANCELLED wishlist_service = get_wishlist_service() - payload = _build_cancelled_task_wishlist_payload(task, profile_id=get_current_profile_id()) - success = wishlist_service.add_spotify_track_to_wishlist(**payload) - - if success: - logger.info(f"[Atomic Cancel] Added '{task.get('track_info', {}).get('name')}' to wishlist") - else: - logger.error(f"[Atomic Cancel] Failed to add '{task.get('track_info', {}).get('name')}' to wishlist") - + profile_id = get_current_profile_id() + track_info = task.get('track_info', {}) or {} + track_id = track_info.get('id') + name = track_info.get('name') + + if not track_id: + logger.warning("[Atomic Cancel] No track id on cancelled task — cannot ignore '%s'", name) + return + + disp_name, disp_artist = extract_display(track_info) + wishlist_service.database.add_to_wishlist_ignore( + track_id, track_name=disp_name, artist_name=disp_artist, + reason=REASON_CANCELLED, profile_id=profile_id) + # Drop it from the active wishlist so the auto-processor stops + # re-attempting it; the gate then blocks any automatic re-add. + wishlist_service.remove_track_from_wishlist(track_id, profile_id=profile_id) + logger.info("[Atomic Cancel] Ignored (TTL) + removed from wishlist: '%s'", name or track_id) + except Exception as e: - logger.error(f"[Atomic Cancel] Critical error adding to wishlist: {e}") + logger.error(f"[Atomic Cancel] Critical error handling cancelled task: {e}") @app.route('/api/playlists//cancel_batch', methods=['POST']) def cancel_batch(batch_id): diff --git a/webui/static/downloads.js b/webui/static/downloads.js index 26303e3b..fe322a78 100644 --- a/webui/static/downloads.js +++ b/webui/static/downloads.js @@ -1186,6 +1186,9 @@ async function openWishlistOverviewModal() { +
@@ -1312,6 +1315,117 @@ function closeWishlistOverviewModal() { console.log('✅ Modal closed'); } +// ── #874: Wishlist ignore-list ("Ignored") modal ──────────────────────── +// Tracks the user removed from the wishlist or cancelled mid-download are +// auto-skipped (not re-queued) until they expire. This modal lets the user +// see what's currently ignored and lift the skip (un-ignore / clear all). + +async function openWishlistIgnoreModal() { + let modal = document.getElementById('wishlist-ignore-modal'); + if (modal) modal.remove(); + modal = document.createElement('div'); + modal.id = 'wishlist-ignore-modal'; + modal.className = 'modal-overlay'; + modal.style.cssText = 'display:flex;position:fixed;inset:0;z-index:10050;align-items:center;justify-content:center;background:rgba(0,0,0,0.6);'; + modal.innerHTML = ` +
+
+

🚫 Ignored Tracks

+ +
+

Removed or cancelled tracks are skipped by auto-download until they expire. Un-ignore to allow auto-download again (you can always download manually).

+
+
Loading...
+
+
+
+ +
+
+
`; + document.body.appendChild(modal); + modal.addEventListener('click', (e) => { if (e.target === modal) closeWishlistIgnoreModal(); }); + await loadWishlistIgnoreList(); +} + +function closeWishlistIgnoreModal() { + const modal = document.getElementById('wishlist-ignore-modal'); + if (modal) modal.remove(); +} + +async function loadWishlistIgnoreList() { + const list = document.getElementById('wishlist-ignore-list'); + const clearBtn = document.getElementById('wishlist-ignore-clear-btn'); + if (!list) return; + try { + const resp = await fetch('/api/wishlist/ignore-list'); + const data = await resp.json(); + const entries = (data && data.entries) || []; + if (clearBtn) clearBtn.style.display = entries.length ? '' : 'none'; + if (!entries.length) { + list.innerHTML = '
🎉

Nothing ignored.
'; + return; + } + const ttl = (data && data.ttl_days) || 30; + list.innerHTML = entries.map(e => { + const title = escapeHtml(e.track_name || e.track_id || 'Unknown'); + const artist = escapeHtml(e.artist_name || ''); + const reason = e.reason === 'cancelled' ? 'Cancelled' : 'Removed'; + const tid = escapeHtml(String(e.track_id || '')); + return `
+
+
${title}
+
${artist}${artist ? ' · ' : ''}${reason} · skips ${ttl}d
+
+ +
`; + }).join(''); + } catch (err) { + list.innerHTML = '
Error loading ignored tracks
'; + } +} + +async function unignoreWishlistTrack(trackId) { + if (!trackId) return; + try { + const resp = await fetch('/api/wishlist/ignore-list/remove', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ track_id: trackId }), + }); + const data = await resp.json(); + if (data && data.success) { + showToast('Track un-ignored — it can be auto-downloaded again.', 'success'); + await loadWishlistIgnoreList(); + } else { + showToast(`Un-ignore failed: ${(data && data.error) || 'unknown'}`, 'error'); + } + } catch (err) { + showToast(`Un-ignore failed: ${err.message}`, 'error'); + } +} + +async function clearWishlistIgnoreList() { + if (!await showConfirmDialog({ + title: 'Clear Ignored List', + message: 'Allow all currently-ignored tracks to be auto-downloaded again?', + confirmText: 'Clear All', + cancelText: 'Cancel', + })) return; + try { + const resp = await fetch('/api/wishlist/ignore-list/clear', { method: 'POST' }); + const data = await resp.json(); + if (data && data.success) { + showToast(`Cleared ${data.cleared || 0} ignored track(s).`, 'success'); + await loadWishlistIgnoreList(); + } else { + showToast(`Clear failed: ${(data && data.error) || 'unknown'}`, 'error'); + } + } catch (err) { + showToast(`Clear failed: ${err.message}`, 'error'); + } +} + async function cleanupWishlistOverview() { console.log('🧹 cleanupWishlistOverview() called');