From 30e9db1a2bb20a0082d63f556f657120acb6aa65 Mon Sep 17 00:00:00 2001 From: cucadmuh <49571317+cucadmuh@users.noreply.github.com> Date: Mon, 8 Jun 2026 00:45:32 +0300 Subject: [PATCH] fix(artists): per-artist links on song rails and shared OpenSubsonic refs (#1023) * fix(artists): per-artist links on song rails and shared OpenSubsonic refs Song cards in Random Picks and Discover Songs showed joined artist credits but navigated to a single artistId. Route track surfaces through resolveTrackArtistRefs and coerce single-object Subsonic JSON payloads. * docs(changelog): note song-rail multi-artist link fix (PR #1023) --- CHANGELOG.md | 9 ++++ src/components/PlayerBar.tsx | 6 ++- src/components/SongCard.test.tsx | 43 +++++++++++++++++++ src/components/SongCard.tsx | 31 ++++++------- src/components/SongRow.tsx | 8 +--- src/components/albumTrackList/TrackRow.tsx | 5 +-- src/components/favorites/FavoriteSongRow.tsx | 12 +++++- .../playlist/PlaylistArtistCell.tsx | 7 ++- src/components/tracks/TracksPageChrome.tsx | 29 ++++++------- .../album/deriveAlbumHeaderArtistRefs.test.ts | 8 ++++ .../album/deriveAlbumHeaderArtistRefs.ts | 14 +++--- src/utils/openArtistRefs.test.ts | 19 ++++++++ src/utils/openArtistRefs.ts | 11 +++++ src/utils/playback/songToTrack.ts | 6 ++- src/utils/playback/trackArtistRefs.test.ts | 9 ++++ src/utils/playback/trackArtistRefs.ts | 6 ++- 16 files changed, 167 insertions(+), 56 deletions(-) create mode 100644 src/components/SongCard.test.tsx create mode 100644 src/utils/openArtistRefs.test.ts create mode 100644 src/utils/openArtistRefs.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 8aaa3e0e..835747a7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -144,6 +144,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 +### Song rails โ€” multi-artist credits link to each artist + +**By [@cucadmuh](https://github.com/cucadmuh), reported by zunoz on Discord, PR [#1023](https://github.com/Psychotoxical/psysonic/pull/1023)** + +* **Random Picks**, **Discover Songs**, and other song cards now split OpenSubsonic `artists[]` into individually clickable names โ€” the same behaviour as album track rows and the player bar, instead of one link for the whole joined credit string. +* Album cards and the rest of the app share the same artist-ref helper, including when Subsonic returns a single ref object instead of a one-element array. + + + ## [1.47.0] > **๐Ÿ™ Thank you to our amazing Discord community.** This release would not have been possible without your tireless support, quality checks, bug reports and all-round collaboration. Every report, every repro and every bit of feedback shaped what shipped here โ€” thank you. Come join us: [discord.gg/AMnDRErm4u](https://discord.gg/AMnDRErm4u) diff --git a/src/components/PlayerBar.tsx b/src/components/PlayerBar.tsx index 34f7036d..e4b13415 100644 --- a/src/components/PlayerBar.tsx +++ b/src/components/PlayerBar.tsx @@ -28,6 +28,8 @@ import PlaybackScheduleBadge from './PlaybackScheduleBadge'; import { usePlaybackScheduleRemaining } from '../utils/format/playbackScheduleFormat'; import { usePreviewStore } from '../store/previewStore'; import { usePerfProbeFlags } from '../utils/perf/perfFlags'; +import { coerceOpenArtistRefs } from '../utils/openArtistRefs'; +import { resolveTrackArtistRefs } from '../utils/playback/trackArtistRefs'; import { formatTrackTime } from '../utils/format/formatDuration'; import { PlayerTrackInfo } from './playerBar/PlayerTrackInfo'; import { PlayerTransportControls } from './playerBar/PlayerTransportControls'; @@ -146,8 +148,8 @@ export default function PlayerBar() { const showPreviewMeta = isPreviewing && !isRadio && previewingTrack !== null; const displayTitle = showPreviewMeta ? previewingTrack!.title : (currentTrack?.title ?? t('player.noTitle')); const displayArtist = showPreviewMeta ? previewingTrack!.artist : (currentTrack?.artist ?? 'โ€”'); - const displayArtistRefs = !showPreviewMeta && currentTrack?.artists && currentTrack.artists.length > 0 - ? currentTrack.artists + const displayArtistRefs = !showPreviewMeta && currentTrack && coerceOpenArtistRefs(currentTrack.artists).length > 0 + ? resolveTrackArtistRefs(currentTrack) : undefined; const coverArtId = showPreviewMeta diff --git a/src/components/SongCard.test.tsx b/src/components/SongCard.test.tsx new file mode 100644 index 00000000..3cc41f9c --- /dev/null +++ b/src/components/SongCard.test.tsx @@ -0,0 +1,43 @@ +import { describe, expect, it, vi } from 'vitest'; +import { screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { renderWithProviders } from '../test/helpers/renderWithProviders'; +import SongCard from './SongCard'; +import type { SubsonicSong } from '../api/subsonicTypes'; + +const navigateToArtist = vi.fn(); + +vi.mock('../hooks/useNavigateToArtist', () => ({ + useNavigateToArtist: () => navigateToArtist, +})); + +vi.mock('../cover/useLibraryCoverRef', () => ({ + useTrackCoverRef: () => undefined, +})); + +function song(overrides: Partial): SubsonicSong { + return { + id: 's1', title: 'Track', artist: 'A', album: 'Alb', albumId: 'al1', duration: 100, + ...overrides, + } as SubsonicSong; +} + +describe('SongCard', () => { + it('splits OpenSubsonic artists into individual links', async () => { + navigateToArtist.mockClear(); + const user = userEvent.setup(); + renderWithProviders( + , + ); + expect(screen.getByText('Apocalyptica')).toHaveClass('track-artist-link'); + expect(screen.getByText('Joe Duplantier')).toHaveClass('track-artist-link'); + await user.click(screen.getByText('Joe Duplantier')); + expect(navigateToArtist).toHaveBeenCalledWith('a2'); + }); +}); diff --git a/src/components/SongCard.tsx b/src/components/SongCard.tsx index d9363bca..3d9ede7c 100644 --- a/src/components/SongCard.tsx +++ b/src/components/SongCard.tsx @@ -1,7 +1,6 @@ import type { SubsonicSong } from '../api/subsonicTypes'; import { songToTrack } from '../utils/playback/songToTrack'; -import React, { memo } from 'react'; -import { useNavigate } from 'react-router-dom'; +import React, { memo, useMemo } from 'react'; import { Play, ListPlus, Star, Disc3 } from 'lucide-react'; import { useTranslation } from 'react-i18next'; import { usePlayerStore } from '../store/playerStore'; @@ -13,6 +12,9 @@ import { enqueueAndPlay } from '../utils/playback/playSong'; import { useDragDrop } from '../contexts/DragDropContext'; import { useOrbitSongRowBehavior } from '../hooks/useOrbitSongRowBehavior'; import { useNavigateToAlbum } from '../hooks/useNavigateToAlbum'; +import { useNavigateToArtist } from '../hooks/useNavigateToArtist'; +import { OpenArtistRefInline } from './OpenArtistRefInline'; +import { resolveTrackArtistRefs } from '../utils/playback/trackArtistRefs'; interface SongCardProps { song: SubsonicSong; @@ -31,7 +33,7 @@ function SongCard({ }: SongCardProps) { const layoutPx = artworkSize ?? displayCssPx; const { t } = useTranslation(); - const navigate = useNavigate(); + const navigateToArtist = useNavigateToArtist(); const openContextMenu = usePlayerStore(s => s.openContextMenu); const enqueue = usePlayerStore(s => s.enqueue); const coverRef = useTrackCoverRef(song, undefined, { libraryResolve: false }); @@ -55,12 +57,7 @@ function SongCard({ }; const handleClick = handlePlay; - - const handleArtistClick = (e: React.MouseEvent) => { - if (!song.artistId) return; - e.stopPropagation(); - navigate(`/artist/${song.artistId}`); - }; + const artistRefs = useMemo(() => resolveTrackArtistRefs(song), [song]); const handleAlbumClick = (e: React.MouseEvent) => { if (!song.albumId) return; @@ -142,12 +139,16 @@ function SongCard({

{song.title}

-

{song.artist}

+

+ navigateToArtist(id)} + as="none" + linkTag="span" + linkClassName="track-artist-link" + /> +

