Blocklist Phase 2b: gate manual downloads with a "download anyway?" confirm
Closes the last acquisition gap — user-initiated downloads. A blocklist isn't
a censor, so search + discography stay fully visible; instead the download
ACTION is gated, visibly and overridably:
- Download modal (start-missing-process): an up-front check — if the WHOLE
album or artist being downloaded is blocklisted, return 409 {blocked:true}
with the entity, before starting a batch. The modal shows "X is blocklisted
— download anyway?" and re-POSTs with ignore_blocklist:true on confirm
(threaded onto the batch so the Phase 2a per-track filter skips it).
Scattered single-track bans still fall through to the 2a filter quietly.
- Manual /api/download (search-result download): source-file-centric, so it
matches the blocked ARTIST by name; same 409 + confirm + override. search.js
now sends artist/title so the guard has something to match.
- Precedence confirmed: force-download overrides "already owned", NOT a ban
(the 2a filter runs on the force-expanded missing list).
Frontend: shared confirmBlockedDownload() helper; modal + search callers
handle the blocked response and retry with the override.
Tests: manual download blocked-by-name / unrelated-allowed / override-passes,
and the modal up-front 409 for a blocked album. 8 blocklist API tests pass.
This commit is contained in:
BoulderBadgeDad
parent
45badf588c
commit
8ee59c7453
5 changed files with 174 additions and 24 deletions
|
|
@ -611,25 +611,30 @@ def run_full_missing_tracks_process(batch_id, playlist_id, tracks_json, deps: Ma
|
|||
# Blocklist (Phase 2a): drop banned artists/albums/tracks before queueing,
|
||||
# so a blocked item can't slip in via playlist sync / album download /
|
||||
# discography. Same ID-cascade brain as the wishlist guard (Phase 1) —
|
||||
# the only other auto-acquisition path. (Manual single-track downloads
|
||||
# are Phase 2b, intentionally not gated here.)
|
||||
try:
|
||||
_bl_before = len(missing_tracks)
|
||||
_bl_kept = []
|
||||
for res in missing_tracks:
|
||||
reason = db.blocklist_reason_for_track(
|
||||
batch_profile_id, res.get('track', {}), source=batch_source)
|
||||
if reason:
|
||||
logger.info("[Blocklist] Skipping %s '%s' from download queue (%s blocked)",
|
||||
reason[0], res.get('track', {}).get('name', '?'), reason[0])
|
||||
else:
|
||||
_bl_kept.append(res)
|
||||
if len(_bl_kept) != _bl_before:
|
||||
logger.info("[Blocklist] Filtered out %d blocklisted track(s) from download queue",
|
||||
_bl_before - len(_bl_kept))
|
||||
missing_tracks = _bl_kept
|
||||
except Exception as _bl_err:
|
||||
logger.debug("blocklist queue filter skipped: %s", _bl_err)
|
||||
# the only other auto-acquisition path. Skipped when the user confirmed
|
||||
# "download anyway" at the modal (Phase 2b override).
|
||||
_ignore_blocklist = False
|
||||
with tasks_lock:
|
||||
if batch_id in download_batches:
|
||||
_ignore_blocklist = download_batches[batch_id].get('ignore_blocklist', False)
|
||||
if not _ignore_blocklist:
|
||||
try:
|
||||
_bl_before = len(missing_tracks)
|
||||
_bl_kept = []
|
||||
for res in missing_tracks:
|
||||
reason = db.blocklist_reason_for_track(
|
||||
batch_profile_id, res.get('track', {}), source=batch_source)
|
||||
if reason:
|
||||
logger.info("[Blocklist] Skipping %s '%s' from download queue (%s blocked)",
|
||||
reason[0], res.get('track', {}).get('name', '?'), reason[0])
|
||||
else:
|
||||
_bl_kept.append(res)
|
||||
if len(_bl_kept) != _bl_before:
|
||||
logger.info("[Blocklist] Filtered out %d blocklisted track(s) from download queue",
|
||||
_bl_before - len(_bl_kept))
|
||||
missing_tracks = _bl_kept
|
||||
except Exception as _bl_err:
|
||||
logger.debug("blocklist queue filter skipped: %s", _bl_err)
|
||||
|
||||
with tasks_lock:
|
||||
if batch_id in download_batches:
|
||||
|
|
|
|||
|
|
@ -59,3 +59,56 @@ def test_add_requires_type_and_name(client):
|
|||
def test_invalid_entity_type_rejected(client):
|
||||
assert client.get("/api/blocklist?entity_type=bogus").status_code == 400
|
||||
assert client.get("/api/blocklist/search?type=bogus&q=x").status_code == 400
|
||||
|
||||
|
||||
# ── Phase 2b: download-time guards (manual + modal up-front) ─────────────────
|
||||
|
||||
@pytest.fixture()
|
||||
def banned_artist(client):
|
||||
"""Block 'Banned Guy' for the test profile; clean up after."""
|
||||
db = web_server.get_database()
|
||||
eid = db.add_blocklist_entry(1, "artist", "Banned Guy", spotify_id="bg-sp")
|
||||
yield
|
||||
db.remove_blocklist_entry(1, eid)
|
||||
|
||||
|
||||
def test_manual_download_blocked_by_artist_name(client, banned_artist):
|
||||
r = client.post("/api/download", json={
|
||||
"result_type": "track", "username": "peer", "filename": "x.flac",
|
||||
"artist": "Banned Guy", "title": "Song"})
|
||||
assert r.status_code == 409
|
||||
body = r.get_json()
|
||||
assert body["blocked"] is True and body["blocked_entity_type"] == "artist"
|
||||
|
||||
|
||||
def test_manual_download_unrelated_artist_not_blocked(client, banned_artist):
|
||||
# Allowed artist → guard passes (the download itself may fail offline; we
|
||||
# only assert it wasn't blocked by the blocklist).
|
||||
r = client.post("/api/download", json={
|
||||
"result_type": "track", "username": "peer", "filename": "y.flac",
|
||||
"artist": "Allowed Artist", "title": "Song"})
|
||||
assert not (r.get_json() or {}).get("blocked")
|
||||
|
||||
|
||||
def test_manual_download_override_passes_guard(client, banned_artist):
|
||||
r = client.post("/api/download", json={
|
||||
"result_type": "track", "username": "peer", "filename": "x.flac",
|
||||
"artist": "Banned Guy", "title": "Song", "ignore_blocklist": True})
|
||||
assert not (r.get_json() or {}).get("blocked") # override skips the guard
|
||||
|
||||
|
||||
def test_modal_blocked_album_returns_409_before_starting(client):
|
||||
db = web_server.get_database()
|
||||
eid = db.add_blocklist_entry(1, "album", "Banned Album", spotify_id="ba-sp")
|
||||
try:
|
||||
r = client.post("/api/playlists/artist_album_test/start-missing-process", json={
|
||||
"tracks": [{"id": "t1", "name": "Track"}],
|
||||
"is_album_download": True,
|
||||
"album_context": {"id": "ba-sp", "name": "Banned Album"},
|
||||
"artist_context": {"id": "ar1", "name": "Someone"},
|
||||
})
|
||||
assert r.status_code == 409
|
||||
body = r.get_json()
|
||||
assert body["blocked"] is True and body["blocked_entity_type"] == "album"
|
||||
finally:
|
||||
db.remove_blocklist_entry(1, eid)
|
||||
|
|
|
|||
|
|
@ -6152,6 +6152,25 @@ def start_download():
|
|||
if not username or not filename:
|
||||
return jsonify({"error": "Missing username or filename."}), 400
|
||||
|
||||
# Blocklist guard (Phase 2b): a manual download is source-file-centric
|
||||
# (no metadata IDs), so this matches the blocked ARTIST by name. The
|
||||
# frontend shows "blocked — download anyway?" and re-POSTs with
|
||||
# ignore_blocklist=true on confirm.
|
||||
if not data.get('ignore_blocklist'):
|
||||
try:
|
||||
_dl_artist = data.get('artist')
|
||||
if _dl_artist and _dl_artist != 'Unknown':
|
||||
_reason = get_database().blocklist_reason_for_track(
|
||||
get_current_profile_id(),
|
||||
{'name': data.get('title'), 'artists': [{'name': _dl_artist}]})
|
||||
if _reason:
|
||||
return jsonify({
|
||||
"success": False, "blocked": True,
|
||||
"blocked_entity_type": _reason[0], "blocked_name": _reason[1],
|
||||
}), 409
|
||||
except Exception as _bl_err:
|
||||
logger.debug("manual download blocklist check skipped: %s", _bl_err)
|
||||
|
||||
download_id = run_async(download_orchestrator.download(username, filename, file_size))
|
||||
logger.info(f"Download ID returned: {download_id}")
|
||||
|
||||
|
|
@ -19147,10 +19166,37 @@ def start_missing_tracks_process(playlist_id):
|
|||
# album-download modal. Stored on the batch and propagated per-track by
|
||||
# the master worker so AcoustID never quarantines this request's files.
|
||||
skip_acoustid = bool(data.get('skip_acoustid', False))
|
||||
# Blocklist override (Phase 2b): set by the modal's "download anyway" confirm.
|
||||
ignore_blocklist = bool(data.get('ignore_blocklist', False))
|
||||
|
||||
if not tracks:
|
||||
return jsonify({"success": False, "error": "No tracks provided"}), 400
|
||||
|
||||
# Blocklist up-front check (Phase 2b): if the WHOLE album or artist being
|
||||
# downloaded is blocklisted, stop here with a clear, actionable response
|
||||
# instead of silently dropping every track in the per-track filter (2a) and
|
||||
# leaving an empty batch. Scattered single-track bans still fall through to
|
||||
# the per-track filter. The modal re-POSTs with ignore_blocklist=true after
|
||||
# the user confirms "download anyway".
|
||||
if not ignore_blocklist and (album_context or artist_context):
|
||||
try:
|
||||
_bsrc = _downloads_history.detect_sync_source(playlist_id)
|
||||
_synthetic = {
|
||||
'album': {'id': (album_context or {}).get('id'),
|
||||
'name': (album_context or {}).get('name')},
|
||||
'artists': [{'id': (artist_context or {}).get('id'),
|
||||
'name': (artist_context or {}).get('name')}],
|
||||
}
|
||||
_reason = get_database().blocklist_reason_for_track(
|
||||
get_current_profile_id(), _synthetic, source=_bsrc)
|
||||
if _reason:
|
||||
return jsonify({
|
||||
"success": False, "blocked": True,
|
||||
"blocked_entity_type": _reason[0], "blocked_name": _reason[1],
|
||||
}), 409
|
||||
except Exception as _bl_err:
|
||||
logger.debug("blocklist up-front check skipped: %s", _bl_err)
|
||||
|
||||
# Log album context if provided
|
||||
if is_album_download and album_context and artist_context:
|
||||
logger.info(f"[Artist Album] Received album context: '{album_context.get('name')}' by '{artist_context.get('name')}' ({album_context.get('album_type', 'album')})")
|
||||
|
|
@ -19208,6 +19254,9 @@ def start_missing_tracks_process(playlist_id):
|
|||
'analysis_results': [],
|
||||
'force_download_all': force_download_all, # Pass the force flag to the batch
|
||||
'ignore_manual_matches': ignore_manual_matches,
|
||||
# Blocklist override (Phase 2b) — the user confirmed "download anyway"
|
||||
# at the modal, so the per-track filter (2a) skips this batch.
|
||||
'ignore_blocklist': ignore_blocklist,
|
||||
'playlist_folder_mode': playlist_folder_mode, # Organize downloads by playlist folder
|
||||
# Album context for artist album downloads (explicit folder structure)
|
||||
'is_album_download': is_album_download,
|
||||
|
|
|
|||
|
|
@ -1,6 +1,15 @@
|
|||
// WING IT — Download without metadata discovery
|
||||
// ==================================================================================
|
||||
|
||||
// Blocklist (Phase 2b): when a download is refused because the artist/album/track
|
||||
// is on the blocklist, the backend returns {blocked:true,...}. Ask the user
|
||||
// whether to override; callers re-POST with ignore_blocklist:true on confirm.
|
||||
function confirmBlockedDownload(data) {
|
||||
const what = data.blocked_entity_type || 'item';
|
||||
const name = data.blocked_name || 'this item';
|
||||
return confirm(`"${name}" is on your blocklist (${what} blocked).\n\nDownload anyway?`);
|
||||
}
|
||||
|
||||
function _toggleWingItDropdown(btn, urlHash) {
|
||||
// Remove any existing dropdown
|
||||
const existing = document.querySelector('.wing-it-dropdown.visible');
|
||||
|
|
@ -2524,13 +2533,27 @@ async function startMissingTracksProcess(playlistId) {
|
|||
}
|
||||
}
|
||||
|
||||
const response = await fetch(`/api/playlists/${playlistId}/start-missing-process`, {
|
||||
let response = await fetch(`/api/playlists/${playlistId}/start-missing-process`, {
|
||||
method: 'POST',
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify(requestBody)
|
||||
});
|
||||
|
||||
const data = await response.json();
|
||||
let data = await response.json();
|
||||
// Blocklist (Phase 2b): whole album/artist is blocked → confirm override.
|
||||
if (data.blocked) {
|
||||
if (!confirmBlockedDownload(data)) {
|
||||
showToast(`Skipped — ${data.blocked_name} is blocklisted`, 'info');
|
||||
return;
|
||||
}
|
||||
requestBody.ignore_blocklist = true;
|
||||
response = await fetch(`/api/playlists/${playlistId}/start-missing-process`, {
|
||||
method: 'POST',
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify(requestBody)
|
||||
});
|
||||
data = await response.json();
|
||||
}
|
||||
if (!data.success) {
|
||||
// Special handling for rate limit
|
||||
if (response.status === 429) {
|
||||
|
|
|
|||
|
|
@ -1059,15 +1059,35 @@ function initializeSearchModeToggle() {
|
|||
try {
|
||||
const downloadData = type === 'album'
|
||||
? { result_type: 'album', tracks: result.tracks || [] }
|
||||
: { result_type: 'track', username: result.username, filename: result.filename, size: result.size };
|
||||
: { result_type: 'track', username: result.username, filename: result.filename,
|
||||
size: result.size,
|
||||
// Carry artist/title so the blocklist guard can match (Phase 2b).
|
||||
artist: result.artist || result.artist_name || '',
|
||||
title: result.title || result.name || '' };
|
||||
|
||||
const response = await fetch('/api/download', {
|
||||
let response = await fetch('/api/download', {
|
||||
method: 'POST',
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify(downloadData)
|
||||
});
|
||||
let data = await response.json();
|
||||
|
||||
const data = await response.json();
|
||||
// Blocklist (Phase 2b): blocked artist → confirm override + retry.
|
||||
if (data.blocked) {
|
||||
if (typeof confirmBlockedDownload === 'function' && confirmBlockedDownload(data)) {
|
||||
downloadData.ignore_blocklist = true;
|
||||
response = await fetch('/api/download', {
|
||||
method: 'POST', headers: { 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify(downloadData)
|
||||
});
|
||||
data = await response.json();
|
||||
} else {
|
||||
showToast(`Skipped — ${data.blocked_name} is blocklisted`, 'info');
|
||||
this.disabled = false;
|
||||
this.innerHTML = '💾 Download';
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
if (data.error) {
|
||||
showToast(`Download error: ${data.error}`, 'error');
|
||||
|
|
|
|||
Loading…
Reference in a new issue