Skip to content

Commit 6adf803

Browse files
BagToadBagToad
authored
Merge pull request cli#12526 from cli/github-cli-1070-multi-select-with-search-ccr
`gh pr edit`: new interactive prompt for assignee selection, performance and accessibility improvements
2 parents 009cf6e + 23e80a9 commit 6adf803

11 files changed

Lines changed: 820 additions & 68 deletions

File tree

‎api/queries_pr.go‎

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -701,6 +701,98 @@ func RemovePullRequestReviews(client *Client, repo ghrepo.Interface, prNumber in
701701
return client.REST(repo.RepoHost(), "DELETE", path, buf, nil)
702702
}
703703

704+
// SuggestedAssignableActors fetches up to 10 suggested actors for a specific assignable
705+
// (Issue or PullRequest) node ID. `assignableID` is the GraphQL node ID for the Issue/PR.
706+
// Returns the actors, the total count of available assignees in the repo, and an error.
707+
func SuggestedAssignableActors(client *Client, repo ghrepo.Interface, assignableID string, query string) ([]AssignableActor, int, error) {
708+
type responseData struct {
709+
Repository struct {
710+
AssignableUsers struct {
711+
TotalCount int
712+
}
713+
} `graphql:"repository(owner: $owner, name: $name)"`
714+
Node struct {
715+
Issue struct {
716+
SuggestedActors struct {
717+
Nodes []struct {
718+
TypeName string `graphql:"__typename"`
719+
User struct {
720+
ID string
721+
Login string
722+
Name string
723+
} `graphql:"... on User"`
724+
Bot struct {
725+
ID string
726+
Login string
727+
} `graphql:"... on Bot"`
728+
}
729+
} `graphql:"suggestedActors(first: 10, query: $query)"`
730+
} `graphql:"... on Issue"`
731+
PullRequest struct {
732+
SuggestedActors struct {
733+
Nodes []struct {
734+
TypeName string `graphql:"__typename"`
735+
User struct {
736+
ID string
737+
Login string
738+
Name string
739+
} `graphql:"... on User"`
740+
Bot struct {
741+
ID string
742+
Login string
743+
} `graphql:"... on Bot"`
744+
}
745+
} `graphql:"suggestedActors(first: 10, query: $query)"`
746+
} `graphql:"... on PullRequest"`
747+
} `graphql:"node(id: $id)"`
748+
}
749+
750+
variables := map[string]interface{}{
751+
"id": githubv4.ID(assignableID),
752+
"query": githubv4.String(query),
753+
"owner": githubv4.String(repo.RepoOwner()),
754+
"name": githubv4.String(repo.RepoName()),
755+
}
756+
757+
var result responseData
758+
if err := client.Query(repo.RepoHost(), "SuggestedAssignableActors", &result, variables); err != nil {
759+
return nil, 0, err
760+
}
761+
762+
availableAssigneesCount := result.Repository.AssignableUsers.TotalCount
763+
764+
var nodes []struct {
765+
TypeName string `graphql:"__typename"`
766+
User struct {
767+
ID string
768+
Login string
769+
Name string
770+
} `graphql:"... on User"`
771+
Bot struct {
772+
ID string
773+
Login string
774+
} `graphql:"... on Bot"`
775+
}
776+
777+
if result.Node.PullRequest.SuggestedActors.Nodes != nil {
778+
nodes = result.Node.PullRequest.SuggestedActors.Nodes
779+
} else if result.Node.Issue.SuggestedActors.Nodes != nil {
780+
nodes = result.Node.Issue.SuggestedActors.Nodes
781+
}
782+
783+
actors := make([]AssignableActor, 0, len(nodes))
784+
785+
for _, n := range nodes {
786+
if n.TypeName == "User" && n.User.Login != "" {
787+
actors = append(actors, AssignableUser{id: n.User.ID, login: n.User.Login, name: n.User.Name})
788+
} else if n.TypeName == "Bot" && n.Bot.Login != "" {
789+
actors = append(actors, AssignableBot{id: n.Bot.ID, login: n.Bot.Login})
790+
}
791+
}
792+
793+
return actors, availableAssigneesCount, nil
794+
}
795+
704796
func UpdatePullRequestBranch(client *Client, repo ghrepo.Interface, params githubv4.UpdatePullRequestBranchInput) error {
705797
var mutation struct {
706798
UpdatePullRequestBranch struct {

‎internal/prompter/accessible_prompter_test.go‎

Lines changed: 214 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,217 @@ func TestAccessiblePrompter(t *testing.T) {
224224
assert.Equal(t, []int{1}, multiSelectValues)
225225
})
226226

227+
t.Run("MultiSelectWithSearch - basic flow", func(t *testing.T) {
228+
console := newTestVirtualTerminal(t)
229+
p := newTestAccessiblePrompter(t, console)
230+
persistentOptions := []string{"persistent-option-1"}
231+
searchFunc := func(input string) prompter.MultiSelectSearchResult {
232+
var searchResultKeys []string
233+
var searchResultLabels []string
234+
235+
// Initial search with no input
236+
if input == "" {
237+
moreResults := 2
238+
searchResultKeys = []string{"initial-result-1", "initial-result-2"}
239+
searchResultLabels = []string{"Initial Result Label 1", "Initial Result Label 2"}
240+
return prompter.MultiSelectSearchResult{
241+
Keys: searchResultKeys,
242+
Labels: searchResultLabels,
243+
MoreResults: moreResults,
244+
Err: nil,
245+
}
246+
}
247+
248+
// Subsequent search with input
249+
moreResults := 0
250+
searchResultKeys = []string{"search-result-1", "search-result-2"}
251+
searchResultLabels = []string{"Search Result Label 1", "Search Result Label 2"}
252+
return prompter.MultiSelectSearchResult{
253+
Keys: searchResultKeys,
254+
Labels: searchResultLabels,
255+
MoreResults: moreResults,
256+
Err: nil,
257+
}
258+
}
259+
260+
go func() {
261+
// Wait for prompt to appear
262+
_, err := console.ExpectString("Select an option \r\n")
263+
require.NoError(t, err)
264+
265+
// Select the search option, which will always be the first option
266+
_, err = console.SendLine("1")
267+
require.NoError(t, err)
268+
269+
// Submit search
270+
_, err = console.SendLine("0")
271+
require.NoError(t, err)
272+
273+
// Wait for the search prompt to appear
274+
_, err = console.ExpectString("Search for an option")
275+
require.NoError(t, err)
276+
277+
// Enter some search text to trigger the search
278+
_, err = console.SendLine("search text")
279+
require.NoError(t, err)
280+
281+
// Wait for the multiselect prompt to re-appear after search
282+
_, err = console.ExpectString("Select an option \r\n")
283+
require.NoError(t, err)
284+
285+
// Select the first search result
286+
_, err = console.SendLine("2")
287+
require.NoError(t, err)
288+
289+
// This confirms selections
290+
_, err = console.SendLine("0")
291+
require.NoError(t, err)
292+
}()
293+
multiSelectValues, err := p.MultiSelectWithSearch("Select an option", "Search for an option", []string{}, persistentOptions, searchFunc)
294+
require.NoError(t, err)
295+
assert.Equal(t, []string{"search-result-1"}, multiSelectValues)
296+
})
297+
298+
t.Run("MultiSelectWithSearch - defaults are pre-selected", func(t *testing.T) {
299+
console := newTestVirtualTerminal(t)
300+
p := newTestAccessiblePrompter(t, console)
301+
initialSearchResultKeys := []string{"initial-result-1"}
302+
initialSearchResultLabels := []string{"Initial Result Label 1"}
303+
defaultOptions := initialSearchResultKeys
304+
searchFunc := func(input string) prompter.MultiSelectSearchResult {
305+
// Initial search with no input
306+
if input == "" {
307+
moreResults := 2
308+
return prompter.MultiSelectSearchResult{
309+
Keys: initialSearchResultKeys,
310+
Labels: initialSearchResultLabels,
311+
MoreResults: moreResults,
312+
Err: nil,
313+
}
314+
}
315+
316+
// No search selected, so this should fail the test.
317+
t.FailNow()
318+
return prompter.MultiSelectSearchResult{
319+
Keys: nil,
320+
Labels: nil,
321+
MoreResults: 0,
322+
Err: nil,
323+
}
324+
}
325+
326+
go func() {
327+
// Wait for prompt to appear
328+
_, err := console.ExpectString("Select an option (default: Initial Result Label 1) \r\n")
329+
require.NoError(t, err)
330+
331+
// This confirms default selections
332+
_, err = console.SendLine("0")
333+
require.NoError(t, err)
334+
}()
335+
multiSelectValues, err := p.MultiSelectWithSearch("Select an option", "Search for an option", defaultOptions, initialSearchResultKeys, searchFunc)
336+
require.NoError(t, err)
337+
assert.Equal(t, defaultOptions, multiSelectValues)
338+
})
339+
340+
t.Run("MultiSelectWithSearch - selected options persist between searches", func(t *testing.T) {
341+
console := newTestVirtualTerminal(t)
342+
p := newTestAccessiblePrompter(t, console)
343+
initialSearchResultKeys := []string{"initial-result-1"}
344+
initialSearchResultLabels := []string{"Initial Result Label 1"}
345+
moreResultKeys := []string{"more-result-1"}
346+
moreResultLabels := []string{"More Result Label 1"}
347+
348+
searchFunc := func(input string) prompter.MultiSelectSearchResult {
349+
// Initial search with no input
350+
if input == "" {
351+
moreResults := 2
352+
return prompter.MultiSelectSearchResult{
353+
Keys: initialSearchResultKeys,
354+
Labels: initialSearchResultLabels,
355+
MoreResults: moreResults,
356+
Err: nil,
357+
}
358+
}
359+
360+
// Subsequent search with input "more"
361+
if input == "more" {
362+
return prompter.MultiSelectSearchResult{
363+
Keys: moreResultKeys,
364+
Labels: moreResultLabels,
365+
MoreResults: 0,
366+
Err: nil,
367+
}
368+
}
369+
370+
// No other searches expected
371+
t.FailNow()
372+
return prompter.MultiSelectSearchResult{
373+
Keys: nil,
374+
Labels: nil,
375+
MoreResults: 0,
376+
Err: nil,
377+
}
378+
}
379+
380+
go func() {
381+
// Wait for prompt to appear
382+
_, err := console.ExpectString("Select an option \r\n")
383+
require.NoError(t, err)
384+
385+
// Select one of our initial search results
386+
_, err = console.SendLine("2")
387+
require.NoError(t, err)
388+
389+
// Select to search
390+
_, err = console.SendLine("1")
391+
require.NoError(t, err)
392+
393+
// Submit the search selection
394+
_, err = console.SendLine("0")
395+
require.NoError(t, err)
396+
397+
// Wait for the search prompt to appear
398+
_, err = console.ExpectString("Search for an option")
399+
require.NoError(t, err)
400+
401+
// Enter some search text to trigger the search
402+
_, err = console.SendLine("more")
403+
require.NoError(t, err)
404+
405+
// Wait for the multiselect prompt to re-appear after search
406+
_, err = console.ExpectString("Select up to")
407+
require.NoError(t, err)
408+
409+
// Select the new option from the new search results
410+
_, err = console.SendLine("3")
411+
require.NoError(t, err)
412+
413+
// Submit selections
414+
_, err = console.SendLine("0")
415+
require.NoError(t, err)
416+
}()
417+
multiSelectValues, err := p.MultiSelectWithSearch("Select an option", "Search for an option", []string{}, []string{}, searchFunc)
418+
require.NoError(t, err)
419+
expectedValues := append(initialSearchResultKeys, moreResultKeys...)
420+
assert.Equal(t, expectedValues, multiSelectValues)
421+
})
422+
423+
t.Run("MultiSelectWithSearch - search error propagates", func(t *testing.T) {
424+
console := newTestVirtualTerminal(t)
425+
p := newTestAccessiblePrompter(t, console)
426+
427+
searchFunc := func(input string) prompter.MultiSelectSearchResult {
428+
return prompter.MultiSelectSearchResult{
429+
Err: fmt.Errorf("search error"),
430+
}
431+
}
432+
433+
_, err := p.MultiSelectWithSearch("Select", "Search", []string{}, []string{}, searchFunc)
434+
require.Error(t, err)
435+
require.Contains(t, err.Error(), "search error")
436+
})
437+
227438
t.Run("Input", func(t *testing.T) {
228439
console := newTestVirtualTerminal(t)
229440
p := newTestAccessiblePrompter(t, console)
@@ -642,6 +853,9 @@ func newTestVirtualTerminal(t *testing.T) *expect.Console {
642853
failOnExpectError(t),
643854
failOnSendError(t),
644855
expect.WithDefaultTimeout(time.Second),
856+
// Use this logger to debug expect based tests by printing the
857+
// characters being read to stdout.
858+
// expect.WithLogger(log.New(os.Stdout, "", 0)),
645859
}
646860

647861
console, err := expect.NewConsole(consoleOpts...)

0 commit comments

Comments
 (0)