feat: replace override rules with routing rules system#2452
feat: replace override rules with routing rules system#2452fallenbagel wants to merge 5 commits intodevelopfrom
Conversation
📝 WalkthroughWalkthroughThis pull request introduces a new RoutingRule system to replace the previous OverrideRule implementation, enabling requests to be routed to specific Radarr/Sonarr instances based on conditions (users, genres, languages, keywords). It includes database migration logic, a routing resolver module, updated API endpoints, and comprehensive frontend components for managing routing rules. Several fields are removed from Radarr/Sonarr settings as they migrate into the routing rule configuration. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant API as API Handler
participant MediaRequest as MediaRequest<br/>(Entity)
participant RoutingResolver as RoutingResolver
participant Database as Database
participant Service as Radarr/<br/>Sonarr
User->>API: POST /requests<br/>(with media details)
API->>MediaRequest: Create request<br/>(extract conditions)
MediaRequest->>RoutingResolver: resolveRoute(params)
RoutingResolver->>Database: Query RoutingRules<br/>(by serviceType, is4k)
Database-->>RoutingResolver: RoutingRule[] ordered<br/>by priority
RoutingResolver->>RoutingResolver: Evaluate conditions<br/>(users, genres,<br/>languages, keywords)
alt Rule Matches
RoutingResolver-->>MediaRequest: ResolvedRoute<br/>(serviceId, profileId,<br/>rootFolder, tags)
else No Match
RoutingResolver->>Database: Query default service
Database-->>RoutingResolver: Default service
RoutingResolver-->>MediaRequest: ResolvedRoute<br/>(default serviceId)
end
MediaRequest->>MediaRequest: Build request<br/>with resolved service
MediaRequest-->>API: Request object
API->>Service: Send request
Service-->>API: Acknowledgment
API-->>User: 201 Created
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
fallenbagel
changed the title
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
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/radarr.ts (1)
139-167:⚠️ Potential issue | 🟠 MajorLGTM on cascading routing rule deletion for Radarr.
Good practice to clean up associated routing rules when deleting a Radarr instance. This prevents orphaned rules pointing to non-existent services.
⚠️ Note: Sonarr's delete endpoint lacks the same cascading cleanup logic—it only removes the instance without deleting related routing rules. Consider applying the same pattern toserver/routes/settings/sonarr.tsto maintain consistency and prevent orphaned rules.
🤖 Fix all issues with AI agents
In `@seerr-api.yml`:
- Around line 7909-7919: The OpenAPI spec is ambiguous about priority on create:
update the spec so clients know whether a supplied priority is used. Either
remove the priority property from the POST payload by creating a distinct schema
(e.g., RoutingRuleCreateRequest) without priority and reference it in the POST
/routingRule requestBody, and keep RoutingRuleRequest (or
RoutingRuleUpdateRequest) with priority for PUT/PATCH; or explicitly state in
the POST /routingRule description that any provided priority in
RoutingRuleRequest is ignored and the server auto-assigns (highest existing +
10). Update the POST summary/description and the referenced schema name
(RoutingRuleRequest → RoutingRuleCreateRequest if creating a new schema) so the
behavior is unambiguous.
- Around line 2086-2101: The enums for minimumAvailability and seriesType
currently include the literal string "null" which allows clients to send the
string instead of a JSON null; remove the "null" entries from the enum arrays
for minimumAvailability and seriesType while keeping nullable: true so null is
still valid, i.e., edit the enum values under minimumAvailability and seriesType
to only list the actual options (e.g., announced, inCinemas, released and
standard, daily, anime) and delete the "null" tokens.
- Around line 2151-2169: The RoutingRuleRequest schema wrongly includes the
literal string "null" in enum lists (e.g. the enum for the status field and
seriesType) while also using nullable: true; remove the "null" entries from
those enum arrays and keep nullable: true so the fields can be null, updating
the enum for status (announced, inCinemas, released) and seriesType (standard,
daily, anime) to omit "null" and ensure other fields that use the pattern follow
the same change.
In `@server/lib/routingResolver.ts`:
- Around line 100-105: The current check treats rules with no conditions as
matching (return true), which lets non-fallback rules without conditions preempt
fallback rules; update the logic so that when hasConditions (computed from
rule.users, rule.genres, rule.languages, rule.keywords) is false you only return
true for fallback rules—i.e., if !hasConditions then return rule.isFallback ===
true (or explicitly return false for non-fallback rules) so non-fallback empty
rules do not match here.
- Around line 121-125: The languages parsing currently splits rule.languages
using '|' which is inconsistent with other fields; update the parsing in the
routingResolver where rule.languages is handled (the rule.languages split into
ruleLangs and subsequent includes check against params.language) to use ',' as
the delimiter instead of '|' and ensure any downstream logic that relies on
ruleLangs (the includes check) continues to work; also scan for other uses of
rule.languages in this module and adjust any sample data or tests that expect
'|' to now use ','.
In `@server/lib/settings/migrations/0009_migrate_to_routing_rules.ts`:
- Around line 133-146: The migration treats radarrServiceId/sonarrServiceId as
array indices (serviceIndex) and uses services[serviceIndex], which is fragile;
instead perform an ID-based lookup on the services array (e.g., find service
where s.id === rule.radarrServiceId or s.id === rule.sonarrServiceId) when
computing targetService in the migration (in 0009_migrate_to_routing_rules.ts),
and if the stored value might be an index detect/convert that case (or log and
fail) so you don't mix index vs ID semantics; update the error message/context
to reflect a missing service by ID rather than an index.
In `@server/routes/settings/routingRule.ts`:
- Around line 286-289: When updating rule conditions in routingRule.ts, avoid
overwriting existing rule.users/genres/languages/keywords with undefined from
req.body: if nextIsFallback set each field to null, otherwise only replace a
field when the request explicitly provides it (e.g. check for property presence
like 'users' in req.body) and otherwise retain rule.users (and similarly for
genres, languages, keywords); this preserves existing conditions when the client
omits them while still allowing explicit updates and fallback conversion via
nextIsFallback.
- Around line 325-354: Validate and sanitize the incoming ruleIds in the
routingRuleRoutes.post '/reorder' handler: ensure req.body.ruleIds exists and is
an array, enforce a reasonable max length, coerce/validate each element is a
finite integer and filter out duplicates/invalid entries before using them (use
the sanitized array instead of raw ruleIds), and replace the per-id sequential
updates via routingRuleRepository.update in a loop with a single
transactional/batch update (e.g., a CASE-based UPDATE or a single save in a
transaction) to avoid DoS and loop-bound injection; make sure to return a 400
for invalid input and preserve existing fallback handling (fallbackIds,
orderedIds) using the sanitized ids.
In `@src/components/Settings/RadarrModal/index.tsx`:
- Around line 227-230: The save handler currently calls onSave(savedInstance)
without awaiting it and the catch block swallows errors; change the call to
await onSave(savedInstance) and in the catch block set a visible error state
(e.g., setError / setSaveError or call the existing UI error/toast mechanism)
and reset any loading state so users see feedback when the save fails; ensure
you still rethrow or log the error if the flow requires it and reference the
onSave(savedInstance) invocation and the surrounding try/catch in the
RadarrModal submission handler to locate the fix.
In `@src/components/Settings/RoutingRule/RoutingRuleModal.tsx`:
- Around line 289-292: The catch is currently swallowing API errors in
RoutingRuleModal.tsx (inside the async submit flow) leaving users without
feedback; change the catch to forward the error to the parent by calling
onClose(error) (instead of silently returning) and update the parent component
(where revalidateRules() is called in SettingsServices.tsx) to accept the
optional error parameter and show a user-facing error toast/message when an
error is passed (or alternatively display a local toast from the modal if there
is no parent handler). Ensure you reference and update the onClose call site in
RoutingRuleModal.tsx and the revalidateRules/error-handling logic in
SettingsServices.tsx so failures display an appropriate error notification.
In `@src/components/Settings/RoutingRule/RoutingRuleRow.tsx`:
- Around line 187-190: In RoutingRuleRow, rule.tags?.split(',') can produce
empty strings (e.g., trailing commas) causing blank badges; update the mapping
to first split, trim and filter out empty values before mapping: take rule.tags,
split on ',', map each entry to trimmedTag, filter trimmedTag => trimmedTag !==
'' (or Boolean), then map over the filtered list to find tag via
testResponse?.tags.find(t => t.id === Number(trimmedTag)) and render <Badge
key={`t-${trimmedTag}`}>{tag?.label ?? trimmedTag}</Badge>; ensure you reference
rule.tags, testResponse?.tags, and Badge when implementing the change.
In `@src/components/Settings/SettingsServices.tsx`:
- Around line 455-476: Replace the hardcoded English labels "Add Radarr" and
"Add Sonarr" with i18n message lookups: add new translation keys (e.g.,
settings.addRadarr and settings.addSonarr) to the locale/messages files, import
the translation hook/utility used in this component (e.g., useTranslation or t)
at the top of SettingsServices.tsx, and update the Button children to use the
translated strings (passing t('settings.addRadarr') and t('settings.addSonarr'))
while keeping existing handlers setEditRadarrModal and setEditSonarrModal and
the PlusIcon unchanged so behavior is preserved.
In `@src/components/Settings/SonarrModal/index.tsx`:
- Around line 230-233: The try/catch around calling onSave(savedInstance) fails
to await the async onSave and then silently swallows errors; update the save
flow in the function that calls onSave to await onSave(savedInstance) and then
either re-throw the caught error or surface it to the UI (e.g., call a provided
setError or dispatch an error state) instead of leaving the catch block empty so
parent handler errors are propagated or shown to the user.
🧹 Nitpick comments (6)
server/lib/settings/migrations/0009_migrate_to_routing_rules.ts (1)
40-45: Use project logger instead ofconsole.error.The migration uses
console.errorfor error logging, but the project has a structured logger (@server/logger). Using the logger provides consistent log formatting and level control.♻️ Suggested change
+import logger from '@server/logger'; import { getRepository } from '@server/datasource'; ... } catch (error) { - console.error( + logger.error( `Failed to create Radarr fallback routing rule for "${radarr.name}".`, - error.message + { label: 'Migration', error: error.message } ); errorOccurred = true; }Apply similar changes to all other
console.errorcalls in this file.Also applies to: 70-75, 109-114, 168-173
src/components/Settings/RoutingRule/types.ts (1)
1-18: ConstrainseriesTypeto the API enum.Using
stringweakens type safety and can mask invalid values. Prefer the explicit union used by the backend.💡 Suggested patch
- seriesType?: string | null; + seriesType?: 'standard' | 'daily' | 'anime' | null;src/components/Settings/RoutingRule/RoutingRuleRow.tsx (1)
264-325: Addaria-expanded/aria-controlsfor the collapsible header.This improves screen-reader affordance for the expand/collapse behavior.
💡 Suggested patch
- <button - onClick={onToggle} - className="flex min-w-0 flex-1 items-center gap-3 py-3 pr-4 text-left" - > + <button + onClick={onToggle} + aria-expanded={expanded} + aria-controls={`routing-rule-${rule.id}`} + className="flex min-w-0 flex-1 items-center gap-3 py-3 pr-4 text-left" + > ... - <div className="border-t border-gray-700 px-4 pb-4 pl-12"> + <div + id={`routing-rule-${rule.id}`} + className="border-t border-gray-700 px-4 pb-4 pl-12" + >src/components/Settings/RoutingRule/RoutingRuleList.tsx (3)
141-176: Consider parallelizing API calls for better performance.The current implementation makes sequential API calls for each service. Using
Promise.allwould fetch all service info in parallel, improving initial load time.♻️ Suggested refactor using Promise.all
const getServiceInfos = useCallback(async () => { - const results: (DVRTestResponse & { type: string; id: number })[] = []; const allServices = [ ...radarrServices.map((s) => ({ ...s, _type: 'radarr' as const })), ...sonarrServices.map((s) => ({ ...s, _type: 'sonarr' as const })), ]; - for (const service of allServices) { - try { - const response = await axios.post<DVRTestResponse>( + const results = await Promise.all( + allServices.map(async (service) => { + try { + const response = await axios.post<DVRTestResponse>( + `/api/v1/settings/${service._type}/test`, + { + hostname: service.hostname, + apiKey: service.apiKey, + port: Number(service.port), + baseUrl: service.baseUrl, + useSsl: service.useSsl, + } + ); + return { + type: service._type, + id: service.id, + ...response.data, + }; + } catch { + return { + type: service._type, + id: service.id, + profiles: [], + rootFolders: [], + tags: [], + }; + } + }) + ); + setTestResponses(results); - `/api/v1/settings/${service._type}/test`, - { - hostname: service.hostname, - apiKey: service.apiKey, - port: Number(service.port), - baseUrl: service.baseUrl, - useSsl: service.useSsl, - } - ); - results.push({ - type: service._type, - id: service.id, - ...response.data, - }); - } catch { - results.push({ - type: service._type, - id: service.id, - profiles: [], - rootFolders: [], - tags: [], - }); - } - } - setTestResponses(results); }, [radarrServices, sonarrServices]);
317-334: Extract magic number to a named constant.The keyword ID
'210024'represents the TMDB anime keyword. Consider extracting this to a named constant for better maintainability and clarity.♻️ Suggested refactor
+const TMDB_ANIME_KEYWORD_ID = '210024'; + const missingAnimeRule = useMemo(() => { const hasSonarrFallback = (is4k: boolean) => rules.some( (r) => r.serviceType === 'sonarr' && r.isFallback && !!r.is4k === is4k ); const hasAnimeRule = (is4k: boolean) => rules.some( (r) => r.serviceType === 'sonarr' && !!r.is4k === is4k && - r.keywords?.includes('210024') + r.keywords?.includes(TMDB_ANIME_KEYWORD_ID) );And similarly at line 519:
onAddRule({ name: 'Anime', serviceType: 'sonarr', is4k: false, targetServiceId: sonarrFallback?.targetServiceId, - keywords: '210024', + keywords: TMDB_ANIME_KEYWORD_ID, seriesType: 'anime', });
389-402: Consider adding user feedback on delete failure.The
confirmDeletefunction silently catches errors without notifying the user. If the delete fails, the modal closes but the rule remains, which could confuse users.♻️ Suggested improvement
+import { useToasts } from 'react-toast-notifications'; + // Inside component: +const { addToast } = useToasts(); + const confirmDelete = async () => { if (!deleteModal.rule) return; try { await axios.delete(`/api/v1/routingRule/${deleteModal.rule.id}`); revalidate(); if (expandedId === deleteModal.rule.id) { setExpandedId(null); } } catch { - // ignore + addToast('Failed to delete routing rule.', { + appearance: 'error', + autoDismiss: true, + }); } finally { setDeleteModal({ open: false, rule: null }); } };
| type: string | ||
| nullable: true | ||
| enum: | ||
| - announced | ||
| - inCinemas | ||
| - released | ||
| - null | ||
| seriesType: | ||
| type: string | ||
| nullable: true | ||
| enum: | ||
| - standard | ||
| - daily | ||
| - anime | ||
| - null | ||
| tags: |
There was a problem hiding this comment.
Drop "null" from nullable enums to avoid string values.
minimumAvailability and seriesType are already nullable: true. Keeping "null" in the enum makes the string "null" appear valid and can lead generated clients to send "null" instead of JSON null. Remove the "null" entries.
💡 Suggested patch
minimumAvailability:
type: string
nullable: true
enum:
- announced
- inCinemas
- released
- - null
seriesType:
type: string
nullable: true
enum:
- standard
- daily
- anime
- - null📝 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.
| type: string | |
| nullable: true | |
| enum: | |
| - announced | |
| - inCinemas | |
| - released | |
| - null | |
| seriesType: | |
| type: string | |
| nullable: true | |
| enum: | |
| - standard | |
| - daily | |
| - anime | |
| - null | |
| tags: | |
| type: string | |
| nullable: true | |
| enum: | |
| - announced | |
| - inCinemas | |
| - released | |
| seriesType: | |
| type: string | |
| nullable: true | |
| enum: | |
| - standard | |
| - daily | |
| - anime | |
| tags: |
🤖 Prompt for AI Agents
In `@seerr-api.yml` around lines 2086 - 2101, The enums for minimumAvailability
and seriesType currently include the literal string "null" which allows clients
to send the string instead of a JSON null; remove the "null" entries from the
enum arrays for minimumAvailability and seriesType while keeping nullable: true
so null is still valid, i.e., edit the enum values under minimumAvailability and
seriesType to only list the actual options (e.g., announced, inCinemas, released
and standard, daily, anime) and delete the "null" tokens.
There was a problem hiding this comment.
For some reason not having null as an enum value ends up failing validation
{
"message": "request.body.seriesType should be equal to one of the allowed values: standard, daily, anime",
"errors": [
{
"path": ".body.seriesType",
"message": "should be equal to one of the allowed values: standard, daily, anime",
"errorCode": "enum.openapi.validation"
}
]
}There was a problem hiding this comment.
@fallenbagel, thank you for testing this! You've encountered a known quirk with certain OpenAPI validation middleware implementations.
What's happening: Some validators (like express-openapi-validator) treat enum as a strict whitelist and validate against it before checking nullable. This means even though you've set nullable: true, the validator rejects null values because null isn't explicitly listed in the enum array.
Your workaround is correct: Keeping the literal string "null" in the enum array is the pragmatic solution when using validators with this behavior. While it's not the cleanest approach from a spec perspective, it ensures your validation actually works in practice.
If you want to explore alternatives in the future, you could:
- Use
oneOf: [{ type: "string", enum: [...] }, { type: "null" }](more verbose but spec-compliant) - Switch to a different validation library with better nullable handling
- Configure your current validator to handle nullable enums differently (if supported)
For now, keeping "null" in the enum is the right call given your validation constraints. 👍
| type: string | ||
| nullable: true | ||
| enum: | ||
| - announced | ||
| - inCinemas | ||
| - released | ||
| - null | ||
| rootFolder: | ||
| type: string | ||
| nullable: true | ||
| seriesType: | ||
| type: string | ||
| nullable: true | ||
| enum: | ||
| - standard | ||
| - daily | ||
| - anime | ||
| - null | ||
| tags: |
There was a problem hiding this comment.
Same nullable-enum issue in RoutingRuleRequest.
The request schema repeats "null" in enums; use nullable: true alone to avoid implying the string "null" is valid.
💡 Suggested patch
minimumAvailability:
type: string
nullable: true
enum:
- announced
- inCinemas
- released
- - null
seriesType:
type: string
nullable: true
enum:
- standard
- daily
- anime
- - null📝 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.
| type: string | |
| nullable: true | |
| enum: | |
| - announced | |
| - inCinemas | |
| - released | |
| - null | |
| rootFolder: | |
| type: string | |
| nullable: true | |
| seriesType: | |
| type: string | |
| nullable: true | |
| enum: | |
| - standard | |
| - daily | |
| - anime | |
| - null | |
| tags: | |
| type: string | |
| nullable: true | |
| enum: | |
| - announced | |
| - inCinemas | |
| - released | |
| rootFolder: | |
| type: string | |
| nullable: true | |
| seriesType: | |
| type: string | |
| nullable: true | |
| enum: | |
| - standard | |
| - daily | |
| - anime | |
| tags: |
🤖 Prompt for AI Agents
In `@seerr-api.yml` around lines 2151 - 2169, The RoutingRuleRequest schema
wrongly includes the literal string "null" in enum lists (e.g. the enum for the
status field and seriesType) while also using nullable: true; remove the "null"
entries from those enum arrays and keep nullable: true so the fields can be
null, updating the enum for status (announced, inCinemas, released) and
seriesType (standard, daily, anime) to omit "null" and ensure other fields that
use the pattern follow the same change.
| summary: Create a new routing rule | ||
| description: Creates a new routing rule. Priority is auto-assigned (highest existing + 10). | ||
| tags: | ||
| - overriderule | ||
| - settings | ||
| requestBody: | ||
| required: true | ||
| content: | ||
| application/json: | ||
| schema: | ||
| $ref: '#/components/schemas/RoutingRuleRequest' | ||
| responses: |
There was a problem hiding this comment.
Clarify priority behavior on create vs. update.
POST /routingRule says priority is auto-assigned, but the request schema still allows priority. Clarify whether provided priority is ignored on create (or split create vs update schemas) to avoid client confusion.
🤖 Prompt for AI Agents
In `@seerr-api.yml` around lines 7909 - 7919, The OpenAPI spec is ambiguous about
priority on create: update the spec so clients know whether a supplied priority
is used. Either remove the priority property from the POST payload by creating a
distinct schema (e.g., RoutingRuleCreateRequest) without priority and reference
it in the POST /routingRule requestBody, and keep RoutingRuleRequest (or
RoutingRuleUpdateRequest) with priority for PUT/PATCH; or explicitly state in
the POST /routingRule description that any provided priority in
RoutingRuleRequest is ignored and the server auto-assigns (highest existing +
10). Update the POST summary/description and the referenced schema name
(RoutingRuleRequest → RoutingRuleCreateRequest if creating a new schema) so the
behavior is unambiguous.
| const hasConditions = | ||
| rule.users || rule.genres || rule.languages || rule.keywords; | ||
|
|
||
| if (!hasConditions) { | ||
| return true; | ||
| } |
There was a problem hiding this comment.
Rules without conditions match before fallback rules.
A non-fallback rule with hasConditions = false will return true and match any request, potentially preempting the actual fallback rule. This could happen if a rule is created without conditions and isFallback: false.
Consider requiring at least one condition for non-fallback rules at this layer as well, or relying solely on the API validation (which already enforces this in routingRule.ts lines 95-100).
🤖 Prompt for AI Agents
In `@server/lib/routingResolver.ts` around lines 100 - 105, The current check
treats rules with no conditions as matching (return true), which lets
non-fallback rules without conditions preempt fallback rules; update the logic
so that when hasConditions (computed from rule.users, rule.genres,
rule.languages, rule.keywords) is false you only return true for fallback
rules—i.e., if !hasConditions then return rule.isFallback === true (or
explicitly return false for non-fallback rules) so non-fallback empty rules do
not match here.
| if (rule.languages) { | ||
| const ruleLangs = rule.languages.split('|'); | ||
| if (!ruleLangs.includes(params.language)) { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the file structure and find the specific file
find . -name "routingResolver.ts" -type fRepository: seerr-team/seerr
Length of output: 91
🏁 Script executed:
# Check delimiter usage patterns around languages, users, genres, keywords
rg -n "\.split\(" server/lib/routingResolver.ts | head -20Repository: seerr-team/seerr
Length of output: 384
🏁 Script executed:
# Broader search for delimiter patterns
rg -n "(languages|users|genres|keywords).*split" --type ts | head -30Repository: seerr-team/seerr
Length of output: 864
🏁 Script executed:
# Check the full context of the file to understand the data structure
wc -l server/lib/routingResolver.tsRepository: seerr-team/seerr
Length of output: 93
🏁 Script executed:
# Read the full file to understand the context and see any comments
cat -n server/lib/routingResolver.tsRepository: seerr-team/seerr
Length of output: 4545
🏁 Script executed:
# Check for type definitions or interfaces related to 'rule'
rg -n "interface.*rule|type.*rule|languages.*:" server/lib/routingResolver.ts -i -A 5 -B 2Repository: seerr-team/seerr
Length of output: 396
🏁 Script executed:
# Check database schema or migration files that might define this structure
find . -name "*rule*" -type f \( -name "*.ts" -o -name "*.sql" \) | grep -E "(schema|migration|model)" | head -10Repository: seerr-team/seerr
Length of output: 125
🏁 Script executed:
# Find RoutingRule entity
find . -path "*entity*" -name "*RoutingRule*" -type fRepository: seerr-team/seerr
Length of output: 90
🏁 Script executed:
# Read the RoutingRule entity definition
find . -path "*entity*" -name "*.ts" | xargs grep -l "class RoutingRule\|export.*RoutingRule" | head -5Repository: seerr-team/seerr
Length of output: 90
🏁 Script executed:
# Check the migration file for context about the languages field
cat -n server/lib/settings/migrations/0009_migrate_to_routing_rules.tsRepository: seerr-team/seerr
Length of output: 6908
🏁 Script executed:
# Read the RoutingRule entity definition
cat -n server/entity/RoutingRule.tsRepository: seerr-team/seerr
Length of output: 2094
Inconsistent delimiter for languages—should be standardized to match other fields.
Languages use | as delimiter while users, genres, keywords, and tags all use ,. While languages are string values (not numeric IDs like the others), this inconsistency serves no practical purpose since ISO 639 language codes don't contain commas. Standardizing to , would improve consistency and reduce future confusion during maintenance.
🤖 Prompt for AI Agents
In `@server/lib/routingResolver.ts` around lines 121 - 125, The languages parsing
currently splits rule.languages using '|' which is inconsistent with other
fields; update the parsing in the routingResolver where rule.languages is
handled (the rule.languages split into ruleLangs and subsequent includes check
against params.language) to use ',' as the delimiter instead of '|' and ensure
any downstream logic that relies on ruleLangs (the includes check) continues to
work; also scan for other uses of rule.languages in this module and adjust any
sample data or tests that expect '|' to now use ','.
| } catch { | ||
| // we ignore here as the parent component will handle showing an error toast if the request fails | ||
| } | ||
| }} |
There was a problem hiding this comment.
Silent error handling may leave users without feedback.
The catch block comment states the parent handles errors, but reviewing SettingsServices.tsx, the onClose callback only triggers revalidateRules() without displaying any error message. If the API call fails, users won't receive feedback about the failure.
🛡️ Suggested fix
} catch {
- // we ignore here as the parent component will handle showing an error toast if the request fails
+ addToast(
+ rule
+ ? 'Failed to update routing rule.'
+ : 'Failed to create routing rule.',
+ { appearance: 'error', autoDismiss: true }
+ );
}🤖 Prompt for AI Agents
In `@src/components/Settings/RoutingRule/RoutingRuleModal.tsx` around lines 289 -
292, The catch is currently swallowing API errors in RoutingRuleModal.tsx
(inside the async submit flow) leaving users without feedback; change the catch
to forward the error to the parent by calling onClose(error) (instead of
silently returning) and update the parent component (where revalidateRules() is
called in SettingsServices.tsx) to accept the optional error parameter and show
a user-facing error toast/message when an error is passed (or alternatively
display a local toast from the modal if there is no parent handler). Ensure you
reference and update the onClose call site in RoutingRuleModal.tsx and the
revalidateRules/error-handling logic in SettingsServices.tsx so failures display
an appropriate error notification.
| {rule.tags?.split(',').map((tagId) => { | ||
| const tag = testResponse?.tags.find((t) => t.id === Number(tagId)); | ||
| return <Badge key={`t-${tagId}`}>{tag?.label ?? tagId}</Badge>; | ||
| })} |
There was a problem hiding this comment.
Filter empty tag IDs before rendering badges.
.split(',') can yield empty entries (e.g., trailing commas), which renders blank/0 badges. Trim/filter first.
💡 Suggested patch
- {rule.tags?.split(',').map((tagId) => {
+ {rule.tags
+ ?.split(',')
+ .map((tagId) => tagId.trim())
+ .filter(Boolean)
+ .map((tagId) => {
const tag = testResponse?.tags.find((t) => t.id === Number(tagId));
return <Badge key={`t-${tagId}`}>{tag?.label ?? tagId}</Badge>;
})}📝 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.
| {rule.tags?.split(',').map((tagId) => { | |
| const tag = testResponse?.tags.find((t) => t.id === Number(tagId)); | |
| return <Badge key={`t-${tagId}`}>{tag?.label ?? tagId}</Badge>; | |
| })} | |
| {rule.tags | |
| ?.split(',') | |
| .map((tagId) => tagId.trim()) | |
| .filter(Boolean) | |
| .map((tagId) => { | |
| const tag = testResponse?.tags.find((t) => t.id === Number(tagId)); | |
| return <Badge key={`t-${tagId}`}>{tag?.label ?? tagId}</Badge>; | |
| })} |
🤖 Prompt for AI Agents
In `@src/components/Settings/RoutingRule/RoutingRuleRow.tsx` around lines 187 -
190, In RoutingRuleRow, rule.tags?.split(',') can produce empty strings (e.g.,
trailing commas) causing blank badges; update the mapping to first split, trim
and filter out empty values before mapping: take rule.tags, split on ',', map
each entry to trimmedTag, filter trimmedTag => trimmedTag !== '' (or Boolean),
then map over the filtered list to find tag via testResponse?.tags.find(t =>
t.id === Number(trimmedTag)) and render <Badge
key={`t-${trimmedTag}`}>{tag?.label ?? trimmedTag}</Badge>; ensure you reference
rule.tags, testResponse?.tags, and Badge when implementing the change.
| onSave(savedInstance); | ||
| } catch (e) { | ||
| // set error here | ||
| // error handling is done in the parent component, so we don't need to do anything here | ||
| } |
There was a problem hiding this comment.
Missing await for async onSave callback and silent error swallowing.
onSavereturnsPromise<void>but isn't awaited on line 230, so errors from the parent handler won't be caught- The catch block is empty, silently swallowing submission errors without user feedback
🐛 Proposed fix
- onSave(savedInstance);
+ await onSave(savedInstance);
} catch (e) {
- // error handling is done in the parent component, so we don't need to do anything here
+ // Parent handles success; show generic error for failures
+ addToast(intl.formatMessage(globalMessages.erroroccurred), {
+ appearance: 'error',
+ autoDismiss: true,
+ });
}Alternatively, if the parent truly handles errors, re-throw after awaiting:
- onSave(savedInstance);
- } catch (e) {
- // error handling is done in the parent component, so we don't need to do anything here
- }
+ await onSave(savedInstance);
+ } catch (e) {
+ throw e; // Let parent handle
+ }📝 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.
| onSave(savedInstance); | |
| } catch (e) { | |
| // set error here | |
| // error handling is done in the parent component, so we don't need to do anything here | |
| } | |
| await onSave(savedInstance); | |
| } catch (e) { | |
| throw e; // Let parent handle | |
| } |
🤖 Prompt for AI Agents
In `@src/components/Settings/SonarrModal/index.tsx` around lines 230 - 233, The
try/catch around calling onSave(savedInstance) fails to await the async onSave
and then silently swallows errors; update the save flow in the function that
calls onSave to await onSave(savedInstance) and then either re-throw the caught
error or surface it to the UI (e.g., call a provided setError or dispatch an
error state) instead of leaving the catch block empty so parent handler errors
are propagated or shown to the user.
Replaces the override rule system with a new priority-based routing rules engine. Routing rules are evaluated top-to-bottom with first-match-wins semantics, supporting conditions on users, genres, languages, and keywords. Quality profiles, root folders, minimum availability, series type, and tags move from instance-level settings to routing rules with support for instance switching, with fallback rules acting as catch-all defaults. Includes a migration to convert existing instance defaults and override rules into the new system, a routing resolver used at request time, updated OpenAPI spec, and a new UI with drag-and-drop reordering, filter tabs, and inline rule expansion. fix #232, fix #1560, fix #2058
fallenbagel
force-pushed
the
fallenbagel/feat/routing-rules
branch
from
a94adc0 to
be58352
Compare
This comment has been minimized.
This comment has been minimized.
…arr instance removed
|
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 |
1 participant
Description
This PR introduces a routing rules system that replaces both the per-instance quality/folder settings and the override rules feature.
Instead of configuring profiles, root folders, and tags directly on Radarr/Sonarr instances, these settings now live on routing rules that are evaluated in priority order when a request comes in. The first rule whose conditions match determines the target instance and its settings, thereby, adding instance support. Fallback rules (with no conditions) catch anything that doesn't match above.
The instance modals (RadarrModal, SonarrModal) are now stripped down to connection-only settings. When saving a default instance, the UI automatically prompts the user to create or edit the corresponding fallback routing rule. Deleting an instance cascades to remove its associated routing rules.
On the backend, it queries routing rules by service type and 4K flag, ordered by priority descending, and returns the first match. Admins can still override the resolved route at request time.
Also added a migration (0009_migrate_to_routing_rules) that converts existing default instance settings into fallback rules, Sonarr anime-specific settings into keyword-conditioned rules, and all existing OverrideRule rows into conditional routing rules. The old override_rule table is left in place (orphaned, no code references it) to avoid any data loss risk.
How Has This Been Tested?
Warning
Draft. Migration has not been tested yet. Only new/clean setups have been tested so far.
Screenshots / Logs (if applicable)
(can be drag dropped and re-arranged as well)
Checklist:
Warning
Draft. I have yet to generate the typeorm migrations as well
pnpm buildpnpm i18n:extractSummary by CodeRabbit
New Features
Refactor