Allow user preference to receive emails only when in attention set

This allows specifying such a user preference that would apply too all
change emails. It wouldn't apply to other emails that don't relate to
changes.

Specifically, if the preference is configured, and the user is not in
the attention set, they will not receive an email regarding that change.

Change-Id: Ib6529a710f3d2853ef03090e7d7454676e140e92
This commit is contained in:
Gal Paikin
2020-10-09 18:40:52 +02:00
parent 4a663b2b38
commit fd238889b3
4 changed files with 78 additions and 13 deletions

View File

@@ -2807,9 +2807,11 @@ empty, which will default columns as determined by the frontend.
|`email_strategy` ||
The type of email strategy to use. On `ENABLED`, the user will receive emails
from Gerrit. On `CC_ON_OWN_COMMENTS` the user will also receive emails for
their own comments. On `DISABLED` the user will not receive any email
notifications from Gerrit.
Allowed values are `ENABLED`, `CC_ON_OWN_COMMENTS`, `DISABLED`.
their own comments. On `ATTENTION_SET_ONLY`, on emails about changes, the user
will receive emails only if they are in the attention set of that change.
On `DISABLED` the user will not receive any email notifications from Gerrit.
Allowed values are `ENABLED`, `CC_ON_OWN_COMMENTS`, `ATTENTION_SET_ONLY`,
`DISABLED`.
|`default_base_for_merges` ||
The base which should be pre-selected in the 'Diff Against' drop-down
list when the change screen is opened for a merge commit.
@@ -2870,9 +2872,11 @@ empty, which will default columns as determined by the frontend.
|`email_strategy` |optional|
The type of email strategy to use. On `ENABLED`, the user will receive emails
from Gerrit. On `CC_ON_OWN_COMMENTS` the user will also receive emails for
their own comments. On `DISABLED` the user will not receive any email
notifications from Gerrit.
Allowed values are `ENABLED`, `CC_ON_OWN_COMMENTS`, `DISABLED`.
their own comments. On `ATTENTION_SET_ONLY`, on emails about changes, the user
will receive emails only if they are in the attention set of that change.
On `DISABLED` the user will not receive any email notifications from Gerrit.
Allowed values are `ENABLED`, `CC_ON_OWN_COMMENTS`, `ATTENTION_SET_ONLY`,
`DISABLED`.
|`default_base_for_merges` |optional|
The base which should be pre-selected in the 'Diff Against' drop-down
list when the change screen is opened for a merge commit.

View File

