diff --git a/PROJECT.org b/PROJECT.org index 66a2c9e..e8fd168 100644 --- a/PROJECT.org +++ b/PROJECT.org @@ -18,7 +18,7 @@ tasks, Todoist tasks, web pages I've read and trails I've hiked has turned out to be sometimes cathartic and sometimes functional as I try to remember when I did a thing. -* Backlog [0/30] :vrobbler:project:personal: +* Backlog [1/31] :vrobbler:project:personal: ** TODO [#C] Configure IMAP folder/start in user profile :imap:settings: *** Description @@ -591,3 +591,16 @@ The Edit log form should have from top to bottom: - Expansion ids (which should a multi-select widget of expansions for this game) - Location (which should be a drop down of BoardGameLocations for this user) +** DONE [#A] Music track scrobbles now frequently find the wrong track :music:metadata:scrobbles: +:PROPERTIES: +:ID: 6f8355f0-e6d2-4f45-a973-2d80420ca346 +:END: +<2026-08-26 Wed> + +*** Description + +When a music track is scrobbled, it look only by title first, which frequently leads to getting the wrong track, where the title is correc, but the artist and album is wrong. + +We should look up by musicbrainz_id first if we have it, then fall back to title/artist__name, and then title. + + diff --git a/tests/scrobbles_tests/test_webhook_enrichment.py b/tests/scrobbles_tests/test_webhook_enrichment.py index ef9a574..cf700f0 100644 --- a/tests/scrobbles_tests/test_webhook_enrichment.py +++ b/tests/scrobbles_tests/test_webhook_enrichment.py @@ -47,6 +47,62 @@ def test_find_or_create_reuses_existing_track_by_mbid(mock_resolve): mock_resolve.assert_not_called() +@pytest.mark.django_db +@patch("music.models.resolve_track") +def test_find_or_create_looks_up_by_mbid_before_title(mock_resolve): + artist = Artist.objects.create(name="AC/DC") + wrong = Track.objects.create(title="TNT", musicbrainz_id="rec-999") + wrong.artists.add(artist) + existing = Track.objects.create(title="TNT", musicbrainz_id="rec-123") + + track = Track.find_or_create( + title="TNT", + artist_name="AC/DC", + mbid="rec-123", + trust_webhook_data=True, + ) + + assert track.id == existing.id + assert track.id != wrong.id + + +@pytest.mark.django_db +@patch("music.models.resolve_track") +def test_find_or_create_title_fallback_skips_track_with_conflicting_mbid( + mock_resolve, +): + wrong = Track.objects.create(title="TNT", musicbrainz_id="rec-999") + + track = Track.find_or_create( + title="TNT", + artist_name="AC/DC", + mbid="rec-123", + trust_webhook_data=True, + ) + + assert track.id != wrong.id + assert track.musicbrainz_id == "rec-123" + wrong.refresh_from_db() + assert wrong.musicbrainz_id == "rec-999" + + +@pytest.mark.django_db +@patch("music.models.resolve_track", return_value=(None, "")) +def test_find_or_create_falls_back_to_title_and_artist(mock_resolve): + artist = Artist.objects.create(name="AC/DC") + wrong = Track.objects.create(title="TNT") + existing = Track.objects.create(title="TNT") + existing.artists.add(artist) + + track = Track.find_or_create( + title="TNT", + artist_name="AC/DC", + ) + + assert track.id == existing.id + assert track.id != wrong.id + + @pytest.mark.django_db @patch("music.models.resolve_track") def test_fix_metadata_enriches_and_tags(mock_resolve): diff --git a/vrobbler/apps/music/models.py b/vrobbler/apps/music/models.py index 9952aa5..aead7a8 100644 --- a/vrobbler/apps/music/models.py +++ b/vrobbler/apps/music/models.py @@ -710,11 +710,18 @@ class Track(ScrobblableMixin): artist_objs.append(artist) track = None - if artist_objs: + if mbid: + track = cls.objects.filter(musicbrainz_id=mbid).first() + + if not track and artist_objs: track = cls.objects.filter(title=title, artists__in=artist_objs).first() if not track: track = cls.objects.filter(title=title).first() + # A title-only match that already claims a different recording is + # not the track we're looking for, so don't reuse and corrupt it. + if track and mbid and track.musicbrainz_id: + track = None if not track: track = cls(title=title)