Suggest Reviewers REST endpoint: Add option to suggest CCs

The reviewer suggestion excludes existing reviewers. This means if it is
used to suggest CCs, it doesn't show existing reviewers and hence users
cannot easily move existing reviewers to CC. Add an option to suggest
accounts for CC that includes existing reviewers, but excludes existing
CCs.

Bug: Issue 11354
Change-Id: Ic8960452d1eb5fe1b4559a27d6d7104428b9be85
Signed-off-by: Edwin Kempin <ekempin@google.com>

Documentation/rest-api-changes.txt

@@ -3035,6 +3035,14 @@ As result a list of link:#suggested-reviewer-info[SuggestedReviewerInfo] entries
   ]
 ----
 
+To suggest CCs `reviewer-state=CC` can be specified as additional URL
+parameter. This includes existing reviewers in the result, but excludes
+existing CCs.
+
+--
+'GET /changes/link:#change-id[\{change-id\}]/suggest_reviewers?q=J&reviewer-state=CC'
+--
+
 [[get-reviewer]]
 === Get Reviewer
 --

java/com/google/gerrit/extensions/api/changes/ChangeApi.java

@@ -18,6 +18,7 @@ import com.google.common.collect.ImmutableListMultimap;
 import com.google.common.collect.Sets;
 import com.google.gerrit.common.Nullable;
 import com.google.gerrit.extensions.client.ListChangesOption;
+import com.google.gerrit.extensions.client.ReviewerState;
 import com.google.gerrit.extensions.common.AccountInfo;
 import com.google.gerrit.extensions.common.ChangeInfo;
 import com.google.gerrit.extensions.common.ChangeMessageInfo;
@@ -216,6 +217,10 @@ public interface ChangeApi {
     return suggestReviewers().withQuery(query);
   }
 
+  default SuggestedReviewersRequest suggestCcs(String query) throws RestApiException {
+    return suggestReviewers().forCc().withQuery(query);
+  }
+
   /**
    * Retrieve reviewers ({@code ReviewerState.REVIEWER} and {@code ReviewerState.CC}) on the change.
    */
