From 588b6be075b54891806c33a0a14b18ba255a10e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Deluan=20Quint=C3=A3o?= Date: Thu, 29 May 2025 20:35:47 -0400 Subject: [PATCH 1/3] Fix playlist star filter --- model/playlist.go | 3 ++ persistence/playlist_repository.go | 14 ++++++--- persistence/playlist_repository_test.go | 40 +++++++++++++++++++++++++ server/subsonic/media_annotation.go | 12 ++++++++ ui/src/layout/PlaylistsSubMenu.jsx | 1 + ui/src/playlist/PlaylistDetails.jsx | 22 +++++++++++++- 6 files changed, 87 insertions(+), 5 deletions(-) diff --git a/model/playlist.go b/model/playlist.go index 96a431b45..c06fbeb9b 100644 --- a/model/playlist.go +++ b/model/playlist.go @@ -9,6 +9,8 @@ import ( ) type Playlist struct { + Annotations `structs:"-" hash:"ignore"` + ID string `structs:"id" json:"id"` Name string `structs:"name" json:"name"` Comment string `structs:"comment" json:"comment"` @@ -92,6 +94,7 @@ type Playlists []Playlist type PlaylistRepository interface { ResourceRepository + AnnotatedRepository CountAll(options ...QueryOptions) (int64, error) Exists(id string) (bool, error) Put(pls *Playlist) error diff --git a/persistence/playlist_repository.go b/persistence/playlist_repository.go index 743eca470..d7d516cf9 100644 --- a/persistence/playlist_repository.go +++ b/persistence/playlist_repository.go @@ -51,12 +51,15 @@ func NewPlaylistRepository(ctx context.Context, db dbx.Builder) model.PlaylistRe r := &playlistRepository{} r.ctx = ctx r.db = db + r.tableName = "playlist" r.registerModel(&model.Playlist{}, map[string]filterFunc{ - "q": playlistFilter, - "smart": smartPlaylistFilter, + "q": playlistFilter, + "smart": smartPlaylistFilter, + "starred": booleanFilter, }) r.setSortMappings(map[string]string{ "owner_name": "owner_name", + "starred_at": "starred, starred_at", }) return r } @@ -87,7 +90,9 @@ func (r *playlistRepository) userFilter() Sqlizer { } func (r *playlistRepository) CountAll(options ...model.QueryOptions) (int64, error) { - sq := Select().Where(r.userFilter()) + sq := r.newSelect() + sq = r.withAnnotation(sq, r.tableName+".id") + sq = sq.Where(r.userFilter()) return r.count(sq, options...) } @@ -198,8 +203,9 @@ func (r *playlistRepository) GetAll(options ...model.QueryOptions) (model.Playli } func (r *playlistRepository) selectPlaylist(options ...model.QueryOptions) SelectBuilder { - return r.newSelect(options...).Join("user on user.id = owner_id"). + query := r.newSelect(options...).Join("user on user.id = owner_id"). Columns(r.tableName+".*", "user.user_name as owner_name") + return r.withAnnotation(query, r.tableName+".id") } func (r *playlistRepository) refreshSmartPlaylist(pls *model.Playlist) bool { diff --git a/persistence/playlist_repository_test.go b/persistence/playlist_repository_test.go index 5a82964c9..9bfa52e3c 100644 --- a/persistence/playlist_repository_test.go +++ b/persistence/playlist_repository_test.go @@ -4,6 +4,7 @@ import ( "context" "time" + sq "github.com/Masterminds/squirrel" "github.com/navidrome/navidrome/conf" "github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/model" @@ -110,6 +111,45 @@ var _ = Describe("PlaylistRepository", func() { Expect(all[0].ID).To(Equal(plsBest.ID)) Expect(all[1].ID).To(Equal(plsCool.ID)) }) + + It("filters starred playlists", func() { + Expect(repo.SetStar(true, plsBest.ID)).To(Succeed()) + + all, err := repo.GetAll(model.QueryOptions{Filters: sq.Eq{"starred": true}}) + Expect(err).ToNot(HaveOccurred()) + Expect(all).To(HaveLen(1)) + Expect(all[0].ID).To(Equal(plsBest.ID)) + + Expect(repo.SetStar(false, plsBest.ID)).To(Succeed()) + }) + + It("counts starred playlists", func() { + Expect(repo.SetStar(true, plsCool.ID)).To(Succeed()) + count, err := repo.CountAll(model.QueryOptions{Filters: sq.Eq{"starred": true}}) + Expect(err).ToNot(HaveOccurred()) + Expect(count).To(Equal(int64(1))) + + Expect(repo.SetStar(false, plsCool.ID)).To(Succeed()) + }) + }) + + Describe("SetStar", func() { + It("should star a playlist", func() { + Expect(repo.SetStar(true, plsBest.ID)).To(Succeed()) + + updated, err := repo.Get(plsBest.ID) + Expect(err).ToNot(HaveOccurred()) + Expect(updated.Starred).To(BeTrue()) + Expect(updated.StarredAt).ToNot(BeNil()) + }) + + It("should unstar a playlist", func() { + Expect(repo.SetStar(false, plsBest.ID)).To(Succeed()) + + updated, err := repo.Get(plsBest.ID) + Expect(err).ToNot(HaveOccurred()) + Expect(updated.Starred).To(BeFalse()) + }) }) Context("Smart Playlists", func() { diff --git a/server/subsonic/media_annotation.go b/server/subsonic/media_annotation.go index 74000856f..18589814d 100644 --- a/server/subsonic/media_annotation.go +++ b/server/subsonic/media_annotation.go @@ -138,6 +138,18 @@ func (api *Router) setStar(ctx context.Context, star bool, ids ...string) error event = event.With("artist", id) continue } + exist, err = tx.Playlist(ctx).Exists(id) + if err != nil { + return err + } + if exist { + err = tx.Playlist(ctx).SetStar(star, id) + if err != nil { + return err + } + event = event.With("playlist", id) + continue + } err = tx.MediaFile(ctx).SetStar(star, id) if err != nil { return err diff --git a/ui/src/layout/PlaylistsSubMenu.jsx b/ui/src/layout/PlaylistsSubMenu.jsx index a9f70b875..56e25d190 100644 --- a/ui/src/layout/PlaylistsSubMenu.jsx +++ b/ui/src/layout/PlaylistsSubMenu.jsx @@ -60,6 +60,7 @@ const PlaylistsSubMenu = ({ state, setState, sidebarIsOpen, dense }) => { perPage: config.maxSidebarPlaylists, }, sort: { field: 'name' }, + filter: config.enableFavourites ? { starred: true } : {}, }, }) diff --git a/ui/src/playlist/PlaylistDetails.jsx b/ui/src/playlist/PlaylistDetails.jsx index acccb15f7..1e2cd17b5 100644 --- a/ui/src/playlist/PlaylistDetails.jsx +++ b/ui/src/playlist/PlaylistDetails.jsx @@ -10,7 +10,13 @@ import { useTranslate } from 'react-admin' import { useCallback, useState, useEffect } from 'react' import Lightbox from 'react-image-lightbox' import 'react-image-lightbox/style.css' -import { CollapsibleComment, DurationField, SizeField } from '../common' +import { + CollapsibleComment, + DurationField, + SizeField, + LoveButton, +} from '../common' +import config from '../config' import subsonic from '../subsonic' const useStyles = makeStyles( @@ -68,6 +74,10 @@ const useStyles = makeStyles( coverLoading: { opacity: 0.5, }, + loveButton: { + top: theme.spacing(-0.2), + left: theme.spacing(0.5), + }, title: { overflow: 'hidden', textOverflow: 'ellipsis', @@ -146,6 +156,16 @@ const PlaylistDetails = (props) => { className={classes.title} > {record.name || translate('ra.page.loading')} + {config.enableFavourites && ( + + )} {record.songCount ? ( From 5a1e9f96f75efcaffbdb43d4363608f170429758 Mon Sep 17 00:00:00 2001 From: Deluan Date: Thu, 29 May 2025 20:47:00 -0400 Subject: [PATCH 2/3] feat(playlists): implement event refresh Signed-off-by: Deluan --- server/subsonic/media_annotation_test.go | 38 ++++++++++++++++++++++++ tests/mock_playlist_repo.go | 20 +++++++++++-- ui/src/layout/PlaylistsSubMenu.jsx | 3 +- 3 files changed, 58 insertions(+), 3 deletions(-) diff --git a/server/subsonic/media_annotation_test.go b/server/subsonic/media_annotation_test.go index 1611250d9..84642c8c9 100644 --- a/server/subsonic/media_annotation_test.go +++ b/server/subsonic/media_annotation_test.go @@ -30,6 +30,44 @@ var _ = Describe("MediaAnnotationController", func() { router = New(ds, nil, nil, nil, nil, nil, nil, eventBroker, nil, playTracker, nil, nil) }) + Describe("Star", func() { + It("should send refresh resource event when starring a playlist", func() { + mockPlaylistRepo := &tests.MockPlaylistRepo{ + Entity: &model.Playlist{ID: "pls-1", Name: "Test Playlist"}, + } + ds.(*tests.MockDataStore).MockedPlaylist = mockPlaylistRepo + + r := newGetRequest("id=pls-1") + _, err := router.Star(r) + + Expect(err).ToNot(HaveOccurred()) + Expect(eventBroker.Events).To(HaveLen(1)) + + event := eventBroker.Events[0].(*events.RefreshResource) + data := event.Data(event) + Expect(data).To(ContainSubstring("playlist")) + Expect(data).To(ContainSubstring("pls-1")) + }) + + It("should send refresh resource event when unstarring a playlist", func() { + mockPlaylistRepo := &tests.MockPlaylistRepo{ + Entity: &model.Playlist{ID: "pls-1", Name: "Test Playlist"}, + } + ds.(*tests.MockDataStore).MockedPlaylist = mockPlaylistRepo + + r := newGetRequest("id=pls-1") + _, err := router.Unstar(r) + + Expect(err).ToNot(HaveOccurred()) + Expect(eventBroker.Events).To(HaveLen(1)) + + event := eventBroker.Events[0].(*events.RefreshResource) + data := event.Data(event) + Expect(data).To(ContainSubstring("playlist")) + Expect(data).To(ContainSubstring("pls-1")) + }) + }) + Describe("Scrobble", func() { It("submit all scrobbles with only the id", func() { submissionTime := time.Now() diff --git a/tests/mock_playlist_repo.go b/tests/mock_playlist_repo.go index 60dc98be9..38f7245a3 100644 --- a/tests/mock_playlist_repo.go +++ b/tests/mock_playlist_repo.go @@ -8,8 +8,9 @@ import ( type MockPlaylistRepo struct { model.PlaylistRepository - Entity *model.Playlist - Error error + Entity *model.Playlist + Error error + SetStarWasCalled bool } func (m *MockPlaylistRepo) Get(_ string) (*model.Playlist, error) { @@ -31,3 +32,18 @@ func (m *MockPlaylistRepo) Count(_ ...rest.QueryOptions) (int64, error) { } return 1, nil } + +func (m *MockPlaylistRepo) Exists(_ string) (bool, error) { + if m.Error != nil { + return false, m.Error + } + return m.Entity != nil, nil +} + +func (m *MockPlaylistRepo) SetStar(starred bool, itemIDs ...string) error { + if m.Error != nil { + return m.Error + } + m.SetStarWasCalled = true + return nil +} diff --git a/ui/src/layout/PlaylistsSubMenu.jsx b/ui/src/layout/PlaylistsSubMenu.jsx index 56e25d190..1d4996909 100644 --- a/ui/src/layout/PlaylistsSubMenu.jsx +++ b/ui/src/layout/PlaylistsSubMenu.jsx @@ -12,7 +12,7 @@ import QueueMusicOutlinedIcon from '@material-ui/icons/QueueMusicOutlined' import { BiCog } from 'react-icons/bi' import { useDrop } from 'react-dnd' import SubMenu from './SubMenu' -import { canChangeTracks } from '../common' +import { canChangeTracks, useResourceRefresh } from '../common' import { DraggableTypes } from '../consts' import config from '../config' @@ -51,6 +51,7 @@ const PlaylistMenuItemLink = ({ pls, sidebarIsOpen }) => { const PlaylistsSubMenu = ({ state, setState, sidebarIsOpen, dense }) => { const history = useHistory() + useResourceRefresh('playlist') const { data, loaded } = useQueryWithStore({ type: 'getList', resource: 'playlist', From f4d06fa82043f45ceaa89c7e8e94ca30cab378c7 Mon Sep 17 00:00:00 2001 From: Deluan Date: Thu, 29 May 2025 21:36:01 -0400 Subject: [PATCH 3/3] fix event broadcasting Signed-off-by: Deluan --- persistence/playlist_repository.go | 5 +++-- server/subsonic/media_annotation.go | 4 +++- server/subsonic/media_annotation_test.go | 6 ++---- ui/src/layout/PlaylistsSubMenu.jsx | 2 +- 4 files changed, 9 insertions(+), 8 deletions(-) diff --git a/persistence/playlist_repository.go b/persistence/playlist_repository.go index d7d516cf9..ac3b60bb2 100644 --- a/persistence/playlist_repository.go +++ b/persistence/playlist_repository.go @@ -53,6 +53,7 @@ func NewPlaylistRepository(ctx context.Context, db dbx.Builder) model.PlaylistRe r.db = db r.tableName = "playlist" r.registerModel(&model.Playlist{}, map[string]filterFunc{ + "id": idFilter(r.tableName), "q": playlistFilter, "smart": smartPlaylistFilter, "starred": booleanFilter, @@ -97,7 +98,7 @@ func (r *playlistRepository) CountAll(options ...model.QueryOptions) (int64, err } func (r *playlistRepository) Exists(id string) (bool, error) { - return r.exists(And{Eq{"id": id}, r.userFilter()}) + return r.exists(And{Eq{"playlist.id": id}, r.userFilter()}) } func (r *playlistRepository) Delete(id string) error { @@ -111,7 +112,7 @@ func (r *playlistRepository) Delete(id string) error { return rest.ErrPermissionDenied } } - return r.delete(And{Eq{"id": id}, r.userFilter()}) + return r.delete(And{Eq{"playlist.id": id}, r.userFilter()}) } func (r *playlistRepository) Put(p *model.Playlist) error { diff --git a/server/subsonic/media_annotation.go b/server/subsonic/media_annotation.go index 18589814d..381db9675 100644 --- a/server/subsonic/media_annotation.go +++ b/server/subsonic/media_annotation.go @@ -147,7 +147,9 @@ func (api *Router) setStar(ctx context.Context, star bool, ids ...string) error if err != nil { return err } - event = event.With("playlist", id) + event = event.With("playlist", "*") + // Ensure the refresh event is sent to all clients, including the originator + ctx = events.BroadcastToAll(ctx) continue } err = tx.MediaFile(ctx).SetStar(star, id) diff --git a/server/subsonic/media_annotation_test.go b/server/subsonic/media_annotation_test.go index 84642c8c9..4b5077a35 100644 --- a/server/subsonic/media_annotation_test.go +++ b/server/subsonic/media_annotation_test.go @@ -45,8 +45,7 @@ var _ = Describe("MediaAnnotationController", func() { event := eventBroker.Events[0].(*events.RefreshResource) data := event.Data(event) - Expect(data).To(ContainSubstring("playlist")) - Expect(data).To(ContainSubstring("pls-1")) + Expect(data).To(ContainSubstring(`"playlist":["*"]`)) }) It("should send refresh resource event when unstarring a playlist", func() { @@ -63,8 +62,7 @@ var _ = Describe("MediaAnnotationController", func() { event := eventBroker.Events[0].(*events.RefreshResource) data := event.Data(event) - Expect(data).To(ContainSubstring("playlist")) - Expect(data).To(ContainSubstring("pls-1")) + Expect(data).To(ContainSubstring(`"playlist":["*"]`)) }) }) diff --git a/ui/src/layout/PlaylistsSubMenu.jsx b/ui/src/layout/PlaylistsSubMenu.jsx index 1d4996909..f0cb39588 100644 --- a/ui/src/layout/PlaylistsSubMenu.jsx +++ b/ui/src/layout/PlaylistsSubMenu.jsx @@ -51,7 +51,7 @@ const PlaylistMenuItemLink = ({ pls, sidebarIsOpen }) => { const PlaylistsSubMenu = ({ state, setState, sidebarIsOpen, dense }) => { const history = useHistory() - useResourceRefresh('playlist') + useResourceRefresh() const { data, loaded } = useQueryWithStore({ type: 'getList', resource: 'playlist',