feat: Add media removal request system with approval workflow, per-season support, and granular permissions#2828
feat: Add media removal request system with approval workflow, per-season support, and granular permissions#2828MrDWilson wants to merge 27 commits into
Conversation
|
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:
📝 WalkthroughWalkthroughThis pull request adds a comprehensive media removal request system, enabling users to request removal of media from Radarr/Sonarr through Seerr, with optional admin approval, auto-approval for privileged users, and multi-user consent logic for shared removal decisions. ChangesMedia Removal Request System
Sequence DiagramsequenceDiagram
participant User
participant ManageSlideOver
participant API as /api/v1/removal-request
participant Route as removalRequest handler
participant MediaRemovalRequest
participant RadarrSonarr as Radarr/Sonarr
participant Database
User->>ManageSlideOver: Click "Request Removal"
ManageSlideOver->>ManageSlideOver: Open RemovalRequestSection
User->>ManageSlideOver: Select seasons (TV) / confirm
ManageSlideOver->>API: POST {mediaId, is4k, seasons?}
API->>Route: Handle POST /removal-request
Route->>Route: Validate permissions & media
Route->>MediaRemovalRequest: Create & auto-approve check
alt shouldAutoApprove
MediaRemovalRequest->>MediaRemovalRequest: executeRemoval()
MediaRemovalRequest->>RadarrSonarr: removeMovie/removeSeries/removeSeasonFiles
RadarrSonarr-->>MediaRemovalRequest: Deletion/cleanup complete
MediaRemovalRequest->>Database: Update removal request status
else Manual approval needed
MediaRemovalRequest->>Database: Create with PENDING status
end
Database-->>Route: Removal request saved
Route-->>API: 201 Created
API-->>ManageSlideOver: Response
ManageSlideOver->>ManageSlideOver: Show success toast
ManageSlideOver->>ManageSlideOver: Close modal & revalidate
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested Reviewers
🚥 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 |
There was a problem hiding this comment.
Pull request overview
Adds a full media-removal request workflow to Seerr (server + UI + API spec), including per-season TV support, approval/retry/decline lifecycle, and new permission bits (migrated to bigint + BigInt-based checks) to safely support >32-bit permission flags.
Changes:
- Introduces
MediaRemovalRequestdata model, migrations (SQLite/Postgres), and/removal-requestAPI routes for create/list/approve/decline/retry/delete. - Extends UI to create/manage removal requests (manage slide-over, request list, per-season modal, status blocks) and adds i18n strings.
- Adds Sonarr per-season file removal support and expands permissions + permission editor UI.
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| src/i18n/locale/en.json | Adds new UI strings for removal-request workflow and permissions. |
| src/components/TvDetails/index.tsx | Expands “manage” entrypoint visibility to removal-request-capable users (scoped). |
| src/components/RequestModal/SeasonRemovalModal.tsx | New modal for selecting seasons to request removal. |
| src/components/RequestList/index.tsx | Shows a “Pending Removal Requests” section for managers. |
| src/components/RemovalRequestBlock/index.tsx | New UI block for displaying/remediating removal requests (approve/decline/retry/delete). |
| src/components/PermissionOption/index.tsx | Adds childrenAutoCheck support and includes new auto-approve removal permission in logic. |
| src/components/PermissionEdit/index.tsx | Adds new removal permissions to the permission editor with hierarchy/requirements. |
| src/components/MovieDetails/index.tsx | Expands “manage” entrypoint visibility to removal-request-capable users (scoped). |
| src/components/ManageSlideOver/index.tsx | Adds removal request section, per-season flow, and displays removal request history. |
| server/routes/removalRequest.ts | New removal request API endpoints (list/create/approve/decline/retry/delete) with validations and execution. |
| server/routes/index.ts | Registers /removal-request routes. |
| server/migration/sqlite/1775397567178-AddMediaRemovalRequest.ts | SQLite migration adding media_removal_request table and migrating user.permissions to bigint. |
| server/migration/postgres/1775397575694-AddMediaRemovalRequest.ts | Postgres migration adding media_removal_request table and migrating user.permissions to bigint. |
| server/lib/permissions.ts | Adds new permission bits and refactors hasPermission() to use BigInt bitwise ops. |
| server/entity/User.ts | Changes permissions column to bigint with transformer. |
| server/entity/MediaRemovalRequest.ts | New entity encapsulating approval state and execution logic for removals (Radarr/Sonarr + Seerr DB updates). |
| server/entity/Media.ts | Adds relation to removal requests and includes them in getMedia() relations. |
| server/constants/media.ts | Adds MediaRemovalRequestStatus enum. |
| server/api/servarr/sonarr.ts | Adds removeSeasonFiles() to enable per-season TV removal behavior. |
| seerr-api.yml | Documents the new /removal-request endpoints in the OpenAPI spec. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (6)
src/components/TvDetails/index.tsx (1)
697-733: Permission logic is correct but has minor redundancy.The nested permission checks work correctly:
- First condition gates on
MANAGE_REQUESTSORREQUEST_REMOVAL- Second condition further restricts
REQUEST_REMOVALusers to those withREMOVAL_ALLor who own existing requestsHowever, checking
hasPermission(Permission.ADMIN)andhasPermission(Permission.MANAGE_REQUESTS)in the second condition is redundant since:
ADMINusers already pass allhasPermissioncalls (including the first one)MANAGE_REQUESTSusers already satisfied the first condition and would satisfy the second via the explicit checkThis redundancy doesn't cause bugs but could be simplified:
♻️ Optional: Simplify redundant permission checks
- {hasPermission( - [Permission.MANAGE_REQUESTS, Permission.REQUEST_REMOVAL], - { type: 'or' } - ) && - (hasPermission(Permission.REMOVAL_ALL) || - hasPermission(Permission.MANAGE_REQUESTS) || - hasPermission(Permission.ADMIN) || - data.mediaInfo?.requests?.some( - (r) => r.requestedBy.id === user?.id - )) && - data.mediaInfo && ( + {(hasPermission(Permission.MANAGE_REQUESTS) || + (hasPermission(Permission.REQUEST_REMOVAL) && + (hasPermission(Permission.REMOVAL_ALL) || + data.mediaInfo?.requests?.some( + (r) => r.requestedBy.id === user?.id + )))) && + data.mediaInfo && (🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/TvDetails/index.tsx` around lines 697 - 733, Remove the redundant permission checks in the nested condition inside the TvDetails component: keep the outer guard hasPermission([Permission.MANAGE_REQUESTS, Permission.REQUEST_REMOVAL], { type: 'or' }) and simplify the inner condition to only check hasPermission(Permission.REMOVAL_ALL) || data.mediaInfo?.requests?.some(r => r.requestedBy.id === user?.id); remove the extra hasPermission(Permission.ADMIN) and hasPermission(Permission.MANAGE_REQUESTS) checks so the rendered Tooltip/Button logic still requires either removal-all permission or ownership of an existing request.seerr-api.yml (1)
6275-6287: Document these responses with a concreteMediaRemovalRequestschema.Right now the list items and create/approve/decline/retry responses are all opaque
objects, so the spec never tells clients about fields likestatus,requestedBy,is4k,seasons, or timestamps. Add a reusable schema here and reference it from these operations so the new API surface is actually typed and much harder to drift from the server.Also applies to: 6318-6325, 6340-6345, 6360-6365, 6380-6385
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@seerr-api.yml` around lines 6275 - 6287, Replace the opaque inline object responses for the removal endpoints with a reusable MediaRemovalRequest schema: change the '200' response's content -> application/json -> schema -> properties -> results -> items to reference '#/components/schemas/MediaRemovalRequest' (instead of type: object) and update the create/approve/decline/retry responses similarly; add a concrete components.schemas.MediaRemovalRequest definition that enumerates fields like id, status, requestedBy, is4k, seasons, createdAt, updatedAt, etc., and reference that schema for all affected response bodies (also update the same pattern at the other response blocks noted in the review: the ranges around 6318-6325, 6340-6345, 6360-6365, 6380-6385) so clients receive typed responses.server/entity/MediaRemovalRequest.ts (1)
116-121: Redundant TMDB API call whentvdbIdalready exists.Consider checking
media.tvdbIdfirst to avoid the TMDB API call when the ID is already available:♻️ Proposed optimization
} else { - const tmdb = new TheMovieDb(); - const series = await tmdb.getTvShow({ tvId: media.tmdbId }); - const tvdbId = series.external_ids.tvdb_id ?? media.tvdbId; + let tvdbId = media.tvdbId; + if (!tvdbId) { + const tmdb = new TheMovieDb(); + const series = await tmdb.getTvShow({ tvId: media.tmdbId }); + tvdbId = series.external_ids.tvdb_id; + } if (!tvdbId) { throw new Error('TVDB ID not found'); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/entity/MediaRemovalRequest.ts` around lines 116 - 121, Avoid the redundant TMDB API call by using media.tvdbId first: check if media.tvdbId exists before instantiating TheMovieDb or calling getTvShow; only create const tmdb and call tmdb.getTvShow({ tvId: media.tmdbId }) to derive tvdbId when media.tvdbId is missing, then set tvdbId accordingly and preserve the existing error throw when no tvdbId is found. Reference symbols: media.tvdbId, TheMovieDb, tmdb, getTvShow, series, tvdbId.server/migration/sqlite/1775397567178-AddMediaRemovalRequest.ts (2)
6-91: Redundant table rebuilds in up() migration.The migration rebuilds
user_push_subscriptiontwice (lines 8-20 and 44-57) andusertable twice (lines 21-28 and 58-65). The first rebuild of each table doesn't change the schema - the actualpermissionscolumn change fromintegertobigintonly happens in the second rebuild (line 59).This doubles the migration time and data copying unnecessarily. Consider consolidating to a single rebuild per table.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/migration/sqlite/1775397567178-AddMediaRemovalRequest.ts` around lines 6 - 91, The up() migration performs duplicate rebuilds of user_push_subscription and user (creating temporary_user_push_subscription/temporary_user twice and temporary_user/temporary_user twice) causing unnecessary data copies; remove the first set of DROP/CREATE/INSERT/DROP/ALTER statements for user_push_subscription and user and keep the single rebuild that introduces the permissions change to bigint (the block that creates temporary_user with permissions bigint), ensuring you still recreate the IDX_03f7958328e311761b0de675fb index and preserve the foreign key constraints on temporary_user_push_subscription and any index recreation for user_push_subscription/user.
143-147: Duplicate DROP INDEX statements in down() migration.Lines 95-98 already drop these same indexes (
IDX_34c6963994828cb30c9b2798df,IDX_148182cef7f27b27b1fdacd7de,IDX_78decd4e1901d80cfdce43b079,IDX_64e0da8892d7f8aabce7198097), then lines 109-120 recreate them, and lines 143-146 try to drop them again before dropping the table at line 147.This appears to work but is unnecessarily complex. The intermediate recreation (lines 99-120) that strips FKs from
media_removal_requestbefore finally dropping it could be simplified to just dropping the table directly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/migration/sqlite/1775397567178-AddMediaRemovalRequest.ts` around lines 143 - 147, The down() method in the AddMediaRemovalRequest migration contains duplicated DROP INDEX calls and an unnecessary recreate-block; simplify it by removing the intermediate recreation and the duplicate DROP INDEX statements so that down() either drops the indexes once (using the existing index names IDX_34c6963994828cb30c9b2798df, IDX_148182cef7f27b27b1fdacd7de, IDX_78decd4e1901d80cfdce43b079, IDX_64e0da8892d7f8aabce7198097) then calls queryRunner.query(`DROP TABLE "media_removal_request"`) or, if the DB will cascade, simply call DROP TABLE "media_removal_request" directly; update the queryRunner.query calls inside the AddMediaRemovalRequest.down() method to remove the duplicated drops and the recreate block so the migration is concise and only drops each index/table once.server/routes/removalRequest.ts (1)
120-132: Consider validating season 0 (specials) consistently with UI.The API accepts season 0 (
s >= 0), butSeasonRemovalModalexplicitly filters out season 0 (specials). If the intention is to disallow removal of specials, consider addings > 0validation here. Otherwise, document that season 0 removal is supported via direct API calls but not through the UI.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/routes/removalRequest.ts` around lines 120 - 132, The current seasons validation in removalRequest.ts allows season 0 (checks s < 0); to match the UI's SeasonRemovalModal which excludes specials, change the validation predicate so seasons.some((s) => typeof s !== 'number' || !Number.isInteger(s) || s <= 0) and update the error message to "Seasons must be an array of positive integers." Ensure this check lives in the same request handler that references the seasons variable so API rejects season 0 consistently.
🤖 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/api/servarr/sonarr.ts`:
- Around line 440-460: The code deletes episode files and updates
series.seasons.monitored but fails to unmonitor individual episodes first, which
can allow Sonarr to re-import missing files; modify the flow in this block so
that after computing episodeFileIds (from getEpisodes) you explicitly set the
affected episodes' monitored flag to false (use the same episode list filtered
by episodeFileId/seasonNumbers) by calling the existing monitorEpisodes or
equivalent API to update those episodes' monitored state before calling
axios.delete for each episodeFileId, then proceed to update series via
axios.put; reference functions/fields: getEpisodes, episodeFileId,
monitorEpisodes, axios.delete, axios.put, and series.seasons in your changes.
In `@server/entity/MediaRemovalRequest.ts`:
- Around line 127-131: The call to service.removeSeasonFiles(tvdbId,
this.seasons!) can partially succeed (files deleted but unmonitor PUT failed)
which leaves Sonarr state inconsistent and the DB transaction aborted; wrap the
call in a try/catch in MediaRemovalRequest (the branch that currently calls
removeSeasonFiles/removeSeries) and handle partial failures by logging a
structured, descriptive message including tvdbId and seasons and the partial
outcome; preferably change removeSeasonFiles to return a status object (or throw
a PartialFailureError) indicating steps completed (e.g., { filesDeleted: true,
unmonitorSucceeded: false }) and, in MediaRemovalRequest, detect that status and
emit a clear warning/error log so retries/ops can observe the partial state
before the DB transaction is aborted.
- Around line 72-77: The updatedAt field in MediaRemovalRequest is using
`@DbAwareColumn` with an unsupported onUpdate option so it never auto-updates;
change the decorator to TypeORM's `@UpdateDateColumn`() on the updatedAt property,
add UpdateDateColumn to the import list (alongside
Column/Entity/PrimaryGeneratedColumn/etc.), and remove the onUpdate/default
options from the current `@DbAwareColumn` usage so updatedAt is managed
automatically by TypeORM; keep the property type as Date and ensure no manual
setting is required elsewhere in MediaRemovalRequest.
In `@server/migration/postgres/1775397575694-AddMediaRemovalRequest.ts`:
- Around line 47-48: The down migration is unsafe because PostgreSQL integer is
32-bit and the new permission constants AUTO_APPROVE_REMOVAL (2147483648) and
REMOVAL_ALL (4294967296) can exceed its range; update the migration file
AddMediaRemovalRequest.ts so the down() path clears/masks these high bits on the
"permissions" column before casting back to integer (e.g. run an UPDATE to
remove AUTO_APPROVE_REMOVAL and REMOVAL_ALL bits from "permissions"), then
perform the ALTER TABLE "user" ALTER COLUMN "permissions" TYPE integer;
alternatively, if you prefer irreversible down(), explicitly throw/mark the down
migration as non-reversible in the same file.
In `@src/components/ManageSlideOver/index.tsx`:
- Around line 131-139: availableSeasons is currently computed and passed to
SeasonRemovalModal only for non-4K (is4k=false), so the UI never surfaces
4K-only season removal; update the logic that builds availableSeasons (and the
code paths around lines referenced) to compute two sets or a parameterized set
depending on an is4k flag on the modal, and pass is4k={true} when the user
toggles or selects the 4K branch so SeasonRemovalModal can show 4K seasons;
specifically modify the branch that inspects mediaType and
data.mediaInfo?.seasons to include seasons whose status/is4k match the requested
removal mode, and ensure the SeasonRemovalModal component invocation
(SeasonRemovalModal) receives the computed availableSeasons and the correct is4k
prop for the 4K UI path.
In `@src/components/RemovalRequestBlock/index.tsx`:
- Around line 171-180: The delete button is shown for all rows even though
server-side (server/routes/removalRequest.ts) only allows the requester or users
with the MANAGE_REQUESTS role plus Permission.REQUEST_REMOVAL to delete; update
RemovalRequestBlock to only render the Tooltip/Button when the current user is
allowed: check that either currentUser.id === request.requesterId (or
request.requester?.id) OR the current user has the MANAGE_REQUESTS
role/Permission.REQUEST_REMOVAL (use your existing permission/role helper or
prop that indicates permissions), and only call deleteRequest() when that
conditional passes. This prevents rendering a button that will always 403 and
keeps deleteRequest as the click handler for authorized cases.
- Around line 136-154: The icon-only buttons (approve/decline and the delete
button) lack accessible names for screen readers; update the Button usages that
render CheckIcon, XMarkIcon (and the delete button at the other block) to
include accessible labels by either adding an aria-label prop with the localized
string (e.g. aria-label={intl.formatMessage(messages.approve)} /
messages.decline / messages.delete) or inserting a visually-hidden text node
(e.g. a span with the sr-only class containing intl.formatMessage(...)) inside
the Button; target the Button components wrapping CheckIcon, XMarkIcon and the
delete icon and use updateRequest and the existing messages keys for the label
text.
In `@src/components/RequestModal/SeasonRemovalModal.tsx`:
- Around line 122-125: The onKeyDown handler is checking for the wrong Space key
string ("Space") so pressing the spacebar won't trigger toggleAllSeasons; update
the condition in the onKeyDown callback (where toggleAllSeasons() is called) to
detect space correctly by checking e.key === ' ' (a single space character) or
alternatively check e.code === 'Space' for robustness, then call
toggleAllSeasons() when either condition is true.
- Around line 182-186: The per-row keyboard handler uses e.key === 'Space' which
is unreliable; update the onKeyDown handler to detect the Space key robustly
(e.g., check e.code === 'Space' in addition to Enter) and call
e.preventDefault() when handling Space/Enter before invoking
toggleSeason(season.seasonNumber) so the action triggers consistently without
causing page scrolling; modify the onKeyDown arrow function that currently
references toggleSeason and season.seasonNumber accordingly.
---
Nitpick comments:
In `@seerr-api.yml`:
- Around line 6275-6287: Replace the opaque inline object responses for the
removal endpoints with a reusable MediaRemovalRequest schema: change the '200'
response's content -> application/json -> schema -> properties -> results ->
items to reference '#/components/schemas/MediaRemovalRequest' (instead of type:
object) and update the create/approve/decline/retry responses similarly; add a
concrete components.schemas.MediaRemovalRequest definition that enumerates
fields like id, status, requestedBy, is4k, seasons, createdAt, updatedAt, etc.,
and reference that schema for all affected response bodies (also update the same
pattern at the other response blocks noted in the review: the ranges around
6318-6325, 6340-6345, 6360-6365, 6380-6385) so clients receive typed responses.
In `@server/entity/MediaRemovalRequest.ts`:
- Around line 116-121: Avoid the redundant TMDB API call by using media.tvdbId
first: check if media.tvdbId exists before instantiating TheMovieDb or calling
getTvShow; only create const tmdb and call tmdb.getTvShow({ tvId: media.tmdbId
}) to derive tvdbId when media.tvdbId is missing, then set tvdbId accordingly
and preserve the existing error throw when no tvdbId is found. Reference
symbols: media.tvdbId, TheMovieDb, tmdb, getTvShow, series, tvdbId.
In `@server/migration/sqlite/1775397567178-AddMediaRemovalRequest.ts`:
- Around line 6-91: The up() migration performs duplicate rebuilds of
user_push_subscription and user (creating
temporary_user_push_subscription/temporary_user twice and
temporary_user/temporary_user twice) causing unnecessary data copies; remove the
first set of DROP/CREATE/INSERT/DROP/ALTER statements for user_push_subscription
and user and keep the single rebuild that introduces the permissions change to
bigint (the block that creates temporary_user with permissions bigint), ensuring
you still recreate the IDX_03f7958328e311761b0de675fb index and preserve the
foreign key constraints on temporary_user_push_subscription and any index
recreation for user_push_subscription/user.
- Around line 143-147: The down() method in the AddMediaRemovalRequest migration
contains duplicated DROP INDEX calls and an unnecessary recreate-block; simplify
it by removing the intermediate recreation and the duplicate DROP INDEX
statements so that down() either drops the indexes once (using the existing
index names IDX_34c6963994828cb30c9b2798df, IDX_148182cef7f27b27b1fdacd7de,
IDX_78decd4e1901d80cfdce43b079, IDX_64e0da8892d7f8aabce7198097) then calls
queryRunner.query(`DROP TABLE "media_removal_request"`) or, if the DB will
cascade, simply call DROP TABLE "media_removal_request" directly; update the
queryRunner.query calls inside the AddMediaRemovalRequest.down() method to
remove the duplicated drops and the recreate block so the migration is concise
and only drops each index/table once.
In `@server/routes/removalRequest.ts`:
- Around line 120-132: The current seasons validation in removalRequest.ts
allows season 0 (checks s < 0); to match the UI's SeasonRemovalModal which
excludes specials, change the validation predicate so seasons.some((s) => typeof
s !== 'number' || !Number.isInteger(s) || s <= 0) and update the error message
to "Seasons must be an array of positive integers." Ensure this check lives in
the same request handler that references the seasons variable so API rejects
season 0 consistently.
In `@src/components/TvDetails/index.tsx`:
- Around line 697-733: Remove the redundant permission checks in the nested
condition inside the TvDetails component: keep the outer guard
hasPermission([Permission.MANAGE_REQUESTS, Permission.REQUEST_REMOVAL], { type:
'or' }) and simplify the inner condition to only check
hasPermission(Permission.REMOVAL_ALL) || data.mediaInfo?.requests?.some(r =>
r.requestedBy.id === user?.id); remove the extra hasPermission(Permission.ADMIN)
and hasPermission(Permission.MANAGE_REQUESTS) checks so the rendered
Tooltip/Button logic still requires either removal-all permission or ownership
of an existing request.
🪄 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: a856ee6b-0247-42b2-ac9a-d17b0024d497
📒 Files selected for processing (20)
seerr-api.ymlserver/api/servarr/sonarr.tsserver/constants/media.tsserver/entity/Media.tsserver/entity/MediaRemovalRequest.tsserver/entity/User.tsserver/lib/permissions.tsserver/migration/postgres/1775397575694-AddMediaRemovalRequest.tsserver/migration/sqlite/1775397567178-AddMediaRemovalRequest.tsserver/routes/index.tsserver/routes/removalRequest.tssrc/components/ManageSlideOver/index.tsxsrc/components/MovieDetails/index.tsxsrc/components/PermissionEdit/index.tsxsrc/components/PermissionOption/index.tsxsrc/components/RemovalRequestBlock/index.tsxsrc/components/RequestList/index.tsxsrc/components/RequestModal/SeasonRemovalModal.tsxsrc/components/TvDetails/index.tsxsrc/i18n/locale/en.json
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/migration/postgres/1775397575694-AddMediaRemovalRequest.ts`:
- Around line 25-27: The FK on media_removal_request currently uses ON DELETE
CASCADE which causes removal requests to vanish when Media is deleted; update
the ALTER TABLE statement that creates constraint
"FK_78decd4e1901d80cfdce43b079f" (the FK on "mediaId" referencing "media"("id"))
so it does not cascade on delete (use ON DELETE NO ACTION or RESTRICT, or ON
DELETE SET NULL if "mediaId" is nullable) to preserve the MediaRemovalRequest
rows after Media deletion; change the SQL in AddMediaRemovalRequest migration
accordingly and keep the FK name and referenced columns the same.
In `@server/routes/removalRequest.ts`:
- Around line 127-139: The seasons validation currently allows an empty array
and doesn't verify selected seasons exist on the target media; update the
validation in removalRequest.ts so that when seasons is provided it must be a
non-empty array of positive integers and every season id must match an existing
season on the loaded media. Specifically: before you build the removal request,
load the media with its seasons (use the same media-loading codepath already
used later), then replace the loose check around the seasons variable with:
reject if !Array.isArray(seasons) or seasons.length === 0 or any element is not
a positive integer, and reject if any season id is not found in media.seasons
(call next({status:400, message:...}) on failure). Apply the same stricter check
to the other seasons validation sites referenced (the blocks around the earlier
141-155 range and the later logic that currently rewrites [] to undefined) and
ensure you do not coerce an empty array into undefined.
In `@src/components/ManageSlideOver/index.tsx`:
- Around line 350-366: The requestRemoval function swallows every 409 and treats
them as "already pending"; instead, only handle the specific duplicate-pending
409 and let other 409 conflicts bubble up. Update requestRemoval (the axios.post
call and its catch) to inspect e.response?.data (e.g., a specific error
code/property such as data.code | data.type | data.reason) and only suppress the
error and proceed to revalidate()/onClose() when that value matches the server's
duplicate-pending identifier; for any other 409 or unknown response shape
rethrow the error. Also move revalidate() and onClose() into the
successful/suppressed branch so they only run when the request truly succeeded
or is the known duplicate-pending case.
🪄 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: 18225dd5-bcf1-4b7b-84ca-d82fed89a7e9
📒 Files selected for processing (8)
server/api/servarr/sonarr.tsserver/entity/MediaRemovalRequest.tsserver/entity/User.tsserver/migration/postgres/1775397575694-AddMediaRemovalRequest.tsserver/routes/removalRequest.tssrc/components/ManageSlideOver/index.tsxsrc/components/RemovalRequestBlock/index.tsxsrc/components/RequestModal/SeasonRemovalModal.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- server/entity/User.ts
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
server/routes/removalRequest.ts (1)
131-147:⚠️ Potential issue | 🔴 CriticalValidate season requests against loaded seasons before they can execute.
seasons: []still passes validation and is rewritten toundefinedat Line 230, which can silently promote a season removal into a full-media removal. Also, these season-removal paths never loadmedia.seasons, butMediaRemovalRequest.executeRemoval()uses that relation to update Seerr season status, so an approved season request can remove files externally without marking the matching local seasons deleted.Also applies to: 226-230, 279-281, 365-367
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/routes/removalRequest.ts` around lines 131 - 147, The seasons validation currently allows an empty array which later gets coerced to undefined and can turn a season-only removal into a full-media removal; update the validation around mediaRepository.findOne so that seasons must be a non-empty array of positive integers (reject []), do not rewrite seasons to undefined, and when handling season-specific removals load the Media.seasons relation (e.g., via mediaRepository.findOne with relations) so MediaRemovalRequest.executeRemoval() has the loaded media.seasons to update local season status; apply the same fix to the other validation sites referenced (around the logic at lines handling seasons → 226-230, 279-281, 365-367).
🧹 Nitpick comments (2)
seerr-api.yml (1)
6287-6294: Define a concrete removal-request schema instead of anonymous objects.All of the new responses here are just
type: object, so generated clients lose fields likestatus,is4k,seasons,reason,media,requestedBy, andmodifiedBy. Please add a reusableMediaRemovalRequestschema plus a paginated list wrapper and reference that from these endpoints.Also applies to: 6331-6332, 6355-6356, 6375-6376, 6395-6396
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@seerr-api.yml` around lines 6287 - 6294, Create a concrete MediaRemovalRequest schema (include fields status, is4k, seasons, reason, media, requestedBy, modifiedBy and their appropriate types) and then define a paginated wrapper schema (e.g., MediaRemovalRequestList) that has pageInfo: $ref '#/components/schemas/PageInfo' and results: type: array, items: $ref '#/components/schemas/MediaRemovalRequest'; replace the anonymous object usages under results in the affected response definitions (the blocks currently using type: object) with $ref to MediaRemovalRequestList so all endpoints reference the reusable schemas instead of anonymous objects.server/routes/removalRequest.ts (1)
20-27: Let the OpenAPI layer own query validation here.
take/skipare being clamped locally and unknownfiltervalues fall back to"all", which silently rewrites invalid requests instead of rejecting them. This router family intentionally relies on the upstream OpenAPI validator for query enforcement, so it would be better to keep only defaults for omitted values and read the validated params directly.Based on learnings: In server/routes/**/*.ts, rely on upstream OpenAPI validator middleware for query parameters (e.g., page). Route handlers should not defensively validate or clamp query parameter values, as this is intentional and aligns with the project convention.
Also applies to: 31-51
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/routes/removalRequest.ts` around lines 20 - 27, This code clamps and silently rewrites query params (rawTake/rawSkip -> pageSize/skip and defaulting invalid filter to "all"); instead let the OpenAPI validator enforce types/limits and read the validated values directly from req.query, only providing defaults when a param is omitted. Remove the Number.isFinite checks, Math.floor/clamping logic and any fallback-for-invalid behavior in the handler (variables: rawTake, rawSkip, pageSize, skip, and filter), and simply use the values supplied by the OpenAPI-validated req.query (or apply an explicit default only if the query key is undefined).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@seerr-api.yml`:
- Around line 6251-6252: Update the OpenAPI description for the GET
removal-requests endpoint to match the handler logic in
server/routes/removalRequest.ts by including the REQUEST_VIEW permission as
allowed to list/filter removal requests globally (in addition to ADMIN and
MANAGE_REQUESTS); locate the GET removal requests operation description entries
(the two places corresponding to the earlier and later description blocks) and
change the text to state that users with ADMIN, MANAGE_REQUESTS, or REQUEST_VIEW
can return all removal requests, while other users only see their own.
In `@server/routes/removalRequest.ts`:
- Around line 313-319: The catch-all catch blocks in the approve/retry/delete
handlers (where logger.error is called and next({ status: 404, message: 'Removal
request not found.' }) is invoked) incorrectly convert any failure into a 404;
update each catch to detect a true not-found (e.g., check for a specific
NotFoundError type, error.name === 'NotFoundError', or error.status === 404) and
only call next({ status: 404, message: 'Removal request not found.' }) in that
case; for all other errors log the full error (using logger.error with the error
object) and propagate a 500 by calling next(err) or next({ status: 500, message:
'Internal server error.' }) so save/remove/runtime failures aren’t masked.
- Around line 183-189: The de-duplication is not atomic: replace the separate
find + insert/approve + executeRemoval flow with an atomic DB operation (use a
transaction or a unique partial index plus upsert) so two submissions cannot
pass the check simultaneously; specifically, modify the code paths that use
removalRequestRepository.find and then create/approve (the blocks around
removalRequestRepository, executeRemoval, and
MediaRemovalRequestStatus.PENDING/APPROVED) to either (a) run inside
removalRequestRepository.manager.transaction and acquire a row/table lock
(SELECT ... FOR UPDATE) to check for existing PENDING/APPROVED requests and then
insert/update and call executeRemoval, or (b) add a unique partial index on
(media_id, is4k) for relevant statuses and perform an insert with ON CONFLICT
handling (upsert) to ensure only one request is created/approved before
executeRemoval; apply the same change to the other occurrences you noted (the
blocks at the other call sites).
In `@src/components/RequestList/index.tsx`:
- Around line 336-360: The empty-state "No results" is shown even when
removalData.results has items; update the conditional that renders the
empty-state in the RequestList component to check both data.results and
removalData.results (e.g., only show "No results" when data?.results.length ===
0 && removalData?.results.length === 0). Locate the two empty-state render
blocks around the existing removal list (uses removalData and
removalData.results) and the other block referenced (uses data and data.results,
around lines ~373-391) and change their gating logic to require both lists to be
empty before rendering the empty-state.
---
Duplicate comments:
In `@server/routes/removalRequest.ts`:
- Around line 131-147: The seasons validation currently allows an empty array
which later gets coerced to undefined and can turn a season-only removal into a
full-media removal; update the validation around mediaRepository.findOne so that
seasons must be a non-empty array of positive integers (reject []), do not
rewrite seasons to undefined, and when handling season-specific removals load
the Media.seasons relation (e.g., via mediaRepository.findOne with relations) so
MediaRemovalRequest.executeRemoval() has the loaded media.seasons to update
local season status; apply the same fix to the other validation sites referenced
(around the logic at lines handling seasons → 226-230, 279-281, 365-367).
---
Nitpick comments:
In `@seerr-api.yml`:
- Around line 6287-6294: Create a concrete MediaRemovalRequest schema (include
fields status, is4k, seasons, reason, media, requestedBy, modifiedBy and their
appropriate types) and then define a paginated wrapper schema (e.g.,
MediaRemovalRequestList) that has pageInfo: $ref '#/components/schemas/PageInfo'
and results: type: array, items: $ref
'#/components/schemas/MediaRemovalRequest'; replace the anonymous object usages
under results in the affected response definitions (the blocks currently using
type: object) with $ref to MediaRemovalRequestList so all endpoints reference
the reusable schemas instead of anonymous objects.
In `@server/routes/removalRequest.ts`:
- Around line 20-27: This code clamps and silently rewrites query params
(rawTake/rawSkip -> pageSize/skip and defaulting invalid filter to "all");
instead let the OpenAPI validator enforce types/limits and read the validated
values directly from req.query, only providing defaults when a param is omitted.
Remove the Number.isFinite checks, Math.floor/clamping logic and any
fallback-for-invalid behavior in the handler (variables: rawTake, rawSkip,
pageSize, skip, and filter), and simply use the values supplied by the
OpenAPI-validated req.query (or apply an explicit default only if the query key
is undefined).
🪄 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: 0f4f17a0-5d1b-469b-98c8-183b862dacb1
📒 Files selected for processing (5)
seerr-api.ymlserver/entity/Media.tsserver/entity/User.tsserver/routes/removalRequest.tssrc/components/RequestList/index.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- server/entity/Media.ts
- server/entity/User.ts
|
For auto-approving removal-requests, what happens if multiple users requested something? For example, if User1 and User2 both request MovieA, then User1 requests it be deleted. Does it delete it from Radarr? Remove the User1 tag from MovieA in Radarr then delete it only if no tags remain? Or does it not consider multiple users requesting the same thing at all and just let any user delete any other's requested media (even if doing so might require an initial request of that media before a deletion request is allowed)? |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/entity/MediaRemovalRequest.ts`:
- Around line 150-161: The loop in MediaRemovalRequest.executeRemoval uses
media.seasons which may be undefined because seasons is not an eager relation;
update executeRemoval (or the code path that handles the season-removal branch)
to explicitly load the seasons relation before iterating — e.g., re-fetch the
Media entity with relations:['seasons'] or use the media repository to load
seasons for media.id, then proceed to set season.status or season.status4k and
call seasonRepository.save(season) as currently done; reference executeRemoval,
MediaRemovalRequest, media.seasons, seasonRepository.save, and is4k when making
the change.
In `@server/routes/removalRequest.ts`:
- Around line 297-299: The removalRequest retrieval uses
removalRequestRepository.findOneOrFail without loading nested relations, so
media.seasons is missing and executeRemoval() can fail for season-level
removals; update the findOneOrFail call to include the nested relation (e.g.,
pass relations including "media" and "media.seasons") so the returned
removalRequest has media.seasons populated before calling executeRemoval().
- Around line 176-196: The ownership check currently queries
mediaRequestRepository.findOne for media id only, so a user who requested the
non‑4K variant can remove the 4K (or vice versa); update the findOne where
clause to also match the 4K variant by including the is4k condition (e.g. add
is4k: media.is4k at the same level as requestedBy, or if is4k is a property of
the related media entity put is4k inside media: { id: media.id, is4k: media.is4k
}) so the query only returns a MediaRequest when the original request matches
the same is4k value; keep the existing permission checks
(Permission.REMOVAL_ALL, Permission.ADMIN, hasPermission) around this lookup.
- Around line 389-391: The code fetches a removalRequest via
removalRequestRepository.findOneOrFail without loading the media.seasons
relation, causing season-level retries to fail; update the call to include the
needed relation (e.g., pass relations: ['media', 'media.seasons'] or the
equivalent nested relation syntax) so that the returned removalRequest has
media.seasons populated before any season-level retry logic executes.
🪄 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: c69793bc-ad17-43ad-9162-46a6a5bd64d6
📒 Files selected for processing (6)
seerr-api.ymlserver/entity/MediaRemovalRequest.tsserver/migration/postgres/1775397575694-AddMediaRemovalRequest.tsserver/migration/sqlite/1775397567178-AddMediaRemovalRequest.tsserver/routes/removalRequest.tssrc/components/RequestList/index.tsx
🚧 Files skipped from review as they are similar to previous changes (4)
- src/components/RequestList/index.tsx
- server/migration/postgres/1775397575694-AddMediaRemovalRequest.ts
- seerr-api.yml
- server/migration/sqlite/1775397567178-AddMediaRemovalRequest.ts
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 `@server/routes/removalRequest.ts`:
- Around line 17-19: The GET handler registered with
removalRequestRoutes.get('/') is missing the isAuthenticated() middleware;
update the route to include isAuthenticated() before the async (req, res, next)
handler so unauthenticated requests are blocked and req.user is guaranteed,
preventing the subsequent req.user?.hasPermission(...) and the userId filter
from receiving undefined; locate the route registration for
removalRequestRoutes.get and insert the isAuthenticated middleware reference
there.
🪄 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: 02541208-feb8-42f2-948d-56f96ed1f2ca
📒 Files selected for processing (1)
server/routes/removalRequest.ts
This is a very good point that I hadn't considered, and I've now addressed it in my most recent commit. The behaviour, for tagging, is now that all users who originally requested must request removal for the media to actually be removed. The system will track who has approved removal requests, and show the media in an intermediate state. |
Thanks for your work on this feature :) Two more questions along these lines though; what if an admin added a users request manually in Radarr but tagged it appropriately? For example, maybe User1 wanted Movie1 and an admin added it in Radarr then used the Seerr-created Radarr rag for User1 to tag it, but didn't bother creating an associated Seerr request? Does your system check Radarr/Sonare tags? Is there an option for that? Question 2: What happens with requests for different seasons of series? Like User1 requests season 1 of Series1, User2 requests season 2 and 3 of Series1, then User2 requests deletion of season 2 and User1 requests deletion of season 1? Or is it only at the series level, not the season level? |
Yeah this is how it's been implemented. There are 3 distinct permissions:
I'm already running this branch on my server to allow my users and I to test it, and 9/10 I enable the removal requests and the auto approve. So people can request to delete their own media, and it automatically goes through, but they can't do it for items they didn't request. |
|
Amazing feature is there a timeline on when this feature will be added? |
|
this also resolves #1792 |
|
Is there a method to hot-patch this onto a running instance as of now? this feature is desperately awaited by my userbase |
I agree on this point. I would love to be able to integrate or teat this feature into an existing instance |
|
I've not had any time to work further on this recently I'm afraid. I've pushed my own docker container with it in so I can run it on my instance, and it's working well, but has a few bugs I'd like to address before a proper review. Feel free to do the same. |
Thank you for your efforts. I created something as a stopgap @InactiveImmortal https://github.com/someRandomDude-a/seerr-discord-bot As a note, there is no existing python package to interact with the seerr APIthat I could find, so I made my own. |
|
I'll see if I can get some time to iron out the kinks today. Like I said, it is already in use on my instance by my users, so just need to address these issues to get it over the line. |
|
If you did want to hot-patch for now until merged, my public docker image is available at: dnywlsn/seerr:latest I'll update this with the newest develop branch changes today (it's a little out of date) and push any fixes I have into it whilst the review is pending. |
Sounds great! No pressure. Like I said, its no longer urgent for me, my discord bot is good enough for the time-being |
The full-vs-partial removal decision counted requesters of the media regardless of quality version or which seasons they wanted, which could both block full removal of one quality version and over-delete seasons a co-requester never agreed to remove. Scope the consent set to the same is4k value and, for season removals, to requesters of the targeted seasons. Season-level partial removals no longer strip the series-level *arr tag, and resolveTvdbId now prefers the stored TVDB id. Align the updatedAt column type with the rest of the entities. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
removeMovie/removeSeries/removeSeasonFiles now no-op when the item is already absent from Radarr/Sonarr, so a removal request that failed after the external deletion succeeded can be retried to completion. The create route's duplicate guard is reduced to a single per-user lookup (one active removal request per media + quality version), removing the unreachable season-overlap branch. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The removal migrations predated the merged UpgradeConnectTypeORM migration, so they ran out of order on existing installs. Regenerate both against develop with later timestamps. The Postgres migration now widens user.permissions to bigint in place (ALTER COLUMN TYPE) instead of DROP/ADD, preserving existing permission values, and uses a timestamptz updatedAt to match the entity; the down path masks out-of-range bits before narrowing back to integer. The SQLite migration is trimmed to the user.permissions widen plus the new table. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Surface success/error/already-pending toasts when submitting a removal request, and surface failures from approve/decline/retry/delete instead of silently swallowing them. Scope the "removal pending" detection to the current user so co-requesters can still add their own request for multi-user removal, and disable the action only when the current user already has an active request. The season removal modal derives its selectable set from the rendered rows (so "select all" can't pick a hidden season), adds accessible names and aria-disabled to the season toggles, and scopes pending state to the current user. The pending removal queue now shows the target media (poster, title, link). Drop the redundant admin permission check on the manage button. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add a reusable MediaRemovalRequest schema and reference it from the removal-request responses instead of opaque objects, include the partially-removed filter value, and document the real 4xx response codes for each endpoint. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add route tests for permission gating, validation, the create/approve/decline/retry/delete lifecycle, list scoping, and the multi-user partial-then-full and 4K-independence consent behaviour. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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/api/servarr/sonarr.ts`:
- Around line 439-460: The review notes that Sonarr update vs create endpoints
must be used correctly: keep removeTagFromSeries as-is (it correctly calls PUT
/series/${series.id}) and modify addSeries so it uses POST /series when creating
a new series and PUT /series/${series.id} when updating an existing one (rather
than PUT /series without the id); update the logic in the addSeries function to
check for series.id (or existence) and call the appropriate endpoint, ensuring
request payloads remain the same and error handling is preserved.
In `@server/entity/MediaRemovalRequest.ts`:
- Around line 117-145: The season-removal overlap check incorrectly treats
full-series requests as non-overlapping because it uses (r.seasons ?? []) which
yields an empty array for full-series; update the isSeasonRemoval branch (the
filter on otherRemovalsForMedia used to compute otherRemovals) to treat a
request with missing/empty r.seasons as matching all seasons — i.e., include the
removal if r.seasons is null/undefined/empty OR if (r.seasons).some(s =>
targetSeasons.includes(s.seasonNumber)); keep the rest of the logic
(uniqueRequesterIds, removalRequestRepository, otherRemovalsForMedia,
is4k/id/status checks) unchanged.
In `@server/migration/sqlite/1780140032396-AddMediaRemovalRequest.ts`:
- Around line 43-49: The rollback INSERT copies the bigint "permissions" value
into an integer column without masking high bits; change the INSERT SELECT to
mask permissions with & 2147483647 (e.g., replace the SELECT column
`"permissions"` with `("permissions" & 2147483647) AS "permissions"`) so the
down migration in the migration's down method (the CREATE TABLE "temporary_user"
/ INSERT INTO "temporary_user" block) narrows permissions the same way Postgres
does.
- Around line 9-16: The migration rebuild of the "user" table (creating
"temporary_user", INSERT ... FROM "user", DROP TABLE "user", ALTER TABLE ...
RENAME TO "user") should be wrapped with SQLite FK checks disabled and
re-enabled to avoid foreign key constraint failures; update both the up() and
down() methods in AddMediaRemovalRequest (where queryRunner is used) to run
PRAGMA foreign_keys=OFF before the temporary table creation/insert/drop/rename
sequence and PRAGMA foreign_keys=ON immediately after the rename completes.
In `@src/components/ManageSlideOver/index.tsx`:
- Around line 130-146: The UI currently uses a non-quality-specific check so
users who requested removal for one quality may see the action for the other;
ensure removal eligibility stays quality-specific by using the per-quality
helpers: keep or update userHasActiveRemoval and userCanRequestRemoval to accept
is4k boolean and then use userCanRequestRemoval(false) for HD action renderings
and userCanRequestRemoval(true) for 4K action renderings (also use
pendingRemoval = userHasActiveRemoval(false) and pendingRemoval4k =
userHasActiveRemoval(true) where needed) so each HD/4K button and season-removal
entry point queries the corresponding variant-only function.
🪄 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: ea0b75b7-a166-4abf-bac4-1e60152ae1c3
📒 Files selected for processing (18)
seerr-api.ymlserver/api/servarr/radarr.tsserver/api/servarr/sonarr.tsserver/constants/media.tsserver/entity/Media.tsserver/entity/MediaRemovalRequest.tsserver/entity/User.tsserver/migration/postgres/1780140245985-AddMediaRemovalRequest.tsserver/migration/sqlite/1780140032396-AddMediaRemovalRequest.tsserver/routes/removalRequest.test.tsserver/routes/removalRequest.tssrc/components/ManageSlideOver/index.tsxsrc/components/MovieDetails/index.tsxsrc/components/RemovalRequestBlock/index.tsxsrc/components/RequestList/index.tsxsrc/components/RequestModal/SeasonRemovalModal.tsxsrc/components/TvDetails/index.tsxsrc/i18n/locale/en.json
🚧 Files skipped from review as they are similar to previous changes (7)
- server/entity/Media.ts
- seerr-api.yml
- server/constants/media.ts
- server/entity/User.ts
- src/components/RemovalRequestBlock/index.tsx
- src/components/RequestList/index.tsx
- src/i18n/locale/en.json
SeasonRemovalModal rendered a bare Modal (whose root is a Headless UI Transition.Child) inside the SlideOver's Transition subtree, so it attached to the slide-over's transition context. Closing the modal then tore down the slide-over's backdrop without firing onClose and stranded the body scroll-lock, freezing the page until reload. Give the modal its own Transition wrapper, render it outside the SlideOver subtree, and skip its redundant scroll-lock (the slide-over already owns it). Also gate the HD vs 4K removal actions on whether the user requested that quality version, so they aren't offered a path the API will reject. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The request list only fetched removal requests for MANAGE_REQUESTS users, so a regular user never saw their own. Fetch them for everyone (the API scopes non-privileged users to their own requests). Render removal entries as full cards mirroring RequestItem (poster, title, status, requester, actions) instead of a thin row, group them under a 'Removal Requests' heading, and label the standard requests group when both are present so the two read as parallel sections. In the manage slide-over, RemovalRequestBlock now mirrors RequestBlock's compact layout for consistency. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
For season-level removals, a full-series request/removal (no specific seasons) now counts as covering every season, so a whole-series requester is included in the consent set and prior full-series removal approvals are honoured before season files are deleted. Also mask out-of-range permission bits in the SQLite down migration to match the Postgres rollback. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Awesome! I’ll definitely check it out. My biggest issue right now is a full server, but as it isn’t urgent yet, I figured I would sit back and wait. I really appreciate all the work you are doing on this! |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/components/RequestList/index.tsx (1)
100-113: Pending-removal queue is capped at 20 with no pagination.For privileged users actioning the global queue,
take=20silently hides additional pending removals beyond the first 20. Consider surfacing a "view all" affordance or pagination if the queue can realistically exceed this in production.🤖 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/RequestList/index.tsx` around lines 100 - 113, The SWR call fetching removal requests (useSWR that returns removalData and revalidateRemovals) is hardcoded with `take=20`, which truncates the pending queue; change the fetch to support pagination or a "view all" mode by parameterizing the query string with page and pageSize (e.g., add component state like page and pageSize or a boolean viewAll flag), include those values when building the `/api/v1/removal-request?filter=pending...` URL (concatenate removalRequestScope as before), update the component to render pagination controls or a "View all" button that updates page/pageSize or toggles viewAll and triggers revalidateRemovals, and ensure UI reflects loading/empty states and that the API supports the new page/pageSize params (or use a larger take when viewAll is toggled).
🤖 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/components/RequestList/index.tsx`:
- Around line 100-113: The SWR call fetching removal requests (useSWR that
returns removalData and revalidateRemovals) is hardcoded with `take=20`, which
truncates the pending queue; change the fetch to support pagination or a "view
all" mode by parameterizing the query string with page and pageSize (e.g., add
component state like page and pageSize or a boolean viewAll flag), include those
values when building the `/api/v1/removal-request?filter=pending...` URL
(concatenate removalRequestScope as before), update the component to render
pagination controls or a "View all" button that updates page/pageSize or toggles
viewAll and triggers revalidateRemovals, and ensure UI reflects loading/empty
states and that the API supports the new page/pageSize params (or use a larger
take when viewAll is toggled).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 135f4c6b-8eeb-4e5a-b6c6-5fb5ff6f3851
📒 Files selected for processing (7)
server/entity/MediaRemovalRequest.tsserver/migration/sqlite/1780140032396-AddMediaRemovalRequest.tssrc/components/ManageSlideOver/index.tsxsrc/components/RemovalRequestBlock/index.tsxsrc/components/RequestList/index.tsxsrc/components/RequestModal/SeasonRemovalModal.tsxsrc/i18n/locale/en.json
🚧 Files skipped from review as they are similar to previous changes (4)
- server/migration/sqlite/1780140032396-AddMediaRemovalRequest.ts
- src/i18n/locale/en.json
- server/entity/MediaRemovalRequest.ts
- src/components/ManageSlideOver/index.tsx
|
This is now ready for review by the project maintainers. |
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
thoughts on possibly incorporating jellyfin's favorites / watched tags per user to see interest? for example if user A requests the show and user B watches the show as well, unless user B has watched all episodes or a certain time passes, user A can only request deletion. OR they can give up ownership and once all members have watched the show or enough time passes since the last time it was accessed, it gets deleted |
Description
Introduces a complete media removal request workflow, allowing users to request the deletion of movies or individual TV seasons from Seerr and the connected Sonarr/Radarr instances. Requests go through an approval pipeline (pending → approved/declined) controlled by a new set of permissions, with optional auto-approval for privileged users.
Important
Due to adding new permissions, the limit of bitwise ops was hit. To solve this, a migration has been put in place and code updated to use bigint. More details below, but this is an area worth paying special attention to.
New Functionality
Removal Request Entity & Data Model
MediaRemovalRequestentity with status tracking (PENDING,APPROVED,DECLINED,FAILED), 4K awareness, optional per-season targeting, and a free-text reason field.media_removal_requesttable with appropriate indexes and foreign keys.API Endpoints (
/removal-request)GET/POST/REQUEST_REMOVALPOST/:id/approveMANAGE_REQUESTSPOST/:id/declineMANAGE_REQUESTSPOST/:id/retryMANAGE_REQUESTSDELETE/:idMANAGE_REQUESTSSonarr Integration
removeSeasonFiles()method onSonarrAPIthat deletes episode files for specified seasons and unmonitors those seasons, enabling granular per-season removal without deleting the entire series.Permissions
REQUEST_REMOVAL(can create requests),AUTO_APPROVE_REMOVAL(requests are immediately executed),REMOVAL_ALL(can request removal of any media, not just self-requested).hasPermission()refactored to useBigIntfor bitwise operations, necessary because the new permission values exceed 2^30 (the safe limit for 32-bit integer bitwise ops in JS).User.permissionscolumn type changed fromintegertobigintto accommodate the expanded permission space.Frontend
ManageSlideOverextended with removal request creation and management (approve/decline/retry/delete) for both movies and TV shows.RemovalRequestBlockcomponent for displaying removal request status on media detail pages.SeasonRemovalModalcomponent for selecting specific seasons to request removal of.OpenAPI Spec
Risks & Considerations
User.permissionswidens fromintegertobigintto fit the new permission bits. The Postgres migration alters the column type in place (preserving every user's existing value) and the down path masks out-of-range bits before narrowing back tointeger. Both migrations were regenerated against the latestdevelop(so they no longer run out of order relative to the merged Connect-TypeORM migration) and verified to apply, reverse, and produce a schema matching the entities on both SQLite and Postgres.Updates since initial review
Follow-up commits address the review feedback:
MediaRemovalRequestschema and documented 4xx response codes.Files Changed (22 files, +3,036 / -61)
Server: New entity, migrations (SQLite + Postgres), API routes, Radarr/Sonarr removal helpers, permission expansion with BigInt refactor
Frontend: Permission editor, manage slide-over, removal request block, season removal modal, request list integration
Spec / Tests: OpenAPI documentation and removal-request route tests
How Has This Been Tested?
All existing tests ran and passed, changes thoroughly dev tested with a local running instance, including a run through of all scenarios and permission combinations. Manually verified end to end across every permission profile — owner/root, manage-requests, request-removal, auto-approve-removal, removal-all, and default — for movies, individual TV seasons, and 4K, covering create/pending, approve, decline, multi-user partial-then-full removal, and force removal. Added automated route tests covering permission gating, validation, the full create/approve/decline/retry/delete lifecycle, list scoping, and multi-user consent.
Screenshots / Logs (if applicable)
N/A — backend + UI change; behaviour is covered by the automated tests and the manual verification described above.
Checklist:
pnpm buildpnpm i18n:extractSummary by CodeRabbit
New Features
Permissions
Documentation
Tests
Bug Fixes