feat(pr edit): fetch assignable actors

api/queries_repo.go

@@ -918,39 +918,30 @@ func RepoMetadata(client *Client, repo ghrepo.Interface, input RepoMetadataInput
 	var g errgroup.Group
 
 	if input.Assignees || input.Reviewers {
-
 		if input.ActorAssignees {
 			g.Go(func() error {
 				actors, err := RepoAssignableActors(client, repo)
 				if err != nil {
-					err = fmt.Errorf("error fetching assignees: %w", err)
+					return fmt.Errorf("error fetching assignees: %w", err)
 				}
 				result.AssignableActors = actors
-				return err
-			})
 
-			// If reviewers are also requested, we still need to fetch the assignable users
-			// since commands use assignable users for reviewers too,
-			// but Actors are not supported for requesting review (need to confirm this).
-			// TODO KW: find out how to do this in the above query so we don't need to
-			// run two potentially expensive queries. When we fetch Actors, this
-			// should still return Users - Users are distinguishable from other Actors
-			// by having a name property. Maybe we can use the Name to filter out
-			// non-user Actors and populate the users list for reviewers based on
-			// that.
-			// Note: this only matters for `gh pr` flows, which currently does not
-			// request actor assignees, so we probably won't hit this until
-			// `gh pr` reqeuests actor assignees.
-			if input.Reviewers {
-				g.Go(func() error {
-					users, err := RepoAssignableUsers(client, repo)
-					if err != nil {
-						err = fmt.Errorf("error fetching assignees: %w", err)
+				// Processing the bots out from the actors
+				// because requesting a reviewer leverages
+				// result.AssignableUsers.
+				// Note: this prevents us from needing to make another
+				// request to fetch assignable users when the user has
+				// selected to modify both reviewers and assignees.
+				var users []AssignableUser
+				for _, a := range actors {
+					if _, ok := a.(AssignableUser); !ok {
+						continue
 					}
-					result.AssignableUsers = users
-					return err
-				})
-			}
+					users = append(users, a.(AssignableUser))
+				}
+				result.AssignableUsers = users
+				return nil
+			})
 		} else {
 			// Not using Actors, fetch legacy assignable users.
 			g.Go(func() error {
@@ -962,7 +953,6 @@ func RepoMetadata(client *Client, repo ghrepo.Interface, input RepoMetadataInput
 				return err
 			})
 		}
-
 	}
 
 	if input.Reviewers {

pkg/cmd/pr/edit/edit.go

@@ -238,7 +238,7 @@ func editRun(opts *EditOptions) error {
 	editable.Base.Default = pr.BaseRefName
 	editable.Reviewers.Default = pr.ReviewRequests.Logins()
 	if issueFeatures.ActorIsAssignable {
-		// editable.Assignees.ActorAssignees = true
+		editable.Assignees.ActorAssignees = true
 		editable.Assignees.Default = pr.AssignedActors.Logins()
 	} else {
 		editable.Assignees.Default = pr.Assignees.Logins()

pkg/cmd/pr/edit/edit_test.go

@@ -9,6 +9,7 @@ import (
 	"testing"
 
 	"github.com/cli/cli/v2/api"
+	fd "github.com/cli/cli/v2/internal/featuredetection"
 	"github.com/cli/cli/v2/internal/ghrepo"
 	shared "github.com/cli/cli/v2/pkg/cmd/pr/shared"
 	"github.com/cli/cli/v2/pkg/cmdutil"
@@ -392,6 +393,7 @@ func Test_editRun(t *testing.T) {
 			httpStubs: func(reg *httpmock.Registry) {
 				mockRepoMetadata(reg, false)
 				mockPullRequestUpdate(reg)
+				mockPullRequestUpdateActorAssignees(reg)
 				mockPullRequestReviewersUpdate(reg)
 				mockPullRequestUpdateLabels(reg)
 				mockProjectV2ItemUpdate(reg)
@@ -448,6 +450,7 @@ func Test_editRun(t *testing.T) {
 			httpStubs: func(reg *httpmock.Registry) {
 				mockRepoMetadata(reg, true)
 				mockPullRequestUpdate(reg)
+				mockPullRequestUpdateActorAssignees(reg)
 				mockPullRequestUpdateLabels(reg)
 				mockProjectV2ItemUpdate(reg)
 			},
@@ -468,6 +471,7 @@ func Test_editRun(t *testing.T) {
 			httpStubs: func(reg *httpmock.Registry) {
 				mockRepoMetadata(reg, false)
 				mockPullRequestUpdate(reg)
+				mockPullRequestUpdateActorAssignees(reg)
 				mockPullRequestReviewersUpdate(reg)
 				mockPullRequestUpdateLabels(reg)
 				mockProjectV2ItemUpdate(reg)
@@ -489,11 +493,50 @@ func Test_editRun(t *testing.T) {
 			httpStubs: func(reg *httpmock.Registry) {
 				mockRepoMetadata(reg, true)
 				mockPullRequestUpdate(reg)
+				mockPullRequestUpdateActorAssignees(reg)
 				mockPullRequestUpdateLabels(reg)
 				mockProjectV2ItemUpdate(reg)
 			},
 			stdout: "https://github.com/OWNER/REPO/pull/123\n",
 		},
+		{
+			name: "Legacy assignee user are fetched and updated on unsupported GitHub Hosts",
+			input: &EditOptions{
+				Detector:    &fd.DisabledDetectorMock{},
+				SelectorArg: "123",
+				Finder: shared.NewMockFinder("123", &api.PullRequest{
+					URL: "https://github.com/OWNER/REPO/pull/123",
+				}, ghrepo.New("OWNER", "REPO")),
+				Interactive: false,
+				Editable: shared.Editable{
+					Assignees: shared.EditableAssignees{
+						EditableSlice: shared.EditableSlice{
+							Add:    []string{"monalisa", "hubot"},
+							Remove: []string{"octocat"},
+							Edited: true,
+						},
+					},
+				},
+				Fetcher: testFetcher{},
+			},
+			httpStubs: func(reg *httpmock.Registry) {
+				// Notice there is no call to mockReplaceActorsForAssignable()
+				// and no GraphQL call to RepositoryAssignableActors below.
+				reg.Register(
+					httpmock.GraphQL(`query RepositoryAssignableUsers\b`),
+					httpmock.StringResponse(`
+					{ "data": { "repository": { "assignableUsers": {
+						"nodes": [
+							{ "login": "hubot", "id": "HUBOTID" },
+							{ "login": "MonaLisa", "id": "MONAID" }
+						],
+						"pageInfo": { "hasNextPage": false }
+					} } } }
+					`))
+				mockPullRequestUpdate(reg)
+			},
+			stdout: "https://github.com/OWNER/REPO/pull/123\n",
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -523,16 +566,16 @@ func Test_editRun(t *testing.T) {
 
 func mockRepoMetadata(reg *httpmock.Registry, skipReviewers bool) {
 	reg.Register(
-		httpmock.GraphQL(`query RepositoryAssignableUsers\b`),
+		httpmock.GraphQL(`query RepositoryAssignableActors\b`),
 		httpmock.StringResponse(`
-		{ "data": { "repository": { "assignableUsers": {
-			"nodes": [
-				{ "login": "hubot", "id": "HUBOTID" },
-				{ "login": "MonaLisa", "id": "MONAID" }
-			],
-			"pageInfo": { "hasNextPage": false }
-		} } } }
-		`))
+			{ "data": { "repository": { "suggestedActors": {
+				"nodes": [
+					{ "login": "hubot", "id": "HUBOTID", "__typename": "Bot" },
+					{ "login": "MonaLisa", "id": "MONAID", "__typename": "User" }
+				],
+				"pageInfo": { "hasNextPage": false }
+			} } } }
+			`))
 	reg.Register(
 		httpmock.GraphQL(`query RepositoryLabelList\b`),
 		httpmock.StringResponse(`
@@ -635,6 +678,15 @@ func mockPullRequestUpdate(reg *httpmock.Registry) {
 		httpmock.StringResponse(`{}`))
 }
 
+func mockPullRequestUpdateActorAssignees(reg *httpmock.Registry) {
+	reg.Register(
+		httpmock.GraphQL(`mutation ReplaceActorsForAssignable\b`),
+		httpmock.GraphQLMutation(`
+		{ "data": { "replaceActorsForAssignable": { "__typename": "" } } }`,
+			func(inputs map[string]interface{}) {}),
+	)
+}
+
 func mockPullRequestReviewersUpdate(reg *httpmock.Registry) {
 	reg.Register(
 		httpmock.GraphQL(`mutation PullRequestUpdateRequestReviews\b`),