Skip to content

Conversation

@fallenbagel
Copy link
Collaborator

Description

This PR refactors the Jellyfin scanner to extend the BaseScanner class, aligning its architecture with the Plex scanner implementation.

This refactor should reduce the code duplication between Plex and Jellyfin scanner implementations and make our codebase more maintainable.

This should also make it easier to implement #2066 as then it would not require any changes to that PR and can be merged after this.

How Has This Been Tested?

  1. Ran full Jellyfin library scan with mixed movie/TV show library and verified all media items were correctly identified and processed.

  2. Ran a Recently Added Scan and tested recently added items and verified incremental updates worked correctly

Screenshots / Logs (if applicable)

Checklist:

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

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors the Jellyfin scanner to extend the BaseScanner class, aligning its architecture with the Plex scanner implementation and reducing code duplication between the two scanner implementations.

Key changes:

  • Refactored JellyfinScanner to extend BaseScanner and utilize its shared media processing logic
  • Extracted movie ID resolution logic into a separate extractMovieIds method
  • Added Jellyfin-specific media ID tracking to BaseScanner for proper Jellyfin media server integration

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
server/lib/scanners/jellyfin/index.ts Refactored to extend BaseScanner, moved duplicate logic to base class, renamed methods for clarity, and delegated media processing to base scanner methods
server/lib/scanners/baseScanner.ts Added support for Jellyfin media IDs in both movie and show processing methods, made tvdbId optional for broader compatibility

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Refactors JellyfinScanner to extend BaseScanner class to align the jellyfin scanner architecture
with the plex scanner and reduce code duplication.
@fallenbagel fallenbagel force-pushed the fallenbagel/refactor/jellyfin-scanner-inheritence branch from c7c86fe to f3786ce Compare December 13, 2025 03:39
@fallenbagel fallenbagel requested a review from Copilot December 13, 2025 03:39

This comment was marked as outdated.

This comment was marked as outdated.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Member

@gauthier-th gauthier-th left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you take #2092 into account? Or should we add the same logic in another PR after?

@fallenbagel
Copy link
Collaborator Author

fallenbagel commented Dec 16, 2025

Did you take #2092 into account? Or should we add the same logic in another PR after?

This PR focuses on refactoring. Once it’s merged, #2092 should be rebased

Copy link
Member

@gauthier-th gauthier-th left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Not tested though.

Copy link
Member

@M0NsTeRRR M0NsTeRRR left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as Gauthier :)

@fallenbagel
Copy link
Collaborator Author

I'm merging this. So far from my testings, it seems to work as intended

@fallenbagel fallenbagel merged commit 57d583e into develop Dec 29, 2025
8 checks passed
@fallenbagel fallenbagel deleted the fallenbagel/refactor/jellyfin-scanner-inheritence branch December 29, 2025 12:05
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.

4 participants