fix(coderd/notifications): simplify TemplateWorkspaceManualBuildFailed (#15067)

This PR closes #15065.

As advised by @mtojek, a template's display name may be set to "", which
is not useful in an email notification. We'd like to provide a friendly
name for the template, but it also needs to be identifiable.

As such, we fall back to template.Name in the case that the template's
display name is empty.
This commit is contained in:
Sas Swart
2024-10-15 21:02:02 +02:00
committed by GitHub
parent 5317c500c8
commit fac77f956e
11 changed files with 73 additions and 44 deletions
@@ -0,0 +1,18 @@
UPDATE notification_templates
SET
body_template = E'Hi {{.UserName}},\n\n' ||
E'A manual build of the workspace **{{.Labels.name}}** using the template **{{.Labels.template_name}}** failed (version: **{{.Labels.template_version_name}}**).\n\n' ||
-- Mention template display name:
E'The template''s display name was **{{.Labels.template_display_name}}**. ' ||
E'The workspace build was initiated by **{{.Labels.initiator}}**.'
WHERE
id = '2faeee0f-26cb-4e96-821c-85ccb9f71513';
UPDATE notification_templates
SET
body_template = E'Hi {{.UserName}},\n\n' || -- Add a comma
E'The template **{{.Labels.name}}** was deleted by **{{ .Labels.initiator }}**.\n\n' ||
-- Mention template display name:
E'The template''s display name was **{{.Labels.display_name}}**.'
WHERE
id = '29a09665-2a4c-403f-9648-54301670e7be';
@@ -0,0 +1,16 @@
UPDATE notification_templates
SET
body_template = E'Hi {{.UserName}},\n\n' ||
-- Revert to a single label for the template name:
E'A manual build of the workspace **{{.Labels.name}}** using the template **{{.Labels.template_name}}** failed (version: **{{.Labels.template_version_name}}**).\n\n' ||
E'The workspace build was initiated by **{{.Labels.initiator}}**.'
WHERE
id = '2faeee0f-26cb-4e96-821c-85ccb9f71513';
UPDATE notification_templates
SET
body_template = E'Hi {{.UserName}},\n\n' ||
-- Revert to a single label for the template name:
E'The template **{{.Labels.name}}** was deleted by **{{ .Labels.initiator }}**.\n\n'
WHERE
id = '29a09665-2a4c-403f-9648-54301670e7be';
+2 -4
View File
@@ -870,9 +870,8 @@ func TestNotificationTemplates_Golden(t *testing.T) {
UserEmail: "bobby@coder.com",
UserUsername: "bobby",
Labels: map[string]string{
"name": "bobby-template",
"display_name": "Bobby's Template",
"initiator": "rob",
"name": "Bobby's Template",
"initiator": "rob",
},
},
},
@@ -886,7 +885,6 @@ func TestNotificationTemplates_Golden(t *testing.T) {
Labels: map[string]string{
"name": "bobby-workspace",
"template_name": "bobby-template",
"template_display_name": "William's Template",
"template_version_name": "bobby-template-version",
"initiator": "joe",
"workspace_owner_username": "mrbobby",
@@ -1,6 +1,6 @@
From: system@coder.com
To: bobby@coder.com
Subject: Template "bobby-template" deleted
Subject: Template "Bobby's Template" deleted
Message-Id: 02ee4935-73be-4fa1-a290-ff9999026b13@blush-whale-48
Date: Fri, 11 Oct 2024 09:03:06 +0000
Content-Type: multipart/alternative; boundary=bbe61b741255b6098bb6b3c1f41b885773df633cb18d2a3002b68e4bc9c4
@@ -12,9 +12,7 @@ Content-Type: text/plain; charset=UTF-8
Hi Bobby,
The template bobby-template was deleted by rob.
The template's display name was Bobby's Template.
The template Bobby's Template was deleted by rob.
View templates: http://test.com/templates
@@ -29,7 +27,7 @@ Content-Type: text/html; charset=UTF-8
<meta charset=3D"UTF-8" />
<meta name=3D"viewport" content=3D"width=3Ddevice-width, initial-scale=
=3D1.0" />
<title>Template "bobby-template" deleted</title>
<title>Template "Bobby's Template" deleted</title>
</head>
<body style=3D"margin: 0; padding: 0; font-family: -apple-system, system-=
ui, BlinkMacSystemFont, 'Segoe UI', 'Roboto', 'Oxygen', 'Ubuntu', 'Cantarel=
@@ -44,16 +42,13 @@ er Logo" style=3D"height: 40px;" />
</div>
<h1 style=3D"text-align: center; font-size: 24px; font-weight: 400; m=
argin: 8px 0 32px; line-height: 1.5;">
Template "bobby-template" deleted
Template "Bobby's Template" deleted
</h1>
<div style=3D"line-height: 1.5;">
<p>Hi Bobby,</p>
<p>The template <strong>bobby-template</strong> was deleted by <strong>rob<=
/strong>.</p>
<p>The template&rsquo;s display name was <strong>Bobby&rsquo;s Template</st=
rong>.</p>
<p>The template <strong>Bobby&rsquo;s Template</strong> was deleted by <str=
ong>rob</strong>.</p>
</div>
<div style=3D"text-align: center; margin-top: 32px;">
=20
@@ -15,8 +15,7 @@ Hi Bobby,
A manual build of the workspace bobby-workspace using the template bobby-te=
mplate failed (version: bobby-template-version).
The template's display name was William's Template. The workspace build was=
initiated by joe.
The workspace build was initiated by joe.
View build: http://test.com/@mrbobby/bobby-workspace/builds/3
@@ -55,8 +54,7 @@ argin: 8px 0 32px; line-height: 1.5;">
he template <strong>bobby-template</strong> failed (version: <strong>bobby-=
template-version</strong>).</p>
<p>The template&rsquo;s display name was <strong>William&rsquo;s Template</=
strong>. The workspace build was initiated by <strong>joe</strong>.</p>
<p>The workspace build was initiated by <strong>joe</strong>.</p>
</div>
<div style=3D"text-align: center; margin-top: 32px;">
=20
@@ -16,14 +16,13 @@
}
],
"labels": {
"display_name": "Bobby's Template",
"initiator": "rob",
"name": "bobby-template"
"name": "Bobby's Template"
},
"data": null
},
"title": "Template \"bobby-template\" deleted",
"title_markdown": "Template \"bobby-template\" deleted",
"body": "Hi Bobby,\n\nThe template bobby-template was deleted by rob.\n\nThe template's display name was Bobby's Template.",
"body_markdown": "Hi Bobby,\n\nThe template **bobby-template** was deleted by **rob**.\n\nThe template's display name was **Bobby's Template**."
"title": "Template \"Bobby's Template\" deleted",
"title_markdown": "Template \"Bobby's Template\" deleted",
"body": "Hi Bobby,\n\nThe template Bobby's Template was deleted by rob.",
"body_markdown": "Hi Bobby,\n\nThe template **Bobby's Template** was deleted by **rob**.\n\n"
}
@@ -18,7 +18,6 @@
"labels": {
"initiator": "joe",
"name": "bobby-workspace",
"template_display_name": "William's Template",
"template_name": "bobby-template",
"template_version_name": "bobby-template-version",
"workspace_build_number": "3",
@@ -28,6 +27,6 @@
},
"title": "Workspace \"bobby-workspace\" manual build failed",
"title_markdown": "Workspace \"bobby-workspace\" manual build failed",
"body": "Hi Bobby,\n\nA manual build of the workspace bobby-workspace using the template bobby-template failed (version: bobby-template-version).\n\nThe template's display name was William's Template. The workspace build was initiated by joe.",
"body_markdown": "Hi Bobby,\n\nA manual build of the workspace **bobby-workspace** using the template **bobby-template** failed (version: **bobby-template-version**).\n\nThe template's display name was **William's Template**. The workspace build was initiated by **joe**."
"body": "Hi Bobby,\n\nA manual build of the workspace bobby-workspace using the template bobby-template failed (version: bobby-template-version).\n\nThe workspace build was initiated by joe.",
"body_markdown": "Hi Bobby,\n\nA manual build of the workspace **bobby-workspace** using the template **bobby-template** failed (version: **bobby-template-version**).\n\nThe workspace build was initiated by **joe**."
}
@@ -1124,16 +1124,20 @@ func (s *server) notifyWorkspaceManualBuildFailed(ctx context.Context, workspace
}
for _, templateAdmin := range templateAdmins {
templateNameLabel := template.DisplayName
if templateNameLabel == "" {
templateNameLabel = template.Name
}
labels := map[string]string{
"name": workspace.Name,
"template_name": templateNameLabel,
"template_version_name": templateVersion.Name,
"initiator": build.InitiatorByUsername,
"workspace_owner_username": workspaceOwner.Username,
"workspace_build_number": strconv.Itoa(int(build.BuildNumber)),
}
if _, err := s.NotificationsEnqueuer.Enqueue(ctx, templateAdmin.ID, notifications.TemplateWorkspaceManualBuildFailed,
map[string]string{
"name": workspace.Name,
"template_name": template.Name,
"template_display_name": template.DisplayName,
"template_version_name": templateVersion.Name,
"initiator": build.InitiatorByUsername,
"workspace_owner_username": workspaceOwner.Username,
"workspace_build_number": strconv.Itoa(int(build.BuildNumber)),
}, "provisionerdserver",
labels, "provisionerdserver",
// Associate this notification with all the related entities.
workspace.ID, workspace.OwnerID, workspace.TemplateID, workspace.OrganizationID,
); err != nil {
@@ -1859,8 +1859,7 @@ func TestNotifications(t *testing.T) {
assert.Contains(t, notifEnq.Sent[0].Targets, workspace.OrganizationID)
assert.Contains(t, notifEnq.Sent[0].Targets, user.ID)
assert.Equal(t, workspace.Name, notifEnq.Sent[0].Labels["name"])
assert.Equal(t, template.Name, notifEnq.Sent[0].Labels["template_name"])
assert.Equal(t, template.DisplayName, notifEnq.Sent[0].Labels["template_display_name"])
assert.Equal(t, template.DisplayName, notifEnq.Sent[0].Labels["template_name"])
assert.Equal(t, version.Name, notifEnq.Sent[0].Labels["template_version_name"])
assert.Equal(t, user.Username, notifEnq.Sent[0].Labels["initiator"])
assert.Equal(t, user.Username, notifEnq.Sent[0].Labels["workspace_owner_username"])
+7 -3
View File
@@ -135,11 +135,15 @@ func (api *API) notifyTemplateDeleted(ctx context.Context, template database.Tem
return
}
templateNameLabel := template.DisplayName
if templateNameLabel == "" {
templateNameLabel = template.Name
}
if _, err := api.NotificationsEnqueuer.Enqueue(ctx, receiverID, notifications.TemplateTemplateDeleted,
map[string]string{
"name": template.Name,
"display_name": template.DisplayName,
"initiator": initiator.Username,
"name": templateNameLabel,
"initiator": initiator.Username,
}, "api-templates-delete",
// Associate this notification with all the related entities.
template.ID, template.OrganizationID,
+1 -2
View File
@@ -1456,8 +1456,7 @@ func TestTemplateNotifications(t *testing.T) {
require.Contains(t, notifiedUsers, n.UserID)
require.Contains(t, n.Targets, template.ID)
require.Contains(t, n.Targets, template.OrganizationID)
require.Equal(t, n.Labels["name"], template.Name)
require.Equal(t, n.Labels["display_name"], template.DisplayName)
require.Equal(t, n.Labels["name"], template.DisplayName)
require.Equal(t, n.Labels["initiator"], coderdtest.FirstUserParams.Username)
}
})