-
-
Notifications
You must be signed in to change notification settings - Fork 283
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
feat: remove requests of unmonitored movies/seasons during scan #817
base: develop
Are you sure you want to change the base?
Conversation
Converted to draft as the title states "WIP". Once it's ready feel free to make this pr ready for review |
@Fallenbagel Thanks I didn't think about the Draft feature This is a WIP but it's basically working as I'd expect. Only drawback is that if you have lots of Unmonitored movies in Radarr the scan should be slower, as it will request each unmonitored movie in db every time. Let me know if you have some thoughts about this |
FYI I've been using this for months and it works as expected. A more complete monitoring status support wouldn't be very useful imo, at least I didn't encounter any use case |
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.
I don't think this should be merged if it's only Radarr. Could you implement the same logic for Sonarr seasons?
And shouldn't there be a setting to enable/disable this behavior? You may have unmonitored a movie/tv show but still want to keep it as requested.
This pull request has merge conflicts. Please resolve the conflicts so the PR can be successfully reviewed and merged. |
00d383a
to
3e24708
Compare
@gauthier-th I've added the Setting, disabled by default Also I've tried to add the same behaviour for Sonarr, but I encountered an issue:
Also, not really related to this issue afaik, but still looks like the same kind of problem: |
3e24708
to
07ccea1
Compare
I'll squash the commits when everything's approved Meanwhile any hint on Sonarr weird behaviour is welcome! I probably missed something, don't know much about the project nor Typescript |
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.
It looks like the way things are done are not consistent between Radarr and Sonarr.
Why do you change the status of the Media entity for movies while you remove the SeasonRequest for series?
It may be linked to the issue you experienced.
server/lib/scanners/baseScanner.ts
Outdated
: settings.main.removeUnmonitoredFromRequestsEnabled && | ||
!season.monitored && | ||
season.episodes == 0 | ||
? MediaStatus.UNKNOWN |
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.
Why is this change necessary? Isn't the Sonarr scanner job enough?
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.
It seems to be necessary. It is part of Sonarr scanner job though!
- At the beginning of the job I check if there is any SeasonRequest and delete it if necessary
- And here I check the Status of the Season, as every Season is parsed and its Status checked/updated, it should set un-monitored season with no file to UNKNOWN (as for Movies)
There are several jobs that update the status of a Media: Radarr/Sonarr jobs, and Jellyfin/Emby/Plex jobs, hence the comment about competing jobs. |
For clarity, there are two potential "problems" with un-monitored medias that I've been trying to fix:
|
I still don't get the logic, why "If the season is already marked as available, we force it to stay available"? (season.totalEpisodes === season.episodes && season.episodes > 0) ||
existingSeason.status === MediaStatus.AVAILABLE And I don't get why we don't check anything on this second line (i.e EDIT: Well I did try this simple change, and it seems to fix this issue for one season that was always shown as Available before (although it has been deleted some time ago) |
@gauthier-th I went with your recommendations, and marked as resolved the comments I was sure about. Not sure why/how, but during my last tests the problem seems fixed!
with the same Season/ID, twice in the log. Which means the first time didn't work, right? Is there something I should know about async deletion in typescript or something? This seems a bit random |
94313cd
to
7e9d84d
Compare
There is another job, "Media Availability Sync" that runs and mark the media as unavailable if necessary.
It could come from your Let me review it one more time with the change you made and with your comments in mind. |
b9ff7d4
to
caaf13e
Compare
@gauthier-th I've updated with |
Is the issue with the log message repeated twice fixed? |
After more tests... Apparently not :/ Just for clear context, the log & remove is pretty simple: this.log(
`Removing request for Season ${season.seasonNumber} of tmdbId ${tmdbId} as it is unmonitored`
);
await seasonRequestRepository.remove(requestedSeason); I've tried without the I've also tried a dirty workaround (skip adding the season to What's next could be a new Issue, but just in case it's related.. I forgot about that, but as pointed out in one of my previous comment the issue for this specific case is also that the Season is still marked as Available, although it's been deleted for some time now. Could it be that Jellyseerr relies only Jellyfin's "basic season status" to check the status of a Season? I wanted to give a try with the official image to confirm I have the same issue, but got the error |
ece1550
to
caaf13e
Compare
I now realize that this last test (and all other similar weird behaviour I had with deleted seasons) is out of scope of this PR.... It's only a Media Availability Sync issue. Still I don't understand why remove does not remove, but it's not related to "an unmonitored tv show", only a "deleted tv show". |
I may have an idea. Your In the availability sync job, we don't really delete the media, but we set its status as Also, you don't remove the request for the movie, but you do remove it for shows. Try to update these 2 functions to mimic the behavior of |
|
I understand! And about my "scenario 2" of this previous comment, I guess it's out of scope of this PR, and could be fixed / worked around by another feature request, for example admins being able to edit requests (something like #585) |
This pull request has merge conflicts. Please resolve the conflicts so the PR can be successfully reviewed and merged. |
caaf13e
to
1bab491
Compare
@gauthier-th FYI I've removed the part about TV Shows from my commit (basically went back to my first iteration...) I don't know how to manage unmonitored shows. Should we check for seasons only, or the whole show? Should we also set it (every episodes, or..?!) to As a reminder, the original bug (it was more a bug fix than a new feature imo) was that: This is mainly due to Radarr's Import Lists, which is a very useful feature imo (I'd even say that's why I've started to use Radarr in the first place). But I guess this is not something that should happen with Shows, I don't think anybody adds monitored TV Shows automatically with Import List in Sonarr (at least I don't), and I actually have no idea if this bug also exists as I don't always understand the logic behind "requested" in Jellyseerr (and I don't have any test env, I don't want to mess with my production env again). I understand if you prefer to close this PR, I'll continue to use my fork anyway |
1bab491
to
6d662e4
Compare
Movies added and monitored in radarr will not show as requested. Can you test this. We don't do bidirectional sync like this. There Was a bug where it deleted requests making it appear as so but this is not the case and this never was the case. If you believe so could you record and show me after adding a movie as monitored (a new movie) and then show me that exact same movie as marked as requested on jellyseerr? Asking because I might have missed something so I want to confirm before proceeding. I want to be able to reproduce this if it's a confirmed bug. |
Well I've just checked some random recently auto-imported (and monitored) movies in Radarr, I confirm it's still the case. But that can make sense, specifically when they're unreleased: users shouldn't need/be able to request movies that will already be automatically downloaded when they're available I guess (at least in current Jellyseer's state; it could be useful if Movies can be requested by more than 1 user though). |
So the ones added today don't appear as Requested as the sync didn't happen yet. (But again, I think in current state of thing this is a wanted behaviour, as Monitored status isn't shown. Otherwise users will request "for nothing" all the unreleased movies that are already monitored! Be able to request an already-monitored movie could make sense only if more than 1 user is able to request a movie imo - they could then be seen more like a "follow" movie) |
Oh so it does change the status. I see. iirc it wasn't like this previously as I remember being able to request ones already added to radarr (but i guess this was changed recently and since I haven't looked into it yet did not notice it). I will have a look at it. |
This has been like that since I use Jellyseer I believe, so more than 1 year. But for sure at least since I've reported the issue (of movies that still appear as requested even after being unmonitored), so since April If you "fix" that behavior, users will be even less informed about what's monitored or not though, right? |
We started as a fork during 2022. IIRC it did not work like that because I remember support requests about it (but i could be misremembering as well)
Yes. Which is why i said I will look into it for a better way to handle this without requiring major refactors |
Description
Quick & easy first implementation of Monitoring status support in Radarr:
When a movie - which was monitored before - is unmonitored, it won't appear as "requested" in Jellyseerr anymore
Issues Fixed or Closed