feat(notifications): Play On Media Server in notification#2392
Open
JackW6809 wants to merge 8 commits intoseerr-team:developfrom
Open
feat(notifications): Play On Media Server in notification#2392JackW6809 wants to merge 8 commits intoseerr-team:developfrom
JackW6809 wants to merge 8 commits intoseerr-team:developfrom
Conversation
JackW6809
force-pushed
the
notification-media-server-links
branch
4 times, most recently
from
6b7b647 to
d90632b
Compare
JackW6809
changed the title
JackW6809
force-pushed
the
notification-media-server-links
branch
2 times, most recently
from
d4691f9 to
c3db226
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:
📝 WalkthroughWalkthroughThe changes extend notification agents (Email, Slack, Telegram) with media server integration. A new utility module provides helpers to resolve media server names and URLs. Notification payloads are enriched with media server context, and conditional action buttons or links are added to templates when media server URLs are available. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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. ✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
JackW6809
force-pushed
the
notification-media-server-links
branch
from
adaed40 to
b64fde1
Compare
Contributor
Author
|
@coderabbitai help |
ChatThere are 3 ways to chat with CodeRabbit:
CodeRabbit commands
Other keywords and placeholders
CodeRabbit configuration file (
|
Contributor
Author
|
@coderabbitai resume |
✅ Actions performedReviews resumed. |
… media is available In the email notification users receive when their media request is now available, I have added a Play on media server (e.g. Play on Plex) button that takes them directly to the media on their media server. feat(notifications): play On Media Server in Discord & Slack Notifications In the Discord & Slack notification users receive when their media request is now available, I have added a Play on media server (e.g. Play on Plex) button that takes them directly to the media on their media server.
…location Created a helper file, this makes it easier to make changes to the function that multiple different agents use in the same fashion. BREAKING CHANGE: Adding extra content to notifications closes seerr-team#2104
Removed some casts from the getAvailableMediaServerUrl as they were unnecessary BREAKING CHANGE: Function updated closes seerr-team#2104
In this commit, I updated the slack button behaviour based on the suggestion made by coderabbitai. This means that now the View Media in Seerr and Play on Plex is on the same line and handled in one variable. BREAKING CHANGE: Changing the way buttons are sent in Slack notifications Closes seerr-team#2104
JackW6809
force-pushed
the
notification-media-server-links
branch
from
db87bf7 to
b2dc582
Compare
![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)
server/lib/notifications/agents/telegram.ts (1)
44-57:⚠️ Potential issue | 🟡 MinorMinor typo in documentation.
Line 45 has a double space: "agent's notification" → "agent's notification"
📝 Proposed fix
/** - * Returns this agent's notification settings. + * Returns this agent's notification settings. * Uses a cached copy when available. * `@protected` */
🧹 Nitpick comments (1)
server/utils/mediaServerHelper.ts (1)
36-40: Consider documenting the cross-resolution fallback behavior.The function falls back to the 4K URL when a non-4K request has no standard URL (and vice versa). This "show something rather than nothing" behavior is reasonable but not mentioned in the docstring. A brief note would help future maintainers understand this is intentional.
Additionally, the
|| undefinedsuffix is only necessary if URLs can be empty strings rather than justundefined. IfmediaUrlandmediaUrl4kare alwaysstring | undefined, this is redundant.
Contributor
Author
|
@coderabbitai generate docstrings |
✅ Actions performedInitiated docstring generation; will generate only if new commits exist. |
|
Note Docstrings generation - SUCCESS |
coderabbitai bot
added a commit
that referenced
this pull request
Feb 15, 2026
Docstrings generation was requested by @JackW6809. * #2392 (comment) The following files were modified: * `server/utils/mediaServerHelper.ts`
Removed a double space from JSDoc, added more into to JSDoc
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@server/lib/notifications/agents/telegram.ts`:
- Around line 73-82: The escapeText method's character class is missing the
backtick and backslash; update the regex in private escapeText(text: string |
undefined): string to include ` and \ as characters to escape, making sure the
backslash is properly escaped in the regex literal (i.e., represent a single
backslash match with an escaped backslash in the pattern). Keep the rest of the
character class and behavior the same so replace still returns '' for undefined
input.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
server/lib/notifications/agents/telegram.ts (1)
101-205:⚠️ Potential issue | 🔴 CriticalRemove local helper shadowing and fix argument mismatch (TS compile error).
Local helpers duplicate the imported
getAvailableMediaServerNameandgetAvailableMediaServerUrlbut declare them to take no parameters. Lines 200-201 invoke these local functions with arguments (mediaServerTypeandpayload), causing a TypeScript error. Remove the local helper definitions and use the imported helpers instead, which properly accept these arguments.🔧 Proposed fix
- function getAvailableMediaServerName() { - if (mediaServerType === MediaServerType.EMBY) { - return 'Emby'; - } - - if (mediaServerType === MediaServerType.PLEX) { - return 'Plex'; - } - - return 'Jellyfin'; - } - - function getAvailableMediaServerUrl(): string | undefined { - const wants4k = payload.request?.is4k; - const url4k = (payload.media as any)?.mediaUrl4k as string | undefined; - const url = (payload.media as any)?.mediaUrl as string | undefined; - - return (wants4k ? (url4k ?? url) : (url ?? url4k)) || undefined; - } - /* eslint-disable no-useless-escape */ let message = `\*${this.escapeText(
🤖 Fix all issues with AI agents
In `@server/lib/notifications/agents/email.ts`:
- Around line 78-106: The file defines mediaServerName and mediaServerUrl twice
and re-declares local helper functions that shadow imported helpers; remove the
duplicate const declarations and the local functions getAvailableMediaServerName
and getAvailableMediaServerUrl and instead call the existing imported helper
functions (use the already destructured mediaServerType and payload to call the
imported getAvailableMediaServerName/mediaServerUrl helpers or the single
imported helpers with the correct arguments), leaving only one const
mediaServerName and one const mediaServerUrl assigned via the imported helper
functions.
In `@server/lib/notifications/agents/slack.ts`:
- Around line 229-289: The second if (!payload.issue) block duplicates the "Play
on <MediaServer>" action already added to actionElements earlier; remove the
later duplicate block (the standalone if (!payload.issue) that calls
getAvailableMediaServerName() and getAvailableMediaServerUrl() with no args and
pushes another actions block) so the media server button is only created via
actionElements; keep the initial creation that uses mediaServerType and payload
and only render the actions block from actionElements when actionElements.length
> 0.
- Around line 85-108: Remove the local helper functions
getAvailableMediaServerName() and getAvailableMediaServerUrl() that shadow the
imported helpers; delete their definitions and use the imported
getAvailableMediaServerName and getAvailableMediaServerUrl instead, passing the
expected arguments (e.g., mediaServerType and payload) where they are called
(the places that currently pass mediaServerType and payload around lines that
generate the "Play on …" action), and ensure you only call them in one place to
avoid duplicating the "Play on …" action block; keep using existing symbols like
MediaServerType and payload and preserve embedPoster usage from
settings.notifications.agents.slack.
🧹 Nitpick comments (1)
server/migration/sqlite/1769971922216-CustomNotifyEmailMigration.ts (1)
7-17:user_push_subscriptiontable is recreated three times in theup()method.This is inefficient and makes the migration harder to understand:
- Lines 7-17: First recreation (removes unique constraint on endpoint+userId)
- Lines 90-99: Second recreation (same schema, appears redundant)
- Lines 162-171: Third recreation (adds back the unique constraint)
If the intent is to add/modify a unique constraint, this could be simplified to a single table recreation.
Also applies to: 90-99, 162-171
JackW6809
force-pushed
the
notification-media-server-links
branch
from
34d66e1 to
6cb90a2
Compare
Contributor
Author
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@server/lib/notifications/agents/telegram.ts`:
- Around line 80-82: The escapeText function's MarkdownV2 regex is missing
backtick and backslash; update the regex used in escapeText(text: string |
undefined) to include both the backtick (`) and the backslash (\) in the
character class so all Telegram MarkdownV2-reserved characters are escaped,
ensuring you correctly escape the backslash in the string literal/regex syntax
for the language runtime.
🧹 Nitpick comments (1)
server/lib/notifications/agents/email.ts (1)
171-174: Consider simplifying the template literal.The template literal wrapping is unnecessary since
mediaServerUrlis already a string.♻️ Suggested simplification
- mediaServerActionUrl: mediaServerUrl - ? `${mediaServerUrl}` - : undefined, + mediaServerActionUrl: mediaServerUrl,
Telegram requires escaping of ` and \ characters, currently does not. Coderabbit found this and I am implementing this change BREAKING CHANGE: Changes the regex formula for escaping characters for telegram
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In the email, Telegram, and Slack notification users receive when their media request is now available, I have added a Play on media server (e.g. Play on Plex) button that takes them directly to the media on their media server.
AI Was used to help understand parts of existing codebase so I could re-use parts in my new work.
Description
In the email notification I have added an extra button to the bottom, in my example I use Plex so for me the button is dynamically called Play on Plex. This takes you directly to the media in the users media stack. The same as the button if you were to view the media in Seerr and click Play on Plex.
It is not "required" but more a quality of life feature, it allows the user to go directly to the media in their chosen media server, eliminates an extra step of having to find it manually in the media server or in Seerr.
How Has This Been Tested?
I have tested the change on all platforms by removing existing media, requesting it, running a full library scan so Seerr picks it back up from Plex, then send an email, slack, and telegram notification to me. I click on the link in each notification individually and it successfully sends me to the media in Plex.
Ran from my Mac from a fresh Seerr dev instance, added my Plex Media Server as the Media Server and ran a full library scan so Seerr has all the latest data.
It affects Notifications but it doesn't remove any features but instead adds more useful features.
Screenshots / Logs (if applicable)
Checklist:
pnpm buildpnpm i18n:extractSummary by CodeRabbit