From bd0a115873ae4ed785276447c8fe6330934cab20 Mon Sep 17 00:00:00 2001
From: Logan Hanks <logan@google.com>
Date: Wed, 26 Apr 2017 16:59:11 +0200
Subject: [PATCH] Fix notifications for comments on draft patch sets

Change-Id: If16e160fe78e28bb3901ddaa08de63953173f32d
---
 .../server/project/ProjectWatchIT.java         | 18 ++++++++++++++++--
 .../gerrit/server/mail/send/CommentSender.java |  2 +-
 2 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/project/ProjectWatchIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/project/ProjectWatchIT.java
index c6a94b29a7..72c196e4a1 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/project/ProjectWatchIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/project/ProjectWatchIT.java
@@ -100,7 +100,7 @@ public class ProjectWatchIT extends AbstractDaemonTest {
     nc.addEmail(addr);
     nc.setName("team");
     nc.setHeader(NotifyConfig.Header.TO);
-    nc.setTypes(EnumSet.of(NotifyType.NEW_CHANGES));
+    nc.setTypes(EnumSet.of(NotifyType.NEW_CHANGES, NotifyType.ALL_COMMENTS));
 
     ProjectConfig cfg = projectCache.checkedGet(project).getConfig();
     cfg.putNotifyConfig("team", nc);
@@ -113,6 +113,13 @@ public class ProjectWatchIT extends AbstractDaemonTest {
     r.assertOkStatus();
 
     assertThat(sender.getMessages()).isEmpty();
+
+    setApiUser(admin);
+    ReviewInput in = new ReviewInput();
+    in.message = "comment";
+    gApi.changes().id(r.getChangeId()).current().review(in);
+
+    assertThat(sender.getMessages()).isEmpty();
   }
 
   @Test
@@ -122,7 +129,7 @@ public class ProjectWatchIT extends AbstractDaemonTest {
     nc.addEmail(addr);
     nc.setName("team");
     nc.setHeader(NotifyConfig.Header.TO);
-    nc.setTypes(EnumSet.of(NotifyType.NEW_PATCHSETS));
+    nc.setTypes(EnumSet.of(NotifyType.NEW_PATCHSETS, NotifyType.ALL_COMMENTS));
 
     ProjectConfig cfg = projectCache.checkedGet(project).getConfig();
     cfg.putNotifyConfig("team", nc);
@@ -143,6 +150,13 @@ public class ProjectWatchIT extends AbstractDaemonTest {
     r.assertOkStatus();
 
     assertThat(sender.getMessages()).isEmpty();
+
+    setApiUser(admin);
+    ReviewInput in = new ReviewInput();
+    in.message = "comment";
+    gApi.changes().id(r.getChangeId()).current().review(in);
+
+    assertThat(sender.getMessages()).isEmpty();
   }
 
   @Test
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/CommentSender.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/CommentSender.java
index d00c905826..ddea42e4f4 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/CommentSender.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/CommentSender.java
@@ -155,7 +155,7 @@ public class CommentSender extends ReplyToChangeSender {
     }
     if (notify.compareTo(NotifyHandling.ALL) >= 0) {
       bccStarredBy();
-      includeWatchers(NotifyType.ALL_COMMENTS);
+      includeWatchers(NotifyType.ALL_COMMENTS, !patchSet.isDraft());
     }
     removeUsersThatIgnoredTheChange();