Skip to content

Commit fac77f9

Browse files
SasSwartSasSwart
authored
fix(coderd/notifications): simplify TemplateWorkspaceManualBuildFailed (coder#15067)
This PR closes coder#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.
1 parent 5317c50 commit fac77f9

11 files changed

+73
-44
lines changed

‎coderd/database/migrations/000264_manual_build_failed_notification_template.down.sql

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
UPDATE notification_templates
2+
SET
3+
body_template = E'Hi {{.UserName}},\n\n' ||
4+
E'A manual build of the workspace **{{.Labels.name}}** using the template **{{.Labels.template_name}}** failed (version: **{{.Labels.template_version_name}}**).\n\n' ||
5+
-- Mention template display name:
6+
E'The template''s display name was **{{.Labels.template_display_name}}**. ' ||
7+
E'The workspace build was initiated by **{{.Labels.initiator}}**.'
8+
WHERE
9+
id = '2faeee0f-26cb-4e96-821c-85ccb9f71513';
10+
11+
UPDATE notification_templates
12+
SET
13+
body_template = E'Hi {{.UserName}},\n\n' || -- Add a comma
14+
E'The template **{{.Labels.name}}** was deleted by **{{ .Labels.initiator }}**.\n\n' ||
15+
-- Mention template display name:
16+
E'The template''s display name was **{{.Labels.display_name}}**.'
17+
WHERE
18+
id = '29a09665-2a4c-403f-9648-54301670e7be';

‎coderd/database/migrations/000264_manual_build_failed_notification_template.up.sql

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
UPDATE notification_templates
2+
SET
3+
body_template = E'Hi {{.UserName}},\n\n' ||
4+
-- Revert to a single label for the template name:
5+
E'A manual build of the workspace **{{.Labels.name}}** using the template **{{.Labels.template_name}}** failed (version: **{{.Labels.template_version_name}}**).\n\n' ||
6+
E'The workspace build was initiated by **{{.Labels.initiator}}**.'
7+
WHERE
8+
id = '2faeee0f-26cb-4e96-821c-85ccb9f71513';
9+
10+
UPDATE notification_templates
11+
SET
12+
body_template = E'Hi {{.UserName}},\n\n' ||
13+
-- Revert to a single label for the template name:
14+
E'The template **{{.Labels.name}}** was deleted by **{{ .Labels.initiator }}**.\n\n'
15+
WHERE
16+
id = '29a09665-2a4c-403f-9648-54301670e7be';

‎coderd/notifications/notifications_test.go

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -870,9 +870,8 @@ func TestNotificationTemplates_Golden(t *testing.T) {
870870
UserEmail: "bobby@coder.com",
871871
UserUsername: "bobby",
872872
Labels: map[string]string{
873-
"name": "bobby-template",
874-
"display_name": "Bobby's Template",
875-
"initiator": "rob",
873+
"name": "Bobby's Template",
874+
"initiator": "rob",
876875
},
877876
},
878877
},
@@ -886,7 +885,6 @@ func TestNotificationTemplates_Golden(t *testing.T) {
886885
Labels: map[string]string{
887886
"name": "bobby-workspace",
888887
"template_name": "bobby-template",
889-
"template_display_name": "William's Template",
890888
"template_version_name": "bobby-template-version",
891889
"initiator": "joe",
892890
"workspace_owner_username": "mrbobby",

‎coderd/notifications/testdata/rendered-templates/smtp/TemplateTemplateDeleted.html.golden

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
From: system@coder.com
22
To: bobby@coder.com
3-
Subject: Template "bobby-template" deleted
3+
Subject: Template "Bobby's Template" deleted
44
Message-Id: 02ee4935-73be-4fa1-a290-ff9999026b13@blush-whale-48
55
Date: Fri, 11 Oct 2024 09:03:06 +0000
66
Content-Type: multipart/alternative; boundary=bbe61b741255b6098bb6b3c1f41b885773df633cb18d2a3002b68e4bc9c4
@@ -12,9 +12,7 @@ Content-Type: text/plain; charset=UTF-8
1212

1313
Hi Bobby,
1414

15-
The template bobby-template was deleted by rob.
16-
17-
The template's display name was Bobby's Template.
15+
The template Bobby's Template was deleted by rob.
1816

1917

2018
View templates: http://test.com/templates
@@ -29,7 +27,7 @@ Content-Type: text/html; charset=UTF-8
2927
<meta charset=3D"UTF-8" />
3028
<meta name=3D"viewport" content=3D"width=3Ddevice-width, initial-scale=
3129
=3D1.0" />
32-
<title>Template "bobby-template" deleted</title>
30+
<title>Template "Bobby's Template" deleted</title>
3331
</head>
3432
<body style=3D"margin: 0; padding: 0; font-family: -apple-system, system-=
3533
ui, BlinkMacSystemFont, 'Segoe UI', 'Roboto', 'Oxygen', 'Ubuntu', 'Cantarel=
@@ -44,16 +42,13 @@ er Logo" style=3D"height: 40px;" />
4442
</div>
4543
<h1 style=3D"text-align: center; font-size: 24px; font-weight: 400; m=
4644
argin: 8px 0 32px; line-height: 1.5;">
47-
Template "bobby-template" deleted
45+
Template "Bobby's Template" deleted
4846
</h1>
4947
<div style=3D"line-height: 1.5;">
5048
<p>Hi Bobby,</p>
5149

52-
<p>The template <strong>bobby-template</strong> was deleted by <strong>rob<=
53-
/strong>.</p>
54-
55-
<p>The template&rsquo;s display name was <strong>Bobby&rsquo;s Template</st=
56-
rong>.</p>
50+
<p>The template <strong>Bobby&rsquo;s Template</strong> was deleted by <str=
51+
ong>rob</strong>.</p>
5752
</div>
5853
<div style=3D"text-align: center; margin-top: 32px;">
5954
=20

‎coderd/notifications/testdata/rendered-templates/smtp/TemplateWorkspaceManualBuildFailed.html.golden

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,7 @@ Hi Bobby,
1515
A manual build of the workspace bobby-workspace using the template bobby-te=
1616
mplate failed (version: bobby-template-version).
1717

18-
The template's display name was William's Template. The workspace build was=
19-
initiated by joe.
18+
The workspace build was initiated by joe.
2019

2120

2221
View build: http://test.com/@mrbobby/bobby-workspace/builds/3
@@ -55,8 +54,7 @@ argin: 8px 0 32px; line-height: 1.5;">
5554
he template <strong>bobby-template</strong> failed (version: <strong>bobby-=
5655
template-version</strong>).</p>
5756

58-
<p>The template&rsquo;s display name was <strong>William&rsquo;s Template</=
59-
strong>. The workspace build was initiated by <strong>joe</strong>.</p>
57+
<p>The workspace build was initiated by <strong>joe</strong>.</p>
6058
</div>
6159
<div style=3D"text-align: center; margin-top: 32px;">
6260
=20

‎coderd/notifications/testdata/rendered-templates/webhook/TemplateTemplateDeleted.json.golden

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,14 +16,13 @@
1616
}
1717
],
1818
"labels": {
19-
"display_name": "Bobby's Template",
2019
"initiator": "rob",
21-
"name": "bobby-template"
20+
"name": "Bobby's Template"
2221
},
2322
"data": null
2423
},
25-
"title": "Template \"bobby-template\" deleted",
26-
"title_markdown": "Template \"bobby-template\" deleted",
27-
"body": "Hi Bobby,\n\nThe template bobby-template was deleted by rob.\n\nThe template's display name was Bobby's Template.",
28-
"body_markdown": "Hi Bobby,\n\nThe template **bobby-template** was deleted by **rob**.\n\nThe template's display name was **Bobby's Template**."
24+
"title": "Template \"Bobby's Template\" deleted",
25+
"title_markdown": "Template \"Bobby's Template\" deleted",
26+
"body": "Hi Bobby,\n\nThe template Bobby's Template was deleted by rob.",
27+
"body_markdown": "Hi Bobby,\n\nThe template **Bobby's Template** was deleted by **rob**.\n\n"
2928
}

