Skip to content

Commit 17af24e

Browse files
authored
Merge pull request cli#10975 from cli/babakks/fix-removing-all-pr-reviewers
Fix bug when removing all PR reviewers
2 parents 5db8cf7 + 5dca16f commit 17af24e

2 files changed

Lines changed: 87 additions & 4 deletions

File tree

pkg/cmd/pr/edit/edit.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -283,8 +283,7 @@ func updatePullRequestReviews(httpClient *http.Client, repo ghrepo.Interface, id
283283
if err != nil {
284284
return err
285285
}
286-
if (userIds == nil || len(*userIds) == 0) &&
287-
(teamIds == nil || len(*teamIds) == 0) {
286+
if userIds == nil && teamIds == nil {
288287
return nil
289288
}
290289
union := githubv4.Boolean(false)

pkg/cmd/pr/edit/edit_test.go

Lines changed: 86 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -446,6 +446,64 @@ func Test_editRun(t *testing.T) {
446446
},
447447
stdout: "https://github.com/OWNER/REPO/pull/123\n",
448448
},
449+
{
450+
name: "non-interactive remove all reviewers",
451+
input: &EditOptions{
452+
SelectorArg: "123",
453+
Finder: shared.NewMockFinder("123", &api.PullRequest{
454+
URL: "https://github.com/OWNER/REPO/pull/123",
455+
}, ghrepo.New("OWNER", "REPO")),
456+
Interactive: false,
457+
Editable: shared.Editable{
458+
Title: shared.EditableString{
459+
Value: "new title",
460+
Edited: true,
461+
},
462+
Body: shared.EditableString{
463+
Value: "new body",
464+
Edited: true,
465+
},
466+
Base: shared.EditableString{
467+
Value: "base-branch-name",
468+
Edited: true,
469+
},
470+
Reviewers: shared.EditableSlice{
471+
Remove: []string{"OWNER/core", "OWNER/external", "monalisa", "hubot", "dependabot"},
472+
Edited: true,
473+
},
474+
Assignees: shared.EditableSlice{
475+
Add: []string{"monalisa", "hubot"},
476+
Remove: []string{"octocat"},
477+
Edited: true,
478+
},
479+
Labels: shared.EditableSlice{
480+
Add: []string{"feature", "TODO", "bug"},
481+
Remove: []string{"docs"},
482+
Edited: true,
483+
},
484+
Projects: shared.EditableProjects{
485+
EditableSlice: shared.EditableSlice{
486+
Add: []string{"Cleanup", "CleanupV2"},
487+
Remove: []string{"Roadmap", "RoadmapV2"},
488+
Edited: true,
489+
},
490+
},
491+
Milestone: shared.EditableString{
492+
Value: "GA",
493+
Edited: true,
494+
},
495+
},
496+
Fetcher: testFetcher{},
497+
},
498+
httpStubs: func(reg *httpmock.Registry) {
499+
mockRepoMetadata(reg, false)
500+
mockPullRequestUpdate(reg)
501+
mockPullRequestReviewersUpdate(reg)
502+
mockPullRequestUpdateLabels(reg)
503+
mockProjectV2ItemUpdate(reg)
504+
},
505+
stdout: "https://github.com/OWNER/REPO/pull/123\n",
506+
},
449507
{
450508
name: "interactive",
451509
input: &EditOptions{
@@ -487,6 +545,27 @@ func Test_editRun(t *testing.T) {
487545
},
488546
stdout: "https://github.com/OWNER/REPO/pull/123\n",
489547
},
548+
{
549+
name: "interactive remove all reviewers",
550+
input: &EditOptions{
551+
SelectorArg: "123",
552+
Finder: shared.NewMockFinder("123", &api.PullRequest{
553+
URL: "https://github.com/OWNER/REPO/pull/123",
554+
}, ghrepo.New("OWNER", "REPO")),
555+
Interactive: true,
556+
Surveyor: testSurveyor{removeAllReviewers: true},
557+
Fetcher: testFetcher{},
558+
EditorRetriever: testEditorRetriever{},
559+
},
560+
httpStubs: func(reg *httpmock.Registry) {
561+
mockRepoMetadata(reg, false)
562+
mockPullRequestUpdate(reg)
563+
mockPullRequestReviewersUpdate(reg)
564+
mockPullRequestUpdateLabels(reg)
565+
mockProjectV2ItemUpdate(reg)
566+
},
567+
stdout: "https://github.com/OWNER/REPO/pull/123\n",
568+
},
490569
}
491570
for _, tt := range tests {
492571
t.Run(tt.name, func(t *testing.T) {
@@ -658,7 +737,8 @@ func mockProjectV2ItemUpdate(reg *httpmock.Registry) {
658737

659738
type testFetcher struct{}
660739
type testSurveyor struct {
661-
skipReviewers bool
740+
skipReviewers bool
741+
removeAllReviewers bool
662742
}
663743
type testEditorRetriever struct{}
664744

@@ -683,7 +763,11 @@ func (s testSurveyor) EditFields(e *shared.Editable, _ string) error {
683763
e.Title.Value = "new title"
684764
e.Body.Value = "new body"
685765
if !s.skipReviewers {
686-
e.Reviewers.Value = []string{"monalisa", "hubot", "OWNER/core", "OWNER/external"}
766+
if s.removeAllReviewers {
767+
e.Reviewers.Remove = []string{"monalisa", "hubot", "OWNER/core", "OWNER/external", "dependabot"}
768+
} else {
769+
e.Reviewers.Value = []string{"monalisa", "hubot", "OWNER/core", "OWNER/external"}
770+
}
687771
}
688772
e.Assignees.Value = []string{"monalisa", "hubot"}
689773
e.Labels.Value = []string{"feature", "TODO", "bug"}

0 commit comments

Comments
 (0)