Skip to content
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: override include_watched w/ collection label #107

Merged
merged 4 commits into from
Nov 25, 2023
Merged

feat: override include_watched w/ collection label #107

merged 4 commits into from
Nov 25, 2023

Conversation

luukleenders
Copy link
Contributor

@luukleenders luukleenders commented Nov 24, 2023

feat: override include_watched w/ collection label

  • Added include_watched method to MetaData and MediaContainer models, looks at both env var and collection label.
  • Added HubWatchedTransform and use it for get_hubs_sections and transform_hubs_home route handlers.
  • Removed WatchedFilter from transform_hubs_home route handler, filtering is now handled by transform.
  • Removed include_watched logic from HubMixTransform (this is now done in HubWatchedTransform).
  • Improve include_watched logic in LibraryMixTransform, account for collection label and get correct size.

feat: change include_watched to exclude_watched

  • Changed include_watched to exclude_watched in config and models.
  • Inverted logic in trasforms.
  • Removed logic from limit in route handler, not sure if this is correct.
  • Updated readme.

- Added `include_watched` method to `MetaData` and `MediaContainer` models, looks at both env var and collection label.
- Added `HubWatchedTransform` and use it for `get_hubs_sections` and `transform_hubs_home` route handlers.
- Removed `WatchedFilter` from `transform_hubs_home` route handler, filtering is now handled by transform.
- Removed include_watched logic from `HubMixTransform` (this is now done in `HubWatchedTransform`).
- Improve include_watched logic in `LibraryMixTransform`, account for collection label and get correct size.
@lostb1t
Copy link
Owner

lostb1t commented Nov 24, 2023

thanks for this. Ill have look at the code this weekend.

One thing is that the naming doesnt make sense anymore (thats my fault). Plex default is include and replex removes that. And adding REPLEX_INCLUDE_WATCHED as a label to exclude watched makes it even weirder.

So might be a good time to invert this and replace REPLEX_INCLUDE_WATCHED with REPLEX_EXCLUDE_WATCHED (default value set to false for global env) for both the global setting and the collection label

Could you also change the readme for this and info for the new label?

- Changed include_watched to exclude_watched in config and models.
- Inverted logic in trasforms.
- Removed logic from limit in route handler, not sure if this is correct.
- Updated readme.
@luukleenders
Copy link
Contributor Author

The name change makes sense, I've changed include to exclude - not sure if I did the 250 limit thing right though.
Also updated the readme :)

Copy link
Owner

@lostb1t lostb1t left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just keep and adapt the pagination and its good togo.

let mut offset: i32 = 0;

// in we dont remove watched then we dont need to limit
if config.include_watched {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this can be adapted and kept. As it enables pagination (not supported when using excluded watched)

let mut limit: i32 = 250;
let mut offset: i32 = 0;

// in we dont remove watched then we dont need to limit
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this can be adapted and kept. As it enables pagination (not supported when using excluded watched)

@luukleenders
Copy link
Contributor Author

I've tried to put pagination back but I should probably dive into how pagination actually works.

With REPLEX_EXCLUDE_WATCHED=0 I get a maximum of 50 items back but the Plex ui won't load more when I scroll (my assumption is that it should?).
With REPLEX_EXCLUDE_WATCHED=1 I get my entire collection back at once.

Maybe I'm also just missing some context of different clients though, all I've tested on so far is the Plex web app.

@lostb1t
Copy link
Owner

lostb1t commented Nov 25, 2023

Pagination across clients is messy. Never got it right.

the gist is that when watched is excluded pagination wont work. So replex returns the max of 250 per media type to counter that a little bit.

When watched is included pagination should work. But in practice the clients are finniky. I think it doesn't work in the main branch properly either so this should be ok.

@lostb1t
Copy link
Owner

lostb1t commented Nov 25, 2023

@luukleenders pagination in default_transform should be put back again aswell

@luukleenders
Copy link
Contributor Author

Whoops, missed the default_transform - added it back.

@lostb1t lostb1t merged commit aad5f7d into lostb1t:main Nov 25, 2023
2 checks passed
@lostb1t
Copy link
Owner

lostb1t commented Nov 25, 2023

top, nogmaals dank hiervoor. Push gelijk een nieuwe release.

@lostb1t
Copy link
Owner

lostb1t commented Nov 25, 2023

🎉 This PR is included in version 0.20.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@luukleenders
Copy link
Contributor Author

top, nogmaals dank hiervoor. Push gelijk een nieuwe release.

Oh NL haha nice!

Maar graag gedaan, was leuk om in te duiken en ik heb er zelf ook wat aan natuurlijk. Het is sowieso een hele mooie toevoeging aan Plex - als je nog andere puten hebt waar ik zou kunnen helpen dan laat maar weten :)

@luukleenders luukleenders deleted the feature/include-watched-label branch April 22, 2024 21:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants