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

Merge pull request #10990 from cli/kw/gh-cli-916-special-actor-assignee-names

Browse source 
`issue edit`, `pr edit`: handle display names in interactive assignee editing
This commit is contained in:
Kynan Ware 2025-05-16 10:16:08 -06:00 • committed by GitHub
commit 475163a126
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 257 additions and 17 deletions
Download patch file Download diff file Expand all files Collapse all files

53
api/queries_issue.go
View file

@ -93,21 +93,60 @@ func (a Assignees) Logins() []string {
}
type AssignedActors struct {
Edges []struct {
Node Actor
}
Nodes []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
logins := make([]string, len(a.Nodes))
for i, a := range a.Nodes {
logins[i] = a.Login
}
return logins
}
// DisplayNames returns a list of display names for the assigned actors.
func (a AssignedActors) DisplayNames() []string {
// These display names are used for populating the "default" assigned actors
// from the AssignedActors type. But, this is only one piece of the puzzle
// as later, other queries will fetch the full list of possible assignable
// actors from the repository, and the two lists will be reconciled.
//
// It's important that the display names are the same between the defaults
// (the values returned here) and the full list (the values returned by
// other repository queries). Any discrepancy would result in an
// "invalid default", which means an assigned actor will not be matched
// to an assignable actor and not presented as a "default" selection.
// Not being presented as a default would cause the actor to be potentially
// unassigned if the edits were submitted.
//
// To prevent this, we need shared logic to look up an actor's display name.
// However, our API types between assignedActors and the full list of
// assignableActors are different. So, as an attempt to maintain
// consistency we convert the assignedActors to the same types as the
// repository's assignableActors, treating the assignableActors DisplayName
// methods as the sources of truth.
// TODO KW: make this comment less of a wall of text if needed.
var displayNames []string
for _, a := range a.Nodes {
if a.TypeName == "User" {
u := NewAssignableUser(
a.ID,
a.Login,
a.Name,
)
displayNames = append(displayNames, u.DisplayName())
} else if a.TypeName == "Bot" {
b := NewAssignableBot(
a.ID,
a.Login,
)
displayNames = append(displayNames, b.DisplayName())
}
}
return displayNames
}
type Labels struct {
Nodes []IssueLabel
TotalCount int

28
api/queries_repo.go
View file

@ -146,11 +146,16 @@ 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.
// Actor is a superset of User and Bot, among others.
// At the time of writing, some of these fields
// are not directly supported by the Actor type and
// instead are only available on the User or Bot types
// directly.
type Actor struct {
ID string `json:"id"`
Login string `json:"login"`
ID string `json:"id"`
Login string `json:"login"`
Name string `json:"name"`
TypeName string `json:"__typename"`
}
// BranchRef is the branch name in a GitHub repository
@ -710,6 +715,11 @@ func (m *RepoMetadataResult) MembersToIDs(names []string) ([]string, error) {
found = true
break
}
if strings.EqualFold(assigneeLogin, a.DisplayName()) {
ids = append(ids, a.ID())
found = true
break
}
}
if !found {
@ -1223,7 +1233,17 @@ type AssignableBot struct {
login string
}
func NewAssignableBot(id, login string) AssignableBot {
return AssignableBot{
id: id,
login: login,
}
}
func (b AssignableBot) DisplayName() string {
if b.login == "copilot-swe-agent" {
return "Copilot (AI)"
}
return b.Login()
}

21
api/query_builder.go
View file

@ -20,6 +20,25 @@ func shortenQuery(q string) string {
return strings.Map(squeeze, q)
}
var assignedActors = shortenQuery(`
assignedActors(first: 10) {
nodes {
...on User {
id,
login,
name,
__typename
}
...on Bot {
id,
login,
__typename
}
},
totalCount
}
`)
var issueComments = shortenQuery(`
comments(first: 100) {
nodes {
@ -367,7 +386,7 @@ func IssueGraphQL(fields []string) string {
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}`)
q = append(q, assignedActors)
case "labels":
q = append(q, `labels(first:100){nodes{id,name,description,color},totalCount}`)
case "projectCards":

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

@ -267,7 +267,8 @@ 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.AssignedActors.DisplayNames()
editable.Assignees.DefaultLogins = issue.AssignedActors.Logins()
} else {
editable.Assignees.Default = issue.Assignees.Logins()
}

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

