Add rewatch feature to episode NextUp widget #275
Conversation
| rewatch_days = int(settings.getSetting("rewatch_days")) | ||
| if rewatch_days > 0: | ||
| rewatch_since = datetime.datetime.today() - datetime.timedelta(days=rewatch_days) | ||
| url_params["nextUpDateCutoff"] = rewatch_since.strftime("%Y-%m-%d") |
There was a problem hiding this comment.
I haven't had a chance to test yet, but this changes the entirety of next up, not just the rewatching feature, no? That doesn't seem like an intentional change that should be included here
There was a problem hiding this comment.
This would indeed change the complete behavior of next up, but it would be the same behavior as the current android client.
Additionally it would change it "only" if the rewatch feature is enabled in the addon settings.
The problem is, that without the date cutoff, the query returns a bunch of episodes that are not relevant (rewatch finished months ago or stopped).
I see two alternatives to the current implementation:
- Make it a new separate widget
- Make a second API call and combine data of next up with next up + rewatch and filter out duplicates
There was a problem hiding this comment.
I don't have too much of an opinion, honestly. I personally don't see the need for the feature, and from what I saw after it was added it's pretty finnicky (or was, at least). I'll likely never use it myself.
Doing multiple api calls will slow the menu down quite a bit. There's the second network round trip of actually doing the request, then merging the data locally. It wouldn't surprise me if it comes down to anywhere from half a second to a full second slower. But it does eliminate the changing behavior of non-rewatching episodes not all being displayed.
Basically, ¯\_(ツ)_/¯. I would say implement it whichever way you think is best, and if it's problematic we can always adjust it later in a future release. As long as it's disabled by default (which it currently is).
|
This now has merge conflicts. You should only edit the english language files here, and then any other languages are translated through https://translate.jellyfin.org. Otherwise it will break the weblate integration and require manual intervention to fix |
|
I will do that. Any feedback on the alternative implementations I suggested? Personally I think the two query approach would be the best option. |
bwachtendorf
force-pushed
the
add-nextup-rewatch
branch
from
5bf2a30
to
b650905
Compare
|
I added a toggle to switch between the original behavior and the two API call approach. |
|
I have been using my feature branch for two weeks now on my NVIDIA Shield and didn't encounter any bugs. |
bwachtendorf
force-pushed
the
add-nextup-rewatch
branch
from
b650905
to
4bbeeba
Compare
|
@TrueTechy could you take a look at this? @mcarlton00 seems to have disappeared and I kind of forgot about this. |
mcarlton00
added
enhancement
Adds support for SenorSmartyPants rewatch feature to the episode NextUp widget.
See jellyfin/jellyfin#7253 for server side implementation.