opendev/gerrit
Code Issues Proposed changes

Removed CCs also get removed from the attention set

Browse Source 
Until now, only removed reviewers got removed from the attention set.
This change ensures removed CC also get removed.

Change-Id: I98da4f78cd22ee447f95290e1578180e2069b7aa
This commit is contained in:
Gal Paikin
2020-09-07 18:17:43 +03:00
parent a99bdff1b7
commit fba9c227ba
2 changed files with 36 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files

10
java/com/google/gerrit/server/notedb/ChangeUpdate.java
View File

@@ -787,12 +787,14 @@ public class ChangeUpdate extends AbstractChangeUpdate {
AttentionSetUpdate.createForWrite(
reviewer.getKey(), AttentionSetUpdate.Operation.ADD, "Reviewer was added"));
}
// Treat both REMOVED and CC as "removed reviewers".
if (!reviewer.getValue().equals(ReviewerStateInternal.REVIEWER)
&& currentReviewers.contains(reviewer.getKey())) {
boolean reviewerRemoved =
!reviewer.getValue().equals(ReviewerStateInternal.REVIEWER)
&& currentReviewers.contains(reviewer.getKey());
boolean ccRemoved = reviewer.getValue().equals(ReviewerStateInternal.REMOVED);
if (reviewerRemoved || ccRemoved) {
updates.add(
AttentionSetUpdate.createForWrite(
reviewer.getKey(), AttentionSetUpdate.Operation.REMOVE, "Reviewer was removed"));
reviewer.getKey(), AttentionSetUpdate.Operation.REMOVE, "Reviewer/Cc was removed"));
}
}
addToPlannedAttentionSetUpdates(updates);

35
javatests/com/google/gerrit/acceptance/rest/change/AttentionSetIT.java
View File

@@ -318,7 +318,32 @@ public class AttentionSetIT extends AbstractDaemonTest {
attentionSet = Iterables.getOnlyElement(r.getChange().attentionSet());
assertThat(attentionSet).hasAccountIdThat().isEqualTo(user.id());
assertThat(attentionSet).hasOperationThat().isEqualTo(AttentionSetUpdate.Operation.REMOVE);
assertThat(attentionSet).hasReasonThat().isEqualTo("Reviewer was removed");
assertThat(attentionSet).hasReasonThat().isEqualTo("Reviewer/Cc was removed");
}
@Test
public void removedCcRemovedFromAttentionSet() throws Exception {
PushOneCommit.Result r = createChange();
// Add cc
AddReviewerInput input = new AddReviewerInput();
input.reviewer = user.email();
input.state = ReviewerState.CC;
change(r).addReviewer(input);
// Add them to the attention set
AttentionSetInput attentionSetInput = new AttentionSetInput();
attentionSetInput.user = user.email();
attentionSetInput.reason = "reason";
change(r).addToAttentionSet(attentionSetInput);
// Remove them from cc
change(r).reviewer(user.email()).remove();
AttentionSetUpdate attentionSet = Iterables.getOnlyElement(r.getChange().attentionSet());
assertThat(attentionSet).hasAccountIdThat().isEqualTo(user.id());
assertThat(attentionSet).hasOperationThat().isEqualTo(AttentionSetUpdate.Operation.REMOVE);
assertThat(attentionSet).hasReasonThat().isEqualTo("Reviewer/Cc was removed");
}
@Test
@@ -337,7 +362,7 @@ public class AttentionSetIT extends AbstractDaemonTest {
attentionSet = Iterables.getOnlyElement(r.getChange().attentionSet());
assertThat(attentionSet).hasAccountIdThat().isEqualTo(user.id());
assertThat(attentionSet).hasOperationThat().isEqualTo(AttentionSetUpdate.Operation.REMOVE);
assertThat(attentionSet).hasReasonThat().isEqualTo("Reviewer was removed");
assertThat(attentionSet).hasReasonThat().isEqualTo("Reviewer/Cc was removed");
}
@Test
@@ -406,7 +431,7 @@ public class AttentionSetIT extends AbstractDaemonTest {
AttentionSetUpdate attentionSet = Iterables.getOnlyElement(r.getChange().attentionSet());
assertThat(attentionSet).hasAccountIdThat().isEqualTo(user.id());
assertThat(attentionSet).hasOperationThat().isEqualTo(AttentionSetUpdate.Operation.REMOVE);
assertThat(attentionSet).hasReasonThat().isEqualTo("Reviewer was removed");
assertThat(attentionSet).hasReasonThat().isEqualTo("Reviewer/Cc was removed");
}
@Test
@@ -457,7 +482,7 @@ public class AttentionSetIT extends AbstractDaemonTest {
Iterables.getOnlyElement(getAttentionSetUpdatesForUser(r, user));
assertThat(attentionSet).hasAccountIdThat().isEqualTo(user.id());
assertThat(attentionSet).hasOperationThat().isEqualTo(AttentionSetUpdate.Operation.REMOVE);
assertThat(attentionSet).hasReasonThat().isEqualTo("Reviewer was removed");
assertThat(attentionSet).hasReasonThat().isEqualTo("Reviewer/Cc was removed");
}
@Test
@@ -993,7 +1018,7 @@ public class AttentionSetIT extends AbstractDaemonTest {
Iterables.getOnlyElement(getAttentionSetUpdatesForUser(r, user));
assertThat(attentionSet).hasAccountIdThat().isEqualTo(user.id());
assertThat(attentionSet).hasOperationThat().isEqualTo(AttentionSetUpdate.Operation.REMOVE);
assertThat(attentionSet).hasReasonThat().isEqualTo("Reviewer was removed");
assertThat(attentionSet).hasReasonThat().isEqualTo("Reviewer/Cc was removed");
}
@Test