fix: notifications: use username in workspace URLs (coder#14011)

mtojek · web-flow · commit d48885339375 · 2024-07-25T12:02:24.000+02:00
diff --git a/coderd/database/dbmem/dbmem.go b/coderd/database/dbmem/dbmem.go
@@ -1929,6 +1929,7 @@ func (q *FakeQuerier) FetchNewMessageMetadata(_ context.Context, arg database.Fe
 	return database.FetchNewMessageMetadataRow{
 		UserEmail:        user.Email,
 		UserName:         userName,
+		UserUsername:     user.Username,
 		NotificationName: "Some notification",
 		Actions:          actions,
 		UserID:           arg.UserID,
diff --git a/coderd/database/migrations/000230_notifications_fix_username.down.sql b/coderd/database/migrations/000230_notifications_fix_username.down.sql
@@ -0,0 +1,3 @@
+UPDATE notification_templates
+SET
+    actions = REPLACE(actions::text, '@{{.UserUsername}}', '@{{.UserName}}')::jsonb;
diff --git a/coderd/database/migrations/000230_notifications_fix_username.up.sql b/coderd/database/migrations/000230_notifications_fix_username.up.sql
@@ -0,0 +1,3 @@
+UPDATE notification_templates
+SET
+    actions = REPLACE(actions::text, '@{{.UserName}}', '@{{.UserUsername}}')::jsonb;
diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go
diff --git a/coderd/database/queries/notifications.sql b/coderd/database/queries/notifications.sql
@@ -4,7 +4,8 @@ SELECT nt.name                                                    AS notificatio
        nt.actions                                                 AS actions,
        u.id                                                       AS user_id,
        u.email                                                    AS user_email,
-       COALESCE(NULLIF(u.name, ''), NULLIF(u.username, ''))::text AS user_name
+       COALESCE(NULLIF(u.name, ''), NULLIF(u.username, ''))::text AS user_name,
+       COALESCE(u.username, '')                                   AS user_username
 FROM notification_templates nt,
      users u
 WHERE nt.id = @notification_template_id
diff --git a/coderd/notifications/enqueuer.go b/coderd/notifications/enqueuer.go
@@ -94,9 +94,10 @@ func (s *StoreEnqueuer) buildPayload(ctx context.Context, userID, templateID uui
 
 		NotificationName: metadata.NotificationName,
 
-		UserID:    metadata.UserID.String(),
-		UserEmail: metadata.UserEmail,
-		UserName:  metadata.UserName,
+		UserID:       metadata.UserID.String(),
+		UserEmail:    metadata.UserEmail,
+		UserName:     metadata.UserName,
+		UserUsername: metadata.UserUsername,
 
 		Labels: labels,
 		// No actions yet
diff --git a/coderd/notifications/notifications_test.go b/coderd/notifications/notifications_test.go
@@ -201,12 +201,13 @@ func TestWebhookDispatch(t *testing.T) {
 	require.NoError(t, err)
 
 	const (
-		email = "bob@coder.com"
-		name  = "Robert McBobbington"
+		email    = "bob@coder.com"
+		name     = "Robert McBobbington"
+		username = "bob"
 	)
 	user := dbgen.User(t, db, database.User{
 		Email:    email,
-		Username: "bob",
+		Username: username,
 		Name:     name,
 	})
 
@@ -229,6 +230,7 @@ func TestWebhookDispatch(t *testing.T) {
 	// UserName is coalesced from `name` and `username`; in this case `name` wins.
 	// This is not strictly necessary for this test, but it's testing some side logic which is too small for its own test.
 	require.Equal(t, payload.Payload.UserName, name)
+	require.Equal(t, payload.Payload.UserUsername, username)
 	// Right now we don't have a way to query notification templates by ID in dbmem, and it's not necessary to add this
 	// just to satisfy this test. We can safely assume that as long as this value is not empty that the given value was delivered.
 	require.NotEmpty(t, payload.Payload.NotificationName)
diff --git a/coderd/notifications/render/gotmpl_test.go b/coderd/notifications/render/gotmpl_test.go
@@ -42,10 +42,11 @@ func TestGoTemplate(t *testing.T) {
 			name: "render workspace URL",
 			in: `[{
 				"label": "View workspace",
-				"url": "{{ base_url }}/@{{.UserName}}/{{.Labels.name}}"
+				"url": "{{ base_url }}/@{{.UserUsername}}/{{.Labels.name}}"
 			}]`,
 			payload: types.MessagePayload{
-				UserName: "johndoe",
+				UserName:     "John Doe",
+				UserUsername: "johndoe",
 				Labels: map[string]string{
 					"name": "my-workspace",
 				},
diff --git a/coderd/notifications/types/payload.go b/coderd/notifications/types/payload.go
@@ -9,9 +9,10 @@ type MessagePayload struct {
 
 	NotificationName string `json:"notification_name"`
 
-	UserID    string `json:"user_id"`
-	UserEmail string `json:"user_email"`
-	UserName  string `json:"user_name"`
+	UserID       string `json:"user_id"`
+	UserEmail    string `json:"user_email"`
+	UserName     string `json:"user_name"`
+	UserUsername string `json:"user_username"`
 
 	Actions []TemplateAction  `json:"actions"`
 	Labels  map[string]string `json:"labels"`

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+UPDATE notification_templates`
	`2`	`+SET`
	`3`	`+ actions = REPLACE(actions::text, '@{{.UserUsername}}', '@{{.UserName}}')::jsonb;`