Merge "Send newchange email to reviewers when review is started"

java/com/google/gerrit/server/restapi/change/PostReview.java

@@ -372,7 +372,10 @@ public class PostReview
         reviewerResult.gatherResults();
       }
 
-      emailReviewers(revision.getChange(), reviewerResults, reviewerNotify, accountsToNotify);
+      boolean readyForReview =
+          (output.ready != null && output.ready) || !revision.getChange().isWorkInProgress();
+      emailReviewers(
+          revision.getChange(), reviewerResults, reviewerNotify, accountsToNotify, readyForReview);
     }
 
     return Response.ok(output);
@@ -405,7 +408,8 @@ public class PostReview
       Change change,
       List<PostReviewers.Addition> reviewerAdditions,
       @Nullable NotifyHandling notify,
-      ListMultimap<RecipientType, Account.Id> accountsToNotify) {
+      ListMultimap<RecipientType, Account.Id> accountsToNotify,
+      boolean readyForReview) {
     List<Account.Id> to = new ArrayList<>();
     List<Account.Id> cc = new ArrayList<>();
     List<Address> toByEmail = new ArrayList<>();
@@ -423,7 +427,8 @@ public class PostReview
       reviewerAdditions
           .get(0)
           .op
-          .emailReviewers(change, to, cc, toByEmail, ccByEmail, notify, accountsToNotify);
+          .emailReviewers(
+              change, to, cc, toByEmail, ccByEmail, notify, accountsToNotify, readyForReview);
     }
   }
 

java/com/google/gerrit/server/restapi/change/PostReviewersOp.java

@@ -202,7 +202,8 @@ public class PostReviewersOp implements BatchUpdateOp {
         reviewersByEmail,
         addedCCsByEmail,
         notify,
-        accountsToNotify);
+        accountsToNotify,
+        !rsrc.getChange().isWorkInProgress());
     if (!addedReviewers.isEmpty()) {
       List<AccountState> reviewers =
           addedReviewers
@@ -221,7 +222,8 @@ public class PostReviewersOp implements BatchUpdateOp {
       Collection<Address> addedByEmail,
       Collection<Address> copiedByEmail,
       NotifyHandling notify,
-      ListMultimap<RecipientType, Account.Id> accountsToNotify) {
+      ListMultimap<RecipientType, Account.Id> accountsToNotify,
+      boolean readyForReview) {
     if (added.isEmpty() && copied.isEmpty() && addedByEmail.isEmpty() && copiedByEmail.isEmpty()) {
       return;
     }
@@ -250,7 +252,7 @@ public class PostReviewersOp implements BatchUpdateOp {
       AddReviewerSender cm = addReviewerSenderFactory.create(change.getProject(), change.getId());
       // Default to silent operation on WIP changes.
       NotifyHandling defaultNotifyHandling =
-          change.isWorkInProgress() ? NotifyHandling.NONE : NotifyHandling.ALL;
+          readyForReview ? NotifyHandling.ALL : NotifyHandling.NONE;
       cm.setNotify(MoreObjects.firstNonNull(notify, defaultNotifyHandling));
       cm.setAccountsToNotify(accountsToNotify);
       cm.setFrom(userId);

javatests/com/google/gerrit/acceptance/server/mail/ChangeNotificationsIT.java

@@ -866,7 +866,8 @@ public class ChangeNotificationsIT extends AbstractNotificationTest {
   }
 
   @Test
-  public void addReviewerOnWipChangeAndStartReview() throws Exception {
+  public void addReviewerOnWipChangeAndStartReviewInNoteDb() throws Exception {
+    assume().that(notesMigration.readChanges()).isTrue();
     StagedChange sc = stageWipChange();
     ReviewInput in = ReviewInput.noScore().reviewer(other.email).setWorkInProgress(false);
     gApi.changes().id(sc.changeId).revision("current").review(in);
@@ -877,6 +878,35 @@ public class ChangeNotificationsIT extends AbstractNotificationTest {
         .bcc(sc.starrer)
         .bcc(ALL_COMMENTS)
         .noOneElse();
+    // TODO(logan): Should CCs be included?
+    assertThat(sender)
+        .sent("newchange", sc)
+        .to(other)
+        .cc(sc.reviewer)
+        .cc(sc.reviewerByEmail, sc.ccerByEmail)
+        .noOneElse();
+    assertThat(sender).notSent();
+  }
+
+  @Test
+  public void addReviewerOnWipChangeAndStartReviewInReviewDb() throws Exception {
+    assume().that(notesMigration.readChanges()).isFalse();
+    StagedChange sc = stageWipChange();
+    ReviewInput in = ReviewInput.noScore().reviewer(other.email).setWorkInProgress(false);
+    gApi.changes().id(sc.changeId).revision("current").review(in);
+    assertThat(sender)
+        .sent("comment", sc)
+        .cc(sc.reviewer, sc.ccer, other)
+        .cc(sc.reviewerByEmail, sc.ccerByEmail)
+        .bcc(sc.starrer)
+        .bcc(ALL_COMMENTS)
+        .noOneElse();
+    assertThat(sender)
+        .sent("newchange", sc)
+        .to(other)
+        .cc(sc.reviewer, sc.ccer)
+        .cc(sc.reviewerByEmail, sc.ccerByEmail)
+        .noOneElse();
     assertThat(sender).notSent();
   }