diff --git a/CHANGELOG.md b/CHANGELOG.md index c7c9bf8b..f53148bf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -339,6 +339,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * Channel publish now refreshes `nix/upstream-sources.json` and `flake.lock` on the channel branch **before** cutting `app-v*` tags, so Nix builds from release tags no longer fail with stale `npmDepsHash` (e.g. after promote finalizes `package-lock.json` version). +### Queue — Infinite Queue and Smart Radio top-ups no longer show `…` / `0:00` + +**By [@Psychotoxical](https://github.com/Psychotoxical), PR [#930](https://github.com/Psychotoxical/psysonic/pull/930)** + +* Tracks added automatically by **Infinite Queue** or by **Smart Radio** could render as `…` / `0:00` instead of their real title and duration when the queue was filled without a queue-replacing playback (single-track enqueue from a song row, search result, etc). +* Same root cause as PR #892 — just on the auto-add paths the earlier fix did not cover. The owning server is now pinned before each auto-top-up so the resolver cache sees the fresh tracks. + ### Advanced Search — centered button label **By [@Psychotoxical](https://github.com/Psychotoxical), PR [#925](https://github.com/Psychotoxical/psysonic/pull/925)** diff --git a/src/store/nextAction.ts b/src/store/nextAction.ts index 15d29fad..6e9e225c 100644 --- a/src/store/nextAction.ts +++ b/src/store/nextAction.ts @@ -2,6 +2,7 @@ import { getSimilarSongs2, getTopSongs } from '../api/subsonicArtists'; import { invoke } from '@tauri-apps/api/core'; import { buildInfiniteQueueCandidates } from '../utils/playback/buildInfiniteQueueCandidates'; import { songToTrack } from '../utils/playback/songToTrack'; +import { ensureQueueServerPinned } from '../utils/playback/playbackServer'; import { useAuthStore } from './authStore'; import { setIsAudioPaused } from './engineState'; import { @@ -35,8 +36,11 @@ type GetState = () => PlayerState; */ function appendTracksAndPlayFirst(set: SetState, get: GetState, fresh: Track[]): void { if (fresh.length === 0) return; + // Pin the server *before* reading state so the appended refs (and the + // resolver seed) carry the canonical server key — otherwise queue rows for + // the appended tracks render as the resolver placeholder. See PR #892. + const serverId = ensureQueueServerPinned(); const state = get(); - const serverId = state.queueServerId ?? ''; if (serverId) seedQueueResolver(serverId, fresh); const incoming: QueueItemRef[] = toQueueItemRefs(serverId, fresh); const playAt = state.queueItems.length; @@ -93,8 +97,12 @@ export function runNext(set: SetState, get: GetState, manual: boolean): void { // an Orbit session between scheduling and resolving. if (isInOrbitSession()) return; if (newTracks.length > 0) { + // Pin before set so the appended refs carry the canonical server + // key; without this the auto-added rows render as '…' / 0:00 + // when the queue was populated without a queue-replacing playTrack + // (see PR #892). + const serverId = ensureQueueServerPinned(); set(state => { - const serverId = state.queueServerId ?? ''; if (serverId) seedQueueResolver(serverId, newTracks); const newItems = [...state.queueItems, ...toQueueItemRefs(serverId, newTracks)]; return { queueItems: newItems }; @@ -146,8 +154,9 @@ export function runNext(set: SetState, get: GetState, manual: boolean): void { // Keep the last HISTORY_KEEP played tracks so the user can still // navigate backwards a few songs. Trimmed ids stay in the seen-set. const HISTORY_KEEP = 5; + // Pin before set; same reasoning as the infinite top-up above. + const serverId = ensureQueueServerPinned(); set(state => { - const serverId = state.queueServerId ?? ''; if (serverId) seedQueueResolver(serverId, fresh); const trimStart = Math.max(0, state.queueIndex - HISTORY_KEEP); const newItems = [ diff --git a/src/store/queueMutationActions.ts b/src/store/queueMutationActions.ts index 5d01375e..cc3468ce 100644 --- a/src/store/queueMutationActions.ts +++ b/src/store/queueMutationActions.ts @@ -22,8 +22,8 @@ import { clearSeekTarget } from './seekTargetState'; import i18n from '../i18n'; import { playListenSessionFinalize } from './playListenSession'; import { - bindQueueServerForPlayback, clearQueueServerForPlayback, + ensureQueueServerPinned, playbackServerDiffersFromActive, } from '../utils/playback/playbackServer'; import { useLuckyMixStore } from './luckyMixStore'; @@ -57,17 +57,6 @@ function seedIncoming(state: PlayerState, tracks: Track[]): void { if (serverId) seedQueueResolver(serverId, tracks); } -/** Pin the active server before a mutation adds new tracks. Without this an - * enqueue against a still-null `queueServerId` (e.g. the user's first action - * is a single-track enqueue rather than a queue-replacing playTrack) leaves - * `seedIncoming` as a no-op — the refs land with the empty server key, the - * cache stays cold, and every new row renders as the resolver placeholder - * in the queue panel. No-op when already pinned, or when no active server - * is available (e.g. unit tests without an authed auth store). */ -function ensureQueueServerPinned(state: PlayerState): void { - if (state.queueServerId == null) bindQueueServerForPlayback(); -} - /** * Eleven queue-mutation actions. Shared invariant: every action except * `setRadioArtistId` pushes a queue-undo snapshot and calls @@ -99,7 +88,7 @@ export function createQueueMutationActions(set: SetState, get: GetState): Pick< return; } if (!skipQueueUndo) pushQueueUndoFromGetter(get); - ensureQueueServerPinned(get()); + ensureQueueServerPinned(); set(state => { seedIncoming(state, tracks); const items = itemsOf(state); @@ -131,7 +120,7 @@ export function createQueueMutationActions(set: SetState, get: GetState): Pick< setCurrentRadioArtistId(artistId); } pushQueueUndoFromGetter(get); - ensureQueueServerPinned(get()); + ensureQueueServerPinned(); set(state => { const items = itemsOf(state); // Drop all upcoming (not yet played) radio tracks — clicking "Start Radio" @@ -183,7 +172,7 @@ export function createQueueMutationActions(set: SetState, get: GetState): Pick< return; } pushQueueUndoFromGetter(get); - ensureQueueServerPinned(get()); + ensureQueueServerPinned(); set(state => { seedIncoming(state, tracks); const items = itemsOf(state); @@ -202,7 +191,7 @@ export function createQueueMutationActions(set: SetState, get: GetState): Pick< playNext: (tracks) => { if (tracks.length === 0) return; if (blockCrossServerEnqueue()) return; - ensureQueueServerPinned(get()); + ensureQueueServerPinned(); const state = get(); const tagged = tracks.map(t => ({ ...t, playNextAdded: true as const })); if (!state.currentTrack) { diff --git a/src/utils/playback/ensureQueueServerPinned.test.ts b/src/utils/playback/ensureQueueServerPinned.test.ts new file mode 100644 index 00000000..9f285544 --- /dev/null +++ b/src/utils/playback/ensureQueueServerPinned.test.ts @@ -0,0 +1,94 @@ +/** + * Regression cluster for the queue auto-add server-pin contract. + * + * The infinite-queue top-up and radio top-up paths in `nextAction.ts` used to + * read `state.queueServerId ?? ''` directly inside their `set(state => ...)` + * callbacks. When the queue was populated via paths that don't replace it (e.g. + * single-track enqueue from a SongRow + button), `queueServerId` stayed null, + * `seedQueueResolver` skipped its store-write under the `if (serverId)` guard, + * and the auto-added refs landed with an empty server key — every new row + * rendered as the resolver placeholder ('…' / 0:00) in the queue panel. + * + * PR #892 fixed the manual-enqueue surface via `ensureQueueServerPinned` inside + * `queueMutationActions.ts`. This file pins the contract for the auto-add + * paths that share the same helper (see `nextAction.ts`). + */ +import { describe, it, expect, beforeEach } from 'vitest'; +import { useAuthStore } from '@/store/authStore'; +import { usePlayerStore } from '@/store/playerStore'; +import { resetAuthStore, resetPlayerStore } from '@/test/helpers/storeReset'; +import { ensureQueueServerPinned } from '@/utils/playback/playbackServer'; + +const SERVER_A = { + id: 'uuid-a', + name: 'A', + url: 'http://a.test', + username: 'u', + password: 'p', +}; +const SERVER_B = { + id: 'uuid-b', + name: 'B', + url: 'http://b.test', + username: 'u', + password: 'p', +}; +const KEY_A = 'a.test'; +const KEY_B = 'b.test'; + +beforeEach(() => { + resetAuthStore(); + resetPlayerStore(); + useAuthStore.setState({ + servers: [SERVER_A, SERVER_B], + activeServerId: SERVER_A.id, + isLoggedIn: true, + }); +}); + +describe('ensureQueueServerPinned', () => { + it('binds the active server when queueServerId is null and returns the canonical key', () => { + expect(usePlayerStore.getState().queueServerId).toBeNull(); + + const serverId = ensureQueueServerPinned(); + + expect(serverId).toBe(KEY_A); + expect(usePlayerStore.getState().queueServerId).toBe(KEY_A); + }); + + it('is idempotent when queueServerId is already bound — no overwrite, no extra writes', () => { + usePlayerStore.setState({ queueServerId: KEY_B }); + // Pin via the helper while the active server is A but the queue is bound to B + // (e.g. cross-server enqueue blocked elsewhere; we should not silently rebind here). + const serverId = ensureQueueServerPinned(); + + expect(serverId).toBe(KEY_B); + expect(usePlayerStore.getState().queueServerId).toBe(KEY_B); + }); + + it('returns an empty string and leaves queueServerId null when no active server is available', () => { + useAuthStore.setState({ activeServerId: null }); + expect(usePlayerStore.getState().queueServerId).toBeNull(); + + const serverId = ensureQueueServerPinned(); + + // Matches the pre-existing no-active-server fallback in + // `bindQueueServerForPlayback`: an empty key signals "do not seed the + // resolver" without crashing the caller (unit tests, pre-login state). + expect(serverId).toBe(''); + expect(usePlayerStore.getState().queueServerId).toBeNull(); + }); + + it('returns the canonical key after a fresh pin even when the active server id is the raw UUID', () => { + // `bindQueueServerForPlayback` converts uuid → canonical index key. + // This documents that the returned value is the same value `toQueueItemRefs` + // should be called with, not the auth-store uuid. + useAuthStore.setState({ activeServerId: SERVER_B.id }); + usePlayerStore.setState({ queueServerId: null }); + + const serverId = ensureQueueServerPinned(); + + expect(serverId).toBe(KEY_B); + expect(serverId).not.toBe(SERVER_B.id); + }); +}); diff --git a/src/utils/playback/playbackServer.ts b/src/utils/playback/playbackServer.ts index 986c8fe5..313b2f55 100644 --- a/src/utils/playback/playbackServer.ts +++ b/src/utils/playback/playbackServer.ts @@ -57,6 +57,27 @@ export function bindQueueServerForPlayback(): void { usePlayerStore.setState({ queueServerId: canonical }); } +/** + * Bind `queueServerId` via {@link bindQueueServerForPlayback} when it is still + * null, then return the (now-bound) server identifier. Call this synchronously + * before any state mutation that adds new tracks to the queue. + * + * Without the pin, refs land with an empty server key, {@link seedQueueResolver} + * skips its store-write, and queue rows render as the resolver placeholder + * (`…` / 0:00) until something else binds the server (see PR #892). Affects + * both the manual enqueue mutations and the auto-add paths (infinite-queue + * top-up, radio top-up). + * + * Idempotent: no-op when already pinned. Returns `''` when no active server is + * available to pin (e.g. unit tests without an authed store). + */ +export function ensureQueueServerPinned(): string { + if (usePlayerStore.getState().queueServerId == null) { + bindQueueServerForPlayback(); + } + return usePlayerStore.getState().queueServerId ?? ''; +} + export function clearQueueServerForPlayback(): void { usePlayerStore.setState({ queueServerId: null }); }