Skip to content

feat(settings): Extract settings interfaces#3216

Open
Schrottfresser wants to merge 3 commits into
seerr-team:developfrom
Schrottfresser:extract-settings-interfaces
Open

feat(settings): Extract settings interfaces#3216
Schrottfresser wants to merge 3 commits into
seerr-team:developfrom
Schrottfresser:extract-settings-interfaces

Conversation

@Schrottfresser

@Schrottfresser Schrottfresser commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Description

This PR extracts all the settings interfaces currently maintained in server/lib/settings/index.ts to server/interfaces/settings.ts. This increases maintainability and makes importing a settings interface in the client possible because the old settings file includes node only imports like fs/promises.

This also is a prerequisite and result of splitting #1608 (see #1608 (comment)).

How Has This Been Tested?

As this only includes structural changes and does not touch any logic only sporadic testing was done:

  • building the project works without any issues
  • starting the app works without any issues
  • clicking around in the app and opening up multiple menus does not lead to errors

Screenshots / Logs (if applicable)

Nothing changed for a user. The app still looks exactly the same and doesn't log anything else then before.

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)

Summary by CodeRabbit

  • Refactor
    • Consolidated server and UI settings type definitions into a shared structure.
    • Updated settings-related screens and service integrations to consistently use the same settings types throughout the app.
    • Adjusted how settings types are referenced to improve consistency for notifications, libraries, scanning, and configuration.
  • Tests
    • Updated test type references to align with the shared settings definitions.
  • Bug Fixes
    • No user-facing behavior changes were introduced; this update focuses on type organization and maintainability.

@Schrottfresser Schrottfresser requested a review from a team as a code owner July 1, 2026 22:40
@coderabbitai

coderabbitai Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 2d1b71c8-52f2-4d45-b8ef-887e78a6dc0e

📥 Commits

Reviewing files that changed from the base of the PR and between 99b525f and cb760fc.

📒 Files selected for processing (1)
  • server/interfaces/settings.ts
💤 Files with no reviewable changes (1)
  • server/interfaces/settings.ts

📝 Walkthrough

Walkthrough

This PR moves settings-related TypeScript types into server/interfaces/settings.ts and updates server/client imports to use that module. Runtime logic and exported behavior remain unchanged.

Changes

Settings type module extraction and import migration

