feat(search): add filters for search results#2725
Open
civ2boss wants to merge 11 commits intoseerr-team:developfrom
Open
feat(search): add filters for search results#2725civ2boss wants to merge 11 commits intoseerr-team:developfrom
civ2boss wants to merge 11 commits intoseerr-team:developfrom
Conversation
- add filters to search results for all, movies, tvshows, people, or collections
- add support for filtering search results by type
📝 WalkthroughWalkthroughAdds a search-type filter end-to-end: new Changes
Sequence DiagramsequenceDiagram
actor User
participant SearchUI as "Search Component"
participant SearchRoute as "Search Route"
participant TMDBWrapper as "TheMovieDb Wrapper"
participant TMDBAPI as "TMDB API"
User->>SearchUI: select searchType & enter query
SearchUI->>SearchRoute: GET /search?query=Q&searchType=type
SearchRoute->>SearchRoute: validate query\nparse searchType
alt searchType == "movie"
SearchRoute->>TMDBWrapper: searchMovies(query, page)
else searchType == "tv"
SearchRoute->>TMDBWrapper: searchTvShows(query, page)
else searchType == "person"
SearchRoute->>TMDBWrapper: searchPerson(query, page)
else searchType == "collection"
SearchRoute->>TMDBWrapper: searchCollections(query, page)
else
SearchRoute->>TMDBWrapper: searchMulti(query, page)
end
TMDBWrapper->>TMDBAPI: GET /search/{endpoint}
TMDBAPI-->>TMDBWrapper: results
TMDBWrapper->>TMDBWrapper: tag results with media_type
TMDBWrapper-->>SearchRoute: tagged results
SearchRoute-->>SearchUI: JSON response
SearchUI-->>User: render results
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📝 Coding Plan
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
seerr-api.yml (1)
5188-5219:⚠️ Potential issue | 🟠 Major
searchType=collectionis undocumented in the/searchresponse schema.You added
collectionas a valid query type, but the200response still only declares movie/tv/person result variants. This creates an API contract mismatch for clients consuming the OpenAPI spec.✅ Suggested schema update
results: type: array items: anyOf: - $ref: '#/components/schemas/MovieResult' - $ref: '#/components/schemas/TvResult' - $ref: '#/components/schemas/PersonResult' + - $ref: '#/components/schemas/CollectionResult'Also add
components/schemas/CollectionResult(search-result shape), since the existingCollectionschema models collection details, not search cards.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@seerr-api.yml` around lines 5188 - 5219, The /search query adds searchType=collection but the 200 response schema under /search still only lists MovieResult, TvResult, and PersonResult; update the response by adding a CollectionResult variant to the anyOf so collection search results are declared (alongside MovieResult, TvResult, PersonResult), and add a new components/schemas/CollectionResult that models the search-card shape for collections (distinct from the existing Collection detail schema) so the OpenAPI contract matches the allowed searchType values.
🧹 Nitpick comments (1)
server/routes/search.test.ts (1)
320-332: The invalid-searchTypefallback test conflicts with middleware-based query validation.This case currently asserts
200+ fallback for an invalid enum value, but production behavior should be validator-driven. Consider asserting400in an integration-style test (with validator middleware mounted), or move fallback testing to a lower-level parser unit test only.Based on learnings, query-parameter validity in
server/routes/**/*.tsis intentionally enforced by the upstream OpenAPI validator middleware rather than route-level defensive handling.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/routes/search.test.ts` around lines 320 - 332, The test "falls back to searchMulti for an unrecognised searchType" conflicts with the OpenAPI validator middleware that should reject invalid enum query params; update the test to reflect validator-driven behavior by expecting a 400 when calling the '/search' route with searchType=invalid (while keeping the tmdb.searchMulti stub as-is), or alternatively remove this integration test and move the fallback behavior into a lower-level unit test of the search-type parser (e.g. the function that maps query.searchType to handler) where you can assert the fallback to tmdb.searchMulti without the validator middleware present.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@server/api/themoviedb/index.ts`:
- Around line 260-286: The searchCollections method ignores the includeAdult
flag from SearchOptions; update the params passed to get() in searchCollections
to include include_adult mapped from the includeAdult option (preserve existing
defaults/behavior), i.e., read includeAdult from the method args and add
params.include_adult = includeAdult so the API receives the adult-filtering
flag; modify the public searchCollections(...) signature/body (the SearchOptions
destructuring and the params object) accordingly.
- Around line 235-258: The searchPerson method is missing the includeAdult
option from SearchOptions; update the searchPerson signature to destructure
includeAdult (e.g., { query, page = 1, language = this.locale, includeAdult =
false }) and pass includeAdult into the params object sent to this.get
(alongside query, page, language) so the TMDB API receives the adult-filter flag
consistent with searchMovies and searchTvShows.
In `@src/components/Search/index.tsx`:
- Around line 62-86: The <select> with id "searchType" lacks an accessible
label; add a programmatic label linked to that control (e.g., a <label
htmlFor="searchType"> or an aria-label/aria-labelledby) so screen readers can
identify it; update the JSX around the existing select (referencing
id="searchType", name="searchType", value={currentSearchType}, onChange={(e) =>
setCurrentSearchType(e.target.value as SearchType)}) to include the label text
(use intl.formatMessage for localization) or an aria-labelledby reference to a
visually-hidden element.
---
Outside diff comments:
In `@seerr-api.yml`:
- Around line 5188-5219: The /search query adds searchType=collection but the
200 response schema under /search still only lists MovieResult, TvResult, and
PersonResult; update the response by adding a CollectionResult variant to the
anyOf so collection search results are declared (alongside MovieResult,
TvResult, PersonResult), and add a new components/schemas/CollectionResult that
models the search-card shape for collections (distinct from the existing
Collection detail schema) so the OpenAPI contract matches the allowed searchType
values.
---
Nitpick comments:
In `@server/routes/search.test.ts`:
- Around line 320-332: The test "falls back to searchMulti for an unrecognised
searchType" conflicts with the OpenAPI validator middleware that should reject
invalid enum query params; update the test to reflect validator-driven behavior
by expecting a 400 when calling the '/search' route with searchType=invalid
(while keeping the tmdb.searchMulti stub as-is), or alternatively remove this
integration test and move the fallback behavior into a lower-level unit test of
the search-type parser (e.g. the function that maps query.searchType to handler)
where you can assert the fallback to tmdb.searchMulti without the validator
middleware present.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1f7a7171-a629-4ee0-9b00-051b86cc0178
📒 Files selected for processing (9)
package.jsonseerr-api.ymlserver/api/themoviedb/index.tsserver/api/themoviedb/interfaces.tsserver/routes/search.test.tsserver/routes/search.tssrc/components/Search/index.tsxsrc/i18n/globalMessages.tssrc/i18n/locale/en.json
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@server/api/themoviedb/index.ts`:
- Around line 235-258: The searchPerson method currently swallows all errors and
returns a fake first-page empty result; change the catch block in searchPerson
(and the analogous collection/person search methods mentioned) to not mask
failures: either rethrow the caught error (throw err) so callers can handle
upstream failures, or if you must return a fallback, log the full error (using
your logger) and return the same structure but preserve the original requested
page (use the incoming page variable) and include error context so paginated
clients know not to stop. Update the catch to capture the error object (catch
(err)) and apply this behavior consistently for the other search methods.
In `@src/components/Search/index.tsx`:
- Around line 29-30: currentSearchType is only kept in component state so the UI
resets on refresh/back; initialize it from router.query.searchType (fallback to
'all') and keep it in sync with the URL: on mount/read router.query set
currentSearchType accordingly, and whenever you call setCurrentSearchType (e.g.,
the handlers referenced around the current change points) also push/replace the
new searchType into the URL via router.push or router.replace (use shallow: true
to avoid a full reload). Also subscribe to router.query changes (useEffect
watching router.query.searchType) to update state when navigation happens
externally. Ensure the query param name is "searchType" and coerce/validate
values to the SearchType union with 'all' as default.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 86fc1409-ae9e-4017-8e9b-395b8eb50bbe
📒 Files selected for processing (3)
seerr-api.ymlserver/api/themoviedb/index.tssrc/components/Search/index.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- seerr-api.yml
Comment on lines
+235
to
+258
| public searchPerson = async ({ | ||
| query, | ||
| page = 1, | ||
| includeAdult = false, | ||
| language = this.locale, | ||
| }: SearchOptions): Promise<TmdbSearchPersonResponse> => { | ||
| try { | ||
| const data = await this.get<TmdbSearchPersonResponse>('/search/person', { | ||
| params: { | ||
| query, | ||
| page, | ||
| include_adult: includeAdult, | ||
| language, | ||
| }, | ||
| }); | ||
|
|
||
| return data; | ||
| } catch { | ||
| return { | ||
| page: 1, | ||
| results: [], | ||
| total_pages: 1, | ||
| total_results: 0, | ||
| }; |
There was a problem hiding this comment.
Don’t mask TMDB failures as an empty first page.
Both new search paths return { page: 1, total_pages: 1, results: [] } on any upstream error. That makes person/collection searches indistinguishable from a legitimate zero-result query, and for page 2+ failures it tells paginated clients to stop loading more results. Bubble the error, or at least preserve the requested page and log the failure before falling back.
Also applies to: 262-288
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/api/themoviedb/index.ts` around lines 235 - 258, The searchPerson
method currently swallows all errors and returns a fake first-page empty result;
change the catch block in searchPerson (and the analogous collection/person
search methods mentioned) to not mask failures: either rethrow the caught error
(throw err) so callers can handle upstream failures, or if you must return a
fallback, log the full error (using your logger) and return the same structure
but preserve the original requested page (use the incoming page variable) and
include error context so paginated clients know not to stop. Update the catch to
capture the error object (catch (err)) and apply this behavior consistently for
the other search methods.
Comment on lines
+29
to
30
| const [currentSearchType, setCurrentSearchType] = useState<SearchType>('all'); | ||
| const router = useRouter(); |
There was a problem hiding this comment.
Persist searchType in the route.
currentSearchType only lives in component state, so refresh/share/back navigation always falls back to all even though the backend already accepts searchType. Initializing from router.query.searchType and pushing changes back into the URL would keep filtered searches bookmarkable and browser history consistent.
Also applies to: 42-45, 66-72
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/Search/index.tsx` around lines 29 - 30, currentSearchType is
only kept in component state so the UI resets on refresh/back; initialize it
from router.query.searchType (fallback to 'all') and keep it in sync with the
URL: on mount/read router.query set currentSearchType accordingly, and whenever
you call setCurrentSearchType (e.g., the handlers referenced around the current
change points) also push/replace the new searchType into the URL via router.push
or router.replace (use shallow: true to avoid a full reload). Also subscribe to
router.query changes (useEffect watching router.query.searchType) to update
state when navigation happens externally. Ensure the query param name is
"searchType" and coerce/validate values to the SearchType union with 'all' as
default.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR adds new filter options on the search results page to allow users to sort content by mediaType (movie, tv, person, collection). Filter UI is based on the one recently added on the trending page.
Tests for the search endpoint were generated by Claude. The rest of the code were written by me with some refactoring done by Claude. Mostly reducing some repetition in the search route code.
How Has This Been Tested?
Screenshots / Logs (if applicable)
Checklist:
pnpm buildpnpm i18n:extractSummary by CodeRabbit
New Features
Bug Fixes
Tests
Chores