Skip to content

Commit 83a1614

Browse files
dprodgerclaude
andcommitted
Add Reject button: blocklist + delete a wrong Spotify match
Sits next to the Verify button on /admin/duration-mismatches/<song>. Confirms first, then in a single transaction: 1. Inserts into bad_streaming_matches at block_level='track' so get_blocked_tracks_for_song will return this Spotify track on the next match attempt — the matcher already consults this list at the top of match_track_to_recording and skips blocked candidates. The block survives every rematch / refresh / sweep cycle. 2. DELETEs the recording_release_streaming_links row. Idempotent: the unique constraint on bad_streaming_matches collapses duplicate inserts; a second reject on the same link 404s on the already-deleted row. Concrete case from production: a Spotify track linked to the wrong album entirely (different artists, different songs). Plain delete without blocking would let the next rematch re-find and re-link the same wrong track. The block tells the matcher "this Spotify track is never a match for this song," permanently. Plumbing: - New helper integrations.spotify.db.block_streaming_track(conn, song_id, track_id, ...) — inserts the blocklist row idempotently. Uses column-list ON CONFLICT (not constraint-name) so test environments bootstrapped from sql/jazz-db-schema.sql work without depending on the named-constraint form in the migration file. - New admin endpoint POST /admin/duration-mismatches/links/<id>/reject with optional body {"reason": "..."} captured into bad_streaming_matches.reason for human review. 404s for unknown link. Goes through the existing admin gate + CSRF double-submit. - Reject button on the Status column with a destructive confirm dialog ("This is permanent until you remove the blocklist entry manually"). Only shown on unverified rows — verified rows have already been intentionally accepted, so a reject affordance there would be confusing. Tests: 7 new in test_admin_verify_match.py covering the db helper's insert + idempotency, and the endpoint's auth gate, success path, reason persistence, 404, and idempotency. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 7fcde90 commit 83a1614

4 files changed

Lines changed: 363 additions & 0 deletions

File tree

