Merge "Fix ignoring change when project is watched"
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 77d3c55..ab543e8 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
@@ -15,13 +15,17 @@
package com.google.gerrit.acceptance.server.project;
import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.server.StarredChangesUtil.IGNORE_LABEL;
+import com.google.common.collect.ImmutableSet;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.NoHttpd;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.Sandboxed;
import com.google.gerrit.acceptance.TestAccount;
import com.google.gerrit.common.data.Permission;
+import com.google.gerrit.extensions.api.changes.ReviewInput;
+import com.google.gerrit.extensions.api.changes.StarsInput;
import com.google.gerrit.extensions.common.GroupInfo;
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.client.Project;
@@ -443,4 +447,37 @@
assertThat(m.body()).contains("Change subject: TRIGGER\n");
assertThat(m.body()).contains("Gerrit-PatchSet: 1\n");
}
+
+ @Test
+ public void watchProjectNoNotificationForIgnoredChange() throws Exception {
+ // watch project
+ String watchedProject = createProject("watchedProject").get();
+ setApiUser(user);
+ watch(watchedProject, null);
+
+ // push a change to watched project
+ setApiUser(admin);
+ TestRepository<InMemoryRepository> watchedRepo =
+ cloneProject(new Project.NameKey(watchedProject), admin);
+ PushOneCommit.Result r =
+ pushFactory
+ .create(db, admin.getIdent(), watchedRepo, "ignored change", "a", "a1")
+ .to("refs/for/master");
+ r.assertOkStatus();
+
+ // ignore the change
+ setApiUser(user);
+ gApi.accounts().self().setStars(r.getChangeId(), new StarsInput(ImmutableSet.of(IGNORE_LABEL)));
+
+ sender.clear();
+
+ // post a comment -> should not trigger email notification since user ignored the change
+ setApiUser(admin);
+ ReviewInput in = new ReviewInput();
+ in.message = "comment";
+ gApi.changes().id(r.getChangeId()).current().review(in);
+
+ // assert email notification
+ assertThat(sender.getMessages()).isEmpty();
+ }
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/AbandonedSender.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/AbandonedSender.java
index 17aee72..ec62833 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/AbandonedSender.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/AbandonedSender.java
@@ -43,6 +43,7 @@
ccAllApprovals();
bccStarredBy();
includeWatchers(NotifyType.ABANDONED_CHANGES);
+ removeUsersThatIgnoredTheChange();
}
@Override
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 461029a..bc09488 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
@@ -69,6 +69,7 @@
protected final Change change;
protected final ChangeData changeData;
+ protected ListMultimap<Account.Id, String> stars;
protected PatchSet patchSet;
protected PatchSetInfo patchSetInfo;
protected String changeMessage;
@@ -164,6 +165,12 @@
}
authors = getAuthors();
+ try {
+ stars = args.starredChangesUtil.byChangeFromIndex(change.getId());
+ } catch (OrmException e) {
+ throw new EmailException("Failed to load stars for change " + change.getChangeId(), e);
+ }
+
super.init();
if (timestamp != null) {
setHeader("Date", new Date(timestamp.getTime()));
@@ -309,28 +316,21 @@
return;
}
- try {
- // BCC anyone who has starred this change
- // and remove anyone who has ignored this change.
- //
- ListMultimap<Account.Id, String> stars =
- args.starredChangesUtil.byChangeFromIndex(change.getId());
- for (Map.Entry<Account.Id, Collection<String>> e : stars.asMap().entrySet()) {
- if (e.getValue().contains(StarredChangesUtil.DEFAULT_LABEL)) {
- super.add(RecipientType.BCC, e.getKey());
- }
- if (e.getValue().contains(StarredChangesUtil.IGNORE_LABEL)) {
- AccountState accountState = args.accountCache.get(e.getKey());
- if (accountState != null) {
- removeUser(accountState.getAccount());
- }
+ for (Map.Entry<Account.Id, Collection<String>> e : stars.asMap().entrySet()) {
+ if (e.getValue().contains(StarredChangesUtil.DEFAULT_LABEL)) {
+ super.add(RecipientType.BCC, e.getKey());
+ }
+ }
+ }
+
+ protected void removeUsersThatIgnoredTheChange() {
+ for (Map.Entry<Account.Id, Collection<String>> e : stars.asMap().entrySet()) {
+ if (e.getValue().contains(StarredChangesUtil.IGNORE_LABEL)) {
+ AccountState accountState = args.accountCache.get(e.getKey());
+ if (accountState != null) {
+ removeUser(accountState.getAccount());
}
}
- } catch (OrmException err) {
- // Just don't BCC everyone. Better to send a partial message to those
- // we already have queued up then to fail deliver entirely to people
- // who have a lower interest in the change.
- log.warn("Cannot BCC users that starred updated change", err);
}
}
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 02160ff..63d7e78 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
@@ -157,6 +157,7 @@
bccStarredBy();
includeWatchers(NotifyType.ALL_COMMENTS);
}
+ removeUsersThatIgnoredTheChange();
// Add header that enables identifying comments on parsed email.
// Grouping is currently done by timestamp.
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/DeleteReviewerSender.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/DeleteReviewerSender.java
index f46eab6..a563846 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/DeleteReviewerSender.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/DeleteReviewerSender.java
@@ -58,6 +58,7 @@
ccExistingReviewers();
includeWatchers(NotifyType.ALL_COMMENTS);
add(RecipientType.TO, reviewers);
+ removeUsersThatIgnoredTheChange();
}
@Override
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/DeleteVoteSender.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/DeleteVoteSender.java
index 13ee4ac..8509f73 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/DeleteVoteSender.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/DeleteVoteSender.java
@@ -43,6 +43,7 @@
ccAllApprovals();
bccStarredBy();
includeWatchers(NotifyType.ALL_COMMENTS);
+ removeUsersThatIgnoredTheChange();
}
@Override
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/MergedSender.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/MergedSender.java
index 0460a70..47115af 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/MergedSender.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/MergedSender.java
@@ -52,6 +52,7 @@
bccStarredBy();
includeWatchers(NotifyType.ALL_COMMENTS);
includeWatchers(NotifyType.SUBMITTED_CHANGES);
+ removeUsersThatIgnoredTheChange();
}
@Override
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/ReplacePatchSetSender.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/ReplacePatchSetSender.java
index 27cf2a44..c90000f 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/ReplacePatchSetSender.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/ReplacePatchSetSender.java
@@ -67,6 +67,7 @@
rcptToAuthors(RecipientType.CC);
bccStarredBy();
includeWatchers(NotifyType.NEW_PATCHSETS, !patchSet.isDraft());
+ removeUsersThatIgnoredTheChange();
}
@Override
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/RestoredSender.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/RestoredSender.java
index 7456d06..6076b46 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/RestoredSender.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/RestoredSender.java
@@ -43,6 +43,7 @@
ccAllApprovals();
bccStarredBy();
includeWatchers(NotifyType.ALL_COMMENTS);
+ removeUsersThatIgnoredTheChange();
}
@Override
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/RevertedSender.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/RevertedSender.java
index e590d94..c4c0a69 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/RevertedSender.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/RevertedSender.java
@@ -42,6 +42,7 @@
ccAllApprovals();
bccStarredBy();
includeWatchers(NotifyType.ALL_COMMENTS);
+ removeUsersThatIgnoredTheChange();
}
@Override
