Allow to explicitly specify whom to notify on a change update

When pushing changes/patch sets and when making change updates through
the REST API callers can specify a 'notify' option to control who
should be notified (not all REST endpoints support this option). This
'notify' option allows to choose if owner, reviewers, watchers and
users that have starred the change should be notified, or if no
notification should be send. For some tools this is not sufficient as
they want to have full control over which users should be notified.
E.g. if a CI integration is triggered by a certain label, the owner
and the user that triggered the CI should be notified.

To support this the REST API input entities that support the 'notify'
option are extended by a 'notify_details' options that allows the
caller to specify accounts that should be notified. These accounts are
always notified, even if notifications are otherwise disabled by
setting 'notify' to 'NONE'.

The 'notify_details' option is a map that maps a recipient type to a
'NotifyInfo' entity. This means callers can control how the accounts
should be notified, as TO, CC or BCC. The 'NotifyInfo' entity hosts
the list of accounts that should be notified. It is an extra entity so
that further entities that should be notified can be added in future.
E.g. one could imagine to add a list of emails (for user without
Gerrit account), a list of groups or a list of star labels (notify all
users that have set this star label on the change).

Also when pushing commits for review, specific accounts can be
explicitly notified by setting 'notify-to=<email>',
'notify-cc=<email>' or 'notify-bcc=<email>' in the push specification:

  git push origin HEAD:refs/for/master%notify-to=foo@bar.com

These accounts are not added as reviewer or CC so that they are not
signed up to receive notifications for futher updates of the change.

Change-Id: Id112b70158b80f84f631b5022f33228871716108
Signed-off-by: Edwin Kempin <ekempin@google.com>
This commit is contained in:
Edwin Kempin 2016-12-01 09:10:09 +01:00
parent 5690d689d9
commit cd07df425d
45 changed files with 658 additions and 126 deletions

View File

@ -4753,17 +4753,20 @@ This can be:
The `AbandonInput` entity contains information for abandoning a change. The `AbandonInput` entity contains information for abandoning a change.
[options="header",cols="1,^1,5"] [options="header",cols="1,^1,5"]
|=========================== |=============================
|Field Name ||Description |Field Name ||Description
|`message` |optional| |`message` |optional|
Message to be added as review comment to the change when abandoning the Message to be added as review comment to the change when abandoning the
change. change.
|`notify` |optional| |`notify` |optional|
Notify handling that defines to whom email notifications should be sent after Notify handling that defines to whom email notifications should be sent after
the change is abandoned. + the change is abandoned. +
Allowed values are `NONE`, `OWNER`, `OWNER_REVIEWERS` and `ALL`. + Allowed values are `NONE`, `OWNER`, `OWNER_REVIEWERS` and `ALL`. +
If not set, the default is `ALL`. If not set, the default is `ALL`.
|=========================== |`notify_details`|optional|
Additional information about whom to notify about the update as a map
of recipient type to link:#notify-info[NotifyInfo] entity.
|=============================
[[action-info]] [[action-info]]
=== ActionInfo === ActionInfo
@ -5206,14 +5209,17 @@ The `DeleteReviewerInput` entity contains options for the deletion of a
reviewer. reviewer.
[options="header",cols="1,^1,5"] [options="header",cols="1,^1,5"]
|======================= |=============================
|Field Name||Description |Field Name ||Description
|`notify` |optional| |`notify` |optional|
Notify handling that defines to whom email notifications should be sent Notify handling that defines to whom email notifications should be sent
after the reviewer is deleted. + after the reviewer is deleted. +
Allowed values are `NONE`, `OWNER`, `OWNER_REVIEWERS` and `ALL`. + Allowed values are `NONE`, `OWNER`, `OWNER_REVIEWERS` and `ALL`. +
If not set, the default is `ALL`. If not set, the default is `ALL`.
|======================= |`notify_details`|optional|
Additional information about whom to notify about the update as a map
of recipient type to link:#notify-info[NotifyInfo] entity.
|=============================
[[delete-vote-input]] [[delete-vote-input]]
=== DeleteVoteInput === DeleteVoteInput
@ -5221,17 +5227,20 @@ The `DeleteVoteInput` entity contains options for the deletion of a
vote. vote.
[options="header",cols="1,^1,5"] [options="header",cols="1,^1,5"]
|======================= |=============================
|Field Name||Description |Field Name ||Description
|`label` |optional| |`label` |optional|
The label for which the vote should be deleted. + The label for which the vote should be deleted. +
If set, must match the label in the URL. If set, must match the label in the URL.
|`notify` |optional| |`notify` |optional|
Notify handling that defines to whom email notifications should be sent Notify handling that defines to whom email notifications should be sent
after the vote is deleted. + after the vote is deleted. +
Allowed values are `NONE`, `OWNER`, `OWNER_REVIEWERS` and `ALL`. + Allowed values are `NONE`, `OWNER`, `OWNER_REVIEWERS` and `ALL`. +
If not set, the default is `ALL`. If not set, the default is `ALL`.
|======================= |`notify_details`|optional|
Additional information about whom to notify about the update as a map
of recipient type to link:#notify-info[NotifyInfo] entity.
|=============================
[[description-input]] [[description-input]]
=== DescriptionInput === DescriptionInput
@ -5623,6 +5632,23 @@ The `MoveInput` entity contains information for moving a change to a new branch.
A message to be posted in this change's comments A message to be posted in this change's comments
|=========================== |===========================
[[notify-info]]
=== NotifyInfo
The `NotifyInfo` entity contains detailed information about who should
be notified about an update. These notifications are sent out even if a
`notify` option in the request input disables normal notifications.
`NotifyInfo` entities are normally contained in a `notify_details` map
in the request input where the key is the recipient type. The recipient
type can be `TO`, `CC` and `BCC`.
[options="header",cols="1,^1,5"]
|=======================
|Field Name||Description
|`accounts`|optional|
A list of link:rest-api-accounts.html#account-id[account IDs] that
identify the accounts that should be should be notified.
|=======================
[[problem-info]] [[problem-info]]
=== ProblemInfo === ProblemInfo
The `ProblemInfo` entity contains a description of a potential consistency problem The `ProblemInfo` entity contains a description of a potential consistency problem
@ -5648,14 +5674,17 @@ The `PublishChangeEditInput` entity contains options for the publishing of
change edit. change edit.
[options="header",cols="1,^1,5"] [options="header",cols="1,^1,5"]
|======================= |=============================
|Field Name||Description |Field Name ||Description
|`notify` |optional| |`notify` |optional|
Notify handling that defines to whom email notifications should be sent Notify handling that defines to whom email notifications should be sent
after the change edit is published. + after the change edit is published. +
Allowed values are `NONE` and `ALL`. + Allowed values are `NONE` and `ALL`. +
If not set, the default is `ALL`. If not set, the default is `ALL`.
|======================= |`notify_details`|optional|
Additional information about whom to notify about the update as a map
of recipient type to link:#notify-info[NotifyInfo] entity.
|=============================
[[push-certificate-info]] [[push-certificate-info]]
=== PushCertificateInfo === PushCertificateInfo
@ -5835,6 +5864,9 @@ Notify handling that defines to whom email notifications should be sent
after the review is stored. + after the review is stored. +
Allowed values are `NONE`, `OWNER`, `OWNER_REVIEWERS` and `ALL`. + Allowed values are `NONE`, `OWNER`, `OWNER_REVIEWERS` and `ALL`. +
If not set, the default is `ALL`. If not set, the default is `ALL`.
|`notify_details` |optional|
Additional information about whom to notify about the update as a map
of recipient type to link:#notify-info[NotifyInfo] entity.
|`omit_duplicate_comments`|optional| |`omit_duplicate_comments`|optional|
If `true`, comments with the same content at the same place will be omitted. If `true`, comments with the same content at the same place will be omitted.
|`on_behalf_of` |optional| |`on_behalf_of` |optional|
@ -5867,28 +5899,31 @@ The `ReviewerInput` entity contains information for adding a reviewer
to a change. to a change.
[options="header",cols="1,^1,5"] [options="header",cols="1,^1,5"]
|=========================== |=============================
|Field Name ||Description |Field Name ||Description
|`reviewer` || |`reviewer` ||
The link:rest-api-accounts.html#account-id[ID] of one account that The link:rest-api-accounts.html#account-id[ID] of one account that
should be added as reviewer or the link:rest-api-groups.html#group-id[ should be added as reviewer or the link:rest-api-groups.html#group-id[
ID] of one group for which all members should be added as reviewers. + ID] of one group for which all members should be added as reviewers. +
If an ID identifies both an account and a group, only the account is If an ID identifies both an account and a group, only the account is
added as reviewer to the change. added as reviewer to the change.
|`state` |optional| |`state` |optional|
Add reviewer in this state. Possible reviewer states are `REVIEWER` Add reviewer in this state. Possible reviewer states are `REVIEWER`
and `CC`. If not given, defaults to `REVIEWER`. and `CC`. If not given, defaults to `REVIEWER`.
|`confirmed` |optional| |`confirmed` |optional|
Whether adding the reviewer is confirmed. + Whether adding the reviewer is confirmed. +
The Gerrit server may be configured to The Gerrit server may be configured to
link:config-gerrit.html#addreviewer.maxWithoutConfirmation[require a link:config-gerrit.html#addreviewer.maxWithoutConfirmation[require a
confirmation] when adding a group as reviewer that has many members. confirmation] when adding a group as reviewer that has many members.
|`notify` |optional| |`notify` |optional|
Notify handling that defines to whom email notifications should be sent Notify handling that defines to whom email notifications should be sent
after the reviewer is added. + after the reviewer is added. +
Allowed values are `NONE`, `OWNER`, `OWNER_REVIEWERS` and `ALL`. + Allowed values are `NONE`, `OWNER`, `OWNER_REVIEWERS` and `ALL`. +
If not set, the default is `ALL`. If not set, the default is `ALL`.
|=========================== |`notify_details`|optional|
Additional information about whom to notify about the update as a map
of recipient type to link:#notify-info[NotifyInfo] entity.
|=============================
[[revision-info]] [[revision-info]]
=== RevisionInfo === RevisionInfo
@ -6014,20 +6049,23 @@ not with the identity of the CI account.
The `SubmitInput` entity contains information for submitting a change. The `SubmitInput` entity contains information for submitting a change.
[options="header",cols="1,^1,5"] [options="header",cols="1,^1,5"]
|=========================== |=============================
|Field Name ||Description |Field Name ||Description
|`on_behalf_of`|optional| |`on_behalf_of` |optional|
If set, submit the change on behalf of the given user. The value may take any If set, submit the change on behalf of the given user. The value may take any
format link:rest-api-accounts.html#account-id[accepted by the accounts REST format link:rest-api-accounts.html#account-id[accepted by the accounts REST
API]. Using this option requires API]. Using this option requires
link:access-control.html#category_submit_on_behalf_of[Submit (On Behalf Of)] link:access-control.html#category_submit_on_behalf_of[Submit (On Behalf Of)]
permission on the branch. permission on the branch.
|`notify`|optional| |`notify` |optional|
Notify handling that defines to whom email notifications should be sent after Notify handling that defines to whom email notifications should be sent after
the change is submitted. + the change is submitted. +
Allowed values are `NONE`, `OWNER`, `OWNER_REVIEWERS` and `ALL`. + Allowed values are `NONE`, `OWNER`, `OWNER_REVIEWERS` and `ALL`. +
If not set, the default is `ALL`. If not set, the default is `ALL`.
|=========================== |`notify_details`|optional|
Additional information about whom to notify about the update as a map
of recipient type to link:#notify-info[NotifyInfo] entity.
|=============================
[[submit-record]] [[submit-record]]
=== SubmitRecord === SubmitRecord

View File

@ -177,6 +177,24 @@ By default all email notifications are sent.
git push ssh://bot@git.example.com:29418/kernel/common HEAD:refs/for/master%notify=NONE git push ssh://bot@git.example.com:29418/kernel/common HEAD:refs/for/master%notify=NONE
---- ----
In addition uploaders can explicitly specify accounts that should be
notified, regardless of the value that is given for the `notify`
option. To notify a specific account specify it by an
`notify-to='email'`, `notify-cc='email'` or `notify-bcc='email'`
option. These options can be specified as many times as necessary to
cover all interested parties. Gerrit will automatically avoid sending
duplicate email notifications, such as if one of the specified accounts
had also requested to receive all new change notifications. The
accounts that are specified by `notify-to='email'`, `notify-cc='email'`
and `notify-bcc='email'` will only be notified about this one push.
They are not added as link:#reviewers[reviewers or CCs], hence they are
not automatically signed up to be notified on further updates of the
change.
----
git push ssh://bot@git.example.com:29418/kernel/common HEAD:refs/for/master%notify=NONE,notify-to=a@a.com
----
[[topic]] [[topic]]
==== Topic ==== Topic

View File

@ -892,6 +892,13 @@ public abstract class AbstractDaemonTest {
.review(ReviewInput.approve()); .review(ReviewInput.approve());
} }
protected void recommend(String id) throws Exception {
gApi.changes()
.id(id)
.revision("current")
.review(ReviewInput.recommend());
}
protected Map<String, ActionInfo> getActions(String id) throws Exception { protected Map<String, ActionInfo> getActions(String id) throws Exception {
return gApi.changes() return gApi.changes()
.id(id) .id(id)
@ -1171,4 +1178,32 @@ public abstract class AbstractDaemonTest {
.containsExactlyElementsIn(Arrays.asList(expected)); .containsExactlyElementsIn(Arrays.asList(expected));
} }
} }
protected void assertNotifyTo(TestAccount expected) {
assertThat(sender.getMessages()).hasSize(1);
Message m = sender.getMessages().get(0);
assertThat(m.rcpt()).containsExactly(expected.emailAddress);
assertThat(
((EmailHeader.AddressList) m.headers().get("To")).getAddressList())
.containsExactly(expected.emailAddress);
assertThat(m.headers().get("CC").isEmpty()).isTrue();
}
protected void assertNotifyCc(TestAccount expected) {
assertThat(sender.getMessages()).hasSize(1);
Message m = sender.getMessages().get(0);
assertThat(m.rcpt()).containsExactly(expected.emailAddress);
assertThat(m.headers().get("To").isEmpty()).isTrue();
assertThat(
((EmailHeader.AddressList) m.headers().get("CC")).getAddressList())
.containsExactly(expected.emailAddress);
}
protected void assertNotifyBcc(TestAccount expected) {
assertThat(sender.getMessages()).hasSize(1);
Message m = sender.getMessages().get(0);
assertThat(m.rcpt()).containsExactly(expected.emailAddress);
assertThat(m.headers().get("To").isEmpty()).isTrue();
assertThat(m.headers().get("CC").isEmpty()).isTrue();
}
} }

