OnCodeOwnerOverride: Detect if override is ignored due to self approval
If an override is ignored due to self approval (because the override
label is configured to ignore self approvals and the override approval
is done by the patch set uploader) say so in the change message so users
can know about it.
Signed-off-by: Edwin Kempin <ekempin@google.com>
Change-Id: Iee7c93729ca96daf8ff7c690cbca6dbaad522969
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/OnCodeOwnerOverride.java b/java/com/google/gerrit/plugins/codeowners/backend/OnCodeOwnerOverride.java
index df24fbb..c200ae0 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/OnCodeOwnerOverride.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/OnCodeOwnerOverride.java
@@ -98,6 +98,18 @@
RequiredApproval overrideApproval) {
LabelVote newVote = getNewVote(overrideApproval, approvals);
+ if (isIgnoredDueToSelfApproval(user, patchSet, overrideApproval)) {
+ if (isCodeOwnerOverrideNewlyApplied(overrideApproval, oldApprovals, newVote)
+ || isCodeOwnerOverrideUpOrDowngraded(overrideApproval, oldApprovals, newVote)) {
+ return Optional.of(
+ String.format(
+ "The vote %s is ignored as code-owner override since the label doesn't allow"
+ + " self approval of the patch set uploader.",
+ newVote));
+ }
+ return Optional.empty();
+ }
+
if (isCodeOwnerOverrideNewlyApplied(overrideApproval, oldApprovals, newVote)) {
return Optional.of(
String.format(
@@ -134,6 +146,12 @@
return Optional.empty();
}
+ private boolean isIgnoredDueToSelfApproval(
+ IdentifiedUser user, PatchSet patchSet, RequiredApproval requiredApproval) {
+ return patchSet.uploader().equals(user.getAccountId())
+ && requiredApproval.labelType().isIgnoreSelfApproval();
+ }
+
private boolean isCodeOwnerOverrideNewlyApplied(
RequiredApproval requiredApproval, Map<String, Short> oldApprovals, LabelVote newVote) {
String labelName = requiredApproval.labelType().getName();
diff --git a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/OnCodeOwnerOverrrideIT.java b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/OnCodeOwnerOverrrideIT.java
index f094f7b..1628c03 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/OnCodeOwnerOverrrideIT.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/OnCodeOwnerOverrrideIT.java
@@ -434,4 +434,174 @@
+ "* %s\n",
admin.fullName(), path));
}
+
+ @Test
+ @GerritConfig(name = "plugin.code-owners.overrideApproval", value = "Owners-Override+1")
+ public void changeMessageExtendedIfCodeOwnerOverrideIsIgnoredDueToSelfApproval()
+ throws Exception {
+ LabelDefinitionInput input = new LabelDefinitionInput();
+ input.values = ImmutableMap.of("+1", "Override", " 0", "No Override");
+ input.ignoreSelfApproval = true;
+ gApi.projects().name(project.get()).label("Owners-Override").create(input);
+
+ // Allow to vote on the Owners-Override label.
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(
+ TestProjectUpdate.allowLabel("Owners-Override")
+ .range(0, 1)
+ .ref("refs/*")
+ .group(REGISTERED_USERS)
+ .build())
+ .update();
+
+ String changeId = createChange().getChangeId();
+
+ gApi.changes().id(changeId).current().review(new ReviewInput().label("Owners-Override", 1));
+
+ Collection<ChangeMessageInfo> messages = gApi.changes().id(changeId).get().messages;
+ assertThat(Iterables.getLast(messages).message)
+ .isEqualTo(
+ "Patch Set 1: Owners-Override+1\n\n"
+ + "The vote Owners-Override+1 is ignored as code-owner override since the label"
+ + " doesn't allow self approval of the patch set uploader.\n");
+ }
+
+ @Test
+ @GerritConfig(name = "plugin.code-owners.overrideApproval", value = "Owners-Override+1")
+ public void changeMessageExtendedIfUpgradedCodeOwnerOverrideIsIgnoredDueToSelfApproval()
+ throws Exception {
+ LabelDefinitionInput input = new LabelDefinitionInput();
+ input.values = ImmutableMap.of("+2", "Override", "+1", "Override", " 0", "No Override");
+ input.ignoreSelfApproval = true;
+ gApi.projects().name(project.get()).label("Owners-Override").create(input);
+
+ // Allow to vote on the Owners-Override label.
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(
+ TestProjectUpdate.allowLabel("Owners-Override")
+ .range(0, 2)
+ .ref("refs/*")
+ .group(REGISTERED_USERS)
+ .build())
+ .update();
+
+ String changeId = createChange().getChangeId();
+
+ gApi.changes().id(changeId).current().review(new ReviewInput().label("Owners-Override", 1));
+
+ // Upgrade the approval from Owners-Override+1 to Owners-Override+2
+ gApi.changes().id(changeId).current().review(new ReviewInput().label("Owners-Override", 2));
+
+ Collection<ChangeMessageInfo> messages = gApi.changes().id(changeId).get().messages;
+ assertThat(Iterables.getLast(messages).message)
+ .isEqualTo(
+ "Patch Set 1: Owners-Override+2\n\n"
+ + "The vote Owners-Override+2 is ignored as code-owner override since the label"
+ + " doesn't allow self approval of the patch set uploader.\n");
+ }
+
+ @Test
+ @GerritConfig(name = "plugin.code-owners.overrideApproval", value = "Owners-Override+1")
+ public void changeMessageExtendedIfDowngradedCodeOwnerOverrideIsIgnoredDueToSelfApproval()
+ throws Exception {
+ LabelDefinitionInput input = new LabelDefinitionInput();
+ input.values = ImmutableMap.of("+2", "Override", "+1", "Override", " 0", "No Override");
+ input.ignoreSelfApproval = true;
+ gApi.projects().name(project.get()).label("Owners-Override").create(input);
+
+ // Allow to vote on the Owners-Override label.
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(
+ TestProjectUpdate.allowLabel("Owners-Override")
+ .range(0, 2)
+ .ref("refs/*")
+ .group(REGISTERED_USERS)
+ .build())
+ .update();
+
+ String changeId = createChange().getChangeId();
+
+ gApi.changes().id(changeId).current().review(new ReviewInput().label("Owners-Override", 2));
+
+ // Downgrade the approval from Owners-Override+2 to Owners-Override+1
+ gApi.changes().id(changeId).current().review(new ReviewInput().label("Owners-Override", 1));
+
+ Collection<ChangeMessageInfo> messages = gApi.changes().id(changeId).get().messages;
+ assertThat(Iterables.getLast(messages).message)
+ .isEqualTo(
+ "Patch Set 1: Owners-Override+1\n\n"
+ + "The vote Owners-Override+1 is ignored as code-owner override since the label"
+ + " doesn't allow self approval of the patch set uploader.\n");
+ }
+
+ @Test
+ @GerritConfig(name = "plugin.code-owners.overrideApproval", value = "Owners-Override+1")
+ public void changeMessageNotExtendedIfIgnoredCodeOwnerOverrideIsRemoved() throws Exception {
+ LabelDefinitionInput input = new LabelDefinitionInput();
+ input.values = ImmutableMap.of("+1", "Override", " 0", "No Override");
+ input.ignoreSelfApproval = true;
+ gApi.projects().name(project.get()).label("Owners-Override").create(input);
+
+ // Allow to vote on the Owners-Override label.
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(
+ TestProjectUpdate.allowLabel("Owners-Override")
+ .range(0, 1)
+ .ref("refs/*")
+ .group(REGISTERED_USERS)
+ .build())
+ .update();
+
+ String changeId = createChange().getChangeId();
+
+ gApi.changes().id(changeId).current().review(new ReviewInput().label("Owners-Override", 1));
+
+ // Remove the override approval
+ gApi.changes().id(changeId).current().review(new ReviewInput().label("Owners-Override", 0));
+
+ Collection<ChangeMessageInfo> messages = gApi.changes().id(changeId).get().messages;
+ assertThat(Iterables.getLast(messages).message).isEqualTo("Patch Set 1: -Owners-Override");
+ }
+
+ @Test
+ @GerritConfig(name = "plugin.code-owners.overrideApproval", value = "Owners-Override+1")
+ public void changeMessageExtendedIfNonSelfApprovalCodeOwnerOverrideIsApplied() throws Exception {
+ LabelDefinitionInput input = new LabelDefinitionInput();
+ input.values = ImmutableMap.of("+1", "Override", " 0", "No Override");
+ input.ignoreSelfApproval = true;
+ gApi.projects().name(project.get()).label("Owners-Override").create(input);
+
+ // Allow to vote on the Owners-Override label.
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(
+ TestProjectUpdate.allowLabel("Owners-Override")
+ .range(0, 1)
+ .ref("refs/*")
+ .group(REGISTERED_USERS)
+ .build())
+ .update();
+
+ String changeId = createChange().getChangeId();
+
+ requestScopeOperations.setApiUser(user.id());
+ gApi.changes().id(changeId).current().review(new ReviewInput().label("Owners-Override", 1));
+
+ Collection<ChangeMessageInfo> messages = gApi.changes().id(changeId).get().messages;
+ assertThat(Iterables.getLast(messages).message)
+ .isEqualTo(
+ String.format(
+ "Patch Set 1: Owners-Override+1\n\n"
+ + "By voting Owners-Override+1 the code-owners submit requirement is overridden by %s\n",
+ user.fullName()));
+ }
}
