fix(discover): fix tv title sorting and validate sortBy per media type - #3305
fix(discover): fix tv title sorting and validate sortBy per media type#3305fallenbagel wants to merge 1 commit into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughTMDB sorting is represented separately for movies and TV, with media-specific API validation and discovery types. TV title sorting uses ChangesTMDB sorting and discovery
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR fixes TV Discover title sorting by switching TMDB TV sort values from the movie-only original_title.* to original_name.*, and tightens server-side validation so only media-type-appropriate sortBy values are forwarded to TMDB. It also refactors the blocklisted tags processor to iterate only valid sort modes per media type and reduce redundant TMDB calls.
Changes:
- Update TV Discover “Title” sorting to use
original_name.asc|desc(and split sort option typing between Movies and TV in the UI). - Validate
sortByper discover route using Zod enums backed by media-type-specific sort option iterables. - Refactor the blocklisted tags job to iterate valid sort values per media type and fetch keyword details once per tag.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/components/Discover/DiscoverTv/index.tsx | Switch TV title sort to original_name.* and use TV-specific sort option typing. |
| src/components/Discover/DiscoverMovies/index.tsx | Use movie-specific sort option typing (no behavior change). |
| server/routes/discover.ts | Split and validate sortBy per media type; stop forwarding arbitrary strings to TMDB. |
| server/job/blocklistedTagsProcessor.ts | Iterate distinct valid sort values per media type; reduce redundant keyword-detail fetches. |
| server/api/themoviedb/index.ts | Split sort option lists/types into movie vs TV, with shared common options. |
Comments suppressed due to low confidence (1)
server/api/themoviedb/index.ts:69
- The server-side
sortByvalidation for TV now only allows values inTvSortOptionsIterable. Previously the API forwarded any string to TMDB, so callers/bookmarks using TMDB'sname.asc|descaliases would now silently fall back to default popularity ordering. If you want to keep compatibility while still validating by media type, consider allowingname.*alongsideoriginal_name.*.
export const TvSortOptionsIterable = [
...CommonSortOptionsIterable,
'first_air_date.desc',
'first_air_date.asc',
'original_name.desc',
'original_name.asc',
] as const;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@server/job/blocklistedTagsProcessor.ts`:
- Around line 155-159: Update the loop using query, page, and sortBy so it
enumerates every sort/page combination exactly once rather than applying
independent modulo cycles. Derive each value from the same linear query index
and preserve fixedSortMode behavior, ensuring all pageLimit pages are paired
with all available sortOptions without omissions or duplicates.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 42d55785-2a22-40f1-a21f-de6f24c672ec
📒 Files selected for processing (5)
server/api/themoviedb/index.tsserver/job/blocklistedTagsProcessor.tsserver/routes/discover.tssrc/components/Discover/DiscoverMovies/index.tsxsrc/components/Discover/DiscoverTv/index.tsx
The TV discover page's title sort sent `sort_by=original_title.*` to `/discover/tv`, which is a movie-only value. TMDB silently ignores invalid `sort_by` parameters and returns its default popularity order, so sorting series by title did nothing. Sort options are now split per media type and validated at the route (previously any string was cast and forwarded unchecked), and TV title sorting uses `original_name.*`. fix #3304
b8418b0 to
1e95c2e
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
src/components/Discover/DiscoverTv/index.tsx:45
- TV sort option values were changed from
original_title.*tooriginal_name.*, butprepareFilterValues(router.query)still accepts anysortBystring. If a user opens an older bookmarked URL like?sortBy=original_title.asc,preparedFilters.sortBybecomes a controlled<select>value that doesn’t match any<option>, which can render a blank selection / cause React controlled-value warnings. Consider normalizing/clearing unknownsortByvalues against the localSortOptionslist before the component usespreparedFilters.
TmdbRatingAsc: 'vote_average.asc',
TmdbRatingDesc: 'vote_average.desc',
TitleAsc: 'original_name.asc',
TitleDesc: 'original_name.desc',
} as const;
Description
The TV discover page's title sort sent
sort_by=original_title.*to/discover/tv, which is a movie-only value. TMDB silently ignores invalidsort_byparameters and returns its default popularity order, so sorting series by title did nothing. Sort options are now split per media type and validated at the route (previously any string was cast and forwarded unchecked), and TV title sorting usesoriginal_name.*.Note
Known limitation (upstream)
title/namewere deliberately not adopted as when I verified against the live TMDB API, their sort orderings are identical to theoriginal_*variants, as insort_by=name.descandsort_by=original_name.descreturn matching ID sequences on/discover/tv, as dotitle.descandoriginal_title.descon/discover/movie. Therefore, switching movies totitlewould change nothing while widening the scope. Thus, this PR keeps Movie'soriginal_title.*, and similarly adoptsoriginal_name.*for TV as the value for sorting.languageparameter affects display strings only. Non-Latin-script titles therefore cluster together in title sorts regardless of their displayed names, no TMDB sort option orders by the localized title, and this cannot be corrected client-side because pagination is server-side. Consistent with earlier reports: title vs original_title exact match, staff confirmation that results are not sorted by translated titles.Invalid sortBy values, including the original_title.* the UI previously emitted for TV are now ignored and fall back to TMDB's default order rather than erroring.
Lastly, the blocklisted tags job now iterates only valid, distinct sort values per media type (previously TV queries and movie queries per tag used cross-type values that silently degraded into duplicate popularity-ordered pages so now this should make fewer requests per tag with strictly better coverage, and keyword details are fetched once per tag instead of once per media type).
How Has This Been Tested?
languageparameter affects ordering./tv.Screenshots / Logs (if applicable)
Checklist:
pnpm buildpnpm i18n:extractSummary by CodeRabbit