feat(web): #415 stage 3 (web) — rotation-aware system playlist play
Web half of Stage 3. System-playlist tile play now:
- calls the new GET /api/playlists/system/{variant}/shuffle endpoint
(rotation-aware order from the server) instead of getPlaylist;
plays the returned order AS-IS — no client Fisher-Yates, since
the server already ordered it. Supersedes #413's client shuffle
for system playlists specifically; user playlists keep getPlaylist
+ stored order.
- tags the queue with the system variant. The player store carries
_queueSource; the events dispatcher includes `source` on
play_started so the server advances that playlist's rotation.
User playlists are unchanged (getPlaylist, plain playQueue, no
source). Tests updated: For-You play hits systemShuffle (not
getPlaylist/refresh) and passes source:for_you; user play uses
getPlaylist + plain playQueue with no source.
Flutter half is blocked — the Flutter client has no play-event
reporting at all (no /api/events POST, no scrobble), so there's no
play_started to attach `source` to. Surfacing that as a separate
decision rather than silently scope-exploding #415.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -19,6 +19,18 @@ export async function getPlaylist(id: string): Promise<PlaylistDetail> {
|
|||||||
return api.get<PlaylistDetail>(`/api/playlists/${encodeURIComponent(id)}`);
|
return api.get<PlaylistDetail>(`/api/playlists/${encodeURIComponent(id)}`);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// #415 stage 2/3: rotation-aware play order for a system playlist.
|
||||||
|
// Same shape as getPlaylist but tracks are server-ordered (unplayed
|
||||||
|
// this rotation first, then heard; resets when exhausted). The model
|
||||||
|
// variant uses underscores; the route segment is hyphenated.
|
||||||
|
// Intentionally uncached — it varies per play, unlike getPlaylist.
|
||||||
|
export async function systemShuffle(variant: string): Promise<PlaylistDetail> {
|
||||||
|
const seg = variant === 'for_you' ? 'for-you' : variant;
|
||||||
|
return api.get<PlaylistDetail>(
|
||||||
|
`/api/playlists/system/${encodeURIComponent(seg)}/shuffle`,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
export type CreatePlaylistInput = {
|
export type CreatePlaylistInput = {
|
||||||
name: string;
|
name: string;
|
||||||
description?: string;
|
description?: string;
|
||||||
|
|||||||
@@ -3,7 +3,7 @@
|
|||||||
import { useQueryClient } from '@tanstack/svelte-query';
|
import { useQueryClient } from '@tanstack/svelte-query';
|
||||||
import type { Playlist, PlaylistTrack, TrackRef } from '$lib/api/types';
|
import type { Playlist, PlaylistTrack, TrackRef } from '$lib/api/types';
|
||||||
import { user } from '$lib/auth/store.svelte';
|
import { user } from '$lib/auth/store.svelte';
|
||||||
import { getPlaylist, refreshDiscover, refreshForYou } from '$lib/api/playlists';
|
import { getPlaylist, systemShuffle, refreshDiscover, refreshForYou } from '$lib/api/playlists';
|
||||||
import { playlistTrackToRef } from '$lib/playlists/playlistTrackToRef';
|
import { playlistTrackToRef } from '$lib/playlists/playlistTrackToRef';
|
||||||
import { errCode } from '$lib/api/errors';
|
import { errCode } from '$lib/api/errors';
|
||||||
import { qk } from '$lib/api/queries';
|
import { qk } from '$lib/api/queries';
|
||||||
@@ -45,14 +45,23 @@
|
|||||||
if (starting) return;
|
if (starting) return;
|
||||||
starting = true;
|
starting = true;
|
||||||
try {
|
try {
|
||||||
const detail = await getPlaylist(playlist.id);
|
const variant = playlist.system_variant;
|
||||||
const refs = toTrackRefs(detail.tracks);
|
if (variant != null) {
|
||||||
if (refs.length > 0) {
|
// #415: server returns the rotation-aware order (unplayed
|
||||||
// System playlists default to shuffle so replaying within a
|
// this rotation first). Play it as-is — no client shuffle —
|
||||||
// day feels different. User playlists keep stored order.
|
// and tag the queue so play_started carries `source` and
|
||||||
// Explicit refresh moved to the kebab menu — playing no
|
// the server advances the rotation.
|
||||||
// longer rebuilds the snapshot.
|
const detail = await systemShuffle(variant);
|
||||||
playQueue(refs, 0, { shuffle: playlist.system_variant != null });
|
const refs = toTrackRefs(detail.tracks);
|
||||||
|
if (refs.length > 0) {
|
||||||
|
playQueue(refs, 0, { source: variant });
|
||||||
|
}
|
||||||
|
} else {
|
||||||
|
const detail = await getPlaylist(playlist.id);
|
||||||
|
const refs = toTrackRefs(detail.tracks);
|
||||||
|
if (refs.length > 0) {
|
||||||
|
playQueue(refs, 0);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
} finally {
|
} finally {
|
||||||
starting = false;
|
starting = false;
|
||||||
|
|||||||
@@ -38,6 +38,36 @@ vi.mock('$lib/api/playlists', () => ({
|
|||||||
}
|
}
|
||||||
]
|
]
|
||||||
} as PlaylistDetail),
|
} as PlaylistDetail),
|
||||||
|
systemShuffle: vi.fn().mockResolvedValue({
|
||||||
|
id: 'p-sys',
|
||||||
|
user_id: 'u-self',
|
||||||
|
owner_username: 'me',
|
||||||
|
name: 'For You',
|
||||||
|
description: '',
|
||||||
|
is_public: false,
|
||||||
|
kind: 'system',
|
||||||
|
system_variant: 'for_you',
|
||||||
|
seed_artist_id: null,
|
||||||
|
cover_url: '',
|
||||||
|
track_count: 1,
|
||||||
|
duration_sec: 60,
|
||||||
|
created_at: '2026-01-01T00:00:00Z',
|
||||||
|
updated_at: '2026-01-01T00:00:00Z',
|
||||||
|
tracks: [
|
||||||
|
{
|
||||||
|
position: 0,
|
||||||
|
track_id: 't-9',
|
||||||
|
album_id: 'al-1',
|
||||||
|
artist_id: 'ar-1',
|
||||||
|
title: 'Rotation Track',
|
||||||
|
artist_name: 'Test Artist',
|
||||||
|
album_title: 'Test Album',
|
||||||
|
duration_sec: 60,
|
||||||
|
stream_url: '/s/t-9',
|
||||||
|
added_at: '2026-01-01T00:00:00Z'
|
||||||
|
}
|
||||||
|
]
|
||||||
|
} as PlaylistDetail),
|
||||||
refreshDiscover: vi.fn().mockResolvedValue({ playlist_id: 'p-1', track_count: 5 }),
|
refreshDiscover: vi.fn().mockResolvedValue({ playlist_id: 'p-1', track_count: 5 }),
|
||||||
refreshForYou: vi.fn().mockResolvedValue({ playlist_id: 'p-new', track_count: 25, track_ids: ['t-1'] })
|
refreshForYou: vi.fn().mockResolvedValue({ playlist_id: 'p-new', track_count: 25, track_ids: ['t-1'] })
|
||||||
}));
|
}));
|
||||||
@@ -228,8 +258,8 @@ describe('PlaylistCard', () => {
|
|||||||
await waitFor(() => expect(refreshForYou).toHaveBeenCalled());
|
await waitFor(() => expect(refreshForYou).toHaveBeenCalled());
|
||||||
});
|
});
|
||||||
|
|
||||||
test('For-You play button does NOT call refreshForYou (uses existing snapshot)', async () => {
|
test('For-You play calls systemShuffle, not getPlaylist or refreshForYou', async () => {
|
||||||
const { refreshForYou, getPlaylist } = await import('$lib/api/playlists');
|
const { refreshForYou, getPlaylist, systemShuffle } = await import('$lib/api/playlists');
|
||||||
const { playQueue } = await import('$lib/player/store.svelte');
|
const { playQueue } = await import('$lib/player/store.svelte');
|
||||||
const forYouPlaylist: Playlist = {
|
const forYouPlaylist: Playlist = {
|
||||||
...base,
|
...base,
|
||||||
@@ -244,13 +274,15 @@ describe('PlaylistCard', () => {
|
|||||||
await fireEvent.click(playButton);
|
await fireEvent.click(playButton);
|
||||||
await new Promise(resolve => setTimeout(resolve, 10));
|
await new Promise(resolve => setTimeout(resolve, 10));
|
||||||
|
|
||||||
// Play uses the existing playlist snapshot — no refresh on press.
|
// #415: system play hits the rotation-aware shuffle endpoint,
|
||||||
|
// not the cached detail GET, and never refreshes on press.
|
||||||
|
expect(systemShuffle).toHaveBeenCalledWith('for_you');
|
||||||
|
expect(getPlaylist).not.toHaveBeenCalled();
|
||||||
expect(refreshForYou).not.toHaveBeenCalled();
|
expect(refreshForYou).not.toHaveBeenCalled();
|
||||||
expect(getPlaylist).toHaveBeenCalledWith('p-1');
|
|
||||||
expect(playQueue).toHaveBeenCalled();
|
expect(playQueue).toHaveBeenCalled();
|
||||||
});
|
});
|
||||||
|
|
||||||
test('For-You play passes shuffle:true to playQueue', async () => {
|
test('For-You play passes source:for_you to playQueue (no client shuffle)', async () => {
|
||||||
const { playQueue } = await import('$lib/player/store.svelte');
|
const { playQueue } = await import('$lib/player/store.svelte');
|
||||||
const forYouPlaylist: Playlist = {
|
const forYouPlaylist: Playlist = {
|
||||||
...base,
|
...base,
|
||||||
@@ -266,22 +298,21 @@ describe('PlaylistCard', () => {
|
|||||||
expect(playQueue).toHaveBeenCalledWith(
|
expect(playQueue).toHaveBeenCalledWith(
|
||||||
expect.any(Array),
|
expect.any(Array),
|
||||||
0,
|
0,
|
||||||
expect.objectContaining({ shuffle: true })
|
expect.objectContaining({ source: 'for_you' })
|
||||||
);
|
);
|
||||||
});
|
});
|
||||||
|
|
||||||
test('user-playlist play passes shuffle:false to playQueue', async () => {
|
test('user-playlist play uses getPlaylist + plain playQueue (no source)', async () => {
|
||||||
|
const { getPlaylist, systemShuffle } = await import('$lib/api/playlists');
|
||||||
const { playQueue } = await import('$lib/player/store.svelte');
|
const { playQueue } = await import('$lib/player/store.svelte');
|
||||||
render(PlaylistCard, { props: { playlist: base } });
|
render(PlaylistCard, { props: { playlist: base } });
|
||||||
const playButton = screen.getByLabelText(/Play Saturday morning/i);
|
const playButton = screen.getByLabelText(/Play Saturday morning/i);
|
||||||
await fireEvent.click(playButton);
|
await fireEvent.click(playButton);
|
||||||
await new Promise(resolve => setTimeout(resolve, 10));
|
await new Promise(resolve => setTimeout(resolve, 10));
|
||||||
|
|
||||||
expect(playQueue).toHaveBeenCalledWith(
|
expect(systemShuffle).not.toHaveBeenCalled();
|
||||||
expect.any(Array),
|
expect(getPlaylist).toHaveBeenCalledWith('p-1');
|
||||||
0,
|
expect(playQueue).toHaveBeenCalledWith(expect.any(Array), 0);
|
||||||
expect.objectContaining({ shuffle: false })
|
|
||||||
);
|
|
||||||
});
|
});
|
||||||
|
|
||||||
test('non-For-You play button does NOT call refreshForYou', async () => {
|
test('non-For-You play button does NOT call refreshForYou', async () => {
|
||||||
|
|||||||
@@ -92,7 +92,11 @@ export function useEventsDispatcher(): void {
|
|||||||
const res = await api.post<PlayStartedResponse>('/api/events', {
|
const res = await api.post<PlayStartedResponse>('/api/events', {
|
||||||
type: 'play_started',
|
type: 'play_started',
|
||||||
track_id: trackId,
|
track_id: trackId,
|
||||||
client_id: clientId
|
client_id: clientId,
|
||||||
|
// #415: tag the play with the system playlist it came from
|
||||||
|
// (null for library / user-playlist / radio) so the server
|
||||||
|
// can advance that playlist's rotation.
|
||||||
|
source: player.queueSource ?? undefined
|
||||||
});
|
});
|
||||||
// The user may have moved on by the time the response arrives. Only
|
// The user may have moved on by the time the response arrives. Only
|
||||||
// adopt the id if we're still on the same track and still playing.
|
// adopt the id if we're still on the same track and still playing.
|
||||||
|
|||||||
@@ -41,6 +41,13 @@ let _pendingRestorePosition: number | null = null;
|
|||||||
let _radioSeedId = $state<string | null>(null);
|
let _radioSeedId = $state<string | null>(null);
|
||||||
let _radioRefreshInFlight = false;
|
let _radioRefreshInFlight = false;
|
||||||
|
|
||||||
|
// #415: which system playlist (if any) this queue was seeded from.
|
||||||
|
// 'for_you' | 'discover' | null. Read by the events dispatcher so
|
||||||
|
// play_started carries `source` and the play counts against that
|
||||||
|
// playlist's rotation. Re-set on every playQueue (a fresh non-system
|
||||||
|
// queue clears it).
|
||||||
|
let _queueSource = $state<string | null>(null);
|
||||||
|
|
||||||
let _audioEl: HTMLAudioElement | null = null;
|
let _audioEl: HTMLAudioElement | null = null;
|
||||||
|
|
||||||
export const player = {
|
export const player = {
|
||||||
@@ -55,6 +62,7 @@ export const player = {
|
|||||||
get shuffle() { return _shuffle; },
|
get shuffle() { return _shuffle; },
|
||||||
get repeat() { return _repeat; },
|
get repeat() { return _repeat; },
|
||||||
get error() { return _error; },
|
get error() { return _error; },
|
||||||
|
get queueSource() { return _queueSource; },
|
||||||
get queueDrawerOpen() { return _queueDrawerOpen; }
|
get queueDrawerOpen() { return _queueDrawerOpen; }
|
||||||
};
|
};
|
||||||
|
|
||||||
@@ -65,9 +73,13 @@ export function registerAudioEl(el: HTMLAudioElement | null): void {
|
|||||||
export function playQueue(
|
export function playQueue(
|
||||||
tracks: TrackRef[],
|
tracks: TrackRef[],
|
||||||
startIndex = 0,
|
startIndex = 0,
|
||||||
opts: { shuffle?: boolean } = {},
|
opts: { shuffle?: boolean; source?: string | null } = {},
|
||||||
): void {
|
): void {
|
||||||
_radioSeedId = null; // M4c: non-radio enqueue clears the radio refresh state
|
_radioSeedId = null; // M4c: non-radio enqueue clears the radio refresh state
|
||||||
|
// #415: a fresh queue resets the system-playlist source. Set only
|
||||||
|
// when seeded from a system playlist (server already returned the
|
||||||
|
// rotation-aware order, so no client shuffle in that path).
|
||||||
|
_queueSource = opts.source ?? null;
|
||||||
if (opts.shuffle && tracks.length > 1) {
|
if (opts.shuffle && tracks.length > 1) {
|
||||||
// Fisher-Yates over the whole list. startIndex is ignored — the
|
// Fisher-Yates over the whole list. startIndex is ignored — the
|
||||||
// caller is asking for "random play from this pool," so the first
|
// caller is asking for "random play from this pool," so the first
|
||||||
|
|||||||
Reference in New Issue
Block a user