feat: Add option to exclude providers from request options#2533
Open
jensdufour wants to merge 17 commits intoseerr-team:developfrom
Open
feat: Add option to exclude providers from request options#2533jensdufour wants to merge 17 commits intoseerr-team:developfrom
jensdufour wants to merge 17 commits intoseerr-team:developfrom
Conversation
Co-authored-by: jensdufour <25767022+jensdufour@users.noreply.github.com>
Co-authored-by: jensdufour <25767022+jensdufour@users.noreply.github.com>
…r request gate Co-authored-by: jensdufour <25767022+jensdufour@users.noreply.github.com>
…r loops Co-authored-by: jensdufour <25767022+jensdufour@users.noreply.github.com>
… and sync API spec Co-authored-by: jensdufour <25767022+jensdufour@users.noreply.github.com>
…n for excluded providers Co-authored-by: jensdufour <25767022+jensdufour@users.noreply.github.com>
…rch pages Co-authored-by: jensdufour <25767022+jensdufour@users.noreply.github.com>
… button click Co-authored-by: jensdufour <25767022+jensdufour@users.noreply.github.com>
Co-authored-by: jensdufour <25767022+jensdufour@users.noreply.github.com>
…luded-providers Add excluded streaming providers: dim posters, provider link button, per-user settings
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds an excludedWatchProviders string setting across API schema, DB, backend services, migrations, and frontend UI; persists pipe-separated provider IDs per user and surfaces the value to media/detail components to enable exclusion-aware UI and provider-specific watch links. Changes
Sequence DiagramsequenceDiagram
actor User
participant SettingsUI as Settings UI
participant Backend as Backend API
participant DB as Database
participant MediaUI as Media Detail UI
participant ProviderAPI as External Provider Data
User->>SettingsUI: Select excluded providers
SettingsUI->>SettingsUI: Serialize to pipe-delimited string
SettingsUI->>Backend: POST /main with excludedWatchProviders
Backend->>DB: Persist excludedWatchProviders
DB-->>Backend: Ack
Backend-->>SettingsUI: Return updated settings
Note over User,MediaUI: Later when viewing media
User->>MediaUI: Open media detail
MediaUI->>Backend: GET user settings
Backend->>DB: Read excludedWatchProviders
DB-->>Backend: Return value
Backend-->>MediaUI: Settings with excludedWatchProviders
MediaUI->>ProviderAPI: Fetch provider details for item
ProviderAPI-->>MediaUI: Provider data
MediaUI->>MediaUI: Match excluded provider, compute watch link
MediaUI->>User: Render dimmed poster / overlay / provider watch button
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Poem
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Comment |
jensdufour
changed the title
![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)
src/components/TitleCard/index.tsx (1)
569-618:⚠️ Potential issue | 🟡 MinorSame fallback issue when
watchProviderLinkis unavailable.Consistent with MovieDetails and TvDetails, when
isAvailableOnExcludedProvideris true butwatchProviderLinkis undefined, no button renders. The fix should be applied here as well to fall back to the request button.Proposed fix
{showRequestButton && (!currentStatus || currentStatus === MediaStatus.UNKNOWN || - currentStatus === MediaStatus.DELETED) && - (isAvailableOnExcludedProvider && - firstExcludedProvider && - watchProviderLink ? ( + currentStatus === MediaStatus.DELETED) ? ( + isAvailableOnExcludedProvider && + firstExcludedProvider && + watchProviderLink ? ( <Tooltip ... > ... </Tooltip> ) : ( <Button ... > ... </Button> - ))} + ) + ) : null}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/TitleCard/index.tsx` around lines 569 - 618, The conditional rendering block for the provider/request button skips rendering the request button when isAvailableOnExcludedProvider is true but watchProviderLink is falsy; update the logic in the JSX that uses showRequestButton, currentStatus/MediaStatus checks and the isAvailableOnExcludedProvider && firstExcludedProvider && watchProviderLink condition so that when isAvailableOnExcludedProvider && firstExcludedProvider is true but watchProviderLink is undefined it falls back to rendering the request Button (the branch that calls setShowRequestModal) instead of rendering nothing; keep the existing Tooltip/Button for the external link when watchProviderLink is present and preserve onClick handlers and props.
🧹 Nitpick comments (1)
src/components/TitleCard/index.tsx (1)
104-124: Watch provider filtering fetches are reasonable given the constraints, but consider optimization strategies for scale.The implementation correctly gates API calls behind the
shouldFetchDetailsguard, so no extra overhead when the feature is unused. SWR's URL-based caching prevents duplicate fetches for the same movie ID.However, with excluded providers configured, a page loading 40+ cards will trigger that many individual API requests initially. While SWR mitigates repeated fetches, the first-page load cost remains notable.
Worth revisiting if this becomes a bottleneck in production, but current optimizations (lazy-fetch, guard, caching) are sound. If needed later, consider:
- Backend enrichment: Include a simplified provider matching flag in the search/discover API response
- Lazy loading: Defer details fetching until hover or card visibility (IntersectionObserver)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/TitleCard/index.tsx` around lines 104 - 124, Large-scale page loads can trigger many per-card detail fetches when excluded providers are set; keep the existing guard (shouldFetchDetails) and SWR usage but mitigate initial-load bursts by moving provider checks off the client: add a server-side flag or minimal field to the primary search/discover response (e.g., include a boolean like hasExcludedProvider or an array of provider IDs) so TitleCard no longer needs to compute streamingProviders for every card; update the TitleCard logic (where shouldFetchDetails, detailsUrl, useSWR, streamingProviders, and excludedWatchProviderIds are referenced) to prefer the enriched search result field and only fall back to fetching details via useSWR when that server-side flag is absent or inconclusive.
🤖 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/MovieDetails/index.tsx`:
- Around line 651-686: When isAvailableOnExcludedProvider is true but
watchProviderLink is undefined the UI renders nothing; update the conditional in
MovieDetails (around RequestButton / Tooltip/Button rendering) to fall back to
rendering the RequestButton (same props: mediaType="movie",
media={data.mediaInfo}, tmdbId={data.id}, onUpdate={() => revalidate()}) when
watchProviderLink is missing—i.e., render the Tooltip/Button only when
watchProviderLink exists, otherwise render RequestButton, referencing
RequestButton, isAvailableOnExcludedProvider, firstExcludedProvider, and
watchProviderLink to locate the change.
In `@src/components/Settings/SettingsMain/index.tsx`:
- Around line 425-441: WatchProviderSelector is only reading its region prop at
mount, so when values.streamingRegion changes while hideRegionSelector is true
the internal watchRegion stays stale; to fix, force a remount by adding a key
tied to the region (e.g., key={values.streamingRegion}) on the
<WatchProviderSelector> element so React recreates WatchProviderSelector when
values.streamingRegion changes, ensuring it re-initializes its internal
watchRegion from the updated region prop; leave the existing props (type,
region, hideRegionSelector, activeProviders, onChange) unchanged.
In `@src/components/TvDetails/index.tsx`:
- Around line 693-730: If isAvailableOnExcludedProvider is true but
watchProviderLink is undefined, render the RequestButton as a fallback so users
always have an action; update the conditional that currently requires both
isAvailableOnExcludedProvider and watchProviderLink (and firstExcludedProvider)
to instead branch: if watchProviderLink and firstExcludedProvider render the
Tooltip/Button block (as now), otherwise render the existing RequestButton with
the same props (mediaType="tv", onUpdate={() => revalidate()},
tmdbId={data?.id}, media={data?.mediaInfo}, isShowComplete={isComplete},
is4kShowComplete={is4kComplete}); ensure you reference RequestButton,
isAvailableOnExcludedProvider, watchProviderLink, and firstExcludedProvider in
the updated logic so the fallback displays when the TMDB link is missing.
---
Outside diff comments:
In `@src/components/TitleCard/index.tsx`:
- Around line 569-618: The conditional rendering block for the provider/request
button skips rendering the request button when isAvailableOnExcludedProvider is
true but watchProviderLink is falsy; update the logic in the JSX that uses
showRequestButton, currentStatus/MediaStatus checks and the
isAvailableOnExcludedProvider && firstExcludedProvider && watchProviderLink
condition so that when isAvailableOnExcludedProvider && firstExcludedProvider is
true but watchProviderLink is undefined it falls back to rendering the request
Button (the branch that calls setShowRequestModal) instead of rendering nothing;
keep the existing Tooltip/Button for the external link when watchProviderLink is
present and preserve onClick handlers and props.
---
Duplicate comments:
In `@src/components/UserProfile/UserSettings/UserGeneralSettings/index.tsx`:
- Around line 480-498: The WatchProviderSelector here can get stuck on the old
region when values.streamingRegion changes; to fix it either force a remount by
adding a key tied to the resolved region (e.g., key={`${values.streamingRegion
|| currentSettings.streamingRegion || 'US'}`) on the WatchProviderSelector, or
update WatchProviderSelector to respond to prop changes for region (implement
componentDidUpdate/useEffect to sync internal state when the region prop
changes). Update references around WatchProviderSelector, region prop, and
onChange/setFieldValue to ensure the selector resets when values.streamingRegion
or currentSettings.streamingRegion changes.
---
Nitpick comments:
In `@src/components/TitleCard/index.tsx`:
- Around line 104-124: Large-scale page loads can trigger many per-card detail
fetches when excluded providers are set; keep the existing guard
(shouldFetchDetails) and SWR usage but mitigate initial-load bursts by moving
provider checks off the client: add a server-side flag or minimal field to the
primary search/discover response (e.g., include a boolean like
hasExcludedProvider or an array of provider IDs) so TitleCard no longer needs to
compute streamingProviders for every card; update the TitleCard logic (where
shouldFetchDetails, detailsUrl, useSWR, streamingProviders, and
excludedWatchProviderIds are referenced) to prefer the enriched search result
field and only fall back to fetching details via useSWR when that server-side
flag is absent or inconclusive.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/components/TvDetails/index.tsx (1)
1349-1375:⚠️ Potential issue | 🟡 MinorMove
keyprop to the outermost element in the map callback.The
keyprop is on the inner<span>(line 1359) instead of the<Tooltip>wrapper. React requires the key on the outermost element returned from a map callback for proper reconciliation.🔧 Proposed fix
{streamingProviders.map((p) => { const isExcluded = excludedWatchProviders.includes(p.id); return ( - <Tooltip content={p.name}> + <Tooltip content={p.name} key={`provider-${p.id}`}> <span className={`relative transition duration-300 ${ isExcluded ? 'opacity-100' : 'opacity-50 hover:opacity-100' }`} - key={`provider-${p.id}`} >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/TvDetails/index.tsx` around lines 1349 - 1375, The mapped list puts the key on the inner <span> instead of the outermost element returned from streamingProviders.map; move the key prop from the inner span (key={`provider-${p.id}`}) to the <Tooltip> wrapper so the Tooltip component is the root element for each iteration (keep the same key value), ensuring proper reconciliation for the map that renders Tooltip -> span -> CachedImage.src/components/MovieDetails/index.tsx (1)
1129-1155:⚠️ Potential issue | 🟡 MinorMove
keyprop to the outermost element in the map callback.Same issue as in TvDetails: the
keyprop is on the inner<span>instead of the<Tooltip>wrapper.🔧 Proposed fix
{streamingProviders.map((p) => { const isExcluded = excludedWatchProviders.includes(p.id); return ( - <Tooltip content={p.name}> + <Tooltip content={p.name} key={`provider-${p.id}`}> <span className={`relative transition duration-300 ${ isExcluded ? 'opacity-100' : 'opacity-50 hover:opacity-100' }`} - key={`provider-${p.id}`} >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/MovieDetails/index.tsx` around lines 1129 - 1155, The map callback over streamingProviders places the React key on the inner <span> instead of the outer <Tooltip>; move the key prop from the inner element to the outer Tooltip component in the streamingProviders.map callback (remove it from the inner span) so the Tooltip is the keyed root for each mapped item.
🧹 Nitpick comments (1)
src/components/MovieDetails/index.tsx (1)
298-316: Consider extracting shared exclusion logic.The exclusion computation (lines 298-316) is duplicated verbatim in TvDetails. Consider extracting this to a shared utility or custom hook to reduce duplication and ensure consistent behavior across components.
♻️ Example extraction
// hooks/useExcludedProviders.ts export function useExcludedProviders( watchProviders: WatchProviderRegion[] | undefined, streamingRegion: string, userExcluded: string | undefined, globalExcluded: string | undefined ) { const streamingProviderData = watchProviders?.find( (provider) => provider.iso_3166_1 === streamingRegion ); const streamingProviders = streamingProviderData?.flatrate ?? []; const watchProviderLink = streamingProviderData?.link; const excludedWatchProviders = (userExcluded ?? globalExcluded ?? '') .split('|') .filter(Boolean) .map(Number); const firstExcludedProvider = streamingProviders.find((p) => excludedWatchProviders.includes(p.id) ); const isAvailableOnExcludedProvider = excludedWatchProviders.length > 0 && streamingProviders.some((p) => excludedWatchProviders.includes(p.id)); return { streamingProviders, watchProviderLink, excludedWatchProviders, firstExcludedProvider, isAvailableOnExcludedProvider, }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/MovieDetails/index.tsx` around lines 298 - 316, Extract the duplicated exclusion logic (the block computing streamingProviderData, streamingProviders, watchProviderLink, excludedWatchProviders, firstExcludedProvider, isAvailableOnExcludedProvider) into a shared helper or custom hook (e.g., useExcludedProviders) and replace the inline code in MovieDetails (index.tsx) and TvDetails with a call to that helper; ensure the helper accepts watchProviders, streamingRegion, user?.settings?.excludedWatchProviders and settings.currentSettings.excludedWatchProviders (or their string values) and returns the same named values so existing references to streamingProviders, watchProviderLink, excludedWatchProviders, firstExcludedProvider, and isAvailableOnExcludedProvider keep working.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/components/MovieDetails/index.tsx`:
- Around line 1129-1155: The map callback over streamingProviders places the
React key on the inner <span> instead of the outer <Tooltip>; move the key prop
from the inner element to the outer Tooltip component in the
streamingProviders.map callback (remove it from the inner span) so the Tooltip
is the keyed root for each mapped item.
In `@src/components/TvDetails/index.tsx`:
- Around line 1349-1375: The mapped list puts the key on the inner <span>
instead of the outermost element returned from streamingProviders.map; move the
key prop from the inner span (key={`provider-${p.id}`}) to the <Tooltip> wrapper
so the Tooltip component is the root element for each iteration (keep the same
key value), ensuring proper reconciliation for the map that renders Tooltip ->
span -> CachedImage.
---
Duplicate comments:
In `@src/components/Settings/SettingsMain/index.tsx`:
- Around line 411-446: The component correctly forces a remount when the
streaming region changes by adding key={values.streamingRegion} to
WatchProviderSelector; ensure this key remains on the WatchProviderSelector
component (alongside props type="movie", region={values.streamingRegion},
hideRegionSelector, activeProviders derived from
values.excludedWatchProviders.split('|').map(Number), and the onChange that sets
'excludedWatchProviders' via setFieldValue) so the selector fully resets when
streamingRegion updates.
---
Nitpick comments:
In `@src/components/MovieDetails/index.tsx`:
- Around line 298-316: Extract the duplicated exclusion logic (the block
computing streamingProviderData, streamingProviders, watchProviderLink,
excludedWatchProviders, firstExcludedProvider, isAvailableOnExcludedProvider)
into a shared helper or custom hook (e.g., useExcludedProviders) and replace the
inline code in MovieDetails (index.tsx) and TvDetails with a call to that
helper; ensure the helper accepts watchProviders, streamingRegion,
user?.settings?.excludedWatchProviders and
settings.currentSettings.excludedWatchProviders (or their string values) and
returns the same named values so existing references to streamingProviders,
watchProviderLink, excludedWatchProviders, firstExcludedProvider, and
isAvailableOnExcludedProvider keep working.
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
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
Allow users and admins to configure a list of excluded streaming providers.
When content is available on one of these providers, the title card and detail pages visually indicate it (dimmed poster) and replace the request button with a direct link to the provider's page on TMDB, using the streaming region configured in general settings.
The excluded providers list can be set globally (server settings) and overridden per-user (user/admin general settings).
Closes #206
How Has This Been Tested?
pnpm buildon Node 22Screenshots / Logs (if applicable)
Checklist:
pnpm buildpnpm i18n:extractSummary by CodeRabbit
New Features
UI / Behavior