feat: Add parental controls and global content exclusions#3127
Open
MagicalPotato0001 wants to merge 1 commit into
Open
feat: Add parental controls and global content exclusions#3127MagicalPotato0001 wants to merge 1 commit into
MagicalPotato0001 wants to merge 1 commit into
Conversation
…over pages Add parental controls and global content exclusions
📝 WalkthroughWalkthroughThis PR introduces a comprehensive parental controls and global media filtering system. It adds per-user parental control settings (certification bounds, unrated blocking) and admin-level global exclusions (rating/certification and tag filters) enforced across discovery, search, movie, TV, and request endpoints using TMDb certification and keyword data. ChangesParental Controls & Global Filtering System
Sequence Diagram(s)sequenceDiagram
participant User
participant MediaRequest
participant ParentalControls
participant TMDb
User->>MediaRequest: Request media
MediaRequest->>ParentalControls: isMediaAllowedByParentalControls
ParentalControls->>TMDb: Fetch media details
TMDb-->>ParentalControls: Movie/TV certification data
ParentalControls->>ParentalControls: Check user cert bounds & adult status
ParentalControls-->>MediaRequest: Allow or throw ParentalControlRestrictedError
alt Allowed
MediaRequest->>MediaRequest: Create media entity and request
else Blocked
MediaRequest-->>User: 403 Forbidden
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
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}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
server/routes/user/usersettings.ts (1)
135-152:⚠️ Potential issue | 🔴 Critical | 🏗️ Heavy liftEnforce permission check for parental control field updates.
The parental control fields are currently accepted from
req.bodywithout verifying that the requester hasMANAGE_USERSpermission. This allows users to modify their own parental controls via direct API calls, even though the UI only exposes these settings to users withMANAGE_USERSpermission (seeUserGeneralSettings/index.tsxline 437).Parental controls should follow the same authorization pattern as quota fields (lines 116–124): only users with
MANAGE_USERSpermission should be able to configure parental controls on other users, and users should not be able to modify their own parental controls.🔒 Proposed fix to add permission guard
user.settings.watchlistSyncMovies = req.body.watchlistSyncMovies; user.settings.watchlistSyncTv = req.body.watchlistSyncTv; - user.settings.parentalControlsEnabled = req.body.parentalControlsEnabled; - user.settings.parentalControlsRegion = req.body.parentalControlsRegion; - user.settings.maxMovieCertification = req.body.maxMovieCertification; - user.settings.maxTvCertification = req.body.maxTvCertification; - user.settings.blockUnrated = req.body.blockUnrated; + + // Update parental controls only if requester has MANAGE_USERS permission + // and target user is not an admin (similar to quota logic at lines 116-124) + if ( + req.user?.hasPermission(Permission.MANAGE_USERS) && + !user.hasPermission(Permission.MANAGE_USERS) && + req.user?.id !== user.id + ) { + user.settings.parentalControlsEnabled = req.body.parentalControlsEnabled; + user.settings.parentalControlsRegion = req.body.parentalControlsRegion; + user.settings.maxMovieCertification = req.body.maxMovieCertification; + user.settings.maxTvCertification = req.body.maxTvCertification; + user.settings.blockUnrated = req.body.blockUnrated; + } }Apply the same guard when creating new
UserSettingsat lines 135–139.🤖 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 `@server/routes/user/usersettings.ts` around lines 135 - 152, When creating or updating UserSettings, guard assignment of parental control fields (parentalControlsEnabled, parentalControlsRegion, maxMovieCertification, maxTvCertification, blockUnrated) with the same MANAGE_USERS permission check used for quota fields: only set these properties on the new UserSettings object or on user.settings when hasManageUsersPermission(req) (or the existing permission helper used for quota) returns true; if the caller lacks MANAGE_USERS, skip assigning those parental control fields so users cannot modify their own parental controls.src/components/BlocklistedTagsSelector/index.tsx (1)
116-184:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winHardcoded key causes duplicate React keys across instances.
Line 167 hardcodes the key as
keyword-select-blocklistedTags, but this component is now reused for multiple fields (excludedMovieTags,excludedTvTags, andblocklistedTagsinSettingsMain). All three instances will have identical React keys, which can cause reconciliation issues.Pass
fieldNameas a prop toControlledKeywordSelectorand use it in the key to ensure uniqueness.Proposed fix
Update the
BaseSelectorMultiPropstype to includefieldName:type BaseSelectorMultiProps = { defaultValue?: string; value: MultiValue<SingleVal> | null; onChange: (value: MultiValue<SingleVal> | null) => void; components?: Partial<typeof components>; + fieldName: string; };Pass
fieldNamefromBlocklistedTagsSelectortoControlledKeywordSelector:<ControlledKeywordSelector value={selectorValue} onChange={update} defaultValue={defaultValue} + fieldName={fieldName} components={{Use
fieldNamein the key:<AsyncSelect - key={`keyword-select-blocklistedTags`} + key={`keyword-select-${fieldName}`} inputId="data"🤖 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/BlocklistedTagsSelector/index.tsx` around lines 116 - 184, ControlledKeywordSelector currently hardcodes key="keyword-select-blocklistedTags" causing duplicate React keys when used for excludedMovieTags, excludedTvTags and blocklistedTags; add a fieldName string to BaseSelectorMultiProps, accept fieldName in ControlledKeywordSelector props, pass fieldName from BlocklistedTagsSelector when rendering the controlled selector, and use it to build a unique key (e.g. `keyword-select-${fieldName}`) instead of the hardcoded value so each instance gets a distinct React key.
🧹 Nitpick comments (1)
src/components/Settings/SettingsMain/index.tsx (1)
441-494: 💤 Low valueCertification exclusions are hardcoded to US region only.
The
onChangehandlers (lines 464, 490) always setexcludedCertificationRegionto'US', and there is no UI control for users to select a different region. This means administrators can only exclude US certifications, not certifications from other regions.This appears intentional (the component is named
USCertificationSelector), but it's a design limitation to be aware of. If you plan to support other regions in the future, consider making the region configurable.🤖 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/Settings/SettingsMain/index.tsx` around lines 441 - 494, The onChange handlers for USCertificationSelector (used for excludedMovieCertifications and excludedTvCertifications) hardcode setFieldValue('excludedCertificationRegion','US'); change this so the region is not hardcoded: either read a region value from the selector callback (e.g., params.region) or add a region prop/UI control and pass that into the selector, then call setFieldValue('excludedCertificationRegion', regionValue) instead; ensure USCertificationSelector and its call sites (type="movie"/"tv", certification prop) are updated to surface or accept the region so administrators can select non-US regions.
🤖 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 `@server/lib/parentalControls.ts`:
- Around line 199-243: The three filter functions
(filterResultsByParentalControls, filterResultsByGlobalRatingExclusions,
filterResultsByGlobalTagExclusions) repeatedly call
TheMovieDb.getMovie/getTvShow for the same items, causing redundant TMDb API
calls; fix by consolidating into a single-pass filter (e.g.,
filterResultsByAllExclusions) or add a per-request details cache: iterate
results once using getResultMediaType, fetch details once via
TheMovieDb.getMovie/getTvShow, then call isMediaAllowedByParentalControls,
isMediaAllowedByGlobalRatingExclusions and isMediaAllowedByGlobalTagExclusions
against that single details object (or pass cached details into existing filter
helpers) and return filtered results; ensure early-return when no filters are
active and preserve type T in the result.
- Around line 218-240: Wrap each TMDb fetch inside the Promise.all map with
try/catch so a single failed fetch (from tmdb.getMovie or tmdb.getTvShow) does
not reject the whole Promise.all; inside the map used to produce allowedResults,
catch errors around the getResultMediaType branch and on error log/ignore the
item and return null so downstream isMediaAllowedByParentalControls is skipped
for failed fetches, preserving the existing return types and still filtering out
disallowed or failed items.
In `@server/lib/settings/index.ts`:
- Around line 427-428: The current defaults set excludedMovieCertifications and
excludedTvCertifications to 'NR', which hides all "Not Rated" content by
default; change these defaults to empty strings so no certifications are
excluded by default (update the values of excludedMovieCertifications and
excludedTvCertifications in settings/index.ts from 'NR' to ''), and adjust any
related docstrings, migration/defaults comments, or unit tests that assume 'NR'
is the default to reflect the new empty-string default.
In `@server/routes/discover.ts`:
- Around line 1307-1318: The response currently computes pagination using the
unfiltered watchlist size (watchlist.totalSize), which yields incorrect
totalResults/totalPages after applying filteredWatchlist; update the response to
derive totals from the filtered results instead: set totalResults to
filteredWatchlist.length and totalPages to Math.ceil(filteredWatchlist.length /
itemsPerPage) while keeping the results mapping (filteredWatchlist.map(...)) and
the page value as-is (page) so pagination reflects the filtered data.
In `@src/components/Settings/SettingsMain/index.tsx`:
- Around line 190-192: The UI currently falls back to the string 'NR' for
certification fields, which duplicates the old server default; update the
SettingsMain component to use empty-string fallbacks so they match the server
change: replace the expressions for excludedMovieCertifications and
excludedTvCertifications in src/components/Settings/SettingsMain/index.tsx (the
data?.excludedMovieCertifications and data?.excludedTvCertifications usages) to
use ?? '' instead of ?? 'NR' so the UI and server defaults stay consistent.
In `@src/components/UserProfile/UserSettings/UserGeneralSettings/index.tsx`:
- Around line 467-510: The two CertificationSelector instances (type="movie" and
type="tv") update parentalControlsRegion independently which can leave
maxMovieCertification or maxTvCertification inconsistent with the current
region; update the onChange handlers in both CertificationSelector usages so
when params.certificationCountry is present and different from
values.parentalControlsRegion you call setFieldValue('parentalControlsRegion',
params.certificationCountry ?? 'US') and also reset both certification fields
(setFieldValue('maxMovieCertification', undefined) and
setFieldValue('maxTvCertification', undefined) or to your form's cleared value)
before setting the new specific certification, ensuring parentalControlsRegion,
maxMovieCertification and maxTvCertification cannot become out-of-sync.
---
Outside diff comments:
In `@server/routes/user/usersettings.ts`:
- Around line 135-152: When creating or updating UserSettings, guard assignment
of parental control fields (parentalControlsEnabled, parentalControlsRegion,
maxMovieCertification, maxTvCertification, blockUnrated) with the same
MANAGE_USERS permission check used for quota fields: only set these properties
on the new UserSettings object or on user.settings when
hasManageUsersPermission(req) (or the existing permission helper used for quota)
returns true; if the caller lacks MANAGE_USERS, skip assigning those parental
control fields so users cannot modify their own parental controls.
In `@src/components/BlocklistedTagsSelector/index.tsx`:
- Around line 116-184: ControlledKeywordSelector currently hardcodes
key="keyword-select-blocklistedTags" causing duplicate React keys when used for
excludedMovieTags, excludedTvTags and blocklistedTags; add a fieldName string to
BaseSelectorMultiProps, accept fieldName in ControlledKeywordSelector props,
pass fieldName from BlocklistedTagsSelector when rendering the controlled
selector, and use it to build a unique key (e.g. `keyword-select-${fieldName}`)
instead of the hardcoded value so each instance gets a distinct React key.
---
Nitpick comments:
In `@src/components/Settings/SettingsMain/index.tsx`:
- Around line 441-494: The onChange handlers for USCertificationSelector (used
for excludedMovieCertifications and excludedTvCertifications) hardcode
setFieldValue('excludedCertificationRegion','US'); change this so the region is
not hardcoded: either read a region value from the selector callback (e.g.,
params.region) or add a region prop/UI control and pass that into the selector,
then call setFieldValue('excludedCertificationRegion', regionValue) instead;
ensure USCertificationSelector and its call sites (type="movie"/"tv",
certification prop) are updated to surface or accept the region so
administrators can select non-US regions.
🪄 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: d45b2564-3e11-4500-9e2e-a5235be8477d
📒 Files selected for processing (19)
next-env.d.tspackage.jsonserver/entity/MediaRequest.tsserver/entity/UserSettings.tsserver/interfaces/api/userSettingsInterfaces.tsserver/lib/parentalControls.tsserver/lib/settings/index.tsserver/migration/postgres/1780776000000-AddParentalControlsToUserSettings.tsserver/migration/sqlite/1780776000000-AddParentalControlsToUserSettings.tsserver/routes/discover.tsserver/routes/movie.tsserver/routes/request.tsserver/routes/search.tsserver/routes/tv.tsserver/routes/user/usersettings.tssrc/components/BlocklistedTagsSelector/index.tsxsrc/components/Settings/SettingsMain/index.tsxsrc/components/UserProfile/UserSettings/UserGeneralSettings/index.tsxsrc/hooks/useDiscover.ts
Comment on lines
+199
to
+243
| export const filterResultsByParentalControls = async < | ||
| T extends FilterableMediaResult, | ||
| >({ | ||
| user, | ||
| results, | ||
| tmdb = new TheMovieDb(), | ||
| mediaType, | ||
| language, | ||
| }: { | ||
| user?: User; | ||
| results: T[]; | ||
| tmdb?: TheMovieDb; | ||
| mediaType?: CertificationMediaType; | ||
| language?: string; | ||
| }): Promise<T[]> => { | ||
| if (!isParentalControlsEnabled(user)) { | ||
| return results; | ||
| } | ||
|
|
||
| const allowedResults: (T | null)[] = await Promise.all( | ||
| results.map(async (result) => { | ||
| const resultMediaType = getResultMediaType(result, mediaType); | ||
|
|
||
| if (!resultMediaType) { | ||
| return result; | ||
| } | ||
|
|
||
| const details = | ||
| resultMediaType === MediaType.MOVIE | ||
| ? await tmdb.getMovie({ movieId: result.id, language }) | ||
| : await tmdb.getTvShow({ tvId: result.id, language }); | ||
|
|
||
| return (await isMediaAllowedByParentalControls({ | ||
| user, | ||
| mediaType: resultMediaType, | ||
| media: details, | ||
| tmdb, | ||
| })) | ||
| ? (result as T) | ||
| : null; | ||
| }) | ||
| ); | ||
|
|
||
| return allowedResults.filter((result): result is T => !!result); | ||
| }; |
There was a problem hiding this comment.
Performance concern: Redundant TMDb API calls across filter functions.
Each of the three filter functions (filterResultsByParentalControls, filterResultsByGlobalRatingExclusions, filterResultsByGlobalTagExclusions) independently fetches TMDb details for every result. When these filters are chained in routes (as shown in discover.ts), the same TMDb details are fetched up to three times per item.
For a typical page of 20 results, this could result in 60 TMDb API calls instead of 20.
Consider consolidating the filtering into a single pass that fetches details once and applies all three checks, or caching the TMDb responses within the request lifecycle.
♻️ Proposed approach: Unified filter function
export const filterResultsByAllExclusions = async <
T extends FilterableMediaResult,
>({
user,
results,
tmdb = new TheMovieDb(),
mediaType,
language,
}: {
user?: User;
results: T[];
tmdb?: TheMovieDb;
mediaType?: CertificationMediaType;
language?: string;
}): Promise<T[]> => {
const parentalEnabled = isParentalControlsEnabled(user);
const hasGlobalRatingExclusions =
!!getSettings().main.excludedMovieCertifications ||
!!getSettings().main.excludedTvCertifications;
const hasGlobalTagExclusions =
getGlobalExcludedKeywordIds(MediaType.MOVIE).length > 0 ||
getGlobalExcludedKeywordIds(MediaType.TV).length > 0;
// Early return if no filtering needed
if (!parentalEnabled && !hasGlobalRatingExclusions && !hasGlobalTagExclusions) {
return results;
}
const allowedResults = await Promise.all(
results.map(async (result) => {
const resultMediaType = getResultMediaType(result, mediaType);
if (!resultMediaType) return result;
// Fetch details once
const details =
resultMediaType === MediaType.MOVIE
? await tmdb.getMovie({ movieId: result.id, language })
: await tmdb.getTvShow({ tvId: result.id, language });
// Apply all checks
if (parentalEnabled) {
const allowed = await isMediaAllowedByParentalControls({
user, mediaType: resultMediaType, media: details, tmdb
});
if (!allowed) return null;
}
if (!isMediaAllowedByGlobalRatingExclusions({ mediaType: resultMediaType, media: details })) {
return null;
}
if (!isMediaAllowedByGlobalTagExclusions({ mediaType: resultMediaType, media: details })) {
return null;
}
return result;
})
);
return allowedResults.filter((result): result is T => !!result);
};Also applies to: 349-394, 481-524
🤖 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 `@server/lib/parentalControls.ts` around lines 199 - 243, The three filter
functions (filterResultsByParentalControls,
filterResultsByGlobalRatingExclusions, filterResultsByGlobalTagExclusions)
repeatedly call TheMovieDb.getMovie/getTvShow for the same items, causing
redundant TMDb API calls; fix by consolidating into a single-pass filter (e.g.,
filterResultsByAllExclusions) or add a per-request details cache: iterate
results once using getResultMediaType, fetch details once via
TheMovieDb.getMovie/getTvShow, then call isMediaAllowedByParentalControls,
isMediaAllowedByGlobalRatingExclusions and isMediaAllowedByGlobalTagExclusions
against that single details object (or pass cached details into existing filter
helpers) and return filtered results; ensure early-return when no filters are
active and preserve type T in the result.
Comment on lines
+218
to
+240
| const allowedResults: (T | null)[] = await Promise.all( | ||
| results.map(async (result) => { | ||
| const resultMediaType = getResultMediaType(result, mediaType); | ||
|
|
||
| if (!resultMediaType) { | ||
| return result; | ||
| } | ||
|
|
||
| const details = | ||
| resultMediaType === MediaType.MOVIE | ||
| ? await tmdb.getMovie({ movieId: result.id, language }) | ||
| : await tmdb.getTvShow({ tvId: result.id, language }); | ||
|
|
||
| return (await isMediaAllowedByParentalControls({ | ||
| user, | ||
| mediaType: resultMediaType, | ||
| media: details, | ||
| tmdb, | ||
| })) | ||
| ? (result as T) | ||
| : null; | ||
| }) | ||
| ); |
There was a problem hiding this comment.
Missing error handling for individual TMDb fetches.
Promise.all will reject entirely if any single TMDb fetch fails (e.g., a deleted movie returns 404). This could cause entire discovery pages to fail due to one problematic item.
Consider wrapping individual fetches with error handling to gracefully exclude failed items rather than failing the entire batch.
🛡️ Proposed fix with error handling
const allowedResults: (T | null)[] = await Promise.all(
results.map(async (result) => {
+ try {
const resultMediaType = getResultMediaType(result, mediaType);
if (!resultMediaType) {
return result;
}
const details =
resultMediaType === MediaType.MOVIE
? await tmdb.getMovie({ movieId: result.id, language })
: await tmdb.getTvShow({ tvId: result.id, language });
return (await isMediaAllowedByParentalControls({
user,
mediaType: resultMediaType,
media: details,
tmdb,
}))
? (result as T)
: null;
+ } catch {
+ // If we can't fetch details, exclude the item to be safe
+ return null;
+ }
})
);📝 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
| const allowedResults: (T | null)[] = await Promise.all( | |
| results.map(async (result) => { | |
| const resultMediaType = getResultMediaType(result, mediaType); | |
| if (!resultMediaType) { | |
| return result; | |
| } | |
| const details = | |
| resultMediaType === MediaType.MOVIE | |
| ? await tmdb.getMovie({ movieId: result.id, language }) | |
| : await tmdb.getTvShow({ tvId: result.id, language }); | |
| return (await isMediaAllowedByParentalControls({ | |
| user, | |
| mediaType: resultMediaType, | |
| media: details, | |
| tmdb, | |
| })) | |
| ? (result as T) | |
| : null; | |
| }) | |
| ); | |
| const allowedResults: (T | null)[] = await Promise.all( | |
| results.map(async (result) => { | |
| try { | |
| const resultMediaType = getResultMediaType(result, mediaType); | |
| if (!resultMediaType) { | |
| return result; | |
| } | |
| const details = | |
| resultMediaType === MediaType.MOVIE | |
| ? await tmdb.getMovie({ movieId: result.id, language }) | |
| : await tmdb.getTvShow({ tvId: result.id, language }); | |
| return (await isMediaAllowedByParentalControls({ | |
| user, | |
| mediaType: resultMediaType, | |
| media: details, | |
| tmdb, | |
| })) | |
| ? (result as T) | |
| : null; | |
| } catch { | |
| // If we can't fetch details, exclude the item to be safe | |
| return null; | |
| } | |
| }) | |
| ); |
🤖 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 `@server/lib/parentalControls.ts` around lines 218 - 240, Wrap each TMDb fetch
inside the Promise.all map with try/catch so a single failed fetch (from
tmdb.getMovie or tmdb.getTvShow) does not reject the whole Promise.all; inside
the map used to produce allowedResults, catch errors around the
getResultMediaType branch and on error log/ignore the item and return null so
downstream isMediaAllowedByParentalControls is skipped for failed fetches,
preserving the existing return types and still filtering out disallowed or
failed items.
Comment on lines
+427
to
+428
| excludedMovieCertifications: 'NR', | ||
| excludedTvCertifications: 'NR', |
There was a problem hiding this comment.
Reconsider defaulting to exclude 'NR' content globally.
These defaults exclude all "Not Rated" content from discover/movie/series pages for every user in new installations. Many movies and TV shows lack ratings (international content, older content, independent films), so this could hide substantial content by default and surprise administrators.
Consider defaulting to empty strings (exclude nothing) and letting administrators explicitly opt-in to global exclusions.
Suggested default values
- excludedMovieCertifications: 'NR',
- excludedTvCertifications: 'NR',
+ excludedMovieCertifications: '',
+ excludedTvCertifications: '',
excludedCertificationRegion: 'US',📝 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
| excludedMovieCertifications: 'NR', | |
| excludedTvCertifications: 'NR', | |
| excludedMovieCertifications: '', | |
| excludedTvCertifications: '', | |
| excludedCertificationRegion: 'US', |
🤖 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 `@server/lib/settings/index.ts` around lines 427 - 428, The current defaults
set excludedMovieCertifications and excludedTvCertifications to 'NR', which
hides all "Not Rated" content by default; change these defaults to empty strings
so no certifications are excluded by default (update the values of
excludedMovieCertifications and excludedTvCertifications in settings/index.ts
from 'NR' to ''), and adjust any related docstrings, migration/defaults
comments, or unit tests that assume 'NR' is the default to reflect the new
empty-string default.
Comment on lines
1307
to
1318
| return res.json({ | ||
| page, | ||
| totalPages: Math.ceil(watchlist.totalSize / itemsPerPage), | ||
| totalResults: watchlist.totalSize, | ||
| results: watchlist.items.map((item) => ({ | ||
| results: filteredWatchlist.map((item) => ({ | ||
| id: item.tmdbId, | ||
| ratingKey: item.ratingKey, | ||
| title: item.title, | ||
| mediaType: item.type === 'show' ? 'tv' : 'movie', | ||
| mediaType: item.mediaType, | ||
| tmdbId: item.tmdbId, | ||
| })), | ||
| }); |
There was a problem hiding this comment.
Pagination metadata does not reflect filtered results.
After filtering the watchlist, totalPages and totalResults still use watchlist.totalSize (the original unfiltered count). This causes incorrect pagination: users may see "page 1 of 5" but only receive 3 results because 17 were filtered out.
Consider recalculating pagination based on filtered results, or documenting this as expected behavior.
🐛 Proposed fix
+ const filteredTotal = filteredWatchlist.length + offset; // Approximate, or track actual filtered total
return res.json({
page,
- totalPages: Math.ceil(watchlist.totalSize / itemsPerPage),
- totalResults: watchlist.totalSize,
+ totalPages: Math.ceil(filteredWatchlist.length / itemsPerPage) || 1,
+ totalResults: filteredWatchlist.length,
results: filteredWatchlist.map((item) => ({Note: A complete solution would require filtering the entire watchlist to get accurate totals, which has performance implications. Consider whether approximate pagination is acceptable.
🤖 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 `@server/routes/discover.ts` around lines 1307 - 1318, The response currently
computes pagination using the unfiltered watchlist size (watchlist.totalSize),
which yields incorrect totalResults/totalPages after applying filteredWatchlist;
update the response to derive totals from the filtered results instead: set
totalResults to filteredWatchlist.length and totalPages to
Math.ceil(filteredWatchlist.length / itemsPerPage) while keeping the results
mapping (filteredWatchlist.map(...)) and the page value as-is (page) so
pagination reflects the filtered data.
Comment on lines
+190
to
+192
| excludedMovieCertifications: | ||
| data?.excludedMovieCertifications ?? 'NR', | ||
| excludedTvCertifications: data?.excludedTvCertifications ?? 'NR', |
There was a problem hiding this comment.
UI defaults propagate the 'NR' exclusion.
The UI fallback values also default to 'NR' for certifications when server data is missing, which compounds the concern raised in server/lib/settings/index.ts (lines 427-428). If you change the server defaults to empty strings, update these UI fallbacks to match.
🤖 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/Settings/SettingsMain/index.tsx` around lines 190 - 192, The
UI currently falls back to the string 'NR' for certification fields, which
duplicates the old server default; update the SettingsMain component to use
empty-string fallbacks so they match the server change: replace the expressions
for excludedMovieCertifications and excludedTvCertifications in
src/components/Settings/SettingsMain/index.tsx (the
data?.excludedMovieCertifications and data?.excludedTvCertifications usages) to
use ?? '' instead of ?? 'NR' so the UI and server defaults stay consistent.
Comment on lines
+467
to
+510
| <div className="form-row"> | ||
| <label className="text-label"> | ||
| {intl.formatMessage(messages.maxMovieCertification)} | ||
| </label> | ||
| <div className="form-input-area"> | ||
| <CertificationSelector | ||
| type="movie" | ||
| certificationCountry={values.parentalControlsRegion} | ||
| certification={values.maxMovieCertification} | ||
| onChange={(params) => { | ||
| setFieldValue( | ||
| 'parentalControlsRegion', | ||
| params.certificationCountry ?? 'US' | ||
| ); | ||
| setFieldValue( | ||
| 'maxMovieCertification', | ||
| params.certification | ||
| ); | ||
| }} | ||
| /> | ||
| </div> | ||
| </div> | ||
| <div className="form-row"> | ||
| <label className="text-label"> | ||
| {intl.formatMessage(messages.maxTvCertification)} | ||
| </label> | ||
| <div className="form-input-area"> | ||
| <CertificationSelector | ||
| type="tv" | ||
| certificationCountry={values.parentalControlsRegion} | ||
| certification={values.maxTvCertification} | ||
| onChange={(params) => { | ||
| setFieldValue( | ||
| 'parentalControlsRegion', | ||
| params.certificationCountry ?? 'US' | ||
| ); | ||
| setFieldValue( | ||
| 'maxTvCertification', | ||
| params.certification | ||
| ); | ||
| }} | ||
| /> | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
Prevent region/certification mismatch across movie and TV selectors.
Both CertificationSelector components share a single parentalControlsRegion field but update it independently. This can create inconsistent state:
- User selects US region with
PG-13for movies - User selects GB region with
12Afor TV - Result:
parentalControlsRegion='GB',maxMovieCertification='PG-13',maxTvCertification='12A'
Now maxMovieCertification holds a US certification (PG-13) while parentalControlsRegion is 'GB'. This mismatch may cause validation errors or unexpected filtering behavior in the enforcement layer.
Consider either:
- Resetting both certification values when the region changes in either selector, or
- Using a single shared region selector above both certification pickers to enforce a consistent region
🔧 Example fix: reset certifications on region change
<CertificationSelector
type="movie"
certificationCountry={values.parentalControlsRegion}
certification={values.maxMovieCertification}
onChange={(params) => {
+ // If region changed, reset both certifications
+ if (params.certificationCountry !== values.parentalControlsRegion) {
+ setFieldValue('maxMovieCertification', undefined);
+ setFieldValue('maxTvCertification', undefined);
+ } else {
+ setFieldValue('maxMovieCertification', params.certification);
+ }
setFieldValue(
'parentalControlsRegion',
params.certificationCountry ?? 'US'
);
- setFieldValue(
- 'maxMovieCertification',
- params.certification
- );
}}
/>Apply similar logic to the TV certification selector.
🤖 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/UserProfile/UserSettings/UserGeneralSettings/index.tsx` around
lines 467 - 510, The two CertificationSelector instances (type="movie" and
type="tv") update parentalControlsRegion independently which can leave
maxMovieCertification or maxTvCertification inconsistent with the current
region; update the onChange handlers in both CertificationSelector usages so
when params.certificationCountry is present and different from
values.parentalControlsRegion you call setFieldValue('parentalControlsRegion',
params.certificationCountry ?? 'US') and also reset both certification fields
(setFieldValue('maxMovieCertification', undefined) and
setFieldValue('maxTvCertification', undefined) or to your form's cleared value)
before setting the new specific certification, ensuring parentalControlsRegion,
maxMovieCertification and maxTvCertification cannot become out-of-sync.
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.
Title
Add parental controls and global content exclusions
Description
Adds per-user parental controls and app-wide content exclusion settings.
This change adds parental control settings to user profiles, allowing admins to set maximum movie/series ratings and optionally block unrated content.
It also adds global Seerr settings for excluding specific ratings and TMDB keyword tags from discover/movie/series browsing independently from the blocklist. This allows admins to allow ratings like
Rwhile still hiding titles with specific tags. Manual search remains unaffected by the global exclusions.AI Assistance Disclosure
This pull request was developed with assistance from AI tooling(codex). The changes were reviewed and tested locally before submission. I have experience as a react/nextjs developer, I was planning on making it mostly myself but decided to try out codex since this feature was very needed for me.
How Has This Been Tested?
Tested locally on Windows with Node/pnpm project dependencies installed.
Ran:
pnpm typecheck:serverpnpm typecheck:clientpnpm buildScreenshots / Logs (if applicable)
Checklist:
pnpm buildpnpm i18n:extract(not sure about this)Summary by CodeRabbit
New Features
Chores