commit 6df8475ac52e4ebf0abdae1f5e7789ff0d19fc58
author Dave Borowitz <dborowitz@google.com> Tue Mar 08 09:50:30 2016 -0800
committer Dave Borowitz <dborowitz@google.com> Wed Mar 09 13:16:32 2016 -0500
tree df140b787029af27bb3277cf6b79b2d9271535d1
parent 807dcd3d35f0cc7df2ff7caf576de3e08cd9d4ea

ReceiveCommits: Add SUBM approval when closing changes

This records the user and timestamp that the change transitioned from
NEW (or another state) to MERGED. It is inconsistent to set it from
the Submit handler but not in the other case, submitting during push.
(The reason for this inconsistency is historical, since the SUBM
approval *used* to indicate transition to SUBMITTED, which in the
ReceiveCommits path it does not.)

Change-Id: I8ae89733002905d8b57db491ede3595c3faa30fa

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/SubmitOnPushIT.java

diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/SubmitOnPushIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/SubmitOnPushIT.java
index 7e107cf..f171232 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/SubmitOnPushIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/SubmitOnPushIT.java
@@ -189,7 +189,7 @@
         .setRefSpecs(new RefSpec(r.getCommit().name() + ":refs/heads/master"))
         .call();
     assertCommit(project, "refs/heads/master");
-    assertThat(getSubmitter(r.getPatchSetId())).isNull();
+    assertSubmitApproval(r.getPatchSetId());
     ChangeInfo c =
         gApi.changes().id(r.getPatchSetId().getParentKey().get()).get();
     assertThat(c.status).isEqualTo(ChangeStatus.MERGED);
@@ -209,7 +209,7 @@
     r.assertOkStatus();
 
     assertCommit(project, "refs/heads/master");
-    assertThat(getSubmitter(r.getPatchSetId())).isNull();
+    assertSubmitApproval(r.getPatchSetId());
     ChangeInfo c =
         gApi.changes().id(r.getPatchSetId().getParentKey().get()).get();
     assertThat(c.status).isEqualTo(ChangeStatus.MERGED);

gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java

diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java
index 5c3bcad..f913a0a 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java
@@ -73,6 +73,7 @@
 import com.google.gerrit.reviewdb.client.Branch;
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.ChangeMessage;
+import com.google.gerrit.reviewdb.client.LabelId;
 import com.google.gerrit.reviewdb.client.PatchSet;
 import com.google.gerrit.reviewdb.client.PatchSetApproval;
 import com.google.gerrit.reviewdb.client.PatchSetInfo;
@@ -2560,6 +2561,16 @@
           msg.setMessage(msgBuf.toString());
           cmUtil.addChangeMessage(ctx.getDb(), update, msg);
 
+          PatchSetApproval submitter = new PatchSetApproval(
+                new PatchSetApproval.Key(
+                    change.currentPatchSetId(),
+                    ctx.getUser().getAccountId(),
+                    LabelId.SUBMIT),
+                    (short) 1, ctx.getWhen());
+          update.putApproval(submitter.getLabel(), submitter.getValue());
+          ctx.getDb().patchSetApprovals().upsert(
+              Collections.singleton(submitter));
+
           return true;
         }