opendev/gerrit
Code Issues Proposed changes

Merge "Downgrade default notify for bots on all comments"

Browse Source
This commit is contained in:
Dave Borowitz
2017-06-23 13:20:47 +00:00
committed by Gerrit Code Review
parent 4fadd2cc99 2f08e0776e
commit 44e46f85de
3 changed files with 58 additions and 30 deletions
Download Patch File Download Diff File Expand all files Collapse all files

17
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/ChangeNotificationsIT.java
View File

@@ -739,6 +739,19 @@ public class ChangeNotificationsIT extends AbstractNotificationTest {
assertThat(sender).notSent(); // TODO(logan): Why not send to owner? assertThat(sender).notSent(); // TODO(logan): Why not send to owner?
} }
@Test
public void commentOnReviewableChangeByBot() throws Exception {
StagedChange sc = stageReviewableChange();
TestAccount bot = sc.testAccount("bot");
review(bot, sc.changeId, ENABLED, null, "autogenerated:bot");
assertThat(sender)
.sent("comment", sc)
.to(sc.owner)
.cc(sc.reviewer, sc.ccer)
.cc(sc.reviewerByEmail, sc.ccerByEmail)
.noOneElse();
}
@Test @Test
public void commentOnWipChangeByOwner() throws Exception { public void commentOnWipChangeByOwner() throws Exception {
StagedChange sc = stageWipChange(); StagedChange sc = stageWipChange();
@@ -770,7 +783,7 @@ public class ChangeNotificationsIT extends AbstractNotificationTest {
public void commentOnWipChangeByBot() throws Exception { public void commentOnWipChangeByBot() throws Exception {
StagedChange sc = stageWipChange(); StagedChange sc = stageWipChange();
TestAccount bot = sc.testAccount("bot"); TestAccount bot = sc.testAccount("bot");
review(bot, sc.changeId, ENABLED, null, "tag"); review(bot, sc.changeId, ENABLED, null, "autogenerated:tag");
assertThat(sender).sent("comment", sc).to(sc.owner).noOneElse(); assertThat(sender).sent("comment", sc).to(sc.owner).noOneElse();
} }
@@ -778,7 +791,7 @@ public class ChangeNotificationsIT extends AbstractNotificationTest {
public void commentOnReviewableWipChangeByBot() throws Exception { public void commentOnReviewableWipChangeByBot() throws Exception {
StagedChange sc = stageReviewableWipChange(); StagedChange sc = stageReviewableWipChange();
TestAccount bot = sc.testAccount("bot"); TestAccount bot = sc.testAccount("bot");
review(bot, sc.changeId, ENABLED, null, "tag"); review(bot, sc.changeId, ENABLED, null, "autogenerated:tag");
assertThat(sender).sent("comment", sc).to(sc.owner).noOneElse(); assertThat(sender).sent("comment", sc).to(sc.owner).noOneElse();
} }

54
gerrit-server/src/main/java/com/google/gerrit/server/ChangeMessagesUtil.java
View File

@@ -43,25 +43,33 @@ import java.util.Objects;
*/ */
@Singleton @Singleton
public class ChangeMessagesUtil { public class ChangeMessagesUtil {
public static final String TAG_ABANDON = "autogenerated:gerrit:abandon"; public static final String AUTOGENERATED_TAG_PREFIX = "autogenerated:";
public static final String TAG_CHERRY_PICK_CHANGE = "autogenerated:gerrit:cherryPickChange";
public static final String TAG_DELETE_ASSIGNEE = "autogenerated:gerrit:deleteAssignee"; public static final String TAG_ABANDON = AUTOGENERATED_TAG_PREFIX + "gerrit:abandon";
public static final String TAG_DELETE_REVIEWER = "autogenerated:gerrit:deleteReviewer"; public static final String TAG_CHERRY_PICK_CHANGE =
public static final String TAG_DELETE_VOTE = "autogenerated:gerrit:deleteVote"; AUTOGENERATED_TAG_PREFIX + "gerrit:cherryPickChange";
public static final String TAG_MERGED = "autogenerated:gerrit:merged"; public static final String TAG_DELETE_ASSIGNEE =
public static final String TAG_MOVE = "autogenerated:gerrit:move"; AUTOGENERATED_TAG_PREFIX + "gerrit:deleteAssignee";
public static final String TAG_RESTORE = "autogenerated:gerrit:restore"; public static final String TAG_DELETE_REVIEWER =
public static final String TAG_REVERT = "autogenerated:gerrit:revert"; AUTOGENERATED_TAG_PREFIX + "gerrit:deleteReviewer";
public static final String TAG_SET_ASSIGNEE = "autogenerated:gerrit:setAssignee"; public static final String TAG_DELETE_VOTE = AUTOGENERATED_TAG_PREFIX + "gerrit:deleteVote";
public static final String TAG_SET_DESCRIPTION = "autogenerated:gerrit:setPsDescription"; public static final String TAG_MERGED = AUTOGENERATED_TAG_PREFIX + "gerrit:merged";
public static final String TAG_SET_HASHTAGS = "autogenerated:gerrit:setHashtag"; public static final String TAG_MOVE = AUTOGENERATED_TAG_PREFIX + "gerrit:move";
public static final String TAG_SET_PRIVATE = "autogenerated:gerrit:setPrivate"; public static final String TAG_RESTORE = AUTOGENERATED_TAG_PREFIX + "gerrit:restore";
public static final String TAG_SET_READY = "autogenerated:gerrit:setReadyForReview"; public static final String TAG_REVERT = AUTOGENERATED_TAG_PREFIX + "gerrit:revert";
public static final String TAG_SET_TOPIC = "autogenerated:gerrit:setTopic"; public static final String TAG_SET_ASSIGNEE = AUTOGENERATED_TAG_PREFIX + "gerrit:setAssignee";
public static final String TAG_SET_WIP = "autogenerated:gerrit:setWorkInProgress"; public static final String TAG_SET_DESCRIPTION =
public static final String TAG_UNSET_PRIVATE = "autogenerated:gerrit:unsetPrivate"; AUTOGENERATED_TAG_PREFIX + "gerrit:setPsDescription";
public static final String TAG_UPLOADED_PATCH_SET = "autogenerated:gerrit:newPatchSet"; public static final String TAG_SET_HASHTAGS = AUTOGENERATED_TAG_PREFIX + "gerrit:setHashtag";
public static final String TAG_UPLOADED_WIP_PATCH_SET = "autogenerated:gerrit:newWipPatchSet"; public static final String TAG_SET_PRIVATE = AUTOGENERATED_TAG_PREFIX + "gerrit:setPrivate";
public static final String TAG_SET_READY = AUTOGENERATED_TAG_PREFIX + "gerrit:setReadyForReview";
public static final String TAG_SET_TOPIC = AUTOGENERATED_TAG_PREFIX + "gerrit:setTopic";
public static final String TAG_SET_WIP = AUTOGENERATED_TAG_PREFIX + "gerrit:setWorkInProgress";
public static final String TAG_UNSET_PRIVATE = AUTOGENERATED_TAG_PREFIX + "gerrit:unsetPrivate";
public static final String TAG_UPLOADED_PATCH_SET =
AUTOGENERATED_TAG_PREFIX + "gerrit:newPatchSet";
public static final String TAG_UPLOADED_WIP_PATCH_SET =
AUTOGENERATED_TAG_PREFIX + "gerrit:newWipPatchSet";
public static ChangeMessage newMessage(ChangeContext ctx, String body, @Nullable String tag) { public static ChangeMessage newMessage(ChangeContext ctx, String body, @Nullable String tag) {
return newMessage(ctx.getChange().currentPatchSetId(), ctx.getUser(), ctx.getWhen(), body, tag); return newMessage(ctx.getChange().currentPatchSetId(), ctx.getUser(), ctx.getWhen(), body, tag);
@@ -125,4 +133,12 @@ public class ChangeMessagesUtil {
update.setTag(changeMessage.getTag()); update.setTag(changeMessage.getTag());
db.changeMessages().insert(Collections.singleton(changeMessage)); db.changeMessages().insert(Collections.singleton(changeMessage));
} }
/**
* @param tag value of a tag, or null.
* @return whether the tag starts with the autogenerated prefix.
*/
public static boolean isAutogenerated(@Nullable String tag) {
return tag != null && tag.startsWith(AUTOGENERATED_TAG_PREFIX);
}
} }

17
gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java
View File

@@ -325,18 +325,17 @@ public class PostReview
} }
private NotifyHandling defaultNotify(Change c, ReviewInput in) { private NotifyHandling defaultNotify(Change c, ReviewInput in) {
if (!c.isWorkInProgress()) { if (ChangeMessagesUtil.isAutogenerated(in.tag)) {
return NotifyHandling.ALL; // Autogenerated comments default to lower notify levels.
return c.isWorkInProgress() ? NotifyHandling.OWNER : NotifyHandling.OWNER_REVIEWERS;
} }
if (in.tag != null) {
// Treat the presence of tag as indicator of a "bot". if (c.isWorkInProgress() && !c.hasReviewStarted()) {
return NotifyHandling.OWNER; // If review hasn't started we want to minimize recipients, no matter who
} // the author is.
if (!c.hasReviewStarted()) {
// If review hasn't started we want to minimize recipients, even on
// "human" comments.
return NotifyHandling.OWNER; return NotifyHandling.OWNER;
} }
return NotifyHandling.ALL; return NotifyHandling.ALL;
} }