feat(discover): filter movie discovery by minimum release - #3384
feat(discover): filter movie discovery by minimum release#3384demigodmode wants to merge 1 commit into
Conversation
Adds a Minimum Release filter to movie discover. Picking a level matches that release type or later, so Digital also covers physical and TV. with_release_type is a no-op on TMDB unless paired with a release_date range, so when it's set the range switches to release_date and the "To" date defaults to today. re seerr-team#579
📝 WalkthroughWalkthroughMovie discovery now supports release type filtering. The UI validates and submits release types ChangesMovie release type filtering
Estimated code review effort: 3 (Moderate) | ~20 minutes Mergeability Score: 🟡 Moderate · up to The new movie discovery filter currently has an unlabeled selector for assistive technologies, and direct API requests can return unfiltered results when only a minimum release stage is provided. These bounded accessibility and correctness issues should be fixed before merging. Sequence Diagram(s)sequenceDiagram
participant DiscoverFilter
participant DiscoverRoute
participant TheMovieDb
DiscoverFilter->>DiscoverRoute: Submit releaseType filter
DiscoverRoute->>TheMovieDb: Forward validated releaseType
TheMovieDb->>TheMovieDb: Expand release types through type 6
TheMovieDb->>TheMovieDb: Build release_date bounds and with_release_type
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@seerr-api.yml`:
- Around line 5604-5610: Ensure the API applies or documents the UI’s
primaryReleaseDateLte=today default when releaseType is provided. Update the
releaseType schema/handling near its description so direct requests with only
releaseType still include the appropriate release-date bound, or clearly
document that callers must supply primaryReleaseDateLte.
In `@src/components/Discover/FilterSlideover/index.tsx`:
- Around line 143-149: Replace the visible label span in the release-type
selector block with a label element and associate it using
htmlFor="releaseType", preserving the existing message text and select id.
🪄 Autofix
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: e2ec2276-f0a8-4206-bf4a-90f752d6fd59
📒 Files selected for processing (6)
seerr-api.ymlserver/api/themoviedb/index.tsserver/routes/discover.tssrc/components/Discover/FilterSlideover/index.tsxsrc/components/Discover/constants.tssrc/i18n/locale/en.json
| description: >- | ||
| Minimum release availability for movie discovery. The selected type | ||
| and everything above it are matched (e.g. 4 = digital also matches 5 | ||
| = physical and 6 = TV, sent to TMDB as with_release_type=4|5|6). | ||
| When set, results are filtered on release_date instead of | ||
| primary_release_date. 2 = limited theatrical, 3 = theatrical, | ||
| 4 = digital, 5 = physical, 6 = TV |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -u
for file in \
"src/components/Discover/FilterSlideover/index.tsx" \
"server/routes/discover.ts" \
"server/api/themoviedb/index.ts"; do
printf '\n== %s ==\n' "$file"
rg -n -C 10 \
'releaseType|primaryReleaseDateGte|primaryReleaseDateLte|release_date|defaultFutureDate|new Date' \
"$file" || true
doneRepository: seerr-team/seerr
Length of output: 14899
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
route = Path("server/routes/discover.ts").read_text()
client = Path("server/api/themoviedb/index.ts").read_text()
ui = Path("src/components/Discover/FilterSlideover/index.tsx").read_text()
assert "releaseType: query.releaseType" in route
assert "primaryReleaseDateLte: query.primaryReleaseDateLte" in route
assert "primaryReleaseDateGte: query.primaryReleaseDateGte" in route
assert "with_release_type: releaseTypeFilter" in client
assert "if (value && !currentFilters[dateLte])" in ui
assert "[dateLte]: today" in ui
def tmdb_dates(gte=None, lte=None):
# Mirrors the date-bound logic in getDiscoverMovies.
past = "1900-01-01"
future = "<defaultFutureDate>"
return (
past if not gte and lte else gte,
future if not lte and gte else lte,
)
print("route forwards releaseType and both date fields without adding a default")
print("no dates ->", tmdb_dates())
print("gte only ->", tmdb_dates("2025-01-01"))
print("lte only ->", tmdb_dates(lte="2025-01-01"))
print("UI adds today's dateLte only when the user selects releaseType")
PYRepository: seerr-team/seerr
Length of output: 406
Apply or document the primaryReleaseDateLte default for releaseType.
The UI sets primaryReleaseDateLte to today, but direct API requests do not. Requests with only releaseType send no release-date bounds, so TMDB does not apply the release-type filter.
🤖 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 `@seerr-api.yml` around lines 5604 - 5610, Ensure the API applies or documents
the UI’s primaryReleaseDateLte=today default when releaseType is provided.
Update the releaseType schema/handling near its description so direct requests
with only releaseType still include the appropriate release-date bound, or
clearly document that callers must supply primaryReleaseDateLte.
| <span className="text-lg font-semibold"> | ||
| {intl.formatMessage(messages.minimumRelease)} | ||
| </span> | ||
| <select | ||
| id="releaseType" | ||
| name="releaseType" | ||
| value={currentFilters.releaseType ?? ''} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Associate the selector with its visible label.
Line 143 renders the control label as a span. The select on lines 146-149 therefore has no accessible name. Use a label with htmlFor="releaseType".
Proposed fix
- <span className="text-lg font-semibold">
+ <label
+ htmlFor="releaseType"
+ className="text-lg font-semibold"
+ >
{intl.formatMessage(messages.minimumRelease)}
- </span>
+ </label>📝 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.
| <span className="text-lg font-semibold"> | |
| {intl.formatMessage(messages.minimumRelease)} | |
| </span> | |
| <select | |
| id="releaseType" | |
| name="releaseType" | |
| value={currentFilters.releaseType ?? ''} | |
| <label | |
| htmlFor="releaseType" | |
| className="text-lg font-semibold" | |
| > | |
| {intl.formatMessage(messages.minimumRelease)} | |
| </label> | |
| <select | |
| id="releaseType" | |
| name="releaseType" | |
| value={currentFilters.releaseType ?? ''} |
🤖 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 `@src/components/Discover/FilterSlideover/index.tsx` around lines 143 - 149,
Replace the visible label span in the release-type selector block with a label
element and associate it using htmlFor="releaseType", preserving the existing
message text and select id.
SoulofAkuma
left a comment
SoulofAkuma
left a comment
There was a problem hiding this comment.
Thanks for the PR, just my 2 cents. I am not a code owner, so we probably need an actual code owner to review this as well.
| keywords?: string; | ||
| excludeKeywords?: string; | ||
| sortBy?: SortOptions; | ||
| releaseType?: string; |
There was a problem hiding this comment.
| releaseType?: string; | |
| minimumReleaseType?: string; |
| voteAverageLte, | ||
| voteCountGte, | ||
| voteCountLte, | ||
| releaseType, |
There was a problem hiding this comment.
| releaseType, | |
| minimumReleaseType, |
| const releaseTypeFilter = releaseType | ||
| ? Array.from( | ||
| { length: 6 - Number(releaseType) + 1 }, | ||
| (_, i) => Number(releaseType) + i | ||
| ).join('|') | ||
| : undefined; | ||
| const releaseDateField = releaseType | ||
| ? 'release_date' | ||
| : 'primary_release_date'; |
There was a problem hiding this comment.
| const releaseTypeFilter = releaseType | |
| ? Array.from( | |
| { length: 6 - Number(releaseType) + 1 }, | |
| (_, i) => Number(releaseType) + i | |
| ).join('|') | |
| : undefined; | |
| const releaseDateField = releaseType | |
| ? 'release_date' | |
| : 'primary_release_date'; | |
| const releaseTypeFilter = minimumReleaseType | |
| ? Array.from( | |
| { length: 6 - Number(minimumReleaseType) + 1 }, | |
| (_, i) => Number(minimumReleaseType) + i | |
| ).join('|') | |
| : undefined; | |
| const releaseDateField = minimumReleaseType | |
| ? 'release_date' | |
| : 'primary_release_date'; |
| type: string | ||
| example: 2023-01-01 | ||
| - in: query | ||
| name: releaseType |
There was a problem hiding this comment.
| name: releaseType | |
| name: minimumReleaseType |
| <option value=""> | ||
| {intl.formatMessage(messages.anyRelease)} | ||
| </option> | ||
| <option value="2"> | ||
| {intl.formatMessage(messages.limitedTheatrical)} | ||
| </option> | ||
| <option value="3"> | ||
| {intl.formatMessage(messages.theatrical)} | ||
| </option> | ||
| <option value="4">{intl.formatMessage(messages.digital)}</option> | ||
| <option value="5">{intl.formatMessage(messages.physical)}</option> | ||
| <option value="6"> | ||
| {intl.formatMessage(messages.tvRelease)} | ||
| </option> |
There was a problem hiding this comment.
The Premiere release type is missing. Is that intentional?
| [dateLte]: today, | ||
| }); | ||
| } else { | ||
| updateQueryParams('releaseType', value); |
There was a problem hiding this comment.
| updateQueryParams('releaseType', value); | |
| updateQueryParams('minimumReleaseType', value); |
|
|
||
| const dateGte = | ||
| type === 'movie' ? 'primaryReleaseDateGte' : 'firstAirDateGte'; | ||
| const dateLte = |
There was a problem hiding this comment.
Not sure what would be the right place to change the primaryReleaseDate to releaseDate. Here or in the function that does the TMDB API call.
| inputClassName="pr-1 sm:pr-4 text-base leading-5" | ||
| /> | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
I would suggest adding a warning message under the date picker here if the type is a movie, a minimumReleaseType other than Any has been selected and both the to and from dates are empty. That warning message should indicate that this will cause the selected minimum release to have no effect.
| sortBy: z.string().optional(), | ||
| primaryReleaseDateGte: z.string().optional(), | ||
| primaryReleaseDateLte: z.string().optional(), | ||
| releaseType: z.enum(['2', '3', '4', '5', '6']).optional(), |
There was a problem hiding this comment.
| releaseType: z.enum(['2', '3', '4', '5', '6']).optional(), | |
| minimumReleaseType: z.enum(['2', '3', '4', '5', '6']).optional(), |
| if (values.releaseType) { | ||
| filterValues.releaseType = values.releaseType; | ||
| } |
There was a problem hiding this comment.
| if (values.releaseType) { | |
| filterValues.releaseType = values.releaseType; | |
| } | |
| if (values.minimumReleaseType) { | |
| filterValues.minimumReleaseType = values.minimumReleaseType; | |
| } |
|
It'd probably be great if this can also have some testing. There is already a discover cypress suite where you might be able to add a test case or two (at least validating that selecting a certain value leads to the correct TMDB query being made) |
2 participants
Description
Adds a "Minimum Release" filter to the movie discover filters (part of #579). You pick a release stage from the dropdown and it shows movies that have reached at least that stage. Picking Digital also covers Physical and TV, since those are all effectively "available at home."
The fiddly bit is that TMDB's
with_release_typedoes nothing on its own. It only filters when it's paired with arelease_daterange, and it works offrelease_daterather than theprimary_release_datefield the existing Release Date filter uses. So when a Minimum Release is selected, the query switches its date range over torelease_dateand defaults the "To" date to today if you haven't set one. With nothing selected, the query is unchanged and the normal Release Date filter behaves exactly as before.Keeping this to movie discover for a first pass. Related to #579 but not closing it, since that issue also covers things like release-status badges on movie pages.
AI disclosure: I originally wrote the first version of this filter myself. For this iteration I had some AI assistance to help rework it, mainly the cumulative release-type logic and to code-review the TMDB query behavior and the diff. The review caught a timezone bug I missed and an unrelated translation change, both fixed. I've reviewed and tested all the changes myself and understand how they work.
How Has This Been Tested?
Ran it locally against a Jellyfin instance:
with_release_type=4|5|6+release_date.lte=<today>) returns ~314k vs ~1.16M unfiltered, and narrows as the floor goes up (theatrical ~1.05M, TV ~105k). No region needed.primary_release_dateand behaves the same.tsc(client + server) andeslintpass.Screenshots / Logs (if applicable)
Checklist:
pnpm buildpnpm i18n:extractSummary by CodeRabbit