feat(discover): add setting to hide requested media from discover pages#3264
Closed
andersennl wants to merge 2 commits into
Closed
feat(discover): add setting to hide requested media from discover pages#3264andersennl wants to merge 2 commits into
andersennl wants to merge 2 commits into
Conversation
Adds a hideRequested setting (default off). When enabled, movies and series with at least one pending or approved request - including partially requested series and 4K requests - are hidden from discover pages, sliders, and recommended/similar listings. Search results are not affected. Unlike the upstream proposal, no request objects are loaded or serialized: when the setting is enabled, getRelatedMedia runs a single grouped query per batch selecting only the IDs of media with active requests and exposes a transient mediaInfo.hasActiveRequest boolean. With the setting disabled no extra query is executed. Client-side visibility filtering is centralized in a shared helper so person and collection results are never dropped, and useDiscover now fetches a bounded number of extra pages when client-side filters empty out entire pages.
Contributor
|
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 a disabled-by-default “Hide Requested Media” setting, exposes active-request metadata, and filters requested movies and series from discover-related views while retaining them in search results. ChangesHide Requested Media
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant SettingsMain
participant SettingsAPI
participant SettingsContext
participant useDiscover
participant filterVisibleTitles
SettingsMain->>SettingsAPI: save hideRequested
SettingsAPI-->>SettingsContext: return public settings
SettingsContext->>useDiscover: provide current settings
useDiscover->>filterVisibleTitles: filter discovered titles
filterVisibleTitles-->>useDiscover: return visible titles
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/hooks/useDiscover.ts (1)
135-156: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winBackfill window can briefly show an empty state, and effect deps are ineffective.
Two related nits in this block:
isEmpty(Lines 135-136) doesn't account for the pending backfill in progress: right after a page finishes loading but before theuseEffectbelow bumpssize,titlescan be thin enough thatisEmptybecomestruefor a render, momentarily flashing a "no results" state — the exact scenario this backfill logic is meant to avoid. Consider folding the backfill condition intoisEmpty/isLoadingMore(e.g., treat "would trigger a backfill fetch" as still-loading).- The
useEffectdependency array includestitles(Line 156), buttitlesis a new array reference every render (from.reduce/.filter), so it provides no real change-detection value — the effect re-evaluates on every render regardless. Prefertitles.lengthto make the intent explicit and avoid redundant re-invocation.💡 Possible adjustment
- const isEmpty = - !isLoadingInitialData && !isLoadingMore && titles?.length === 0; + const willBackfill = + !!data && titles.length < 20 && size < 5 && !isReachingEnd; + const isEmpty = + !isLoadingInitialData && + !isLoadingMore && + !willBackfill && + titles?.length === 0; const isReachingEnd = (!!data && (data[data.length - 1]?.results.length ?? 0) < 20) || (!!data && (data[data.length - 1]?.totalResults ?? 0) <= size * 20) || (!!data && (data[data.length - 1]?.totalResults ?? 0) < 41); useEffect(() => { - if ( - !!data && - titles.length < 20 && - size < 5 && - !isReachingEnd && - !isLoadingMore && - !isValidating - ) { + if (willBackfill && !isLoadingMore && !isValidating) { setSize(size + 1); } - }, [data, titles, size, isReachingEnd, isLoadingMore, isValidating, setSize]); + }, [willBackfill, isLoadingMore, isValidating, size, setSize]);🤖 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/hooks/useDiscover.ts` around lines 135 - 156, Update the empty-state and backfill logic around isEmpty and the useEffect: treat a pending backfill fetch as loading so isEmpty cannot briefly become true before setSize runs, while preserving normal empty-state behavior. Replace the useEffect dependency on the derived titles array with titles.length, and update references as needed so the dependency list tracks the value that controls backfill decisions.
🤖 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.
Nitpick comments:
In `@src/hooks/useDiscover.ts`:
- Around line 135-156: Update the empty-state and backfill logic around isEmpty
and the useEffect: treat a pending backfill fetch as loading so isEmpty cannot
briefly become true before setSize runs, while preserving normal empty-state
behavior. Replace the useEffect dependency on the derived titles array with
titles.length, and update references as needed so the dependency list tracks the
value that controls backfill decisions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 11de00f4-f1fa-4d67-9dbe-63bf212d7676
📒 Files selected for processing (14)
docs/using-seerr/settings/general.mdseerr-api.ymlserver/entity/Media.tsserver/interfaces/api/settingsInterfaces.tsserver/lib/settings/index.tsserver/test/entity/Media.test.tssrc/components/MediaSlider/index.tsxsrc/components/Search/index.tsxsrc/components/Settings/SettingsMain/index.tsxsrc/context/SettingsContext.tsxsrc/hooks/useDiscover.tssrc/i18n/locale/en.jsonsrc/pages/_app.tsxsrc/utils/mediaFilters.ts
Fold the backfill condition into isEmpty so a fully filtered page does not briefly render the no-results state before the bounded refetch kicks in, and depend on titles.length instead of the per-render titles array reference in the effect.
7 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
This PR adds a new "Hide Requested Media" setting (default: off) to Settings → General. When enabled, movies and series with at least one pending or approved request are hidden from the Discover home page, sliders, and the Recommended/Similar sections on detail pages. This includes partially requested series (e.g. only some seasons requested) and 4K-only requests. Search results are intentionally unaffected, so hidden items can still be found by title.
It is an alternative to #1855 that implements the review feedback given there. Credit to the original author for the settings plumbing, which this PR reuses largely unchanged.
Semantics
PENDINGorAPPROVEDrequest (any season, standard or 4K)DECLINED,FAILED, orCOMPLETEDrequestsFAILEDstays visible on purpose so failed requests remain discoverable and can be retried.PARTIALLY_AVAILABLEseries are hidden only while a season request is actually active —hasActiveRequestis the source of truth, not the media status.Implementation notes
Unlike #1855, no request entities are loaded or serialized (see the review concern about
leftJoinAndSelect('media.requests'), which would leak request metadata to every user, multiply rows against the existing watchlist join, and bloat the payload):Media.getRelatedMedia()runs one additional grouped query per batch that selects only the IDs of media with active requests, and exposes a transientmediaInfo.hasActiveRequest: boolean. No request, requester, or season data leaves the server.Client-side changes:
src/utils/mediaFilters.ts), used by bothuseDiscoverandMediaSlider. This also fixes a pre-existing bug where thehideAvailable/hideBlocklistedfilters dropped person and collection results from mixed sliders.useDiscovernow fetches a bounded number of extra pages (max 5, mirroring the existingMediaSliderbehavior) when client-side filters empty out entire pages, so lists no longer appear exhausted while the server still has results. TheisEmptyshortcut was removed fromisReachingEndfor the same reason.How Has This Been Tested?
server/test/entity/Media.test.ts(node:test with the real SQLite test DB) covering: pending/approved → flagged; declined/failed/completed → not flagged; 4K-only requests; partially available series with and without an active season request; batch behavior; no extra computation when the setting is disabled; and that the serialized result contains norequests/requestedBy/modifiedBydata.server/test/entity/rather than next to the entity because the TypeORM configs globserver/entity/**/*.tsas entity classes (and the server build shipsdist/entity/**/*.js), so a colocated test file would be loaded as an entity.pnpm lint,pnpm typecheck,pnpm test(131/131), andpnpm buildall pass on this branch.Checklist:
pnpm buildpnpm i18n:extractSummary by CodeRabbit
New Features
Documentation