ApprovalCopier: Respect ChangeKind.NO_CHANGE in more cases

If a label is configured to copy approvals on a trivial rebase, also
copy the approvals if the rebase is even *more* trivial in that the
parent didn't change. In this case, ChangeKindCache will return
NO_CHANGE for the kind, so add some more logic to handle this.

The same argument applies to copyAllScoresIfNoCodeChange and
copyAllScoresOnMergeFirstParentUpdate.

Change-Id: Ifb68f93bacfc9f47605e5da43eb91805b571a18f

Documentation/config-labels.txt

@@ -256,10 +256,11 @@ applies if the patch set is a merge commit.
 
 If true, all scores for the label are copied forward when a new
 patch set is uploaded that is a new merge commit which only
-differs from the previous patch set in its first parent. The
-first parent would be the parent of the merge commit that is part
-of the change's target branch, whereas the other parent(s) refer to
-the feature branch(es) to be merged.
+differs from the previous patch set in its first parent, or has
+identical parents. The first parent would be the parent of the merge
+commit that is part of the change's target branch, whereas the other
+parent(s) refer to the feature branch(es) to be merged.
+
 Defaults to false.
 
 [[label_copyAllScoresOnTrivialRebase]]
@@ -269,10 +270,13 @@ If true, all scores for the label are copied forward when a new patch
 set is uploaded that is a trivial rebase. A new patch set is considered
 as trivial rebase if the commit message is the same as in the previous
 patch set and if it has the same code delta as the previous patch set.
-This is the case if the change was rebased onto a different parent.
+This is the case if the change was rebased onto a different parent, or
+if the parent did not change at all.
+
 This can be used to enable sticky approvals, reducing turn-around for
 trivial rebases prior to submitting a change.
 For the pre-installed Code-Review label this is enabled by default.
+
 Defaults to false.
 
 [[label_copyAllScoresIfNoCodeChange]]
@@ -286,6 +290,7 @@ approvals on labels that only depend on the code, reducing turn-around
 if only the commit message is changed prior to submitting a change.
 For the Verified label that is installed by the link:pgm-init.html[init]
 site program this is enabled by default.
+
 Defaults to false.
 
 [[label_copyAllScoresIfNoChange]]
@@ -297,7 +302,9 @@ message as the previous patch set. This means that only the patch
 set SHA1 is different. This can be used to enable sticky
 approvals, reducing turn-around for this special case.
 It is recommended to leave this enabled for both Verified and
-Code-Review labels. Defaults to true.
+Code-Review labels.
+
+Defaults to true.
 
 [[label_canOverride]]
 === `label.Label-Name.canOverride`

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/project/LabelTypeIT.java

@@ -133,6 +133,40 @@ public class LabelTypeIT extends AbstractDaemonTest {
     assertApproval(patchSet, 0);
   }
 
+  @Test
+  public void copyAllScoresIfNoCodeChangeAppliesToNoChange() throws Exception {
+    codeReview.setCopyAllScoresIfNoCodeChange(true);
+    codeReview.setCopyAllScoresIfNoChange(false);
+    saveLabelConfig();
+
+    PushOneCommit.Result patchSet = readyPatchSetForNoChangeRebase();
+    rebase(patchSet);
+    assertApproval(patchSet, 1);
+  }
+
+  @Test
+  public void copyAllScoresOnTrivialRebaseAppliesToNoChange() throws Exception {
+    codeReview.setCopyAllScoresOnTrivialRebase(true);
+    codeReview.setCopyAllScoresIfNoChange(false);
+    saveLabelConfig();
+
+    PushOneCommit.Result patchSet = readyPatchSetForNoChangeRebase();
+    rebase(patchSet);
+    assertApproval(patchSet, 1);
+  }
+
+  @Test
+  public void copyAllScoresOnMergeFirstParentUpdateAppliesToNoChange()
+      throws Exception {
+    codeReview.setCopyAllScoresOnMergeFirstParentUpdate(true);
+    codeReview.setCopyAllScoresIfNoChange(false);
+    saveLabelConfig();
+
+    PushOneCommit.Result patchSet = readyPatchSetForNoChangeRebase();
+    rebase(patchSet);
+    assertApproval(patchSet, 1);
+  }
+
   @Test
   public void copyAllScoresIfNoChange() throws Exception {
     PushOneCommit.Result patchSet = readyPatchSetForNoChangeRebase();

gerrit-server/src/main/java/com/google/gerrit/server/ApprovalCopier.java

@@ -16,10 +16,6 @@ package com.google.gerrit.server;
 
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkNotNull;
-import static com.google.gerrit.server.change.ChangeKind.MERGE_FIRST_PARENT_UPDATE;
-import static com.google.gerrit.server.change.ChangeKind.NO_CHANGE;
-import static com.google.gerrit.server.change.ChangeKind.NO_CODE_CHANGE;
-import static com.google.gerrit.server.change.ChangeKind.TRIVIAL_REBASE;
 
 import com.google.common.collect.HashBasedTable;
 import com.google.common.collect.ListMultimap;
@@ -173,10 +169,22 @@ public class ApprovalCopier {
       // may not be psId.get() - 1).
       return true;
     }
-    return (type.isCopyAllScoresOnMergeFirstParentUpdate() && kind == MERGE_FIRST_PARENT_UPDATE)
-        || (type.isCopyAllScoresOnTrivialRebase() && kind == TRIVIAL_REBASE)
-        || (type.isCopyAllScoresIfNoCodeChange() && kind == NO_CODE_CHANGE)
-        || (type.isCopyAllScoresIfNoChange() && kind == NO_CHANGE);
+    switch (kind) {
+      case MERGE_FIRST_PARENT_UPDATE:
+        return type.isCopyAllScoresOnMergeFirstParentUpdate();
+      case NO_CODE_CHANGE:
+        return type.isCopyAllScoresIfNoCodeChange();
+      case TRIVIAL_REBASE:
+        return type.isCopyAllScoresOnTrivialRebase();
+      case NO_CHANGE:
+        return type.isCopyAllScoresIfNoChange()
+            || type.isCopyAllScoresOnTrivialRebase()
+            || type.isCopyAllScoresOnMergeFirstParentUpdate()
+            || type.isCopyAllScoresIfNoCodeChange();
+      case REWORK:
+      default:
+        return false;
+    }
   }
 
   private static PatchSetApproval copy(PatchSetApproval src, PatchSet.Id psId) {