feat: add notification deduplication trigger #14172
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
e571901 to
43ea55e
Compare
dannykopping
commented
Aug 5, 2024
| @@ -13,14 +13,15 @@ WHERE nt.id = @notification_template_id | |||
| AND u.id = @user_id; | |||
|
|
|||
| -- name: EnqueueNotificationMessage :exec | |||
| INSERT INTO notification_messages (id, notification_template_id, user_id, method, payload, targets, created_by) | |||
| INSERT INTO notification_messages (id, notification_template_id, user_id, method, payload, targets, created_by, created_at) | |||
There was a problem hiding this comment.
I added created_at to be passed in to make the dedupe hash testable by using Quartz to advance the clock.
Signed-off-by: Danny Kopping <danny@coder.com>
43ea55e to
675aa8f
Compare
johnstcn
reviewed
Aug 5, 2024
mafredri
approved these changes
Aug 6, 2024
johnstcn
approved these changes
Aug 21, 2024
Signed-off-by: Danny Kopping <danny@coder.com>
mafredri
approved these changes
Aug 21, 2024
| @@ -13,6 +13,8 @@ import ( | |||
| "github.com/stretchr/testify/assert" | |||
| "github.com/stretchr/testify/require" | |||
|
|
|||
| "github.com/coder/quartz" | |||
|
|
|||
| "github.com/coder/serpent" | |||
There was a problem hiding this comment.
Another instance of import grouping, this is the job of the tooling tbh (and it's failing), so up to you if you want to fix it 😄. (There are actually a few more of these too but I'll leave those uncommented.)
There was a problem hiding this comment.
I'll leave this for now; we need the tooling to be updated as you suggest.
Thanks for pointing this out though
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Closes coder/internal#26
Maintains a deduplication hash for each
notification_messageentry on insert/update to prevent identical notifications from being enqueued on the same day.I plumbed in
coder/quartzto help manipulate time to make theTestNotificationDuplicatessimple and readable.