From 8ebbd1d4bf0ad67c420d41bbe04fdf4ccb46fa15 Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Wed, 7 May 2025 12:19:16 -0600 Subject: [PATCH 01/21] feat(fd): add ActorIsAssignable to IssueFeatures --- internal/featuredetection/feature_detection.go | 9 ++++++--- internal/featuredetection/feature_detection_test.go | 9 ++++++--- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/internal/featuredetection/feature_detection.go b/internal/featuredetection/feature_detection.go index fba317f58..5ff08a573 100644 --- a/internal/featuredetection/feature_detection.go +++ b/internal/featuredetection/feature_detection.go @@ -18,11 +18,13 @@ type Detector interface { } type IssueFeatures struct { - StateReason bool + StateReason bool + ActorIsAssignable bool } var allIssueFeatures = IssueFeatures{ - StateReason: true, + StateReason: true, + ActorIsAssignable: true, } type PullRequestFeatures struct { @@ -70,7 +72,8 @@ func (d *detector) IssueFeatures() (IssueFeatures, error) { } features := IssueFeatures{ - StateReason: false, + StateReason: false, + ActorIsAssignable: false, } var featureDetection struct { diff --git a/internal/featuredetection/feature_detection_test.go b/internal/featuredetection/feature_detection_test.go index f1152da2c..2c7d19071 100644 --- a/internal/featuredetection/feature_detection_test.go +++ b/internal/featuredetection/feature_detection_test.go @@ -23,7 +23,8 @@ func TestIssueFeatures(t *testing.T) { name: "github.com", hostname: "github.com", wantFeatures: IssueFeatures{ - StateReason: true, + StateReason: true, + ActorIsAssignable: true, }, wantErr: false, }, @@ -31,7 +32,8 @@ func TestIssueFeatures(t *testing.T) { name: "ghec data residency (ghe.com)", hostname: "stampname.ghe.com", wantFeatures: IssueFeatures{ - StateReason: true, + StateReason: true, + ActorIsAssignable: true, }, wantErr: false, }, @@ -42,7 +44,8 @@ func TestIssueFeatures(t *testing.T) { `query Issue_fields\b`: `{"data": {}}`, }, wantFeatures: IssueFeatures{ - StateReason: false, + StateReason: false, + ActorIsAssignable: false, }, wantErr: false, }, From 38e52db3779e9a6a199b6799283a452bcbe0802b Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Thu, 8 May 2025 11:06:34 -0600 Subject: [PATCH 02/21] feat(issue edit): fetch currently assigned actors --- api/queries_issue.go | 16 +++++++ api/queries_repo.go | 7 +++ pkg/cmd/issue/edit/edit.go | 41 +++++++++++++++--- pkg/cmd/issue/edit/edit_test.go | 75 +++++++++++++++++++++++++++++++++ 4 files changed, 132 insertions(+), 7 deletions(-) diff --git a/api/queries_issue.go b/api/queries_issue.go index f09360152..2b15a41b9 100644 --- a/api/queries_issue.go +++ b/api/queries_issue.go @@ -38,6 +38,7 @@ type Issue struct { Comments Comments Author Author Assignees Assignees + AssignedActors ActorAssignees Labels Labels ProjectCards ProjectCards ProjectItems ProjectItems @@ -91,6 +92,21 @@ func (a Assignees) Logins() []string { return logins } +type ActorAssignees struct { + Edges []struct { + Node Actor + } + TotalCount int +} + +func (a ActorAssignees) Logins() []string { + logins := make([]string, len(a.Edges)) + for i, a := range a.Edges { + logins[i] = a.Node.Login + } + return logins +} + type Labels struct { Nodes []IssueLabel TotalCount int diff --git a/api/queries_repo.go b/api/queries_repo.go index 93a32d80c..06ae48b12 100644 --- a/api/queries_repo.go +++ b/api/queries_repo.go @@ -146,6 +146,13 @@ type GitHubUser struct { Name string `json:"name"` } +// Actor is a superset of User and Bot +// At the time of writing, it does not support Name. +type Actor struct { + ID string `json:"id"` + Login string `json:"login"` +} + // BranchRef is the branch name in a GitHub repository type BranchRef struct { Name string `json:"name"` diff --git a/pkg/cmd/issue/edit/edit.go b/pkg/cmd/issue/edit/edit.go index 8386cbcfa..a5694e6c9 100644 --- a/pkg/cmd/issue/edit/edit.go +++ b/pkg/cmd/issue/edit/edit.go @@ -197,9 +197,35 @@ func editRun(opts *EditOptions) error { } } + if opts.Detector == nil { + cachedClient := api.NewCachedHTTPClient(httpClient, time.Hour*24) + opts.Detector = fd.NewDetector(cachedClient, baseRepo.RepoHost()) + } + + issueFeatures, err := opts.Detector.IssueFeatures() + if err != nil { + return err + } + lookupFields := []string{"id", "number", "title", "body", "url"} if editable.Assignees.Edited { - lookupFields = append(lookupFields, "assignees") + if issueFeatures.ActorIsAssignable { + // At the time of writing, only 10 Actors can be assigned to an issue. + assignedActors := heredoc.Doc(` + assignedActors(first: 10) { + edges { + node { + ... on Actor { + login + } + } + } + } + `) + lookupFields = append(lookupFields, assignedActors) + } else { + lookupFields = append(lookupFields, "assignees") + } } if editable.Labels.Edited { lookupFields = append(lookupFields, "labels") @@ -207,11 +233,6 @@ func editRun(opts *EditOptions) error { if editable.Projects.Edited { // TODO projectsV1Deprecation // Remove this section as we should no longer add projectCards - if opts.Detector == nil { - cachedClient := api.NewCachedHTTPClient(httpClient, time.Hour*24) - opts.Detector = fd.NewDetector(cachedClient, baseRepo.RepoHost()) - } - projectsV1Support := opts.Detector.ProjectsV1() if projectsV1Support == gh.ProjectsV1Supported { lookupFields = append(lookupFields, "projectCards") @@ -254,7 +275,13 @@ func editRun(opts *EditOptions) error { editable.Title.Default = issue.Title editable.Body.Default = issue.Body - editable.Assignees.Default = issue.Assignees.Logins() + // We use Actors as the default assignees if Actors are assignable + // on this GitHub host. + if issueFeatures.ActorIsAssignable { + editable.Assignees.Default = issue.AssignedActors.Logins() + } else { + editable.Assignees.Default = issue.Assignees.Logins() + } editable.Labels.Default = issue.Labels.Names() editable.Projects.Default = append(issue.ProjectCards.ProjectNames(), issue.ProjectItems.ProjectTitles()...) projectItems := map[string]string{} diff --git a/pkg/cmd/issue/edit/edit_test.go b/pkg/cmd/issue/edit/edit_test.go index c9aa4c409..d0115ab4e 100644 --- a/pkg/cmd/issue/edit/edit_test.go +++ b/pkg/cmd/issue/edit/edit_test.go @@ -875,3 +875,78 @@ func TestProjectsV1Deprecation(t *testing.T) { reg.Verify(t) }) } + +func TestActorIsAssignable(t *testing.T) { + t.Run("when actors are assignable, query includes assignedActors", func(t *testing.T) { + ios, _, _, _ := iostreams.Test() + + reg := &httpmock.Registry{} + reg.Register( + httpmock.GraphQL(`assignedActors`), + // Simulate a GraphQL error to early exit the test. + httpmock.StatusStringResponse(500, ""), + ) + + _, cmdTeardown := run.Stub() + defer cmdTeardown(t) + + // Ignore the error because we don't care. + _ = editRun(&EditOptions{ + IO: ios, + HttpClient: func() (*http.Client, error) { + return &http.Client{Transport: reg}, nil + }, + BaseRepo: func() (ghrepo.Interface, error) { + return ghrepo.New("OWNER", "REPO"), nil + }, + Detector: &fd.EnabledDetectorMock{}, + IssueNumbers: []int{123}, + Editable: prShared.Editable{ + Assignees: prShared.EditableSlice{ + Add: []string{"monalisa", "octocat"}, + Edited: true, + }, + }, + }) + + reg.Verify(t) + }) + + t.Run("when actors are not assignable, query includes assignees instead", func(t *testing.T) { + ios, _, _, _ := iostreams.Test() + + reg := &httpmock.Registry{} + // This test should NOT include assignedActors in the query + reg.Exclude(t, httpmock.GraphQL(`assignedActors`)) + // It should include the regular assignees field + reg.Register( + httpmock.GraphQL(`assignees`), + // Simulate a GraphQL error to early exit the test. + httpmock.StatusStringResponse(500, ""), + ) + + _, cmdTeardown := run.Stub() + defer cmdTeardown(t) + + // Ignore the error because we're not really interested in it. + _ = editRun(&EditOptions{ + IO: ios, + HttpClient: func() (*http.Client, error) { + return &http.Client{Transport: reg}, nil + }, + BaseRepo: func() (ghrepo.Interface, error) { + return ghrepo.New("OWNER", "REPO"), nil + }, + Detector: &fd.DisabledDetectorMock{}, + IssueNumbers: []int{123}, + Editable: prShared.Editable{ + Assignees: prShared.EditableSlice{ + Add: []string{"monalisa", "octocat"}, + Edited: true, + }, + }, + }) + + reg.Verify(t) + }) +} From ee9d16920425caf41dc9f1f1ca4bd7fc388acbd4 Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Fri, 9 May 2025 13:37:15 -0600 Subject: [PATCH 03/21] feat(issue edit): fetch assignable actors --- api/queries_repo.go | 141 +++++++++++++++++++++++++++----- pkg/cmd/issue/edit/edit.go | 4 +- pkg/cmd/issue/edit/edit_test.go | 97 ++++++++++++++-------- pkg/cmd/pr/edit/edit_test.go | 36 ++++---- pkg/cmd/pr/shared/editable.go | 39 +++++++-- 5 files changed, 240 insertions(+), 77 deletions(-) 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 From 0efdfed0680c5afc8daf32da5c44928139684ca5 Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Fri, 9 May 2025 22:56:09 -0600 Subject: [PATCH 04/21] feat(issue edit): support assigning actors to issues --- pkg/cmd/issue/edit/edit_test.go | 187 +++++++++++++++++------------ pkg/cmd/pr/shared/editable_http.go | 46 +++++++ 2 files changed, 154 insertions(+), 79 deletions(-) diff --git a/pkg/cmd/issue/edit/edit_test.go b/pkg/cmd/issue/edit/edit_test.go index 01cd74900..d27bbdee6 100644 --- a/pkg/cmd/issue/edit/edit_test.go +++ b/pkg/cmd/issue/edit/edit_test.go @@ -394,6 +394,7 @@ func Test_editRun(t *testing.T) { mockIssueProjectItemsGet(t, reg) mockRepoMetadata(t, reg) mockIssueUpdate(t, reg) + mockIssueUpdateActorAssignees(t, reg) mockIssueUpdateLabels(t, reg) mockProjectV2ItemUpdate(t, reg) }, @@ -441,6 +442,8 @@ func Test_editRun(t *testing.T) { mockIssueProjectItemsGet(t, reg) mockIssueUpdate(t, reg) mockIssueUpdate(t, reg) + mockIssueUpdateActorAssignees(t, reg) + mockIssueUpdateActorAssignees(t, reg) mockIssueUpdateLabels(t, reg) mockIssueUpdateLabels(t, reg) mockProjectV2ItemUpdate(t, reg) @@ -546,6 +549,14 @@ func Test_editRun(t *testing.T) { mockIssueNumberGet(t, reg, 123) mockIssueNumberGet(t, reg, 456) // Updating 123 should succeed. + reg.Register( + httpmock.GraphQLMutationMatcher(`mutation ReplaceActorsForAssignable\b`, func(m map[string]interface{}) bool { + return m["assignableId"] == "123" + }), + httpmock.GraphQLMutation(` + { "data": { "replaceActorsForAssignable": { "__typename": "" } } }`, + func(inputs map[string]interface{}) {}), + ) reg.Register( httpmock.GraphQLMutationMatcher(`mutation IssueUpdate\b`, func(m map[string]interface{}) bool { return m["id"] == "123" @@ -555,6 +566,14 @@ func Test_editRun(t *testing.T) { func(inputs map[string]interface{}) {}), ) // Updating 456 should fail. + reg.Register( + httpmock.GraphQLMutationMatcher(`mutation ReplaceActorsForAssignable\b`, func(m map[string]interface{}) bool { + return m["assignableId"] == "456" + }), + httpmock.GraphQLMutation(` + { "errors": [ { "message": "test error" } ] }`, + func(inputs map[string]interface{}) {}), + ) reg.Register( httpmock.GraphQLMutationMatcher(`mutation IssueUpdate\b`, func(m map[string]interface{}) bool { return m["id"] == "456" @@ -603,6 +622,7 @@ func Test_editRun(t *testing.T) { mockIssueProjectItemsGet(t, reg) mockRepoMetadata(t, reg) mockIssueUpdate(t, reg) + mockIssueUpdateActorAssignees(t, reg) mockIssueUpdateLabels(t, reg) mockProjectV2ItemUpdate(t, reg) }, @@ -792,6 +812,15 @@ func mockIssueUpdate(t *testing.T, reg *httpmock.Registry) { ) } +func mockIssueUpdateActorAssignees(t *testing.T, reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL(`mutation ReplaceActorsForAssignable\b`), + httpmock.GraphQLMutation(` + { "data": { "replaceActorsForAssignable": { "__typename": "" } } }`, + func(inputs map[string]interface{}) {}), + ) +} + func mockIssueUpdateLabels(t *testing.T, reg *httpmock.Registry) { reg.Register( httpmock.GraphQL(`mutation LabelAdd\b`), @@ -816,6 +845,85 @@ func mockProjectV2ItemUpdate(t *testing.T, reg *httpmock.Registry) { ) } +func TestActorIsAssignable(t *testing.T) { + t.Run("when actors are assignable, query includes assignedActors", func(t *testing.T) { + ios, _, _, _ := iostreams.Test() + + reg := &httpmock.Registry{} + reg.Register( + httpmock.GraphQL(`assignedActors`), + // Simulate a GraphQL error to early exit the test. + httpmock.StatusStringResponse(500, ""), + ) + + _, cmdTeardown := run.Stub() + defer cmdTeardown(t) + + // Ignore the error because we don't care. + _ = editRun(&EditOptions{ + IO: ios, + HttpClient: func() (*http.Client, error) { + return &http.Client{Transport: reg}, nil + }, + BaseRepo: func() (ghrepo.Interface, error) { + return ghrepo.New("OWNER", "REPO"), nil + }, + Detector: &fd.EnabledDetectorMock{}, + IssueNumbers: []int{123}, + Editable: prShared.Editable{ + Assignees: prShared.EditableAssignees{ + EditableSlice: prShared.EditableSlice{ + Add: []string{"monalisa", "octocat"}, + Edited: true, + }, + }, + }, + }) + + reg.Verify(t) + }) + + t.Run("when actors are not assignable, query includes assignees instead", func(t *testing.T) { + ios, _, _, _ := iostreams.Test() + + reg := &httpmock.Registry{} + // This test should NOT include assignedActors in the query + reg.Exclude(t, httpmock.GraphQL(`assignedActors`)) + // It should include the regular assignees field + reg.Register( + httpmock.GraphQL(`assignees`), + // Simulate a GraphQL error to early exit the test. + httpmock.StatusStringResponse(500, ""), + ) + + _, cmdTeardown := run.Stub() + defer cmdTeardown(t) + + // Ignore the error because we're not really interested in it. + _ = editRun(&EditOptions{ + IO: ios, + HttpClient: func() (*http.Client, error) { + return &http.Client{Transport: reg}, nil + }, + BaseRepo: func() (ghrepo.Interface, error) { + return ghrepo.New("OWNER", "REPO"), nil + }, + Detector: &fd.DisabledDetectorMock{}, + IssueNumbers: []int{123}, + Editable: prShared.Editable{ + Assignees: prShared.EditableAssignees{ + EditableSlice: prShared.EditableSlice{ + Add: []string{"monalisa", "octocat"}, + Edited: true, + }, + }, + }, + }) + + reg.Verify(t) + }) +} + // TODO projectsV1Deprecation // Remove this test. func TestProjectsV1Deprecation(t *testing.T) { @@ -900,82 +1008,3 @@ func TestProjectsV1Deprecation(t *testing.T) { reg.Verify(t) }) } - -func TestActorIsAssignable(t *testing.T) { - t.Run("when actors are assignable, query includes assignedActors", func(t *testing.T) { - ios, _, _, _ := iostreams.Test() - - reg := &httpmock.Registry{} - reg.Register( - httpmock.GraphQL(`assignedActors`), - // Simulate a GraphQL error to early exit the test. - httpmock.StatusStringResponse(500, ""), - ) - - _, cmdTeardown := run.Stub() - defer cmdTeardown(t) - - // Ignore the error because we don't care. - _ = editRun(&EditOptions{ - IO: ios, - HttpClient: func() (*http.Client, error) { - return &http.Client{Transport: reg}, nil - }, - BaseRepo: func() (ghrepo.Interface, error) { - return ghrepo.New("OWNER", "REPO"), nil - }, - Detector: &fd.EnabledDetectorMock{}, - IssueNumbers: []int{123}, - Editable: prShared.Editable{ - Assignees: prShared.EditableAssignees{ - EditableSlice: prShared.EditableSlice{ - Add: []string{"monalisa", "octocat"}, - Edited: true, - }, - }, - }, - }) - - reg.Verify(t) - }) - - t.Run("when actors are not assignable, query includes assignees instead", func(t *testing.T) { - ios, _, _, _ := iostreams.Test() - - reg := &httpmock.Registry{} - // This test should NOT include assignedActors in the query - reg.Exclude(t, httpmock.GraphQL(`assignedActors`)) - // It should include the regular assignees field - reg.Register( - httpmock.GraphQL(`assignees`), - // Simulate a GraphQL error to early exit the test. - httpmock.StatusStringResponse(500, ""), - ) - - _, cmdTeardown := run.Stub() - defer cmdTeardown(t) - - // Ignore the error because we're not really interested in it. - _ = editRun(&EditOptions{ - IO: ios, - HttpClient: func() (*http.Client, error) { - return &http.Client{Transport: reg}, nil - }, - BaseRepo: func() (ghrepo.Interface, error) { - return ghrepo.New("OWNER", "REPO"), nil - }, - Detector: &fd.DisabledDetectorMock{}, - IssueNumbers: []int{123}, - Editable: prShared.Editable{ - Assignees: prShared.EditableAssignees{ - EditableSlice: prShared.EditableSlice{ - Add: []string{"monalisa", "octocat"}, - Edited: true, - }, - }, - }, - }) - - reg.Verify(t) - }) -} diff --git a/pkg/cmd/pr/shared/editable_http.go b/pkg/cmd/pr/shared/editable_http.go index fcc30095a..2c09f2e69 100644 --- a/pkg/cmd/pr/shared/editable_http.go +++ b/pkg/cmd/pr/shared/editable_http.go @@ -58,6 +58,26 @@ func UpdateIssue(httpClient *http.Client, repo ghrepo.Interface, id string, isPR }) } + // updateIssue mutation does not support Actors so assignment needs to + // be in a separate request when our assignees are Actors. + if options.Assignees.Edited && options.Assignees.ActorAssignees { + apiClient := api.NewClientFromHTTP(httpClient) + assigneeIds, err := options.AssigneeIds(apiClient, repo) + if err != nil { + return err + } + + // Disable the edited flag for assignees so that it doesn't + // trigger a second update in the replaceIssueFields function. + // It's important that this is done AFTER the call to + // options.AssigneeIds() above because otherwise that just exits. + options.Assignees.Edited = false + + wg.Go(func() error { + return replaceActorAssigneesForEditable(apiClient, repo, id, assigneeIds) + }) + } + if dirtyExcludingLabels(options) { wg.Go(func() error { return replaceIssueFields(httpClient, repo, id, isPR, options) @@ -67,6 +87,32 @@ func UpdateIssue(httpClient *http.Client, repo ghrepo.Interface, id string, isPR return wg.Wait() } +func replaceActorAssigneesForEditable(apiClient *api.Client, repo ghrepo.Interface, id string, assigneeIds *[]string) error { + type ReplaceActorsForAssignableInput struct { + AssignableID githubv4.ID `json:"assignableId"` + ActorIDs []githubv4.ID `json:"actorIds"` + } + + params := ReplaceActorsForAssignableInput{ + AssignableID: githubv4.ID(id), + ActorIDs: *ghIds(assigneeIds), + } + + var mutation struct { + ReplaceActorsForAssignable struct { + Typename string `graphql:"__typename"` + } `graphql:"replaceActorsForAssignable(input: $input)"` + } + + variables := map[string]interface{}{"input": params} + err := apiClient.Mutate(repo.RepoHost(), "ReplaceActorsForAssignable", &mutation, variables) + if err != nil { + return err + } + + return nil +} + func replaceIssueFields(httpClient *http.Client, repo ghrepo.Interface, id string, isPR bool, options Editable) error { apiClient := api.NewClientFromHTTP(httpClient) assigneeIds, err := options.AssigneeIds(apiClient, repo) From e0afc91fef1c3fbfe81d34c3054dbbd147e4b231 Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Fri, 9 May 2025 23:02:15 -0600 Subject: [PATCH 05/21] chore(issue edit): comments cleanup --- api/queries_issue.go | 1 + pkg/cmd/issue/edit/edit_test.go | 14 +------------- 2 files changed, 2 insertions(+), 13 deletions(-) diff --git a/api/queries_issue.go b/api/queries_issue.go index 2b15a41b9..833faaa47 100644 --- a/api/queries_issue.go +++ b/api/queries_issue.go @@ -99,6 +99,7 @@ type ActorAssignees struct { TotalCount int } +// TODO kw: Display names for actors with special display names. func (a ActorAssignees) Logins() []string { logins := make([]string, len(a.Edges)) for i, a := range a.Edges { diff --git a/pkg/cmd/issue/edit/edit_test.go b/pkg/cmd/issue/edit/edit_test.go index d27bbdee6..5a06614bc 100644 --- a/pkg/cmd/issue/edit/edit_test.go +++ b/pkg/cmd/issue/edit/edit_test.go @@ -701,18 +701,6 @@ 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 RepositoryAssignableActors\b`), httpmock.StringResponse(` @@ -721,7 +709,7 @@ func mockRepoMetadata(_ *testing.T, reg *httpmock.Registry) { { "login": "hubot", "id": "HUBOTID" }, { "login": "MonaLisa", "id": "MONAID" } ], - "pageInfo": { "hasNextPage": false, "endCursor": "Mg" } + "pageInfo": { "hasNextPage": false } } } } } `)) From 218152f7c5d454bffd019eb63e58645cd1f54639 Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Fri, 9 May 2025 23:26:06 -0600 Subject: [PATCH 06/21] fix(issue edit): resolve race condition in actor assignment --- pkg/cmd/pr/shared/editable_http.go | 30 ++++++++++++++---------------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/pkg/cmd/pr/shared/editable_http.go b/pkg/cmd/pr/shared/editable_http.go index 2c09f2e69..1475f978c 100644 --- a/pkg/cmd/pr/shared/editable_http.go +++ b/pkg/cmd/pr/shared/editable_http.go @@ -61,19 +61,13 @@ func UpdateIssue(httpClient *http.Client, repo ghrepo.Interface, id string, isPR // updateIssue mutation does not support Actors so assignment needs to // be in a separate request when our assignees are Actors. if options.Assignees.Edited && options.Assignees.ActorAssignees { - apiClient := api.NewClientFromHTTP(httpClient) - assigneeIds, err := options.AssigneeIds(apiClient, repo) - if err != nil { - return err - } - - // Disable the edited flag for assignees so that it doesn't - // trigger a second update in the replaceIssueFields function. - // It's important that this is done AFTER the call to - // options.AssigneeIds() above because otherwise that just exits. - options.Assignees.Edited = false - wg.Go(func() error { + apiClient := api.NewClientFromHTTP(httpClient) + assigneeIds, err := options.AssigneeIds(apiClient, repo) + if err != nil { + return err + } + return replaceActorAssigneesForEditable(apiClient, repo, id, assigneeIds) }) } @@ -115,16 +109,20 @@ func replaceActorAssigneesForEditable(apiClient *api.Client, repo ghrepo.Interfa func replaceIssueFields(httpClient *http.Client, repo ghrepo.Interface, id string, isPR bool, options Editable) error { apiClient := api.NewClientFromHTTP(httpClient) - assigneeIds, err := options.AssigneeIds(apiClient, repo) - if err != nil { - return err - } projectIds, err := options.ProjectIds() if err != nil { return err } + var assigneeIds *[]string + if !options.Assignees.ActorAssignees { + assigneeIds, err = options.AssigneeIds(apiClient, repo) + if err != nil { + return err + } + } + milestoneId, err := options.MilestoneId() if err != nil { return err From 29241cb7a51a8d672bfe00c706cb9ea09f84b6f4 Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Mon, 12 May 2025 11:34:31 -0600 Subject: [PATCH 07/21] refactor(issue edit): improve actor type handling This improves actor type handling while fetching repository assignable actors. --- api/queries_repo.go | 37 +++++++++++++++++++++++---------- pkg/cmd/issue/edit/edit_test.go | 8 +++---- 2 files changed, 30 insertions(+), 15 deletions(-) diff --git a/api/queries_repo.go b/api/queries_repo.go index 99041e49f..a5b926fc0 100644 --- a/api/queries_repo.go +++ b/api/queries_repo.go @@ -1235,17 +1235,25 @@ func RepoAssignableUsers(client *Client, repo ghrepo.Interface) ([]RepoAssignee, // 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 repoBotAssignee struct { + ID string + Login string + TypeName string `graphql:"__typename"` + } + + type repoUserAssignee struct { + ID string + Login string + Name string + TypeName string `graphql:"__typename"` } type responseData struct { Repository struct { SuggestedActors struct { Nodes []struct { - User RepoAssignee `graphql:"... on User"` - Bot repoActorAssignee `graphql:"... on Bot"` + User repoUserAssignee `graphql:"... on User"` + Bot repoBotAssignee `graphql:"... on Bot"` } PageInfo struct { HasNextPage bool @@ -1270,13 +1278,20 @@ func RepoAssignableActors(client *Client, repo ghrepo.Interface) ([]RepoAssignee } 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 + if node.User.TypeName == "User" { + actor := RepoAssignee{ + ID: node.User.ID, + Login: node.User.Login, + Name: node.User.Name, + } + actors = append(actors, actor) + } else if node.Bot.TypeName == "Bot" { + actor := RepoAssignee{ + ID: node.Bot.ID, + Login: node.Bot.Login, + } + actors = append(actors, actor) } - actors = append(actors, node.User) } if !query.Repository.SuggestedActors.PageInfo.HasNextPage { diff --git a/pkg/cmd/issue/edit/edit_test.go b/pkg/cmd/issue/edit/edit_test.go index 5a06614bc..bf2a1c417 100644 --- a/pkg/cmd/issue/edit/edit_test.go +++ b/pkg/cmd/issue/edit/edit_test.go @@ -528,8 +528,8 @@ func Test_editRun(t *testing.T) { httpmock.StringResponse(` { "data": { "repository": { "suggestedActors": { "nodes": [ - { "login": "hubot", "id": "HUBOTID" }, - { "login": "MonaLisa", "id": "MONAID" } + { "login": "hubot", "id": "HUBOTID", "__typename": "Bot" }, + { "login": "MonaLisa", "id": "MONAID", "__typename": "User" } ], "pageInfo": { "hasNextPage": false, "endCursor": "Mg" } } } } } @@ -706,8 +706,8 @@ func mockRepoMetadata(_ *testing.T, reg *httpmock.Registry) { httpmock.StringResponse(` { "data": { "repository": { "suggestedActors": { "nodes": [ - { "login": "hubot", "id": "HUBOTID" }, - { "login": "MonaLisa", "id": "MONAID" } + { "login": "hubot", "id": "HUBOTID", "__typename": "Bot" }, + { "login": "MonaLisa", "id": "MONAID", "__typename": "User" } ], "pageInfo": { "hasNextPage": false } } } } } From 73e5589059eebefedbc9116bae7adaad1c8f8768 Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Mon, 12 May 2025 11:37:37 -0600 Subject: [PATCH 08/21] doc(issue): comment why assignable actors disabled --- internal/featuredetection/feature_detection.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/featuredetection/feature_detection.go b/internal/featuredetection/feature_detection.go index 5ff08a573..a2f34a60b 100644 --- a/internal/featuredetection/feature_detection.go +++ b/internal/featuredetection/feature_detection.go @@ -73,7 +73,7 @@ func (d *detector) IssueFeatures() (IssueFeatures, error) { features := IssueFeatures{ StateReason: false, - ActorIsAssignable: false, + ActorIsAssignable: false, // replaceActorsForAssignable GraphQL mutation unavailable on GHES } var featureDetection struct { From cff2fa71e2b044befe0ea865b174cd08e9ab91d6 Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Mon, 12 May 2025 11:49:44 -0600 Subject: [PATCH 09/21] doc(repo queries): clarify reviewer actor fetching logic --- api/queries_repo.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/api/queries_repo.go b/api/queries_repo.go index a5b926fc0..6b9f690eb 100644 --- a/api/queries_repo.go +++ b/api/queries_repo.go @@ -929,7 +929,7 @@ func RepoMetadata(client *Client, repo ghrepo.Interface, input RepoMetadataInput return err }) - // If reviewers are requested, we still need to fetch the assignable users + // 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 @@ -938,6 +938,9 @@ func RepoMetadata(client *Client, repo ghrepo.Interface, input RepoMetadataInput // 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) From 35792827ad3e0bd142bf91d2dc38fac9195fa526 Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Tue, 13 May 2025 07:09:04 -0600 Subject: [PATCH 10/21] refactor(issue edit): rename AssignedActors to ActorAssignees --- api/queries_issue.go | 2 +- pkg/cmd/issue/edit/edit.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/api/queries_issue.go b/api/queries_issue.go index 833faaa47..69e93c974 100644 --- a/api/queries_issue.go +++ b/api/queries_issue.go @@ -38,7 +38,7 @@ type Issue struct { Comments Comments Author Author Assignees Assignees - AssignedActors ActorAssignees + ActorAssignees ActorAssignees Labels Labels ProjectCards ProjectCards ProjectItems ProjectItems diff --git a/pkg/cmd/issue/edit/edit.go b/pkg/cmd/issue/edit/edit.go index 9177f60ad..b86f20550 100644 --- a/pkg/cmd/issue/edit/edit.go +++ b/pkg/cmd/issue/edit/edit.go @@ -280,7 +280,7 @@ func editRun(opts *EditOptions) error { // We use Actors as the default assignees if Actors are assignable // on this GitHub host. if editable.Assignees.ActorAssignees { - editable.Assignees.Default = issue.AssignedActors.Logins() + editable.Assignees.Default = issue.ActorAssignees.Logins() } else { editable.Assignees.Default = issue.Assignees.Logins() } From 3bed77836a5ff2f95dab3d01f5c819d19c729bd3 Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Tue, 13 May 2025 07:11:24 -0600 Subject: [PATCH 11/21] refactor(issue edit): rename loop variable for clarity --- api/queries_repo.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/api/queries_repo.go b/api/queries_repo.go index 6b9f690eb..ea60319a3 100644 --- a/api/queries_repo.go +++ b/api/queries_repo.go @@ -704,9 +704,9 @@ func (m *RepoMetadataResult) MembersToIDs(names []string) ([]string, error) { } // 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) + for _, a := range m.AssignableActors { + if strings.EqualFold(assigneeLogin, a.Login) { + ids = append(ids, a.ID) found = true break } From 261297f0a2ccb0322c4dce8bb2fc99ae3e5c0c49 Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Tue, 13 May 2025 07:28:42 -0600 Subject: [PATCH 12/21] refactor(issue edit): add assignedActors to lookupFields --- api/query_builder.go | 2 ++ pkg/cmd/issue/edit/edit.go | 15 +-------------- 2 files changed, 3 insertions(+), 14 deletions(-) diff --git a/api/query_builder.go b/api/query_builder.go index 47fb4c225..0ef44a347 100644 --- a/api/query_builder.go +++ b/api/query_builder.go @@ -366,6 +366,8 @@ func IssueGraphQL(fields []string) string { q = append(q, `headRepository{id,name}`) case "assignees": q = append(q, `assignees(first:100){nodes{id,login,name},totalCount}`) + case "assignedActors": + q = append(q, `assignedActors(first: 10){edges{node{...on Actor{login}}},totalCount}`) case "labels": q = append(q, `labels(first:100){nodes{id,name,description,color},totalCount}`) case "projectCards": diff --git a/pkg/cmd/issue/edit/edit.go b/pkg/cmd/issue/edit/edit.go index b86f20550..a2e824ebf 100644 --- a/pkg/cmd/issue/edit/edit.go +++ b/pkg/cmd/issue/edit/edit.go @@ -211,20 +211,7 @@ func editRun(opts *EditOptions) error { 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) { - edges { - node { - ... on Actor { - login - } - } - } - } - `) - lookupFields = append(lookupFields, assignedActors) + lookupFields = append(lookupFields, `assignedActors`) } else { lookupFields = append(lookupFields, "assignees") } From 21bd797c6c31859d65edc37cdb6b0fa82bbb1f68 Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Tue, 13 May 2025 11:45:40 -0600 Subject: [PATCH 13/21] fix(issue edit): use double quotes for assignedActors --- pkg/cmd/issue/edit/edit.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/issue/edit/edit.go b/pkg/cmd/issue/edit/edit.go index a2e824ebf..c9c76746b 100644 --- a/pkg/cmd/issue/edit/edit.go +++ b/pkg/cmd/issue/edit/edit.go @@ -211,7 +211,7 @@ func editRun(opts *EditOptions) error { if editable.Assignees.Edited { if issueFeatures.ActorIsAssignable { editable.Assignees.ActorAssignees = true - lookupFields = append(lookupFields, `assignedActors`) + lookupFields = append(lookupFields, "assignedActors") } else { lookupFields = append(lookupFields, "assignees") } From 1e5c3c7426144987f7dece2913c799ad7ca9a454 Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Tue, 13 May 2025 13:02:20 -0600 Subject: [PATCH 14/21] fix(issue edit): revert rename of ActorAssignees --- api/queries_issue.go | 6 +++--- pkg/cmd/issue/edit/edit.go | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/api/queries_issue.go b/api/queries_issue.go index 69e93c974..701b92039 100644 --- a/api/queries_issue.go +++ b/api/queries_issue.go @@ -38,7 +38,7 @@ type Issue struct { Comments Comments Author Author Assignees Assignees - ActorAssignees ActorAssignees + AssignedActors AssignedActors Labels Labels ProjectCards ProjectCards ProjectItems ProjectItems @@ -92,7 +92,7 @@ func (a Assignees) Logins() []string { return logins } -type ActorAssignees struct { +type AssignedActors struct { Edges []struct { Node Actor } @@ -100,7 +100,7 @@ type ActorAssignees struct { } // TODO kw: Display names for actors with special display names. -func (a ActorAssignees) Logins() []string { +func (a AssignedActors) Logins() []string { logins := make([]string, len(a.Edges)) for i, a := range a.Edges { logins[i] = a.Node.Login diff --git a/pkg/cmd/issue/edit/edit.go b/pkg/cmd/issue/edit/edit.go index c9c76746b..5cb789543 100644 --- a/pkg/cmd/issue/edit/edit.go +++ b/pkg/cmd/issue/edit/edit.go @@ -267,7 +267,7 @@ func editRun(opts *EditOptions) error { // We use Actors as the default assignees if Actors are assignable // on this GitHub host. if editable.Assignees.ActorAssignees { - editable.Assignees.Default = issue.ActorAssignees.Logins() + editable.Assignees.Default = issue.AssignedActors.Logins() } else { editable.Assignees.Default = issue.Assignees.Logins() } From dfd600713f001e3c1385ee9ea21597d78eaca535 Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Wed, 14 May 2025 08:53:47 -0600 Subject: [PATCH 15/21] refactor(api): rename assignable user types and methods --- api/queries_repo.go | 148 +++++++++++++++++++++++-------- api/queries_repo_test.go | 6 +- pkg/cmd/pr/shared/completion.go | 8 +- pkg/cmd/pr/shared/editable.go | 4 +- pkg/cmd/pr/shared/survey.go | 2 +- pkg/cmd/pr/shared/survey_test.go | 6 +- 6 files changed, 123 insertions(+), 51 deletions(-) diff --git a/api/queries_repo.go b/api/queries_repo.go index ea60319a3..9e822767d 100644 --- a/api/queries_repo.go +++ b/api/queries_repo.go @@ -682,8 +682,8 @@ func RepoFindForks(client *Client, repo ghrepo.Interface, limit int) ([]*Reposit type RepoMetadataResult struct { CurrentLogin string - AssignableUsers []RepoAssignee - AssignableActors []RepoAssignee + AssignableUsers []AssignableUser + AssignableActors []AssignableActor Labels []RepoLabel Projects []RepoProject ProjectsV2 []ProjectV2 @@ -696,8 +696,8 @@ func (m *RepoMetadataResult) MembersToIDs(names []string) ([]string, error) { for _, assigneeLogin := range names { found := false for _, u := range m.AssignableUsers { - if strings.EqualFold(assigneeLogin, u.Login) { - ids = append(ids, u.ID) + if strings.EqualFold(assigneeLogin, (u.Login())) { + ids = append(ids, u.ID()) found = true break } @@ -705,8 +705,8 @@ func (m *RepoMetadataResult) MembersToIDs(names []string) ([]string, error) { // Look for ID in assignable actors if not found in assignable users for _, a := range m.AssignableActors { - if strings.EqualFold(assigneeLogin, a.Login) { - ids = append(ids, a.ID) + if strings.EqualFold(assigneeLogin, a.Login()) { + ids = append(ids, a.ID()) found = true break } @@ -1126,12 +1126,16 @@ func RepoResolveMetadataIDs(client *Client, repo ghrepo.Interface, input RepoRes result.Teams = append(result.Teams, t) } default: - user := RepoAssignee{} + user := struct { + Id string + Login string + Name string + }{} err := json.Unmarshal(v, &user) if err != nil { return result, err } - result.AssignableUsers = append(result.AssignableUsers, user) + result.AssignableUsers = append(result.AssignableUsers, NewAssignableUser(user.Id, user.Login, user.Name)) } } @@ -1183,26 +1187,86 @@ func RepoProjects(client *Client, repo ghrepo.Interface) ([]RepoProject, error) return projects, nil } -type RepoAssignee struct { - ID string - Login string - Name string +type AssignableActor interface { + DisplayName() string + ID() string + Login() string + + sealedAssignableActor() +} + +// Always a user +type AssignableUser struct { + id string + login string + name string +} + +// NewAssignableUser is a test helper to create a new AssignableUser +// since the ID and Login are private fields +func NewAssignableUser(id, login, name string) AssignableUser { + return AssignableUser{ + id: id, + login: login, + name: name, + } } // DisplayName returns a formatted string that uses Login and Name to be displayed e.g. 'Login (Name)' or 'Login' -func (ra RepoAssignee) DisplayName() string { - if ra.Name != "" { - return fmt.Sprintf("%s (%s)", ra.Login, ra.Name) +func (u AssignableUser) DisplayName() string { + if u.name != "" { + return fmt.Sprintf("%s (%s)", u.login, u.name) } - return ra.Login + return u.login } +func (u AssignableUser) ID() string { + return u.id +} + +func (u AssignableUser) Login() string { + return u.login +} + +func (u AssignableUser) Name() string { + return u.name +} + +func (u AssignableUser) sealedAssignableActor() {} + +type AssignableBot struct { + id string + login string +} + +func (b AssignableBot) DisplayName() string { + return b.Login() +} + +func (b AssignableBot) ID() string { + return b.id +} + +func (b AssignableBot) Login() string { + return b.login +} + +func (b AssignableBot) Name() string { + return "" +} + +func (b AssignableBot) sealedAssignableActor() {} + // RepoAssignableUsers fetches all the assignable users for a repository -func RepoAssignableUsers(client *Client, repo ghrepo.Interface) ([]RepoAssignee, error) { +func RepoAssignableUsers(client *Client, repo ghrepo.Interface) ([]AssignableUser, error) { type responseData struct { Repository struct { AssignableUsers struct { - Nodes []RepoAssignee + Nodes []struct { + ID string + Login string + Name string + } PageInfo struct { HasNextPage bool EndCursor string @@ -1217,7 +1281,7 @@ func RepoAssignableUsers(client *Client, repo ghrepo.Interface) ([]RepoAssignee, "endCursor": (*githubv4.String)(nil), } - var users []RepoAssignee + var users []AssignableUser for { var query responseData err := client.Query(repo.RepoHost(), "RepositoryAssignableUsers", &query, variables) @@ -1225,7 +1289,15 @@ func RepoAssignableUsers(client *Client, repo ghrepo.Interface) ([]RepoAssignee, return nil, err } - users = append(users, query.Repository.AssignableUsers.Nodes...) + for _, node := range query.Repository.AssignableUsers.Nodes { + user := AssignableUser{ + id: node.ID, + login: node.Login, + name: node.Name, + } + + users = append(users, user) + } if !query.Repository.AssignableUsers.PageInfo.HasNextPage { break } @@ -1237,26 +1309,26 @@ func RepoAssignableUsers(client *Client, repo ghrepo.Interface) ([]RepoAssignee, // 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 repoBotAssignee struct { - ID string - Login string - TypeName string `graphql:"__typename"` - } - - type repoUserAssignee struct { +func RepoAssignableActors(client *Client, repo ghrepo.Interface) ([]AssignableActor, error) { + type assignableUser struct { ID string Login string Name string TypeName string `graphql:"__typename"` } + type assignableBot struct { + ID string + Login string + TypeName string `graphql:"__typename"` + } + type responseData struct { Repository struct { SuggestedActors struct { Nodes []struct { - User repoUserAssignee `graphql:"... on User"` - Bot repoBotAssignee `graphql:"... on Bot"` + User assignableUser `graphql:"... on User"` + Bot assignableBot `graphql:"... on Bot"` } PageInfo struct { HasNextPage bool @@ -1272,7 +1344,7 @@ func RepoAssignableActors(client *Client, repo ghrepo.Interface) ([]RepoAssignee "endCursor": (*githubv4.String)(nil), } - var actors []RepoAssignee + var actors []AssignableActor for { var query responseData err := client.Query(repo.RepoHost(), "RepositoryAssignableActors", &query, variables) @@ -1282,16 +1354,16 @@ func RepoAssignableActors(client *Client, repo ghrepo.Interface) ([]RepoAssignee for _, node := range query.Repository.SuggestedActors.Nodes { if node.User.TypeName == "User" { - actor := RepoAssignee{ - ID: node.User.ID, - Login: node.User.Login, - Name: node.User.Name, + actor := AssignableUser{ + id: node.User.ID, + login: node.User.Login, + name: node.User.Name, } actors = append(actors, actor) } else if node.Bot.TypeName == "Bot" { - actor := RepoAssignee{ - ID: node.Bot.ID, - Login: node.Bot.Login, + actor := AssignableBot{ + id: node.Bot.ID, + login: node.Bot.Login, } actors = append(actors, actor) } diff --git a/api/queries_repo_test.go b/api/queries_repo_test.go index 01fc7a4c7..9040a0018 100644 --- a/api/queries_repo_test.go +++ b/api/queries_repo_test.go @@ -526,17 +526,17 @@ func Test_RepoMilestones(t *testing.T) { func TestDisplayName(t *testing.T) { tests := []struct { name string - assignee RepoAssignee + assignee AssignableUser want string }{ { name: "assignee with name", - assignee: RepoAssignee{"123", "octocat123", "Octavious Cath"}, + assignee: AssignableUser{"123", "octocat123", "Octavious Cath"}, want: "octocat123 (Octavious Cath)", }, { name: "assignee without name", - assignee: RepoAssignee{"123", "octocat123", ""}, + assignee: AssignableUser{"123", "octocat123", ""}, want: "octocat123", }, } diff --git a/pkg/cmd/pr/shared/completion.go b/pkg/cmd/pr/shared/completion.go index e07abc5a7..c1296be71 100644 --- a/pkg/cmd/pr/shared/completion.go +++ b/pkg/cmd/pr/shared/completion.go @@ -21,13 +21,13 @@ func RequestableReviewersForCompletion(httpClient *http.Client, repo ghrepo.Inte results := []string{} for _, user := range metadata.AssignableUsers { - if strings.EqualFold(user.Login, metadata.CurrentLogin) { + if strings.EqualFold(user.Login(), metadata.CurrentLogin) { continue } - if user.Name != "" { - results = append(results, fmt.Sprintf("%s\t%s", user.Login, user.Name)) + if user.Name() != "" { + results = append(results, fmt.Sprintf("%s\t%s", user.Login(), user.Name())) } else { - results = append(results, user.Login) + results = append(results, user.Login()) } } for _, team := range metadata.Teams { diff --git a/pkg/cmd/pr/shared/editable.go b/pkg/cmd/pr/shared/editable.go index 238939efe..d51405acd 100644 --- a/pkg/cmd/pr/shared/editable.go +++ b/pkg/cmd/pr/shared/editable.go @@ -407,11 +407,11 @@ func FetchOptions(client *api.Client, repo ghrepo.Interface, editable *Editable) var users []string for _, u := range metadata.AssignableUsers { - users = append(users, u.Login) + users = append(users, u.Login()) } var actors []string for _, a := range metadata.AssignableActors { - actors = append(actors, a.Login) + actors = append(actors, a.Login()) } var teams []string for _, t := range metadata.Teams { diff --git a/pkg/cmd/pr/shared/survey.go b/pkg/cmd/pr/shared/survey.go index bf4476ca1..b6c927a2d 100644 --- a/pkg/cmd/pr/shared/survey.go +++ b/pkg/cmd/pr/shared/survey.go @@ -192,7 +192,7 @@ func MetadataSurvey(p Prompt, io *iostreams.IOStreams, baseRepo ghrepo.Interface var reviewers []string for _, u := range metadataResult.AssignableUsers { - if u.Login != metadataResult.CurrentLogin { + if u.Login() != metadataResult.CurrentLogin { reviewers = append(reviewers, u.DisplayName()) } } diff --git a/pkg/cmd/pr/shared/survey_test.go b/pkg/cmd/pr/shared/survey_test.go index 6895b52ac..7097d0761 100644 --- a/pkg/cmd/pr/shared/survey_test.go +++ b/pkg/cmd/pr/shared/survey_test.go @@ -28,9 +28,9 @@ func TestMetadataSurvey_selectAll(t *testing.T) { fetcher := &metadataFetcher{ metadataResult: &api.RepoMetadataResult{ - AssignableUsers: []api.RepoAssignee{ - {Login: "hubot"}, - {Login: "monalisa"}, + AssignableUsers: []api.AssignableUser{ + api.NewAssignableUser("", "hubot", ""), + api.NewAssignableUser("", "monalisa", ""), }, Labels: []api.RepoLabel{ {Name: "help wanted"}, From 712eeabd4acbef9af03a297c38eec4e476cc993f Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Wed, 14 May 2025 08:55:55 -0600 Subject: [PATCH 16/21] doc(api): remove needless comment --- api/queries_repo.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/api/queries_repo.go b/api/queries_repo.go index 9e822767d..e6286bb0b 100644 --- a/api/queries_repo.go +++ b/api/queries_repo.go @@ -1202,8 +1202,6 @@ type AssignableUser struct { name string } -// NewAssignableUser is a test helper to create a new AssignableUser -// since the ID and Login are private fields func NewAssignableUser(id, login, name string) AssignableUser { return AssignableUser{ id: id, From 08cd1dc7db690477fcfc32aec3f8c171fc53bb80 Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Wed, 14 May 2025 11:43:29 -0600 Subject: [PATCH 17/21] feat(issue edit): replacing actor assignee is done synchronously with updateIssue --- pkg/cmd/issue/edit/edit_test.go | 8 -------- pkg/cmd/pr/shared/editable_http.go | 29 +++++++++++++++++------------ 2 files changed, 17 insertions(+), 20 deletions(-) diff --git a/pkg/cmd/issue/edit/edit_test.go b/pkg/cmd/issue/edit/edit_test.go index bf2a1c417..0df6d5e9f 100644 --- a/pkg/cmd/issue/edit/edit_test.go +++ b/pkg/cmd/issue/edit/edit_test.go @@ -574,14 +574,6 @@ func Test_editRun(t *testing.T) { { "errors": [ { "message": "test error" } ] }`, func(inputs map[string]interface{}) {}), ) - reg.Register( - httpmock.GraphQLMutationMatcher(`mutation IssueUpdate\b`, func(m map[string]interface{}) bool { - return m["id"] == "456" - }), - httpmock.GraphQLMutation(` - { "errors": [ { "message": "test error" } ] }`, - func(inputs map[string]interface{}) {}), - ) }, stdout: heredoc.Doc(` https://github.com/OWNER/REPO/issue/123 diff --git a/pkg/cmd/pr/shared/editable_http.go b/pkg/cmd/pr/shared/editable_http.go index 1475f978c..465bfda6c 100644 --- a/pkg/cmd/pr/shared/editable_http.go +++ b/pkg/cmd/pr/shared/editable_http.go @@ -58,23 +58,28 @@ func UpdateIssue(httpClient *http.Client, repo ghrepo.Interface, id string, isPR }) } - // updateIssue mutation does not support Actors so assignment needs to - // be in a separate request when our assignees are Actors. - if options.Assignees.Edited && options.Assignees.ActorAssignees { + if dirtyExcludingLabels(options) { wg.Go(func() error { - apiClient := api.NewClientFromHTTP(httpClient) - assigneeIds, err := options.AssigneeIds(apiClient, repo) + // updateIssue mutation does not support Actors so assignment needs to + // be in a separate request when our assignees are Actors. + if options.Assignees.Edited && options.Assignees.ActorAssignees { + apiClient := api.NewClientFromHTTP(httpClient) + assigneeIds, err := options.AssigneeIds(apiClient, repo) + if err != nil { + return err + } + + err = replaceActorAssigneesForEditable(apiClient, repo, id, assigneeIds) + if err != nil { + return err + } + } + err := replaceIssueFields(httpClient, repo, id, isPR, options) if err != nil { return err } - return replaceActorAssigneesForEditable(apiClient, repo, id, assigneeIds) - }) - } - - if dirtyExcludingLabels(options) { - wg.Go(func() error { - return replaceIssueFields(httpClient, repo, id, isPR, options) + return nil }) } From 5dc854c75e1735fb906640963dcaea462000f4c9 Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Wed, 14 May 2025 11:50:11 -0600 Subject: [PATCH 18/21] doc(issue edit): clarify synchronous handling of assignees --- pkg/cmd/pr/shared/editable_http.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pkg/cmd/pr/shared/editable_http.go b/pkg/cmd/pr/shared/editable_http.go index 465bfda6c..cd183e565 100644 --- a/pkg/cmd/pr/shared/editable_http.go +++ b/pkg/cmd/pr/shared/editable_http.go @@ -62,6 +62,10 @@ func UpdateIssue(httpClient *http.Client, repo ghrepo.Interface, id string, isPR wg.Go(func() error { // updateIssue mutation does not support Actors so assignment needs to // be in a separate request when our assignees are Actors. + // Note: this is intentionally done synchronously with updating + // other issue fields to ensure consistency with how legacy + // user assignees are handled. + // https://github.com/cli/cli/pull/10960#discussion_r2086725348 if options.Assignees.Edited && options.Assignees.ActorAssignees { apiClient := api.NewClientFromHTTP(httpClient) assigneeIds, err := options.AssigneeIds(apiClient, repo) From da40e087460c0d01f741fba8200b1c5d825b2bee Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Thu, 15 May 2025 08:33:30 -0600 Subject: [PATCH 19/21] doc(api): code comment typo Co-authored-by: Andy Feller --- api/queries_repo.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/queries_repo.go b/api/queries_repo.go index e6286bb0b..aa180a015 100644 --- a/api/queries_repo.go +++ b/api/queries_repo.go @@ -940,7 +940,7 @@ func RepoMetadata(client *Client, repo ghrepo.Interface, input RepoMetadataInput // 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. + // `gh pr` requests actor assignees. if input.Reviewers { g.Go(func() error { users, err := RepoAssignableUsers(client, repo) From ea85b92b35dccaae5a4b359a33c74d5a83c21707 Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Thu, 15 May 2025 09:09:36 -0600 Subject: [PATCH 20/21] refactor(api): remove needless parenthesis Co-authored-by: Andy Feller --- api/queries_repo.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/queries_repo.go b/api/queries_repo.go index aa180a015..978e844c4 100644 --- a/api/queries_repo.go +++ b/api/queries_repo.go @@ -696,7 +696,7 @@ func (m *RepoMetadataResult) MembersToIDs(names []string) ([]string, error) { for _, assigneeLogin := range names { found := false for _, u := range m.AssignableUsers { - if strings.EqualFold(assigneeLogin, (u.Login())) { + if strings.EqualFold(assigneeLogin, u.Login()) { ids = append(ids, u.ID()) found = true break From bcd47f1fb178c60ad7e87f75d3cfbe00f00545d1 Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Thu, 15 May 2025 09:13:46 -0600 Subject: [PATCH 21/21] fix(api): correct var name capitalization --- pkg/cmd/pr/shared/editable_http.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/pr/shared/editable_http.go b/pkg/cmd/pr/shared/editable_http.go index cd183e565..8cd51c349 100644 --- a/pkg/cmd/pr/shared/editable_http.go +++ b/pkg/cmd/pr/shared/editable_http.go @@ -103,7 +103,7 @@ func replaceActorAssigneesForEditable(apiClient *api.Client, repo ghrepo.Interfa var mutation struct { ReplaceActorsForAssignable struct { - Typename string `graphql:"__typename"` + TypeName string `graphql:"__typename"` } `graphql:"replaceActorsForAssignable(input: $input)"` }