diff --git a/.github/workflows/pipeline.yml b/.github/workflows/pipeline.yml index 4ac1b2c6b..d2375a6e6 100644 --- a/.github/workflows/pipeline.yml +++ b/.github/workflows/pipeline.yml @@ -14,7 +14,7 @@ concurrency: cancel-in-progress: true env: - CROSS_TAGLIB_VERSION: "2.0.2-1" + CROSS_TAGLIB_VERSION: "2.1.0-1" IS_RELEASE: ${{ startsWith(github.ref, 'refs/tags/') && 'true' || 'false' }} jobs: diff --git a/Dockerfile b/Dockerfile index 4b4c3d18c..2606d2153 100644 --- a/Dockerfile +++ b/Dockerfile @@ -2,7 +2,7 @@ FROM --platform=$BUILDPLATFORM ghcr.io/crazy-max/osxcross:14.5-debian AS osxcros ######################################################################################################################## ### Build xx (orignal image: tonistiigi/xx) -FROM --platform=$BUILDPLATFORM public.ecr.aws/docker/library/alpine:3.21 AS xx-build +FROM --platform=$BUILDPLATFORM public.ecr.aws/docker/library/alpine:3.19 AS xx-build # v1.5.0 ENV XX_VERSION=b4e4c451c778822e6742bfc9d9a91d7c7d885c8a @@ -26,9 +26,9 @@ COPY --from=xx-build /out/ /usr/bin/ ######################################################################################################################## ### Get TagLib -FROM --platform=$BUILDPLATFORM public.ecr.aws/docker/library/alpine:3.21 AS taglib-build +FROM --platform=$BUILDPLATFORM public.ecr.aws/docker/library/alpine:3.19 AS taglib-build ARG TARGETPLATFORM -ARG CROSS_TAGLIB_VERSION=2.0.2-1 +ARG CROSS_TAGLIB_VERSION=2.1.0-1 ENV CROSS_TAGLIB_RELEASES_URL=https://github.com/navidrome/cross-taglib/releases/download/v${CROSS_TAGLIB_VERSION}/ RUN < 0 { - return &mfs[0], nil +func (e *provider) loadTracksByMBID(ctx context.Context, songs []agents.Song) (map[string]model.MediaFile, error) { + var mbids []string + for _, s := range songs { + if s.MBID != "" { + mbids = append(mbids, s.MBID) } - return e.findMatchingTrack(ctx, "", artistID, title) } - mfs, err := e.ds.MediaFile(ctx).GetAll(model.QueryOptions{ + matches := map[string]model.MediaFile{} + if len(mbids) == 0 { + return matches, nil + } + res, err := e.ds.MediaFile(ctx).GetAll(model.QueryOptions{ + Filters: squirrel.And{ + squirrel.Eq{"mbz_recording_id": mbids}, + squirrel.Eq{"missing": false}, + }, + }) + if err != nil { + return matches, err + } + for _, mf := range res { + if id := mf.MbzRecordingID; id != "" { + if _, ok := matches[id]; !ok { + matches[id] = mf + } + } + } + return matches, nil +} + +func (e *provider) loadTracksByTitle(ctx context.Context, songs []agents.Song, artist *auxArtist, mbidMatches map[string]model.MediaFile) (map[string]model.MediaFile, error) { + titleMap := map[string]string{} + for _, s := range songs { + if s.MBID != "" && mbidMatches[s.MBID].ID != "" { + continue + } + sanitized := str.SanitizeFieldForSorting(s.Name) + titleMap[sanitized] = s.Name + } + matches := map[string]model.MediaFile{} + if len(titleMap) == 0 { + return matches, nil + } + titleFilters := squirrel.Or{} + for sanitized := range titleMap { + titleFilters = append(titleFilters, squirrel.Like{"order_title": sanitized}) + } + + res, err := e.ds.MediaFile(ctx).GetAll(model.QueryOptions{ Filters: squirrel.And{ squirrel.Or{ - squirrel.Eq{"artist_id": artistID}, - squirrel.Eq{"album_artist_id": artistID}, + squirrel.Eq{"artist_id": artist.ID}, + squirrel.Eq{"album_artist_id": artist.ID}, }, - squirrel.Like{"order_title": str.SanitizeFieldForSorting(title)}, + titleFilters, squirrel.Eq{"missing": false}, }, Sort: "starred desc, rating desc, year asc, compilation asc ", - Max: 1, }) - if err != nil || len(mfs) == 0 { - return nil, model.ErrNotFound + if err != nil { + return matches, err } - return &mfs[0], nil + for _, mf := range res { + sanitized := str.SanitizeFieldForSorting(mf.Title) + if _, ok := matches[sanitized]; !ok { + matches[sanitized] = mf + } + } + return matches, nil +} + +func (e *provider) selectTopSongs(songs []agents.Song, byMBID, byTitle map[string]model.MediaFile, count int) model.MediaFiles { + var mfs model.MediaFiles + for _, t := range songs { + if len(mfs) == count { + break + } + if t.MBID != "" { + if mf, ok := byMBID[t.MBID]; ok { + mfs = append(mfs, mf) + continue + } + } + if mf, ok := byTitle[str.SanitizeFieldForSorting(t.Name)]; ok { + mfs = append(mfs, mf) + } + } + return mfs } func (e *provider) callGetURL(ctx context.Context, agent agents.ArtistURLRetriever, artist *auxArtist) { diff --git a/core/external/provider_similarsongs_test.go b/core/external/provider_similarsongs_test.go index fd622746a..e7b3cee1f 100644 --- a/core/external/provider_similarsongs_test.go +++ b/core/external/provider_similarsongs_test.go @@ -50,9 +50,9 @@ var _ = Describe("Provider - SimilarSongs", func() { It("returns similar songs from main artist and similar artists", func() { artist1 := model.Artist{ID: "artist-1", Name: "Artist One"} similarArtist := model.Artist{ID: "artist-3", Name: "Similar Artist"} - song1 := model.MediaFile{ID: "song-1", Title: "Song One", ArtistID: "artist-1"} - song2 := model.MediaFile{ID: "song-2", Title: "Song Two", ArtistID: "artist-1"} - song3 := model.MediaFile{ID: "song-3", Title: "Song Three", ArtistID: "artist-3"} + song1 := model.MediaFile{ID: "song-1", Title: "Song One", ArtistID: "artist-1", MbzRecordingID: "mbid-1"} + song2 := model.MediaFile{ID: "song-2", Title: "Song Two", ArtistID: "artist-1", MbzRecordingID: "mbid-2"} + song3 := model.MediaFile{ID: "song-3", Title: "Song Three", ArtistID: "artist-3", MbzRecordingID: "mbid-3"} artistRepo.On("Get", "artist-1").Return(&artist1, nil).Maybe() artistRepo.On("Get", "artist-3").Return(&similarArtist, nil).Maybe() @@ -82,9 +82,8 @@ var _ = Describe("Provider - SimilarSongs", func() { {Name: "Song Three", MBID: "mbid-3"}, }, nil).Once() - mediaFileRepo.FindByMBID("mbid-1", song1) - mediaFileRepo.FindByMBID("mbid-2", song2) - mediaFileRepo.FindByMBID("mbid-3", song3) + mediaFileRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.MediaFiles{song1, song2}, nil).Once() + mediaFileRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.MediaFiles{song3}, nil).Once() songs, err := provider.SimilarSongs(ctx, "artist-1", 3) @@ -111,7 +110,7 @@ var _ = Describe("Provider - SimilarSongs", func() { It("returns songs from main artist when GetSimilarArtists returns error", func() { artist1 := model.Artist{ID: "artist-1", Name: "Artist One"} - song1 := model.MediaFile{ID: "song-1", Title: "Song One", ArtistID: "artist-1"} + song1 := model.MediaFile{ID: "song-1", Title: "Song One", ArtistID: "artist-1", MbzRecordingID: "mbid-1"} artistRepo.On("Get", "artist-1").Return(&artist1, nil).Maybe() artistRepo.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool { @@ -130,7 +129,7 @@ var _ = Describe("Provider - SimilarSongs", func() { {Name: "Song One", MBID: "mbid-1"}, }, nil).Once() - mediaFileRepo.FindByMBID("mbid-1", song1) + mediaFileRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.MediaFiles{song1}, nil).Once() songs, err := provider.SimilarSongs(ctx, "artist-1", 5) @@ -165,8 +164,8 @@ var _ = Describe("Provider - SimilarSongs", func() { It("respects count parameter", func() { artist1 := model.Artist{ID: "artist-1", Name: "Artist One"} - song1 := model.MediaFile{ID: "song-1", Title: "Song One", ArtistID: "artist-1"} - song2 := model.MediaFile{ID: "song-2", Title: "Song Two", ArtistID: "artist-1"} + song1 := model.MediaFile{ID: "song-1", Title: "Song One", ArtistID: "artist-1", MbzRecordingID: "mbid-1"} + song2 := model.MediaFile{ID: "song-2", Title: "Song Two", ArtistID: "artist-1", MbzRecordingID: "mbid-2"} artistRepo.On("Get", "artist-1").Return(&artist1, nil).Maybe() artistRepo.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool { @@ -186,8 +185,7 @@ var _ = Describe("Provider - SimilarSongs", func() { {Name: "Song Two", MBID: "mbid-2"}, }, nil).Once() - mediaFileRepo.FindByMBID("mbid-1", song1) - mediaFileRepo.FindByMBID("mbid-2", song2) + mediaFileRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.MediaFiles{song1, song2}, nil).Once() songs, err := provider.SimilarSongs(ctx, "artist-1", 1) diff --git a/core/external/provider_topsongs_test.go b/core/external/provider_topsongs_test.go index 4ce7911de..443be36dd 100644 --- a/core/external/provider_topsongs_test.go +++ b/core/external/provider_topsongs_test.go @@ -58,11 +58,10 @@ var _ = Describe("Provider - TopSongs", func() { } ag.On("GetArtistTopSongs", ctx, "artist-1", "Artist One", "mbid-artist-1", 2).Return(agentSongs, nil).Once() - // Mock finding matching tracks + // Mock finding matching tracks (both returned in a single query) song1 := model.MediaFile{ID: "song-1", Title: "Song One", ArtistID: "artist-1", MbzRecordingID: "mbid-song-1"} song2 := model.MediaFile{ID: "song-2", Title: "Song Two", ArtistID: "artist-1", MbzRecordingID: "mbid-song-2"} - mediaFileRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.MediaFiles{song1}, nil).Once() - mediaFileRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.MediaFiles{song2}, nil).Once() + mediaFileRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.MediaFiles{song1, song2}, nil).Once() songs, err := p.TopSongs(ctx, "Artist One", 2) @@ -155,11 +154,10 @@ var _ = Describe("Provider - TopSongs", func() { } ag.On("GetArtistTopSongs", ctx, "artist-1", "Artist One", "mbid-artist-1", 2).Return(agentSongs, nil).Once() - // Mock finding matching tracks (only find song 1) + // Mock finding matching tracks (only find song 1 on bulk query) song1 := model.MediaFile{ID: "song-1", Title: "Song One", ArtistID: "artist-1", MbzRecordingID: "mbid-song-1"} - mediaFileRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.MediaFiles{song1}, nil).Once() - mediaFileRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.MediaFiles{}, nil).Once() // For mbid-song-2 (fails) - mediaFileRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.MediaFiles{}, nil).Once() // For title fallback (fails) + mediaFileRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.MediaFiles{song1}, nil).Once() // bulk MBID query + mediaFileRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.MediaFiles{}, nil).Once() // title fallback for song2 songs, err := p.TopSongs(ctx, "Artist One", 2) @@ -190,4 +188,64 @@ var _ = Describe("Provider - TopSongs", func() { artistRepo.AssertExpectations(GinkgoT()) ag.AssertExpectations(GinkgoT()) }) + + It("falls back to title matching when MbzRecordingID is missing", func() { + // Mock finding the artist + artist1 := model.Artist{ID: "artist-1", Name: "Artist One", MbzArtistID: "mbid-artist-1"} + artistRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.Artists{artist1}, nil).Once() + + // Mock agent response with songs that have NO MBID (empty string) + agentSongs := []agents.Song{ + {Name: "Song One", MBID: ""}, // No MBID, should fall back to title matching + {Name: "Song Two", MBID: ""}, // No MBID, should fall back to title matching + } + ag.On("GetArtistTopSongs", ctx, "artist-1", "Artist One", "mbid-artist-1", 2).Return(agentSongs, nil).Once() + + // Since there are no MBIDs, loadTracksByMBID should not make any database call + // loadTracksByTitle should make a database call for title matching + song1 := model.MediaFile{ID: "song-1", Title: "Song One", ArtistID: "artist-1", MbzRecordingID: "", OrderTitle: "song one"} + song2 := model.MediaFile{ID: "song-2", Title: "Song Two", ArtistID: "artist-1", MbzRecordingID: "", OrderTitle: "song two"} + mediaFileRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.MediaFiles{song1, song2}, nil).Once() + + songs, err := p.TopSongs(ctx, "Artist One", 2) + + Expect(err).ToNot(HaveOccurred()) + Expect(songs).To(HaveLen(2)) + Expect(songs[0].ID).To(Equal("song-1")) + Expect(songs[1].ID).To(Equal("song-2")) + artistRepo.AssertExpectations(GinkgoT()) + ag.AssertExpectations(GinkgoT()) + mediaFileRepo.AssertExpectations(GinkgoT()) + }) + + It("combines MBID and title matching when some songs have missing MbzRecordingID", func() { + // Mock finding the artist + artist1 := model.Artist{ID: "artist-1", Name: "Artist One", MbzArtistID: "mbid-artist-1"} + artistRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.Artists{artist1}, nil).Once() + + // Mock agent response with mixed MBID availability + agentSongs := []agents.Song{ + {Name: "Song One", MBID: "mbid-song-1"}, // Has MBID, should match by MBID + {Name: "Song Two", MBID: ""}, // No MBID, should fall back to title matching + } + ag.On("GetArtistTopSongs", ctx, "artist-1", "Artist One", "mbid-artist-1", 2).Return(agentSongs, nil).Once() + + // Mock the MBID query (finds song1 by MBID) + song1 := model.MediaFile{ID: "song-1", Title: "Song One", ArtistID: "artist-1", MbzRecordingID: "mbid-song-1", OrderTitle: "song one"} + mediaFileRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.MediaFiles{song1}, nil).Once() + + // Mock the title fallback query (finds song2 by title) + song2 := model.MediaFile{ID: "song-2", Title: "Song Two", ArtistID: "artist-1", MbzRecordingID: "", OrderTitle: "song two"} + mediaFileRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.MediaFiles{song2}, nil).Once() + + songs, err := p.TopSongs(ctx, "Artist One", 2) + + Expect(err).ToNot(HaveOccurred()) + Expect(songs).To(HaveLen(2)) + Expect(songs[0].ID).To(Equal("song-1")) // Found by MBID + Expect(songs[1].ID).To(Equal("song-2")) // Found by title + artistRepo.AssertExpectations(GinkgoT()) + ag.AssertExpectations(GinkgoT()) + mediaFileRepo.AssertExpectations(GinkgoT()) + }) }) diff --git a/model/metadata/persistent_ids.go b/model/metadata/persistent_ids.go index a71749e81..0a1451cfb 100644 --- a/model/metadata/persistent_ids.go +++ b/model/metadata/persistent_ids.go @@ -24,6 +24,7 @@ type hashFunc = func(...string) string func createGetPID(hash hashFunc) func(mf model.MediaFile, md Metadata, spec string) string { var getPID func(mf model.MediaFile, md Metadata, spec string) string getAttr := func(mf model.MediaFile, md Metadata, attr string) string { + attr = strings.TrimSpace(strings.ToLower(attr)) switch attr { case "albumid": return getPID(mf, md, conf.Server.PID.Album) diff --git a/model/metadata/persistent_ids_test.go b/model/metadata/persistent_ids_test.go index 6903abc05..d07b36331 100644 --- a/model/metadata/persistent_ids_test.go +++ b/model/metadata/persistent_ids_test.go @@ -61,6 +61,7 @@ var _ = Describe("getPID", func() { }) }) }) + Context("calculated attributes", func() { BeforeEach(func() { DeferCleanup(configtest.SetupConfig()) @@ -114,4 +115,36 @@ var _ = Describe("getPID", func() { }) }) }) + + Context("edge cases", func() { + When("the spec has spaces between groups", func() { + It("should return the pid", func() { + spec := "albumartist| Album" + md.tags = map[model.TagName][]string{ + "album": {"album name"}, + } + Expect(getPID(mf, md, spec)).To(Equal("(album name)")) + }) + }) + When("the spec has spaces", func() { + It("should return the pid", func() { + spec := "albumartist, album" + md.tags = map[model.TagName][]string{ + "albumartist": {"Album Artist"}, + "album": {"album name"}, + } + Expect(getPID(mf, md, spec)).To(Equal("(Album Artist\\album name)")) + }) + }) + When("the spec has mixed case fields", func() { + It("should return the pid", func() { + spec := "albumartist,Album" + md.tags = map[model.TagName][]string{ + "albumartist": {"Album Artist"}, + "album": {"album name"}, + } + Expect(getPID(mf, md, spec)).To(Equal("(Album Artist\\album name)")) + }) + }) + }) }) diff --git a/resources/i18n/pt-br.json b/resources/i18n/pt-br.json index cfb3c8485..285a71523 100644 --- a/resources/i18n/pt-br.json +++ b/resources/i18n/pt-br.json @@ -124,6 +124,11 @@ "remixer": "Remixador |||| Remixadores", "djmixer": "DJ Mixer |||| DJ Mixers", "performer": "Músico |||| Músicos" + }, + "actions": { + "topSongs": "Mais tocadas", + "shuffle": "Aleatório", + "radio": "Rádio" } }, "user": { @@ -407,6 +412,8 @@ "transcodingDisabled": "Por questão de segurança, esta tela de configuração está desabilitada. Se você quiser alterar estas configurações, reinicie o servidor com a opção %{config}", "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)?", diff --git a/ui/src/artist/ArtistActions.jsx b/ui/src/artist/ArtistActions.jsx new file mode 100644 index 000000000..c33ee892b --- /dev/null +++ b/ui/src/artist/ArtistActions.jsx @@ -0,0 +1,128 @@ +import React from 'react' +import PropTypes from 'prop-types' +import { useDispatch } from 'react-redux' +import { useMediaQuery } from '@material-ui/core' +import { makeStyles } from '@material-ui/core/styles' +import { + Button, + TopToolbar, + sanitizeListRestProps, + useDataProvider, + useNotify, + 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 { playShuffle, playSimilar, playTopSongs } from './actions.js' + +const useStyles = makeStyles((theme) => ({ + toolbar: { + minHeight: 'auto', + padding: '0 !important', + background: 'transparent', + boxShadow: 'none', + '& .MuiToolbar-root': { + minHeight: 'auto', + padding: '0 !important', + background: 'transparent', + }, + }, + button: { + [theme.breakpoints.down('xs')]: { + minWidth: 'auto', + padding: '8px 12px', + fontSize: '0.75rem', + '& .MuiButton-startIcon': { + marginRight: '4px', + }, + }, + }, + radioIcon: { + [theme.breakpoints.down('xs')]: { + fontSize: '1.5rem', + }, + }, +})) + +const ArtistActions = ({ className, record, ...rest }) => { + const dispatch = useDispatch() + const translate = useTranslate() + const dataProvider = useDataProvider() + const notify = useNotify() + 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 { + await playShuffle(dataProvider, dispatch, record.id) + } catch (e) { + // eslint-disable-next-line no-console + console.error('Error fetching songs for shuffle:', e) + notify('ra.page.error', 'warning') + } + }, [dataProvider, dispatch, record, notify]) + + const handleRadio = React.useCallback(async () => { + try { + await playSimilar(dispatch, notify, record.id) + } catch (e) { + // eslint-disable-next-line no-console + console.error('Error starting radio for artist:', e) + notify('ra.page.error', 'warning') + } + }, [dispatch, notify, record]) + + return ( + + + + + + ) +} + +ArtistActions.propTypes = { + className: PropTypes.string, + record: PropTypes.object.isRequired, +} + +ArtistActions.defaultProps = { + className: '', +} + +export default ArtistActions diff --git a/ui/src/artist/ArtistActions.test.jsx b/ui/src/artist/ArtistActions.test.jsx new file mode 100644 index 000000000..90be28409 --- /dev/null +++ b/ui/src/artist/ArtistActions.test.jsx @@ -0,0 +1,188 @@ +import React from 'react' +import { render, fireEvent, waitFor, screen } from '@testing-library/react' +import { TestContext } from 'ra-test' +import { describe, it, expect, vi, beforeEach } from 'vitest' +import ArtistActions from './ArtistActions' +import subsonic from '../subsonic' +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(), getTopSongs: vi.fn() }, +})) + +const mockNotify = vi.fn() +const mockGetList = vi.fn().mockResolvedValue({ data: [{ id: 's1' }] }) + +vi.mock('react-admin', async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + useNotify: () => mockNotify, + useDataProvider: () => ({ getList: mockGetList }), + useTranslate: () => (x) => x, + } +}) + +describe('ArtistActions', () => { + const defaultRecord = { id: 'ar1', name: 'Artist' } + + const renderArtistActions = (record = defaultRecord) => { + const theme = createTheme() + return render( + + + + + , + ) + } + + 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': { + status: 'ok', + similarSongs2: { song: [{ id: 'rec1' }] }, + }, + }, + }) + subsonic.getTopSongs.mockResolvedValue({ + json: { + 'subsonic-response': { + status: 'ok', + topSongs: { song: [{ id: 'rec1' }] }, + }, + }, + }) + }) + + describe('Shuffle action', () => { + it('shuffles songs when clicked', async () => { + renderArtistActions() + clickActionButton('shuffle') + + await waitFor(() => + expect(mockGetList).toHaveBeenCalledWith('song', { + pagination: { page: 1, perPage: 500 }, + sort: { field: 'random', order: 'ASC' }, + filter: { album_artist_id: 'ar1', missing: false }, + }), + ) + expect(mockDispatch).toHaveBeenCalled() + }) + }) + + describe('Radio action', () => { + it('starts radio when clicked', async () => { + renderArtistActions() + clickActionButton('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() + }) + }) +}) diff --git a/ui/src/artist/ArtistShow.jsx b/ui/src/artist/ArtistShow.jsx index e8e03f52e..db8ed4566 100644 --- a/ui/src/artist/ArtistShow.jsx +++ b/ui/src/artist/ArtistShow.jsx @@ -14,6 +14,39 @@ import AlbumGridView from '../album/AlbumGridView' import MobileArtistDetails from './MobileArtistDetails' import DesktopArtistDetails from './DesktopArtistDetails' import { useAlbumsPerPage, useResourceRefresh, Title } from '../common/index.js' +import ArtistActions from './ArtistActions' +import { makeStyles } from '@material-ui/core' + +const useStyles = makeStyles( + (theme) => ({ + actions: { + width: '100%', + justifyContent: 'flex-start', + display: 'flex', + paddingTop: '0.25em', + paddingBottom: '0.25em', + paddingLeft: '1em', + paddingRight: '1em', + flexWrap: 'wrap', + overflowX: 'auto', + [theme.breakpoints.down('xs')]: { + paddingLeft: '0.5em', + paddingRight: '0.5em', + gap: '0.5em', + justifyContent: 'space-around', + }, + }, + actionsContainer: { + paddingLeft: '.75rem', + [theme.breakpoints.down('xs')]: { + padding: '.5rem', + }, + }, + }), + { + name: 'NDArtistShow', + }, +) const ArtistDetails = (props) => { const record = useRecordContext(props) @@ -56,6 +89,7 @@ const ArtistShowLayout = (props) => { const record = useRecordContext() const { width } = props const [, perPageOptions] = useAlbumsPerPage(width) + const classes = useStyles() useResourceRefresh('artist', 'album') const maxPerPage = 90 @@ -79,6 +113,11 @@ const ArtistShowLayout = (props) => { <> {record && } />} {record && } + {record && ( +
+ +
+ )} {record && ( { + 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)) +} diff --git a/ui/src/audioplayer/AudioTitle.test.jsx b/ui/src/audioplayer/AudioTitle.test.jsx index c3f566f6b..7b297c07e 100644 --- a/ui/src/audioplayer/AudioTitle.test.jsx +++ b/ui/src/audioplayer/AudioTitle.test.jsx @@ -12,11 +12,12 @@ vi.mock('@material-ui/core', async () => { }) vi.mock('react-router-dom', () => ({ - Link: ({ to, children, ...props }) => ( - + // eslint-disable-next-line react/display-name + Link: React.forwardRef(({ to, children, ...props }, ref) => ( + {children} - ), + )), })) vi.mock('react-dnd', () => ({ diff --git a/ui/src/dialogs/SaveQueueDialog.jsx b/ui/src/dialogs/SaveQueueDialog.jsx index 69f07dab7..f916a0793 100644 --- a/ui/src/dialogs/SaveQueueDialog.jsx +++ b/ui/src/dialogs/SaveQueueDialog.jsx @@ -57,7 +57,10 @@ export const SaveQueueDialog = () => { return res }) .then((res) => { - notify('ra.notification.created', 'info', { smart_count: 1 }) + notify('ra.notification.created', { + type: 'info', + messageArgs: { smart_count: 1 }, + }) dispatch(closeSaveQueueDialog()) refresh() history.push(`/playlist/${res.data.id}/show`) diff --git a/ui/src/i18n/en.json b/ui/src/i18n/en.json index 52363a350..b3f94ab42 100644 --- a/ui/src/i18n/en.json +++ b/ui/src/i18n/en.json @@ -125,6 +125,11 @@ "remixer": "Remixer |||| Remixers", "djmixer": "DJ Mixer |||| DJ Mixers", "performer": "Performer |||| Performers" + }, + "actions": { + "topSongs": "Top Songs", + "shuffle": "Shuffle", + "radio": "Radio" } }, "user": { @@ -410,6 +415,8 @@ "transcodingDisabled": "Changing the transcoding configuration through the web interface is disabled for security reasons. If you would like to change (edit or add) transcoding options, restart the server with the %{config} configuration option.", "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)?", diff --git a/ui/src/subsonic/index.js b/ui/src/subsonic/index.js index 857e33f3c..f42ca24e3 100644 --- a/ui/src/subsonic/index.js +++ b/ui/src/subsonic/index.js @@ -82,6 +82,14 @@ const getAlbumInfo = (id) => { return httpClient(url('getAlbumInfo', id)) } +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, { @@ -106,4 +114,6 @@ export default { streamUrl, getAlbumInfo, getArtistInfo, + getTopSongs, + getSimilarSongs2, } diff --git a/ui/src/themes/nord.js b/ui/src/themes/nord.js index 8c346eefe..5420bbc60 100644 --- a/ui/src/themes/nord.js +++ b/ui/src/themes/nord.js @@ -259,7 +259,6 @@ export default { }, details: { fontSize: '.875rem', - minWidth: '75vw', color: 'rgba(255,255,255, 0.8)', }, }, diff --git a/ui/src/themes/spotify.js b/ui/src/themes/spotify.js index 703d8159e..c40ed20aa 100644 --- a/ui/src/themes/spotify.js +++ b/ui/src/themes/spotify.js @@ -204,7 +204,6 @@ export default { }, details: { fontSize: '.875rem', - minWidth: '75vw', color: 'rgba(255,255,255, 0.8)', }, }, @@ -243,6 +242,64 @@ export default { NDPlaylistShow: { playlistActions: musicListActions, }, + NDArtistShow: { + actions: { + padding: '2rem 0', + alignItems: 'center', + overflow: 'visible', + minHeight: '120px', + '@global': { + button: { + border: '1px solid transparent', + backgroundColor: 'inherit', + color: '#b3b3b3', + margin: '0 0.5rem', + '&:hover': { + border: '1px solid #b3b3b3', + backgroundColor: 'inherit !important', + }, + }, + // Hide shuffle button label (first button) + 'button:first-child>span:first-child>span': { + display: 'none', + }, + // Style shuffle button (first button) + 'button:first-child': { + '@media screen and (max-width: 720px)': { + transform: 'scale(1.5)', + margin: '1rem', + '&:hover': { + transform: 'scale(1.6) !important', + }, + }, + transform: 'scale(2)', + margin: '1.5rem', + minWidth: 0, + padding: 5, + transition: 'transform .3s ease', + background: spotifyGreen['500'], + color: '#fff', + borderRadius: 500, + border: 0, + '&:hover': { + transform: 'scale(2.1)', + backgroundColor: `${spotifyGreen['500']} !important`, + border: 0, + }, + }, + 'button:first-child>span:first-child': { + padding: 0, + }, + 'button>span:first-child>span, button:not(:first-child)>span:first-child>svg': + { + color: '#b3b3b3', + }, + }, + }, + actionsContainer: { + overflow: 'visible', + }, + }, NDAudioPlayer: { audioTitle: { color: '#fff', diff --git a/ui/src/themes/theme.test.js b/ui/src/themes/theme.test.js new file mode 100644 index 000000000..b65c3a5fe --- /dev/null +++ b/ui/src/themes/theme.test.js @@ -0,0 +1,14 @@ +import themes from './index' +import { describe, it, expect } from 'vitest' + +describe('NDPlaylistDetails styles', () => { + const themeEntries = Object.entries(themes) + + it.each(themeEntries)( + '%s should not set minWidth on details', + (themeName, theme) => { + const details = theme.overrides?.NDPlaylistDetails?.details + expect(details?.minWidth).toBeUndefined() + }, + ) +})