diff --git a/api/queries_repo.go b/api/queries_repo.go index 3190745ea99..6552aa230cb 100644 --- a/api/queries_repo.go +++ b/api/queries_repo.go @@ -1033,114 +1033,6 @@ func RepoMetadata(client *Client, repo ghrepo.Interface, input RepoMetadataInput return &result, nil } -type RepoResolveInput struct { - Assignees []string - Reviewers []string - Labels []string - ProjectsV1 bool - ProjectsV2 bool - Milestones []string -} - -// RepoResolveMetadataIDs looks up GraphQL node IDs in bulk -func RepoResolveMetadataIDs(client *Client, repo ghrepo.Interface, input RepoResolveInput) (*RepoMetadataResult, error) { - users := input.Assignees - hasUser := func(target string) bool { - for _, u := range users { - if strings.EqualFold(u, target) { - return true - } - } - return false - } - - var teams []string - for _, r := range input.Reviewers { - if i := strings.IndexRune(r, '/'); i > -1 { - teams = append(teams, r[i+1:]) - } else if !hasUser(r) { - users = append(users, r) - } - } - - // there is no way to look up projects nor milestones by name, so preload them all - mi := RepoMetadataInput{ - ProjectsV1: input.ProjectsV1, - ProjectsV2: input.ProjectsV2, - Milestones: len(input.Milestones) > 0, - } - result, err := RepoMetadata(client, repo, mi) - if err != nil { - return result, err - } - if len(users) == 0 && len(teams) == 0 && len(input.Labels) == 0 { - return result, nil - } - - query := &bytes.Buffer{} - fmt.Fprint(query, "query RepositoryResolveMetadataIDs {\n") - for i, u := range users { - fmt.Fprintf(query, "u%03d: user(login:%q){id,login}\n", i, u) - } - if len(input.Labels) > 0 { - fmt.Fprintf(query, "repository(owner:%q,name:%q){\n", repo.RepoOwner(), repo.RepoName()) - for i, l := range input.Labels { - fmt.Fprintf(query, "l%03d: label(name:%q){id,name}\n", i, l) - } - fmt.Fprint(query, "}\n") - } - if len(teams) > 0 { - fmt.Fprintf(query, "organization(login:%q){\n", repo.RepoOwner()) - for i, t := range teams { - fmt.Fprintf(query, "t%03d: team(slug:%q){id,slug}\n", i, t) - } - fmt.Fprint(query, "}\n") - } - fmt.Fprint(query, "}\n") - - response := make(map[string]json.RawMessage) - err = client.GraphQL(repo.RepoHost(), query.String(), nil, &response) - if err != nil { - return result, err - } - - for key, v := range response { - switch key { - case "repository": - repoResponse := make(map[string]RepoLabel) - err := json.Unmarshal(v, &repoResponse) - if err != nil { - return result, err - } - for _, l := range repoResponse { - result.Labels = append(result.Labels, l) - } - case "organization": - orgResponse := make(map[string]OrgTeam) - err := json.Unmarshal(v, &orgResponse) - if err != nil { - return result, err - } - for _, t := range orgResponse { - result.Teams = append(result.Teams, t) - } - default: - user := struct { - Id string - Login string - Name string - }{} - err := json.Unmarshal(v, &user) - if err != nil { - return result, err - } - result.AssignableUsers = append(result.AssignableUsers, NewAssignableUser(user.Id, user.Login, user.Name)) - } - } - - return result, nil -} - type RepoProject struct { ID string `json:"id"` Name string `json:"name"` @@ -1190,6 +1082,7 @@ func RepoProjects(client *Client, repo ghrepo.Interface) ([]RepoProject, error) // This is returned from assignable actors and issue/pr assigned actors. // We use this to check if the actor is Copilot. const CopilotActorLogin = "copilot-swe-agent" +const CopilotActorName = "Copilot" type AssignableActor interface { DisplayName() string @@ -1250,7 +1143,7 @@ func NewAssignableBot(id, login string) AssignableBot { func (b AssignableBot) DisplayName() string { if b.login == CopilotActorLogin { - return "Copilot (AI)" + return fmt.Sprintf("%s (AI)", CopilotActorName) } return b.Login() } diff --git a/api/queries_repo_test.go b/api/queries_repo_test.go index c291fc4686a..c885b99682a 100644 --- a/api/queries_repo_test.go +++ b/api/queries_repo_test.go @@ -379,88 +379,6 @@ func Test_ProjectNamesToPaths(t *testing.T) { }) } -func Test_RepoResolveMetadataIDs(t *testing.T) { - http := &httpmock.Registry{} - client := newTestClient(http) - - repo, _ := ghrepo.FromFullName("OWNER/REPO") - input := RepoResolveInput{ - Assignees: []string{"monalisa", "hubot"}, - Reviewers: []string{"monalisa", "octocat", "OWNER/core", "/robots"}, - Labels: []string{"bug", "help wanted"}, - } - - expectedQuery := `query RepositoryResolveMetadataIDs { -u000: user(login:"monalisa"){id,login} -u001: user(login:"hubot"){id,login} -u002: user(login:"octocat"){id,login} -repository(owner:"OWNER",name:"REPO"){ -l000: label(name:"bug"){id,name} -l001: label(name:"help wanted"){id,name} -} -organization(login:"OWNER"){ -t000: team(slug:"core"){id,slug} -t001: team(slug:"robots"){id,slug} -} -} -` - responseJSON := ` - { "data": { - "u000": { "login": "MonaLisa", "id": "MONAID" }, - "u001": { "login": "hubot", "id": "HUBOTID" }, - "u002": { "login": "octocat", "id": "OCTOID" }, - "repository": { - "l000": { "name": "bug", "id": "BUGID" }, - "l001": { "name": "Help Wanted", "id": "HELPID" } - }, - "organization": { - "t000": { "slug": "core", "id": "COREID" }, - "t001": { "slug": "Robots", "id": "ROBOTID" } - } - } } - ` - - http.Register( - httpmock.GraphQL(`query RepositoryResolveMetadataIDs\b`), - httpmock.GraphQLQuery(responseJSON, func(q string, _ map[string]interface{}) { - if q != expectedQuery { - t.Errorf("expected query %q, got %q", expectedQuery, q) - } - })) - - result, err := RepoResolveMetadataIDs(client, repo, input) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - - expectedMemberIDs := []string{"MONAID", "HUBOTID", "OCTOID"} - memberIDs, err := result.MembersToIDs([]string{"monalisa", "hubot", "octocat"}) - if err != nil { - t.Errorf("error resolving members: %v", err) - } - if !sliceEqual(memberIDs, expectedMemberIDs) { - t.Errorf("expected members %v, got %v", expectedMemberIDs, memberIDs) - } - - expectedTeamIDs := []string{"COREID", "ROBOTID"} - teamIDs, err := result.TeamsToIDs([]string{"/core", "/robots"}) - if err != nil { - t.Errorf("error resolving teams: %v", err) - } - if !sliceEqual(teamIDs, expectedTeamIDs) { - t.Errorf("expected members %v, got %v", expectedTeamIDs, teamIDs) - } - - expectedLabelIDs := []string{"BUGID", "HELPID"} - labelIDs, err := result.LabelsToIDs([]string{"bug", "help wanted"}) - if err != nil { - t.Errorf("error resolving labels: %v", err) - } - if !sliceEqual(labelIDs, expectedLabelIDs) { - t.Errorf("expected members %v, got %v", expectedLabelIDs, labelIDs) - } -} - func TestMembersToIDs(t *testing.T) { t.Parallel() diff --git a/pkg/cmd/issue/create/create.go b/pkg/cmd/issue/create/create.go index 2978a21fc7b..bc38c52b356 100644 --- a/pkg/cmd/issue/create/create.go +++ b/pkg/cmd/issue/create/create.go @@ -16,6 +16,7 @@ import ( prShared "github.com/cli/cli/v2/pkg/cmd/pr/shared" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/iostreams" + "github.com/cli/cli/v2/pkg/set" "github.com/spf13/cobra" ) @@ -68,6 +69,10 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co Adding an issue to projects requires authorization with the %[1]sproject%[1]s scope. To authorize, run %[1]sgh auth refresh -s project%[1]s. + + The %[1]s--assignee%[1]s flag supports the following special values: + - %[1]s@me%[1]s: assign yourself + - %[1]s@copilot%[1]s: assign Copilot (not supported on GitHub Enterprise Server) `, "`"), Example: heredoc.Doc(` $ gh issue create --title "I found a bug" --body "Nothing works" @@ -75,6 +80,7 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co $ gh issue create --label bug --label "help wanted" $ gh issue create --assignee monalisa,hubot $ gh issue create --assignee "@me" + $ gh issue create --assignee "@copilot" $ gh issue create --project "Roadmap" $ gh issue create --template "Bug Report" `), @@ -158,6 +164,10 @@ func createRun(opts *CreateOptions) (err error) { } projectsV1Support := opts.Detector.ProjectsV1() + issueFeatures, err := opts.Detector.IssueFeatures() + if err != nil { + return err + } isTerminal := opts.IO.IsStdoutTTY() @@ -166,20 +176,30 @@ func createRun(opts *CreateOptions) (err error) { milestones = []string{opts.Milestone} } + // Replace special values in assignees + // For web mode, @copilot should be replaced by name; otherwise, login. + assigneeSet := set.NewStringSet() meReplacer := prShared.NewMeReplacer(apiClient, baseRepo.RepoHost()) + copilotReplacer := prShared.NewCopilotReplacer(!opts.WebMode) assignees, err := meReplacer.ReplaceSlice(opts.Assignees) if err != nil { return err } + if issueFeatures.ActorIsAssignable { + assignees = copilotReplacer.ReplaceSlice(assignees) + } + assigneeSet.AddValues(assignees) + tb := prShared.IssueMetadataState{ - Type: prShared.IssueMetadata, - Assignees: assignees, - Labels: opts.Labels, - ProjectTitles: opts.Projects, - Milestones: milestones, - Title: opts.Title, - Body: opts.Body, + Type: prShared.IssueMetadata, + ActorAssignees: issueFeatures.ActorIsAssignable, + Assignees: assigneeSet.ToSlice(), + Labels: opts.Labels, + ProjectTitles: opts.Projects, + Milestones: milestones, + Title: opts.Title, + Body: opts.Body, } if opts.RecoverFile != "" { diff --git a/pkg/cmd/issue/create/create_test.go b/pkg/cmd/issue/create/create_test.go index 1211c0c1d93..80c8f76d35b 100644 --- a/pkg/cmd/issue/create/create_test.go +++ b/pkg/cmd/issue/create/create_test.go @@ -256,6 +256,7 @@ func Test_createRun(t *testing.T) { name string opts CreateOptions httpStubs func(*httpmock.Registry) + promptStubs func(*prompter.PrompterMock) wantsStdout string wantsStderr string wantsBrowse string @@ -264,7 +265,8 @@ func Test_createRun(t *testing.T) { { name: "no args", opts: CreateOptions{ - WebMode: true, + Detector: &fd.EnabledDetectorMock{}, + WebMode: true, }, wantsBrowse: "https://github.com/OWNER/REPO/issues/new", wantsStderr: "Opening https://github.com/OWNER/REPO/issues/new in your browser.\n", @@ -272,9 +274,10 @@ func Test_createRun(t *testing.T) { { name: "title and body", opts: CreateOptions{ - WebMode: true, - Title: "myissue", - Body: "hello cli", + Detector: &fd.EnabledDetectorMock{}, + WebMode: true, + Title: "myissue", + Body: "hello cli", }, wantsBrowse: "https://github.com/OWNER/REPO/issues/new?body=hello+cli&title=myissue", wantsStderr: "Opening https://github.com/OWNER/REPO/issues/new in your browser.\n", @@ -282,6 +285,7 @@ func Test_createRun(t *testing.T) { { name: "assignee", opts: CreateOptions{ + Detector: &fd.EnabledDetectorMock{}, WebMode: true, Assignees: []string{"monalisa"}, }, @@ -291,6 +295,7 @@ func Test_createRun(t *testing.T) { { name: "@me", opts: CreateOptions{ + Detector: &fd.EnabledDetectorMock{}, WebMode: true, Assignees: []string{"@me"}, }, @@ -305,9 +310,20 @@ func Test_createRun(t *testing.T) { wantsBrowse: "https://github.com/OWNER/REPO/issues/new?assignees=MonaLisa&body=", wantsStderr: "Opening https://github.com/OWNER/REPO/issues/new in your browser.\n", }, + { + name: "@copilot", + opts: CreateOptions{ + Detector: &fd.EnabledDetectorMock{}, + WebMode: true, + Assignees: []string{"@copilot"}, + }, + wantsBrowse: "https://github.com/OWNER/REPO/issues/new?assignees=Copilot&body=", + wantsStderr: "Opening https://github.com/OWNER/REPO/issues/new in your browser.\n", + }, { name: "project", opts: CreateOptions{ + Detector: &fd.EnabledDetectorMock{}, WebMode: true, Projects: []string{"cleanup"}, }, @@ -364,7 +380,8 @@ func Test_createRun(t *testing.T) { { name: "has templates", opts: CreateOptions{ - WebMode: true, + Detector: &fd.EnabledDetectorMock{}, + WebMode: true, }, httpStubs: func(r *httpmock.Registry) { r.Register( @@ -384,8 +401,9 @@ func Test_createRun(t *testing.T) { { name: "too long body", opts: CreateOptions{ - WebMode: true, - Body: strings.Repeat("A", 9216), + Detector: &fd.EnabledDetectorMock{}, + WebMode: true, + Body: strings.Repeat("A", 9216), }, wantsErr: "cannot open in browser: maximum URL length exceeded", }, @@ -395,22 +413,23 @@ func Test_createRun(t *testing.T) { r.Register( httpmock.GraphQL(`query RepositoryInfo\b`), httpmock.StringResponse(` - { "data": { "repository": { - "id": "REPOID", - "hasIssuesEnabled": true - } } }`)) + { "data": { "repository": { + "id": "REPOID", + "hasIssuesEnabled": true + } } }`)) r.Register( httpmock.GraphQL(`mutation IssueCreate\b`), httpmock.GraphQLMutation(` - { "data": { "createIssue": { "issue": { - "URL": "https://github.com/OWNER/REPO/issues/12" - } } } } - `, func(inputs map[string]interface{}) { + { "data": { "createIssue": { "issue": { + "URL": "https://github.com/OWNER/REPO/issues/12" + } } } } + `, func(inputs map[string]interface{}) { assert.Equal(t, "title", inputs["title"]) assert.Equal(t, "body", inputs["body"]) })) }, opts: CreateOptions{ + Detector: &fd.EnabledDetectorMock{}, EditorMode: true, TitledEditSurvey: func(string, string) (string, string, error) { return "title", "body", nil }, }, @@ -448,6 +467,7 @@ func Test_createRun(t *testing.T) { })) }, opts: CreateOptions{ + Detector: &fd.EnabledDetectorMock{}, EditorMode: true, Template: "Bug report", TitledEditSurvey: func(title string, body string) (string, string, error) { return title, body, nil }, @@ -455,6 +475,152 @@ func Test_createRun(t *testing.T) { wantsStdout: "https://github.com/OWNER/REPO/issues/12\n", wantsStderr: "\nCreating issue in OWNER/REPO\n\n", }, + { + name: "interactive prompts with actor assignee display names when actors available", + opts: CreateOptions{ + Interactive: true, + Detector: &fd.EnabledDetectorMock{}, + Title: "test `gh issue create` actor assignees", + Body: "Actor assignees allow users and bots to be assigned to issues", + }, + promptStubs: func(pm *prompter.PrompterMock) { + firstConfirmSubmission := true + pm.InputFunc = func(message, defaultValue string) (string, error) { + switch message { + default: + return "", fmt.Errorf("unexpected input prompt: %s", message) + } + } + pm.MultiSelectFunc = func(message string, defaults []string, options []string) ([]int, error) { + switch message { + case "What would you like to add?": + return prompter.IndexesFor(options, "Assignees") + case "Assignees": + return prompter.IndexesFor(options, "Copilot (AI)", "MonaLisa (Mona Display Name)") + default: + return nil, fmt.Errorf("unexpected multi-select prompt: %s", message) + } + } + pm.SelectFunc = func(message, defaultValue string, options []string) (int, error) { + switch message { + case "What's next?": + if firstConfirmSubmission { + firstConfirmSubmission = false + return prompter.IndexFor(options, "Add metadata") + } + return prompter.IndexFor(options, "Submit") + default: + return 0, fmt.Errorf("unexpected select prompt: %s", message) + } + } + }, + httpStubs: func(r *httpmock.Registry) { + r.Register( + httpmock.GraphQL(`query RepositoryInfo\b`), + httpmock.StringResponse(` + { "data": { "repository": { + "id": "REPOID", + "hasIssuesEnabled": true, + "viewerPermission": "WRITE" + } } } + `)) + r.Register( + httpmock.GraphQL(`query RepositoryAssignableActors\b`), + httpmock.StringResponse(` + { "data": { "repository": { "suggestedActors": { + "nodes": [ + { "login": "copilot-swe-agent", "id": "COPILOTID", "name": "Copilot (AI)", "__typename": "Bot" }, + { "login": "MonaLisa", "id": "MONAID", "name": "Mona Display Name", "__typename": "User" } + ], + "pageInfo": { "hasNextPage": false } + } } } } + `)) + r.Register( + httpmock.GraphQL(`mutation IssueCreate\b`), + httpmock.GraphQLMutation(` + { "data": { "createIssue": { "issue": { + "URL": "https://github.com/OWNER/REPO/issues/12" + } } } } + `, func(inputs map[string]interface{}) { + assert.Equal(t, []interface{}{"COPILOTID", "MONAID"}, inputs["assigneeIds"]) + })) + }, + wantsStdout: "https://github.com/OWNER/REPO/issues/12\n", + wantsStderr: "\nCreating issue in OWNER/REPO\n\n", + }, + { + name: "interactive prompts with user assignee logins when actors unavailable", + opts: CreateOptions{ + Interactive: true, + Detector: &fd.DisabledDetectorMock{}, + Title: "test `gh issue create` user assignees", + Body: "User assignees allow only users to be assigned to issues", + }, + promptStubs: func(pm *prompter.PrompterMock) { + firstConfirmSubmission := true + pm.InputFunc = func(message, defaultValue string) (string, error) { + switch message { + default: + return "", fmt.Errorf("unexpected input prompt: %s", message) + } + } + pm.MultiSelectFunc = func(message string, defaults []string, options []string) ([]int, error) { + switch message { + case "What would you like to add?": + return prompter.IndexesFor(options, "Assignees") + case "Assignees": + return prompter.IndexesFor(options, "hubot", "MonaLisa (Mona Display Name)") + default: + return nil, fmt.Errorf("unexpected multi-select prompt: %s", message) + } + } + pm.SelectFunc = func(message, defaultValue string, options []string) (int, error) { + switch message { + case "What's next?": + if firstConfirmSubmission { + firstConfirmSubmission = false + return prompter.IndexFor(options, "Add metadata") + } + return prompter.IndexFor(options, "Submit") + default: + return 0, fmt.Errorf("unexpected select prompt: %s", message) + } + } + }, + httpStubs: func(r *httpmock.Registry) { + r.Register( + httpmock.GraphQL(`query RepositoryInfo\b`), + httpmock.StringResponse(` + { "data": { "repository": { + "id": "REPOID", + "hasIssuesEnabled": true, + "viewerPermission": "WRITE" + } } } + `)) + r.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 } + } } } } + `)) + r.Register( + httpmock.GraphQL(`mutation IssueCreate\b`), + httpmock.GraphQLMutation(` + { "data": { "createIssue": { "issue": { + "URL": "https://github.com/OWNER/REPO/issues/12" + } } } } + `, func(inputs map[string]interface{}) { + assert.Equal(t, []interface{}{"HUBOTID", "MONAID"}, inputs["assigneeIds"]) + })) + }, + wantsStdout: "https://github.com/OWNER/REPO/issues/12\n", + wantsStderr: "\nCreating issue in OWNER/REPO\n\n", + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -474,10 +640,15 @@ func Test_createRun(t *testing.T) { opts.BaseRepo = func() (ghrepo.Interface, error) { return ghrepo.New("OWNER", "REPO"), nil } - opts.Detector = &fd.EnabledDetectorMock{} browser := &browser.Stub{} opts.Browser = browser + prompterMock := &prompter.PrompterMock{} + opts.Prompter = prompterMock + if tt.promptStubs != nil { + tt.promptStubs(prompterMock) + } + err := createRun(opts) if tt.wantsErr == "" { require.NoError(t, err) @@ -589,16 +760,17 @@ func TestIssueCreate_recover(t *testing.T) { "id": "REPOID", "hasIssuesEnabled": true } } }`)) + // Should only be one fetch of metadata. http.Register( - httpmock.GraphQL(`query RepositoryResolveMetadataIDs\b`), + httpmock.GraphQL(`query RepositoryLabelList\b`), httpmock.StringResponse(` - { "data": { - "u000": { "login": "MonaLisa", "id": "MONAID" }, - "repository": { - "l000": { "name": "bug", "id": "BUGID" }, - "l001": { "name": "TODO", "id": "TODOID" } - } - } } + { "data": { "repository": { "labels": { + "nodes": [ + { "name": "TODO", "id": "TODOID" }, + { "name": "bug", "id": "BUGID" } + ], + "pageInfo": { "hasNextPage": false } + } } } } `)) http.Register( httpmock.GraphQL(`mutation IssueCreate\b`), @@ -777,15 +949,25 @@ func TestIssueCreate_metadata(t *testing.T) { http.StubRepoInfoResponse("OWNER", "REPO", "main") http.Register( - httpmock.GraphQL(`query RepositoryResolveMetadataIDs\b`), + httpmock.GraphQL(`query RepositoryAssignableActors\b`), httpmock.StringResponse(` - { "data": { - "u000": { "login": "MonaLisa", "id": "MONAID" }, - "repository": { - "l000": { "name": "bug", "id": "BUGID" }, - "l001": { "name": "TODO", "id": "TODOID" } - } - } } + { "data": { "repository": { "suggestedActors": { + "nodes": [ + { "login": "MonaLisa", "id": "MONAID", "name": "Mona Display Name", "__typename": "User" } + ], + "pageInfo": { "hasNextPage": false } + } } } } + `)) + http.Register( + httpmock.GraphQL(`query RepositoryLabelList\b`), + httpmock.StringResponse(` + { "data": { "repository": { "labels": { + "nodes": [ + { "name": "TODO", "id": "TODOID" }, + { "name": "bug", "id": "BUGID" } + ], + "pageInfo": { "hasNextPage": false } + } } } } `)) http.Register( httpmock.GraphQL(`query RepositoryMilestoneList\b`), @@ -910,18 +1092,16 @@ func TestIssueCreate_AtMeAssignee(t *testing.T) { } } } `)) http.Register( - httpmock.GraphQL(`query RepositoryResolveMetadataIDs\b`), + httpmock.GraphQL(`query RepositoryAssignableActors\b`), httpmock.StringResponse(` - { "data": { - "u000": { "login": "MonaLisa", "id": "MONAID" }, - "u001": { "login": "SomeOneElse", "id": "SOMEID" }, - "repository": { - "l000": { "name": "bug", "id": "BUGID" }, - "l001": { "name": "TODO", "id": "TODOID" } - } - } } - `), - ) + { "data": { "repository": { "suggestedActors": { + "nodes": [ + { "login": "MonaLisa", "id": "MONAID", "name": "Mona Display Name", "__typename": "User" }, + { "login": "SomeOneElse", "id": "SOMEID", "name": "Someone else", "__typename": "User" } + ], + "pageInfo": { "hasNextPage": false } + } } } } + `)) http.Register( httpmock.GraphQL(`mutation IssueCreate\b`), httpmock.GraphQLMutation(` @@ -942,6 +1122,48 @@ func TestIssueCreate_AtMeAssignee(t *testing.T) { assert.Equal(t, "https://github.com/OWNER/REPO/issues/12\n", output.String()) } +func TestIssueCreate_AtCopilotAssignee(t *testing.T) { + http := &httpmock.Registry{} + defer http.Verify(t) + + http.Register( + httpmock.GraphQL(`query RepositoryInfo\b`), + httpmock.StringResponse(` + { "data": { "repository": { + "id": "REPOID", + "hasIssuesEnabled": true + } } } + `)) + http.Register( + httpmock.GraphQL(`query RepositoryAssignableActors\b`), + httpmock.StringResponse(` + { "data": { "repository": { "suggestedActors": { + "nodes": [ + { "login": "copilot-swe-agent", "id": "COPILOTID", "name": "Copilot (AI)", "__typename": "Bot" } + ], + "pageInfo": { "hasNextPage": false } + } } } } + `)) + http.Register( + httpmock.GraphQL(`mutation IssueCreate\b`), + httpmock.GraphQLMutation(` + { "data": { "createIssue": { "issue": { + "URL": "https://github.com/OWNER/REPO/issues/12" + } } } } + `, func(inputs map[string]interface{}) { + assert.Equal(t, "hello", inputs["title"]) + assert.Equal(t, "cash rules everything around me", inputs["body"]) + assert.Equal(t, []interface{}{"COPILOTID"}, inputs["assigneeIds"]) + })) + + output, err := runCommand(http, true, `-a @copilot -t hello -b "cash rules everything around me"`, nil) + if err != nil { + t.Errorf("error running command `issue create`: %v", err) + } + + assert.Equal(t, "https://github.com/OWNER/REPO/issues/12\n", output.String()) +} + func TestIssueCreate_projectsV2(t *testing.T) { http := &httpmock.Registry{} defer http.Verify(t) diff --git a/pkg/cmd/pr/create/create_test.go b/pkg/cmd/pr/create/create_test.go index 5ae5a52e66a..7f1c8884faf 100644 --- a/pkg/cmd/pr/create/create_test.go +++ b/pkg/cmd/pr/create/create_test.go @@ -429,20 +429,29 @@ func Test_createRun(t *testing.T) { }, httpStubs: func(reg *httpmock.Registry, t *testing.T) { reg.Register( - httpmock.GraphQL(`query RepositoryResolveMetadataIDs\b`), + httpmock.GraphQL(`query UserCurrent\b`), + httpmock.StringResponse(`{"data": {"viewer": {"login": "OWNER"} } }`)) + reg.Register( + httpmock.GraphQL(`query RepositoryAssignableUsers\b`), httpmock.StringResponse(` - { "data": { - "u000": { "login": "MonaLisa", "id": "MONAID" }, - "u001": { "login": "hubot", "id": "HUBOTID" }, - "repository": { - "l000": { "name": "bug", "id": "BUGID" }, - "l001": { "name": "TODO", "id": "TODOID" } - }, - "organization": { - "t000": { "slug": "core", "id": "COREID" }, - "t001": { "slug": "robots", "id": "ROBOTID" } - } - } } + { "data": { "repository": { "assignableUsers": { + "nodes": [ + { "login": "hubot", "id": "HUBOTID", "name": "" }, + { "login": "MonaLisa", "id": "MONAID", "name": "Mona Display Name" } + ], + "pageInfo": { "hasNextPage": false } + } } } } + `)) + reg.Register( + httpmock.GraphQL(`query RepositoryLabelList\b`), + httpmock.StringResponse(` + { "data": { "repository": { "labels": { + "nodes": [ + { "name": "TODO", "id": "TODOID" }, + { "name": "bug", "id": "BUGID" } + ], + "pageInfo": { "hasNextPage": false } + } } } } `)) reg.Register( httpmock.GraphQL(`query RepositoryMilestoneList\b`), @@ -455,6 +464,17 @@ func Test_createRun(t *testing.T) { "pageInfo": { "hasNextPage": false } } } } } `)) + reg.Register( + httpmock.GraphQL(`query OrganizationTeamList\b`), + httpmock.StringResponse(` + { "data": { "organization": { "teams": { + "nodes": [ + { "slug": "core", "id": "COREID" }, + { "slug": "robots", "id": "ROBOTID" } + ], + "pageInfo": { "hasNextPage": false } + } } } } + `)) mockRetrieveProjects(t, reg) }, expectedOutputs: []string{ @@ -526,20 +546,29 @@ func Test_createRun(t *testing.T) { }, httpStubs: func(reg *httpmock.Registry, t *testing.T) { reg.Register( - httpmock.GraphQL(`query RepositoryResolveMetadataIDs\b`), + httpmock.GraphQL(`query UserCurrent\b`), + httpmock.StringResponse(`{"data": {"viewer": {"login": "OWNER"} } }`)) + reg.Register( + httpmock.GraphQL(`query RepositoryAssignableUsers\b`), httpmock.StringResponse(` - { "data": { - "u000": { "login": "MonaLisa", "id": "MONAID" }, - "u001": { "login": "hubot", "id": "HUBOTID" }, - "repository": { - "l000": { "name": "bug", "id": "BUGID" }, - "l001": { "name": "TODO", "id": "TODOID" } - }, - "organization": { - "t000": { "slug": "core", "id": "COREID" }, - "t001": { "slug": "robots", "id": "ROBOTID" } - } - } } + { "data": { "repository": { "assignableUsers": { + "nodes": [ + { "login": "hubot", "id": "HUBOTID", "name": "" }, + { "login": "MonaLisa", "id": "MONAID", "name": "Mona Display Name" } + ], + "pageInfo": { "hasNextPage": false } + } } } } + `)) + reg.Register( + httpmock.GraphQL(`query RepositoryLabelList\b`), + httpmock.StringResponse(` + { "data": { "repository": { "labels": { + "nodes": [ + { "name": "TODO", "id": "TODOID" }, + { "name": "bug", "id": "BUGID" } + ], + "pageInfo": { "hasNextPage": false } + } } } } `)) reg.Register( httpmock.GraphQL(`query RepositoryMilestoneList\b`), @@ -552,6 +581,17 @@ func Test_createRun(t *testing.T) { "pageInfo": { "hasNextPage": false } } } } } `)) + reg.Register( + httpmock.GraphQL(`query OrganizationTeamList\b`), + httpmock.StringResponse(` + { "data": { "organization": { "teams": { + "nodes": [ + { "slug": "core", "id": "COREID" }, + { "slug": "robots", "id": "ROBOTID" } + ], + "pageInfo": { "hasNextPage": false } + } } } } + `)) mockRetrieveProjects(t, reg) }, expectedOutputs: []string{ @@ -1028,20 +1068,29 @@ func Test_createRun(t *testing.T) { }, httpStubs: func(reg *httpmock.Registry, t *testing.T) { reg.Register( - httpmock.GraphQL(`query RepositoryResolveMetadataIDs\b`), + httpmock.GraphQL(`query UserCurrent\b`), + httpmock.StringResponse(`{"data": {"viewer": {"login": "OWNER"} } }`)) + reg.Register( + httpmock.GraphQL(`query RepositoryAssignableUsers\b`), httpmock.StringResponse(` - { "data": { - "u000": { "login": "MonaLisa", "id": "MONAID" }, - "u001": { "login": "hubot", "id": "HUBOTID" }, - "repository": { - "l000": { "name": "bug", "id": "BUGID" }, - "l001": { "name": "TODO", "id": "TODOID" } - }, - "organization": { - "t000": { "slug": "core", "id": "COREID" }, - "t001": { "slug": "robots", "id": "ROBOTID" } - } - } } + { "data": { "repository": { "assignableUsers": { + "nodes": [ + { "login": "hubot", "id": "HUBOTID", "name": "" }, + { "login": "MonaLisa", "id": "MONAID", "name": "Mona Display Name" } + ], + "pageInfo": { "hasNextPage": false } + } } } } + `)) + reg.Register( + httpmock.GraphQL(`query RepositoryLabelList\b`), + httpmock.StringResponse(` + { "data": { "repository": { "labels": { + "nodes": [ + { "name": "TODO", "id": "TODOID" }, + { "name": "bug", "id": "BUGID" } + ], + "pageInfo": { "hasNextPage": false } + } } } } `)) reg.Register( httpmock.GraphQL(`query RepositoryMilestoneList\b`), @@ -1054,6 +1103,17 @@ func Test_createRun(t *testing.T) { "pageInfo": { "hasNextPage": false } } } } } `)) + reg.Register( + httpmock.GraphQL(`query OrganizationTeamList\b`), + httpmock.StringResponse(` + { "data": { "organization": { "teams": { + "nodes": [ + { "slug": "core", "id": "COREID" }, + { "slug": "robots", "id": "ROBOTID" } + ], + "pageInfo": { "hasNextPage": false } + } } } } + `)) mockRetrieveProjects(t, reg) reg.Register( httpmock.GraphQL(`mutation PullRequestCreate\b`), @@ -1242,30 +1302,42 @@ func Test_createRun(t *testing.T) { tty: true, httpStubs: func(reg *httpmock.Registry, t *testing.T) { reg.Register( - httpmock.GraphQL(`query RepositoryResolveMetadataIDs\b`), + httpmock.GraphQL(`query UserCurrent\b`), + httpmock.StringResponse(`{"data": {"viewer": {"login": "OWNER"} } }`)) + reg.Register( + httpmock.GraphQL(`query RepositoryAssignableUsers\b`), httpmock.StringResponse(` - { "data": { - "u000": { "login": "jillValentine", "id": "JILLID" }, - "repository": {}, - "organization": {} - } } - `)) + { "data": { "repository": { "assignableUsers": { + "nodes": [ + { "login": "jillValentine", "id": "JILLID", "name": "Jill Valentine" } + ], + "pageInfo": { "hasNextPage": false } + } } } } + `)) + reg.Register( + httpmock.GraphQL(`query OrganizationTeamList\b`), + httpmock.StringResponse(` + { "data": { "organization": { "teams": { + "nodes": [], + "pageInfo": { "hasNextPage": false } + } } } } + `)) reg.Register( httpmock.GraphQL(`mutation PullRequestCreateRequestReviews\b`), httpmock.GraphQLMutation(` - { "data": { "requestReviews": { - "clientMutationId": "" - } } } - `, func(inputs map[string]interface{}) { + { "data": { "requestReviews": { + "clientMutationId": "" + } } } + `, func(inputs map[string]interface{}) { assert.Equal(t, []interface{}{"JILLID"}, inputs["userIds"]) })) reg.Register( httpmock.GraphQL(`mutation PullRequestCreate\b`), httpmock.GraphQLMutation(` - { "data": { "createPullRequest": { "pullRequest": { - "URL": "https://github.com/OWNER/REPO/pull/12" - } } } } - `, func(input map[string]interface{}) { + { "data": { "createPullRequest": { "pullRequest": { + "URL": "https://github.com/OWNER/REPO/pull/12" + } } } } + `, func(input map[string]interface{}) { assert.Equal(t, "recovered title", input["title"].(string)) assert.Equal(t, "recovered body", input["body"].(string)) })) diff --git a/pkg/cmd/pr/shared/editable.go b/pkg/cmd/pr/shared/editable.go index 2f51f2ae814..ffe1642da40 100644 --- a/pkg/cmd/pr/shared/editable.go +++ b/pkg/cmd/pr/shared/editable.go @@ -118,7 +118,7 @@ func (e Editable) AssigneeIds(client *api.Client, repo ghrepo.Interface) (*[]str // 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()) - copilotReplacer := NewCopilotReplacer() + copilotReplacer := NewCopilotReplacer(true) replaceSpecialAssigneeNames := func(value []string) ([]string, error) { replaced, err := meReplacer.ReplaceSlice(value) diff --git a/pkg/cmd/pr/shared/params.go b/pkg/cmd/pr/shared/params.go index 1fa45652abd..d94f338c5e6 100644 --- a/pkg/cmd/pr/shared/params.go +++ b/pkg/cmd/pr/shared/params.go @@ -55,56 +55,28 @@ func ValidURL(urlStr string) bool { return len(urlStr) < 8192 } -// Ensure that tb.MetadataResult object exists and contains enough pre-fetched API data to be able -// to resolve all object listed in tb to GraphQL IDs. -func fillMetadata(client *api.Client, baseRepo ghrepo.Interface, tb *IssueMetadataState, projectV1Support gh.ProjectsV1Support) error { - resolveInput := api.RepoResolveInput{} - - if len(tb.Assignees) > 0 && (tb.MetadataResult == nil || len(tb.MetadataResult.AssignableUsers) == 0) { - resolveInput.Assignees = tb.Assignees - } - - if len(tb.Reviewers) > 0 && (tb.MetadataResult == nil || len(tb.MetadataResult.AssignableUsers) == 0) { - resolveInput.Reviewers = tb.Reviewers - } - - if len(tb.Labels) > 0 && (tb.MetadataResult == nil || len(tb.MetadataResult.Labels) == 0) { - resolveInput.Labels = tb.Labels - } - - if len(tb.ProjectTitles) > 0 && (tb.MetadataResult == nil || len(tb.MetadataResult.Projects) == 0) { - if projectV1Support == gh.ProjectsV1Supported { - resolveInput.ProjectsV1 = true - } - - resolveInput.ProjectsV2 = true - } - - if len(tb.Milestones) > 0 && (tb.MetadataResult == nil || len(tb.MetadataResult.Milestones) == 0) { - resolveInput.Milestones = tb.Milestones - } - - metadataResult, err := api.RepoResolveMetadataIDs(client, baseRepo, resolveInput) - if err != nil { - return err - } - - if tb.MetadataResult == nil { - tb.MetadataResult = metadataResult - } else { - tb.MetadataResult.Merge(metadataResult) - } - - return nil -} - func AddMetadataToIssueParams(client *api.Client, baseRepo ghrepo.Interface, params map[string]interface{}, tb *IssueMetadataState, projectV1Support gh.ProjectsV1Support) error { if !tb.HasMetadata() { return nil } - if err := fillMetadata(client, baseRepo, tb, projectV1Support); err != nil { - return err + // Retrieve minimal information needed to resolve metadata if this was not previously cached from additional metadata survey. + if tb.MetadataResult == nil { + input := api.RepoMetadataInput{ + Reviewers: len(tb.Reviewers) > 0, + Assignees: len(tb.Assignees) > 0, + ActorAssignees: tb.ActorAssignees, + Labels: len(tb.Labels) > 0, + ProjectsV1: len(tb.ProjectTitles) > 0 && projectV1Support == gh.ProjectsV1Supported, + ProjectsV2: len(tb.ProjectTitles) > 0, + Milestones: len(tb.Milestones) > 0, + } + + metadataResult, err := api.RepoMetadata(client, baseRepo, input) + if err != nil { + return err + } + tb.MetadataResult = metadataResult } assigneeIDs, err := tb.MetadataResult.MembersToIDs(tb.Assignees) @@ -313,18 +285,26 @@ func (r *MeReplacer) ReplaceSlice(handles []string) ([]string, error) { return res, nil } -// CopilotReplacer resolves usages of `@copilot` to Copilot's login. -type CopilotReplacer struct{} +// CopilotReplacer resolves usages of `@copilot` to either Copilot's login or name. +// Login is generally needed for API calls; name is used when launching web browser. +type CopilotReplacer struct { + returnLogin bool +} -func NewCopilotReplacer() *CopilotReplacer { - return &CopilotReplacer{} +func NewCopilotReplacer(returnLogin bool) *CopilotReplacer { + return &CopilotReplacer{ + returnLogin: returnLogin, + } } func (r *CopilotReplacer) replace(handle string) string { - if strings.EqualFold(handle, "@copilot") { + if !strings.EqualFold(handle, "@copilot") { + return handle + } + if r.returnLogin { return api.CopilotActorLogin } - return handle + return api.CopilotActorName } // ReplaceSlice replaces usages of `@copilot` in a slice with Copilot's login. diff --git a/pkg/cmd/pr/shared/params_test.go b/pkg/cmd/pr/shared/params_test.go index 53eb6328fb6..3c95a9a5da9 100644 --- a/pkg/cmd/pr/shared/params_test.go +++ b/pkg/cmd/pr/shared/params_test.go @@ -192,17 +192,26 @@ func TestCopilotReplacer_ReplaceSlice(t *testing.T) { handles []string } tests := []struct { - name string - args args - want []string + name string + returnLogin bool + args args + want []string }{ { - name: "replaces @copilot with copilot-swe-agent", + name: "replaces @copilot with login", + returnLogin: true, args: args{ handles: []string{"monalisa", "@copilot", "hubot"}, }, want: []string{"monalisa", "copilot-swe-agent", "hubot"}, }, + { + name: "replaces @copilot with name", + args: args{ + handles: []string{"monalisa", "@copilot", "hubot"}, + }, + want: []string{"monalisa", "Copilot", "hubot"}, + }, { name: "handles no @copilot mentions", args: args{ @@ -211,14 +220,16 @@ func TestCopilotReplacer_ReplaceSlice(t *testing.T) { want: []string{"monalisa", "user", "hubot"}, }, { - name: "replaces multiple @copilot mentions", + name: "replaces multiple @copilot mentions", + returnLogin: true, args: args{ handles: []string{"@copilot", "user", "@copilot"}, }, want: []string{"copilot-swe-agent", "user", "copilot-swe-agent"}, }, { - name: "handles @copilot case-insensitively", + name: "handles @copilot case-insensitively", + returnLogin: true, args: args{ handles: []string{"@Copilot", "user", "@CoPiLoT"}, }, @@ -241,7 +252,7 @@ func TestCopilotReplacer_ReplaceSlice(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - r := NewCopilotReplacer() + r := NewCopilotReplacer(tt.returnLogin) got := r.ReplaceSlice(tt.args.handles) require.Equal(t, tt.want, got) }) diff --git a/pkg/cmd/pr/shared/state.go b/pkg/cmd/pr/shared/state.go index 7e7da436d03..b9f2c029372 100644 --- a/pkg/cmd/pr/shared/state.go +++ b/pkg/cmd/pr/shared/state.go @@ -18,7 +18,8 @@ const ( type IssueMetadataState struct { Type metadataStateType - Draft bool + Draft bool + ActorAssignees bool Body string Title string diff --git a/pkg/cmd/pr/shared/survey.go b/pkg/cmd/pr/shared/survey.go index b6c927a2d9b..af1fa871a6a 100644 --- a/pkg/cmd/pr/shared/survey.go +++ b/pkg/cmd/pr/shared/survey.go @@ -3,6 +3,7 @@ package shared import ( "errors" "fmt" + "slices" "strings" "github.com/cli/cli/v2/api" @@ -177,13 +178,15 @@ func MetadataSurvey(p Prompt, io *iostreams.IOStreams, baseRepo ghrepo.Interface state.Metadata = append(state.Metadata, extraFieldsOptions[i]) } + // Retrieve and process data for survey prompts based on the extra fields selected metadataInput := api.RepoMetadataInput{ - Reviewers: isChosen("Reviewers"), - Assignees: isChosen("Assignees"), - Labels: isChosen("Labels"), - ProjectsV1: isChosen("Projects") && projectsV1Support == gh.ProjectsV1Supported, - ProjectsV2: isChosen("Projects"), - Milestones: isChosen("Milestone"), + Reviewers: isChosen("Reviewers"), + Assignees: isChosen("Assignees"), + ActorAssignees: isChosen("Assignees") && state.ActorAssignees, + Labels: isChosen("Labels"), + ProjectsV1: isChosen("Projects") && projectsV1Support == gh.ProjectsV1Supported, + ProjectsV2: isChosen("Projects"), + Milestones: isChosen("Milestone"), } metadataResult, err := fetcher.RepoMetadataFetch(metadataInput) if err != nil { @@ -199,9 +202,28 @@ func MetadataSurvey(p Prompt, io *iostreams.IOStreams, baseRepo ghrepo.Interface for _, t := range metadataResult.Teams { reviewers = append(reviewers, fmt.Sprintf("%s/%s", baseRepo.RepoOwner(), t.Slug)) } + + // Populate the list of selectable assignees and their default selections. + // This logic maps the default assignees from `state` to the corresponding actors or users + // so that the correct display names are preselected in the prompt. var assignees []string - for _, u := range metadataResult.AssignableUsers { - assignees = append(assignees, u.DisplayName()) + var assigneesDefault []string + if state.ActorAssignees { + for _, u := range metadataResult.AssignableActors { + assignees = append(assignees, u.DisplayName()) + + if slices.Contains(state.Assignees, u.Login()) { + assigneesDefault = append(assigneesDefault, u.DisplayName()) + } + } + } else { + for _, u := range metadataResult.AssignableUsers { + assignees = append(assignees, u.DisplayName()) + + if slices.Contains(state.Assignees, u.Login()) { + assigneesDefault = append(assigneesDefault, u.DisplayName()) + } + } } var labels []string for _, l := range metadataResult.Labels { @@ -219,6 +241,7 @@ func MetadataSurvey(p Prompt, io *iostreams.IOStreams, baseRepo ghrepo.Interface milestones = append(milestones, m.Title) } + // Prompt user for additional metadata based on selected fields values := struct { Reviewers []string Assignees []string @@ -242,12 +265,20 @@ func MetadataSurvey(p Prompt, io *iostreams.IOStreams, baseRepo ghrepo.Interface } if isChosen("Assignees") { if len(assignees) > 0 { - selected, err := p.MultiSelect("Assignees", state.Assignees, assignees) + selected, err := p.MultiSelect("Assignees", assigneesDefault, assignees) if err != nil { return err } for _, i := range selected { - values.Assignees = append(values.Assignees, assignees[i]) + // Previously, this logic relied upon `assignees` being in `` or ` ()` form, + // however the inclusion of actors breaks this convention. + // Instead, we map the selected indexes to the source that populated `assignees` rather than + // relying on parsing the information out. + if state.ActorAssignees { + values.Assignees = append(values.Assignees, metadataResult.AssignableActors[i].Login()) + } else { + values.Assignees = append(values.Assignees, metadataResult.AssignableUsers[i].Login()) + } } } else { fmt.Fprintln(io.ErrOut, "warning: no assignable users") @@ -297,6 +328,7 @@ func MetadataSurvey(p Prompt, io *iostreams.IOStreams, baseRepo ghrepo.Interface } } + // Update issue / pull request metadata state if isChosen("Reviewers") { var logins []string for _, r := range values.Reviewers { @@ -306,12 +338,7 @@ func MetadataSurvey(p Prompt, io *iostreams.IOStreams, baseRepo ghrepo.Interface state.Reviewers = logins } if isChosen("Assignees") { - var logins []string - for _, a := range values.Assignees { - // Extract user login from display name - logins = append(logins, (strings.Split(a, " "))[0]) - } - state.Assignees = logins + state.Assignees = values.Assignees } if isChosen("Labels") { state.Labels = values.Labels