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
(cherry picked from commit fd238889b3)
This commit is contained in:
committed by
Luca Milanesio
parent
40621847c1
commit
d5e4b054fb
@@ -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.
|
||||
|
||||
@@ -75,6 +75,7 @@ public class GeneralPreferencesInfo {
|
||||
public enum EmailStrategy {
|
||||
ENABLED,
|
||||
CC_ON_OWN_COMMENTS,
|
||||
ATTENTION_SET_ONLY,
|
||||
DISABLED
|
||||
}
|
||||
|
||||
|
||||
@@ -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");
|
||||
|
||||
@@ -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()
|
||||
|
||||
Reference in New Issue
Block a user