-
Notifications
You must be signed in to change notification settings - Fork 331
[DO NOT MERGE] Email notification system #4338
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Conversation
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
Have fully removed the old email functionality. Waiting on approval for current email messages + prod deployments of email templates on modrinth.com/mail before merge. |
Gaming32
requested changes
Sep 12, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Quite a bit of feedback on this one, though granted some of them are smaller and more nitpicky.
apps/labrinth/migrations/20250902133943_notification-extension.sql
Outdated
Show resolved
Hide resolved
Co-authored-by: Josiah Glosson <soujournme@gmail.com> Signed-off-by: François-Xavier Talbot <108630700+fetchfern@users.noreply.github.com>
Co-authored-by: Josiah Glosson <soujournme@gmail.com> Signed-off-by: François-Xavier Talbot <108630700+fetchfern@users.noreply.github.com>
Co-authored-by: Josiah Glosson <soujournme@gmail.com> Signed-off-by: François-Xavier Talbot <108630700+fetchfern@users.noreply.github.com>
….sql Co-authored-by: Josiah Glosson <soujournme@gmail.com> Signed-off-by: François-Xavier Talbot <108630700+fetchfern@users.noreply.github.com>
…ces_item.rs Co-authored-by: Josiah Glosson <soujournme@gmail.com> Signed-off-by: François-Xavier Talbot <108630700+fetchfern@users.noreply.github.com>
Gaming32
approved these changes
Sep 15, 2025
This reverts commit e0d6614.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This adds an email notification system with user preferences, email templates and an email delivery queue, extending the existing
notifications
table & functionality. It's extensible to multiple delivery channels (now there's just'email'
).notifications_deliveries
: the actual queue. Values are inserted in the same transaction as the insert into notifications (outbox pattern). A poll-based queue worker processes item in the queue with support for concurrent workers and delivery priorities.Because all emails are implemented via notifications this requires every email type to have a matching notification type. Since we don't want to show all these new notifications types on the website, the
notifications_types
table has aexpose_in_site_notifications
column.Templates are housed in
notifications_templates
, have a URL to fetch the HTML body from (so that the frontend team can use vue-email and write email templates within the frontend) and a plain-text fallback.There's a route for other services to issue notifications as well—
POST /_internal/external_notifications
. It is guarded byX-External-Notification-Key
/LABRINTH_EXTERNAL_NOTIFICATION_KEY
. The situation with authentication keys on Labrinth is getting quite messy, lots of random keys specified via environment variables. Perhaps worth looking into storing encrypted keys with specific scopes in the database at some point?Further PRs will add:
Hooking current emails into this system.Actually will entirely remove the previoussend_email
function and move them to notifications within this PR.Routes for the servers backend to issue notifications.Also done in this PR