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

Bot actions don't update the attention set, except if they apply a
negative vote.

Bots can also apply (or more correctly re-apply) negative votes on
merged changes, which currently leads to an unwanted attention set
update (due to the negative bot vote the owner is added back to the
attention set). This is unwanted since this change is already merged and
there is nothing to do for the owner.

If a change is merged we do not allow downgrades of votes, including
applying negative votes if a vote was not present yet (as this would be
a downgrade from 0 to the negative vote). However if the negative vote
is already present on submit (e.g. on an old patch set or a non-blocking
label), the same negative vote can be re-applied on the merged change
(since it's not changing the vote, it's not a downgrade).

Release-Notes: Don't add owner to attention set on merged change if bot re-applies a negative vote
Signed-off-by: Edwin Kempin <ekempin@google.com>
Change-Id: I7e9b8ea6bb4fd9a05d06814f85900325635da0d5

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 ea52690..10bf335 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(