Merge "Add a User Preference to Receive only Plaintext Emails"

Documentation/intro-user.txt

@@ -644,6 +644,27 @@ on comments that you write yourself.
 +
 Email notifications are disabled.
 
+- [[email-format]]`Email Format`:
++
+This setting controls the email format Gerrit sends. Note that this
+setting has no effect if the administrator has disabled HTML emails
+for the Gerrit instance.
++
+** `Plaintext Only`:
++
+Email notifications contain only plaintext content.
+Note that you will receive an individual email where you are the sole
+recipient while all other users that have HTML enabled will receive an
+email where they are all listed as recipients making it easier to start
+a discussion thread over email.
+The effect of this scenario is reduced if the administrator has enabled
+inbound email for this instance as replies are then parsed by Gerrit and
+Gerrit takes care of sending out new notification emails.
++
+** `HTML and Plaintext`:
++
+Email notifications contain both HTML and plaintext content.
+
 - [[default-base-for-merges]]`Default Base For Merges`:
 +
 This setting controls which base should be pre-selected in the

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/GeneralPreferencesIT.java

@@ -26,6 +26,7 @@ import com.google.gerrit.extensions.client.GeneralPreferencesInfo.DateFormat;
 import com.google.gerrit.extensions.client.GeneralPreferencesInfo.DefaultBase;
 import com.google.gerrit.extensions.client.GeneralPreferencesInfo.DiffView;
 import com.google.gerrit.extensions.client.GeneralPreferencesInfo.DownloadCommand;
+import com.google.gerrit.extensions.client.GeneralPreferencesInfo.EmailFormat;
 import com.google.gerrit.extensions.client.GeneralPreferencesInfo.EmailStrategy;
 import com.google.gerrit.extensions.client.GeneralPreferencesInfo.ReviewCategoryStrategy;
 import com.google.gerrit.extensions.client.GeneralPreferencesInfo.TimeFormat;
