Repository navigation
feat: Tag formatting options - #2504
MagicLegend wants to merge 11 commits into
Conversation
MagicLegend
changed the title
1d9e817 to
86e98dc
Compare
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds an exported enum Changes
Sequence Diagram(s)sequenceDiagram
actor Admin
participant UI as RadarrModal/SonarrModal
participant API as Settings API
participant Config as DVRSettings
participant Subscriber as MediaRequestSubscriber
participant Arr as Arr API
Admin->>UI: Choose tagRequestsFormat
UI->>API: Save settings (include tagRequestsFormat)
API->>Config: Persist tagRequestsFormat
Arr->>Subscriber: New media request event
Subscriber->>Config: Read tagRequestsFormat
alt userid
Subscriber->>Subscriber: tagLabel = "123"
else userid-username
Subscriber->>Subscriber: tagLabel = "123-tomjones"
else username
Subscriber->>Subscriber: tagLabel = "tomjones"
end
Subscriber->>Arr: create/update tag (`tagLabel`) and attach to request
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/i18n/locale/fr.json (1)
1204-1213:⚠️ Potential issue | 🟡 MinorFrench locale still uses legacy tag keys and misses new tag-format strings.
The UI now references
tagRequestsFormat+tagFormatUserId/UserIdUsername/Username, but fr.json only adds the oldtagRequestsNoIDkeys and keeps the old info text. This will surface missing keys in French and misdescribe the feature.✏️ Suggested i18n updates
- "components.Settings.RadarrModal.tagRequestsNoID": "Pas de ID dans le tag", - "components.Settings.RadarrModal.tagRequestsNoIDInfo": "Ne pas ajouter le préfixe ID utilisateur au tag. Requis que l'option Tager les demandes soit active.", + "components.Settings.RadarrModal.tagRequestsFormat": "Format du tag", + "components.Settings.RadarrModal.tagFormatUserId": "ID utilisateur", + "components.Settings.RadarrModal.tagFormatUserIdUsername": "ID utilisateur - Nom d'utilisateur", + "components.Settings.RadarrModal.tagFormatUsername": "Nom d'utilisateur", - "components.Settings.SonarrModal.tagRequestsNoID": "Pas de ID dans le tag", - "components.Settings.SonarrModal.tagRequestsNoIDInfo": "Ne pas ajouter le préfixe ID utilisateur au tag. Requis que l'option Tager les demandes soit active.", + "components.Settings.SonarrModal.tagRequestsFormat": "Format du tag", + "components.Settings.SonarrModal.tagFormatUserId": "ID utilisateur", + "components.Settings.SonarrModal.tagFormatUserIdUsername": "ID utilisateur - Nom d'utilisateur", + "components.Settings.SonarrModal.tagFormatUsername": "Nom d'utilisateur", - "components.Settings.SonarrModal.tagRequestsInfo": "Ajouter automatiquement un tag supplémentaire avec l'identifiant et le nom du demandeur", + "components.Settings.SonarrModal.tagRequestsInfo": "Ajouter automatiquement un tag supplémentaire avec l'identifiant utilisateur et/ou le nom d'affichage du demandeur", - "components.Settings.RadarrModal.tagRequestsInfo": "Ajouter automatiquement un tag supplémentaire avec l'identifiant et le nom du demandeur", + "components.Settings.RadarrModal.tagRequestsInfo": "Ajouter automatiquement un tag supplémentaire avec l'identifiant utilisateur et/ou le nom d'affichage du demandeur",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/i18n/locale/fr.json` around lines 1204 - 1213, The French locale is missing the new tag-format keys and still uses legacy keys; add translations for the new keys used by the UI: components.Settings.RadarrModal.tagRequestsFormat, components.Settings.RadarrModal.tagFormatUserId, components.Settings.RadarrModal.tagFormatUserIdUsername, components.Settings.RadarrModal.tagFormatUsername (and the corresponding components.Settings.SonarrModal.* keys), and update components.Settings.*.tagRequestsInfo to reflect the new format text (replace the old "tagRequestsNoID"/"tagRequestsNoIDInfo" wording). Ensure each new key has an accurate French string equivalent to the English originals so the UI no longer shows missing keys or incorrect descriptions.
🧹 Nitpick comments (1)
server/subscriber/MediaRequestSubscriber.ts (1)
652-687: Consider extracting tag handling logic to reduce duplication.The tag format handling logic at lines 652-687 is nearly identical to the Radarr logic at lines 306-341. While the current implementation is correct and follows the existing pattern in this file, consider extracting this into a shared helper method to reduce duplication.
♻️ Suggested helper extraction
private async resolveOrCreateUserTag( api: RadarrAPI | SonarrAPI, tagFormat: TagRequestsFormat, requestedBy: { id: number; displayName: string }, entity: MediaRequest ): Promise<number | undefined> { const tags = await api.getTags(); let tagLabel = requestedBy.id + '-' + sanitizeDisplayName(requestedBy.displayName); let userTag = tags.find((v) => v.label.startsWith(requestedBy.id + '-')); if (tagFormat === TagRequestsFormat.USERID) { tagLabel = String(requestedBy.id); userTag = tags.find((v) => v.label === tagLabel); } else if (tagFormat === TagRequestsFormat.USERNAME) { tagLabel = sanitizeDisplayName(requestedBy.displayName.toLowerCase()); userTag = tags.find((v) => v.label === tagLabel); } else if (!userTag) { userTag = tags.find((v) => v.label.startsWith(requestedBy.id + ' - ')); } if (!userTag) { userTag = await api.createTag({ label: tagLabel }); } return userTag?.id; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/subscriber/MediaRequestSubscriber.ts` around lines 652 - 687, Extract the duplicated Sonarr/Radarr tag-resolution and creation logic in MediaRequestSubscriber.ts into a shared helper (e.g., resolveOrCreateUserTag) that accepts the API client (RadarrAPI | SonarrAPI), TagRequestsFormat, requestedBy ({id, displayName}) and the MediaRequest entity, then moves the tag label construction, lookups (startsWith id+'-' and legacy ' - '), format branches (USERID, USERNAME) and createTag call into that helper and return the resolved tag id; replace the in-place logic in the Sonarr block (the code handling tagFormat, tagLabel, userTag, and createTag) with a call to this helper to remove duplication with the Radarr block.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/i18n/locale/fr.json`:
- Around line 1204-1213: The French locale is missing the new tag-format keys
and still uses legacy keys; add translations for the new keys used by the UI:
components.Settings.RadarrModal.tagRequestsFormat,
components.Settings.RadarrModal.tagFormatUserId,
components.Settings.RadarrModal.tagFormatUserIdUsername,
components.Settings.RadarrModal.tagFormatUsername (and the corresponding
components.Settings.SonarrModal.* keys), and update
components.Settings.*.tagRequestsInfo to reflect the new format text (replace
the old "tagRequestsNoID"/"tagRequestsNoIDInfo" wording). Ensure each new key
has an accurate French string equivalent to the English originals so the UI no
longer shows missing keys or incorrect descriptions.
---
Nitpick comments:
In `@server/subscriber/MediaRequestSubscriber.ts`:
- Around line 652-687: Extract the duplicated Sonarr/Radarr tag-resolution and
creation logic in MediaRequestSubscriber.ts into a shared helper (e.g.,
resolveOrCreateUserTag) that accepts the API client (RadarrAPI | SonarrAPI),
TagRequestsFormat, requestedBy ({id, displayName}) and the MediaRequest entity,
then moves the tag label construction, lookups (startsWith id+'-' and legacy ' -
'), format branches (USERID, USERNAME) and createTag call into that helper and
return the resolved tag id; replace the in-place logic in the Sonarr block (the
code handling tagFormat, tagLabel, userTag, and createTag) with a call to this
helper to remove duplication with the Radarr block.
|
Ah, Coderabbit is of course rightfully mad about the French translation now. Is there maybe a French speaker who can confirm the AI translations Coderabbit gave? I also agree with the code duplication nitpick, but since this seemed to be the pattern in this file I ended up duplicating the code anyway. |
|
I speak french, what do you need? |
gauthier-th
left a comment
gauthier-th
left a comment
There was a problem hiding this comment.
Also please change the description of User ID - Username to UserID-Username to avoid confusion with spaces in tags.
Translations (other than English) don't belong to the PR. As said above, they are handled by our Weblate. |
3450dcd to
11670fa
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
server/subscriber/MediaRequestSubscriber.ts (1)
304-356: Consider extracting shared tagging logic.The Radarr (lines 304-356) and Sonarr (lines 650-702) tagging blocks are nearly identical. While the PR author noted following existing patterns, a helper function could reduce duplication and ensure consistent behavior across both integrations.
This is not blocking since the current implementation works correctly.
Example helper extraction
async function resolveOrCreateUserTag( api: RadarrAPI | SonarrAPI, tagFormat: TagRequestsFormat, userId: number, displayName: string ): Promise<{ id?: number; label: string }> { const tags = await api.getTags(); const sanitized = sanitizeDisplayName(displayName.toLowerCase()); let tagLabel: string; let userTag: typeof tags[number] | undefined; switch (tagFormat) { case TagRequestsFormat.USERID: tagLabel = String(userId); userTag = tags.find((v) => v.label === tagLabel); break; case TagRequestsFormat.USERNAME: tagLabel = sanitized; userTag = tags.find((v) => v.label === tagLabel); break; default: // USERID_USERNAME tagLabel = `${userId}-${sanitized}`; userTag = tags.find((v) => v.label.startsWith(`${userId}-`)) ?? tags.find((v) => v.label.startsWith(`${userId} - `)); break; } return userTag ?? await api.createTag({ label: tagLabel }); }Also applies to: 650-702
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/subscriber/MediaRequestSubscriber.ts` around lines 304 - 356, The repeated tagging logic in MediaRequestSubscriber.ts (the Radarr block around the Radarr tag handling and the Sonarr block at lines ~650-702) should be extracted into a shared helper (e.g., resolveOrCreateUserTag) that accepts the API client (RadarrAPI|SonarrAPI), TagRequestsFormat, userId, displayName and returns the resolved/created tag object/label; replace both inline blocks with calls to that helper and then push the returned tag.id into the existing tags array if present, preserving existing logging and radarr/sonarr.createTag behavior and types used by functions like radarr.getTags()/createTag and sanitizeDisplayName so imports and types align.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@server/subscriber/MediaRequestSubscriber.ts`:
- Around line 310-326: The tag casing is inconsistent: when tagFormat is
TagRequestsFormat.USERNAME the code lowercases entity.requestedBy.displayName
before calling sanitizeDisplayName but for the USERID_USERNAME branch it does
not, producing mixed-case tags; update the USERID_USERNAME construction (the
tagLabel and the radarrTags lookup for userTag) to lowercase the display name
before sanitization just like the USERNAME branch (apply to variables tagLabel,
userTag, sanitizeDisplayName and the radarrTags.find calls that use
entity.requestedBy.displayName) so both formats produce lowercase sanitized
tags.
---
Nitpick comments:
In `@server/subscriber/MediaRequestSubscriber.ts`:
- Around line 304-356: The repeated tagging logic in MediaRequestSubscriber.ts
(the Radarr block around the Radarr tag handling and the Sonarr block at lines
~650-702) should be extracted into a shared helper (e.g.,
resolveOrCreateUserTag) that accepts the API client (RadarrAPI|SonarrAPI),
TagRequestsFormat, userId, displayName and returns the resolved/created tag
object/label; replace both inline blocks with calls to that helper and then push
the returned tag.id into the existing tags array if present, preserving existing
logging and radarr/sonarr.createTag behavior and types used by functions like
radarr.getTags()/createTag and sanitizeDisplayName so imports and types align.
11670fa to
6385feb
Compare
I considered this as well, but ended up not doing it for readability. I've changed the labels. I've also updated the |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
server/subscriber/MediaRequestSubscriber.ts (1)
304-356: Consider extracting shared tag-handling logic.The tagging logic for Radarr (lines 304-356) and Sonarr (lines 650-702) is nearly identical. A shared helper function could reduce duplication and ensure both flows stay in sync.
That said, I acknowledge your PR comment about matching existing file patterns—this is a reasonable tradeoff and can be addressed in a future cleanup.
Example helper extraction
interface TagHandler { getTags: () => Promise<{ id: number; label: string }[]>; createTag: (options: { label: string }) => Promise<{ id: number; label: string }>; } async function resolveUserTag( handler: TagHandler, tagFormat: TagRequestsFormat, userId: number, displayName: string ): Promise<{ id: number; label: string } | undefined> { const tags = await handler.getTags(); const sanitizedName = sanitizeDisplayName(displayName.toLowerCase()); let tagLabel: string; let userTag: { id: number; label: string } | undefined; switch (tagFormat) { case TagRequestsFormat.USERID: tagLabel = String(userId); userTag = tags.find((v) => v.label === tagLabel); break; case TagRequestsFormat.USERNAME: tagLabel = sanitizedName; userTag = tags.find((v) => v.label === tagLabel); break; default: // USERID_USERNAME tagLabel = `${userId}-${sanitizedName}`; userTag = tags.find((v) => v.label.startsWith(`${userId}-`)) ?? tags.find((v) => v.label.startsWith(`${userId} - `)); break; } if (!userTag) { userTag = await handler.createTag({ label: tagLabel }); } return userTag; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/subscriber/MediaRequestSubscriber.ts` around lines 304 - 356, Extract the duplicated tag resolution/creation logic into a shared helper (e.g., resolveUserTag) that accepts an adapter implementing a small TagHandler interface (methods: getTags and createTag), the TagRequestsFormat, userId and displayName, and returns the resolved tag object or undefined; replace the Radarr block (radarr.getTags, radarr.createTag, tagLabel/userTag logic) and the Sonarr block with calls to this helper, reusing existing utilities TagRequestsFormat and sanitizeDisplayName to build labels and to preserve the old-space fallback (startsWith(userId + ' - ')) for USERID_USERNAME mode. Ensure callers push the returned tag.id into their tags array and keep the same logging paths (logger.info/warn) when createTag fails.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@server/subscriber/MediaRequestSubscriber.ts`:
- Around line 310-326: The tag generation is inconsistent: in the branch that
builds USERID_USERNAME tags it calls
sanitizeDisplayName(entity.requestedBy.displayName) without lowercasing, while
the USERNAME branch lowercases before sanitizing; update the USERID_USERNAME
branch so tagLabel uses
sanitizeDisplayName(entity.requestedBy.displayName.toLowerCase()) and ensure
userTag lookup uses the same lowercased/sanitized label (references: tagLabel,
userTag, sanitizeDisplayName, and TagRequestsFormat.USERNAME/USERID_USERNAME) so
all display-name-based tags are consistently lowercase.
- Around line 656-672: The Sonarr tag-label logic in MediaRequestSubscriber.ts
is inconsistent: when tagFormat === TagRequestsFormat.USERID_USERNAME you build
tagLabel using sanitizeDisplayName(entity.requestedBy.displayName) (not
lowercased) but when tagFormat === TagRequestsFormat.USERNAME you use
sanitizeDisplayName(entity.requestedBy.displayName.toLowerCase()), causing case
mismatches; update the USERID_USERNAME branch to lowercase the display name
before sanitizing (use the same .toLowerCase() call used in the USERNAME branch)
and ensure the userTag lookups (sonarrTags.find) compare against the
normalized/lowercased sanitized label consistently so sanitizeDisplayName and
TagRequestsFormat branches produce matching case.
---
Nitpick comments:
In `@server/subscriber/MediaRequestSubscriber.ts`:
- Around line 304-356: Extract the duplicated tag resolution/creation logic into
a shared helper (e.g., resolveUserTag) that accepts an adapter implementing a
small TagHandler interface (methods: getTags and createTag), the
TagRequestsFormat, userId and displayName, and returns the resolved tag object
or undefined; replace the Radarr block (radarr.getTags, radarr.createTag,
tagLabel/userTag logic) and the Sonarr block with calls to this helper, reusing
existing utilities TagRequestsFormat and sanitizeDisplayName to build labels and
to preserve the old-space fallback (startsWith(userId + ' - ')) for
USERID_USERNAME mode. Ensure callers push the returned tag.id into their tags
array and keep the same logging paths (logger.info/warn) when createTag fails.
76f2364 to
2bf7980
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@server/lib/settings/index.ts`:
- Around line 79-81: The settings schema currently drops legacy boolean
tagRequestsNoID and defaults tagRequestsFormat to USERID_USERNAME, causing
installs that previously had tagRequestsNoID=true to revert; add a migration or
load-time normalization that maps tagRequestsNoID → tagRequestsFormat (if
tagRequestsNoID === true set tagRequestsFormat = TagRequestsFormat.USERNAME, if
false set TagRequestsFormat.USERID_USERNAME) before the rest of the app reads
settings, update the settings loader/initializer (where tagRequestsFormat is
read) to apply this mapping and consider making tagRequestsFormat optional in
the Settings interface until migration runs so the runtime accurately reflects
legacy state.
2bf7980 to
f966dce
Compare
|
This PR is waiting on a reply about the following:
@gauthier-th Would the preference be to keep the user-supplied capitalization, or to standardize it to lowercase? Personally I would do the latter to ensure consistency. I'd like to get this merged to get this in the next release, I'm waiting on this feature :) |
We always used capitalized usernames. I don't think we should change it. Admins/users will decide if they want uppercase in the usernames. |
e179a3d to
1cd5905
Compare
|
@gauthier-th Gotcha! I've removed the Is there anything else needed from my side to get this merged? |
Patience 🙂 |
|
This pull request has merge conflicts. Please resolve the conflicts so the PR can be successfully reviewed and merged. |
|
if this change appears in main, should we expect a sync method to push new format and remove old one (based on old requests)? |
…sed tag format Created a dropdown to give the user control of what tag style is used. This change means its no longer mandatory to have the internal userID in the tag. The dropdown is also automatically hidden if the tagging is disabled to decrease UI clutter. This commit refactors work from commit bbf0f06. Resolves issue seerr-team#727 Builds on and obsoletes PR seerr-team#2282
…tag formatting options Changed the hard-coded strings to an enum to decrease the possibility of spelling mistakes causing broken code.
… handled through Weblate
…ect actual generated tag
Forgot to run i18n:extract, oops
d2dcfb5 to
b9f1227
Compare
github-actions
Bot
removed
the
merge conflict
There was a problem hiding this comment.
Pull request overview
Adds configurable tag formatting for Radarr/Sonarr “Tag Requests”, allowing admins to choose whether requester tags are created as user id only, username only, or id-username, and updates the subscriber logic accordingly.
Changes:
- Introduces
TagRequestsFormatenum and persists atagRequestsFormatsetting alongsidetagRequests. - Updates Radarr/Sonarr settings modals to show a nested “Tag Format” dropdown when “Tag Requests” is enabled.
- Updates
MediaRequestSubscriberto create/find tags according to the selected format (with legacy tag matching support).
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
src/i18n/locale/en.json |
Adds new locale keys for the tag format dropdown and updates tag request help text. |
src/components/Settings/SonarrModal/index.tsx |
Adds tag format dropdown UI and submits tagRequestsFormat. |
src/components/Settings/RadarrModal/index.tsx |
Adds tag format dropdown UI and submits tagRequestsFormat. |
server/subscriber/MediaRequestSubscriber.ts |
Updates Radarr/Sonarr tag matching/creation logic to honor the configured format. |
server/lib/settings/index.ts |
Adds tagRequestsFormat to DVR settings typing and re-exports the enum. |
server/constants/server.ts |
Defines TagRequestsFormat enum values. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (3)
server/lib/settings/index.ts:87
tagRequestsFormatis a newly introduced setting and appears to be optional in practice (no migration + existing stored Radarr/Sonarr instances may not have it). Keeping it required inDVRSettingsmakes the type system assume it always exists, which can hide runtimeundefinedreads elsewhere.
tagRequestsFormat: TagRequestsFormat;
server/subscriber/MediaRequestSubscriber.ts:318
- In the USERID_USERNAME path, the initial lookup uses
startsWith("<id>-" ), which can incorrectly match a USERNAME-format tag when the (sanitized) username itself starts with "-" (the file comment already notes this collision case). Excluding the username-only label from the prefix match avoids picking the wrong tag in that scenario.
// new tags do not have spaces around the hyphen, since spaces are not allowed anymore
// note: this can theoretically collide if a USERNAME-format tag starts with this user's id
let userTag = radarrTags.find((v) =>
v.label.startsWith(entity.requestedBy.id + '-')
);
server/subscriber/MediaRequestSubscriber.ts:680
- Same collision risk as the Radarr block: the USERID_USERNAME lookup uses a broad
startsWith("<id>-" )match that can accidentally pick a USERNAME-format tag if the username begins with "-". Excluding the username-only label from the prefix match prevents that wrong-tag selection.
// new tags do not have spaces around the hyphen, since spaces are not allowed anymore
// note: this can theoretically collide if a USERNAME-format tag starts with this user's id
let userTag = sonarrTags.find((v) =>
v.label.startsWith(entity.requestedBy.id + '-')
);
| "components.Settings.RadarrModal.tagFormatUserId": "UserID", | ||
| "components.Settings.RadarrModal.tagFormatUserIdUsername": "UserID-Username", | ||
| "components.Settings.RadarrModal.tagFormatUsername": "Username", |
| "components.Settings.SonarrModal.tagFormatUserId": "UserID", | ||
| "components.Settings.SonarrModal.tagFormatUserIdUsername": "UserID-Username", | ||
| "components.Settings.SonarrModal.tagFormatUsername": "Username", |
| tagRequestsFormat: 'Tag Format', | ||
| tagFormatUserId: 'UserID', | ||
| tagFormatUserIdUsername: 'UserID-Username', | ||
| tagFormatUsername: 'Username', |
| tagRequestsFormat: 'Tag Format', | ||
| tagFormatUserId: 'UserID', | ||
| tagFormatUserIdUsername: 'UserID-Username', | ||
| tagFormatUsername: 'Username', |
Hmm, right, so the problem is if Radarr already has a tag Since, like you also mentioned, both Radarr and Sonarr do not allow duplicate usernames I believe the simple fix would be to simply match tags based on lowercase. I don't think an administrator would need to do anything, since this is a simple fix on our side that does not introduce issues with possible conflicts. An alternative solution would be an extra check before trying to create a new tag: diff --git a/server/subscriber/MediaRequestSubscriber.ts b/server/subscriber/MediaRequestSubscriber.ts
index d8b636b6..5804522c 100644
--- a/server/subscriber/MediaRequestSubscriber.ts
+++ b/server/subscriber/MediaRequestSubscriber.ts
@@ -328,6 +328,29 @@ export class MediaRequestSubscriber implements EntitySubscriberInterface<MediaRe
if (radarrSettings.tagRequests) {
const radarrTags = await radarr.getTags();
const tagFormat =
radarrSettings.tagRequestsFormat ??
TagRequestsFormat.USERID_USERNAME;
let tagLabel =
entity.requestedBy.id +
'-' +
sanitizeDisplayName(entity.requestedBy.displayName);
// new tags do not have spaces around the hyphen, since spaces are not allowed anymore
// note: this can theoretically collide if a USERNAME-format tag starts with this user's id
let userTag = radarrTags.find((v) =>
v.label.startsWith(entity.requestedBy.id + '-')
);
if (tagFormat === TagRequestsFormat.USERID) {
tagLabel = String(entity.requestedBy.id);
userTag = radarrTags.find((v) => v.label === tagLabel);
} else if (tagFormat === TagRequestsFormat.USERNAME) {
tagLabel = sanitizeDisplayName(entity.requestedBy.displayName);
userTag = radarrTags.find((v) => v.label === tagLabel);
} else if (!userTag) {
// old tags had space around the hyphen
userTag = radarrTags.find((v) =>
v.label.startsWith(entity.requestedBy.id + ' - ')
);
}
+ if (!userTag) {
+ // Radarr treats tag labels as case-insensitively unique, but our lookups
+ // above are case-sensitive. Fall back to a case-insensitive match so we
+ // don't attempt to create a tag that only differs by case, which Radarr
+ // will reject with a UNIQUE constraint error.
+ const caseInsensitiveMatch = radarrTags.find(
+ (v) => v.label.toLowerCase() === tagLabel.toLowerCase()
+ );
+ if (caseInsensitiveMatch) {
+ logger.warn(
+ `Requester's tag already exists in Radarr with different casing. Using existing tag to avoid a duplicate tag creation error`,
+ {
+ label: 'Media Request',
+ requestId: entity.id,
+ mediaId: entity.media.id,
+ userId: entity.requestedBy.id,
+ expectedTag: tagLabel,
+ existingTag: caseInsensitiveMatch.label,
+ }
+ );
+ userTag = caseInsensitiveMatch;
+ }
+ }
if (!userTag) {
logger.info(`Requester has no active tag. Creating new`, {
label: 'Media Request',
requestId: entity.id,
mediaId: entity.media.id,
userId: entity.requestedBy.id,
newTag: tagLabel,
});
userTag = await radarr.createTag({ label: tagLabel });
}
if (userTag.id) {
if (!tags?.find((v) => v === userTag?.id)) {
tags?.push(userTag.id);
}
} else {
logger.warn(`Requester has no tag and failed to add one`, {
label: 'Media Request',
requestId: entity.id,
mediaId: entity.media.id,
userId: entity.requestedBy.id,
radarrServer: radarrSettings.hostname + ':' + radarrSettings.port,
});
}
}I'm not a fan of this solution, since I believe you can simply check for the lowercase names. The change-to-lowercase approach: diff --git a/server/subscriber/MediaRequestSubscriber.ts b/server/subscriber/MediaRequestSubscriber.ts
index 5804522c..ac1f709e 100644
--- a/server/subscriber/MediaRequestSubscriber.ts
+++ b/server/subscriber/MediaRequestSubscriber.ts
@@ -313,44 +313,27 @@ export class MediaRequestSubscriber implements EntitySubscriberInterface<MediaRe
if (radarrSettings.tagRequests) {
const radarrTags = await radarr.getTags();
const tagFormat =
radarrSettings.tagRequestsFormat ??
TagRequestsFormat.USERID_USERNAME;
let tagLabel =
entity.requestedBy.id +
'-' +
sanitizeDisplayName(entity.requestedBy.displayName);
// new tags do not have spaces around the hyphen, since spaces are not allowed anymore
// note: this can theoretically collide if a USERNAME-format tag starts with this user's id
+ // tag labels are compared case-insensitively, since Radarr enforces case-insensitive
+ // uniqueness on tag labels
let userTag = radarrTags.find((v) =>
- v.label.startsWith(entity.requestedBy.id + '-')
+ v.label.toLowerCase().startsWith(entity.requestedBy.id + '-')
);
if (tagFormat === TagRequestsFormat.USERID) {
tagLabel = String(entity.requestedBy.id);
- userTag = radarrTags.find((v) => v.label === tagLabel);
+ userTag = radarrTags.find(
+ (v) => v.label.toLowerCase() === tagLabel.toLowerCase()
+ );
} else if (tagFormat === TagRequestsFormat.USERNAME) {
tagLabel = sanitizeDisplayName(entity.requestedBy.displayName);
- userTag = radarrTags.find((v) => v.label === tagLabel);
+ userTag = radarrTags.find(
+ (v) => v.label.toLowerCase() === tagLabel.toLowerCase()
+ );
} else if (!userTag) {
// old tags had space around the hyphen
userTag = radarrTags.find((v) =>
- v.label.startsWith(entity.requestedBy.id + ' - ')
+ v.label.toLowerCase().startsWith(entity.requestedBy.id + ' - ')
);
}
if (!userTag) {
logger.info(`Requester has no active tag. Creating new`, {
label: 'Media Request',
requestId: entity.id,
mediaId: entity.media.id,
userId: entity.requestedBy.id,
newTag: tagLabel,
});
userTag = await radarr.createTag({ label: tagLabel });
}
if (userTag.id) {
if (!tags?.find((v) => v === userTag?.id)) {
tags?.push(userTag.id);
}
} else {
logger.warn(`Requester has no tag and failed to add one`, {
label: 'Media Request',
requestId: entity.id,
mediaId: entity.media.id,
userId: entity.requestedBy.id,
radarrServer: radarrSettings.hostname + ':' + radarrSettings.port,
});
}
}I agree that this might not be a problem that a user would run into often, but as I mentioned it's a problem that I personally ran into with how my Radarr/Sonarr was set up from the past. And given how simple the fix is I don't see a reason not to simply do this.
If it would continue to assign movies to the old tag, it would only increase the amount of manual work the administrator needs to do to migrate (because you have to manually unassign all tags from all movies before you can remove a tag). Another option would be to make sure the migration can be called manually from the settings page. Note again that this case would only happen if both the old and new style tags would exist. @fallenbagel wdyt? I still miss this feature every time a romantic comedy for my parents comes up in my library because I can't easily separate their requests from mine in Radarr 😄 |
|
This PR is stale because it has been open 30 days with no activity. Please address the feedback or provide an update to keep it open. |
|
I think I f this could be a migration function that upon changing your tag format, It asks if you wish to update the existing tags and pushes a modification to Radarr/Sonarr etc... This way the check isn't run every single time but offers a per change opportunity to fix the tags. |
Oh sort of like how immich does their media storage template migrations right? |
Yeah, exactly. |
commented
Sep 28, 2026
|
This PR is stale because it has been open 30 days with no activity. Please address the feedback or provide an update to keep it open. |
commented
Oct 29, 2026
|
This PR was closed because it has been stalled for 30 days with no activity. You can reopen it once you address the feedback or provide the requested changes. |
Description
Took the liberty of implementing the proposed dropdown, indentation and hiding changes proposed by fallenbagel in #2282 (comment).
This creates an option for the user to choose how they want the tags formatted.
Note that in my Radarr instance migration 0007 did not actually update the tags to the new format. Wasn't it supposed to do this automatically? (See screenshot below) I'm aware that the current matching logic could have collisions if the user changes to a different format and has an username starting with a number followed by a dash that also matches an existing label. (e.g.
1-the-man).AI was used for this PR. Please open me for more details.
Model used: Anthropic Claude Sonnet 4.6.
Prompt:
Context files given:
src/components/Settings/RadarrModal/index.tsxsrc/components/PermissionOption/index.tsxserver/subscriber/MediaRequestSubscriber.tsNote that it took the changes to the French i18n file and updated those, and created a migration to accommodate the changed config value. Given that the change this was built upon has not reached GA yet, I removed the migration. I also did not include the changes it made to the French i18n file because I could not validate the translation myself.
Complete reasoning output of the initial changes:
I then got it to change to an enum:
Prompt:
Then I prompted for a refactor to better match the coding style of the rest of the file:
Prompt:
How Has This Been Tested?
I tested this by connecting the development instance running with
pnpmto my 'production' Radarr and Sonarr instances. Then, I tried all three configurations and confirmed that the tag was either created or set correctly. I also validated the logic by attaching a Node debugger toMediaRequestSubscriber.tsto verify the logic.Node Debugger stuff in case somebody is interested
Got Claude Sonnet 4.5 to figure out the configs, so they are indeed garbage, hence I'm not committing any of it. But they do work. Would be great if somebody who is more familiar with these tools could tell me which settings are indeed required for debugging and source maps so this can be documented properly.
package.json:Added a debug command to start in development mode with the debug server:
Then, created a
launch.json:{ "version": "0.2.0", "configurations": [ { "type": "node", "request": "attach", "name": "Attach to Debug Server", "port": 9229, "restart": true, "skipFiles": ["<node_internals>/**", "**/node_modules/**"], "protocol": "inspector", "sourceMaps": true, "localRoot": "${workspaceFolder}", "remoteRoot": "${workspaceFolder}", "resolveSourceMapLocations": [ "${workspaceFolder}/**", "!**/node_modules/**" ], "console": "integratedTerminal", "internalConsoleOptions": "neverOpen" } ] }This should be enough to get a debugger working. Try placing a breakpoint in
server/index.tsat line 1 or 2 and it should bind (= become red).Then, to also get React Devtools working I added configs for sourcemapping:
tsconfig.json, add:{ "compilerOptions": { "sourceMap": true } }server/tsconfig.json, replace:{ "extends": "../tsconfig.json", "compilerOptions": { "target": "ES2020", "module": "commonjs", "outDir": "../dist", "noEmit": false, "incremental": true, "baseUrl": ".", "sourceMap": false, "inlineSourceMap": true, "inlineSources": true, "paths": { "@server/*": ["*"] } }, "include": ["**/*.ts"], "ts-node": { "files": true, "transpileOnly": false, "compilerOptions": { "sourceMap": false, "inlineSourceMap": true, "inlineSources": true } } }No, not all settings are probably needed, but I couldn't be arsed with figuring out which ones did the trick, I just wanted it to work.
Screenshots / Logs (if applicable)
No dropdown if the tag option is disabled:
Added dropdown:
Result from tests:
Old tags with spaces:
Just username tag:
Just ID:
Checklist:
pnpm buildpnpm i18n:extractSummary by CodeRabbit
New Features
Localization