@ -620,6 +620,123 @@ func Test_editRun(t *testing.T) {
},
stdout: "https://github.com/OWNER/REPO/issue/123\n",
},
{
name: "interactive prompts with actor assignee display names when actors available",
input: &EditOptions{
IssueNumbers: []int{123},
Interactive: true,
FieldsToEditSurvey: func(p prShared.EditPrompter, eo *prShared.Editable) error {
eo.Assignees.Edited = true
return nil
},
EditFieldsSurvey: func(p prShared.EditPrompter, eo *prShared.Editable, _ string) error {
// Checking that the display name is being used in the prompt.
require.Equal(t, eo.Assignees.Default, []string{"hubot", "MonaLisa (Mona Display Name)"})
// Mocking a selection of only MonaLisa in the prompt.
eo.Assignees.Value = []string{"MonaLisa (Mona Display Name)"}
return nil
},
FetchOptions: prShared.FetchOptions,
DetermineEditor: func() (string, error) { return "vim", nil },
},
httpStubs: func(t *testing.T, reg *httpmock.Registry) {
mockIsssueNumberGetWithAssignedActors(t, reg, 123)
reg.Register(
httpmock.GraphQL(`query RepositoryAssignableActors\b`),
httpmock.StringResponse(`
{ "data": { "repository": { "suggestedActors": {
"nodes": [
{ "login": "hubot", "id": "HUBOTID", "__typename": "Bot" },
{ "login": "MonaLisa", "id": "MONAID", "name": "Mona Display Name", "__typename": "User" }
],
"pageInfo": { "hasNextPage": false }
} } } }
`))
mockIssueUpdate(t, reg)
reg.Register(
httpmock.GraphQL(`mutation ReplaceActorsForAssignable\b`),
httpmock.GraphQLMutation(`
{ "data": { "replaceActorsForAssignable": { "__typename": "" } } }`,
func(inputs map[string]interface{}) {
// Checking that despite the display name being returned
// from the EditFieldsSurvey, the ID is still
// used in the mutation.
require.Contains(t, inputs["actorIds"], "MONAID")
}),
)
},
stdout: "https://github.com/OWNER/REPO/issue/123\n",
},
{
name: "interactive prompts with user assignee logins when actors unavailable",
input: &EditOptions{
IssueNumbers: []int{123},
Interactive: true,
FieldsToEditSurvey: func(p prShared.EditPrompter, eo *prShared.Editable) error {
eo.Assignees.Edited = true
return nil
},
EditFieldsSurvey: func(p prShared.EditPrompter, eo *prShared.Editable, _ string) error {
// Checking that only the login is used in the prompt (no display name)
require.Equal(t, eo.Assignees.Default, []string{"hubot", "MonaLisa"})
// Mocking a selection of only MonaLisa in the prompt.
eo.Assignees.Value = []string{"MonaLisa"}
return nil
},
FetchOptions: prShared.FetchOptions,
DetermineEditor: func() (string, error) { return "vim", nil },
Detector: &fd.DisabledDetectorMock{},
},
httpStubs: func(t *testing.T, reg *httpmock.Registry) {
reg.Register(
httpmock.GraphQL(`query IssueByNumber\b`),
httpmock.StringResponse(fmt.Sprintf(`
{ "data": { "repository": { "hasIssuesEnabled": true, "issue": {
"id": "%[1]d",
"number": %[1]d,
"url": "https://github.com/OWNER/REPO/issue/123",
"assignees": {
"nodes": [
{
"id": "HUBOTID",
"login": "hubot",
"name": ""
},
{
"id": "MONAID",
"login": "MonaLisa",
"name": "Mona Display Name"
}
],
"totalCount": 2
}
} } } }`, 123)),
)
reg.Register(
httpmock.GraphQL(`query RepositoryAssignableUsers\b`),
httpmock.StringResponse(`
{ "data": { "repository": { "assignableUsers": {
"nodes": [
{ "login": "hubot", "id": "HUBOTID", "name": "" },
{ "login": "MonaLisa", "id": "MONAID", "name": "Mona Display Name" }
],
"pageInfo": { "hasNextPage": false }
} } } }
`))
reg.Register(
httpmock.GraphQL(`mutation IssueUpdate\b`),
httpmock.GraphQLMutation(`
{ "data": { "updateIssue": { "__typename": "" } } }`,
func(inputs map[string]interface{}) {
// Checking that we still assigned the expected ID.
require.Contains(t, inputs["assigneeIds"], "MONAID")
}),
)
},
stdout: "https://github.com/OWNER/REPO/issue/123\n",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
@ -678,6 +795,34 @@ func mockIssueNumberGet(_ *testing.T, reg *httpmock.Registry, number int) {
)
}
func mockIsssueNumberGetWithAssignedActors(_ *testing.T, reg *httpmock.Registry, number int) {
reg.Register(
httpmock.GraphQL(`query IssueByNumber\b`),
httpmock.StringResponse(fmt.Sprintf(`
{ "data": { "repository": { "hasIssuesEnabled": true, "issue": {
"id": "%[1]d",
"number": %[1]d,
"url": "https://github.com/OWNER/REPO/issue/%[1]d",
"assignedActors": {
"nodes": [
{
"id": "HUBOTID",
"login": "hubot",
"__typename": "Bot"
},
{
"id": "MONAID",
"login": "MonaLisa",
"name": "Mona Display Name",
"__typename": "User"
}
],
"totalCount": 2
}
} } } }`, number)),
)
}
func mockIssueProjectItemsGet(_ *testing.T, reg *httpmock.Registry) {
reg.Register(
httpmock.GraphQL(`query IssueProjectItems\b`),
@ -699,7 +844,7 @@ func mockRepoMetadata(_ *testing.T, reg *httpmock.Registry) {
{ "data": { "repository": { "suggestedActors": {
"nodes": [
{ "login": "hubot", "id": "HUBOTID", "__typename": "Bot" },
{ "login": "MonaLisa", "id": "MONAID", "__typename": "User" }
{ "login": "MonaLisa", "id": "MONAID", "name": "Mona Display Name", "__typename": "User" }
],
"pageInfo": { "hasNextPage": false }
} } } }

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

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

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

@ -43,6 +43,7 @@ type EditableSlice struct {
type EditableAssignees struct {
EditableSlice
ActorAssignees bool
DefaultLogins []string // For disambiguating actors from display names
}
// ProjectsV2 mutations require a mapping of an item ID to a project ID.
@ -112,10 +113,25 @@ func (e Editable) AssigneeIds(client *api.Client, repo ghrepo.Interface) (*[]str
if !e.Assignees.Edited {
return nil, nil
}
// If assignees came in from command line flags, we need to
// curate the final list of assignees from the default list.
if len(e.Assignees.Add) != 0 || len(e.Assignees.Remove) != 0 {
meReplacer := NewMeReplacer(client, repo.RepoHost())
s := set.NewStringSet()
s.AddValues(e.Assignees.Default)
// This check below is required because in a non-interactive flow,
// the user gives us a login and not the DisplayName, and when
// we have actor assignees e.Assignees.Default will contain
// DisplayNames and not logins (this is to accommodate special actor
// display names in the interactive flow).
// So, we need to add the default logins here instead of the DisplayNames.
// Otherwise, the value the user provided won't be found in the
// set to be added or removed, causing unexpected behavior.
if e.Assignees.ActorAssignees {
s.AddValues(e.Assignees.DefaultLogins)
} else {
s.AddValues(e.Assignees.Default)
}
add, err := meReplacer.ReplaceSlice(e.Assignees.Add)
if err != nil {
return nil, err
@ -411,7 +427,7 @@ func FetchOptions(client *api.Client, repo ghrepo.Interface, editable *Editable)
}
var actors []string
for _, a := range metadata.AssignableActors {
actors = append(actors, a.Login())
actors = append(actors, a.DisplayName())
}
var teams []string
for _, t := range metadata.Teams {