refactor: refactor notification email template (#14208)

2026-06-02 20:48:20 +00:00 · 2024-08-09 11:25:19 -03:00
parent abbcffe181
commit 27b8f201a4
9 changed files with 68 additions and 52 deletions
@@ -993,9 +993,10 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd.
 			if experiments.Enabled(codersdk.ExperimentNotifications) {
 				cfg := options.DeploymentValues.Notifications
 				metrics := notifications.NewMetrics(options.PrometheusRegistry)
+				helpers := templateHelpers(options)

 				// The enqueuer is responsible for enqueueing notifications to the given store.
-				enqueuer, err := notifications.NewStoreEnqueuer(cfg, options.Database, templateHelpers(options), logger.Named("notifications.enqueuer"))
+				enqueuer, err := notifications.NewStoreEnqueuer(cfg, options.Database, helpers, logger.Named("notifications.enqueuer"))
 				if err != nil {
 					return xerrors.Errorf("failed to instantiate notification store enqueuer: %w", err)
 				}
@@ -1004,7 +1005,7 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd.
 				// The notification manager is responsible for:
 				//   - creating notifiers and managing their lifecycles (notifiers are responsible for dequeueing/sending notifications)
 				//   - keeping the store updated with status updates
-				notificationsManager, err = notifications.NewManager(cfg, options.Database, metrics, logger.Named("notifications.manager"))
+				notificationsManager, err = notifications.NewManager(cfg, options.Database, helpers, metrics, logger.Named("notifications.manager"))
 				if err != nil {
 					return xerrors.Errorf("failed to instantiate notification manager: %w", err)
 				}
@@ -1291,7 +1292,8 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd.
 // We can later use this to inject whitelabel fields when app name / logo URL are overridden.
 func templateHelpers(options *coderd.Options) map[string]any {
 	return map[string]any{
-		"base_url": func() string { return options.AccessURL.String() },
+		"base_url":     func() string { return options.AccessURL.String() },
+		"current_year": func() string { return strconv.Itoa(time.Now().Year()) },
 	}
 }

@@ -16,6 +16,7 @@ import (
 	"slices"
 	"strings"
 	"sync"
+	"text/template"
 	"time"

 	"github.com/emersion/go-sasl"
@@ -53,10 +54,12 @@ type SMTPHandler struct {
 	log slog.Logger

 	loginWarnOnce sync.Once
+
+	helpers template.FuncMap
 }

-func NewSMTPHandler(cfg codersdk.NotificationsEmailConfig, log slog.Logger) *SMTPHandler {
-	return &SMTPHandler{cfg: cfg, log: log}
+func NewSMTPHandler(cfg codersdk.NotificationsEmailConfig, helpers template.FuncMap, log slog.Logger) *SMTPHandler {
+	return &SMTPHandler{cfg: cfg, helpers: helpers, log: log}
 }

 func (s *SMTPHandler) Dispatcher(payload types.MessagePayload, titleTmpl, bodyTmpl string) (DeliveryFunc, error) {
@@ -75,12 +78,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, nil)
+	htmlBody, err = render.GoTemplate(htmlTemplate, payload, s.helpers)
 	if err != nil {
 		return nil, xerrors.Errorf("render full html template: %w", err)
 	}
 	payload.Labels["_body"] = plainBody
-	plainBody, err = render.GoTemplate(plainTemplate, payload, nil)
+	plainBody, err = render.GoTemplate(plainTemplate, payload, s.helpers)
 	if err != nil {
 		return nil, xerrors.Errorf("render full plaintext template: %w", err)
 	}
@@ -1,27 +1,32 @@
-<!DOCTYPE html>
+<!doctype html>
 <html lang="en">
-<head>
-    <meta charset="UTF-8">
-    <meta name="viewport" content="width=device-width, initial-scale=1.0">
+  <head>
+    <meta charset="UTF-8" />
+    <meta name="viewport" content="width=device-width, initial-scale=1.0" />
    <title>{{ .Labels._subject }}</title>
-</head>
-<body style="font-family: Arial, sans-serif; background-color: #1d1d20; margin: 0; padding: 0;">
-<div style="max-width: 600px; margin: 20px auto; background-color: #3f556d; border: 1px solid #34495E; padding: 20px; border-radius: 8px;">
-    <div style="text-align: center; padding: 20px 0; border-bottom: 1px solid #34495E;">
-        <img width="215" height="47" src="https://coder.com/logo-wide-white.png"/>
-    </div>
-    <div style="padding: 20px; color: #ECF0F1; line-height: 1.6;">
-        <h1 style="color: #ECF0F1;">{{ .Labels._subject }}</h1>
+  </head>
+  <body style="margin: 0; padding: 0; font-family: -apple-system, system-ui, BlinkMacSystemFont, 'Segoe UI', 'Roboto', 'Oxygen', 'Ubuntu', 'Cantarell', 'Fira Sans', 'Droid Sans', 'Helvetica Neue', sans-serif; color: #020617; background: #f8fafc;">
+    <div style="max-width: 600px; margin: 20px auto; padding: 60px; border: 1px solid #e2e8f0; border-radius: 8px; background-color: #fff; text-align: left; font-size: 14px; line-height: 1.5;">
+      <div style="text-align: center;">
+        <img src="https://coder.com/coder-logo-horizontal.png" alt="Coder Logo" style="height: 40px;" />
+      </div>
+      <h1 style="text-align: center; font-size: 24px; font-weight: 400; margin: 8px 0 32px; line-height: 1.5;">
+        {{ .Labels._subject }}
+      </h1>
+      <div style="line-height: 1.5;">
        {{ .Labels._body }}
-
+      </div>
+      <div style="text-align: center; margin-top: 32px;">
        {{ range $action := .Actions }}
-        <a href="{{ $action.URL }}" style="display: inline-block; padding: 10px 20px; background-color: #3D74DB; color: #ffffff; text-decoration: none; border-radius: 4px; margin-top: 20px;">{{ $action.Label }}</a><br>
+        <a href="{{ $action.URL }}" style="display: inline-block; padding: 13px 24px; background-color: #020617; color: #f8fafc; text-decoration: none; border-radius: 8px; margin: 0 4px;">
+          {{ $action.Label }}
+        </a>
        {{ end }}
+      </div>
+      <div style="border-top: 1px solid #e2e8f0; color: #475569; font-size: 12px; margin-top: 64px; padding-top: 24px; line-height: 1.6;">
+        <p>&copy;&nbsp;{{ current_year }}&nbsp;Coder. All rights reserved&nbsp;-&nbsp;<a href="{{ base_url }}" style="color: #2563eb; text-decoration: none;">{{ base_url }}</a></p>
+        <p><a href="{{ base_url }}/settings/notifications" style="color: #2563eb; text-decoration: none;">Click here to manage your notification settings</a></p>
+      </div>
    </div>
-    <div style="text-align: center; padding: 10px 0; border-top: 1px solid #34495E; margin-top: 20px; color: #BDC3C7;">
-        <!-- TODO: dynamic copyright -->
-        &copy; 2024 Coder. All rights reserved.
-    </div>
-</div>
-</body>
-</html>
+  </body>
+</html>
@@ -417,7 +417,11 @@ func TestSMTP(t *testing.T) {
 			require.NoError(t, hp.Set(listen.Addr().String()))
 			tc.cfg.Smarthost = hp

-			handler := dispatch.NewSMTPHandler(tc.cfg, logger.Named("smtp"))
+			helpers := map[string]any{
+				"base_url":     func() string { return "http://test.com" },
+				"current_year": func() string { return "2024" },
+			}
+			handler := dispatch.NewSMTPHandler(tc.cfg, helpers, logger.Named("smtp"))

 			// Start mock SMTP server in the background.
 			var wg sync.WaitGroup
@@ -3,6 +3,7 @@ package notifications
 import (
 	"context"
 	"sync"
+	"text/template"
 	"time"

 	"github.com/google/uuid"
@@ -59,7 +60,7 @@ type Manager struct {
 //
 // helpers is a map of template helpers which are used to customize notification messages to use global settings like
 // access URL etc.
-func NewManager(cfg codersdk.NotificationsConfig, store Store, metrics *Metrics, log slog.Logger) (*Manager, error) {
+func NewManager(cfg codersdk.NotificationsConfig, store Store, helpers template.FuncMap, metrics *Metrics, log slog.Logger) (*Manager, error) {
 	// TODO(dannyk): add the ability to use multiple notification methods.
 	var method database.NotificationMethod
 	if err := method.Scan(cfg.Method.String()); err != nil {
@@ -93,14 +94,14 @@ func NewManager(cfg codersdk.NotificationsConfig, store Store, metrics *Metrics,
 		stop: make(chan any),
 		done: make(chan any),

-		handlers: defaultHandlers(cfg, log),
+		handlers: defaultHandlers(cfg, helpers, log),
 	}, nil
 }

 // 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, helpers template.FuncMap, log slog.Logger) map[database.NotificationMethod]Handler {
 	return map[database.NotificationMethod]Handler{
-		database.NotificationMethodSmtp:    dispatch.NewSMTPHandler(cfg.SMTP, log.Named("dispatcher.smtp")),
+		database.NotificationMethodSmtp:    dispatch.NewSMTPHandler(cfg.SMTP, helpers, log.Named("dispatcher.smtp")),
 		database.NotificationMethodWebhook: dispatch.NewWebhookHandler(cfg.Webhook, log.Named("dispatcher.webhook")),
 	}
 }
@@ -34,7 +34,7 @@ func TestBufferedUpdates(t *testing.T) {
 	cfg.StoreSyncInterval = serpent.Duration(time.Hour) // Ensure we don't sync the store automatically.

 	// GIVEN: a manager which will pass or fail notifications based on their "nice" labels
-	mgr, err := notifications.NewManager(cfg, interceptor, createMetrics(), logger.Named("notifications-manager"))
+	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,
@@ -150,7 +150,7 @@ func TestStopBeforeRun(t *testing.T) {
 	ctx, logger, db := setupInMemory(t)

 	// GIVEN: a standard manager
-	mgr, err := notifications.NewManager(defaultNotificationsConfig(database.NotificationMethodSmtp), db, createMetrics(), logger.Named("notifications-manager"))
+	mgr, err := notifications.NewManager(defaultNotificationsConfig(database.NotificationMethodSmtp), db, defaultHelpers(), createMetrics(), logger.Named("notifications-manager"))
 	require.NoError(t, err)

 	// THEN: validate that the manager can be stopped safely without Run() having been called yet
@@ -51,7 +51,7 @@ func TestMetrics(t *testing.T) {
 	cfg.RetryInterval = serpent.Duration(time.Millisecond * 50)
 	cfg.StoreSyncInterval = serpent.Duration(time.Millisecond * 100) // Twice as long as fetch interval to ensure we catch pending updates.

-	mgr, err := notifications.NewManager(cfg, store, metrics, logger.Named("manager"))
+	mgr, err := notifications.NewManager(cfg, store, defaultHelpers(), metrics, logger.Named("manager"))
 	require.NoError(t, err)
 	t.Cleanup(func() {
 		assert.NoError(t, mgr.Stop(ctx))
@@ -218,7 +218,7 @@ func TestPendingUpdatesMetric(t *testing.T) {

 	syncer := &syncInterceptor{Store: store}
 	interceptor := newUpdateSignallingInterceptor(syncer)
-	mgr, err := notifications.NewManager(cfg, interceptor, metrics, logger.Named("manager"))
+	mgr, err := notifications.NewManager(cfg, interceptor, defaultHelpers(), metrics, logger.Named("manager"))
 	require.NoError(t, err)
 	t.Cleanup(func() {
 		assert.NoError(t, mgr.Stop(ctx))
@@ -292,7 +292,7 @@ func TestInflightDispatchesMetric(t *testing.T) {
 	cfg.RetryInterval = serpent.Duration(time.Hour) // Delay retries so they don't interfere.
 	cfg.StoreSyncInterval = serpent.Duration(time.Millisecond * 100)

-	mgr, err := notifications.NewManager(cfg, store, metrics, logger.Named("manager"))
+	mgr, err := notifications.NewManager(cfg, store, defaultHelpers(), metrics, logger.Named("manager"))
 	require.NoError(t, err)
 	t.Cleanup(func() {
 		assert.NoError(t, mgr.Stop(ctx))
@@ -371,7 +371,7 @@ func TestCustomMethodMetricCollection(t *testing.T) {

 	// WHEN: two notifications (each with different templates) are enqueued.
 	cfg := defaultNotificationsConfig(defaultMethod)
-	mgr, err := notifications.NewManager(cfg, store, metrics, logger.Named("manager"))
+	mgr, err := notifications.NewManager(cfg, store, defaultHelpers(), metrics, logger.Named("manager"))
 	require.NoError(t, err)
 	t.Cleanup(func() {
 		assert.NoError(t, mgr.Stop(ctx))
@@ -65,7 +65,7 @@ func TestBasicNotificationRoundtrip(t *testing.T) {
 	interceptor := &syncInterceptor{Store: db}
 	cfg := defaultNotificationsConfig(method)
 	cfg.RetryInterval = serpent.Duration(time.Hour) // Ensure retries don't interfere with the test
-	mgr, err := notifications.NewManager(cfg, interceptor, createMetrics(), logger.Named("manager"))
+	mgr, err := notifications.NewManager(cfg, interceptor, defaultHelpers(), createMetrics(), logger.Named("manager"))
 	require.NoError(t, err)
 	mgr.WithHandlers(map[database.NotificationMethod]notifications.Handler{method: handler})
 	t.Cleanup(func() {
@@ -138,8 +138,8 @@ func TestSMTPDispatch(t *testing.T) {
 		Smarthost: serpent.HostPort{Host: "localhost", Port: fmt.Sprintf("%d", mockSMTPSrv.PortNumber())},
 		Hello:     "localhost",
 	}
-	handler := newDispatchInterceptor(dispatch.NewSMTPHandler(cfg.SMTP, logger.Named("smtp")))
-	mgr, err := notifications.NewManager(cfg, db, createMetrics(), logger.Named("manager"))
+	handler := newDispatchInterceptor(dispatch.NewSMTPHandler(cfg.SMTP, defaultHelpers(), logger.Named("smtp")))
+	mgr, err := notifications.NewManager(cfg, db, defaultHelpers(), createMetrics(), logger.Named("manager"))
 	require.NoError(t, err)
 	mgr.WithHandlers(map[database.NotificationMethod]notifications.Handler{method: handler})
 	t.Cleanup(func() {
@@ -200,7 +200,7 @@ func TestWebhookDispatch(t *testing.T) {
 	cfg.Webhook = codersdk.NotificationsWebhookConfig{
 		Endpoint: *serpent.URLOf(endpoint),
 	}
-	mgr, err := notifications.NewManager(cfg, db, createMetrics(), logger.Named("manager"))
+	mgr, err := notifications.NewManager(cfg, db, defaultHelpers(), createMetrics(), logger.Named("manager"))
 	require.NoError(t, err)
 	t.Cleanup(func() {
 		assert.NoError(t, mgr.Stop(ctx))
@@ -298,7 +298,7 @@ func TestBackpressure(t *testing.T) {
 	storeInterceptor := &syncInterceptor{Store: db}

 	// GIVEN: a notification manager whose updates will be intercepted
-	mgr, err := notifications.NewManager(cfg, storeInterceptor, createMetrics(), logger.Named("manager"))
+	mgr, err := notifications.NewManager(cfg, storeInterceptor, defaultHelpers(), createMetrics(), logger.Named("manager"))
 	require.NoError(t, err)
 	mgr.WithHandlers(map[database.NotificationMethod]notifications.Handler{method: handler})
 	enq, err := notifications.NewStoreEnqueuer(cfg, db, defaultHelpers(), logger.Named("enqueuer"))
@@ -393,7 +393,7 @@ func TestRetries(t *testing.T) {
 	// Intercept calls to submit the buffered updates to the store.
 	storeInterceptor := &syncInterceptor{Store: db}

-	mgr, err := notifications.NewManager(cfg, storeInterceptor, createMetrics(), logger.Named("manager"))
+	mgr, err := notifications.NewManager(cfg, storeInterceptor, defaultHelpers(), createMetrics(), logger.Named("manager"))
 	require.NoError(t, err)
 	t.Cleanup(func() {
 		assert.NoError(t, mgr.Stop(ctx))
@@ -454,7 +454,7 @@ func TestExpiredLeaseIsRequeued(t *testing.T) {
 	mgrCtx, cancelManagerCtx := context.WithCancel(context.Background())
 	t.Cleanup(cancelManagerCtx)

-	mgr, err := notifications.NewManager(cfg, noopInterceptor, createMetrics(), logger.Named("manager"))
+	mgr, err := notifications.NewManager(cfg, noopInterceptor, defaultHelpers(), createMetrics(), logger.Named("manager"))
 	require.NoError(t, err)
 	enq, err := notifications.NewStoreEnqueuer(cfg, db, defaultHelpers(), logger.Named("enqueuer"))
 	require.NoError(t, err)
@@ -501,7 +501,7 @@ func TestExpiredLeaseIsRequeued(t *testing.T) {
 	// Intercept calls to submit the buffered updates to the store.
 	storeInterceptor := &syncInterceptor{Store: db}
 	handler := newDispatchInterceptor(&fakeHandler{})
-	mgr, err = notifications.NewManager(cfg, storeInterceptor, createMetrics(), logger.Named("manager"))
+	mgr, err = notifications.NewManager(cfg, storeInterceptor, defaultHelpers(), createMetrics(), logger.Named("manager"))
 	require.NoError(t, err)
 	mgr.WithHandlers(map[database.NotificationMethod]notifications.Handler{method: handler})

@@ -542,7 +542,7 @@ func TestInvalidConfig(t *testing.T) {
 	cfg.DispatchTimeout = serpent.Duration(leasePeriod)

 	// WHEN: the manager is created with invalid config
-	_, err := notifications.NewManager(cfg, db, createMetrics(), logger.Named("manager"))
+	_, err := notifications.NewManager(cfg, db, defaultHelpers(), createMetrics(), logger.Named("manager"))

 	// THEN: the manager will fail to be created, citing invalid config as error
 	require.ErrorIs(t, err, notifications.ErrInvalidDispatchTimeout)
@@ -560,7 +560,7 @@ func TestNotifierPaused(t *testing.T) {
 	user := createSampleUser(t, db)

 	cfg := defaultNotificationsConfig(method)
-	mgr, err := notifications.NewManager(cfg, db, createMetrics(), logger.Named("manager"))
+	mgr, err := notifications.NewManager(cfg, db, defaultHelpers(), createMetrics(), logger.Named("manager"))
 	require.NoError(t, err)
 	mgr.WithHandlers(map[database.NotificationMethod]notifications.Handler{method: handler})
 	t.Cleanup(func() {
@@ -831,7 +831,7 @@ func TestDisabledAfterEnqueue(t *testing.T) {
 	method := database.NotificationMethodSmtp
 	cfg := defaultNotificationsConfig(method)

-	mgr, err := notifications.NewManager(cfg, db, createMetrics(), logger.Named("manager"))
+	mgr, err := notifications.NewManager(cfg, db, defaultHelpers(), createMetrics(), logger.Named("manager"))
 	require.NoError(t, err)
 	t.Cleanup(func() {
 		assert.NoError(t, mgr.Stop(ctx))
@@ -937,7 +937,7 @@ func TestCustomNotificationMethod(t *testing.T) {
 		Endpoint: *serpent.URLOf(endpoint),
 	}

-	mgr, err := notifications.NewManager(cfg, db, createMetrics(), logger.Named("manager"))
+	mgr, err := notifications.NewManager(cfg, db, defaultHelpers(), createMetrics(), logger.Named("manager"))
 	require.NoError(t, err)
 	t.Cleanup(func() {
 		_ = mgr.Stop(ctx)
@@ -77,7 +77,8 @@ func defaultNotificationsConfig(method database.NotificationMethod) codersdk.Not

 func defaultHelpers() map[string]any {
 	return map[string]any{
-		"base_url": func() string { return "http://test.com" },
+		"base_url":     func() string { return "http://test.com" },
+		"current_year": func() string { return "2024" },
 	}
 }