Support to set cherry-pick notification options This change adds support to set notification options for the existing cherry-pick change/commit REST endpoints. Change-Id: I3b419a4d68ea186092163cddd354b5bf4de0daeb
diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt index b7c50f4..75d31d2c 100644 --- a/Documentation/rest-api-changes.txt +++ b/Documentation/rest-api-changes.txt
@@ -5689,6 +5689,14 @@ |`destination` ||Destination branch |`parent` |optional, defaults to 1| Number of the parent relative to which the cherry-pick should be considered. +|`notify` |optional| +Notify handling that defines to whom email notifications should be sent +after the cherry-pick. + +Allowed values are `NONE`, `OWNER`, `OWNER_REVIEWERS` and `ALL`. + +If not set, the default is `NONE`. +|`notify_details` |optional| +Additional information about whom to notify about the update as a map +of recipient type to link:#notify-info[NotifyInfo] entity. |=========================== [[comment-info]]
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/revision/RevisionIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/revision/RevisionIT.java index a4a2cbc..dd44cb9a6 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/revision/RevisionIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/revision/RevisionIT.java
@@ -38,12 +38,16 @@ import com.google.gerrit.acceptance.AbstractDaemonTest; import com.google.gerrit.acceptance.PushOneCommit; import com.google.gerrit.acceptance.RestResponse; +import com.google.gerrit.acceptance.TestAccount; import com.google.gerrit.acceptance.TestProjectInput; import com.google.gerrit.common.data.Permission; import com.google.gerrit.extensions.api.changes.ChangeApi; import com.google.gerrit.extensions.api.changes.CherryPickInput; import com.google.gerrit.extensions.api.changes.DraftApi; import com.google.gerrit.extensions.api.changes.DraftInput; +import com.google.gerrit.extensions.api.changes.NotifyHandling; +import com.google.gerrit.extensions.api.changes.NotifyInfo; +import com.google.gerrit.extensions.api.changes.RecipientType; import com.google.gerrit.extensions.api.changes.ReviewInput; import com.google.gerrit.extensions.api.changes.ReviewInput.CommentInput; import com.google.gerrit.extensions.api.changes.RevisionApi; @@ -70,6 +74,7 @@ import com.google.gerrit.extensions.restapi.ResourceNotFoundException; import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.Branch; +import com.google.gerrit.reviewdb.client.Branch.NameKey; import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.PatchSetApproval; import com.google.gerrit.server.change.GetRevisionActions; @@ -623,6 +628,47 @@ } @Test + public void cherryPickNotify() throws Exception { + createBranch(new NameKey(project, "branch-1")); + createBranch(new NameKey(project, "branch-2")); + createBranch(new NameKey(project, "branch-3")); + + // Creates a change for 'admin'. + PushOneCommit.Result result = createChange(); + String changeId = project.get() + "~master~" + result.getChangeId(); + + // 'user' cherry-picks the change to a new branch, the source change's author/committer('admin') + // will be added as a reviewer of the newly created change. + setApiUser(user); + CherryPickInput input = new CherryPickInput(); + input.message = "it goes to a new branch"; + + // Enable the notification. 'admin' as a reviewer should be notified. + input.destination = "branch-1"; + input.notify = NotifyHandling.ALL; + sender.clear(); + gApi.changes().id(changeId).current().cherryPick(input); + assertNotifyCc(admin); + + // Disable the notification. 'admin' as a reviewer should not be notified any more. + input.destination = "branch-2"; + input.notify = NotifyHandling.NONE; + sender.clear(); + gApi.changes().id(changeId).current().cherryPick(input); + assertThat(sender.getMessages()).hasSize(0); + + // Disable the notification. The user provided in the 'notifyDetails' should still be notified. + TestAccount userToNotify = accounts.user2(); + input.destination = "branch-3"; + input.notify = NotifyHandling.NONE; + input.notifyDetails = + ImmutableMap.of(RecipientType.TO, new NotifyInfo(ImmutableList.of(userToNotify.email))); + sender.clear(); + gApi.changes().id(changeId).current().cherryPick(input); + assertNotifyTo(userToNotify); + } + + @Test public void canRebase() throws Exception { PushOneCommit push = pushFactory.create(db, admin.getIdent(), testRepo); PushOneCommit.Result r1 = push.to("refs/for/master");
diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/CherryPickInput.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/CherryPickInput.java index 2e1bb13..3ac3601 100644 --- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/CherryPickInput.java +++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/CherryPickInput.java
@@ -14,8 +14,13 @@ package com.google.gerrit.extensions.api.changes; +import java.util.Map; + public class CherryPickInput { public String message; public String destination; public Integer parent; + + public NotifyHandling notify = NotifyHandling.NONE; + public Map<RecipientType, NotifyInfo> notifyDetails; }
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPick.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPick.java index 1a1f8cc..35aa4ad 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPick.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPick.java
@@ -66,7 +66,7 @@ BatchUpdate.Factory updateFactory, RevisionResource revision, CherryPickInput input) throws OrmException, IOException, UpdateException, RestApiException { final ChangeControl control = revision.getControl(); - int parent = input.parent == null ? 1 : input.parent; + input.parent = input.parent == null ? 1 : input.parent; if (input.message == null || input.message.trim().isEmpty()) { throw new BadRequestException("message must be non-empty"); @@ -100,10 +100,9 @@ updateFactory, revision.getChange(), revision.getPatchSet(), - input.message, + input, refName, - refControl, - parent); + refControl); return json.noOptions().format(revision.getProject(), cherryPickedChangeId); } catch (InvalidChangeOperationException e) { throw new BadRequestException(e.getMessage());
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPickChange.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPickChange.java index a540298..7c0a7be 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPickChange.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPickChange.java
@@ -18,7 +18,8 @@ import com.google.gerrit.common.FooterConstants; import com.google.gerrit.common.Nullable; import com.google.gerrit.common.TimeUtil; -import com.google.gerrit.extensions.api.changes.NotifyHandling; +import com.google.gerrit.extensions.api.changes.CherryPickInput; +import com.google.gerrit.extensions.restapi.BadRequestException; import com.google.gerrit.extensions.restapi.MergeConflictException; import com.google.gerrit.extensions.restapi.RestApiException; import com.google.gerrit.reviewdb.client.Branch; @@ -80,6 +81,7 @@ private final MergeUtil.Factory mergeUtilFactory; private final ChangeMessagesUtil changeMessagesUtil; private final PatchSetUtil psUtil; + private final NotifyUtil notifyUtil; @Inject CherryPickChange( @@ -93,7 +95,8 @@ PatchSetInserter.Factory patchSetInserterFactory, MergeUtil.Factory mergeUtilFactory, ChangeMessagesUtil changeMessagesUtil, - PatchSetUtil psUtil) { + PatchSetUtil psUtil, + NotifyUtil notifyUtil) { this.db = db; this.seq = seq; this.queryProvider = queryProvider; @@ -105,16 +108,16 @@ this.mergeUtilFactory = mergeUtilFactory; this.changeMessagesUtil = changeMessagesUtil; this.psUtil = psUtil; + this.notifyUtil = notifyUtil; } public Change.Id cherryPick( BatchUpdate.Factory batchUpdateFactory, Change change, PatchSet patch, - String message, + CherryPickInput input, String ref, - RefControl refControl, - int parent) + RefControl refControl) throws OrmException, IOException, InvalidChangeOperationException, IntegrationException, UpdateException, RestApiException { return cherryPick( @@ -125,10 +128,9 @@ change.getTopic(), change.getProject(), ObjectId.fromString(patch.getRevision().get()), - message, + input, ref, - refControl, - parent); + refControl); } public Change.Id cherryPick( @@ -139,10 +141,9 @@ @Nullable String sourceChangeTopic, Project.NameKey project, ObjectId sourceCommit, - String message, + CherryPickInput input, String targetRef, - RefControl targetRefControl, - int parent) + RefControl targetRefControl) throws OrmException, IOException, InvalidChangeOperationException, IntegrationException, UpdateException, RestApiException { @@ -170,12 +171,12 @@ CodeReviewCommit commitToCherryPick = revWalk.parseCommit(sourceCommit); - if (parent <= 0 || parent > commitToCherryPick.getParentCount()) { + if (input.parent <= 0 || input.parent > commitToCherryPick.getParentCount()) { throw new InvalidChangeOperationException( String.format( "Cherry Pick: Parent %s does not exist. Please specify a parent in" + " range [1, %s].", - parent, commitToCherryPick.getParentCount())); + input.parent, commitToCherryPick.getParentCount())); } Timestamp now = TimeUtil.nowTs(); @@ -187,8 +188,8 @@ mergeTip, commitToCherryPick.getAuthorIdent(), committerIdent, - message); - String commitMessage = ChangeIdUtil.insertId(message, computedChangeId).trim() + '\n'; + input.message); + String commitMessage = ChangeIdUtil.insertId(input.message, computedChangeId).trim() + '\n'; CodeReviewCommit cherryPickCommit; try { @@ -204,7 +205,7 @@ committerIdent, commitMessage, revWalk, - parent - 1, + input.parent - 1, false); Change.Key changeKey; @@ -234,7 +235,7 @@ // will be added as a new patch set. ChangeControl destCtl = targetRefControl.getProjectControl().controlFor(destChanges.get(0).notes()); - result = insertPatchSet(bu, git, destCtl, cherryPickCommit); + result = insertPatchSet(bu, git, destCtl, cherryPickCommit, input); } else { // Change key not found on destination branch. We can create a new // change. @@ -249,7 +250,8 @@ targetRefControl.getRefName(), newTopic, sourceBranch, - sourceCommit); + sourceCommit, + input); if (sourceChangeId != null && sourcePatchId != null) { bu.addOp( @@ -268,20 +270,23 @@ } private Change.Id insertPatchSet( - BatchUpdate bu, Repository git, ChangeControl destCtl, CodeReviewCommit cherryPickCommit) - throws IOException, OrmException { + BatchUpdate bu, + Repository git, + ChangeControl destCtl, + CodeReviewCommit cherryPickCommit, + CherryPickInput input) + throws IOException, OrmException, BadRequestException { Change destChange = destCtl.getChange(); PatchSet.Id psId = ChangeUtil.nextPatchSetId(git, destChange.currentPatchSetId()); - PatchSetInserter inserter = patchSetInserterFactory.create(destCtl, psId, cherryPickCommit); - PatchSet.Id newPatchSetId = inserter.getPatchSetId(); PatchSet current = psUtil.current(db.get(), destCtl.getNotes()); - bu.addOp( - destChange.getId(), - inserter - .setMessage("Uploaded patch set " + newPatchSetId.get() + ".") - .setDraft(current.isDraft()) - .setNotify(NotifyHandling.NONE)); + PatchSetInserter inserter = patchSetInserterFactory.create(destCtl, psId, cherryPickCommit); + inserter + .setMessage("Uploaded patch set " + inserter.getPatchSetId().get() + ".") + .setDraft(current.isDraft()) + .setNotify(input.notify) + .setAccountsToNotify(notifyUtil.resolveAccounts(input.notifyDetails)); + bu.addOp(destChange.getId(), inserter); return destChange.getId(); } @@ -291,12 +296,15 @@ String refName, String topic, Branch.NameKey sourceBranch, - ObjectId sourceCommit) - throws OrmException, IOException { + ObjectId sourceCommit, + CherryPickInput input) + throws OrmException, IOException, BadRequestException { Change.Id changeId = new Change.Id(seq.nextChangeId()); ChangeInserter ins = changeInserterFactory.create(changeId, cherryPickCommit, refName).setTopic(topic); - ins.setMessage(messageForDestinationChange(ins.getPatchSetId(), sourceBranch, sourceCommit)); + ins.setMessage(messageForDestinationChange(ins.getPatchSetId(), sourceBranch, sourceCommit)) + .setNotify(input.notify) + .setAccountsToNotify(notifyUtil.resolveAccounts(input.notifyDetails)); bu.insertChange(ins); return changeId; }
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPickCommit.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPickCommit.java index 4c027dd..b44a8b7 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPickCommit.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPickCommit.java
@@ -59,9 +59,11 @@ public ChangeInfo applyImpl( BatchUpdate.Factory updateFactory, CommitResource rsrc, CherryPickInput input) throws OrmException, IOException, UpdateException, RestApiException { + RevCommit commit = rsrc.getCommit(); String message = Strings.nullToEmpty(input.message).trim(); + input.message = message.isEmpty() ? commit.getFullMessage() : message; String destination = Strings.nullToEmpty(input.destination).trim(); - int parent = input.parent == null ? 1 : input.parent; + input.parent = input.parent == null ? 1 : input.parent; if (destination.isEmpty()) { throw new BadRequestException("destination must be non-empty"); @@ -73,7 +75,6 @@ throw new AuthException(capable.getMessage()); } - RevCommit commit = rsrc.getCommit(); String refName = RefNames.fullName(destination); RefControl refControl = projectControl.controlForRef(refName); if (!refControl.canUpload()) { @@ -84,17 +85,7 @@ try { Change.Id cherryPickedChangeId = cherryPickChange.cherryPick( - updateFactory, - null, - null, - null, - null, - project, - commit, - message.isEmpty() ? commit.getFullMessage() : message, - refName, - refControl, - parent); + updateFactory, null, null, null, null, project, commit, input, refName, refControl); return json.noOptions().format(project, cherryPickedChangeId); } catch (InvalidChangeOperationException e) { throw new BadRequestException(e.getMessage());