Suggest reviewers based on past reviewers instead of approvals Currently, reviewers are suggested to users based on substring of the query, and reviewers that approved recent changes of the users gain priority. This change changes 'approved recent changes' to 'are reviewers of recent changes' Change-Id: I70304e870992b41823a89baf125df4bcacc8209e
diff --git a/java/com/google/gerrit/server/restapi/change/ReviewerRecommender.java b/java/com/google/gerrit/server/restapi/change/ReviewerRecommender.java index f07d815..c939819 100644 --- a/java/com/google/gerrit/server/restapi/change/ReviewerRecommender.java +++ b/java/com/google/gerrit/server/restapi/change/ReviewerRecommender.java
@@ -22,7 +22,6 @@ import com.google.common.flogger.FluentLogger; import com.google.gerrit.common.Nullable; import com.google.gerrit.entities.Account; -import com.google.gerrit.entities.PatchSetApproval; import com.google.gerrit.extensions.client.ReviewerState; import com.google.gerrit.index.query.QueryParseException; import com.google.gerrit.server.ApprovalsUtil; @@ -202,23 +201,24 @@ private Map<Account.Id, MutableDouble> baseRanking( double baseWeight, String query, List<Account.Id> candidateList) throws IOException, ConfigInvalidException { - // Get the user's last 25 changes, check approvals + // Get the user's last 25 changes, check reviewers try { List<ChangeData> result = queryProvider .get() .setLimit(25) - .setRequestedFields(ChangeField.APPROVAL) + .setRequestedFields(ChangeField.REVIEWER) .query(changeQueryBuilder.owner("self")); Map<Account.Id, MutableDouble> suggestions = new LinkedHashMap<>(); // Put those candidates at the bottom of the list candidateList.stream().forEach(id -> suggestions.put(id, new MutableDouble(0))); for (ChangeData cd : result) { - for (PatchSetApproval approval : cd.currentApprovals()) { - Account.Id id = approval.accountId(); - if (Strings.isNullOrEmpty(query) || accountMatchesQuery(id, query)) { - suggestions.computeIfAbsent(id, (ignored) -> new MutableDouble(0)).add(baseWeight); + for (Account.Id reviewer : cd.reviewers().all()) { + if (Strings.isNullOrEmpty(query) || accountMatchesQuery(reviewer, query)) { + suggestions + .computeIfAbsent(reviewer, (ignored) -> new MutableDouble(0)) + .add(baseWeight); } } }
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/SuggestReviewersIT.java b/javatests/com/google/gerrit/acceptance/rest/change/SuggestReviewersIT.java index 568c63b..1bd2d99 100644 --- a/javatests/com/google/gerrit/acceptance/rest/change/SuggestReviewersIT.java +++ b/javatests/com/google/gerrit/acceptance/rest/change/SuggestReviewersIT.java
@@ -39,7 +39,6 @@ import com.google.gerrit.entities.Project; import com.google.gerrit.extensions.api.accounts.EmailInput; import com.google.gerrit.extensions.api.changes.AddReviewerInput; -import com.google.gerrit.extensions.api.changes.ReviewInput; import com.google.gerrit.extensions.client.ReviewerState; import com.google.gerrit.extensions.common.ChangeInput; import com.google.gerrit.extensions.common.SuggestedReviewerInfo; @@ -397,19 +396,13 @@ requestScopeOperations.setApiUser(user1.id()); String changeId1 = createChangeFromApi(); - requestScopeOperations.setApiUser(reviewer1.id()); - reviewChange(changeId1); + reviewChange(changeId1, reviewer1); - requestScopeOperations.setApiUser(user1.id()); String changeId2 = createChangeFromApi(); - requestScopeOperations.setApiUser(reviewer1.id()); - reviewChange(changeId2); + reviewChange(changeId2, reviewer1); + reviewChange(changeId2, reviewer2); - requestScopeOperations.setApiUser(reviewer2.id()); - reviewChange(changeId2); - - requestScopeOperations.setApiUser(user1.id()); String changeId3 = createChangeFromApi(); List<SuggestedReviewerInfo> reviewers = suggestReviewers(changeId3, null, 4); assertThat(reviewers.stream().map(r -> r.account._accountId).collect(toList())) @@ -440,13 +433,11 @@ String name = name("foo"); TestAccount foo1 = accountCreator.create(name + "-1"); - requestScopeOperations.setApiUser(foo1.id()); - reviewChange(changeIdReviewed); + reviewChange(changeIdReviewed, foo1); assertThat(gApi.accounts().id(foo1.username()).getActive()).isTrue(); TestAccount foo2 = accountCreator.create(name + "-2"); - requestScopeOperations.setApiUser(foo2.id()); - reviewChange(changeIdReviewed); + reviewChange(changeIdReviewed, foo2); assertThat(gApi.accounts().id(foo2.username()).getActive()).isTrue(); assertReviewers( @@ -466,12 +457,10 @@ String name = name("foo"); TestAccount foo1 = accountCreator.create(name + "-1"); - requestScopeOperations.setApiUser(foo1.id()); - reviewChange(changeIdReviewed); + reviewChange(changeIdReviewed, foo1); TestAccount foo2 = accountCreator.create(name + "-2"); - requestScopeOperations.setApiUser(foo2.id()); - reviewChange(changeIdReviewed); + reviewChange(changeIdReviewed, foo2); assertReviewers( suggestReviewers(changeId, name), ImmutableList.of(foo1, foo2), ImmutableList.of()); @@ -488,12 +477,10 @@ String name = name("foo"); TestAccount foo1 = accountCreator.create(name + "-1"); - requestScopeOperations.setApiUser(foo1.id()); - reviewChange(changeIdReviewed); + reviewChange(changeIdReviewed, foo1); TestAccount foo2 = accountCreator.create(name + "-2"); - requestScopeOperations.setApiUser(foo2.id()); - reviewChange(changeIdReviewed); + reviewChange(changeIdReviewed, foo2); assertReviewers( suggestReviewers(changeId, name), ImmutableList.of(foo1, foo2), ImmutableList.of()); @@ -514,12 +501,10 @@ String name = name("foo"); TestAccount foo1 = accountCreator.create(name + "-1"); - requestScopeOperations.setApiUser(foo1.id()); - reviewChange(changeIdReviewed); + reviewChange(changeIdReviewed, foo1); TestAccount foo2 = accountCreator.create(name + "-2"); - requestScopeOperations.setApiUser(foo2.id()); - reviewChange(changeIdReviewed); + reviewChange(changeIdReviewed, foo2); assertReviewers(suggestCcs(changeId, name), ImmutableList.of(foo1, foo2), ImmutableList.of()); @@ -575,8 +560,7 @@ String changeIdReviewed = createChangeFromApi(); TestAccount reviewer = accountCreator.create("newReviewer"); - requestScopeOperations.setApiUser(reviewer.id()); - reviewChange(changeIdReviewed); + reviewChange(changeIdReviewed, reviewer); List<SuggestedReviewerInfo> reviewers = suggestReviewers(changeId, "new", 4); assertThat(reviewers.stream().map(r -> r.account._accountId).collect(toList())) @@ -624,10 +608,8 @@ return user(name, fullName, name); } - private void reviewChange(String changeId) throws RestApiException { - ReviewInput ri = new ReviewInput(); - ri.label("Code-Review", 1); - gApi.changes().id(changeId).current().review(ri); + private void reviewChange(String changeId, TestAccount reviewer) throws RestApiException { + gApi.changes().id(changeId).addReviewer(reviewer.id().toString()); } private String createChangeFromApi() throws RestApiException {