From f7ed41867db8962756aacd43690ff59ff6289bd3 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Thu, 28 May 2026 12:00:29 -0700 Subject: [PATCH] Fix: enhanced artist view 404s for library artists opened via source ID MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Opening a library artist from a non-library search result (e.g. a MusicBrainz hit) leaves the artist-detail page holding the source ID — the MBID — not the integer library PK. The standard /api/artist-detail route resolves that via find_library_artist_for_source, but the enhanced-view (`/api/library/artist//enhanced`) and quality-analysis endpoints call get_artist_full_detail directly with whatever ID the page holds. Its lookup was `WHERE id = ?` only, so it 404'd ("Artist with ID not found") and the enhanced view failed to load. When the direct PK lookup misses, fall back to matching any per-service ID column, reusing SOURCE_ID_FIELD as the single source of truth so the resolution covers every source (MusicBrainz, Spotify, Deezer, iTunes, Discogs, Hydrabase, Amazon), not just MusicBrainz. Adds 4 isolated DB-method tests: direct PK still works, resolves by MBID, resolves by Spotify ID, and unknown IDs still 404. --- database/music_database.py | 18 ++ tests/test_artist_full_detail_source_id.py | 196 +++++++++++++++++++++ 2 files changed, 214 insertions(+) create mode 100644 tests/test_artist_full_detail_source_id.py diff --git a/database/music_database.py b/database/music_database.py index ea8eb98f..bc08e179 100644 --- a/database/music_database.py +++ b/database/music_database.py @@ -9983,6 +9983,24 @@ class MusicDatabase: # Get artist with all columns cursor.execute("SELECT * FROM artists WHERE id = ?", (artist_id,)) artist_row = cursor.fetchone() + if not artist_row: + # `artist_id` may be a *source* ID (e.g. a MusicBrainz MBID + # from a search result) rather than the integer library PK. + # The /api/artist-detail route resolves this upstream via + # find_library_artist_for_source, but the enhanced-view and + # quality-analysis endpoints call this method directly with + # whatever ID the page holds — for a library artist opened + # from a non-library search result that's the source ID, so + # the page 404'd. Resolve by matching any per-service ID + # column (single source of truth: SOURCE_ID_FIELD). + from core.artist_source_lookup import SOURCE_ID_FIELD + id_columns = list(dict.fromkeys(SOURCE_ID_FIELD.values())) + where = ' OR '.join(f"{col} = ?" for col in id_columns) + cursor.execute( + f"SELECT * FROM artists WHERE {where} LIMIT 1", + tuple(str(artist_id) for _ in id_columns), + ) + artist_row = cursor.fetchone() if not artist_row: return {'success': False, 'error': f'Artist with ID {artist_id} not found'} diff --git a/tests/test_artist_full_detail_source_id.py b/tests/test_artist_full_detail_source_id.py new file mode 100644 index 00000000..7f05e619 --- /dev/null +++ b/tests/test_artist_full_detail_source_id.py @@ -0,0 +1,196 @@ +"""Regression tests for ``MusicDatabase.get_artist_full_detail`` source-ID +resolution. + +Bug (reported by Boulder, 2026-05-28): opening a library artist from a +*non-library* search result (e.g. a MusicBrainz hit) leaves the artist-detail +page holding the source ID — the MBID — not the integer library PK. The +standard /api/artist-detail route resolves that via +``find_library_artist_for_source``, but the **enhanced-view** endpoint +(``/api/library/artist//enhanced``) and the quality-analysis endpoint call +``get_artist_full_detail`` directly with whatever ID the page holds. With only +a ``WHERE id = ?`` lookup, that 404'd ("Artist with ID not found") and +the enhanced view failed to load. + +Fix: when the direct PK lookup misses, resolve against any per-service ID +column (``SOURCE_ID_FIELD``). + +These are isolated DB-method tests — no Flask, no route layer — so the SQL +fallback itself is exercised. +""" + +import sqlite3 +import sys +import types + +import pytest + + +# ── stubs (same shape used elsewhere in the test suite) ─────────────────── +if "spotipy" not in sys.modules: + spotipy = types.ModuleType("spotipy") + spotipy.Spotify = object + oauth2 = types.ModuleType("spotipy.oauth2") + oauth2.SpotifyOAuth = object + oauth2.SpotifyClientCredentials = object + spotipy.oauth2 = oauth2 + sys.modules["spotipy"] = spotipy + sys.modules["spotipy.oauth2"] = oauth2 + +if "config.settings" not in sys.modules: + config_pkg = types.ModuleType("config") + settings_mod = types.ModuleType("config.settings") + + class _DummyConfigManager: + def get(self, key, default=None): + return default + + def get_active_media_server(self): + return "primary" + + settings_mod.config_manager = _DummyConfigManager() + config_pkg.settings = settings_mod + sys.modules["config"] = config_pkg + sys.modules["config.settings"] = settings_mod + + +from database.music_database import MusicDatabase # noqa: E402 + + +class _InMemoryDB(MusicDatabase): + """MusicDatabase backed by an in-memory sqlite that survives across + ``_get_connection()`` calls.""" + + def __init__(self): + self._conn = sqlite3.connect(":memory:") + self._conn.row_factory = sqlite3.Row + + def _get_connection(self): + return _NonClosingConn(self._conn) + + +class _NonClosingConn: + def __init__(self, real): + self._real = real + + def cursor(self): + return self._real.cursor() + + def commit(self): + return self._real.commit() + + def close(self): + pass + + def __enter__(self): + return self + + def __exit__(self, *args): + pass + + +def _seed_schema(db): + cur = db._conn.cursor() + # Only the columns get_artist_full_detail touches (it uses SELECT *, so + # the per-service ID columns must exist for the resolution fallback). + cur.execute(""" + CREATE TABLE artists ( + id INTEGER PRIMARY KEY, + name TEXT, + server_source TEXT, + genres TEXT, + musicbrainz_id TEXT, + spotify_artist_id TEXT, + deezer_id TEXT, + itunes_artist_id TEXT, + discogs_id TEXT, + soul_id TEXT, + amazon_id TEXT + ) + """) + cur.execute(""" + CREATE TABLE albums ( + id TEXT PRIMARY KEY, + artist_id INTEGER, + title TEXT, + year INTEGER, + genres TEXT, + record_type TEXT, + track_count INTEGER + ) + """) + cur.execute(""" + CREATE TABLE tracks ( + id TEXT PRIMARY KEY, + album_id TEXT, + title TEXT, + track_number INTEGER + ) + """) + db._conn.commit() + + +def _seed_kendrick(db, **id_columns): + """Insert a Kendrick Lamar library artist (PK 187926) with one album, + setting whichever per-service ID columns the test needs.""" + cols = ['id', 'name', 'server_source'] + list(id_columns) + vals = [187926, 'Kendrick Lamar', 'primary'] + list(id_columns.values()) + placeholders = ','.join('?' * len(cols)) + cur = db._conn.cursor() + cur.execute(f"INSERT INTO artists ({','.join(cols)}) VALUES ({placeholders})", vals) + cur.execute( + "INSERT INTO albums (id, artist_id, title, year, record_type) VALUES (?, ?, ?, ?, ?)", + ('alb-1', 187926, 'DAMN.', 2017, 'album'), + ) + db._conn.commit() + + +@pytest.fixture +def db(): + d = _InMemoryDB() + _seed_schema(d) + return d + + +def test_direct_pk_lookup_still_works(db): + """The primary path — integer library PK — must be unaffected by the + new fallback.""" + _seed_kendrick(db, musicbrainz_id='381086ea-mbid') + + result = db.get_artist_full_detail(187926) + + assert result['success'] is True + assert result['artist']['name'] == 'Kendrick Lamar' + assert [a['title'] for a in result['albums']] == ['DAMN.'] + + +def test_resolves_by_musicbrainz_id(db): + """The exact bug: page holds the MBID, not the PK. Must resolve and + return the library artist + albums instead of 404ing.""" + _seed_kendrick(db, musicbrainz_id='381086ea-mbid') + + result = db.get_artist_full_detail('381086ea-mbid') + + assert result['success'] is True + assert result['artist']['name'] == 'Kendrick Lamar' + assert [a['title'] for a in result['albums']] == ['DAMN.'] + + +def test_resolves_by_spotify_id(db): + """Resolution isn't MusicBrainz-specific — any per-service ID column + works (proves SOURCE_ID_FIELD reuse, not a hardcoded mbid check).""" + _seed_kendrick(db, spotify_artist_id='sp-kdot') + + result = db.get_artist_full_detail('sp-kdot') + + assert result['success'] is True + assert result['artist']['name'] == 'Kendrick Lamar' + + +def test_unknown_id_returns_not_found(db): + """An ID that matches neither the PK nor any source column still 404s.""" + _seed_kendrick(db, musicbrainz_id='381086ea-mbid') + + result = db.get_artist_full_detail('totally-unknown-id') + + assert result['success'] is False + assert 'not found' in result['error']