Merge "Remove deprecated logic to always make caller a reviewer"
diff --git a/java/com/google/gerrit/server/restapi/change/PostReview.java b/java/com/google/gerrit/server/restapi/change/PostReview.java
index 4ba1ab7..a16d4f9 100644
--- a/java/com/google/gerrit/server/restapi/change/PostReview.java
+++ b/java/com/google/gerrit/server/restapi/change/PostReview.java
@@ -49,7 +49,6 @@
 import com.google.gerrit.entities.Comment;
 import com.google.gerrit.entities.FixReplacement;
 import com.google.gerrit.entities.FixSuggestion;
-import com.google.gerrit.entities.LabelId;
 import com.google.gerrit.entities.Patch;
 import com.google.gerrit.entities.PatchSet;
 import com.google.gerrit.entities.PatchSetApproval;
@@ -1286,8 +1285,6 @@
         return false;
       }
 
-      forceCallerAsReviewer(projectState, ctx, current, ups, del);
-
       return !del.isEmpty() || !ups.isEmpty();
     }
 
@@ -1358,41 +1355,6 @@
       }
     }
 
-    private void forceCallerAsReviewer(
-        ProjectState projectState,
-        ChangeContext ctx,
-        Map<String, PatchSetApproval> current,
-        List<PatchSetApproval> ups,
-        List<PatchSetApproval> del) {
-      if (current.isEmpty() && ups.isEmpty()) {
-        // TODO Find another way to link reviewers to changes.
-        if (del.isEmpty()) {
-          // If no existing label is being set to 0, hack in the caller
-          // as a reviewer by picking the first server-wide LabelType.
-          List<LabelType> labelTypes = projectState.getLabelTypes(ctx.getNotes()).getLabelTypes();
-          if (labelTypes.isEmpty()) {
-            logger.atWarning().log(
-                "no label type found for project %s, change %s",
-                projectState.getName(), ctx.getChange().getChangeId());
-            return;
-          }
-
-          LabelId labelId = labelTypes.get(0).getLabelId();
-          ups.add(
-              ApprovalsUtil.newApproval(psId, user, labelId, 0, ctx.getWhen())
-                  .tag(Optional.ofNullable(in.tag))
-                  .granted(ctx.getWhen())
-                  .build());
-        } else {
-          // Pick a random label that is about to be deleted and keep it.
-          Iterator<PatchSetApproval> i = del.iterator();
-          ups.add(i.next().toBuilder().value(0).granted(ctx.getWhen()).build());
-          i.remove();
-        }
-      }
-      ctx.getUpdate(ctx.getChange().currentPatchSetId()).putReviewer(user.getAccountId(), REVIEWER);
-    }
-
     private Map<String, PatchSetApproval> scanLabels(
         ProjectState projectState, ChangeContext ctx, List<PatchSetApproval> del)
         throws IOException {
diff --git a/javatests/com/google/gerrit/acceptance/api/change/PostReviewIT.java b/javatests/com/google/gerrit/acceptance/api/change/PostReviewIT.java
index be0cc04..7d73374 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/PostReviewIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/PostReviewIT.java
@@ -31,13 +31,16 @@
 import com.google.gerrit.acceptance.AbstractDaemonTest;
 import com.google.gerrit.acceptance.PushOneCommit;
 import com.google.gerrit.acceptance.config.GerritConfig;
+import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
 import com.google.gerrit.extensions.annotations.Exports;
 import com.google.gerrit.extensions.api.changes.DraftInput;
 import com.google.gerrit.extensions.api.changes.ReviewInput;
 import com.google.gerrit.extensions.api.changes.ReviewInput.CommentInput;
 import com.google.gerrit.extensions.api.changes.ReviewInput.DraftHandling;
 import com.google.gerrit.extensions.api.changes.ReviewInput.RobotCommentInput;
+import com.google.gerrit.extensions.client.ReviewerState;
 import com.google.gerrit.extensions.client.Side;
+import com.google.gerrit.extensions.common.AccountInfo;
 import com.google.gerrit.extensions.common.ChangeMessageInfo;
 import com.google.gerrit.extensions.common.RobotCommentInfo;
 import com.google.gerrit.extensions.config.FactoryModule;
@@ -54,6 +57,7 @@
 import java.sql.Timestamp;
 import java.util.Collection;
 import java.util.List;
+import java.util.Map;
 import org.junit.Before;
 import org.junit.Test;
 import org.mockito.ArgumentCaptor;
@@ -64,6 +68,7 @@
 
   @Inject private CommentValidator mockCommentValidator;
   @Inject private TestCommentHelper testCommentHelper;
+  @Inject private RequestScopeOperations requestScopeOperations;
 
   private static final String COMMENT_TEXT = "The comment text";
   private static final CommentForValidation FILE_COMMENT_FOR_VALIDATION =
@@ -382,6 +387,93 @@
         .contains("Exceeding maximum cumulative size of comments");
   }
 
+  @Test
+  public void ccToReviewer() throws Exception {
+    PushOneCommit.Result r = createChange();
+    // User adds themselves and changes state
+    requestScopeOperations.setApiUser(user.id());
+
+    ReviewInput input = new ReviewInput().reviewer(user.id().toString(), ReviewerState.CC, false);
+    gApi.changes().id(r.getChangeId()).current().review(input);
+
+    Map<ReviewerState, Collection<AccountInfo>> reviewers =
+        gApi.changes().id(r.getChangeId()).get().reviewers;
+    assertThat(reviewers).hasSize(1);
+    AccountInfo reviewer = Iterables.getOnlyElement(reviewers.get(ReviewerState.CC));
+    assertThat(reviewer._accountId).isEqualTo(user.id().get());
+
+    // CC -> Reviewer
+    ReviewInput input2 = new ReviewInput().reviewer(user.id().toString());
+    gApi.changes().id(r.getChangeId()).current().review(input2);
+
+    Map<ReviewerState, Collection<AccountInfo>> reviewers2 =
+        gApi.changes().id(r.getChangeId()).get().reviewers;
+    assertThat(reviewers2).hasSize(1);
+    AccountInfo reviewer2 = Iterables.getOnlyElement(reviewers2.get(ReviewerState.REVIEWER));
+    assertThat(reviewer2._accountId).isEqualTo(user.id().get());
+  }
+
+  @Test
+  public void reviewerToCc() throws Exception {
+    // Admin owns the change
+    PushOneCommit.Result r = createChange();
+    // User adds themselves and changes state
+    requestScopeOperations.setApiUser(user.id());
+
+    ReviewInput input = new ReviewInput().reviewer(user.id().toString());
+    gApi.changes().id(r.getChangeId()).current().review(input);
+
+    Map<ReviewerState, Collection<AccountInfo>> reviewers =
+        gApi.changes().id(r.getChangeId()).get().reviewers;
+    assertThat(reviewers).hasSize(1);
+    AccountInfo reviewer = Iterables.getOnlyElement(reviewers.get(ReviewerState.REVIEWER));
+    assertThat(reviewer._accountId).isEqualTo(user.id().get());
+
+    // Reviewer -> CC
+    ReviewInput input2 = new ReviewInput().reviewer(user.id().toString(), ReviewerState.CC, false);
+    gApi.changes().id(r.getChangeId()).current().review(input2);
+
+    Map<ReviewerState, Collection<AccountInfo>> reviewers2 =
+        gApi.changes().id(r.getChangeId()).get().reviewers;
+    assertThat(reviewers2).hasSize(1);
+    AccountInfo reviewer2 = Iterables.getOnlyElement(reviewers2.get(ReviewerState.CC));
+    assertThat(reviewer2._accountId).isEqualTo(user.id().get());
+  }
+
+  @Test
+  public void votingMakesCallerReviewer() throws Exception {
+    // Admin owns the change
+    PushOneCommit.Result r = createChange();
+    // User adds themselves and changes state
+    requestScopeOperations.setApiUser(user.id());
+
+    ReviewInput input = new ReviewInput().label("Code-Review", 1);
+    gApi.changes().id(r.getChangeId()).current().review(input);
+
+    Map<ReviewerState, Collection<AccountInfo>> reviewers =
+        gApi.changes().id(r.getChangeId()).get().reviewers;
+    assertThat(reviewers).hasSize(1);
+    AccountInfo reviewer = Iterables.getOnlyElement(reviewers.get(ReviewerState.REVIEWER));
+    assertThat(reviewer._accountId).isEqualTo(user.id().get());
+  }
+
+  @Test
+  public void commentingMakesUserCC() throws Exception {
+    // Admin owns the change
+    PushOneCommit.Result r = createChange();
+    // User adds themselves and changes state
+    requestScopeOperations.setApiUser(user.id());
+
+    ReviewInput input = new ReviewInput().message("Foo bar!");
+    gApi.changes().id(r.getChangeId()).current().review(input);
+
+    Map<ReviewerState, Collection<AccountInfo>> reviewers =
+        gApi.changes().id(r.getChangeId()).get().reviewers;
+    assertThat(reviewers).hasSize(1);
+    AccountInfo reviewer = Iterables.getOnlyElement(reviewers.get(ReviewerState.CC));
+    assertThat(reviewer._accountId).isEqualTo(user.id().get());
+  }
+
   private List<RobotCommentInfo> getRobotComments(String changeId) throws RestApiException {
     return gApi.changes().id(changeId).robotComments().values().stream()
         .flatMap(Collection::stream)