fix(now-playing): keep metadata cards when the track plays from local cache (#1042)

* fix(now-playing): keep metadata cards when the track plays from local cache

The Now Playing metadata fetches (album, artist info, discography, top songs)
gated on shouldAttemptSubsonicForServer(serverId, trackId). That guard returns
false when the current track's audio resolves to a psysonic-local:// URL (hot
cache / offline bytes), which is correct for playback-byte calls but wrongly
suppressed the metadata calls. On track change the prefetched next track plays
from the hot cache, so every Subsonic-backed card blanked and only the Last.fm
bio fallback and Bandsintown tour (both non-Subsonic) remained.

Drop the trackId from the four Now Playing metadata gates so metadata is fetched
whenever the server is reachable, regardless of where the audio bytes come from.
True-offline is still handled by the online / reachability checks in the guard.
The guard and all other call sites are unchanged.

* docs: add CHANGELOG entry for PR #1042

* refactor(now-playing): single Subsonic network gate owned by the fetchers hook

The metadata reachability guard (shouldAttemptSubsonicForServer) ran 2-3
times per Now Playing render/effect cycle: callers folded it into
fetchEnabled in NowPlaying.tsx and useNowPlayingPrewarm on top of the
checks already inside useNowPlayingFetchers and prewarmNowPlayingFetchers.

