Skip to content

Commit bdd9263

Browse files
dannykoppingdannykopping
committed
Test database business logic
Signed-off-by: Danny Kopping <danny@coder.com>
1 parent 62f6dec commit bdd9263

File tree

4 files changed

+125
-3
lines changed

4 files changed

+125
-3
lines changed

‎coderd/notifications/enqueuer.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package notifications
33
import (
44
"context"
55
"encoding/json"
6+
"strings"
67
"text/template"
78

89
"github.com/google/uuid"
@@ -16,6 +17,8 @@ import (
1617
"github.com/coder/coder/v2/codersdk"
1718
)
1819

20+
var ErrCannotEnqueueDisabledNotification = xerrors.New("user has disabled this notification")
21+
1922
type StoreEnqueuer struct {
2023
store Store
2124
log slog.Logger
@@ -80,6 +83,10 @@ func (s *StoreEnqueuer) Enqueue(ctx context.Context, userID, templateID uuid.UUI
8083
CreatedBy: createdBy,
8184
})
8285
if err != nil {
86+
if strings.Contains(err.Error(), ErrCannotEnqueueDisabledNotification.Error()) {
87+
return nil, ErrCannotEnqueueDisabledNotification
88+
}
89+
8390
s.log.Warn(ctx, "failed to enqueue notification", slog.F("template_id", templateID), slog.F("input", input), slog.Error(err))
8491
return nil, xerrors.Errorf("enqueue notification: %w", err)
8592
}

‎coderd/notifications/manager.go

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -249,15 +249,24 @@ func (m *Manager) syncUpdates(ctx context.Context) {
249249
for i := 0; i < nFailure; i++ {
250250
res := <-m.failure
251251

252-
status := database.NotificationMessageStatusPermanentFailure
253-
if res.retryable {
252+
var (
253+
reason string
254+
status database.NotificationMessageStatus
255+
)
256+
257+
switch {
258+
case res.retryable:
254259
status = database.NotificationMessageStatusTemporaryFailure
260+
case res.inhibited:
261+
status = database.NotificationMessageStatusInhibited
262+
reason = "disabled by user"
263+
default:
264+
status = database.NotificationMessageStatusPermanentFailure
255265
}
256266

257267
failureParams.IDs = append(failureParams.IDs, res.msg)
258268
failureParams.FailedAts = append(failureParams.FailedAts, res.ts)
259269
failureParams.Statuses = append(failureParams.Statuses, status)
260-
var reason string
261270
if res.err != nil {
262271
reason = res.err.Error()
263272
}
@@ -367,4 +376,5 @@ type dispatchResult struct {
367376
ts time.Time
368377
err error
369378
retryable bool
379+
inhibited bool
370380
}

‎coderd/notifications/notifications_test.go

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -705,6 +705,94 @@ func TestNotificationTemplatesBody(t *testing.T) {
705705
}
706706
}
707707

708+
// TestDisabledBeforeEnqueue ensures that notifications cannot be enqueued once a user has disabled that notification template
709+
func TestDisabledBeforeEnqueue(t *testing.T) {
710+
t.Parallel()
711+
712+
// SETUP
713+
if !dbtestutil.WillUsePostgres() {
714+
t.Skip("This test requires postgres; it is testing business-logic implemented in the database")
715+
}
716+
717+
ctx, logger, db := setup(t)
718+
719+
// GIVEN: an enqueuer & a sample user
720+
cfg := defaultNotificationsConfig(database.NotificationMethodSmtp)
721+
enq, err := notifications.NewStoreEnqueuer(cfg, db, defaultHelpers(), logger.Named("enqueuer"))
722+
require.NoError(t, err)
723+
user := createSampleUser(t, db)
724+
725+
// WHEN: the user has a preference set to not receive the "workspace deleted" notification
726+
templateId := notifications.TemplateWorkspaceDeleted
727+
n, err := db.UpdateUserNotificationPreferences(ctx, database.UpdateUserNotificationPreferencesParams{
728+
UserID: user.ID,
729+
NotificationTemplateIds: []uuid.UUID{templateId},
730+
Disableds: []bool{true},
731+
})
732+
require.NoError(t, err, "failed to set preferences")
733+
require.EqualValues(t, 1, n, "unexpected number of affected rows")
734+
735+
// THEN: enqueuing the "workspace deleted" notification should fail with an error
736+
_, err = enq.Enqueue(ctx, user.ID, templateId, map[string]string{}, "test")
737+
require.ErrorIs(t, err, notifications.ErrCannotEnqueueDisabledNotification, "enqueueing did not fail with expected error")
738+
}
739+
740+
// TestDisabledAfterEnqueue ensures that notifications enqueued before a notification template was disabled will not be
741+
// sent, and will instead be marked as "inhibited".
742+
func TestDisabledAfterEnqueue(t *testing.T) {
743+
t.Parallel()
744+
745+
// SETUP
746+
if !dbtestutil.WillUsePostgres() {
747+
t.Skip("This test requires postgres; it is testing business-logic implemented in the database")
748+
}
749+
750+
ctx, logger, db := setup(t)
751+
752+
method := database.NotificationMethodSmtp
753+
cfg := defaultNotificationsConfig(method)
754+
755+
mgr, err := notifications.NewManager(cfg, db, createMetrics(), logger.Named("manager"))
756+
require.NoError(t, err)
757+
t.Cleanup(func() {
758+
assert.NoError(t, mgr.Stop(ctx))
759+
})
760+
761+
enq, err := notifications.NewStoreEnqueuer(cfg, db, defaultHelpers(), logger.Named("enqueuer"))
762+
require.NoError(t, err)
763+
user := createSampleUser(t, db)
764+
765+
// GIVEN: a notification is enqueued which has not (yet) been disabled
766+
templateId := notifications.TemplateWorkspaceDeleted
767+
msgId, err := enq.Enqueue(ctx, user.ID, templateId, map[string]string{}, "test")
768+
require.NoError(t, err)
769+
770+
// Disable the notification template.
771+
n, err := db.UpdateUserNotificationPreferences(ctx, database.UpdateUserNotificationPreferencesParams{
772+
UserID: user.ID,
773+
NotificationTemplateIds: []uuid.UUID{templateId},
774+
Disableds: []bool{true},
775+
})
776+
require.NoError(t, err, "failed to set preferences")
777+
require.EqualValues(t, 1, n, "unexpected number of affected rows")
778+
779+
// WHEN: running the manager to trigger dequeueing of (now-disabled) messages
780+
mgr.Run(ctx)
781+
782+
// THEN: the message should not be sent, and must be set to "inhibited"
783+
require.EventuallyWithT(t, func(ct *assert.CollectT) {
784+
m, err := db.GetNotificationMessagesByStatus(ctx, database.GetNotificationMessagesByStatusParams{
785+
Status: database.NotificationMessageStatusInhibited,
786+
Limit: 10,
787+
})
788+
assert.NoError(ct, err)
789+
if assert.Equal(ct, len(m), 1) {
790+
assert.Equal(ct, m[0].ID.String(), msgId.String())
791+
assert.Contains(ct, m[0].StatusReason.String, "disabled by user")
792+
}
793+
}, testutil.WaitLong, testutil.IntervalFast, "did not find the expected inhibited message")
794+
}
795+
708796
func TestCustomNotificationMethod(t *testing.T) {
709797
t.Parallel()
710798

‎coderd/notifications/notifier.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,13 @@ func (n *notifier) process(ctx context.Context, success chan<- dispatchResult, f
142142

143143
var eg errgroup.Group
144144
for _, msg := range msgs {
145+
146+
// If a notification template has been disabled by the user after a notification was enqueued, mark it as inhibited
147+
if msg.Disabled {
148+
failure <- n.newInhibitedDispatch(msg)
149+
continue
150+
}
151+
145152
// A message failing to be prepared correctly should not affect other messages.
146153
deliverFn, err := n.prepare(ctx, msg)
147154
if err != nil {
@@ -310,6 +317,16 @@ func (n *notifier) newFailedDispatch(msg database.AcquireNotificationMessagesRow
310317
}
311318
}
312319

320+
func (n *notifier) newInhibitedDispatch(msg database.AcquireNotificationMessagesRow) dispatchResult {
321+
return dispatchResult{
322+
notifier: n.id,
323+
msg: msg.ID,
324+
ts: time.Now(),
325+
retryable: false,
326+
inhibited: true,
327+
}
328+
}
329+
313330
// stop stops the notifier from processing any new notifications.
314331
// This is a graceful stop, so any in-flight notifications will be completed before the notifier stops.
315332
// Once a notifier has stopped, it cannot be restarted.

0 commit comments

Comments
 (0)