feat(ui): add Play Artist's Top Songs button (#4204)

* ui: add Play button to artist toolbar

* refactor

Signed-off-by: Deluan <deluan@navidrome.org>

* test(ui): add tests for Play button functionality in ArtistActions

Signed-off-by: Deluan <deluan@navidrome.org>

* ui: update Play button label to Top Songs in ArtistActions

Signed-off-by: Deluan <deluan@navidrome.org>

---------

Signed-off-by: Deluan <deluan@navidrome.org>

resources/i18n/pt-br.json

@@ -126,6 +126,7 @@
         "performer": "Músico |||| Músicos"
       },
       "actions": {
+        "topSongs": "Mais tocadas",
         "shuffle": "Aleatório",
         "radio": "Rádio"
       }
@@ -412,6 +413,7 @@
     "transcodingEnabled": "Navidrome está sendo executado com a opção %{config}. Isto permite que potencialmente se execute comandos do sistema pela interface Web. É recomendado que vc mantenha esta opção desabilitada, e só a habilite quando precisar configurar opções de Conversão",
     "songsAddedToPlaylist": "Música adicionada à playlist |||| %{smart_count} músicas adicionadas à playlist",
     "noSimilarSongsFound": "Nenhuma música semelhante encontrada",
+    "noTopSongsFound": "Nenhuma música mais tocada encontrada",
     "noPlaylistsAvailable": "Nenhuma playlist",
     "delete_user_title": "Excluir usuário '%{name}'",
     "delete_user_content": "Você tem certeza que deseja excluir o usuário e todos os seus dados (incluindo suas playlists e preferências)?",

ui/src/artist/ArtistActions.jsx

@@ -12,9 +12,9 @@ import {
   useTranslate,
 } from 'react-admin'
 import ShuffleIcon from '@material-ui/icons/Shuffle'
+import PlayArrowIcon from '@material-ui/icons/PlayArrow'
 import { IoIosRadio } from 'react-icons/io'