View File

@ -43,6 +43,7 @@ import com.google.gerrit.acceptance.GerritConfig;
import com.google.gerrit.acceptance.GitUtil; import com.google.gerrit.acceptance.GitUtil;
import com.google.gerrit.acceptance.NoHttpd; import com.google.gerrit.acceptance.NoHttpd;
import com.google.gerrit.acceptance.PushOneCommit; import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.TestAccount;
import com.google.gerrit.acceptance.TestProjectInput; import com.google.gerrit.acceptance.TestProjectInput;
import com.google.gerrit.common.FooterConstants; import com.google.gerrit.common.FooterConstants;
import com.google.gerrit.common.TimeUtil; import com.google.gerrit.common.TimeUtil;
@ -52,7 +53,9 @@ import com.google.gerrit.extensions.api.changes.AddReviewerInput;
import com.google.gerrit.extensions.api.changes.DeleteReviewerInput; import com.google.gerrit.extensions.api.changes.DeleteReviewerInput;
import com.google.gerrit.extensions.api.changes.DeleteVoteInput; import com.google.gerrit.extensions.api.changes.DeleteVoteInput;
import com.google.gerrit.extensions.api.changes.NotifyHandling; import com.google.gerrit.extensions.api.changes.NotifyHandling;
import com.google.gerrit.extensions.api.changes.NotifyInfo;
import com.google.gerrit.extensions.api.changes.RebaseInput; import com.google.gerrit.extensions.api.changes.RebaseInput;
import com.google.gerrit.extensions.api.changes.RecipientType;
import com.google.gerrit.extensions.api.changes.ReviewInput; import com.google.gerrit.extensions.api.changes.ReviewInput;
import com.google.gerrit.extensions.api.changes.RevisionApi; import com.google.gerrit.extensions.api.changes.RevisionApi;
import com.google.gerrit.extensions.api.projects.BranchInput; import com.google.gerrit.extensions.api.projects.BranchInput;
@ -1373,10 +1376,7 @@ public class ChangeIT extends AbstractDaemonTest {
.review(ReviewInput.approve()); .review(ReviewInput.approve());
setApiUser(user); setApiUser(user);
gApi.changes() recommend(r.getChangeId());
.id(r.getChangeId())
.revision(r.getCommit().name())
.review(ReviewInput.recommend());
setApiUser(admin); setApiUser(admin);
sender.clear(); sender.clear();
@ -1425,10 +1425,7 @@ public class ChangeIT extends AbstractDaemonTest {
.review(ReviewInput.approve()); .review(ReviewInput.approve());
setApiUser(user); setApiUser(user);
gApi.changes() recommend(r.getChangeId());
.id(r.getChangeId())
.revision(r.getCommit().name())
.review(ReviewInput.recommend());
setApiUser(admin); setApiUser(admin);
sender.clear(); sender.clear();
@ -1442,6 +1439,62 @@ public class ChangeIT extends AbstractDaemonTest {
assertThat(sender.getMessages()).hasSize(0); assertThat(sender.getMessages()).hasSize(0);
} }
@Test
public void deleteVoteNotifyAccount() throws Exception {
PushOneCommit.Result r = createChange();
gApi.changes()
.id(r.getChangeId())
.revision(r.getCommit().name())
.review(ReviewInput.approve());
DeleteVoteInput in = new DeleteVoteInput();
in.label = "Code-Review";
in.notify = NotifyHandling.NONE;
// notify unrelated account as TO
TestAccount user2 = accounts.user2();
setApiUser(user);
recommend(r.getChangeId());
setApiUser(admin);
sender.clear();
in.notifyDetails = new HashMap<>();
in.notifyDetails.put(RecipientType.TO,
new NotifyInfo(ImmutableList.of(user2.email)));
gApi.changes()
.id(r.getChangeId())
.reviewer(user.getId().toString())
.deleteVote(in);
assertNotifyTo(user2);
// notify unrelated account as CC
setApiUser(user);
recommend(r.getChangeId());
setApiUser(admin);
sender.clear();
in.notifyDetails = new HashMap<>();
in.notifyDetails.put(RecipientType.CC,
new NotifyInfo(ImmutableList.of(user2.email)));
gApi.changes()
.id(r.getChangeId())
.reviewer(user.getId().toString())
.deleteVote(in);
assertNotifyCc(user2);
// notify unrelated account as BCC
setApiUser(user);
recommend(r.getChangeId());
setApiUser(admin);
sender.clear();
in.notifyDetails = new HashMap<>();
in.notifyDetails.put(RecipientType.BCC,
new NotifyInfo(ImmutableList.of(user2.email)));
gApi.changes()
.id(r.getChangeId())
.reviewer(user.getId().toString())
.deleteVote(in);
assertNotifyBcc(user2);
}
@Test @Test
public void deleteVoteNotPermitted() throws Exception { public void deleteVoteNotPermitted() throws Exception {
PushOneCommit.Result r = createChange(); PushOneCommit.Result r = createChange();

View File

@ -21,6 +21,7 @@ import static java.nio.charset.StandardCharsets.UTF_8;
import static java.util.concurrent.TimeUnit.SECONDS; import static java.util.concurrent.TimeUnit.SECONDS;
import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableListMultimap;
import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Lists; import com.google.common.collect.Lists;
import com.google.gerrit.acceptance.AbstractDaemonTest; import com.google.gerrit.acceptance.AbstractDaemonTest;
@ -174,8 +175,10 @@ public class ChangeEditIT extends AbstractDaemonTest {
.isEqualTo(RefUpdate.Result.NEW); .isEqualTo(RefUpdate.Result.NEW);
assertThat( assertThat(
modifier.modifyFile(editUtil.byChange(change).get(), FILE_NAME, modifier.modifyFile(editUtil.byChange(change).get(), FILE_NAME,
RawInputUtil.create(CONTENT_NEW2))).isEqualTo(RefUpdate.Result.FORCED); RawInputUtil.create(CONTENT_NEW2)))
editUtil.publish(editUtil.byChange(change).get(), NotifyHandling.NONE); .isEqualTo(RefUpdate.Result.FORCED);
editUtil.publish(editUtil.byChange(change).get(), NotifyHandling.NONE,
ImmutableListMultimap.of());
Optional<ChangeEdit> edit = editUtil.byChange(change); Optional<ChangeEdit> edit = editUtil.byChange(change);
assertThat(edit.isPresent()).isFalse(); assertThat(edit.isPresent()).isFalse();
assertChangeMessages(change, assertChangeMessages(change,
@ -386,7 +389,8 @@ public class ChangeEditIT extends AbstractDaemonTest {
edit = editUtil.byChange(change); edit = editUtil.byChange(change);
assertThat(edit.get().getEditCommit().getFullMessage()).isEqualTo(msg); assertThat(edit.get().getEditCommit().getFullMessage()).isEqualTo(msg);
editUtil.publish(edit.get(), NotifyHandling.NONE); editUtil.publish(edit.get(), NotifyHandling.NONE,
ImmutableListMultimap.of());
assertThat(editUtil.byChange(change).isPresent()).isFalse(); assertThat(editUtil.byChange(change).isPresent()).isFalse();
ChangeInfo info = get(changeId, ListChangesOption.CURRENT_COMMIT, ChangeInfo info = get(changeId, ListChangesOption.CURRENT_COMMIT,
@ -429,7 +433,8 @@ public class ChangeEditIT extends AbstractDaemonTest {
assertThat(readContentFromJson(r)).isEqualTo(commit.getFullMessage()); assertThat(readContentFromJson(r)).isEqualTo(commit.getFullMessage());
} }
editUtil.publish(edit.get(), NotifyHandling.NONE); editUtil.publish(edit.get(), NotifyHandling.NONE,
ImmutableListMultimap.of());
assertChangeMessages(change, assertChangeMessages(change,
ImmutableList.of("Uploaded patch set 1.", ImmutableList.of("Uploaded patch set 1.",
"Uploaded patch set 2.", "Uploaded patch set 2.",
@ -732,7 +737,8 @@ public class ChangeEditIT extends AbstractDaemonTest {
assertThat(modifier.modifyMessage(edit.get(), newMsg)) assertThat(modifier.modifyMessage(edit.get(), newMsg))
.isEqualTo(RefUpdate.Result.FORCED); .isEqualTo(RefUpdate.Result.FORCED);
edit = editUtil.byChange(change); edit = editUtil.byChange(change);
editUtil.publish(edit.get(), NotifyHandling.NONE); editUtil.publish(edit.get(), NotifyHandling.NONE,
ImmutableListMultimap.of());
ChangeInfo info = get(changeId); ChangeInfo info = get(changeId);
assertThat(info.subject).isEqualTo(newSubj); assertThat(info.subject).isEqualTo(newSubj);
@ -759,7 +765,8 @@ public class ChangeEditIT extends AbstractDaemonTest {
editUtil.delete(editUtil.byChange(change).get()); editUtil.delete(editUtil.byChange(change).get());
assertThat(queryEdits()).hasSize(1); assertThat(queryEdits()).hasSize(1);
editUtil.publish(editUtil.byChange(change2).get(), NotifyHandling.NONE); editUtil.publish(editUtil.byChange(change2).get(), NotifyHandling.NONE,
ImmutableListMultimap.of());
assertThat(queryEdits()).hasSize(0); assertThat(queryEdits()).hasSize(0);
setApiUser(user); setApiUser(user);

View File

@ -262,6 +262,43 @@ public abstract class AbstractPushForReview extends AbstractDaemonTest {
m = sender.getMessages().get(0); m = sender.getMessages().get(0);
assertThat(m.rcpt()).containsExactly(user.emailAddress, user2.emailAddress, assertThat(m.rcpt()).containsExactly(user.emailAddress, user2.emailAddress,
user3.emailAddress); user3.emailAddress);
sender.clear();
r = pushTo(pushSpec + ",notify=" + NotifyHandling.NONE + ",notify-to="
+ user3.email);
r.assertOkStatus();
assertNotifyTo(user3);
sender.clear();
r = pushTo(pushSpec + ",notify=" + NotifyHandling.NONE + ",notify-cc="
+ user3.email);
r.assertOkStatus();
assertNotifyCc(user3);
sender.clear();
r = pushTo(pushSpec + ",notify=" + NotifyHandling.NONE + ",notify-bcc="
+ user3.email);
r.assertOkStatus();
assertNotifyBcc(user3);
// request that sender gets notified as TO, CC and BCC, email should be sent
// even if the sender is the only recipient
sender.clear();
r = pushTo(pushSpec + ",notify=" + NotifyHandling.NONE + ",notify-to="
+ admin.email);
assertNotifyTo(admin);
sender.clear();
r = pushTo(pushSpec + ",notify=" + NotifyHandling.NONE + ",notify-cc="
+ admin.email);
r.assertOkStatus();
assertNotifyCc(admin);
sender.clear();
r = pushTo(pushSpec + ",notify=" + NotifyHandling.NONE + ",notify-bcc="
+ admin.email);
r.assertOkStatus();
assertNotifyBcc(admin);
} }
@Test @Test

View File

@ -16,9 +16,12 @@ package com.google.gerrit.extensions.api.changes;
import com.google.gerrit.extensions.restapi.DefaultInput; import com.google.gerrit.extensions.restapi.DefaultInput;
import java.util.Map;
public class AbandonInput { public class AbandonInput {
@DefaultInput @DefaultInput
public String message; public String message;
public NotifyHandling notify = NotifyHandling.ALL; public NotifyHandling notify = NotifyHandling.ALL;
public Map<RecipientType, NotifyInfo> notifyDetails;
} }

View File

@ -19,12 +19,15 @@ import static com.google.gerrit.extensions.client.ReviewerState.REVIEWER;
import com.google.gerrit.extensions.client.ReviewerState; import com.google.gerrit.extensions.client.ReviewerState;
import com.google.gerrit.extensions.restapi.DefaultInput; import com.google.gerrit.extensions.restapi.DefaultInput;
import java.util.Map;
public class AddReviewerInput { public class AddReviewerInput {
@DefaultInput @DefaultInput
public String reviewer; public String reviewer;
public Boolean confirmed; public Boolean confirmed;
public ReviewerState state; public ReviewerState state;
public NotifyHandling notify; public NotifyHandling notify;
public Map<RecipientType, NotifyInfo> notifyDetails;
public boolean confirmed() { public boolean confirmed() {
return (confirmed != null) ? confirmed : false; return (confirmed != null) ? confirmed : false;

View File

@ -14,8 +14,11 @@
package com.google.gerrit.extensions.api.changes; package com.google.gerrit.extensions.api.changes;
import java.util.Map;
/** Input passed to {@code DELETE /changes/[id]/reviewers/[id]}. */ /** Input passed to {@code DELETE /changes/[id]/reviewers/[id]}. */
public class DeleteReviewerInput { public class DeleteReviewerInput {
/** Who to send email notifications to after the reviewer is deleted. */ /** Who to send email notifications to after the reviewer is deleted. */
public NotifyHandling notify = NotifyHandling.ALL; public NotifyHandling notify = NotifyHandling.ALL;
public Map<RecipientType, NotifyInfo> notifyDetails;
} }

View File

@ -16,6 +16,8 @@ package com.google.gerrit.extensions.api.changes;
import com.google.gerrit.extensions.restapi.DefaultInput; import com.google.gerrit.extensions.restapi.DefaultInput;
import java.util.Map;
/** Input passed to {@code DELETE /changes/[id]/reviewers/[id]/votes/[label]}. */ /** Input passed to {@code DELETE /changes/[id]/reviewers/[id]/votes/[label]}. */
public class DeleteVoteInput { public class DeleteVoteInput {
@DefaultInput @DefaultInput
@ -23,4 +25,5 @@ public class DeleteVoteInput {
/** Who to send email notifications to after vote is deleted. */ /** Who to send email notifications to after vote is deleted. */
public NotifyHandling notify = NotifyHandling.ALL; public NotifyHandling notify = NotifyHandling.ALL;
public Map<RecipientType, NotifyInfo> notifyDetails;
} }

View File

@ -0,0 +1,26 @@
// Copyright (C) 2016 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.extensions.api.changes;
import java.util.List;
/** Detailed information about who should be notified about an update. */
public class NotifyInfo {
public List<String> accounts;
public NotifyInfo(List<String> accounts) {
this.accounts = accounts;
}
}

View File

@ -14,8 +14,11 @@
package com.google.gerrit.extensions.api.changes; package com.google.gerrit.extensions.api.changes;
import java.util.Map;
/** Input passed to {@code POST /changes/[id]/edit:publish/}. */ /** Input passed to {@code POST /changes/[id]/edit:publish/}. */
public class PublishChangeEditInput { public class PublishChangeEditInput {
/** Who to send email notifications to after the change edit is published. */ /** Who to send email notifications to after the change edit is published. */
public NotifyHandling notify = NotifyHandling.ALL; public NotifyHandling notify = NotifyHandling.ALL;
public Map<RecipientType, NotifyInfo> notifyDetails;
} }

View File

@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and // See the License for the specific language governing permissions and
// limitations under the License. // limitations under the License.
package com.google.gerrit.server.mail; package com.google.gerrit.extensions.api.changes;
public enum RecipientType { public enum RecipientType {
TO, CC, BCC TO, CC, BCC

View File

@ -57,6 +57,7 @@ public class ReviewInput {
/** Who to send email notifications to after review is stored. */ /** Who to send email notifications to after review is stored. */
public NotifyHandling notify = NotifyHandling.ALL; public NotifyHandling notify = NotifyHandling.ALL;
public Map<RecipientType, NotifyInfo> notifyDetails;
/** /**
* If true check to make sure that the comments being posted aren't already * If true check to make sure that the comments being posted aren't already

View File

@ -14,6 +14,8 @@
package com.google.gerrit.extensions.api.changes; package com.google.gerrit.extensions.api.changes;
import java.util.Map;
public class SubmitInput { public class SubmitInput {
/** Not used anymore, kept for backward compatibility */ /** Not used anymore, kept for backward compatibility */
@Deprecated @Deprecated
@ -22,4 +24,5 @@ public class SubmitInput {
public String onBehalfOf; public String onBehalfOf;
public NotifyHandling notify = NotifyHandling.ALL; public NotifyHandling notify = NotifyHandling.ALL;
public Map<RecipientType, NotifyInfo> notifyDetails;
} }

View File

@ -14,9 +14,12 @@
package com.google.gerrit.server.change; package com.google.gerrit.server.change;
import com.google.common.collect.ImmutableListMultimap;
import com.google.common.collect.Multimap;
import com.google.gerrit.common.TimeUtil; import com.google.gerrit.common.TimeUtil;
import com.google.gerrit.extensions.api.changes.AbandonInput; import com.google.gerrit.extensions.api.changes.AbandonInput;
import com.google.gerrit.extensions.api.changes.NotifyHandling; import com.google.gerrit.extensions.api.changes.NotifyHandling;
import com.google.gerrit.extensions.api.changes.RecipientType;
import com.google.gerrit.extensions.common.ChangeInfo; import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.ResourceConflictException; import com.google.gerrit.extensions.restapi.ResourceConflictException;
@ -51,17 +54,20 @@ public class Abandon implements RestModifyView<ChangeResource, AbandonInput>,
private final ChangeJson.Factory json; private final ChangeJson.Factory json;
private final BatchUpdate.Factory batchUpdateFactory; private final BatchUpdate.Factory batchUpdateFactory;
private final AbandonOp.Factory abandonOpFactory; private final AbandonOp.Factory abandonOpFactory;
private final NotifyUtil notifyUtil;
@Inject @Inject
Abandon( Abandon(
Provider<ReviewDb> dbProvider, Provider<ReviewDb> dbProvider,
ChangeJson.Factory json, ChangeJson.Factory json,
BatchUpdate.Factory batchUpdateFactory, BatchUpdate.Factory batchUpdateFactory,
AbandonOp.Factory abandonOpFactory) { AbandonOp.Factory abandonOpFactory,
NotifyUtil notifyUtil) {
this.dbProvider = dbProvider; this.dbProvider = dbProvider;
this.json = json; this.json = json;
this.batchUpdateFactory = batchUpdateFactory; this.batchUpdateFactory = batchUpdateFactory;
this.abandonOpFactory = abandonOpFactory; this.abandonOpFactory = abandonOpFactory;
this.notifyUtil = notifyUtil;
} }
@Override @Override
@ -71,27 +77,32 @@ public class Abandon implements RestModifyView<ChangeResource, AbandonInput>,
if (!control.canAbandon(dbProvider.get())) { if (!control.canAbandon(dbProvider.get())) {
throw new AuthException("abandon not permitted"); throw new AuthException("abandon not permitted");
} }
Change change = abandon(control, input.message, input.notify); Change change = abandon(control, input.message, input.notify,
notifyUtil.resolveAccounts(input.notifyDetails));
return json.create(ChangeJson.NO_OPTIONS).format(change); return json.create(ChangeJson.NO_OPTIONS).format(change);
} }
public Change abandon(ChangeControl control) public Change abandon(ChangeControl control)
throws RestApiException, UpdateException { throws RestApiException, UpdateException {
return abandon(control, "", NotifyHandling.ALL); return abandon(control, "", NotifyHandling.ALL, ImmutableListMultimap.of());
} }
public Change abandon(ChangeControl control, String msgTxt) public Change abandon(ChangeControl control, String msgTxt)
throws RestApiException, UpdateException { throws RestApiException, UpdateException {
return abandon(control, msgTxt, NotifyHandling.ALL); return abandon(control, msgTxt, NotifyHandling.ALL,
ImmutableListMultimap.of());
} }
public Change abandon(ChangeControl control, String msgTxt, public Change abandon(ChangeControl control, String msgTxt,
NotifyHandling notifyHandling) throws RestApiException, UpdateException { NotifyHandling notifyHandling,
Multimap<RecipientType, Account.Id> accountsToNotify)
throws RestApiException, UpdateException {
CurrentUser user = control.getUser(); CurrentUser user = control.getUser();
Account account = user.isIdentifiedUser() Account account = user.isIdentifiedUser()
? user.asIdentifiedUser().getAccount() ? user.asIdentifiedUser().getAccount()
: null; : null;
AbandonOp op = abandonOpFactory.create(account, msgTxt, notifyHandling); AbandonOp op = abandonOpFactory.create(account, msgTxt, notifyHandling,
accountsToNotify);
try (BatchUpdate u = try (BatchUpdate u =
batchUpdateFactory.create( batchUpdateFactory.create(
dbProvider.get(), dbProvider.get(),
@ -113,7 +124,9 @@ public class Abandon implements RestModifyView<ChangeResource, AbandonInput>,
*/ */
public void batchAbandon(Project.NameKey project, CurrentUser user, public void batchAbandon(Project.NameKey project, CurrentUser user,
Collection<ChangeControl> controls, String msgTxt, Collection<ChangeControl> controls, String msgTxt,
NotifyHandling notifyHandling) throws RestApiException, UpdateException { NotifyHandling notifyHandling,
Multimap<RecipientType, Account.Id> accountsToNotify)
throws RestApiException, UpdateException {
if (controls.isEmpty()) { if (controls.isEmpty()) {
return; return;
} }
@ -132,7 +145,8 @@ public class Abandon implements RestModifyView<ChangeResource, AbandonInput>,
} }
u.addOp( u.addOp(
control.getId(), control.getId(),
abandonOpFactory.create(account, msgTxt, notifyHandling)); abandonOpFactory.create(account, msgTxt, notifyHandling,
accountsToNotify));
} }
u.execute(); u.execute();
} }
@ -141,13 +155,15 @@ public class Abandon implements RestModifyView<ChangeResource, AbandonInput>,
public void batchAbandon(Project.NameKey project, CurrentUser user, public void batchAbandon(Project.NameKey project, CurrentUser user,
Collection<ChangeControl> controls, String msgTxt) Collection<ChangeControl> controls, String msgTxt)
throws RestApiException, UpdateException { throws RestApiException, UpdateException {
batchAbandon(project, user, controls, msgTxt, NotifyHandling.ALL); batchAbandon(project, user, controls, msgTxt, NotifyHandling.ALL,
ImmutableListMultimap.of());
} }
public void batchAbandon(Project.NameKey project, CurrentUser user, public void batchAbandon(Project.NameKey project, CurrentUser user,
Collection<ChangeControl> controls) Collection<ChangeControl> controls)
throws RestApiException, UpdateException { throws RestApiException, UpdateException {
batchAbandon(project, user, controls, "", NotifyHandling.ALL); batchAbandon(project, user, controls, "", NotifyHandling.ALL,
ImmutableListMultimap.of());
} }
@Override @Override

View File

@ -20,10 +20,13 @@ import static com.google.gerrit.reviewdb.client.Change.INITIAL_PATCH_SET_ID;
import static java.util.stream.Collectors.toSet; import static java.util.stream.Collectors.toSet;
import com.google.common.base.MoreObjects; import com.google.common.base.MoreObjects;
import com.google.common.collect.ImmutableListMultimap;
import com.google.common.collect.Multimap;
import com.google.gerrit.common.FooterConstants; import com.google.gerrit.common.FooterConstants;
import com.google.gerrit.common.data.LabelType; import com.google.gerrit.common.data.LabelType;
import com.google.gerrit.common.data.LabelTypes; import com.google.gerrit.common.data.LabelTypes;
import com.google.gerrit.extensions.api.changes.NotifyHandling; import com.google.gerrit.extensions.api.changes.NotifyHandling;
import com.google.gerrit.extensions.api.changes.RecipientType;
import com.google.gerrit.extensions.restapi.ResourceConflictException; import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Branch; import com.google.gerrit.reviewdb.client.Branch;
@ -112,6 +115,8 @@ public class ChangeInserter extends BatchUpdate.InsertChangeOp {
private CommitValidators.Policy validatePolicy = private CommitValidators.Policy validatePolicy =
CommitValidators.Policy.GERRIT; CommitValidators.Policy.GERRIT;
private NotifyHandling notify = NotifyHandling.ALL; private NotifyHandling notify = NotifyHandling.ALL;
private Multimap<RecipientType, Account.Id> accountsToNotify =
ImmutableListMultimap.of();
private Set<Account.Id> reviewers; private Set<Account.Id> reviewers;
private Set<Account.Id> extraCC; private Set<Account.Id> extraCC;
private Map<String, Short> approvals; private Map<String, Short> approvals;
@ -235,6 +240,12 @@ public class ChangeInserter extends BatchUpdate.InsertChangeOp {
return this; return this;
} }
public ChangeInserter setAccountsToNotify(
Multimap<RecipientType, Account.Id> accountsToNotify) {
this.accountsToNotify = checkNotNull(accountsToNotify);
return this;
}
public ChangeInserter setReviewers(Set<Account.Id> reviewers) { public ChangeInserter setReviewers(Set<Account.Id> reviewers) {
this.reviewers = reviewers; this.reviewers = reviewers;
return this; return this;
@ -401,7 +412,8 @@ public class ChangeInserter extends BatchUpdate.InsertChangeOp {
@Override @Override
public void postUpdate(Context ctx) throws OrmException, NoSuchChangeException { public void postUpdate(Context ctx) throws OrmException, NoSuchChangeException {
if (sendMail) { if (sendMail
&& (notify != NotifyHandling.NONE || !accountsToNotify.isEmpty())) {
Runnable sender = new Runnable() { Runnable sender = new Runnable() {
@Override @Override
public void run() { public void run() {
@ -411,6 +423,7 @@ public class ChangeInserter extends BatchUpdate.InsertChangeOp {
cm.setFrom(change.getOwner()); cm.setFrom(change.getOwner());
cm.setPatchSet(patchSet, patchSetInfo); cm.setPatchSet(patchSet, patchSetInfo);
cm.setNotify(notify); cm.setNotify(notify);
cm.setAccountsToNotify(accountsToNotify);
cm.addReviewers(reviewers); cm.addReviewers(reviewers);
cm.addExtraCC(extraCC); cm.addExtraCC(extraCC);
cm.send(); cm.send();

View File

@ -79,6 +79,7 @@ public class DeleteReviewer
private final Provider<IdentifiedUser> user; private final Provider<IdentifiedUser> user;
private final DeleteReviewerSender.Factory deleteReviewerSenderFactory; private final DeleteReviewerSender.Factory deleteReviewerSenderFactory;
private final NotesMigration migration; private final NotesMigration migration;
private final NotifyUtil notifyUtil;
@Inject @Inject
DeleteReviewer(Provider<ReviewDb> dbProvider, DeleteReviewer(Provider<ReviewDb> dbProvider,
@ -90,7 +91,8 @@ public class DeleteReviewer
ReviewerDeleted reviewerDeleted, ReviewerDeleted reviewerDeleted,
Provider<IdentifiedUser> user, Provider<IdentifiedUser> user,
DeleteReviewerSender.Factory deleteReviewerSenderFactory, DeleteReviewerSender.Factory deleteReviewerSenderFactory,
NotesMigration migration) { NotesMigration migration,
NotifyUtil notifyUtil) {
this.dbProvider = dbProvider; this.dbProvider = dbProvider;
this.approvalsUtil = approvalsUtil; this.approvalsUtil = approvalsUtil;
this.psUtil = psUtil; this.psUtil = psUtil;
@ -101,6 +103,7 @@ public class DeleteReviewer
this.user = user; this.user = user;
this.deleteReviewerSenderFactory = deleteReviewerSenderFactory; this.deleteReviewerSenderFactory = deleteReviewerSenderFactory;
this.migration = migration; this.migration = migration;
this.notifyUtil = notifyUtil;
} }
@Override @Override
@ -198,7 +201,7 @@ public class DeleteReviewer
@Override @Override
public void postUpdate(Context ctx) { public void postUpdate(Context ctx) {
if (input.notify.compareTo(NotifyHandling.NONE) > 0) { if (NotifyUtil.shouldNotify(input.notify, input.notifyDetails)) {
emailReviewers(ctx.getProject(), currChange, del, changeMessage); emailReviewers(ctx.getProject(), currChange, del, changeMessage);
} }
reviewerDeleted.fire(currChange, currPs, reviewer, reviewerDeleted.fire(currChange, currPs, reviewer,
@ -262,6 +265,8 @@ public class DeleteReviewer
cm.setChangeMessage(changeMessage.getMessage(), cm.setChangeMessage(changeMessage.getMessage(),
changeMessage.getWrittenOn()); changeMessage.getWrittenOn());
cm.setNotify(input.notify); cm.setNotify(input.notify);
cm.setAccountsToNotify(
notifyUtil.resolveAccounts(input.notifyDetails));
cm.send(); cm.send();
} catch (Exception err) { } catch (Exception err) {
log.error("Cannot email update for change " + change.getId(), err); log.error("Cannot email update for change " + change.getId(), err);

View File

@ -73,6 +73,7 @@ public class DeleteVote
private final IdentifiedUser.GenericFactory userFactory; private final IdentifiedUser.GenericFactory userFactory;
private final VoteDeleted voteDeleted; private final VoteDeleted voteDeleted;
private final DeleteVoteSender.Factory deleteVoteSenderFactory; private final DeleteVoteSender.Factory deleteVoteSenderFactory;
private final NotifyUtil notifyUtil;
@Inject @Inject
DeleteVote(Provider<ReviewDb> db, DeleteVote(Provider<ReviewDb> db,
@ -82,7 +83,8 @@ public class DeleteVote
ChangeMessagesUtil cmUtil, ChangeMessagesUtil cmUtil,
IdentifiedUser.GenericFactory userFactory, IdentifiedUser.GenericFactory userFactory,
VoteDeleted voteDeleted, VoteDeleted voteDeleted,
DeleteVoteSender.Factory deleteVoteSenderFactory) { DeleteVoteSender.Factory deleteVoteSenderFactory,
NotifyUtil notifyUtil) {
this.db = db; this.db = db;
this.batchUpdateFactory = batchUpdateFactory; this.batchUpdateFactory = batchUpdateFactory;
this.approvalsUtil = approvalsUtil; this.approvalsUtil = approvalsUtil;
@ -91,6 +93,7 @@ public class DeleteVote
this.userFactory = userFactory; this.userFactory = userFactory;
this.voteDeleted = voteDeleted; this.voteDeleted = voteDeleted;
this.deleteVoteSenderFactory = deleteVoteSenderFactory; this.deleteVoteSenderFactory = deleteVoteSenderFactory;
this.notifyUtil = notifyUtil;
} }
@Override @Override
@ -208,13 +211,15 @@ public class DeleteVote
} }
IdentifiedUser user = ctx.getIdentifiedUser(); IdentifiedUser user = ctx.getIdentifiedUser();
if (input.notify.compareTo(NotifyHandling.NONE) > 0) { if (NotifyUtil.shouldNotify(input.notify, input.notifyDetails)) {
try { try {
ReplyToChangeSender cm = deleteVoteSenderFactory.create( ReplyToChangeSender cm = deleteVoteSenderFactory.create(
ctx.getProject(), change.getId()); ctx.getProject(), change.getId());
cm.setFrom(user.getAccountId()); cm.setFrom(user.getAccountId());
cm.setChangeMessage(changeMessage.getMessage(), ctx.getWhen()); cm.setChangeMessage(changeMessage.getMessage(), ctx.getWhen());
cm.setNotify(input.notify); cm.setNotify(input.notify);
cm.setAccountsToNotify(
notifyUtil.resolveAccounts(input.notifyDetails));
cm.send(); cm.send();
} catch (Exception e) { } catch (Exception e) {
log.error("Cannot email update for change " + change.getId(), e); log.error("Cannot email update for change " + change.getId(), e);

View File

@ -16,7 +16,10 @@ package com.google.gerrit.server.change;
import static com.google.gerrit.server.CommentsUtil.COMMENT_ORDER; import static com.google.gerrit.server.CommentsUtil.COMMENT_ORDER;
import com.google.common.collect.Multimap;
import com.google.gerrit.extensions.api.changes.NotifyHandling; import com.google.gerrit.extensions.api.changes.NotifyHandling;
import com.google.gerrit.extensions.api.changes.RecipientType;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.ChangeMessage; import com.google.gerrit.reviewdb.client.ChangeMessage;
import com.google.gerrit.reviewdb.client.Comment; import com.google.gerrit.reviewdb.client.Comment;
import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gerrit.reviewdb.client.PatchSet;
@ -48,6 +51,7 @@ public class EmailReviewComments implements Runnable, RequestContext {
interface Factory { interface Factory {
EmailReviewComments create( EmailReviewComments create(
NotifyHandling notify, NotifyHandling notify,
Multimap<RecipientType, Account.Id> accountsToNotify,
ChangeNotes notes, ChangeNotes notes,
PatchSet patchSet, PatchSet patchSet,
IdentifiedUser user, IdentifiedUser user,
@ -62,6 +66,7 @@ public class EmailReviewComments implements Runnable, RequestContext {
private final ThreadLocalRequestContext requestContext; private final ThreadLocalRequestContext requestContext;
private final NotifyHandling notify; private final NotifyHandling notify;
private final Multimap<RecipientType, Account.Id> accountsToNotify;
private final ChangeNotes notes; private final ChangeNotes notes;
private final PatchSet patchSet; private final PatchSet patchSet;
private final IdentifiedUser user; private final IdentifiedUser user;
@ -77,6 +82,7 @@ public class EmailReviewComments implements Runnable, RequestContext {
SchemaFactory<ReviewDb> schemaFactory, SchemaFactory<ReviewDb> schemaFactory,
ThreadLocalRequestContext requestContext, ThreadLocalRequestContext requestContext,
@Assisted NotifyHandling notify, @Assisted NotifyHandling notify,
@Assisted Multimap<RecipientType, Account.Id> accountsToNotify,
@Assisted ChangeNotes notes, @Assisted ChangeNotes notes,
@Assisted PatchSet patchSet, @Assisted PatchSet patchSet,
@Assisted IdentifiedUser user, @Assisted IdentifiedUser user,
@ -88,6 +94,7 @@ public class EmailReviewComments implements Runnable, RequestContext {
this.schemaFactory = schemaFactory; this.schemaFactory = schemaFactory;
this.requestContext = requestContext; this.requestContext = requestContext;
this.notify = notify; this.notify = notify;
this.accountsToNotify = accountsToNotify;
this.notes = notes; this.notes = notes;
this.patchSet = patchSet; this.patchSet = patchSet;
this.user = user; this.user = user;
@ -112,6 +119,7 @@ public class EmailReviewComments implements Runnable, RequestContext {
cm.setChangeMessage(message.getMessage(), message.getWrittenOn()); cm.setChangeMessage(message.getMessage(), message.getWrittenOn());
cm.setComments(comments); cm.setComments(comments);
cm.setNotify(notify); cm.setNotify(notify);
cm.setAccountsToNotify(accountsToNotify);
cm.send(); cm.send();
} catch (Exception e) { } catch (Exception e) {
log.error("Cannot email comments for " + patchSet.getId(), e); log.error("Cannot email comments for " + patchSet.getId(), e);

View File

@ -0,0 +1,122 @@
// Copyright (C) 2016 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.server.change;
import static java.util.stream.Collectors.joining;
import com.google.common.collect.ArrayListMultimap;
import com.google.common.collect.ImmutableListMultimap;
import com.google.common.collect.Multimap;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.extensions.api.changes.NotifyHandling;
import com.google.gerrit.extensions.api.changes.NotifyInfo;
import com.google.gerrit.extensions.api.changes.RecipientType;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.account.AccountResolver;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
@Singleton
public class NotifyUtil {
private final Provider<ReviewDb> dbProvider;
private final AccountResolver accountResolver;
@Inject
NotifyUtil(Provider<ReviewDb> dbProvider,
AccountResolver accountResolver) {
this.dbProvider = dbProvider;
this.accountResolver = accountResolver;
}
public static boolean shouldNotify(NotifyHandling notify,
@Nullable Map<RecipientType, NotifyInfo> notifyDetails) {
if (!isNullOrEmpty(notifyDetails)) {
return true;
}
return notify.compareTo(NotifyHandling.NONE) > 0;
}
private static boolean isNullOrEmpty(
@Nullable Map<RecipientType, NotifyInfo> notifyDetails) {
if (notifyDetails == null || notifyDetails.isEmpty()) {
return true;
}
for (NotifyInfo notifyInfo : notifyDetails.values()) {
if (!isEmpty(notifyInfo)) {
return false;
}
}
return true;
}
private static boolean isEmpty(NotifyInfo notifyInfo) {
return notifyInfo.accounts == null || notifyInfo.accounts.isEmpty();
}
public Multimap<RecipientType, Account.Id> resolveAccounts(
@Nullable Map<RecipientType, NotifyInfo> notifyDetails)
throws OrmException, BadRequestException {
if (isNullOrEmpty(notifyDetails)) {
return ImmutableListMultimap.of();
}
Multimap<RecipientType, Account.Id> m = null;
for (Entry<RecipientType, NotifyInfo> e : notifyDetails.entrySet()) {
List<String> accounts = e.getValue().accounts;
if (accounts != null) {
if (m == null) {
m = ArrayListMultimap.create();
}
m.putAll(e.getKey(), find(dbProvider.get(), accounts));
}
}
return m != null ? m : ImmutableListMultimap.of();
}
private List<Account.Id> find(ReviewDb db, List<String> nameOrEmails)
throws OrmException, BadRequestException {
List<String> missing = new ArrayList<>(nameOrEmails.size());
List<Account.Id> r = new ArrayList<>(nameOrEmails.size());
for (String nameOrEmail : nameOrEmails) {
Account a = accountResolver.find(db, nameOrEmail);
if (a != null) {
r.add(a.getId());
} else {
missing.add(nameOrEmail);
}
}
if (!missing.isEmpty()) {
throw new BadRequestException(
"The following accounts that should be notified could not be resolved: "
+ missing.stream().distinct().sorted().collect(joining(", ")));
}
return r;
}
}

View File

@ -19,9 +19,13 @@ import static com.google.common.base.Preconditions.checkState;
import static com.google.gerrit.server.notedb.ReviewerStateInternal.CC; import static com.google.gerrit.server.notedb.ReviewerStateInternal.CC;
import static com.google.gerrit.server.notedb.ReviewerStateInternal.REVIEWER; import static com.google.gerrit.server.notedb.ReviewerStateInternal.REVIEWER;
import com.google.common.collect.ImmutableListMultimap;
import com.google.common.collect.Multimap;
import com.google.gerrit.extensions.api.changes.NotifyHandling; import com.google.gerrit.extensions.api.changes.NotifyHandling;
import com.google.gerrit.extensions.api.changes.RecipientType;
import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.ResourceConflictException; import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.ChangeMessage; import com.google.gerrit.reviewdb.client.ChangeMessage;
import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gerrit.reviewdb.client.PatchSet;
@ -94,6 +98,8 @@ public class PatchSetInserter extends BatchUpdate.Op {
private List<String> groups = Collections.emptyList(); private List<String> groups = Collections.emptyList();
private boolean fireRevisionCreated = true; private boolean fireRevisionCreated = true;
private NotifyHandling notify = NotifyHandling.ALL; private NotifyHandling notify = NotifyHandling.ALL;
private Multimap<RecipientType, Account.Id> accountsToNotify =
ImmutableListMultimap.of();
private boolean allowClosed; private boolean allowClosed;
private boolean copyApprovals = true; private boolean copyApprovals = true;
@ -165,6 +171,12 @@ public class PatchSetInserter extends BatchUpdate.Op {
return this; return this;
} }
public PatchSetInserter setAccountsToNotify(
Multimap<RecipientType, Account.Id> accountsToNotify) {
this.accountsToNotify = checkNotNull(accountsToNotify);
return this;
}
public PatchSetInserter setAllowClosed(boolean allowClosed) { public PatchSetInserter setAllowClosed(boolean allowClosed) {
this.allowClosed = allowClosed; this.allowClosed = allowClosed;
return this; return this;
@ -246,7 +258,7 @@ public class PatchSetInserter extends BatchUpdate.Op {
@Override @Override
public void postUpdate(Context ctx) throws OrmException { public void postUpdate(Context ctx) throws OrmException {
if (notify != NotifyHandling.NONE) { if (notify != NotifyHandling.NONE || !accountsToNotify.isEmpty()) {
try { try {
ReplacePatchSetSender cm = replacePatchSetFactory.create( ReplacePatchSetSender cm = replacePatchSetFactory.create(
ctx.getProject(), change.getId()); ctx.getProject(), change.getId());
@ -256,6 +268,7 @@ public class PatchSetInserter extends BatchUpdate.Op {
cm.addReviewers(oldReviewers.byState(REVIEWER)); cm.addReviewers(oldReviewers.byState(REVIEWER));
cm.addExtraCC(oldReviewers.byState(CC)); cm.addExtraCC(oldReviewers.byState(CC));
cm.setNotify(notify); cm.setNotify(notify);
cm.setAccountsToNotify(accountsToNotify);
cm.send(); cm.send();
} catch (Exception err) { } catch (Exception err) {
log.error("Cannot send email for new patch set on change " log.error("Cannot send email for new patch set on change "

View File

@ -30,6 +30,7 @@ import com.google.common.base.Strings;
import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Lists; import com.google.common.collect.Lists;
import com.google.common.collect.Maps; import com.google.common.collect.Maps;
import com.google.common.collect.Multimap;
import com.google.common.collect.Ordering; import com.google.common.collect.Ordering;
import com.google.common.hash.HashCode; import com.google.common.hash.HashCode;
import com.google.common.hash.Hashing; import com.google.common.hash.Hashing;
@ -42,6 +43,7 @@ import com.google.gerrit.common.data.PermissionRange;
import com.google.gerrit.extensions.api.changes.AddReviewerInput; import com.google.gerrit.extensions.api.changes.AddReviewerInput;
import com.google.gerrit.extensions.api.changes.AddReviewerResult; import com.google.gerrit.extensions.api.changes.AddReviewerResult;
import com.google.gerrit.extensions.api.changes.NotifyHandling; import com.google.gerrit.extensions.api.changes.NotifyHandling;
import com.google.gerrit.extensions.api.changes.RecipientType;
import com.google.gerrit.extensions.api.changes.ReviewInput; import com.google.gerrit.extensions.api.changes.ReviewInput;
import com.google.gerrit.extensions.api.changes.ReviewInput.CommentInput; import com.google.gerrit.extensions.api.changes.ReviewInput.CommentInput;
import com.google.gerrit.extensions.api.changes.ReviewInput.DraftHandling; import com.google.gerrit.extensions.api.changes.ReviewInput.DraftHandling;
@ -130,6 +132,7 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
private final CommentAdded commentAdded; private final CommentAdded commentAdded;
private final PostReviewers postReviewers; private final PostReviewers postReviewers;
private final NotesMigration migration; private final NotesMigration migration;
private final NotifyUtil notifyUtil;
@Inject @Inject
PostReview(Provider<ReviewDb> db, PostReview(Provider<ReviewDb> db,
@ -145,7 +148,8 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
EmailReviewComments.Factory email, EmailReviewComments.Factory email,
CommentAdded commentAdded, CommentAdded commentAdded,
PostReviewers postReviewers, PostReviewers postReviewers,
NotesMigration migration) { NotesMigration migration,
NotifyUtil notifyUtil) {
this.db = db; this.db = db;
this.batchUpdateFactory = batchUpdateFactory; this.batchUpdateFactory = batchUpdateFactory;
this.changes = changes; this.changes = changes;
@ -160,6 +164,7 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
this.commentAdded = commentAdded; this.commentAdded = commentAdded;
this.postReviewers = postReviewers; this.postReviewers = postReviewers;
this.migration = migration; this.migration = migration;
this.notifyUtil = notifyUtil;
} }
@Override @Override
@ -198,6 +203,9 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
input.notify = NotifyHandling.NONE; input.notify = NotifyHandling.NONE;
} }
Multimap<RecipientType, Account.Id> accountsToNotify =
notifyUtil.resolveAccounts(input.notifyDetails);
Map<String, AddReviewerResult> reviewerJsonResults = null; Map<String, AddReviewerResult> reviewerJsonResults = null;
List<PostReviewers.Addition> reviewerResults = Lists.newArrayList(); List<PostReviewers.Addition> reviewerResults = Lists.newArrayList();
boolean hasError = false; boolean hasError = false;
@ -274,23 +282,25 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
bu.addOp(revision.getChange().getId(), selfAddition.op); bu.addOp(revision.getChange().getId(), selfAddition.op);
} }
bu.addOp( bu.addOp(revision.getChange().getId(),
revision.getChange().getId(), new Op(revision.getPatchSet().getId(), input, accountsToNotify,
new Op(revision.getPatchSet().getId(), input, reviewerResults)); reviewerResults));
bu.execute(); bu.execute();
for (PostReviewers.Addition reviewerResult : reviewerResults) { for (PostReviewers.Addition reviewerResult : reviewerResults) {
reviewerResult.gatherResults(); reviewerResult.gatherResults();
} }
emailReviewers(revision.getChange(), reviewerResults, input.notify); emailReviewers(revision.getChange(), reviewerResults, input.notify,
accountsToNotify);
} }
return Response.ok(output); return Response.ok(output);
} }
private void emailReviewers(Change change, private void emailReviewers(Change change,
List<PostReviewers.Addition> reviewerAdditions, NotifyHandling notify) { List<PostReviewers.Addition> reviewerAdditions, NotifyHandling notify,
Multimap<RecipientType, Account.Id> accountsToNotify) {
List<Account.Id> to = new ArrayList<>(); List<Account.Id> to = new ArrayList<>();
List<Account.Id> cc = new ArrayList<>(); List<Account.Id> cc = new ArrayList<>();
for (PostReviewers.Addition addition : reviewerAdditions) { for (PostReviewers.Addition addition : reviewerAdditions) {
@ -300,7 +310,7 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
cc.addAll(addition.op.reviewers.keySet()); cc.addAll(addition.op.reviewers.keySet());
} }
} }
postReviewers.emailReviewers(change, to, cc, notify); postReviewers.emailReviewers(change, to, cc, notify, accountsToNotify);
} }
private RevisionResource onBehalfOf(RevisionResource rev, ReviewInput in) private RevisionResource onBehalfOf(RevisionResource rev, ReviewInput in)
@ -551,6 +561,7 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
private class Op extends BatchUpdate.Op { private class Op extends BatchUpdate.Op {
private final PatchSet.Id psId; private final PatchSet.Id psId;
private final ReviewInput in; private final ReviewInput in;
private final Multimap<RecipientType, Account.Id> accountsToNotify;
private final List<PostReviewers.Addition> reviewerResults; private final List<PostReviewers.Addition> reviewerResults;
private IdentifiedUser user; private IdentifiedUser user;
@ -563,9 +574,11 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
private Map<String, Short> oldApprovals = new HashMap<>(); private Map<String, Short> oldApprovals = new HashMap<>();
private Op(PatchSet.Id psId, ReviewInput in, private Op(PatchSet.Id psId, ReviewInput in,
Multimap<RecipientType, Account.Id> accountsToNotify,
List<PostReviewers.Addition> reviewerResults) { List<PostReviewers.Addition> reviewerResults) {
this.psId = psId; this.psId = psId;
this.in = in; this.in = in;
this.accountsToNotify = checkNotNull(accountsToNotify);
this.reviewerResults = reviewerResults; this.reviewerResults = reviewerResults;
} }
@ -584,13 +597,15 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
} }
@Override @Override
public void postUpdate(Context ctx) { public void postUpdate(Context ctx) throws OrmException {
if (message == null) { if (message == null) {
return; return;
} }
if (in.notify.compareTo(NotifyHandling.NONE) > 0) { if (in.notify.compareTo(NotifyHandling.NONE) > 0
|| !accountsToNotify.isEmpty()) {
email.create( email.create(
in.notify, in.notify,
accountsToNotify,
notes, notes,
ps, ps,
user, user,

View File

@ -14,18 +14,22 @@
package com.google.gerrit.server.change; package com.google.gerrit.server.change;
import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.gerrit.extensions.client.ReviewerState.CC; import static com.google.gerrit.extensions.client.ReviewerState.CC;
import static com.google.gerrit.extensions.client.ReviewerState.REVIEWER; import static com.google.gerrit.extensions.client.ReviewerState.REVIEWER;
import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableListMultimap;
import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Lists; import com.google.common.collect.Lists;
import com.google.common.collect.Multimap;
import com.google.gerrit.common.TimeUtil; import com.google.gerrit.common.TimeUtil;
import com.google.gerrit.common.data.GroupDescription; import com.google.gerrit.common.data.GroupDescription;
import com.google.gerrit.common.errors.NoSuchGroupException; import com.google.gerrit.common.errors.NoSuchGroupException;
import com.google.gerrit.extensions.api.changes.AddReviewerInput; import com.google.gerrit.extensions.api.changes.AddReviewerInput;
import com.google.gerrit.extensions.api.changes.AddReviewerResult; import com.google.gerrit.extensions.api.changes.AddReviewerResult;
import com.google.gerrit.extensions.api.changes.NotifyHandling; import com.google.gerrit.extensions.api.changes.NotifyHandling;
import com.google.gerrit.extensions.api.changes.RecipientType;
import com.google.gerrit.extensions.api.changes.ReviewerInfo; import com.google.gerrit.extensions.api.changes.ReviewerInfo;
import com.google.gerrit.extensions.client.ReviewerState; import com.google.gerrit.extensions.client.ReviewerState;
import com.google.gerrit.extensions.restapi.BadRequestException; import com.google.gerrit.extensions.restapi.BadRequestException;
@ -102,6 +106,7 @@ public class PostReviewers
private final ReviewerAdded reviewerAdded; private final ReviewerAdded reviewerAdded;
private final NotesMigration migration; private final NotesMigration migration;
private final AccountCache accountCache; private final AccountCache accountCache;
private final NotifyUtil notifyUtil;
@Inject @Inject
PostReviewers(AccountsCollection accounts, PostReviewers(AccountsCollection accounts,
@ -120,7 +125,8 @@ public class PostReviewers
ReviewerJson json, ReviewerJson json,
ReviewerAdded reviewerAdded, ReviewerAdded reviewerAdded,
NotesMigration migration, NotesMigration migration,
AccountCache accountCache) { AccountCache accountCache,
NotifyUtil notifyUtil) {
this.accounts = accounts; this.accounts = accounts;
this.reviewerFactory = reviewerFactory; this.reviewerFactory = reviewerFactory;
this.approvalsUtil = approvalsUtil; this.approvalsUtil = approvalsUtil;
@ -138,6 +144,7 @@ public class PostReviewers
this.reviewerAdded = reviewerAdded; this.reviewerAdded = reviewerAdded;
this.migration = migration; this.migration = migration;
this.accountCache = accountCache; this.accountCache = accountCache;
this.notifyUtil = notifyUtil;
} }
@Override @Override
@ -181,24 +188,27 @@ public class PostReviewers
.format(ChangeMessages.get().reviewerNotFoundUser, input.reviewer)); .format(ChangeMessages.get().reviewerNotFoundUser, input.reviewer));
} }
return putAccount(input.reviewer, reviewerFactory.create(rsrc, accountId), return putAccount(input.reviewer, reviewerFactory.create(rsrc, accountId),
input.state(), input.notify); input.state(), input.notify,
notifyUtil.resolveAccounts(input.notifyDetails));
} }
Addition ccCurrentUser(CurrentUser user, RevisionResource revision) { Addition ccCurrentUser(CurrentUser user, RevisionResource revision) {
return new Addition( return new Addition(
user.getUserName(), revision.getChangeResource(), user.getUserName(), revision.getChangeResource(),
ImmutableMap.of(user.getAccountId(), revision.getControl()), ImmutableMap.of(user.getAccountId(), revision.getControl()),
CC, NotifyHandling.NONE); CC, NotifyHandling.NONE, ImmutableListMultimap.of());
} }
private Addition putAccount(String reviewer, ReviewerResource rsrc, private Addition putAccount(String reviewer, ReviewerResource rsrc,
ReviewerState state, NotifyHandling notify) ReviewerState state, NotifyHandling notify,
throws UnprocessableEntityException { Multimap<RecipientType, Account.Id> accountsToNotify)
throws UnprocessableEntityException {
Account member = rsrc.getReviewerUser().getAccount(); Account member = rsrc.getReviewerUser().getAccount();
ChangeControl control = rsrc.getReviewerControl(); ChangeControl control = rsrc.getReviewerControl();
if (isValidReviewer(member, control)) { if (isValidReviewer(member, control)) {
return new Addition(reviewer, rsrc.getChangeResource(), return new Addition(reviewer, rsrc.getChangeResource(),
ImmutableMap.of(member.getId(), control), state, notify); ImmutableMap.of(member.getId(), control), state, notify,
accountsToNotify);
} }
if (member.isActive()) { if (member.isActive()) {
throw new UnprocessableEntityException( throw new UnprocessableEntityException(
@ -256,7 +266,7 @@ public class PostReviewers
} }
return new Addition(input.reviewer, rsrc, reviewers, input.state(), return new Addition(input.reviewer, rsrc, reviewers, input.state(),
input.notify); input.notify, notifyUtil.resolveAccounts(input.notifyDetails));
} }
private boolean isValidReviewer(Account member, ChangeControl control) { private boolean isValidReviewer(Account member, ChangeControl control) {
@ -287,12 +297,13 @@ public class PostReviewers
private final Map<Account.Id, ChangeControl> reviewers; private final Map<Account.Id, ChangeControl> reviewers;
protected Addition(String reviewer) { protected Addition(String reviewer) {
this(reviewer, null, null, REVIEWER, null); this(reviewer, null, null, REVIEWER, null, ImmutableListMultimap.of());
} }
protected Addition(String reviewer, ChangeResource rsrc, protected Addition(String reviewer, ChangeResource rsrc,
Map<Account.Id, ChangeControl> reviewers, ReviewerState state, Map<Account.Id, ChangeControl> reviewers, ReviewerState state,
NotifyHandling notify) { NotifyHandling notify,
Multimap<RecipientType, Account.Id> accountsToNotify) {
result = new AddReviewerResult(reviewer); result = new AddReviewerResult(reviewer);
if (reviewers == null) { if (reviewers == null) {
this.reviewers = ImmutableMap.of(); this.reviewers = ImmutableMap.of();
@ -300,7 +311,7 @@ public class PostReviewers
return; return;
} }
this.reviewers = reviewers; this.reviewers = reviewers;
op = new Op(rsrc, reviewers, state, notify); op = new Op(rsrc, reviewers, state, notify, accountsToNotify);
} }
void gatherResults() throws OrmException { void gatherResults() throws OrmException {
@ -331,6 +342,7 @@ public class PostReviewers
final Map<Account.Id, ChangeControl> reviewers; final Map<Account.Id, ChangeControl> reviewers;
final ReviewerState state; final ReviewerState state;
final NotifyHandling notify; final NotifyHandling notify;
final Multimap<RecipientType, Account.Id> accountsToNotify;
List<PatchSetApproval> addedReviewers; List<PatchSetApproval> addedReviewers;
Collection<Account.Id> addedCCs; Collection<Account.Id> addedCCs;
@ -338,11 +350,13 @@ public class PostReviewers
private PatchSet patchSet; private PatchSet patchSet;
Op(ChangeResource rsrc, Map<Account.Id, ChangeControl> reviewers, Op(ChangeResource rsrc, Map<Account.Id, ChangeControl> reviewers,
ReviewerState state, NotifyHandling notify) { ReviewerState state, NotifyHandling notify,
Multimap<RecipientType, Account.Id> accountsToNotify) {
this.rsrc = rsrc; this.rsrc = rsrc;
this.reviewers = reviewers; this.reviewers = reviewers;
this.state = state; this.state = state;
this.notify = notify; this.notify = notify;
this.accountsToNotify = checkNotNull(accountsToNotify);
} }
@Override @Override
@ -380,7 +394,7 @@ public class PostReviewers
} }
emailReviewers(rsrc.getChange(), emailReviewers(rsrc.getChange(),
Lists.transform(addedReviewers, r -> r.getAccountId()), addedCCs, Lists.transform(addedReviewers, r -> r.getAccountId()), addedCCs,
notify); notify, accountsToNotify);
if (!addedReviewers.isEmpty()) { if (!addedReviewers.isEmpty()) {
List<Account> reviewers = Lists.transform(addedReviewers, List<Account> reviewers = Lists.transform(addedReviewers,
psa -> accountCache.get(psa.getAccountId()).getAccount()); psa -> accountCache.get(psa.getAccountId()).getAccount());
@ -392,7 +406,8 @@ public class PostReviewers
} }
public void emailReviewers(Change change, Collection<Account.Id> added, public void emailReviewers(Change change, Collection<Account.Id> added,
Collection<Account.Id> copied, NotifyHandling notify) { Collection<Account.Id> copied, NotifyHandling notify,
Multimap<RecipientType, Account.Id> accountsToNotify) {
if (added.isEmpty() && copied.isEmpty()) { if (added.isEmpty() && copied.isEmpty()) {
return; return;
} }
@ -419,7 +434,11 @@ public class PostReviewers
try { try {
AddReviewerSender cm = addReviewerSenderFactory AddReviewerSender cm = addReviewerSenderFactory
.create(change.getProject(), change.getId(), notify); .create(change.getProject(), change.getId());
if (notify != null) {
cm.setNotify(notify);
}
cm.setAccountsToNotify(accountsToNotify);
cm.setFrom(userId); cm.setFrom(userId);
cm.addReviewers(toMail); cm.addReviewers(toMail);
cm.addExtraCC(toCopy); cm.addExtraCC(toCopy);

View File

@ -76,10 +76,13 @@ public class PublishChangeEdit implements
implements RestModifyView<ChangeResource, PublishChangeEditInput> { implements RestModifyView<ChangeResource, PublishChangeEditInput> {
private final ChangeEditUtil editUtil; private final ChangeEditUtil editUtil;
private final NotifyUtil notifyUtil;
@Inject @Inject
Publish(ChangeEditUtil editUtil) { Publish(ChangeEditUtil editUtil,
NotifyUtil notifyUtil) {
this.editUtil = editUtil; this.editUtil = editUtil;
this.notifyUtil = notifyUtil;
} }
@Override @Override
@ -101,7 +104,8 @@ public class PublishChangeEdit implements
if (in == null) { if (in == null) {
in = new PublishChangeEditInput(); in = new PublishChangeEditInput();
} }
editUtil.publish(edit.get(), in.notify); editUtil.publish(edit.get(), in.notify,
notifyUtil.resolveAccounts(in.notifyDetails));
return Response.none(); return Response.none();
} }
} }

View File

@ -16,12 +16,15 @@ package com.google.gerrit.server.edit;
import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkArgument;
import com.google.common.collect.Multimap;
import com.google.gerrit.common.TimeUtil; import com.google.gerrit.common.TimeUtil;
import com.google.gerrit.extensions.api.changes.NotifyHandling; import com.google.gerrit.extensions.api.changes.NotifyHandling;
import com.google.gerrit.extensions.api.changes.RecipientType;
import com.google.gerrit.extensions.client.ChangeKind; import com.google.gerrit.extensions.client.ChangeKind;
import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.ResourceConflictException; import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.RestApiException; import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Change.Status; import com.google.gerrit.reviewdb.client.Change.Status;
import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gerrit.reviewdb.client.PatchSet;
@ -155,19 +158,23 @@ public class ChangeEditUtil {
} }
/** /**
* Promote change edit to patch set, by squashing the edit into * Promote change edit to patch set, by squashing the edit into its parent.
* its parent.
* *
* @param edit change edit to publish * @param edit change edit to publish
* @param notify Notify handling that defines to whom email notifications
* should be sent after the change edit is published.
* @param accountsToNotify Accounts that should be notified after the change
* edit is published.
* @throws NoSuchChangeException * @throws NoSuchChangeException
* @throws IOException * @throws IOException
* @throws OrmException * @throws OrmException
* @throws UpdateException * @throws UpdateException
* @throws RestApiException * @throws RestApiException
*/ */
public void publish(final ChangeEdit edit, NotifyHandling notify) public void publish(final ChangeEdit edit, NotifyHandling notify,
throws NoSuchChangeException, IOException, OrmException, RestApiException, Multimap<RecipientType, Account.Id> accountsToNotify)
UpdateException { throws NoSuchChangeException, IOException, OrmException,
RestApiException, UpdateException {
Change change = edit.getChange(); Change change = edit.getChange();
try (Repository repo = gitManager.openRepository(change.getProject()); try (Repository repo = gitManager.openRepository(change.getProject());
RevWalk rw = new RevWalk(repo); RevWalk rw = new RevWalk(repo);
@ -185,7 +192,8 @@ public class ChangeEditUtil {
ChangeUtil.nextPatchSetId(repo, change.currentPatchSetId()); ChangeUtil.nextPatchSetId(repo, change.currentPatchSetId());
PatchSetInserter inserter = patchSetInserterFactory PatchSetInserter inserter = patchSetInserterFactory
.create(ctl, psId, squashed) .create(ctl, psId, squashed)
.setNotify(notify); .setNotify(notify)
.setAccountsToNotify(accountsToNotify);
StringBuilder message = new StringBuilder("Patch Set ") StringBuilder message = new StringBuilder("Patch Set ")
.append(inserter.getPatchSetId().get()) .append(inserter.getPatchSetId().get())

View File

@ -15,8 +15,10 @@
package com.google.gerrit.server.git; package com.google.gerrit.server.git;
import com.google.common.base.Strings; import com.google.common.base.Strings;
import com.google.common.collect.Multimap;
import com.google.gerrit.common.Nullable; import com.google.gerrit.common.Nullable;
import com.google.gerrit.extensions.api.changes.NotifyHandling; import com.google.gerrit.extensions.api.changes.NotifyHandling;
import com.google.gerrit.extensions.api.changes.RecipientType;
import com.google.gerrit.extensions.restapi.ResourceConflictException; import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.Change;
@ -47,6 +49,7 @@ public class AbandonOp extends BatchUpdate.Op {
private final String msgTxt; private final String msgTxt;
private final NotifyHandling notifyHandling; private final NotifyHandling notifyHandling;
private final Multimap<RecipientType, Account.Id> accountsToNotify;
private final Account account; private final Account account;
private Change change; private Change change;
@ -57,7 +60,8 @@ public class AbandonOp extends BatchUpdate.Op {
AbandonOp create( AbandonOp create(
@Assisted @Nullable Account account, @Assisted @Nullable Account account,
@Assisted @Nullable String msgTxt, @Assisted @Nullable String msgTxt,
@Assisted NotifyHandling notifyHandling); @Assisted NotifyHandling notifyHandling,
@Assisted Multimap<RecipientType, Account.Id> accountsToNotify);
} }
@AssistedInject @AssistedInject
@ -68,7 +72,8 @@ public class AbandonOp extends BatchUpdate.Op {
ChangeAbandoned changeAbandoned, ChangeAbandoned changeAbandoned,
@Assisted @Nullable Account account, @Assisted @Nullable Account account,
@Assisted @Nullable String msgTxt, @Assisted @Nullable String msgTxt,
@Assisted NotifyHandling notifyHandling) { @Assisted NotifyHandling notifyHandling,
@Assisted Multimap<RecipientType, Account.Id> accountsToNotify) {
this.abandonedSenderFactory = abandonedSenderFactory; this.abandonedSenderFactory = abandonedSenderFactory;
this.cmUtil = cmUtil; this.cmUtil = cmUtil;
this.psUtil = psUtil; this.psUtil = psUtil;
@ -77,6 +82,7 @@ public class AbandonOp extends BatchUpdate.Op {
this.account = account; this.account = account;
this.msgTxt = Strings.nullToEmpty(msgTxt); this.msgTxt = Strings.nullToEmpty(msgTxt);
this.notifyHandling = notifyHandling; this.notifyHandling = notifyHandling;
this.accountsToNotify = accountsToNotify;
} }
@Nullable @Nullable
@ -127,6 +133,7 @@ public class AbandonOp extends BatchUpdate.Op {
} }
cm.setChangeMessage(message.getMessage(), ctx.getWhen()); cm.setChangeMessage(message.getMessage(), ctx.getWhen());
cm.setNotify(notifyHandling); cm.setNotify(notifyHandling);
cm.setAccountsToNotify(accountsToNotify);
cm.send(); cm.send();
} catch (Exception e) { } catch (Exception e) {
log.error("Cannot email update for change " + change.getId(), e); log.error("Cannot email update for change " + change.getId(), e);

View File

@ -14,8 +14,10 @@
package com.google.gerrit.server.git; package com.google.gerrit.server.git;
import com.google.common.collect.Multimap;
import com.google.gerrit.common.Nullable; import com.google.gerrit.common.Nullable;
import com.google.gerrit.extensions.api.changes.NotifyHandling; import com.google.gerrit.extensions.api.changes.NotifyHandling;
import com.google.gerrit.extensions.api.changes.RecipientType;
import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.client.Project;
@ -43,7 +45,8 @@ public class EmailMerge implements Runnable, RequestContext {
public interface Factory { public interface Factory {
EmailMerge create(Project.NameKey project, Change.Id changeId, EmailMerge create(Project.NameKey project, Change.Id changeId,
Account.Id submitter, NotifyHandling notifyHandling); Account.Id submitter, NotifyHandling notifyHandling,
Multimap<RecipientType, Account.Id> accountsToNotify);
} }
private final ExecutorService sendEmailsExecutor; private final ExecutorService sendEmailsExecutor;
@ -56,6 +59,8 @@ public class EmailMerge implements Runnable, RequestContext {
private final Change.Id changeId; private final Change.Id changeId;
private final Account.Id submitter; private final Account.Id submitter;
private final NotifyHandling notifyHandling; private final NotifyHandling notifyHandling;
private final Multimap<RecipientType, Account.Id> accountsToNotify;
private ReviewDb db; private ReviewDb db;
@Inject @Inject
@ -67,7 +72,8 @@ public class EmailMerge implements Runnable, RequestContext {
@Assisted Project.NameKey project, @Assisted Project.NameKey project,
@Assisted Change.Id changeId, @Assisted Change.Id changeId,
@Assisted @Nullable Account.Id submitter, @Assisted @Nullable Account.Id submitter,
@Assisted NotifyHandling notifyHandling) { @Assisted NotifyHandling notifyHandling,
@Assisted Multimap<RecipientType, Account.Id> accountsToNotify) {
this.sendEmailsExecutor = executor; this.sendEmailsExecutor = executor;
this.mergedSenderFactory = mergedSenderFactory; this.mergedSenderFactory = mergedSenderFactory;
this.schemaFactory = schemaFactory; this.schemaFactory = schemaFactory;
@ -77,6 +83,7 @@ public class EmailMerge implements Runnable, RequestContext {
this.changeId = changeId; this.changeId = changeId;
this.submitter = submitter; this.submitter = submitter;
this.notifyHandling = notifyHandling; this.notifyHandling = notifyHandling;
this.accountsToNotify = accountsToNotify;
} }
public void sendAsync() { public void sendAsync() {
@ -92,6 +99,7 @@ public class EmailMerge implements Runnable, RequestContext {
cm.setFrom(submitter); cm.setFrom(submitter);
} }
cm.setNotify(notifyHandling); cm.setNotify(notifyHandling);
cm.setAccountsToNotify(accountsToNotify);
cm.send(); cm.send();
} catch (Exception e) { } catch (Exception e) {
log.error("Cannot email merged notification for " + changeId, e); log.error("Cannot email merged notification for " + changeId, e);

View File

@ -33,11 +33,13 @@ import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.TimeUtil; import com.google.gerrit.common.TimeUtil;
import com.google.gerrit.common.data.SubmitRecord; import com.google.gerrit.common.data.SubmitRecord;
import com.google.gerrit.common.data.SubmitTypeRecord; import com.google.gerrit.common.data.SubmitTypeRecord;
import com.google.gerrit.extensions.api.changes.RecipientType;
import com.google.gerrit.extensions.api.changes.SubmitInput; import com.google.gerrit.extensions.api.changes.SubmitInput;
import com.google.gerrit.extensions.client.SubmitType; import com.google.gerrit.extensions.client.SubmitType;
import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.ResourceConflictException; import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.RestApiException; import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Branch; import com.google.gerrit.reviewdb.client.Branch;
import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.ChangeMessage; import com.google.gerrit.reviewdb.client.ChangeMessage;
@ -47,6 +49,7 @@ import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.ChangeMessagesUtil; import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.InternalUser; import com.google.gerrit.server.InternalUser;
import com.google.gerrit.server.change.NotifyUtil;
import com.google.gerrit.server.git.BatchUpdate.ChangeContext; import com.google.gerrit.server.git.BatchUpdate.ChangeContext;
import com.google.gerrit.server.git.MergeOpRepoManager.OpenBranch; import com.google.gerrit.server.git.MergeOpRepoManager.OpenBranch;
import com.google.gerrit.server.git.MergeOpRepoManager.OpenRepo; import com.google.gerrit.server.git.MergeOpRepoManager.OpenRepo;
@ -217,6 +220,7 @@ public class MergeOp implements AutoCloseable {
private final SubmitStrategyFactory submitStrategyFactory; private final SubmitStrategyFactory submitStrategyFactory;
private final SubmoduleOp.Factory subOpFactory; private final SubmoduleOp.Factory subOpFactory;
private final MergeOpRepoManager orm; private final MergeOpRepoManager orm;
private final NotifyUtil notifyUtil;
private Timestamp ts; private Timestamp ts;
private RequestId submissionId; private RequestId submissionId;
@ -225,6 +229,7 @@ public class MergeOp implements AutoCloseable {
private CommitStatus commits; private CommitStatus commits;
private ReviewDb db; private ReviewDb db;
private SubmitInput submitInput; private SubmitInput submitInput;
private Multimap<RecipientType, Account.Id> accountsToNotify;
private Set<Project.NameKey> allProjects; private Set<Project.NameKey> allProjects;
private boolean dryrun; private boolean dryrun;
@ -237,7 +242,8 @@ public class MergeOp implements AutoCloseable {
InternalChangeQuery internalChangeQuery, InternalChangeQuery internalChangeQuery,
SubmitStrategyFactory submitStrategyFactory, SubmitStrategyFactory submitStrategyFactory,
SubmoduleOp.Factory subOpFactory, SubmoduleOp.Factory subOpFactory,
MergeOpRepoManager orm) { MergeOpRepoManager orm,
NotifyUtil notifyUtil) {
this.cmUtil = cmUtil; this.cmUtil = cmUtil;
this.batchUpdateFactory = batchUpdateFactory; this.batchUpdateFactory = batchUpdateFactory;
this.internalUserFactory = internalUserFactory; this.internalUserFactory = internalUserFactory;
@ -247,6 +253,7 @@ public class MergeOp implements AutoCloseable {
this.submitStrategyFactory = submitStrategyFactory; this.submitStrategyFactory = submitStrategyFactory;
this.subOpFactory = subOpFactory; this.subOpFactory = subOpFactory;
this.orm = orm; this.orm = orm;
this.notifyUtil = notifyUtil;
} }
@Override @Override
@ -398,6 +405,8 @@ public class MergeOp implements AutoCloseable {
boolean checkSubmitRules, SubmitInput submitInput, boolean dryrun) boolean checkSubmitRules, SubmitInput submitInput, boolean dryrun)
throws OrmException, RestApiException { throws OrmException, RestApiException {
this.submitInput = submitInput; this.submitInput = submitInput;
this.accountsToNotify =
notifyUtil.resolveAccounts(submitInput.notifyDetails);
this.dryrun = dryrun; this.dryrun = dryrun;
this.caller = caller; this.caller = caller;
this.ts = TimeUtil.nowTs(); this.ts = TimeUtil.nowTs();
@ -545,8 +554,8 @@ public class MergeOp implements AutoCloseable {
throws IntegrationException { throws IntegrationException {
return submitStrategyFactory.create(submitType, db, or.repo, or.rw, or.ins, return submitStrategyFactory.create(submitType, db, or.repo, or.rw, or.ins,
or.canMergeFlag, getAlreadyAccepted(or, branchTip), destBranch, caller, or.canMergeFlag, getAlreadyAccepted(or, branchTip), destBranch, caller,
mergeTip, commits, submissionId, submitInput.notify, submoduleOp, mergeTip, commits, submissionId, submitInput.notify, accountsToNotify,
dryrun); submoduleOp, dryrun);
} }
private Set<RevCommit> getAlreadyAccepted(OpenRepo or, private Set<RevCommit> getAlreadyAccepted(OpenRepo or,

View File

@ -46,6 +46,7 @@ import com.google.common.collect.LinkedListMultimap;
import com.google.common.collect.ListMultimap; import com.google.common.collect.ListMultimap;
import com.google.common.collect.Lists; import com.google.common.collect.Lists;
import com.google.common.collect.Maps; import com.google.common.collect.Maps;
import com.google.common.collect.Multimap;
import com.google.common.collect.SetMultimap; import com.google.common.collect.SetMultimap;
import com.google.common.collect.Sets; import com.google.common.collect.Sets;
import com.google.common.collect.SortedSetMultimap; import com.google.common.collect.SortedSetMultimap;
@ -58,6 +59,7 @@ import com.google.gerrit.common.data.Permission;
import com.google.gerrit.common.data.PermissionRule; import com.google.gerrit.common.data.PermissionRule;
import com.google.gerrit.extensions.api.changes.HashtagsInput; import com.google.gerrit.extensions.api.changes.HashtagsInput;
import com.google.gerrit.extensions.api.changes.NotifyHandling; import com.google.gerrit.extensions.api.changes.NotifyHandling;
import com.google.gerrit.extensions.api.changes.RecipientType;
import com.google.gerrit.extensions.api.changes.SubmitInput; import com.google.gerrit.extensions.api.changes.SubmitInput;
import com.google.gerrit.extensions.api.projects.ProjectConfigEntryType; import com.google.gerrit.extensions.api.projects.ProjectConfigEntryType;
import com.google.gerrit.extensions.registration.DynamicMap; import com.google.gerrit.extensions.registration.DynamicMap;
@ -1250,6 +1252,18 @@ public class ReceiveCommits {
+ "OWNER_REVIEWERS, ALL. If not set, the default is ALL.") + "OWNER_REVIEWERS, ALL. If not set, the default is ALL.")
NotifyHandling notify = NotifyHandling.ALL; NotifyHandling notify = NotifyHandling.ALL;
@Option(name = "--notify-to", metaVar = "USER",
usage = "user that should be notified")
List<Account.Id> tos = new ArrayList<>();
@Option(name = "--notify-cc", metaVar = "USER",
usage = "user that should be CC'd")
List<Account.Id> ccs = new ArrayList<>();
@Option(name = "--notify-bcc", metaVar = "USER",
usage = "user that should be BCC'd")
List<Account.Id> bccs = new ArrayList<>();
@Option(name = "--reviewer", aliases = {"-r"}, metaVar = "EMAIL", @Option(name = "--reviewer", aliases = {"-r"}, metaVar = "EMAIL",
usage = "add reviewer to changes") usage = "add reviewer to changes")
void reviewer(Account.Id id) { void reviewer(Account.Id id) {
@ -1311,6 +1325,15 @@ public class ReceiveCommits {
return new MailRecipients(reviewer, cc); return new MailRecipients(reviewer, cc);
} }
Multimap<RecipientType, Account.Id> getAccountsToNotify() {
Multimap<RecipientType, Account.Id> accountsToNotify =
ArrayListMultimap.create();
accountsToNotify.putAll(RecipientType.TO, tos);
accountsToNotify.putAll(RecipientType.CC, ccs);
accountsToNotify.putAll(RecipientType.BCC, bccs);
return accountsToNotify;
}
String parse(CmdLineParser clp, Repository repo, Set<String> refs, String parse(CmdLineParser clp, Repository repo, Set<String> refs,
ListMultimap<String, String> pushOptions) throws CmdLineException { ListMultimap<String, String> pushOptions) throws CmdLineException {
String ref = RefNames.fullName( String ref = RefNames.fullName(
@ -2080,6 +2103,7 @@ public class ReceiveCommits {
.setApprovals(approvals) .setApprovals(approvals)
.setMessage(msg.toString()) .setMessage(msg.toString())
.setNotify(magicBranch.notify) .setNotify(magicBranch.notify)
.setAccountsToNotify(magicBranch.getAccountsToNotify())
.setRequestScopePropagator(requestScopePropagator) .setRequestScopePropagator(requestScopePropagator)
.setSendMail(true) .setSendMail(true)
.setUpdateRef(false) .setUpdateRef(false)

View File

@ -381,8 +381,9 @@ public class ReplaceOp extends BatchUpdate.Op {
cm.setFrom(account.getId()); cm.setFrom(account.getId());
cm.setPatchSet(newPatchSet, info); cm.setPatchSet(newPatchSet, info);
cm.setChangeMessage(msg.getMessage(), ctx.getWhen()); cm.setChangeMessage(msg.getMessage(), ctx.getWhen());
if (magicBranch != null && magicBranch.notify != null) { if (magicBranch != null) {
cm.setNotify(magicBranch.notify); cm.setNotify(magicBranch.notify);
cm.setAccountsToNotify(magicBranch.getAccountsToNotify());
} }
cm.addReviewers(recipients.getReviewers()); cm.addReviewers(recipients.getReviewers());
cm.addExtraCC(recipients.getCcOnly()); cm.addExtraCC(recipients.getCcOnly());

View File

@ -16,10 +16,13 @@ package com.google.gerrit.server.git.strategy;
import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.base.Preconditions.checkNotNull;
import com.google.common.collect.Multimap;
import com.google.common.collect.Sets; import com.google.common.collect.Sets;
import com.google.gerrit.extensions.api.changes.NotifyHandling; import com.google.gerrit.extensions.api.changes.NotifyHandling;
import com.google.gerrit.extensions.api.changes.RecipientType;
import com.google.gerrit.extensions.client.SubmitType; import com.google.gerrit.extensions.client.SubmitType;
import com.google.gerrit.extensions.config.FactoryModule; import com.google.gerrit.extensions.config.FactoryModule;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Branch; import com.google.gerrit.reviewdb.client.Branch;
import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.ApprovalsUtil; import com.google.gerrit.server.ApprovalsUtil;
@ -96,6 +99,7 @@ public abstract class SubmitStrategy {
Set<RevCommit> alreadyAccepted, Set<RevCommit> alreadyAccepted,
RequestId submissionId, RequestId submissionId,
NotifyHandling notifyHandling, NotifyHandling notifyHandling,
Multimap<RecipientType, Account.Id> accountsToNotify,
SubmoduleOp submoduleOp, SubmoduleOp submoduleOp,
boolean dryrun); boolean dryrun);
} }
@ -129,6 +133,7 @@ public abstract class SubmitStrategy {
final RequestId submissionId; final RequestId submissionId;
final SubmitType submitType; final SubmitType submitType;
final NotifyHandling notifyHandling; final NotifyHandling notifyHandling;
final Multimap<RecipientType, Account.Id> accountsToNotify;
final SubmoduleOp submoduleOp; final SubmoduleOp submoduleOp;
final ProjectState project; final ProjectState project;
@ -167,6 +172,7 @@ public abstract class SubmitStrategy {
@Assisted RequestId submissionId, @Assisted RequestId submissionId,
@Assisted SubmitType submitType, @Assisted SubmitType submitType,
@Assisted NotifyHandling notifyHandling, @Assisted NotifyHandling notifyHandling,
@Assisted Multimap<RecipientType, Account.Id> accountsToNotify,
@Assisted SubmoduleOp submoduleOp, @Assisted SubmoduleOp submoduleOp,
@Assisted boolean dryrun) { @Assisted boolean dryrun) {
this.accountCache = accountCache; this.accountCache = accountCache;
@ -198,6 +204,7 @@ public abstract class SubmitStrategy {
this.submissionId = submissionId; this.submissionId = submissionId;
this.submitType = submitType; this.submitType = submitType;
this.notifyHandling = notifyHandling; this.notifyHandling = notifyHandling;
this.accountsToNotify = accountsToNotify;
this.submoduleOp = submoduleOp; this.submoduleOp = submoduleOp;
this.dryrun = dryrun; this.dryrun = dryrun;

View File

@ -14,8 +14,11 @@
package com.google.gerrit.server.git.strategy; package com.google.gerrit.server.git.strategy;
import com.google.common.collect.Multimap;
import com.google.gerrit.extensions.api.changes.NotifyHandling; import com.google.gerrit.extensions.api.changes.NotifyHandling;
import com.google.gerrit.extensions.api.changes.RecipientType;
import com.google.gerrit.extensions.client.SubmitType; import com.google.gerrit.extensions.client.SubmitType;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Branch; import com.google.gerrit.reviewdb.client.Branch;
import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.IdentifiedUser;
@ -55,11 +58,13 @@ public class SubmitStrategyFactory {
RevFlag canMergeFlag, Set<RevCommit> alreadyAccepted, RevFlag canMergeFlag, Set<RevCommit> alreadyAccepted,
Branch.NameKey destBranch, IdentifiedUser caller, MergeTip mergeTip, Branch.NameKey destBranch, IdentifiedUser caller, MergeTip mergeTip,
CommitStatus commits, RequestId submissionId, CommitStatus commits, RequestId submissionId,
NotifyHandling notifyHandling, SubmoduleOp submoduleOp, boolean dryrun) NotifyHandling notifyHandling,
throws IntegrationException { Multimap<RecipientType, Account.Id> accountsToNotify,
SubmoduleOp submoduleOp, boolean dryrun) throws IntegrationException {
SubmitStrategy.Arguments args = argsFactory.create(submitType, destBranch, SubmitStrategy.Arguments args = argsFactory.create(submitType, destBranch,
commits, rw, caller, mergeTip, inserter, repo, canMergeFlag, db, commits, rw, caller, mergeTip, inserter, repo, canMergeFlag, db,
alreadyAccepted, submissionId, notifyHandling, submoduleOp, dryrun); alreadyAccepted, submissionId, notifyHandling, accountsToNotify,
submoduleOp, dryrun);
switch (submitType) { switch (submitType) {
case CHERRY_PICK: case CHERRY_PICK:
return new CherryPick(args); return new CherryPick(args);

View File

@ -499,7 +499,7 @@ abstract class SubmitStrategyOp extends BatchUpdate.Op {
try { try {
args.mergedSenderFactory args.mergedSenderFactory
.create(ctx.getProject(), getId(), submitter.getAccountId(), .create(ctx.getProject(), getId(), submitter.getAccountId(),
args.notifyHandling) args.notifyHandling, args.accountsToNotify)
.sendAsync(); .sendAsync();
} catch (Exception e) { } catch (Exception e) {
log.error("Cannot email merged notification for " + getId(), e); log.error("Cannot email merged notification for " + getId(), e);

View File

@ -16,10 +16,10 @@ package com.google.gerrit.server.mail.send;
import com.google.common.base.Joiner; import com.google.common.base.Joiner;
import com.google.gerrit.common.errors.EmailException; import com.google.gerrit.common.errors.EmailException;
import com.google.gerrit.extensions.api.changes.RecipientType;
import com.google.gerrit.reviewdb.client.AccountSshKey; import com.google.gerrit.reviewdb.client.AccountSshKey;
import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.mail.Address; import com.google.gerrit.server.mail.Address;
import com.google.gerrit.server.mail.RecipientType;
import com.google.inject.assistedinject.Assisted; import com.google.inject.assistedinject.Assisted;
import com.google.inject.assistedinject.AssistedInject; import com.google.inject.assistedinject.AssistedInject;

View File

@ -14,9 +14,7 @@
package com.google.gerrit.server.mail.send; package com.google.gerrit.server.mail.send;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.errors.EmailException; import com.google.gerrit.common.errors.EmailException;
import com.google.gerrit.extensions.api.changes.NotifyHandling;
import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.client.Project;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
@ -26,20 +24,15 @@ import com.google.inject.assistedinject.Assisted;
/** Asks a user to review a change. */ /** Asks a user to review a change. */
public class AddReviewerSender extends NewChangeSender { public class AddReviewerSender extends NewChangeSender {
public interface Factory { public interface Factory {
AddReviewerSender create(Project.NameKey project, Change.Id id, AddReviewerSender create(Project.NameKey project, Change.Id id);
NotifyHandling notify);
} }
@Inject @Inject
public AddReviewerSender(EmailArguments ea, public AddReviewerSender(EmailArguments ea,
@Assisted Project.NameKey project, @Assisted Project.NameKey project,
@Assisted Change.Id id, @Assisted Change.Id id)
@Assisted @Nullable NotifyHandling notify)
throws OrmException { throws OrmException {
super(ea, newChangeData(ea, project, id)); super(ea, newChangeData(ea, project, id));
if (notify != null) {
setNotify(notify);
}
} }
@Override @Override

View File

@ -17,6 +17,7 @@ package com.google.gerrit.server.mail.send;
import com.google.common.collect.Multimap; import com.google.common.collect.Multimap;
import com.google.gerrit.common.errors.EmailException; import com.google.gerrit.common.errors.EmailException;
import com.google.gerrit.extensions.api.changes.NotifyHandling; import com.google.gerrit.extensions.api.changes.NotifyHandling;
import com.google.gerrit.extensions.api.changes.RecipientType;
import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountGroup; import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.client.AccountProjectWatch.NotifyType; import com.google.gerrit.reviewdb.client.AccountProjectWatch.NotifyType;
@ -29,7 +30,6 @@ import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.StarredChangesUtil; import com.google.gerrit.server.StarredChangesUtil;
import com.google.gerrit.server.account.AccountState; import com.google.gerrit.server.account.AccountState;
import com.google.gerrit.server.mail.RecipientType;
import com.google.gerrit.server.mail.send.ProjectWatch.Watchers; import com.google.gerrit.server.mail.send.ProjectWatch.Watchers;
import com.google.gerrit.server.notedb.ReviewerStateInternal; import com.google.gerrit.server.notedb.ReviewerStateInternal;
import com.google.gerrit.server.patch.PatchList; import com.google.gerrit.server.patch.PatchList;

View File

@ -16,11 +16,11 @@ package com.google.gerrit.server.mail.send;
import com.google.common.collect.Iterables; import com.google.common.collect.Iterables;
import com.google.gerrit.common.errors.EmailException; import com.google.gerrit.common.errors.EmailException;
import com.google.gerrit.extensions.api.changes.RecipientType;
import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountProjectWatch.NotifyType; import com.google.gerrit.reviewdb.client.AccountProjectWatch.NotifyType;
import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.mail.RecipientType;
import com.google.gerrit.server.mail.send.ProjectWatch.Watchers; import com.google.gerrit.server.mail.send.ProjectWatch.Watchers;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject; import com.google.inject.Inject;

View File

@ -15,11 +15,11 @@
package com.google.gerrit.server.mail.send; package com.google.gerrit.server.mail.send;
import com.google.gerrit.common.errors.EmailException; import com.google.gerrit.common.errors.EmailException;
import com.google.gerrit.extensions.api.changes.RecipientType;
import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountProjectWatch.NotifyType; import com.google.gerrit.reviewdb.client.AccountProjectWatch.NotifyType;
import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.mail.RecipientType;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject; import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted; import com.google.inject.assistedinject.Assisted;

View File

@ -15,8 +15,8 @@
package com.google.gerrit.server.mail.send; package com.google.gerrit.server.mail.send;
import com.google.gerrit.common.errors.EmailException; import com.google.gerrit.common.errors.EmailException;
import com.google.gerrit.extensions.api.changes.RecipientType;
import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.server.mail.RecipientType;
import com.google.gerrit.server.query.change.ChangeData; import com.google.gerrit.server.query.change.ChangeData;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;

View File

@ -16,11 +16,11 @@ package com.google.gerrit.server.mail.send;
import com.google.common.collect.Iterables; import com.google.common.collect.Iterables;
import com.google.gerrit.common.errors.EmailException; import com.google.gerrit.common.errors.EmailException;
import com.google.gerrit.extensions.api.changes.RecipientType;
import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountProjectWatch.NotifyType; import com.google.gerrit.reviewdb.client.AccountProjectWatch.NotifyType;
import com.google.gerrit.reviewdb.client.Branch; import com.google.gerrit.reviewdb.client.Branch;
import com.google.gerrit.server.mail.Address; import com.google.gerrit.server.mail.Address;
import com.google.gerrit.server.mail.RecipientType;
import com.google.gerrit.server.mail.send.ProjectWatch.Watchers; import com.google.gerrit.server.mail.send.ProjectWatch.Watchers;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;

View File

@ -14,18 +14,21 @@
package com.google.gerrit.server.mail.send; package com.google.gerrit.server.mail.send;
import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.gerrit.extensions.client.GeneralPreferencesInfo.EmailStrategy.CC_ON_OWN_COMMENTS; import static com.google.gerrit.extensions.client.GeneralPreferencesInfo.EmailStrategy.CC_ON_OWN_COMMENTS;
import static com.google.gerrit.extensions.client.GeneralPreferencesInfo.EmailStrategy.DISABLED; import static com.google.gerrit.extensions.client.GeneralPreferencesInfo.EmailStrategy.DISABLED;
import static java.nio.charset.StandardCharsets.UTF_8; import static java.nio.charset.StandardCharsets.UTF_8;
import com.google.common.collect.ImmutableListMultimap;
import com.google.common.collect.Multimap;
import com.google.gerrit.common.errors.EmailException; import com.google.gerrit.common.errors.EmailException;
import com.google.gerrit.extensions.api.changes.NotifyHandling; 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;
import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.UserIdentity; import com.google.gerrit.reviewdb.client.UserIdentity;
import com.google.gerrit.server.account.AccountState; import com.google.gerrit.server.account.AccountState;
import com.google.gerrit.server.mail.Address; import com.google.gerrit.server.mail.Address;
import com.google.gerrit.server.mail.RecipientType;
import com.google.gerrit.server.mail.send.EmailHeader.AddressList; import com.google.gerrit.server.mail.send.EmailHeader.AddressList;
import com.google.gerrit.server.validators.OutgoingEmailValidationListener; import com.google.gerrit.server.validators.OutgoingEmailValidationListener;
import com.google.gerrit.server.validators.ValidationException; import com.google.gerrit.server.validators.ValidationException;
@ -73,6 +76,8 @@ public abstract class OutgoingEmail {
private Address smtpFromAddress; private Address smtpFromAddress;
private StringBuilder textBody; private StringBuilder textBody;
private StringBuilder htmlBody; private StringBuilder htmlBody;
private Multimap<RecipientType, Account.Id> accountsToNotify =
ImmutableListMultimap.of();
protected VelocityContext velocityContext; protected VelocityContext velocityContext;
protected Map<String, Object> soyContext; protected Map<String, Object> soyContext;
protected Map<String, Object> soyContextEmailData; protected Map<String, Object> soyContextEmailData;
@ -92,7 +97,12 @@ public abstract class OutgoingEmail {
} }
public void setNotify(NotifyHandling notify) { public void setNotify(NotifyHandling notify) {
this.notify = notify; this.notify = checkNotNull(notify);
}
public void setAccountsToNotify(
Multimap<RecipientType, Account.Id> accountsToNotify) {
this.accountsToNotify = checkNotNull(accountsToNotify);
} }
/** /**
@ -101,7 +111,7 @@ public abstract class OutgoingEmail {
* @throws EmailException * @throws EmailException
*/ */
public void send() throws EmailException { public void send() throws EmailException {
if (NotifyHandling.NONE.equals(notify)) { if (NotifyHandling.NONE.equals(notify) && accountsToNotify.isEmpty()) {
return; return;
} }
@ -132,7 +142,8 @@ public abstract class OutgoingEmail {
// on their behalf to others. // on their behalf to others.
// //
add(RecipientType.CC, fromId); add(RecipientType.CC, fromId);
} else if (rcptTo.remove(fromId)) { } else if (!accountsToNotify.containsValue(fromId)
&& rcptTo.remove(fromId)) {
// If they don't want a copy, but we queued one up anyway, // If they don't want a copy, but we queued one up anyway,
// drop them from the recipient lists. // drop them from the recipient lists.
// //
@ -199,6 +210,10 @@ public abstract class OutgoingEmail {
headers.put(HDR_CC, new EmailHeader.AddressList()); headers.put(HDR_CC, new EmailHeader.AddressList());
setHeader("Message-ID", ""); setHeader("Message-ID", "");
for (RecipientType recipientType : accountsToNotify.keySet()) {
add(recipientType, accountsToNotify.get(recipientType));
}
if (fromId != null) { if (fromId != null) {
// If we have a user that this message is supposedly caused by // If we have a user that this message is supposedly caused by
// but the From header on the email does not match the user as // but the From header on the email does not match the user as
@ -381,7 +396,9 @@ public abstract class OutgoingEmail {
return false; return false;
} }
if (smtpRcptTo.size() == 1 && rcptTo.size() == 1 && rcptTo.contains(fromId)) { if ((accountsToNotify == null || accountsToNotify.isEmpty())
&& smtpRcptTo.size() == 1 && rcptTo.size() == 1
&& rcptTo.contains(fromId)) {
// If the only recipient is also the sender, don't bother. // If the only recipient is also the sender, don't bother.
// //
return false; return false;

View File

@ -17,10 +17,10 @@ package com.google.gerrit.server.mail.send;
import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.base.Preconditions.checkNotNull;
import com.google.gerrit.common.errors.EmailException; import com.google.gerrit.common.errors.EmailException;
import com.google.gerrit.extensions.api.changes.RecipientType;
import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.mail.Address; import com.google.gerrit.server.mail.Address;
import com.google.gerrit.server.mail.EmailTokenVerifier; import com.google.gerrit.server.mail.EmailTokenVerifier;
import com.google.gerrit.server.mail.RecipientType;
import com.google.inject.Inject; import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted; import com.google.inject.assistedinject.Assisted;

View File

@ -15,11 +15,11 @@
package com.google.gerrit.server.mail.send; package com.google.gerrit.server.mail.send;
import com.google.gerrit.common.errors.EmailException; import com.google.gerrit.common.errors.EmailException;
import com.google.gerrit.extensions.api.changes.RecipientType;
import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountProjectWatch.NotifyType; import com.google.gerrit.reviewdb.client.AccountProjectWatch.NotifyType;
import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.mail.RecipientType;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject; import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted; import com.google.inject.assistedinject.Assisted;

View File

@ -15,9 +15,9 @@
package com.google.gerrit.server.mail.send; package com.google.gerrit.server.mail.send;
import com.google.gerrit.common.errors.EmailException; import com.google.gerrit.common.errors.EmailException;
import com.google.gerrit.extensions.api.changes.RecipientType;
import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.mail.RecipientType;
import com.google.gerrit.server.query.change.ChangeData; import com.google.gerrit.server.query.change.ChangeData;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;