Merge "Fix ignoring change when project is watched"

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 @@ public class ProjectWatchIT extends AbstractDaemonTest {
     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();
+  }
 }

gerrit-server/src/main/java/com/google/gerrit/server/mail/send/AbandonedSender.java

@@ -43,6 +43,7 @@ public class AbandonedSender extends ReplyToChangeSender {
     ccAllApprovals();
     bccStarredBy();
     includeWatchers(NotifyType.ABANDONED_CHANGES);
+    removeUsersThatIgnoredTheChange();
   }
 
   @Override

gerrit-server/src/main/java/com/google/gerrit/server/mail/send/ChangeEmail.java

@@ -69,6 +69,7 @@ public abstract class ChangeEmail extends NotificationEmail {
 
   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 @@ public abstract class ChangeEmail extends NotificationEmail {
     }
     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 @@ public abstract class ChangeEmail extends NotificationEmail {
       return;
     }
 
-    try {
+    for (Map.Entry<Account.Id, Collection<String>> e : stars.asMap().entrySet()) {
-      // BCC anyone who has starred this change
+      if (e.getValue().contains(StarredChangesUtil.DEFAULT_LABEL)) {
-      // and remove anyone who has ignored this change.
+        super.add(RecipientType.BCC, e.getKey());
-      //
+      }
-      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)) {
+  protected void removeUsersThatIgnoredTheChange() {
-          super.add(RecipientType.BCC, e.getKey());
+    for (Map.Entry<Account.Id, Collection<String>> e : stars.asMap().entrySet()) {
-        }
+      if (e.getValue().contains(StarredChangesUtil.IGNORE_LABEL)) {
-        if (e.getValue().contains(StarredChangesUtil.IGNORE_LABEL)) {
+        AccountState accountState = args.accountCache.get(e.getKey());
-          AccountState accountState = args.accountCache.get(e.getKey());
+        if (accountState != null) {
-          if (accountState != null) {
+          removeUser(accountState.getAccount());
-            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);
     }
   }
 

gerrit-server/src/main/java/com/google/gerrit/server/mail/send/CommentSender.java

@@ -157,6 +157,7 @@ public class CommentSender extends ReplyToChangeSender {
       bccStarredBy();
       includeWatchers(NotifyType.ALL_COMMENTS);
     }
+    removeUsersThatIgnoredTheChange();
 
     // Add header that enables identifying comments on parsed email.
     // Grouping is currently done by timestamp.

gerrit-server/src/main/java/com/google/gerrit/server/mail/send/DeleteReviewerSender.java

@@ -58,6 +58,7 @@ public class DeleteReviewerSender extends ReplyToChangeSender {
     ccExistingReviewers();
     includeWatchers(NotifyType.ALL_COMMENTS);
     add(RecipientType.TO, reviewers);
+    removeUsersThatIgnoredTheChange();
   }
 
   @Override

gerrit-server/src/main/java/com/google/gerrit/server/mail/send/DeleteVoteSender.java

@@ -43,6 +43,7 @@ public class DeleteVoteSender extends ReplyToChangeSender {
     ccAllApprovals();
     bccStarredBy();
     includeWatchers(NotifyType.ALL_COMMENTS);
+    removeUsersThatIgnoredTheChange();
   }
 
   @Override

gerrit-server/src/main/java/com/google/gerrit/server/mail/send/MergedSender.java

@@ -52,6 +52,7 @@ public class MergedSender extends ReplyToChangeSender {
     bccStarredBy();
     includeWatchers(NotifyType.ALL_COMMENTS);
     includeWatchers(NotifyType.SUBMITTED_CHANGES);
+    removeUsersThatIgnoredTheChange();
   }
 
   @Override

gerrit-server/src/main/java/com/google/gerrit/server/mail/send/ReplacePatchSetSender.java

@@ -67,6 +67,7 @@ public class ReplacePatchSetSender extends ReplyToChangeSender {
     rcptToAuthors(RecipientType.CC);
     bccStarredBy();
     includeWatchers(NotifyType.NEW_PATCHSETS, !patchSet.isDraft());
+    removeUsersThatIgnoredTheChange();
   }
 
   @Override

gerrit-server/src/main/java/com/google/gerrit/server/mail/send/RestoredSender.java

@@ -43,6 +43,7 @@ public class RestoredSender extends ReplyToChangeSender {
     ccAllApprovals();
     bccStarredBy();
     includeWatchers(NotifyType.ALL_COMMENTS);
+    removeUsersThatIgnoredTheChange();
   }
 
   @Override

gerrit-server/src/main/java/com/google/gerrit/server/mail/send/RevertedSender.java

@@ -42,6 +42,7 @@ public class RevertedSender extends ReplyToChangeSender {
     ccAllApprovals();
     bccStarredBy();
     includeWatchers(NotifyType.ALL_COMMENTS);
+    removeUsersThatIgnoredTheChange();
   }
 
   @Override