Add Support for Requested Seasons for Seer Webhook#1392
Open
spelech wants to merge 3 commits intorivenmedia:mainfrom
Open
Add Support for Requested Seasons for Seer Webhook#1392spelech wants to merge 3 commits intorivenmedia:mainfrom
spelech wants to merge 3 commits intorivenmedia:mainfrom
Conversation
Contributor
WalkthroughThe changes add propagation of a requested_seasons field through MediaItem syncing, TVDB indexing, and Overseerr webhook handling, enabling requested-season-aware creation and state transitions for seasons and emitting events when paused seasons are promoted. Changes
Sequence DiagramsequenceDiagram
participant Overseerr as Overseerr
participant Webhook as Webhook Handler
participant DB as Database
participant Show as Show Item
participant Season as Season Item
participant Events as Event System
Overseerr->>Webhook: POST TV item (includes requested_seasons)
Webhook->>DB: get_item_by_external_id(tvdbId/tmdbId)
alt show found
DB-->>Webhook: return existing show
Webhook->>Show: read requested_seasons
loop for each season in Show
alt season.number in requested_seasons and season.last_state == Paused
Webhook->>Season: set last_state = Requested
Webhook->>Events: emit season requested event
end
end
Webhook->>DB: commit
Webhook-->>Overseerr: 200 OK
else show not found
DB-->>Webhook: no show
Webhook->>Show: create new show (tvdb_id) and attach requested_seasons
Webhook->>DB: persist new show
Webhook-->>Overseerr: 200 OK
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 Tip CodeRabbit can use your project's `pylint` configuration to improve the quality of Python code reviews.Add a pylint configuration file to your project to customize how CodeRabbit runs |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/tests/test_overseerr_webhook.py (1)
4-39: These tests never hit the new behavior.They only validate
OverseerrWebhookparsing. The new control flow lives insrc/routers/secure/webhooks.py:77-115andsrc/program/services/indexers/tvdb_indexer.py:402-483, so regressions in the actual requested-seasons path can slip through. Please add one handler-level test for promoting paused seasons on an existing show and one indexer test for a fresh show request carryingrequested_seasons.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/tests/test_overseerr_webhook.py` around lines 4 - 39, Add two tests exercising the actual control flow: (1) a handler-level test that sends an OverseerrWebhook-like payload to the webhooks handler in src/routers/secure/webhooks.py (the function that consumes OverseerrWebhook and promotes paused seasons) for an existing show with paused seasons and assert the handler promotes the requested seasons (e.g., season 1/2/5 become active); (2) an indexer unit test in src/program/services/indexers/tvdb_indexer.py that simulates a fresh show request carrying requested_seasons and invokes the TvdbIndexer method that handles new requests (the indexer function that currently processes requested_seasons) asserting the indexer persists/applies the requested_seasons for the new show; mock external dependencies (DB/indexer clients) as needed and reuse the existing OverseerrWebhook payload structure from src/tests/test_overseerr_webhook.py for consistency.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/program/services/indexers/base.py`:
- Line 39: The mapped Show isn’t getting requested_seasons when creating a fresh
show, so ensure requested_seasons is set on the mapped Show before seasons are
added: in TVDBIndexer._create_show_from_id (the branch that builds a new Show)
assign mapped_show.requested_seasons = source_show.requested_seasons (or
metadata.get('requested_seasons')) immediately after constructing the mapped
Show and before calling _add_seasons_to_show(); additionally, update
copy_items()’s Show branch to include copying show-level attrs
(requested_seasons) so subsequent metadata-copy paths remain consistent.
In `@src/routers/secure/webhooks.py`:
- Around line 92-103: The current loop calls season.store_state() and
Program.em.add_event() before session.commit(), causing notifications and
event-queue reads to see transient/rolled-back data; change the flow to first
collect the target season IDs/objects (from existing_show.seasons where
season.number in requested_seasons and season.last_state == States.Paused),
perform the database state update in a non-notifying way (e.g., set season.state
or use a low-level update) and call session.commit(), then after a successful
commit iterate the collected seasons and call
season.store_state(States.Requested) and di[Program].em.add_event(Event(...)) to
emit events; reference season.store_state, Program.em.add_event,
existing_show.seasons, States.Requested, States.Paused, and
OverseerrWebhookResponse when locating the code to change.
---
Nitpick comments:
In `@src/tests/test_overseerr_webhook.py`:
- Around line 4-39: Add two tests exercising the actual control flow: (1) a
handler-level test that sends an OverseerrWebhook-like payload to the webhooks
handler in src/routers/secure/webhooks.py (the function that consumes
OverseerrWebhook and promotes paused seasons) for an existing show with paused
seasons and assert the handler promotes the requested seasons (e.g., season
1/2/5 become active); (2) an indexer unit test in
src/program/services/indexers/tvdb_indexer.py that simulates a fresh show
request carrying requested_seasons and invokes the TvdbIndexer method that
handles new requests (the indexer function that currently processes
requested_seasons) asserting the indexer persists/applies the requested_seasons
for the new show; mock external dependencies (DB/indexer clients) as needed and
reuse the existing OverseerrWebhook payload structure from
src/tests/test_overseerr_webhook.py for consistency.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: a54c85b5-e6f9-4eb0-9652-069a5fc9bea7
📒 Files selected for processing (4)
src/program/services/indexers/base.pysrc/program/services/indexers/tvdb_indexer.pysrc/routers/secure/webhooks.pysrc/tests/test_overseerr_webhook.py
Comment on lines
+92
to
+103
| if requested_seasons: | ||
| for season in existing_show.seasons: # type: ignore | ||
| if getattr(season, "number", None) in requested_seasons and season.last_state == States.Paused: | ||
| season.store_state(States.Requested) | ||
|
|
||
| di[Program].em.add_event(Event( | ||
| emitted_by=Overseerr.__class__.__name__, | ||
| item_id=season.id | ||
| )) | ||
|
|
||
| session.commit() | ||
| return OverseerrWebhookResponse(success=True) |
Contributor
There was a problem hiding this comment.
Defer state-change side effects until after commit.
season.store_state() notifies immediately (src/program/media/item.py:202-230), and Program.em.add_event() queues by re-reading the item from the DB (src/program/managers/event_manager.py:173-219). Because both happen before Line 102 commits, a failed commit can leak a rolled-back transition, and the queue can still see the old Paused row. At minimum, move event emission behind a successful commit; ideally this whole path should use an after-commit/outbox pattern.
Minimum safer ordering
if existing_show:
logger.info(f"Show {existing_show.log_string} already exists, handling requested seasons from overseerr")
if requested_seasons:
+ season_ids_to_queue = []
for season in existing_show.seasons: # type: ignore
if getattr(season, "number", None) in requested_seasons and season.last_state == States.Paused:
season.store_state(States.Requested)
-
- di[Program].em.add_event(Event(
- emitted_by=Overseerr.__class__.__name__,
- item_id=season.id
- ))
+ if season.id is not None:
+ season_ids_to_queue.append(season.id)
session.commit()
+ for season_id in season_ids_to_queue:
+ di[Program].em.add_event(
+ Event(
+ emitted_by=Overseerr.__class__.__name__,
+ item_id=season_id,
+ )
+ )
return OverseerrWebhookResponse(success=True)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/routers/secure/webhooks.py` around lines 92 - 103, The current loop calls
season.store_state() and Program.em.add_event() before session.commit(), causing
notifications and event-queue reads to see transient/rolled-back data; change
the flow to first collect the target season IDs/objects (from
existing_show.seasons where season.number in requested_seasons and
season.last_state == States.Paused), perform the database state update in a
non-notifying way (e.g., set season.state or use a low-level update) and call
session.commit(), then after a successful commit iterate the collected seasons
and call season.store_state(States.Requested) and
di[Program].em.add_event(Event(...)) to emit events; reference
season.store_state, Program.em.add_event, existing_show.seasons,
States.Requested, States.Paused, and OverseerrWebhookResponse when locating the
code to change.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (3)
src/program/services/indexers/tvdb_indexer.py (3)
448-452: Logic is correct; consider hoisting the import.The season-pausing logic correctly handles the case when
requested_seasonsis specified but doesn't include the current season. However, the inline import ofStatesat line 450 is unusual. SinceStatesis a stable enum with no circular dependency risk, consider moving it to the top-level imports for consistency.♻️ Proposed refactor
Add to top-level imports:
from program.media.state import StatesThen simplify:
if requested_seasons and season_item.number not in requested_seasons: - from program.media.state import States season_item.last_state = States.Paused🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/program/services/indexers/tvdb_indexer.py` around lines 448 - 452, Hoist the inline import of States out of the conditional in tvdb_indexer.py: add "from program.media.state import States" to the module top-level imports and remove the inline "from program.media.state import States" inside the if block; keep the existing logic that sets season_item.last_state = States.Paused when requested_seasons is set and season_item.number is not in requested_seasons (refer to the conditional using requested_seasons and season_item.last_state).
273-275: Same issue: replacesetattrwith direct assignment.Consistent with the fix at lines 246-248.
♻️ Proposed fix
if show_item: if item: - setattr(show_item, "requested_seasons", getattr(item, "requested_seasons", None)) + show_item.requested_seasons = getattr(item, "requested_seasons", None) self._add_seasons_to_show(show_item, show_details)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/program/services/indexers/tvdb_indexer.py` around lines 273 - 275, The code uses setattr(show_item, "requested_seasons", getattr(item, "requested_seasons", None)) which should be replaced with a direct assignment for clarity and consistency with the earlier fix; change this to show_item.requested_seasons = getattr(item, "requested_seasons", None) (keep the getattr to safely fetch from item), leaving the subsequent call to self._add_seasons_to_show(show_item, show_details) unchanged so _add_seasons_to_show continues to operate on the updated show_item.
246-248: Replacesetattrwith direct attribute assignment.Per static analysis (Ruff B010), using
setattrwith a constant attribute name is unnecessary. Direct assignment is clearer and equally safe.♻️ Proposed fix
if show_item: if item: - setattr(show_item, "requested_seasons", getattr(item, "requested_seasons", None)) + show_item.requested_seasons = getattr(item, "requested_seasons", None) self._add_seasons_to_show(show_item, show_details)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/program/services/indexers/tvdb_indexer.py` around lines 246 - 248, Replace the dynamic setattr call on show_item with a direct attribute assignment: when item exists assign show_item.requested_seasons = getattr(item, "requested_seasons", None) instead of using setattr(show_item, "requested_seasons", ...); keep the surrounding logic (the if item check and the subsequent call to self._add_seasons_to_show(show_item, show_details)) unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/program/services/indexers/tvdb_indexer.py`:
- Around line 448-452: Hoist the inline import of States out of the conditional
in tvdb_indexer.py: add "from program.media.state import States" to the module
top-level imports and remove the inline "from program.media.state import States"
inside the if block; keep the existing logic that sets season_item.last_state =
States.Paused when requested_seasons is set and season_item.number is not in
requested_seasons (refer to the conditional using requested_seasons and
season_item.last_state).
- Around line 273-275: The code uses setattr(show_item, "requested_seasons",
getattr(item, "requested_seasons", None)) which should be replaced with a direct
assignment for clarity and consistency with the earlier fix; change this to
show_item.requested_seasons = getattr(item, "requested_seasons", None) (keep the
getattr to safely fetch from item), leaving the subsequent call to
self._add_seasons_to_show(show_item, show_details) unchanged so
_add_seasons_to_show continues to operate on the updated show_item.
- Around line 246-248: Replace the dynamic setattr call on show_item with a
direct attribute assignment: when item exists assign show_item.requested_seasons
= getattr(item, "requested_seasons", None) instead of using setattr(show_item,
"requested_seasons", ...); keep the surrounding logic (the if item check and the
subsequent call to self._add_seasons_to_show(show_item, show_details))
unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 040a730d-090d-49ae-a238-2ffd1bec8b18
📒 Files selected for processing (3)
README.mdsrc/program/services/indexers/base.pysrc/program/services/indexers/tvdb_indexer.py
🚧 Files skipped from review as they are similar to previous changes (1)
- src/program/services/indexers/base.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request Check List
Resolves: #issue-number-here
Description:
Finishes piping through the optional requested seasons field for Seer Webhook notifications. Initial implementation looked to be there but not fully integrated
Summary by CodeRabbit
New Features
Tests
Documentation