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

gh pr create: CCR and multiselectwithsearch

Browse source
This commit is contained in:
Kynan Ware 2026-01-30 15:50:56 -07:00
parent a9a0486c70
commit 9904f7d1b9
9 changed files with 731 additions and 251 deletions
Download patch file Download diff file Expand all files Collapse all files

170
api/queries_pr.go
View file

@ -615,29 +615,41 @@ func CreatePullRequest(client *Client, repo *Repository, params map[string]inter
}
}
// reviewers are requested in yet another additional mutation
reviewParams := make(map[string]interface{})
if ids, ok := params["userReviewerIds"]; ok && !isBlank(ids) {
reviewParams["userIds"] = ids
}
if ids, ok := params["teamReviewerIds"]; ok && !isBlank(ids) {
reviewParams["teamIds"] = ids
}
// Request reviewers using either login-based (github.com) or ID-based (GHES) mutation
userLogins, hasUserLogins := params["userReviewerLogins"].([]string)
teamSlugs, hasTeamSlugs := params["teamReviewerSlugs"].([]string)
//TODO: How much work to extract this into own method and use for create and edit?
if len(reviewParams) > 0 {
reviewQuery := `
if hasUserLogins || hasTeamSlugs {
// Use login-based mutation (RequestReviewsByLogin) for github.com
err := RequestReviewsByLogin(client, repo, pr.ID, userLogins, nil, teamSlugs, true)
if err != nil {
return pr, err
}
} else {
// Use ID-based mutation (requestReviews) for GHES compatibility
reviewParams := make(map[string]interface{})
if ids, ok := params["userReviewerIds"]; ok && !isBlank(ids) {
reviewParams["userIds"] = ids
}
if ids, ok := params["teamReviewerIds"]; ok && !isBlank(ids) {
reviewParams["teamIds"] = ids
}
//TODO: How much work to extract this into own method and use for create and edit?
if len(reviewParams) > 0 {
reviewQuery := `
mutation PullRequestCreateRequestReviews($input: RequestReviewsInput!) {
requestReviews(input: $input) { clientMutationId }
}`
reviewParams["pullRequestId"] = pr.ID
reviewParams["union"] = true
variables := map[string]interface{}{
"input": reviewParams,
}
err := client.GraphQL(repo.RepoHost(), reviewQuery, variables, &result)
if err != nil {
return pr, err
reviewParams["pullRequestId"] = pr.ID
reviewParams["union"] = true
variables := map[string]interface{}{
"input": reviewParams,
}
err := client.GraphQL(repo.RepoHost(), reviewQuery, variables, &result)
if err != nil {
return pr, err
}
}
}
@ -1109,6 +1121,126 @@ func SuggestedReviewerActors(client *Client, repo ghrepo.Interface, prID string,
return candidates, moreResults, nil
}
// SuggestedReviewerActorsForRepo fetches potential reviewers for a repository.
// Unlike SuggestedReviewerActors, this doesn't require an existing PR - used for gh pr create.
// It combines results from two sources using a cascading quota system:
// - repository collaborators (base quota: 5)
// - organization teams (base quota: 5 + unfilled from collaborators)
//
// This ensures we show up to 10 total candidates, with each source filling any
// unfilled quota from the previous source. Results are deduplicated.
// Returns the candidates, a MoreResults count, and an error.
func SuggestedReviewerActorsForRepo(client *Client, repo ghrepo.Interface, query string) ([]ReviewerCandidate, int, error) {
type responseData struct {
Repository struct {
// Check for Copilot availability by looking at any open PR's suggested reviewers
PullRequests struct {
Nodes []struct {
SuggestedActors struct {
Nodes []struct {
Reviewer struct {
TypeName string `graphql:"__typename"`
Bot struct {
Login string
} `graphql:"... on Bot"`
}
}
} `graphql:"suggestedReviewerActors(first: 10)"`
}
} `graphql:"pullRequests(first: 1, states: [OPEN])"`
Collaborators struct {
Nodes []struct {
Login string
Name string
}
} `graphql:"collaborators(first: 10, query: $query)"`
CollaboratorsTotalCount struct {
TotalCount int
} `graphql:"collaboratorsTotalCount: collaborators(first: 0)"`
} `graphql:"repository(owner: $owner, name: $name)"`
Organization struct {
Teams struct {
Nodes []struct {
Slug string
}
} `graphql:"teams(first: 10, query: $query)"`
TeamsTotalCount struct {
TotalCount int
} `graphql:"teamsTotalCount: teams(first: 0)"`
} `graphql:"organization(login: $owner)"`
}
variables := map[string]interface{}{
"query": githubv4.String(query),
"owner": githubv4.String(repo.RepoOwner()),
"name": githubv4.String(repo.RepoName()),
}
var result responseData
err := client.Query(repo.RepoHost(), "SuggestedReviewerActorsForRepo", &result, variables)
// Handle the case where the owner is not an organization - the query still returns
// partial data (repository), so we can continue processing.
if err != nil && !strings.Contains(err.Error(), errorResolvingOrganization) {
return nil, 0, err
}
// Build candidates using cascading quota logic
seen := make(map[string]bool)
var candidates []ReviewerCandidate
const baseQuota = 5
// Check for Copilot availability from open PR's suggested reviewers
for _, pr := range result.Repository.PullRequests.Nodes {
for _, actor := range pr.SuggestedActors.Nodes {
if actor.Reviewer.TypeName == "Bot" && actor.Reviewer.Bot.Login == CopilotReviewerLogin {
candidates = append(candidates, NewReviewerBot(CopilotReviewerLogin))
seen[CopilotReviewerLogin] = true
break
}
}
}
// Collaborators
collaboratorsAdded := 0
for _, c := range result.Repository.Collaborators.Nodes {
if collaboratorsAdded >= baseQuota {
break
}
if c.Login == "" {
continue
}
if !seen[c.Login] {
seen[c.Login] = true
candidates = append(candidates, NewReviewerUser(c.Login, c.Name))
collaboratorsAdded++
}
}
// Teams: quota = base + unfilled from collaborators
teamsQuota := baseQuota + (baseQuota - collaboratorsAdded)
teamsAdded := 0
ownerName := repo.RepoOwner()
for _, t := range result.Organization.Teams.Nodes {
if teamsAdded >= teamsQuota {
break
}
if t.Slug == "" {
continue
}
teamLogin := fmt.Sprintf("%s/%s", ownerName, t.Slug)
if !seen[teamLogin] {
seen[teamLogin] = true
candidates = append(candidates, NewReviewerTeam(ownerName, t.Slug))
teamsAdded++
}
}
// MoreResults uses unfiltered total counts (teams will be 0 for personal repos)
moreResults := result.Repository.CollaboratorsTotalCount.TotalCount + result.Organization.TeamsTotalCount.TotalCount
return candidates, moreResults, nil
}
func UpdatePullRequestBranch(client *Client, repo ghrepo.Interface, params githubv4.UpdatePullRequestBranchInput) error {
var mutation struct {
UpdatePullRequestBranch struct {

167
api/queries_pr_test.go
View file

@ -362,3 +362,170 @@ func TestSuggestedReviewerActors(t *testing.T) {
})
}
}
// mockReviewerResponseForRepo generates a GraphQL response for SuggestedReviewerActorsForRepo tests.
// It creates collaborators (c1, c2...) and teams (team1, team2...).
func mockReviewerResponseForRepo(collabs, teams, totalCollabs, totalTeams int) string {
return mockReviewerResponseForRepoWithCopilot(collabs, teams, totalCollabs, totalTeams, false)
}
// mockReviewerResponseForRepoWithCopilot generates a GraphQL response for SuggestedReviewerActorsForRepo tests.
// If copilotAvailable is true, includes Copilot in the first open PR's suggested reviewers.
func mockReviewerResponseForRepoWithCopilot(collabs, teams, totalCollabs, totalTeams int, copilotAvailable bool) string {
var collabNodes, teamNodes []string
for i := 1; i <= collabs; i++ {
collabNodes = append(collabNodes,
fmt.Sprintf(`{"login": "c%d", "name": "C%d"}`, i, i))
}
for i := 1; i <= teams; i++ {
teamNodes = append(teamNodes,
fmt.Sprintf(`{"slug": "team%d"}`, i))
}
pullRequestsJSON := `"pullRequests": {"nodes": []}`
if copilotAvailable {
pullRequestsJSON = `"pullRequests": {"nodes": [{"suggestedReviewerActors": {"nodes": [{"reviewer": {"__typename": "Bot", "login": "copilot-pull-request-reviewer"}}]}}]}`
}
return fmt.Sprintf(`{
"data": {
"repository": {
%s,
"collaborators": {"nodes": [%s]},
"collaboratorsTotalCount": {"totalCount": %d}
},
"organization": {
"teams": {"nodes": [%s]},
"teamsTotalCount": {"totalCount": %d}
}
}
}`, pullRequestsJSON, strings.Join(collabNodes, ","), totalCollabs,
strings.Join(teamNodes, ","), totalTeams)
}
func TestSuggestedReviewerActorsForRepo(t *testing.T) {
tests := []struct {
name string
httpStubs func(*httpmock.Registry)
expectedCount int
expectedLogins []string
expectedMore int
expectError bool
}{
{
name: "both sources plentiful - 5 each from cascading quota",
httpStubs: func(reg *httpmock.Registry) {
reg.Register(
httpmock.GraphQL(`query SuggestedReviewerActorsForRepo\b`),
httpmock.StringResponse(mockReviewerResponseForRepo(6, 6, 20, 10)))
},
expectedCount: 10,
expectedLogins: []string{"c1", "c2", "c3", "c4", "c5", "OWNER/team1", "OWNER/team2", "OWNER/team3", "OWNER/team4", "OWNER/team5"},
expectedMore: 30,
},
{
name: "few collaborators - teams fill gap",
httpStubs: func(reg *httpmock.Registry) {
reg.Register(
httpmock.GraphQL(`query SuggestedReviewerActorsForRepo\b`),
httpmock.StringResponse(mockReviewerResponseForRepo(2, 10, 2, 15)))
},
expectedCount: 10,
expectedLogins: []string{"c1", "c2", "OWNER/team1", "OWNER/team2", "OWNER/team3", "OWNER/team4", "OWNER/team5", "OWNER/team6", "OWNER/team7", "OWNER/team8"},
expectedMore: 17,
},
{
name: "no collaborators - teams only",
httpStubs: func(reg *httpmock.Registry) {
reg.Register(
httpmock.GraphQL(`query SuggestedReviewerActorsForRepo\b`),
httpmock.StringResponse(mockReviewerResponseForRepo(0, 10, 0, 20)))
},
expectedCount: 10,
expectedLogins: []string{"OWNER/team1", "OWNER/team2", "OWNER/team3", "OWNER/team4", "OWNER/team5", "OWNER/team6", "OWNER/team7", "OWNER/team8", "OWNER/team9", "OWNER/team10"},
expectedMore: 20,
},
{
name: "personal repo - no organization teams",
httpStubs: func(reg *httpmock.Registry) {
reg.Register(
httpmock.GraphQL(`query SuggestedReviewerActorsForRepo\b`),
httpmock.StringResponse(`{
"data": {
"repository": {
"pullRequests": {"nodes": []},
"collaborators": {"nodes": [{"login": "c1", "name": "C1"}]},
"collaboratorsTotalCount": {"totalCount": 3}
},
"organization": null
},
"errors": [{"message": "Could not resolve to an Organization with the login of 'OWNER'."}]
}`))
},
expectedCount: 1,
expectedLogins: []string{"c1"},
expectedMore: 3,
},
{
name: "empty repo",
httpStubs: func(reg *httpmock.Registry) {
reg.Register(
httpmock.GraphQL(`query SuggestedReviewerActorsForRepo\b`),
httpmock.StringResponse(mockReviewerResponseForRepo(0, 0, 0, 0)))
},
expectedCount: 0,
expectedLogins: []string{},
expectedMore: 0,
},
{
name: "copilot available - prepended to candidates",
httpStubs: func(reg *httpmock.Registry) {
reg.Register(
httpmock.GraphQL(`query SuggestedReviewerActorsForRepo\b`),
httpmock.StringResponse(mockReviewerResponseForRepoWithCopilot(3, 2, 5, 5, true)))
},
expectedCount: 6,
expectedLogins: []string{"copilot-pull-request-reviewer", "c1", "c2", "c3", "OWNER/team1", "OWNER/team2"},
expectedMore: 10,
},
{
name: "copilot not available - not included",
httpStubs: func(reg *httpmock.Registry) {
reg.Register(
httpmock.GraphQL(`query SuggestedReviewerActorsForRepo\b`),
httpmock.StringResponse(mockReviewerResponseForRepoWithCopilot(3, 2, 5, 5, false)))
},
expectedCount: 5,
expectedLogins: []string{"c1", "c2", "c3", "OWNER/team1", "OWNER/team2"},
expectedMore: 10,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
reg := &httpmock.Registry{}
if tt.httpStubs != nil {
tt.httpStubs(reg)
}
client := newTestClient(reg)
repo, _ := ghrepo.FromFullName("OWNER/REPO")
candidates, moreResults, err := SuggestedReviewerActorsForRepo(client, repo, "")
if tt.expectError {
assert.Error(t, err)
return
}
assert.NoError(t, err)
assert.Equal(t, tt.expectedCount, len(candidates), "candidate count mismatch")
assert.Equal(t, tt.expectedMore, moreResults, "moreResults mismatch")
logins := make([]string, len(candidates))
for i, c := range candidates {
logins[i] = c.Login()
}
assert.Equal(t, tt.expectedLogins, logins)
})
}
}

2
pkg/cmd/issue/create/create.go
View file

@ -312,7 +312,7 @@ func createRun(opts *CreateOptions) (err error) {
Repo: baseRepo,
State: &tb,
}
err = prShared.MetadataSurvey(opts.Prompter, opts.IO, baseRepo, fetcher, &tb, projectsV1Support)
err = prShared.MetadataSurvey(opts.Prompter, opts.IO, baseRepo, fetcher, &tb, projectsV1Support, nil)
if err != nil {
return
}

28
pkg/cmd/pr/create/create.go
View file

@ -21,6 +21,7 @@ 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"
"github.com/cli/cli/v2/pkg/cmd/pr/shared"
"github.com/cli/cli/v2/pkg/cmdutil"
@ -397,11 +398,36 @@ func createRun(opts *CreateOptions) error {
client := ctx.Client
// Detect ActorIsAssignable feature to determine if we can use search-based
// reviewer selection (github.com) or need to use traditional ID-based selection (GHES)
issueFeatures, _ := opts.Detector.IssueFeatures()
var reviewerSearchFunc func(string) prompter.MultiSelectSearchResult
if issueFeatures.ActorIsAssignable {
// Create search function for reviewer selection using login-based API
reviewerSearchFunc = func(query string) prompter.MultiSelectSearchResult {
candidates, moreResults, err := api.SuggestedReviewerActorsForRepo(client, ctx.PRRefs.BaseRepo(), query)
if err != nil {
return prompter.MultiSelectSearchResult{Err: err}
}
keys := make([]string, len(candidates))
labels := make([]string, len(candidates))
for i, c := range candidates {
keys[i] = c.Login()
labels[i] = c.DisplayName()
}
return prompter.MultiSelectSearchResult{Keys: keys, Labels: labels, MoreResults: moreResults}
}
}
state, err := NewIssueState(*ctx, *opts)
if err != nil {
return err
}
if issueFeatures.ActorIsAssignable {
state.ActorReviewers = true
}
var openURL string
if opts.WebMode {
@ -568,7 +594,7 @@ func createRun(opts *CreateOptions) error {
Repo: ctx.PRRefs.BaseRepo(),
State: state,
}
err = shared.MetadataSurvey(opts.Prompter, opts.IO, ctx.PRRefs.BaseRepo(), fetcher, state, projectsV1Support)
err = shared.MetadataSurvey(opts.Prompter, opts.IO, ctx.PRRefs.BaseRepo(), fetcher, state, projectsV1Support, reviewerSearchFunc)
if err != nil {
return err
}

548
pkg/cmd/pr/create/create_test.go
View file

@ -434,92 +434,6 @@ func Test_createRun(t *testing.T) {
},
expectedErrOut: "",
},
{
name: "dry-run-nontty-with-all-opts",
tty: false,
setup: func(opts *CreateOptions, t *testing.T) func() {
opts.TitleProvided = true
opts.BodyProvided = true
opts.Title = "TITLE"
opts.Body = "BODY"
opts.BaseBranch = "trunk"
opts.HeadBranch = "feature"
opts.Assignees = []string{"monalisa"}
opts.Labels = []string{"bug", "todo"}
opts.Projects = []string{"roadmap"}
opts.Reviewers = []string{"hubot", "monalisa", "/core", "/robots"}
opts.Milestone = "big one.oh"
opts.DryRun = true
return func() {}
},
httpStubs: func(reg *httpmock.Registry, t *testing.T) {
reg.Register(
httpmock.GraphQL(`query UserCurrent\b`),
httpmock.StringResponse(`{"data": {"viewer": {"login": "OWNER"} } }`))
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(`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`),
httpmock.StringResponse(`
{ "data": { "repository": { "milestones": {
"nodes": [
{ "title": "GA", "id": "GAID" },
{ "title": "Big One.oh", "id": "BIGONEID" }
],
"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{
"Would have created a Pull Request with:",
`title: TITLE`,
`draft: false`,
`base: trunk`,
`head: feature`,
`labels: bug, todo`,
`reviewers: hubot, monalisa, /core, /robots`,
`assignees: monalisa`,
`milestones: big one.oh`,
`projects: roadmap`,
`maintainerCanModify: false`,
`body:`,
`BODY`,
``,
},
expectedErrOut: "",
},
{
name: "dry-run-tty-with-default-base",
tty: true,
@ -549,98 +463,6 @@ func Test_createRun(t *testing.T) {
Dry Running pull request for feature into master in OWNER/REPO
`),
},
{
name: "dry-run-tty-with-all-opts",
tty: true,
setup: func(opts *CreateOptions, t *testing.T) func() {
opts.TitleProvided = true
opts.BodyProvided = true
opts.Title = "TITLE"
opts.Body = "BODY"
opts.BaseBranch = "trunk"
opts.HeadBranch = "feature"
opts.Assignees = []string{"monalisa"}
opts.Labels = []string{"bug", "todo"}
opts.Projects = []string{"roadmap"}
opts.Reviewers = []string{"hubot", "monalisa", "/core", "/robots"}
opts.Milestone = "big one.oh"
opts.DryRun = true
return func() {}
},
httpStubs: func(reg *httpmock.Registry, t *testing.T) {
reg.Register(
httpmock.GraphQL(`query UserCurrent\b`),
httpmock.StringResponse(`{"data": {"viewer": {"login": "OWNER"} } }`))
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(`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`),
httpmock.StringResponse(`
{ "data": { "repository": { "milestones": {
"nodes": [
{ "title": "GA", "id": "GAID" },
{ "title": "Big One.oh", "id": "BIGONEID" }
],
"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{
`Would have created a Pull Request with:`,
`Title: TITLE`,
`Draft: false`,
`Base: trunk`,
`Head: feature`,
`Labels: bug, todo`,
`Reviewers: hubot, monalisa, /core, /robots`,
`Assignees: monalisa`,
`Milestones: big one.oh`,
`Projects: roadmap`,
`MaintainerCanModify: false`,
`Body:`,
``,
` BODY `,
``,
``,
},
expectedErrOut: heredoc.Doc(`
Dry Running pull request for feature into trunk in OWNER/REPO
`),
},
{
@ -1092,9 +914,6 @@ func Test_createRun(t *testing.T) {
return func() {}
},
httpStubs: func(reg *httpmock.Registry, t *testing.T) {
reg.Register(
httpmock.GraphQL(`query UserCurrent\b`),
httpmock.StringResponse(`{"data": {"viewer": {"login": "OWNER"} } }`))
reg.Register(
httpmock.GraphQL(`query RepositoryAssignableUsers\b`),
httpmock.StringResponse(`
@ -1128,17 +947,6 @@ 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`),
@ -1171,15 +979,15 @@ func Test_createRun(t *testing.T) {
assert.Equal(t, "BIGONEID", inputs["milestoneId"])
}))
reg.Register(
httpmock.GraphQL(`mutation PullRequestCreateRequestReviews\b`),
httpmock.GraphQL(`mutation RequestReviewsByLogin\b`),
httpmock.GraphQLMutation(`
{ "data": { "requestReviews": {
{ "data": { "requestReviewsByLogin": {
"clientMutationId": ""
} } }
`, func(inputs map[string]interface{}) {
assert.Equal(t, "NEWPULLID", inputs["pullRequestId"])
assert.Equal(t, []interface{}{"HUBOTID", "MONAID"}, inputs["userIds"])
assert.Equal(t, []interface{}{"COREID", "ROBOTID"}, inputs["teamIds"])
assert.Equal(t, []interface{}{"hubot", "monalisa"}, inputs["userLogins"])
assert.Equal(t, []interface{}{"core", "robots"}, inputs["teamSlugs"])
assert.Equal(t, true, inputs["union"])
}))
},
@ -1679,6 +1487,327 @@ func Test_createRun(t *testing.T) {
},
expectedOut: "https://github.com/OWNER/REPO/pull/12\n",
},
{
name: "request reviewers by login",
setup: func(opts *CreateOptions, t *testing.T) func() {
opts.TitleProvided = true
opts.BodyProvided = true
opts.Title = "my title"
opts.Body = "my body"
opts.Reviewers = []string{"hubot", "monalisa", "org/core", "org/robots"}
opts.HeadBranch = "feature"
return func() {}
},
httpStubs: func(reg *httpmock.Registry, t *testing.T) {
reg.Register(
httpmock.GraphQL(`mutation PullRequestCreate\b`),
httpmock.GraphQLMutation(`
{ "data": { "createPullRequest": { "pullRequest": {
"URL": "https://github.com/OWNER/REPO/pull/12",
"id": "NEWPULLID"
} } } }`,
func(input map[string]interface{}) {}))
reg.Register(
httpmock.GraphQL(`mutation RequestReviewsByLogin\b`),
httpmock.GraphQLMutation(`
{ "data": { "requestReviewsByLogin": {
"clientMutationId": ""
} } }
`, func(inputs map[string]interface{}) {
assert.Equal(t, "NEWPULLID", inputs["pullRequestId"])
assert.Equal(t, []interface{}{"hubot", "monalisa"}, inputs["userLogins"])
assert.Equal(t, []interface{}{"core", "robots"}, inputs["teamSlugs"])
assert.Equal(t, true, inputs["union"])
}))
},
expectedOut: "https://github.com/OWNER/REPO/pull/12\n",
expectedErrOut: "",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
branch := "feature"
reg := &httpmock.Registry{}
reg.StubRepoInfoResponse("OWNER", "REPO", "master")
defer reg.Verify(t)
if tt.httpStubs != nil {
tt.httpStubs(reg, t)
}
pm := &prompter.PrompterMock{}
if tt.promptStubs != nil {
tt.promptStubs(pm)
}
cs, cmdTeardown := run.Stub()
defer cmdTeardown(t)
if !tt.customBranchConfig {
cs.Register(`git config --get-regexp \^branch\\\..+\\\.\(remote\|merge\|pushremote\|gh-merge-base\)\$`, 0, "")
}
if tt.cmdStubs != nil {
tt.cmdStubs(cs)
}
opts := CreateOptions{}
opts.Prompter = pm
ios, _, stdout, stderr := iostreams.Test()
ios.SetStdoutTTY(tt.tty)
ios.SetStdinTTY(tt.tty)
ios.SetStderrTTY(tt.tty)
browser := &browser.Stub{}
opts.IO = ios
opts.Browser = browser
opts.HttpClient = func() (*http.Client, error) {
return &http.Client{Transport: reg}, nil
}
opts.Config = func() (gh.Config, error) {
return config.NewBlankConfig(), nil
}
opts.Remotes = func() (context.Remotes, error) {
return context.Remotes{
{
Remote: &git.Remote{
Name: "origin",
Resolved: "base",
},
Repo: ghrepo.New("OWNER", "REPO"),
},
}, nil
}
opts.Branch = func() (string, error) {
return branch, nil
}
opts.Finder = shared.NewMockFinder(branch, nil, nil)
opts.GitClient = &git.Client{
GhPath: "some/path/gh",
GitPath: "some/path/git",
}
cleanSetup := func() {}
if tt.setup != nil {
cleanSetup = tt.setup(&opts, t)
}
defer cleanSetup()
// All tests in this function use github.com behavior
opts.Detector = &fd.EnabledDetectorMock{}
if opts.HeadBranch == "" {
cs.Register(`git status --porcelain`, 0, "")
}
err := createRun(&opts)
output := &test.CmdOut{
OutBuf: stdout,
ErrBuf: stderr,
BrowsedURL: browser.BrowsedURL(),
}
if tt.wantErr != "" {
assert.EqualError(t, err, tt.wantErr)
} else {
assert.NoError(t, err)
if tt.expectedOut != "" {
assert.Equal(t, tt.expectedOut, output.String())
}
if len(tt.expectedOutputs) > 0 {
assert.Equal(t, tt.expectedOutputs, strings.Split(output.String(), "\n"))
}
assert.Equal(t, tt.expectedErrOut, output.Stderr())
assert.Equal(t, tt.expectedBrowse, output.BrowsedURL)
}
})
}
}
func Test_createRun_GHES(t *testing.T) {
tests := []struct {
name string
setup func(*CreateOptions, *testing.T) func()
cmdStubs func(*run.CommandStubber)
promptStubs func(*prompter.PrompterMock)
httpStubs func(*httpmock.Registry, *testing.T)
expectedOutputs []string
expectedOut string
expectedErrOut string
tty bool
customBranchConfig bool
}{
{
name: "dry-run-nontty-with-all-opts",
tty: false,
setup: func(opts *CreateOptions, t *testing.T) func() {
opts.TitleProvided = true
opts.BodyProvided = true
opts.Title = "TITLE"
opts.Body = "BODY"
opts.BaseBranch = "trunk"
opts.HeadBranch = "feature"
opts.Assignees = []string{"monalisa"}
opts.Labels = []string{"bug", "todo"}
opts.Reviewers = []string{"hubot", "monalisa", "/core", "/robots"}
opts.Milestone = "big one.oh"
opts.DryRun = true
return func() {}
},
httpStubs: func(reg *httpmock.Registry, t *testing.T) {
reg.Register(
httpmock.GraphQL(`query UserCurrent\b`),
httpmock.StringResponse(`{"data": {"viewer": {"login": "OWNER"} } }`))
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(`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`),
httpmock.StringResponse(`
{ "data": { "repository": { "milestones": {
"nodes": [
{ "title": "GA", "id": "GAID" },
{ "title": "Big One.oh", "id": "BIGONEID" }
],
"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 }
} } } }
`))
},
expectedOutputs: []string{
"Would have created a Pull Request with:",
`title: TITLE`,
`draft: false`,
`base: trunk`,
`head: feature`,
`labels: bug, todo`,
`reviewers: hubot, monalisa, /core, /robots`,
`assignees: monalisa`,
`milestones: big one.oh`,
`maintainerCanModify: false`,
`body:`,
`BODY`,
``,
},
expectedErrOut: "",
},
{
name: "dry-run-tty-with-all-opts",
tty: true,
setup: func(opts *CreateOptions, t *testing.T) func() {
opts.TitleProvided = true
opts.BodyProvided = true
opts.Title = "TITLE"
opts.Body = "BODY"
opts.BaseBranch = "trunk"
opts.HeadBranch = "feature"
opts.Assignees = []string{"monalisa"}
opts.Labels = []string{"bug", "todo"}
opts.Reviewers = []string{"hubot", "monalisa", "/core", "/robots"}
opts.Milestone = "big one.oh"
opts.DryRun = true
return func() {}
},
httpStubs: func(reg *httpmock.Registry, t *testing.T) {
reg.Register(
httpmock.GraphQL(`query UserCurrent\b`),
httpmock.StringResponse(`{"data": {"viewer": {"login": "OWNER"} } }`))
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(`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`),
httpmock.StringResponse(`
{ "data": { "repository": { "milestones": {
"nodes": [
{ "title": "GA", "id": "GAID" },
{ "title": "Big One.oh", "id": "BIGONEID" }
],
"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 }
} } } }
`))
},
expectedOutputs: []string{
`Would have created a Pull Request with:`,
`Title: TITLE`,
`Draft: false`,
`Base: trunk`,
`Head: feature`,
`Labels: bug, todo`,
`Reviewers: hubot, monalisa, /core, /robots`,
`Assignees: monalisa`,
`Milestones: big one.oh`,
`MaintainerCanModify: false`,
`Body:`,
``,
` BODY `,
``,
``,
},
expectedErrOut: heredoc.Doc(`
Dry Running pull request for feature into trunk in OWNER/REPO
`),
},
{
name: "fetch org teams non-interactively if reviewer contains any team",
setup: func(opts *CreateOptions, t *testing.T) func() {
@ -1951,11 +2080,10 @@ func Test_createRun(t *testing.T) {
}
opts := CreateOptions{}
opts.Detector = &fd.EnabledDetectorMock{}
opts.Detector = &fd.DisabledDetectorMock{}
opts.Prompter = pm
ios, _, stdout, stderr := iostreams.Test()
// TODO do i need to bother with this
ios.SetStdoutTTY(tt.tty)
ios.SetStdinTTY(tt.tty)
ios.SetStderrTTY(tt.tty)
@ -1999,23 +2127,17 @@ func Test_createRun(t *testing.T) {
err := createRun(&opts)
output := &test.CmdOut{
OutBuf: stdout,
ErrBuf: stderr,
BrowsedURL: browser.BrowsedURL(),
OutBuf: stdout,
ErrBuf: stderr,
}
if tt.wantErr != "" {
assert.EqualError(t, err, tt.wantErr)
} else {
assert.NoError(t, err)
if tt.expectedOut != "" {
assert.Equal(t, tt.expectedOut, output.String())
}
if len(tt.expectedOutputs) > 0 {
assert.Equal(t, tt.expectedOutputs, strings.Split(output.String(), "\n"))
}
assert.Equal(t, tt.expectedErrOut, output.Stderr())
assert.Equal(t, tt.expectedBrowse, output.BrowsedURL)
assert.NoError(t, err)
if tt.expectedOut != "" {
assert.Equal(t, tt.expectedOut, output.String())
}
if len(tt.expectedOutputs) > 0 {
assert.Equal(t, tt.expectedOutputs, strings.Split(output.String(), "\n"))
}
assert.Equal(t, tt.expectedErrOut, output.Stderr())
})
}
}

42
pkg/cmd/pr/shared/params.go
View file

@ -61,11 +61,14 @@ func AddMetadataToIssueParams(client *api.Client, baseRepo ghrepo.Interface, par
return nil
}
// When ActorReviewers is true, we use login-based mutation and don't need to resolve reviewer IDs.
needReviewerIDs := len(tb.Reviewers) > 0 && !tb.ActorReviewers
// 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,
TeamReviewers: len(tb.Reviewers) > 0 && slices.ContainsFunc(tb.Reviewers, func(r string) bool {
Reviewers: needReviewerIDs,
TeamReviewers: needReviewerIDs && slices.ContainsFunc(tb.Reviewers, func(r string) bool {
return strings.ContainsRune(r, '/')
}),
Assignees: len(tb.Assignees) > 0,
@ -124,17 +127,34 @@ func AddMetadataToIssueParams(client *api.Client, baseRepo ghrepo.Interface, par
}
}
userReviewerIDs, err := tb.MetadataResult.MembersToIDs(userReviewers)
if err != nil {
return fmt.Errorf("could not request reviewer: %w", err)
}
params["userReviewerIds"] = userReviewerIDs
// When ActorReviewers is true (github.com), pass logins directly for use with
// RequestReviewsByLogin mutation. Otherwise, resolve to IDs for GHES compatibility.
if tb.ActorReviewers {
params["userReviewerLogins"] = userReviewers
// Extract team slugs from org/slug format
teamSlugs := make([]string, len(teamReviewers))
for i, t := range teamReviewers {
parts := strings.SplitN(t, "/", 2)
if len(parts) == 2 {
teamSlugs[i] = parts[1]
} else {
teamSlugs[i] = t
}
}
params["teamReviewerSlugs"] = teamSlugs
} else {
userReviewerIDs, err := tb.MetadataResult.MembersToIDs(userReviewers)
if err != nil {
return fmt.Errorf("could not request reviewer: %w", err)
}
params["userReviewerIds"] = userReviewerIDs
teamReviewerIDs, err := tb.MetadataResult.TeamsToIDs(teamReviewers)
if err != nil {
return fmt.Errorf("could not request reviewer: %w", err)
teamReviewerIDs, err := tb.MetadataResult.TeamsToIDs(teamReviewers)
if err != nil {
return fmt.Errorf("could not request reviewer: %w", err)
}
params["teamReviewerIds"] = teamReviewerIDs
}
params["teamReviewerIds"] = teamReviewerIDs
return nil
}

1
pkg/cmd/pr/shared/state.go
View file

@ -20,6 +20,7 @@ type IssueMetadataState struct {
Draft bool
ActorAssignees bool
ActorReviewers bool
Body string
Title string

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

@ -154,7 +154,7 @@ type RepoMetadataFetcher interface {
RepoMetadataFetch(api.RepoMetadataInput) (*api.RepoMetadataResult, error)
}
func MetadataSurvey(p Prompt, io *iostreams.IOStreams, baseRepo ghrepo.Interface, fetcher RepoMetadataFetcher, state *IssueMetadataState, projectsV1Support gh.ProjectsV1Support) error {
func MetadataSurvey(p Prompt, io *iostreams.IOStreams, baseRepo ghrepo.Interface, fetcher RepoMetadataFetcher, state *IssueMetadataState, projectsV1Support gh.ProjectsV1Support, reviewerSearchFunc func(string) prompter.MultiSelectSearchResult) error {
isChosen := func(m string) bool {
for _, c := range state.Metadata {
if m == c {
@ -254,7 +254,19 @@ func MetadataSurvey(p Prompt, io *iostreams.IOStreams, baseRepo ghrepo.Interface
}{}
if isChosen("Reviewers") {
if len(reviewers) > 0 {
if reviewerSearchFunc != nil {
// Use search-based selection (github.com with ActorIsAssignable)
selectedReviewers, err := p.MultiSelectWithSearch(
"Reviewers",
"Search reviewers",
state.Reviewers,
[]string{},
reviewerSearchFunc)
if err != nil {
return err
}
values.Reviewers = selectedReviewers
} else if len(reviewers) > 0 {
selected, err := p.MultiSelect("Reviewers", state.Reviewers, reviewers)
if err != nil {
return err

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

@ -71,7 +71,7 @@ func TestMetadataSurvey_selectAll(t *testing.T) {
Assignees: []string{"hubot"},
Type: PRMetadata,
}
err := MetadataSurvey(pm, ios, repo, fetcher, state, gh.ProjectsV1Supported)
err := MetadataSurvey(pm, ios, repo, fetcher, state, gh.ProjectsV1Supported, nil)
assert.NoError(t, err)
assert.Equal(t, "", stdout.String())
@ -117,7 +117,7 @@ func TestMetadataSurvey_keepExisting(t *testing.T) {
Assignees: []string{"hubot"},
}
err := MetadataSurvey(pm, ios, repo, fetcher, state, gh.ProjectsV1Supported)
err := MetadataSurvey(pm, ios, repo, fetcher, state, gh.ProjectsV1Supported, nil)
assert.NoError(t, err)
assert.Equal(t, "", stdout.String())
@ -146,7 +146,7 @@ func TestMetadataSurveyProjectV1Deprecation(t *testing.T) {
return []int{0}, nil
})
err := MetadataSurvey(pm, ios, repo, fetcher, &IssueMetadataState{}, gh.ProjectsV1Supported)
err := MetadataSurvey(pm, ios, repo, fetcher, &IssueMetadataState{}, gh.ProjectsV1Supported, nil)
require.ErrorContains(t, err, "expected test error")
require.True(t, fetcher.projectsV1Requested, "expected projectsV1 to be requested")
@ -167,7 +167,7 @@ func TestMetadataSurveyProjectV1Deprecation(t *testing.T) {
return []int{0}, nil
})
err := MetadataSurvey(pm, ios, repo, fetcher, &IssueMetadataState{}, gh.ProjectsV1Unsupported)
err := MetadataSurvey(pm, ios, repo, fetcher, &IssueMetadataState{}, gh.ProjectsV1Unsupported, nil)
require.ErrorContains(t, err, "expected test error")
require.False(t, fetcher.projectsV1Requested, "expected projectsV1 not to be requested")