feat(coderd): add new dispatch logic for coder inbox (coder#16764)

This PR is [resolving the dispatch part of Coder
Inbocx](coder/internal#403).

Since the DB layer has been merged - we now want to insert notifications
into Coder Inbox in parallel of the other delivery target.

To do so, we push two messages instead of one using the `Enqueue`
method.

Author: defelmnq

coderd/database/dump.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

coderd/database/migrations/000299_notifications_method_inbox.down.sql

@@ -0,0 +1 @@
+ALTER TYPE notification_method ADD VALUE IF NOT EXISTS 'inbox';

coderd/database/migrations/000299_notifications_method_inbox.up.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,

coderd/database/models.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))
 	}
 

coderd/database/queries.sql.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
+	}
+}

coderd/database/queries/notifications.sql

@@ -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)
+			}
+		})
+	}
+}

coderd/notifications.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
 }