From f651ab937b25eab04833e955f32412b1f83893e7 Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Thu, 30 Jan 2025 17:44:04 +0000 Subject: [PATCH] chore: add 'email' field to notifications (#16336) Closes https://github.com/coder/internal/issues/323 This PR adds an `email` field to the `data.owner` payload for workspace created and workspace manually updated notifications, as well as user account created/activated/suspended. --- coderd/users.go | 28 +++++++++++++++++++++------- coderd/users_test.go | 26 ++++++++++++++++++++------ coderd/workspacebuilds.go | 2 +- coderd/workspacebuilds_test.go | 8 +++++++- coderd/workspaces.go | 2 +- coderd/workspaces_test.go | 6 ++++++ 6 files changed, 56 insertions(+), 16 deletions(-) diff --git a/coderd/users.go b/coderd/users.go index 56f2959868..964f187244 100644 --- a/coderd/users.go +++ b/coderd/users.go @@ -918,6 +918,7 @@ func (api *API) putUserStatus(status database.UserStatus) func(rw http.ResponseW func (api *API) notifyUserStatusChanged(ctx context.Context, actingUserName string, targetUser database.User, status database.UserStatus) error { var labels map[string]string + var data map[string]any var adminTemplateID, personalTemplateID uuid.UUID switch status { case database.UserStatusSuspended: @@ -926,6 +927,9 @@ func (api *API) notifyUserStatusChanged(ctx context.Context, actingUserName stri "suspended_account_user_name": targetUser.Name, "initiator": actingUserName, } + data = map[string]any{ + "user": map[string]any{"id": targetUser.ID, "name": targetUser.Name, "email": targetUser.Email}, + } adminTemplateID = notifications.TemplateUserAccountSuspended personalTemplateID = notifications.TemplateYourAccountSuspended case database.UserStatusActive: @@ -934,6 +938,9 @@ func (api *API) notifyUserStatusChanged(ctx context.Context, actingUserName stri "activated_account_user_name": targetUser.Name, "initiator": actingUserName, } + data = map[string]any{ + "user": map[string]any{"id": targetUser.ID, "name": targetUser.Name, "email": targetUser.Email}, + } adminTemplateID = notifications.TemplateUserAccountActivated personalTemplateID = notifications.TemplateYourAccountActivated default: @@ -949,16 +956,16 @@ func (api *API) notifyUserStatusChanged(ctx context.Context, actingUserName stri // Send notifications to user admins and affected user for _, u := range userAdmins { // nolint:gocritic // Need notifier actor to enqueue notifications - if _, err := api.NotificationsEnqueuer.Enqueue(dbauthz.AsNotifier(ctx), u.ID, adminTemplateID, - labels, "api-put-user-status", + if _, err := api.NotificationsEnqueuer.EnqueueWithData(dbauthz.AsNotifier(ctx), u.ID, adminTemplateID, + labels, data, "api-put-user-status", targetUser.ID, ); err != nil { api.Logger.Warn(ctx, "unable to notify about changed user's status", slog.F("affected_user", targetUser.Username), slog.Error(err)) } } // nolint:gocritic // Need notifier actor to enqueue notifications - if _, err := api.NotificationsEnqueuer.Enqueue(dbauthz.AsNotifier(ctx), targetUser.ID, personalTemplateID, - labels, "api-put-user-status", + if _, err := api.NotificationsEnqueuer.EnqueueWithData(dbauthz.AsNotifier(ctx), targetUser.ID, personalTemplateID, + labels, data, "api-put-user-status", targetUser.ID, ); err != nil { api.Logger.Warn(ctx, "unable to notify user about status change of their account", slog.F("affected_user", targetUser.Username), slog.Error(err)) @@ -1424,13 +1431,20 @@ func (api *API) CreateUser(ctx context.Context, store database.Store, req Create } for _, u := range userAdmins { - // nolint:gocritic // Need notifier actor to enqueue notifications - if _, err := api.NotificationsEnqueuer.Enqueue(dbauthz.AsNotifier(ctx), u.ID, notifications.TemplateUserAccountCreated, + if _, err := api.NotificationsEnqueuer.EnqueueWithData( + // nolint:gocritic // Need notifier actor to enqueue notifications + dbauthz.AsNotifier(ctx), + u.ID, + notifications.TemplateUserAccountCreated, map[string]string{ "created_account_name": user.Username, "created_account_user_name": user.Name, "initiator": req.accountCreatorName, - }, "api-users-create", + }, + map[string]any{ + "user": map[string]any{"id": user.ID, "name": user.Name, "email": user.Email}, + }, + "api-users-create", user.ID, ); err != nil { api.Logger.Warn(ctx, "unable to notify about created user", slog.F("created_user", user.Username), slog.Error(err)) diff --git a/coderd/users_test.go b/coderd/users_test.go index 1386d76f3e..53ec98b30d 100644 --- a/coderd/users_test.go +++ b/coderd/users_test.go @@ -392,12 +392,19 @@ func TestNotifyUserStatusChanged(t *testing.T) { // Validate that each expected notification is present in notifyEnq.Sent() for _, expected := range expectedNotifications { found := false - for _, sent := range notifyEnq.Sent() { + for _, sent := range notifyEnq.Sent(notificationstest.WithTemplateID(expected.TemplateID)) { if sent.TemplateID == expected.TemplateID && sent.UserID == expected.UserID && slices.Contains(sent.Targets, member.ID) && sent.Labels[label] == member.Username { found = true + + require.IsType(t, map[string]any{}, sent.Data["user"]) + userData := sent.Data["user"].(map[string]any) + require.Equal(t, member.ID, userData["id"]) + require.Equal(t, member.Name, userData["name"]) + require.Equal(t, member.Email, userData["email"]) + break } } @@ -858,11 +865,18 @@ func TestNotifyCreatedUser(t *testing.T) { require.NoError(t, err) // then - require.Len(t, notifyEnq.Sent(), 1) - require.Equal(t, notifications.TemplateUserAccountCreated, notifyEnq.Sent()[0].TemplateID) - require.Equal(t, firstUser.UserID, notifyEnq.Sent()[0].UserID) - require.Contains(t, notifyEnq.Sent()[0].Targets, user.ID) - require.Equal(t, user.Username, notifyEnq.Sent()[0].Labels["created_account_name"]) + sent := notifyEnq.Sent(notificationstest.WithTemplateID(notifications.TemplateUserAccountCreated)) + require.Len(t, sent, 1) + require.Equal(t, notifications.TemplateUserAccountCreated, sent[0].TemplateID) + require.Equal(t, firstUser.UserID, sent[0].UserID) + require.Contains(t, sent[0].Targets, user.ID) + require.Equal(t, user.Username, sent[0].Labels["created_account_name"]) + + require.IsType(t, map[string]any{}, sent[0].Data["user"]) + userData := sent[0].Data["user"].(map[string]any) + require.Equal(t, user.ID, userData["id"]) + require.Equal(t, user.Name, userData["name"]) + require.Equal(t, user.Email, userData["email"]) }) t.Run("UserAdminNotified", func(t *testing.T) { diff --git a/coderd/workspacebuilds.go b/coderd/workspacebuilds.go index 375eaab5cd..76166bfcb6 100644 --- a/coderd/workspacebuilds.go +++ b/coderd/workspacebuilds.go @@ -527,7 +527,7 @@ func (api *API) notifyWorkspaceUpdated( "workspace": map[string]any{"id": workspace.ID, "name": workspace.Name}, "template": map[string]any{"id": template.ID, "name": template.Name}, "template_version": map[string]any{"id": version.ID, "name": version.Name}, - "owner": map[string]any{"id": owner.ID, "name": owner.Name}, + "owner": map[string]any{"id": owner.ID, "name": owner.Name, "email": owner.Email}, "parameters": buildParameters, }, "api-workspaces-updated", diff --git a/coderd/workspacebuilds_test.go b/coderd/workspacebuilds_test.go index da4c09329c..fc8961a8c7 100644 --- a/coderd/workspacebuilds_test.go +++ b/coderd/workspacebuilds_test.go @@ -648,7 +648,7 @@ func TestWorkspaceBuildWithUpdatedTemplateVersionSendsNotification(t *testing.T) client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true, NotificationsEnqueuer: notify}) first := coderdtest.CreateFirstUser(t, client) templateAdminClient, templateAdmin := coderdtest.CreateAnotherUser(t, client, first.OrganizationID, rbac.RoleTemplateAdmin()) - userClient, _ := coderdtest.CreateAnotherUser(t, client, first.OrganizationID) + userClient, user := coderdtest.CreateAnotherUser(t, client, first.OrganizationID) // Create a template with an initial version version := coderdtest.CreateTemplateVersion(t, templateAdminClient, first.OrganizationID, nil) @@ -684,6 +684,12 @@ func TestWorkspaceBuildWithUpdatedTemplateVersionSendsNotification(t *testing.T) require.Contains(t, sent[0].Targets, workspace.ID) require.Contains(t, sent[0].Targets, workspace.OrganizationID) require.Contains(t, sent[0].Targets, workspace.OwnerID) + + owner, ok := sent[0].Data["owner"].(map[string]any) + require.True(t, ok, "notification data should have owner") + require.Equal(t, user.ID, owner["id"]) + require.Equal(t, user.Name, owner["name"]) + require.Equal(t, user.Email, owner["email"]) }) } diff --git a/coderd/workspaces.go b/coderd/workspaces.go index 158f27132b..7a64648033 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -809,7 +809,7 @@ func (api *API) notifyWorkspaceCreated( "workspace": map[string]any{"id": workspace.ID, "name": workspace.Name}, "template": map[string]any{"id": template.ID, "name": template.Name}, "template_version": map[string]any{"id": version.ID, "name": version.Name}, - "owner": map[string]any{"id": owner.ID, "name": owner.Name}, + "owner": map[string]any{"id": owner.ID, "name": owner.Name, "email": owner.Email}, "parameters": buildParameters, }, "api-workspaces-create", diff --git a/coderd/workspaces_test.go b/coderd/workspaces_test.go index e74d517412..b8bf71c3eb 100644 --- a/coderd/workspaces_test.go +++ b/coderd/workspaces_test.go @@ -639,6 +639,12 @@ func TestPostWorkspacesByOrganization(t *testing.T) { require.Contains(t, sent[0].Targets, workspace.ID) require.Contains(t, sent[0].Targets, workspace.OrganizationID) require.Contains(t, sent[0].Targets, workspace.OwnerID) + + owner, ok := sent[0].Data["owner"].(map[string]any) + require.True(t, ok, "notification data should have owner") + require.Equal(t, memberUser.ID, owner["id"]) + require.Equal(t, memberUser.Name, owner["name"]) + require.Equal(t, memberUser.Email, owner["email"]) }) t.Run("CreateWithAuditLogs", func(t *testing.T) {