Keep status of non-staged cherry-picked changes on build approve

A fix on build approve method.

When running 'build approve' after merging a branch to another
branch, an incorrect status change was fulfilled:
The status of cherry picked change (on targed branch) was changed
from new/abandoned to merged.

This occurred because cherry picked change had the same change-id as
one of the merged changes on source branch.

Fixes: QTQAINFRA-3411
Fixes: QTQAINFRA-3374
Change-Id: Ie04c5268b0196f5ff9e43618fe93c300865b9ea6
Reviewed-by: Jukka Jokiniva <[email protected]>

Author: vs-systencess

src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtCommandBuildApprove.java

@@ -236,7 +236,7 @@ private void approveBuildChanges()
     }
 
     updateChanges(
-        affectedChanges, Change.Status.MERGED, null, message, ChangeMessagesUtil.TAG_MERGED, true);
+        affectedChanges, Change.Status.MERGED, Change.Status.INTEGRATING, message, ChangeMessagesUtil.TAG_MERGED, true);
 
     logger.atInfo().log(
         "qtcodereview: staging-approve build %s merged into branch %s", buildBranch, destBranchKey);
@@ -273,7 +273,7 @@ private void rejectBuildChanges()
 
   private void updateChanges(
       List<Entry<ChangeData, RevCommit>> list,
-      Change.Status status,
+      Change.Status newStatus,
       Change.Status oldStatus,
       String changeMessage,
       String tag,
@@ -284,14 +284,13 @@ private void updateChanges(
         new ArrayList<Map.Entry<ChangeData, RevCommit>>();
 
     // do the db update
-    QtChangeUpdateOp op = qtUpdateFactory.create(status, oldStatus, changeMessage, null, tag, null);
+    QtChangeUpdateOp op = qtUpdateFactory.create(newStatus, oldStatus, changeMessage, null, tag, null);
     try (BatchUpdate u = updateFactory.create(projectKey, user, TimeUtil.nowTs())) {
       for (Entry<ChangeData, RevCommit> item : list) {
         ChangeData cd = item.getKey();
         Change change = cd.change();
-        if ((oldStatus == null || change.getStatus() == oldStatus)
-            && change.getStatus() != Change.Status.MERGED) {
-          if (status == Change.Status.MERGED) {
+        if (change.getStatus() == oldStatus) {
+          if (newStatus == Change.Status.MERGED) {
             ObjectId obj = git.resolve(cd.currentPatchSet().getRevision().get());
             CodeReviewCommit currCommit = new CodeReviewCommit(obj);
             currCommit.setPatchsetId(cd.currentPatchSet().getId());
@@ -303,7 +302,7 @@ private void updateChanges(
             }
             u.addOp(
                 changeId,
-                qtUpdateFactory.create(status, oldStatus, changeMessage, null, tag, currCommit));
+                qtUpdateFactory.create(newStatus, oldStatus, changeMessage, null, tag, currCommit));
           } else {
             u.addOp(change.getId(), op);
           }

src/test/java/com/googlesource/gerrit/plugins/qtcodereview/QtCodeReviewIT.java

@@ -14,6 +14,7 @@
 import com.google.gerrit.acceptance.TestAccount;
 import com.google.gerrit.acceptance.TestPlugin;
 import com.google.gerrit.common.FooterConstants;
+import com.google.gerrit.extensions.api.changes.CherryPickInput;
 import com.google.gerrit.extensions.client.ChangeStatus;
 import com.google.gerrit.extensions.common.ApprovalInfo;
 import com.google.gerrit.extensions.common.ChangeInfo;
@@ -103,6 +104,24 @@ protected RestResponse call_REST_API_Stage(String changeId, String revisionId) t
     return response;
   }
 
+  protected ChangeInfo cherryPick(final PushOneCommit.Result c, final String branch) throws Exception {
+    // If the commit message does not specify a Change-Id, a new one is picked for the destination change.
+    final CherryPickInput input = new CherryPickInput();
+    input.message = "CHERRY" + c.getCommit().getFullMessage();
+    input.destination = branch;
+
+    final RestResponse response = call_REST_API_CherryPick(c.getChangeId(), c.getCommit().getName(), input);
+    response.assertOK();
+    return newGson().fromJson(response.getReader(), ChangeInfo.class);
+  }
+
+  protected RestResponse call_REST_API_CherryPick(
+    final String changeId, final String revisionId, final CherryPickInput input)
+        throws Exception {
+    final String url = "/changes/" + changeId + "/revisions/" + revisionId + "/cherrypick";
+    return userRestSession.post(url, input);
+  }
+
   protected void QtUnStage(PushOneCommit.Result c) throws Exception {
     RestResponse response = call_REST_API_UnStage(c.getChangeId(), getCurrentPatchId(c));
     response.assertOK();

src/test/java/com/googlesource/gerrit/plugins/qtcodereview/QtCommandBuildApproveIT.java

@@ -3,12 +3,18 @@
 package com.googlesource.gerrit.plugins.qtcodereview;
 
 import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.extensions.client.ListChangesOption.CURRENT_REVISION;
 import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
 
+import com.google.common.collect.ImmutableList;
 import com.google.gerrit.acceptance.PushOneCommit;
 import com.google.gerrit.acceptance.TestPlugin;
 import com.google.gerrit.acceptance.UseSsh;
 import com.google.gerrit.common.data.Permission;
+import com.google.gerrit.extensions.api.changes.Changes;
+import com.google.gerrit.extensions.client.ChangeStatus;
+import com.google.gerrit.extensions.common.ChangeInfo;
+import com.google.gerrit.reviewdb.client.Branch;
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.ChangeMessage;
 import java.io.StringBufferInputStream;
@@ -29,7 +35,10 @@ public class QtCommandBuildApproveIT extends QtCodeReviewIT {
 
   @Before
   public void SetDefaultPermissions() throws Exception {
+    createBranch(new Branch.NameKey(project, "feature"));
+
     grant(project, "refs/heads/master", Permission.QT_STAGE, false, REGISTERED_USERS);
+    grant(project, "refs/heads/feature", Permission.QT_STAGE, false, REGISTERED_USERS);
     grant(project, "refs/staging/*", Permission.PUSH, false, adminGroupUuid());
     grant(project, "refs/builds/*", Permission.CREATE, false, adminGroupUuid());
   }
@@ -79,6 +88,48 @@ public void singleChange_New_Staged_Integrating_Fail() throws Exception {
     RevCommit updatedHead = qtFailBuild("master", "test-build-200", c, initialHead);
   }
 
+  @Test
+  public void cherryPicked_Stays_Intact_After_Merge_And_Build() throws Exception {
+    // make a change on feature branch
+    final PushOneCommit.Result f1 = pushCommit("feature", "f1-commitmsg", "f1-file", "f1-content");
+    approve(f1.getChangeId());
+    gApi.changes().id(f1.getCommit().getName()).current().submit();
+
+    // cherry pick it to the master branch (now there are two changes with same change-id)
+    final ChangeInfo cp = cherryPick(f1, "master");
+
+    // make another change on feature branch
+    final PushOneCommit.Result f2 = pushCommit("feature", "f2-commitmsg", "f2-file", "f2-content");
+    approve(f2.getChangeId());
+    QtStage(f2);
+    QtNewBuild("feature", "feature-build-000");
+    QtApproveBuild("feature", "feature-build-000");
+
+    // make a change on master branch
+    final PushOneCommit.Result m1 = pushCommit("master", "m1-commitmsg", "m1-file", "m1-content");
+    approve(m1.getChangeId());
+    QtStage(m1);
+    QtNewBuild("master", "master-build-000");
+    QtApproveBuild("master", "master-build-000");
+
+    // merge feature branch into master
+    final PushOneCommit mm = pushFactory.create(admin.newIdent(), testRepo);
+    mm.setParents(ImmutableList.of(f2.getCommit(), m1.getCommit()));
+    final PushOneCommit.Result m = mm.to("refs/for/master");
+    m.assertOkStatus();
+    approve(m.getChangeId());
+    QtStage(m);
+    QtNewBuild("master", "merge-build-000");
+    QtApproveBuild("master", "merge-build-000");
+
+    final Changes changes = gApi.changes();
+    assertThat(changes.id(project.get(), "feature", f1.getChangeId()).get(CURRENT_REVISION).status).isEqualTo(ChangeStatus.MERGED);
+    assertThat(changes.id(project.get(), "feature", f2.getChangeId()).get(CURRENT_REVISION).status).isEqualTo(ChangeStatus.MERGED);
+    assertThat(changes.id(project.get(), "master", m1.getChangeId()).get(CURRENT_REVISION).status).isEqualTo(ChangeStatus.MERGED);
+    assertThat(changes.id(project.get(), "master", m.getChangeId()).get(CURRENT_REVISION).status).isEqualTo(ChangeStatus.MERGED);
+    assertThat(changes.id(project.get(), "master", cp.changeId).get(CURRENT_REVISION).status).isEqualTo(ChangeStatus.NEW);
+  }
+
   @Test
   public void multiChange_New_Staged_Integrating_Failed() throws Exception {
     // Push 3 independent commits