{song.albumId && (
); case 'artist': { - const artistRefs = song.artists && song.artists.length > 0 - ? song.artists - : [{ id: song.artistId, name: song.artist }]; + const artistRefs = resolveTrackArtistRefs(song); return (
{artistRefs.map((a, i) => ( diff --git a/src/components/favorites/FavoriteSongRow.tsx b/src/components/favorites/FavoriteSongRow.tsx index 540c39b3..9ca2bff7 100644 --- a/src/components/favorites/FavoriteSongRow.tsx +++ b/src/components/favorites/FavoriteSongRow.tsx @@ -8,6 +8,8 @@ import { formatLastSeen } from '../../utils/componentHelpers/userMgmtHelpers'; import i18n from '../../i18n'; import { formatTrackTime } from '../../utils/format/formatDuration'; import StarRating from '../StarRating'; +import { OpenArtistRefInline } from '../OpenArtistRefInline'; +import { resolveTrackArtistRefs } from '../../utils/playback/trackArtistRefs'; export interface FavoriteSongRowCallbacks { activate: (song: SubsonicSong, index: number, e: React.MouseEvent) => void; @@ -100,7 +102,15 @@ function FavoriteSongRow({ ); case 'artist': return (
- { if (song.artistId) { e.stopPropagation(); cb.navArtist(song.artistId, song.serverId); } }}>{song.artist} + cb.navArtist(id, song.serverId)} + as="none" + linkTag="span" + linkClassName="track-artist track-artist-link" + separatorClassName="track-artist-sep" + />
); case 'album': return ( diff --git a/src/components/playlist/PlaylistArtistCell.tsx b/src/components/playlist/PlaylistArtistCell.tsx index 04a9779f..32511f92 100644 --- a/src/components/playlist/PlaylistArtistCell.tsx +++ b/src/components/playlist/PlaylistArtistCell.tsx @@ -1,6 +1,7 @@ -import React from 'react'; +import React, { useMemo } from 'react'; import { useNavigate } from 'react-router-dom'; import type { SubsonicSong } from '../../api/subsonicTypes'; +import { resolveTrackArtistRefs } from '../../utils/playback/trackArtistRefs'; /** * Multi-artist credit for playlist track rows (main list + suggestions). @@ -11,9 +12,7 @@ import type { SubsonicSong } from '../../api/subsonicTypes'; */ export function PlaylistArtistCell({ song }: { song: SubsonicSong }) { const navigate = useNavigate(); - const artistRefs = song.artists && song.artists.length > 0 - ? song.artists - : [{ id: song.artistId, name: song.artist }]; + const artistRefs = useMemo(() => resolveTrackArtistRefs(song), [song]); return (
{artistRefs.map((a, i) => ( diff --git a/src/components/tracks/TracksPageChrome.tsx b/src/components/tracks/TracksPageChrome.tsx index 32b263ac..ae45ae32 100644 --- a/src/components/tracks/TracksPageChrome.tsx +++ b/src/components/tracks/TracksPageChrome.tsx @@ -13,6 +13,8 @@ import { ndListSongs, ndInvalidateSongsCache } from '../../api/navidromeBrowse'; import { usePerfProbeFlags } from '../../utils/perf/perfFlags'; import { useNavigateToAlbum } from '../../hooks/useNavigateToAlbum'; import { useNavigateToArtist } from '../../hooks/useNavigateToArtist'; +import { OpenArtistRefInline } from '../OpenArtistRefInline'; +import { resolveTrackArtistRefs } from '../../utils/playback/trackArtistRefs'; const RANDOM_RAIL_SIZE = 18; const RATED_RAIL_FETCH = 60; @@ -105,13 +107,7 @@ export default function TracksPageChrome({ [random, hero], ); - // Split a multi-artist feature into individually clickable links - // (OpenSubsonic `artists[]`), falling back to the single flat artist. - const heroArtistRefs = hero - ? (hero.artists && hero.artists.length > 0 - ? hero.artists - : [{ id: hero.artistId, name: hero.artist }]) - : []; + const heroArtistRefs = hero ? resolveTrackArtistRefs(hero) : []; return ( <> @@ -148,16 +144,15 @@ export default function TracksPageChrome({

{hero.title}

- {heroArtistRefs.map((a, i) => ( - - {i > 0 &&  ยท } - a.id && navigateToArtist(a.id)} - >{a.name ?? hero.artist} - - ))} + navigateToArtist(id)} + as="none" + linkTag="span" + linkClassName="track-artist-link" + separatorClassName="track-artist-sep" + /> {hero.album && ( <> ยท diff --git a/src/utils/album/deriveAlbumHeaderArtistRefs.test.ts b/src/utils/album/deriveAlbumHeaderArtistRefs.test.ts index 8ab03ed7..b2dac0f7 100644 --- a/src/utils/album/deriveAlbumHeaderArtistRefs.test.ts +++ b/src/utils/album/deriveAlbumHeaderArtistRefs.test.ts @@ -29,6 +29,14 @@ describe('deriveAlbumArtistRefs', () => { expect(deriveAlbumArtistRefs({ ...baseAlbum(), artistId: ' ', artist: 'Solo' })) .toEqual([{ name: 'Solo' }]); }); + + it('coerces a single-object OpenSubsonic artists payload', () => { + const album: SubsonicAlbum = { + ...baseAlbum(), + artists: { id: 'a1', name: 'Solo' } as unknown as SubsonicAlbum['artists'], + }; + expect(deriveAlbumArtistRefs(album)).toEqual([{ id: 'a1', name: 'Solo' }]); + }); }); describe('deriveAlbumHeaderArtistRefs', () => { diff --git a/src/utils/album/deriveAlbumHeaderArtistRefs.ts b/src/utils/album/deriveAlbumHeaderArtistRefs.ts index e2c88bab..be57de62 100644 --- a/src/utils/album/deriveAlbumHeaderArtistRefs.ts +++ b/src/utils/album/deriveAlbumHeaderArtistRefs.ts @@ -1,7 +1,8 @@ import type { SubsonicAlbum, SubsonicOpenArtistRef, SubsonicSong } from '../../api/subsonicTypes'; +import { coerceOpenArtistRefs } from '../openArtistRefs'; -function nonEmpty(refs: SubsonicOpenArtistRef[] | undefined): refs is SubsonicOpenArtistRef[] { - return !!refs && refs.length > 0; +function nonEmpty(refs: SubsonicOpenArtistRef[]): refs is SubsonicOpenArtistRef[] { + return refs.length > 0; } /** @@ -10,7 +11,8 @@ function nonEmpty(refs: SubsonicOpenArtistRef[] | undefined): refs is SubsonicOp * OpenSubsonic `artists` array; falls back to legacy `artist` + `artistId`. */ export function deriveAlbumArtistRefs(album: SubsonicAlbum): SubsonicOpenArtistRef[] { - if (nonEmpty(album.artists)) return album.artists; + const albumArtists = coerceOpenArtistRefs(album.artists); + if (nonEmpty(albumArtists)) return albumArtists; const name = album.artist?.trim() || 'โ€”'; const id = album.artistId?.trim(); return id ? [{ id, name }] : [{ name }]; @@ -26,9 +28,11 @@ export function deriveAlbumHeaderArtistRefs( album: SubsonicAlbum, songs: SubsonicSong[], ): SubsonicOpenArtistRef[] { - if (nonEmpty(album.artists)) return album.artists; + const albumArtists = coerceOpenArtistRefs(album.artists); + if (nonEmpty(albumArtists)) return albumArtists; for (const s of songs) { - if (nonEmpty(s.albumArtists)) return s.albumArtists; + const songAlbumArtists = coerceOpenArtistRefs(s.albumArtists); + if (nonEmpty(songAlbumArtists)) return songAlbumArtists; } return deriveAlbumArtistRefs(album); } diff --git a/src/utils/openArtistRefs.test.ts b/src/utils/openArtistRefs.test.ts new file mode 100644 index 00000000..7516c045 --- /dev/null +++ b/src/utils/openArtistRefs.test.ts @@ -0,0 +1,19 @@ +import { describe, expect, it } from 'vitest'; +import { coerceOpenArtistRefs } from './openArtistRefs'; + +describe('coerceOpenArtistRefs', () => { + it('returns an empty array for nullish input', () => { + expect(coerceOpenArtistRefs(undefined)).toEqual([]); + expect(coerceOpenArtistRefs(null)).toEqual([]); + }); + + it('passes through arrays', () => { + const refs = [{ id: 'a1', name: 'One' }, { id: 'a2', name: 'Two' }]; + expect(coerceOpenArtistRefs(refs)).toBe(refs); + }); + + it('wraps a single ref object from Subsonic JSON', () => { + const ref = { id: 'a1', name: 'Solo' }; + expect(coerceOpenArtistRefs(ref)).toEqual([ref]); + }); +}); diff --git a/src/utils/openArtistRefs.ts b/src/utils/openArtistRefs.ts new file mode 100644 index 00000000..6e6b19e6 --- /dev/null +++ b/src/utils/openArtistRefs.ts @@ -0,0 +1,11 @@ +import type { SubsonicOpenArtistRef } from '../api/subsonicTypes'; + +/** Subsonic JSON may return one ref object instead of a one-element array. */ +export function coerceOpenArtistRefs( + refs: SubsonicOpenArtistRef[] | SubsonicOpenArtistRef | undefined | null, +): SubsonicOpenArtistRef[] { + if (refs == null) return []; + if (Array.isArray(refs)) return refs; + if (typeof refs === 'object') return [refs]; + return []; +} diff --git a/src/utils/playback/songToTrack.ts b/src/utils/playback/songToTrack.ts index 1e835782..16f033f9 100644 --- a/src/utils/playback/songToTrack.ts +++ b/src/utils/playback/songToTrack.ts @@ -1,5 +1,6 @@ import type { SubsonicSong } from '../../api/subsonicTypes'; import type { Track } from '../../store/playerStoreTypes'; +import { coerceOpenArtistRefs } from '../openArtistRefs'; import { activeServerProfileId } from './trackServerScope'; export function songToTrack(song: SubsonicSong): Track { @@ -10,7 +11,10 @@ export function songToTrack(song: SubsonicSong): Track { album: song.album, albumId: song.albumId, artistId: song.artistId, - artists: song.artists && song.artists.length > 0 ? song.artists : undefined, + artists: (() => { + const artists = coerceOpenArtistRefs(song.artists); + return artists.length > 0 ? artists : undefined; + })(), duration: song.duration, coverArt: song.coverArt, discNumber: song.discNumber, diff --git a/src/utils/playback/trackArtistRefs.test.ts b/src/utils/playback/trackArtistRefs.test.ts index a973e163..35da2767 100644 --- a/src/utils/playback/trackArtistRefs.test.ts +++ b/src/utils/playback/trackArtistRefs.test.ts @@ -1,4 +1,5 @@ import { describe, expect, it } from 'vitest'; +import type { Track } from '../../store/playerStoreTypes'; import { primaryTrackArtistRef, resolveTrackArtistRefs } from './trackArtistRefs'; describe('resolveTrackArtistRefs', () => { @@ -21,6 +22,14 @@ describe('resolveTrackArtistRefs', () => { it('returns name-only ref when no id', () => { expect(resolveTrackArtistRefs({ artist: 'Unknown' })).toEqual([{ name: 'Unknown' }]); }); + + it('coerces a single-object OpenSubsonic artists payload', () => { + expect(resolveTrackArtistRefs({ + artist: 'Joined', + artistId: 'legacy', + artists: { id: 'a1', name: 'Solo' } as unknown as Track['artists'], + })).toEqual([{ id: 'a1', name: 'Solo' }]); + }); }); describe('primaryTrackArtistRef', () => { diff --git a/src/utils/playback/trackArtistRefs.ts b/src/utils/playback/trackArtistRefs.ts index 82269003..6d41c24c 100644 --- a/src/utils/playback/trackArtistRefs.ts +++ b/src/utils/playback/trackArtistRefs.ts @@ -1,12 +1,14 @@ import type { SubsonicOpenArtistRef } from '../../api/subsonicTypes'; import type { Track } from '../../store/playerStoreTypes'; +import { coerceOpenArtistRefs } from '../openArtistRefs'; type TrackArtistFields = Pick; /** OpenSubsonic `artists` when present; else legacy `artistId` + `artist` (album track rows). */ export function resolveTrackArtistRefs(track: TrackArtistFields): SubsonicOpenArtistRef[] { - if (track.artists && track.artists.length > 0) { - return track.artists; + const structured = coerceOpenArtistRefs(track.artists); + if (structured.length > 0) { + return structured; } const id = track.artistId?.trim(); if (id) {