feat: Adding notifications for Discord - #285
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds Discord webhook notifications to Jellysweep, allowing cleanup notifications to be sent to a Discord channel listing all media items marked for deletion with user mentions. It also enriches the user data model with Jellyseerr notification settings and adds a notify CLI command for testing notifications (Discord, email, ntfy).
Changes:
- New Discord notification client and webhook payload builder with embed splitting for Discord's size limits, plus configuration and environment variable support
- Enhanced
RequestedByfrom a plainstringto a*jellyseerr.RequestInfopointer across the engine, pulling richer user data (including Discord ID) from Jellyseerr's notification settings API - New
jellysweep notify {discord,email,ntfy}CLI commands for sending test notifications with dummy data
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
internal/notify/discord/discord.go |
New Discord webhook client with send/dry-run/marshal logic |
internal/notify/discord/constructor.go |
Builds Discord embed payloads, handles splitting across messages |
internal/engine/notification.go |
Integrates Discord notifications into the engine's cleanup flow |
internal/engine/engine.go |
Adds Discord client, allMediaItems tracking, and updated RequestedBy handling |
internal/engine/arr/arr.go |
Changes RequestedBy from string to *jellyseerr.RequestInfo |
internal/engine/requester.go |
Fetches and caches user notification settings from Jellyseerr |
pkg/jellyseerr/client.go |
New GetUserNotificationSettings API, RequesterNotificationSettings type, UserID in RequestInfo |
pkg/jellyseerr/client_test.go |
Test for GetUserNotificationSettings |
internal/config/config.go |
DiscordConfig struct, defaults, env bindings, and validation |
cmd/notify.go |
New notify discord/email/ntfy subcommands for testing |
README.md |
Documentation for Discord config, env vars, and notify commands |
馃挕 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| defer resp.Body.Close() //nolint:errcheck | ||
|
|
||
| if resp.StatusCode < 200 || resp.StatusCode >= 300 { | ||
| return fmt.Errorf("discord: webhook %d returned non-2xx status: %s", i+1, resp.Status) | ||
| } |
There was a problem hiding this comment.
I'm not so worried about this. I only added multiple payloads to be safe, you'd need over ~250 items before it sends multiple payloads. It shouldn't reach the point where it will have so many payloads that it would cause an issue
|
@jon4hz - I believe this is now up to date with your latest changes |
Background
My Jellyfin/Jellyseerr setup is basically a shared server with me and some friends. We use the Jellyseerr Discord notifications to a shared server so we are all updated about new media/requests so I wanted to mimic that as much as possible.
Changes
Adding Discord Notifications / Config
Adding the needed config to send notifications to Discord, I've tried to mimic Jellyseerr's settings as much as possible. This is how it looks in Discord:
This is different to how email notifications work where it only notifies the requestor. Instead it sends a notification to the server listing every media item that will be removed and says who requested it. If the user has added their Discord ID in their user notification settings in Jellyseerr, it will instead tag the Discord user.
Pull richer notification data from Jellyseerr
To enable the Discord notifications to tag users, we need to pull more user data from Jellyseerr. I've added a new API call whilst populating requestor info and
RequestInfowill now contain more rich information about the requestor. This contains additional information like Telegram/Pushover/PushBullet user notification settings which could be used in a future enhancement, but only Discord settings are used for now.When the data is stored in the database, it will only store the email address.
Add command to test notifications
I just wanted a way to test the look of the notification as I was making the feature but I thought it was helpful so I've expanded it to handle email/ntfy as well.
You can now use:
To send a test notification using your config file (but using some fake data)
For email, you need to specify an email address to send to:
fixes #104
Let me know if you have any thoughts / would like any changes. Where possible, I've tried to touch as little of the current functionality as possible to keep it low risk. I've tested it and seems to be working well for me.