Skip to content

refactor: refactor notification email templates #14208

New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Aug 9, 2024
Merged

refactor: refactor notification email templates #14208

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
898014c
Update template email
BrunoQuaresma Aug 7, 2024
a17381f
Pass extra data to template
BrunoQuaresma Aug 7, 2024
0d347fc
Add dark mode
BrunoQuaresma Aug 7, 2024
ec17ad5
Update colors
BrunoQuaresma Aug 7, 2024
3270cd5
Simplify passing data
BrunoQuaresma Aug 7, 2024
bfbf974
Fix data passing
BrunoQuaresma Aug 8, 2024
9ebdb6f
Fix test
BrunoQuaresma Aug 9, 2024
79ee99e
Use horizontal logo
BrunoQuaresma Aug 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Fix data passing
  • Loading branch information
@BrunoQuaresma
BrunoQuaresma committed Aug 8, 2024
commit bfbf974ac4337c3455c700335b777e95ae220dbb
8 changes: 5 additions & 3 deletions cli/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -993,9 +993,10 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd.
if experiments.Enabled(codersdk.ExperimentNotifications) {
cfg := options.DeploymentValues.Notifications
metrics := notifications.NewMetrics(options.PrometheusRegistry)
helpers := templateHelpers(options)

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

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

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

loginWarnOnce sync.Once

helpers template.FuncMap
}

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

