Merge "Toggling theme reloads the page"
diff --git a/java/com/google/gerrit/acceptance/AbstractDaemonTest.java b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
index a3b5e8d..f7b343b 100644
--- a/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
+++ b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
@@ -38,12 +38,14 @@
import com.github.rholder.retry.BlockStrategy;
import com.google.common.base.Strings;
+import com.google.common.base.Ticker;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Iterables;
import com.google.common.collect.Lists;
import com.google.common.jimfs.Jimfs;
import com.google.common.primitives.Chars;
+import com.google.common.testing.FakeTicker;
import com.google.gerrit.acceptance.AcceptanceTestRequestScope.Context;
import com.google.gerrit.acceptance.PushOneCommit.Result;
import com.google.gerrit.acceptance.testsuite.account.TestSshKeys;
@@ -290,6 +292,7 @@
@Inject protected ChangeNotes.Factory notesFactory;
@Inject protected BatchAbandon batchAbandon;
@Inject protected TestSshKeys sshKeys;
+ @Inject protected TestTicker testTicker;
protected EventRecorder eventRecorder;
protected GerritServer server;
@@ -703,6 +706,8 @@
}
SystemReader.setInstance(oldSystemReader);
oldSystemReader = null;
+ // Set useDefaultTicker in afterTest, so the next beforeTest will use the default ticker
+ testTicker.useDefaultTicker();
}
protected void closeSsh() {
@@ -1764,4 +1769,32 @@
moduleClass.getName());
}
}
+
+ /** {@link Ticker} implementation for mocking without restarting GerritServer */
+ public static class TestTicker extends Ticker {
+ Ticker actualTicker;
+
+ public TestTicker() {
+ useDefaultTicker();
+ }
+
+ /** Switches to system ticker */
+ public Ticker useDefaultTicker() {
+ this.actualTicker = Ticker.systemTicker();
+ return actualTicker;
+ }
+
+ /** Switches to {@link FakeTicker} */
+ public FakeTicker useFakeTicker() {
+ if (!(this.actualTicker instanceof FakeTicker)) {
+ this.actualTicker = new FakeTicker();
+ }
+ return (FakeTicker) actualTicker;
+ }
+
+ @Override
+ public long read() {
+ return actualTicker.read();
+ }
+ }
}
diff --git a/java/com/google/gerrit/acceptance/BUILD b/java/com/google/gerrit/acceptance/BUILD
index fa62cd9..fe6e160 100644
--- a/java/com/google/gerrit/acceptance/BUILD
+++ b/java/com/google/gerrit/acceptance/BUILD
@@ -124,6 +124,7 @@
"//lib/truth",
"//lib/truth:truth-java8-extension",
"//lib/greenmail",
+ "//lib:guava-testlib",
] + TEST_DEPS
java_library(
diff --git a/java/com/google/gerrit/acceptance/GerritServer.java b/java/com/google/gerrit/acceptance/GerritServer.java
index 33abc68..402d21d 100644
--- a/java/com/google/gerrit/acceptance/GerritServer.java
+++ b/java/com/google/gerrit/acceptance/GerritServer.java
@@ -22,7 +22,9 @@
import com.google.auto.value.AutoValue;
import com.google.common.base.MoreObjects;
import com.google.common.base.Strings;
+import com.google.common.base.Ticker;
import com.google.common.collect.ImmutableList;
+import com.google.gerrit.acceptance.AbstractDaemonTest.TestTicker;
import com.google.gerrit.acceptance.FakeGroupAuditService.FakeGroupAuditServiceModule;
import com.google.gerrit.acceptance.ReindexGroupsAtStartup.ReindexGroupsAtStartupModule;
import com.google.gerrit.acceptance.ReindexProjectsAtStartup.ReindexProjectsAtStartupModule;
@@ -75,6 +77,7 @@
import com.google.inject.Module;
import com.google.inject.Provides;
import com.google.inject.Singleton;
+import com.google.inject.multibindings.OptionalBinder;
import java.lang.annotation.Annotation;
import java.lang.annotation.Retention;
import java.lang.reflect.Field;
@@ -429,6 +432,23 @@
.to(GitObjectVisibilityChecker.class);
}
});
+ daemon.addAdditionalSysModuleForTesting(
+ new AbstractModule() {
+ @Override
+ protected void configure() {
+ super.configure();
+ // GerritServer isn't restarted between tests. TestTicker allows to replace actual
+ // Ticker in tests without restarting server and transparently for other code.
+ // Alternative option with Provider<Ticker> is less convinient, because it affects how
+ // gerrit code should be written - i.e. Ticker must not be stored in fields and must
+ // always be obtained from the provider.
+ TestTicker testTicker = new TestTicker();
+ OptionalBinder.newOptionalBinder(binder(), Ticker.class)
+ .setBinding()
+ .toInstance(testTicker);
+ bind(TestTicker.class).toInstance(testTicker);
+ }
+ });
if (desc.memory()) {
checkArgument(additionalArgs.length == 0, "cannot pass args to in-memory server");
diff --git a/java/com/google/gerrit/entities/RefNames.java b/java/com/google/gerrit/entities/RefNames.java
index 2263aba..349b67e 100644
--- a/java/com/google/gerrit/entities/RefNames.java
+++ b/java/com/google/gerrit/entities/RefNames.java
@@ -489,7 +489,7 @@
return Integer.parseInt(rest.substring(0, ie));
}
- static Integer parseRefSuffix(String name) {
+ public static Integer parseRefSuffix(String name) {
if (name == null) {
return null;
}
diff --git a/java/com/google/gerrit/entities/SubmitRequirementExpressionResult.java b/java/com/google/gerrit/entities/SubmitRequirementExpressionResult.java
index 900b2e2..9e4416b 100644
--- a/java/com/google/gerrit/entities/SubmitRequirementExpressionResult.java
+++ b/java/com/google/gerrit/entities/SubmitRequirementExpressionResult.java
@@ -39,17 +39,17 @@
/**
* List leaf predicates that are fulfilled, for example the expression
*
- * <p><i>label:code-review=+2 and branch:refs/heads/master</i>
+ * <p><i>label:Code-Review=+2 and branch:refs/heads/master</i>
*
* <p>has two leaf predicates:
*
* <ul>
- * <li>label:code-review=+2
+ * <li>label:Code-Review=+2
* <li>branch:refs/heads/master
* </ul>
*
* This method will return the leaf predicates that were fulfilled, for example if only the first
- * predicate was fulfilled, the returned list will be equal to ["label:code-review=+2"].
+ * predicate was fulfilled, the returned list will be equal to ["label:Code-Review=+2"].
*/
public abstract ImmutableList<String> passingAtoms();
diff --git a/java/com/google/gerrit/server/CommentsUtil.java b/java/com/google/gerrit/server/CommentsUtil.java
index ba9f6d6..1d38877 100644
--- a/java/com/google/gerrit/server/CommentsUtil.java
+++ b/java/com/google/gerrit/server/CommentsUtil.java
@@ -52,9 +52,11 @@
import java.sql.Timestamp;
import java.util.ArrayList;
import java.util.Collection;
+import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Optional;
+import java.util.Set;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.Repository;
@@ -465,10 +467,35 @@
}
}
+ /** returns all changes that contain draft comments of {@code accountId}. */
+ public Collection<Change.Id> getChangesWithDrafts(Account.Id accountId) {
+ try (Repository repo = repoManager.openRepository(allUsers)) {
+ return getChangesWithDrafts(repo, accountId);
+ } catch (IOException e) {
+ throw new StorageException(e);
+ }
+ }
+
private Collection<Ref> getDraftRefs(Repository repo, Change.Id changeId) throws IOException {
return repo.getRefDatabase().getRefsByPrefix(RefNames.refsDraftCommentsPrefix(changeId));
}
+ private Collection<Change.Id> getChangesWithDrafts(Repository repo, Account.Id accountId)
+ throws IOException {
+ Set<Change.Id> changes = new HashSet<>();
+ for (Ref ref : repo.getRefDatabase().getRefsByPrefix(RefNames.REFS_DRAFT_COMMENTS)) {
+ Integer accountIdFromRef = RefNames.parseRefSuffix(ref.getName());
+ if (accountIdFromRef != null && accountIdFromRef == accountId.get()) {
+ Change.Id changeId = Change.Id.fromAllUsersRef(ref.getName());
+ if (changeId == null) {
+ continue;
+ }
+ changes.add(changeId);
+ }
+ }
+ return changes;
+ }
+
private static <T extends Comment> List<T> sort(List<T> comments) {
comments.sort(COMMENT_ORDER);
return comments;
diff --git a/java/com/google/gerrit/server/StarredChangesUtil.java b/java/com/google/gerrit/server/StarredChangesUtil.java
index 5dcbd01..7c61c92 100644
--- a/java/com/google/gerrit/server/StarredChangesUtil.java
+++ b/java/com/google/gerrit/server/StarredChangesUtil.java
@@ -289,6 +289,35 @@
}
}
+ public ImmutableSet<Change.Id> byAccountId(Account.Id accountId, String label) {
+ try (Repository repo = repoManager.openRepository(allUsers)) {
+ ImmutableSet.Builder<Change.Id> builder = ImmutableSet.builder();
+ for (Ref ref : repo.getRefDatabase().getRefsByPrefix(RefNames.REFS_STARRED_CHANGES)) {
+ Account.Id currentAccountId = Account.Id.fromRef(ref.getName());
+ // Skip all refs that don't correspond with accountId.
+ if (currentAccountId == null || !currentAccountId.equals(accountId)) {
+ continue;
+ }
+ // Skip all refs that don't contain the required label.
+ StarRef starRef = readLabels(repo, ref.getName());
+ if (!starRef.labels().contains(label)) {
+ continue;
+ }
+
+ // Skip invalid change ids.
+ Change.Id changeId = Change.Id.fromAllUsersRef(ref.getName());
+ if (changeId == null) {
+ continue;
+ }
+ builder.add(changeId);
+ }
+ return builder.build();
+ } catch (IOException e) {
+ throw new StorageException(
+ String.format("Get starred changes for account %d failed", accountId.get()), e);
+ }
+ }
+
public ImmutableListMultimap<Account.Id, String> byChangeFromIndex(Change.Id changeId) {
List<ChangeData> changeData =
queryProvider
diff --git a/java/com/google/gerrit/server/change/ChangeInserter.java b/java/com/google/gerrit/server/change/ChangeInserter.java
index 85482e4..c8001bb 100644
--- a/java/com/google/gerrit/server/change/ChangeInserter.java
+++ b/java/com/google/gerrit/server/change/ChangeInserter.java
@@ -364,7 +364,7 @@
* <p>Should not be used in new code, as it doesn't result in a single atomic batch ref update for
* code and NoteDb meta refs.
*
- * @param updateRef whether to update the ref during {@code updateRepo}.
+ * @param updateRef whether to update the ref during {@link #updateRepo(RepoContext)}.
*/
@Deprecated
public ChangeInserter setUpdateRef(boolean updateRef) {
diff --git a/java/com/google/gerrit/server/config/GerritGlobalModule.java b/java/com/google/gerrit/server/config/GerritGlobalModule.java
index 3a7f2b2..6c25bae 100644
--- a/java/com/google/gerrit/server/config/GerritGlobalModule.java
+++ b/java/com/google/gerrit/server/config/GerritGlobalModule.java
@@ -16,6 +16,7 @@
import static com.google.inject.Scopes.SINGLETON;
+import com.google.common.base.Ticker;
import com.google.common.cache.Cache;
import com.google.gerrit.extensions.annotations.Exports;
import com.google.gerrit.extensions.api.changes.ActionVisitor;
@@ -187,6 +188,7 @@
import com.google.gerrit.server.project.ProjectCacheImpl;
import com.google.gerrit.server.project.ProjectNameLockManager;
import com.google.gerrit.server.project.ProjectState;
+import com.google.gerrit.server.project.SubmitRequirementExpressionsValidator;
import com.google.gerrit.server.project.SubmitRequirementsEvaluatorImpl;
import com.google.gerrit.server.project.SubmitRuleEvaluator;
import com.google.gerrit.server.query.approval.ApprovalModule;
@@ -224,6 +226,7 @@
import com.google.inject.Inject;
import com.google.inject.TypeLiteral;
import com.google.inject.internal.UniqueAnnotations;
+import com.google.inject.multibindings.OptionalBinder;
import com.google.template.soy.jbcsrc.api.SoySauce;
import java.util.List;
import org.eclipse.jgit.lib.Config;
@@ -337,6 +340,9 @@
bind(PatchSetInfoFactory.class);
bind(IdentifiedUser.GenericFactory.class).in(SINGLETON);
bind(AccountControl.Factory.class);
+ OptionalBinder.newOptionalBinder(binder(), Ticker.class)
+ .setDefault()
+ .toInstance(Ticker.systemTicker());
bind(UiActions.class);
@@ -388,6 +394,8 @@
DynamicSet.bind(binder(), EventListener.class).to(EventsMetrics.class);
DynamicSet.setOf(binder(), UserScopedEventListener.class);
DynamicSet.setOf(binder(), CommitValidationListener.class);
+ DynamicSet.bind(binder(), CommitValidationListener.class)
+ .to(SubmitRequirementExpressionsValidator.class);
DynamicSet.setOf(binder(), CommentValidator.class);
DynamicSet.setOf(binder(), ChangeMessageModifier.class);
DynamicSet.setOf(binder(), RefOperationValidationListener.class);
diff --git a/java/com/google/gerrit/server/experiments/ExperimentFeaturesConstants.java b/java/com/google/gerrit/server/experiments/ExperimentFeaturesConstants.java
index 1486559..65f8f2d 100644
--- a/java/com/google/gerrit/server/experiments/ExperimentFeaturesConstants.java
+++ b/java/com/google/gerrit/server/experiments/ExperimentFeaturesConstants.java
@@ -45,6 +45,13 @@
GERRIT_BACKEND_REQUEST_FEATURE_ENABLE_SUBMIT_REQUIREMENTS_BACKFILLING_ON_DASHBOARD =
"GerritBackendRequestFeature__enable_submit_requirements_backfilling_on_dashboard";
+ /**
+ * When set, we compute information from All-Users repository if able, instead of computing it
+ * from the change index.
+ */
+ public static final String GERRIT_BACKEND_REQUEST_FEATURE_COMPUTE_FROM_ALL_USERS_REPOSITORY =
+ "GerritBackendRequestFeature__compute_from_all_users_repository";
+
/** 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/java/com/google/gerrit/server/git/MultiProgressMonitor.java b/java/com/google/gerrit/server/git/MultiProgressMonitor.java
index 3de8ff3..09f08bd 100644
--- a/java/com/google/gerrit/server/git/MultiProgressMonitor.java
+++ b/java/com/google/gerrit/server/git/MultiProgressMonitor.java
@@ -19,6 +19,7 @@
import static java.util.concurrent.TimeUnit.NANOSECONDS;
import com.google.common.base.Strings;
+import com.google.common.base.Ticker;
import com.google.common.flogger.FluentLogger;
import com.google.common.util.concurrent.UncheckedExecutionException;
import com.google.gerrit.server.CancellationMetrics;
@@ -180,6 +181,7 @@
private Optional<Long> timeout = Optional.empty();
private final long maxIntervalNanos;
+ private final Ticker ticker;
/**
* Create a new progress monitor for multiple sub-tasks.
@@ -190,10 +192,11 @@
@AssistedInject
private MultiProgressMonitor(
CancellationMetrics cancellationMetrics,
+ Ticker ticker,
@Assisted OutputStream out,
@Assisted TaskKind taskKind,
@Assisted String taskName) {
- this(cancellationMetrics, out, taskKind, taskName, 500, MILLISECONDS);
+ this(cancellationMetrics, ticker, out, taskKind, taskName, 500, MILLISECONDS);
}
/**
@@ -207,12 +210,14 @@
@AssistedInject
private MultiProgressMonitor(
CancellationMetrics cancellationMetrics,
+ Ticker ticker,
@Assisted OutputStream out,
@Assisted TaskKind taskKind,
@Assisted String taskName,
@Assisted long maxIntervalTime,
@Assisted TimeUnit maxIntervalUnit) {
this.cancellationMetrics = cancellationMetrics;
+ this.ticker = ticker;
this.out = out;
this.taskKind = taskKind;
this.taskName = taskName;
@@ -262,7 +267,7 @@
long cancellationTimeoutTime,
TimeUnit cancellationTimeoutUnit)
throws TimeoutException {
- long overallStart = System.nanoTime();
+ long overallStart = ticker.read();
long cancellationNanos =
cancellationTimeoutTime > 0
? NANOSECONDS.convert(cancellationTimeoutTime, cancellationTimeoutUnit)
@@ -278,16 +283,33 @@
synchronized (this) {
long left = maxIntervalNanos;
while (!done) {
- long start = System.nanoTime();
+ long start = ticker.read();
try {
- NANOSECONDS.timedWait(this, left);
+ // Conditions below gives better granularity for timeouts.
+ // Originally, code always used fixed interval:
+ // NANOSECONDS.timedWait(this, maxIntervalNanos);
+ // As a result, the actual check for timeouts happened only every maxIntervalNanos
+ // (default value 500ms); so even if timout was set to 1ms, the actual timeout was 500ms.
+ // This is not a big issue, however it made our tests for timeouts flaky. For example,
+ // some tests in the CancellationIT set timeout to 1ms and expect that server returns
+ // timeout. However, server often returned OK result, because a request takes less than
+ // 500ms.
+ if (deadlineExceeded || deadline == 0) {
+ // We want to set deadlineExceeded flag as earliest as possible. If it is already
+ // set - there is no reason to wait less than maxIntervalNanos
+ NANOSECONDS.timedWait(this, maxIntervalNanos);
+ } else if (start <= deadline) {
+ // if deadlineExceeded is not set, then we should wait until deadline, but no longer
+ // than maxIntervalNanos (because we want to report a progress every maxIntervalNanos).
+ NANOSECONDS.timedWait(this, Math.min(deadline - start + 1, maxIntervalNanos));
+ }
} catch (InterruptedException e) {
throw new UncheckedExecutionException(e);
}
// Send an update on every wakeup (manual or spurious), but only move
// the spinner every maxInterval.
- long now = System.nanoTime();
+ long now = ticker.read();
if (deadline > 0 && now > deadline) {
if (!deadlineExceeded) {
diff --git a/java/com/google/gerrit/server/index/IndexModule.java b/java/com/google/gerrit/server/index/IndexModule.java
index e580f50..a2ef070 100644
--- a/java/com/google/gerrit/server/index/IndexModule.java
+++ b/java/com/google/gerrit/server/index/IndexModule.java
@@ -17,6 +17,7 @@
import static com.google.gerrit.server.git.QueueProvider.QueueType.BATCH;
import static com.google.gerrit.server.git.QueueProvider.QueueType.INTERACTIVE;
+import com.google.common.base.Ticker;
import com.google.common.collect.FluentIterable;
import com.google.common.collect.ImmutableCollection;
import com.google.common.collect.ImmutableList;
@@ -60,6 +61,7 @@
import com.google.inject.Provides;
import com.google.inject.ProvisionException;
import com.google.inject.Singleton;
+import com.google.inject.multibindings.OptionalBinder;
import java.util.Collection;
import java.util.Set;
import java.util.concurrent.TimeUnit;
@@ -112,6 +114,9 @@
@Override
protected void configure() {
factory(MultiProgressMonitor.Factory.class);
+ OptionalBinder.newOptionalBinder(binder(), Ticker.class)
+ .setDefault()
+ .toInstance(Ticker.systemTicker());
bind(AccountIndexRewriter.class);
bind(AccountIndexCollection.class);
diff --git a/java/com/google/gerrit/server/project/ProjectConfig.java b/java/com/google/gerrit/server/project/ProjectConfig.java
index 8f0b535..11ffcad 100644
--- a/java/com/google/gerrit/server/project/ProjectConfig.java
+++ b/java/com/google/gerrit/server/project/ProjectConfig.java
@@ -28,6 +28,7 @@
import com.google.common.base.Splitter;
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Maps;
import com.google.common.collect.Sets;
import com.google.common.flogger.FluentLogger;
@@ -130,6 +131,13 @@
public static final String KEY_SR_SUBMITTABILITY_EXPRESSION = "submittableIf";
public static final String KEY_SR_OVERRIDE_EXPRESSION = "overrideIf";
public static final String KEY_SR_OVERRIDE_IN_CHILD_PROJECTS = "canOverrideInChildProjects";
+ public static final ImmutableSet<String> SR_KEYS =
+ ImmutableSet.of(
+ KEY_SR_DESCRIPTION,
+ KEY_SR_APPLICABILITY_EXPRESSION,
+ KEY_SR_SUBMITTABILITY_EXPRESSION,
+ KEY_SR_OVERRIDE_EXPRESSION,
+ KEY_SR_OVERRIDE_IN_CHILD_PROJECTS);
public static final String KEY_MATCH = "match";
private static final String KEY_HTML = "html";
@@ -936,6 +944,8 @@
}
private void loadSubmitRequirementSections(Config rc) {
+ checkForUnsupportedSubmitRequirementParams(rc);
+
Map<String, String> lowerNames = new HashMap<>();
submitRequirementSections = new LinkedHashMap<>();
for (String name : rc.getSubsections(SUBMIT_REQUIREMENT)) {
@@ -951,18 +961,34 @@
String appExpr = rc.getString(SUBMIT_REQUIREMENT, name, KEY_SR_APPLICABILITY_EXPRESSION);
String blockExpr = rc.getString(SUBMIT_REQUIREMENT, name, KEY_SR_SUBMITTABILITY_EXPRESSION);
String overrideExpr = rc.getString(SUBMIT_REQUIREMENT, name, KEY_SR_OVERRIDE_EXPRESSION);
- boolean canInherit =
- rc.getBoolean(SUBMIT_REQUIREMENT, name, KEY_SR_OVERRIDE_IN_CHILD_PROJECTS, false);
+ boolean canInherit;
+ try {
+ canInherit =
+ rc.getBoolean(SUBMIT_REQUIREMENT, name, KEY_SR_OVERRIDE_IN_CHILD_PROJECTS, false);
+ } catch (IllegalArgumentException e) {
+ String canInheritValue =
+ rc.getString(SUBMIT_REQUIREMENT, name, KEY_SR_OVERRIDE_IN_CHILD_PROJECTS);
+ error(
+ String.format(
+ "Invalid value %s.%s.%s for submit requirement '%s': %s",
+ SUBMIT_REQUIREMENT,
+ name,
+ KEY_SR_OVERRIDE_IN_CHILD_PROJECTS,
+ name,
+ canInheritValue));
+ continue;
+ }
if (blockExpr == null) {
error(
String.format(
- "Submit requirement '%s' does not define a submittability expression.", name));
+ "Setting a submittability expression for submit requirement '%s' is required:"
+ + " Missing %s.%s.%s",
+ name, SUBMIT_REQUIREMENT, name, KEY_SR_SUBMITTABILITY_EXPRESSION));
continue;
}
- // TODO(SR): add expressions validation. Expressions are stored as strings so we need to
- // validate their syntax.
+ // The expressions are validated in SubmitRequirementExpressionsValidator.
SubmitRequirement submitRequirement =
SubmitRequirement.builder()
@@ -978,6 +1004,43 @@
}
}
+ /**
+ * Report unsupported submit requirement parameters as errors.
+ *
+ * <p>Unsupported are submit requirements parameters that
+ *
+ * <ul>
+ * <li>are directly set in the {@code submit-requirement} section (as submit requirements are
+ * solely defined in subsections)
+ * <li>are unknown (maybe they were accidentally misspelled?)
+ * </ul>
+ */
+ private void checkForUnsupportedSubmitRequirementParams(Config rc) {
+ Set<String> directSubmitRequirementParams = rc.getNames(SUBMIT_REQUIREMENT);
+ if (!directSubmitRequirementParams.isEmpty()) {
+ error(
+ String.format(
+ "Submit requirements must be defined in %s.<name> subsections."
+ + " Setting parameters directly in the %s section is not allowed: %s",
+ SUBMIT_REQUIREMENT,
+ SUBMIT_REQUIREMENT,
+ directSubmitRequirementParams.stream().sorted().collect(toImmutableList())));
+ }
+
+ for (String subsection : rc.getSubsections(SUBMIT_REQUIREMENT)) {
+ ImmutableList<String> unknownSubmitRequirementParams =
+ rc.getNames(SUBMIT_REQUIREMENT, subsection).stream()
+ .filter(p -> !SR_KEYS.contains(p))
+ .collect(toImmutableList());
+ if (!unknownSubmitRequirementParams.isEmpty()) {
+ error(
+ String.format(
+ "Unsupported parameters for submit requirement '%s': %s",
+ subsection, unknownSubmitRequirementParams));
+ }
+ }
+ }
+
private void loadLabelSections(Config rc) {
Map<String, String> lowerNames = Maps.newHashMapWithExpectedSize(2);
labelSections = new LinkedHashMap<>();
diff --git a/java/com/google/gerrit/server/project/SubmitRequirementExpressionsValidator.java b/java/com/google/gerrit/server/project/SubmitRequirementExpressionsValidator.java
new file mode 100644
index 0000000..738e71b
--- /dev/null
+++ b/java/com/google/gerrit/server/project/SubmitRequirementExpressionsValidator.java
@@ -0,0 +1,184 @@
+// 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.project;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.entities.RefNames;
+import com.google.gerrit.entities.SubmitRequirement;
+import com.google.gerrit.entities.SubmitRequirementExpression;
+import com.google.gerrit.index.query.QueryParseException;
+import com.google.gerrit.server.events.CommitReceivedEvent;
+import com.google.gerrit.server.git.validators.CommitValidationException;
+import com.google.gerrit.server.git.validators.CommitValidationListener;
+import com.google.gerrit.server.git.validators.CommitValidationMessage;
+import com.google.gerrit.server.git.validators.ValidationMessage;
+import com.google.gerrit.server.patch.DiffNotAvailableException;
+import com.google.gerrit.server.patch.DiffOperations;
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
+import org.eclipse.jgit.errors.ConfigInvalidException;
+
+/**
+ * Validates the expressions of submit requirements in {@code project.config}.
+ *
+ * <p>Other validation of submit requirements is done in {@link ProjectConfig}, see {@code
+ * ProjectConfig#loadSubmitRequirementSections(Config)}.
+ *
+ * <p>The validation of the expressions cannot be in {@link ProjectConfig} as it requires injecting
+ * {@link SubmitRequirementsEvaluator} and we cannot do injections into {@link ProjectConfig} (since
+ * {@link ProjectConfig} is cached in the project cache).
+ */
+@Singleton
+public class SubmitRequirementExpressionsValidator implements CommitValidationListener {
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+
+ private final DiffOperations diffOperations;
+ private final ProjectConfig.Factory projectConfigFactory;
+ private final SubmitRequirementsEvaluator submitRequirementsEvaluator;
+
+ @Inject
+ SubmitRequirementExpressionsValidator(
+ DiffOperations diffOperations,
+ ProjectConfig.Factory projectConfigFactory,
+ SubmitRequirementsEvaluator submitRequirementsEvaluator) {
+ this.diffOperations = diffOperations;
+ this.projectConfigFactory = projectConfigFactory;
+ this.submitRequirementsEvaluator = submitRequirementsEvaluator;
+ }
+
+ @Override
+ public List<CommitValidationMessage> onCommitReceived(CommitReceivedEvent event)
+ throws CommitValidationException {
+ try {
+ if (!event.refName.equals(RefNames.REFS_CONFIG)
+ || !isFileChanged(event, ProjectConfig.PROJECT_CONFIG)) {
+ // the project.config file in refs/meta/config was not modified, hence we do not need to
+ // validate the submit requirements in it
+ return ImmutableList.of();
+ }
+
+ ProjectConfig projectConfig = getProjectConfig(event);
+ ImmutableList<CommitValidationMessage> validationMessages =
+ validateSubmitRequirementExpressions(
+ projectConfig.getSubmitRequirementSections().values());
+ if (!validationMessages.isEmpty()) {
+ throw new CommitValidationException(
+ String.format(
+ "invalid submit requirement expressions in %s (revision = %s)",
+ ProjectConfig.PROJECT_CONFIG, projectConfig.getRevision()),
+ validationMessages);
+ }
+ return ImmutableList.of();
+ } catch (IOException | DiffNotAvailableException | ConfigInvalidException e) {
+ String errorMessage =
+ String.format(
+ "failed to validate submit requirement expressions in %s for revision %s in ref %s"
+ + " of project %s",
+ ProjectConfig.PROJECT_CONFIG,
+ event.commit.getName(),
+ RefNames.REFS_CONFIG,
+ event.project.getNameKey());
+ logger.atSevere().withCause(e).log(errorMessage);
+ throw new CommitValidationException(errorMessage, e);
+ }
+ }
+
+ /**
+ * Whether the given file was changed in the given revision.
+ *
+ * @param receiveEvent the receive event
+ * @param fileName the name of the file
+ */
+ private boolean isFileChanged(CommitReceivedEvent receiveEvent, String fileName)
+ throws DiffNotAvailableException {
+ return diffOperations
+ .listModifiedFilesAgainstParent(
+ receiveEvent.project.getNameKey(), receiveEvent.commit, /* parentNum=*/ 0)
+ .keySet().stream()
+ .anyMatch(fileName::equals);
+ }
+
+ private ProjectConfig getProjectConfig(CommitReceivedEvent receiveEvent)
+ throws IOException, ConfigInvalidException {
+ ProjectConfig projectConfig = projectConfigFactory.create(receiveEvent.project.getNameKey());
+ projectConfig.load(receiveEvent.revWalk, receiveEvent.commit);
+ return projectConfig;
+ }
+
+ private ImmutableList<CommitValidationMessage> validateSubmitRequirementExpressions(
+ Collection<SubmitRequirement> submitRequirements) {
+ List<CommitValidationMessage> validationMessages = new ArrayList<>();
+ for (SubmitRequirement submitRequirement : submitRequirements) {
+ validateSubmitRequirementExpression(
+ validationMessages,
+ submitRequirement,
+ submitRequirement.submittabilityExpression(),
+ ProjectConfig.KEY_SR_SUBMITTABILITY_EXPRESSION);
+ submitRequirement
+ .applicabilityExpression()
+ .ifPresent(
+ expression ->
+ validateSubmitRequirementExpression(
+ validationMessages,
+ submitRequirement,
+ expression,
+ ProjectConfig.KEY_SR_APPLICABILITY_EXPRESSION));
+ submitRequirement
+ .overrideExpression()
+ .ifPresent(
+ expression ->
+ validateSubmitRequirementExpression(
+ validationMessages,
+ submitRequirement,
+ expression,
+ ProjectConfig.KEY_SR_OVERRIDE_EXPRESSION));
+ }
+ return ImmutableList.copyOf(validationMessages);
+ }
+
+ private void validateSubmitRequirementExpression(
+ List<CommitValidationMessage> validationMessages,
+ SubmitRequirement submitRequirement,
+ SubmitRequirementExpression expression,
+ String configKey) {
+ try {
+ submitRequirementsEvaluator.validateExpression(expression);
+ } catch (QueryParseException e) {
+ if (validationMessages.isEmpty()) {
+ validationMessages.add(
+ new CommitValidationMessage(
+ "Invalid project configuration", ValidationMessage.Type.ERROR));
+ }
+ validationMessages.add(
+ new CommitValidationMessage(
+ String.format(
+ " %s: Expression '%s' of submit requirement '%s' (parameter %s.%s.%s) is"
+ + " invalid: %s",
+ ProjectConfig.PROJECT_CONFIG,
+ expression.expressionString(),
+ submitRequirement.name(),
+ ProjectConfig.SUBMIT_REQUIREMENT,
+ submitRequirement.name(),
+ configKey,
+ e.getMessage()),
+ ValidationMessage.Type.ERROR));
+ }
+ }
+}
diff --git a/java/com/google/gerrit/server/query/change/ChangePredicates.java b/java/com/google/gerrit/server/query/change/ChangePredicates.java
index 043b37d..3afdcdd 100644
--- a/java/com/google/gerrit/server/query/change/ChangePredicates.java
+++ b/java/com/google/gerrit/server/query/change/ChangePredicates.java
@@ -14,6 +14,8 @@
package com.google.gerrit.server.query.change;
+import static com.google.common.collect.ImmutableSet.toImmutableSet;
+
import com.google.common.base.CharMatcher;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.Change;
@@ -21,12 +23,15 @@
import com.google.gerrit.entities.Project;
import com.google.gerrit.git.ObjectIds;
import com.google.gerrit.index.query.Predicate;
+import com.google.gerrit.server.CommentsUtil;
+import com.google.gerrit.server.StarredChangesUtil;
import com.google.gerrit.server.change.HashtagsUtil;
import com.google.gerrit.server.index.change.ChangeField;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
import java.util.Locale;
+import java.util.Set;
/** Predicates that match against {@link ChangeData}. */
public class ChangePredicates {
@@ -76,8 +81,34 @@
* Returns a predicate that matches changes where the provided {@link
* com.google.gerrit.entities.Account.Id} has a pending draft comment.
*/
- public static Predicate<ChangeData> draftBy(Account.Id id) {
- return new ChangeIndexPredicate(ChangeField.DRAFTBY, id.toString());
+ public static Predicate<ChangeData> draftBy(
+ boolean computeFromAllUsersRepository, CommentsUtil commentsUtil, Account.Id id) {
+ if (!computeFromAllUsersRepository) {
+ return new ChangeIndexPredicate(ChangeField.DRAFTBY, id.toString());
+ }
+ Set<Predicate<ChangeData>> changeIdPredicates =
+ commentsUtil.getChangesWithDrafts(id).stream()
+ .map(ChangePredicates::idStr)
+ .collect(toImmutableSet());
+ return Predicate.or(changeIdPredicates);
+ }
+
+ /**
+ * Returns a predicate that matches changes where the provided {@link
+ * com.google.gerrit.entities.Account.Id} has starred changes with {@code label}.
+ */
+ public static Predicate<ChangeData> starBy(
+ boolean computeFromAllUsersRepository,
+ StarredChangesUtil starredChangesUtil,
+ Account.Id id,
+ String label) {
+ if (!computeFromAllUsersRepository) {
+ return new StarPredicate(id, label);
+ }
+ return Predicate.or(
+ starredChangesUtil.byAccountId(id, label).stream()
+ .map(ChangePredicates::idStr)
+ .collect(toImmutableSet()));
}
/**
diff --git a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
index da36633..57191c5 100644
--- a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
+++ b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
@@ -17,6 +17,7 @@
import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static com.google.gerrit.entities.Change.CHANGE_ID_PATTERN;
import static com.google.gerrit.server.account.AccountResolver.isSelf;
+import static com.google.gerrit.server.experiments.ExperimentFeaturesConstants.GERRIT_BACKEND_REQUEST_FEATURE_COMPUTE_FROM_ALL_USERS_REPOSITORY;
import static com.google.gerrit.server.query.change.ChangeData.asChanges;
import static java.util.stream.Collectors.toList;
import static java.util.stream.Collectors.toSet;
@@ -70,6 +71,7 @@
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.config.HasOperandAliasConfig;
import com.google.gerrit.server.config.OperatorAliasConfig;
+import com.google.gerrit.server.experiments.ExperimentFeatures;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.index.change.ChangeField;
import com.google.gerrit.server.index.change.ChangeIndex;
@@ -253,6 +255,7 @@
final OperatorAliasConfig operatorAliasConfig;
final boolean indexMergeable;
final boolean conflictsPredicateEnabled;
+ final ExperimentFeatures experimentFeatures;
final HasOperandAliasConfig hasOperandAliasConfig;
final PluginSetContext<SubmitRule> submitRules;
@@ -288,6 +291,7 @@
GroupMembers groupMembers,
OperatorAliasConfig operatorAliasConfig,
@GerritServerConfig Config gerritConfig,
+ ExperimentFeatures experimentFeatures,
HasOperandAliasConfig hasOperandAliasConfig,
ChangeIsVisibleToPredicate.Factory changeIsVisbleToPredicateFactory,
PluginSetContext<SubmitRule> submitRules) {
@@ -320,6 +324,7 @@
operatorAliasConfig,
MergeabilityComputationBehavior.fromConfig(gerritConfig).includeInIndex(),
gerritConfig.getBoolean("change", null, "conflictsPredicateEnabled", true),
+ experimentFeatures,
hasOperandAliasConfig,
changeIsVisbleToPredicateFactory,
submitRules);
@@ -354,6 +359,7 @@
OperatorAliasConfig operatorAliasConfig,
boolean indexMergeable,
boolean conflictsPredicateEnabled,
+ ExperimentFeatures experimentFeatures,
HasOperandAliasConfig hasOperandAliasConfig,
ChangeIsVisibleToPredicate.Factory changeIsVisbleToPredicateFactory,
PluginSetContext<SubmitRule> submitRules) {
@@ -386,6 +392,7 @@
this.operatorAliasConfig = operatorAliasConfig;
this.indexMergeable = indexMergeable;
this.conflictsPredicateEnabled = conflictsPredicateEnabled;
+ this.experimentFeatures = experimentFeatures;
this.hasOperandAliasConfig = hasOperandAliasConfig;
this.submitRules = submitRules;
}
@@ -420,6 +427,7 @@
operatorAliasConfig,
indexMergeable,
conflictsPredicateEnabled,
+ experimentFeatures,
hasOperandAliasConfig,
changeIsVisbleToPredicateFactory,
submitRules);
@@ -1089,15 +1097,29 @@
}
private Predicate<ChangeData> ignoredBySelf() throws QueryParseException {
- return new StarPredicate(self(), StarredChangesUtil.IGNORE_LABEL);
+ return ChangePredicates.starBy(
+ args.experimentFeatures.isFeatureEnabled(
+ GERRIT_BACKEND_REQUEST_FEATURE_COMPUTE_FROM_ALL_USERS_REPOSITORY),
+ args.starredChangesUtil,
+ self(),
+ StarredChangesUtil.IGNORE_LABEL);
}
private Predicate<ChangeData> starredBySelf() throws QueryParseException {
- return new StarPredicate(self(), StarredChangesUtil.DEFAULT_LABEL);
+ return ChangePredicates.starBy(
+ args.experimentFeatures.isFeatureEnabled(
+ GERRIT_BACKEND_REQUEST_FEATURE_COMPUTE_FROM_ALL_USERS_REPOSITORY),
+ args.starredChangesUtil,
+ self(),
+ StarredChangesUtil.DEFAULT_LABEL);
}
private Predicate<ChangeData> draftBySelf() throws QueryParseException {
- return ChangePredicates.draftBy(self());
+ return ChangePredicates.draftBy(
+ args.experimentFeatures.isFeatureEnabled(
+ GERRIT_BACKEND_REQUEST_FEATURE_COMPUTE_FROM_ALL_USERS_REPOSITORY),
+ args.commentsUtil,
+ self());
}
@Operator
diff --git a/java/com/google/gerrit/server/restapi/account/DeleteDraftComments.java b/java/com/google/gerrit/server/restapi/account/DeleteDraftComments.java
index 1485a6e56..ad3c56b 100644
--- a/java/com/google/gerrit/server/restapi/account/DeleteDraftComments.java
+++ b/java/com/google/gerrit/server/restapi/account/DeleteDraftComments.java
@@ -15,6 +15,7 @@
package com.google.gerrit.server.restapi.account;
import static com.google.common.collect.ImmutableList.toImmutableList;
+import static com.google.gerrit.server.experiments.ExperimentFeaturesConstants.GERRIT_BACKEND_REQUEST_FEATURE_COMPUTE_FROM_ALL_USERS_REPOSITORY;
import com.google.common.base.CharMatcher;
import com.google.common.base.Strings;
@@ -39,6 +40,7 @@
import com.google.gerrit.server.PatchSetUtil;
import com.google.gerrit.server.account.AccountResource;
import com.google.gerrit.server.change.ChangeJson;
+import com.google.gerrit.server.experiments.ExperimentFeatures;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.query.change.ChangePredicates;
@@ -75,6 +77,7 @@
private final Provider<CommentJson> commentJsonProvider;
private final CommentsUtil commentsUtil;
private final PatchSetUtil psUtil;
+ private final ExperimentFeatures experimentFeatures;
@Inject
DeleteDraftComments(
@@ -86,7 +89,8 @@
ChangeJson.Factory changeJsonFactory,
Provider<CommentJson> commentJsonProvider,
CommentsUtil commentsUtil,
- PatchSetUtil psUtil) {
+ PatchSetUtil psUtil,
+ ExperimentFeatures experimentFeatures) {
this.userProvider = userProvider;
this.batchUpdateFactory = batchUpdateFactory;
this.queryBuilderProvider = queryBuilderProvider;
@@ -96,6 +100,7 @@
this.commentJsonProvider = commentJsonProvider;
this.commentsUtil = commentsUtil;
this.psUtil = psUtil;
+ this.experimentFeatures = experimentFeatures;
}
@Override
@@ -146,7 +151,12 @@
private Predicate<ChangeData> predicate(Account.Id accountId, DeleteDraftCommentsInput input)
throws BadRequestException {
- Predicate<ChangeData> hasDraft = ChangePredicates.draftBy(accountId);
+ Predicate<ChangeData> hasDraft =
+ ChangePredicates.draftBy(
+ experimentFeatures.isFeatureEnabled(
+ GERRIT_BACKEND_REQUEST_FEATURE_COMPUTE_FROM_ALL_USERS_REPOSITORY),
+ commentsUtil,
+ accountId);
if (CharMatcher.whitespace().trimFrom(Strings.nullToEmpty(input.query)).isEmpty()) {
return hasDraft;
}
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
index 0d7210f..74407c0 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -225,7 +225,6 @@
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevWalk;
import org.eclipse.jgit.transport.PushResult;
-import org.junit.Ignore;
import org.junit.Test;
@NoHttpd
@@ -4059,7 +4058,7 @@
assertThat(testLabel.status).isEqualTo(SubmitRecordInfo.Label.Status.OK);
assertThat(testLabel.appliedBy).isNull();
- voteLabel(changeId, "code-review", 2);
+ voteLabel(changeId, "Code-Review", 2);
// Code review record is satisfied after voting +2
change = gApi.changes().id(changeId).get();
assertThat(change.submitRecords).hasSize(2);
@@ -4166,9 +4165,9 @@
configSubmitRequirement(
project,
SubmitRequirement.builder()
- .setName("code-review")
+ .setName("Code-Review")
.setSubmittabilityExpression(
- SubmitRequirementExpression.create("label:code-review=MAX"))
+ SubmitRequirementExpression.create("label:Code-Review=MAX"))
.setAllowOverrideInChildProjects(false)
.build());
@@ -4178,13 +4177,13 @@
ChangeInfo change = gApi.changes().id(changeId).get();
assertThat(change.submitRequirements).hasSize(1);
assertSubmitRequirementStatus(
- change.submitRequirements, "code-review", Status.UNSATISFIED, /* isLegacy= */ false);
+ change.submitRequirements, "Code-Review", Status.UNSATISFIED, /* isLegacy= */ false);
- voteLabel(changeId, "code-review", 2);
+ voteLabel(changeId, "Code-Review", 2);
change = gApi.changes().id(changeId).get();
assertThat(change.submitRequirements).hasSize(1);
assertSubmitRequirementStatus(
- change.submitRequirements, "code-review", Status.SATISFIED, /* isLegacy= */ false);
+ change.submitRequirements, "Code-Review", Status.SATISFIED, /* isLegacy= */ false);
}
@Test
@@ -4239,9 +4238,9 @@
configSubmitRequirement(
project,
SubmitRequirement.builder()
- .setName("code-review")
+ .setName("Code-Review")
.setSubmittabilityExpression(
- SubmitRequirementExpression.create("-label:code-review=MIN"))
+ SubmitRequirementExpression.create("-label:Code-Review=MIN"))
.setAllowOverrideInChildProjects(false)
.build());
@@ -4252,27 +4251,27 @@
assertThat(change.submitRequirements).hasSize(2);
// Requirement is satisfied because there are no votes
assertSubmitRequirementStatus(
- change.submitRequirements, "code-review", Status.SATISFIED, /* isLegacy= */ false);
+ change.submitRequirements, "Code-Review", Status.SATISFIED, /* isLegacy= */ false);
// Legacy requirement (coming from the label function definition) is not satisfied. We return
// both legacy and non-legacy requirements in this case since their statuses are not identical.
assertSubmitRequirementStatus(
change.submitRequirements, "Code-Review", Status.UNSATISFIED, /* isLegacy= */ true);
- voteLabel(changeId, "code-review", -1);
+ voteLabel(changeId, "Code-Review", -1);
change = gApi.changes().id(changeId).get();
assertThat(change.submitRequirements).hasSize(2);
// Requirement is still satisfied because -1 is not the max negative value
assertSubmitRequirementStatus(
- change.submitRequirements, "code-review", Status.SATISFIED, /* isLegacy= */ false);
+ change.submitRequirements, "Code-Review", Status.SATISFIED, /* isLegacy= */ false);
assertSubmitRequirementStatus(
change.submitRequirements, "Code-Review", Status.UNSATISFIED, /* isLegacy= */ true);
- voteLabel(changeId, "code-review", -2);
+ voteLabel(changeId, "Code-Review", -2);
change = gApi.changes().id(changeId).get();
assertThat(change.submitRequirements).hasSize(1);
// Requirement is now unsatisfied because -2 is the max negative value
assertSubmitRequirementStatus(
- change.submitRequirements, "code-review", Status.UNSATISFIED, /* isLegacy= */ false);
+ change.submitRequirements, "Code-Review", Status.UNSATISFIED, /* isLegacy= */ false);
}
@Test
@@ -4335,9 +4334,9 @@
configSubmitRequirement(
project,
SubmitRequirement.builder()
- .setName("code-review")
+ .setName("Code-Review")
.setSubmittabilityExpression(
- SubmitRequirementExpression.create("label:code-review=ANY"))
+ SubmitRequirementExpression.create("label:Code-Review=ANY"))
.setAllowOverrideInChildProjects(false)
.build());
@@ -4347,14 +4346,14 @@
ChangeInfo change = gApi.changes().id(changeId).get();
assertThat(change.submitRequirements).hasSize(1);
assertSubmitRequirementStatus(
- change.submitRequirements, "code-review", Status.UNSATISFIED, /* isLegacy= */ false);
+ change.submitRequirements, "Code-Review", Status.UNSATISFIED, /* isLegacy= */ false);
- voteLabel(changeId, "code-review", 1);
+ voteLabel(changeId, "Code-Review", 1);
change = gApi.changes().id(changeId).get();
assertThat(change.submitRequirements).hasSize(2);
// Legacy and non-legacy requirements have mismatching status. Both are returned from the API.
assertSubmitRequirementStatus(
- change.submitRequirements, "code-review", Status.SATISFIED, /* isLegacy= */ false);
+ change.submitRequirements, "Code-Review", Status.SATISFIED, /* isLegacy= */ false);
assertSubmitRequirementStatus(
change.submitRequirements, "Code-Review", Status.UNSATISFIED, /* isLegacy= */ true);
}
@@ -4368,15 +4367,15 @@
configSubmitRequirement(
project,
SubmitRequirement.builder()
- .setName("code-review")
- .setSubmittabilityExpression(SubmitRequirementExpression.create("label:code-review=+2"))
+ .setName("Code-Review")
+ .setSubmittabilityExpression(SubmitRequirementExpression.create("label:Code-Review=+2"))
.setAllowOverrideInChildProjects(false)
.build());
configSubmitRequirement(
project,
SubmitRequirement.builder()
- .setName("verified")
- .setSubmittabilityExpression(SubmitRequirementExpression.create("label:verified=+1"))
+ .setName("Verified")
+ .setSubmittabilityExpression(SubmitRequirementExpression.create("label:Verified=+1"))
.setAllowOverrideInChildProjects(false)
.build());
@@ -4386,18 +4385,18 @@
ChangeInfo change = gApi.changes().id(changeId).get();
assertThat(change.submitRequirements).hasSize(2);
assertSubmitRequirementStatus(
- change.submitRequirements, "code-review", Status.UNSATISFIED, /* isLegacy= */ false);
+ change.submitRequirements, "Code-Review", Status.UNSATISFIED, /* isLegacy= */ false);
assertSubmitRequirementStatus(
- change.submitRequirements, "verified", Status.UNSATISFIED, /* isLegacy= */ false);
+ change.submitRequirements, "Verified", Status.UNSATISFIED, /* isLegacy= */ false);
- voteLabel(changeId, "code-review", 2);
+ voteLabel(changeId, "Code-Review", 2);
change = gApi.changes().id(changeId).get();
assertThat(change.submitRequirements).hasSize(2);
assertSubmitRequirementStatus(
- change.submitRequirements, "code-review", Status.SATISFIED, /* isLegacy= */ false);
+ change.submitRequirements, "Code-Review", Status.SATISFIED, /* isLegacy= */ false);
assertSubmitRequirementStatus(
- change.submitRequirements, "verified", Status.UNSATISFIED, /* isLegacy= */ false);
+ change.submitRequirements, "Verified", Status.UNSATISFIED, /* isLegacy= */ false);
}
@Test
@@ -4409,9 +4408,9 @@
configSubmitRequirement(
project,
SubmitRequirement.builder()
- .setName("code-review")
+ .setName("Code-Review")
.setApplicabilityExpression(SubmitRequirementExpression.of("project:foo"))
- .setSubmittabilityExpression(SubmitRequirementExpression.create("label:code-review=+2"))
+ .setSubmittabilityExpression(SubmitRequirementExpression.create("label:Code-Review=+2"))
.setAllowOverrideInChildProjects(false)
.build());
@@ -4421,7 +4420,7 @@
ChangeInfo change = gApi.changes().id(changeId).get();
assertThat(change.submitRequirements).hasSize(2);
assertSubmitRequirementStatus(
- change.submitRequirements, "code-review", Status.NOT_APPLICABLE, /* isLegacy= */ false);
+ change.submitRequirements, "Code-Review", Status.NOT_APPLICABLE, /* isLegacy= */ false);
assertSubmitRequirementStatus(
change.submitRequirements, "Code-Review", Status.UNSATISFIED, /* isLegacy= */ true);
}
@@ -4445,8 +4444,8 @@
configSubmitRequirement(
project,
SubmitRequirement.builder()
- .setName("code-review")
- .setSubmittabilityExpression(SubmitRequirementExpression.create("label:code-review=+2"))
+ .setName("Code-Review")
+ .setSubmittabilityExpression(SubmitRequirementExpression.create("label:Code-Review=+2"))
.setOverrideExpression(SubmitRequirementExpression.of("label:build-cop-override=+1"))
.setAllowOverrideInChildProjects(false)
.build());
@@ -4456,36 +4455,38 @@
ChangeInfo change = gApi.changes().id(changeId).get();
assertThat(change.submitRequirements).hasSize(1);
assertSubmitRequirementStatus(
- change.submitRequirements, "code-review", Status.UNSATISFIED, /* isLegacy= */ false);
+ change.submitRequirements, "Code-Review", Status.UNSATISFIED, /* isLegacy= */ false);
voteLabel(changeId, "build-cop-override", 1);
change = gApi.changes().id(changeId).get();
assertThat(change.submitRequirements).hasSize(2);
assertSubmitRequirementStatus(
- change.submitRequirements, "code-review", Status.OVERRIDDEN, /* isLegacy= */ false);
+ change.submitRequirements, "Code-Review", Status.OVERRIDDEN, /* isLegacy= */ false);
assertSubmitRequirementStatus(
change.submitRequirements, "Code-Review", Status.UNSATISFIED, /* isLegacy= */ true);
}
@Test
- @Ignore("Test is flaky")
+ @GerritConfig(
+ name = "experiments.enabled",
+ value = ExperimentFeaturesConstants.GERRIT_BACKEND_REQUEST_FEATURE_ENABLE_SUBMIT_REQUIREMENTS)
public void submitRequirement_overriddenInChildProject() throws Exception {
configSubmitRequirement(
allProjects,
SubmitRequirement.builder()
- .setName("code-review")
- .setSubmittabilityExpression(SubmitRequirementExpression.create("label:code-review=+1"))
+ .setName("Code-Review")
+ .setSubmittabilityExpression(SubmitRequirementExpression.create("label:Code-Review=+1"))
.setOverrideExpression(SubmitRequirementExpression.of("label:build-cop-override=+1"))
.setAllowOverrideInChildProjects(true)
.build());
- // Override submit requirement in child project (requires code-review=+2 instead of +1)
+ // Override submit requirement in child project (requires Code-Review=+2 instead of +1)
configSubmitRequirement(
project,
SubmitRequirement.builder()
- .setName("code-review")
- .setSubmittabilityExpression(SubmitRequirementExpression.create("label:code-review=+2"))
+ .setName("Code-Review")
+ .setSubmittabilityExpression(SubmitRequirementExpression.create("label:Code-Review=+2"))
.setOverrideExpression(SubmitRequirementExpression.of("label:build-cop-override=+1"))
.setAllowOverrideInChildProjects(false)
.build());
@@ -4495,19 +4496,19 @@
ChangeInfo change = gApi.changes().id(changeId).get();
assertThat(change.submitRequirements).hasSize(1);
assertSubmitRequirementStatus(
- change.submitRequirements, "code-review", Status.UNSATISFIED, /* isLegacy= */ false);
+ change.submitRequirements, "Code-Review", Status.UNSATISFIED, /* isLegacy= */ false);
- voteLabel(changeId, "code-review", 1);
+ voteLabel(changeId, "Code-Review", 1);
change = gApi.changes().id(changeId).get();
assertThat(change.submitRequirements).hasSize(1);
assertSubmitRequirementStatus(
- change.submitRequirements, "code-review", Status.UNSATISFIED, /* isLegacy= */ false);
+ change.submitRequirements, "Code-Review", Status.UNSATISFIED, /* isLegacy= */ false);
- voteLabel(changeId, "code-review", 2);
+ voteLabel(changeId, "Code-Review", 2);
change = gApi.changes().id(changeId).get();
assertThat(change.submitRequirements).hasSize(1);
assertSubmitRequirementStatus(
- change.submitRequirements, "code-review", Status.SATISFIED, /* isLegacy= */ false);
+ change.submitRequirements, "Code-Review", Status.SATISFIED, /* isLegacy= */ false);
}
@Test
@@ -4518,8 +4519,8 @@
configSubmitRequirement(
allProjects,
SubmitRequirement.builder()
- .setName("code-review")
- .setSubmittabilityExpression(SubmitRequirementExpression.create("label:code-review=+1"))
+ .setName("Code-Review")
+ .setSubmittabilityExpression(SubmitRequirementExpression.create("label:Code-Review=+1"))
.setOverrideExpression(SubmitRequirementExpression.of("label:build-cop-override=+1"))
.setAllowOverrideInChildProjects(false)
.build());
@@ -4529,13 +4530,13 @@
ChangeInfo change = gApi.changes().id(changeId).get();
assertThat(change.submitRequirements).hasSize(1);
assertSubmitRequirementStatus(
- change.submitRequirements, "code-review", Status.UNSATISFIED, /* isLegacy= */ false);
+ change.submitRequirements, "Code-Review", Status.UNSATISFIED, /* isLegacy= */ false);
- voteLabel(changeId, "code-review", 1);
+ voteLabel(changeId, "Code-Review", 1);
change = gApi.changes().id(changeId).get();
assertThat(change.submitRequirements).hasSize(2);
assertSubmitRequirementStatus(
- change.submitRequirements, "code-review", Status.SATISFIED, /* isLegacy= */ false);
+ change.submitRequirements, "Code-Review", Status.SATISFIED, /* isLegacy= */ false);
// Legacy requirement is coming from the label MaxWithBlock function. Still unsatisfied.
assertSubmitRequirementStatus(
change.submitRequirements, "Code-Review", Status.UNSATISFIED, /* isLegacy= */ true);
@@ -4550,19 +4551,19 @@
configSubmitRequirement(
allProjects,
SubmitRequirement.builder()
- .setName("code-review")
- .setSubmittabilityExpression(SubmitRequirementExpression.create("label:code-review=+1"))
+ .setName("Code-Review")
+ .setSubmittabilityExpression(SubmitRequirementExpression.create("label:Code-Review=+1"))
.setOverrideExpression(SubmitRequirementExpression.of("label:build-cop-override=+1"))
.setAllowOverrideInChildProjects(false)
.build());
- // Override submit requirement in child project (requires code-review=+2 instead of +1).
+ // Override submit requirement in child project (requires Code-Review=+2 instead of +1).
// Will have no effect since parent does not allow override.
configSubmitRequirement(
project,
SubmitRequirement.builder()
- .setName("code-review")
- .setSubmittabilityExpression(SubmitRequirementExpression.create("label:code-review=+2"))
+ .setName("Code-Review")
+ .setSubmittabilityExpression(SubmitRequirementExpression.create("label:Code-Review=+2"))
.setOverrideExpression(SubmitRequirementExpression.of("label:build-cop-override=+1"))
.setAllowOverrideInChildProjects(false)
.build());
@@ -4572,14 +4573,14 @@
ChangeInfo change = gApi.changes().id(changeId).get();
assertThat(change.submitRequirements).hasSize(1);
assertSubmitRequirementStatus(
- change.submitRequirements, "code-review", Status.UNSATISFIED, /* isLegacy= */ false);
+ change.submitRequirements, "Code-Review", Status.UNSATISFIED, /* isLegacy= */ false);
- voteLabel(changeId, "code-review", 1);
+ voteLabel(changeId, "Code-Review", 1);
change = gApi.changes().id(changeId).get();
assertThat(change.submitRequirements).hasSize(2);
// +1 was enough to fulfill the requirement: override in child project was ignored
assertSubmitRequirementStatus(
- change.submitRequirements, "code-review", Status.SATISFIED, /* isLegacy= */ false);
+ change.submitRequirements, "Code-Review", Status.SATISFIED, /* isLegacy= */ false);
// Legacy requirement is coming from the label MaxWithBlock function. Still unsatisfied.
assertSubmitRequirementStatus(
change.submitRequirements, "Code-Review", Status.UNSATISFIED, /* isLegacy= */ true);
@@ -4600,9 +4601,9 @@
configSubmitRequirement(
project,
SubmitRequirement.builder()
- .setName("code-review")
+ .setName("Code-Review")
.setSubmittabilityExpression(
- SubmitRequirementExpression.create("label:code-review=+2"))
+ SubmitRequirementExpression.create("label:Code-Review=+2"))
.setAllowOverrideInChildProjects(false)
.build());
@@ -4610,12 +4611,12 @@
createChange(repo, "master", "Add a file", "foo", "content", "topic");
String changeId = r.getChangeId();
- voteLabel(changeId, "code-review", 2);
+ voteLabel(changeId, "Code-Review", 2);
ChangeInfo change = gApi.changes().id(changeId).get();
assertThat(change.submitRequirements).hasSize(1);
assertSubmitRequirementStatus(
- change.submitRequirements, "code-review", Status.SATISFIED, /* isLegacy= */ false);
+ change.submitRequirements, "Code-Review", Status.SATISFIED, /* isLegacy= */ false);
RevisionApi revision = gApi.changes().id(r.getChangeId()).current();
revision.review(ReviewInput.approve());
@@ -4629,7 +4630,7 @@
assertThat(result.submittabilityExpressionResult().status())
.isEqualTo(SubmitRequirementExpressionResult.Status.PASS);
assertThat(result.submittabilityExpressionResult().expression().expressionString())
- .isEqualTo("label:code-review=+2");
+ .isEqualTo("label:Code-Review=+2");
}
}
@@ -4645,8 +4646,8 @@
configSubmitRequirement(
project,
SubmitRequirement.builder()
- .setName("code-review")
- .setSubmittabilityExpression(SubmitRequirementExpression.create("label:code-review=+2"))
+ .setName("Code-Review")
+ .setSubmittabilityExpression(SubmitRequirementExpression.create("label:Code-Review=+2"))
.setAllowOverrideInChildProjects(false)
.build());
@@ -4656,14 +4657,14 @@
ChangeInfo change = gApi.changes().id(changeId).get();
assertThat(change.submitRequirements).hasSize(1);
assertSubmitRequirementStatus(
- change.submitRequirements, "code-review", Status.UNSATISFIED, /* isLegacy= */ false);
+ change.submitRequirements, "Code-Review", Status.UNSATISFIED, /* isLegacy= */ false);
- voteLabel(changeId, "code-review", 2);
+ voteLabel(changeId, "Code-Review", 2);
change = gApi.changes().id(changeId).get();
assertThat(change.submitRequirements).hasSize(1);
assertSubmitRequirementStatus(
- change.submitRequirements, "code-review", Status.SATISFIED, /* isLegacy= */ false);
+ change.submitRequirements, "Code-Review", Status.SATISFIED, /* isLegacy= */ false);
gApi.changes().id(changeId).current().submit();
@@ -4671,16 +4672,16 @@
configSubmitRequirement(
project,
SubmitRequirement.builder()
- .setName("verified")
- .setSubmittabilityExpression(SubmitRequirementExpression.create("label:verified=+1"))
+ .setName("Verified")
+ .setSubmittabilityExpression(SubmitRequirementExpression.create("label:Verified=+1"))
.setAllowOverrideInChildProjects(false)
.build());
- // The new "verified" submit requirement is not returned, since this change is closed
+ // The new "Verified" submit requirement is not returned, since this change is closed
change = gApi.changes().id(changeId).get();
assertThat(change.submitRequirements).hasSize(1);
assertSubmitRequirementStatus(
- change.submitRequirements, "code-review", Status.SATISFIED, /* isLegacy= */ false);
+ change.submitRequirements, "Code-Review", Status.SATISFIED, /* isLegacy= */ false);
}
@Test
@@ -4869,15 +4870,15 @@
configSubmitRequirement(
project,
SubmitRequirement.builder()
- .setName("code-review")
- .setSubmittabilityExpression(SubmitRequirementExpression.create("label:code-review=+2"))
+ .setName("Code-Review")
+ .setSubmittabilityExpression(SubmitRequirementExpression.create("label:Code-Review=+2"))
.setAllowOverrideInChildProjects(false)
.build());
PushOneCommit.Result r = createChange();
String changeId = r.getChangeId();
- voteLabel(changeId, "code-review", 2);
+ voteLabel(changeId, "Code-Review", 2);
// Query the change. ChangeInfo is back-filled from the change index.
List<ChangeInfo> changeInfos =
@@ -4889,7 +4890,7 @@
assertThat(changeInfos).hasSize(1);
assertSubmitRequirementStatus(
changeInfos.get(0).submitRequirements,
- "code-review",
+ "Code-Review",
Status.SATISFIED,
/* isLegacy= */ false);
}
@@ -4906,15 +4907,15 @@
configSubmitRequirement(
project,
SubmitRequirement.builder()
- .setName("code-review")
- .setSubmittabilityExpression(SubmitRequirementExpression.create("label:code-review=+2"))
+ .setName("Code-Review")
+ .setSubmittabilityExpression(SubmitRequirementExpression.create("label:Code-Review=+2"))
.setAllowOverrideInChildProjects(false)
.build());
PushOneCommit.Result r = createChange();
String changeId = r.getChangeId();
- voteLabel(changeId, "code-review", 2);
+ voteLabel(changeId, "Code-Review", 2);
gApi.changes().id(changeId).current().submit();
// Query the change. ChangeInfo is back-filled from the change index.
@@ -4927,7 +4928,7 @@
assertThat(changeInfos).hasSize(1);
assertSubmitRequirementStatus(
changeInfos.get(0).submitRequirements,
- "code-review",
+ "Code-Review",
Status.SATISFIED,
/* isLegacy= */ false);
}
@@ -4937,8 +4938,8 @@
configSubmitRequirement(
project,
SubmitRequirement.builder()
- .setName("code-review")
- .setSubmittabilityExpression(SubmitRequirementExpression.create("label:code-review=+2"))
+ .setName("Code-Review")
+ .setSubmittabilityExpression(SubmitRequirementExpression.create("label:Code-Review=+2"))
.setAllowOverrideInChildProjects(false)
.build());
@@ -4948,11 +4949,11 @@
ChangeInfo change = gApi.changes().id(changeId).get();
assertThat(change.submitRequirements).isEmpty();
- voteLabel(changeId, "code-review", -1);
+ voteLabel(changeId, "Code-Review", -1);
change = gApi.changes().id(changeId).get();
assertThat(change.submitRequirements).isEmpty();
- voteLabel(changeId, "code-review", 2);
+ voteLabel(changeId, "Code-Review", 2);
change = gApi.changes().id(changeId).get();
assertThat(change.submitRequirements).isEmpty();
diff --git a/javatests/com/google/gerrit/acceptance/rest/CancellationIT.java b/javatests/com/google/gerrit/acceptance/rest/CancellationIT.java
index ed5e559..c868d0b 100644
--- a/javatests/com/google/gerrit/acceptance/rest/CancellationIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/CancellationIT.java
@@ -35,6 +35,7 @@
import com.google.gerrit.server.validators.ProjectCreationValidationListener;
import com.google.gerrit.server.validators.ValidationException;
import com.google.inject.Inject;
+import java.time.Duration;
import java.util.ArrayList;
import java.util.List;
import org.apache.http.message.BasicHeader;
@@ -194,6 +195,7 @@
@GerritConfig(name = "deadline.default.timeout", value = "1ms")
public void abortIfServerDeadlineExceeded() throws Exception {
+ testTicker.useFakeTicker().setAutoIncrementStep(Duration.ofMillis(2));
RestResponse response = adminRestSession.putWithHeaders("/projects/" + name("new"));
assertThat(response.getStatusCode()).isEqualTo(SC_REQUEST_TIMEOUT);
assertThat(response.getEntityContent()).isEqualTo("Server Deadline Exceeded\n\ntimeout=1ms");
@@ -203,6 +205,7 @@
@GerritConfig(name = "deadline.foo.timeout", value = "1ms")
@GerritConfig(name = "deadline.bar.timeout", value = "100ms")
public void stricterDeadlineTakesPrecedence() throws Exception {
+ testTicker.useFakeTicker().setAutoIncrementStep(Duration.ofMillis(2));
RestResponse response = adminRestSession.putWithHeaders("/projects/" + name("new"));
assertThat(response.getStatusCode()).isEqualTo(SC_REQUEST_TIMEOUT);
assertThat(response.getEntityContent())
@@ -213,6 +216,7 @@
@GerritConfig(name = "deadline.default.timeout", value = "1ms")
@GerritConfig(name = "deadline.default.requestType", value = "REST")
public void abortIfServerDeadlineExceeded_requestType() throws Exception {
+ testTicker.useFakeTicker().setAutoIncrementStep(Duration.ofMillis(2));
RestResponse response = adminRestSession.putWithHeaders("/projects/" + name("new"));
assertThat(response.getStatusCode()).isEqualTo(SC_REQUEST_TIMEOUT);
assertThat(response.getEntityContent())
@@ -223,6 +227,7 @@
@GerritConfig(name = "deadline.default.timeout", value = "1ms")
@GerritConfig(name = "deadline.default.requestUriPattern", value = "/projects/.*")
public void abortIfServerDeadlineExceeded_requestUriPattern() throws Exception {
+ testTicker.useFakeTicker().setAutoIncrementStep(Duration.ofMillis(2));
RestResponse response = adminRestSession.putWithHeaders("/projects/" + name("new"));
assertThat(response.getStatusCode()).isEqualTo(SC_REQUEST_TIMEOUT);
assertThat(response.getEntityContent())
@@ -235,6 +240,7 @@
name = "deadline.default.excludedRequestUriPattern",
value = "/projects/non-matching")
public void abortIfServerDeadlineExceeded_excludedRequestUriPattern() throws Exception {
+ testTicker.useFakeTicker().setAutoIncrementStep(Duration.ofMillis(2));
RestResponse response = adminRestSession.putWithHeaders("/projects/" + name("new"));
assertThat(response.getStatusCode()).isEqualTo(SC_REQUEST_TIMEOUT);
assertThat(response.getEntityContent())
@@ -249,6 +255,7 @@
value = "/projects/non-matching")
public void abortIfServerDeadlineExceeded_requestUriPatternAndExcludedRequestUriPattern()
throws Exception {
+ testTicker.useFakeTicker().setAutoIncrementStep(Duration.ofMillis(2));
RestResponse response = adminRestSession.putWithHeaders("/projects/" + name("new"));
assertThat(response.getStatusCode()).isEqualTo(SC_REQUEST_TIMEOUT);
assertThat(response.getEntityContent())
@@ -259,6 +266,7 @@
@GerritConfig(name = "deadline.default.timeout", value = "1ms")
@GerritConfig(name = "deadline.default.projectPattern", value = ".*new.*")
public void abortIfServerDeadlineExceeded_projectPattern() throws Exception {
+ testTicker.useFakeTicker().setAutoIncrementStep(Duration.ofMillis(2));
RestResponse response = adminRestSession.putWithHeaders("/projects/" + name("new"));
assertThat(response.getStatusCode()).isEqualTo(SC_REQUEST_TIMEOUT);
assertThat(response.getEntityContent())
@@ -269,6 +277,7 @@
@GerritConfig(name = "deadline.default.timeout", value = "1ms")
@GerritConfig(name = "deadline.default.account", value = "1000000")
public void abortIfServerDeadlineExceeded_account() throws Exception {
+ testTicker.useFakeTicker().setAutoIncrementStep(Duration.ofMillis(2));
RestResponse response = adminRestSession.putWithHeaders("/projects/" + name("new"));
assertThat(response.getStatusCode()).isEqualTo(SC_REQUEST_TIMEOUT);
assertThat(response.getEntityContent())
@@ -279,6 +288,7 @@
@GerritConfig(name = "deadline.default.timeout", value = "1ms")
@GerritConfig(name = "deadline.default.requestType", value = "SSH")
public void nonMatchingServerDeadlineIsIgnored_requestType() throws Exception {
+ testTicker.useFakeTicker().setAutoIncrementStep(Duration.ofMillis(2));
RestResponse response = adminRestSession.putWithHeaders("/projects/" + name("new"));
response.assertCreated();
}
@@ -287,6 +297,7 @@
@GerritConfig(name = "deadline.default.timeout", value = "1ms")
@GerritConfig(name = "deadline.default.requestUriPattern", value = "/changes/.*")
public void nonMatchingServerDeadlineIsIgnored_requestUriPattern() throws Exception {
+ testTicker.useFakeTicker().setAutoIncrementStep(Duration.ofMillis(2));
RestResponse response = adminRestSession.putWithHeaders("/projects/" + name("new"));
response.assertCreated();
}
@@ -295,6 +306,7 @@
@GerritConfig(name = "deadline.default.timeout", value = "1ms")
@GerritConfig(name = "deadline.default.excludedRequestUriPattern", value = "/projects/.*")
public void nonMatchingServerDeadlineIsIgnored_excludedRequestUriPattern() throws Exception {
+ testTicker.useFakeTicker().setAutoIncrementStep(Duration.ofMillis(2));
RestResponse response = adminRestSession.putWithHeaders("/projects/" + name("new"));
response.assertCreated();
}
@@ -305,6 +317,7 @@
@GerritConfig(name = "deadline.default.excludedRequestUriPattern", value = "/projects/.*new")
public void nonMatchingServerDeadlineIsIgnored_requestUriPatternAndExcludedRequestUriPattern()
throws Exception {
+ testTicker.useFakeTicker().setAutoIncrementStep(Duration.ofMillis(2));
RestResponse response = adminRestSession.putWithHeaders("/projects/" + name("new"));
response.assertCreated();
}
@@ -313,6 +326,7 @@
@GerritConfig(name = "deadline.default.timeout", value = "1ms")
@GerritConfig(name = "deadline.default.projectPattern", value = ".*foo.*")
public void nonMatchingServerDeadlineIsIgnored_projectPattern() throws Exception {
+ testTicker.useFakeTicker().setAutoIncrementStep(Duration.ofMillis(2));
RestResponse response = adminRestSession.putWithHeaders("/projects/" + name("new"));
response.assertCreated();
}
@@ -321,6 +335,7 @@
@GerritConfig(name = "deadline.default.timeout", value = "1ms")
@GerritConfig(name = "deadline.default.account", value = "999")
public void nonMatchingServerDeadlineIsIgnored_account() throws Exception {
+ testTicker.useFakeTicker().setAutoIncrementStep(Duration.ofMillis(2));
RestResponse response = adminRestSession.putWithHeaders("/projects/" + name("new"));
response.assertCreated();
}
@@ -329,6 +344,7 @@
@GerritConfig(name = "deadline.default.timeout", value = "1ms")
@GerritConfig(name = "deadline.default.isAdvisory", value = "true")
public void advisoryServerDeadlineIsIgnored() throws Exception {
+ testTicker.useFakeTicker().setAutoIncrementStep(Duration.ofMillis(2));
RestResponse response = adminRestSession.putWithHeaders("/projects/" + name("new"));
response.assertCreated();
}
@@ -338,6 +354,7 @@
@GerritConfig(name = "deadline.test.isAdvisory", value = "true")
@GerritConfig(name = "deadline.default.timeout", value = "2ms")
public void nonAdvisoryDeadlineIsAppliedIfStricterAdvisoryDeadlineExists() throws Exception {
+ testTicker.useFakeTicker().setAutoIncrementStep(Duration.ofMillis(4));
RestResponse response = adminRestSession.putWithHeaders("/projects/" + name("new"));
assertThat(response.getStatusCode()).isEqualTo(SC_REQUEST_TIMEOUT);
assertThat(response.getEntityContent())
@@ -347,6 +364,7 @@
@Test
@GerritConfig(name = "deadline.default.timeout", value = "1")
public void invalidServerDeadlineIsIgnored_missingTimeUnit() throws Exception {
+ testTicker.useFakeTicker().setAutoIncrementStep(Duration.ofMillis(2));
RestResponse response = adminRestSession.putWithHeaders("/projects/" + name("new"));
response.assertCreated();
}
@@ -354,6 +372,7 @@
@Test
@GerritConfig(name = "deadline.default.timeout", value = "1x")
public void invalidServerDeadlineIsIgnored_invalidTimeUnit() throws Exception {
+ testTicker.useFakeTicker().setAutoIncrementStep(Duration.ofMillis(2));
RestResponse response = adminRestSession.putWithHeaders("/projects/" + name("new"));
response.assertCreated();
}
@@ -369,6 +388,7 @@
@GerritConfig(name = "deadline.default.timeout", value = "1ms")
@GerritConfig(name = "deadline.default.requestType", value = "INVALID")
public void invalidServerDeadlineIsIgnored_invalidRequestType() throws Exception {
+ testTicker.useFakeTicker().setAutoIncrementStep(Duration.ofMillis(2));
RestResponse response = adminRestSession.putWithHeaders("/projects/" + name("new"));
response.assertCreated();
}
@@ -377,6 +397,7 @@
@GerritConfig(name = "deadline.default.timeout", value = "1ms")
@GerritConfig(name = "deadline.default.requestUriPattern", value = "][")
public void invalidServerDeadlineIsIgnored_invalidRequestUriPattern() throws Exception {
+ testTicker.useFakeTicker().setAutoIncrementStep(Duration.ofMillis(2));
RestResponse response = adminRestSession.putWithHeaders("/projects/" + name("new"));
response.assertCreated();
}
@@ -385,6 +406,7 @@
@GerritConfig(name = "deadline.default.timeout", value = "1ms")
@GerritConfig(name = "deadline.default.excludedRequestUriPattern", value = "][")
public void invalidServerDeadlineIsIgnored_invalidExcludedRequestUriPattern() throws Exception {
+ testTicker.useFakeTicker().setAutoIncrementStep(Duration.ofMillis(2));
RestResponse response = adminRestSession.putWithHeaders("/projects/" + name("new"));
response.assertCreated();
}
@@ -393,6 +415,7 @@
@GerritConfig(name = "deadline.default.timeout", value = "1ms")
@GerritConfig(name = "deadline.default.projectPattern", value = "][")
public void invalidServerDeadlineIsIgnored_invalidProjectPattern() throws Exception {
+ testTicker.useFakeTicker().setAutoIncrementStep(Duration.ofMillis(2));
RestResponse response = adminRestSession.putWithHeaders("/projects/" + name("new"));
response.assertCreated();
}
@@ -401,6 +424,7 @@
@GerritConfig(name = "deadline.default.timeout", value = "1ms")
@GerritConfig(name = "deadline.default.account", value = "invalid")
public void invalidServerDeadlineIsIgnored_invalidAccount() throws Exception {
+ testTicker.useFakeTicker().setAutoIncrementStep(Duration.ofMillis(2));
RestResponse response = adminRestSession.putWithHeaders("/projects/" + name("new"));
response.assertCreated();
}
@@ -416,6 +440,7 @@
@GerritConfig(name = "deadline.default.timeout", value = "0ms")
@GerritConfig(name = "deadline.default.requestType", value = "REST")
public void deadlineConfigWithZeroTimeoutIsIgnored() throws Exception {
+ testTicker.useFakeTicker().setAutoIncrementStep(Duration.ofMillis(2));
RestResponse response = adminRestSession.putWithHeaders("/projects/" + name("new"));
response.assertCreated();
}
@@ -449,6 +474,7 @@
@Test
@GerritConfig(name = "deadline.default.timeout", value = "1ms")
public void clientProvidedDeadlineOverridesServerDeadline() throws Exception {
+ testTicker.useFakeTicker().setAutoIncrementStep(Duration.ofMillis(2));
RestResponse response =
adminRestSession.putWithHeaders(
"/projects/" + name("new"), new BasicHeader(RestApiServlet.X_GERRIT_DEADLINE, "2ms"));
@@ -460,6 +486,7 @@
@Test
@GerritConfig(name = "deadline.default.timeout", value = "1ms")
public void clientCanDisableDeadlineBySettingZeroAsDeadline() throws Exception {
+ testTicker.useFakeTicker().setAutoIncrementStep(Duration.ofMillis(2));
RestResponse response =
adminRestSession.putWithHeaders(
"/projects/" + name("new"), new BasicHeader(RestApiServlet.X_GERRIT_DEADLINE, "0"));
@@ -574,6 +601,7 @@
@Test
@GerritConfig(name = "deadline.default.timeout", value = "1ms")
public void abortPushIfServerDeadlineExceeded() throws Exception {
+ testTicker.useFakeTicker().setAutoIncrementStep(Duration.ofMillis(2));
PushOneCommit push = pushFactory.create(admin.newIdent(), testRepo);
PushOneCommit.Result r = push.to("refs/for/master");
r.assertErrorStatus("Server Deadline Exceeded (default.timeout=1ms)");
@@ -582,6 +610,7 @@
@Test
@GerritConfig(name = "receive.timeout", value = "1ms")
public void abortPushIfTimeoutExceeded() throws Exception {
+ testTicker.useFakeTicker().setAutoIncrementStep(Duration.ofMillis(2));
PushOneCommit push = pushFactory.create(admin.newIdent(), testRepo);
PushOneCommit.Result r = push.to("refs/for/master");
r.assertErrorStatus("Server Deadline Exceeded (receive.timeout=1ms)");
@@ -591,6 +620,7 @@
@GerritConfig(name = "receive.timeout", value = "1ms")
@GerritConfig(name = "deadline.default.timeout", value = "10s")
public void receiveTimeoutTakesPrecedence() throws Exception {
+ testTicker.useFakeTicker().setAutoIncrementStep(Duration.ofMillis(2));
PushOneCommit push = pushFactory.create(admin.newIdent(), testRepo);
PushOneCommit.Result r = push.to("refs/for/master");
r.assertErrorStatus("Server Deadline Exceeded (receive.timeout=1ms)");
@@ -649,6 +679,7 @@
@Test
@GerritConfig(name = "deadline.default.timeout", value = "1ms")
public void clientProvidedDeadlineOnPushOverridesServerDeadline() throws Exception {
+ testTicker.useFakeTicker().setAutoIncrementStep(Duration.ofMillis(2));
List<String> pushOptions = new ArrayList<>();
pushOptions.add("deadline=2ms");
PushOneCommit push = pushFactory.create(admin.newIdent(), testRepo);
@@ -660,6 +691,7 @@
@Test
@GerritConfig(name = "receive.timeout", value = "1ms")
public void clientProvidedDeadlineOnPushDoesntOverrideServerTimeout() throws Exception {
+ testTicker.useFakeTicker().setAutoIncrementStep(Duration.ofMillis(2));
List<String> pushOptions = new ArrayList<>();
pushOptions.add("deadline=10m");
PushOneCommit push = pushFactory.create(admin.newIdent(), testRepo);
@@ -671,6 +703,7 @@
@Test
@GerritConfig(name = "deadline.default.timeout", value = "1ms")
public void clientCanDisableDeadlineOnPushBySettingZeroAsDeadline() throws Exception {
+ testTicker.useFakeTicker().setAutoIncrementStep(Duration.ofMillis(2));
List<String> pushOptions = new ArrayList<>();
pushOptions.add("deadline=0");
PushOneCommit push = pushFactory.create(admin.newIdent(), testRepo);
diff --git a/javatests/com/google/gerrit/acceptance/server/project/SubmitRequirementsEvaluatorIT.java b/javatests/com/google/gerrit/acceptance/server/project/SubmitRequirementsEvaluatorIT.java
index 6e19c39..f511683 100644
--- a/javatests/com/google/gerrit/acceptance/server/project/SubmitRequirementsEvaluatorIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/project/SubmitRequirementsEvaluatorIT.java
@@ -138,13 +138,13 @@
SubmitRequirement sr =
createSubmitRequirement(
/* applicabilityExpr= */ "project:" + project.get(),
- /* submittabilityExpr= */ "label:\"code-review=+2\"",
+ /* submittabilityExpr= */ "label:\"Code-Review=+2\"",
/* overrideExpr= */ "");
SubmitRequirementResult result = evaluator.evaluateRequirement(sr, changeData);
assertThat(result.status()).isEqualTo(SubmitRequirementResult.Status.UNSATISFIED);
assertThat(result.submittabilityExpressionResult().failingAtoms())
- .containsExactly("label:\"code-review=+2\"");
+ .containsExactly("label:\"Code-Review=+2\"");
}
@Test
@@ -160,7 +160,7 @@
SubmitRequirement sr =
createSubmitRequirement(
/* applicabilityExpr= */ "project:" + project.get(),
- /* submittabilityExpr= */ "label:\"code-review=+2\"",
+ /* submittabilityExpr= */ "label:\"Code-Review=+2\"",
/* overrideExpr= */ "label:\"build-cop-override=+1\"");
SubmitRequirementResult result = evaluator.evaluateRequirement(sr, changeData);
@@ -175,7 +175,7 @@
SubmitRequirement sr =
createSubmitRequirement(
/* applicabilityExpr= */ "invalid_field:invalid_value",
- /* submittabilityExpr= */ "label:\"code-review=+2\"",
+ /* submittabilityExpr= */ "label:\"Code-Review=+2\"",
/* overrideExpr= */ "label:\"build-cop-override=+1\"");
SubmitRequirementResult result = evaluator.evaluateRequirement(sr, changeData);
@@ -206,7 +206,7 @@
SubmitRequirement sr =
createSubmitRequirement(
/* applicabilityExpr= */ "project:" + project.get(),
- /* submittabilityExpr= */ "label:\"code-review=+2\"",
+ /* submittabilityExpr= */ "label:\"Code-Review=+2\"",
/* overrideExpr= */ "invalid_field:invalid_value");
SubmitRequirementResult result = evaluator.evaluateRequirement(sr, changeData);
diff --git a/javatests/com/google/gerrit/acceptance/server/project/SubmitRequirementsValidationIT.java b/javatests/com/google/gerrit/acceptance/server/project/SubmitRequirementsValidationIT.java
index 4675bc0..d8aa789 100644
--- a/javatests/com/google/gerrit/acceptance/server/project/SubmitRequirementsValidationIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/project/SubmitRequirementsValidationIT.java
@@ -50,7 +50,7 @@
ProjectConfig.SUBMIT_REQUIREMENT,
/* subsection= */ submitRequirementName,
/* name= */ ProjectConfig.KEY_SR_SUBMITTABILITY_EXPRESSION,
- /* value= */ "label:\"code-review=+2\""));
+ /* value= */ "label:\"Code-Review=+2\""));
PushResult r = pushRefsMetaConfig();
assertOkStatus(r);
@@ -77,7 +77,7 @@
ProjectConfig.SUBMIT_REQUIREMENT,
/* subsection= */ submitRequirementName,
/* name= */ ProjectConfig.KEY_SR_SUBMITTABILITY_EXPRESSION,
- /* value= */ "label:\"code-review=+2\"");
+ /* value= */ "label:\"Code-Review=+2\"");
projectConfig.setString(
ProjectConfig.SUBMIT_REQUIREMENT,
/* subsection= */ submitRequirementName,
@@ -95,6 +95,78 @@
}
@Test
+ public void parametersDirectlyInSubmitRequirementsSectionAreRejected() throws Exception {
+ fetchRefsMetaConfig();
+
+ updateProjectConfig(
+ projectConfig -> {
+ projectConfig.setString(
+ ProjectConfig.SUBMIT_REQUIREMENT,
+ /* subsection= */ null,
+ /* name= */ ProjectConfig.KEY_SR_DESCRIPTION,
+ /* value= */ "foo bar description");
+ projectConfig.setString(
+ ProjectConfig.SUBMIT_REQUIREMENT,
+ /* subsection= */ null,
+ /* name= */ ProjectConfig.KEY_SR_SUBMITTABILITY_EXPRESSION,
+ /* value= */ "label:\"Code-Review=+2\"");
+ });
+
+ PushResult r = pushRefsMetaConfig();
+ assertErrorStatus(
+ r,
+ "Invalid project configuration",
+ String.format(
+ "project.config: Submit requirements must be defined in submit-requirement.<name>"
+ + " subsections. Setting parameters directly in the submit-requirement section is"
+ + " not allowed: [%s, %s]",
+ ProjectConfig.KEY_SR_DESCRIPTION, ProjectConfig.KEY_SR_SUBMITTABILITY_EXPRESSION));
+ }
+
+ @Test
+ public void unsupportedParameterDirectlyInSubmitRequirementsSectionIsRejected() throws Exception {
+ fetchRefsMetaConfig();
+
+ updateProjectConfig(
+ projectConfig ->
+ projectConfig.setString(
+ ProjectConfig.SUBMIT_REQUIREMENT,
+ /* subsection= */ null,
+ /* name= */ "unknown",
+ /* value= */ "value"));
+
+ PushResult r = pushRefsMetaConfig();
+ assertErrorStatus(
+ r,
+ "Invalid project configuration",
+ "project.config: Submit requirements must be defined in submit-requirement.<name>"
+ + " subsections. Setting parameters directly in the submit-requirement section is"
+ + " not allowed: [unknown]");
+ }
+
+ @Test
+ public void unsupportedParameterForSubmitRequirementIsRejected() throws Exception {
+ fetchRefsMetaConfig();
+
+ String submitRequirementName = "Code-Review";
+ updateProjectConfig(
+ projectConfig ->
+ projectConfig.setString(
+ ProjectConfig.SUBMIT_REQUIREMENT,
+ /* subsection= */ submitRequirementName,
+ /* name= */ "unknown",
+ /* value= */ "value"));
+
+ PushResult r = pushRefsMetaConfig();
+ assertErrorStatus(
+ r,
+ "Invalid project configuration",
+ String.format(
+ "project.config: Unsupported parameters for submit requirement '%s': [unknown]",
+ submitRequirementName));
+ }
+
+ @Test
public void conflictingSubmitRequirementsAreRejected() throws Exception {
fetchRefsMetaConfig();
@@ -105,12 +177,12 @@
ProjectConfig.SUBMIT_REQUIREMENT,
/* subsection= */ submitRequirementName,
/* name= */ ProjectConfig.KEY_SR_SUBMITTABILITY_EXPRESSION,
- /* value= */ "label:\"code-review=+2\"");
+ /* value= */ "label:\"Code-Review=+2\"");
projectConfig.setString(
ProjectConfig.SUBMIT_REQUIREMENT,
/* subsection= */ submitRequirementName.toLowerCase(Locale.US),
/* name= */ ProjectConfig.KEY_SR_SUBMITTABILITY_EXPRESSION,
- /* value= */ "label:\"code-review=+2\"");
+ /* value= */ "label:\"Code-Review=+2\"");
});
PushResult r = pushRefsMetaConfig();
@@ -132,7 +204,7 @@
ProjectConfig.SUBMIT_REQUIREMENT,
/* subsection= */ submitRequirementName,
/* name= */ ProjectConfig.KEY_SR_SUBMITTABILITY_EXPRESSION,
- /* value= */ "label:\"code-review=+2\""));
+ /* value= */ "label:\"Code-Review=+2\""));
PushResult r = pushRefsMetaConfig();
assertOkStatus(r);
@@ -142,7 +214,7 @@
ProjectConfig.SUBMIT_REQUIREMENT,
/* subsection= */ submitRequirementName.toLowerCase(Locale.US),
/* name= */ ProjectConfig.KEY_SR_SUBMITTABILITY_EXPRESSION,
- /* value= */ "label:\"code-review=+2\""));
+ /* value= */ "label:\"Code-Review=+2\""));
r = pushRefsMetaConfig();
assertErrorStatus(
r,
@@ -170,8 +242,139 @@
r,
"Invalid project configuration",
String.format(
- "project.config: Submit requirement '%s' does not define a submittability expression.",
- submitRequirementName));
+ "project.config: Setting a submittability expression for submit requirement '%s' is"
+ + " required: Missing %s.%s.%s",
+ submitRequirementName,
+ ProjectConfig.SUBMIT_REQUIREMENT,
+ submitRequirementName,
+ ProjectConfig.KEY_SR_SUBMITTABILITY_EXPRESSION));
+ }
+
+ @Test
+ public void submitRequirementWithInvalidSubmittabilityExpressionIsRejected() throws Exception {
+ fetchRefsMetaConfig();
+
+ String submitRequirementName = "Code-Review";
+ String invalidExpression = "invalid_field:invalid_value";
+ updateProjectConfig(
+ projectConfig ->
+ projectConfig.setString(
+ ProjectConfig.SUBMIT_REQUIREMENT,
+ /* subsection= */ submitRequirementName,
+ /* name= */ ProjectConfig.KEY_SR_SUBMITTABILITY_EXPRESSION,
+ /* value= */ invalidExpression));
+
+ PushResult r = pushRefsMetaConfig();
+ assertErrorStatus(
+ r,
+ "Invalid project configuration",
+ String.format(
+ "project.config: Expression '%s' of submit requirement '%s' (parameter %s.%s.%s) is"
+ + " invalid: Unsupported operator %s",
+ invalidExpression,
+ submitRequirementName,
+ ProjectConfig.SUBMIT_REQUIREMENT,
+ submitRequirementName,
+ ProjectConfig.KEY_SR_SUBMITTABILITY_EXPRESSION,
+ invalidExpression));
+ }
+
+ @Test
+ public void submitRequirementWithInvalidApplicabilityExpressionIsRejected() throws Exception {
+ fetchRefsMetaConfig();
+
+ String submitRequirementName = "Code-Review";
+ String invalidExpression = "invalid_field:invalid_value";
+ updateProjectConfig(
+ projectConfig -> {
+ projectConfig.setString(
+ ProjectConfig.SUBMIT_REQUIREMENT,
+ /* subsection= */ submitRequirementName,
+ /* name= */ ProjectConfig.KEY_SR_SUBMITTABILITY_EXPRESSION,
+ /* value= */ "label:\"Code-Review=+2\"");
+ projectConfig.setString(
+ ProjectConfig.SUBMIT_REQUIREMENT,
+ /* subsection= */ submitRequirementName,
+ /* name= */ ProjectConfig.KEY_SR_APPLICABILITY_EXPRESSION,
+ /* value= */ invalidExpression);
+ });
+
+ PushResult r = pushRefsMetaConfig();
+ assertErrorStatus(
+ r,
+ "Invalid project configuration",
+ String.format(
+ "project.config: Expression '%s' of submit requirement '%s' (parameter %s.%s.%s) is"
+ + " invalid: Unsupported operator %s",
+ invalidExpression,
+ submitRequirementName,
+ ProjectConfig.SUBMIT_REQUIREMENT,
+ submitRequirementName,
+ ProjectConfig.KEY_SR_APPLICABILITY_EXPRESSION,
+ invalidExpression));
+ }
+
+ @Test
+ public void submitRequirementWithInvalidOverrideExpressionIsRejected() throws Exception {
+ fetchRefsMetaConfig();
+
+ String submitRequirementName = "Code-Review";
+ String invalidExpression = "invalid_field:invalid_value";
+ updateProjectConfig(
+ projectConfig -> {
+ projectConfig.setString(
+ ProjectConfig.SUBMIT_REQUIREMENT,
+ /* subsection= */ submitRequirementName,
+ /* name= */ ProjectConfig.KEY_SR_SUBMITTABILITY_EXPRESSION,
+ /* value= */ "label:\"Code-Review=+2\"");
+ projectConfig.setString(
+ ProjectConfig.SUBMIT_REQUIREMENT,
+ /* subsection= */ submitRequirementName,
+ /* name= */ ProjectConfig.KEY_SR_OVERRIDE_EXPRESSION,
+ /* value= */ invalidExpression);
+ });
+
+ PushResult r = pushRefsMetaConfig();
+ assertErrorStatus(
+ r,
+ "Invalid project configuration",
+ String.format(
+ "project.config: Expression '%s' of submit requirement '%s' (parameter %s.%s.%s) is"
+ + " invalid: Unsupported operator %s",
+ invalidExpression,
+ submitRequirementName,
+ ProjectConfig.SUBMIT_REQUIREMENT,
+ submitRequirementName,
+ ProjectConfig.KEY_SR_OVERRIDE_EXPRESSION,
+ invalidExpression));
+ }
+
+ @Test
+ public void submitRequirementWithInvalidAllowOverrideInChildProjectsIsRejected()
+ throws Exception {
+ fetchRefsMetaConfig();
+
+ String submitRequirementName = "Code-Review";
+ String invalidValue = "invalid";
+ updateProjectConfig(
+ projectConfig ->
+ projectConfig.setString(
+ ProjectConfig.SUBMIT_REQUIREMENT,
+ /* subsection= */ submitRequirementName,
+ /* name= */ ProjectConfig.KEY_SR_OVERRIDE_IN_CHILD_PROJECTS,
+ /* value= */ invalidValue));
+
+ PushResult r = pushRefsMetaConfig();
+ assertErrorStatus(
+ r,
+ "Invalid project configuration",
+ String.format(
+ "project.config: Invalid value %s.%s.%s for submit requirement '%s': %s",
+ ProjectConfig.SUBMIT_REQUIREMENT,
+ submitRequirementName,
+ ProjectConfig.KEY_SR_OVERRIDE_IN_CHILD_PROJECTS,
+ submitRequirementName,
+ invalidValue));
}
private void fetchRefsMetaConfig() throws Exception {
diff --git a/javatests/com/google/gerrit/server/cache/serialize/entities/SubmitRequirementSerializerTest.java b/javatests/com/google/gerrit/server/cache/serialize/entities/SubmitRequirementSerializerTest.java
index a1dee1a..6993dfe 100644
--- a/javatests/com/google/gerrit/server/cache/serialize/entities/SubmitRequirementSerializerTest.java
+++ b/javatests/com/google/gerrit/server/cache/serialize/entities/SubmitRequirementSerializerTest.java
@@ -26,7 +26,7 @@
public class SubmitRequirementSerializerTest {
private static final SubmitRequirement submitReq =
SubmitRequirement.builder()
- .setName("code-review")
+ .setName("Code-Review")
.setDescription(Optional.of("require code review +2"))
.setApplicabilityExpression(SubmitRequirementExpression.of("branch(refs/heads/master)"))
.setSubmittabilityExpression(SubmitRequirementExpression.create("label(code-review, 2+)"))
diff --git a/javatests/com/google/gerrit/server/index/change/FakeQueryBuilder.java b/javatests/com/google/gerrit/server/index/change/FakeQueryBuilder.java
index 66a98e8..67b0342 100644
--- a/javatests/com/google/gerrit/server/index/change/FakeQueryBuilder.java
+++ b/javatests/com/google/gerrit/server/index/change/FakeQueryBuilder.java
@@ -56,6 +56,7 @@
new Config(),
null,
null,
+ null,
null));
}
diff --git a/javatests/com/google/gerrit/server/notedb/ChangeNotesStateTest.java b/javatests/com/google/gerrit/server/notedb/ChangeNotesStateTest.java
index 61002f9..0c26f1a 100644
--- a/javatests/com/google/gerrit/server/notedb/ChangeNotesStateTest.java
+++ b/javatests/com/google/gerrit/server/notedb/ChangeNotesStateTest.java
@@ -696,7 +696,7 @@
.setApplicabilityExpression(
SubmitRequirementExpression.of("project:foo"))
.setSubmittabilityExpression(
- SubmitRequirementExpression.create("label:code-review=+2"))
+ SubmitRequirementExpression.create("label:Code-Review=+2"))
.setAllowOverrideInChildProjects(false)
.build())
.applicabilityExpressionResult(
@@ -708,10 +708,10 @@
ImmutableList.of())))
.submittabilityExpressionResult(
SubmitRequirementExpressionResult.create(
- SubmitRequirementExpression.create("label:code-review=+2"),
+ SubmitRequirementExpression.create("label:Code-Review=+2"),
SubmitRequirementExpressionResult.Status.FAIL,
ImmutableList.of(),
- ImmutableList.of("label:code-review=+2")))
+ ImmutableList.of("label:Code-Review=+2")))
.build()))
.build(),
newProtoBuilder()
@@ -726,7 +726,7 @@
SubmitRequirementProto.newBuilder()
.setName("Code-Review")
.setApplicabilityExpression("project:foo")
- .setSubmittabilityExpression("label:code-review=+2")
+ .setSubmittabilityExpression("label:Code-Review=+2")
.setAllowOverrideInChildProjects(false)
.build())
.setApplicabilityExpressionResult(
@@ -737,9 +737,9 @@
.build())
.setSubmittabilityExpressionResult(
SubmitRequirementExpressionResultProto.newBuilder()
- .setExpression("label:code-review=+2")
+ .setExpression("label:Code-Review=+2")
.setStatus("FAIL")
- .addFailingAtoms("label:code-review=+2")
+ .addFailingAtoms("label:Code-Review=+2")
.build())
.build())
.build());
diff --git a/javatests/com/google/gerrit/server/project/ProjectConfigTest.java b/javatests/com/google/gerrit/server/project/ProjectConfigTest.java
index 9df59c2..aed1648 100644
--- a/javatests/com/google/gerrit/server/project/ProjectConfigTest.java
+++ b/javatests/com/google/gerrit/server/project/ProjectConfigTest.java
@@ -217,7 +217,7 @@
"[submit-requirement \"Code-review\"]\n"
+ " description = At least one Code Review +2\n"
+ " applicableIf =branch(refs/heads/master)\n"
- + " submittableIf = label(code-review, +2)\n"
+ + " submittableIf = label(Code-Review, +2)\n"
+ "[submit-requirement \"api-review\"]\n"
+ " description = Additional review required for API modifications\n"
+ " applicableIf =commit_filepath_contains(\\\"/api/.*\\\")\n"
@@ -237,7 +237,7 @@
.setApplicabilityExpression(
SubmitRequirementExpression.of("branch(refs/heads/master)"))
.setSubmittabilityExpression(
- SubmitRequirementExpression.create("label(code-review, +2)"))
+ SubmitRequirementExpression.create("label(Code-Review, +2)"))
.setOverrideExpression(Optional.empty())
.setAllowOverrideInChildProjects(false)
.build(),
@@ -262,19 +262,19 @@
.add("groups", group(developers))
.add(
"project.config",
- "[submit-requirement \"code-review\"]\n"
- + " submittableIf = label(code-review, +2)\n")
+ "[submit-requirement \"Code-Review\"]\n"
+ + " submittableIf = label(Code-Review, +2)\n")
.create();
ProjectConfig cfg = read(rev);
Map<String, SubmitRequirement> submitRequirements = cfg.getSubmitRequirementSections();
assertThat(submitRequirements)
.containsExactly(
- "code-review",
+ "Code-Review",
SubmitRequirement.builder()
- .setName("code-review")
+ .setName("Code-Review")
.setSubmittabilityExpression(
- SubmitRequirementExpression.create("label(code-review, +2)"))
+ SubmitRequirementExpression.create("label(Code-Review, +2)"))
.setAllowOverrideInChildProjects(false)
.build());
}
@@ -320,8 +320,8 @@
.add("groups", group(developers))
.add(
"project.config",
- "[submit-requirement \"code-review\"]\n"
- + " applicableIf =label(code-review, +2)\n")
+ "[submit-requirement \"Code-Review\"]\n"
+ + " applicableIf =label(Code-Review, +2)\n")
.create();
ProjectConfig cfg = read(rev);
@@ -330,8 +330,9 @@
assertThat(cfg.getValidationErrors()).hasSize(1);
assertThat(Iterables.getOnlyElement(cfg.getValidationErrors()).getMessage())
.isEqualTo(
- "project.config: Submit requirement 'code-review' does not define a submittability"
- + " expression.");
+ "project.config: Setting a submittability expression for submit requirement"
+ + " 'Code-Review' is required: Missing"
+ + " submit-requirement.Code-Review.submittableIf");
}
@Test
@@ -952,10 +953,10 @@
tr.commit()
.add(
"project.config",
- "[submit-requirement \"code-review\"]\n"
+ "[submit-requirement \"Code-Review\"]\n"
+ " description = At least one Code Review +2\n"
+ " applicableIf =branch(refs/heads/master)\n"
- + " submittableIf = label(code-review, +2)\n"
+ + " submittableIf = label(Code-Review, +2)\n"
+ "[notify \"name\"]\n"
+ " email = example@example.com\n")
.create();
diff --git a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
index 332548a..3b671aa 100644
--- a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
+++ b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
@@ -111,6 +111,7 @@
import com.google.gerrit.server.change.NotifyResolver;
import com.google.gerrit.server.change.PatchSetInserter;
import com.google.gerrit.server.config.AllUsersName;
+import com.google.gerrit.server.experiments.ExperimentFeaturesConstants;
import com.google.gerrit.server.git.meta.MetaDataUpdate;
import com.google.gerrit.server.index.change.ChangeField;
import com.google.gerrit.server.index.change.ChangeIndexCollection;
@@ -2358,7 +2359,21 @@
}
@Test
- public void byHasDraft() throws Exception {
+ public void byHasDraft_draftsComputedFromIndex() throws Exception {
+ byHasDraft();
+ }
+
+ @Test
+ @GerritConfig(
+ name = "experiments.enabled",
+ value =
+ ExperimentFeaturesConstants
+ .GERRIT_BACKEND_REQUEST_FEATURE_COMPUTE_FROM_ALL_USERS_REPOSITORY)
+ public void byHasDraft_draftsComputedFromAllUsersRepository() throws Exception {
+ byHasDraft();
+ }
+
+ private void byHasDraft() throws Exception {
TestRepository<Repo> repo = createProject("repo");
Change change1 = insert(repo, newChange(repo));
Change change2 = insert(repo, newChange(repo));
@@ -2386,7 +2401,11 @@
assertQuery("has:draft");
}
- @Test
+ /**
+ * This test does not have a test about drafts computed from All-Users Repository because zombie
+ * drafts can't be filtered when computing from All-Users repository. TODO(paiking): During
+ * rollout, we should find a way to fix zombie drafts.
+ */
public void byHasDraftExcludesZombieDrafts() throws Exception {
Project.NameKey project = Project.nameKey("repo");
TestRepository<Repo> repo = createProject(project.get());
@@ -2424,8 +2443,62 @@
assertQuery("has:draft");
}
+ public void byHasDraftWithManyDrafts_draftsComputedFromIndex() throws Exception {
+ byHasDraftWithManyDrafts();
+ }
+
+ @GerritConfig(
+ name = "experiments.enabled",
+ value =
+ ExperimentFeaturesConstants
+ .GERRIT_BACKEND_REQUEST_FEATURE_COMPUTE_FROM_ALL_USERS_REPOSITORY)
+ public void byHasDraftWithManyDrafts_draftsComputedFromAllUsersRepository() throws Exception {
+ byHasDraftWithManyDrafts();
+ }
+
+ private void byHasDraftWithManyDrafts() throws Exception {
+ TestRepository<Repo> repo = createProject("repo");
+ Change[] changesWithDrafts = new Change[30];
+
+ // unrelated change not shown in the result.
+ insert(repo, newChange(repo));
+
+ for (int i = 0; i < changesWithDrafts.length; i++) {
+ // put the changes in reverse order since this is the order we receive them from the index.
+ changesWithDrafts[changesWithDrafts.length - 1 - i] = insert(repo, newChange(repo));
+ DraftInput in = new DraftInput();
+ in.line = 1;
+ in.message = "nit: trailing whitespace";
+ in.path = Patch.COMMIT_MSG;
+ gApi.changes()
+ .id(changesWithDrafts[changesWithDrafts.length - 1 - i].getId().get())
+ .current()
+ .createDraft(in);
+ }
+ assertQuery("has:draft", changesWithDrafts);
+
+ Account.Id user2 =
+ accountManager.authenticate(authRequestFactory.createForUser("anotheruser")).getAccountId();
+ requestContext.setContext(newRequestContext(user2));
+ assertQuery("has:draft");
+ }
+
@Test
- public void byStarredBy() throws Exception {
+ public void byStarredBy_starsComputedFromIndex() throws Exception {
+ byStarredBy();
+ }
+
+ @GerritConfig(
+ name = "experiments.enabled",
+ value =
+ ExperimentFeaturesConstants
+ .GERRIT_BACKEND_REQUEST_FEATURE_COMPUTE_FROM_ALL_USERS_REPOSITORY)
+ @Test
+ public void byStarredBy_starsComputedFromAllUsersRepository() throws Exception {
+ byStarredBy();
+ }
+
+ private void byStarredBy() throws Exception {
TestRepository<Repo> repo = createProject("repo");
Change change1 = insert(repo, newChange(repo));
Change change2 = insert(repo, newChange(repo));
@@ -2446,7 +2519,21 @@
}
@Test
- public void byStar() throws Exception {
+ public void byStar_starsComputedFromIndex() throws Exception {
+ byStar();
+ }
+
+ @GerritConfig(
+ name = "experiments.enabled",
+ value =
+ ExperimentFeaturesConstants
+ .GERRIT_BACKEND_REQUEST_FEATURE_COMPUTE_FROM_ALL_USERS_REPOSITORY)
+ @Test
+ public void byStar_starsComputedFromAllUsersRepository() throws Exception {
+ byStar();
+ }
+
+ private void byStar() throws Exception {
TestRepository<Repo> repo = createProject("repo");
Change change1 = insert(repo, newChangeWithStatus(repo, Change.Status.MERGED));
Change change2 = insert(repo, newChangeWithStatus(repo, Change.Status.MERGED));
@@ -2472,7 +2559,21 @@
}
@Test
- public void byIgnore() throws Exception {
+ public void byIgnore_starsComputedFromIndex() throws Exception {
+ byIgnore();
+ }
+
+ @Test
+ @GerritConfig(
+ name = "experiments.enabled",
+ value =
+ ExperimentFeaturesConstants
+ .GERRIT_BACKEND_REQUEST_FEATURE_COMPUTE_FROM_ALL_USERS_REPOSITORY)
+ public void byIgnore_starsComputedFromAllUsersRepository() throws Exception {
+ byIgnore();
+ }
+
+ private void byIgnore() throws Exception {
TestRepository<Repo> repo = createProject("repo");
Account.Id user2 =
accountManager.authenticate(authRequestFactory.createForUser("anotheruser")).getAccountId();
@@ -2492,6 +2593,42 @@
assertQuery("-star:ignore", change2, change1);
}
+ public void byStarWithManyStars_starsComputedFromIndex() throws Exception {
+ byStarWithManyStars();
+ }
+
+ @GerritConfig(
+ name = "experiments.enabled",
+ value =
+ ExperimentFeaturesConstants
+ .GERRIT_BACKEND_REQUEST_FEATURE_COMPUTE_FROM_ALL_USERS_REPOSITORY)
+ public void byStarWithManyStars_starsComputedFromAllUsersRepository() throws Exception {
+ byStarWithManyStars();
+ }
+
+ private void byStarWithManyStars() throws Exception {
+ TestRepository<Repo> repo = createProject("repo");
+ Change[] changesWithDrafts = new Change[30];
+ for (int i = 0; i < changesWithDrafts.length; i++) {
+ // put the changes in reverse order since this is the order we receive them from the index.
+ changesWithDrafts[changesWithDrafts.length - 1 - i] = insert(repo, newChange(repo));
+
+ // star the change
+ gApi.accounts()
+ .self()
+ .starChange(changesWithDrafts[changesWithDrafts.length - 1 - i].getId().toString());
+
+ // ignore the change
+ gApi.changes()
+ .id(changesWithDrafts[changesWithDrafts.length - 1 - i].getId().toString())
+ .ignore(true);
+ }
+
+ // all changes are both starred and ignored.
+ assertQuery("is:ignored", changesWithDrafts);
+ assertQuery("is:starred", changesWithDrafts);
+ }
+
@Test
public void byFrom() throws Exception {
TestRepository<Repo> repo = createProject("repo");
diff --git a/lib/BUILD b/lib/BUILD
index f924e4ca..b2810cf 100644
--- a/lib/BUILD
+++ b/lib/BUILD
@@ -128,6 +128,15 @@
)
java_library(
+ name = "guava-testlib",
+ data = ["//lib:LICENSE-Apache2.0"],
+ visibility = ["//visibility:public"],
+ exports = [
+ "@guava-testlib//jar",
+ ],
+)
+
+java_library(
name = "caffeine",
data = ["//lib:LICENSE-Apache2.0"],
visibility = [
diff --git a/lib/nongoogle_test.sh b/lib/nongoogle_test.sh
index 591e76e..90d38b0 100755
--- a/lib/nongoogle_test.sh
+++ b/lib/nongoogle_test.sh
@@ -23,6 +23,7 @@
flogger-log4j-backend
flogger-system-backend
guava
+guava-testlib
guice-assistedinject
guice-library
guice-servlet
diff --git a/polygerrit-ui/app/BUILD b/polygerrit-ui/app/BUILD
index 3a647d4..896f9ac 100644
--- a/polygerrit-ui/app/BUILD
+++ b/polygerrit-ui/app/BUILD
@@ -125,7 +125,6 @@
"elements/diff/gr-diff-preferences-dialog/gr-diff-preferences-dialog_html.ts",
"elements/diff/gr-diff-view/gr-diff-view_html.ts",
"elements/diff/gr-diff/gr-diff_html.ts",
- "elements/diff/gr-patch-range-select/gr-patch-range-select_html.ts",
"elements/gr-app-element_html.ts",
"elements/settings/gr-watched-projects-editor/gr-watched-projects-editor_html.ts",
"elements/shared/gr-account-list/gr-account-list_html.ts",
diff --git a/polygerrit-ui/app/api/rest-api.ts b/polygerrit-ui/app/api/rest-api.ts
index f86e825..39b40b6 100644
--- a/polygerrit-ui/app/api/rest-api.ts
+++ b/polygerrit-ui/app/api/rest-api.ts
@@ -1081,6 +1081,7 @@
applicability_expression_result?: SubmitRequirementExpressionInfo;
submittability_expression_result: SubmitRequirementExpressionInfo;
override_expression_result?: SubmitRequirementExpressionInfo;
+ is_legacy?: boolean;
}
/**
diff --git a/polygerrit-ui/app/constants/constants.ts b/polygerrit-ui/app/constants/constants.ts
index 645e770..6ff2894 100644
--- a/polygerrit-ui/app/constants/constants.ts
+++ b/polygerrit-ui/app/constants/constants.ts
@@ -256,7 +256,6 @@
export function createDefaultPreferences() {
return {
changes_per_page: 25,
- default_diff_view: DiffViewMode.SIDE_BY_SIDE,
diff_view: DiffViewMode.SIDE_BY_SIDE,
size_bar_in_change_table: true,
} as PreferencesInfo;
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-column/gr-change-list-column.ts b/polygerrit-ui/app/elements/change-list/gr-change-list-column/gr-change-list-column.ts
new file mode 100644
index 0000000..2b9abfd
--- /dev/null
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-column/gr-change-list-column.ts
@@ -0,0 +1,87 @@
+/**
+ * @license
+ * Copyright (C) 2015 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import '../../change/gr-submit-requirement-dashboard-hovercard/gr-submit-requirement-dashboard-hovercard';
+import {LitElement, css, html} from 'lit';
+import {customElement, property} from 'lit/decorators';
+import {ChangeInfo, SubmitRequirementStatus} from '../../../api/rest-api';
+import {changeIsMerged} from '../../../utils/change-util';
+
+@customElement('gr-change-list-column-requirements')
+export class GrChangeListColumRequirements extends LitElement {
+ @property({type: Object})
+ change?: ChangeInfo;
+
+ static override get styles() {
+ return [
+ css`
+ iron-icon {
+ width: var(--line-height-normal, 20px);
+ height: var(--line-height-normal, 20px);
+ vertical-align: top;
+ }
+ span {
+ line-height: var(--line-height-normal);
+ }
+ .check {
+ color: var(--success-foreground);
+ }
+ iron-icon.close {
+ color: var(--error-foreground);
+ }
+ `,
+ ];
+ }
+
+ override render() {
+ if (changeIsMerged(this.change)) {
+ return this.renderState('check', 'Merged');
+ }
+
+ const submitRequirements = (this.change?.submit_requirements ?? []).filter(
+ req => req.status !== SubmitRequirementStatus.NOT_APPLICABLE
+ );
+ if (!submitRequirements.length) return html`n/a`;
+ const numOfRequirements = submitRequirements.length;
+ const numOfSatisfiedRequirements = submitRequirements.filter(
+ req => req.status === SubmitRequirementStatus.SATISFIED
+ ).length;
+
+ if (numOfSatisfiedRequirements === numOfRequirements) {
+ return this.renderState('check', 'Ready');
+ }
+ return this.renderState(
+ 'close',
+ `${numOfSatisfiedRequirements} of ${numOfRequirements} granted`
+ );
+ }
+
+ renderState(icon: string, message: string) {
+ return html`<span class="${icon}"
+ ><gr-submit-requirement-dashboard-hovercard .change=${this.change}>
+ </gr-submit-requirement-dashboard-hovercard>
+ <iron-icon class="${icon}" icon="gr-icons:${icon}" role="img"></iron-icon
+ >${message}</span
+ >`;
+ }
+}
+
+declare global {
+ interface HTMLElementTagNameMap {
+ 'gr-change-list-column-requirements': GrChangeListColumRequirements;
+ }
+}
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.ts b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.ts
index c476d2d..43f6730 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.ts
@@ -26,6 +26,8 @@
import '../../../styles/shared-styles';
import '../../plugins/gr-endpoint-decorator/gr-endpoint-decorator';
import '../../plugins/gr-endpoint-param/gr-endpoint-param';
+import '../gr-change-list-column/gr-change-list-column';
+import '../../shared/gr-tooltip-content/gr-tooltip-content';
import {PolymerElement} from '@polymer/polymer/polymer-element';
import {htmlTemplate} from './gr-change-list-item_html';
import {GerritNav} from '../../core/gr-navigation/gr-navigation';
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_html.ts b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_html.ts
index a0aa962..c59b2e3 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_html.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_html.ts
@@ -290,6 +290,13 @@
</template>
</gr-tooltip-content>
</td>
+ <td
+ class="cell requirements"
+ hidden$="[[_computeIsColumnHidden('Requirements', visibleChangeTableColumns)]]"
+ >
+ <gr-change-list-column-requirements change="[[change]]">
+ </gr-change-list-column-requirements>
+ </td>
<template is="dom-repeat" items="[[labelNames]]" as="labelName">
<td
title$="[[_computeLabelTitle(change, labelName)]]"
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_test.ts b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_test.ts
index 34cb6eb..35be3de 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_test.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_test.ts
@@ -369,6 +369,7 @@
'Branch',
'Updated',
'Size',
+ 'Requirements',
];
await flush();
@@ -392,6 +393,7 @@
'Branch',
'Updated',
'Size',
+ 'Requirements',
];
await flush();
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.ts b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.ts
index a79dd8e..12cf314 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.ts
@@ -50,6 +50,8 @@
import {fireEvent, fireReload} from '../../../utils/event-util';
import {ScrollMode} from '../../../constants/constants';
import {listen} from '../../../services/shortcuts/shortcuts-service';
+import {KnownExperimentId} from '../../../services/flags/flags';
+import {PRIORITY_REQUIREMENTS_ORDER} from '../../../utils/label-util';
const NUMBER_FIXED_COLUMNS = 3;
const CLOSED_STATUS = ['MERGED', 'ABANDONED'];
@@ -67,6 +69,7 @@
'Branch',
'Updated',
'Size',
+ 'Requirements',
];
export interface ChangeListSection {
@@ -262,6 +265,8 @@
if (!config || !config.change) return true;
if (column === 'Assignee') return !!config.change.enable_assignee;
if (column === 'Comments') return experiments.includes('comments-column');
+ if (column === 'Requirements')
+ return experiments.includes(KnownExperimentId.SUBMIT_REQUIREMENTS_UI);
return true;
}
@@ -316,6 +321,13 @@
labels = labels.concat(currentLabels.filter(nonExistingLabel));
}
}
+ if (
+ this.flagsService.enabledExperiments.includes(
+ KnownExperimentId.SUBMIT_REQUIREMENTS_UI
+ )
+ ) {
+ labels = labels.filter(l => PRIORITY_REQUIREMENTS_ORDER.includes(l));
+ }
return labels.sort();
}
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.js b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.js
index de1a0a2..472435c 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.js
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.js
@@ -282,6 +282,7 @@
'Branch',
'Updated',
'Size',
+ 'Requirements',
],
};
element._config = {};
@@ -319,6 +320,7 @@
'Branch',
'Updated',
'Size',
+ 'Requirements',
],
};
element._config = {};
diff --git a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_html.ts b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_html.ts
index 97101f6..25dab25 100644
--- a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_html.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_html.ts
@@ -256,7 +256,6 @@
id="assigneeValue"
placeholder="Set assignee..."
max-count="1"
- skip-suggest-on-empty=""
accounts="{{_assignee}}"
readonly="[[_computeAssigneeReadOnly(_mutable, change)]]"
suggestions-provider="[[_getReviewerSuggestionsProvider(change)]]"
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 b635d3f..82deeb5 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
@@ -61,12 +61,12 @@
import {getPluginEndpoints} from '../../shared/gr-js-api-interface/gr-plugin-endpoints';
import {getPluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader';
import {RevisionInfo as RevisionInfoClass} from '../../shared/revision-info/revision-info';
-import {DiffViewMode} from '../../../api/diff';
import {
ChangeStatus,
DefaultBase,
PrimaryTab,
SecondaryTab,
+ DiffViewMode,
} from '../../../constants/constants';
import {NO_ROBOT_COMMENTS_THREADS_MSG} from '../../../constants/messages';
@@ -196,6 +196,7 @@
hasAttention,
} from '../../../utils/attention-set-util';
import {listen} from '../../../services/shortcuts/shortcuts-service';
+import {preferenceDiffViewMode$} from '../../../services/user/user-model';
const MIN_LINES_FOR_COMMIT_COLLAPSE = 18;
@@ -551,6 +552,8 @@
restApiService = appContext.restApiService;
+ private readonly userService = appContext.userService;
+
private readonly commentsService = appContext.commentsService;
private readonly shortcuts = appContext.shortcutsService;
@@ -611,6 +614,8 @@
private lastStarredTimestamp?: number;
+ private diffViewMode?: DiffViewMode;
+
override ready() {
super.ready();
aPluginHasRegistered$.pipe(takeUntil(this.disconnected$)).subscribe(b => {
@@ -622,6 +627,11 @@
drafts$.pipe(takeUntil(this.disconnected$)).subscribe(drafts => {
this._diffDrafts = {...drafts};
});
+ preferenceDiffViewMode$
+ .pipe(takeUntil(this.disconnected$))
+ .subscribe(diffViewMode => {
+ this.diffViewMode = diffViewMode;
+ });
changeComments$
.pipe(takeUntil(this.disconnected$))
.subscribe(changeComments => {
@@ -666,7 +676,6 @@
this._account = acct;
});
}
- this._setDiffViewMode();
});
this.replyDialogResizeObserver = new ResizeObserver(() =>
@@ -735,24 +744,6 @@
return this.shadowRoot!.querySelector<GrThreadList>('gr-thread-list');
}
- _setDiffViewMode(opt_reset?: boolean) {
- if (!opt_reset && this.viewState.diffViewMode) {
- return;
- }
-
- return this._getPreferences()
- .then(prefs => {
- if (!this.viewState.diffMode && prefs) {
- this.set('viewState.diffMode', prefs.default_diff_view);
- }
- })
- .then(() => {
- if (!this.viewState.diffMode) {
- this.set('viewState.diffMode', 'SIDE_BY_SIDE');
- }
- });
- }
-
_onOpenFixPreview(e: OpenFixPreviewEvent) {
this.$.applyFixDialog.open(e);
}
@@ -762,10 +753,12 @@
}
_handleToggleDiffMode() {
- if (this.viewState.diffMode === DiffViewMode.SIDE_BY_SIDE) {
- this.$.fileListHeader.setDiffViewMode(DiffViewMode.UNIFIED);
+ if (this.diffViewMode === DiffViewMode.SIDE_BY_SIDE) {
+ this.userService.updatePreferences({diff_view: DiffViewMode.UNIFIED});
} else {
- this.$.fileListHeader.setDiffViewMode(DiffViewMode.SIDE_BY_SIDE);
+ this.userService.updatePreferences({
+ diff_view: DiffViewMode.SIDE_BY_SIDE,
+ });
}
}
@@ -1416,9 +1409,6 @@
!!this.viewState.changeNum &&
this.viewState.changeNum !== this._changeNum
) {
- // Reset the diff mode to null when navigating from one change to
- // another, so that the user's preference is restored.
- this._setDiffViewMode(true);
this.set('_numFilesShown', DEFAULT_NUM_FILES_SHOWN);
}
this.set('viewState.changeNum', this._changeNum);
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.ts
index 0b77bc7..d42fc17 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.ts
@@ -533,7 +533,6 @@
server-config="[[_serverConfig]]"
shown-file-count="[[_shownFileCount]]"
diff-prefs="[[_diffPrefs]]"
- diff-view-mode="{{viewState.diffMode}}"
patch-num="{{_patchRange.patchNum}}"
base-patch-num="{{_patchRange.basePatchNum}}"
files-expanded="[[_filesExpanded]]"
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts
index 6cbe59c..acdaefe 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts
@@ -26,6 +26,8 @@
HttpMethod,
MessageTag,
PrimaryTab,
+ createDefaultPreferences,
+ createDefaultDiffPrefs,
} from '../../../constants/constants';
import {GrEditConstants} from '../../edit/gr-edit-constants';
import {_testOnly_resetEndpoints} from '../../shared/gr-js-api-interface/gr-plugin-endpoints';
@@ -35,7 +37,7 @@
import {EventType, PluginApi} from '../../../api/plugin';
import 'lodash/lodash';
-import {mockPromise, stubRestApi} from '../../../test/test-utils';
+import {mockPromise, stubRestApi, stubUsers} from '../../../test/test-utils';
import {
createAppElementChangeViewParams,
createApproval,
@@ -94,6 +96,7 @@
import {GrRelatedChangesList} from '../gr-related-changes-list/gr-related-changes-list';
import {appContext} from '../../../services/app-context';
import {ChangeStates} from '../../shared/gr-change-status/gr-change-status';
+import {_testOnly_setState} from '../../../services/user/user-model';
const pluginApi = _testOnly_initGerritPluginApi();
const fixture = fixtureFromElement('gr-change-view');
@@ -803,20 +806,36 @@
assert.isTrue(stub.called);
});
- test('m should toggle diff mode', () => {
- const setModeStub = sinon.stub(
- element.$.fileListHeader,
- 'setDiffViewMode'
+ test('m should toggle diff mode', async () => {
+ const updatePreferencesStub = stubUsers('updatePreferences');
+ await flush();
+
+ const prefs = {
+ ...createDefaultPreferences(),
+ diff_view: DiffViewMode.SIDE_BY_SIDE,
+ };
+ _testOnly_setState({
+ preferences: prefs,
+ diffPreferences: createDefaultDiffPrefs(),
+ });
+ element._handleToggleDiffMode();
+ assert.isTrue(
+ updatePreferencesStub.calledWith({diff_view: DiffViewMode.UNIFIED})
);
- flush();
- element.viewState.diffMode = DiffViewMode.SIDE_BY_SIDE;
+ const newPrefs = {
+ ...createDefaultPreferences(),
+ diff_view: DiffViewMode.UNIFIED,
+ };
+ _testOnly_setState({
+ preferences: newPrefs,
+ diffPreferences: createDefaultDiffPrefs(),
+ });
+ await flush();
element._handleToggleDiffMode();
- assert.isTrue(setModeStub.calledWith(DiffViewMode.UNIFIED));
-
- element.viewState.diffMode = DiffViewMode.UNIFIED;
- element._handleToggleDiffMode();
- assert.isTrue(setModeStub.calledWith(DiffViewMode.SIDE_BY_SIDE));
+ assert.isTrue(
+ updatePreferencesStub.calledWith({diff_view: DiffViewMode.SIDE_BY_SIDE})
+ );
});
});
@@ -1278,52 +1297,6 @@
assert.equal(element._numFilesShown, 200);
});
- test('_setDiffViewMode is called with reset when new change is loaded', () => {
- const setDiffViewModeStub = sinon.stub(element, '_setDiffViewMode');
- element.viewState = {changeNum: 1 as NumericChangeId};
- element._changeNum = 2 as NumericChangeId;
- element._resetFileListViewState();
- assert.isTrue(setDiffViewModeStub.calledWithExactly(true));
- });
-
- test('diffViewMode is propagated from file list header', () => {
- element.viewState = {diffMode: DiffViewMode.UNIFIED};
- element.$.fileListHeader.diffViewMode = DiffViewMode.SIDE_BY_SIDE;
- assert.equal(element.viewState.diffMode, DiffViewMode.SIDE_BY_SIDE);
- });
-
- test('diffMode defaults to side by side without preferences', async () => {
- stubRestApi('getPreferences').returns(Promise.resolve(createPreferences()));
- // No user prefs or diff view mode set.
-
- await element._setDiffViewMode()!;
- assert.equal(element.viewState.diffMode, DiffViewMode.SIDE_BY_SIDE);
- });
-
- test('diffMode defaults to preference when not already set', async () => {
- stubRestApi('getPreferences').returns(
- Promise.resolve({
- ...createPreferences(),
- default_diff_view: DiffViewMode.UNIFIED,
- })
- );
-
- await element._setDiffViewMode()!;
- assert.equal(element.viewState.diffMode, DiffViewMode.UNIFIED);
- });
-
- test('existing diffMode overrides preference', async () => {
- element.viewState.diffMode = DiffViewMode.SIDE_BY_SIDE;
- stubRestApi('getPreferences').returns(
- Promise.resolve({
- ...createPreferences(),
- default_diff_view: DiffViewMode.UNIFIED,
- })
- );
- await element._setDiffViewMode()!;
- assert.equal(element.viewState.diffMode, DiffViewMode.SIDE_BY_SIDE);
- });
-
test('don’t reload entire page when patchRange changes', async () => {
const reloadStub = sinon
.stub(element, 'loadData')
diff --git a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.ts b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.ts
index ae3eee5..1b44e35 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.ts
+++ b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.ts
@@ -41,7 +41,6 @@
import {DiffPreferencesInfo} from '../../../types/diff';
import {ChangeComments} from '../../diff/gr-comment-api/gr-comment-api';
import {GrDiffModeSelector} from '../../diff/gr-diff-mode-selector/gr-diff-mode-selector';
-import {DiffViewMode} from '../../../constants/constants';
import {GrButton} from '../../shared/gr-button/gr-button';
import {fireEvent} from '../../../utils/event-util';
import {
@@ -122,9 +121,6 @@
@property({type: Object})
diffPrefs?: DiffPreferencesInfo;
- @property({type: String, notify: true})
- diffViewMode?: DiffViewMode;
-
@property({type: String})
patchNum?: PatchSetNum;
@@ -144,10 +140,6 @@
private readonly shortcuts = appContext.shortcutsService;
- setDiffViewMode(mode: DiffViewMode) {
- this.$.modeSelect.setMode(mode);
- }
-
_expandAllDiffs() {
fireEvent(this, 'expand-diffs');
}
diff --git a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header_html.ts b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header_html.ts
index 73d0819..5a85531 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header_html.ts
+++ b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header_html.ts
@@ -169,7 +169,6 @@
<span class="fileViewActionsLabel">Diff view:</span>
<gr-diff-mode-selector
id="modeSelect"
- mode="{{diffViewMode}}"
save-on-change="[[loggedIn]]"
></gr-diff-mode-selector>
<span
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
index cc76145..95984b8 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
@@ -83,11 +83,15 @@
import {ParsedChangeInfo, PatchSetFile} from '../../../types/types';
import {Timing} from '../../../constants/reporting';
import {RevisionInfo} from '../../shared/revision-info/revision-info';
-import {preferences$} from '../../../services/user/user-model';
+import {
+ diffPreferences$,
+ sizeBarInChangeTable$,
+} from '../../../services/user/user-model';
import {changeComments$} from '../../../services/comments/comments-model';
import {Subject} from 'rxjs';
import {takeUntil} from 'rxjs/operators';
import {listen} from '../../../services/shortcuts/shortcuts-service';
+import {diffViewMode$} from '../../../services/browser/browser-model';
export const DEFAULT_NUM_FILES_SHOWN = 200;
@@ -316,6 +320,8 @@
private readonly restApiService = appContext.restApiService;
+ private readonly userService = appContext.userService;
+
disconnected$ = new Subject();
/** Called in disconnectedCallback. */
@@ -377,6 +383,20 @@
.subscribe(changeComments => {
this.changeComments = changeComments;
});
+ diffViewMode$
+ .pipe(takeUntil(this.disconnected$))
+ .subscribe(diffView => (this.diffViewMode = diffView));
+ diffPreferences$
+ .pipe(takeUntil(this.disconnected$))
+ .subscribe(diffPreferences => {
+ this.diffPrefs = diffPreferences;
+ });
+ sizeBarInChangeTable$
+ .pipe(takeUntil(this.disconnected$))
+ .subscribe(sizeBarInChangeTable => {
+ this._showSizeBars = sizeBarInChangeTable;
+ });
+
getPluginLoader()
.awaitPluginsLoaded()
.then(() => {
@@ -421,7 +441,9 @@
});
this.cleanups.push(
addGlobalShortcut({key: Key.ESC}, _ => this._handleEscKey()),
- addShortcut(this, {key: Key.ENTER}, _ => this.handleOpenFile())
+ addShortcut(this, {key: Key.ENTER}, _ => this.handleOpenFile(), {
+ shouldSuppress: true,
+ })
);
}
@@ -472,16 +494,6 @@
})
);
- promises.push(
- this._getDiffPreferences().then(prefs => {
- this.diffPrefs = prefs;
- })
- );
-
- preferences$.pipe(takeUntil(this.disconnected$)).subscribe(prefs => {
- this._showSizeBars = !!prefs?.size_bar_in_change_table;
- });
-
return Promise.all(promises).then(() => {
this._loading = false;
this._detectChromiteButler();
@@ -1638,9 +1650,7 @@
}
_handleReloadingDiffPreference() {
- this._getDiffPreferences().then(prefs => {
- this.diffPrefs = prefs;
- });
+ this.userService.getDiffPreferences();
}
/**
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_html.ts b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_html.ts
index f7be36b..e8371e3 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_html.ts
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_html.ts
@@ -643,7 +643,6 @@
prefs="[[diffPrefs]]"
project-name="[[change.project]]"
no-render-on-prefs-change=""
- view-mode="[[diffViewMode]]"
></gr-diff-host>
</template>
</div>
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.js b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.js
index 7409be7..0db7690 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.js
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.js
@@ -820,10 +820,8 @@
MockInteractions.tap(row);
flush();
- const diffDisplay = element.diffs[0];
- element._userPrefs = {default_diff_view: 'SIDE_BY_SIDE'};
+ element._userPrefs = {diff_view: 'SIDE_BY_SIDE'};
element.set('diffViewMode', 'UNIFIED_DIFF');
- assert.equal(diffDisplay.viewMode, 'UNIFIED_DIFF');
assert.isTrue(element._updateDiffPreferences.called);
});
diff --git a/polygerrit-ui/app/elements/change/gr-submit-requirement-dashboard-hovercard/gr-submit-requirement-dashboard-hovercard.ts b/polygerrit-ui/app/elements/change/gr-submit-requirement-dashboard-hovercard/gr-submit-requirement-dashboard-hovercard.ts
new file mode 100644
index 0000000..ebe3ce3
--- /dev/null
+++ b/polygerrit-ui/app/elements/change/gr-submit-requirement-dashboard-hovercard/gr-submit-requirement-dashboard-hovercard.ts
@@ -0,0 +1,57 @@
+/**
+ * @license
+ * 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.
+ */
+import '../gr-submit-requirements/gr-submit-requirements';
+import {customElement, property} from 'lit/decorators';
+import {css, html, LitElement} from 'lit';
+import {HovercardMixin} from '../../../mixins/hovercard-mixin/hovercard-mixin';
+import {ParsedChangeInfo} from '../../../types/types';
+
+// This avoids JSC_DYNAMIC_EXTENDS_WITHOUT_JSDOC closure compiler error.
+const base = HovercardMixin(LitElement);
+
+@customElement('gr-submit-requirement-dashboard-hovercard')
+export class GrSubmitRequirementDashboardHovercard extends base {
+ @property({type: Object})
+ change?: ParsedChangeInfo;
+
+ static override get styles() {
+ return [
+ base.styles || [],
+ css`
+ #container {
+ padding: var(--spacing-xl);
+ padding-left: var(--spacing-s);
+ }
+ `,
+ ];
+ }
+
+ override render() {
+ return html`<div id="container" role="tooltip" tabindex="-1">
+ <gr-submit-requirements
+ .change=${this.change}
+ suppress-title
+ ></gr-submit-requirements>
+ </div>`;
+ }
+}
+
+declare global {
+ interface HTMLElementTagNameMap {
+ 'gr-submit-requirement-dashboard-hovercard': GrSubmitRequirementDashboardHovercard;
+ }
+}
diff --git a/polygerrit-ui/app/elements/change/gr-submit-requirements/gr-submit-requirements.ts b/polygerrit-ui/app/elements/change/gr-submit-requirements/gr-submit-requirements.ts
index c493be4..157336d 100644
--- a/polygerrit-ui/app/elements/change/gr-submit-requirements/gr-submit-requirements.ts
+++ b/polygerrit-ui/app/elements/change/gr-submit-requirements/gr-submit-requirements.ts
@@ -49,6 +49,9 @@
import {Category} from '../../../api/checks';
import '../../shared/gr-vote-chip/gr-vote-chip';
+/**
+ * @attr {Boolean} suppress-title - hide titles, currently for hovercard view
+ */
@customElement('gr-submit-requirements')
export class GrSubmitRequirements extends LitElement {
@property({type: Object})
@@ -67,6 +70,9 @@
return [
fontStyles,
css`
+ :host([suppress-title]) .metadata-title {
+ display: none;
+ }
.metadata-title {
color: var(--deemphasized-text-color);
padding-left: var(--metadata-horizontal-padding);
@@ -108,6 +114,7 @@
}
td {
padding: var(--spacing-s);
+ white-space: nowrap;
}
.votes-cell {
display: flex;
@@ -132,10 +139,19 @@
}
override render() {
- const submit_requirements = orderSubmitRequirements(
+ let submit_requirements = orderSubmitRequirements(
this.change?.submit_requirements ?? []
).filter(req => req.status !== SubmitRequirementStatus.NOT_APPLICABLE);
+ const hasNonLegacyRequirements = submit_requirements.some(
+ req => req.is_legacy === false
+ );
+ if (hasNonLegacyRequirements) {
+ submit_requirements = submit_requirements.filter(
+ req => req.is_legacy === false
+ );
+ }
+
return html` <h3
class="metadata-title heading-3"
id="submit-requirements-caption"
diff --git a/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api.ts b/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api.ts
index b99a17b..fc22a58 100644
--- a/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api.ts
+++ b/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api.ts
@@ -580,7 +580,10 @@
/**
* Computes a number of unresolved comment threads in a given file and path.
*/
- computeUnresolvedNum(file: PatchSetFile | PatchNumOnly) {
+ computeUnresolvedNum(
+ file: PatchSetFile | PatchNumOnly,
+ ignorePatchsetLevelComments?: boolean
+ ) {
let comments: Comment[] = [];
let drafts: Comment[] = [];
@@ -595,7 +598,11 @@
comments = comments.concat(drafts);
const threads = createCommentThreads(comments);
- const unresolvedThreads = threads.filter(isUnresolved);
+ let unresolvedThreads = threads.filter(isUnresolved);
+ if (ignorePatchsetLevelComments)
+ unresolvedThreads = unresolvedThreads.filter(
+ thread => !isPatchsetLevel(thread)
+ );
return unresolvedThreads.length;
}
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts
index b1bad1c..a828b9c 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts
@@ -88,10 +88,11 @@
import {TokenHighlightLayer} from '../gr-diff-builder/token-highlight-layer';
import {Timing} from '../../../constants/reporting';
import {changeComments$} from '../../../services/comments/comments-model';
-import {takeUntil} from 'rxjs/operators';
import {ChangeComments} from '../gr-comment-api/gr-comment-api';
import {Subject} from 'rxjs';
import {RenderPreferences} from '../../../api/diff';
+import {diffViewMode$} from '../../../services/browser/browser-model';
+import {takeUntil} from 'rxjs/operators';
const EMPTY_BLAME = 'No blame information for this diff.';
@@ -205,12 +206,12 @@
@property({type: Boolean})
lineWrapping = false;
- @property({type: String})
- viewMode = DiffViewMode.SIDE_BY_SIDE;
-
@property({type: Object})
lineOfInterest?: LineOfInterest;
+ @property({type: String})
+ viewMode = DiffViewMode.SIDE_BY_SIDE;
+
@property({type: Boolean})
showLoadFailure?: boolean;
@@ -312,6 +313,9 @@
override connectedCallback() {
super.connectedCallback();
+ diffViewMode$
+ .pipe(takeUntil(this.disconnected$))
+ .subscribe(diffView => (this.viewMode = diffView));
this._getLoggedIn().then(loggedIn => {
this._loggedIn = loggedIn;
});
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-mode-selector/gr-diff-mode-selector.ts b/polygerrit-ui/app/elements/diff/gr-diff-mode-selector/gr-diff-mode-selector.ts
index b47c51c..3d43ef3 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-mode-selector/gr-diff-mode-selector.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-mode-selector/gr-diff-mode-selector.ts
@@ -26,6 +26,9 @@
import {FixIronA11yAnnouncer} from '../../../types/types';
import {appContext} from '../../../services/app-context';
import {fireIronAnnounce} from '../../../utils/event-util';
+import {diffViewMode$} from '../../../services/browser/browser-model';
+import {Subject} from 'rxjs';
+import {takeUntil} from 'rxjs/operators';
@customElement('gr-diff-mode-selector')
export class GrDiffModeSelector extends PolymerElement {
@@ -34,7 +37,7 @@
}
@property({type: String, notify: true})
- mode?: DiffViewMode;
+ mode: DiffViewMode = DiffViewMode.SIDE_BY_SIDE;
/**
* If set to true, the user's preference will be updated every time a
@@ -48,11 +51,24 @@
private readonly userService = appContext.userService;
+ disconnected$ = new Subject();
+
+ constructor() {
+ super();
+ }
+
override connectedCallback() {
super.connectedCallback();
(
IronA11yAnnouncer as unknown as FixIronA11yAnnouncer
).requestAvailability();
+ diffViewMode$
+ .pipe(takeUntil(this.disconnected$))
+ .subscribe(diffView => (this.mode = diffView));
+ }
+
+ override disconnectedCallback() {
+ this.disconnected$.next();
}
/**
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-mode-selector/gr-diff-mode-selector_test.ts b/polygerrit-ui/app/elements/diff/gr-diff-mode-selector/gr-diff-mode-selector_test.ts
index 8b06c75..fe5f389 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-mode-selector/gr-diff-mode-selector_test.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-mode-selector/gr-diff-mode-selector_test.ts
@@ -20,6 +20,7 @@
import {GrDiffModeSelector} from './gr-diff-mode-selector';
import {DiffViewMode} from '../../../constants/constants';
import {stubUsers} from '../../../test/test-utils';
+import {_testOnly_setState} from '../../../services/browser/browser-model';
const basicFixture = fixtureFromElement('gr-diff-mode-selector');
@@ -47,8 +48,10 @@
});
test('setMode', () => {
+ _testOnly_setState({screenWidth: 0});
const saveStub = stubUsers('updatePreferences');
+ flush();
// Setting the mode initially does not save prefs.
element.saveOnChange = true;
element.setMode(DiffViewMode.SIDE_BY_SIDE);
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-preferences-dialog/gr-diff-preferences-dialog.ts b/polygerrit-ui/app/elements/diff/gr-diff-preferences-dialog/gr-diff-preferences-dialog.ts
index 5a8c55d..9f38655b 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-preferences-dialog/gr-diff-preferences-dialog.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-preferences-dialog/gr-diff-preferences-dialog.ts
@@ -87,18 +87,16 @@
});
}
- _handleSaveDiffPreferences() {
+ async _handleSaveDiffPreferences() {
this.diffPrefs = this._editableDiffPrefs;
- this.$.diffPreferences.save().then(() => {
- this.dispatchEvent(
- new CustomEvent('reload-diff-preference', {
- composed: true,
- bubbles: false,
- })
- );
-
- this.$.diffPrefsOverlay.close();
- });
+ await this.$.diffPreferences.save();
+ this.dispatchEvent(
+ new CustomEvent('reload-diff-preference', {
+ composed: true,
+ bubbles: false,
+ })
+ );
+ this.$.diffPrefsOverlay.close();
}
}
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
index 0813900..e83f948 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
@@ -98,7 +98,7 @@
getPatchRangeForCommentUrl,
isInBaseOfPatchRange,
} from '../../../utils/comment-util';
-import {AppElementParams} from '../../gr-app-types';
+import {AppElementParams, AppElementDiffViewParam} from '../../gr-app-types';
import {EventType, OpenFixPreviewEvent} from '../../../types/events';
import {fireAlert, fireEvent, fireTitleChange} from '../../../utils/event-util';
import {GerritView} from '../../../services/router/router-model';
@@ -109,8 +109,11 @@
import {changeComments$} from '../../../services/comments/comments-model';
import {takeUntil} from 'rxjs/operators';
import {Subject} from 'rxjs';
-import {preferences$} from '../../../services/user/user-model';
import {listen} from '../../../services/shortcuts/shortcuts-service';
+import {
+ preferences$,
+ diffPreferences$,
+} from '../../../services/user/user-model';
const ERR_REVIEW_STATUS = 'Couldn’t change file review status.';
const LOADING_BLAME = 'Loading blame...';
@@ -165,7 +168,7 @@
@property({type: Object, observer: '_paramsChanged'})
params?: AppElementParams;
- @property({type: Object, notify: true, observer: '_changeViewStateChanged'})
+ @property({type: Object, notify: true})
changeViewState: Partial<ChangeViewState> = {};
@property({type: Object})
@@ -222,12 +225,6 @@
@property({type: Object})
_userPrefs?: PreferencesInfo;
- @property({
- type: String,
- computed: '_getDiffViewMode(changeViewState.diffMode, _userPrefs)',
- })
- _diffMode?: string;
-
@property({type: Boolean})
_isImageDiff?: boolean;
@@ -349,6 +346,8 @@
private readonly restApiService = appContext.restApiService;
+ private readonly userService = appContext.userService;
+
private readonly commentsService = appContext.commentsService;
private readonly shortcuts = appContext.shortcutsService;
@@ -369,10 +368,6 @@
this._getLoggedIn().then(loggedIn => {
this._loggedIn = loggedIn;
});
- // TODO(brohlfs): This just ensures that the userService is instantiated at
- // all. We need the service to manage the model, but we are not making any
- // direct calls. Will need to find a better solution to this problem ...
- assertIsDefined(appContext.userService);
changeComments$
.pipe(takeUntil(this.disconnected$))
@@ -383,6 +378,11 @@
preferences$.pipe(takeUntil(this.disconnected$)).subscribe(preferences => {
this._userPrefs = preferences;
});
+ diffPreferences$
+ .pipe(takeUntil(this.disconnected$))
+ .subscribe(diffPreferences => {
+ this._prefs = diffPreferences;
+ });
this.addEventListener('open-fix-preview', e => this._onOpenFixPreview(e));
this.cursor.replaceDiffs([this.$.diffHost]);
this._onRenderHandler = (_: Event) => {
@@ -502,12 +502,6 @@
});
}
- _getDiffPreferences() {
- return this.restApiService.getDiffPreferences().then(prefs => {
- this._prefs = prefs;
- });
- }
-
_getPreferences() {
return this.restApiService.getPreferences();
}
@@ -716,10 +710,13 @@
}
_handleToggleDiffMode() {
- if (this._getDiffViewMode() === DiffViewMode.SIDE_BY_SIDE) {
- this.$.modeSelect.setMode(DiffViewMode.UNIFIED);
+ if (!this._userPrefs) return;
+ if (this._userPrefs.diff_view === DiffViewMode.SIDE_BY_SIDE) {
+ this.userService.updatePreferences({diff_view: DiffViewMode.UNIFIED});
} else {
- this.$.modeSelect.setMode(DiffViewMode.SIDE_BY_SIDE);
+ this.userService.updatePreferences({
+ diff_view: DiffViewMode.SIDE_BY_SIDE,
+ });
}
}
@@ -1017,6 +1014,14 @@
);
}
+ private isSameDiffLoaded(value: AppElementDiffViewParam) {
+ return (
+ this._patchRange?.basePatchNum === value.basePatchNum &&
+ this._patchRange?.patchNum === value.patchNum &&
+ this._path === value.path
+ );
+ }
+
_paramsChanged(value: AppElementParams) {
if (value.view !== GerritView.DIFF) {
return;
@@ -1028,6 +1033,10 @@
if (this._changeNum !== undefined && changeChanged) {
fireEvent(this, EventType.RECREATE_DIFF_VIEW);
return;
+ } else if (this._changeNum !== undefined && this.isSameDiffLoaded(value)) {
+ // changeNum has not changed, so check if there are changes in patchRange
+ // path. If no changes then we can simply render the view as is.
+ return;
}
this._files = {sortedFileList: [], changeFilesByPath: {}};
@@ -1056,8 +1065,6 @@
const promises: Promise<unknown>[] = [];
- promises.push(this._getDiffPreferences());
-
if (!this._change) promises.push(this._getChangeDetail(this._changeNum));
if (!this._changeComments) this._loadComments(value.patchNum);
@@ -1126,17 +1133,6 @@
});
}
- _changeViewStateChanged(changeViewState: Partial<ChangeViewState>) {
- if (changeViewState.diffMode === null) {
- // If screen size is small, always default to unified view.
- this.restApiService.getPreferences().then(prefs => {
- if (prefs) {
- this.set('changeViewState.diffMode', prefs.default_diff_view);
- }
- });
- }
- }
-
@observe('_path', '_prefs', '_reviewedFiles', '_patchRange')
_setReviewedObserver(
path?: string,
@@ -1351,29 +1347,6 @@
this.$.diffPreferencesDialog.open();
}
- /**
- * _getDiffViewMode: Get the diff view (side-by-side or unified) based on
- * the current state.
- *
- * The expected behavior is to use the mode specified in the user's
- * preferences unless they have manually chosen the alternative view or they
- * are on a mobile device. If the user navigates up to the change view, it
- * should clear this choice and revert to the preference the next time a
- * diff is viewed.
- *
- * Use side-by-side if the user is not logged in.
- */
- _getDiffViewMode() {
- if (this.changeViewState.diffMode) {
- return this.changeViewState.diffMode;
- } else if (this._userPrefs) {
- this.set('changeViewState.diffMode', this._userPrefs.default_diff_view);
- return this._userPrefs.default_diff_view;
- } else {
- return 'SIDE_BY_SIDE';
- }
- }
-
_computeModeSelectHideClass(diff?: DiffInfo) {
return !diff || diff.binary ? 'hide' : '';
}
@@ -1744,7 +1717,7 @@
}
_handleReloadingDiffPreference() {
- this._getDiffPreferences();
+ this.userService.getDiffPreferences();
}
_computeCanEdit(
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_html.ts b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_html.ts
index 308c353..16adb45 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_html.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_html.ts
@@ -339,7 +339,6 @@
<gr-diff-mode-selector
id="modeSelect"
save-on-change="[[_loggedIn]]"
- mode="{{changeViewState.diffMode}}"
show-tooltip-below=""
></gr-diff-mode-selector>
</div>
@@ -409,7 +408,6 @@
path="[[_path]]"
prefs="[[_prefs]]"
project-name="[[_change.project]]"
- view-mode="[[_diffMode]]"
is-blame-loaded="{{_isBlameLoaded}}"
on-comment-anchor-tap="_onLineSelected"
on-line-selected="_onLineSelected"
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js
index e4a8aa4..46ec5d0 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js
@@ -18,8 +18,8 @@
import '../../../test/common-test-setup-karma.js';
import './gr-diff-view.js';
import {GerritNav} from '../../core/gr-navigation/gr-navigation.js';
-import {ChangeStatus} from '../../../constants/constants.js';
-import {stubRestApi} from '../../../test/test-utils.js';
+import {ChangeStatus, DiffViewMode} from '../../../constants/constants.js';
+import {stubRestApi, stubUsers} from '../../../test/test-utils.js';
import {ChangeComments} from '../gr-comment-api/gr-comment-api.js';
import {GerritView} from '../../../services/router/router-model.js';
import {
@@ -30,11 +30,10 @@
import {EditPatchSetNum} from '../../../types/common.js';
import {CursorMoveResult} from '../../../api/core.js';
import {EventType} from '../../../types/events.js';
+import {_testOnly_resetState, _testOnly_setState} from '../../../services/browser/browser-model.js';
const basicFixture = fixtureFromElement('gr-diff-view');
-const blankFixture = fixtureFromElement('div');
-
suite('gr-diff-view tests', () => {
suite('basic tests', () => {
let element;
@@ -70,6 +69,8 @@
stubRestApi('getDiffDrafts').returns(Promise.resolve({}));
stubRestApi('getPortedComments').returns(Promise.resolve({}));
+ _testOnly_resetState();
+
element = basicFixture.instantiate();
element._changeNum = '42';
element._path = 'some/path.txt';
@@ -330,6 +331,7 @@
assert.isFalse(getDiffChangeDetailStub.called);
sinon.stub(element.reporting, 'diffViewDisplayed');
sinon.stub(element, '_loadBlame');
+ sinon.stub(element, '_pathChanged');
sinon.stub(element.$.diffHost, 'reload').returns(Promise.resolve());
sinon.spy(element, '_paramsChanged');
element._change = undefined;
@@ -432,6 +434,7 @@
test('keyboard shortcuts', () => {
element._changeNum = '42';
+ _testOnly_setState({screenWidth: 0});
element._patchRange = {
basePatchNum: PARENT,
patchNum: 10,
@@ -510,11 +513,11 @@
'_computeContainerClass');
MockInteractions.pressAndReleaseKeyOn(element, 74, null, 'j');
assert(computeContainerClassStub.lastCall.calledWithExactly(
- false, 'SIDE_BY_SIDE', true));
+ false, DiffViewMode.SIDE_BY_SIDE, true));
MockInteractions.pressAndReleaseKeyOn(element, 27, null, 'Escape');
assert(computeContainerClassStub.lastCall.calledWithExactly(
- false, 'SIDE_BY_SIDE', false));
+ false, DiffViewMode.SIDE_BY_SIDE, false));
// Note that stubbing _setReviewed means that the value of the
// `element.$.reviewed` checkbox is not flipped.
@@ -991,7 +994,9 @@
});
suite('diff prefs hidden', () => {
- test('whenlogged out', () => {
+ test('when no prefs or logged out', () => {
+ element._prefs = undefined;
+ element.disableDiffPrefs = false;
element._loggedIn = false;
flush();
assert.isTrue(element.$.diffPrefsContainer.hidden);
@@ -1308,47 +1313,23 @@
test('diff mode selector correctly toggles the diff', () => {
const select = element.$.modeSelect;
const diffDisplay = element.$.diffHost;
- element._userPrefs = {default_diff_view: 'SIDE_BY_SIDE'};
+ element._userPrefs = {diff_view: DiffViewMode.SIDE_BY_SIDE};
+ _testOnly_setState({screenWidth: 0});
+ const userStub = stubUsers('updatePreferences');
+
+ flush();
// The mode selected in the view state reflects the selected option.
- assert.equal(element._getDiffViewMode(), select.mode);
+ // assert.equal(element._userPrefs.diff_view, select.mode);
// The mode selected in the view state reflects the view rednered in the
// diff.
assert.equal(select.mode, diffDisplay.viewMode);
// We will simulate a user change of the selected mode.
- const newMode = 'UNIFIED_DIFF';
-
- // Set the mode, and simulate the change event.
- element.set('changeViewState.diffMode', newMode);
-
- // Make sure the handler was called and the state is still coherent.
- assert.equal(element._getDiffViewMode(), newMode);
- assert.equal(element._getDiffViewMode(), select.mode);
- assert.equal(element._getDiffViewMode(), diffDisplay.viewMode);
- });
-
- test('diff mode selector initializes from preferences', () => {
- let resolvePrefs;
- const prefsPromise = new Promise(resolve => {
- resolvePrefs = resolve;
- });
- stubRestApi('getPreferences')
- .callsFake(() => prefsPromise);
-
- // Attach a new gr-diff-view so we can intercept the preferences fetch.
- const view = document.createElement('gr-diff-view');
- blankFixture.instantiate().appendChild(view);
- flush();
-
- // At this point the diff mode doesn't yet have the user's preference.
- assert.equal(view._getDiffViewMode(), 'SIDE_BY_SIDE');
-
- // Receive the overriding preference.
- resolvePrefs({default_diff_view: 'UNIFIED'});
- flush();
- assert.equal(element._getDiffViewMode(), 'SIDE_BY_SIDE');
+ element._handleToggleDiffMode();
+ assert.isTrue(userStub.calledWithExactly({
+ diff_view: DiffViewMode.UNIFIED}));
});
test('diff mode selector should be hidden for binary', async () => {
@@ -1509,32 +1490,22 @@
assert.isTrue(getUrlStub.lastCall.args[6]);
});
- test('_getDiffViewMode', () => {
- // No user prefs or change view state set.
- assert.equal(element._getDiffViewMode(), 'SIDE_BY_SIDE');
-
- // User prefs but no change view state set.
- element.changeViewState.diffMode = undefined;
- element._userPrefs = {default_diff_view: 'UNIFIED_DIFF'};
- assert.equal(element._getDiffViewMode(), 'UNIFIED_DIFF');
-
- // User prefs and change view state set.
- element.changeViewState = {diffMode: 'SIDE_BY_SIDE'};
- assert.equal(element._getDiffViewMode(), 'SIDE_BY_SIDE');
- });
-
test('_handleToggleDiffMode', () => {
+ const userStub = stubUsers('updatePreferences');
const e = new CustomEvent('keydown', {
detail: {keyboardEvent: new KeyboardEvent('keydown'), key: 'x'},
});
- // Initial state.
- assert.equal(element._getDiffViewMode(), 'SIDE_BY_SIDE');
+ element._userPrefs = {diff_view: DiffViewMode.SIDE_BY_SIDE};
element._handleToggleDiffMode(e);
- assert.equal(element._getDiffViewMode(), 'UNIFIED_DIFF');
+ assert.deepEqual(userStub.lastCall.args[0], {
+ diff_view: DiffViewMode.UNIFIED});
+
+ element._userPrefs = {diff_view: DiffViewMode.UNIFIED};
element._handleToggleDiffMode(e);
- assert.equal(element._getDiffViewMode(), 'SIDE_BY_SIDE');
+ assert.deepEqual(userStub.lastCall.args[0], {
+ diff_view: DiffViewMode.SIDE_BY_SIDE});
});
suite('_initPatchRange', () => {
diff --git a/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select.ts b/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select.ts
index 0d6cadc..8e18473 100644
--- a/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select.ts
+++ b/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select.ts
@@ -21,7 +21,7 @@
import {dom, EventApi} from '@polymer/polymer/lib/legacy/polymer.dom';
import {PolymerElement} from '@polymer/polymer/polymer-element';
import {htmlTemplate} from './gr-patch-range-select_html';
-import {pluralize} from '../../../utils/string-util';
+import {convertToString, pluralize} from '../../../utils/string-util';
import {appContext} from '../../../services/app-context';
import {
computeLatestPatchNum,
@@ -74,6 +74,15 @@
};
}
+declare global {
+ interface HTMLElementEventMap {
+ 'value-change': DropDownValueChangeEvent;
+ }
+ interface HTMLElementTagNameMap {
+ 'gr-patch-range-select': GrPatchRangeSelect;
+ }
+}
+
/**
* Fired when the patch range changes
*
@@ -381,7 +390,10 @@
);
const commentThreadString = pluralize(commentThreadCount, 'comment');
- const unresolvedCount = changeComments.computeUnresolvedNum({patchNum});
+ const unresolvedCount = changeComments.computeUnresolvedNum(
+ {patchNum},
+ true
+ );
const unresolvedString =
unresolvedCount === 0 ? '' : `${unresolvedCount} unresolved`;
@@ -456,10 +468,8 @@
new CustomEvent('patch-range-change', {detail, bubbles: false})
);
}
-}
-declare global {
- interface HTMLElementTagNameMap {
- 'gr-patch-range-select': GrPatchRangeSelect;
+ convertToString(value?: unknown) {
+ return convertToString(value);
}
}
diff --git a/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select_html.ts b/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select_html.ts
index 26944a4..b268863 100644
--- a/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select_html.ts
+++ b/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select_html.ts
@@ -51,7 +51,7 @@
<span class="patchRange" aria-label="patch range starts with">
<gr-dropdown-list
id="basePatchDropdown"
- value="[[basePatchNum]]"
+ value="[[convertToString(basePatchNum)]]"
on-value-change="_handlePatchChange"
items="[[_baseDropdownContent]]"
>
@@ -68,7 +68,7 @@
<span class="patchRange" aria-label="patch range ends with">
<gr-dropdown-list
id="patchNumDropdown"
- value="[[patchNum]]"
+ value="[[convertToString(patchNum)]]"
on-value-change="_handlePatchChange"
items="[[_patchDropdownContent]]"
>
diff --git a/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select_test.js b/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select_test.js
index 0fe1fe2..28ebbac 100644
--- a/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select_test.js
+++ b/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select_test.js
@@ -351,15 +351,15 @@
],
abc: [],
// Patchset level comment does not contribute to the count
- [SpecialFilePath.PATCHSET_LEVEL_COMMENTS]: {
+ [SpecialFilePath.PATCHSET_LEVEL_COMMENTS]: [{
id: '27dcee4d_f7b77cfa',
message: 'test',
patch_set: 1,
unresolved: true,
updated: '2017-10-11 20:48:40.000000000',
- },
+ }],
};
- element.changeComments = new ChangeComments(comments, {}, {}, 123);
+ element.changeComments = new ChangeComments(comments);
assert.equal(element._computePatchSetCommentsString(
element.changeComments, 1), ' (3 comments, 1 unresolved)');
diff --git a/polygerrit-ui/app/elements/documentation/gr-documentation-search/gr-documentation-search.ts b/polygerrit-ui/app/elements/documentation/gr-documentation-search/gr-documentation-search.ts
index 3adb0f3..173a27e 100644
--- a/polygerrit-ui/app/elements/documentation/gr-documentation-search/gr-documentation-search.ts
+++ b/polygerrit-ui/app/elements/documentation/gr-documentation-search/gr-documentation-search.ts
@@ -14,28 +14,23 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-import '../../../styles/gr-table-styles';
-import '../../../styles/shared-styles';
import '../../shared/gr-list-view/gr-list-view';
-import {PolymerElement} from '@polymer/polymer/polymer-element';
-import {htmlTemplate} from './gr-documentation-search_html';
import {getBaseUrl} from '../../../utils/url-util';
-import {customElement, property} from '@polymer/decorators';
import {DocResult} from '../../../types/common';
import {fireTitleChange} from '../../../utils/event-util';
import {appContext} from '../../../services/app-context';
import {ListViewParams} from '../../gr-app-types';
+import {sharedStyles} from '../../../styles/shared-styles';
+import {tableStyles} from '../../../styles/gr-table-styles';
+import {LitElement, PropertyValues, html} from 'lit';
+import {customElement, property} from 'lit/decorators';
@customElement('gr-documentation-search')
-export class GrDocumentationSearch extends PolymerElement {
- static get template() {
- return htmlTemplate;
- }
-
+export class GrDocumentationSearch extends LitElement {
/**
* URL params passed from the router.
*/
- @property({type: Object, observer: '_paramsChanged'})
+ @property({type: Object})
params?: ListViewParams;
@property({type: Array})
@@ -54,7 +49,57 @@
fireTitleChange(this, 'Documentation Search');
}
- _paramsChanged(params: ListViewParams) {
+ static override get styles() {
+ return [sharedStyles, tableStyles];
+ }
+
+ override render() {
+ return html` <gr-list-view
+ .filter="${this._filter}"
+ .offset="${0}"
+ .loading="${this._loading}"
+ .path="/Documentation"
+ >
+ <table id="list" class="genericList">
+ <tbody>
+ <tr class="headerRow">
+ <th class="name topHeader">Name</th>
+ <th class="name topHeader"></th>
+ <th class="name topHeader"></th>
+ </tr>
+ <tr
+ id="loading"
+ class="loadingMsg ${this.computeLoadingClass(this._loading)}"
+ >
+ <td>Loading...</td>
+ </tr>
+ </tbody>
+ <tbody class="${this.computeLoadingClass(this._loading)}">
+ ${this._documentationSearches?.map(
+ search => html`
+ <tr class="table">
+ <td class="name">
+ <a href="${this._computeSearchUrl(search.url)}"
+ >${search.title}</a
+ >
+ </td>
+ <td></td>
+ <td></td>
+ </tr>
+ `
+ )}
+ </tbody>
+ </table>
+ </gr-list-view>`;
+ }
+
+ override updated(changedProperties: PropertyValues) {
+ if (changedProperties.has('params')) {
+ this._paramsChanged(this.params);
+ }
+ }
+
+ _paramsChanged(params?: ListViewParams) {
this._loading = true;
this._filter = params?.filter ?? '';
diff --git a/polygerrit-ui/app/elements/documentation/gr-documentation-search/gr-documentation-search_html.ts b/polygerrit-ui/app/elements/documentation/gr-documentation-search/gr-documentation-search_html.ts
deleted file mode 100644
index 95ce1ec..0000000
--- a/polygerrit-ui/app/elements/documentation/gr-documentation-search/gr-documentation-search_html.ts
+++ /dev/null
@@ -1,56 +0,0 @@
-/**
- * @license
- * Copyright (C) 2020 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-import {html} from '@polymer/polymer/lib/utils/html-tag';
-
-export const htmlTemplate = html`
- <style include="shared-styles">
- /* Workaround for empty style block - see https://github.com/Polymer/tools/issues/408 */
- </style>
- <style include="gr-table-styles">
- /* Workaround for empty style block - see https://github.com/Polymer/tools/issues/408 */
- </style>
- <gr-list-view
- filter="[[_filter]]"
- offset="0"
- loading="[[_loading]]"
- path="/Documentation"
- >
- <table id="list" class="genericList">
- <tbody>
- <tr class="headerRow">
- <th class="name topHeader">Name</th>
- <th class="name topHeader"></th>
- <th class="name topHeader"></th>
- </tr>
- <tr id="loading" class$="loadingMsg [[computeLoadingClass(_loading)]]">
- <td>Loading...</td>
- </tr>
- </tbody>
- <tbody class$="[[computeLoadingClass(_loading)]]">
- <template is="dom-repeat" items="[[_documentationSearches]]">
- <tr class="table">
- <td class="name">
- <a href$="[[_computeSearchUrl(item.url)]]">[[item.title]]</a>
- </td>
- <td></td>
- <td></td>
- </tr>
- </template>
- </tbody>
- </table>
- </gr-list-view>
-`;
diff --git a/polygerrit-ui/app/elements/documentation/gr-documentation-search/gr-documentation-search_test.ts b/polygerrit-ui/app/elements/documentation/gr-documentation-search/gr-documentation-search_test.ts
index bf6a0d5..47c83da 100644
--- a/polygerrit-ui/app/elements/documentation/gr-documentation-search/gr-documentation-search_test.ts
+++ b/polygerrit-ui/app/elements/documentation/gr-documentation-search/gr-documentation-search_test.ts
@@ -20,7 +20,7 @@
import {GrDocumentationSearch} from './gr-documentation-search';
import {page} from '../../../utils/page-wrapper-utils';
import 'lodash/lodash';
-import {stubRestApi} from '../../../test/test-utils';
+import {queryAndAssert, stubRestApi} from '../../../test/test-utils';
import {DocResult} from '../../../types/common';
import {ListViewParams} from '../../gr-app-types';
@@ -40,10 +40,11 @@
let value: ListViewParams;
- setup(() => {
+ setup(async () => {
sinon.stub(page, 'show');
element = basicFixture.instantiate();
counter = 0;
+ await flush();
});
suite('list with searches for documentation', () => {
@@ -87,13 +88,19 @@
test('correct contents are displayed', async () => {
assert.isTrue(element._loading);
assert.equal(element.computeLoadingClass(element._loading), 'loading');
- assert.equal(getComputedStyle(element.$.loading).display, 'block');
+ assert.equal(
+ getComputedStyle(queryAndAssert(element, '#loading')).display,
+ 'block'
+ );
element._loading = false;
await flush();
assert.equal(element.computeLoadingClass(element._loading), '');
- assert.equal(getComputedStyle(element.$.loading).display, 'none');
+ assert.equal(
+ getComputedStyle(queryAndAssert(element, '#loading')).display,
+ 'none'
+ );
});
});
});
diff --git a/polygerrit-ui/app/elements/gr-app-element.ts b/polygerrit-ui/app/elements/gr-app-element.ts
index d88eeaf..7f7749a 100644
--- a/polygerrit-ui/app/elements/gr-app-element.ts
+++ b/polygerrit-ui/app/elements/gr-app-element.ts
@@ -212,6 +212,8 @@
private readonly restApiService = appContext.restApiService;
+ private readonly browserService = appContext.browserService;
+
override keyboardShortcuts(): ShortcutListener[] {
return [
listen(Shortcut.OPEN_SHORTCUT_HELP_DIALOG, _ =>
@@ -252,6 +254,8 @@
this.handleRecreateView(GerritView.DIFF)
);
document.addEventListener(EventType.GR_RPC_LOG, e => this._handleRpcLog(e));
+ const resizeObserver = this.browserService.observeWidth();
+ resizeObserver.observe(this);
}
override ready() {
diff --git a/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.ts b/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.ts
index 127ac69..1ab469f 100644
--- a/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.ts
+++ b/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.ts
@@ -240,7 +240,6 @@
this.$.groupList.loadData(),
this.$.identities.loadData(),
this.$.editPrefs.loadData(),
- this.$.diffPrefs.loadData(),
];
// TODO(dhruvsri): move this to the service
diff --git a/polygerrit-ui/app/elements/shared/gr-account-entry/gr-account-entry.ts b/polygerrit-ui/app/elements/shared/gr-account-entry/gr-account-entry.ts
index df5f441..acb8348 100644
--- a/polygerrit-ui/app/elements/shared/gr-account-entry/gr-account-entry.ts
+++ b/polygerrit-ui/app/elements/shared/gr-account-entry/gr-account-entry.ts
@@ -62,9 +62,6 @@
@property({type: String})
placeholder = '';
- @property({type: Number})
- suggestFrom = 0;
-
@property({type: Object, notify: true})
querySuggestions: AutocompleteQuery = () => Promise.resolve([]);
diff --git a/polygerrit-ui/app/elements/shared/gr-account-entry/gr-account-entry_html.ts b/polygerrit-ui/app/elements/shared/gr-account-entry/gr-account-entry_html.ts
index c6c2b7f..d84ef62 100644
--- a/polygerrit-ui/app/elements/shared/gr-account-entry/gr-account-entry_html.ts
+++ b/polygerrit-ui/app/elements/shared/gr-account-entry/gr-account-entry_html.ts
@@ -28,7 +28,6 @@
id="input"
borderless="[[borderless]]"
placeholder="[[placeholder]]"
- threshold="[[suggestFrom]]"
query="[[querySuggestions]]"
allow-non-suggested-values="[[allowAnyInput]]"
on-commit="_handleInputCommit"
diff --git a/polygerrit-ui/app/elements/shared/gr-account-list/gr-account-list.ts b/polygerrit-ui/app/elements/shared/gr-account-list/gr-account-list.ts
index d97e38e..5449981 100644
--- a/polygerrit-ui/app/elements/shared/gr-account-list/gr-account-list.ts
+++ b/polygerrit-ui/app/elements/shared/gr-account-list/gr-account-list.ts
@@ -178,12 +178,6 @@
@property({type: Object})
_querySuggestions: (input: string) => Promise<SuggestionItem[]>;
- /**
- * Set to true to disable suggestions on empty input.
- */
- @property({type: Boolean})
- skipSuggestOnEmpty = false;
-
reporting: ReportingService;
private pendingRemoval: Set<AccountInput> = new Set();
@@ -206,17 +200,10 @@
}
_getSuggestions(input: string) {
- if (this.skipSuggestOnEmpty && !input) {
- return Promise.resolve([]);
- }
const provider = this.suggestionsProvider;
- if (!provider) {
- return Promise.resolve([]);
- }
+ if (!provider) return Promise.resolve([]);
return provider.getSuggestions(input).then(suggestions => {
- if (!suggestions) {
- return [];
- }
+ if (!suggestions) return [];
if (this.filter) {
suggestions = suggestions.filter(this.filter);
}
diff --git a/polygerrit-ui/app/elements/shared/gr-account-list/gr-account-list_test.ts b/polygerrit-ui/app/elements/shared/gr-account-list/gr-account-list_test.ts
index b667aba..23e5a72 100644
--- a/polygerrit-ui/app/elements/shared/gr-account-list/gr-account-list_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-account-list/gr-account-list_test.ts
@@ -398,53 +398,6 @@
assert.equal(makeSuggestionItemSpy.getCalls().length, 2);
});
- test('suggestion on empty', async () => {
- element.skipSuggestOnEmpty = false;
- const suggestions: Suggestion[] = [
- {
- email: 'abc@example.com' as EmailAddress,
- text: 'abcd',
- } as AccountInfo,
- {
- email: 'qwe@example.com' as EmailAddress,
- text: 'qwer',
- } as AccountInfo,
- ];
- const getSuggestionsStub = sinon
- .stub(suggestionsProvider, 'getSuggestions')
- .returns(Promise.resolve(suggestions));
-
- const makeSuggestionItemSpy = sinon.spy(
- suggestionsProvider,
- 'makeSuggestionItem'
- );
-
- const input = element.$.entry.$.input;
-
- input.text = '';
- MockInteractions.focus(input.$.input);
- input.noDebounce = true;
- await flush();
- assert.isTrue(getSuggestionsStub.calledOnce);
- assert.equal(getSuggestionsStub.lastCall.args[0], '');
- assert.equal(makeSuggestionItemSpy.getCalls().length, 2);
- });
-
- test('skip suggestion on empty', async () => {
- element.skipSuggestOnEmpty = true;
- const getSuggestionsStub = sinon
- .stub(suggestionsProvider, 'getSuggestions')
- .returns(Promise.resolve([]));
-
- const input = element.$.entry.$.input;
-
- input.text = '';
- MockInteractions.focus(input.$.input);
- input.noDebounce = true;
- await flush();
- assert.isTrue(getSuggestionsStub.notCalled);
- });
-
suite('allowAnyInput', () => {
setup(() => {
element.allowAnyInput = true;
diff --git a/polygerrit-ui/app/elements/shared/gr-button/gr-button.ts b/polygerrit-ui/app/elements/shared/gr-button/gr-button.ts
index 8dc23e2..ea5b5bb 100644
--- a/polygerrit-ui/app/elements/shared/gr-button/gr-button.ts
+++ b/polygerrit-ui/app/elements/shared/gr-button/gr-button.ts
@@ -88,7 +88,9 @@
background-color: var(--background-color);
color: var(--text-color);
display: flex;
- font-family: inherit;
+ font-family: var(--font-family, inherit);
+ /** Without this '.keyboard-focus' buttons will get bolded. */
+ font-weight: var(--font-weight-normal, inherit);
justify-content: center;
margin: var(--margin, 0);
min-width: var(--border, 0);
diff --git a/polygerrit-ui/app/elements/shared/gr-diff-preferences/gr-diff-preferences.ts b/polygerrit-ui/app/elements/shared/gr-diff-preferences/gr-diff-preferences.ts
index e560773..6ddaccf 100644
--- a/polygerrit-ui/app/elements/shared/gr-diff-preferences/gr-diff-preferences.ts
+++ b/polygerrit-ui/app/elements/shared/gr-diff-preferences/gr-diff-preferences.ts
@@ -24,6 +24,9 @@
import {DiffPreferencesInfo, IgnoreWhitespaceType} from '../../../types/diff';
import {GrSelect} from '../gr-select/gr-select';
import {appContext} from '../../../services/app-context';
+import {diffPreferences$} from '../../../services/user/user-model';
+import {takeUntil} from 'rxjs/operators';
+import {Subject} from 'rxjs';
export interface GrDiffPreferences {
$: {
@@ -39,7 +42,7 @@
contextSelect: GrSelect;
ignoreWhiteSpace: HTMLInputElement;
};
- save(): Promise<void>;
+ save(): void;
}
@customElement('gr-diff-preferences')
@@ -54,12 +57,22 @@
@property({type: Object})
diffPrefs?: DiffPreferencesInfo;
- private readonly restApiService = appContext.restApiService;
+ private readonly userService = appContext.userService;
- loadData() {
- return this.restApiService.getDiffPreferences().then(prefs => {
- this.diffPrefs = prefs;
- });
+ private readonly disconnected$ = new Subject();
+
+ override connectedCallback() {
+ super.connectedCallback();
+ diffPreferences$
+ .pipe(takeUntil(this.disconnected$))
+ .subscribe(diffPreferences => {
+ this.diffPrefs = diffPreferences;
+ });
+ }
+
+ override disconnectedCallback() {
+ this.disconnected$.next();
+ super.disconnectedCallback();
}
_handleDiffPrefsChanged() {
@@ -125,12 +138,10 @@
this._handleDiffPrefsChanged();
}
- save() {
- if (!this.diffPrefs)
- return Promise.reject(new Error('Missing diff preferences'));
- return this.restApiService.saveDiffPreferences(this.diffPrefs).then(_ => {
- this.hasUnsavedChanges = false;
- });
+ async save() {
+ if (!this.diffPrefs) return;
+ await this.userService.updateDiffPreference(this.diffPrefs);
+ this.hasUnsavedChanges = false;
}
/**
diff --git a/polygerrit-ui/app/elements/shared/gr-diff-preferences/gr-diff-preferences_test.ts b/polygerrit-ui/app/elements/shared/gr-diff-preferences/gr-diff-preferences_test.ts
index 6c1404e..41ac3e3 100644
--- a/polygerrit-ui/app/elements/shared/gr-diff-preferences/gr-diff-preferences_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-diff-preferences/gr-diff-preferences_test.ts
@@ -51,7 +51,6 @@
element = basicFixture.instantiate();
- await element.loadData();
await flush();
});
diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.ts b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.ts
index 63576c2..c2b0269 100644
--- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.ts
+++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.ts
@@ -149,7 +149,6 @@
createDefaultDiffPrefs,
createDefaultEditPrefs,
createDefaultPreferences,
- DiffViewMode,
HttpMethod,
ReviewerState,
} from '../../../constants/constants';
@@ -159,8 +158,6 @@
import {FlagsService, KnownExperimentId} from '../../../services/flags/flags';
const MAX_PROJECT_RESULTS = 25;
-// This value is somewhat arbitrary and not based on research or calculations.
-const MAX_UNIFIED_DEFAULT_WINDOW_WIDTH_PX = 850;
const Requests = {
SEND_DIFF_DRAFT: 'sendDiffDraft',
@@ -977,13 +974,6 @@
return res;
}
const prefInfo = res as unknown as PreferencesInfo;
- if (this._isNarrowScreen()) {
- // Note that this can be problematic, because the diff will stay
- // unified even after increasing the window width.
- prefInfo.default_diff_view = DiffViewMode.UNIFIED;
- } else {
- prefInfo.default_diff_view = prefInfo.diff_view;
- }
return prefInfo;
});
}
@@ -1019,10 +1009,6 @@
});
}
- _isNarrowScreen() {
- return window.innerWidth < MAX_UNIFIED_DEFAULT_WINDOW_WIDTH_PX;
- }
-
getChanges(
changesPerPage?: number,
query?: string,
@@ -1145,7 +1131,8 @@
_getChangesOptionsHex() {
if (
window.DEFAULT_DETAIL_HEXES &&
- window.DEFAULT_DETAIL_HEXES.dashboardPage
+ window.DEFAULT_DETAIL_HEXES.dashboardPage &&
+ !this.flagService?.isEnabled(KnownExperimentId.SUBMIT_REQUIREMENTS_UI)
) {
return window.DEFAULT_DETAIL_HEXES.dashboardPage;
}
@@ -1153,6 +1140,9 @@
ListChangesOption.LABELS,
ListChangesOption.DETAILED_ACCOUNTS,
];
+ if (this.flagService?.isEnabled(KnownExperimentId.SUBMIT_REQUIREMENTS_UI)) {
+ options.push(ListChangesOption.SUBMIT_REQUIREMENTS);
+ }
return listChangesOptionsToHex(...options);
}
diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.js b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.js
index a60a1ef..6a02985 100644
--- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.js
+++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.js
@@ -333,26 +333,23 @@
stub.lastCall.args[0].errFn({});
});
- const preferenceSetup = function(testJSON, loggedIn, smallScreen) {
+ const preferenceSetup = function(testJSON, loggedIn) {
sinon.stub(element, 'getLoggedIn')
.callsFake(() => Promise.resolve(loggedIn));
- sinon.stub(element, '_isNarrowScreen').callsFake(() => smallScreen);
sinon.stub(
element._restApiHelper,
'fetchCacheURL')
.callsFake(() => Promise.resolve(testJSON));
};
- test('getPreferences returns correctly on small screens logged in',
+ test('getPreferences returns correctly logged in',
() => {
const testJSON = {diff_view: 'SIDE_BY_SIDE'};
const loggedIn = true;
- const smallScreen = true;
- preferenceSetup(testJSON, loggedIn, smallScreen);
+ preferenceSetup(testJSON, loggedIn);
return element.getPreferences().then(obj => {
- assert.equal(obj.default_diff_view, 'UNIFIED_DIFF');
assert.equal(obj.diff_view, 'SIDE_BY_SIDE');
});
});
@@ -361,12 +358,10 @@
() => {
const testJSON = {diff_view: 'UNIFIED_DIFF'};
const loggedIn = true;
- const smallScreen = false;
- preferenceSetup(testJSON, loggedIn, smallScreen);
+ preferenceSetup(testJSON, loggedIn);
return element.getPreferences().then(obj => {
- assert.equal(obj.default_diff_view, 'UNIFIED_DIFF');
assert.equal(obj.diff_view, 'UNIFIED_DIFF');
});
});
@@ -375,12 +370,10 @@
() => {
const testJSON = {diff_view: 'UNIFIED_DIFF'};
const loggedIn = false;
- const smallScreen = false;
- preferenceSetup(testJSON, loggedIn, smallScreen);
+ preferenceSetup(testJSON, loggedIn);
return element.getPreferences().then(obj => {
- assert.equal(obj.default_diff_view, 'SIDE_BY_SIDE');
assert.equal(obj.diff_view, 'SIDE_BY_SIDE');
});
});
diff --git a/polygerrit-ui/app/mixins/hovercard-mixin/hovercard-mixin.ts b/polygerrit-ui/app/mixins/hovercard-mixin/hovercard-mixin.ts
index 793e5d6..fd4da4f 100644
--- a/polygerrit-ui/app/mixins/hovercard-mixin/hovercard-mixin.ts
+++ b/polygerrit-ui/app/mixins/hovercard-mixin/hovercard-mixin.ts
@@ -159,7 +159,12 @@
}
private addTargetEventListeners() {
- this._target?.addEventListener('mouseenter', this.debounceShow);
+ // We intentionally listen on 'mousemove' instead of 'mouseenter', because
+ // otherwise the target appearing under the mouse cursor would also
+ // trigger the hovercard, which can annoying for the user, for example
+ // when added reviewer chips appear in the reply dialog via keyboard
+ // interaction.
+ this._target?.addEventListener('mousemove', this.debounceShow);
this._target?.addEventListener('focus', this.debounceShow);
this._target?.addEventListener('mouseleave', this.debounceHide);
this._target?.addEventListener('blur', this.debounceHide);
@@ -167,7 +172,7 @@
}
private removeTargetEventListeners() {
- this._target?.removeEventListener('mouseenter', this.debounceShow);
+ this._target?.removeEventListener('mousemove', this.debounceShow);
this._target?.removeEventListener('focus', this.debounceShow);
this._target?.removeEventListener('mouseleave', this.debounceHide);
this._target?.removeEventListener('blur', this.debounceHide);
diff --git a/polygerrit-ui/app/mixins/hovercard-mixin/hovercard-mixin_test.ts b/polygerrit-ui/app/mixins/hovercard-mixin/hovercard-mixin_test.ts
index bd12789..dd86b38 100644
--- a/polygerrit-ui/app/mixins/hovercard-mixin/hovercard-mixin_test.ts
+++ b/polygerrit-ui/app/mixins/hovercard-mixin/hovercard-mixin_test.ts
@@ -130,12 +130,12 @@
test('card is scheduled to show on enter and hides on leave', async () => {
const button = document.querySelector('button');
const enterPromise = mockPromise();
- button!.addEventListener('mouseenter', () => enterPromise.resolve());
+ button!.addEventListener('mousemove', () => enterPromise.resolve());
const leavePromise = mockPromise();
button!.addEventListener('mouseleave', () => leavePromise.resolve());
assert.isFalse(element._isShowing);
- button!.dispatchEvent(new CustomEvent('mouseenter'));
+ button!.dispatchEvent(new CustomEvent('mousemove'));
await enterPromise;
await flush();
@@ -158,12 +158,12 @@
const button = document.querySelector('button');
const enterPromise = mockPromise();
const clickPromise = mockPromise();
- button!.addEventListener('mouseenter', () => enterPromise.resolve());
+ button!.addEventListener('mousemove', () => enterPromise.resolve());
button!.addEventListener('click', () => clickPromise.resolve());
assert.isFalse(element._isShowing);
- button!.dispatchEvent(new CustomEvent('mouseenter'));
+ button!.dispatchEvent(new CustomEvent('mousemove'));
await enterPromise;
await flush();
diff --git a/polygerrit-ui/app/services/app-context-init.ts b/polygerrit-ui/app/services/app-context-init.ts
index 3a6f7c5..b9c4f49 100644
--- a/polygerrit-ui/app/services/app-context-init.ts
+++ b/polygerrit-ui/app/services/app-context-init.ts
@@ -28,6 +28,7 @@
import {UserService} from './user/user-service';
import {CommentsService} from './comments/comments-service';
import {ShortcutsService} from './shortcuts/shortcuts-service';
+import {BrowserService} from './browser/browser-service';
type ServiceName = keyof AppContext;
type ServiceCreator<T> = () => T;
@@ -84,5 +85,6 @@
configService: () => new ConfigService(),
userService: () => new UserService(appContext.restApiService),
shortcutsService: () => new ShortcutsService(appContext.reportingService),
+ browserService: () => new BrowserService(),
});
}
diff --git a/polygerrit-ui/app/services/app-context.ts b/polygerrit-ui/app/services/app-context.ts
index e5828d6..47da722 100644
--- a/polygerrit-ui/app/services/app-context.ts
+++ b/polygerrit-ui/app/services/app-context.ts
@@ -27,6 +27,7 @@
import {UserService} from './user/user-service';
import {CommentsService} from './comments/comments-service';
import {ShortcutsService} from './shortcuts/shortcuts-service';
+import {BrowserService} from './browser/browser-service';
export interface AppContext {
flagsService: FlagsService;
@@ -41,6 +42,7 @@
storageService: StorageService;
configService: ConfigService;
userService: UserService;
+ browserService: BrowserService;
shortcutsService: ShortcutsService;
}
diff --git a/polygerrit-ui/app/services/browser/browser-model.ts b/polygerrit-ui/app/services/browser/browser-model.ts
new file mode 100644
index 0000000..db790f6
--- /dev/null
+++ b/polygerrit-ui/app/services/browser/browser-model.ts
@@ -0,0 +1,74 @@
+/**
+ * @license
+ * 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.
+ */
+
+import {BehaviorSubject, Observable, combineLatest} from 'rxjs';
+import {distinctUntilChanged, map} from 'rxjs/operators';
+import {preferenceDiffViewMode$} from '../user/user-model';
+import {DiffViewMode} from '../../api/diff';
+
+// This value is somewhat arbitrary and not based on research or calculations.
+const MAX_UNIFIED_DEFAULT_WINDOW_WIDTH_PX = 850;
+
+interface BrowserState {
+ /**
+ * We maintain the screen width in the state so that the app can react to
+ * changes in the width such as automatically changing to unified diff view
+ */
+ screenWidth?: number;
+}
+
+const initialState: BrowserState = {};
+
+// Mutable for testing
+let privateState$ = new BehaviorSubject(initialState);
+
+export function _testOnly_resetState() {
+ privateState$ = new BehaviorSubject(initialState);
+}
+
+export function _testOnly_setState(state: BrowserState) {
+ privateState$.next(state);
+}
+
+export function _testOnly_getState() {
+ return privateState$.getValue();
+}
+
+export const viewState$: Observable<BrowserState> = privateState$;
+
+export function updateStateScreenWidth(screenWidth: number) {
+ privateState$.next({...privateState$.getValue(), screenWidth});
+}
+
+export const isScreenTooSmall$ = viewState$.pipe(
+ map(
+ state =>
+ !!state.screenWidth &&
+ state.screenWidth < MAX_UNIFIED_DEFAULT_WINDOW_WIDTH_PX
+ ),
+ distinctUntilChanged()
+);
+
+export const diffViewMode$: Observable<DiffViewMode> = combineLatest([
+ isScreenTooSmall$,
+ preferenceDiffViewMode$,
+]).pipe(
+ map(([isScreenTooSmall, preferenceDiffViewMode]) => {
+ if (isScreenTooSmall) return DiffViewMode.UNIFIED;
+ else return preferenceDiffViewMode;
+ }, distinctUntilChanged())
+);
diff --git a/polygerrit-ui/app/services/browser/browser-service.ts b/polygerrit-ui/app/services/browser/browser-service.ts
new file mode 100644
index 0000000..d98f8f7
--- /dev/null
+++ b/polygerrit-ui/app/services/browser/browser-service.ts
@@ -0,0 +1,29 @@
+import {updateStateScreenWidth} from './browser-model';
+
+/**
+ * @license
+ * 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.
+ */
+
+export class BrowserService {
+ /* Observer the screen width so that the app can react to changes to it */
+ observeWidth() {
+ return new ResizeObserver(entries => {
+ entries.forEach(entry => {
+ updateStateScreenWidth(entry.contentRect.width);
+ });
+ });
+ }
+}
diff --git a/polygerrit-ui/app/services/shortcuts/shortcuts-service.ts b/polygerrit-ui/app/services/shortcuts/shortcuts-service.ts
index 19f12c5..a26fa08 100644
--- a/polygerrit-ui/app/services/shortcuts/shortcuts-service.ts
+++ b/polygerrit-ui/app/services/shortcuts/shortcuts-service.ts
@@ -28,6 +28,7 @@
Key,
Modifier,
Binding,
+ shouldSuppress,
} from '../../utils/dom-util';
import {ReportingService} from '../gr-reporting/gr-reporting';
@@ -154,34 +155,8 @@
shouldSuppress(e: KeyboardEvent) {
if (this.shortcutsDisabled) return true;
+ if (shouldSuppress(e)) return true;
- // Note that when you listen on document, then `e.currentTarget` will be the
- // document and `e.target` will be `<gr-app>` due to shadow dom, but by
- // using the composedPath() you can actually find the true origin of the
- // event.
- const rootTarget = e.composedPath()[0];
- if (!isElementTarget(rootTarget)) return false;
- const tagName = rootTarget.tagName;
- const type = rootTarget.getAttribute('type');
-
- if (
- // Suppress shortcuts on <input> and <textarea>, but not on
- // checkboxes, because we want to enable workflows like 'click
- // mark-reviewed and then press ] to go to the next file'.
- (tagName === 'INPUT' && type !== 'checkbox') ||
- tagName === 'TEXTAREA' ||
- // Suppress shortcuts if the key is 'enter'
- // and target is an anchor or button or paper-tab.
- (e.keyCode === 13 &&
- (tagName === 'A' || tagName === 'BUTTON' || tagName === 'PAPER-TAB'))
- ) {
- return true;
- }
- const path: EventTarget[] = e.composedPath() ?? [];
- for (const el of path) {
- if (!isElementTarget(el)) continue;
- if (el.tagName === 'GR-OVERLAY') return true;
- }
// eg: {key: "k:keydown", ..., from: "gr-diff-view"}
let key = `${e.key}:${e.type}`;
if (this.isInSpecificComboKeyMode(ComboKey.G)) key = 'g+' + key;
diff --git a/polygerrit-ui/app/services/user/user-model.ts b/polygerrit-ui/app/services/user/user-model.ts
index 72ce3e1..000887c 100644
--- a/polygerrit-ui/app/services/user/user-model.ts
+++ b/polygerrit-ui/app/services/user/user-model.ts
@@ -17,7 +17,11 @@
import {AccountDetailInfo, PreferencesInfo} from '../../types/common';
import {BehaviorSubject, Observable} from 'rxjs';
import {map, distinctUntilChanged} from 'rxjs/operators';
-import {createDefaultPreferences} from '../../constants/constants';
+import {
+ createDefaultPreferences,
+ createDefaultDiffPrefs,
+} from '../../constants/constants';
+import {DiffPreferencesInfo, DiffViewMode} from '../../api/diff';
interface UserState {
/**
@@ -25,13 +29,28 @@
*/
account?: AccountDetailInfo;
preferences: PreferencesInfo;
+ diffPreferences: DiffPreferencesInfo;
}
const initialState: UserState = {
preferences: createDefaultPreferences(),
+ diffPreferences: createDefaultDiffPrefs(),
};
-const privateState$ = new BehaviorSubject(initialState);
+// Mutable for testing
+let privateState$ = new BehaviorSubject(initialState);
+
+export function _testOnly_resetState() {
+ privateState$ = new BehaviorSubject(initialState);
+}
+
+export function _testOnly_setState(state: UserState) {
+ privateState$.next(state);
+}
+
+export function _testOnly_getState() {
+ return privateState$.getValue();
+}
// Re-exporting as Observable so that you can only subscribe, but not emit.
export const userState$: Observable<UserState> = privateState$;
@@ -46,6 +65,11 @@
privateState$.next({...current, preferences});
}
+export function updateDiffPreferences(diffPreferences: DiffPreferencesInfo) {
+ const current = privateState$.getValue();
+ privateState$.next({...current, diffPreferences});
+}
+
export const account$ = userState$.pipe(
map(userState => userState.account),
distinctUntilChanged()
@@ -56,11 +80,26 @@
distinctUntilChanged()
);
+export const diffPreferences$ = userState$.pipe(
+ map(userState => userState.diffPreferences),
+ distinctUntilChanged()
+);
+
+export const preferenceDiffViewMode$ = preferences$.pipe(
+ map(preference => preference.diff_view ?? DiffViewMode.SIDE_BY_SIDE),
+ distinctUntilChanged()
+);
+
export const myTopMenuItems$ = preferences$.pipe(
map(preferences => preferences?.my ?? []),
distinctUntilChanged()
);
+export const sizeBarInChangeTable$ = preferences$.pipe(
+ map(prefs => !!prefs?.size_bar_in_change_table),
+ distinctUntilChanged()
+);
+
export const disableShortcuts$ = preferences$.pipe(
map(preferences => preferences?.disable_keyboard_shortcuts ?? false),
distinctUntilChanged()
diff --git a/polygerrit-ui/app/services/user/user-service.ts b/polygerrit-ui/app/services/user/user-service.ts
index 125d20c..d08da8b 100644
--- a/polygerrit-ui/app/services/user/user-service.ts
+++ b/polygerrit-ui/app/services/user/user-service.ts
@@ -14,16 +14,21 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-import {
- AccountDetailInfo,
- PreferencesInfo,
- PreferencesInput,
-} from '../../types/common';
+import {AccountDetailInfo, PreferencesInfo} from '../../types/common';
import {from, of} from 'rxjs';
-import {account$, updateAccount, updatePreferences} from './user-model';
+import {
+ account$,
+ updateAccount,
+ updatePreferences,
+ updateDiffPreferences,
+} from './user-model';
import {switchMap} from 'rxjs/operators';
-import {createDefaultPreferences} from '../../constants/constants';
+import {
+ createDefaultPreferences,
+ createDefaultDiffPrefs,
+} from '../../constants/constants';
import {RestApiService} from '../gr-rest-api/gr-rest-api';
+import {DiffPreferencesInfo} from '../../types/diff';
export class UserService {
constructor(readonly restApiService: RestApiService) {
@@ -42,9 +47,19 @@
.subscribe((preferences?: PreferencesInfo) => {
updatePreferences(preferences ?? createDefaultPreferences());
});
+ account$
+ .pipe(
+ switchMap(account => {
+ if (!account) return of(createDefaultDiffPrefs());
+ return from(this.restApiService.getDiffPreferences());
+ })
+ )
+ .subscribe((diffPrefs?: DiffPreferencesInfo) => {
+ updateDiffPreferences(diffPrefs ?? createDefaultDiffPrefs());
+ });
}
- updatePreferences(prefs: PreferencesInput) {
+ updatePreferences(prefs: Partial<PreferencesInfo>) {
this.restApiService
.savePreferences(prefs)
.then((newPrefs: PreferencesInfo | undefined) => {
@@ -52,4 +67,23 @@
updatePreferences(newPrefs);
});
}
+
+ updateDiffPreference(diffPrefs: DiffPreferencesInfo) {
+ return this.restApiService
+ .saveDiffPreferences(diffPrefs)
+ .then((response: Response) => {
+ this.restApiService.getResponseObject(response).then(obj => {
+ const newPrefs = obj as unknown as DiffPreferencesInfo;
+ if (!newPrefs) return;
+ updateDiffPreferences(newPrefs);
+ });
+ });
+ }
+
+ getDiffPreferences() {
+ return this.restApiService.getDiffPreferences().then(prefs => {
+ if (!prefs) return;
+ updateDiffPreferences(prefs);
+ });
+ }
}
diff --git a/polygerrit-ui/app/test/test-utils.ts b/polygerrit-ui/app/test/test-utils.ts
index 557fe71..391d56f 100644
--- a/polygerrit-ui/app/test/test-utils.ts
+++ b/polygerrit-ui/app/test/test-utils.ts
@@ -25,6 +25,7 @@
import {ReportingService} from '../services/gr-reporting/gr-reporting';
import {CommentsService} from '../services/comments/comments-service';
import {UserService} from '../services/user/user-service';
+import {ShortcutsService} from '../services/shortcuts/shortcuts-service';
export {query, queryAll, queryAndAssert} from '../utils/common-util';
export interface MockPromise extends Promise<unknown> {
@@ -116,6 +117,10 @@
return sinon.stub(appContext.userService, method);
}
+export function stubShortcuts<K extends keyof ShortcutsService>(method: K) {
+ return sinon.stub(appContext.shortcutsService, method);
+}
+
export function stubStorage<K extends keyof StorageService>(method: K) {
return sinon.stub(appContext.storageService, method);
}
diff --git a/polygerrit-ui/app/types/common.ts b/polygerrit-ui/app/types/common.ts
index 1617aa3..3ac7c7b 100644
--- a/polygerrit-ui/app/types/common.ts
+++ b/polygerrit-ui/app/types/common.ts
@@ -1147,8 +1147,6 @@
work_in_progress_by_default?: boolean;
// The email_format doesn't mentioned in doc, but exists in Java class GeneralPreferencesInfo
email_format?: EmailFormat;
- // The following property doesn't exist in RestAPI, it is added by GrRestApiInterface
- default_diff_view?: DiffViewMode;
}
/**
diff --git a/polygerrit-ui/app/types/diff.ts b/polygerrit-ui/app/types/diff.ts
index 223f290..8146eb3 100644
--- a/polygerrit-ui/app/types/diff.ts
+++ b/polygerrit-ui/app/types/diff.ts
@@ -98,7 +98,7 @@
export interface DiffPreferencesInfo extends DiffPreferenceInfoApi {
expand_all_comments?: boolean;
- cursor_blink_rate: number;
+ cursor_blink_rate?: number;
manual_review?: boolean;
retain_header?: boolean;
skip_deleted?: boolean;
diff --git a/polygerrit-ui/app/types/types.ts b/polygerrit-ui/app/types/types.ts
index 2d4d412..b90b12f 100644
--- a/polygerrit-ui/app/types/types.ts
+++ b/polygerrit-ui/app/types/types.ts
@@ -189,7 +189,6 @@
showDownloadDialog: boolean;
diffMode: DiffViewMode | null;
numFilesShown: number | null;
- diffViewMode?: boolean;
}
export interface ChangeListViewState {
diff --git a/polygerrit-ui/app/utils/dom-util.ts b/polygerrit-ui/app/utils/dom-util.ts
index b6dece0..e2fa8fe 100644
--- a/polygerrit-ui/app/utils/dom-util.ts
+++ b/polygerrit-ui/app/utils/dom-util.ts
@@ -398,10 +398,16 @@
export function addShortcut(
element: HTMLElement,
shortcut: Binding,
- listener: (e: KeyboardEvent) => void
+ listener: (e: KeyboardEvent) => void,
+ options: {
+ shouldSuppress: boolean;
+ } = {
+ shouldSuppress: false,
+ }
) {
const wrappedListener = (e: KeyboardEvent) => {
if (e.repeat) return;
+ if (options.shouldSuppress && shouldSuppress(e)) return;
if (eventMatchesShortcut(e, shortcut)) {
listener(e);
}
@@ -417,3 +423,43 @@
export function shiftPressed(e: KeyboardEvent) {
return e.shiftKey;
}
+
+/**
+ * When you listen on keyboard events, then within Gerrit's web app you may want
+ * to avoid firing in certain common scenarios such as key strokes from <input>
+ * elements. But this can also be undesirable, for example Ctrl-Enter from
+ * <input> should trigger a save event.
+ *
+ * The shortcuts-service has a stateful method `shouldSuppress()` with
+ * reporting functionality, which delegates to here.
+ */
+export function shouldSuppress(e: KeyboardEvent): boolean {
+ // Note that when you listen on document, then `e.currentTarget` will be the
+ // document and `e.target` will be `<gr-app>` due to shadow dom, but by
+ // using the composedPath() you can actually find the true origin of the
+ // event.
+ const rootTarget = e.composedPath()[0];
+ if (!isElementTarget(rootTarget)) return false;
+ const tagName = rootTarget.tagName;
+ const type = rootTarget.getAttribute('type');
+
+ if (
+ // Suppress shortcuts on <input> and <textarea>, but not on
+ // checkboxes, because we want to enable workflows like 'click
+ // mark-reviewed and then press ] to go to the next file'.
+ (tagName === 'INPUT' && type !== 'checkbox') ||
+ tagName === 'TEXTAREA' ||
+ // Suppress shortcuts if the key is 'enter'
+ // and target is an anchor or button or paper-tab.
+ (e.keyCode === 13 &&
+ (tagName === 'A' || tagName === 'BUTTON' || tagName === 'PAPER-TAB'))
+ ) {
+ return true;
+ }
+ const path: EventTarget[] = e.composedPath() ?? [];
+ for (const el of path) {
+ if (!isElementTarget(el)) continue;
+ if (el.tagName === 'GR-OVERLAY') return true;
+ }
+ return false;
+}
diff --git a/polygerrit-ui/app/utils/dom-util_test.ts b/polygerrit-ui/app/utils/dom-util_test.ts
index 2993c0e..9dd5be2 100644
--- a/polygerrit-ui/app/utils/dom-util_test.ts
+++ b/polygerrit-ui/app/utils/dom-util_test.ts
@@ -22,6 +22,7 @@
getEventPath,
Modifier,
querySelectorAll,
+ shouldSuppress,
strToClassName,
} from './dom-util';
import {PolymerElement} from '@polymer/polymer/polymer-element';
@@ -29,6 +30,22 @@
import * as MockInteractions from '@polymer/iron-test-helpers/mock-interactions';
import {queryAndAssert} from '../test/test-utils';
+async function keyEventOn(
+ el: HTMLElement,
+ callback: (e: KeyboardEvent) => void,
+ keyCode = 75,
+ key = 'k'
+): Promise<KeyboardEvent> {
+ let resolve: (e: KeyboardEvent) => void;
+ const promise = new Promise<KeyboardEvent>(r => (resolve = r));
+ el.addEventListener('keydown', (e: KeyboardEvent) => {
+ callback(e);
+ resolve(e);
+ });
+ MockInteractions.keyDownOn(el, keyCode, null, key);
+ return await promise;
+}
+
class TestEle extends PolymerElement {
static get is() {
return 'dom-util-test-element';
@@ -266,4 +283,53 @@
assert.isTrue(eventMatchesShortcut(e, sShift));
});
});
+
+ suite('shouldSuppress', () => {
+ test('do not suppress shortcut event from <div>', async () => {
+ await keyEventOn(document.createElement('div'), e => {
+ assert.isFalse(shouldSuppress(e));
+ });
+ });
+
+ test('suppress shortcut event from <input>', async () => {
+ await keyEventOn(document.createElement('input'), e => {
+ assert.isTrue(shouldSuppress(e));
+ });
+ });
+
+ test('suppress shortcut event from <textarea>', async () => {
+ await keyEventOn(document.createElement('textarea'), e => {
+ assert.isTrue(shouldSuppress(e));
+ });
+ });
+
+ test('do not suppress shortcut event from checkbox <input>', async () => {
+ const inputEl = document.createElement('input');
+ inputEl.setAttribute('type', 'checkbox');
+ await keyEventOn(inputEl, e => {
+ assert.isFalse(shouldSuppress(e));
+ });
+ });
+
+ test('suppress shortcut event from children of <gr-overlay>', async () => {
+ const overlay = document.createElement('gr-overlay');
+ const div = document.createElement('div');
+ overlay.appendChild(div);
+ await keyEventOn(div, e => {
+ assert.isTrue(shouldSuppress(e));
+ });
+ });
+
+ test('suppress "enter" shortcut event from <a>', async () => {
+ await keyEventOn(document.createElement('a'), e => {
+ assert.isFalse(shouldSuppress(e));
+ });
+ await keyEventOn(
+ document.createElement('a'),
+ e => assert.isTrue(shouldSuppress(e)),
+ 13,
+ 'enter'
+ );
+ });
+ });
});
diff --git a/polygerrit-ui/app/utils/label-util.ts b/polygerrit-ui/app/utils/label-util.ts
index c50bbe2..9e9d0a3 100644
--- a/polygerrit-ui/app/utils/label-util.ts
+++ b/polygerrit-ui/app/utils/label-util.ts
@@ -204,19 +204,27 @@
return;
}
-export function extractAssociatedLabels(
- requirement: SubmitRequirementResultInfo
-): string[] {
+function extractLabelsFrom(expression: string) {
const pattern = new RegExp('label[0-9]*:([\\w-]+)', 'g');
const labels = [];
let match;
- while (
- (match = pattern.exec(
- requirement.submittability_expression_result.expression
- )) !== null
- ) {
+ while ((match = pattern.exec(expression)) !== null) {
labels.push(match[1]);
}
+ return labels;
+}
+
+export function extractAssociatedLabels(
+ requirement: SubmitRequirementResultInfo
+): string[] {
+ let labels = extractLabelsFrom(
+ requirement.submittability_expression_result.expression
+ );
+ if (requirement.override_expression_result) {
+ labels = labels.concat(
+ extractLabelsFrom(requirement.override_expression_result.expression)
+ );
+ }
return labels.filter(unique);
}
@@ -236,7 +244,7 @@
}
// TODO(milutin): This may be temporary for demo purposes
-const PRIORITY_REQUIREMENTS_ORDER: string[] = [
+export const PRIORITY_REQUIREMENTS_ORDER: string[] = [
StandardLabels.CODE_REVIEW,
StandardLabels.CODE_OWNERS,
StandardLabels.PRESUBMIT_VERIFIED,
diff --git a/polygerrit-ui/app/utils/label-util_test.ts b/polygerrit-ui/app/utils/label-util_test.ts
index 9360688..2d59294 100644
--- a/polygerrit-ui/app/utils/label-util_test.ts
+++ b/polygerrit-ui/app/utils/label-util_test.ts
@@ -256,5 +256,18 @@
const labels = extractAssociatedLabels(submitRequirement);
assert.deepEqual(labels, ['Verified', 'Code-Review']);
});
+ test('overridden label', () => {
+ const submitRequirement = {
+ ...createSubmitRequirementExpressionInfoWith(
+ 'label:Verified=MAX -label:Verified=MIN'
+ ),
+ override_expression_result: {
+ ...createSubmitRequirementExpressionInfo(),
+ expression: 'label:Build-cop-override',
+ },
+ };
+ const labels = extractAssociatedLabels(submitRequirement);
+ assert.deepEqual(labels, ['Verified', 'Build-cop-override']);
+ });
});
});
diff --git a/polygerrit-ui/app/utils/string-util.ts b/polygerrit-ui/app/utils/string-util.ts
index 43c0765..0b217ec 100644
--- a/polygerrit-ui/app/utils/string-util.ts
+++ b/polygerrit-ui/app/utils/string-util.ts
@@ -38,3 +38,12 @@
if (n % 10 === 3 && n % 100 !== 13) return `${n}rd`;
return `${n}th`;
}
+
+/**
+ * This converts any inputed value into string.
+ *
+ * This is so typescript checker doesn't fail.
+ */
+export function convertToString(key?: unknown) {
+ return key !== undefined ? String(key) : '';
+}
diff --git a/tools/BUILD b/tools/BUILD
index 47a2a2e..5d8491a 100644
--- a/tools/BUILD
+++ b/tools/BUILD
@@ -244,7 +244,7 @@
"-Xep:InvalidInlineTag:ERROR",
"-Xep:InvalidJavaTimeConstant:ERROR",
"-Xep:InvalidLink:ERROR",
- # "-Xep:InvalidParam:WARN",
+ "-Xep:InvalidParam:ERROR",
"-Xep:InvalidPatternSyntax:ERROR",
"-Xep:InvalidThrows:ERROR",
"-Xep:InvalidThrowsLink:ERROR",
diff --git a/tools/nongoogle.bzl b/tools/nongoogle.bzl
index b1ebb5c..51be39f 100644
--- a/tools/nongoogle.bzl
+++ b/tools/nongoogle.bzl
@@ -4,6 +4,8 @@
GUAVA_BIN_SHA1 = "00d0c3ce2311c9e36e73228da25a6e99b2ab826f"
+GUAVA_TESTLIB_BIN_SHA1 = "798c3827308605cd69697d8f1596a1735d3ef6e2"
+
GUAVA_DOC_URL = "https://google.github.io/guava/releases/" + GUAVA_VERSION + "/api/docs/"
TESTCONTAINERS_VERSION = "1.15.3"
@@ -147,6 +149,12 @@
sha1 = GUAVA_BIN_SHA1,
)
+ maven_jar(
+ name = "guava-testlib",
+ artifact = "com.google.guava:guava-testlib:" + GUAVA_VERSION,
+ sha1 = GUAVA_TESTLIB_BIN_SHA1,
+ )
+
GUICE_VERS = "5.0.1"
maven_jar(
