mirror of
https://github.com/Psychotoxical/psysonic.git
synced 2026-07-22 23:35:44 +00:00
fix(song-info): preserve track server ownership
This commit is contained in:
@@ -160,6 +160,19 @@ describe('ContextMenu — type=song', () => {
|
||||
fireEvent.click(getByText('Play Next'));
|
||||
expect(usePlayerStore.getState().contextMenu.isOpen).toBe(false);
|
||||
});
|
||||
|
||||
it('opens Song Info with the context track owner', () => {
|
||||
openMenuFor('song', makeTrack({ id: 'shared', serverId: 'srv-owner' }));
|
||||
const { getByText } = renderWithProviders(<ContextMenu />);
|
||||
|
||||
fireEvent.click(getByText('Song Info'));
|
||||
|
||||
expect(usePlayerStore.getState().songInfoModal).toEqual({
|
||||
isOpen: true,
|
||||
songId: 'shared',
|
||||
serverId: 'srv-owner',
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('ContextMenu — type=album', () => {
|
||||
|
||||
@@ -9,6 +9,7 @@ import StarRating from '@/ui/StarRating';
|
||||
import { AddToPlaylistSubmenu } from '@/features/contextMenu/components/AddToPlaylistSubmenu';
|
||||
import type { ContextMenuItemsProps } from '@/features/contextMenu/components/contextMenuItemTypes';
|
||||
import { buildArtistDetailPath } from '@/lib/navigation/detailServerScope';
|
||||
import { buildAlbumDetailPath } from '@/lib/navigation/detailServerScope';
|
||||
import { ownedEntityKey } from '@/lib/util/ownedEntityKey';
|
||||
|
||||
export default function QueueItemContextItems(props: ContextMenuItemsProps) {
|
||||
@@ -58,7 +59,9 @@ export default function QueueItemContextItems(props: ContextMenuItemsProps) {
|
||||
</div>
|
||||
<div className="context-menu-divider" />
|
||||
{song.albumId && (
|
||||
<div className="context-menu-item" onClick={() => handleAction(() => navigateLibrary(`/album/${song.albumId}`))}>
|
||||
<div className="context-menu-item" onClick={() => handleAction(() => navigateLibrary(
|
||||
buildAlbumDetailPath(song.albumId!, { serverId: song.serverId }),
|
||||
))}>
|
||||
<Disc3 size={14} /> {t('contextMenu.openAlbum')}
|
||||
</div>
|
||||
)}
|
||||
@@ -114,10 +117,10 @@ export default function QueueItemContextItems(props: ContextMenuItemsProps) {
|
||||
/>
|
||||
</div>
|
||||
<div className="context-menu-divider" />
|
||||
<div className="context-menu-item" onClick={() => handleAction(() => copyShareLink('track', song.id))}>
|
||||
<div className="context-menu-item" onClick={() => handleAction(() => copyShareLink('track', song.id, song.serverId))}>
|
||||
<Share2 size={14} /> {t('contextMenu.shareLink')}
|
||||
</div>
|
||||
<div className="context-menu-item" onClick={() => handleAction(() => openSongInfo(song.id))}>
|
||||
<div className="context-menu-item" onClick={() => handleAction(() => openSongInfo(song.id, song.serverId))}>
|
||||
<Info size={14} /> {t('contextMenu.songInfo')}
|
||||
</div>
|
||||
</>
|
||||
|
||||
@@ -180,7 +180,7 @@ export default function SongContextItems(props: ContextMenuItemsProps) {
|
||||
<div className="context-menu-item" onClick={() => handleAction(() => copyShareLink('track', song.id, song.serverId))}>
|
||||
<Share2 size={14} /> {t('contextMenu.shareLink')}
|
||||
</div>
|
||||
<div className="context-menu-item" onClick={() => handleAction(() => openSongInfo(song.id))}>
|
||||
<div className="context-menu-item" onClick={() => handleAction(() => openSongInfo(song.id, song.serverId))}>
|
||||
<Info size={14} /> {t('contextMenu.songInfo')}
|
||||
</div>
|
||||
{offlinePolicy.canEditPlaylist && playlistId && playlistSongIndex !== undefined && playlistSongRemove && (
|
||||
@@ -261,7 +261,10 @@ export default function SongContextItems(props: ContextMenuItemsProps) {
|
||||
)}
|
||||
<div className="context-menu-divider" />
|
||||
{song.albumId && (
|
||||
<div className="context-menu-item" onClick={() => handleAction(() => navigateToAlbum(song.albumId!))}>
|
||||
<div className="context-menu-item" onClick={() => handleAction(() => navigateToAlbum(
|
||||
song.albumId!,
|
||||
{ search: appendServerQuery(undefined, song.serverId) },
|
||||
))}>
|
||||
<Disc3 size={14} /> {t('contextMenu.openAlbum')}
|
||||
</div>
|
||||
)}
|
||||
@@ -314,10 +317,10 @@ export default function SongContextItems(props: ContextMenuItemsProps) {
|
||||
</div>
|
||||
)}
|
||||
<div className="context-menu-divider" />
|
||||
<div className="context-menu-item" onClick={() => handleAction(() => copyShareLink('track', song.id))}>
|
||||
<div className="context-menu-item" onClick={() => handleAction(() => copyShareLink('track', song.id, song.serverId))}>
|
||||
<Share2 size={14} /> {t('contextMenu.shareLink')}
|
||||
</div>
|
||||
<div className="context-menu-item" onClick={() => handleAction(() => openSongInfo(song.id))}>
|
||||
<div className="context-menu-item" onClick={() => handleAction(() => openSongInfo(song.id, song.serverId))}>
|
||||
<Info size={14} /> {t('contextMenu.songInfo')}
|
||||
</div>
|
||||
{offlinePolicy.canFavorite && (
|
||||
|
||||
@@ -33,7 +33,7 @@ export interface ContextMenuItemsProps {
|
||||
setStarredOverride: (id: string, starred: boolean) => void;
|
||||
networkLovedCache: Record<string, boolean>;
|
||||
setNetworkLovedForSong: (title: string, artist: string, loved: boolean) => void;
|
||||
openSongInfo: (id: string) => void;
|
||||
openSongInfo: (id: string, serverId?: string) => void;
|
||||
userRatingOverrides: Record<string, number>;
|
||||
setKeyboardRating: React.Dispatch<React.SetStateAction<KeyboardRating | null>>;
|
||||
keyboardRating: KeyboardRating | null;
|
||||
|
||||
@@ -6,6 +6,7 @@ import { useTranslation } from 'react-i18next';
|
||||
import { Play, Trash2, Disc3, User, Heart, Info } from 'lucide-react';
|
||||
import type { MiniTrackInfo } from '@/features/miniPlayer/utils/miniPlayerBridge';
|
||||
import { buildArtistDetailPath } from '@/lib/navigation/detailServerScope';
|
||||
import { buildAlbumDetailPath } from '@/lib/navigation/detailServerScope';
|
||||
|
||||
interface Props {
|
||||
x: number;
|
||||
@@ -94,7 +95,9 @@ export default function MiniContextMenu({ x, y, track, index, onClose }: Props)
|
||||
{track.albumId && (
|
||||
<div
|
||||
className="context-menu-item"
|
||||
onClick={() => run(() => emit('mini:navigate', { to: `/album/${track.albumId}` }))}
|
||||
onClick={() => run(() => emit('mini:navigate', {
|
||||
to: buildAlbumDetailPath(track.albumId!, { serverId: track.serverId }),
|
||||
}))}
|
||||
>
|
||||
<Disc3 size={14} /> {t('contextMenu.openAlbum')}
|
||||
</div>
|
||||
@@ -116,7 +119,7 @@ export default function MiniContextMenu({ x, y, track, index, onClose }: Props)
|
||||
<div className="context-menu-divider" />
|
||||
<div
|
||||
className="context-menu-item"
|
||||
onClick={() => run(() => emit('mini:song-info', { id: track.id }))}
|
||||
onClick={() => run(() => emit('mini:song-info', { id: track.id, serverId: track.serverId }))}
|
||||
>
|
||||
<Info size={14} /> {t('contextMenu.songInfo')}
|
||||
</div>
|
||||
|
||||
@@ -239,14 +239,14 @@ export function initMiniPlayerBridgeOnMain(): () => void {
|
||||
});
|
||||
|
||||
// Open the SongInfo modal in main for a given track id.
|
||||
const songInfoUnlisten = listen<{ id: string }>('mini:song-info', (e) => {
|
||||
const songInfoUnlisten = listen<{ id: string; serverId?: string }>('mini:song-info', (e) => {
|
||||
const id = e.payload?.id;
|
||||
if (!id) return;
|
||||
const w = getCurrentWindow();
|
||||
w.unminimize().catch(() => {});
|
||||
w.show().catch(() => {});
|
||||
w.setFocus().catch(() => {});
|
||||
usePlayerStore.getState().openSongInfo(id);
|
||||
usePlayerStore.getState().openSongInfo(id, e.payload?.serverId);
|
||||
});
|
||||
|
||||
return () => {
|
||||
|
||||
@@ -0,0 +1,71 @@
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
import { waitFor } from '@testing-library/react';
|
||||
import { renderWithProviders } from '@/test/helpers/renderWithProviders';
|
||||
|
||||
const mocks = vi.hoisted(() => ({
|
||||
getSong: vi.fn(),
|
||||
getSongForServer: vi.fn(),
|
||||
libraryGetFacts: vi.fn(),
|
||||
ndGetSongPath: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock('@/lib/api/subsonicLibrary', () => ({
|
||||
getSong: mocks.getSong,
|
||||
getSongForServer: mocks.getSongForServer,
|
||||
}));
|
||||
vi.mock('@/lib/api/library', () => ({ libraryGetFacts: mocks.libraryGetFacts }));
|
||||
vi.mock('@/lib/api/navidromeAdmin', () => ({ ndGetSongPath: mocks.ndGetSongPath }));
|
||||
vi.mock('@/lib/library/libraryReady', () => ({ libraryIsReady: vi.fn(() => Promise.resolve(true)) }));
|
||||
vi.mock('@/store/libraryIndexStore', () => ({
|
||||
useLibraryIndexStore: { getState: () => ({ isIndexEnabled: () => true }) },
|
||||
}));
|
||||
|
||||
import SongInfoModal from './SongInfoModal';
|
||||
import { resetAuthStore, resetPlayerStore } from '@/test/helpers/storeReset';
|
||||
import { useAuthStore } from '@/store/authStore';
|
||||
import { usePlayerStore } from '@/features/playback/store/playerStore';
|
||||
|
||||
describe('SongInfoModal server ownership', () => {
|
||||
beforeEach(() => {
|
||||
resetAuthStore();
|
||||
resetPlayerStore();
|
||||
Object.values(mocks).forEach(mock => mock.mockReset());
|
||||
useAuthStore.setState({
|
||||
activeServerId: 'srv-active',
|
||||
servers: [
|
||||
{ id: 'srv-active', name: 'Active', url: 'https://active.test', username: 'a', password: 'p' },
|
||||
{ id: 'srv-owner', name: 'Owner', url: 'https://owner.test', username: 'owner', password: 'secret' },
|
||||
],
|
||||
subsonicServerIdentityByServer: {
|
||||
'srv-owner': { type: 'navidrome', serverVersion: '0.62.0', openSubsonic: true },
|
||||
},
|
||||
});
|
||||
mocks.getSongForServer.mockResolvedValue({
|
||||
id: 'shared',
|
||||
serverId: 'srv-owner',
|
||||
title: 'Owner Song',
|
||||
artist: 'Owner Artist',
|
||||
album: 'Owner Album',
|
||||
duration: 120,
|
||||
});
|
||||
mocks.libraryGetFacts.mockResolvedValue([]);
|
||||
mocks.ndGetSongPath.mockResolvedValue('/owner/music/song.flac');
|
||||
});
|
||||
|
||||
it('loads metadata, local facts, and native path from the captured owner', async () => {
|
||||
usePlayerStore.getState().openSongInfo('shared', 'srv-owner');
|
||||
const view = renderWithProviders(<SongInfoModal />);
|
||||
|
||||
expect(await view.findByText('Owner Song')).toBeInTheDocument();
|
||||
expect(mocks.getSongForServer).toHaveBeenCalledWith('srv-owner', 'shared');
|
||||
expect(mocks.getSong).not.toHaveBeenCalled();
|
||||
await waitFor(() => expect(mocks.libraryGetFacts).toHaveBeenCalledWith('srv-owner', 'shared'));
|
||||
expect(mocks.ndGetSongPath).toHaveBeenCalledWith(
|
||||
'https://owner.test',
|
||||
'owner',
|
||||
'secret',
|
||||
'shared',
|
||||
);
|
||||
expect(await view.findByText('/owner/music/song.flac')).toBeInTheDocument();
|
||||
});
|
||||
});
|
||||
@@ -1,4 +1,4 @@
|
||||
import { getSong } from '@/lib/api/subsonicLibrary';
|
||||
import { getSong, getSongForServer } from '@/lib/api/subsonicLibrary';
|
||||
import { libraryGetFacts } from '@/lib/api/library';
|
||||
import type { SubsonicSong } from '@/lib/api/subsonicTypes';
|
||||
import React, { useEffect, useState } from 'react';
|
||||
@@ -92,8 +92,11 @@ export default function SongInfoModal() {
|
||||
setEnrichment(null);
|
||||
setAbsolutePath(null);
|
||||
const songId = songInfoModal.songId;
|
||||
const ownerServerId = songInfoModal.serverId;
|
||||
void (async () => {
|
||||
const s = await getSong(songId);
|
||||
const s = ownerServerId
|
||||
? await getSongForServer(ownerServerId, songId)
|
||||
: await getSong(songId);
|
||||
if (cancelled) return;
|
||||
setSong(s);
|
||||
setLoading(false);
|
||||
@@ -101,8 +104,7 @@ export default function SongInfoModal() {
|
||||
setEnrichment(null);
|
||||
return;
|
||||
}
|
||||
const auth = useAuthStore.getState();
|
||||
const sid = auth.activeServerId;
|
||||
const sid = ownerServerId ?? useAuthStore.getState().activeServerId;
|
||||
const indexEnabled = sid ? useLibraryIndexStore.getState().isIndexEnabled(sid) : false;
|
||||
if (sid && indexEnabled && await libraryIsReady(sid)) {
|
||||
try {
|
||||
@@ -121,7 +123,7 @@ export default function SongInfoModal() {
|
||||
// and we have credentials. Failures are silent — modal falls back to
|
||||
// whatever the Subsonic `path` field carried (typically nothing).
|
||||
const auth = useAuthStore.getState();
|
||||
const sid = auth.activeServerId;
|
||||
const sid = ownerServerId ?? auth.activeServerId;
|
||||
const profile = sid ? auth.servers.find(p => p.id === sid) : null;
|
||||
const identity = sid ? auth.subsonicServerIdentityByServer[sid] : undefined;
|
||||
const isNavidrome = identity?.type?.trim().toLowerCase() === 'navidrome';
|
||||
@@ -132,7 +134,7 @@ export default function SongInfoModal() {
|
||||
});
|
||||
}
|
||||
return () => { cancelled = true; };
|
||||
}, [songInfoModal.isOpen, songInfoModal.songId]);
|
||||
}, [songInfoModal.isOpen, songInfoModal.songId, songInfoModal.serverId]);
|
||||
|
||||
useEffect(() => {
|
||||
if (!songInfoModal.isOpen) return;
|
||||
|
||||
@@ -119,8 +119,12 @@ describe('openContextMenu / closeContextMenu', () => {
|
||||
|
||||
describe('openSongInfo / closeSongInfo', () => {
|
||||
it('opens with the song id and clears on close', () => {
|
||||
usePlayerStore.getState().openSongInfo('song-1');
|
||||
expect(usePlayerStore.getState().songInfoModal).toEqual({ isOpen: true, songId: 'song-1' });
|
||||
usePlayerStore.getState().openSongInfo('song-1', 'srv-owner');
|
||||
expect(usePlayerStore.getState().songInfoModal).toEqual({
|
||||
isOpen: true,
|
||||
songId: 'song-1',
|
||||
serverId: 'srv-owner',
|
||||
});
|
||||
|
||||
usePlayerStore.getState().closeSongInfo();
|
||||
expect(usePlayerStore.getState().songInfoModal).toEqual({ isOpen: false, songId: null });
|
||||
|
||||
@@ -188,7 +188,7 @@ export interface PlayerState {
|
||||
) => void;
|
||||
closeContextMenu: () => void;
|
||||
|
||||
songInfoModal: { isOpen: boolean; songId: string | null };
|
||||
openSongInfo: (songId: string) => void;
|
||||
songInfoModal: { isOpen: boolean; songId: string | null; serverId?: string };
|
||||
openSongInfo: (songId: string, serverId?: string) => void;
|
||||
closeSongInfo: () => void;
|
||||
}
|
||||
|
||||
@@ -81,7 +81,7 @@ export function createUiStateActions(set: SetState): Pick<
|
||||
contextMenu: { ...state.contextMenu, isOpen: false },
|
||||
})),
|
||||
|
||||
openSongInfo: (songId) => set({ songInfoModal: { isOpen: true, songId } }),
|
||||
openSongInfo: (songId, serverId) => set({ songInfoModal: { isOpen: true, songId, serverId } }),
|
||||
closeSongInfo: () => set({ songInfoModal: { isOpen: false, songId: null } }),
|
||||
|
||||
toggleQueue: () =>
|
||||
|
||||
@@ -2,6 +2,7 @@ import { beforeEach, describe, expect, it } from 'vitest';
|
||||
import { useAuthStore } from '@/store/authStore';
|
||||
import {
|
||||
appendServerQuery,
|
||||
buildAlbumDetailPath,
|
||||
buildArtistDetailPath,
|
||||
readDetailServerId,
|
||||
} from '@/lib/navigation/detailServerScope';
|
||||
@@ -42,6 +43,13 @@ describe('detailServerScope', () => {
|
||||
})).toBe('/artist/art-1?lossless=1&server=srv-b&tab=albums');
|
||||
});
|
||||
|
||||
it('buildAlbumDetailPath preserves search and owning server', () => {
|
||||
expect(buildAlbumDetailPath('album-1', {
|
||||
serverId: 'srv-b',
|
||||
search: 'lossless=1',
|
||||
})).toBe('/album/album-1?lossless=1&server=srv-b');
|
||||
});
|
||||
|
||||
it('buildArtistDetailPath preserves an existing server when no owner is supplied', () => {
|
||||
expect(buildArtistDetailPath('art-1', { search: 'server=srv-a&lossless=1' }))
|
||||
.toBe('/artist/art-1?server=srv-a&lossless=1');
|
||||
|
||||
@@ -5,6 +5,17 @@ export interface ArtistDetailPathOptions {
|
||||
search?: string | URLSearchParams;
|
||||
}
|
||||
|
||||
/** Build an album detail path while preserving query parameters and owning server. */
|
||||
export function buildAlbumDetailPath(
|
||||
albumId: string,
|
||||
options: ArtistDetailPathOptions = {},
|
||||
): string {
|
||||
const params = new URLSearchParams(options.search ?? '');
|
||||
if (options.serverId) params.set('server', options.serverId);
|
||||
const query = params.toString();
|
||||
return `/album/${albumId}${query ? `?${query}` : ''}`;
|
||||
}
|
||||
|
||||
/** Build an artist detail path while preserving query parameters and owning server. */
|
||||
export function buildArtistDetailPath(
|
||||
artistId: string,
|
||||
|
||||
Reference in New Issue
Block a user