STACKITGit/cli
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Merge pull request #10984 from cli/kw/gh-cli-909-911-assign-actors-to-prs

Browse source 
`gh pr edit`: Assign actors to pull requests
This commit is contained in:
Kynan Ware 2025-05-15 14:22:02 -06:00 • committed by GitHub
commit 98b289de8d
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 114 additions and 45 deletions
Download patch file Download diff file Expand all files Collapse all files

1
api/queries_pr.go
View file

@ -84,6 +84,7 @@ type PullRequest struct {
}
Assignees Assignees
AssignedActors AssignedActors
Labels Labels
ProjectCards ProjectCards
ProjectItems ProjectItems

40
api/queries_repo.go
View file

@ -918,51 +918,37 @@ 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 assignable actors: %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` requests 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)
// Filter actors for users to use for pull request reviewers,
// skip retrieving the same info through RepoAssignableUsers().
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 {
users, err := RepoAssignableUsers(client, repo)
if err != nil {
err = fmt.Errorf("error fetching assignees: %w", err)
err = fmt.Errorf("error fetching assignable users: %w", err)
}
result.AssignableUsers = users
return err
})
}
}
if input.Reviewers {

47
pkg/cmd/pr/edit/edit.go
View file

@ -3,6 +3,7 @@ package edit
import (
"fmt"
"net/http"
"time"
"github.com/MakeNowJust/heredoc"
"github.com/cli/cli/v2/api"
@ -20,15 +21,14 @@ import (
type EditOptions struct {
HttpClient func() (*http.Client, error)
IO *iostreams.IOStreams
// TODO projectsV1Deprecation
// Remove this detector since it is only used for test validation.
Detector fd.Detector
Finder shared.PRFinder
Surveyor Surveyor
Fetcher EditableOptionsFetcher
EditorRetriever EditorRetriever
Prompter shared.EditPrompter
Detector fd.Detector
BaseRepo func() (ghrepo.Interface, error)
SelectorArg string
Interactive bool
@ -73,6 +73,7 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Comman
Args: cobra.MaximumNArgs(1),
RunE: func(cmd *cobra.Command, args []string) error {
opts.Finder = shared.NewFinder(f)
opts.BaseRepo = f.BaseRepo
if len(args) > 0 {
opts.SelectorArg = args[0]
@ -196,9 +197,36 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Comman
func editRun(opts *EditOptions) error {
findOptions := shared.FindOptions{
Selector: opts.SelectorArg,
Fields: []string{"id", "url", "title", "body", "baseRefName", "reviewRequests", "assignees", "labels", "projectCards", "projectItems", "milestone"},
Fields: []string{"id", "url", "title", "body", "baseRefName", "reviewRequests", "labels", "projectCards", "projectItems", "milestone"},
Detector: opts.Detector,
}
httpClient, err := opts.HttpClient()
if err != nil {
return err
}
if opts.Detector == nil {
baseRepo, err := opts.BaseRepo()
if err != nil {
return err
}
cachedClient := api.NewCachedHTTPClient(httpClient, time.Hour*24)
opts.Detector = fd.NewDetector(cachedClient, baseRepo.RepoHost())
}
issueFeatures, err := opts.Detector.IssueFeatures()
if err != nil {
return err
}
if issueFeatures.ActorIsAssignable {
findOptions.Fields = append(findOptions.Fields, "assignedActors")
} else {
findOptions.Fields = append(findOptions.Fields, "assignees")
}
pr, repo, err := opts.Finder.Find(findOptions)
if err != nil {
return err
@ -210,7 +238,12 @@ func editRun(opts *EditOptions) error {
editable.Body.Default = pr.Body
editable.Base.Default = pr.BaseRefName
editable.Reviewers.Default = pr.ReviewRequests.Logins()
editable.Assignees.Default = pr.Assignees.Logins()
if issueFeatures.ActorIsAssignable {
editable.Assignees.ActorAssignees = true
editable.Assignees.Default = pr.AssignedActors.Logins()
} else {
editable.Assignees.Default = pr.Assignees.Logins()
}
editable.Labels.Default = pr.Labels.Names()
editable.Projects.Default = append(pr.ProjectCards.ProjectNames(), pr.ProjectItems.ProjectTitles()...)
projectItems := map[string]string{}
@ -229,10 +262,6 @@ func editRun(opts *EditOptions) error {
}
}
httpClient, err := opts.HttpClient()
if err != nil {
return err
}
apiClient := api.NewClientFromHTTP(httpClient)
opts.IO.StartProgressIndicator()

71
pkg/cmd/pr/edit/edit_test.go
View file

@ -393,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)
@ -449,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)
},
@ -469,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)
@ -490,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 users 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) {
@ -508,9 +550,11 @@ func Test_editRun(t *testing.T) {
tt.httpStubs(reg)
httpClient := func() (*http.Client, error) { return &http.Client{Transport: reg}, nil }
baseRepo := func() (ghrepo.Interface, error) { return ghrepo.New("OWNER", "REPO"), nil }
tt.input.IO = ios
tt.input.HttpClient = httpClient
tt.input.BaseRepo = baseRepo
err := editRun(tt.input)
assert.NoError(t, err)
@ -522,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(`
@ -634,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`),