nik/navidrome
1
0
Fork 0
mirror of https://github.com/navidrome/navidrome.git synced 2026-08-31 07:30:32 +00:00
Code Issues Packages Projects Releases Wiki Activity

Merge de4484da855f39cbfd5ac78886fa45d4b174b124 into dbd26ba2e71d0a5b79dba873a2beeff59f1cd8dd

Browse Source
This commit is contained in:
polybjorn 2026-08-31 08:23:22 +03:00 committed by GitHub
commit 14fc9cec61
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 65 additions and 70 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

22
ui/src/common/useRating.jsx
View File

@ -1,11 +1,12 @@
import { useState, useCallback, useEffect, useRef } from 'react'
import { useDataProvider, useNotify } from 'react-admin'
import { useDataProvider, useNotify, useRefresh } from 'react-admin'
import subsonic from '../subsonic'
export const useRating = (resource, record) => {
const [loading, setLoading] = useState(false)
const notify = useNotify()
const dataProvider = useDataProvider()
const refresh = useRefresh()
const mountedRef = useRef(false)
const rating = record.rating
@ -17,23 +18,18 @@ export const useRating = (resource, record) => {
}, [])
const refreshRating = useCallback(() => {
// 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)
// A playlistTrack id is a position, not a stable key: rating a song can drop it out
// of a smart playlist, and that position then holds a different track. Refetching
// the row by id would write the neighbour's data under this row, so reload the list.
dataProvider
.getOne('song', { id: record.mediaFileId })
.catch((e) => {
// eslint-disable-next-line no-console
console.log('Error encountered: ' + e)
})
.finally(() => {
refresh()
if (mountedRef.current) {
setLoading(false)
}
@ -52,7 +48,7 @@ export const useRating = (resource, record) => {
}
})
}
}, [dataProvider, record.id, record.mediaFileId, record.playlistId, resource])
}, [dataProvider, record.id, record.mediaFileId, refresh, resource])
const rate = (val, id) => {
setLoading(true)

43
ui/src/common/useRating.test.js
View File

@ -4,6 +4,8 @@ import { useRating } from './useRating'
import subsonic from '../subsonic'
import { useDataProvider } from 'react-admin'
const mockRefresh = vi.fn()
vi.mock('../subsonic', () => ({
default: {
setRating: vi.fn(() => Promise.resolve()),
@ -16,13 +18,16 @@ vi.mock('react-admin', async () => {
...actual,
useDataProvider: vi.fn(),
useNotify: vi.fn(() => vi.fn()),
useRefresh: vi.fn(() => mockRefresh),
}
})
describe('useRating', () => {
let getOne
beforeEach(() => {
getOne = vi.fn(() => Promise.resolve())
getOne = vi.fn((resource, params) =>
Promise.resolve({ data: { id: params.id } }),
)
useDataProvider.mockReturnValue({ getOne })
vi.clearAllMocks()
})
@ -56,9 +61,9 @@ describe('useRating', () => {
})
describe('playlist track scenarios', () => {
it('refreshes both playlist track and song for playlist tracks', async () => {
it('refreshes the song and reloads the list for playlist tracks', async () => {
const record = {
id: 'pt-1',
id: '1',
mediaFileId: 'sg-1',
playlistId: 'pl-1',
rating: 2,
@ -71,18 +76,21 @@ describe('useRating', () => {
// 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' },
})
// The row is a position in the playlist, so it cannot be refetched by id:
// rating can drop the track out of a smart playlist and shift every row up
expect(getOne).toHaveBeenCalledTimes(1)
expect(getOne).toHaveBeenCalledWith('song', { id: 'sg-1' })
expect(getOne).not.toHaveBeenCalledWith(
'playlistTrack',
expect.anything(),
)
expect(mockRefresh).toHaveBeenCalled()
})
it('includes playlist_id filter when refreshing playlist tracks', async () => {
it('reloads the list even when the song refresh fails', async () => {
getOne.mockImplementation(() => Promise.reject(new Error('boom')))
const record = {
id: 'pt-5',
id: '5',
mediaFileId: 'sg-10',
playlistId: 'pl-123',
rating: 1,
@ -92,16 +100,8 @@ describe('useRating', () => {
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' })
expect(mockRefresh).toHaveBeenCalled()
})
it('only refreshes original resource when no mediaFileId present', async () => {
@ -111,9 +111,10 @@ describe('useRating', () => {
await result.current[0](2, 'sg-1')
})
// Should only refresh the original resource (song)
// Should only refresh the original resource (song), without reloading the list
expect(getOne).toHaveBeenCalledTimes(1)
expect(getOne).toHaveBeenCalledWith('song', { id: 'sg-1' })
expect(mockRefresh).not.toHaveBeenCalled()
})
it('does not include playlist_id filter for non-playlist resources', async () => {

31
ui/src/common/useToggleLove.jsx
View File

@ -1,5 +1,5 @@
import { useCallback, useEffect, useRef, useState } from 'react'
import { useDataProvider, useNotify } from 'react-admin'
import { useDataProvider, useNotify, useRefresh } from 'react-admin'
import subsonic from '../subsonic'
export const useToggleLove = (resource, record = {}) => {
@ -15,33 +15,32 @@ export const useToggleLove = (resource, record = {}) => {
}, [])
const dataProvider = useDataProvider()
const refresh = useRefresh()
const refreshRecord = useCallback(() => {
const promises = []
// A playlistTrack id is a position, not a stable key: loving a song can drop it out of
// a smart playlist, and that position then holds a different track. Refetching the row
// by id would write the neighbour's data under this row, so reload the list instead.
const isPlaylistTrack = !!record.mediaFileId
const target = isPlaylistTrack
? { resource: 'song', params: { id: record.mediaFileId } }
: { resource, params: { id: record.id } }
// 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)
dataProvider
.getOne(target.resource, target.params)
.catch((e) => {
// eslint-disable-next-line no-console
console.log('Error encountered: ' + e)
})
.finally(() => {
if (isPlaylistTrack) {
refresh()
}
if (mountedRef.current) {
setLoading(false)
}
})
}, [dataProvider, record.mediaFileId, record.id, record.playlistId, resource])
}, [dataProvider, record.mediaFileId, record.id, refresh, resource])
const toggleLove = () => {
const toggle = record.starred ? subsonic.unstar : subsonic.star

39
ui/src/common/useToggleLove.test.js
View File

@ -4,6 +4,8 @@ import { useToggleLove } from './useToggleLove'
import subsonic from '../subsonic'
import { useDataProvider } from 'react-admin'
const mockRefresh = vi.fn()
vi.mock('../subsonic', () => ({
default: {
star: vi.fn(() => Promise.resolve()),
@ -17,6 +19,7 @@ vi.mock('react-admin', async () => {
...actual,
useDataProvider: vi.fn(),
useNotify: vi.fn(() => vi.fn()),
useRefresh: vi.fn(() => mockRefresh),
}
})
@ -58,9 +61,9 @@ describe('useToggleLove', () => {
})
describe('playlist track scenarios', () => {
it('refreshes both playlist track and song for playlist tracks', async () => {
it('refreshes the song and reloads the list for playlist tracks', async () => {
const record = {
id: 'pt-1',
id: '1',
mediaFileId: 'sg-1',
playlistId: 'pl-1',
starred: false,
@ -75,18 +78,21 @@ describe('useToggleLove', () => {
// 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' },
})
// The row is a position in the playlist, so it cannot be refetched by id:
// loving can drop the track out of a smart playlist and shift every row up
expect(getOne).toHaveBeenCalledTimes(1)
expect(getOne).toHaveBeenCalledWith('song', { id: 'sg-1' })
expect(getOne).not.toHaveBeenCalledWith(
'playlistTrack',
expect.anything(),
)
expect(mockRefresh).toHaveBeenCalled()
})
it('includes playlist_id filter when refreshing playlist tracks', async () => {
it('reloads the list even when the song refresh fails', async () => {
getOne.mockImplementation(() => Promise.reject(new Error('boom')))
const record = {
id: 'pt-5',
id: '5',
mediaFileId: 'sg-10',
playlistId: 'pl-123',
starred: true,
@ -98,16 +104,8 @@ describe('useToggleLove', () => {
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' })
expect(mockRefresh).toHaveBeenCalled()
})
it('only refreshes original resource when no mediaFileId present', async () => {
@ -117,9 +115,10 @@ describe('useToggleLove', () => {
await result.current[0]()
})
// Should only refresh the original resource (song)
// Should only refresh the original resource (song), without reloading the list
expect(getOne).toHaveBeenCalledTimes(1)
expect(getOne).toHaveBeenCalledWith('song', { id: 'sg-1' })
expect(mockRefresh).not.toHaveBeenCalled()
})
it('does not include playlist_id filter for non-playlist resources', async () => {