Merge "Add an emailOnlyAuthors property to AccountGroups"

8 files changed, 96 insertions, 15 deletions

diff --git a/gerrit-common/src/main/java/com/google/gerrit/common/data/GroupOptions.java b/gerrit-common/src/main/java/com/google/gerrit/common/data/GroupOptions.java
index 1dc4004cef..82e061846f 100644
--- a/gerrit-common/src/main/java/com/google/gerrit/common/data/GroupOptions.java
+++ b/gerrit-common/src/main/java/com/google/gerrit/common/data/GroupOptions.java
@@ -22,15 +22,22 @@ import com.google.gerrit.reviewdb.AccountGroup;
 public class GroupOptions {
 
   private boolean visibleToAll;
+  private boolean emailOnlyAuthors;
 
   protected GroupOptions() {
   }
 
-  public GroupOptions(final boolean visibleToAll) {
+  public GroupOptions(final boolean visibleToAll,
+       final boolean emailOnlyAuthors) {
     this.visibleToAll = visibleToAll;
+    this.emailOnlyAuthors = emailOnlyAuthors;
   }
 
   public boolean isVisibleToAll() {
     return visibleToAll;
   }
+
+  public boolean isEmailOnlyAuthors() {
+    return emailOnlyAuthors;
+  }
 }
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/AccountGroupScreen.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/AccountGroupScreen.java
index 56abefa4d6..431faffd2c 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/AccountGroupScreen.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/AccountGroupScreen.java
@@ -99,6 +99,7 @@ public class AccountGroupScreen extends AccountScreen {
 
   private Panel groupOptionsPanel;
   private CheckBox visibleToAllCheckBox;
+  private CheckBox emailOnlyAuthors;
   private Button saveGroupOptions;
 
   public AccountGroupScreen(final AccountGroup.Id toShow) {
@@ -133,6 +134,7 @@ public class AccountGroupScreen extends AccountScreen {
     initDescription();
     initGroupOptions();
     initGroupType();
+
     initMemberList();
     initIncludeList();
     initExternal();
@@ -147,6 +149,7 @@ public class AccountGroupScreen extends AccountScreen {
     externalNameFilter.setEnabled(canModify);
     externalNameSearch.setEnabled(canModify);
     visibleToAllCheckBox.setEnabled(canModify);
+    emailOnlyAuthors.setEnabled(canModify);
   }
 
   private void initName() {
@@ -242,15 +245,25 @@ public class AccountGroupScreen extends AccountScreen {
     groupOptionsPanel.add(new SmallHeading(Util.C.headingGroupOptions()));
 
     visibleToAllCheckBox = new CheckBox(Util.C.isVisibleToAll());
+    visibleToAllCheckBox.setEnabled(false);
     groupOptionsPanel.add(visibleToAllCheckBox);
 
+    emailOnlyAuthors = new CheckBox(Util.C.emailOnlyAuthors());
+    emailOnlyAuthors.setEnabled(false);
+
+    final VerticalPanel vp = new VerticalPanel();
+    vp.add(new Label(Util.C.descriptionNotifications()));
+    vp.add(emailOnlyAuthors);
+    groupOptionsPanel.add(vp);
+
     saveGroupOptions = new Button(Util.C.buttonSaveGroupOptions());
     saveGroupOptions.setEnabled(false);
     saveGroupOptions.addClickHandler(new ClickHandler() {
       @Override
       public void onClick(final ClickEvent event) {
         final GroupOptions groupOptions =
-            new GroupOptions(visibleToAllCheckBox.getValue());
+            new GroupOptions(visibleToAllCheckBox.getValue(),
+              emailOnlyAuthors.getValue());
         Util.GROUP_SVC.changeGroupOptions(groupId, groupOptions,
             new GerritCallback<VoidResult>() {
               public void onSuccess(final VoidResult result) {
@@ -263,7 +276,9 @@ public class AccountGroupScreen extends AccountScreen {
 
     add(groupOptionsPanel);
 
-    new OnEditEnabler(saveGroupOptions, visibleToAllCheckBox);
+    final OnEditEnabler enabler = new OnEditEnabler(saveGroupOptions);
+    enabler.listenTo(visibleToAllCheckBox);
+    enabler.listenTo(emailOnlyAuthors);
   }
 
   private void initGroupType() {
@@ -528,6 +543,11 @@ public class AccountGroupScreen extends AccountScreen {
     }
     descTxt.setText(group.getDescription());
 
+    visibleToAllCheckBox.setValue(group.isVisibleToAll());
+    visibleToAllCheckBox.setEnabled(true);
+    emailOnlyAuthors.setValue(group.isEmailOnlyAuthors());
+    emailOnlyAuthors.setEnabled(true);
+
     switch (group.getType()) {
       case INTERNAL:
         accounts = result.accounts;
@@ -543,8 +563,6 @@ public class AccountGroupScreen extends AccountScreen {
     }
 
     setType(group.getType());
-
-    visibleToAllCheckBox.setValue(group.isVisibleToAll());
   }
 
   void doAddNewMember() {
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/AdminConstants.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/AdminConstants.java
index ca191c6470..4631a29b36 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/AdminConstants.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/AdminConstants.java
@@ -41,6 +41,8 @@ public interface AdminConstants extends Constants {
   String requireChangeID();
   String headingGroupOptions();
   String isVisibleToAll();
+  String emailOnlyAuthors();
+  String descriptionNotifications();
   String buttonSaveGroupOptions();
 
   String headingOwner();
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/AdminConstants.properties b/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/AdminConstants.properties
index 94bb87f6be..4bad368056 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/AdminConstants.properties
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/AdminConstants.properties
@@ -24,6 +24,9 @@ headingGroupOptions = Group Options
 isVisibleToAll = Make group visible to all registered users.
 buttonSaveGroupOptions = Save Group Options
 
+emailOnlyAuthors = Authors
+descriptionNotifications = Send email notifications about comments and actions by users in this group only to:
+
 headingOwner = Owners
 headingParentProjectName = Rights Inherit From
 headingDescription = Description
diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/account/GroupAdminServiceImpl.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/account/GroupAdminServiceImpl.java
index b3918d3dad..9e8a24d603 100644
--- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/account/GroupAdminServiceImpl.java
+++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/account/GroupAdminServiceImpl.java
@@ -145,6 +145,7 @@ class GroupAdminServiceImpl extends BaseServiceImplementation implements
         final AccountGroup group = db.accountGroups().get(groupId);
         assertAmGroupOwner(db, group);
         group.setVisibleToAll(groupOptions.isVisibleToAll());
+        group.setEmailOnlyAuthors(groupOptions.isEmailOnlyAuthors());
         db.accountGroups().update(Collections.singleton(group));
         groupCache.evict(group);
         return VoidResult.INSTANCE;
diff --git a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/AccountGroup.java b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/AccountGroup.java
index a2ebe5a953..d2aceaa613 100644
--- a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/AccountGroup.java
+++ b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/AccountGroup.java
@@ -171,6 +171,11 @@ public final class AccountGroup {
   @Column(id = 7)
   protected boolean visibleToAll;
 
+  /** Comment and action email notifications by users in this group are only
+   *  sent to change authors. */
+  @Column(id = 8)
+  protected boolean emailOnlyAuthors;
+
   protected AccountGroup() {
   }
 
@@ -238,4 +243,12 @@ public final class AccountGroup {
   public boolean isVisibleToAll() {
     return visibleToAll;
   }
+
+  public boolean isEmailOnlyAuthors() {
+    return emailOnlyAuthors;
+  }
+
+  public void setEmailOnlyAuthors(boolean emailOnlyAuthors) {
+    this.emailOnlyAuthors = emailOnlyAuthors;
+  }
 }
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 3725a3c1df..f5a7870525 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
@@ -54,11 +54,28 @@ public abstract class ChangeEmail extends OutgoingEmail {
 
   protected ProjectState projectState;
   protected ChangeData changeData;
+  protected Set<Account.Id> authors;
+  protected boolean emailOnlyAuthors;
 
   protected ChangeEmail(EmailArguments ea, final Change c, final String mc) {
     super(ea, mc);
     change = c;
     changeData = change != null ? new ChangeData(change) : null;
+    emailOnlyAuthors = false;
+  }
+
+  public void setFrom(final Account.Id id) {
+    super.setFrom(id);
+
+    /** Is the from user in an email squelching group? */
+    final IdentifiedUser user =  args.identifiedUserFactory.create(id);
+    final Set<AccountGroup.Id> gids = user.getEffectiveGroups();
+    for (final AccountGroup.Id gid : gids) {
+      if (args.groupCache.get(gid).isEmailOnlyAuthors()) {
+        emailOnlyAuthors = true;
+        break;
+      }
+    }
   }
 
   public void setPatchSet(final PatchSet ps) {
@@ -123,6 +140,7 @@ public abstract class ChangeEmail extends OutgoingEmail {
         patchSetInfo = null;
       }
     }
+    authors = getAuthors();
 
     super.init();
 
@@ -264,13 +282,8 @@ public abstract class ChangeEmail extends OutgoingEmail {
 
   /** TO or CC all vested parties (change owner, patch set uploader, author). */
   protected void rcptToAuthors(final RecipientType rt) {
-    add(rt, change.getOwner());
-    if (patchSet != null) {
-      add(rt, patchSet.getUploader());
-    }
-    if (patchSetInfo != null) {
-      add(rt, patchSetInfo.getAuthor());
-      add(rt, patchSetInfo.getCommitter());
+    for (final Account.Id id : authors) {
+      add(rt, id);
     }
   }
 
@@ -380,6 +393,12 @@ public abstract class ChangeEmail extends OutgoingEmail {
     }
   }
 
+  protected void add(final RecipientType rt, final Account.Id to) {
+    if (! emailOnlyAuthors || authors.contains(to)) {
+      super.add(rt, to);
+    }
+  }
+
   protected boolean isVisibleTo(final Account.Id to) {
     return projectState == null
         || change == null
@@ -387,6 +406,21 @@ public abstract class ChangeEmail extends OutgoingEmail {
             .controlFor(change).isVisible();
   }
 
+  /** Find all users who are authors of any part of this change. */
+  protected Set<Account.Id> getAuthors() {
+    Set<Account.Id> authors = new HashSet<Account.Id>();
+
+    authors.add(change.getOwner());
+    if (patchSet != null) {
+      authors.add(patchSet.getUploader());
+    }
+    if (patchSetInfo != null) {
+      authors.add(patchSetInfo.getAuthor().getAccount());
+      authors.add(patchSetInfo.getCommitter().getAccount());
+    }
+    return authors;
+  }
+
   @Override
   protected void setupVelocityContext() {
     super.setupVelocityContext();
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/EmailArguments.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/EmailArguments.java
index f2ab9fa4a9..4a99eb47c0 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/EmailArguments.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/EmailArguments.java
@@ -19,6 +19,7 @@ import com.google.gerrit.reviewdb.ReviewDb;
 import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.IdentifiedUser.GenericFactory;
 import com.google.gerrit.server.account.AccountCache;
+import com.google.gerrit.server.account.GroupCache;
 import com.google.gerrit.server.config.CanonicalWebUrl;
 import com.google.gerrit.server.config.SitePaths;
 import com.google.gerrit.server.config.WildProjectName;
@@ -36,6 +37,7 @@ import javax.annotation.Nullable;
 class EmailArguments {
   final GitRepositoryManager server;
   final ProjectCache projectCache;
+  final GroupCache groupCache;
   final AccountCache accountCache;
   final PatchListCache patchListCache;
   final FromAddressGenerator fromAddressGenerator;
@@ -52,9 +54,9 @@ class EmailArguments {
 
   @Inject
   EmailArguments(GitRepositoryManager server, ProjectCache projectCache,
-      AccountCache accountCache, PatchListCache patchListCache,
-      FromAddressGenerator fromAddressGenerator, EmailSender emailSender,
-      PatchSetInfoFactory patchSetInfoFactory,
+      GroupCache groupCache, AccountCache accountCache,
+      PatchListCache patchListCache, FromAddressGenerator fromAddressGenerator,
+      EmailSender emailSender, PatchSetInfoFactory patchSetInfoFactory,
       GenericFactory identifiedUserFactory,
       @CanonicalWebUrl @Nullable Provider<String> urlProvider,
       @WildProjectName Project.NameKey wildProject,
@@ -63,6 +65,7 @@ class EmailArguments {
       SitePaths site) {
     this.server = server;
     this.projectCache = projectCache;
+    this.groupCache = groupCache;
     this.accountCache = accountCache;
     this.patchListCache = patchListCache;
     this.fromAddressGenerator = fromAddressGenerator;