From 898014c8a650390b403e75e7fea64a0c5b689705 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Wed, 7 Aug 2024 16:09:14 +0000 Subject: [PATCH 1/8] Update template email --- cli/server.go | 3 +- coderd/notifications/dispatch/smtp.go | 11 +- .../notifications/dispatch/smtp/html.gotmpl | 107 ++++++++++++++---- coderd/notifications/manager.go | 8 +- 4 files changed, 100 insertions(+), 29 deletions(-) diff --git a/cli/server.go b/cli/server.go index f76872a78c342..74cd0c5aed472 100644 --- a/cli/server.go +++ b/cli/server.go @@ -992,6 +992,7 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd. ) if experiments.Enabled(codersdk.ExperimentNotifications) { cfg := options.DeploymentValues.Notifications + accessUrl := options.DeploymentValues.AccessURL.Value().String() metrics := notifications.NewMetrics(options.PrometheusRegistry) // The enqueuer is responsible for enqueueing notifications to the given store. @@ -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, accessUrl, metrics, logger.Named("notifications.manager")) if err != nil { return xerrors.Errorf("failed to instantiate notification manager: %w", err) } diff --git a/coderd/notifications/dispatch/smtp.go b/coderd/notifications/dispatch/smtp.go index 218668e65d02e..f7048ac6f0861 100644 --- a/coderd/notifications/dispatch/smtp.go +++ b/coderd/notifications/dispatch/smtp.go @@ -49,14 +49,15 @@ var ( // NOTE: auth and TLS is currently *not* enabled in this initial thin slice. // TODO: implement DKIM/SPF/DMARC? https://github.com/emersion/go-msgauth type SMTPHandler struct { - cfg codersdk.NotificationsEmailConfig - log slog.Logger + cfg codersdk.NotificationsEmailConfig + log slog.Logger + accessURL string loginWarnOnce sync.Once } -func NewSMTPHandler(cfg codersdk.NotificationsEmailConfig, log slog.Logger) *SMTPHandler { - return &SMTPHandler{cfg: cfg, log: log} +func NewSMTPHandler(cfg codersdk.NotificationsEmailConfig, accessURL string, log slog.Logger) *SMTPHandler { + return &SMTPHandler{cfg: cfg, accessURL: accessURL, log: log} } func (s *SMTPHandler) Dispatcher(payload types.MessagePayload, titleTmpl, bodyTmpl string) (DeliveryFunc, error) { @@ -75,6 +76,8 @@ 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 + payload.Labels["_year"] = time.Now().Format("2006") + payload.Labels["_accessURL"] = s.accessURL htmlBody, err = render.GoTemplate(htmlTemplate, payload, nil) if err != nil { return nil, xerrors.Errorf("render full html template: %w", err) diff --git a/coderd/notifications/dispatch/smtp/html.gotmpl b/coderd/notifications/dispatch/smtp/html.gotmpl index 00005179316bf..9b1eac52233cb 100644 --- a/coderd/notifications/dispatch/smtp/html.gotmpl +++ b/coderd/notifications/dispatch/smtp/html.gotmpl @@ -1,27 +1,94 @@ - + - - - + + + {{ .Labels._subject }} - - -
-
- -
-
-

{{ .Labels._subject }}

- {{ .Labels._body }} + + + +
+ +

{{ .Labels._subject }}

+ {{ .Labels._body }} + +
{{ range $action := .Actions }} - {{ $action.Label }}
+ {{ $action.Label }} {{ end }} -
-
+
+ +
- © 2024 Coder. All rights reserved. +

© 2024 Coder. All rights reserved.

+

+ Click here to manage your notification settings +

+
-
- - \ No newline at end of file + + diff --git a/coderd/notifications/manager.go b/coderd/notifications/manager.go index 91580d5fc4fb7..659d19b016049 100644 --- a/coderd/notifications/manager.go +++ b/coderd/notifications/manager.go @@ -59,7 +59,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, host string, 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 +93,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, host, 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, accessURL string, 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, accessURL, log.Named("dispatcher.smtp")), database.NotificationMethodWebhook: dispatch.NewWebhookHandler(cfg.Webhook, log.Named("dispatcher.webhook")), } } From a17381f76c4ab95574498d9c659cafacfa7601c1 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Wed, 7 Aug 2024 17:09:05 +0000 Subject: [PATCH 2/8] Pass extra data to template --- .../notifications/dispatch/smtp/html.gotmpl | 13 +++++-- coderd/notifications/dispatch/smtp_test.go | 3 +- coderd/notifications/manager_test.go | 12 +++++-- coderd/notifications/metrics_test.go | 12 ++++--- coderd/notifications/notifications_test.go | 34 ++++++++++++------- 5 files changed, 53 insertions(+), 21 deletions(-) diff --git a/coderd/notifications/dispatch/smtp/html.gotmpl b/coderd/notifications/dispatch/smtp/html.gotmpl index 9b1eac52233cb..57ee3bc608894 100644 --- a/coderd/notifications/dispatch/smtp/html.gotmpl +++ b/coderd/notifications/dispatch/smtp/html.gotmpl @@ -26,6 +26,8 @@ .logo { width: 60px; + display: block; + margin: auto; } .title { @@ -82,9 +84,16 @@
-

© 2024 Coder. All rights reserved.

- {{ .Labels._accessURL }} +

+

+ Click here to manage your notification settings

diff --git a/coderd/notifications/dispatch/smtp_test.go b/coderd/notifications/dispatch/smtp_test.go index 2605157f2b210..967bd9e5145b7 100644 --- a/coderd/notifications/dispatch/smtp_test.go +++ b/coderd/notifications/dispatch/smtp_test.go @@ -416,8 +416,9 @@ func TestSMTP(t *testing.T) { var hp serpent.HostPort require.NoError(t, hp.Set(listen.Addr().String())) tc.cfg.Smarthost = hp + accessURL := "http://localhost:8080" - handler := dispatch.NewSMTPHandler(tc.cfg, logger.Named("smtp")) + handler := dispatch.NewSMTPHandler(tc.cfg, accessURL, logger.Named("smtp")) // Start mock SMTP server in the background. var wg sync.WaitGroup diff --git a/coderd/notifications/manager_test.go b/coderd/notifications/manager_test.go index 2e264c534ccfa..2bd4acd9a8217 100644 --- a/coderd/notifications/manager_test.go +++ b/coderd/notifications/manager_test.go @@ -34,7 +34,8 @@ 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")) + accessURL := "http://localhost:8080" + mgr, err := notifications.NewManager(cfg, interceptor, accessURL, createMetrics(), logger.Named("notifications-manager")) require.NoError(t, err) mgr.WithHandlers(map[database.NotificationMethod]notifications.Handler{ database.NotificationMethodSmtp: santa, @@ -150,7 +151,14 @@ 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")) + accessURL := "http://localhost:8080" + mgr, err := notifications.NewManager( + defaultNotificationsConfig(database.NotificationMethodSmtp), + db, + accessURL, + createMetrics(), + logger.Named("notifications-manager"), + ) require.NoError(t, err) // THEN: validate that the manager can be stopped safely without Run() having been called yet diff --git a/coderd/notifications/metrics_test.go b/coderd/notifications/metrics_test.go index 139f7ae18c6c6..67dd7bee83c75 100644 --- a/coderd/notifications/metrics_test.go +++ b/coderd/notifications/metrics_test.go @@ -51,7 +51,8 @@ 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")) + accessURL := "http://localhost:8080" + mgr, err := notifications.NewManager(cfg, store, accessURL, metrics, logger.Named("manager")) require.NoError(t, err) t.Cleanup(func() { assert.NoError(t, mgr.Stop(ctx)) @@ -218,7 +219,8 @@ func TestPendingUpdatesMetric(t *testing.T) { syncer := &syncInterceptor{Store: store} interceptor := newUpdateSignallingInterceptor(syncer) - mgr, err := notifications.NewManager(cfg, interceptor, metrics, logger.Named("manager")) + accessURL := "http://localhost:8080" + mgr, err := notifications.NewManager(cfg, interceptor, accessURL, metrics, logger.Named("manager")) require.NoError(t, err) t.Cleanup(func() { assert.NoError(t, mgr.Stop(ctx)) @@ -292,7 +294,8 @@ 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")) + accessURL := "http://localhost:8080" + mgr, err := notifications.NewManager(cfg, store, accessURL, metrics, logger.Named("manager")) require.NoError(t, err) t.Cleanup(func() { assert.NoError(t, mgr.Stop(ctx)) @@ -371,7 +374,8 @@ 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")) + accessURL := "http://localhost:8080" + mgr, err := notifications.NewManager(cfg, store, accessURL, metrics, logger.Named("manager")) require.NoError(t, err) t.Cleanup(func() { assert.NoError(t, mgr.Stop(ctx)) diff --git a/coderd/notifications/notifications_test.go b/coderd/notifications/notifications_test.go index baa7174b1e08d..7f0410e79036e 100644 --- a/coderd/notifications/notifications_test.go +++ b/coderd/notifications/notifications_test.go @@ -65,7 +65,8 @@ 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")) + accessURL := "http://localhost:8080" + mgr, err := notifications.NewManager(cfg, interceptor, accessURL, createMetrics(), logger.Named("manager")) require.NoError(t, err) mgr.WithHandlers(map[database.NotificationMethod]notifications.Handler{method: handler}) t.Cleanup(func() { @@ -138,8 +139,9 @@ 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")) + accessURL := "http://localhost:8080" + handler := newDispatchInterceptor(dispatch.NewSMTPHandler(cfg.SMTP, accessURL, logger.Named("smtp"))) + mgr, err := notifications.NewManager(cfg, db, accessURL, createMetrics(), logger.Named("manager")) require.NoError(t, err) mgr.WithHandlers(map[database.NotificationMethod]notifications.Handler{method: handler}) t.Cleanup(func() { @@ -200,7 +202,8 @@ func TestWebhookDispatch(t *testing.T) { cfg.Webhook = codersdk.NotificationsWebhookConfig{ Endpoint: *serpent.URLOf(endpoint), } - mgr, err := notifications.NewManager(cfg, db, createMetrics(), logger.Named("manager")) + accessURL := "http://localhost:8080" + mgr, err := notifications.NewManager(cfg, db, accessURL, createMetrics(), logger.Named("manager")) require.NoError(t, err) t.Cleanup(func() { assert.NoError(t, mgr.Stop(ctx)) @@ -298,7 +301,8 @@ 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")) + accessURL := "http://localhost:8080" + mgr, err := notifications.NewManager(cfg, storeInterceptor, accessURL, 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 +397,8 @@ 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")) + accessURL := "http://localhost:8080" + mgr, err := notifications.NewManager(cfg, storeInterceptor, accessURL, createMetrics(), logger.Named("manager")) require.NoError(t, err) t.Cleanup(func() { assert.NoError(t, mgr.Stop(ctx)) @@ -454,7 +459,8 @@ func TestExpiredLeaseIsRequeued(t *testing.T) { mgrCtx, cancelManagerCtx := context.WithCancel(context.Background()) t.Cleanup(cancelManagerCtx) - mgr, err := notifications.NewManager(cfg, noopInterceptor, createMetrics(), logger.Named("manager")) + accessURL := "http://localhost:8080" + mgr, err := notifications.NewManager(cfg, noopInterceptor, accessURL, createMetrics(), logger.Named("manager")) require.NoError(t, err) enq, err := notifications.NewStoreEnqueuer(cfg, db, defaultHelpers(), logger.Named("enqueuer")) require.NoError(t, err) @@ -501,7 +507,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, accessURL, createMetrics(), logger.Named("manager")) require.NoError(t, err) mgr.WithHandlers(map[database.NotificationMethod]notifications.Handler{method: handler}) @@ -542,7 +548,8 @@ 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")) + accessURL := "http://localhost:8080" + _, err := notifications.NewManager(cfg, db, accessURL, 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 +567,8 @@ func TestNotifierPaused(t *testing.T) { user := createSampleUser(t, db) cfg := defaultNotificationsConfig(method) - mgr, err := notifications.NewManager(cfg, db, createMetrics(), logger.Named("manager")) + accessURL := "http://localhost:8080" + mgr, err := notifications.NewManager(cfg, db, accessURL, createMetrics(), logger.Named("manager")) require.NoError(t, err) mgr.WithHandlers(map[database.NotificationMethod]notifications.Handler{method: handler}) t.Cleanup(func() { @@ -830,8 +838,9 @@ func TestDisabledAfterEnqueue(t *testing.T) { method := database.NotificationMethodSmtp cfg := defaultNotificationsConfig(method) + accessURL := "http://localhost:8080" - mgr, err := notifications.NewManager(cfg, db, createMetrics(), logger.Named("manager")) + mgr, err := notifications.NewManager(cfg, db, accessURL, createMetrics(), logger.Named("manager")) require.NoError(t, err) t.Cleanup(func() { assert.NoError(t, mgr.Stop(ctx)) @@ -937,7 +946,8 @@ func TestCustomNotificationMethod(t *testing.T) { Endpoint: *serpent.URLOf(endpoint), } - mgr, err := notifications.NewManager(cfg, db, createMetrics(), logger.Named("manager")) + accessURL := "http://localhost:8080" + mgr, err := notifications.NewManager(cfg, db, accessURL, createMetrics(), logger.Named("manager")) require.NoError(t, err) t.Cleanup(func() { _ = mgr.Stop(ctx) From 0d347fc54218546aa894ebab707044f43f507e70 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Wed, 7 Aug 2024 17:32:48 +0000 Subject: [PATCH 3/8] Add dark mode --- .../notifications/dispatch/smtp/html.gotmpl | 38 ++++++++++++++----- 1 file changed, 29 insertions(+), 9 deletions(-) diff --git a/coderd/notifications/dispatch/smtp/html.gotmpl b/coderd/notifications/dispatch/smtp/html.gotmpl index 57ee3bc608894..64266d999176a 100644 --- a/coderd/notifications/dispatch/smtp/html.gotmpl +++ b/coderd/notifications/dispatch/smtp/html.gotmpl @@ -5,19 +5,39 @@ {{ .Labels._subject }} From ec17ad546425b1eca9af9d99b6648d9538f3a895 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Wed, 7 Aug 2024 17:58:39 +0000 Subject: [PATCH 4/8] Update colors --- .../notifications/dispatch/smtp/html.gotmpl | 54 +++++++++++++++---- 1 file changed, 43 insertions(+), 11 deletions(-) diff --git a/coderd/notifications/dispatch/smtp/html.gotmpl b/coderd/notifications/dispatch/smtp/html.gotmpl index 64266d999176a..a84017d1d4a65 100644 --- a/coderd/notifications/dispatch/smtp/html.gotmpl +++ b/coderd/notifications/dispatch/smtp/html.gotmpl @@ -9,8 +9,10 @@ --text-primary-color: #020617; --text-secondary-color: #475569; --border-color: #e2e8f0; - --bg-primary-color: #020617; - --text-bg-primary-color: #f8fafc; + --bg-body-color: #f8fafc; + --bg-card-color: #fff; + --bg-btn-color: #020617; + --text-btn-color: #f8fafc; --text-link-color: #2563eb; } @@ -18,10 +20,12 @@ :root { --text-primary-color: #f8fafc; --text-secondary-color: #94a3b8; - --border-color: #334155; - --bg-primary-color: #f8fafc; - --text-bg-primary-color: #0f172a; - --text-link-color: #3b82f6; + --border-color: #475569; + --bg-body-color: #020617; + --bg-card-color: #1e293b; + --bg-btn-color: #f8fafc; + --text-btn-color: #0f172a; + --text-link-color: #60a5fa; } } @@ -32,12 +36,14 @@ margin: 0; padding: 0; color: var(--text-primary-color); + background: var(--bg-body-color); } .card { max-width: 600px; margin: 20px auto; border: 1px solid var(--border-color); + background-color: var(--bg-card-color); padding: 60px; border-radius: 8px; line-height: 1.5; @@ -66,13 +72,14 @@ margin-top: 32px; } - .action { + .btn { display: inline-block; padding: 13px 24px; - background-color: var(--bg-primary-color); - color: var(--text-bg-primary-color); + background-color: var(--bg-btn-color); + color: var(--text-btn-color); text-decoration: none; border-radius: 8px; + font-weight: 500; } .footer { @@ -92,13 +99,38 @@
- + +

{{ .Labels._subject }}

{{ .Labels._body }}
{{ range $action := .Actions }} - {{ $action.Label }} + {{ $action.Label }} {{ end }}
From 3270cd5dc42b34423ef22d03216944c1145074c3 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Wed, 7 Aug 2024 19:13:12 +0000 Subject: [PATCH 5/8] Simplify passing data --- cli/server.go | 3 +- coderd/notifications/dispatch/smtp.go | 11 +++--- .../notifications/dispatch/smtp/html.gotmpl | 10 ++---- coderd/notifications/dispatch/smtp_test.go | 3 +- coderd/notifications/manager.go | 8 ++--- coderd/notifications/manager_test.go | 12 ++----- coderd/notifications/metrics_test.go | 12 +++---- coderd/notifications/notifications_test.go | 34 +++++++------------ coderd/notifications/render/gotmpl_test.go | 3 +- coderd/notifications/utils_test.go | 3 +- 10 files changed, 35 insertions(+), 64 deletions(-) diff --git a/cli/server.go b/cli/server.go index 74cd0c5aed472..f76872a78c342 100644 --- a/cli/server.go +++ b/cli/server.go @@ -992,7 +992,6 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd. ) if experiments.Enabled(codersdk.ExperimentNotifications) { cfg := options.DeploymentValues.Notifications - accessUrl := options.DeploymentValues.AccessURL.Value().String() metrics := notifications.NewMetrics(options.PrometheusRegistry) // The enqueuer is responsible for enqueueing notifications to the given store. @@ -1005,7 +1004,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, accessUrl, metrics, logger.Named("notifications.manager")) + notificationsManager, err = notifications.NewManager(cfg, options.Database, metrics, logger.Named("notifications.manager")) if err != nil { return xerrors.Errorf("failed to instantiate notification manager: %w", err) } diff --git a/coderd/notifications/dispatch/smtp.go b/coderd/notifications/dispatch/smtp.go index f7048ac6f0861..218668e65d02e 100644 --- a/coderd/notifications/dispatch/smtp.go +++ b/coderd/notifications/dispatch/smtp.go @@ -49,15 +49,14 @@ var ( // NOTE: auth and TLS is currently *not* enabled in this initial thin slice. // TODO: implement DKIM/SPF/DMARC? https://github.com/emersion/go-msgauth type SMTPHandler struct { - cfg codersdk.NotificationsEmailConfig - log slog.Logger - accessURL string + cfg codersdk.NotificationsEmailConfig + log slog.Logger loginWarnOnce sync.Once } -func NewSMTPHandler(cfg codersdk.NotificationsEmailConfig, accessURL string, log slog.Logger) *SMTPHandler { - return &SMTPHandler{cfg: cfg, accessURL: accessURL, 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) { @@ -76,8 +75,6 @@ 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 - payload.Labels["_year"] = time.Now().Format("2006") - payload.Labels["_accessURL"] = s.accessURL htmlBody, err = render.GoTemplate(htmlTemplate, payload, nil) if err != nil { return nil, xerrors.Errorf("render full html template: %w", err) diff --git a/coderd/notifications/dispatch/smtp/html.gotmpl b/coderd/notifications/dispatch/smtp/html.gotmpl index a84017d1d4a65..14816ed0aa511 100644 --- a/coderd/notifications/dispatch/smtp/html.gotmpl +++ b/coderd/notifications/dispatch/smtp/html.gotmpl @@ -137,15 +137,11 @@

- © {{ .Labels._year }} Coder. All rights reserved - - {{ .Labels._accessURL }} + © {{ current_year }} Coder. All rights reserved - + {{ base_url }}

- Click here to manage your notification settings

diff --git a/coderd/notifications/dispatch/smtp_test.go b/coderd/notifications/dispatch/smtp_test.go index 967bd9e5145b7..2605157f2b210 100644 --- a/coderd/notifications/dispatch/smtp_test.go +++ b/coderd/notifications/dispatch/smtp_test.go @@ -416,9 +416,8 @@ func TestSMTP(t *testing.T) { var hp serpent.HostPort require.NoError(t, hp.Set(listen.Addr().String())) tc.cfg.Smarthost = hp - accessURL := "http://localhost:8080" - handler := dispatch.NewSMTPHandler(tc.cfg, accessURL, logger.Named("smtp")) + handler := dispatch.NewSMTPHandler(tc.cfg, logger.Named("smtp")) // Start mock SMTP server in the background. var wg sync.WaitGroup diff --git a/coderd/notifications/manager.go b/coderd/notifications/manager.go index 659d19b016049..91580d5fc4fb7 100644 --- a/coderd/notifications/manager.go +++ b/coderd/notifications/manager.go @@ -59,7 +59,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, host string, metrics *Metrics, log slog.Logger) (*Manager, error) { +func NewManager(cfg codersdk.NotificationsConfig, store Store, 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 +93,14 @@ func NewManager(cfg codersdk.NotificationsConfig, store Store, host string, metr stop: make(chan any), done: make(chan any), - handlers: defaultHandlers(cfg, host, log), + handlers: defaultHandlers(cfg, 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, accessURL string, 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, accessURL, 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 index 2bd4acd9a8217..2e264c534ccfa 100644 --- a/coderd/notifications/manager_test.go +++ b/coderd/notifications/manager_test.go @@ -34,8 +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 - accessURL := "http://localhost:8080" - mgr, err := notifications.NewManager(cfg, interceptor, accessURL, createMetrics(), logger.Named("notifications-manager")) + mgr, err := notifications.NewManager(cfg, interceptor, createMetrics(), logger.Named("notifications-manager")) require.NoError(t, err) mgr.WithHandlers(map[database.NotificationMethod]notifications.Handler{ database.NotificationMethodSmtp: santa, @@ -151,14 +150,7 @@ func TestStopBeforeRun(t *testing.T) { ctx, logger, db := setupInMemory(t) // GIVEN: a standard manager - accessURL := "http://localhost:8080" - mgr, err := notifications.NewManager( - defaultNotificationsConfig(database.NotificationMethodSmtp), - db, - accessURL, - createMetrics(), - logger.Named("notifications-manager"), - ) + mgr, err := notifications.NewManager(defaultNotificationsConfig(database.NotificationMethodSmtp), db, createMetrics(), logger.Named("notifications-manager")) require.NoError(t, err) // THEN: validate that the manager can be stopped safely without Run() having been called yet diff --git a/coderd/notifications/metrics_test.go b/coderd/notifications/metrics_test.go index 67dd7bee83c75..139f7ae18c6c6 100644 --- a/coderd/notifications/metrics_test.go +++ b/coderd/notifications/metrics_test.go @@ -51,8 +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. - accessURL := "http://localhost:8080" - mgr, err := notifications.NewManager(cfg, store, accessURL, metrics, logger.Named("manager")) + mgr, err := notifications.NewManager(cfg, store, metrics, logger.Named("manager")) require.NoError(t, err) t.Cleanup(func() { assert.NoError(t, mgr.Stop(ctx)) @@ -219,8 +218,7 @@ func TestPendingUpdatesMetric(t *testing.T) { syncer := &syncInterceptor{Store: store} interceptor := newUpdateSignallingInterceptor(syncer) - accessURL := "http://localhost:8080" - mgr, err := notifications.NewManager(cfg, interceptor, accessURL, metrics, logger.Named("manager")) + mgr, err := notifications.NewManager(cfg, interceptor, metrics, logger.Named("manager")) require.NoError(t, err) t.Cleanup(func() { assert.NoError(t, mgr.Stop(ctx)) @@ -294,8 +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) - accessURL := "http://localhost:8080" - mgr, err := notifications.NewManager(cfg, store, accessURL, metrics, logger.Named("manager")) + mgr, err := notifications.NewManager(cfg, store, metrics, logger.Named("manager")) require.NoError(t, err) t.Cleanup(func() { assert.NoError(t, mgr.Stop(ctx)) @@ -374,8 +371,7 @@ func TestCustomMethodMetricCollection(t *testing.T) { // WHEN: two notifications (each with different templates) are enqueued. cfg := defaultNotificationsConfig(defaultMethod) - accessURL := "http://localhost:8080" - mgr, err := notifications.NewManager(cfg, store, accessURL, metrics, logger.Named("manager")) + mgr, err := notifications.NewManager(cfg, store, metrics, logger.Named("manager")) require.NoError(t, err) t.Cleanup(func() { assert.NoError(t, mgr.Stop(ctx)) diff --git a/coderd/notifications/notifications_test.go b/coderd/notifications/notifications_test.go index 7f0410e79036e..baa7174b1e08d 100644 --- a/coderd/notifications/notifications_test.go +++ b/coderd/notifications/notifications_test.go @@ -65,8 +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 - accessURL := "http://localhost:8080" - mgr, err := notifications.NewManager(cfg, interceptor, accessURL, createMetrics(), logger.Named("manager")) + mgr, err := notifications.NewManager(cfg, interceptor, createMetrics(), logger.Named("manager")) require.NoError(t, err) mgr.WithHandlers(map[database.NotificationMethod]notifications.Handler{method: handler}) t.Cleanup(func() { @@ -139,9 +138,8 @@ func TestSMTPDispatch(t *testing.T) { Smarthost: serpent.HostPort{Host: "localhost", Port: fmt.Sprintf("%d", mockSMTPSrv.PortNumber())}, Hello: "localhost", } - accessURL := "http://localhost:8080" - handler := newDispatchInterceptor(dispatch.NewSMTPHandler(cfg.SMTP, accessURL, logger.Named("smtp"))) - mgr, err := notifications.NewManager(cfg, db, accessURL, createMetrics(), logger.Named("manager")) + handler := newDispatchInterceptor(dispatch.NewSMTPHandler(cfg.SMTP, logger.Named("smtp"))) + mgr, err := notifications.NewManager(cfg, db, createMetrics(), logger.Named("manager")) require.NoError(t, err) mgr.WithHandlers(map[database.NotificationMethod]notifications.Handler{method: handler}) t.Cleanup(func() { @@ -202,8 +200,7 @@ func TestWebhookDispatch(t *testing.T) { cfg.Webhook = codersdk.NotificationsWebhookConfig{ Endpoint: *serpent.URLOf(endpoint), } - accessURL := "http://localhost:8080" - mgr, err := notifications.NewManager(cfg, db, accessURL, createMetrics(), logger.Named("manager")) + mgr, err := notifications.NewManager(cfg, db, createMetrics(), logger.Named("manager")) require.NoError(t, err) t.Cleanup(func() { assert.NoError(t, mgr.Stop(ctx)) @@ -301,8 +298,7 @@ func TestBackpressure(t *testing.T) { storeInterceptor := &syncInterceptor{Store: db} // GIVEN: a notification manager whose updates will be intercepted - accessURL := "http://localhost:8080" - mgr, err := notifications.NewManager(cfg, storeInterceptor, accessURL, createMetrics(), logger.Named("manager")) + mgr, err := notifications.NewManager(cfg, storeInterceptor, 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")) @@ -397,8 +393,7 @@ func TestRetries(t *testing.T) { // Intercept calls to submit the buffered updates to the store. storeInterceptor := &syncInterceptor{Store: db} - accessURL := "http://localhost:8080" - mgr, err := notifications.NewManager(cfg, storeInterceptor, accessURL, createMetrics(), logger.Named("manager")) + mgr, err := notifications.NewManager(cfg, storeInterceptor, createMetrics(), logger.Named("manager")) require.NoError(t, err) t.Cleanup(func() { assert.NoError(t, mgr.Stop(ctx)) @@ -459,8 +454,7 @@ func TestExpiredLeaseIsRequeued(t *testing.T) { mgrCtx, cancelManagerCtx := context.WithCancel(context.Background()) t.Cleanup(cancelManagerCtx) - accessURL := "http://localhost:8080" - mgr, err := notifications.NewManager(cfg, noopInterceptor, accessURL, createMetrics(), logger.Named("manager")) + mgr, err := notifications.NewManager(cfg, noopInterceptor, createMetrics(), logger.Named("manager")) require.NoError(t, err) enq, err := notifications.NewStoreEnqueuer(cfg, db, defaultHelpers(), logger.Named("enqueuer")) require.NoError(t, err) @@ -507,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, accessURL, createMetrics(), logger.Named("manager")) + mgr, err = notifications.NewManager(cfg, storeInterceptor, createMetrics(), logger.Named("manager")) require.NoError(t, err) mgr.WithHandlers(map[database.NotificationMethod]notifications.Handler{method: handler}) @@ -548,8 +542,7 @@ func TestInvalidConfig(t *testing.T) { cfg.DispatchTimeout = serpent.Duration(leasePeriod) // WHEN: the manager is created with invalid config - accessURL := "http://localhost:8080" - _, err := notifications.NewManager(cfg, db, accessURL, createMetrics(), logger.Named("manager")) + _, err := notifications.NewManager(cfg, db, createMetrics(), logger.Named("manager")) // THEN: the manager will fail to be created, citing invalid config as error require.ErrorIs(t, err, notifications.ErrInvalidDispatchTimeout) @@ -567,8 +560,7 @@ func TestNotifierPaused(t *testing.T) { user := createSampleUser(t, db) cfg := defaultNotificationsConfig(method) - accessURL := "http://localhost:8080" - mgr, err := notifications.NewManager(cfg, db, accessURL, createMetrics(), logger.Named("manager")) + mgr, err := notifications.NewManager(cfg, db, createMetrics(), logger.Named("manager")) require.NoError(t, err) mgr.WithHandlers(map[database.NotificationMethod]notifications.Handler{method: handler}) t.Cleanup(func() { @@ -838,9 +830,8 @@ func TestDisabledAfterEnqueue(t *testing.T) { method := database.NotificationMethodSmtp cfg := defaultNotificationsConfig(method) - accessURL := "http://localhost:8080" - mgr, err := notifications.NewManager(cfg, db, accessURL, createMetrics(), logger.Named("manager")) + mgr, err := notifications.NewManager(cfg, db, createMetrics(), logger.Named("manager")) require.NoError(t, err) t.Cleanup(func() { assert.NoError(t, mgr.Stop(ctx)) @@ -946,8 +937,7 @@ func TestCustomNotificationMethod(t *testing.T) { Endpoint: *serpent.URLOf(endpoint), } - accessURL := "http://localhost:8080" - mgr, err := notifications.NewManager(cfg, db, accessURL, createMetrics(), logger.Named("manager")) + mgr, err := notifications.NewManager(cfg, db, createMetrics(), logger.Named("manager")) require.NoError(t, err) t.Cleanup(func() { _ = mgr.Stop(ctx) diff --git a/coderd/notifications/render/gotmpl_test.go b/coderd/notifications/render/gotmpl_test.go index ec2ec7ffe6237..e49077cca9863 100644 --- a/coderd/notifications/render/gotmpl_test.go +++ b/coderd/notifications/render/gotmpl_test.go @@ -65,7 +65,8 @@ func TestGoTemplate(t *testing.T) { t.Parallel() out, err := render.GoTemplate(tc.in, tc.payload, map[string]any{ - "base_url": func() string { return "https://mocked-server-address" }, + "base_url": func() string { return "https://mocked-server-address" }, + "current_year": func() string { return "2024" }, }) if tc.expectedErr == nil { require.NoError(t, err) diff --git a/coderd/notifications/utils_test.go b/coderd/notifications/utils_test.go index 24cd361ede276..890317b82178b 100644 --- a/coderd/notifications/utils_test.go +++ b/coderd/notifications/utils_test.go @@ -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() int { return time.Now().Year() }, } } From bfbf974ac4337c3455c700335b777e95ae220dbb Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Thu, 8 Aug 2024 14:58:51 +0000 Subject: [PATCH 6/8] Fix data passing --- cli/server.go | 8 +- coderd/notifications/dispatch/smtp.go | 11 +- .../notifications/dispatch/smtp/html.gotmpl | 155 ++---------------- coderd/notifications/dispatch/smtp_test.go | 3 +- coderd/notifications/manager.go | 9 +- coderd/notifications/manager_test.go | 4 +- coderd/notifications/metrics_test.go | 8 +- coderd/notifications/notifications_test.go | 24 +-- coderd/notifications/render/gotmpl_test.go | 3 +- coderd/notifications/utils_test.go | 2 +- 10 files changed, 57 insertions(+), 170 deletions(-) diff --git a/cli/server.go b/cli/server.go index f76872a78c342..c8c1e9232bbe2 100644 --- a/cli/server.go +++ b/cli/server.go @@ -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()) }, } } diff --git a/coderd/notifications/dispatch/smtp.go b/coderd/notifications/dispatch/smtp.go index 218668e65d02e..e0c1b89f6154e 100644 --- a/coderd/notifications/dispatch/smtp.go +++ b/coderd/notifications/dispatch/smtp.go @@ -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) } diff --git a/coderd/notifications/dispatch/smtp/html.gotmpl b/coderd/notifications/dispatch/smtp/html.gotmpl index 14816ed0aa511..1002c4724d3ec 100644 --- a/coderd/notifications/dispatch/smtp/html.gotmpl +++ b/coderd/notifications/dispatch/smtp/html.gotmpl @@ -4,147 +4,28 @@ {{ .Labels._subject }} - - -
- - -

{{ .Labels._subject }}

- {{ .Labels._body }} - -
+ +
+
+ Coder Logo +
+

+ {{ .Labels._subject }} +

+
+ {{ .Labels._body }} +
+
{{ range $action := .Actions }} - {{ $action.Label }} + + {{ $action.Label }} + {{ end }}
- -
- -

- © {{ current_year }} Coder. All rights reserved - - {{ base_url }} -

-

- Click here to manage your notification settings -

+
+

© {{ current_year }} Coder. All rights reserved - {{ base_url }}

+

Click here to manage your notification settings

diff --git a/coderd/notifications/dispatch/smtp_test.go b/coderd/notifications/dispatch/smtp_test.go index 2605157f2b210..4531eaef327aa 100644 --- a/coderd/notifications/dispatch/smtp_test.go +++ b/coderd/notifications/dispatch/smtp_test.go @@ -9,6 +9,7 @@ import ( "net" "sync" "testing" + "text/template" "github.com/emersion/go-sasl" "github.com/emersion/go-smtp" @@ -417,7 +418,7 @@ 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")) + handler := dispatch.NewSMTPHandler(tc.cfg, template.FuncMap{}, logger.Named("smtp")) // Start mock SMTP server in the background. var wg sync.WaitGroup diff --git a/coderd/notifications/manager.go b/coderd/notifications/manager.go index 91580d5fc4fb7..7ce26ffbd40c2 100644 --- a/coderd/notifications/manager.go +++ b/coderd/notifications/manager.go @@ -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")), } } diff --git a/coderd/notifications/manager_test.go b/coderd/notifications/manager_test.go index 2e264c534ccfa..fedc7f6817d3c 100644 --- a/coderd/notifications/manager_test.go +++ b/coderd/notifications/manager_test.go @@ -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 diff --git a/coderd/notifications/metrics_test.go b/coderd/notifications/metrics_test.go index 139f7ae18c6c6..f0735f4001272 100644 --- a/coderd/notifications/metrics_test.go +++ b/coderd/notifications/metrics_test.go @@ -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)) diff --git a/coderd/notifications/notifications_test.go b/coderd/notifications/notifications_test.go index baa7174b1e08d..077018b32581c 100644 --- a/coderd/notifications/notifications_test.go +++ b/coderd/notifications/notifications_test.go @@ -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) diff --git a/coderd/notifications/render/gotmpl_test.go b/coderd/notifications/render/gotmpl_test.go index e49077cca9863..ec2ec7ffe6237 100644 --- a/coderd/notifications/render/gotmpl_test.go +++ b/coderd/notifications/render/gotmpl_test.go @@ -65,8 +65,7 @@ func TestGoTemplate(t *testing.T) { t.Parallel() out, err := render.GoTemplate(tc.in, tc.payload, map[string]any{ - "base_url": func() string { return "https://mocked-server-address" }, - "current_year": func() string { return "2024" }, + "base_url": func() string { return "https://mocked-server-address" }, }) if tc.expectedErr == nil { require.NoError(t, err) diff --git a/coderd/notifications/utils_test.go b/coderd/notifications/utils_test.go index 890317b82178b..b15d27d32afdb 100644 --- a/coderd/notifications/utils_test.go +++ b/coderd/notifications/utils_test.go @@ -78,7 +78,7 @@ func defaultNotificationsConfig(method database.NotificationMethod) codersdk.Not func defaultHelpers() map[string]any { return map[string]any{ "base_url": func() string { return "http://test.com" }, - "current_year": func() int { return time.Now().Year() }, + "current_year": func() string { return "2024" }, } } From 9ebdb6f8d7ac436a9b165d88de022dcede01e3d1 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Fri, 9 Aug 2024 13:47:34 +0000 Subject: [PATCH 7/8] Fix test --- coderd/notifications/dispatch/smtp_test.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/coderd/notifications/dispatch/smtp_test.go b/coderd/notifications/dispatch/smtp_test.go index 4531eaef327aa..dbafabc969bc4 100644 --- a/coderd/notifications/dispatch/smtp_test.go +++ b/coderd/notifications/dispatch/smtp_test.go @@ -9,7 +9,6 @@ import ( "net" "sync" "testing" - "text/template" "github.com/emersion/go-sasl" "github.com/emersion/go-smtp" @@ -418,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, template.FuncMap{}, 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 From 79ee99e125d47349186b859c12a86dfd0e1991c8 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Fri, 9 Aug 2024 14:03:22 +0000 Subject: [PATCH 8/8] Use horizontal logo --- coderd/notifications/dispatch/smtp/html.gotmpl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/coderd/notifications/dispatch/smtp/html.gotmpl b/coderd/notifications/dispatch/smtp/html.gotmpl index 1002c4724d3ec..ac0527b9742d2 100644 --- a/coderd/notifications/dispatch/smtp/html.gotmpl +++ b/coderd/notifications/dispatch/smtp/html.gotmpl @@ -8,7 +8,7 @@
- Coder Logo + Coder Logo

{{ .Labels._subject }}