Merge "Fix box-shadow of collapsed commit message"
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index 880626e..fc4b00a 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -1223,12 +1223,13 @@
high CPU usage, memory pressure, persistent cache bloat, and other problems.
+
The following operations are allowed even when a change is at the limit:
+
* Abandon
* Submit
* Submit by push with `%submit`
* Auto-close by pushing directly to the branch
* Fix with link:rest-api-changes.html#fix-input[`expect_merged_as`]
-+
+
By default 1000.
[[change.move]]change.move::
@@ -4784,6 +4785,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/httpd/SetThreadNameFilter.java b/java/com/google/gerrit/httpd/SetThreadNameFilter.java
new file mode 100644
index 0000000..c7d977d
--- /dev/null
+++ b/java/com/google/gerrit/httpd/SetThreadNameFilter.java
@@ -0,0 +1,91 @@
+// Copyright (C) 2020 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.httpd;
+
+import com.google.gerrit.server.CurrentUser;
+import com.google.inject.Inject;
+import com.google.inject.Module;
+import com.google.inject.Provider;
+import com.google.inject.Singleton;
+import com.google.inject.servlet.ServletModule;
+import java.io.IOException;
+import javax.servlet.Filter;
+import javax.servlet.FilterChain;
+import javax.servlet.FilterConfig;
+import javax.servlet.ServletException;
+import javax.servlet.ServletRequest;
+import javax.servlet.ServletResponse;
+import javax.servlet.http.HttpServletRequest;
+
+@Singleton
+public class SetThreadNameFilter implements Filter {
+ private static final int MAX_PATH_LENGTH = 120;
+
+ public static Module module() {
+ return new ServletModule() {
+ @Override
+ protected void configureServlets() {
+ filter("/*").through(SetThreadNameFilter.class);
+ }
+ };
+ }
+
+ private final Provider<CurrentUser> user;
+
+ @Inject
+ public SetThreadNameFilter(Provider<CurrentUser> user) {
+ this.user = user;
+ }
+
+ @Override
+ public void init(FilterConfig filterConfig) {}
+
+ @Override
+ public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain)
+ throws IOException, ServletException {
+ Thread current = Thread.currentThread();
+ String old = current.getName();
+ try {
+ current.setName(computeName((HttpServletRequest) request));
+ chain.doFilter(request, response);
+ } finally {
+ current.setName(old);
+ }
+ }
+
+ private String computeName(HttpServletRequest req) {
+ StringBuilder s = new StringBuilder();
+ s.append("HTTP ");
+ s.append(req.getMethod());
+ s.append(" ");
+ s.append(req.getRequestURI());
+ String query = req.getQueryString();
+ if (query != null) {
+ s.append("?").append(query);
+ }
+ if (s.length() > MAX_PATH_LENGTH) {
+ s.delete(MAX_PATH_LENGTH, s.length());
+ }
+ s.append(" (");
+ s.append(user.get().getUserName().orElse("N/A"));
+ s.append(" from ");
+ s.append(req.getRemoteAddr());
+ s.append(")");
+ return s.toString();
+ }
+
+ @Override
+ public void destroy() {}
+}
diff --git a/java/com/google/gerrit/httpd/init/WebAppInitializer.java b/java/com/google/gerrit/httpd/init/WebAppInitializer.java
index 3da968e..769396e 100644
--- a/java/com/google/gerrit/httpd/init/WebAppInitializer.java
+++ b/java/com/google/gerrit/httpd/init/WebAppInitializer.java
@@ -31,6 +31,7 @@
import com.google.gerrit.httpd.RequestContextFilter;
import com.google.gerrit.httpd.RequestMetricsFilter;
import com.google.gerrit.httpd.RequireSslFilter;
+import com.google.gerrit.httpd.SetThreadNameFilter;
import com.google.gerrit.httpd.WebModule;
import com.google.gerrit.httpd.WebSshGlueModule;
import com.google.gerrit.httpd.auth.oauth.OAuthModule;
@@ -385,6 +386,7 @@
modules.add(sysInjector.getInstance(GerritAuthModule.class));
modules.add(sysInjector.getInstance(GitOverHttpModule.class));
modules.add(RequestCleanupFilter.module());
+ modules.add(SetThreadNameFilter.module());
modules.add(AllRequestFilter.module());
modules.add(sysInjector.getInstance(WebModule.class));
modules.add(sysInjector.getInstance(RequireSslFilter.Module.class));
diff --git a/java/com/google/gerrit/pgm/Daemon.java b/java/com/google/gerrit/pgm/Daemon.java
index d8beb76..2e9ef2f 100644
--- a/java/com/google/gerrit/pgm/Daemon.java
+++ b/java/com/google/gerrit/pgm/Daemon.java
@@ -35,6 +35,7 @@
import com.google.gerrit.httpd.RequestContextFilter;
import com.google.gerrit.httpd.RequestMetricsFilter;
import com.google.gerrit.httpd.RequireSslFilter;
+import com.google.gerrit.httpd.SetThreadNameFilter;
import com.google.gerrit.httpd.WebModule;
import com.google.gerrit.httpd.WebSshGlueModule;
import com.google.gerrit.httpd.auth.oauth.OAuthModule;
@@ -549,6 +550,7 @@
}
modules.add(RequestCleanupFilter.module());
modules.add(AllRequestFilter.module());
+ modules.add(SetThreadNameFilter.module());
modules.add(sysInjector.getInstance(WebModule.class));
modules.add(sysInjector.getInstance(RequireSslFilter.Module.class));
modules.add(new HttpPluginModule());
diff --git a/java/com/google/gerrit/server/edit/ChangeEditUtil.java b/java/com/google/gerrit/server/edit/ChangeEditUtil.java
index 926e0d5..710916e 100644
--- a/java/com/google/gerrit/server/edit/ChangeEditUtil.java
+++ b/java/com/google/gerrit/server/edit/ChangeEditUtil.java
@@ -169,7 +169,10 @@
RevCommit squashed = squashEdit(rw, oi, edit.getEditCommit(), basePatchSet);
PatchSet.Id psId = ChangeUtil.nextPatchSetId(repo, change.currentPatchSetId());
- PatchSetInserter inserter = patchSetInserterFactory.create(notes, psId, squashed);
+ PatchSetInserter inserter =
+ patchSetInserterFactory
+ .create(notes, psId, squashed)
+ .setSendEmail(!change.isWorkInProgress());
StringBuilder message =
new StringBuilder("Patch Set ").append(inserter.getPatchSetId().get()).append(": ");
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/java/com/google/gerrit/testing/TestCommentHelper.java b/java/com/google/gerrit/testing/TestCommentHelper.java
index b72cca7..deda355 100644
--- a/java/com/google/gerrit/testing/TestCommentHelper.java
+++ b/java/com/google/gerrit/testing/TestCommentHelper.java
@@ -16,14 +16,21 @@
import static java.util.stream.Collectors.toList;
+import com.google.common.collect.ImmutableList;
import com.google.gerrit.extensions.api.GerritApi;
import com.google.gerrit.extensions.api.changes.DraftInput;
+import com.google.gerrit.extensions.api.changes.ReviewInput;
+import com.google.gerrit.extensions.api.changes.ReviewInput.RobotCommentInput;
import com.google.gerrit.extensions.client.Comment;
import com.google.gerrit.extensions.client.Comment.Range;
import com.google.gerrit.extensions.client.Side;
import com.google.gerrit.extensions.common.CommentInfo;
+import com.google.gerrit.extensions.common.FixSuggestionInfo;
import com.google.inject.Inject;
+import java.util.Arrays;
import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
/** Test helper for dealing with comments/drafts. */
public class TestCommentHelper {
@@ -44,7 +51,7 @@
}
public void addDraft(String changeId, String revId, DraftInput in) throws Exception {
- gApi.changes().id(changeId).revision(revId).createDraft(in).get();
+ gApi.changes().id(changeId).revision(revId).createDraft(in);
}
public Collection<CommentInfo> getPublishedComments(String changeId) throws Exception {
@@ -104,4 +111,34 @@
range.endCharacter = 5;
return range;
}
+
+ public static RobotCommentInput createRobotCommentInputWithMandatoryFields(String path) {
+ RobotCommentInput in = new RobotCommentInput();
+ in.robotId = "happyRobot";
+ in.robotRunId = "1";
+ in.line = 1;
+ in.message = "nit: trailing whitespace";
+ in.path = path;
+ return in;
+ }
+
+ public static RobotCommentInput createRobotCommentInput(
+ String path, FixSuggestionInfo... fixSuggestionInfos) {
+ RobotCommentInput in = TestCommentHelper.createRobotCommentInputWithMandatoryFields(path);
+ in.url = "http://www.happy-robot.com";
+ in.properties = new HashMap<>();
+ in.properties.put("key1", "value1");
+ in.properties.put("key2", "value2");
+ in.fixSuggestions = Arrays.asList(fixSuggestionInfos);
+ return in;
+ }
+
+ public void addRobotComment(String targetChangeId, RobotCommentInput robotCommentInput)
+ throws Exception {
+ ReviewInput reviewInput = new ReviewInput();
+ reviewInput.robotComments =
+ Collections.singletonMap(robotCommentInput.path, ImmutableList.of(robotCommentInput));
+ reviewInput.message = "robot comment test";
+ gApi.changes().id(targetChangeId).current().review(reviewInput);
+ }
}
diff --git a/javatests/com/google/gerrit/acceptance/api/revision/RobotCommentsIT.java b/javatests/com/google/gerrit/acceptance/api/revision/RobotCommentsIT.java
index abf0279..c35ded6 100644
--- a/javatests/com/google/gerrit/acceptance/api/revision/RobotCommentsIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/revision/RobotCommentsIT.java
@@ -28,7 +28,6 @@
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.config.GerritConfig;
-import com.google.gerrit.extensions.api.changes.ReviewInput;
import com.google.gerrit.extensions.api.changes.ReviewInput.RobotCommentInput;
import com.google.gerrit.extensions.client.Comment;
import com.google.gerrit.extensions.common.ChangeInfo;
@@ -45,9 +44,10 @@
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.testing.BinaryResultSubject;
+import com.google.gerrit.testing.TestCommentHelper;
+import com.google.inject.Inject;
import java.util.Arrays;
import java.util.Collections;
-import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
@@ -57,6 +57,8 @@
import org.junit.Test;
public class RobotCommentsIT extends AbstractDaemonTest {
+ @Inject private TestCommentHelper testCommentHelper;
+
private static final String PLAIN_TEXT_CONTENT_TYPE = "text/plain";
private static final String FILE_NAME = "file_to_fix.txt";
@@ -86,7 +88,8 @@
fixReplacementInfo = createFixReplacementInfo();
fixSuggestionInfo = createFixSuggestionInfo(fixReplacementInfo);
- withFixRobotCommentInput = createRobotCommentInput(fixSuggestionInfo);
+ withFixRobotCommentInput =
+ TestCommentHelper.createRobotCommentInput(FILE_NAME, fixSuggestionInfo);
}
@Test
@@ -100,8 +103,8 @@
@Test
public void addedRobotCommentsCanBeRetrieved() throws Exception {
- RobotCommentInput in = createRobotCommentInput();
- addRobotComment(changeId, in);
+ RobotCommentInput in = TestCommentHelper.createRobotCommentInput(FILE_NAME);
+ testCommentHelper.addRobotComment(changeId, in);
Map<String, List<RobotCommentInfo>> out = gApi.changes().id(changeId).current().robotComments();
@@ -112,13 +115,13 @@
@Test
public void addedRobotCommentsCanBeRetrievedByChange() throws Exception {
- RobotCommentInput in = createRobotCommentInput();
- addRobotComment(changeId, in);
+ RobotCommentInput in = TestCommentHelper.createRobotCommentInput(FILE_NAME);
+ testCommentHelper.addRobotComment(changeId, in);
pushFactory.create(admin.newIdent(), testRepo, changeId).to("refs/for/master");
- RobotCommentInput in2 = createRobotCommentInput();
- addRobotComment(changeId, in2);
+ RobotCommentInput in2 = TestCommentHelper.createRobotCommentInput(FILE_NAME);
+ testCommentHelper.addRobotComment(changeId, in2);
Map<String, List<RobotCommentInfo>> out = gApi.changes().id(changeId).robotComments();
@@ -133,8 +136,8 @@
@Test
public void robotCommentsCanBeRetrievedAsList() throws Exception {
- RobotCommentInput robotCommentInput = createRobotCommentInput();
- addRobotComment(changeId, robotCommentInput);
+ RobotCommentInput robotCommentInput = TestCommentHelper.createRobotCommentInput(FILE_NAME);
+ testCommentHelper.addRobotComment(changeId, robotCommentInput);
List<RobotCommentInfo> robotCommentInfos =
gApi.changes().id(changeId).current().robotCommentsAsList();
@@ -146,8 +149,8 @@
@Test
public void specificRobotCommentCanBeRetrieved() throws Exception {
- RobotCommentInput robotCommentInput = createRobotCommentInput();
- addRobotComment(changeId, robotCommentInput);
+ RobotCommentInput robotCommentInput = TestCommentHelper.createRobotCommentInput(FILE_NAME);
+ testCommentHelper.addRobotComment(changeId, robotCommentInput);
List<RobotCommentInfo> robotCommentInfos = getRobotComments();
RobotCommentInfo robotCommentInfo = Iterables.getOnlyElement(robotCommentInfos);
@@ -159,8 +162,8 @@
@Test
public void robotCommentWithoutOptionalFieldsCanBeAdded() throws Exception {
- RobotCommentInput in = createRobotCommentInputWithMandatoryFields();
- addRobotComment(changeId, in);
+ RobotCommentInput in = TestCommentHelper.createRobotCommentInputWithMandatoryFields(FILE_NAME);
+ testCommentHelper.addRobotComment(changeId, in);
Map<String, List<RobotCommentInfo>> out = gApi.changes().id(changeId).current().robotComments();
assertThat(out).hasSize(1);
@@ -169,14 +172,15 @@
}
@Test
- public void hugeRobotCommentIsRejected() throws Exception {
+ public void hugeRobotCommentIsRejected() {
int defaultSizeLimit = 1024 * 1024;
int sizeOfRest = 451;
fixReplacementInfo.replacement = getStringFor(defaultSizeLimit - sizeOfRest + 1);
BadRequestException thrown =
assertThrows(
- BadRequestException.class, () -> addRobotComment(changeId, withFixRobotCommentInput));
+ BadRequestException.class,
+ () -> testCommentHelper.addRobotComment(changeId, withFixRobotCommentInput));
assertThat(thrown).hasMessageThat().contains("limit");
}
@@ -186,7 +190,7 @@
int sizeOfRest = 451;
fixReplacementInfo.replacement = getStringFor(defaultSizeLimit - sizeOfRest);
- addRobotComment(changeId, withFixRobotCommentInput);
+ testCommentHelper.addRobotComment(changeId, withFixRobotCommentInput);
List<RobotCommentInfo> robotCommentInfos = getRobotComments();
assertThat(robotCommentInfos).hasSize(1);
@@ -194,13 +198,14 @@
@Test
@GerritConfig(name = "change.robotCommentSizeLimit", value = "10k")
- public void maximumAllowedSizeOfRobotCommentCanBeAdjusted() throws Exception {
+ public void maximumAllowedSizeOfRobotCommentCanBeAdjusted() {
int sizeLimit = 10 * 1024;
fixReplacementInfo.replacement = getStringFor(sizeLimit);
BadRequestException thrown =
assertThrows(
- BadRequestException.class, () -> addRobotComment(changeId, withFixRobotCommentInput));
+ BadRequestException.class,
+ () -> testCommentHelper.addRobotComment(changeId, withFixRobotCommentInput));
assertThat(thrown).hasMessageThat().contains("limit");
}
@@ -210,7 +215,7 @@
int defaultSizeLimit = 1024 * 1024;
fixReplacementInfo.replacement = getStringFor(defaultSizeLimit);
- addRobotComment(changeId, withFixRobotCommentInput);
+ testCommentHelper.addRobotComment(changeId, withFixRobotCommentInput);
List<RobotCommentInfo> robotCommentInfos = getRobotComments();
assertThat(robotCommentInfos).hasSize(1);
@@ -223,7 +228,7 @@
int defaultSizeLimit = 1024 * 1024;
fixReplacementInfo.replacement = getStringFor(defaultSizeLimit);
- addRobotComment(changeId, withFixRobotCommentInput);
+ testCommentHelper.addRobotComment(changeId, withFixRobotCommentInput);
List<RobotCommentInfo> robotCommentInfos = getRobotComments();
assertThat(robotCommentInfos).hasSize(1);
@@ -231,7 +236,7 @@
@Test
public void addedFixSuggestionCanBeRetrieved() throws Exception {
- addRobotComment(changeId, withFixRobotCommentInput);
+ testCommentHelper.addRobotComment(changeId, withFixRobotCommentInput);
List<RobotCommentInfo> robotCommentInfos = getRobotComments();
assertThatList(robotCommentInfos).onlyElement().onlyFixSuggestion().isNotNull();
@@ -239,7 +244,7 @@
@Test
public void fixIdIsGeneratedForFixSuggestion() throws Exception {
- addRobotComment(changeId, withFixRobotCommentInput);
+ testCommentHelper.addRobotComment(changeId, withFixRobotCommentInput);
List<RobotCommentInfo> robotCommentInfos = getRobotComments();
assertThatList(robotCommentInfos).onlyElement().onlyFixSuggestion().fixId().isNotEmpty();
@@ -252,7 +257,7 @@
@Test
public void descriptionOfFixSuggestionIsAcceptedAsIs() throws Exception {
- addRobotComment(changeId, withFixRobotCommentInput);
+ testCommentHelper.addRobotComment(changeId, withFixRobotCommentInput);
List<RobotCommentInfo> robotCommentInfos = getRobotComments();
assertThatList(robotCommentInfos)
@@ -263,12 +268,13 @@
}
@Test
- public void descriptionOfFixSuggestionIsMandatory() throws Exception {
+ public void descriptionOfFixSuggestionIsMandatory() {
fixSuggestionInfo.description = null;
BadRequestException thrown =
assertThrows(
- BadRequestException.class, () -> addRobotComment(changeId, withFixRobotCommentInput));
+ BadRequestException.class,
+ () -> testCommentHelper.addRobotComment(changeId, withFixRobotCommentInput));
assertThat(thrown)
.hasMessageThat()
.contains(
@@ -279,7 +285,7 @@
@Test
public void addedFixReplacementCanBeRetrieved() throws Exception {
- addRobotComment(changeId, withFixRobotCommentInput);
+ testCommentHelper.addRobotComment(changeId, withFixRobotCommentInput);
List<RobotCommentInfo> robotCommentInfos = getRobotComments();
assertThatList(robotCommentInfos)
@@ -290,12 +296,13 @@
}
@Test
- public void fixReplacementsAreMandatory() throws Exception {
+ public void fixReplacementsAreMandatory() {
fixSuggestionInfo.replacements = Collections.emptyList();
BadRequestException thrown =
assertThrows(
- BadRequestException.class, () -> addRobotComment(changeId, withFixRobotCommentInput));
+ BadRequestException.class,
+ () -> testCommentHelper.addRobotComment(changeId, withFixRobotCommentInput));
assertThat(thrown)
.hasMessageThat()
.contains(
@@ -307,7 +314,7 @@
@Test
public void pathOfFixReplacementIsAcceptedAsIs() throws Exception {
- addRobotComment(changeId, withFixRobotCommentInput);
+ testCommentHelper.addRobotComment(changeId, withFixRobotCommentInput);
List<RobotCommentInfo> robotCommentInfos = getRobotComments();
@@ -320,12 +327,13 @@
}
@Test
- public void pathOfFixReplacementIsMandatory() throws Exception {
+ public void pathOfFixReplacementIsMandatory() {
fixReplacementInfo.path = null;
BadRequestException thrown =
assertThrows(
- BadRequestException.class, () -> addRobotComment(changeId, withFixRobotCommentInput));
+ BadRequestException.class,
+ () -> testCommentHelper.addRobotComment(changeId, withFixRobotCommentInput));
assertThat(thrown)
.hasMessageThat()
.contains(
@@ -336,7 +344,7 @@
@Test
public void rangeOfFixReplacementIsAcceptedAsIs() throws Exception {
- addRobotComment(changeId, withFixRobotCommentInput);
+ testCommentHelper.addRobotComment(changeId, withFixRobotCommentInput);
List<RobotCommentInfo> robotCommentInfos = getRobotComments();
@@ -349,12 +357,13 @@
}
@Test
- public void rangeOfFixReplacementIsMandatory() throws Exception {
+ public void rangeOfFixReplacementIsMandatory() {
fixReplacementInfo.range = null;
BadRequestException thrown =
assertThrows(
- BadRequestException.class, () -> addRobotComment(changeId, withFixRobotCommentInput));
+ BadRequestException.class,
+ () -> testCommentHelper.addRobotComment(changeId, withFixRobotCommentInput));
assertThat(thrown)
.hasMessageThat()
.contains(
@@ -364,17 +373,17 @@
}
@Test
- public void rangeOfFixReplacementNeedsToBeValid() throws Exception {
+ public void rangeOfFixReplacementNeedsToBeValid() {
fixReplacementInfo.range = createRange(13, 9, 5, 10);
BadRequestException thrown =
assertThrows(
- BadRequestException.class, () -> addRobotComment(changeId, withFixRobotCommentInput));
+ BadRequestException.class,
+ () -> testCommentHelper.addRobotComment(changeId, withFixRobotCommentInput));
assertThat(thrown).hasMessageThat().contains("Range (13:9 - 5:10)");
}
@Test
- public void rangesOfFixReplacementsOfSameFixSuggestionForSameFileMayNotOverlap()
- throws Exception {
+ public void rangesOfFixReplacementsOfSameFixSuggestionForSameFileMayNotOverlap() {
FixReplacementInfo fixReplacementInfo1 = new FixReplacementInfo();
fixReplacementInfo1.path = FILE_NAME;
fixReplacementInfo1.range = createRange(2, 0, 3, 1);
@@ -391,7 +400,8 @@
BadRequestException thrown =
assertThrows(
- BadRequestException.class, () -> addRobotComment(changeId, withFixRobotCommentInput));
+ BadRequestException.class,
+ () -> testCommentHelper.addRobotComment(changeId, withFixRobotCommentInput));
assertThat(thrown).hasMessageThat().contains("overlap");
}
@@ -412,7 +422,7 @@
createFixSuggestionInfo(fixReplacementInfo1, fixReplacementInfo2);
withFixRobotCommentInput.fixSuggestions = ImmutableList.of(fixSuggestionInfo);
- addRobotComment(changeId, withFixRobotCommentInput);
+ testCommentHelper.addRobotComment(changeId, withFixRobotCommentInput);
List<RobotCommentInfo> robotCommentInfos = getRobotComments();
assertThatList(robotCommentInfos).onlyElement().fixSuggestions().hasSize(1);
@@ -436,7 +446,7 @@
withFixRobotCommentInput.fixSuggestions =
ImmutableList.of(fixSuggestionInfo1, fixSuggestionInfo2);
- addRobotComment(changeId, withFixRobotCommentInput);
+ testCommentHelper.addRobotComment(changeId, withFixRobotCommentInput);
List<RobotCommentInfo> robotCommentInfos = getRobotComments();
assertThatList(robotCommentInfos).onlyElement().fixSuggestions().hasSize(2);
@@ -463,7 +473,7 @@
createFixSuggestionInfo(fixReplacementInfo2, fixReplacementInfo1, fixReplacementInfo3);
withFixRobotCommentInput.fixSuggestions = ImmutableList.of(fixSuggestionInfo);
- addRobotComment(changeId, withFixRobotCommentInput);
+ testCommentHelper.addRobotComment(changeId, withFixRobotCommentInput);
List<RobotCommentInfo> robotCommentInfos = getRobotComments();
assertThatList(robotCommentInfos).onlyElement().onlyFixSuggestion().replacements().hasSize(3);
@@ -471,7 +481,7 @@
@Test
public void replacementStringOfFixReplacementIsAcceptedAsIs() throws Exception {
- addRobotComment(changeId, withFixRobotCommentInput);
+ testCommentHelper.addRobotComment(changeId, withFixRobotCommentInput);
List<RobotCommentInfo> robotCommentInfos = getRobotComments();
@@ -484,12 +494,13 @@
}
@Test
- public void replacementStringOfFixReplacementIsMandatory() throws Exception {
+ public void replacementStringOfFixReplacementIsMandatory() {
fixReplacementInfo.replacement = null;
BadRequestException thrown =
assertThrows(
- BadRequestException.class, () -> addRobotComment(changeId, withFixRobotCommentInput));
+ BadRequestException.class,
+ () -> testCommentHelper.addRobotComment(changeId, withFixRobotCommentInput));
assertThat(thrown)
.hasMessageThat()
.contains(
@@ -505,7 +516,7 @@
fixReplacementInfo.replacement = "Modified content";
fixReplacementInfo.range = createRange(3, 1, 3, 3);
- addRobotComment(changeId, withFixRobotCommentInput);
+ testCommentHelper.addRobotComment(changeId, withFixRobotCommentInput);
List<RobotCommentInfo> robotCommentInfos = getRobotComments();
List<String> fixIds = getFixIds(robotCommentInfos);
@@ -528,7 +539,7 @@
fixReplacementInfo.replacement = "Modified content\n5";
fixReplacementInfo.range = createRange(3, 2, 5, 3);
- addRobotComment(changeId, withFixRobotCommentInput);
+ testCommentHelper.addRobotComment(changeId, withFixRobotCommentInput);
List<RobotCommentInfo> robotCommentInfos = getRobotComments();
List<String> fixIds = getFixIds(robotCommentInfos);
String fixId = Iterables.getOnlyElement(fixIds);
@@ -560,7 +571,7 @@
createFixSuggestionInfo(fixReplacementInfo1, fixReplacementInfo2);
withFixRobotCommentInput.fixSuggestions = ImmutableList.of(fixSuggestionInfo);
- addRobotComment(changeId, withFixRobotCommentInput);
+ testCommentHelper.addRobotComment(changeId, withFixRobotCommentInput);
List<RobotCommentInfo> robotCommentInfos = getRobotComments();
List<String> fixIds = getFixIds(robotCommentInfos);
String fixId = Iterables.getOnlyElement(fixIds);
@@ -590,10 +601,12 @@
fixReplacementInfo2.replacement = "Some other modified content\n";
FixSuggestionInfo fixSuggestionInfo2 = createFixSuggestionInfo(fixReplacementInfo2);
- RobotCommentInput robotCommentInput1 = createRobotCommentInput(fixSuggestionInfo1);
- RobotCommentInput robotCommentInput2 = createRobotCommentInput(fixSuggestionInfo2);
- addRobotComment(changeId, robotCommentInput1);
- addRobotComment(changeId, robotCommentInput2);
+ RobotCommentInput robotCommentInput1 =
+ TestCommentHelper.createRobotCommentInput(FILE_NAME, fixSuggestionInfo1);
+ RobotCommentInput robotCommentInput2 =
+ TestCommentHelper.createRobotCommentInput(FILE_NAME, fixSuggestionInfo2);
+ testCommentHelper.addRobotComment(changeId, robotCommentInput1);
+ testCommentHelper.addRobotComment(changeId, robotCommentInput2);
List<RobotCommentInfo> robotCommentInfos = getRobotComments();
List<String> fixIds = getFixIds(robotCommentInfos);
@@ -623,10 +636,12 @@
fixReplacementInfo2.replacement = "Some other modified content\n";
FixSuggestionInfo fixSuggestionInfo2 = createFixSuggestionInfo(fixReplacementInfo2);
- RobotCommentInput robotCommentInput1 = createRobotCommentInput(fixSuggestionInfo1);
- RobotCommentInput robotCommentInput2 = createRobotCommentInput(fixSuggestionInfo2);
- addRobotComment(changeId, robotCommentInput1);
- addRobotComment(changeId, robotCommentInput2);
+ RobotCommentInput robotCommentInput1 =
+ TestCommentHelper.createRobotCommentInput(FILE_NAME, fixSuggestionInfo1);
+ RobotCommentInput robotCommentInput2 =
+ TestCommentHelper.createRobotCommentInput(FILE_NAME, fixSuggestionInfo2);
+ testCommentHelper.addRobotComment(changeId, robotCommentInput1);
+ testCommentHelper.addRobotComment(changeId, robotCommentInput2);
List<RobotCommentInfo> robotCommentInfos = getRobotComments();
List<String> fixIds = getFixIds(robotCommentInfos);
@@ -655,7 +670,7 @@
withFixRobotCommentInput.fixSuggestions =
ImmutableList.of(fixSuggestionInfo1, fixSuggestionInfo2);
- addRobotComment(changeId, withFixRobotCommentInput);
+ testCommentHelper.addRobotComment(changeId, withFixRobotCommentInput);
List<RobotCommentInfo> robotCommentInfos = getRobotComments();
List<String> fixIds = getFixIds(robotCommentInfos);
@@ -677,7 +692,7 @@
fixReplacementInfo.range = createRange(2, 0, 3, 0);
fixReplacementInfo.replacement = "Modified content\n";
- addRobotComment(changeId, withFixRobotCommentInput);
+ testCommentHelper.addRobotComment(changeId, withFixRobotCommentInput);
List<RobotCommentInfo> robotCommentInfos = getRobotComments();
List<String> fixIds = getFixIds(robotCommentInfos);
String fixId = Iterables.getOnlyElement(fixIds);
@@ -707,7 +722,7 @@
createFixSuggestionInfo(fixReplacementInfo1, fixReplacementInfo2);
withFixRobotCommentInput.fixSuggestions = ImmutableList.of(fixSuggestionInfo);
- addRobotComment(changeId, withFixRobotCommentInput);
+ testCommentHelper.addRobotComment(changeId, withFixRobotCommentInput);
List<RobotCommentInfo> robotCommentInfos = getRobotComments();
List<String> fixIds = getFixIds(robotCommentInfos);
String fixId = Iterables.getOnlyElement(fixIds);
@@ -734,7 +749,7 @@
fixReplacementInfo.range = createRange(1, 0, 2, 0);
fixReplacementInfo.replacement = "Modified content\n";
- addRobotComment(changeId, withFixRobotCommentInput);
+ testCommentHelper.addRobotComment(changeId, withFixRobotCommentInput);
List<RobotCommentInfo> robotCommentInfos = getRobotComments();
List<String> fixIds = getFixIds(robotCommentInfos);
String fixId = Iterables.getOnlyElement(fixIds);
@@ -750,7 +765,7 @@
fixReplacementInfo.replacement = "Modified content";
fixReplacementInfo.range = createRange(3, 1, 3, 3);
- addRobotComment(changeId, withFixRobotCommentInput);
+ testCommentHelper.addRobotComment(changeId, withFixRobotCommentInput);
List<RobotCommentInfo> robotCommentInfos = getRobotComments();
// Remember patch set and add another one.
@@ -776,7 +791,7 @@
fixReplacementInfo.replacement = "Modified content";
fixReplacementInfo.range = createRange(3, 1, 3, 3);
- addRobotComment(changeId, withFixRobotCommentInput);
+ testCommentHelper.addRobotComment(changeId, withFixRobotCommentInput);
List<RobotCommentInfo> robotCommentInfos = getRobotComments();
// Remember patch set and add another one.
@@ -811,7 +826,7 @@
fixReplacementInfo.replacement = "Modified content";
fixReplacementInfo.range = createRange(3, 1, 3, 3);
- addRobotComment(changeId, withFixRobotCommentInput);
+ testCommentHelper.addRobotComment(changeId, withFixRobotCommentInput);
List<RobotCommentInfo> robotCommentInfos = getRobotComments();
List<String> fixIds = getFixIds(robotCommentInfos);
@@ -833,7 +848,7 @@
fixReplacementInfo.replacement = "Modified content";
fixReplacementInfo.range = createRange(3, 1, 3, 3);
- addRobotComment(changeId, withFixRobotCommentInput);
+ testCommentHelper.addRobotComment(changeId, withFixRobotCommentInput);
List<RobotCommentInfo> robotCommentInfos = getRobotComments();
List<String> fixIds = getFixIds(robotCommentInfos);
@@ -851,7 +866,7 @@
fixReplacementInfo.replacement = "Modified content";
fixReplacementInfo.range = createRange(3, 1, 3, 3);
- addRobotComment(changeId, withFixRobotCommentInput);
+ testCommentHelper.addRobotComment(changeId, withFixRobotCommentInput);
List<RobotCommentInfo> robotCommentInfos = getRobotComments();
List<String> fixIds = getFixIds(robotCommentInfos);
@@ -874,7 +889,7 @@
fixReplacementInfo.replacement = "Modified content";
fixReplacementInfo.range = createRange(3, 1, 3, 3);
- addRobotComment(changeId, withFixRobotCommentInput);
+ testCommentHelper.addRobotComment(changeId, withFixRobotCommentInput);
List<RobotCommentInfo> robotCommentInfos = getRobotComments();
List<String> fixIds = getFixIds(robotCommentInfos);
@@ -892,7 +907,7 @@
fixReplacementInfo.replacement = "Modified content";
fixReplacementInfo.range = createRange(3, 1, 3, 3);
- addRobotComment(changeId, withFixRobotCommentInput);
+ testCommentHelper.addRobotComment(changeId, withFixRobotCommentInput);
List<RobotCommentInfo> robotCommentInfos = getRobotComments();
List<String> fixIds = getFixIds(robotCommentInfos);
@@ -915,7 +930,7 @@
fixReplacementInfo.replacement = "Modified content";
fixReplacementInfo.range = createRange(3, 1, 3, 3);
- addRobotComment(changeId, withFixRobotCommentInput);
+ testCommentHelper.addRobotComment(changeId, withFixRobotCommentInput);
List<RobotCommentInfo> robotCommentInfos = getRobotComments();
List<String> fixIds = getFixIds(robotCommentInfos);
@@ -938,7 +953,7 @@
fixReplacementInfo.replacement = "Modified content";
fixReplacementInfo.range = createRange(3, 1, 3, 3);
- addRobotComment(changeId, withFixRobotCommentInput);
+ testCommentHelper.addRobotComment(changeId, withFixRobotCommentInput);
List<RobotCommentInfo> robotCommentInfos = getRobotComments();
List<String> fixIds = getFixIds(robotCommentInfos);
@@ -957,7 +972,8 @@
.create(admin.newIdent(), testRepo, SUBJECT, FILE_NAME, "new content", r1.getChangeId())
.to("refs/for/master");
- addRobotComment(r2.getChangeId(), createRobotCommentInputWithMandatoryFields());
+ testCommentHelper.addRobotComment(
+ r2.getChangeId(), TestCommentHelper.createRobotCommentInputWithMandatoryFields(FILE_NAME));
try (AutoCloseable ignored = disableNoteDb()) {
ChangeInfo result = Iterables.getOnlyElement(query(r2.getChangeId()));
@@ -971,7 +987,7 @@
@Test
public void getFixPreviewWithNonexistingFixId() throws Exception {
- addRobotComment(changeId, withFixRobotCommentInput);
+ testCommentHelper.addRobotComment(changeId, withFixRobotCommentInput);
assertThrows(
ResourceNotFoundException.class,
@@ -986,7 +1002,7 @@
fixReplacementInfo.range = createRange(1, 0, 2, 0);
fixReplacementInfo.replacement = "Modified content\n";
- addRobotComment(changeId, withFixRobotCommentInput);
+ testCommentHelper.addRobotComment(changeId, withFixRobotCommentInput);
List<RobotCommentInfo> robotCommentInfos = getRobotComments();
List<String> fixIds = getFixIds(robotCommentInfos);
String fixId = Iterables.getOnlyElement(fixIds);
@@ -1010,9 +1026,10 @@
fixSuggestionInfo = createFixSuggestionInfo(fixReplacementInfoFile1, fixReplacementInfoFile2);
- withFixRobotCommentInput = createRobotCommentInput(fixSuggestionInfo);
+ withFixRobotCommentInput =
+ TestCommentHelper.createRobotCommentInput(FILE_NAME, fixSuggestionInfo);
- addRobotComment(changeId, withFixRobotCommentInput);
+ testCommentHelper.addRobotComment(changeId, withFixRobotCommentInput);
List<RobotCommentInfo> robotCommentInfos = getRobotComments();
List<String> fixIds = getFixIds(robotCommentInfos);
@@ -1105,27 +1122,6 @@
assertThat(diff2).content().element(2).linesOfB().isNull();
}
- private static RobotCommentInput createRobotCommentInputWithMandatoryFields() {
- RobotCommentInput in = new RobotCommentInput();
- in.robotId = "happyRobot";
- in.robotRunId = "1";
- in.line = 1;
- in.message = "nit: trailing whitespace";
- in.path = FILE_NAME;
- return in;
- }
-
- private static RobotCommentInput createRobotCommentInput(
- FixSuggestionInfo... fixSuggestionInfos) {
- RobotCommentInput in = createRobotCommentInputWithMandatoryFields();
- in.url = "http://www.happy-robot.com";
- in.properties = new HashMap<>();
- in.properties.put("key1", "value1");
- in.properties.put("key2", "value2");
- in.fixSuggestions = Arrays.asList(fixSuggestionInfos);
- return in;
- }
-
private static FixSuggestionInfo createFixSuggestionInfo(
FixReplacementInfo... fixReplacementInfos) {
FixSuggestionInfo newFixSuggestionInfo = new FixSuggestionInfo();
@@ -1153,15 +1149,6 @@
return range;
}
- private void addRobotComment(String targetChangeId, RobotCommentInput robotCommentInput)
- throws Exception {
- ReviewInput reviewInput = new ReviewInput();
- reviewInput.robotComments =
- Collections.singletonMap(robotCommentInput.path, ImmutableList.of(robotCommentInput));
- reviewInput.message = "robot comment test";
- gApi.changes().id(targetChangeId).current().review(reviewInput);
- }
-
private List<RobotCommentInfo> getRobotComments() throws RestApiException {
return gApi.changes().id(changeId).current().robotCommentsAsList();
}
diff --git a/javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java b/javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java
index 2883d8c..e72bf06 100644
--- a/javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java
+++ b/javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java
@@ -142,8 +142,11 @@
public void publishEdit() throws Exception {
createArbitraryEditFor(changeId);
+ AddReviewerInput in = new AddReviewerInput();
+ in.reviewer = user.email();
+ gApi.changes().id(changeId).addReviewer(in);
+
PublishChangeEditInput publishInput = new PublishChangeEditInput();
- publishInput.notify = NotifyHandling.NONE;
gApi.changes().id(changeId).edit().publish(publishInput);
assertThat(getEdit(changeId)).isAbsent();
@@ -160,8 +163,10 @@
assertThat(info.messages).isNotEmpty();
assertThat(Iterables.getLast(info.messages).tag)
.isEqualTo(ChangeMessagesUtil.TAG_UPLOADED_PATCH_SET);
+ assertThat(sender.getMessages()).isNotEmpty();
// Move the change to WIP, repeat, and verify.
+ sender.clear();
gApi.changes().id(changeId).setWorkInProgress();
createEmptyEditFor(changeId);
gApi.changes().id(changeId).edit().modifyFile(FILE_NAME, RawInputUtil.create(CONTENT_NEW2));
@@ -170,6 +175,7 @@
assertThat(info.messages).isNotEmpty();
assertThat(Iterables.getLast(info.messages).tag)
.isEqualTo(ChangeMessagesUtil.TAG_UPLOADED_WIP_PATCH_SET);
+ assertThat(sender.getMessages()).isEmpty();
}
@Test
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/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java b/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java
index c67a842..53acdeb 100644
--- a/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java
+++ b/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java
@@ -63,7 +63,7 @@
case V7_4:
return "blacktop/elasticsearch:7.4.2";
case V7_5:
- return "blacktop/elasticsearch:7.5.1";
+ return "blacktop/elasticsearch:7.5.2";
}
throw new IllegalStateException("No tests for version: " + version.name());
}
diff --git a/polygerrit-ui/README.md b/polygerrit-ui/README.md
index e645972..a35bd6f 100644
--- a/polygerrit-ui/README.md
+++ b/polygerrit-ui/README.md
@@ -109,7 +109,7 @@
For daily development you typically only want to run and debug individual tests.
Run the local [Go proxy server](#go-server) and navigate for example to
-<http://localhost:8081/elements/change/gr-account-entry/gr-account-entry_test.html>.
+<http://localhost:8081/elements/shared/gr-account-entry/gr-account-entry_test.html>.
Check "Disable cache" in the "Network" tab of Chrome's dev tools, so code
changes are picked up on "reload".
diff --git a/polygerrit-ui/app/elements/admin/gr-permission/gr-permission.js b/polygerrit-ui/app/elements/admin/gr-permission/gr-permission.js
index e719154..508c3a2 100644
--- a/polygerrit-ui/app/elements/admin/gr-permission/gr-permission.js
+++ b/polygerrit-ui/app/elements/admin/gr-permission/gr-permission.js
@@ -271,7 +271,12 @@
// API to work as expected.
const groupId = decodeURIComponent(e.detail.value.id)
.replace(/\+/g, ' ');
- this.set(['permission', 'value', 'rules', groupId], {});
+ // We cannot use "this.set(...)" here, because groupId may contain dots,
+ // and dots in property path names are totally unsupported by Polymer.
+ // Apparently Polymer picks up this change anyway, otherwise we should
+ // have looked at using MutableData:
+ // https://polymer-library.polymer-project.org/2.0/docs/devguide/data-system#mutable-data
+ this.permission.value.rules[groupId] = {};
// Purposely don't recompute sorted array so that the newly added rule
// is the last item of the array.
@@ -292,7 +297,8 @@
Polymer.dom.flush();
const value = this._rules[this._rules.length - 1].value;
value.added = true;
- this.set(['permission', 'value', 'rules', groupId], value);
+ // See comment above for why we cannot use "this.set(...)" here.
+ this.permission.value.rules[groupId] = value;
this.dispatchEvent(
new CustomEvent('access-modified', {bubbles: true, composed: true}));
}
diff --git a/polygerrit-ui/app/elements/admin/gr-permission/gr-permission_test.html b/polygerrit-ui/app/elements/admin/gr-permission/gr-permission_test.html
index 7919b28..9928b9e 100644
--- a/polygerrit-ui/app/elements/admin/gr-permission/gr-permission_test.html
+++ b/polygerrit-ui/app/elements/admin/gr-permission/gr-permission_test.html
@@ -312,11 +312,11 @@
element.name = 'Priority';
element.section = 'refs/*';
element.groups = {};
- element.$.groupAutocomplete.text = 'ldap/tests tests';
+ element.$.groupAutocomplete.text = 'ldap/tests te.st';
const e = {
detail: {
value: {
- id: 'ldap:CN=test+test',
+ id: 'ldap:CN=test+te.st',
},
},
};
@@ -325,11 +325,11 @@
assert.equal(Object.keys(element._groupsWithRules).length, 2);
element._handleAddRuleItem(e);
flushAsynchronousOperations();
- assert.deepEqual(element.groups, {'ldap:CN=test test': {
- name: 'ldap/tests tests'}});
+ assert.deepEqual(element.groups, {'ldap:CN=test te.st': {
+ name: 'ldap/tests te.st'}});
assert.equal(element._rules.length, 3);
assert.equal(Object.keys(element._groupsWithRules).length, 3);
- assert.deepEqual(element.permission.value.rules['ldap:CN=test test'],
+ assert.deepEqual(element.permission.value.rules['ldap:CN=test te.st'],
{action: 'ALLOW', min: -2, max: 2, added: true});
// New rule should be removed if cancel from editing.
element.editing = false;
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js
index a4ed899..c0a603e 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js
@@ -1166,6 +1166,14 @@
}
}
+ _handleRevertSubmissionDialogConfirm() {
+ const el = this.$.confirmRevertSubmissionDialog;
+ this.$.overlay.close();
+ el.hidden = true;
+ this._fireAction('/revert_submission', this.actions.revert_submission,
+ false, {message: el.message});
+ }
+
_handleAbandonDialogConfirm() {
const el = this.$.confirmAbandonDialog;
this.$.overlay.close();
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
index a44a7d2..02b017b 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
@@ -135,15 +135,15 @@
line-height: var(--line-height-mono);
margin-right: var(--spacing-l);
margin-bottom: var(--spacing-l);
- /* Account for border and padding */
- max-width: calc(72ch + 2px + 2*var(--spacing-m));
+ /* Account for border and padding and rounding errors. */
+ max-width: calc(72ch + 2px + 2*var(--spacing-m) + 0.4px);
}
.commitMessage gr-linked-text {
word-break: break-word;
}
#commitMessageEditor {
- /* Account for border and padding */
- min-width: calc(72ch + 2px + 2*var(--spacing-m));
+ /* Account for border and padding and rounding errors. */
+ min-width: calc(72ch + 2px + 2*var(--spacing-m) + 0.4px);
}
.editCommitMessage {
margin-top: var(--spacing-l);
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
index 5b0eb34..7536c2c 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
@@ -722,10 +722,12 @@
}
_handleReplySent(e) {
+ this.addEventListener('change-details-loaded',
+ () => {
+ this.$.reporting.timeEnd(SEND_REPLY_TIMING_LABEL);
+ }, {once: true});
this.$.replyOverlay.close();
- this._reload().then(() => {
- this.$.reporting.timeEnd(SEND_REPLY_TIMING_LABEL);
- });
+ this._reload();
}
_handleReplyCancel(e) {
@@ -1468,7 +1470,11 @@
// Resolves when the loading flag is set to false, meaning that some
// change content may start appearing.
const loadingFlagSet = detailCompletes
- .then(() => { this._loading = false; })
+ .then(() => {
+ this._loading = false;
+ this.dispatchEvent(new CustomEvent('change-details-loaded',
+ {bubbles: true, composed: true}));
+ })
.then(() => {
this.$.reporting.timeEnd(CHANGE_RELOAD_TIMING_LABEL);
if (opt_isLocationChange) {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
index ef22188..5d32e9b 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
@@ -815,10 +815,9 @@
// for each line from the start.
let lastEl;
for (const threadEl of addedThreadEls) {
- const lineNumString = threadEl.getAttribute('line-num') || 'FILE';
const commentSide = threadEl.getAttribute('comment-side');
- const lineEl = this.$.diffBuilder.getLineElByNumber(
- lineNumString, commentSide);
+ const lineEl = this._getLineElement(threadEl,
+ commentSide);
const contentText = this.$.diffBuilder.getContentByLineEl(lineEl);
const contentEl = contentText.parentElement;
const threadGroupEl = this._getOrCreateThreadGroup(
@@ -844,6 +843,18 @@
});
}
+ _getLineElement(threadEl, commentSide) {
+ const lineNumString = threadEl.getAttribute('line-num') || 'FILE';
+ const lineEl = this.$.diffBuilder.getLineElByNumber(
+ lineNumString, commentSide);
+ if (lineEl) {
+ return lineEl;
+ }
+ // It is possible to add comment to non-existing line via API
+ threadEl.invalidLineNumber = true;
+ return this.$.diffBuilder.getLineElByNumber('FILE', commentSide);
+ }
+
_unobserveIncrementalNodes() {
if (this._incrementalNodeObserver) {
Polymer.dom(this).unobserveNodes(this._incrementalNodeObserver);
diff --git a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.html b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.html
index cce1d19..7bc93b5 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.html
+++ b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.html
@@ -16,6 +16,8 @@
-->
<link rel="import" href="/bower_components/polymer/polymer.html">
+<link rel="import" href="/bower_components/iron-icon/iron-icon.html">
+<link rel="import" href="../../shared/gr-icons/gr-icons.html">
<link rel="import" href="../../../behaviors/fire-behavior/fire-behavior.html">
<link rel="import" href="../../../behaviors/gr-path-list-behavior/gr-path-list-behavior.html">
<link rel="import" href="../../../styles/shared-styles.html">
@@ -78,6 +80,18 @@
margin-left: var(--spacing-m);
font-style: italic;
}
+ .invalidLineNumber {
+ padding: var(--spacing-m);
+ }
+ .invalidLineNumberText {
+ color: var(--error-text-color);
+ }
+ .invalidLineNumberIcon {
+ color: var(--error-text-color);
+ vertical-align: top;
+ margin-right: var(--spacing-s);
+ }
+
</style>
<template is="dom-if" if="[[showFilePath]]">
<div class="pathInfo">
@@ -86,6 +100,11 @@
</div>
</template>
<div id="container" class$="[[_computeHostClass(unresolved, isRobotComment)]]">
+ <template is="dom-if" if="[[invalidLineNumber]]">
+ <div class="invalidLineNumber">
+ <span class="invalidLineNumberText"><iron-icon icon="gr-icons:error" class="invalidLineNumberIcon"></iron-icon>This comment thread is attached to non-existing line [[lineNum]].</span>
+ </div>
+ </template>
<template id="commentList" is="dom-repeat" items="[[_orderedComments]]"
as="comment">
<gr-comment
diff --git a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.js b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.js
index d8a56f8..00ff03a 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.js
+++ b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.js
@@ -124,6 +124,11 @@
type: Boolean,
value: false,
},
+ /** It is possible to add comment to non-existing line via API */
+ invalidLineNumber: {
+ type: Number,
+ reflectToAttribute: true,
+ },
/** Necessary only if showFilePath is true or when used with gr-diff */
lineNum: {
type: Number,
diff --git a/polygerrit-ui/app/elements/shared/gr-icons/gr-icons.html b/polygerrit-ui/app/elements/shared/gr-icons/gr-icons.html
index 743923b..5ecae00 100644
--- a/polygerrit-ui/app/elements/shared/gr-icons/gr-icons.html
+++ b/polygerrit-ui/app/elements/shared/gr-icons/gr-icons.html
@@ -54,6 +54,8 @@
<g id="info"><path d="M12 2C6.48 2 2 6.48 2 12s4.48 10 10 10 10-4.48 10-10S17.52 2 12 2zm1 15h-2v-6h2v6zm0-8h-2V7h2v2z"></path></g>
<!-- This SVG is a copy from material.io https://material.io/icons/#ic_hourglass_full-->
<g id="hourglass"><path d="M6 2v6h.01L6 8.01 10 12l-4 4 .01.01H6V22h12v-5.99h-.01L18 16l-4-4 4-3.99-.01-.01H18V2H6z"/><path d="M0 0h24v24H0V0z" fill="none"/></g>
+ <!-- This SVG is a copy from iron-icons https://github.com/PolymerElements/iron-icons/blob/master/iron-icons.js -->
+ <g id="error"><path d="M12 2C6.48 2 2 6.48 2 12s4.48 10 10 10 10-4.48 10-10S17.52 2 12 2zm1 15h-2v-2h2v2zm0-4h-2V7h2v6z"></path></g>
<!-- This is a custom PolyGerrit SVG -->
<g id="side-by-side"><path d="M17.1578947,10.8888889 L2.84210526,10.8888889 C2.37894737,10.8888889 2,11.2888889 2,11.7777778 L2,17.1111111 C2,17.6 2.37894737,18 2.84210526,18 L17.1578947,18 C17.6210526,18 18,17.6 18,17.1111111 L18,11.7777778 C18,11.2888889 17.6210526,10.8888889 17.1578947,10.8888889 Z M17.1578947,2 L2.84210526,2 C2.37894737,2 2,2.4 2,2.88888889 L2,8.22222222 C2,8.71111111 2.37894737,9.11111111 2.84210526,9.11111111 L17.1578947,9.11111111 C17.6210526,9.11111111 18,8.71111111 18,8.22222222 L18,2.88888889 C18,2.4 17.6210526,2 17.1578947,2 Z M16.1973628,2 L2.78874238,2 C2.35493407,2 2,2.4 2,2.88888889 L2,8.22222222 C2,8.71111111 2.35493407,9.11111111 2.78874238,9.11111111 L16.1973628,9.11111111 C16.6311711,9.11111111 16.9861052,8.71111111 16.9861052,8.22222222 L16.9861052,2.88888889 C16.9861052,2.4 16.6311711,2 16.1973628,2 Z" id="Shape" transform="scale(1.2) translate(10.000000, 10.000000) rotate(-90.000000) translate(-10.000000, -10.000000)"/></g>
<!-- This is a custom PolyGerrit SVG -->
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);
};
diff --git a/tools/nongoogle.bzl b/tools/nongoogle.bzl
index 3af4a28..acf01fa 100644
--- a/tools/nongoogle.bzl
+++ b/tools/nongoogle.bzl
@@ -102,8 +102,8 @@
# and httpasyncclient as necessary.
maven_jar(
name = "elasticsearch-rest-client",
- artifact = "org.elasticsearch.client:elasticsearch-rest-client:7.5.1",
- sha1 = "094c155906dc94146fc5adc344ea2c676d487cf2",
+ artifact = "org.elasticsearch.client:elasticsearch-rest-client:7.5.2",
+ sha1 = "e11393f600a425b7f62e6f653e19a9e53556fd79",
)
maven_jar(
