From 9e5757d5a766646f5e4153c6e30407471a0d2556 Mon Sep 17 00:00:00 2001
From: nagl-resourcely <137814572+nagl-resourcely@users.noreply.github.com>
Date: Sun, 27 Oct 2024 06:06:11 -0700
Subject: [PATCH] Allow RemoveReviewers to remove only teams (#3337)

---
 github/pulls_reviewers.go      | 22 +++++++++++++++++++++-
 github/pulls_reviewers_test.go | 21 +++++++++++++++++++++
 2 files changed, 42 insertions(+), 1 deletion(-)

diff --git a/github/pulls_reviewers.go b/github/pulls_reviewers.go
index 3f0c50b746a..9dd60ae6888 100644
--- a/github/pulls_reviewers.go
+++ b/github/pulls_reviewers.go
@@ -23,6 +23,13 @@ type Reviewers struct {
 	Teams []*Team `json:"teams,omitempty"`
 }
 
+type removeReviewersRequest struct {
+	NodeID *string `json:"node_id,omitempty"`
+	// Note the lack of omitempty! See comment in RemoveReviewers.
+	Reviewers     []string `json:"reviewers"`
+	TeamReviewers []string `json:"team_reviewers,omitempty"`
+}
+
 // RequestReviewers creates a review request for the provided reviewers for the specified pull request.
 //
 // GitHub API docs: https://docs.github.com/rest/pulls/review-requests#request-reviewers-for-a-pull-request
@@ -76,8 +83,21 @@ func (s *PullRequestsService) ListReviewers(ctx context.Context, owner, repo str
 //
 //meta:operation DELETE /repos/{owner}/{repo}/pulls/{pull_number}/requested_reviewers
 func (s *PullRequestsService) RemoveReviewers(ctx context.Context, owner, repo string, number int, reviewers ReviewersRequest) (*Response, error) {
+	// reviewers.Reviewers may be empty if the caller wants to remove teams, but not users. Unlike AddReviewers,
+	// "reviewers" is a required param here. Reference: https://github.com/google/go-github/issues/3336
+	removeRequest := removeReviewersRequest{
+		NodeID:        reviewers.NodeID,
+		Reviewers:     reviewers.Reviewers,
+		TeamReviewers: reviewers.TeamReviewers,
+	}
+
+	if removeRequest.Reviewers == nil {
+		// GitHub accepts the empty list, but rejects null. Removing `omitempty` is not enough - we also have to promote nil to [].
+		removeRequest.Reviewers = []string{}
+	}
+
 	u := fmt.Sprintf("repos/%s/%s/pulls/%d/requested_reviewers", owner, repo, number)
-	req, err := s.client.NewRequest("DELETE", u, &reviewers)
+	req, err := s.client.NewRequest("DELETE", u, &removeRequest)
 	if err != nil {
 		return nil, err
 	}
diff --git a/github/pulls_reviewers_test.go b/github/pulls_reviewers_test.go
index ac025d10e26..dfe7e6b7216 100644
--- a/github/pulls_reviewers_test.go
+++ b/github/pulls_reviewers_test.go
@@ -172,6 +172,27 @@ func TestRemoveReviewers(t *testing.T) {
 	})
 }
 
+func TestRemoveReviewers_teamsOnly(t *testing.T) {
+	t.Parallel()
+	client, mux, _ := setup(t)
+
+	mux.HandleFunc("/repos/o/r/pulls/1/requested_reviewers", func(w http.ResponseWriter, r *http.Request) {
+		testMethod(t, r, "DELETE")
+		testBody(t, r, `{"reviewers":[],"team_reviewers":["justice-league"]}`+"\n")
+	})
+
+	ctx := context.Background()
+	_, err := client.PullRequests.RemoveReviewers(ctx, "o", "r", 1, ReviewersRequest{TeamReviewers: []string{"justice-league"}})
+	if err != nil {
+		t.Errorf("PullRequests.RemoveReviewers returned error: %v", err)
+	}
+
+	const methodName = "RemoveReviewers"
+	testNewRequestAndDoFailure(t, methodName, client, func() (*Response, error) {
+		return client.PullRequests.RemoveReviewers(ctx, "o", "r", 1, ReviewersRequest{TeamReviewers: []string{"justice-league"}})
+	})
+}
+
 func TestListReviewers(t *testing.T) {
 	t.Parallel()
 	client, mux, _ := setup(t)