feat(exclusions): add watchlist exclusion system#1178
Conversation
chore: merge develop into master for v0.15.5
…t loops When Delete Sync removes content but items remain on users' Plex watchlists, the sync engine would re-request them on the next cycle. This adds an exclusion system that lets admins mark items to skip during sync. Exclusions clear automatically when users remove items from their watchlists. Backend: migration, DB methods, API routes, sync engine integration Frontend: table-based management UI with filters, sorting, per-row actions Docs: utilities page, README, intro, API reference, OpenAPI tags
|
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:
WalkthroughThis PR implements a complete watchlist exclusions feature allowing users to prevent specific Plex watchlist items from being routed to Sonarr/Radarr. The system spans database persistence (watchlist_exclusions table), API endpoints for CRUD operations, sync engine gating that skips excluded items, routing integration that prevents enriched item routing, cleanup flows for deleted watchlist items, and a full-featured client UI with single/bulk exclusion management. The PR also integrates exclusion-driven GUIDs into tag-based deletion workflows and refactors shared table selection UI logic across components. ChangesWatchlist Exclusions Feature
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes The PR introduces a substantial multi-layered feature spanning database schema, backend orchestration, API contracts, client data management, and a full-featured UI with complex state transitions. The integration points across sync engines, routing, and delete-sync workflows demand careful review. Tag-based deletion integration adds conditional logic around exclusion-driven GUIDs. The client page component manages multiple concurrent mutations with aggregated error handling and modal state coordination. Moderate scope and moderate cohesion across tightly-coupled systems increase review complexity. Possibly related PRs
Suggested labels
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
src/client/features/utilities/pages/exclusions.tsx (1)
157-169: ⚡ Quick winUse a precomputed exclusion lookup to avoid O(n×m) joins.
At Line 159, doing
findper row scales poorly as data grows. A memoized map keyed by${user_id}-${key}keeps lookup O(1).🤖 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/client/features/utilities/pages/exclusions.tsx` around lines 157 - 169, The current tableData computation does an O(n×m) join by calling exclusions.find for each watchlistItems row; replace this with a memoized lookup map (keyed by `${user_id}-${key}`) computed with React.useMemo (e.g., build an exclusionsByKey map from exclusions) and then map over watchlistItems to set id, isExcluded, and exclusionId by doing a constant-time lookup into exclusionsByKey; update references to exclusions, watchlistItems, and the produced exclusionId/isExcluded in the tableData useMemo dependencies accordingly.
🤖 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 `@src/client/features/utilities/hooks/useExclusions.ts`:
- Around line 50-54: The effect in useExclusions.ts calls fetchExclusions(false)
without handling rejections, which can cause unhandled promise rejections
because the store action rethrows; update the useEffect to invoke
fetchExclusions inside an async IIFE or attach a .catch() handler to handle
errors (e.g., call fetchExclusions(false).catch(err => {/* log or noop */})),
referencing the existing useEffect, fetchExclusions, and hasLoadedExclusions so
the initial fetch failure is caught and prevented from bubbling as an unhandled
rejection.
In `@src/client/features/utilities/pages/exclusions.tsx`:
- Around line 498-510: The icon-only refresh Button lacks an accessible name;
update the Button (the component rendering the refresh control that uses
onClick={handleRefresh}, disabled={isRefreshing}, and children
Loader2/RefreshCw) to include an aria-label (and optionally title) that clearly
describes the action (e.g., "Refresh exclusions" or switch to "Refreshing" when
isRefreshing is true) so screen readers can announce it; keep the existing props
and visuals but add the aria-label/title attributes to the Button element.
- Line 124: The early return when users is empty leaves the watchlist load flag
unset; update the branch in the component that currently does "if
(!users?.length) return" to mark the watchlists as loaded before returning by
calling the state updater (e.g., setHasLoadedWatchlists(true)) or otherwise
setting hasLoadedWatchlists to true, so the skeleton/unresolved initial-load
state is cleared when users length is zero; keep the existing return to avoid
running later logic after setting the flag.
In `@src/schemas/exclusions/exclusions.schema.ts`:
- Around line 19-21: The numeric exclusion IDs currently accept floats and
negatives; update the zod schemas to require positive integers by changing the
array element to .array(z.number().int().positive()) for userIds and use
.coerce.number().int().positive() for the individual userId and id parameters
(wherever the fields named userId and id are defined) so all incoming exclusion
IDs are coerced to integers and validated as positive before DB use.
In `@src/services/database/methods/exclusion.ts`:
- Around line 33-34: The insert-count logic in methods like the one setting
`inserted` uses dialect-specific shapes (`this.isPostgres` and `(result as
unknown as { rowCount: number }).rowCount` vs `(result as unknown as
number[])[0]`) which is unreliable; change the insert query that produced
`result` to include `.returning('id')` so it consistently returns inserted rows
and then compute `const inserted = Array.isArray(result) ? (result as
unknown[]).length : 0` (or simply `(result as any).length`) and return that
count (Number or 0) instead of using `rowCount`/indexing. Ensure the code that
constructs the Knex insert uses `.returning('id')` and the `inserted`
calculation uses `.length`.
---
Nitpick comments:
In `@src/client/features/utilities/pages/exclusions.tsx`:
- Around line 157-169: The current tableData computation does an O(n×m) join by
calling exclusions.find for each watchlistItems row; replace this with a
memoized lookup map (keyed by `${user_id}-${key}`) computed with React.useMemo
(e.g., build an exclusionsByKey map from exclusions) and then map over
watchlistItems to set id, isExcluded, and exclusionId by doing a constant-time
lookup into exclusionsByKey; update references to exclusions, watchlistItems,
and the produced exclusionId/isExcluded in the tableData useMemo dependencies
accordingly.
🪄 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: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 6e3e2747-ce3e-4ef2-826d-80853f977d44
⛔ Files ignored due to path filters (4)
docs/docs/api-documentation.mdis excluded by!docs/**docs/docs/intro.mdis excluded by!docs/**docs/docs/utilities/watchlist-exclusions.mdis excluded by!docs/**docs/sidebars.tsis excluded by!docs/**
📒 Files selected for processing (18)
README.mdmigrations/migrations/091_20260516_add_watchlist_exclusions.tssrc/client/components/AppSidebar.tsxsrc/client/features/utilities/components/exclusions/exclusions-delete-confirmation-modal.tsxsrc/client/features/utilities/components/exclusions/exclusions-skeleton.tsxsrc/client/features/utilities/hooks/useExclusions.tssrc/client/features/utilities/pages/exclusions.tsxsrc/client/features/utilities/store/exclusionsStore.tssrc/client/router/router.tsxsrc/plugins/external/swagger.tssrc/routes/v1/exclusions/exclusions.tssrc/schemas/exclusions/exclusions.schema.tssrc/services/database.service.tssrc/services/database/methods/exclusion.tssrc/services/database/methods/watchlist.tssrc/services/database/types/exclusion-methods.tssrc/services/watchlist-workflow/orchestration/sync-engine.tssrc/types/exclusion.types.ts
- Handle zero-users case in watchlist fetch (avoids permanent skeleton)
- Set hasLoadedExclusions on fetch failure (avoids stuck loading state)
- Add aria-label to refresh button for accessibility
- Use .returning('id') for insert count instead of dialect-specific logic
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/client/features/utilities/pages/exclusions.tsx (1)
161-173: ⚡ Quick winConsider using a Map for O(1) exclusion lookups.
The current implementation is O(n×m) due to
.find()inside.map(). Pre-building a keyed Map would reduce this to O(n+m).♻️ Proposed refactor
const tableData = React.useMemo<ExclusionTableRow[]>(() => { + const exclusionMap = new Map( + exclusions.map((e) => [`${e.user_id}-${e.key}`, e]) + ) return watchlistItems.map((item) => { - const exclusion = exclusions.find( - (e) => e.key === item.key && e.user_id === item.userId, - ) + const exclusion = exclusionMap.get(`${item.userId}-${item.key}`) return { ...item, id: `${item.userId}-${item.key}`, isExcluded: !!exclusion, exclusionId: exclusion?.id ?? null, } }) }, [watchlistItems, exclusions])🤖 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/client/features/utilities/pages/exclusions.tsx` around lines 161 - 173, The tableData creation is currently O(n*m) because it calls exclusions.find() for each watchlist item; instead, inside the same React.useMemo for tableData, first build a Map keyed by the same identity used for row ids (e.g. `${e.user_id}-${e.key}`) from the exclusions array, then replace the .find() with a constant-time Map lookup to set isExcluded and exclusionId for each watchlist item (keep the same row shape: id: `${item.userId}-${item.key}`, isExcluded, exclusionId); ensure types (ExclusionTableRow) still align and keep the useMemo dependency array as [watchlistItems, exclusions].
🤖 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 `@src/client/features/utilities/pages/exclusions.tsx`:
- Around line 155-159: The effect's guard uses users?.length which is falsy for
an empty array and prevents fetchAllWatchlistItems from running; update the
React.useEffect condition to check for users being defined (e.g., isInitialized
&& users !== undefined && !hasLoadedWatchlists) or simply remove the
users?.length check so fetchAllWatchlistItems is invoked when users is present
(even if empty), since fetchAllWatchlistItems already handles the empty-array
case; keep references to React.useEffect, users?.length, fetchAllWatchlistItems,
hasLoadedWatchlists, and isInitialized when making the change.
---
Nitpick comments:
In `@src/client/features/utilities/pages/exclusions.tsx`:
- Around line 161-173: The tableData creation is currently O(n*m) because it
calls exclusions.find() for each watchlist item; instead, inside the same
React.useMemo for tableData, first build a Map keyed by the same identity used
for row ids (e.g. `${e.user_id}-${e.key}`) from the exclusions array, then
replace the .find() with a constant-time Map lookup to set isExcluded and
exclusionId for each watchlist item (keep the same row shape: id:
`${item.userId}-${item.key}`, isExcluded, exclusionId); ensure types
(ExclusionTableRow) still align and keep the useMemo dependency array as
[watchlistItems, exclusions].
🪄 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: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 4b747b81-d08e-46a6-9c46-d8ba6b6f5823
📒 Files selected for processing (3)
src/client/features/utilities/pages/exclusions.tsxsrc/client/features/utilities/store/exclusionsStore.tssrc/services/database/methods/exclusion.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/services/database/methods/exclusion.ts
- src/client/features/utilities/store/exclusionsStore.ts
The useEffect guard checked users?.length which is falsy for an empty array, preventing the early-return path from setting hasLoadedWatchlists.
|
Hi @AhmedNSidd Thanks ill set some time aside to go over this. I agree that a exclusion system is desirable and has something that's been on the docket for a while. Can you explain the delete sync loop a bit though? I don't fully understand that loop as delete sync is supposed to check all watchlists prior to running, so it shouldnt actually remove anything still on users watchlists in the first place. Regardless, this is a welcomed addition. |
|
Hey @jamcalli Appreciate it. The scenario I'm hitting isn't really about delete sync specifically — it's more about wanting to clean up stale content from my library manually. My users request a lot of stuff, watch it briefly, then it sits untouched for months. Occasionally I want to clear out that stale media from Sonarr/Radarr, but since the items are still on their Plex watchlists, Pulsarr just picks them up again on the next sync cycle and re-adds them. I can't easily clean other users' watchlists for them, and unmonitoring in the arrs is a workaround but not ideal since it clutters things up. Exclusions give me a way to say "stop syncing this item" without needing users to manage their own watchlists. |
fastify-autoload derives the URL prefix from the directory structure, so routes in routes/v1/exclusions/ already have the /v1/exclusions prefix. The route handlers were redundantly specifying /exclusions again, resulting in /v1/exclusions/exclusions instead of /v1/exclusions.
When a user removes an item from their Plex watchlist and re-adds it, the exclusion for that item should be cleared so the watchlist sync can request the item again. The existing logic clears the exclusion inside deleteWatchlistItems, which only fires during the 2-hour full reconciliation — too slow for an interactive re-request flow. This routes the same clear through the real-time path. When RSS or ETag polling surfaces an item that has a matching exclusion for the user, treat that as the re-add signal: clear the exclusion and force-route the item past the categorizer's already-linked dedup so the content router actually fires. Adds findExcludedKeys helper and updates clearExclusions to return the delete count.
|
Makes sense. Without having time to thoroughly sit down and comment, but just upon a first glance the most noticeable deviation: The client is half migrated to using react query, and this appears to be using the old zustand fetch approach. Could you please mirror the query format used by session monitoring, the dashboard, and approvals etc? |
Move exclusions data fetching off the zustand store and onto the same react-query pattern used by approvals and session-monitoring: - useExclusions: query hook backed by useAppQuery, with an exclusionKeys factory for targeted invalidation - useExclusionMutations: useCreateExclusion / useRemoveExclusion using useAppMutation, both invalidating the list cache on success - Drop exclusionsStore.ts entirely — server state lives in the cache, per-row UI state moves to component-local React state (activeExcludeRowId for the create spinner, mutation.variables comparison for the delete spinner) - Clean up an unused itemKey prop on the delete confirmation modal that was tripping noUnusedParameters Mirrors the pattern flagged on the PR review.
- Remove CreateExclusionData from exclusion.types.ts. It was never imported — excludeWatchlistItem takes (key, userIds) as separate params, so the wrapper type was dead. - Group findExcludedKeys with the other read methods in exclusion.ts/exclusion-methods.ts. Was previously sandwiched between two write methods.
|
Hey @jamcalli — pushed a few updates. The big one is the react-query migration you flagged. Both the fetch and the mutations now go through While I was in there I also went back through the whole PR diff against the codebase conventions to catch anything else I might have miscopied from an older pattern. Two small things came out of that:
There's also one architectural decision I want to flag separately that came up while I was testing this end-to-end. I had the exclusion-clear inside |
The dedup gate in checkExistingApprovalRequest was sending auto_approved records to the default branch (silently skip routing) while approved records called routeUsingApprovedDecision and re-routed. Both statuses mean "this user is cleared for this content" — the only difference is whether an admin had to click a button. Falling through to the same handler makes re-requests work for users with bypass approval enabled. Surfaced while testing the watchlist exclusion re-add flow: after the exclusion cleared correctly, the content router still refused to send the item to Sonarr because of a stale auto_approved record from the original request.
|
This looks fairly good. Could I request you just clean up the jsdocs a bit? They are quite verbose. |
|
I'll try to carve some time out either this evening or in the coming days and I'll do that yeah, I had a feeling the docs were a bit too verbose as well. Also let me know if there's any other preferences you have or things you want changed with respect to how the UI looks too. I was debating between either calling it Watchlist Exclusions or just Exclusions in the sidebar. And I also added sorting ability on the "Exclude" column as well which should allow the admin user to group which watchlist items are excluded / not excluded faster. I also had the thought that we should probably also add a button for "Exclude all" which would turn into a "Unexclude all", probably would be helpful for some admin users so they wouldn't have to individually try to exclude every item that's appearing in the table list, but we can always save this for a future feature PR as well |
|
Looks pretty decent to me. It would be worthwhile to have a global setting and not per user (as the re-add clears and flow anyways). Should it just be global per item and not per specific users? This will lay the backbone for the maintainerr integration that I have been thinking through, where it will send webhooks when it removes items which will add to the exclusions list (which will not be user aware). As for the button handlers, please bind the button to the mutation's Right now you clear it in |
The real-time path interpreted RSS feed activity as a user "re-add" signal and used it to clear exclusions and force-route items past the normal dedup. That interpretation isn't safe: the RSS feed is a shared resource across all friends, and bulk operations on one user's watchlist (e.g. mass deletes via the Plex.tv API) can evict other users' items from the feed window, then surface them again as "new" once the feed settles. Those items then trip the re-add path even though no user touched them, producing spurious requests to Sonarr/Radarr for content that was deliberately excluded. Exclusions are designed as a sticky administrative flag - they should only be cleared by explicit user/admin action, not by indirect real-time signals. Clean-up of orphan exclusions (items no longer on any user's watchlist) should happen against authoritative state in the periodic full reconciliation, not in the real-time path. Removes both the exclusion-clearing block (formerly Step 1b) and the force-route-past-dedup block (formerly Step 6b) from processItemsForUser. The brand-new and existing-link paths continue to route legitimate adds normally.
…exclusions
Adds two complementary cleanup paths so the exclusions table stays coherent
with the rest of the system.
1. Orphan exclusion cleanup (handleRemovedItems): when a watchlist_items row
is deleted because the item is no longer on the user's Plex watchlist,
also clear any matching (user_id, key) exclusion. The exclusion is dead
weight once the item is gone — keeping it serves no behavioral purpose
and would silently kick back in if the user later re-adds. Wires the
existing clearExclusions method, which was defined for this purpose but
previously unused.
2. Excluded-content cleanup (delete-sync step 4.5): drop watchlist_items
rows where a matching exclusion exists and the row has been routed
(status != 'pending'). Removed rows fall out of the protected GUID set
built immediately after, so the standard *arr deletion pass prunes the
content naturally without any new *arr-side logic. Skipped on dry-run.
The exclusion records themselves are preserved in path 2 — they are the
user's persistent rejection signal and must keep blocking future routes.
If the item is still on the user's Plex watchlist, RSS/ETag will recreate
a fresh pending row that the routing layer's exclusion gate vetoes; this
gives the dashboard accurate visibility ("on watchlist, excluded, not
routed") without producing *arr churn.
The status != 'pending' gate avoids churn on pre-excluded items: rows that
never got past the routing veto are already in the correct state, and
deleting them would only get them recreated by the next RSS poll.
Allows exclusions to apply system-wide rather than only to the user who
created them. An exclusion row with user_id = 0 (the existing System user)
vetoes routing for the matching key for every user, alongside any per-user
exclusions that may also exist.
Motivated by an upcoming maintainerr integration: when maintainerr decides
content should no longer exist in the library, its webhook will write an
exclusion at SYSTEM_USER_ID, preventing any user's watchlist from
re-requesting that content. Per-user exclusions are unchanged and continue
to serve the case where one user's noisy watchlist shouldn't dictate what
other users can request.
Touch points:
- SYSTEM_USER_ID constant exported from the exclusion module
- sync-engine routing gate vetoes on either user_id match or
SYSTEM_USER_ID match
- delete-sync cleanup query (cleanupExcludedWatchlistItems) extended so
rows are pruned when matched by either a per-user or a global exclusion
No schema change. clearExclusions and excludeWatchlistItem are unchanged;
maintainerr (or any caller) writes globals by passing [SYSTEM_USER_ID] to
excludeWatchlistItem, and per-user removals naturally leave globals
untouched.
UI button state (exclusions page + delete-confirmation modal):
- Drop the activeExcludeRowId state in favor of deriving per-row
pending state from createExclusionMutation.variables, mirroring
the unexclude path. Aligns with the pattern jamcalli pointed at.
- Remove the setActiveExcludeRowId(null) clearing in the exclude
handler's finally; useAppMutation's minimum-loading window now
drives the visual state on its own.
- Move the modal close out of the confirm button's finally (which
fired the moment mutateAsync resolved and cut the 500ms min-load
short) and into the parent's success path via setPendingUnexclude(null).
Errors keep the modal open so the user can retry or cancel.
JSDoc cleanup (exclusion methods + type declarations):
- Trim multi-paragraph descriptions on the exclusion DB methods to
one-line summaries plus the relevant param/return, matching the
style of other database/methods/ files (e.g. anime.ts).
Matches the page title and the multi-word convention used by every other entry in the Utilities sidebar (API Keys, Delete Sync, Log Viewer, Plex Labels, etc.). The bare "Exclusions" was ambiguous and stood out as the only single-word item in the section.
Brings the page in line with the approval-table and user-table reference
implementations, which already handle narrow viewports cleanly.
- Wrap the table region in overflow-x-auto so a wide table degrades to
horizontal scroll inside its container instead of breaking the whole
page layout (matches approval-table.tsx and user-table.tsx).
- Restructure the toolbar into stacked rows with flex-wrap on the filter
row so the search input, faceted filters, refresh button, and column
selector wrap gracefully instead of overflowing on small screens
(mirrors approval-table-toolbar.tsx).
- Make the pagination info adaptive: "Showing X-Y of Z" on sm and up,
"Page X of Y" on smaller screens, with the same "No results" fallback
pattern the approvals table uses.
- Replace remaining "Exclusions" references with "Watchlist Exclusions"
so the doc matches the sidebar entry and page heading.
- Add a brief info callout in How It Works noting that Delete Sync now
treats excluded items as unwatchlisted — if you exclude something
already in your library, the next Delete Sync run removes it. New
behavior worth surfacing since it can lead to library content being
removed.
|
@jamcalli Pushed updates. On the per-user vs global question, I went with both rather than collapsing to global-only. There's a real per-user case I didn't want to lose: if one user has a noisy watchlist, an admin can quiet specific items for that user without blocking the same content for others who legitimately want it. Going fully global takes that away. So per-user stays as the primary case, and I added a Other items from your comment:
A few related things I also did while in there:
Let me know what you think. |
jamcalli
force-pushed
the
feature/watchlist-exclusions
branch
from
c3f84b1 to
6a7d85b
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/client/features/utilities/pages/watchlist-exclusions.tsx (1)
271-281: 💤 Low valueInconsistent modal feedback compared to
handleBulkExclude.
handleBulkExcludeuses a 600ms delay before closing the modal, allowing users to see the success/error state. Here, the modal closes immediately after setting the status, so the success/error feedback is never visible.🔧 Suggested fix to align behavior
const failures = results.filter((r) => r.status === 'rejected').length setBulkRemoveStatus(failures === 0 ? 'success' : 'error') await refetchExclusions() activeTableRef.current?.clearSelection() - setPendingBulkRemove(null) - setBulkRemoveStatus('idle') + setTimeout(() => { + setPendingBulkRemove(null) + setBulkRemoveStatus('idle') + }, 600)🤖 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/client/features/utilities/pages/watchlist-exclusions.tsx` around lines 271 - 281, The modal is closing immediately so users never see the success/error state; mirror handleBulkExclude by inserting a ~600ms delay after you setBulkRemoveStatus(failures === 0 ? 'success' : 'error') and after awaiting refetchExclusions(), then only clear selection, setPendingBulkRemove(null) and setBulkRemoveStatus('idle') (or close the modal) after that delay. Target the block using setBulkRemoveStatus, refetchExclusions, activeTableRef.current?.clearSelection and setPendingBulkRemove to add an await new Promise(res => setTimeout(res, 600)) (or equivalent) so the success/error state is visible before resetting the modal.
🤖 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/client/features/utilities/pages/watchlist-exclusions.tsx`:
- Around line 271-281: The modal is closing immediately so users never see the
success/error state; mirror handleBulkExclude by inserting a ~600ms delay after
you setBulkRemoveStatus(failures === 0 ? 'success' : 'error') and after awaiting
refetchExclusions(), then only clear selection, setPendingBulkRemove(null) and
setBulkRemoveStatus('idle') (or close the modal) after that delay. Target the
block using setBulkRemoveStatus, refetchExclusions,
activeTableRef.current?.clearSelection and setPendingBulkRemove to add an await
new Promise(res => setTimeout(res, 600)) (or equivalent) so the success/error
state is visible before resetting the modal.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: b0c6ed31-9a81-4e04-b993-b7d87b07a1cb
📒 Files selected for processing (6)
src/client/components/table/data-table-select-column.tsxsrc/client/features/utilities/components/watchlist-exclusions/watchlist-exclusions-table-toolbar.tsxsrc/client/features/utilities/pages/watchlist-exclusions.tsxsrc/services/approval.service.tssrc/services/content-router.service.tssrc/utils/arr-error.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- src/utils/arr-error.ts
- src/client/components/table/data-table-select-column.tsx
- src/services/approval.service.ts
- src/services/content-router.service.ts
- src/client/features/utilities/components/watchlist-exclusions/watchlist-exclusions-table-toolbar.tsx
|
@jamcalli Honest thoughts are, I'm not sure how I feel about the UX with two split tables. From a UX standpoint I think it was easier for a user to manage their watchlist exclusions all from a single table. Now, if a user needs to manage their exclusions they do it from one table, but anything else they have to scroll down to a new table to delete any exclusions, and we may potentially add a different set of search filters for that second table, etc to make it easier for user to remove exclusions, and then the design and UX starts to feel worse with having to manage two different tables with their own filters / potential pagination, etc. This is just my opinion as a user I still prefer the design of a single table use case, but I think there are other positives that we should keep in mind. The |
|
Makes sense. Reverted it to a single table. Also modified the migration so that the global exclusion can persist even when all other references were removed from watchlists.
That being said, let me know how you think global exclusions should be handled. Currently they persist for forever, regardless of watchlist states. This might be desired, or perhaps they should prune when the last item is removed from a watchlist? I will defer to you though, as this is your feature. |
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/services/database/methods/watchlist-exclusion.ts (1)
125-139:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGlobal exclusions are still filtered out by the inner join
Line 126 uses an inner join, so
user_id = 0sentinel exclusions won’t appear in “all exclusions,” making them unmanageable from this path.Suggested fix
- const rows = await this.knex('watchlist_exclusions as we') - .join('users as u', 'we.user_id', 'u.id') + const rows = await this.knex('watchlist_exclusions as we') + .leftJoin('users as u', 'we.user_id', 'u.id') .select( 'we.id', 'we.user_id', 'we.key', 'we.title', 'we.type', 'we.guids', 'we.excluded_at', - 'u.name as username', + this.knex.raw("COALESCE(u.name, 'Global') as username"), )🤖 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/services/database/methods/watchlist-exclusion.ts` around lines 125 - 139, The query in the watchlist-exclusion retrieval uses an inner join on 'users as u' which drops sentinel global exclusions where user_id = 0; change the join to a left join (e.g., use this.knex(...).leftJoin('users as u', 'we.user_id', 'u.id')) so rows with no matching user are retained, and ensure downstream handling of the selected username (u.name as username) tolerates nulls (e.g., leave username null or set a default) while keeping the parseGuids(row.guids) mapping intact.
🤖 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.
Duplicate comments:
In `@src/services/database/methods/watchlist-exclusion.ts`:
- Around line 125-139: The query in the watchlist-exclusion retrieval uses an
inner join on 'users as u' which drops sentinel global exclusions where user_id
= 0; change the join to a left join (e.g., use this.knex(...).leftJoin('users as
u', 'we.user_id', 'u.id')) so rows with no matching user are retained, and
ensure downstream handling of the selected username (u.name as username)
tolerates nulls (e.g., leave username null or set a default) while keeping the
parseGuids(row.guids) mapping intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 618fb18b-701f-4bb6-894a-f748f089dc87
📒 Files selected for processing (14)
migrations/migrations/091_20260516_add_watchlist_exclusions.tssrc/client/features/utilities/components/watchlist-exclusions/watchlist-exclusions-bulk-modal.tsxsrc/client/features/utilities/components/watchlist-exclusions/watchlist-exclusions-bulk-remove-modal.tsxsrc/client/features/utilities/components/watchlist-exclusions/watchlist-exclusions-delete-confirmation-modal.tsxsrc/client/features/utilities/components/watchlist-exclusions/watchlist-exclusions-exclude-confirmation-modal.tsxsrc/client/features/utilities/components/watchlist-exclusions/watchlist-exclusions-table-columns.tsxsrc/client/features/utilities/components/watchlist-exclusions/watchlist-exclusions-table-toolbar.tsxsrc/client/features/utilities/components/watchlist-exclusions/watchlist-exclusions-table.tsxsrc/client/features/utilities/pages/watchlist-exclusions.tsxsrc/routes/v1/watchlist-exclusions/watchlist-exclusions.tssrc/schemas/watchlist-exclusions/watchlist-exclusions.schema.tssrc/services/database/methods/watchlist-exclusion.tssrc/services/database/types/watchlist-exclusion-methods.tssrc/types/watchlist-exclusion.types.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- src/client/features/utilities/pages/watchlist-exclusions.tsx
- src/client/features/utilities/components/watchlist-exclusions/watchlist-exclusions-exclude-confirmation-modal.tsx
- src/client/features/utilities/components/watchlist-exclusions/watchlist-exclusions-bulk-modal.tsx
- src/routes/v1/watchlist-exclusions/watchlist-exclusions.ts
- src/services/database/types/watchlist-exclusion-methods.ts
- src/schemas/watchlist-exclusions/watchlist-exclusions.schema.ts
- src/client/features/utilities/components/watchlist-exclusions/watchlist-exclusions-table.tsx
|
I'll actually defer to your original intent on this because the global exclusions is part of your motivation: If one of the ideas behind global exclusions is to provide an integration point with Maintainerr, then it makes perfect sense to me that global exclusions should be independent of a user's watchlists. It should hold a stricter bar than user watchlist exclusions. We wouldn't want to get into a situation where a global level exclusion that's added by maintainerr in Pulsarr is being overwritten by a user removing the item from their watchlist. It's a strict barrier for current and future users. Global exclusions feels like it should be stricter than simple user watchlist exclusions |
|
I conjoined back into a single table with a global user. Check it out and let me know. Still provides the same functionality. |
|
Yep I checked it out. It looks really good to me imo. There's some fine tunings that we can do but the general layout looks pretty solid to me |
|
What sort of fine tuning are you thinking? |
Some of these are low hanging fruits, some of these are more involved decisions, but yeah this is what I was thinking about for future PRs |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/client/features/utilities/components/watchlist-exclusions/watchlist-exclusions-table-toolbar.tsx (1)
170-171: 💤 Low valueConsider defining a typed meta interface for columns.
The type assertion
(column.columnDef.meta as { displayName?: string })is safe but could benefit from a typed interface. This is an optional enhancement for better type safety across column definitions.♻️ Optional typed meta interface
Define a column meta type in
watchlist-exclusions-table-columns.tsx:interface ColumnMeta { displayName?: string } // Then use it in column definitions: const columns: ColumnDef<WatchlistExclusionTableRow, unknown>[] = [ { id: 'example', meta: { displayName: 'Example Column' } satisfies ColumnMeta, // ... } ]And reference it here:
- {(column.columnDef.meta as { displayName?: string }) - ?.displayName || column.id} + {(column.columnDef.meta as ColumnMeta)?.displayName || column.id}🤖 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/client/features/utilities/components/watchlist-exclusions/watchlist-exclusions-table-toolbar.tsx` around lines 170 - 171, Define a typed meta interface (e.g., interface ColumnMeta { displayName?: string }) and use it as the ColumnDef generic for your table columns (e.g., ColumnDef<WatchlistExclusionTableRow, ColumnMeta> and ensure each column.meta satisfies ColumnMeta when you build the columns array), then replace the inline assertion (column.columnDef.meta as { displayName?: string }) with safe typed access (column.columnDef.meta?.displayName) so TypeScript knows meta has the displayName property; update the columns definition file where the columns array is created and the toolbar component that reads column.columnDef.meta.
🤖 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/client/features/utilities/components/watchlist-exclusions/watchlist-exclusions-table-toolbar.tsx`:
- Around line 170-171: Define a typed meta interface (e.g., interface ColumnMeta
{ displayName?: string }) and use it as the ColumnDef generic for your table
columns (e.g., ColumnDef<WatchlistExclusionTableRow, ColumnMeta> and ensure each
column.meta satisfies ColumnMeta when you build the columns array), then replace
the inline assertion (column.columnDef.meta as { displayName?: string }) with
safe typed access (column.columnDef.meta?.displayName) so TypeScript knows meta
has the displayName property; update the columns definition file where the
columns array is created and the toolbar component that reads
column.columnDef.meta.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 1f1aca8d-7f34-4539-aa9b-1d4f2543415e
📒 Files selected for processing (7)
src/client/components/ui/alert.tsxsrc/client/features/utilities/components/watchlist-exclusions/watchlist-exclusions-bulk-modal.tsxsrc/client/features/utilities/components/watchlist-exclusions/watchlist-exclusions-bulk-remove-modal.tsxsrc/client/features/utilities/components/watchlist-exclusions/watchlist-exclusions-delete-confirmation-modal.tsxsrc/client/features/utilities/components/watchlist-exclusions/watchlist-exclusions-exclude-confirmation-modal.tsxsrc/client/features/utilities/components/watchlist-exclusions/watchlist-exclusions-table-columns.tsxsrc/client/features/utilities/components/watchlist-exclusions/watchlist-exclusions-table-toolbar.tsx
🚧 Files skipped from review as they are similar to previous changes (5)
- src/client/features/utilities/components/watchlist-exclusions/watchlist-exclusions-delete-confirmation-modal.tsx
- src/client/features/utilities/components/watchlist-exclusions/watchlist-exclusions-bulk-remove-modal.tsx
- src/client/features/utilities/components/watchlist-exclusions/watchlist-exclusions-exclude-confirmation-modal.tsx
- src/client/features/utilities/components/watchlist-exclusions/watchlist-exclusions-bulk-modal.tsx
- src/client/features/utilities/components/watchlist-exclusions/watchlist-exclusions-table-columns.tsx
|
Fair points. I've addressed them all. Give that a once over. I reused an existing yellow-ish color from the pallet. Let me know if that looks better. The global user already was present in the users table as a separate row, but perhaps causes confusion when an item is globally excluded. Modified how the individual users exclusions and global exclusions interact. Give that a once over too and let me know your thoughts. |
The Global synthetic entry was appended after the alphabetical user list, which buried it below real users in the dropdown. Prepend it so it stays anchored at the top regardless of user count or name.
…bulk warning The 'X items are already in your library — next Delete Sync run will remove them' notice previously rendered as a separate Alert beneath the main warning. Two same-color same-icon panels stacked back-to-back read as visually indecisive, and the second one's lack of title made it feel like an orphaned continuation. Fold the notice into the primary warning's AlertDescription as a second paragraph with mt-2 spacing. Phrasing changes from 'X items are already in your library' to 'X of them are already in your library' to tie the count to the exclusion count above. This also aligns the bulk modal with the single-item exclude modal, which already folds the same sentence into one Alert.
|
Oooh yes, this looks really nice, I like it: 
One thing I'll also mention is, for the user experience, it may be better for Global to be pinned to to the top of the user filter list, so I created a commit to do this: I also created a commit to collapse the two warnings for bulk exclude into a single warning panel. Before the first warning panel was yellow and the second was blue, so visually this looks a bit stronger: 
Everything else genuinely looks really good to me. Let me know if you have any other thoughts on this / the changes made. |
|
I think we are almost there. A couple small wrinkles to iron out. Currently, the delete sync only removes excluded items in watchlist-based removal. It doesn't for tag-based. Is this intentional or an oversight? |
AhmedNSidd
force-pushed
the
feature/watchlist-exclusions
branch
from
57daa9d to
d219849
Compare
Watchlist-mode delete sync already respects exclusions via
cleanupExcludedWatchlistItems, which drops per-user watchlist_items
rows so the standard prune naturally removes items whose last
watchlister has excluded them. Tag-based mode bypassed that mechanism
entirely — it walked watchlist_items only via the per-user
consensus-tag computation in user-tag.service, and delete-sync itself
just trusted the consensus tag. As a result, both per-user and global
exclusions had no effect on tag-based delete-sync's decisions.
Add a parallel deletion signal: getExclusionDrivenDeletionGuids()
returns the union of GUIDs across keys where exclusions effectively
cover every current watchlister. A key qualifies when, after
subtracting per-user excluders from the set of users with
watchlist_items rows for it (status != 'pending'), no non-excluded
watchlister remains. Global exclusions short-circuit this check (any
user is "covered" by a global), so any item with a global exclusion
qualifies regardless of watchlist state. This mirrors watchlist-mode's
semantic — last excluder wins; one user excluding while others still
want it is a no-op — but expressed as a single read so tag-based mode
doesn't need to mutate watchlist_items.
validateTagBasedDeletion and the safety-check counters short-circuit
their hasRemovalTag gate when the item's GUIDs intersect the
exclusion-driven set. Downstream filters (Plex playlist protection,
deleteSyncTrackedOnly) still apply, with one carve-out: the
deleteSyncRequiredTagRegex filter is bypassed for exclusion-driven
candidates since exclusions are explicit deletion decisions that
shouldn't be gated by tag-pattern filters.
The DB method uses raw SQL because the doubly-nested correlated
subquery doesn't fit knex's nested-this typing. Tested against both
postgres ({ rows: [...] }) and sqlite (array) result shapes.
AhmedNSidd
force-pushed
the
feature/watchlist-exclusions
branch
from
e73ce9b to
601b64f
Compare
|
Yeah, that was an oversight, thanks for catching. Just pushed a fix — tag-based mode now respects exclusions (both per-user and global) the same way watchlist mode does. One small thing worth flagging: I let exclusion-driven candidates bypass the Let me know what you think |
|
Cheers. Almost there. I noticed that the item-router for etag and rss both don't avoid routing the excluded items. Was this intentional? If not, any removal / re-addition to the watchlist would re-route. I pushed a small commit mirror that logic from the full-sync. If this omission is intentional we can roll that back. |
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/services/delete-sync.service.ts (1)
286-307:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't let exclusions trip the empty-watchlist safety abort.
cleanupExcludedWatchlistItems()runs before the Line 306 empty-set guard. If exclusions remove the last routedwatchlist_itemsrow,allWatchlistItems.sizebecomes0and the run aborts as if refresh failed, so a fully excluded watchlist can never drive deletions. Dry runs also diverge because this cleanup is skipped there.🤖 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/services/delete-sync.service.ts` around lines 286 - 307, The safety check is happening after cleanupExcludedWatchlistItems so exclusions can zero out allWatchlistItems and incorrectly trigger handleSafetyTriggered, and dry runs diverge because cleanup is skipped; fix by calling getAllWatchlistItems(...) and evaluating allWatchlistItems.size before invoking cleanupExcludedWatchlistItems(), keeping the cleanup conditional on !dryRun so dry runs retain the original set for the safety check; ensure references to cleanupExcludedWatchlistItems, getAllWatchlistItems, allWatchlistItems.size, dryRun and handleSafetyTriggered are updated accordingly.
♻️ Duplicate comments (1)
src/services/database/methods/watchlist-exclusion.ts (1)
125-140:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGlobal exclusions are silently dropped from the "all exclusions" query.
Line 126 uses an inner join to
users, so sentinel global rows (user_id = 0) never appear in results. This makes global exclusions invisible/unmanageable via this endpoint.Suggested fix
export async function getAllExclusions( this: DatabaseService, ): Promise<Array<WatchlistExclusion & { username: string }>> { const rows = await this.knex('watchlist_exclusions as we') - .join('users as u', 'we.user_id', 'u.id') + .leftJoin('users as u', 'we.user_id', 'u.id') .select( 'we.id', 'we.user_id', 'we.key', 'we.title', 'we.type', 'we.guids', 'we.excluded_at', - 'u.name as username', + this.knex.raw("COALESCE(u.name, 'Global') as username"), ) .orderBy('we.excluded_at', 'desc') }🤖 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/services/database/methods/watchlist-exclusion.ts` around lines 125 - 140, The query against 'watchlist_exclusions as we' currently uses an inner join to 'users as u', which drops sentinel global rows (user_id = 0); change the join to a leftJoin so rows with no matching user are returned, and update the projection/mapping in the result mapping (the rows.map that calls parseGuids) to handle null/undefined u.name (e.g., set username to null or a canonical value like 'global' or 'system') so global exclusions are visible and properly represented.
🧹 Nitpick comments (2)
migrations/migrations/091_20260516_add_watchlist_exclusions.ts (1)
15-15: Verify JSON column default'[]'behavior across SQLite and PostgreSQL
'[]'is a valid JSON array literal, so it should parse/cast correctly as theDEFAULTfor bothjson/jsonbin PostgreSQL and as plain text for SQLite; this line is consistent with that. Optional: if you want extra Postgres unambiguity, use an explicit cast (e.g.,'[]'::jsonb) instead of relying on implicit casting.🤖 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 `@migrations/migrations/091_20260516_add_watchlist_exclusions.ts` at line 15, The JSON column default currently uses a plain string literal ('[]') which works but to ensure Postgres unambiguity update the migration to set the default using an explicit cast for Postgres and keep the plain string for SQLite: detect the active client and use table.json('guids').defaultTo(knex.raw("'[]'::jsonb")) when client is 'pg'/'postgresql' and table.json('guids').defaultTo('[]') for sqlite; reference the migration symbol table.json('guids') in 091_20260516_add_watchlist_exclusions.ts and implement the client-conditional default assignment using knex.client.config.client or a similar runtime client check.src/client/components/table/data-table-select-column.tsx (1)
36-36: 💤 Low valueConsider using a ternary for the conditional meta spread.
The current pattern
...(options.meta !== undefined && { meta: options.meta })works (spreadsfalsewhen meta is undefined, which is a no-op), but a ternary is more idiomatic and clearer:- ...(options.meta !== undefined && { meta: options.meta }), + ...(options.meta !== undefined ? { meta: options.meta } : {}),🤖 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/client/components/table/data-table-select-column.tsx` at line 36, In the object spread where options.meta is conditionally included (currently written as ...(options.meta !== undefined && { meta: options.meta })) replace that pattern with an explicit ternary to make intent clearer: use ...(options.meta !== undefined ? { meta: options.meta } : {}) so the meta property is only spread when present; update the code in data-table-select-column.tsx at the object construction that references options.meta.
🤖 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 `@migrations/migrations/091_20260516_add_watchlist_exclusions.ts`:
- Around line 6-11: The migration adds a NOT NULL foreign key on column user_id
referencing users.id but the codebase uses SYSTEM_USER_ID = 0 as a sentinel for
global exclusions, causing FK violations for id=0; change the migration to allow
global exclusions by making table.column user_id nullable and remove or alter
the direct FK constraint so NULL represents global entries (and update any
unique/index constraints accordingly), then update application logic in
src/services/database/methods/watchlist-exclusion.ts to treat user_id IS NULL as
the global exclusion instead of 0 (or alternatively insert a system user id=0
row if you prefer that approach).
In `@src/services/database/methods/watchlist-exclusion.ts`:
- Around line 24-44: The root problem is the migration FK on
watchlist_exclusions.user_id blocks inserts for SYSTEM_USER_ID (0); update the
migration that creates the watchlist_exclusions table to relax that constraint
(choose one: allow a system row by inserting a users.id = 0, remove/skip the FK
for system entries, or make user_id nullable and use NULL for system entries, or
add a conditional FK/check that exempts 0). After changing the migration, update
the insert path in watchlist-exclusion.ts (the code that builds rows using
userIds and calls this.knex('watchlist_exclusions').insert(...)) to match the
new schema (e.g., filter out or map SYSTEM_USER_ID to NULL if you made user_id
nullable), so inserts no longer violate the FK.
---
Outside diff comments:
In `@src/services/delete-sync.service.ts`:
- Around line 286-307: The safety check is happening after
cleanupExcludedWatchlistItems so exclusions can zero out allWatchlistItems and
incorrectly trigger handleSafetyTriggered, and dry runs diverge because cleanup
is skipped; fix by calling getAllWatchlistItems(...) and evaluating
allWatchlistItems.size before invoking cleanupExcludedWatchlistItems(), keeping
the cleanup conditional on !dryRun so dry runs retain the original set for the
safety check; ensure references to cleanupExcludedWatchlistItems,
getAllWatchlistItems, allWatchlistItems.size, dryRun and handleSafetyTriggered
are updated accordingly.
---
Duplicate comments:
In `@src/services/database/methods/watchlist-exclusion.ts`:
- Around line 125-140: The query against 'watchlist_exclusions as we' currently
uses an inner join to 'users as u', which drops sentinel global rows (user_id =
0); change the join to a leftJoin so rows with no matching user are returned,
and update the projection/mapping in the result mapping (the rows.map that calls
parseGuids) to handle null/undefined u.name (e.g., set username to null or a
canonical value like 'global' or 'system') so global exclusions are visible and
properly represented.
---
Nitpick comments:
In `@migrations/migrations/091_20260516_add_watchlist_exclusions.ts`:
- Line 15: The JSON column default currently uses a plain string literal ('[]')
which works but to ensure Postgres unambiguity update the migration to set the
default using an explicit cast for Postgres and keep the plain string for
SQLite: detect the active client and use
table.json('guids').defaultTo(knex.raw("'[]'::jsonb")) when client is
'pg'/'postgresql' and table.json('guids').defaultTo('[]') for sqlite; reference
the migration symbol table.json('guids') in
091_20260516_add_watchlist_exclusions.ts and implement the client-conditional
default assignment using knex.client.config.client or a similar runtime client
check.
In `@src/client/components/table/data-table-select-column.tsx`:
- Line 36: In the object spread where options.meta is conditionally included
(currently written as ...(options.meta !== undefined && { meta: options.meta }))
replace that pattern with an explicit ternary to make intent clearer: use
...(options.meta !== undefined ? { meta: options.meta } : {}) so the meta
property is only spread when present; update the code in
data-table-select-column.tsx at the object construction that references
options.meta.
🪄 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: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 94c12a03-4526-473b-b009-55fb7ac46281
⛔ Files ignored due to path filters (31)
docs/docs/api-documentation.mdis excluded by!docs/**docs/docs/api/bulk-manage-rolling-monitored-shows.api.mdxis excluded by!docs/**docs/docs/api/configure-plex-notifications.api.mdxis excluded by!docs/**docs/docs/api/create-radarr-instance.api.mdxis excluded by!docs/**docs/docs/api/create-radarr-tag.api.mdxis excluded by!docs/**docs/docs/api/create-schedule.api.mdxis excluded by!docs/**docs/docs/api/create-sonarr-instance.api.mdxis excluded by!docs/**docs/docs/api/create-sonarr-tag.api.mdxis excluded by!docs/**docs/docs/api/get-all-dashboard-stats.api.mdxis excluded by!docs/**docs/docs/api/get-all-schedules.api.mdxis excluded by!docs/**docs/docs/api/get-most-watched-movies.api.mdxis excluded by!docs/**docs/docs/api/get-most-watched-shows.api.mdxis excluded by!docs/**docs/docs/api/get-radarr-quality-profiles.api.mdxis excluded by!docs/**docs/docs/api/get-radarr-root-folders.api.mdxis excluded by!docs/**docs/docs/api/get-radarr-tags.api.mdxis excluded by!docs/**docs/docs/api/get-schedule-by-name.api.mdxis excluded by!docs/**docs/docs/api/get-sonarr-quality-profiles.api.mdxis excluded by!docs/**docs/docs/api/get-sonarr-root-folders.api.mdxis excluded by!docs/**docs/docs/api/get-sonarr-shows.api.mdxis excluded by!docs/**docs/docs/api/get-sonarr-tags.api.mdxis excluded by!docs/**docs/docs/api/get-top-genres.api.mdxis excluded by!docs/**docs/docs/api/get-top-users.api.mdxis excluded by!docs/**docs/docs/api/get-user-by-id.api.mdxis excluded by!docs/**docs/docs/api/sync-instance.api.mdxis excluded by!docs/**docs/docs/api/update-schedule.api.mdxis excluded by!docs/**docs/docs/api/update-user.api.mdxis excluded by!docs/**docs/docs/intro.mdis excluded by!docs/**docs/docs/utilities/watchlist-exclusions.mdis excluded by!docs/**docs/sidebars.tsis excluded by!docs/**docs/static/img/Watchlist-Exclusions.pngis excluded by!**/*.png,!docs/**,!**/*.{png,jpg,jpeg,gif,svg,webp,ico,woff,woff2}docs/static/openapi.jsonis excluded by!docs/**
📒 Files selected for processing (42)
README.mdmigrations/migrations/091_20260516_add_watchlist_exclusions.tspackage.jsonsrc/client/components/AppSidebar.tsxsrc/client/components/table/data-table-select-column.tsxsrc/client/components/ui/alert.tsxsrc/client/features/approvals/components/approval-table-columns.tsxsrc/client/features/plex/components/user/user-table.tsxsrc/client/features/utilities/components/session-monitoring/manage-rolling-sheet.tsxsrc/client/features/utilities/components/watchlist-exclusions/watchlist-exclusions-bulk-modal.tsxsrc/client/features/utilities/components/watchlist-exclusions/watchlist-exclusions-bulk-remove-modal.tsxsrc/client/features/utilities/components/watchlist-exclusions/watchlist-exclusions-delete-confirmation-modal.tsxsrc/client/features/utilities/components/watchlist-exclusions/watchlist-exclusions-exclude-confirmation-modal.tsxsrc/client/features/utilities/components/watchlist-exclusions/watchlist-exclusions-skeleton.tsxsrc/client/features/utilities/components/watchlist-exclusions/watchlist-exclusions-table-columns.tsxsrc/client/features/utilities/components/watchlist-exclusions/watchlist-exclusions-table-toolbar.tsxsrc/client/features/utilities/components/watchlist-exclusions/watchlist-exclusions-table.tsxsrc/client/features/utilities/hooks/useWatchlistExclusionMutations.tssrc/client/features/utilities/hooks/useWatchlistExclusions.tssrc/client/features/utilities/pages/watchlist-exclusions.tsxsrc/client/router/router.tsxsrc/plugins/external/swagger.tssrc/routes/v1/watchlist-exclusions/watchlist-exclusions.tssrc/schemas/watchlist-exclusions/watchlist-exclusions.schema.tssrc/services/approval.service.tssrc/services/content-router.service.tssrc/services/database.service.tssrc/services/database/methods/watchlist-exclusion.tssrc/services/database/methods/watchlist.tssrc/services/database/types/watchlist-exclusion-methods.tssrc/services/delete-sync.service.tssrc/services/delete-sync/orchestration/tag-based-deletion.tssrc/services/delete-sync/tag-operations/tag-counter.tssrc/services/delete-sync/validation/content-validator.tssrc/services/plex-watchlist/orchestration/unified-processor.tssrc/services/watchlist-workflow/orchestration/sync-engine.tssrc/services/watchlist-workflow/routing/item-router.tssrc/types/watchlist-exclusion.types.tssrc/utils/arr-error.tstest/unit/services/delete-sync/tag-operations/tag-counter.test.tstest/unit/services/delete-sync/validation/content-validator.test.tstest/unit/services/watchlist-workflow/routing/item-router.test.ts
jamcalli
force-pushed
the
feature/watchlist-exclusions
branch
from
7fe4129 to
04dd69b
Compare
3 participants
Hey, I've been running Pulsarr for my family's Plex server and ran into a re-request loop. Delete Sync removes content from Sonarr/Radarr, but the items stay on users' Plex watchlists. Next sync cycle, Pulsarr picks them up again and re-requests them. The only fix right now is asking everyone to manually remove items from their watchlists, which doesn't really work at scale.
This adds a watchlist exclusion system — you can mark items to skip during sync directly from a new Utilities page. Exclusions clear automatically when a user removes the item from their watchlist, so re-adding it later still works normally.
What's included
Backend
watchlist_exclusionstable (migration for SQLite and PostgreSQL)/v1/exclusions/exclusions, auto-registered via fastifyAutoloadskippedDueToExclusionstatFrontend
Docs
Testing
Been running this on my own instance. Happy to adjust anything that doesn't fit with the direction of the project.
Summary by CodeRabbit
Release Notes