Merge "Support notify option for delete vote REST endpoint"

Documentation/rest-api-changes.txt

@@ -2400,9 +2400,27 @@ The entries in the map are sorted by label name.
 Deletes a single vote from a change. Note, that even when the last vote of
 a reviewer is removed the reviewer itself is still listed on the change.
 
+Options can be provided in the request body as a
+link:#delete-vote-input[DeleteVoteInput] entity.
+
 .Request
 ----
   DELETE /changes/myProject~master~I8473b95934b5732ac55d26311a706c9c2bde9940/reviewers/John%20Doe/votes/Code-Review HTTP/1.0
+  POST /changes/myProject~master~I8473b95934b5732ac55d26311a706c9c2bde9940/reviewers/John%20Doe/votes/Code-Review/delete HTTP/1.0
+----
+
+Please note that some proxies prohibit request bodies for DELETE
+requests. In this case, if you want to specify options, use a POST
+request:
+
+.Request
+----
+  POST /changes/myProject~master~I8473b95934b5732ac55d26311a706c9c2bde9940/reviewers/John%20Doe/votes/Code-Review/delete HTTP/1.0
+  Content-Type: application/json; charset=UTF-8
+
+  {
+    "notify": "NONE"
+  }
 ----
 
 .Response
@@ -4384,6 +4402,24 @@ Links to the commit in external sites as a list of
 link:#web-link-info[WebLinkInfo] entities.
 |===========================
 
+[[delete-vote-input]]
+=== DeleteVoteInput
+The `DeleteVoteInput` entity contains options for the deletion of a
+vote.
+
+[options="header",cols="1,^1,5"]
+|=======================
+|Field Name||Description
+|`label`   |optional|
+The label for which the vote should be deleted. +
+If set, must match the label in the URL.
+|`notify`  |optional|
+Notify handling that defines to whom email notifications should be sent
+after the vote is deleted. +
+Allowed values are `NONE`, `OWNER`, `OWNER_REVIEWERS` and `ALL`. +
+If not set, the default is `ALL`.
+|=======================
+
 [[diff-content]]
 === DiffContent
 The `DiffContent` entity contains information about the content differences

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java

@@ -40,9 +40,11 @@ import com.google.gerrit.acceptance.TestProjectInput;
 import com.google.gerrit.common.data.LabelType;
 import com.google.gerrit.common.data.Permission;
 import com.google.gerrit.extensions.api.changes.AddReviewerInput;
+import com.google.gerrit.extensions.api.changes.DeleteVoteInput;
 import com.google.gerrit.extensions.api.changes.RebaseInput;
 import com.google.gerrit.extensions.api.changes.ReviewInput;
 import com.google.gerrit.extensions.api.changes.RevisionApi;
+import com.google.gerrit.extensions.api.changes.ReviewInput.NotifyHandling;
 import com.google.gerrit.extensions.api.projects.BranchInput;
 import com.google.gerrit.extensions.client.ChangeStatus;
 import com.google.gerrit.extensions.client.ListChangesOption;
@@ -705,11 +707,22 @@ public class ChangeIT extends AbstractDaemonTest {
         .review(ReviewInput.recommend());
 
     setApiUser(admin);
+    sender.clear();
     gApi.changes()
         .id(r.getChangeId())
         .reviewer(user.getId().toString())
         .deleteVote("Code-Review");
 
+    List<Message> messages = sender.getMessages();
+    assertThat(messages).hasSize(1);
+    Message msg = messages.get(0);
+    assertThat(msg.rcpt()).containsExactly(user.emailAddress);
+    assertThat(msg.body()).contains(
+        admin.fullName + " has removed a vote on this change.\n");
+    assertThat(msg.body()).contains(
+        "Removed Code-Review+1 by "
+            + user.fullName + " <" + user.email + ">" + "\n");
+
     Map<String, Short> m = gApi.changes()
         .id(r.getChangeId())
         .reviewer(user.getId().toString())
@@ -753,6 +766,32 @@ public class ChangeIT extends AbstractDaemonTest {
     }
   }
 
