Merge "Insert submission id to autoclosed changes"
diff --git a/java/com/google/gerrit/server/change/ChangeInserter.java b/java/com/google/gerrit/server/change/ChangeInserter.java
index 1bf5103..4263373 100644
--- a/java/com/google/gerrit/server/change/ChangeInserter.java
+++ b/java/com/google/gerrit/server/change/ChangeInserter.java
@@ -55,6 +55,7 @@
import com.google.gerrit.server.git.GroupCollector;
import com.google.gerrit.server.git.validators.CommitValidationException;
import com.google.gerrit.server.git.validators.CommitValidators;
+import com.google.gerrit.server.logging.RequestId;
import com.google.gerrit.server.mail.send.CreateChangeSender;
import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.patch.PatchSetInfoFactory;
@@ -386,7 +387,7 @@
psUtil.insert(
ctx.getRevWalk(), update, psId, commitId, newGroups, pushCert, patchSetDescription);
- /* TODO: fixStatus is used here because the tests
+ /* TODO: fixStatusToMerged is used here because the tests
* (byStatusClosed() in AbstractQueryChangesTest)
* insert changes that are already merged,
* and setStatus may not be used to set the Status to merged
@@ -394,7 +395,11 @@
* is it possible to make the tests use the merge code path,
* instead of setting the status directly?
*/
- update.fixStatus(change.getStatus());
+ if (change.getStatus() == Change.Status.MERGED) {
+ update.fixStatusToMerged(new RequestId(ctx.getChange().getId().toString()));
+ } else {
+ update.setStatus(change.getStatus());
+ }
reviewerAdditions =
reviewerAdder.prepare(ctx.getNotes(), ctx.getUser(), getReviewerInputs(), true);
diff --git a/java/com/google/gerrit/server/change/ConsistencyChecker.java b/java/com/google/gerrit/server/change/ConsistencyChecker.java
index 0374a1c..19db5ee 100644
--- a/java/com/google/gerrit/server/change/ConsistencyChecker.java
+++ b/java/com/google/gerrit/server/change/ConsistencyChecker.java
@@ -44,6 +44,7 @@
import com.google.gerrit.server.PatchSetUtil;
import com.google.gerrit.server.account.Accounts;
import com.google.gerrit.server.git.GitRepositoryManager;
+import com.google.gerrit.server.logging.RequestId;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.PatchSetState;
import com.google.gerrit.server.patch.PatchSetInfoFactory;
@@ -576,7 +577,8 @@
@Override
public boolean updateChange(ChangeContext ctx) {
ctx.getChange().setStatus(Change.Status.MERGED);
- ctx.getUpdate(ctx.getChange().currentPatchSetId()).fixStatus(Change.Status.MERGED);
+ ctx.getUpdate(ctx.getChange().currentPatchSetId())
+ .fixStatusToMerged(new RequestId(ctx.getChange().getId().toString()));
p.status = Status.FIXED;
p.outcome = "Marked change as merged";
return true;
diff --git a/java/com/google/gerrit/server/git/MergedByPushOp.java b/java/com/google/gerrit/server/git/MergedByPushOp.java
index 858a55a..c8b5e3f 100644
--- a/java/com/google/gerrit/server/git/MergedByPushOp.java
+++ b/java/com/google/gerrit/server/git/MergedByPushOp.java
@@ -26,6 +26,7 @@
import com.google.gerrit.server.PatchSetUtil;
import com.google.gerrit.server.config.SendEmailExecutor;
import com.google.gerrit.server.extensions.events.ChangeMerged;
+import com.google.gerrit.server.logging.RequestId;
import com.google.gerrit.server.mail.send.MergedSender;
import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.patch.PatchSetInfoFactory;
@@ -132,9 +133,11 @@
}
change.setCurrentPatchSet(info);
change.setStatus(Change.Status.MERGED);
+ RequestId submissionId = new RequestId(change.getId().toString());
+ change.setSubmissionId(submissionId.toStringForStorage());
// we cannot reconstruct the submit records for when this change was
- // submitted, this is why we must fix the status
- update.fixStatus(Change.Status.MERGED);
+ // submitted, this is why we must fix the status and other details.
+ update.fixStatusToMerged(submissionId);
update.setCurrentPatchSet();
if (change.isWorkInProgress()) {
change.setWorkInProgress(false);
diff --git a/java/com/google/gerrit/server/notedb/ChangeUpdate.java b/java/com/google/gerrit/server/notedb/ChangeUpdate.java
index b2f85fc..02a4dcc 100644
--- a/java/com/google/gerrit/server/notedb/ChangeUpdate.java
+++ b/java/com/google/gerrit/server/notedb/ChangeUpdate.java
@@ -220,8 +220,10 @@
this.status = status;
}
- public void fixStatus(Change.Status status) {
- this.status = status;
+ public void fixStatusToMerged(RequestId submissionId) {
+ checkArgument(submissionId != null, "submission id must be set for merged changes");
+ this.status = Change.Status.MERGED;
+ this.submissionId = submissionId.toStringForStorage();
}
public void putApproval(String label, short value) {
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
index 1df5ca3..e38babb 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -3390,6 +3390,7 @@
ChangeInfo change = gApi.changes().id(r.getChangeId()).get();
assertThat(change.status).isEqualTo(ChangeStatus.MERGED);
+ assertThat(change.submissionId).isNotNull();
assertThat(change.labels.keySet()).containsExactly("Code-Review");
assertThat(change.permittedLabels.keySet()).containsExactly("Code-Review");
assertPermitted(change, "Code-Review", 2);
@@ -3550,6 +3551,7 @@
ChangeInfo change = gApi.changes().id(r.getChangeId()).get();
assertThat(change.status).isEqualTo(ChangeStatus.MERGED);
+ assertThat(change.submissionId).isNotNull();
assertThat(change.labels.keySet()).containsExactly("Code-Review", "Non-Author-Code-Review");
assertThat(change.permittedLabels.keySet()).containsExactly("Code-Review");
assertPermitted(change, "Code-Review", 0, 1, 2);
@@ -3565,6 +3567,7 @@
ChangeInfo change = gApi.changes().id(r.getChangeId()).get();
assertThat(change.status).isEqualTo(ChangeStatus.MERGED);
+ assertThat(change.submissionId).isNotNull();
assertThat(change.labels.keySet()).containsExactly("Code-Review");
assertPermitted(change, "Code-Review", 0, 1, 2);
}
diff --git a/javatests/com/google/gerrit/acceptance/server/change/BUILD b/javatests/com/google/gerrit/acceptance/server/change/BUILD
index 4d1634d..500ab06 100644
--- a/javatests/com/google/gerrit/acceptance/server/change/BUILD
+++ b/javatests/com/google/gerrit/acceptance/server/change/BUILD
@@ -4,5 +4,8 @@
srcs = glob(["*IT.java"]),
group = "server_change",
labels = ["server"],
- deps = ["//java/com/google/gerrit/server/util/time"],
+ deps = [
+ "//java/com/google/gerrit/server/logging",
+ "//java/com/google/gerrit/server/util/time",
+ ],
)
diff --git a/javatests/com/google/gerrit/acceptance/server/change/ConsistencyCheckerIT.java b/javatests/com/google/gerrit/acceptance/server/change/ConsistencyCheckerIT.java
index 1e2d1ba..08719d3 100644
--- a/javatests/com/google/gerrit/acceptance/server/change/ConsistencyCheckerIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/change/ConsistencyCheckerIT.java
@@ -39,6 +39,7 @@
import com.google.gerrit.server.change.ConsistencyChecker;
import com.google.gerrit.server.change.NotifyResolver;
import com.google.gerrit.server.change.PatchSetInserter;
+import com.google.gerrit.server.logging.RequestId;
import com.google.gerrit.server.notedb.ChangeNoteUtil;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.Sequences;
@@ -313,7 +314,8 @@
@Override
public boolean updateChange(ChangeContext ctx) {
ctx.getChange().setStatus(Change.Status.MERGED);
- ctx.getUpdate(ctx.getChange().currentPatchSetId()).fixStatus(Change.Status.MERGED);
+ ctx.getUpdate(ctx.getChange().currentPatchSetId())
+ .fixStatusToMerged(new RequestId(ctx.getChange().getId().toString()));
return true;
}
});
@@ -862,7 +864,8 @@
@Override
public boolean updateChange(ChangeContext ctx) {
ctx.getChange().setStatus(Change.Status.MERGED);
- ctx.getUpdate(ctx.getChange().currentPatchSetId()).fixStatus(Change.Status.MERGED);
+ ctx.getUpdate(ctx.getChange().currentPatchSetId())
+ .fixStatusToMerged(new RequestId(ctx.getChange().getId().toString()));
return true;
}
});
