Add a user to the attention set if their vote is removed
This is only applicable if someone else removed their vote. This change
is useful since if the user's vote was removed, it means they probably
need to take some action (unless they are no longer reviewers which
means they won't be in the attention set).
We think this rule usually will make sense, but it's likely that in some
cases the attention set will just need to manually be adjusted to remove
the user that was added from this logic.
Change-Id: I76300115f073b91701a937edf58dd81021d76e68
diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt
index ccffa44..d68489d 100644
--- a/Documentation/rest-api-changes.txt
+++ b/Documentation/rest-api-changes.txt
@@ -3513,6 +3513,9 @@
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.
+If another user removed a user's vote, the user with the deleted vote will be
+added to the attention set.
+
.Request
----
DELETE /changes/myProject~master~I8473b95934b5732ac55d26311a706c9c2bde9940/reviewers/John%20Doe/votes/Code-Review HTTP/1.0
@@ -5877,6 +5880,9 @@
Note, that even when the last vote of a reviewer is removed the reviewer itself
is still listed on the change.
+If another user removed a user's vote, the user with the deleted vote will be
+added to the attention set.
+
.Request
----
DELETE /changes/myProject~master~I8473b95934b5732ac55d26311a706c9c2bde9940/revisions/674ac754f91e64a0efb8087e59a176484bd534d1/reviewers/John%20Doe/votes/Code-Review HTTP/1.0
@@ -6055,6 +6061,7 @@
* The change is marked ready for review.
* As an owner/uploader, when someone replies on your change.
* As a reviewer, when the owner/uploader replies.
+* When the user's vote is deleted by another user.
Users are removed from the attention set if one the following apply:
diff --git a/Documentation/user-attention-set.txt b/Documentation/user-attention-set.txt
index 4697afc..ea927c7 100644
--- a/Documentation/user-attention-set.txt
+++ b/Documentation/user-attention-set.txt
@@ -47,6 +47,8 @@
attention set.
* For merged and abandoned changes the owner is added only when a human creates
an unresolved comment.
+* If another user removed a user's vote, the user with the deleted vote will be
+ added to the attention set.
* Only owner, uploader, reviewers and ccs can be in the attention set.
* The rules for service accounts are different, see link:#bots[Bots].
diff --git a/java/com/google/gerrit/server/restapi/change/DeleteVote.java b/java/com/google/gerrit/server/restapi/change/DeleteVote.java
index 4b813df..cc73e9a 100644
--- a/java/com/google/gerrit/server/restapi/change/DeleteVote.java
+++ b/java/com/google/gerrit/server/restapi/change/DeleteVote.java
@@ -36,9 +36,11 @@
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.ChangeMessagesUtil;
+import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.PatchSetUtil;
import com.google.gerrit.server.account.AccountState;
+import com.google.gerrit.server.change.AddToAttentionSetOp;
import com.google.gerrit.server.change.NotifyResolver;
import com.google.gerrit.server.change.ReviewerResource;
import com.google.gerrit.server.change.VoteResource;
@@ -58,6 +60,7 @@
import com.google.gerrit.server.util.LabelVote;
import com.google.gerrit.server.util.time.TimeUtil;
import com.google.inject.Inject;
+import com.google.inject.Provider;
import com.google.inject.Singleton;
import java.io.IOException;
import java.util.HashMap;
@@ -79,6 +82,8 @@
private final RemoveReviewerControl removeReviewerControl;
private final ProjectCache projectCache;
private final MessageIdGenerator messageIdGenerator;
+ private final AddToAttentionSetOp.Factory attentionSetOpfactory;
+ private final Provider<CurrentUser> currentUserProvider;
@Inject
DeleteVote(
@@ -92,7 +97,9 @@
NotifyResolver notifyResolver,
RemoveReviewerControl removeReviewerControl,
ProjectCache projectCache,
- MessageIdGenerator messageIdGenerator) {
+ MessageIdGenerator messageIdGenerator,
+ AddToAttentionSetOp.Factory attentionSetOpFactory,
+ Provider<CurrentUser> currentUserProvider) {
this.updateFactory = updateFactory;
this.approvalsUtil = approvalsUtil;
this.psUtil = psUtil;
@@ -104,6 +111,8 @@
this.removeReviewerControl = removeReviewerControl;
this.projectCache = projectCache;
this.messageIdGenerator = messageIdGenerator;
+ this.attentionSetOpfactory = attentionSetOpFactory;
+ this.currentUserProvider = currentUserProvider;
}
@Override
@@ -140,6 +149,14 @@
r.getReviewerUser().state(),
rsrc.getLabel(),
input));
+ if (!r.getReviewerUser().getAccountId().equals(currentUserProvider.get().getAccountId())) {
+ bu.addOp(
+ change.getId(),
+ attentionSetOpfactory.create(
+ r.getReviewerUser().getAccountId(),
+ /* reason= */ "Their vote was deleted",
+ /* notify= */ false));
+ }
bu.execute();
}
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/AttentionSetIT.java b/javatests/com/google/gerrit/acceptance/rest/change/AttentionSetIT.java
index d2a48be1..6555b50 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/AttentionSetIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/AttentionSetIT.java
@@ -37,6 +37,7 @@
import com.google.gerrit.entities.AttentionSetUpdate;
import com.google.gerrit.entities.AttentionSetUpdate.Operation;
import com.google.gerrit.entities.Change;
+import com.google.gerrit.entities.LabelId;
import com.google.gerrit.entities.Patch;
import com.google.gerrit.extensions.api.changes.AddReviewerInput;
import com.google.gerrit.extensions.api.changes.AttentionSetInput;
@@ -1758,6 +1759,40 @@
.isEqualTo(Operation.REMOVE);
}
+ @Test
+ public void deleteSelfVotesDoesNotAddToAttentionSet() throws Exception {
+ PushOneCommit.Result r = createChange();
+ approve(r.getChangeId());
+ gApi.changes()
+ .id(r.getChangeId())
+ .current()
+ .reviewer(admin.id().toString())
+ .deleteVote(LabelId.CODE_REVIEW);
+
+ assertThat(getAttentionSetUpdates(r.getChange().getId())).isEmpty();
+ }
+
+ @Test
+ public void deleteVotesOfOthersAddThemToAttentionSet() throws Exception {
+ PushOneCommit.Result r = createChange();
+
+ requestScopeOperations.setApiUser(user.id());
+ recommend(r.getChangeId());
+
+ requestScopeOperations.setApiUser(admin.id());
+ gApi.changes()
+ .id(r.getChangeId())
+ .current()
+ .reviewer(user.id().toString())
+ .deleteVote(LabelId.CODE_REVIEW);
+
+ AttentionSetUpdate attentionSet =
+ Iterables.getOnlyElement(getAttentionSetUpdatesForUser(r, user));
+ assertThat(attentionSet).hasAccountIdThat().isEqualTo(user.id());
+ assertThat(attentionSet).hasOperationThat().isEqualTo(AttentionSetUpdate.Operation.ADD);
+ assertThat(attentionSet).hasReasonThat().isEqualTo("Their vote was deleted");
+ }
+
private List<AttentionSetUpdate> getAttentionSetUpdatesForUser(
PushOneCommit.Result r, TestAccount account) {
return getAttentionSetUpdates(r.getChange().getId()).stream()
