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: add genre/studio/network view to Discover results #1067

Merged
merged 3 commits into from Mar 4, 2021

Conversation

TheCatLady
Copy link
Collaborator

@TheCatLady TheCatLady commented Mar 2, 2021

Description

Adds filtered "Discover" views by genre, studio, and network at:

  • /discover/movies/genre/{genreId}
  • /discover/tv/genre/{genreId}
  • /discover/movie/studio/{studioId}
  • /discover/tv/network/{networkId}

Links to these views added to movie & TV detail pages.

Screenshot (if UI-related)

image

To-Dos

  • Successful build yarn build
  • Translation keys yarn i18n:extract

Issues Fixed or Closed

@TheCatLady TheCatLady requested a review from sct as a code owner March 2, 2021 21:13
@TheCatLady TheCatLady force-pushed the feature/discover-genres branch 4 times, most recently from 5262bbc to bdc5579 Compare March 2, 2021 22:52
@TheCatLady TheCatLady marked this pull request as draft March 2, 2021 23:30
@TheCatLady TheCatLady marked this pull request as ready for review March 2, 2021 23:59
@TheCatLady TheCatLady changed the title feat: add genres view to movie/series Discover results feat: add genre/studio/network view to Discover results Mar 3, 2021
@@ -368,17 +378,18 @@ class TheMovieDb extends ExternalAPI {
page,
include_adult: includeAdult,
language,
with_release_type: '3|2',
Copy link
Owner

Choose a reason for hiding this comment

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

Why are we removing this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It was removing valid results (e.g., items which only have a digital release). I'm unsure why this was added in the first place?

Copy link
Owner

Choose a reason for hiding this comment

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

It ignores the region without it, from what we saw.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@sct It didn't seem to be the case when I tested, but let me do some more testing to verify.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@sct I verified that it does respect the region without with_release_type being set. with_release_type is just an additional filter so that returned results must have at least one release of the specified types in the region. Without with_release_type, results are just required to have a release of any type for the region.

Setting this value to 3|2 just made the region setting appear to do more, since it limited the results to items with a theatrical release in the specified region. I don't think this is desirable, since it filters out items that are exclusive to streaming services, for example.

@sct sct enabled auto-merge (squash) March 4, 2021 04:06
@sct sct merged commit f28112f into develop Mar 4, 2021
@sct sct deleted the feature/discover-genres branch March 4, 2021 04:22
@github-actions
Copy link

🎉 This PR is included in version 1.21.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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.

Make Studio and Genre clickable and searchable
2 participants