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

Merge pull request #11361 from cli/kw/cli-11360-regression-gh-pr-create-fails-on-self-hosted-runner-in-v2760-resource-not-accessible-by-integration-organizationteams

Browse source 
Fix: `gh pr create`, only fetch teams when reviewers contain a team
This commit is contained in:
Andy Feller 2025-07-23 13:09:34 -04:00 • committed by GitHub
commit 0d80c36dcc
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 180 additions and 17 deletions
Download patch file Download diff file Expand all files Collapse all files

3
api/queries_repo.go
View file

@ -919,6 +919,7 @@ type RepoMetadataInput struct {
Assignees bool
ActorAssignees bool
Reviewers bool
TeamReviewers bool
Labels bool
ProjectsV1 bool
ProjectsV2 bool
@ -964,7 +965,7 @@ func RepoMetadata(client *Client, repo ghrepo.Interface, input RepoMetadataInput
}
}
if input.Reviewers {
if input.Reviewers && input.TeamReviewers {
g.Go(func() error {
teams, err := OrganizationTeams(client, repo)
// TODO: better detection of non-org repos

50
api/queries_repo_test.go
View file

@ -42,12 +42,13 @@ func Test_RepoMetadata(t *testing.T) {
repo, _ := ghrepo.FromFullName("OWNER/REPO")
input := RepoMetadataInput{
Assignees: true,
Reviewers: true,
Labels: true,
ProjectsV1: true,
ProjectsV2: true,
Milestones: true,
Assignees: true,
Reviewers: true,
TeamReviewers: true,
Labels: true,
ProjectsV1: true,
ProjectsV2: true,
Milestones: true,
}
http.Register(
@ -213,6 +214,43 @@ func Test_RepoMetadata(t *testing.T) {
}
}
// Test that RepoMetadata only fetches teams if the input specifies it
func Test_RepoMetadata_TeamsAreConditionallyFetched(t *testing.T) {
http := &httpmock.Registry{}
client := newTestClient(http)
repo, _ := ghrepo.FromFullName("OWNER/REPO")
input := RepoMetadataInput{
Reviewers: true,
TeamReviewers: false, // Do not fetch teams
}
http.Register(
httpmock.GraphQL(`query RepositoryAssignableUsers\b`),
httpmock.StringResponse(`
{ "data": { "repository": { "assignableUsers": {
"nodes": [
{ "login": "hubot", "id": "HUBOTID" },
{ "login": "MonaLisa", "id": "MONAID" }
],
"pageInfo": { "hasNextPage": false }
} } } }
`))
http.Register(
httpmock.GraphQL(`query UserCurrent\b`),
httpmock.StringResponse(`
{ "data": { "viewer": { "login": "monalisa" } } }
`))
http.Exclude(
t,
httpmock.GraphQL(`query OrganizationTeamList\b`),
)
_, err := RepoMetadata(client, repo, input)
require.NoError(t, err)
}
func Test_ProjectNamesToPaths(t *testing.T) {
t.Run("when projectsV1 is supported, requests them", func(t *testing.T) {
http := &httpmock.Registry{}

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

@ -1314,14 +1314,6 @@ func Test_createRun(t *testing.T) {
"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(`
@ -1662,6 +1654,125 @@ func Test_createRun(t *testing.T) {
},
expectedOut: "https://github.com/OWNER/REPO/pull/12\n",
},
{
name: "if reviewer contains any team, fetch teams",
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(`query RepositoryAssignableUsers\b`),
httpmock.StringResponse(`
{ "data": { "repository": { "assignableUsers": {
"nodes": [
{ "login": "hubot", "id": "HUBOTID" },
{ "login": "MonaLisa", "id": "MONAID" }
],
"pageInfo": { "hasNextPage": false }
} } } }
`))
reg.Register(
httpmock.GraphQL(`query UserCurrent\b`),
httpmock.StringResponse(`
{ "data": { "viewer": { "login": "monalisa" } } }
`))
reg.Register(
httpmock.GraphQL(`query OrganizationTeamList\b`),
httpmock.StringResponse(`
{ "data": { "organization": { "teams": {
"nodes": [
{ "slug": "core", "id": "COREID" },
{ "slug": "robots", "id": "ROBOTID" }
],
"pageInfo": { "hasNextPage": false }
} } } }
`))
reg.Register(
httpmock.GraphQL(`mutation PullRequestCreateRequestReviews\b`),
httpmock.GraphQLMutation(`
{ "data": { "requestReviews": {
"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, true, inputs["union"])
}))
},
expectedOut: "https://github.com/OWNER/REPO/pull/12\n",
expectedErrOut: "",
},
{
name: "if reviewer does NOT contain any team, do NOT fetch teams",
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"}
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(`query RepositoryAssignableUsers\b`),
httpmock.StringResponse(`
{ "data": { "repository": { "assignableUsers": {
"nodes": [
{ "login": "hubot", "id": "HUBOTID" },
{ "login": "MonaLisa", "id": "MONAID" }
],
"pageInfo": { "hasNextPage": false }
} } } }
`))
reg.Register(
httpmock.GraphQL(`query UserCurrent\b`),
httpmock.StringResponse(`
{ "data": { "viewer": { "login": "monalisa" } } }
`))
reg.Exclude(
t,
httpmock.GraphQL(`query OrganizationTeamList\b`),
)
reg.Register(
httpmock.GraphQL(`mutation PullRequestCreateRequestReviews\b`),
httpmock.GraphQLMutation(`
{ "data": { "requestReviews": {
"clientMutationId": ""
} } }
`, func(inputs map[string]interface{}) {
assert.Equal(t, "NEWPULLID", inputs["pullRequestId"])
assert.Equal(t, []interface{}{"HUBOTID", "MONAID"}, inputs["userIds"])
assert.NotEqual(t, []interface{}{"COREID", "ROBOTID"}, inputs["teamIds"])
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) {

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

@ -429,7 +429,16 @@ func FieldsToEditSurvey(p EditPrompter, editable *Editable) error {
func FetchOptions(client *api.Client, repo ghrepo.Interface, editable *Editable) error {
input := api.RepoMetadataInput{
Reviewers: editable.Reviewers.Edited,
Reviewers: editable.Reviewers.Edited,
// TeamReviewers is always true if Reviewers is true because
// this is the existing `pr edit` behavior. This means
// always fetch teams.
// TODO: evaluate whether this can follow the same logic as
// `pr create` to conditionally fetch teams if a reviewer contains
// a slash.
// See https://github.com/cli/cli/blob/449920b40fc8a5015d1578ca10a301aa385a1914/pkg/cmd/pr/shared/params.go#L67-L71
// See https://github.com/cli/cli/issues/11360
TeamReviewers: editable.Reviewers.Edited,
Assignees: editable.Assignees.Edited,
ActorAssignees: editable.Assignees.ActorAssignees,
Labels: editable.Labels.Edited,

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

@ -3,6 +3,7 @@ package shared
import (
"fmt"
"net/url"
"slices"
"strings"
"github.com/cli/cli/v2/api"
@ -63,7 +64,10 @@ func AddMetadataToIssueParams(client *api.Client, baseRepo ghrepo.Interface, par
// 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,
Reviewers: len(tb.Reviewers) > 0,
TeamReviewers: len(tb.Reviewers) > 0 && slices.ContainsFunc(tb.Reviewers, func(r string) bool {
return strings.ContainsRune(r, '/')
}),
Assignees: len(tb.Assignees) > 0,
ActorAssignees: tb.ActorAssignees,
Labels: len(tb.Labels) > 0,