perf: use a single query for notification target lookups

ethanndickson · ethanndickson · commit a131bc3e20d1 · 2025-10-30T03:35:20.000Z
diff --git a/coderd/templates.go b/coderd/templates.go
@@ -1130,19 +1130,11 @@ func (api *API) convertTemplate(
 
 // findTemplateAdmins fetches all users with template admin permission including owners.
 func findTemplateAdmins(ctx context.Context, store database.Store) ([]database.GetUsersRow, error) {
-	// Notice: we can't scrape the user information in parallel as pq
-	// fails with: unexpected describe rows response: 'D'
-	owners, err := store.GetUsers(ctx, database.GetUsersParams{
-		RbacRole: []string{codersdk.RoleOwner},
-	})
-	if err != nil {
-		return nil, xerrors.Errorf("get owners: %w", err)
-	}
 	templateAdmins, err := store.GetUsers(ctx, database.GetUsersParams{
-		RbacRole: []string{codersdk.RoleTemplateAdmin},
+		RbacRole: []string{codersdk.RoleTemplateAdmin, codersdk.RoleOwner},
 	})
 	if err != nil {
-		return nil, xerrors.Errorf("get template admins: %w", err)
+		return nil, xerrors.Errorf("get owners: %w", err)
 	}
-	return append(owners, templateAdmins...), nil
+	return templateAdmins, nil
 }
diff --git a/coderd/users.go b/coderd/users.go
@@ -1537,21 +1537,13 @@ func (api *API) CreateUser(ctx context.Context, store database.Store, req Create
 
 // findUserAdmins fetches all users with user admin permission including owners.
 func findUserAdmins(ctx context.Context, store database.Store) ([]database.GetUsersRow, error) {
-	// Notice: we can't scrape the user information in parallel as pq
-	// fails with: unexpected describe rows response: 'D'
-	owners, err := store.GetUsers(ctx, database.GetUsersParams{
-		RbacRole: []string{codersdk.RoleOwner},
-	})
-	if err != nil {
-		return nil, xerrors.Errorf("get owners: %w", err)
-	}
 	userAdmins, err := store.GetUsers(ctx, database.GetUsersParams{
-		RbacRole: []string{codersdk.RoleUserAdmin},
+		RbacRole: []string{codersdk.RoleOwner, codersdk.RoleUserAdmin},
 	})
 	if err != nil {
-		return nil, xerrors.Errorf("get user admins: %w", err)
+		return nil, xerrors.Errorf("get owners: %w", err)
 	}
-	return append(owners, userAdmins...), nil
+	return userAdmins, nil
 }
 
 func convertUsers(users []database.User, organizationIDsByUserID map[uuid.UUID][]uuid.UUID) []codersdk.User {
diff --git a/coderd/users_test.go b/coderd/users_test.go
@@ -604,16 +604,22 @@ func TestNotifyDeletedUser(t *testing.T) {
 		// sent[2]: "Member" account created, "user admin" notified
 
 		// "Member" account deleted, "owner" notified
-		require.Equal(t, notifications.TemplateUserAccountDeleted, sent[3].TemplateID)
-		require.Equal(t, firstUser.UserID, sent[3].UserID)
-		require.Contains(t, sent[3].Targets, member.ID)
-		require.Equal(t, member.Username, sent[3].Labels["deleted_account_name"])
+		ownerNotifications := notifyEnq.Sent(func(n *notificationstest.FakeNotification) bool {
+			return n.TemplateID == notifications.TemplateUserAccountDeleted &&
+				n.UserID == firstUser.UserID &&
+				slices.Contains(n.Targets, member.ID) &&
+				n.Labels["deleted_account_name"] == member.Username
+		})
+		require.Len(t, ownerNotifications, 1)
 
 		// "Member" account deleted, "user admin" notified
-		require.Equal(t, notifications.TemplateUserAccountDeleted, sent[4].TemplateID)
-		require.Equal(t, userAdmin.ID, sent[4].UserID)
-		require.Contains(t, sent[4].Targets, member.ID)
-		require.Equal(t, member.Username, sent[4].Labels["deleted_account_name"])
+		adminNotifications := notifyEnq.Sent(func(n *notificationstest.FakeNotification) bool {
+			return n.TemplateID == notifications.TemplateUserAccountDeleted &&
+				n.UserID == userAdmin.ID &&
+				slices.Contains(n.Targets, member.ID) &&
+				n.Labels["deleted_account_name"] == member.Username
+		})
+		require.Len(t, adminNotifications, 1)
 	})
 }
 
@@ -960,22 +966,31 @@ func TestNotifyCreatedUser(t *testing.T) {
 		require.Len(t, sent, 3)
 
 		// "User admin" account created, "owner" notified
-		require.Equal(t, notifications.TemplateUserAccountCreated, sent[0].TemplateID)
-		require.Equal(t, firstUser.UserID, sent[0].UserID)
-		require.Contains(t, sent[0].Targets, userAdmin.ID)
-		require.Equal(t, userAdmin.Username, sent[0].Labels["created_account_name"])
+		ownerNotifiedAboutUserAdmin := notifyEnq.Sent(func(n *notificationstest.FakeNotification) bool {
+			return n.TemplateID == notifications.TemplateUserAccountCreated &&
+				n.UserID == firstUser.UserID &&
+				slices.Contains(n.Targets, userAdmin.ID) &&
+				n.Labels["created_account_name"] == userAdmin.Username
+		})
+		require.Len(t, ownerNotifiedAboutUserAdmin, 1)
 
 		// "Member" account created, "owner" notified
-		require.Equal(t, notifications.TemplateUserAccountCreated, sent[1].TemplateID)
-		require.Equal(t, firstUser.UserID, sent[1].UserID)
-		require.Contains(t, sent[1].Targets, member.ID)
-		require.Equal(t, member.Username, sent[1].Labels["created_account_name"])
+		ownerNotifiedAboutMember := notifyEnq.Sent(func(n *notificationstest.FakeNotification) bool {
+			return n.TemplateID == notifications.TemplateUserAccountCreated &&
+				n.UserID == firstUser.UserID &&
+				slices.Contains(n.Targets, member.ID) &&
+				n.Labels["created_account_name"] == member.Username
+		})
+		require.Len(t, ownerNotifiedAboutMember, 1)
 
 		// "Member" account created, "user admin" notified
-		require.Equal(t, notifications.TemplateUserAccountCreated, sent[1].TemplateID)
-		require.Equal(t, userAdmin.ID, sent[2].UserID)
-		require.Contains(t, sent[2].Targets, member.ID)
-		require.Equal(t, member.Username, sent[2].Labels["created_account_name"])
+		userAdminNotifiedAboutMember := notifyEnq.Sent(func(n *notificationstest.FakeNotification) bool {
+			return n.TemplateID == notifications.TemplateUserAccountCreated &&
+				n.UserID == userAdmin.ID &&
+				slices.Contains(n.Targets, member.ID) &&
+				n.Labels["created_account_name"] == member.Username
+		})
+		require.Len(t, userAdminNotifiedAboutMember, 1)
 	})
 }
 
