Skip to content

Commit 40512a8

Browse files
authored
Merge pull request cli#11987 from cli/kw/fix-projectsv1-fetching-in-pr-edit
fix `gh pr edit`: do not fetch V1 projects on unsupported GitHub hosts
2 parents e627f01 + 31398fe commit 40512a8

5 files changed

Lines changed: 79 additions & 10 deletions

File tree

pkg/cmd/issue/edit/edit.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ type EditOptions struct {
3030
DetermineEditor func() (string, error)
3131
FieldsToEditSurvey func(prShared.EditPrompter, *prShared.Editable) error
3232
EditFieldsSurvey func(prShared.EditPrompter, *prShared.Editable, string) error
33-
FetchOptions func(*api.Client, ghrepo.Interface, *prShared.Editable) error
33+
FetchOptions func(*api.Client, ghrepo.Interface, *prShared.Editable, gh.ProjectsV1Support) error
3434

3535
IssueNumbers []int
3636
Interactive bool
@@ -248,7 +248,7 @@ func editRun(opts *EditOptions) error {
248248
// Fetch editable shared fields once for all issues.
249249
apiClient := api.NewClientFromHTTP(httpClient)
250250
opts.IO.StartProgressIndicatorWithLabel("Fetching repository information")
251-
err = opts.FetchOptions(apiClient, baseRepo, &editable)
251+
err = opts.FetchOptions(apiClient, baseRepo, &editable, opts.Detector.ProjectsV1())
252252
opts.IO.StopProgressIndicator()
253253
if err != nil {
254254
return err

pkg/cmd/issue/edit/edit_test.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -347,6 +347,7 @@ func Test_editRun(t *testing.T) {
347347
{
348348
name: "non-interactive",
349349
input: &EditOptions{
350+
Detector: &fd.EnabledDetectorMock{},
350351
IssueNumbers: []int{123},
351352
Interactive: false,
352353
Editable: prShared.Editable{
@@ -403,6 +404,7 @@ func Test_editRun(t *testing.T) {
403404
{
404405
name: "non-interactive multiple issues",
405406
input: &EditOptions{
407+
Detector: &fd.EnabledDetectorMock{},
406408
IssueNumbers: []int{456, 123},
407409
Interactive: false,
408410
Editable: prShared.Editable{
@@ -457,6 +459,7 @@ func Test_editRun(t *testing.T) {
457459
{
458460
name: "non-interactive multiple issues with fetch failures",
459461
input: &EditOptions{
462+
Detector: &fd.EnabledDetectorMock{},
460463
IssueNumbers: []int{123, 9999},
461464
Interactive: false,
462465
Editable: prShared.Editable{
@@ -504,6 +507,7 @@ func Test_editRun(t *testing.T) {
504507
{
505508
name: "non-interactive multiple issues with update failures",
506509
input: &EditOptions{
510+
Detector: &fd.EnabledDetectorMock{},
507511
IssueNumbers: []int{123, 456},
508512
Interactive: false,
509513
Editable: prShared.Editable{
@@ -584,6 +588,7 @@ func Test_editRun(t *testing.T) {
584588
{
585589
name: "interactive",
586590
input: &EditOptions{
591+
Detector: &fd.EnabledDetectorMock{},
587592
IssueNumbers: []int{123},
588593
Interactive: true,
589594
FieldsToEditSurvey: func(p prShared.EditPrompter, eo *prShared.Editable) error {
@@ -623,6 +628,7 @@ func Test_editRun(t *testing.T) {
623628
{
624629
name: "interactive prompts with actor assignee display names when actors available",
625630
input: &EditOptions{
631+
Detector: &fd.EnabledDetectorMock{},
626632
IssueNumbers: []int{123},
627633
Interactive: true,
628634
FieldsToEditSurvey: func(p prShared.EditPrompter, eo *prShared.Editable) error {

pkg/cmd/pr/edit/edit.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -293,7 +293,7 @@ func editRun(opts *EditOptions) error {
293293
apiClient := api.NewClientFromHTTP(httpClient)
294294

295295
opts.IO.StartProgressIndicator()
296-
err = opts.Fetcher.EditableOptionsFetch(apiClient, repo, &editable)
296+
err = opts.Fetcher.EditableOptionsFetch(apiClient, repo, &editable, opts.Detector.ProjectsV1())
297297
opts.IO.StopProgressIndicator()
298298
if err != nil {
299299
return err
@@ -398,13 +398,13 @@ func (s surveyor) EditFields(editable *shared.Editable, editorCmd string) error
398398
}
399399

400400
type EditableOptionsFetcher interface {
401-
EditableOptionsFetch(*api.Client, ghrepo.Interface, *shared.Editable) error
401+
EditableOptionsFetch(*api.Client, ghrepo.Interface, *shared.Editable, gh.ProjectsV1Support) error
402402
}
403403

404404
type fetcher struct{}
405405

406-
func (f fetcher) EditableOptionsFetch(client *api.Client, repo ghrepo.Interface, opts *shared.Editable) error {
407-
return shared.FetchOptions(client, repo, opts)
406+
func (f fetcher) EditableOptionsFetch(client *api.Client, repo ghrepo.Interface, opts *shared.Editable, projectsV1Support gh.ProjectsV1Support) error {
407+
return shared.FetchOptions(client, repo, opts, projectsV1Support)
408408
}
409409

410410
type EditorRetriever interface {

pkg/cmd/pr/edit/edit_test.go

Lines changed: 64 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010

1111
"github.com/cli/cli/v2/api"
1212
fd "github.com/cli/cli/v2/internal/featuredetection"
13+
"github.com/cli/cli/v2/internal/gh"
1314
"github.com/cli/cli/v2/internal/ghrepo"
1415
shared "github.com/cli/cli/v2/pkg/cmd/pr/shared"
1516
"github.com/cli/cli/v2/pkg/cmdutil"
@@ -894,6 +895,67 @@ func Test_editRun(t *testing.T) {
894895
},
895896
stdout: "https://github.com/OWNER/REPO/pull/123\n",
896897
},
898+
{
899+
name: "non-interactive projects v1 unsupported doesn't fetch v1 metadata",
900+
input: &EditOptions{
901+
Detector: &fd.DisabledDetectorMock{},
902+
SelectorArg: "123",
903+
Finder: shared.NewMockFinder("123", &api.PullRequest{
904+
URL: "https://github.com/OWNER/REPO/pull/123",
905+
}, ghrepo.New("OWNER", "REPO")),
906+
Interactive: false,
907+
Editable: shared.Editable{
908+
Projects: shared.EditableProjects{
909+
EditableSlice: shared.EditableSlice{
910+
Add: []string{"CleanupV2"},
911+
Remove: []string{"RoadmapV2"},
912+
Edited: true,
913+
},
914+
},
915+
},
916+
Fetcher: testFetcher{},
917+
},
918+
httpStubs: func(t *testing.T, reg *httpmock.Registry) {
919+
// Ensure v1 project queries are NOT made.
920+
reg.Exclude(t, httpmock.GraphQL(`query RepositoryProjectList\b`))
921+
reg.Exclude(t, httpmock.GraphQL(`query OrganizationProjectList\b`))
922+
// Provide only v2 project metadata queries.
923+
reg.Register(
924+
httpmock.GraphQL(`query RepositoryProjectV2List\b`),
925+
httpmock.StringResponse(`
926+
{ "data": { "repository": { "projectsV2": {
927+
"nodes": [
928+
{ "title": "CleanupV2", "id": "CLEANUPV2ID" },
929+
{ "title": "RoadmapV2", "id": "ROADMAPV2ID" }
930+
],
931+
"pageInfo": { "hasNextPage": false }
932+
} } } }
933+
`))
934+
reg.Register(
935+
httpmock.GraphQL(`query OrganizationProjectV2List\b`),
936+
httpmock.StringResponse(`
937+
{ "data": { "organization": { "projectsV2": {
938+
"nodes": [
939+
{ "title": "TriageV2", "id": "TRIAGEV2ID" }
940+
],
941+
"pageInfo": { "hasNextPage": false }
942+
} } } }
943+
`))
944+
reg.Register(
945+
httpmock.GraphQL(`query UserProjectV2List\b`),
946+
httpmock.StringResponse(`
947+
{ "data": { "viewer": { "projectsV2": {
948+
"nodes": [
949+
{ "title": "MonalisaV2", "id": "MONALISAV2ID" }
950+
],
951+
"pageInfo": { "hasNextPage": false }
952+
} } } }
953+
`))
954+
mockProjectV2ItemUpdate(reg)
955+
mockPullRequestUpdate(reg)
956+
},
957+
stdout: "https://github.com/OWNER/REPO/pull/123\n",
958+
},
897959
}
898960
for _, tt := range tests {
899961
t.Run(tt.name, func(t *testing.T) {
@@ -1102,8 +1164,8 @@ func mockProjectV2ItemUpdate(reg *httpmock.Registry) {
11021164

11031165
type testFetcher struct{}
11041166

1105-
func (f testFetcher) EditableOptionsFetch(client *api.Client, repo ghrepo.Interface, opts *shared.Editable) error {
1106-
return shared.FetchOptions(client, repo, opts)
1167+
func (f testFetcher) EditableOptionsFetch(client *api.Client, repo ghrepo.Interface, opts *shared.Editable, projectsV1Support gh.ProjectsV1Support) error {
1168+
return shared.FetchOptions(client, repo, opts, projectsV1Support)
11071169
}
11081170

11091171
type testSurveyor struct {

pkg/cmd/pr/shared/editable.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"fmt"
55

66
"github.com/cli/cli/v2/api"
7+
"github.com/cli/cli/v2/internal/gh"
78
"github.com/cli/cli/v2/internal/ghrepo"
89
"github.com/cli/cli/v2/pkg/set"
910
)
@@ -395,7 +396,7 @@ func FieldsToEditSurvey(p EditPrompter, editable *Editable) error {
395396
return nil
396397
}
397398

398-
func FetchOptions(client *api.Client, repo ghrepo.Interface, editable *Editable) error {
399+
func FetchOptions(client *api.Client, repo ghrepo.Interface, editable *Editable, projectV1Support gh.ProjectsV1Support) error {
399400
// Determine whether to fetch organization teams.
400401
// Interactive reviewer editing (Edited true, but no Add/Remove slices) still needs
401402
// team data for selection UI. For non-interactive flows, we never need to fetch teams.
@@ -413,7 +414,7 @@ func FetchOptions(client *api.Client, repo ghrepo.Interface, editable *Editable)
413414
Assignees: editable.Assignees.Edited,
414415
ActorAssignees: editable.Assignees.ActorAssignees,
415416
Labels: editable.Labels.Edited,
416-
ProjectsV1: editable.Projects.Edited,
417+
ProjectsV1: editable.Projects.Edited && projectV1Support == gh.ProjectsV1Supported,
417418
ProjectsV2: editable.Projects.Edited,
418419
Milestones: editable.Milestone.Edited,
419420
}

0 commit comments

Comments
 (0)