From 3a4017ea2bc9af18286d6934d10b8d2b41b5bde8 Mon Sep 17 00:00:00 2001 From: Broque Thomas <26755000+Nezreka@users.noreply.github.com> Date: Mon, 18 May 2026 13:07:54 -0700 Subject: [PATCH 1/3] =?UTF-8?q?feat:=20artist-detail=20deep=20linking=20?= =?UTF-8?q?=E2=80=94=20/artist-detail/:source/:id?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Artist detail pages previously always pushed /artist-detail to the URL, so refreshing the page or sharing a link would drop users on a broken empty page with no artist loaded. URL format is now /artist-detail/:source/:id (e.g. /artist-detail/spotify/4tZwfgrHOc3mvqsCAfo4LT or /artist-detail/library/42). The source segment lets the backend synthesize a response from the right metadata client without a DB hit. Changes: Client routing (legacy shell + TanStack bridge) - buildArtistDetailPath / _getDeepLinkArtistDetail added to init.js; parse both new :source/:id and legacy bare :id formats so old bookmarks still work - navigateToPage passes artistId + artistSource through to the router bridge, which builds the dynamic href instead of hardcoding route.path - resolveShellPageFromPath / resolveLegacyShellPageFromPath use a prefix match so /artist-detail/* resolves to artist-detail page-id - globals.d.ts typed for artistId / artistSource options - activateLegacyPath and syncActivePageFromLocation (popstate) both restore artist from URL using skipRouteChange:true to avoid a re-navigation loop back to /artist-detail - loadInitialData restores artist from URL on page load (router not yet mounted at DOMContentLoaded so legacy path runs unconditionally) - Same-artist guard in navigateToArtistDetail prevents double-fetch when the router fires activateLegacyPath after the initial navigation Server - artist_source_detail.build_source_only_artist_detail now resolves artist name from the source API when none is supplied, so deep-link restores with an empty name string still render correctly Tests - test_spa_deep_linking: /artist-detail/42 and /artist-detail/spotify/ID both serve index.html - bridge.test.ts: source-aware URL building and library fallback - route-manifest.test.ts: prefix path resolution - artist_source_detail: name resolved from source when input is empty --- core/artist_source_detail.py | 10 ++++ tests/metadata/test_artist_source_detail.py | 21 ++++++++ tests/test_spa_deep_linking.py | 12 +++++ webui/src/platform/shell/bridge.test.ts | 36 ++++++++++++- webui/src/platform/shell/bridge.ts | 11 ++-- webui/src/platform/shell/globals.d.ts | 2 + .../src/platform/shell/route-manifest.test.ts | 2 + webui/src/platform/shell/route-manifest.ts | 8 +++ webui/static/init.js | 53 +++++++++++++++++-- webui/static/library.js | 26 ++++++++- webui/static/search.js | 11 +++- webui/static/shell-bridge.js | 20 +++++++ 12 files changed, 200 insertions(+), 12 deletions(-) diff --git a/core/artist_source_detail.py b/core/artist_source_detail.py index 35b3e4de..d8e8b45c 100644 --- a/core/artist_source_detail.py +++ b/core/artist_source_detail.py @@ -68,6 +68,8 @@ def build_source_only_artist_detail( if source == "spotify" and spotify_client is not None: sp_artist = spotify_client.get_artist(artist_id, allow_fallback=False) if sp_artist: + if not artist_name and sp_artist.get("name"): + resolved_name = sp_artist["name"] source_genres = sp_artist.get("genres") or [] source_followers = (sp_artist.get("followers") or {}).get("total") if not image_url and sp_artist.get("images"): @@ -75,19 +77,27 @@ def build_source_only_artist_detail( elif source == "deezer" and deezer_client is not None: dz_artist = deezer_client.get_artist_info(artist_id) if dz_artist: + if not artist_name and dz_artist.get("name"): + resolved_name = dz_artist["name"] source_genres = dz_artist.get("genres") or [] source_followers = (dz_artist.get("followers") or {}).get("total") elif source == "itunes" and itunes_client is not None: it_artist = itunes_client.get_artist(artist_id) if it_artist: + if not artist_name and it_artist.get("name"): + resolved_name = it_artist["name"] source_genres = it_artist.get("genres") or [] elif source == "discogs" and discogs_client is not None: dc_artist = discogs_client.get_artist(artist_id) if dc_artist: + if not artist_name and dc_artist.get("name"): + resolved_name = dc_artist["name"] source_genres = dc_artist.get("genres") or [] elif source == "amazon" and amazon_client is not None: az_artist = amazon_client.get_artist(resolved_name or artist_id) if az_artist: + if not artist_name and az_artist.get("name"): + resolved_name = az_artist["name"] source_genres = az_artist.get("genres") or [] if not image_url and az_artist.get("images"): image_url = az_artist["images"][0].get("url") diff --git a/tests/metadata/test_artist_source_detail.py b/tests/metadata/test_artist_source_detail.py index 5b21769f..a1a1c2c8 100644 --- a/tests/metadata/test_artist_source_detail.py +++ b/tests/metadata/test_artist_source_detail.py @@ -147,6 +147,7 @@ class TestPerSourceEnrichment: def test_spotify_extracts_genres_followers_and_image_fallback(self, _stub_metadata): spotify = SimpleNamespace( get_artist=lambda aid, allow_fallback=False: { + "name": "Artist", "genres": ["alt rock", "emo"], "followers": {"total": 12345}, "images": [{"url": "https://sp/img.jpg"}], @@ -160,6 +161,26 @@ class TestPerSourceEnrichment: # image_url falls back to Spotify's image when metadata returned None assert payload["artist"]["image_url"] == "https://sp/img.jpg" + def test_empty_name_uses_source_artist_name_when_available(self, _stub_metadata): + spotify = SimpleNamespace( + get_artist=lambda aid, allow_fallback=False: { + "name": "Kendrick Lamar", + "genres": [], + "followers": {}, + "images": [], + } + ) + + payload, _ = build_source_only_artist_detail( + "2YZyLoL8N0Wb9xBt1NhZWg", "", "spotify", spotify_client=spotify, + ) + + assert payload["artist"]["name"] == "Kendrick Lamar" + assert _stub_metadata["last_discog_call"] == ( + "2YZyLoL8N0Wb9xBt1NhZWg", + "Kendrick Lamar", + ) + def test_deezer_extracts_genres_and_followers(self, _stub_metadata): deezer = SimpleNamespace( get_artist_info=lambda aid: { diff --git a/tests/test_spa_deep_linking.py b/tests/test_spa_deep_linking.py index 38a90534..f57a1d90 100644 --- a/tests/test_spa_deep_linking.py +++ b/tests/test_spa_deep_linking.py @@ -95,6 +95,18 @@ class TestSpaRoutes: assert resp.status_code == 200 assert resp.data == b'INDEX_HTML' + def test_artist_detail_with_id_serves_index(self, client): + # /artist-detail/:id deep-links must serve index so the client can restore the artist. + resp = client.get('/artist-detail/42') + assert resp.status_code == 200 + assert resp.data == b'INDEX_HTML' + + def test_artist_detail_with_source_and_id_serves_index(self, client): + # /artist-detail/:source/:id is the canonical source-aware artist deep-link. + resp = client.get('/artist-detail/spotify/2YZyLoL8N0Wb9xBt1NhZWg') + assert resp.status_code == 200 + assert resp.data == b'INDEX_HTML' + # --------------------------------------------------------------------------- # Group B — Reserved prefixes are not shadowed diff --git a/webui/src/platform/shell/bridge.test.ts b/webui/src/platform/shell/bridge.test.ts index 24787e1b..00dd88d4 100644 --- a/webui/src/platform/shell/bridge.test.ts +++ b/webui/src/platform/shell/bridge.test.ts @@ -2,7 +2,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; import type { ShellProfileContext } from './bridge'; -import { SHELL_PROFILE_CONTEXT_CHANGED_EVENT, waitForShellContext } from './bridge'; +import { SHELL_PROFILE_CONTEXT_CHANGED_EVENT, bindWindowWebRouter, waitForShellContext } from './bridge'; describe('waitForShellContext', () => { beforeEach(() => { @@ -55,3 +55,37 @@ describe('waitForShellContext', () => { }); }); }); + +describe('bindWindowWebRouter', () => { + it('navigates artist detail pages with source-aware URLs', async () => { + const navigate = vi.fn().mockResolvedValue(undefined); + + bindWindowWebRouter({ navigate } as never); + + await window.SoulSyncWebRouter?.navigateToPage('artist-detail', { + artistId: '2YZyLoL8N0Wb9xBt1NhZWg', + artistSource: 'spotify', + }); + + expect(navigate).toHaveBeenCalledWith({ + href: '/artist-detail/spotify/2YZyLoL8N0Wb9xBt1NhZWg', + replace: false, + }); + }); + + it('falls back artist detail URLs to library source when none is supplied', async () => { + const navigate = vi.fn().mockResolvedValue(undefined); + + bindWindowWebRouter({ navigate } as never); + + await window.SoulSyncWebRouter?.navigateToPage('artist-detail', { + artistId: '42', + replace: true, + }); + + expect(navigate).toHaveBeenCalledWith({ + href: '/artist-detail/library/42', + replace: true, + }); + }); +}); diff --git a/webui/src/platform/shell/bridge.ts b/webui/src/platform/shell/bridge.ts index 04db84c8..1ca85768 100644 --- a/webui/src/platform/shell/bridge.ts +++ b/webui/src/platform/shell/bridge.ts @@ -89,10 +89,13 @@ export function bindWindowWebRouter(router: AnyRouter) { const route = getShellRouteByPageId(pageId); if (!route) return false; - await router.navigate({ - href: route.path, - replace: options?.replace === true, - }); + let href: `/${string}` = route.path; + if (pageId === 'artist-detail' && options?.artistId) { + const source = options.artistSource ? String(options.artistSource) : 'library'; + href = `/artist-detail/${encodeURIComponent(source)}/${encodeURIComponent(String(options.artistId))}` as `/${string}`; + } + + await router.navigate({ href, replace: options?.replace === true }); return true; }, }; diff --git a/webui/src/platform/shell/globals.d.ts b/webui/src/platform/shell/globals.d.ts index c7bd15a1..27de813c 100644 --- a/webui/src/platform/shell/globals.d.ts +++ b/webui/src/platform/shell/globals.d.ts @@ -23,6 +23,8 @@ declare global { pageId: ShellPageId, options?: { replace?: boolean; + artistId?: string | number; + artistSource?: string | null; }, ) => Promise; }; diff --git a/webui/src/platform/shell/route-manifest.test.ts b/webui/src/platform/shell/route-manifest.test.ts index 4c384094..4430dd11 100644 --- a/webui/src/platform/shell/route-manifest.test.ts +++ b/webui/src/platform/shell/route-manifest.test.ts @@ -17,6 +17,7 @@ describe('shellRouteManifest', () => { expect(resolveShellPageFromPath('/watchlist')).toBe('watchlist'); expect(resolveShellPageFromPath('/active-downloads')).toBe('active-downloads'); expect(resolveShellPageFromPath('/artist-detail')).toBe('artist-detail'); + expect(resolveShellPageFromPath('/artist-detail/spotify/2YZyLoL8N0Wb9xBt1NhZWg')).toBe('artist-detail'); expect(resolveShellPageFromPath('/artists')).toBeNull(); }); @@ -49,6 +50,7 @@ describe('shellRouteManifest', () => { expect(resolveLegacyShellPageFromPath('/search')).toBe('search'); expect(resolveLegacyShellPageFromPath('/active-downloads')).toBe('active-downloads'); expect(resolveLegacyShellPageFromPath('/tools')).toBe('tools'); + expect(resolveLegacyShellPageFromPath('/artist-detail/deezer/12345')).toBe('artist-detail'); expect(resolveLegacyShellPageFromPath('/issues')).toBeNull(); expect(resolveLegacyShellPageFromPath('/does-not-exist')).toBeNull(); }); diff --git a/webui/src/platform/shell/route-manifest.ts b/webui/src/platform/shell/route-manifest.ts index 1f07f582..6b1a209e 100644 --- a/webui/src/platform/shell/route-manifest.ts +++ b/webui/src/platform/shell/route-manifest.ts @@ -71,10 +71,18 @@ export function getShellRouteByPath(pathname: string): ShellRouteDefinition | un } export function resolveShellPageFromPath(pathname: string): ShellPageId | null { + const normalized = normalizeShellPath(pathname); + if (normalized === '/artist-detail' || normalized.startsWith('/artist-detail/')) { + return 'artist-detail'; + } return getShellRouteByPath(pathname)?.pageId ?? null; } export function resolveLegacyShellPageFromPath(pathname: string): ShellPageId | null { + const normalized = normalizeShellPath(pathname); + if (normalized === '/artist-detail' || normalized.startsWith('/artist-detail/')) { + return 'artist-detail'; + } const route = getShellRouteByPath(pathname); return route?.kind === 'legacy' ? route.pageId : null; } diff --git a/webui/static/init.js b/webui/static/init.js index bfb61999..c874f6fc 100644 --- a/webui/static/init.js +++ b/webui/static/init.js @@ -2150,14 +2150,53 @@ function _getPageFromPath() { const path = window.location.pathname.replace(/^\/+|\/+$/g, ''); if (!path) return 'dashboard'; - const basePage = path.split('/')[0]; + const segs = path.split('/'); + const basePage = segs[0]; if (!_DEEPLINK_VALID_PAGES.has(basePage)) return 'dashboard'; // Context-dependent pages fall back to a sensible parent - if (basePage === 'artist-detail') return 'library'; + if (basePage === 'artist-detail') { + // /artist-detail/:id deep-link — keep on artist-detail; bare /artist-detail falls back to library + return (segs.length >= 2 && segs[1]) ? 'artist-detail' : 'library'; + } if (basePage === 'playlist-explorer') return 'library'; return basePage; } +function _normalizeArtistDetailSource(source) { + const value = (source || '').toString().trim().toLowerCase(); + return value || 'library'; +} + +function buildArtistDetailPath(artistId, source = null) { + if (!artistId) return '/artist-detail'; + const normalizedSource = _normalizeArtistDetailSource(source); + return '/artist-detail/' + encodeURIComponent(normalizedSource) + '/' + encodeURIComponent(String(artistId)); +} + +/** Extract artist source + ID from /artist-detail/:source/:id or legacy /artist-detail/:id URLs. */ +function _getDeepLinkArtistDetail(pathname = window.location.pathname) { + const path = (pathname || '').replace(/^\/+|\/+$/g, ''); + const segs = path.split('/'); + if (segs[0] !== 'artist-detail' || !segs[1]) return null; + + if (segs[2]) { + return { + source: _normalizeArtistDetailSource(decodeURIComponent(segs[1])), + artistId: decodeURIComponent(segs.slice(2).join('/')), + }; + } + + return { + source: 'library', + artistId: decodeURIComponent(segs[1]), + }; +} + +/** Legacy convenience wrapper for callers that only need the artist ID. */ +function _getDeepLinkArtistId() { + return _getDeepLinkArtistDetail()?.artistId || null; +} + // =============================== // MOBILE NAVIGATION // =============================== @@ -2279,7 +2318,11 @@ function navigateToPage(pageId, options = {}) { showReactHost(pageId); setActivePageChrome(pageId); } - return router.navigateToPage(pageId, { replace: options.replace === true }); + return router.navigateToPage(pageId, { + replace: options.replace === true, + artistId: options.artistId, + artistSource: options.artistSource, + }); } // Fallback path for initial bootstrap or environments without TanStack routing. @@ -2294,7 +2337,9 @@ function navigateToPage(pageId, options = {}) { } if (!options.skipPushState) { - const urlPath = pageId === 'dashboard' ? '/' : '/' + pageId; + const urlPath = pageId === 'dashboard' ? '/' + : (pageId === 'artist-detail' && options.artistId) ? buildArtistDetailPath(options.artistId, options.artistSource) + : '/' + pageId; if (window.location.pathname !== urlPath) { if (options.replace === true) { history.replaceState({ page: pageId }, '', urlPath); diff --git a/webui/static/library.js b/webui/static/library.js index d4d912fa..8e5b6a62 100644 --- a/webui/static/library.js +++ b/webui/static/library.js @@ -803,6 +803,24 @@ const _ARTIST_DETAIL_BACK_LABELS = { }; function navigateToArtistDetail(artistId, artistName, sourceOverride = null, options = {}) { + const normalizedSource = sourceOverride || null; + + // Skip reload if already on this exact artist/source (prevents double-fetch + // when the router fires activateLegacyPath after navigating to an + // /artist-detail/:source/:id URL). + if (artistId && + String(artistId) === String(artistDetailPageState.currentArtistId) && + String(normalizedSource || '') === String(artistDetailPageState.currentArtistSource || '')) { + if (currentPage !== 'artist-detail') { + navigateToPage('artist-detail', { + artistId, + artistSource: normalizedSource, + skipRouteChange: true + }); + _updateArtistDetailBackButtonLabel(); + } + return; + } console.log(`🎵 Navigating to artist detail: ${artistName} (ID: ${artistId}${sourceOverride ? `, source: ${sourceOverride}` : ''})`); // Capture the current location on the origin stack BEFORE navigateToPage @@ -856,7 +874,7 @@ function navigateToArtistDetail(artistId, artistName, sourceOverride = null, opt // Store current artist info and reset enhanced view state artistDetailPageState.currentArtistId = artistId; artistDetailPageState.currentArtistName = artistName; - artistDetailPageState.currentArtistSource = sourceOverride || null; + artistDetailPageState.currentArtistSource = normalizedSource; artistDetailPageState.enhancedData = null; artistDetailPageState.expandedAlbums = new Set(); artistDetailPageState.selectedTracks = new Set(); @@ -885,7 +903,11 @@ function navigateToArtistDetail(artistId, artistName, sourceOverride = null, opt if (bulkBar) bulkBar.classList.remove('visible'); // Navigate to artist detail page - navigateToPage('artist-detail'); + navigateToPage('artist-detail', { + artistId, + artistSource: normalizedSource, + skipRouteChange: options.skipRouteChange === true + }); // Update back-button label to reflect where the next pop will land. _updateArtistDetailBackButtonLabel(); diff --git a/webui/static/search.js b/webui/static/search.js index 8362e774..fd6d2604 100644 --- a/webui/static/search.js +++ b/webui/static/search.js @@ -1197,7 +1197,16 @@ async function loadInitialData() { return; } - navigateToPage(targetPage, { skipRouteChange: true, forceReload: true }); + if (targetPage === 'artist-detail') { + const deepArtist = _getDeepLinkArtistDetail(); + if (deepArtist?.artistId && typeof navigateToArtistDetail === 'function') { + navigateToArtistDetail(deepArtist.artistId, '', deepArtist.source === 'library' ? null : deepArtist.source); + } else { + navigateToPage('library', { skipRouteChange: true, forceReload: true }); + } + } else { + navigateToPage(targetPage, { skipRouteChange: true, forceReload: true }); + } } catch (error) { console.error('Error loading initial data:', error); } diff --git a/webui/static/shell-bridge.js b/webui/static/shell-bridge.js index 6d5e38f9..8d2ac58a 100644 --- a/webui/static/shell-bridge.js +++ b/webui/static/shell-bridge.js @@ -80,6 +80,16 @@ function activateLegacyPath(pathname) { return; } + if (targetPage === 'artist-detail') { + const deepArtist = _getDeepLinkArtistDetail(pathname); + if (deepArtist?.artistId && typeof navigateToArtistDetail === 'function') { + navigateToArtistDetail(deepArtist.artistId, '', deepArtist.source === 'library' ? null : deepArtist.source, { skipOriginPush: true, skipRouteChange: true }); + return; + } + navigateToPage('library', { replace: true }); + return; + } + notifyPageWillChange(targetPage); activatePage(targetPage, { forceReload: true }); } @@ -97,6 +107,16 @@ function syncActivePageFromLocation() { return; } + if (targetPage === 'artist-detail') { + const deepArtist = _getDeepLinkArtistDetail(); + if (deepArtist?.artistId && typeof navigateToArtistDetail === 'function') { + navigateToArtistDetail(deepArtist.artistId, '', deepArtist.source === 'library' ? null : deepArtist.source, { skipOriginPush: true, skipRouteChange: true }); + return; + } + navigateToPage('library', { replace: true }); + return; + } + notifyPageWillChange(targetPage); const route = router?.routeManifest?.find((entry) => entry.pageId === targetPage); if (route?.kind === 'react') { From a6282b3009cae1988a6fce12fb4d9de5ee1ada48 Mon Sep 17 00:00:00 2001 From: Broque Thomas <26755000+Nezreka@users.noreply.github.com> Date: Mon, 18 May 2026 13:50:10 -0700 Subject: [PATCH 2/3] Fix source artist detail navigation from discover modals Preserve source metadata for seasonal and cached discover album modals so artist links use real provider IDs instead of falling back to library/name routes. Treat source-only artist detail discographies as clickable missing releases and skip library-only ownership/enhancement checks. --- core/seasonal_discovery.py | 3 ++- webui/static/discover.js | 49 ++++++++++++++++++++++++++------------ webui/static/downloads.js | 40 +++++++++++++++++++++++++++++-- webui/static/library.js | 17 +++++++++++-- 4 files changed, 89 insertions(+), 20 deletions(-) diff --git a/core/seasonal_discovery.py b/core/seasonal_discovery.py index 159776f7..b813717a 100644 --- a/core/seasonal_discovery.py +++ b/core/seasonal_discovery.py @@ -707,7 +707,8 @@ class SeasonalDiscoveryService: artist_name, album_cover_url, release_date, - popularity + popularity, + source FROM seasonal_albums WHERE season_key = ? AND source = ? ORDER BY popularity DESC, album_name ASC diff --git a/webui/static/discover.js b/webui/static/discover.js index aa22b3aa..af8fdd91 100644 --- a/webui/static/discover.js +++ b/webui/static/discover.js @@ -3945,6 +3945,35 @@ function hideSeasonalSections() { } } +function _buildDiscoverArtistContext(source, artistName, sourceData = {}, albumData = {}) { + const normalizedSource = (source || '').toString().toLowerCase(); + const albumArtist = Array.isArray(albumData.artists) ? albumData.artists[0] : null; + const activeSource = (source || sourceData.active_source || sourceData.source || '').toString().toLowerCase(); + const context = { + ...sourceData, + id: sourceData.active_source_id || sourceData.artist_id || albumArtist?.id || '', + name: artistName || sourceData.artist_name || sourceData.name || albumArtist?.name || '', + source: normalizedSource || activeSource || '', + spotify_artist_id: sourceData.spotify_artist_id || sourceData.artist_spotify_id || ((normalizedSource || activeSource) === 'spotify' ? albumArtist?.id : '') || '', + itunes_artist_id: sourceData.itunes_artist_id || sourceData.artist_itunes_id || ((normalizedSource || activeSource) === 'itunes' ? albumArtist?.id : '') || '', + deezer_artist_id: sourceData.deezer_artist_id || sourceData.artist_deezer_id || sourceData.deezer_id || ((normalizedSource || activeSource) === 'deezer' ? albumArtist?.id : '') || '', + discogs_artist_id: sourceData.discogs_artist_id || sourceData.artist_discogs_id || sourceData.discogs_id || '', + amazon_artist_id: sourceData.amazon_artist_id || sourceData.artist_amazon_id || sourceData.amazon_id || '', + soul_id: sourceData.soul_id || sourceData.hydrabase_artist_id || '', + }; + + const sourceIdBySource = { + spotify: context.spotify_artist_id, + itunes: context.itunes_artist_id, + deezer: context.deezer_artist_id, + discogs: context.discogs_artist_id, + amazon: context.amazon_artist_id, + hydrabase: context.soul_id, + }; + context.id = sourceIdBySource[normalizedSource || activeSource] || context.id; + return context; +} + async function openDownloadModalForSeasonalAlbum(albumIndex) { const album = discoverSeasonalAlbums[albumIndex]; if (!album) { @@ -4004,14 +4033,12 @@ async function openDownloadModalForSeasonalAlbum(albumIndex) { const virtualPlaylistId = `seasonal_album_${albumId}`; // Pass proper artist/album context for album download (1 worker + source reuse) - const artistContext = { - name: album.artist_name, - source: source - }; + const artistContext = _buildDiscoverArtistContext(source, album.artist_name, album, albumData); const albumContext = { id: albumData.id, name: albumData.name, + source: source, album_type: albumData.album_type || 'album', total_tracks: albumData.total_tracks || 0, release_date: albumData.release_date || '', @@ -6986,7 +7013,7 @@ async function openCacheDiscoverAlbum(sectionKey, index) { duration_ms: track.duration_ms || 0, track_number: track.track_number || 0, }; }); - const artistContext = { id: albumData.artists?.[0]?.id || '', name: artistName, source: resolvedSource }; + const artistContext = _buildDiscoverArtistContext(resolvedSource, artistName, item, albumData); const albumContext = { id: albumData.id, name: albumData.name, album_type: albumData.album_type || 'album', total_tracks: albumData.total_tracks || 0, release_date: albumData.release_date || '', images: albumData.images || [] }; await openDownloadMissingModalForYouTube(`discover_cache_${resolvedId}`, albumData.name, spotifyTracks, artistContext, albumContext); hideLoadingOverlay(); @@ -7046,11 +7073,7 @@ async function openCacheDiscoverAlbum(sectionKey, index) { }; }); - const artistContext = { - id: albumData.artists?.[0]?.id || '', - name: item.artist_name || albumData.artists?.[0]?.name || '', - source: source, - }; + const artistContext = _buildDiscoverArtistContext(source, item.artist_name || albumData.artists?.[0]?.name || '', item, albumData); const albumContext = { id: albumData.id, name: albumData.name, @@ -7970,11 +7993,7 @@ async function openDownloadModalForRecentAlbum(albumIndex) { const virtualPlaylistId = `discover_album_${albumId}`; // CRITICAL FIX: Pass proper artist/album context for modal display - const artistContext = { - id: source === 'spotify' ? album.artist_spotify_id : source === 'deezer' ? album.artist_deezer_id : album.artist_itunes_id, - name: album.artist_name, - source: source - }; + const artistContext = _buildDiscoverArtistContext(source, album.artist_name, album, albumData); const albumContext = { id: albumData.id, diff --git a/webui/static/downloads.js b/webui/static/downloads.js index 8edca77e..7374753d 100644 --- a/webui/static/downloads.js +++ b/webui/static/downloads.js @@ -501,7 +501,10 @@ async function openDownloadMissingModalForYouTube(virtualPlaylistId, playlistNam const heroContext = isDiscoverAlbum && album && artist ? { type: 'album', artist: { + ...artist, name: artist.name, + id: artist.id || artist.artist_id || null, + source: artist.source || album.source || null, image_url: artist.image_url || null }, album: { @@ -634,9 +637,42 @@ function _navigateToArtistFromModal(artistId, artistName, imageUrl, source, play if (!artistName) return; // Close the download modal const process = playlistId ? activeDownloadProcesses[playlistId] : null; - const resolvedSource = source || process?.artist?.source || process?.album?.source || process?.source || null; + const artistContext = process?.artist || {}; + const inferredSource = artistContext.spotify_artist_id ? 'spotify' + : artistContext.itunes_artist_id ? 'itunes' + : (artistContext.deezer_artist_id || artistContext.deezer_id) ? 'deezer' + : (artistContext.discogs_artist_id || artistContext.discogs_id) ? 'discogs' + : (artistContext.amazon_artist_id || artistContext.amazon_id) ? 'amazon' + : (artistContext.soul_id || artistContext.hydrabase_artist_id) ? 'hydrabase' + : null; + const resolvedSource = source || process?.artist?.source || process?.album?.source || process?.source || inferredSource; + const sourceKey = (resolvedSource || '').toString().toLowerCase(); + const sourceIdFields = { + spotify: ['spotify_artist_id', 'id', 'artist_id'], + itunes: ['itunes_artist_id', 'artist_id', 'id'], + deezer: ['deezer_artist_id', 'deezer_id', 'artist_id', 'id'], + discogs: ['discogs_artist_id', 'discogs_id', 'artist_id', 'id'], + amazon: ['amazon_artist_id', 'amazon_id', 'artist_id', 'id'], + hydrabase: ['soul_id', 'hydrabase_artist_id', 'artist_id', 'id'], + musicbrainz: ['musicbrainz_id', 'artist_id', 'id'], + }; + let resolvedArtistId = artistId; + for (const field of (sourceIdFields[sourceKey] || ['artist_id', 'id'])) { + const candidate = artistContext?.[field]; + if (candidate) { + resolvedArtistId = candidate; + break; + } + } + if (resolvedArtistId && String(resolvedArtistId).toLowerCase() === String(artistName).toLowerCase()) { + resolvedArtistId = null; + } if (playlistId) closeDownloadMissingModal(playlistId); - navigateToArtistDetail(artistId || artistName, artistName, resolvedSource || null); + if (!resolvedArtistId || !resolvedSource) { + showToast(`Artist details are not available for ${artistName}`, 'warning'); + return; + } + navigateToArtistDetail(resolvedArtistId, artistName, resolvedSource); } async function closeDownloadMissingModal(playlistId) { diff --git a/webui/static/library.js b/webui/static/library.js index 8e5b6a62..e1ea24d3 100644 --- a/webui/static/library.js +++ b/webui/static/library.js @@ -1042,6 +1042,17 @@ async function loadArtistDetailData(artistId, artistName) { throw new Error(data.error || 'Failed to load artist data'); } + const isSourceOnlyArtist = !data.artist?.server_source; + if (isSourceOnlyArtist && data.discography) { + for (const bucket of ['albums', 'eps', 'singles']) { + for (const release of (data.discography[bucket] || [])) { + if (release.owned === null || typeof release.owned === 'undefined') { + release.owned = false; + } + } + } + } + console.log(`✅ Loaded artist detail data:`, data); // Hide loading and show all content @@ -1077,7 +1088,7 @@ async function loadArtistDetailData(artistId, artistName) { renderArtistEnrichmentCoverage(data.enrichment_coverage); // Start streaming ownership checks if we have Spotify discography with checking state - if (data.discography && data.discography.albums) { + if (!isSourceOnlyArtist && data.discography && data.discography.albums) { const hasChecking = [...(data.discography.albums || []), ...(data.discography.eps || []), ...(data.discography.singles || [])] .some(r => r.owned === null); if (hasChecking) { @@ -1091,7 +1102,9 @@ async function loadArtistDetailData(artistId, artistName) { // Use currentArtistId (not the closure arg) because the library-upgrade // branch above may have rewritten it from the source ID to the library PK, // and /api/library/artist//quality-analysis only works on library PKs. - checkArtistEnhanceEligibility(artistDetailPageState.currentArtistId); + if (!isSourceOnlyArtist) { + checkArtistEnhanceEligibility(artistDetailPageState.currentArtistId); + } } catch (error) { console.error(`❌ Error loading artist detail data:`, error); From 098c78786170950d0a20080d45e48e86a0557789 Mon Sep 17 00:00:00 2001 From: Broque Thomas <26755000+Nezreka@users.noreply.github.com> Date: Mon, 18 May 2026 15:07:09 -0700 Subject: [PATCH 3/3] Add release art fallback for artist detail hero Use the first available album, EP, or single artwork when an artist portrait is missing or fails to load, keeping artist detail pages visually populated across library and source-only artists. Refresh the PR description for the artist detail deep-link branch. --- pr_description.md | 69 +++++++++++++++++++++-------------------- webui/static/library.js | 38 ++++++++++++++++++----- 2 files changed, 66 insertions(+), 41 deletions(-) diff --git a/pr_description.md b/pr_description.md index da9aeb6c..510794f5 100644 --- a/pr_description.md +++ b/pr_description.md @@ -1,56 +1,57 @@ ## Summary -Adds a new **Manual Library Match** tool that lets users map a wishlist/sync source track to an existing library track. Once saved, SoulSync can treat that source track as already owned/found instead of repeatedly trying to download it. +Adds source-aware artist detail deep links so artist pages can be opened directly as `/artist-detail/:source/:id`, including metadata-source artists from Spotify, Deezer, iTunes, Discogs, Amazon, Hydrabase, and existing library artists. + +This also fixes Discover/download modal artist links that were falling back to `/artist-detail/library/` or using the wrong album/card ID as an artist ID. ## What Changed -- Added a centralized Manual Library Match tool on the Tools page and a shortcut from the Sync page. -- Added side-by-side source-track and library-track search UI, plus an existing matches table with removal support. -- Added `manual_library_track_matches` persistence with profile/source/source-track scoping. -- Added backend search/list/save/delete endpoints for manual matches. -- Integrated manual matches into download analysis so matched source tracks are marked found and skipped. -- Preserved the distinction between wishlist internal force mode and explicit user Force Download All: - - Wishlist/internal `force_download_all` still honors manual matches. - - User-facing Force Download All ignores manual matches and downloads anyway. -- Updated wishlist cleanup so manual matches are removed from the wishlist through: - - manual wishlist cleanup, - - automatic cleanup after DB update, - - wishlist download analysis. -- Prevented manually matched source tracks from being re-added to the wishlist in the common database add path. -- Polished the Tools page card so Manual Library Match visually matches the Discovery Pool tool card. +- Added canonical artist detail routes in the SPA: + - `/artist-detail/library/` + - `/artist-detail/spotify/` + - `/artist-detail/deezer/` + - `/artist-detail/itunes/` + - plus other supported metadata sources. +- Preserved legacy `/artist-detail/` behavior as a library fallback. +- Updated shell routing and deep-link activation so refresh/direct navigation works for nested artist-detail URLs. +- Updated artist detail navigation to carry `artistSource` through the SPA instead of relying only on artist name/id. +- Improved source-only artist detail loading so provider-fetched artist names are used when the URL only contains the source ID. +- Prevented source-only artist pages from running library-only ownership/enhancement checks. +- Treats unknown ownership on source-only discographies as missing/clickable instead of leaving cards stuck on "still checking ownership." +- Uses release artwork as a generic artist-detail hero fallback when an artist portrait is missing or fails to load. +- Preserved source/artist IDs from Discover album modals, seasonal albums, cached discovery albums, recent releases, and download modal hero links. +- Prevented modal artist links from falling back to fake library routes when a real source artist ID is unavailable. +- Returned seasonal album `source` from cached seasonal album rows so seasonal modal links retain their provider context. ## Behavior -- Manual match saved: - - `source + source_track_id + profile_id -> library_track_id` -- Wishlist cleanup: - - removes the item if a manual match exists. -- Wishlist add: - - skips inserting the item if a manual match already exists, returning a harmless success/no-op. -- Wishlist download: - - checks manual match first, marks found, skips download, and attempts wishlist removal. -- Normal download with Force Download All off: - - checks manual match before normal library matching. -- Normal download with Force Download All on: - - skips manual matches and downloads selected tracks intentionally. +- Clicking a Spotify artist result can now land on: + - `/artist-detail/spotify/2YZyLoL8N0Wb9xBt1NhZWg` +- Clicking a Deezer artist result can now land on: + - `/artist-detail/deezer/525046` +- Existing library artist links continue to resolve through the library path. +- If a source artist resolves to an existing library artist, the page upgrades to the library-backed artist and keeps library-only tools/checks available. +- If a source artist is not in the library, the page shows source discography as missing/clickable and skips library-only endpoints. +- If a modal lacks a trustworthy source artist ID, it shows a warning instead of navigating to an invalid library artist URL. ## Tests -Verified by user: +Frontend route tests: ```bash -./.venv/bin/python -m pytest tests/test_manual_library_match.py tests/downloads/test_downloads_master.py +cd webui +npm.cmd test -- --run src/platform/shell/route-manifest.test.ts src/platform/shell/bridge.test.ts ``` Result: ```text -43 passed in 10.29s +2 test files passed +10 tests passed ``` -Additional local verification: +Recommended backend verification: -```text -py_compile passed for touched Python files -node --check passed for touched JS +```bash +./.venv/bin/python -m pytest tests/test_spa_deep_linking.py tests/metadata/test_artist_source_detail.py ``` diff --git a/webui/static/library.js b/webui/static/library.js index e1ea24d3..19818b16 100644 --- a/webui/static/library.js +++ b/webui/static/library.js @@ -1349,15 +1349,34 @@ function updateArtistHeaderStats(albumCount, trackCount) { console.log("📊 Using new hero section instead of old header stats"); } +function _isUsableArtistHeroImageUrl(url) { + return typeof url === 'string' && url.trim() !== '' && url !== 'null'; +} + +function _getArtistHeroReleaseImage(discography) { + for (const bucket of ['albums', 'eps', 'singles']) { + for (const release of (discography?.[bucket] || [])) { + if (_isUsableArtistHeroImageUrl(release?.image_url)) { + return release.image_url; + } + } + } + return ''; +} + function updateArtistHeroSection(artist, discography) { console.log("🖼️ Updating artist hero section"); + const artistImageUrl = _isUsableArtistHeroImageUrl(artist.image_url) ? artist.image_url : ''; + const releaseImageUrl = _getArtistHeroReleaseImage(discography); + const primaryHeroImageUrl = artistImageUrl || releaseImageUrl; + // Blurred background image (inline-Artists hero treatment) — set whenever // we have an image_url; falls back to clearing the bg if not. const heroBg = document.getElementById("artist-detail-hero-bg"); if (heroBg) { - if (artist.image_url && artist.image_url.trim() !== "" && artist.image_url !== "null") { - heroBg.style.backgroundImage = `url('${artist.image_url}')`; + if (primaryHeroImageUrl) { + heroBg.style.backgroundImage = `url('${primaryHeroImageUrl}')`; } else { heroBg.style.backgroundImage = ''; } @@ -1374,9 +1393,11 @@ function updateArtistHeroSection(artist, discography) { console.log(` - Image element:`, imageElement); console.log(` - Fallback element:`, fallbackElement); - if (artist.image_url && artist.image_url.trim() !== "" && artist.image_url !== "null") { - console.log(`✅ Setting image src to: ${artist.image_url}`); - imageElement.src = artist.image_url; + if (primaryHeroImageUrl) { + console.log(`✅ Setting image src to: ${primaryHeroImageUrl}`); + imageElement.dataset.triedDeezer = ''; + imageElement.dataset.triedReleaseFallback = artistImageUrl ? '' : 'true'; + imageElement.src = primaryHeroImageUrl; imageElement.alt = artist.name; imageElement.style.display = "block"; if (fallbackElement) { @@ -1388,11 +1409,14 @@ function updateArtistHeroSection(artist, discography) { }; imageElement.onerror = () => { - console.error(`❌ Failed to load artist image: ${artist.image_url}`); - // Try Deezer fallback before emoji + console.error(`❌ Failed to load artist image: ${imageElement.src}`); + // Try Deezer fallback, then release art, before the generic icon. if (artist.deezer_id && !imageElement.dataset.triedDeezer) { imageElement.dataset.triedDeezer = 'true'; imageElement.src = `https://api.deezer.com/artist/${artist.deezer_id}/image?size=big`; + } else if (releaseImageUrl && imageElement.src !== releaseImageUrl && !imageElement.dataset.triedReleaseFallback) { + imageElement.dataset.triedReleaseFallback = 'true'; + imageElement.src = releaseImageUrl; } else { imageElement.style.display = "none"; if (fallbackElement) {