OnCodeOwnerApproval: Detect if approval is ignored due to self approval If an approval is ignored due to self approval (because the required approval label is configured to ignore self approvals and the 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: I8c5571459544ae2697812c0b1f8dc975020c3cf4
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/OnCodeOwnerApproval.java b/java/com/google/gerrit/plugins/codeowners/backend/OnCodeOwnerApproval.java index e818163..dff2876 100644 --- a/java/com/google/gerrit/plugins/codeowners/backend/OnCodeOwnerApproval.java +++ b/java/com/google/gerrit/plugins/codeowners/backend/OnCodeOwnerApproval.java
@@ -124,6 +124,18 @@ return Optional.empty(); } + if (isIgnoredDueToSelfApproval(user, patchSet, requiredApproval)) { + if (isCodeOwnerApprovalNewlyApplied(requiredApproval, oldApprovals, newVote) + || isCodeOwnerApprovalUpOrDowngraded(requiredApproval, oldApprovals, newVote)) { + return Optional.of( + String.format( + "The vote %s is ignored as code-owner approval since the label doesn't allow" + + " self approval of the patch set uploader.", + newVote)); + } + return Optional.empty(); + } + boolean hasImplicitApprovalByUser = codeOwnersPluginConfiguration.areImplicitApprovalsEnabled(changeNotes.getProjectName()) && patchSet.uploader().equals(user.getAccountId()); @@ -222,6 +234,12 @@ pathsToAppend.forEach(path -> message.append(String.format("* %s\n", JgitPath.of(path).get()))); } + private boolean isIgnoredDueToSelfApproval( + IdentifiedUser user, PatchSet patchSet, RequiredApproval requiredApproval) { + return patchSet.uploader().equals(user.getAccountId()) + && requiredApproval.labelType().isIgnoreSelfApproval(); + } + private boolean isCodeOwnerApprovalNewlyApplied( RequiredApproval requiredApproval, Map<String, Short> oldApprovals, LabelVote newVote) { String labelName = requiredApproval.labelType().getName();
diff --git a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/OnCodeOwnerApprovalIT.java b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/OnCodeOwnerApprovalIT.java index 7c0114b..415dcc6 100644 --- a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/OnCodeOwnerApprovalIT.java +++ b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/OnCodeOwnerApprovalIT.java
@@ -945,4 +945,115 @@ Collection<ChangeMessageInfo> messages = gApi.changes().id(changeId).get().messages; assertThat(Iterables.getLast(messages).message).isEqualTo("Patch Set 1: Code-Review+1"); } + + @Test + public void changeMessageExtendedIfCodeOwnerApprovalIsIgnoredDueToSelfApproval() + throws Exception { + LabelDefinitionInput input = new LabelDefinitionInput(); + input.ignoreSelfApproval = true; + gApi.projects().name(allProjects.get()).label("Code-Review").update(input); + + String changeId = createChange().getChangeId(); + + recommend(changeId); + + Collection<ChangeMessageInfo> messages = gApi.changes().id(changeId).get().messages; + assertThat(Iterables.getLast(messages).message) + .isEqualTo( + "Patch Set 1: Code-Review+1\n\n" + + "The vote Code-Review+1 is ignored as code-owner approval since the label" + + " doesn't allow self approval of the patch set uploader.\n"); + } + + @Test + public void changeMessageExtendedIfUpgradedCodeOwnerApprovalIsIgnoredDueToSelfApproval() + throws Exception { + LabelDefinitionInput input = new LabelDefinitionInput(); + input.ignoreSelfApproval = true; + gApi.projects().name(allProjects.get()).label("Code-Review").update(input); + + String changeId = createChange().getChangeId(); + + recommend(changeId); + + // Upgrade the approval from Code-Review+1 to Code-Review+2 + approve(changeId); + + Collection<ChangeMessageInfo> messages = gApi.changes().id(changeId).get().messages; + assertThat(Iterables.getLast(messages).message) + .isEqualTo( + "Patch Set 1: Code-Review+2\n\n" + + "The vote Code-Review+2 is ignored as code-owner approval since the label" + + " doesn't allow self approval of the patch set uploader.\n"); + } + + @Test + public void changeMessageExtendedIfDowngradedCodeOwnerApprovalIsIgnoredDueToSelfApproval() + throws Exception { + LabelDefinitionInput input = new LabelDefinitionInput(); + input.ignoreSelfApproval = true; + gApi.projects().name(allProjects.get()).label("Code-Review").update(input); + + String changeId = createChange().getChangeId(); + + approve(changeId); + + // Downgrade the approval from Code-Review+2 to Code-Review+1 + recommend(changeId); + + Collection<ChangeMessageInfo> messages = gApi.changes().id(changeId).get().messages; + assertThat(Iterables.getLast(messages).message) + .isEqualTo( + "Patch Set 1: Code-Review+1\n\n" + + "The vote Code-Review+1 is ignored as code-owner approval since the label" + + " doesn't allow self approval of the patch set uploader.\n"); + } + + @Test + public void changeMessageNotExtendedIfIgnoredCodeOwnerApprovalIsRemoved() throws Exception { + LabelDefinitionInput input = new LabelDefinitionInput(); + input.ignoreSelfApproval = true; + gApi.projects().name(allProjects.get()).label("Code-Review").update(input); + + String changeId = createChange().getChangeId(); + + recommend(changeId); + + // Remove the code-owner approval + gApi.changes().id(changeId).current().review(new ReviewInput().label("Code-Review", 0)); + + Collection<ChangeMessageInfo> messages = gApi.changes().id(changeId).get().messages; + assertThat(Iterables.getLast(messages).message).isEqualTo("Patch Set 1: -Code-Review"); + } + + @Test + public void changeMessageExtendedIfNonSelfApprovalCodeOwnerApprovalIsApplied() throws Exception { + codeOwnerConfigOperations + .newCodeOwnerConfig() + .project(project) + .branch("master") + .folderPath("/foo/") + .addCodeOwnerEmail(user.email()) + .create(); + + LabelDefinitionInput input = new LabelDefinitionInput(); + input.ignoreSelfApproval = true; + gApi.projects().name(allProjects.get()).label("Code-Review").update(input); + + String path = "foo/bar.baz"; + String changeId = createChange("Test Change", path, "file content").getChangeId(); + + requestScopeOperations.setApiUser(user.id()); + recommend(changeId); + + Collection<ChangeMessageInfo> messages = gApi.changes().id(changeId).get().messages; + assertThat(Iterables.getLast(messages).message) + .isEqualTo( + String.format( + "Patch Set 1: Code-Review+1\n\n" + + "By voting Code-Review+1 the following files are now code-owner approved by" + + " %s:\n" + + "* %s\n", + user.fullName(), path)); + } }