diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt index fbad212049..422765544f 100644 --- a/Documentation/rest-api-changes.txt +++ b/Documentation/rest-api-changes.txt @@ -7393,6 +7393,10 @@ Name of the topic for the revert change. If not set, the default for Revert endpoint is the topic of the change being reverted, and the default for the RevertSubmission endpoint is `revert-{submission_id}-{timestamp.now}`. Topic can't contain quotation marks. +|`work_in_progress` |optional| +When present, change is marked as Work In Progress. This will also override +the notify value to `OWNER`. + +If not set, the default is false. |============================= [[revert-submission-info]] diff --git a/java/com/google/gerrit/extensions/api/changes/RevertInput.java b/java/com/google/gerrit/extensions/api/changes/RevertInput.java index c4272e493c..148d24a63c 100644 --- a/java/com/google/gerrit/extensions/api/changes/RevertInput.java +++ b/java/com/google/gerrit/extensions/api/changes/RevertInput.java @@ -17,6 +17,10 @@ package com.google.gerrit.extensions.api.changes; import com.google.gerrit.extensions.restapi.DefaultInput; import java.util.Map; +/** + * Input passed to {@code POST /changes/[change-id]/revert} and {@code POST + * /changes/[change-id]/revert_submission} + */ public class RevertInput { @DefaultInput public String message; @@ -26,4 +30,10 @@ public class RevertInput { public Map notifyDetails; public String topic; + + /** + * Mark the change as work-in-progress. This will also override the {@link #notify} value to + * {@link NotifyHandling#OWNER} + */ + public boolean workInProgress; } diff --git a/java/com/google/gerrit/server/git/CommitUtil.java b/java/com/google/gerrit/server/git/CommitUtil.java index 0f46199057..47cbd6091c 100644 --- a/java/com/google/gerrit/server/git/CommitUtil.java +++ b/java/com/google/gerrit/server/git/CommitUtil.java @@ -265,6 +265,9 @@ public class CommitUtil { RevCommit revertCommit = revWalk.parseCommit(revertCommitId); Change changeToRevert = notes.getChange(); Change.Id changeId = Change.id(seq.nextChangeId()); + if (input.workInProgress) { + input.notify = NotifyHandling.OWNER; + } NotifyResolver.Result notify = notifyResolver.resolve(firstNonNull(input.notify, NotifyHandling.ALL), input.notifyDetails); @@ -284,6 +287,7 @@ public class CommitUtil { ccs.remove(user.getAccountId()); ins.setReviewersAndCcs(reviewers, ccs); ins.setRevertOf(notes.getChangeId()); + ins.setWorkInProgress(input.workInProgress); try (BatchUpdate bu = updateFactory.create(notes.getProjectName(), user, ts)) { bu.setRepository(git, revWalk, oi); diff --git a/java/com/google/gerrit/server/restapi/change/CherryPickChange.java b/java/com/google/gerrit/server/restapi/change/CherryPickChange.java index 44dc6e1a07..5eb30b2ea8 100644 --- a/java/com/google/gerrit/server/restapi/change/CherryPickChange.java +++ b/java/com/google/gerrit/server/restapi/change/CherryPickChange.java @@ -173,6 +173,7 @@ public class CherryPickChange { TimeUtil.nowTs(), null, null, + null, null); } @@ -204,7 +205,7 @@ public class CherryPickChange { throws IOException, InvalidChangeOperationException, UpdateException, RestApiException, ConfigInvalidException, NoSuchProjectException { return cherryPick( - sourceChange, project, sourceCommit, input, dest, TimeUtil.nowTs(), null, null, null); + sourceChange, project, sourceCommit, input, dest, TimeUtil.nowTs(), null, null, null, null); } /** @@ -245,7 +246,8 @@ public class CherryPickChange { Timestamp timestamp, @Nullable Change.Id revertedChange, @Nullable ObjectId changeIdForNewChange, - @Nullable Change.Id idForNewChange) + @Nullable Change.Id idForNewChange, + @Nullable Boolean workInProgress) throws IOException, InvalidChangeOperationException, UpdateException, RestApiException, ConfigInvalidException, NoSuchProjectException { @@ -365,7 +367,8 @@ public class CherryPickChange { destChanges.get(0).notes(), cherryPickCommit, sourceChange.currentPatchSetId(), - newTopic); + newTopic, + workInProgress); } else { // Change key not found on destination branch. We can create a new // change. @@ -380,7 +383,8 @@ public class CherryPickChange { sourceCommit, input, revertedChange, - idForNewChange); + idForNewChange, + workInProgress); } bu.execute(); return Result.create(changeId, cherryPickCommit.getFilesWithGitConflicts()); @@ -448,13 +452,17 @@ public class CherryPickChange { ChangeNotes destNotes, CodeReviewCommit cherryPickCommit, PatchSet.Id sourcePatchSetId, - String topic) + String topic, + @Nullable Boolean workInProgress) throws IOException { Change destChange = destNotes.getChange(); PatchSet.Id psId = ChangeUtil.nextPatchSetId(git, destChange.currentPatchSetId()); PatchSetInserter inserter = patchSetInserterFactory.create(destNotes, psId, cherryPickCommit); inserter.setMessage("Uploaded patch set " + inserter.getPatchSetId().get() + "."); inserter.setTopic(topic); + if (workInProgress != null) { + inserter.setWorkInProgress(workInProgress); + } bu.addOp(destChange.getId(), inserter); if (destChange.getCherryPickOf() == null || !destChange.getCherryPickOf().equals(sourcePatchSetId)) { @@ -474,11 +482,19 @@ public class CherryPickChange { @Nullable ObjectId sourceCommit, CherryPickInput input, @Nullable Change.Id revertOf, - @Nullable Change.Id idForNewChange) + @Nullable Change.Id idForNewChange, + @Nullable Boolean workInProgress) throws IOException, InvalidChangeOperationException { Change.Id changeId = idForNewChange != null ? idForNewChange : Change.id(seq.nextChangeId()); ChangeInserter ins = changeInserterFactory.create(changeId, cherryPickCommit, refName); ins.setRevertOf(revertOf); + if (workInProgress != null) { + ins.setWorkInProgress(workInProgress); + } else { + ins.setWorkInProgress( + (sourceChange != null && sourceChange.isWorkInProgress()) + || !cherryPickCommit.getFilesWithGitConflicts().isEmpty()); + } BranchNameKey sourceBranch = sourceChange == null ? null : sourceChange.getDest(); PatchSet.Id sourcePatchSetId = sourceChange == null ? null : sourceChange.currentPatchSetId(); ins.setMessage( @@ -488,10 +504,7 @@ public class CherryPickChange { : "Uploaded patch set 1.") // For revert commits, the message should not include // cherry-pick information. .setTopic(topic) - .setCherryPickOf(sourcePatchSetId) - .setWorkInProgress( - (sourceChange != null && sourceChange.isWorkInProgress()) - || !cherryPickCommit.getFilesWithGitConflicts().isEmpty()); + .setCherryPickOf(sourcePatchSetId); if (input.keepReviewers && sourceChange != null) { ReviewerSet reviewerSet = approvalsUtil.getReviewers(changeNotesFactory.createChecked(sourceChange)); diff --git a/java/com/google/gerrit/server/restapi/change/RevertSubmission.java b/java/com/google/gerrit/server/restapi/change/RevertSubmission.java index cb3a375fe9..ca39a576a2 100644 --- a/java/com/google/gerrit/server/restapi/change/RevertSubmission.java +++ b/java/com/google/gerrit/server/restapi/change/RevertSubmission.java @@ -203,6 +203,7 @@ public class RevertSubmission checkPermissionsForAllChanges(changeResource, changeDatas); input.topic = createTopic(input.topic, submissionId); + return Response.ok(revertSubmission(changeDatas, input)); } @@ -258,6 +259,9 @@ public class RevertSubmission cherryPickInput.base = null; Project.NameKey project = projectAndBranch.project(); cherryPickInput.destination = projectAndBranch.branch(); + if (revertInput.workInProgress) { + cherryPickInput.notify = NotifyHandling.OWNER; + } Collection changesInProjectAndBranch = changesPerProjectAndBranch.get(projectAndBranch); @@ -332,7 +336,11 @@ public class RevertSubmission bu.addOp( changeNotes.getChange().getId(), new CreateCherryPickOp( - revCommitId, generatedChangeId, cherryPickRevertChangeId, timestamp)); + revCommitId, + generatedChangeId, + cherryPickRevertChangeId, + timestamp, + revertInput.workInProgress)); bu.addOp(changeNotes.getChange().getId(), new PostRevertedMessageOp(generatedChangeId)); bu.addOp( cherryPickRevertChangeId, @@ -549,16 +557,19 @@ public class RevertSubmission private final ObjectId computedChangeId; private final Change.Id cherryPickRevertChangeId; private final Timestamp timestamp; + private final boolean workInProgress; CreateCherryPickOp( ObjectId revCommitId, ObjectId computedChangeId, Change.Id cherryPickRevertChangeId, - Timestamp timestamp) { + Timestamp timestamp, + Boolean workInProgress) { this.revCommitId = revCommitId; this.computedChangeId = computedChangeId; this.cherryPickRevertChangeId = cherryPickRevertChangeId; this.timestamp = timestamp; + this.workInProgress = workInProgress; } @Override @@ -575,7 +586,8 @@ public class RevertSubmission timestamp, change.getId(), computedChangeId, - cherryPickRevertChangeId); + cherryPickRevertChangeId, + workInProgress); // save the commit as base for next cherryPick of that branch cherryPickInput.base = changeNotesFactory diff --git a/javatests/com/google/gerrit/acceptance/api/change/RevertIT.java b/javatests/com/google/gerrit/acceptance/api/change/RevertIT.java index 69278b470b..a3a089fd27 100644 --- a/javatests/com/google/gerrit/acceptance/api/change/RevertIT.java +++ b/javatests/com/google/gerrit/acceptance/api/change/RevertIT.java @@ -249,6 +249,17 @@ public class RevertIT extends AbstractDaemonTest { assertThat(revertChange.revertOf).isEqualTo(gApi.changes().id(r.getChangeId()).get()._number); } + @Test + public void revertChangeWithWip() throws Exception { + PushOneCommit.Result r = createChange(); + gApi.changes().id(r.getChangeId()).revision(r.getCommit().name()).review(ReviewInput.approve()); + gApi.changes().id(r.getChangeId()).revision(r.getCommit().name()).submit(); + + RevertInput in = createWipRevertInput(); + ChangeInfo revertChange = gApi.changes().id(r.getChangeId()).revert(in).get(); + assertThat(revertChange.workInProgress).isTrue(); + } + @Test public void revertWithDefaultTopic() throws Exception { PushOneCommit.Result result = createChange(); @@ -320,6 +331,18 @@ public class RevertIT extends AbstractDaemonTest { assertThat(sender.getMessages(r.getChangeId(), "revert")).hasSize(1); } + @Test + public void revertNotificationsSupressedOnWip() throws Exception { + PushOneCommit.Result r = createChange(); + gApi.changes().id(r.getChangeId()).addReviewer(user.email()); + gApi.changes().id(r.getChangeId()).revision(r.getCommit().name()).review(ReviewInput.approve()); + gApi.changes().id(r.getChangeId()).revision(r.getCommit().name()).submit(); + + sender.clear(); + gApi.changes().id(r.getChangeId()).revert(createWipRevertInput()).get(); + assertThat(sender.getMessages()).isEmpty(); + } + @Test public void suppressRevertNotifications() throws Exception { PushOneCommit.Result r = createChange(); @@ -658,6 +681,49 @@ public class RevertIT extends AbstractDaemonTest { assertThat(sender.getMessages(secondResult, "revert")).hasSize(1); } + @Test + public void revertSubmissionWipNotificationsAreSupressed() throws Exception { + String changeId1 = createChange("first change", "a.txt", "message").getChangeId(); + approve(changeId1); + gApi.changes().id(changeId1).addReviewer(user.email()); + String changeId2 = createChange("second change", "b.txt", "other").getChangeId(); + approve(changeId2); + gApi.changes().id(changeId2).addReviewer(user.email()); + + gApi.changes().id(changeId2).current().submit(); + + sender.clear(); + + RevertInput revertInput = createWipRevertInput(); + // Setting the Notifications to ALL will be overridden because the WIP flag overrides the + // notifications to OWNER + revertInput.notify = NotifyHandling.ALL; + gApi.changes().id(changeId2).revertSubmission(revertInput); + + assertThat(sender.getMessages()).isEmpty(); + } + + @Test + public void revertSubmissionWipMarksAllChangesAsWip() throws Exception { + String changeId1 = createChange("first change", "a.txt", "message").getChangeId(); + approve(changeId1); + gApi.changes().id(changeId1).addReviewer(user.email()); + String changeId2 = createChange("second change", "b.txt", "other").getChangeId(); + approve(changeId2); + gApi.changes().id(changeId2).addReviewer(user.email()); + + gApi.changes().id(changeId2).current().submit(); + + sender.clear(); + + RevertInput revertInput = createWipRevertInput(); + RevertSubmissionInfo revertSubmissionInfo = + gApi.changes().id(changeId2).revertSubmission(revertInput); + + assertThat(revertSubmissionInfo.revertChanges.stream().allMatch(r -> r.workInProgress)) + .isTrue(); + } + @Test public void revertSubmissionIdenticalTreeIsAllowed() throws Exception { String unrelatedChange = createChange("change1", "a.txt", "message").getChangeId(); @@ -1294,4 +1360,10 @@ public class RevertIT extends AbstractDaemonTest { } return results; } + + private RevertInput createWipRevertInput() { + RevertInput input = new RevertInput(); + input.workInProgress = true; + return input; + } }