Merge "Tweak `isExpandable` of check result row"
diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt
index 75d8847..2810d1e 100644
--- a/Documentation/rest-api-changes.txt
+++ b/Documentation/rest-api-changes.txt
@@ -6709,6 +6709,7 @@
|`patch` |required|
The patch to be applied. Must be compatible with `git diff` output.
For example, link:#get-patch[Get Patch] output.
+The patch must be provided as UTF-8 text, either directly or base64-encoded.
|=================================
[[applypatchpatchset-input]]
diff --git a/java/com/google/gerrit/acceptance/GerritServer.java b/java/com/google/gerrit/acceptance/GerritServer.java
index a149f29..36bc3c4 100644
--- a/java/com/google/gerrit/acceptance/GerritServer.java
+++ b/java/com/google/gerrit/acceptance/GerritServer.java
@@ -67,6 +67,7 @@
import com.google.gerrit.server.util.ReplicaUtil;
import com.google.gerrit.server.util.SocketUtil;
import com.google.gerrit.server.util.SystemLog;
+import com.google.gerrit.testing.FakeAccountPatchReviewStore.FakeAccountPatchReviewStoreModule;
import com.google.gerrit.testing.FakeEmailSender.FakeEmailSenderModule;
import com.google.gerrit.testing.InMemoryRepositoryManager;
import com.google.gerrit.testing.SshMode;
@@ -414,6 +415,7 @@
}
},
site);
+ daemon.setAccountPatchReviewStoreModuleForTesting(new FakeAccountPatchReviewStoreModule());
daemon.setEmailModuleForTesting(new FakeEmailSenderModule());
daemon.setAuditEventModuleForTesting(
MoreObjects.firstNonNull(testAuditModule, new FakeGroupAuditServiceModule()));
diff --git a/java/com/google/gerrit/acceptance/TestMetricMaker.java b/java/com/google/gerrit/acceptance/TestMetricMaker.java
index 85c5b6d..647eb9d 100644
--- a/java/com/google/gerrit/acceptance/TestMetricMaker.java
+++ b/java/com/google/gerrit/acceptance/TestMetricMaker.java
@@ -14,20 +14,26 @@
package com.google.gerrit.acceptance;
+import com.google.auto.value.AutoValue;
+import com.google.common.collect.ImmutableList;
import com.google.gerrit.metrics.Counter0;
+import com.google.gerrit.metrics.Counter1;
+import com.google.gerrit.metrics.Counter2;
+import com.google.gerrit.metrics.Counter3;
import com.google.gerrit.metrics.Description;
import com.google.gerrit.metrics.DisabledMetricMaker;
+import com.google.gerrit.metrics.Field;
import com.google.inject.Singleton;
+import java.util.Arrays;
import java.util.concurrent.ConcurrentHashMap;
import org.apache.commons.lang3.mutable.MutableLong;
/**
* {@link com.google.gerrit.metrics.MetricMaker} to be bound in tests.
*
- * <p>Records how often {@link Counter0} metrics are invoked. Metrics of other types are not
- * recorded.
+ * <p>Records how often counter metrics are invoked. Metrics of other types are not recorded.
*
- * <p>Allows test to check how much a {@link Counter0} metrics is increased by an operation.
+ * <p>Allows test to check how much a counter metrics is increased by an operation.
*
* <p>Example:
*
@@ -48,18 +54,18 @@
*/
@Singleton
public class TestMetricMaker extends DisabledMetricMaker {
- private final ConcurrentHashMap<String, MutableLong> counts = new ConcurrentHashMap<>();
+ private final ConcurrentHashMap<CounterKey, MutableLong> counts = new ConcurrentHashMap<>();
- public long getCount(String counter0Name) {
- return get(counter0Name).longValue();
+ public long getCount(String counterName, Object... fieldValues) {
+ return get(CounterKey.create(counterName, fieldValues)).longValue();
}
public void reset() {
counts.clear();
}
- private MutableLong get(String counter0Name) {
- return counts.computeIfAbsent(counter0Name, name -> new MutableLong(0));
+ private MutableLong get(CounterKey counterKey) {
+ return counts.computeIfAbsent(counterKey, key -> new MutableLong(0));
}
@Override
@@ -67,11 +73,64 @@
return new Counter0() {
@Override
public void incrementBy(long value) {
- get(name).add(value);
+ get(CounterKey.create(name)).add(value);
}
@Override
public void remove() {}
};
}
+
+ @Override
+ public <F1> Counter1<F1> newCounter(String name, Description desc, Field<F1> field1) {
+ return new Counter1<>() {
+ @Override
+ public void incrementBy(F1 field1, long value) {
+ get(CounterKey.create(name, field1)).add(value);
+ }
+
+ @Override
+ public void remove() {}
+ };
+ }
+
+ @Override
+ public <F1, F2> Counter2<F1, F2> newCounter(
+ String name, Description desc, Field<F1> field1, Field<F2> field2) {
+ return new Counter2<>() {
+ @Override
+ public void incrementBy(F1 field1, F2 field2, long value) {
+ get(CounterKey.create(name, field1, field2)).add(value);
+ }
+
+ @Override
+ public void remove() {}
+ };
+ }
+
+ @Override
+ public <F1, F2, F3> Counter3<F1, F2, F3> newCounter(
+ String name, Description desc, Field<F1> field1, Field<F2> field2, Field<F3> field3) {
+ return new Counter3<>() {
+ @Override
+ public void incrementBy(F1 field1, F2 field2, F3 field3, long value) {
+ get(CounterKey.create(name, field1, field2, field3)).add(value);
+ }
+
+ @Override
+ public void remove() {}
+ };
+ }
+
+ @AutoValue
+ abstract static class CounterKey {
+ abstract String name();
+
+ abstract ImmutableList<Object> fieldValues();
+
+ static CounterKey create(String name, Object... fieldValues) {
+ return new AutoValue_TestMetricMaker_CounterKey(
+ name, ImmutableList.copyOf(Arrays.asList(fieldValues)));
+ }
+ }
}
diff --git a/java/com/google/gerrit/entities/RefNames.java b/java/com/google/gerrit/entities/RefNames.java
index 66ffa42..e79c530 100644
--- a/java/com/google/gerrit/entities/RefNames.java
+++ b/java/com/google/gerrit/entities/RefNames.java
@@ -195,7 +195,7 @@
return ref.startsWith(REFS_TAGS);
}
- /** True if the provided ref is {@link REFS_EXTERNAL_IDS}. */
+ /** True if the provided ref is {@link #REFS_EXTERNAL_IDS}. */
public static boolean isExternalIdRef(String ref) {
return REFS_EXTERNAL_IDS.equals(ref);
}
diff --git a/java/com/google/gerrit/httpd/raw/IndexHtmlUtil.java b/java/com/google/gerrit/httpd/raw/IndexHtmlUtil.java
index 72bfe40..92788b7 100644
--- a/java/com/google/gerrit/httpd/raw/IndexHtmlUtil.java
+++ b/java/com/google/gerrit/httpd/raw/IndexHtmlUtil.java
@@ -121,7 +121,7 @@
data.put("userIsAuthenticated", true);
if (page == RequestedPage.DASHBOARD) {
data.put("defaultDashboardHex", ListOption.toHex(IndexPreloadingUtil.DASHBOARD_OPTIONS));
- data.put("dashboardQuery", IndexPreloadingUtil.computeDashboardQueryList(serverApi));
+ data.put("dashboardQuery", IndexPreloadingUtil.computeDashboardQueryList());
}
} catch (AuthException e) {
logger.atFine().log("Can't inline account-related data because user is unauthenticated");
diff --git a/java/com/google/gerrit/httpd/raw/IndexPreloadingUtil.java b/java/com/google/gerrit/httpd/raw/IndexPreloadingUtil.java
index bb3b6d5..afaeaf6 100644
--- a/java/com/google/gerrit/httpd/raw/IndexPreloadingUtil.java
+++ b/java/com/google/gerrit/httpd/raw/IndexPreloadingUtil.java
@@ -22,9 +22,7 @@
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.UsedAt;
import com.google.gerrit.common.UsedAt.Project;
-import com.google.gerrit.extensions.api.config.Server;
import com.google.gerrit.extensions.client.ListChangesOption;
-import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.Url;
import java.net.URI;
import java.net.URISyntaxException;
@@ -188,7 +186,7 @@
return Optional.empty();
}
- public static List<String> computeDashboardQueryList(Server serverApi) throws RestApiException {
+ public static List<String> computeDashboardQueryList() {
List<String> queryList = new ArrayList<>();
queryList.add(SELF_DASHBOARD_HAS_UNPUBLISHED_DRAFTS_QUERY);
queryList.add(SELF_YOUR_TURN);
diff --git a/java/com/google/gerrit/pgm/Daemon.java b/java/com/google/gerrit/pgm/Daemon.java
index 845cc9a..744f91b 100644
--- a/java/com/google/gerrit/pgm/Daemon.java
+++ b/java/com/google/gerrit/pgm/Daemon.java
@@ -215,6 +215,7 @@
private Path runFile;
private boolean inMemoryTest;
private AbstractModule indexModule;
+ private Module accountPatchReviewStoreModule;
private Module emailModule;
private List<Module> testSysModules = new ArrayList<>();
private List<Module> testSshModules = new ArrayList<>();
@@ -333,6 +334,11 @@
}
@VisibleForTesting
+ public void setAccountPatchReviewStoreModuleForTesting(Module module) {
+ accountPatchReviewStoreModule = module;
+ }
+
+ @VisibleForTesting
public void setEmailModuleForTesting(Module module) {
emailModule = module;
}
@@ -442,7 +448,11 @@
modules.add(new WorkQueueModule());
modules.add(new StreamEventsApiListenerModule());
modules.add(new EventBrokerModule());
- modules.add(new JdbcAccountPatchReviewStoreModule(config));
+ if (accountPatchReviewStoreModule != null) {
+ modules.add(accountPatchReviewStoreModule);
+ } else {
+ modules.add(new JdbcAccountPatchReviewStoreModule(config));
+ }
modules.add(new SysExecutorModule());
modules.add(new DiffExecutorModule());
modules.add(new MimeUtil2Module());
diff --git a/java/com/google/gerrit/server/index/change/ChangeField.java b/java/com/google/gerrit/server/index/change/ChangeField.java
index 8e443f82..7984737 100644
--- a/java/com/google/gerrit/server/index/change/ChangeField.java
+++ b/java/com/google/gerrit/server/index/change/ChangeField.java
@@ -505,9 +505,11 @@
/** The user assigned to the change. */
// The getter always returns NO_ASSIGNEE, since assignee field is deprecated.
+ @Deprecated
public static final IndexedField<ChangeData, Integer> ASSIGNEE_FIELD =
IndexedField.<ChangeData>integerBuilder("Assignee").build(changeGetter(c -> NO_ASSIGNEE));
+ @Deprecated
public static final IndexedField<ChangeData, Integer>.SearchSpec ASSIGNEE_SPEC =
ASSIGNEE_FIELD.integer(ChangeQueryBuilder.FIELD_ASSIGNEE);
diff --git a/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java b/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java
index 6ddf7a3..82b8f18 100644
--- a/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java
+++ b/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java
@@ -240,6 +240,7 @@
.build();
/** Remove assignee field. */
+ @SuppressWarnings("deprecation")
static final Schema<ChangeData> V82 =
new Schema.Builder<ChangeData>()
.add(V81)
diff --git a/java/com/google/gerrit/server/plugins/ServerPluginInfoModule.java b/java/com/google/gerrit/server/plugins/ServerPluginInfoModule.java
index 60dff84..e91f7b7 100644
--- a/java/com/google/gerrit/server/plugins/ServerPluginInfoModule.java
+++ b/java/com/google/gerrit/server/plugins/ServerPluginInfoModule.java
@@ -72,13 +72,15 @@
if (!ready) {
synchronized (dataDir) {
if (!ready) {
- try {
- Files.createDirectories(dataDir);
- } catch (IOException e) {
- throw new ProvisionException(
- String.format(
- "Cannot create %s for plugin %s", dataDir.toAbsolutePath(), plugin.getName()),
- e);
+ if (!Files.isDirectory(dataDir)) {
+ try {
+ Files.createDirectories(dataDir);
+ } catch (IOException e) {
+ throw new ProvisionException(
+ String.format(
+ "Cannot create %s for plugin %s", dataDir.toAbsolutePath(), plugin.getName()),
+ e);
+ }
}
ready = true;
}
diff --git a/java/com/google/gerrit/server/restapi/BUILD b/java/com/google/gerrit/server/restapi/BUILD
index 62da2f2..dd0ec78d 100644
--- a/java/com/google/gerrit/server/restapi/BUILD
+++ b/java/com/google/gerrit/server/restapi/BUILD
@@ -34,6 +34,7 @@
"//lib/auto:auto-factory",
"//lib/auto:auto-value",
"//lib/auto:auto-value-annotations",
+ "//lib/commons:codec",
"//lib/commons:compress",
"//lib/commons:lang3",
"//lib/errorprone:annotations",
diff --git a/java/com/google/gerrit/server/restapi/change/ApplyPatchUtil.java b/java/com/google/gerrit/server/restapi/change/ApplyPatchUtil.java
index d4f549a..4021f77 100644
--- a/java/com/google/gerrit/server/restapi/change/ApplyPatchUtil.java
+++ b/java/com/google/gerrit/server/restapi/change/ApplyPatchUtil.java
@@ -23,6 +23,7 @@
import java.io.IOException;
import java.io.InputStream;
import java.nio.charset.StandardCharsets;
+import org.apache.commons.codec.binary.Base64;
import org.eclipse.jgit.api.errors.PatchApplyException;
import org.eclipse.jgit.api.errors.PatchFormatException;
import org.eclipse.jgit.lib.ObjectId;
@@ -51,8 +52,12 @@
throws IOException, RestApiException {
checkNotNull(mergeTip);
RevTree tip = mergeTip.getTree();
- InputStream patchStream =
- new ByteArrayInputStream(input.patch.getBytes(StandardCharsets.UTF_8));
+ InputStream patchStream;
+ if (Base64.isBase64(input.patch)) {
+ patchStream = new ByteArrayInputStream(org.eclipse.jgit.util.Base64.decode(input.patch));
+ } else {
+ patchStream = new ByteArrayInputStream(input.patch.getBytes(StandardCharsets.UTF_8));
+ }
try {
PatchApplier applier = new PatchApplier(repo, tip, oi);
PatchApplier.Result applyResult = applier.applyPatch(patchStream);
diff --git a/java/com/google/gerrit/server/update/context/RefUpdateContext.java b/java/com/google/gerrit/server/update/context/RefUpdateContext.java
index 3ac0f0d..1144e4f 100644
--- a/java/com/google/gerrit/server/update/context/RefUpdateContext.java
+++ b/java/com/google/gerrit/server/update/context/RefUpdateContext.java
@@ -22,7 +22,7 @@
import java.util.Deque;
/**
- * Passes additional information about an operation to the {@link BatchRefUpdate#execute} method.
+ * Passes additional information about an operation to the {@code BatchRefUpdate#execute} method.
*
* <p>To pass the additional information {@link RefUpdateContext}, wraps a code into an open
* RefUpdateContext, e.g.:
@@ -34,7 +34,7 @@
* }
* }</pre>
*
- * When the {@link BatchRefUpdate#execute} method is executed, it can get all opened contexts and
+ * When the {@code BatchRefUpdate#execute} method is executed, it can get all opened contexts and
* use it for an additional actions, e.g. it can put it in the reflog.
*
* <p>The information provided by this class is used internally in google.
@@ -42,7 +42,7 @@
* <p>The InMemoryRepositoryManager file makes some validation to ensure that RefUpdateContext is
* used correctly within the code (see thee validateRefUpdateContext method).
*
- * <p>The class includes only operations from open-source gerrit and can be extended (see {@link
+ * <p>The class includes only operations from open-source gerrit and can be extended (see {@code
* TestActionRefUpdateContext} for example how to extend it).
*/
public class RefUpdateContext implements AutoCloseable {
diff --git a/java/com/google/gerrit/testing/BUILD b/java/com/google/gerrit/testing/BUILD
index fb9e64e..81a6443 100644
--- a/java/com/google/gerrit/testing/BUILD
+++ b/java/com/google/gerrit/testing/BUILD
@@ -51,6 +51,7 @@
"//lib:junit",
"//lib/auto:auto-value",
"//lib/auto:auto-value-annotations",
+ "//lib/errorprone:annotations",
"//lib/flogger:api",
"//lib/guice",
"//lib/guice:guice-servlet",
diff --git a/java/com/google/gerrit/testing/FakeAccountPatchReviewStore.java b/java/com/google/gerrit/testing/FakeAccountPatchReviewStore.java
new file mode 100644
index 0000000..1533aeb
--- /dev/null
+++ b/java/com/google/gerrit/testing/FakeAccountPatchReviewStore.java
@@ -0,0 +1,141 @@
+// Copyright (C) 2023 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.testing;
+
+import com.google.auto.value.AutoValue;
+import com.google.common.collect.ImmutableSet;
+import com.google.errorprone.annotations.CanIgnoreReturnValue;
+import com.google.gerrit.entities.Account;
+import com.google.gerrit.entities.Change;
+import com.google.gerrit.entities.PatchSet;
+import com.google.gerrit.extensions.events.LifecycleListener;
+import com.google.gerrit.extensions.registration.DynamicItem;
+import com.google.gerrit.lifecycle.LifecycleModule;
+import com.google.gerrit.server.change.AccountPatchReviewStore;
+import com.google.inject.Singleton;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Optional;
+import java.util.Set;
+
+/**
+ * An implementation of the {@link AccountPatchReviewStore} that's only used in tests. This
+ * implementation stores reviewed files in memory.
+ */
+@Singleton
+public class FakeAccountPatchReviewStore implements AccountPatchReviewStore, LifecycleListener {
+
+ private final Set<Entity> store = new HashSet<>();
+
+ @Override
+ public void start() {}
+
+ @Override
+ public void stop() {}
+
+ public static class FakeAccountPatchReviewStoreModule extends LifecycleModule {
+ @Override
+ protected void configure() {
+ DynamicItem.bind(binder(), AccountPatchReviewStore.class)
+ .to(FakeAccountPatchReviewStore.class);
+ listener().to(FakeAccountPatchReviewStore.class);
+ }
+ }
+
+ @AutoValue
+ abstract static class Entity {
+ abstract PatchSet.Id psId();
+
+ abstract Account.Id accountId();
+
+ abstract String path();
+
+ static Entity create(PatchSet.Id psId, Account.Id accountId, String path) {
+ return new AutoValue_FakeAccountPatchReviewStore_Entity(psId, accountId, path);
+ }
+ }
+
+ @Override
+ @CanIgnoreReturnValue
+ public boolean markReviewed(PatchSet.Id psId, Account.Id accountId, String path) {
+ synchronized (store) {
+ Entity entity = Entity.create(psId, accountId, path);
+ return store.add(entity);
+ }
+ }
+
+ @Override
+ public void markReviewed(PatchSet.Id psId, Account.Id accountId, Collection<String> paths) {
+ paths.forEach(path -> markReviewed(psId, accountId, path));
+ }
+
+ @Override
+ public void clearReviewed(PatchSet.Id psId, Account.Id accountId, String path) {
+ synchronized (store) {
+ store.remove(Entity.create(psId, accountId, path));
+ }
+ }
+
+ @Override
+ public void clearReviewed(PatchSet.Id psId) {
+ synchronized (store) {
+ List<Entity> toRemove = new ArrayList<>();
+ for (Entity entity : store) {
+ if (entity.psId().equals(psId)) {
+ toRemove.add(entity);
+ }
+ }
+ store.removeAll(toRemove);
+ }
+ }
+
+ @Override
+ public void clearReviewed(Change.Id changeId) {
+ synchronized (store) {
+ List<Entity> toRemove = new ArrayList<>();
+ for (Entity entity : store) {
+ if (entity.psId().changeId().equals(changeId)) {
+ toRemove.add(entity);
+ }
+ }
+ store.removeAll(toRemove);
+ }
+ }
+
+ @Override
+ public Optional<PatchSetWithReviewedFiles> findReviewed(PatchSet.Id psId, Account.Id accountId) {
+ synchronized (store) {
+ int matchedPsNumber = -1;
+ Optional<PatchSetWithReviewedFiles> result = Optional.empty();
+ for (Entity entity : store) {
+ if (entity.accountId() != accountId || !entity.psId().changeId().equals(psId.changeId())) {
+ continue;
+ }
+ int entityPsNumber = Integer.parseInt(entity.psId().getId());
+ if (entityPsNumber <= psId.get() && entityPsNumber > matchedPsNumber) {
+ matchedPsNumber = entityPsNumber;
+ result =
+ Optional.of(
+ PatchSetWithReviewedFiles.create(
+ PatchSet.id(psId.changeId(), matchedPsNumber),
+ ImmutableSet.of(entity.path())));
+ }
+ }
+ return result;
+ }
+ }
+}
diff --git a/java/com/google/gerrit/testing/InMemoryRepositoryManager.java b/java/com/google/gerrit/testing/InMemoryRepositoryManager.java
index 650c218..8d1130c 100644
--- a/java/com/google/gerrit/testing/InMemoryRepositoryManager.java
+++ b/java/com/google/gerrit/testing/InMemoryRepositoryManager.java
@@ -171,7 +171,7 @@
private RefUpdateContextValidator addSpecialRef(
Predicate<String> refNamePredicate, RefUpdateType... validRefUpdateTypes) {
specialRefs.add(
- new SimpleImmutableEntry<Predicate<String>, ImmutableList<RefUpdateType>>(
+ new SimpleImmutableEntry<>(
refNamePredicate, ImmutableList.copyOf(validRefUpdateTypes)));
return this;
}
diff --git a/javatests/com/google/gerrit/acceptance/TestMetricMakerTest.java b/javatests/com/google/gerrit/acceptance/TestMetricMakerTest.java
new file mode 100644
index 0000000..3464d21
--- /dev/null
+++ b/javatests/com/google/gerrit/acceptance/TestMetricMakerTest.java
@@ -0,0 +1,202 @@
+// Copyright (C) 2023 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 static com.google.common.truth.Truth.assertThat;
+
+import com.google.gerrit.metrics.Counter0;
+import com.google.gerrit.metrics.Counter1;
+import com.google.gerrit.metrics.Counter2;
+import com.google.gerrit.metrics.Counter3;
+import com.google.gerrit.metrics.Description;
+import com.google.gerrit.metrics.Field;
+import org.junit.Before;
+import org.junit.Test;
+
+/** Tests for {@link TestMetricMaker}. */
+public class TestMetricMakerTest {
+ private TestMetricMaker testMetricMaker = new TestMetricMaker();
+
+ @Before
+ public void setUp() {
+ testMetricMaker.reset();
+ }
+
+ @Test
+ public void counter0() throws Exception {
+ String counterName = "test_counter";
+ Counter0 counter = testMetricMaker.newCounter(counterName, new Description("Test Counter"));
+ assertThat(testMetricMaker.getCount(counterName)).isEqualTo(0);
+
+ counter.increment();
+ assertThat(testMetricMaker.getCount(counterName)).isEqualTo(1);
+
+ counter.incrementBy(/* value= */ 3);
+ assertThat(testMetricMaker.getCount(counterName)).isEqualTo(4);
+ }
+
+ @Test
+ public void counter1_booleanField() throws Exception {
+ String counterName = "test_counter";
+ Counter1<Boolean> counter =
+ testMetricMaker.newCounter(
+ counterName,
+ new Description("Test Counter"),
+ Field.ofBoolean("boolean_field", (metadataBuilder, booleanField) -> {}).build());
+ assertThat(testMetricMaker.getCount(counterName, true)).isEqualTo(0);
+ assertThat(testMetricMaker.getCount(counterName, false)).isEqualTo(0);
+
+ counter.increment(/* field1= */ true);
+ assertThat(testMetricMaker.getCount(counterName, true)).isEqualTo(1);
+ assertThat(testMetricMaker.getCount(counterName, false)).isEqualTo(0);
+
+ counter.incrementBy(/* field1= */ true, /* value= */ 3);
+ assertThat(testMetricMaker.getCount(counterName, true)).isEqualTo(4);
+ assertThat(testMetricMaker.getCount(counterName, false)).isEqualTo(0);
+
+ counter.increment(/* field1= */ false);
+ assertThat(testMetricMaker.getCount(counterName, true)).isEqualTo(4);
+ assertThat(testMetricMaker.getCount(counterName, false)).isEqualTo(1);
+
+ counter.incrementBy(/* field1= */ false, /* value= */ 4);
+ assertThat(testMetricMaker.getCount(counterName, true)).isEqualTo(4);
+ assertThat(testMetricMaker.getCount(counterName, false)).isEqualTo(5);
+
+ assertThat(testMetricMaker.getCount(counterName)).isEqualTo(0);
+ }
+
+ @Test
+ public void counter1_stringField() throws Exception {
+ String counterName = "test_counter";
+ Counter1<String> counter =
+ testMetricMaker.newCounter(
+ counterName,
+ new Description("Test Counter"),
+ Field.ofString("string_field", (metadataBuilder, stringField) -> {}).build());
+ assertThat(testMetricMaker.getCount(counterName, "foo")).isEqualTo(0);
+ assertThat(testMetricMaker.getCount(counterName, "bar")).isEqualTo(0);
+
+ counter.increment(/* field1= */ "foo");
+ assertThat(testMetricMaker.getCount(counterName, "foo")).isEqualTo(1);
+ assertThat(testMetricMaker.getCount(counterName, "bar")).isEqualTo(0);
+
+ counter.incrementBy(/* field1= */ "foo", /* value= */ 3);
+ assertThat(testMetricMaker.getCount(counterName, "foo")).isEqualTo(4);
+ assertThat(testMetricMaker.getCount(counterName, "bar")).isEqualTo(0);
+
+ counter.increment(/* field1= */ "bar");
+ assertThat(testMetricMaker.getCount(counterName, "foo")).isEqualTo(4);
+ assertThat(testMetricMaker.getCount(counterName, "bar")).isEqualTo(1);
+
+ counter.incrementBy(/* field1= */ "bar", /* value= */ 4);
+ assertThat(testMetricMaker.getCount(counterName, "foo")).isEqualTo(4);
+ assertThat(testMetricMaker.getCount(counterName, "bar")).isEqualTo(5);
+
+ assertThat(testMetricMaker.getCount(counterName)).isEqualTo(0);
+ }
+
+ @Test
+ public void counter2() throws Exception {
+ String counterName = "test_counter";
+ Counter2<Boolean, String> counter =
+ testMetricMaker.newCounter(
+ counterName,
+ new Description("Test Counter"),
+ Field.ofBoolean("boolean_field", (metadataBuilder, booleanField) -> {}).build(),
+ Field.ofString("string_field", (metadataBuilder, stringField) -> {}).build());
+ assertThat(testMetricMaker.getCount(counterName, true, "foo")).isEqualTo(0);
+ assertThat(testMetricMaker.getCount(counterName, false, "foo")).isEqualTo(0);
+
+ counter.increment(/* field1= */ true, /* field2= */ "foo");
+ assertThat(testMetricMaker.getCount(counterName, true, "foo")).isEqualTo(1);
+ assertThat(testMetricMaker.getCount(counterName, false, "foo")).isEqualTo(0);
+
+ counter.incrementBy(/* field1= */ true, /* field2= */ "foo", /* value= */ 3);
+ assertThat(testMetricMaker.getCount(counterName, true, "foo")).isEqualTo(4);
+ assertThat(testMetricMaker.getCount(counterName, false, "foo")).isEqualTo(0);
+
+ counter.increment(/* field1= */ false, /* field2= */ "foo");
+ assertThat(testMetricMaker.getCount(counterName, true, "foo")).isEqualTo(4);
+ assertThat(testMetricMaker.getCount(counterName, false, "foo")).isEqualTo(1);
+
+ counter.incrementBy(/* field1= */ false, /* field2= */ "foo", /* value= */ 4);
+ assertThat(testMetricMaker.getCount(counterName, true, "foo")).isEqualTo(4);
+ assertThat(testMetricMaker.getCount(counterName, false, "foo")).isEqualTo(5);
+
+ counter.increment(/* field1= */ true, /* field2= */ "bar");
+ assertThat(testMetricMaker.getCount(counterName, true, "foo")).isEqualTo(4);
+ assertThat(testMetricMaker.getCount(counterName, true, "bar")).isEqualTo(1);
+
+ counter.incrementBy(/* field1= */ true, /* field2= */ "bar", /* value= */ 5);
+ assertThat(testMetricMaker.getCount(counterName, true, "foo")).isEqualTo(4);
+ assertThat(testMetricMaker.getCount(counterName, true, "bar")).isEqualTo(6);
+
+ assertThat(testMetricMaker.getCount(counterName)).isEqualTo(0);
+ assertThat(testMetricMaker.getCount(counterName, true)).isEqualTo(0);
+ assertThat(testMetricMaker.getCount(counterName, false)).isEqualTo(0);
+ }
+
+ @Test
+ public void counter3() throws Exception {
+ String counterName = "test_counter";
+ Counter3<Boolean, String, Integer> counter =
+ testMetricMaker.newCounter(
+ counterName,
+ new Description("Test Counter"),
+ Field.ofBoolean("boolean_field", (metadataBuilder, booleanField) -> {}).build(),
+ Field.ofString("string_field", (metadataBuilder, stringField) -> {}).build(),
+ Field.ofInteger("integer_field", (metadataBuilder, stringField) -> {}).build());
+ assertThat(testMetricMaker.getCount(counterName, true, "foo", 0)).isEqualTo(0);
+ assertThat(testMetricMaker.getCount(counterName, false, "foo", 0)).isEqualTo(0);
+
+ counter.increment(/* field1= */ true, /* field2= */ "foo", /* field3= */ 0);
+ assertThat(testMetricMaker.getCount(counterName, true, "foo", 0)).isEqualTo(1);
+ assertThat(testMetricMaker.getCount(counterName, false, "foo", 0)).isEqualTo(0);
+
+ counter.incrementBy(/* field1= */ true, /* field2= */ "foo", /* field3= */ 0, /* value= */ 3);
+ assertThat(testMetricMaker.getCount(counterName, true, "foo", 0)).isEqualTo(4);
+ assertThat(testMetricMaker.getCount(counterName, false, "foo", 0)).isEqualTo(0);
+
+ counter.increment(/* field1= */ false, /* field2= */ "foo", /* field3= */ 0);
+ assertThat(testMetricMaker.getCount(counterName, true, "foo", 0)).isEqualTo(4);
+ assertThat(testMetricMaker.getCount(counterName, false, "foo", 0)).isEqualTo(1);
+
+ counter.incrementBy(/* field1= */ false, /* field2= */ "foo", /* field3= */ 0, /* value= */ 4);
+ assertThat(testMetricMaker.getCount(counterName, true, "foo", 0)).isEqualTo(4);
+ assertThat(testMetricMaker.getCount(counterName, false, "foo", 0)).isEqualTo(5);
+
+ counter.increment(/* field1= */ true, /* field2= */ "bar", /* field3= */ 0);
+ assertThat(testMetricMaker.getCount(counterName, true, "foo", 0)).isEqualTo(4);
+ assertThat(testMetricMaker.getCount(counterName, true, "bar", 0)).isEqualTo(1);
+
+ counter.incrementBy(/* field1= */ true, /* field2= */ "bar", /* field3= */ 0, /* value= */ 5);
+ assertThat(testMetricMaker.getCount(counterName, true, "foo", 0)).isEqualTo(4);
+ assertThat(testMetricMaker.getCount(counterName, true, "bar", 0)).isEqualTo(6);
+
+ counter.increment(/* field1= */ false, /* field2= */ "foo", /* field3= */ 1);
+ assertThat(testMetricMaker.getCount(counterName, true, "foo", 0)).isEqualTo(4);
+ assertThat(testMetricMaker.getCount(counterName, false, "foo", 1)).isEqualTo(1);
+
+ counter.incrementBy(/* field1= */ false, /* field2= */ "foo", /* field3= */ 1, /* value= */ 6);
+ assertThat(testMetricMaker.getCount(counterName, true, "foo", 0)).isEqualTo(4);
+ assertThat(testMetricMaker.getCount(counterName, false, "foo", 1)).isEqualTo(7);
+
+ assertThat(testMetricMaker.getCount(counterName)).isEqualTo(0);
+ assertThat(testMetricMaker.getCount(counterName, true)).isEqualTo(0);
+ assertThat(testMetricMaker.getCount(counterName, false)).isEqualTo(0);
+ assertThat(testMetricMaker.getCount(counterName, true, "foo")).isEqualTo(0);
+ assertThat(testMetricMaker.getCount(counterName, false, "foo")).isEqualTo(0);
+ }
+}
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ApplyPatchIT.java b/javatests/com/google/gerrit/acceptance/api/change/ApplyPatchIT.java
index 898e1ff..0b55563 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ApplyPatchIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ApplyPatchIT.java
@@ -213,6 +213,29 @@
}
@Test
+ public void applyGerritBasedPatchUsingRestWithEncodedPatch_success() throws Exception {
+ String head = getHead(repo(), HEAD).name();
+ createBranchWithRevision(BranchNameKey.create(project, "branch"), head);
+ PushOneCommit.Result baseCommit = createChange("Add file", ADDED_FILE_NAME, ADDED_FILE_CONTENT);
+ baseCommit.assertOkStatus();
+ createBranchWithRevision(BranchNameKey.create(project, DESTINATION_BRANCH), head);
+ RestResponse patchResp =
+ userRestSession.get("/changes/" + baseCommit.getChangeId() + "/revisions/current/patch");
+ patchResp.assertOK();
+ String originalEncodedPatch = patchResp.getEntityContent();
+ String originalDecodedPatch = new String(Base64.decode(patchResp.getEntityContent()), UTF_8);
+ ApplyPatchPatchSetInput in = buildInput(originalEncodedPatch);
+ PushOneCommit.Result destChange = createChange();
+
+ RestResponse resp =
+ adminRestSession.post("/changes/" + destChange.getChangeId() + "/patch:apply", in);
+
+ resp.assertOK();
+ BinaryResult resultPatch = gApi.changes().id(destChange.getChangeId()).current().patch();
+ assertThat(removeHeader(resultPatch)).isEqualTo(removeHeader(originalDecodedPatch));
+ }
+
+ @Test
public void applyPatchWithConflict_fails() throws Exception {
initBaseWithFile(MODIFIED_FILE_NAME, "Unexpected base content");
ApplyPatchPatchSetInput in = buildInput(MODIFIED_FILE_DIFF);
@@ -404,6 +427,6 @@
}
private String removeHeader(String s) {
- return s.substring(s.indexOf("\ndiff --git"), s.length() - 1);
+ return s.substring(s.lastIndexOf("\ndiff --git"), s.length() - 1);
}
}
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 721d650..bb27237 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
@@ -1244,7 +1244,6 @@
<div class="changeStatuses">
${this.changeStatuses.map(
status => html` <gr-change-status
- .change=${this.change}
.revertedChange=${this.revertedChange}
.status=${status}
.resolveWeblinks=${resolveWeblinks}
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-rebase-dialog/gr-confirm-rebase-dialog.ts b/polygerrit-ui/app/elements/change/gr-confirm-rebase-dialog/gr-confirm-rebase-dialog.ts
index c2739f3..ad2ba8f 100644
--- a/polygerrit-ui/app/elements/change/gr-confirm-rebase-dialog/gr-confirm-rebase-dialog.ts
+++ b/polygerrit-ui/app/elements/change/gr-confirm-rebase-dialog/gr-confirm-rebase-dialog.ts
@@ -22,7 +22,6 @@
import {sharedStyles} from '../../../styles/shared-styles';
import {ValueChangedEvent} from '../../../types/events';
import {throwingErrorCallback} from '../../shared/gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper';
-import {KnownExperimentId} from '../../../services/flags/flags';
export interface RebaseChange {
name: string;
@@ -99,8 +98,6 @@
private readonly restApiService = getAppContext().restApiService;
- private readonly flagsService = getAppContext().flagsService;
-
constructor() {
super();
this.query = input => this.getChangeSuggestions(input);
@@ -234,8 +231,7 @@
>
</div>
${when(
- this.flagsService.isEnabled(KnownExperimentId.REBASE_CHAIN) &&
- this.hasParent,
+ this.hasParent,
() =>
html`<div>
<input
diff --git a/polygerrit-ui/app/elements/change/gr-label-scores/gr-label-scores.ts b/polygerrit-ui/app/elements/change/gr-label-scores/gr-label-scores.ts
index 873e6ce..3a83fa1 100644
--- a/polygerrit-ui/app/elements/change/gr-label-scores/gr-label-scores.ts
+++ b/polygerrit-ui/app/elements/change/gr-label-scores/gr-label-scores.ts
@@ -5,7 +5,7 @@
*/
import '../gr-label-score-row/gr-label-score-row';
import '../../../styles/shared-styles';
-import {LitElement, css, html} from 'lit';
+import {LitElement, css, html, nothing} from 'lit';
import {customElement, property} from 'lit/decorators.js';
import {
ChangeInfo,
@@ -107,8 +107,7 @@
label => !this.permittedLabels || this.permittedLabels[label.name]
).length === 0
) {
- return html`<h3 class="heading-4">Trigger Votes</h3>
- <div class="permissionMessage">You don't have permission to vote</div>`;
+ return nothing;
}
return html`<h3 class="heading-4">Trigger Votes</h3>
${this.renderLabels(labels)}`;
diff --git a/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar.ts b/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar.ts
index fbc87ed..314e126 100644
--- a/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar.ts
+++ b/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar.ts
@@ -278,7 +278,7 @@
// fallback to gerrit's official doc
let baseUrl =
this.docsBaseUrl ||
- 'https://gerrit-review.googlesource.com/documentation/';
+ 'https://gerrit-review.googlesource.com/Documentation/';
if (baseUrl.endsWith('/')) {
baseUrl = baseUrl.substring(0, baseUrl.length - 1);
}
diff --git a/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar_test.ts b/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar_test.ts
index 43b1f86..0694453 100644
--- a/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar_test.ts
+++ b/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar_test.ts
@@ -319,7 +319,7 @@
await element.updateComplete;
assert.equal(
element.computeHelpDocLink(),
- 'https://gerrit-review.googlesource.com/documentation/' +
+ 'https://gerrit-review.googlesource.com/Documentation/' +
'user-search.html'
);
});
diff --git a/polygerrit-ui/app/elements/gr-app-element.ts b/polygerrit-ui/app/elements/gr-app-element.ts
index b096f76..e5991f6 100644
--- a/polygerrit-ui/app/elements/gr-app-element.ts
+++ b/polygerrit-ui/app/elements/gr-app-element.ts
@@ -500,11 +500,12 @@
}
private renderEditorView() {
- // The `cache()` is required for re-using the editor view when switching
- // back and forth between change, diff and editor views.
- return cache(
- this.isEditorView() ? html`<gr-editor-view></gr-editor-view>` : nothing
- );
+ // For some reason caching the editor view caused an issue (b/269308770).
+ // We did not bother to root cause that issue, but instead let's forgo
+ // caching of the editor view. It does not help much anyway.
+ return this.isEditorView()
+ ? html`<gr-editor-view></gr-editor-view>`
+ : nothing;
}
private isEditorView() {
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 91e601c..661ffa0 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
@@ -35,6 +35,16 @@
selected: HTMLElement | null;
}
+export enum AutocompleteQueryStatusType {
+ LOADING = 'loading',
+ ERROR = 'error',
+}
+
+export interface AutocompleteQueryStatus {
+ type: AutocompleteQueryStatusType;
+ message: string;
+}
+
@customElement('gr-autocomplete-dropdown')
export class GrAutocompleteDropdown extends LitElement {
/**
@@ -58,8 +68,8 @@
/** If specified a single non-interactable line is shown instead of
* suggestions.
*/
- @property({type: String})
- errorMessage?: String;
+ @property({type: Object})
+ queryStatus?: AutocompleteQueryStatus;
@property({type: Number})
verticalOffset = 0;
@@ -117,10 +127,12 @@
li.selected {
background-color: var(--hover-background-color);
}
- li.query-error {
+ li.query-status {
background-color: var(--disabled-background);
- color: var(--error-foreground);
cursor: default;
+ }
+ li.query-status.error {
+ color: var(--error-foreground);
white-space: pre-wrap;
}
@media only screen and (max-height: 35em) {
@@ -140,7 +152,7 @@
}
private isSuggestionListInteractible() {
- return !this.isHidden && !this.errorMessage;
+ return !this.isHidden && !this.queryStatus;
}
constructor() {
@@ -172,7 +184,8 @@
override updated(changedProperties: PropertyValues) {
if (
changedProperties.has('suggestions') ||
- changedProperties.has('isHidden')
+ changedProperties.has('isHidden') ||
+ changedProperties.has('queryStatus')
) {
if (!this.isHidden) {
this.computeCursorStopsAndRefit();
@@ -180,15 +193,19 @@
}
}
- private renderError() {
+ private renderStatus() {
return html`
<li
tabindex="-1"
- aria-label="autocomplete query error"
- class="query-error"
+ aria-label="autocomplete query status"
+ class="query-status ${this.queryStatus?.type}"
>
- <span>${this.errorMessage}</span>
- <span class="label">ERROR</span>
+ <span>${this.queryStatus?.message}</span>
+ <span class="label"
+ >${this.queryStatus?.type === AutocompleteQueryStatusType.ERROR
+ ? 'ERROR'
+ : ''}</span
+ >
</li>
`;
}
@@ -198,8 +215,8 @@
<div class="dropdown-content" id="suggestions" role="listbox">
<ul>
${when(
- this.errorMessage,
- () => this.renderError(),
+ this.queryStatus,
+ () => this.renderStatus(),
() => html`
${repeat(
this.suggestions,
@@ -236,7 +253,7 @@
}
getCurrentText() {
- if (!this.errorMessage) {
+ if (!this.queryStatus) {
return this.getCursorTarget()?.dataset['value'] || '';
}
return '';
diff --git a/polygerrit-ui/app/elements/shared/gr-autocomplete-dropdown/gr-autocomplete-dropdown_test.ts b/polygerrit-ui/app/elements/shared/gr-autocomplete-dropdown/gr-autocomplete-dropdown_test.ts
index 54d054b..10ba5d0 100644
--- a/polygerrit-ui/app/elements/shared/gr-autocomplete-dropdown/gr-autocomplete-dropdown_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-autocomplete-dropdown/gr-autocomplete-dropdown_test.ts
@@ -5,7 +5,10 @@
*/
import '../../../test/common-test-setup';
import './gr-autocomplete-dropdown';
-import {GrAutocompleteDropdown} from './gr-autocomplete-dropdown';
+import {
+ AutocompleteQueryStatusType,
+ GrAutocompleteDropdown,
+} from './gr-autocomplete-dropdown';
import {
pressKey,
queryAll,
@@ -177,7 +180,7 @@
});
});
- suite('error tests', () => {
+ suite('status tests', () => {
let element: GrAutocompleteDropdown;
setup(async () => {
@@ -185,7 +188,10 @@
html`<gr-autocomplete-dropdown></gr-autocomplete-dropdown>`
);
element.open();
- element.errorMessage = 'Failed query error';
+ element.queryStatus = {
+ type: AutocompleteQueryStatusType.ERROR,
+ message: 'Failed query error',
+ };
await waitEventLoop();
});
@@ -200,8 +206,8 @@
<div class="dropdown-content" id="suggestions" role="listbox">
<ul>
<li
- aria-label="autocomplete query error"
- class="query-error"
+ aria-label="autocomplete query status"
+ class="query-status error"
tabindex="-1"
>
<span>Failed query error</span>
@@ -213,6 +219,31 @@
);
});
+ test('renders loading', async () => {
+ element.queryStatus = {
+ type: AutocompleteQueryStatusType.LOADING,
+ message: 'Loading...',
+ };
+ await waitEventLoop();
+ assert.shadowDom.equal(
+ element,
+ /* HTML */ `
+ <div class="dropdown-content" id="suggestions" role="listbox">
+ <ul>
+ <li
+ aria-label="autocomplete query status"
+ class="query-status loading"
+ tabindex="-1"
+ >
+ <span>Loading...</span>
+ <span class="label"></span>
+ </li>
+ </ul>
+ </div>
+ `
+ );
+ });
+
test('escape key close dropdown with error', async () => {
const closeSpy = sinon.spy(element, 'close');
pressKey(element, Key.ESC);
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 ab74e8b..db0d236 100644
--- a/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.ts
+++ b/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.ts
@@ -7,7 +7,11 @@
import '../gr-autocomplete-dropdown/gr-autocomplete-dropdown';
import '../gr-cursor-manager/gr-cursor-manager';
import '../../../styles/shared-styles';
-import {GrAutocompleteDropdown} from '../gr-autocomplete-dropdown/gr-autocomplete-dropdown';
+import {
+ AutocompleteQueryStatus,
+ AutocompleteQueryStatusType,
+ GrAutocompleteDropdown,
+} from '../gr-autocomplete-dropdown/gr-autocomplete-dropdown';
import {fire, fireEvent} from '../../../utils/event-util';
import {
debounce,
@@ -166,7 +170,7 @@
@state() suggestions: AutocompleteSuggestion[] = [];
- @state() queryErrorMessage?: string;
+ @state() queryStatus?: AutocompleteQueryStatus;
@state() index: number | null = null;
@@ -179,8 +183,23 @@
@state() selected: HTMLElement | null = null;
+ /**
+ * The query id that status or suggestions correspond to.
+ */
+ private activeQueryId = 0;
+
+ /**
+ * Last scheduled update suggestions task.
+ */
private updateSuggestionsTask?: DelayedTask;
+ // Generate ids for scheduled suggestion queries to easily distinguish them.
+ private static NEXT_QUERY_ID = 1;
+
+ private static getNextQueryId() {
+ return GrAutocomplete.NEXT_QUERY_ID++;
+ }
+
/**
* @return Promise that resolves when suggestions are update.
*/
@@ -266,7 +285,7 @@
}
if (
changedProperties.has('suggestions') ||
- changedProperties.has('queryErrorMessage')
+ changedProperties.has('queryStatus')
) {
this.updateDropdownVisibility();
}
@@ -310,7 +329,7 @@
@item-selected=${this.handleItemSelect}
@dropdown-closed=${this.focusWithoutDisplayingSuggestions}
.suggestions=${this.suggestions}
- .errorMessage=${this.queryErrorMessage}
+ .queryStatus=${this.queryStatus}
role="listbox"
.index=${this.index}
>
@@ -412,8 +431,7 @@
// Reset suggestions for every update
// This will also prevent from carrying over suggestions:
// @see Issue 12039
- this.suggestions = [];
- this.queryErrorMessage = undefined;
+ this.resetQueryOutput();
// TODO(taoalpha): Also skip if text has not changed
@@ -421,8 +439,7 @@
return;
}
- const query = this.query;
- if (!query) {
+ if (!this.query) {
return;
}
@@ -435,49 +452,69 @@
return;
}
- const update = () => {
- query(this.text)
- .then(suggestions => {
- if (this.text !== this.text) {
- // Late response.
- return;
- }
- for (const suggestion of suggestions) {
- suggestion.text = suggestion?.name ?? '';
- }
- this.suggestions = suggestions;
- if (this.index === -1) {
- this.value = '';
- }
- })
- .catch(e => {
- this.value = '';
- if (typeof e === 'string') {
- this.queryErrorMessage = e;
- } else if (e instanceof Error) {
- this.queryErrorMessage = e.message;
- }
- });
- };
-
+ const queryId = GrAutocomplete.getNextQueryId();
+ this.activeQueryId = queryId;
+ this.setQueryStatus({
+ type: AutocompleteQueryStatusType.LOADING,
+ message: 'Loading...',
+ });
this.updateSuggestionsTask = debounce(
this.updateSuggestionsTask,
- update,
+ this.createUpdateTask(queryId, this.query, this.text),
DEBOUNCE_WAIT_MS
);
}
+ private createUpdateTask(
+ queryId: number,
+ query: AutocompleteQuery,
+ text: string
+ ): () => Promise<void> {
+ return async () => {
+ let suggestions: AutocompleteSuggestion[];
+ try {
+ suggestions = await query(text);
+ } catch (e) {
+ this.value = '';
+ if (typeof e === 'string') {
+ this.setQueryStatus({
+ type: AutocompleteQueryStatusType.ERROR,
+ message: e,
+ });
+ } else if (e instanceof Error) {
+ this.setQueryStatus({
+ type: AutocompleteQueryStatusType.ERROR,
+ message: e.message,
+ });
+ }
+ return;
+ }
+ if (queryId !== this.activeQueryId) {
+ // Late response.
+ return;
+ }
+ for (const suggestion of suggestions) {
+ suggestion.text = suggestion?.name ?? '';
+ }
+ this.setSuggestions(suggestions);
+ if (this.index === -1) {
+ this.value = '';
+ }
+ };
+ }
+
setFocus(focused: boolean) {
if (focused === this.focused) return;
this.focused = focused;
this.updateDropdownVisibility();
}
+ private shouldShowDropdown() {
+ return (this.suggestions.length > 0 || this.queryStatus) && this.focused;
+ }
+
updateDropdownVisibility() {
- if (
- (this.suggestions.length > 0 || this.queryErrorMessage) &&
- this.focused
- ) {
+ if (this.shouldShowDropdown()) {
this.suggestionsDropdown?.open();
return;
}
@@ -510,10 +547,26 @@
this.cancel();
break;
case 'Tab':
- if (this.suggestions.length > 0 && this.tabComplete) {
+ if (
+ this.queryStatus?.type === AutocompleteQueryStatusType.LOADING &&
+ this.tabComplete
+ ) {
e.preventDefault();
+ // Queue tab on load.
+ this.queryStatus = {
+ type: AutocompleteQueryStatusType.LOADING,
+ message: 'Loading... (Handle Tab on load)',
+ };
+ const queryId = this.activeQueryId;
+ this.latestSuggestionUpdateComplete?.then(() => {
+ if (queryId === this.activeQueryId) {
+ this.handleInputCommit(/* _tabComplete=*/ true);
+ }
+ });
+ } else if (this.suggestions.length > 0 && this.tabComplete) {
+ e.preventDefault();
+ this.handleInputCommit(/* _tabComplete=*/ true);
this.focus();
- this.handleInputCommit(true);
} else {
this.setFocus(false);
}
@@ -522,12 +575,24 @@
if (modifierPressed(e)) {
break;
}
- if (this.suggestions.length > 0) {
+ e.preventDefault();
+ if (this.queryStatus?.type === AutocompleteQueryStatusType.LOADING) {
+ // Queue enter on load.
+ this.queryStatus = {
+ type: AutocompleteQueryStatusType.LOADING,
+ message: 'Loading... (Handle Enter on load)',
+ };
+ const queryId = this.activeQueryId;
+ this.latestSuggestionUpdateComplete?.then(() => {
+ if (queryId === this.activeQueryId) {
+ this.handleItemSelectEnter(e);
+ }
+ });
+ } else if (this.suggestions.length > 0) {
// If suggestions are shown, act as if the keypress is in dropdown.
// suggestions length is 0 if error is shown.
this.handleItemSelectEnter(e);
} else {
- e.preventDefault();
this.handleInputCommit();
}
break;
@@ -540,7 +605,8 @@
// been based on a previous input. Clear them. This prevents an
// outdated suggestion from being used if the input keystroke is
// immediately followed by a commit keystroke. @see Issue 8655
- this.suggestions = [];
+ this.resetQueryOutput();
+ this.activeQueryId = 0;
}
this.dispatchEvent(
new CustomEvent('input-keydown', {
@@ -552,9 +618,11 @@
}
cancel() {
- if (this.suggestions.length || this.queryErrorMessage) {
- this.suggestions = [];
- this.queryErrorMessage = undefined;
+ if (this.shouldShowDropdown()) {
+ this.resetQueryOutput();
+ // If query is in flight by setting id to 0 we indicate that the results
+ // are outdated.
+ this.activeQueryId = 0;
this.requestUpdate();
} else {
fireEvent(this, 'cancel');
@@ -565,7 +633,7 @@
// Nothing to do if no suggestions.
if (
!this.allowNonSuggestedValues &&
- (this.suggestionsDropdown?.isHidden || this.queryErrorMessage)
+ (this.suggestionsDropdown?.isHidden || this.suggestions.length === 0)
) {
return;
}
@@ -607,6 +675,7 @@
}
}
this.setFocus(false);
+ this.activeQueryId = 0;
};
/**
@@ -643,8 +712,7 @@
}
}
- this.suggestions = [];
- this.queryErrorMessage = undefined;
+ this.resetQueryOutput();
// we need willUpdate to send text-changed event before we can send the
// 'commit' event
await this.updateComplete;
@@ -658,6 +726,23 @@
);
}
}
+
+ // resetQueryOutput, setSuggestions and setQueryStatus insure that suggestions
+ // and queryStatus are never set at the same time.
+ private resetQueryOutput() {
+ this.suggestions = [];
+ this.queryStatus = undefined;
+ }
+
+ private setSuggestions(suggestions: AutocompleteSuggestion[]) {
+ this.suggestions = suggestions;
+ this.queryStatus = undefined;
+ }
+
+ private setQueryStatus(queryStatus: AutocompleteQueryStatus) {
+ this.suggestions = [];
+ this.queryStatus = queryStatus;
+ }
}
/**
diff --git a/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete_test.ts b/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete_test.ts
index 81949c7..c59b8e8 100644
--- a/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete_test.ts
@@ -8,11 +8,15 @@
import {AutocompleteSuggestion, GrAutocomplete} from './gr-autocomplete';
import {
assertFails,
+ mockPromise,
pressKey,
queryAndAssert,
waitUntil,
} from '../../../test/test-utils';
-import {GrAutocompleteDropdown} from '../gr-autocomplete-dropdown/gr-autocomplete-dropdown';
+import {
+ AutocompleteQueryStatusType,
+ GrAutocompleteDropdown,
+} from '../gr-autocomplete-dropdown/gr-autocomplete-dropdown';
import {PaperInputElement} from '@polymer/paper-input/paper-input';
import {fixture, html, assert} from '@open-wc/testing';
import {Key, Modifier} from '../../../utils/dom-util';
@@ -30,9 +34,7 @@
const inputEl = () => queryAndAssert<HTMLInputElement>(element, '#input');
setup(async () => {
- element = await fixture(
- html`<gr-autocomplete no-debounce></gr-autocomplete>`
- );
+ element = await fixture(html`<gr-autocomplete></gr-autocomplete>`);
});
test('renders', () => {
@@ -151,7 +153,10 @@
],
}
);
- assert.equal(element.suggestionsDropdown?.errorMessage, 'blah not allowed');
+ assert.equal(
+ element.suggestionsDropdown?.queryStatus?.message,
+ 'blah not allowed'
+ );
});
test('cursor starts on suggestions', async () => {
@@ -240,17 +245,21 @@
await element.updateComplete;
return assertFails(promise).then(async () => {
+ await element.latestSuggestionUpdateComplete;
await waitUntil(() => !suggestionsEl().isHidden);
const cancelHandler = sinon.spy();
element.addEventListener('cancel', cancelHandler);
- assert.equal(element.queryErrorMessage, 'Test error');
+ assert.deepEqual(element.queryStatus, {
+ type: AutocompleteQueryStatusType.ERROR,
+ message: 'Test error',
+ });
pressKey(inputEl(), Key.ESC);
await waitUntil(() => suggestionsEl().isHidden);
assert.isFalse(cancelHandler.called);
- assert.isUndefined(element.queryErrorMessage);
+ assert.isUndefined(element.queryStatus);
pressKey(inputEl(), Key.ESC);
await element.updateComplete;
@@ -260,16 +269,14 @@
});
test('emits commit and handles cursor movement', async () => {
- let promise: Promise<AutocompleteSuggestion[]> = Promise.resolve([]);
- const queryStub = sinon.spy(
- (input: string) =>
- (promise = Promise.resolve([
- {name: input + ' 0', value: '0'},
- {name: input + ' 1', value: '1'},
- {name: input + ' 2', value: '2'},
- {name: input + ' 3', value: '3'},
- {name: input + ' 4', value: '4'},
- ] as AutocompleteSuggestion[]))
+ const queryStub = sinon.spy((input: string) =>
+ Promise.resolve([
+ {name: input + ' 0', value: '0'},
+ {name: input + ' 1', value: '1'},
+ {name: input + ' 2', value: '2'},
+ {name: input + ' 3', value: '3'},
+ {name: input + ' 4', value: '4'},
+ ] as AutocompleteSuggestion[])
);
element.query = queryStub;
await element.updateComplete;
@@ -280,7 +287,7 @@
element.text = 'blah';
await element.updateComplete;
- return promise.then(async () => {
+ return element.latestSuggestionUpdateComplete!.then(async () => {
await waitUntil(() => !suggestionsEl().isHidden);
const commitHandler = sinon.spy();
@@ -456,24 +463,22 @@
});
test('suggestions should not carry over', async () => {
- let promise: Promise<AutocompleteSuggestion[]> = Promise.resolve([]);
const queryStub = sinon
.stub()
- .returns(
- (promise = Promise.resolve([
- {name: 'suggestion', value: '0'},
- ] as AutocompleteSuggestion[]))
- );
+ .resolves([{name: 'suggestion', value: '0'}] as AutocompleteSuggestion[]);
element.query = queryStub;
focusOnInput();
element.text = 'bla';
await element.updateComplete;
- return promise.then(async () => {
+ return element.latestSuggestionUpdateComplete!.then(async () => {
await waitUntil(() => element.suggestions.length > 0);
assert.equal(element.suggestions.length, 1);
+
+ queryStub.resolves([] as AutocompleteSuggestion[]);
element.text = '';
element.threshold = 0;
await element.updateComplete;
+ await element.latestSuggestionUpdateComplete;
assert.equal(element.suggestions.length, 0);
});
});
@@ -488,11 +493,15 @@
element.text = 'bla';
await element.updateComplete;
return assertFails(promise).then(async () => {
- await waitUntil(() => element.queryErrorMessage === 'Test error');
+ await element.latestSuggestionUpdateComplete;
+ await waitUntil(() => element.queryStatus?.message === 'Test error');
+
+ queryStub.resolves([] as AutocompleteSuggestion[]);
element.text = '';
element.threshold = 0;
await element.updateComplete;
- assert.isUndefined(element.queryErrorMessage);
+ await element.latestSuggestionUpdateComplete;
+ assert.isUndefined(element.queryStatus);
});
});
@@ -514,6 +523,7 @@
return promise.then(async () => {
const commitHandler = sinon.spy();
element.addEventListener('commit', commitHandler);
+ await element.latestSuggestionUpdateComplete;
await waitUntil(() => element.suggestionsDropdown?.isHidden === false);
pressKey(inputEl(), Key.ENTER);
@@ -524,15 +534,24 @@
});
test('tabComplete flag functions', async () => {
+ element.query = sinon
+ .stub()
+ .resolves([
+ {name: 'tunnel snakes rule!', value: 'snakes'},
+ ] as AutocompleteSuggestion[]);
+
// commitHandler checks for the commit event, whereas commitSpy checks for
// the _commit function of the element.
const commitHandler = sinon.spy();
element.addEventListener('commit', commitHandler);
const commitSpy = sinon.spy(element, '_commit');
element.setFocus(true);
-
- element.suggestions = [{text: 'tunnel snakes rule!', name: ''}];
element.tabComplete = false;
+ element.text = 'text1';
+ await element.updateComplete;
+
+ await element.latestSuggestionUpdateComplete;
+ await element.updateComplete;
pressKey(inputEl(), Key.TAB);
await element.updateComplete;
@@ -540,9 +559,12 @@
assert.isFalse(commitSpy.called);
assert.isFalse(element.focused);
- element.tabComplete = true;
- await element.updateComplete;
element.setFocus(true);
+ element.tabComplete = true;
+ element.text = 'text2';
+ await element.updateComplete;
+
+ await element.latestSuggestionUpdateComplete;
await element.updateComplete;
pressKey(inputEl(), Key.TAB);
@@ -597,7 +619,11 @@
' allowNonSuggestedValues',
() => {
const commitStub = sinon.stub(element, '_commit');
- element.queryErrorMessage = 'Error';
+ element.queryStatus = {
+ type: AutocompleteQueryStatusType.ERROR,
+ message: 'Error',
+ };
+ element.suggestions = [];
element.handleInputCommit();
assert.isFalse(commitStub.called);
}
@@ -620,7 +646,11 @@
() => {
const commitStub = sinon.stub(element, '_commit');
element.allowNonSuggestedValues = true;
- element.queryErrorMessage = 'Error';
+ element.queryStatus = {
+ type: AutocompleteQueryStatusType.ERROR,
+ message: 'Error',
+ };
+ element.suggestions = [];
element.handleInputCommit();
assert.isTrue(commitStub.called);
}
@@ -629,6 +659,7 @@
test('handleInputCommit with autocomplete open calls commit', () => {
const commitStub = sinon.stub(element, '_commit');
suggestionsEl().isHidden = false;
+ element.suggestions = [{name: 'first suggestion'}];
element.handleInputCommit();
assert.isTrue(commitStub.calledOnce);
});
@@ -671,6 +702,215 @@
assert.equal(element.text, 'file:x');
});
+ test('render loading replace with suggestions when done', async () => {
+ const queryPromise = mockPromise<AutocompleteSuggestion[]>();
+ element.query = (_: string) => queryPromise;
+
+ element.setFocus(true);
+ element.text = 'blah';
+ await element.updateComplete;
+ await waitUntil(() => !suggestionsEl().isHidden);
+ assert.deepEqual(element.queryStatus, {
+ type: AutocompleteQueryStatusType.LOADING,
+ message: 'Loading...',
+ });
+
+ queryPromise.resolve([{name: 'suggestion 1'}] as AutocompleteSuggestion[]);
+ await element.latestSuggestionUpdateComplete;
+ await element.updateComplete;
+
+ assert.equal(element.suggestions.length, 1);
+ assert.isUndefined(element.queryStatus);
+ });
+
+ test('render loading replace with error when done', async () => {
+ const queryPromise = mockPromise<AutocompleteSuggestion[]>();
+ element.query = (_: string) => queryPromise;
+
+ element.setFocus(true);
+ element.text = 'blah';
+ await element.updateComplete;
+ await waitUntil(() => !suggestionsEl().isHidden);
+ assert.deepEqual(element.queryStatus, {
+ type: AutocompleteQueryStatusType.LOADING,
+ message: 'Loading...',
+ });
+
+ queryPromise.reject(new Error('Test error'));
+ await assertFails(queryPromise);
+ await element.latestSuggestionUpdateComplete;
+ await element.updateComplete;
+
+ assert.equal(element.suggestions.length, 0);
+ assert.deepEqual(element.queryStatus, {
+ type: AutocompleteQueryStatusType.ERROR,
+ message: 'Test error',
+ });
+ });
+
+ test('render loading esc cancels', async () => {
+ const queryPromise = mockPromise<AutocompleteSuggestion[]>();
+ element.query = (_: string) => queryPromise;
+
+ element.setFocus(true);
+ element.text = 'blah';
+ await element.updateComplete;
+ await waitUntil(() => !suggestionsEl().isHidden);
+ assert.deepEqual(element.queryStatus, {
+ type: AutocompleteQueryStatusType.LOADING,
+ message: 'Loading...',
+ });
+
+ const cancelHandler = sinon.spy();
+ element.addEventListener('cancel', cancelHandler);
+ pressKey(inputEl(), Key.ESC);
+ await waitUntil(() => suggestionsEl().isHidden);
+
+ assert.isFalse(cancelHandler.called);
+ assert.isUndefined(element.queryStatus);
+
+ pressKey(inputEl(), Key.ESC);
+ await element.updateComplete;
+
+ assert.isTrue(cancelHandler.called);
+ });
+
+ test('while loading queue enter commits', async () => {
+ const commitHandler = sinon.stub();
+ element.addEventListener('commit', commitHandler);
+ let resolvePromise: (value: AutocompleteSuggestion[]) => void;
+ const blockingPromise = new Promise<AutocompleteSuggestion[]>(resolve => {
+ resolvePromise = resolve;
+ });
+ element.query = (_: string) => blockingPromise;
+
+ element.setFocus(true);
+ element.text = 'blah';
+ await element.updateComplete;
+ await waitUntil(() => !suggestionsEl().isHidden);
+ assert.deepEqual(element.queryStatus, {
+ type: AutocompleteQueryStatusType.LOADING,
+ message: 'Loading...',
+ });
+
+ pressKey(inputEl(), Key.ENTER);
+ await element.updateComplete;
+ assert.deepEqual(element.queryStatus, {
+ type: AutocompleteQueryStatusType.LOADING,
+ message: 'Loading... (Handle Enter on load)',
+ });
+
+ resolvePromise!([{name: 'suggestion 1'}] as AutocompleteSuggestion[]);
+ await element.latestSuggestionUpdateComplete;
+ await element.updateComplete;
+
+ assert.equal(element.suggestions.length, 0);
+ assert.isUndefined(element.queryStatus);
+ assert.isTrue(commitHandler.called);
+ });
+
+ test('while loading queue tab completes', async () => {
+ element.tabComplete = true;
+ const commitHandler = sinon.stub();
+ element.addEventListener('commit', commitHandler);
+ const queryPromise = mockPromise<AutocompleteSuggestion[]>();
+ element.query = (_: string) => queryPromise;
+
+ element.setFocus(true);
+ element.text = 'blah';
+ await element.updateComplete;
+ await waitUntil(() => !suggestionsEl().isHidden);
+ assert.deepEqual(element.queryStatus, {
+ type: AutocompleteQueryStatusType.LOADING,
+ message: 'Loading...',
+ });
+
+ pressKey(inputEl(), Key.TAB);
+ await element.updateComplete;
+ assert.deepEqual(element.queryStatus, {
+ type: AutocompleteQueryStatusType.LOADING,
+ message: 'Loading... (Handle Tab on load)',
+ });
+
+ queryPromise.resolve([{name: 'suggestion 1'}] as AutocompleteSuggestion[]);
+ await element.latestSuggestionUpdateComplete;
+ await element.updateComplete;
+
+ assert.equal(element.suggestions.length, 0);
+ assert.isUndefined(element.queryStatus);
+ assert.isFalse(commitHandler.called);
+ assert.equal(element.text, 'suggestion 1');
+ });
+
+ test('while loading and queued update text cancels', async () => {
+ const commitHandler = sinon.stub();
+ element.addEventListener('commit', commitHandler);
+ const queryPromise = mockPromise<AutocompleteSuggestion[]>();
+ element.query = (_: string) => queryPromise;
+
+ element.setFocus(true);
+ element.text = 'blah';
+ await element.updateComplete;
+ await waitUntil(() => !suggestionsEl().isHidden);
+ assert.deepEqual(element.queryStatus, {
+ type: AutocompleteQueryStatusType.LOADING,
+ message: 'Loading...',
+ });
+
+ pressKey(inputEl(), Key.ENTER);
+ await element.updateComplete;
+ assert.deepEqual(element.queryStatus, {
+ type: AutocompleteQueryStatusType.LOADING,
+ message: 'Loading... (Handle Enter on load)',
+ });
+
+ element.text = 'more blah';
+ await element.updateComplete;
+
+ queryPromise.resolve([{name: 'suggestion 1'}] as AutocompleteSuggestion[]);
+ await element.latestSuggestionUpdateComplete;
+ await element.updateComplete;
+
+ // Commit for stale request is not called.
+ assert.isFalse(commitHandler.called);
+ });
+
+ test('while loading and queued esc cancels', async () => {
+ const commitHandler = sinon.stub();
+ element.addEventListener('commit', commitHandler);
+ const queryPromise = mockPromise<AutocompleteSuggestion[]>();
+ element.query = (_: string) => queryPromise;
+
+ element.setFocus(true);
+ element.text = 'blah';
+ await element.updateComplete;
+ await waitUntil(() => !suggestionsEl().isHidden);
+ assert.deepEqual(element.queryStatus, {
+ type: AutocompleteQueryStatusType.LOADING,
+ message: 'Loading...',
+ });
+
+ pressKey(inputEl(), Key.ENTER);
+ await element.updateComplete;
+ assert.deepEqual(element.queryStatus, {
+ type: AutocompleteQueryStatusType.LOADING,
+ message: 'Loading... (Handle Enter on load)',
+ });
+
+ pressKey(inputEl(), Key.ESC);
+ await element.updateComplete;
+
+ queryPromise.resolve([{name: 'suggestion 1'}] as AutocompleteSuggestion[]);
+ await element.latestSuggestionUpdateComplete;
+ await element.updateComplete;
+
+ // Commit for stale request is not called.
+ assert.isFalse(commitHandler.called);
+ // Query results and status are cleared
+ assert.equal(element.suggestions.length, 0);
+ assert.isUndefined(element.queryStatus);
+ });
+
suite('focus', () => {
let commitSpy: sinon.SinonSpy;
let focusSpy: sinon.SinonSpy;
@@ -688,13 +928,16 @@
await element.updateComplete;
assert.equal(element.suggestions.length, 0);
- assert.isUndefined(element.queryErrorMessage);
+ assert.isUndefined(element.queryStatus);
assert.isTrue(suggestionsEl().isHidden);
});
test('enter in input does not re-render error', async () => {
element.allowNonSuggestedValues = true;
- element.queryErrorMessage = 'Error message';
+ element.queryStatus = {
+ type: AutocompleteQueryStatusType.ERROR,
+ message: 'Error message',
+ };
pressKey(inputEl(), Key.ENTER);
@@ -702,7 +945,7 @@
await element.updateComplete;
assert.equal(element.suggestions.length, 0);
- assert.isUndefined(element.queryErrorMessage);
+ assert.isUndefined(element.queryStatus);
assert.isTrue(suggestionsEl().isHidden);
});
diff --git a/polygerrit-ui/app/elements/shared/gr-change-status/gr-change-status.ts b/polygerrit-ui/app/elements/shared/gr-change-status/gr-change-status.ts
index d2b9e2d..073d9f1 100644
--- a/polygerrit-ui/app/elements/shared/gr-change-status/gr-change-status.ts
+++ b/polygerrit-ui/app/elements/shared/gr-change-status/gr-change-status.ts
@@ -7,7 +7,6 @@
import '../gr-tooltip-content/gr-tooltip-content';
import '../../../styles/shared-styles';
import {ChangeInfo} from '../../../types/common';
-import {ParsedChangeInfo} from '../../../types/types';
import {sharedStyles} from '../../../styles/shared-styles';
import {LitElement, PropertyValues, html, css} from 'lit';
import {customElement, property, state} from 'lit/decorators.js';
@@ -50,9 +49,6 @@
@property({type: Boolean, reflect: true})
flat = false;
- @property({type: Object})
- change?: ChangeInfo | ParsedChangeInfo;
-
@property({type: String})
status?: ChangeStates;
diff --git a/polygerrit-ui/app/elements/shared/gr-editable-label/gr-editable-label_test.ts b/polygerrit-ui/app/elements/shared/gr-editable-label/gr-editable-label_test.ts
index d916118..3bb058e 100644
--- a/polygerrit-ui/app/elements/shared/gr-editable-label/gr-editable-label_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-editable-label/gr-editable-label_test.ts
@@ -295,7 +295,10 @@
suggestions = [{name: 'value text 1'}, {name: 'value text 2'}];
await element.open();
+ // Waiting until dropdown not hidden, will ensure dialog is open and input
+ // is focused, but not that the suggestion has loaded.
await waitUntil(() => !autocomplete.suggestionsDropdown!.isHidden);
+ await autocomplete.latestSuggestionUpdateComplete;
pressKey(autocomplete.input!, Key.ENTER);
@@ -312,7 +315,11 @@
test('autocomplete suggestions closed enter saves suggestion', async () => {
suggestions = [{name: 'value text 1'}, {name: 'value text 2'}];
await element.open();
+ // Waiting until dropdown not hidden, will ensure dialog is open and input
+ // is focused, but not that the suggestion has loaded.
await waitUntil(() => !autocomplete.suggestionsDropdown!.isHidden);
+ await autocomplete.latestSuggestionUpdateComplete;
+
// Press enter to close suggestions.
pressKey(autocomplete.input!, Key.ENTER);
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-row.ts b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-row.ts
index 13a6b00..b97ae59 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-row.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-row.ts
@@ -160,6 +160,8 @@
${this.renderLineNumberCell(Side.RIGHT)}
${this.renderSignCell(Side.RIGHT)} ${this.renderContentCell(Side.RIGHT)}
</tr>
+ ${this.renderPostLineSlot(Side.LEFT)}
+ ${this.renderPostLineSlot(Side.RIGHT)}
`;
if (this.addTableWrapperForTesting) {
return html`<table>
@@ -456,6 +458,13 @@
id=${this.contentId(side)}
>${textElement}</div>`;
}
+
+ private renderPostLineSlot(side: Side) {
+ const lineNumber = this.lineNumber(side);
+ return lineNumber && Number.isInteger(lineNumber)
+ ? html`<slot name="post-${side}-line-${lineNumber}"></slot>`
+ : nothing;
+ }
}
declare global {
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-row_test.ts b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-row_test.ts
index 1c7b311..42d30aa 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-row_test.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-row_test.ts
@@ -87,6 +87,8 @@
</div>
</td>
</tr>
+ <slot name="post-left-line-1"></slot>
+ <slot name="post-right-line-1"></slot>
</tbody>
</table>
`
@@ -147,6 +149,8 @@
</div>
</td>
</tr>
+ <slot name="post-left-line-1"></slot>
+ <slot name="post-right-line-1"></slot>
</tbody>
</table>
`
@@ -201,6 +205,7 @@
<slot name="right-1"> </slot>
</div>
</td>
+ <slot name="post-right-line-1"></slot>
</tr>
</tbody>
</table>
@@ -257,6 +262,7 @@
<div class="contentText gr-diff" data-side="right"></div>
</td>
</tr>
+ <slot name="post-left-line-1"></slot>
</tbody>
</table>
`
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-section_test.ts b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-section_test.ts
index 14f4536..381f9b2 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-section_test.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-section_test.ts
@@ -126,8 +126,14 @@
element,
/* HTML */ `
<gr-diff-row class="left-1 right-1"> </gr-diff-row>
+ <slot name="post-left-line-1"></slot>
+ <slot name="post-right-line-1"></slot>
<gr-diff-row class="left-1 right-1"> </gr-diff-row>
+ <slot name="post-left-line-1"></slot>
+ <slot name="post-right-line-1"></slot>
<gr-diff-row class="left-1 right-1"> </gr-diff-row>
+ <slot name="post-left-line-1"></slot>
+ <slot name="post-right-line-1"></slot>
<table>
<tbody class="both gr-diff section">
<tr
diff --git a/polygerrit-ui/app/models/accounts-model/accounts-model.ts b/polygerrit-ui/app/models/accounts-model/accounts-model.ts
index 2bf6068..1c67857 100644
--- a/polygerrit-ui/app/models/accounts-model/accounts-model.ts
+++ b/polygerrit-ui/app/models/accounts-model/accounts-model.ts
@@ -8,11 +8,14 @@
import {RestApiService} from '../../services/gr-rest-api/gr-rest-api';
import {UserId} from '../../types/common';
import {getUserId, isDetailedAccount} from '../../utils/account-util';
+import {hasOwnProperty} from '../../utils/common-util';
import {define} from '../dependency';
import {Model} from '../model';
export interface AccountsState {
- accounts: {[id: UserId]: AccountDetailInfo};
+ accounts: {
+ [id: UserId]: AccountDetailInfo | AccountInfo;
+ };
}
export const accountsModelToken = define<AccountsModel>('accounts-model');
@@ -24,33 +27,36 @@
});
}
- private updateStateAccount(id: UserId, account?: AccountDetailInfo) {
+ private updateStateAccount(
+ id: UserId,
+ account: AccountDetailInfo | AccountInfo
+ ) {
if (!account) return;
const current = {...this.getState()};
current.accounts = {...current.accounts, [id]: account};
this.setState(current);
}
- async getAccount(partialAccount: AccountInfo) {
+ async getAccount(
+ partialAccount: AccountInfo
+ ): Promise<AccountDetailInfo | AccountInfo> {
const current = this.getState();
const id = getUserId(partialAccount);
- if (current.accounts[id]) return current.accounts[id];
+ if (hasOwnProperty(current.accounts, id)) return current.accounts[id];
// It is possible to add emails to CC when they don't have a Gerrit
- // account. In this case getAccountDetails will return a 404 error hence
- // pass an empty error function to handle that.
+ // account. In this case getAccountDetails will return a 404 error then
+ // we at least use what is in partialAccount.
const account = await this.restApiService.getAccountDetails(id, () => {
- this.updateStateAccount(id, partialAccount as AccountDetailInfo);
+ this.updateStateAccount(id, partialAccount);
return;
});
if (account) this.updateStateAccount(id, account);
- return account;
+ return account ?? partialAccount;
}
async fillDetails(account: AccountInfo) {
if (!isDetailedAccount(account)) {
- if (account.email) return await this.getAccount({email: account.email});
- else if (account._account_id)
- return await this.getAccount({_account_id: account._account_id});
+ return await this.getAccount(account);
}
return account;
}
diff --git a/polygerrit-ui/app/services/flags/flags.ts b/polygerrit-ui/app/services/flags/flags.ts
index 29e9259..7488e79 100644
--- a/polygerrit-ui/app/services/flags/flags.ts
+++ b/polygerrit-ui/app/services/flags/flags.ts
@@ -19,5 +19,4 @@
PUSH_NOTIFICATIONS_DEVELOPER = 'UiFeature__push_notifications_developer',
PUSH_NOTIFICATIONS = 'UiFeature__push_notifications',
SUGGEST_EDIT = 'UiFeature__suggest_edit',
- REBASE_CHAIN = 'UiFeature__rebase_chain',
}
diff --git a/polygerrit-ui/app/services/gr-rest-api/gr-rest-api-impl.ts b/polygerrit-ui/app/services/gr-rest-api/gr-rest-api-impl.ts
index 0d0c88f..610d8f3 100644
--- a/polygerrit-ui/app/services/gr-rest-api/gr-rest-api-impl.ts
+++ b/polygerrit-ui/app/services/gr-rest-api/gr-rest-api-impl.ts
@@ -769,7 +769,7 @@
userId: AccountId | EmailAddress,
errFn?: ErrorCallback
): Promise<AccountDetailInfo | undefined> {
- return this._restApiHelper.fetchJSON({
+ return this._fetchSharedCacheURL({
url: `/accounts/${encodeURIComponent(userId)}/detail`,
anonymizedUrl: '/accounts/*/detail',
errFn,
diff --git a/polygerrit-ui/app/utils/async-util.ts b/polygerrit-ui/app/utils/async-util.ts
index 752de62..905b16c 100644
--- a/polygerrit-ui/app/utils/async-util.ts
+++ b/polygerrit-ui/app/utils/async-util.ts
@@ -62,6 +62,8 @@
/**
* Promise that is resolved after the callback is run or the task is
* cancelled.
+ *
+ * If callback returns a Promise this resolves after the promise is settled.
*/
public readonly promise: Promise<ResolvedDelayedTaskStatus>;
@@ -69,14 +71,25 @@
value: ResolvedDelayedTaskStatus | PromiseLike<ResolvedDelayedTaskStatus>
) => void;
- constructor(private callback: () => void, waitMs = 0) {
+ private callCallbackAndResolveOnCompletion() {
+ let callbackResult;
+ if (this.callback) callbackResult = this.callback();
+ if (callbackResult instanceof Promise) {
+ callbackResult.finally(() =>
+ this.resolvePromise!(ResolvedDelayedTaskStatus.CALLBACK_EXECUTED)
+ );
+ } else {
+ this.resolvePromise!(ResolvedDelayedTaskStatus.CALLBACK_EXECUTED);
+ }
+ }
+
+ constructor(private callback: () => void | Promise<void>, waitMs = 0) {
this.promise = new Promise(resolve => {
this.resolvePromise = resolve;
this.timerId = window.setTimeout(() => {
if (this.timerId) _testOnly_allTasks.delete(this.timerId);
this.timerId = undefined;
- if (this.callback) this.callback();
- resolve(ResolvedDelayedTaskStatus.CALLBACK_EXECUTED);
+ this.callCallbackAndResolveOnCompletion();
}, waitMs);
_testOnly_allTasks.set(this.timerId, this);
});
@@ -98,8 +111,7 @@
flush() {
if (this.isActive()) {
this.cancelTimer();
- if (this.callback) this.callback();
- this.resolvePromise?.(ResolvedDelayedTaskStatus.CALLBACK_EXECUTED);
+ this.callCallbackAndResolveOnCompletion();
}
}
diff --git a/polygerrit-ui/app/utils/async-util_test.ts b/polygerrit-ui/app/utils/async-util_test.ts
index 9f029b8..ee4f73a 100644
--- a/polygerrit-ui/app/utils/async-util_test.ts
+++ b/polygerrit-ui/app/utils/async-util_test.ts
@@ -6,8 +6,8 @@
import {assert} from '@open-wc/testing';
import {SinonFakeTimers} from 'sinon';
import '../test/common-test-setup';
-import {waitEventLoop} from '../test/test-utils';
-import {asyncForeach, debounceP} from './async-util';
+import {mockPromise, waitEventLoop, waitUntil} from '../test/test-utils';
+import {asyncForeach, debounceP, DelayedTask} from './async-util';
suite('async-util tests', () => {
suite('asyncForeach', () => {
@@ -205,4 +205,16 @@
await waitEventLoop();
});
});
+
+ test('DelayedTask promise resolved when callback is done', async () => {
+ const callbackPromise = mockPromise<void>();
+ const task = new DelayedTask(() => callbackPromise);
+ let completed = false;
+ task.promise.then(() => (completed = true));
+ await waitUntil(() => !task.isActive());
+
+ assert.isFalse(completed);
+ callbackPromise.resolve();
+ await waitUntil(() => completed);
+ });
});
