Skip to content

Commit c6076d2

Browse files
dannykoppingdannykopping
authored
chore: improve notification template tests' resilience (coder#14196)
1 parent e09ad1d commit c6076d2

6 files changed

+114
-16
lines changed

‎coderd/autobuild/lifecycle_executor.go

+6-4
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
"golang.org/x/xerrors"
1414

1515
"cdr.dev/slog"
16+
1617
"github.com/coder/coder/v2/coderd/audit"
1718
"github.com/coder/coder/v2/coderd/database"
1819
"github.com/coder/coder/v2/coderd/database/dbauthz"
@@ -296,10 +297,11 @@ func (e *Executor) runOnce(t time.Time) Stats {
296297

297298
if _, err := e.notificationsEnqueuer.Enqueue(e.ctx, ws.OwnerID, notifications.TemplateWorkspaceAutoUpdated,
298299
map[string]string{
299-
"name": ws.Name,
300-
"initiator": "autobuild",
301-
"reason": nextBuildReason,
302-
"template_version_name": activeTemplateVersion.Name,
300+
"name": ws.Name,
301+
"initiator": "autobuild",
302+
"reason": nextBuildReason,
303+
"template_version_name": activeTemplateVersion.Name,
304+
"template_version_message": activeTemplateVersion.Message,
303305
}, "autobuild",
304306
// Associate this notification with all the related entities.
305307
ws.ID, ws.OwnerID, ws.TemplateID, ws.OrganizationID,

‎coderd/database/migrations/000240_notification_workspace_updated_version_message.down.sql

+4
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
UPDATE notification_templates
2+
SET body_template = E'Hi {{.UserName}}\n' ||
3+
E'Your workspace **{{.Labels.name}}** has been updated automatically to the latest template version ({{.Labels.template_version_name}}).'
4+
WHERE id = 'c34a0c09-0704-4cac-bd1c-0c0146811c2b';

‎coderd/database/migrations/000240_notification_workspace_updated_version_message.up.sql

+6
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
UPDATE notification_templates
2+
SET name = 'Workspace Updated Automatically', -- drive-by fix for capitalization to match other templates
3+
body_template = E'Hi {{.UserName}}\n' ||
4+
E'Your workspace **{{.Labels.name}}** has been updated automatically to the latest template version ({{.Labels.template_version_name}}).\n' ||
5+
E'Reason for update: **{{.Labels.template_version_message}}**' -- include template version message
6+
WHERE id = 'c34a0c09-0704-4cac-bd1c-0c0146811c2b';

‎coderd/notifications/events.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ package notifications
33
import "github.com/google/uuid"
44

55
// These vars are mapped to UUIDs in the notification_templates table.
6-
// TODO: autogenerate these.
6+
// TODO: autogenerate these: https://github.com/coder/team-coconut/issues/36
77

88
// Workspace-related events.
99
var (

‎coderd/notifications/notifications_test.go

+87-10
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,14 @@
11
package notifications_test
22

33
import (
4+
"bytes"
45
"context"
6+
_ "embed"
57
"encoding/json"
68
"fmt"
9+
"go/ast"
10+
"go/parser"
11+
"go/token"
712
"net/http"
813
"net/http/httptest"
914
"net/url"
@@ -606,7 +611,42 @@ func TestNotifierPaused(t *testing.T) {
606611
}, testutil.WaitShort, testutil.IntervalFast)
607612
}
608613

609-
func TestNotificationTemplatesBody(t *testing.T) {
614+
//go:embed events.go
615+
var events []byte
616+
617+
// enumerateAllTemplates gets all the template names from the coderd/notifications/events.go file.
618+
// TODO(dannyk): use code-generation to create a list of all templates: https://github.com/coder/team-coconut/issues/36
619+
func enumerateAllTemplates(t *testing.T) ([]string, error) {
620+
t.Helper()
621+
622+
fset := token.NewFileSet()
623+
624+
node, err := parser.ParseFile(fset, "", bytes.NewBuffer(events), parser.AllErrors)
625+
if err != nil {
626+
return nil, err
627+
}
628+
629+
var out []string
630+
// Traverse the AST and extract variable names.
631+
ast.Inspect(node, func(n ast.Node) bool {
632+
// Check if the node is a declaration statement.
633+
if decl, ok := n.(*ast.GenDecl); ok && decl.Tok == token.VAR {
634+
for _, spec := range decl.Specs {
635+
// Type assert the spec to a ValueSpec.
636+
if valueSpec, ok := spec.(*ast.ValueSpec); ok {
637+
for _, name := range valueSpec.Names {
638+
out = append(out, name.String())
639+
}
640+
}
641+
}
642+
}
643+
return true
644+
})
645+
646+
return out, nil
647+
}
648+
649+
func TestNotificationTemplatesCanRender(t *testing.T) {
610650
t.Parallel()
611651

612652
if !dbtestutil.WillUsePostgres() {
@@ -647,10 +687,11 @@ func TestNotificationTemplatesBody(t *testing.T) {
647687
payload: types.MessagePayload{
648688
UserName: "bobby",
649689
Labels: map[string]string{
650-
"name": "bobby-workspace",
651-
"reason": "breached the template's threshold for inactivity",
652-
"initiator": "autobuild",
653-
"dormancyHours": "24",
690+
"name": "bobby-workspace",
691+
"reason": "breached the template's threshold for inactivity",
692+
"initiator": "autobuild",
693+
"dormancyHours": "24",
694+
"timeTilDormant": "24h",
654695
},
655696
},
656697
},
@@ -660,8 +701,9 @@ func TestNotificationTemplatesBody(t *testing.T) {
660701
payload: types.MessagePayload{
661702
UserName: "bobby",
662703
Labels: map[string]string{
663-
"name": "bobby-workspace",
664-
"template_version_name": "1.0",
704+
"name": "bobby-workspace",
705+
"template_version_name": "1.0",
706+
"template_version_message": "template now includes catnip",
665707
},
666708
},
667709
},
@@ -671,12 +713,46 @@ func TestNotificationTemplatesBody(t *testing.T) {
671713
payload: types.MessagePayload{
672714
UserName: "bobby",
673715
Labels: map[string]string{
674-
"name": "bobby-workspace",
675-
"reason": "template updated to new dormancy policy",
676-
"dormancyHours": "24",
716+
"name": "bobby-workspace",
717+
"reason": "template updated to new dormancy policy",
718+
"dormancyHours": "24",
719+
"timeTilDormant": "24h",
720+
},
721+
},
722+
},
723+
{
724+
name: "TemplateUserAccountCreated",
725+
id: notifications.TemplateUserAccountCreated,
726+
payload: types.MessagePayload{
727+
UserName: "bobby",
728+
Labels: map[string]string{
729+
"created_account_name": "bobby",
677730
},
678731
},
679732
},
733+
{
734+
name: "TemplateUserAccountDeleted",
735+
id: notifications.TemplateUserAccountDeleted,
736+
payload: types.MessagePayload{
737+
UserName: "bobby",
738+
Labels: map[string]string{
739+
"deleted_account_name": "bobby",
740+
},
741+
},
742+
},
743+
}
744+
745+
allTemplates, err := enumerateAllTemplates(t)
746+
require.NoError(t, err)
747+
for _, name := range allTemplates {
748+
var found bool
749+
for _, tc := range tests {
750+
if tc.name == name {
751+
found = true
752+
}
753+
}
754+
755+
require.Truef(t, found, "could not find test case for %q", name)
680756
}
681757

682758
for _, tc := range tests {
@@ -697,6 +773,7 @@ func TestNotificationTemplatesBody(t *testing.T) {
697773
require.NoError(t, err, "failed to query body template for template:", tc.id)
698774

699775
title, err := render.GoTemplate(titleTmpl, tc.payload, nil)
776+
require.NotContainsf(t, title, render.NoValue, "template %q is missing a label value", tc.name)
700777
require.NoError(t, err, "failed to render notification title template")
701778
require.NotEmpty(t, title, "title should not be empty")
702779

‎coderd/notifications/render/gotmpl.go

+10-1
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,19 @@ import (
99
"github.com/coder/coder/v2/coderd/notifications/types"
1010
)
1111

12+
// NoValue is used when a template variable is not found.
13+
// This string is not exported as a const from the text/template.
14+
const NoValue = "<no value>"
15+
1216
// GoTemplate attempts to substitute the given payload into the given template using Go's templating syntax.
1317
// TODO: memoize templates for memory efficiency?
1418
func GoTemplate(in string, payload types.MessagePayload, extraFuncs template.FuncMap) (string, error) {
15-
tmpl, err := template.New("text").Funcs(extraFuncs).Parse(in)
19+
tmpl, err := template.New("text").
20+
Funcs(extraFuncs).
21+
// text/template substitutes a missing label with "<no value>".
22+
// NOTE: html/template does not, for obvious reasons.
23+
Option("missingkey=invalid").
24+
Parse(in)
1625
if err != nil {
1726
return "", xerrors.Errorf("template parse: %w", err)
1827
}

0 commit comments

Comments
 (0)