#874: wishlist ignore-list — stop auto-retrying removed/cancelled tracks
A user who removes a wishlist track, or cancels an in-flight wishlist download, would have it re-added on the next auto cycle (watchlist scan, failed-track capture, or the cancel handler's own re-add), so the same release downloaded -> failed/cancelled -> re-queued forever. Adds a TTL'd skip-gate (30 days), softer than the blocklist: it expires so the track is reconsidered later, and never blocks a manual force-download — only the automatic re-queue. - core/wishlist/ignore.py: pure TTL/normalization/display logic + a best-effort orchestrator (no DB handle, caller passes now). - database/music_database.py: migration-safe wishlist_ignore table + add/check/remove/list(+purge)/clear methods, and the gate in add_to_wishlist beside the blocklist guard. Fail-open throughout — an ignore error can never block a legitimate add; a manual add bypasses the gate AND clears the ignore. - routes.py: user remove (single/album/batch) records an ignore. Hooked at the route layer, NOT the DB remove, so success-cleanup never ignores (regression-tested). - web_server.py: cancel now ignores + removes from the wishlist instead of re-adding for endless retry; three /api/wishlist/ignore-list* endpoints. - downloads.js: 'Ignored' modal (view / un-ignore / clear all). - 13 tests: pure logic, DB seam, gate (block/bypass/fail-open), route wiring, and the success-cleanup-does-not-ignore regression.
This commit is contained in:
BoulderBadgeDad
parent
46be97b195
commit
48e86a1a58
6 changed files with 755 additions and 19 deletions
162
core/wishlist/ignore.py
Normal file
162
core/wishlist/ignore.py
Normal file
|
|
@ -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 ``<track_id>::<album_id>``
|
||||
(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
|
||||
|
|
@ -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 {
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
190
tests/wishlist/test_wishlist_ignore.py
Normal file
190
tests/wishlist/test_wishlist_ignore.py
Normal file
|
|
@ -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
|
||||
|
|
@ -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/<batch_id>/cancel_batch', methods=['POST'])
|
||||
def cancel_batch(batch_id):
|
||||
|
|
|
|||
|
|
@ -1186,6 +1186,9 @@ async function openWishlistOverviewModal() {
|
|||
<button class="playlist-modal-btn playlist-modal-btn-warning" onclick="cleanupWishlistOverview()">
|
||||
🧹 Cleanup Wishlist
|
||||
</button>
|
||||
<button class="playlist-modal-btn playlist-modal-btn-secondary" onclick="openWishlistIgnoreModal()" title="Tracks you removed or cancelled — auto-skipped until they expire">
|
||||
🚫 Ignored
|
||||
</button>
|
||||
</div>
|
||||
<div class="playlist-modal-footer-right">
|
||||
<button class="playlist-modal-btn playlist-modal-btn-secondary" onclick="closeWishlistOverviewModal()">Close</button>
|
||||
|
|
@ -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 = `
|
||||
<div class="playlist-modal-content" style="max-width:560px;width:90%;max-height:80vh;display:flex;flex-direction:column;">
|
||||
<div class="playlist-modal-header">
|
||||
<h2 style="margin:0;">🚫 Ignored Tracks</h2>
|
||||
<button class="playlist-modal-btn playlist-modal-btn-secondary" onclick="closeWishlistIgnoreModal()">Close</button>
|
||||
</div>
|
||||
<p style="opacity:0.7;font-size:13px;margin:8px 16px 0;">Removed or cancelled tracks are skipped by auto-download until they expire. Un-ignore to allow auto-download again (you can always download manually).</p>
|
||||
<div id="wishlist-ignore-list" class="playlist-tracks-scroll" style="flex:1;overflow-y:auto;padding:12px 16px;">
|
||||
<div class="loading-indicator">Loading...</div>
|
||||
</div>
|
||||
<div class="playlist-modal-footer">
|
||||
<div class="playlist-modal-footer-left">
|
||||
<button id="wishlist-ignore-clear-btn" class="playlist-modal-btn playlist-modal-btn-danger" onclick="clearWishlistIgnoreList()" style="display:none;">Clear All</button>
|
||||
</div>
|
||||
</div>
|
||||
</div>`;
|
||||
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 = '<div class="playlist-empty-state" style="text-align:center;opacity:0.6;padding:30px 0;">🎉<br><br>Nothing ignored.</div>';
|
||||
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 `<div class="wishlist-ignore-row" style="display:flex;align-items:center;gap:10px;padding:8px 4px;border-bottom:1px solid rgba(255,255,255,0.06);">
|
||||
<div style="flex:1;min-width:0;">
|
||||
<div style="font-weight:600;white-space:nowrap;overflow:hidden;text-overflow:ellipsis;">${title}</div>
|
||||
<div style="opacity:0.6;font-size:12px;white-space:nowrap;overflow:hidden;text-overflow:ellipsis;">${artist}${artist ? ' · ' : ''}${reason} · skips ${ttl}d</div>
|
||||
</div>
|
||||
<button class="playlist-modal-btn playlist-modal-btn-secondary" style="flex-shrink:0;" data-track-id="${tid}" onclick="unignoreWishlistTrack(this.dataset.trackId)">Un-ignore</button>
|
||||
</div>`;
|
||||
}).join('');
|
||||
} catch (err) {
|
||||
list.innerHTML = '<div class="playlist-empty-state" style="text-align:center;opacity:0.6;padding:30px 0;">Error loading ignored tracks</div>';
|
||||
}
|
||||
}
|
||||
|
||||
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');
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue