fix: update series status to partially available when seasons are missing#3044
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:
📝 WalkthroughWalkthroughIncorporates TMDB lookups into AvailabilitySync to backfill missing seasons, tightens season deletion/status transitions, makes availability rollups scanner-input-aware, adjusts Jellyfin/Plex/Sonarr season handling, normalizes logging, and adds tests for Jellyfin and AvailabilitySync TMDB behavior. ChangesMedia Availability Sync and Status Computation
Scanner Logic and Tests
Other
Sequence DiagramsequenceDiagram
participant AvailabilitySync
participant TheMovieDb
participant SeasonUpdater
participant Database
AvailabilitySync->>TheMovieDb: getTvShow(tmdbId or tvdbId)
TheMovieDb-->>AvailabilitySync: TmdbTvDetails (seasons with episode_count)
AvailabilitySync->>SeasonUpdater: merge TMDB seasons into finalSeasons/finalSeasons4k
SeasonUpdater->>Database: skip UNKNOWN deletions, update season statuses, possibly set show PARTIALLY_AVAILABLE
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 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.
Actionable comments posted: 2
🧹 Nitpick comments (1)
server/lib/availabilitySync.ts (1)
360-372: ⚡ Quick winAdd error handling for TMDB API calls.
If
getTvShoworgetShowByTvdbIdfails (network error, rate limiting, 404), the error bubbles up and the entire media item is skipped. Consider wrapping in try-catch and continuing with best-effort season tracking when TMDB is unavailable, rather than failing the entire item.♻️ Proposed fix
// We need to fetch from TMDB to get the episode count for each season let tvShow: TmdbTvDetails; - if (media.tmdbId) { - tvShow = await this.tmdb.getTvShow({ - tvId: Number(media.tmdbId), - }); - } else if (media.tvdbId) { - tvShow = await this.tmdb.getShowByTvdbId({ - tvdbId: Number(media.tvdbId), - }); - } else { - throw new Error('No ID provided'); - } + try { + if (media.tmdbId) { + tvShow = await this.tmdb.getTvShow({ + tvId: Number(media.tmdbId), + }); + } else if (media.tvdbId) { + tvShow = await this.tmdb.getShowByTvdbId({ + tvdbId: Number(media.tvdbId), + }); + } else { + throw new Error('No ID provided'); + } + } catch (e) { + logger.debug( + `Failed to fetch TMDB data for show [TMDB ID ${media.tmdbId}]. Skipping TMDB season enrichment.`, + { label: 'AvailabilitySync', errorMessage: e.message } + ); + tvShow = { seasons: [] } as TmdbTvDetails; + }🤖 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 `@server/lib/availabilitySync.ts` around lines 360 - 372, The TMDB calls (getTvShow / getShowByTvdbId) can throw and currently bubble up; wrap the calls that assign tvShow in a try-catch around the branches where you call this.tmdb.getTvShow and this.tmdb.getShowByTvdbId, catching errors, logging a warning (include error.message and media identifiers), and then fall back to best-effort processing: if tvShow is not available use media.seasons (or assume unknown episode counts) to continue season tracking rather than throwing; ensure the rest of the function handles a possibly undefined tvShow value safely (check tvShow before reading properties) so the media item is not skipped on TMDB failures.
🤖 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/lib/availabilitySync.test.ts`:
- Around line 692-790: The test fails because availabilitySync.run() now calls
TheMovieDb.getTvShow/getShowByTvdbId but this test doesn't mock them; add a
TheMovieDb mock (create getTvShowImpl and expose it via Object.defineProperty on
TheMovieDb.prototype for getTvShow and getShowByTvdbId) placed after the
SonarrAPI mock so tests don't call the real API, reset getTvShowImpl in
beforeEach, and in this specific test set getTvShowImpl to return a minimal
TmdbTvDetails with seasons containing appropriate episode_count values for each
season.
In `@server/lib/scanners/baseScanner.ts`:
- Around line 473-478: The filter expression using seasons.find(...)?
.totalEpisodes ?? Infinity > 0 has ambiguous operator precedence and should be
parenthesized as (seasons.find(...)? .totalEpisodes ?? Infinity) > 0; update all
occurrences of that expression in baseScanner.ts (the places paired with
.filter(...).every((s) => s.status === MediaStatus.AVAILABLE) and the two other
similar filter/every blocks noted in the review) so the nullish-coalescing
applies before the > comparison to preserve the intended logic.
---
Nitpick comments:
In `@server/lib/availabilitySync.ts`:
- Around line 360-372: The TMDB calls (getTvShow / getShowByTvdbId) can throw
and currently bubble up; wrap the calls that assign tvShow in a try-catch around
the branches where you call this.tmdb.getTvShow and this.tmdb.getShowByTvdbId,
catching errors, logging a warning (include error.message and media
identifiers), and then fall back to best-effort processing: if tvShow is not
available use media.seasons (or assume unknown episode counts) to continue
season tracking rather than throwing; ensure the rest of the function handles a
possibly undefined tvShow value safely (check tvShow before reading properties)
so the media item is not skipped on TMDB failures.
🪄 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: 61437a59-9bd3-494d-a8c3-6c29c9267948
📒 Files selected for processing (6)
next-env.d.tsserver/lib/availabilitySync.test.tsserver/lib/availabilitySync.tsserver/lib/scanners/baseScanner.tsserver/lib/scanners/jellyfin/index.tsserver/lib/scanners/jellyfin/jellyfin.test.ts
bb3fe2c to
4ba9c28
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/lib/scanners/sonarr/index.ts`:
- Around line 162-179: When building filteredSeasons from the TMDB-driven
tvShow.seasons, apply the enableSpecialEpisodes gate before mapping so season 0
(specials) is excluded when enableSpecialEpisodes is false; filter
tvShow.seasons (or use a conditional .filter) to skip entries with season_number
=== 0 when enableSpecialEpisodes is falsy, then map the remaining seasons to the
existing logic that looks up sonarrSeries.seasons and returns either the
sonarrSeason or the constructed object; update the code around filteredSeasons
to reference enableSpecialEpisodes and keep the rest of the mapping unchanged.
🪄 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: 706841c0-5822-4893-ae11-a9e2a305a953
📒 Files selected for processing (7)
next-env.d.tsserver/lib/availabilitySync.test.tsserver/lib/availabilitySync.tsserver/lib/scanners/baseScanner.tsserver/lib/scanners/jellyfin/index.tsserver/lib/scanners/jellyfin/jellyfin.test.tsserver/lib/scanners/sonarr/index.ts
✅ Files skipped from review due to trivial changes (1)
- next-env.d.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- server/lib/availabilitySync.test.ts
- server/lib/scanners/jellyfin/index.ts
- server/lib/scanners/baseScanner.ts
- server/lib/scanners/jellyfin/jellyfin.test.ts
- server/lib/availabilitySync.ts
4ba9c28 to
b092069
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
server/lib/availabilitySync.test.ts (1)
775-873: ⚡ Quick winOverride getTvShowImpl to match the 4-season test scenario.
The test sets up a 4-season show (seasons 1-4) with Sonarr returning 4 seasons, but the TMDB mock uses the
beforeEachdefault which returns 12 seasons. This mismatch means availabilitySync will process 12 seasons instead of the 4 described in the test scenario, potentially adding seasons 5-12 as UNKNOWN.While the test will still pass (the result will be PARTIALLY_AVAILABLE regardless), overriding
getTvShowImplto return exactly 4 seasons would make the test more accurate and easier to understand.♻️ Suggested override for clarity
Add this before calling
availabilitySync.run()(after line 859):}; + getTvShowImpl = async ({ tvId }) => + fakeTmdbShow(tvId, [ + { id: 1, air_date: '2024-01-01', episode_count: 10, name: 'Season 1', overview: '', season_number: 1 }, + { id: 2, air_date: '2024-01-01', episode_count: 10, name: 'Season 2', overview: '', season_number: 2 }, + { id: 3, air_date: '2024-01-01', episode_count: 10, name: 'Season 3', overview: '', season_number: 3 }, + { id: 4, air_date: '2024-01-01', episode_count: 10, name: 'Season 4', overview: '', season_number: 4 }, + ]); + await availabilitySync.run();🤖 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 `@server/lib/availabilitySync.test.ts` around lines 775 - 873, The TMDB mock used in beforeEach returns 12 seasons which conflicts with this test's 4-season scenario; override getTvShowImpl to return a TMDB show with exactly 4 seasons (matching seasons 1–4) before calling availabilitySync.run(); locate the test and add a getTvShowImpl implementation that returns the 4-season fake TMDB show (using the same fake helper used elsewhere) so availabilitySync processes only the intended four seasons.
🤖 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 `@server/lib/availabilitySync.test.ts`:
- Around line 775-873: The TMDB mock used in beforeEach returns 12 seasons which
conflicts with this test's 4-season scenario; override getTvShowImpl to return a
TMDB show with exactly 4 seasons (matching seasons 1–4) before calling
availabilitySync.run(); locate the test and add a getTvShowImpl implementation
that returns the 4-season fake TMDB show (using the same fake helper used
elsewhere) so availabilitySync processes only the intended four seasons.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 147c2750-b38a-42f1-a106-04b44d795e8a
📒 Files selected for processing (7)
next-env.d.tsserver/lib/availabilitySync.test.tsserver/lib/availabilitySync.tsserver/lib/scanners/baseScanner.tsserver/lib/scanners/jellyfin/index.tsserver/lib/scanners/jellyfin/jellyfin.test.tsserver/lib/scanners/sonarr/index.ts
✅ Files skipped from review due to trivial changes (1)
- next-env.d.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- server/lib/scanners/jellyfin/index.ts
- server/lib/scanners/baseScanner.ts
- server/lib/scanners/sonarr/index.ts
- server/lib/scanners/jellyfin/jellyfin.test.ts
- server/lib/availabilitySync.ts
b092069 to
88a2171
Compare
fallenbagel
left a comment
There was a problem hiding this comment.
A few things I noticed that might cause issues or are worth addressing.
Also the test nit from @coderabbitai looks good to fix. The new "PARTIALLY_AVAILABLE when some seasons are AVAILABLE and some are UNKNOWN" test uses the default 12-season beforeEach setup, even though the scenario describes a 4-season show. The assertion still holds, but overriding getTvShowImpl with a 4-season mock would make the test easier to read.
88a2171 to
9d75818
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
server/lib/availabilitySync.ts (1)
616-632: 💤 Low valueMinor log message accuracy improvement.
The log message lists
seasonKeysas "not found", but the deletion loop above may have skipped some of those seasons if they were in PROCESSING or UNKNOWN state. Consider tracking which seasons were actually marked DELETED for a more accurate log.🔧 Suggested improvement
+ const deletedSeasons: number[] = []; for (const mediaSeason of media.seasons) { if ( seasonsPendingRemoval.has(mediaSeason.seasonNumber) && (mediaSeason[is4k ? 'status4k' : 'status'] === MediaStatus.AVAILABLE || mediaSeason[is4k ? 'status4k' : 'status'] === MediaStatus.PARTIALLY_AVAILABLE) ) { mediaSeason[is4k ? 'status4k' : 'status'] = MediaStatus.DELETED; + deletedSeasons.push(mediaSeason.seasonNumber); } } if (media[is4k ? 'status4k' : 'status'] === MediaStatus.AVAILABLE) { media[is4k ? 'status4k' : 'status'] = MediaStatus.PARTIALLY_AVAILABLE; logger.debug( `Marking the ${ is4k ? '4K' : 'non-4K' - } show [TMDB ID ${media.tmdbId}] as PARTIALLY_AVAILABLE because season(s) [${seasonKeys}] was not found in any ${ + } show [TMDB ID ${media.tmdbId}] as PARTIALLY_AVAILABLE because season(s) [${deletedSeasons.length > 0 ? deletedSeasons : 'some'}] ${deletedSeasons.length > 0 ? 'were deleted' : 'are missing'} from ${🤖 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 `@server/lib/availabilitySync.ts` around lines 616 - 632, The log currently states seasonKeys were "not found" but may include seasons that were skipped (PROCESSING/UNKNOWN); update the deletion loop above to collect only the actually deleted season keys into a new array (e.g., deletedSeasonKeys) when you set status to MediaStatus.DELETED, then use that deletedSeasonKeys array in the logger.debug message (instead of seasonKeys) so the message accurately reflects which seasons were removed; preserve existing context fields (media, is4k, media.mediaType, mediaServerType, MediaStatus) and handle the empty-case (no deletedSeasonKeys) appropriately in the log.server/lib/availabilitySync.test.ts (2)
125-138: 💤 Low valueTheMovieDb mock looks good; consider adding
getShowByTvdbIdfor completeness.The
getTvShowmock is correctly set up. However,availabilitySync.tsalso callsgetShowByTvdbIdas a fallback whentmdbIdis missing. Current tests all usetmdbId, so this isn't blocking, but mockinggetShowByTvdbIdwould provide fuller coverage.🔧 Optional: Add getShowByTvdbId mock
Object.defineProperty(TheMovieDb.prototype, 'getTvShow', { get() { return async (args: { tvId: number; language?: string }) => getTvShowImpl(args); }, set() {}, configurable: true, }); + +Object.defineProperty(TheMovieDb.prototype, 'getShowByTvdbId', { + get() { + return async (args: { tvdbId: number; language?: string }) => + getTvShowImpl({ tvId: args.tvdbId }); + }, + set() {}, + configurable: true, +});🤖 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 `@server/lib/availabilitySync.test.ts` around lines 125 - 138, Add a mock for TheMovieDb.prototype.getShowByTvdbId similar to the existing getTvShow mock: declare a getShowByTvdbIdImpl function (e.g., async (args: { tvdbId: number; language?: string }) => Promise<TmdbTvDetails>) and use Object.defineProperty on TheMovieDb.prototype with a getter that returns an async wrapper calling getShowByTvdbIdImpl and an empty setter; ensure the property is configurable so tests can override it and reuse fakeTmdbShow/fixtures as the return value.
775-873: 💤 Low valueTest correctly validates the PARTIALLY_AVAILABLE scenario.
The test properly exercises the case where some seasons are AVAILABLE and others are UNKNOWN, verifying the show becomes PARTIALLY_AVAILABLE. This directly tests the bug fix described in the PR objectives.
For completeness, consider adding assertions for individual season statuses:
🧪 Optional: Add season-level assertions
assert.strictEqual( updated.status, MediaStatus.PARTIALLY_AVAILABLE, 'Show should be PARTIALLY_AVAILABLE when some seasons are available and some are unknown' ); + + // Verify individual season statuses + const s1 = updated.seasons.find((s) => s.seasonNumber === 1); + const s2 = updated.seasons.find((s) => s.seasonNumber === 2); + const s3 = updated.seasons.find((s) => s.seasonNumber === 3); + const s4 = updated.seasons.find((s) => s.seasonNumber === 4); + assert.strictEqual(s1?.status, MediaStatus.AVAILABLE, 'Season 1 should remain AVAILABLE'); + assert.strictEqual(s2?.status, MediaStatus.AVAILABLE, 'Season 2 should remain AVAILABLE'); + assert.strictEqual(s3?.status, MediaStatus.UNKNOWN, 'Season 3 should remain UNKNOWN'); + assert.strictEqual(s4?.status, MediaStatus.UNKNOWN, 'Season 4 should remain UNKNOWN'); });🤖 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 `@server/lib/availabilitySync.test.ts` around lines 775 - 873, Add explicit season-level assertions to the "should mark show as PARTIALLY_AVAILABLE when some seasons are available and some are unknown" test: after fetching updated via mediaRepository.findOneOrFail (relations: ['seasons']) assert that updated.seasons contains entries for seasonNumber 1..4 and that seasons 1 and 2 have status === MediaStatus.AVAILABLE (and status4k === MediaStatus.UNKNOWN if desired) while seasons 3 and 4 have status === MediaStatus.UNKNOWN (and status4k === MediaStatus.UNKNOWN), referencing the test's media.seasons, updated.seasons, MediaStatus enum and the availabilitySync.run invocation to locate where to insert these checks.
🤖 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/lib/scanners/baseScanner.ts`:
- Around line 470-479: The current computed predicate
isAllStandardSeasonsAvailable uses .filter(...).every(...) which returns true on
an empty filtered list; change the logic to first build the filtered array
(e.g., const participatingSeasons = nonSpecialSeasons.filter(...)), then set
isAllStandardSeasonsAvailable = participatingSeasons.length > 0 &&
participatingSeasons.every(s => s.status === MediaStatus.AVAILABLE). Apply the
same pattern to the other similar predicates in this file (the next two/all
analogous checks around the blocks that compute availability/partialAvailability
using .filter(...).every(...)) so each requires at least one participating
season before .every() is relied on.
---
Nitpick comments:
In `@server/lib/availabilitySync.test.ts`:
- Around line 125-138: Add a mock for TheMovieDb.prototype.getShowByTvdbId
similar to the existing getTvShow mock: declare a getShowByTvdbIdImpl function
(e.g., async (args: { tvdbId: number; language?: string }) =>
Promise<TmdbTvDetails>) and use Object.defineProperty on TheMovieDb.prototype
with a getter that returns an async wrapper calling getShowByTvdbIdImpl and an
empty setter; ensure the property is configurable so tests can override it and
reuse fakeTmdbShow/fixtures as the return value.
- Around line 775-873: Add explicit season-level assertions to the "should mark
show as PARTIALLY_AVAILABLE when some seasons are available and some are
unknown" test: after fetching updated via mediaRepository.findOneOrFail
(relations: ['seasons']) assert that updated.seasons contains entries for
seasonNumber 1..4 and that seasons 1 and 2 have status === MediaStatus.AVAILABLE
(and status4k === MediaStatus.UNKNOWN if desired) while seasons 3 and 4 have
status === MediaStatus.UNKNOWN (and status4k === MediaStatus.UNKNOWN),
referencing the test's media.seasons, updated.seasons, MediaStatus enum and the
availabilitySync.run invocation to locate where to insert these checks.
In `@server/lib/availabilitySync.ts`:
- Around line 616-632: The log currently states seasonKeys were "not found" but
may include seasons that were skipped (PROCESSING/UNKNOWN); update the deletion
loop above to collect only the actually deleted season keys into a new array
(e.g., deletedSeasonKeys) when you set status to MediaStatus.DELETED, then use
that deletedSeasonKeys array in the logger.debug message (instead of seasonKeys)
so the message accurately reflects which seasons were removed; preserve existing
context fields (media, is4k, media.mediaType, mediaServerType, MediaStatus) and
handle the empty-case (no deletedSeasonKeys) appropriately in the log.
🪄 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: b6252f55-b49d-4d23-9b57-e40532ddd5a1
📒 Files selected for processing (8)
next-env.d.tsserver/lib/availabilitySync.test.tsserver/lib/availabilitySync.tsserver/lib/scanners/baseScanner.tsserver/lib/scanners/jellyfin/index.tsserver/lib/scanners/jellyfin/jellyfin.test.tsserver/lib/scanners/plex/index.tsserver/lib/scanners/sonarr/index.ts
9d75818 to
67edf3b
Compare
67edf3b to
e3cd34a
Compare
fallenbagel
left a comment
There was a problem hiding this comment.
LGTM. Tested on my prod and works as intended so far as well.
Description
This PR fixes the Availability scanner and the Jellyfin scanner updating series and season statuses.
Previously, a series with 1 available season and 1 unknown season was marked as "available", while it should have been marked as "partially available".
This fixes a regression introduced by #2958 that conflicted with #2412.
This also fixes some minor logging issues (
infoinstead ofdebugand duplicate logs).Tests written by Claude have been added to ensure this regression doesn't happen again for these 2 scanners.
How Has This Been Tested?
Tested with
(as well as the newly introduced tests)
Screenshots / Logs (if applicable)
N/A
Checklist:
pnpm buildpnpm i18n:extractSummary by CodeRabbit
New Features
Bug Fixes
Tests
Refactor