Remove existing reviewer if reassign is triggerred. (#13460)

Author: iyabchen

.ci/magician/cmd/interfaces.go

@@ -33,6 +33,7 @@ type GithubClient interface {
 	PostComment(prNumber, comment string) error
 	UpdateComment(prNumber, comment string, id int) error
 	RequestPullRequestReviewers(prNumber string, reviewers []string) error
+	RemovePullRequestReviewers(prNumber string, reviewers []string) error
 	AddLabels(prNumber string, labels []string) error
 	RemoveLabel(prNumber, label string) error
 	CreateWorkflowDispatchEvent(workflowFileName string, inputs map[string]any) error

.ci/magician/cmd/mock_github_test.go

@@ -74,6 +74,11 @@ func (m *mockGithub) RequestPullRequestReviewers(prNumber string, reviewers []st
 	return nil
 }
 
+func (m *mockGithub) RemovePullRequestReviewers(prNumber string, reviewers []string) error {
+	m.calledMethods["RemovePullRequestReviewers"] = append(m.calledMethods["RemovePullRequestReviewers"], []any{prNumber, reviewers})
+	return nil
+}
+
 func (m *mockGithub) PostComment(prNumber string, comment string) error {
 	m.calledMethods["PostComment"] = append(m.calledMethods["PostComment"], []any{prNumber, comment})
 	return nil

.ci/magician/cmd/reassign_reviewer.go

@@ -96,6 +96,9 @@ func execReassignReviewer(prNumber, newPrimaryReviewer string, gh GithubClient)
 			return err
 		}
 	} else {
+		if err := gh.RemovePullRequestReviewers(prNumber, []string{currentReviewer}); err != nil {
+			fmt.Printf("Failed to remove reviewer %s from pull request: %s\n", currentReviewer, err)
+		}
 		fmt.Println("Updating reviewer comment")
 		err := gh.UpdateComment(prNumber, comment, reviewerComment.ID)
 		if err != nil {

.ci/magician/cmd/reassign_reviewer_test.go

@@ -0,0 +1,104 @@
+/*
+* Copyright 2025 Google LLC. All Rights Reserved.
+*
+* Licensed under the Apache License, Version 2.0 (the "License");
+* you may not use this file except in compliance with the License.
+* You may obtain a copy of the License at
+*
+*     http://www.apache.org/licenses/LICENSE-2.0
+*
+* Unless required by applicable law or agreed to in writing, software
+* distributed under the License is distributed on an "AS IS" BASIS,
+* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+* See the License for the specific language governing permissions and
+* limitations under the License.
+ */
+package cmd
+
+import (
+	"magician/github"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestExecReassignReviewer(t *testing.T) {
+	availableReviewers := github.AvailableReviewers(nil)
+	if len(availableReviewers) < 3 {
+		t.Fatalf("not enough available reviewers (%v) to run TestExecRequestReviewer (need at least 3)", availableReviewers)
+	}
+	cases := map[string]struct {
+		newPrimaryReviewer      string
+		comments                []github.PullRequestComment
+		expectSpecificReviewers []string
+		expectRemovedReviewers  []string
+	}{
+		"reassign from no current reviewer to random reviewer": {
+			comments: []github.PullRequestComment{},
+		},
+		"reassign from no current reviewer to alice": {
+			newPrimaryReviewer:      "alice",
+			comments:                []github.PullRequestComment{},
+			expectSpecificReviewers: []string{"alice"},
+		},
+		"reassign from bob to random reviewer": {
+			comments: []github.PullRequestComment{
+				{
+					Body: github.FormatReviewerComment("bob"),
+					ID:   1234,
+				},
+			},
+			expectRemovedReviewers: []string{"bob"},
+		},
+		"reassign from bob to alice": {
+			newPrimaryReviewer: "alice",
+			comments: []github.PullRequestComment{
+				{
+					Body: github.FormatReviewerComment("bob"),
+					ID:   1234,
+				},
+			},
+			expectSpecificReviewers: []string{"alice"},
+			expectRemovedReviewers:  []string{"bob"},
+		},
+	}
+	for tn, tc := range cases {
+		t.Run(tn, func(t *testing.T) {
+			gh := &mockGithub{
+				pullRequest: github.PullRequest{
+					User: github.User{Login: "author"},
+				},
+				calledMethods:       make(map[string][][]any),
+				pullRequestComments: tc.comments,
+			}
+
+			err := execReassignReviewer("1", tc.newPrimaryReviewer, gh)
+			if err != nil {
+				t.Fatalf("execReassignReviewer failed: %v", err)
+			}
+			if len(gh.calledMethods["RequestPullRequestReviewers"]) != 1 {
+				t.Errorf("Expected RequestPullRequestReviewers called 1 time, got %v", len(gh.calledMethods["RequestPullRequestReviewers"]))
+			}
+
+			var assignedReviewers []string
+			for _, args := range gh.calledMethods["RequestPullRequestReviewers"] {
+				assignedReviewers = append(assignedReviewers, args[1].([]string)...)
+			}
+			var removedReviewers []string
+			for _, args := range gh.calledMethods["RemovePullRequestReviewers"] {
+				removedReviewers = append(removedReviewers, args[1].([]string)...)
+			}
+
+			if tc.expectSpecificReviewers != nil {
+				for _, reviewer := range assignedReviewers {
+					assert.Contains(t, tc.expectSpecificReviewers, reviewer)
+				}
+			}
+			if tc.expectRemovedReviewers != nil {
+				for _, reviewer := range removedReviewers {
+					assert.Contains(t, tc.expectRemovedReviewers, reviewer)
+				}
+			}
+		})
+	}
+}

.ci/magician/github/set.go

@@ -92,6 +92,24 @@ func (gh *Client) RequestPullRequestReviewers(prNumber string, reviewers []strin
 	return nil
 }
 
+func (gh *Client) RemovePullRequestReviewers(prNumber string, reviewers []string) error {
+	url := fmt.Sprintf("https://api.github.com/repos/GoogleCloudPlatform/magic-modules/pulls/%s/requested_reviewers", prNumber)
+
+	body := map[string][]string{
+		"reviewers":      reviewers,
+		"team_reviewers": {},
+	}
+
+	err := utils.RequestCall(url, "DELETE", gh.token, nil, body)
+	if err != nil {
+		return err
+	}
+
+	fmt.Printf("Successfully removed reviewers %v to pull request %s\n", reviewers, prNumber)
+
+	return nil
+}
+
 func (gh *Client) AddLabels(prNumber string, labels []string) error {
 	url := fmt.Sprintf("https://api.github.com/repos/GoogleCloudPlatform/magic-modules/issues/%s/labels", prNumber)