feat(search): add filters for search results#2725
feat(search): add filters for search results#2725civ2boss 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
| 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.
| 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.
There was a problem hiding this comment.
Pull request overview
Adds a media-type filter to the search experience (UI + API) so users can narrow search results to movies, TV, people, or collections, mirroring the trending filter pattern.
Changes:
- Added
searchTypefiltering to the search UI and wired it into the/api/v1/searchquery. - Extended TMDB integration and server search mapping to support person + collection searches.
- Added a comprehensive server test suite for search routes and updated the OpenAPI spec accordingly.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/i18n/locale/en.json | Adds English strings for “Person/People”. |
| src/i18n/globalMessages.ts | Adds global i18n message keys for person/people. |
| src/components/Search/index.tsx | Adds search type selector and passes searchType to the search API. |
| server/routes/search.ts | Adds searchType query param handling and routes to TMDB type-specific endpoints. |
| server/routes/search.test.ts | New route tests covering searchType, provider queries, and error handling. |
| server/api/themoviedb/interfaces.ts | Adds response interfaces for person/collection search endpoints. |
| server/api/themoviedb/index.ts | Implements searchPerson and searchCollections. |
| seerr-api.yml | Documents searchType param and includes collection results in the response union. |
| package.json | Updates test script invocation to use --experimental-strip-types. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - $ref: '#/components/schemas/MovieResult' | ||
| - $ref: '#/components/schemas/TvResult' | ||
| - $ref: '#/components/schemas/PersonResult' | ||
| - $ref: '#/components/schemas/CollectionResult' |
There was a problem hiding this comment.
OpenAPI spec adds CollectionResult to the /search response, but there is no corresponding components/schemas/CollectionResult defined in this file. This makes the spec invalid for generators/validators. Add a CollectionResult schema (matching the API response fields) or reference an existing schema name if one already exists.
| - $ref: '#/components/schemas/CollectionResult' | |
| - type: object | |
| properties: | |
| id: | |
| type: number | |
| example: 10 | |
| name: | |
| type: string | |
| example: 'Star Wars Collection' | |
| overview: | |
| type: string | |
| example: 'An epic space-opera media franchise.' | |
| posterPath: | |
| type: string | |
| nullable: true | |
| example: '/abc123.jpg' | |
| backdropPath: | |
| type: string | |
| nullable: true | |
| example: '/def456.jpg' | |
| mediaType: | |
| type: string | |
| example: collection |
| try { | ||
| if (searchProvider) { | ||
| const [id] = queryString | ||
| .toLowerCase() | ||
| .match(searchProvider.pattern) as RegExpMatchArray; | ||
| results = await searchProvider.search({ | ||
| id, | ||
| language: (req.query.language as string) ?? req.locale, | ||
| query: queryString, | ||
| }); | ||
| } else { |
There was a problem hiding this comment.
searchType is ignored when a special searchProvider (tmdb:/imdb:/tvdb:/year:) matches, so requests like ?query=year:2020&searchType=movie will still return mixed types. If the UI/API contract is that searchType filters results, apply it in the provider branch too (e.g., filter/tag provider results by searchType after searchProvider.search() returns).
| const searchParams = { | ||
| query: queryString, | ||
| page: Number(req.query.page), |
There was a problem hiding this comment.
page: Number(req.query.page) will be NaN when page is missing or non-numeric, which bypasses the TMDB client’s default page = 1 and can produce invalid TMDB requests. Consider parsing/validating page and only passing it when it’s a finite positive number (otherwise omit it so defaults apply).
| const searchParams = { | |
| query: queryString, | |
| page: Number(req.query.page), | |
| const parsedPage = Number(req.query.page); | |
| const searchParams = { | |
| query: queryString, | |
| ...(Number.isFinite(parsedPage) && parsedPage > 0 | |
| ? { page: parsedPage } | |
| : {}), |
| searchRoutes.get('/', async (req, res, next) => { | ||
| const queryString = req.query.query as string; | ||
|
|
||
| if (!queryString) { | ||
| return next({ status: 400, message: 'query is required.' }); | ||
| } |
There was a problem hiding this comment.
const queryString = req.query.query as string; plus if (!queryString) doesn’t guard against query being a string array (Express can represent repeated query params that way), and toLowerCase()/.match() will throw. Consider validating typeof req.query.query === 'string' (and possibly trim().length > 0) before using it.
| type SearchType = 'all' | 'movie' | 'tv' | 'person' | 'collection'; | ||
|
|
||
| const Search = () => { | ||
| const intl = useIntl(); | ||
| const [currentSearchType, setCurrentSearchType] = useState<SearchType>('all'); | ||
| const router = useRouter(); | ||
|
|
There was a problem hiding this comment.
The search type filter state is not initialized from router.query.searchType, so deep-linking to /search?query=...&searchType=person (or a page refresh) will still request searchType=all. Consider deriving initial state from the router query (and/or pushing the selected searchType into the URL on change) so the filter is shareable and survives reloads.
| titles, | ||
| fetchMore, | ||
| error, | ||
| } = useDiscover<MovieResult | TvResult | PersonResult>( | ||
| } = useDiscover<MovieResult | TvResult | PersonResult | CollectionResult>( | ||
| `/api/v1/search`, | ||
| { | ||
| query: router.query.query, | ||
| searchType: currentSearchType, | ||
| }, |
There was a problem hiding this comment.
Search results can contain multiple media types (movie/tv/person/collection), but useDiscover de-dupes by id only. If two different types share the same numeric TMDB id, one result will be dropped. Consider using a composite key like ${mediaType}:${id} for de-duping (either in useDiscover or by using a search-specific hook).
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