feat(discover): add TMDB lists as custom-slider#1896
feat(discover): add TMDB lists as custom-slider#1896AintGotNoLoveToday wants to merge 18 commits intoseerr-team:developfrom
Conversation
AintGotNoLoveToday
requested review from
fallenbagel and
gauthier-th
as code owners
|
Can a preview be done this ? |
I think you would like to ask if I can show you a little preview? :) If you wanna build this with docker and try this on your own: Then you can use a compose.yaml like this: services:
jellyseerr:
build:
#THIS MUST BE THE PATH WHERE YOU CLONE MY FORK INTO
context: ./jellyseerr-tmdb-lists-feat/
dockerfile: Dockerfile.local
args:
COMMIT_TAG: local
image: jellyseerr-fork:dev
container_name: jellyseerr-fork
environment:
- LOG_LEVEL=debug
- TZ=Europe/London
- PORT=5055 # optional
- COMMIT_TAG=local # <<< important: you need this Build-ARG
network_mode: host
ports:
- 5055:5055
restart: no |
Thank you for responding, but the message was addressed to the dev team cause they can launch a GitHub CI to make a Docker image of your PR so we can test it |
|
Beware, low-effort AI slop. |
|
Is there anything else I can do? I don't want to bother anyone, but every PR takes time, and it's always nice when something comes out of it. I also see that there are a lot of PRs at the moment. As I said, if there's anything else I can do to help, please let me know. |
|
We are merging PRs almost every day, but every review takes time, we can't merge everything at the same time. |
|
This pull request has merge conflicts. Please resolve the conflicts so the PR can be successfully reviewed and merged. |
support creating discover sliders from public tmdb lists
|
So, I finally managed to complete this 👍 Just a brief note regarding your new contribution guidelines: For this update, I used an AI tool purely as a supportive tool — mainly to better understand certain parts of the codebase and to streamline the development and merge process. In addition, I thoroughly tested the feature. I believe this feature is a nice value-add for Seerr. If any adjustments or further tests are needed, feel free to let me know 👍 |
fallenbagel
left a comment
fallenbagel
left a comment
There was a problem hiding this comment.
Unfortunately, this PR cannot be reviewed nor merged as it is. It has picked up all commits from develop, showing 121 commits instead of just your TMDB list changes.
To fix this, you'll need to rebase your branch on the latest develop or create a fresh branch with only your original commits, then force push. Happy to review once the PR shows only your changes.
oh, upsi. I will check this. Thanks for your time 👍 |
okay, I think its done :D Just my commits are now showed up instead of the 121.... |
There was a problem hiding this comment.
Pull request overview
This PR introduces support for TMDB lists as a new custom slider type, allowing administrators to configure public TMDB list IDs and display their items in the discovery interface. The implementation includes both v3 and v4 TMDB API fallbacks, frontend UI components for creating and editing these sliders, and comprehensive API documentation. Additionally, unrelated changes to TVDB API handling filter out season 0 content.
- New
TMDB_LISTslider type added to the discovery system - Backend endpoint
/api/v1/discover/list/:listIdsupporting both TMDB API v3 and v4 - Frontend components for creating, editing, and displaying TMDB list sliders with numeric input validation
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| server/constants/discover.ts | Adds TMDB_LIST enum value to DiscoverSliderType |
| src/components/Discover/constants.ts | Adds translation key for TMDB list slider title |
| src/components/Discover/index.tsx | Implements rendering logic for TMDB list slider type |
| src/components/Discover/DiscoverSliderEdit/index.tsx | Adds display name handling for TMDB list slider in edit mode |
| src/components/Discover/CreateSlider/index.tsx | Adds UI for creating/editing TMDB list sliders with numeric input field |
| server/routes/discover.ts | Implements new API endpoint with v3/v4 fallback logic; fixes error handling type safety |
| server/api/themoviedb/index.ts | Adds getList method for fetching TMDB list data via v3 API |
| server/api/tvdb/index.ts | Filters out season 0 content and improves season number validation (unrelated to main feature) |
| seerr-api.yml | Documents new /discover/list/{listId} endpoint with parameters and response schema |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const axiosInstance: any = (tmdb as any).axios; | ||
| const v4Url = `https://api.themoviedb.org/4/list/${listId}`; | ||
|
|
||
| try { | ||
| const resp = await axiosInstance.get(v4Url, { |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
| const mappedResults = data.results.map((result) => | ||
| isMovie(result) | ||
| ? mapMovieResult( | ||
| result, | ||
| media.find( | ||
| (m) => | ||
| m.tmdbId === result.id && m.mediaType === MediaType.MOVIE | ||
| ) | ||
| ) | ||
| : isPerson(result) | ||
| ? mapPersonResult(result) | ||
| : isCollection(result) | ||
| ? mapCollectionResult(result) | ||
| : mapTvResult( | ||
| result, | ||
| media.find( | ||
| (m) => m.tmdbId === result.id && m.mediaType === MediaType.TV | ||
| ) | ||
| ) |
There was a problem hiding this comment.
There's significant code duplication in mapping results between v3 (lines 1009-1028) and v4 (lines 968-989) list processing. The mapping logic for movies, TV shows, persons, and collections is nearly identical. Consider extracting this into a shared helper function to improve maintainability.
| const mappedResults = data.results.map((result) => | |
| isMovie(result) | |
| ? mapMovieResult( | |
| result, | |
| media.find( | |
| (m) => | |
| m.tmdbId === result.id && m.mediaType === MediaType.MOVIE | |
| ) | |
| ) | |
| : isPerson(result) | |
| ? mapPersonResult(result) | |
| : isCollection(result) | |
| ? mapCollectionResult(result) | |
| : mapTvResult( | |
| result, | |
| media.find( | |
| (m) => m.tmdbId === result.id && m.mediaType === MediaType.TV | |
| ) | |
| ) | |
| const mapSearchResultWithMedia = ( | |
| result: any, | |
| mediaItems: Media[] | |
| ) => { | |
| if (isMovie(result)) { | |
| return mapMovieResult( | |
| result, | |
| mediaItems.find( | |
| (m) => | |
| m.tmdbId === result.id && | |
| m.mediaType === MediaType.MOVIE | |
| ) | |
| ); | |
| } | |
| if (isPerson(result)) { | |
| return mapPersonResult(result); | |
| } | |
| if (isCollection(result)) { | |
| return mapCollectionResult(result); | |
| } | |
| return mapTvResult( | |
| result, | |
| mediaItems.find( | |
| (m) => | |
| m.tmdbId === result.id && m.mediaType === MediaType.TV | |
| ) | |
| ); | |
| }; | |
| const mappedResults = data.results.map((result) => | |
| mapSearchResultWithMedia(result, media) |
| return res.status(200).json({ | ||
| page: v4.page ?? 1, | ||
| totalPages: v4.total_pages ?? 1, | ||
| totalResults: mappedResults.length, |
There was a problem hiding this comment.
The totalResults field should use the TMDB API's total_results value (v4.total_results ?? 0) instead of mappedResults.length. Using mappedResults.length only reflects the current page's result count, not the total number of results across all pages. This breaks pagination logic in the UI.
| totalResults: mappedResults.length, | |
| totalResults: v4.total_results ?? 0, |
| case DiscoverSliderType.TMDB_LIST: | ||
| sliderComponent = ( | ||
| <MediaSlider | ||
| sliderKey={`custom-slider-${slider.id}`} | ||
| title={slider.title ?? ''} | ||
| url={`/api/v1/discover/list/${slider.data}`} | ||
| // linkUrl intentionally omitted | ||
| /> | ||
| ); | ||
| break; |
There was a problem hiding this comment.
The new TMDB_LIST slider type lacks test coverage. Consider adding an E2E test case in cypress/e2e/settings/discover-customization.cy.ts or cypress/e2e/discover.cy.ts to verify that TMDB list sliders can be created, display correctly, and handle both valid and invalid list IDs appropriately.
gauthier-th
left a comment
gauthier-th
left a comment
There was a problem hiding this comment.
Can you please also add Cypress tests?
|
This pull request has merge conflicts. Please resolve the conflicts so the PR can be successfully reviewed and merged. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds TMDB List–based discovery: API spec endpoint, TMDB client methods (v3 + v4 fallback), backend route to fetch/map list items with local Media lookup and pagination, and frontend UI to create, edit, and render TMDB List sliders. Changes
Sequence DiagramsequenceDiagram
participant UI as Frontend UI
participant Route as Route Handler<br/>/list/:listId
participant TMDBV3 as TMDB v3 API<br/>/list/{id}
participant TMDBV4 as TMDB v4 API<br/>/4/list/{id}
participant DB as Local Database<br/>(Media lookup)
participant Response as HTTP 200<br/>Response
UI->>Route: GET /api/v1/discover/list/:listId?language=...&page=...
Route->>TMDBV3: Fetch list contents
alt V3 returns results
TMDBV3-->>Route: List data with results
Route->>DB: Lookup Media by tmdbId + mediaType for each result
DB-->>Route: Local Media entities
else V3 empty or fails
TMDBV3-->>Route: Empty or error
Route->>TMDBV4: Fallback: Fetch v4 list (paged)
TMDBV4-->>Route: V4 list data
Route->>DB: Lookup Media by tmdbId + mediaType for each result
DB-->>Route: Local Media entities
end
Route->>Route: Map TMDB results to Movie/Tv/Person/Collection results
Route-->>Response: { page, totalPages, totalResults, results[] }
Response-->>UI: Paginated list display
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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
♻️ Duplicate comments (1)
server/routes/discover.ts (1)
959-980:⚠️ Potential issue | 🔴 CriticalDon't cast v4 list entries into the existing search mappers.
With the current
TmdbSearchResponseV4shape, theseas anycasts are hiding a real contract mismatch.server/models/Search.tsexpects full movie/tv/person/collection result objects here;mapPersonResult()immediately dereferencesknown_for.map(...), and the other branches read titles/posters/overviews that are not present on the v4 entries. Convert the v4 payload to the mapper shape first or hydrate each ID before mapping.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/routes/discover.ts` around lines 959 - 980, The code is casting TmdbSearchResponseV4 entries to the existing mapper shapes which causes runtime errors because v4 entries lack fields expected by mapMovieResult, mapTvResult, mapPersonResult, and mapCollectionResult (see server/models/Search.ts expectations); instead, transform the v4 payload into the mapper shape or fetch/hydrate full items by ID before calling the mappers: update the mappedResults logic to first convert each v4 entry into the full Search result shape (or load the full movie/tv/person/collection by r.id) and then call mapMovieResult/mapTvResult/mapPersonResult/mapCollectionResult with the correct fully-hydrated objects rather than using as any casts.
🤖 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 771-798: The getList method is incorrectly using data.results
instead of the TMDB list payload's items array; update the implementation in
getList to read const items = data?.items ?? [] and return those items (keep the
surrounding pagination shape and types TmdbSearchMultiResponse), ensuring any
downstream code sees the list entries instead of an empty results array.
In `@server/routes/discover.ts`:
- Around line 937-943: The current try/catch blocks around tmdb.getList (v3) and
its v4 fallback swallow errors and return an empty 200 response; change the
logic so that after attempting the v3 call (tmdb.getList) and the v4 fallback
(the subsequent TMDB request), if both fail you call next(err) or otherwise
propagate the error instead of setting data = null/returning success. Update the
error handling in the blocks around the v3→v4 fallback (the try/catch using
tmdb.getList and its fallback code) and the similar blocks mentioned (lines
covering 946-990 and 1029-1035) to preserve the original error (or wrap it with
context) and pass it to next(...) so upstream middleware can surface TMDB
failures rather than returning empty results.
In `@src/components/Discover/index.tsx`:
- Around line 399-408: The TMDB list case renders MediaSlider with
url={`/api/v1/discover/list/${slider.data}`} even when slider.data may be
undefined; update the Discover component's case for DiscoverSliderType.TMDB_LIST
to guard against missing list IDs by checking slider.data (or
slider.data?.toString()) before creating the MediaSlider—if absent, skip
rendering the slider (return null / omit setting sliderComponent) or render a
safe fallback without the API URL; ensure you reference the
DiscoverSliderType.TMDB_LIST branch and the MediaSlider props (sliderKey, title,
url) when making the change.
---
Duplicate comments:
In `@server/routes/discover.ts`:
- Around line 959-980: The code is casting TmdbSearchResponseV4 entries to the
existing mapper shapes which causes runtime errors because v4 entries lack
fields expected by mapMovieResult, mapTvResult, mapPersonResult, and
mapCollectionResult (see server/models/Search.ts expectations); instead,
transform the v4 payload into the mapper shape or fetch/hydrate full items by ID
before calling the mappers: update the mappedResults logic to first convert each
v4 entry into the full Search result shape (or load the full
movie/tv/person/collection by r.id) and then call
mapMovieResult/mapTvResult/mapPersonResult/mapCollectionResult with the correct
fully-hydrated objects rather than using as any casts.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5e174874-24c6-4ab4-988e-d7f1350761b4
📒 Files selected for processing (10)
seerr-api.ymlserver/api/themoviedb/index.tsserver/api/themoviedb/interfaces.tsserver/api/tvdb/index.tsserver/constants/discover.tsserver/routes/discover.tssrc/components/Discover/CreateSlider/index.tsxsrc/components/Discover/DiscoverSliderEdit/index.tsxsrc/components/Discover/constants.tssrc/components/Discover/index.tsx
| /** | ||
| * Retrieve a public TMDB list by its ID. The TMDB API returns a list of mixed | ||
| * media items under the `items` property. This helper normalises the response | ||
| * to the same paginated structure used by discover and trending endpoints. | ||
| * If the list is private or does not exist, an exception will be thrown. | ||
| */ | ||
| public getList = async ({ | ||
| listId, | ||
| language = this.locale, | ||
| }: { | ||
| listId: number; | ||
| language?: string; | ||
| }): Promise<TmdbSearchMultiResponse> => { | ||
| try { | ||
| const data = await this.get<TmdbSearchMultiResponse>(`/list/${listId}`, { | ||
| params: { | ||
| language, | ||
| }, | ||
| }); | ||
|
|
||
| const items = data?.results ?? []; | ||
|
|
||
| return { | ||
| page: 1, | ||
| total_pages: 1, | ||
| total_results: items.length, | ||
| results: items | ||
| }; |
There was a problem hiding this comment.
Use the TMDB list payload's items array here.
The new docblock says /list/{id} returns entries under items, but this implementation reads data.results. That makes successful v3 list fetches look empty and forces server/routes/discover.ts into the v4 fallback every time.
🐛 Proposed fix
+type TmdbListResponse = {
+ items: Array<
+ TmdbMovieResult | TmdbTvResult | TmdbPersonResult | TmdbCollectionResult
+ >;
+ item_count: number;
+};
+
public getList = async ({
listId,
language = this.locale,
}: {
listId: number;
language?: string;
}): Promise<TmdbSearchMultiResponse> => {
try {
- const data = await this.get<TmdbSearchMultiResponse>(`/list/${listId}`, {
+ const data = await this.get<TmdbListResponse>(`/list/${listId}`, {
params: {
language,
},
});
- const items = data?.results ?? [];
+ const items = data.items ?? [];
return {
page: 1,
total_pages: 1,
- total_results: items.length,
- results: items
+ total_results: data.item_count ?? items.length,
+ results: items,
};
} catch (e) {
throw new Error(`[TMDB] Failed to fetch list: ${e.message}`);
}
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** | |
| * Retrieve a public TMDB list by its ID. The TMDB API returns a list of mixed | |
| * media items under the `items` property. This helper normalises the response | |
| * to the same paginated structure used by discover and trending endpoints. | |
| * If the list is private or does not exist, an exception will be thrown. | |
| */ | |
| public getList = async ({ | |
| listId, | |
| language = this.locale, | |
| }: { | |
| listId: number; | |
| language?: string; | |
| }): Promise<TmdbSearchMultiResponse> => { | |
| try { | |
| const data = await this.get<TmdbSearchMultiResponse>(`/list/${listId}`, { | |
| params: { | |
| language, | |
| }, | |
| }); | |
| const items = data?.results ?? []; | |
| return { | |
| page: 1, | |
| total_pages: 1, | |
| total_results: items.length, | |
| results: items | |
| }; | |
| type TmdbListResponse = { | |
| items: Array< | |
| TmdbMovieResult | TmdbTvResult | TmdbPersonResult | TmdbCollectionResult | |
| >; | |
| item_count: number; | |
| }; | |
| /** | |
| * Retrieve a public TMDB list by its ID. The TMDB API returns a list of mixed | |
| * media items under the `items` property. This helper normalises the response | |
| * to the same paginated structure used by discover and trending endpoints. | |
| * If the list is private or does not exist, an exception will be thrown. | |
| */ | |
| public getList = async ({ | |
| listId, | |
| language = this.locale, | |
| }: { | |
| listId: number; | |
| language?: string; | |
| }): Promise<TmdbSearchMultiResponse> => { | |
| try { | |
| const data = await this.get<TmdbListResponse>(`/list/${listId}`, { | |
| params: { | |
| language, | |
| }, | |
| }); | |
| const items = data.items ?? []; | |
| return { | |
| page: 1, | |
| total_pages: 1, | |
| total_results: data.item_count ?? items.length, | |
| results: items, | |
| }; | |
| } catch (e) { | |
| throw new Error(`[TMDB] Failed to fetch list: ${e.message}`); | |
| } | |
| }; |
🤖 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 771 - 798, The getList method is
incorrectly using data.results instead of the TMDB list payload's items array;
update the implementation in getList to read const items = data?.items ?? [] and
return those items (keep the surrounding pagination shape and types
TmdbSearchMultiResponse), ensuring any downstream code sees the list entries
instead of an empty results array.
| // 1) v3: all items come without pagination | ||
| try { | ||
| const v3 = await tmdb.getList({ listId, language }); | ||
| data = v3; | ||
| } catch { | ||
| data = null; | ||
| } |
There was a problem hiding this comment.
Surface TMDB failures instead of returning an empty success.
Both inner catch blocks discard the upstream error and fall through to the empty 200 response. That makes a private/nonexistent list or a TMDB outage indistinguishable from a genuinely empty list, which is rough for admin setup and monitoring. Keep the v3→v4 fallback, but once both requests fail, bubble an error via next(...) instead of returning empty results.
Also applies to: 946-990, 1029-1035
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/routes/discover.ts` around lines 937 - 943, The current try/catch
blocks around tmdb.getList (v3) and its v4 fallback swallow errors and return an
empty 200 response; change the logic so that after attempting the v3 call
(tmdb.getList) and the v4 fallback (the subsequent TMDB request), if both fail
you call next(err) or otherwise propagate the error instead of setting data =
null/returning success. Update the error handling in the blocks around the v3→v4
fallback (the try/catch using tmdb.getList and its fallback code) and the
similar blocks mentioned (lines covering 946-990 and 1029-1035) to preserve the
original error (or wrap it with context) and pass it to next(...) so upstream
middleware can surface TMDB failures rather than returning empty results.
| case DiscoverSliderType.TMDB_LIST: | ||
| sliderComponent = ( | ||
| <MediaSlider | ||
| sliderKey={`custom-slider-${slider.id}`} | ||
| title={slider.title ?? ''} | ||
| url={`/api/v1/discover/list/${slider.data}`} | ||
| // linkUrl intentionally omitted | ||
| /> | ||
| ); | ||
| break; |
There was a problem hiding this comment.
Guard against missing TMDB list ID before rendering the slider.
Line 404 can hit /api/v1/discover/list/undefined if slider.data is absent, causing avoidable 400s.
Suggested fix
case DiscoverSliderType.TMDB_LIST:
+ if (!slider.data) {
+ sliderComponent = null;
+ break;
+ }
sliderComponent = (
<MediaSlider
sliderKey={`custom-slider-${slider.id}`}
title={slider.title ?? ''}
url={`/api/v1/discover/list/${slider.data}`}
// linkUrl intentionally omitted
/>
);
break;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| case DiscoverSliderType.TMDB_LIST: | |
| sliderComponent = ( | |
| <MediaSlider | |
| sliderKey={`custom-slider-${slider.id}`} | |
| title={slider.title ?? ''} | |
| url={`/api/v1/discover/list/${slider.data}`} | |
| // linkUrl intentionally omitted | |
| /> | |
| ); | |
| break; | |
| case DiscoverSliderType.TMDB_LIST: | |
| if (!slider.data) { | |
| sliderComponent = null; | |
| break; | |
| } | |
| sliderComponent = ( | |
| <MediaSlider | |
| sliderKey={`custom-slider-${slider.id}`} | |
| title={slider.title ?? ''} | |
| url={`/api/v1/discover/list/${slider.data}`} | |
| // linkUrl intentionally omitted | |
| /> | |
| ); | |
| break; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/Discover/index.tsx` around lines 399 - 408, The TMDB list case
renders MediaSlider with url={`/api/v1/discover/list/${slider.data}`} even when
slider.data may be undefined; update the Discover component's case for
DiscoverSliderType.TMDB_LIST to guard against missing list IDs by checking
slider.data (or slider.data?.toString()) before creating the MediaSlider—if
absent, skip rendering the slider (return null / omit setting sliderComponent)
or render a safe fallback without the API URL; ensure you reference the
DiscoverSliderType.TMDB_LIST branch and the MediaSlider props (sliderKey, title,
url) when making the change.
github-actions
Bot
removed
the
merge conflict
There was a problem hiding this comment.
♻️ Duplicate comments (3)
server/routes/discover.ts (2)
994-999:⚠️ Potential issue | 🟠 MajorBubble the last TMDB error instead of returning an empty success.
Both inner catches erase the upstream failure and the outer catch turns it into the same empty
200payload as a genuinely empty list. Admins won't be able to distinguish an invalid/private list or TMDB outage from a real empty slider.🐛 Proposed fix
- let data: TmdbSearchMultiResponse | null = null; + let data: TmdbSearchMultiResponse | null = null; + let lastError: Error | null = null; ... - } catch { - data = null; + } catch (error) { + data = null; + lastError = error as Error; } ... - } catch { - // if v4 also fails → continue to empty response below + } catch (error) { + lastError = error as Error; } } ... - return res.status(200).json({ + if (lastError) { + return next(lastError); + } + + return res.status(200).json({ page: 1, totalPages: 1, totalResults: 0, results: [], }); } catch (err) { logger.debug('TMDB list slider failed', { label: 'API', errorMessage: (err as Error).message, listId: req.params.listId, }); - return res.status(200).json({ - page: 1, - totalPages: 1, - totalResults: 0, - results: [], - }); + return next(err); }Also applies to: 1044-1046, 1092-1103
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/routes/discover.ts` around lines 994 - 999, Current try/catch blocks swallow TMDB errors (e.g., around the call to tmdb.getList and assignments to data), causing the route to return 200 with an empty payload; modify these blocks to rethrow or propagate the caught error instead of setting data = null so upstream error handling returns an appropriate error status and message. Locate the try/catch wrapping tmdb.getList, and the similar catch blocks around the other TMDB calls (the ones at the other noted ranges), change the empty catch bodies to either throw the caught error (throw err) or call next(err)/pass the error to the route error handler so the real TMDB failure (private/invalid list or outage) bubbles up rather than turning into a successful empty response.
1004-1035:⚠️ Potential issue | 🔴 CriticalHydrate v4 list items before mapping them.
TmdbSearchResponseV4.resultsonly exposesidandmedia_type, but the mappers used here read full TMDB fields like titles, artwork, overview, and votes. Castingrtoanyhides that mismatch and will return mostly undefined cards whenever the v4 fallback is hit. Either enrich each item with the corresponding TMDB detail endpoint before mapping, or drop the v4 path until that data exists.#!/bin/bash set -euo pipefail printf '\nTmdbSearchResponseV4 definition:\n' sed -n '59,80p' server/api/themoviedb/interfaces.ts printf '\nSearch result mappers:\n' sed -n '71,127p' server/models/Search.ts printf '\nCurrent v4 fallback mapping:\n' sed -n '1004,1035p' server/routes/discover.tsExpected result:
TmdbSearchResponseV4.resultsonly containsid/media_type, while the mappers read fields such astitle,overview,poster_path,vote_average, andorigin_country.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/routes/discover.ts` around lines 1004 - 1035, The v4 branch is using TmdbSearchResponseV4 items that only have id/media_type but then calls mappers (mapMovieResult, mapTvResult, mapPersonResult, mapCollectionResult) which expect full TMDB fields; fix by hydrating each v4 item before mapping: after receiving resp from tmdb.getListV4, build a hydratedResults array by Promise.all over resp.results where for each r you call the proper detail endpoint (e.g., tmdb.getMovie(...)/tmdb.getTv(...)/tmdb.getPerson(...)) based on r.media_type, merge the returned detail object into r, then pass hydratedResults into Media.getRelatedMedia and into the mapping step (use the same id/mediaType matching when finding related Media); alternatively remove the v4 fallback path if you cannot call the detail endpoints. Ensure use of Promise.all to parallelize and proper error handling for missing detail responses.server/api/themoviedb/index.ts (1)
886-913:⚠️ Potential issue | 🔴 CriticalRead v3 list entries from
items, notresults.The docblock already says
/list/{id}returnsitems, but the implementation still types the response asTmdbSearchMultiResponseand readsdata.results. Successful v3 list calls will normalize to an empty array and force every request down the fallback path.🐛 Proposed fix
+type TmdbListResponse = { + items: Array< + TmdbMovieResult | TmdbTvResult | TmdbPersonResult | TmdbCollectionResult + >; + item_count: number; +}; + public getList = async ({ listId, language = this.locale, }: { listId: number; language?: string; }): Promise<TmdbSearchMultiResponse> => { try { - const data = await this.get<TmdbSearchMultiResponse>(`/list/${listId}`, { + const data = await this.get<TmdbListResponse>(`/list/${listId}`, { params: { language, }, }); - const items = data?.results ?? []; + const items = data.items ?? []; return { page: 1, total_pages: 1, - total_results: items.length, - results: items + total_results: data.item_count ?? items.length, + results: items, }; } catch (e) { throw new Error(`[TMDB] Failed to fetch list: ${e.message}`); } };#!/bin/bash set -euo pipefail printf '\nCurrent getList implementation:\n' sed -n '886,913p' server/api/themoviedb/index.tsExpected result: the docblock says the TMDB list payload uses
items, but the implementation still readsdata.results.🤖 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 886 - 913, The getList method currently treats the TMDB /list/{id} response as TmdbSearchMultiResponse and reads data.results; update getList to read from data.items instead (e.g., const items = data?.items ?? []), adjust the generic/type used in this.get from TmdbSearchMultiResponse to the correct list response type (or create a TmdbListResponse if needed), and ensure the returned pagination fields (page, total_pages, total_results, results) are derived from items so the normalized response is correct; keep the function name getList and the returned shape intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@server/api/themoviedb/index.ts`:
- Around line 886-913: The getList method currently treats the TMDB /list/{id}
response as TmdbSearchMultiResponse and reads data.results; update getList to
read from data.items instead (e.g., const items = data?.items ?? []), adjust the
generic/type used in this.get from TmdbSearchMultiResponse to the correct list
response type (or create a TmdbListResponse if needed), and ensure the returned
pagination fields (page, total_pages, total_results, results) are derived from
items so the normalized response is correct; keep the function name getList and
the returned shape intact.
In `@server/routes/discover.ts`:
- Around line 994-999: Current try/catch blocks swallow TMDB errors (e.g.,
around the call to tmdb.getList and assignments to data), causing the route to
return 200 with an empty payload; modify these blocks to rethrow or propagate
the caught error instead of setting data = null so upstream error handling
returns an appropriate error status and message. Locate the try/catch wrapping
tmdb.getList, and the similar catch blocks around the other TMDB calls (the ones
at the other noted ranges), change the empty catch bodies to either throw the
caught error (throw err) or call next(err)/pass the error to the route error
handler so the real TMDB failure (private/invalid list or outage) bubbles up
rather than turning into a successful empty response.
- Around line 1004-1035: The v4 branch is using TmdbSearchResponseV4 items that
only have id/media_type but then calls mappers (mapMovieResult, mapTvResult,
mapPersonResult, mapCollectionResult) which expect full TMDB fields; fix by
hydrating each v4 item before mapping: after receiving resp from tmdb.getListV4,
build a hydratedResults array by Promise.all over resp.results where for each r
you call the proper detail endpoint (e.g.,
tmdb.getMovie(...)/tmdb.getTv(...)/tmdb.getPerson(...)) based on r.media_type,
merge the returned detail object into r, then pass hydratedResults into
Media.getRelatedMedia and into the mapping step (use the same id/mediaType
matching when finding related Media); alternatively remove the v4 fallback path
if you cannot call the detail endpoints. Ensure use of Promise.all to
parallelize and proper error handling for missing detail responses.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 22b54b40-be84-49de-8ddd-a1dde93e6055
📒 Files selected for processing (5)
seerr-api.ymlserver/api/themoviedb/index.tsserver/api/themoviedb/interfaces.tsserver/api/tvdb/index.tsserver/routes/discover.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- server/api/themoviedb/interfaces.ts
- seerr-api.yml
|
This PR is stale because it has been open 30 days with no activity. Please address the feedback or provide an update to keep it open. |
Description
This PR introduces a new discovery slider type (
TMDB_LIST) that allows administrators to configure a public TMDB list ID and display its items.Built and tested locally with Docker (
Dockerfile.local), slider works as expected.Screenshot (if UI-related)
To-Dos
pnpm buildpnpm i18n:extract(New keys such as
tmdbListandprovidetmdblistidwere added and need to be present in the i18n schema.)Issues Fixed or Closed
Summary by CodeRabbit