Oct 22, 2024 · Oct 2, 2024 · Oct 3, 2024 · Oct 3, 2024 · Oct 3, 2024 · Oct 3, 2024
diff --git a/coderd/notifications/dispatch/smtp.go b/coderd/notifications/dispatch/smtp.go

 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 {
 // 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
 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
 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
 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
 "errors"
 "io"
 "net/http"
 "text/template"

 "github.com/google/uuid"
 "golang.org/x/xerrors"
 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
 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
 "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) {
 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
 stop: make(chan any),
 done: make(chan any),

 handlers: defaultHandlers(cfg,helpers,log),
 handlers: defaultHandlers(cfg, log),
 helpers:  helpers,

 clock: quartz.NewReal(),
 }

 // 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
 "encoding/json"
 "sync/atomic"
 "testing"
 "text/template"
 "time"

 "github.com/google/uuid"
 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
 "strconv"
 "sync"
 "testing"
 "text/template"
 "time"

 "github.com/google/uuid"

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

 const (
 method      = database.NotificationMethodSmtp

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

 const method = database.NotificationMethodSmtp


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

 const method = database.NotificationMethodSmtp

 var (
 reg             = prometheus.NewRegistry()
 metrics         = notifications.NewMetrics(reg)
 template        = notifications.TemplateWorkspaceDeleted
 anotherTemplate = notifications.TemplateWorkspaceDormant
 )

 }
 }

 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
 }
Original file line number	Diff line number	Diff line change
Expand Up		@@ -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) {
Copy link Contributor dannykoppingOct 18, 2024 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. Nit: when passing arguments, I try think of them in order of significance. The helpers are not the most significant argument here; they're likely the least. I'd move`helpers` to the end.
		// First render the subject & body into their own discrete strings.
		subject, err := markdown.PlaintextFromMarkdown(titleTmpl)
		if err != nil {
Expand All		@@ -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)
		}
Expand Down
Original file line number	Diff line number	Diff line change
Expand Up		@@ -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
Expand DownExpand Up		@@ -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()
Expand Down
Original file line number	Diff line number	Diff line change
		@@ -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" },
		}
		)
Original file line number	Diff line number	Diff line change
Expand Up		@@ -7,6 +7,7 @@
		"errors"
		"io"
		"net/http"
		"text/template"

		"github.com/google/uuid"
		"golang.org/x/xerrors"
Expand DownExpand Up		@@ -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) {
Check failure on line 45 in coderd/notifications/dispatch/webhook.go View workflow job for this annotation GitHub Actions/ lint `unused-parameter: parameter 'helpers' seems to be unused, consider removing or renaming it as _ (revive)`
		if w.cfg.Endpoint.String() == "" {
		return nil, xerrors.New("webhook endpoint not defined")
		}
Expand Down
Original file line number	Diff line number	Diff line change
Expand Up		@@ -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)
Expand Down
Original file line number	Diff line number	Diff line change
Expand Up		@@ -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) {
Expand All		@@ -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) {
Expand Down
Original file line number	Diff line number	Diff line change
Expand Up		@@ -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(),
Expand All		@@ -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")),
		}
		}
Expand Down
Original file line number	Diff line number	Diff line change
Expand Up		@@ -5,6 +5,7 @@
		"encoding/json"
		"sync/atomic"
		"testing"
		"text/template"
		"time"

		"github.com/google/uuid"
Expand DownExpand Up		@@ -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) {
Check failure on line 209 in coderd/notifications/manager_test.go View workflow job for this annotation GitHub Actions/ lint `unused-parameter: parameter 'helpers' seems to be unused, consider removing or renaming it as _ (revive)`
		return func(ctx context.Context, msgID uuid.UUID) (retryable bool, err error) {
		if payload.Labels["nice"] != "true" {
		s.naughty.Add(1)
Expand Down
Original file line number	Diff line number	Diff line change
Expand Up		@@ -5,6 +5,7 @@
		"strconv"
		"sync"
		"testing"
		"text/template"
		"time"

		"github.com/google/uuid"
Expand DownExpand Up		@@ -43,7 +44,7 @@

		reg := prometheus.NewRegistry()
		metrics := notifications.NewMetrics(reg)
		template := notifications.TemplateWorkspaceDeleted
Check failure on line 47 in coderd/notifications/metrics_test.go View workflow job for this annotation GitHub Actions/ lint `import-shadowing: The name 'template' shadows an import name (revive)`

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

		reg := prometheus.NewRegistry()
		metrics := notifications.NewMetrics(reg)
		template := notifications.TemplateWorkspaceDeleted
Check failure on line 220 in coderd/notifications/metrics_test.go View workflow job for this annotation GitHub Actions/ lint `import-shadowing: The name 'template' shadows an import name (revive)`

		const method = database.NotificationMethodSmtp

Expand DownExpand Up		@@ -300,7 +301,7 @@

		reg := prometheus.NewRegistry()
		metrics := notifications.NewMetrics(reg)
		template := notifications.TemplateWorkspaceDeleted
Check failure on line 304 in coderd/notifications/metrics_test.go View workflow job for this annotation GitHub Actions/ lint `import-shadowing: The name 'template' shadows an import name (revive)`

		const method = database.NotificationMethodSmtp

Expand DownExpand Up		@@ -379,7 +380,7 @@
		var (
		reg = prometheus.NewRegistry()
		metrics = notifications.NewMetrics(reg)
		template = notifications.TemplateWorkspaceDeleted
Check failure on line 383 in coderd/notifications/metrics_test.go View workflow job for this annotation GitHub Actions/ lint `import-shadowing: The name 'template' shadows an import name (revive)`
		anotherTemplate = notifications.TemplateWorkspaceDormant
		)

Expand DownExpand Up		@@ -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
		}
Expand Down