opendev/gerrit
Code Issues Proposed changes

ReviewerinPredicate: Only return users in state REVIEWER

Browse Source 
On NoteDb, reviewer and CC are separate so the query result should not
include users that were added as CC. However under ReviewDb there is
no distinction between reviewer and CC so the results should include
users that were added as CC.

Expand the reviewerin test to cover this.

Bug: Issue 8850
Change-Id: I9f1d53d9ae6aef1e2c10da586951fb7d76978151
(cherry picked from commit 5dd0174832)
This commit is contained in:
David Pursehouse
2018-04-25 21:06:19 +09:00
parent 217fab6ab1
commit e4b7ccc393
2 changed files with 30 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files

35
gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
View File

@@ -1955,11 +1955,12 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
public void reviewerin() throws Exception {
Account.Id user1 = accountManager.authenticate(AuthRequest.forUser("user1")).getAccountId();
Account.Id user2 = accountManager.authenticate(AuthRequest.forUser("user2")).getAccountId();
Account.Id user3 = accountManager.authenticate(AuthRequest.forUser("user3")).getAccountId();
TestRepository<Repo> repo = createProject("repo");
Change change1 = insert(repo, newChange(repo));
Change change2 = insert(repo, newChange(repo));
insert(repo, newChange(repo));
Change change3 = insert(repo, newChange(repo));
AddReviewerInput rin = new AddReviewerInput();
rin.reviewer = user1.toString();
@@ -1971,8 +1972,13 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
rin.state = ReviewerState.REVIEWER;
gApi.changes().id(change2.getId().get()).addReviewer(rin);
rin = new AddReviewerInput();
rin.reviewer = user3.toString();
rin.state = ReviewerState.CC;
gApi.changes().id(change3.getId().get()).addReviewer(rin);
String group = gApi.groups().create("foo").get().name;
gApi.groups().id(group).addMembers(user2.toString());
gApi.groups().id(group).addMembers(user2.toString(), user3.toString());
List<String> members =
gApi.groups()
@@ -1983,15 +1989,30 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
.collect(toList());
assertThat(members).contains(user2.toString());
assertQuery("reviewerin:\"Registered Users\"", change2, change1);
assertQuery("reviewerin:" + group, change2);
if (notesMigration.readChanges()) {
// CC and REVIEWER are separate in NoteDB
assertQuery("reviewerin:\"Registered Users\"", change2, change1);
assertQuery("reviewerin:" + group, change2);
} else {
// CC and REVIEWER are the same in ReviewDb
assertQuery("reviewerin:\"Registered Users\"", change3, change2, change1);
assertQuery("reviewerin:" + group, change3, change2);
}
gApi.changes().id(change2.getId().get()).current().review(ReviewInput.approve());
gApi.changes().id(change2.getId().get()).current().submit();
assertQuery("reviewerin:" + group);
assertQuery("project:repo reviewerin:" + group, change2);
assertQuery("status:merged reviewerin:" + group, change2);
if (notesMigration.readChanges()) {
// CC and REVIEWER are separate in NoteDB
assertQuery("reviewerin:" + group);
assertQuery("project:repo reviewerin:" + group, change2);
assertQuery("status:merged reviewerin:" + group, change2);
} else {
// CC and REVIEWER are the same in ReviewDb
assertQuery("reviewerin:" + group, change3);
assertQuery("project:repo reviewerin:" + group, change2, change3);
assertQuery("status:merged reviewerin:" + group, change2);
}
}
@Test