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

kylecarbs · web-flow · commit daa34cf7b83e · 2022-10-10T18:03:54.000Z
Fixes coder#4359.
diff --git a/coderd/database/databasefake/databasefake.go b/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)
 		}
 	}
diff --git a/coderd/database/querier.go b/coderd/database/querier.go
diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go
diff --git a/coderd/database/queries/users.sql b/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
diff --git a/coderd/workspacebuilds.go b/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)
 	}
diff --git a/coderd/workspacebuilds_test.go b/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})
diff --git a/coderd/workspaces.go b/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.",

Original file line number	Diff line number	Diff line change
`@@ -479,19 +479,16 @@ func (q *fakeQuerier) GetUsers(_ context.Context, params database.GetUsersParams`
`479`	`479`	`return users, nil`
`480`	`480`	`}`
`481`	`481`
`482`		`-func (q *fakeQuerier) GetUsersByIDs(_ context.Context, params database.GetUsersByIDsParams) ([]database.User, error) {`
	`482`	`+func (q *fakeQuerier) GetUsersByIDs(_ context.Context, ids []uuid.UUID) ([]database.User, error) {`
`483`	`483`	`q.mutex.RLock()`
`484`	`484`	`defer q.mutex.RUnlock()`
`485`	`485`
`486`	`486`	`users := make([]database.User, 0)`
`487`	`487`	`for _, user := range q.users {`
`488`		`- for _, id := range params.IDs {`
	`488`	`+ for _, id := range ids {`
`489`	`489`	`if user.ID.String() != id.String() {`
`490`	`490`	`continue`
`491`	`491`	`}`
`492`		`- if user.Deleted != params.Deleted {`
`493`		`- continue`
`494`		`- }`
`495`	`492`	`users = append(users, user)`
`496`	`493`	`}`
`497`	`494`	`}`