@@ -382,6 +387,7 @@ public interface ChangeApi {
     private String query;
     private int limit;
     private boolean excludeGroups;
+    private ReviewerState reviewerState = ReviewerState.REVIEWER;
 
     public abstract List<SuggestedReviewerInfo> get() throws RestApiException;
 
@@ -400,6 +406,11 @@ public interface ChangeApi {
       return this;
     }
 
+    public SuggestedReviewersRequest forCc() {
+      this.reviewerState = ReviewerState.CC;
+      return this;
+    }
+
     public String getQuery() {
       return query;
     }
@@ -411,6 +422,10 @@ public interface ChangeApi {
     public boolean getExcludeGroups() {
       return excludeGroups;
     }
+
+    public ReviewerState getReviewerState() {
+      return reviewerState;
+    }
   }
 
   /**

java/com/google/gerrit/server/api/changes/ChangeApiImpl.java

@@ -449,6 +449,7 @@ class ChangeApiImpl implements ChangeApi {
       suggestReviewers.setQuery(r.getQuery());
       suggestReviewers.setLimit(r.getLimit());
       suggestReviewers.setExcludeGroups(r.getExcludeGroups());
+      suggestReviewers.setReviewerState(r.getReviewerState());
       return suggestReviewers.apply(change).value();
     } catch (Exception e) {
       throw asRestApiException("Cannot retrieve suggested reviewers", e);

java/com/google/gerrit/server/restapi/change/ReviewerRecommender.java

@@ -14,7 +14,6 @@
 
 package com.google.gerrit.server.restapi.change;
 
-import static com.google.gerrit.server.notedb.ReviewerStateInternal.REVIEWER;
 import static java.util.stream.Collectors.toList;
 
 import com.google.common.base.Strings;
@@ -23,6 +22,7 @@ import com.google.common.collect.ImmutableMap;
 import com.google.common.flogger.FluentLogger;
 import com.google.gerrit.common.Nullable;
 import com.google.gerrit.common.data.LabelType;
+import com.google.gerrit.extensions.client.ReviewerState;
 import com.google.gerrit.index.query.Predicate;
 import com.google.gerrit.index.query.QueryParseException;
 import com.google.gerrit.reviewdb.client.Account;
@@ -34,6 +34,7 @@ import com.google.gerrit.server.change.SuggestedReviewer;
 import com.google.gerrit.server.config.GerritServerConfig;
 import com.google.gerrit.server.index.change.ChangeField;
 import com.google.gerrit.server.notedb.ChangeNotes;
+import com.google.gerrit.server.notedb.ReviewerStateInternal;
 import com.google.gerrit.server.plugincontext.PluginMapContext;
 import com.google.gerrit.server.project.ProjectState;
 import com.google.gerrit.server.query.change.ChangeData;
@@ -96,6 +97,7 @@ public class ReviewerRecommender {
   }
 
   public List<Account.Id> suggestReviewers(
+      ReviewerState reviewerState,
       @Nullable ChangeNotes changeNotes,
       SuggestReviewers suggestReviewers,
       ProjectState projectState,
@@ -178,7 +180,7 @@ public class ReviewerRecommender {
       // Remove existing reviewers
       approvalsUtil
           .getReviewers(changeNotes)
-          .byState(REVIEWER)
+          .byState(ReviewerStateInternal.fromReviewerState(reviewerState))
           .forEach(
               r -> {
                 if (reviewerScores.remove(r) != null) {

java/com/google/gerrit/server/restapi/change/ReviewersUtil.java

@@ -24,6 +24,7 @@ import com.google.common.collect.Sets;
 import com.google.common.flogger.FluentLogger;
 import com.google.gerrit.common.Nullable;
 import com.google.gerrit.common.data.GroupReference;
+import com.google.gerrit.extensions.client.ReviewerState;
 import com.google.gerrit.extensions.common.GroupBaseInfo;
 import com.google.gerrit.extensions.common.SuggestedReviewerInfo;
 import com.google.gerrit.extensions.restapi.Url;
@@ -157,6 +158,7 @@ public class ReviewersUtil {
   }
 
   public List<SuggestedReviewerInfo> suggestReviewers(
+      ReviewerState reviewerState,
       @Nullable ChangeNotes changeNotes,
       SuggestReviewers suggestReviewers,
       ProjectState projectState,
@@ -190,7 +192,8 @@ public class ReviewersUtil {
     }
 
     List<Account.Id> sortedRecommendations =
-        recommendAccounts(changeNotes, suggestReviewers, projectState, candidateList);
+        recommendAccounts(
+            reviewerState, changeNotes, suggestReviewers, projectState, candidateList);
     logger.atFine().log("Sorted recommendations: %s", sortedRecommendations);
 
     // Filter accounts by visibility and enforce limit
@@ -286,6 +289,7 @@ public class ReviewersUtil {
   }
 
   private List<Account.Id> recommendAccounts(
+      ReviewerState reviewerState,
       @Nullable ChangeNotes changeNotes,
       SuggestReviewers suggestReviewers,
       ProjectState projectState,
@@ -293,7 +297,7 @@ public class ReviewersUtil {
       throws IOException, ConfigInvalidException {
     try (Timer0.Context ctx = metrics.recommendAccountsLatency.start()) {
       return reviewerRecommender.suggestReviewers(
-          changeNotes, suggestReviewers, projectState, candidateList);
+          reviewerState, changeNotes, suggestReviewers, projectState, candidateList);
     }
   }
 

java/com/google/gerrit/server/restapi/change/SuggestChangeReviewers.java

@@ -14,6 +14,7 @@
 
 package com.google.gerrit.server.restapi.change;
 
+import com.google.gerrit.extensions.client.ReviewerState;
 import com.google.gerrit.extensions.common.AccountVisibility;
 import com.google.gerrit.extensions.common.SuggestedReviewerInfo;
 import com.google.gerrit.extensions.restapi.AuthException;
@@ -44,6 +45,7 @@ public class SuggestChangeReviewers extends SuggestReviewers
   private final ProjectCache projectCache;
 
   private boolean excludeGroups;
+  private ReviewerState reviewerState = ReviewerState.REVIEWER;
 
   @Option(
       name = "--exclude-groups",
@@ -54,6 +56,16 @@ public class SuggestChangeReviewers extends SuggestReviewers
     return this;
   }
 
+  @Option(
+      name = "--reviewer-state",
+      usage =
+          "The type of reviewers that should be suggested"
+              + " (can be 'REVIEWER' or 'CC', default is 'REVIEWER')")
+  public SuggestChangeReviewers setReviewerState(ReviewerState reviewerState) {
+    this.reviewerState = reviewerState;
+    return this;
+  }
+
   @Inject
   SuggestChangeReviewers(
       AccountVisibility av,
@@ -75,8 +87,13 @@ public class SuggestChangeReviewers extends SuggestReviewers
     if (!self.get().isIdentifiedUser()) {
       throw new AuthException("Authentication required");
     }
+    if (reviewerState.equals(ReviewerState.REMOVED)) {
+      throw new BadRequestException(
+          String.format("Unsupported reviewer state: %s", ReviewerState.REMOVED));
+    }
     return Response.ok(
         reviewersUtil.suggestReviewers(
+            reviewerState,
             rsrc.getNotes(),
             this,
             projectCache.checkedGet(rsrc.getProject()),

javatests/com/google/gerrit/acceptance/rest/change/SuggestReviewersIT.java

@@ -547,6 +547,22 @@ public class SuggestReviewersIT extends AbstractDaemonTest {
         suggestReviewers(changeId, name), ImmutableList.of(foo1, foo2), ImmutableList.of());
   }
 
+  @Test
+  public void suggestReviewerAsCc() throws Exception {
+    String name = name("foo");
+    TestAccount foo1 = accountCreator.create(name + "-1");
+    TestAccount foo2 = accountCreator.create(name + "-2");
+
+    String changeId = createChange().getChangeId();
+    assertReviewers(suggestCcs(changeId, name), ImmutableList.of(foo1, foo2), ImmutableList.of());
+
+    AddReviewerInput reviewerInput = new AddReviewerInput();
+    reviewerInput.reviewer = foo2.id().toString();
+    reviewerInput.state = ReviewerState.REVIEWER;
+    gApi.changes().id(changeId).addReviewer(reviewerInput);
+    assertReviewers(suggestCcs(changeId, name), ImmutableList.of(foo1, foo2), ImmutableList.of());
+  }
+
   @Test
   public void suggestBySecondaryEmailWithModifyAccount() throws Exception {
     String secondaryEmail = "foo.secondary@example.com";
@@ -611,6 +627,10 @@ public class SuggestReviewersIT extends AbstractDaemonTest {
     return gApi.changes().id(changeId).suggestReviewers(query).withLimit(n).get();
   }
 
+  private List<SuggestedReviewerInfo> suggestCcs(String changeId, String query) throws Exception {
+    return gApi.changes().id(changeId).suggestCcs(query).get();
+  }
+
   private AccountGroup.UUID createGroupWithArbitraryMembers(int numMembers) {
     Set<Account.Id> members =
         IntStream.rangeClosed(1, numMembers)