@@ -75,6 +75,7 @@ public class GeneralPreferencesInfo {
public enum EmailStrategy {
ENABLED,
CC_ON_OWN_COMMENTS,
ATTENTION_SET_ONLY,
DISABLED
}

View File

@@ -33,9 +33,11 @@ import com.google.gerrit.exceptions.EmailException;
import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.extensions.api.changes.NotifyHandling;
import com.google.gerrit.extensions.api.changes.RecipientType;
import com.google.gerrit.extensions.client.GeneralPreferencesInfo.EmailStrategy;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.mail.MailHeader;
import com.google.gerrit.server.StarredChangesUtil;
import com.google.gerrit.server.account.AccountState;
import com.google.gerrit.server.mail.send.ProjectWatch.Watchers;
import com.google.gerrit.server.notedb.ReviewerStateInternal;
import com.google.gerrit.server.patch.PatchList;
@@ -56,6 +58,7 @@ import java.util.Date;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.TreeSet;
import java.util.stream.Collectors;
@@ -75,6 +78,7 @@ public abstract class ChangeEmail extends NotificationEmail {
return ea.changeDataFactory.create(project, id);
}
private final Set<Account.Id> currentAttentionSet;
protected final Change change;
protected final ChangeData changeData;
protected ListMultimap<Account.Id, String> stars;
@@ -92,6 +96,7 @@ public abstract class ChangeEmail extends NotificationEmail {
this.changeData = changeData;
this.change = changeData.change();
this.emailOnlyAuthors = false;
this.currentAttentionSet = getAttentionSet();
}
@Override
@@ -387,9 +392,19 @@ public abstract class ChangeEmail extends NotificationEmail {
@Override
protected void add(RecipientType rt, Account.Id to) {
if (!emailOnlyAuthors || authors.contains(to)) {
super.add(rt, to);
Optional<AccountState> accountState = args.accountCache.get(to);
if (!accountState.isPresent()) {
return;
}
if (accountState.get().generalPreferences().getEmailStrategy()
== EmailStrategy.ATTENTION_SET_ONLY
&& !currentAttentionSet.contains(to)) {
return;
}
if (emailOnlyAuthors && !authors.contains(to)) {
return;
}
super.add(rt, to);
}
@Override
@@ -487,8 +502,8 @@ public abstract class ChangeEmail extends NotificationEmail {
for (String reviewer : getEmailsByState(ReviewerStateInternal.CC)) {
footers.add(MailHeader.CC.withDelimiter() + reviewer);
}
for (String attentionUser : getAttentionSet()) {
footers.add(MailHeader.ATTENTION.withDelimiter() + attentionUser);
for (Account.Id attentionUser : currentAttentionSet) {
footers.add(MailHeader.ATTENTION.withDelimiter() + getNameEmailFor(attentionUser));
}
}
@@ -515,12 +530,12 @@ public abstract class ChangeEmail extends NotificationEmail {
return reviewers;
}
private Set<String> getAttentionSet() {
Set<String> attentionSet = new TreeSet<>();
private Set<Account.Id> getAttentionSet() {
Set<Account.Id> attentionSet = new TreeSet<>();
try {
attentionSet =
additionsOnly(changeData.attentionSet()).stream()
.map(a -> getNameEmailFor(a.account()))
.map(a -> a.account())
.collect(Collectors.toSet());
} catch (StorageException e) {
logger.atWarning().withCause(e).log("Cannot get change attention set");

View File

@@ -41,6 +41,8 @@ import com.google.gerrit.extensions.api.changes.AttentionSetInput;
import com.google.gerrit.extensions.api.changes.DeleteReviewerInput;
import com.google.gerrit.extensions.api.changes.HashtagsInput;
import com.google.gerrit.extensions.api.changes.ReviewInput;
import com.google.gerrit.extensions.client.GeneralPreferencesInfo;
import com.google.gerrit.extensions.client.GeneralPreferencesInfo.EmailStrategy;
import com.google.gerrit.extensions.client.ReviewerState;
import com.google.gerrit.extensions.client.Side;
import com.google.gerrit.extensions.restapi.BadRequestException;
@@ -1360,6 +1362,49 @@ public class AttentionSetIT extends AbstractDaemonTest {
sender.clear();
}
@Test
public void attentionSetWithEmailFilter() throws Exception {
PushOneCommit.Result r = createChange();
// Add preference for the user such that they only receive an email on changes that require
// their attention.
requestScopeOperations.setApiUser(user.id());
GeneralPreferencesInfo prefs = gApi.accounts().self().getPreferences();
prefs.emailStrategy = EmailStrategy.ATTENTION_SET_ONLY;
gApi.accounts().self().setPreferences(prefs);
requestScopeOperations.setApiUser(admin.id());
// Add user to attention set. They receive an email since they are in the attention set.
change(r).addReviewer(user.id().toString());
assertThat(sender.getMessages()).isNotEmpty();
sender.clear();
// Irrelevant reply, User is still in the attention set, thus got another email.
change(r).current().review(ReviewInput.approve());
assertThat(sender.getMessages()).isNotEmpty();
sender.clear();
// Abandon the change which removes user from attention set; the user doesn't receive an email
// since they are not in the attention set.
change(r).abandon();
assertThat(sender.getMessages()).isEmpty();
}
@Test
public void attentionSetWithEmailFilterImpactingOnlyChangeEmails() throws Exception {
// Add preference for the user such that they only receive an email on changes that require
// their attention.
requestScopeOperations.setApiUser(user.id());
GeneralPreferencesInfo prefs = gApi.accounts().self().getPreferences();
prefs.emailStrategy = EmailStrategy.ATTENTION_SET_ONLY;
gApi.accounts().self().setPreferences(prefs);
requestScopeOperations.setApiUser(admin.id());
// Ensure emails that don't relate to changes are still sent.
gApi.accounts().id(user.id().get()).generateHttpPassword();
assertThat(sender.getMessages()).isNotEmpty();
}
private List<AttentionSetUpdate> getAttentionSetUpdatesForUser(
PushOneCommit.Result r, TestAccount account) {
return getAttentionSetUpdates(r.getChange().getId()).stream()