Skip to content

chore: improve notification template tests' resilience #14196

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.

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 2 commits into from
Aug 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 6 additions & 4 deletions coderd/autobuild/lifecycle_executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"golang.org/x/xerrors"

"cdr.dev/slog"

"github.com/coder/coder/v2/coderd/audit"
"github.com/coder/coder/v2/coderd/database"
"github.com/coder/coder/v2/coderd/database/dbauthz"
Expand Down Expand Up @@ -296,10 +297,11 @@ func (e *Executor) runOnce(t time.Time) Stats {

if _, err := e.notificationsEnqueuer.Enqueue(e.ctx, ws.OwnerID, notifications.TemplateWorkspaceAutoUpdated,
map[string]string{
"name": ws.Name,
"initiator": "autobuild",
"reason": nextBuildReason,
"template_version_name": activeTemplateVersion.Name,
"name": ws.Name,
"initiator": "autobuild",
"reason": nextBuildReason,
"template_version_name": activeTemplateVersion.Name,
"template_version_message": activeTemplateVersion.Message,
}, "autobuild",
// Associate this notification with all the related entities.
ws.ID, ws.OwnerID, ws.TemplateID, ws.OrganizationID,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
UPDATE notification_templates
SET body_template = E'Hi {{.UserName}}\n' ||
E'Your workspace **{{.Labels.name}}** has been updated automatically to the latest template version ({{.Labels.template_version_name}}).'
WHERE id = 'c34a0c09-0704-4cac-bd1c-0c0146811c2b';
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
UPDATE notification_templates
SET name = 'Workspace Updated Automatically', -- drive-by fix for capitalization to match other templates
body_template = E'Hi {{.UserName}}\n' ||
E'Your workspace **{{.Labels.name}}** has been updated automatically to the latest template version ({{.Labels.template_version_name}}).\n' ||
E'Reason for update: **{{.Labels.template_version_message}}**' -- include template version message
WHERE id = 'c34a0c09-0704-4cac-bd1c-0c0146811c2b';
2 changes: 1 addition & 1 deletion coderd/notifications/events.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ package notifications
import "github.com/google/uuid"

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

// Workspace-related events.
var (
Expand Down
97 changes: 87 additions & 10 deletions coderd/notifications/notifications_test.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,14 @@
package notifications_test

import (
"bytes"
"context"
_ "embed"
"encoding/json"
"fmt"
"go/ast"
"go/parser"
"go/token"
"net/http"
"net/http/httptest"
"net/url"
Expand Down Expand Up @@ -606,7 +611,42 @@ func TestNotifierPaused(t *testing.T) {
}, testutil.WaitShort, testutil.IntervalFast)
}

func TestNotificationTemplatesBody(t *testing.T) {
//go:embed events.go
var events []byte

// enumerateAllTemplates gets all the template names from the coderd/notifications/events.go file.
// TODO(dannyk): use code-generation to create a list of all templates: https://github.com/coder/team-coconut/issues/36
func enumerateAllTemplates(t *testing.T) ([]string, error) {
t.Helper()

fset := token.NewFileSet()

node, err := parser.ParseFile(fset, "", bytes.NewBuffer(events), parser.AllErrors)
if err != nil {
return nil, err
}

var out []string
// Traverse the AST and extract variable names.
ast.Inspect(node, func(n ast.Node) bool {
// Check if the node is a declaration statement.
if decl, ok := n.(*ast.GenDecl); ok && decl.Tok == token.VAR {
for _, spec := range decl.Specs {
// Type assert the spec to a ValueSpec.
if valueSpec, ok := spec.(*ast.ValueSpec); ok {
for _, name := range valueSpec.Names {
out = append(out, name.String())
}
}
}
}
return true
})

return out, nil
}

func TestNotificationTemplatesCanRender(t *testing.T) {
t.Parallel()

if !dbtestutil.WillUsePostgres() {
Expand Down Expand Up @@ -647,10 +687,11 @@ func TestNotificationTemplatesBody(t *testing.T) {
payload: types.MessagePayload{
UserName: "bobby",
Labels: map[string]string{
"name": "bobby-workspace",
"reason": "breached the template's threshold for inactivity",
"initiator": "autobuild",
"dormancyHours": "24",
"name": "bobby-workspace",
"reason": "breached the template's threshold for inactivity",
"initiator": "autobuild",
"dormancyHours": "24",
"timeTilDormant": "24h",
},
},
},
Expand All @@ -660,8 +701,9 @@ func TestNotificationTemplatesBody(t *testing.T) {
payload: types.MessagePayload{
UserName: "bobby",
Labels: map[string]string{
"name": "bobby-workspace",
"template_version_name": "1.0",
"name": "bobby-workspace",
"template_version_name": "1.0",
"template_version_message": "template now includes catnip",
},
},
},
Expand All @@ -671,12 +713,46 @@ func TestNotificationTemplatesBody(t *testing.T) {
payload: types.MessagePayload{
UserName: "bobby",
Labels: map[string]string{
"name": "bobby-workspace",
"reason": "template updated to new dormancy policy",
"dormancyHours": "24",
"name": "bobby-workspace",
"reason": "template updated to new dormancy policy",
"dormancyHours": "24",
"timeTilDormant": "24h",
},
},
},
{
name: "TemplateUserAccountCreated",
id: notifications.TemplateUserAccountCreated,
payload: types.MessagePayload{
UserName: "bobby",
Labels: map[string]string{
"created_account_name": "bobby",
},
},
},
{
name: "TemplateUserAccountDeleted",
id: notifications.TemplateUserAccountDeleted,
payload: types.MessagePayload{
UserName: "bobby",
Labels: map[string]string{
"deleted_account_name": "bobby",
},
},
},
}

allTemplates, err := enumerateAllTemplates(t)
require.NoError(t, err)
for _, name := range allTemplates {
var found bool
for _, tc := range tests {
if tc.name == name {
found = true
}
}

require.Truef(t, found, "could not find test case for %q", name)
}

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

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

Expand Down
11 changes: 10 additions & 1 deletion coderd/notifications/render/gotmpl.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,19 @@ import (
"github.com/coder/coder/v2/coderd/notifications/types"
)

// NoValue is used when a template variable is not found.
// This string is not exported as a const from the text/template.
const NoValue = "<no value>"

// GoTemplate attempts to substitute the given payload into the given template using Go's templating syntax.
// TODO: memoize templates for memory efficiency?
func GoTemplate(in string, payload types.MessagePayload, extraFuncs template.FuncMap) (string, error) {
tmpl, err := template.New("text").Funcs(extraFuncs).Parse(in)
tmpl, err := template.New("text").
Funcs(extraFuncs).
// text/template substitutes a missing label with "<no value>".
// NOTE: html/template does not, for obvious reasons.
Option("missingkey=invalid").
Parse(in)
if err != nil {
return "", xerrors.Errorf("template parse: %w", err)
}
Expand Down
Loading