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

review: address code review feedback

Browse source 
- Fix tests: assert logins (not display names) in actorLogins
- Remove dead ReplaceActorsForAssignableByID (no callers)
- Extract shared AssigneeSearchFunc to pkg/cmd/pr/shared/editable.go
- Remove duplicate assigneeSearchFunc from pr/edit and issue/edit

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
Kynan Ware 2026-03-23 17:36:52 -06:00
parent 947f8fb1b7
commit 33783748f3
6 changed files with 47 additions and 129 deletions
Download patch file Download diff file Expand all files Collapse all files

39
api/queries_pr.go
View file

@ -602,6 +602,12 @@ func ReplaceActorsForAssignableByLogin(client *Client, repo ghrepo.Interface, as
actorLogins[i] = githubv4.String(l)
}
var mutation struct {
ReplaceActorsForAssignable struct {
TypeName string `graphql:"__typename"`
} `graphql:"replaceActorsForAssignable(input: $input)"`
}
variables := map[string]interface{}{
"input": ReplaceActorsForAssignableInput{
AssignableID: githubv4.ID(assignableID),
@ -609,39 +615,6 @@ func ReplaceActorsForAssignableByLogin(client *Client, repo ghrepo.Interface, as
},
}
return replaceActorsForAssignable(client, repo, variables)
}
// ReplaceActorsForAssignableByID calls the replaceActorsForAssignable mutation
// using actor node IDs. Used for GHES and edit flows that resolve IDs from search results.
func ReplaceActorsForAssignableByID(client *Client, repo ghrepo.Interface, assignableID string, actorIDs []string) error {
type ReplaceActorsForAssignableInput struct {
AssignableID githubv4.ID `json:"assignableId"`
ActorIDs []githubv4.ID `json:"actorIds"`
}
ids := make([]githubv4.ID, len(actorIDs))
for i, id := range actorIDs {
ids[i] = githubv4.ID(id)
}
variables := map[string]interface{}{
"input": ReplaceActorsForAssignableInput{
AssignableID: githubv4.ID(assignableID),
ActorIDs: ids,
},
}
return replaceActorsForAssignable(client, repo, variables)
}
func replaceActorsForAssignable(client *Client, repo ghrepo.Interface, variables map[string]interface{}) error {
var mutation struct {
ReplaceActorsForAssignable struct {
TypeName string `graphql:"__typename"`
} `graphql:"replaceActorsForAssignable(input: $input)"`
}
return client.Mutate(repo.RepoHost(), "ReplaceActorsForAssignable", &mutation, variables)
}

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