‎coderd/notifications/testdata/rendered-templates/webhook/TemplateWorkspaceManualBuildFailed.json.golden

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818
"labels": {
1919
"initiator": "joe",
2020
"name": "bobby-workspace",
21-
"template_display_name": "William's Template",
2221
"template_name": "bobby-template",
2322
"template_version_name": "bobby-template-version",
2423
"workspace_build_number": "3",
@@ -28,6 +27,6 @@
2827
},
2928
"title": "Workspace \"bobby-workspace\" manual build failed",
3029
"title_markdown": "Workspace \"bobby-workspace\" manual build failed",
31-
"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.",
32-
"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**."
30+
"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.",
31+
"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**."
3332
}

‎coderd/provisionerdserver/provisionerdserver.go

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1124,16 +1124,20 @@ func (s *server) notifyWorkspaceManualBuildFailed(ctx context.Context, workspace
11241124
}
11251125

11261126
for _, templateAdmin := range templateAdmins {
1127+
templateNameLabel := template.DisplayName
1128+
if templateNameLabel == "" {
1129+
templateNameLabel = template.Name
1130+
}
1131+
labels := map[string]string{
1132+
"name": workspace.Name,
1133+
"template_name": templateNameLabel,
1134+
"template_version_name": templateVersion.Name,
1135+
"initiator": build.InitiatorByUsername,
1136+
"workspace_owner_username": workspaceOwner.Username,
1137+
"workspace_build_number": strconv.Itoa(int(build.BuildNumber)),
1138+
}
11271139
if _, err := s.NotificationsEnqueuer.Enqueue(ctx, templateAdmin.ID, notifications.TemplateWorkspaceManualBuildFailed,
1128-
map[string]string{
1129-
"name": workspace.Name,
1130-
"template_name": template.Name,
1131-
"template_display_name": template.DisplayName,
1132-
"template_version_name": templateVersion.Name,
1133-
"initiator": build.InitiatorByUsername,
1134-
"workspace_owner_username": workspaceOwner.Username,
1135-
"workspace_build_number": strconv.Itoa(int(build.BuildNumber)),
1136-
}, "provisionerdserver",
1140+
labels, "provisionerdserver",
11371141
// Associate this notification with all the related entities.
11381142
workspace.ID, workspace.OwnerID, workspace.TemplateID, workspace.OrganizationID,
11391143
); err != nil {

‎coderd/provisionerdserver/provisionerdserver_test.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1859,8 +1859,7 @@ func TestNotifications(t *testing.T) {
18591859
assert.Contains(t, notifEnq.Sent[0].Targets, workspace.OrganizationID)
18601860
assert.Contains(t, notifEnq.Sent[0].Targets, user.ID)
18611861
assert.Equal(t, workspace.Name, notifEnq.Sent[0].Labels["name"])
1862-
assert.Equal(t, template.Name, notifEnq.Sent[0].Labels["template_name"])
1863-
assert.Equal(t, template.DisplayName, notifEnq.Sent[0].Labels["template_display_name"])
1862+
assert.Equal(t, template.DisplayName, notifEnq.Sent[0].Labels["template_name"])
18641863
assert.Equal(t, version.Name, notifEnq.Sent[0].Labels["template_version_name"])
18651864
assert.Equal(t, user.Username, notifEnq.Sent[0].Labels["initiator"])
18661865
assert.Equal(t, user.Username, notifEnq.Sent[0].Labels["workspace_owner_username"])

‎coderd/templates.go

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -135,11 +135,15 @@ func (api *API) notifyTemplateDeleted(ctx context.Context, template database.Tem
135135
return
136136
}
137137

138+
templateNameLabel := template.DisplayName
139+
if templateNameLabel == "" {
140+
templateNameLabel = template.Name
141+
}
142+
138143
if _, err := api.NotificationsEnqueuer.Enqueue(ctx, receiverID, notifications.TemplateTemplateDeleted,
139144
map[string]string{
140-
"name": template.Name,
141-
"display_name": template.DisplayName,
142-
"initiator": initiator.Username,
145+
"name": templateNameLabel,
146+
"initiator": initiator.Username,
143147
}, "api-templates-delete",
144148
// Associate this notification with all the related entities.
145149
template.ID, template.OrganizationID,

‎coderd/templates_test.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1456,8 +1456,7 @@ func TestTemplateNotifications(t *testing.T) {
14561456
require.Contains(t, notifiedUsers, n.UserID)
14571457
require.Contains(t, n.Targets, template.ID)
14581458
require.Contains(t, n.Targets, template.OrganizationID)
1459-
require.Equal(t, n.Labels["name"], template.Name)
1460-
require.Equal(t, n.Labels["display_name"], template.DisplayName)
1459+
require.Equal(t, n.Labels["name"], template.DisplayName)
14611460
require.Equal(t, n.Labels["initiator"], coderdtest.FirstUserParams.Username)
14621461
}
14631462
})

0 commit comments

Comments
 (0)