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

Merge pull request #10960 from cli/kw/gh-cli-901-910-assign-actors-to-issues

Browse source 
`gh issue edit`: actors are assignable to issues
This commit is contained in:
Kynan Ware 2025-05-15 09:24:32 -06:00 • committed by GitHub
commit 999ffbdfe7
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
14 changed files with 554 additions and 120 deletions
Download patch file Download diff file Expand all files Collapse all files

17
api/queries_issue.go
View file

@ -38,6 +38,7 @@ type Issue struct {
Comments Comments
Author Author
Assignees Assignees
AssignedActors AssignedActors
Labels Labels
ProjectCards ProjectCards
ProjectItems ProjectItems
@ -91,6 +92,22 @@ func (a Assignees) Logins() []string {
return logins
}
type AssignedActors struct {
Edges []struct {
Node Actor
}
TotalCount int
}
// TODO kw: Display names for actors with special display names.
func (a AssignedActors) 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

268
api/queries_repo.go
View file

@ -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"`
@ -674,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 []AssignableUser
AssignableActors []AssignableActor
Labels []RepoLabel
Projects []RepoProject
ProjectsV2 []ProjectV2
Milestones []RepoMilestone
Teams []OrgTeam
}
func (m *RepoMetadataResult) MembersToIDs(names []string) ([]string, error) {
@ -688,12 +696,22 @@ 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
}
}
// 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())
found = true
break
}
}
if !found {
return nil, fmt.Errorf("'%s' not found", assigneeLogin)
}
@ -885,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
@ -899,14 +918,51 @@ 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 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)
}
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 {
@ -1070,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))
}
}
@ -1127,26 +1187,84 @@ 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
}
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
@ -1161,7 +1279,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)
@ -1169,7 +1287,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
}
@ -1179,6 +1305,76 @@ 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) ([]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 assignableUser `graphql:"... on User"`
Bot assignableBot `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 []AssignableActor
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 {
if node.User.TypeName == "User" {
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 := AssignableBot{
id: node.Bot.ID,
login: node.Bot.Login,
}
actors = append(actors, actor)
}
}
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

6
api/queries_repo_test.go
View file

@ -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",
},
}

2
api/query_builder.go
View file

@ -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":

9
internal/featuredetection/feature_detection.go
View file

@ -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, // replaceActorsForAssignable GraphQL mutation unavailable on GHES
}
var featureDetection struct {

9
internal/featuredetection/feature_detection_test.go
View file

@ -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,
},

30
pkg/cmd/issue/edit/edit.go
View file

@ -197,9 +197,24 @@ 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 {
editable.Assignees.ActorAssignees = true
lookupFields = append(lookupFields, "assignedActors")
} else {
lookupFields = append(lookupFields, "assignees")
}
}
if editable.Labels.Edited {
lookupFields = append(lookupFields, "labels")
@ -207,11 +222,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 +264,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 editable.Assignees.ActorAssignees {
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{}

179
pkg/cmd/issue/edit/edit_test.go
View file

@ -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"},
@ -388,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)
},
@ -399,10 +406,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"},
@ -433,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)
@ -449,10 +460,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 +507,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 +524,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" }
{ "login": "hubot", "id": "HUBOTID", "__typename": "Bot" },
{ "login": "MonaLisa", "id": "MONAID", "__typename": "User" }
],
"pageInfo": { "hasNextPage": false }
"pageInfo": { "hasNextPage": false, "endCursor": "Mg" }
} } } }
`))
reg.Register(
@ -534,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"
@ -544,8 +567,8 @@ func Test_editRun(t *testing.T) {
)
// Updating 456 should fail.
reg.Register(
httpmock.GraphQLMutationMatcher(`mutation IssueUpdate\b`, func(m map[string]interface{}) bool {
return m["id"] == "456"
httpmock.GraphQLMutationMatcher(`mutation ReplaceActorsForAssignable\b`, func(m map[string]interface{}) bool {
return m["assignableId"] == "456"
}),
httpmock.GraphQLMutation(`
{ "errors": [ { "message": "test error" } ] }`,
@ -591,6 +614,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)
},
@ -670,16 +694,17 @@ func mockIssueProjectItemsGet(_ *testing.T, reg *httpmock.Registry) {
func mockRepoMetadata(_ *testing.T, reg *httpmock.Registry) {
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" }
{ "login": "hubot", "id": "HUBOTID", "__typename": "Bot" },
{ "login": "MonaLisa", "id": "MONAID", "__typename": "User" }
],
"pageInfo": { "hasNextPage": false }
} } } }
`))
reg.Register(
httpmock.GraphQL(`query RepositoryLabelList\b`),
httpmock.StringResponse(`
@ -767,6 +792,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`),
@ -791,6 +825,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) {

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

@ -166,9 +166,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,
},
},
},
},
@ -180,9 +182,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,
},
},
},
},
@ -360,10 +364,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"},
@ -414,10 +420,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"},

8
pkg/cmd/pr/shared/completion.go
View file

@ -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 {

41
pkg/cmd/pr/shared/editable.go
View file

@ -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 {
@ -392,7 +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())
}
var teams []string
for _, t := range metadata.Teams {
@ -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

61
pkg/cmd/pr/shared/editable_http.go
View file

@ -60,25 +60,78 @@ func UpdateIssue(httpClient *http.Client, repo ghrepo.Interface, id string, isPR
if dirtyExcludingLabels(options) {
wg.Go(func() error {
return replaceIssueFields(httpClient, repo, id, isPR, options)
// 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)
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 nil
})
}
return wg.Wait()
}
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)
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)
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

2
pkg/cmd/pr/shared/survey.go
View file

@ -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())
}
}

6
pkg/cmd/pr/shared/survey_test.go
View file

@ -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"},