feat(server): add option Lastfm.ScrobbleFirstArtistOnly to send only the first artist
#4131
Conversation
…y the first artist fixes #3791 Signed-off-by: Deluan <deluan@navidrome.org>
There was a problem hiding this comment.
Hello @deluan, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
Summary of Changes
Hello! Gemini here, providing a summary of this pull request. This PR introduces a new configuration option for the Last.fm integration: Lastfm.ScrobbleFirstArtistOnly. The purpose of this option is to give users control over how tracks with multiple artists are scrobbled. When enabled, only the first artist listed for a track will be sent to Last.fm, rather than the potentially concatenated list or primary artist field. This change involves adding the configuration field, setting its default value, modifying the Last.fm agent logic to respect the new setting, and adding corresponding tests. This work directly addresses issue #3791.
Highlights
- New Last.fm Configuration Option: A new boolean configuration option,
Lastfm.ScrobbleFirstArtistOnly, has been added to control whether only the first artist is used when scrobbling tracks with multiple artists. The default value is set tofalse. - Last.fm Agent Logic Update: A new helper function
getArtistForScrobblewas introduced in the Last.fm agent. This function checks the new configuration option and the track's participant list to determine the correct artist name to use for both 'Now Playing' updates and scrobbles. TheNowPlayingandScrobblemethods were updated to utilize this new function. - Testing for New Configuration: The Last.fm agent tests were updated to include tracks with multiple artists in the test setup. A new test case was added specifically to verify that when
ScrobbleFirstArtistOnlyis enabled, the agent correctly sends only the first artist's name during scrobbling.
Changelog
- conf/configuration.go
- Added
ScrobbleFirstArtistOnly boolfield to thelastfmOptionsstruct (line 161). - Set the default value for
lastfm.scrobblefirstartistonlytofalseinsetViperDefaults(line 532).
- Added
- core/agents/lastfm/agent.go
- Added a new function
getArtistForScrobbleto determine the artist based on the new configuration and track participants (lines 282-286). - Updated the
NowPlayingmethod to usel.getArtistForScrobble(track)for the artist name (line 296). - Updated the
Scrobblemethod to usel.getArtistForScrobble(&s.MediaFile)for the artist name (line 322).
- Added a new function
- core/agents/lastfm/agent_test.go
- Updated the test track definition to include a
Participantsmap with multiple artists for theRoleArtist(lines 199-204). - Added a new
Whenblock andIttest case to verify scrobbling behavior whenScrobbleFirstArtistOnlyis true, asserting that only the 'First Artist' is sent (lines 256-270).
- Updated the test track definition to include a
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Scrobble sends the tune,
Artist names beneath the moon,
One or many sent.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request introduces a new configuration option, ScrobbleFirstArtistOnly, for Last.fm integration. The option allows users to specify whether only the first listed artist should be scrobbled for tracks with multiple artists. The changes include updates to configuration handling, the Last.fm agent logic, and associated tests. Overall, this is a solid contribution.
Summary of Findings
- Artist string source for scrobbling: The
getArtistForScrobblemethod correctly implements the new feature. However, whenScrobbleFirstArtistOnlyis false and multiple artists are present intrack.Participants, the system relies ontrack.Artist. It's worth considering if, in this scenario, the artist string should be derived directly fromtrack.Participantsto ensure the most complete information is scrobbled, assumingtrack.Participantsis the canonical source.
Merge Readiness
The pull request is well-implemented and adds a useful feature. The new tests cover the functionality effectively. There is one medium-severity point raised for discussion regarding the source of the artist string when scrobbling multiple artists and the new option is disabled. Addressing or clarifying this point would be beneficial. I am unable to approve pull requests myself, but I recommend discussing this point before merging. If the current behavior of relying on track.Artist in that specific case is confirmed as intended, the PR is otherwise in good shape.
This pull request introduces a new configuration option,
ScrobbleFirstArtistOnly, for Last.fm integration. The option allows users to specify whether only the first listed artist should be scrobbled for tracks with multiple artists. The changes include updates to configuration handling, the Last.fm agent logic, and associated tests.Configuration Updates:
ScrobbleFirstArtistOnly, to thelastfmOptionsstruct inconf/configuration.goto support the new configuration option.ScrobbleFirstArtistOnlytofalsein thesetViperDefaultsfunction.Last.fm Agent Logic:
getArtistForScrobble, incore/agents/lastfm/agent.goto determine the artist to scrobble based on theScrobbleFirstArtistOnlyconfiguration. Updated theNowPlayingandScrobblemethods to use this logic. [1] [2]Tests:
lastfmAgenttest setup incore/agents/lastfm/agent_test.goto include multiple artists in theParticipantsfield of the test track.ScrobbleFirstArtistOnlyconfiguration correctly scrobbles only the first artist when enabled.fixes #3791