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
This commit is contained in:
Gal Paikin
2020-11-25 12:38:06 +01:00
parent fcefd46b52
commit 1340e49486
2 changed files with 38 additions and 2 deletions

View File

@@ -40,6 +40,7 @@ import com.google.gerrit.server.extensions.events.ReviewerDeleted;
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 @@ public class DeleteReviewerOp implements BatchUpdateOp {
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;

View File

@@ -2258,6 +2258,10 @@ public class ChangeIT extends AbstractDaemonTest {
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();
@@ -2272,6 +2276,31 @@ public class ChangeIT extends AbstractDaemonTest {
() -> gApi.changes().id(changeId).reviewer(user.id().toString()).remove());
}
@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);