feat(ui): playlist favourites (heart, list filter, sidebar favourites-only toggle) (#5805)

* feat(playlists): register starred REST filter on playlist repository

* feat(ui): add persisted sidebarPlaylistsOnlyFavourites setting

* feat(ui): playlist favourites heart column and list filter

* feat(ui): favourite heart on playlist details header

* feat(ui): sidebar favourites-only playlist toggle with live refresh

* fix(playlists): qualify id in REST filter to avoid ambiguous column

The annotation join in selectPlaylist made a bare id filter ambiguous, so
GET /api/playlist?id=X (react-admin getMany, used by the sidebar refetch and
useResourceRefresh) failed with 'ambiguous column name: id'. Register
idFilter("playlist") like the album/artist/mediafile repos.

* fix(ui): refresh favourites sidebar on local star toggle

The SSE broker skips the client that originated a star, so the acting client
never got the refreshResource echo and its favourites-only sidebar went stale.
Key the sidebar query on a fingerprint of locally-known starred playlists so a
star/unstar on this client refetches; SSE still covers other clients.

* style(ui): prettier-format PlaylistsSubMenu test

* feat(ui): refine playlist favourites layout

- Move the favourite heart column to just before the edit button
- Space the Playlists sidebar header text from its action icons
- Use a list icon instead of a cog for the playlist-management action

* feat(ui): make the playlist Favourite column toggleable

Move the heart into the toggleable fields map so users can show/hide it
from the column selector like the other optional columns, keeping it last
so it stays just before the edit button.

* refactor(ui): memoize sidebar star fingerprint and gate it on favourites-only

- Derive starFingerprint via useMemo on the playlist data reference instead
  of recomputing sort/join inside useSelector on every app-wide dispatch.
- Only include the fingerprint in the query payload when favourites-only is
  on, so a star toggle no longer refetches the sidebar when it shows all
  playlists.

* fix(ui): don't refetch favourites sidebar on SSE events when showing all

When favourites-only is off the sidebar shows every playlist, so a star
event from another client changes nothing visible. Gate the SSE-driven
refresh counter (and its payload key) on onlyFavourites so the sidebar no
longer redraws on unrelated playlist star events.

* fix(ui): address automated review feedback on playlist favourites

- Ignore a persisted favourites-only preference when EnableFavourites is off,
  so disabling the feature later can't strand a filtered sidebar (Codex P2).
- Make PlaylistLove's datagrid header props explicit (source/sortable via
  defaultProps, className forwarded) instead of relying on prop pass-through.
- Add aria-label to the SubMenu secondary action button.
- Cover the PlaylistLove list column with tests (Codex P1).
This commit is contained in:
Deluan Quintão 2026-07-18 14:45:47 -04:00 committed by GitHub
parent 64430af9ce
commit deaa5e6c02
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
12 changed files with 437 additions and 16 deletions

View File

