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)
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.
41a7f8b to
3786a27
Compare
|
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 |
|
@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.
20cc802 to
1a904f8
Compare
… with existing users
… Emby as main server
99ba4fb to
ce959d6
Compare
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