Skip to content

refactor: extract hardcoded route paths into shared constants#2762

Open
Logvin wants to merge 1 commit intoseerr-team:developfrom
Logvin:refactor/extract-route-constants
Open

refactor: extract hardcoded route paths into shared constants#2762
Logvin wants to merge 1 commit intoseerr-team:developfrom
Logvin:refactor/extract-route-constants

Conversation

@Logvin
Copy link
Copy Markdown

@Logvin Logvin commented Mar 26, 2026

Description

Replace all hardcoded API and proxy path strings (/api/v1, /api-docs, /imageproxy, /avatarproxy) with centralized constants and a helper function. No behavior changes, pure mechanical refactor.

Why

There are 201 occurrences of /api/v1 scattered across 73 client files, plus hardcoded mounts in the server. That's 201 places where a single path is repeated as a raw string literal. If any of these paths ever need to change, every occurrence has to be found and updated manually. That's error-prone and hard to verify.

Extracting these into shared constants is a well-established refactoring pattern:

  • The DRY principle from The Pragmatic Programmer (Tip build(deps): bump react-select from 4.3.1 to 5.2.2 #15) - "Every piece of knowledge must have a single, unambiguous, authoritative representation within a system." When a path is duplicated 201 times, changing it means hunting down every copy and hoping you didn't miss one.
  • Martin Fowler's "Replace Magic Literal" (Refactoring, 2nd Ed.) - replace repeated literals with a single named constant
  • Robert C. Martin's Clean Code (Ch. 17, G25) - raw literals should be hidden behind well-named constants

After this change, /api/v1 is defined in exactly 2 places (one server constant, one client constant). Everything else references them. This makes the codebase easier to maintain and opens the door for things like API versioning, base path configuration, or reverse proxy flexibility without having to touch every file again.

What changed

  • New file: server/constants/routes.ts - exports API_BASE_PATH, API_DOCS_PATH, IMAGE_PROXY_PATH, AVATAR_PROXY_PATH
  • New file: src/utils/apiUrl.ts - helper function that prepends the API base to a path
  • server/index.ts - route mounts now reference constants
  • server/routes/index.ts - deprecated route paths reference constants
  • src/components/Common/CachedImage/index.tsx - image proxy paths reference constants
  • 135 client files - all /api/v1 string literals replaced with apiUrl() calls

How Has This Been Tested?

  • TypeScript compiles with zero errors
  • ESLint + Prettier pass on all 140 files (via pre-commit hooks)
  • Ran an independent full-codebase review checking for broken references, missing imports, and malformed template literals. All clean
  • Running live on a personal server with existing Overseerr data migrated to Seerr. Login, search, requests, Plex/Sonarr/Radarr/Tautulli integration all working
  • pnpm build succeeds
  • pnpm i18n:extract produced no changes (expected, no user-facing text was modified)

Screenshots / Logs (if applicable)

N/A. No visual changes, this is a pure code refactor.

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)

AI Disclosure: This PR was developed with Claude Code as a pair programming tool. I directed the approach, defined the constants/helper pattern, reviewed all 140 file changes, and ran an independent review pass to catch issues. I'm the author, Claude is my editor.

Summary by CodeRabbit

  • Refactor
    • Centralized API route path definitions to improve maintainability and simplify configuration of API endpoints throughout the application.

Replace all hardcoded API and proxy path strings with centralized
constants and a helper function. There are no behavior changes — this
is a pure refactor to improve maintainability.

Paths like /api/v1, /api-docs, /imageproxy, and /avatarproxy are now
defined once and referenced everywhere. This makes the codebase easier
to update if these paths ever need to change for things like API
versioning, base path configuration, or reverse proxy flexibility.
@Logvin Logvin requested a review from a team as a code owner March 26, 2026 15:32
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 26, 2026

📝 Walkthrough

Walkthrough

This PR refactors API endpoint management by creating centralized route constants and a URL builder utility, then systematically replacing 150+ hardcoded /api/v1 paths with the new apiUrl() helper across client components and server routing logic.

Changes