@ -51,8 +51,10 @@ func NewPlaylistRepository(ctx context.Context, db dbx.Builder) model.PlaylistRe
r.ctx = ctx
r.db = db
r.registerModel(&model.Playlist{}, map[string]filterFunc{
"q": playlistFilter,
"smart": smartPlaylistFilter,
"id": idFilter("playlist"),
"q": playlistFilter,
"smart": smartPlaylistFilter,
"starred": annotationBoolFilter("starred"),
})
r.setSortMappings(map[string]string{
"owner_name": "owner_name",

View File

@ -4,6 +4,7 @@ import (
"slices"
"github.com/Masterminds/squirrel"
"github.com/deluan/rest"
"github.com/navidrome/navidrome/log"
"github.com/navidrome/navidrome/model"
"github.com/navidrome/navidrome/model/request"
@ -155,6 +156,34 @@ var _ = Describe("PlaylistRepository", func() {
Expect(count).To(Equal(int64(len(starred))))
})
It("filters starred playlists through the registered REST filter", func() {
Expect(repo.SetStar(true, plsID)).To(Succeed())
res, err := repo.(model.ResourceRepository).ReadAll(rest.QueryOptions{
Filters: map[string]any{"starred": "true"},
})
Expect(err).ToNot(HaveOccurred())
starred := res.(model.Playlists)
Expect(starred).To(ContainElement(HaveField("ID", plsID)))
for _, p := range starred {
Expect(p.Starred).To(BeTrue())
}
res, err = repo.(model.ResourceRepository).ReadAll(rest.QueryOptions{
Filters: map[string]any{"starred": "false"},
})
Expect(err).ToNot(HaveOccurred())
Expect(res.(model.Playlists)).ToNot(ContainElement(HaveField("ID", plsID)))
})
It("reads a playlist by id through the REST id filter without ambiguity", func() {
res, err := repo.(model.ResourceRepository).ReadAll(rest.QueryOptions{
Filters: map[string]any{"id": plsID},
})
Expect(err).ToNot(HaveOccurred())
Expect(res.(model.Playlists)).To(ContainElement(HaveField("ID", plsID)))
})
It("does not leak an annotation row of another item_type sharing the playlist id", func() {
// Older builds (and the star fallthrough) can leave a media_file-typed row
// under a playlist id; the item_type-scoped join must not surface or dupe it.

View File

@ -1,6 +1,8 @@
export const SET_NOTIFICATIONS_STATE = 'SET_NOTIFICATIONS_STATE'
export const SET_TOGGLEABLE_FIELDS = 'SET_TOGGLEABLE_FIELDS'
export const SET_OMITTED_FIELDS = 'SET_OMITTED_FIELDS'
export const SET_SIDEBAR_PLAYLISTS_FAVOURITES =
'SET_SIDEBAR_PLAYLISTS_FAVOURITES'
export const setNotificationsState = (enabled) => ({
type: SET_NOTIFICATIONS_STATE,
@ -16,3 +18,8 @@ export const setOmittedFields = (obj) => ({
type: SET_OMITTED_FIELDS,
data: obj,
})
export const setSidebarPlaylistsOnlyFavourites = (enabled) => ({
type: SET_SIDEBAR_PLAYLISTS_FAVOURITES,
data: enabled,
})

View File

@ -210,7 +210,8 @@
"songCount": "Songs",
"comment": "Comment",
"sync": "Auto-import",
"path": "Import from"
"path": "Import from",
"starred": "Favourite"
},
"actions": {
"selectPlaylist": "Select a playlist:",
@ -635,6 +636,7 @@
},
"albumList": "Albums",
"playlists": "Playlists",
"onlyFavourites": "Only show favourites",
"sharedPlaylists": "Shared Playlists",
"about": "About"
},

View File

@ -1,19 +1,24 @@
import React, { useCallback } from 'react'
import React, { useCallback, useMemo, useState } from 'react'
import { useDispatch, useSelector } from 'react-redux'
import {
MenuItemLink,
useDataProvider,
useNotify,
useQueryWithStore,
useTranslate,
} from 'react-admin'
import { useHistory } from 'react-router-dom'
import QueueMusicIcon from '@material-ui/icons/QueueMusic'
import { Typography } from '@material-ui/core'
import QueueMusicOutlinedIcon from '@material-ui/icons/QueueMusicOutlined'
import { BiCog } from 'react-icons/bi'
import FavoriteIcon from '@material-ui/icons/Favorite'
import FavoriteBorderIcon from '@material-ui/icons/FavoriteBorder'
import { BiListUl } from 'react-icons/bi'
import { useDrop } from 'react-dnd'
import SubMenu from './SubMenu'
import { canChangeTracks, OverflowTooltip } from '../common'
import { canChangeTracks, OverflowTooltip, useRefreshOnEvents } from '../common'
import { DraggableTypes } from '../consts'
import { setSidebarPlaylistsOnlyFavourites } from '../actions'
import config from '../config'
const PlaylistMenuItemLink = ({ pls, sidebarIsOpen }) => {
@ -53,6 +58,37 @@ const PlaylistMenuItemLink = ({ pls, sidebarIsOpen }) => {
const PlaylistsSubMenu = ({ state, setState, sidebarIsOpen, dense }) => {
const history = useHistory()
const dispatch = useDispatch()
const translate = useTranslate()
const onlyFavourites = useSelector(
(state) => state.settings.sidebarPlaylistsOnlyFavourites,
)
// Ignore a persisted preference when the feature is off, so disabling it later
// (with the toggle now hidden) doesn't strand the user on a filtered sidebar
const showFavouritesOnly = config.enableFavourites && onlyFavourites
const playlistData = useSelector(
(state) => state.admin.resources.playlist?.data,
)
// Fingerprint of local star state; changes only when a playlist is (un)starred,
// so a local toggle refetches the sidebar without the SSE echo the actor never gets
const starFingerprint = useMemo(() => {
const data = playlistData || {}
return Object.keys(data)
.filter((id) => data[id]?.starred)
.sort()
.join(',')
}, [playlistData])
const [refreshCount, setRefreshCount] = useState(0)
// Only the favourites-only view depends on star state changing elsewhere;
// when showing all playlists a star event from another client changes nothing
// async because useRefreshOnEvents calls .catch() on the returned value
const onRefresh = useCallback(async () => {
if (showFavouritesOnly) setRefreshCount((count) => count + 1)
}, [showFavouritesOnly])
useRefreshOnEvents({ events: ['playlist'], onRefresh })
// A changed payload signature makes useQueryWithStore refetch
const { data, loaded } = useQueryWithStore({
type: 'getList',
resource: 'playlist',
@ -62,6 +98,11 @@ const PlaylistsSubMenu = ({ state, setState, sidebarIsOpen, dense }) => {
perPage: config.maxSidebarPlaylists,
},
sort: { field: 'name' },
...(showFavouritesOnly && {
filter: { starred: true },
starFingerprint,
refresh: refreshCount,
}),
},
})
@ -98,6 +139,10 @@ const PlaylistsSubMenu = ({ state, setState, sidebarIsOpen, dense }) => {
[history],
)
const handleToggleFavourites = useCallback(() => {
dispatch(setSidebarPlaylistsOnlyFavourites(!onlyFavourites))
}, [dispatch, onlyFavourites])
return (
<>
<SubMenu
@ -107,8 +152,20 @@ const PlaylistsSubMenu = ({ state, setState, sidebarIsOpen, dense }) => {
name={'menu.playlists'}
icon={<QueueMusicIcon />}
dense={dense}
actionIcon={<BiCog />}
actionIcon={<BiListUl />}
onAction={onPlaylistConfig}
onSecondaryAction={
config.enableFavourites ? handleToggleFavourites : undefined
}
secondaryActionIcon={
onlyFavourites ? (
<FavoriteIcon fontSize={'small'} />
) : (
<FavoriteBorderIcon fontSize={'small'} />
)
}
secondaryActionTitle={translate('menu.onlyFavourites')}
secondaryActionActive={onlyFavourites}
>
{myPlaylists.map(renderPlaylistMenuItemLink)}
</SubMenu>

View File

@ -0,0 +1,180 @@
import React from 'react'
import { render, screen, fireEvent, act } from '@testing-library/react'
import { describe, it, expect, beforeEach, vi } from 'vitest'
import { Provider } from 'react-redux'
import { createStore, combineReducers } from 'redux'
import { ThemeProvider, createTheme } from '@material-ui/core/styles'
import { settingsReducer, activityReducer } from '../reducers'
import { processEvent, EVENT_REFRESH_RESOURCE } from '../actions'
import PlaylistsSubMenu from './PlaylistsSubMenu'
const mockUseQueryWithStore = vi.fn()
vi.mock('../config', () => ({
// losslessFormats is read at module-load time by common/QualityInfo.jsx,
// pulled in transitively via the '../common' barrel file
default: {
enableFavourites: true,
maxSidebarPlaylists: 100,
losslessFormats: '',
},
}))
vi.mock('react-dnd', () => ({
useDrop: () => [{}, () => {}],
}))
vi.mock('react-router-dom', () => ({
useHistory: () => ({ push: vi.fn() }),
}))
vi.mock('react-admin', async (importOriginal) => {
const actual = await importOriginal()
return {
...actual,
useTranslate: () => (x) => x,
useDataProvider: () => ({ addToPlaylist: vi.fn() }),
useNotify: () => vi.fn(),
useQueryWithStore: (query) => mockUseQueryWithStore(query),
MenuItemLink: ({ primaryText }) => <div>{primaryText}</div>,
}
})
const playlists = {
'pl-1': { id: 'pl-1', name: 'Mine', ownerId: 'user-1' },
'pl-2': { id: 'pl-2', name: 'Theirs', ownerId: 'user-2' },
}
const SET_PLAYLIST_DATA = 'TEST/SET_PLAYLIST_DATA'
const adminReducer = (state = { resources: {} }, action) =>
action.type === SET_PLAYLIST_DATA
? { resources: { playlist: { data: action.data } } }
: state
const renderMenu = (preloadedSettings = {}, preloadedPlaylistData) => {
const store = createStore(
combineReducers({
settings: settingsReducer,
activity: activityReducer,
admin: adminReducer,
}),
{
settings: preloadedSettings,
activity: {},
admin: {
resources: preloadedPlaylistData
? { playlist: { data: preloadedPlaylistData } }
: {},
},
},
)
const theme = createTheme()
render(
<Provider store={store}>
<ThemeProvider theme={theme}>
<PlaylistsSubMenu
state={{ menuPlaylists: true, menuSharedPlaylists: true }}
setState={vi.fn()}
sidebarIsOpen={true}
dense={false}
/>
</ThemeProvider>
</Provider>,
)
return store
}
const lastQuery = () =>
mockUseQueryWithStore.mock.calls[
mockUseQueryWithStore.mock.calls.length - 1
][0]
describe('<PlaylistsSubMenu />', () => {
beforeEach(() => {
vi.clearAllMocks()
localStorage.setItem('userId', 'user-1')
mockUseQueryWithStore.mockReturnValue({ data: playlists, loaded: true })
// SubMenu uses MUI's useMediaQuery, which needs window.matchMedia in jsdom
window.matchMedia = (query) => ({
matches: false,
media: query,
addListener: () => {},
removeListener: () => {},
})
// OverflowTooltip (via MenuItemLink) needs ResizeObserver, unavailable in jsdom
window.ResizeObserver = class {
observe() {}
unobserve() {}
disconnect() {}
}
})
it('queries without a starred filter by default', () => {
renderMenu()
expect(lastQuery().payload.filter).toBeUndefined()
expect(screen.getByText('Mine')).not.toBeNull()
expect(screen.getByText('Theirs')).not.toBeNull()
})
it('adds the starred filter when favourites-only is enabled', () => {
renderMenu({ sidebarPlaylistsOnlyFavourites: true })
expect(lastQuery().payload.filter).toEqual({ starred: true })
})
it('toggles the setting when the heart action is clicked', () => {
const store = renderMenu()
fireEvent.click(screen.getByTitle('menu.onlyFavourites'))
expect(store.getState().settings.sidebarPlaylistsOnlyFavourites).toBe(true)
expect(lastQuery().payload.filter).toEqual({ starred: true })
})
it('refetches on a playlist SSE event when favourites-only is on', async () => {
const store = renderMenu({ sidebarPlaylistsOnlyFavourites: true })
const before = lastQuery().payload.refresh
// useRefreshOnEvents compares Date.now() timestamps; make sure it advances
await act(() => new Promise((resolve) => setTimeout(resolve, 5)))
act(() => {
store.dispatch(
processEvent(EVENT_REFRESH_RESOURCE, { playlist: ['pl-1'] }),
)
})
expect(lastQuery().payload.refresh).toBe(before + 1)
})
it('does not change the query signature on an SSE event when favourites-only is off', async () => {
const store = renderMenu()
const before = JSON.stringify(lastQuery().payload)
await act(() => new Promise((resolve) => setTimeout(resolve, 5)))
act(() => {
store.dispatch(
processEvent(EVENT_REFRESH_RESOURCE, { playlist: ['pl-1'] }),
)
})
// Signature unchanged useQueryWithStore dedupes, no wasted refetch
expect(lastQuery().payload.refresh).toBeUndefined()
expect(JSON.stringify(lastQuery().payload)).toBe(before)
})
it('refetches when a playlist is starred locally (no SSE echo)', () => {
const store = renderMenu(
{ sidebarPlaylistsOnlyFavourites: true },
{ 'pl-1': { id: 'pl-1', name: 'Mine', ownerId: 'user-1' } },
)
const before = lastQuery().payload.starFingerprint
act(() => {
store.dispatch({
type: SET_PLAYLIST_DATA,
data: {
'pl-1': {
id: 'pl-1',
name: 'Mine',
ownerId: 'user-1',
starred: true,
},
},
})
})
expect(lastQuery().payload.starFingerprint).not.toBe(before)
expect(lastQuery().payload.starFingerprint).toContain('pl-1')
})
})

View File

@ -33,6 +33,9 @@ const useStyles = makeStyles(
menuHeader: {
width: '100%',
},
headerText: {
flexGrow: 1,
},
headerWrapper: {
display: 'flex',
'&:hover $actionIcon': {
@ -55,6 +58,10 @@ const SubMenu = ({
dense,
onAction,
actionIcon,
onSecondaryAction,
secondaryActionIcon,
secondaryActionTitle,
secondaryActionActive,
}) => {
const translate = useTranslate()
const classes = useStyles()
@ -70,6 +77,11 @@ const SubMenu = ({
}
}
const handleSecondaryClick = (e) => {
e.stopPropagation()
onSecondaryAction(e)
}
const header = (
<div className={classes.headerWrapper}>
<MenuItem
@ -81,9 +93,26 @@ const SubMenu = ({
<ListItemIcon className={classes.icon}>
{isOpen ? <ExpandMore /> : icon}
</ListItemIcon>
<Typography variant="inherit" color="textSecondary">
<Typography
variant="inherit"
color="textSecondary"
className={classes.headerText}
>
{translate(name)}
</Typography>
{onSecondaryAction && sidebarIsOpen && (
<IconButton
size={'small'}
title={secondaryActionTitle}
aria-label={secondaryActionTitle}
className={
isDesktop && !secondaryActionActive ? classes.actionIcon : null
}
onClick={handleSecondaryClick}
>
{secondaryActionIcon}
</IconButton>
)}
{onAction && sidebarIsOpen && (
<IconButton
size={'small'}

View File

@ -13,6 +13,7 @@ import {
CollapsibleComment,
DurationField,
ImageUploadOverlay,
LoveButton,
SizeField,
isWritable,
OverflowTooltip,
@ -81,6 +82,15 @@ const useStyles = makeStyles(
overflow: 'hidden',
textOverflow: 'ellipsis',
wordBreak: 'break-word',
minWidth: 0,
},
titleRow: {
display: 'flex',
alignItems: 'center',
},
loveButton: {
marginLeft: theme.spacing(0.5),
flexShrink: 0,
},
stats: {
marginTop: '1em',
@ -139,14 +149,24 @@ const PlaylistDetails = (props) => {
</div>
<div className={classes.details}>
<CardContent className={classes.content}>
<OverflowTooltip title={record.name || ''}>
<Typography
variant={isDesktop ? 'h5' : 'h6'}
className={classes.title}
>
{record.name || translate('ra.page.loading')}
</Typography>
</OverflowTooltip>
<div className={classes.titleRow}>
<OverflowTooltip title={record.name || ''}>
<Typography
variant={isDesktop ? 'h5' : 'h6'}
className={classes.title}
>
{record.name || translate('ra.page.loading')}
</Typography>
</OverflowTooltip>
<LoveButton
className={classes.loveButton}
record={record}
resource={'playlist'}
size={isDesktop ? 'default' : 'small'}
aria-label="love"
color="primary"
/>
</div>
<Typography component="p" className={classes.stats}>
{record.songCount ? (
<span>

View File

@ -4,6 +4,7 @@ import {
DateField,
EditButton,
Filter,
NullableBooleanInput,
NumberField,
ReferenceInput,
SearchInput,
@ -22,11 +23,14 @@ import {
CoverArtAvatar,
DurationField,
List,
LoveButton,
Writable,
isWritable,
useSelectedFields,
useResourceRefresh,
} from '../common'
import FavoriteIcon from '@material-ui/icons/Favorite'
import config from '../config'
import PlaylistListActions from './PlaylistListActions'
import ChangePublicStatusButton from './ChangePublicStatusButton'
@ -53,6 +57,12 @@ const PlaylistFilter = (props) => {
<SelectInput optionText="name" />
</ReferenceInput>
)}
{config.enableFavourites && (
<NullableBooleanInput
source="starred"
label={<FavoriteIcon fontSize={'small'} />}
/>
)}
</Filter>
)
}
@ -139,6 +149,13 @@ const PlaylistListBulkActions = (props) => {
)
}
// Datagrid reads `source`/`sortable`/`label` off this element for the column
// header; only record/resource are forwarded so they never leak onto the button.
export const PlaylistLove = ({ record, className }) => (
<LoveButton record={record} resource={'playlist'} className={className} />
)
PlaylistLove.defaultProps = { source: 'starred', sortable: false }
const PlaylistList = (props) => {
const isXsmall = useMediaQuery((theme) => theme.breakpoints.down('xs'))
const isDesktop = useMediaQuery((theme) => theme.breakpoints.up('md'))
@ -159,6 +176,7 @@ const PlaylistList = (props) => {
sync: !isXsmall && (
<ToggleAutoImport source="sync" sortByOrder={'DESC'} />
),
starred: config.enableFavourites && <PlaylistLove />,
}),
[isDesktop, isXsmall],
)

View File

@ -0,0 +1,34 @@
import React from 'react'
import { render, screen } from '@testing-library/react'
import { describe, it, expect, vi } from 'vitest'
import { PlaylistLove } from './PlaylistList'
vi.mock('../config', () => ({
default: { enableFavourites: true },
}))
vi.mock('../common', () => ({
LoveButton: ({ record, resource }) => (
<button data-testid="love" data-resource={resource}>
{record?.starred ? 'starred' : 'not-starred'}
</button>
),
}))
describe('<PlaylistLove />', () => {
it('renders a LoveButton bound to the playlist resource', () => {
render(<PlaylistLove record={{ id: 'pl-1', starred: true }} />)
const btn = screen.getByTestId('love')
expect(btn.getAttribute('data-resource')).toBe('playlist')
expect(btn.textContent).toBe('starred')
})
it('exposes datagrid header props so the column renders unsorted', () => {
// The Datagrid reads these off the element; the wrapper body must not
// forward them to the button (which would leak onto the DOM).
expect(PlaylistLove.defaultProps).toEqual({
source: 'starred',
sortable: false,
})
})
})

View File

@ -1,6 +1,7 @@
import {
SET_NOTIFICATIONS_STATE,
SET_OMITTED_FIELDS,
SET_SIDEBAR_PLAYLISTS_FAVOURITES,
SET_TOGGLEABLE_FIELDS,
} from '../actions'
@ -8,6 +9,7 @@ const initialState = {
notifications: false,
toggleableFields: {},
omittedFields: {},
sidebarPlaylistsOnlyFavourites: false,
}
export const settingsReducer = (previousState = initialState, payload) => {
@ -34,6 +36,11 @@ export const settingsReducer = (previousState = initialState, payload) => {
...data,
},
}
case SET_SIDEBAR_PLAYLISTS_FAVOURITES:
return {
...previousState,
sidebarPlaylistsOnlyFavourites: data,
}
default:
return previousState
}

View File

@ -0,0 +1,36 @@
import { describe, it, expect } from 'vitest'
import { settingsReducer } from './settingsReducer'
import {
SET_SIDEBAR_PLAYLISTS_FAVOURITES,
setSidebarPlaylistsOnlyFavourites,
} from '../actions'
describe('settingsReducer', () => {
it('defaults sidebarPlaylistsOnlyFavourites to false', () => {
const state = settingsReducer(undefined, { type: 'UNKNOWN' })
expect(state.sidebarPlaylistsOnlyFavourites).toBe(false)
})
it('enables the flag via the action creator', () => {
const state = settingsReducer(
undefined,
setSidebarPlaylistsOnlyFavourites(true),
)
expect(state.sidebarPlaylistsOnlyFavourites).toBe(true)
})
it('disables the flag and preserves other settings', () => {
const initial = settingsReducer(undefined, { type: 'UNKNOWN' })
const on = settingsReducer(initial, {
type: SET_SIDEBAR_PLAYLISTS_FAVOURITES,
data: true,
})
const off = settingsReducer(on, {
type: SET_SIDEBAR_PLAYLISTS_FAVOURITES,
data: false,
})
expect(off.sidebarPlaylistsOnlyFavourites).toBe(false)
expect(off.notifications).toEqual(initial.notifications)
expect(off.toggleableFields).toEqual(initial.toggleableFields)
})
})