Skip to content

Commit 297089e

Browse files
defelmnqdefelmnq
defelmnq
and
defelmnq
authored
feat(coderd): add company logo when available for email notifications (#14935)
This PR aims to close #14253 We keep the default behavior using the Coder logo if there's no logo set. Otherwise we want to use the logo based on the URL set in appearance. --------- Co-authored-by: defelmnq <yvincent@coder.com>
1 parent c42f487 commit 297089e

16 files changed

+289
-52
lines changed

‎coderd/notifications/dispatch/smtp.go

+5-7
Original file line numberDiff line numberDiff line change
@@ -55,15 +55,13 @@ type SMTPHandler struct {
5555

5656
noAuthWarnOnce sync.Once
5757
loginWarnOnce sync.Once
58-
59-
helpers template.FuncMap
6058
}
6159

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

66-
func (s *SMTPHandler) Dispatcher(payload types.MessagePayload, titleTmpl, bodyTmpl string) (DeliveryFunc, error) {
64+
func (s *SMTPHandler) Dispatcher(payload types.MessagePayload, titleTmpl, bodyTmpl string, helpers template.FuncMap) (DeliveryFunc, error) {
6765
// First render the subject & body into their own discrete strings.
6866
subject, err := markdown.PlaintextFromMarkdown(titleTmpl)
6967
if err != nil {
@@ -79,12 +77,12 @@ func (s *SMTPHandler) Dispatcher(payload types.MessagePayload, titleTmpl, bodyTm
7977
// Then, reuse these strings in the HTML & plain body templates.
8078
payload.Labels["_subject"] = subject
8179
payload.Labels["_body"] = htmlBody
82-
htmlBody, err = render.GoTemplate(htmlTemplate, payload, s.helpers)
80+
htmlBody, err = render.GoTemplate(htmlTemplate, payload, helpers)
8381
if err != nil {
8482
return nil, xerrors.Errorf("render full html template: %w", err)
8583
}
8684
payload.Labels["_body"] = plainBody
87-
plainBody, err = render.GoTemplate(plainTemplate, payload, s.helpers)
85+
plainBody, err = render.GoTemplate(plainTemplate, payload, helpers)
8886
if err != nil {
8987
return nil, xerrors.Errorf("render full plaintext template: %w", err)
9088
}

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
<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;">
99
<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;">
1010
<div style="text-align: center;">
11-
<img src="https://coder.com/coder-logo-horizontal.png" alt="Coder Logo" style="height: 40px;" />
11+
<img src="{{ logo_url }}" alt="{{ app_name }} Logo" style="height: 40px;" />
1212
</div>
1313
<h1 style="text-align: center; font-size: 24px; font-weight: 400; margin: 8px 0 32px; line-height: 1.5;">
1414
{{ .Labels._subject }}

‎coderd/notifications/dispatch/smtp_test.go

+2-6
Original file line numberDiff line numberDiff line change
@@ -442,11 +442,7 @@ func TestSMTP(t *testing.T) {
442442
require.NoError(t, hp.Set(listen.Addr().String()))
443443
tc.cfg.Smarthost = hp
444444

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

451447
// Start mock SMTP server in the background.
452448
var wg sync.WaitGroup
@@ -484,7 +480,7 @@ func TestSMTP(t *testing.T) {
484480
Labels: make(map[string]string),
485481
}
486482

487-
dispatchFn, err := handler.Dispatcher(payload, subject, body)
483+
dispatchFn, err := handler.Dispatcher(payload, subject, body, helpers())
488484
require.NoError(t, err)
489485

490486
msgID := uuid.New()

‎coderd/notifications/dispatch/utils_test.go

+10
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
package dispatch_test
2+
3+
func helpers() map[string]any {
4+
return map[string]any{
5+
"base_url": func() string { return "http://test.com" },
6+
"current_year": func() string { return "2024" },
7+
"logo_url": func() string { return "https://coder.com/coder-logo-horizontal.png" },
8+
"app_name": func() string { return "Coder" },
9+
}
10+
}

‎coderd/notifications/dispatch/webhook.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"errors"
88
"io"
99
"net/http"
10+
"text/template"
1011

1112
"github.com/google/uuid"
1213
"golang.org/x/xerrors"
@@ -41,7 +42,7 @@ func NewWebhookHandler(cfg codersdk.NotificationsWebhookConfig, log slog.Logger)
4142
return &WebhookHandler{cfg: cfg, log: log, cl: &http.Client{}}
4243
}
4344

44-
func (w *WebhookHandler) Dispatcher(payload types.MessagePayload, titleMarkdown, bodyMarkdown string) (DeliveryFunc, error) {
45+
func (w *WebhookHandler) Dispatcher(payload types.MessagePayload, titleMarkdown, bodyMarkdown string, _ template.FuncMap) (DeliveryFunc, error) {
4546
if w.cfg.Endpoint.String() == "" {
4647
return nil, xerrors.New("webhook endpoint not defined")
4748
}

‎coderd/notifications/dispatch/webhook_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@ func TestWebhook(t *testing.T) {
141141
Endpoint: *serpent.URLOf(endpoint),
142142
}
143143
handler := dispatch.NewWebhookHandler(cfg, logger.With(slog.F("test", tc.name)))
144-
deliveryFn, err := handler.Dispatcher(msgPayload, titleMarkdown, bodyMarkdown)
144+
deliveryFn, err := handler.Dispatcher(msgPayload, titleMarkdown, bodyMarkdown, helpers())
145145
require.NoError(t, err)
146146

147147
retryable, err := deliveryFn(ctx, msgID)

‎coderd/notifications/fetcher.go

+57
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
package notifications
2+
3+
import (
4+
"context"
5+
"database/sql"
6+
"errors"
7+
"text/template"
8+
9+
"golang.org/x/xerrors"
10+
11+
"cdr.dev/slog"
12+
)
13+
14+
func (n *notifier) fetchHelpers(ctx context.Context) (map[string]any, error) {
15+
appName, err := n.fetchAppName(ctx)
16+
if err != nil {
17+
n.log.Error(ctx, "failed to fetch app name", slog.Error(err))
18+
return nil, xerrors.Errorf("fetch app name: %w", err)
19+
}
20+
logoURL, err := n.fetchLogoURL(ctx)
21+
if err != nil {
22+
n.log.Error(ctx, "failed to fetch logo URL", slog.Error(err))
23+
return nil, xerrors.Errorf("fetch logo URL: %w", err)
24+
}
25+
26+
helpers := make(template.FuncMap)
27+
for k, v := range n.helpers {
28+
helpers[k] = v
29+
}
30+
31+
helpers["app_name"] = func() string { return appName }
32+
helpers["logo_url"] = func() string { return logoURL }
33+
34+
return helpers, nil
35+
}
36+
37+
func (n *notifier) fetchAppName(ctx context.Context) (string, error) {
38+
appName, err := n.store.GetApplicationName(ctx)
39+
if err != nil {
40+
if errors.Is(err, sql.ErrNoRows) {
41+
return notificationsDefaultAppName, nil
42+
}
43+
return "", xerrors.Errorf("get application name: %w", err)
44+
}
45+
return appName, nil
46+
}
47+
48+
func (n *notifier) fetchLogoURL(ctx context.Context) (string, error) {
49+
logoURL, err := n.store.GetLogoURL(ctx)
50+
if err != nil {
51+
if errors.Is(err, sql.ErrNoRows) {
52+
return notificationsDefaultLogoURL, nil
53+
}
54+
return "", xerrors.Errorf("get logo URL: %w", err)
55+
}
56+
return logoURL, nil
57+
}

‎coderd/notifications/manager.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ func NewManager(cfg codersdk.NotificationsConfig, store Store, helpers template.
109109
stop: make(chan any),
110110
done: make(chan any),
111111

112-
handlers: defaultHandlers(cfg, helpers, log),
112+
handlers: defaultHandlers(cfg, log),
113113
helpers: helpers,
114114

115115
clock: quartz.NewReal(),
@@ -121,9 +121,9 @@ func NewManager(cfg codersdk.NotificationsConfig, store Store, helpers template.
121121
}
122122

123123
// defaultHandlers builds a set of known handlers; panics if any error occurs as these handlers should be valid at compile time.
124-
func defaultHandlers(cfg codersdk.NotificationsConfig, helpers template.FuncMap, log slog.Logger) map[database.NotificationMethod]Handler {
124+
func defaultHandlers(cfg codersdk.NotificationsConfig, log slog.Logger) map[database.NotificationMethod]Handler {
125125
return map[database.NotificationMethod]Handler{
126-
database.NotificationMethodSmtp: dispatch.NewSMTPHandler(cfg.SMTP, helpers, log.Named("dispatcher.smtp")),
126+
database.NotificationMethodSmtp: dispatch.NewSMTPHandler(cfg.SMTP, log.Named("dispatcher.smtp")),
127127
database.NotificationMethodWebhook: dispatch.NewWebhookHandler(cfg.Webhook, log.Named("dispatcher.webhook")),
128128
}
129129
}

‎coderd/notifications/manager_test.go

+3-2
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"encoding/json"
66
"sync/atomic"
77
"testing"
8+
"text/template"
89
"time"
910

1011
"github.com/google/uuid"
@@ -210,8 +211,8 @@ type santaHandler struct {
210211
nice atomic.Int32
211212
}
212213

213-
func (s *santaHandler) Dispatcher(payload types.MessagePayload, _, _ string) (dispatch.DeliveryFunc, error) {
214-
return func(ctx context.Context, msgID uuid.UUID) (retryable bool, err error) {
214+
func (s *santaHandler) Dispatcher(payload types.MessagePayload, _, _ string, _ template.FuncMap) (dispatch.DeliveryFunc, error) {
215+
return func(_ context.Context, _ uuid.UUID) (retryable bool, err error) {
215216
if payload.Labels["nice"] != "true" {
216217
s.naughty.Add(1)
217218
return false, xerrors.New("be nice")

‎coderd/notifications/metrics_test.go

+18-17
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"strconv"
66
"sync"
77
"testing"
8+
"text/template"
89
"time"
910

1011
"github.com/google/uuid"
@@ -44,7 +45,7 @@ func TestMetrics(t *testing.T) {
4445

4546
reg := prometheus.NewRegistry()
4647
metrics := notifications.NewMetrics(reg)
47-
template := notifications.TemplateWorkspaceDeleted
48+
tmpl := notifications.TemplateWorkspaceDeleted
4849

4950
const (
5051
method = database.NotificationMethodSmtp
@@ -76,7 +77,7 @@ func TestMetrics(t *testing.T) {
7677
user := createSampleUser(t, store)
7778

7879
// Build fingerprints for the two different series we expect.
79-
methodTemplateFP := fingerprintLabels(notifications.LabelMethod, string(method), notifications.LabelTemplateID, template.String())
80+
methodTemplateFP := fingerprintLabels(notifications.LabelMethod, string(method), notifications.LabelTemplateID, tmpl.String())
8081
methodFP := fingerprintLabels(notifications.LabelMethod, string(method))
8182

8283
expected := map[string]func(metric *dto.Metric, series string) bool{
@@ -90,7 +91,7 @@ func TestMetrics(t *testing.T) {
9091

9192
var match string
9293
for result, val := range results {
93-
seriesFP := fingerprintLabels(notifications.LabelMethod, string(method), notifications.LabelTemplateID, template.String(), notifications.LabelResult, result)
94+
seriesFP := fingerprintLabels(notifications.LabelMethod, string(method), notifications.LabelTemplateID, tmpl.String(), notifications.LabelResult, result)
9495
if !hasMatchingFingerprint(metric, seriesFP) {
9596
continue
9697
}
@@ -165,9 +166,9 @@ func TestMetrics(t *testing.T) {
165166
}
166167

167168
// WHEN: 2 notifications are enqueued, 1 of which will fail until its retries are exhausted, and another which will succeed
168-
_, err = enq.Enqueue(ctx, user.ID, template, map[string]string{"type": "success"}, "test") // this will succeed
169+
_, err = enq.Enqueue(ctx, user.ID, tmpl, map[string]string{"type": "success"}, "test") // this will succeed
169170
require.NoError(t, err)
170-
_, err = enq.Enqueue(ctx, user.ID, template, map[string]string{"type": "failure"}, "test2") // this will fail and retry (maxAttempts - 1) times
171+
_, err = enq.Enqueue(ctx, user.ID, tmpl, map[string]string{"type": "failure"}, "test2") // this will fail and retry (maxAttempts - 1) times
171172
require.NoError(t, err)
172173

173174
mgr.Run(ctx)
@@ -218,7 +219,7 @@ func TestPendingUpdatesMetric(t *testing.T) {
218219

219220
reg := prometheus.NewRegistry()
220221
metrics := notifications.NewMetrics(reg)
221-
template := notifications.TemplateWorkspaceDeleted
222+
tmpl := notifications.TemplateWorkspaceDeleted
222223

223224
const method = database.NotificationMethodSmtp
224225

@@ -252,9 +253,9 @@ func TestPendingUpdatesMetric(t *testing.T) {
252253
user := createSampleUser(t, store)
253254

254255
// WHEN: 2 notifications are enqueued, one of which will fail and one which will succeed
255-
_, err = enq.Enqueue(ctx, user.ID, template, map[string]string{"type": "success"}, "test") // this will succeed
256+
_, err = enq.Enqueue(ctx, user.ID, tmpl, map[string]string{"type": "success"}, "test") // this will succeed
256257
require.NoError(t, err)
257-
_, err = enq.Enqueue(ctx, user.ID, template, map[string]string{"type": "failure"}, "test2") // this will fail and retry (maxAttempts - 1) times
258+
_, err = enq.Enqueue(ctx, user.ID, tmpl, map[string]string{"type": "failure"}, "test2") // this will fail and retry (maxAttempts - 1) times
258259
require.NoError(t, err)
259260

260261
mgr.Run(ctx)
@@ -309,7 +310,7 @@ func TestInflightDispatchesMetric(t *testing.T) {
309310

310311
reg := prometheus.NewRegistry()
311312
metrics := notifications.NewMetrics(reg)
312-
template := notifications.TemplateWorkspaceDeleted
313+
tmpl := notifications.TemplateWorkspaceDeleted
313314

314315
const method = database.NotificationMethodSmtp
315316

@@ -342,7 +343,7 @@ func TestInflightDispatchesMetric(t *testing.T) {
342343

343344
// WHEN: notifications are enqueued which will succeed (and be delayed during dispatch)
344345
for i := 0; i < msgCount; i++ {
345-
_, err = enq.Enqueue(ctx, user.ID, template, map[string]string{"type": "success", "i": strconv.Itoa(i)}, "test")
346+
_, err = enq.Enqueue(ctx, user.ID, tmpl, map[string]string{"type": "success", "i": strconv.Itoa(i)}, "test")
346347
require.NoError(t, err)
347348
}
348349

@@ -351,7 +352,7 @@ func TestInflightDispatchesMetric(t *testing.T) {
351352
// THEN:
352353
// Ensure we see the dispatches of the messages inflight.
353354
require.Eventually(t, func() bool {
354-
return promtest.ToFloat64(metrics.InflightDispatches.WithLabelValues(string(method), template.String())) == msgCount
355+
return promtest.ToFloat64(metrics.InflightDispatches.WithLabelValues(string(method), tmpl.String())) == msgCount
355356
}, testutil.WaitShort, testutil.IntervalFast)
356357

357358
for i := 0; i < msgCount; i++ {
@@ -389,7 +390,7 @@ func TestCustomMethodMetricCollection(t *testing.T) {
389390
var (
390391
reg = prometheus.NewRegistry()
391392
metrics = notifications.NewMetrics(reg)
392-
template = notifications.TemplateWorkspaceDeleted
393+
tmpl = notifications.TemplateWorkspaceDeleted
393394
anotherTemplate = notifications.TemplateWorkspaceDormant
394395
)
395396

@@ -400,7 +401,7 @@ func TestCustomMethodMetricCollection(t *testing.T) {
400401

401402
// GIVEN: a template whose notification method differs from the default.
402403
out, err := store.UpdateNotificationTemplateMethodByID(ctx, database.UpdateNotificationTemplateMethodByIDParams{
403-
ID: template,
404+
ID: tmpl,
404405
Method: database.NullNotificationMethod{NotificationMethod: customMethod, Valid: true},
405406
})
406407
require.NoError(t, err)
@@ -426,7 +427,7 @@ func TestCustomMethodMetricCollection(t *testing.T) {
426427

427428
user := createSampleUser(t, store)
428429

429-
_, err = enq.Enqueue(ctx, user.ID, template, map[string]string{"type": "success"}, "test")
430+
_, err = enq.Enqueue(ctx, user.ID, tmpl, map[string]string{"type": "success"}, "test")
430431
require.NoError(t, err)
431432
_, err = enq.Enqueue(ctx, user.ID, anotherTemplate, map[string]string{"type": "success"}, "test")
432433
require.NoError(t, err)
@@ -447,7 +448,7 @@ func TestCustomMethodMetricCollection(t *testing.T) {
447448
// THEN: we should have metric series for both the default and custom notification methods.
448449
require.Eventually(t, func() bool {
449450
return promtest.ToFloat64(metrics.DispatchAttempts.WithLabelValues(string(defaultMethod), anotherTemplate.String(), notifications.ResultSuccess)) > 0 &&
450-
promtest.ToFloat64(metrics.DispatchAttempts.WithLabelValues(string(customMethod), template.String(), notifications.ResultSuccess)) > 0
451+
promtest.ToFloat64(metrics.DispatchAttempts.WithLabelValues(string(customMethod), tmpl.String(), notifications.ResultSuccess)) > 0
451452
}, testutil.WaitShort, testutil.IntervalFast)
452453
}
453454

@@ -525,8 +526,8 @@ func newBarrierHandler(total int, handler notifications.Handler) *barrierHandler
525526
}
526527
}
527528

528-
func (bh *barrierHandler) Dispatcher(payload types.MessagePayload, title, body string) (dispatch.DeliveryFunc, error) {
529-
deliverFn, err := bh.h.Dispatcher(payload, title, body)
529+
func (bh *barrierHandler) Dispatcher(payload types.MessagePayload, title, body string, helpers template.FuncMap) (dispatch.DeliveryFunc, error) {
530+
deliverFn, err := bh.h.Dispatcher(payload, title, body, helpers)
530531
if err != nil {
531532
return nil, err
532533
}

0 commit comments

Comments
 (0)