Merge "Don't add owner to attention set on merged change if bot votes negatively"

java/com/google/gerrit/server/restapi/change/ReplyAttentionSetUpdates.java

diff --git a/java/com/google/gerrit/server/restapi/change/ReplyAttentionSetUpdates.java b/java/com/google/gerrit/server/restapi/change/ReplyAttentionSetUpdates.java
index 3d9d588..b262b46 100644
--- a/java/com/google/gerrit/server/restapi/change/ReplyAttentionSetUpdates.java
+++ b/java/com/google/gerrit/server/restapi/change/ReplyAttentionSetUpdates.java
@@ -259,10 +259,13 @@
 
   /**
    * Bots don't process automatic rules, the only attention set change they do is this rule: Add
-   * owner and uploader when a bot votes negatively.
+   * owner and uploader when a bot votes negatively, but only if the change is open.
    */
   private void botsWithNegativeLabelsAddOwnerAndUploader(
       BatchUpdate bu, ChangeNotes changeNotes, ReviewInput input) {
+    if (changeNotes.getChange().isClosed()) {
+      return;
+    }
     if (input.labels != null && input.labels.values().stream().anyMatch(vote -> vote < 0)) {
       Account.Id uploader = changeNotes.getCurrentPatchSet().uploader();
       Account.Id owner = changeNotes.getChange().getOwner();

javatests/com/google/gerrit/acceptance/rest/change/AttentionSetIT.java

diff --git a/javatests/com/google/gerrit/acceptance/rest/change/AttentionSetIT.java b/javatests/com/google/gerrit/acceptance/rest/change/AttentionSetIT.java
index 443e064..1d2ddc2 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/AttentionSetIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/AttentionSetIT.java
@@ -1488,6 +1488,64 @@
   }
 
   @Test
+  public void robotReviewWithNegativeLabelDoesntAddOwnerIfChangeIsMerged() throws Exception {
+    TestAccount robot =
+        accountCreator.create(
+            "robot2", "robot2@example.com", "Ro Bot", "Ro", ServiceUserClassifier.SERVICE_USERS);
+
+    PushOneCommit.Result r = createChange();
+
+    // The robot votes with Code-Review-1 on patch set 1.
+    // Without this vote the robot cannot (re-)apply a negative vote on the change after it was
+    // merged change later.
+    requestScopeOperations.setApiUser(robot.id());
+    change(r).revision(1).review(ReviewInput.dislike());
+
+    // Amend the change so that patch set 2 gets created.
+    requestScopeOperations.setApiUser(admin.id());
+    amendChange(r.getChangeId()).assertOkStatus();
+
+    // Approve the change.
+    approve(r.getChangeId());
+
+    // User adds a comment so that the admin user is added to the attention set.
+    // This has to be a comment from a user, since comments from robots do not trigger attention set
+    // updates.
+    requestScopeOperations.setApiUser(user.id());
+    ReviewInput reviewInput = new ReviewInput();
+    reviewInput.message = "A comment";
+    change(r).current().review(reviewInput);
+
+    // Verify that the admin user was added to the attention set.
+    AttentionSetUpdate attentionSet =
+        Iterables.getOnlyElement(getAttentionSetUpdatesForUser(r, admin));
+    assertThat(attentionSet).hasOperationThat().isEqualTo(AttentionSetUpdate.Operation.ADD);
+    assertThat(attentionSet).hasReasonThat().isEqualTo("Someone else replied on the change");
+
+    // Submit the change.
+    requestScopeOperations.setApiUser(admin.id());
+    change(r).current().submit();
+
+    // Verify that the attention set was cleared on submit.
+    attentionSet = Iterables.getOnlyElement(getAttentionSetUpdatesForUser(r, admin));
+    assertThat(attentionSet).hasOperationThat().isEqualTo(AttentionSetUpdate.Operation.REMOVE);
+    assertThat(attentionSet).hasReasonThat().isEqualTo("Change was submitted");
+
+    // Re-apply the negative robot vote on patch set 1.
+    // Note it's possible to a apply a negative vote on merged changes if it wasn't already present
+    // since we disallow downgrading votes on merged changes (e.g. downgrade from not present aka 0
+    // to -1 is not allowed).
+    requestScopeOperations.setApiUser(robot.id());
+    change(r).revision(1).review(ReviewInput.dislike());
+
+    // Verify that re-applying the negative robot vote on patch set 1 didn't add the admin user
+    // back to the attention set.
+    attentionSet = Iterables.getOnlyElement(getAttentionSetUpdatesForUser(r, admin));
+    assertThat(attentionSet).hasOperationThat().isEqualTo(AttentionSetUpdate.Operation.REMOVE);
+    assertThat(attentionSet).hasReasonThat().isEqualTo("Change was submitted");
+  }
+
+  @Test
   public void robotCommentDoesNotAddOwnerOnClosedChanges() throws Exception {
     TestAccount robot =
         accountCreator.create(