Support ignore label that suppresses notifications on update

Users can now ignore a change by setting an ignore label on it.
Ignoring a change means that the user doesn't get any email
notifications for this change when it is updated.

Also ignored changes are filtered out from the 'Incoming reviews'
section of the user dashboard.

Bug: Issue 2576
Change-Id: I1a43f49c2f738ab74182f9dd9d469a0aaf4fe1df
Signed-off-by: Edwin Kempin <ekempin@google.com>
diff --git a/Documentation/dev-stars.txt b/Documentation/dev-stars.txt
index 1a98438..d9fd10c 100644
--- a/Documentation/dev-stars.txt
+++ b/Documentation/dev-stars.txt
@@ -43,6 +43,24 @@
 
 The default star is represented by the special star label 'star'.
 
+[[ignore-star]]
+== Ignore Star
+
+If the ignore star is set by a user, this user gets no email
+notifications for updates of that change, even if this user is a
+reviewer of the change or the change is matched by a project watch of
+the user.
+
+Since changes can only be ignored once they are created, users that
+watch a project will always get the email notifications for the change
+creation. Only then the change can be ignored.
+
+Users that are added as reviewer to a change that they have ignored
+will be notified about this, so that they know about the review
+request. They can the decide to remove the ignore star.
+
+The ignore star is represented by the special star label 'ignore'.
+
 [[query-stars]]
 == Query Stars
 
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/AccountIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/AccountIT.java
index bce9249..e350cc9 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/AccountIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/AccountIT.java
@@ -24,6 +24,7 @@
 import static com.google.gerrit.gpg.testutil.TestKeys.validKeyWithSecondUserId;
 import static com.google.gerrit.gpg.testutil.TestKeys.validKeyWithoutExpiration;
 import static com.google.gerrit.server.StarredChangesUtil.DEFAULT_LABEL;
+import static com.google.gerrit.server.StarredChangesUtil.IGNORE_LABEL;
 import static java.nio.charset.StandardCharsets.UTF_8;
 
 import com.google.common.base.Function;
@@ -36,6 +37,7 @@
 import com.google.gerrit.acceptance.PushOneCommit;
 import com.google.gerrit.acceptance.TestAccount;
 import com.google.gerrit.extensions.api.accounts.EmailInput;
+import com.google.gerrit.extensions.api.changes.AddReviewerInput;
 import com.google.gerrit.extensions.api.changes.StarsInput;
 import com.google.gerrit.extensions.common.AccountInfo;
 import com.google.gerrit.extensions.common.ChangeInfo;
@@ -53,6 +55,7 @@
 import com.google.gerrit.reviewdb.client.AccountExternalId;
 import com.google.gerrit.server.config.AllUsersName;
 import com.google.gerrit.testutil.ConfigSuite;
+import com.google.gerrit.testutil.FakeEmailSender.Message;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
 
@@ -248,6 +251,70 @@
   }
 
   @Test
+  public void starWithDefaultAndIgnoreLabel() throws Exception {
+    PushOneCommit.Result r = createChange();
+    String triplet = project.get() + "~master~" + r.getChangeId();
+    exception.expect(BadRequestException.class);
+    exception.expectMessage("The labels " + DEFAULT_LABEL
+        + " and " + IGNORE_LABEL + " are mutually exclusive."
+        + " Only one of them can be set.");
+    gApi.accounts().self().setStars(triplet,
+        new StarsInput(ImmutableSet.of(DEFAULT_LABEL, "blue", IGNORE_LABEL)));
+  }
+
+  @Test
+  public void ignoreChange() throws Exception {
+    PushOneCommit.Result r = createChange();
+
+    AddReviewerInput in = new AddReviewerInput();
+    in.reviewer = user.email;
+    gApi.changes()
+        .id(r.getChangeId())
+        .addReviewer(in);
+
+    TestAccount user2 = accounts.user2();
+    in = new AddReviewerInput();
+    in.reviewer = user2.email;
+    gApi.changes()
+        .id(r.getChangeId())
+        .addReviewer(in);
+
+    setApiUser(user);
+    gApi.accounts().self().setStars(r.getChangeId(),
+        new StarsInput(ImmutableSet.of(IGNORE_LABEL)));
+
+    sender.clear();
+    setApiUser(admin);
+    gApi.changes()
+        .id(r.getChangeId())
+        .abandon();
+    List<Message> messages = sender.getMessages();
+    assertThat(messages).hasSize(1);
+    assertThat(messages.get(0).rcpt()).containsExactly(user2.emailAddress);
+  }
+
+  @Test
+  public void addReviewerToIgnoredChange() throws Exception {
+    PushOneCommit.Result r = createChange();
+
+    setApiUser(user);
+    gApi.accounts().self().setStars(r.getChangeId(),
+        new StarsInput(ImmutableSet.of(IGNORE_LABEL)));
+
+    sender.clear();
+    setApiUser(admin);
+
+    AddReviewerInput in = new AddReviewerInput();
+    in.reviewer = user.email;
+    gApi.changes()
+        .id(r.getChangeId())
+        .addReviewer(in);
+    List<Message> messages = sender.getMessages();
+    assertThat(messages).hasSize(1);
+    assertThat(messages.get(0).rcpt()).containsExactly(user.emailAddress);
+  }
+
+  @Test
   public void suggestAccounts() throws Exception {
     String adminUsername = "admin";
     List<AccountInfo> result = gApi.accounts()
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/AccountDashboardScreen.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/AccountDashboardScreen.java
index f53ecf8..62c14cb 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/AccountDashboardScreen.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/AccountDashboardScreen.java
@@ -95,7 +95,7 @@
   }
 
   private static String queryIncoming(String who) {
-    return "is:open reviewer:" + who + " -owner:" + who;
+    return "is:open reviewer:" + who + " -owner:" + who + " -star:ignore";
   }
 
   private static String queryClosed(String who) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/StarredChangesUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/StarredChangesUtil.java
