From 98d795632e9023455e4cb9802a8e4519ad94a709 Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Mon, 27 Jan 2025 13:31:58 +0000 Subject: [PATCH 1/4] chore: add email to webhook data --- coderd/workspacebuilds.go | 2 +- coderd/workspaces.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/coderd/workspacebuilds.go b/coderd/workspacebuilds.go index 375eaab5cd33b..76166bfcb6164 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/workspaces.go b/coderd/workspaces.go index 158f27132b427..7a64648033c79 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", From adc08157560e880047400e94079a3e566a638489 Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Mon, 27 Jan 2025 16:04:21 +0000 Subject: [PATCH 2/4] chore: add tests to ensure email field is present --- coderd/workspacebuilds_test.go | 8 +++++++- coderd/workspaces_test.go | 6 ++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/coderd/workspacebuilds_test.go b/coderd/workspacebuilds_test.go index da4c09329cc39..fc8961a8c74ac 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_test.go b/coderd/workspaces_test.go index e74d5174123a1..b8bf71c3eb3ac 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) { From 844cf8e142a5572b0f8189753cb78938f0edb8a7 Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Thu, 30 Jan 2025 12:51:53 +0000 Subject: [PATCH 3/4] chore: add data to some account notifications --- coderd/users.go | 28 +++++++++++++++++++++------- coderd/users_test.go | 25 +++++++++++++++++++------ 2 files changed, 40 insertions(+), 13 deletions(-) diff --git a/coderd/users.go b/coderd/users.go index 56f295986859c..964f18724449a 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 1386d76f3e0bf..852a5e3097375 100644 --- a/coderd/users_test.go +++ b/coderd/users_test.go @@ -393,10 +393,16 @@ func TestNotifyUserStatusChanged(t *testing.T) { for _, expected := range expectedNotifications { found := false for _, sent := range notifyEnq.Sent() { + userData, userDataOk := sent.Data["user"].(map[string]any) + if sent.TemplateID == expected.TemplateID && sent.UserID == expected.UserID && slices.Contains(sent.Targets, member.ID) && - sent.Labels[label] == member.Username { + sent.Labels[label] == member.Username && + userDataOk && + userData["id"] == member.ID && + userData["name"] == member.Name && + userData["email"] == member.Email { found = true break } @@ -858,11 +864,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"]) + + userData, ok := sent[0].Data["user"].(map[string]any) + require.True(t, ok, "notification data should have user") + 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) { From 3b15dca9da121bea3ba6d7cc464156f41d8e9b85 Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Thu, 30 Jan 2025 15:13:25 +0000 Subject: [PATCH 4/4] chore: use 'require.IsType' --- coderd/users_test.go | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/coderd/users_test.go b/coderd/users_test.go index 852a5e3097375..53ec98b30d911 100644 --- a/coderd/users_test.go +++ b/coderd/users_test.go @@ -392,18 +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() { - userData, userDataOk := sent.Data["user"].(map[string]any) - + 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 && - userDataOk && - userData["id"] == member.ID && - userData["name"] == member.Name && - userData["email"] == member.Email { + 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 } } @@ -871,8 +872,8 @@ func TestNotifyCreatedUser(t *testing.T) { require.Contains(t, sent[0].Targets, user.ID) require.Equal(t, user.Username, sent[0].Labels["created_account_name"]) - userData, ok := sent[0].Data["user"].(map[string]any) - require.True(t, ok, "notification data should have user") + 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"])