Merge changes from topic 'subm'

* changes:
  Rework SUBM label in LabelId
  ReceiveCommits: Add SUBM approval when closing changes
This commit is contained in:
Dave Borowitz
2016-03-09 22:04:35 +00:00
committed by Gerrit Code Review
14 changed files with 32 additions and 17 deletions

View File

@@ -189,7 +189,7 @@ public class SubmitOnPushIT extends AbstractDaemonTest {
.setRefSpecs(new RefSpec(r.getCommit().name() + ":refs/heads/master")) .setRefSpecs(new RefSpec(r.getCommit().name() + ":refs/heads/master"))
.call(); .call();
assertCommit(project, "refs/heads/master"); assertCommit(project, "refs/heads/master");
assertThat(getSubmitter(r.getPatchSetId())).isNull(); assertSubmitApproval(r.getPatchSetId());
ChangeInfo c = ChangeInfo c =
gApi.changes().id(r.getPatchSetId().getParentKey().get()).get(); gApi.changes().id(r.getPatchSetId().getParentKey().get()).get();
assertThat(c.status).isEqualTo(ChangeStatus.MERGED); assertThat(c.status).isEqualTo(ChangeStatus.MERGED);
@@ -209,7 +209,7 @@ public class SubmitOnPushIT extends AbstractDaemonTest {
r.assertOkStatus(); r.assertOkStatus();
assertCommit(project, "refs/heads/master"); assertCommit(project, "refs/heads/master");
assertThat(getSubmitter(r.getPatchSetId())).isNull(); assertSubmitApproval(r.getPatchSetId());
ChangeInfo c = ChangeInfo c =
gApi.changes().id(r.getPatchSetId().getParentKey().get()).get(); gApi.changes().id(r.getPatchSetId().getParentKey().get()).get();
assertThat(c.status).isEqualTo(ChangeStatus.MERGED); assertThat(c.status).isEqualTo(ChangeStatus.MERGED);
@@ -225,7 +225,7 @@ public class SubmitOnPushIT extends AbstractDaemonTest {
private void assertSubmitApproval(PatchSet.Id patchSetId) throws Exception { private void assertSubmitApproval(PatchSet.Id patchSetId) throws Exception {
PatchSetApproval a = getSubmitter(patchSetId); PatchSetApproval a = getSubmitter(patchSetId);
assertThat(a.isSubmit()).isTrue(); assertThat(a.isLegacySubmit()).isTrue();
assertThat(a.getValue()).isEqualTo((short) 1); assertThat(a.getValue()).isEqualTo((short) 1);
assertThat(a.getAccountId()).isEqualTo(admin.id); assertThat(a.getAccountId()).isEqualTo(admin.id);
} }

View File

@@ -356,7 +356,7 @@ public abstract class AbstractSubmit extends AbstractDaemonTest {
PatchSetApproval submitter = approvalsUtil.getSubmitter( PatchSetApproval submitter = approvalsUtil.getSubmitter(
db, cn, new PatchSet.Id(cn.getChangeId(), psId)); db, cn, new PatchSet.Id(cn.getChangeId(), psId));
assertThat(submitter).isNotNull(); assertThat(submitter).isNotNull();
assertThat(submitter.isSubmit()).isTrue(); assertThat(submitter.isLegacySubmit()).isTrue();
assertThat(submitter.getAccountId()).isEqualTo(admin.getId()); assertThat(submitter.getAccountId()).isEqualTo(admin.getId());
} }

View File

@@ -20,7 +20,11 @@ import com.google.gwtorm.client.StringKey;
public class LabelId extends StringKey<com.google.gwtorm.client.Key<?>> { public class LabelId extends StringKey<com.google.gwtorm.client.Key<?>> {
private static final long serialVersionUID = 1L; private static final long serialVersionUID = 1L;
public static final LabelId SUBMIT = new LabelId("SUBM"); static final String LEGACY_SUBMIT_NAME = "SUBM";
public static LabelId legacySubmit() {
return new LabelId(LEGACY_SUBMIT_NAME);
}
@Column(id = 1) @Column(id = 1)
public String id; public String id;

View File

@@ -149,8 +149,8 @@ public final class PatchSetApproval {
return getLabelId().get(); return getLabelId().get();
} }
public boolean isSubmit() { public boolean isLegacySubmit() {
return LabelId.SUBMIT.get().equals(getLabel()); return LabelId.LEGACY_SUBMIT_NAME.equals(getLabel());
} }
@Override @Override

View File

@@ -321,7 +321,7 @@ public class ApprovalsUtil {
} }
PatchSetApproval submitter = null; PatchSetApproval submitter = null;
for (PatchSetApproval a : approvals) { for (PatchSetApproval a : approvals) {
if (a.getPatchSetId().equals(c) && a.getValue() > 0 && a.isSubmit()) { if (a.getPatchSetId().equals(c) && a.getValue() > 0 && a.isLegacySubmit()) {
if (submitter == null if (submitter == null
|| a.getGranted().compareTo(submitter.getGranted()) > 0) { || a.getGranted().compareTo(submitter.getGranted()) > 0) {
submitter = a; submitter = a;

View File

@@ -611,7 +611,7 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
for (PatchSetApproval a : approvalsUtil.byPatchSetUser( for (PatchSetApproval a : approvalsUtil.byPatchSetUser(
ctx.getDb(), ctx.getControl(), psId, user.getAccountId())) { ctx.getDb(), ctx.getControl(), psId, user.getAccountId())) {
if (a.isSubmit()) { if (a.isLegacySubmit()) {
continue; continue;
} }

View File

@@ -127,7 +127,7 @@ public class LabelNormalizer {
checkArgument(changeId.equals(ctl.getId()), checkArgument(changeId.equals(ctl.getId()),
"Approval %s does not match change %s", "Approval %s does not match change %s",
psa.getKey(), ctl.getChange().getKey()); psa.getKey(), ctl.getChange().getKey());
if (psa.isSubmit()) { if (psa.isLegacySubmit()) {
unchanged.add(psa); unchanged.add(psa);
continue; continue;
} }

View File

@@ -253,7 +253,7 @@ public class MergeUtil {
continue; continue;
} }
if (a.isSubmit()) { if (a.isLegacySubmit()) {
// Submit is treated specially, below (becomes committer) // Submit is treated specially, below (becomes committer)
// //
if (submitAudit == null if (submitAudit == null

View File

@@ -73,6 +73,7 @@ import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Branch; import com.google.gerrit.reviewdb.client.Branch;
import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.ChangeMessage; 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.PatchSet;
import com.google.gerrit.reviewdb.client.PatchSetApproval; import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.reviewdb.client.PatchSetInfo; import com.google.gerrit.reviewdb.client.PatchSetInfo;
@@ -2560,6 +2561,16 @@ public class ReceiveCommits {
msg.setMessage(msgBuf.toString()); msg.setMessage(msgBuf.toString());
cmUtil.addChangeMessage(ctx.getDb(), update, msg); cmUtil.addChangeMessage(ctx.getDb(), update, msg);
PatchSetApproval submitter = new PatchSetApproval(
new PatchSetApproval.Key(
change.currentPatchSetId(),
ctx.getUser().getAccountId(),
LabelId.legacySubmit()),
(short) 1, ctx.getWhen());
update.putApproval(submitter.getLabel(), submitter.getValue());
ctx.getDb().patchSetApprovals().upsert(
Collections.singleton(submitter));
return true; return true;
} }

View File

@@ -301,7 +301,7 @@ public class ReplaceOp extends BatchUpdate.Op {
for (PatchSetApproval a : approvalsUtil.byPatchSetUser(ctx.getDb(), for (PatchSetApproval a : approvalsUtil.byPatchSetUser(ctx.getDb(),
ctx.getControl(), priorPatchSetId, ctx.getControl(), priorPatchSetId,
ctx.getUser().getAccountId())) { ctx.getUser().getAccountId())) {
if (a.isSubmit()) { if (a.isLegacySubmit()) {
continue; continue;
} }

View File

@@ -337,7 +337,7 @@ abstract class SubmitStrategyOp extends BatchUpdate.Op {
new PatchSetApproval.Key( new PatchSetApproval.Key(
psId, psId,
ctx.getUser().getAccountId(), ctx.getUser().getAccountId(),
LabelId.SUBMIT), LabelId.legacySubmit()),
(short) 1, ctx.getWhen()); (short) 1, ctx.getWhen());
byKey.put(submitter.getKey(), submitter); byKey.put(submitter.getKey(), submitter);
submitter.setValue((short) 1); submitter.setValue((short) 1);
@@ -373,7 +373,7 @@ abstract class SubmitStrategyOp extends BatchUpdate.Op {
// TODO(dborowitz): Don't use a label in notedb; just check when status // TODO(dborowitz): Don't use a label in notedb; just check when status
// change happened. // change happened.
for (PatchSetApproval psa : normalized.unchanged()) { for (PatchSetApproval psa : normalized.unchanged()) {
if (includeUnchanged || psa.isSubmit()) { if (includeUnchanged || psa.isLegacySubmit()) {
logDebug("Adding submit label " + psa); logDebug("Adding submit label " + psa);
update.putApprovalFor( update.putApprovalFor(
psa.getAccountId(), psa.getLabel(), psa.getValue()); psa.getAccountId(), psa.getLabel(), psa.getValue());

View File

@@ -359,7 +359,7 @@ public class ChangeField {
Set<String> allApprovals = Sets.newHashSet(); Set<String> allApprovals = Sets.newHashSet();
Set<String> distinctApprovals = Sets.newHashSet(); Set<String> distinctApprovals = Sets.newHashSet();
for (PatchSetApproval a : input.currentApprovals()) { for (PatchSetApproval a : input.currentApprovals()) {
if (a.getValue() != 0 && !a.isSubmit()) { if (a.getValue() != 0 && !a.isLegacySubmit()) {
allApprovals.add(formatLabel(a.getLabel(), a.getValue(), allApprovals.add(formatLabel(a.getLabel(), a.getValue(),
a.getAccountId())); a.getAccountId()));
distinctApprovals.add(formatLabel(a.getLabel(), a.getValue())); distinctApprovals.add(formatLabel(a.getLabel(), a.getValue()));

View File

@@ -857,7 +857,7 @@ public class ChangeData {
public Optional<PatchSetApproval> getSubmitApproval() public Optional<PatchSetApproval> getSubmitApproval()
throws OrmException { throws OrmException {
for (PatchSetApproval psa : currentApprovals()) { for (PatchSetApproval psa : currentApprovals()) {
if (psa.isSubmit()) { if (psa.isLegacySubmit()) {
return Optional.fromNullable(psa); return Optional.fromNullable(psa);
} }
} }