#897: surface the ignore-list on the wishlist page + stop blocking manual re-adds
Two issues behind #897: 1) Discoverability — the "Ignored" management modal (view/un-ignore/clear-all, shipped with #874) was only reachable from the wishlist *overview modal* footer, which most users never open. Add the same button to the wishlist page toolbar next to Cleanup / Clear All, wired to openWishlistIgnoreModal(). 2) Manual re-add silently blocked (carlosjfcasero) — the album-modal "add to wishlist" endpoint passes source_type=album, but the ignore gate only bypasses+clears for source_type=manual, so re-adding a previously-cancelled track failed. We cannot just send manual: source_type drives Albums/Singles categorisation and repair_worker legitimately uses album too. Thread an explicit user_initiated flag (db.add_to_wishlist -> service -> album route) that bypasses+clears the ignore while preserving the real source_type. Regression test pins both: an automatic source_type=album add stays blocked, the user_initiated add goes through, clears the ignore, and keeps source_type=album.
This commit is contained in:
parent
1c93a5640d
commit
15ea87a154
5 changed files with 41 additions and 2 deletions
|
|
@ -501,6 +501,10 @@ def add_album_track_to_wishlist(
|
||||||
source_type=source_type,
|
source_type=source_type,
|
||||||
source_context=enhanced_source_context,
|
source_context=enhanced_source_context,
|
||||||
profile_id=runtime.profile_id,
|
profile_id=runtime.profile_id,
|
||||||
|
# Explicit user click in the album modal — must bypass + clear the
|
||||||
|
# ignore-list, even if the user previously cancelled this track
|
||||||
|
# (otherwise the add is silently dropped — carlosjfcasero, #897).
|
||||||
|
user_initiated=True,
|
||||||
)
|
)
|
||||||
|
|
||||||
if success:
|
if success:
|
||||||
|
|
|
||||||
|
|
@ -100,6 +100,7 @@ class WishlistService:
|
||||||
source_type: str = "manual",
|
source_type: str = "manual",
|
||||||
source_context: Dict[str, Any] = None,
|
source_context: Dict[str, Any] = None,
|
||||||
profile_id: int = 1,
|
profile_id: int = 1,
|
||||||
|
user_initiated: bool = False,
|
||||||
) -> bool:
|
) -> bool:
|
||||||
"""
|
"""
|
||||||
Directly add a track to the wishlist.
|
Directly add a track to the wishlist.
|
||||||
|
|
@ -110,6 +111,8 @@ class WishlistService:
|
||||||
source_type: Source type ('playlist', 'album', 'manual')
|
source_type: Source type ('playlist', 'album', 'manual')
|
||||||
source_context: Additional context information
|
source_context: Additional context information
|
||||||
profile_id: Profile to add to
|
profile_id: Profile to add to
|
||||||
|
user_initiated: True for an explicit user add — bypasses + clears the
|
||||||
|
ignore-list while keeping the real source_type (#874/#897).
|
||||||
"""
|
"""
|
||||||
if track_data is None:
|
if track_data is None:
|
||||||
track_data = spotify_track_data
|
track_data = spotify_track_data
|
||||||
|
|
@ -124,6 +127,7 @@ class WishlistService:
|
||||||
source_type=source_type,
|
source_type=source_type,
|
||||||
source_info=source_context or {},
|
source_info=source_context or {},
|
||||||
profile_id=profile_id,
|
profile_id=profile_id,
|
||||||
|
user_initiated=user_initiated,
|
||||||
)
|
)
|
||||||
|
|
||||||
def add_spotify_track_to_wishlist(
|
def add_spotify_track_to_wishlist(
|
||||||
|
|
|
||||||
|
|
@ -8925,8 +8925,15 @@ class MusicDatabase:
|
||||||
source_info: Dict[str, Any] = None,
|
source_info: Dict[str, Any] = None,
|
||||||
profile_id: int = 1,
|
profile_id: int = 1,
|
||||||
track_data: Dict[str, Any] = None,
|
track_data: Dict[str, Any] = None,
|
||||||
|
user_initiated: bool = False,
|
||||||
) -> bool:
|
) -> bool:
|
||||||
"""Add a failed track to the wishlist for retry"""
|
"""Add a failed track to the wishlist for retry.
|
||||||
|
|
||||||
|
``user_initiated`` marks an explicit user add (e.g. the library album
|
||||||
|
"add to wishlist" modal). Like ``source_type == 'manual'`` it bypasses
|
||||||
|
the ignore-list gate AND clears any stale ignore — but unlike changing
|
||||||
|
``source_type`` it preserves the real provenance ('album'), which the
|
||||||
|
wishlist categorisation (Albums vs Singles) relies on (#874/#897)."""
|
||||||
try:
|
try:
|
||||||
if track_data is not None and spotify_track_data is None:
|
if track_data is not None and spotify_track_data is None:
|
||||||
spotify_track_data = track_data
|
spotify_track_data = track_data
|
||||||
|
|
@ -8956,7 +8963,7 @@ class MusicDatabase:
|
||||||
# clear any stale ignore so it sticks. Fail-open: any error here
|
# clear any stale ignore so it sticks. Fail-open: any error here
|
||||||
# must never block a legitimate wishlist add.
|
# must never block a legitimate wishlist add.
|
||||||
try:
|
try:
|
||||||
if source_type == 'manual':
|
if source_type == 'manual' or user_initiated:
|
||||||
self.remove_from_wishlist_ignore(track_id, profile_id=profile_id)
|
self.remove_from_wishlist_ignore(track_id, profile_id=profile_id)
|
||||||
elif self.is_track_ignored(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)
|
logger.info("Skipping wishlist add — track is on the ignore-list (#874): %s", track_id)
|
||||||
|
|
|
||||||
|
|
@ -146,6 +146,26 @@ def test_gate_blocks_auto_readd_but_manual_bypasses_and_clears(db):
|
||||||
assert db.is_track_ignored("t1") is False
|
assert db.is_track_ignored("t1") is False
|
||||||
|
|
||||||
|
|
||||||
|
def test_user_initiated_add_bypasses_and_clears_keeping_source_type(db):
|
||||||
|
# #897 / carlosjfcasero: a user manually adds an album track they had
|
||||||
|
# previously cancelled. It must bypass the gate AND clear the ignore — but
|
||||||
|
# WITHOUT pretending to be source_type='manual' (the album modal sends
|
||||||
|
# source_type='album', which the Albums/Singles categorisation relies on,
|
||||||
|
# and which an automatic path like repair_worker also legitimately uses).
|
||||||
|
track = _track("t7")
|
||||||
|
db.add_to_wishlist_ignore("t7", "Owned Song", "Owned Artist", REASON_CANCELLED)
|
||||||
|
# An automatic 'album' add (e.g. repair_worker) is still correctly blocked.
|
||||||
|
assert db.add_to_wishlist(track, source_type="album") is False
|
||||||
|
assert db.is_track_ignored("t7") is True
|
||||||
|
# The explicit user click (user_initiated) goes through and clears the ignore,
|
||||||
|
# while the stored source_type stays 'album'.
|
||||||
|
assert db.add_to_wishlist(track, source_type="album", user_initiated=True) is True
|
||||||
|
assert db.is_track_ignored("t7") is False
|
||||||
|
# Provenance preserved: the stored row is still source_type='album', NOT 'manual'.
|
||||||
|
row = next(r for r in db.get_wishlist_tracks() if str(r.get("spotify_track_id")) == "t7")
|
||||||
|
assert row.get("source_type") == "album"
|
||||||
|
|
||||||
|
|
||||||
def test_gate_failopen_when_ignore_table_errors(db, monkeypatch):
|
def test_gate_failopen_when_ignore_table_errors(db, monkeypatch):
|
||||||
# If the ignore check raises, the add must still succeed (never block).
|
# 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")))
|
monkeypatch.setattr(db, "is_track_ignored", lambda *a, **k: (_ for _ in ()).throw(RuntimeError("boom")))
|
||||||
|
|
|
||||||
|
|
@ -7320,6 +7320,10 @@
|
||||||
</div>
|
</div>
|
||||||
</div>
|
</div>
|
||||||
<div class="wishlist-page-header-right">
|
<div class="wishlist-page-header-right">
|
||||||
|
<button class="btn btn--secondary" onclick="openWishlistIgnoreModal()" title="Tracks you removed or cancelled — auto-skipped until they expire. Un-ignore to allow auto-download again.">
|
||||||
|
<svg width="14" height="14" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round"><circle cx="12" cy="12" r="10"/><line x1="4.93" y1="4.93" x2="19.07" y2="19.07"/></svg>
|
||||||
|
Ignored
|
||||||
|
</button>
|
||||||
<button class="btn btn--secondary" onclick="cleanupWishlistOverview()">
|
<button class="btn btn--secondary" onclick="cleanupWishlistOverview()">
|
||||||
<svg width="14" height="14" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round"><path d="M3 6h18"/><path d="M8 6V4h8v2"/></svg>
|
<svg width="14" height="14" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round"><path d="M3 6h18"/><path d="M8 6V4h8v2"/></svg>
|
||||||
Cleanup
|
Cleanup
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue