-
-
Notifications
You must be signed in to change notification settings - Fork 289
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
fix(listview): tmdb can return duplicate movies from it's api, this c… #708
fix(listview): tmdb can return duplicate movies from it's api, this c… #708
Conversation
Do you have a movie to test this PR which appear duplicated, or the step to reproduce this error ? |
@Gauvino I'm surprised nobody has raised this. It's coming from the upstream TMDB API (https://api.themoviedb.org/3/discover/movie). It seems to be inconsistent and not always reproducible because it's coming from them. The best thing jellyseerr can do on it's side is to guard against duplicates in API responses. I have screenshots I can show but that's really the best I can do. I were to be like, "look it's happening right now with this movie" it would likely be different by the time you read my message lol. I'm guessing it stems from some bug TMBD has in their pagination code as when I was messing around with it, it was typically the last item in a page, and the first item in the next page. |
Here's an API example that I can show that's happening currently. If you look at the last result of the first page and the first result of the the second page you can see the duplicates coming from them. GET https://api.themoviedb.org/3/discover/movie?api_key=db55323b8d3e4154498498a75642b381&page=1
GET https://api.themoviedb.org/3/discover/movie?api_key=db55323b8d3e4154498498a75642b381&page=2
Also, here's a very recent discussion saying that tmdb makes no guarantees that every movie returned will be unique and it's expected behavior for them. |
At a quick glance, you probably want to add that filter to the Disclaimer I do not maintain this repo |
Is this related to: |
Seems like it. I only noticed the issue when looking for something to watch and checked the issues here, but not at overseerr.. I will say feels like a lazy response in that issue. I actually abhor writing JavaScript and don't do nearly any react so there's a bunch about state I don't know. However, I would assume a solution that stores a hashmap of every id added, and as items are added, look ups against the hashmap for the id are done before adding a new item. That would solve any performance problems by using a tiny bit more memory. I just have no idea where to do those things in this codebase. Edit: Pushed a cleaner fix. |
20b3eb6
to
f887fff
Compare
fixing duplicate movies that can be returned from the tmdb api
f887fff
to
f378f3e
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.
I know that Overseerr maintainers don't want to implement this because of some performance concerns.
However, i personally think that this should not be an issue. Having (at most) a few hundreds of IDs stored in a Set as well as a for loop iterating on these IDs will only cause a very small performance overhead that is neglectable.
@fallenbagel WDYT about this?
@allcontributors please add @gageorsburn for code |
I've put up a pull request to add @gageorsburn! 🎉 |
fixing duplicate movies that can be returned from the tmdb api
🎉 This PR is included in version 2.2.0 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
…hange filters out duplicates
Description
Screenshot (if UI-related)
To-Dos
yarn build
yarn i18n:extract
Issues Fixed or Closed