Merge "Reload repo and group list after creating a repo or group"
diff --git a/.gitignore b/.gitignore
index e544356..319b3cf 100644
--- a/.gitignore
+++ b/.gitignore
@@ -31,3 +31,4 @@
/test_site
/tools/format
/.vscode
+/.ijwb
diff --git a/BUILD b/BUILD
index d924417..a104dd5 100644
--- a/BUILD
+++ b/BUILD
@@ -2,10 +2,6 @@
load("//tools/bzl:genrule2.bzl", "genrule2")
load("//tools/bzl:pkg_war.bzl", "pkg_war")
-load(
- "@bazel_tools//tools/jdk:default_java_toolchain.bzl",
- "default_java_toolchain",
-)
config_setting(
name = "java9",
@@ -15,42 +11,12 @@
)
config_setting(
- name = "java10",
+ name = "java_next",
values = {
- "java_toolchain": ":toolchain_vanilla",
+ "java_toolchain": "@bazel_tools//tools/jdk:toolchain_vanilla",
},
)
-# TODO(davido): Switch to consuming it from @bazel_tool//tools/jdk:absolute_javabase
-# when new Bazel version is released with this change included:
-# https://github.com/bazelbuild/bazel/issues/6012
-# https://github.com/bazelbuild/bazel/commit/0173bdbf7bdd1874379d4dd3eb70d5321e0f1816
-# As the interim use a hack that works around it by putting the variable reference
-# behind a select
-config_setting(
- name = "use_absolute_javabase",
- values = {"define": "USE_ABSOLUTE_JAVABASE=true"},
-)
-
-java_runtime(
- name = "absolute_javabase",
- java_home = select({
- ":use_absolute_javabase": "$(ABSOLUTE_JAVABASE)",
- "//conditions:default": "",
- }),
- visibility = ["//visibility:public"],
-)
-
-# TODO(davido): Switch to consuming it from @bazel_tool//tools/jdk:toolchain_vanilla
-# when my change is included in released Bazel version:
-# https://github.com/bazelbuild/bazel/commit/0bef68e054eccecd690e5d9f46db8a0c4b2d887a
-default_java_toolchain(
- name = "toolchain_vanilla",
- forcibly_disable_header_compilation = True,
- javabuilder = ["@bazel_tools//tools/jdk:VanillaJavaBuilder_deploy.jar"],
- jvm_opts = [],
-)
-
genrule(
name = "gen_version",
outs = ["version.txt"],
diff --git a/Documentation/dev-bazel.txt b/Documentation/dev-bazel.txt
index a68c3ac..aa8609a 100644
--- a/Documentation/dev-bazel.txt
+++ b/Documentation/dev-bazel.txt
@@ -6,7 +6,7 @@
To build Gerrit from source, you need:
* A Linux or macOS system (Windows is not supported at this time)
-* A JDK for Java 8|9|10
+* A JDK for Java 8|9|10|11|...
* Python 2 or 3
* Node.js
* link:https://www.bazel.io/versions/master/docs/install.html[Bazel]
@@ -14,27 +14,55 @@
* zip, unzip
* gcc
-[[Java 10 support]]
-Java 10 is supported through vanilla java toolchain
+[[Java 10 and newer version support]]
+Java 10 (and newer is) supported through vanilla java toolchain
link:https://docs.bazel.build/versions/master/toolchains.html[Bazel option].
-To build Gerrit with Java 10, specify vanilla java toolchain and provide
-path to Java 10 home:
+To build Gerrit with Java 10 and newer, specify vanilla java toolchain and
+provide the path to JDK home:
```
- $ bazel build --host_javabase=:absolute_javabase \
+ $ bazel build \
--define=ABSOLUTE_JAVABASE=<path-to-java-10> \
- --define=USE_ABSOLUTE_JAVABASE=true \
- --host_java_toolchain=//:toolchain_vanilla \
- --java_toolchain=//:toolchain_vanilla \
+ --host_javabase=@bazel_tools//tools/jdk:absolute_javabase \
+ --host_java_toolchain=@bazel_tools//tools/jdk:toolchain_vanilla \
+ --java_toolchain=@bazel_tools//tools/jdk:toolchain_vanilla \
:release
```
-Note that the following options must be added to `container.javaOptions`
-in `$gerrit_site/etc/gerrit.config` to run Gerrit with Java 10:
+To run the tests, `--javabase` option must be passed as well, because
+bazel test runs the test using the target javabase:
+
+```
+ $ bazel test \
+ --define=ABSOLUTE_JAVABASE=<path-to-java-10> \
+ --javabase=@bazel_tools//tools/jdk:absolute_javabase \
+ --host_javabase=@bazel_tools//tools/jdk:absolute_javabase \
+ --host_java_toolchain=@bazel_tools//tools/jdk:toolchain_vanilla \
+ --java_toolchain=@bazel_tools//tools/jdk:toolchain_vanilla \
+ //...
+```
+
+To avoid passing all those options on every Bazel build invocation,
+they could be added to ~/.bazelrc resource file:
+
+```
+$ cat << EOF > ~/.bazelrc
+> build --define=ABSOLUTE_JAVABASE=<path-to-java-10>
+> build --javabase=@bazel_tools//tools/jdk:absolute_javabase
+> build --host_javabase=@bazel_tools//tools/jdk:absolute_javabase
+> build --host_java_toolchain=@bazel_tools//tools/jdk:toolchain_vanilla
+> build --java_toolchain=@bazel_tools//tools/jdk:toolchain_vanilla
+> EOF
+```
+
+Now, invoking Bazel with just `bazel build :release` would include
+all those options.
+
+Note that the follow option must be added to `container.javaOptions`
+in `$gerrit_site/etc/gerrit.config` to run Gerrit with Java 10|11|...:
```
[container]
- javaOptions = --add-modules java.activation
javaOptions = --add-opens=jdk.management/com.sun.management.internal=ALL-UNNAMED
```
@@ -51,13 +79,12 @@
:release
```
-Note that the following option must be added to `container.javaOptions`
+Note that the follow option must be added to `container.javaOptions`
in `$gerrit_site/etc/gerrit.config` to run Gerrit with Java 9:
```
[container]
- javaOptions = --add-modules java.activation \
- --add-opens=jdk.management/com.sun.management.internal=ALL-UNNAMED
+ javaOptions = --add-opens=jdk.management/com.sun.management.internal=ALL-UNNAMED
```
[[build]]
diff --git a/Documentation/replace_macros.py b/Documentation/replace_macros.py
index 6f90697..309a135 100755
--- a/Documentation/replace_macros.py
+++ b/Documentation/replace_macros.py
@@ -89,7 +89,7 @@
</button>
<script type="text/javascript">
var f = function() {
- window.location = '../#/Documentation/' +
+ window.location = '../#/Documentation/q/' +
encodeURIComponent(document.getElementById("docSearch").value);
}
document.getElementById("searchBox").onclick = f;
diff --git a/WORKSPACE b/WORKSPACE
index a5dbbaf..a98c42e 100644
--- a/WORKSPACE
+++ b/WORKSPACE
@@ -1127,6 +1127,12 @@
sha1 = "65bd0cacc9c79a21c6ed8e9f588577cd3c2f85b9",
)
+maven_jar(
+ name = "javax-activation",
+ artifact = "javax.activation:activation:1.1.1",
+ sha1 = "485de3a253e23f645037828c07f1d7f1af40763a",
+)
+
load("//tools/bzl:js.bzl", "bower_archive", "npm_binary")
# NPM binaries bundled along with their dependencies.
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/Dispatcher.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/Dispatcher.java
index c9346f4..4b84864 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/Dispatcher.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/Dispatcher.java
@@ -236,7 +236,7 @@
if (matchPrefix(QUERY, token)) {
query(token);
- } else if (matchPrefix("/Documentation/", token)) {
+ } else if (matchPrefix("/Documentation/q/", token)) {
docSearch(token);
} else if (matchPrefix("/c/", token)) {
diff --git a/java/com/google/gerrit/acceptance/AbstractDaemonTest.java b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
index d168919..0107bae 100644
--- a/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
+++ b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
@@ -38,6 +38,7 @@
import com.google.common.primitives.Chars;
import com.google.gerrit.acceptance.AcceptanceTestRequestScope.Context;
import com.google.gerrit.acceptance.testsuite.account.TestSshKeys;
+import com.google.gerrit.acceptance.testsuite.group.GroupOperations;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.data.AccessSection;
import com.google.gerrit.common.data.ContributorAgreement;
@@ -55,7 +56,6 @@
import com.google.gerrit.extensions.api.changes.RevisionApi;
import com.google.gerrit.extensions.api.changes.SubmittedTogetherInfo;
import com.google.gerrit.extensions.api.groups.GroupApi;
-import com.google.gerrit.extensions.api.groups.GroupInput;
import com.google.gerrit.extensions.api.projects.BranchApi;
import com.google.gerrit.extensions.api.projects.BranchInput;
import com.google.gerrit.extensions.api.projects.ProjectInput;
@@ -264,6 +264,7 @@
@Inject protected ChangeNotes.Factory notesFactory;
@Inject protected BatchAbandon batchAbandon;
@Inject protected TestSshKeys sshKeys;
+ @Inject protected GroupOperations groupOperations;
protected EventRecorder eventRecorder;
protected GerritServer server;
@@ -1193,27 +1194,7 @@
return changeResourceFactory.create(notes.get(0), atrScope.get().getUser());
}
- protected String createGroup(String name) throws Exception {
- return createGroup(name, "Administrators");
- }
-
- protected String createGroupWithRealName(String name) throws Exception {
- GroupInput in = new GroupInput();
- in.name = name;
- in.ownerId = "Administrators";
- gApi.groups().create(in);
- return name;
- }
-
- protected String createGroup(String name, String owner) throws Exception {
- name = name(name);
- GroupInput in = new GroupInput();
- in.name = name;
- in.ownerId = owner;
- gApi.groups().create(in);
- return name;
- }
-
+ @Nullable
protected RevCommit getHead(Repository repo, String name) throws Exception {
try (RevWalk rw = new RevWalk(repo)) {
Ref r = repo.exactRef(name);
@@ -1225,16 +1206,19 @@
return getHead(repo, "HEAD");
}
+ @Nullable
protected RevCommit getRemoteHead(Project.NameKey project, String branch) throws Exception {
try (Repository repo = repoManager.openRepository(project)) {
return getHead(repo, branch.startsWith(Constants.R_REFS) ? branch : "refs/heads/" + branch);
}
}
+ @Nullable
protected RevCommit getRemoteHead(String project, String branch) throws Exception {
return getRemoteHead(new Project.NameKey(project), branch);
}
+ @Nullable
protected RevCommit getRemoteHead() throws Exception {
return getRemoteHead(project, "master");
}
@@ -1248,7 +1232,8 @@
protected ContributorAgreement configureContributorAgreement(boolean autoVerify)
throws Exception {
ContributorAgreement ca;
- String g = createGroup(autoVerify ? "cla-test-group" : "cla-test-no-auto-verify-group");
+ String name = autoVerify ? "cla-test-group" : "cla-test-no-auto-verify-group";
+ String g = groupOperations.newGroup().name(name).create().get();
GroupApi groupApi = gApi.groups().id(g);
groupApi.description("CLA test group");
InternalGroup caGroup = group(new AccountGroup.UUID(groupApi.detail().id));
diff --git a/java/com/google/gerrit/extensions/api/changes/ChangeApi.java b/java/com/google/gerrit/extensions/api/changes/ChangeApi.java
index dfa2128..eb27260 100644
--- a/java/com/google/gerrit/extensions/api/changes/ChangeApi.java
+++ b/java/com/google/gerrit/extensions/api/changes/ChangeApi.java
@@ -176,7 +176,7 @@
/** Publishes a draft change. */
@Deprecated
- default void publish() throws RestApiException {
+ default void publish() {
throw new UnsupportedOperationException("draft workflow is discontinued");
}
diff --git a/java/com/google/gerrit/extensions/api/changes/RevisionApi.java b/java/com/google/gerrit/extensions/api/changes/RevisionApi.java
index 14dc589..a6df45f 100644
--- a/java/com/google/gerrit/extensions/api/changes/RevisionApi.java
+++ b/java/com/google/gerrit/extensions/api/changes/RevisionApi.java
@@ -35,7 +35,7 @@
public interface RevisionApi {
@Deprecated
- default void delete() throws RestApiException {
+ default void delete() {
throw new UnsupportedOperationException("draft workflow is discontinued");
}
@@ -59,7 +59,7 @@
BinaryResult submitPreview(String format) throws RestApiException;
@Deprecated
- default void publish() throws RestApiException {
+ default void publish() {
throw new UnsupportedOperationException("draft workflow is discontinued");
}
diff --git a/java/com/google/gerrit/extensions/api/projects/RefInfo.java b/java/com/google/gerrit/extensions/api/projects/RefInfo.java
index c573600..d5695fd 100644
--- a/java/com/google/gerrit/extensions/api/projects/RefInfo.java
+++ b/java/com/google/gerrit/extensions/api/projects/RefInfo.java
@@ -14,8 +14,15 @@
package com.google.gerrit.extensions.api.projects;
+import com.google.common.base.MoreObjects;
+
public class RefInfo {
public String ref;
public String revision;
public Boolean canDelete;
+
+ @Override
+ public String toString() {
+ return MoreObjects.toStringHelper(this).add("ref", ref).add("revision", revision).toString();
+ }
}
diff --git a/java/com/google/gerrit/httpd/raw/StaticModule.java b/java/com/google/gerrit/httpd/raw/StaticModule.java
index 124ad1c..062d776 100644
--- a/java/com/google/gerrit/httpd/raw/StaticModule.java
+++ b/java/com/google/gerrit/httpd/raw/StaticModule.java
@@ -71,7 +71,15 @@
*/
public static final ImmutableList<String> POLYGERRIT_INDEX_PATHS =
ImmutableList.of(
- "/", "/c/*", "/p/*", "/q/*", "/x/*", "/admin/*", "/dashboard/*", "/settings/*");
+ "/",
+ "/c/*",
+ "/p/*",
+ "/q/*",
+ "/x/*",
+ "/admin/*",
+ "/dashboard/*",
+ "/settings/*",
+ "/Documentation/q/*");
// TODO(dborowitz): These fragments conflict with the REST API
// namespace, so they will need to use a different path.
// "/groups/*",
diff --git a/java/com/google/gerrit/index/project/ProjectSchemaDefinitions.java b/java/com/google/gerrit/index/project/ProjectSchemaDefinitions.java
index 6229041..cefc13c 100644
--- a/java/com/google/gerrit/index/project/ProjectSchemaDefinitions.java
+++ b/java/com/google/gerrit/index/project/ProjectSchemaDefinitions.java
@@ -38,7 +38,9 @@
public static final ProjectSchemaDefinitions INSTANCE = new ProjectSchemaDefinitions();
+ public static final String NAME = "projects";
+
private ProjectSchemaDefinitions() {
- super("projects", ProjectData.class);
+ super(NAME, ProjectData.class);
}
}
diff --git a/java/com/google/gerrit/launcher/GerritLauncher.java b/java/com/google/gerrit/launcher/GerritLauncher.java
index 0d26fe7..5406d52 100644
--- a/java/com/google/gerrit/launcher/GerritLauncher.java
+++ b/java/com/google/gerrit/launcher/GerritLauncher.java
@@ -78,7 +78,7 @@
* @throws Exception if any error occurs.
*/
public static int mainImpl(String[] argv) throws Exception {
- if (argv.length == 0) {
+ if (argv.length == 0 || "-h".equals(argv[0]) || "--help".equals(argv[0])) {
File me;
try {
me = getDistributionArchive();
diff --git a/java/com/google/gerrit/pgm/BUILD b/java/com/google/gerrit/pgm/BUILD
index b34aec0..0bebad4 100644
--- a/java/com/google/gerrit/pgm/BUILD
+++ b/java/com/google/gerrit/pgm/BUILD
@@ -24,6 +24,7 @@
"//java/com/google/gerrit/httpd/auth/oauth",
"//java/com/google/gerrit/httpd/auth/openid",
"//java/com/google/gerrit/index",
+ "//java/com/google/gerrit/index/project",
"//java/com/google/gerrit/launcher",
"//java/com/google/gerrit/lifecycle",
"//java/com/google/gerrit/lucene",
diff --git a/java/com/google/gerrit/pgm/Init.java b/java/com/google/gerrit/pgm/Init.java
index a93e64c..1739de9 100644
--- a/java/com/google/gerrit/pgm/Init.java
+++ b/java/com/google/gerrit/pgm/Init.java
@@ -14,11 +14,15 @@
package com.google.gerrit.pgm;
+import static java.util.stream.Collectors.joining;
+
import com.google.common.base.Joiner;
+import com.google.common.collect.ImmutableList;
import com.google.common.collect.Sets;
import com.google.gerrit.common.IoUtil;
import com.google.gerrit.common.PageLinks;
import com.google.gerrit.common.PluginData;
+import com.google.gerrit.index.project.ProjectSchemaDefinitions;
import com.google.gerrit.pgm.init.BaseInit;
import com.google.gerrit.pgm.init.Browser;
import com.google.gerrit.pgm.init.InitPlugins;
@@ -55,6 +59,9 @@
@Option(name = "--no-auto-start", usage = "Don't automatically start daemon after init")
private boolean noAutoStart;
+ @Option(name = "--no-reindex", usage = "Don't automatically reindex any entities")
+ private boolean noReindex;
+
@Option(name = "--skip-plugins", usage = "Don't install plugins")
private boolean skipPlugins;
@@ -137,6 +144,7 @@
});
modules.add(new GerritServerConfigModule());
Guice.createInjector(modules).injectMembers(this);
+ reindexProjects();
start(run);
}
@@ -194,7 +202,6 @@
if (run.flags.autoStart) {
if (HostPlatform.isWin32()) {
System.err.println("Automatic startup not supported on Win32.");
-
} else {
startDaemon(run);
if (!run.ui.isBatch()) {
@@ -249,6 +256,25 @@
}
}
+ private void reindexProjects() throws Exception {
+ if (noReindex) {
+ return;
+ }
+ // Reindex all projects, so that we bootstrap the project index for new installations
+ List<String> reindexArgs =
+ ImmutableList.of(
+ "--site-path",
+ getSitePath().toString(),
+ "--threads",
+ Integer.toString(1),
+ "--index",
+ ProjectSchemaDefinitions.NAME);
+ getConsoleUI().message("Init complete, reindexing projects with:");
+ getConsoleUI().message(" reindex " + reindexArgs.stream().collect(joining(" ")));
+ Reindex reindexPgm = new Reindex();
+ reindexPgm.main(reindexArgs.stream().toArray(String[]::new));
+ }
+
private static boolean nullOrEmpty(List<?> list) {
return list == null || list.isEmpty();
}
diff --git a/java/com/google/gerrit/server/plugincontext/PluginSetEntryContext.java b/java/com/google/gerrit/server/plugincontext/PluginSetEntryContext.java
index afffbef..2268c07 100644
--- a/java/com/google/gerrit/server/plugincontext/PluginSetEntryContext.java
+++ b/java/com/google/gerrit/server/plugincontext/PluginSetEntryContext.java
@@ -103,8 +103,9 @@
*
* <p>Should only be used in exceptional cases to get direct access to the extension
* implementation. If possible the extension should be invoked through {@link
- * #run(ExtensionImplConsumer)}, {@link #run(ExtensionImplConsumer, Class)}, {@link
- * #call(ExtensionImplFunction)} and {@link #call(CheckedExtensionImplFunction, Class)}.
+ * #run(PluginContext.ExtensionImplConsumer)}, {@link #run(PluginContext.ExtensionImplConsumer,
+ * java.lang.Class)}, {@link #call(PluginContext.ExtensionImplFunction)} and {@link
+ * #call(PluginContext.CheckedExtensionImplFunction, java.lang.Class)}.
*
* @return the implementation of this extension
*/
diff --git a/java/com/google/gerrit/server/project/CreateProjectArgs.java b/java/com/google/gerrit/server/project/CreateProjectArgs.java
index a68bd84..df31c19 100644
--- a/java/com/google/gerrit/server/project/CreateProjectArgs.java
+++ b/java/com/google/gerrit/server/project/CreateProjectArgs.java
@@ -49,6 +49,7 @@
enableSignedPush = InheritableBoolean.INHERIT;
requireSignedPush = InheritableBoolean.INHERIT;
submitType = SubmitType.MERGE_IF_NECESSARY;
+ rejectEmptyCommit = InheritableBoolean.INHERIT;
}
public Project.NameKey getProject() {
diff --git a/java/com/google/gerrit/server/restapi/change/Rebase.java b/java/com/google/gerrit/server/restapi/change/Rebase.java
index 2e2f565..eed0896 100644
--- a/java/com/google/gerrit/server/restapi/change/Rebase.java
+++ b/java/com/google/gerrit/server/restapi/change/Rebase.java
@@ -161,7 +161,8 @@
Base base = rebaseUtil.parseBase(rsrc, str);
if (base == null) {
- throw new ResourceConflictException("base revision is missing: " + str);
+ throw new ResourceConflictException(
+ "base revision is missing from the destination branch: " + str);
}
PatchSet.Id baseId = base.patchSet().getId();
if (change.getId().equals(baseId.getParentKey())) {
diff --git a/java/com/google/gerrit/server/restapi/project/CreateProject.java b/java/com/google/gerrit/server/restapi/project/CreateProject.java
index 7773914..60a24d8 100644
--- a/java/com/google/gerrit/server/restapi/project/CreateProject.java
+++ b/java/com/google/gerrit/server/restapi/project/CreateProject.java
@@ -109,7 +109,7 @@
private final MetaDataUpdate.User metaDataUpdateFactory;
private final GitReferenceUpdated referenceUpdated;
private final RepositoryConfig repositoryCfg;
- private final PersonIdent serverIdent;
+ private final Provider<PersonIdent> serverIdent;
private final Provider<IdentifiedUser> identifiedUser;
private final Provider<PutConfig> putConfig;
private final AllProjectsName allProjects;
@@ -131,7 +131,7 @@
MetaDataUpdate.User metaDataUpdateFactory,
GitReferenceUpdated referenceUpdated,
RepositoryConfig repositoryCfg,
- @GerritPersonIdent PersonIdent serverIdent,
+ @GerritPersonIdent Provider<PersonIdent> serverIdent,
Provider<IdentifiedUser> identifiedUser,
Provider<PutConfig> putConfig,
AllProjectsName allProjects,
@@ -357,7 +357,7 @@
CommitBuilder cb = new CommitBuilder();
cb.setTreeId(oi.insert(Constants.OBJ_TREE, new byte[] {}));
cb.setAuthor(metaDataUpdateFactory.getUserPersonIdent());
- cb.setCommitter(serverIdent);
+ cb.setCommitter(serverIdent.get());
cb.setMessage("Initial empty repository\n");
ObjectId id = oi.insert(cb);
diff --git a/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java b/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java
index 5e46a03..9886db5 100644
--- a/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java
@@ -56,6 +56,7 @@
import com.google.gerrit.acceptance.UseSsh;
import com.google.gerrit.acceptance.testsuite.account.AccountOperations;
import com.google.gerrit.acceptance.testsuite.account.TestSshKeys;
+import com.google.gerrit.acceptance.testsuite.group.GroupOperations;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.data.AccessSection;
import com.google.gerrit.common.data.GlobalCapability;
@@ -97,6 +98,7 @@
import com.google.gerrit.gpg.testing.TestKey;
import com.google.gerrit.mail.Address;
import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.client.Branch;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Project;
@@ -233,6 +235,8 @@
@Inject private AccountManager accountManager;
+ @Inject protected GroupOperations groupOperations;
+
private AccountIndexedCounter accountIndexedCounter;
private RegistrationHandle accountIndexEventCounterHandle;
private RefUpdateCounter refUpdateCounter;
@@ -2208,7 +2212,9 @@
public void allGroupsForAUserAccountCanBeRetrieved() throws Exception {
String username = name("user1");
accountOperations.newAccount().username(username).create();
- String group = createGroup("group");
+ AccountGroup.UUID groupID = groupOperations.newGroup().name("group").create();
+ String group = groupOperations.group(groupID).get().name();
+
gApi.groups().id(group).addMembers(username);
List<GroupInfo> allGroups = gApi.accounts().id(username).getGroups();
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
index 30aef73..744d1e9 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -68,6 +68,7 @@
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.TestProjectInput;
import com.google.gerrit.acceptance.testsuite.account.AccountOperations;
+import com.google.gerrit.acceptance.testsuite.group.GroupOperations;
import com.google.gerrit.common.FooterConstants;
import com.google.gerrit.common.data.LabelFunction;
import com.google.gerrit.common.data.LabelType;
@@ -194,6 +195,8 @@
@Inject private ChangeIndexCollection changeIndexCollection;
@Inject private IndexConfig indexConfig;
+ @Inject protected GroupOperations groupOperations;
+
private ChangeIndexedCounter changeIndexedCounter;
private RegistrationHandle changeIndexedCounterHandle;
@@ -1823,7 +1826,7 @@
.preferredEmail(email)
.fullname(fullname)
.create();
- String testGroup = createGroupWithRealName("ab");
+ String testGroup = groupOperations.newGroup().name("ab").create().get();
GroupApi groupApi = gApi.groups().id(testGroup);
groupApi.description("test group");
groupApi.addMembers(user.fullName);
@@ -1884,7 +1887,7 @@
.fullname(myGroupUserFullname)
.create();
- String testGroup = createGroupWithRealName("kobe");
+ String testGroup = groupOperations.newGroup().name("kobe").create().get();
GroupApi groupApi = gApi.groups().id(testGroup);
groupApi.description("test group");
groupApi.addMembers(myGroupUserFullname);
diff --git a/javatests/com/google/gerrit/acceptance/api/group/GroupsConsistencyIT.java b/javatests/com/google/gerrit/acceptance/api/group/GroupsConsistencyIT.java
index 87a566e..491cb3a 100644
--- a/javatests/com/google/gerrit/acceptance/api/group/GroupsConsistencyIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/group/GroupsConsistencyIT.java
@@ -20,6 +20,7 @@
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.NoHttpd;
import com.google.gerrit.acceptance.Sandboxed;
+import com.google.gerrit.acceptance.testsuite.group.GroupOperations;
import com.google.gerrit.common.data.GlobalCapability;
import com.google.gerrit.extensions.api.config.ConsistencyCheckInfo;
import com.google.gerrit.extensions.api.config.ConsistencyCheckInfo.ConsistencyProblemInfo;
@@ -30,6 +31,7 @@
import com.google.gerrit.server.group.db.GroupConfig;
import com.google.gerrit.server.group.db.GroupNameNotes;
import com.google.gerrit.server.group.db.testing.GroupTestUtil;
+import com.google.inject.Inject;
import java.util.List;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.RefRename;
@@ -47,6 +49,8 @@
@Sandboxed
@NoHttpd
public class GroupsConsistencyIT extends AbstractDaemonTest {
+
+ @Inject protected GroupOperations groupOperations;
private GroupInfo gAdmin;
private GroupInfo g1;
private GroupInfo g2;
@@ -57,8 +61,8 @@
public void basicSetup() throws Exception {
allowGlobalCapabilities(REGISTERED_USERS, GlobalCapability.ACCESS_DATABASE);
- String name1 = createGroup("g1");
- String name2 = createGroup("g2");
+ String name1 = groupOperations.newGroup().name("g1").create().get();
+ String name2 = groupOperations.newGroup().name("g2").create().get();
gApi.groups().id(name1).addMembers(user.fullName);
gApi.groups().id(name2).addMembers(admin.fullName);
@@ -218,7 +222,7 @@
@Test
public void cyclicSubgroup() throws Exception {
updateGroupFile(RefNames.refsGroups(new AccountGroup.UUID(g1.id)), "subgroups", g1.id + "\n");
- assertWarning("cyclic");
+ assertWarning("cycle");
}
private void assertError(String msg) throws Exception {
diff --git a/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java b/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java
index ade0f3c..4a4ee2a 100644
--- a/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java
@@ -144,6 +144,23 @@
}
}
+ // Creates a group, but with uniquified name.
+ protected String createGroup(String name) throws Exception {
+ // TODO(hanwen): rewrite this test in terms of UUID. This requires redoing the assertion helpers
+ // too.
+ AccountGroup.UUID g = groupOperations.newGroup().ownerGroupUuid(adminGroupUuid()).create();
+ return groupRef(g).getName();
+ }
+
+ protected String createGroup(String name, String owner) throws Exception {
+ name = name(name);
+ GroupInput in = new GroupInput();
+ in.name = name;
+ in.ownerId = owner;
+ gApi.groups().create(in);
+ return name;
+ }
+
@Override
protected ProjectResetter.Config resetProjects() {
// Don't reset All-Users since deleting users makes groups inconsistent (e.g. groups would
diff --git a/javatests/com/google/gerrit/acceptance/pgm/BUILD b/javatests/com/google/gerrit/acceptance/pgm/BUILD
index a91b815..e0d2f86 100644
--- a/javatests/com/google/gerrit/acceptance/pgm/BUILD
+++ b/javatests/com/google/gerrit/acceptance/pgm/BUILD
@@ -13,6 +13,8 @@
vm_args = ["-Xmx512m"],
deps = [
":util",
+ "//java/com/google/gerrit/index",
+ "//java/com/google/gerrit/index/project",
"//java/com/google/gerrit/server/schema",
],
)
diff --git a/javatests/com/google/gerrit/acceptance/pgm/InitIT.java b/javatests/com/google/gerrit/acceptance/pgm/InitIT.java
new file mode 100644
index 0000000..a573e35
--- /dev/null
+++ b/javatests/com/google/gerrit/acceptance/pgm/InitIT.java
@@ -0,0 +1,51 @@
+// Copyright (C) 2018 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.acceptance.pgm;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.common.collect.ImmutableSet;
+import com.google.gerrit.acceptance.NoHttpd;
+import com.google.gerrit.acceptance.StandaloneSiteTest;
+import com.google.gerrit.index.IndexConfig;
+import com.google.gerrit.index.QueryOptions;
+import com.google.gerrit.index.project.ProjectData;
+import com.google.gerrit.index.project.ProjectIndexCollection;
+import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.server.config.AllProjectsName;
+import com.google.gerrit.server.config.AllUsersName;
+import java.util.Optional;
+import org.junit.Test;
+
+@NoHttpd
+public class InitIT extends StandaloneSiteTest {
+
+ @Test
+ public void indexesAllProjectsAndAllUsers() throws Exception {
+ runGerrit("init", "-d", sitePaths.site_path.toString(), "--show-stack-trace");
+ try (ServerContext ctx = startServer()) {
+ ProjectIndexCollection projectIndex =
+ ctx.getInjector().getInstance(ProjectIndexCollection.class);
+ Project.NameKey allProjects = ctx.getInjector().getInstance(AllProjectsName.class);
+ Project.NameKey allUsers = ctx.getInjector().getInstance(AllUsersName.class);
+ QueryOptions opts =
+ QueryOptions.create(IndexConfig.createDefault(), 0, 1, ImmutableSet.of("name"));
+ Optional<ProjectData> allProjectsData = projectIndex.getSearchIndex().get(allProjects, opts);
+ assertThat(allProjectsData.isPresent()).isTrue();
+ Optional<ProjectData> allUsersData = projectIndex.getSearchIndex().get(allUsers, opts);
+ assertThat(allUsersData.isPresent()).isTrue();
+ }
+ }
+}
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/ChangeReviewersIT.java b/javatests/com/google/gerrit/acceptance/rest/change/ChangeReviewersIT.java
index d1f4d84..6a9a27c 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/ChangeReviewersIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/ChangeReviewersIT.java
@@ -31,6 +31,7 @@
import com.google.gerrit.acceptance.RestResponse;
import com.google.gerrit.acceptance.Sandboxed;
import com.google.gerrit.acceptance.TestAccount;
+import com.google.gerrit.acceptance.testsuite.group.GroupOperations;
import com.google.gerrit.common.data.Permission;
import com.google.gerrit.extensions.api.changes.AddReviewerInput;
import com.google.gerrit.extensions.api.changes.AddReviewerResult;
@@ -52,6 +53,7 @@
import com.google.gerrit.server.change.ReviewerAdder;
import com.google.gerrit.testing.FakeEmailSender.Message;
import com.google.gson.stream.JsonReader;
+import com.google.inject.Inject;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
@@ -62,12 +64,15 @@
import org.junit.Test;
public class ChangeReviewersIT extends AbstractDaemonTest {
+
+ @Inject protected GroupOperations groupOperations;
+
@Test
public void addGroupAsReviewer() throws Exception {
// Set up two groups, one that is too large too add as reviewer, and one
// that is too large to add without confirmation.
- String largeGroup = createGroup("largeGroup");
- String mediumGroup = createGroup("mediumGroup");
+ String largeGroup = groupOperations.newGroup().name("largeGroup").create().get();
+ String mediumGroup = groupOperations.newGroup().name("mediumGroup").create().get();
int largeGroupSize = ReviewerAdder.DEFAULT_MAX_REVIEWERS + 1;
int mediumGroupSize = ReviewerAdder.DEFAULT_MAX_REVIEWERS_WITHOUT_CHECK + 1;
@@ -170,7 +175,7 @@
PushOneCommit.Result r = createChange();
String changeId = r.getChangeId();
AddReviewerInput in = new AddReviewerInput();
- in.reviewer = createGroup("cc1");
+ in.reviewer = groupOperations.newGroup().name("cc1").create().get();
in.state = CC;
gApi.groups()
.id(in.reviewer)
@@ -209,7 +214,7 @@
result = addReviewer(changeId, reviewer.username);
assertThat(result.error).isNull();
sender.clear();
- in.reviewer = createGroup("cc2");
+ in.reviewer = groupOperations.newGroup().name("cc2").create().get();
gApi.groups().id(in.reviewer).addMembers(usernames.toArray(new String[usernames.size()]));
gApi.groups().id(in.reviewer).addMembers(reviewer.username);
result = addReviewer(changeId, in);
@@ -479,8 +484,8 @@
usernames.add(u.username);
}
- String largeGroup = createGroup("largeGroup");
- String mediumGroup = createGroup("mediumGroup");
+ String largeGroup = groupOperations.newGroup().name("largeGroup").create().get();
+ String mediumGroup = groupOperations.newGroup().name("mediumGroup").create().get();
gApi.groups().id(largeGroup).addMembers(usernames.toArray(new String[largeGroupSize]));
gApi.groups()
.id(mediumGroup)
@@ -622,8 +627,8 @@
accountCreator.create(name("user2"), emailPrefix + "user2@example.com", "User2");
TestAccount user3 =
accountCreator.create(name("user3"), emailPrefix + "user3@example.com", "User3");
- String group1 = createGroup("group1");
- String group2 = createGroup("group2");
+ 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);
gApi.groups().id(group2).addMembers(user2.username, user3.username);
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/IndexChangeIT.java b/javatests/com/google/gerrit/acceptance/rest/change/IndexChangeIT.java
index 6555fe8..bb114e7 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/IndexChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/IndexChangeIT.java
@@ -20,17 +20,22 @@
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.TestAccount;
+import com.google.gerrit.acceptance.testsuite.group.GroupOperations;
import com.google.gerrit.common.data.Permission;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.project.testing.Util;
+import com.google.inject.Inject;
import java.util.List;
import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
import org.eclipse.jgit.junit.TestRepository;
import org.junit.Test;
public class IndexChangeIT extends AbstractDaemonTest {
+
+ @Inject protected GroupOperations groupOperations;
+
@Test
public void indexChange() throws Exception {
String changeId = createChange().getChangeId();
@@ -48,7 +53,8 @@
public void indexChangeAfterOwnerLosesVisibility() throws Exception {
// Create a test group with 2 users as members
TestAccount user2 = accountCreator.user2();
- String group = createGroup("test");
+ AccountGroup.UUID groupId = groupOperations.newGroup().name("test").create();
+ String group = groupOperations.group(groupId).get().name();
gApi.groups().id(group).addMembers("admin", "user", user2.username);
// Create a project and restrict its visibility to the group
diff --git a/lib/greenmail/BUILD b/lib/greenmail/BUILD
index 55eb9f3..b09f27b 100644
--- a/lib/greenmail/BUILD
+++ b/lib/greenmail/BUILD
@@ -1,8 +1,23 @@
package(default_visibility = ["//visibility:public"])
+POST_JDK8_DEPS = [":javax-activation"]
+
+java_library(
+ name = "javax-activation",
+ testonly = 1,
+ data = ["//lib:LICENSE-DO_NOT_DISTRIBUTE"],
+ exports = ["@javax-activation//jar"],
+)
+
java_library(
name = "greenmail",
+ testonly = 1,
data = ["//lib:LICENSE-Apache2.0"],
visibility = ["//visibility:public"],
exports = ["@greenmail//jar"],
+ runtime_deps = select({
+ "//:java9": POST_JDK8_DEPS,
+ "//:java_next": POST_JDK8_DEPS,
+ "//conditions:default": [],
+ }),
)
diff --git a/polygerrit-ui/app/behaviors/keyboard-shortcut-behavior/keyboard-shortcut-behavior.html b/polygerrit-ui/app/behaviors/keyboard-shortcut-behavior/keyboard-shortcut-behavior.html
index 0a685da..af982cf 100644
--- a/polygerrit-ui/app/behaviors/keyboard-shortcut-behavior/keyboard-shortcut-behavior.html
+++ b/polygerrit-ui/app/behaviors/keyboard-shortcut-behavior/keyboard-shortcut-behavior.html
@@ -142,6 +142,7 @@
UP_TO_CHANGE: 'UP_TO_CHANGE',
TOGGLE_DIFF_MODE: 'TOGGLE_DIFF_MODE',
REFRESH_CHANGE: 'REFRESH_CHANGE',
+ EDIT_TOPIC: 'EDIT_TOPIC',
NEXT_LINE: 'NEXT_LINE',
PREV_LINE: 'PREV_LINE',
@@ -164,6 +165,7 @@
PREV_FILE: 'PREV_FILE',
NEXT_FILE_WITH_COMMENTS: 'NEXT_FILE_WITH_COMMENTS',
PREV_FILE_WITH_COMMENTS: 'PREV_FILE_WITH_COMMENTS',
+ NEXT_UNREVIEWED_FILE: 'NEXT_UNREVIEWED_FILE',
CURSOR_NEXT_FILE: 'CURSOR_NEXT_FILE',
CURSOR_PREV_FILE: 'CURSOR_PREV_FILE',
OPEN_FILE: 'OPEN_FILE',
@@ -223,6 +225,8 @@
'Refresh list of changes');
_describe(Shortcut.TOGGLE_CHANGE_STAR, ShortcutSection.ACTIONS,
'Star/unstar change');
+ _describe(Shortcut.EDIT_TOPIC, ShortcutSection.ACTIONS,
+ 'Add a change topic');
_describe(Shortcut.NEXT_LINE, ShortcutSection.DIFFS, 'Go to next line');
_describe(Shortcut.PREV_LINE, ShortcutSection.DIFFS, 'Go to previous line');
@@ -252,6 +256,8 @@
'Mark/unmark file as reviewed');
_describe(Shortcut.TOGGLE_DIFF_MODE, ShortcutSection.DIFFS,
'Toggle unified/side-by-side diff');
+ _describe(Shortcut.NEXT_UNREVIEWED_FILE, ShortcutSection.DIFFS,
+ 'Mark file as reviewed and go to next unreviewed file');
_describe(Shortcut.NEXT_FILE, ShortcutSection.NAVIGATION, 'Select next file');
_describe(Shortcut.PREV_FILE, ShortcutSection.NAVIGATION,
diff --git a/polygerrit-ui/app/elements/admin/gr-create-change-dialog/gr-create-change-dialog.html b/polygerrit-ui/app/elements/admin/gr-create-change-dialog/gr-create-change-dialog.html
index ad12a44..987b63d 100644
--- a/polygerrit-ui/app/elements/admin/gr-create-change-dialog/gr-create-change-dialog.html
+++ b/polygerrit-ui/app/elements/admin/gr-create-change-dialog/gr-create-change-dialog.html
@@ -55,7 +55,7 @@
padding: 0 .15em;
}
}
- .hideBranch {
+ .hide {
display: none;
}
</style>
@@ -108,7 +108,7 @@
</iron-autogrow-textarea>
</span>
</section>
- <section>
+ <section class$="[[_computePrivateSectionClass(_privateChangesEnabled)]]">
<label
class="title"
for="privateChangeCheckBox">Private change</label>
diff --git a/polygerrit-ui/app/elements/admin/gr-create-change-dialog/gr-create-change-dialog.js b/polygerrit-ui/app/elements/admin/gr-create-change-dialog/gr-create-change-dialog.js
index 826a6dc..8e15755 100644
--- a/polygerrit-ui/app/elements/admin/gr-create-change-dialog/gr-create-change-dialog.js
+++ b/polygerrit-ui/app/elements/admin/gr-create-change-dialog/gr-create-change-dialog.js
@@ -44,6 +44,7 @@
notify: true,
value: false,
},
+ _privateChangesEnabled: Boolean,
},
behaviors: [
@@ -52,10 +53,23 @@
],
attached() {
- if (!this.repoName) { return; }
- this.$.restAPI.getProjectConfig(this.repoName).then(config => {
- this.privateByDefault = config.private_by_default;
- });
+ if (!this.repoName) { return Promise.resolve(); }
+
+ const promises = [];
+
+ promises.push(this.$.restAPI.getProjectConfig(this.repoName)
+ .then(config => {
+ this.privateByDefault = config.private_by_default;
+ }));
+
+ promises.push(this.$.restAPI.getConfig().then(config => {
+ if (!config) { return; }
+
+ this._privateConfig = config && config.change &&
+ config.change.disable_private_changes;
+ }));
+
+ return Promise.all(promises);
},
observers: [
@@ -63,7 +77,7 @@
],
_computeBranchClass(baseChange) {
- return baseChange ? 'hideBranch' : '';
+ return baseChange ? 'hide' : '';
},
_allowCreate(branch, subject) {
@@ -120,5 +134,9 @@
return false;
}
},
+
+ _computePrivateSectionClass(config) {
+ return config ? 'hide' : '';
+ },
});
})();
diff --git a/polygerrit-ui/app/elements/admin/gr-create-change-dialog/gr-create-change-dialog_test.html b/polygerrit-ui/app/elements/admin/gr-create-change-dialog/gr-create-change-dialog_test.html
index 08c569c..aa4da68 100644
--- a/polygerrit-ui/app/elements/admin/gr-create-change-dialog/gr-create-change-dialog_test.html
+++ b/polygerrit-ui/app/elements/admin/gr-create-change-dialog/gr-create-change-dialog_test.html
@@ -158,5 +158,15 @@
done();
});
});
+
+ test('_computeBranchClass', () => {
+ assert.equal(element._computeBranchClass(true), 'hide');
+ assert.equal(element._computeBranchClass(false), '');
+ });
+
+ test('_computePrivateSectionClass', () => {
+ assert.equal(element._computePrivateSectionClass(true), 'hide');
+ assert.equal(element._computePrivateSectionClass(false), '');
+ });
});
</script>
diff --git a/polygerrit-ui/app/elements/admin/gr-create-repo-dialog/gr-create-repo-dialog.html b/polygerrit-ui/app/elements/admin/gr-create-repo-dialog/gr-create-repo-dialog.html
index d55d7e9..f43a3e2 100644
--- a/polygerrit-ui/app/elements/admin/gr-create-repo-dialog/gr-create-repo-dialog.html
+++ b/polygerrit-ui/app/elements/admin/gr-create-repo-dialog/gr-create-repo-dialog.html
@@ -71,6 +71,17 @@
</span>
</section>
<section>
+ <span class="title">Owner</span>
+ <span class="value">
+ <gr-autocomplete
+ id="ownerInput"
+ text="{{_repoOwner}}"
+ value="{{_repoOwnerId}}"
+ query="[[_queryGroups]]">
+ </gr-autocomplete>
+ </span>
+ </section>
+ <section>
<span class="title">Create initial empty commit</span>
<span class="value">
<gr-select
diff --git a/polygerrit-ui/app/elements/admin/gr-create-repo-dialog/gr-create-repo-dialog.js b/polygerrit-ui/app/elements/admin/gr-create-repo-dialog/gr-create-repo-dialog.js
index 9dde290..bb2b5f2 100644
--- a/polygerrit-ui/app/elements/admin/gr-create-repo-dialog/gr-create-repo-dialog.js
+++ b/polygerrit-ui/app/elements/admin/gr-create-repo-dialog/gr-create-repo-dialog.js
@@ -43,6 +43,11 @@
type: Boolean,
value: false,
},
+ _repoOwner: String,
+ _repoOwnerId: {
+ type: String,
+ observer: '_repoOwnerIdUpdate',
+ },
_query: {
type: Function,
@@ -50,6 +55,12 @@
return this._getRepoSuggestions.bind(this);
},
},
+ _queryGroups: {
+ type: Function,
+ value() {
+ return this._getGroupSuggestions.bind(this);
+ },
+ },
},
observers: [
@@ -70,6 +81,14 @@
this.hasNewRepoName = !!name;
},
+ _repoOwnerIdUpdate(id) {
+ if (id) {
+ this.set('_repoConfig.owners', [id]);
+ } else {
+ this.set('_repoConfig.owners', undefined);
+ }
+ },
+
handleCreateRepo() {
return this.$.restAPI.createRepo(this._repoConfig)
.then(repoRegistered => {
@@ -94,5 +113,20 @@
return repos;
});
},
+
+ _getGroupSuggestions(input) {
+ return this.$.restAPI.getSuggestedGroups(input)
+ .then(response => {
+ const groups = [];
+ for (const key in response) {
+ if (!response.hasOwnProperty(key)) { continue; }
+ groups.push({
+ name: key,
+ value: decodeURIComponent(response[key].id),
+ });
+ }
+ return groups;
+ });
+ },
});
})();
diff --git a/polygerrit-ui/app/elements/admin/gr-create-repo-dialog/gr-create-repo-dialog_test.html b/polygerrit-ui/app/elements/admin/gr-create-repo-dialog/gr-create-repo-dialog_test.html
index e70c11a..6bc3522 100644
--- a/polygerrit-ui/app/elements/admin/gr-create-repo-dialog/gr-create-repo-dialog_test.html
+++ b/polygerrit-ui/app/elements/admin/gr-create-repo-dialog/gr-create-repo-dialog_test.html
@@ -60,6 +60,7 @@
create_empty_commit: true,
parent: 'All-Project',
permissions_only: false,
+ owners: ['testId'],
};
const saveStub = sandbox.stub(element.$.restAPI,
@@ -76,8 +77,12 @@
permissions_only: false,
};
+ element._repoOwner = 'test';
+ element._repoOwnerId = 'testId';
+
element.$.repoNameInput.bindValue = configInputObj.name;
element.$.rightsInheritFromInput.bindValue = configInputObj.parent;
+ element.$.ownerInput.text = configInputObj.owners[0];
element.$.initalCommit.bindValue =
configInputObj.create_empty_commit;
element.$.parentRepo.bindValue =
@@ -92,5 +97,10 @@
done();
});
});
+
+ test('testing observer of _repoOwner', () => {
+ element._repoOwnerId = 'test-5';
+ assert.deepEqual(element._repoConfig.owners, ['test-5']);
+ });
});
</script>
diff --git a/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access.js b/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access.js
index 799b831..fe043d5 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access.js
+++ b/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access.js
@@ -133,6 +133,8 @@
* @return {!Promise}
*/
_repoChanged(repo) {
+ this._loading = true;
+
if (!repo) { return Promise.resolve(); }
return this._reload(repo);
diff --git a/polygerrit-ui/app/elements/admin/gr-repo-dashboards/gr-repo-dashboards.html b/polygerrit-ui/app/elements/admin/gr-repo-dashboards/gr-repo-dashboards.html
index 36a7d76..1d49db9 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo-dashboards/gr-repo-dashboards.html
+++ b/polygerrit-ui/app/elements/admin/gr-repo-dashboards/gr-repo-dashboards.html
@@ -53,7 +53,7 @@
</tr>
<template is="dom-repeat" items="[[item.dashboards]]">
<tr class="table">
- <td class="name"><a href$="[[_getUrl(item.project, item.sections)]]">[[item.path]]</a></td>
+ <td class="name"><a href$="[[_getUrl(item.project, item.id)]]">[[item.path]]</a></td>
<td class="title">[[item.title]]</td>
<td class="desc">[[item.description]]</td>
<td class="inherited">[[_computeInheritedFrom(item.project, item.defining_project)]]</td>
diff --git a/polygerrit-ui/app/elements/admin/gr-repo-dashboards/gr-repo-dashboards.js b/polygerrit-ui/app/elements/admin/gr-repo-dashboards/gr-repo-dashboards.js
index c0fc0cb..7dea7f4 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo-dashboards/gr-repo-dashboards.js
+++ b/polygerrit-ui/app/elements/admin/gr-repo-dashboards/gr-repo-dashboards.js
@@ -43,24 +43,24 @@
this.$.restAPI.getRepoDashboards(this.repo, errFn).then(res => {
if (!res) { return Promise.resolve(); }
- // Flatten 2 dimenional array, and sort by id.
+ // Group by ref and sort by id.
const dashboards = res.concat.apply([], res).sort((a, b) =>
- a.id > b.id);
- const customList = dashboards.filter(a => a.ref === 'custom');
- const defaultList = dashboards.filter(a => a.ref === 'default');
+ a.id < b.id ? -1 : 1);
+ const dashboardsByRef = {};
+ dashboards.forEach(d => {
+ if (!dashboardsByRef[d.ref]) {
+ dashboardsByRef[d.ref] = [];
+ }
+ dashboardsByRef[d.ref].push(d);
+ });
+
const dashboardBuilder = [];
- if (customList.length) {
+ Object.keys(dashboardsByRef).sort().forEach(ref => {
dashboardBuilder.push({
- section: 'Custom',
- dashboards: customList,
+ section: ref,
+ dashboards: dashboardsByRef[ref],
});
- }
- if (defaultList.length) {
- dashboardBuilder.push({
- section: 'Default',
- dashboards: defaultList,
- });
- }
+ });
this._dashboards = dashboardBuilder;
this._loading = false;
@@ -68,10 +68,10 @@
});
},
- _getUrl(project, sections) {
- if (!project || !sections) { return ''; }
+ _getUrl(project, id) {
+ if (!project || !id) { return ''; }
- return Gerrit.Nav.getUrlForCustomDashboard(project, sections);
+ return Gerrit.Nav.getUrlForRepoDashboard(project, id);
},
_computeLoadingClass(loading) {
diff --git a/polygerrit-ui/app/elements/admin/gr-repo-dashboards/gr-repo-dashboards_test.html b/polygerrit-ui/app/elements/admin/gr-repo-dashboards/gr-repo-dashboards_test.html
index 4d86a0c..94bf5e0 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo-dashboards/gr-repo-dashboards_test.html
+++ b/polygerrit-ui/app/elements/admin/gr-repo-dashboards/gr-repo-dashboards_test.html
@@ -46,54 +46,61 @@
sandbox.restore();
});
- suite('with default only', () => {
+ suite('dashboard table', () => {
setup(() => {
sandbox.stub(element.$.restAPI, 'getRepoDashboards').returns(
Promise.resolve([
- [
- {
- id: 'default:contributor',
- project: 'gerrit',
- defining_project: 'gerrit',
- ref: 'default',
- path: 'contributor',
- description: 'Own contributions.',
- foreach: 'owner:self',
- url: '/dashboard/?params',
- title: 'Contributor Dashboard',
- sections: [
- {
- name: 'Mine To Rebase',
- query: 'is:open -is:mergeable',
- },
- {
- name: 'My Recently Merged',
- query: 'is:merged limit:10',
- },
- ],
- },
- ],
- [
- {
- id: 'default:open',
- project: 'gerrit',
- defining_project: 'Public-Projects',
- ref: 'default',
- path: 'open',
- description: 'Recent open changes.',
- url: '/dashboard/?params',
- title: 'Open Changes',
- sections: [
- {
- name: 'Open Changes',
- query: 'status:open project:${project} -age:7w',
- },
- ],
- },
- ],
+ {
+ id: 'default:contributor',
+ project: 'gerrit',
+ defining_project: 'gerrit',
+ ref: 'default',
+ path: 'contributor',
+ description: 'Own contributions.',
+ foreach: 'owner:self',
+ url: '/dashboard/?params',
+ title: 'Contributor Dashboard',
+ sections: [
+ {
+ name: 'Mine To Rebase',
+ query: 'is:open -is:mergeable',
+ },
+ {
+ name: 'My Recently Merged',
+ query: 'is:merged limit:10',
+ },
+ ],
+ },
+ {
+ id: 'custom:custom2',
+ project: 'gerrit',
+ defining_project: 'Public-Projects',
+ ref: 'custom',
+ path: 'open',
+ description: 'Recent open changes.',
+ url: '/dashboard/?params',
+ title: 'Open Changes',
+ sections: [
+ {
+ name: 'Open Changes',
+ query: 'status:open project:${project} -age:7w',
+ },
+ ],
+ },
+ {
+ id: 'default:abc',
+ project: 'gerrit',
+ ref: 'default',
+ },
+ {
+ id: 'custom:custom1',
+ project: 'gerrit',
+ ref: 'custom',
+ },
]));
});
- test('loading', done => {
+
+ test('loading, sections, and ordering', done => {
assert.isTrue(element._loading);
assert.notEqual(getComputedStyle(element.$.loadingContainer).display,
'none');
@@ -101,143 +108,20 @@
'none');
element.repo = 'test';
flush(() => {
- assert.equal(element._dashboards.length, 1);
- assert.equal(element._dashboards[0].section, 'Default');
- assert.equal(element._dashboards[0].dashboards.length, 2);
assert.equal(getComputedStyle(element.$.loadingContainer).display,
'none');
assert.notEqual(getComputedStyle(element.$.dashboards).display,
'none');
- done();
- });
- });
- test('dispatched command-tap on button tap', done => {
- element.repo = 'test';
- flush(() => {
- assert.equal(element._dashboards.length, 1);
- assert.equal(element._dashboards[0].section, 'Default');
- assert.equal(element._dashboards[0].dashboards.length, 2);
- done();
- });
- });
- });
-
- suite('with custom only', () => {
- setup(() => {
- sandbox.stub(element.$.restAPI, 'getRepoDashboards').returns(
- Promise.resolve([
- [
- {
- id: 'custom:custom1',
- project: 'gerrit',
- defining_project: 'gerrit',
- ref: 'custom',
- path: 'contributor',
- description: 'Own contributions.',
- foreach: 'owner:self',
- url: '/dashboard/?params',
- title: 'Contributor Dashboard',
- sections: [
- {
- name: 'Mine To Rebase',
- query: 'is:open -is:mergeable',
- },
- {
- name: 'My Recently Merged',
- query: 'is:merged limit:10',
- },
- ],
- },
- ],
- [
- {
- id: 'custom:custom2',
- project: 'gerrit',
- defining_project: 'Public-Projects',
- ref: 'custom',
- path: 'open',
- description: 'Recent open changes.',
- url: '/dashboard/?params',
- title: 'Open Changes',
- sections: [
- {
- name: 'Open Changes',
- query: 'status:open project:${project} -age:7w',
- },
- ],
- },
- ],
- ]));
- });
-
- test('dispatched command-tap on button tap', done => {
- element.repo = 'test';
- flush(() => {
- assert.equal(element._dashboards.length, 1);
- assert.equal(element._dashboards[0].section, 'Custom');
- assert.equal(element._dashboards[0].dashboards.length, 2);
- done();
- });
- });
- });
-
- suite('with custom and default', () => {
- setup(() => {
- sandbox.stub(element.$.restAPI, 'getRepoDashboards').returns(
- Promise.resolve([
- [
- {
- id: 'default:contributor',
- project: 'gerrit',
- defining_project: 'gerrit',
- ref: 'default',
- path: 'contributor',
- description: 'Own contributions.',
- foreach: 'owner:self',
- url: '/dashboard/?params',
- title: 'Contributor Dashboard',
- sections: [
- {
- name: 'Mine To Rebase',
- query: 'is:open -is:mergeable',
- },
- {
- name: 'My Recently Merged',
- query: 'is:merged limit:10',
- },
- ],
- },
- ],
- [
- {
- id: 'custom:custom2',
- project: 'gerrit',
- defining_project: 'Public-Projects',
- ref: 'custom',
- path: 'open',
- description: 'Recent open changes.',
- url: '/dashboard/?params',
- title: 'Open Changes',
- sections: [
- {
- name: 'Open Changes',
- query: 'status:open project:${project} -age:7w',
- },
- ],
- },
- ],
- ]));
- });
-
- test('dispatched command-tap on button tap', done => {
- element.repo = 'test';
- flush(() => {
assert.equal(element._dashboards.length, 2);
- assert.equal(element._dashboards[0].section, 'Custom');
- assert.equal(element._dashboards[1].section, 'Default');
- assert.equal(element._dashboards[0].dashboards.length, 1);
- assert.equal(element._dashboards[1].dashboards.length, 1);
+ assert.equal(element._dashboards[0].section, 'custom');
+ assert.equal(element._dashboards[1].section, 'default');
+
+ const dashboards = element._dashboards[0].dashboards;
+ assert.equal(dashboards.length, 2);
+ assert.equal(dashboards[0].id, 'custom:custom1');
+ assert.equal(dashboards[1].id, 'custom:custom2');
+
done();
});
});
@@ -245,7 +129,7 @@
suite('test url', () => {
test('_getUrl', () => {
- sandbox.stub(Gerrit.Nav, 'getUrlForCustomDashboard',
+ sandbox.stub(Gerrit.Nav, 'getUrlForRepoDashboard',
() => '/r/dashboard/test');
assert.equal(element._getUrl('/dashboard/test', {}), '/r/dashboard/test');
diff --git a/polygerrit-ui/app/elements/admin/gr-repo-list/gr-repo-list.html b/polygerrit-ui/app/elements/admin/gr-repo-list/gr-repo-list.html
index 7e1c385..0490db2 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo-list/gr-repo-list.html
+++ b/polygerrit-ui/app/elements/admin/gr-repo-list/gr-repo-list.html
@@ -43,6 +43,7 @@
<tr class="headerRow">
<th class="name topHeader">Repository Name</th>
<th class="description topHeader">Repository Description</th>
+ <th class="changesLink topHeader">Changes</th>
<th class="repositoryBrowser topHeader">Repository Browser</th>
<th class="readOnly topHeader">Read only</th>
</tr>
@@ -56,6 +57,7 @@
<a href$="[[_computeRepoUrl(item.name)]]">[[item.name]]</a>
</td>
<td class="description">[[item.description]]</td>
+ <td class="changesLink"><a href$="[[_computeChangesLink(item.name)]]">(view all)</a></td>
<td class="repositoryBrowser">
<template is="dom-repeat"
items="[[_computeWeblink(item)]]" as="link">
diff --git a/polygerrit-ui/app/elements/admin/gr-repo-list/gr-repo-list.js b/polygerrit-ui/app/elements/admin/gr-repo-list/gr-repo-list.js
index bf1f8809..116f084 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo-list/gr-repo-list.js
+++ b/polygerrit-ui/app/elements/admin/gr-repo-list/gr-repo-list.js
@@ -102,6 +102,10 @@
return this.getUrl(this._path + '/', name);
},
+ _computeChangesLink(name) {
+ return Gerrit.Nav.getUrlForProjectChanges(name);
+ },
+
_getCreateRepoCapability() {
return this.$.restAPI.getAccount().then(account => {
if (!account) { return; }
diff --git a/polygerrit-ui/app/elements/admin/gr-repo/gr-repo.html b/polygerrit-ui/app/elements/admin/gr-repo/gr-repo.html
index eabe7a3..5fa8187 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo/gr-repo.html
+++ b/polygerrit-ui/app/elements/admin/gr-repo/gr-repo.html
@@ -55,7 +55,17 @@
</style>
<style include="gr-form-styles"></style>
<main class="gr-form-styles read-only">
- <h1 id="Title">[[repo]]</h1>
+ <style include="shared-styles"></style>
+ <style include="dashboard-header-styles"></style>
+ <div class="info">
+ <h1 id="Title" class$="name">
+ [[repo]]
+ <hr/>
+ </h1>
+ <div>
+ <a href$="[[_computeChangesUrl(repo)]]">(view changes)</a>
+ </div>
+ </div>
<div id="loading" class$="[[_computeLoadingClass(_loading)]]">Loading...</div>
<div id="loadedContent" class$="[[_computeLoadingClass(_loading)]]">
<div id="downloadContent" class$="[[_computeDownloadClass(_schemes)]]">
diff --git a/polygerrit-ui/app/elements/admin/gr-repo/gr-repo.js b/polygerrit-ui/app/elements/admin/gr-repo/gr-repo.js
index 94b9e3f..6443095 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo/gr-repo.js
+++ b/polygerrit-ui/app/elements/admin/gr-repo/gr-repo.js
@@ -318,5 +318,9 @@
_computeRepositoriesClass(config) {
return config ? 'showConfig': '';
},
+
+ _computeChangesUrl(name) {
+ return Gerrit.Nav.getUrlForProjectChanges(name);
+ },
});
})();
diff --git a/polygerrit-ui/app/elements/change-list/gr-dashboard-view/gr-dashboard-view.html b/polygerrit-ui/app/elements/change-list/gr-dashboard-view/gr-dashboard-view.html
index 99aa265..b0ba8a2b 100644
--- a/polygerrit-ui/app/elements/change-list/gr-dashboard-view/gr-dashboard-view.html
+++ b/polygerrit-ui/app/elements/change-list/gr-dashboard-view/gr-dashboard-view.html
@@ -87,7 +87,7 @@
<div hidden$="[[_loading]]" hidden>
<gr-user-header
user-id="[[params.user]]"
- class$="[[_computeUserHeaderClass(params.user)]]"></gr-user-header>
+ class$="[[_computeUserHeaderClass(params)]]"></gr-user-header>
<gr-change-list
show-star
show-reviewed-state
diff --git a/polygerrit-ui/app/elements/change-list/gr-dashboard-view/gr-dashboard-view.js b/polygerrit-ui/app/elements/change-list/gr-dashboard-view/gr-dashboard-view.js
index 3fc4089..0625bbe 100644
--- a/polygerrit-ui/app/elements/change-list/gr-dashboard-view/gr-dashboard-view.js
+++ b/polygerrit-ui/app/elements/change-list/gr-dashboard-view/gr-dashboard-view.js
@@ -37,7 +37,7 @@
/** @type {{ selectedChangeIndex: number }} */
viewState: Object,
- /** @type {{ user: string }} */
+ /** @type {{ project: string, user: string }} */
params: {
type: Object,
},
@@ -217,8 +217,12 @@
});
},
- _computeUserHeaderClass(userParam) {
- return userParam === 'self' ? 'hide' : '';
+ _computeUserHeaderClass(params) {
+ if (!params || !!params.project || !params.user
+ || params.user === 'self') {
+ return 'hide';
+ }
+ return '';
},
_handleToggleStar(e) {
diff --git a/polygerrit-ui/app/elements/change-list/gr-dashboard-view/gr-dashboard-view_test.html b/polygerrit-ui/app/elements/change-list/gr-dashboard-view/gr-dashboard-view_test.html
index 78fdee6..618ec65 100644
--- a/polygerrit-ui/app/elements/change-list/gr-dashboard-view/gr-dashboard-view_test.html
+++ b/polygerrit-ui/app/elements/change-list/gr-dashboard-view/gr-dashboard-view_test.html
@@ -314,10 +314,13 @@
});
test('_computeUserHeaderClass', () => {
- assert.equal(element._computeUserHeaderClass(undefined), '');
- assert.equal(element._computeUserHeaderClass(''), '');
- assert.equal(element._computeUserHeaderClass('self'), 'hide');
- assert.equal(element._computeUserHeaderClass('user'), '');
+ assert.equal(element._computeUserHeaderClass(undefined), 'hide');
+ assert.equal(element._computeUserHeaderClass({}), 'hide');
+ assert.equal(element._computeUserHeaderClass({user: 'self'}), 'hide');
+ assert.equal(element._computeUserHeaderClass({user: 'user'}), '');
+ assert.equal(
+ element._computeUserHeaderClass({project: 'p', user: 'user'}),
+ 'hide');
});
test('404 page', done => {
diff --git a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.html b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.html
index e521576..bec08a8 100644
--- a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.html
+++ b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.html
@@ -27,6 +27,7 @@
<link rel="import" href="../../shared/gr-account-link/gr-account-link.html">
<link rel="import" href="../../shared/gr-date-formatter/gr-date-formatter.html">
<link rel="import" href="../../shared/gr-editable-label/gr-editable-label.html">
+<link rel="import" href="../../shared/gr-icons/gr-icons.html">
<link rel="import" href="../../shared/gr-limited-text/gr-limited-text.html">
<link rel="import" href="../../shared/gr-linked-chip/gr-linked-chip.html">
<link rel="import" href="../../shared/gr-tooltip-content/gr-tooltip-content.html">
@@ -114,6 +115,19 @@
#parentNotCurrentMessage {
display: none;
}
+ .icon {
+ margin: -.25em 0;
+ }
+ .icon.help,
+ .icon.notTrusted {
+ color: #FFA62F;
+ }
+ .icon.invalid {
+ color: var(--vote-text-color-disliked);
+ }
+ .icon.trusted {
+ color: var(--vote-text-color-recommended);
+ }
.parentList.notCurrent.nonMerge #parentNotCurrentMessage {
--arrow-color: #ffa62f;
display: inline-block;
@@ -137,13 +151,40 @@
<span class="title">Owner</span>
<span class="value">
<gr-account-link account="[[change.owner]]"></gr-account-link>
+ <template is="dom-if" if="[[_pushCertificateValidation]]">
+ <gr-tooltip-content
+ has-tooltip
+ title$="[[_pushCertificateValidation.message]]">
+ <iron-icon
+ class$="icon [[_pushCertificateValidation.class]]"
+ icon="[[_pushCertificateValidation.icon]]">
+ </iron-icon>
+ </gr-tooltip-content>
+ </template>
</span>
</section>
- <section class$="[[_computeShowUploaderHide(change)]]">
+ <section class$="[[_computeShowRoleClass(change, _CHANGE_ROLE.UPLOADER)]]">
<span class="title">Uploader</span>
<span class="value">
<gr-account-link
- account="[[_computeShowUploader(change)]]"></gr-account-link>
+ account="[[_getNonOwnerRole(change, _CHANGE_ROLE.UPLOADER)]]"
+ ></gr-account-link>
+ </span>
+ </section>
+ <section class$="[[_computeShowRoleClass(change, _CHANGE_ROLE.AUTHOR)]]">
+ <span class="title">Author</span>
+ <span class="value">
+ <gr-account-link
+ account="[[_getNonOwnerRole(change, _CHANGE_ROLE.AUTHOR)]]"
+ ></gr-account-link>
+ </span>
+ </section>
+ <section class$="[[_computeShowRoleClass(change, _CHANGE_ROLE.COMMITTER)]]">
+ <span class="title">Committer</span>
+ <span class="value">
+ <gr-account-link
+ account="[[_getNonOwnerRole(change, _CHANGE_ROLE.COMMITTER)]]"
+ ></gr-account-link>
</span>
</section>
<section class="assignee">
@@ -244,6 +285,7 @@
is="dom-if"
if="[[_showAddTopic(change.*, _settingTopic)]]">
<gr-editable-label
+ class="topicEditableLabel"
label-text="Add a topic"
value="[[change.topic]]"
max-length="1024"
diff --git a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.js b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.js
index 8b119d8..d3fc7e0 100644
--- a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.js
+++ b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.js
@@ -17,6 +17,17 @@
(function() {
'use strict';
+ const Defs = {};
+
+ /**
+ * @typedef {{
+ * message: string,
+ * icon: string,
+ * class: string,
+ * }}
+ */
+ Defs.PushCertificateValidation;
+
const HASHTAG_ADD_MESSAGE = 'Add Hashtag';
const SubmitTypeLabel = {
@@ -30,6 +41,24 @@
const NOT_CURRENT_MESSAGE = 'Not current - rebase possible';
+ /**
+ * @enum {string}
+ */
+ const CertificateStatus = {
+ /**
+ * This certificate status is bad.
+ */
+ BAD: 'BAD',
+ /**
+ * This certificate status is OK.
+ */
+ OK: 'OK',
+ /**
+ * This certificate status is TRUSTED.
+ */
+ TRUSTED: 'TRUSTED',
+ };
+
Polymer({
is: 'gr-change-metadata',
@@ -76,6 +105,13 @@
type: Boolean,
computed: '_computeShowReviewersByState(serverConfig)',
},
+ /**
+ * @type {Defs.PushCertificateValidation}
+ */
+ _pushCertificateValidation: {
+ type: Object,
+ computed: '_computePushCertificateValidation(serverConfig, change)',
+ },
_showRequirements: {
type: Boolean,
computed: '_computeShowRequirements(change)',
@@ -97,6 +133,18 @@
type: Array,
computed: '_computeParents(change)',
},
+
+ /** @type {?} */
+ _CHANGE_ROLE: {
+ type: Object,
+ readOnly: true,
+ value: {
+ OWNER: 'owner',
+ UPLOADER: 'uploader',
+ AUTHOR: 'author',
+ COMMITTER: 'committer',
+ },
+ },
},
behaviors: [
@@ -248,6 +296,59 @@
return hasRequirements || hasLabels || !!change.work_in_progress;
},
+ /**
+ * @return {?Defs.PushCertificateValidation} object representing data for
+ * the push validation.
+ */
+ _computePushCertificateValidation(serverConfig, change) {
+ if (!serverConfig || !serverConfig.receive ||
+ !serverConfig.receive.enable_signed_push) {
+ return null;
+ }
+ const rev = change.revisions[change.current_revision];
+ if (!rev.push_certificate || !rev.push_certificate.key) {
+ return {
+ class: 'help',
+ icon: 'gr-icons:help',
+ message: 'This patch set was created without a push certificate',
+ };
+ }
+
+ const key = rev.push_certificate.key;
+ switch (key.status) {
+ case CertificateStatus.BAD:
+ return {
+ class: 'invalid',
+ icon: 'gr-icons:close',
+ message: this._problems('Push certificate is invalid', key),
+ };
+ case CertificateStatus.OK:
+ return {
+ class: 'notTrusted',
+ icon: 'gr-icons:info',
+ message: this._problems(
+ 'Push certificate is valid, but key is not trusted', key),
+ };
+ case CertificateStatus.TRUSTED:
+ return {
+ class: 'trusted',
+ icon: 'gr-icons:check',
+ message: this._problems(
+ 'Push certificate is valid and key is trusted', key),
+ };
+ default:
+ throw new Error(`unknown certificate status: ${key.status}`);
+ }
+ },
+
+ _problems(msg, key) {
+ if (!key || !key.problems || key.problems.length === 0) {
+ return msg;
+ }
+
+ return [msg + ':'].concat(key.problems).join('\n');
+ },
+
_computeProjectURL(project) {
return Gerrit.Nav.getUrlForProjectChanges(project);
},
@@ -299,24 +400,45 @@
return !!change.work_in_progress;
},
- _computeShowUploaderHide(change) {
- return this._computeShowUploader(change) ? '' : 'hideDisplay';
+ _computeShowRoleClass(change, role) {
+ return this._getNonOwnerRole(change, role) ? '' : 'hideDisplay';
},
- _computeShowUploader(change) {
+ /**
+ * Get the user with the specified role on the change. Returns null if the
+ * user with that role is the same as the owner.
+ * @param {!Object} change
+ * @param {string} role One of the values from _CHANGE_ROLE
+ * @return {Object|null} either an accound or null.
+ */
+ _getNonOwnerRole(change, role) {
if (!change.current_revision ||
!change.revisions[change.current_revision]) {
return null;
}
const rev = change.revisions[change.current_revision];
+ if (!rev) { return null; }
- if (!rev || !rev.uploader ||
- change.owner._account_id === rev.uploader._account_id) {
- return null;
+ if (role === this._CHANGE_ROLE.UPLOADER &&
+ rev.uploader &&
+ change.owner._account_id !== rev.uploader._account_id) {
+ return rev.uploader;
}
- return rev.uploader;
+ if (role === this._CHANGE_ROLE.AUTHOR &&
+ rev.commit && rev.commit.author &&
+ change.owner.email !== rev.commit.author.email) {
+ return rev.commit.author;
+ }
+
+ if (role === this._CHANGE_ROLE.COMMITTER &&
+ rev.commit && rev.commit.committer &&
+ change.owner.email !== rev.commit.committer.email) {
+ return rev.commit.committer;
+ }
+
+ return null;
},
_computeParents(change) {
@@ -343,5 +465,12 @@
_computeIsMutable(account) {
return !!Object.keys(account).length;
},
+
+ editTopic() {
+ if (this._topicReadOnly || this.change.topic) { return; }
+ // Cannot use `this.$.ID` syntax because the element exists inside of a
+ // dom-if.
+ this.$$('.topicEditableLabel').open();
+ },
});
})();
diff --git a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_test.html b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_test.html
index 17c3d70..c5a569e 100644
--- a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_test.html
+++ b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_test.html
@@ -185,7 +185,138 @@
assert.equal(element._computeWebLinks(element.commitInfo).length, 1);
});
- test('_computeShowUploader test for uploader', () => {
+ suite('_getNonOwnerRole', () => {
+ let change;
+
+ setup(() => {
+ change = {
+ owner: {
+ email: 'abc@def',
+ _account_id: 1019328,
+ },
+ revisions: {
+ rev1: {
+ _number: 1,
+ uploader: {
+ email: 'ghi@def',
+ _account_id: 1011123,
+ },
+ commit: {
+ author: {email: 'jkl@def'},
+ committer: {email: 'ghi@def'},
+ },
+ },
+ },
+ current_revision: 'rev1',
+ };
+ });
+
+ suite('role=uploader', () => {
+ test('_getNonOwnerRole for uploader', () => {
+ assert.deepEqual(
+ element._getNonOwnerRole(change, element._CHANGE_ROLE.UPLOADER),
+ {email: 'ghi@def', _account_id: 1011123});
+ });
+
+ test('_getNonOwnerRole that it does not return uploader', () => {
+ // Set the uploader email to be the same as the owner.
+ change.revisions.rev1.uploader._account_id = 1019328;
+ assert.isNull(element._getNonOwnerRole(change,
+ element._CHANGE_ROLE.UPLOADER));
+ });
+
+ test('_getNonOwnerRole null for uploader with no current rev', () => {
+ delete change.current_revision;
+ assert.isNull(element._getNonOwnerRole(change,
+ element._CHANGE_ROLE.UPLOADER));
+ });
+
+ test('_computeShowRoleClass show uploader', () => {
+ assert.equal(element._computeShowRoleClass(
+ change, element._CHANGE_ROLE.UPLOADER), '');
+ });
+
+ test('_computeShowRoleClass hide uploader', () => {
+ // Set the uploader email to be the same as the owner.
+ change.revisions.rev1.uploader._account_id = 1019328;
+ assert.equal(element._computeShowRoleClass(change,
+ element._CHANGE_ROLE.UPLOADER), 'hideDisplay');
+ });
+ });
+
+ suite('role=committer', () => {
+ test('_getNonOwnerRole for committer', () => {
+ assert.deepEqual(
+ element._getNonOwnerRole(change, element._CHANGE_ROLE.COMMITTER),
+ {email: 'ghi@def'});
+ });
+
+ test('_getNonOwnerRole that it does not return committer', () => {
+ // Set the committer email to be the same as the owner.
+ change.revisions.rev1.commit.committer.email = 'abc@def';
+ assert.isNull(element._getNonOwnerRole(change,
+ element._CHANGE_ROLE.COMMITTER));
+ });
+
+ test('_getNonOwnerRole null for committer with no current rev', () => {
+ delete change.current_revision;
+ assert.isNull(element._getNonOwnerRole(change,
+ element._CHANGE_ROLE.COMMITTER));
+ });
+
+ test('_getNonOwnerRole null for committer with no commit', () => {
+ delete change.revisions.rev1.commit;
+ assert.isNull(element._getNonOwnerRole(change,
+ element._CHANGE_ROLE.COMMITTER));
+ });
+
+ test('_getNonOwnerRole null for committer with no committer', () => {
+ delete change.revisions.rev1.commit.committer;
+ assert.isNull(element._getNonOwnerRole(change,
+ element._CHANGE_ROLE.COMMITTER));
+ });
+ });
+
+ suite('role=author', () => {
+ test('_getNonOwnerRole for author', () => {
+ assert.deepEqual(
+ element._getNonOwnerRole(change, element._CHANGE_ROLE.AUTHOR),
+ {email: 'jkl@def'});
+ });
+
+ test('_getNonOwnerRole that it does not return author', () => {
+ // Set the author email to be the same as the owner.
+ change.revisions.rev1.commit.author.email = 'abc@def';
+ assert.isNull(element._getNonOwnerRole(change,
+ element._CHANGE_ROLE.AUTHOR));
+ });
+
+ test('_getNonOwnerRole null for author with no current rev', () => {
+ delete change.current_revision;
+ assert.isNull(element._getNonOwnerRole(change,
+ element._CHANGE_ROLE.AUTHOR));
+ });
+
+ test('_getNonOwnerRole null for author with no commit', () => {
+ delete change.revisions.rev1.commit;
+ assert.isNull(element._getNonOwnerRole(change,
+ element._CHANGE_ROLE.AUTHOR));
+ });
+
+ test('_getNonOwnerRole null for author with no author', () => {
+ delete change.revisions.rev1.commit.author;
+ assert.isNull(element._getNonOwnerRole(change,
+ element._CHANGE_ROLE.AUTHOR));
+ });
+ });
+ });
+
+ test('Push Certificate Validation test BAD', () => {
+ const serverConfig = {
+ receive: {
+ enable_signed_push: true,
+ },
+ };
const change = {
change_id: 'Iad9dc96274af6946f3632be53b106ef80f7ba6ca',
owner: {
@@ -194,8 +325,13 @@
revisions: {
rev1: {
_number: 1,
- uploader: {
- _account_id: 1011123,
+ push_certificate: {
+ key: {
+ status: 'BAD',
+ problems: [
+ 'No public keys found for key ID E5E20E52',
+ ],
+ },
},
},
},
@@ -204,54 +340,21 @@
labels: {},
mergeable: true,
};
- assert.deepEqual(element._computeShowUploader(change),
- {_account_id: 1011123});
+ const result =
+ element._computePushCertificateValidation(serverConfig, change);
+ assert.equal(result.message,
+ 'Push certificate is invalid:\n' +
+ 'No public keys found for key ID E5E20E52');
+ assert.equal(result.icon, 'gr-icons:close');
+ assert.equal(result.class, 'invalid');
});
- test('_computeShowUploader test that it does not return uploader', () => {
- const change = {
- change_id: 'Iad9dc96274af6946f3632be53b106ef80f7ba6ca',
- owner: {
- _account_id: 1011123,
+ test('Push Certificate Validation test TRUSTED', () => {
+ const serverConfig = {
+ receive: {
+ enable_signed_push: true,
},
- revisions: {
- rev1: {
- _number: 1,
- uploader: {
- _account_id: 1011123,
- },
- },
- },
- current_revision: 'rev1',
- status: 'NEW',
- labels: {},
- mergeable: true,
};
- assert.isNotOk(element._computeShowUploader(change));
- });
-
- test('no current_revision makes _computeShowUploader return null', () => {
- const change = {
- change_id: 'Iad9dc96274af6946f3632be53b106ef80f7ba6ca',
- owner: {
- _account_id: 1011123,
- },
- revisions: {
- rev1: {
- _number: 1,
- uploader: {
- _account_id: 1011123,
- },
- },
- },
- status: 'NEW',
- labels: {},
- mergeable: true,
- };
- assert.isNotOk(element._computeShowUploader(change));
- });
-
- test('_computeShowUploaderHide test for string which equals true', () => {
const change = {
change_id: 'Iad9dc96274af6946f3632be53b106ef80f7ba6ca',
owner: {
@@ -260,8 +363,10 @@
revisions: {
rev1: {
_number: 1,
- uploader: {
- _account_id: 1011123,
+ push_certificate: {
+ key: {
+ status: 'TRUSTED',
+ },
},
},
},
@@ -270,21 +375,28 @@
labels: {},
mergeable: true,
};
- assert.equal(element._computeShowUploaderHide(change), '');
+ const result =
+ element._computePushCertificateValidation(serverConfig, change);
+ assert.equal(result.message,
+ 'Push certificate is valid and key is trusted');
+ assert.equal(result.icon, 'gr-icons:check');
+ assert.equal(result.class, 'trusted');
});
- test('_computeShowUploaderHide test for hideDisplay', () => {
+ test('Push Certificate Validation is missing test', () => {
+ const serverConfig = {
+ receive: {
+ enable_signed_push: true,
+ },
+ };
const change = {
change_id: 'Iad9dc96274af6946f3632be53b106ef80f7ba6ca',
owner: {
- _account_id: 1011123,
+ _account_id: 1019328,
},
revisions: {
rev1: {
_number: 1,
- uploader: {
- _account_id: 1011123,
- },
},
},
current_revision: 'rev1',
@@ -292,8 +404,12 @@
labels: {},
mergeable: true,
};
- assert.equal(
- element._computeShowUploaderHide(change), 'hideDisplay');
+ const result =
+ element._computePushCertificateValidation(serverConfig, change);
+ assert.equal(result.message,
+ 'This patch set was created without a push certificate');
+ assert.equal(result.icon, 'gr-icons:help');
+ assert.equal(result.class, 'help');
});
test('_computeParents', () => {
@@ -587,6 +703,20 @@
});
});
+ test('editTopic', () => {
+ element.account = {test: true};
+ element.change = {actions: {topic: {enabled: true}}};
+ flushAsynchronousOperations();
+
+ const label = element.$$('.topicEditableLabel');
+ assert.ok(label);
+ sandbox.stub(label, 'open');
+ element.editTopic();
+ flushAsynchronousOperations();
+
+ assert.isTrue(label.open.called);
+ });
+
suite('plugin endpoints', () => {
test('endpoint params', done => {
element.change = {labels: {}};
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 24453be..bb51fcc 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
@@ -291,6 +291,7 @@
[this.Shortcut.EXPAND_ALL_MESSAGES]: '_handleExpandAllMessages',
[this.Shortcut.COLLAPSE_ALL_MESSAGES]: '_handleCollapseAllMessages',
[this.Shortcut.OPEN_DIFF_PREFS]: '_handleOpenDiffPrefsShortcut',
+ [this.Shortcut.EDIT_TOPIC]: '_handleEditTopic',
};
},
@@ -461,9 +462,9 @@
},
_handleCommentSave(e) {
- if (!e.target.comment.__draft) { return; }
+ const draft = e.detail.comment;
+ if (!draft.__draft) { return; }
- const draft = e.target.comment;
draft.patch_set = draft.patch_set || this._patchRange.patchNum;
// The use of path-based notification helpers (set, push) can’t be used
@@ -493,9 +494,9 @@
},
_handleCommentDiscard(e) {
- if (!e.target.comment.__draft) { return; }
+ const draft = e.detail.comment;
+ if (!draft.__draft) { return; }
- const draft = e.target.comment;
if (!this._diffDrafts[draft.path]) {
return;
}
@@ -944,6 +945,14 @@
this.$.downloadOverlay.open();
},
+ _handleEditTopic(e) {
+ if (this.shouldSuppressKeyboardShortcut(e) ||
+ this.modifierPressed(e)) { return; }
+
+ e.preventDefault();
+ this.$.metadata.editTopic();
+ },
+
_handleRefreshChange(e) {
if (this.shouldSuppressKeyboardShortcut(e)) { return; }
e.preventDefault();
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 df06e55..a88142e 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
@@ -54,6 +54,7 @@
kb.bindShortcut(kb.Shortcut.EXPAND_ALL_MESSAGES, 'x');
kb.bindShortcut(kb.Shortcut.COLLAPSE_ALL_MESSAGES, 'z');
kb.bindShortcut(kb.Shortcut.OPEN_DIFF_PREFS, ',');
+ kb.bindShortcut(kb.Shortcut.EDIT_TOPIC, 't');
let element;
let sandbox;
@@ -109,6 +110,12 @@
});
suite('keyboard shortcuts', () => {
+ test('t to add topic', () => {
+ const editStub = sandbox.stub(element.$.metadata, 'editTopic');
+ MockInteractions.pressAndReleaseKeyOn(element, 83, null, 't');
+ assert(editStub.called);
+ });
+
test('S should toggle the CL star', () => {
const starStub = sandbox.stub(element.$.changeStar, 'toggleStar');
MockInteractions.pressAndReleaseKeyOn(element, 83, null, 's');
@@ -592,6 +599,7 @@
};
element._change = {
change_id: 'Iad9dc96274af6946f3632be53b106ef80f7ba6ca',
+ owner: {email: 'abc@def'},
revisions: {
rev2: {_number: 2, commit: {parents: []}},
rev1: {_number: 1, commit: {parents: []}},
@@ -658,12 +666,12 @@
path: '/foo/bar.txt',
text: 'hello',
};
- element._handleCommentSave({target: {comment: draft}});
+ element._handleCommentSave({detail: {comment: draft}});
draft.patch_set = 2;
assert.deepEqual(element._diffDrafts, {'/foo/bar.txt': [draft]});
draft.patch_set = null;
draft.text = 'hello, there';
- element._handleCommentSave({target: {comment: draft}});
+ element._handleCommentSave({detail: {comment: draft}});
draft.patch_set = 2;
assert.deepEqual(element._diffDrafts, {'/foo/bar.txt': [draft]});
const draft2 = {
@@ -672,14 +680,14 @@
path: '/foo/bar.txt',
text: 'hola',
};
- element._handleCommentSave({target: {comment: draft2}});
+ element._handleCommentSave({detail: {comment: draft2}});
draft2.patch_set = 2;
assert.deepEqual(element._diffDrafts, {'/foo/bar.txt': [draft, draft2]});
draft.patch_set = null;
- element._handleCommentDiscard({target: {comment: draft}});
+ element._handleCommentDiscard({detail: {comment: draft}});
draft.patch_set = 2;
assert.deepEqual(element._diffDrafts, {'/foo/bar.txt': [draft2]});
- element._handleCommentDiscard({target: {comment: draft2}});
+ element._handleCommentDiscard({detail: {comment: draft2}});
assert.deepEqual(element._diffDrafts, {});
});
diff --git a/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.js b/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.js
index f2cfe87..39f4d8d 100644
--- a/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.js
+++ b/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.js
@@ -82,7 +82,7 @@
],
observers: [
- '_resultsChanged(_relatedResponse.changes, _submittedTogether, ' +
+ '_resultsChanged(_relatedResponse, _submittedTogether, ' +
'_conflicts, _cherryPicks, _sameTopic)',
],
diff --git a/polygerrit-ui/app/elements/core/gr-navigation/gr-navigation.html b/polygerrit-ui/app/elements/core/gr-navigation/gr-navigation.html
index 5be259c..b1433a3 100644
--- a/polygerrit-ui/app/elements/core/gr-navigation/gr-navigation.html
+++ b/polygerrit-ui/app/elements/core/gr-navigation/gr-navigation.html
@@ -166,6 +166,7 @@
CHANGE: 'change',
DASHBOARD: 'dashboard',
DIFF: 'diff',
+ DOCUMENTATION_SEARCH: 'documentation-search',
EDIT: 'edit',
GROUP: 'group',
PLUGIN_SCREEN: 'plugin-screen',
@@ -521,14 +522,15 @@
/**
* @param {string} repo The name of the repo.
- * @param {!Array} sections The sections to display in the dashboard
+ * @param {string} dashboard The ID of the dashboard, in the form of
+ * '<ref>:<path>'.
* @return {string}
*/
- getUrlForCustomDashboard(repo, sections) {
+ getUrlForRepoDashboard(repo, dashboard) {
return this._getUrlFor({
- repo,
view: Gerrit.Nav.View.DASHBOARD,
- sections,
+ repo,
+ dashboard,
});
},
diff --git a/polygerrit-ui/app/elements/core/gr-router/gr-router.js b/polygerrit-ui/app/elements/core/gr-router/gr-router.js
index 2ea7e37..8ae30ad 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router.js
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router.js
@@ -148,6 +148,10 @@
IMPROPERLY_ENCODED_PLUS: /^\/c\/(.+)\/\ \/(.+)$/,
PLUGIN_SCREEN: /^\/x\/([\w-]+)\/([\w-]+)\/?/,
+
+ DOCUMENTATION_SEARCH_FILTER: '/Documentation/q/filter::filter',
+ DOCUMENTATION_SEARCH: /^\/Documentation\/q\/(.*)$/,
+ DOCUMENTATION: /^\/Documentation(\/)?(.+)?/,
};
/**
@@ -320,14 +324,10 @@
if (!weblinks || !weblinks.length) return [];
return weblinks.filter(weblink => !this._isDirectCommit(weblink)).map(
({name, url}) => {
- if (url.startsWith('https:') || url.startsWith('http:')) {
- return {name, url};
- } else {
- return {
- name,
- url: `../../${url}`,
- };
+ if (!url.startsWith('https:') && !url.startsWith('http:')) {
+ url = this.getBaseUrl() + (url.startsWith('/') ? '' : '/') + url;
}
+ return {name, url};
});
},
@@ -423,7 +423,8 @@
return `/dashboard/${user}?${queryParams.join('&')}`;
} else if (repoName) {
// Project dashboard.
- return `/p/${repoName}/+/dashboard/${params.dashboard}`;
+ const encodedRepo = this.encodeURL(repoName, true);
+ return `/p/${encodedRepo}/+/dashboard/${params.dashboard}`;
} else {
// User dashboard.
return `/dashboard/${params.user || 'self'}`;
@@ -848,6 +849,17 @@
this._mapRoute(RoutePattern.PLUGIN_SCREEN, '_handlePluginScreen');
+ this._mapRoute(RoutePattern.DOCUMENTATION_SEARCH_FILTER,
+ '_handleDocumentationSearchRoute');
+
+ // redirects /Documentation/q/* to /Documentation/q/filter:*
+ this._mapRoute(RoutePattern.DOCUMENTATION_SEARCH,
+ '_handleDocumentationSearchRedirectRoute');
+
+ // Makes sure /Documentation/* links work (doin't return 404)
+ this._mapRoute(RoutePattern.DOCUMENTATION,
+ '_handleDocumentationRedirectRoute');
+
// Note: this route should appear last so it only catches URLs unmatched
// by other patterns.
this._mapRoute(RoutePattern.DEFAULT, '_handleDefaultRoute');
@@ -1425,6 +1437,27 @@
this._setParams({view, plugin, screen});
},
+ _handleDocumentationSearchRoute(data) {
+ this._setParams({
+ view: Gerrit.Nav.View.DOCUMENTATION_SEARCH,
+ filter: data.params.filter || null,
+ });
+ },
+
+ _handleDocumentationSearchRedirectRoute(data) {
+ this._redirect('/Documentation/q/filter:' +
+ encodeURIComponent(data.params[0]));
+ },
+
+ _handleDocumentationRedirectRoute(data) {
+ if (data.params[1]) {
+ location.reload();
+ } else {
+ // Redirect /Documentation to /Documentation/index.html
+ this._redirect('/Documentation/index.html');
+ }
+ },
+
/**
* Catchall route for when no other route is matched.
*/
diff --git a/polygerrit-ui/app/elements/core/gr-router/gr-router_test.html b/polygerrit-ui/app/elements/core/gr-router/gr-router_test.html
index 7d4f9ba..584fb35 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router_test.html
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router_test.html
@@ -44,6 +44,23 @@
teardown(() => { sandbox.restore(); });
+ test('_getChangeWeblinks', () => {
+ sandbox.stub(element, '_isDirectCommit').returns(false);
+ sandbox.stub(element, 'getBaseUrl').returns('base');
+ const link = {name: 'test', url: 'test/url'};
+ const mapLinksToConfig = weblink => ({options: {weblinks: [weblink]}});
+ assert.deepEqual(element._getChangeWeblinks(mapLinksToConfig(link))[0],
+ {name: 'test', url: 'base/test/url'});
+
+ link.url = '/' + link.url;
+ assert.deepEqual(element._getChangeWeblinks(mapLinksToConfig(link))[0],
+ {name: 'test', url: 'base/test/url'});
+
+ link.url = 'https:/' + link.url;
+ assert.deepEqual(element._getChangeWeblinks(mapLinksToConfig(link))[0],
+ {name: 'test', url: 'https://test/url'});
+ });
+
test('_getHashFromCanonicalPath', () => {
let url = '/foo/bar';
let hash = element._getHashFromCanonicalPath(url);
@@ -157,6 +174,9 @@
'_handleDefaultRoute',
'_handleChangeLegacyRoute',
'_handleDiffLegacyRoute',
+ '_handleDocumentationRedirectRoute',
+ '_handleDocumentationSearchRoute',
+ '_handleDocumentationSearchRedirectRoute',
'_handleLegacyLinenum',
'_handleImproperlyEncodedPlusRoute',
'_handlePassThroughRoute',
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-binary.js b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-binary.js
index b2fc64c..a9242be 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-binary.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-binary.js
@@ -14,14 +14,16 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-(function(window, GrDiffBuilderSideBySide) {
+(function(window, GrDiffBuilder) {
'use strict';
// Prevent redefinition.
if (window.GrDiffBuilderBinary) { return; }
- function GrDiffBuilderBinary(diff, comments, prefs, outputEl) {
- GrDiffBuilder.call(this, diff, comments, null, prefs, outputEl);
+ function GrDiffBuilderBinary(diff, patchRange, commentThreadEls, prefs,
+ outputEl) {
+ GrDiffBuilder.call(this, diff, patchRange, commentThreadEls, prefs,
+ outputEl);
}
GrDiffBuilderBinary.prototype = Object.create(GrDiffBuilder.prototype);
@@ -43,4 +45,4 @@
};
window.GrDiffBuilderBinary = GrDiffBuilderBinary;
-})(window, GrDiffBuilderSideBySide);
+})(window, GrDiffBuilder);
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-image.js b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-image.js
index 88ff79b..c52a504 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-image.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-image.js
@@ -22,9 +22,9 @@
const IMAGE_MIME_PATTERN = /^image\/(bmp|gif|jpeg|jpg|png|tiff|webp)$/;
- function GrDiffBuilderImage(diff, comments, createThreadGroupFn, prefs,
+ function GrDiffBuilderImage(diff, patchRange, commentThreadEls, prefs,
outputEl, baseImage, revisionImage) {
- GrDiffBuilderSideBySide.call(this, diff, comments, createThreadGroupFn,
+ GrDiffBuilderSideBySide.call(this, diff, patchRange, commentThreadEls,
prefs, outputEl, []);
this._baseImage = baseImage;
this._revisionImage = revisionImage;
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-side-by-side.js b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-side-by-side.js
index fafae63..da085c2 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-side-by-side.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-side-by-side.js
@@ -20,9 +20,9 @@
// Prevent redefinition.
if (window.GrDiffBuilderSideBySide) { return; }
- function GrDiffBuilderSideBySide(diff, comments, createThreadGroupFn, prefs,
- outputEl, layers) {
- GrDiffBuilder.call(this, diff, comments, createThreadGroupFn, prefs,
+ function GrDiffBuilderSideBySide(diff, patchRange, commentThreadEls,
+ prefs, outputEl, layers) {
+ GrDiffBuilder.call(this, diff, patchRange, commentThreadEls, prefs,
outputEl, layers);
}
GrDiffBuilderSideBySide.prototype = Object.create(GrDiffBuilder.prototype);
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-unified.js b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-unified.js
index 9a04b1f..0657ee4 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-unified.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-unified.js
@@ -20,9 +20,9 @@
// Prevent redefinition.
if (window.GrDiffBuilderUnified) { return; }
- function GrDiffBuilderUnified(diff, comments, createThreadGroupFn, prefs,
+ function GrDiffBuilderUnified(diff, patchRange, commentThreadEls, prefs,
outputEl, layers) {
- GrDiffBuilder.call(this, diff, comments, createThreadGroupFn, prefs,
+ GrDiffBuilder.call(this, diff, patchRange, commentThreadEls, prefs,
outputEl, layers);
}
GrDiffBuilderUnified.prototype = Object.create(GrDiffBuilder.prototype);
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.html b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.html
index 0997fee..e77eb57 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.html
@@ -16,9 +16,9 @@
-->
<link rel="import" href="../../../bower_components/polymer/polymer.html">
<link rel="import" href="../../core/gr-reporting/gr-reporting.html">
-<link rel="import" href="../gr-diff-comment-thread/gr-diff-comment-thread.html">
-<link rel="import" href="../gr-diff-comment-thread-group/gr-diff-comment-thread-group.html">
+<link rel="import" href="../../shared/gr-js-api-interface/gr-js-api-interface.html">
<link rel="import" href="../gr-diff-processor/gr-diff-processor.html">
+<link rel="import" href="../gr-diff-comment-thread/gr-diff-comment-thread.html">
<link rel="import" href="../gr-ranged-comment-layer/gr-ranged-comment-layer.html">
<link rel="import" href="../gr-syntax-layer/gr-syntax-layer.html">
@@ -29,7 +29,7 @@
</div>
<gr-ranged-comment-layer
id="rangeLayer"
- comments="[[comments]]"></gr-ranged-comment-layer>
+ comment-ranges="[[commentRanges]]"></gr-ranged-comment-layer>
<gr-syntax-layer
id="syntaxLayer"
diff="[[diff]]"></gr-syntax-layer>
@@ -109,31 +109,35 @@
changeNum: String,
patchNum: String,
viewMode: String,
- comments: Object,
isImageDiff: Boolean,
baseImage: Object,
revisionImage: Object,
+ parentIndex: Number,
+ path: String,
projectName: String,
/**
* @type {Defs.LineOfInterest|null}
*/
lineOfInterest: Object,
- /**
- * @type {function(number, booleam, !string)}
- */
- createCommentFn: Function,
-
_builder: Object,
_groups: Array,
_layers: Array,
_showTabs: Boolean,
+ /** @type {!Array<!Gerrit.HoveredRange>} */
+ commentRanges: {
+ type: Array,
+ },
},
get diffElement() {
return this.queryEffectiveChildren('#diffTable');
},
+ get _commentThreadElements() {
+ return this.queryAllEffectiveChildren('.comment-thread');
+ },
+
observers: [
'_groupsChanged(_groups.splices)',
],
@@ -155,10 +159,6 @@
}
this._layers = layers;
-
- this.async(() => {
- this._preRenderThread();
- });
},
render(comments, prefs) {
@@ -169,7 +169,8 @@
// Stop the processor and syntax layer (if they're running).
this.cancel();
- this._builder = this._getDiffBuilder(this.diff, comments, prefs);
+ this._builder = this._getDiffBuilder(
+ this.diff, comments.meta.patchRange, prefs);
this.$.processor.context = prefs.context;
this.$.processor.keyLocations = this._getKeyLocations(comments,
@@ -293,7 +294,7 @@
throw Error(`Invalid preference value: ${pref}`);
},
- _getDiffBuilder(diff, comments, prefs) {
+ _getDiffBuilder(diff, patchRange, prefs) {
if (isNaN(prefs.tab_size) || prefs.tab_size <= 0) {
this._handlePreferenceError('tab size');
return;
@@ -305,20 +306,22 @@
}
let builder = null;
- const createFn = this.createCommentFn;
if (this.isImageDiff) {
- builder = new GrDiffBuilderImage(diff, comments, createFn, prefs,
- this.diffElement, this.baseImage, this.revisionImage);
+ builder = new GrDiffBuilderImage(diff, patchRange,
+ this._commentThreadElements, prefs, this.diffElement,
+ this.baseImage, this.revisionImage);
} else if (diff.binary) {
// If the diff is binary, but not an image.
- return new GrDiffBuilderBinary(diff, comments, prefs,
- this.diffElement);
+ return new GrDiffBuilderBinary(diff, patchRange,
+ this._commentThreadElements, prefs, this.diffElement);
} else if (this.viewMode === DiffViewMode.SIDE_BY_SIDE) {
- builder = new GrDiffBuilderSideBySide(diff, comments, createFn,
- prefs, this.diffElement, this._layers);
+ builder = new GrDiffBuilderSideBySide(diff, patchRange,
+ this._commentThreadElements, prefs, this.diffElement,
+ this._layers);
} else if (this.viewMode === DiffViewMode.UNIFIED) {
- builder = new GrDiffBuilderUnified(diff, comments, createFn, prefs,
- this.diffElement, this._layers);
+ builder = new GrDiffBuilderUnified(diff, patchRange,
+ this._commentThreadElements, prefs, this.diffElement,
+ this._layers);
}
if (!builder) {
throw Error('Unsupported diff view mode: ' + this.viewMode);
@@ -445,25 +448,6 @@
},
/**
- * In pages with large diffs, creating the first comment thread can be
- * slow because nested Polymer elements (particularly
- * iron-autogrow-textarea) add style elements to the document head,
- * which, in turn, triggers a reflow on the page. Create a hidden
- * thread, attach it to the page, and remove it so the stylesheet will
- * already exist and the user's comment will be quick to load.
- * @see https://gerrit-review.googlesource.com/c/82213/
- */
- _preRenderThread() {
- const thread = document.createElement('gr-diff-comment-thread');
- thread.setAttribute('hidden', true);
- thread.addDraft();
- const parent = Polymer.dom(this.root);
- parent.appendChild(thread);
- Polymer.dom.flush();
- parent.removeChild(thread);
- },
-
- /**
* @return {boolean} whether any of the lines in _groups are longer
* than SYNTAX_MAX_LINE_LENGTH.
*/
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js
index be53fda..6ea48ac 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js
@@ -20,6 +20,60 @@
// Prevent redefinition.
if (window.GrDiffBuilder) { return; }
+ /** @enum {string} */
+ Gerrit.DiffSide = {
+ LEFT: 'left',
+ RIGHT: 'right',
+ BOTH: 'both',
+ };
+
+ /**
+ * @param {!Array<!HTMLElement>} threadEls
+ * @param {!{beforeNumber: (number|string|undefined),
+ * afterNumber: (number|string|undefined)}}
+ * lineInfo
+ * @param {!Gerrit.DiffSide=} side The side (LEFT, RIGHT, BOTH) for
+ * which to return the threads (default: BOTH).
+ * @return {!Array<!HTMLElement>} The thread elements matching the given
+ * location.
+ */
+ Gerrit.filterThreadElsForLocation = function(
+ threadEls, lineInfo, side = Gerrit.DiffSide.BOTH) {
+ function matchesLeftLine(threadEl) {
+ return threadEl.getAttribute('comment-side') ==
+ Gerrit.DiffSide.LEFT &&
+ threadEl.getAttribute('line-num') == lineInfo.beforeNumber;
+ }
+ function matchesRightLine(threadEl) {
+ return threadEl.getAttribute('comment-side') ==
+ Gerrit.DiffSide.RIGHT &&
+ threadEl.getAttribute('line-num') == lineInfo.afterNumber;
+ }
+ function matchesFileComment(threadEl) {
+ return (side === Gerrit.DiffSide.BOTH ||
+ threadEl.getAttribute('comment-side') == side) &&
+ // line/range comments have 1-based line set, if line is falsy it's
+ // a file comment
+ !threadEl.getAttribute('line-num');
+ }
+
+ // Select the appropriate matchers for the desired side and line
+ // If side is BOTH, we want both the left and right matcher.
+ const matchers = [];
+ if (side !== Gerrit.DiffSide.RIGHT) {
+ matchers.push(matchesLeftLine);
+ }
+ if (side !== Gerrit.DiffSide.LEFT) {
+ matchers.push(matchesRightLine);
+ }
+ if (lineInfo.afterNumber === 'FILE' ||
+ lineInfo.beforeNumber === 'FILE') {
+ matchers.push(matchesFileComment);
+ }
+ return threadEls.filter(threadEl =>
+ matchers.some(matcher => matcher(threadEl)));
+ };
+
/**
* In JS, unicode code points above 0xFFFF occupy two elements of a string.
* For example '𐀏'.length is 2. An occurence of such a code point is called a
@@ -42,11 +96,10 @@
*/
const REGEX_TAB_OR_SURROGATE_PAIR = /\t|[\uD800-\uDBFF][\uDC00-\uDFFF]/;
- function GrDiffBuilder(diff, comments, createThreadGroupFn, prefs, outputEl,
- layers) {
+ function GrDiffBuilder(diff, patchRange, commentThreadEls, prefs,
+ outputEl, layers) {
this._diff = diff;
- this._comments = comments;
- this._createThreadGroupFn = createThreadGroupFn;
+ this._patchRange = patchRange;
this._prefs = prefs;
this._outputEl = outputEl;
this.groups = [];
@@ -68,15 +121,7 @@
}
}
- const allComments = [];
- for (const side of [GrDiffBuilder.Side.LEFT, GrDiffBuilder.Side.RIGHT]) {
- // This is needed by the threading.
- for (const comment of this._comments[side]) {
- comment.__commentSide = side;
- }
- allComments.push(...this._comments[side]);
- }
- this._threads = this._createThreads(allComments);
+ this._threadEls = commentThreadEls;
}
GrDiffBuilder.GroupType = {
@@ -330,150 +375,26 @@
};
/**
- * @param {!Array<Object>} threads
* @param {!GrDiffLine} line
* @param {!GrDiffBuilder.Side=} side The side (LEFT, RIGHT, BOTH) for which
- * to return the threads (default: BOTH).
- */
- GrDiffBuilder.prototype._filterThreadsForLine = function(
- threads, line, side = GrDiffBuilder.Side.BOTH) {
- function matchesLeftLine(thread) {
- return thread.commentSide == GrDiffBuilder.Side.LEFT &&
- thread.comments[0].line == line.beforeNumber;
- }
- function matchesRightLine(thread) {
- return thread.commentSide == GrDiffBuilder.Side.RIGHT &&
- thread.comments[0].line == line.afterNumber;
- }
- function matchesFileComment(thread) {
- return (side === GrDiffBuilder.Side.BOTH ||
- thread.commentSide == side) &&
- // line/range comments have 1-based line set, if line is falsy it's
- // a file comment
- !thread.comments[0].line;
- }
-
- // Select the appropriate matchers for the desired side and line
- // If side is BOTH, we want both the left and right matcher.
- const matchers = [];
- if (side !== GrDiffBuilder.Side.RIGHT) {
- matchers.push(matchesLeftLine);
- }
- if (side !== GrDiffBuilder.Side.LEFT) {
- matchers.push(matchesRightLine);
- }
- if (line.afterNumber === GrDiffLine.FILE ||
- line.beforeNumber === GrDiffLine.FILE) {
- matchers.push(matchesFileComment);
- }
-
- return threads.filter(thread => matchers.find(matcher => matcher(thread)));
- };
-
- /**
- * @param {Array<Object>} comments
- */
- GrDiffBuilder.prototype._createThreads = function(comments) {
- const sortedComments = comments.slice(0).sort((a, b) => {
- if (b.__draft && !a.__draft ) { return 0; }
- if (a.__draft && !b.__draft ) { return 1; }
- return util.parseDate(a.updated) - util.parseDate(b.updated);
- });
-
- const threads = [];
- for (const comment of sortedComments) {
- // If the comment is in reply to another comment, find that comment's
- // thread and append to it.
- if (comment.in_reply_to) {
- const thread = threads.find(thread =>
- thread.comments.some(c => c.id === comment.in_reply_to));
- if (thread) {
- thread.comments.push(comment);
- continue;
- }
- }
-
- // Otherwise, this comment starts its own thread.
- const newThread = {
- start_datetime: comment.updated,
- comments: [comment],
- commentSide: comment.__commentSide,
- patchNum: comment.patch_set,
- rootId: comment.id || comment.__draftID,
- };
- if (comment.range) {
- newThread.range = Object.assign({}, comment.range);
- }
- threads.push(newThread);
- }
- return threads;
- };
-
- /**
- * Returns the patch number that new comment threads should be attached to.
- *
- * @param {GrDiffLine} line The line new thread will be attached to.
- * @param {string=} opt_side Set to LEFT to force adding it to the LEFT side -
- * will be ignored if the left is a parent or a merge parent
- * @return {number} Patch set to attach the new thread to
- */
- GrDiffBuilder.prototype._determinePatchNumForNewThreads = function(
- patchRange, line, opt_side) {
- if ((line.type === GrDiffLine.Type.REMOVE ||
- opt_side === GrDiffBuilder.Side.LEFT) &&
- patchRange.basePatchNum !== 'PARENT' &&
- !Gerrit.PatchSetBehavior.isMergeParent(patchRange.basePatchNum)) {
- return patchRange.basePatchNum;
- } else {
- return patchRange.patchNum;
- }
- };
-
- /**
- * Returns whether the comments on the given line are on a (merge) parent.
- *
- * @param {string} firstCommentSide
- * @param {{basePatchNum: number, patchNum: number}} patchRange
- * @param {GrDiffLine} line The line the comments are on.
- * @param {string=} opt_side
- * @return {boolean} True iff the comments on the given line are on a (merge)
- * parent.
- */
- GrDiffBuilder.prototype._determineIsOnParent = function(
- firstCommentSide, patchRange, line, opt_side) {
- return ((line.type === GrDiffLine.Type.REMOVE ||
- opt_side === GrDiffBuilder.Side.LEFT) &&
- (patchRange.basePatchNum === 'PARENT' ||
- Gerrit.PatchSetBehavior.isMergeParent(
- patchRange.basePatchNum))) ||
- firstCommentSide === 'PARENT';
- };
-
- /**
- * @param {!GrDiffLine} line
- * @param {!GrDiffBuilder.Side=} side The side (LEFT, RIGHT, BOTH) for which to return
- * the thread group (default: BOTH).
+ * to return the thread group (default: BOTH).
* @return {!Object}
*/
GrDiffBuilder.prototype._commentThreadGroupForLine = function(
- line, side = GrDiffBuilder.Side.BOTH) {
- const threads =
- this._filterThreadsForLine(this._threads, line, side);
- if (!threads || threads.length === 0) {
+ line, commentSide = GrDiffBuilder.Side.BOTH) {
+ const threadElsForGroup =
+ Gerrit.filterThreadElsForLocation(this._threadEls, line, commentSide);
+ if (!threadElsForGroup || threadElsForGroup.length === 0) {
return null;
}
- const patchRange = this._comments.meta.patchRange;
- const patchNumForNewThread = this._determinePatchNumForNewThreads(
- patchRange, line, side);
- const isOnParent = this._determineIsOnParent(
- threads[0].side, patchRange, line, side);
-
- const threadGroupEl = this._createThreadGroupFn(
- patchNumForNewThread, isOnParent, side);
- threadGroupEl.threads = threads;
- if (side) {
- threadGroupEl.setAttribute('data-side', side);
+ const threadGroupEl = document.createElement('div');
+ threadGroupEl.className = 'thread-group';
+ for (const threadEl of threadElsForGroup) {
+ Polymer.dom(threadGroupEl).appendChild(threadEl);
+ }
+ if (commentSide) {
+ threadGroupEl.setAttribute('data-side', commentSide);
}
return threadGroupEl;
};
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder_test.html b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder_test.html
index 80b45a4..c277f34 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder_test.html
@@ -60,7 +60,6 @@
let prefs;
let element;
let builder;
- let createThreadGroupFn;
let sandbox;
const LINE_FEED_HTML = '<span class="style-scope gr-diff br"></span>';
@@ -76,142 +75,69 @@
show_tabs: true,
tab_size: 4,
};
- createThreadGroupFn = sinon.spy(() => ({
- setAttribute: sinon.spy(),
- }));
builder = new GrDiffBuilder(
- {content: []}, {left: [], right: []}, createThreadGroupFn, prefs);
+ {content: []}, {left: [], right: []}, [], prefs);
});
teardown(() => { sandbox.restore(); });
- test('_createThreads', () => {
- const comments = [
- {
- id: 'sallys_confession',
- message: 'i like you, jack',
- updated: '2015-12-23 15:00:20.396000000',
- __commentSide: 'left',
- }, {
- id: 'jacks_reply',
- message: 'i like you, too',
- updated: '2015-12-24 15:01:20.396000000',
- __commentSide: 'left',
- in_reply_to: 'sallys_confession',
- },
- {
- id: 'new_draft',
- message: 'i do not like either of you',
- __commentSide: 'left',
- __draft: true,
- updated: '2015-12-20 15:01:20.396000000',
- },
- ];
+ test('filterThreadElsForLocation with no threads', () => {
+ const line = {beforeNumber: 3, afterNumber: 5};
- const expectedThreadGroups = [
- {
- start_datetime: '2015-12-23 15:00:20.396000000',
- commentSide: 'left',
- comments: [{
- id: 'sallys_confession',
- message: 'i like you, jack',
- updated: '2015-12-23 15:00:20.396000000',
- __commentSide: 'left',
- }, {
- id: 'jacks_reply',
- message: 'i like you, too',
- updated: '2015-12-24 15:01:20.396000000',
- __commentSide: 'left',
- in_reply_to: 'sallys_confession',
- }],
- patchNum: undefined,
- rootId: 'sallys_confession',
- },
- {
- start_datetime: '2015-12-20 15:01:20.396000000',
- commentSide: 'left',
- comments: [
- {
- id: 'new_draft',
- message: 'i do not like either of you',
- __commentSide: 'left',
- __draft: true,
- updated: '2015-12-20 15:01:20.396000000',
- },
- ],
- patchNum: undefined,
- rootId: 'new_draft',
- },
- ];
-
- assert.deepEqual(
- builder._createThreads(comments),
- expectedThreadGroups);
+ const threads = [];
+ assert.deepEqual(Gerrit.filterThreadElsForLocation(threads, line), []);
+ assert.deepEqual(Gerrit.filterThreadElsForLocation(threads, line,
+ Gerrit.DiffSide.LEFT), []);
+ assert.deepEqual(Gerrit.filterThreadElsForLocation(threads, line,
+ Gerrit.DiffSide.RIGHT), []);
});
- test('_createThreads inherits patchNum amd range', () => {
- const comments = [{
- id: 'betsys_confession',
- message: 'i like you, jack',
- updated: '2015-12-24 15:00:10.396000000',
- range: {
- start_line: 1,
- start_character: 1,
- end_line: 1,
- end_character: 2,
- },
- patch_set: 5,
- __commentSide: 'left',
- }];
+ test('filterThreadElsForLocation for line comments', () => {
+ const line = {beforeNumber: 3, afterNumber: 5};
- expectedThreadGroups = [
- {
- start_datetime: '2015-12-24 15:00:10.396000000',
- commentSide: 'left',
- comments: [{
- id: 'betsys_confession',
- message: 'i like you, jack',
- updated: '2015-12-24 15:00:10.396000000',
- range: {
- start_line: 1,
- start_character: 1,
- end_line: 1,
- end_character: 2,
- },
- patch_set: 5,
- __commentSide: 'left',
- }],
- patchNum: 5,
- rootId: 'betsys_confession',
- range: {
- start_line: 1,
- start_character: 1,
- end_line: 1,
- end_character: 2,
- },
- },
- ];
+ const l3 = document.createElement('div');
+ l3.setAttribute('line-num', 3);
+ l3.setAttribute('comment-side', 'left');
- assert.deepEqual(
- builder._createThreads(comments),
- expectedThreadGroups);
+ const l5 = document.createElement('div');
+ l5.setAttribute('line-num', 5);
+ l5.setAttribute('comment-side', 'left');
+
+ const r3 = document.createElement('div');
+ r3.setAttribute('line-num', 3);
+ r3.setAttribute('comment-side', 'right');
+
+ const r5 = document.createElement('div');
+ r5.setAttribute('line-num', 5);
+ r5.setAttribute('comment-side', 'right');
+
+ const threadEls = [l3, l5, r3, r5];
+ assert.deepEqual(Gerrit.filterThreadElsForLocation(threadEls, line),
+ [l3, r5]);
+ assert.deepEqual(Gerrit.filterThreadElsForLocation(threadEls, line,
+ Gerrit.DiffSide.LEFT), [l3]);
+ assert.deepEqual(Gerrit.filterThreadElsForLocation(threadEls, line,
+ Gerrit.DiffSide.RIGHT), [r5]);
});
- test('multiple comments at same location but not threaded', () => {
- const comments = [
- {
- id: 'sallys_confession',
- message: 'i like you, jack',
- updated: '2015-12-23 15:00:20.396000000',
- __commentSide: 'left',
- }, {
- id: 'jacks_reply',
- message: 'i like you, too',
- updated: '2015-12-24 15:01:20.396000000',
- __commentSide: 'left',
- },
- ];
- assert.equal(builder._createThreads(comments).length, 2);
+ test('filterThreadElsForLocation for file comments', () => {
+ const line = {beforeNumber: 'FILE', afterNumber: 'FILE'};
+
+ const l = document.createElement('div');
+ l.setAttribute('comment-side', 'left');
+
+ const r = document.createElement('div');
+ r.setAttribute('comment-side', 'right');
+
+ const threadEls = [l, r];
+ assert.deepEqual(Gerrit.filterThreadElsForLocation(threadEls, line),
+ [l, r]);
+ assert.deepEqual(Gerrit.filterThreadElsForLocation(threadEls, line,
+ Gerrit.DiffSide.BOTH), [l, r]);
+ assert.deepEqual(Gerrit.filterThreadElsForLocation(threadEls, line,
+ Gerrit.DiffSide.LEFT), [l]);
+ assert.deepEqual(Gerrit.filterThreadElsForLocation(threadEls, line,
+ Gerrit.DiffSide.RIGHT), [r]);
});
test('_createElement classStr applies all classes', () => {
@@ -387,177 +313,83 @@
}
});
- test('_filterThreadsForLine with no threads', () => {
- const line = new GrDiffLine(GrDiffLine.Type.BOTH);
- line.beforeNumber = 3;
- line.afterNumber = 5;
-
- const threads = [];
- assert.deepEqual(
- builder._filterThreadsForLine(threads, line), []);
- assert.deepEqual(builder._filterThreadsForLine(threads, line,
- GrDiffBuilder.Side.LEFT), []);
- assert.deepEqual(builder._filterThreadsForLine(threads, line,
- GrDiffBuilder.Side.RIGHT), []);
- });
-
- test('_filterThreadsForLine for line comments', () => {
- const line = new GrDiffLine(GrDiffLine.Type.BOTH);
- line.beforeNumber = 3;
- line.afterNumber = 5;
-
- const l3 = {
- comments: [{id: 'l3', line: 3}],
- range: {end_line: 3},
- commentSide: 'left',
- };
- const l5 = {
- comments: [{id: 'l5', line: 5}],
- range: {end_line: 5},
- commentSide: 'left',
- };
- const r3 = {
- comments: [{id: 'r3', line: 3}],
- range: {end_line: 3},
- commentSide: 'right',
- };
- const r5 = {
- comments: [{id: 'r5', line: 5}],
- range: {end_line: 5},
- commentSide: 'right',
- };
-
- const threads = [l3, l5, r3, r5];
- assert.deepEqual(builder._filterThreadsForLine(threads, line),
- [l3, r5]);
- assert.deepEqual(builder._filterThreadsForLine(threads, line,
- GrDiffBuilder.Side.LEFT), [l3]);
- assert.deepEqual(builder._filterThreadsForLine(threads, line,
- GrDiffBuilder.Side.RIGHT), [r5]);
- });
-
- test('_filterThreadsForLine for file comments', () => {
- const line = new GrDiffLine(GrDiffLine.Type.BOTH);
- line.beforeNumber = GrDiffLine.FILE;
- line.afterNumber = GrDiffLine.FILE;
-
- const l = {
- comments: [{id: 'l', line: undefined}],
- commentSide: 'left',
- };
- const r = {
- comments: [{id: 'r', line: undefined}],
- commentSide: 'right',
- };
-
- const threads = [l, r];
- assert.deepEqual(builder._filterThreadsForLine(threads, line),
- [l, r]);
- assert.deepEqual(builder._filterThreadsForLine(threads, line,
- GrDiffBuilder.Side.BOTH), [l, r]);
- assert.deepEqual(builder._filterThreadsForLine(threads, line,
- GrDiffBuilder.Side.LEFT), [l]);
- assert.deepEqual(builder._filterThreadsForLine(threads, line,
- GrDiffBuilder.Side.RIGHT), [r]);
- });
-
test('comment thread group creation', () => {
- const l3 = {id: 'l3', line: 3, updated: '2016-08-09 00:42:32.000000000',
- __commentSide: 'left'};
- const l5 = {id: 'l5', line: 5, updated: '2016-08-09 00:42:32.000000000',
- __commentSide: 'left'};
- const r5 = {id: 'r5', line: 5, updated: '2016-08-09 00:42:32.000000000',
- __commentSide: 'right'};
+ const l3 = document.createElement('div');
+ l3.className = 'comment-thread';
+ l3.setAttribute('comment-side', 'left');
+ l3.setAttribute('line-num', 3);
+
+ const l5 = document.createElement('div');
+ l5.className = 'comment-thread';
+ l5.setAttribute('comment-side', 'left');
+ l5.setAttribute('line-num', 5);
+
+ const r5 = document.createElement('div');
+ r5.className = 'comment-thread';
+ r5.setAttribute('comment-side', 'right');
+ r5.setAttribute('line-num', 5);
builder = new GrDiffBuilder(
- {content: []}, {
- meta: {
- changeNum: '42',
- patchRange: {
- basePatchNum: 'PARENT',
- patchNum: '3',
- },
- path: '/path/to/foo',
- projectConfig: {foo: 'bar'},
- },
- left: [l3, l5],
- right: [r5],
- }, createThreadGroupFn, prefs);
+ {content: []}, {basePatchNum: 'PARENT', patchNum: '3'}, [l3, l5, r5],
+ prefs);
- function threadForComment(c, patchNum) {
- return {
- commentSide: c.__commentSide,
- comments: [c],
- patchNum,
- rootId: c.id,
- start_datetime: c.updated,
- };
- }
-
- function checkThreadGroupProps(threadGroupEl, patchNum, isOnParent,
- comments) {
- assert.equal(createThreadGroupFn.lastCall.args[0], patchNum);
- assert.equal(createThreadGroupFn.lastCall.args[1], isOnParent);
- assert.deepEqual(
- threadGroupEl.threads,
- comments.map(c => threadForComment(c, undefined)));
+ function checkThreadGroupProps(threadGroupEl,
+ expectedThreadEls) {
+ const threadEls = Polymer.dom(threadGroupEl).queryDistributedElements(
+ '.comment-thread');
+ assert.equal(threadEls.length, expectedThreadEls.length);
+ for (let i=0; i<expectedThreadEls.length; i++) {
+ assert.equal(threadEls[i], expectedThreadEls[i]);
+ }
}
let line = new GrDiffLine(GrDiffLine.Type.BOTH);
line.beforeNumber = 5;
line.afterNumber = 5;
let threadGroupEl = builder._commentThreadGroupForLine(line);
- assert.isTrue(createThreadGroupFn.calledOnce);
- checkThreadGroupProps(threadGroupEl, '3', false, [l5, r5]);
+ checkThreadGroupProps(threadGroupEl, [l5, r5]);
threadGroupEl =
builder._commentThreadGroupForLine(line, GrDiffBuilder.Side.RIGHT);
- assert.isTrue(createThreadGroupFn.calledTwice);
- checkThreadGroupProps(threadGroupEl, '3', false, [r5]);
+ checkThreadGroupProps(threadGroupEl, [r5]);
threadGroupEl =
builder._commentThreadGroupForLine(line, GrDiffBuilder.Side.LEFT);
- assert.isTrue(createThreadGroupFn.calledThrice);
- checkThreadGroupProps(threadGroupEl, '3', true, [l5]);
+ checkThreadGroupProps(threadGroupEl, [l5]);
- builder._comments.meta.patchRange.basePatchNum = '1';
+ builder._patchRange.basePatchNum = '1';
threadGroupEl = builder._commentThreadGroupForLine(line);
- assert.equal(createThreadGroupFn.callCount, 4);
- checkThreadGroupProps(threadGroupEl, '3', false, [l5, r5]);
+ checkThreadGroupProps(threadGroupEl, [l5, r5]);
threadEl =
builder._commentThreadGroupForLine(line, GrDiffBuilder.Side.LEFT);
- assert.equal(createThreadGroupFn.callCount, 5);
- checkThreadGroupProps(threadEl, '1', false, [l5]);
+ checkThreadGroupProps(threadEl, [l5]);
threadGroupEl =
builder._commentThreadGroupForLine(line, GrDiffBuilder.Side.RIGHT);
- assert.equal(createThreadGroupFn.callCount, 6);
- checkThreadGroupProps(threadGroupEl, '3', false, [r5]);
+ checkThreadGroupProps(threadGroupEl, [r5]);
- builder._comments.meta.patchRange.basePatchNum = 'PARENT';
+ builder._patchRange.basePatchNum = 'PARENT';
line = new GrDiffLine(GrDiffLine.Type.REMOVE);
line.beforeNumber = 5;
line.afterNumber = 5;
threadGroupEl = builder._commentThreadGroupForLine(line);
- assert.equal(createThreadGroupFn.callCount, 7);
- checkThreadGroupProps(threadGroupEl, '3', true, [l5, r5]);
+ checkThreadGroupProps(threadGroupEl, [l5, r5]);
line = new GrDiffLine(GrDiffLine.Type.ADD);
line.beforeNumber = 3;
line.afterNumber = 5;
threadGroupEl = builder._commentThreadGroupForLine(line);
- assert.equal(createThreadGroupFn.callCount, 8);
- checkThreadGroupProps(threadGroupEl, '3', false, [l3, r5]);
+ checkThreadGroupProps(threadGroupEl, [l3, r5]);
});
test('_handlePreferenceError called with invalid preference', () => {
sandbox.stub(element, '_handlePreferenceError');
const prefs = {tab_size: 0};
- element._getDiffBuilder(element.diff, element.comments, prefs);
+ element._getDiffBuilder(element.diff, undefined, prefs);
assert.isTrue(element._handlePreferenceError.lastCall
.calledWithExactly('tab size'));
});
@@ -584,16 +416,18 @@
});
const lineOfInterest = {number: 789, leftSide: true};
- assert.deepEqual(element._getKeyLocations(comments, lineOfInterest), {
- left: {FILE: true, 123: true, 789: true},
- right: {456: true},
- });
+ assert.deepEqual(
+ element._getKeyLocations(comments, lineOfInterest), {
+ left: {FILE: true, 123: true, 789: true},
+ right: {456: true},
+ });
delete lineOfInterest.leftSide;
- assert.deepEqual(element._getKeyLocations(comments, lineOfInterest), {
- left: {FILE: true, 123: true},
- right: {456: true, 789: true},
- });
+ assert.deepEqual(
+ element._getKeyLocations(comments, lineOfInterest), {
+ left: {FILE: true, 123: true},
+ right: {456: true, 789: true},
+ });
});
suite('_isTotal', () => {
@@ -1035,7 +869,7 @@
processStub = sandbox.stub(element.$.processor, 'process')
.returns(Promise.resolve());
sandbox.stub(element, '_anyLineTooLong').returns(true);
- comments = {left: [], right: []};
+ comments = {left: [], right: [], meta: {patchRange: undefined}};
prefs = {
line_length: 10,
show_tabs: true,
@@ -1083,6 +917,7 @@
suite('rendering', () => {
let content;
let outputEl;
+ let comments;
setup(done => {
const prefs = {
@@ -1110,9 +945,10 @@
});
element = fixture('basic');
outputEl = element.queryEffectiveChildren('#diffTable');
+ comments = {left: [], right: [], meta: {patchRange: undefined}};
sandbox.stub(element, '_getDiffBuilder', () => {
const builder = new GrDiffBuilder(
- {content}, {left: [], right: []}, null, prefs, outputEl);
+ {content}, undefined, [], prefs, outputEl);
sandbox.stub(builder, 'addColumns');
builder.buildSectionElement = function(group) {
const section = document.createElement('stub');
@@ -1124,7 +960,7 @@
return builder;
});
element.diff = {content};
- element.render({left: [], right: []}, prefs).then(done);
+ element.render(comments, prefs).then(done);
});
test('reporting', done => {
@@ -1149,7 +985,7 @@
});
test('addColumns is called', done => {
- element.render({left: [], right: []}, {}).then(done);
+ element.render(comments, {}).then(done);
assert.isTrue(element._builder.addColumns.called);
});
@@ -1173,7 +1009,7 @@
test('render-start and render are fired', done => {
const dispatchEventStub = sandbox.stub(element, 'dispatchEvent');
- element.render({left: [], right: []}, {}).then(() => {
+ element.render(comments, {}).then(() => {
const firedEventTypes = dispatchEventStub.getCalls()
.map(c => { return c.args[0].type; });
assert.include(firedEventTypes, 'render-start');
@@ -1201,7 +1037,7 @@
context: -1,
syntax_highlighting: true,
};
- element.render({left: [], right: []}, prefs);
+ element.render(comments, prefs);
});
test('cancel', () => {
@@ -1218,6 +1054,7 @@
let builder;
let diff;
let prefs;
+ let comments;
setup(done => {
element = fixture('mock-diff');
@@ -1229,8 +1066,9 @@
show_tabs: true,
tab_size: 4,
};
+ comments = {left: [], right: [], meta: {patchRange: undefined}};
- element.render({left: [], right: []}, prefs).then(() => {
+ element.render(comments, prefs).then(() => {
builder = element._builder;
done();
});
@@ -1340,7 +1178,7 @@
test('_getNextContentOnSide unified left', done => {
// Re-render as unified:
element.viewMode = 'UNIFIED_DIFF';
- element.render({left: [], right: []}, prefs).then(() => {
+ element.render(comments, prefs).then(() => {
builder = element._builder;
const startElem = builder.getContentByLine(5, 'left',
@@ -1360,7 +1198,7 @@
test('_getNextContentOnSide unified right', done => {
// Re-render as unified:
element.viewMode = 'UNIFIED_DIFF';
- element.render({left: [], right: []}, prefs).then(() => {
+ element.render(comments, prefs).then(() => {
builder = element._builder;
const startElem = builder.getContentByLine(5, 'right',
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment-thread-group/gr-diff-comment-thread-group.html b/polygerrit-ui/app/elements/diff/gr-diff-comment-thread-group/gr-diff-comment-thread-group.html
deleted file mode 100644
index 58b7c32..0000000
--- a/polygerrit-ui/app/elements/diff/gr-diff-comment-thread-group/gr-diff-comment-thread-group.html
+++ /dev/null
@@ -1,50 +0,0 @@
-<!--
-@license
-Copyright (C) 2017 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.
--->
-
-<link rel="import" href="../../../bower_components/polymer/polymer.html">
-<link rel="import" href="../gr-diff-comment-thread/gr-diff-comment-thread.html">
-<link rel="import" href="../../../styles/shared-styles.html">
-
-<dom-module id="gr-diff-comment-thread-group">
- <template>
- <style include="shared-styles">
- :host {
- display: block;
- max-width: var(--content-width, 80ch);
- white-space: normal;
- }
- gr-diff-comment-thread + gr-diff-comment-thread {
- margin-top: .2em;
- }
- </style>
- <template is="dom-repeat" items="[[threads]]" as="thread">
- <gr-diff-comment-thread
- comments="[[thread.comments]]"
- comment-side="[[thread.commentSide]]"
- is-on-parent="[[isOnParent]]"
- parent-index="[[parentIndex]]"
- change-num="[[changeNum]]"
- patch-num="[[thread.patchNum]]"
- root-id="{{thread.rootId}}"
- path="[[path]]"
- project-name="[[projectName]]"
- range="[[thread.range]]"
- on-thread-discard="_handleThreadDiscard"></gr-diff-comment-thread>
- </template>
- </template>
- <script src="gr-diff-comment-thread-group.js"></script>
-</dom-module>
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment-thread-group/gr-diff-comment-thread-group.js b/polygerrit-ui/app/elements/diff/gr-diff-comment-thread-group/gr-diff-comment-thread-group.js
deleted file mode 100644
index 23d0a58..0000000
--- a/polygerrit-ui/app/elements/diff/gr-diff-comment-thread-group/gr-diff-comment-thread-group.js
+++ /dev/null
@@ -1,108 +0,0 @@
-/**
- * @license
- * Copyright (C) 2017 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.
- */
-(function() {
- 'use strict';
-
- Polymer({
- is: 'gr-diff-comment-thread-group',
-
- properties: {
- changeNum: String,
- projectName: String,
- patchForNewThreads: String,
- isOnParent: {
- type: Boolean,
- value: false,
- },
- parentIndex: {
- type: Number,
- value: null,
- },
- threads: {
- type: Array,
- value() { return []; },
- },
- },
-
- get threadEls() {
- return Polymer.dom(this.root).querySelectorAll('gr-diff-comment-thread');
- },
-
- /**
- * Adds a new thread. Range is optional because a comment can be
- * added to a line without a range selected.
- *
- * @param {!Object} opt_range
- */
- addNewThread(commentSide, opt_range) {
- this.push('threads', {
- comments: [],
- commentSide,
- patchNum: this.patchForNewThreads,
- range: opt_range,
- });
- },
-
- removeThread(rootId) {
- for (let i = 0; i < this.threads.length; i++) {
- if (this.threads[i].rootId === rootId) {
- this.splice('threads', i, 1);
- return;
- }
- }
- },
-
- /**
- * Fetch the thread group at the given range, or the range-less thread
- * on the line if no range is provided, lineNum, and side.
- *
- * @param {string} side
- * @param {!Object=} opt_range
- * @return {!Object|undefined}
- */
- getThread(side, opt_range) {
- const threads = [].filter.call(this.threadEls,
- thread => this._rangesEqual(thread.range, opt_range))
- .filter(thread => thread.commentSide === side);
- if (threads.length === 1) {
- return threads[0];
- }
- },
-
- _handleThreadDiscard(e) {
- this.removeThread(e.detail.rootId);
- },
-
- /**
- * Compare two ranges. Either argument may be falsy, but will only return
- * true if both are falsy or if neither are falsy and have the same position
- * values.
- *
- * @param {Object=} a range 1
- * @param {Object=} b range 2
- * @return {boolean}
- */
- _rangesEqual(a, b) {
- if (!a && !b) { return true; }
- if (!a || !b) { return false; }
- return a.startLine === b.startLine &&
- a.startChar === b.startChar &&
- a.endLine === b.endLine &&
- a.endChar === b.endChar;
- },
- });
-})();
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment-thread-group/gr-diff-comment-thread-group_test.html b/polygerrit-ui/app/elements/diff/gr-diff-comment-thread-group/gr-diff-comment-thread-group_test.html
deleted file mode 100644
index 81181b1..0000000
--- a/polygerrit-ui/app/elements/diff/gr-diff-comment-thread-group/gr-diff-comment-thread-group_test.html
+++ /dev/null
@@ -1,168 +0,0 @@
-<!DOCTYPE html>
-<!--
-@license
-Copyright (C) 2017 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.
--->
-
-<meta name="viewport" content="width=device-width, minimum-scale=1.0, initial-scale=1.0, user-scalable=yes">
-<title>gr-diff-comment-thread-group</title>
-
-<script src="../../../bower_components/webcomponentsjs/webcomponents-lite.min.js"></script>
-<script src="../../../bower_components/web-component-tester/browser.js"></script>
-<link rel="import" href="../../../test/common-test-setup.html"/>
-<script src="../../../scripts/util.js"></script>
-
-<link rel="import" href="gr-diff-comment-thread-group.html">
-
-<script>void(0);</script>
-
-<test-fixture id="basic">
- <template>
- <gr-diff-comment-thread-group></gr-diff-comment-thread-group>
- </template>
-</test-fixture>
-
-<script>
- suite('gr-diff-comment-thread-group tests', () => {
- let element;
- let sandbox;
-
- setup(() => {
- sandbox = sinon.sandbox.create();
- stub('gr-rest-api-interface', {
- getLoggedIn() { return Promise.resolve(false); },
- });
- element = fixture('basic');
- });
-
- teardown(() => {
- sandbox.restore();
- });
-
- test('getThread', () => {
- const range = {
- start_line: 1,
- start_character: 1,
- end_line: 1,
- end_character: 2,
- };
- element.threads = [
- {
- rootId: 'sallys_confession',
- commentSide: 'left',
- comments: [
- {
- id: 'sallys_confession',
- message: 'i like you, jack',
- updated: '2015-12-23 15:00:20.396000000',
- __commentSide: 'left',
- }, {
- id: 'jacks_reply',
- message: 'i like you, too',
- updated: '2015-12-24 15:01:20.396000000',
- __commentSide: 'left',
- in_reply_to: 'sallys_confession',
- }, {
- id: 'new_draft',
- message: 'i do not like either of you',
- __commentSide: 'left',
- __draft: true,
- in_reply_to: 'sallys_confession',
- updated: '2015-12-20 15:01:20.396000000',
- },
- ],
- },
- {
- rootId: 'right_side_comment',
- commentSide: 'right',
- comments: [
- {
- id: 'right_side_comment',
- message: 'right side comment',
- __commentSide: 'right',
- __draft: true,
- updated: '2015-12-20 15:01:20.396000000',
- },
- ],
- }, {
- rootId: 'betsys_confession',
- commentSide: 'left',
- range,
- comments: [
- {
- id: 'betsys_confession',
- message: 'i like you more, jack',
- updated: '2015-12-24 15:00:10.396000000',
- range,
- __commentSide: 'left',
- },
- ],
- },
- ];
-
- flushAsynchronousOperations();
- assert.deepEqual(element.getThread('right').rootId, 'right_side_comment');
- assert.deepEqual(element.getThread('right').comments.length, 1);
- assert.deepEqual(element.getThread('left').rootId, 'sallys_confession');
- assert.deepEqual(element.getThread('left').comments.length, 3);
- assert.deepEqual(element.getThread('left', range).rootId,
- 'betsys_confession');
- assert.deepEqual(element.getThread('left', range).comments.length, 1);
- });
-
- test('addNewThread', () => {
- const commentSide = 'left';
- const range = {startLine: 1, endLine: 2, startChar: 3, endChar: 4};
- element.patchForNewThreads = 5;
- element.addNewThread(commentSide, range);
- assert.equal(element.threads.length, 1);
- assert.equal(element.threads[0].comments.length, 0);
- assert.equal(element.threads[0].commentSide, commentSide);
- assert.equal(element.threads[0].patchNum, 5);
- assert.equal(element.threads[0].range.startLine, range.startLine);
- assert.equal(element.threads[0].range.endLine, range.endLine);
- assert.equal(element.threads[0].range.startChar, range.startChar);
- assert.equal(element.threads[0].range.endChar, range.endChar);
- });
-
- test('removeThread', () => {
- element.threads = [
- {rootId: 4711},
- {rootId: 42},
- ];
- element.removeThread(4711);
- assert.equal(element.threads.length, 1);
- assert.equal(element.threads[0].rootId, 42);
- });
-
- test('_rangesEqual', () => {
- const range1 =
- {startLine: 123, startChar: 345, endLine: 234, endChar: 456};
- const range2 =
- {startLine: 1, startChar: 2, endLine: 3, endChar: 4};
-
- assert.isTrue(element._rangesEqual(null, null));
- assert.isTrue(element._rangesEqual(null, undefined));
- assert.isTrue(element._rangesEqual(undefined, null));
- assert.isTrue(element._rangesEqual(undefined, undefined));
-
- assert.isFalse(element._rangesEqual(range1, null));
- assert.isFalse(element._rangesEqual(null, range1));
- assert.isFalse(element._rangesEqual(range1, range2));
-
- assert.isTrue(element._rangesEqual(range1, Object.assign({}, range1)));
- });
- });
-</script>
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread.js b/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread.js
index f3e3249..a2439d7 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread.js
@@ -48,11 +48,12 @@
* version is the one whose line number column is further to the left.
*
* range:
- * The range of text that the comment refers to (startLine, startChar,
- * endLine, endChar), serialized as JSON. If set, range's startLine
- * will have the same value as line-num. Line numbers are 1-based,
- * char numbers are 0-based. The start position (startLine, startChar)
- * is inclusive, and the end position (endLine, endChar) is exclusive.
+ * The range of text that the comment refers to (start_line,
+ * start_character, end_line, end_character), serialized as JSON. If
+ * set, range's end_line will have the same value as line-num. Line
+ * numbers are 1-based, char numbers are 0-based. The start position
+ * (start_line, start_character) is inclusive, and the end position
+ * (end_line, end_character) is exclusive.
*/
properties: {
changeNum: String,
@@ -61,8 +62,8 @@
value() { return []; },
},
/**
- * @type {?{startLine: number, startChar: number, endLine: number,
- * endChar: number}}
+ * @type {?{start_line: number, start_character: number, end_line: number,
+ * end_character: number}}
*/
range: {
type: Object,
@@ -390,12 +391,7 @@
d.line = opt_lineNum;
}
if (opt_range) {
- d.range = {
- start_line: opt_range.startLine,
- start_character: opt_range.startChar,
- end_line: opt_range.endLine,
- end_character: opt_range.endChar,
- };
+ d.range = opt_range;
}
if (this.parentIndex) {
d.parent = this.parentIndex;
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread_test.html b/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread_test.html
index 58648bf..1881497 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread_test.html
@@ -724,15 +724,25 @@
});
test('reflects range to JSON serialized attribute if set', () => {
- element.range = {startLine: 4, endLine: 5, startChar: 6, endChar: 7};
+ element.range = {
+ start_line: 4,
+ end_line: 5,
+ start_character: 6,
+ end_character: 7,
+ };
assert.deepEqual(
JSON.parse(element.getAttribute('range')),
- {startLine: 4, endLine: 5, startChar: 6, endChar: 7});
+ {start_line: 4, end_line: 5, start_character: 6, end_character: 7});
});
test('removes range attribute if range is unset', () => {
- element.range = {startLine: 4, endLine: 5, startChar: 6, endChar: 7};
+ element.range = {
+ start_line: 4,
+ end_line: 5,
+ start_character: 6,
+ end_character: 7,
+ };
element.range = undefined;
assert.notOk(element.hasAttribute('range'));
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor_test.html b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor_test.html
index 9668a54..f111378 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor_test.html
@@ -60,7 +60,11 @@
diffElement.loggedIn = false;
diffElement.patchRange = {basePatchNum: 1, patchNum: 2};
- diffElement.comments = {left: [], right: []};
+ diffElement.comments = {
+ left: [],
+ right: [],
+ meta: {patchRange: undefined},
+ };
const setupDone = () => {
cursorElement._updateStops();
cursorElement.moveToFirstChunk();
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight.js b/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight.js
index 577eec6..85ba202 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight.js
@@ -21,7 +21,11 @@
is: 'gr-diff-highlight',
properties: {
- comments: Object,
+ /** @type {!Array<!Gerrit.HoveredRange>} */
+ commentRanges: {
+ type: Array,
+ notify: true,
+ },
loggedIn: Boolean,
/**
* querySelector can return null, so needs to be nullable.
@@ -71,35 +75,44 @@
},
_handleCommentMouseOver(e) {
- const comment = e.detail.comment;
- if (!comment.range) { return; }
- const lineEl = this.diffBuilder.getLineElByChild(e.target);
- const side = this.diffBuilder.getSideByLineEl(lineEl);
- const index = this._indexOfComment(side, comment);
+ const threadEl = Polymer.dom(e).localTarget;
+ const index = this._indexForThreadEl(threadEl);
+
if (index !== undefined) {
- this.set(['comments', side, index, '__hovering'], true);
+ this.set(['commentRanges', index, 'hovering'], true);
}
},
_handleCommentMouseOut(e) {
- const comment = e.detail.comment;
- if (!comment.range) { return; }
- const lineEl = this.diffBuilder.getLineElByChild(e.target);
- const side = this.diffBuilder.getSideByLineEl(lineEl);
- const index = this._indexOfComment(side, comment);
+ const threadEl = Polymer.dom(e).localTarget;
+ const index = this._indexForThreadEl(threadEl);
+
if (index !== undefined) {
- this.set(['comments', side, index, '__hovering'], false);
+ this.set(['commentRanges', index, 'hovering'], false);
}
},
- _indexOfComment(side, comment) {
- const idProp = comment.id ? 'id' : '__draftID';
- for (let i = 0; i < this.comments[side].length; i++) {
- if (comment[idProp] &&
- this.comments[side][i][idProp] === comment[idProp]) {
- return i;
- }
+ _indexForThreadEl(threadEl) {
+ const side = threadEl.getAttribute('comment-side');
+ const range = JSON.parse(threadEl.getAttribute('range'));
+
+ if (!range) return undefined;
+
+ return this._indexOfCommentRange(side, range);
+ },
+
+ _indexOfCommentRange(side, range) {
+ function rangesEqual(a, b) {
+ if (!a && !b) { return true; }
+ if (!a || !b) { return false; }
+ return a.start_line === b.start_line &&
+ a.start_character === b.start_character &&
+ a.end_line === b.end_line &&
+ a.end_character === b.end_character;
}
+
+ return this.commentRanges.findIndex(commentRange =>
+ commentRange.side === side && rangesEqual(commentRange.range, range));
},
/**
@@ -295,10 +308,10 @@
const root = Polymer.dom(this.root);
root.insertBefore(actionBox, root.firstElementChild);
actionBox.range = {
- startLine: start.line,
- startChar: start.column,
- endLine: end.line,
- endChar: end.column,
+ start_line: start.line,
+ start_character: start.column,
+ end_line: end.line,
+ end_character: end.column,
};
actionBox.side = start.side;
if (start.line === end.line) {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight_test.html b/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight_test.html
index 98d55c0..23de407 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight_test.html
@@ -205,7 +205,7 @@
test('comment-mouse-over from ranged comment causes set', () => {
sandbox.stub(element, 'set');
- sandbox.stub(element, '_indexOfComment').returns(0);
+ sandbox.stub(element, '_indexForThreadEl').returns(0);
element.fire('comment-mouse-over', {comment: {range: {}}});
assert.isTrue(element.set.called);
});
@@ -318,10 +318,10 @@
const actionBox = element.$$('gr-selection-action-box');
assert.isTrue(element.isRangeSelected());
assert.deepEqual(getActionRange(), {
- startLine: 138,
- startChar: 5,
- endLine: 138,
- endChar: 12,
+ start_line: 138,
+ start_character: 5,
+ end_line: 138,
+ end_character: 12,
});
assert.equal(getActionSide(), 'left');
assert.notOk(actionBox.positionBelow);
@@ -337,10 +337,10 @@
const actionBox = element.$$('gr-selection-action-box');
assert.deepEqual(getActionRange(), {
- startLine: 119,
- startChar: 10,
- endLine: 120,
- endChar: 36,
+ start_line: 119,
+ start_character: 10,
+ end_line: 120,
+ end_character: 36,
});
assert.equal(getActionSide(), 'right');
assert.notOk(actionBox.positionBelow);
@@ -370,10 +370,10 @@
element._handleSelection();
assert.isTrue(element.isRangeSelected());
assert.deepEqual(getActionRange(), {
- startLine: 119,
- startChar: 10,
- endLine: 120,
- endChar: 36,
+ start_line: 119,
+ start_character: 10,
+ end_line: 120,
+ end_character: 36,
});
});
@@ -383,10 +383,10 @@
emulateSelection(startContent.firstChild, 10, endContent.firstChild, 2);
assert.isTrue(element.isRangeSelected());
assert.deepEqual(getActionRange(), {
- startLine: 119,
- startChar: 10,
- endLine: 120,
- endChar: 2,
+ start_line: 119,
+ start_character: 10,
+ end_line: 120,
+ end_character: 2,
});
assert.equal(getActionSide(), 'right');
});
@@ -404,10 +404,10 @@
emulateSelection(hl.firstChild, 2, hl.nextSibling, 7);
assert.isTrue(element.isRangeSelected());
assert.deepEqual(getActionRange(), {
- startLine: 140,
- startChar: 8,
- endLine: 140,
- endChar: 23,
+ start_line: 140,
+ start_character: 8,
+ end_line: 140,
+ end_character: 23,
});
assert.equal(getActionSide(), 'left');
});
@@ -418,10 +418,10 @@
emulateSelection(hl.previousSibling, 2, hl.firstChild, 3);
assert.isTrue(element.isRangeSelected());
assert.deepEqual(getActionRange(), {
- startLine: 140,
- startChar: 18,
- endLine: 140,
- endChar: 27,
+ start_line: 140,
+ start_character: 18,
+ end_line: 140,
+ end_character: 27,
});
});
@@ -431,10 +431,10 @@
emulateSelection(content.firstChild, 2, hl.firstChild, 2);
assert.isTrue(element.isRangeSelected());
assert.deepEqual(getActionRange(), {
- startLine: 140,
- startChar: 2,
- endLine: 140,
- endChar: 61,
+ start_line: 140,
+ start_character: 2,
+ end_line: 140,
+ end_character: 61,
});
assert.equal(getActionSide(), 'left');
});
@@ -470,10 +470,10 @@
emulateSelection(comment.firstChild, 2, endContent.firstChild, 4);
assert.isTrue(element.isRangeSelected());
assert.deepEqual(getActionRange(), {
- startLine: 140,
- startChar: 83,
- endLine: 141,
- endChar: 4,
+ start_line: 140,
+ start_character: 83,
+ end_line: 141,
+ end_character: 4,
});
assert.equal(getActionSide(), 'left');
});
@@ -485,10 +485,10 @@
emulateSelection(content.firstChild, 4, comment.firstChild, 1);
assert.isTrue(element.isRangeSelected());
assert.deepEqual(getActionRange(), {
- startLine: 140,
- startChar: 4,
- endLine: 140,
- endChar: 83,
+ start_line: 140,
+ start_character: 4,
+ end_line: 140,
+ end_character: 83,
});
assert.equal(getActionSide(), 'left');
});
@@ -517,10 +517,10 @@
emulateSelection(startContent.firstChild, 3, endContent.firstChild, 14);
assert.isTrue(element.isRangeSelected());
assert.deepEqual(getActionRange(), {
- startLine: 130,
- startChar: 3,
- endLine: 146,
- endChar: 14,
+ start_line: 130,
+ start_character: 3,
+ end_line: 146,
+ end_character: 14,
});
assert.equal(getActionSide(), 'right');
});
@@ -531,10 +531,10 @@
content.firstChild, 1, content.querySelector('span'), 0);
assert.isTrue(element.isRangeSelected());
assert.deepEqual(getActionRange(), {
- startLine: 140,
- startChar: 1,
- endLine: 140,
- endChar: 51,
+ start_line: 140,
+ start_character: 1,
+ end_line: 140,
+ end_character: 51,
});
assert.equal(getActionSide(), 'left');
});
@@ -546,10 +546,10 @@
content.querySelectorAll('span')[1].nextSibling, 1);
assert.isTrue(element.isRangeSelected());
assert.deepEqual(getActionRange(), {
- startLine: 140,
- startChar: 51,
- endLine: 140,
- endChar: 71,
+ start_line: 140,
+ start_character: 51,
+ end_line: 140,
+ end_character: 71,
});
assert.equal(getActionSide(), 'left');
});
@@ -582,10 +582,10 @@
emulateSelection(startContent.firstChild, 0, endContent.firstChild, 0);
assert.isTrue(element.isRangeSelected());
assert.deepEqual(getActionRange(), {
- startLine: 119,
- startChar: 0,
- endLine: 119,
- endChar: element._getLength(startContent),
+ start_line: 119,
+ start_character: 0,
+ end_line: 119,
+ end_character: element._getLength(startContent),
});
assert.equal(getActionSide(), 'right');
});
@@ -597,10 +597,10 @@
endContent.parentElement.previousElementSibling, 0);
assert.isTrue(element.isRangeSelected());
assert.deepEqual(getActionRange(), {
- startLine: 146,
- startChar: 0,
- endLine: 146,
- endChar: 84,
+ start_line: 146,
+ start_character: 0,
+ end_line: 146,
+ end_character: 84,
});
assert.equal(getActionSide(), 'right');
});
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.html b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.html
index a5f5fd9..d335e7a 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.html
@@ -19,6 +19,7 @@
<link rel="import" href="../../../behaviors/gr-patch-set-behavior/gr-patch-set-behavior.html">
<link rel="import" href="../../core/gr-reporting/gr-reporting.html">
<link rel="import" href="../../shared/gr-rest-api-interface/gr-rest-api-interface.html">
+<link rel="import" href="../gr-diff-comment-thread/gr-diff-comment-thread.html">
<link rel="import" href="../gr-diff/gr-diff.html">
<dom-module id="gr-diff-host">
@@ -46,8 +47,7 @@
base-image="[[_baseImage]]"
revision-image=[[_revisionImage]]
blame="[[_blame]]"
- diff="[[_diff]]"
- parent-index="[[_parentIndex]]"></gr-diff>
+ diff="[[_diff]]"></gr-diff>
<gr-rest-api-interface id="restAPI"></gr-rest-api-interface>
<gr-reporting id="reporting" category="diff"></gr-reporting>
</template>
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.js b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.js
index 056ab60..814c7268 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.js
@@ -46,12 +46,29 @@
}
/**
+ * Compare two ranges. Either argument may be falsy, but will only return
+ * true if both are falsy or if neither are falsy and have the same position
+ * values.
+ *
+ * @param {Gerrit.Range=} a range 1
+ * @param {Gerrit.Range=} b range 2
+ * @return {boolean}
+ */
+ function rangesEqual(a, b) {
+ if (!a && !b) { return true; }
+ if (!a || !b) { return false; }
+ return a.start_line === b.start_line &&
+ a.start_character === b.start_character &&
+ a.end_line === b.end_line &&
+ a.end_character === b.end_character;
+ }
+
+ /**
* Wrapper around gr-diff.
*
* Webcomponent fetching diffs and related data from restAPI and passing them
* to the presentational gr-diff for rendering.
*/
- // TODO(oler): Move all calls to restAPI from gr-diff here.
Polymer({
is: 'gr-diff-host',
@@ -108,7 +125,10 @@
type: Boolean,
value: false,
},
- comments: Object,
+ comments: {
+ type: Object,
+ observer: '_commentsChanged',
+ },
lineWrapping: {
type: Boolean,
value: false,
@@ -176,6 +196,11 @@
type: Number,
computed: '_computeParentIndex(patchRange.*)',
},
+
+ _threadEls: {
+ type: Array,
+ value: [],
+ },
},
behaviors: [
@@ -310,7 +335,7 @@
* @return {!Array<!HTMLElement>}
*/
getThreadEls() {
- return this.$.diff.getThreadEls();
+ return this._threadEls;
},
/** @param {HTMLElement} el */
@@ -437,6 +462,70 @@
return isImageDiff(diff);
},
+
+ _commentsChanged(newComments) {
+ const allComments = [];
+ for (const side of [GrDiffBuilder.Side.LEFT, GrDiffBuilder.Side.RIGHT]) {
+ // This is needed by the threading.
+ for (const comment of newComments[side]) {
+ comment.__commentSide = side;
+ }
+ allComments.push(...newComments[side]);
+ }
+ // Currently, the only way this is ever changed here is when the initial
+ // comments are loaded, so it's okay performance wise to clear the threads
+ // and recreate them. If this changes in future, we might want to reuse
+ // some DOM nodes here.
+ this._clearThreads();
+ const threads = this._createThreads(allComments);
+ for (const thread of threads) {
+ const threadEl = this._createThreadElement(thread);
+ this._attachThreadElement(threadEl);
+ }
+ },
+
+ /**
+ * @param {!Array<!Object>} comments
+ * @return {!Array<!Object>} Threads for the given comments.
+ */
+ _createThreads(comments) {
+ const sortedComments = comments.slice(0).sort((a, b) => {
+ if (b.__draft && !a.__draft ) { return 0; }
+ if (a.__draft && !b.__draft ) { return 1; }
+ return util.parseDate(a.updated) - util.parseDate(b.updated);
+ });
+
+ const threads = [];
+ for (const comment of sortedComments) {
+ // If the comment is in reply to another comment, find that comment's
+ // thread and append to it.
+ if (comment.in_reply_to) {
+ const thread = threads.find(thread =>
+ thread.comments.some(c => c.id === comment.in_reply_to));
+ if (thread) {
+ thread.comments.push(comment);
+ continue;
+ }
+ }
+
+ // Otherwise, this comment starts its own thread.
+ const newThread = {
+ start_datetime: comment.updated,
+ comments: [comment],
+ commentSide: comment.__commentSide,
+ patchNum: comment.patch_set,
+ rootId: comment.id || comment.__draftID,
+ lineNum: comment.line,
+ isOnParent: comment.side === 'PARENT',
+ };
+ if (comment.range) {
+ newThread.range = Object.assign({}, comment.range);
+ }
+ threads.push(newThread);
+ }
+ return threads;
+ },
+
/**
* @param {Object} blame
* @return {boolean}
@@ -456,32 +545,117 @@
/** @param {CustomEvent} e */
_handleCreateComment(e) {
- const {threadGroupEl, lineNum, side, range} = e.detail;
- const threadEl = this._getOrCreateThread(threadGroupEl, side, range);
+ const {lineNum, side, patchNum, isOnParent, range} = e.detail;
+ const threadEl = this._getOrCreateThread(patchNum, lineNum, side, range,
+ isOnParent);
threadEl.addOrEditDraft(lineNum, range);
+
this.$.reporting.recordDraftInteraction();
},
/**
- * Gets or creates a comment thread from a specific thread group.
- * May include a range, if the comment is a range comment.
+ * Gets or creates a comment thread at a given location.
+ * May provide a range, to get/create a range comment.
*
- * @param {!Object} threadGroupEl
+ * @param {string} patchNum
+ * @param {?number} lineNum
* @param {string} commentSide
- * @param {!Object=} range
+ * @param {Gerrit.Range|undefined} range
+ * @param {boolean} isOnParent
* @return {!Object}
*/
- _getOrCreateThread(threadGroupEl, commentSide, range=undefined) {
- let threadEl = threadGroupEl.getThread(commentSide, range);
-
+ _getOrCreateThread(patchNum, lineNum, commentSide, range, isOnParent) {
+ let threadEl = this._getThreadEl(lineNum, commentSide, range);
if (!threadEl) {
- threadGroupEl.addNewThread(commentSide, range);
- Polymer.dom.flush();
- threadEl = threadGroupEl.getThread(commentSide, range);
+ threadEl = this._createThreadElement({
+ comments: [],
+ commentSide,
+ patchNum,
+ lineNum,
+ range,
+ isOnParent,
+ });
+ this._attachThreadElement(threadEl);
}
return threadEl;
},
+ _attachThreadElement(threadEl) {
+ this._threadEls.push(threadEl);
+ Polymer.dom(this.$.diff).appendChild(threadEl);
+ },
+
+ _clearThreads() {
+ for (const threadEl of this._threadEls) {
+ const parent = Polymer.dom(threadEl).parentNode;
+ Polymer.dom(parent).removeChild(threadEl);
+ }
+ this._threadEls = [];
+ },
+
+ _createThreadElement(thread) {
+ const threadEl = document.createElement('gr-diff-comment-thread');
+ threadEl.className = 'comment-thread';
+ threadEl.comments = thread.comments;
+ threadEl.commentSide = thread.commentSide;
+ threadEl.isOnParent = !!thread.isOnParent;
+ threadEl.parentIndex = this._parentIndex;
+ threadEl.changeNum = this.changeNum;
+ threadEl.patchNum = thread.patchNum;
+ threadEl.lineNum = thread.lineNum;
+ const rootIdChangedListener = changeEvent => {
+ thread.rootId = changeEvent.detail.value;
+ };
+ threadEl.addEventListener('root-id-changed', rootIdChangedListener);
+ threadEl.path = this.path;
+ threadEl.projectName = this.projectName;
+ threadEl.range = thread.range;
+ const threadDiscardListener = e => {
+ const threadEl = /** @type {!Node} */ (e.currentTarget);
+
+ const parent = Polymer.dom(threadEl).parentNode;
+ Polymer.dom(parent).removeChild(threadEl);
+
+ const i = this._threadEls.findIndex(
+ threadEl => threadEl.rootId == e.detail.rootId);
+ this._threadEls.splice(i, 1);
+
+ threadEl.removeEventListener('root-id-changed', rootIdChangedListener);
+ threadEl.removeEventListener('thread-discard', threadDiscardListener);
+ };
+ threadEl.addEventListener('thread-discard', threadDiscardListener);
+ return threadEl;
+ },
+
+ /**
+ * Gets a comment thread element at a given location.
+ * May provide a range, to get a range comment.
+ *
+ * @param {?number} lineNum
+ * @param {string} commentSide
+ * @param {!Gerrit.Range=} range
+ * @return {?Node}
+ */
+ _getThreadEl(lineNum, commentSide, range=undefined) {
+ let line;
+ if (commentSide === GrDiffBuilder.Side.LEFT) {
+ line = {beforeNumber: lineNum};
+ } else if (commentSide === GrDiffBuilder.Side.RIGHT) {
+ line = {afterNumber: lineNum};
+ } else {
+ throw new Error(`Unknown side: ${commentSide}`);
+ }
+ function matchesRange(threadEl) {
+ const threadRange = /** @type {!Gerrit.Range} */(
+ JSON.parse(threadEl.getAttribute('range')));
+ return rangesEqual(threadRange, range);
+ }
+
+ const filteredThreadEls = Gerrit.filterThreadElsForLocation(
+ this._threadEls, line, commentSide).filter(matchesRange);
+ return filteredThreadEls.length ? filteredThreadEls[0] : null;
+ },
+
/**
* Take a diff that was loaded with a ignore-whitespace other than
* IGNORE_NONE, and convert delta chunks labeled as common into shared
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.html b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.html
index 24afe87..423bdc6 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.html
@@ -46,12 +46,41 @@
async getLoggedIn() { return getLoggedIn; },
});
element = fixture('basic');
+ // For reasons beyond me, fixture reuses elements, cleans out some
+ // stuff but not that list.
+ element._threadEls = [];
});
teardown(() => {
sandbox.restore();
});
+ test('thread-discard handling', () => {
+ const threads = [
+ {comments: [{id: 4711}]},
+ {comments: [{id: 42}]},
+ ];
+ element._parentIndex = 1;
+ element.changeNum = '2';
+ element.path = 'some/path';
+ element.projectName = 'Some project';
+ const threadEls = threads.map(
+ thread => element._createThreadElement(thread));
+ assert.equal(threadEls.length, 2);
+ assert.equal(threadEls[0].rootId, 4711);
+ assert.equal(threadEls[1].rootId, 42);
+ for (const threadEl of threadEls) {
+ Polymer.dom(element).appendChild(threadEl);
+ }
+
+ threadEls[0].dispatchEvent(
+ new CustomEvent('thread-discard', {detail: {rootId: 4711}}));
+ const attachedThreads = element.queryAllEffectiveChildren(
+ 'gr-diff-comment-thread');
+ assert.equal(attachedThreads.length, 1);
+ assert.equal(attachedThreads[0].rootId, 42);
+ });
+
test('reload() cancels before network resolves', () => {
const cancelStub = sandbox.stub(element.$.diff, 'cancel');
@@ -182,7 +211,11 @@
});
element.patchRange = {basePatchNum: 'PARENT', patchNum: 1};
- element.comments = {left: [], right: []};
+ element.comments = {
+ left: [],
+ right: [],
+ meta: {patchRange: element.patchRange},
+ };
});
test('renders image diffs with same file name', done => {
@@ -556,13 +589,10 @@
});
});
- test('delegates getThreadEls()', () => {
+ test('getThreadEls() returns _threadEls', () => {
const returnValue = [document.createElement('b')];
- const stub = sandbox.stub(element.$.diff, 'getThreadEls')
- .returns(returnValue);
+ element._threadEls = returnValue;
assert.equal(element.getThreadEls(), returnValue);
- assert.isTrue(stub.calledOnce);
- assert.equal(stub.lastCall.args.length, 0);
});
test('delegates addDraftAtLine(el)', () => {
@@ -771,27 +801,188 @@
});
});
+ test('_createThreads', () => {
+ const comments = [
+ {
+ id: 'sallys_confession',
+ message: 'i like you, jack',
+ updated: '2015-12-23 15:00:20.396000000',
+ line: 1,
+ __commentSide: 'left',
+ }, {
+ id: 'jacks_reply',
+ message: 'i like you, too',
+ updated: '2015-12-24 15:01:20.396000000',
+ __commentSide: 'left',
+ line: 1,
+ in_reply_to: 'sallys_confession',
+ },
+ {
+ id: 'new_draft',
+ message: 'i do not like either of you',
+ __commentSide: 'left',
+ __draft: true,
+ updated: '2015-12-20 15:01:20.396000000',
+ },
+ ];
+
+ const actualThreads = element._createThreads(comments);
+
+ assert.equal(actualThreads.length, 2);
+
+ assert.equal(
+ actualThreads[0].start_datetime, '2015-12-23 15:00:20.396000000');
+ assert.equal(actualThreads[0].commentSide, 'left');
+ assert.equal(actualThreads[0].comments.length, 2);
+ assert.deepEqual(actualThreads[0].comments[0], comments[0]);
+ assert.deepEqual(actualThreads[0].comments[1], comments[1]);
+ assert.equal(actualThreads[0].patchNum, undefined);
+ assert.equal(actualThreads[0].rootId, 'sallys_confession');
+ assert.equal(actualThreads[0].lineNum, 1);
+
+ assert.equal(
+ actualThreads[1].start_datetime, '2015-12-20 15:01:20.396000000');
+ assert.equal(actualThreads[1].commentSide, 'left');
+ assert.equal(actualThreads[1].comments.length, 1);
+ assert.deepEqual(actualThreads[1].comments[0], comments[2]);
+ assert.equal(actualThreads[1].patchNum, undefined);
+ assert.equal(actualThreads[1].rootId, 'new_draft');
+ assert.equal(actualThreads[1].lineNum, undefined);
+ });
+
+ test('_createThreads inherits patchNum and range', () => {
+ const comments = [{
+ id: 'betsys_confession',
+ message: 'i like you, jack',
+ updated: '2015-12-24 15:00:10.396000000',
+ range: {
+ start_line: 1,
+ start_character: 1,
+ end_line: 1,
+ end_character: 2,
+ },
+ patch_set: 5,
+ __commentSide: 'left',
+ line: 1,
+ }];
+
+ expectedThreads = [
+ {
+ start_datetime: '2015-12-24 15:00:10.396000000',
+ commentSide: 'left',
+ comments: [{
+ id: 'betsys_confession',
+ message: 'i like you, jack',
+ updated: '2015-12-24 15:00:10.396000000',
+ range: {
+ start_line: 1,
+ start_character: 1,
+ end_line: 1,
+ end_character: 2,
+ },
+ patch_set: 5,
+ __commentSide: 'left',
+ line: 1,
+ }],
+ patchNum: 5,
+ rootId: 'betsys_confession',
+ range: {
+ start_line: 1,
+ start_character: 1,
+ end_line: 1,
+ end_character: 2,
+ },
+ lineNum: 1,
+ isOnParent: false,
+ },
+ ];
+
+ assert.deepEqual(
+ element._createThreads(comments),
+ expectedThreads);
+ });
+
+ test('_createThreads does not thread unrelated comments at same location',
+ () => {
+ const comments = [
+ {
+ id: 'sallys_confession',
+ message: 'i like you, jack',
+ updated: '2015-12-23 15:00:20.396000000',
+ __commentSide: 'left',
+ }, {
+ id: 'jacks_reply',
+ message: 'i like you, too',
+ updated: '2015-12-24 15:01:20.396000000',
+ __commentSide: 'left',
+ },
+ ];
+ assert.equal(element._createThreads(comments).length, 2);
+ });
+
+ test('_createThreads derives isOnParent using side from first comment',
+ () => {
+ const comments = [
+ {
+ id: 'sallys_confession',
+ message: 'i like you, jack',
+ updated: '2015-12-23 15:00:20.396000000',
+ // line: 1,
+ // __commentSide: 'left',
+ }, {
+ id: 'jacks_reply',
+ message: 'i like you, too',
+ updated: '2015-12-24 15:01:20.396000000',
+ // __commentSide: 'left',
+ // line: 1,
+ in_reply_to: 'sallys_confession',
+ },
+ ];
+
+ assert.equal(element._createThreads(comments)[0].isOnParent, false);
+
+ comments[0].side = 'REVISION';
+ assert.equal(element._createThreads(comments)[0].isOnParent, false);
+
+ comments[0].side = 'PARENT';
+ assert.equal(element._createThreads(comments)[0].isOnParent, true);
+ });
+
test('_getOrCreateThread', () => {
- const threadGroupEl =
- document.createElement('gr-diff-comment-thread-group');
const commentSide = 'left';
- assert.isOk(element._getOrCreateThread(threadGroupEl,
- commentSide));
+ assert.isOk(element._getOrCreateThread('2', 3,
+ commentSide, undefined, false));
+
+ let threads = Polymer.dom(element.$.diff)
+ .queryDistributedElements('gr-diff-comment-thread');
+
+ assert.equal(threads.length, 1);
+ assert.equal(threads[0].commentSide, commentSide);
+ assert.equal(threads[0].range, undefined);
+ assert.equal(threads[0].isOnParent, false);
+ assert.equal(threads[0].patchNum, 2);
+
// Try to fetch a thread with a different range.
range = {
- startLine: 1,
- startChar: 1,
- endLine: 1,
- endChar: 3,
+ start_line: 1,
+ start_character: 1,
+ end_line: 1,
+ end_character: 3,
};
assert.isOk(element._getOrCreateThread(
- threadGroupEl, commentSide, range));
- const threadCount = Polymer.dom(threadGroupEl.root).
- querySelectorAll('gr-diff-comment-thread').length;
- assert.equal(threadCount, 2);
+ '3', 1, commentSide, range, true));
+
+ threads = Polymer.dom(element.$.diff)
+ .queryDistributedElements('gr-diff-comment-thread');
+
+ assert.equal(threads.length, 2);
+ assert.equal(threads[1].commentSide, commentSide);
+ assert.equal(threads[1].range, range);
+ assert.equal(threads[1].isOnParent, true);
+ assert.equal(threads[1].patchNum, 3);
});
suite('_translateChunksToIgnore', () => {
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 8acbd5d..b3210cc 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
@@ -332,7 +332,6 @@
patch-range="[[_patchRange]]"
path="[[_path]]"
prefs="[[_prefs]]"
- project-config="[[_projectConfig]]"
project-name="[[_change.project]]"
view-mode="[[_diffMode]]"
is-blame-loaded="{{_isBlameLoaded}}"
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 59c5b1f..fd54af4 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
@@ -169,6 +169,10 @@
type: Object,
computed: '_getRevisionInfo(_change)',
},
+ _reviewedFiles: {
+ type: Object,
+ value: () => new Set(),
+ },
},
behaviors: [
@@ -215,6 +219,7 @@
[this.Shortcut.TOGGLE_DIFF_MODE]: '_handleToggleDiffMode',
[this.Shortcut.TOGGLE_FILE_REVIEWED]: '_handleToggleFileReviewed',
[this.Shortcut.EXPAND_ALL_DIFF_CONTEXT]: '_handleExpandAllDiffContext',
+ [this.Shortcut.NEXT_UNREVIEWED_FILE]: '_handleNextUnreviewedFile',
// Final two are actually handled by gr-diff-comment-thread.
[this.Shortcut.EXPAND_ALL_COMMENT_THREADS]: null,
@@ -564,10 +569,18 @@
return {path: fileList[idx]};
},
+ _getReviewedFiles(changeNum, patchNum) {
+ return this.$.restAPI.getReviewedFiles(changeNum, patchNum)
+ .then(files => {
+ this._reviewedFiles = new Set(files);
+ return this._reviewedFiles;
+ });
+ },
+
_getReviewedStatus(editMode, changeNum, patchNum, path) {
if (editMode) { return Promise.resolve(false); }
- return this.$.restAPI.getReviewedFiles(changeNum, patchNum)
- .then(files => files.includes(path));
+ return this._getReviewedFiles(changeNum, patchNum)
+ .then(files => files.has(path));
},
_paramsChanged(value) {
@@ -1025,5 +1038,15 @@
_computeDiffPrefsDisabled(disableDiffPrefs, loggedIn) {
return disableDiffPrefs || !loggedIn;
},
+
+ _handleNextUnreviewedFile(e) {
+ this._setReviewed(true);
+ // Ensure that the currently viewed file always appears in unreviewedFiles
+ // so we resolve the right "next" file.
+ const unreviewedFiles = this._fileList
+ .filter(file =>
+ (file === this._path || !this._reviewedFiles.has(file)));
+ this._navToFile(this._path, unreviewedFiles, 1);
+ },
});
})();
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 3a5ca51..0274330 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
@@ -67,6 +67,7 @@
kb.bindShortcut(kb.Shortcut.EXPAND_ALL_DIFF_CONTEXT, 'shift+x');
kb.bindShortcut(kb.Shortcut.EXPAND_ALL_COMMENT_THREADS, 'e');
kb.bindShortcut(kb.Shortcut.COLLAPSE_ALL_COMMENT_THREADS, 'shift+e');
+ kb.bindShortcut(kb.Shortcut.NEXT_UNREVIEWED_FILE, 'shift+m');
let element;
let sandbox;
@@ -1127,5 +1128,22 @@
assert.isTrue(setStub.calledOnce);
assert.isTrue(setStub.calledWith(101, 'test-project'));
});
+
+ test('shift+m navigates to next unreviewed file', () => {
+ element._fileList = ['file1', 'file2', 'file3'];
+ element._reviewedFiles = new Set(['file1', 'file2']);
+ element._path = 'file1';
+ const reviewedStub = sandbox.stub(element, '_setReviewed');
+ const navStub = sandbox.stub(element, '_navToFile');
+ MockInteractions.pressAndReleaseKeyOn(element, 77, 'shift', 'm');
+ flushAsynchronousOperations();
+
+ assert.isTrue(reviewedStub.lastCall.args[0]);
+ assert.deepEqual(navStub.lastCall.args, [
+ 'file1',
+ ['file1', 'file3'],
+ 1,
+ ]);
+ });
});
</script>
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
index adb4dd6..862db10 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
@@ -20,7 +20,6 @@
<link rel="import" href="../../../styles/shared-styles.html">
<link rel="import" href="../../shared/gr-button/gr-button.html">
<link rel="import" href="../gr-diff-builder/gr-diff-builder.html">
-<link rel="import" href="../gr-diff-comment-thread/gr-diff-comment-thread.html">
<link rel="import" href="../gr-diff-highlight/gr-diff-highlight.html">
<link rel="import" href="../gr-diff-selection/gr-diff-selection.html">
<link rel="import" href="../gr-syntax-themes/gr-syntax-theme.html">
@@ -36,6 +35,11 @@
:host(.no-left) .sideBySide ::content .right:not([data-value]) + td {
display: none;
}
+ .thread-group, ::slotted(*) .thread-group {
+ display: block;
+ max-width: var(--content-width, 80ch);
+ white-space: normal;
+ }
.diffContainer {
display: flex;
font-family: var(--monospace-font-family);
@@ -276,10 +280,10 @@
<gr-diff-highlight
id="highlights"
logged-in="[[loggedIn]]"
- comments="{{comments}}">
+ comment-ranges="{{_commentRanges}}">
<gr-diff-builder
id="diffBuilder"
- comments="[[comments]]"
+ comment-ranges="[[_commentRanges]]"
project-name="[[projectName]]"
diff="[[diff]]"
diff-path="[[path]]"
@@ -290,8 +294,8 @@
is-image-diff="[[isImageDiff]]"
base-image="[[baseImage]]"
revision-image="[[revisionImage]]"
- create-comment-fn="[[_createThreadGroupFn]]"
line-of-interest="[[lineOfInterest]]">
+ <slot></slot>
<table
id="diffTable"
class$="[[_diffTableClass]]"
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
index 6016a5a..f87e46f 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
@@ -39,6 +39,15 @@
const FULL_CONTEXT = -1;
const LIMITED_CONTEXT = 10;
+ /** @typedef {{start_line: number, start_character: number,
+ * end_line: number, end_character: number}} */
+ Gerrit.Range;
+
+ function isThreadEl(node) {
+ return node.nodeType === Node.ELEMENT_NODE &&
+ node.classList.contains('comment-thread');
+ }
+
Polymer({
is: 'gr-diff',
@@ -96,6 +105,11 @@
type: Object,
value: {left: [], right: []},
},
+ /** @type {!Array<!Gerrit.HoveredRange>} */
+ _commentRanges: {
+ type: Array,
+ value: [],
+ },
lineWrapping: {
type: Boolean,
value: false,
@@ -179,17 +193,22 @@
computed: '_computeNewlineWarning(diff)',
},
- /**
- * @type {function(number, boolean, !string)}
- */
- _createThreadGroupFn: {
- type: Function,
- value() {
- return this._createCommentThreadGroup.bind(this);
- },
- },
-
_diffLength: Number,
+
+ /**
+ * Observes comment nodes added or removed after the initial render.
+ * Can be used to unregister when the entire diff is (re-)rendered or upon
+ * detachment.
+ * @type {?PolymerDomApi.ObserveHandle}
+ */
+ _incrementalNodeObserver: Object,
+
+ /**
+ * Observes comment nodes added or removed at any point.
+ * Can be used to unregister upon detachment.
+ * @type {?PolymerDomApi.ObserveHandle}
+ */
+ _nodeObserver: Object,
},
behaviors: [
@@ -201,6 +220,37 @@
'comment-update': '_handleCommentUpdate',
'comment-save': '_handleCommentSave',
'create-range-comment': '_handleCreateRangeComment',
+ 'render-content': '_handleRenderContent',
+ },
+
+ attached() {
+ this._updateRangesWhenNodesChange();
+ },
+
+ detached() {
+ this._unobserveIncrementalNodes();
+ this._unobserveNodes();
+ },
+
+ _updateRangesWhenNodesChange() {
+ function commentRangeFromThreadEl(threadEl) {
+ const side = threadEl.getAttribute('comment-side');
+ const range = JSON.parse(threadEl.getAttribute('range'));
+ return {side, range, hovering: false};
+ }
+
+ this._nodeObserver = Polymer.dom(this).observeNodes(info => {
+ const addedThreadEls = info.addedNodes.filter(isThreadEl);
+ const addedCommentRanges = addedThreadEls
+ .map(commentRangeFromThreadEl)
+ .filter(({range}) => range);
+ this.push('_commentRanges', ...addedCommentRanges);
+ // In principal we should also handle removed nodes, but I have not
+ // figured out how to do that yet without also catching all the removals
+ // caused by further redistribution. Right now, comments are never
+ // removed by no longer slotting them in, so I decided to not handle
+ // this situation until it occurs.
+ });
},
/** Cancel any remaining diff builder rendering work. */
@@ -240,17 +290,6 @@
{bubbles: true}));
},
- /** @return {!Array<!HTMLElement>} */
- getThreadEls() {
- let threads = [];
- const threadGroupEls = Polymer.dom(this.root)
- .querySelectorAll('gr-diff-comment-thread-group');
- for (const threadGroupEl of threadGroupEls) {
- threads = threads.concat(threadGroupEl.threadEls);
- }
- return threads;
- },
-
/** @return {string} */
_computeContainerClass(loggedIn, viewMode, displayLine) {
const classes = ['diffContainer'];
@@ -318,7 +357,7 @@
_handleCreateRangeComment(e) {
const range = e.detail.range;
const side = e.detail.side;
- const lineNum = range.endLine;
+ const lineNum = range.end_line;
const lineEl = this.$.diffBuilder.getLineElByNumber(lineNum, side);
if (this._isValidElForComment(lineEl)) {
@@ -361,66 +400,44 @@
const contentEl = contentText.parentElement;
side = side ||
this._getCommentSideByLineAndContent(lineEl, contentEl);
- const patchNum = this._getPatchNumByLineAndContent(lineEl, contentEl);
+ const patchForNewThreads = this._getPatchNumByLineAndContent(
+ lineEl, contentEl);
const isOnParent =
- this._getIsParentCommentByLineAndContent(lineEl, contentEl);
- const threadGroupEl = this._getOrCreateThreadGroup(contentEl, patchNum,
- side, isOnParent);
+ this._getIsParentCommentByLineAndContent(lineEl, contentEl);
this.dispatchEvent(new CustomEvent('create-comment', {
bubbles: true,
detail: {
- threadGroupEl,
lineNum,
side,
+ patchNum: patchForNewThreads,
+ isOnParent,
range,
},
}));
},
_getThreadGroupForLine(contentEl) {
- return contentEl.querySelector('gr-diff-comment-thread-group');
+ return contentEl.querySelector('.thread-group');
},
/**
* Gets or creates a comment thread group for a specific line and side on a
* diff.
* @param {!Object} contentEl
- * @param {number} patchNum
- * @param {string} commentSide
- * @param {boolean} isOnParent
- * @return {!Object}
+ * @return {!Node}
*/
- _getOrCreateThreadGroup(contentEl, patchNum, commentSide, isOnParent) {
+ _getOrCreateThreadGroup(contentEl) {
// Check if thread group exists.
let threadGroupEl = this._getThreadGroupForLine(contentEl);
if (!threadGroupEl) {
- threadGroupEl = this._createCommentThreadGroup(patchNum, isOnParent,
- commentSide);
+ threadGroupEl = document.createElement('div');
+ threadGroupEl.className = 'thread-group';
contentEl.appendChild(threadGroupEl);
}
return threadGroupEl;
},
/**
- * @param {number} patchNum
- * @param {boolean} isOnParent
- * @param {!string} commentSide
- * @return {!Object}
- */
- _createCommentThreadGroup(patchNum, isOnParent, commentSide) {
- const threadGroupEl =
- document.createElement('gr-diff-comment-thread-group');
- threadGroupEl.changeNum = this.changeNum;
- threadGroupEl.commentSide = commentSide;
- threadGroupEl.patchForNewThreads = patchNum;
- threadGroupEl.path = this.path;
- threadGroupEl.isOnParent = isOnParent;
- threadGroupEl.projectName = this.projectName;
- threadGroupEl.parentIndex = this._parentIndex;
- return threadGroupEl;
- },
-
- /**
* The value to be used for the patch number of new comments created at the
* given line and content elements.
*
@@ -608,6 +625,7 @@
},
_renderDiffTable() {
+ this._unobserveIncrementalNodes();
if (!this.prefs) {
this.dispatchEvent(new CustomEvent('render', {bubbles: true}));
return;
@@ -624,6 +642,39 @@
this.$.diffBuilder.render(this.comments, this._getBypassPrefs());
},
+ _handleRenderContent() {
+ this._incrementalNodeObserver = Polymer.dom(this).observeNodes(info => {
+ const addedThreadEls = info.addedNodes.filter(isThreadEl);
+ // In principal we should also handle removed nodes, but I have not
+ // figured out how to do that yet without also catching all the removals
+ // caused by further redistribution. Right now, comments are never
+ // removed by no longer slotting them in, so I decided to not handle
+ // this situation until it occurs.
+ for (const threadEl of addedThreadEls) {
+ const lineNumString = threadEl.getAttribute('line-num') || 'FILE';
+ const commentSide = threadEl.getAttribute('comment-side');
+ const lineEl = this.$.diffBuilder.getLineElByNumber(
+ lineNumString, commentSide);
+ const contentText = this.$.diffBuilder.getContentByLineEl(lineEl);
+ const contentEl = contentText.parentElement;
+ const threadGroupEl = this._getOrCreateThreadGroup(contentEl);
+ Polymer.dom(threadGroupEl).appendChild(threadEl);
+ }
+ });
+ },
+
+ _unobserveIncrementalNodes() {
+ if (this._incrementalNodeObserver) {
+ Polymer.dom(this).unobserveNodes(this._incrementalNodeObserver);
+ }
+ },
+
+ _unobserveNodes() {
+ if (this._nodeObserver) {
+ Polymer.dom(this).unobserveNodes(this._nodeObserver);
+ }
+ },
+
/**
* Get the preferences object including the safety bypass context (if any).
*/
@@ -635,6 +686,7 @@
},
clearDiffContent() {
+ this._unobserveIncrementalNodes();
this.$.diffTable.innerHTML = null;
},
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html
index 07584c7..4befd2f 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html
@@ -295,9 +295,6 @@
test('thread groups', () => {
const contentEl = document.createElement('div');
- const commentSide = 'left';
- const patchNum = 1;
- const side = 'PARENT';
element.changeNum = 123;
element.patchRange = {basePatchNum: 1, patchNum: 2};
@@ -312,15 +309,13 @@
assert.isNotOk(element._getThreadGroupForLine(contentEl));
// A thread group gets created.
- const threadGroupEl = element._getOrCreateThreadGroup(contentEl,
- patchNum, commentSide, side);
+ const threadGroupEl = element._getOrCreateThreadGroup(contentEl);
assert.isOk(threadGroupEl);
// The new thread group can be fetched.
assert.isOk(element._getThreadGroupForLine(contentEl));
- assert.equal(contentEl.querySelectorAll(
- 'gr-diff-comment-thread-group').length, 1);
+ assert.equal(contentEl.querySelectorAll('.thread-group').length, 1);
});
suite('image diffs', () => {
@@ -339,7 +334,11 @@
};
element.patchRange = {basePatchNum: 'PARENT', patchNum: 1};
- element.comments = {left: [], right: []};
+ element.comments = {
+ left: [],
+ right: [],
+ meta: {patchRange: undefined},
+ };
element.isImageDiff = true;
element.prefs = {
auto_hide_diff_table_header: true,
@@ -668,6 +667,7 @@
element.comments = {
left: [],
right: [],
+ meta: {patchRange: undefined},
};
element.prefs = {
context: 10,
diff --git a/polygerrit-ui/app/elements/diff/gr-ranged-comment-layer/gr-ranged-comment-layer.js b/polygerrit-ui/app/elements/diff/gr-ranged-comment-layer/gr-ranged-comment-layer.js
index db14fc8..fa488f0 100644
--- a/polygerrit-ui/app/elements/diff/gr-ranged-comment-layer/gr-ranged-comment-layer.js
+++ b/polygerrit-ui/app/elements/diff/gr-ranged-comment-layer/gr-ranged-comment-layer.js
@@ -17,31 +17,34 @@
(function() {
'use strict';
- const HOVER_PATH_PATTERN = /^comments\.(left|right)\.\#(\d+)\.__hovering$/;
- const SPLICE_PATH_PATTERN = /^comments\.(left|right)\.splices$/;
+ const HOVER_PATH_PATTERN = /^commentRanges\.\#(\d+)\.hovering$/;
const RANGE_HIGHLIGHT = 'range';
const HOVER_HIGHLIGHT = 'rangeHighlight';
const NORMALIZE_RANGE_EVENT = 'normalize-range';
+ /** @typedef {{side: string, range: Gerrit.Range, hovering: boolean}} */
+ Gerrit.HoveredRange;
+
Polymer({
is: 'gr-ranged-comment-layer',
properties: {
- comments: Object,
+ /** @type {!Array<!Gerrit.HoveredRange>} */
+ commentRanges: Array,
_listeners: {
type: Array,
value() { return []; },
},
- _commentMap: {
+ _rangesMap: {
type: Object,
- value() { return {left: [], right: []}; },
+ value() { return {left: {}, right: {}}; },
},
},
observers: [
- '_handleCommentChange(comments.*)',
+ '_handleCommentRangesChange(commentRanges.*)',
],
/**
@@ -93,97 +96,78 @@
},
/**
- * Handle change in the comments by updating the comment maps and by
+ * Handle change in the ranges by updating the ranges maps and by
* emitting appropriate update notifications.
* @param {Object} record The change record.
*/
- _handleCommentChange(record) {
- if (!record.path) { return; }
+ _handleCommentRangesChange(record) {
+ if (!record) return;
// If the entire set of comments was changed.
- if (record.path === 'comments') {
- this._commentMap.left = this._computeCommentMap(this.comments.left);
- this._commentMap.right = this._computeCommentMap(this.comments.right);
- return;
+ if (record.path === 'commentRanges') {
+ this._rangesMap = {left: {}, right: {}};
+ for (const {side, range, hovering} of record.value) {
+ this._updateRangesMap(
+ side, range, hovering, (forLine, start, end, hovering) => {
+ forLine.push({start, end, hovering});
+ });
+ }
}
// If the change only changed the `hovering` property of a comment.
- let match = record.path.match(HOVER_PATH_PATTERN);
- let side;
-
+ const match = record.path.match(HOVER_PATH_PATTERN);
if (match) {
- side = match[1];
- const index = match[2];
- const comment = this.comments[side][index];
- if (comment && comment.range) {
- this._commentMap[side] = this._computeCommentMap(this.comments[side]);
- this._notifyUpdateRange(
- comment.range.start_line, comment.range.end_line, side);
- }
- return;
+ const commentRangesIndex = match[1];
+ const {side, range, hovering} = this.commentRanges[commentRangesIndex];
+ this._updateRangesMap(
+ side, range, hovering, (forLine, start, end, hovering) => {
+ const index = forLine.findIndex(lineRange =>
+ lineRange.start === start && lineRange.end === end);
+ forLine[index].hovering = hovering;
+ });
}
// If comments were spliced in or out.
- match = record.path.match(SPLICE_PATH_PATTERN);
- if (match) {
- side = match[1];
- this._commentMap[side] = this._computeCommentMap(this.comments[side]);
- this._handleCommentSplice(record.value, side);
+ if (record.path === 'commentRanges.splices') {
+ for (const indexSplice of record.value.indexSplices) {
+ const removed = indexSplice.removed;
+ for (const {side, range, hovering} of removed) {
+ this._updateRangesMap(
+ side, range, hovering, (forLine, start, end) => {
+ const index = forLine.findIndex(lineRange =>
+ lineRange.start === start && lineRange.end === end);
+ forLine.splice(index, 1);
+ });
+ }
+ const added = indexSplice.object.slice(
+ indexSplice.index, indexSplice.index + indexSplice.addedCount);
+ for (const {side, range, hovering} of added) {
+ this._updateRangesMap(
+ side, range, hovering, (forLine, start, end, hovering) => {
+ forLine.push({start, end, hovering});
+ });
+ }
+ }
}
},
- /**
- * Take a list of comments and return a sparse list mapping line numbers to
- * partial ranges. Uses an end-character-index of -1 to indicate the end of
- * the line.
- * @param {?} commentList The list of comments.
- * Getting this param to match closure requirements caused problems.
- * @return {!Object} The sparse list.
- */
- _computeCommentMap(commentList) {
- const result = {};
- for (const comment of commentList) {
- if (!comment.range) { continue; }
- const range = comment.range;
- for (let line = range.start_line; line <= range.end_line; line++) {
- if (!result[line]) { result[line] = []; }
- result[line].push({
- comment,
- start: line === range.start_line ? range.start_character : 0,
- end: line === range.end_line ? range.end_character : -1,
- });
- }
+ _updateRangesMap(side, range, hovering, operation) {
+ const forSide = this._rangesMap[side] || (this._rangesMap[side] = {});
+ for (let line = range.start_line; line <= range.end_line; line++) {
+ const forLine = forSide[line] || (forSide[line] = []);
+ const start = line === range.start_line ? range.start_character : 0;
+ const end = line === range.end_line ? range.end_character : -1;
+ operation(forLine, start, end, hovering);
}
- return result;
- },
-
- /**
- * Translate a splice record into range update notifications.
- */
- _handleCommentSplice(record, side) {
- if (!record || !record.indexSplices) { return; }
-
- for (const splice of record.indexSplices) {
- const ranges = splice.removed.length ?
- splice.removed.map(c => { return c.range; }) :
- [splice.object[splice.index].range];
- for (const range of ranges) {
- if (!range) { continue; }
- this._notifyUpdateRange(range.start_line, range.end_line, side);
- }
- }
+ this._notifyUpdateRange(range.start_line, range.end_line, side);
},
_getRangesForLine(line, side) {
const lineNum = side === 'left' ? line.beforeNumber : line.afterNumber;
- const ranges = this.get(['_commentMap', side, lineNum]) || [];
+ const ranges = this.get(['_rangesMap', side, lineNum]) || [];
return ranges
.map(range => {
- range = {
- start: range.start,
- end: range.end === -1 ? line.text.length : range.end,
- hovering: !!range.comment.__hovering,
- };
+ range.end = range.end === -1 ? line.text.length : range.end;
// Normalize invalid ranges where the start is after the end but the
// start still makes sense. Set the end to the end of the line.
diff --git a/polygerrit-ui/app/elements/diff/gr-ranged-comment-layer/gr-ranged-comment-layer_test.html b/polygerrit-ui/app/elements/diff/gr-ranged-comment-layer/gr-ranged-comment-layer_test.html
index c541e26..c198ace 100644
--- a/polygerrit-ui/app/elements/diff/gr-ranged-comment-layer/gr-ranged-comment-layer_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-ranged-comment-layer/gr-ranged-comment-layer_test.html
@@ -40,62 +40,48 @@
let sandbox;
setup(() => {
- const initialComments = {
- left: [
- {
- id: '12345',
- line: 39,
- message: 'range comment',
- range: {
- end_character: 9,
- end_line: 39,
- start_character: 6,
- start_line: 36,
- },
- }, {
- id: '23456',
- line: 100,
- message: 'non range comment',
+ const initialCommentRanges = [
+ {
+ side: 'left',
+ range: {
+ end_character: 9,
+ end_line: 39,
+ start_character: 6,
+ start_line: 36,
},
- ],
- right: [
- {
- id: '34567',
- line: 10,
- message: 'range comment',
- range: {
- end_character: 22,
- end_line: 12,
- start_character: 10,
- start_line: 10,
- },
- }, {
- id: '45678',
- line: 100,
- message: 'single line range comment',
- range: {
- end_character: 15,
- end_line: 100,
- start_character: 5,
- start_line: 100,
- },
- }, {
- id: '8675309',
- line: 55,
- message: 'nonsense range',
- range: {
- end_character: 2,
- end_line: 55,
- start_character: 32,
- start_line: 55,
- },
+ },
+ {
+ side: 'right',
+ range: {
+ end_character: 22,
+ end_line: 12,
+ start_character: 10,
+ start_line: 10,
},
- ],
- };
+ },
+ {
+ side: 'right',
+ range: {
+ end_character: 15,
+ end_line: 100,
+ start_character: 5,
+ start_line: 100,
+ },
+ },
+ {
+ side: 'right',
+ range: {
+ end_character: 2,
+ end_line: 55,
+ start_character: 32,
+ start_line: 55,
+ },
+ },
+ ];
sandbox = sinon.sandbox.create();
element = fixture('basic');
- element.comments = initialComments;
+ element.commentRanges = initialCommentRanges;
});
teardown(() => {
@@ -149,7 +135,7 @@
test('type=Remove has-comment hovering', () => {
line.type = GrDiffLine.Type.REMOVE;
line.beforeNumber = 36;
- element.set(['comments', 'left', 0, '__hovering'], true);
+ element.set(['commentRanges', 0, 'hovering'], true);
const expectedStart = 6;
const expectedLength = line.text.length - expectedStart;
@@ -210,29 +196,18 @@
});
});
- test('_handleCommentChange overwrite', () => {
- const handlerSpy = sandbox.spy(element, '_handleCommentChange');
- const mapSpy = sandbox.spy(element, '_computeCommentMap');
+ test('_handleCommentRangesChange overwrite', () => {
+ element.set('commentRanges', []);
- element.set('comments', {left: [], right: []});
-
- assert.isTrue(handlerSpy.called);
- assert.equal(mapSpy.callCount, 2);
-
- assert.equal(Object.keys(element._commentMap.left).length, 0);
- assert.equal(Object.keys(element._commentMap.right).length, 0);
+ assert.equal(Object.keys(element._rangesMap.left).length, 0);
+ assert.equal(Object.keys(element._rangesMap.right).length, 0);
});
- test('_handleCommentChange hovering', () => {
- const handlerSpy = sandbox.spy(element, '_handleCommentChange');
- const mapSpy = sandbox.spy(element, '_computeCommentMap');
+ test('_handleCommentRangesChange hovering', () => {
const notifyStub = sinon.stub();
element.addListener(notifyStub);
- element.set(['comments', 'right', 0, '__hovering'], true);
-
- assert.isTrue(handlerSpy.called);
- assert.isTrue(mapSpy.called);
+ element.set(['commentRanges', 1, 'hovering'], true);
assert.isTrue(notifyStub.called);
const lastCall = notifyStub.lastCall;
@@ -241,16 +216,11 @@
assert.equal(lastCall.args[2], 'right');
});
- test('_handleCommentChange splice out', () => {
- const handlerSpy = sandbox.spy(element, '_handleCommentChange');
- const mapSpy = sandbox.spy(element, '_computeCommentMap');
+ test('_handleCommentRangesChange splice out', () => {
const notifyStub = sinon.stub();
element.addListener(notifyStub);
- element.splice('comments.right', 0, 1);
-
- assert.isTrue(handlerSpy.called);
- assert.isTrue(mapSpy.called);
+ element.splice('commentRanges', 1, 1);
assert.isTrue(notifyStub.called);
const lastCall = notifyStub.lastCall;
@@ -259,16 +229,12 @@
assert.equal(lastCall.args[2], 'right');
});
- test('_handleCommentChange splice in', () => {
- const handlerSpy = sandbox.spy(element, '_handleCommentChange');
- const mapSpy = sandbox.spy(element, '_computeCommentMap');
+ test('_handleCommentRangesChange splice in', () => {
const notifyStub = sinon.stub();
element.addListener(notifyStub);
- element.splice('comments.left', element.comments.left.length, 0, {
- id: '56123',
- line: 250,
- message: 'new range comment',
+ element.splice('commentRanges', 1, 0, {
+ side: 'left',
range: {
end_character: 15,
end_line: 275,
@@ -277,9 +243,6 @@
},
});
- assert.isTrue(handlerSpy.called);
- assert.isTrue(mapSpy.called);
-
assert.isTrue(notifyStub.called);
const lastCall = notifyStub.lastCall;
assert.equal(lastCall.args[0], 250);
@@ -291,48 +254,48 @@
// There is only one ranged comment on the left, but it spans ll.36-39.
const leftKeys = [];
for (let i = 36; i <= 39; i++) { leftKeys.push('' + i); }
- assert.deepEqual(Object.keys(element._commentMap.left).sort(),
+ assert.deepEqual(Object.keys(element._rangesMap.left).sort(),
leftKeys.sort());
- assert.equal(element._commentMap.left[36].length, 1);
- assert.equal(element._commentMap.left[36][0].start, 6);
- assert.equal(element._commentMap.left[36][0].end, -1);
+ assert.equal(element._rangesMap.left[36].length, 1);
+ assert.equal(element._rangesMap.left[36][0].start, 6);
+ assert.equal(element._rangesMap.left[36][0].end, -1);
- assert.equal(element._commentMap.left[37].length, 1);
- assert.equal(element._commentMap.left[37][0].start, 0);
- assert.equal(element._commentMap.left[37][0].end, -1);
+ assert.equal(element._rangesMap.left[37].length, 1);
+ assert.equal(element._rangesMap.left[37][0].start, 0);
+ assert.equal(element._rangesMap.left[37][0].end, -1);
- assert.equal(element._commentMap.left[38].length, 1);
- assert.equal(element._commentMap.left[38][0].start, 0);
- assert.equal(element._commentMap.left[38][0].end, -1);
+ assert.equal(element._rangesMap.left[38].length, 1);
+ assert.equal(element._rangesMap.left[38][0].start, 0);
+ assert.equal(element._rangesMap.left[38][0].end, -1);
- assert.equal(element._commentMap.left[39].length, 1);
- assert.equal(element._commentMap.left[39][0].start, 0);
- assert.equal(element._commentMap.left[39][0].end, 9);
+ assert.equal(element._rangesMap.left[39].length, 1);
+ assert.equal(element._rangesMap.left[39][0].start, 0);
+ assert.equal(element._rangesMap.left[39][0].end, 9);
// The right has two ranged comments, one spanning ll.10-12 and the other
// on line 100.
const rightKeys = [];
for (let i = 10; i <= 12; i++) { rightKeys.push('' + i); }
rightKeys.push('55', '100');
- assert.deepEqual(Object.keys(element._commentMap.right).sort(),
+ assert.deepEqual(Object.keys(element._rangesMap.right).sort(),
rightKeys.sort());
- assert.equal(element._commentMap.right[10].length, 1);
- assert.equal(element._commentMap.right[10][0].start, 10);
- assert.equal(element._commentMap.right[10][0].end, -1);
+ assert.equal(element._rangesMap.right[10].length, 1);
+ assert.equal(element._rangesMap.right[10][0].start, 10);
+ assert.equal(element._rangesMap.right[10][0].end, -1);
- assert.equal(element._commentMap.right[11].length, 1);
- assert.equal(element._commentMap.right[11][0].start, 0);
- assert.equal(element._commentMap.right[11][0].end, -1);
+ assert.equal(element._rangesMap.right[11].length, 1);
+ assert.equal(element._rangesMap.right[11][0].start, 0);
+ assert.equal(element._rangesMap.right[11][0].end, -1);
- assert.equal(element._commentMap.right[12].length, 1);
- assert.equal(element._commentMap.right[12][0].start, 0);
- assert.equal(element._commentMap.right[12][0].end, 22);
+ assert.equal(element._rangesMap.right[12].length, 1);
+ assert.equal(element._rangesMap.right[12][0].start, 0);
+ assert.equal(element._rangesMap.right[12][0].end, 22);
- assert.equal(element._commentMap.right[100].length, 1);
- assert.equal(element._commentMap.right[100][0].start, 5);
- assert.equal(element._commentMap.right[100][0].end, 15);
+ assert.equal(element._rangesMap.right[100].length, 1);
+ assert.equal(element._rangesMap.right[100][0].start, 5);
+ assert.equal(element._rangesMap.right[100][0].end, 15);
});
test('_getRangesForLine normalizes invalid ranges', () => {
diff --git a/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box.js b/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box.js
index 0f84877..fa5c810 100644
--- a/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box.js
+++ b/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box.js
@@ -34,10 +34,10 @@
range: {
type: Object,
value: {
- startLine: NaN,
- startChar: NaN,
- endLine: NaN,
- endChar: NaN,
+ start_line: NaN,
+ start_character: NaN,
+ end_line: NaN,
+ end_character: NaN,
},
},
positionBelow: Boolean,
diff --git a/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box_test.html b/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box_test.html
index 4f1065a..dece366 100644
--- a/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box_test.html
@@ -89,10 +89,10 @@
test('event fired contains playload', () => {
const side = 'left';
const range = {
- startLine: 1,
- startChar: 11,
- endLine: 2,
- endChar: 42,
+ start_line: 1,
+ start_character: 11,
+ end_line: 2,
+ end_character: 42,
};
element.side = 'left';
element.range = range;
diff --git a/polygerrit-ui/app/elements/documentation/gr-documentation-search/gr-documentation-search.html b/polygerrit-ui/app/elements/documentation/gr-documentation-search/gr-documentation-search.html
new file mode 100644
index 0000000..720f353
--- /dev/null
+++ b/polygerrit-ui/app/elements/documentation/gr-documentation-search/gr-documentation-search.html
@@ -0,0 +1,62 @@
+<!--
+@license
+Copyright (C) 2018 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.
+-->
+<link rel="import" href="../../../bower_components/polymer/polymer.html">
+
+<link rel="import" href="../../../behaviors/base-url-behavior/base-url-behavior.html">
+<link rel="import" href="../../../behaviors/gr-list-view-behavior/gr-list-view-behavior.html">
+<link rel="import" href="../../../bower_components/iron-input/iron-input.html">
+<link rel="import" href="../../../styles/gr-table-styles.html">
+<link rel="import" href="../../../styles/shared-styles.html">
+<link rel="import" href="../../shared/gr-list-view/gr-list-view.html">
+<link rel="import" href="../../shared/gr-rest-api-interface/gr-rest-api-interface.html">
+
+<dom-module id="gr-documentation-search">
+ <template>
+ <style include="shared-styles"></style>
+ <style include="gr-table-styles"></style>
+ <gr-list-view
+ filter="[[_filter]]"
+ items=false
+ offset=0
+ loading="[[_loading]]"
+ path="[[_path]]">
+ <table id="list" class="genericList">
+ <tr class="headerRow">
+ <th class="name topHeader">Name</th>
+ <th class="name topHeader"></th>
+ <th class="name topHeader"></th>
+ </tr>
+ <tr id="loading" class$="loadingMsg [[computeLoadingClass(_loading)]]">
+ <td>Loading...</td>
+ </tr>
+ <tbody class$="[[computeLoadingClass(_loading)]]">
+ <template is="dom-repeat" items="[[_documentationSearches]]">
+ <tr class="table">
+ <td class="name">
+ <a href$="[[_computeSearchUrl(item.url)]]">[[item.title]]</a>
+ </td>
+ <td></td>
+ <td></td>
+ </tr>
+ </template>
+ </tbody>
+ </table>
+ </gr-list-view>
+ <gr-rest-api-interface id="restAPI"></gr-rest-api-interface>
+ </template>
+ <script src="gr-documentation-search.js"></script>
+</dom-module>
diff --git a/polygerrit-ui/app/elements/documentation/gr-documentation-search/gr-documentation-search.js b/polygerrit-ui/app/elements/documentation/gr-documentation-search/gr-documentation-search.js
new file mode 100644
index 0000000..f850b9d
--- /dev/null
+++ b/polygerrit-ui/app/elements/documentation/gr-documentation-search/gr-documentation-search.js
@@ -0,0 +1,81 @@
+/**
+ * @license
+ * Copyright (C) 2018 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.
+ */
+(function() {
+ 'use strict';
+
+ Polymer({
+ is: 'gr-documentation-search',
+
+ properties: {
+ /**
+ * URL params passed from the router.
+ */
+ params: {
+ type: Object,
+ observer: '_paramsChanged',
+ },
+
+ _path: {
+ type: String,
+ readOnly: true,
+ value: '/Documentation',
+ },
+ _documentationSearches: Array,
+
+ _loading: {
+ type: Boolean,
+ value: true,
+ },
+ _filter: {
+ type: String,
+ value: '',
+ },
+ },
+
+ behaviors: [
+ Gerrit.ListViewBehavior,
+ ],
+
+ attached() {
+ this.dispatchEvent(
+ new CustomEvent('title-change', {title: 'Documentation Search'}));
+ },
+
+ _paramsChanged(params) {
+ this._loading = true;
+ this._filter = this.getFilterValue(params);
+
+ return this._getDocumentationSearches(this._filter);
+ },
+
+ _getDocumentationSearches(filter) {
+ this._documentationSearches = [];
+ return this.$.restAPI.getDocumentationSearches(filter)
+ .then(searches => {
+ // Late response.
+ if (filter !== this._filter || !searches) { return; }
+ this._documentationSearches = searches;
+ this._loading = false;
+ });
+ },
+
+ _computeSearchUrl(url) {
+ if (!url) { return ''; }
+ return this.getBaseUrl() + '/' + url;
+ },
+ });
+})();
diff --git a/polygerrit-ui/app/elements/documentation/gr-documentation-search/gr-documentation-search_test.html b/polygerrit-ui/app/elements/documentation/gr-documentation-search/gr-documentation-search_test.html
new file mode 100644
index 0000000..84addb0
--- /dev/null
+++ b/polygerrit-ui/app/elements/documentation/gr-documentation-search/gr-documentation-search_test.html
@@ -0,0 +1,120 @@
+<!DOCTYPE html>
+<!--
+@license
+Copyright (C) 2018 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.
+-->
+
+<meta name="viewport" content="width=device-width, minimum-scale=1.0, initial-scale=1.0, user-scalable=yes">
+<title>gr-documentation-search</title>
+
+<script src="../../../bower_components/page/page.js"></script>
+<script src="../../../bower_components/webcomponentsjs/webcomponents-lite.min.js"></script>
+<script src="../../../bower_components/web-component-tester/browser.js"></script>
+<link rel="import" href="../../../test/common-test-setup.html"/>
+<link rel="import" href="gr-documentation-search.html">
+
+<script>void(0);</script>
+
+<test-fixture id="basic">
+ <template>
+ <gr-documentation-search></gr-documentation-search>
+ </template>
+</test-fixture>
+
+<script>
+ let counter;
+ const documentationGenerator = () => {
+ return {
+ title: `Gerrit Code Review - REST API Developers Notes${++counter}`,
+ url: 'Documentation/dev-rest-api.html',
+ };
+ };
+
+ suite('gr-documentation-search tests', () => {
+ let element;
+ let documentationSearches;
+ let sandbox;
+ let value;
+
+ setup(() => {
+ sandbox = sinon.sandbox.create();
+ sandbox.stub(page, 'show');
+ element = fixture('basic');
+ counter = 0;
+ });
+
+ teardown(() => {
+ sandbox.restore();
+ });
+
+ suite('list with searches for documentation', () => {
+ setup(done => {
+ documentationSearches = _.times(26, documentationGenerator);
+ stub('gr-rest-api-interface', {
+ getDocumentationSearches() {
+ return Promise.resolve(documentationSearches);
+ },
+ });
+ element._paramsChanged(value).then(() => { flush(done); });
+ });
+
+ test('test for test repo in the list', done => {
+ flush(() => {
+ assert.equal(element._documentationSearches[0].title,
+ 'Gerrit Code Review - REST API Developers Notes1');
+ assert.equal(element._documentationSearches[0].url,
+ 'Documentation/dev-rest-api.html');
+ done();
+ });
+ });
+ });
+
+ suite('filter', () => {
+ setup(() => {
+ documentationSearches = _.times(25, documentationGenerator);
+ documentationSearchesFiltered = _.times(1, documentationSearches);
+ });
+
+ test('_paramsChanged', done => {
+ sandbox.stub(element.$.restAPI, 'getDocumentationSearches', () => {
+ return Promise.resolve(documentationSearches);
+ });
+ const value = {
+ filter: 'test',
+ };
+ element._paramsChanged(value).then(() => {
+ assert.isTrue(element.$.restAPI.getDocumentationSearches.lastCall
+ .calledWithExactly('test'));
+ done();
+ });
+ });
+ });
+
+ suite('loading', () => {
+ test('correct contents are displayed', () => {
+ assert.isTrue(element._loading);
+ assert.equal(element.computeLoadingClass(element._loading), 'loading');
+ assert.equal(getComputedStyle(element.$.loading).display, 'block');
+
+ element._loading = false;
+ element._repos = _.times(25, documentationGenerator);
+
+ flushAsynchronousOperations();
+ assert.equal(element.computeLoadingClass(element._loading), '');
+ assert.equal(getComputedStyle(element.$.loading).display, 'none');
+ });
+ });
+ });
+</script>
diff --git a/polygerrit-ui/app/elements/gr-app.html b/polygerrit-ui/app/elements/gr-app.html
index 787a1c6..b34cd0a 100644
--- a/polygerrit-ui/app/elements/gr-app.html
+++ b/polygerrit-ui/app/elements/gr-app.html
@@ -48,6 +48,7 @@
<link rel="import" href="../styles/shared-styles.html">
<link rel="import" href="../styles/themes/app-theme.html">
<link rel="import" href="./admin/gr-admin-view/gr-admin-view.html">
+<link rel="import" href="./documentation/gr-documentation-search/gr-documentation-search.html">
<link rel="import" href="./change-list/gr-change-list-view/gr-change-list-view.html">
<link rel="import" href="./change-list/gr-dashboard-view/gr-dashboard-view.html">
<link rel="import" href="./change/gr-change-view/gr-change-view.html">
@@ -198,6 +199,11 @@
<template is="dom-if" if="[[_showCLAView]]" restamp="true">
<gr-cla-view></gr-cla-view>
</template>
+ <template is="dom-if" if="[[_showDocumentationSearch]]" restamp="true">
+ <gr-documentation-search
+ params="[[params]]">
+ </gr-documentation-search>
+ </template>
<div id="errorView" class="errorView">
<div class="errorEmoji">[[_lastError.emoji]]</div>
<div class="errorText">[[_lastError.text]]</div>
diff --git a/polygerrit-ui/app/elements/gr-app.js b/polygerrit-ui/app/elements/gr-app.js
index b0cc514..321dc58 100644
--- a/polygerrit-ui/app/elements/gr-app.js
+++ b/polygerrit-ui/app/elements/gr-app.js
@@ -72,6 +72,7 @@
_showCLAView: Boolean,
_showEditorView: Boolean,
_showPluginScreen: Boolean,
+ _showDocumentationSearch: Boolean,
/** @type {?} */
_viewState: Object,
/** @type {?} */
@@ -202,6 +203,8 @@
this.Shortcut.TOGGLE_CHANGE_STAR, 's');
this.bindShortcut(
this.Shortcut.REFRESH_CHANGE_LIST, 'shift+r');
+ this.bindShortcut(
+ this.Shortcut.EDIT_TOPIC, 't');
this.bindShortcut(
this.Shortcut.OPEN_REPLY_DIALOG, 'a');
@@ -272,6 +275,8 @@
this.bindShortcut(
this.Shortcut.TOGGLE_FILE_REVIEWED, 'r');
this.bindShortcut(
+ this.Shortcut.NEXT_UNREVIEWED_FILE, 'shift+m');
+ this.bindShortcut(
this.Shortcut.TOGGLE_ALL_INLINE_DIFFS, 'shift+i:keyup');
this.bindShortcut(
this.Shortcut.TOGGLE_INLINE_DIFF, 'i:keyup');
@@ -315,6 +320,8 @@
if (isPluginScreen) {
this.async(() => this.set('_showPluginScreen', true), 1);
}
+ this.set('_showDocumentationSearch',
+ view === Gerrit.Nav.View.DOCUMENTATION_SEARCH);
if (this.params.justRegistered) {
this.$.registrationOverlay.open();
this.$.registrationDialog.loadData().then(() => {
diff --git a/polygerrit-ui/app/elements/settings/gr-edit-preferences/gr-edit-preferences.html b/polygerrit-ui/app/elements/settings/gr-edit-preferences/gr-edit-preferences.html
index 782100e..b3e6990 100644
--- a/polygerrit-ui/app/elements/settings/gr-edit-preferences/gr-edit-preferences.html
+++ b/polygerrit-ui/app/elements/settings/gr-edit-preferences/gr-edit-preferences.html
@@ -35,6 +35,7 @@
prevent-invalid-input
allowed-pattern="[0-9]"
bind-value="{{editPrefs.tab_size}}"
+ on-keypress="_handleEditPrefsChanged"
on-change="_handleEditPrefsChanged">
</span>
</section>
@@ -47,6 +48,7 @@
prevent-invalid-input
allowed-pattern="[0-9]"
bind-value="{{editPrefs.line_length}}"
+ on-keypress="_handleEditPrefsChanged"
on-change="_handleEditPrefsChanged">
</span>
</section>
@@ -59,6 +61,7 @@
prevent-invalid-input
allowed-pattern="[0-9]"
bind-value="{{editPrefs.indent_unit}}"
+ on-keypress="_handleEditPrefsChanged"
on-change="_handleEditPrefsChanged">
</span>
</section>
diff --git a/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown.js b/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown.js
index dcb428f..3d9d36b 100644
--- a/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown.js
+++ b/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown.js
@@ -229,7 +229,7 @@
if (typeof link.url === 'undefined') {
return '';
}
- if (link.target) {
+ if (link.target || !link.url.startsWith('/')) {
return link.url;
}
return this._computeRelativeURL(link.url);
diff --git a/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown_test.html b/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown_test.html
index 456f235..7bb4dce 100644
--- a/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown_test.html
@@ -73,6 +73,12 @@
test('link URLs', () => {
assert.equal(
+ element._computeLinkURL({url: 'http://example.com/test'}),
+ 'http://example.com/test');
+ assert.equal(
+ element._computeLinkURL({url: 'https://example.com/test'}),
+ 'https://example.com/test');
+ assert.equal(
element._computeLinkURL({url: '/test'}),
'//' + window.location.host + '/test');
assert.equal(
diff --git a/polygerrit-ui/app/elements/shared/gr-editable-label/gr-editable-label.js b/polygerrit-ui/app/elements/shared/gr-editable-label/gr-editable-label.js
index b7d65d3..3514492 100644
--- a/polygerrit-ui/app/elements/shared/gr-editable-label/gr-editable-label.js
+++ b/polygerrit-ui/app/elements/shared/gr-editable-label/gr-editable-label.js
@@ -99,6 +99,12 @@
});
},
+ open() {
+ return this._open().then(() => {
+ this.$.input.$.input.focus();
+ });
+ },
+
_open(...args) {
this.$.dropdown.open();
this._inputText = this.value;
diff --git a/polygerrit-ui/app/elements/shared/gr-icons/gr-icons.html b/polygerrit-ui/app/elements/shared/gr-icons/gr-icons.html
index 4f80475..4ea8cc7 100644
--- a/polygerrit-ui/app/elements/shared/gr-icons/gr-icons.html
+++ b/polygerrit-ui/app/elements/shared/gr-icons/gr-icons.html
@@ -48,6 +48,10 @@
<g id="publishEdit"><path d="M5 4v2h14V4H5zm0 10h4v6h6v-6h4l-7-7-7 7z"/></g>
<!-- This SVG is a copy from iron-icons https://github.com/PolymerElements/iron-icons/blob/master/editor-icons.html -->
<g id="delete"><path d="M6 19c0 1.1.9 2 2 2h8c1.1 0 2-.9 2-2V7H6v12zM19 4h-3.5l-1-1h-5l-1 1H5v2h14V4z"/></g>
+ <!-- This SVG is a copy from iron-icons https://github.com/PolymerElements/iron-icons/blob/master/iron-icons.js -->
+ <g id="help"><path d="M12 2C6.48 2 2 6.48 2 12s4.48 10 10 10 10-4.48 10-10S17.52 2 12 2zm1 17h-2v-2h2v2zm2.07-7.75l-.9.92C13.45 12.9 13 13.5 13 15h-2v-.5c0-1.1.45-2.1 1.17-2.83l1.24-1.26c.37-.36.59-.86.59-1.41 0-1.1-.9-2-2-2s-2 .9-2 2H8c0-2.21 1.79-4 4-4s4 1.79 4 4c0 .88-.36 1.68-.93 2.25z"></path></g>
+ <!-- This SVG is a copy from iron-icons https://github.com/PolymerElements/iron-icons/blob/master/iron-icons.js -->
+ <g id="info"><path d="M12 2C6.48 2 2 6.48 2 12s4.48 10 10 10 10-4.48 10-10S17.52 2 12 2zm1 15h-2v-6h2v6zm0-8h-2V7h2v2z"></path></g>
<!-- This SVG is a copy from material.io https://material.io/icons/#ic_hourglass_full-->
<g id="hourglass"><path d="M6 2v6h.01L6 8.01 10 12l-4 4 .01.01H6V22h12v-5.99h-.01L18 16l-4-4 4-3.99-.01-.01H18V2H6z"/><path d="M0 0h24v24H0V0z" fill="none"/></g>
<!-- This is a custom PolyGerrit SVG -->
diff --git a/polygerrit-ui/app/elements/shared/gr-label-info/gr-label-info.js b/polygerrit-ui/app/elements/shared/gr-label-info/gr-label-info.js
index 2fe5f7b1..40edc28 100644
--- a/polygerrit-ui/app/elements/shared/gr-label-info/gr-label-info.js
+++ b/polygerrit-ui/app/elements/shared/gr-label-info/gr-label-info.js
@@ -127,27 +127,8 @@
this.$.restAPI.deleteVote(this.change._number, accountID, this.label)
.then(response => {
target.disabled = false;
- if (!response.ok) { return response; }
-
- const label = this.change.labels[this.label];
- const labels = label.all || [];
- let wasChanged = false;
- for (let i = 0; i < labels.length; i++) {
- if (labels[i]._account_id === accountID) {
- for (const key in label) {
- if (label.hasOwnProperty(key) &&
- label[key]._account_id === accountID) {
- // Remove special label field, keeping change label values
- // in sync with the backend.
- this.change.labels[this.label][key] = null;
- }
- }
- this.change.labels[this.label].all.splice(i, 1);
- wasChanged = true;
- break;
- }
- }
- if (wasChanged) { this.notifySplices('change.labels'); }
+ if (!response.ok) { return; }
+ Gerrit.Nav.navigateToChange(this.change);
}).catch(err => {
target.disabled = false;
return;
diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
index be8b763..d9b0cbf 100644
--- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
+++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
@@ -1321,21 +1321,27 @@
* @param {function()=} opt_cancelCondition
*/
getChangeDetail(changeNum, opt_errFn, opt_cancelCondition) {
- const options = this.listChangesOptionsToHex(
- this.ListChangesOption.ALL_COMMITS,
- this.ListChangesOption.ALL_REVISIONS,
- this.ListChangesOption.CHANGE_ACTIONS,
- this.ListChangesOption.CURRENT_ACTIONS,
- this.ListChangesOption.DETAILED_LABELS,
- this.ListChangesOption.DOWNLOAD_COMMANDS,
- this.ListChangesOption.MESSAGES,
- this.ListChangesOption.SUBMITTABLE,
- this.ListChangesOption.WEB_LINKS,
- this.ListChangesOption.SKIP_MERGEABLE
- );
- return this._getChangeDetail(
- changeNum, options, opt_errFn, opt_cancelCondition)
- .then(GrReviewerUpdatesParser.parse);
+ const options = [
+ this.ListChangesOption.ALL_COMMITS,
+ this.ListChangesOption.ALL_REVISIONS,
+ this.ListChangesOption.CHANGE_ACTIONS,
+ this.ListChangesOption.CURRENT_ACTIONS,
+ this.ListChangesOption.DETAILED_LABELS,
+ this.ListChangesOption.DOWNLOAD_COMMANDS,
+ this.ListChangesOption.MESSAGES,
+ this.ListChangesOption.SUBMITTABLE,
+ this.ListChangesOption.WEB_LINKS,
+ this.ListChangesOption.SKIP_MERGEABLE,
+ ];
+ return this.getConfig(false).then(config => {
+ if (config.receive && config.receive.enable_signed_push) {
+ options.push(this.ListChangesOption.PUSH_CERTIFICATES);
+ }
+ const optionsHex = this.listChangesOptionsToHex(...options);
+ return this._getChangeDetail(
+ changeNum, optionsHex, opt_errFn, opt_cancelCondition)
+ .then(GrReviewerUpdatesParser.parse);
+ });
},
/**
@@ -3035,6 +3041,22 @@
});
},
+ /**
+ * @param {string} filter
+ * @return {!Promise<?Object>}
+ */
+ getDocumentationSearches(filter) {
+ filter = filter.trim();
+ const encodedFilter = encodeURIComponent(filter);
+
+ // TODO(kaspern): Rename rest api from /projects/ to /repos/ once backend
+ // supports it.
+ return this._fetchSharedCacheURL({
+ url: `/Documentation/?q=${encodedFilter}`,
+ anonymizedUrl: '/Documentation/?*',
+ });
+ },
+
getMergeable(changeNum) {
return this._getChangeURLAndFetch({
changeNum,
diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.html b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.html
index b2bf42b..667f24c 100644
--- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.html
@@ -734,15 +734,6 @@
assert.deepEqual(element._send.lastCall.args[0].body, {token: 'foo'});
});
- test('GrReviewerUpdatesParser.parse is used', () => {
- sandbox.stub(GrReviewerUpdatesParser, 'parse').returns(
- Promise.resolve('foo'));
- return element.getChangeDetail(42).then(result => {
- assert.isTrue(GrReviewerUpdatesParser.parse.calledOnce);
- assert.equal(result, 'foo');
- });
- });
-
test('setAccountStatus', () => {
sandbox.stub(element, '_send').returns(Promise.resolve('OOO'));
element._cache.set('/accounts/self/detail', {});
@@ -1114,7 +1105,49 @@
});
});
- suite('_getChangeDetail', () => {
+ suite('getChangeDetail', () => {
+ suite('change detail options', () => {
+ let toHexStub;
+
+ setup(() => {
+ toHexStub = sandbox.stub(element, 'listChangesOptionsToHex',
+ options => 'deadbeef');
+ sandbox.stub(element, '_getChangeDetail',
+ async (changeNum, options) => ({changeNum, options}));
+ });
+
+ test('signed pushes disabled', async () => {
+ const {PUSH_CERTIFICATES} = element.ListChangesOption;
+ sandbox.stub(element, 'getConfig', async () => ({}));
+ const {changeNum, options} = await element.getChangeDetail(123);
+ assert.strictEqual(123, changeNum);
+ assert.strictEqual('deadbeef', options);
+ assert.isTrue(toHexStub.calledOnce);
+ assert.isFalse(toHexStub.lastCall.args.includes(PUSH_CERTIFICATES));
+ });
+
+ test('signed pushes enabled', async () => {
+ const {PUSH_CERTIFICATES} = element.ListChangesOption;
+ sandbox.stub(element, 'getConfig', async () => {
+ return {receive: {enable_signed_push: true}};
+ });
+ const {changeNum, options} = await element.getChangeDetail(123);
+ assert.strictEqual(123, changeNum);
+ assert.strictEqual('deadbeef', options);
+ assert.isTrue(toHexStub.calledOnce);
+ assert.isTrue(toHexStub.lastCall.args.includes(PUSH_CERTIFICATES));
+ });
+ });
+
+ test('GrReviewerUpdatesParser.parse is used', () => {
+ sandbox.stub(GrReviewerUpdatesParser, 'parse').returns(
+ Promise.resolve('foo'));
+ return element.getChangeDetail(42).then(result => {
+ assert.isTrue(GrReviewerUpdatesParser.parse.calledOnce);
+ assert.equal(result, 'foo');
+ });
+ });
+
test('_getChangeDetail passes params to ETags decorator', () => {
const changeNum = 4321;
element._projectLookup[changeNum] = 'test';
diff --git a/polygerrit-ui/app/test/index.html b/polygerrit-ui/app/test/index.html
index 5b9ae15..20a4a1e 100644
--- a/polygerrit-ui/app/test/index.html
+++ b/polygerrit-ui/app/test/index.html
@@ -103,7 +103,6 @@
'core/gr-smart-search/gr-smart-search_test.html',
'diff/gr-comment-api/gr-comment-api_test.html',
'diff/gr-diff-builder/gr-diff-builder_test.html',
- 'diff/gr-diff-comment-thread-group/gr-diff-comment-thread-group_test.html',
'diff/gr-diff-comment-thread/gr-diff-comment-thread_test.html',
'diff/gr-diff-comment/gr-diff-comment_test.html',
'diff/gr-diff-cursor/gr-diff-cursor_test.html',
@@ -120,6 +119,7 @@
'diff/gr-ranged-comment-layer/gr-ranged-comment-layer_test.html',
'diff/gr-selection-action-box/gr-selection-action-box_test.html',
'diff/gr-syntax-layer/gr-syntax-layer_test.html',
+ 'documentation/gr-documentation-search/gr-documentation-search_test.html',
'edit/gr-default-editor/gr-default-editor_test.html',
'edit/gr-edit-controls/gr-edit-controls_test.html',
'edit/gr-edit-file-controls/gr-edit-file-controls_test.html',
diff --git a/tools/bzl/junit.bzl b/tools/bzl/junit.bzl
index 08d5045..ccde467 100644
--- a/tools/bzl/junit.bzl
+++ b/tools/bzl/junit.bzl
@@ -68,7 +68,6 @@
# Enforce JDK 8 compatibility on Java 9, see
# https://docs.oracle.com/javase/9/intl/internationalization-enhancements-jdk-9.htm#JSINT-GUID-AF5AECA7-07C1-4E7D-BC10-BC7E73DC6C7F
"-Djava.locale.providers=COMPAT,CLDR,SPI",
- "--add-modules java.activation",
"--add-opens=jdk.management/com.sun.management.internal=ALL-UNNAMED",
]
@@ -82,7 +81,7 @@
jvm_flags = kwargs.get("jvm_flags", [])
jvm_flags = jvm_flags + select({
"//:java9": POST_JDK8_OPTS,
- "//:java10": POST_JDK8_OPTS,
+ "//:java_next": POST_JDK8_OPTS,
"//conditions:default": [],
})
native.java_test(
