diff --git a/api/queries_repo.go b/api/queries_repo.go index 06ae48b12..99041e49f 100644 --- a/api/queries_repo.go +++ b/api/queries_repo.go @@ -681,13 +681,14 @@ func RepoFindForks(client *Client, repo ghrepo.Interface, limit int) ([]*Reposit } type RepoMetadataResult struct { - CurrentLogin string - AssignableUsers []RepoAssignee - Labels []RepoLabel - Projects []RepoProject - ProjectsV2 []ProjectV2 - Milestones []RepoMilestone - Teams []OrgTeam + CurrentLogin string + AssignableUsers []RepoAssignee + AssignableActors []RepoAssignee + Labels []RepoLabel + Projects []RepoProject + ProjectsV2 []ProjectV2 + Milestones []RepoMilestone + Teams []OrgTeam } func (m *RepoMetadataResult) MembersToIDs(names []string) ([]string, error) { @@ -701,6 +702,16 @@ func (m *RepoMetadataResult) MembersToIDs(names []string) ([]string, error) { break } } + + // Look for ID in assignable actors if not found in assignable users + for _, u := range m.AssignableActors { + if strings.EqualFold(assigneeLogin, u.Login) { + ids = append(ids, u.ID) + found = true + break + } + } + if !found { return nil, fmt.Errorf("'%s' not found", assigneeLogin) } @@ -892,12 +903,13 @@ func (m *RepoMetadataResult) Merge(m2 *RepoMetadataResult) { } type RepoMetadataInput struct { - Assignees bool - Reviewers bool - Labels bool - ProjectsV1 bool - ProjectsV2 bool - Milestones bool + Assignees bool + ActorAssignees bool + Reviewers bool + Labels bool + ProjectsV1 bool + ProjectsV2 bool + Milestones bool } // RepoMetadata pre-fetches the metadata for attaching to issues and pull requests @@ -906,14 +918,48 @@ func RepoMetadata(client *Client, repo ghrepo.Interface, input RepoMetadataInput var g errgroup.Group if input.Assignees || input.Reviewers { - g.Go(func() error { - users, err := RepoAssignableUsers(client, repo) - if err != nil { - err = fmt.Errorf("error fetching assignees: %w", err) + + if input.ActorAssignees { + g.Go(func() error { + actors, err := RepoAssignableActors(client, repo) + if err != nil { + err = fmt.Errorf("error fetching assignees: %w", err) + } + result.AssignableActors = actors + return err + }) + + // If reviewers are 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. + if input.Reviewers { + g.Go(func() error { + users, err := RepoAssignableUsers(client, repo) + if err != nil { + err = fmt.Errorf("error fetching assignees: %w", err) + } + result.AssignableUsers = users + return err + }) } - result.AssignableUsers = users - return err - }) + } else { + // Not using Actors, fetch legacy assignable users. + g.Go(func() error { + users, err := RepoAssignableUsers(client, repo) + if err != nil { + err = fmt.Errorf("error fetching assignees: %w", err) + } + result.AssignableUsers = users + return err + }) + } + } if input.Reviewers { @@ -1186,6 +1232,61 @@ func RepoAssignableUsers(client *Client, repo ghrepo.Interface) ([]RepoAssignee, return users, nil } +// RepoAssignableActors fetches all the assignable actors for a repository on +// GitHub hosts that support Actor assignees. +func RepoAssignableActors(client *Client, repo ghrepo.Interface) ([]RepoAssignee, error) { + type repoActorAssignee struct { + ID string + Login string + } + + type responseData struct { + Repository struct { + SuggestedActors struct { + Nodes []struct { + User RepoAssignee `graphql:"... on User"` + Bot repoActorAssignee `graphql:"... on Bot"` + } + PageInfo struct { + HasNextPage bool + EndCursor string + } + } `graphql:"suggestedActors(first: 100, after: $endCursor, capabilities: CAN_BE_ASSIGNED)"` + } `graphql:"repository(owner: $owner, name: $name)"` + } + + variables := map[string]interface{}{ + "owner": githubv4.String(repo.RepoOwner()), + "name": githubv4.String(repo.RepoName()), + "endCursor": (*githubv4.String)(nil), + } + + var actors []RepoAssignee + for { + var query responseData + err := client.Query(repo.RepoHost(), "RepositoryAssignableActors", &query, variables) + if err != nil { + return nil, err + } + + for _, node := range query.Repository.SuggestedActors.Nodes { + // Edge case if the Actor is not a Bot or a User, + // it won't be unmarshalled properly, and we'll have an + // zero value node. + if node.User.ID == "" || node.User.Login == "" { + continue + } + actors = append(actors, node.User) + } + + if !query.Repository.SuggestedActors.PageInfo.HasNextPage { + break + } + variables["endCursor"] = githubv4.String(query.Repository.SuggestedActors.PageInfo.EndCursor) + } + return actors, nil +} + type RepoLabel struct { ID string Name string diff --git a/pkg/cmd/issue/edit/edit.go b/pkg/cmd/issue/edit/edit.go index a5694e6c9..9177f60ad 100644 --- a/pkg/cmd/issue/edit/edit.go +++ b/pkg/cmd/issue/edit/edit.go @@ -210,6 +210,8 @@ func editRun(opts *EditOptions) error { lookupFields := []string{"id", "number", "title", "body", "url"} if editable.Assignees.Edited { if issueFeatures.ActorIsAssignable { + editable.Assignees.ActorAssignees = true + // At the time of writing, only 10 Actors can be assigned to an issue. assignedActors := heredoc.Doc(` assignedActors(first: 10) { @@ -277,7 +279,7 @@ func editRun(opts *EditOptions) error { editable.Body.Default = issue.Body // We use Actors as the default assignees if Actors are assignable // on this GitHub host. - if issueFeatures.ActorIsAssignable { + if editable.Assignees.ActorAssignees { editable.Assignees.Default = issue.AssignedActors.Logins() } else { editable.Assignees.Default = issue.Assignees.Logins() diff --git a/pkg/cmd/issue/edit/edit_test.go b/pkg/cmd/issue/edit/edit_test.go index d0115ab4e..01cd74900 100644 --- a/pkg/cmd/issue/edit/edit_test.go +++ b/pkg/cmd/issue/edit/edit_test.go @@ -118,9 +118,11 @@ func TestNewCmdEdit(t *testing.T) { output: EditOptions{ IssueNumbers: []int{23}, Editable: prShared.Editable{ - Assignees: prShared.EditableSlice{ - Add: []string{"monalisa", "hubot"}, - Edited: true, + Assignees: prShared.EditableAssignees{ + EditableSlice: prShared.EditableSlice{ + Add: []string{"monalisa", "hubot"}, + Edited: true, + }, }, }, }, @@ -132,9 +134,11 @@ func TestNewCmdEdit(t *testing.T) { output: EditOptions{ IssueNumbers: []int{23}, Editable: prShared.Editable{ - Assignees: prShared.EditableSlice{ - Remove: []string{"monalisa", "hubot"}, - Edited: true, + Assignees: prShared.EditableAssignees{ + EditableSlice: prShared.EditableSlice{ + Remove: []string{"monalisa", "hubot"}, + Edited: true, + }, }, }, }, @@ -354,10 +358,12 @@ func Test_editRun(t *testing.T) { Value: "new body", Edited: true, }, - Assignees: prShared.EditableSlice{ - Add: []string{"monalisa", "hubot"}, - Remove: []string{"octocat"}, - Edited: true, + Assignees: prShared.EditableAssignees{ + EditableSlice: prShared.EditableSlice{ + Add: []string{"monalisa", "hubot"}, + Remove: []string{"octocat"}, + Edited: true, + }, }, Labels: prShared.EditableSlice{ Add: []string{"feature", "TODO", "bug"}, @@ -399,10 +405,12 @@ func Test_editRun(t *testing.T) { IssueNumbers: []int{456, 123}, Interactive: false, Editable: prShared.Editable{ - Assignees: prShared.EditableSlice{ - Add: []string{"monalisa", "hubot"}, - Remove: []string{"octocat"}, - Edited: true, + Assignees: prShared.EditableAssignees{ + EditableSlice: prShared.EditableSlice{ + Add: []string{"monalisa", "hubot"}, + Remove: []string{"octocat"}, + Edited: true, + }, }, Labels: prShared.EditableSlice{ Add: []string{"feature", "TODO", "bug"}, @@ -449,10 +457,12 @@ func Test_editRun(t *testing.T) { IssueNumbers: []int{123, 9999}, Interactive: false, Editable: prShared.Editable{ - Assignees: prShared.EditableSlice{ - Add: []string{"monalisa", "hubot"}, - Remove: []string{"octocat"}, - Edited: true, + Assignees: prShared.EditableAssignees{ + EditableSlice: prShared.EditableSlice{ + Add: []string{"monalisa", "hubot"}, + Remove: []string{"octocat"}, + Edited: true, + }, }, Labels: prShared.EditableSlice{ Add: []string{"feature", "TODO", "bug"}, @@ -494,10 +504,12 @@ func Test_editRun(t *testing.T) { IssueNumbers: []int{123, 456}, Interactive: false, Editable: prShared.Editable{ - Assignees: prShared.EditableSlice{ - Add: []string{"monalisa", "hubot"}, - Remove: []string{"octocat"}, - Edited: true, + Assignees: prShared.EditableAssignees{ + EditableSlice: prShared.EditableSlice{ + Add: []string{"monalisa", "hubot"}, + Remove: []string{"octocat"}, + Edited: true, + }, }, Milestone: prShared.EditableString{ Value: "GA", @@ -509,14 +521,14 @@ func Test_editRun(t *testing.T) { httpStubs: func(t *testing.T, reg *httpmock.Registry) { // Should only be one fetch of metadata. reg.Register( - httpmock.GraphQL(`query RepositoryAssignableUsers\b`), + httpmock.GraphQL(`query RepositoryAssignableActors\b`), httpmock.StringResponse(` - { "data": { "repository": { "assignableUsers": { + { "data": { "repository": { "suggestedActors": { "nodes": [ { "login": "hubot", "id": "HUBOTID" }, { "login": "MonaLisa", "id": "MONAID" } ], - "pageInfo": { "hasNextPage": false } + "pageInfo": { "hasNextPage": false, "endCursor": "Mg" } } } } } `)) reg.Register( @@ -669,17 +681,30 @@ func mockIssueProjectItemsGet(_ *testing.T, reg *httpmock.Registry) { } func mockRepoMetadata(_ *testing.T, reg *httpmock.Registry) { + // reg.Register( + // httpmock.GraphQL(`query RepositoryAssignableUsers\b`), + // httpmock.StringResponse(` + // { "data": { "repository": { "assignableUsers": { + // "nodes": [ + // { "login": "hubot", "id": "HUBOTID" }, + // { "login": "MonaLisa", "id": "MONAID" } + // ], + // "pageInfo": { "hasNextPage": false } + // } } } } + // `)) + reg.Register( - httpmock.GraphQL(`query RepositoryAssignableUsers\b`), + httpmock.GraphQL(`query RepositoryAssignableActors\b`), httpmock.StringResponse(` - { "data": { "repository": { "assignableUsers": { + { "data": { "repository": { "suggestedActors": { "nodes": [ { "login": "hubot", "id": "HUBOTID" }, { "login": "MonaLisa", "id": "MONAID" } ], - "pageInfo": { "hasNextPage": false } + "pageInfo": { "hasNextPage": false, "endCursor": "Mg" } } } } } `)) + reg.Register( httpmock.GraphQL(`query RepositoryLabelList\b`), httpmock.StringResponse(` @@ -902,9 +927,11 @@ func TestActorIsAssignable(t *testing.T) { Detector: &fd.EnabledDetectorMock{}, IssueNumbers: []int{123}, Editable: prShared.Editable{ - Assignees: prShared.EditableSlice{ - Add: []string{"monalisa", "octocat"}, - Edited: true, + Assignees: prShared.EditableAssignees{ + EditableSlice: prShared.EditableSlice{ + Add: []string{"monalisa", "octocat"}, + Edited: true, + }, }, }, }) @@ -940,9 +967,11 @@ func TestActorIsAssignable(t *testing.T) { Detector: &fd.DisabledDetectorMock{}, IssueNumbers: []int{123}, Editable: prShared.Editable{ - Assignees: prShared.EditableSlice{ - Add: []string{"monalisa", "octocat"}, - Edited: true, + Assignees: prShared.EditableAssignees{ + EditableSlice: prShared.EditableSlice{ + Add: []string{"monalisa", "octocat"}, + Edited: true, + }, }, }, }) diff --git a/pkg/cmd/pr/edit/edit_test.go b/pkg/cmd/pr/edit/edit_test.go index 3c4882961..f45984c76 100644 --- a/pkg/cmd/pr/edit/edit_test.go +++ b/pkg/cmd/pr/edit/edit_test.go @@ -165,9 +165,11 @@ func TestNewCmdEdit(t *testing.T) { output: EditOptions{ SelectorArg: "23", Editable: shared.Editable{ - Assignees: shared.EditableSlice{ - Add: []string{"monalisa", "hubot"}, - Edited: true, + Assignees: shared.EditableAssignees{ + EditableSlice: shared.EditableSlice{ + Add: []string{"monalisa", "hubot"}, + Edited: true, + }, }, }, }, @@ -179,9 +181,11 @@ func TestNewCmdEdit(t *testing.T) { output: EditOptions{ SelectorArg: "23", Editable: shared.Editable{ - Assignees: shared.EditableSlice{ - Remove: []string{"monalisa", "hubot"}, - Edited: true, + Assignees: shared.EditableAssignees{ + EditableSlice: shared.EditableSlice{ + Remove: []string{"monalisa", "hubot"}, + Edited: true, + }, }, }, }, @@ -359,10 +363,12 @@ func Test_editRun(t *testing.T) { Remove: []string{"dependabot"}, Edited: true, }, - Assignees: shared.EditableSlice{ - Add: []string{"monalisa", "hubot"}, - Remove: []string{"octocat"}, - Edited: true, + Assignees: shared.EditableAssignees{ + EditableSlice: shared.EditableSlice{ + Add: []string{"monalisa", "hubot"}, + Remove: []string{"octocat"}, + Edited: true, + }, }, Labels: shared.EditableSlice{ Add: []string{"feature", "TODO", "bug"}, @@ -413,10 +419,12 @@ func Test_editRun(t *testing.T) { Value: "base-branch-name", Edited: true, }, - Assignees: shared.EditableSlice{ - Add: []string{"monalisa", "hubot"}, - Remove: []string{"octocat"}, - Edited: true, + Assignees: shared.EditableAssignees{ + EditableSlice: shared.EditableSlice{ + Add: []string{"monalisa", "hubot"}, + Remove: []string{"octocat"}, + Edited: true, + }, }, Labels: shared.EditableSlice{ Add: []string{"feature", "TODO", "bug"}, diff --git a/pkg/cmd/pr/shared/editable.go b/pkg/cmd/pr/shared/editable.go index e73b3c294..238939efe 100644 --- a/pkg/cmd/pr/shared/editable.go +++ b/pkg/cmd/pr/shared/editable.go @@ -14,7 +14,7 @@ type Editable struct { Body EditableString Base EditableString Reviewers EditableSlice - Assignees EditableSlice + Assignees EditableAssignees Labels EditableSlice Projects EditableProjects Milestone EditableString @@ -38,6 +38,13 @@ type EditableSlice struct { Allowed bool } +// EditableAssignees is a special case of EditableSlice. +// It contains a flag to indicate whether the assignees are actors or not. +type EditableAssignees struct { + EditableSlice + ActorAssignees bool +} + // ProjectsV2 mutations require a mapping of an item ID to a project ID. // Keep that map along with standard EditableSlice data. type EditableProjects struct { @@ -245,6 +252,13 @@ func (es *EditableSlice) clone() EditableSlice { return cpy } +func (ea *EditableAssignees) clone() EditableAssignees { + return EditableAssignees{ + EditableSlice: ea.EditableSlice.clone(), + ActorAssignees: ea.ActorAssignees, + } +} + func (ep *EditableProjects) clone() EditableProjects { return EditableProjects{ EditableSlice: ep.EditableSlice.clone(), @@ -378,12 +392,13 @@ func FieldsToEditSurvey(p EditPrompter, editable *Editable) error { func FetchOptions(client *api.Client, repo ghrepo.Interface, editable *Editable) error { input := api.RepoMetadataInput{ - Reviewers: editable.Reviewers.Edited, - Assignees: editable.Assignees.Edited, - Labels: editable.Labels.Edited, - ProjectsV1: editable.Projects.Edited, - ProjectsV2: editable.Projects.Edited, - Milestones: editable.Milestone.Edited, + Reviewers: editable.Reviewers.Edited, + Assignees: editable.Assignees.Edited, + ActorAssignees: editable.Assignees.ActorAssignees, + Labels: editable.Labels.Edited, + ProjectsV1: editable.Projects.Edited, + ProjectsV2: editable.Projects.Edited, + Milestones: editable.Milestone.Edited, } metadata, err := api.RepoMetadata(client, repo, input) if err != nil { @@ -394,6 +409,10 @@ func FetchOptions(client *api.Client, repo ghrepo.Interface, editable *Editable) for _, u := range metadata.AssignableUsers { users = append(users, u.Login) } + var actors []string + for _, a := range metadata.AssignableActors { + actors = append(actors, a.Login) + } var teams []string for _, t := range metadata.Teams { teams = append(teams, fmt.Sprintf("%s/%s", repo.RepoOwner(), t.Slug)) @@ -416,7 +435,11 @@ func FetchOptions(client *api.Client, repo ghrepo.Interface, editable *Editable) editable.Metadata = *metadata editable.Reviewers.Options = append(users, teams...) - editable.Assignees.Options = users + if editable.Assignees.ActorAssignees { + editable.Assignees.Options = actors + } else { + editable.Assignees.Options = users + } editable.Labels.Options = labels editable.Projects.Options = projects editable.Milestone.Options = milestones