feat(ui): add song Love and Rating functionality to playlist view (#4134)

* feat(ui): add playlist track love button

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

* feat(ui): add star rating feature for playlist tracks

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

* fix(ui): handle loading state and error logging in toggle love and rating components

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

---------

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

persistence/playlist_track_repository.go

@@ -99,10 +99,10 @@ func (r *playlistTrackRepository) Read(id string) (interface{}, error) {
 			"playlist_tracks.*",
 		).
 		Join("media_file f on f.id = media_file_id").
-		Where(And{Eq{"playlist_id": r.playlistId}, Eq{"id": id}})
+		Where(And{Eq{"playlist_id": r.playlistId}, Eq{"playlist_tracks.id": id}})
 	var trk dbPlaylistTrack
 	err := r.queryOne(sel, &trk)
-	return trk.PlaylistTrack.MediaFile, err
+	return trk.PlaylistTrack, err
 }
 
 func (r *playlistTrackRepository) GetAll(options ...model.QueryOptions) (model.PlaylistTracks, error) {

server/nativeapi/native_api.go

@@ -133,6 +133,9 @@ func (n *Router) addPlaylistTrackRoute(r chi.Router) {
 		})
 		r.Route("/{id}", func(r chi.Router) {
 			r.Use(server.URLParamsMiddleware)
+			r.Get("/", func(w http.ResponseWriter, r *http.Request) {
+				getPlaylistTrack(n.ds)(w, r)
+			})
 			r.Put("/", func(w http.ResponseWriter, r *http.Request) {
 				reorderItem(n.ds)(w, r)
 			})

server/nativeapi/playlists.go

@@ -45,6 +45,23 @@ func getPlaylist(ds model.DataStore) http.HandlerFunc {
 	}
 }
 
+func getPlaylistTrack(ds model.DataStore) http.HandlerFunc {
+	// Add a middleware to capture the playlistId
+	wrapper := func(handler restHandler) http.HandlerFunc {
+		return func(w http.ResponseWriter, r *http.Request) {
+			constructor := func(ctx context.Context) rest.Repository {
+				plsRepo := ds.Playlist(ctx)
+				plsId := chi.URLParam(r, "playlistId")
+				return plsRepo.Tracks(plsId, true)
+			}
+
+			handler(constructor).ServeHTTP(w, r)
+		}
+	}
+
+	return wrapper(rest.Get)
+}
+
 func createPlaylistFromM3U(playlists core.Playlists) http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
 		ctx := r.Context()

ui/src/common/RatingField.jsx

@@ -38,15 +38,16 @@ export const RatingField = ({
 
   const handleRating = useCallback(
     (e, val) => {
-      rate(val ?? 0, e.target.name)
+      const targetId = record.mediaFileId || record.id
+      rate(val ?? 0, targetId)
     },
-    [rate],
+    [rate, record.mediaFileId, record.id],
   )
 
   return (
     <span onClick={(e) => stopPropagation(e)}>
       <Rating
-        name={record.id}
+        name={record.mediaFileId || record.id}
         className={clsx(
           className,
           classes.rating,

ui/src/common/useRating.jsx

@@ -17,18 +17,42 @@ export const useRating = (resource, record) => {
   }, [])
 
   const refreshRating = useCallback(() => {
-    dataProvider
-      .getOne(resource, { id: record.id })
-      .then(() => {
-        if (mountedRef.current) {
-          setLoading(false)
-        }
-      })
-      .catch((e) => {
-        // eslint-disable-next-line no-console
-        console.log('Error encountered: ' + e)
-      })
-  }, [dataProvider, record, resource])
+    // For playlist tracks, refresh both resources to keep data in sync
+    if (record.mediaFileId) {
+      // This is a playlist track - refresh both the playlist track and the song
+      const promises = [
+        dataProvider.getOne('song', { id: record.mediaFileId }),
+        dataProvider.getOne('playlistTrack', {
+          id: record.id,
+          filter: { playlist_id: record.playlistId },
+        }),
+      ]
+
+      Promise.all(promises)
+        .catch((e) => {
+          // eslint-disable-next-line no-console
+          console.log('Error encountered: ' + e)
+        })
+        .finally(() => {
+          if (mountedRef.current) {
+            setLoading(false)
+          }
+        })
+    } else {
+      // Regular song or other resource
+      dataProvider
+        .getOne(resource, { id: record.id })
+        .catch((e) => {
+          // eslint-disable-next-line no-console
+          console.log('Error encountered: ' + e)
+        })
+        .finally(() => {
+          if (mountedRef.current) {
+            setLoading(false)
+          }
+        })
+    }
+  }, [dataProvider, record.id, record.mediaFileId, record.playlistId, resource])
 
   const rate = (val, id) => {
     setLoading(true)

ui/src/common/useRating.test.js

@@ -0,0 +1,165 @@
+import { renderHook, act } from '@testing-library/react-hooks'
+import { vi, describe, it, expect, beforeEach } from 'vitest'
+import { useRating } from './useRating'
+import subsonic from '../subsonic'
+import { useDataProvider } from 'react-admin'
+
+vi.mock('../subsonic', () => ({
+  default: {
+    setRating: vi.fn(() => Promise.resolve()),
+  },
+}))
+
+vi.mock('react-admin', async () => {
+  const actual = await vi.importActual('react-admin')
+  return {
+    ...actual,
+    useDataProvider: vi.fn(),
+    useNotify: vi.fn(() => vi.fn()),
+  }
+})
+
+describe('useRating', () => {
+  let getOne
+  beforeEach(() => {
+    getOne = vi.fn(() => Promise.resolve())
+    useDataProvider.mockReturnValue({ getOne })
+    vi.clearAllMocks()
+  })
+
+  it('returns rating value from record', () => {
+    const record = { id: 'sg-1', rating: 3 }
+    const { result } = renderHook(() => useRating('song', record))
+    const [rate, rating, loading] = result.current
+    expect(rating).toBe(3)
+    expect(loading).toBe(false)
+    expect(typeof rate).toBe('function')
+  })
+
+  it('sets rating using targetId and calls setRating API', async () => {
+    const record = { id: 'sg-1', rating: 0 }
+    const { result } = renderHook(() => useRating('song', record))
+    await act(async () => {
+      await result.current[0](4, 'sg-1')
+    })
+    expect(subsonic.setRating).toHaveBeenCalledWith('sg-1', 4)
+    expect(getOne).toHaveBeenCalledWith('song', { id: 'sg-1' })
+  })
+
+  it('handles zero rating (unrate)', async () => {
+    const record = { id: 'sg-1', rating: 5 }
+    const { result } = renderHook(() => useRating('song', record))
+    await act(async () => {
+      await result.current[0](0, 'sg-1')
+    })
+    expect(subsonic.setRating).toHaveBeenCalledWith('sg-1', 0)
+  })
+
+  describe('playlist track scenarios', () => {
+    it('refreshes both playlist track and song for playlist tracks', async () => {
+      const record = {
+        id: 'pt-1',
+        mediaFileId: 'sg-1',
+        playlistId: 'pl-1',
+        rating: 2,
+      }
+      const { result } = renderHook(() => useRating('playlistTrack', record))
+      await act(async () => {
+        await result.current[0](5, 'sg-1')
+      })
+
+      // Should rate using the media file ID
+      expect(subsonic.setRating).toHaveBeenCalledWith('sg-1', 5)
+
+      // Should refresh both the playlist track and the song
+      expect(getOne).toHaveBeenCalledTimes(2)
+      expect(getOne).toHaveBeenCalledWith('playlistTrack', {
+        id: 'pt-1',
+        filter: { playlist_id: 'pl-1' },
+      })
+      expect(getOne).toHaveBeenCalledWith('song', { id: 'sg-1' })
+    })
+
+    it('includes playlist_id filter when refreshing playlist tracks', async () => {
+      const record = {
+        id: 'pt-5',
+        mediaFileId: 'sg-10',
+        playlistId: 'pl-123',
+        rating: 1,
+      }
+      const { result } = renderHook(() => useRating('playlistTrack', record))
+      await act(async () => {
+        await result.current[0](3, 'sg-10')
+      })
+
+      // Should rate using the media file ID
+      expect(subsonic.setRating).toHaveBeenCalledWith('sg-10', 3)
+
+      // Should refresh playlist track with correct playlist_id filter
+      expect(getOne).toHaveBeenCalledWith('playlistTrack', {
+        id: 'pt-5',
+        filter: { playlist_id: 'pl-123' },
+      })
+      // Should also refresh the underlying song
+      expect(getOne).toHaveBeenCalledWith('song', { id: 'sg-10' })
+    })
+
+    it('only refreshes original resource when no mediaFileId present', async () => {
+      const record = { id: 'sg-1', rating: 4 }
+      const { result } = renderHook(() => useRating('song', record))
+      await act(async () => {
+        await result.current[0](2, 'sg-1')
+      })
+
+      // Should only refresh the original resource (song)
+      expect(getOne).toHaveBeenCalledTimes(1)
+      expect(getOne).toHaveBeenCalledWith('song', { id: 'sg-1' })
+    })
+
+    it('does not include playlist_id filter for non-playlist resources', async () => {
+      const record = { id: 'sg-1', rating: 0 }
+      const { result } = renderHook(() => useRating('song', record))
+      await act(async () => {
+        await result.current[0](5, 'sg-1')
+      })
+
+      // Should refresh without any filter
+      expect(getOne).toHaveBeenCalledWith('song', { id: 'sg-1' })
+    })
+  })
+
+  describe('component integration scenarios', () => {
+    it('handles mediaFileId fallback correctly for playlist tracks', async () => {
+      const record = {
+        id: 'pt-1',
+        mediaFileId: 'sg-1',
+        playlistId: 'pl-1',
+        rating: 0,
+      }
+      const { result } = renderHook(() => useRating('playlistTrack', record))
+
+      // Simulate RatingField component behavior: uses mediaFileId || record.id
+      const targetId = record.mediaFileId || record.id
+      await act(async () => {
+        await result.current[0](4, targetId)
+      })
+
+      expect(subsonic.setRating).toHaveBeenCalledWith('sg-1', 4)
+    })
+
+    it('handles regular song rating without mediaFileId', async () => {
+      const record = { id: 'sg-1', rating: 2 }
+      const { result } = renderHook(() => useRating('song', record))
+
+      // Simulate RatingField component behavior: uses mediaFileId || record.id
+      const targetId = record.mediaFileId || record.id
+      await act(async () => {
+        await result.current[0](5, targetId)
+      })
+
+      expect(subsonic.setRating).toHaveBeenCalledWith('sg-1', 5)
+      expect(getOne).toHaveBeenCalledTimes(1)
+      expect(getOne).toHaveBeenCalledWith('song', { id: 'sg-1' })
+    })
+  })
+})

ui/src/common/useToggleLove.jsx

@@ -17,18 +17,38 @@ export const useToggleLove = (resource, record = {}) => {
   const dataProvider = useDataProvider()
 
   const refreshRecord = useCallback(() => {
-    dataProvider.getOne(resource, { id: record.id }).then(() => {
-      if (mountedRef.current) {
-        setLoading(false)
-      }
-    })
-  }, [dataProvider, record.id, resource])
+    const promises = []
+
+    // Always refresh the original resource
+    const params = { id: record.id }
+    if (record.playlistId) {
+      params.filter = { playlist_id: record.playlistId }
+    }
+    promises.push(dataProvider.getOne(resource, params))
+
+    // If we have a mediaFileId, also refresh the song
+    if (record.mediaFileId) {
+      promises.push(dataProvider.getOne('song', { id: record.mediaFileId }))
+    }
+
+    Promise.all(promises)
+      .catch((e) => {
+        // eslint-disable-next-line no-console
+        console.log('Error encountered: ' + e)
+      })
+      .finally(() => {
+        if (mountedRef.current) {
+          setLoading(false)
+        }
+      })
+  }, [dataProvider, record.mediaFileId, record.id, record.playlistId, resource])
 
   const toggleLove = () => {
     const toggle = record.starred ? subsonic.unstar : subsonic.star
+    const id = record.mediaFileId || record.id
 
     setLoading(true)
-    toggle(record.id)
+    toggle(id)
       .then(refreshRecord)
       .catch((e) => {
         // eslint-disable-next-line no-console

ui/src/common/useToggleLove.test.js

@@ -0,0 +1,136 @@
+import { renderHook, act } from '@testing-library/react-hooks'
+import { vi, describe, it, expect, beforeEach } from 'vitest'
+import { useToggleLove } from './useToggleLove'
+import subsonic from '../subsonic'
+import { useDataProvider } from 'react-admin'
+
+vi.mock('../subsonic', () => ({
+  default: {
+    star: vi.fn(() => Promise.resolve()),
+    unstar: vi.fn(() => Promise.resolve()),
+  },
+}))
+
+vi.mock('react-admin', async () => {
+  const actual = await vi.importActual('react-admin')
+  return {
+    ...actual,
+    useDataProvider: vi.fn(),
+    useNotify: vi.fn(() => vi.fn()),
+  }
+})
+
+describe('useToggleLove', () => {
+  let getOne
+  beforeEach(() => {
+    getOne = vi.fn(() => Promise.resolve())
+    useDataProvider.mockReturnValue({ getOne })
+    vi.clearAllMocks()
+  })
+
+  it('uses mediaFileId when present', async () => {
+    const record = { id: 'pt-1', mediaFileId: 'sg-1', starred: false }
+    const { result } = renderHook(() => useToggleLove('song', record))
+    await act(async () => {
+      await result.current[0]()
+    })
+    expect(subsonic.star).toHaveBeenCalledWith('sg-1')
+    expect(getOne).toHaveBeenCalledWith('song', { id: 'sg-1' })
+  })
+
+  it('falls back to id when mediaFileId not present', async () => {
+    const record = { id: 'sg-1', starred: false }
+    const { result } = renderHook(() => useToggleLove('song', record))
+    await act(async () => {
+      await result.current[0]()
+    })
+    expect(subsonic.star).toHaveBeenCalledWith('sg-1')
+    expect(getOne).toHaveBeenCalledWith('song', { id: 'sg-1' })
+  })
+
+  it('calls unstar when record is already loved', async () => {
+    const record = { id: 'sg-1', starred: true }
+    const { result } = renderHook(() => useToggleLove('song', record))
+    await act(async () => {
+      await result.current[0]()
+    })
+    expect(subsonic.unstar).toHaveBeenCalledWith('sg-1')
+  })
+
+  describe('playlist track scenarios', () => {
+    it('refreshes both playlist track and song for playlist tracks', async () => {
+      const record = {
+        id: 'pt-1',
+        mediaFileId: 'sg-1',
+        playlistId: 'pl-1',
+        starred: false,
+      }
+      const { result } = renderHook(() =>
+        useToggleLove('playlistTrack', record),
+      )
+      await act(async () => {
+        await result.current[0]()
+      })
+
+      // Should star using the media file ID
+      expect(subsonic.star).toHaveBeenCalledWith('sg-1')
+
+      // Should refresh both the playlist track and the song
+      expect(getOne).toHaveBeenCalledTimes(2)
+      expect(getOne).toHaveBeenCalledWith('playlistTrack', {
+        id: 'pt-1',
+        filter: { playlist_id: 'pl-1' },
+      })
+      expect(getOne).toHaveBeenCalledWith('song', { id: 'sg-1' })
+    })
+
+    it('includes playlist_id filter when refreshing playlist tracks', async () => {
+      const record = {
+        id: 'pt-5',
+        mediaFileId: 'sg-10',
+        playlistId: 'pl-123',
+        starred: true,
+      }
+      const { result } = renderHook(() =>
+        useToggleLove('playlistTrack', record),
+      )
+      await act(async () => {
+        await result.current[0]()
+      })
+
+      // Should unstar using the media file ID
+      expect(subsonic.unstar).toHaveBeenCalledWith('sg-10')
+
+      // Should refresh playlist track with correct playlist_id filter
+      expect(getOne).toHaveBeenCalledWith('playlistTrack', {
+        id: 'pt-5',
+        filter: { playlist_id: 'pl-123' },
+      })
+      // Should also refresh the underlying song
+      expect(getOne).toHaveBeenCalledWith('song', { id: 'sg-10' })
+    })
+
+    it('only refreshes original resource when no mediaFileId present', async () => {
+      const record = { id: 'sg-1', starred: false }
+      const { result } = renderHook(() => useToggleLove('song', record))
+      await act(async () => {
+        await result.current[0]()
+      })
+
+      // Should only refresh the original resource (song)
+      expect(getOne).toHaveBeenCalledTimes(1)
+      expect(getOne).toHaveBeenCalledWith('song', { id: 'sg-1' })
+    })
+
+    it('does not include playlist_id filter for non-playlist resources', async () => {
+      const record = { id: 'sg-1', starred: false }
+      const { result } = renderHook(() => useToggleLove('song', record))
+      await act(async () => {
+        await result.current[0]()
+      })
+
+      // Should refresh without any filter
+      expect(getOne).toHaveBeenCalledWith('song', { id: 'sg-1' })
+    })
+  })
+})

ui/src/playlist/PlaylistSongs.jsx

@@ -26,11 +26,13 @@ import {
   useResourceRefresh,
   DateField,
   ArtistLinkField,
+  RatingField,
 } from '../common'
 import { AlbumLinkField } from '../song/AlbumLinkField'
 import { playTracks } from '../actions'
 import PlaylistSongBulkActions from './PlaylistSongBulkActions'
 import ExpandInfoDialog from '../dialogs/ExpandInfoDialog'
+import config from '../config'
 
 const useStyles = makeStyles(
   (theme) => ({
@@ -66,11 +68,17 @@ const useStyles = makeStyles(
         '& $contextMenu': {
           visibility: 'visible',
         },
+        '& $ratingField': {
+          visibility: 'visible',
+        },
       },
     },
     contextMenu: {
       visibility: (props) => (props.isDesktop ? 'hidden' : 'visible'),
     },
+    ratingField: {
+      visibility: 'hidden',
+    },
   }),
   { name: 'RaList' },
 )
@@ -161,8 +169,16 @@ const PlaylistSongs = ({ playlistId, readOnly, actions, ...props }) => {
       quality: isDesktop && <QualityInfo source="quality" sortable={false} />,
       channels: isDesktop && <NumberField source="channels" />,
       bpm: isDesktop && <NumberField source="bpm" />,
+      rating: config.enableStarRating && (
+        <RatingField
+          source="rating"
+          sortByOrder={'DESC'}
+          resource={'song'}
+          className={classes.ratingField}
+        />
+      ),
     }
-  }, [isDesktop, classes.draggable])
+  }, [isDesktop, classes.draggable, classes.ratingField])
 
   const columns = useSelectedFields({
     resource: 'playlistTrack',
@@ -174,6 +190,7 @@ const PlaylistSongs = ({ playlistId, readOnly, actions, ...props }) => {
       'playCount',
       'playDate',
       'albumArtist',
+      'rating',
     ],
   })
 
@@ -213,7 +230,7 @@ const PlaylistSongs = ({ playlistId, readOnly, actions, ...props }) => {
               {columns}
               <SongContextMenu
                 onAddToPlaylist={onAddToPlaylist}
-                showLove={false}
+                showLove={true}
                 className={classes.contextMenu}
               />
             </SongDatagrid>