fix: Return deleted users when fetching workspace builds (#4441)

Fixes #4359.

coderd/database/databasefake/databasefake.go

@@ -479,19 +479,16 @@ func (q *fakeQuerier) GetUsers(_ context.Context, params database.GetUsersParams
 	return users, nil
 }
 
-func (q *fakeQuerier) GetUsersByIDs(_ context.Context, params database.GetUsersByIDsParams) ([]database.User, error) {
+func (q *fakeQuerier) GetUsersByIDs(_ context.Context, ids []uuid.UUID) ([]database.User, error) {
 	q.mutex.RLock()
 	defer q.mutex.RUnlock()
 
 	users := make([]database.User, 0)
 	for _, user := range q.users {
-		for _, id := range params.IDs {
+		for _, id := range ids {
 			if user.ID.String() != id.String() {
 				continue
 			}
-			if user.Deleted != params.Deleted {
-				continue
-			}
 			users = append(users, user)
 		}
 	}

coderd/database/querier.go

@@ -76,7 +76,10 @@ type querier interface {
 	GetUserLinkByLinkedID(ctx context.Context, linkedID string) (UserLink, error)
 	GetUserLinkByUserIDLoginType(ctx context.Context, arg GetUserLinkByUserIDLoginTypeParams) (UserLink, error)
 	GetUsers(ctx context.Context, arg GetUsersParams) ([]User, error)
-	GetUsersByIDs(ctx context.Context, arg GetUsersByIDsParams) ([]User, error)
+	// This shouldn't check for deleted, because it's frequently used
+	// to look up references to actions. eg. a user could build a workspace
+	// for another user, then be deleted... we still want them to appear!
+	GetUsersByIDs(ctx context.Context, ids []uuid.UUID) ([]User, error)
 	GetWorkspaceAgentByAuthToken(ctx context.Context, authToken uuid.UUID) (WorkspaceAgent, error)
 	GetWorkspaceAgentByID(ctx context.Context, id uuid.UUID) (WorkspaceAgent, error)
 	GetWorkspaceAgentByInstanceID(ctx context.Context, authInstanceID string) (WorkspaceAgent, error)

coderd/database/queries.sql.go

@@ -3305,16 +3305,14 @@ func (q *sqlQuerier) GetUsers(ctx context.Context, arg GetUsersParams) ([]User,
 }
 
 const getUsersByIDs = `-- name: GetUsersByIDs :many
-SELECT id, email, username, hashed_password, created_at, updated_at, status, rbac_roles, login_type, avatar_url, deleted, last_seen_at FROM users WHERE id = ANY($1 :: uuid [ ]) AND deleted = $2
+SELECT id, email, username, hashed_password, created_at, updated_at, status, rbac_roles, login_type, avatar_url, deleted, last_seen_at FROM users WHERE id = ANY($1 :: uuid [ ])
 `
 
-type GetUsersByIDsParams struct {
-	IDs     []uuid.UUID `db:"ids" json:"ids"`
-	Deleted bool        `db:"deleted" json:"deleted"`
-}
-
-func (q *sqlQuerier) GetUsersByIDs(ctx context.Context, arg GetUsersByIDsParams) ([]User, error) {
-	rows, err := q.db.QueryContext(ctx, getUsersByIDs, pq.Array(arg.IDs), arg.Deleted)
+// This shouldn't check for deleted, because it's frequently used
+// to look up references to actions. eg. a user could build a workspace
+// for another user, then be deleted... we still want them to appear!
+func (q *sqlQuerier) GetUsersByIDs(ctx context.Context, ids []uuid.UUID) ([]User, error) {
+	rows, err := q.db.QueryContext(ctx, getUsersByIDs, pq.Array(ids))
 	if err != nil {
 		return nil, err
 	}

coderd/database/queries/users.sql

@@ -9,7 +9,10 @@ LIMIT
 	1;
 
 -- name: GetUsersByIDs :many
-SELECT * FROM users WHERE id = ANY(@ids :: uuid [ ]) AND deleted = @deleted;
+-- This shouldn't check for deleted, because it's frequently used
+-- to look up references to actions. eg. a user could build a workspace
+-- for another user, then be deleted... we still want them to appear!
+SELECT * FROM users WHERE id = ANY(@ids :: uuid [ ]);
 
 -- name: GetUserByEmailOrUsername :one
 SELECT

coderd/workspacebuilds.go

@@ -492,11 +492,9 @@ func (api *API) postWorkspaceBuilds(rw http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	users, err := api.Database.GetUsersByIDs(ctx, database.GetUsersByIDsParams{
-		IDs: []uuid.UUID{
-			workspace.OwnerID,
-			workspaceBuild.InitiatorID,
-		},
+	users, err := api.Database.GetUsersByIDs(ctx, []uuid.UUID{
+		workspace.OwnerID,
+		workspaceBuild.InitiatorID,
 	})
 	if err != nil {
 		httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
@@ -679,9 +677,7 @@ func (api *API) workspaceBuildsData(ctx context.Context, workspaces []database.W
 	for _, workspace := range workspaces {
 		userIDs = append(userIDs, workspace.OwnerID)
 	}
-	users, err := api.Database.GetUsersByIDs(ctx, database.GetUsersByIDsParams{
-		IDs: userIDs,
-	})
+	users, err := api.Database.GetUsersByIDs(ctx, userIDs)
 	if err != nil {
 		return workspaceBuildsData{}, xerrors.Errorf("get users: %w", err)
 	}

coderd/workspacebuilds_test.go

@@ -178,6 +178,37 @@ func TestWorkspaceBuilds(t *testing.T) {
 		require.Len(t, builds, 1)
 	})
 
+	t.Run("DeletedInitiator", func(t *testing.T) {
+		t.Parallel()
+		client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true})
+		first := coderdtest.CreateFirstUser(t, client)
+		second := coderdtest.CreateAnotherUser(t, client, first.OrganizationID, "owner")
+
+		ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
+		defer cancel()
+
+		secondUser, err := second.User(ctx, codersdk.Me)
+		require.NoError(t, err, "fetch me")
+		version := coderdtest.CreateTemplateVersion(t, client, first.OrganizationID, nil)
+		template := coderdtest.CreateTemplate(t, client, first.OrganizationID, version.ID)
+		coderdtest.AwaitTemplateVersionJob(t, client, version.ID)
+		workspace, err := second.CreateWorkspace(ctx, first.OrganizationID, first.UserID.String(), codersdk.CreateWorkspaceRequest{
+			TemplateID: template.ID,
+			Name:       "example",
+		})
+		require.NoError(t, err)
+		coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID)
+
+		err = client.DeleteUser(ctx, secondUser.ID)
+		require.NoError(t, err)
+
+		builds, err := client.WorkspaceBuilds(ctx, codersdk.WorkspaceBuildsRequest{WorkspaceID: workspace.ID})
+		require.Len(t, builds, 1)
+		require.Equal(t, int32(1), builds[0].BuildNumber)
+		require.Equal(t, secondUser.Username, builds[0].InitiatorUsername)
+		require.NoError(t, err)
+	})
+
 	t.Run("PaginateNonExistentRow", func(t *testing.T) {
 		t.Parallel()
 		client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true})

coderd/workspaces.go

@@ -463,9 +463,7 @@ func (api *API) postWorkspacesByOrganization(rw http.ResponseWriter, r *http.Req
 	}
 	aReq.New = workspace
 
-	users, err := api.Database.GetUsersByIDs(ctx, database.GetUsersByIDsParams{
-		IDs: []uuid.UUID{user.ID, workspaceBuild.InitiatorID},
-	})
+	users, err := api.Database.GetUsersByIDs(ctx, []uuid.UUID{user.ID, workspaceBuild.InitiatorID})
 	if err != nil {
 		httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
 			Message: "Internal error fetching user.",