Merge "Filter robotComments without Human Reply"
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index 77979a7..618621b 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -3412,9 +3412,9 @@
[[experiments]]
=== Section experiments
-This section covers experimental new features. Gerrit's frontend uses experiments
-to research new behavior. Once the research is done, the experimental feature
-either stays and the experimentation flag gets removed, or the feature as a whole
+This section covers experimental new features. Gerrit uses experiments
+to research new behavior in frontend and core backend. Once the research is done, the experimental
+feature either stays and the experimentation flag gets removed, or the feature as a whole
gets removed
[[experiments.enabled]]experiments.enabled::
diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt
index 9876b53..096b068 100644
--- a/Documentation/rest-api-changes.txt
+++ b/Documentation/rest-api-changes.txt
@@ -14,10 +14,11 @@
'POST /changes/'
--
-The change input link:#change-input[ChangeInput] entity must be provided in the
-request body. It is not allowed to create changes on refs/tags/* or Gerrit
-internal refs such as refs/changes/*, refs/meta/external-ids/*, refs/users/*,
-etc.. and the request would fail with `400 Bad Request` in this case.
+The change input link:#change-input[ChangeInput] entity must be
+provided in the request body. It is not allowed to create changes
+under `refs/tags/` or Gerrit internal ref namespaces such as
+`refs/changes/`, `refs/meta/external-ids/`, and `refs/users/`. The
+request would fail with `400 Bad Request` in this case.
To create a change the calling user must be allowed to
link:access-control.html#category_push_review[upload to code review].
diff --git a/java/com/google/gerrit/acceptance/GerritServer.java b/java/com/google/gerrit/acceptance/GerritServer.java
index b02dc87..f2cc9d1 100644
--- a/java/com/google/gerrit/acceptance/GerritServer.java
+++ b/java/com/google/gerrit/acceptance/GerritServer.java
@@ -50,6 +50,7 @@
import com.google.gerrit.server.config.GerritRuntime;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.config.SitePath;
+import com.google.gerrit.server.experiments.ConfigExperimentFeatures;
import com.google.gerrit.server.git.receive.AsyncReceiveCommits;
import com.google.gerrit.server.schema.JdbcAccountPatchReviewStore;
import com.google.gerrit.server.ssh.NoSshModule;
@@ -437,7 +438,8 @@
protected void configure() {
bind(GerritRuntime.class).toInstance(GerritRuntime.DAEMON);
}
- }));
+ },
+ new ConfigExperimentFeatures.Module()));
daemon.addAdditionalSysModuleForTesting(
new ReindexProjectsAtStartup.Module(), new ReindexGroupsAtStartup.Module());
daemon.start();
diff --git a/java/com/google/gerrit/httpd/raw/IndexHtmlUtil.java b/java/com/google/gerrit/httpd/raw/IndexHtmlUtil.java
index 46dde41..8d52f5a 100644
--- a/java/com/google/gerrit/httpd/raw/IndexHtmlUtil.java
+++ b/java/com/google/gerrit/httpd/raw/IndexHtmlUtil.java
@@ -20,7 +20,6 @@
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.ImmutableSet;
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.UsedAt;
@@ -31,6 +30,7 @@
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.json.OutputFormat;
+import com.google.gerrit.server.experiments.ExperimentFeatures;
import com.google.gson.Gson;
import com.google.template.soy.data.SanitizedContent;
import java.net.URI;
@@ -38,21 +38,15 @@
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
-import java.util.HashSet;
import java.util.Map;
import java.util.Set;
import java.util.function.Function;
-import org.eclipse.jgit.lib.Config;
/** Helper for generating parts of {@code index.html}. */
@UsedAt(Project.GOOGLE)
public class IndexHtmlUtil {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
- static final ImmutableSet<String> DEFAULT_EXPERIMENTS =
- ImmutableSet.of(
- "UiFeature__patchset_comments", "UiFeature__patchset_choice_for_comment_links");
-
private static final Gson GSON = OutputFormat.JSON_COMPACT.newGson();
/**
* Returns both static and dynamic parameters of {@code index.html}. The result is to be used when
@@ -60,7 +54,7 @@
*/
public static ImmutableMap<String, Object> templateData(
GerritApi gerritApi,
- Config gerritServerConfig,
+ ExperimentFeatures experimentFeatures,
String canonicalURL,
String cdnPath,
String faviconPath,
@@ -73,14 +67,8 @@
staticTemplateData(
canonicalURL, cdnPath, faviconPath, urlParameterMap, urlInScriptTagOrdainer))
.putAll(dynamicTemplateData(gerritApi, requestedURL));
+ Set<String> enabledExperiments = experimentFeatures.getEnabledExperimentFeatures();
- Set<String> enabledExperiments = new HashSet<>();
- Arrays.stream(gerritServerConfig.getStringList("experiments", null, "enabled"))
- .forEach(enabledExperiments::add);
- DEFAULT_EXPERIMENTS.forEach(enabledExperiments::add);
- Arrays.stream(gerritServerConfig.getStringList("experiments", null, "disabled"))
- .forEach(enabledExperiments::remove);
- experimentData(urlParameterMap).forEach(enabledExperiments::add);
if (!enabledExperiments.isEmpty()) {
data.put("enabledExperiments", serializeObject(GSON, enabledExperiments).toString());
}
diff --git a/java/com/google/gerrit/httpd/raw/IndexServlet.java b/java/com/google/gerrit/httpd/raw/IndexServlet.java
index b2bdf7c..3f2c202 100644
--- a/java/com/google/gerrit/httpd/raw/IndexServlet.java
+++ b/java/com/google/gerrit/httpd/raw/IndexServlet.java
@@ -22,6 +22,7 @@
import com.google.gerrit.common.Nullable;
import com.google.gerrit.extensions.api.GerritApi;
import com.google.gerrit.extensions.restapi.RestApiException;
+import com.google.gerrit.server.experiments.ExperimentFeatures;
import com.google.template.soy.SoyFileSet;
import com.google.template.soy.data.SanitizedContent;
import com.google.template.soy.data.UnsafeSanitizedContentOrdainer;
@@ -34,7 +35,6 @@
import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
-import org.eclipse.jgit.lib.Config;
public class IndexServlet extends HttpServlet {
private static final long serialVersionUID = 1L;
@@ -43,7 +43,7 @@
@Nullable private final String cdnPath;
@Nullable private final String faviconPath;
private final GerritApi gerritApi;
- private final Config gerritServerConfig;
+ private final ExperimentFeatures experimentFeatures;
private final SoySauce soySauce;
private final Function<String, SanitizedContent> urlOrdainer;
@@ -52,12 +52,12 @@
@Nullable String cdnPath,
@Nullable String faviconPath,
GerritApi gerritApi,
- Config gerritServerConfig) {
+ ExperimentFeatures experimentFeatures) {
this.canonicalUrl = canonicalUrl;
this.cdnPath = cdnPath;
this.faviconPath = faviconPath;
this.gerritApi = gerritApi;
- this.gerritServerConfig = gerritServerConfig;
+ this.experimentFeatures = experimentFeatures;
this.soySauce =
SoyFileSet.builder()
.add(Resources.getResource("com/google/gerrit/httpd/raw/PolyGerritIndexHtml.soy"))
@@ -79,7 +79,7 @@
ImmutableMap<String, Object> templateData =
IndexHtmlUtil.templateData(
gerritApi,
- gerritServerConfig,
+ experimentFeatures,
canonicalUrl,
cdnPath,
faviconPath,
diff --git a/java/com/google/gerrit/httpd/raw/StaticModule.java b/java/com/google/gerrit/httpd/raw/StaticModule.java
index 66e107b..cac716f 100644
--- a/java/com/google/gerrit/httpd/raw/StaticModule.java
+++ b/java/com/google/gerrit/httpd/raw/StaticModule.java
@@ -30,6 +30,7 @@
import com.google.gerrit.server.config.GerritOptions;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.config.SitePaths;
+import com.google.gerrit.server.experiments.ExperimentFeatures;
import com.google.inject.Inject;
import com.google.inject.Key;
import com.google.inject.Provides;
@@ -221,11 +222,12 @@
HttpServlet getPolyGerritUiIndexServlet(
@CanonicalWebUrl @Nullable String canonicalUrl,
@GerritServerConfig Config cfg,
- GerritApi gerritApi) {
+ GerritApi gerritApi,
+ ExperimentFeatures experimentFeatures) {
String cdnPath =
options.useDevCdn() ? options.devCdn() : cfg.getString("gerrit", null, "cdnPath");
String faviconPath = cfg.getString("gerrit", null, "faviconPath");
- return new IndexServlet(canonicalUrl, cdnPath, faviconPath, gerritApi, cfg);
+ return new IndexServlet(canonicalUrl, cdnPath, faviconPath, gerritApi, experimentFeatures);
}
@Provides
diff --git a/java/com/google/gerrit/pgm/util/SiteProgram.java b/java/com/google/gerrit/pgm/util/SiteProgram.java
index 98558fb..c3be0a4 100644
--- a/java/com/google/gerrit/pgm/util/SiteProgram.java
+++ b/java/com/google/gerrit/pgm/util/SiteProgram.java
@@ -28,6 +28,7 @@
import com.google.gerrit.server.config.GerritRuntime;
import com.google.gerrit.server.config.GerritServerConfigModule;
import com.google.gerrit.server.config.SitePath;
+import com.google.gerrit.server.experiments.ConfigExperimentFeatures;
import com.google.gerrit.server.git.GitRepositoryManagerModule;
import com.google.gerrit.server.git.SystemReaderInstaller;
import com.google.gerrit.server.schema.SchemaModule;
@@ -128,6 +129,9 @@
modules.add(new SchemaModule());
modules.add(cfgInjector.getInstance(GitRepositoryManagerModule.class));
+ // The only implementation of experiments is available in all programs that can use
+ // gerrit.config
+ modules.add(new ConfigExperimentFeatures.Module());
try {
return Guice.createInjector(
diff --git a/java/com/google/gerrit/server/experiments/ConfigExperimentFeatures.java b/java/com/google/gerrit/server/experiments/ConfigExperimentFeatures.java
new file mode 100644
index 0000000..f526935
--- /dev/null
+++ b/java/com/google/gerrit/server/experiments/ConfigExperimentFeatures.java
@@ -0,0 +1,63 @@
+// Copyright (C) 2021 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.experiments;
+
+import com.google.common.collect.ImmutableSet;
+import com.google.gerrit.server.config.GerritServerConfig;
+import com.google.inject.AbstractModule;
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.Set;
+import org.eclipse.jgit.lib.Config;
+
+/**
+ * An implementation of {@link ExperimentFeatures} that uses gerrit.config to evaluate the status of
+ * the feature.
+ */
+@Singleton
+public class ConfigExperimentFeatures implements ExperimentFeatures {
+
+ public static class Module extends AbstractModule {
+ @Override
+ protected void configure() {
+ bind(ExperimentFeatures.class).to(ConfigExperimentFeatures.class);
+ }
+ }
+
+ private ImmutableSet<String> enabledExperimentFeatures;
+
+ @Inject
+ public ConfigExperimentFeatures(@GerritServerConfig Config gerritServerConfig) {
+ Set<String> enabledExperiments = new HashSet<>();
+ Arrays.stream(gerritServerConfig.getStringList("experiments", null, "enabled"))
+ .forEach(enabledExperiments::add);
+ ExperimentFeaturesConstants.DEFAULT_ENABLED_FEATURES.forEach(enabledExperiments::add);
+ Arrays.stream(gerritServerConfig.getStringList("experiments", null, "disabled"))
+ .forEach(enabledExperiments::remove);
+ enabledExperimentFeatures = ImmutableSet.copyOf(enabledExperiments);
+ }
+
+ @Override
+ public boolean isFeatureEnabled(String featureFlag) {
+ return getEnabledExperimentFeatures().contains(featureFlag);
+ }
+
+ @Override
+ public ImmutableSet<String> getEnabledExperimentFeatures() {
+ return enabledExperimentFeatures;
+ }
+}
diff --git a/java/com/google/gerrit/server/experiments/ExperimentFeatures.java b/java/com/google/gerrit/server/experiments/ExperimentFeatures.java
new file mode 100644
index 0000000..dc9148a
--- /dev/null
+++ b/java/com/google/gerrit/server/experiments/ExperimentFeatures.java
@@ -0,0 +1,44 @@
+// Copyright (C) 2021 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.experiments;
+
+import com.google.common.collect.ImmutableSet;
+
+/**
+ * Features that can be enabled/disabled on Gerrit (e. g. experiments to research new behavior in
+ * the current release).
+ *
+ * <p>It may depend on the implementation if the result is decided on the per-request basis or not,
+ * so the outcomes should not be persisted in {@link com.google.inject.Singleton}.
+ */
+public interface ExperimentFeatures {
+
+ /**
+ * Given the name of the feature, returns if it is enabled on the Gerrit server.
+ *
+ * <p>Depending on the implementation, it can be more efficient than filtering the results of
+ * {@link ExperimentFeatures#getEnabledExperimentFeatures}.
+ *
+ * @param featureFlag the name of the feature to test.
+ * @return if the feature is enabled.
+ */
+ boolean isFeatureEnabled(String featureFlag);
+
+ /**
+ * Returns the names of the features that are enabled on Gerrit instance (either by default or via
+ * gerrit.config).
+ */
+ ImmutableSet<String> getEnabledExperimentFeatures();
+}
diff --git a/java/com/google/gerrit/server/experiments/ExperimentFeaturesConstants.java b/java/com/google/gerrit/server/experiments/ExperimentFeaturesConstants.java
new file mode 100644
index 0000000..af49438
--- /dev/null
+++ b/java/com/google/gerrit/server/experiments/ExperimentFeaturesConstants.java
@@ -0,0 +1,28 @@
+// Copyright (C) 2021 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.experiments;
+
+import com.google.common.collect.ImmutableSet;
+
+/** Constants for Gerrit {@link ExperimentFeatures} */
+public class ExperimentFeaturesConstants {
+
+ /** Features that are known experiments and can be referenced in the code. */
+ public static String UI_FEATURE_PATCHSET_COMMENTS = "UiFeature__patchset_comments";
+
+ /** Features, enabled by default in the current release. */
+ public static final ImmutableSet<String> DEFAULT_ENABLED_FEATURES =
+ ImmutableSet.of(UI_FEATURE_PATCHSET_COMMENTS);
+}
diff --git a/javatests/com/google/gerrit/acceptance/server/experiments/BUILD b/javatests/com/google/gerrit/acceptance/server/experiments/BUILD
new file mode 100644
index 0000000..0f01ffa
--- /dev/null
+++ b/javatests/com/google/gerrit/acceptance/server/experiments/BUILD
@@ -0,0 +1,7 @@
+load("//javatests/com/google/gerrit/acceptance:tests.bzl", "acceptance_tests")
+
+acceptance_tests(
+ srcs = glob(["*IT.java"]),
+ group = "server_experiments",
+ labels = ["server"],
+)
diff --git a/javatests/com/google/gerrit/acceptance/server/experiments/ExperimentFeaturesIT.java b/javatests/com/google/gerrit/acceptance/server/experiments/ExperimentFeaturesIT.java
new file mode 100644
index 0000000..09e6dfe
--- /dev/null
+++ b/javatests/com/google/gerrit/acceptance/server/experiments/ExperimentFeaturesIT.java
@@ -0,0 +1,77 @@
+// Copyright (C) 2021 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.experiments;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.common.collect.ImmutableSet;
+import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.config.GerritConfig;
+import com.google.gerrit.server.experiments.ExperimentFeatures;
+import com.google.gerrit.server.experiments.ExperimentFeaturesConstants;
+import com.google.inject.Inject;
+import org.junit.Test;
+
+/** Tests for {@link ExperimentFeatures} */
+public class ExperimentFeaturesIT extends AbstractDaemonTest {
+
+ @Inject ExperimentFeatures experimentFeatures;
+
+ @Test
+ public void emptyConfig_defaultFeatures_enabled() {
+ for (String defaultFeature : ExperimentFeaturesConstants.DEFAULT_ENABLED_FEATURES) {
+ assertThat(experimentFeatures.isFeatureEnabled(defaultFeature)).isTrue();
+ }
+
+ assertThat(experimentFeatures.getEnabledExperimentFeatures())
+ .isEqualTo(ExperimentFeaturesConstants.DEFAULT_ENABLED_FEATURES);
+ }
+
+ @Test
+ @GerritConfig(
+ name = "experiments.enabled",
+ values = {"enabledFeature", "enabledThenDisabledFeature"})
+ @GerritConfig(
+ name = "experiments.disabled",
+ values = {"enabledThenDisabledFeature"})
+ public void configOverride_anyFeatureAllowed() {
+ assertThat(experimentFeatures.isFeatureEnabled("enabledFeature")).isTrue();
+ assertThat(experimentFeatures.isFeatureEnabled("enabledThenDisabledFeature")).isFalse();
+ assertThat(experimentFeatures.isFeatureEnabled("unknownFeature")).isFalse();
+ ImmutableSet<String> expectedEnabledFeatures =
+ new ImmutableSet.Builder<String>()
+ .addAll(ExperimentFeaturesConstants.DEFAULT_ENABLED_FEATURES)
+ .add("enabledFeature")
+ .build();
+ assertThat(experimentFeatures.getEnabledExperimentFeatures())
+ .isEqualTo(expectedEnabledFeatures);
+ }
+
+ @Test
+ @GerritConfig(
+ name = "experiments.enabled",
+ values = {"enabledFeature"})
+ @GerritConfig(
+ name = "experiments.disabled",
+ values = {"UiFeature__patchset_comments"})
+ public void configOverride_defaultFeatureDisabled() {
+ assertThat(experimentFeatures.isFeatureEnabled("enabledFeature")).isTrue();
+ assertThat(
+ experimentFeatures.isFeatureEnabled(
+ ExperimentFeaturesConstants.UI_FEATURE_PATCHSET_COMMENTS))
+ .isFalse();
+ assertThat(experimentFeatures.getEnabledExperimentFeatures()).containsExactly("enabledFeature");
+ }
+}
diff --git a/javatests/com/google/gerrit/httpd/raw/IndexServletTest.java b/javatests/com/google/gerrit/httpd/raw/IndexServletTest.java
index ba9475f..634231f 100644
--- a/javatests/com/google/gerrit/httpd/raw/IndexServletTest.java
+++ b/javatests/com/google/gerrit/httpd/raw/IndexServletTest.java
@@ -15,19 +15,24 @@
package com.google.gerrit.httpd.raw;
import static com.google.common.truth.Truth.assertThat;
-import static java.util.stream.Collectors.joining;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
import com.google.gerrit.extensions.api.GerritApi;
import com.google.gerrit.extensions.api.accounts.Accounts;
import com.google.gerrit.extensions.api.config.Config;
import com.google.gerrit.extensions.api.config.Server;
import com.google.gerrit.extensions.common.ServerInfo;
import com.google.gerrit.extensions.restapi.AuthException;
+import com.google.gerrit.server.experiments.ConfigExperimentFeatures;
+import com.google.gerrit.server.experiments.ExperimentFeatures;
+import com.google.gerrit.server.experiments.ExperimentFeaturesConstants;
import com.google.gerrit.util.http.testutil.FakeHttpServletRequest;
import com.google.gerrit.util.http.testutil.FakeHttpServletResponse;
+import java.util.ArrayList;
+import java.util.List;
import org.junit.Test;
public class IndexServletTest {
@@ -55,14 +60,19 @@
String testCdnPath = "bar-cdn";
String testFaviconURL = "zaz-url";
- String disabledDefault = IndexHtmlUtil.DEFAULT_EXPERIMENTS.asList().get(0);
+ // Pick any known experiment enabled by default;
+ String disabledDefault = ExperimentFeaturesConstants.UI_FEATURE_PATCHSET_COMMENTS;
+ assertThat(ExperimentFeaturesConstants.DEFAULT_ENABLED_FEATURES).contains(disabledDefault);
+
org.eclipse.jgit.lib.Config serverConfig = new org.eclipse.jgit.lib.Config();
serverConfig.setStringList(
"experiments", null, "enabled", ImmutableList.of("NewFeature", "DisabledFeature"));
serverConfig.setStringList(
"experiments", null, "disabled", ImmutableList.of("DisabledFeature", disabledDefault));
+ ExperimentFeatures experimentFeatures = new ConfigExperimentFeatures(serverConfig);
IndexServlet servlet =
- new IndexServlet(testCanonicalUrl, testCdnPath, testFaviconURL, gerritApi, serverConfig);
+ new IndexServlet(
+ testCanonicalUrl, testCdnPath, testFaviconURL, gerritApi, experimentFeatures);
FakeHttpServletResponse response = new FakeHttpServletResponse();
@@ -85,14 +95,17 @@
+ "\\x22\\/config\\/server\\/info\\x22: \\x7b\\x22default_theme\\x22:"
+ "\\x22my-default-theme\\x22\\x7d, \\x22\\/config\\/server\\/top-menus\\x22: "
+ "\\x5b\\x5d\\x7d');");
- String enabledDefaults =
- IndexHtmlUtil.DEFAULT_EXPERIMENTS.stream()
+ ImmutableSet<String> enabledDefaults =
+ ExperimentFeaturesConstants.DEFAULT_ENABLED_FEATURES.stream()
.filter(e -> !e.equals(disabledDefault))
- .collect(joining("\\x22,"));
+ .collect(ImmutableSet.toImmutableSet());
+ List<String> expectedEnabled = new ArrayList<>();
+ expectedEnabled.add("NewFeature");
+ expectedEnabled.addAll(enabledDefaults);
assertThat(output)
.contains(
- "window.ENABLED_EXPERIMENTS = JSON.parse('\\x5b\\x22NewFeature\\x22,\\x22"
- + enabledDefaults
+ "window.ENABLED_EXPERIMENTS = JSON.parse('\\x5b\\x22"
+ + String.join("\\x22,", expectedEnabled)
+ "\\x22\\x5d');</script>");
}
}
diff --git a/polygerrit-ui/app/elements/change/gr-change-summary/gr-change-summary.ts b/polygerrit-ui/app/elements/change/gr-change-summary/gr-change-summary.ts
index 991a6cc..021d7c4 100644
--- a/polygerrit-ui/app/elements/change/gr-change-summary/gr-change-summary.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-summary/gr-change-summary.ts
@@ -79,8 +79,9 @@
color: var(--chip-color);
cursor: pointer;
display: inline-block;
- padding: var(--spacing-xxs) var(--spacing-s) var(--spacing-xxs)
+ padding: var(--spacing-xxs) var(--spacing-m) var(--spacing-xxs)
var(--spacing-s);
+ margin-right: var(--spacing-s);
border-radius: 12px;
border: 1px solid gray;
vertical-align: top;
@@ -410,15 +411,13 @@
!!countUnresolvedComments}
>
No Comments</gr-summary-chip
- >
- <gr-summary-chip
+ ><gr-summary-chip
styleType=${SummaryChipStyles.WARNING}
icon="edit"
?hidden=${!draftCount}
>
${pluralize(draftCount, 'draft')}</gr-summary-chip
- >
- <gr-summary-chip
+ ><gr-summary-chip
styleType=${SummaryChipStyles.WARNING}
icon="message"
?hidden=${!countUnresolvedComments}
@@ -432,8 +431,7 @@
></gr-avatar>`
)}
${countUnresolvedComments} unresolved</gr-summary-chip
- >
- <gr-summary-chip
+ ><gr-summary-chip
styleType=${SummaryChipStyles.CHECK}
icon="markChatRead"
?hidden=${!countResolvedComments}
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 95a6af9..36764f3 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
@@ -137,7 +137,7 @@
};
const ButtonTooltips = {
- SAVE: 'Save but do not send notification or change review state',
+ SAVE: 'Send changes and comments as work in progress but do not start review',
START_REVIEW: 'Mark as ready for review and send reply',
SEND: 'Send reply',
DISABLED_COMMENT_EDITING: 'Save draft comments to enable send',
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_html.ts b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_html.ts
index 1235512..79569bc 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_html.ts
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_html.ts
@@ -577,7 +577,7 @@
has-tooltip=""
title="[[_saveTooltip]]"
on-click="_saveClickHandler"
- >Save</gr-button
+ >Send As WIP</gr-button
>
</template>
<gr-button
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 cb64001..6ee82c9 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
@@ -326,6 +326,10 @@
return this.restApiService.getLoggedIn();
}
+ _getUnresolvedLabel(unresolved?: boolean) {
+ return unresolved ? 'Unresolved' : 'Resolved';
+ }
+
@observe('comments.*')
_commentsChanged() {
this._orderedComments = sortComments(this.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 ba541be..1a89a79 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
@@ -132,7 +132,7 @@
id="commentInfoContainer"
hidden$="[[_hideActions(_showActions, _lastComment)]]"
>
- <span id="unresolvedLabel" hidden$="[[!unresolved]]">Unresolved</span>
+ <span id="unresolvedLabel">[[_getUnresolvedLabel(unresolved)]]</span>
<div id="actions">
<gr-button
id="replyBtn"
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 e1c6bff..f2df89d 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
@@ -836,7 +836,7 @@
element.unresolved = false;
const label = element.shadowRoot?.querySelector('#unresolvedLabel');
assert.isOk(label);
- assert.isTrue(label!.hasAttribute('hidden'));
+ assert.isFalse(label!.hasAttribute('hidden'));
element.unresolved = true;
assert.isFalse(label!.hasAttribute('hidden'));
});
diff --git a/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content_html.ts b/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content_html.ts
index 73dc2d2..374cc62 100644
--- a/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content_html.ts
+++ b/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content_html.ts
@@ -38,7 +38,7 @@
}
.editor.new-change-summary-true iron-autogrow-textarea,
.viewer.new-change-summary-true {
- min-height: 160px;
+ min-height: 100px;
}
.editor iron-autogrow-textarea {
background-color: var(--view-background-color);
diff --git a/polygerrit-ui/app/services/flags/flags.ts b/polygerrit-ui/app/services/flags/flags.ts
index 891716b..f96afaa 100644
--- a/polygerrit-ui/app/services/flags/flags.ts
+++ b/polygerrit-ui/app/services/flags/flags.ts
@@ -30,4 +30,5 @@
CI_REBOOT_CHECKS = 'UiFeature__ci_reboot_checks',
NEW_CHANGE_SUMMARY_UI = 'UiFeature__new_change_summary_ui',
PORTING_COMMENTS = 'UiFeature__porting_comments',
+ NEW_IMAGE_DIFF_UI = 'UiFeature__new_image_diff_ui',
}
