Repository navigation
Conversation
Adds a TMDB_LIST custom Discover slider so admins can surface any public TMDB list as a row of posters, with the usual request buttons and a browsable "show more" page. Closes seerr-team#1837. v3 vs v4 investigation ---------------------- The lists people want to display are created through the TMDB v4 API. Verified against the live API that the v3 endpoint `GET /3/list/{id}` reads those v4 lists just fine, with the same status codes as the v4 endpoint (200 for public, 401 for private, 404 for unknown), the same per-item `media_type` discriminator, and 20-items-per-page pagination: GET /3/list/8542986?api_key=<seerr's shipped v3 key> -> 200, media_type: tv GET /4/list/8542986 (v4 read bearer) -> 200, same item So no v4 read token and no new admin configuration are needed; the existing TheMovieDb client and its shipped v3 key are used as-is. Server ------ The internals are built as a small list-provider abstraction (server/lib/medialists) so other sources such as Trakt or MDBList can be added later: a provider resolves a list id to an ordered set of {mediaType, tmdbId} items, hydration fills in any missing TMDB payload (a no-op for TMDB's own lists, which already carry it), and the service returns a paginated, MediaSlider-compatible response. Arbitrary-URL fetching is deliberately not supported; validateListId is the single choke point where the admin-supplied slider data is checked, and TMDB list ids must be plain integers. The same check rejects bad data at slider create/update time. Caching reuses the existing cacheManager: a new medialist cache holds the fully hydrated list per provider/list/language, fresh for 6 hours and resident for 48 so a stale copy can still be served when TMDB is unreachable. Concurrent requests for the same list are collapsed into a single upstream fetch. Deleted or private lists are recorded as empty, logged once, and render an empty slider rather than failing the page. Local availability is resolved per request since it is user-specific. Frontend -------- New slider type in the Discover switch and in the Create Slider form (with the live preview the other types have), a browsable /discover/list/[listId] page mirroring the network and studio pages, and English strings via the repo's message system. Mixed movie/series lists work and list order is preserved throughout. The enum member is appended at the end of DiscoverSliderType since the values are persisted; a comment now records that constraint.
The list provider accumulated up to 25 upstream pages into a single cache
entry, which both truncated long lists (the endpoint claimed to paginate
the whole list but never served past item 500) and turned one cold slider
render into 25 outbound requests.
Providers now resolve exactly one page: `fetchListPage(listId, { page })`
requests only that page from TMDB and reports `totalPages`/`totalResults`
straight from the upstream response, so the pagination contract is honest
for lists of any size and a cold fill costs one request. The cache is
keyed per (provider, listId, language, page) and keeps the same 6h fresh /
48h stale-on-error / single-flight semantics.
Also bounds what can reach the cache:
- the language dimension is no longer taken from the query string; the
route pins it to the locale resolved server-side and the service
canonicalises it, so the key can only take a handful of values
- the cache is capped at 500 entries, evicting least-recently-fetched
- `page` is parsed once and must be a safe integer >= 1, else HTTP 400
(rejects 0, -1, 1.5, Infinity, oversized values, arrays and objects)
- the TMDB payload is validated at the boundary: pagination counters must
be numbers, `items` an array of objects, ids safe positive integers and
`media_type` one of movie/tv. A malformed payload is treated as an
upstream failure, so a stale copy is served instead of being replaced
with garbage, and nothing malformed is ever cached.
The response now distinguishes a list that could not be read at all
(`list.unavailable: true`) from one that exists and is empty, so the
frontend can react differently to each.
Tests go from 13 to 28: single-flight is proven with a deferred barrier
(all callers pending before the upstream resolves), a rejected in-flight
fetch is shown to be evicted so the next request retries, concurrent stale
callers are shown to share one failed refresh, an entry past its residency
window is shown not to be served, and the page/list-id validation and
malformed-payload cases are enumerated. The discover router is mounted
behind `isAuthenticated()` as it is in production, with an unauthenticated
request asserted to be rejected.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Constrains what the endpoint actually accepts and documents what it actually answers: page is an integer with a minimum of 1, listId carries the same pattern the server validates against, the language query parameter is gone (it is resolved server-side), the stable response fields are marked required, list.unavailable is described, and the 403 and 500 responses are documented alongside the existing 400. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A TMDB list that has been deleted or made private used to render as a slider heading over an empty strip on Discover, and as the generic "No results." grid on its own page, neither of which tells anyone what happened. The list slider now sets `hideWhenEmpty`, so the row disappears instead of advertising an empty one, and the list page renders a dedicated, localized "no longer available" state, keeping the list name as the heading when it is still known. Two supporting fixes in the shared slider: - `hideWhenEmpty` was evaluated before the row had answered, because the optional chain collapsed "not fetched yet" and "fetched, empty" into the same value; it now waits for data, which also removes the crash on an empty `data` array - `isEmpty` was hardcoded to false, so an empty slider drew nothing at all rather than its empty message; it is now derived from the rendered items Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The list id field was a bare text input: nothing said where the number comes from, a wrong id produced a silent, permanently disabled Add button, and every keystroke fired a preview request for an id that was still being typed. - persistent help text under the field points at the number in the list URL - client-side validation now mirrors the server pattern, including the 12 digit cap, instead of accepting any positive integer - the preview waits 400ms after the last keystroke and only fires for a well-formed id - a preview that cannot be read (unknown id, private list, upstream error) renders an explicit message saying so Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Every MediaSlider on Discover issued its two initial page requests on
mount, so a customised Discover with a dozen rows fired two dozen requests
up front for posters most of which are never scrolled to. TMDB list rows
make that worse, since each one is a distinct upstream list.
Rows now hold their SWR key back until an IntersectionObserver reports the
row heading within 600px of the viewport, which keeps above-the-fold rows
loading immediately and defers the rest. Environments without
IntersectionObserver, and rows opting out with `lazy={false}` (the slider
previews, whose result count gates the Add button), fetch as before.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Both slider navigation buttons were icon-only with no accessible name, so screen readers announced two unlabelled buttons on every row of every discover page. They now carry localized aria-labels. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The OpenAPI request validator coerces the documented integer page query parameter to a number before the route handler runs, so rejecting every non-string value returned 400 for all requests, including the default page. Accept both representations; the safe-integer check still rejects malformed input.
Contributor
|
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)
📝 WalkthroughWalkthroughAdded TMDB public-list support across provider contracts, caching, API routes, slider administration, and frontend rendering. The change supports mixed movie and TV results, pagination, unavailable lists, validation, localized labels, lazy loading, and cache fallback behavior. ChangesTMDB list discovery
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
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 |
Author
|
Screenshots — Discover page with several TMDB list sliders (production instance, French locale): 
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
server/routes/discover.test.ts (1)
363-371: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the cache ceiling and the language normalization.
The suite covers freshness, single-flight, stale fallback, and residency well. Two new behaviors in
server/lib/medialists/index.tsremain untested:
evictOverflowandMAX_CACHE_ENTRIES: no test proves that a burst of distinct list/page combinations stops growing the cache, or that the oldest entry is dropped first.normalizeMediaListLanguage: no test proves that variant locales such asen,EN, anden-uscollapse to one cache key, and that an unrecognized value collapses to the empty string.Both are the mechanisms that bound request-controlled cache growth, so a regression there is silent.
Do you want me to generate these two tests?
🤖 Prompt for 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. In `@server/routes/discover.test.ts` around lines 363 - 371, Add tests to the media list caching suite covering both cache-bounding behaviors in server/lib/medialists/index.ts: exercise enough distinct list/page combinations to exceed MAX_CACHE_ENTRIES and assert evictOverflow removes the oldest entry while keeping the cache capped, then request equivalent en, EN, and en-us language variants and verify they share one cache key, while an unrecognized language produces the empty-string normalization key.
🤖 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/api/themoviedb/interfaces.ts`:
- Around line 96-105: Update handling of TmdbListResponse.items so list entries
missing media_type are normalized to media_type "movie" before union validation
or conversion. Preserve existing movie and TV handling when media_type is
present, and ensure classic movie-only lists do not trigger the malformed-list
error path.
In `@src/components/Discover/CreateSlider/index.tsx`:
- Around line 86-103: Update the list-ID change handling around the
debouncedListId flow to call onNewTitles(0) immediately when the raw list ID
changes, before the debounced value is updated. Preserve the existing validation
and request-error reset behavior in the effect, while ensuring a valid
replacement ID cannot retain the previous preview count during the debounce
interval.
In `@src/components/Discover/DiscoverList/index.tsx`:
- Around line 21-35: Guard the useDiscover call in DiscoverListPage/DiscoverList
until router.isReady and router.query.listId is a valid string, preventing
requests to an /undefined endpoint during the initial client render. Pass the
resolved listId only after the dynamic route is ready, and update useDiscover if
necessary to safely skip a null endpoint while preserving normal fetching once
available.
---
Nitpick comments:
In `@server/routes/discover.test.ts`:
- Around line 363-371: Add tests to the media list caching suite covering both
cache-bounding behaviors in server/lib/medialists/index.ts: exercise enough
distinct list/page combinations to exceed MAX_CACHE_ENTRIES and assert
evictOverflow removes the oldest entry while keeping the cache capped, then
request equivalent en, EN, and en-us language variants and verify they share one
cache key, while an unrecognized language produces the empty-string
normalization key.
🪄 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: 6aec66f6-5f5f-4613-8af2-d4eecd7edf26
📒 Files selected for processing (20)
seerr-api.ymlserver/api/themoviedb/index.tsserver/api/themoviedb/interfaces.tsserver/constants/discover.tsserver/lib/cache.tsserver/lib/medialists/index.tsserver/lib/medialists/providers/tmdb.tsserver/lib/medialists/types.tsserver/routes/discover.test.tsserver/routes/discover.tsserver/routes/settings/discover.tssrc/components/Discover/CreateSlider/index.tsxsrc/components/Discover/DiscoverList/index.tsxsrc/components/Discover/DiscoverSliderEdit/index.tsxsrc/components/Discover/constants.tssrc/components/Discover/index.tsxsrc/components/MediaSlider/index.tsxsrc/components/Slider/index.tsxsrc/i18n/locale/en.jsonsrc/pages/discover/list/[listId]/index.tsx
Classic v3 lists are movie-only and their items omit the `media_type` discriminator entirely; only v4 lists, which can mix movies and series, carry one. The boundary validator rejected the missing field as a malformed payload, so every classic list answered HTTP 500 instead of rendering. `TmdbListItem` now models the field as optional on the movie arm, and the provider reads an absent discriminator as a movie, normalising it on the way through. A `media_type` that is present but unsupported is still treated as malformed. Reported by CodeRabbit on seerr-team#3344. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The preview debounces for 400ms, during which the row below still shows the previous list and its reported result count still stands. Replacing a working id with another well-formed one therefore left the Add button enabled on the strength of a preview belonging to a different list. The raw id changing now zeroes the count immediately, so the button can only re-enable once the new preview has actually returned titles. Reported by CodeRabbit on seerr-team#3344. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The two guards that keep the media list cache from being driven by request input had no tests. Adds coverage for both, exporting the entry ceiling so the test cannot drift from the implementation: - locales differing only in case or padding collapse to one cache key, a genuinely different locale gets its own, and anything unusable falls back to the single empty-language key - filling the cache past its ceiling holds the entry count and evicts the least recently fetched page Also covers a classic v3 list whose items carry no media_type. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
👋 @judarange, thank you for your contribution! |
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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
Adds a new custom discover slider type, TMDB List: admins can create a Discover row from any public TMDB list id. This implements the minimal proposal from #1837.
New
DiscoverSliderType.TMDB_LIST(appended at the end of the enum — persisted values unchanged)New endpoint
GET /api/v1/discover/list/:listIdwith strict input validation (numeric id, safe-integer page), page-oriented caching (6h fresh, 48h stale-on-error, single-flight, bounded to 500 entries), and runtime validation of the TMDB payload before anything is cachedWorks with mixed movie/TV lists, preserves list order, resolves local availability per user after the shared cache
Reads lists through the already-shipped TMDB v3 client —
GET /3/list/{id}also returns v4-created lists (verified against live ids), so no new API keys, no new admin settingsBrowsable
/discover/list/[listId]page following the network/studio page pattern; unavailable (deleted/private) lists hide the row and show a dedicated state on the list pageCreateSlider: help text with an example id, client validation aligned with the server, debounced live preview with an explicit error state
MediaSliderrows now lazy-mount near the viewport (generic, default-on; previews opt out) — many custom sliders no longer fire all their fetches on Discover mounta11y: the slider prev/next buttons now have localized aria-labels
Fixes Feature Request: Support for Custom Sliders Based on External or Dynamic Sources (e.g. RSS, TMDB Lists, Keywords) #1837
How Has This Been Tested?
pnpm lint(0 new warnings),pnpm typecheck,pnpm build,pnpm format:check,pnpm i18n:extractall green on Node 22.19 / pnpm 10.24Screenshots / Logs (if applicable)
(screenshots in the first comment below)
Checklist:
seerr-api.yml).pnpm buildpnpm i18n:extractSummary by CodeRabbit