diff --git a/webui/src/app/router.test.tsx b/webui/src/app/router.test.tsx index f347a5be..7f1921f9 100644 --- a/webui/src/app/router.test.tsx +++ b/webui/src/app/router.test.tsx @@ -60,6 +60,7 @@ function createShellBridge(overrides: Partial = {}): ShellBridge { resolveLegacyPath: vi.fn<(pathname: string) => ShellPageId | null>(() => 'search'), setActivePageChrome: vi.fn(), activateLegacyPath: vi.fn(), + navigateToArtistDetail: vi.fn(), showReactHost: vi.fn(), ...overrides, }; diff --git a/webui/src/platform/shell/bridge.test.ts b/webui/src/platform/shell/bridge.test.ts index 00dd88d4..a33f44cd 100644 --- a/webui/src/platform/shell/bridge.test.ts +++ b/webui/src/platform/shell/bridge.test.ts @@ -88,4 +88,13 @@ describe('bindWindowWebRouter', () => { replace: true, }); }); + + it('refuses artist detail navigation without an artist id', async () => { + const navigate = vi.fn().mockResolvedValue(undefined); + + bindWindowWebRouter({ navigate } as never); + + await expect(window.SoulSyncWebRouter?.navigateToPage('artist-detail', {} as never)).resolves.toBe(false); + expect(navigate).not.toHaveBeenCalled(); + }); }); diff --git a/webui/src/platform/shell/bridge.ts b/webui/src/platform/shell/bridge.ts index 1ca85768..1c9683f5 100644 --- a/webui/src/platform/shell/bridge.ts +++ b/webui/src/platform/shell/bridge.ts @@ -88,6 +88,9 @@ export function bindWindowWebRouter(router: AnyRouter) { async navigateToPage(pageId, options) { const route = getShellRouteByPageId(pageId); if (!route) return false; + if (pageId === 'artist-detail' && !options?.artistId) { + return false; + } let href: `/${string}` = route.path; if (pageId === 'artist-detail' && options?.artistId) { diff --git a/webui/src/platform/shell/globals.d.ts b/webui/src/platform/shell/globals.d.ts index 27de813c..241464d3 100644 --- a/webui/src/platform/shell/globals.d.ts +++ b/webui/src/platform/shell/globals.d.ts @@ -35,6 +35,15 @@ declare global { resolveLegacyPath: (pathname: string) => ShellPageId | null; setActivePageChrome: (pageId: ShellPageId) => void; activateLegacyPath: (pathname: string) => void; + navigateToArtistDetail: ( + artistId: string | number, + artistName: string, + sourceOverride?: string | null, + options?: { + skipOriginPush?: boolean; + skipRouteChange?: boolean; + }, + ) => void; showReactHost: (pageId: ShellPageId) => void; }; } diff --git a/webui/src/platform/shell/route-manifest.test.ts b/webui/src/platform/shell/route-manifest.test.ts index 4430dd11..369f7b3d 100644 --- a/webui/src/platform/shell/route-manifest.test.ts +++ b/webui/src/platform/shell/route-manifest.test.ts @@ -16,7 +16,7 @@ describe('shellRouteManifest', () => { expect(resolveShellPageFromPath('/discover')).toBe('discover'); expect(resolveShellPageFromPath('/watchlist')).toBe('watchlist'); expect(resolveShellPageFromPath('/active-downloads')).toBe('active-downloads'); - expect(resolveShellPageFromPath('/artist-detail')).toBe('artist-detail'); + expect(resolveShellPageFromPath('/artist-detail')).toBeNull(); expect(resolveShellPageFromPath('/artist-detail/spotify/2YZyLoL8N0Wb9xBt1NhZWg')).toBe('artist-detail'); expect(resolveShellPageFromPath('/artists')).toBeNull(); }); @@ -50,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')).toBeNull(); 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 6b1a209e..469d46d1 100644 --- a/webui/src/platform/shell/route-manifest.ts +++ b/webui/src/platform/shell/route-manifest.ts @@ -72,7 +72,10 @@ 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/')) { + if (normalized === '/artist-detail') { + return null; + } + if (normalized.startsWith('/artist-detail/')) { return 'artist-detail'; } return getShellRouteByPath(pathname)?.pageId ?? null; @@ -80,7 +83,10 @@ export function resolveShellPageFromPath(pathname: string): ShellPageId | null { export function resolveLegacyShellPageFromPath(pathname: string): ShellPageId | null { const normalized = normalizeShellPath(pathname); - if (normalized === '/artist-detail' || normalized.startsWith('/artist-detail/')) { + if (normalized === '/artist-detail') { + return null; + } + if (normalized.startsWith('/artist-detail/')) { return 'artist-detail'; } const route = getShellRouteByPath(pathname); diff --git a/webui/src/routeTree.gen.ts b/webui/src/routeTree.gen.ts index d90bb0ce..ca94e35b 100644 --- a/webui/src/routeTree.gen.ts +++ b/webui/src/routeTree.gen.ts @@ -12,6 +12,7 @@ import { Route as rootRouteImport } from './routes/__root' import { Route as SplatRouteImport } from './routes/$' import { Route as IssuesRouteRouteImport } from './routes/issues/route' import { Route as IndexRouteImport } from './routes/index' +import { Route as ArtistDetailSourceIdRouteImport } from './routes/artist-detail/$source/$id' const SplatRoute = SplatRouteImport.update({ id: '/$', @@ -28,35 +29,44 @@ const IndexRoute = IndexRouteImport.update({ path: '/', getParentRoute: () => rootRouteImport, } as any) +const ArtistDetailSourceIdRoute = ArtistDetailSourceIdRouteImport.update({ + id: '/artist-detail/$source/$id', + path: '/artist-detail/$source/$id', + getParentRoute: () => rootRouteImport, +} as any) export interface FileRoutesByFullPath { '/': typeof IndexRoute '/issues': typeof IssuesRouteRoute '/$': typeof SplatRoute + '/artist-detail/$source/$id': typeof ArtistDetailSourceIdRoute } export interface FileRoutesByTo { '/': typeof IndexRoute '/issues': typeof IssuesRouteRoute '/$': typeof SplatRoute + '/artist-detail/$source/$id': typeof ArtistDetailSourceIdRoute } export interface FileRoutesById { __root__: typeof rootRouteImport '/': typeof IndexRoute '/issues': typeof IssuesRouteRoute '/$': typeof SplatRoute + '/artist-detail/$source/$id': typeof ArtistDetailSourceIdRoute } export interface FileRouteTypes { fileRoutesByFullPath: FileRoutesByFullPath - fullPaths: '/' | '/issues' | '/$' + fullPaths: '/' | '/issues' | '/$' | '/artist-detail/$source/$id' fileRoutesByTo: FileRoutesByTo - to: '/' | '/issues' | '/$' - id: '__root__' | '/' | '/issues' | '/$' + to: '/' | '/issues' | '/$' | '/artist-detail/$source/$id' + id: '__root__' | '/' | '/issues' | '/$' | '/artist-detail/$source/$id' fileRoutesById: FileRoutesById } export interface RootRouteChildren { IndexRoute: typeof IndexRoute IssuesRouteRoute: typeof IssuesRouteRoute SplatRoute: typeof SplatRoute + ArtistDetailSourceIdRoute: typeof ArtistDetailSourceIdRoute } declare module '@tanstack/react-router' { @@ -82,6 +92,13 @@ declare module '@tanstack/react-router' { preLoaderRoute: typeof IndexRouteImport parentRoute: typeof rootRouteImport } + '/artist-detail/$source/$id': { + id: '/artist-detail/$source/$id' + path: '/artist-detail/$source/$id' + fullPath: '/artist-detail/$source/$id' + preLoaderRoute: typeof ArtistDetailSourceIdRouteImport + parentRoute: typeof rootRouteImport + } } } @@ -89,6 +106,7 @@ const rootRouteChildren: RootRouteChildren = { IndexRoute: IndexRoute, IssuesRouteRoute: IssuesRouteRoute, SplatRoute: SplatRoute, + ArtistDetailSourceIdRoute: ArtistDetailSourceIdRoute, } export const routeTree = rootRouteImport ._addFileChildren(rootRouteChildren) diff --git a/webui/src/routes/artist-detail/$source/$id.tsx b/webui/src/routes/artist-detail/$source/$id.tsx new file mode 100644 index 00000000..8ae2f2c9 --- /dev/null +++ b/webui/src/routes/artist-detail/$source/$id.tsx @@ -0,0 +1,27 @@ +import { createFileRoute } from '@tanstack/react-router'; +import { useLayoutEffect } from 'react'; + +import { useShellBridge } from '@/platform/shell/route-controllers'; + +export const Route = createFileRoute('/artist-detail/$source/$id')({ + component: ArtistDetailPage, +}); + +// Thin legacy handoff: TanStack owns the URL shape here, but the vanilla JS +// artist-detail page still renders the actual experience for now. +function ArtistDetailPage() { + const bridge = useShellBridge(); + const { source, id } = Route.useParams(); + + useLayoutEffect(() => { + if (!bridge) return; + + const normalizedSource = source.toLowerCase() === 'library' ? null : source.toLowerCase(); + bridge.navigateToArtistDetail(id, '', normalizedSource, { + skipOriginPush: true, + skipRouteChange: true, + }); + }, [bridge, id, source]); + + return null; +} diff --git a/webui/src/routes/artist-detail/-route.test.tsx b/webui/src/routes/artist-detail/-route.test.tsx new file mode 100644 index 00000000..0f96ac5c --- /dev/null +++ b/webui/src/routes/artist-detail/-route.test.tsx @@ -0,0 +1,76 @@ +import { createMemoryHistory } from '@tanstack/react-router'; +import { render, waitFor } from '@testing-library/react'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +import type { ShellBridge, ShellPageId } from '@/platform/shell/bridge'; + +import { createAppQueryClient } from '@/app/query-client'; +import { AppRouterProvider, createAppRouter } from '@/app/router'; + +function createShellBridge(overrides: Partial = {}): ShellBridge { + return { + getCurrentProfileContext: vi.fn(() => ({ profileId: 2, isAdmin: false })), + isPageAllowed: vi.fn(() => true), + getProfileHomePage: vi.fn<() => ShellPageId>(() => 'discover'), + resolveLegacyPath: vi.fn<(pathname: string) => ShellPageId | null>(() => 'artist-detail'), + setActivePageChrome: vi.fn(), + activateLegacyPath: vi.fn(), + navigateToArtistDetail: vi.fn(), + showReactHost: vi.fn(), + ...overrides, + }; +} + +function renderArtistDetailRoute(initialEntries = ['/artist-detail/library/42']) { + const queryClient = createAppQueryClient(); + const history = createMemoryHistory({ initialEntries }); + const router = createAppRouter({ history, queryClient }); + + return { + history, + router, + ...render(), + }; +} + +describe('artist-detail route', () => { + beforeEach(() => { + window.SoulSyncWebShellBridge = createShellBridge(); + }); + + afterEach(() => { + window.SoulSyncWebShellBridge = undefined; + }); + + it('hands off canonical artist-detail URLs to the legacy shell', async () => { + renderArtistDetailRoute(['/artist-detail/spotify/2YZyLoL8N0Wb9xBt1NhZWg']); + + await waitFor(() => { + expect(window.SoulSyncWebShellBridge?.navigateToArtistDetail).toHaveBeenCalledWith( + '2YZyLoL8N0Wb9xBt1NhZWg', + '', + 'spotify', + { + skipOriginPush: true, + skipRouteChange: true, + }, + ); + }); + }); + + it('normalizes library sources before handing off', async () => { + renderArtistDetailRoute(['/artist-detail/library/42']); + + await waitFor(() => { + expect(window.SoulSyncWebShellBridge?.navigateToArtistDetail).toHaveBeenCalledWith( + '42', + '', + null, + { + skipOriginPush: true, + skipRouteChange: true, + }, + ); + }); + }); +}); diff --git a/webui/src/routes/issues/-route.test.tsx b/webui/src/routes/issues/-route.test.tsx index 402966e1..547745ef 100644 --- a/webui/src/routes/issues/-route.test.tsx +++ b/webui/src/routes/issues/-route.test.tsx @@ -22,6 +22,7 @@ function createShellBridge(overrides: Partial = {}): ShellBridge { resolveLegacyPath: vi.fn<(pathname: string) => ShellPageId | null>(() => 'search'), setActivePageChrome: vi.fn(), activateLegacyPath: vi.fn(), + navigateToArtistDetail: vi.fn(), showReactHost: vi.fn(), ...overrides, }; diff --git a/webui/static/init.js b/webui/static/init.js index c874f6fc..f00e5852 100644 --- a/webui/static/init.js +++ b/webui/static/init.js @@ -2154,10 +2154,6 @@ function _getPageFromPath() { 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') { - // /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; } @@ -2168,35 +2164,13 @@ function _normalizeArtistDetailSource(source) { } function buildArtistDetailPath(artistId, source = null) { - if (!artistId) return '/artist-detail'; + if (!artistId) { + throw new Error('artistId is required for artist-detail navigation'); + } 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 // =============================== @@ -2310,6 +2284,10 @@ function navigateToPage(pageId, options = {}) { return; } + if (pageId === 'artist-detail' && !options.artistId) { + return false; + } + const router = getWebRouter(); if (router && !options.skipRouteChange) { notifyPageWillChange(pageId); @@ -2386,7 +2364,10 @@ async function loadPageData(pageId) { case 'library': // Check if we should return to artist detail view instead of list if (artistDetailPageState.currentArtistId && artistDetailPageState.currentArtistName) { - navigateToPage('artist-detail'); + navigateToPage('artist-detail', { + artistId: artistDetailPageState.currentArtistId, + artistSource: artistDetailPageState.currentArtistSource, + }); if (!artistDetailPageState.isInitialized) { initializeArtistDetailPage(); loadArtistDetailData(artistDetailPageState.currentArtistId, artistDetailPageState.currentArtistName); @@ -2400,7 +2381,7 @@ async function loadPageData(pageId) { } break; case 'artist-detail': - // Artist detail page is handled separately by navigateToArtistDetail() + // Artist detail page is entered through the route handoff and legacy navigator. break; case 'discover': if (!discoverPageInitialized) { diff --git a/webui/static/library.js b/webui/static/library.js index 19818b16..dfdc5fac 100644 --- a/webui/static/library.js +++ b/webui/static/library.js @@ -815,7 +815,7 @@ function navigateToArtistDetail(artistId, artistName, sourceOverride = null, opt navigateToPage('artist-detail', { artistId, artistSource: normalizedSource, - skipRouteChange: true + skipRouteChange: options.skipRouteChange === true }); _updateArtistDetailBackButtonLabel(); } diff --git a/webui/static/search.js b/webui/static/search.js index fd6d2604..682d48f0 100644 --- a/webui/static/search.js +++ b/webui/static/search.js @@ -1197,16 +1197,7 @@ async function loadInitialData() { return; } - 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 }); - } + navigateToPage(targetPage, { 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 8d2ac58a..64c73f6e 100644 --- a/webui/static/shell-bridge.js +++ b/webui/static/shell-bridge.js @@ -80,16 +80,6 @@ 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 }); } @@ -107,16 +97,6 @@ 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') { @@ -185,6 +165,7 @@ window.SoulSyncWebShellBridge = { activateLegacyPath(pathname) { activateLegacyPath(pathname); }, + navigateToArtistDetail, showReactHost(pageId) { showReactHost(pageId); },