-
-
Notifications
You must be signed in to change notification settings - Fork 547
feat: music support #2132
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
feat: music support #2132
Conversation
|
Little bits to fix up, but its mostly working again against the new code. I am trying to reduce some of the changes though, I feel like it touches and changes more than it needs to in some places. Any feedback helpful. All credit to @0-Pierre for 99% of the work. |
|
This pull request has merge conflicts. Please resolve the conflicts so the PR can be successfully reviewed and merged. |
|
This is kinda off topic but if you want you can add at end of the commit message Also might be good practice to rebase commit a commit or just clone the branch and update from there. Bellow is a clean way of getting the old branch on your repo: git checkout -b lidarr-bk lidarr #saves your branch
git branch -D lidarr
git remote add 0pierre https://github.com/0-Pierre/jellyseerr.git
git fetch 0pierre
git checkout -b lidarr origin/0pierre |
|
Thank you! I did have a really nice rebase that included all of pierre's commit history, then my lack of git expertise caused me to fumble and nuke it all. I'll try and get it back with that :) |
|
This pull request has merge conflicts. Please resolve the conflicts so the PR can be successfully reviewed and merged. |
|
Eyyy got it back! |
Indeed, if you feel like things can be removed/refactored to reduce the number of changes, please do. |
|
This pull request has merge conflicts. Please resolve the conflicts so the PR can be successfully reviewed and merged. |
|
Hello, Why is there a need to supersede PR #1238 ? In his last message, 0-Pierre confirmed he is still available to work on until we (Seerr) are ready for it. |
|
Hi everyone, To be honest I'm pretty happy someone with an other vision refactor the code, I'm very busy and don't have that much time this last 6 months. So enjoy, let's makes Seerr even better 👍 |
|
Thanks for your quick feedback @0-Pierre I can close your PR in favor of this one so :) |
| await this.axios.delete(`/album/${albumId}`, { | ||
| params: { | ||
| deleteFiles: 'true', | ||
| addImportExclusion: 'false', | ||
| }, | ||
| }); |
Check failure
Code scanning / CodeQL
Server-side request forgery Critical
host
user-provided value
The
host
user-provided value
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be fixed by my pr on HiItsStolas fork, branch lidarr
| const updatedAlbum = await this.axios.put<LidarrAlbum>( | ||
| `/album/${existingAlbums[0].id}`, | ||
| { | ||
| ...existingAlbums[0], | ||
| monitored: true, | ||
| } | ||
| ); |
Check failure
Code scanning / CodeQL
Server-side request forgery Critical
host
user-provided value
The
host
user-provided value
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above
|
Thanks Pierre! Rebasing this today to keep it up to date. I've been using on my own setup for a while now without any troubles - but keen to get it into testing with other people. |
…ts based on Lidarr configuration selection
…lbums in Lidarr upon request
… using Plex as the media server
…ULL constraint failure in metadata_album.caaUrl
…repo develop branch
…h AddMusicSupport
separted the discography and similar-artist from the monolithic artists call
Music was being blacklisted, and the API call to listenbrainz was wrong
Aligned the request notifications with radarr and sonarr, which fixed how it used updateParentStatus
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR implements music support for Seerr by integrating with MusicBrainz, ListenBrainz, Cover Art Archive, and Lidarr. The implementation allows users to discover, request, and manage music albums similarly to movies and TV series.
Key changes include:
- Addition of music-specific components and pages for browsing albums and artists
- Integration with Lidarr for music automation
- Extension of existing components (TitleCard, RequestModal, etc.) to support music media types
- New database entities and API routes for music metadata
- Progressive cover art loading system for albums
Reviewed changes
Copilot reviewed 133 out of 148 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/components/UserProfile/index.tsx | Updated user profile to display music requests and quota information |
| src/components/TitleCard/index.tsx | Extended TitleCard to support album media type with square artwork |
| src/components/Settings/SettingsServices.tsx | Added Lidarr server configuration UI |
| src/components/RequestModal/MusicRequestModal.tsx | New modal component for requesting music albums |
| src/components/MusicDetails/index.tsx | New page component for displaying album details |
| src/components/PersonDetails/index.tsx | Updated to support music artists with discography |
| src/components/Discover/DiscoverMusic/index.tsx | New discovery page for browsing music |
| src/components/Common/CachedImage/index.tsx | Added support for Cover Art Archive and TheAudioDB image caching |
| server/subscriber/MediaRequestSubscriber.ts | Implemented Lidarr integration for processing music requests |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (2)
src/components/RequestModal/QuotaDisplay/index.tsx:1
- The parentheses around
limit ?? 0are unnecessary. The nullish coalescing operator has lower precedence than the comparison operator, so the expression can be simplified to(data?.music.limit ?? 0) > 0.
import ProgressCircle from '@app/components/Common/ProgressCircle';
server/subscriber/MediaRequestSubscriber.ts:1
- Inconsistent use of equality operators. Line 114 uses loose equality (
==) while line 115 uses strict equality (===). Both should use strict equality for consistency and to avoid potential type coercion issues.
import CoverArtArchive from '@server/api/coverartarchive';
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <Alert | ||
| title={intl.formatMessage(messages.noDefaultServer, { | ||
| serverType: 'Lidarr', | ||
| mediaType: intl.formatMessage(messages.mediaTypeSeries), |
Copilot
AI
Dec 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The message key used for Lidarr's 'no default server' alert is incorrect. It references 'mediaTypeSeries' but should reference 'mediaTypeMusic' since this alert is for the Lidarr (music) section, not Sonarr (series).
| mediaType: intl.formatMessage(messages.mediaTypeSeries), | |
| mediaType: intl.formatMessage(messages.mediaTypeMusic), |
| asSingle | ||
| containerClassName="datepicker-wrapper" | ||
| inputClassName="pr-1 sm:pr-4 text-base leading-5" | ||
| displayFormat="YYYY-MM-DD" // Add this to enforce the correct format |
Copilot
AI
Dec 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove the inline comment as it's redundant. The code is self-explanatory and the comment doesn't add value.
| asSingle | ||
| containerClassName="datepicker-wrapper" | ||
| inputClassName="pr-1 sm:pr-4 text-base leading-5" | ||
| displayFormat="YYYY-MM-DD" // Add this to enforce the correct format |
Copilot
AI
Dec 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove the inline comment as it's redundant. The code is self-explanatory and the comment doesn't add value.
|
This pull request has merge conflicts. Please resolve the conflicts so the PR can be successfully reviewed and merged. |
Description
This is 99% https://github.com/0-Pierre 's branch (#1238), all credit to them!
Haven't had any action on the other PR for a while, so this is a re-based + updated PR based on that PR.
Screenshot (if UI-related)
To-Dos
pnpm buildpnpm i18n:extractIssues Fixed or Closed
Please use this discussion #2160 as a place to discuss testing / questions related to the work-in-progress Music support feature. This will allow the PR comments to be used for code review instead of general discussion.