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') {