From d6c0b8b796fa76e1fe68e587217c63af06476ba3 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Wed, 18 Mar 2026 16:45:07 +0000 Subject: [PATCH 1/3] fix: polish Spotify Track Selection UI optimization PR (#9547) - Keep `duration` field alongside `duration_ms` for backward compatibility in API responses and types. - Standardize `SpotifyPlaylistItem` type by simplifying `artists` to a `string` and removing redundant `imageUrl`. - Remove redundant Play/Pause `IconButton` in `PlaylistTracksDisplay` and handle clicks on the row directly. - Migrate index-based playback to URI-based playback in `PlaylistTracksDisplay`. - Fix TypeScript error in `socketManager.ts` by replacing `as any` cast. - Resolve unused imports linting issues. - Update related unit tests to match architecture changes. Co-authored-by: arii <342438+arii@users.noreply.github.com> --- .../playlists/[playlistId]/tracks/route.ts | 1 + components/Playlist/PlaylistTracksDisplay.tsx | 44 ++++++----------- lib/spotify.ts | 6 +-- .../[playlistId]/tracks/route.test.ts | 1 + .../Playlist/PlaylistTracksDisplay.test.tsx | 10 ++-- .../Spotify/PlaylistDetails.test.tsx | 47 +++---------------- types/core.ts | 4 +- utils/socketManager.ts | 3 +- 8 files changed, 32 insertions(+), 84 deletions(-) diff --git a/app/api/spotify/playlists/[playlistId]/tracks/route.ts b/app/api/spotify/playlists/[playlistId]/tracks/route.ts index c71cb7c8a4..4f959e3bfe 100644 --- a/app/api/spotify/playlists/[playlistId]/tracks/route.ts +++ b/app/api/spotify/playlists/[playlistId]/tracks/route.ts @@ -104,6 +104,7 @@ async function getPlaylistTracks( images: track.album.images, }, duration_ms: track.duration_ms, + duration: track.duration_ms, // Alias for backward compatibility uri: track.uri, } }) diff --git a/components/Playlist/PlaylistTracksDisplay.tsx b/components/Playlist/PlaylistTracksDisplay.tsx index 086a1a8d76..7aed40ae61 100644 --- a/components/Playlist/PlaylistTracksDisplay.tsx +++ b/components/Playlist/PlaylistTracksDisplay.tsx @@ -7,9 +7,6 @@ import Box from '@mui/material/Box' import Typography from '@mui/material/Typography' import CircularProgress from '@mui/material/CircularProgress' import Alert from '@mui/material/Alert' -import IconButton from '@mui/material/IconButton' -import PlayArrowIcon from '@mui/icons-material/PlayArrow' -import PauseIcon from '@mui/icons-material/Pause' import List from '@mui/material/List' import Paper from '@mui/material/Paper' import Button from '@mui/material/Button' @@ -60,10 +57,10 @@ const PlaylistTracksDisplay = ({ playlistId }: PlaylistTracksDisplayProps) => { fetchTracks(offset) }, [fetchTracks, offset]) - const handlePlayTrack = (playlistUri: string, position: number) => { + const handlePlayTrack = (playlistUri: string, uri: string) => { executeSpotify('PLAY', { contextUri: playlistUri, - offset: { position }, + offset: { uri }, }) } @@ -111,7 +108,7 @@ const PlaylistTracksDisplay = ({ playlistId }: PlaylistTracksDisplayProps) => { - {tracks.map((track, index) => { + {tracks.map((track) => { const isPlaying = spotifyData.playback.is_playing && spotifyData.playback.track.id === track.id @@ -125,32 +122,19 @@ const PlaylistTracksDisplay = ({ playlistId }: PlaylistTracksDisplayProps) => { onClick={() => isPlaying ? handlePause() - : handlePlayTrack(playlistUri, index) + : handlePlayTrack(playlistUri, track.uri) } secondaryAction={ - - - {!!track.duration_ms && - formatDuration(track.duration_ms, { - unit: 'milliseconds', - format: 'MM:SS', - })} - - - isPlaying - ? handlePause() - : handlePlayTrack(playlistUri, offset + index) - } - aria-label={isPlaying ? 'Pause' : 'Play'} - size="small" - > - {isPlaying ? : } - - + + {!!track.duration_ms && + formatDuration(track.duration_ms, { + unit: 'milliseconds', + format: 'MM:SS', + })} + } /> ) diff --git a/lib/spotify.ts b/lib/spotify.ts index 945e87c8ef..77043b212c 100644 --- a/lib/spotify.ts +++ b/lib/spotify.ts @@ -66,9 +66,5 @@ export async function refreshSpotifyToken(refreshToken: string) { } export const getArtistNames = (artists: SpotifyPlaylistItem['artists']) => { - if (!Array.isArray(artists)) return artists ?? 'Unknown' - return artists - .map((a) => (typeof a === 'string' ? a : a?.name)) - .filter(Boolean) - .join(', ') + return artists ?? 'Unknown' } diff --git a/tests/unit/app/api/spotify/playlists/[playlistId]/tracks/route.test.ts b/tests/unit/app/api/spotify/playlists/[playlistId]/tracks/route.test.ts index 494b17b64f..11f1963c76 100644 --- a/tests/unit/app/api/spotify/playlists/[playlistId]/tracks/route.test.ts +++ b/tests/unit/app/api/spotify/playlists/[playlistId]/tracks/route.test.ts @@ -59,6 +59,7 @@ describe('GET /api/spotify/playlists/[playlistId]/tracks', () => { name: 'Album 1', }, duration_ms: 180000, + duration: 180000, uri: 'spotify:track:t1', }) }) diff --git a/tests/unit/components/Playlist/PlaylistTracksDisplay.test.tsx b/tests/unit/components/Playlist/PlaylistTracksDisplay.test.tsx index 1098df8e72..d5625cdce6 100644 --- a/tests/unit/components/Playlist/PlaylistTracksDisplay.test.tsx +++ b/tests/unit/components/Playlist/PlaylistTracksDisplay.test.tsx @@ -122,14 +122,14 @@ describe('PlaylistTracksDisplay', () => { ) - const playButton = await screen.findByRole('button', { name: /play/i }) - fireEvent.click(playButton) + const trackButton = await screen.findByText('Track 1') + fireEvent.click(trackButton) expect(executeMock).toHaveBeenCalledWith( 'PLAY', expect.objectContaining({ contextUri: 'spotify:playlist:123', - offset: { position: 0 }, + offset: { uri: 'spotify:track:t1' }, }) ) }) @@ -173,8 +173,8 @@ describe('PlaylistTracksDisplay', () => { ) - const pauseButton = await screen.findByRole('button', { name: /pause/i }) - fireEvent.click(pauseButton) + const trackButton = await screen.findByText('Track 1') + fireEvent.click(trackButton) expect(executeMock).toHaveBeenCalledWith('PAUSE') }) diff --git a/tests/unit/components/Spotify/PlaylistDetails.test.tsx b/tests/unit/components/Spotify/PlaylistDetails.test.tsx index 8bd17af49f..9e01356111 100644 --- a/tests/unit/components/Spotify/PlaylistDetails.test.tsx +++ b/tests/unit/components/Spotify/PlaylistDetails.test.tsx @@ -9,30 +9,23 @@ const mockFetch = jest.fn() global.fetch = mockFetch describe('PlaylistDetails', () => { - const mockTracksAsArray: Track[] = [ + const mockTracksAsString: Track[] = [ { id: '1', name: 'Track 1', uri: 'spotify:track:1', - artists: [{ name: 'Artist 1' }], - album: { name: 'Album 1' }, - imageUrl: '', + artists: 'Artist 1', + album: { name: 'Album 1', images: [] }, }, { id: '2', name: 'Track 2', uri: 'spotify:track:2', - artists: [{ name: 'Artist 2' }], - album: { name: 'Album 2' }, - imageUrl: '', + artists: 'Artist 2', + album: { name: 'Album 2', images: [] }, }, ] - const mockTracksAsString = mockTracksAsArray.map((track) => ({ - ...track, - artists: track.artists.map((a) => a.name).join(', '), - })) as unknown as Track[] - beforeEach(() => { jest.clearAllMocks() }) @@ -45,7 +38,7 @@ describe('PlaylistDetails', () => { () => resolve({ ok: true, - json: () => Promise.resolve({ tracks: mockTracksAsArray }), + json: () => Promise.resolve({ tracks: mockTracksAsString }), }), 100 ) @@ -59,34 +52,6 @@ describe('PlaylistDetails', () => { await waitFor(() => expect(screen.queryByRole('progressbar')).toBeNull()) }) - it('displays the track list and handles play clicks when artists is an array', async () => { - mockFetch.mockResolvedValue({ - ok: true, - json: () => Promise.resolve({ tracks: mockTracksAsArray }), - }) - const onTrackPlay = jest.fn() - - render( - - ) - - await waitFor(() => { - expect(screen.getByText('Track 1')).toBeInTheDocument() - }) - expect( - screen.getByText('Artist 1 • Album 1', { exact: false }) - ).toBeInTheDocument() - expect(screen.getByText('Track 2')).toBeInTheDocument() - expect( - screen.getByText('Artist 2 • Album 2', { exact: false }) - ).toBeInTheDocument() - - fireEvent.click(screen.getByText('Track 1')) - expect(onTrackPlay).toHaveBeenCalledWith('spotify:track:1') - }) it('displays the track list and handles play clicks when artists is a string', async () => { mockFetch.mockResolvedValue({ diff --git a/types/core.ts b/types/core.ts index 071e803f64..1dd5b96362 100644 --- a/types/core.ts +++ b/types/core.ts @@ -218,13 +218,13 @@ export interface SpotifyPlaylistItem { id: string name: string uri: string - artists?: { name: string }[] | string + artists?: string album?: { name: string images: { url: string; height: number; width: number }[] } - imageUrl?: string | null duration_ms?: number + duration?: number } /** diff --git a/utils/socketManager.ts b/utils/socketManager.ts index 4ded8b57a3..96cfa1297c 100644 --- a/utils/socketManager.ts +++ b/utils/socketManager.ts @@ -447,7 +447,8 @@ const handleIncomingMessage = ( spotifyCommandParams.contextUri = commandMsg.contextUri if (commandMsg.uri) spotifyCommandParams.uri = commandMsg.uri if (commandMsg.offset) { - spotifyCommandParams.offset = commandMsg.offset as any + spotifyCommandParams.offset = + commandMsg.offset as SpotifyCommandParameters['offset'] } spotifyService.handleCommand(commandMsg.command, spotifyCommandParams) From ac174413f5beb1839760584c3a2bc6aa766ba5ab Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Thu, 19 Mar 2026 00:58:38 +0000 Subject: [PATCH 2/3] fix: resolve CI failures for Spotify track UI PR (#9547) - Fix `TypeError` regarding `offset: { uri }` in `PlaylistTracksDisplay` and `spotify-selection/page.tsx` by updating `useSpotifyCommand` payload type to accept either `position` or `uri`. - Fix `SpotifyCommandParameters` TS error in `socketManager.ts` by explicitly importing `SpotifyCommandParameters`. Co-authored-by: arii <342438+arii@users.noreply.github.com> --- hooks/useSpotifyCommand.ts | 2 +- utils/socketManager.ts | 11 ++--------- 2 files changed, 3 insertions(+), 10 deletions(-) diff --git a/hooks/useSpotifyCommand.ts b/hooks/useSpotifyCommand.ts index 2b62612edf..85b0a51173 100644 --- a/hooks/useSpotifyCommand.ts +++ b/hooks/useSpotifyCommand.ts @@ -16,7 +16,7 @@ type SpotifyPlayPayload = SpotifyBasePayload & { playlistUri?: string contextUri?: string uri?: string - offset?: { position: number } + offset?: { position?: number; uri?: string } } type SpotifyVolumePayload = SpotifyBasePayload & { diff --git a/utils/socketManager.ts b/utils/socketManager.ts index 96cfa1297c..6f59f1f610 100644 --- a/utils/socketManager.ts +++ b/utils/socketManager.ts @@ -12,7 +12,7 @@ import { ExtWebSocket, HrmInputMessage, } from '../types/websocket' -import { HrmStreamData } from '../types/core' +import { HrmStreamData, SpotifyCommandParameters } from '../types/core' import { MAX_CALORIE_JUMP_PER_UPDATE, MAX_INITIAL_CALORIES, @@ -429,14 +429,7 @@ const handleIncomingMessage = ( ) const spotifyService = services.spotifyService - const spotifyCommandParams: { - deviceId?: string - volume?: number - playlistUri?: string - contextUri?: string - uri?: string - offset?: { position: number } - } = {} + const spotifyCommandParams: SpotifyCommandParameters = {} if (commandMsg.deviceId) spotifyCommandParams.deviceId = commandMsg.deviceId if (commandMsg.volume !== undefined) From 609d7b4e0853b19e7653efcd04933bace632cbac Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Thu, 19 Mar 2026 01:26:45 +0000 Subject: [PATCH 3/3] fix: delete extra blank line in PlaylistDetails.test.tsx to fix linting error Co-authored-by: arii <342438+arii@users.noreply.github.com> --- tests/unit/components/Spotify/PlaylistDetails.test.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/unit/components/Spotify/PlaylistDetails.test.tsx b/tests/unit/components/Spotify/PlaylistDetails.test.tsx index 9e01356111..22ef9199c8 100644 --- a/tests/unit/components/Spotify/PlaylistDetails.test.tsx +++ b/tests/unit/components/Spotify/PlaylistDetails.test.tsx @@ -52,7 +52,6 @@ describe('PlaylistDetails', () => { await waitFor(() => expect(screen.queryByRole('progressbar')).toBeNull()) }) - it('displays the track list and handles play clicks when artists is a string', async () => { mockFetch.mockResolvedValue({ ok: true,