index 77064fe..5e5e43c 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/StarredChangesUtil.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/StarredChangesUtil.java
@@ -65,11 +65,8 @@
 import org.slf4j.LoggerFactory;
 
 import java.io.IOException;
-import java.util.Collection;
 import java.util.Collections;
-import java.util.HashSet;
 import java.util.List;
-import java.util.Map;
 import java.util.Set;
 import java.util.SortedSet;
 import java.util.TreeSet;
@@ -116,6 +113,13 @@
               Joiner.on(", ").join(invalidLabels)));
     }
 
+    static IllegalLabelException mutuallyExclusiveLabels(String label1,
+        String label2) {
+      return new IllegalLabelException(
+          String.format("The labels %s and %s are mutually exclusive."
+              + " Only one of them can be set.", label1, label2));
+    }
+
     IllegalLabelException(String message) {
       super(message);
     }
@@ -125,6 +129,7 @@
       LoggerFactory.getLogger(StarredChangesUtil.class);
 
   public static final String DEFAULT_LABEL = "star";
+  public static final String IGNORE_LABEL = "ignore";
   public static final ImmutableSortedSet<String> DEFAULT_LABELS =
       ImmutableSortedSet.of(DEFAULT_LABEL);
 
@@ -180,6 +185,7 @@
       if (labels.isEmpty()) {
         deleteRef(repo, refName, oldObjectId);
       } else {
+        checkMutuallyExclusiveLabels(labels);
         updateLabels(repo, refName, oldObjectId, labels);
       }
 
@@ -289,18 +295,6 @@
     return changeData.get(0).stars();
   }
 
-  public Set<Account.Id> byChangeFromIndex(Change.Id changeId, String label)
-      throws OrmException, NoSuchChangeException {
-    Set<Account.Id> accounts = new HashSet<>();
-    for (Map.Entry<Account.Id, Collection<String>> e : byChangeFromIndex(
-        changeId).asMap().entrySet()) {
-      if (e.getValue().contains(label)) {
-        accounts.add(e.getKey());
-      }
-    }
-    return accounts;
-  }
-
   @Deprecated
   public ResultSet<Change.Id> queryFromIndex(final Account.Id accountId) {
     try {
@@ -380,6 +374,13 @@
     }
   }
 
+  private static void checkMutuallyExclusiveLabels(Set<String> labels) {
+    if (labels.containsAll(ImmutableSet.of(DEFAULT_LABEL, IGNORE_LABEL))) {
+      throw IllegalLabelException.mutuallyExclusiveLabels(DEFAULT_LABEL,
+          IGNORE_LABEL);
+    }
+  }
+
   private static void validateLabels(Set<String> labels) {
     if (labels == null) {
       return;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/ChangeEmail.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/ChangeEmail.java
index 367159d..803bcf8 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/ChangeEmail.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/ChangeEmail.java
@@ -16,6 +16,7 @@
 
 import static com.google.gerrit.server.notedb.ReviewerStateInternal.REVIEWER;
 
+import com.google.common.collect.Multimap;
 import com.google.gerrit.common.errors.EmailException;
 import com.google.gerrit.extensions.api.changes.ReviewInput.NotifyHandling;
 import com.google.gerrit.reviewdb.client.Account;
@@ -29,6 +30,7 @@
 import com.google.gerrit.reviewdb.client.Project;
 import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.StarredChangesUtil;
+import com.google.gerrit.server.account.AccountState;
 import com.google.gerrit.server.mail.ProjectWatch.Watchers;
 import com.google.gerrit.server.patch.PatchList;
 import com.google.gerrit.server.patch.PatchListEntry;
@@ -49,9 +51,11 @@
 
 import java.io.IOException;
 import java.text.MessageFormat;
+import java.util.Collection;
 import java.util.Collections;
 import java.util.Date;
 import java.util.HashSet;
+import java.util.Map;
 import java.util.Set;
 import java.util.TreeSet;
 
@@ -304,11 +308,22 @@
     }
 
     try {
-      // BCC anyone who has starred this change.
+      // BCC anyone who has starred this change
+      // and remove anyone who has ignored this change.
       //
-      for (Account.Id accountId : args.starredChangesUtil.byChangeFromIndex(
-          change.getId(), StarredChangesUtil.DEFAULT_LABEL)) {
-        super.add(RecipientType.BCC, accountId);
+      Multimap<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());
+          }
+        }
       }
     } catch (OrmException | NoSuchChangeException err) {
       // Just don't BCC everyone. Better to send a partial message to those
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/OutgoingEmail.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/OutgoingEmail.java
index 4834efd..04085b6 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/OutgoingEmail.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/OutgoingEmail.java
@@ -486,7 +486,7 @@
     return r.toString();
   }
 
-  private void removeUser(Account user) {
+  protected void removeUser(Account user) {
     String fromEmail = user.getPreferredEmail();
     for (Iterator<Address> j = smtpRcptTo.iterator(); j.hasNext();) {
       if (j.next().email.equals(fromEmail)) {