opendev/gerrit
Code Issues Proposed changes

Set 'Status' field in notedb when state changes

Browse Source 
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
This commit is contained in:
Richard Ipsum
2015-12-10 15:00:21 +00:00
parent 679574697b
commit eed786ae10
4 changed files with 22 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
gerrit-server/src/main/java/com/google/gerrit/server/change/Abandon.java
View File

@@ -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<ChangeResource, AbandonInput>,
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<ChangeResource, AbandonInput>,
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 {

11
gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java
View File

@@ -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.<Account.Id> emptySet());

3
gerrit-server/src/main/java/com/google/gerrit/server/change/PublishDraftPatchSet.java
View File

@@ -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<RevisionResource, In
private void saveChange(ChangeContext ctx) throws OrmException {
change = ctx.getChange();
ChangeUpdate update = ctx.getChangeUpdate();
wasDraftChange = change.getStatus() == Change.Status.DRAFT;
if (wasDraftChange) {
change.setStatus(Change.Status.NEW);
update.setStatus(change.getStatus());
ChangeUtil.updated(change);
ctx.getDb().changes().update(Collections.singleton(change));
}

5
gerrit-server/src/main/java/com/google/gerrit/server/change/Restore.java
View File

@@ -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.ReplyToChangeSender;
import com.google.gerrit.server.mail.RestoredSender;
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;
@@ -109,6 +110,7 @@ public class Restore implements RestModifyView<ChangeResource, RestoreInput>,
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<ChangeResource, RestoreInput>,
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 {