@@ -85,6 +86,7 @@ public class GeneralPreferencesIT extends AbstractDaemonTest {
     i.dateFormat = DateFormat.US;
     i.timeFormat = TimeFormat.HHMM_24;
     i.emailStrategy = EmailStrategy.DISABLED;
+    i.emailFormat = EmailFormat.PLAINTEXT;
     i.defaultBaseForMerges = DefaultBase.AUTO_MERGE;
     i.expandInlineDiffs ^= true;
     i.highlightAssigneeInChangeTable ^= true;

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/NotificationMailFormatIT.java

@@ -0,0 +1,69 @@
+// Copyright (C) 2017 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.acceptance.server.mail;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.PushOneCommit;
+import com.google.gerrit.extensions.api.changes.ReviewInput;
+import com.google.gerrit.extensions.client.GeneralPreferencesInfo;
+import com.google.gerrit.extensions.client.GeneralPreferencesInfo.EmailFormat;
+import org.junit.Test;
+
+public class NotificationMailFormatIT extends AbstractDaemonTest {
+
+  @Test
+  public void userReceivesPlaintextEmail() throws Exception {
+    // Set user preference to receive only plaintext content
+    GeneralPreferencesInfo i = new GeneralPreferencesInfo();
+    i.emailFormat = EmailFormat.PLAINTEXT;
+    gApi.accounts().id(admin.getId().toString()).setPreferences(i);
+
+    // Create change as admin and review as user
+    PushOneCommit.Result r = createChange();
+    setApiUser(user);
+    gApi.changes()
+        .id(r.getChangeId())
+        .revision(r.getCommit().name())
+        .review(ReviewInput.recommend());
+
+    // Check that admin has received only plaintext content
+    assertThat(sender.getMessages()).hasSize(1);
+    assertThat(sender.getMessages().get(0).body()).isNotNull();
+    assertThat(sender.getMessages().get(0).htmlBody()).isNull();
+
+    // Reset user preference
+    setApiUser(admin);
+    i.emailFormat = EmailFormat.HTML_PLAINTEXT;
+    gApi.accounts().id(admin.getId().toString()).setPreferences(i);
+  }
+
+  @Test
+  public void userReceivesHtmlAndPlaintextEmail() throws Exception {
+    // Create change as admin and review as user
+    PushOneCommit.Result r = createChange();
+    setApiUser(user);
+    gApi.changes()
+        .id(r.getChangeId())
+        .revision(r.getCommit().name())
+        .review(ReviewInput.recommend());
+
+    // Check that admin has received both HTML and plaintext content
+    assertThat(sender.getMessages()).hasSize(1);
+    assertThat(sender.getMessages().get(0).body()).isNotNull();
+    assertThat(sender.getMessages().get(0).htmlBody()).isNotNull();
+  }
+}

gerrit-extension-api/src/main/java/com/google/gerrit/extensions/client/GeneralPreferencesInfo.java

@@ -87,6 +87,11 @@ public class GeneralPreferencesInfo {
     DISABLED
   }
 
+  public enum EmailFormat {
+    PLAINTEXT,
+    HTML_PLAINTEXT
+  }
+
   public enum DefaultBase {
     AUTO_MERGE(null),
     FIRST_PARENT(-1);
@@ -150,6 +155,7 @@ public class GeneralPreferencesInfo {
   public List<String> changeTable;
   public Map<String, String> urlAliases;
   public EmailStrategy emailStrategy;
+  public EmailFormat emailFormat;
   public DefaultBase defaultBaseForMerges;
 
   public boolean isShowInfoInReviewCategory() {
@@ -191,12 +197,20 @@ public class GeneralPreferencesInfo {
     return emailStrategy;
   }
 
+  public EmailFormat getEmailFormat() {
+    if (emailFormat == null) {
+      return EmailFormat.HTML_PLAINTEXT;
+    }
+    return emailFormat;
+  }
+
   public static GeneralPreferencesInfo defaults() {
     GeneralPreferencesInfo p = new GeneralPreferencesInfo();
     p.changesPerPage = DEFAULT_PAGESIZE;
     p.showSiteHeader = true;
     p.useFlashClipboard = true;
     p.emailStrategy = EmailStrategy.ENABLED;
+    p.emailFormat = EmailFormat.HTML_PLAINTEXT;
     p.reviewCategoryStrategy = ReviewCategoryStrategy.NONE;
     p.downloadScheme = null;
     p.downloadCommand = DownloadCommand.CHECKOUT;

gerrit-gwtui-common/src/main/java/com/google/gerrit/client/info/GeneralPreferences.java

@@ -22,6 +22,7 @@ import com.google.gerrit.extensions.client.GeneralPreferencesInfo.DateFormat;
 import com.google.gerrit.extensions.client.GeneralPreferencesInfo.DefaultBase;
 import com.google.gerrit.extensions.client.GeneralPreferencesInfo.DiffView;
 import com.google.gerrit.extensions.client.GeneralPreferencesInfo.DownloadCommand;
+import com.google.gerrit.extensions.client.GeneralPreferencesInfo.EmailFormat;
 import com.google.gerrit.extensions.client.GeneralPreferencesInfo.EmailStrategy;
 import com.google.gerrit.extensions.client.GeneralPreferencesInfo.ReviewCategoryStrategy;
 import com.google.gerrit.extensions.client.GeneralPreferencesInfo.TimeFormat;
@@ -52,6 +53,7 @@ public class GeneralPreferences extends JavaScriptObject {
     p.legacycidInChangeTable(d.legacycidInChangeTable);
     p.muteCommonPathPrefixes(d.muteCommonPathPrefixes);
     p.signedOffBy(d.signedOffBy);
+    p.emailFormat(d.emailFormat);
     p.reviewCategoryStrategy(d.getReviewCategoryStrategy());
     p.diffView(d.getDiffView());
     p.emailStrategy(d.emailStrategy);
@@ -132,6 +134,13 @@ public class GeneralPreferences extends JavaScriptObject {
 
   private native String emailStrategyRaw() /*-{ return this.email_strategy }-*/;
 
+  public final EmailFormat emailFormat() {
+    String s = emailFormatRaw();
+    return s != null ? EmailFormat.valueOf(s) : null;
+  }
+
+  private native String emailFormatRaw() /*-{ return this.email_format }-*/;
+
   public final DefaultBase defaultBaseForMerges() {
     String s = defaultBaseForMergesRaw();
     return s != null ? DefaultBase.valueOf(s) : null;
@@ -203,6 +212,12 @@ public class GeneralPreferences extends JavaScriptObject {
 
   private native void emailStrategyRaw(String s) /*-{ this.email_strategy = s }-*/;
 
+  public final void emailFormat(EmailFormat f) {
+    emailFormatRaw(f != null ? f.toString() : null);
+  }
+
+  private native void emailFormatRaw(String s) /*-{ this.email_format = s }-*/;
+
   public final void defaultBaseForMerges(DefaultBase b) {
     defaultBaseForMergesRaw(b != null ? b.toString() : null);
   }

gerrit-gwtui/src/main/java/com/google/gerrit/client/account/AccountConstants.java

@@ -287,6 +287,12 @@ public interface AccountConstants extends Constants {
 
   String emailFieldLabel();
 
+  String emailFormatFieldLabel();
+
+  String messagePlaintextOnly();
+
+  String messageHtmlPlaintext();
+
   String defaultBaseForMerges();
 
   String autoMerge();

gerrit-gwtui/src/main/java/com/google/gerrit/client/account/AccountConstants.properties

@@ -19,6 +19,10 @@ messageCCMeOnMyComments = Every Comment
 messageEnabled = Only Comments Left By Others
 messageDisabled = None
 
+emailFormatFieldLabel = Email Format:
+messagePlaintextOnly = Plaintext Only
+messageHtmlPlaintext = HTML and Plaintext
+
 defaultBaseForMerges = Default Base For Merges:
 autoMerge = Auto Merge
 firstParent = First Parent

gerrit-gwtui/src/main/java/com/google/gerrit/client/account/MyPreferencesScreen.java

@@ -61,6 +61,7 @@ public class MyPreferencesScreen extends SettingsScreen {
   private ListBox reviewCategoryStrategy;
   private ListBox diffView;
   private ListBox emailStrategy;
+  private ListBox emailFormat;
   private ListBox defaultBaseForMerges;
   private StringListPanel myMenus;
   private Button save;
@@ -102,6 +103,12 @@ public class MyPreferencesScreen extends SettingsScreen {
     emailStrategy.addItem(
         Util.C.messageDisabled(), GeneralPreferencesInfo.EmailStrategy.DISABLED.name());
 
+    emailFormat = new ListBox();
+    emailFormat.addItem(
+        Util.C.messagePlaintextOnly(), GeneralPreferencesInfo.EmailFormat.PLAINTEXT.name());
+    emailFormat.addItem(
+        Util.C.messageHtmlPlaintext(), GeneralPreferencesInfo.EmailFormat.HTML_PLAINTEXT.name());
+
     defaultBaseForMerges = new ListBox();
     defaultBaseForMerges.addItem(
         Util.C.autoMerge(), GeneralPreferencesInfo.DefaultBase.AUTO_MERGE.name());
@@ -157,7 +164,7 @@ public class MyPreferencesScreen extends SettingsScreen {
     signedOffBy = new CheckBox(Util.C.signedOffBy());
 
     boolean flashClippy = !UserAgent.hasJavaScriptClipboard() && UserAgent.Flash.isInstalled();
-    final Grid formGrid = new Grid(13 + (flashClippy ? 1 : 0), 2);
+    final Grid formGrid = new Grid(14 + (flashClippy ? 1 : 0), 2);
 
     int row = 0;
 
@@ -177,6 +184,10 @@ public class MyPreferencesScreen extends SettingsScreen {
     formGrid.setWidget(row, fieldIdx, emailStrategy);
     row++;
 
+    formGrid.setText(row, labelIdx, Util.C.emailFormatFieldLabel());
+    formGrid.setWidget(row, fieldIdx, emailFormat);
+    row++;
+
     formGrid.setText(row, labelIdx, Util.C.defaultBaseForMerges());
     formGrid.setWidget(row, fieldIdx, defaultBaseForMerges);
     row++;
@@ -250,6 +261,7 @@ public class MyPreferencesScreen extends SettingsScreen {
     e.listenTo(diffView);
     e.listenTo(reviewCategoryStrategy);
     e.listenTo(emailStrategy);
+    e.listenTo(emailFormat);
     e.listenTo(defaultBaseForMerges);
   }
 
@@ -287,6 +299,7 @@ public class MyPreferencesScreen extends SettingsScreen {
     reviewCategoryStrategy.setEnabled(on);
     diffView.setEnabled(on);
     emailStrategy.setEnabled(on);
+    emailFormat.setEnabled(on);
     defaultBaseForMerges.setEnabled(on);
   }
 
@@ -314,6 +327,7 @@ public class MyPreferencesScreen extends SettingsScreen {
         p.reviewCategoryStrategy());
     setListBox(diffView, GeneralPreferencesInfo.DiffView.SIDE_BY_SIDE, p.diffView());
     setListBox(emailStrategy, GeneralPreferencesInfo.EmailStrategy.ENABLED, p.emailStrategy());
+    setListBox(emailFormat, GeneralPreferencesInfo.EmailFormat.HTML_PLAINTEXT, p.emailFormat());
     setListBox(
         defaultBaseForMerges,
         GeneralPreferencesInfo.DefaultBase.FIRST_PARENT,
@@ -414,6 +428,12 @@ public class MyPreferencesScreen extends SettingsScreen {
             GeneralPreferencesInfo.EmailStrategy.ENABLED,
             GeneralPreferencesInfo.EmailStrategy.values()));
 
+    p.emailFormat(
+        getListBox(
+            emailFormat,
+            GeneralPreferencesInfo.EmailFormat.HTML_PLAINTEXT,
+            GeneralPreferencesInfo.EmailFormat.values()));
+
     p.defaultBaseForMerges(
         getListBox(
             defaultBaseForMerges,

gerrit-server/BUILD

@@ -86,6 +86,7 @@ java_library(
 
 TESTUTIL_DEPS = [
     ":server",
+    "//gerrit-common:annotations",
     "//gerrit-common:server",
     "//gerrit-cache-h2:cache-h2",
     "//gerrit-extension-api:api",
@@ -186,7 +187,6 @@ java_library(
         ":testutil",
         "//gerrit-antlr:query_exception",
         "//gerrit-antlr:query_parser",
-        "//gerrit-common:annotations",
         "//gerrit-server/src/main/prolog:common",
         "//lib/antlr:java_runtime",
     ],
@@ -201,7 +201,6 @@ junit_tests(
         ":testutil",
         "//gerrit-antlr:query_exception",
         "//gerrit-antlr:query_parser",
-        "//gerrit-common:annotations",
         "//gerrit-server/src/main/prolog:common",
         "//lib/antlr:java_runtime",
     ],
@@ -219,7 +218,6 @@ junit_tests(
     deps = TESTUTIL_DEPS + [
         ":testutil",
         "//gerrit-antlr:query_exception",
-        "//gerrit-common:annotations",
         "//gerrit-patch-jgit:server",
         "//gerrit-server/src/main/prolog:common",
         "//lib:args4j",

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

@@ -19,12 +19,14 @@ import static com.google.gerrit.extensions.client.GeneralPreferencesInfo.EmailSt
 import static com.google.gerrit.extensions.client.GeneralPreferencesInfo.EmailStrategy.DISABLED;
 import static java.nio.charset.StandardCharsets.UTF_8;
 
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableListMultimap;
 import com.google.common.collect.ListMultimap;
 import com.google.gerrit.common.errors.EmailException;
 import com.google.gerrit.extensions.api.changes.NotifyHandling;
 import com.google.gerrit.extensions.api.changes.RecipientType;
 import com.google.gerrit.extensions.client.GeneralPreferencesInfo;
+import com.google.gerrit.extensions.client.GeneralPreferencesInfo.EmailFormat;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.UserIdentity;
 import com.google.gerrit.server.account.AccountState;
@@ -71,6 +73,7 @@ public abstract class OutgoingEmail {
   private final HashSet<Account.Id> rcptTo = new HashSet<>();
   private final Map<String, EmailHeader> headers;
   private final Set<Address> smtpRcptTo = new HashSet<>();
+  private final Set<Address> smtpRcptToPlaintextOnly = new HashSet<>();
   private Address smtpFromAddress;
   private StringBuilder textBody;
   private StringBuilder htmlBody;
@@ -145,14 +148,19 @@ public abstract class OutgoingEmail {
         }
 
         // Check the preferences of all recipients. If any user has disabled
-        // his email notifications then drop him from recipients' list
+        // his email notifications then drop him from recipients' list.
+        // In addition, check if users only want to receive plaintext email.
         for (Account.Id id : rcptTo) {
           Account thisUser = args.accountCache.get(id).getAccount();
           GeneralPreferencesInfo prefs = thisUser.getGeneralPreferencesInfo();
           if (prefs == null || prefs.getEmailStrategy() == DISABLED) {
             removeUser(thisUser);
+          } else if (useHtml() && prefs.getEmailFormat() == EmailFormat.PLAINTEXT) {
+            removeUser(thisUser);
+            smtpRcptToPlaintextOnly.add(
+                new Address(thisUser.getFullName(), thisUser.getPreferredEmail()));
           }
-          if (smtpRcptTo.isEmpty()) {
+          if (smtpRcptTo.isEmpty() && smtpRcptToPlaintextOnly.isEmpty()) {
             return;
           }
         }
@@ -166,6 +174,7 @@ public abstract class OutgoingEmail {
       va.headers = headers;
 
       va.body = textPart;
+
       if (useHtml()) {
         va.htmlBody = htmlBody.toString();
       } else {
@@ -180,7 +189,27 @@ public abstract class OutgoingEmail {
         }
       }
 
-      args.emailSender.send(va.smtpFromAddress, va.smtpRcptTo, va.headers, va.body, va.htmlBody);
+      if (!smtpRcptTo.isEmpty()) {
+        args.emailSender.send(va.smtpFromAddress, va.smtpRcptTo, va.headers, va.body, va.htmlBody);
+      }
+
+      // Send plaintext emails: This is different than HTML emails as users have explicitly agreed
+      // to receive this email as plaintext only at the cost of not being in the same email thread
+      // as the other recipients.
+      for (Address a : smtpRcptToPlaintextOnly) {
+        // Shallow copy map
+        Map<String, EmailHeader> shallowCopy = new HashMap<>();
+        shallowCopy.putAll(headers);
+        // Remove To and Cc
+        shallowCopy.remove(HDR_TO);
+        shallowCopy.remove(HDR_CC);
+        // Add new To
+        EmailHeader.AddressList to = new EmailHeader.AddressList();
+        to.add(a);
+        shallowCopy.put(HDR_TO, to);
+        // Send message
+        args.emailSender.send(va.smtpFromAddress, ImmutableList.of(a), shallowCopy, va.body);
+      }
     }
   }
 

gerrit-server/src/test/java/com/google/gerrit/testutil/FakeEmailSender.java

@@ -19,6 +19,7 @@ import static java.util.stream.Collectors.toList;
 import com.google.auto.value.AutoValue;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
+import com.google.gerrit.common.Nullable;
 import com.google.gerrit.common.errors.EmailException;
 import com.google.gerrit.server.git.WorkQueue;
 import com.google.gerrit.server.mail.Address;
@@ -58,9 +59,13 @@ public class FakeEmailSender implements EmailSender {
   @AutoValue
   public abstract static class Message {
     private static Message create(
-        Address from, Collection<Address> rcpt, Map<String, EmailHeader> headers, String body) {
+        Address from,
+        Collection<Address> rcpt,
+        Map<String, EmailHeader> headers,
+        String body,
+        String htmlBody) {
       return new AutoValue_FakeEmailSender_Message(
-          from, ImmutableList.copyOf(rcpt), ImmutableMap.copyOf(headers), body);
+          from, ImmutableList.copyOf(rcpt), ImmutableMap.copyOf(headers), body, htmlBody);
     }
 
     public abstract Address from();
@@ -70,6 +75,9 @@ public class FakeEmailSender implements EmailSender {
     public abstract ImmutableMap<String, EmailHeader> headers();
 
     public abstract String body();
+
+    @Nullable
+    public abstract String htmlBody();
   }
 
   private final WorkQueue workQueue;
@@ -95,7 +103,18 @@ public class FakeEmailSender implements EmailSender {
   public void send(
       Address from, Collection<Address> rcpt, Map<String, EmailHeader> headers, String body)
       throws EmailException {
-    messages.add(Message.create(from, rcpt, headers, body));
+    send(from, rcpt, headers, body, null);
+  }
+
+  @Override
+  public void send(
+      Address from,
+      Collection<Address> rcpt,
+      Map<String, EmailHeader> headers,
+      String body,
+      String htmlBody)
+      throws EmailException {
+    messages.add(Message.create(from, rcpt, headers, body, htmlBody));
   }
 
   public void clear() {

polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.html

@@ -174,6 +174,17 @@ limitations under the License.
               </select>
             </span>
           </section>
+          <section>
+            <span class="title">Email Format</span>
+            <span class="value">
+              <select
+                  is="gr-select"
+                  bind-value="{{_localPrefs.email_format}}">
+                <option value="HTML_PLAINTEXT">HTML and Plaintext</option>
+                <option value="PLAINTEXT">Plaintext Only</option>
+              </select>
+            </span>
+          </section>
           <section>
             <span class="title">Diff View</span>
             <span class="value">