-import { playTracks } from '../actions'
+import { playShuffle, playSimilar, playTopSongs } from './actions.js'
-import { playSimilar } from '../utils'
 
 const useStyles = makeStyles((theme) => ({
   toolbar: {
@@ -53,21 +53,19 @@ const ArtistActions = ({ className, record, ...rest }) => {
   const classes = useStyles()
   const isMobile = useMediaQuery((theme) => theme.breakpoints.down('xs'))
 
+  const handlePlay = React.useCallback(async () => {
+    try {
+      await playTopSongs(dispatch, notify, record.name)
+    } catch (e) {
+      // eslint-disable-next-line no-console
+      console.error('Error fetching top songs for artist:', e)
+      notify('ra.page.error', 'warning')
+    }
+  }, [dispatch, notify, record])
+
   const handleShuffle = React.useCallback(async () => {
     try {
-      const res = await dataProvider.getList('song', {
+      await playShuffle(dataProvider, dispatch, record.id)
-        pagination: { page: 1, perPage: 500 },
-        sort: { field: 'random', order: 'ASC' },
-        filter: { album_artist_id: record.id, missing: false },
-      })
-
-      const data = {}
-      const ids = []
-      res.data.forEach((s) => {
-        data[s.id] = s
-        ids.push(s.id)
-      })
-      dispatch(playTracks(data, ids))
     } catch (e) {
       // eslint-disable-next-line no-console
       console.error('Error fetching songs for shuffle:', e)
@@ -90,6 +88,14 @@ const ArtistActions = ({ className, record, ...rest }) => {
       className={`${className} ${classes.toolbar}`}
       {...sanitizeListRestProps(rest)}
     >
+      <Button
+        onClick={handlePlay}
+        label={translate('resources.artist.actions.topSongs')}
+        className={classes.button}
+        size={isMobile ? 'small' : 'medium'}
+      >
+        <PlayArrowIcon />
+      </Button>
       <Button
         onClick={handleShuffle}
         label={translate('resources.artist.actions.shuffle')}

ui/src/artist/ArtistActions.test.jsx

@@ -4,13 +4,13 @@ import { TestContext } from 'ra-test'
 import { describe, it, expect, vi, beforeEach } from 'vitest'
 import ArtistActions from './ArtistActions'
 import subsonic from '../subsonic'
-import { ThemeProvider, createMuiTheme } from '@material-ui/core/styles'
+import { ThemeProvider, createTheme } from '@material-ui/core/styles'
 
 const mockDispatch = vi.fn()
 vi.mock('react-redux', () => ({ useDispatch: () => mockDispatch }))
 
 vi.mock('../subsonic', () => ({
-  default: { getSimilarSongs2: vi.fn() },
+  default: { getSimilarSongs2: vi.fn(), getTopSongs: vi.fn() },
 }))
 
 const mockNotify = vi.fn()
@@ -27,8 +27,28 @@ vi.mock('react-admin', async (importOriginal) => {
 })
 
 describe('ArtistActions', () => {
+  const defaultRecord = { id: 'ar1', name: 'Artist' }
+
+  const renderArtistActions = (record = defaultRecord) => {
+    const theme = createTheme()
+    return render(
+      <TestContext>
+        <ThemeProvider theme={theme}>
+          <ArtistActions record={record} />
+        </ThemeProvider>
+      </TestContext>,
+    )
+  }
+
+  const clickActionButton = (actionKey) => {
+    fireEvent.click(screen.getByText(`resources.artist.actions.${actionKey}`))
+  }
+
   beforeEach(() => {
     vi.clearAllMocks()
+    // Mock console.error to suppress error logging in tests
+    vi.spyOn(console, 'error').mockImplementation(() => {})
+
     subsonic.getSimilarSongs2.mockResolvedValue({
       json: {
         'subsonic-response': {
@@ -37,19 +57,21 @@ describe('ArtistActions', () => {
         },
       },
     })
+    subsonic.getTopSongs.mockResolvedValue({
+      json: {
+        'subsonic-response': {
+          status: 'ok',
+          topSongs: { song: [{ id: 'rec1' }] },
+        },
+      },
+    })
   })
 
-  it('shuffles songs when Shuffle is clicked', async () => {
+  describe('Shuffle action', () => {
-    const theme = createMuiTheme()
+    it('shuffles songs when clicked', async () => {
-    render(
+      renderArtistActions()
-      <TestContext>
+      clickActionButton('shuffle')
-        <ThemeProvider theme={theme}>
-          <ArtistActions record={{ id: 'ar1' }} />
-        </ThemeProvider>
-      </TestContext>,
-    )
 
-    fireEvent.click(screen.getByText('resources.artist.actions.shuffle'))
       await waitFor(() =>
         expect(mockGetList).toHaveBeenCalledWith('song', {
           pagination: { page: 1, perPage: 500 },
@@ -59,21 +81,108 @@ describe('ArtistActions', () => {
       )
       expect(mockDispatch).toHaveBeenCalled()
     })
+  })
 
-  it('starts radio when Radio is clicked', async () => {
+  describe('Radio action', () => {
-    const theme = createMuiTheme()
+    it('starts radio when clicked', async () => {
-    render(
+      renderArtistActions()
-      <TestContext>
+      clickActionButton('radio')
-        <ThemeProvider theme={theme}>
-          <ArtistActions record={{ id: 'ar1' }} />
-        </ThemeProvider>
-      </TestContext>,
-    )
 
-    fireEvent.click(screen.getByText('resources.artist.actions.radio'))
       await waitFor(() =>
         expect(subsonic.getSimilarSongs2).toHaveBeenCalledWith('ar1', 100),
       )
       expect(mockDispatch).toHaveBeenCalled()
     })
   })
+
+  describe('Play action', () => {
+    it('plays top songs when clicked', async () => {
+      renderArtistActions()
+      clickActionButton('topSongs')
+
+      await waitFor(() =>
+        expect(subsonic.getTopSongs).toHaveBeenCalledWith('Artist', 100),
+      )
+      expect(mockDispatch).toHaveBeenCalled()
+    })
+
+    it('handles API rejection', async () => {
+      subsonic.getTopSongs.mockRejectedValue(new Error('Network error'))
+
+      renderArtistActions()
+      clickActionButton('topSongs')
+
+      await waitFor(() =>
+        expect(subsonic.getTopSongs).toHaveBeenCalledWith('Artist', 100),
+      )
+      expect(mockNotify).toHaveBeenCalledWith('ra.page.error', 'warning')
+      expect(mockDispatch).not.toHaveBeenCalled()
+    })
+
+    it('handles failed API response', async () => {
+      subsonic.getTopSongs.mockResolvedValue({
+        json: {
+          'subsonic-response': {
+            status: 'failed',
+            error: { code: 40, message: 'Wrong username or password' },
+          },
+        },
+      })
+
+      renderArtistActions()
+      clickActionButton('topSongs')
+
+      await waitFor(() =>
+        expect(subsonic.getTopSongs).toHaveBeenCalledWith('Artist', 100),
+      )
+      expect(mockNotify).toHaveBeenCalledWith('ra.page.error', 'warning')
+      expect(mockDispatch).not.toHaveBeenCalled()
+    })
+
+    it('handles empty song list', async () => {
+      subsonic.getTopSongs.mockResolvedValue({
+        json: {
+          'subsonic-response': {
+            status: 'ok',
+            topSongs: { song: [] },
+          },
+        },
+      })
+
+      renderArtistActions()
+      clickActionButton('topSongs')
+
+      await waitFor(() =>
+        expect(subsonic.getTopSongs).toHaveBeenCalledWith('Artist', 100),
+      )
+      expect(mockNotify).toHaveBeenCalledWith(
+        'message.noTopSongsFound',
+        'warning',
+      )
+      expect(mockDispatch).not.toHaveBeenCalled()
+    })
+
+    it('handles missing topSongs property', async () => {
+      subsonic.getTopSongs.mockResolvedValue({
+        json: {
+          'subsonic-response': {
+            status: 'ok',
+            // topSongs property is missing
+          },
+        },
+      })
+
+      renderArtistActions()
+      clickActionButton('topSongs')
+
+      await waitFor(() =>
+        expect(subsonic.getTopSongs).toHaveBeenCalledWith('Artist', 100),
+      )
+      expect(mockNotify).toHaveBeenCalledWith(
+        'message.noTopSongsFound',
+        'warning',
+      )
+      expect(mockDispatch).not.toHaveBeenCalled()
+    })
+  })
+})

ui/src/artist/actions.js

@@ -0,0 +1,68 @@
+import subsonic from '../subsonic/index.js'
+import { playTracks } from '../actions/index.js'
+
+export const playTopSongs = async (dispatch, notify, artistName) => {
+  const res = await subsonic.getTopSongs(artistName, 100)
+  const data = res.json['subsonic-response']
+
+  if (data.status !== 'ok') {
+    throw new Error(
+      `Error fetching top songs: ${data.error?.message || 'Unknown error'} (Code: ${data.error?.code || 'unknown'})`,
+    )
+  }
+
+  const songs = data.topSongs?.song || []
+  if (!songs.length) {
+    notify('message.noTopSongsFound', 'warning')
+    return
+  }
+
+  const songData = {}
+  const ids = []
+  songs.forEach((s) => {
+    songData[s.id] = s
+    ids.push(s.id)
+  })
+  dispatch(playTracks(songData, ids))
+}
+
+export const playSimilar = async (dispatch, notify, id) => {
+  const res = await subsonic.getSimilarSongs2(id, 100)
+  const data = res.json['subsonic-response']
+
+  if (data.status !== 'ok') {
+    throw new Error(
+      `Error fetching similar songs: ${data.error?.message || 'Unknown error'} (Code: ${data.error?.code || 'unknown'})`,
+    )
+  }
+
+  const songs = data.similarSongs2?.song || []
+  if (!songs.length) {
+    notify('message.noSimilarSongsFound', 'warning')
+    return
+  }
+
+  const songData = {}
+  const ids = []
+  songs.forEach((s) => {
+    songData[s.id] = s
+    ids.push(s.id)
+  })
+  dispatch(playTracks(songData, ids))
+}
+
+export const playShuffle = async (dataProvider, dispatch, id) => {
+  const res = await dataProvider.getList('song', {
+    pagination: { page: 1, perPage: 500 },
+    sort: { field: 'random', order: 'ASC' },
+    filter: { album_artist_id: id, missing: false },
+  })
+
+  const data = {}
+  const ids = []
+  res.data.forEach((s) => {
+    data[s.id] = s
+    ids.push(s.id)
+  })
+  dispatch(playTracks(data, ids))
+}

ui/src/i18n/en.json

@@ -127,6 +127,7 @@
         "performer": "Performer |||| Performers"
       },
       "actions": {
+        "topSongs": "Top Songs",
         "shuffle": "Shuffle",
         "radio": "Radio"
       }
@@ -415,6 +416,7 @@
     "transcodingEnabled": "Navidrome is currently running with %{config}, making it possible to run system commands from the transcoding settings using the web interface. We recommend to disable it for security reasons and only enable it when configuring Transcoding options.",
     "songsAddedToPlaylist": "Added 1 song to playlist |||| Added %{smart_count} songs to playlist",
     "noSimilarSongsFound": "No similar songs found",
+    "noTopSongsFound": "No top songs found",
     "noPlaylistsAvailable": "None available",
     "delete_user_title": "Delete user '%{name}'",
     "delete_user_content": "Are you sure you want to delete this user and all their data (including playlists and preferences)?",

ui/src/subsonic/index.js

@@ -86,6 +86,10 @@ const getSimilarSongs2 = (id, count = 100) => {
   return httpClient(url('getSimilarSongs2', id, { count }))
 }
 
+const getTopSongs = (artist, count = 50) => {
+  return httpClient(url('getTopSongs', null, { artist, count }))
+}
+
 const streamUrl = (id, options) => {
   return baseUrl(
     url('stream', id, {
@@ -110,5 +114,6 @@ export default {
   streamUrl,
   getAlbumInfo,
   getArtistInfo,
+  getTopSongs,
   getSimilarSongs2,
 }

ui/src/utils/index.js

@@ -3,4 +3,3 @@ export * from './intersperse'
 export * from './notifications'
 export * from './openInNewTab'
 export * from './urls'
-export * from './playSimilar'

ui/src/utils/playSimilar.js

@@ -1,27 +0,0 @@
-import subsonic from '../subsonic'
-import { playTracks } from '../actions'
-
-export const playSimilar = async (dispatch, notify, id) => {
-  const res = await subsonic.getSimilarSongs2(id, 100)
-  const data = res.json['subsonic-response']
-
-  if (data.status !== 'ok') {
-    throw new Error(
-      `Error fetching similar songs: ${data.error?.message || 'Unknown error'} (Code: ${data.error?.code || 'unknown'})`,
-    )
-  }
-
-  const songs = data.similarSongs2?.song || []
-  if (!songs.length) {
-    notify('message.noSimilarSongsFound', 'warning')
-    return
-  }
-
-  const songData = {}
-  const ids = []
-  songs.forEach((s) => {
-    songData[s.id] = s
-    ids.push(s.id)
-  })
-  dispatch(playTracks(songData, ids))
-}