Skip to content

fix(subscriber): await season status saves when approving TV requests#2743

Closed
dougrathbone wants to merge 1 commit intoseerr-team:developfrom
dougrathbone:dougrathbone/fix/season-approval-status
Closed

fix(subscriber): await season status saves when approving TV requests#2743
dougrathbone wants to merge 1 commit intoseerr-team:developfrom
dougrathbone:dougrathbone/fix/season-approval-status

Conversation

@dougrathbone
Copy link
Copy Markdown
Contributor

@dougrathbone dougrathbone commented Mar 22, 2026

Description

When a TV MediaRequest is approved, updateParentStatus iterates its SeasonRequest children and marks each one APPROVED. The saves were issued via forEach without await, making them fire-and-forget. Anything reading season statuses immediately after approval -- notification logic, media status checks -- could observe stale PENDING values.

The declined path in the same function already does this correctly using for...of with await. The approved path was inconsistent with it.

Before:

entity.seasons.forEach((season) => {
  season.status = MediaRequestStatus.APPROVED;
  seasonRequestRepository.save(season); // unawaited
});

After (matching the pattern used in the declined branch):

for (const season of entity.seasons) {
  season.status = MediaRequestStatus.APPROVED;
  await seasonRequestRepository.save(season);
}

AI Disclosure: I used Claude Code to help write the tests. I reviewed and understood all generated code before submitting.

How Has This Been Tested?

Integration tests at server/subscriber/MediaRequestSubscriber.test.ts using the real SQLite test database via setupTestDb().

Tests:

  • updateParentStatus -- persists APPROVED status to all child SeasonRequest rows in the database (regression guard for the unawaited save)
  • updateParentStatus -- persists DECLINED status to all child SeasonRequest rows (regression guard for the already-correct declined path)

To reproduce the bug manually:

  1. Create a TV show request spanning multiple seasons.
  2. Approve the request via the admin panel or PUT /api/v1/request/:id/approve.
  3. Immediately fetch the request and inspect seasons[].status.
  4. Before this fix: season statuses may still read PENDING due to unawaited saves racing with the response.
  5. After this fix: all season requests reliably reflect APPROVED.

Screenshots / Logs (if applicable)

N/A -- backend-only change with no UI impact.

Checklist:

  • I have read and followed the contribution guidelines.
  • Disclosed any use of AI (see our policy)
  • I have updated the documentation accordingly. (no documentation changes required for this fix)
  • All new and existing tests passed.
  • Successful build pnpm build
  • Translation keys pnpm i18n:extract (no new UI strings added)
  • Database migration (if required) (no schema changes)

When a TV MediaRequest was approved, child SeasonRequest statuses were
updated via forEach without awaiting the save calls, making them
fire-and-forget. Any code reading season statuses immediately after
approval (notification logic, media status checks) could observe stale
PENDING values.

The declined path in the same function already used for...of with await
correctly. This aligns the approved path to the same pattern.

Adds tests for both paths to confirm status changes are persisted to
the database before updateParentStatus returns.
@dougrathbone dougrathbone requested a review from a team as a code owner March 22, 2026 02:31
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 22, 2026

📝 Walkthrough

Walkthrough

This change adds test coverage for the MediaRequestSubscriber and fixes an async handling issue in the subscriber's updateParentStatus method where season request updates were not being properly awaited.

Changes

Cohort / File(s) Summary
Test File Addition
server/subscriber/MediaRequestSubscriber.test.ts
New test file with 111 lines covering MediaRequestSubscriber.updateParentStatus. Includes test database initialization, helper function to create media and season requests, and two test cases validating that child season statuses are correctly updated to match parent status (APPROVED and DECLINED).
Async Flow Fix
server/subscriber/MediaRequestSubscriber.ts
Changed season update loop from non-awaited synchronous forEach to async for...of loop, ensuring each seasonRequestRepository.save() call is properly awaited before proceeding to the next season.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

Suggested labels

preview

Suggested reviewers

  • fallenbagel

Poem

🐰 A fix most fine, async'd at last!
Seasons await their fate so vast,
No more forgotten saves that slip,
For loops bring order to the trip! ✨

🚥 Pre-merge checks | ✅ 2
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The PR title clearly and specifically describes the main fix: awaiting season status saves when approving TV requests, which directly addresses the root cause identified in the PR objectives.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ 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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@dougrathbone
Copy link
Copy Markdown
Contributor Author

Closing this - the exact same fix was already merged in #2760. Nice to see someone else spotted it too!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant