ChangeIT: Assert that submitting a change doesn't remove non-voting reviewers

If a project is configured with two approval labels, for example Verified
and Code-Review, and a user has permission to approve on only one of them,
when the change is submitted without the user giving any review, the user
should not be removed from the review.

Bug: Issue 4163
Change-Id: Iff87398d3fba2ca0b53f0040a1ebd122cc0e079b

2 files changed, 70 insertions, 0 deletions

diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java
index 23bb1b9531..d92a887027 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -18,6 +18,7 @@ import static com.google.common.truth.Truth.assertThat;
 import static com.google.gerrit.acceptance.PushOneCommit.FILE_NAME;
 import static com.google.gerrit.acceptance.PushOneCommit.SUBJECT;
 import static com.google.gerrit.server.group.SystemGroupBackend.ANONYMOUS_USERS;
+import static com.google.gerrit.server.group.SystemGroupBackend.CHANGE_OWNER;
 import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
 import static com.google.gerrit.server.project.Util.blockLabel;
 import static com.google.gerrit.server.project.Util.category;
@@ -346,6 +347,71 @@ public class ChangeIT extends AbstractDaemonTest {
   }
 
   @Test
+  public void nonVotingReviewerStaysAfterSubmit() throws Exception {
+    LabelType verified = category("Verified",
+        value(1, "Passes"), value(0, "No score"), value(-1, "Failed"));
+    ProjectConfig cfg = projectCache.checkedGet(project).getConfig();
+    cfg.getLabelSections().put(verified.getName(), verified);
+    String heads = "refs/heads/*";
+    AccountGroup.UUID owners =
+        SystemGroupBackend.getGroup(CHANGE_OWNER).getUUID();
+    AccountGroup.UUID registered =
+        SystemGroupBackend.getGroup(REGISTERED_USERS).getUUID();
+    Util.allow(cfg,
+        Permission.forLabel(verified.getName()), -1, 1, owners, heads);
+    Util.allow(cfg,
+        Permission.forLabel("Code-Review"), -2, +2, registered, heads);
+    saveProjectConfig(project, cfg);
+
+    // Set Code-Review+2 and Verified+1 as admin (change owner)
+    PushOneCommit.Result r = createChange();
+    String changeId = r.getChangeId();
+    String commit = r.getCommit().name();
+    ReviewInput input = ReviewInput.approve();
+    input.label(verified.getName(), 1);
+    gApi.changes()
+        .id(changeId)
+        .revision(commit)
+        .review(input);
+
+    // Reviewers should only be "admin"
+    assertThat(getReviewers(changeId))
+        .containsExactlyElementsIn(ImmutableSet.of(admin.getId()));
+
+    // Add the user as reviewer
+    AddReviewerInput in = new AddReviewerInput();
+    in.reviewer = user.email;
+    gApi.changes()
+        .id(changeId)
+        .addReviewer(in);
+    assertThat(getReviewers(changeId))
+        .containsExactlyElementsIn(ImmutableSet.of(admin.getId(), user.getId()));
+
+    // Approve the change as user, then remove the approval
+    // (only to confirm that the user does have Code-Review+2 permission)
+    setApiUser(user);
+    gApi.changes()
+        .id(changeId)
+        .revision(commit)
+        .review(ReviewInput.approve());
+    gApi.changes()
+        .id(changeId)
+        .revision(commit)
+        .review(ReviewInput.noScore());
+
+    // Submit the change
+    setApiUser(admin);
+    gApi.changes()
+        .id(changeId)
+        .revision(commit)
+        .submit();
+
+    // User should still be on the change
+    assertThat(getReviewers(changeId))
+        .containsExactlyElementsIn(ImmutableSet.of(admin.getId(), user.getId()));
+  }
+
+  @Test
   public void createEmptyChange() throws Exception {
     ChangeInfo in = new ChangeInfo();
     in.branch = Constants.MASTER;
diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/ReviewInput.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/ReviewInput.java
index e6ce6b2ee4..ee043ebafb 100644
--- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/ReviewInput.java
+++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/ReviewInput.java
@@ -122,6 +122,10 @@ public class ReviewInput {
     return new ReviewInput().label("Code-Review", -1);
   }
 
+  public static ReviewInput noScore() {
+    return new ReviewInput().label("Code-Review", 0);
+  }
+
   public static ReviewInput approve() {
     return new ReviewInput().label("Code-Review", 2);
   }