Merge "PostReview: Fix email for removing an email without Gerrit account as CC"
diff --git a/java/com/google/gerrit/server/mail/send/NewChangeSender.java b/java/com/google/gerrit/server/mail/send/NewChangeSender.java
index 001de52..e899fc5 100644
--- a/java/com/google/gerrit/server/mail/send/NewChangeSender.java
+++ b/java/com/google/gerrit/server/mail/send/NewChangeSender.java
@@ -116,7 +116,7 @@
names.add(getNameFor(id));
}
for (Address address : removedByEmailReviewers) {
- names.add(address.name());
+ names.add(address.toString());
}
return names;
}
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
index 8b5eaf8..aaefb2d 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -4817,7 +4817,7 @@
}
@Test
- public void ccNonExistentAccountByEmailThenRemove() throws Exception {
+ public void ccNonExistentAccountByEmailThenRemoveByDelete() throws Exception {
// Create a project that allows reviewers by email.
Project.NameKey project = projectOperations.newProject().create();
try (ProjectConfigUpdate u = updateProject(project)) {
@@ -4862,6 +4862,54 @@
.contains(String.format("%s has removed %s", admin.fullName(), reviewerInput.reviewer));
}
+ @Test
+ public void ccNonExistentAccountByEmailThenRemoveByPostReview() throws Exception {
+ // Create a project that allows reviewers by email.
+ Project.NameKey project = projectOperations.newProject().create();
+ try (ProjectConfigUpdate u = updateProject(project)) {
+ u.getConfig()
+ .updateProject(
+ b ->
+ b.setBooleanConfig(
+ BooleanProjectConfig.ENABLE_REVIEWER_BY_EMAIL, InheritableBoolean.TRUE));
+ u.save();
+ }
+
+ // Create a change
+ TestRepository<?> testRepo = cloneProject(project, admin);
+ PushOneCommit push = pushFactory.create(admin.newIdent(), testRepo);
+ PushOneCommit.Result r = push.to("refs/for/master");
+ r.assertOkStatus();
+
+ // Add an email as a CC for which no Gerrit account exists.
+ sender.clear();
+ ReviewerInput reviewerInput = new ReviewerInput();
+ reviewerInput.state = CC;
+ reviewerInput.reviewer = "email-without-account@example.com";
+ gApi.changes().id(r.getChangeId()).addReviewer(reviewerInput);
+
+ // Check that the email was added as a CC and an email was sent.
+ AccountInfo ccedAccountInfo =
+ Iterables.getOnlyElement(
+ gApi.changes().id(r.getChangeId()).get().reviewers.get(ReviewerState.CC));
+ assertThat(ccedAccountInfo.email).isEqualTo(reviewerInput.reviewer);
+ assertThat(ccedAccountInfo._accountId).isNull();
+ assertThat(ccedAccountInfo.name).isNull();
+ assertThat(Iterables.getOnlyElement(sender.getMessages()).body())
+ .contains(String.format("%s has uploaded this change for review", admin.fullName()));
+
+ // Remove the CC.
+ sender.clear();
+ ReviewInput reviewInput = new ReviewInput();
+ reviewInput.reviewer(reviewerInput.reviewer, ReviewerState.REMOVED, /* confirmed= */ false);
+ gApi.changes().id(r.getChangeId()).current().review(reviewInput);
+
+ // Check that the email was removed as a CC and an email was sent.
+ assertThat(gApi.changes().id(r.getChangeId()).get().reviewers).isEmpty();
+ assertThat(Iterables.getOnlyElement(sender.getMessages()).body())
+ .contains(String.format("%s has removed %s", admin.fullName(), reviewerInput.reviewer));
+ }
+
private PushOneCommit.Result createWorkInProgressChange() throws Exception {
return pushTo("refs/for/master%wip");
}
