feat: add default watchlist sync toggles for newly-created users#3048
feat: add default watchlist sync toggles for newly-created users#3048csmarshall wants to merge 5 commits into
Conversation
Plex Watchlist Sync silently skips users where both watchlistSyncMovies and watchlistSyncTv are null (the default), even when the user has AUTO_REQUEST permission. This means every new user — whether from Plex OAuth signin or from the admin Import Users from Plex/Jellyfin button — inherits null/null settings and never has their watchlist synced unless an admin manually toggles their settings. Add two MainSettings fields, defaulting to false to preserve existing behavior: - defaultWatchlistSyncMovies - defaultWatchlistSyncTv Apply these to UserSettings on new-user creation in: - server/routes/auth.ts (Plex + Jellyfin OAuth signin paths) - server/routes/user/index.ts (import-from-plex + import-from-jellyfin) Skips the admin-bootstrap creation paths since the very first user is created with full ADMIN permissions and configures their own settings deliberately. OpenAPI schema updated. Frontend toggles in SettingsMain are not yet added — backend can land first and UI can be a follow-up.
Mirror the existing hideAvailable/hideBlocklisted pattern in SettingsMain to expose the two new MainSettings booleans (added in 8c17233) as labeled checkbox toggles, placed after the enableSpecialEpisodes toggle. Each toggle includes a label-tip explaining that the value is the default applied to newly-created users on creation, and that existing users are unaffected (they may still override their own setting on their user profile). i18n keys generated via pnpm i18n:extract; non-en locales are left for Weblate translators per CONTRIBUTING.md.
Add four tests in auth.test.ts (POST /auth/plex new-user creation) and three in user/index.test.ts (POST /api/v1/user/import-from-plex), each mocking PlexTvAPI prototype methods so they need no network or real Plex.tv account. Cover the false/true/mixed permutations of defaultWatchlistSyncMovies and defaultWatchlistSyncTv plus a regression test asserting that an existing user's settings are not overwritten by the MainSettings defaults on subsequent sign-ins.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughAdds two admin-configurable boolean defaults (movies/TV) to MainSettings, exposes checkboxes in the Settings UI, and applies those defaults when creating or importing users via Plex and Jellyfin. Tests verify persistence and that existing user settings are not overwritten. ChangesWatchlist sync defaults for new users
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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/routes/auth.ts`:
- Around line 184-192: Set user.settings to a new UserSettings (using
settings.main.defaultWatchlistSyncMovies and
settings.main.defaultWatchlistSyncTv) before calling userRepository.save so the
new user and their default watchlist flags are persisted in a single atomic
save; update both creation paths that currently call userRepository.save twice
(the block that constructs UserSettings and the other path that repeats this
logic) to assign user.settings first and perform a single save via
userRepository.save(user).
🪄 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: b8a24574-0301-47ce-a146-83883f382cf7
📒 Files selected for processing (8)
seerr-api.ymlserver/lib/settings/index.tsserver/routes/auth.test.tsserver/routes/auth.tsserver/routes/user/index.test.tsserver/routes/user/index.tssrc/components/Settings/SettingsMain/index.tsxsrc/i18n/locale/en.json
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| session({ | ||
| secret: 'test-secret', | ||
| resave: false, | ||
| saveUninitialized: false, | ||
| }) |
|
How does this close #2820?? Also enabling watchlist sync by default still wouldn't grab their watchlits since their plex token is only populated when they log in once... |
- Persist new-user settings atomically: set user.settings before the first userRepository.save() and let the cascade-true relation on User.settings handle the UserSettings persistence in one transaction. Previously two separate saves were issued, so a failure between them could leave a user without their default watchlist flags. Applies to all four user-creation sites: POST /auth/plex, POST /auth/jellyfin, POST /api/v1/user/import-from-plex, POST /api/v1/user/import-from-jellyfin. Reported by CodeRabbit on the PR. - Fix UserSettings import ordering in server/routes/user/index.ts so prettier --check . passes in CI. - Set cookie.secure to 'auto' in the express-session config used by the new user/index.test.ts to satisfy CodeQL's clear-text-cookie check without breaking the supertest-over-HTTP test harness. 40/40 tests still pass.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
server/routes/user/index.test.ts (1)
143-162: 💤 Low valueConsider adding test for the remaining permutation (movies=true, tv=false).
The three test cases cover false/false, true/true, and false/true. Adding a fourth test for movies=true with tv=false would provide complete permutation coverage and further demonstrate that the two settings are independently applied.
✅ Optional test case for completeness
+ it('propagates mixed defaultWatchlistSync flags (movies=true, tv=false)', async () => { + const settings = getSettings(); + settings.main.defaultWatchlistSyncMovies = true; + settings.main.defaultWatchlistSyncTv = false; + + const agent = await adminAgent(); + const res = await agent + .post('/api/v1/user/import-from-plex') + .send({ plexIds: [String(TARGET_PLEX_ID)] }); + + assert.strictEqual(res.status, 201); + + const created = await getRepository(User).findOneOrFail({ + where: { email: TARGET_EMAIL }, + relations: { settings: true }, + }); + assert.ok(created.settings); + assert.strictEqual(created.settings.watchlistSyncMovies, true); + assert.strictEqual(created.settings.watchlistSyncTv, false); + });🤖 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/routes/user/index.test.ts` around lines 143 - 162, Add a fourth unit test mirroring the existing "propagates mixed defaultWatchlistSync flags" test but with settings.main.defaultWatchlistSyncMovies = true and settings.main.defaultWatchlistSyncTv = false: call getSettings(), set those two flags, use adminAgent() to POST to '/api/v1/user/import-from-plex' with the same plexIds payload, assert 201 response, then load the created User via getRepository(User).findOneOrFail (relations: { settings: true }) and assert created.settings.watchlistSyncMovies === true and created.settings.watchlistSyncTv === false to verify independent propagation.
🤖 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/routes/user/index.test.ts`:
- Around line 143-162: Add a fourth unit test mirroring the existing "propagates
mixed defaultWatchlistSync flags" test but with
settings.main.defaultWatchlistSyncMovies = true and
settings.main.defaultWatchlistSyncTv = false: call getSettings(), set those two
flags, use adminAgent() to POST to '/api/v1/user/import-from-plex' with the same
plexIds payload, assert 201 response, then load the created User via
getRepository(User).findOneOrFail (relations: { settings: true }) and assert
created.settings.watchlistSyncMovies === true and
created.settings.watchlistSyncTv === false to verify independent propagation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0d184606-cd28-4372-b5ba-5c155ac8a031
📒 Files selected for processing (3)
server/routes/auth.tsserver/routes/user/index.test.tsserver/routes/user/index.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- server/routes/user/index.ts
- server/routes/auth.ts
Eh - it's designed to grab their watch lists on first log in when we do have their credential - so that if someone adds a user and they log in, their automatically going to find that things were requested for them that they had been wanting to watch - for stuff already in the library it's a no-op obviously - and it would populate a bunch of requests for approval if not already auto-approved. |
But that still doesnt close #2820 which was about a support ticket |
I specifically went and remove any reference to 2820 as this really is not a bug fix but a feature add. |
3 participants
Description
WatchlistSync.syncUserWatchlistreturns early for any user whosewatchlistSyncMoviesandwatchlistSyncTvare both null or false. New users created via Plex OAuth, Plex friend import, Jellyfin OAuth, and Jellyfin import all land with both flagsundefined, so they're silently skipped by server-level watchlist sync until someone toggles the setting per-user. For multi-user installs that's a footgun: an admin enables watchlist sync at the server level expecting it to apply to everyone, and it doesn't.This PR adds two new
MainSettingsbooleans —defaultWatchlistSyncMoviesanddefaultWatchlistSyncTv— exposed as toggles in Settings → General. Whatever's set there is applied toUserSettingsat every new-user creation site. Defaults arefalse, so behavior on upgrade is unchanged. Existing users are not modified by anything in this PR; only newly-created users get the admin defaults.Changes:
MainSettingsinterface + Settings constructor defaults (false)new UserSettings({...})at all four user-creation sites:POST /auth/plexPOST /auth/jellyfinPOST /api/v1/user/import-from-plexPOST /api/v1/user/import-from-jellyfinSettings → General Settings, following the existinghideAvailable/enableSpecialEpisodesform patternseerr-api.ymlen.json(non-en locales left for Weblate per CONTRIBUTING.md)How Has This Been Tested?
Automated:
pnpm test— 40/40 pass; 7 new in this PR (server/routes/auth.test.ts,server/routes/user/index.test.ts). Tests mockPlexTvAPI.prototype.{getUser, getUsers, checkUserAccess}so they run without a live Plex.tv account. Verified stable across 3 consecutive runs.pnpm build—build:nextandbuild:serverboth succeed.pnpm typecheck:server— clean.pnpm lint— clean for files in this PR; pre-existing warnings in untouched files only.pnpm i18n:extract— adds 4 keys toen.jsononly.Manual:
seerr/config/, startedpnpm dev, confirmed the auto-writtensettings.jsoncontains bothdefaultWatchlistSyncMoviesanddefaultWatchlistSyncTvdefaulted tofalseat the top ofmain.Screenshots / Logs (if applicable)
Checklist:
pnpm buildpnpm i18n:extractAI Disclosure
Claude Code (Anthropic Opus 4.7) drafted the backend changes, the UI toggles, the seven propagation tests, and an initial version of this description. I reviewed every diff, ran
pnpm build,pnpm test,pnpm typecheck:server, andpnpm lintlocally, and verified the toggles on apnpm devinstance. During review I caught that the original toggle labels and tips on Settings → General didn't mention "Plex Watchlist" anywhere — admins had no way to know what feature these toggles controlled. I had Claude revise the wording twice before it read clearly; the final labels are "Default: Auto-Request Movies/Series added to Plex Watchlist". I will respond to review feedback myself, without LLM assistance.Summary by CodeRabbit