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

This reverts commit 39777363a6.

Reason for revert: the frontend part of this change doesn't work.

Change-Id: I53138e939a1b98578787457e48743fdc8abf658c
This commit is contained in:
Wyatt Allen
2017-02-15 00:14:25 +00:00
committed by David Pursehouse
parent ab879094c0
commit 0aeceaa5be
12 changed files with 10 additions and 218 deletions

View File

@@ -644,27 +644,6 @@ 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

View File

@@ -26,7 +26,6 @@ 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;
@@ -86,7 +85,6 @@ 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;

View File

@@ -1,69 +0,0 @@
// 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();
}
}

View File

@@ -87,11 +87,6 @@ public class GeneralPreferencesInfo {
DISABLED
}
public enum EmailFormat {
PLAINTEXT,
HTML_PLAINTEXT
}
public enum DefaultBase {
AUTO_MERGE(null),
FIRST_PARENT(-1);
@@ -155,7 +150,6 @@ public class GeneralPreferencesInfo {
public List<String> changeTable;
public Map<String, String> urlAliases;
public EmailStrategy emailStrategy;
public EmailFormat emailFormat;
public DefaultBase defaultBaseForMerges;
public boolean isShowInfoInReviewCategory() {
@@ -197,20 +191,12 @@ 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;

View File

@@ -22,7 +22,6 @@ 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;
@@ -53,7 +52,6 @@ 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);
@@ -134,13 +132,6 @@ 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;
@@ -212,12 +203,6 @@ 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);
}

View File

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

View File

@@ -19,10 +19,6 @@ 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

View File

@@ -61,7 +61,6 @@ 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;
@@ -103,12 +102,6 @@ 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());
@@ -164,7 +157,7 @@ public class MyPreferencesScreen extends SettingsScreen {
signedOffBy = new CheckBox(Util.C.signedOffBy());
boolean flashClippy = !UserAgent.hasJavaScriptClipboard() && UserAgent.Flash.isInstalled();
final Grid formGrid = new Grid(14 + (flashClippy ? 1 : 0), 2);
final Grid formGrid = new Grid(13 + (flashClippy ? 1 : 0), 2);
int row = 0;
@@ -184,10 +177,6 @@ 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++;
@@ -261,7 +250,6 @@ public class MyPreferencesScreen extends SettingsScreen {
e.listenTo(diffView);
e.listenTo(reviewCategoryStrategy);
e.listenTo(emailStrategy);
e.listenTo(emailFormat);
e.listenTo(defaultBaseForMerges);
}
@@ -299,7 +287,6 @@ public class MyPreferencesScreen extends SettingsScreen {
reviewCategoryStrategy.setEnabled(on);
diffView.setEnabled(on);
emailStrategy.setEnabled(on);
emailFormat.setEnabled(on);
defaultBaseForMerges.setEnabled(on);
}
@@ -327,7 +314,6 @@ 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,
@@ -428,12 +414,6 @@ 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,

View File

@@ -86,7 +86,6 @@ java_library(
TESTUTIL_DEPS = [
":server",
"//gerrit-common:annotations",
"//gerrit-common:server",
"//gerrit-cache-h2:cache-h2",
"//gerrit-extension-api:api",
@@ -187,6 +186,7 @@ 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,6 +201,7 @@ junit_tests(
":testutil",
"//gerrit-antlr:query_exception",
"//gerrit-antlr:query_parser",
"//gerrit-common:annotations",
"//gerrit-server/src/main/prolog:common",
"//lib/antlr:java_runtime",
],
@@ -218,6 +219,7 @@ 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",

View File

@@ -19,14 +19,12 @@ 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;
@@ -73,7 +71,6 @@ 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;
@@ -148,19 +145,14 @@ public abstract class OutgoingEmail {
}
// Check the preferences of all recipients. If any user has disabled
// his email notifications then drop him from recipients' list.
// In addition, check if users only want to receive plaintext email.
// his email notifications then drop him from recipients' list
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() && smtpRcptToPlaintextOnly.isEmpty()) {
if (smtpRcptTo.isEmpty()) {
return;
}
}
@@ -174,7 +166,6 @@ public abstract class OutgoingEmail {
va.headers = headers;
va.body = textPart;
if (useHtml()) {
va.htmlBody = htmlBody.toString();
} else {
@@ -189,27 +180,7 @@ public abstract class OutgoingEmail {
}
}
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);
}
args.emailSender.send(va.smtpFromAddress, va.smtpRcptTo, va.headers, va.body, va.htmlBody);
}
}

View File

@@ -19,7 +19,6 @@ 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;
@@ -59,13 +58,9 @@ 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,
String htmlBody) {
Address from, Collection<Address> rcpt, Map<String, EmailHeader> headers, String body) {
return new AutoValue_FakeEmailSender_Message(
from, ImmutableList.copyOf(rcpt), ImmutableMap.copyOf(headers), body, htmlBody);
from, ImmutableList.copyOf(rcpt), ImmutableMap.copyOf(headers), body);
}
public abstract Address from();
@@ -75,9 +70,6 @@ public class FakeEmailSender implements EmailSender {
public abstract ImmutableMap<String, EmailHeader> headers();
public abstract String body();
@Nullable
public abstract String htmlBody();
}
private final WorkQueue workQueue;
@@ -103,18 +95,7 @@ public class FakeEmailSender implements EmailSender {
public void send(
Address from, Collection<Address> rcpt, Map<String, EmailHeader> headers, String body)
throws EmailException {
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));
messages.add(Message.create(from, rcpt, headers, body));
}
public void clear() {

View File

@@ -174,17 +174,6 @@ 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">