feat(discover): added availability filter to discover pages#2935
Open
BaLion29 wants to merge 4 commits into
Open
feat(discover): added availability filter to discover pages#2935BaLion29 wants to merge 4 commits into
BaLion29 wants to merge 4 commits into
Conversation
Contributor
📝 WalkthroughWalkthroughAdds a new availability filter to Discover: server enum, UI selector, query param wiring, schema support, and post-fetch availability-based filtering in the discover hook. Changes
Sequence DiagramsequenceDiagram
participant UI as FilterSlideover UI
participant Sel as AvailabilitySelector
participant QP as Query Params
participant Hook as useDiscover Hook
participant API as TMDB API
participant Post as Availability Post-Filter
UI->>Sel: Render with current availability value
Note over Sel: User selects an option
Sel->>UI: onChange(selected filter value)
UI->>QP: updateQueryParams(availability or clear)
QP->>Hook: Trigger fetch (availability removed from TMDB params)
Hook->>API: Fetch TMDB results (tmdbOptions)
API-->>Hook: Return titles
Hook->>Post: Apply availability-based filtering
Post-->>Hook: Return filtered titles
Hook-->>UI: Render filtered discover items
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/hooks/useDiscover.ts (1)
127-165:⚠️ Potential issue | 🟠 Major
hideAvailableshort-circuits theAVAILABLE_OR_REQUESTEDavailability filter.When
settings.currentSettings.hideAvailableis enabled (the default),AVAILABLEandPARTIALLY_AVAILABLEitems are stripped on lines 127-134 before your new availability filter runs. If the user then explicitly selects "Show Available and Requested only", lines 148-156 will never see available items and can only returnPROCESSING/PENDING, which contradicts the user's explicit choice.Skip the implicit
hideAvailablepass when the user has set an explicit availability filter.🐛 Proposed fix
- if (settings.currentSettings.hideAvailable && hideAvailable) { + if ( + settings.currentSettings.hideAvailable && + hideAvailable && + availability === DiscoverAvailabilityFilter.ALL + ) { titles = titles.filter( (i) => (i.mediaType === 'movie' || i.mediaType === 'tv') && i.mediaInfo?.status !== MediaStatus.AVAILABLE && i.mediaInfo?.status !== MediaStatus.PARTIALLY_AVAILABLE ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useDiscover.ts` around lines 127 - 165, The implicit hideAvailable filtering (the block that checks settings.currentSettings.hideAvailable && hideAvailable) is removing AVAILABLE/PARTIALLY_AVAILABLE items before the explicit availability filter runs; change the logic so that the hideAvailable pass is skipped when the user has chosen an explicit availability filter (i.e., when availability is set to something other than the "all"/default value). Concretely, in useDiscover.ts adjust the condition that runs the hideAvailable filter to also require availability to be the default/all option (so keep the current checks for settings.currentSettings.hideAvailable and hideAvailable, but add a check like availability === DiscoverAvailabilityFilter.ALL or availability == null), leaving the existing availability-based branches (DiscoverAvailabilityFilter.AVAILABLE_OR_REQUESTED and NOT_AVAILABLE_OR_REQUESTED) untouched.
🧹 Nitpick comments (2)
src/components/Discover/constants.ts (1)
118-118: Tighten validation to the enum and filter outALLto avoid phantom active filter count.Two related refinements:
z.string().optional()accepts any string (e.g.?availability=foo). Usez.nativeEnum(DiscoverAvailabilityFilter)so invalid values are stripped on parse rather than flowing intofilterValuesand being counted bycountActiveFilters.- When a user lands with
?availability=all(e.g. shared URL),prepareFilterValuescurrently stores it. It is then counted as an active filter incountActiveFilters(line 279) even thoughFilterSlideoverintentionally clears the param whenALLis selected. TreatALLas "no filter" on ingest.♻️ Proposed refactor
+import { DiscoverAvailabilityFilter } from '@server/constants/discover'; ... - availability: z.string().optional(), + availability: z.nativeEnum(DiscoverAvailabilityFilter).optional(),- if (values.availability) { + if (values.availability && values.availability !== DiscoverAvailabilityFilter.ALL) { filterValues.availability = values.availability; }Also applies to: 230-232
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Discover/constants.ts` at line 118, Replace the loose availability schema and ingestion logic so invalid or "ALL" values don't become active filters: change the Zod schema for availability from z.string().optional() to z.nativeEnum(DiscoverAvailabilityFilter).optional() so only enum members survive parsing, and update prepareFilterValues to treat DiscoverAvailabilityFilter.ALL as equivalent to undefined (i.e., strip it out) before returning/storing filterValues; this prevents countActiveFilters from counting phantom/ALL values while leaving other enum handling and FilterSlideover behavior intact.src/hooks/useDiscover.ts (1)
67-68: Avoidas any; narrow the options type instead.Casting
options as anysilently discards type safety for every caller ofuseDiscover. Since all known call sites pass objects that are a superset ofFilterOptions(seeDiscoverMovies/DiscoverTv), constrainOand destructure without the cast.♻️ Proposed refactor
const useDiscover = < T extends BaseMedia, S = Record<string, never>, - O = Record<string, unknown>, + O extends { availability?: DiscoverAvailabilityFilter | string } = Record< + string, + unknown + >, >( endpoint: string, options?: O, { hideAvailable = true, hideBlocklisted = true } = {} ): DiscoverResult<T, S> => { ... - const { availability = DiscoverAvailabilityFilter.ALL, ...tmdbOptions } = - options as any; + const { availability = DiscoverAvailabilityFilter.ALL, ...tmdbOptions } = + options ?? ({} as O);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useDiscover.ts` around lines 67 - 68, The destructure is using "options as any" which removes type safety; update the generic/parameter so O extends FilterOptions (or the appropriate FilterOptions union used by DiscoverMovies/DiscoverTv) and remove the cast, i.e. ensure useDiscover's signature constrains O to FilterOptions and then destructure directly: const { availability = DiscoverAvailabilityFilter.ALL, ...tmdbOptions } = options; also update any related overloads or call sites if needed to satisfy the narrower generic (referencing useDiscover, the O generic, FilterOptions, DiscoverAvailabilityFilter, and callers DiscoverMovies/DiscoverTv).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/Discover/FilterSlideover/index.tsx`:
- Line 50: Fix the typo in the user-facing label by changing the value for the
availability key from "Availabilty" to "Availability" in the FilterSlideover
labels object (the availability entry in
src/components/Discover/FilterSlideover/index.tsx), and update the matching key
in the i18n dictionary (the availability entry in en.json) so both the component
label and the locale string read "Availability".
---
Outside diff comments:
In `@src/hooks/useDiscover.ts`:
- Around line 127-165: The implicit hideAvailable filtering (the block that
checks settings.currentSettings.hideAvailable && hideAvailable) is removing
AVAILABLE/PARTIALLY_AVAILABLE items before the explicit availability filter
runs; change the logic so that the hideAvailable pass is skipped when the user
has chosen an explicit availability filter (i.e., when availability is set to
something other than the "all"/default value). Concretely, in useDiscover.ts
adjust the condition that runs the hideAvailable filter to also require
availability to be the default/all option (so keep the current checks for
settings.currentSettings.hideAvailable and hideAvailable, but add a check like
availability === DiscoverAvailabilityFilter.ALL or availability == null),
leaving the existing availability-based branches
(DiscoverAvailabilityFilter.AVAILABLE_OR_REQUESTED and
NOT_AVAILABLE_OR_REQUESTED) untouched.
---
Nitpick comments:
In `@src/components/Discover/constants.ts`:
- Line 118: Replace the loose availability schema and ingestion logic so invalid
or "ALL" values don't become active filters: change the Zod schema for
availability from z.string().optional() to
z.nativeEnum(DiscoverAvailabilityFilter).optional() so only enum members survive
parsing, and update prepareFilterValues to treat DiscoverAvailabilityFilter.ALL
as equivalent to undefined (i.e., strip it out) before returning/storing
filterValues; this prevents countActiveFilters from counting phantom/ALL values
while leaving other enum handling and FilterSlideover behavior intact.
In `@src/hooks/useDiscover.ts`:
- Around line 67-68: The destructure is using "options as any" which removes
type safety; update the generic/parameter so O extends FilterOptions (or the
appropriate FilterOptions union used by DiscoverMovies/DiscoverTv) and remove
the cast, i.e. ensure useDiscover's signature constrains O to FilterOptions and
then destructure directly: const { availability =
DiscoverAvailabilityFilter.ALL, ...tmdbOptions } = options; also update any
related overloads or call sites if needed to satisfy the narrower generic
(referencing useDiscover, the O generic, FilterOptions,
DiscoverAvailabilityFilter, and callers DiscoverMovies/DiscoverTv).
🪄 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
Run ID: 92a1a1f6-6926-4f4d-b169-a5941aebca6e
📒 Files selected for processing (7)
server/constants/discover.tssrc/components/Discover/FilterSlideover/index.tsxsrc/components/Discover/constants.tssrc/components/Selector/AvailabilitySelector.tsxsrc/components/Selector/index.tsxsrc/hooks/useDiscover.tssrc/i18n/locale/en.json
| voteCount: 'Number of votes between {minValue} and {maxValue}', | ||
| status: 'Status', | ||
| certification: 'Content Rating', | ||
| availability: 'Availabilty', |
Contributor
There was a problem hiding this comment.
Typo in user-facing label: "Availabilty" → "Availability".
This label renders as the section heading in the filters slideover. The corresponding key value in src/i18n/locale/en.json (line 80) has the same typo and should be fixed together.
✏️ Proposed fix
- availability: 'Availabilty',
+ availability: 'Availability',📝 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.
Suggested change
| availability: 'Availabilty', | |
| availability: 'Availability', |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/Discover/FilterSlideover/index.tsx` at line 50, Fix the typo
in the user-facing label by changing the value for the availability key from
"Availabilty" to "Availability" in the FilterSlideover labels object (the
availability entry in src/components/Discover/FilterSlideover/index.tsx), and
update the matching key in the i18n dictionary (the availability entry in
en.json) so both the component label and the locale string read "Availability".
fixed typo in FilterSlideover messages, from 'Availabilty' to 'Availability'. no influence on rest of code, just UI
…ity filters won't clash changed hideAvailable filter behavior to only catch if availability is set to ALL, so that "Show Available and Requested only" and "Hide Available and Requested" still work when hideAvailable is set to true
2 tasks
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
Adds a per-user availability filter on the /discover/movies and /discover/tv pages, allowing users to filter content by availability status directly from the filter slideover.
Why?
Currently, the only availability-related control is a global admin toggle (General Settings → Hide Available Media), which applies server-wide and is not designed as a per-session, per-user control.
As a result, regular users cannot filter discovery pages by availability status. In some cases, however, it may be desirable for a user to filter media based on its availability, e.g. when a large portion of the most popular titles is already available and hinders the discovery of new content.
How?
Known Limitations
Related Issues
How Has This Been Tested?
Tested locally against a Jellyfin instance with a mixed library (available movies, partially available series, pending requests...)
Screenshots / Logs
Checklist:
Summary by CodeRabbit
New Features
Documentation