Don't add reviewers to merged changes
If a change goes directly from WIP to MERGED due to the push of a
WIP change (e.g.)
1. Upload a change with %wip.
2. Push the commit, thus closing the change automatically.
Reviewers are added and put in Attention-Set, because the change goes
public as soon as it is merged. Now they have notifications to act on a
change that is merged.
To avoid this, simply don't automatically add reviewers to merged
changes.
Change-Id: I8eb03a0507624a4e09f837e59eb764e8e6548c69
diff --git a/src/main/java/com/googlesource/gerrit/plugins/reviewers/Reviewers.java b/src/main/java/com/googlesource/gerrit/plugins/reviewers/Reviewers.java
index 188cb77..bdbee0e 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/reviewers/Reviewers.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/reviewers/Reviewers.java
@@ -14,6 +14,8 @@
package com.googlesource.gerrit.plugins.reviewers;
+import static com.google.gerrit.extensions.client.ChangeStatus.MERGED;
+
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.Project;
@@ -96,6 +98,9 @@
if (config.ignoreWip() && Boolean.TRUE.equals(c.workInProgress)) {
return;
}
+ if (c.status.equals(MERGED)) {
+ return;
+ }
Project.NameKey projectName = Project.nameKey(c.project);
List<ReviewerFilter> filters = getFilters(projectName);
diff --git a/src/test/java/com/googlesource/gerrit/plugins/reviewers/ReviewersIT.java b/src/test/java/com/googlesource/gerrit/plugins/reviewers/ReviewersIT.java
index bedd4de..6e4df58 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/reviewers/ReviewersIT.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/reviewers/ReviewersIT.java
@@ -104,7 +104,7 @@
}
@Test
- public void privateBitFlippedAndReviewersAddedOnSubmit() throws Exception {
+ public void reviewersNotAddedOnSubmitOfPrivateChange() throws Exception {
createFilters(filter("*").reviewer(user));
PushOneCommit.Result r = createChange("refs/for/master%private");
assertThat(r.getChange().change().isPrivate()).isTrue();
@@ -113,14 +113,29 @@
// This adds admin as reviewer
gApi.changes().id(changeId).current().review(ReviewInput.approve());
gApi.changes().id(changeId).current().submit();
- assertThat(reviewersFor(changeId))
- .containsExactlyElementsIn(ImmutableSet.of(admin.id(), user.id()));
+ assertThat(reviewersFor(changeId)).containsExactlyElementsIn(ImmutableSet.of(admin.id()));
ChangeInfo info = gApi.changes().id(changeId).get();
assertThat(info.status).isEqualTo(ChangeStatus.MERGED);
assertThat(info.isPrivate).isNull();
}
@Test
+ public void reviewersNotAddedOnPushOfWIPChange() throws Exception {
+ createFilters(filter("*").reviewer(user));
+ PushOneCommit.Result r = createChange("refs/for/master%wip");
+ assertThat(r.getChange().change().isWorkInProgress()).isTrue();
+ String changeId = r.getChangeId();
+ assertNoReviewersAddedFor(changeId);
+ // This adds admin as reviewer
+ gApi.changes().id(changeId).current().review(ReviewInput.approve());
+ pushToHeads("refs/heads/master", changeId);
+ assertThat(reviewersFor(changeId)).containsExactlyElementsIn(ImmutableSet.of(admin.id()));
+ ChangeInfo info = gApi.changes().id(changeId).get();
+ assertThat(info.status).isEqualTo(ChangeStatus.MERGED);
+ assertThat(info.workInProgress).isNull();
+ }
+
+ @Test
public void reviewerAddedOnPrivateBitFlip() throws Exception {
createFilters(filter("*").reviewer(user));
PushOneCommit.Result r = createChange("refs/for/master%private");
@@ -203,4 +218,9 @@
assertThat(gApi.changes().id(changeId).get().reviewers.get(REVIEWER)).isNull();
assertThat(gApi.changes().id(changeId).get().reviewers.get(CC)).isNull();
}
+
+ private PushOneCommit.Result pushToHeads(String ref, String changeId) throws Exception {
+ PushOneCommit push = pushFactory.create(admin.newIdent(), testRepo, changeId);
+ return push.to(ref);
+ }
}