+  @Test
+  public void deleteVoteNotifyNone() throws Exception {
+    PushOneCommit.Result r = createChange();
+    gApi.changes()
+        .id(r.getChangeId())
+        .revision(r.getCommit().name())
+        .review(ReviewInput.approve());
+
+    setApiUser(user);
+    gApi.changes()
+        .id(r.getChangeId())
+        .revision(r.getCommit().name())
+        .review(ReviewInput.recommend());
+
+    setApiUser(admin);
+    sender.clear();
+    DeleteVoteInput in = new DeleteVoteInput();
+    in.label = "Code-Review";
+    in.notify = NotifyHandling.NONE;
+    gApi.changes()
+        .id(r.getChangeId())
+        .reviewer(user.getId().toString())
+        .deleteVote(in);
+    assertThat(sender.getMessages()).hasSize(0);
+  }
+
   @Test
   public void deleteVoteNotPermitted() throws Exception {
     PushOneCommit.Result r = createChange();

gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/DeleteVoteInput.java

@@ -0,0 +1,27 @@
+// Copyright (C) 2016 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.extensions.api.changes;
+
+import com.google.gerrit.extensions.api.changes.ReviewInput.NotifyHandling;
+import com.google.gerrit.extensions.restapi.DefaultInput;
+
+/** Input passed to {@code DELETE /changes/[id]/reviewers/[id]/votes/[label]}. */
+public class DeleteVoteInput {
+  @DefaultInput
+  public String label;
+
+  /** Who to send email notifications to after vote is deleted. */
+  public NotifyHandling notify = NotifyHandling.ALL;
+}

gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/ReviewerApi.java

@@ -23,6 +23,7 @@ public interface ReviewerApi {
 
   Map<String, Short> votes() throws RestApiException;
   void deleteVote(String label) throws RestApiException;
+  void deleteVote(DeleteVoteInput input) throws RestApiException;
   void remove() throws RestApiException;
 
   /**
@@ -40,6 +41,11 @@ public interface ReviewerApi {
       throw new NotImplementedException();
     }
 
+    @Override
+    public void deleteVote(DeleteVoteInput input) throws RestApiException {
+      throw new NotImplementedException();
+    }
+
     @Override
     public void remove() throws RestApiException {
       throw new NotImplementedException();

gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ReviewerApiImpl.java

@@ -14,6 +14,7 @@
 
 package com.google.gerrit.server.api.changes;
 
+import com.google.gerrit.extensions.api.changes.DeleteVoteInput;
 import com.google.gerrit.extensions.api.changes.ReviewerApi;
 import com.google.gerrit.extensions.restapi.RestApiException;
 import com.google.gerrit.server.change.DeleteReviewer;
@@ -67,6 +68,15 @@ public class ReviewerApiImpl implements ReviewerApi {
     }
   }
 
+  @Override
+  public void deleteVote(DeleteVoteInput input) throws RestApiException {
+    try {
+      deleteVote.apply(new VoteResource(reviewer, input.label), input);
+    } catch (UpdateException e) {
+      throw new RestApiException("Cannot delete vote", e);
+    }
+  }
+
   @Override
   public void remove() throws RestApiException {
     try {

gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteVote.java

@@ -18,7 +18,10 @@ import com.google.gerrit.common.ChangeHooks;
 import com.google.gerrit.common.TimeUtil;
 import com.google.gerrit.common.data.LabelType;
 import com.google.gerrit.common.data.LabelTypes;
+import com.google.gerrit.extensions.api.changes.DeleteVoteInput;
+import com.google.gerrit.extensions.api.changes.ReviewInput.NotifyHandling;
 import com.google.gerrit.extensions.restapi.AuthException;
+import com.google.gerrit.extensions.restapi.BadRequestException;
 import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
 import com.google.gerrit.extensions.restapi.Response;
 import com.google.gerrit.extensions.restapi.RestApiException;
@@ -34,7 +37,6 @@ import com.google.gerrit.server.ChangeMessagesUtil;
 import com.google.gerrit.server.ChangeUtil;
 import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.PatchSetUtil;
-import com.google.gerrit.server.change.DeleteVote.Input;
 import com.google.gerrit.server.git.BatchUpdate;
 import com.google.gerrit.server.git.BatchUpdate.ChangeContext;
 import com.google.gerrit.server.git.BatchUpdate.Context;
@@ -55,12 +57,10 @@ import java.util.HashMap;
 import java.util.Map;
 
 @Singleton
-public class DeleteVote implements RestModifyView<VoteResource, Input> {
+public class DeleteVote
+    implements RestModifyView<VoteResource, DeleteVoteInput> {
   private static final Logger log = LoggerFactory.getLogger(DeleteVote.class);
 
-  public static class Input {
-  }
-
   private final Provider<ReviewDb> db;
   private final BatchUpdate.Factory batchUpdateFactory;
   private final ApprovalsUtil approvalsUtil;
@@ -90,14 +90,23 @@ public class DeleteVote implements RestModifyView<VoteResource, Input> {
   }
 
   @Override
-  public Response<?> apply(VoteResource rsrc, Input input)
+  public Response<?> apply(VoteResource rsrc, DeleteVoteInput input)
       throws RestApiException, UpdateException {
+    if (input == null) {
+      input = new DeleteVoteInput();
+    }
+    if (input.label != null && !rsrc.getLabel().equals(input.label)) {
+      throw new BadRequestException("label must match URL");
+    }
+    if (input.notify == null) {
+      input.notify = NotifyHandling.ALL;
+    }
     ReviewerResource r = rsrc.getReviewer();
     Change change = r.getChange();
     try (BatchUpdate bu = batchUpdateFactory.create(db.get(),
           change.getProject(), r.getControl().getUser(), TimeUtil.nowTs())) {
       bu.addOp(change.getId(),
-          new Op(r.getReviewerUser().getAccountId(), rsrc.getLabel()));
+          new Op(r.getReviewerUser().getAccountId(), rsrc.getLabel(), input));
       bu.execute();
     }
 
@@ -107,15 +116,17 @@ public class DeleteVote implements RestModifyView<VoteResource, Input> {
   private class Op extends BatchUpdate.Op {
     private final Account.Id accountId;
     private final String label;
+    private final DeleteVoteInput input;
     private ChangeMessage changeMessage;
     private Change change;
     private PatchSet ps;
     private Map<String, Short> newApprovals = new HashMap<>();
     private Map<String, Short> oldApprovals = new HashMap<>();
 
-    private Op(Account.Id accountId, String label) {
+    private Op(Account.Id accountId, String label, DeleteVoteInput input) {
       this.accountId = accountId;
       this.label = label;
+      this.input = input;
     }
 
     @Override
@@ -191,14 +202,17 @@ public class DeleteVote implements RestModifyView<VoteResource, Input> {
       }
 
       IdentifiedUser user = ctx.getUser().asIdentifiedUser();
-      try {
+      if (input.notify.compareTo(NotifyHandling.NONE) > 0) {
-        ReplyToChangeSender cm = deleteVoteSenderFactory.create(
+        try {
-            ctx.getProject(), change.getId());
+          ReplyToChangeSender cm = deleteVoteSenderFactory.create(
-        cm.setFrom(user.getAccountId());
+              ctx.getProject(), change.getId());
-        cm.setChangeMessage(changeMessage);
+          cm.setFrom(user.getAccountId());
-        cm.send();
+          cm.setChangeMessage(changeMessage);
-      } catch (Exception e) {
+          cm.setNotify(input.notify);
-        log.error("Cannot email update for change " + change.getId(), e);
+          cm.send();
+        } catch (Exception e) {
+          log.error("Cannot email update for change " + change.getId(), e);
+        }
       }
 
       try {

gerrit-server/src/main/java/com/google/gerrit/server/change/Module.java

@@ -80,6 +80,7 @@ public class Module extends RestApiModule {
     delete(REVIEWER_KIND).to(DeleteReviewer.class);
     child(REVIEWER_KIND, "votes").to(Votes.class);
     delete(VOTE_KIND).to(DeleteVote.class);
+    post(VOTE_KIND, "delete").to(DeleteVote.class);
 
     child(CHANGE_KIND, "revisions").to(Revisions.class);
     get(REVISION_KIND, "actions").to(GetRevisionActions.class);