Cohort / File(s) Summary
Route Constants & Utilities
server/constants/routes.ts, src/utils/apiUrl.ts
New files defining API route constants (API_BASE_PATH, API_DOCS_PATH, IMAGE_PROXY_PATH, AVATAR_PROXY_PATH) and a URL builder utility function that centralizes base path composition.
Server Routing
server/index.ts, server/routes/index.ts
Updated Express route mounting to use imported route constants instead of hardcoded path strings; adjusted deprecation middleware configuration to use template literals with API_BASE_PATH.
Client Components (SWR & Axios)
src/components/Blocklist/..., src/components/Discover/..., src/components/Issue.../..., src/components/Movie.../..., src/components/Request.../..., src/components/Settings/..., src/components/Tv.../..., src/components/User.../..., etc. (150+ files)
Systematically replaced hardcoded /api/v1/... endpoint strings with apiUrl(...) utility calls for all SWR fetches, Axios requests, and cache mutation keys. No functional logic changes beyond URL construction.
Image Proxy Integration
src/components/Common/CachedImage/index.tsx
Updated cached image proxy URL rewriting to use the IMAGE_PROXY_PATH constant instead of hardcoded /imageproxy prefix.
Hooks & Context
src/hooks/useRequestOverride.ts, src/hooks/useUser.ts, src/context/SettingsContext.tsx, src/utils/pushSubscriptionHelpers.ts, src/pages/_app.tsx
Updated shared hooks and context providers to construct SWR keys via apiUrl() for consistency with component-level changes.
Page-level Data Fetching
src/pages/collection/..., src/pages/movie/..., src/pages/tv/...
Updated server-side getServerSideProps axios calls to build API paths using the apiUrl utility instead of hardcoded /api/v1 prefixes.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • seerr-team/seerr#2531: Modifies imageproxy route handling, directly related to the IMAGE_PROXY_PATH constant and usage introduced in this PR.

Suggested reviewers

  • gauthier-th
  • 0xSysR3ll
  • fallenbagel

Poem

🐰 Whiskers twitch as paths align,
From scattered strings to constants fine,
One helper builds them all so neat,
A hundred URLs now complete! 🌿✨

🚥 Pre-merge checks | ✅ 2
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title clearly and specifically summarizes the main change: extracting hardcoded route paths into shared constants, which is the primary objective of this refactor.

✏️ 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.

❤️ Share

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

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (1)
src/utils/apiUrl.ts (1)

1-5: Consider exporting API_BASE and adding path validation.

The utility is clean and focused. Two optional improvements:

  1. Export API_BASE: Some call sites may need just the base path (e.g., for conditional logic or documentation). The server-side equivalent in server/constants/routes.ts exports API_BASE_PATH directly.

  2. Path validation: The function assumes path starts with /. Paths without a leading slash would produce malformed URLs (e.g., apiUrl('discover/tv')/api/v1discover/tv).

