Skip to content

Commit 02123ae

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

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
@@ -69,6 +72,10 @@ func (s *StoreEnqueuer) Enqueue(ctx context.Context, userID, templateID uuid.UUI
6972
CreatedBy: createdBy,
7073
})
7174
if err != nil {
75+
if strings.Contains(err.Error(), ErrCannotEnqueueDisabledNotification.Error()) {
76+
return nil, ErrCannotEnqueueDisabledNotification
77+
}
78+
7279
s.log.Warn(ctx, "failed to enqueue notification", slog.F("template_id", templateID), slog.F("input", input), slog.Error(err))
7380
return nil, xerrors.Errorf("enqueue notification: %w", err)
7481
}

‎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 TestNotifcationTemplatesBody(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
type fakeHandler struct {
709797
mu sync.RWMutex
710798
succeeded, failed []string

‎coderd/notifications/notifier.go

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

145145
var eg errgroup.Group
146146
for _, msg := range msgs {
147+
148+
// If a notification template has been disabled by the user after a notification was enqueued, mark it as inhibited
149+
if msg.Disabled {
150+
failure <- n.newInhibitedDispatch(msg)
151+
continue
152+
}
153+
147154
// A message failing to be prepared correctly should not affect other messages.
148155
deliverFn, err := n.prepare(ctx, msg)
149156
if err != nil {
@@ -312,6 +319,16 @@ func (n *notifier) newFailedDispatch(msg database.AcquireNotificationMessagesRow
312319
}
313320
}
314321

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

0 commit comments

Comments
 (0)