feat(settings): hide already requested media#1855
Conversation
|
This pull request has merge conflicts. Please resolve the conflicts so the PR can be successfully reviewed and merged. |
6510445 to
4974f31
Compare
|
This pull request has merge conflicts. Please resolve the conflicts so the PR can be successfully reviewed and merged. |
|
@0xSysR3ll would you be willing to resolve the merge conflicts so it can be merged ? :) Thanks a lot for your contribution ! |
I could, but since we're currently in feature freeze, it's not relevant at the moment. |
4974f31 to
ed467b7
Compare
|
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:
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 (3)
📝 WalkthroughWalkthroughAdds the ChangesHide requested media setting
Estimated code review effort: 3 (Moderate) | ~22 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant SettingsUI
participant SettingsAPI
participant Discover
participant MediaGetRelatedMedia
participant Filter
User->>SettingsUI: Enable hideRequested
SettingsUI->>SettingsAPI: Submit hideRequested=true
SettingsAPI-->>SettingsUI: Return updated settings
Discover->>MediaGetRelatedMedia: Fetch related media
MediaGetRelatedMedia-->>Discover: Return hasActiveRequest state
Discover->>Filter: Remove requested movie and TV titles
Filter-->>Discover: Return filtered 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}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/hooks/useDiscover.ts (1)
16-19: Consider improving type precision forrequests.The
requestsfield is typed asunknown[], but based on the server entity (server/entity/Media.ts), this is actuallyMediaRequest[]. Whileunknown[]works since only.lengthis accessed, a more precise type would improve maintainability if future code needs to access request properties.🔧 Suggested type refinement
interface BaseMedia { id: number; mediaType: string; mediaInfo?: { status: MediaStatus; - requests?: unknown[]; + requests?: { length: number }[]; }; }Alternatively, if
MediaRequestis importable from@server/entity/Media, a full type could be used.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useDiscover.ts` around lines 16 - 19, The mediaInfo.requests property is typed too loosely as unknown[]; change its type to MediaRequest[] (importing MediaRequest from `@server/entity/Media`) so the mediaInfo definition in useDiscover.ts becomes mediaInfo?: { status: MediaStatus; requests?: MediaRequest[]; } to improve type safety and future property access.
🤖 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/MediaSlider/index.tsx`:
- Around line 85-94: The filter in titles.filter incorrectly requires
(i.mediaType === 'movie' || i.mediaType === 'tv') for all entries, which drops
person results when settings.currentSettings.hideRequested is true; change the
predicate to explicitly allow person entries through (e.g., (i.mediaType ===
'person') || ((i.mediaType === 'movie' || i.mediaType === 'tv') &&
(i.mediaInfo?.status === MediaStatus.AVAILABLE || i.mediaInfo?.status ===
MediaStatus.PARTIALLY_AVAILABLE || !i.mediaInfo?.requests ||
i.mediaInfo.requests.length === 0))). Keep the null-safe accesses (i.mediaInfo?)
and use the existing symbols settings.currentSettings.hideRequested,
titles.filter, mediaType, MediaStatus, and mediaInfo to locate and update the
code.
---
Nitpick comments:
In `@src/hooks/useDiscover.ts`:
- Around line 16-19: The mediaInfo.requests property is typed too loosely as
unknown[]; change its type to MediaRequest[] (importing MediaRequest from
`@server/entity/Media`) so the mediaInfo definition in useDiscover.ts becomes
mediaInfo?: { status: MediaStatus; requests?: MediaRequest[]; } to improve type
safety and future property access.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7cd9fd0b-4b5d-4007-aff4-907919a1d6bd
📒 Files selected for processing (11)
docs/using-seerr/settings/general.mdseerr-api.ymlserver/entity/Media.tsserver/interfaces/api/settingsInterfaces.tsserver/lib/settings/index.tssrc/components/MediaSlider/index.tsxsrc/components/Search/index.tsxsrc/components/Settings/SettingsMain/index.tsxsrc/context/SettingsContext.tsxsrc/hooks/useDiscover.tssrc/pages/_app.tsx
There was a problem hiding this comment.
Pull request overview
Adds a new main setting to optionally hide requested-but-unavailable media from Discover surfaces (home “Discover” and related recommendation/similar lists), while keeping requested items visible in search.
Changes:
- Introduces
hideRequestedin server/public settings and wires it through the Settings UI. - Implements client-side filtering in
useDiscoverandMediaSliderbased on requested state. - Updates docs, OpenAPI schema, and English locale strings for the new option.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/pages/_app.tsx | Adds hideRequested to initial public settings fallback. |
| src/i18n/locale/en.json | Adds English strings for the new toggle + tooltip. |
| src/hooks/useDiscover.ts | Adds hideRequested option and filtering logic for discover-style queries. |
| src/context/SettingsContext.tsx | Adds hideRequested to default settings context. |
| src/components/Settings/SettingsMain/index.tsx | Adds the “Hide Requested Media” checkbox and includes it in save payload. |
| src/components/Search/index.tsx | Explicitly disables requested filtering for search results. |
| src/components/MediaSlider/index.tsx | Adds requested filtering for slider-based lists. |
| server/lib/settings/index.ts | Adds hideRequested to settings models, defaults, and public settings exposure. |
| server/interfaces/api/settingsInterfaces.ts | Extends PublicSettingsResponse with hideRequested. |
| server/entity/Media.ts | Loads media.requests in getRelatedMedia to support requested detection. |
| seerr-api.yml | Adds hideRequested to the documented MainSettings schema. |
| docs/using-seerr/settings/general.md | Documents the new setting and expected behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
0xSysR3ll
changed the title
|
Could this also hide previously requested and available content that is now unavailable? Would be nice for hiding content you grabbed them deleted without having to manually add to a blocklist |
|
Thanks for working on this. I’d really like to see this feature land. I noticed the PR is still open and has a few unresolved review comments from March. Is this still planned, and is there anything specific blocking it? I’d be happy to help test it if that would be useful. |
Hello, This is still planned. As you may have noticed, I'm a member of the Seerr team so I've been busy with other stuff (including PRs) – like the rest of the team. I'll get back to this as soon as I can, and we'll review it once it's ready. |
Hi, thanks for getting back to me. I didn't realize you were on the Seerr team when I first asked. Quick heads-up: not knowing whether this PR was still active, I opened a separate one in the meantime and reused part of your work (credited in my PR). Since you're clearly the right person to land this, I'm happy to defer to you. If anything in my version is useful, you're welcome to take it. Otherwise just say the word and I'll close mine so we don't have two open. Whatever's easiest for you. |
FYI, a PR is considered as stale after 30 days of inactivity and a label is applied to it.
I saw your PR (thanks for the credit btw) but I'll take over this PR and have a look at what you added. |
This PR introduces a new setting that allows users to hide media that has been requested but is not yet available from the "Discover" home page and related categories. Signed-off-by: 0xsysr3ll <0xsysr3ll@pm.me>
…tter type safety Signed-off-by: 0xsysr3ll <0xsysr3ll@pm.me>
…y status Signed-off-by: 0xsysr3ll <0xsysr3ll@pm.me>
cfeeb8c to
276853d
Compare
Description
This PR introduces a new setting that allows users to hide media that has been requested but is not yet available from the "Discover" home page and related categories.
How Has This Been Tested?
Screenshots / Logs (if applicable)
Checklist:
pnpm buildpnpm i18n:extractIssues Fixed or Closed
Summary by CodeRabbit