Limit abandon notification of fully WIP changes
Previously, the default notify level for abandons was ALL. With this change, it defaults instead to OWNER if review has not started on the change. The caller can still override with a specific notify level if interested. Change-Id: Ie747ec2028fba0cb52dbf342d3df052109a842c7
This commit is contained in:
@@ -1091,6 +1091,19 @@ the error message is contained in the response body.
|
||||
change is merged
|
||||
----
|
||||
|
||||
.Notifications
|
||||
|
||||
An email will be sent using the "abandon" template. The notify handling is ALL.
|
||||
Notifications are suppressed on WIP changes that have never started review.
|
||||
|
||||
[options="header",cols="1,1"]
|
||||
|=============================
|
||||
|WIP State|notify=ALL
|
||||
|Ready for review|owner, reviewers, CCs, stars, ABANDONED_CHANGES watchers
|
||||
|Work in progress|not sent
|
||||
|Reviewable WIP |owner, reviewers, CCs, stars, ABANDONED_CHANGES watchers
|
||||
|=============================
|
||||
|
||||
[[restore-change]]
|
||||
=== Restore Change
|
||||
--
|
||||
|
@@ -14,7 +14,11 @@
|
||||
|
||||
package com.google.gerrit.acceptance.server.mail;
|
||||
|
||||
import static com.google.gerrit.extensions.api.changes.NotifyHandling.*;
|
||||
import static com.google.common.truth.TruthJUnit.assume;
|
||||
import static com.google.gerrit.extensions.api.changes.NotifyHandling.ALL;
|
||||
import static com.google.gerrit.extensions.api.changes.NotifyHandling.NONE;
|
||||
import static com.google.gerrit.extensions.api.changes.NotifyHandling.OWNER;
|
||||
import static com.google.gerrit.extensions.api.changes.NotifyHandling.OWNER_REVIEWERS;
|
||||
import static com.google.gerrit.extensions.client.GeneralPreferencesInfo.EmailStrategy.CC_ON_OWN_COMMENTS;
|
||||
import static com.google.gerrit.server.account.WatchConfig.NotifyType.ABANDONED_CHANGES;
|
||||
import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
|
||||
@@ -170,15 +174,7 @@ public class AbandonedSenderIT extends AbstractNotificationTest {
|
||||
public void abandonWipChange() throws Exception {
|
||||
StagedChange sc = stageWipChange(ABANDONED_CHANGES);
|
||||
abandon(sc.changeId, sc.owner);
|
||||
// TODO(logan): This should behave like notify=OWNER in the future.
|
||||
assertThat(sender)
|
||||
.sent("abandon", sc)
|
||||
.notTo(sc.owner)
|
||||
.cc(sc.reviewer, sc.ccer)
|
||||
.to(sc.reviewerByEmail)
|
||||
.cc(sc.ccerByEmail)
|
||||
.bcc(sc.starrer)
|
||||
.bcc(ABANDONED_CHANGES);
|
||||
assertThat(sender).notSent();
|
||||
}
|
||||
|
||||
@Test
|
||||
@@ -189,7 +185,7 @@ public class AbandonedSenderIT extends AbstractNotificationTest {
|
||||
.sent("abandon", sc)
|
||||
.notTo(sc.owner)
|
||||
.cc(sc.reviewer, sc.ccer)
|
||||
.to(sc.reviewerByEmail)
|
||||
.to(sc.reviewerByEmail) // TODO(logan): This is unintentionally TO, should be CC.
|
||||
.cc(sc.ccerByEmail)
|
||||
.bcc(sc.starrer)
|
||||
.bcc(ABANDONED_CHANGES);
|
||||
|
@@ -19,6 +19,6 @@ import java.util.Map;
|
||||
|
||||
public class AbandonInput {
|
||||
@DefaultInput public String message;
|
||||
public NotifyHandling notify = NotifyHandling.ALL;
|
||||
public NotifyHandling notify;
|
||||
public Map<RecipientType, NotifyInfo> notifyDetails;
|
||||
}
|
||||
|
@@ -71,24 +71,30 @@ public class Abandon extends RetryingRestModifyView<ChangeResource, AbandonInput
|
||||
throws RestApiException, UpdateException, OrmException, PermissionBackendException {
|
||||
req.permissions().database(dbProvider).check(ChangePermission.ABANDON);
|
||||
|
||||
NotifyHandling notify = input.notify == null ? defaultNotify(req.getControl()) : input.notify;
|
||||
Change change =
|
||||
abandon(
|
||||
updateFactory,
|
||||
req.getControl(),
|
||||
input.message,
|
||||
input.notify,
|
||||
notify,
|
||||
notifyUtil.resolveAccounts(input.notifyDetails));
|
||||
return json.noOptions().format(change);
|
||||
}
|
||||
|
||||
private NotifyHandling defaultNotify(ChangeControl control) {
|
||||
return control.getChange().hasReviewStarted() ? NotifyHandling.ALL : NotifyHandling.OWNER;
|
||||
}
|
||||
|
||||
public Change abandon(BatchUpdate.Factory updateFactory, ChangeControl control)
|
||||
throws RestApiException, UpdateException {
|
||||
return abandon(updateFactory, control, "", NotifyHandling.ALL, ImmutableListMultimap.of());
|
||||
return abandon(updateFactory, control, "", defaultNotify(control), ImmutableListMultimap.of());
|
||||
}
|
||||
|
||||
public Change abandon(BatchUpdate.Factory updateFactory, ChangeControl control, String msgTxt)
|
||||
throws RestApiException, UpdateException {
|
||||
return abandon(updateFactory, control, msgTxt, NotifyHandling.ALL, ImmutableListMultimap.of());
|
||||
return abandon(
|
||||
updateFactory, control, msgTxt, defaultNotify(control), ImmutableListMultimap.of());
|
||||
}
|
||||
|
||||
public Change abandon(
|
||||
|
Reference in New Issue
Block a user