Skip to content

refactor: refactor notification email templates #14208

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Aug 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions cli/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand All @@ -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)
}
Expand Down Expand Up @@ -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()) },
}
}

Expand Down
11 changes: 7 additions & 4 deletions coderd/notifications/dispatch/smtp.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"slices"
"strings"
"sync"
"text/template"
"time"

"github.com/emersion/go-sasl"
Expand Down Expand Up @@ -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) {
Expand All @@ -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)
}
Expand Down
47 changes: 26 additions & 21 deletions coderd/notifications/dispatch/smtp/html.gotmpl
Original file line number Diff line number Diff line change
@@ -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>
6 changes: 5 additions & 1 deletion coderd/notifications/dispatch/smtp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 5 additions & 4 deletions coderd/notifications/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package notifications
import (
"context"
"sync"
"text/template"
"time"

"github.com/google/uuid"
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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")),
}
}
Expand Down
4 changes: 2 additions & 2 deletions coderd/notifications/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions coderd/notifications/metrics_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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))
Expand Down
24 changes: 12 additions & 12 deletions coderd/notifications/notifications_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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"))
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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})

Expand Down Expand Up @@ -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)
Expand All @@ -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() {
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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)
Expand Down
3 changes: 2 additions & 1 deletion coderd/notifications/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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" },
}
}

Expand Down
Loading