feat: artist-detail deep linking — /artist-detail/:source/:id
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
This commit is contained in:
parent
e061f12a05
commit
3a4017ea2b
12 changed files with 200 additions and 12 deletions
|
|
@ -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")
|
||||
|
|
|
|||
|
|
@ -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: {
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
},
|
||||
};
|
||||
|
|
|
|||
2
webui/src/platform/shell/globals.d.ts
vendored
2
webui/src/platform/shell/globals.d.ts
vendored
|
|
@ -23,6 +23,8 @@ declare global {
|
|||
pageId: ShellPageId,
|
||||
options?: {
|
||||
replace?: boolean;
|
||||
artistId?: string | number;
|
||||
artistSource?: string | null;
|
||||
},
|
||||
) => Promise<boolean>;
|
||||
};
|
||||
|
|
|
|||
|
|
@ -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();
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
|
|
|
|||
|
|
@ -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();
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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') {
|
||||
|
|
|
|||
Loading…
Reference in a new issue