Merge "Change the default permissions from READ on refs/* to refs/heads/*"
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index ac5e3b7..645954d 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -2336,8 +2336,8 @@
[[gerrit.experimentalRollingUpgrade]]gerrit.experimentalRollingUpgrade::
+
Enable Gerrit rolling upgrade to the next version.
-For example if Gerrit v3.1 is version N (All-Projects:refs/meta/version=181)
-then its next version N+1 is v3.2 (All-Projects:refs/meta/version=183).
+For example if Gerrit v3.2 is version N (All-Projects:refs/meta/version=183)
+then its next version N+1 is v3.3 (All-Projects:refs/meta/version=184).
Allow Gerrit to start even if the underlying schema version has been bumped to
the next Gerrit version.
+
@@ -2354,7 +2354,7 @@
1. Set gerrit.experimentalRollingUpgrade to true on all Gerrit masters
2. Set the first master unhealthy
3. Shutdown the first master and [upgrade](install.html#init) to the next version
-4. Startup the first master, wait for the online reindex to complete
+4. Startup the first master, wait for the online reindex to complete (where applicable)
5. Verify the the first master upgrade is successful and online reindex is complete
6. Set the first master healthy
7. Repeat steps 2. to 6. for all the other Gerrit nodes
@@ -5625,10 +5625,12 @@
[[receive.autogc]]receive.autogc::
+
-By default, `git-receive-pack` will run auto gc after receiving data from git-push and updating refs.
+By default, up to Gerrit 3.2 `git-receive-pack` will run auto gc after receiving data from git-push and updating refs.
You can stop it by setting this variable to `false`. This is recommended in gerrit to avoid the
additional load this creates. Instead schedule gc using link:cmd-gc.html#gc.startTime[gc.startTime]
and link:cmd-gc.html#gc.interval[gc.interval] or e.g. in a cron job that runs gc in a separate process.
+Since Gerrit 3.3 the init command will auto-configure `git-receive-pack = false` in `etc/jgit.config` if
+it wasn't set manually and show a warning if it was set to `true` manually.
GERRIT
------
diff --git a/Documentation/dev-plugins.txt b/Documentation/dev-plugins.txt
index 2f4c46c..c770733 100644
--- a/Documentation/dev-plugins.txt
+++ b/Documentation/dev-plugins.txt
@@ -1453,6 +1453,19 @@
[...]
----
+[post_review_extensions]
+== Post Review Extensions
+
+By implementing the `com.google.gerrit.server.restapi.change.OnPostReview`
+interface plugins can extend the change message that is being posted when the
+[post review](rest-api-changes.html#set-review) REST endpoint is invoked.
+
+This is useful if certain approvals have a special meaning (e.g. custom logic
+that is implemented in Prolog submit rules, signal for triggering an action
+like running CI etc.), as it allows the plugin to tell users about this meaning
+in the change message. This makes the effect of a given approval more
+transparent to the user.
+
[[ui_extension]]
== UI Extension
diff --git a/Documentation/dev-processes.txt b/Documentation/dev-processes.txt
index 0eb3972..68e56ba 100644
--- a/Documentation/dev-processes.txt
+++ b/Documentation/dev-processes.txt
@@ -271,6 +271,15 @@
It's also possible that the ESC decides that an issue is not a security issue
and the embargo is lifted immediately.
+. Filing a CVE
++
+For every security issue a CVE that describes the issue and lists the affected
+releases should be filed. Filing a CVE can be done by any maintainer that works
+for an organization that can request CVE numbers (e.g. Googlers). The CVE
+number must be included in the release notes. The CVE itself is only made
+public after fixed released have been published and the embargo has been
+lifted.
+
. Implementation of the security fix:
+
To keep the embargo intact, security fixes cannot be developed and reviewed in
@@ -316,6 +325,8 @@
This ends the embargo and any issue that discusses the security vulnerability
should be made public.
+. Publish the CVE
+
. Follow-Up
+
The ESC should discuss if there are any learnings from the security
diff --git a/Documentation/rest-api-projects.txt b/Documentation/rest-api-projects.txt
index 6889de3..50ebad7 100644
--- a/Documentation/rest-api-projects.txt
+++ b/Documentation/rest-api-projects.txt
@@ -1702,7 +1702,9 @@
'GET /projects/link:#project-name[\{project-name\}]/branches/link:#branch-id[\{branch-id\}]'
--
-Retrieves a branch of a project.
+Retrieves a branch of a project. For the "All-Users" repository, the magic
+branch "refs/users/self" is automatically resolved to the user branch of the
+calling user.
.Request
----
diff --git a/WORKSPACE b/WORKSPACE
index 01decd5..cb1d22c 100644
--- a/WORKSPACE
+++ b/WORKSPACE
@@ -899,48 +899,48 @@
sha1 = "7e060dd5b19431e6d198e91ff670644372f60fbd",
)
-JETTY_VERS = "9.4.32.v20200930"
+JETTY_VERS = "9.4.33.v20201020"
maven_jar(
name = "jetty-servlet",
artifact = "org.eclipse.jetty:jetty-servlet:" + JETTY_VERS,
- sha1 = "4253dd46c099e0bca4dd763fc1e10774e10de00a",
+ sha1 = "101609e8e5365c4406e4448099459eb605ac551f",
)
maven_jar(
name = "jetty-security",
artifact = "org.eclipse.jetty:jetty-security:" + JETTY_VERS,
- sha1 = "16a6110fa40e49050146de5f597ab3a3a3fa83b5",
+ sha1 = "c150bf2aca6cb1636e7195f844a2bb156546e50e",
)
maven_jar(
name = "jetty-server",
artifact = "org.eclipse.jetty:jetty-server:" + JETTY_VERS,
- sha1 = "d2d89099be5237cf68254bc943a7d800d3ee1945",
+ sha1 = "f586ff2ee048ad2575866c1833d854288f402307",
)
maven_jar(
name = "jetty-jmx",
artifact = "org.eclipse.jetty:jetty-jmx:" + JETTY_VERS,
- sha1 = "5e8e87a6f89b8eabf5b5b1765e3d758209001570",
+ sha1 = "56b723070eeafc51b943cd9bf1a064a037e806a7",
)
maven_jar(
name = "jetty-http",
artifact = "org.eclipse.jetty:jetty-http:" + JETTY_VERS,
- sha1 = "5fdcefd82178d11f895690f4fe6e843be69394b3",
+ sha1 = "ad28940f89ffde6ec1bd1656fe3f8493b01ba3c2",
)
maven_jar(
name = "jetty-io",
artifact = "org.eclipse.jetty:jetty-io:" + JETTY_VERS,
- sha1 = "0d0f32c3b511d6b3a542787f95ed229731588810",
+ sha1 = "9e4b0048285b71f4769908780f957a470eca11da",
)
maven_jar(
name = "jetty-util",
artifact = "org.eclipse.jetty:jetty-util:" + JETTY_VERS,
- sha1 = "efefd29006dcc9c9960a679263504287ce4e6896",
+ sha1 = "c88807f210ab216aa831b48569ef50bd797384bc",
)
maven_jar(
diff --git a/java/com/google/gerrit/acceptance/AbstractPluginLogFileTest.java b/java/com/google/gerrit/acceptance/AbstractPluginLogFileTest.java
new file mode 100644
index 0000000..60def29
--- /dev/null
+++ b/java/com/google/gerrit/acceptance/AbstractPluginLogFileTest.java
@@ -0,0 +1,112 @@
+// Copyright (C) 2020 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.acceptance;
+
+import com.google.gerrit.extensions.annotations.Exports;
+import com.google.gerrit.extensions.events.LifecycleListener;
+import com.google.gerrit.extensions.systemstatus.ServerInformation;
+import com.google.gerrit.server.DynamicOptions;
+import com.google.gerrit.server.config.SitePaths;
+import com.google.gerrit.server.util.PluginLogFile;
+import com.google.gerrit.server.util.SystemLog;
+import com.google.gerrit.sshd.commands.Query;
+import com.google.inject.AbstractModule;
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
+import com.google.inject.internal.UniqueAnnotations;
+import java.util.Collections;
+import org.apache.log4j.AsyncAppender;
+import org.apache.log4j.Layout;
+import org.apache.log4j.PatternLayout;
+import org.eclipse.jgit.lib.Config;
+import org.kohsuke.args4j.Option;
+
+public class AbstractPluginLogFileTest extends AbstractDaemonTest {
+ protected static class Module extends AbstractModule {
+ @Override
+ public void configure() {
+ bind(com.google.gerrit.server.DynamicOptions.DynamicBean.class)
+ .annotatedWith(Exports.named(Query.class))
+ .to(MyClassNameProvider.class);
+ }
+ }
+
+ protected static class MyClassNameProvider implements DynamicOptions.ModulesClassNamesProvider {
+ @Override
+ public String getClassName() {
+ return "com.google.gerrit.acceptance.AbstractPluginLogFileTest$MyOptions";
+ }
+
+ @Override
+ public Iterable<String> getModulesClassNames() {
+ return Collections.singleton(
+ "com.google.gerrit.acceptance.AbstractPluginLogFileTest$MyOptions$MyOptionsModule");
+ }
+ }
+
+ public static class MyOptions implements DynamicOptions.DynamicBean {
+ @Option(name = "--opt")
+ public boolean opt;
+
+ public static class MyOptionsModule extends AbstractModule {
+ @Override
+ protected void configure() {
+ bind(LifecycleListener.class)
+ .annotatedWith(UniqueAnnotations.create())
+ .to(MyPluginLogFile.class);
+ }
+ }
+ }
+
+ protected static class MyPluginLogFile extends PluginLogFile {
+ protected static final String logName = "test_log";
+
+ @Inject
+ public MyPluginLogFile(MySystemLog mySystemLog, ServerInformation serverInfo) {
+ super(mySystemLog, serverInfo, logName, new PatternLayout("[%d] [%t] %m%n"));
+ }
+ }
+
+ @Singleton
+ protected static class MySystemLog extends SystemLog {
+ protected InvocationCounter invocationCounter;
+
+ @Inject
+ public MySystemLog(SitePaths site, Config config, InvocationCounter invocationCounter) {
+ super(site, config);
+ this.invocationCounter = invocationCounter;
+ }
+
+ @Override
+ public AsyncAppender createAsyncAppender(
+ String name, Layout layout, boolean rotate, boolean forPlugin) {
+ invocationCounter.increment();
+ return super.createAsyncAppender(name, layout, rotate, forPlugin);
+ }
+ }
+
+ @Singleton
+ public static class InvocationCounter {
+ private int counter = 0;
+
+ public int getCounter() {
+ return counter;
+ }
+
+ public synchronized void increment() {
+ counter++;
+ }
+ }
+}
diff --git a/java/com/google/gerrit/acceptance/BUILD b/java/com/google/gerrit/acceptance/BUILD
index db0dc84..28f67b8 100644
--- a/java/com/google/gerrit/acceptance/BUILD
+++ b/java/com/google/gerrit/acceptance/BUILD
@@ -47,6 +47,7 @@
"//lib/guice",
"//lib/guice:guice-assistedinject",
"//lib/guice:guice-servlet",
+ "//lib/log:log4j",
"//lib/mail",
"//lib/mina:sshd",
"//lib:guava",
diff --git a/java/com/google/gerrit/acceptance/ExtensionRegistry.java b/java/com/google/gerrit/acceptance/ExtensionRegistry.java
index 03644a6..5d01dcb 100644
--- a/java/com/google/gerrit/acceptance/ExtensionRegistry.java
+++ b/java/com/google/gerrit/acceptance/ExtensionRegistry.java
@@ -43,6 +43,7 @@
import com.google.gerrit.server.git.validators.OnSubmitValidationListener;
import com.google.gerrit.server.git.validators.RefOperationValidationListener;
import com.google.gerrit.server.logging.PerformanceLogger;
+import com.google.gerrit.server.restapi.change.OnPostReview;
import com.google.gerrit.server.rules.SubmitRule;
import com.google.gerrit.server.validators.AccountActivationValidationListener;
import com.google.gerrit.server.validators.ProjectCreationValidationListener;
@@ -83,6 +84,7 @@
private final DynamicMap<CapabilityDefinition> capabilityDefinitions;
private final DynamicMap<PluginProjectPermissionDefinition> pluginProjectPermissionDefinitions;
private final DynamicMap<ProjectConfigEntry> pluginConfigEntries;
+ private final DynamicSet<OnPostReview> onPostReviews;
@Inject
ExtensionRegistry(
@@ -113,7 +115,8 @@
DynamicSet<WorkInProgressStateChangedListener> workInProgressStateChangedListeners,
DynamicMap<CapabilityDefinition> capabilityDefinitions,
DynamicMap<PluginProjectPermissionDefinition> pluginProjectPermissionDefinitions,
- DynamicMap<ProjectConfigEntry> pluginConfigEntries) {
+ DynamicMap<ProjectConfigEntry> pluginConfigEntries,
+ DynamicSet<OnPostReview> onPostReviews) {
this.accountIndexedListeners = accountIndexedListeners;
this.changeIndexedListeners = changeIndexedListeners;
this.groupIndexedListeners = groupIndexedListeners;
@@ -142,6 +145,7 @@
this.capabilityDefinitions = capabilityDefinitions;
this.pluginProjectPermissionDefinitions = pluginProjectPermissionDefinitions;
this.pluginConfigEntries = pluginConfigEntries;
+ this.onPostReviews = onPostReviews;
}
public Registration newRegistration() {
@@ -270,6 +274,10 @@
return add(pluginConfigEntries, pluginConfigEntry, exportName);
}
+ public Registration add(OnPostReview onPostReview) {
+ return add(onPostReviews, onPostReview);
+ }
+
private <T> Registration add(DynamicSet<T> dynamicSet, T extension) {
return add(dynamicSet, extension, "gerrit");
}
diff --git a/java/com/google/gerrit/elasticsearch/ElasticQueryBuilder.java b/java/com/google/gerrit/elasticsearch/ElasticQueryBuilder.java
index d05e91c..40ac603 100644
--- a/java/com/google/gerrit/elasticsearch/ElasticQueryBuilder.java
+++ b/java/com/google/gerrit/elasticsearch/ElasticQueryBuilder.java
@@ -29,7 +29,6 @@
import com.google.gerrit.index.query.QueryParseException;
import com.google.gerrit.index.query.RegexPredicate;
import com.google.gerrit.index.query.TimestampRangePredicate;
-import com.google.gerrit.server.query.change.AfterPredicate;
import java.time.Instant;
public class ElasticQueryBuilder {
@@ -130,7 +129,9 @@
private <T> QueryBuilder timestampQuery(IndexPredicate<T> p) throws QueryParseException {
if (p instanceof TimestampRangePredicate) {
TimestampRangePredicate<T> r = (TimestampRangePredicate<T>) p;
- if (p instanceof AfterPredicate) {
+ if (r.getMaxTimestamp().getTime() == Long.MAX_VALUE) {
+ // The time range only has the start value, search from the start to the max supported value
+ // Long.MAX_VALUE
return QueryBuilders.rangeQuery(r.getField().getName())
.gte(Instant.ofEpochMilli(r.getMinTimestamp().getTime()));
}
diff --git a/java/com/google/gerrit/entities/RefNames.java b/java/com/google/gerrit/entities/RefNames.java
index e789aa2..f18c122 100644
--- a/java/com/google/gerrit/entities/RefNames.java
+++ b/java/com/google/gerrit/entities/RefNames.java
@@ -255,6 +255,10 @@
return ref.startsWith(REFS_USERS);
}
+ public static boolean isRefsUsersSelf(String ref, boolean isAllUsers) {
+ return isAllUsers && REFS_USERS_SELF.equals(ref);
+ }
+
/**
* Whether the ref is a group branch that stores NoteDb data of a group. Returns {@code true} for
* all refs that start with {@code refs/groups/}.
diff --git a/java/com/google/gerrit/httpd/restapi/RestApiServlet.java b/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
index 0e525ce..51e032a 100644
--- a/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
+++ b/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
@@ -1348,7 +1348,7 @@
TemporaryBuffer.Heap buf = heap(HEAP_EST_SIZE, Integer.MAX_VALUE);
buf.write(JSON_MAGIC);
Writer w = new BufferedWriter(new OutputStreamWriter(buf, UTF_8));
- Gson gson = newGson(config, req);
+ Gson gson = newGson(config);
if (result instanceof JsonElement) {
gson.toJson((JsonElement) result, w);
} else {
@@ -1375,25 +1375,18 @@
req, res, asBinaryResult(buf).setContentType(JSON_TYPE).setCharacterEncoding(UTF_8));
}
- private static Gson newGson(
- ListMultimap<String, String> config, @Nullable HttpServletRequest req) {
+ private static Gson newGson(ListMultimap<String, String> config) {
GsonBuilder gb = OutputFormat.JSON_COMPACT.newGsonBuilder();
- enablePrettyPrint(gb, config, req);
+ enablePrettyPrint(gb, config);
enablePartialGetFields(gb, config);
return gb.create();
}
- private static void enablePrettyPrint(
- GsonBuilder gb, ListMultimap<String, String> config, @Nullable HttpServletRequest req) {
- String pp = Iterables.getFirst(config.get("pp"), null);
- if (pp == null) {
- pp = Iterables.getFirst(config.get("prettyPrint"), null);
- if (pp == null && req != null) {
- pp = acceptsJson(req) ? "0" : "1";
- }
- }
+ private static void enablePrettyPrint(GsonBuilder gb, ListMultimap<String, String> config) {
+ String pp =
+ Iterables.getFirst(config.get("pp"), Iterables.getFirst(config.get("prettyPrint"), "0"));
if ("1".equals(pp) || "true".equals(pp)) {
gb.setPrettyPrinting();
}
@@ -1903,10 +1896,6 @@
return CharMatcher.anyOf("<&").matchesAnyOf(text);
}
- private static boolean acceptsJson(HttpServletRequest req) {
- return req != null && isType(JSON_TYPE, req.getHeader(HttpHeaders.ACCEPT));
- }
-
private static boolean acceptsGzip(HttpServletRequest req) {
if (req != null) {
String accepts = req.getHeader(HttpHeaders.ACCEPT_ENCODING);
diff --git a/java/com/google/gerrit/index/query/TimestampRangePredicate.java b/java/com/google/gerrit/index/query/TimestampRangePredicate.java
index 38b2b73..42f8aa8 100644
--- a/java/com/google/gerrit/index/query/TimestampRangePredicate.java
+++ b/java/com/google/gerrit/index/query/TimestampRangePredicate.java
@@ -34,6 +34,10 @@
super(def, name, value);
}
+ protected Timestamp getValueTimestamp(I object) {
+ return (Timestamp) this.getField().get(object);
+ }
+
public abstract Date getMinTimestamp();
public abstract Date getMaxTimestamp();
diff --git a/java/com/google/gerrit/pgm/init/InitJGitConfig.java b/java/com/google/gerrit/pgm/init/InitJGitConfig.java
new file mode 100644
index 0000000..6e37f7f
--- /dev/null
+++ b/java/com/google/gerrit/pgm/init/InitJGitConfig.java
@@ -0,0 +1,102 @@
+// Copyright (C) 2020 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.pgm.init;
+
+import static com.google.gerrit.pgm.init.api.InitUtil.die;
+
+import com.google.gerrit.pgm.init.api.ConsoleUI;
+import com.google.gerrit.pgm.init.api.InitStep;
+import com.google.gerrit.server.config.SitePaths;
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
+import java.io.IOException;
+import org.eclipse.jgit.errors.ConfigInvalidException;
+import org.eclipse.jgit.lib.ConfigConstants;
+import org.eclipse.jgit.storage.file.FileBasedConfig;
+import org.eclipse.jgit.transport.TransferConfig;
+import org.eclipse.jgit.util.FS;
+
+/** Initialize the JGit configuration. */
+@Singleton
+class InitJGitConfig implements InitStep {
+ private final ConsoleUI ui;
+ private final SitePaths sitePaths;
+
+ @Inject
+ InitJGitConfig(ConsoleUI ui, SitePaths sitePaths) {
+ this.ui = ui;
+ this.sitePaths = sitePaths;
+ }
+
+ @Override
+ public void run() {
+ ui.header("JGit Configuration");
+ FileBasedConfig jgitConfig = new FileBasedConfig(sitePaths.jgit_config.toFile(), FS.DETECTED);
+ try {
+ jgitConfig.load();
+ if (!jgitConfig
+ .getNames(ConfigConstants.CONFIG_RECEIVE_SECTION)
+ .contains(ConfigConstants.CONFIG_KEY_AUTOGC)) {
+ jgitConfig.setBoolean(
+ ConfigConstants.CONFIG_RECEIVE_SECTION, null, ConfigConstants.CONFIG_KEY_AUTOGC, false);
+ jgitConfig.save();
+ ui.error(
+ "Auto-configured \"receive.autogc = false\" to disable auto-gc after git-receive-pack.");
+ } else if (jgitConfig.getBoolean(
+ ConfigConstants.CONFIG_RECEIVE_SECTION, ConfigConstants.CONFIG_KEY_AUTOGC, true)) {
+ ui.error(
+ "WARNING: JGit option \"receive.autogc = true\". This is not recommended in Gerrit.\n"
+ + "git-receive-pack will run auto gc after receiving data from "
+ + "git-push and updating refs.\n"
+ + "Disable this behavior to avoid the additional load it creates: "
+ + "gc should be configured in gc config section or run as a separate process.");
+ }
+
+ if (!jgitConfig
+ .getNames(ConfigConstants.CONFIG_PROTOCOL_SECTION)
+ .contains(ConfigConstants.CONFIG_KEY_VERSION)) {
+ jgitConfig.setString(
+ ConfigConstants.CONFIG_PROTOCOL_SECTION,
+ null,
+ ConfigConstants.CONFIG_KEY_VERSION,
+ TransferConfig.ProtocolVersion.V2.version());
+ jgitConfig.save();
+ ui.error(
+ String.format(
+ "Auto-configured \"%s.%s = %s\" to activate git wire protocol version 2.",
+ ConfigConstants.CONFIG_PROTOCOL_SECTION,
+ ConfigConstants.CONFIG_KEY_VERSION,
+ TransferConfig.ProtocolVersion.V2.version()));
+ } else {
+ String version =
+ jgitConfig.getString(
+ ConfigConstants.CONFIG_PROTOCOL_SECTION, null, ConfigConstants.CONFIG_KEY_VERSION);
+ if (!TransferConfig.ProtocolVersion.V2.version().equals(version)) {
+ ui.error(
+ String.format(
+ "HINT: JGit option \"%s.%s = %s\". It's recommended to activate git\n"
+ + "wire protocol version 2 to improve git fetch performance.",
+ ConfigConstants.CONFIG_PROTOCOL_SECTION,
+ ConfigConstants.CONFIG_KEY_VERSION,
+ version));
+ }
+ }
+ } catch (IOException e) {
+ throw die(String.format("Handling JGit configuration %s failed", sitePaths.jgit_config), e);
+ } catch (ConfigInvalidException e) {
+ throw die(String.format("Invalid JGit configuration %s", sitePaths.jgit_config), e);
+ }
+ }
+}
diff --git a/java/com/google/gerrit/pgm/init/InitModule.java b/java/com/google/gerrit/pgm/init/InitModule.java
index b658675..32c6697 100644
--- a/java/com/google/gerrit/pgm/init/InitModule.java
+++ b/java/com/google/gerrit/pgm/init/InitModule.java
@@ -40,6 +40,7 @@
// Steps are executed in the order listed here.
//
step().to(InitGitManager.class);
+ step().to(InitJGitConfig.class);
step().to(InitLogging.class);
step().to(InitIndex.class);
step().to(InitAuth.class);
diff --git a/java/com/google/gerrit/server/api/projects/BranchApiImpl.java b/java/com/google/gerrit/server/api/projects/BranchApiImpl.java
index c7cca6f..78f5c5f 100644
--- a/java/com/google/gerrit/server/api/projects/BranchApiImpl.java
+++ b/java/com/google/gerrit/server/api/projects/BranchApiImpl.java
@@ -16,6 +16,7 @@
import static com.google.gerrit.server.api.ApiUtil.asRestApiException;
+import com.google.gerrit.entities.RefNames;
import com.google.gerrit.extensions.api.projects.BranchApi;
import com.google.gerrit.extensions.api.projects.BranchInfo;
import com.google.gerrit.extensions.api.projects.BranchInput;
@@ -126,6 +127,10 @@
private BranchResource resource()
throws RestApiException, IOException, PermissionBackendException {
- return branches.parse(project, IdString.fromDecoded(ref));
+ String refName = ref;
+ if (RefNames.isRefsUsersSelf(ref, project.getProjectState().isAllUsers())) {
+ refName = RefNames.refsUsers(project.getUser().getAccountId());
+ }
+ return branches.parse(project, IdString.fromDecoded(refName));
}
}
diff --git a/java/com/google/gerrit/server/cache/serialize/entities/StoredCommentLinkInfoSerializer.java b/java/com/google/gerrit/server/cache/serialize/entities/StoredCommentLinkInfoSerializer.java
index d7bd373..a7a84f7 100644
--- a/java/com/google/gerrit/server/cache/serialize/entities/StoredCommentLinkInfoSerializer.java
+++ b/java/com/google/gerrit/server/cache/serialize/entities/StoredCommentLinkInfoSerializer.java
@@ -19,6 +19,7 @@
import com.google.gerrit.entities.StoredCommentLinkInfo;
import com.google.gerrit.server.cache.proto.Cache;
+import java.util.Optional;
/** Helper to (de)serialize values for caches. */
public class StoredCommentLinkInfoSerializer {
@@ -38,7 +39,7 @@
.setMatch(nullToEmpty(autoValue.getMatch()))
.setLink(nullToEmpty(autoValue.getLink()))
.setHtml(nullToEmpty(autoValue.getHtml()))
- .setEnabled(autoValue.getEnabled())
+ .setEnabled(Optional.ofNullable(autoValue.getEnabled()).orElse(true))
.setOverrideOnly(autoValue.getOverrideOnly())
.build();
}
diff --git a/java/com/google/gerrit/server/comment/CommentContextKey.java b/java/com/google/gerrit/server/comment/CommentContextKey.java
index e4a927a..ccd50b7 100644
--- a/java/com/google/gerrit/server/comment/CommentContextKey.java
+++ b/java/com/google/gerrit/server/comment/CommentContextKey.java
@@ -6,11 +6,10 @@
import com.google.gerrit.proto.Protos;
import com.google.gerrit.server.cache.proto.Cache;
import com.google.gerrit.server.cache.serialize.CacheSerializer;
-import java.util.Collection;
/**
* An identifier of a comment that should be used to load the comment context using {@link
- * CommentContextCache#get(CommentContextKey)}, or {@link CommentContextCache#getAll(Collection)}.
+ * CommentContextCache#get(CommentContextKey)}, or {@link CommentContextCache#getAll(Iterable)}.
*
* <p>The {@link CommentContextCacheImpl} implementation uses this class as the cache key, while
* replacing the {@link #path()} field with the hashed path.
diff --git a/java/com/google/gerrit/server/config/GerritGlobalModule.java b/java/com/google/gerrit/server/config/GerritGlobalModule.java
index 6a25afd..b1b2711 100644
--- a/java/com/google/gerrit/server/config/GerritGlobalModule.java
+++ b/java/com/google/gerrit/server/config/GerritGlobalModule.java
@@ -180,6 +180,7 @@
import com.google.gerrit.server.query.change.ChangeQueryBuilder;
import com.google.gerrit.server.query.change.ConflictsCacheImpl;
import com.google.gerrit.server.quota.QuotaEnforcer;
+import com.google.gerrit.server.restapi.change.OnPostReview;
import com.google.gerrit.server.restapi.change.SuggestReviewers;
import com.google.gerrit.server.restapi.group.GroupModule;
import com.google.gerrit.server.rules.DefaultSubmitRule;
@@ -413,6 +414,7 @@
DynamicSet.setOf(binder(), ExceptionHook.class);
DynamicSet.bind(binder(), ExceptionHook.class).to(ExceptionHookImpl.class);
DynamicSet.setOf(binder(), MailSoyTemplateProvider.class);
+ DynamicSet.setOf(binder(), OnPostReview.class);
DynamicMap.mapOf(binder(), MailFilter.class);
bind(MailFilter.class).annotatedWith(Exports.named("ListMailFilter")).to(ListMailFilter.class);
diff --git a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
index fb8a9d3..fb89240 100644
--- a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
+++ b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
@@ -21,11 +21,12 @@
import static com.google.common.flogger.LazyArgs.lazy;
import static com.google.gerrit.entities.RefNames.REFS_CHANGES;
import static com.google.gerrit.entities.RefNames.isConfigRef;
+import static com.google.gerrit.entities.RefNames.isRefsUsersSelf;
import static com.google.gerrit.git.ObjectIds.abbreviateName;
import static com.google.gerrit.server.change.HashtagsUtil.cleanupHashtag;
import static com.google.gerrit.server.git.MultiProgressMonitor.UNKNOWN;
import static com.google.gerrit.server.git.receive.ReceiveConstants.COMMAND_REJECTION_MESSAGE_FOOTER;
-import static com.google.gerrit.server.git.receive.ReceiveConstants.ONLY_CHANGE_OWNER_OR_PROJECT_OWNER_CAN_MODIFY_WIP;
+import static com.google.gerrit.server.git.receive.ReceiveConstants.ONLY_USERS_WITH_TOGGLE_WIP_STATE_PERM_CAN_MODIFY_WIP;
import static com.google.gerrit.server.git.receive.ReceiveConstants.PUSH_OPTION_SKIP_VALIDATION;
import static com.google.gerrit.server.git.receive.ReceiveConstants.SAME_CHANGE_ID_IN_MULTIPLE_CHANGES;
import static com.google.gerrit.server.git.validators.CommitValidators.NEW_PATCHSET_PATTERN;
@@ -1080,7 +1081,7 @@
private ReceiveCommand wrapReceiveCommand(ReceiveCommand cmd, Task progress) {
String refname = cmd.getRefName();
- if (projectState.isAllUsers() && RefNames.REFS_USERS_SELF.equals(cmd.getRefName())) {
+ if (isRefsUsersSelf(cmd.getRefName(), projectState.isAllUsers())) {
refname = RefNames.refsUsers(user.getAccountId());
logger.atFine().log("Swapping out command for %s to %s", RefNames.REFS_USERS_SELF, refname);
}
@@ -2883,9 +2884,9 @@
if (!hasWriteConfigPermission) {
try {
- permissionBackend.user(user).check(GlobalPermission.ADMINISTRATE_SERVER);
+ permissions.change(notes).check(ChangePermission.TOGGLE_WORK_IN_PROGRESS_STATE);
} catch (AuthException e1) {
- reject(inputCommand, ONLY_CHANGE_OWNER_OR_PROJECT_OWNER_CAN_MODIFY_WIP);
+ reject(inputCommand, ONLY_USERS_WITH_TOGGLE_WIP_STATE_PERM_CAN_MODIFY_WIP);
}
}
}
diff --git a/java/com/google/gerrit/server/git/receive/ReceiveConstants.java b/java/com/google/gerrit/server/git/receive/ReceiveConstants.java
index 03a1b33..df1888b 100644
--- a/java/com/google/gerrit/server/git/receive/ReceiveConstants.java
+++ b/java/com/google/gerrit/server/git/receive/ReceiveConstants.java
@@ -20,8 +20,8 @@
public static final String PUSH_OPTION_SKIP_VALIDATION = "skip-validation";
@VisibleForTesting
- public static final String ONLY_CHANGE_OWNER_OR_PROJECT_OWNER_CAN_MODIFY_WIP =
- "only change owner or project owner can modify Work-in-Progress";
+ public static final String ONLY_USERS_WITH_TOGGLE_WIP_STATE_PERM_CAN_MODIFY_WIP =
+ "only users with Toogle-Wip-State permission can modify Work-in-Progress";
static final String COMMAND_REJECTION_MESSAGE_FOOTER =
"Contact an administrator to fix the permissions";
diff --git a/java/com/google/gerrit/server/notedb/ChangeNotes.java b/java/com/google/gerrit/server/notedb/ChangeNotes.java
index b816264..e8f1fe1 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNotes.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNotes.java
@@ -138,8 +138,9 @@
}
/**
- * Create change notes based on a {@link Change.Id}. This requires using the Change index and
- * should only be used when {@link Project.NameKey} and the numeric change ID are not available.
+ * Create change notes based on a {@link com.google.gerrit.entities.Change.Id}. This requires
+ * using the Change index and should only be used when {@link
+ * com.google.gerrit.entities.Project.NameKey} and the numeric change ID are not available.
*/
public ChangeNotes createCheckedUsingIndexLookup(Change.Id changeId) {
InternalChangeQuery query = queryProvider.get().noFields();
@@ -155,9 +156,9 @@
}
/**
- * Create change notes based on a list of {@link Change.Id}s. This requires using the Change
- * index and should only be used when {@link Project.NameKey} and the numeric change ID are not
- * available.
+ * Create change notes based on a list of {@link com.google.gerrit.entities.Change.Id}s. This
+ * requires using the Change index and should only be used when {@link
+ * com.google.gerrit.entities.Project.NameKey} and the numeric change ID are not available.
*/
public List<ChangeNotes> createUsingIndexLookup(Collection<Change.Id> changeIds) {
List<ChangeNotes> notes = new ArrayList<>();
diff --git a/java/com/google/gerrit/server/patch/gitfilediff/FileHeaderUtil.java b/java/com/google/gerrit/server/patch/gitfilediff/FileHeaderUtil.java
index 9827a69..8f53751 100644
--- a/java/com/google/gerrit/server/patch/gitfilediff/FileHeaderUtil.java
+++ b/java/com/google/gerrit/server/patch/gitfilediff/FileHeaderUtil.java
@@ -56,7 +56,8 @@
/**
* Returns the old file path associated with the {@link FileHeader}, or empty if the file is
- * {@link Patch.ChangeType#ADDED} or {@link Patch.ChangeType#REWRITE}.
+ * {@link com.google.gerrit.entities.Patch.ChangeType#ADDED} or {@link
+ * com.google.gerrit.entities.Patch.ChangeType#REWRITE}.
*/
static Optional<String> getOldPath(FileHeader header) {
Patch.ChangeType changeType = getChangeType(header);
@@ -76,7 +77,7 @@
/**
* Returns the new file path associated with the {@link FileHeader}, or empty if the file is
- * {@link Patch.ChangeType#DELETED}.
+ * {@link com.google.gerrit.entities.Patch.ChangeType#DELETED}.
*/
static Optional<String> getNewPath(FileHeader header) {
Patch.ChangeType changeType = getChangeType(header);
diff --git a/java/com/google/gerrit/server/permissions/DefaultPermissionBackend.java b/java/com/google/gerrit/server/permissions/DefaultPermissionBackend.java
index 41db9ee..66299a8 100644
--- a/java/com/google/gerrit/server/permissions/DefaultPermissionBackend.java
+++ b/java/com/google/gerrit/server/permissions/DefaultPermissionBackend.java
@@ -99,8 +99,8 @@
}
/**
- * Returns the {@link Account.Id} of the current user if a user is signed in. Catches exceptions
- * so that background jobs don't get impacted.
+ * Returns the {@link com.google.gerrit.entities.Account.Id} of the current user if a user is
+ * signed in. Catches exceptions so that background jobs don't get impacted.
*/
private Optional<Account.Id> getAccountIdOfIdentifiedUser() {
try {
diff --git a/java/com/google/gerrit/server/query/change/AfterPredicate.java b/java/com/google/gerrit/server/query/change/AfterPredicate.java
index df5a71d..8f92d9a 100644
--- a/java/com/google/gerrit/server/query/change/AfterPredicate.java
+++ b/java/com/google/gerrit/server/query/change/AfterPredicate.java
@@ -14,15 +14,21 @@
package com.google.gerrit.server.query.change;
+import com.google.gerrit.index.FieldDef;
import com.google.gerrit.index.query.QueryParseException;
-import com.google.gerrit.server.index.change.ChangeField;
+import java.sql.Timestamp;
import java.util.Date;
+/**
+ * Predicate that matches a {@link Timestamp} field from the index in a range from the passed {@code
+ * String} representation of the Timestamp value to the maximum supported time.
+ */
public class AfterPredicate extends TimestampRangeChangePredicate {
protected final Date cut;
- public AfterPredicate(String value) throws QueryParseException {
- super(ChangeField.UPDATED, ChangeQueryBuilder.FIELD_BEFORE, value);
+ public AfterPredicate(FieldDef<ChangeData, Timestamp> def, String name, String value)
+ throws QueryParseException {
+ super(def, name, value);
cut = parse(value);
}
@@ -38,6 +44,10 @@
@Override
public boolean match(ChangeData cd) {
- return cd.change().getLastUpdatedOn().getTime() >= cut.getTime();
+ Timestamp valueTimestamp = this.getValueTimestamp(cd);
+ if (valueTimestamp == null) {
+ return false;
+ }
+ return valueTimestamp.getTime() >= cut.getTime();
}
}
diff --git a/java/com/google/gerrit/server/query/change/AgePredicate.java b/java/com/google/gerrit/server/query/change/AgePredicate.java
index 36eb5b7..d38789f 100644
--- a/java/com/google/gerrit/server/query/change/AgePredicate.java
+++ b/java/com/google/gerrit/server/query/change/AgePredicate.java
@@ -17,7 +17,6 @@
import static java.util.concurrent.TimeUnit.MILLISECONDS;
import static java.util.concurrent.TimeUnit.SECONDS;
-import com.google.gerrit.entities.Change;
import com.google.gerrit.server.config.ConfigUtil;
import com.google.gerrit.server.index.change.ChangeField;
import com.google.gerrit.server.util.time.TimeUtil;
@@ -46,7 +45,10 @@
@Override
public boolean match(ChangeData object) {
- Change change = object.change();
- return change != null && change.getLastUpdatedOn().getTime() <= cut;
+ Timestamp valueTimestamp = this.getValueTimestamp(object);
+ if (valueTimestamp == null) {
+ return false;
+ }
+ return valueTimestamp.getTime() <= cut;
}
}
diff --git a/java/com/google/gerrit/server/query/change/BeforePredicate.java b/java/com/google/gerrit/server/query/change/BeforePredicate.java
index dacabc0..6e28ce6 100644
--- a/java/com/google/gerrit/server/query/change/BeforePredicate.java
+++ b/java/com/google/gerrit/server/query/change/BeforePredicate.java
@@ -14,15 +14,21 @@
package com.google.gerrit.server.query.change;
+import com.google.gerrit.index.FieldDef;
import com.google.gerrit.index.query.QueryParseException;
-import com.google.gerrit.server.index.change.ChangeField;
+import java.sql.Timestamp;
import java.util.Date;
+/**
+ * Predicate that matches a {@link Timestamp} field from the index in a range from the the epoch to
+ * the passed {@code String} representation of the Timestamp value.
+ */
public class BeforePredicate extends TimestampRangeChangePredicate {
protected final Date cut;
- public BeforePredicate(String value) throws QueryParseException {
- super(ChangeField.UPDATED, ChangeQueryBuilder.FIELD_BEFORE, value);
+ public BeforePredicate(FieldDef<ChangeData, Timestamp> def, String name, String value)
+ throws QueryParseException {
+ super(def, name, value);
cut = parse(value);
}
@@ -38,6 +44,10 @@
@Override
public boolean match(ChangeData cd) {
- return cd.change().getLastUpdatedOn().getTime() <= cut.getTime();
+ Timestamp valueTimestamp = this.getValueTimestamp(cd);
+ if (valueTimestamp == null) {
+ return false;
+ }
+ return valueTimestamp.getTime() <= cut.getTime();
}
}
diff --git a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
index ff90c3f..8f0e711 100644
--- a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
+++ b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
@@ -142,7 +142,7 @@
public static final String FIELD_ASSIGNEE = "assignee";
public static final String FIELD_AUTHOR = "author";
public static final String FIELD_EXACTAUTHOR = "exactauthor";
- public static final String FIELD_BEFORE = "before";
+
public static final String FIELD_CHANGE = "change";
public static final String FIELD_CHANGE_ID = "change_id";
public static final String FIELD_COMMENT = "comment";
@@ -205,6 +205,10 @@
public static final String ARG_ID_OWNER = "owner";
public static final Account.Id OWNER_ACCOUNT_ID = Account.id(0);
+ // Operators to match on the last time the change was updated. Naming for legacy reasons.
+ public static final String OPERATOR_BEFORE = "before";
+ public static final String OPERATOR_AFTER = "after";
+
private static final QueryBuilder.Definition<ChangeData, ChangeQueryBuilder> mydef =
new QueryBuilder.Definition<>(ChangeQueryBuilder.class);
@@ -471,7 +475,7 @@
@Operator
public Predicate<ChangeData> before(String value) throws QueryParseException {
- return new BeforePredicate(value);
+ return new BeforePredicate(ChangeField.UPDATED, ChangeQueryBuilder.OPERATOR_BEFORE, value);
}
@Operator
@@ -481,7 +485,7 @@
@Operator
public Predicate<ChangeData> after(String value) throws QueryParseException {
- return new AfterPredicate(value);
+ return new AfterPredicate(ChangeField.UPDATED, ChangeQueryBuilder.OPERATOR_AFTER, value);
}
@Operator
diff --git a/java/com/google/gerrit/server/restapi/change/OnPostReview.java b/java/com/google/gerrit/server/restapi/change/OnPostReview.java
new file mode 100644
index 0000000..b179d02
--- /dev/null
+++ b/java/com/google/gerrit/server/restapi/change/OnPostReview.java
@@ -0,0 +1,47 @@
+// Copyright (C) 2020 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.restapi.change;
+
+import com.google.gerrit.entities.PatchSet;
+import com.google.gerrit.extensions.annotations.ExtensionPoint;
+import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.notedb.ChangeNotes;
+import java.util.Map;
+import java.util.Optional;
+
+/** Extension point that is invoked on post review. */
+@ExtensionPoint
+public interface OnPostReview {
+ /**
+ * Allows implementors to return a message that should be included into the change message that is
+ * posted on post review.
+ *
+ * @param user the user that posts the review
+ * @param changeNotes the change on which post review is performed
+ * @param patchSet the patch set on which post review is performed
+ * @param oldApprovals old approvals that changed as result of post review
+ * @param approvals all current approvals
+ * @return message that should be included into the change message that is posted on post review,
+ * {@link Optional#empty()} if the change message should not be extended
+ */
+ default Optional<String> getChangeMessageAddOn(
+ IdentifiedUser user,
+ ChangeNotes changeNotes,
+ PatchSet patchSet,
+ Map<String, Short> oldApprovals,
+ Map<String, Short> approvals) {
+ return Optional.empty();
+ }
+}
diff --git a/java/com/google/gerrit/server/restapi/change/PostReview.java b/java/com/google/gerrit/server/restapi/change/PostReview.java
index 604c87f..a66abaa 100644
--- a/java/com/google/gerrit/server/restapi/change/PostReview.java
+++ b/java/com/google/gerrit/server/restapi/change/PostReview.java
@@ -30,6 +30,7 @@
import static javax.servlet.http.HttpServletResponse.SC_BAD_REQUEST;
import com.google.auto.value.AutoValue;
+import com.google.common.base.Joiner;
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Lists;
@@ -177,6 +178,7 @@
private final ProjectCache projectCache;
private final PermissionBackend permissionBackend;
private final PluginSetContext<CommentValidator> commentValidators;
+ private final PluginSetContext<OnPostReview> onPostReviews;
private final ReplyAttentionSetUpdates replyAttentionSetUpdates;
private final boolean strictLabels;
@@ -202,6 +204,7 @@
ProjectCache projectCache,
PermissionBackend permissionBackend,
PluginSetContext<CommentValidator> commentValidators,
+ PluginSetContext<OnPostReview> onPostReviews,
ReplyAttentionSetUpdates replyAttentionSetUpdates) {
this.updateFactory = updateFactory;
this.changeResourceFactory = changeResourceFactory;
@@ -222,6 +225,7 @@
this.projectCache = projectCache;
this.permissionBackend = permissionBackend;
this.commentValidators = commentValidators;
+ this.onPostReviews = onPostReviews;
this.replyAttentionSetUpdates = replyAttentionSetUpdates;
this.strictLabels = gerritConfig.getBoolean("change", "strictLabels", false);
}
@@ -1413,6 +1417,23 @@
} else if (in.ready) {
buf.append("\n\n" + START_REVIEW_MESSAGE);
}
+
+ List<String> pluginMessages = new ArrayList<>();
+ onPostReviews.runEach(
+ onPostReview ->
+ onPostReview
+ .getChangeMessageAddOn(user, ctx.getNotes(), ps, oldApprovals, approvals)
+ .ifPresent(
+ pluginMessage ->
+ pluginMessages.add(
+ !pluginMessage.endsWith("\n")
+ ? pluginMessage + "\n"
+ : pluginMessage)));
+ if (!pluginMessages.isEmpty()) {
+ buf.append("\n\n");
+ buf.append(Joiner.on("\n").join(pluginMessages));
+ }
+
if (buf.length() == 0) {
return false;
}
diff --git a/java/com/google/gerrit/server/restapi/project/ListBranches.java b/java/com/google/gerrit/server/restapi/project/ListBranches.java
index fecdc8e..2c26933 100644
--- a/java/com/google/gerrit/server/restapi/project/ListBranches.java
+++ b/java/com/google/gerrit/server/restapi/project/ListBranches.java
@@ -143,7 +143,11 @@
BranchInfo toBranchInfo(BranchResource rsrc)
throws IOException, ResourceNotFoundException, PermissionBackendException {
try (Repository db = repoManager.openRepository(rsrc.getNameKey())) {
- Ref r = db.exactRef(rsrc.getRef());
+ String refName = rsrc.getRef();
+ if (RefNames.isRefsUsersSelf(refName, rsrc.getProjectState().isAllUsers())) {
+ refName = RefNames.refsUsers(rsrc.getUser().getAccountId());
+ }
+ Ref r = db.exactRef(refName);
if (r == null) {
throw new ResourceNotFoundException();
}
diff --git a/java/com/google/gerrit/server/util/PluginLogFile.java b/java/com/google/gerrit/server/util/PluginLogFile.java
index 8235623..345e1b3 100644
--- a/java/com/google/gerrit/server/util/PluginLogFile.java
+++ b/java/com/google/gerrit/server/util/PluginLogFile.java
@@ -16,7 +16,6 @@
import com.google.gerrit.extensions.events.LifecycleListener;
import com.google.gerrit.extensions.systemstatus.ServerInformation;
-import org.apache.log4j.AsyncAppender;
import org.apache.log4j.Layout;
import org.apache.log4j.LogManager;
import org.apache.log4j.Logger;
@@ -38,12 +37,11 @@
@Override
public void start() {
- AsyncAppender asyncAppender = systemLog.createAsyncAppender(logName, layout, true, true);
Logger logger = LogManager.getLogger(logName);
if (logger.getAppender(logName) == null) {
- synchronized (this) {
+ synchronized (systemLog) {
if (logger.getAppender(logName) == null) {
- logger.addAppender(asyncAppender);
+ logger.addAppender(systemLog.createAsyncAppender(logName, layout, true, true));
}
}
}
diff --git a/javatests/com/google/gerrit/acceptance/api/change/PostReviewIT.java b/javatests/com/google/gerrit/acceptance/api/change/PostReviewIT.java
index 7d73374..029d5a2 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/PostReviewIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/PostReviewIT.java
@@ -29,9 +29,16 @@
import com.google.common.collect.Iterables;
import com.google.common.truth.Correspondence;
import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.ExtensionRegistry;
+import com.google.gerrit.acceptance.ExtensionRegistry.Registration;
import com.google.gerrit.acceptance.PushOneCommit;
+import com.google.gerrit.acceptance.TestAccount;
import com.google.gerrit.acceptance.config.GerritConfig;
import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
+import com.google.gerrit.common.Nullable;
+import com.google.gerrit.entities.Account;
+import com.google.gerrit.entities.Change;
+import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.extensions.annotations.Exports;
import com.google.gerrit.extensions.api.changes.DraftInput;
import com.google.gerrit.extensions.api.changes.ReviewInput;
@@ -49,6 +56,9 @@
import com.google.gerrit.extensions.validators.CommentForValidation;
import com.google.gerrit.extensions.validators.CommentValidationContext;
import com.google.gerrit.extensions.validators.CommentValidator;
+import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.notedb.ChangeNotes;
+import com.google.gerrit.server.restapi.change.OnPostReview;
import com.google.gerrit.server.restapi.change.PostReview;
import com.google.gerrit.server.update.CommentsRejectedException;
import com.google.gerrit.testing.TestCommentHelper;
@@ -58,6 +68,7 @@
import java.util.Collection;
import java.util.List;
import java.util.Map;
+import java.util.Optional;
import org.junit.Before;
import org.junit.Test;
import org.mockito.ArgumentCaptor;
@@ -69,6 +80,7 @@
@Inject private CommentValidator mockCommentValidator;
@Inject private TestCommentHelper testCommentHelper;
@Inject private RequestScopeOperations requestScopeOperations;
+ @Inject private ExtensionRegistry extensionRegistry;
private static final String COMMENT_TEXT = "The comment text";
private static final CommentForValidation FILE_COMMENT_FOR_VALIDATION =
@@ -474,6 +486,124 @@
assertThat(reviewer._accountId).isEqualTo(user.id().get());
}
+ @Test
+ public void extendChangeMessageFromPlugin() throws Exception {
+ PushOneCommit.Result r = createChange();
+
+ String testMessage = "hello from plugin";
+ TestOnPostReview testOnPostReview = new TestOnPostReview(testMessage);
+ try (Registration registration = extensionRegistry.newRegistration().add(testOnPostReview)) {
+ ReviewInput input = new ReviewInput().label("Code-Review", 1);
+ gApi.changes().id(r.getChangeId()).current().review(input);
+ Collection<ChangeMessageInfo> messages = gApi.changes().id(r.getChangeId()).get().messages;
+ assertThat(Iterables.getLast(messages).message)
+ .isEqualTo(String.format("Patch Set 1: Code-Review+1\n\n%s\n", testMessage));
+ }
+ }
+
+ @Test
+ public void extendChangeMessageFromMultiplePlugins() throws Exception {
+ PushOneCommit.Result r = createChange();
+
+ String testMessage1 = "hello from plugin 1";
+ String testMessage2 = "message from plugin 2";
+ TestOnPostReview testOnPostReview1 = new TestOnPostReview(testMessage1);
+ TestOnPostReview testOnPostReview2 = new TestOnPostReview(testMessage2);
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(testOnPostReview1).add(testOnPostReview2)) {
+ ReviewInput input = new ReviewInput().label("Code-Review", 1);
+ gApi.changes().id(r.getChangeId()).current().review(input);
+ Collection<ChangeMessageInfo> messages = gApi.changes().id(r.getChangeId()).get().messages;
+ assertThat(Iterables.getLast(messages).message)
+ .isEqualTo(
+ String.format(
+ "Patch Set 1: Code-Review+1\n\n%s\n\n%s\n", testMessage1, testMessage2));
+ }
+ }
+
+ @Test
+ public void onPostReviewExtensionThatDoesntExtendTheChangeMessage() throws Exception {
+ PushOneCommit.Result r = createChange();
+
+ TestOnPostReview testOnPostReview = new TestOnPostReview(/* message= */ null);
+ try (Registration registration = extensionRegistry.newRegistration().add(testOnPostReview)) {
+ ReviewInput input = new ReviewInput().label("Code-Review", 1);
+ gApi.changes().id(r.getChangeId()).current().review(input);
+ Collection<ChangeMessageInfo> messages = gApi.changes().id(r.getChangeId()).get().messages;
+ assertThat(Iterables.getLast(messages).message).isEqualTo("Patch Set 1: Code-Review+1");
+ }
+ }
+
+ @Test
+ public void onPostReviewCallbackGetsCorrectChangeAndPatchSet() throws Exception {
+ PushOneCommit.Result r = createChange();
+ amendChange(r.getChangeId());
+
+ TestOnPostReview testOnPostReview = new TestOnPostReview(/* message= */ null);
+ try (Registration registration = extensionRegistry.newRegistration().add(testOnPostReview)) {
+ ReviewInput input = new ReviewInput().label("Code-Review", 1);
+
+ // Vote on current patch set.
+ gApi.changes().id(r.getChangeId()).current().review(input);
+ testOnPostReview.assertChangeAndPatchSet(r.getChange().getId(), 2);
+
+ // Vote on old patch set.
+ gApi.changes().id(r.getChangeId()).revision(1).review(input);
+ testOnPostReview.assertChangeAndPatchSet(r.getChange().getId(), 1);
+ }
+ }
+
+ @Test
+ public void onPostReviewCallbackGetsCorrectUser() throws Exception {
+ PushOneCommit.Result r = createChange();
+
+ TestOnPostReview testOnPostReview = new TestOnPostReview(/* message= */ null);
+ try (Registration registration = extensionRegistry.newRegistration().add(testOnPostReview)) {
+ ReviewInput input = new ReviewInput().label("Code-Review", 1);
+
+ // Vote from admin.
+ gApi.changes().id(r.getChangeId()).current().review(input);
+ testOnPostReview.assertUser(admin);
+
+ // Vote from user.
+ requestScopeOperations.setApiUser(user.id());
+ gApi.changes().id(r.getChangeId()).current().review(input);
+ testOnPostReview.assertUser(user);
+ }
+ }
+
+ @Test
+ public void onPostReviewCallbackGetsCorrectApprovals() throws Exception {
+ PushOneCommit.Result r = createChange();
+
+ TestOnPostReview testOnPostReview = new TestOnPostReview(/* message= */ null);
+ try (Registration registration = extensionRegistry.newRegistration().add(testOnPostReview)) {
+ // Add a new vote.
+ ReviewInput input = new ReviewInput().label("Code-Review", 1);
+ gApi.changes().id(r.getChangeId()).current().review(input);
+ testOnPostReview.assertApproval(
+ "Code-Review", /* expectedOldValue= */ 0, /* expectedNewValue= */ 1);
+
+ // Update an existing vote.
+ input = new ReviewInput().label("Code-Review", 2);
+ gApi.changes().id(r.getChangeId()).current().review(input);
+ testOnPostReview.assertApproval(
+ "Code-Review", /* expectedOldValue= */ 1, /* expectedNewValue= */ 2);
+
+ // Post without changing the vote.
+ input = new ReviewInput().label("Code-Review", 2);
+ gApi.changes().id(r.getChangeId()).current().review(input);
+ testOnPostReview.assertApproval(
+ "Code-Review", /* expectedOldValue= */ null, /* expectedNewValue= */ 2);
+
+ // Delete the vote.
+ input = new ReviewInput().label("Code-Review", 0);
+ gApi.changes().id(r.getChangeId()).current().review(input);
+ testOnPostReview.assertApproval(
+ "Code-Review", /* expectedOldValue= */ 2, /* expectedNewValue= */ 0);
+ }
+ }
+
private List<RobotCommentInfo> getRobotComments(String changeId) throws RestApiException {
return gApi.changes().id(changeId).robotComments().values().stream()
.flatMap(Collection::stream)
@@ -495,4 +625,50 @@
.comparingElementsUsing(COMMENT_CORRESPONDENCE)
.containsExactly(commentsForValidation);
}
+
+ private static class TestOnPostReview implements OnPostReview {
+ private final Optional<String> message;
+
+ private Change.Id changeId;
+ private PatchSet.Id patchSetId;
+ private Account.Id accountId;
+ private Map<String, Short> oldApprovals;
+ private Map<String, Short> approvals;
+
+ TestOnPostReview(@Nullable String message) {
+ this.message = Optional.ofNullable(message);
+ }
+
+ @Override
+ public Optional<String> getChangeMessageAddOn(
+ IdentifiedUser user,
+ ChangeNotes changeNotes,
+ PatchSet patchSet,
+ Map<String, Short> oldApprovals,
+ Map<String, Short> approvals) {
+ this.changeId = changeNotes.getChangeId();
+ this.patchSetId = patchSet.id();
+ this.accountId = user.getAccountId();
+ this.oldApprovals = oldApprovals;
+ this.approvals = approvals;
+ return message;
+ }
+
+ public void assertChangeAndPatchSet(Change.Id expectedChangeId, int expectedPatchSetNum) {
+ assertThat(changeId).isEqualTo(expectedChangeId);
+ assertThat(patchSetId.get()).isEqualTo(expectedPatchSetNum);
+ }
+
+ public void assertUser(TestAccount expectedUser) {
+ assertThat(accountId).isEqualTo(expectedUser.id());
+ }
+
+ public void assertApproval(
+ String labelName, @Nullable Integer expectedOldValue, int expectedNewValue) {
+ assertThat(oldApprovals)
+ .containsExactly(
+ labelName, expectedOldValue != null ? expectedOldValue.shortValue() : null);
+ assertThat(approvals).containsExactly(labelName, (short) expectedNewValue);
+ }
+ }
}
diff --git a/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java b/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java
index c9b0a9f..a50bbcf 100644
--- a/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java
+++ b/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java
@@ -899,7 +899,19 @@
GitUtil.fetch(user2Repo, r.getPatchSet().refName() + ":ps");
user2Repo.reset("ps");
r = amendChange(r.getChangeId(), "refs/for/master%ready", user2, user2Repo);
- r.assertErrorStatus(ReceiveConstants.ONLY_CHANGE_OWNER_OR_PROJECT_OWNER_CAN_MODIFY_WIP);
+ r.assertErrorStatus(ReceiveConstants.ONLY_USERS_WITH_TOGGLE_WIP_STATE_PERM_CAN_MODIFY_WIP);
+
+ // Non owner, non admin and non project owner with toggleWipState should succeed.
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(
+ allow(Permission.TOGGLE_WORK_IN_PROGRESS_STATE)
+ .ref(RefNames.REFS_HEADS + "*")
+ .group(REGISTERED_USERS))
+ .update();
+ r = amendChange(r.getChangeId(), "refs/for/master%ready", user2, user2Repo);
+ r.assertOkStatus();
// Project owner trying to move from WIP to ready should succeed.
projectOperations
diff --git a/javatests/com/google/gerrit/acceptance/rest/RestApiServletIT.java b/javatests/com/google/gerrit/acceptance/rest/RestApiServletIT.java
new file mode 100644
index 0000000..bc45460
--- /dev/null
+++ b/javatests/com/google/gerrit/acceptance/rest/RestApiServletIT.java
@@ -0,0 +1,76 @@
+// Copyright (C) 2020 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.acceptance.rest;
+
+import static com.google.common.truth.Truth.assertThat;
+import static org.apache.http.HttpStatus.SC_OK;
+
+import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.RestResponse;
+import com.google.gerrit.httpd.restapi.RestApiServlet;
+import java.io.IOException;
+import java.util.regex.Pattern;
+import org.apache.http.message.BasicHeader;
+import org.junit.Test;
+
+public class RestApiServletIT extends AbstractDaemonTest {
+ private static String ANY_REST_API = "/accounts/self/capabilities";
+ private static BasicHeader ACCEPT_STAR_HEADER = new BasicHeader("Accept", "*/*");
+ private static Pattern ANY_SPACE = Pattern.compile("\\s");
+
+ @Test
+ public void restResponseBodyShouldBeCompactWithoutSpaces() throws Exception {
+ RestResponse response = adminRestSession.getWithHeader(ANY_REST_API, ACCEPT_STAR_HEADER);
+ assertThat(response.getStatusCode()).isEqualTo(SC_OK);
+
+ assertThat(contentWithoutMagicJson(response)).doesNotContainMatch(ANY_SPACE);
+ }
+
+ @Test
+ public void restResponseBodyShouldBeCompactWithoutSpacesWhenPPIsZero() throws Exception {
+ assertThat(contentWithoutMagicJson(prettyJsonRestResponse("prettyPrint", 0)))
+ .doesNotContainMatch(ANY_SPACE);
+ }
+
+ @Test
+ public void restResponseBodyShouldBeCompactWithoutSpacesWhenPrerryPrintIsZero() throws Exception {
+ assertThat(contentWithoutMagicJson(prettyJsonRestResponse("pp", 0)))
+ .doesNotContainMatch(ANY_SPACE);
+ }
+
+ @Test
+ public void restResponseBodyShouldBePrettyfiedWhenPPIsOne() throws Exception {
+ assertThat(contentWithoutMagicJson(prettyJsonRestResponse("pp", 1))).containsMatch(ANY_SPACE);
+ }
+
+ @Test
+ public void restResponseBodyShouldBePrettyfiedWhenPrettyPrintIsOne() throws Exception {
+ assertThat(contentWithoutMagicJson(prettyJsonRestResponse("prettyPrint", 1)))
+ .containsMatch(ANY_SPACE);
+ }
+
+ private RestResponse prettyJsonRestResponse(String ppArgument, int ppValue) throws Exception {
+ RestResponse response =
+ adminRestSession.getWithHeader(
+ ANY_REST_API + "?" + ppArgument + "=" + ppValue, ACCEPT_STAR_HEADER);
+ assertThat(response.getStatusCode()).isEqualTo(SC_OK);
+
+ return response;
+ }
+
+ private String contentWithoutMagicJson(RestResponse response) throws IOException {
+ return response.getEntityContent().substring(RestApiServlet.JSON_MAGIC.length);
+ }
+}
diff --git a/javatests/com/google/gerrit/acceptance/rest/TraceIT.java b/javatests/com/google/gerrit/acceptance/rest/TraceIT.java
index cb34bdb..7f8add8 100644
--- a/javatests/com/google/gerrit/acceptance/rest/TraceIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/TraceIT.java
@@ -18,6 +18,13 @@
import static org.apache.http.HttpStatus.SC_CREATED;
import static org.apache.http.HttpStatus.SC_INTERNAL_SERVER_ERROR;
import static org.apache.http.HttpStatus.SC_OK;
+import static org.mockito.ArgumentMatchers.anyLong;
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.Mockito.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.timeout;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyZeroInteractions;
import com.google.auto.value.AutoValue;
import com.google.common.collect.ImmutableList;
@@ -52,7 +59,6 @@
import com.google.gerrit.server.validators.ProjectCreationValidationListener;
import com.google.gerrit.server.validators.ValidationException;
import com.google.inject.Inject;
-import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.Optional;
@@ -370,37 +376,31 @@
@Test
public void performanceLoggingForRestCall() throws Exception {
- TestPerformanceLogger testPerformanceLogger = new TestPerformanceLogger();
+ PerformanceLogger testPerformanceLogger = mock(PerformanceLogger.class);
try (Registration registration =
extensionRegistry.newRegistration().add(testPerformanceLogger)) {
RestResponse response = adminRestSession.put("/projects/new10");
assertThat(response.getStatusCode()).isEqualTo(SC_CREATED);
-
- // This assertion assumes that the server invokes the PerformanceLogger plugins before it
- // sends
- // the response to the client. If this assertion gets flaky it's likely that this got changed
- // on
- // server-side.
- assertThat(testPerformanceLogger.logEntries()).isNotEmpty();
+ verify(testPerformanceLogger, timeout(5000).atLeastOnce()).log(anyString(), anyLong(), any());
}
}
@Test
public void performanceLoggingForPush() throws Exception {
- TestPerformanceLogger testPerformanceLogger = new TestPerformanceLogger();
+ PerformanceLogger testPerformanceLogger = mock(PerformanceLogger.class);
try (Registration registration =
extensionRegistry.newRegistration().add(testPerformanceLogger)) {
PushOneCommit push = pushFactory.create(admin.newIdent(), testRepo);
PushOneCommit.Result r = push.to("refs/heads/master");
r.assertOkStatus();
- assertThat(testPerformanceLogger.logEntries()).isNotEmpty();
+ verify(testPerformanceLogger, timeout(5000).atLeastOnce()).log(anyString(), anyLong(), any());
}
}
@Test
@GerritConfig(name = "tracing.performanceLogging", value = "false")
public void noPerformanceLoggingIfDisabled() throws Exception {
- TestPerformanceLogger testPerformanceLogger = new TestPerformanceLogger();
+ PerformanceLogger testPerformanceLogger = mock(PerformanceLogger.class);
try (Registration registration =
extensionRegistry.newRegistration().add(testPerformanceLogger)) {
RestResponse response = adminRestSession.put("/projects/new11");
@@ -410,7 +410,7 @@
PushOneCommit.Result r = push.to("refs/heads/master");
r.assertOkStatus();
- assertThat(testPerformanceLogger.logEntries()).isEmpty();
+ verifyZeroInteractions(testPerformanceLogger);
}
}
@@ -844,19 +844,6 @@
}
}
- private static class TestPerformanceLogger implements PerformanceLogger {
- private List<PerformanceLogEntry> logEntries = new ArrayList<>();
-
- @Override
- public void log(String operation, long durationMs, Metadata metadata) {
- logEntries.add(PerformanceLogEntry.create(operation, metadata));
- }
-
- ImmutableList<PerformanceLogEntry> logEntries() {
- return ImmutableList.copyOf(logEntries);
- }
- }
-
@AutoValue
abstract static class PerformanceLogEntry {
static PerformanceLogEntry create(String operation, Metadata metadata) {
diff --git a/javatests/com/google/gerrit/acceptance/rest/project/GetBranchIT.java b/javatests/com/google/gerrit/acceptance/rest/project/GetBranchIT.java
index b4b1be0..13c20dd 100644
--- a/javatests/com/google/gerrit/acceptance/rest/project/GetBranchIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/project/GetBranchIT.java
@@ -262,9 +262,12 @@
requestScopeOperations.setApiUser(user.id());
assertBranchFound(allUsers, RefNames.refsUsers(user.id()));
- // TODO: every user can see the own user ref via the magic ref/users/self ref
- // requestScopeOperations.setApiUser(user.id());
- // assertBranchFound(allUsers, RefNames.REFS_USERS_SELF);
+ // every user can see the own user ref via the magic ref/users/self ref. For this special case,
+ // the branch in the request is refs/users/self, but the response contains the actual
+ // refs/users/$sharded_id/$id
+ BranchInfo branchInfo =
+ gApi.projects().name(allUsers.get()).branch(RefNames.REFS_USERS_SELF).get();
+ assertThat(branchInfo.ref).isEqualTo(RefNames.refsUsers(user.id()));
}
@Test
diff --git a/javatests/com/google/gerrit/acceptance/server/util/BUILD b/javatests/com/google/gerrit/acceptance/server/util/BUILD
new file mode 100644
index 0000000..ea25784
--- /dev/null
+++ b/javatests/com/google/gerrit/acceptance/server/util/BUILD
@@ -0,0 +1,7 @@
+load("//javatests/com/google/gerrit/acceptance:tests.bzl", "acceptance_tests")
+
+acceptance_tests(
+ srcs = glob(["*IT.java"]),
+ group = "server_util",
+ labels = ["server"],
+)
diff --git a/javatests/com/google/gerrit/acceptance/server/util/PluginLogFileIT.java b/javatests/com/google/gerrit/acceptance/server/util/PluginLogFileIT.java
new file mode 100644
index 0000000..06bf1ae
--- /dev/null
+++ b/javatests/com/google/gerrit/acceptance/server/util/PluginLogFileIT.java
@@ -0,0 +1,58 @@
+// Copyright (C) 2020 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.acceptance.server.util;
+
+import static junit.framework.TestCase.assertEquals;
+import static org.junit.Assert.fail;
+
+import com.google.gerrit.acceptance.AbstractPluginLogFileTest;
+import com.google.gerrit.acceptance.NoHttpd;
+import com.google.gerrit.acceptance.UseSsh;
+import com.google.inject.Inject;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import org.junit.Test;
+
+@NoHttpd
+@UseSsh
+public class PluginLogFileIT extends AbstractPluginLogFileTest {
+ @Inject private InvocationCounter invocationCounter;
+ private static final int NUMBER_OF_THREADS = 5;
+
+ @Test
+ public void testMultiThreadedPluginLogFile() throws Exception {
+ try (AutoCloseable ignored = installPlugin("my-plugin", Module.class)) {
+ ExecutorService service = Executors.newFixedThreadPool(NUMBER_OF_THREADS);
+ CountDownLatch latch = new CountDownLatch(NUMBER_OF_THREADS);
+ createChange();
+ for (int i = 0; i < NUMBER_OF_THREADS; i++) {
+ service.execute(
+ () -> {
+ try {
+ adminSshSession.exec("gerrit query --format json status:open --my-plugin--opt");
+ adminSshSession.assertSuccess();
+ } catch (Exception e) {
+ fail(e.getMessage());
+ } finally {
+ latch.countDown();
+ }
+ });
+ }
+ latch.await();
+ assertEquals(1, invocationCounter.getCounter());
+ }
+ }
+}
diff --git a/javatests/com/google/gerrit/server/cache/serialize/entities/StoredCommentLinkInfoSerializerTest.java b/javatests/com/google/gerrit/server/cache/serialize/entities/StoredCommentLinkInfoSerializerTest.java
index 3a51b70..e293493 100644
--- a/javatests/com/google/gerrit/server/cache/serialize/entities/StoredCommentLinkInfoSerializerTest.java
+++ b/javatests/com/google/gerrit/server/cache/serialize/entities/StoredCommentLinkInfoSerializerTest.java
@@ -56,4 +56,19 @@
.build();
assertThat(deserialize(serialize(autoValue))).isEqualTo(autoValue);
}
+
+ @Test
+ public void nullEnabled_roundTrip() {
+ StoredCommentLinkInfo sourceAutoValue =
+ StoredCommentLinkInfo.builder("name").setLink("<p>html").setMatch("*").build();
+
+ StoredCommentLinkInfo storedAutoValue =
+ StoredCommentLinkInfo.builder("name")
+ .setLink("<p>html")
+ .setMatch("*")
+ .setEnabled(true)
+ .build();
+
+ assertThat(deserialize(serialize(sourceAutoValue))).isEqualTo(storedAutoValue);
+ }
}
diff --git a/javatests/com/google/gerrit/server/index/change/ChangeIndexRewriterTest.java b/javatests/com/google/gerrit/server/index/change/ChangeIndexRewriterTest.java
index f5d3bf7..e5b2ffb 100644
--- a/javatests/com/google/gerrit/server/index/change/ChangeIndexRewriterTest.java
+++ b/javatests/com/google/gerrit/server/index/change/ChangeIndexRewriterTest.java
@@ -107,6 +107,32 @@
}
@Test
+ public void threeLevelTreeWithMultipleSources() throws Exception {
+ Predicate<ChangeData> in = parse("-status:abandoned (foo:a OR file:b)");
+ Predicate<ChangeData> out = rewrite(in);
+ assertThat(out.getClass()).isSameInstanceAs(AndChangeSource.class);
+
+ Predicate<ChangeData> firstIndexedSubQuery = parse("-status:abandoned");
+
+ assertThat(out.getChild(0)).isEqualTo(query(firstIndexedSubQuery));
+
+ assertThat(out.getChild(1).getClass()).isSameInstanceAs(OrSource.class);
+ OrSource indexedSubTree = (OrSource) out.getChild(1);
+
+ Predicate<ChangeData> secondIndexedSubQuery = parse("foo:a OR file:b");
+ assertThat(indexedSubTree.getChildren())
+ .containsExactly(
+ query(secondIndexedSubQuery.getChild(1)), secondIndexedSubQuery.getChild(0))
+ .inOrder();
+
+ // Same at the assertions above, that were added for readability
+ assertThat(out.getChild(0)).isEqualTo(query(in.getChild(0)));
+ assertThat(indexedSubTree.getChildren())
+ .containsExactly(query(in.getChild(1).getChild(1)), in.getChild(1).getChild(0))
+ .inOrder();
+ }
+
+ @Test
public void threeLevelTreeWithSomeIndexPredicates() throws Exception {
Predicate<ChangeData> in = parse("-foo:a (file:b OR file:c)");
Predicate<ChangeData> out = rewrite(in);
diff --git a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
index 1de548f..5274d94 100644
--- a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
+++ b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
@@ -85,6 +85,7 @@
import com.google.gerrit.index.FieldDef;
import com.google.gerrit.index.IndexConfig;
import com.google.gerrit.index.Schema;
+import com.google.gerrit.index.query.IndexPredicate;
import com.google.gerrit.index.query.Predicate;
import com.google.gerrit.index.query.QueryParseException;
import com.google.gerrit.lifecycle.LifecycleManager;
@@ -110,6 +111,7 @@
import com.google.gerrit.server.index.change.ChangeField;
import com.google.gerrit.server.index.change.ChangeIndexCollection;
import com.google.gerrit.server.index.change.ChangeIndexer;
+import com.google.gerrit.server.index.change.IndexedChangeQuery;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.Sequences;
import com.google.gerrit.server.project.ProjectCache;
@@ -1573,6 +1575,10 @@
assertThat(nowMs - lastUpdatedMs(change2)).isEqualTo(thirtyHoursInMs);
assertThat(TimeUtil.nowMs()).isEqualTo(nowMs);
+ // Change1 was last updated on 2009-09-30 21:00:00 -0000
+ // Change2 was last updated on 2009-10-02 03:00:00 -0000
+ // The endpoint is 2009-10-03 09:00:00 -0000
+
assertQuery("-age:1d");
assertQuery("-age:" + (30 * 60 - 1) + "m");
assertQuery("-age:2d", change2);
@@ -1580,6 +1586,15 @@
assertQuery("age:3d");
assertQuery("age:2d", change1);
assertQuery("age:1d", change2, change1);
+
+ // Same test as above, but using filter code path.
+ assertQuery(makeIndexedPredicateFilterQuery("-age:1d"));
+ assertQuery(makeIndexedPredicateFilterQuery("-age:" + (30 * 60 - 1) + "m"));
+ assertQuery(makeIndexedPredicateFilterQuery("-age:2d"), change2);
+ assertQuery(makeIndexedPredicateFilterQuery("-age:3d"), change2, change1);
+ assertQuery(makeIndexedPredicateFilterQuery("age:3d"));
+ assertQuery(makeIndexedPredicateFilterQuery("age:2d"), change1);
+ assertQuery(makeIndexedPredicateFilterQuery("age:1d"), change2, change1);
}
@Test
@@ -1592,6 +1607,9 @@
Change change2 = insert(repo, newChange(repo), null, new Timestamp(startMs + thirtyHoursInMs));
TestTimeUtil.setClockStep(0, MILLISECONDS);
+ // Change1 was last updated on 2009-09-30 21:00:00 -0000
+ // Change2 was last updated on 2009-10-02 03:00:00 -0000
+
for (String predicate : Lists.newArrayList("before:", "until:")) {
assertQuery(predicate + "2009-09-29");
assertQuery(predicate + "2009-09-30");
@@ -1604,6 +1622,22 @@
assertQuery(predicate + "2009-10-01", change1);
assertQuery(predicate + "2009-10-03", change2, change1);
}
+
+ // Same test as above, but using filter code path.
+ for (String predicate : Lists.newArrayList("before:", "until:")) {
+ assertQuery(makeIndexedPredicateFilterQuery(predicate + "2009-09-29"));
+ assertQuery(makeIndexedPredicateFilterQuery(predicate + "2009-09-30"));
+ assertQuery(makeIndexedPredicateFilterQuery(predicate + "\"2009-09-30 16:59:00 -0400\""));
+ assertQuery(makeIndexedPredicateFilterQuery(predicate + "\"2009-09-30 20:59:00 -0000\""));
+ assertQuery(makeIndexedPredicateFilterQuery(predicate + "\"2009-09-30 20:59:00\""));
+ assertQuery(
+ makeIndexedPredicateFilterQuery(predicate + "\"2009-09-30 17:02:00 -0400\""), change1);
+ assertQuery(
+ makeIndexedPredicateFilterQuery(predicate + "\"2009-10-01 21:02:00 -0000\""), change1);
+ assertQuery(makeIndexedPredicateFilterQuery(predicate + "\"2009-10-01 21:02:00\""), change1);
+ assertQuery(makeIndexedPredicateFilterQuery(predicate + "2009-10-01"), change1);
+ assertQuery(makeIndexedPredicateFilterQuery(predicate + "2009-10-03"), change2, change1);
+ }
}
@Test
@@ -1616,6 +1650,8 @@
Change change2 = insert(repo, newChange(repo), null, new Timestamp(startMs + thirtyHoursInMs));
TestTimeUtil.setClockStep(0, MILLISECONDS);
+ // Change1 was last updated on 2009-09-30 21:00:00 -0000
+ // Change2 was last updated on 2009-10-02 03:00:00 -0000
for (String predicate : Lists.newArrayList("after:", "since:")) {
assertQuery(predicate + "2009-10-03");
assertQuery(predicate + "\"2009-10-01 20:59:59 -0400\"", change2);
@@ -1623,6 +1659,17 @@
assertQuery(predicate + "2009-10-01", change2);
assertQuery(predicate + "2009-09-30", change2, change1);
}
+
+ // Same test as above, but using filter code path.
+ for (String predicate : Lists.newArrayList("after:", "since:")) {
+ assertQuery(makeIndexedPredicateFilterQuery(predicate + "2009-10-03"));
+ assertQuery(
+ makeIndexedPredicateFilterQuery(predicate + "\"2009-10-01 20:59:59 -0400\""), change2);
+ assertQuery(
+ makeIndexedPredicateFilterQuery(predicate + "\"2009-10-01 20:59:59 -0000\""), change2);
+ assertQuery(makeIndexedPredicateFilterQuery(predicate + "2009-10-01"), change2);
+ assertQuery(makeIndexedPredicateFilterQuery(predicate + "2009-09-30"), change2, change1);
+ }
}
@Test
@@ -3589,6 +3636,48 @@
return c.getLastUpdatedOn().getTime();
}
+ /**
+ * Generates a search query to test {@link com.google.gerrit.index.query.Matchable} implementation
+ * of change {@link IndexPredicate}
+ *
+ * <p>This code path requires triggering the condition, when
+ *
+ * <ol>
+ * <li>The query is rewritten into multiple {@link IndexedChangeQuery} by {@link
+ * com.google.gerrit.server.index.change.ChangeIndexRewriter#rewrite}
+ * <li>The changes are returned from the index by the first {@link IndexedChangeQuery}
+ * <li>Then constrained in {@link com.google.gerrit.index.query.AndSource#match} by applying all
+ * parsed predicates from the search query
+ * <li>Thus, the rest of {@link IndexedChangeQuery} work as filters on the index results, see
+ * {@link IndexedChangeQuery#match}
+ * </ol>
+ *
+ * The constructed query only constrains by the passed searchTerm for the operator that is being
+ * tested (for all changes without a reviewer):
+ *
+ * <ul>
+ * <li>The search term 'status:new OR status:merged OR status:abandoned' is used to return all
+ * changes from the search index.
+ * <li>The non-indexed search term 'reviewerin:"Empty Group"' is only used to make the right AND
+ * operand work as a filter (not a data source).
+ * <li>See how it is rewritten in {@link
+ * com.google.gerrit.server.index.change.ChangeIndexRewriterTest#threeLevelTreeWithMultipleSources}
+ * </ul>
+ *
+ * @param searchTerm change search term that maps to {@link IndexPredicate} and needs to be tested
+ * as filter
+ * @return a search query that allows to test the {@code searchTerm} as a filter.
+ */
+ protected String makeIndexedPredicateFilterQuery(String searchTerm) throws Exception {
+ String emptyGroupName = "Empty Group";
+ if (gApi.groups().query(emptyGroupName).get().isEmpty()) {
+ createGroup(emptyGroupName, "Administrators");
+ }
+ String queryPattern =
+ "(status:new OR status:merged OR status:abandoned) AND (reviewerin:\"%s\" OR %s)";
+ return String.format(queryPattern, emptyGroupName, searchTerm);
+ }
+
private void addComment(int changeId, String message, Boolean unresolved) throws Exception {
ReviewInput input = new ReviewInput();
ReviewInput.CommentInput comment = new ReviewInput.CommentInput();
diff --git a/modules/jgit b/modules/jgit
index 5cd485e..9a1065a 160000
--- a/modules/jgit
+++ b/modules/jgit
@@ -1 +1 @@
-Subproject commit 5cd485e5dda41d2ef06226a692c64f1aa221eb25
+Subproject commit 9a1065afec18c5a76d3a95e77aa70d7a24252056
diff --git a/plugins/gitiles b/plugins/gitiles
index a33c8b8..0cd08bb 160000
--- a/plugins/gitiles
+++ b/plugins/gitiles
@@ -1 +1 @@
-Subproject commit a33c8b8d61b778f8ca84196b1a7cc1fd4fe24946
+Subproject commit 0cd08bbfac1a938079be8aaf6a6eef0e82ff9a41
diff --git a/plugins/replication b/plugins/replication
index 4efef1d..9c21a2f 160000
--- a/plugins/replication
+++ b/plugins/replication
@@ -1 +1 @@
-Subproject commit 4efef1d481eefaee287b27488be94f9acb044360
+Subproject commit 9c21a2f5cc539b5a13d2646edc15bd17d7977fb3
diff --git a/polygerrit-ui/app/elements/admin/gr-access-section/gr-access-section.ts b/polygerrit-ui/app/elements/admin/gr-access-section/gr-access-section.ts
index 7c5364e..cbb3d95 100644
--- a/polygerrit-ui/app/elements/admin/gr-access-section/gr-access-section.ts
+++ b/polygerrit-ui/app/elements/admin/gr-access-section/gr-access-section.ts
@@ -42,6 +42,7 @@
LabelNameToLabelTypeInfoMap,
} from '../../../types/common';
import {PolymerDomRepeatEvent} from '../../../types/types';
+import {fireEvent} from '../../../utils/event-util';
/**
* Fired when the section has been modified or removed.
@@ -140,9 +141,7 @@
// For a new section, this is not fired because new permissions and
// rules have to be added in order to save, modifying the ref is not
// enough.
- this.dispatchEvent(
- new CustomEvent('access-modified', {bubbles: true, composed: true})
- );
+ fireEvent(this, 'access-modified');
}
this.section.value.updatedId = this.section.id;
}
@@ -275,18 +274,11 @@
return;
}
if (this.section.value.added) {
- this.dispatchEvent(
- new CustomEvent('added-section-removed', {
- bubbles: true,
- composed: true,
- })
- );
+ fireEvent(this, 'added-section-removed');
}
this._deleted = true;
this.section.value.deleted = true;
- this.dispatchEvent(
- new CustomEvent('access-modified', {bubbles: true, composed: true})
- );
+ fireEvent(this, 'access-modified');
}
_handleUndoRemove() {
diff --git a/polygerrit-ui/app/elements/admin/gr-group/gr-group.ts b/polygerrit-ui/app/elements/admin/gr-group/gr-group.ts
index 4311039..f5170ed 100644
--- a/polygerrit-ui/app/elements/admin/gr-group/gr-group.ts
+++ b/polygerrit-ui/app/elements/admin/gr-group/gr-group.ts
@@ -34,7 +34,11 @@
import {GroupId, GroupInfo, GroupName} from '../../../types/common';
import {ErrorCallback} from '../../../services/services/gr-rest-api/gr-rest-api';
import {hasOwnProperty} from '../../../utils/common-util';
-import {firePageError, fireTitleChange} from '../../../utils/event-util';
+import {
+ fireEvent,
+ firePageError,
+ fireTitleChange,
+} from '../../../utils/event-util';
import {appContext} from '../../../services/app-context';
const INTERNAL_GROUP_REGEX = /^[\da-f]{40}$/;
@@ -209,6 +213,7 @@
name: groupName,
external: !this._groupIsInternal,
};
+ fireEvent(this, 'name-changed');
this.dispatchEvent(
new CustomEvent('name-changed', {
detail,
diff --git a/polygerrit-ui/app/elements/admin/gr-permission/gr-permission.ts b/polygerrit-ui/app/elements/admin/gr-permission/gr-permission.ts
index e7dc56e..65a3b00 100644
--- a/polygerrit-ui/app/elements/admin/gr-permission/gr-permission.ts
+++ b/polygerrit-ui/app/elements/admin/gr-permission/gr-permission.ts
@@ -55,6 +55,7 @@
} from '../gr-repo-access/gr-repo-access-interfaces';
import {PolymerDomRepeatEvent} from '../../../types/types';
import {appContext} from '../../../services/app-context';
+import {fireEvent} from '../../../utils/event-util';
const MAX_AUTOCOMPLETE_RESULTS = 20;
@@ -222,9 +223,7 @@
}
this.permission.value.modified = true;
// Allows overall access page to know a change has been made.
- this.dispatchEvent(
- new CustomEvent('access-modified', {bubbles: true, composed: true})
- );
+ fireEvent(this, 'access-modified');
}
_handleRemovePermission() {
@@ -232,18 +231,11 @@
return;
}
if (this.permission.value.added) {
- this.dispatchEvent(
- new CustomEvent('added-permission-removed', {
- bubbles: true,
- composed: true,
- })
- );
+ fireEvent(this, 'added-permission-removed');
}
this._deleted = true;
this.permission.value.deleted = true;
- this.dispatchEvent(
- new CustomEvent('access-modified', {bubbles: true, composed: true})
- );
+ fireEvent(this, 'access-modified');
}
@observe('_rules.splices')
@@ -407,9 +399,7 @@
value.added = true;
// See comment above for why we cannot use "this.set(...)" here.
this.permission.value.rules[groupId] = value;
- this.dispatchEvent(
- new CustomEvent('access-modified', {bubbles: true, composed: true})
- );
+ fireEvent(this, 'access-modified');
}
_computeHasRange(name: string) {
diff --git a/polygerrit-ui/app/elements/admin/gr-rule-editor/gr-rule-editor.ts b/polygerrit-ui/app/elements/admin/gr-rule-editor/gr-rule-editor.ts
index 55bd1a4..13d0e50 100644
--- a/polygerrit-ui/app/elements/admin/gr-rule-editor/gr-rule-editor.ts
+++ b/polygerrit-ui/app/elements/admin/gr-rule-editor/gr-rule-editor.ts
@@ -26,6 +26,7 @@
import {encodeURL, getBaseUrl} from '../../../utils/url-util';
import {AccessPermissionId} from '../../../utils/access-util';
import {property, customElement, observe} from '@polymer/decorators';
+import {fireEvent} from '../../../utils/event-util';
/**
* Fired when the rule has been modified or removed.
@@ -267,15 +268,11 @@
_handleRemoveRule() {
if (!this.rule) return;
if (this.rule.value.added) {
- this.dispatchEvent(
- new CustomEvent('added-rule-removed', {bubbles: true, composed: true})
- );
+ fireEvent(this, 'added-rule-removed');
}
this._deleted = true;
this.rule.value.deleted = true;
- this.dispatchEvent(
- new CustomEvent('access-modified', {bubbles: true, composed: true})
- );
+ fireEvent(this, 'access-modified');
}
_handleUndoRemove() {
@@ -304,9 +301,7 @@
}
this.rule.value.modified = true;
// Allows overall access page to know a change has been made.
- this.dispatchEvent(
- new CustomEvent('access-modified', {bubbles: true, composed: true})
- );
+ fireEvent(this, 'access-modified');
}
_setOriginalRuleValues(value: RuleValue) {
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.ts b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.ts
index 287a34f..9afc5f4 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.ts
@@ -54,6 +54,7 @@
isAttentionSetEnabled,
} from '../../../utils/attention-set-util';
import {CustomKeyboardEvent} from '../../../types/events';
+import {fireEvent} from '../../../utils/event-util';
const NUMBER_FIXED_COLUMNS = 3;
const CLOSED_STATUS = ['MERGED', 'ABANDONED'];
@@ -423,12 +424,7 @@
}
e.preventDefault();
- this.dispatchEvent(
- new CustomEvent('next-page', {
- composed: true,
- bubbles: true,
- })
- );
+ fireEvent(this, 'next-page');
}
_prevPage(e: CustomKeyboardEvent) {
diff --git a/polygerrit-ui/app/elements/change-list/gr-create-change-help/gr-create-change-help.ts b/polygerrit-ui/app/elements/change-list/gr-create-change-help/gr-create-change-help.ts
index 35f3aeb..f320296 100644
--- a/polygerrit-ui/app/elements/change-list/gr-create-change-help/gr-create-change-help.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-create-change-help/gr-create-change-help.ts
@@ -23,6 +23,7 @@
import {PolymerElement} from '@polymer/polymer/polymer-element';
import {customElement} from '@polymer/decorators';
import {htmlTemplate} from './gr-create-change-help_html';
+import {fireEvent} from '../../../utils/event-util';
declare global {
interface HTMLElementTagNameMap {
@@ -43,8 +44,6 @@
*/
_handleCreateTap(e: Event) {
e.preventDefault();
- this.dispatchEvent(
- new CustomEvent('create-tap', {bubbles: true, composed: true})
- );
+ fireEvent(this, 'create-tap');
}
}
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts
index 471f638..069d4a9 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts
@@ -45,6 +45,7 @@
} from '../../../utils/patch-set-util';
import {
changeIsOpen,
+ isOwner,
ListChangesOption,
listChangesOptionsToHex,
} from '../../../utils/change-util';
@@ -66,6 +67,7 @@
ErrorCallback,
} from '../../../services/services/gr-rest-api/gr-rest-api';
import {
+ AccountInfo,
ActionInfo,
ActionNameToActionInfoMap,
BranchName,
@@ -113,7 +115,11 @@
UIActionInfo,
} from '../../shared/gr-js-api-interface/gr-change-actions-js-api';
import {fireAlert} from '../../../utils/event-util';
-import {CODE_REVIEW} from '../../../utils/label-util';
+import {
+ CODE_REVIEW,
+ getApprovalInfo,
+ getVotingRange,
+} from '../../../utils/label-util';
const ERR_BRANCH_EMPTY = 'The destination branch can’t be empty.';
const ERR_COMMIT_EMPTY = 'The commit message can’t be empty.';
@@ -402,6 +408,9 @@
@property({type: Boolean})
_hideQuickApproveAction = false;
+ @property({type: Object})
+ account?: AccountInfo;
+
@property({type: String})
changeNum?: NumericChangeId;
@@ -949,29 +958,38 @@
}
}
// Allow the user to use quick approve to vote the max score on code review
- // even if it is already granted.
+ // even if it is already granted by someone else. Does not apply if the
+ // user owns the change or has already granted the max score themselves.
+ const codeReviewLabel = this.change.labels[CODE_REVIEW];
+ const codeReviewPermittedValues = this.change.permitted_labels[CODE_REVIEW];
if (
!result &&
- this.change.labels[CODE_REVIEW] &&
- this._getLabelStatus(this.change.labels[CODE_REVIEW]) ===
- LabelStatus.OK &&
- this.change.permitted_labels[CODE_REVIEW]
+ codeReviewLabel &&
+ codeReviewPermittedValues &&
+ this.account?._account_id &&
+ isDetailedLabelInfo(codeReviewLabel) &&
+ this._getLabelStatus(codeReviewLabel) === LabelStatus.OK &&
+ !isOwner(this.change, this.account) &&
+ getApprovalInfo(codeReviewLabel, this.account)?.value !==
+ getVotingRange(codeReviewLabel)?.max
) {
result = CODE_REVIEW;
}
if (result) {
- const score = this.change.permitted_labels[result].slice(-1)[0];
const labelInfo = this.change.labels[result];
if (!isDetailedLabelInfo(labelInfo)) {
return null;
}
- const maxScore = Object.keys(labelInfo.values).slice(-1)[0];
- if (score === maxScore) {
+ const permittedValues = this.change.permitted_labels[result];
+ const usersMaxPermittedScore =
+ permittedValues[permittedValues.length - 1];
+ const maxScoreForLabel = getVotingRange(labelInfo)?.max;
+ if (Number(usersMaxPermittedScore) === maxScoreForLabel) {
// Allow quick approve only for maximal score.
return {
label: result,
- score,
+ score: usersMaxPermittedScore,
};
}
}
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.js b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.js
index 3b6a2bf..5a06ceb 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.js
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.js
@@ -21,6 +21,8 @@
import {GerritNav} from '../../core/gr-navigation/gr-navigation.js';
import {getPluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader.js';
import {
+ createAccountWithId,
+ createApproval,
createChange,
createChangeMessages,
createRevisions,
@@ -105,6 +107,9 @@
enabled: true,
},
};
+ element.account = {
+ _account_id: 123,
+ };
sinon.stub(appContext.restApiService, 'getRepoBranches').returns(
Promise.resolve([]));
@@ -1525,9 +1530,6 @@
setup(() => {
element.change = {
current_revision: 'abc1234',
- };
- element.change = {
- current_revision: 'abc1234',
labels: {
foo: {
values: {
@@ -1734,10 +1736,66 @@
};
flush();
const approveButton =
- element.shadowRoot
- .querySelector('gr-button[data-action-key=\'review\']');
+ element.shadowRoot
+ .querySelector('gr-button[data-action-key=\'review\']');
assert.isNotNull(approveButton);
});
+
+ test('not added when the user has already approved', () => {
+ const vote = {
+ ...createApproval(),
+ _account_id: 123,
+ name: 'name',
+ value: 2,
+ };
+ element.change = {
+ current_revision: 'abc1234',
+ labels: {
+ 'Code-Review': {
+ approved: {},
+ values: {
+ ' 0': '',
+ '+1': '',
+ '+2': '',
+ },
+ all: [vote],
+ },
+ },
+ permitted_labels: {
+ 'Code-Review': [' 0', '+1', '+2'],
+ },
+ };
+ flush();
+ const approveButton =
+ element.shadowRoot
+ .querySelector('gr-button[data-action-key=\'review\']');
+ assert.isNull(approveButton);
+ });
+
+ test('not added when user owns the change', () => {
+ element.change = {
+ current_revision: 'abc1234',
+ owner: createAccountWithId(123),
+ labels: {
+ 'Code-Review': {
+ approved: {},
+ values: {
+ ' 0': '',
+ '+1': '',
+ '+2': '',
+ },
+ },
+ },
+ permitted_labels: {
+ 'Code-Review': [' 0', '+1', '+2'],
+ },
+ };
+ flush();
+ const approveButton =
+ element.shadowRoot
+ .querySelector('gr-button[data-action-key=\'review\']');
+ assert.isNull(approveButton);
+ });
});
test('adds download revision action', () => {
diff --git a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.ts b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.ts
index d2bdfd7..36ef429 100644
--- a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.ts
@@ -81,6 +81,7 @@
isSectionSet,
DisplayRules,
} from '../../../utils/change-metadata-util';
+import {fireEvent} from '../../../utils/event-util';
const HASHTAG_ADD_MESSAGE = 'Add Hashtag';
@@ -315,9 +316,7 @@
this._settingTopic = false;
this.set(['change', 'topic'], newTopic);
if (newTopic !== lastTopic) {
- this.dispatchEvent(
- new CustomEvent('topic-changed', {bubbles: true, composed: true})
- );
+ fireEvent(this, 'topic-changed');
}
});
}
@@ -360,12 +359,7 @@
.setChangeHashtag(this.change._number, {add: [newHashtag]})
.then(newHashtag => {
this.set(['change', 'hashtags'], newHashtag);
- this.dispatchEvent(
- new CustomEvent('hashtag-changed', {
- bubbles: true,
- composed: true,
- })
- );
+ fireEvent(this, 'hashtag-changed');
});
}
@@ -516,9 +510,7 @@
.then(() => {
target.disabled = false;
this.set(['change', 'topic'], '');
- this.dispatchEvent(
- new CustomEvent('topic-changed', {bubbles: true, composed: true})
- );
+ fireEvent(this, 'topic-changed');
})
.catch(() => {
target.disabled = false;
diff --git a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_html.ts b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_html.ts
index a2f72b7..3b89a33 100644
--- a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_html.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_html.ts
@@ -49,7 +49,15 @@
pointer-events: none;
}
.hashtagChip {
- margin-bottom: var(--spacing-m);
+ padding-bottom: var(--spacing-s);
+ }
+ /* consistent with section .title, .value */
+ .hashtagChip.new-change-summary-true:not(last-of-type) {
+ padding-bottom: var(--spacing-s);
+ }
+ .hashtagChip.new-change-summary-true:last-of-type {
+ display: inline;
+ vertical-align: top;
}
#externalStyle {
display: block;
@@ -340,6 +348,7 @@
placeholder="[[_computeTopicPlaceholder(_topicReadOnly)]]"
read-only="[[_topicReadOnly]]"
on-changed="_handleTopicChanged"
+ show-as-edit-pencil="[[_isNewChangeSummaryUiEnabled]]"
></gr-editable-label>
</template>
</span>
@@ -377,7 +386,7 @@
<span class="value">
<template is="dom-repeat" items="[[change.hashtags]]">
<gr-linked-chip
- class="hashtagChip"
+ class$="hashtagChip new-change-summary-[[_isNewChangeSummaryUiEnabled]]"
text="[[item]]"
href="[[_computeHashtagUrl(item)]]"
removable="[[!_hashtagReadOnly]]"
@@ -394,6 +403,7 @@
placeholder="[[_computeHashtagPlaceholder(_hashtagReadOnly)]]"
read-only="[[_hashtagReadOnly]]"
on-changed="_handleHashtagChanged"
+ show-as-edit-pencil="[[_isNewChangeSummaryUiEnabled]]"
></gr-editable-label>
</template>
</span>
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
index f00f4eb..df52323 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
@@ -149,8 +149,7 @@
import {GrButton} from '../../shared/gr-button/gr-button';
import {GrMessagesList} from '../gr-messages-list/gr-messages-list';
import {GrThreadList} from '../gr-thread-list/gr-thread-list';
-import {PORTING_COMMENTS_CHANGE_LATENCY_LABEL} from '../../../services/gr-reporting/gr-reporting';
-import {fireAlert, firePageError} from '../../../utils/event-util';
+import {fireAlert, fireEvent, firePageError} from '../../../utils/event-util';
import {KnownExperimentId} from '../../../services/flags/flags';
import {fireTitleChange} from '../../../utils/event-util';
@@ -1616,12 +1615,7 @@
}
this._getLoggedIn().then(isLoggedIn => {
if (!isLoggedIn) {
- this.dispatchEvent(
- new CustomEvent('show-auth-required', {
- composed: true,
- bubbles: true,
- })
- );
+ fireEvent(this, 'show-auth-required');
return;
}
@@ -2085,19 +2079,11 @@
if (!this._changeNum)
throw new Error('missing required changeNum property');
- const portedCommentsPromise = this.$.commentAPI.getPortedComments(
- this._changeNum
- );
- const commentsPromise = this.$.commentAPI
- .loadAll(this._changeNum)
+ return this.$.commentAPI
+ .loadAll(this._changeNum, this._patchRange?.patchNum)
.then(comments => {
- this.reporting.time(PORTING_COMMENTS_CHANGE_LATENCY_LABEL);
this._recomputeComments(comments);
});
- Promise.all([portedCommentsPromise, commentsPromise]).then(() => {
- this.reporting.timeEnd(PORTING_COMMENTS_CHANGE_LATENCY_LABEL);
- });
- return commentsPromise;
}
/**
@@ -2169,12 +2155,7 @@
const loadingFlagSet = detailCompletes
.then(() => {
this._loading = false;
- this.dispatchEvent(
- new CustomEvent('change-details-loaded', {
- bubbles: true,
- composed: true,
- })
- );
+ fireEvent(this, 'change-details-loaded');
})
.then(() => {
this.reporting.timeEnd(CHANGE_RELOAD_TIMING_LABEL);
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.ts
index 7225bb9..cb8ed6a 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.ts
@@ -405,6 +405,7 @@
has-parent="[[hasParent]]"
actions="[[_change.actions]]"
revision-actions="{{_currentRevisionActions}}"
+ account="[[_account]]"
change-num="[[_changeNum]]"
change-status="[[_change.status]]"
commit-num="[[_commitInfo.commit]]"
diff --git a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.ts b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.ts
index 89df252..3c99648 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.ts
+++ b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.ts
@@ -54,6 +54,7 @@
import {DiffViewMode} from '../../../constants/constants';
import {GrButton} from '../../shared/gr-button/gr-button';
import {appContext} from '../../../services/app-context';
+import {fireEvent} from '../../../utils/event-util';
// Maximum length for patch set descriptions.
const PATCH_DESC_MAX_LENGTH = 500;
@@ -190,21 +191,11 @@
}
_expandAllDiffs() {
- this.dispatchEvent(
- new CustomEvent('expand-diffs', {
- composed: true,
- bubbles: true,
- })
- );
+ fireEvent(this, 'expand-diffs');
}
_collapseAllDiffs() {
- this.dispatchEvent(
- new CustomEvent('collapse-diffs', {
- composed: true,
- bubbles: true,
- })
- );
+ fireEvent(this, 'collapse-diffs');
}
_computeExpandedClass(filesExpanded: FilesExpandedState) {
@@ -341,22 +332,12 @@
_handlePrefsTap(e: Event) {
e.preventDefault();
- this.dispatchEvent(
- new CustomEvent('open-diff-prefs', {
- composed: true,
- bubbles: true,
- })
- );
+ fireEvent(this, 'open-diff-prefs');
}
_handleIncludedInTap(e: Event) {
e.preventDefault();
- this.dispatchEvent(
- new CustomEvent('open-included-in-dialog', {
- composed: true,
- bubbles: true,
- })
- );
+ fireEvent(this, 'open-included-in-dialog');
}
_handleDownloadTap(e: Event) {
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
index f3ccd61..fb995b7 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
@@ -72,8 +72,9 @@
import {PolymerSpliceChange} from '@polymer/polymer/interfaces';
import {ChangeComments} from '../../diff/gr-comment-api/gr-comment-api';
import {ParsedChangeInfo} from '../../shared/gr-rest-api-interface/gr-reviewer-updates-parser';
-import {PatchSetFile} from '../../../types/types';
import {CustomKeyboardEvent} from '../../../types/events';
+import {PatchSetFile} from '../../../types/types';
+import {KnownExperimentId} from '../../../services/flags/flags';
export const DEFAULT_NUM_FILES_SHOWN = 200;
@@ -321,10 +322,14 @@
@property({type: Array})
_dynamicPrependedContentEndpoints?: string[];
+ private _isPortingCommentsExperimentEnabled = false;
+
private readonly reporting = appContext.reportingService;
private readonly restApiService = appContext.restApiService;
+ private readonly flagsService = appContext.flagsService;
+
get keyBindings() {
return {
esc: '_handleEscKey',
@@ -367,6 +372,9 @@
/** @override */
attached() {
super.attached();
+ this._isPortingCommentsExperimentEnabled = this.flagsService.isEnabled(
+ KnownExperimentId.PORTING_COMMENTS
+ );
getPluginLoader()
.awaitPluginsLoaded()
.then(() => {
@@ -1544,9 +1552,11 @@
'changeComments, patchRange and diffPrefs must be set'
);
}
+
diffElem.threads = this.changeComments.getThreadsBySideForFile(
file,
- this.patchRange
+ this.patchRange,
+ this._isPortingCommentsExperimentEnabled
);
const promises: Array<Promise<unknown>> = [diffElem.reload()];
if (this._loggedIn && !this.diffPrefs.manual_review) {
@@ -1633,14 +1643,9 @@
return;
}
- // Comments are not returned with the commentSide attribute from
- // the api, but it's necessary to be stored on the diff's
- // comments due to use in the _handleCommentUpdate function.
- // The comment thread already has a side associated with it, so
- // set the comment's side to match.
- threadEl.comments = newComments.map(c =>
- Object.assign(c, {diffSide: threadEl.diffSide})
- );
+ threadEl.comments = newComments.map(c => {
+ return {...c};
+ });
flush();
}
diff --git a/polygerrit-ui/app/elements/change/gr-message/gr-message.ts b/polygerrit-ui/app/elements/change/gr-message/gr-message.ts
index 9e57b63..ba92227 100644
--- a/polygerrit-ui/app/elements/change/gr-message/gr-message.ts
+++ b/polygerrit-ui/app/elements/change/gr-message/gr-message.ts
@@ -44,6 +44,7 @@
import {hasOwnProperty} from '../../../utils/common-util';
import {appContext} from '../../../services/app-context';
import {pluralize} from '../../../utils/string-util';
+import {fireEvent} from '../../../utils/event-util';
const PATCH_SET_PREFIX_PATTERN = /^(?:Uploaded\s*)?(?:P|p)atch (?:S|s)et \d+:\s*(.*)/;
const LABEL_TITLE_SCORE_PATTERN = /^(-?)([A-Za-z0-9-]+?)([+-]\d+)?[.]?$/;
@@ -229,12 +230,7 @@
// or gerrit level events
// emit the event so change-view can also get updated with latest changes
- this.dispatchEvent(
- new CustomEvent('comment-refresh', {
- composed: true,
- bubbles: true,
- })
- );
+ fireEvent(this, 'comment-refresh');
}
_computeMessageContentExpanded(content?: string, tag?: ReviewInputTag) {
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts
index 593d1db..182a296 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts
@@ -106,8 +106,8 @@
import {isAttentionSetEnabled} from '../../../utils/attention-set-util';
import {CODE_REVIEW, getMaxAccounts} from '../../../utils/label-util';
import {isUnresolved} from '../../../utils/comment-util';
-import {fireAlert, fireServerError} from '../../../utils/event-util';
import {pluralize} from '../../../utils/string-util';
+import {fireAlert, fireEvent, fireServerError} from '../../../utils/event-util';
const STORAGE_DEBOUNCE_INTERVAL_MS = 400;
@@ -447,12 +447,7 @@
if (this.restApiService.hasPendingDiffDrafts()) {
this._savingComments = true;
this.restApiService.awaitPendingDiffDrafts().then(() => {
- this.dispatchEvent(
- new CustomEvent('comment-refresh', {
- composed: true,
- bubbles: true,
- })
- );
+ fireEvent(this, 'comment-refresh');
this._savingComments = false;
});
}
@@ -888,9 +883,7 @@
_onAttentionExpandedChange() {
// If the attention-detail section is expanded without dispatching this
// event, then the dialog may expand beyond the screen's bottom border.
- this.dispatchEvent(
- new CustomEvent('iron-resize', {composed: true, bubbles: true})
- );
+ fireEvent(this, 'iron-resize');
}
_showAttentionSummary(config?: ServerInfo, attentionExpanded?: boolean) {
@@ -1361,12 +1354,7 @@
}
_handleHeightChanged() {
- this.dispatchEvent(
- new CustomEvent('autogrow', {
- composed: true,
- bubbles: true,
- })
- );
+ fireEvent(this, 'autogrow');
}
_handleLabelsChanged() {
@@ -1477,12 +1465,7 @@
// or gerrit level events
// emit the event so change-view can also get updated with latest changes
- this.dispatchEvent(
- new CustomEvent('comment-refresh', {
- composed: true,
- bubbles: true,
- })
- );
+ fireEvent(this, 'comment-refresh');
}
reportAttentionSetChanges(
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.js b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.js
index 3379cd4..09f8636 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.js
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.js
@@ -1280,36 +1280,6 @@
});
});
- suite('post review API', () => {
- let startReviewStub;
-
- setup(() => {
- startReviewStub = sinon.stub(
- element.restApiService,
- 'startReview')
- .callsFake(() => Promise.resolve());
- });
-
- test('ready property in review input on start review', () => {
- stubSaveReview(review => {
- assert.isTrue(review.ready);
- return {ready: true};
- });
- return element.send(true, true).then(() => {
- assert.isFalse(startReviewStub.called);
- });
- });
-
- test('no ready property in review input on save review', () => {
- stubSaveReview(review => {
- assert.isUndefined(review.ready);
- });
- return element.send(true, false).then(() => {
- assert.isFalse(startReviewStub.called);
- });
- });
- });
-
suite('start review and save buttons', () => {
let sendStub;
diff --git a/polygerrit-ui/app/elements/change/gr-reviewer-list/gr-reviewer-list.ts b/polygerrit-ui/app/elements/change/gr-reviewer-list/gr-reviewer-list.ts
index a2beb36..254ecca 100644
--- a/polygerrit-ui/app/elements/change/gr-reviewer-list/gr-reviewer-list.ts
+++ b/polygerrit-ui/app/elements/change/gr-reviewer-list/gr-reviewer-list.ts
@@ -42,6 +42,7 @@
import {isRemovableReviewer} from '../../../utils/change-util';
import {ReviewerState} from '../../../constants/constants';
import {appContext} from '../../../services/app-context';
+import {KnownExperimentId} from '../../../services/flags/flags';
@customElement('gr-reviewer-list')
export class GrReviewerList extends GestureEventListeners(
@@ -87,8 +88,21 @@
@property({type: Object})
_xhrPromise?: Promise<Response | undefined>;
+ @property({type: Boolean})
+ _isNewChangeSummaryUiEnabled = false;
+
private readonly restApiService = appContext.restApiService;
+ private readonly flagsService = appContext.flagsService;
+
+ /** @override */
+ ready() {
+ super.ready();
+ this._isNewChangeSummaryUiEnabled = this.flagsService.isEnabled(
+ KnownExperimentId.NEW_CHANGE_SUMMARY_UI
+ );
+ }
+
@computed('ccsOnly')
get _addLabel() {
return this.ccsOnly ? 'Add CC' : 'Add reviewer';
diff --git a/polygerrit-ui/app/elements/change/gr-reviewer-list/gr-reviewer-list_html.ts b/polygerrit-ui/app/elements/change/gr-reviewer-list/gr-reviewer-list_html.ts
index dd18d03..ccdcf8d 100644
--- a/polygerrit-ui/app/elements/change/gr-reviewer-list/gr-reviewer-list_html.ts
+++ b/polygerrit-ui/app/elements/change/gr-reviewer-list/gr-reviewer-list_html.ts
@@ -28,6 +28,16 @@
.container {
display: block;
}
+ .addReviewer iron-icon {
+ color: inherit;
+ --iron-icon-height: 18px;
+ --iron-icon-width: 18px;
+ }
+ gr-button.addReviewer.new-change-summary-true {
+ --padding: 1px 4px;
+ vertical-align: top;
+ top: 1px;
+ }
gr-button {
--gr-button: {
padding: 0px 0px;
@@ -51,6 +61,16 @@
>
</gr-account-chip>
</template>
+ <template is="dom-if" if="[[_isNewChangeSummaryUiEnabled]]">
+ <gr-button
+ link=""
+ id="addReviewer"
+ class="addReviewer new-change-summary-true"
+ on-click="_handleAddTap"
+ title="[[_addLabel]]"
+ ><iron-icon icon="gr-icons:edit"></iron-icon
+ ></gr-button>
+ </template>
</div>
<gr-button
class="hiddenReviewers"
@@ -59,14 +79,16 @@
on-click="_handleViewAll"
>and [[_hiddenReviewerCount]] more</gr-button
>
- <div class="controlsContainer" hidden$="[[!mutable]]">
- <gr-button
- link=""
- id="addReviewer"
- class="addReviewer"
- on-click="_handleAddTap"
- >[[_addLabel]]</gr-button
- >
- </div>
+ <template is="dom-if" if="[[!_isNewChangeSummaryUiEnabled]]">
+ <div class="controlsContainer" hidden$="[[!mutable]]">
+ <gr-button
+ link=""
+ id="addReviewer"
+ class="addReviewer"
+ on-click="_handleAddTap"
+ >[[_addLabel]]</gr-button
+ >
+ </div>
+ </template>
</div>
`;
diff --git a/polygerrit-ui/app/elements/change/gr-reviewer-list/gr-reviewer-list_test.js b/polygerrit-ui/app/elements/change/gr-reviewer-list/gr-reviewer-list_test.js
index d29abfc..89332c2 100644
--- a/polygerrit-ui/app/elements/change/gr-reviewer-list/gr-reviewer-list_test.js
+++ b/polygerrit-ui/app/elements/change/gr-reviewer-list/gr-reviewer-list_test.js
@@ -36,6 +36,7 @@
});
test('controls hidden on immutable element', () => {
+ flush();
element.mutable = false;
assert.isTrue(element.shadowRoot
.querySelector('.controlsContainer').hasAttribute('hidden'));
@@ -48,6 +49,7 @@
element.addEventListener('show-reply-dialog', () => {
done();
});
+ flush();
MockInteractions.tap(element.shadowRoot
.querySelector('.addReviewer'));
});
diff --git a/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list_html.ts b/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list_html.ts
index e62d481..2c21b4a 100644
--- a/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list_html.ts
+++ b/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list_html.ts
@@ -127,6 +127,7 @@
</template>
<gr-comment-thread
show-file-path=""
+ show-ported-comment="[[thread.ported]]"
change-num="[[changeNum]]"
comments="[[thread.comments]]"
diff-side="[[thread.diffSide]]"
diff --git a/polygerrit-ui/app/elements/core/gr-account-dropdown/gr-account-dropdown.ts b/polygerrit-ui/app/elements/core/gr-account-dropdown/gr-account-dropdown.ts
index 6b7e0b6..0361b9a 100644
--- a/polygerrit-ui/app/elements/core/gr-account-dropdown/gr-account-dropdown.ts
+++ b/polygerrit-ui/app/elements/core/gr-account-dropdown/gr-account-dropdown.ts
@@ -26,6 +26,7 @@
import {customElement, property} from '@polymer/decorators';
import {AccountInfo, ServerInfo} from '../../../types/common';
import {appContext} from '../../../services/app-context';
+import {fireEvent} from '../../../utils/event-util';
const INTERPOLATE_URL_PATTERN = /\${([\w]+)}/g;
@@ -115,12 +116,7 @@
}
_handleShortcutsTap() {
- this.dispatchEvent(
- new CustomEvent('show-keyboard-shortcuts', {
- bubbles: true,
- composed: true,
- })
- );
+ fireEvent(this, 'show-keyboard-shortcuts');
}
_handleLocationChange() {
diff --git a/polygerrit-ui/app/elements/core/gr-key-binding-display/gr-key-binding-display.css.ts b/polygerrit-ui/app/elements/core/gr-key-binding-display/gr-key-binding-display.css.ts
deleted file mode 100644
index ccba289..0000000
--- a/polygerrit-ui/app/elements/core/gr-key-binding-display/gr-key-binding-display.css.ts
+++ /dev/null
@@ -1,30 +0,0 @@
-/**
- * @license
- * Copyright (C) 2020 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-import {css} from 'lit-element';
-
-export const cssTemplate = css`
- .key {
- background-color: var(--chip-background-color);
- color: var(--primary-text-color);
- border: 1px solid var(--border-color);
- border-radius: var(--border-radius);
- display: inline-block;
- font-weight: var(--font-weight-bold);
- padding: var(--spacing-xxs) var(--spacing-m);
- text-align: center;
- }
-`;
diff --git a/polygerrit-ui/app/elements/core/gr-key-binding-display/gr-key-binding-display.ts b/polygerrit-ui/app/elements/core/gr-key-binding-display/gr-key-binding-display.ts
index 091684f..796a167 100644
--- a/polygerrit-ui/app/elements/core/gr-key-binding-display/gr-key-binding-display.ts
+++ b/polygerrit-ui/app/elements/core/gr-key-binding-display/gr-key-binding-display.ts
@@ -14,11 +14,12 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-import {html} from 'lit-html';
-import {GrLitElement} from '../../lit/gr-lit-element';
-import {customElement, property} from 'lit-element';
-import {cssTemplate} from './gr-key-binding-display.css';
-import {sharedStyles} from '../../../styles/shared-styles';
+import '../../../styles/shared-styles';
+import {GestureEventListeners} from '@polymer/polymer/lib/mixins/gesture-event-listeners';
+import {LegacyElementMixin} from '@polymer/polymer/lib/legacy/legacy-element-mixin';
+import {PolymerElement} from '@polymer/polymer/polymer-element';
+import {htmlTemplate} from './gr-key-binding-display_html';
+import {customElement, property} from '@polymer/decorators';
declare global {
interface HTMLElementTagNameMap {
@@ -27,30 +28,21 @@
}
@customElement('gr-key-binding-display')
-export class GrKeyBindingDisplay extends GrLitElement {
- static get styles() {
- return [sharedStyles, cssTemplate];
- }
-
- render() {
- const items = this.binding.map((binding, index) => [
- index > 0 ? html` or ` : html``,
- this._computeModifiers(binding).map(
- modifier => html`<span class="key modifier">${modifier}</span> `
- ),
- html`<span class="key">${this._computeKey(binding)}</span>`,
- ]);
- return html`${items}`;
+export class GrKeyBindingDisplay extends GestureEventListeners(
+ LegacyElementMixin(PolymerElement)
+) {
+ static get template() {
+ return htmlTemplate;
}
@property({type: Array})
binding: string[][] = [];
- _computeModifiers(binding: string[]) {
+ _computeModifiers(binding: string[][]) {
return binding.slice(0, binding.length - 1);
}
- _computeKey(binding: string[]) {
+ _computeKey(binding: string[][]) {
return binding[binding.length - 1];
}
}
diff --git a/polygerrit-ui/app/elements/core/gr-key-binding-display/gr-key-binding-display_html.ts b/polygerrit-ui/app/elements/core/gr-key-binding-display/gr-key-binding-display_html.ts
new file mode 100644
index 0000000..251e30f
--- /dev/null
+++ b/polygerrit-ui/app/elements/core/gr-key-binding-display/gr-key-binding-display_html.ts
@@ -0,0 +1,41 @@
+/**
+ * @license
+ * Copyright (C) 2020 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+import {html} from '@polymer/polymer/lib/utils/html-tag';
+
+export const htmlTemplate = html`
+ <style include="shared-styles">
+ .key {
+ background-color: var(--chip-background-color);
+ color: var(--primary-text-color);
+ border: 1px solid var(--border-color);
+ border-radius: var(--border-radius);
+ display: inline-block;
+ font-weight: var(--font-weight-bold);
+ padding: var(--spacing-xxs) var(--spacing-m);
+ text-align: center;
+ }
+ </style>
+ <template is="dom-repeat" items="[[binding]]">
+ <template is="dom-if" if="[[index]]">
+ or
+ </template>
+ <template is="dom-repeat" items="[[_computeModifiers(item)]]" as="modifier">
+ <span class="key modifier">[[modifier]]</span>
+ </template>
+ <span class="key">[[_computeKey(item)]]</span>
+ </template>
+`;
diff --git a/polygerrit-ui/app/elements/core/gr-key-binding-display/gr-key-binding-display_test.ts b/polygerrit-ui/app/elements/core/gr-key-binding-display/gr-key-binding-display_test.js
similarity index 86%
rename from polygerrit-ui/app/elements/core/gr-key-binding-display/gr-key-binding-display_test.ts
rename to polygerrit-ui/app/elements/core/gr-key-binding-display/gr-key-binding-display_test.js
index 875cde8..0c25e6e 100644
--- a/polygerrit-ui/app/elements/core/gr-key-binding-display/gr-key-binding-display_test.ts
+++ b/polygerrit-ui/app/elements/core/gr-key-binding-display/gr-key-binding-display_test.js
@@ -17,12 +17,11 @@
import '../../../test/common-test-setup-karma.js';
import './gr-key-binding-display.js';
-import {GrKeyBindingDisplay} from './gr-key-binding-display.js';
const basicFixture = fixtureFromElement('gr-key-binding-display');
suite('gr-key-binding-display tests', () => {
- let element: GrKeyBindingDisplay;
+ let element;
setup(() => {
element = basicFixture.instantiate();
@@ -46,10 +45,10 @@
test('key with modifiers', () => {
assert.deepEqual(element._computeModifiers(['Ctrl', 'x']), ['Ctrl']);
- assert.deepEqual(element._computeModifiers(['Shift', 'Meta', 'x']), [
- 'Shift',
- 'Meta',
- ]);
+ assert.deepEqual(
+ element._computeModifiers(['Shift', 'Meta', 'x']),
+ ['Shift', 'Meta']);
});
});
});
+
diff --git a/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog.ts b/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog.ts
index d45c8e6..43b0588 100644
--- a/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog.ts
+++ b/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog.ts
@@ -39,6 +39,7 @@
import {isRobot} from '../../../utils/comment-util';
import {OpenFixPreviewEvent} from '../../../types/events';
import {appContext} from '../../../services/app-context';
+import {fireEvent} from '../../../utils/event-util';
export interface GrApplyFixDialog {
$: {
@@ -129,12 +130,7 @@
);
return Promise.all(promises).then(() => {
// ensures gr-overlay repositions overlay in center
- this.$.applyFixOverlay.dispatchEvent(
- new CustomEvent('iron-resize', {
- composed: true,
- bubbles: true,
- })
- );
+ fireEvent(this.$.applyFixOverlay, 'iron-resize');
});
}
@@ -142,12 +138,7 @@
super.attached();
this.refitOverlay = () => {
// re-center the dialog as content changed
- this.$.applyFixOverlay.dispatchEvent(
- new CustomEvent('iron-resize', {
- composed: true,
- bubbles: true,
- })
- );
+ fireEvent(this.$.applyFixOverlay, 'iron-resize');
};
this.addEventListener('diff-context-expanded', this.refitOverlay);
}
@@ -242,12 +233,7 @@
this._currentPreviews = [];
this._isApplyFixLoading = false;
- this.dispatchEvent(
- new CustomEvent('close-fix-preview', {
- bubbles: true,
- composed: true,
- })
- );
+ fireEvent(this, 'close-fix-preview');
this.$.applyFixOverlay.close();
}
diff --git a/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api.ts b/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api.ts
index 1f84680..cb90f20 100644
--- a/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api.ts
+++ b/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api.ts
@@ -28,7 +28,7 @@
RobotCommentInfo,
UrlEncodedCommentId,
NumericChangeId,
- RevisionId,
+ PathToCommentsInfoMap,
} from '../../../types/common';
import {hasOwnProperty} from '../../../utils/common-util';
import {
@@ -43,9 +43,13 @@
UIRobot,
createCommentThreads,
isInPatchRange,
+ isDraftThread,
+ isInBaseOfPatchRange,
+ isInRevisionOfPatchRange,
} from '../../../utils/comment-util';
import {PatchSetFile, PatchNumOnly, isPatchSetFile} from '../../../types/types';
import {appContext} from '../../../services/app-context';
+import {CommentSide, Side} from '../../../constants/constants';
export type CommentIdToCommentThreadMap = {
[urlEncodedCommentId: string]: CommentThread;
@@ -58,6 +62,10 @@
private readonly _drafts: {[path: string]: UIDraft[]};
+ private readonly _portedComments: PathToCommentsInfoMap;
+
+ private readonly _portedDrafts: PathToCommentsInfoMap;
+
/**
* Construct a change comments object, which can be data-bound to child
* elements of that which uses the gr-comment-api.
@@ -65,11 +73,15 @@
constructor(
comments: {[path: string]: UIHuman[]} | undefined,
robotComments: {[path: string]: UIRobot[]} | undefined,
- drafts: {[path: string]: UIDraft[]} | undefined
+ drafts: {[path: string]: UIDraft[]} | undefined,
+ portedComments: PathToCommentsInfoMap | undefined,
+ portedDrafts: PathToCommentsInfoMap | undefined
) {
this._comments = this._addPath(comments);
this._robotComments = this._addPath(robotComments);
this._drafts = this._addPath(drafts);
+ this._portedComments = portedComments || {};
+ this._portedDrafts = portedDrafts || {};
}
/**
@@ -94,18 +106,10 @@
return updatedComments;
}
- get comments() {
- return this._comments;
- }
-
get drafts() {
return this._drafts;
}
- get robotComments() {
- return this._robotComments;
- }
-
findCommentById(commentId?: UrlEncodedCommentId): UIComment | undefined {
if (!commentId) return undefined;
const findComment = (comments: {[path: string]: UIComment[]}) => {
@@ -135,9 +139,9 @@
*/
getPaths(patchRange?: PatchRange): CommentMap {
const responses: {[path: string]: UIComment[]}[] = [
- this.comments,
+ this._comments,
this.drafts,
- this.robotComments,
+ this._robotComments,
];
const commentMap: CommentMap = {};
for (const response of responses) {
@@ -258,6 +262,16 @@
return allComments;
}
+ cloneWithUpdatedDrafts(drafts: {[path: string]: UIDraft[]} | undefined) {
+ return new ChangeComments(
+ this._comments,
+ this._robotComments,
+ drafts,
+ this._portedComments,
+ this._portedDrafts
+ );
+ }
+
/**
* Get the drafts for a path and optional patch num.
*
@@ -289,16 +303,6 @@
return allDrafts;
}
- getThreadsBySideForPath(
- path: string,
- patchRange: PatchRange
- ): CommentThread[] {
- return createCommentThreads(
- this.getCommentsForPath(path, patchRange),
- patchRange
- );
- }
-
/**
* Get the comments (with drafts and robot comments) for a path and
* patch-range. Returns an object with left and right properties mapping to
@@ -313,14 +317,14 @@
let comments: Comment[] = [];
let drafts: DraftInfo[] = [];
let robotComments: RobotCommentInfo[] = [];
- if (this.comments && this.comments[path]) {
- comments = this.comments[path];
+ if (this._comments && this._comments[path]) {
+ comments = this._comments[path];
}
if (this.drafts && this.drafts[path]) {
drafts = this.drafts[path];
}
- if (this.robotComments && this.robotComments[path]) {
- robotComments = this.robotComments[path];
+ if (this._robotComments && this._robotComments[path]) {
+ robotComments = this._robotComments[path];
}
drafts.forEach(d => {
@@ -336,14 +340,97 @@
});
}
- getThreadsBySideForFile(
+ /**
+ * Get the ported threads for given patch range.
+ * Ported threads are comment threads that were posted on an older patchset
+ * and are displayed on a later patchset.
+ * It is simply the original thread displayed on a newer patchset.
+ *
+ * Threads are ported over to all subsequent patchsets. So, a thread created
+ * on patchset 5 say will be ported over to patchsets 6,7,8 and beyond.
+ *
+ * Ported threads add a boolean property ported true to the thread object
+ * to indicate to the user that this is a ported thread.
+ *
+ * Any interactions with ported threads are reflected on the original threads.
+ * Replying to a ported thread ported from Patchset 6 shown on Patchset 10
+ * say creates a draft reply associated with Patchset 6, since the user is
+ * interacting with the original thread.
+ *
+ * Only threads with unresolved comments or drafts are ported over.
+ * If the thread is associated with either the left patchset or the right
+ * patchset, then we filter that ported thread from the return value
+ * as it will be rendered by default.
+ *
+ * If there is no appropriate range for the ported comments, then the backend
+ * does not return the range of the ported thread and it becomes a file level
+ * thread.
+ *
+ * @return only the ported threads for the specified file and patch range
+ */
+ _getPortedCommentThreads(
file: PatchSetFile,
patchRange: PatchRange
): CommentThread[] {
- return createCommentThreads(
+ const portedComments = this._portedComments[file.path] || [];
+ portedComments.push(...(this._portedDrafts[file.path] || []));
+ if (file.basePath) {
+ portedComments.push(...(this._portedComments[file.basePath] || []));
+ portedComments.push(...(this._portedDrafts[file.basePath] || []));
+ }
+ if (!portedComments.length) return [];
+
+ // when forming threads in diff view, we filter for current patchrange but
+ // ported comments will involve comments that may not belong to the
+ // current patchrange, so we need to form threads for them using all
+ // comments
+ const allComments: UIComment[] = this.getAllCommentsForFile(file, true);
+
+ return createCommentThreads(allComments).filter(thread => {
+ // Robot comments and drafts are not ported over. A human reply to
+ // the robot comment will be ported over, thefore it's possible to
+ // have the root comment of the thread not be ported, hence loop over
+ // entire thread
+ const portedComment = portedComments.find(portedComment =>
+ thread.comments.some(c => portedComment.id === c.id)
+ );
+ if (!portedComment) return false;
+
+ if (
+ isInBaseOfPatchRange(thread.comments[0], patchRange) ||
+ isInRevisionOfPatchRange(thread.comments[0], patchRange)
+ ) {
+ // no need to port this thread as it will be rendered by default
+ return false;
+ }
+
+ // TODO(dhruvsri): Add handling for thread.commentSide = PARENT
+ if (thread.commentSide === CommentSide.PARENT) return false;
+
+ if (!isUnresolved(thread) && !isDraftThread(thread)) return false;
+
+ thread.range = portedComment.range;
+ thread.line = portedComment.line;
+ thread.ported = true;
+ thread.diffSide = Side.RIGHT;
+ return true;
+ });
+ }
+
+ getThreadsBySideForFile(
+ file: PatchSetFile,
+ patchRange: PatchRange,
+ includePortedComments?: boolean
+ ): CommentThread[] {
+ const threads = createCommentThreads(
this.getCommentsForFile(file, patchRange),
patchRange
);
+
+ if (includePortedComments) {
+ threads.push(...this._getPortedCommentThreads(file, patchRange));
+ }
+ return threads;
}
/**
@@ -451,41 +538,40 @@
super.created();
}
- getPortedComments(changeNum: NumericChangeId, revision?: RevisionId) {
- if (!revision) revision = CURRENT;
- return Promise.all([
- this.restApiService.getPortedComments(changeNum, revision),
- this.restApiService.getPortedDrafts(changeNum, revision),
- ]).then(result => {
- return {
- portedComments: result[0],
- portedDrafts: result[1],
- };
- });
- }
-
/**
* Load all comments (with drafts and robot comments) for the given change
* number. The returned promise resolves when the comments have loaded, but
* does not yield the comment data.
*/
- loadAll(changeNum: NumericChangeId) {
- const promises = [];
- promises.push(this.restApiService.getDiffComments(changeNum));
- promises.push(this.restApiService.getDiffRobotComments(changeNum));
- promises.push(this.restApiService.getDiffDrafts(changeNum));
+ loadAll(changeNum: NumericChangeId, patchNum?: PatchSetNum) {
+ const revision = patchNum || CURRENT;
+ const commentsPromise: [
+ Promise<PathToCommentsInfoMap | undefined>,
+ Promise<PathToRobotCommentsInfoMap | undefined>,
+ Promise<PathToCommentsInfoMap | undefined>,
+ Promise<PathToCommentsInfoMap | undefined>,
+ Promise<PathToCommentsInfoMap | undefined>
+ ] = [
+ this.restApiService.getDiffComments(changeNum),
+ this.restApiService.getDiffRobotComments(changeNum),
+ this.restApiService.getDiffDrafts(changeNum),
+ this.restApiService.getPortedComments(changeNum, revision),
+ this.restApiService.getPortedDrafts(changeNum, revision),
+ ];
- return Promise.all(promises).then(([comments, robotComments, drafts]) => {
- this._changeComments = new ChangeComments(
- comments,
- // TODO(TS): Promise.all somehow resolve all types to
- // PathToCommentsInfoMap given its PathToRobotCommentsInfoMap
- // returned from the second promise
- robotComments as PathToRobotCommentsInfoMap,
- drafts
- );
- return this._changeComments;
- });
+ return Promise.all(commentsPromise).then(
+ ([comments, robotComments, drafts, portedComments, portedDrafts]) => {
+ this._changeComments = new ChangeComments(
+ comments,
+ // TS 4.0.5 fails without 'as'
+ robotComments as PathToRobotCommentsInfoMap | undefined,
+ drafts,
+ portedComments,
+ portedDrafts
+ );
+ return this._changeComments;
+ }
+ );
}
/**
@@ -497,11 +583,8 @@
if (!this._changeComments) {
return this.loadAll(changeNum);
}
- const oldChangeComments = this._changeComments;
return this.restApiService.getDiffDrafts(changeNum).then(drafts => {
- this._changeComments = new ChangeComments(
- oldChangeComments.comments,
- (oldChangeComments.robotComments as unknown) as PathToRobotCommentsInfoMap,
+ this._changeComments = this._changeComments!.cloneWithUpdatedDrafts(
drafts
);
return this._changeComments;
diff --git a/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api_test.js b/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api_test.js
index 6b6756e..c9bce3d 100644
--- a/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api_test.js
+++ b/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api_test.js
@@ -19,7 +19,8 @@
import './gr-comment-api.js';
import {ChangeComments} from './gr-comment-api.js';
import {CommentSide} from '../../../constants/constants.js';
-import {isInRevisionOfPatchRange, isInBaseOfPatchRange} from '../../../utils/comment-util.js';
+import {isInRevisionOfPatchRange, isInBaseOfPatchRange, createCommentThreads, isDraftThread, isUnresolved} from '../../../utils/comment-util.js';
+import {createDraft, createComment} from '../../../test/test-data-generators.js';
const basicFixture = fixtureFromElement('gr-comment-api');
@@ -147,7 +148,183 @@
});
});
- test('isInBaseOfPatchRange', () => {
+ suite('ported comments', () => {
+ let portedComments;
+ let changeComments;
+ const comment1 = {
+ ...createComment(),
+ unresolved: true,
+ id: 'db977012_e1f13818',
+ line: 136,
+ patch_set: 2,
+ range: {
+ start_line: 1,
+ start_character: 1,
+ end_line: 1,
+ end_character: 1,
+ },
+ };
+
+ const comment2 = {
+ ...createComment(),
+ patch_set: 2,
+ id: 'ecf0b9fa_fe1a5f62',
+ line: 5,
+ };
+
+ const draft1 = {
+ ...createDraft(),
+ id: 'db977012_e1f13828',
+ line: 4,
+ patch_set: 2,
+ };
+ const draft2 = {
+ ...createDraft(),
+ id: '503008e2_0ab203ee',
+ line: 11,
+ unresolved: true,
+ // slightly larger timestamp so it's sorted higher
+ updated: '2018-02-13 22:49:48.018000001',
+ patch_set: 2,
+ };
+
+ setup(() => {
+ portedComments = {
+ 'karma.conf.js': [{
+ ...comment1,
+ patchNum: 4,
+ range: {
+ start_line: 136,
+ start_character: 16,
+ end_line: 136,
+ end_character: 29,
+ },
+ }],
+ };
+
+ changeComments = new ChangeComments(
+ {/* comments */
+ 'karma.conf.js': [
+ // resolved comment that will not be ported over
+ comment2,
+ // original comment that will be ported over to patchset 4
+ comment1,
+ ],
+ },
+ {}/* robot comments */,
+ {}/* drafts */,
+ portedComments,
+ {}/* ported drafts */
+ );
+ });
+
+ test('threads containing ported comment are returned', () => {
+ assert.equal(changeComments.getAllThreadsForChange().length,
+ 2);
+
+ const portedThreads = changeComments._getPortedCommentThreads(
+ {path: 'karma.conf.js'}, {patchNum: 4, basePatchNum: 'PARENT'});
+
+ assert.equal(portedThreads.length, 1);
+ // check range of thread is from the ported comment and not the original
+ assert.deepEqual(portedThreads[0].range, {
+ start_line: 136,
+ start_character: 16,
+ end_line: 136,
+ end_character: 29,
+ });
+
+ // thread ported over if comparing patchset 1 vs patchset 4
+ assert.equal(changeComments._getPortedCommentThreads(
+ {path: 'karma.conf.js'}, {patchNum: 4, basePatchNum: 1}
+ ).length, 1);
+
+ // verify ported thread is not returned if original thread will be
+ // shown
+ // original thread attached to right side
+ assert.equal(changeComments._getPortedCommentThreads(
+ {path: 'karma.conf.js'}, {patchNum: 2, basePatchNum: 'PARENT'}
+ ).length, 0);
+ assert.equal(changeComments._getPortedCommentThreads(
+ {path: 'karma.conf.js'}, {patchNum: 2, basePatchNum: 1}
+ ).length, 0);
+
+ // original thread attached to left side
+ assert.equal(changeComments._getPortedCommentThreads(
+ {path: 'karma.conf.js'}, {patchNum: 3, basePatchNum: 2}
+ ).length, 0);
+ });
+
+ test('threads without any ported comment are filtered out', () => {
+ changeComments = new ChangeComments(
+ {/* comments */
+ // comment that is not ported over
+ 'karma.conf.js': [comment2],
+ },
+ {}/* robot comments */,
+ {/* drafts */
+ 'karma.conf.js': [draft2],
+ },
+ // comment1 that is ported over but does not have any thread
+ // that has a comment that matches it
+ portedComments,
+ {}/* ported drafts */
+ );
+
+ assert.equal(createCommentThreads(changeComments
+ .getAllCommentsForPath('karma.conf.js')).length, 1);
+ assert.equal(changeComments._getPortedCommentThreads(
+ {path: 'karma.conf.js'}, {patchNum: 4, basePatchNum: 'PARENT'}
+ ).length, 0);
+ });
+
+ test('drafts are ported over', () => {
+ changeComments = new ChangeComments(
+ {}/* comments */,
+ {}/* robotComments */,
+ {/* drafts */
+ // draft1: resolved draft that will be ported over to ps 4
+ // draft2: unresolved draft that will be ported over to ps 4
+ 'karma.conf.js': [draft1, draft2],
+ },
+ {}/* ported comments */,
+ {/* ported drafts */
+ 'karma.conf.js': [
+ {
+ ...draft1,
+ line: 5,
+ patch_set: 4,
+ },
+ {
+ ...draft2,
+ line: 31,
+ patch_set: 4,
+ },
+ ],
+ }
+ );
+
+ const portedThreads = changeComments._getPortedCommentThreads(
+ {path: 'karma.conf.js'}, {patchNum: 4, basePatchNum: 'PARENT'});
+
+ // resolved draft is ported over
+ assert.equal(portedThreads.length, 2);
+ assert.equal(portedThreads[0].line, 5);
+ assert.isTrue(isDraftThread(portedThreads[0]));
+ assert.isFalse(isUnresolved(portedThreads[0]));
+
+ // unresolved draft is ported over
+ assert.equal(portedThreads[1].line, 31);
+ assert.isTrue(isDraftThread(portedThreads[1]));
+ assert.isTrue(isUnresolved(portedThreads[1]));
+
+ assert.equal(createCommentThreads(
+ changeComments.getAllCommentsForPath('karma.conf.js'),
+ {patchNum: 4, basePatchNum: 'PARENT'}).length, 0);
+ });
+ });
+
+ test('_isInBaseOfPatchRange', () => {
const comment = {patch_set: 1};
const patchRange = {basePatchNum: 1, patchNum: 2};
assert.isTrue(isInBaseOfPatchRange(comment,
@@ -296,7 +473,7 @@
],
};
element._changeComments =
- new ChangeComments(comments, robotComments, drafts, 1234);
+ new ChangeComments(comments, robotComments, drafts, {}, 1234);
});
test('getPaths', () => {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-element.ts b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-element.ts
index 912d59e..6e613d3 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-element.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-element.ts
@@ -48,7 +48,7 @@
import {GrDiffGroup} from '../gr-diff/gr-diff-group';
import {PolymerSpliceChange} from '@polymer/polymer/interfaces';
import {getLineNumber} from '../gr-diff/gr-diff-utils';
-import {fireAlert} from '../../../utils/event-util';
+import {fireAlert, fireEvent} from '../../../utils/event-util';
const DiffViewMode = {
SIDE_BY_SIDE: 'SIDE_BY_SIDE',
@@ -219,17 +219,13 @@
const isBinary = !!(this.isImageDiff || this.diff.binary);
- this.dispatchEvent(
- new CustomEvent('render-start', {bubbles: true, composed: true})
- );
+ fireEvent(this, 'render-start');
this._cancelableRenderPromise = util.makeCancelable(
this.$.processor.process(this.diff.content, isBinary).then(() => {
if (this.isImageDiff) {
(this._builder as GrDiffBuilderImage).renderDiff();
}
- this.dispatchEvent(
- new CustomEvent('render-content', {bubbles: true, composed: true})
- );
+ fireEvent(this, 'render-content');
})
);
return (
@@ -336,16 +332,7 @@
sectionEl.parentNode.removeChild(sectionEl);
}
- this.async(
- () =>
- this.dispatchEvent(
- new CustomEvent('render-content', {
- composed: true,
- bubbles: true,
- })
- ),
- 1
- );
+ this.async(() => fireEvent(this, 'render-content'), 1);
}
cancel() {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.ts b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.ts
index 43ed77f..52ac7cc 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.ts
@@ -36,7 +36,7 @@
import {PolymerDomWrapper} from '../../../types/types';
import {GrDiffGroupType} from '../gr-diff/gr-diff-group';
import {GrDiff} from '../gr-diff/gr-diff';
-import {fireAlert} from '../../../utils/event-util';
+import {fireAlert, fireEvent} from '../../../utils/event-util';
const DiffViewMode = {
SIDE_BY_SIDE: 'SIDE_BY_SIDE',
@@ -220,12 +220,7 @@
) {
// reset for next file
this.lastDisplayedNavigateToNextFileToast = null;
- this.dispatchEvent(
- new CustomEvent('navigate-to-next-unreviewed-file', {
- composed: true,
- bubbles: true,
- })
- );
+ fireEvent(this, 'navigate-to-next-unreviewed-file');
} else {
this.lastDisplayedNavigateToNextFileToast = Date.now();
fireAlert(this, 'Press n again to navigate to next unreviewed file');
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts
index 3c401d0..d084457 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts
@@ -72,6 +72,7 @@
firePageError,
fireAlert,
fireServerError,
+ fireEvent,
} from '../../../utils/event-util';
const MSG_EMPTY_BLAME = 'No blame information for this diff.';
@@ -778,6 +779,7 @@
threadEl.changeNum = this.changeNum;
threadEl.patchNum = thread.patchNum;
threadEl.showPatchset = false;
+ threadEl.showPortedComment = !!thread.ported;
// GrCommentThread does not understand 'FILE', but requires undefined.
threadEl.lineNum = thread.line !== 'FILE' ? thread.line : undefined;
threadEl.projectName = this.projectName;
@@ -913,9 +915,7 @@
}
_handleCommentSaveOrDiscard() {
- this.dispatchEvent(
- new CustomEvent('diff-comments-modified', {bubbles: true, composed: true})
- );
+ fireEvent(this, 'diff-comments-modified');
}
_isSyntaxHighlightingEnabled(
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.js b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.js
index 9bde122..2c050f9 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.js
@@ -24,6 +24,7 @@
import {Side, CommentSide} from '../../../constants/constants.js';
import {createChange} from '../../../test/test-data-generators.js';
import {FILE} from '../gr-diff/gr-diff-line.js';
+import {CoverageType} from '../../../types/types.js';
const basicFixture = fixtureFromElement('gr-diff-host');
@@ -1003,14 +1004,14 @@
assert.equal(actualThreads.length, 2);
- assert.equal(actualThreads[0].diffSide, 'left');
+ assert.equal(actualThreads[0].diffSide, Side.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, 1);
assert.equal(actualThreads[0].line, 1);
- assert.equal(actualThreads[1].diffSide, 'left');
+ assert.equal(actualThreads[1].diffSide, Side.LEFT);
assert.equal(actualThreads[1].comments.length, 1);
assert.deepEqual(actualThreads[1].comments[0], comments[2]);
assert.equal(actualThreads[1].patchNum, 1);
@@ -1035,7 +1036,7 @@
const expectedThreads = [
{
- diffSide: 'left',
+ diffSide: Side.LEFT,
commentSide: CommentSide.REVISION,
path: '/p',
rootId: 'betsys_confession',
@@ -1090,7 +1091,7 @@
});
test('_getOrCreateThread', () => {
- const diffSide = 'left';
+ const diffSide = Side.LEFT;
const commentSide = CommentSide.PARENT;
assert.isOk(element._getOrCreateThread('2', 3,
@@ -1126,7 +1127,7 @@
test('thread should use old file path if first created ' +
'on patch set (left) before renaming', () => {
- const diffSide = 'left';
+ const diffSide = Side.LEFT;
element.file = {basePath: 'file_renamed.txt', path: element.path};
assert.isOk(element._getOrCreateThread('2', 3,
@@ -1142,7 +1143,7 @@
test('thread should use new file path if first created' +
'on patch set (right) after renaming', () => {
- const diffSide = 'right';
+ const diffSide = Side.RIGHT;
element.file = {basePath: 'file_renamed.txt', path: element.path};
assert.isOk(element._getOrCreateThread('2', 3,
@@ -1158,7 +1159,7 @@
test('thread should use new file path if first created' +
'on patch set (left) but is base', () => {
- const diffSide = 'left';
+ const diffSide = Side.LEFT;
element.file = {basePath: 'file_renamed.txt', path: element.path};
assert.isOk(element._getOrCreateThread('2', 3,
@@ -1188,19 +1189,19 @@
const l3 = document.createElement('div');
l3.setAttribute('line-num', 3);
- l3.setAttribute('diff-side', 'left');
+ l3.setAttribute('diff-side', Side.LEFT);
const l5 = document.createElement('div');
l5.setAttribute('line-num', 5);
- l5.setAttribute('diff-side', 'left');
+ l5.setAttribute('diff-side', Side.LEFT);
const r3 = document.createElement('div');
r3.setAttribute('line-num', 3);
- r3.setAttribute('diff-side', 'right');
+ r3.setAttribute('diff-side', Side.RIGHT);
const r5 = document.createElement('div');
r5.setAttribute('line-num', 5);
- r5.setAttribute('diff-side', 'right');
+ r5.setAttribute('diff-side', Side.RIGHT);
const threadEls = [l3, l5, r3, r5];
assert.deepEqual(element._filterThreadElsForLocation(threadEls, line,
@@ -1213,11 +1214,11 @@
const line = {beforeNumber: 'FILE', afterNumber: 'FILE'};
const l = document.createElement('div');
- l.setAttribute('diff-side', 'left');
+ l.setAttribute('diff-side', Side.LEFT);
l.setAttribute('line-num', 'FILE');
const r = document.createElement('div');
- r.setAttribute('diff-side', 'right');
+ r.setAttribute('diff-side', Side.RIGHT);
r.setAttribute('line-num', 'FILE');
const threadEls = [l, r];
@@ -1320,31 +1321,37 @@
suite('coverage layer', () => {
let notifyStub;
+ let coverageProviderStub;
+ const exampleRanges = [
+ {
+ type: CoverageType.COVERED,
+ side: Side.RIGHT,
+ code_range: {
+ start_line: 1,
+ end_line: 2,
+ },
+ },
+ {
+ type: CoverageType.NOT_COVERED,
+ side: Side.RIGHT,
+ code_range: {
+ start_line: 3,
+ end_line: 4,
+ },
+ },
+ ];
+
setup(() => {
notifyStub = sinon.stub();
+ coverageProviderStub = sinon.stub().returns(
+ Promise.resolve(exampleRanges));
+
stub('gr-js-api-interface', {
getCoverageAnnotationApis() {
return Promise.resolve([{
notify: notifyStub,
getCoverageProvider() {
- return () => Promise.resolve([
- {
- type: 'COVERED',
- side: 'right',
- code_range: {
- start_line: 1,
- end_line: 2,
- },
- },
- {
- type: 'NOT_COVERED',
- side: 'right',
- code_range: {
- start_line: 3,
- end_line: 4,
- },
- },
- ]);
+ return coverageProviderStub;
},
}]);
},
@@ -1380,9 +1387,38 @@
element.reload();
flush(() => {
assert.equal(notifyStub.callCount, 2);
+ assert.isTrue(notifyStub.calledWithExactly(
+ 'some/path', 1, 2, Side.RIGHT));
+ assert.isTrue(notifyStub.calledWithExactly(
+ 'some/path', 3, 4, Side.RIGHT));
done();
});
});
+
+ test('provider is called with appropriate params', done => {
+ element.patchRange.basePatchNum = 1;
+ element.patchRange.patchNum = 3;
+
+ element.reload();
+ flush(() => {
+ assert.isTrue(coverageProviderStub.calledWithExactly(
+ 123, 'some/path', 1, 3, element.change));
+ done();
+ });
+ });
+
+ test('provider is called with appropriate params - special patchset values',
+ done => {
+ element.patchRange.basePatchNum = 'PARENT';
+ element.patchRange.patchNum = 'invalid';
+
+ element.reload();
+ flush(() => {
+ assert.isTrue(coverageProviderStub.calledWithExactly(
+ 123, 'some/path', undefined, undefined, element.change));
+ done();
+ });
+ });
});
suite('trailing newlines', () => {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
index a6f4717..853d3c6 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
@@ -47,7 +47,6 @@
computeLatestPatchNum,
patchNumEquals,
PatchSet,
- CURRENT,
} from '../../../utils/patch-set-util';
import {
addUnmodifiedFiles,
@@ -95,9 +94,9 @@
import {CommentMap, isInBaseOfPatchRange} from '../../../utils/comment-util';
import {AppElementParams} from '../../gr-app-types';
import {CustomKeyboardEvent, OpenFixPreviewEvent} from '../../../types/events';
-import {PORTING_COMMENTS_DIFF_LATENCY_LABEL} from '../../../services/gr-reporting/gr-reporting';
import {fireAlert, fireTitleChange} from '../../../utils/event-util';
+import {KnownExperimentId} from '../../../services/flags/flags';
const ERR_REVIEW_STATUS = 'Couldn’t change file review status.';
const MSG_LOADING_BLAME = 'Loading blame...';
const MSG_LOADED_BLAME = 'Blame loaded';
@@ -264,6 +263,8 @@
@property({type: Number})
_focusLineNum?: number;
+ private _isPortingCommentsExperimentEnabled = false;
+
get keyBindings() {
return {
esc: '_handleEscKey',
@@ -343,6 +344,9 @@
this.$.cursor.reInitCursor();
};
this.$.diffHost.addEventListener('render', this._onRenderHandler);
+ this._isPortingCommentsExperimentEnabled = this.flagsService.isEnabled(
+ KnownExperimentId.PORTING_COMMENTS
+ );
}
/** @override */
@@ -1047,11 +1051,6 @@
return;
}
- const portedCommentsPromise = this.$.commentAPI.getPortedComments(
- value.changeNum,
- value.patchNum || CURRENT
- );
-
const promises: Promise<unknown>[] = [];
promises.push(this._getDiffPreferences());
@@ -1063,7 +1062,7 @@
);
promises.push(this._getChangeDetail(this._changeNum));
- promises.push(this._loadComments());
+ promises.push(this._loadComments(value.patchNum));
promises.push(this._getChangeEdit());
@@ -1072,19 +1071,22 @@
this._loading = true;
return Promise.all(promises)
.then(r => {
- this.reporting.time(PORTING_COMMENTS_DIFF_LATENCY_LABEL);
this._loading = false;
this._initPatchRange();
this._initCommitRange();
- if (this._changeComments && this._path && this._patchRange) {
- this.$.diffHost.threads = this._changeComments.getThreadsBySideForPath(
- this._path,
- this._patchRange
- );
- }
- portedCommentsPromise.then(() => {
- this.reporting.timeEnd(PORTING_COMMENTS_DIFF_LATENCY_LABEL);
- });
+
+ if (!this._path) throw new Error('path must be defined');
+ if (!this._changeComments)
+ throw new Error('change comments must be defined');
+ if (!this._patchRange) throw new Error('patch range must be defined');
+
+ // TODO(dhruvsri): check if basePath should be set here
+ this.$.diffHost.threads = this._changeComments.getThreadsBySideForFile(
+ {path: this._path},
+ this._patchRange,
+ this._isPortingCommentsExperimentEnabled
+ );
+
const edit = r[4] as EditInfo | undefined;
if (edit) {
this.set(`_change.revisions.${edit.commit.commit}`, {
@@ -1542,11 +1544,13 @@
return url;
}
- _loadComments() {
+ _loadComments(patchSet?: PatchSetNum) {
if (!this._changeNum) throw new Error('Missing this._changeNum');
- return this.$.commentAPI.loadAll(this._changeNum).then(comments => {
- this._changeComments = comments;
- });
+ return this.$.commentAPI
+ .loadAll(this._changeNum, patchSet)
+ .then(comments => {
+ this._changeComments = comments;
+ });
}
@observe('_files.changeFilesByPath', '_path', '_patchRange', '_projectConfig')
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js
index a8d3abd..1cb22e8 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js
@@ -23,7 +23,6 @@
import {SPECIAL_PATCH_SET_NUM} from '../../../utils/patch-set-util.js';
import {Shortcut} from '../../../mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin.js';
import {_testOnly_findCommentById} from '../gr-comment-api/gr-comment-api.js';
-import {appContext} from '../../../services/app-context.js';
import {GerritView} from '../../core/gr-navigation/gr-navigation.js';
import {
createChange,
@@ -90,7 +89,6 @@
setup(async () => {
clock = sinon.useFakeTimers();
- sinon.stub(appContext.flagsService, 'isEnabled').returns(true);
stub('gr-rest-api-interface', {
getConfig() {
return Promise.resolve({change: {}});
@@ -119,6 +117,9 @@
getDiffDrafts() {
return Promise.resolve({});
},
+ getPortedComments() {
+ return Promise.resolve({});
+ },
getReviewedFiles() {
return Promise.resolve([]);
},
@@ -151,8 +152,7 @@
computeCommentThreadCount: () => {},
computeUnresolvedNum: () => {},
getPaths: () => {},
- getThreadsBySideForPath: () => {},
- getThreadsBySideForFile: () => {},
+ getThreadsBySideForFile: () => [],
findCommentById: _testOnly_findCommentById,
}));
@@ -178,6 +178,8 @@
basePatchNum: 1,
path: '/COMMIT_MSG',
};
+ element._path = '/COMMIT_MSG';
+ element._patchRange = {};
return element._paramsChanged.returnValues[0].then(() => {
assert.isTrue(element.reporting.diffViewDisplayed.calledOnce);
});
@@ -235,6 +237,8 @@
basePatchNum: 1,
path: '/COMMIT_MSG',
};
+ element._path = '/COMMIT_MSG';
+ element._patchRange = {};
return element._paramsChanged.returnValues[0].then(() => {
assert.isTrue(element._isBlameLoaded);
assert.isTrue(element._loadBlame.calledOnce);
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-utils.ts b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-utils.ts
index eebcb15..038153a 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-utils.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-utils.ts
@@ -83,7 +83,7 @@
// For Gerrit these are instances of GrCommentThread, but other gr-diff users
// have different HTML elements in use for comment threads.
// TODO: Also document the required HTML attritbutes that thread elements must
-// have, e.g. 'comment-side', 'range', 'line-num', 'data-value'.
+// have, e.g. 'diff-side', 'range', 'line-num', 'data-value'.
export interface GrDiffThreadElement extends HTMLElement {
rootId: string;
}
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.ts b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.ts
index 7092361..04714bb 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.ts
@@ -66,7 +66,7 @@
import {FlattenedNodesObserver} from '@polymer/polymer/lib/utils/flattened-nodes-observer';
import {PolymerDeepPropertyChange} from '@polymer/polymer/interfaces';
import {AbortStop} from '../../shared/gr-cursor-manager/gr-cursor-manager';
-import {fireAlert} from '../../../utils/event-util';
+import {fireAlert, fireEvent} from '../../../utils/event-util';
import {MovedChunkGoToLineEvent} from '../../../types/events';
const NO_NEWLINE_BASE = 'No newline at end of base file.';
@@ -438,20 +438,10 @@
_redispatchHoverEvents(addedThreadEls: HTMLElement[]) {
for (const threadEl of addedThreadEls) {
threadEl.addEventListener('mouseenter', () => {
- threadEl.dispatchEvent(
- new CustomEvent('comment-thread-mouseenter', {
- bubbles: true,
- composed: true,
- })
- );
+ fireEvent(threadEl, 'comment-thread-mouseenter');
});
threadEl.addEventListener('mouseleave', () => {
- threadEl.dispatchEvent(
- new CustomEvent('comment-thread-mouseleave', {
- bubbles: true,
- composed: true,
- })
- );
+ fireEvent(threadEl, 'comment-thread-mouseleave');
});
}
}
@@ -604,12 +594,7 @@
_isValidElForComment(el: Element) {
if (!this.loggedIn) {
- this.dispatchEvent(
- new CustomEvent('show-auth-required', {
- composed: true,
- bubbles: true,
- })
- );
+ fireEvent(this, 'show-auth-required');
return false;
}
if (!this.patchRange) {
@@ -843,9 +828,7 @@
_renderDiffTable() {
if (!this.prefs) {
- this.dispatchEvent(
- new CustomEvent('render', {bubbles: true, composed: true})
- );
+ fireEvent(this, 'render');
return;
}
if (
@@ -855,9 +838,7 @@
this._safetyBypass === null
) {
this._showWarning = true;
- this.dispatchEvent(
- new CustomEvent('render', {bubbles: true, composed: true})
- );
+ fireEvent(this, 'render');
return;
}
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_html.ts b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_html.ts
index 2c8b704..557d1eb 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_html.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_html.ts
@@ -552,9 +552,9 @@
/** Make comments selectable when selected */
.selected-left.selected-comment
- ::slotted(gr-comment-thread[comment-side='left']),
+ ::slotted(gr-comment-thread[diff-side='left']),
.selected-right.selected-comment
- ::slotted(gr-comment-thread[comment-side='right']) {
+ ::slotted(gr-comment-thread[diff-side='right']) {
-webkit-user-select: text;
-moz-user-select: text;
-ms-user-select: text;
diff --git a/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box.ts b/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box.ts
index ff6efe6..ee52ab6 100644
--- a/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box.ts
+++ b/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box.ts
@@ -22,6 +22,7 @@
import {LegacyElementMixin} from '@polymer/polymer/lib/legacy/legacy-element-mixin';
import {PolymerElement} from '@polymer/polymer/polymer-element';
import {htmlTemplate} from './gr-selection-action-box_html';
+import {fireEvent} from '../../../utils/event-util';
declare global {
interface HTMLElementTagNameMap {
@@ -122,11 +123,6 @@
} // 0 = main button
e.preventDefault();
e.stopPropagation();
- this.dispatchEvent(
- new CustomEvent('create-comment-requested', {
- composed: true,
- bubbles: true,
- })
- );
+ fireEvent(this, 'create-comment-requested');
}
}
diff --git a/polygerrit-ui/app/elements/gr-app-element_html.ts b/polygerrit-ui/app/elements/gr-app-element_html.ts
index b978ed8..e1fe637 100644
--- a/polygerrit-ui/app/elements/gr-app-element_html.ts
+++ b/polygerrit-ui/app/elements/gr-app-element_html.ts
@@ -40,6 +40,8 @@
border-left: 0;
border-top: 0;
box-shadow: var(--header-box-shadow);
+ /* Make sure the header is above the main content, to preserve box-shadow visibility */
+ z-index: 1;
}
footer {
background: var(
diff --git a/polygerrit-ui/app/elements/plugins/gr-repo-api/gr-plugin-repo-command.ts b/polygerrit-ui/app/elements/plugins/gr-repo-api/gr-plugin-repo-command.ts
index b3a40c5..a9aba13 100644
--- a/polygerrit-ui/app/elements/plugins/gr-repo-api/gr-plugin-repo-command.ts
+++ b/polygerrit-ui/app/elements/plugins/gr-repo-api/gr-plugin-repo-command.ts
@@ -17,6 +17,7 @@
import {PolymerElement} from '@polymer/polymer/polymer-element';
import {htmlTemplate} from './gr-plugin-repo-command_html';
import {customElement, property} from '@polymer/decorators';
+import {fireEvent} from '../../../utils/event-util';
declare global {
interface HTMLElementTagNameMap {
@@ -33,8 +34,6 @@
}
_handleClick() {
- this.dispatchEvent(
- new CustomEvent('command-tap', {composed: true, bubbles: true})
- );
+ fireEvent(this, 'command-tap');
}
}
diff --git a/polygerrit-ui/app/elements/settings/gr-account-info/gr-account-info.ts b/polygerrit-ui/app/elements/settings/gr-account-info/gr-account-info.ts
index 0544a9a..5786576 100644
--- a/polygerrit-ui/app/elements/settings/gr-account-info/gr-account-info.ts
+++ b/polygerrit-ui/app/elements/settings/gr-account-info/gr-account-info.ts
@@ -27,6 +27,7 @@
import {AccountInfo, ServerInfo} from '../../../types/common';
import {EditableAccountField} from '../../../constants/constants';
import {appContext} from '../../../services/app-context';
+import {fireEvent} from '../../../utils/event-util';
@customElement('gr-account-info')
export class GrAccountInfo extends GestureEventListeners(
@@ -151,12 +152,7 @@
this._hasDisplayNameChange = false;
this._hasStatusChange = false;
this._saving = false;
- this.dispatchEvent(
- new CustomEvent('account-detail-update', {
- composed: true,
- bubbles: true,
- })
- );
+ fireEvent(this, 'account-detail-update');
});
}
diff --git a/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog.ts b/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog.ts
index fbc4607..85e692d 100644
--- a/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog.ts
+++ b/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog.ts
@@ -26,6 +26,7 @@
import {ServerInfo, AccountDetailInfo} from '../../../types/common';
import {EditableAccountField} from '../../../constants/constants';
import {appContext} from '../../../services/app-context';
+import {fireEvent} from '../../../utils/event-util';
export interface GrRegistrationDialog {
$: {
@@ -135,12 +136,7 @@
return Promise.all(promises).then(() => {
this._saving = false;
- this.dispatchEvent(
- new CustomEvent('account-detail-update', {
- composed: true,
- bubbles: true,
- })
- );
+ fireEvent(this, 'account-detail-update');
});
}
@@ -156,12 +152,7 @@
close() {
this._saving = true; // disable buttons indefinitely
- this.dispatchEvent(
- new CustomEvent('close', {
- composed: true,
- bubbles: true,
- })
- );
+ fireEvent(this, 'close');
}
_computeSaveDisabled(name?: string, email?: string, saving?: boolean) {
diff --git a/polygerrit-ui/app/elements/settings/gr-watched-projects-editor/gr-watched-projects-editor.ts b/polygerrit-ui/app/elements/settings/gr-watched-projects-editor/gr-watched-projects-editor.ts
index f676538..36444a3 100644
--- a/polygerrit-ui/app/elements/settings/gr-watched-projects-editor/gr-watched-projects-editor.ts
+++ b/polygerrit-ui/app/elements/settings/gr-watched-projects-editor/gr-watched-projects-editor.ts
@@ -82,7 +82,7 @@
}
save() {
- let deletePromise;
+ let deletePromise: Promise<Response | undefined>;
if (this._projectsToRemove.length) {
deletePromise = this.restApiService.deleteWatchedProjects(
this._projectsToRemove
diff --git a/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label.ts b/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label.ts
index 08bc0f7..a6c4201 100644
--- a/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label.ts
+++ b/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label.ts
@@ -29,6 +29,7 @@
import {ReportingService} from '../../../services/gr-reporting/gr-reporting';
import {ChangeInfo, AccountInfo, ServerInfo} from '../../../types/common';
import {hasOwnProperty} from '../../../utils/common-util';
+import {fireEvent} from '../../../utils/event-util';
@customElement('gr-account-label')
export class GrAccountLabel extends GestureEventListeners(
@@ -234,9 +235,7 @@
reason
)
.then(() => {
- this.dispatchEvent(
- new CustomEvent('hide-alert', {bubbles: true, composed: true})
- );
+ fireEvent(this, 'hide-alert');
});
}
diff --git a/polygerrit-ui/app/elements/shared/gr-autocomplete-dropdown/gr-autocomplete-dropdown.ts b/polygerrit-ui/app/elements/shared/gr-autocomplete-dropdown/gr-autocomplete-dropdown.ts
index b7c7b69..aff6d50 100644
--- a/polygerrit-ui/app/elements/shared/gr-autocomplete-dropdown/gr-autocomplete-dropdown.ts
+++ b/polygerrit-ui/app/elements/shared/gr-autocomplete-dropdown/gr-autocomplete-dropdown.ts
@@ -27,6 +27,7 @@
import {customElement, property, observe} from '@polymer/decorators';
import {IronFitBehavior} from '@polymer/iron-fit-behavior/iron-fit-behavior';
import {GrCursorManager} from '../gr-cursor-manager/gr-cursor-manager';
+import {fireEvent} from '../../../utils/event-util';
export interface GrAutocompleteDropdown {
$: {
@@ -204,12 +205,7 @@
}
_fireClose() {
- this.dispatchEvent(
- new CustomEvent('dropdown-closed', {
- composed: true,
- bubbles: true,
- })
- );
+ fireEvent(this, 'dropdown-closed');
}
getCursorTarget() {
diff --git a/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.ts b/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.ts
index 06555a7..6151a1d9 100644
--- a/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.ts
+++ b/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.ts
@@ -30,6 +30,7 @@
import {GrCursorManager} from '../gr-cursor-manager/gr-cursor-manager';
import {PaperInputElementExt} from '../../../types/types';
import {CustomKeyboardEvent} from '../../../types/events';
+import {fireEvent} from '../../../utils/event-util';
const TOKENIZE_REGEX = /(?:[^\s"]+|"[^"]*")+/g;
const DEBOUNCE_WAIT_MS = 200;
@@ -404,12 +405,7 @@
if (this._suggestions.length) {
this.set('_suggestions', []);
} else {
- this.dispatchEvent(
- new CustomEvent('cancel', {
- composed: true,
- bubbles: true,
- })
- );
+ fireEvent(this, 'cancel');
}
}
diff --git a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.ts b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.ts
index 0e0ee0c..dd48556 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.ts
@@ -172,6 +172,9 @@
showFileName = true;
@property({type: Boolean})
+ showPortedComment = false;
+
+ @property({type: Boolean})
showPatchset = true;
get keyBindings() {
@@ -242,20 +245,24 @@
);
}
- _getDiffUrlForPath(path: string) {
- if (!this.changeNum) throw new Error('changeNum is missing');
- if (!this.projectName) throw new Error('projectName is missing');
+ _getDiffUrlForPath(
+ projectName?: RepoName,
+ changeNum?: NumericChangeId,
+ path?: string,
+ patchNum?: PatchSetNum
+ ) {
+ if (!changeNum || !projectName || !path) return undefined;
if (isDraft(this.comments[0])) {
return GerritNav.getUrlForDiffById(
- this.changeNum,
- this.projectName,
+ changeNum,
+ projectName,
path,
- this.patchNum
+ patchNum
);
}
const id = this.comments[0].id;
if (!id) throw new Error('A published comment is missing the id.');
- return GerritNav.getUrlForComment(this.changeNum, this.projectName, id);
+ return GerritNav.getUrlForComment(changeNum, projectName, id);
}
_getDiffUrlForComment(
@@ -287,6 +294,11 @@
return path === SpecialFilePath.PATCHSET_LEVEL_COMMENTS;
}
+ _computeShowPortedComment(comment: UIComment) {
+ if (this._orderedComments.length === 0) return false;
+ return this.showPortedComment && comment.id === this._orderedComments[0].id;
+ }
+
_computeDisplayPath(path: string) {
const displayPath = computeDisplayPath(path);
if (displayPath === SpecialFilePath.PATCHSET_LEVEL_COMMENTS) {
diff --git a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_html.ts b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_html.ts
index 64be155..dd9f9e8 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_html.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_html.ts
@@ -83,7 +83,9 @@
<span> [[_computeDisplayPath(path)]] </span>
</template>
<template is="dom-if" if="[[!_isPatchsetLevelComment(path)]]">
- <a href$="[[_getDiffUrlForPath(path)]]">
+ <a
+ href$="[[_getDiffUrlForPath(projectName, changeNum, path, patchNum)]]"
+ >
[[_computeDisplayPath(path)]]
</a>
</template>
@@ -113,10 +115,12 @@
comments="{{comments}}"
robot-button-disabled="[[_shouldDisableAction(_showActions, _lastComment)]]"
change-num="[[changeNum]]"
+ project-name="[[projectName]]"
patch-num="[[patchNum]]"
draft="[[_isDraft(comment)]]"
show-actions="[[_showActions]]"
show-patchset="[[showPatchset]]"
+ show-ported-comment="[[_computeShowPortedComment(comment)]]"
side="[[comment.side]]"
project-config="[[_projectConfig]]"
on-create-fix-comment="_handleCommentFix"
diff --git a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_test.ts b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_test.ts
index c1367a6..c8409fa 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_test.ts
@@ -44,6 +44,7 @@
tap,
pressAndReleaseKeyOn,
} from '@polymer/iron-test-helpers/mock-interactions';
+import {html} from '@polymer/polymer/lib/utils/html-tag.js';
const basicFixture = fixtureFromElement('gr-comment-thread');
@@ -66,6 +67,14 @@
flush();
});
+ test('renders without patchNum and changeNum', async () => {
+ const fixture = fixtureFromTemplate(
+ html`<gr-comment-thread show-file-path="" path="path/to/file"></gr-change-metadata>`
+ );
+ fixture.instantiate();
+ await flush();
+ });
+
test('comments are sorted correctly', () => {
const comments: UIComment[] = [
{
diff --git a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
index b2dcb88..3dd851e 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
@@ -38,6 +38,7 @@
import {getRootElement} from '../../../scripts/rootElement';
import {appContext} from '../../../services/app-context';
import {customElement, observe, property} from '@polymer/decorators';
+import {GerritNav} from '../../core/gr-navigation/gr-navigation';
import {GrTextarea} from '../gr-textarea/gr-textarea';
import {GrStorage, StorageLocation} from '../gr-storage/gr-storage';
import {GrOverlay} from '../gr-overlay/gr-overlay';
@@ -46,6 +47,7 @@
NumericChangeId,
ConfigInfo,
PatchSetNum,
+ RepoName,
} from '../../../types/common';
import {GrButton} from '../gr-button/gr-button';
import {GrConfirmDeleteCommentDialog} from '../gr-confirm-delete-comment-dialog/gr-confirm-delete-comment-dialog';
@@ -152,6 +154,9 @@
@property({type: Number})
changeNum?: NumericChangeId;
+ @property({type: String})
+ projectName?: RepoName;
+
@property({type: Object, notify: true, observer: '_commentChanged'})
comment?: UIComment;
@@ -254,6 +259,9 @@
@property({type: Object})
_selfAccount?: AccountDetailInfo;
+ @property({type: Boolean})
+ showPortedComment = false;
+
get keyBindings() {
return {
'ctrl+enter meta+enter ctrl+s meta+s': '_handleSaveKey',
@@ -294,6 +302,16 @@
return comment.author || this._selfAccount;
}
+ _getUrlForComment(comment: UIComment) {
+ if (!this.changeNum || !this.projectName) return '';
+ if (!comment.id) throw new Error('comment must have an id');
+ return GerritNav.getUrlForComment(
+ this.changeNum as NumericChangeId,
+ this.projectName,
+ comment.id
+ );
+ }
+
@observe('editing')
_onEditingChange(editing?: boolean) {
this.dispatchEvent(
diff --git a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_html.ts b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_html.ts
index 64f0be1..d9a3187 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_html.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_html.ts
@@ -245,6 +245,9 @@
.draft gr-account-label {
width: unset;
}
+ .portedMessage {
+ margin: 0 var(--spacing-m);
+ }
</style>
<div id="container" class="container">
<div class="header" id="header" on-click="_handleToggleCollapsed">
@@ -260,6 +263,13 @@
>
</gr-account-label>
</template>
+ <template is="dom-if" if="[[showPortedComment]]">
+ <a href="[[_getUrlForComment(comment)]]">
+ <span class="portedMessage">
+ Ported from patchset [[comment.patch_set]]
+ </span>
+ </a>
+ </template>
<gr-tooltip-content
class="draftTooltip"
has-tooltip=""
diff --git a/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content.ts b/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content.ts
index 9c9363b..10e8916 100644
--- a/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content.ts
+++ b/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content.ts
@@ -24,7 +24,7 @@
import {PolymerElement} from '@polymer/polymer/polymer-element';
import {customElement, property} from '@polymer/decorators';
import {htmlTemplate} from './gr-editable-content_html';
-import {fireAlert} from '../../../utils/event-util';
+import {fireAlert, fireEvent} from '../../../utils/event-util';
const RESTORED_MESSAGE = 'Content restored from a previous edit.';
const STORAGE_DEBOUNCE_INTERVAL_MS = 400;
@@ -188,11 +188,6 @@
_handleCancel(e: Event) {
e.preventDefault();
this.editing = false;
- this.dispatchEvent(
- new CustomEvent('editable-content-cancel', {
- composed: true,
- bubbles: true,
- })
- );
+ fireEvent(this, 'editable-content-cancel');
}
}
diff --git a/polygerrit-ui/app/elements/shared/gr-editable-label/gr-editable-label.ts b/polygerrit-ui/app/elements/shared/gr-editable-label/gr-editable-label.ts
index 9e1a5bf..3b839cb 100644
--- a/polygerrit-ui/app/elements/shared/gr-editable-label/gr-editable-label.ts
+++ b/polygerrit-ui/app/elements/shared/gr-editable-label/gr-editable-label.ts
@@ -89,6 +89,9 @@
@property({type: Number})
readonly _verticalOffset = -30;
+ @property({type: Boolean})
+ showAsEditPencil = false;
+
/** @override */
ready() {
super.ready();
diff --git a/polygerrit-ui/app/elements/shared/gr-editable-label/gr-editable-label_html.ts b/polygerrit-ui/app/elements/shared/gr-editable-label/gr-editable-label_html.ts
index 5e36166..7700689 100644
--- a/polygerrit-ui/app/elements/shared/gr-editable-label/gr-editable-label_html.ts
+++ b/polygerrit-ui/app/elements/shared/gr-editable-label/gr-editable-label_html.ts
@@ -68,13 +68,32 @@
}
--paper-input-container-focus-color: var(--link-color);
}
+ gr-button iron-icon {
+ color: inherit;
+ --iron-icon-height: 18px;
+ --iron-icon-width: 18px;
+ }
+ gr-button.new-change-summary-true {
+ --padding: 1px 4px;
+ }
</style>
- <label
- class$="[[_computeLabelClass(readOnly, value, placeholder)]]"
- title$="[[_computeLabel(value, placeholder)]]"
- on-click="_showDropdown"
- >[[_computeLabel(value, placeholder)]]</label
- >
+ <template is="dom-if" if="[[!showAsEditPencil]]">
+ <label
+ class$="[[_computeLabelClass(readOnly, value, placeholder)]]"
+ title$="[[_computeLabel(value, placeholder)]]"
+ on-click="_showDropdown"
+ >[[_computeLabel(value, placeholder)]]</label
+ >
+ </template>
+ <template is="dom-if" if="[[showAsEditPencil]]">
+ <gr-button
+ link=""
+ class$="new-change-summary-true [[_computeLabelClass(readOnly, value, placeholder)]]"
+ on-click="_showDropdown"
+ title="[[_computeLabel(value, placeholder)]]"
+ ><iron-icon icon="gr-icons:edit"></iron-icon
+ ></gr-button>
+ </template>
<iron-dropdown
id="dropdown"
vertical-align="auto"
diff --git a/polygerrit-ui/app/elements/shared/gr-editable-label/gr-editable-label_test.js b/polygerrit-ui/app/elements/shared/gr-editable-label/gr-editable-label_test.js
index d8f085e..2bdc570 100644
--- a/polygerrit-ui/app/elements/shared/gr-editable-label/gr-editable-label_test.js
+++ b/polygerrit-ui/app/elements/shared/gr-editable-label/gr-editable-label_test.js
@@ -45,6 +45,7 @@
setup(async () => {
element = basicFixture.instantiate();
elementNoPlaceholder = noPlaceholderFixture.instantiate();
+ flush();
label = element.shadowRoot.querySelector('label');
await flush();
@@ -178,6 +179,7 @@
setup(() => {
element = readOnlyFixture.instantiate();
+ flush();
label = element.shadowRoot
.querySelector('label');
});
diff --git a/polygerrit-ui/app/elements/shared/gr-linked-chip/gr-linked-chip.ts b/polygerrit-ui/app/elements/shared/gr-linked-chip/gr-linked-chip.ts
index dbb8725..fa244f6 100644
--- a/polygerrit-ui/app/elements/shared/gr-linked-chip/gr-linked-chip.ts
+++ b/polygerrit-ui/app/elements/shared/gr-linked-chip/gr-linked-chip.ts
@@ -24,6 +24,7 @@
import {PolymerElement} from '@polymer/polymer/polymer-element';
import {customElement, property} from '@polymer/decorators';
import {htmlTemplate} from './gr-linked-chip_html';
+import {fireEvent} from '../../../utils/event-util';
declare global {
interface HTMLElementTagNameMap {
@@ -64,11 +65,6 @@
_handleRemoveTap(e: Event) {
e.preventDefault();
- this.dispatchEvent(
- new CustomEvent('remove', {
- composed: true,
- bubbles: true,
- })
- );
+ fireEvent(this, 'remove');
}
}
diff --git a/polygerrit-ui/app/elements/shared/gr-list-view/gr-list-view.ts b/polygerrit-ui/app/elements/shared/gr-list-view/gr-list-view.ts
index 3403e87..97b45ac 100644
--- a/polygerrit-ui/app/elements/shared/gr-list-view/gr-list-view.ts
+++ b/polygerrit-ui/app/elements/shared/gr-list-view/gr-list-view.ts
@@ -25,6 +25,7 @@
import {encodeURL, getBaseUrl} from '../../../utils/url-util';
import {page} from '../../../utils/page-wrapper-utils';
import {property, customElement} from '@polymer/decorators';
+import {fireEvent} from '../../../utils/event-util';
const REQUEST_DEBOUNCE_INTERVAL_MS = 200;
@@ -96,12 +97,7 @@
}
_createNewItem() {
- this.dispatchEvent(
- new CustomEvent('create-clicked', {
- composed: true,
- bubbles: true,
- })
- );
+ fireEvent(this, 'create-clicked');
}
_computeNavLink(
diff --git a/polygerrit-ui/app/elements/shared/gr-overlay/gr-overlay.ts b/polygerrit-ui/app/elements/shared/gr-overlay/gr-overlay.ts
index 026cb3a..68a73933 100644
--- a/polygerrit-ui/app/elements/shared/gr-overlay/gr-overlay.ts
+++ b/polygerrit-ui/app/elements/shared/gr-overlay/gr-overlay.ts
@@ -20,9 +20,10 @@
import {PolymerElement} from '@polymer/polymer/polymer-element';
import {htmlTemplate} from './gr-overlay_html';
import {IronOverlayMixin} from '../../../mixins/iron-overlay-mixin/iron-overlay-mixin';
-import {customElement, property} from '@polymer/decorators';
+import {customElement} from '@polymer/decorators';
import {IronOverlayBehavior} from '@polymer/iron-overlay-behavior/iron-overlay-behavior';
import {findActiveElement} from '../../../utils/dom-util';
+import {fireEvent} from '../../../utils/event-util';
const AWAIT_MAX_ITERS = 10;
const AWAIT_STEP = 5;
@@ -55,7 +56,6 @@
* @event fullscreen-overlay-opened
*/
- @property({type: Boolean})
private _fullScreenOpen = false;
private _boundHandleClose: () => void = () => super.close();
@@ -98,12 +98,7 @@
return new Promise((resolve, reject) => {
super.open.apply(this);
if (this._isMobile()) {
- this.dispatchEvent(
- new CustomEvent('fullscreen-overlay-opened', {
- composed: true,
- bubbles: true,
- })
- );
+ fireEvent(this, 'fullscreen-overlay-opened');
this._fullScreenOpen = true;
}
this._awaitOpen(resolve, reject);
@@ -118,12 +113,7 @@
_overlayClosed() {
window.removeEventListener('popstate', this._boundHandleClose);
if (this._fullScreenOpen) {
- this.dispatchEvent(
- new CustomEvent('fullscreen-overlay-closed', {
- composed: true,
- bubbles: true,
- })
- );
+ fireEvent(this, 'fullscreen-overlay-closed');
this._fullScreenOpen = false;
}
if (this.returnFocusTo) {
diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.ts b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.ts
index 1c6e0b6..078e07f 100644
--- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.ts
+++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.ts
@@ -840,48 +840,28 @@
}) as Promise<EmailInfo[] | undefined>;
}
- addAccountEmail(email: string): Promise<Response>;
-
- addAccountEmail(
- email: string,
- errFn?: ErrorCallback
- ): Promise<Response | undefined>;
-
- addAccountEmail(email: string, errFn?: ErrorCallback) {
+ addAccountEmail(email: string): Promise<Response> {
return this._restApiHelper.send({
method: HttpMethod.PUT,
url: '/accounts/self/emails/' + encodeURIComponent(email),
- errFn,
anonymizedUrl: '/account/self/emails/*',
});
}
- deleteAccountEmail(email: string): Promise<Response>;
-
- deleteAccountEmail(
- email: string,
- errFn?: ErrorCallback
- ): Promise<Response | undefined>;
-
- deleteAccountEmail(email: string, errFn?: ErrorCallback) {
+ deleteAccountEmail(email: string): Promise<Response> {
return this._restApiHelper.send({
method: HttpMethod.DELETE,
url: '/accounts/self/emails/' + encodeURIComponent(email),
- errFn,
anonymizedUrl: '/accounts/self/email/*',
});
}
- setPreferredAccountEmail(
- email: string,
- errFn?: ErrorCallback
- ): Promise<void> {
+ setPreferredAccountEmail(email: string): Promise<void> {
// TODO(TS): add correct error handling
const encodedEmail = encodeURIComponent(email);
const req = {
method: HttpMethod.PUT,
url: `/accounts/self/emails/${encodedEmail}/preferred`,
- errFn,
anonymizedUrl: '/accounts/self/emails/*/preferred',
};
return this._restApiHelper.send(req).then(() => {
@@ -911,13 +891,12 @@
}
}
- setAccountName(name: string, errFn?: ErrorCallback): Promise<void> {
+ setAccountName(name: string): Promise<void> {
// TODO(TS): add correct error handling
const req: SendJSONRequest = {
method: HttpMethod.PUT,
url: '/accounts/self/name',
body: {name},
- errFn,
parseResponse: true,
reportUrlAsIs: true,
};
@@ -928,13 +907,12 @@
);
}
- setAccountUsername(username: string, errFn?: ErrorCallback): Promise<void> {
+ setAccountUsername(username: string): Promise<void> {
// TODO(TS): add correct error handling
const req: SendJSONRequest = {
method: HttpMethod.PUT,
url: '/accounts/self/username',
body: {username},
- errFn,
parseResponse: true,
reportUrlAsIs: true,
};
@@ -945,16 +923,12 @@
);
}
- setAccountDisplayName(
- displayName: string,
- errFn?: ErrorCallback
- ): Promise<void> {
+ setAccountDisplayName(displayName: string): Promise<void> {
// TODO(TS): add correct error handling
const req: SendJSONRequest = {
method: HttpMethod.PUT,
url: '/accounts/self/displayname',
body: {display_name: displayName},
- errFn,
parseResponse: true,
reportUrlAsIs: true,
};
@@ -965,13 +939,12 @@
);
}
- setAccountStatus(status: string, errFn?: ErrorCallback): Promise<void> {
+ setAccountStatus(status: string): Promise<void> {
// TODO(TS): add correct error handling
const req: SendJSONRequest = {
method: HttpMethod.PUT,
url: '/accounts/self/status',
body: {status},
- errFn,
parseResponse: true,
reportUrlAsIs: true,
};
@@ -1096,34 +1069,22 @@
}
saveWatchedProjects(
- projects: ProjectWatchInfo[],
- errFn?: ErrorCallback
+ projects: ProjectWatchInfo[]
): Promise<ProjectWatchInfo[]> {
return (this._restApiHelper.send({
method: HttpMethod.POST,
url: '/accounts/self/watched.projects',
body: projects,
- errFn,
parseResponse: true,
reportUrlAsIs: true,
}) as unknown) as Promise<ProjectWatchInfo[]>;
}
- deleteWatchedProjects(
- projects: ProjectWatchInfo[]
- ): Promise<Response | undefined>;
-
- deleteWatchedProjects(
- projects: ProjectWatchInfo[],
- errFn: ErrorCallback
- ): Promise<Response | undefined>;
-
- deleteWatchedProjects(projects: ProjectWatchInfo[], errFn?: ErrorCallback) {
+ deleteWatchedProjects(projects: ProjectWatchInfo[]): Promise<Response> {
return this._restApiHelper.send({
method: HttpMethod.POST,
url: '/accounts/self/watched.projects:delete',
body: projects,
- errFn,
reportUrlAsIs: true,
});
}
@@ -1302,11 +1263,7 @@
return listChangesOptionsToHex(...options);
}
- getDiffChangeDetail(
- changeNum: NumericChangeId,
- errFn?: ErrorCallback,
- cancelCondition?: CancelConditionCallback
- ) {
+ getDiffChangeDetail(changeNum: NumericChangeId) {
let optionsHex = '';
if (window.DEFAULT_DETAIL_HEXES?.diffPage) {
optionsHex = window.DEFAULT_DETAIL_HEXES.diffPage;
@@ -1317,7 +1274,7 @@
ListChangesOption.SKIP_DIFFSTAT
);
}
- return this._getChangeDetail(changeNum, optionsHex, errFn, cancelCondition);
+ return this._getChangeDetail(changeNum, optionsHex);
}
/**
@@ -1467,37 +1424,22 @@
>;
}
- getChangeSuggestedReviewers(
- changeNum: NumericChangeId,
- inputVal: string,
- errFn?: ErrorCallback
- ) {
+ getChangeSuggestedReviewers(changeNum: NumericChangeId, inputVal: string) {
return this._getChangeSuggestedGroup(
ReviewerState.REVIEWER,
changeNum,
- inputVal,
- errFn
+ inputVal
);
}
- getChangeSuggestedCCs(
- changeNum: NumericChangeId,
- inputVal: string,
- errFn?: ErrorCallback
- ) {
- return this._getChangeSuggestedGroup(
- ReviewerState.CC,
- changeNum,
- inputVal,
- errFn
- );
+ getChangeSuggestedCCs(changeNum: NumericChangeId, inputVal: string) {
+ return this._getChangeSuggestedGroup(ReviewerState.CC, changeNum, inputVal);
}
_getChangeSuggestedGroup(
reviewerState: ReviewerState,
changeNum: NumericChangeId,
- inputVal: string,
- errFn?: ErrorCallback
+ inputVal: string
): Promise<SuggestedReviewerInfo[] | undefined> {
// More suggestions may obscure content underneath in the reply dialog,
// see issue 10793.
@@ -1511,7 +1453,6 @@
return this._getChangeURLAndFetch({
changeNum,
endpoint: '/suggest_reviewers',
- errFn,
params,
reportEndpointAsIs: true,
}) as Promise<SuggestedReviewerInfo[] | undefined>;
@@ -1732,8 +1673,7 @@
getSuggestedGroups(
inputVal: string,
- n?: number,
- errFn?: ErrorCallback
+ n?: number
): Promise<GroupNameToGroupInfoMap | undefined> {
const params: QueryGroupsParams = {s: inputVal};
if (n) {
@@ -1741,7 +1681,6 @@
}
return this._restApiHelper.fetchJSON({
url: '/groups/',
- errFn,
params,
reportUrlAsIs: true,
}) as Promise<GroupNameToGroupInfoMap | undefined>;
@@ -1749,8 +1688,7 @@
getSuggestedProjects(
inputVal: string,
- n?: number,
- errFn?: ErrorCallback
+ n?: number
): Promise<NameToProjectInfoMap | undefined> {
const params = {
m: inputVal,
@@ -1762,7 +1700,6 @@
}
return this._restApiHelper.fetchJSON({
url: '/projects/',
- errFn,
params,
reportUrlAsIs: true,
});
@@ -1770,8 +1707,7 @@
getSuggestedAccounts(
inputVal: string,
- n?: number,
- errFn?: ErrorCallback
+ n?: number
): Promise<AccountInfo[] | undefined> {
if (!inputVal) {
return Promise.resolve([]);
@@ -1782,7 +1718,6 @@
}
return this._restApiHelper.fetchJSON({
url: '/accounts/',
- errFn,
params,
anonymizedUrl: '/accounts/?n=*',
}) as Promise<AccountInfo[] | undefined>;
@@ -1943,29 +1878,12 @@
patchNum: PatchSetNum,
path: string,
reviewed: boolean
- ): Promise<Response>;
-
- saveFileReviewed(
- changeNum: NumericChangeId,
- patchNum: PatchSetNum,
- path: string,
- reviewed: boolean,
- errFn: ErrorCallback
- ): Promise<Response | undefined>;
-
- saveFileReviewed(
- changeNum: NumericChangeId,
- patchNum: PatchSetNum,
- path: string,
- reviewed: boolean,
- errFn?: ErrorCallback
- ) {
+ ): Promise<Response> {
return this._getChangeURLAndSend({
changeNum,
method: reviewed ? HttpMethod.PUT : HttpMethod.DELETE,
patchNum,
endpoint: `/files/${encodeURIComponent(path)}/reviewed`,
- errFn,
anonymizedEndpoint: '/files/*/reviewed',
});
}
@@ -3085,10 +3003,9 @@
}) as Promise<CapabilityInfoMap | undefined>;
}
- getTopMenus(errFn?: ErrorCallback): Promise<TopMenuEntryInfo[] | undefined> {
+ getTopMenus(): Promise<TopMenuEntryInfo[] | undefined> {
return this._fetchSharedCacheURL({
url: '/config/server/top-menus',
- errFn,
reportUrlAsIs: true,
}) as Promise<TopMenuEntryInfo[] | undefined>;
}
@@ -3142,21 +3059,6 @@
});
}
- startReview(
- changeNum: NumericChangeId,
- body?: RequestPayload,
- errFn?: ErrorCallback
- ) {
- return this._getChangeURLAndSend({
- changeNum,
- method: HttpMethod.POST,
- endpoint: '/ready',
- body,
- errFn,
- reportUrlAsIs: true,
- });
- }
-
deleteComment(
changeNum: NumericChangeId,
patchNum: PatchSetNum,
diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.js b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.js
index 4bc7dd3..717fe54 100644
--- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.js
+++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.js
@@ -649,19 +649,6 @@
{message: 'revising...'});
});
- test('startReview', () => {
- const sendStub = sinon.stub(element, '_getChangeURLAndSend')
- .returns(Promise.resolve({}));
- element.startReview('42', {message: 'Please review.'});
- assert.isTrue(sendStub.calledOnce);
- assert.equal(sendStub.lastCall.args[0].changeNum, '42');
- assert.equal(sendStub.lastCall.args[0].method, 'POST');
- assert.isNotOk(sendStub.lastCall.args[0].patchNum);
- assert.equal(sendStub.lastCall.args[0].endpoint, '/ready');
- assert.deepEqual(sendStub.lastCall.args[0].body,
- {message: 'Please review.'});
- });
-
test('deleteComment', () => {
const sendStub = sinon.stub(element, '_getChangeURLAndSend')
.returns(Promise.resolve('some response'));
diff --git a/polygerrit-ui/app/services/flags/flags.ts b/polygerrit-ui/app/services/flags/flags.ts
index ba33954..36fe858 100644
--- a/polygerrit-ui/app/services/flags/flags.ts
+++ b/polygerrit-ui/app/services/flags/flags.ts
@@ -28,4 +28,5 @@
NEW_CONTEXT_CONTROLS = 'UiFeature__new_context_controls',
CI_REBOOT_CHECKS = 'UiFeature__ci_reboot_checks',
NEW_CHANGE_SUMMARY_UI = 'UiFeature__new_change_summary_ui',
+ PORTING_COMMENTS = 'UiFeature__porting_comments',
}
diff --git a/polygerrit-ui/app/services/gr-reporting/gr-reporting.ts b/polygerrit-ui/app/services/gr-reporting/gr-reporting.ts
index 48c90f7..090c42f 100644
--- a/polygerrit-ui/app/services/gr-reporting/gr-reporting.ts
+++ b/polygerrit-ui/app/services/gr-reporting/gr-reporting.ts
@@ -21,10 +21,6 @@
// eslint-disable-next-line @typescript-eslint/no-explicit-any
export type EventDetails = any;
-export const PORTING_COMMENTS_DIFF_LATENCY_LABEL = 'PortingCommentsDiffLatency';
-export const PORTING_COMMENTS_CHANGE_LATENCY_LABEL =
- 'PortingCommentsChangeLatency';
-
export interface Timer {
reset(): this;
end(): this;
diff --git a/polygerrit-ui/app/services/services/gr-rest-api/gr-rest-api.ts b/polygerrit-ui/app/services/services/gr-rest-api/gr-rest-api.ts
index 8a0f912..f556af0 100644
--- a/polygerrit-ui/app/services/services/gr-rest-api/gr-rest-api.ts
+++ b/polygerrit-ui/app/services/services/gr-rest-api/gr-rest-api.ts
@@ -182,23 +182,19 @@
getChangeSuggestedReviewers(
changeNum: NumericChangeId,
- input: string,
- errFn?: ErrorCallback
+ input: string
): Promise<SuggestedReviewerInfo[] | undefined>;
getChangeSuggestedCCs(
changeNum: NumericChangeId,
- input: string,
- errFn?: ErrorCallback
+ input: string
): Promise<SuggestedReviewerInfo[] | undefined>;
getSuggestedAccounts(
input: string,
- n?: number,
- errFn?: ErrorCallback
+ n?: number
): Promise<AccountInfo[] | undefined>;
getSuggestedGroups(
input: string,
- n?: number,
- errFn?: ErrorCallback
+ n?: number
): Promise<GroupNameToGroupInfoMap | undefined>;
executeChangeAction(
changeNum: NumericChangeId,
@@ -239,7 +235,7 @@
getAccountEmails(): Promise<EmailInfo[] | undefined>;
deleteAccountEmail(email: string): Promise<Response>;
- setPreferredAccountEmail(email: string, errFn?: ErrorCallback): Promise<void>;
+ setPreferredAccountEmail(email: string): Promise<void>;
getAccountSSHKeys(): Promise<SshKeyInfo[] | undefined>;
deleteAccountSSHKey(key: string): void;
@@ -417,9 +413,7 @@
): Promise<Response>;
getDiffChangeDetail(
- changeNum: NumericChangeId,
- errFn?: ErrorCallback,
- cancelCondition?: CancelConditionCallback
+ changeNum: NumericChangeId
): Promise<ChangeInfo | undefined | null>;
getPortedComments(
@@ -534,33 +528,21 @@
generateAccountHttpPassword(): Promise<Password>;
- setAccountName(name: string, errFn?: ErrorCallback): Promise<void>;
+ setAccountName(name: string): Promise<void>;
- setAccountUsername(username: string, errFn?: ErrorCallback): Promise<void>;
+ setAccountUsername(username: string): Promise<void>;
getWatchedProjects(): Promise<ProjectWatchInfo[] | undefined>;
saveWatchedProjects(
- projects: ProjectWatchInfo[],
- errFn?: ErrorCallback
+ projects: ProjectWatchInfo[]
): Promise<ProjectWatchInfo[]>;
- deleteWatchedProjects(
- projects: ProjectWatchInfo[]
- ): Promise<Response | undefined>;
- deleteWatchedProjects(
- projects: ProjectWatchInfo[],
- errFn: ErrorCallback
- ): Promise<Response | undefined>;
- deleteWatchedProjects(
- projects: ProjectWatchInfo[],
- errFn?: ErrorCallback
- ): Promise<Response | undefined>;
+ deleteWatchedProjects(projects: ProjectWatchInfo[]): Promise<Response>;
getSuggestedProjects(
inputVal: string,
- n?: number,
- errFn?: ErrorCallback
+ n?: number
): Promise<NameToProjectInfoMap | undefined>;
invalidateGroupsCache(): void;
@@ -576,11 +558,8 @@
user: AccountId | undefined | null,
reason: string
): Promise<Response>;
- setAccountDisplayName(
- displayName: string,
- errFn?: ErrorCallback
- ): Promise<void>;
- setAccountStatus(status: string, errFn?: ErrorCallback): Promise<void>;
+ setAccountDisplayName(displayName: string): Promise<void>;
+ setAccountStatus(status: string): Promise<void>;
getAvatarChangeUrl(): Promise<string | undefined>;
setDescription(
changeNum: NumericChangeId,
@@ -744,11 +723,6 @@
addAccountEmail(email: string): Promise<Response>;
- addAccountEmail(
- email: string,
- errFn?: ErrorCallback
- ): Promise<Response | undefined>;
-
saveChangeReviewed(
changeNum: NumericChangeId,
reviewed: boolean
@@ -803,15 +777,7 @@
reviewed: boolean
): Promise<Response>;
- saveFileReviewed(
- changeNum: NumericChangeId,
- patchNum: PatchSetNum,
- path: string,
- reviewed: boolean,
- errFn: ErrorCallback
- ): Promise<Response | undefined>;
-
- getTopMenus(errFn?: ErrorCallback): Promise<TopMenuEntryInfo[] | undefined>;
+ getTopMenus(): Promise<TopMenuEntryInfo[] | undefined>;
setInProjectLookup(changeNum: NumericChangeId, project: RepoName): void;
getMergeable(changeNum: NumericChangeId): Promise<MergeableInfo | undefined>;
diff --git a/polygerrit-ui/app/styles/shared-styles.ts b/polygerrit-ui/app/styles/shared-styles.ts
index c2baaa9..695ae24 100644
--- a/polygerrit-ui/app/styles/shared-styles.ts
+++ b/polygerrit-ui/app/styles/shared-styles.ts
@@ -15,8 +15,6 @@
* limitations under the License.
*/
-import {css} from 'lit-element';
-
// Mark the file as a module. Otherwise typescript assumes this is a script
// and $_documentContainer is a global variable.
// See: https://www.typescriptlang.org/docs/handbook/modules.html
@@ -24,276 +22,189 @@
const $_documentContainer = document.createElement('template');
-export const sharedStyles = css`
- /* CSS reset */
-
- html,
- body,
- button,
- div,
- span,
- applet,
- object,
- iframe,
- h1,
- h2,
- h3,
- h4,
- h5,
- h6,
- p,
- blockquote,
- pre,
- a,
- abbr,
- acronym,
- address,
- big,
- cite,
- code,
- del,
- dfn,
- em,
- img,
- ins,
- kbd,
- q,
- s,
- samp,
- small,
- strike,
- strong,
- sub,
- sup,
- tt,
- var,
- b,
- u,
- i,
- center,
- dl,
- dt,
- dd,
- ol,
- ul,
- li,
- fieldset,
- form,
- label,
- legend,
- table,
- caption,
- tbody,
- tfoot,
- thead,
- tr,
- th,
- td,
- article,
- aside,
- canvas,
- details,
- embed,
- figure,
- figcaption,
- footer,
- header,
- hgroup,
- main,
- menu,
- nav,
- output,
- ruby,
- section,
- summary,
- time,
- mark,
- audio,
- video {
- border: 0;
- box-sizing: border-box;
- font-size: 100%;
- font: inherit;
- margin: 0;
- padding: 0;
- vertical-align: baseline;
- }
- *::after,
- *::before {
- box-sizing: border-box;
- }
- input {
- background-color: var(--background-color-primary);
- border: 1px solid var(--border-color);
- border-radius: var(--border-radius);
- box-sizing: border-box;
- color: var(--primary-text-color);
- margin: 0;
- padding: var(--spacing-s);
- }
- iron-autogrow-textarea {
- background-color: inherit;
- color: var(--primary-text-color);
- border: 1px solid var(--border-color);
- border-radius: var(--border-radius);
- padding: 0;
- box-sizing: border-box;
- /* iron-autogrow-textarea has a "-webkit-appearance: textarea" :host
- css rule, which prevents overriding the border color. Clear that. */
- -webkit-appearance: none;
-
- --iron-autogrow-textarea: {
- box-sizing: border-box;
- padding: var(--spacing-s);
- }
- }
- a {
- color: var(--link-color);
- }
- input,
- textarea,
- select,
- button {
- font: inherit;
- }
- ol,
- ul {
- list-style: none;
- }
- blockquote,
- q {
- quotes: none;
- }
- blockquote:before,
- blockquote:after,
- q:before,
- q:after {
- content: '';
- content: none;
- }
- table {
- border-collapse: collapse;
- border-spacing: 0;
- }
-
- /* Fonts */
-
- .font-normal {
- font-size: var(--font-size-normal);
- font-weight: var(--font-weight-normal);
- line-height: var(--line-height-normal);
- }
- .font-small {
- font-size: var(--font-size-small);
- font-weight: var(--font-weight-normal);
- line-height: var(--line-height-small);
- }
- .heading-1 {
- font-family: var(--header-font-family);
- font-size: var(--font-size-h1);
- font-weight: var(--font-weight-h1);
- line-height: var(--line-height-h1);
- }
- .heading-2 {
- font-family: var(--header-font-family);
- font-size: var(--font-size-h2);
- font-weight: var(--font-weight-h2);
- line-height: var(--line-height-h2);
- }
- .heading-3 {
- font-family: var(--header-font-family);
- font-size: var(--font-size-h3);
- font-weight: var(--font-weight-h3);
- line-height: var(--line-height-h3);
- }
- iron-icon {
- color: var(--deemphasized-text-color);
- --iron-icon-height: 20px;
- --iron-icon-width: 20px;
- }
-
- /* Stopgap solution until we remove hidden$ attributes. */
-
- :host([hidden]),
- [hidden] {
- display: none !important;
- }
- .separator {
- border-left: 1px solid var(--border-color);
- height: 20px;
- margin: 0 8px;
- }
- .separator.transparent {
- border-color: transparent;
- }
- paper-toggle-button {
- --paper-toggle-button-checked-bar-color: var(--link-color);
- --paper-toggle-button-checked-button-color: var(--link-color);
- }
- paper-tabs {
- font-size: var(--font-size-h3);
- font-weight: var(--font-weight-h3);
- line-height: var(--line-height-h3);
- --paper-font-common-base: {
- font-family: var(--header-font-family);
- -webkit-font-smoothing: initial;
- }
- --paper-tab-content-focused: {
- /* paper-tabs uses 700 here, which can look awkward */
- font-weight: var(--font-weight-h3);
- }
- --paper-tab-content-unselected: {
- /* paper-tabs uses 0.8 here, but we want to control the color directly */
- opacity: 1;
- color: var(--deemphasized-text-color);
- }
- }
- iron-autogrow-textarea {
- /** This is needed for firefox */
- --iron-autogrow-textarea_-_white-space: pre-wrap;
- }
- strong {
- font-weight: var(--font-weight-bold);
- }
-
- .assistive-tech-only {
- user-select: none;
- clip: rect(1px, 1px, 1px, 1px);
- height: 1px;
- margin: 0;
- overflow: hidden;
- padding: 0;
- position: absolute;
- white-space: nowrap;
- width: 1px;
- z-index: -1000;
- }
-
- /** BEGIN: loading spiner */
- .loadingSpin {
- border: 2px solid var(--disabled-button-background-color);
- border-top: 2px solid var(--primary-button-background-color);
- border-radius: 50%;
- width: 10px;
- height: 10px;
- animation: spin 2s linear infinite;
- margin-right: var(--spacing-s);
- }
- @keyframes spin {
- 0% {
- transform: rotate(0deg);
- }
- 100% {
- transform: rotate(360deg);
- }
- }
- /** END: loading spiner */
-`;
-
$_documentContainer.innerHTML = `<dom-module id="shared-styles">
<template>
<style>
- ${sharedStyles.cssText}
+
+ /* CSS reset */
+
+ html, body, button, div, span, applet, object, iframe, h1, h2, h3,
+ h4, h5, h6, p, blockquote, pre, a, abbr, acronym, address, big, cite,
+ code, del, dfn, em, img, ins, kbd, q, s, samp, small, strike, strong, sub,
+ sup, tt, var, b, u, i, center, dl, dt, dd, ol, ul, li, fieldset, form,
+ label, legend, table, caption, tbody, tfoot, thead, tr, th, td, article,
+ aside, canvas, details, embed, figure, figcaption, footer, header, hgroup,
+ main, menu, nav, output, ruby, section, summary, time, mark, audio, video {
+ border: 0;
+ box-sizing: border-box;
+ font-size: 100%;
+ font: inherit;
+ margin: 0;
+ padding: 0;
+ vertical-align: baseline;
+ }
+ *::after,
+ *::before {
+ box-sizing: border-box;
+ }
+ input {
+ background-color: var(--background-color-primary);
+ border: 1px solid var(--border-color);
+ border-radius: var(--border-radius);
+ box-sizing: border-box;
+ color: var(--primary-text-color);
+ margin: 0;
+ padding: var(--spacing-s);
+ }
+ iron-autogrow-textarea {
+ background-color: inherit;
+ color: var(--primary-text-color);
+ border: 1px solid var(--border-color);
+ border-radius: var(--border-radius);
+ padding: 0;
+ box-sizing: border-box;
+ /* iron-autogrow-textarea has a "-webkit-appearance: textarea" :host
+ css rule, which prevents overriding the border color. Clear that. */
+ -webkit-appearance: none;
+
+ --iron-autogrow-textarea: {
+ box-sizing: border-box;
+ padding: var(--spacing-s);
+ };
+ }
+ a {
+ color: var(--link-color);
+ }
+ input,
+ textarea,
+ select,
+ button {
+ font: inherit;
+ }
+ ol, ul {
+ list-style: none;
+ }
+ blockquote, q {
+ quotes: none;
+ }
+ blockquote:before, blockquote:after,
+ q:before, q:after {
+ content: '';
+ content: none;
+ }
+ table {
+ border-collapse: collapse;
+ border-spacing: 0;
+ }
+
+ /* Fonts */
+
+ .font-normal {
+ font-size: var(--font-size-normal);
+ font-weight: var(--font-weight-normal);
+ line-height: var(--line-height-normal);
+ }
+ .font-small {
+ font-size: var(--font-size-small);
+ font-weight: var(--font-weight-normal);
+ line-height: var(--line-height-small);
+ }
+ .heading-1 {
+ font-family: var(--header-font-family);
+ font-size: var(--font-size-h1);
+ font-weight: var(--font-weight-h1);
+ line-height: var(--line-height-h1);
+ }
+ .heading-2 {
+ font-family: var(--header-font-family);
+ font-size: var(--font-size-h2);
+ font-weight: var(--font-weight-h2);
+ line-height: var(--line-height-h2);
+ }
+ .heading-3 {
+ font-family: var(--header-font-family);
+ font-size: var(--font-size-h3);
+ font-weight: var(--font-weight-h3);
+ line-height: var(--line-height-h3);
+ }
+ iron-icon {
+ color: var(--deemphasized-text-color);
+ --iron-icon-height: 20px;
+ --iron-icon-width: 20px;
+ }
+
+ /* Stopgap solution until we remove hidden$ attributes. */
+
+ :host([hidden]),
+ [hidden] {
+ display: none !important;
+ }
+ .separator {
+ border-left: 1px solid var(--border-color);
+ height: 20px;
+ margin: 0 8px;
+ }
+ .separator.transparent {
+ border-color: transparent;
+ }
+ paper-toggle-button {
+ --paper-toggle-button-checked-bar-color: var(--link-color);
+ --paper-toggle-button-checked-button-color: var(--link-color);
+ }
+ paper-tabs {
+ font-size: var(--font-size-h3);
+ font-weight: var(--font-weight-h3);
+ line-height: var(--line-height-h3);
+ --paper-font-common-base: {
+ font-family: var(--header-font-family);
+ -webkit-font-smoothing: initial;
+ };
+ --paper-tab-content-focused: {
+ /* paper-tabs uses 700 here, which can look awkward */
+ font-weight: var(--font-weight-h3);
+ };
+ --paper-tab-content-unselected: {
+ /* paper-tabs uses 0.8 here, but we want to control the color directly */
+ opacity: 1;
+ color: var(--deemphasized-text-color);
+ };
+ }
+ iron-autogrow-textarea {
+ /** This is needed for firefox */
+ --iron-autogrow-textarea_-_white-space: pre-wrap;
+ }
+ strong {
+ font-weight: var(--font-weight-bold);
+ }
+
+ .assistive-tech-only {
+ user-select: none;
+ clip: rect(1px, 1px, 1px, 1px);
+ height: 1px;
+ margin: 0;
+ overflow: hidden;
+ padding: 0;
+ position: absolute;
+ white-space: nowrap;
+ width: 1px;
+ z-index: -1000;
+ }
+
+ /** BEGIN: loading spiner */
+ .loadingSpin {
+ border: 2px solid var(--disabled-button-background-color);
+ border-top: 2px solid var(--primary-button-background-color);
+ border-radius: 50%;
+ width: 10px;
+ height: 10px;
+ animation: spin 2s linear infinite;
+ margin-right: var(--spacing-s);
+ }
+ @keyframes spin {
+ 0% { transform: rotate(0deg); }
+ 100% { transform: rotate(360deg); }
+ }
+ /** END: loading spiner */
</style>
</template>
</dom-module>`;
diff --git a/polygerrit-ui/app/test/test-data-generators.ts b/polygerrit-ui/app/test/test-data-generators.ts
index 248e937..1d07cf3 100644
--- a/polygerrit-ui/app/test/test-data-generators.ts
+++ b/polygerrit-ui/app/test/test-data-generators.ts
@@ -60,6 +60,7 @@
AccountDetailInfo,
Requirement,
RequirementType,
+ UrlEncodedCommentId,
} from '../types/common';
import {
AccountsVisibility,
@@ -77,6 +78,7 @@
SubmitType,
TimeFormat,
RequirementStatus,
+ CommentSide,
} from '../constants/constants';
import {formatDate} from '../utils/date-util';
import {GetDiffCommentsOutput} from '../services/services/gr-rest-api/gr-rest-api';
@@ -88,6 +90,7 @@
} from '../elements/shared/gr-rest-api-interface/gr-reviewer-updates-parser';
import {CommitInfoWithRequiredCommit} from '../elements/change/gr-change-metadata/gr-change-metadata';
import {WebLinkInfo} from '../types/diff';
+import {UIComment, UIDraft} from '../utils/comment-util';
export function dateToTimestamp(date: Date): Timestamp {
const nanosecondSuffix = '.000000000';
@@ -441,3 +444,24 @@
image_url: 'gitiles.jpg',
};
}
+
+export function createComment(): UIComment {
+ return {
+ patch_set: 1 as PatchSetNum,
+ id: '12345' as UrlEncodedCommentId,
+ side: CommentSide.REVISION,
+ line: 1,
+ message: 'hello world',
+ updated: '2018-02-13 22:48:48.018000000' as Timestamp,
+ unresolved: false,
+ };
+}
+
+export function createDraft(): UIDraft {
+ return {
+ ...createComment(),
+ collapsed: false,
+ __draft: true,
+ __editing: false,
+ };
+}
diff --git a/polygerrit-ui/app/types/common.ts b/polygerrit-ui/app/types/common.ts
index b454b6e..1628134 100644
--- a/polygerrit-ui/app/types/common.ts
+++ b/polygerrit-ui/app/types/common.ts
@@ -1175,11 +1175,6 @@
export type PathToCommentsInfoMap = {[path: string]: CommentInfo[]};
-export type PortedCommentsAndDrafts = {
- portedComments?: PathToCommentsInfoMap;
- portedDrafts?: PathToCommentsInfoMap;
-};
-
/**
* The CommentRange entity describes the range of an inline comment.
* https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#comment-range
diff --git a/polygerrit-ui/app/utils/comment-util.ts b/polygerrit-ui/app/utils/comment-util.ts
index 0726f67..8627591 100644
--- a/polygerrit-ui/app/utils/comment-util.ts
+++ b/polygerrit-ui/app/utils/comment-util.ts
@@ -158,6 +158,7 @@
rootId?: UrlEncodedCommentId;
diffSide?: Side;
range?: CommentRange;
+ ported?: boolean; // is the comment ported over from a previous patchset
}
export function getLastComment(thread?: CommentThread): UIComment | undefined {
diff --git a/polygerrit-ui/app/utils/event-util.ts b/polygerrit-ui/app/utils/event-util.ts
index 36341bd..3dd1b9e 100644
--- a/polygerrit-ui/app/utils/event-util.ts
+++ b/polygerrit-ui/app/utils/event-util.ts
@@ -25,6 +25,15 @@
TITLE_CHANGE = 'title-change',
}
+export function fireEvent(target: EventTarget, type: string) {
+ target.dispatchEvent(
+ new CustomEvent(type, {
+ composed: true,
+ bubbles: true,
+ })
+ );
+}
+
export function fireAlert(target: EventTarget, message: string) {
target.dispatchEvent(
new CustomEvent(EventType.SHOW_ALERT, {
diff --git a/polygerrit-ui/app/utils/label-util.ts b/polygerrit-ui/app/utils/label-util.ts
index 4313745..fc83d77 100644
--- a/polygerrit-ui/app/utils/label-util.ts
+++ b/polygerrit-ui/app/utils/label-util.ts
@@ -15,7 +15,9 @@
* limitations under the License.
*/
import {
+ AccountInfo,
ApprovalInfo,
+ DetailedLabelInfo,
isDetailedLabelInfo,
LabelInfo,
VotingRangeInfo,
@@ -42,3 +44,10 @@
const votingRange = getVotingRangeOrDefault(label);
return label.all.filter(account => account.value === votingRange.max);
}
+
+export function getApprovalInfo(
+ label: DetailedLabelInfo,
+ account: AccountInfo
+): ApprovalInfo | undefined {
+ return label.all?.filter(x => x._account_id === account._account_id)[0];
+}
diff --git a/polygerrit-ui/app/utils/label-util_test.js b/polygerrit-ui/app/utils/label-util_test.js
index d6f7b3e..6a2f768 100644
--- a/polygerrit-ui/app/utils/label-util_test.js
+++ b/polygerrit-ui/app/utils/label-util_test.js
@@ -20,6 +20,7 @@
getVotingRange,
getVotingRangeOrDefault,
getMaxAccounts,
+ getApprovalInfo,
} from './label-util.js';
const VALUES_1 = {
@@ -87,4 +88,29 @@
assert.isEmpty(getMaxAccounts({}));
assert.isEmpty(getMaxAccounts({values: VALUES_2}));
});
+
+ test('getApprovalInfo', () => {
+ const myAccountInfo = {_account_id: 314};
+ const myApprovalInfo = {value: 2, _account_id: 314};
+ const label = {
+ values: VALUES_2,
+ all: [myApprovalInfo, {value: 1, _account_id: 777}],
+ };
+ assert.equal(
+ getApprovalInfo(label, myAccountInfo),
+ myApprovalInfo
+ );
+ });
+
+ test('getApprovalInfo no approval for user', () => {
+ const myAccountInfo = {_account_id: 123};
+ const label = {
+ values: VALUES_2,
+ all: [
+ {value: 2, _account_id: 314},
+ {value: 1, _account_id: 777},
+ ],
+ };
+ assert.isUndefined(getApprovalInfo(label, myAccountInfo));
+ });
});
