Skip to content

Commit 6fbc331

Browse files
BrunoQuaresmajaaydenh
BrunoQuaresma
authored and
jaaydenh
committed
refactor: refactor notification email template (#14208)
1 parent 14da05a commit 6fbc331

File tree

9 files changed

+68
-52
lines changed

9 files changed

+68
-52
lines changed

‎cli/server.go

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -993,9 +993,10 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd.
993993
if experiments.Enabled(codersdk.ExperimentNotifications) {
994994
cfg := options.DeploymentValues.Notifications
995995
metrics := notifications.NewMetrics(options.PrometheusRegistry)
996+
helpers := templateHelpers(options)
996997

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

‎coderd/notifications/dispatch/smtp.go

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616
"slices"
1717
"strings"
1818
"sync"
19+
"text/template"
1920
"time"
2021

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

5556
loginWarnOnce sync.Once
57+
58+
helpers template.FuncMap
5659
}
5760

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

6265
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
7578
// Then, reuse these strings in the HTML & plain body templates.
7679
payload.Labels["_subject"] = subject
7780
payload.Labels["_body"] = htmlBody
78-
htmlBody, err = render.GoTemplate(htmlTemplate, payload, nil)
81+
htmlBody, err = render.GoTemplate(htmlTemplate, payload, s.helpers)
7982
if err != nil {
8083
return nil, xerrors.Errorf("render full html template: %w", err)
8184
}
8285
payload.Labels["_body"] = plainBody
83-
plainBody, err = render.GoTemplate(plainTemplate, payload, nil)
86+
plainBody, err = render.GoTemplate(plainTemplate, payload, s.helpers)
8487
if err != nil {
8588
return nil, xerrors.Errorf("render full plaintext template: %w", err)
8689
}

‎coderd/notifications/dispatch/smtp/html.gotmpl

Lines changed: 26 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,27 +1,32 @@
1-
<!DOCTYPE html>
1+
<!doctype html>
22
<html lang="en">
3-
<head>
4-
<meta charset="UTF-8">
5-
<meta name="viewport" content="width=device-width, initial-scale=1.0">
3+
<head>
4+
<meta charset="UTF-8" />
5+
<meta name="viewport" content="width=device-width, initial-scale=1.0" />
66
<title>{{ .Labels._subject }}</title>
7-
</head>
8-
<body style="font-family: Arial, sans-serif; background-color: #1d1d20; margin: 0; padding: 0;">
9-
<div style="max-width: 600px; margin: 20px auto; background-color: #3f556d; border: 1px solid #34495E; padding: 20px; border-radius: 8px;">
10-
<div style="text-align: center; padding: 20px 0; border-bottom: 1px solid #34495E;">
11-
<img width="215" height="47" src="https://coder.com/logo-wide-white.png"/>
12-
</div>
13-
<div style="padding: 20px; color: #ECF0F1; line-height: 1.6;">
14-
<h1 style="color: #ECF0F1;">{{ .Labels._subject }}</h1>
7+
</head>
8+
<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;">
9+
<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;">
10+
<div style="text-align: center;">
11+
<img src="https://coder.com/coder-logo-horizontal.png" alt="Coder Logo" style="height: 40px;" />
12+
</div>
13+
<h1 style="text-align: center; font-size: 24px; font-weight: 400; margin: 8px 0 32px; line-height: 1.5;">
14+
{{ .Labels._subject }}
15+
</h1>
16+
<div style="line-height: 1.5;">
1517
{{ .Labels._body }}
16-
18+
</div>
19+
<div style="text-align: center; margin-top: 32px;">
1720
{{ range $action := .Actions }}
18-
<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>
21+
<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;">
22+
{{ $action.Label }}
23+
</a>
1924
{{ end }}
25+
</div>
26+
<div style="border-top: 1px solid #e2e8f0; color: #475569; font-size: 12px; margin-top: 64px; padding-top: 24px; line-height: 1.6;">
27+
<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>
28+
<p><a href="{{ base_url }}/settings/notifications" style="color: #2563eb; text-decoration: none;">Click here to manage your notification settings</a></p>
29+
</div>
2030
</div>
21-
<div style="text-align: center; padding: 10px 0; border-top: 1px solid #34495E; margin-top: 20px; color: #BDC3C7;">
22-
<!-- TODO: dynamic copyright -->
23-
&copy; 2024 Coder. All rights reserved.
24-
</div>
25-
</div>
26-
</body>
27-
</html>
31+
</body>
32+
</html>

‎coderd/notifications/dispatch/smtp_test.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -417,7 +417,11 @@ func TestSMTP(t *testing.T) {
417417
require.NoError(t, hp.Set(listen.Addr().String()))
418418
tc.cfg.Smarthost = hp
419419

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

422426
// Start mock SMTP server in the background.
423427
var wg sync.WaitGroup

‎coderd/notifications/manager.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package notifications
33
import (
44
"context"
55
"sync"
6+
"text/template"
67
"time"
78

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

96-
handlers: defaultHandlers(cfg, log),
97+
handlers: defaultHandlers(cfg, helpers, log),
9798
}, nil
9899
}
99100

100101
// defaultHandlers builds a set of known handlers; panics if any error occurs as these handlers should be valid at compile time.
101-
func defaultHandlers(cfg codersdk.NotificationsConfig, log slog.Logger) map[database.NotificationMethod]Handler {
102+
func defaultHandlers(cfg codersdk.NotificationsConfig, helpers template.FuncMap, log slog.Logger) map[database.NotificationMethod]Handler {
102103
return map[database.NotificationMethod]Handler{
103-
database.NotificationMethodSmtp: dispatch.NewSMTPHandler(cfg.SMTP, log.Named("dispatcher.smtp")),
104+
database.NotificationMethodSmtp: dispatch.NewSMTPHandler(cfg.SMTP, helpers, log.Named("dispatcher.smtp")),
104105
database.NotificationMethodWebhook: dispatch.NewWebhookHandler(cfg.Webhook, log.Named("dispatcher.webhook")),
105106
}
106107
}

‎coderd/notifications/manager_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ func TestBufferedUpdates(t *testing.T) {
3434
cfg.StoreSyncInterval = serpent.Duration(time.Hour) // Ensure we don't sync the store automatically.
3535

3636
// GIVEN: a manager which will pass or fail notifications based on their "nice" labels
37-
mgr, err := notifications.NewManager(cfg, interceptor, createMetrics(), logger.Named("notifications-manager"))
37+
mgr, err := notifications.NewManager(cfg, interceptor, defaultHelpers(), createMetrics(), logger.Named("notifications-manager"))
3838
require.NoError(t, err)
3939
mgr.WithHandlers(map[database.NotificationMethod]notifications.Handler{
4040
database.NotificationMethodSmtp: santa,
@@ -150,7 +150,7 @@ func TestStopBeforeRun(t *testing.T) {
150150
ctx, logger, db := setupInMemory(t)
151151

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

156156
// THEN: validate that the manager can be stopped safely without Run() having been called yet

‎coderd/notifications/metrics_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ func TestMetrics(t *testing.T) {
5151
cfg.RetryInterval = serpent.Duration(time.Millisecond * 50)
5252
cfg.StoreSyncInterval = serpent.Duration(time.Millisecond * 100) // Twice as long as fetch interval to ensure we catch pending updates.
5353

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

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

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

372372
// WHEN: two notifications (each with different templates) are enqueued.
373373
cfg := defaultNotificationsConfig(defaultMethod)
374-
mgr, err := notifications.NewManager(cfg, store, metrics, logger.Named("manager"))
374+
mgr, err := notifications.NewManager(cfg, store, defaultHelpers(), metrics, logger.Named("manager"))
375375
require.NoError(t, err)
376376
t.Cleanup(func() {
377377
assert.NoError(t, mgr.Stop(ctx))

‎coderd/notifications/notifications_test.go

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ func TestBasicNotificationRoundtrip(t *testing.T) {
6565
interceptor := &syncInterceptor{Store: db}
6666
cfg := defaultNotificationsConfig(method)
6767
cfg.RetryInterval = serpent.Duration(time.Hour) // Ensure retries don't interfere with the test
68-
mgr, err := notifications.NewManager(cfg, interceptor, createMetrics(), logger.Named("manager"))
68+
mgr, err := notifications.NewManager(cfg, interceptor, defaultHelpers(), createMetrics(), logger.Named("manager"))
6969
require.NoError(t, err)
7070
mgr.WithHandlers(map[database.NotificationMethod]notifications.Handler{method: handler})
7171
t.Cleanup(func() {
@@ -138,8 +138,8 @@ func TestSMTPDispatch(t *testing.T) {
138138
Smarthost: serpent.HostPort{Host: "localhost", Port: fmt.Sprintf("%d", mockSMTPSrv.PortNumber())},
139139
Hello: "localhost",
140140
}
141-
handler := newDispatchInterceptor(dispatch.NewSMTPHandler(cfg.SMTP, logger.Named("smtp")))
142-
mgr, err := notifications.NewManager(cfg, db, createMetrics(), logger.Named("manager"))
141+
handler := newDispatchInterceptor(dispatch.NewSMTPHandler(cfg.SMTP, defaultHelpers(), logger.Named("smtp")))
142+
mgr, err := notifications.NewManager(cfg, db, defaultHelpers(), createMetrics(), logger.Named("manager"))
143143
require.NoError(t, err)
144144
mgr.WithHandlers(map[database.NotificationMethod]notifications.Handler{method: handler})
145145
t.Cleanup(func() {
@@ -200,7 +200,7 @@ func TestWebhookDispatch(t *testing.T) {
200200
cfg.Webhook = codersdk.NotificationsWebhookConfig{
201201
Endpoint: *serpent.URLOf(endpoint),
202202
}
203-
mgr, err := notifications.NewManager(cfg, db, createMetrics(), logger.Named("manager"))
203+
mgr, err := notifications.NewManager(cfg, db, defaultHelpers(), createMetrics(), logger.Named("manager"))
204204
require.NoError(t, err)
205205
t.Cleanup(func() {
206206
assert.NoError(t, mgr.Stop(ctx))
@@ -298,7 +298,7 @@ func TestBackpressure(t *testing.T) {
298298
storeInterceptor := &syncInterceptor{Store: db}
299299

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

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

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

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

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

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

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

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

940-
mgr, err := notifications.NewManager(cfg, db, createMetrics(), logger.Named("manager"))
940+
mgr, err := notifications.NewManager(cfg, db, defaultHelpers(), createMetrics(), logger.Named("manager"))
941941
require.NoError(t, err)
942942
t.Cleanup(func() {
943943
_ = mgr.Stop(ctx)

‎coderd/notifications/utils_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,8 @@ func defaultNotificationsConfig(method database.NotificationMethod) codersdk.Not
7777

7878
func defaultHelpers() map[string]any {
7979
return map[string]any{
80-
"base_url": func() string { return "http://test.com" },
80+
"base_url": func() string { return "http://test.com" },
81+
"current_year": func() string { return "2024" },
8182
}
8283
}
8384

0 commit comments

Comments
 (0)