diff --git a/flutter_client/lib/player/player_provider.dart b/flutter_client/lib/player/player_provider.dart index 14f4afe5..cc14d645 100644 --- a/flutter_client/lib/player/player_provider.dart +++ b/flutter_client/lib/player/player_provider.dart @@ -1,3 +1,5 @@ +import 'dart:math' show Random; + import 'package:audio_service/audio_service.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; @@ -65,7 +67,18 @@ class PlayerActions { } final Ref _ref; - Future playTracks(List tracks, {int initialIndex = 0}) async { + Future playTracks( + List tracks, { + int initialIndex = 0, + bool shuffle = false, + }) async { + // shuffle=true means "play this pool randomly, starting at a + // random track." Used for system playlists so the first track of + // a re-play within the day is different from the prior one. + var startAt = initialIndex; + if (shuffle && tracks.length > 1) { + startAt = Random().nextInt(tracks.length); + } final url = await _ref.read(serverUrlProvider.future); final token = await _ref.read(secureStorageProvider).read(key: 'session_token'); final cache = _ref.read(albumCoverCacheProvider); @@ -78,7 +91,10 @@ class PlayerActions { audioCacheManager: audioCache, likeBridge: _buildLikeBridge(), ); - await h.setQueueFromTracks(tracks, initialIndex: initialIndex); + await h.setQueueFromTracks(tracks, initialIndex: startAt); + if (shuffle) { + await h.setShuffleMode(AudioServiceShuffleMode.all); + } await h.play(); } diff --git a/flutter_client/lib/playlists/widgets/playlist_card.dart b/flutter_client/lib/playlists/widgets/playlist_card.dart index 47e75033..66566558 100644 --- a/flutter_client/lib/playlists/widgets/playlist_card.dart +++ b/flutter_client/lib/playlists/widgets/playlist_card.dart @@ -120,6 +120,13 @@ class PlaylistCard extends ConsumerWidget { )); } if (refs.isEmpty) return; - await ref.read(playerActionsProvider).playTracks(refs, initialIndex: 0); + // System playlists default to shuffle so replaying within a day + // doesn't deliver the identical experience. User playlists keep + // stored order. + await ref.read(playerActionsProvider).playTracks( + refs, + initialIndex: 0, + shuffle: playlist.isSystem, + ); } } diff --git a/web/src/lib/components/PlaylistCard.svelte b/web/src/lib/components/PlaylistCard.svelte index 45f58e22..78c64bb7 100644 --- a/web/src/lib/components/PlaylistCard.svelte +++ b/web/src/lib/components/PlaylistCard.svelte @@ -14,6 +14,11 @@ const isOwn = $derived(user.value?.id === playlist.user_id); const isDiscover = $derived(playlist.system_variant === 'discover'); + const isForYou = $derived(playlist.system_variant === 'for_you'); + const isSystem = $derived(playlist.system_variant != null); + const refreshLabel = $derived( + isDiscover ? 'Refresh Discover' : isForYou ? 'Refresh For You' : 'Refresh', + ); const queryClient = useQueryClient(); @@ -40,38 +45,34 @@ if (starting) return; starting = true; try { - let detailID = playlist.id; - // For-You: refresh first so click means "give me a fresh mix - // and play it." Atomic replace creates a new playlist row, so - // the response's playlist_id is what we follow up with. - if (playlist.system_variant === 'for_you') { - const refreshed = await refreshForYou(); - if (!refreshed.playlist_id) { - // Empty library — nothing to play. - return; - } - detailID = refreshed.playlist_id; - // Invalidate caches so the home + detail views see the new - // playlist on their next read. - await queryClient.invalidateQueries({ queryKey: qk.playlists() }); - } - const detail = await getPlaylist(detailID); + const detail = await getPlaylist(playlist.id); const refs = toTrackRefs(detail.tracks); if (refs.length > 0) { - playQueue(refs, 0); + // System playlists default to shuffle so replaying within a + // day feels different. User playlists keep stored order. + // Explicit refresh moved to the kebab menu — playing no + // longer rebuilds the snapshot. + playQueue(refs, 0, { shuffle: playlist.system_variant != null }); } } finally { starting = false; } } - async function onRefreshDiscover(e: MouseEvent) { + async function onRefresh(e: MouseEvent) { e.preventDefault(); e.stopPropagation(); menuOpen = false; try { - await refreshDiscover(); - pushToast('Discover refreshed.'); + if (isDiscover) { + await refreshDiscover(); + pushToast('Discover refreshed.'); + } else if (isForYou) { + await refreshForYou(); + pushToast('For You refreshed.'); + } else { + return; + } await queryClient.invalidateQueries({ queryKey: qk.playlist(playlist.id) }); await queryClient.invalidateQueries({ queryKey: qk.playlists() }); } catch (err: unknown) { @@ -83,7 +84,7 @@ { menuOpen = false; }} />
- {#if isDiscover} + {#if isSystem}
{/if} diff --git a/web/src/lib/components/PlaylistCard.test.ts b/web/src/lib/components/PlaylistCard.test.ts index 1213c5f9..688f86d9 100644 --- a/web/src/lib/components/PlaylistCard.test.ts +++ b/web/src/lib/components/PlaylistCard.test.ts @@ -143,7 +143,7 @@ describe('PlaylistCard', () => { expect(playQueue).toHaveBeenCalled(); }); - test('shows kebab button only on Discover playlists', () => { + test('shows kebab button on Discover playlists', () => { const discoverPlaylist: Playlist = { ...base, kind: 'system', @@ -154,7 +154,7 @@ describe('PlaylistCard', () => { expect(screen.getByLabelText(/playlist actions/i)).toBeInTheDocument(); }); - test('does not show kebab button on for_you playlists', () => { + test('shows kebab button on For You playlists', () => { const forYouPlaylist: Playlist = { ...base, kind: 'system', @@ -162,7 +162,7 @@ describe('PlaylistCard', () => { name: 'For You' }; render(PlaylistCard, { props: { playlist: forYouPlaylist } }); - expect(screen.queryByLabelText(/playlist actions/i)).not.toBeInTheDocument(); + expect(screen.getByLabelText(/playlist actions/i)).toBeInTheDocument(); }); test('does not show kebab button on user playlists', () => { @@ -170,7 +170,7 @@ describe('PlaylistCard', () => { expect(screen.queryByLabelText(/playlist actions/i)).not.toBeInTheDocument(); }); - test('Refresh Discover item appears in kebab menu when opened', async () => { + test('Refresh Discover item appears in Discover kebab', async () => { const discoverPlaylist: Playlist = { ...base, kind: 'system', @@ -183,7 +183,7 @@ describe('PlaylistCard', () => { expect(screen.getByRole('menuitem', { name: /refresh discover/i })).toBeInTheDocument(); }); - test('Refresh Discover item is absent from for_you card kebab', () => { + test('Refresh For You item appears in For You kebab', async () => { const forYouPlaylist: Playlist = { ...base, kind: 'system', @@ -191,7 +191,9 @@ describe('PlaylistCard', () => { name: 'For You' }; render(PlaylistCard, { props: { playlist: forYouPlaylist } }); - expect(screen.queryByRole('menuitem', { name: /refresh discover/i })).not.toBeInTheDocument(); + const kebabBtn = screen.getByLabelText(/playlist actions/i); + await fireEvent.click(kebabBtn); + expect(screen.getByRole('menuitem', { name: /refresh for you/i })).toBeInTheDocument(); }); test('clicking Refresh Discover calls refreshDiscover', async () => { @@ -210,7 +212,23 @@ describe('PlaylistCard', () => { await waitFor(() => expect(refreshDiscover).toHaveBeenCalled()); }); - test('For-You play button calls refreshForYou before getPlaylist', async () => { + test('clicking Refresh For You calls refreshForYou', async () => { + const { refreshForYou } = await import('$lib/api/playlists'); + const forYouPlaylist: Playlist = { + ...base, + kind: 'system', + system_variant: 'for_you', + name: 'For You' + }; + render(PlaylistCard, { props: { playlist: forYouPlaylist } }); + const kebabBtn = screen.getByLabelText(/playlist actions/i); + await fireEvent.click(kebabBtn); + const refreshItem = screen.getByRole('menuitem', { name: /refresh for you/i }); + await fireEvent.click(refreshItem); + await waitFor(() => expect(refreshForYou).toHaveBeenCalled()); + }); + + test('For-You play button does NOT call refreshForYou (uses existing snapshot)', async () => { const { refreshForYou, getPlaylist } = await import('$lib/api/playlists'); const { playQueue } = await import('$lib/player/store.svelte'); const forYouPlaylist: Playlist = { @@ -220,70 +238,50 @@ describe('PlaylistCard', () => { name: 'For You', track_count: 25 }; - // getPlaylist should return a detail with the NEW id - (getPlaylist as ReturnType).mockResolvedValueOnce({ - id: 'p-new', - 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-1', - album_id: 'al-1', - artist_id: 'ar-1', - title: 'Test Track', - artist_name: 'Test Artist', - album_title: 'Test Album', - duration_sec: 60, - stream_url: '/s/t-1', - added_at: '2026-01-01T00:00:00Z' - } - ] - } as PlaylistDetail); render(PlaylistCard, { props: { playlist: forYouPlaylist } }); const playButton = screen.getByLabelText(/Play For You/i); await fireEvent.click(playButton); await new Promise(resolve => setTimeout(resolve, 10)); - expect(refreshForYou).toHaveBeenCalled(); - expect(getPlaylist).toHaveBeenCalledWith('p-new'); + // Play uses the existing playlist snapshot — no refresh on press. + expect(refreshForYou).not.toHaveBeenCalled(); + expect(getPlaylist).toHaveBeenCalledWith('p-1'); expect(playQueue).toHaveBeenCalled(); }); - test('For-You play with empty-library response does not call playQueue', async () => { - const { refreshForYou } = await import('$lib/api/playlists'); + test('For-You play passes shuffle:true to playQueue', async () => { const { playQueue } = await import('$lib/player/store.svelte'); - (refreshForYou as ReturnType).mockResolvedValueOnce({ - playlist_id: null, - track_count: 0, - track_ids: [] - }); const forYouPlaylist: Playlist = { ...base, kind: 'system', system_variant: 'for_you', - name: 'For You', - track_count: 1 + name: 'For You' }; render(PlaylistCard, { props: { playlist: forYouPlaylist } }); const playButton = screen.getByLabelText(/Play For You/i); await fireEvent.click(playButton); await new Promise(resolve => setTimeout(resolve, 10)); - expect(refreshForYou).toHaveBeenCalled(); - expect(playQueue).not.toHaveBeenCalled(); + expect(playQueue).toHaveBeenCalledWith( + expect.any(Array), + 0, + expect.objectContaining({ shuffle: true }) + ); + }); + + test('user-playlist play passes shuffle:false to playQueue', async () => { + const { playQueue } = await import('$lib/player/store.svelte'); + render(PlaylistCard, { props: { playlist: base } }); + const playButton = screen.getByLabelText(/Play Saturday morning/i); + await fireEvent.click(playButton); + await new Promise(resolve => setTimeout(resolve, 10)); + + expect(playQueue).toHaveBeenCalledWith( + expect.any(Array), + 0, + expect.objectContaining({ shuffle: false }) + ); }); test('non-For-You play button does NOT call refreshForYou', async () => { diff --git a/web/src/lib/player/store.svelte.ts b/web/src/lib/player/store.svelte.ts index 6a0d9d82..8339916f 100644 --- a/web/src/lib/player/store.svelte.ts +++ b/web/src/lib/player/store.svelte.ts @@ -62,15 +62,34 @@ export function registerAudioEl(el: HTMLAudioElement | null): void { _audioEl = el; } -export function playQueue(tracks: TrackRef[], startIndex = 0): void { +export function playQueue( + tracks: TrackRef[], + startIndex = 0, + opts: { shuffle?: boolean } = {}, +): void { _radioSeedId = null; // M4c: non-radio enqueue clears the radio refresh state - _queue = tracks; - if (tracks.length === 0) { + if (opts.shuffle && tracks.length > 1) { + // Fisher-Yates over the whole list. startIndex is ignored — the + // caller is asking for "random play from this pool," so the first + // track should also be random, not the one at startIndex. + const arr = tracks.slice(); + for (let j = arr.length - 1; j > 0; j--) { + const r = Math.floor(_rng() * (j + 1)); + [arr[j], arr[r]] = [arr[r], arr[j]]; + } + _queue = arr; _index = 0; - _state = 'idle'; - } else { - _index = Math.max(0, Math.min(startIndex, tracks.length - 1)); + _shuffle = true; _state = 'loading'; + } else { + _queue = tracks; + if (tracks.length === 0) { + _index = 0; + _state = 'idle'; + } else { + _index = Math.max(0, Math.min(startIndex, tracks.length - 1)); + _state = 'loading'; + } } _position = 0; _duration = 0;