@ -12,7 +12,6 @@ import (
fd "github.com/cli/cli/v2/internal/featuredetection"
"github.com/cli/cli/v2/internal/gh"
"github.com/cli/cli/v2/internal/ghrepo"
"github.com/cli/cli/v2/internal/prompter"
"github.com/cli/cli/v2/internal/text"
shared "github.com/cli/cli/v2/pkg/cmd/issue/shared"
prShared "github.com/cli/cli/v2/pkg/cmd/pr/shared"
@ -253,7 +252,7 @@ func editRun(opts *EditOptions) error {
// Wire up search function for assignees when ActorIsAssignable is available.
// Interactive mode only supports a single issue, so we use its ID for the search query.
if issueFeatures.ActorIsAssignable && opts.Interactive && len(issues) == 1 {
editable.AssigneeSearchFunc = assigneeSearchFunc(apiClient, baseRepo, issues[0].ID)
editable.AssigneeSearchFunc = prShared.AssigneeSearchFunc(apiClient, baseRepo, issues[0].ID)
}
opts.IO.StartProgressIndicatorWithLabel("Fetching repository information")
@ -359,36 +358,3 @@ func editRun(opts *EditOptions) error {
return nil
}
func assigneeSearchFunc(apiClient *api.Client, repo ghrepo.Interface, assignableID string) func(string) prompter.MultiSelectSearchResult {
return func(input string) prompter.MultiSelectSearchResult {
actors, availableAssigneesCount, err := api.SuggestedAssignableActors(
apiClient,
repo,
assignableID,
input)
if err != nil {
return prompter.MultiSelectSearchResult{Err: err}
}
logins := make([]string, 0, len(actors))
displayNames := make([]string, 0, len(actors))
for _, a := range actors {
if a.Login() == "" {
continue
}
logins = append(logins, a.Login())
if a.DisplayName() != "" {
displayNames = append(displayNames, a.DisplayName())
} else {
displayNames = append(displayNames, a.Login())
}
}
return prompter.MultiSelectSearchResult{
Keys: logins,
Labels: displayNames,
MoreResults: availableAssigneesCount,
}
}
}

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

@ -629,8 +629,9 @@ func Test_editRun(t *testing.T) {
require.Equal(t, []string{"hubot"}, eo.Assignees.Default)
require.Equal(t, []string{"hubot"}, eo.Assignees.DefaultLogins)
// Adding MonaLisa as PR assignee, should preserve hubot.
eo.Assignees.Value = []string{"hubot", "MonaLisa (Mona Display Name)"}
// Adding MonaLisa as issue assignee, should preserve hubot.
// MultiSelectWithSearch returns Keys (logins), not display names.
eo.Assignees.Value = []string{"hubot", "MonaLisa"}
return nil
},
FetchOptions: prShared.FetchOptions,
@ -644,10 +645,7 @@ func Test_editRun(t *testing.T) {
httpmock.GraphQLMutation(`
{ "data": { "replaceActorsForAssignable": { "__typename": "" } } }`,
func(inputs map[string]interface{}) {
// Checking that despite the display name being returned
// from the EditFieldsSurvey, the login is still
// used in the mutation.
require.Subset(t, inputs["actorLogins"], []interface{}{"hubot", "MonaLisa (Mona Display Name)"})
require.Subset(t, inputs["actorLogins"], []interface{}{"hubot", "MonaLisa"})
}),
)
},

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

@ -303,7 +303,7 @@ func editRun(opts *EditOptions) error {
// to legacy reviewer/assignee fetching.
// TODO actorIsAssignableCleanup
if issueFeatures.ActorIsAssignable {
editable.AssigneeSearchFunc = assigneeSearchFunc(apiClient, repo, pr.ID)
editable.AssigneeSearchFunc = shared.AssigneeSearchFunc(apiClient, repo, pr.ID)
editable.ReviewerSearchFunc = reviewerSearchFunc(apiClient, repo, &editable, pr.ID)
}
@ -346,50 +346,6 @@ func editRun(opts *EditOptions) error {
return nil
}
// assigneeSearchFunc is intended to be an arg for MultiSelectWithSearch
// to return potential assignee actors.
func assigneeSearchFunc(apiClient *api.Client, repo ghrepo.Interface, assignableID string) func(string) prompter.MultiSelectSearchResult {
searchFunc := func(input string) prompter.MultiSelectSearchResult {
actors, availableAssigneesCount, err := api.SuggestedAssignableActors(
apiClient,
repo,
assignableID,
input)
if err != nil {
return prompter.MultiSelectSearchResult{
Keys: nil,
Labels: nil,
MoreResults: 0,
Err: err,
}
}
logins := make([]string, 0, len(actors))
displayNames := make([]string, 0, len(actors))
for _, a := range actors {
if a.Login() != "" {
logins = append(logins, a.Login())
} else {
continue
}
if a.DisplayName() != "" {
displayNames = append(displayNames, a.DisplayName())
} else {
displayNames = append(displayNames, a.Login())
}
}
return prompter.MultiSelectSearchResult{
Keys: logins,
Labels: displayNames,
MoreResults: availableAssigneesCount,
Err: nil,
}
}
return searchFunc
}
// reviewerSearchFunc is intended to be an arg for MultiSelectWithSearch
// to return potential reviewer candidates (users, bots, and teams).
// It also updates the editable's metadata for later ID resolution.

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

@ -912,12 +912,8 @@ func Test_editRun(t *testing.T) {
require.Equal(t, []string{"hubot"}, e.Assignees.DefaultLogins)
// Adding monalisa as PR assignee, should preserve hubot.
e.Assignees.Value = []string{"hubot", "monalisa (Mona Display Name)"}
// Populate metadata to simulate what searchFunc would do during prompting
e.Metadata.AssignableActors = []api.AssignableActor{
api.NewAssignableBot("HUBOTID", "hubot"),
api.NewAssignableUser("MONAID", "monalisa", "Mona Display Name"),
}
// MultiSelectWithSearch returns Keys (logins), not display names.
e.Assignees.Value = []string{"hubot", "monalisa"}
return nil
},
},
@ -931,10 +927,7 @@ func Test_editRun(t *testing.T) {
httpmock.GraphQLMutation(`
{ "data": { "replaceActorsForAssignable": { "__typename": "" } } }`,
func(inputs map[string]interface{}) {
// Checking that despite the display name being returned
// from the EditFieldsSurvey, the login is still
// used in the mutation.
require.Subset(t, inputs["actorLogins"], []interface{}{"hubot", "monalisa (Mona Display Name)"})
require.Subset(t, inputs["actorLogins"], []interface{}{"hubot", "monalisa"})
}),
)
},

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

@ -611,3 +611,35 @@ func milestoneSurvey(p EditPrompter, title string, opts []string) (result string
result = opts[selected]
return
}
// AssigneeSearchFunc returns a search function for MultiSelectWithSearch that
// dynamically fetches assignable actors for the given assignable (Issue/PR) node ID.
func AssigneeSearchFunc(apiClient *api.Client, repo ghrepo.Interface, assignableID string) func(string) prompter.MultiSelectSearchResult {
return func(input string) prompter.MultiSelectSearchResult {
actors, availableAssigneesCount, err := api.SuggestedAssignableActors(
apiClient, repo, assignableID, input)
if err != nil {
return prompter.MultiSelectSearchResult{Err: err}
}
logins := make([]string, 0, len(actors))
displayNames := make([]string, 0, len(actors))
for _, a := range actors {
if a.Login() == "" {
continue
}
logins = append(logins, a.Login())
if a.DisplayName() != "" {
displayNames = append(displayNames, a.DisplayName())
} else {
displayNames = append(displayNames, a.Login())
}
}
return prompter.MultiSelectSearchResult{
Keys: logins,
Labels: displayNames,
MoreResults: availableAssigneesCount,
}
}
}