New project setting - whether to add commit author+commiter as reviewers
The setting will apply to any new change in the project.
Motivation: some projects have a lot of cherry-picks from other Git
repositories. In case the original change's author or commiter are
Gerrit users, they are automatically added as reviewers, even if they
do not work on the same project; which causes a lot of noise to everyone
involved.
Change-Id: I6d0fff4c6ab14e7dd8c271e54a9a6037c0fc479e
Release-Notes: skip
Forward-Compatible: checked
Bug: Google b/26172409
diff --git a/Documentation/config-project-config.txt b/Documentation/config-project-config.txt
index 7a0e305..31008f6 100644
--- a/Documentation/config-project-config.txt
+++ b/Documentation/config-project-config.txt
@@ -736,6 +736,15 @@
the parent project. If the property is not set in any parent project, the
default value is `FALSE`.
+[[reviewer.skipAddingAuthorAndCommitterAsReviewers]]reviewer.skipAddingAuthorAndCommitterAsReviewers::
++
+Whether to skip adding the Git commit author and committer as reviewers for
+a new change.
++
+Default is `INHERIT`, which means that this property is inherited from
+the parent project. If the property is not set in any parent project, the
+default value is `FALSE`.
+
[[file-groups]]
== The file +groups+
diff --git a/Documentation/rest-api-projects.txt b/Documentation/rest-api-projects.txt
index 94db15e..9e71df7 100644
--- a/Documentation/rest-api-projects.txt
+++ b/Documentation/rest-api-projects.txt
@@ -3896,17 +3896,19 @@
Map with the comment link configurations of the project. The name of
the comment link configuration is mapped to a link:#commentlink-info[
CommentlinkInfo] entity.
-|`plugin_config` |optional|
+|`plugin_config` |optional|
Plugin configuration as map which maps the plugin name to a map of
parameter names to link:#config-parameter-info[ConfigParameterInfo]
entities. Only filled for users who have read access to `refs/meta/config`.
-|`actions` |optional|
+|`actions` |optional|
Actions the caller might be able to perform on this project. The
information is a map of view names to
-|`reject_empty_commit` |optional|
+|`reject_empty_commit` |optional|
link:#inherited-boolean-info[InheritedBooleanInfo] that tells whether
empty commits should be rejected when a change is merged.
link:rest-api-changes.html#action-info[ActionInfo] entities.
+|`skip_adding_author_and_committer_as_reviewers` |optional|
+Whether to skip adding the Git commit author and committer as reviewers for a new change.
|=======================================================
[[config-input]]
diff --git a/java/com/google/gerrit/acceptance/AbstractDaemonTest.java b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
index 3e48eec..5b4a9e5 100644
--- a/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
+++ b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
@@ -1080,6 +1080,19 @@
}
}
+ protected void setSkipAddingAuthorAndCommitterAsReviewers(InheritableBoolean value)
+ throws Exception {
+ try (MetaDataUpdate md = metaDataUpdateFactory.create(project)) {
+ ProjectConfig config = projectConfigFactory.read(md);
+ config.updateProject(
+ p ->
+ p.setBooleanConfig(
+ BooleanProjectConfig.SKIP_ADDING_AUTHOR_AND_COMMITTER_AS_REVIEWERS, value));
+ config.commit(md);
+ projectCache.evictAndReindex(config.getProject());
+ }
+ }
+
protected void blockAnonymousRead() throws Exception {
String allRefs = RefNames.REFS + "*";
projectOperations
diff --git a/java/com/google/gerrit/entities/BooleanProjectConfig.java b/java/com/google/gerrit/entities/BooleanProjectConfig.java
index 5201f6d..605c40c 100644
--- a/java/com/google/gerrit/entities/BooleanProjectConfig.java
+++ b/java/com/google/gerrit/entities/BooleanProjectConfig.java
@@ -41,7 +41,9 @@
ENABLE_REVIEWER_BY_EMAIL("reviewer", "enableByEmail"),
MATCH_AUTHOR_TO_COMMITTER_DATE("submit", "matchAuthorToCommitterDate"),
REJECT_EMPTY_COMMIT("submit", "rejectEmptyCommit"),
- WORK_IN_PROGRESS_BY_DEFAULT("change", "workInProgressByDefault");
+ WORK_IN_PROGRESS_BY_DEFAULT("change", "workInProgressByDefault"),
+ SKIP_ADDING_AUTHOR_AND_COMMITTER_AS_REVIEWERS(
+ "reviewer", "skipAddingAuthorAndCommitterAsReviewers");
// Git config
private final String section;
diff --git a/java/com/google/gerrit/extensions/api/projects/ConfigInfo.java b/java/com/google/gerrit/extensions/api/projects/ConfigInfo.java
index 3ba1277..1a51c15 100644
--- a/java/com/google/gerrit/extensions/api/projects/ConfigInfo.java
+++ b/java/com/google/gerrit/extensions/api/projects/ConfigInfo.java
@@ -40,6 +40,7 @@
public InheritedBooleanInfo enableReviewerByEmail;
public InheritedBooleanInfo matchAuthorToCommitterDate;
public InheritedBooleanInfo rejectEmptyCommit;
+ public InheritedBooleanInfo skipAddingAuthorAndCommitterAsReviewers;
public MaxObjectSizeLimitInfo maxObjectSizeLimit;
@Deprecated // Equivalent to defaultSubmitType.value
diff --git a/java/com/google/gerrit/extensions/api/projects/ConfigInput.java b/java/com/google/gerrit/extensions/api/projects/ConfigInput.java
index 8005fc5..906fc4c 100644
--- a/java/com/google/gerrit/extensions/api/projects/ConfigInput.java
+++ b/java/com/google/gerrit/extensions/api/projects/ConfigInput.java
@@ -34,6 +34,7 @@
public InheritableBoolean enableReviewerByEmail;
public InheritableBoolean matchAuthorToCommitterDate;
public InheritableBoolean rejectEmptyCommit;
+ public InheritableBoolean skipAddingAuthorAndCommitterAsReviewers;
public String maxObjectSizeLimit;
public SubmitType submitType;
public ProjectState state;
diff --git a/java/com/google/gerrit/server/change/ChangeInserter.java b/java/com/google/gerrit/server/change/ChangeInserter.java
index d575324..2883ef8 100644
--- a/java/com/google/gerrit/server/change/ChangeInserter.java
+++ b/java/com/google/gerrit/server/change/ChangeInserter.java
@@ -32,6 +32,7 @@
import com.google.errorprone.annotations.CanIgnoreReturnValue;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.Account;
+import com.google.gerrit.entities.BooleanProjectConfig;
import com.google.gerrit.entities.BranchNameKey;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.LabelType;
@@ -653,6 +654,9 @@
}
private ImmutableList<InternalReviewerInput> getReviewerInputs() {
+ if (projectState.is(BooleanProjectConfig.SKIP_ADDING_AUTHOR_AND_COMMITTER_AS_REVIEWERS)) {
+ return reviewerInputs;
+ }
return Streams.concat(
reviewerInputs.stream(),
Streams.stream(
diff --git a/java/com/google/gerrit/server/project/BooleanProjectConfigTransformations.java b/java/com/google/gerrit/server/project/BooleanProjectConfigTransformations.java
index ae9828a..9ccbf90 100644
--- a/java/com/google/gerrit/server/project/BooleanProjectConfigTransformations.java
+++ b/java/com/google/gerrit/server/project/BooleanProjectConfigTransformations.java
@@ -71,6 +71,11 @@
.put(
BooleanProjectConfig.WORK_IN_PROGRESS_BY_DEFAULT,
new Mapper(i -> i.workInProgressByDefault, (i, v) -> i.workInProgressByDefault = v))
+ .put(
+ BooleanProjectConfig.SKIP_ADDING_AUTHOR_AND_COMMITTER_AS_REVIEWERS,
+ new Mapper(
+ i -> i.skipAddingAuthorAndCommitterAsReviewers,
+ (i, v) -> i.skipAddingAuthorAndCommitterAsReviewers = v))
.build();
static {
diff --git a/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java b/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java
index 08f65da..50bb8ae 100644
--- a/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java
+++ b/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java
@@ -1169,6 +1169,27 @@
}
@Test
+ public void pushForMasterWithForgedAuthorAndCommitter_skipAddingAuthorAndCommitterAsReviewers()
+ throws Exception {
+ setSkipAddingAuthorAndCommitterAsReviewers(InheritableBoolean.TRUE);
+ TestAccount user2 = accountCreator.user2();
+ // Create a commit with different forged author and committer.
+ RevCommit c =
+ commitBuilder()
+ .author(user.newIdent())
+ .committer(user2.newIdent())
+ .add(PushOneCommit.FILE_NAME, PushOneCommit.FILE_CONTENT)
+ .message(PushOneCommit.SUBJECT)
+ .create();
+ // Push commit as "Administrator".
+ pushHead(testRepo, "refs/for/master");
+
+ String changeId = GitUtil.getChangeId(testRepo, c).get();
+ assertThat(getOwnerEmail(changeId)).isEqualTo(admin.email());
+ assertThat(getReviewerEmails(changeId, ReviewerState.CC)).isEmpty();
+ }
+
+ @Test
public void pushForMasterWithNonExistingForgedAuthorAndCommitter() throws Exception {
// Create a commit with different forged author and committer.
RevCommit c =
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java b/javatests/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java
index 67c784b..079f84e 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java
@@ -57,8 +57,10 @@
import com.google.gerrit.extensions.api.changes.ReviewInput;
import com.google.gerrit.extensions.api.changes.RevisionApi;
import com.google.gerrit.extensions.api.projects.BranchInput;
+import com.google.gerrit.extensions.api.projects.ConfigInput;
import com.google.gerrit.extensions.client.ChangeStatus;
import com.google.gerrit.extensions.client.GeneralPreferencesInfo;
+import com.google.gerrit.extensions.client.InheritableBoolean;
import com.google.gerrit.extensions.client.ReviewerState;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.ChangeInput;
@@ -494,6 +496,20 @@
}
@Test
+ public void createAuthorNotAddedAsCcWithAvoidAddingOriginalAuthorAsReviewer() throws Exception {
+ ConfigInput config = new ConfigInput();
+ config.skipAddingAuthorAndCommitterAsReviewers = InheritableBoolean.TRUE;
+ gApi.projects().name(project.get()).config(config);
+ ChangeInput input = newChangeInput(ChangeStatus.NEW);
+ input.author = new AccountInput();
+ input.author.email = user.email();
+ input.author.name = user.fullName();
+
+ ChangeInfo info = assertCreateSucceeds(input);
+ assertThat(info.reviewers).isEmpty();
+ }
+
+ @Test
public void createNewWorkInProgressChange() throws Exception {
ChangeInput input = newChangeInput(ChangeStatus.NEW);
input.workInProgress = true;
diff --git a/javatests/com/google/gerrit/server/cache/serialize/entities/ProjectSerializerTest.java b/javatests/com/google/gerrit/server/cache/serialize/entities/ProjectSerializerTest.java
index 29fd5ed..1f725f8 100644
--- a/javatests/com/google/gerrit/server/cache/serialize/entities/ProjectSerializerTest.java
+++ b/javatests/com/google/gerrit/server/cache/serialize/entities/ProjectSerializerTest.java
@@ -40,6 +40,9 @@
.setBooleanConfig(
BooleanProjectConfig.CREATE_NEW_CHANGE_FOR_ALL_NOT_IN_TARGET,
InheritableBoolean.INHERIT)
+ .setBooleanConfig(
+ BooleanProjectConfig.SKIP_ADDING_AUTHOR_AND_COMMITTER_AS_REVIEWERS,
+ InheritableBoolean.TRUE)
.build();
@Test
