Fix the change message for removing cc
Until now, whether we removed a reviewer or cc, the change message
always was "removed reviewer ..". With this change, when removing ccs
the change message will be accordingly.
While at it, added a test for removing ccs and improved the current
test for reviewer to also check the change message.
Bug: Issue 13734
Change-Id: I1df74a083c47b57a5d32f71e5d0d522796726107
diff --git a/java/com/google/gerrit/server/change/DeleteReviewerOp.java b/java/com/google/gerrit/server/change/DeleteReviewerOp.java
index 07cb04f..bf00d27 100644
--- a/java/com/google/gerrit/server/change/DeleteReviewerOp.java
+++ b/java/com/google/gerrit/server/change/DeleteReviewerOp.java
@@ -40,6 +40,7 @@
import com.google.gerrit.server.mail.send.DeleteReviewerSender;
import com.google.gerrit.server.mail.send.MessageIdGenerator;
import com.google.gerrit.server.notedb.ChangeUpdate;
+import com.google.gerrit.server.notedb.ReviewerStateInternal;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.RemoveReviewerControl;
@@ -132,9 +133,15 @@
for (LabelType lt : labelTypes.getLabelTypes()) {
newApprovals.put(lt.getName(), (short) 0);
}
-
+ String ccOrReviewer =
+ approvalsUtil
+ .getReviewers(ctx.getNotes())
+ .byState(ReviewerStateInternal.CC)
+ .contains(reviewerId)
+ ? "cc"
+ : "reviewer";
StringBuilder msg = new StringBuilder();
- msg.append("Removed reviewer " + reviewer.account().fullName());
+ msg.append(String.format("Removed %s %s", ccOrReviewer, reviewer.account().fullName()));
StringBuilder removedVotesMsg = new StringBuilder();
removedVotesMsg.append(" with the following votes:\n\n");
boolean votesRemoved = false;
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
index f59fba0..b7f1ef0 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -2258,6 +2258,10 @@
assertThat(message.body()).contains("Removed reviewer " + user.fullName() + ".");
assertThat(message.body()).doesNotContain("with the following votes");
+ // Make sure the change message for removing a reviewer is correct.
+ assertThat(Iterables.getLast(gApi.changes().id(changeId).messages()).message)
+ .contains("Removed reviewer " + user.fullName());
+
// Make sure the reviewer can still be added again.
gApi.changes().id(changeId).addReviewer(user.id().toString());
c = gApi.changes().id(changeId).get();
@@ -2273,6 +2277,31 @@
}
@Test
+ public void removeCC() throws Exception {
+ PushOneCommit.Result result = createChange();
+ String changeId = result.getChangeId();
+ // Add a cc
+ AddReviewerInput addReviewerInput = new AddReviewerInput();
+ addReviewerInput.state = CC;
+ addReviewerInput.reviewer = user.id().toString();
+ gApi.changes().id(changeId).addReviewer(addReviewerInput);
+
+ // Remove a cc
+ sender.clear();
+ gApi.changes().id(changeId).reviewer(user.id().toString()).remove();
+ assertThat(gApi.changes().id(changeId).get().reviewers).isEmpty();
+
+ // Make sure the email for removing a cc is correct.
+ assertThat(sender.getMessages()).hasSize(1);
+ Message message = sender.getMessages().get(0);
+ assertThat(message.body()).contains("Removed cc " + user.fullName() + ".");
+
+ // Make sure the change message for removing a reviewer is correct.
+ assertThat(Iterables.getLast(gApi.changes().id(changeId).messages()).message)
+ .contains("Removed cc " + user.fullName());
+ }
+
+ @Test
public void removeReviewer() throws Exception {
testRemoveReviewer(true);
}
