Merge "Converting to class and renaming gr-diff-builder element"
diff --git a/Documentation/rest-api-accounts.txt b/Documentation/rest-api-accounts.txt
index 28ecc97..10a2ff3 100644
--- a/Documentation/rest-api-accounts.txt
+++ b/Documentation/rest-api-accounts.txt
@@ -1728,6 +1728,10 @@
Retrieves the external ids of a user account.
+Only external ids belonging to the caller may be requested. Users that have
+link:access-control.html#capability_modifyAccount[Modify Account] can request
+external ids that belong to other accounts.
+
.Request
----
GET /a/accounts/self/external.ids HTTP/1.0
@@ -1761,7 +1765,9 @@
Delete a list of external ids for a user account. The target external ids must
be provided as a list in the request body.
-Only external ids belonging to the caller may be deleted.
+Only external ids belonging to the caller may be deleted. Users that have
+link:access-control.html#capability_modifyAccount[Modify Account] can delete
+external ids that belong to other accounts.
.Request
----
diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt
index 3b243b0..59af694 100644
--- a/Documentation/rest-api-changes.txt
+++ b/Documentation/rest-api-changes.txt
@@ -6266,7 +6266,7 @@
|`tag` |optional, drafts only|
Value of the `tag` field. Only allowed on link:#create-draft[draft comment] +
inputs; for published comments, use the `tag` field in +
-link#review-input[ReviewInput]. Votes/comments that contain `tag` with
+link:#review-input[ReviewInput]. Votes/comments that contain `tag` with
'autogenerated:' prefix can be filtered out in the web UI.
|`unresolved` |optional|
Whether or not the comment must be addressed by the user. This value will
diff --git a/java/com/google/gerrit/acceptance/AbstractDaemonTest.java b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
index ca105f6..cfee970 100644
--- a/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
+++ b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
@@ -1282,7 +1282,7 @@
}
protected void assertNotifyCc(TestAccount expected) {
- assertNotifyCc(expected.getEmailAddress());
+ assertNotifyCc(expected.getNameEmail());
}
protected void assertNotifyCc(String expectedEmail, String expectedFullname) {
@@ -1302,7 +1302,7 @@
protected void assertNotifyBcc(TestAccount expected) {
assertThat(sender.getMessages()).hasSize(1);
Message m = sender.getMessages().get(0);
- assertThat(m.rcpt()).containsExactly(expected.getEmailAddress());
+ assertThat(m.rcpt()).containsExactly(expected.getNameEmail());
assertThat(m.headers().get("To").isEmpty()).isTrue();
assertThat(m.headers().get("Cc").isEmpty()).isTrue();
}
diff --git a/java/com/google/gerrit/acceptance/AbstractNotificationTest.java b/java/com/google/gerrit/acceptance/AbstractNotificationTest.java
index a372089..e2dd31c 100644
--- a/java/com/google/gerrit/acceptance/AbstractNotificationTest.java
+++ b/java/com/google/gerrit/acceptance/AbstractNotificationTest.java
@@ -407,14 +407,14 @@
public TestAccount testAccount(String name) throws Exception {
String username = name(name);
- TestAccount account = accountCreator.create(username, email(username), name);
+ TestAccount account = accountCreator.create(username, email(username), name, null);
accountsByEmail.put(account.email(), account);
return account;
}
public TestAccount testAccount(String name, String groupName) throws Exception {
String username = name(name);
- TestAccount account = accountCreator.create(username, email(username), name, groupName);
+ TestAccount account = accountCreator.create(username, email(username), name, null, groupName);
accountsByEmail.put(account.email(), account);
return account;
}
diff --git a/java/com/google/gerrit/acceptance/AccountCreator.java b/java/com/google/gerrit/acceptance/AccountCreator.java
index 75d0d2f..44e2d2d 100644
--- a/java/com/google/gerrit/acceptance/AccountCreator.java
+++ b/java/com/google/gerrit/acceptance/AccountCreator.java
@@ -69,6 +69,7 @@
@Nullable String username,
@Nullable String email,
@Nullable String fullName,
+ @Nullable String displayName,
String... groupNames)
throws Exception {
@@ -94,7 +95,11 @@
.insert(
"Create Test Account",
id,
- u -> u.setFullName(fullName).setPreferredEmail(email).addExternalIds(extIds));
+ u ->
+ u.setFullName(fullName)
+ .setDisplayName(displayName)
+ .setPreferredEmail(email)
+ .addExternalIds(extIds));
if (groupNames != null) {
for (String n : groupNames) {
@@ -107,7 +112,7 @@
}
}
- account = TestAccount.create(id, username, email, fullName, httpPass);
+ account = TestAccount.create(id, username, email, fullName, displayName, httpPass);
if (username != null) {
accounts.put(username, account);
}
@@ -115,7 +120,7 @@
}
public TestAccount create(@Nullable String username, String group) throws Exception {
- return create(username, null, username, group);
+ return create(username, null, username, null, group);
}
public TestAccount create() throws Exception {
@@ -123,23 +128,23 @@
}
public TestAccount create(@Nullable String username) throws Exception {
- return create(username, null, username, (String[]) null);
+ return create(username, null, username, null, (String[]) null);
}
public TestAccount admin() throws Exception {
- return create("admin", "admin@example.com", "Administrator", "Administrators");
+ return create("admin", "admin@example.com", "Administrator", "Adminny", "Administrators");
}
public TestAccount admin2() throws Exception {
- return create("admin2", "admin2@example.com", "Administrator2", "Administrators");
+ return create("admin2", "admin2@example.com", "Administrator2", null, "Administrators");
}
public TestAccount user() throws Exception {
- return create("user", "user@example.com", "User");
+ return create("user", "user@example.com", "User", null);
}
public TestAccount user2() throws Exception {
- return create("user2", "user2@example.com", "User2");
+ return create("user2", "user2@example.com", "User2", null);
}
public TestAccount get(String username) {
diff --git a/java/com/google/gerrit/acceptance/TestAccount.java b/java/com/google/gerrit/acceptance/TestAccount.java
index 07bb739..87ce70a 100644
--- a/java/com/google/gerrit/acceptance/TestAccount.java
+++ b/java/com/google/gerrit/acceptance/TestAccount.java
@@ -47,8 +47,9 @@
@Nullable String username,
@Nullable String email,
@Nullable String fullName,
+ @Nullable String displayName,
@Nullable String httpPassword) {
- return new AutoValue_TestAccount(id, username, email, fullName, httpPassword);
+ return new AutoValue_TestAccount(id, username, email, fullName, displayName, httpPassword);
}
public abstract Account.Id id();
@@ -63,6 +64,9 @@
public abstract String fullName();
@Nullable
+ public abstract String displayName();
+
+ @Nullable
public abstract String httpPassword();
public PersonIdent newIdent() {
@@ -79,7 +83,7 @@
.toString();
}
- public Address getEmailAddress() {
+ public Address getNameEmail() {
// Address is weird enough that it's safer and clearer to create a new instance in a
// non-abstract method rather than, say, having an abstract emailAddress() as part of this
// AutoValue class. Specifically:
diff --git a/java/com/google/gerrit/httpd/raw/IndexHtmlUtil.java b/java/com/google/gerrit/httpd/raw/IndexHtmlUtil.java
index 43470af..b6375c8 100644
--- a/java/com/google/gerrit/httpd/raw/IndexHtmlUtil.java
+++ b/java/com/google/gerrit/httpd/raw/IndexHtmlUtil.java
@@ -186,12 +186,6 @@
if (urlParameterMap.containsKey("ce")) {
data.put("polyfillCE", "true");
}
- if (urlParameterMap.containsKey("sd")) {
- data.put("polyfillSD", "true");
- }
- if (urlParameterMap.containsKey("sc")) {
- data.put("polyfillSC", "true");
- }
if (urlParameterMap.containsKey("gf")) {
data.put("useGoogleFonts", "true");
}
diff --git a/java/com/google/gerrit/httpd/restapi/RestApiServlet.java b/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
index b32da89..89ebdc1 100644
--- a/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
+++ b/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
@@ -328,10 +328,10 @@
try (TraceContext traceContext = enableTracing(req, res)) {
List<IdString> path = splitPath(req);
- RequestInfo requestInfo = createRequestInfo(traceContext, requestUri(req), path);
- globals.requestListeners.runEach(l -> l.onRequest(requestInfo));
-
try (PerThreadCache ignored = PerThreadCache.create()) {
+ RequestInfo requestInfo = createRequestInfo(traceContext, requestUri(req), path);
+ globals.requestListeners.runEach(l -> l.onRequest(requestInfo));
+
// It's important that the PerformanceLogContext is closed before the response is sent to
// the client. Only this way it is ensured that the invocation of the PerformanceLogger
// plugins happens before the client sees the response. This is needed for being able to
@@ -1702,9 +1702,9 @@
if (rootCollection instanceof ProjectsCollection) {
requestInfo.project(Project.nameKey(resourceId));
} else if (rootCollection instanceof ChangesCollection) {
- ChangeNotes changeNotes = globals.changeFinder.findOne(resourceId);
- if (changeNotes != null) {
- requestInfo.project(changeNotes.getProjectName());
+ Optional<ChangeNotes> changeNotes = globals.changeFinder.findOne(resourceId);
+ if (changeNotes.isPresent()) {
+ requestInfo.project(changeNotes.get().getProjectName());
}
}
return requestInfo.build();
diff --git a/java/com/google/gerrit/mail/Address.java b/java/com/google/gerrit/mail/Address.java
index 24ab353..455cc90 100644
--- a/java/com/google/gerrit/mail/Address.java
+++ b/java/com/google/gerrit/mail/Address.java
@@ -16,6 +16,7 @@
import com.google.gerrit.common.Nullable;
+/** Represents an address (name + email) in an email message. */
public class Address {
public static Address parse(String in) {
final int lt = in.indexOf('<');
diff --git a/java/com/google/gerrit/pgm/http/jetty/BUILD b/java/com/google/gerrit/pgm/http/jetty/BUILD
index 6ceb242..cd188f5 100644
--- a/java/com/google/gerrit/pgm/http/jetty/BUILD
+++ b/java/com/google/gerrit/pgm/http/jetty/BUILD
@@ -10,10 +10,10 @@
"//java/com/google/gerrit/lifecycle",
"//java/com/google/gerrit/metrics",
"//java/com/google/gerrit/server",
- "//java/com/google/gerrit/server/logging",
"//java/com/google/gerrit/server/util/time",
"//java/com/google/gerrit/sshd",
"//java/com/google/gerrit/util/http",
+ "//java/com/google/gerrit/util/logging",
"//lib:gson",
"//lib:guava",
"//lib:jgit",
diff --git a/java/com/google/gerrit/pgm/http/jetty/HttpLogJsonLayout.java b/java/com/google/gerrit/pgm/http/jetty/HttpLogJsonLayout.java
index 33916ac..73d9ee4 100644
--- a/java/com/google/gerrit/pgm/http/jetty/HttpLogJsonLayout.java
+++ b/java/com/google/gerrit/pgm/http/jetty/HttpLogJsonLayout.java
@@ -24,8 +24,8 @@
import static com.google.gerrit.pgm.http.jetty.HttpLog.P_USER;
import static com.google.gerrit.pgm.http.jetty.HttpLog.P_USER_AGENT;
-import com.google.gerrit.server.logging.JsonLayout;
-import com.google.gerrit.server.logging.JsonLogEntry;
+import com.google.gerrit.util.logging.JsonLayout;
+import com.google.gerrit.util.logging.JsonLogEntry;
import java.time.format.DateTimeFormatter;
import org.apache.log4j.spi.LoggingEvent;
diff --git a/java/com/google/gerrit/server/account/AccountDirectory.java b/java/com/google/gerrit/server/account/AccountDirectory.java
index 60c1678..63fa551 100644
--- a/java/com/google/gerrit/server/account/AccountDirectory.java
+++ b/java/com/google/gerrit/server/account/AccountDirectory.java
@@ -26,7 +26,7 @@
public abstract class AccountDirectory {
/** Fields to be populated for a REST API response. */
public enum FillOptions {
- /** Human friendly display name presented in the web interface. */
+ /** Full name or username. */
NAME,
/** Preferred email address to contact the user at. */
@@ -48,7 +48,10 @@
STATUS,
/** The state of the account (e.g. active or inactive) */
- STATE
+ STATE,
+
+ /** Human friendly display name presented in the web interface chosen by the user. */
+ DISPLAY_NAME
}
public abstract void fillAccountInfo(Iterable<? extends AccountInfo> in, Set<FillOptions> options)
diff --git a/java/com/google/gerrit/server/account/AccountLoader.java b/java/com/google/gerrit/server/account/AccountLoader.java
index a8e4194..9acf078 100644
--- a/java/com/google/gerrit/server/account/AccountLoader.java
+++ b/java/com/google/gerrit/server/account/AccountLoader.java
@@ -41,6 +41,7 @@
FillOptions.NAME,
FillOptions.EMAIL,
FillOptions.USERNAME,
+ FillOptions.DISPLAY_NAME,
FillOptions.STATUS,
FillOptions.STATE,
FillOptions.AVATARS));
diff --git a/java/com/google/gerrit/server/account/CreateGroupArgs.java b/java/com/google/gerrit/server/account/CreateGroupArgs.java
index 2a764cc..ba58c3f 100644
--- a/java/com/google/gerrit/server/account/CreateGroupArgs.java
+++ b/java/com/google/gerrit/server/account/CreateGroupArgs.java
@@ -35,10 +35,6 @@
}
public void setGroupName(String n) {
- groupName = n != null ? AccountGroup.nameKey(n) : null;
- }
-
- public void setGroupName(AccountGroup.NameKey n) {
- groupName = n;
+ groupName = n != null ? AccountGroup.nameKey(n.trim()) : null;
}
}
diff --git a/java/com/google/gerrit/server/account/InternalAccountDirectory.java b/java/com/google/gerrit/server/account/InternalAccountDirectory.java
index e27b77c..3137c95 100644
--- a/java/com/google/gerrit/server/account/InternalAccountDirectory.java
+++ b/java/com/google/gerrit/server/account/InternalAccountDirectory.java
@@ -143,6 +143,10 @@
info.username = accountState.userName().orElse(null);
}
+ if (options.contains(FillOptions.DISPLAY_NAME)) {
+ info.displayName = account.displayName();
+ }
+
if (options.contains(FillOptions.STATUS)) {
info.status = account.status();
}
diff --git a/java/com/google/gerrit/server/account/StoredPreferences.java b/java/com/google/gerrit/server/account/StoredPreferences.java
index 0e8eb04..3591453c 100644
--- a/java/com/google/gerrit/server/account/StoredPreferences.java
+++ b/java/com/google/gerrit/server/account/StoredPreferences.java
@@ -386,7 +386,7 @@
my = my(defaultCfg);
}
if (my.isEmpty()) {
- my.add(new MenuItem("Changes", "#/dashboard/self", null));
+ my.add(new MenuItem("Dashboard", "#/dashboard/self", null));
my.add(new MenuItem("Draft Comments", "#/q/has:draft", null));
my.add(new MenuItem("Edits", "#/q/has:edit", null));
my.add(new MenuItem("Watched Changes", "#/q/is:watched+is:open", null));
diff --git a/java/com/google/gerrit/server/change/ChangeFinder.java b/java/com/google/gerrit/server/change/ChangeFinder.java
index da3650d..f2b6dd6 100644
--- a/java/com/google/gerrit/server/change/ChangeFinder.java
+++ b/java/com/google/gerrit/server/change/ChangeFinder.java
@@ -19,6 +19,7 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Sets;
+import com.google.common.flogger.FluentLogger;
import com.google.common.primitives.Ints;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.Project;
@@ -54,6 +55,8 @@
@Singleton
public class ChangeFinder {
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+
private static final String CACHE_NAME = "changeid_project";
public static Module module() {
@@ -108,12 +111,12 @@
this.allowedIdTypes = ImmutableSet.copyOf(configuredChangeIdTypes);
}
- public ChangeNotes findOne(String id) {
+ public Optional<ChangeNotes> findOne(String id) {
List<ChangeNotes> ctls = find(id);
if (ctls.size() != 1) {
- return null;
+ return Optional.empty();
}
- return ctls.get(0);
+ return Optional.of(ctls.get(0));
}
/**
@@ -211,12 +214,12 @@
}
}
- public ChangeNotes findOne(Change.Id id) {
+ public Optional<ChangeNotes> findOne(Change.Id id) {
List<ChangeNotes> notes = find(id);
if (notes.size() != 1) {
- throw new NoSuchChangeException(id);
+ return Optional.empty();
}
- return notes.get(0);
+ return Optional.of(notes.get(0));
}
public List<ChangeNotes> find(Change.Id id) {
@@ -239,7 +242,11 @@
List<ChangeNotes> notes = new ArrayList<>(cds.size());
if (!indexConfig.separateChangeSubIndexes()) {
for (ChangeData cd : cds) {
- notes.add(cd.notes());
+ try {
+ notes.add(cd.notes());
+ } catch (NoSuchChangeException e) {
+ logger.atWarning().log("Change %s seen in index, but missing in NoteDb", e.getMessage());
+ }
}
return notes;
}
@@ -253,7 +260,11 @@
Set<Change.Id> seen = Sets.newHashSetWithExpectedSize(cds.size());
for (ChangeData cd : cds) {
if (seen.add(cd.getId())) {
- notes.add(cd.notes());
+ try {
+ notes.add(cd.notes());
+ } catch (NoSuchChangeException e) {
+ logger.atWarning().log("Change %s seen in index, but missing in NoteDb", e.getMessage());
+ }
}
}
return notes;
diff --git a/java/com/google/gerrit/server/git/MergeUtil.java b/java/com/google/gerrit/server/git/MergeUtil.java
index 39fd103..89a2036 100644
--- a/java/com/google/gerrit/server/git/MergeUtil.java
+++ b/java/com/google/gerrit/server/git/MergeUtil.java
@@ -21,7 +21,6 @@
import static com.google.gerrit.git.ObjectIds.abbreviateName;
import static java.nio.charset.StandardCharsets.UTF_8;
import static java.util.Comparator.naturalOrder;
-import static java.util.Objects.requireNonNull;
import static java.util.stream.Collectors.joining;
import com.google.common.base.Strings;
@@ -44,8 +43,6 @@
import com.google.gerrit.exceptions.MergeWithConflictsNotSupportedException;
import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.extensions.registration.DynamicItem;
-import com.google.gerrit.extensions.registration.DynamicSet;
-import com.google.gerrit.extensions.registration.Extension;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.MergeConflictException;
import com.google.gerrit.extensions.restapi.MethodNotAllowedException;
@@ -62,7 +59,6 @@
import com.google.gerrit.server.submit.IntegrationException;
import com.google.gerrit.server.submit.MergeIdenticalTreeException;
import com.google.gerrit.server.submit.MergeSorter;
-import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
import com.google.inject.assistedinject.AssistedInject;
import java.io.IOException;
@@ -128,44 +124,6 @@
*/
private static final int NAME_ABBREV_LEN = 6;
- static class PluggableCommitMessageGenerator {
- private final DynamicSet<ChangeMessageModifier> changeMessageModifiers;
-
- @Inject
- PluggableCommitMessageGenerator(DynamicSet<ChangeMessageModifier> changeMessageModifiers) {
- this.changeMessageModifiers = changeMessageModifiers;
- }
-
- public String generate(
- RevCommit original, RevCommit mergeTip, BranchNameKey dest, String originalMessage) {
- requireNonNull(original.getRawBuffer());
- if (mergeTip != null) {
- requireNonNull(mergeTip.getRawBuffer());
- }
-
- int count = 0;
- String current = originalMessage;
- for (Extension<ChangeMessageModifier> ext : changeMessageModifiers.entries()) {
- ChangeMessageModifier changeMessageModifier = ext.get();
- String className = changeMessageModifier.getClass().getName();
- current = changeMessageModifier.onSubmit(current, original, mergeTip, dest);
- checkState(
- current != null,
- "%s.onSubmit from plugin %s returned null instead of new commit message",
- className,
- ext.getPluginName());
- count++;
- logger.atFine().log(
- "Invoked %s from plugin %s, message length now %d",
- className, ext.getPluginName(), current.length());
- }
- logger.atFine().log(
- "Invoked %d ChangeMessageModifiers on message with original length %d",
- count, originalMessage.length());
- return current;
- }
- }
-
private static final String R_HEADS_MASTER = Constants.R_HEADS + Constants.MASTER;
public static boolean useRecursiveMerge(Config cfg) {
@@ -432,7 +390,33 @@
RevCommit originalCommit,
String mergeStrategy,
boolean allowConflicts,
- PersonIdent committerIndent,
+ PersonIdent committerIdent,
+ String commitMsg,
+ CodeReviewRevWalk rw)
+ throws IOException, MergeIdenticalTreeException, MergeConflictException,
+ InvalidMergeStrategyException {
+ return createMergeCommit(
+ inserter,
+ repoConfig,
+ mergeTip,
+ originalCommit,
+ mergeStrategy,
+ allowConflicts,
+ committerIdent,
+ committerIdent,
+ commitMsg,
+ rw);
+ }
+
+ public static CodeReviewCommit createMergeCommit(
+ ObjectInserter inserter,
+ Config repoConfig,
+ RevCommit mergeTip,
+ RevCommit originalCommit,
+ String mergeStrategy,
+ boolean allowConflicts,
+ PersonIdent authorIdent,
+ PersonIdent committerIdent,
String commitMsg,
CodeReviewRevWalk rw)
throws IOException, MergeIdenticalTreeException, MergeConflictException,
@@ -496,8 +480,8 @@
CommitBuilder mergeCommit = new CommitBuilder();
mergeCommit.setTreeId(tree);
mergeCommit.setParentIds(mergeTip, originalCommit);
- mergeCommit.setAuthor(committerIndent);
- mergeCommit.setCommitter(committerIndent);
+ mergeCommit.setAuthor(authorIdent);
+ mergeCommit.setCommitter(committerIdent);
mergeCommit.setMessage(commitMsg);
CodeReviewCommit commit = rw.parseCommit(inserter.insert(mergeCommit));
commit.setFilesWithGitConflicts(filesWithGitConflicts);
diff --git a/java/com/google/gerrit/server/git/PluggableCommitMessageGenerator.java b/java/com/google/gerrit/server/git/PluggableCommitMessageGenerator.java
new file mode 100644
index 0000000..804a218
--- /dev/null
+++ b/java/com/google/gerrit/server/git/PluggableCommitMessageGenerator.java
@@ -0,0 +1,71 @@
+// 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.server.git;
+
+import static com.google.common.base.Preconditions.checkState;
+import static java.util.Objects.requireNonNull;
+
+import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.entities.BranchNameKey;
+import com.google.gerrit.extensions.registration.DynamicSet;
+import com.google.gerrit.extensions.registration.Extension;
+import com.google.inject.Inject;
+import org.eclipse.jgit.revwalk.RevCommit;
+
+/** Helper to call plugins that want to change the commit message before a change is merged. */
+public class PluggableCommitMessageGenerator {
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+
+ private final DynamicSet<ChangeMessageModifier> changeMessageModifiers;
+
+ @Inject
+ PluggableCommitMessageGenerator(DynamicSet<ChangeMessageModifier> changeMessageModifiers) {
+ this.changeMessageModifiers = changeMessageModifiers;
+ }
+
+ /**
+ * Returns the commit message as modified by plugins. The returned message can be equal to {@code
+ * originalMessage} in case no plugins are registered or the registered plugins decided not to
+ * modify the message.
+ */
+ public String generate(
+ RevCommit original, RevCommit mergeTip, BranchNameKey dest, String originalMessage) {
+ requireNonNull(original.getRawBuffer());
+ if (mergeTip != null) {
+ requireNonNull(mergeTip.getRawBuffer());
+ }
+
+ int count = 0;
+ String current = originalMessage;
+ for (Extension<ChangeMessageModifier> ext : changeMessageModifiers.entries()) {
+ ChangeMessageModifier changeMessageModifier = ext.get();
+ String className = changeMessageModifier.getClass().getName();
+ current = changeMessageModifier.onSubmit(current, original, mergeTip, dest);
+ checkState(
+ current != null,
+ "%s.onSubmit from plugin %s returned null instead of new commit message",
+ className,
+ ext.getPluginName());
+ count++;
+ logger.atFine().log(
+ "Invoked %s from plugin %s, message length now %d",
+ className, ext.getPluginName(), current.length());
+ }
+ logger.atFine().log(
+ "Invoked %d ChangeMessageModifiers on message with original length %d",
+ count, originalMessage.length());
+ return current;
+ }
+}
diff --git a/java/com/google/gerrit/server/query/account/AccountQueryBuilder.java b/java/com/google/gerrit/server/query/account/AccountQueryBuilder.java
index 42a8310..4a95b2e 100644
--- a/java/com/google/gerrit/server/query/account/AccountQueryBuilder.java
+++ b/java/com/google/gerrit/server/query/account/AccountQueryBuilder.java
@@ -41,6 +41,7 @@
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.ProvisionException;
+import java.util.Optional;
/** Parses a query string meant to be applied to account objects. */
public class AccountQueryBuilder extends QueryBuilder<AccountState, AccountQueryBuilder> {
@@ -115,20 +116,23 @@
@Operator
public Predicate<AccountState> cansee(String change)
throws QueryParseException, PermissionBackendException {
- ChangeNotes changeNotes = args.changeFinder.findOne(change);
- if (changeNotes == null) {
+ Optional<ChangeNotes> changeNotes = args.changeFinder.findOne(change);
+ if (!changeNotes.isPresent()) {
throw error(String.format("change %s not found", change));
}
try {
- args.permissionBackend.user(args.getUser()).change(changeNotes).check(ChangePermission.READ);
+ args.permissionBackend
+ .user(args.getUser())
+ .change(changeNotes.get())
+ .check(ChangePermission.READ);
} catch (AuthException e) {
String msg = String.format("change %s not found", change);
logger.atSevere().withCause(e).log(msg);
throw error(msg);
}
- return AccountPredicates.cansee(args, changeNotes);
+ return AccountPredicates.cansee(args, changeNotes.get());
}
@Operator
diff --git a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
index f8610db..7401657 100644
--- a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
+++ b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
@@ -703,7 +703,7 @@
}
@Operator
- public Predicate<ChangeData> branch(String name) {
+ public Predicate<ChangeData> branch(String name) throws QueryParseException {
if (name.startsWith("^")) {
return ref("^" + RefNames.fullName(name.substring(1)));
}
@@ -732,7 +732,7 @@
}
@Operator
- public Predicate<ChangeData> ref(String ref) {
+ public Predicate<ChangeData> ref(String ref) throws QueryParseException {
if (ref.startsWith("^")) {
return new RegexRefPredicate(ref);
}
diff --git a/java/com/google/gerrit/server/query/change/RegexRefPredicate.java b/java/com/google/gerrit/server/query/change/RegexRefPredicate.java
index 6d404b4..b2dba72 100644
--- a/java/com/google/gerrit/server/query/change/RegexRefPredicate.java
+++ b/java/com/google/gerrit/server/query/change/RegexRefPredicate.java
@@ -15,6 +15,7 @@
package com.google.gerrit.server.query.change;
import com.google.gerrit.entities.Change;
+import com.google.gerrit.index.query.QueryParseException;
import com.google.gerrit.server.index.change.ChangeField;
import dk.brics.automaton.RegExp;
import dk.brics.automaton.RunAutomaton;
@@ -22,7 +23,7 @@
public class RegexRefPredicate extends ChangeRegexPredicate {
protected final RunAutomaton pattern;
- public RegexRefPredicate(String re) {
+ public RegexRefPredicate(String re) throws QueryParseException {
super(ChangeField.REF, re);
if (re.startsWith("^")) {
@@ -33,7 +34,11 @@
re = re.substring(0, re.length() - 1);
}
- this.pattern = new RunAutomaton(new RegExp(re).toAutomaton());
+ try {
+ this.pattern = new RunAutomaton(new RegExp(re).toAutomaton());
+ } catch (IllegalArgumentException e) {
+ throw new QueryParseException(String.format("invalid regular expression: %s", re), e);
+ }
}
@Override
diff --git a/java/com/google/gerrit/server/restapi/account/DeleteExternalIds.java b/java/com/google/gerrit/server/restapi/account/DeleteExternalIds.java
index 80f0bb1..445a5d6 100644
--- a/java/com/google/gerrit/server/restapi/account/DeleteExternalIds.java
+++ b/java/com/google/gerrit/server/restapi/account/DeleteExternalIds.java
@@ -73,7 +73,7 @@
public Response<?> apply(AccountResource resource, List<String> extIds)
throws RestApiException, IOException, ConfigInvalidException, PermissionBackendException {
if (!self.get().hasSameAccountId(resource.getUser())) {
- permissionBackend.currentUser().check(GlobalPermission.ACCESS_DATABASE);
+ permissionBackend.currentUser().check(GlobalPermission.MODIFY_ACCOUNT);
}
if (extIds == null || extIds.isEmpty()) {
diff --git a/java/com/google/gerrit/server/restapi/account/GetExternalIds.java b/java/com/google/gerrit/server/restapi/account/GetExternalIds.java
index c5b4454..a3c48b9 100644
--- a/java/com/google/gerrit/server/restapi/account/GetExternalIds.java
+++ b/java/com/google/gerrit/server/restapi/account/GetExternalIds.java
@@ -67,7 +67,7 @@
public Response<List<AccountExternalIdInfo>> apply(AccountResource resource)
throws RestApiException, IOException, PermissionBackendException {
if (!self.get().hasSameAccountId(resource.getUser())) {
- permissionBackend.currentUser().check(GlobalPermission.ACCESS_DATABASE);
+ permissionBackend.currentUser().check(GlobalPermission.MODIFY_ACCOUNT);
}
Collection<ExternalId> ids = externalIds.byAccount(resource.getUser().getAccountId());
diff --git a/java/com/google/gerrit/server/restapi/change/CreateChange.java b/java/com/google/gerrit/server/restapi/change/CreateChange.java
index a6a1eef..9e792d0 100644
--- a/java/com/google/gerrit/server/restapi/change/CreateChange.java
+++ b/java/com/google/gerrit/server/restapi/change/CreateChange.java
@@ -335,9 +335,10 @@
Timestamp now = TimeUtil.nowTs();
+ PersonIdent committer = me.newCommitterIdent(now, serverTimeZone);
PersonIdent author =
input.author == null
- ? me.newCommitterIdent(now, serverTimeZone)
+ ? committer
: new PersonIdent(input.author.name, input.author.email, now, serverTimeZone);
String commitMessage = getCommitMessage(input.subject, me);
@@ -345,7 +346,9 @@
CodeReviewCommit c;
if (input.merge != null) {
// create a merge commit
- c = newMergeCommit(git, oi, rw, projectState, mergeTip, input.merge, author, commitMessage);
+ c =
+ newMergeCommit(
+ git, oi, rw, projectState, mergeTip, input.merge, author, committer, commitMessage);
if (!c.getFilesWithGitConflicts().isEmpty()) {
logger.atFine().log(
"merge commit has conflicts in the following files: %s",
@@ -353,7 +356,7 @@
}
} else {
// create an empty commit
- c = newCommit(oi, rw, author, mergeTip, commitMessage);
+ c = newCommit(oi, rw, author, committer, mergeTip, commitMessage);
}
Change.Id changeId = Change.id(seq.nextChangeId());
@@ -495,6 +498,7 @@
ObjectInserter oi,
CodeReviewRevWalk rw,
PersonIdent authorIdent,
+ PersonIdent committerIdent,
RevCommit mergeTip,
String commitMessage)
throws IOException {
@@ -507,7 +511,7 @@
commit.setParentId(mergeTip);
}
commit.setAuthor(authorIdent);
- commit.setCommitter(authorIdent);
+ commit.setCommitter(committerIdent);
commit.setMessage(commitMessage);
return rw.parseCommit(insert(oi, commit));
}
@@ -520,6 +524,7 @@
RevCommit mergeTip,
MergeInput merge,
PersonIdent authorIdent,
+ PersonIdent committerIdent,
String commitMessage)
throws RestApiException, IOException {
logger.atFine().log(
@@ -556,6 +561,7 @@
mergeStrategy,
merge.allowConflicts,
authorIdent,
+ committerIdent,
commitMessage,
rw);
} catch (NoMergeBaseException e) {
diff --git a/java/com/google/gerrit/sshd/BUILD b/java/com/google/gerrit/sshd/BUILD
index f567a3a..a5b88b4 100644
--- a/java/com/google/gerrit/sshd/BUILD
+++ b/java/com/google/gerrit/sshd/BUILD
@@ -22,6 +22,7 @@
"//java/com/google/gerrit/server/restapi",
"//java/com/google/gerrit/server/util/time",
"//java/com/google/gerrit/util/cli",
+ "//java/com/google/gerrit/util/logging",
"//lib:args4j",
"//lib:gson",
"//lib:guava",
diff --git a/java/com/google/gerrit/sshd/SshLogJsonLayout.java b/java/com/google/gerrit/sshd/SshLogJsonLayout.java
index 8495ed1..5ece5af 100644
--- a/java/com/google/gerrit/sshd/SshLogJsonLayout.java
+++ b/java/com/google/gerrit/sshd/SshLogJsonLayout.java
@@ -23,8 +23,8 @@
import static com.google.gerrit.sshd.SshLog.P_USER_NAME;
import static com.google.gerrit.sshd.SshLog.P_WAIT;
-import com.google.gerrit.server.logging.JsonLayout;
-import com.google.gerrit.server.logging.JsonLogEntry;
+import com.google.gerrit.util.logging.JsonLayout;
+import com.google.gerrit.util.logging.JsonLogEntry;
import java.time.format.DateTimeFormatter;
import org.apache.log4j.spi.LoggingEvent;
diff --git a/java/com/google/gerrit/sshd/commands/PatchSetParser.java b/java/com/google/gerrit/sshd/commands/PatchSetParser.java
index f804c2d..4ebf15e 100644
--- a/java/com/google/gerrit/sshd/commands/PatchSetParser.java
+++ b/java/com/google/gerrit/sshd/commands/PatchSetParser.java
@@ -22,7 +22,6 @@
import com.google.gerrit.server.PatchSetUtil;
import com.google.gerrit.server.change.ChangeFinder;
import com.google.gerrit.server.notedb.ChangeNotes;
-import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gerrit.server.project.ProjectState;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.query.change.InternalChangeQuery;
@@ -32,6 +31,7 @@
import com.google.inject.Singleton;
import java.util.ArrayList;
import java.util.List;
+import java.util.Optional;
@Singleton
public class PatchSetParser {
@@ -126,12 +126,11 @@
if (projectState != null) {
return notesFactory.create(projectState.getNameKey(), changeId);
}
- try {
- ChangeNotes notes = changeFinder.findOne(changeId);
- return notesFactory.create(notes.getProjectName(), changeId);
- } catch (NoSuchChangeException e) {
- throw error("\"" + changeId + "\" no such change", e);
+ Optional<ChangeNotes> notes = changeFinder.findOne(changeId);
+ if (!notes.isPresent()) {
+ throw error("\"" + changeId + "\" no such change");
}
+ return notesFactory.create(notes.get().getProjectName(), changeId);
}
private static boolean inProject(Change change, ProjectState projectState) {
diff --git a/java/com/google/gerrit/util/logging/BUILD b/java/com/google/gerrit/util/logging/BUILD
new file mode 100644
index 0000000..b8db49bd
--- /dev/null
+++ b/java/com/google/gerrit/util/logging/BUILD
@@ -0,0 +1,13 @@
+load("@rules_java//java:defs.bzl", "java_library")
+
+java_library(
+ name = "logging",
+ srcs = glob(
+ ["*.java"],
+ ),
+ visibility = ["//visibility:public"],
+ deps = [
+ "//lib:gson",
+ "//lib/log:log4j",
+ ],
+)
diff --git a/java/com/google/gerrit/server/logging/JsonLayout.java b/java/com/google/gerrit/util/logging/JsonLayout.java
similarity index 97%
rename from java/com/google/gerrit/server/logging/JsonLayout.java
rename to java/com/google/gerrit/util/logging/JsonLayout.java
index ce3473f..45fbd32 100644
--- a/java/com/google/gerrit/server/logging/JsonLayout.java
+++ b/java/com/google/gerrit/util/logging/JsonLayout.java
@@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.
-package com.google.gerrit.server.logging;
+package com.google.gerrit.util.logging;
import com.google.gson.FieldNamingPolicy;
import com.google.gson.Gson;
diff --git a/java/com/google/gerrit/server/logging/JsonLogEntry.java b/java/com/google/gerrit/util/logging/JsonLogEntry.java
similarity index 94%
rename from java/com/google/gerrit/server/logging/JsonLogEntry.java
rename to java/com/google/gerrit/util/logging/JsonLogEntry.java
index bc16c70..ceac427 100644
--- a/java/com/google/gerrit/server/logging/JsonLogEntry.java
+++ b/java/com/google/gerrit/util/logging/JsonLogEntry.java
@@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.
-package com.google.gerrit.server.logging;
+package com.google.gerrit.util.logging;
import org.apache.log4j.spi.LoggingEvent;
diff --git a/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java b/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java
index 8ef49a1..f1a2f40 100644
--- a/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java
@@ -857,7 +857,7 @@
gApi.changes().id(r.getChangeId()).abandon();
List<Message> messages = sender.getMessages();
assertThat(messages).hasSize(1);
- assertThat(messages.get(0).rcpt()).containsExactly(user2.getEmailAddress());
+ assertThat(messages.get(0).rcpt()).containsExactly(user2.getNameEmail());
accountIndexedCounter.assertNoReindex();
}
}
@@ -883,7 +883,7 @@
List<Message> messages = sender.getMessages();
assertThat(messages).hasSize(1);
Message message = messages.get(0);
- assertThat(message.rcpt()).containsExactly(user.getEmailAddress());
+ assertThat(message.rcpt()).containsExactly(user.getNameEmail());
assertMailReplyTo(message, admin.email());
accountIndexedCounter.assertNoReindex();
}
@@ -913,7 +913,7 @@
String email = "preferred@example.com";
String name = "Foo";
String username = name("foo");
- TestAccount foo = accountCreator.create(username, email, name);
+ TestAccount foo = accountCreator.create(username, email, name, null);
String secondaryEmail = "secondary@example.com";
EmailInput input = newEmailInput(secondaryEmail);
gApi.accounts().id(foo.id().get()).addEmail(input);
@@ -938,7 +938,7 @@
public void detailOfOtherAccountDoesntIncludeSecondaryEmailsWithoutModifyAccount()
throws Exception {
String email = "preferred@example.com";
- TestAccount foo = accountCreator.create(name("foo"), email, "Foo");
+ TestAccount foo = accountCreator.create(name("foo"), email, "Foo", null);
String secondaryEmail = "secondary@example.com";
EmailInput input = newEmailInput(secondaryEmail);
gApi.accounts().id(foo.id().get()).addEmail(input);
@@ -951,7 +951,7 @@
@Test
public void detailOfOtherAccountIncludeSecondaryEmailsWithModifyAccount() throws Exception {
String email = "preferred@example.com";
- TestAccount foo = accountCreator.create(name("foo"), email, "Foo");
+ TestAccount foo = accountCreator.create(name("foo"), email, "Foo", null);
String secondaryEmail = "secondary@example.com";
EmailInput input = newEmailInput(secondaryEmail);
gApi.accounts().id(foo.id().get()).addEmail(input);
@@ -963,7 +963,7 @@
@Test
public void getOwnEmails() throws Exception {
String email = "preferred@example.com";
- TestAccount foo = accountCreator.create(name("foo"), email, "Foo");
+ TestAccount foo = accountCreator.create(name("foo"), email, "Foo", null);
requestScopeOperations.setApiUser(foo.id());
assertThat(getEmails()).containsExactly(email);
@@ -980,7 +980,7 @@
@Test
public void cannotGetEmailsOfOtherAccountWithoutModifyAccount() throws Exception {
String email = "preferred2@example.com";
- TestAccount foo = accountCreator.create(name("foo"), email, "Foo");
+ TestAccount foo = accountCreator.create(name("foo"), email, "Foo", null);
requestScopeOperations.setApiUser(user.id());
AuthException thrown =
@@ -992,7 +992,7 @@
public void getEmailsOfOtherAccount() throws Exception {
String email = "preferred3@example.com";
String secondaryEmail = "secondary3@example.com";
- TestAccount foo = accountCreator.create(name("foo"), email, "Foo");
+ TestAccount foo = accountCreator.create(name("foo"), email, "Foo", null);
EmailInput input = newEmailInput(secondaryEmail);
gApi.accounts().id(foo.id().get()).addEmail(input);
@@ -1840,7 +1840,7 @@
assertThat(sender.getMessages()).hasSize(1);
Message message = sender.getMessages().get(0);
- assertThat(message.rcpt()).containsExactly(user.getEmailAddress());
+ assertThat(message.rcpt()).containsExactly(user.getNameEmail());
assertThat(message.body()).contains("new SSH keys have been added");
// Delete key
@@ -1852,7 +1852,7 @@
assertThat(sender.getMessages()).hasSize(1);
message = sender.getMessages().get(0);
- assertThat(message.rcpt()).containsExactly(user.getEmailAddress());
+ assertThat(message.rcpt()).containsExactly(user.getNameEmail());
assertThat(message.body()).contains("SSH keys have been deleted");
}
}
@@ -1908,7 +1908,7 @@
// Create an account with a preferred email.
String username = name("foo");
String email = username + "@example.com";
- TestAccount account = accountCreator.create(username, email, "Foo Bar");
+ TestAccount account = accountCreator.create(username, email, "Foo Bar", null);
ConsistencyCheckInput input = new ConsistencyCheckInput();
input.checkAccounts = new CheckAccountsInput();
diff --git a/javatests/com/google/gerrit/acceptance/api/accounts/GeneralPreferencesIT.java b/javatests/com/google/gerrit/acceptance/api/accounts/GeneralPreferencesIT.java
index 6717fb7..746e6fe 100644
--- a/javatests/com/google/gerrit/acceptance/api/accounts/GeneralPreferencesIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/accounts/GeneralPreferencesIT.java
@@ -47,7 +47,7 @@
@Before
public void setUp() throws Exception {
String name = name("user42");
- user42 = accountCreator.create(name, name + "@example.com", "User 42");
+ user42 = accountCreator.create(name, name + "@example.com", "User 42", null);
}
@Test
@@ -56,7 +56,7 @@
assertPrefs(o, GeneralPreferencesInfo.defaults(), "my", "changeTable");
assertThat(o.my)
.containsExactly(
- new MenuItem("Changes", "#/dashboard/self", null),
+ new MenuItem("Dashboard", "#/dashboard/self", null),
new MenuItem("Draft Comments", "#/q/has:draft", null),
new MenuItem("Edits", "#/q/has:edit", null),
new MenuItem("Watched Changes", "#/q/is:watched+is:open", null),
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
index c28d22c..9399c3b 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -1518,7 +1518,7 @@
assertThat(messages).hasSize(1);
Message m = messages.get(0);
assertThat(m.from().getName()).isEqualTo("Administrator (Code Review)");
- assertThat(m.rcpt()).containsExactly(user.getEmailAddress());
+ assertThat(m.rcpt()).containsExactly(user.getNameEmail());
assertThat(m.body()).contains("I'd like you to do a code review");
assertThat(m.body()).contains("Change subject: " + PushOneCommit.SUBJECT + "\n");
assertMailReplyTo(m, admin.email());
@@ -1587,7 +1587,7 @@
List<Message> messages = sender.getMessages();
assertThat(messages).hasSize(1);
Message m = messages.get(0);
- assertThat(m.rcpt()).containsExactly(user.getEmailAddress());
+ assertThat(m.rcpt()).containsExactly(user.getNameEmail());
assertThat(m.body()).contains("Hello " + user.fullName() + ",\n");
assertThat(m.body()).contains("I'd like you to do a code review.");
assertThat(m.body()).contains("Change subject: " + PushOneCommit.SUBJECT + "\n");
@@ -1771,7 +1771,7 @@
List<Message> messages = sender.getMessages();
assertThat(messages).hasSize(1);
Message m = messages.get(0);
- assertThat(m.rcpt()).containsExactly(user.getEmailAddress());
+ assertThat(m.rcpt()).containsExactly(user.getNameEmail());
assertThat(m.body()).contains("Hello " + user.fullName() + ",\n");
assertThat(m.body()).contains("I'd like you to do a code review.");
assertThat(m.body()).contains("Change subject: " + PushOneCommit.SUBJECT + "\n");
@@ -2162,7 +2162,7 @@
assertThat(sender.getMessages()).hasSize(1);
Message m = sender.getMessages().get(0);
- assertThat(m.rcpt()).containsExactly(user.getEmailAddress());
+ assertThat(m.rcpt()).containsExactly(user.getNameEmail());
}
@Test
@@ -2411,7 +2411,7 @@
List<Message> messages = sender.getMessages();
assertThat(messages).hasSize(1);
Message msg = messages.get(0);
- assertThat(msg.rcpt()).containsExactly(user.getEmailAddress());
+ assertThat(msg.rcpt()).containsExactly(user.getNameEmail());
assertThat(msg.body()).contains(admin.fullName() + " has removed a vote from this change.");
assertThat(msg.body())
.contains("Removed Code-Review+1 by " + user.fullName() + " <" + user.email() + ">\n");
@@ -4527,7 +4527,7 @@
List<Message> messages = sender.getMessages();
assertThat(messages).hasSize(1);
- assertThat(messages.get(0).rcpt()).containsExactly(user.getEmailAddress());
+ assertThat(messages.get(0).rcpt()).containsExactly(user.getNameEmail());
}
@Test
diff --git a/javatests/com/google/gerrit/acceptance/api/change/QueryChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/QueryChangeIT.java
index bf5acee..52bff58 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/QueryChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/QueryChangeIT.java
@@ -17,6 +17,7 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.block;
import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
+import static com.google.gerrit.testing.GerritJUnit.assertThrows;
import static java.util.stream.Collectors.toList;
import static javax.servlet.http.HttpServletResponse.SC_OK;
@@ -31,6 +32,7 @@
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.Project;
import com.google.gerrit.extensions.common.ChangeInfo;
+import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.TopLevelResource;
import com.google.gerrit.server.restapi.change.QueryChanges;
import com.google.inject.Inject;
@@ -225,6 +227,26 @@
assertThat(queryChanges.apply(TopLevelResource.INSTANCE).statusCode()).isEqualTo(SC_OK);
}
+ @Test
+ public void defaultQueryCannotBeParsedDueToInvalidRegEx() throws Exception {
+ QueryChanges queryChanges = queryChangesProvider.get();
+ queryChanges.addQuery("^[A");
+ BadRequestException e =
+ assertThrows(
+ BadRequestException.class, () -> queryChanges.apply(TopLevelResource.INSTANCE));
+ assertThat(e).hasMessageThat().contains("no viable alternative at character '['");
+ }
+
+ @Test
+ public void defaultQueryWithInvalidQuotedRegEx() throws Exception {
+ QueryChanges queryChanges = queryChangesProvider.get();
+ queryChanges.addQuery("\"^[A\"");
+ BadRequestException e =
+ assertThrows(
+ BadRequestException.class, () -> queryChanges.apply(TopLevelResource.INSTANCE));
+ assertThat(e).hasMessageThat().isEqualTo("invalid regular expression: [A");
+ }
+
private static void assertNoChangeHasMoreChangesSet(List<ChangeInfo> results) {
for (ChangeInfo info : results) {
assertThat(info._moreChanges).isNull();
diff --git a/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java b/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java
index 17a0ea6..30eaf22 100644
--- a/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java
@@ -363,6 +363,13 @@
}
@Test
+ public void createGroupNameIsTrimmed() throws Exception {
+ String newGroupName = name("newGroup");
+ GroupInfo g = gApi.groups().create(" " + newGroupName + " ").get();
+ assertGroupInfo(group(newGroupName), g);
+ }
+
+ @Test
public void createDuplicateInternalGroupCaseSensitiveName_Conflict() throws Exception {
String dupGroupName = name("dupGroup");
gApi.groups().create(dupGroupName);
@@ -1349,17 +1356,6 @@
}
@Test
- public void groupNamesWithLeadingAndTrailingWhitespace() throws Exception {
- for (String leading : ImmutableList.of("", " ", " ")) {
- for (String trailing : ImmutableList.of("", " ", " ")) {
- String name = leading + name("group") + trailing;
- GroupInfo g = gApi.groups().create(name).get();
- assertThat(g.name).isEqualTo(name);
- }
- }
- }
-
- @Test
@Sandboxed
public void groupsOfUserCanBeListedInSlaveMode() throws Exception {
GroupInput groupInput = new GroupInput();
diff --git a/javatests/com/google/gerrit/acceptance/api/project/CheckAccessIT.java b/javatests/com/google/gerrit/acceptance/api/project/CheckAccessIT.java
index b262244..3fc6e44 100644
--- a/javatests/com/google/gerrit/acceptance/api/project/CheckAccessIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/project/CheckAccessIT.java
@@ -62,7 +62,7 @@
AccountGroup.UUID privilegedGroupUuid =
groupOperations.newGroup().name(name("privilegedGroup")).create();
- privilegedUser = accountCreator.create("privilegedUser", "snowden@nsa.gov", "Ed Snowden");
+ privilegedUser = accountCreator.create("privilegedUser", "snowden@nsa.gov", "Ed Snowden", null);
groupOperations.group(privilegedGroupUuid).forUpdate().addMember(privilegedUser.id()).update();
projectOperations
diff --git a/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java b/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java
index 51dee72..f190d59 100644
--- a/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java
+++ b/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java
@@ -516,7 +516,7 @@
@Test
public void pushForMasterWithNotify() throws Exception {
// create a user that watches the project
- TestAccount user3 = accountCreator.create("user3", "user3@example.com", "User3");
+ TestAccount user3 = accountCreator.create("user3", "user3@example.com", "User3", null);
List<ProjectWatchInfo> projectsToWatch = new ArrayList<>();
ProjectWatchInfo pwi = new ProjectWatchInfo();
pwi.project = project.get();
@@ -545,7 +545,7 @@
r.assertOkStatus();
assertThat(sender.getMessages()).hasSize(1);
Message m = sender.getMessages().get(0);
- assertThat(m.rcpt()).containsExactly(user.getEmailAddress());
+ assertThat(m.rcpt()).containsExactly(user.getNameEmail());
sender.clear();
r = pushTo(pushSpec + ",notify=" + NotifyHandling.ALL);
@@ -553,7 +553,7 @@
assertThat(sender.getMessages()).hasSize(1);
m = sender.getMessages().get(0);
assertThat(m.rcpt())
- .containsExactly(user.getEmailAddress(), user2.getEmailAddress(), user3.getEmailAddress());
+ .containsExactly(user.getNameEmail(), user2.getNameEmail(), user3.getNameEmail());
sender.clear();
r = pushTo(pushSpec + ",notify=" + NotifyHandling.NONE + ",notify-to=" + user3.email());
@@ -676,7 +676,7 @@
// add several reviewers
TestAccount user2 =
- accountCreator.create("another-user", "another.user@example.com", "Another User");
+ accountCreator.create("another-user", "another.user@example.com", "Another User", null);
r =
pushTo(
"refs/for/master%topic="
@@ -1116,7 +1116,7 @@
assertThat(sender.getMessages()).hasSize(1);
assertThat(sender.getMessages().get(0).rcpt())
- .containsExactly(user.getEmailAddress(), user2.getEmailAddress());
+ .containsExactly(user.getNameEmail(), user2.getNameEmail());
}
@Test
@@ -1141,7 +1141,7 @@
assertThat(sender.getMessages()).hasSize(1);
assertThat(sender.getMessages().get(0).rcpt())
- .containsExactly(user.getEmailAddress(), user2.getEmailAddress());
+ .containsExactly(user.getNameEmail(), user2.getNameEmail());
}
/**
@@ -1838,7 +1838,7 @@
@Test
public void pushWithEmailInFooter() throws Exception {
- pushWithReviewerInFooter(user.getEmailAddress().toString(), user);
+ pushWithReviewerInFooter(user.getNameEmail().toString(), user);
}
@Test
diff --git a/javatests/com/google/gerrit/acceptance/git/PushAccountIT.java b/javatests/com/google/gerrit/acceptance/git/PushAccountIT.java
index 2f677e2..87f3c98 100644
--- a/javatests/com/google/gerrit/acceptance/git/PushAccountIT.java
+++ b/javatests/com/google/gerrit/acceptance/git/PushAccountIT.java
@@ -182,7 +182,8 @@
AccountIndexedCounter accountIndexedCounter = new AccountIndexedCounter();
try (Registration registration =
extensionRegistry.newRegistration().add(accountIndexedCounter)) {
- TestAccount foo = accountCreator.create(name("foo"), name("foo") + "@example.com", "Foo");
+ TestAccount foo =
+ accountCreator.create(name("foo"), name("foo") + "@example.com", "Foo", null);
String userRef = RefNames.refsUsers(foo.id());
accountIndexedCounter.clear();
@@ -442,7 +443,7 @@
AccountIndexedCounter accountIndexedCounter = new AccountIndexedCounter();
try (Registration registration =
extensionRegistry.newRegistration().add(accountIndexedCounter)) {
- TestAccount oooUser = accountCreator.create("away", "away@mail.invalid", "Ambrose Way");
+ TestAccount oooUser = accountCreator.create("away", "away@mail.invalid", "Ambrose Way", null);
requestScopeOperations.setApiUser(oooUser.id());
// Must clone as oooUser to ensure the push is allowed.
@@ -539,7 +540,8 @@
AccountIndexedCounter accountIndexedCounter = new AccountIndexedCounter();
try (Registration registration =
extensionRegistry.newRegistration().add(accountIndexedCounter)) {
- TestAccount foo = accountCreator.create(name("foo"), name("foo") + "@example.com", "Foo");
+ TestAccount foo =
+ accountCreator.create(name("foo"), name("foo") + "@example.com", "Foo", null);
String userRef = RefNames.refsUsers(foo.id());
String noEmail = "no.email";
@@ -584,7 +586,8 @@
AccountIndexedCounter accountIndexedCounter = new AccountIndexedCounter();
try (Registration registration =
extensionRegistry.newRegistration().add(accountIndexedCounter)) {
- TestAccount foo = accountCreator.create(name("foo"), name("foo") + "@example.com", "Foo");
+ TestAccount foo =
+ accountCreator.create(name("foo"), name("foo") + "@example.com", "Foo", null);
String userRef = RefNames.refsUsers(foo.id());
accountIndexedCounter.clear();
diff --git a/javatests/com/google/gerrit/acceptance/rest/account/AccountAssert.java b/javatests/com/google/gerrit/acceptance/rest/account/AccountAssert.java
index 03202f2..c0feda9 100644
--- a/javatests/com/google/gerrit/acceptance/rest/account/AccountAssert.java
+++ b/javatests/com/google/gerrit/acceptance/rest/account/AccountAssert.java
@@ -32,6 +32,7 @@
public static void assertAccountInfo(TestAccount testAccount, AccountInfo accountInfo) {
assertThat(accountInfo._accountId).isEqualTo(testAccount.id().get());
assertThat(accountInfo.name).isEqualTo(testAccount.fullName());
+ assertThat(accountInfo.displayName).isEqualTo(testAccount.displayName());
assertThat(accountInfo.email).isEqualTo(testAccount.email());
assertThat(accountInfo.inactive).isNull();
}
diff --git a/javatests/com/google/gerrit/acceptance/rest/account/ExternalIdIT.java b/javatests/com/google/gerrit/acceptance/rest/account/ExternalIdIT.java
index 6e16435..53e871f 100644
--- a/javatests/com/google/gerrit/acceptance/rest/account/ExternalIdIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/account/ExternalIdIT.java
@@ -132,14 +132,14 @@
AuthException thrown =
assertThrows(
AuthException.class, () -> gApi.accounts().id(admin.id().get()).getExternalIds());
- assertThat(thrown).hasMessageThat().contains("access database not permitted");
+ assertThat(thrown).hasMessageThat().contains("modify account not permitted");
}
@Test
- public void getExternalIdsOfOtherUserWithAccessDatabase() throws Exception {
+ public void getExternalIdsOfOtherUserWithModifyAccount() throws Exception {
projectOperations
.allProjectsForUpdate()
- .add(allowCapability(GlobalCapability.ACCESS_DATABASE).group(REGISTERED_USERS))
+ .add(allowCapability(GlobalCapability.MODIFY_ACCOUNT).group(REGISTERED_USERS))
.update();
Collection<ExternalId> expectedIds = getAccountState(admin.id()).externalIds();
@@ -193,7 +193,7 @@
gApi.accounts()
.id(admin.id().get())
.deleteExternalIds(extIds.stream().map(e -> e.identity).collect(toList())));
- assertThat(thrown).hasMessageThat().contains("access database not permitted");
+ assertThat(thrown).hasMessageThat().contains("modify account not permitted");
}
@Test
@@ -213,10 +213,10 @@
}
@Test
- public void deleteExternalIdsOfOtherUserWithAccessDatabase() throws Exception {
+ public void deleteExternalIdsOfOtherUserWithModifyAccount() throws Exception {
projectOperations
.allProjectsForUpdate()
- .add(allowCapability(GlobalCapability.ACCESS_DATABASE).group(REGISTERED_USERS))
+ .add(allowCapability(GlobalCapability.MODIFY_ACCOUNT).group(REGISTERED_USERS))
.update();
List<AccountExternalIdInfo> externalIds = gApi.accounts().self().getExternalIds();
@@ -464,7 +464,7 @@
public void readExternalIdsWhenInvalidExternalIdsExist() throws Exception {
projectOperations
.allProjectsForUpdate()
- .add(allowCapability(GlobalCapability.ACCESS_DATABASE).group(REGISTERED_USERS))
+ .add(allowCapability(GlobalCapability.MODIFY_ACCOUNT).group(REGISTERED_USERS))
.update();
requestScopeOperations.resetCurrentApiUser();
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/AssigneeIT.java b/javatests/com/google/gerrit/acceptance/rest/change/AssigneeIT.java
index 0f5def6..420ddda 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/AssigneeIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/AssigneeIT.java
@@ -61,7 +61,7 @@
assertThat(sender.getMessages()).hasSize(1);
Message m = sender.getMessages().get(0);
- assertThat(m.rcpt()).containsExactly(user.getEmailAddress());
+ assertThat(m.rcpt()).containsExactly(user.getNameEmail());
}
@Test
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/ChangeReviewersByEmailIT.java b/javatests/com/google/gerrit/acceptance/rest/change/ChangeReviewersByEmailIT.java
index ac00e38..090146e 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/ChangeReviewersByEmailIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/ChangeReviewersByEmailIT.java
@@ -209,7 +209,7 @@
List<Message> messages = sender.getMessages();
assertThat(messages).hasSize(1);
assertThat(messages.get(0).rcpt())
- .containsExactly(Address.parse(addInput.reviewer), user.getEmailAddress());
+ .containsExactly(Address.parse(addInput.reviewer), user.getNameEmail());
sender.clear();
}
}
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/ChangeReviewersIT.java b/javatests/com/google/gerrit/acceptance/rest/change/ChangeReviewersIT.java
index fbe6533..35576b8 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/ChangeReviewersIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/ChangeReviewersIT.java
@@ -150,7 +150,7 @@
List<Message> messages = sender.getMessages();
assertThat(messages).hasSize(1);
Message m = messages.get(0);
- assertThat(m.rcpt()).containsExactly(user.getEmailAddress());
+ assertThat(m.rcpt()).containsExactly(user.getNameEmail());
assertThat(m.body()).contains(admin.fullName() + " has uploaded this change for review.");
}
@@ -188,13 +188,14 @@
Message m = messages.get(0);
List<Address> expectedAddresses = new ArrayList<>(firstUsers.size());
for (TestAccount u : firstUsers) {
- expectedAddresses.add(u.getEmailAddress());
+ expectedAddresses.add(u.getNameEmail());
}
assertThat(m.rcpt()).containsExactlyElementsIn(expectedAddresses);
// CC a group that overlaps with some existing reviewers and CCed accounts.
TestAccount reviewer =
- accountCreator.create(name("reviewer"), "addCcGroup-reviewer@example.com", "Reviewer");
+ accountCreator.create(
+ name("reviewer"), "addCcGroup-reviewer@example.com", "Reviewer", null);
result = addReviewer(changeId, reviewer.username());
assertThat(result.error).isNull();
sender.clear();
@@ -216,9 +217,9 @@
m = messages.get(0);
expectedAddresses = new ArrayList<>(4);
for (int i = 0; i < 3; i++) {
- expectedAddresses.add(users.get(users.size() - i - 1).getEmailAddress());
+ expectedAddresses.add(users.get(users.size() - i - 1).getNameEmail());
}
- expectedAddresses.add(reviewer.getEmailAddress());
+ expectedAddresses.add(reviewer.getNameEmail());
assertThat(m.rcpt()).containsExactlyElementsIn(expectedAddresses);
}
@@ -398,13 +399,13 @@
assertThat(messages).hasSize(2);
Message m = messages.get(0);
- assertThat(m.rcpt()).containsExactly(user.getEmailAddress(), observer.getEmailAddress());
+ assertThat(m.rcpt()).containsExactly(user.getNameEmail(), observer.getNameEmail());
assertThat(m.body()).contains(admin.fullName() + " has posted comments on this change.");
assertThat(m.body()).contains("Change subject: " + PushOneCommit.SUBJECT + "\n");
assertThat(m.body()).contains("Patch Set 1: Code-Review+2");
m = messages.get(1);
- assertThat(m.rcpt()).containsExactly(user.getEmailAddress(), observer.getEmailAddress());
+ assertThat(m.rcpt()).containsExactly(user.getNameEmail(), observer.getNameEmail());
assertThat(m.body()).contains("Hello " + user.fullName() + ",\n");
assertThat(m.body()).contains("I'd like you to do a code review.");
}
@@ -546,11 +547,11 @@
public void addOverlappingGroups() throws Exception {
String emailPrefix = "addOverlappingGroups-";
TestAccount user1 =
- accountCreator.create(name("user1"), emailPrefix + "user1@example.com", "User1");
+ accountCreator.create(name("user1"), emailPrefix + "user1@example.com", "User1", null);
TestAccount user2 =
- accountCreator.create(name("user2"), emailPrefix + "user2@example.com", "User2");
+ accountCreator.create(name("user2"), emailPrefix + "user2@example.com", "User2", null);
TestAccount user3 =
- accountCreator.create(name("user3"), emailPrefix + "user3@example.com", "User3");
+ accountCreator.create(name("user3"), emailPrefix + "user3@example.com", "User3", null);
String group1 = groupOperations.newGroup().name("group1").create().get();
String group2 = groupOperations.newGroup().name("group2").create().get();
gApi.groups().id(group1).addMembers(user1.username(), user2.username());
@@ -635,7 +636,7 @@
sender.clear();
gApi.changes().id(r.getChangeId()).current().review(reviewInput);
assertThat(sender.getMessages()).hasSize(1);
- assertThat(sender.getMessages().get(0).rcpt()).containsExactly(userToNotify.getEmailAddress());
+ assertThat(sender.getMessages().get(0).rcpt()).containsExactly(userToNotify.getNameEmail());
}
@Test
@@ -652,7 +653,7 @@
sender.clear();
gApi.changes().id(r.getChangeId()).addReviewer(addReviewer);
assertThat(sender.getMessages()).hasSize(1);
- assertThat(sender.getMessages().get(0).rcpt()).containsExactly(userToNotify.getEmailAddress());
+ assertThat(sender.getMessages().get(0).rcpt()).containsExactly(userToNotify.getNameEmail());
}
@Test
@@ -829,7 +830,7 @@
}
private void assertThatUserIsOnlyReviewer(String changeId) throws Exception {
- AccountInfo userInfo = new AccountInfo(user.fullName(), user.getEmailAddress().getEmail());
+ AccountInfo userInfo = new AccountInfo(user.fullName(), user.getNameEmail().getEmail());
userInfo._accountId = user.id().get();
userInfo.username = user.username();
assertThat(gApi.changes().id(changeId).get().reviewers)
@@ -912,7 +913,7 @@
for (int i = 0; i < n; i++) {
result.add(
accountCreator.create(
- name("u" + i), emailPrefix + "-" + i + "@example.com", "Full Name " + i));
+ name("u" + i), emailPrefix + "-" + i + "@example.com", "Full Name " + i, null));
}
return result;
}
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java b/javatests/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java
index 81280a4..d831b62 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java
@@ -216,7 +216,7 @@
List<Message> messages = sender.getMessages();
assertThat(messages).hasSize(1);
Message m = messages.get(0);
- assertThat(m.rcpt()).containsExactly(user.getEmailAddress());
+ assertThat(m.rcpt()).containsExactly(user.getNameEmail());
assertThat(m.body()).contains(admin.fullName() + " has uploaded this change for review.");
// check that watcher is not notified if notify=NONE
@@ -299,9 +299,11 @@
ChangeInfo info = assertCreateSucceeds(input);
RevisionApi rApi = gApi.changes().id(info.id).current();
- GitPerson person = rApi.commit(false).author;
- assertThat(person).email().isEqualTo(input.author.email);
- assertThat(person).name().isEqualTo(input.author.name);
+ GitPerson author = rApi.commit(false).author;
+ assertThat(author).email().isEqualTo(input.author.email);
+ assertThat(author).name().isEqualTo(input.author.name);
+ GitPerson committer = rApi.commit(false).committer;
+ assertThat(committer).email().isEqualTo(admin.getNameEmail().getEmail());
}
@Test
@@ -449,6 +451,22 @@
}
@Test
+ public void createMergeChangeAuthor() throws Exception {
+ changeInTwoBranches("branchA", "a.txt", "branchB", "b.txt");
+ ChangeInput in = newMergeChangeInput("branchA", "branchB", "");
+ in.author = new AccountInput();
+ in.author.name = "Gerritless Jane";
+ in.author.email = "gerritlessjane@invalid";
+ ChangeInfo change = assertCreateSucceeds(in);
+
+ RevisionApi rApi = gApi.changes().id(change.id).current();
+ GitPerson author = rApi.commit(false).author;
+ assertThat(author).email().isEqualTo(in.author.email);
+ GitPerson committer = rApi.commit(false).committer;
+ assertThat(committer).email().isEqualTo(admin.getNameEmail().getEmail());
+ }
+
+ @Test
public void createMergeChange_Conflicts() throws Exception {
changeInTwoBranches("branchA", "shared.txt", "branchB", "shared.txt");
ChangeInput in = newMergeChangeInput("branchA", "branchB", "");
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/DeleteVoteIT.java b/javatests/com/google/gerrit/acceptance/rest/change/DeleteVoteIT.java
index 37fa2ce..25e5647 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/DeleteVoteIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/DeleteVoteIT.java
@@ -73,7 +73,7 @@
List<FakeEmailSender.Message> messages = sender.getMessages();
assertThat(messages).hasSize(1);
FakeEmailSender.Message msg = messages.get(0);
- assertThat(msg.rcpt()).containsExactly(user.getEmailAddress());
+ assertThat(msg.rcpt()).containsExactly(user.getNameEmail());
assertThat(msg.body()).contains(admin.fullName() + " has removed a vote from this change.");
assertThat(msg.body())
.contains("Removed Code-Review+1 by " + user.fullName() + " <" + user.email() + ">\n");
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/SuggestReviewersIT.java b/javatests/com/google/gerrit/acceptance/rest/change/SuggestReviewersIT.java
index 1bd2d99..551a349 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/SuggestReviewersIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/SuggestReviewersIT.java
@@ -570,7 +570,7 @@
private TestAccount createAccountWithSecondaryEmail(String name, String secondaryEmail)
throws Exception {
- TestAccount foo = accountCreator.create(name(name), "foo.primary@example.com", "Foo");
+ TestAccount foo = accountCreator.create(name(name), "foo.primary@example.com", "Foo", null);
EmailInput input = new EmailInput();
input.email = secondaryEmail;
input.noConfirmation = true;
@@ -601,7 +601,7 @@
}
private TestAccount user(String name, String fullName, String emailName) throws Exception {
- return accountCreator.create(name(name), name(emailName) + "@example.com", fullName);
+ return accountCreator.create(name(name), name(emailName) + "@example.com", fullName, null);
}
private TestAccount user(String name, String fullName) throws Exception {
diff --git a/javatests/com/google/gerrit/acceptance/server/mail/AbstractMailIT.java b/javatests/com/google/gerrit/acceptance/server/mail/AbstractMailIT.java
index 3b0e27b..9e0de92 100644
--- a/javatests/com/google/gerrit/acceptance/server/mail/AbstractMailIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/mail/AbstractMailIT.java
@@ -38,8 +38,8 @@
MailMessage.Builder messageBuilderWithDefaultFields() {
MailMessage.Builder b = MailMessage.builder();
b.id("some id");
- b.from(user.getEmailAddress());
- b.addTo(user.getEmailAddress()); // Not evaluated
+ b.from(user.getNameEmail());
+ b.addTo(user.getNameEmail()); // Not evaluated
b.subject("");
b.dateReceived(Instant.now());
return b;
diff --git a/javatests/com/google/gerrit/acceptance/server/mail/ChangeNotificationsIT.java b/javatests/com/google/gerrit/acceptance/server/mail/ChangeNotificationsIT.java
index 2dc1e24..d74cd71 100644
--- a/javatests/com/google/gerrit/acceptance/server/mail/ChangeNotificationsIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/mail/ChangeNotificationsIT.java
@@ -76,9 +76,9 @@
@Before
public void createExtraAccounts() throws Exception {
extraReviewer =
- accountCreator.create("extraReviewer", "extraReviewer@example.com", "extraReviewer");
- extraCcer = accountCreator.create("extraCcer", "extraCcer@example.com", "extraCcer");
- other = accountCreator.create("other", "other@example.com", "other");
+ accountCreator.create("extraReviewer", "extraReviewer@example.com", "extraReviewer", null);
+ extraCcer = accountCreator.create("extraCcer", "extraCcer@example.com", "extraCcer", null);
+ other = accountCreator.create("other", "other@example.com", "other", null);
}
@Before
@@ -129,7 +129,7 @@
@Test
public void abandonReviewableChangeByOther() throws Exception {
StagedChange sc = stageReviewableChange();
- TestAccount other = accountCreator.create("other", "other@example.com", "other");
+ TestAccount other = accountCreator.create("other", "other@example.com", "other", null);
abandon(sc.changeId, other);
assertThat(sender)
.sent("abandon", sc)
@@ -145,7 +145,7 @@
@Test
public void abandonReviewableChangeByOtherCcingSelf() throws Exception {
StagedChange sc = stageReviewableChange();
- TestAccount other = accountCreator.create("other", "other@example.com", "other");
+ TestAccount other = accountCreator.create("other", "other@example.com", "other", null);
abandon(sc.changeId, other, CC_ON_OWN_COMMENTS);
assertThat(sender)
.sent("abandon", sc)
@@ -190,7 +190,7 @@
@Test
public void abandonReviewableChangeByOtherCcingSelfNotifyOwner() throws Exception {
StagedChange sc = stageReviewableChange();
- TestAccount other = accountCreator.create("other", "other@example.com", "other");
+ TestAccount other = accountCreator.create("other", "other@example.com", "other", null);
abandon(sc.changeId, other, CC_ON_OWN_COMMENTS, OWNER);
assertThat(sender).sent("abandon", sc).to(sc.owner).cc(other).noOneElse();
assertThat(sender).didNotSend();
@@ -277,7 +277,7 @@
private void addReviewerToReviewableChange(Adder adder) throws Exception {
StagedChange sc = stageReviewableChange();
- TestAccount reviewer = accountCreator.create("added", "added@example.com", "added");
+ TestAccount reviewer = accountCreator.create("added", "added@example.com", "added", null);
addReviewer(adder, sc.changeId, sc.owner, reviewer.email());
// TODO(logan): Should CCs be included?
assertThat(sender)
@@ -301,7 +301,7 @@
private void addReviewerToReviewableChangeByOwnerCcingSelf(Adder adder) throws Exception {
StagedChange sc = stageReviewableChange();
- TestAccount reviewer = accountCreator.create("added", "added@example.com", "added");
+ TestAccount reviewer = accountCreator.create("added", "added@example.com", "added", null);
addReviewer(adder, sc.changeId, sc.owner, reviewer.email(), CC_ON_OWN_COMMENTS, null);
// TODO(logan): Should CCs be included?
assertThat(sender)
@@ -324,9 +324,9 @@
}
private void addReviewerToReviewableChangeByOther(Adder adder) throws Exception {
- TestAccount other = accountCreator.create("other", "other@example.com", "other");
+ TestAccount other = accountCreator.create("other", "other@example.com", "other", null);
StagedChange sc = stageReviewableChange();
- TestAccount reviewer = accountCreator.create("added", "added@example.com", "added");
+ TestAccount reviewer = accountCreator.create("added", "added@example.com", "added", null);
addReviewer(adder, sc.changeId, other, reviewer.email());
// TODO(logan): Should CCs be included?
assertThat(sender)
@@ -349,9 +349,9 @@
}
private void addReviewerToReviewableChangeByOtherCcingSelf(Adder adder) throws Exception {
- TestAccount other = accountCreator.create("other", "other@example.com", "other");
+ TestAccount other = accountCreator.create("other", "other@example.com", "other", null);
StagedChange sc = stageReviewableChange();
- TestAccount reviewer = accountCreator.create("added", "added@example.com", "added");
+ TestAccount reviewer = accountCreator.create("added", "added@example.com", "added", null);
addReviewer(adder, sc.changeId, other, reviewer.email(), CC_ON_OWN_COMMENTS, null);
// TODO(logan): Should CCs be included?
assertThat(sender)
@@ -399,7 +399,7 @@
private void addReviewerToWipChange(Adder adder) throws Exception {
StagedChange sc = stageWipChange();
- TestAccount reviewer = accountCreator.create("added", "added@example.com", "added");
+ TestAccount reviewer = accountCreator.create("added", "added@example.com", "added", null);
addReviewer(adder, sc.changeId, sc.owner, reviewer.email());
assertThat(sender).didNotSend();
}
@@ -417,7 +417,7 @@
@Test
public void addReviewerToReviewableWipChangeSingly() throws Exception {
StagedChange sc = stageReviewableWipChange();
- TestAccount reviewer = accountCreator.create("added", "added@example.com", "added");
+ TestAccount reviewer = accountCreator.create("added", "added@example.com", "added", null);
addReviewer(singly(), sc.changeId, sc.owner, reviewer.email());
// TODO(dborowitz): In theory this should match the batch case, but we don't currently pass
// enough info into AddReviewersEmail#emailReviewers to distinguish the reviewStarted case.
@@ -429,7 +429,7 @@
@Test
public void addReviewerToReviewableWipChangeBatch() throws Exception {
StagedChange sc = stageReviewableWipChange();
- TestAccount reviewer = accountCreator.create("added", "added@example.com", "added");
+ TestAccount reviewer = accountCreator.create("added", "added@example.com", "added", null);
addReviewer(batch(), sc.changeId, sc.owner, reviewer.email());
// For a review-started WIP change, same as in the notify=ALL case. It's not especially
// important to notify just because a reviewer is added, but we do want to notify in the other
@@ -444,7 +444,7 @@
private void addReviewerToWipChangeNotifyAll(Adder adder) throws Exception {
StagedChange sc = stageWipChange();
- TestAccount reviewer = accountCreator.create("added", "added@example.com", "added");
+ TestAccount reviewer = accountCreator.create("added", "added@example.com", "added", null);
addReviewer(adder, sc.changeId, sc.owner, reviewer.email(), NotifyHandling.ALL);
// TODO(logan): Should CCs be included?
assertThat(sender)
@@ -468,7 +468,7 @@
private void addReviewerToReviewableChangeNotifyOwnerReviewers(Adder adder) throws Exception {
StagedChange sc = stageReviewableChange();
- TestAccount reviewer = accountCreator.create("added", "added@example.com", "added");
+ TestAccount reviewer = accountCreator.create("added", "added@example.com", "added", null);
addReviewer(adder, sc.changeId, sc.owner, reviewer.email(), OWNER_REVIEWERS);
// TODO(logan): Should CCs be included?
assertThat(sender)
@@ -493,7 +493,7 @@
private void addReviewerToReviewableChangeByOwnerCcingSelfNotifyOwner(Adder adder)
throws Exception {
StagedChange sc = stageReviewableChange();
- TestAccount reviewer = accountCreator.create("added", "added@example.com", "added");
+ TestAccount reviewer = accountCreator.create("added", "added@example.com", "added", null);
addReviewer(adder, sc.changeId, sc.owner, reviewer.email(), CC_ON_OWN_COMMENTS, OWNER);
assertThat(sender).didNotSend();
}
@@ -511,7 +511,7 @@
private void addReviewerToReviewableChangeByOwnerCcingSelfNotifyNone(Adder adder)
throws Exception {
StagedChange sc = stageReviewableChange();
- TestAccount reviewer = accountCreator.create("added", "added@example.com", "added");
+ TestAccount reviewer = accountCreator.create("added", "added@example.com", "added", null);
addReviewer(adder, sc.changeId, sc.owner, reviewer.email(), CC_ON_OWN_COMMENTS, NONE);
assertThat(sender).didNotSend();
}
@@ -695,7 +695,7 @@
@Test
public void commentOnReviewableChangeByOther() throws Exception {
- TestAccount other = accountCreator.create("other", "other@example.com", "other");
+ TestAccount other = accountCreator.create("other", "other@example.com", "other", null);
StagedChange sc = stageReviewableChange();
review(other, sc.changeId, ENABLED);
assertThat(sender)
@@ -711,7 +711,7 @@
@Test
public void commentOnReviewableChangeByOtherCcingSelf() throws Exception {
- TestAccount other = accountCreator.create("other", "other@example.com", "other");
+ TestAccount other = accountCreator.create("other", "other@example.com", "other", null);
StagedChange sc = stageReviewableChange();
review(other, sc.changeId, CC_ON_OWN_COMMENTS);
assertThat(sender)
@@ -2353,7 +2353,7 @@
@Test
public void changeAssigneeOnReviewableChange() throws Exception {
StagedChange sc = stageReviewableChange();
- TestAccount other = accountCreator.create("other", "other@example.com", "other");
+ TestAccount other = accountCreator.create("other", "other@example.com", "other", null);
assign(sc, sc.owner, other);
sender.clear();
assign(sc, sc.owner, sc.assignee);
diff --git a/javatests/com/google/gerrit/acceptance/server/mail/MailProcessorIT.java b/javatests/com/google/gerrit/acceptance/server/mail/MailProcessorIT.java
index 502194a..49f25e4 100644
--- a/javatests/com/google/gerrit/acceptance/server/mail/MailProcessorIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/mail/MailProcessorIT.java
@@ -237,7 +237,7 @@
newPlaintextBody(getChangeUrl(changeInfo) + "/1", "Test Message", null, null, null);
MailMessage.Builder b =
messageBuilderWithDefaultFields()
- .from(user.getEmailAddress())
+ .from(user.getNameEmail())
.textContent(txt + textFooterForChange(changeInfo._number, ts));
sender.clear();
@@ -259,7 +259,7 @@
newPlaintextBody(getChangeUrl(changeInfo) + "/1", "Test Message", null, null, null);
MailMessage.Builder b =
messageBuilderWithDefaultFields()
- .from(user.getEmailAddress())
+ .from(user.getNameEmail())
.textContent(txt + textFooterForChange(changeInfo._number, ts));
sender.clear();
diff --git a/javatests/com/google/gerrit/acceptance/server/project/ProjectWatchIT.java b/javatests/com/google/gerrit/acceptance/server/project/ProjectWatchIT.java
index 29574c4..7ad8e14 100644
--- a/javatests/com/google/gerrit/acceptance/server/project/ProjectWatchIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/project/ProjectWatchIT.java
@@ -241,7 +241,7 @@
List<Message> messages = sender.getMessages();
assertThat(messages).hasSize(1);
Message m = messages.get(0);
- assertThat(m.rcpt()).containsExactly(user.getEmailAddress());
+ assertThat(m.rcpt()).containsExactly(user.getNameEmail());
assertThat(m.body()).contains("Change subject: TRIGGER\n");
assertThat(m.body()).contains("Gerrit-PatchSet: 1\n");
}
@@ -273,13 +273,13 @@
List<Message> messages = sender.getMessages();
assertThat(messages).hasSize(1);
Message m = messages.get(0);
- assertThat(m.rcpt()).containsExactly(user.getEmailAddress());
+ assertThat(m.rcpt()).containsExactly(user.getNameEmail());
assertThat(m.body()).contains("Change subject: TRIGGER\n");
assertThat(m.body()).contains("Gerrit-PatchSet: 1\n");
sender.clear();
// watch project as user2
- TestAccount user2 = accountCreator.create("user2", "user2@test.com", "User2");
+ TestAccount user2 = accountCreator.create("user2", "user2@test.com", "User2", null);
requestScopeOperations.setApiUser(user2.id());
watch(watchedProject);
@@ -295,7 +295,7 @@
messages = sender.getMessages();
assertThat(messages).hasSize(1);
m = messages.get(0);
- assertThat(m.rcpt()).containsExactly(user2.getEmailAddress());
+ assertThat(m.rcpt()).containsExactly(user2.getNameEmail());
assertThat(m.body()).contains("Change subject: TRIGGER_USER2\n");
assertThat(m.body()).contains("Gerrit-PatchSet: 1\n");
}
@@ -322,7 +322,7 @@
List<Message> messages = sender.getMessages();
assertThat(messages).hasSize(1);
Message m = messages.get(0);
- assertThat(m.rcpt()).containsExactly(user.getEmailAddress());
+ assertThat(m.rcpt()).containsExactly(user.getNameEmail());
assertThat(m.body()).contains("Change subject: Document multiprimary setup\n");
assertThat(m.body()).contains("Gerrit-PatchSet: 1\n");
sender.clear();
@@ -357,7 +357,7 @@
List<Message> messages = sender.getMessages();
assertThat(messages).hasSize(1);
Message m = messages.get(0);
- assertThat(m.rcpt()).containsExactly(user.getEmailAddress());
+ assertThat(m.rcpt()).containsExactly(user.getNameEmail());
assertThat(m.body()).contains("Change subject: TRIGGER\n");
assertThat(m.body()).contains("Gerrit-PatchSet: 1\n");
}
@@ -385,13 +385,13 @@
List<Message> messages = sender.getMessages();
assertThat(messages).hasSize(1);
Message m = messages.get(0);
- assertThat(m.rcpt()).containsExactly(user.getEmailAddress());
+ assertThat(m.rcpt()).containsExactly(user.getNameEmail());
assertThat(m.body()).contains("Change subject: TRIGGER\n");
assertThat(m.body()).contains("Gerrit-PatchSet: 1\n");
sender.clear();
// watch project as user2
- TestAccount user2 = accountCreator.create("user2", "user2@test.com", "User2");
+ TestAccount user2 = accountCreator.create("user2", "user2@test.com", "User2", null);
requestScopeOperations.setApiUser(user2.id());
watch(anyProject);
@@ -407,7 +407,7 @@
messages = sender.getMessages();
assertThat(messages).hasSize(1);
m = messages.get(0);
- assertThat(m.rcpt()).containsExactly(user2.getEmailAddress());
+ assertThat(m.rcpt()).containsExactly(user2.getNameEmail());
assertThat(m.body()).contains("Change subject: TRIGGER_USER2\n");
assertThat(m.body()).contains("Gerrit-PatchSet: 1\n");
}
@@ -434,7 +434,7 @@
List<Message> messages = sender.getMessages();
assertThat(messages).hasSize(1);
Message m = messages.get(0);
- assertThat(m.rcpt()).containsExactly(user.getEmailAddress());
+ assertThat(m.rcpt()).containsExactly(user.getNameEmail());
assertThat(m.body()).contains("Change subject: Document multiprimary setup\n");
assertThat(m.body()).contains("Gerrit-PatchSet: 1\n");
sender.clear();
@@ -528,7 +528,7 @@
// watch project as user that can view all private change
TestAccount userThatCanViewPrivateChanges =
accountCreator.create(
- "user2", "user2@test.com", "User2", groupThatCanViewPrivateChanges.name);
+ "user2", "user2@test.com", "User2", null, groupThatCanViewPrivateChanges.name);
requestScopeOperations.setApiUser(userThatCanViewPrivateChanges.id());
watch(watchedProject);
@@ -547,7 +547,7 @@
List<Message> messages = sender.getMessages();
assertThat(messages).hasSize(1);
Message m = messages.get(0);
- assertThat(m.rcpt()).containsExactly(userThatCanViewPrivateChanges.getEmailAddress());
+ assertThat(m.rcpt()).containsExactly(userThatCanViewPrivateChanges.getNameEmail());
assertThat(m.body()).contains("Change subject: TRIGGER\n");
assertThat(m.body()).contains("Gerrit-PatchSet: 1\n");
}
diff --git a/lib/ba-linkify/src/ba-linkify.js b/lib/ba-linkify/src/ba-linkify.js
index 32fbea3..461aff9 100644
--- a/lib/ba-linkify/src/ba-linkify.js
+++ b/lib/ba-linkify/src/ba-linkify.js
@@ -23,6 +23,12 @@
* FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
* OTHER DEALINGS IN THE SOFTWARE.
*/
+ /**
+ * Note(taoalpha):
+ *
+ * To support emails with dots in the name: `foo.bar@test.com`,
+ * the match regex was modified to match `email` first before urls.
+ */
/*!
* JavaScript Linkify - v0.3 - 6/27/2009
* http://benalman.com/projects/javascript-linkify/
@@ -108,7 +114,7 @@
MAILTO = "mailto:",
EMAIL = "(?:" + MAILTO + ")?[a-z0-9!#$%&'*+/=?^_`{|}~-]+(?:\\.[a-z0-9!#$%&'*+/=?^_`{|}~-]+)*@" + HOST_OR_IP + QUERY_FRAG + "(?!\\w)",
- URI_RE = new RegExp( "(?:" + URI1 + "|" + URI2 + "|" + EMAIL + ")", "ig" ),
+ URI_RE = new RegExp( "(?:" + EMAIL + "|" + URI1 + "|" + URI2 + ")", "ig" ),
SCHEME_RE = new RegExp( "^" + SCHEME, "i" ),
quotes = {
diff --git a/plugins/replication b/plugins/replication
index 8be6db9..718d621 160000
--- a/plugins/replication
+++ b/plugins/replication
@@ -1 +1 @@
-Subproject commit 8be6db961b2c6c3a32e0b3db2f04475ca2c4c520
+Subproject commit 718d6210721cbfc38bbac6a3d976636fe17e4b72
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 bd21015..ff6a093 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
@@ -255,6 +255,9 @@
gr-messages-list {
display: block;
}
+ gr-thread-list {
+ min-height: 250px;
+ }
#includedInOverlay {
width: 65em;
}
@@ -535,23 +538,25 @@
</div>
</section>
- <section class="patchInfo">
- <template is="dom-if" if="[[_showPrimaryTabs]]">
- <paper-tabs id="primaryTabs" on-selected-changed="_handleFileTabChange">
- <paper-tab data-name$="[[_files_tab_name]]">Files</paper-tab>
- <template is="dom-repeat" items="[[_dynamicTabHeaderEndpoints]]"
- as="tabHeader">
- <paper-tab data-name$="[[tabHeader]]">
- <gr-endpoint-decorator name$="[[tabHeader]]">
- <gr-endpoint-param name="change" value="[[_change]]">
- </gr-endpoint-param>
- <gr-endpoint-param name="revision" value="[[_selectedRevision]]">
- </gr-endpoint-param>
- </gr-endpoint-decorator>
- </paper-tab>
- </template>
- </paper-tabs>
+ <paper-tabs id="primaryTabs" on-selected-changed="_handleFileTabChange">
+ <paper-tab data-name$="[[_files_tab_name]]">Files</paper-tab>
+ <template is="dom-repeat" items="[[_dynamicTabHeaderEndpoints]]"
+ as="tabHeader">
+ <paper-tab data-name$="[[tabHeader]]">
+ <gr-endpoint-decorator name$="[[tabHeader]]">
+ <gr-endpoint-param name="change" value="[[_change]]">
+ </gr-endpoint-param>
+ <gr-endpoint-param name="revision" value="[[_selectedRevision]]">
+ </gr-endpoint-param>
+ </gr-endpoint-decorator>
+ </paper-tab>
</template>
+ <paper-tab data-name$="[[_findings_tab_name]]">
+ Findings
+ </paper-tab>
+ </paper-tabs>
+
+ <section class="patchInfo">
<div hidden$="[[!_findIfTabMatches(_currentTabName, _files_tab_name)]]">
<gr-file-list-header
id="fileListHeader"
@@ -573,15 +578,14 @@
base-patch-num="{{_patchRange.basePatchNum}}"
files-expanded="[[_filesExpanded]]"
diff-prefs-disabled="[[_diffPrefsDisabled]]"
- show-title="[[!_showPrimaryTabs]]"
on-open-diff-prefs="_handleOpenDiffPrefs"
on-open-download-dialog="_handleOpenDownloadDialog"
on-open-upload-help-dialog="_handleOpenUploadHelpDialog"
on-open-included-in-dialog="_handleOpenIncludedInDialog"
on-expand-diffs="_expandAllDiffs"
on-collapse-diffs="_collapseAllDiffs">
- </gr-file-list-header>
- <gr-file-list
+ </gr-file-list-header>
+ <gr-file-list
id="fileList"
class="hideOnMobileOverlay"
diff-prefs="{{_diffPrefs}}"
@@ -600,9 +604,27 @@
file-list-increment="{{_numFilesShown}}"
on-files-shown-changed="_setShownFiles"
on-file-action-tap="_handleFileActionTap"
- on-reload-drafts="_reloadDraftsWithCallback"></gr-file-list>
+ on-reload-drafts="_reloadDraftsWithCallback">
+ </gr-file-list>
</div>
+ <template is="dom-if" if="[[_findIfTabMatches(_currentTabName, _findings_tab_name)]]">
+ <gr-dropdown-list
+ class="patch-set-dropdown"
+ items="[[_robotCommentsPatchSetDropdownItems]]"
+ on-value-change="_handleRobotCommentPatchSetChanged"
+ value="[[_currentRobotCommentsPatchSet]]">
+ </gr-dropdown-list>
+ <gr-thread-list
+ threads="[[_robotCommentThreads]]"
+ change="[[_change]]"
+ change-num="[[_changeNum]]"
+ logged-in="[[_loggedIn]]"
+ tab="[[_findings_tab_name]]"
+ hide-toggle-buttons
+ on-thread-list-modified="_handleReloadDiffComments"></gr-thread-list>
+ </template>
+
<template is="dom-if" if="[[_findIfTabMatches(_currentTabName, _selectedTabPluginHeader)]]">
<gr-endpoint-decorator name$="[[_selectedTabPluginEndpoint]]">
<gr-endpoint-param name="change" value="[[_change]]">
@@ -631,9 +653,7 @@
title$="[[_computeTotalCommentCounts(_change.unresolved_comment_count, _changeComments)]]">
<span>Comment Threads</span></gr-tooltip-content>
</paper-tab>
- <paper-tab class="robotComments">Findings</paper-tab>
</paper-tabs>
-
<section class="changeLog">
<template is="dom-if" if="[[_isSelectedView(_currentView,
_commentTabs.CHANGE_LOG)]]">
@@ -655,28 +675,10 @@
threads="[[_commentThreads]]"
change="[[_change]]"
change-num="[[_changeNum]]"
- comment-tab="[[_currentView]]"
logged-in="[[_loggedIn]]"
only-show-robot-comments-with-human-reply
on-thread-list-modified="_handleReloadDiffComments"></gr-thread-list>
</template>
- <template is="dom-if" if="[[_isSelectedView(_currentView,
- _commentTabs.ROBOT_COMMENTS)]]">
- <gr-dropdown-list
- class="patch-set-dropdown"
- items="[[_robotCommentsPatchSetDropdownItems]]"
- on-value-change="_handleRobotCommentPatchSetChanged"
- value="[[_currentRobotCommentsPatchSet]]">
- </gr-dropdown-list>
- <gr-thread-list
- threads="[[_robotCommentThreads]]"
- change="[[_change]]"
- change-num="[[_changeNum]]"
- logged-in="[[_loggedIn]]"
- comment-tab="[[_currentView]]"
- hide-toggle-buttons
- on-thread-list-modified="_handleReloadDiffComments"></gr-thread-list>
- </template>
</section>
</div>
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 8dcd8b8..929c214 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
@@ -68,7 +68,9 @@
const CHANGE_DATA_TIMING_LABEL = 'ChangeDataLoaded';
const CHANGE_RELOAD_TIMING_LABEL = 'ChangeReloaded';
const SEND_REPLY_TIMING_LABEL = 'SendReply';
- const FILES_TAB_NAME = 'files';
+ // Making the tab names more unique in case a plugin adds one with same name
+ const FILES_TAB_NAME = '__gerrit_internal_files';
+ const FINDINGS_TAB_NAME = '__gerrit_internal_findings';
/**
* @appliesMixin Gerrit.FireMixin
@@ -315,10 +317,6 @@
_dynamicTabHeaderEndpoints: {
type: Array,
},
- _showPrimaryTabs: {
- type: Boolean,
- computed: '_computeShowPrimaryTabs(_dynamicTabHeaderEndpoints)',
- },
/** @type {Array<string>} */
_dynamicTabContentEndpoints: {
type: Array,
@@ -334,7 +332,8 @@
_robotCommentsPatchSetDropdownItems: {
type: Array,
value() { return []; },
- computed: '_computeRobotCommentsPatchSetDropdownItems(_change)',
+ computed: '_computeRobotCommentsPatchSetDropdownItems(_change, ' +
+ '_commentThreads)',
},
_currentRobotCommentsPatchSet: {
type: Number,
@@ -343,6 +342,10 @@
type: String,
value: FILES_TAB_NAME,
},
+ _findings_tab_name: {
+ type: String,
+ value: FINDINGS_TAB_NAME,
+ },
_currentTabName: {
type: String,
value: FILES_TAB_NAME,
@@ -629,13 +632,35 @@
return false;
}
- _computeRobotCommentsPatchSetDropdownItems(change) {
- if (!change.revisions) return [];
+ _robotCommentCountPerPatchSet(threads) {
+ return threads.reduce((robotCommentCountMap, thread) => {
+ const comments = thread.comments;
+ const robotCommentsCount = comments.reduce((acc, comment) =>
+ (comment.robot_id ? acc + 1 : acc), 0);
+ robotCommentCountMap[comments[0].patch_set] =
+ (robotCommentCountMap[comments[0].patch_set] || 0) +
+ robotCommentsCount;
+ return robotCommentCountMap;
+ }, {});
+ }
+
+ _computeText(patch, commentThreads) {
+ const commentCount = this._robotCommentCountPerPatchSet(commentThreads);
+ const commentCnt = commentCount[patch._number] || 0;
+ if (commentCnt === 0) return `Patchset ${patch._number}`;
+ const findingsText = commentCnt === 1 ? 'finding' : 'findings';
+ return `Patchset ${patch._number}`
+ + ` (${commentCnt} ${findingsText})`;
+ }
+
+ _computeRobotCommentsPatchSetDropdownItems(change, commentThreads) {
+ if (!change || !commentThreads || !change.revisions) return [];
+
return Object.values(change.revisions)
.filter(patch => patch._number !== 'edit')
.map(patch => {
return {
- text: 'Patchset ' + patch._number,
+ text: this._computeText(patch, commentThreads),
value: patch._number,
};
})
@@ -1111,10 +1136,6 @@
return 'PARENT';
}
- _computeShowPrimaryTabs(dynamicTabHeaderEndpoints) {
- return dynamicTabHeaderEndpoints && dynamicTabHeaderEndpoints.length > 0;
- }
-
_computeChangeUrl(change) {
return Gerrit.Nav.getUrlForChange(change);
}
@@ -1470,8 +1491,14 @@
} else {
this._selectedRevision =
Object.values(this._change.revisions).find(
- revision => revision._number ===
- parseInt(this._patchRange.patchNum, 10));
+ revision => {
+ // edit patchset is a special one
+ const thePatchNum = this._patchRange.patchNum;
+ if (thePatchNum === 'edit') {
+ return revision._number === thePatchNum;
+ }
+ return revision._number === parseInt(thePatchNum, 10);
+ });
}
});
}
@@ -1963,8 +1990,12 @@
if (!this._selectedRevision) {
return;
}
- // If patchNumStr is"edit", then patchNum is undefined hence an OR
- const patchNum = parseInt(patchNumStr, 10) || patchNumStr;
+
+ let patchNum = parseInt(patchNumStr, 10);
+ if (patchNumStr === 'edit') {
+ patchNum = patchNumStr;
+ }
+
if (patchNum === this._selectedRevision._number) {
return;
}
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
index 3fe8faf..17a6921 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
@@ -80,6 +80,21 @@
name: 'user',
username: 'user',
},
+ patch_set: 2,
+ robot_id: 'rb1',
+ id: 'ecf9fa_fe1a5f62',
+ line: 5,
+ updated: '2018-02-08 18:49:18.000000000',
+ message: 'test',
+ unresolved: true,
+ },
+ {
+ __path: '/COMMIT_MSG',
+ author: {
+ _account_id: 1000000,
+ name: 'user',
+ username: 'user',
+ },
patch_set: 4,
id: 'ecf0b9fa_fe1a5f62',
line: 5,
@@ -110,6 +125,21 @@
{
comments: [
{
+ __path: '/COMMIT_MSG',
+ author: {
+ _account_id: 1000000,
+ name: 'user',
+ username: 'user',
+ },
+ patch_set: 3,
+ id: 'ecf0b9fa_fe5f62',
+ robot_id: 'rb2',
+ line: 5,
+ updated: '2018-02-08 18:49:18.000000000',
+ message: 'test',
+ unresolved: true,
+ },
+ {
__path: 'test.txt',
author: {
_account_id: 1000000,
@@ -330,7 +360,8 @@
assert(element._dynamicTabHeaderEndpoints.includes(
'change-view-tab-header-url'));
const paperTabs = element.shadowRoot.querySelector('#primaryTabs');
- assert.equal(paperTabs.querySelectorAll('paper-tab').length, 2);
+ // 3 Tabs are : Files, Plugin, Findings
+ assert.equal(paperTabs.querySelectorAll('paper-tab').length, 3);
assert.equal(paperTabs.querySelectorAll('paper-tab')[1].dataset.name,
'change-view-tab-header-url');
});
@@ -734,6 +765,23 @@
done();
});
});
+
+ test('robot comments count per patchset', () => {
+ const count = element._robotCommentCountPerPatchSet(THREADS);
+ const expectedCount = {
+ 2: 1,
+ 3: 1,
+ 4: 2,
+ };
+ assert.deepEqual(count, expectedCount);
+ assert.equal(element._computeText({_number: 2}, THREADS),
+ 'Patchset 2 (1 finding)');
+ assert.equal(element._computeText({_number: 4}, THREADS),
+ 'Patchset 4 (2 findings)');
+ assert.equal(element._computeText({_number: 5}, THREADS),
+ 'Patchset 5');
+ });
+
test('only robot comments are rendered', () => {
assert.equal(element._robotCommentThreads.length, 2);
assert.equal(element._robotCommentThreads[0].comments[0].robot_id,
@@ -744,7 +792,7 @@
test('changing patchsets resets robot comments', done => {
element.set('_change.current_revision', 'rev3');
flush(() => {
- assert.equal(element._robotCommentThreads.length, 0);
+ assert.equal(element._robotCommentThreads.length, 1);
done();
});
});
diff --git a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.html b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.html
index 600f2ab..79f6c50 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.html
+++ b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.html
@@ -154,10 +154,6 @@
</style>
<div class$="patchInfo-header [[_computeEditModeClass(editMode)]] [[_computePatchInfoClass(patchNum, allPatchSets)]]">
<div class="patchInfo-left">
- <template is="dom-if"
- if="[[showTitle]]">
- <h3 class="label">Files</h3>
- </template>
<div class="patchInfoContent">
<gr-patch-range-select
id="rangeSelect"
diff --git a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.js b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.js
index eb7c1f9..34e1cfa 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.js
+++ b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.js
@@ -93,10 +93,6 @@
type: String,
value: '',
},
- showTitle: {
- type: Boolean,
- value: true,
- },
_descriptionReadOnly: {
type: Boolean,
computed: '_computeDescriptionReadOnly(loggedIn, change, account)',
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
index 8fc61b7..53e852f 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
@@ -36,6 +36,7 @@
<link rel="import" href="../../shared/gr-select/gr-select.html">
<link rel="import" href="../../shared/gr-count-string-formatter/gr-count-string-formatter.html">
<link rel="import" href="../../shared/gr-tooltip-content/gr-tooltip-content.html">
+<link rel="import" href="../../shared/gr-copy-clipboard/gr-copy-clipboard.html">
<link rel="import" href="../gr-file-list-constants.html">
<dom-module id="gr-file-list">
@@ -115,14 +116,10 @@
.path {
cursor: pointer;
flex: 1;
- text-decoration: none;
/* Wrap it into multiple lines if too long. */
white-space: normal;
word-break: break-word;
}
- .path:hover :first-child {
- text-decoration: underline;
- }
.oldPath {
color: var(--deemphasized-text-color);
}
@@ -245,6 +242,26 @@
display: inline-block;
margin: -2px 0;
padding: var(--spacing-s) 0;
+ text-decoration: none;
+ }
+ .pathLink:hover {
+ text-decoration: underline;
+ }
+
+ /** copy on file path **/
+ .pathLink gr-copy-clipboard,
+ .oldPath gr-copy-clipboard {
+ display: inline-block;
+ visibility: hidden;
+ vertical-align: bottom;
+ text-decoration: none;
+ --gr-button: {
+ padding: 0px;
+ }
+ }
+ .pathLink:hover gr-copy-clipboard,
+ .oldPath:hover gr-copy-clipboard {
+ visibility: visible;
}
/** small screen breakpoint: 768px */
@@ -273,7 +290,7 @@
}
.expanded .fullFileName,
.truncatedFileName {
- display: block;
+ display: inline;
}
.expanded .truncatedFileName,
.fullFileName {
@@ -330,11 +347,18 @@
class="truncatedFileName">
[[computeTruncatedPath(file.__path)]]
</span>
+ <gr-copy-clipboard
+ hide-input
+ text="[[file.__path]]"></gr-copy-clipboard>
</a>
- <div class="oldPath" hidden$="[[!file.old_path]]" hidden
- title$="[[file.old_path]]">
- [[file.old_path]]
- </div>
+ <template is="dom-if" if="[[file.old_path]]">
+ <div class="oldPath" title$="[[file.old_path]]">
+ [[file.old_path]]
+ <gr-copy-clipboard
+ hide-input
+ text="[[file.old_path]]"></gr-copy-clipboard>
+ </div>
+ </template>
</span>
<div class="comments desktop">
<span class="drafts">
diff --git a/polygerrit-ui/app/elements/change/gr-message/gr-message.html b/polygerrit-ui/app/elements/change/gr-message/gr-message.html
index b807b37..f7ef7e6 100644
--- a/polygerrit-ui/app/elements/change/gr-message/gr-message.html
+++ b/polygerrit-ui/app/elements/change/gr-message/gr-message.html
@@ -36,7 +36,6 @@
</style>
<style include="shared-styles">
:host {
- border-bottom: 1px solid var(--border-color);
display: block;
position: relative;
cursor: pointer;
diff --git a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.html b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.html
index 6154ca5..60ec6b0 100644
--- a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.html
+++ b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.html
@@ -64,6 +64,9 @@
align-items: center;
display: flex;
}
+ gr-message:not(:last-of-type) {
+ border-bottom: 1px solid var(--border-color);
+ }
gr-message:nth-child(2n) {
background-color: var(--background-color-secondary);
}
diff --git a/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list.html b/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list.html
index eacc0d0..f04a39a 100644
--- a/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list.html
+++ b/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list.html
@@ -25,7 +25,6 @@
<style include="shared-styles">
#threads {
display: block;
- min-height: 20rem;
padding: var(--spacing-l);
}
gr-comment-thread {
@@ -76,7 +75,7 @@
</template>
<div id="threads">
<template is="dom-if" if="[[!threads.length]]">
- [[_computeNoThreadsMessage(commentTab)]]
+ [[_computeNoThreadsMessage(tab)]]
</template>
<template
is="dom-repeat"
diff --git a/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list.js b/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list.js
index d8c7b61..e99367e 100644
--- a/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list.js
+++ b/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list.js
@@ -27,12 +27,7 @@
+ 'for this change.';
const NO_ROBOT_COMMENTS_THREADS_MESSAGE = 'There are no findings for this ' +
'patchset.';
-
- const CommentTabs = {
- CHANGE_LOG: 0,
- COMMENT_THREADS: 1,
- ROBOT_COMMENTS: 2,
- };
+ const FINDINGS_TAB_NAME = '__gerrit_internal_findings';
class GrThreadList extends Polymer.GestureEventListeners(
Polymer.LegacyElementMixin(
@@ -73,7 +68,10 @@
type: Boolean,
value: false,
},
- commentTab: Number,
+ tab: {
+ type: String,
+ value: '',
+ },
};
}
@@ -83,8 +81,8 @@
return loggedIn ? 'show' : '';
}
- _computeNoThreadsMessage(commentTab) {
- if (commentTab === CommentTabs.ROBOT_COMMENTS) {
+ _computeNoThreadsMessage(tab) {
+ if (tab === FINDINGS_TAB_NAME) {
return NO_ROBOT_COMMENTS_THREADS_MESSAGE;
}
return NO_THREADS_MESSAGE;
diff --git a/polygerrit-ui/app/elements/core/gr-smart-search/gr-smart-search.js b/polygerrit-ui/app/elements/core/gr-smart-search/gr-smart-search.js
index 664d59f..cfdd524 100644
--- a/polygerrit-ui/app/elements/core/gr-smart-search/gr-smart-search.js
+++ b/polygerrit-ui/app/elements/core/gr-smart-search/gr-smart-search.js
@@ -72,10 +72,6 @@
}
}
- _accountOrAnon(name) {
- return this.getUserName(this._serverConfig, name, false);
- }
-
/**
* Fetch from the API the predicted projects.
*
@@ -154,11 +150,12 @@
_mapAccountsHelper(accounts, predicate) {
return accounts.map(account => {
+ const userName = this.getUserName(this._serverConfig, account, false);
return {
label: account.name || '',
text: account.email ?
`${predicate}:${account.email}` :
- `${predicate}:"${this._accountOrAnon(account)}"`,
+ `${predicate}:"${userName}"`,
};
});
}
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html
index 7705186..947ccbd 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html
@@ -313,13 +313,15 @@
on-click="_toggleBlame">[[_computeBlameToggleLabel(_isBlameLoaded, _isBlameLoading)]]</gr-button>
</span>
<template is="dom-if" if="[[_computeIsLoggedIn(_loggedIn)]]">
+ <span class="separator"></span>
<span class="editButton">
- <a href$="[[_computeEditURL(_change, _patchRange, _path)]]">
- <iron-icon icon="gr-icons:edit"></iron-icon>
- </a>
- <span class="separator"></span>
+ <gr-button
+ link
+ title="Edit current file"
+ on-click="_goToEditFile">edit</gr-button>
</span>
</template>
+ <span class="separator"></span>
<div class$="diffModeSelector [[_computeModeSelectHideClass(_isImageDiff)]]">
<span>Diff view:</span>
<gr-diff-mode-selector
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
index e070a01..eb5ea017 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
@@ -604,12 +604,11 @@
return this._getDiffUrl(this._change, this._patchRange, newPath.path);
}
- _computeEditURL(change, patchRange, path) {
- if ([change, patchRange, path].some(arg => arg === undefined)) {
- return '';
- }
- return Gerrit.Nav.getEditUrlForDiff(
- change, path, patchRange.patchNum);
+ _goToEditFile() {
+ // TODO(taoalpha): add a shortcut for editing
+ const editUrl = Gerrit.Nav.getEditUrlForDiff(
+ this._change, this._path, this._patchRange.patchNum);
+ return Gerrit.Nav.navigateToRelativeUrl(editUrl);
}
/**
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.html b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.html
index 7636282..88ae24f 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.html
@@ -376,10 +376,51 @@
PARENT), 'Should navigate to /c/42/1');
});
- test('_computeEditURL uses Gerrit.Nav', () => {
- const getUrlStub = sandbox.stub(Gerrit.Nav, 'getEditUrlForDiff');
- element._computeEditURL(123, {patchNum: undefined}, 'abc/def');
- assert.isTrue(getUrlStub.called);
+ test('edit should redirect to edit page', done => {
+ element._loggedIn = true;
+ element._path = 't.txt';
+ element._patchRange = {
+ basePatchNum: PARENT,
+ patchNum: '1',
+ };
+ element._change = {
+ _number: 42,
+ revisions: {
+ a: {_number: 1, commit: {parents: []}},
+ b: {_number: 2, commit: {parents: []}},
+ },
+ };
+ const redirectStub = sandbox.stub(Gerrit.Nav, 'navigateToRelativeUrl');
+ flush(() => {
+ const editBtn = element.shadowRoot
+ .querySelector('.editButton gr-button');
+ assert.isTrue(!!editBtn);
+ MockInteractions.tap(editBtn);
+ assert.isTrue(redirectStub.called);
+ done();
+ });
+ });
+
+ test('edit hidden when not logged in', done => {
+ element._loggedIn = false;
+ element._path = 't.txt';
+ element._patchRange = {
+ basePatchNum: PARENT,
+ patchNum: '1',
+ };
+ element._change = {
+ _number: 42,
+ revisions: {
+ a: {_number: 1, commit: {parents: []}},
+ b: {_number: 2, commit: {parents: []}},
+ },
+ };
+ flush(() => {
+ const editBtn = element.shadowRoot
+ .querySelector('.editButton gr-button');
+ assert.isFalse(!!editBtn);
+ done();
+ });
});
suite('diff prefs hidden', () => {
diff --git a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.html b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.html
index 5797ff7..18ffc0e 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.html
+++ b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.html
@@ -325,6 +325,7 @@
</div>
<div>
<a
+ tabIndex="-1"
on-click="_onRespectfulReadMoreClick"
href="https://testing.googleblog.com/2019/11/code-health-respectful-reviews-useful.html"
target="_blank">
diff --git a/polygerrit-ui/app/elements/shared/gr-copy-clipboard/gr-copy-clipboard.js b/polygerrit-ui/app/elements/shared/gr-copy-clipboard/gr-copy-clipboard.js
index 2ce03e3..9be6852 100644
--- a/polygerrit-ui/app/elements/shared/gr-copy-clipboard/gr-copy-clipboard.js
+++ b/polygerrit-ui/app/elements/shared/gr-copy-clipboard/gr-copy-clipboard.js
@@ -53,7 +53,10 @@
Polymer.dom(e).rootTarget.select();
}
- _copyToClipboard() {
+ _copyToClipboard(e) {
+ e.preventDefault();
+ e.stopPropagation();
+
if (this.hideInput) {
this.$.input.style.display = 'block';
}
diff --git a/polygerrit-ui/app/elements/shared/gr-copy-clipboard/gr-copy-clipboard_test.html b/polygerrit-ui/app/elements/shared/gr-copy-clipboard/gr-copy-clipboard_test.html
index 55eb198..05014f1 100644
--- a/polygerrit-ui/app/elements/shared/gr-copy-clipboard/gr-copy-clipboard_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-copy-clipboard/gr-copy-clipboard_test.html
@@ -90,5 +90,16 @@
flushAsynchronousOperations();
assert.equal(getComputedStyle(element.$.input).display, 'none');
});
+
+ test('stop events propagation', () => {
+ const divParent = document.createElement('div');
+ divParent.appendChild(element);
+ const clickStub = sinon.stub();
+ divParent.addEventListener('click', clickStub);
+ element.stopPropagation = true;
+ const copyBtn = element.shadowRoot.querySelector('.copyToClipboard');
+ MockInteractions.tap(copyBtn);
+ assert.isFalse(clickStub.called);
+ });
});
</script>
diff --git a/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text.js b/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text.js
index 10f7ed6..f970734 100644
--- a/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text.js
+++ b/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text.js
@@ -79,10 +79,11 @@
// Ensure that external links originating from HTML commentlink configs
// open in a new tab. @see Issue 5567
// Ensure links to the same host originating from commentlink configs
- // open in the same tab. @see Issue 4616
+ // open in the same tab. When target is not set - default is _self
+ // @see Issue 4616
output.querySelectorAll('a').forEach(anchor => {
if (anchor.hostname === window.location.hostname) {
- anchor.setAttribute('target', '_self');
+ anchor.removeAttribute('target');
} else {
anchor.setAttribute('target', '_blank');
}
diff --git a/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text_test.html b/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text_test.html
index ff94bc7..8a225f3 100644
--- a/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text_test.html
@@ -146,7 +146,7 @@
const linkEl = element.$.output.childNodes[1];
assert.equal(textNode.textContent, prefix);
const url = '/q/' + changeID;
- assert.equal(linkEl.target, '_self');
+ assert.isFalse(linkEl.hasAttribute('target'));
// Since url is a path, the host is added automatically.
assert.isTrue(linkEl.href.endsWith(url));
assert.equal(linkEl.textContent, changeID);
@@ -164,7 +164,7 @@
const linkEl = element.$.output.childNodes[1];
assert.equal(textNode.textContent, prefix);
const url = '/r/q/' + changeID;
- assert.equal(linkEl.target, '_self');
+ assert.isFalse(linkEl.hasAttribute('target'));
// Since url is a path, the host is added automatically.
assert.isTrue(linkEl.href.endsWith(url));
assert.equal(linkEl.textContent, changeID);
@@ -205,7 +205,7 @@
assert.equal(textNode.textContent, prefix);
- assert.equal(changeLinkEl.target, '_self');
+ assert.isFalse(changeLinkEl.hasAttribute('target'));
assert.isTrue(changeLinkEl.href.endsWith(changeUrl));
assert.equal(changeLinkEl.textContent, changeID);
diff --git a/polygerrit-ui/app/scripts/gr-display-name-utils/gr-display-name-utils.js b/polygerrit-ui/app/scripts/gr-display-name-utils/gr-display-name-utils.js
index 6e503c7..f0d0e7f 100644
--- a/polygerrit-ui/app/scripts/gr-display-name-utils/gr-display-name-utils.js
+++ b/polygerrit-ui/app/scripts/gr-display-name-utils/gr-display-name-utils.js
@@ -44,17 +44,13 @@
}
static getAccountDisplayName(config, account, enableEmail) {
- const reviewerName = this._accountOrAnon(config, account, enableEmail);
+ const reviewerName = this.getUserName(config, account, !!enableEmail);
const reviewerEmail = this._accountEmail(account.email);
const reviewerStatus = account.status ? '(' + account.status + ')' : '';
return [reviewerName, reviewerEmail, reviewerStatus]
.filter(p => p.length > 0).join(' ');
}
- static _accountOrAnon(config, reviewer, enableEmail) {
- return this.getUserName(config, reviewer, !!enableEmail);
- }
-
static _accountEmail(email) {
if (typeof email !== 'undefined') {
return '<' + email + '>';
diff --git a/resources/com/google/gerrit/httpd/raw/PolyGerritIndexHtml.soy b/resources/com/google/gerrit/httpd/raw/PolyGerritIndexHtml.soy
index dbb47a3..212f504 100644
--- a/resources/com/google/gerrit/httpd/raw/PolyGerritIndexHtml.soy
+++ b/resources/com/google/gerrit/httpd/raw/PolyGerritIndexHtml.soy
@@ -25,8 +25,6 @@
{@param? faviconPath: ?}
{@param? versionInfo: ?}
{@param? polyfillCE: ?}
- {@param? polyfillSD: ?}
- {@param? polyfillSC: ?}
{@param? useGoogleFonts: ?}
{@param? changeRequestsPath: ?}
{@param? defaultChangeDetailHex: ?}
@@ -62,8 +60,6 @@
{if $staticResourcePath != ''}window.STATIC_RESOURCE_PATH = '{$staticResourcePath}';{/if}
{if $assetsPath}window.ASSETS_PATH = '{$assetsPath}';{/if}
{if $polyfillCE}if (window.customElements) window.customElements.forcePolyfill = true;{/if}
- {if $polyfillSD}{literal}ShadyDOM = { force: true };{/literal}{/if}
- {if $polyfillSC}{literal}ShadyCSS = { shimcssproperties: true};{/literal}{/if}
{if $gerritInitialData}
// INITIAL_DATA is a string that represents a JSON map. It's inlined here so that we can
// spare calls to the API when starting up the app.
