From eed786ae10ffeee7b3c668cf26f1da52e6dce311 Mon Sep 17 00:00:00 2001 From: Richard Ipsum Date: Thu, 10 Dec 2015 15:00:21 +0000 Subject: [PATCH] Set 'Status' field in notedb when state changes Merges already set the status field, so no changes required for that. Using fixStatus in ChangeInserter rather than setStatus because the tests insert changes that already have their status set to 'merged', and the setStatus method cannot be used to set a ChangeUpdate's status to 'merged'. Change-Id: Ia6ff97e88771a273a0524f1afa237711a4748e23 --- .../java/com/google/gerrit/server/change/Abandon.java | 5 ++++- .../google/gerrit/server/change/ChangeInserter.java | 11 +++++++++++ .../gerrit/server/change/PublishDraftPatchSet.java | 3 +++ .../java/com/google/gerrit/server/change/Restore.java | 5 ++++- 4 files changed, 22 insertions(+), 2 deletions(-) diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Abandon.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Abandon.java index ff1035112f..3ece2f6502 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Abandon.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Abandon.java @@ -38,6 +38,7 @@ import com.google.gerrit.server.git.BatchUpdate.Context; import com.google.gerrit.server.git.UpdateException; import com.google.gerrit.server.mail.AbandonedSender; import com.google.gerrit.server.mail.ReplyToChangeSender; +import com.google.gerrit.server.notedb.ChangeUpdate; import com.google.gerrit.server.project.ChangeControl; import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; @@ -118,6 +119,7 @@ public class Abandon implements RestModifyView, public void updateChange(ChangeContext ctx) throws OrmException, ResourceConflictException { change = ctx.getChange(); + ChangeUpdate update = ctx.getChangeUpdate(); if (change == null || !change.getStatus().isOpen()) { throw new ResourceConflictException("change is " + status(change)); } else if (change.getStatus() == Change.Status.DRAFT) { @@ -129,8 +131,9 @@ public class Abandon implements RestModifyView, change.setLastUpdatedOn(ctx.getWhen()); ctx.getDb().changes().update(Collections.singleton(change)); + update.setStatus(change.getStatus()); message = newMessage(ctx.getDb()); - cmUtil.addChangeMessage(ctx.getDb(), ctx.getChangeUpdate(), message); + cmUtil.addChangeMessage(ctx.getDb(), update, message); } private ChangeMessage newMessage(ReviewDb db) throws OrmException { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java index 1217f34212..22d26b6f3c 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java @@ -251,6 +251,17 @@ public class ChangeInserter extends BatchUpdate.InsertChangeOp { db.patchSets().insert(Collections.singleton(patchSet)); db.changes().insert(Collections.singleton(change)); update.setTopic(change.getTopic()); + + /* TODO: fixStatus 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 + * + * is it possible to make the tests use the merge code path, + * instead of setting the status directly? + */ + update.fixStatus(change.getStatus()); + LabelTypes labelTypes = ctl.getProjectControl().getLabelTypes(); approvalsUtil.addReviewers(db, update, labelTypes, change, patchSet, patchSetInfo, reviewers, Collections. emptySet()); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PublishDraftPatchSet.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PublishDraftPatchSet.java index 9f2a3f9f18..fc00018121 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PublishDraftPatchSet.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PublishDraftPatchSet.java @@ -47,6 +47,7 @@ import com.google.gerrit.server.git.UpdateException; import com.google.gerrit.server.mail.CreateChangeSender; import com.google.gerrit.server.mail.MailUtil.MailRecipients; import com.google.gerrit.server.mail.ReplacePatchSetSender; +import com.google.gerrit.server.notedb.ChangeUpdate; import com.google.gerrit.server.patch.PatchSetInfoFactory; import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; @@ -194,9 +195,11 @@ public class PublishDraftPatchSet implements RestModifyView, ResourceConflictException { caller = ctx.getUser().asIdentifiedUser(); change = ctx.getChange(); + ChangeUpdate update = ctx.getChangeUpdate(); if (change == null || change.getStatus() != Status.ABANDONED) { throw new ResourceConflictException("change is " + status(change)); } @@ -116,9 +118,10 @@ public class Restore implements RestModifyView, change.setStatus(Status.NEW); change.setLastUpdatedOn(ctx.getWhen()); ctx.getDb().changes().update(Collections.singleton(change)); + update.setStatus(change.getStatus()); message = newMessage(ctx.getDb()); - cmUtil.addChangeMessage(ctx.getDb(), ctx.getChangeUpdate(), message); + cmUtil.addChangeMessage(ctx.getDb(), update, message); } private ChangeMessage newMessage(ReviewDb db) throws OrmException {