🔧 Optional: Export constant and add defensive check
-const API_BASE = '/api/v1';
+export const API_BASE = '/api/v1';

 export function apiUrl(path: string): string {
+  if (path && !path.startsWith('/')) {
+    path = '/' + path;
+  }
   return `${API_BASE}${path}`;
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/utils/apiUrl.ts` around lines 1 - 5, Export the API_BASE constant and
make apiUrl robust to missing leading slashes: export API_BASE (previously const
API_BASE) so other modules can import it, and update the apiUrl(path: string)
function to validate the path (e.g., if path is empty return API_BASE, and if
path does not start with '/' prepend '/' before concatenation) to avoid
producing malformed URLs when callers pass 'discover/tv' instead of
'/discover/tv'.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/components/UserList/JellyfinImportModal.tsx`:
- Line 57: JellyfinImportModal is interpolating `children` (a React.ReactNode)
into the query string for apiUrl(`/user?take=${children}`), which can be
non-numeric; replace that with a validated numeric value (e.g., use a prop/state
like `take`, `pageSize` or `takeCount` or compute `const take =
Number(props.take ?? DEFAULT_TAKE)`), validate/fallback if NaN, and then call
`apiUrl(`/user?take=${take}`)`; reference `apiUrl`, `JellyfinImportModal`, and
`children` in your change so you replace the `children` usage with the numeric
`take` variable.

In
`@src/components/UserProfile/UserSettings/UserNotificationSettings/UserNotificationsWebPush/index.tsx`:
- Line 78: The SWR key is being constructed unconditionally with
apiUrl(`/user/${user?.id}/pushSubscriptions`) which yields /user/undefined/...
on initial render; update the SWR call in the UserNotificationsWebPush component
to only pass a concrete URL when user is defined (e.g., use user ?
apiUrl(`/user/${user.id}/pushSubscriptions`) : null) so the fetch is skipped
until user is loaded, mirroring the guarded pattern used at the earlier SWR
call.

---

Nitpick comments:
In `@src/utils/apiUrl.ts`:
- Around line 1-5: Export the API_BASE constant and make apiUrl robust to
missing leading slashes: export API_BASE (previously const API_BASE) so other
modules can import it, and update the apiUrl(path: string) function to validate
the path (e.g., if path is empty return API_BASE, and if path does not start
with '/' prepend '/' before concatenation) to avoid producing malformed URLs
when callers pass 'discover/tv' instead of '/discover/tv'.
🪄 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: 8c03cdca-bdbb-4e4c-8512-f14374f49ecf

📥 Commits

Reviewing files that changed from the base of the PR and between 5373da4 and 1d381a3.

📒 Files selected for processing (140)
  • server/constants/routes.ts
  • server/index.ts
  • server/routes/index.ts
  • src/components/AppDataWarning/index.tsx
  • src/components/Blocklist/index.tsx
  • src/components/BlocklistBlock/index.tsx
  • src/components/BlocklistModal/index.tsx
  • src/components/BlocklistedTagsBadge/index.tsx
  • src/components/BlocklistedTagsSelector/index.tsx
  • src/components/CollectionDetails/index.tsx
  • src/components/Common/CachedImage/index.tsx
  • src/components/CompanyTag/index.tsx
  • src/components/Discover/CreateSlider/index.tsx
  • src/components/Discover/DiscoverMovieGenre/index.tsx
  • src/components/Discover/DiscoverMovieKeyword/index.tsx
  • src/components/Discover/DiscoverMovieLanguage/index.tsx
  • src/components/Discover/DiscoverMovies/index.tsx
  • src/components/Discover/DiscoverNetwork/index.tsx
  • src/components/Discover/DiscoverSliderEdit/index.tsx
  • src/components/Discover/DiscoverStudio/index.tsx
  • src/components/Discover/DiscoverTv/index.tsx
  • src/components/Discover/DiscoverTvGenre/index.tsx
  • src/components/Discover/DiscoverTvKeyword/index.tsx
  • src/components/Discover/DiscoverTvLanguage/index.tsx
  • src/components/Discover/DiscoverTvUpcoming.tsx
  • src/components/Discover/DiscoverWatchlist/index.tsx
  • src/components/Discover/MovieGenreList/index.tsx
  • src/components/Discover/MovieGenreSlider/index.tsx
  • src/components/Discover/PlexWatchlistSlider/index.tsx
  • src/components/Discover/RecentRequestsSlider/index.tsx
  • src/components/Discover/RecentlyAddedSlider/index.tsx
  • src/components/Discover/Trending.tsx
  • src/components/Discover/TvGenreList/index.tsx
  • src/components/Discover/TvGenreSlider/index.tsx
  • src/components/Discover/Upcoming.tsx
  • src/components/Discover/index.tsx
  • src/components/GenreTag/index.tsx
  • src/components/IssueDetails/IssueComment/index.tsx
  • src/components/IssueDetails/index.tsx
  • src/components/IssueList/IssueItem/index.tsx
  • src/components/IssueList/index.tsx
  • src/components/IssueModal/CreateIssueModal/index.tsx
  • src/components/KeywordTag/index.tsx
  • src/components/LanguageSelector/index.tsx
  • src/components/Layout/UserDropdown/MiniQuotaDisplay/index.tsx
  • src/components/Layout/UserDropdown/index.tsx
  • src/components/Layout/VersionStatus/index.tsx
  • src/components/Layout/index.tsx
  • src/components/Login/AddEmailModal.tsx
  • src/components/Login/JellyfinLogin.tsx
  • src/components/Login/LocalLogin.tsx
  • src/components/Login/index.tsx
  • src/components/ManageSlideOver/index.tsx
  • src/components/MovieDetails/MovieCast/index.tsx
  • src/components/MovieDetails/MovieCrew/index.tsx
  • src/components/MovieDetails/MovieRecommendations.tsx
  • src/components/MovieDetails/MovieSimilar.tsx
  • src/components/MovieDetails/index.tsx
  • src/components/PersonDetails/index.tsx
  • src/components/RegionSelector/index.tsx
  • src/components/RequestBlock/index.tsx
  • src/components/RequestButton/index.tsx
  • src/components/RequestCard/index.tsx
  • src/components/RequestList/RequestItem/index.tsx
  • src/components/RequestList/index.tsx
  • src/components/RequestModal/AdvancedRequester/index.tsx
  • src/components/RequestModal/CollectionRequestModal.tsx
  • src/components/RequestModal/MovieRequestModal.tsx
  • src/components/RequestModal/SearchByNameModal/index.tsx
  • src/components/RequestModal/TvRequestModal.tsx
  • src/components/ResetPassword/RequestResetLink.tsx
  • src/components/ResetPassword/index.tsx
  • src/components/Search/index.tsx
  • src/components/Selector/CertificationSelector.tsx
  • src/components/Selector/index.tsx
  • src/components/Settings/Notifications/NotificationsDiscord.tsx
  • src/components/Settings/Notifications/NotificationsEmail.tsx
  • src/components/Settings/Notifications/NotificationsGotify/index.tsx
  • src/components/Settings/Notifications/NotificationsNtfy/index.tsx
  • src/components/Settings/Notifications/NotificationsPushbullet/index.tsx
  • src/components/Settings/Notifications/NotificationsPushover/index.tsx
  • src/components/Settings/Notifications/NotificationsSlack/index.tsx
  • src/components/Settings/Notifications/NotificationsTelegram.tsx
  • src/components/Settings/Notifications/NotificationsWebPush/index.tsx
  • src/components/Settings/Notifications/NotificationsWebhook/index.tsx
  • src/components/Settings/OverrideRule/OverrideRuleModal.tsx
  • src/components/Settings/OverrideRule/OverrideRuleTiles.tsx
  • src/components/Settings/RadarrModal/index.tsx
  • src/components/Settings/SettingsAbout/index.tsx
  • src/components/Settings/SettingsJellyfin.tsx
  • src/components/Settings/SettingsJobsCache/index.tsx
  • src/components/Settings/SettingsLogs/index.tsx
  • src/components/Settings/SettingsMain/index.tsx
  • src/components/Settings/SettingsMetadata.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/JellyfinSetup.tsx
  • src/components/Setup/LoginWithPlex.tsx
  • src/components/Setup/SetupLogin.tsx
  • src/components/Setup/index.tsx
  • src/components/StatusChecker/index.tsx
  • src/components/TitleCard/ErrorCard.tsx
  • src/components/TitleCard/TmdbTitleCard.tsx
  • src/components/TitleCard/index.tsx
  • src/components/TvDetails/Season/index.tsx
  • src/components/TvDetails/TvCast/index.tsx
  • src/components/TvDetails/TvCrew/index.tsx
  • src/components/TvDetails/TvRecommendations.tsx
  • src/components/TvDetails/TvSimilar.tsx
  • src/components/TvDetails/index.tsx
  • src/components/UserList/BulkEditModal.tsx
  • src/components/UserList/JellyfinImportModal.tsx
  • src/components/UserList/PlexImportModal.tsx
  • src/components/UserList/index.tsx
  • src/components/UserProfile/UserSettings/UserGeneralSettings/index.tsx
  • src/components/UserProfile/UserSettings/UserLinkedAccountsSettings/LinkJellyfinModal.tsx
  • src/components/UserProfile/UserSettings/UserLinkedAccountsSettings/index.tsx
  • src/components/UserProfile/UserSettings/UserNotificationSettings/UserNotificationsDiscord.tsx
  • src/components/UserProfile/UserSettings/UserNotificationSettings/UserNotificationsEmail.tsx
  • src/components/UserProfile/UserSettings/UserNotificationSettings/UserNotificationsPushbullet.tsx
  • src/components/UserProfile/UserSettings/UserNotificationSettings/UserNotificationsPushover.tsx
  • src/components/UserProfile/UserSettings/UserNotificationSettings/UserNotificationsTelegram.tsx
  • src/components/UserProfile/UserSettings/UserNotificationSettings/UserNotificationsWebPush/index.tsx
  • src/components/UserProfile/UserSettings/UserNotificationSettings/index.tsx
  • src/components/UserProfile/UserSettings/UserPasswordChange/index.tsx
  • src/components/UserProfile/UserSettings/UserPermissions/index.tsx
  • src/components/UserProfile/UserSettings/index.tsx
  • src/components/UserProfile/index.tsx
  • src/context/SettingsContext.tsx
  • src/hooks/useRequestOverride.ts
  • src/hooks/useUser.ts
  • src/pages/_app.tsx
  • src/pages/collection/[collectionId]/index.tsx
  • src/pages/movie/[movieId]/index.tsx
  • src/pages/tv/[tvId]/index.tsx
  • src/utils/apiUrl.ts
  • src/utils/pushSubscriptionHelpers.ts


const { data: existingUsers } = useSWR<UserResultsResponse>(
`/api/v1/user?take=${children}`
apiUrl(`/user?take=${children}`)
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 26, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Use a numeric source for take instead of children

Line 57 builds take from children (React.ReactNode), which can be non-numeric/undefined and cause incomplete existing-user results (allowing already-imported users to reappear).

💡 Proposed fix
 interface JellyfinImportProps {
   onCancel?: () => void;
   onComplete?: () => void;
   children?: React.ReactNode;
+  userFetchTake?: number;
 }

 const JellyfinImportModal: React.FC<JellyfinImportProps> = ({
   onCancel,
   onComplete,
   children,
+  userFetchTake = 1000,
 }) => {
@@
   const { data: existingUsers } = useSWR<UserResultsResponse>(
-    apiUrl(`/user?take=${children}`)
+    apiUrl(`/user?take=${userFetchTake}`)
   );
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/UserList/JellyfinImportModal.tsx` at line 57,
JellyfinImportModal is interpolating `children` (a React.ReactNode) into the
query string for apiUrl(`/user?take=${children}`), which can be non-numeric;
replace that with a validated numeric value (e.g., use a prop/state like `take`,
`pageSize` or `takeCount` or compute `const take = Number(props.take ??
DEFAULT_TAKE)`), validate/fallback if NaN, and then call
`apiUrl(`/user?take=${take}`)`; reference `apiUrl`, `JellyfinImportModal`, and
`children` in your change so you replace the `children` usage with the numeric
`take` variable.

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.

Out of scope

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@0xSysR3ll, understood! The children-as-take issue is indeed pre-existing and unrelated to this refactor. Would you like me to open a follow-up GitHub issue to track it for a future fix?


🧠 Learnings used
Learnt from: afonsojramos
Repo: seerr-team/seerr PR: 2690
File: src/components/UserProfile/UserSettings/UserLinkedAccountsSettings/LinkJellyfinModal.tsx:59-62
Timestamp: 2026-03-18T13:53:44.567Z
Learning: In `src/components/UserProfile/UserSettings/UserLinkedAccountsSettings/LinkJellyfinModal.tsx`, Jellyfin and Emby are mutually exclusive in practice — only one can be configured as the media server at a time. Therefore, deriving `mediaServerName` from `settings.currentSettings.primaryMediaServer` is always correct and does not need a separate prop for the selected server type.

Learnt from: fallenbagel
Repo: seerr-team/seerr PR: 2212
File: server/routes/auth.ts:744-756
Timestamp: 2026-03-19T10:47:13.570Z
Learning: In seerr-team/seerr (`server/routes/auth.ts`), Jellyfin/Emby user creation always sets `email: account.User.Name` (i.e., username as the email field) as a deliberate fallback because Jellyfin does not store email addresses. This applies to all Jellyfin user creation paths: regular login, user import, and Quick Connect. The `userEmailRequired` warning at the `/me` endpoint is the designed mechanism to prompt users to provide a real email when the admin has enabled email notifications. Do not flag `email: account.User.Name` in Jellyfin user creation blocks as a bug or inconsistency.

Learnt from: michaelhthomas
Repo: seerr-team/seerr PR: 2715
File: server/routes/auth.ts:864-880
Timestamp: 2026-03-20T18:39:29.322Z
Learning: Repo seerr-team/seerr — OIDC required-claims semantics: In server/routes/auth.ts, the hasRequiredClaims check is intentionally strict (claim value must be boolean true). Non-boolean claim values should fail for now; future PRs may add granular control. This preference should guide future reviews and suggestions.

Learnt from: michaelhthomas
Repo: seerr-team/seerr PR: 2715
File: src/components/UserProfile/UserSettings/UserLinkedAccountsSettings/index.tsx:91-114
Timestamp: 2026-03-20T20:33:29.810Z
Learning: Repo seerr-team/seerr — In src/components/UserProfile/UserSettings/UserLinkedAccountsSettings/index.tsx (users/[userId]/settings/linked-accounts), it is acceptable to clear all query parameters after the OIDC callback using router.replace(router.pathname, undefined, { shallow: true }); the dynamic route param [userId] remains intact, and this simpler approach is preferred in this codebase.

Learnt from: fallenbagel
Repo: seerr-team/seerr PR: 2731
File: server/lib/i18n/extractMessages.ts:71-71
Timestamp: 2026-03-20T01:39:15.788Z
Learning: In seerr-team/seerr, the server-side i18n extraction script (`server/lib/i18n/extractMessages.ts`) intentionally calls `saveMessages()` without a `.catch()` handler, mirroring the pre-existing pattern in the frontend extractor. This is acceptable because Node.js 15+ exits with a non-zero code on unhandled promise rejections by default, ensuring CI correctly detects failures. Do not flag this as an issue in future reviews.

Learnt from: michaelhthomas
Repo: seerr-team/seerr PR: 2715
File: src/components/UserProfile/UserSettings/UserLinkedAccountsSettings/index.tsx:248-250
Timestamp: 2026-03-26T16:06:26.280Z
Learning: Repo seerr-team/seerr — In server/routes/user/usersettings.ts, the DELETE endpoints for media servers (`/linked-accounts/plex` and `/linked-accounts/jellyfin`) require the user to have a local email and password; having multiple linked accounts alone is not sufficient. Client UI should gate media-server unlink on `hasPassword` (and primary-admin restriction) rather than `accounts.length > 1`.

Learnt from: gauthier-th
Repo: seerr-team/seerr PR: 2600
File: src/components/Discover/AvailableMedia/index.tsx:9-9
Timestamp: 2026-03-26T15:49:55.253Z
Learning: In this repo, when a React component imports the Next.js error page from 'app/pages/_error', import it with an explicit local alias (e.g., `import ErrorPage from 'app/pages/_error'`) rather than using the identifier `ErrorPage`/`Error` directly in a way that could shadow the global `Error` constructor. This avoids shadowing issues and satisfies Biome’s `noShadowRestrictedNames` rule.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Agreed, pre-existing. I'll file a separate issue for this so it doesn't get lost.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

createdAt: Date;
}[]
>(`/api/v1/user/${user?.id}/pushSubscriptions`, { revalidateOnMount: true });
>(apiUrl(`/user/${user?.id}/pushSubscriptions`), { revalidateOnMount: true });
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 26, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

cat -n src/components/UserProfile/UserSettings/UserNotificationSettings/UserNotificationsWebPush/index.tsx | sed -n '1,100p'

Repository: seerr-team/seerr

Length of output: 4620


🏁 Script executed:

cat -n src/components/UserProfile/UserSettings/UserNotificationSettings/UserNotificationsWebPush/index.tsx | sed -n '240,280p'

Repository: seerr-team/seerr

Length of output: 1747


🏁 Script executed:

cat -n src/components/UserProfile/UserSettings/UserNotificationSettings/UserNotificationsWebPush/index.tsx | sed -n '100,150p'

Repository: seerr-team/seerr

Length of output: 1852


Guard the device-subscriptions SWR key until user is loaded.

Line 78 builds a concrete URL unconditionally, unlike the guarded pattern at line 68. When user is undefined on initial render, this produces /user/undefined/pushSubscriptions and triggers an unnecessary bogus fetch.

🛠️ Proposed fix
   const { data: dataDevices, mutate: revalidateDevices } = useSWR<
     {
       endpoint: string;
       p256dh: string;
       auth: string;
       userAgent: string;
       createdAt: Date;
     }[]
-  >(apiUrl(`/user/${user?.id}/pushSubscriptions`), { revalidateOnMount: true });
+  >(
+    user ? apiUrl(`/user/${user.id}/pushSubscriptions`) : null,
+    { revalidateOnMount: true }
+  );
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
>(apiUrl(`/user/${user?.id}/pushSubscriptions`), { revalidateOnMount: true });
const { data: dataDevices, mutate: revalidateDevices } = useSWR<
{
endpoint: string;
p256dh: string;
auth: string;
userAgent: string;
createdAt: Date;
}[]
>(
user ? apiUrl(`/user/${user.id}/pushSubscriptions`) : null,
{ revalidateOnMount: true }
);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/components/UserProfile/UserSettings/UserNotificationSettings/UserNotificationsWebPush/index.tsx`
at line 78, The SWR key is being constructed unconditionally with
apiUrl(`/user/${user?.id}/pushSubscriptions`) which yields /user/undefined/...
on initial render; update the SWR call in the UserNotificationsWebPush component
to only pass a concrete URL when user is defined (e.g., use user ?
apiUrl(`/user/${user.id}/pushSubscriptions`) : null) so the fetch is skipped
until user is loaded, mirroring the guarded pattern used at the earlier SWR
call.

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.

Out of scope

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@0xSysR3ll, understood — the unguarded SWR key is a pre-existing issue and not introduced by this refactor. Would you like me to open a follow-up GitHub issue to track the fix separately?


🧠 Learnings used
Learnt from: michaelhthomas
Repo: seerr-team/seerr PR: 2715
File: src/components/UserProfile/UserSettings/UserLinkedAccountsSettings/index.tsx:91-114
Timestamp: 2026-03-20T20:33:29.810Z
Learning: Repo seerr-team/seerr — In src/components/UserProfile/UserSettings/UserLinkedAccountsSettings/index.tsx (users/[userId]/settings/linked-accounts), it is acceptable to clear all query parameters after the OIDC callback using router.replace(router.pathname, undefined, { shallow: true }); the dynamic route param [userId] remains intact, and this simpler approach is preferred in this codebase.

Learnt from: afonsojramos
Repo: seerr-team/seerr PR: 2690
File: src/components/Login/index.tsx:73-77
Timestamp: 2026-03-18T13:54:35.854Z
Learning: In `src/components/Login/index.tsx` (PR `#2690`), the state where `enabledAuthMethods` is empty AND `localLogin` is false is considered an admin misconfiguration that cannot occur in practice. The `SettingsUsers` settings UI enforces a validation rule requiring at least one of `localLogin` or one of the enabled auth method flags (plexEnabled, jellyfinEnabled, embyEnabled) to be active before saving. Therefore, the `'plex'` fallback in `defaultMethod` computation is unreachable in a correctly configured Seerr instance and does not need a defensive misconfiguration UI.

Learnt from: michaelhthomas
Repo: seerr-team/seerr PR: 2715
File: src/components/UserProfile/UserSettings/UserLinkedAccountsSettings/index.tsx:248-250
Timestamp: 2026-03-26T16:06:26.280Z
Learning: Repo seerr-team/seerr — In server/routes/user/usersettings.ts, the DELETE endpoints for media servers (`/linked-accounts/plex` and `/linked-accounts/jellyfin`) require the user to have a local email and password; having multiple linked accounts alone is not sufficient. Client UI should gate media-server unlink on `hasPassword` (and primary-admin restriction) rather than `accounts.length > 1`.

Learnt from: michaelhthomas
Repo: seerr-team/seerr PR: 2715
File: server/routes/auth.ts:864-880
Timestamp: 2026-03-20T18:39:29.322Z
Learning: Repo seerr-team/seerr — OIDC required-claims semantics: In server/routes/auth.ts, the hasRequiredClaims check is intentionally strict (claim value must be boolean true). Non-boolean claim values should fail for now; future PRs may add granular control. This preference should guide future reviews and suggestions.

Learnt from: fallenbagel
Repo: seerr-team/seerr PR: 2731
File: server/lib/i18n/extractMessages.ts:71-71
Timestamp: 2026-03-20T01:39:15.788Z
Learning: In seerr-team/seerr, the server-side i18n extraction script (`server/lib/i18n/extractMessages.ts`) intentionally calls `saveMessages()` without a `.catch()` handler, mirroring the pre-existing pattern in the frontend extractor. This is acceptable because Node.js 15+ exits with a non-zero code on unhandled promise rejections by default, ensuring CI correctly detects failures. Do not flag this as an issue in future reviews.

Learnt from: gauthier-th
Repo: seerr-team/seerr PR: 2600
File: src/components/Discover/AvailableMedia/index.tsx:9-9
Timestamp: 2026-03-26T15:49:55.253Z
Learning: In this repo, when a React component imports the Next.js error page from 'app/pages/_error', import it with an explicit local alias (e.g., `import ErrorPage from 'app/pages/_error'`) rather than using the identifier `ErrorPage`/`Error` directly in a way that could shadow the global `Error` constructor. This avoids shadowing issues and satisfies Biome’s `noShadowRestrictedNames` rule.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Same here. Filing separately.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

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.

2 participants