mirror of
https://github.com/Psychotoxical/psysonic.git
synced 2026-07-21 23:05:46 +00:00
fix(queue): pin queueServerId on auto-add paths so infinite + radio top-up refs resolve (#930)
* fix(queue): extend server-pin contract to auto-add paths The infinite-queue top-up and radio top-up paths in nextAction.ts read state.queueServerId directly inside their set callbacks. When the queue was populated without a queue-replacing playTrack (single- track enqueue from a SongRow + button, AdvancedSearch row, etc), 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 auto-added row rendered as the resolver placeholder (… / 0:00) until the next time something happened to bind the server. Same symptom PR #892 fixed for the manual enqueue surface, just on the auto-add paths. Extract ensureQueueServerPinned() from the private helper in queueMutationActions.ts into playbackServer.ts so it can be shared. Call it before every set callback that appends or splices refs in nextAction.ts — appendTracksAndPlayFirst, proactive infinite top-up, proactive radio top-up. Helper returns the pinned canonical key so the caller does not need a second store read. Regression coverage in ensureQueueServerPinned.test.ts: pin on null + active server, idempotent on already-bound, empty-string fallback when no active server, canonical-key return value matches what toQueueItemRefs expects (not the raw auth uuid). Existing b1QueueServerIdentity.test.ts continues to cover the manual enqueue surface unchanged. * docs(release): CHANGELOG for queue auto-top-up placeholder fix (PR #930)
This commit is contained in:
committed by
GitHub
parent
ae1572f370
commit
2a88ca3248
@@ -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)**
|
||||
|
||||
+12
-3
@@ -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 = [
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
});
|
||||
@@ -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 });
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user