-
-
Notifications
You must be signed in to change notification settings - Fork 586
feat: decouple Plex as a requirement for setting up/using Overseerr #3015
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?
Conversation
0f30ece to
a231eba
Compare
|
This pull request introduces 1 alert when merging a231eba into 30141f7 - view on LGTM.com new alerts:
|
overseerr
|
||||||||||||||||||||||||||||
| Project |
overseerr
|
| Branch Review |
refs/pull/3015/merge
|
| Run status |
|
| Run duration | 03m 22s |
| Commit |
|
| Committer | Ryan Cohen |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
|
|
0
|
|
|
1
|
|
|
0
|
|
|
0
|
|
|
28
|
| View all changes introduced in this branch ↗︎ | |
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
|
For those who have patiently been waiting for this, it's still in progress! I am trying to sneak in time to get this done this new year's vacation. It's super close, so please wait just a little bit longer! 🙏 |
|
I'm looking forward for this 🎉 |
|
Hey @sct! Just checking in and seeing how this is going. This is the final piece of the puzzle for me. Hope you've had a wonderful New Year's vacation! |
We are still working on it! |
|
Is there a way to help, and most importantly, what is the estimated release date? That's crucial information because a very viable and full featured release already exists with Jellyseerr, so if there isn't a near term road map folks will want to use that instead. Are you working with the Jellyseerr developers to pull in their improvements? If the release date is significantly distant, it'd make sense to focus on a migration path for Jellyseerr, as well as to double down on collaboration with the devs of that project. Please let us know the best way to collaborate to get this feature released. |
We don't provide ETAs. We spare whatever free time we have and that's the best I can tell you in terms of delivery time. I am aware of Jellyseerr. That's the beauty of open source. We also now have an internal roadmap with Jellyseerr devs we are referencing to get Jellyfin support back ported to Overseerr once this is merged. I know people are waiting for this. It's close. Just hang on a bit longer. |
8008312 to
e9545d4
Compare
This comment was marked as resolved.
This comment was marked as resolved.
|
Can't wait for this to be merged. I assume this is purely a structural change to allow for jellyfin/emby but doesn't directly add them? |
|
Has there been any movement here? I have still have #3105 open which was tacitly waiting on this work to shake out, but it's not clear to me if this work has stalled entirely or what. |
b9f5c0c to
9891df2
Compare
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.
Should we keep the play on plex deep link even if plex is disconnected? Same probably goes for the import plex users button.
OwsleyJr
left a comment
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.
We also probably want to disable any Plex related jobs if Plex is disconnected.
TheCatLady
left a comment
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.
Quickly looked through the changes and don't see any obvious issues, so I'm just nitpicking here 🐱
I'm still kinda jetlagged though, so I'll look over everything again and do some testing in the next couple days 😸
It'll be great to have these changes finally merged! 🎉
src/components/UserProfile/UserSettings/UserGeneralSettings/index.tsx
Outdated
Show resolved
Hide resolved
src/components/UserProfile/UserSettings/UserGeneralSettings/index.tsx
Outdated
Show resolved
Hide resolved
|
I've been playing with an instance without Plex configured, and I think we should not have the jobs that are related to Plex registered to be ran. Having logs referring to Plex (or any other media server really), despite not having a Plex server configured isn't a good experience, and also seems easily preventable by not registering them in the first place. That would also give us the added benefit of not having those jobs in the UI neither, while the endpoints related to jobs would not have to be changed. One downside would be having to re-trigger the start of the jobs once a media server is configured in the setup, but I think that should be fine? |
| type="checkbox" | ||
| id="localLogin" | ||
| name="localLogin" | ||
| disabled={!currentSettings.plexLoginEnabled} |
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.
For the "Enable New Plex Sign-In" setting below, I think we should only show/allow changing that setting if a plex server is configured
|
Without a Plex server configured, we should remove the "Import Plex Users" button from the UI on the users list page - especially since the modal only brings up "There are no Plex users to import" while the server doesn't have an admin user token to use to authenticate to the plex API. |
Description
This is the first step to opening up Overseerr to allow other media server integrations. Previously, Overseerr required signing in with a Plex account to get it configured. You can now create a local account instead of using Plex. If a local account is used, you will need to connect your Plex account still to use the Plex Media Server features.
Screenshot (if UI-related)
To-Dos
yarn buildyarn i18n:extract