Callers now pass fetchEnabled as intent only ("we have a playback server
id"); the single reachability decision lives in the hook / prewarm
function. Behaviour is unchanged — only the duplicated guard calls go.

* test(now-playing): cover local-playback metadata behaviour, not only call shape

Add a guard test proving the metadata gate (no trackId) bypasses the
psysonic-local:// skip while a byte-style call stays blocked, and a hook
test that fetches album/discography/top songs with a trackId-sensitive
guard — so a reintroduced trackId at the gate fails the suite instead of
silently blanking the cards.
This commit is contained in:
Psychotoxical
2026-06-09 11:31:01 +02:00
committed by GitHub
parent ad8e376c9c
commit a151cf5deb
6 changed files with 90 additions and 11 deletions
+52 -1
View File
@@ -18,9 +18,10 @@ vi.mock('../api/subsonicLibrary');
vi.mock('../api/bandsintown');
vi.mock('../api/lastfm');
vi.mock('../utils/network/subsonicNetworkGuard', () => ({
shouldAttemptSubsonicForServer: () => true,
shouldAttemptSubsonicForServer: vi.fn(() => true),
}));
import { shouldAttemptSubsonicForServer } from '../utils/network/subsonicNetworkGuard';
import { getArtistForServer, getArtistInfoForServer, getTopSongsForServer } from '../api/subsonicArtists';
import { getAlbumForServer, getSongForServer } from '../api/subsonicLibrary';
import { fetchBandsintownEvents } from '../api/bandsintown';
@@ -212,3 +213,53 @@ describe('useNowPlayingFetchers — id-gated songMeta / albumData / discography'
await waitFor(() => expect(result.current.discography.map(a => a.id)).toEqual(['al-D2']));
});
});
describe('useNowPlayingFetchers — local-playback metadata', () => {
// Regression: the metadata gate must never pass the playing track id, or the
// guard's `psysonic-local://` skip would blank every Subsonic card whenever
// the track plays from hot-cache / offline bytes. Guard is called with the
// server id only.
// Keep the shared guard mock at its permissive default after the behaviour
// case below swaps in a trackId-sensitive implementation.
afterEach(() => {
vi.mocked(shouldAttemptSubsonicForServer).mockImplementation(() => true);
});
it('queries the network guard without a trackId', async () => {
const guard = vi.mocked(shouldAttemptSubsonicForServer);
renderHook(() => useNowPlayingFetchers({ ...baseDeps, songId: 'song-1', albumId: 'al-1', artistId: 'art-1', artistName: 'Artist' }));
await waitFor(() => expect(guard).toHaveBeenCalled());
for (const call of guard.mock.calls) {
expect(call).toHaveLength(1);
expect(call[1]).toBeUndefined();
}
});
it('still loads album / discography / top songs when the playback bytes are local', async () => {
// Mirror the real guard: a byte-style call (with a trackId resolving to
// psysonic-local://) is blocked, but the metadata gate (server id only) is
// allowed. If the hook ever passed the trackId again, every fetch below
// would be gated off and the cards would blank — exactly the #1042 bug.
// Ids are unique to this test so the shared module caches don't short-circuit it.
vi.mocked(shouldAttemptSubsonicForServer).mockImplementation(
(_serverId, trackId) => trackId === undefined,
);
vi.mocked(getSongForServer).mockResolvedValue({ id: 'np-song', title: 'Local Track' } as SubsonicSong);
vi.mocked(getAlbumForServer).mockResolvedValue(
{ album: { id: 'np-al', name: 'Album' } as SubsonicAlbum, songs: [] } as any,
);
vi.mocked(getArtistForServer).mockResolvedValue({ albums: [{ id: 'np-al' }] } as any);
vi.mocked(getTopSongsForServer).mockResolvedValue([{ id: 'np-top' }] as any);
const { result } = renderHook(() =>
useNowPlayingFetchers({ ...baseDeps, songId: 'np-song', albumId: 'np-al', artistId: 'np-art', artistName: 'NP Artist' }),
);
await waitFor(() => expect(getAlbumForServer).toHaveBeenCalledWith('srv1', 'np-al'));
await waitFor(() => expect(getArtistForServer).toHaveBeenCalledWith('srv1', 'np-art'));
await waitFor(() => expect(getTopSongsForServer).toHaveBeenCalledWith('srv1', 'NP Artist'));
await waitFor(() => expect(result.current.albumData?.album.id).toBe('np-al'));
await waitFor(() => expect(result.current.discography.map(a => a.id)).toEqual(['np-al']));
await waitFor(() => expect(result.current.topSongs.map(s => s.id)).toEqual(['np-top']));
});
});
+13 -3
View File
@@ -32,7 +32,12 @@ export interface NowPlayingFetchersDeps {
currentTrack: { artist: string; title: string } | null;
/** Subsonic server for API calls — must match the playing queue server. */
subsonicServerId: string;
/** When false, skip network fetches (e.g. no server id). */
/**
* Caller intent / prerequisites only (e.g. "we have a playback server id").
* The network reachability decision — online, server reachable, and no
* trackId so local-cache playback still loads metadata — is made here via
* `shouldAttemptSubsonicForServer`; callers must not pre-apply that guard.
*/
fetchEnabled?: boolean;
}
@@ -81,7 +86,10 @@ export async function prewarmNowPlayingFetchers(
} = deps;
if (!fetchEnabled || !subsonicServerId) return;
if (!shouldAttemptSubsonicForServer(subsonicServerId, songId)) return;
// The only network gate in this function. No trackId: metadata must be fetched
// even when the track's audio is local (hot-cache / offline). Offline is still
// covered by the online/reachability checks inside the guard.
if (!shouldAttemptSubsonicForServer(subsonicServerId)) return;
const jobs: Array<Promise<unknown>> = [];
@@ -200,9 +208,11 @@ export function useNowPlayingFetchers(deps: NowPlayingFetchersDeps): NowPlayingF
seedKeySlot(lfmArtistKey, k => lfmArtistCache.get(k)));
const { status: connStatus } = useConnectionStatus();
// No trackId: see prewarm note — metadata is fetched whenever the server is
// reachable, regardless of whether the track's audio plays from local cache.
const subsonicFetchAllowed = fetchEnabled
&& !!subsonicServerId
&& shouldAttemptSubsonicForServer(subsonicServerId, songId);
&& shouldAttemptSubsonicForServer(subsonicServerId);
// Fetch batch per entity change (not per song switch — same-artist songs share artist/top/tour fetches)
useEffect(() => {
+3 -2
View File
@@ -13,7 +13,6 @@ import { useAuthStore } from '../store/authStore';
import { usePlayerStore } from '../store/playerStore';
import { usePlaybackServerId } from './usePlaybackServerId';
import { primaryTrackArtistRef } from '../utils/playback/trackArtistRefs';
import { shouldAttemptSubsonicForServer } from '../utils/network/subsonicNetworkGuard';
const NOW_PLAYING_COVER_CSS_PX = 800;
@@ -65,7 +64,9 @@ export function useNowPlayingPrewarm(): void {
lastfmUsername,
currentTrack,
subsonicServerId: playbackServerId,
fetchEnabled: shouldAttemptSubsonicForServer(playbackServerId, currentTrack.id),
// No `fetchEnabled` / no trackId: prewarmNowPlayingFetchers owns the single
// reachability gate, and metadata must warm even when the track's audio
// plays from local cache.
});
if (currentTrack.albumId && currentTrack.id) {