backend/integrations/spotify/db.py

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -668,6 +668,57 @@ def set_track_link_manual_override(conn, streaming_link_id: str,
668668
return True
669669

670670

671+
def block_streaming_track(conn, song_id: str, track_id: str,
672+
service: str = 'spotify',
673+
reason: str = None,
674+
log: logging.Logger = None) -> bool:
675+
"""Add an entry to bad_streaming_matches at block_level='track'.
676+
677+
Tells the matcher to never re-link this Spotify track to recordings
678+
of `song_id`. The matcher consults this list via
679+
get_blocked_tracks_for_song on every track-match attempt, so the
680+
block survives all subsequent rematch / refresh / sweep cycles.
681+
682+
Used by the admin "Reject" action — call this in the same transaction
683+
as the streaming-link delete so a wrong link gets both removed AND
684+
blocklisted in one shot.
685+
686+
Returns True when a new row was inserted, False when an equivalent
687+
block already existed (the unique constraint
688+
bad_streaming_matches_unique collapses duplicates).
689+
"""
690+
log = log or logger
691+
with conn.cursor() as cur:
692+
# Column-list ON CONFLICT instead of constraint-name ON CONFLICT —
693+
# the constraint is named in the migration file but anonymous in
694+
# sql/jazz-db-schema.sql, so test environments bootstrapped from
695+
# the schema file don't have the named constraint to reference.
696+
# The column list matches both definitions.
697+
cur.execute(
698+
"""
699+
INSERT INTO bad_streaming_matches
700+
(service, block_level, service_id, song_id, reason)
701+
VALUES (%s, 'track', %s, %s, %s)
702+
ON CONFLICT (service, block_level, service_id, song_id)
703+
DO NOTHING
704+
RETURNING id
705+
""",
706+
(service, track_id, song_id, reason),
707+
)
708+
new_row = cur.fetchone()
709+
if new_row:
710+
log.info(
711+
"Blocked %s track %s for song %s (reason=%r)",
712+
service, track_id, song_id, reason,
713+
)
714+
return True
715+
log.debug(
716+
"%s track %s already blocked for song %s — no-op",
717+
service, track_id, song_id,
718+
)
719+
return False
720+
721+
671722
def is_album_manual_override(conn, release_id: str, service: str = 'spotify') -> bool:
672723
"""
673724
Check if an existing album link is a manual override that should be preserved.

backend/routes/admin.py

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
from integrations.musicbrainz.performer_importer import PerformerImporter
2626
from integrations.musicbrainz.utils import MusicBrainzSearcher
2727
from integrations.spotify.db import (
28+
block_streaming_track,
2829
is_track_manual_override,
2930
set_track_link_manual_override,
3031
)
@@ -3041,6 +3042,86 @@ def duration_mismatches_verify_link(link_id):
30413042
})
30423043

30433044

3045+
@admin_bp.route('/duration-mismatches/links/<link_id>/reject', methods=['POST'])
3046+
def duration_mismatches_reject_link(link_id):
3047+
"""Block + delete a wrong Spotify streaming link.
3048+
3049+
Two-step operation in a single transaction:
3050+
3051+
1. Insert into bad_streaming_matches at block_level='track', so the
3052+
matcher's get_blocked_tracks_for_song lookup will skip this
3053+
(song_id, spotify_track_id) pair on every future match attempt.
3054+
2. Delete the streaming link itself. The page reloads to confirm.
3055+
3056+
Idempotent against repeated invocations: the bad_streaming_matches
3057+
unique constraint collapses duplicate blocks, and the DELETE
3058+
no-ops on a missing link.
3059+
3060+
Body: optional JSON {"reason": "..."} captured into bad_streaming_matches.reason
3061+
for human review. The default is "rejected via admin UI".
3062+
"""
3063+
data = request.get_json(silent=True) or {}
3064+
reason = (data.get('reason') or 'rejected via admin UI').strip()
3065+
3066+
try:
3067+
with get_db_connection() as db:
3068+
with db.cursor() as cur:
3069+
# Pull (song_id, service_id) from the link we're about to nuke.
3070+
# Going through recording_releases → recordings → songs.id is
3071+
# the only reachable path; rrsl itself has no song_id.
3072+
cur.execute(
3073+
"""
3074+
SELECT rec.song_id AS song_id, rrsl.service_id AS spotify_track_id
3075+
FROM recording_release_streaming_links rrsl
3076+
JOIN recording_releases rr ON rr.id = rrsl.recording_release_id
3077+
JOIN recordings rec ON rec.id = rr.recording_id
3078+
WHERE rrsl.id = %s
3079+
AND rrsl.service = 'spotify'
3080+
""",
3081+
(link_id,),
3082+
)
3083+
row = cur.fetchone()
3084+
if not row:
3085+
return jsonify({'error': 'Streaming link not found'}), 404
3086+
song_id = str(row['song_id'])
3087+
spotify_track_id = row['spotify_track_id']
3088+
3089+
# Step 1 — block the (song, track) pair so the matcher skips it.
3090+
# Skipped silently if there's no service_id to block (defensive;
3091+
# in practice every spotify link has one).
3092+
if spotify_track_id:
3093+
block_streaming_track(
3094+
db, song_id, spotify_track_id,
3095+
service='spotify', reason=reason, log=logger,
3096+
)
3097+
3098+
# Step 2 — delete the link itself.
3099+
with db.cursor() as cur:
3100+
cur.execute(
3101+
"""
3102+
DELETE FROM recording_release_streaming_links
3103+
WHERE id = %s AND service = 'spotify'
3104+
RETURNING id
3105+
""",
3106+
(link_id,),
3107+
)
3108+
deleted = cur.fetchone()
3109+
3110+
db.commit()
3111+
except Exception as e:
3112+
logger.error(f"Error rejecting link {link_id}: {e}")
3113+
return jsonify({'error': str(e)}), 500
3114+
3115+
return jsonify({
3116+
'success': True,
3117+
'link_id': link_id,
3118+
'song_id': song_id,
3119+
'spotify_track_id': spotify_track_id,
3120+
'deleted': bool(deleted),
3121+
'blocked': bool(spotify_track_id),
3122+
})
3123+
3124+
30443125
@admin_bp.route('/users')
30453126
def users_list():
30463127
"""List user accounts with email search and pagination."""

backend/templates/admin/duration_mismatches_review.html

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -329,6 +329,29 @@
329329
cursor: wait;
330330
}
331331

332+
/* Destructive sibling to the Verify button — deletes the link AND
333+
writes to bad_streaming_matches so the matcher won't re-link.
334+
Uses a deliberately distinct red so admins don't fat-finger it. */
335+
.reject-btn {
336+
font-family: inherit;
337+
font-size: 11px;
338+
padding: 3px 10px;
339+
margin-left: 4px;
340+
border: 1px solid #b73d3d;
341+
background: #fff;
342+
color: #b73d3d;
343+
border-radius: 4px;
344+
cursor: pointer;
345+
line-height: 1.3;
346+
}
347+
.reject-btn:hover {
348+
background: #fbeaea;
349+
}
350+
.reject-btn:disabled {
351+
opacity: 0.5;
352+
cursor: wait;
353+
}
354+
332355
/* When the row is verified-and-shown (?include_verified=1), tone
333356
it down so it visually recedes from the still-actionable rows. */
334357
tr.verified-row {
@@ -614,6 +637,10 @@ <h1>Duration Mismatches</h1>
614637
data-action="verify"
615638
onclick="toggleVerify(this)"
616639
title="Mark this match as manually verified — matcher will leave it alone on rematch">Verify</button>
640+
<button type="button" class="reject-btn"
641+
data-link-id="{{ link.streaming_link_id }}"
642+
onclick="rejectLink(this)"
643+
title="Wrong match — delete this link AND blocklist this Spotify track for this song so the matcher never re-links it">Reject</button>
617644
{% endif %}
618645
</td>
619646
</tr>
@@ -687,6 +714,50 @@ <h2>No mismatches found</h2>
687714
}
688715
}
689716

717+
async function rejectLink(btn) {
718+
// Destructive: delete the link AND blocklist the Spotify track
719+
// for this song. Confirm before firing because there's no
720+
// single-click undo (admin can manually remove the
721+
// bad_streaming_matches row, but that's a DB op).
722+
if (!confirm(
723+
'Reject this match?\n\n' +
724+
'- The Spotify link will be deleted.\n' +
725+
'- The Spotify track will be added to the blocklist for ' +
726+
'this song so the matcher never re-links it.\n\n' +
727+
'This is permanent until you remove the blocklist entry ' +
728+
'manually.'
729+
)) {
730+
return;
731+
}
732+
const linkId = btn.dataset.linkId;
733+
btn.disabled = true;
734+
try {
735+
const resp = await fetch(
736+
`/admin/duration-mismatches/links/${linkId}/reject`,
737+
{
738+
method: 'POST',
739+
headers: {
740+
'Content-Type': 'application/json',
741+
'Accept': 'application/json',
742+
},
743+
body: JSON.stringify({}),
744+
}
745+
);
746+
const data = await resp.json();
747+
if (!resp.ok || !data.success) {
748+
throw new Error(data.error || ('HTTP ' + resp.status));
749+
}
750+
showToast(
751+
'Match rejected and Spotify track blocklisted',
752+
'success',
753+
);
754+
setTimeout(() => window.location.reload(), 500);
755+
} catch (err) {
756+
btn.disabled = false;
757+
showToast('Failed: ' + err.message, 'error');
758+
}
759+
}
760+
690761
// Enter key in threshold input
691762
document.getElementById('thresholdInput').addEventListener('keydown', function(e) {
692763
if (e.key === 'Enter') updateThreshold();

0 commit comments

Comments
 (0)