Layer / File(s) Summary
New settings interfaces module
server/interfaces/settings.ts
Defines the shared settings types, enums, and aggregates used across server and client code.
Trimmed settings lib index
server/lib/settings/index.ts
Keeps runtime settings-path logic and updates imports to use the new interfaces module.
Core server API/entity/util imports
server/api/metadata.ts, server/api/plexapi.ts, server/api/servarr/base.ts, server/api/tautulli.ts, server/entity/UserSettings.ts, server/interfaces/api/plexInterfaces.ts, server/interfaces/api/userSettingsInterfaces.ts, server/job/schedule.ts, server/utils/restartFlag.ts, server/utils/customProxyAgent.ts, server/utils/customProxyAgent.test.ts
Switches settings type imports to the new interfaces module.
Notification agent imports
server/lib/email/index.ts, server/lib/notifications/agents/*
Updates notification agent type imports to use server/interfaces/settings.ts.
Scanner modules and tests
server/lib/scanners/{jellyfin,plex,radarr,sonarr}/*, server/lib/availabilitySync.ts, server/lib/availabilitySync.test.ts
Switches scanner-related settings type imports to the new interfaces module and reorders related imports.
Settings migration modules
server/lib/settings/migrations/000*.ts, server/lib/settings/migrator.ts
Updates migration type imports to source AllSettings from the new module.
Settings routes imports
server/routes/settings/{index,metadata,radarr,sonarr}.ts
Updates route modules to import settings types from the new interfaces module.
Client component and hook imports
src/components/..., src/hooks/useUser.ts
Updates frontend settings-type imports to use the new interfaces module.

Estimated code review effort: 2 (Simple) | ~15 minutes

Suggested reviewers: gauthier-th, fallenbagel, M0NsTeRRR

Poem

I hopped through types from lib to nest,
and tucked them in a cleaner chest.
Same logic now, just imports bright,
a tidy burrow feels just right.
🐰

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: extracting settings interfaces into a dedicated module.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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/interfaces/settings.ts`:
- Around line 108-117: Remove the duplicate ProxySettings declaration in
settings.ts so only one interface remains; keep the canonical ProxySettings
definition and delete the repeated identical copy to avoid dead duplicate code
and unintended interface merging. Use the ProxySettings symbol in the settings
interface block to locate both declarations and ensure any references still
point to the single remaining interface.
🪄 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: 44a0478a-5b94-4117-9019-f422d2800e32

📥 Commits

Reviewing files that changed from the base of the PR and between ebac489 and 99b525f.

📒 Files selected for processing (65)
  • server/api/metadata.ts
  • server/api/plexapi.ts
  • server/api/servarr/base.ts
  • server/api/tautulli.ts
  • server/entity/UserSettings.ts
  • server/interfaces/api/plexInterfaces.ts
  • server/interfaces/api/userSettingsInterfaces.ts
  • server/interfaces/settings.ts
  • server/job/schedule.ts
  • server/lib/availabilitySync.test.ts
  • server/lib/availabilitySync.ts
  • server/lib/email/index.ts
  • server/lib/notifications/agents/agent.ts
  • server/lib/notifications/agents/discord.ts
  • server/lib/notifications/agents/email.ts
  • server/lib/notifications/agents/gotify.ts
  • server/lib/notifications/agents/ntfy.ts
  • server/lib/notifications/agents/pushbullet.ts
  • server/lib/notifications/agents/pushover.ts
  • server/lib/notifications/agents/slack.ts
  • server/lib/notifications/agents/telegram.ts
  • server/lib/notifications/agents/webhook.ts
  • server/lib/notifications/agents/webpush.ts
  • server/lib/scanners/jellyfin/index.ts
  • server/lib/scanners/jellyfin/jellyfin.test.ts
  • server/lib/scanners/plex/index.ts
  • server/lib/scanners/radarr/index.ts
  • server/lib/scanners/radarr/radarr.test.ts
  • server/lib/scanners/sonarr/index.ts
  • server/lib/scanners/sonarr/sonarr.test.ts
  • server/lib/settings/index.ts
  • server/lib/settings/migrations/0001_migrate_hostname.ts
  • server/lib/settings/migrations/0002_migrate_apitokens.ts
  • server/lib/settings/migrations/0003_emby_media_server_type.ts
  • server/lib/settings/migrations/0004_migrate_region_setting.ts
  • server/lib/settings/migrations/0005_migrate_network_settings.ts
  • server/lib/settings/migrations/0006_remove_lunasea.ts
  • server/lib/settings/migrations/0007_migrate_arr_tags.ts
  • server/lib/settings/migrations/0008_migrate_blacklist_to_blocklist.ts
  • server/lib/settings/migrator.ts
  • server/routes/settings/index.ts
  • server/routes/settings/metadata.ts
  • server/routes/settings/radarr.ts
  • server/routes/settings/sonarr.ts
  • server/utils/customProxyAgent.test.ts
  • server/utils/customProxyAgent.ts
  • server/utils/restartFlag.ts
  • src/components/LanguageSelector/index.tsx
  • src/components/ManageSlideOver/index.tsx
  • src/components/RegionSelector/index.tsx
  • src/components/Selector/CertificationSelector.tsx
  • src/components/Settings/Notifications/NotificationsNtfy/index.tsx
  • src/components/Settings/OverrideRule/OverrideRuleModal.tsx
  • src/components/Settings/OverrideRule/OverrideRuleTiles.tsx
  • src/components/Settings/RadarrModal/index.tsx
  • src/components/Settings/SettingsJellyfin.tsx
  • src/components/Settings/SettingsJobsCache/index.tsx
  • src/components/Settings/SettingsMain/index.tsx
  • src/components/Settings/SettingsNetwork/index.tsx
  • src/components/Settings/SettingsPlex.tsx
  • src/components/Settings/SettingsServices.tsx
  • src/components/Settings/SettingsUsers/index.tsx
  • src/components/Settings/SonarrModal/index.tsx
  • src/components/Setup/index.tsx
  • src/hooks/useUser.ts

Comment thread server/interfaces/settings.ts

@gauthier-th gauthier-th left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This increases maintainability and makes importing a settings interface in the client possible because the old settings file includes node only imports like fs/promises.

I'm not sure to understand that. We were already importing types from server/lib/settings and it's fine?

@github-actions github-actions Bot added the merge conflict Cannot merge due to merge conflicts label Jul 3, 2026
@github-actions

github-actions Bot commented Jul 3, 2026

Copy link
Copy Markdown

This pull request has merge conflicts. Please resolve the conflicts so the PR can be successfully reviewed and merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge conflict Cannot merge due to merge conflicts

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants