diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql index e206b3ea7c136..4d495fefe628e 100644 --- a/coderd/database/dump.sql +++ b/coderd/database/dump.sql @@ -113,7 +113,8 @@ CREATE TYPE notification_message_status AS ENUM ( CREATE TYPE notification_method AS ENUM ( 'smtp', - 'webhook' + 'webhook', + 'inbox' ); CREATE TYPE notification_template_kind AS ENUM ( diff --git a/coderd/database/migrations/000299_notifications_method_inbox.down.sql b/coderd/database/migrations/000299_notifications_method_inbox.down.sql new file mode 100644 index 0000000000000..d2138f05c5c3a --- /dev/null +++ b/coderd/database/migrations/000299_notifications_method_inbox.down.sql @@ -0,0 +1,3 @@ +-- The migration is about an enum value change +-- As we can not remove a value from an enum, we can let the down migration empty +-- In order to avoid any failure, we use ADD VALUE IF NOT EXISTS to add the value diff --git a/coderd/database/migrations/000299_notifications_method_inbox.up.sql b/coderd/database/migrations/000299_notifications_method_inbox.up.sql new file mode 100644 index 0000000000000..40eec69d0cf95 --- /dev/null +++ b/coderd/database/migrations/000299_notifications_method_inbox.up.sql @@ -0,0 +1 @@ +ALTER TYPE notification_method ADD VALUE IF NOT EXISTS 'inbox'; diff --git a/coderd/database/models.go b/coderd/database/models.go index 3e0f59e6e9391..07d5838b793ae 100644 --- a/coderd/database/models.go +++ b/coderd/database/models.go @@ -878,6 +878,7 @@ type NotificationMethod string const ( NotificationMethodSmtp NotificationMethod = "smtp" NotificationMethodWebhook NotificationMethod = "webhook" + NotificationMethodInbox NotificationMethod = "inbox" ) func (e *NotificationMethod) Scan(src interface{}) error { @@ -918,7 +919,8 @@ func (ns NullNotificationMethod) Value() (driver.Value, error) { func (e NotificationMethod) Valid() bool { switch e { case NotificationMethodSmtp, - NotificationMethodWebhook: + NotificationMethodWebhook, + NotificationMethodInbox: return true } return false @@ -928,6 +930,7 @@ func AllNotificationMethodValues() []NotificationMethod { return []NotificationMethod{ NotificationMethodSmtp, NotificationMethodWebhook, + NotificationMethodInbox, } } diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index a8421e62d8245..30746503fe9a2 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -3809,6 +3809,7 @@ SELECT nm.method, nm.attempt_count::int AS attempt_count, nm.queued_seconds::float AS queued_seconds, + nm.targets, -- template nt.id AS template_id, nt.title_template, @@ -3834,6 +3835,7 @@ type AcquireNotificationMessagesRow struct { Method NotificationMethod `db:"method" json:"method"` AttemptCount int32 `db:"attempt_count" json:"attempt_count"` QueuedSeconds float64 `db:"queued_seconds" json:"queued_seconds"` + Targets []uuid.UUID `db:"targets" json:"targets"` TemplateID uuid.UUID `db:"template_id" json:"template_id"` TitleTemplate string `db:"title_template" json:"title_template"` BodyTemplate string `db:"body_template" json:"body_template"` @@ -3870,6 +3872,7 @@ func (q *sqlQuerier) AcquireNotificationMessages(ctx context.Context, arg Acquir &i.Method, &i.AttemptCount, &i.QueuedSeconds, + pq.Array(&i.Targets), &i.TemplateID, &i.TitleTemplate, &i.BodyTemplate, diff --git a/coderd/database/queries/notifications.sql b/coderd/database/queries/notifications.sql index f2d1a14c3aae7..921a58379db39 100644 --- a/coderd/database/queries/notifications.sql +++ b/coderd/database/queries/notifications.sql @@ -84,6 +84,7 @@ SELECT nm.method, nm.attempt_count::int AS attempt_count, nm.queued_seconds::float AS queued_seconds, + nm.targets, -- template nt.id AS template_id, nt.title_template, diff --git a/coderd/notifications.go b/coderd/notifications.go index 812d8cd3e450b..670f3625f41bc 100644 --- a/coderd/notifications.go +++ b/coderd/notifications.go @@ -157,6 +157,11 @@ func (api *API) systemNotificationTemplates(rw http.ResponseWriter, r *http.Requ func (api *API) notificationDispatchMethods(rw http.ResponseWriter, r *http.Request) { var methods []string for _, nm := range database.AllNotificationMethodValues() { + // Skip inbox method as for now this is an implicit delivery target and should not appear + // anywhere in the Web UI. + if nm == database.NotificationMethodInbox { + continue + } methods = append(methods, string(nm)) } diff --git a/coderd/notifications/dispatch/inbox.go b/coderd/notifications/dispatch/inbox.go new file mode 100644 index 0000000000000..036424decf3c7 --- /dev/null +++ b/coderd/notifications/dispatch/inbox.go @@ -0,0 +1,81 @@ +package dispatch + +import ( + "context" + "encoding/json" + "text/template" + + "golang.org/x/xerrors" + + "cdr.dev/slog" + + "github.com/google/uuid" + + "github.com/coder/coder/v2/coderd/database" + "github.com/coder/coder/v2/coderd/database/dbtime" + "github.com/coder/coder/v2/coderd/notifications/types" + markdown "github.com/coder/coder/v2/coderd/render" +) + +type InboxStore interface { + InsertInboxNotification(ctx context.Context, arg database.InsertInboxNotificationParams) (database.InboxNotification, error) +} + +// InboxHandler is responsible for dispatching notification messages to the Coder Inbox. +type InboxHandler struct { + log slog.Logger + store InboxStore +} + +func NewInboxHandler(log slog.Logger, store InboxStore) *InboxHandler { + return &InboxHandler{log: log, store: store} +} + +func (s *InboxHandler) Dispatcher(payload types.MessagePayload, titleTmpl, bodyTmpl string, _ template.FuncMap) (DeliveryFunc, error) { + subject, err := markdown.PlaintextFromMarkdown(titleTmpl) + if err != nil { + return nil, xerrors.Errorf("render subject: %w", err) + } + + htmlBody, err := markdown.PlaintextFromMarkdown(bodyTmpl) + if err != nil { + return nil, xerrors.Errorf("render html body: %w", err) + } + + return s.dispatch(payload, subject, htmlBody), nil +} + +func (s *InboxHandler) dispatch(payload types.MessagePayload, title, body string) DeliveryFunc { + return func(ctx context.Context, msgID uuid.UUID) (bool, error) { + userID, err := uuid.Parse(payload.UserID) + if err != nil { + return false, xerrors.Errorf("parse user ID: %w", err) + } + templateID, err := uuid.Parse(payload.NotificationTemplateID) + if err != nil { + return false, xerrors.Errorf("parse template ID: %w", err) + } + + actions, err := json.Marshal(payload.Actions) + if err != nil { + return false, xerrors.Errorf("marshal actions: %w", err) + } + + // nolint:exhaustruct + _, err = s.store.InsertInboxNotification(ctx, database.InsertInboxNotificationParams{ + ID: msgID, + UserID: userID, + TemplateID: templateID, + Targets: payload.Targets, + Title: title, + Content: body, + Actions: actions, + CreatedAt: dbtime.Now(), + }) + if err != nil { + return false, xerrors.Errorf("insert inbox notification: %w", err) + } + + return false, nil + } +} diff --git a/coderd/notifications/dispatch/inbox_test.go b/coderd/notifications/dispatch/inbox_test.go new file mode 100644 index 0000000000000..72547122b2e01 --- /dev/null +++ b/coderd/notifications/dispatch/inbox_test.go @@ -0,0 +1,109 @@ +package dispatch_test + +import ( + "context" + "testing" + + "cdr.dev/slog" + "cdr.dev/slog/sloggers/slogtest" + + "github.com/google/uuid" + "github.com/stretchr/testify/require" + + "github.com/coder/coder/v2/coderd/database" + "github.com/coder/coder/v2/coderd/database/dbgen" + "github.com/coder/coder/v2/coderd/database/dbtestutil" + "github.com/coder/coder/v2/coderd/notifications" + "github.com/coder/coder/v2/coderd/notifications/dispatch" + "github.com/coder/coder/v2/coderd/notifications/types" +) + +func TestInbox(t *testing.T) { + t.Parallel() + + logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).Leveled(slog.LevelDebug) + tests := []struct { + name string + msgID uuid.UUID + payload types.MessagePayload + expectedErr string + expectedRetry bool + }{ + { + name: "OK", + msgID: uuid.New(), + payload: types.MessagePayload{ + NotificationName: "test", + NotificationTemplateID: notifications.TemplateWorkspaceDeleted.String(), + UserID: "valid", + Actions: []types.TemplateAction{ + { + Label: "View my workspace", + URL: "https://coder.com/workspaces/1", + }, + }, + }, + }, + { + name: "InvalidUserID", + payload: types.MessagePayload{ + NotificationName: "test", + NotificationTemplateID: notifications.TemplateWorkspaceDeleted.String(), + UserID: "invalid", + Actions: []types.TemplateAction{}, + }, + expectedErr: "parse user ID", + expectedRetry: false, + }, + { + name: "InvalidTemplateID", + payload: types.MessagePayload{ + NotificationName: "test", + NotificationTemplateID: "invalid", + UserID: "valid", + Actions: []types.TemplateAction{}, + }, + expectedErr: "parse template ID", + expectedRetry: false, + }, + } + + for _, tc := range tests { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + db, _ := dbtestutil.NewDB(t) + + if tc.payload.UserID == "valid" { + user := dbgen.User(t, db, database.User{}) + tc.payload.UserID = user.ID.String() + } + + ctx := context.Background() + + handler := dispatch.NewInboxHandler(logger.Named("smtp"), db) + dispatcherFunc, err := handler.Dispatcher(tc.payload, "", "", nil) + require.NoError(t, err) + + retryable, err := dispatcherFunc(ctx, tc.msgID) + + if tc.expectedErr != "" { + require.ErrorContains(t, err, tc.expectedErr) + require.Equal(t, tc.expectedRetry, retryable) + } else { + require.NoError(t, err) + require.False(t, retryable) + uid := uuid.MustParse(tc.payload.UserID) + notifs, err := db.GetInboxNotificationsByUserID(ctx, database.GetInboxNotificationsByUserIDParams{ + UserID: uid, + ReadStatus: database.InboxNotificationReadStatusAll, + }) + + require.NoError(t, err) + require.Len(t, notifs, 1) + require.Equal(t, tc.msgID, notifs[0].ID) + } + }) + } +} diff --git a/coderd/notifications/enqueuer.go b/coderd/notifications/enqueuer.go index df91efe31d003..dbcc67d1c5e70 100644 --- a/coderd/notifications/enqueuer.go +++ b/coderd/notifications/enqueuer.go @@ -53,13 +53,13 @@ func NewStoreEnqueuer(cfg codersdk.NotificationsConfig, store Store, helpers tem } // Enqueue queues a notification message for later delivery, assumes no structured input data. -func (s *StoreEnqueuer) Enqueue(ctx context.Context, userID, templateID uuid.UUID, labels map[string]string, createdBy string, targets ...uuid.UUID) (*uuid.UUID, error) { +func (s *StoreEnqueuer) Enqueue(ctx context.Context, userID, templateID uuid.UUID, labels map[string]string, createdBy string, targets ...uuid.UUID) ([]uuid.UUID, error) { return s.EnqueueWithData(ctx, userID, templateID, labels, nil, createdBy, targets...) } // Enqueue queues a notification message for later delivery. // Messages will be dequeued by a notifier later and dispatched. -func (s *StoreEnqueuer) EnqueueWithData(ctx context.Context, userID, templateID uuid.UUID, labels map[string]string, data map[string]any, createdBy string, targets ...uuid.UUID) (*uuid.UUID, error) { +func (s *StoreEnqueuer) EnqueueWithData(ctx context.Context, userID, templateID uuid.UUID, labels map[string]string, data map[string]any, createdBy string, targets ...uuid.UUID) ([]uuid.UUID, error) { metadata, err := s.store.FetchNewMessageMetadata(ctx, database.FetchNewMessageMetadataParams{ UserID: userID, NotificationTemplateID: templateID, @@ -85,40 +85,48 @@ func (s *StoreEnqueuer) EnqueueWithData(ctx context.Context, userID, templateID return nil, xerrors.Errorf("failed encoding input labels: %w", err) } - id := uuid.New() - err = s.store.EnqueueNotificationMessage(ctx, database.EnqueueNotificationMessageParams{ - ID: id, - UserID: userID, - NotificationTemplateID: templateID, - Method: dispatchMethod, - Payload: input, - Targets: targets, - CreatedBy: createdBy, - CreatedAt: dbtime.Time(s.clock.Now().UTC()), - }) - if err != nil { - // We have a trigger on the notification_messages table named `inhibit_enqueue_if_disabled` which prevents messages - // from being enqueued if the user has disabled them via notification_preferences. The trigger will fail the insertion - // with the message "cannot enqueue message: user has disabled this notification". - // - // This is more efficient than fetching the user's preferences for each enqueue, and centralizes the business logic. - if strings.Contains(err.Error(), ErrCannotEnqueueDisabledNotification.Error()) { - return nil, ErrCannotEnqueueDisabledNotification - } - - // If the enqueue fails due to a dedupe hash conflict, this means that a notification has already been enqueued - // today with identical properties. It's far simpler to prevent duplicate sends in this central manner, rather than - // having each notification enqueue handle its own logic. - if database.IsUniqueViolation(err, database.UniqueNotificationMessagesDedupeHashIndex) { - return nil, ErrDuplicate + uuids := make([]uuid.UUID, 0, 2) + // All the enqueued messages are enqueued both on the dispatch method set by the user (or default one) and the inbox. + // As the inbox is not configurable per the user and is always enabled, we always enqueue the message on the inbox. + // The logic is done here in order to have two completely separated processing and retries are handled separately. + for _, method := range []database.NotificationMethod{dispatchMethod, database.NotificationMethodInbox} { + id := uuid.New() + err = s.store.EnqueueNotificationMessage(ctx, database.EnqueueNotificationMessageParams{ + ID: id, + UserID: userID, + NotificationTemplateID: templateID, + Method: method, + Payload: input, + Targets: targets, + CreatedBy: createdBy, + CreatedAt: dbtime.Time(s.clock.Now().UTC()), + }) + if err != nil { + // We have a trigger on the notification_messages table named `inhibit_enqueue_if_disabled` which prevents messages + // from being enqueued if the user has disabled them via notification_preferences. The trigger will fail the insertion + // with the message "cannot enqueue message: user has disabled this notification". + // + // This is more efficient than fetching the user's preferences for each enqueue, and centralizes the business logic. + if strings.Contains(err.Error(), ErrCannotEnqueueDisabledNotification.Error()) { + return nil, ErrCannotEnqueueDisabledNotification + } + + // If the enqueue fails due to a dedupe hash conflict, this means that a notification has already been enqueued + // today with identical properties. It's far simpler to prevent duplicate sends in this central manner, rather than + // having each notification enqueue handle its own logic. + if database.IsUniqueViolation(err, database.UniqueNotificationMessagesDedupeHashIndex) { + return nil, ErrDuplicate + } + + s.log.Warn(ctx, "failed to enqueue notification", slog.F("template_id", templateID), slog.F("input", input), slog.Error(err)) + return nil, xerrors.Errorf("enqueue notification: %w", err) } - s.log.Warn(ctx, "failed to enqueue notification", slog.F("template_id", templateID), slog.F("input", input), slog.Error(err)) - return nil, xerrors.Errorf("enqueue notification: %w", err) + uuids = append(uuids, id) } - s.log.Debug(ctx, "enqueued notification", slog.F("msg_id", id)) - return &id, nil + s.log.Debug(ctx, "enqueued notification", slog.F("msg_ids", uuids)) + return uuids, nil } // buildPayload creates the payload that the notification will for variable substitution and/or routing. @@ -165,12 +173,12 @@ func NewNoopEnqueuer() *NoopEnqueuer { return &NoopEnqueuer{} } -func (*NoopEnqueuer) Enqueue(context.Context, uuid.UUID, uuid.UUID, map[string]string, string, ...uuid.UUID) (*uuid.UUID, error) { +func (*NoopEnqueuer) Enqueue(context.Context, uuid.UUID, uuid.UUID, map[string]string, string, ...uuid.UUID) ([]uuid.UUID, error) { // nolint:nilnil // irrelevant. return nil, nil } -func (*NoopEnqueuer) EnqueueWithData(context.Context, uuid.UUID, uuid.UUID, map[string]string, map[string]any, string, ...uuid.UUID) (*uuid.UUID, error) { +func (*NoopEnqueuer) EnqueueWithData(context.Context, uuid.UUID, uuid.UUID, map[string]string, map[string]any, string, ...uuid.UUID) ([]uuid.UUID, error) { // nolint:nilnil // irrelevant. return nil, nil } diff --git a/coderd/notifications/manager.go b/coderd/notifications/manager.go index ff516bfe5d2ec..02b4893981abf 100644 --- a/coderd/notifications/manager.go +++ b/coderd/notifications/manager.go @@ -109,7 +109,7 @@ func NewManager(cfg codersdk.NotificationsConfig, store Store, helpers template. stop: make(chan any), done: make(chan any), - handlers: defaultHandlers(cfg, log), + handlers: defaultHandlers(cfg, log, store), helpers: helpers, clock: quartz.NewReal(), @@ -121,10 +121,11 @@ func NewManager(cfg codersdk.NotificationsConfig, store Store, helpers template. } // defaultHandlers builds a set of known handlers; panics if any error occurs as these handlers should be valid at compile time. -func defaultHandlers(cfg codersdk.NotificationsConfig, log slog.Logger) map[database.NotificationMethod]Handler { +func defaultHandlers(cfg codersdk.NotificationsConfig, log slog.Logger, store Store) map[database.NotificationMethod]Handler { return map[database.NotificationMethod]Handler{ database.NotificationMethodSmtp: dispatch.NewSMTPHandler(cfg.SMTP, log.Named("dispatcher.smtp")), database.NotificationMethodWebhook: dispatch.NewWebhookHandler(cfg.Webhook, log.Named("dispatcher.webhook")), + database.NotificationMethodInbox: dispatch.NewInboxHandler(log.Named("dispatcher.inbox"), store), } } diff --git a/coderd/notifications/manager_test.go b/coderd/notifications/manager_test.go index 1897213efda70..f9f8920143e3c 100644 --- a/coderd/notifications/manager_test.go +++ b/coderd/notifications/manager_test.go @@ -38,6 +38,7 @@ func TestBufferedUpdates(t *testing.T) { interceptor := &syncInterceptor{Store: store} santa := &santaHandler{} + santaInbox := &santaHandler{} cfg := defaultNotificationsConfig(database.NotificationMethodSmtp) cfg.StoreSyncInterval = serpent.Duration(time.Hour) // Ensure we don't sync the store automatically. @@ -45,9 +46,13 @@ func TestBufferedUpdates(t *testing.T) { // GIVEN: a manager which will pass or fail notifications based on their "nice" labels mgr, err := notifications.NewManager(cfg, interceptor, defaultHelpers(), createMetrics(), logger.Named("notifications-manager")) require.NoError(t, err) - mgr.WithHandlers(map[database.NotificationMethod]notifications.Handler{ - database.NotificationMethodSmtp: santa, - }) + + handlers := map[database.NotificationMethod]notifications.Handler{ + database.NotificationMethodSmtp: santa, + database.NotificationMethodInbox: santaInbox, + } + + mgr.WithHandlers(handlers) enq, err := notifications.NewStoreEnqueuer(cfg, interceptor, defaultHelpers(), logger.Named("notifications-enqueuer"), quartz.NewReal()) require.NoError(t, err) @@ -79,7 +84,7 @@ func TestBufferedUpdates(t *testing.T) { // Wait for the expected number of buffered updates to be accumulated. require.Eventually(t, func() bool { success, failure := mgr.BufferedUpdatesCount() - return success == expectedSuccess && failure == expectedFailure + return success == expectedSuccess*len(handlers) && failure == expectedFailure*len(handlers) }, testutil.WaitShort, testutil.IntervalFast) // Stop the manager which forces an update of buffered updates. @@ -93,8 +98,8 @@ func TestBufferedUpdates(t *testing.T) { ct.FailNow() } - assert.EqualValues(ct, expectedFailure, interceptor.failed.Load()) - assert.EqualValues(ct, expectedSuccess, interceptor.sent.Load()) + assert.EqualValues(ct, expectedFailure*len(handlers), interceptor.failed.Load()) + assert.EqualValues(ct, expectedSuccess*len(handlers), interceptor.sent.Load()) }, testutil.WaitMedium, testutil.IntervalFast) } @@ -229,7 +234,7 @@ type enqueueInterceptor struct { } func newEnqueueInterceptor(db notifications.Store, metadataFn func() database.FetchNewMessageMetadataRow) *enqueueInterceptor { - return &enqueueInterceptor{Store: db, payload: make(chan types.MessagePayload, 1), metadataFn: metadataFn} + return &enqueueInterceptor{Store: db, payload: make(chan types.MessagePayload, 2), metadataFn: metadataFn} } func (e *enqueueInterceptor) EnqueueNotificationMessage(_ context.Context, arg database.EnqueueNotificationMessageParams) error { diff --git a/coderd/notifications/metrics_test.go b/coderd/notifications/metrics_test.go index a1937add18b47..2780596fb2c66 100644 --- a/coderd/notifications/metrics_test.go +++ b/coderd/notifications/metrics_test.go @@ -67,7 +67,8 @@ func TestMetrics(t *testing.T) { }) handler := &fakeHandler{} mgr.WithHandlers(map[database.NotificationMethod]notifications.Handler{ - method: handler, + method: handler, + database.NotificationMethodInbox: &fakeHandler{}, }) enq, err := notifications.NewStoreEnqueuer(cfg, store, defaultHelpers(), logger.Named("enqueuer"), quartz.NewReal()) @@ -77,7 +78,10 @@ func TestMetrics(t *testing.T) { // Build fingerprints for the two different series we expect. methodTemplateFP := fingerprintLabels(notifications.LabelMethod, string(method), notifications.LabelTemplateID, tmpl.String()) + methodTemplateFPWithInbox := fingerprintLabels(notifications.LabelMethod, string(database.NotificationMethodInbox), notifications.LabelTemplateID, tmpl.String()) + methodFP := fingerprintLabels(notifications.LabelMethod, string(method)) + methodFPWithInbox := fingerprintLabels(notifications.LabelMethod, string(database.NotificationMethodInbox)) expected := map[string]func(metric *dto.Metric, series string) bool{ "coderd_notifications_dispatch_attempts_total": func(metric *dto.Metric, series string) bool { @@ -91,7 +95,8 @@ func TestMetrics(t *testing.T) { var match string for result, val := range results { seriesFP := fingerprintLabels(notifications.LabelMethod, string(method), notifications.LabelTemplateID, tmpl.String(), notifications.LabelResult, result) - if !hasMatchingFingerprint(metric, seriesFP) { + seriesFPWithInbox := fingerprintLabels(notifications.LabelMethod, string(database.NotificationMethodInbox), notifications.LabelTemplateID, tmpl.String(), notifications.LabelResult, result) + if !hasMatchingFingerprint(metric, seriesFP) && !hasMatchingFingerprint(metric, seriesFPWithInbox) { continue } @@ -115,7 +120,7 @@ func TestMetrics(t *testing.T) { return metric.Counter.GetValue() == target }, "coderd_notifications_retry_count": func(metric *dto.Metric, series string) bool { - assert.Truef(t, hasMatchingFingerprint(metric, methodTemplateFP), "found unexpected series %q", series) + assert.Truef(t, hasMatchingFingerprint(metric, methodTemplateFP) || hasMatchingFingerprint(metric, methodTemplateFPWithInbox), "found unexpected series %q", series) if debug { t.Logf("coderd_notifications_retry_count == %v: %v", maxAttempts-1, metric.Counter.GetValue()) @@ -125,7 +130,7 @@ func TestMetrics(t *testing.T) { return metric.Counter.GetValue() == maxAttempts-1 }, "coderd_notifications_queued_seconds": func(metric *dto.Metric, series string) bool { - assert.Truef(t, hasMatchingFingerprint(metric, methodFP), "found unexpected series %q", series) + assert.Truef(t, hasMatchingFingerprint(metric, methodFP) || hasMatchingFingerprint(metric, methodFPWithInbox), "found unexpected series %q", series) if debug { t.Logf("coderd_notifications_queued_seconds > 0: %v", metric.Histogram.GetSampleSum()) @@ -140,7 +145,7 @@ func TestMetrics(t *testing.T) { return metric.Histogram.GetSampleSum() > 0 }, "coderd_notifications_dispatcher_send_seconds": func(metric *dto.Metric, series string) bool { - assert.Truef(t, hasMatchingFingerprint(metric, methodFP), "found unexpected series %q", series) + assert.Truef(t, hasMatchingFingerprint(metric, methodFP) || hasMatchingFingerprint(metric, methodFPWithInbox), "found unexpected series %q", series) if debug { t.Logf("coderd_notifications_dispatcher_send_seconds > 0: %v", metric.Histogram.GetSampleSum()) @@ -170,7 +175,7 @@ func TestMetrics(t *testing.T) { } // 1 message will exceed its maxAttempts, 1 will succeed on the first try. - return metric.Counter.GetValue() == maxAttempts+1 + return metric.Counter.GetValue() == (maxAttempts+1)*2 // *2 because we have 2 enqueuers. }, } @@ -252,8 +257,11 @@ func TestPendingUpdatesMetric(t *testing.T) { assert.NoError(t, mgr.Stop(ctx)) }) handler := &fakeHandler{} + inboxHandler := &fakeHandler{} + mgr.WithHandlers(map[database.NotificationMethod]notifications.Handler{ - method: handler, + method: handler, + database.NotificationMethodInbox: inboxHandler, }) enq, err := notifications.NewStoreEnqueuer(cfg, store, defaultHelpers(), logger.Named("enqueuer"), quartz.NewReal()) @@ -285,7 +293,7 @@ func TestPendingUpdatesMetric(t *testing.T) { }() // Both handler calls should be pending in the metrics. - require.EqualValues(t, 2, promtest.ToFloat64(metrics.PendingUpdates)) + require.EqualValues(t, 4, promtest.ToFloat64(metrics.PendingUpdates)) // THEN: // Trigger syncing updates @@ -293,13 +301,13 @@ func TestPendingUpdatesMetric(t *testing.T) { // Wait until we intercept the calls to sync the pending updates to the store. success := testutil.RequireRecvCtx(testutil.Context(t, testutil.WaitShort), t, interceptor.updateSuccess) - require.EqualValues(t, 1, success) + require.EqualValues(t, 2, success) failure := testutil.RequireRecvCtx(testutil.Context(t, testutil.WaitShort), t, interceptor.updateFailure) - require.EqualValues(t, 1, failure) + require.EqualValues(t, 2, failure) // Validate that the store synced the expected number of updates. require.Eventually(t, func() bool { - return syncer.sent.Load() == 1 && syncer.failed.Load() == 1 + return syncer.sent.Load() == 2 && syncer.failed.Load() == 2 }, testutil.WaitShort, testutil.IntervalFast) // Wait for the updates to be synced and the metric to reflect that. @@ -342,7 +350,8 @@ func TestInflightDispatchesMetric(t *testing.T) { // Barrier handler will wait until all notification messages are in-flight. barrier := newBarrierHandler(msgCount, handler) mgr.WithHandlers(map[database.NotificationMethod]notifications.Handler{ - method: barrier, + method: barrier, + database.NotificationMethodInbox: &fakeHandler{}, }) enq, err := notifications.NewStoreEnqueuer(cfg, store, defaultHelpers(), logger.Named("enqueuer"), quartz.NewReal()) @@ -378,7 +387,7 @@ func TestInflightDispatchesMetric(t *testing.T) { // Wait for the updates to be synced and the metric to reflect that. require.Eventually(t, func() bool { - return promtest.ToFloat64(metrics.InflightDispatches) == 0 + return promtest.ToFloat64(metrics.InflightDispatches.WithLabelValues(string(method), tmpl.String())) == 0 }, testutil.WaitShort, testutil.IntervalFast) } @@ -427,8 +436,9 @@ func TestCustomMethodMetricCollection(t *testing.T) { smtpHandler := &fakeHandler{} webhookHandler := &fakeHandler{} mgr.WithHandlers(map[database.NotificationMethod]notifications.Handler{ - defaultMethod: smtpHandler, - customMethod: webhookHandler, + defaultMethod: smtpHandler, + customMethod: webhookHandler, + database.NotificationMethodInbox: &fakeHandler{}, }) enq, err := notifications.NewStoreEnqueuer(cfg, store, defaultHelpers(), logger.Named("enqueuer"), quartz.NewReal()) diff --git a/coderd/notifications/notifications_test.go b/coderd/notifications/notifications_test.go index f6287993a3a91..3ef8f59228093 100644 --- a/coderd/notifications/notifications_test.go +++ b/coderd/notifications/notifications_test.go @@ -82,7 +82,10 @@ func TestBasicNotificationRoundtrip(t *testing.T) { cfg.RetryInterval = serpent.Duration(time.Hour) // Ensure retries don't interfere with the test mgr, err := notifications.NewManager(cfg, interceptor, defaultHelpers(), createMetrics(), logger.Named("manager")) require.NoError(t, err) - mgr.WithHandlers(map[database.NotificationMethod]notifications.Handler{method: handler}) + mgr.WithHandlers(map[database.NotificationMethod]notifications.Handler{ + method: handler, + database.NotificationMethodInbox: &fakeHandler{}, + }) t.Cleanup(func() { assert.NoError(t, mgr.Stop(ctx)) }) @@ -103,14 +106,14 @@ func TestBasicNotificationRoundtrip(t *testing.T) { require.Eventually(t, func() bool { handler.mu.RLock() defer handler.mu.RUnlock() - return slices.Contains(handler.succeeded, sid.String()) && - slices.Contains(handler.failed, fid.String()) + return slices.Contains(handler.succeeded, sid[0].String()) && + slices.Contains(handler.failed, fid[0].String()) }, testutil.WaitLong, testutil.IntervalFast) // THEN: we expect the store to be called with the updates of the earlier dispatches require.Eventually(t, func() bool { - return interceptor.sent.Load() == 1 && - interceptor.failed.Load() == 1 + return interceptor.sent.Load() == 2 && + interceptor.failed.Load() == 2 }, testutil.WaitLong, testutil.IntervalFast) // THEN: we verify that the store contains notifications in their expected state @@ -119,13 +122,13 @@ func TestBasicNotificationRoundtrip(t *testing.T) { Limit: 10, }) require.NoError(t, err) - require.Len(t, success, 1) + require.Len(t, success, 2) failed, err := store.GetNotificationMessagesByStatus(ctx, database.GetNotificationMessagesByStatusParams{ Status: database.NotificationMessageStatusTemporaryFailure, Limit: 10, }) require.NoError(t, err) - require.Len(t, failed, 1) + require.Len(t, failed, 2) } func TestSMTPDispatch(t *testing.T) { @@ -160,7 +163,10 @@ func TestSMTPDispatch(t *testing.T) { handler := newDispatchInterceptor(dispatch.NewSMTPHandler(cfg.SMTP, logger.Named("smtp"))) mgr, err := notifications.NewManager(cfg, store, defaultHelpers(), createMetrics(), logger.Named("manager")) require.NoError(t, err) - mgr.WithHandlers(map[database.NotificationMethod]notifications.Handler{method: handler}) + mgr.WithHandlers(map[database.NotificationMethod]notifications.Handler{ + method: handler, + database.NotificationMethodInbox: &fakeHandler{}, + }) t.Cleanup(func() { assert.NoError(t, mgr.Stop(ctx)) }) @@ -172,6 +178,7 @@ func TestSMTPDispatch(t *testing.T) { // WHEN: a message is enqueued msgID, err := enq.Enqueue(ctx, user.ID, notifications.TemplateWorkspaceDeleted, map[string]string{}, "test") require.NoError(t, err) + require.Len(t, msgID, 2) mgr.Run(ctx) @@ -187,7 +194,7 @@ func TestSMTPDispatch(t *testing.T) { require.Len(t, msgs, 1) require.Contains(t, msgs[0].MsgRequest(), fmt.Sprintf("From: %s", from)) require.Contains(t, msgs[0].MsgRequest(), fmt.Sprintf("To: %s", user.Email)) - require.Contains(t, msgs[0].MsgRequest(), fmt.Sprintf("Message-Id: %s", msgID)) + require.Contains(t, msgs[0].MsgRequest(), fmt.Sprintf("Message-Id: %s", msgID[0])) } func TestWebhookDispatch(t *testing.T) { @@ -255,7 +262,7 @@ func TestWebhookDispatch(t *testing.T) { // THEN: the webhook is received by the mock server and has the expected contents payload := testutil.RequireRecvCtx(testutil.Context(t, testutil.WaitShort), t, sent) require.EqualValues(t, "1.1", payload.Version) - require.Equal(t, *msgID, payload.MsgID) + require.Equal(t, msgID[0], payload.MsgID) require.Equal(t, payload.Payload.Labels, input) require.Equal(t, payload.Payload.UserEmail, email) // UserName is coalesced from `name` and `username`; in this case `name` wins. @@ -315,7 +322,10 @@ func TestBackpressure(t *testing.T) { mgr, err := notifications.NewManager(cfg, storeInterceptor, defaultHelpers(), createMetrics(), logger.Named("manager"), notifications.WithTestClock(mClock)) require.NoError(t, err) - mgr.WithHandlers(map[database.NotificationMethod]notifications.Handler{method: handler}) + mgr.WithHandlers(map[database.NotificationMethod]notifications.Handler{ + method: handler, + database.NotificationMethodInbox: handler, + }) enq, err := notifications.NewStoreEnqueuer(cfg, store, defaultHelpers(), logger.Named("enqueuer"), mClock) require.NoError(t, err) @@ -463,7 +473,10 @@ func TestRetries(t *testing.T) { t.Cleanup(func() { assert.NoError(t, mgr.Stop(ctx)) }) - mgr.WithHandlers(map[database.NotificationMethod]notifications.Handler{method: handler}) + mgr.WithHandlers(map[database.NotificationMethod]notifications.Handler{ + method: handler, + database.NotificationMethodInbox: &fakeHandler{}, + }) enq, err := notifications.NewStoreEnqueuer(cfg, store, defaultHelpers(), logger.Named("enqueuer"), quartz.NewReal()) require.NoError(t, err) @@ -478,11 +491,14 @@ func TestRetries(t *testing.T) { mgr.Run(ctx) - // THEN: we expect to see all but the final attempts failing + // the number of tries is equal to the number of messages times the number of attempts + // times 2 as the Enqueue method pushes into both the defined dispatch method and inbox + nbTries := msgCount * maxAttempts * 2 + + // THEN: we expect to see all but the final attempts failing on webhook, and all messages to fail on inbox require.Eventually(t, func() bool { - // We expect all messages to fail all attempts but the final; - return storeInterceptor.failed.Load() == msgCount*(maxAttempts-1) && - // ...and succeed on the final attempt. + // nolint:gosec + return storeInterceptor.failed.Load() == int32(nbTries-msgCount) && storeInterceptor.sent.Load() == msgCount }, testutil.WaitLong, testutil.IntervalFast) } @@ -533,10 +549,11 @@ func TestExpiredLeaseIsRequeued(t *testing.T) { // WHEN: a few notifications are enqueued which will all succeed var msgs []string for i := 0; i < msgCount; i++ { - id, err := enq.Enqueue(ctx, user.ID, notifications.TemplateWorkspaceDeleted, + ids, err := enq.Enqueue(ctx, user.ID, notifications.TemplateWorkspaceDeleted, map[string]string{"type": "success", "index": fmt.Sprintf("%d", i)}, "test") require.NoError(t, err) - msgs = append(msgs, id.String()) + require.Len(t, ids, 2) + msgs = append(msgs, ids[0].String(), ids[1].String()) } mgr.Run(mgrCtx) @@ -551,7 +568,7 @@ func TestExpiredLeaseIsRequeued(t *testing.T) { // Fetch any messages currently in "leased" status, and verify that they're exactly the ones we enqueued. leased, err := store.GetNotificationMessagesByStatus(ctx, database.GetNotificationMessagesByStatusParams{ Status: database.NotificationMessageStatusLeased, - Limit: msgCount, + Limit: msgCount * 2, }) require.NoError(t, err) @@ -573,7 +590,10 @@ func TestExpiredLeaseIsRequeued(t *testing.T) { handler := newDispatchInterceptor(&fakeHandler{}) mgr, err = notifications.NewManager(cfg, storeInterceptor, defaultHelpers(), createMetrics(), logger.Named("manager")) require.NoError(t, err) - mgr.WithHandlers(map[database.NotificationMethod]notifications.Handler{method: handler}) + mgr.WithHandlers(map[database.NotificationMethod]notifications.Handler{ + method: handler, + database.NotificationMethodInbox: &fakeHandler{}, + }) // Use regular context now. t.Cleanup(func() { @@ -584,7 +604,7 @@ func TestExpiredLeaseIsRequeued(t *testing.T) { // Wait until all messages are sent & updates flushed to the database. require.Eventually(t, func() bool { return handler.sent.Load() == msgCount && - storeInterceptor.sent.Load() == msgCount + storeInterceptor.sent.Load() == msgCount*2 }, testutil.WaitLong, testutil.IntervalFast) // Validate that no more messages are in "leased" status. @@ -639,7 +659,10 @@ func TestNotifierPaused(t *testing.T) { cfg.FetchInterval = serpent.Duration(fetchInterval) mgr, err := notifications.NewManager(cfg, store, defaultHelpers(), createMetrics(), logger.Named("manager")) require.NoError(t, err) - mgr.WithHandlers(map[database.NotificationMethod]notifications.Handler{method: handler}) + mgr.WithHandlers(map[database.NotificationMethod]notifications.Handler{ + method: handler, + database.NotificationMethodInbox: &fakeHandler{}, + }) t.Cleanup(func() { assert.NoError(t, mgr.Stop(ctx)) }) @@ -667,8 +690,9 @@ func TestNotifierPaused(t *testing.T) { Limit: 10, }) require.NoError(t, err) - require.Len(t, pendingMessages, 1) - require.Equal(t, pendingMessages[0].ID.String(), sid.String()) + require.Len(t, pendingMessages, 2) + require.Equal(t, pendingMessages[0].ID.String(), sid[0].String()) + require.Equal(t, pendingMessages[1].ID.String(), sid[1].String()) // Wait a few fetch intervals to be sure that no new notifications are being sent. // TODO: use quartz instead. @@ -691,7 +715,7 @@ func TestNotifierPaused(t *testing.T) { require.Eventually(t, func() bool { handler.mu.RLock() defer handler.mu.RUnlock() - return slices.Contains(handler.succeeded, sid.String()) + return slices.Contains(handler.succeeded, sid[0].String()) }, fetchInterval*5, testutil.IntervalFast) } @@ -767,6 +791,10 @@ func TestNotificationTemplates_Golden(t *testing.T) { "reason": "autodeleted due to dormancy", "initiator": "autobuild", }, + Targets: []uuid.UUID{ + uuid.MustParse("5c6ea841-ca63-46cc-9c37-78734c7a788b"), + uuid.MustParse("b8355e3a-f3c5-4dd1-b382-7eb1fae7db52"), + }, }, }, { @@ -780,6 +808,10 @@ func TestNotificationTemplates_Golden(t *testing.T) { "name": "bobby-workspace", "reason": "autostart", }, + Targets: []uuid.UUID{ + uuid.MustParse("5c6ea841-ca63-46cc-9c37-78734c7a788b"), + uuid.MustParse("b8355e3a-f3c5-4dd1-b382-7eb1fae7db52"), + }, }, }, { @@ -1298,6 +1330,7 @@ func TestNotificationTemplates_Golden(t *testing.T) { ) require.NoError(t, err) + tc.payload.Targets = append(tc.payload.Targets, user.ID) _, err = smtpEnqueuer.EnqueueWithData( ctx, user.ID, @@ -1305,7 +1338,7 @@ func TestNotificationTemplates_Golden(t *testing.T) { tc.payload.Labels, tc.payload.Data, user.Username, - user.ID, + tc.payload.Targets..., ) require.NoError(t, err) @@ -1620,8 +1653,8 @@ func TestDisabledAfterEnqueue(t *testing.T) { Limit: 10, }) assert.NoError(ct, err) - if assert.Equal(ct, len(m), 1) { - assert.Equal(ct, m[0].ID.String(), msgID.String()) + if assert.Equal(ct, len(m), 2) { + assert.Contains(ct, []string{m[0].ID.String(), m[1].ID.String()}, msgID[0].String()) assert.Contains(ct, m[0].StatusReason.String, "disabled by user") } }, testutil.WaitLong, testutil.IntervalFast, "did not find the expected inhibited message") @@ -1713,7 +1746,7 @@ func TestCustomNotificationMethod(t *testing.T) { mgr.Run(ctx) receivedMsgID := testutil.RequireRecvCtx(ctx, t, received) - require.Equal(t, msgID.String(), receivedMsgID.String()) + require.Equal(t, msgID[0].String(), receivedMsgID.String()) // Ensure no messages received by default method (SMTP): msgs := mockSMTPSrv.MessagesAndPurge() @@ -1725,7 +1758,7 @@ func TestCustomNotificationMethod(t *testing.T) { require.EventuallyWithT(t, func(ct *assert.CollectT) { msgs := mockSMTPSrv.MessagesAndPurge() if assert.Len(ct, msgs, 1) { - assert.Contains(ct, msgs[0].MsgRequest(), fmt.Sprintf("Message-Id: %s", msgID)) + assert.Contains(ct, msgs[0].MsgRequest(), fmt.Sprintf("Message-Id: %s", msgID[0])) } }, testutil.WaitLong, testutil.IntervalFast) } diff --git a/coderd/notifications/notificationstest/fake_enqueuer.go b/coderd/notifications/notificationstest/fake_enqueuer.go index b26501cf492eb..8fbc2cee25806 100644 --- a/coderd/notifications/notificationstest/fake_enqueuer.go +++ b/coderd/notifications/notificationstest/fake_enqueuer.go @@ -59,15 +59,15 @@ func (f *FakeEnqueuer) assertRBACNoLock(ctx context.Context) { } } -func (f *FakeEnqueuer) Enqueue(ctx context.Context, userID, templateID uuid.UUID, labels map[string]string, createdBy string, targets ...uuid.UUID) (*uuid.UUID, error) { +func (f *FakeEnqueuer) Enqueue(ctx context.Context, userID, templateID uuid.UUID, labels map[string]string, createdBy string, targets ...uuid.UUID) ([]uuid.UUID, error) { return f.EnqueueWithData(ctx, userID, templateID, labels, nil, createdBy, targets...) } -func (f *FakeEnqueuer) EnqueueWithData(ctx context.Context, userID, templateID uuid.UUID, labels map[string]string, data map[string]any, createdBy string, targets ...uuid.UUID) (*uuid.UUID, error) { +func (f *FakeEnqueuer) EnqueueWithData(ctx context.Context, userID, templateID uuid.UUID, labels map[string]string, data map[string]any, createdBy string, targets ...uuid.UUID) ([]uuid.UUID, error) { return f.enqueueWithDataLock(ctx, userID, templateID, labels, data, createdBy, targets...) } -func (f *FakeEnqueuer) enqueueWithDataLock(ctx context.Context, userID, templateID uuid.UUID, labels map[string]string, data map[string]any, createdBy string, targets ...uuid.UUID) (*uuid.UUID, error) { +func (f *FakeEnqueuer) enqueueWithDataLock(ctx context.Context, userID, templateID uuid.UUID, labels map[string]string, data map[string]any, createdBy string, targets ...uuid.UUID) ([]uuid.UUID, error) { f.mu.Lock() defer f.mu.Unlock() f.assertRBACNoLock(ctx) @@ -82,7 +82,7 @@ func (f *FakeEnqueuer) enqueueWithDataLock(ctx context.Context, userID, template }) id := uuid.New() - return &id, nil + return []uuid.UUID{id}, nil } func (f *FakeEnqueuer) Clear() { diff --git a/coderd/notifications/spec.go b/coderd/notifications/spec.go index 7ac40b6cae8b8..4fc3c513c4b7b 100644 --- a/coderd/notifications/spec.go +++ b/coderd/notifications/spec.go @@ -25,6 +25,8 @@ type Store interface { GetNotificationsSettings(ctx context.Context) (string, error) GetApplicationName(ctx context.Context) (string, error) GetLogoURL(ctx context.Context) (string, error) + + InsertInboxNotification(ctx context.Context, arg database.InsertInboxNotificationParams) (database.InboxNotification, error) } // Handler is responsible for preparing and delivering a notification by a given method. @@ -35,6 +37,6 @@ type Handler interface { // Enqueuer enqueues a new notification message in the store and returns its ID, should it enqueue without failure. type Enqueuer interface { - Enqueue(ctx context.Context, userID, templateID uuid.UUID, labels map[string]string, createdBy string, targets ...uuid.UUID) (*uuid.UUID, error) - EnqueueWithData(ctx context.Context, userID, templateID uuid.UUID, labels map[string]string, data map[string]any, createdBy string, targets ...uuid.UUID) (*uuid.UUID, error) + Enqueue(ctx context.Context, userID, templateID uuid.UUID, labels map[string]string, createdBy string, targets ...uuid.UUID) ([]uuid.UUID, error) + EnqueueWithData(ctx context.Context, userID, templateID uuid.UUID, labels map[string]string, data map[string]any, createdBy string, targets ...uuid.UUID) ([]uuid.UUID, error) } diff --git a/coderd/notifications/testdata/rendered-templates/webhook/TemplateTemplateDeleted.json.golden b/coderd/notifications/testdata/rendered-templates/webhook/TemplateTemplateDeleted.json.golden index 4390a3ddfb84b..d4d7b5cbf46ce 100644 --- a/coderd/notifications/testdata/rendered-templates/webhook/TemplateTemplateDeleted.json.golden +++ b/coderd/notifications/testdata/rendered-templates/webhook/TemplateTemplateDeleted.json.golden @@ -19,7 +19,8 @@ "initiator": "rob", "name": "Bobby's Template" }, - "data": null + "data": null, + "targets": null }, "title": "Template \"Bobby's Template\" deleted", "title_markdown": "Template \"Bobby's Template\" deleted", diff --git a/coderd/notifications/testdata/rendered-templates/webhook/TemplateTemplateDeprecated.json.golden b/coderd/notifications/testdata/rendered-templates/webhook/TemplateTemplateDeprecated.json.golden index c4202271c5257..053cec2c56370 100644 --- a/coderd/notifications/testdata/rendered-templates/webhook/TemplateTemplateDeprecated.json.golden +++ b/coderd/notifications/testdata/rendered-templates/webhook/TemplateTemplateDeprecated.json.golden @@ -24,7 +24,8 @@ "organization": "coder", "template": "alpha" }, - "data": null + "data": null, + "targets": null }, "title": "Template 'alpha' has been deprecated", "title_markdown": "Template 'alpha' has been deprecated", diff --git a/coderd/notifications/testdata/rendered-templates/webhook/TemplateTestNotification.json.golden b/coderd/notifications/testdata/rendered-templates/webhook/TemplateTestNotification.json.golden index a941faff134c2..e2c5744adb64b 100644 --- a/coderd/notifications/testdata/rendered-templates/webhook/TemplateTestNotification.json.golden +++ b/coderd/notifications/testdata/rendered-templates/webhook/TemplateTestNotification.json.golden @@ -16,7 +16,8 @@ } ], "labels": {}, - "data": null + "data": null, + "targets": null }, "title": "A test notification", "title_markdown": "A test notification", diff --git a/coderd/notifications/testdata/rendered-templates/webhook/TemplateUserAccountActivated.json.golden b/coderd/notifications/testdata/rendered-templates/webhook/TemplateUserAccountActivated.json.golden index 96bfdf14ecbe1..fc777758ef17d 100644 --- a/coderd/notifications/testdata/rendered-templates/webhook/TemplateUserAccountActivated.json.golden +++ b/coderd/notifications/testdata/rendered-templates/webhook/TemplateUserAccountActivated.json.golden @@ -20,7 +20,8 @@ "activated_account_user_name": "William Tables", "initiator": "rob" }, - "data": null + "data": null, + "targets": null }, "title": "User account \"bobby\" activated", "title_markdown": "User account \"bobby\" activated", diff --git a/coderd/notifications/testdata/rendered-templates/webhook/TemplateUserAccountCreated.json.golden b/coderd/notifications/testdata/rendered-templates/webhook/TemplateUserAccountCreated.json.golden index 272a5628a20a7..6408398b55a93 100644 --- a/coderd/notifications/testdata/rendered-templates/webhook/TemplateUserAccountCreated.json.golden +++ b/coderd/notifications/testdata/rendered-templates/webhook/TemplateUserAccountCreated.json.golden @@ -20,7 +20,8 @@ "created_account_user_name": "William Tables", "initiator": "rob" }, - "data": null + "data": null, + "targets": null }, "title": "User account \"bobby\" created", "title_markdown": "User account \"bobby\" created", diff --git a/coderd/notifications/testdata/rendered-templates/webhook/TemplateUserAccountDeleted.json.golden b/coderd/notifications/testdata/rendered-templates/webhook/TemplateUserAccountDeleted.json.golden index 10b7ddbca6853..71260e8e8ba8e 100644 --- a/coderd/notifications/testdata/rendered-templates/webhook/TemplateUserAccountDeleted.json.golden +++ b/coderd/notifications/testdata/rendered-templates/webhook/TemplateUserAccountDeleted.json.golden @@ -20,7 +20,8 @@ "deleted_account_user_name": "William Tables", "initiator": "rob" }, - "data": null + "data": null, + "targets": null }, "title": "User account \"bobby\" deleted", "title_markdown": "User account \"bobby\" deleted", diff --git a/coderd/notifications/testdata/rendered-templates/webhook/TemplateUserAccountSuspended.json.golden b/coderd/notifications/testdata/rendered-templates/webhook/TemplateUserAccountSuspended.json.golden index bd1dec7608974..7d5afe2642f5b 100644 --- a/coderd/notifications/testdata/rendered-templates/webhook/TemplateUserAccountSuspended.json.golden +++ b/coderd/notifications/testdata/rendered-templates/webhook/TemplateUserAccountSuspended.json.golden @@ -20,7 +20,8 @@ "suspended_account_name": "bobby", "suspended_account_user_name": "William Tables" }, - "data": null + "data": null, + "targets": null }, "title": "User account \"bobby\" suspended", "title_markdown": "User account \"bobby\" suspended", diff --git a/coderd/notifications/testdata/rendered-templates/webhook/TemplateUserRequestedOneTimePasscode.json.golden b/coderd/notifications/testdata/rendered-templates/webhook/TemplateUserRequestedOneTimePasscode.json.golden index e5f2da431f112..0d22706cd2d85 100644 --- a/coderd/notifications/testdata/rendered-templates/webhook/TemplateUserRequestedOneTimePasscode.json.golden +++ b/coderd/notifications/testdata/rendered-templates/webhook/TemplateUserRequestedOneTimePasscode.json.golden @@ -18,7 +18,8 @@ "labels": { "one_time_passcode": "00000000-0000-0000-0000-000000000000" }, - "data": null + "data": null, + "targets": null }, "title": "Reset your password for Coder", "title_markdown": "Reset your password for Coder", diff --git a/coderd/notifications/testdata/rendered-templates/webhook/TemplateWorkspaceAutoUpdated.json.golden b/coderd/notifications/testdata/rendered-templates/webhook/TemplateWorkspaceAutoUpdated.json.golden index 917904a2495aa..a6f566448efd8 100644 --- a/coderd/notifications/testdata/rendered-templates/webhook/TemplateWorkspaceAutoUpdated.json.golden +++ b/coderd/notifications/testdata/rendered-templates/webhook/TemplateWorkspaceAutoUpdated.json.golden @@ -20,7 +20,8 @@ "template_version_message": "template now includes catnip", "template_version_name": "1.0" }, - "data": null + "data": null, + "targets": null }, "title": "Workspace \"bobby-workspace\" updated automatically", "title_markdown": "Workspace \"bobby-workspace\" updated automatically", diff --git a/coderd/notifications/testdata/rendered-templates/webhook/TemplateWorkspaceAutobuildFailed.json.golden b/coderd/notifications/testdata/rendered-templates/webhook/TemplateWorkspaceAutobuildFailed.json.golden index 45b64a31a0adb..2d4c8da409f4f 100644 --- a/coderd/notifications/testdata/rendered-templates/webhook/TemplateWorkspaceAutobuildFailed.json.golden +++ b/coderd/notifications/testdata/rendered-templates/webhook/TemplateWorkspaceAutobuildFailed.json.golden @@ -19,7 +19,8 @@ "name": "bobby-workspace", "reason": "autostart" }, - "data": null + "data": null, + "targets": null }, "title": "Workspace \"bobby-workspace\" autobuild failed", "title_markdown": "Workspace \"bobby-workspace\" autobuild failed", diff --git a/coderd/notifications/testdata/rendered-templates/webhook/TemplateWorkspaceBuildsFailedReport.json.golden b/coderd/notifications/testdata/rendered-templates/webhook/TemplateWorkspaceBuildsFailedReport.json.golden index c6dabbfb89d80..bacf59837fdbf 100644 --- a/coderd/notifications/testdata/rendered-templates/webhook/TemplateWorkspaceBuildsFailedReport.json.golden +++ b/coderd/notifications/testdata/rendered-templates/webhook/TemplateWorkspaceBuildsFailedReport.json.golden @@ -57,7 +57,8 @@ } ], "total_builds": 55 - } + }, + "targets": null }, "title": "Workspace builds failed for template \"Bobby First Template\"", "title_markdown": "Workspace builds failed for template \"Bobby First Template\"", diff --git a/coderd/notifications/testdata/rendered-templates/webhook/TemplateWorkspaceCreated.json.golden b/coderd/notifications/testdata/rendered-templates/webhook/TemplateWorkspaceCreated.json.golden index 924f299b228b2..baa032fee5bae 100644 --- a/coderd/notifications/testdata/rendered-templates/webhook/TemplateWorkspaceCreated.json.golden +++ b/coderd/notifications/testdata/rendered-templates/webhook/TemplateWorkspaceCreated.json.golden @@ -20,7 +20,8 @@ "version": "alpha", "workspace": "bobby-workspace" }, - "data": null + "data": null, + "targets": null }, "title": "Workspace 'bobby-workspace' has been created", "title_markdown": "Workspace 'bobby-workspace' has been created", diff --git a/coderd/notifications/testdata/rendered-templates/webhook/TemplateWorkspaceDeleted.json.golden b/coderd/notifications/testdata/rendered-templates/webhook/TemplateWorkspaceDeleted.json.golden index 171e893dd943f..0ef7a16ae1789 100644 --- a/coderd/notifications/testdata/rendered-templates/webhook/TemplateWorkspaceDeleted.json.golden +++ b/coderd/notifications/testdata/rendered-templates/webhook/TemplateWorkspaceDeleted.json.golden @@ -24,7 +24,8 @@ "name": "bobby-workspace", "reason": "autodeleted due to dormancy" }, - "data": null + "data": null, + "targets": null }, "title": "Workspace \"bobby-workspace\" deleted", "title_markdown": "Workspace \"bobby-workspace\" deleted", diff --git a/coderd/notifications/testdata/rendered-templates/webhook/TemplateWorkspaceDeleted_CustomAppearance.json.golden b/coderd/notifications/testdata/rendered-templates/webhook/TemplateWorkspaceDeleted_CustomAppearance.json.golden index 171e893dd943f..0ef7a16ae1789 100644 --- a/coderd/notifications/testdata/rendered-templates/webhook/TemplateWorkspaceDeleted_CustomAppearance.json.golden +++ b/coderd/notifications/testdata/rendered-templates/webhook/TemplateWorkspaceDeleted_CustomAppearance.json.golden @@ -24,7 +24,8 @@ "name": "bobby-workspace", "reason": "autodeleted due to dormancy" }, - "data": null + "data": null, + "targets": null }, "title": "Workspace \"bobby-workspace\" deleted", "title_markdown": "Workspace \"bobby-workspace\" deleted", diff --git a/coderd/notifications/testdata/rendered-templates/webhook/TemplateWorkspaceDormant.json.golden b/coderd/notifications/testdata/rendered-templates/webhook/TemplateWorkspaceDormant.json.golden index 00c591d9d15d3..5e672c16578d2 100644 --- a/coderd/notifications/testdata/rendered-templates/webhook/TemplateWorkspaceDormant.json.golden +++ b/coderd/notifications/testdata/rendered-templates/webhook/TemplateWorkspaceDormant.json.golden @@ -22,7 +22,8 @@ "reason": "breached the template's threshold for inactivity", "timeTilDormant": "24 hours" }, - "data": null + "data": null, + "targets": null }, "title": "Workspace \"bobby-workspace\" marked as dormant", "title_markdown": "Workspace \"bobby-workspace\" marked as dormant", diff --git a/coderd/notifications/testdata/rendered-templates/webhook/TemplateWorkspaceManualBuildFailed.json.golden b/coderd/notifications/testdata/rendered-templates/webhook/TemplateWorkspaceManualBuildFailed.json.golden index 6b406a1928a70..e06fdb36a24d0 100644 --- a/coderd/notifications/testdata/rendered-templates/webhook/TemplateWorkspaceManualBuildFailed.json.golden +++ b/coderd/notifications/testdata/rendered-templates/webhook/TemplateWorkspaceManualBuildFailed.json.golden @@ -23,7 +23,8 @@ "workspace_build_number": "3", "workspace_owner_username": "mrbobby" }, - "data": null + "data": null, + "targets": null }, "title": "Workspace \"bobby-workspace\" manual build failed", "title_markdown": "Workspace \"bobby-workspace\" manual build failed", diff --git a/coderd/notifications/testdata/rendered-templates/webhook/TemplateWorkspaceManuallyUpdated.json.golden b/coderd/notifications/testdata/rendered-templates/webhook/TemplateWorkspaceManuallyUpdated.json.golden index 7fbda32e194f4..af80db4cf73a0 100644 --- a/coderd/notifications/testdata/rendered-templates/webhook/TemplateWorkspaceManuallyUpdated.json.golden +++ b/coderd/notifications/testdata/rendered-templates/webhook/TemplateWorkspaceManuallyUpdated.json.golden @@ -26,7 +26,8 @@ "version": "alpha", "workspace": "bobby-workspace" }, - "data": null + "data": null, + "targets": null }, "title": "Workspace 'bobby-workspace' has been manually updated", "title_markdown": "Workspace 'bobby-workspace' has been manually updated", diff --git a/coderd/notifications/testdata/rendered-templates/webhook/TemplateWorkspaceMarkedForDeletion.json.golden b/coderd/notifications/testdata/rendered-templates/webhook/TemplateWorkspaceMarkedForDeletion.json.golden index 3cb1690b0b583..2701337b344d7 100644 --- a/coderd/notifications/testdata/rendered-templates/webhook/TemplateWorkspaceMarkedForDeletion.json.golden +++ b/coderd/notifications/testdata/rendered-templates/webhook/TemplateWorkspaceMarkedForDeletion.json.golden @@ -21,7 +21,8 @@ "reason": "template updated to new dormancy policy", "timeTilDormant": "24 hours" }, - "data": null + "data": null, + "targets": null }, "title": "Workspace \"bobby-workspace\" marked for deletion", "title_markdown": "Workspace \"bobby-workspace\" marked for deletion", diff --git a/coderd/notifications/testdata/rendered-templates/webhook/TemplateWorkspaceOutOfDisk.json.golden b/coderd/notifications/testdata/rendered-templates/webhook/TemplateWorkspaceOutOfDisk.json.golden index 1bc671f52b6f9..a87d32d4b3fd1 100644 --- a/coderd/notifications/testdata/rendered-templates/webhook/TemplateWorkspaceOutOfDisk.json.golden +++ b/coderd/notifications/testdata/rendered-templates/webhook/TemplateWorkspaceOutOfDisk.json.golden @@ -25,7 +25,8 @@ "threshold": "90%" } ] - } + }, + "targets": null }, "title": "Your workspace \"bobby-workspace\" is low on volume space", "title_markdown": "Your workspace \"bobby-workspace\" is low on volume space", diff --git a/coderd/notifications/testdata/rendered-templates/webhook/TemplateWorkspaceOutOfDisk_MultipleVolumes.json.golden b/coderd/notifications/testdata/rendered-templates/webhook/TemplateWorkspaceOutOfDisk_MultipleVolumes.json.golden index c876fb1754dd1..d2d666377bed8 100644 --- a/coderd/notifications/testdata/rendered-templates/webhook/TemplateWorkspaceOutOfDisk_MultipleVolumes.json.golden +++ b/coderd/notifications/testdata/rendered-templates/webhook/TemplateWorkspaceOutOfDisk_MultipleVolumes.json.golden @@ -33,7 +33,8 @@ "threshold": "95%" } ] - } + }, + "targets": null }, "title": "Your workspace \"bobby-workspace\" is low on volume space", "title_markdown": "Your workspace \"bobby-workspace\" is low on volume space", diff --git a/coderd/notifications/testdata/rendered-templates/webhook/TemplateWorkspaceOutOfMemory.json.golden b/coderd/notifications/testdata/rendered-templates/webhook/TemplateWorkspaceOutOfMemory.json.golden index a0fce437e3c56..4787c5c256334 100644 --- a/coderd/notifications/testdata/rendered-templates/webhook/TemplateWorkspaceOutOfMemory.json.golden +++ b/coderd/notifications/testdata/rendered-templates/webhook/TemplateWorkspaceOutOfMemory.json.golden @@ -19,7 +19,8 @@ "threshold": "90%", "workspace": "bobby-workspace" }, - "data": null + "data": null, + "targets": null }, "title": "Your workspace \"bobby-workspace\" is low on memory", "title_markdown": "Your workspace \"bobby-workspace\" is low on memory", diff --git a/coderd/notifications/testdata/rendered-templates/webhook/TemplateYourAccountActivated.json.golden b/coderd/notifications/testdata/rendered-templates/webhook/TemplateYourAccountActivated.json.golden index 2e01ab7c631dc..df0681c76e7cf 100644 --- a/coderd/notifications/testdata/rendered-templates/webhook/TemplateYourAccountActivated.json.golden +++ b/coderd/notifications/testdata/rendered-templates/webhook/TemplateYourAccountActivated.json.golden @@ -19,7 +19,8 @@ "activated_account_name": "bobby", "initiator": "rob" }, - "data": null + "data": null, + "targets": null }, "title": "Your account \"bobby\" has been activated", "title_markdown": "Your account \"bobby\" has been activated", diff --git a/coderd/notifications/testdata/rendered-templates/webhook/TemplateYourAccountSuspended.json.golden b/coderd/notifications/testdata/rendered-templates/webhook/TemplateYourAccountSuspended.json.golden index 53516dbdab5ce..8bfeff26a387f 100644 --- a/coderd/notifications/testdata/rendered-templates/webhook/TemplateYourAccountSuspended.json.golden +++ b/coderd/notifications/testdata/rendered-templates/webhook/TemplateYourAccountSuspended.json.golden @@ -14,7 +14,8 @@ "initiator": "rob", "suspended_account_name": "bobby" }, - "data": null + "data": null, + "targets": null }, "title": "Your account \"bobby\" has been suspended", "title_markdown": "Your account \"bobby\" has been suspended", diff --git a/coderd/notifications/types/payload.go b/coderd/notifications/types/payload.go index dbd21c29be517..a50aaa96c6c02 100644 --- a/coderd/notifications/types/payload.go +++ b/coderd/notifications/types/payload.go @@ -1,5 +1,7 @@ package types +import "github.com/google/uuid" + // MessagePayload describes the JSON payload to be stored alongside the notification message, which specifies all of its // metadata, labels, and routing information. // @@ -18,4 +20,5 @@ type MessagePayload struct { Actions []TemplateAction `json:"actions"` Labels map[string]string `json:"labels"` Data map[string]any `json:"data"` + Targets []uuid.UUID `json:"targets"` } diff --git a/coderd/notifications_test.go b/coderd/notifications_test.go index d50464869298b..bae8b8827fe79 100644 --- a/coderd/notifications_test.go +++ b/coderd/notifications_test.go @@ -296,6 +296,9 @@ func TestNotificationDispatchMethods(t *testing.T) { var allMethods []string for _, nm := range database.AllNotificationMethodValues() { + if nm == database.NotificationMethodInbox { + continue + } allMethods = append(allMethods, string(nm)) } slices.Sort(allMethods) diff --git a/enterprise/coderd/notifications_test.go b/enterprise/coderd/notifications_test.go index b71bde86a5736..77b057bf41657 100644 --- a/enterprise/coderd/notifications_test.go +++ b/enterprise/coderd/notifications_test.go @@ -114,7 +114,7 @@ func TestUpdateNotificationTemplateMethod(t *testing.T) { require.Equal(t, "Invalid request to update notification template method", sdkError.Response.Message) require.Len(t, sdkError.Response.Validations, 1) require.Equal(t, "method", sdkError.Response.Validations[0].Field) - require.Equal(t, fmt.Sprintf("%q is not a valid method; smtp, webhook are the available options", method), sdkError.Response.Validations[0].Detail) + require.Equal(t, fmt.Sprintf("%q is not a valid method; smtp, webhook, inbox are the available options", method), sdkError.Response.Validations[0].Detail) }) t.Run("Not modified", func(t *testing.T) {