Merge "Suggest Reviewers REST endpoint: Add option to suggest CCs"

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)