Skip to content

Commit f651ab9

Browse files
DanielleMaywoodDanielleMaywood
authored
chore: add 'email' field to notifications (#16336)
Closes coder/internal#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.
1 parent 447cc0d commit f651ab9

File tree

6 files changed

+56
-16
lines changed

6 files changed

+56
-16
lines changed

‎coderd/users.go

+21-7
Original file line numberDiff line numberDiff line change
@@ -918,6 +918,7 @@ func (api *API) putUserStatus(status database.UserStatus) func(rw http.ResponseW
918918

919919
func (api *API) notifyUserStatusChanged(ctx context.Context, actingUserName string, targetUser database.User, status database.UserStatus) error {
920920
var labels map[string]string
921+
var data map[string]any
921922
var adminTemplateID, personalTemplateID uuid.UUID
922923
switch status {
923924
case database.UserStatusSuspended:
@@ -926,6 +927,9 @@ func (api *API) notifyUserStatusChanged(ctx context.Context, actingUserName stri
926927
"suspended_account_user_name": targetUser.Name,
927928
"initiator": actingUserName,
928929
}
930+
data = map[string]any{
931+
"user": map[string]any{"id": targetUser.ID, "name": targetUser.Name, "email": targetUser.Email},
932+
}
929933
adminTemplateID = notifications.TemplateUserAccountSuspended
930934
personalTemplateID = notifications.TemplateYourAccountSuspended
931935
case database.UserStatusActive:
@@ -934,6 +938,9 @@ func (api *API) notifyUserStatusChanged(ctx context.Context, actingUserName stri
934938
"activated_account_user_name": targetUser.Name,
935939
"initiator": actingUserName,
936940
}
941+
data = map[string]any{
942+
"user": map[string]any{"id": targetUser.ID, "name": targetUser.Name, "email": targetUser.Email},
943+
}
937944
adminTemplateID = notifications.TemplateUserAccountActivated
938945
personalTemplateID = notifications.TemplateYourAccountActivated
939946
default:
@@ -949,16 +956,16 @@ func (api *API) notifyUserStatusChanged(ctx context.Context, actingUserName stri
949956
// Send notifications to user admins and affected user
950957
for _, u := range userAdmins {
951958
// nolint:gocritic // Need notifier actor to enqueue notifications
952-
if _, err := api.NotificationsEnqueuer.Enqueue(dbauthz.AsNotifier(ctx), u.ID, adminTemplateID,
953-
labels, "api-put-user-status",
959+
if _, err := api.NotificationsEnqueuer.EnqueueWithData(dbauthz.AsNotifier(ctx), u.ID, adminTemplateID,
960+
labels, data, "api-put-user-status",
954961
targetUser.ID,
955962
); err != nil {
956963
api.Logger.Warn(ctx, "unable to notify about changed user's status", slog.F("affected_user", targetUser.Username), slog.Error(err))
957964
}
958965
}
959966
// nolint:gocritic // Need notifier actor to enqueue notifications
960-
if _, err := api.NotificationsEnqueuer.Enqueue(dbauthz.AsNotifier(ctx), targetUser.ID, personalTemplateID,
961-
labels, "api-put-user-status",
967+
if _, err := api.NotificationsEnqueuer.EnqueueWithData(dbauthz.AsNotifier(ctx), targetUser.ID, personalTemplateID,
968+
labels, data, "api-put-user-status",
962969
targetUser.ID,
963970
); err != nil {
964971
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
14241431
}
14251432

14261433
for _, u := range userAdmins {
1427-
// nolint:gocritic // Need notifier actor to enqueue notifications
1428-
if _, err := api.NotificationsEnqueuer.Enqueue(dbauthz.AsNotifier(ctx), u.ID, notifications.TemplateUserAccountCreated,
1434+
if _, err := api.NotificationsEnqueuer.EnqueueWithData(
1435+
// nolint:gocritic // Need notifier actor to enqueue notifications
1436+
dbauthz.AsNotifier(ctx),
1437+
u.ID,
1438+
notifications.TemplateUserAccountCreated,
14291439
map[string]string{
14301440
"created_account_name": user.Username,
14311441
"created_account_user_name": user.Name,
14321442
"initiator": req.accountCreatorName,
1433-
}, "api-users-create",
1443+
},
1444+
map[string]any{
1445+
"user": map[string]any{"id": user.ID, "name": user.Name, "email": user.Email},
1446+
},
1447+
"api-users-create",
14341448
user.ID,
14351449
); err != nil {
14361450
api.Logger.Warn(ctx, "unable to notify about created user", slog.F("created_user", user.Username), slog.Error(err))

‎coderd/users_test.go

+20-6
Original file line numberDiff line numberDiff line change
@@ -392,12 +392,19 @@ func TestNotifyUserStatusChanged(t *testing.T) {
392392
// Validate that each expected notification is present in notifyEnq.Sent()
393393
for _, expected := range expectedNotifications {
394394
found := false
395-
for _, sent := range notifyEnq.Sent() {
395+
for _, sent := range notifyEnq.Sent(notificationstest.WithTemplateID(expected.TemplateID)) {
396396
if sent.TemplateID == expected.TemplateID &&
397397
sent.UserID == expected.UserID &&
398398
slices.Contains(sent.Targets, member.ID) &&
399399
sent.Labels[label] == member.Username {
400400
found = true
401+
402+
require.IsType(t, map[string]any{}, sent.Data["user"])
403+
userData := sent.Data["user"].(map[string]any)
404+
require.Equal(t, member.ID, userData["id"])
405+
require.Equal(t, member.Name, userData["name"])
406+
require.Equal(t, member.Email, userData["email"])
407+
401408
break
402409
}
403410
}
@@ -858,11 +865,18 @@ func TestNotifyCreatedUser(t *testing.T) {
858865
require.NoError(t, err)
859866

860867
// then
861-
require.Len(t, notifyEnq.Sent(), 1)
862-
require.Equal(t, notifications.TemplateUserAccountCreated, notifyEnq.Sent()[0].TemplateID)
863-
require.Equal(t, firstUser.UserID, notifyEnq.Sent()[0].UserID)
864-
require.Contains(t, notifyEnq.Sent()[0].Targets, user.ID)
865-
require.Equal(t, user.Username, notifyEnq.Sent()[0].Labels["created_account_name"])
868+
sent := notifyEnq.Sent(notificationstest.WithTemplateID(notifications.TemplateUserAccountCreated))
869+
require.Len(t, sent, 1)
870+
require.Equal(t, notifications.TemplateUserAccountCreated, sent[0].TemplateID)
871+
require.Equal(t, firstUser.UserID, sent[0].UserID)
872+
require.Contains(t, sent[0].Targets, user.ID)
873+
require.Equal(t, user.Username, sent[0].Labels["created_account_name"])
874+
875+
require.IsType(t, map[string]any{}, sent[0].Data["user"])
876+
userData := sent[0].Data["user"].(map[string]any)
877+
require.Equal(t, user.ID, userData["id"])
878+
require.Equal(t, user.Name, userData["name"])
879+
require.Equal(t, user.Email, userData["email"])
866880
})
867881

868882
t.Run("UserAdminNotified", func(t *testing.T) {

‎coderd/workspacebuilds.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -527,7 +527,7 @@ func (api *API) notifyWorkspaceUpdated(
527527
"workspace": map[string]any{"id": workspace.ID, "name": workspace.Name},
528528
"template": map[string]any{"id": template.ID, "name": template.Name},
529529
"template_version": map[string]any{"id": version.ID, "name": version.Name},
530-
"owner": map[string]any{"id": owner.ID, "name": owner.Name},
530+
"owner": map[string]any{"id": owner.ID, "name": owner.Name, "email": owner.Email},
531531
"parameters": buildParameters,
532532
},
533533
"api-workspaces-updated",

‎coderd/workspacebuilds_test.go

+7-1
Original file line numberDiff line numberDiff line change
@@ -648,7 +648,7 @@ func TestWorkspaceBuildWithUpdatedTemplateVersionSendsNotification(t *testing.T)
648648
client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true, NotificationsEnqueuer: notify})
649649
first := coderdtest.CreateFirstUser(t, client)
650650
templateAdminClient, templateAdmin := coderdtest.CreateAnotherUser(t, client, first.OrganizationID, rbac.RoleTemplateAdmin())
651-
userClient, _ := coderdtest.CreateAnotherUser(t, client, first.OrganizationID)
651+
userClient, user := coderdtest.CreateAnotherUser(t, client, first.OrganizationID)
652652

653653
// Create a template with an initial version
654654
version := coderdtest.CreateTemplateVersion(t, templateAdminClient, first.OrganizationID, nil)
@@ -684,6 +684,12 @@ func TestWorkspaceBuildWithUpdatedTemplateVersionSendsNotification(t *testing.T)
684684
require.Contains(t, sent[0].Targets, workspace.ID)
685685
require.Contains(t, sent[0].Targets, workspace.OrganizationID)
686686
require.Contains(t, sent[0].Targets, workspace.OwnerID)
687+
688+
owner, ok := sent[0].Data["owner"].(map[string]any)
689+
require.True(t, ok, "notification data should have owner")
690+
require.Equal(t, user.ID, owner["id"])
691+
require.Equal(t, user.Name, owner["name"])
692+
require.Equal(t, user.Email, owner["email"])
687693
})
688694
}
689695

‎coderd/workspaces.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -809,7 +809,7 @@ func (api *API) notifyWorkspaceCreated(
809809
"workspace": map[string]any{"id": workspace.ID, "name": workspace.Name},
810810
"template": map[string]any{"id": template.ID, "name": template.Name},
811811
"template_version": map[string]any{"id": version.ID, "name": version.Name},
812-
"owner": map[string]any{"id": owner.ID, "name": owner.Name},
812+
"owner": map[string]any{"id": owner.ID, "name": owner.Name, "email": owner.Email},
813813
"parameters": buildParameters,
814814
},
815815
"api-workspaces-create",

‎coderd/workspaces_test.go

+6
Original file line numberDiff line numberDiff line change
@@ -639,6 +639,12 @@ func TestPostWorkspacesByOrganization(t *testing.T) {
639639
require.Contains(t, sent[0].Targets, workspace.ID)
640640
require.Contains(t, sent[0].Targets, workspace.OrganizationID)
641641
require.Contains(t, sent[0].Targets, workspace.OwnerID)
642+
643+
owner, ok := sent[0].Data["owner"].(map[string]any)
644+
require.True(t, ok, "notification data should have owner")
645+
require.Equal(t, memberUser.ID, owner["id"])
646+
require.Equal(t, memberUser.Name, owner["name"])
647+
require.Equal(t, memberUser.Email, owner["email"])
642648
})
643649

644650
t.Run("CreateWithAuditLogs", func(t *testing.T) {

0 commit comments

Comments
 (0)