feat: per-user parental controls (content rating limits)#2415
feat: per-user parental controls (content rating limits)#2415ProgenyAlpha wants to merge 26 commits intoseerr-team:developfrom
Conversation
ProgenyAlpha
force-pushed
the
feature/parental-controls
branch
3 times, most recently
from
d4bda48 to
f8ee51d
Compare
|
Isnt this the same as |
I wasn't aware of #2275 because it wasn't linked to any of the issues, so it didn't come up when I scoped out existing work before starting. But probably wouldn't have matter as I technically started this back in December 2025 and wanted to use it for an extensive period of time before submitting it. That said, the implementations are architecturally different. #2275 makes live TMDB API calls per-item at request time to fetch certifications, which adds latency to every discovery and search page and scales poorly with TMDB rate limits. This PR uses a static rating hierarchy; filtering is instant with zero external API overhead. A few other things this PR brings to the table:
Happy to collaborate or consolidate, the static rating approach could drop right into #2275 as a performance improvement if the maintainers prefer that path. |
|
Nice work — this PR looks cleaner and will perform better than mine, especially with the backfill. I do have a few suggestions and one gap to call out; I'd love to see a hybrid that combines your architecture with a couple of features from my branch. High-level takeaways What I'd bring from my PR Enhanced rating fallback (applies when blockUnrated=true) Detailed logging Bulk edit for parental controls Architecture note Missing coverage /movies/studio/:studioId I'm excited about this — I can't really let friends and family use the app until something like this is in place. If you want, I can open a follow-up PR that merges the rating fallback and bulk edit into this branch. |
|
Hey, wanted to follow up — after digging deeper into your branch I realize my earlier comment overstated the route coverage gap. You're actually filtering 10 out of 11 discover routes, which is great. The only one missing filtering is /trending. Apologies for the inaccuracy there. I still think there are a few things from my branch that could complement yours nicely — bulk edit (setting parental controls on multiple users at once), smarter certification lookup (excluding NR from unrated director's cuts so the theatrical rating wins, plus international fallback when there's no US rating), and an optional "block adult content" toggle. These all slot into your existing architecture without extra API overhead. Happy to put together a PR into your branch if you're interested. |
|
Hey! Sorry, saw your email but it's been a busy week so I didn't get a chance to reply. Just saw your PR, so I'll dig into your comments and review everything this weekend. Thanks for putting this together! |
Admin-enforced content rating limits per user: - Max movie rating (MPAA: G through NC-17) - Max TV rating (US Parental Guidelines: TV-Y through TV-MA) - Block unrated content toggle (fail-closed) Filtering applied to all discover routes and search with parallel TMDB certification lookups. Backfills from next page when filtering drops results below 15. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Critical bug: postFilterDiscoverMovies and postFilterDiscoverTv had an early return that only ran the rating post-filter when blockUnrated was enabled. Routes like /trending that lack TMDB certification.lte pre-filter rely entirely on post-filtering, so R-rated movies and TV-MA shows passed through unfiltered when a user had a max rating set but blockUnrated was false. Fixes: postFilterDiscoverMovies runs when maxMovieRating OR blockUnrated is set. postFilterDiscoverTv runs when maxTvRating OR blockUnrated is set. filterMovieBatch and filterTvBatch use actual limits.blockUnrated instead of hardcoded true.
…Adult to search - Import getMovieRatingOptions/getTvRatingOptions from contentRatings.ts instead of duplicating arrays in BulkEditModal and ParentalControlsSettings - Add server-side validation for rating values on parental controls endpoints - Extend blockAdult filtering to search route (was only on discover routes) - Add blockUnrated and blockAdult fields to OpenAPI spec - Fix indentation in trending route map callbacks - Remove unused norestriction i18n keys
ProgenyAlpha
force-pushed
the
feature/parental-controls
branch
from
f8ee51d to
4621c8d
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:
📝 WalkthroughWalkthroughAdds per-user parental controls: DB migrations, new content-rating constants and filtering helpers, discovery/search enforcement with optional backfill, admin API endpoints to get/update settings, bulk-edit and user settings UI, a Next.js settings page, and new localization keys. Changes
Sequence Diagram(s)sequenceDiagram
participant Admin as Admin
participant API as Overseerr API
participant DB as Database
participant TMDB as TMDB
Admin->>API: PUT /user (bulk) with parental control fields
API->>API: Validate ratings (MOVIE_RATINGS/TV_RATINGS)
API->>DB: Load/create & save user.settings (maxMovieRating,maxTvRating,blockUnrated,blockAdult)
DB-->>API: Persisted settings
API-->>Admin: 200 OK
Note over API,TMDB: Per-user limits applied during discovery/search
participant User as Target User
participant Discover as Discover Route
User->>Discover: GET /discover/movies
Discover->>DB: Fetch user's content limits
DB-->>Discover: maxMovieRating, blockUnrated, blockAdult
Discover->>TMDB: TMDB search
TMDB-->>Discover: Results page
Discover->>TMDB: Fetch details/certifications for results
TMDB-->>Discover: Details with ratings
Discover->>Discover: Apply shouldFilterMovie/shouldFilterTv per item
alt Filtered count < threshold
Discover->>TMDB: Fetch next page (backfill)
TMDB-->>Discover: Additional results
Discover->>Discover: Re-apply filters
end
Discover-->>User: Filtered results
sequenceDiagram
participant Browser as Admin Browser
participant NextJS as Next.js Page
participant API as Overseerr API
participant DB as Database
Browser->>NextJS: Load /users/[id]/settings/parental-controls
NextJS->>NextJS: Check MANAGE_USERS permission
NextJS-->>Browser: Render Parental Controls page
Browser->>API: GET /api/v1/user/{id}/settings/parental-controls
API->>DB: Load user.settings
DB-->>API: Settings JSON
API-->>Browser: 200 + settings
Browser->>Browser: Formik initializes with values
Browser->>API: POST /api/v1/user/{id}/settings/parental-controls
API->>API: Validate ratings
API->>DB: Update/create user.settings
DB-->>API: Updated record
API-->>Browser: 200 + updated settings
Browser->>Browser: Show success toast & revalidate
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
🚥 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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/components/UserList/BulkEditModal.tsx (1)
47-73:⚠️ Potential issue | 🟠 MajorAvoid unintentionally clearing parental controls during bulk edits.
Right now the modal always sends
maxMovieRating,maxTvRating,blockUnrated, andblockAdultwith default''/false, which will wipe existing restrictions even when the admin only edits permissions. Please only send these fields when the admin actually changes them (or prefill and compare).✅ Suggested fix (track “touched” fields and conditionally include in payload)
- const [currentMaxMovieRating, setCurrentMaxMovieRating] = useState< - string | undefined - >(undefined); - const [currentMaxTvRating, setCurrentMaxTvRating] = useState< - string | undefined - >(undefined); - const [currentBlockUnrated, setCurrentBlockUnrated] = useState(false); - const [currentBlockAdult, setCurrentBlockAdult] = useState(false); + const [currentMaxMovieRating, setCurrentMaxMovieRating] = useState< + string | undefined + >(undefined); + const [currentMaxTvRating, setCurrentMaxTvRating] = useState< + string | undefined + >(undefined); + const [currentBlockUnrated, setCurrentBlockUnrated] = useState(false); + const [currentBlockAdult, setCurrentBlockAdult] = useState(false); + const [maxMovieRatingTouched, setMaxMovieRatingTouched] = useState(false); + const [maxTvRatingTouched, setMaxTvRatingTouched] = useState(false); + const [blockUnratedTouched, setBlockUnratedTouched] = useState(false); + const [blockAdultTouched, setBlockAdultTouched] = useState(false); const { data: updated } = await axios.put<User[]>(`/api/v1/user`, { ids: selectedUserIds, permissions: currentPermission, - maxMovieRating: currentMaxMovieRating || '', - maxTvRating: currentMaxTvRating || '', - blockUnrated: currentBlockUnrated, - blockAdult: currentBlockAdult, + ...(maxMovieRatingTouched + ? { maxMovieRating: currentMaxMovieRating || '' } + : {}), + ...(maxTvRatingTouched ? { maxTvRating: currentMaxTvRating || '' } : {}), + ...(blockUnratedTouched ? { blockUnrated: currentBlockUnrated } : {}), + ...(blockAdultTouched ? { blockAdult: currentBlockAdult } : {}), });- onChange={(e) => - setCurrentMaxMovieRating(e.target.value || undefined) - } + onChange={(e) => { + setMaxMovieRatingTouched(true); + setCurrentMaxMovieRating(e.target.value || undefined); + }} - onChange={(e) => - setCurrentMaxTvRating(e.target.value || undefined) - } + onChange={(e) => { + setMaxTvRatingTouched(true); + setCurrentMaxTvRating(e.target.value || undefined); + }} - onChange={(e) => setCurrentBlockUnrated(e.target.checked)} + onChange={(e) => { + setBlockUnratedTouched(true); + setCurrentBlockUnrated(e.target.checked); + }} - onChange={(e) => setCurrentBlockAdult(e.target.checked)} + onChange={(e) => { + setBlockAdultTouched(true); + setCurrentBlockAdult(e.target.checked); + }}Also applies to: 128-205
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/UserList/BulkEditModal.tsx` around lines 47 - 73, The bulk-edit currently always sends maxMovieRating/maxTvRating/blockUnrated/blockAdult (defaulting to ''/false) and can unintentionally clear parental controls; change the component to track whether each parental-control field was touched (e.g., add touchedMaxMovieRating/touchedMaxTvRating/touchedBlockUnrated/touchedBlockAdult or make block states undefined initially) and update the form controls to mark those touched flags when the admin changes them; then in updateUsers build the PUT payload starting with ids: selectedUserIds and permissions: currentPermission and only attach maxMovieRating/currentMaxTvRating/currentBlockUnrated/currentBlockAdult if their corresponding touched flag is true (or value !== undefined) so unchanged fields are omitted from the request; ensure references to currentMaxMovieRating, currentMaxTvRating, currentBlockUnrated, currentBlockAdult and updateUsers are updated accordingly.server/routes/discover.ts (1)
1338-1381:⚠️ Potential issue | 🟡 MinorMissing parental controls filtering on keyword movies endpoint.
This endpoint does not apply content rating filtering, which could allow users to bypass parental controls by navigating to keyword-based movie listings.
🔒 Proposed fix to add filtering
discoverRoutes.get<{ keywordId: string }>( '/keyword/:keywordId/movies', async (req, res, next) => { const tmdb = new TheMovieDb(); + const ratingLimits = getUserContentRatingLimits(req.user); try { const data = await tmdb.getMoviesByKeyword({ keywordId: Number(req.params.keywordId), page: Number(req.query.page), language: (req.query.language as string) ?? req.locale, }); + // Post-filter for parental controls + const filteredResults = await postFilterDiscoverMovies( + data.results, + tmdb, + ratingLimits, + undefined, + false // no pre-filter applied + ); + const media = await Media.getRelatedMedia( req.user, - data.results.map((result) => result.id) + filteredResults.map((result) => result.id) ); return res.status(200).json({ page: data.page, totalPages: data.total_pages, totalResults: data.total_results, - results: data.results.map((result) => + results: filteredResults.map((result) => mapMovieResult( result, media.find( (med) => med.tmdbId === result.id && med.mediaType === MediaType.MOVIE ) ) ), });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/routes/discover.ts` around lines 1338 - 1381, The keyword movies handler (discoverRoutes.get -> tmdb.getMoviesByKeyword) currently returns TMDB results without applying parental controls; update the handler to filter data.results using the same parental-controls helper used elsewhere (apply before mapping and before computing results sent to the client) based on req.user (e.g., the helper used by other discover/search endpoints), then call Media.getRelatedMedia and map via mapMovieResult only for the filtered IDs (MediaType.MOVIE); ensure the response results array and counts reflect the filtered set (or explicitly document preserving TMDB counts) and keep the existing error handling.
🧹 Nitpick comments (2)
server/routes/search.ts (1)
66-125: Skip certification lookups when onlyblockAdultis active.If no rating/unrated limits are set, you can return the adult-pre-filtered list immediately and avoid extra TMDB calls.
⚡️ Suggested optimization
- const settled = await Promise.allSettled( - preFiltered.map((r) => getCertification(r, tmdb)) - ); + if (!maxMovieRating && !maxTvRating && !blockUnrated) { + return preFiltered; + } + + const settled = await Promise.allSettled( + preFiltered.map((r) => getCertification(r, tmdb)) + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/routes/search.ts` around lines 66 - 125, The function filterSearchBatch does unnecessary TMDB certification lookups when there are no rating/unrated constraints—add an early return that returns preFiltered immediately if there are no rating limits and blockUnrated is false (i.e. when !maxMovieRating && !maxTvRating && !blockUnrated); place this check before calling Promise.allSettled so getCertification is not invoked, referencing the existing preFiltered variable and the getCertification calls inside filterSearchBatch.server/routes/discover.ts (1)
268-300: TV filtering lacks international rating fallback unlike movies.The movie filtering at lines 181-192 falls back to international ratings if no US rating is found, but the TV filtering only checks for US ratings. This inconsistency may result in more TV content being treated as unrated compared to movies.
Consider adding similar international fallback logic for TV shows if consistent behavior across media types is desired. This could help reduce false positives for international TV content when
blockUnratedis enabled.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/routes/discover.ts` around lines 268 - 300, filterTvBatch currently only uses the US rating from details.content_ratings for TV shows, so add the same international-fallback behavior used in movie filtering: when tmdb.getTvShow(...) returns details and no US rating is found, fall back to the first available rating from details.content_ratings.results (or a suitable non-US entry) to compute cert before calling shouldFilterTv; update the destructuring/assignment where you derive cert (from details.content_ratings?.results?.find(...)) to try US first then fallback to an international rating, and keep the logger.debug call (logger.debug, tvId/show.id, tvTitle/title, certification, maxRating) unchanged so blocked shows still log with the chosen certification.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@server/constants/contentRatings.ts`:
- Around line 1-6: The top header comment in contentRatings.ts incorrectly
states "Lower index = more restrictive" while the rating arrays are ordered from
least restrictive to most restrictive; update that comment to say something like
"Lower index = less restrictive (suitable for older audiences)" or "Lists are
ordered from least restrictive to most restrictive" so it matches the ordering
used by the rating arrays (e.g., TV and movie rating constants) in this file.
In `@server/routes/search.ts`:
- Around line 212-245: The current computation of filterRatio can exceed 1 when
backfill adds results, inflating totalPages/totalResults; change the filterRatio
calculation in this block to clamp it to a maximum of 1. Specifically, update
the filterRatio assignment (which uses originalCount and filteredCount) to use
Math.min(1, filteredCount / originalCount) when originalCount > 0 (leave it as 1
when originalCount === 0), so the totals computed after calling
filterSearchResultsByRating and Media.getRelatedMedia cannot exceed TMDB's
original totals.
In `@server/routes/user/index.ts`:
- Around line 493-524: The bulk update loop in users.map updates parental
controls without protecting admin users; before applying any parental-controls
fields (maxMovieRating, maxTvRating, blockUnrated, blockAdult) check the user's
permissions (e.g., user.permissions includes 'MANAGE_USERS' or the constant used
elsewhere) and either skip applying those settings or throw an error consistent
with usersettings.ts behavior; locate the block inside the async map where
UserSettings is created/modified and add the same guard as in usersettings.ts
(reject or no-op for users with MANAGE_USERS) before mutating settings and
calling userRepository.save(user).
---
Outside diff comments:
In `@server/routes/discover.ts`:
- Around line 1338-1381: The keyword movies handler (discoverRoutes.get ->
tmdb.getMoviesByKeyword) currently returns TMDB results without applying
parental controls; update the handler to filter data.results using the same
parental-controls helper used elsewhere (apply before mapping and before
computing results sent to the client) based on req.user (e.g., the helper used
by other discover/search endpoints), then call Media.getRelatedMedia and map via
mapMovieResult only for the filtered IDs (MediaType.MOVIE); ensure the response
results array and counts reflect the filtered set (or explicitly document
preserving TMDB counts) and keep the existing error handling.
In `@src/components/UserList/BulkEditModal.tsx`:
- Around line 47-73: The bulk-edit currently always sends
maxMovieRating/maxTvRating/blockUnrated/blockAdult (defaulting to ''/false) and
can unintentionally clear parental controls; change the component to track
whether each parental-control field was touched (e.g., add
touchedMaxMovieRating/touchedMaxTvRating/touchedBlockUnrated/touchedBlockAdult
or make block states undefined initially) and update the form controls to mark
those touched flags when the admin changes them; then in updateUsers build the
PUT payload starting with ids: selectedUserIds and permissions:
currentPermission and only attach
maxMovieRating/currentMaxTvRating/currentBlockUnrated/currentBlockAdult if their
corresponding touched flag is true (or value !== undefined) so unchanged fields
are omitted from the request; ensure references to currentMaxMovieRating,
currentMaxTvRating, currentBlockUnrated, currentBlockAdult and updateUsers are
updated accordingly.
---
Nitpick comments:
In `@server/routes/discover.ts`:
- Around line 268-300: filterTvBatch currently only uses the US rating from
details.content_ratings for TV shows, so add the same international-fallback
behavior used in movie filtering: when tmdb.getTvShow(...) returns details and
no US rating is found, fall back to the first available rating from
details.content_ratings.results (or a suitable non-US entry) to compute cert
before calling shouldFilterTv; update the destructuring/assignment where you
derive cert (from details.content_ratings?.results?.find(...)) to try US first
then fallback to an international rating, and keep the logger.debug call
(logger.debug, tvId/show.id, tvTitle/title, certification, maxRating) unchanged
so blocked shows still log with the chosen certification.
In `@server/routes/search.ts`:
- Around line 66-125: The function filterSearchBatch does unnecessary TMDB
certification lookups when there are no rating/unrated constraints—add an early
return that returns preFiltered immediately if there are no rating limits and
blockUnrated is false (i.e. when !maxMovieRating && !maxTvRating &&
!blockUnrated); place this check before calling Promise.allSettled so
getCertification is not invoked, referencing the existing preFiltered variable
and the getCertification calls inside filterSearchBatch.
Update: Merged contributions from #2275 + cleanupI've rebased onto the latest Bug fix (critical)The post-filter functions ( New features (from @mcfalld)
Cleanup
Performance noteAdded a Big thanks to @mcfalld for the thorough review, the critical bug catch, and the feature additions. The static rating hierarchy from this PR combined with his enhanced cert lookup and bulk edit makes for a much more complete solution than either PR alone. One thought: would the team be open to adding a small feature announcement (info banner or similar) so admins know parental controls are available after updating? Happy to implement whatever pattern you'd prefer, or leave it to the release notes. |
- Track touched fields in BulkEditModal to prevent wiping existing parental controls when only editing permissions - Add parental controls filtering to /keyword/:keywordId/movies endpoint - Skip expensive TMDB cert lookups when only blockAdult is active - Skip parental controls on admin/MANAGE_USERS users in bulk edit - Add international fallback to TV certification lookup - Clamp filterRatio to max 1 to prevent inflated search totals - Fix comment: lower index = less restrictive
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
server/routes/search.ts (2)
190-191: Consider validating page number to avoid unexpected behavior.
searchPagedefaults to1ifNaN, but doesn't guard against negative values or zero. While TMDB likely handles invalid page numbers gracefully, explicit validation is defensive.🛡️ Optional defensive validation
- const searchPage = Number(req.query.page) || 1; + const searchPage = Math.max(1, Number(req.query.page) || 1);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/routes/search.ts` around lines 190 - 191, The parsed searchPage from req.query.page can be NaN, zero or negative; after parsing Number(req.query.page) ensure it is a valid integer >= 1 (e.g., use Number.parseInt/Math.floor and then clamp to at least 1) before using it in the search flow so invalid values default to 1; update the logic around searchPage (the variable assigned from req.query.page) to validate and normalize the value defensively.
74-77: Verify adult filtering covers all result types with theadultproperty.The pre-filter checks
'adult' in rbut then casts toTmdbMovieResult. Per the interfaces,TmdbPersonResultandTmdbCollectionResultalso haveadultproperties. The current implementation works because'adult' in ris checked first, but the cast is misleading.♻️ Suggested clarification (optional)
- const preFiltered = blockAdult - ? results.filter((r) => !('adult' in r && (r as TmdbMovieResult).adult)) - : results; + const preFiltered = blockAdult + ? results.filter((r) => !('adult' in r && r.adult)) + : results;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/routes/search.ts` around lines 74 - 77, Pre-filtering currently checks 'adult' in r then casts to TmdbMovieResult which is misleading because TmdbPersonResult and TmdbCollectionResult also have adult; remove the cast and either use a union cast or a small type guard: replace (r as TmdbMovieResult).adult with (r as TmdbMovieResult | TmdbPersonResult | TmdbCollectionResult).adult or implement a hasAdult(r): r is { adult: boolean } type guard, and use that in the preFiltered computation (refer to variable preFiltered and the results array).src/components/UserList/BulkEditModal.tsx (1)
148-170: Consider showing a hint when selected users have different rating limits.The select dropdowns start empty, but selected users may have varying limits. While the
touchedFieldspattern prevents accidental overwrites, admins have no visibility into the current state of selected users' parental controls.For the permissions section, common values are computed and displayed. A similar approach (or at least a helper message) could improve UX.
This is a nice-to-have enhancement for future iteration, not a blocker.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/UserList/BulkEditModal.tsx` around lines 148 - 170, Add a UI hint for mixed max movie ratings similar to the permissions section by computing the common value across selected users (e.g., create a helper like computeCommonMaxMovieRating or reuse existing common-value logic) and use that to populate or annotate the select when currentMaxMovieRating is undefined and the field is not yet touched; update the select display logic around currentMaxMovieRating and the markTouched usage so that when values differ you show a helper message such as "Multiple values selected" (or the common rating if one exists) next to the select, and ensure getMovieRatingOptions() remains the option source while keeping markTouched('maxMovieRating') and setCurrentMaxMovieRating unchanged.server/routes/user/index.ts (1)
513-517: Clarify behavior: empty string clears the rating but validation logic differs.At line 464, validation skips if
maxMovieRatingis falsy (empty string). At line 514,settings.maxMovieRating = req.body.maxMovieRating || undefinedconverts empty string toundefined. This works but the coupling is implicit.The validation uses truthy check (
if (req.body.maxMovieRating && ...)) while the assignment uses|| undefined. Both handle empty string as "clear", but through different mechanisms.Consider adding a brief comment clarifying that empty string is intentionally used to clear ratings, or use explicit
=== ''checks for clarity.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/routes/user/index.ts` around lines 513 - 517, The code treats an empty string as a signal to clear ratings but does so inconsistently (validation uses a truthy check while assignment uses `|| undefined`), so make the behavior explicit: update the validation and assignment for `req.body.maxMovieRating` and `req.body.maxTvRating` to check for `''` (empty string) and `undefined` explicitly (e.g., if `req.body.maxMovieRating === ''` then set `settings.maxMovieRating = undefined`, else if `req.body.maxMovieRating !== undefined` validate and assign the provided value), and add a short comment near `settings.maxMovieRating`/`settings.maxTvRating` explaining that an empty string is intentionally used to clear the rating.
🤖 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/UserList/BulkEditModal.tsx`:
- Around line 194-223: The blockUnrated/blockAdult checkboxes (controlled by
currentBlockUnrated/currentBlockAdult and updated via
setCurrentBlockUnrated/setCurrentBlockAdult with markTouched) default to false
and can unintentionally overwrite differing user settings; compute and set an
initial/common/indeterminate state for these controls based on the selected
users (the same logic used for permissions) or switch to a
tri-state/indeterminate UI so the checkbox reflects "mixed" values, and only
call markTouched when the admin explicitly changes the value; update the
initialization logic where currentBlockUnrated/currentBlockAdult are derived and
the onChange handlers to respect and clear indeterminate state instead of
forcing false for all users.
---
Nitpick comments:
In `@server/routes/search.ts`:
- Around line 190-191: The parsed searchPage from req.query.page can be NaN,
zero or negative; after parsing Number(req.query.page) ensure it is a valid
integer >= 1 (e.g., use Number.parseInt/Math.floor and then clamp to at least 1)
before using it in the search flow so invalid values default to 1; update the
logic around searchPage (the variable assigned from req.query.page) to validate
and normalize the value defensively.
- Around line 74-77: Pre-filtering currently checks 'adult' in r then casts to
TmdbMovieResult which is misleading because TmdbPersonResult and
TmdbCollectionResult also have adult; remove the cast and either use a union
cast or a small type guard: replace (r as TmdbMovieResult).adult with (r as
TmdbMovieResult | TmdbPersonResult | TmdbCollectionResult).adult or implement a
hasAdult(r): r is { adult: boolean } type guard, and use that in the preFiltered
computation (refer to variable preFiltered and the results array).
In `@server/routes/user/index.ts`:
- Around line 513-517: The code treats an empty string as a signal to clear
ratings but does so inconsistently (validation uses a truthy check while
assignment uses `|| undefined`), so make the behavior explicit: update the
validation and assignment for `req.body.maxMovieRating` and
`req.body.maxTvRating` to check for `''` (empty string) and `undefined`
explicitly (e.g., if `req.body.maxMovieRating === ''` then set
`settings.maxMovieRating = undefined`, else if `req.body.maxMovieRating !==
undefined` validate and assign the provided value), and add a short comment near
`settings.maxMovieRating`/`settings.maxTvRating` explaining that an empty string
is intentionally used to clear the rating.
In `@src/components/UserList/BulkEditModal.tsx`:
- Around line 148-170: Add a UI hint for mixed max movie ratings similar to the
permissions section by computing the common value across selected users (e.g.,
create a helper like computeCommonMaxMovieRating or reuse existing common-value
logic) and use that to populate or annotate the select when
currentMaxMovieRating is undefined and the field is not yet touched; update the
select display logic around currentMaxMovieRating and the markTouched usage so
that when values differ you show a helper message such as "Multiple values
selected" (or the common rating if one exists) next to the select, and ensure
getMovieRatingOptions() remains the option source while keeping
markTouched('maxMovieRating') and setCurrentMaxMovieRating unchanged.
CodeRabbit Review — All Findings AddressedRound 1 (7 findings → all fixed in
|
| # | Finding | Severity | Resolution |
|---|---|---|---|
| 1 | BulkEditModal clears parental controls on permission-only edits | 🟠 Major | Added touchedFields tracking — parental control fields are only sent in the PUT payload when the admin actually modifies them |
| 2 | Keyword movies endpoint bypasses parental controls | 🟡 Minor | Added postFilterDiscoverMovies() call to /keyword/:keywordId/movies |
| 3 | Bulk edit missing admin user protection | 🟠 Major | Added MANAGE_USERS permission check — parental controls are skipped for admin users, matching the individual endpoint behavior in usersettings.ts |
| 4 | Search: unnecessary cert lookups when only blockAdult active |
Nitpick | Added early return in filterSearchBatch — skips expensive TMDB certification API calls when no rating/unrated constraints are set |
| 5 | TV filtering lacks international rating fallback | Nitpick | Added getTvCertFromDetails() with international fallback, matching the existing movie certification lookup behavior |
| 6 | contentRatings.ts comment inversion |
🟡 Minor | Fixed header comment: "Lower index = less restrictive" |
| 7 | filterRatio exceeds 1.0 after backfill |
🟡 Minor | Clamped with Math.min(1, filteredCount / originalCount) |
- BulkEditModal: fetch parental controls on mount, show indeterminate checkboxes and hint text when selected users have mixed values - search.ts: clamp searchPage to min 1, fix misleading adult type cast - user/index.ts: add clarifying comment for empty-string rating clearing
Round 2 — Fixed in
|
| # | Finding | Fix |
|---|---|---|
| 1 | Checkbox indeterminate state for mixed settings | BulkEditModal now fetches parental controls for all selected users on mount. Checkboxes show indeterminate (semi-filled) state when values differ, with hint text: "Enabled for some but not all users" |
| 2 | Mixed rating value hints | Select dropdowns show "Varies across selected users" when selected users have different rating limits |
| 3 | searchPage negative/zero validation |
Clamped with Math.max(1, ...) |
| 4 | Misleading adult cast |
Replaced (r as TmdbMovieResult).adult with (r as { adult?: boolean }).adult — no longer implies only movies have the field |
| 5 | Empty string rating clearing | Added clarifying comment: // Empty string = "No Restriction" from the UI dropdown; convert to undefined for DB storage |
Build and lint both pass clean.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
server/routes/search.ts (1)
74-77: Consider simplifying the adult type guard.The type cast is correct but verbose. A simpler approach using a type guard function could improve readability.
♻️ Optional simplification
- const preFiltered = blockAdult - ? results.filter((r) => !('adult' in r && (r as { adult?: boolean }).adult)) - : results; + const preFiltered = blockAdult + ? results.filter((r) => { + if ('adult' in r) { + return !(r as TmdbMovieResult | TmdbPersonResult | TmdbCollectionResult).adult; + } + return true; + }) + : results;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/routes/search.ts` around lines 74 - 77, The filter for preFiltered is using a verbose cast; introduce a small type guard like hasAdultFlag(value): value is { adult?: boolean } and then replace the predicate with results.filter(r => !(hasAdultFlag(r) && r.adult)) so you avoid the (r as { adult?: boolean }) cast while keeping the same logic; update the local symbols preFiltered, blockAdult, results to use the new hasAdultFlag guard in search.ts.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@server/routes/search.ts`:
- Around line 74-77: The filter for preFiltered is using a verbose cast;
introduce a small type guard like hasAdultFlag(value): value is { adult?:
boolean } and then replace the predicate with results.filter(r =>
!(hasAdultFlag(r) && r.adult)) so you avoid the (r as { adult?: boolean }) cast
while keeping the same logic; update the local symbols preFiltered, blockAdult,
results to use the new hasAdultFlag guard in search.ts.
- Dropdowns show "Restrictions vary between selected users" as selected placeholder when users have different rating limits - Replace inline adult cast with hasAdultFlag() type guard
Follow-up fix (
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/UserList/BulkEditModal.tsx`:
- Around line 92-151: Reset the "mixed" flags and touched fields whenever the
selection changes to avoid stale UI state: at the start of fetchParentalControls
(or in a useEffect watching selectedUserIds) call the setters to clear mixed
flags (setMixedMovieRating(false), setMixedTvRating(false),
setMixedBlockUnrated(false), setMixedBlockAdult(false)) and reset touchedFields
via setTouchedFields({}) (and optionally clear current values like
setCurrentMaxMovieRating(undefined) / setCurrentMaxTvRating(undefined) if you
want a clean initial state) before fetching; this ensures previous mixed/touched
indicators don't persist across selection changes.
Merge Conflict ResolutionUpstream What changed upstream:
How we resolved it:
Build passes clean ( |
|
Nice work man, I'll pull it tonight at some point and test it again. |
|
Looks good — As mentioned previously, a handful of pages remain sparse due to filtering, but with the API limit in mind, this is fine. |
…ng gaps Server-side content filtering can consume multiple TMDB pages per request during backfill, causing the frontend to re-request already-consumed pages and skip content. All paginated endpoints now return a `nextPage` field so the frontend requests the correct next page. TMDB's API has no `certification.lte` support for TV discover, so every TV show requires an individual detail fetch for its content rating. With multiple sliders loading simultaneously (6-10 concurrent route handlers), each firing 20+ parallel cert lookups, TMDB rate limits were hit regularly — up to 9/17 TV lookups failing per batch. Added: - Global AIMD concurrency limiter for cert lookups (starts at 8 parallel, halves on 429, +1 on success, bounds 2-20, retries 2x with backoff) - Shared in-memory certification cache across all callers - blockAdult pre-filtering in filterMovieBatch, filterTvBatch, and filterTvListByRating (previously only movie list filters checked this) Fixed: - ratingCheck.ts was making direct TMDB calls, bypassing both the cert cache and the throttle — switched to getCachedMovieCert/getCachedTvCert - Watchlist TV branch missing blockAdult guard - Removed dead variables (originalCount, filteredCount) and stale imports
Major Update: Pagination, Rate Limiting & Filtering Overhaul (85cb67f)10 files changed, +645/-443 lines — this is a significant rework of how content filtering interacts with TMDB's API and how paginated results flow to the frontend. BackgroundTesting parental controls under strict settings (PG / TV-G) exposed fundamental issues with how filtering, pagination, and TMDB API calls work together:
What ChangedCursor-based pagination (5 server files + 2 frontend files)All paginated endpoints ( Global AIMD concurrency limiter (
|
It was bothering me how sparse everything looked, you should see significant improvement in the results and behavior and now infinite scroll works as intended. Thanks for bringing it up, seeing it bother you made me rework the entire process for the better. |
|
This pull request has merge conflicts. Please resolve the conflicts so the PR can be successfully reviewed and merged. |
Combine upstream Error→ErrorPage rename with our 403 status code handling in CollectionDetails and TvDetails. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
ProgenyAlpha
force-pushed
the
feature/parental-controls
branch
from
fc52266 to
5db9b83
Compare
github-actions
bot
removed
the
merge conflict
|
Merge conflicts resolved and build verified. Ready for review. @fallenbagel @OwsleyJr — would appreciate a review when you get a chance. |
|
This pull request has merge conflicts. Please resolve the conflicts so the PR can be successfully reviewed and merged. |
Upstream changed getRelatedMedia() signature to accept objects with tmdbId + mediaType instead of plain IDs. Resolved by using our filteredResults (parental controls filtering) with the new object format.
|
Merge conflicts with Ready for review when you get a chance 🙏 |
github-actions
bot
removed
the
merge conflict
|
Thank you for working on this! I'm really excited for it! |
|
Hey, is this due to be pushed to a stable version anytime soon? |
Not until it's properly reviewed, which is not. |
Remove unused catch parameter in UserParentalControlsSettings and run i18n:extract to sync translation strings.
Not trying to be pushy. I'm genuinely curious, what does properly reviewed entail? |
Us maintainers reviewing the code for its quality, architectural design, maintainability etc. |
8 participants
Description
Adds admin-enforced per-user content rating limits (parental controls). Admins can set maximum movie (MPAA) and TV (US Parental Guidelines) ratings per user, plus a "block unrated" toggle for fail-closed filtering. Restricted users see no indication that filtering exists.
Screenshot
Admin view: User > Settings > Parental Controls
How It Works
Admin Sets Limits
A new Parental Controls tab appears in user settings (admin-only — restricted users cannot see or modify their own limits). Admins choose:
Limits are stored as new columns on
UserSettings(maxMovieRating,maxTvRating,blockUnrated) with TypeORM migrations for both PostgreSQL and SQLite.Discover Filtering (Two-Layer)
certification.lte/certification_countryquery params remove rated content above the limit at the API level — no extra requests neededblockUnratedusers, results that slipped through (unrated content TMDB doesn't filter) are caught server-side by fetching each item's US certification and checking against the hierarchySearch Filtering
Search results don't support TMDB's certification params, so all filtering is server-side. Certifications are fetched in parallel via
Promise.allSettledfor each result, then filtered against the user's limits.Backfill
When post-filtering drops a page below 15 results, the next TMDB page is automatically fetched and filtered to prevent sparse/empty pages. This applies to both discover and search routes.
Fail-Closed Design
blockUnratedis true, allowed when falseRating Hierarchies
Single source of truth in
server/constants/contentRatings.ts:Scoped to US ratings — TMDB's certification data is most complete for the US market.
Files Changed
server/constants/contentRatings.tsserver/entity/UserSettings.tsmaxMovieRating,maxTvRating,blockUnratedserver/migration/server/routes/discover.tscertificationLte, post-filter for unrated, backfillserver/routes/search.tsserver/routes/user/usersettings.tssrc/.../UserParentalControlsSettings/src/i18n/locale/en.jsonTesting
blockUnratedhides content without US certificationpnpm buildpasses clean (bothbuild:nextandbuild:server)AI Disclosure
This PR was developed with Claude Code (Claude Opus 4.6), with human review, testing, and architectural direction at every step. All code was verified against existing codebase patterns and deployed before submission.
Checklist
pnpm buildpnpm i18n:extractSummary by CodeRabbit