From b7c4128b1bdea95a4b8e9f5b5a2764bb88d1013b Mon Sep 17 00:00:00 2001 From: maya doshi Date: Wed, 3 Dec 2025 15:55:25 -0500 Subject: [PATCH] fix(server): Lastfm.ScrobbleFirstArtistOnly also only scrobbles the first artist of the album (#4762) * feat(server): add option Lastfm.ScrobbleFirstAlbumArtistOnly to send only the first album artist * fix: remove config parameter scrobbleFirstAlbumArtist * test: add NowPlaying test for ScrobbleFirstArtistOnly Add a test case for the NowPlaying function when ScrobbleFirstArtistOnly is enabled. This ensures that only the first artist from the Participants list is sent to Last.fm for both artist and album artist fields, matching the existing test coverage for the Scrobble function. * refactor: consolidate getArtistForScrobble and getAlbumArtistForScrobble Merge the separate getArtistForScrobble and getAlbumArtistForScrobble functions into a single parameterized function. This eliminates code duplication and makes the scrobble artist handling logic more maintainable. The function now accepts a role parameter and display name, allowing it to handle both artist and album artist extraction based on the ScrobbleFirstArtistOnly configuration. --------- Co-authored-by: Deluan --- core/agents/lastfm/agent.go | 16 ++++++++-------- core/agents/lastfm/agent_test.go | 22 ++++++++++++++++++++++ 2 files changed, 30 insertions(+), 8 deletions(-) diff --git a/core/agents/lastfm/agent.go b/core/agents/lastfm/agent.go index fafa6afe..e3e53b23 100644 --- a/core/agents/lastfm/agent.go +++ b/core/agents/lastfm/agent.go @@ -290,11 +290,11 @@ func (l *lastfmAgent) callArtistGetTopTracks(ctx context.Context, artistName str return t.Track, nil } -func (l *lastfmAgent) getArtistForScrobble(track *model.MediaFile) string { - if conf.Server.LastFM.ScrobbleFirstArtistOnly && len(track.Participants[model.RoleArtist]) > 0 { - return track.Participants[model.RoleArtist][0].Name +func (l *lastfmAgent) getArtistForScrobble(track *model.MediaFile, role model.Role, displayName string) string { + if conf.Server.LastFM.ScrobbleFirstArtistOnly && len(track.Participants[role]) > 0 { + return track.Participants[role][0].Name } - return track.Artist + return displayName } func (l *lastfmAgent) NowPlaying(ctx context.Context, userId string, track *model.MediaFile, position int) error { @@ -304,13 +304,13 @@ func (l *lastfmAgent) NowPlaying(ctx context.Context, userId string, track *mode } err = l.client.updateNowPlaying(ctx, sk, ScrobbleInfo{ - artist: l.getArtistForScrobble(track), + artist: l.getArtistForScrobble(track, model.RoleArtist, track.Artist), track: track.Title, album: track.Album, trackNumber: track.TrackNumber, mbid: track.MbzRecordingID, duration: int(track.Duration), - albumArtist: track.AlbumArtist, + albumArtist: l.getArtistForScrobble(track, model.RoleAlbumArtist, track.AlbumArtist), }) if err != nil { log.Warn(ctx, "Last.fm client.updateNowPlaying returned error", "track", track.Title, err) @@ -330,13 +330,13 @@ func (l *lastfmAgent) Scrobble(ctx context.Context, userId string, s scrobbler.S return nil } err = l.client.scrobble(ctx, sk, ScrobbleInfo{ - artist: l.getArtistForScrobble(&s.MediaFile), + artist: l.getArtistForScrobble(&s.MediaFile, model.RoleArtist, s.Artist), track: s.Title, album: s.Album, trackNumber: s.TrackNumber, mbid: s.MbzRecordingID, duration: int(s.Duration), - albumArtist: s.AlbumArtist, + albumArtist: l.getArtistForScrobble(&s.MediaFile, model.RoleAlbumArtist, s.AlbumArtist), timestamp: s.TimeStamp, }) if err == nil { diff --git a/core/agents/lastfm/agent_test.go b/core/agents/lastfm/agent_test.go index 18e7facf..fc623840 100644 --- a/core/agents/lastfm/agent_test.go +++ b/core/agents/lastfm/agent_test.go @@ -201,6 +201,10 @@ var _ = Describe("lastfmAgent", func() { {Artist: model.Artist{ID: "ar-1", Name: "First Artist"}}, {Artist: model.Artist{ID: "ar-2", Name: "Second Artist"}}, }, + model.RoleAlbumArtist: []model.Participant{ + {Artist: model.Artist{ID: "ar-1", Name: "First Album Artist"}}, + {Artist: model.Artist{ID: "ar-2", Name: "Second Album Artist"}}, + }, }, } }) @@ -229,6 +233,23 @@ var _ = Describe("lastfmAgent", func() { err := agent.NowPlaying(ctx, "user-2", track, 0) Expect(err).To(MatchError(scrobbler.ErrNotAuthorized)) }) + + When("ScrobbleFirstArtistOnly is true", func() { + BeforeEach(func() { + conf.Server.LastFM.ScrobbleFirstArtistOnly = true + }) + + It("uses only the first artist", func() { + httpClient.Res = http.Response{Body: io.NopCloser(bytes.NewBufferString("{}")), StatusCode: 200} + + err := agent.NowPlaying(ctx, "user-1", track, 0) + + Expect(err).ToNot(HaveOccurred()) + sentParams := httpClient.SavedRequest.URL.Query() + Expect(sentParams.Get("artist")).To(Equal("First Artist")) + Expect(sentParams.Get("albumArtist")).To(Equal("First Album Artist")) + }) + }) }) Describe("scrobble", func() { @@ -267,6 +288,7 @@ var _ = Describe("lastfmAgent", func() { Expect(err).ToNot(HaveOccurred()) sentParams := httpClient.SavedRequest.URL.Query() Expect(sentParams.Get("artist")).To(Equal("First Artist")) + Expect(sentParams.Get("albumArtist")).To(Equal("First Album Artist")) }) })