Merge "Improve Send Reply Metric"
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index 880626e..6968e83 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -4784,6 +4784,15 @@
used for suggesting accounts when adding members to a group.
+
By default 0.
+[[suggest.relevantChanges]]suggest.relevantChanges::
++
+When suggesting reviewers, we go over recent changes of the user, and
+give priority to users that are present as reviewers in any of those
+changes. The number of changes we go over is `sugggest.relevantChanges`.
++
+By default 50. This nubmer is a tradeoff between speed and accuracy.
+A high number would be accurate but slow, and a low number would be
+fast but inaccurate.
[[tracing]]
=== Section tracing
diff --git a/WORKSPACE b/WORKSPACE
index 6fa4973..6e9fb38 100644
--- a/WORKSPACE
+++ b/WORKSPACE
@@ -862,30 +862,30 @@
sha1 = "42a25dc3219429f0e5d060061f71acb49bf010a0",
)
-TRUTH_VERS = "1.0"
+TRUTH_VERS = "1.0.1"
maven_jar(
name = "truth",
artifact = "com.google.truth:truth:" + TRUTH_VERS,
- sha1 = "998e5fb3fa31df716574b4c9e8d374855e800451",
+ sha1 = "361459309085bd9441cb97b62f160e8b353a93c0",
)
maven_jar(
name = "truth-java8-extension",
artifact = "com.google.truth.extensions:truth-java8-extension:" + TRUTH_VERS,
- sha1 = "d85fbc1daf0510821f552f2aa71d9605e97aa438",
+ sha1 = "ef07b2cc2201472381fdd3bcf773310e22bb9080",
)
maven_jar(
name = "truth-liteproto-extension",
artifact = "com.google.truth.extensions:truth-liteproto-extension:" + TRUTH_VERS,
- sha1 = "7a279c50a0f93da15533cef4993b45606cf67d72",
+ sha1 = "bd1f5ac8a5f66e60cd1738f7b95c97a582ffcef9",
)
maven_jar(
name = "truth-proto-extension",
artifact = "com.google.truth.extensions:truth-proto-extension:" + TRUTH_VERS,
- sha1 = "8c0c2ea61750f02d0d5ce9c653106b6a5dc82d12",
+ sha1 = "039aa2d7c9196b30d367eac7cb467ecaa726e23d",
)
maven_jar(
diff --git a/java/com/google/gerrit/server/restapi/change/ReviewerRecommender.java b/java/com/google/gerrit/server/restapi/change/ReviewerRecommender.java
index f07d815..39df82d 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,25 @@
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
+ int numberOfRelevantChanges = config.getInt("suggest", "relevantChanges", 50);
+ // Get the user's last 25 changes, check reviewers
try {
List<ChangeData> result =
queryProvider
.get()
- .setLimit(25)
- .setRequestedFields(ChangeField.APPROVAL)
+ .setLimit(numberOfRelevantChanges)
+ .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 {
diff --git a/polygerrit-ui/app/elements/shared/gr-overlay/gr-overlay.js b/polygerrit-ui/app/elements/shared/gr-overlay/gr-overlay.js
index 8821bcb..5dee2fe 100644
--- a/polygerrit-ui/app/elements/shared/gr-overlay/gr-overlay.js
+++ b/polygerrit-ui/app/elements/shared/gr-overlay/gr-overlay.js
@@ -104,6 +104,12 @@
fn.call(this);
} else if (iters++ < AWAIT_MAX_ITERS) {
step.call(this);
+ } else {
+ // TODO(crbug.com/gerrit/10774): Once this is confirmed as the root
+ // cause of the bug, fix it by either making sure to resolve the fn
+ // function or find a better way to listen on the overlay being
+ // shown.
+ console.warn('gr-overlay _awaitOpen failed to resolve');
}
}, AWAIT_STEP);
};
