feat(notifications): change helpers logic to be defined in the Dispat…

…cher function
coder · defelmnq · Oct 22, 2024 · Oct 2, 2024 · Oct 3, 2024 · Oct 3, 2024
commit 34d661187959b1cc13aef5d92cda88206b047da4
diff --git a/coderd/notifications/dispatch/smtp.go b/coderd/notifications/dispatch/smtp.go
@@ -55,15 +55,13 @@ type SMTPHandler struct {
 
 	noAuthWarnOnce sync.Once
 	loginWarnOnce  sync.Once
-
-	helpers template.FuncMap
 }
 
-func NewSMTPHandler(cfg codersdk.NotificationsEmailConfig, helpers template.FuncMap, log slog.Logger) *SMTPHandler {
-	return &SMTPHandler{cfg: cfg, helpers: helpers, log: log}
+func NewSMTPHandler(cfg codersdk.NotificationsEmailConfig, log slog.Logger) *SMTPHandler {
+	return &SMTPHandler{cfg: cfg, log: log}
 }
 
-func (s *SMTPHandler) Dispatcher(payload types.MessagePayload, titleTmpl, bodyTmpl string) (DeliveryFunc, error) {
+func (s *SMTPHandler) Dispatcher(helpers template.FuncMap, payload types.MessagePayload, titleTmpl, bodyTmpl string) (DeliveryFunc, error) {
 	// First render the subject & body into their own discrete strings.
 	subject, err := markdown.PlaintextFromMarkdown(titleTmpl)
 	if err != nil {
@@ -79,12 +77,12 @@ func (s *SMTPHandler) Dispatcher(payload types.MessagePayload, titleTmpl, bodyTm
 	// Then, reuse these strings in the HTML & plain body templates.
 	payload.Labels["_subject"] = subject
 	payload.Labels["_body"] = htmlBody
-	htmlBody, err = render.GoTemplate(htmlTemplate, payload, s.helpers)
+	htmlBody, err = render.GoTemplate(htmlTemplate, payload, helpers)
 	if err != nil {
 		return nil, xerrors.Errorf("render full html template: %w", err)
 	}
 	payload.Labels["_body"] = plainBody
-	plainBody, err = render.GoTemplate(plainTemplate, payload, s.helpers)
+	plainBody, err = render.GoTemplate(plainTemplate, payload, helpers)
 	if err != nil {
 		return nil, xerrors.Errorf("render full plaintext template: %w", err)
 	}

diff --git a/coderd/notifications/dispatch/smtp_test.go b/coderd/notifications/dispatch/smtp_test.go
@@ -442,13 +442,7 @@ func TestSMTP(t *testing.T) {
 			require.NoError(t, hp.Set(listen.Addr().String()))
 			tc.cfg.Smarthost = hp
 
-			helpers := map[string]any{
-				"base_url":     func() string { return "http://test.com" },
-				"current_year": func() string { return "2024" },
-				"logo_url":     func() string { return "https://coder.com/coder-logo-horizontal.png" },
-				"app_name":     func() string { return "Coder" },
-			}
-			handler := dispatch.NewSMTPHandler(tc.cfg, helpers, logger.Named("smtp"))
+			handler := dispatch.NewSMTPHandler(tc.cfg, logger.Named("smtp"))
 
 			// Start mock SMTP server in the background.
 			var wg sync.WaitGroup
@@ -486,7 +480,7 @@ func TestSMTP(t *testing.T) {
 				Labels:    make(map[string]string),
 			}
 
-			dispatchFn, err := handler.Dispatcher(payload, subject, body)
+			dispatchFn, err := handler.Dispatcher(helpers, payload, subject, body)
 			require.NoError(t, err)
 
 			msgID := uuid.New()

diff --git a/coderd/notifications/dispatch/utils_test.go b/coderd/notifications/dispatch/utils_test.go
@@ -0,0 +1,10 @@
+package dispatch_test
+
+var (
+	helpers = map[string]any{
+		"base_url":     func() string { return "http://test.com" },
+		"current_year": func() string { return "2024" },
+		"logo_url":     func() string { return "https://coder.com/coder-logo-horizontal.png" },
+		"app_name":     func() string { return "Coder" },
+	}
+)
diff --git a/coderd/notifications/dispatch/webhook.go b/coderd/notifications/dispatch/webhook.go
@@ -7,6 +7,7 @@
 	"errors"
 	"io"
 	"net/http"
+	"text/template"
 
 	"github.com/google/uuid"
 	"golang.org/x/xerrors"
@@ -41,7 +42,7 @@
 	return &WebhookHandler{cfg: cfg, log: log, cl: &http.Client{}}
 }
 
-func (w *WebhookHandler) Dispatcher(payload types.MessagePayload, titleMarkdown, bodyMarkdown string) (DeliveryFunc, error) {
+func (w *WebhookHandler) Dispatcher(helpers template.FuncMap, payload types.MessagePayload, titleMarkdown, bodyMarkdown string) (DeliveryFunc, error) {
 	if w.cfg.Endpoint.String() == "" {
 		return nil, xerrors.New("webhook endpoint not defined")
 	}

diff --git a/coderd/notifications/dispatch/webhook_test.go b/coderd/notifications/dispatch/webhook_test.go
@@ -141,7 +141,7 @@ func TestWebhook(t *testing.T) {
 				Endpoint: *serpent.URLOf(endpoint),
 			}
 			handler := dispatch.NewWebhookHandler(cfg, logger.With(slog.F("test", tc.name)))
-			deliveryFn, err := handler.Dispatcher(msgPayload, titleMarkdown, bodyMarkdown)
+			deliveryFn, err := handler.Dispatcher(helpers, msgPayload, titleMarkdown, bodyMarkdown)
 			require.NoError(t, err)
 
 			retryable, err := deliveryFn(ctx, msgID)

diff --git a/coderd/notifications/fetcher.go b/coderd/notifications/fetcher.go
@@ -8,7 +8,7 @@ import (
 	"golang.org/x/xerrors"
 )
 
-func (n *notifier) FetchAppName(ctx context.Context) (string, error) {
+func (n *notifier) fetchAppName(ctx context.Context) (string, error) {
 	appName, err := n.store.GetApplicationName(ctx)
 	if err != nil {
 		if errors.Is(err, sql.ErrNoRows) {
@@ -19,7 +19,7 @@ func (n *notifier) FetchAppName(ctx context.Context) (string, error) {
 	return appName, nil
 }
 
-func (n *notifier) FetchLogoURL(ctx context.Context) (string, error) {
+func (n *notifier) fetchLogoURL(ctx context.Context) (string, error) {
 	logoURL, err := n.store.GetLogoURL(ctx)
 	if err != nil {
 		if errors.Is(err, sql.ErrNoRows) {

diff --git 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, helpers, log),
+		handlers: defaultHandlers(cfg, log),
 		helpers:  helpers,
 
 		clock: quartz.NewReal(),
@@ -121,9 +121,9 @@ 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, helpers template.FuncMap, log slog.Logger) map[database.NotificationMethod]Handler {
+func defaultHandlers(cfg codersdk.NotificationsConfig, log slog.Logger) map[database.NotificationMethod]Handler {
 	return map[database.NotificationMethod]Handler{
-		database.NotificationMethodSmtp:    dispatch.NewSMTPHandler(cfg.SMTP, helpers, log.Named("dispatcher.smtp")),
+		database.NotificationMethodSmtp:    dispatch.NewSMTPHandler(cfg.SMTP, log.Named("dispatcher.smtp")),
 		database.NotificationMethodWebhook: dispatch.NewWebhookHandler(cfg.Webhook, log.Named("dispatcher.webhook")),
 	}
 }

diff --git a/coderd/notifications/manager_test.go b/coderd/notifications/manager_test.go
@@ -5,6 +5,7 @@
 	"encoding/json"
 	"sync/atomic"
 	"testing"
+	"text/template"
 	"time"
 
 	"github.com/google/uuid"
@@ -205,7 +206,7 @@
 	nice    atomic.Int32
 }
 
-func (s *santaHandler) Dispatcher(payload types.MessagePayload, _, _ string) (dispatch.DeliveryFunc, error) {
+func (s *santaHandler) Dispatcher(helpers template.FuncMap, payload types.MessagePayload, _, _ string) (dispatch.DeliveryFunc, error) {
 	return func(ctx context.Context, msgID uuid.UUID) (retryable bool, err error) {
 		if payload.Labels["nice"] != "true" {
 			s.naughty.Add(1)

diff --git a/coderd/notifications/metrics_test.go b/coderd/notifications/metrics_test.go
@@ -5,6 +5,7 @@
 	"strconv"
 	"sync"
 	"testing"
+	"text/template"
 	"time"
 
 	"github.com/google/uuid"
@@ -43,7 +44,7 @@

 	reg := prometheus.NewRegistry()
 	metrics := notifications.NewMetrics(reg)
 	template := notifications.TemplateWorkspaceDeleted

 	const (
 		method      = database.NotificationMethodSmtp
@@ -216,7 +217,7 @@

 	reg := prometheus.NewRegistry()
 	metrics := notifications.NewMetrics(reg)
 	template := notifications.TemplateWorkspaceDeleted

 	const method = database.NotificationMethodSmtp

@@ -300,7 +301,7 @@

 	reg := prometheus.NewRegistry()
 	metrics := notifications.NewMetrics(reg)
 	template := notifications.TemplateWorkspaceDeleted

 	const method = database.NotificationMethodSmtp

@@ -379,7 +380,7 @@
 	var (
 		reg             = prometheus.NewRegistry()
 		metrics         = notifications.NewMetrics(reg)
 		template        = notifications.TemplateWorkspaceDeleted
 		anotherTemplate = notifications.TemplateWorkspaceDormant
 	)

@@ -515,8 +516,8 @@
 	}
 }
 
-func (bh *barrierHandler) Dispatcher(payload types.MessagePayload, title, body string) (dispatch.DeliveryFunc, error) {
-	deliverFn, err := bh.h.Dispatcher(payload, title, body)
+func (bh *barrierHandler) Dispatcher(helpers template.FuncMap, payload types.MessagePayload, title, body string) (dispatch.DeliveryFunc, error) {
+	deliverFn, err := bh.h.Dispatcher(helpers, payload, title, body)
 	if err != nil {
 		return nil, err
 	}