feat(settings): add endpoint and UI for switching media server#2539
feat(settings): add endpoint and UI for switching media server#2539
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds a media-server switching feature: new POST API to switch servers, server-side state migrations (users, media, watchlists), updated auth/linking flows for Plex/Jellyfin/Emby, UI components for switching and linking, and a tightened Media entity update guard. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant UI as Settings UI
participant API as Settings API
participant DB as Database
participant BG as Background Jobs
User->>UI: Choose target server and confirm
UI->>API: POST /settings/switch-media-server { targetServerType }
API->>API: Validate current configuration & token(s)
API->>DB: Begin transaction / update settings
rect rgba(100,150,200,0.5)
API->>DB: Update User records (clear/set server fields)
API->>DB: Reset Media records (clear server-specific IDs)
API->>DB: Reset Watchlist entries
end
API->>API: Save settings, commit transaction
API->>BG: Restart scheduled background jobs
BG->>BG: Reinitialize jobs with new server config
API-->>UI: 200 Success
UI->>User: Show success toast and reload
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Don't mind the big refactor on this one, I had some UI issues that generated a lot of changes (it's mainly spaces)
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
server/routes/settings/index.ts (1)
175-219:⚠️ Potential issue | 🟡 MinorHandle missing Plex token explicitly.
If the admin token is absent (e.g., post-switch), this returns
undefinedwith 200. A clear 400 response is safer.💡 Suggested fix
- const authToken = admin.plexToken ?? null; - const plexTvClient = authToken ? new PlexTvAPI(authToken) : null; + const authToken = admin.plexToken ?? null; + if (!authToken) { + return next({ + status: 400, + message: 'Plex token missing. Sign in with Plex first.', + }); + } + const plexTvClient = new PlexTvAPI(authToken);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/routes/settings/index.ts` around lines 175 - 219, The route currently proceeds when the admin Plex token is missing and later returns undefined; before creating PlexTvAPI/calling plexTvClient.getDevices(), check admin.plexToken (authToken) and if null/undefined immediately return a 400 Bad Request response (with a clear message like "Plex token missing") instead of continuing; update the logic around getSettings / userRepository.findOneOrFail / PlexTvAPI / plexTvClient to short-circuit on missing authToken so no calls to plexTvClient.getDevices() or PlexAPI are attempted.
🧹 Nitpick comments (1)
seerr-api.yml (1)
2293-2320: Document the optional target server type in the request body.Clients won’t know how to request Emby vs. Jellyfin without a schema. Consider adding an optional
targetServerTypefield.📝 Suggested OpenAPI addition
/settings/switch-media-server: post: summary: Switch media server tags: - settings + requestBody: + required: false + content: + application/json: + schema: + type: object + properties: + targetServerType: + type: string + enum: [jellyfin, emby] + description: Target server type; defaults to jellyfin when omitted. responses: '200': description: Media server cleared🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@seerr-api.yml` around lines 2293 - 2320, The POST /settings/switch-media-server operation lacks a requestBody schema describing how to specify which server type to switch to; add a requestBody (content: application/json) with an optional property named targetServerType (string) and constrain it to allowed values (e.g., enum: ["emby","jellyfin"]) so clients know how to request Emby vs Jellyfin; update the operation under the path "/settings/switch-media-server" to include this requestBody and mark the property as optional and documented in the response examples.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@server/routes/auth.ts`:
- Around line 61-92: The current block lets any authenticated user overwrite the
admin Plex token; update the authorization to allow only admins to perform this
action by checking the authenticated user's admin role (e.g., req.user.isAdmin
or req.user.role === 'admin') before proceeding with PlexTvAPI validation and
saving to userRepository; if the user is not an admin, return a 403 error (do
not change the existing admin user retrieval logic that uses findOneOrFail for
id 1 and the PlexTvAPI validation flow).
In `@server/routes/settings/index.ts`:
- Around line 453-569: The /switch-media-server handler
(settingsRoutes.post('/switch-media-server', ...)) is unprotected; add an
admin-only guard at the top of the handler (or attach an existing admin
middleware) so only admins can call it: verify req.user exists and has admin
rights (e.g., check req.user.userType === UserType.ADMIN or an isAdmin flag on
the User entity) and return 403 if not authorized before any state changes
(before getSettings(), repository updates, settings.save(), startJobs(), etc.);
reference User and UserType used in the route for the check or reuse your app's
ensureAdmin middleware if available.
In `@server/routes/user/usersettings.ts`:
- Around line 419-429: The unlink (Jellyfin delete) route currently blocks
unlinking when Jellyfin/Emby isn't the main server; update its guard to mirror
the linking check by using the same logic as isMainJellyfin and
jellyfinForLinking: allow unlink if either settings.main.mediaServerType ===
MediaServerType.JELLYFIN/EMBY (isMainJellyfin) or settings.jellyfin?.ip is
present; replace the strict isMainJellyfin-only check in the delete handler with
the combined condition (!isMainJellyfin && !jellyfinForLinking?.ip) so users who
linked against a configured non-main Jellyfin can unlink.
In `@src/components/Settings/SettingsPlex.tsx`:
- Around line 792-801: The Formik form for Tautulli (the Formik component with
initialValues built from dataTautulli and using TautulliSettingsSchema) can
mount before dataTautulli is available and won't pick up updates; enable
reinitialization by adding enableReinitialize to the Formik props (or
conditionally render Formik only when dataTautulli is defined) so the form's
initialValues update when dataTautulli arrives and avoid blank
defaults/accidental overwrites.
In `@src/components/Settings/SwitchMediaServerSection.tsx`:
- Around line 107-113: In the catch block inside SwitchMediaServerSection (the
try/catch that builds the toast message), broaden the Axios error extraction to
check response.data.message as well as response.data.error and fallback to
err.message before using the generic intl.formatMessage; update the conditional
that sets message (currently using axios.isAxiosError(err) &&
err.response?.data?.error) to instead test axios.isAxiosError(err) and then
prefer String(err.response?.data?.error ?? err.response?.data?.message ??
err.message) so you capture endpoints that return message or a plain Error
message while preserving the existing generic fallback.
- Around line 95-106: The success toast message is hard-coded; replace it with a
localized string by calling the project's i18n translator instead of the literal
text. In the onClick handler inside SwitchMediaServerSection (where addToast is
called after axios.post), import or use the existing translation function (e.g.,
useTranslation's t or i18n.t) and pass a translation key like
t('settings.mediaServer.cleared') (with an appropriate entry added to your
locale files) to addToast instead of the hard-coded "Media server cleared. You
may need to restart."; keep the same appearance options and reload logic.
---
Outside diff comments:
In `@server/routes/settings/index.ts`:
- Around line 175-219: The route currently proceeds when the admin Plex token is
missing and later returns undefined; before creating PlexTvAPI/calling
plexTvClient.getDevices(), check admin.plexToken (authToken) and if
null/undefined immediately return a 400 Bad Request response (with a clear
message like "Plex token missing") instead of continuing; update the logic
around getSettings / userRepository.findOneOrFail / PlexTvAPI / plexTvClient to
short-circuit on missing authToken so no calls to plexTvClient.getDevices() or
PlexAPI are attempted.
---
Nitpick comments:
In `@seerr-api.yml`:
- Around line 2293-2320: The POST /settings/switch-media-server operation lacks
a requestBody schema describing how to specify which server type to switch to;
add a requestBody (content: application/json) with an optional property named
targetServerType (string) and constrain it to allowed values (e.g., enum:
["emby","jellyfin"]) so clients know how to request Emby vs Jellyfin; update the
operation under the path "/settings/switch-media-server" to include this
requestBody and mark the property as optional and documented in the response
examples.
0xSysR3ll
force-pushed
the
0xsysr3ll/feat/media-server-migration
branch
from
41a7f8b to
3786a27
Compare
github-actions
bot
added
the
i18n-out-of-sync
|
The i18n check failed because translation messages are out of sync. This usually happens when you've added or modified translation strings in your code but haven't updated the translation file. Please run |
github-actions
bot
removed
the
i18n-out-of-sync
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
server/subscriber/MediaSubscriber.ts (1)
155-158: Guard logic is correct; consider removing redundant optional chaining.The guard now ensures both
event.entityandevent.databaseEntityare defined. However, lines 178 and 191 still useevent.databaseEntity?.statusandevent.databaseEntity?.status4k— the optional chaining is now redundant and could be simplified for consistency.♻️ Optional cleanup
if ( - (event.entity.status !== event.databaseEntity?.status || + (event.entity.status !== event.databaseEntity.status || (event.entity.mediaType === MediaType.TV && seasonStatusCheck(false))) && validStatuses.includes(event.entity.status)if ( - (event.entity.status4k !== event.databaseEntity?.status4k || + (event.entity.status4k !== event.databaseEntity.status4k || (event.entity.mediaType === MediaType.TV && seasonStatusCheck(true))) && validStatuses.includes(event.entity.status4k)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/subscriber/MediaSubscriber.ts` around lines 155 - 158, The guard in MediaSubscriber.afterUpdate already ensures event.entity and event.databaseEntity are non-null, so remove the redundant optional chaining: replace occurrences of event.databaseEntity?.status and event.databaseEntity?.status4k with direct access event.databaseEntity.status and event.databaseEntity.status4k (e.g., in the conditional checks currently using ?. at lines referencing those properties) while keeping the existing early-return guard intact.server/routes/settings/index.ts (1)
479-492: Consider: User update queries always match all users.The WHERE clause
user.id >= :zerowithzero: 0will always match all users since IDs are positive integers. While this works correctly, it's semantically unclear.Consider simplifying to make the intent explicit:
♻️ Suggested clarification
await userRepository .createQueryBuilder() .update(User) .set({ plexId: null, plexUsername: null, plexToken: null }) - .where('user.id >= :zero', { zero: 0 }) + .execute(); // Updates all usersOr if you need a WHERE clause for safety:
- .where('user.id >= :zero', { zero: 0 }) + .where('user.id IS NOT NULL') // All users🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/routes/settings/index.ts` around lines 479 - 492, The first update query uses a redundant WHERE ('user.id >= :zero', { zero: 0 }) that effectively matches all users; remove that semantic noise by calling the same userRepository.createQueryBuilder().update(User).set({ plexId: null, plexUsername: null, plexToken: null }).execute() so the intent to update all rows is explicit and clear (keep the second query using .where('user.jellyfinUserId IS NOT NULL') as-is).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@server/routes/settings/index.ts`:
- Around line 500-506: The watchlist clearing is asymmetric and uses
inconsistent null handling: update the Watchlist clearing logic that uses
watchlistRepository.createQueryBuilder().update(Watchlist) so that
Watchlist.ratingKey is cleared whenever switching away from Plex (add the
corresponding branch or call in the switch-to-Plex/from-Plex flows), and make
the clearing consistent with other media fields by setting ratingKey to null
instead of ''. Replace .set({ ratingKey: '' }) with .set({ ratingKey: null })
and change the predicate to .where("watchlist.ratingKey IS NOT NULL") (or
equivalent ORM null check), and ensure the same null convention is applied
wherever media fields are cleared.
---
Nitpick comments:
In `@server/routes/settings/index.ts`:
- Around line 479-492: The first update query uses a redundant WHERE ('user.id
>= :zero', { zero: 0 }) that effectively matches all users; remove that semantic
noise by calling the same userRepository.createQueryBuilder().update(User).set({
plexId: null, plexUsername: null, plexToken: null }).execute() so the intent to
update all rows is explicit and clear (keep the second query using
.where('user.jellyfinUserId IS NOT NULL') as-is).
In `@server/subscriber/MediaSubscriber.ts`:
- Around line 155-158: The guard in MediaSubscriber.afterUpdate already ensures
event.entity and event.databaseEntity are non-null, so remove the redundant
optional chaining: replace occurrences of event.databaseEntity?.status and
event.databaseEntity?.status4k with direct access event.databaseEntity.status
and event.databaseEntity.status4k (e.g., in the conditional checks currently
using ?. at lines referencing those properties) while keeping the existing
early-return guard intact.
server/routes/settings/index.ts
Outdated
| const watchlistRepository = getRepository(Watchlist); | ||
| await watchlistRepository | ||
| .createQueryBuilder() | ||
| .update(Watchlist) | ||
| .set({ ratingKey: '' }) | ||
| .where("watchlist.ratingKey != ''") | ||
| .execute(); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's see the structure and locate the file
git ls-files | grep -E "(settings|index)" | head -20Repository: seerr-team/seerr
Length of output: 831
🏁 Script executed:
# Check if the file exists and get its size
wc -l server/routes/settings/index.tsRepository: seerr-team/seerr
Length of output: 95
🏁 Script executed:
# Read the relevant sections - lines around 500-506 and 549-557
sed -n '490,520p' server/routes/settings/index.tsRepository: seerr-team/seerr
Length of output: 1227
🏁 Script executed:
# Read the second section around lines 549-557
sed -n '540,570p' server/routes/settings/index.tsRepository: seerr-team/seerr
Length of output: 1112
🏁 Script executed:
# Search for ratingKey usage to understand empty string vs null handling
rg -n "ratingKey" --type ts -C 3 | head -120Repository: seerr-team/seerr
Length of output: 10528
Watchlist clearing is asymmetric between switch directions.
When switching from Plex to Jellyfin/Emby, Watchlist.ratingKey is cleared with an empty string. However, when switching from Jellyfin/Emby to Plex, the watchlist is not cleared at all. Since ratingKey is a Plex-specific identifier, it should be cleared when switching away from Plex to maintain consistency.
Additionally, the code uses inconsistent null handling: Media fields are set to null, but Watchlist.ratingKey is set to an empty string ''. This should be aligned for consistency—either both use null or both use ''.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/routes/settings/index.ts` around lines 500 - 506, The watchlist
clearing is asymmetric and uses inconsistent null handling: update the Watchlist
clearing logic that uses
watchlistRepository.createQueryBuilder().update(Watchlist) so that
Watchlist.ratingKey is cleared whenever switching away from Plex (add the
corresponding branch or call in the switch-to-Plex/from-Plex flows), and make
the clearing consistent with other media fields by setting ratingKey to null
instead of ''. Replace .set({ ratingKey: '' }) with .set({ ratingKey: null })
and change the predicate to .where("watchlist.ratingKey IS NOT NULL") (or
equivalent ORM null check), and ensure the same null convention is applied
wherever media fields are cleared.
There was a problem hiding this comment.
Not true. The DB has ratingKey as NOT NULL.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check the Watchlist entity definition to verify the ratingKey column schema
fd -e ts Watchlist.ts --exec cat {}
Length of output: 4203
@0xSysR3ll, you're absolutely right—I apologize for the incorrect review. The Watchlist.ratingKey column is defined as NOT NULL in the entity (no nullable: true in the TypeORM column definition), so using an empty string '' is the correct approach. Setting it to null would cause a database constraint violation.
The code is handling this correctly: Media.ratingKey and Media.ratingKey4k are nullable fields (set to null), while Watchlist.ratingKey is a NOT NULL field (set to ''). This isn't inconsistent—it's the proper way to handle different column constraints.
🧬 Code Graph Analysis Results
: server/routes/settings/index.ts
[Explanation of relevant context for user comment]
- Summary of handling related to ratingKey NOT NULL:
- In the "Switch media server" flow when current server is Plex and switching away (to Emby/Jellyfin), the code updates several database fields to clear references to Plex-specific keys, including:
- Media: sets ratingKey and ratingKey4k to null where they are not already null.
- Watchlist: sets ratingKey to an empty string where it is not already ''.
- These operations are performed via repository update queries before saving settings and restarting jobs.
- In the "Switch media server" flow when current server is Plex and switching away (to Emby/Jellyfin), the code updates several database fields to clear references to Plex-specific keys, including:
[Snippet summary (describes the implemented behavior, no external code pasted)]
- Location: server/routes/settings/index.ts
- Context: Inside the branch handling switching away from Plex in the /switch-media-server route.
- Behavior:
- mediaRepository.update(Media).set({ ratingKey: null, ratingKey4k: null }).where('media.ratingKey IS NOT NULL OR media.ratingKey4k IS NOT NULL').execute();
- watchlistRepository.update(Watchlist).set({ ratingKey: '' }).where("watchlist.ratingKey != ''").execute();
- Effect: Clears Plex-related rating keys from Media and Watchlist records as part of switching to a non-Plex server.
[Note]
- If you need the exact line ranges, I can extract and present the precise lines from the file.
✏️ Learnings added
Learnt from: 0xSysR3ll
Repo: seerr-team/seerr PR: 2539
File: server/routes/settings/index.ts:500-506
Timestamp: 2026-02-22T17:54:54.195Z
Learning: In the Watchlist entity (server/entity/Watchlist.ts), the `ratingKey` column is defined as NOT NULL (`Column({ type: 'varchar' })` with default value `''`). When clearing this field in database operations, use an empty string `''` instead of `null` to avoid constraint violations.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
0xSysR3ll
force-pushed
the
0xsysr3ll/feat/media-server-migration
branch
from
20cc802 to
1a904f8
Compare
… with existing users
… Emby as main server
0xSysR3ll
force-pushed
the
0xsysr3ll/feat/media-server-migration
branch
from
99ba4fb to
ce959d6
Compare
1 participant
Description
Caution
Work in progress
This feature is still experimental. Do not use it on a production instance unless you have a verified backup and a tested rollback plan.
See Backups.
This PR adds an in‑app way to switch a single Seerr instance between media servers (Plex <=> Jellyfin/Emby) without losing users, permissions, requests, or general settings.
The same database and application instance are reused; only the active media server and authentication method change.
The goal is to make media‑server migrations practical without needing a second Seerr instance or manual database edits. Admins configure the new server, users link their new accounts, and then the instance is switched in place.
Note
This will initially ship as a preview so the flow can be tested and refined properly.
Migration flow (preview)
Back up first
Configure your target media server
Have users link their accounts
Switch media server in settings
Restart and sign in again
Verify your data
How Has This Been Tested?
=> And vice-versa from Jellyfin to Plex.
Screenshots / Logs (if applicable)
Checklist:
pnpm buildpnpm i18n:extractSummary by CodeRabbit
New Features
Improvements
Localization