func (s *SMTPHandler) Dispatcher(payload types.MessagePayload, titleTmpl, bodyTmpl string) (DeliveryFunc, error) {
Expand All @@ -75,12 +78,12 @@ func (s *SMTPHandler) Dispatcher(payload types.MessagePayload, titleTmpl, bodyTm
// Then, reuse these strings in the HTML & plain body templates.
payload.Labels["_subject"] = subject
payload.Labels["_body"] = htmlBody
htmlBody, err = render.GoTemplate(htmlTemplate, payload, nil)
htmlBody, err = render.GoTemplate(htmlTemplate, payload, s.helpers)
if err != nil {
return nil, xerrors.Errorf("render full html template: %w", err)
}
payload.Labels["_body"] = plainBody
plainBody, err = render.GoTemplate(plainTemplate, payload, nil)
plainBody, err = render.GoTemplate(plainTemplate, payload, s.helpers)
if err != nil {
return nil, xerrors.Errorf("render full plaintext template: %w", err)
}
Expand Down
155 changes: 18 additions & 137 deletions coderd/notifications/dispatch/smtp/html.gotmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,147 +4,28 @@
<meta charset="UTF-8" />
<meta name="viewport" content="width=device-width, initial-scale=1.0" />
<title>{{ .Labels._subject }}</title>
<style type="text/css">
:root {
--text-primary-color: #020617;
--text-secondary-color: #475569;
--border-color: #e2e8f0;
--bg-body-color: #f8fafc;
--bg-card-color: #fff;
--bg-btn-color: #020617;
--text-btn-color: #f8fafc;
--text-link-color: #2563eb;
}

@media (prefers-color-scheme: dark) {
:root {
--text-primary-color: #f8fafc;
--text-secondary-color: #94a3b8;
--border-color: #475569;
--bg-body-color: #020617;
--bg-card-color: #1e293b;
--bg-btn-color: #f8fafc;
--text-btn-color: #0f172a;
--text-link-color: #60a5fa;
}
}

body {
font-family: -apple-system, system-ui, BlinkMacSystemFont, "Segoe UI",
"Roboto", "Oxygen", "Ubuntu", "Cantarell", "Fira Sans", "Droid Sans",
"Helvetica Neue", sans-serif;
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;
font-size: 14px;
}

.logo {
width: 60px;
display: block;
margin: auto;
}

.title {
text-align: center;
font-weight: 400;
font-size: 24px;
margin: 8px 0 32px;
line-height: 1.5;
}

.actions {
display: flex;
align-items: center;
justify-content: center;
gap: 8px;
margin-top: 32px;
}

.btn {
display: inline-block;
padding: 13px 24px;
background-color: var(--bg-btn-color);
color: var(--text-btn-color);
text-decoration: none;
border-radius: 8px;
font-weight: 500;
}

.footer {
border-top: 1px solid var(--border-color);
color: var(--text-secondary-color);
font-size: 12px;
margin-top: 64px;
padding-top: 24px;
line-height: 1.6;
}

.link {
color: var(--text-link-color);
text-decoration: none;
}
</style>
</head>
<body>
<div class="card">
<svg
class="logo"
viewBox="0 0 509 358"
fill="currentColor"
xmlns="http://www.w3.org/2000/svg"
>
<path
d="M496.593 155.299C486.451 155.299 479.693 149.445 479.693 137.427V68.4055C479.693 24.3424 461.254 0 413.623 0H391.497V46.5278H398.258C417.003 46.5278 425.915 56.6964 425.915 74.876V135.887C425.915 162.386 433.904 173.171 451.421 178.717C433.904 183.956 425.915 195.048 425.915 221.547C425.915 236.646 425.915 251.744 425.915 266.844C425.915 279.476 425.915 291.802 422.534 304.435C419.154 316.144 413.623 327.237 405.94 336.788C401.638 342.336 396.721 346.957 391.191 351.272V357.434H413.315C460.947 357.434 479.385 333.091 479.385 289.028V220.007C479.385 207.681 485.837 202.135 496.286 202.135H508.886V155.607H496.593V155.299Z"
/>
<path
d="M346.022 70.2636H277.801C276.264 70.2636 275.036 69.0309 275.036 67.4903V62.2525C275.036 60.7114 276.264 59.4792 277.801 59.4792H346.329C347.865 59.4792 349.094 60.7114 349.094 62.2525V67.4903C349.094 69.0309 347.557 70.2636 346.022 70.2636Z"
/>
<path
d="M357.694 136.818H307.912C306.375 136.818 305.145 135.584 305.145 134.044V128.806C305.145 127.266 306.375 126.033 307.912 126.033H357.694C359.231 126.033 360.459 127.266 360.459 128.806V134.044C360.459 135.277 359.231 136.818 357.694 136.818Z"
/>
<path
d="M377.365 103.54H277.801C276.264 103.54 275.036 102.308 275.036 100.767V95.5288C275.036 93.9882 276.264 92.7559 277.801 92.7559H377.058C378.595 92.7559 379.824 93.9882 379.824 95.5288V100.767C379.824 102 378.902 103.54 377.365 103.54Z"
/>
<path
d="M198.821 85.3529C205.581 85.3529 212.342 85.9693 218.795 87.5099V74.876C218.795 57.0043 228.014 46.5278 246.452 46.5278H253.213V0H231.087C183.455 0 165.018 24.3424 165.018 68.4055V91.2071C175.773 87.5099 187.144 85.3529 198.821 85.3529Z"
/>
<path
d="M398.259 252.97C393.342 213.837 363.226 181.175 324.507 173.78C313.752 171.623 302.996 171.314 292.548 173.163C292.241 173.163 292.241 172.855 291.934 172.855C275.032 137.42 238.771 114.002 199.437 114.002C160.102 114.002 124.149 136.804 106.94 172.239C106.632 172.239 106.632 172.548 106.325 172.548C95.2627 171.314 84.1998 171.93 73.1369 174.704C35.032 183.947 6.14596 215.994 0.921893 254.818C0.307298 258.824 0 262.829 0 266.528C0 278.236 7.98976 289.021 19.667 290.562C34.11 292.719 46.7093 281.626 46.402 267.452C46.402 265.295 46.402 262.829 46.7093 260.673C49.1678 240.952 64.2253 224.314 83.8921 219.691C90.0385 218.15 96.1843 217.843 102.023 218.767C120.768 221.232 139.206 211.68 147.196 195.041C153.035 182.716 162.254 171.93 174.546 166.076C188.066 159.605 203.432 158.681 217.568 163.612C232.317 168.849 243.38 179.942 250.141 193.808C257.208 207.367 260.588 216.918 275.647 218.767C281.792 219.691 299.001 219.383 305.455 219.075C318.054 219.075 330.653 223.389 339.565 232.325C345.403 238.487 349.705 246.191 351.549 254.818C354.315 268.684 350.935 282.55 342.637 293.027C336.798 300.422 328.809 305.968 319.897 308.433C315.595 309.666 311.293 309.974 306.991 309.974C304.533 309.974 301.152 309.974 297.158 309.974C284.866 309.974 258.745 309.974 239.077 309.974C225.557 309.974 214.801 299.19 214.801 285.631V195.348C214.801 191.651 211.729 188.57 208.041 188.57H198.515C179.769 188.878 164.712 209.832 164.712 232.016C164.712 254.202 164.712 313.056 164.712 313.056C164.712 337.09 184.071 356.502 208.041 356.502C208.041 356.502 314.674 356.193 316.21 356.193C340.793 353.729 363.534 341.095 378.898 321.683C394.264 302.887 401.331 278.236 398.259 252.97Z"
/>
</svg>

<h1 class="title">{{ .Labels._subject }}</h1>
{{ .Labels._body }}

<div class="actions">
<body style="margin: 0; padding: 0; font-family: -apple-system, system-ui, BlinkMacSystemFont, 'Segoe UI', 'Roboto', 'Oxygen', 'Ubuntu', 'Cantarell', 'Fira Sans', 'Droid Sans', 'Helvetica Neue', sans-serif; color: #020617; background: #f8fafc;">
<div style="max-width: 600px; margin: 20px auto; padding: 60px; border: 1px solid #e2e8f0; border-radius: 8px; background-color: #fff; text-align: left; font-size: 14px; line-height: 1.5;">
<div style="text-align: center;">
<img src="https://coder.com/coder-logo-300x300.png" alt="Coder Logo" style="width: 60px;" />
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we probably want a logo with our name in it, though.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm... I like the clean aspect of the icon, but I don't have a strong opinion. Why do you prefer the full logo instead of the icon?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Our logo is not very recognisable.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe for people who are not using the product, it could be less recognizable. For example, Vercel's logo is a generic triangle, but their users recognize it because they use the product, and I think it's the same for Coder. In the UI, for example, we only use the icon, and I think people know it's Coder. My preference would be to use only the icon for visual consistency and a cleaner look. What do you think, @stirby?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's important that folks know that these emails are coming from coder so that they don't trash them or mark them as spam.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the footer, we include the Coder company name and the deployment URL. In my opinion, the subject is the most important information, and as I mentioned before, the notification emails will be sent to Coder users who are already familiar with our branding and context. However, since this isn't a blocker, I'll replace the icon with the horizontal logo that includes the name, as you suggested. We can revisit this discussion later if we think it adds value. Does that sound like a good plan?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SGTM 👍

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Screenshot 2024-08-09 at 11 02 21

</div>
<h1 style="text-align: center; font-size: 24px; font-weight: 400; margin: 8px 0 32px; line-height: 1.5;">
{{ .Labels._subject }}
</h1>
<div style="line-height: 1.5;">
{{ .Labels._body }}
</div>
<div style="text-align: center; margin-top: 32px;">
{{ range $action := .Actions }}
<a href="{{ $action.URL }}" class="btn">{{ $action.Label }}</a>
<a href="{{ $action.URL }}" style="display: inline-block; padding: 13px 24px; background-color: #020617; color: #f8fafc; text-decoration: none; border-radius: 8px; margin: 0 4px;">
{{ $action.Label }}
</a>
{{ end }}
</div>

<div class="footer">
<!-- TODO: dynamic copyright -->
<p>
&copy; {{ current_year }} Coder. All rights reserved -
<a href="{{ base_url }}" class="link">{{ base_url }}</a>
</p>
<p>
<a href="{{ base_url }}/settings/notifications" class="link"
>Click here to manage your notification settings</a
>
</p>
<div style="border-top: 1px solid #e2e8f0; color: #475569; font-size: 12px; margin-top: 64px; padding-top: 24px; line-height: 1.6;">
<p>&copy;&nbsp;{{ current_year }}&nbsp;Coder. All rights reserved&nbsp;-&nbsp;<a href="{{ base_url }}" style="color: #2563eb; text-decoration: none;">{{ base_url }}</a></p>
<p><a href="{{ base_url }}/settings/notifications" style="color: #2563eb; text-decoration: none;">Click here to manage your notification settings</a></p>
</div>
</div>
</body>
Expand Down
3 changes: 2 additions & 1 deletion coderd/notifications/dispatch/smtp_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"net"
"sync"
"testing"
"text/template"

"github.com/emersion/go-sasl"
"github.com/emersion/go-smtp"
Expand Down Expand Up @@ -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
Expand Down
9 changes: 5 additions & 4 deletions coderd/notifications/manager.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package notifications
import (
"context"
"sync"
"text/template"
"time"

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

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

// defaultHandlers builds a set of known handlers; panics if any error occurs as these handlers should be valid at compile time.
func defaultHandlers(cfg codersdk.NotificationsConfig, log slog.Logger) map[database.NotificationMethod]Handler {
func defaultHandlers(cfg codersdk.NotificationsConfig, helpers template.FuncMap, log slog.Logger) map[database.NotificationMethod]Handler {
return map[database.NotificationMethod]Handler{
database.NotificationMethodSmtp: dispatch.NewSMTPHandler(cfg.SMTP, log.Named("dispatcher.smtp")),
database.NotificationMethodSmtp: dispatch.NewSMTPHandler(cfg.SMTP, helpers, log.Named("dispatcher.smtp")),
database.NotificationMethodWebhook: dispatch.NewWebhookHandler(cfg.Webhook, log.Named("dispatcher.webhook")),
}
}
Expand Down
4 changes: 2 additions & 2 deletions coderd/notifications/manager_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ func TestBufferedUpdates(t *testing.T) {
cfg.StoreSyncInterval = serpent.Duration(time.Hour) // Ensure we don't sync the store automatically.

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

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

// THEN: validate that the manager can be stopped safely without Run() having been called yet
Expand Down
8 changes: 4 additions & 4 deletions coderd/notifications/metrics_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ func TestMetrics(t *testing.T) {
cfg.RetryInterval = serpent.Duration(time.Millisecond * 50)
cfg.StoreSyncInterval = serpent.Duration(time.Millisecond * 100) // Twice as long as fetch interval to ensure we catch pending updates.

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

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

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

// WHEN: two notifications (each with different templates) are enqueued.
cfg := defaultNotificationsConfig(defaultMethod)
mgr, err := notifications.NewManager(cfg, store, metrics, logger.Named("manager"))
mgr, err := notifications.NewManager(cfg, store, defaultHelpers(), metrics, logger.Named("manager"))
require.NoError(t, err)
t.Cleanup(func() {
assert.NoError(t, mgr.Stop(ctx))
Expand Down
Loading
Loading