Fix notifications for draft changes
Notifications that are configured in the project.config must not be
considered for draft changes. For draft changes we only want to notify
the owner, reviewers and watchers that can view all draft. This was
broken by change I6a8e2f62f.
Change-Id: Ifdbce7bde9001a78f1c3e8cd0dda78f6b8fd28b3
Signed-off-by: Edwin Kempin <ekempin@google.com>
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 b976460..891d387 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
@@ -81,6 +81,28 @@
}
@Test
+ public void noNotificationForDraftChangesForWatchersInNotifyConfig() throws Exception {
+ Address addr = new Address("Watcher", "watcher@example.com");
+ NotifyConfig nc = new NotifyConfig();
+ nc.addEmail(addr);
+ nc.setName("team");
+ nc.setHeader(NotifyConfig.Header.TO);
+ nc.setTypes(EnumSet.of(NotifyType.NEW_CHANGES));
+
+ ProjectConfig cfg = projectCache.checkedGet(project).getConfig();
+ cfg.putNotifyConfig("team", nc);
+ saveProjectConfig(project, cfg);
+
+ PushOneCommit.Result r =
+ pushFactory
+ .create(db, admin.getIdent(), testRepo, "draft change", "a", "a1")
+ .to("refs/for/master%draft");
+ r.assertOkStatus();
+
+ assertThat(sender.getMessages()).isEmpty();
+ }
+
+ @Test
public void watchProject() throws Exception {
// watch project
String watchedProject = createProject("watchedProject").get();
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/ChangeEmail.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/ChangeEmail.java
index fbecaef..461029a 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/ChangeEmail.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/ChangeEmail.java
@@ -335,13 +335,14 @@
}
@Override
- protected final Watchers getWatchers(NotifyType type) throws OrmException {
+ protected final Watchers getWatchers(NotifyType type, boolean includeWatchersFromNotifyConfig)
+ throws OrmException {
if (!NotifyHandling.ALL.equals(notify)) {
return new Watchers();
}
ProjectWatch watch = new ProjectWatch(args, branch.getParentKey(), projectState, changeData);
- return watch.getWatchers(type);
+ return watch.getWatchers(type, includeWatchersFromNotifyConfig);
}
/** Any user who has published comments on this change. */
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/CreateChangeSender.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/CreateChangeSender.java
index 3d00c75..3e9e62c 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/CreateChangeSender.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/CreateChangeSender.java
@@ -47,9 +47,10 @@
protected void init() throws EmailException {
super.init();
+ boolean isDraft = change.getStatus() == Change.Status.DRAFT;
try {
// Try to mark interested owners with TO and CC or BCC line.
- Watchers matching = getWatchers(NotifyType.NEW_CHANGES);
+ Watchers matching = getWatchers(NotifyType.NEW_CHANGES, !isDraft);
for (Account.Id user :
Iterables.concat(matching.to.accounts, matching.cc.accounts, matching.bcc.accounts)) {
if (isOwnerOfProjectOrBranch(user)) {
@@ -68,7 +69,7 @@
log.warn("Cannot notify watchers for new change", err);
}
- includeWatchers(NotifyType.NEW_PATCHSETS);
+ includeWatchers(NotifyType.NEW_PATCHSETS, !isDraft);
}
private boolean isOwnerOfProjectOrBranch(Account.Id user) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/NotificationEmail.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/NotificationEmail.java
index b63694c..bceac72 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/NotificationEmail.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/NotificationEmail.java
@@ -59,8 +59,13 @@
/** Include users and groups that want notification of events. */
protected void includeWatchers(NotifyType type) {
+ includeWatchers(type, true);
+ }
+
+ /** Include users and groups that want notification of events. */
+ protected void includeWatchers(NotifyType type, boolean includeWatchersFromNotifyConfig) {
try {
- Watchers matching = getWatchers(type);
+ Watchers matching = getWatchers(type, includeWatchersFromNotifyConfig);
add(RecipientType.TO, matching.to);
add(RecipientType.CC, matching.cc);
add(RecipientType.BCC, matching.bcc);
@@ -73,7 +78,8 @@
}
/** Returns all watchers that are relevant */
- protected abstract Watchers getWatchers(NotifyType type) throws OrmException;
+ protected abstract Watchers getWatchers(NotifyType type, boolean includeWatchersFromNotifyConfig)
+ throws OrmException;
/** Add users or email addresses to the TO, CC, or BCC list. */
protected void add(RecipientType type, Watchers.List list) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/ProjectWatch.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/ProjectWatch.java
index 3eb6f05..b459d25 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/ProjectWatch.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/ProjectWatch.java
@@ -65,7 +65,8 @@
}
/** Returns all watchers that are relevant */
- public final Watchers getWatchers(NotifyType type) throws OrmException {
+ public final Watchers getWatchers(NotifyType type, boolean includeWatchersFromNotifyConfig)
+ throws OrmException {
Watchers matching = new Watchers();
Set<Account.Id> projectWatchers = new HashSet<>();
@@ -91,6 +92,10 @@
}
}
+ if (!includeWatchersFromNotifyConfig) {
+ return matching;
+ }
+
for (ProjectState state : projectState.tree()) {
for (NotifyConfig nc : state.getConfig().getNotifyConfigs()) {
if (nc.isNotify(type)) {
