feat: add notification preferences business logic & APIs #14117
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
This stack of pull requests is managed by Graphite. Learn more about stacking. Join @dannykopping and the rest of your teammates on |
|
Meticulous was unable to execute a test run for this PR because the most recent commit is associated with multiple PRs. To execute a test run, please try pushing up a new commit that is only associated with this PR. Last updated for commit 11eacb8. This comment will update as new commits are pushed. |
4a0e0f2 to
8041cc7
Compare
6e027ef to
01e7b50
Compare
30f3525 to
62f6dec
Compare
01e7b50 to
0aef037
Compare
88b9796 to
ce7f0c3
Compare
fe8aa2d to
3516354
Compare
0386dab to
8bec5b1
Compare
8bec5b1 to
f2926a3
Compare
3516354 to
2dd0b09
Compare
b3679fe to
d627fef
Compare
Signed-off-by: Danny Kopping <danny@coder.com>
Signed-off-by: Danny Kopping <danny@coder.com>
Signed-off-by: Danny Kopping <danny@coder.com>
Signed-off-by: Danny Kopping <danny@coder.com>
Test fix Signed-off-by: Danny Kopping <danny@coder.com>
Signed-off-by: Danny Kopping <danny@coder.com>
Signed-off-by: Danny Kopping <danny@coder.com>
2dd0b09 to
7bb1ee0
Compare
johnstcn
reviewed
Aug 5, 2024
| input := database.UpdateUserNotificationPreferencesParams{ | ||
| UserID: user.ID, | ||
| NotificationTemplateIds: make([]uuid.UUID, 0, len(prefs.TemplateDisabledMap)), | ||
| Disableds: make([]bool, 0, len(prefs.TemplateDisabledMap)), |
There was a problem hiding this comment.
Suggest setting Enabled in the external API instead.
There was a problem hiding this comment.
Yeah I vacillated on this quite a bit.
I think this is more correct though because the feature is to disable notifications, and this happens to line up with the database design as well.
johnstcn
reviewed
Aug 5, 2024
johnstcn
approved these changes
Aug 5, 2024
33e9bc9
into
dk/notification-prefs/admin-template-method
31 checks passed
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.
Part of coder/internal#19
Users should be empowered to control which notifications they do not wish to receive.
Added two new routes:
GET /users/{users}/notifications/preferencesPUT /users/{users}/notifications/preferencesWhen a user has disabled a notification template in their preferences, we need to ensure that:
notifications.ErrCannotEnqueueDisabledNotificationerrorThe database logic was implemented in #14100, see the
inhibit_enqueue_if_disabledtrigger.