From e3ace4f50b40fd7b3855efcb8ddb2d8786bd988c Mon Sep 17 00:00:00 2001
From: Jim Bishopp <jamesbishopp@gmail.com>
Date: Wed, 7 Feb 2024 12:44:31 -0800
Subject: [PATCH] Add preferredOnly flag

---
 bot/internal/review/review.go      | 21 +++++++++++++++++++++
 bot/internal/review/review_test.go | 12 ++++++------
 2 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/bot/internal/review/review.go b/bot/internal/review/review.go
index 2e2caac6..4b2c8f76 100644
--- a/bot/internal/review/review.go
+++ b/bot/internal/review/review.go
@@ -94,6 +94,8 @@ func isAllowedRobot(author string) bool {
 type Reviewer struct {
 	// Owner is true if the reviewer is a code or docs owner (required for all reviews).
 	Owner bool `json:"owner"`
+	// PreferredOnly is true if the reviewer should only be included in preferred reviewer paths.
+	PreferredOnly bool `json:"preferredOnly",omitempty`
 	// PreferredReviewerFor contains a list of file paths that this reviewer
 	// should be selected to review.
 	PreferredReviewerFor []string `json:"preferredReviewerFor,omitempty"`
@@ -286,10 +288,14 @@ func (r *Assignments) getCodeReviewers(e *env.Environment, files []github.PullRe
 	// pick from the overall set at random.
 	resultingSetA := preferredSetA
 	if len(resultingSetA) == 0 {
+		// Remove reviewers from setA whose preferredOnly field is false.
+		setA = filterPreferredOnly(reviewers, setA, false)
 		resultingSetA = append(resultingSetA, setA[r.c.Rand.Intn(len(setA))])
 	}
 	resultingSetB := preferredSetB
 	if len(resultingSetB) == 0 {
+		// Remove reviewers from setB whose preferredOnly field is false.
+		setB = filterPreferredOnly(reviewers, setB, false)
 		resultingSetB = append(resultingSetB, setB[r.c.Rand.Intn(len(setB))])
 	}
 
@@ -574,6 +580,21 @@ func reviewsByAuthor(reviews []github.Review) map[string]string {
 	return m
 }
 
+// filterPreferredOnly returns all names in set whose preferredOnly field is set to the preferredOnly parameter.
+func filterPreferredOnly(reviewers map[string]Reviewer, set []string, preferredOnly bool) []string {
+	filtered := make([]string, 0, len(set))
+	for _, name := range set {
+		reviewer, ok := reviewers[name]
+		if !ok {
+			continue
+		}
+		if reviewer.PreferredOnly == preferredOnly {
+			filtered = append(filtered, name)
+		}
+	}
+	return filtered
+}
+
 const (
 	// Commented is a code review where the reviewer has left comments only.
 	Commented = "COMMENTED"
diff --git a/bot/internal/review/review_test.go b/bot/internal/review/review_test.go
index 467506b4..9965ffbc 100644
--- a/bot/internal/review/review_test.go
+++ b/bot/internal/review/review_test.go
@@ -192,7 +192,7 @@ func TestIsInternal(t *testing.T) {
 }
 
 // TestGetCodeReviewers checks internal code review assignments.
-func TestGetCodeReviewers(t *testing.T) {
+func TestGetCodeReviewerSets(t *testing.T) {
 	tests := []struct {
 		desc        string
 		assignments *Assignments
@@ -1174,15 +1174,15 @@ func (r *randStatic) Intn(n int) int {
 	return 0
 }
 
-func TestPreferredReviewers(t *testing.T) {
+func TestGetCodeReviewers(t *testing.T) {
 	assignments := &Assignments{
 		c: &Config{
 			Rand: &randStatic{},
 			CoreReviewers: map[string]Reviewer{
-				"1": {Owner: true, PreferredReviewerFor: []string{"lib/srv/db", "lib/srv/app"}},
+				"1": {Owner: true, PreferredReviewerFor: []string{"lib/srv/db", "lib/srv/app"}, PreferredOnly: true},
 				"2": {Owner: true, PreferredReviewerFor: []string{"lib/srv/db", "lib/alpn"}},
 				"3": {Owner: true},
-				"4": {Owner: false, PreferredReviewerFor: []string{"lib/srv/app"}},
+				"4": {Owner: false, PreferredReviewerFor: []string{"lib/srv/app"}, PreferredOnly: true},
 				"5": {Owner: false, PreferredReviewerFor: []string{"lib/srv/db"}},
 				"6": {Owner: false},
 			},
@@ -1209,7 +1209,7 @@ func TestPreferredReviewers(t *testing.T) {
 			files: []github.PullRequestFile{
 				{Name: "lib/alpn/proxy.go"},
 			},
-			expected: []string{"2", "4"},
+			expected: []string{"2", "5"},
 		},
 		{
 			description: "preferred reviewers for different files",
@@ -1226,7 +1226,7 @@ func TestPreferredReviewers(t *testing.T) {
 			files: []github.PullRequestFile{
 				{Name: "lib/service/service.go"},
 			},
-			expected: []string{"1", "4"},
+			expected: []string{"2", "5"},
 		},
 		{
 			description: "covered paths: don't add new or duplicate reviewers for paths already covered",