Merge "Make file list test slightly smaller" into stable-3.7
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index 234389e..be6e140 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -2543,6 +2543,26 @@
by the target version of the upgrade. Refer to the release notes and check whether
the rolling upgrade is possible or not and the associated constraints.
+[[gerrit.importedServerId]]gerrit.importedServerId::
++
+ServerId of the repositories imported from other Gerrit servers. Changes coming
+associated with the imported serverIds are indexed and displayed in the UI.
++
+Specify multiple `gerrit.importedServerId` for allowing the import from multiple
+Gerrit servers with different serverIds.
++
+[NOTE]
+The account-ids referenced in the imported changes are used for looking up the
+associated account-id locally, using the `imported:` external-id.
+Example: the account-id 1000 from the imported server-id 59a4964e-6376-4ed9-beef
+will be looked up in the local accounts using the `imported:1000@59a4964e-6376-4ed9-beef`
+external-id.
++
+If this value is not set, all changes imported from other Gerrit servers will be
+ignored.
++
+By default empty.
+
[[gerrit.serverId]]gerrit.serverId::
+
Used by NoteDb to, amongst other things, identify author identities from
diff --git a/java/com/google/gerrit/entities/Account.java b/java/com/google/gerrit/entities/Account.java
index 303e79f..85dbdeb 100644
--- a/java/com/google/gerrit/entities/Account.java
+++ b/java/com/google/gerrit/entities/Account.java
@@ -46,6 +46,10 @@
*/
@AutoValue
public abstract class Account {
+
+ /** Placeholder for indicating an account-id that does not correspond to any local account */
+ public static final Id UNKNOWN_ACCOUNT_ID = id(0);
+
public static Id id(int id) {
return new AutoValue_Account_Id(id);
}
diff --git a/java/com/google/gerrit/server/account/externalids/ExternalId.java b/java/com/google/gerrit/server/account/externalids/ExternalId.java
index 0a51171..1616198 100644
--- a/java/com/google/gerrit/server/account/externalids/ExternalId.java
+++ b/java/com/google/gerrit/server/account/externalids/ExternalId.java
@@ -135,6 +135,9 @@
/** Scheme used for GPG public keys. */
public static final String SCHEME_GPGKEY = "gpgkey";
+ /** Scheme for imported accounts from other servers with different GerritServerId */
+ public static final String SCHEME_IMPORTED = "imported";
+
/** Scheme for external auth used during authentication, e.g. OAuth Token */
public static final String SCHEME_EXTERNAL = "external";
diff --git a/java/com/google/gerrit/server/account/externalids/ExternalIdCache.java b/java/com/google/gerrit/server/account/externalids/ExternalIdCache.java
index 9f766d0..fe8feac 100644
--- a/java/com/google/gerrit/server/account/externalids/ExternalIdCache.java
+++ b/java/com/google/gerrit/server/account/externalids/ExternalIdCache.java
@@ -30,7 +30,7 @@
*
* <p>All returned collections are unmodifiable.
*/
-interface ExternalIdCache {
+public interface ExternalIdCache {
Optional<ExternalId> byKey(ExternalId.Key key) throws IOException;
ImmutableSet<ExternalId> byAccount(Account.Id accountId) throws IOException;
diff --git a/java/com/google/gerrit/server/config/GerritImportedServerIds.java b/java/com/google/gerrit/server/config/GerritImportedServerIds.java
new file mode 100644
index 0000000..c47d3be
--- /dev/null
+++ b/java/com/google/gerrit/server/config/GerritImportedServerIds.java
@@ -0,0 +1,29 @@
+// Copyright (C) 2022 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.config;
+
+import static java.lang.annotation.RetentionPolicy.RUNTIME;
+
+import com.google.inject.BindingAnnotation;
+import java.lang.annotation.Retention;
+
+/**
+ * List of ServerIds of the Gerrit data imported from other servers.
+ *
+ * <p>This values correspond to the {@code GerritServerId} of other servers.
+ */
+@Retention(RUNTIME)
+@BindingAnnotation
+public @interface GerritImportedServerIds {}
diff --git a/java/com/google/gerrit/server/config/GerritImportedServerIdsProvider.java b/java/com/google/gerrit/server/config/GerritImportedServerIdsProvider.java
new file mode 100644
index 0000000..f3f7645
--- /dev/null
+++ b/java/com/google/gerrit/server/config/GerritImportedServerIdsProvider.java
@@ -0,0 +1,37 @@
+// Copyright (C) 2022 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.config;
+
+import com.google.common.collect.ImmutableSet;
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import org.eclipse.jgit.lib.Config;
+
+public class GerritImportedServerIdsProvider implements Provider<ImmutableSet<String>> {
+ public static final String SECTION = "gerrit";
+ public static final String KEY = "importedServerId";
+
+ private final ImmutableSet<String> importedIds;
+
+ @Inject
+ public GerritImportedServerIdsProvider(@GerritServerConfig Config cfg) {
+ importedIds = ImmutableSet.copyOf(cfg.getStringList(SECTION, null, KEY));
+ }
+
+ @Override
+ public ImmutableSet<String> get() {
+ return importedIds;
+ }
+}
diff --git a/java/com/google/gerrit/server/notedb/AbstractChangeNotes.java b/java/com/google/gerrit/server/notedb/AbstractChangeNotes.java
index 158972f..3757244 100644
--- a/java/com/google/gerrit/server/notedb/AbstractChangeNotes.java
+++ b/java/com/google/gerrit/server/notedb/AbstractChangeNotes.java
@@ -17,6 +17,7 @@
import static java.util.Objects.requireNonNull;
import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.ImmutableSet;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.UsedAt;
import com.google.gerrit.entities.Change;
@@ -24,6 +25,7 @@
import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.metrics.Timer0;
import com.google.gerrit.server.config.AllUsersName;
+import com.google.gerrit.server.config.GerritImportedServerIds;
import com.google.gerrit.server.config.GerritServerId;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.notedb.ChangeNotesCommit.ChangeNotesRevWalk;
@@ -51,6 +53,7 @@
public final AllUsersName allUsers;
public final NoteDbMetrics metrics;
public final String serverId;
+ public final ImmutableSet<String> importedServerIds;
// Providers required to avoid dependency cycles.
@@ -64,7 +67,8 @@
ChangeNoteJson changeNoteJson,
NoteDbMetrics metrics,
Provider<ChangeNotesCache> cache,
- @GerritServerId String serverId) {
+ @GerritServerId String serverId,
+ @GerritImportedServerIds ImmutableSet<String> importedServerIds) {
this.failOnLoadForTest = new AtomicBoolean();
this.repoManager = repoManager;
this.allUsers = allUsers;
@@ -72,6 +76,7 @@
this.metrics = metrics;
this.cache = cache;
this.serverId = serverId;
+ this.importedServerIds = importedServerIds;
}
}
diff --git a/java/com/google/gerrit/server/notedb/ChangeNotes.java b/java/com/google/gerrit/server/notedb/ChangeNotes.java
index 26d5933..31934d5 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNotes.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNotes.java
@@ -406,6 +406,10 @@
return state.metaId();
}
+ public String getServerId() {
+ return state.serverId();
+ }
+
public ImmutableSortedMap<PatchSet.Id, PatchSet> getPatchSets() {
if (patchSets == null) {
ImmutableSortedMap.Builder<PatchSet.Id, PatchSet> b =
@@ -655,7 +659,9 @@
* be to bump the cache version, but that would invalidate all persistent cache entries, what we
* rather try to avoid.
*/
- if (!Strings.isNullOrEmpty(stateServerId) && !args.serverId.equals(stateServerId)) {
+ if (!Strings.isNullOrEmpty(stateServerId)
+ && !args.serverId.equals(stateServerId)
+ && !args.importedServerIds.contains(stateServerId)) {
throw new InvalidServerIdException(args.serverId, stateServerId);
}
diff --git a/java/com/google/gerrit/server/notedb/ChangeNotesCache.java b/java/com/google/gerrit/server/notedb/ChangeNotesCache.java
index 19044e6..0f2c877 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNotesCache.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNotesCache.java
@@ -26,6 +26,7 @@
import com.google.gerrit.proto.Protos;
import com.google.gerrit.server.ReviewerByEmailSet;
import com.google.gerrit.server.ReviewerSet;
+import com.google.gerrit.server.account.externalids.ExternalIdCache;
import com.google.gerrit.server.cache.CacheModule;
import com.google.gerrit.server.cache.proto.Cache.ChangeNotesKeyProto;
import com.google.gerrit.server.cache.serialize.CacheSerializer;
@@ -366,7 +367,13 @@
"Load change notes for change %s of project %s", key.changeId(), key.project());
ChangeNotesParser parser =
new ChangeNotesParser(
- key.changeId(), key.id(), walkSupplier.get(), args.changeNoteJson, args.metrics);
+ key.changeId(),
+ key.id(),
+ walkSupplier.get(),
+ args.changeNoteJson,
+ args.metrics,
+ args.serverId,
+ externalIdCache);
ChangeNotesState result = parser.parseAll();
// This assignment only happens if call() was actually called, which only
// happens when Cache#get(K, Callable<V>) incurs a cache miss.
@@ -377,11 +384,16 @@
private final Cache<Key, ChangeNotesState> cache;
private final Args args;
+ private final ExternalIdCache externalIdCache;
@Inject
- ChangeNotesCache(@Named(CACHE_NAME) Cache<Key, ChangeNotesState> cache, Args args) {
+ ChangeNotesCache(
+ @Named(CACHE_NAME) Cache<Key, ChangeNotesState> cache,
+ Args args,
+ ExternalIdCache externalIdCache) {
this.cache = cache;
this.args = args;
+ this.externalIdCache = externalIdCache;
}
Value get(
diff --git a/java/com/google/gerrit/server/notedb/ChangeNotesParser.java b/java/com/google/gerrit/server/notedb/ChangeNotesParser.java
index 6e5e19c..f2a659d 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNotesParser.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNotesParser.java
@@ -79,6 +79,7 @@
import com.google.gerrit.server.ReviewerByEmailSet;
import com.google.gerrit.server.ReviewerSet;
import com.google.gerrit.server.ReviewerStatusUpdate;
+import com.google.gerrit.server.account.externalids.ExternalIdCache;
import com.google.gerrit.server.notedb.ChangeNoteUtil.ParsedPatchSetApproval;
import com.google.gerrit.server.notedb.ChangeNotesCommit.ChangeNotesRevWalk;
import com.google.gerrit.server.util.LabelVote;
@@ -199,18 +200,24 @@
// the latest record unsets the field).
private Optional<PatchSet.Id> cherryPickOf;
private Instant mergedOn;
+ private final ExternalIdCache externalIdCache;
+ private final String gerritServerId;
ChangeNotesParser(
Change.Id changeId,
ObjectId tip,
ChangeNotesRevWalk walk,
ChangeNoteJson changeNoteJson,
- NoteDbMetrics metrics) {
+ NoteDbMetrics metrics,
+ String gerritServerId,
+ ExternalIdCache externalIdCache) {
this.id = changeId;
this.tip = tip;
this.walk = walk;
this.changeNoteJson = changeNoteJson;
this.metrics = metrics;
+ this.externalIdCache = externalIdCache;
+ this.gerritServerId = gerritServerId;
approvals = new LinkedHashMap<>();
bufferedApprovals = new ArrayList<>();
reviewers = HashBasedTable.create();
@@ -1406,7 +1413,7 @@
}
private Account.Id parseIdent(PersonIdent ident) throws ConfigInvalidException {
- return NoteDbUtil.parseIdent(ident)
+ return NoteDbUtil.parseIdent(ident, gerritServerId, externalIdCache)
.orElseThrow(
() -> parseException("cannot retrieve account id: %s", ident.getEmailAddress()));
}
diff --git a/java/com/google/gerrit/server/notedb/NoteDbUtil.java b/java/com/google/gerrit/server/notedb/NoteDbUtil.java
index 396e29b..64bf430 100644
--- a/java/com/google/gerrit/server/notedb/NoteDbUtil.java
+++ b/java/com/google/gerrit/server/notedb/NoteDbUtil.java
@@ -20,8 +20,12 @@
import com.google.common.primitives.Ints;
import com.google.gerrit.entities.Account;
import com.google.gerrit.extensions.restapi.RestModifyView;
+import com.google.gerrit.server.account.externalids.ExternalId;
+import com.google.gerrit.server.account.externalids.ExternalIdCache;
+import java.io.IOException;
import java.sql.Timestamp;
import java.util.Optional;
+import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.util.GitDateFormatter;
import org.eclipse.jgit.util.GitDateFormatter.Format;
@@ -48,6 +52,45 @@
return Optional.empty();
}
+ /**
+ * Returns an AccountId for the given email address and the current serverId. Reverse lookup the
+ * AccountId using the ExternalIdCache if the account has a foreign serverId.
+ *
+ * @param ident the accountId@serverId identity
+ * @param serverId the Gerrit's serverId
+ * @param externalIdCache reference to the cache for looking up the external ids
+ * @return a defined accountId if the account was found, {@link Account#UNKNOWN_ACCOUNT_ID} if the
+ * lookup via external-id did not return any account, or an empty value if the identity was
+ * malformed.
+ * @throws ConfigInvalidException when the lookup of the external-id failed
+ */
+ public static Optional<Account.Id> parseIdent(
+ PersonIdent ident, String serverId, ExternalIdCache externalIdCache)
+ throws ConfigInvalidException {
+ String email = ident.getEmailAddress();
+ int at = email.indexOf('@');
+ if (at >= 0) {
+ Integer id = Ints.tryParse(email.substring(0, at));
+ String accountServerId = email.substring(at + 1);
+ if (id != null) {
+ if (accountServerId.equals(serverId)) {
+ return Optional.of(Account.id(id));
+ }
+
+ ExternalId.Key extIdKey = ExternalId.Key.create(ExternalId.SCHEME_IMPORTED, email, false);
+ try {
+ return externalIdCache
+ .byKey(extIdKey)
+ .map(ExternalId::accountId)
+ .or(() -> Optional.of(Account.UNKNOWN_ACCOUNT_ID));
+ } catch (IOException e) {
+ throw new ConfigInvalidException("Unable to lookup external id from cache", e);
+ }
+ }
+ }
+ return Optional.empty();
+ }
+
public static String extractHostPartFromPersonIdent(PersonIdent ident) {
String email = ident.getEmailAddress();
int at = email.indexOf('@');
diff --git a/java/com/google/gerrit/server/schema/SchemaModule.java b/java/com/google/gerrit/server/schema/SchemaModule.java
index ff2073d..e0e64a3 100644
--- a/java/com/google/gerrit/server/schema/SchemaModule.java
+++ b/java/com/google/gerrit/server/schema/SchemaModule.java
@@ -16,6 +16,7 @@
import static com.google.inject.Scopes.SINGLETON;
+import com.google.common.collect.ImmutableSet;
import com.google.gerrit.extensions.config.FactoryModule;
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.GerritPersonIdentProvider;
@@ -25,9 +26,12 @@
import com.google.gerrit.server.config.AllUsersNameProvider;
import com.google.gerrit.server.config.AnonymousCowardName;
import com.google.gerrit.server.config.AnonymousCowardNameProvider;
+import com.google.gerrit.server.config.GerritImportedServerIds;
+import com.google.gerrit.server.config.GerritImportedServerIdsProvider;
import com.google.gerrit.server.config.GerritServerId;
import com.google.gerrit.server.config.GerritServerIdProvider;
import com.google.gerrit.server.index.group.GroupIndexCollection;
+import com.google.inject.TypeLiteral;
import org.eclipse.jgit.lib.PersonIdent;
/** Bindings for low-level Gerrit schema data. */
@@ -51,6 +55,11 @@
.toProvider(GerritServerIdProvider.class)
.in(SINGLETON);
+ bind(new TypeLiteral<ImmutableSet<String>>() {})
+ .annotatedWith(GerritImportedServerIds.class)
+ .toProvider(GerritImportedServerIdsProvider.class)
+ .in(SINGLETON);
+
// It feels wrong to have this binding in a seemingly unrelated module, but it's a dependency of
// SchemaCreatorImpl, so it's needed.
// TODO(dborowitz): Is there any way to untangle this?
diff --git a/java/com/google/gerrit/testing/InMemoryModule.java b/java/com/google/gerrit/testing/InMemoryModule.java
index b00cadb..b1ab6b1 100644
--- a/java/com/google/gerrit/testing/InMemoryModule.java
+++ b/java/com/google/gerrit/testing/InMemoryModule.java
@@ -19,6 +19,7 @@
import static com.google.inject.Scopes.SINGLETON;
import com.google.common.base.Strings;
+import com.google.common.collect.ImmutableSet;
import com.google.common.util.concurrent.ListeningExecutorService;
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
import com.google.gerrit.acceptance.testsuite.project.ProjectOperationsImpl;
@@ -61,6 +62,8 @@
import com.google.gerrit.server.config.FileBasedAllProjectsConfigProvider;
import com.google.gerrit.server.config.FileBasedGlobalPluginConfigProvider;
import com.google.gerrit.server.config.GerritGlobalModule;
+import com.google.gerrit.server.config.GerritImportedServerIds;
+import com.google.gerrit.server.config.GerritImportedServerIdsProvider;
import com.google.gerrit.server.config.GerritInstanceIdModule;
import com.google.gerrit.server.config.GerritInstanceNameModule;
import com.google.gerrit.server.config.GerritOptions;
@@ -311,6 +314,17 @@
@Provides
@Singleton
+ @GerritImportedServerIds
+ public ImmutableSet<String> createImportedServerIds() {
+ ImmutableSet<String> serverIds =
+ ImmutableSet.copyOf(
+ cfg.getStringList(
+ GerritServerIdProvider.SECTION, null, GerritImportedServerIdsProvider.KEY));
+ return serverIds;
+ }
+
+ @Provides
+ @Singleton
@SendEmailExecutor
public ExecutorService createSendEmailExecutor() {
return newDirectExecutorService();
diff --git a/javatests/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java b/javatests/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java
index d7c779e..1b286d1 100644
--- a/javatests/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java
+++ b/javatests/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java
@@ -18,6 +18,7 @@
import static java.util.concurrent.TimeUnit.SECONDS;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.Comment;
@@ -39,6 +40,8 @@
import com.google.gerrit.server.account.GroupBackend;
import com.google.gerrit.server.account.Realm;
import com.google.gerrit.server.account.ServiceUserClassifier;
+import com.google.gerrit.server.account.externalids.DisabledExternalIdCache;
+import com.google.gerrit.server.account.externalids.ExternalIdCache;
import com.google.gerrit.server.approval.PatchSetApprovalUuidGenerator;
import com.google.gerrit.server.approval.testing.TestPatchSetApprovalUuidGenerator;
import com.google.gerrit.server.config.AllUsersName;
@@ -48,11 +51,13 @@
import com.google.gerrit.server.config.CanonicalWebUrl;
import com.google.gerrit.server.config.DefaultUrlFormatter.DefaultUrlFormatterModule;
import com.google.gerrit.server.config.EnablePeerIPInReflogRecord;
+import com.google.gerrit.server.config.GerritImportedServerIds;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.config.GerritServerId;
import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
import com.google.gerrit.server.git.GitModule;
import com.google.gerrit.server.git.GitRepositoryManager;
+import com.google.gerrit.server.git.RepositoryCaseMismatchException;
import com.google.gerrit.server.group.SystemGroupBackend;
import com.google.gerrit.server.project.NullProjectCache;
import com.google.gerrit.server.project.ProjectCache;
@@ -67,9 +72,12 @@
import com.google.inject.Guice;
import com.google.inject.Inject;
import com.google.inject.Injector;
+import com.google.inject.Module;
+import com.google.inject.TypeLiteral;
import java.time.Instant;
import java.time.ZoneId;
import java.util.concurrent.ExecutorService;
+import org.eclipse.jgit.errors.RepositoryNotFoundException;
import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
import org.eclipse.jgit.junit.TestRepository;
import org.eclipse.jgit.lib.Config;
@@ -84,10 +92,13 @@
@Ignore
@RunWith(ConfigSuite.class)
public abstract class AbstractChangeNotesTest {
+ protected static final String LOCAL_SERVER_ID = "gerrit";
+
private static final ZoneId ZONE_ID = ZoneId.of("America/Los_Angeles");
@ConfigSuite.Parameter public Config testConfig;
+ protected Account.Id changeOwnerId;
protected Account.Id otherUserId;
protected FakeAccountCache accountCache;
protected IdentifiedUser changeOwner;
@@ -111,11 +122,21 @@
@Inject @GerritServerId protected String serverId;
+ @Inject protected ExternalIdCache externalIdCache;
+
protected Injector injector;
private String systemTimeZone;
@Before
public void setUpTestEnvironment() throws Exception {
+ setupTestPrerequisites();
+
+ injector = createTestInjector(LOCAL_SERVER_ID);
+ createAllUsers(injector);
+ injector.injectMembers(this);
+ }
+
+ protected void setupTestPrerequisites() throws Exception {
setTimeForTesting();
serverIdent = new PersonIdent("Gerrit Server", "noreply@gerrit.com", TimeUtil.now(), ZONE_ID);
@@ -134,60 +155,77 @@
ou.setPreferredEmail("other@account.com");
accountCache.put(ou.build());
assertableFanOutExecutor = new AssertableExecutorService();
-
- injector =
- Guice.createInjector(
- new FactoryModule() {
- @Override
- public void configure() {
- install(new GitModule());
-
- install(new DefaultUrlFormatterModule());
- install(NoteDbModule.forTest());
- bind(AllUsersName.class).toProvider(AllUsersNameProvider.class);
- bind(String.class).annotatedWith(GerritServerId.class).toInstance("gerrit");
- bind(GitRepositoryManager.class).toInstance(repoManager);
- bind(ProjectCache.class).to(NullProjectCache.class);
- bind(Config.class).annotatedWith(GerritServerConfig.class).toInstance(testConfig);
- bind(String.class)
- .annotatedWith(AnonymousCowardName.class)
- .toProvider(AnonymousCowardNameProvider.class);
- bind(String.class)
- .annotatedWith(CanonicalWebUrl.class)
- .toInstance("http://localhost:8080/");
- bind(Boolean.class)
- .annotatedWith(EnablePeerIPInReflogRecord.class)
- .toInstance(Boolean.FALSE);
- bind(Realm.class).to(FakeRealm.class);
- bind(GroupBackend.class).to(SystemGroupBackend.class).in(SINGLETON);
- bind(AccountCache.class).toInstance(accountCache);
- bind(PersonIdent.class)
- .annotatedWith(GerritPersonIdent.class)
- .toInstance(serverIdent);
- bind(GitReferenceUpdated.class).toInstance(GitReferenceUpdated.DISABLED);
- bind(MetricMaker.class).to(DisabledMetricMaker.class);
- bind(ExecutorService.class)
- .annotatedWith(FanOutExecutor.class)
- .toInstance(assertableFanOutExecutor);
- bind(ServiceUserClassifier.class).to(ServiceUserClassifier.NoOp.class);
- bind(InternalChangeQuery.class)
- .toProvider(
- () -> {
- throw new UnsupportedOperationException();
- });
- bind(PatchSetApprovalUuidGenerator.class)
- .to(TestPatchSetApprovalUuidGenerator.class);
- }
- });
-
- injector.injectMembers(this);
- repoManager.createRepository(allUsers);
- changeOwner = userFactory.create(co.id());
- otherUser = userFactory.create(ou.id());
- otherUserId = otherUser.getAccountId();
+ changeOwnerId = co.id();
+ otherUserId = ou.id();
internalUser = new InternalUser();
}
+ protected Injector createTestInjector(String serverId, String... importedServerIds)
+ throws Exception {
+ return createTestInjector(DisabledExternalIdCache.module(), serverId, importedServerIds);
+ }
+
+ protected Injector createTestInjector(
+ Module extraGuiceModule, String serverId, String... importedServerIds) throws Exception {
+
+ return Guice.createInjector(
+ new FactoryModule() {
+ @Override
+ public void configure() {
+ install(extraGuiceModule);
+ install(new GitModule());
+
+ install(new DefaultUrlFormatterModule());
+ install(NoteDbModule.forTest());
+ bind(AllUsersName.class).toProvider(AllUsersNameProvider.class);
+ bind(String.class).annotatedWith(GerritServerId.class).toInstance(serverId);
+ bind(new TypeLiteral<ImmutableSet<String>>() {})
+ .annotatedWith(GerritImportedServerIds.class)
+ .toInstance(new ImmutableSet.Builder<String>().add(importedServerIds).build());
+ bind(GitRepositoryManager.class).toInstance(repoManager);
+ bind(ProjectCache.class).to(NullProjectCache.class);
+ bind(Config.class).annotatedWith(GerritServerConfig.class).toInstance(testConfig);
+ bind(String.class)
+ .annotatedWith(AnonymousCowardName.class)
+ .toProvider(AnonymousCowardNameProvider.class);
+ bind(String.class)
+ .annotatedWith(CanonicalWebUrl.class)
+ .toInstance("http://localhost:8080/");
+ bind(Boolean.class)
+ .annotatedWith(EnablePeerIPInReflogRecord.class)
+ .toInstance(Boolean.FALSE);
+ bind(Realm.class).to(FakeRealm.class);
+ bind(GroupBackend.class).to(SystemGroupBackend.class).in(SINGLETON);
+ bind(AccountCache.class).toInstance(accountCache);
+ bind(PersonIdent.class).annotatedWith(GerritPersonIdent.class).toInstance(serverIdent);
+ bind(GitReferenceUpdated.class).toInstance(GitReferenceUpdated.DISABLED);
+ bind(MetricMaker.class).to(DisabledMetricMaker.class);
+ bind(ExecutorService.class)
+ .annotatedWith(FanOutExecutor.class)
+ .toInstance(assertableFanOutExecutor);
+ bind(ServiceUserClassifier.class).to(ServiceUserClassifier.NoOp.class);
+ bind(InternalChangeQuery.class)
+ .toProvider(
+ () -> {
+ throw new UnsupportedOperationException();
+ });
+ bind(PatchSetApprovalUuidGenerator.class).to(TestPatchSetApprovalUuidGenerator.class);
+ }
+ });
+ }
+
+ protected void createAllUsers(Injector injector)
+ throws RepositoryCaseMismatchException, RepositoryNotFoundException {
+ AllUsersName allUsersName = injector.getInstance(AllUsersName.class);
+
+ repoManager.createRepository(allUsersName);
+
+ IdentifiedUser.GenericFactory identifiedUserFactory =
+ injector.getInstance(IdentifiedUser.GenericFactory.class);
+ changeOwner = identifiedUserFactory.create(changeOwnerId);
+ otherUser = identifiedUserFactory.create(otherUserId);
+ }
+
private void setTimeForTesting() {
systemTimeZone = System.setProperty("user.timezone", "US/Eastern");
TestTimeUtil.resetWithClockStep(1, SECONDS);
@@ -200,8 +238,12 @@
}
protected Change newChange(boolean workInProgress) throws Exception {
+ return newChange(injector, workInProgress);
+ }
+
+ protected Change newChange(Injector injector, boolean workInProgress) throws Exception {
Change c = TestChanges.newChange(project, changeOwner.getAccountId());
- ChangeUpdate u = newUpdateForNewChange(c, changeOwner);
+ ChangeUpdate u = newUpdate(injector, c, changeOwner, false);
u.setChangeId(c.getKey().get());
u.setBranch(c.getDest().branch());
u.setWorkInProgress(workInProgress);
@@ -218,15 +260,20 @@
}
protected ChangeUpdate newUpdateForNewChange(Change c, CurrentUser user) throws Exception {
- return newUpdate(c, user, false);
+ return newUpdate(injector, c, user, false);
}
protected ChangeUpdate newUpdate(Change c, CurrentUser user) throws Exception {
- return newUpdate(c, user, true);
+ return newUpdate(injector, c, user, true);
}
protected ChangeUpdate newUpdate(Change c, CurrentUser user, boolean shouldExist)
throws Exception {
+ return newUpdate(injector, c, user, shouldExist);
+ }
+
+ protected ChangeUpdate newUpdate(
+ Injector injector, Change c, CurrentUser user, boolean shouldExist) throws Exception {
ChangeUpdate update = TestChanges.newUpdate(injector, c, user, shouldExist);
update.setPatchSetId(c.currentPatchSetId());
update.setAllowWriteToNewRef(true);
@@ -237,6 +284,10 @@
return new ChangeNotes(args, c, true, null).load();
}
+ protected ChangeNotes newNotes(AbstractChangeNotes.Args cArgs, Change c) {
+ return new ChangeNotes(cArgs, c, true, null).load();
+ }
+
protected static SubmitRecord submitRecord(
String status, String errorMessage, SubmitRecord.Label... labels) {
SubmitRecord rec = new SubmitRecord();
diff --git a/javatests/com/google/gerrit/server/notedb/ChangeNotesCommitTest.java b/javatests/com/google/gerrit/server/notedb/ChangeNotesCommitTest.java
index 666b8fc..9445f4a 100644
--- a/javatests/com/google/gerrit/server/notedb/ChangeNotesCommitTest.java
+++ b/javatests/com/google/gerrit/server/notedb/ChangeNotesCommitTest.java
@@ -98,7 +98,8 @@
private ChangeNotesParser newParser(ObjectId tip) throws Exception {
walk.reset();
ChangeNoteJson changeNoteJson = injector.getInstance(ChangeNoteJson.class);
- return new ChangeNotesParser(newChange().getId(), tip, walk, changeNoteJson, args.metrics);
+ return new ChangeNotesParser(
+ newChange().getId(), tip, walk, changeNoteJson, args.metrics, serverId, externalIdCache);
}
private RevCommit writeCommit(String body) throws Exception {
diff --git a/javatests/com/google/gerrit/server/notedb/ChangeNotesParserTest.java b/javatests/com/google/gerrit/server/notedb/ChangeNotesParserTest.java
index 276f093..4543b50 100644
--- a/javatests/com/google/gerrit/server/notedb/ChangeNotesParserTest.java
+++ b/javatests/com/google/gerrit/server/notedb/ChangeNotesParserTest.java
@@ -201,11 +201,11 @@
+ "Branch: refs/heads/master\n"
+ "Change-id: I577fb248e474018276351785930358ec0450e9f7\n"
+ "Patch-set: 1\n"
- + "Copied-Label: Label1=+1 Account <1@gerrit>,Other Account <2@Gerrit>\n"
+ + "Copied-Label: Label1=+1 Account <1@gerrit>,Other Account <2@gerrit>\n"
+ "Copied-Label: Label2=+1 Account <1@gerrit>\n"
- + "Copied-Label: Label3=+1 Account <1@gerrit>,Other Account <2@Gerrit> :\"tag\"\n"
- + "Copied-Label: Label4=+1 Account <1@Gerrit> :\"tag with characters %^#@^( *::!\"\n"
- + "Copied-Label: -Label1 Account <1@gerrit>,Other Account <2@Gerrit>\\n"
+ + "Copied-Label: Label3=+1 Account <1@gerrit>,Other Account <2@gerrit> :\"tag\"\n"
+ + "Copied-Label: Label4=+1 Account <1@gerrit> :\"tag with characters %^#@^( *::!\"\n"
+ + "Copied-Label: -Label1 Account <1@gerrit>,Other Account <2@gerrit>\\n"
+ "Copied-Label: -Label1 Account <1@gerrit>\n"
+ "Subject: This is a test change\n");
@@ -782,6 +782,7 @@
private ChangeNotesParser newParser(ObjectId tip) throws Exception {
walk.reset();
ChangeNoteJson changeNoteJson = injector.getInstance(ChangeNoteJson.class);
- return new ChangeNotesParser(newChange().getId(), tip, walk, changeNoteJson, args.metrics);
+ return new ChangeNotesParser(
+ newChange().getId(), tip, walk, changeNoteJson, args.metrics, serverId, externalIdCache);
}
}
diff --git a/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java b/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java
index 8def660..4edfa8b4 100644
--- a/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java
+++ b/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java
@@ -2084,7 +2084,13 @@
try (ChangeNotesRevWalk rw = ChangeNotesCommit.newRevWalk(repo)) {
ChangeNotesParser notesWithComments =
new ChangeNotesParser(
- c.getId(), commitWithComments.copy(), rw, changeNoteJson, args.metrics);
+ c.getId(),
+ commitWithComments.copy(),
+ rw,
+ changeNoteJson,
+ args.metrics,
+ serverId,
+ externalIdCache);
ChangeNotesState state = notesWithComments.parseAll();
assertThat(state.approvals()).isEmpty();
assertThat(state.publishedComments()).hasSize(1);
@@ -2093,7 +2099,13 @@
try (ChangeNotesRevWalk rw = ChangeNotesCommit.newRevWalk(repo)) {
ChangeNotesParser notesWithApprovals =
new ChangeNotesParser(
- c.getId(), commitWithApprovals.copy(), rw, changeNoteJson, args.metrics);
+ c.getId(),
+ commitWithApprovals.copy(),
+ rw,
+ changeNoteJson,
+ args.metrics,
+ serverId,
+ externalIdCache);
ChangeNotesState state = notesWithApprovals.parseAll();
assertThat(state.approvals()).hasSize(1);
diff --git a/javatests/com/google/gerrit/server/notedb/ImportedChangeNotesTest.java b/javatests/com/google/gerrit/server/notedb/ImportedChangeNotesTest.java
new file mode 100644
index 0000000..bb49a6d
--- /dev/null
+++ b/javatests/com/google/gerrit/server/notedb/ImportedChangeNotesTest.java
@@ -0,0 +1,114 @@
+// Copyright (C) 2022 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.notedb;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.testing.GerritJUnit.assertThrows;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import com.google.gerrit.entities.Account;
+import com.google.gerrit.entities.Change;
+import com.google.gerrit.server.account.externalids.ExternalId;
+import com.google.gerrit.server.account.externalids.ExternalIdCache;
+import com.google.gerrit.server.git.RepositoryCaseMismatchException;
+import com.google.inject.AbstractModule;
+import java.util.Optional;
+import org.eclipse.jgit.errors.RepositoryNotFoundException;
+import org.junit.Before;
+import org.junit.Test;
+
+public class ImportedChangeNotesTest extends AbstractChangeNotesTest {
+
+ private static final String FOREIGN_SERVER_ID = "foreign-server-id";
+ private static final String IMPORTED_SERVER_ID = "gerrit-imported-1";
+
+ private ExternalIdCache externalIdCacheMock;
+
+ @Before
+ @Override
+ public void setUpTestEnvironment() throws Exception {
+ setupTestPrerequisites();
+ }
+
+ private void initServerIds(String serverId, String... importedServerIds)
+ throws Exception, RepositoryCaseMismatchException, RepositoryNotFoundException {
+ externalIdCacheMock = mock(ExternalIdCache.class);
+ injector =
+ createTestInjector(
+ new AbstractModule() {
+ @Override
+ protected void configure() {
+ bind(ExternalIdCache.class).toInstance(externalIdCacheMock);
+ }
+ },
+ serverId,
+ importedServerIds);
+ injector.injectMembers(this);
+ createAllUsers(injector);
+ }
+
+ @Test
+ public void allowChangeFromImportedServerId() throws Exception {
+ initServerIds(LOCAL_SERVER_ID, IMPORTED_SERVER_ID);
+
+ ExternalId.Key importedAccountIdKey =
+ ExternalId.Key.create(
+ ExternalId.SCHEME_IMPORTED,
+ changeOwner.getAccountId() + "@" + IMPORTED_SERVER_ID,
+ false);
+ ExternalId importedAccountId =
+ ExternalId.create(importedAccountIdKey, changeOwner.getAccountId(), null, null, null);
+
+ when(externalIdCacheMock.byKey(eq(importedAccountIdKey)))
+ .thenReturn(Optional.of(importedAccountId));
+
+ Change importedChange = newChange(createTestInjector(IMPORTED_SERVER_ID), false);
+ Change localChange = newChange();
+
+ assertThat(newNotes(importedChange).getServerId()).isEqualTo(IMPORTED_SERVER_ID);
+ assertThat(newNotes(localChange).getServerId()).isEqualTo(LOCAL_SERVER_ID);
+ }
+
+ @Test
+ public void rejectChangeWithForeignServerId() throws Exception {
+ initServerIds(LOCAL_SERVER_ID);
+ when(externalIdCacheMock.byKey(any())).thenReturn(Optional.empty());
+
+ Change foreignChange = newChange(createTestInjector(FOREIGN_SERVER_ID), false);
+
+ InvalidServerIdException invalidServerIdEx =
+ assertThrows(InvalidServerIdException.class, () -> newNotes(foreignChange));
+
+ String invalidServerIdMessage = invalidServerIdEx.getMessage();
+ assertThat(invalidServerIdMessage).contains("expected " + LOCAL_SERVER_ID);
+ assertThat(invalidServerIdMessage).contains("actual: " + FOREIGN_SERVER_ID);
+ }
+
+ @Test
+ public void changeFromImportedServerIdWithUnknownAccountId() throws Exception {
+ initServerIds(LOCAL_SERVER_ID, IMPORTED_SERVER_ID);
+
+ when(externalIdCacheMock.byKey(any())).thenReturn(Optional.empty());
+
+ Change importedChange = newChange(createTestInjector(IMPORTED_SERVER_ID), false);
+ assertThat(newNotes(importedChange).getServerId()).isEqualTo(IMPORTED_SERVER_ID);
+
+ assertThat(newNotes(importedChange).getChange().getOwner())
+ .isEqualTo(Account.UNKNOWN_ACCOUNT_ID);
+ }
+}
diff --git a/polygerrit-ui/app/constants/constants.ts b/polygerrit-ui/app/constants/constants.ts
index 99f7606..bb7b313 100644
--- a/polygerrit-ui/app/constants/constants.ts
+++ b/polygerrit-ui/app/constants/constants.ts
@@ -270,6 +270,7 @@
change_table: [],
email_strategy: EmailStrategy.ATTENTION_SET_ONLY,
default_base_for_merges: DefaultBase.AUTO_MERGE,
+ allow_browser_notifications: false,
};
}
diff --git a/polygerrit-ui/app/elements/change-list/gr-create-change-help/gr-create-change-help.ts b/polygerrit-ui/app/elements/change-list/gr-create-change-help/gr-create-change-help.ts
index 0afb273..9c53fea 100644
--- a/polygerrit-ui/app/elements/change-list/gr-create-change-help/gr-create-change-help.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-create-change-help/gr-create-change-help.ts
@@ -4,7 +4,6 @@
* SPDX-License-Identifier: Apache-2.0
*/
import '../../shared/gr-button/gr-button';
-import '../../shared/gr-icons/gr-icons';
import {fireEvent} from '../../../utils/event-util';
import {sharedStyles} from '../../../styles/shared-styles';
import {LitElement, css, html} from 'lit';
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts
index 77a0eb2..87e9576 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts
@@ -8,7 +8,6 @@
import '../../shared/gr-dialog/gr-dialog';
import '../../shared/gr-dropdown/gr-dropdown';
import '../../shared/gr-icon/gr-icon';
-import '../../shared/gr-icons/gr-icons';
import '../../shared/gr-overlay/gr-overlay';
import '../gr-confirm-abandon-dialog/gr-confirm-abandon-dialog';
import '../gr-confirm-cherrypick-dialog/gr-confirm-cherrypick-dialog';
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 8293daa..1000925 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
@@ -15,7 +15,7 @@
import '../../shared/gr-change-star/gr-change-star';
import '../../shared/gr-change-status/gr-change-status';
import '../../shared/gr-editable-content/gr-editable-content';
-import '../../shared/gr-linked-text/gr-linked-text';
+import '../../shared/gr-formatted-text/gr-formatted-text';
import '../../shared/gr-overlay/gr-overlay';
import '../../shared/gr-tooltip-content/gr-tooltip-content';
import '../gr-change-actions/gr-change-actions';
@@ -104,12 +104,7 @@
import {GrIncludedInDialog} from '../gr-included-in-dialog/gr-included-in-dialog';
import {GrDownloadDialog} from '../gr-download-dialog/gr-download-dialog';
import {GrChangeMetadata} from '../gr-change-metadata/gr-change-metadata';
-import {
- assertIsDefined,
- assert,
- query as queryEl,
- queryAll,
-} from '../../../utils/common-util';
+import {assertIsDefined, assert, queryAll} from '../../../utils/common-util';
import {GrEditControls} from '../../edit/gr-edit-controls/gr-edit-controls';
import {
CommentThread,
@@ -121,14 +116,12 @@
import {GrFileList} from '../gr-file-list/gr-file-list';
import {EditRevisionInfo, ParsedChangeInfo} from '../../../types/types';
import {
- ChecksTabState,
CloseFixPreviewEvent,
EditableContentSaveEvent,
EventType,
OpenFixPreviewEvent,
ShowAlertEventDetail,
SwitchTabEvent,
- SwitchTabEventDetail,
TabState,
ValueChangedEvent,
} from '../../../types/events';
@@ -486,9 +479,12 @@
@state()
private changeViewAriaHidden = false;
+ /**
+ * This can be a string only for plugin provided tabs.
+ */
// visible for testing
@state()
- activeTab = Tab.FILES;
+ activeTab: Tab | string = Tab.FILES;
@property({type: Boolean})
unresolvedOnly = true;
@@ -702,6 +698,11 @@
);
subscribe(
this,
+ () => this.getViewModel().tab$,
+ t => (this.activeTab = t ?? Tab.FILES)
+ );
+ subscribe(
+ this,
() => this.getChecksModel().aPluginHasRegistered$,
b => {
this.showChecksTab = b;
@@ -959,7 +960,7 @@
/* Account for border and padding and rounding errors. */
max-width: calc(72ch + 2px + 2 * var(--spacing-m) + 0.4px);
}
- .commitMessage gr-linked-text {
+ .commitMessage gr-formatted-text {
word-break: break-word;
}
#commitMessageEditor {
@@ -1460,12 +1461,10 @@
.commitCollapsible=${this.computeCommitCollapsible()}
remove-zero-width-space=""
>
- <gr-linked-text
- pre=""
- .content=${this.latestCommitMessage}
- .config=${this.projectConfig?.commentlinks}
- remove-zero-width-space=""
- ></gr-linked-text>
+ <gr-formatted-text
+ .content=${this.latestCommitMessage ?? ''}
+ .markdown=${false}
+ ></gr-formatted-text>
</gr-editable-content>
</div>
<h3 class="assistive-tech-only">Comments and Checks Summary</h3>
@@ -1496,7 +1495,10 @@
private renderTabHeaders() {
return html`
- <paper-tabs id="tabs" @selected-changed=${this.setActiveTab}>
+ <paper-tabs
+ id="tabs"
+ @selected-changed=${this.onPaperTabSelectionChanged}
+ >
<paper-tab @click=${this.onPaperTabClick} data-name=${Tab.FILES}
><span>Files</span></paper-tab
>
@@ -1729,38 +1731,38 @@
}
}
- setActiveTab(e: SwitchTabEvent) {
- const paperTabs = queryEl<PaperTabsElement>(this, '#tabs');
- if (!paperTabs) return;
- const tabs = [...queryAll<HTMLElement>(paperTabs, 'paper-tab')];
+ onPaperTabSelectionChanged(e: ValueChangedEvent) {
+ if (!this.tabs) return;
+ const tabs = [...queryAll<HTMLElement>(this.tabs, 'paper-tab')];
if (!tabs) return;
- let tabName = e.detail.tab;
- let tabIndex = e.detail.value;
+ const tabIndex = Number(e.detail.value);
+ assert(
+ Number.isInteger(tabIndex) && 0 <= tabIndex && tabIndex < tabs.length,
+ `${tabIndex} must be integer`
+ );
+ const tab = tabs[tabIndex].dataset['name'];
- if (tabIndex === undefined) {
- assert(tabName !== undefined, 'tabName or tabIndex must be defined');
- tabIndex = tabs.findIndex(t => t.dataset['name'] === tabName);
- assert(tabIndex !== -1, `tab ${tabName} not found`);
+ this.getViewModel().updateState({tab});
+ }
+
+ setActiveTab(e: SwitchTabEvent) {
+ if (!this.tabs) return;
+ const tabs = [...queryAll<HTMLElement>(this.tabs, 'paper-tab')];
+ if (!tabs) return;
+
+ const tab = e.detail.tab;
+ const tabIndex = tabs.findIndex(t => t.dataset['name'] === tab);
+ assert(tabIndex !== -1, `tab ${tab} not found`);
+
+ if (this.tabs.selected !== tabIndex) {
+ this.tabs.selected = tabIndex;
}
- if (tabName === undefined) {
- tabName = tabs[tabIndex].dataset['name'];
- }
-
- if (paperTabs.selected !== tabIndex) {
- // paperTabs.selected is undefined during rendering
- if (paperTabs.selected !== undefined) {
- const src = (e.composedPath()?.[0] as Element | undefined)?.tagName;
- this.reporting.reportInteraction(Interaction.SHOW_TAB, {tabName, src});
- }
- paperTabs.selected = tabIndex;
- }
-
- this.activeTab = tabName as Tab;
+ this.getViewModel().updateState({tab});
if (e.detail.tabState) this.tabState = e.detail.tabState;
- if (e.detail.scrollIntoView) paperTabs.scrollIntoView({block: 'center'});
+ if (e.detail.scrollIntoView) this.tabs.scrollIntoView({block: 'center'});
}
/**
@@ -2246,19 +2248,7 @@
} else if (this.viewState?.commentId) {
tab = Tab.COMMENT_THREADS;
}
- const detail: SwitchTabEventDetail = {
- tab,
- };
- if (tab === Tab.CHECKS) {
- const state: ChecksTabState = {};
- detail.tabState = {checksTab: state};
- }
-
- this.setActiveTab(
- new CustomEvent(EventType.SHOW_TAB, {
- detail,
- })
- );
+ this.setActiveTab(new CustomEvent(EventType.SHOW_TAB, {detail: {tab}}));
}
// Private but used in tests.
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 ad84fb0..f3017f8 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
@@ -433,9 +433,7 @@
id="commitMessageEditor"
remove-zero-width-space=""
>
- <gr-linked-text pre="" remove-zero-width-space="">
- <span id="output" slot="insert"></span>
- </gr-linked-text>
+ <gr-formatted-text></gr-formatted-text>
</gr-editable-content>
</div>
<h3 class="assistive-tech-only">
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts
index b540c89..96a2aba 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts
@@ -854,7 +854,6 @@
.reviewerState=${ReviewerState.CC}
@account-added=${this.handleAccountAdded}
@accounts-changed=${this.handleCcsChanged}
- .removableValues=${this.change?.removable_reviewers}
.filter=${this.filterCCSuggestion}
.pendingConfirmation=${this.ccPendingConfirmation}
@pending-confirmation-changed=${this.handleCcsConfirmationChanged}
diff --git a/polygerrit-ui/app/elements/change/gr-submit-requirement-hovercard/gr-submit-requirement-hovercard.ts b/polygerrit-ui/app/elements/change/gr-submit-requirement-hovercard/gr-submit-requirement-hovercard.ts
index 7fe070c..66ad38c 100644
--- a/polygerrit-ui/app/elements/change/gr-submit-requirement-hovercard/gr-submit-requirement-hovercard.ts
+++ b/polygerrit-ui/app/elements/change/gr-submit-requirement-hovercard/gr-submit-requirement-hovercard.ts
@@ -30,7 +30,6 @@
import {ReviewInput} from '../../../types/common';
import {getAppContext} from '../../../services/app-context';
import {assertIsDefined} from '../../../utils/common-util';
-import {CURRENT} from '../../../utils/patch-set-util';
import {fireReload} from '../../../utils/event-util';
import {submitRequirementsStyles} from '../../../styles/gr-submit-requirements-styles';
import {
@@ -321,7 +320,11 @@
},
};
return this.restApiService
- .saveChangeReview(this.change._number, CURRENT, review)
+ .saveChangeReview(
+ this.change._number,
+ this.change.current_revision,
+ review
+ )
.then(() => {
fireReload(this, true);
});
diff --git a/polygerrit-ui/app/elements/change/gr-submit-requirement-hovercard/gr-submit-requirement-hovercard_test.ts b/polygerrit-ui/app/elements/change/gr-submit-requirement-hovercard/gr-submit-requirement-hovercard_test.ts
index 2117ec5..4e78e8a 100644
--- a/polygerrit-ui/app/elements/change/gr-submit-requirement-hovercard/gr-submit-requirement-hovercard_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-submit-requirement-hovercard/gr-submit-requirement-hovercard_test.ts
@@ -18,7 +18,7 @@
createSubmitRequirementResultInfo,
} from '../../../test/test-data-generators';
import {ParsedChangeInfo} from '../../../types/types';
-import {query, queryAndAssert} from '../../../test/test-utils';
+import {query, queryAndAssert, stubRestApi} from '../../../test/test-utils';
import {GrButton} from '../../shared/gr-button/gr-button';
import {ChangeStatus, SubmitRequirementResultInfo} from '../../../api/rest-api';
@@ -321,6 +321,22 @@
assert.isUndefined(query(element, '.quickApprove'));
});
+ test('uses patchset from change', async () => {
+ const saveChangeReview = stubRestApi('saveChangeReview').resolves();
+ const element = await fixture<GrSubmitRequirementHovercard>(
+ html`<gr-submit-requirement-hovercard
+ .requirement=${submitRequirement}
+ .change=${change}
+ .account=${account}
+ ></gr-submit-requirement-hovercard>`
+ );
+
+ queryAndAssert<GrButton>(element, '.quickApprove > gr-button').click();
+
+ assert.equal(saveChangeReview.callCount, 1);
+ assert.equal(saveChangeReview.firstCall.args[1], change.current_revision);
+ });
+
test('override button renders', async () => {
const submitRequirement: SubmitRequirementResultInfo = {
...createSubmitRequirementResultInfo(),
diff --git a/polygerrit-ui/app/elements/checks/gr-checks-results.ts b/polygerrit-ui/app/elements/checks/gr-checks-results.ts
index a4e26fe..848948f 100644
--- a/polygerrit-ui/app/elements/checks/gr-checks-results.ts
+++ b/polygerrit-ui/app/elements/checks/gr-checks-results.ts
@@ -77,6 +77,7 @@
import {DropdownItem} from '../shared/gr-dropdown-list/gr-dropdown-list';
import './gr-checks-attempt';
import {createDiffUrl} from '../../models/views/diff';
+import {changeViewModelToken} from '../../models/views/change';
/**
* Firing this event sets the regular expression of the results filter.
@@ -756,7 +757,7 @@
filterInput?: HTMLInputElement;
@state()
- filterRegExp = new RegExp('');
+ filterRegExp = '';
/** All runs. Shown should only the selected/filtered ones. */
@property({attribute: false})
@@ -766,8 +767,8 @@
* Check names of runs that are selected in the runs panel. When this array
* is empty, then no run is selected and all runs should be shown.
*/
- @property({attribute: false})
- selectedRuns: string[] = [];
+ @state()
+ selectedRuns: Set<string> = new Set();
@state()
actions: Action[] = [];
@@ -808,6 +809,8 @@
*/
private isSectionExpandedByUser = new Map<Category, boolean>();
+ private readonly getViewModel = resolve(this, changeViewModelToken);
+
private readonly getChangeModel = resolve(this, changeModelToken);
private readonly getChecksModel = resolve(this, checksModelToken);
@@ -853,6 +856,16 @@
() => this.getChecksModel().someProvidersAreLoadingSelected$,
x => (this.someProvidersAreLoading = x)
);
+ subscribe(
+ this,
+ () => this.getViewModel().checksRunsSelected$,
+ x => (this.selectedRuns = x)
+ );
+ subscribe(
+ this,
+ () => this.getViewModel().checksResultsFilter$,
+ x => (this.filterRegExp = x)
+ );
}
static override get styles() {
@@ -1051,6 +1064,9 @@
protected override updated(changedProperties: PropertyValues) {
super.updated(changedProperties);
+ if (changedProperties.has('filterRegExp') && this.filterInput) {
+ this.filterInput.value = this.filterRegExp;
+ }
if (changedProperties.has('tabState') && this.tabState) {
const {statusOrCategory, checkName} = this.tabState;
if (isCategory(statusOrCategory)) {
@@ -1224,11 +1240,10 @@
}
private handleFilter(e: ChecksResultsFilterEvent) {
- if (!this.filterInput) return;
- const oldValue = this.filterInput.value ?? '';
const newValue = e.detail.filterRegExp ?? '';
- this.filterInput.value = oldValue === newValue ? '' : newValue;
- this.onFilterInputChange();
+ this.getViewModel().updateState({
+ checksResultsFilter: this.filterRegExp === newValue ? '' : newValue,
+ });
}
private renderAction(action?: Action) {
@@ -1289,10 +1304,7 @@
}
isRunSelected(run: {checkName: string}) {
- return (
- this.selectedRuns.length === 0 ||
- this.selectedRuns.includes(run.checkName)
- );
+ return this.selectedRuns.size === 0 || this.selectedRuns.has(run.checkName);
}
renderFilter() {
@@ -1300,10 +1312,11 @@
run =>
this.isRunSelected(run) && isAttemptSelected(this.selectedAttempt, run)
);
- if (this.selectedRuns.length === 0 && allResults(runs).length <= 3) {
- if (this.filterRegExp.source.length > 0) {
- this.filterRegExp = new RegExp('');
- }
+ if (
+ this.selectedRuns.size === 0 &&
+ allResults(runs).length <= 3 &&
+ this.filterRegExp === ''
+ ) {
return;
}
return html`
@@ -1325,7 +1338,9 @@
{},
{deduping: Deduping.EVENT_ONCE_PER_CHANGE}
);
- this.filterRegExp = new RegExp(this.filterInput.value, 'i');
+ this.getViewModel().updateState({
+ checksResultsFilter: this.filterInput.value,
+ });
}
renderSection(category: Category) {
@@ -1342,18 +1357,32 @@
],
[]
);
- const isSelection = this.selectedRuns.length > 0;
+ const isSelectionActive = this.selectedRuns.size > 0;
const selected = all.filter(result => this.isRunSelected(result));
- const filtered = selected.filter(result =>
- matches(result, this.filterRegExp)
- );
+ const re = new RegExp(this.filterRegExp, 'i');
+ const filtered = selected.filter(result => matches(result, re));
+ const isFilterActiveWithResults =
+ this.filterRegExp !== '' && filtered.length > 0;
+
+ // The logic for deciding whether to expand a section by default is a bit
+ // complicated, but we want to collapse empty and info/success sections by
+ // default for a clean and focused user experience. However, as soon as the
+ // user starts selecting or filtering we must take this into account and
+ // prefer to expand the sections.
let expanded = this.isSectionExpanded.get(category);
const expandedByUser = this.isSectionExpandedByUser.get(category) ?? false;
if (!expandedByUser || expanded === undefined) {
- expanded = selected.length > 0 && (isWarningOrError || isSelection);
+ // Note that we are using `selected` for `isEmpty` and not `filtered`,
+ // because if the filter is what makes a section empty, then we want to
+ // show an expanded section, which contains a message about this.
+ const isEmpty = selected.length === 0;
+ expanded =
+ !isEmpty &&
+ (isWarningOrError || isSelectionActive || isFilterActiveWithResults);
this.isSectionExpanded.set(category, expanded);
}
const expandedClass = expanded ? 'expanded' : 'collapsed';
+
const isShowAll = this.isShowAll.get(category) ?? false;
const resultCount = filtered.length;
const empty = resultCount === 0 ? 'empty' : '';
diff --git a/polygerrit-ui/app/elements/checks/gr-checks-runs.ts b/polygerrit-ui/app/elements/checks/gr-checks-runs.ts
index 09a2558..128a9b0a 100644
--- a/polygerrit-ui/app/elements/checks/gr-checks-runs.ts
+++ b/polygerrit-ui/app/elements/checks/gr-checks-runs.ts
@@ -44,7 +44,7 @@
} from '../../models/checks/checks-fakes';
import {assertIsDefined} from '../../utils/common-util';
import {modifierPressed, whenVisible} from '../../utils/dom-util';
-import {fireRunSelected, fireRunSelectionReset} from './gr-checks-util';
+import {fireRunSelected, RunSelectedEvent} from './gr-checks-util';
import {ChecksTabState} from '../../types/events';
import {charsOnly} from '../../utils/string-util';
import {getAppContext} from '../../services/app-context';
@@ -57,6 +57,7 @@
import {Interaction} from '../../constants/reporting';
import {Deduping} from '../../api/reporting';
import {when} from 'lit/directives/when.js';
+import {changeViewModelToken} from '../../models/views/change';
@customElement('gr-checks-run')
export class GrChecksRun extends LitElement {
@@ -403,8 +404,8 @@
@property({type: Boolean, reflect: true})
collapsed = false;
- @property({attribute: false})
- selectedRuns: string[] = [];
+ @state()
+ selectedRuns: Set<string> = new Set();
@state()
selectedAttempt: AttemptChoice = LATEST_ATTEMPT;
@@ -424,6 +425,8 @@
private getChecksModel = resolve(this, checksModelToken);
+ private readonly getViewModel = resolve(this, changeViewModelToken);
+
private readonly reporting = getAppContext().reportingService;
constructor() {
@@ -453,6 +456,11 @@
() => this.getChecksModel().runFilterRegexp$,
x => (this.filterRegExp = x)
);
+ subscribe(
+ this,
+ () => this.getViewModel().checksRunsSelected$,
+ x => (this.selectedRuns = x)
+ );
this.addEventListener('click', () => {
if (this.collapsed) this.toggleCollapsed();
});
@@ -660,8 +668,8 @@
private renderTitleButtons() {
if (this.collapsed) return;
- if (this.selectedRuns.length < 2) return;
- const actions = this.selectedRuns.map(selected => {
+ if (this.selectedRuns.size < 2) return;
+ const actions = [...this.selectedRuns].map(selected => {
const run = this.runs.find(
run => run.isLatestAttempt && run.checkName === selected
);
@@ -676,7 +684,8 @@
<gr-button
class="font-normal"
link
- @click=${() => fireRunSelectionReset(this)}
+ @click=${() =>
+ this.getViewModel().updateState({checksRunsSelected: undefined})}
>Unselect All</gr-button
>
<gr-tooltip-content
@@ -820,16 +829,23 @@
}
renderRun(run: CheckRun) {
- const selectedRun = this.selectedRuns.includes(run.checkName);
- const deselected = !selectedRun && this.selectedRuns.length > 0;
+ const selectedRun = this.selectedRuns.has(run.checkName);
+ const deselected = !selectedRun && this.selectedRuns.size > 0;
return html`<gr-checks-run
.run=${run}
?condensed=${this.collapsed}
.selected=${selectedRun}
.deselected=${deselected}
+ @run-selected=${this.handleRunSelected}
></gr-checks-run>`;
}
+ handleRunSelected(e: RunSelectedEvent) {
+ if (e.detail.checkName) {
+ this.getViewModel().toggleSelectedCheckRun(e.detail.checkName);
+ }
+ }
+
showFilter(): boolean {
if (this.collapsed) return false;
return this.runs.length > 10 || !!this.filterRegExp;
diff --git a/polygerrit-ui/app/elements/checks/gr-checks-tab.ts b/polygerrit-ui/app/elements/checks/gr-checks-tab.ts
index 82893bc..c1bcdc1 100644
--- a/polygerrit-ui/app/elements/checks/gr-checks-tab.ts
+++ b/polygerrit-ui/app/elements/checks/gr-checks-tab.ts
@@ -14,7 +14,6 @@
import './gr-checks-runs';
import './gr-checks-results';
import {NumericChangeId, PatchSetNumber} from '../../types/common';
-import {RunSelectedEvent} from './gr-checks-util';
import {TabState} from '../../types/events';
import {getAppContext} from '../../services/app-context';
import {subscribe} from '../lit/subscription-controller';
@@ -50,9 +49,6 @@
@state()
changeNum: NumericChangeId | undefined = undefined;
- @state()
- selectedRuns: string[] = [];
-
private readonly getChangeModel = resolve(this, changeModelToken);
private readonly getChecksModel = resolve(this, checksModelToken);
@@ -132,41 +128,16 @@
class="runs"
?collapsed=${this.offsetWidth < 1000}
.runs=${this.runs}
- .selectedRuns=${this.selectedRuns}
.tabState=${this.tabState?.checksTab}
- @run-selected=${this.handleRunSelected}
></gr-checks-runs>
<gr-checks-results
class="results"
.tabState=${this.tabState?.checksTab}
.runs=${this.runs}
- .selectedRuns=${this.selectedRuns}
></gr-checks-results>
</div>
`;
}
-
- handleRunSelected(e: RunSelectedEvent) {
- this.reporting.reportInteraction(Interaction.CHECKS_RUN_SELECTED, {
- checkName: e.detail.checkName,
- reset: e.detail.reset,
- });
- if (e.detail.reset) {
- this.selectedRuns = [];
- return;
- }
- if (e.detail.checkName) {
- this.toggleSelected(e.detail.checkName);
- }
- }
-
- toggleSelected(checkName: string) {
- if (this.selectedRuns.includes(checkName)) {
- this.selectedRuns = this.selectedRuns.filter(r => r !== checkName);
- } else {
- this.selectedRuns = [...this.selectedRuns, checkName];
- }
- }
}
declare global {
diff --git a/polygerrit-ui/app/elements/checks/gr-checks-util.ts b/polygerrit-ui/app/elements/checks/gr-checks-util.ts
index 939a87e..c7477c4 100644
--- a/polygerrit-ui/app/elements/checks/gr-checks-util.ts
+++ b/polygerrit-ui/app/elements/checks/gr-checks-util.ts
@@ -11,7 +11,6 @@
} from '../../models/checks/checks-util';
export interface RunSelectedEventDetail {
- reset: boolean;
checkName?: string;
}
@@ -33,16 +32,6 @@
);
}
-export function fireRunSelectionReset(target: EventTarget) {
- target.dispatchEvent(
- new CustomEvent('run-selected', {
- detail: {reset: true},
- composed: true,
- bubbles: true,
- })
- );
-}
-
export function isAttemptSelected(
selectedAttempt: AttemptChoice,
run: CheckRun
diff --git a/polygerrit-ui/app/elements/core/gr-router/gr-router.ts b/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
index a35ca40..f829313 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
@@ -315,11 +315,12 @@
// Note that router model view must be updated before view model state.
// So this check is slightly fragile, but should work.
if (this.view !== GerritView.CHANGE) return;
- const browserUrl = window.location.toString();
- const stateUrl = new URL(createChangeUrl(state), browserUrl).toString();
- if (browserUrl !== stateUrl) {
+ const browserUrl = new URL(window.location.toString());
+ const stateUrl = new URL(createChangeUrl(state), browserUrl);
+ stateUrl.hash = browserUrl.hash;
+ if (browserUrl.toString() !== stateUrl.toString()) {
page.replace(
- stateUrl,
+ stateUrl.toString(),
null,
/* init: */ false,
/* dispatch: */ false
@@ -1405,8 +1406,12 @@
}
const filter = queryMap.get('filter');
if (filter) state.filter = filter;
+ const checksResultsFilter = queryMap.get('checksResultsFilter');
+ if (checksResultsFilter) state.checksResultsFilter = checksResultsFilter;
const attempt = stringToAttemptChoice(queryMap.get('attempt'));
if (attempt && attempt !== LATEST_ATTEMPT) state.attempt = attempt;
+ const selected = queryMap.get('checksRunsSelected');
+ if (selected) state.checksRunsSelected = new Set(selected.split(','));
assertIsDefined(state.project, 'project');
this.reporting.setRepoName(state.project);
diff --git a/polygerrit-ui/app/elements/core/gr-router/gr-router_test.ts b/polygerrit-ui/app/elements/core/gr-router/gr-router_test.ts
index ce3ed0d..119ba48 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router_test.ts
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router_test.ts
@@ -1149,6 +1149,8 @@
queryMap.set('filter', 'fff');
queryMap.set('select', 'sss');
queryMap.set('attempt', '1');
+ queryMap.set('checksRunsSelected', 'asdf,qwer');
+ queryMap.set('checksResultsFilter', 'asdf.*qwer');
ctx.querystring = queryMap.toString();
assertctxToParams(ctx, 'handleChangeRoute', {
view: GerritView.CHANGE,
@@ -1159,6 +1161,8 @@
attempt: 1,
filter: 'fff',
tab: 'checks',
+ checksRunsSelected: new Set(['asdf', 'qwer']),
+ checksResultsFilter: 'asdf.*qwer',
});
});
});
diff --git a/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog.ts b/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog.ts
index 262f1fd..17d7516 100644
--- a/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog.ts
+++ b/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog.ts
@@ -22,7 +22,7 @@
import {PROVIDED_FIX_ID} from '../../../utils/comment-util';
import {OpenFixPreviewEvent} from '../../../types/events';
import {getAppContext} from '../../../services/app-context';
-import {fireCloseFixPreview, fireEvent} from '../../../utils/event-util';
+import {fireCloseFixPreview} from '../../../utils/event-util';
import {DiffLayer, ParsedChangeInfo} from '../../../types/types';
import {GrButton} from '../../shared/gr-button/gr-button';
import {TokenHighlightLayer} from '../../../embed/diff/gr-diff-builder/token-highlight-layer';
@@ -33,6 +33,7 @@
import {assert} from '../../../utils/common-util';
import {resolve} from '../../../models/dependency';
import {createChangeUrl} from '../../../models/views/change';
+import {GrDialog} from '../../shared/gr-dialog/gr-dialog';
interface FilePreview {
filepath: string;
@@ -44,6 +45,15 @@
@query('#applyFixOverlay')
applyFixOverlay?: GrOverlay;
+ @query('#applyFixDialog')
+ applyFixDialog?: GrDialog;
+
+ /** The currently observed dialog by `dialogOberserver`. */
+ observedDialog?: GrDialog;
+
+ /** The current observer observing the `observedDialog`. */
+ dialogObserver?: ResizeObserver;
+
@query('#nextFix')
nextFix?: GrButton;
@@ -102,9 +112,6 @@
this.diffPrefs = diffPreferences;
}
);
- this.addEventListener('diff-context-expanded', () => {
- if (this.applyFixOverlay) fireEvent(this.applyFixOverlay, 'iron-resize');
- });
}
static override styles = [
@@ -148,6 +155,39 @@
`;
}
+ override updated() {
+ this.updateDialogObserver();
+ }
+
+ override disconnectedCallback() {
+ this.removeDialogObserver();
+ super.disconnectedCallback();
+ }
+
+ private removeDialogObserver() {
+ this.dialogObserver?.disconnect();
+ this.dialogObserver = undefined;
+ this.observedDialog = undefined;
+ }
+
+ private updateDialogObserver() {
+ if (
+ this.applyFixDialog === this.observedDialog &&
+ this.dialogObserver !== undefined
+ ) {
+ return;
+ }
+
+ this.removeDialogObserver();
+ if (!this.applyFixDialog) return;
+
+ this.observedDialog = this.applyFixDialog;
+ this.dialogObserver = new ResizeObserver(() => {
+ this.applyFixOverlay?.refit();
+ });
+ this.dialogObserver.observe(this.observedDialog);
+ }
+
private renderHeader() {
return html`
<div slot="header">${this.currentFix?.description ?? ''}</div>
@@ -202,7 +242,7 @@
* Given event with fixSuggestions, fetch diffs associated with first
* suggested fix and open dialog.
*/
- async open(e: OpenFixPreviewEvent) {
+ open(e: OpenFixPreviewEvent) {
this.patchNum = e.detail.patchNum;
this.fixSuggestions = e.detail.fixSuggestions;
assert(this.fixSuggestions.length > 0, 'no fix in the event');
@@ -212,9 +252,6 @@
this.showSelectedFixSuggestion(this.fixSuggestions[0]),
this.applyFixOverlay?.open()
);
- return Promise.all(promises).then(() => {
- if (this.applyFixOverlay) fireEvent(this.applyFixOverlay, 'iron-resize');
- });
}
private async showSelectedFixSuggestion(fixSuggestion: FixSuggestionInfo) {
diff --git a/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog_test.ts b/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog_test.ts
index b6e4a95..4d2d454 100644
--- a/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog_test.ts
+++ b/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog_test.ts
@@ -52,7 +52,7 @@
}
async function open(detail: OpenFixPreviewEventDetail) {
- await element.open(
+ element.open(
new CustomEvent<OpenFixPreviewEventDetail>(EventType.OPEN_FIX_PREVIEW, {
detail,
})
diff --git a/polygerrit-ui/app/elements/gr-app.ts b/polygerrit-ui/app/elements/gr-app.ts
index 33ba1e4..0cec8dd 100644
--- a/polygerrit-ui/app/elements/gr-app.ts
+++ b/polygerrit-ui/app/elements/gr-app.ts
@@ -70,7 +70,6 @@
appContext.flagsService,
appContext.userModel
);
- this.serviceWorkerInstaller.init();
}
}
diff --git a/polygerrit-ui/app/elements/lit/fit-controller.ts b/polygerrit-ui/app/elements/lit/fit-controller.ts
new file mode 100644
index 0000000..423a4f0
--- /dev/null
+++ b/polygerrit-ui/app/elements/lit/fit-controller.ts
@@ -0,0 +1,213 @@
+/**
+ * @license
+ * Copyright 2022 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+import {ReactiveController, ReactiveControllerHost} from 'lit';
+
+export interface FitControllerHost {
+ /**
+ * This offset will increase or decrease the distance to the left side
+ * of the screen, a negative offset will move the dropdown to the left
+ * a positive one, to the right.
+ *
+ */
+ horizontalOffset: number;
+
+ /**
+ * This offset will increase or decrease the distance to the top
+ * side of the screen: a negative offset will move the dropdown upwards
+ * , a positive one, downwards.
+ *
+ */
+ verticalOffset: number;
+}
+
+export interface PositionStyles {
+ top: string;
+ left: string;
+ position: string;
+ maxWidth: string;
+ maxHeight: string;
+ boxSizing: string;
+}
+
+/**
+ * `FitController` fits an element in another element using `max-height`
+ * and `max-width`.
+ *
+ * FitController overrides all properties defined in PositionStyles for the
+ * host.
+ * The element will only be sized and/or positioned if it has not already been
+ * sized and/or positioned by CSS.
+ * CSS properties | Action
+ * --------------------------|-------------------------------------------
+ * `position` set | Element is not centered horizontally/vertically
+ * `top` or `bottom` set | Element is not vertically centered
+ * `left` or `right` set | Element is not horizontally centered
+ * `max-height` set | Element respects `max-height`
+ * `max-width` set | Element respects `max-width`
+ *
+ * `FitController` positions an element into another element and gives it
+ * a horizontalAlignment = left and verticalAlignment = top.
+ * This will override the element's css position.
+ *
+ * Use `horizontalOffset, verticalOffset` to offset the element from its
+ * `positionTarget`; `FitController` will collapse these in order to
+ * keep the element within `window` boundaries, while preserving the element's
+ * CSS margin values.
+ *
+ */
+export class FitController implements ReactiveController {
+ host: ReactiveControllerHost & HTMLElement & FitControllerHost;
+
+ private originalStyles?: PositionStyles;
+
+ private positionTarget?: HTMLElement;
+
+ constructor(host: ReactiveControllerHost & HTMLElement & FitControllerHost) {
+ (this.host = host).addController(this);
+ }
+
+ hostConnected() {
+ this.positionTarget = this.getPositionTarget();
+ }
+
+ hostDisconnected() {}
+
+ // private but used in tests
+ getPositionTarget() {
+ let parent = this.host.parentNode;
+
+ if (parent && parent.nodeType === Node.DOCUMENT_FRAGMENT_NODE) {
+ parent = (parent as ShadowRoot).host;
+ }
+
+ return parent as HTMLElement;
+ }
+
+ private saveOriginalStyles() {
+ // These properties are changed in position() hence keep the original
+ // values to reset the host styles later.
+ this.originalStyles = {
+ top: this.host.style.top || '',
+ left: this.host.style.left || '',
+ position: this.host.style.position || '',
+ maxWidth: this.host.style.maxWidth || '',
+ maxHeight: this.host.style.maxHeight || '',
+ boxSizing: this.host.style.boxSizing || '',
+ };
+ }
+
+ /**
+ * Reset the host style, and clear the memoized data.
+ */
+ private resetStyles() {
+ // It is necessary to clear the max-width:0px and max-height:0px.
+ // A component may call refit() multiple times, in which case we don't
+ // want the values assigned from the first call which may not be precisely
+ // correct to influence the second call.
+ // Hence we reset the styles here.
+ if (this.originalStyles !== undefined) {
+ Object.assign(this.host.style, this.originalStyles);
+ }
+ this.originalStyles = undefined;
+ }
+
+ setPositionTarget(target: HTMLElement) {
+ this.positionTarget = target;
+ }
+
+ /**
+ * Equivalent to calling `resetStyles()` and `position()`.
+ * Useful to call this after the element or the `window` element has
+ * been resized, or if any of the positioning properties
+ * (e.g. `horizontalOffset, verticalOffset`) are updated.
+ * It preserves the scroll position of the host.
+ */
+ refit() {
+ const scrollLeft = this.host.scrollLeft;
+ const scrollTop = this.host.scrollTop;
+ this.resetStyles();
+ this.position();
+ this.host.scrollLeft = scrollLeft;
+ this.host.scrollTop = scrollTop;
+ }
+
+ private position() {
+ this.saveOriginalStyles();
+
+ this.host.style.position = 'fixed';
+ // Need border-box for margin/padding.
+ this.host.style.boxSizing = 'border-box';
+
+ const hostRect = this.host.getBoundingClientRect();
+ const positionRect = this.getNormalizedRect(this.positionTarget!);
+ const windowRect = this.getNormalizedRect(window);
+
+ this.calculateAndSetPositions(hostRect, positionRect, windowRect);
+ }
+
+ // private but used in tests
+ calculateAndSetPositions(
+ hostRect: DOMRect,
+ positionRect: DOMRect,
+ windowRect: DOMRect
+ ) {
+ const hostStyles = (window as Window).getComputedStyle(this.host);
+ const hostMinWidth = parseInt(hostStyles.minWidth) || 0;
+ const hostMinHeight = parseInt(hostStyles.minHeight) || 0;
+
+ const hostMargin = {
+ top: parseInt(hostStyles.marginTop) || 0,
+ right: parseInt(hostStyles.marginRight) || 0,
+ bottom: parseInt(hostStyles.marginBottom) || 0,
+ left: parseInt(hostStyles.marginLeft) || 0,
+ };
+
+ let leftPosition =
+ positionRect.left + this.host.horizontalOffset + hostMargin.left;
+ let topPosition =
+ positionRect.top + this.host.verticalOffset + hostMargin.top;
+
+ // Limit right/bottom within window respecting the margin.
+ const rightPosition = Math.min(
+ windowRect.right - hostMargin.right,
+ leftPosition + hostRect.width
+ );
+ const bottomPosition = Math.min(
+ windowRect.bottom - hostMargin.bottom,
+ topPosition + hostRect.height
+ );
+
+ // Respect hostMinWidth and hostMinHeight
+ // Current width is rightPosition - leftPosition or hostRect.width
+ // rightPosition - leftPosition >= hostMinWidth
+ // => leftPosition <= rightPosition - hostMinWidth
+ leftPosition = Math.min(leftPosition, rightPosition - hostMinWidth);
+ topPosition = Math.min(topPosition, bottomPosition - hostMinHeight);
+
+ // Limit left/top within window respecting the margin.
+ leftPosition = Math.max(windowRect.left + hostMargin.left, leftPosition);
+ topPosition = Math.max(windowRect.top + hostMargin.top, topPosition);
+
+ // Use right/bottom to set maxWidth/maxHeight and respect
+ // minWidth/minHeight.
+ const maxWidth = Math.max(rightPosition - leftPosition, hostMinWidth);
+ const maxHeight = Math.max(bottomPosition - topPosition, hostMinHeight);
+
+ this.host.style.maxWidth = `${maxWidth}px`;
+ this.host.style.maxHeight = `${maxHeight}px`;
+
+ this.host.style.left = `${leftPosition}px`;
+ this.host.style.top = `${topPosition}px`;
+ }
+
+ private getNormalizedRect(target: Window | HTMLElement): DOMRect {
+ if (target === document.documentElement || target === window) {
+ return new DOMRect(0, 0, window.innerWidth, window.innerHeight);
+ }
+ return (target as HTMLElement).getBoundingClientRect();
+ }
+}
diff --git a/polygerrit-ui/app/elements/lit/fit-controller_test.ts b/polygerrit-ui/app/elements/lit/fit-controller_test.ts
new file mode 100644
index 0000000..2a60b83
--- /dev/null
+++ b/polygerrit-ui/app/elements/lit/fit-controller_test.ts
@@ -0,0 +1,143 @@
+/**
+ * @license
+ * Copyright 2022 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+import '../../test/common-test-setup';
+import {fixture, html, assert} from '@open-wc/testing';
+import {FitController} from './fit-controller';
+import {LitElement} from 'lit';
+import {customElement} from 'lit/decorators.js';
+
+@customElement('fit-element')
+class FitElement extends LitElement {
+ fitController = new FitController(this);
+
+ horizontalOffset = 0;
+
+ verticalOffset = 0;
+
+ override render() {
+ return html`<div></div>`;
+ }
+}
+
+suite('fit controller', () => {
+ let element: FitElement;
+ setup(async () => {
+ element = await fixture(html`<fit-element></fit-element>`);
+ });
+
+ test('refit positioning', async () => {
+ const hostRect = new DOMRect(0, 0, 50, 50);
+
+ const positionRect = new DOMRect(37, 37, 300, 60);
+
+ const windowRect = new DOMRect(0, 0, 600, 600);
+
+ element.fitController.calculateAndSetPositions(
+ hostRect,
+ positionRect,
+ windowRect
+ );
+
+ assert.equal(element.style.top, '37px');
+ assert.equal(element.style.left, '37px');
+ });
+
+ test('refit positioning with offset', async () => {
+ const elementWithOffset: FitElement = await fixture(
+ html`<fit-element></fit-element>`
+ );
+ elementWithOffset.verticalOffset = 10;
+ elementWithOffset.horizontalOffset = 20;
+
+ const hostRect = new DOMRect(0, 0, 50, 50);
+
+ const positionRect = new DOMRect(37, 37, 300, 60);
+
+ const windowRect = new DOMRect(0, 0, 600, 600);
+
+ elementWithOffset.fitController.calculateAndSetPositions(
+ hostRect,
+ positionRect,
+ windowRect
+ );
+
+ assert.equal(elementWithOffset.style.top, '47px');
+ assert.equal(elementWithOffset.style.left, '57px');
+ });
+
+ test('host margin updates positioning', async () => {
+ const hostRect = new DOMRect(0, 0, 50, 50);
+
+ const positionRect = new DOMRect(37, 37, 300, 60);
+
+ const windowRect = new DOMRect(0, 0, 600, 600);
+
+ element.style.marginLeft = '10px';
+ element.style.marginTop = '10px';
+ element.fitController.calculateAndSetPositions(
+ hostRect,
+ positionRect,
+ windowRect
+ );
+
+ // is 10px extra from the previous test due to host margin
+ assert.equal(element.style.top, '47px');
+ assert.equal(element.style.left, '47px');
+ });
+
+ test('host minWidth, minHeight overrides positioning', async () => {
+ const hostRect = new DOMRect(0, 0, 50, 50);
+
+ const positionRect = new DOMRect(37, 37, 300, 60);
+
+ const windowRect = new DOMRect(0, 0, 600, 600);
+
+ element.style.marginLeft = '10px';
+ element.style.marginTop = '10px';
+
+ element.style.minHeight = '50px';
+ element.style.minWidth = '60px';
+
+ element.fitController.calculateAndSetPositions(
+ hostRect,
+ positionRect,
+ windowRect
+ );
+
+ assert.equal(element.style.top, '47px');
+
+ // Should be 47 like the previous test but that would make it overall
+ // smaller in width than the minWidth defined
+ assert.equal(element.style.left, '37px');
+ assert.equal(element.style.maxWidth, '60px');
+ });
+
+ test('positioning happens within window size ', async () => {
+ const hostRect = new DOMRect(0, 0, 50, 50);
+
+ const positionRect = new DOMRect(37, 37, 300, 60);
+
+ // window size is small hence limits the position
+ const windowRect = new DOMRect(0, 0, 50, 50);
+
+ element.style.marginLeft = '10px';
+ element.style.marginTop = '10px';
+
+ element.fitController.calculateAndSetPositions(
+ hostRect,
+ positionRect,
+ windowRect
+ );
+
+ assert.equal(element.style.top, '47px');
+ assert.equal(element.style.left, '47px');
+ // With the window size being 50, the element is styled with width 3px
+ // width = windowSize - leftPosition = 50 - 47 = 3px
+ // Without the window width restriction, in previous test maxWidth is 60px
+ assert.equal(element.style.maxWidth, '3px');
+ });
+});
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 c4ca51e..ff2904a 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
@@ -28,7 +28,11 @@
import {GrGroupList} from '../gr-group-list/gr-group-list';
import {GrIdentities} from '../gr-identities/gr-identities';
import {GrDiffPreferences} from '../../shared/gr-diff-preferences/gr-diff-preferences';
-import {PreferencesInput, ServerInfo} from '../../../types/common';
+import {
+ AccountDetailInfo,
+ PreferencesInput,
+ ServerInfo,
+} from '../../../types/common';
import {GrSshEditor} from '../gr-ssh-editor/gr-ssh-editor';
import {GrGpgEditor} from '../gr-gpg-editor/gr-gpg-editor';
import {GrEmailEditor} from '../gr-email-editor/gr-email-editor';
@@ -58,6 +62,7 @@
import {subscribe} from '../../lit/subscription-controller';
import {resolve} from '../../../models/dependency';
import {settingsViewModelToken} from '../../../models/views/settings';
+import {areNotificationsEnabled} from '../../../utils/worker-util';
const GERRIT_DOCS_BASE_URL =
'https://gerrit-review.googlesource.com/' + 'Documentation';
@@ -111,6 +116,9 @@
@query('#publishCommentsOnPush') publishCommentsOnPush!: HTMLInputElement;
+ @query('#allowBrowserNotifications')
+ allowBrowserNotifications?: HTMLInputElement;
+
@query('#disableKeyboardShortcuts')
disableKeyboardShortcuts!: HTMLInputElement;
@@ -186,6 +194,8 @@
// private but used in test
@state() showNumber?: boolean;
+ @state() account?: AccountDetailInfo;
+
// private but used in test
public _testOnly_loadingPromise?: Promise<void>;
@@ -193,7 +203,8 @@
private readonly userModel = getAppContext().userModel;
- private readonly flagsService = getAppContext().flagsService;
+ // private but used in test
+ readonly flagsService = getAppContext().flagsService;
private readonly getViewModel = resolve(this, settingsViewModelToken);
@@ -209,6 +220,13 @@
);
subscribe(
this,
+ () => this.userModel.account$,
+ acc => {
+ this.account = acc;
+ }
+ );
+ subscribe(
+ this,
() => this.userModel.preferences$,
prefs => {
if (!prefs) {
@@ -399,7 +417,8 @@
<fieldset id="preferences">
${this.renderTheme()} ${this.renderChangesPerPages()}
${this.renderDateTimeFormat()} ${this.renderEmailNotification()}
- ${this.renderEmailFormat()} ${this.renderDefaultBaseForMerges()}
+ ${this.renderEmailFormat()} ${this.renderBrowserNotifications()}
+ ${this.renderDefaultBaseForMerges()}
${this.renderRelativeDateInChangeTable()} ${this.renderDiffView()}
${this.renderShowSizeBarsInFileList()}
${this.renderPublishCommentsOnPush()}
@@ -869,6 +888,31 @@
`;
}
+ private renderBrowserNotifications() {
+ if (!this.flagsService.isEnabled(KnownExperimentId.PUSH_NOTIFICATIONS))
+ return nothing;
+ if (!areNotificationsEnabled(this.account)) return nothing;
+ return html`
+ <section id="allowBrowserNotificationsSection">
+ <label class="title" for="allowBrowserNotifications"
+ >Allow browser notifications</label
+ >
+ <span class="value">
+ <input
+ id="allowBrowserNotifications"
+ type="checkbox"
+ ?checked=${this.localPrefs.allow_browser_notifications}
+ @change=${() => {
+ this.localPrefs.allow_browser_notifications =
+ this.allowBrowserNotifications!.checked;
+ this.prefsChanged = true;
+ }}
+ />
+ </span>
+ </section>
+ `;
+ }
+
private renderDefaultBaseForMerges() {
if (!this.localPrefs.default_base_for_merges) return nothing;
return nothing;
diff --git a/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view_test.ts b/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view_test.ts
index 72c19b4..0798991 100644
--- a/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view_test.ts
+++ b/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view_test.ts
@@ -6,7 +6,13 @@
import '../../../test/common-test-setup';
import './gr-settings-view';
import {GrSettingsView} from './gr-settings-view';
-import {queryAll, stubRestApi, waitEventLoop} from '../../../test/test-utils';
+import {
+ queryAll,
+ queryAndAssert,
+ stubFlags,
+ stubRestApi,
+ waitEventLoop,
+} from '../../../test/test-utils';
import {
AuthInfo,
AccountDetailInfo,
@@ -510,6 +516,24 @@
);
});
+ test('allow browser notifications', async () => {
+ stubFlags('isEnabled').returns(true);
+ element = await fixture(html`<gr-settings-view></gr-settings-view>`);
+ element.account = createAccountDetailWithId();
+ await element.updateComplete;
+ assert.dom.equal(
+ queryAndAssert(element, '#allowBrowserNotificationsSection'),
+ /* HTML */ `<section id="allowBrowserNotificationsSection">
+ <label class="title" for="allowBrowserNotifications">
+ Allow browser notifications
+ </label>
+ <span class="value">
+ <input checked="" id="allowBrowserNotifications" type="checkbox" />
+ </span>
+ </section>`
+ );
+ });
+
test('calls the title-change event', async () => {
const titleChangedStub = sinon.stub();
diff --git a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.ts b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.ts
index 78a1509..9dfbc04 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.ts
@@ -73,6 +73,7 @@
import {Interaction} from '../../../constants/reporting';
import {HtmlPatched} from '../../../utils/lit-util';
import {createDiffUrl} from '../../../models/views/diff';
+import {createChangeUrl} from '../../../models/views/change';
declare global {
interface HTMLElementEventMap {
@@ -779,13 +780,20 @@
if (!comment) return;
assertIsDefined(this.changeNum, 'changeNum');
assertIsDefined(this.repoName, 'repoName');
- const url = generateAbsoluteUrl(
- createDiffUrl({
+ let url: string;
+ if (this.isPatchsetLevel()) {
+ url = createChangeUrl({
changeNum: this.changeNum,
project: this.repoName,
commentId: comment.id,
- })
- );
+ });
+ } else {
+ url = createDiffUrl({
+ changeNum: this.changeNum,
+ project: this.repoName,
+ commentId: comment.id,
+ });
+ }
assertIsDefined(url, 'url for comment');
copyToClipbard(generateAbsoluteUrl(url), 'Link');
}
diff --git a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_test.ts b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_test.ts
index b767a32..10e54c7 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_test.ts
@@ -28,6 +28,8 @@
import {SinonStub} from 'sinon';
import {fixture, html, waitUntil, assert} from '@open-wc/testing';
import {GrButton} from '../gr-button/gr-button';
+import {SpecialFilePath} from '../../../constants/constants';
+import {GrIcon} from '../gr-icon/gr-icon';
const c1 = {
author: {name: 'Kermit'},
@@ -447,4 +449,35 @@
},
]);
});
+
+ test('patchset comments link to /comments URL', async () => {
+ const clipboardStub = sinon.stub(navigator.clipboard, 'writeText');
+ element.thread = {
+ ...createThread(c1),
+ path: SpecialFilePath.PATCHSET_LEVEL_COMMENTS,
+ };
+ await element.updateComplete;
+
+ queryAndAssert<GrIcon>(element, 'gr-icon.copy').click();
+
+ assert.equal(1, clipboardStub.callCount);
+ assert.equal(
+ clipboardStub.firstCall.args[0],
+ 'http://localhost:9876/c/test-repo-name/+/1/comments/the-root'
+ );
+ });
+
+ test('file comments link to /comment URL', async () => {
+ const clipboardStub = sinon.stub(navigator.clipboard, 'writeText');
+ element.thread = createThread(c1);
+ await element.updateComplete;
+
+ queryAndAssert<GrIcon>(element, 'gr-icon.copy').click();
+
+ assert.equal(1, clipboardStub.callCount);
+ assert.equal(
+ clipboardStub.firstCall.args[0],
+ 'http://localhost:9876/c/test-repo-name/+/1/comment/the-root/'
+ );
+ });
});
diff --git a/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text.ts b/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text.ts
index 6ab0ec4..5a1db30 100644
--- a/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text.ts
+++ b/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text.ts
@@ -16,11 +16,7 @@
import {subscribe} from '../../lit/subscription-controller';
import {configModelToken} from '../../../models/config/config-model';
import {CommentLinks, EmailAddress} from '../../../api/rest-api';
-import {
- applyHtmlRewritesFromConfig,
- applyLinkRewritesFromConfig,
- linkifyNormalUrls,
-} from '../../../utils/link-util';
+import {linkifyUrlsAndApplyRewrite} from '../../../utils/link-util';
import '../gr-account-chip/gr-account-chip';
import {KnownExperimentId} from '../../../services/flags/flags';
import {getAppContext} from '../../../services/app-context';
@@ -125,7 +121,7 @@
}
private renderAsPlaintext() {
- const linkedText = this.rewriteText(
+ const linkedText = linkifyUrlsAndApplyRewrite(
htmlEscape(this.content).toString(),
this.repoCommentLinks
);
@@ -140,7 +136,7 @@
// renderer so we wrap 'this.rewriteText' so that 'this' is preserved via
// closure.
const boundRewriteText = (text: string) =>
- this.rewriteText(text, this.repoCommentLinks);
+ linkifyUrlsAndApplyRewrite(text, this.repoCommentLinks);
// We are overriding some marked-element renderers for a few reasons:
// 1. Disable inline images as a design/policy choice.
@@ -201,24 +197,6 @@
return text;
}
- private rewriteText(text: string, repoCommentLinks: CommentLinks) {
- // Turn universally identifiable URLs into links. Ex: www.google.com. The
- // markdown library inside marked-element does this too, but is more
- // conservative and misses some URLs like "google.com" without "www" prefix.
- text = linkifyNormalUrls(text);
-
- // Apply the host's config-specific regex replacements to create links. Ex:
- // link "Bug 12345" to "google.com/bug/12345"
- text = applyLinkRewritesFromConfig(text, repoCommentLinks);
-
- // Apply the host's config-specific regex replacements to write arbitrary
- // html. Most examples seen in the wild are also used for linking but with
- // finer control over the rendered text. Ex: "Bug 12345" => "#12345"
- text = applyHtmlRewritesFromConfig(text, repoCommentLinks);
-
- return text;
- }
-
override updated() {
// Look for @mentions and replace them with an account-label chip.
if (this.flagsService.isEnabled(KnownExperimentId.MENTION_USERS)) {
diff --git a/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text_test.ts b/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text_test.ts
index 6391347..ca498318 100644
--- a/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text_test.ts
@@ -77,6 +77,76 @@
await element.updateComplete;
});
+ test('does not apply rewrites within links', async () => {
+ element.content = 'google.com/LinkRewriteMe';
+ await element.updateComplete;
+
+ assert.shadowDom.equal(
+ element,
+ /* HTML */ `
+ <pre class="plaintext">
+ <a
+ href="http://google.com/LinkRewriteMe"
+ rel="noopener"
+ target="_blank"
+ >
+ google.com/LinkRewriteMe
+ </a>
+ </pre>
+ `
+ );
+ });
+
+ test('does not apply rewrites on rewritten text', async () => {
+ await setCommentLinks({
+ capitalizeFoo: {
+ match: 'foo',
+ html: 'FOO',
+ },
+ lowercaseFoo: {
+ match: 'FOO',
+ html: 'foo',
+ },
+ });
+ element.content = 'foo';
+ await element.updateComplete;
+
+ assert.shadowDom.equal(
+ element,
+ /* HTML */ `
+ <pre class="plaintext">
+ FOO
+ </pre
+ >
+ `
+ );
+ });
+
+ test('supports overlapping rewrites', async () => {
+ await setCommentLinks({
+ bracketNum: {
+ match: '(Start:) ([0-9]+)',
+ html: '$1 [$2]',
+ },
+ bracketNum2: {
+ match: '(Start: [0-9]+) ([0-9]+)',
+ html: '$1 [$2]',
+ },
+ });
+ element.content = 'Start: 123 456';
+ await element.updateComplete;
+
+ assert.shadowDom.equal(
+ element,
+ /* HTML */ `
+ <pre class="plaintext">
+ Start: [123] [456]
+ </pre
+ >
+ `
+ );
+ });
+
test('renders text with links and rewrites', async () => {
element.content = `text with plain link: google.com
\ntext with config link: LinkRewriteMe
diff --git a/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text.ts b/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text.ts
deleted file mode 100644
index 16a60e7..0000000
--- a/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text.ts
+++ /dev/null
@@ -1,178 +0,0 @@
-/**
- * @license
- * Copyright 2015 Google LLC
- * SPDX-License-Identifier: Apache-2.0
- */
-import '../../../styles/shared-styles';
-import {GrLinkTextParser, LinkTextParserConfig} from './link-text-parser';
-import {LitElement, css, html, PropertyValues} from 'lit';
-import {customElement, property} from 'lit/decorators.js';
-import {assertIsDefined} from '../../../utils/common-util';
-
-declare global {
- interface HTMLElementTagNameMap {
- 'gr-linked-text': GrLinkedText;
- }
-}
-
-@customElement('gr-linked-text')
-export class GrLinkedText extends LitElement {
- private outputElement?: HTMLSpanElement;
-
- @property({type: Boolean, attribute: 'remove-zero-width-space'})
- removeZeroWidthSpace?: boolean;
-
- @property({type: String})
- content = '';
-
- @property({type: Boolean, attribute: true})
- pre = false;
-
- @property({type: Boolean, attribute: true})
- disabled = false;
-
- @property({type: Boolean, attribute: true})
- inline = false;
-
- @property({type: Object})
- config?: LinkTextParserConfig;
-
- static override get styles() {
- return css`
- :host {
- display: block;
- }
- :host([inline]) {
- display: inline;
- }
- :host([pre]) ::slotted(span) {
- white-space: var(--linked-text-white-space, pre-wrap);
- word-wrap: var(--linked-text-word-wrap, break-word);
- }
- `;
- }
-
- override render() {
- return html`<slot name="insert"></slot>`;
- }
-
- // NOTE: LinkTextParser dynamically creates HTML fragments based on backend
- // configuration commentLinks. These commentLinks can contain arbitrary HTML
- // fragments. This means that arbitrary HTML needs to be injected into the
- // DOM-tree, where this HTML is is controlled on the server-side in the
- // server-configuration rather than by arbitrary users.
- // To enable this injection of 'unsafe' HTML, LinkTextParser generates
- // HTML fragments. Lit does not support inserting html fragments directly
- // into its DOM-tree as it controls the DOM-tree that it generates.
- // Therefore, to get around this we create a single element that we slot into
- // the Lit-owned DOM. This element will not be part of this LitElement as
- // it's slotted in and thus can be modified on the fly by handleParseResult.
- override firstUpdated(_changedProperties: PropertyValues): void {
- this.outputElement = document.createElement('span');
- this.outputElement.id = 'output';
- this.outputElement.slot = 'insert';
- this.append(this.outputElement);
- }
-
- override updated(changedProperties: PropertyValues): void {
- if (changedProperties.has('content') || changedProperties.has('config')) {
- this._contentOrConfigChanged();
- } else if (changedProperties.has('disabled')) {
- this.styleLinks();
- }
- }
-
- /**
- * Because either the source text or the linkification config has changed,
- * the content should be re-parsed.
- * Private but used in tests.
- *
- * @param content The raw, un-linkified source string to parse.
- * @param config The server config specifying commentLink patterns
- */
- _contentOrConfigChanged() {
- if (!this.config) {
- assertIsDefined(this.outputElement);
- this.outputElement.textContent = this.content;
- return;
- }
-
- assertIsDefined(this.outputElement);
- this.outputElement.textContent = '';
- const parser = new GrLinkTextParser(
- this.config,
- (text: string | null, href: string | null, fragment?: DocumentFragment) =>
- this.handleParseResult(text, href, fragment),
- this.removeZeroWidthSpace
- );
- parser.parse(this.content);
-
- // Ensure that external links originating from HTML commentlink configs
- // open in a new tab. @see Issue 5567
- // Ensure links to the same host originating from commentlink configs
- // open in the same tab. When target is not set - default is _self
- // @see Issue 4616
- this.outputElement.querySelectorAll('a').forEach(anchor => {
- if (anchor.hostname === window.location.hostname) {
- anchor.removeAttribute('target');
- } else {
- anchor.setAttribute('target', '_blank');
- }
- anchor.setAttribute('rel', 'noopener');
- });
-
- this.styleLinks();
- }
-
- /**
- * Styles the links based on whether gr-linked-text is disabled or not
- */
- private styleLinks() {
- assertIsDefined(this.outputElement);
- this.outputElement.querySelectorAll('a').forEach(anchor => {
- anchor.setAttribute('style', this.computeLinkStyle());
- });
- }
-
- private computeLinkStyle() {
- if (this.disabled) {
- return `
- color: inherit;
- text-decoration: none;
- pointer-events: none;
- `;
- } else {
- return 'color: var(--link-color)';
- }
- }
-
- /**
- * This method is called when the GrLikTextParser emits a partial result
- * (used as the "callback" parameter). It will be called in either of two
- * ways:
- * - To create a link: when called with `text` and `href` arguments, a link
- * element should be created and attached to the resulting DOM.
- * - To attach an arbitrary fragment: when called with only the `fragment`
- * argument, the fragment should be attached to the resulting DOM as is.
- */
- private handleParseResult(
- text: string | null,
- href: string | null,
- fragment?: DocumentFragment
- ) {
- assertIsDefined(this.outputElement);
- const output = this.outputElement;
- if (href) {
- const a = document.createElement('a');
- a.setAttribute('href', href);
- // GrLinkTextParser either pass text and href together or
- // only DocumentFragment - see LinkTextParserCallback
- a.textContent = text!;
- a.target = '_blank';
- a.setAttribute('rel', 'noopener');
- output.appendChild(a);
- } else if (fragment) {
- output.appendChild(fragment);
- }
- }
-}
diff --git a/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text_test.ts b/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text_test.ts
deleted file mode 100644
index 00e0313..0000000
--- a/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text_test.ts
+++ /dev/null
@@ -1,471 +0,0 @@
-/**
- * @license
- * Copyright 2015 Google LLC
- * SPDX-License-Identifier: Apache-2.0
- */
-import '../../../test/common-test-setup';
-import './gr-linked-text';
-import {fixture, html, assert} from '@open-wc/testing';
-import {GrLinkedText} from './gr-linked-text';
-import {queryAndAssert} from '../../../test/test-utils';
-
-suite('gr-linked-text tests', () => {
- let element: GrLinkedText;
-
- let originalCanonicalPath: string | undefined;
-
- setup(async () => {
- originalCanonicalPath = window.CANONICAL_PATH;
- element = await fixture<GrLinkedText>(html`
- <gr-linked-text>
- <div id="output"></div>
- </gr-linked-text>
- `);
-
- element.config = {
- ph: {
- match: '([Bb]ug|[Ii]ssue)\\s*#?(\\d+)',
- link: 'https://bugs.chromium.org/p/gerrit/issues/detail?id=$2',
- },
- prefixsameinlinkandpattern: {
- match: '([Hh][Tt][Tt][Pp]example)\\s*#?(\\d+)',
- link: 'https://bugs.chromium.org/p/gerrit/issues/detail?id=$2',
- },
- changeid: {
- match: '(I[0-9a-f]{8,40})',
- link: '#/q/$1',
- },
- changeid2: {
- match: 'Change-Id: +(I[0-9a-f]{8,40})',
- link: '#/q/$1',
- },
- googlesearch: {
- match: 'google:(.+)',
- link: 'https://bing.com/search?q=$1', // html should supersede link.
- html: '<a href="https://google.com/search?q=$1">$1</a>',
- },
- hashedhtml: {
- match: 'hash:(.+)',
- html: '<a href="#/awesomesauce">$1</a>',
- },
- baseurl: {
- match: 'test (.+)',
- html: '<a href="/r/awesomesauce">$1</a>',
- },
- anotatstartwithbaseurl: {
- match: 'a test (.+)',
- html: '[Lookup: <a href="/r/awesomesauce">$1</a>]',
- },
- disabledconfig: {
- match: 'foo:(.+)',
- link: 'https://google.com/search?q=$1',
- enabled: false,
- },
- };
- });
-
- teardown(() => {
- window.CANONICAL_PATH = originalCanonicalPath;
- });
-
- test('render', async () => {
- element.content =
- 'https://bugs.chromium.org/p/gerrit/issues/detail?id=3650';
- await element.updateComplete;
- assert.lightDom.equal(
- element,
- /* HTML */ `
- <div id="output"></div>
- <span id="output" slot="insert">
- <a
- href="https://bugs.chromium.org/p/gerrit/issues/detail?id=3650"
- rel="noopener"
- style="color: var(--link-color)"
- target="_blank"
- >
- https://bugs.chromium.org/p/gerrit/issues/detail?id=3650
- </a>
- </span>
- `
- );
- });
-
- test('URL pattern was parsed and linked.', async () => {
- // Regular inline link.
- const url = 'https://bugs.chromium.org/p/gerrit/issues/detail?id=3650';
- element.content = url;
- await element.updateComplete;
-
- const linkEl = queryAndAssert(element, 'span#output')
- .childNodes[0] as HTMLAnchorElement;
- assert.equal(linkEl.target, '_blank');
- assert.equal(linkEl.rel, 'noopener');
- assert.equal(linkEl.href, url);
- assert.equal(linkEl.textContent, url);
- });
-
- test('Bug pattern was parsed and linked', async () => {
- // "Issue/Bug" pattern.
- element.content = 'Issue 3650';
- await element.updateComplete;
-
- let linkEl = queryAndAssert(element, 'span#output')
- .childNodes[0] as HTMLAnchorElement;
- const url = 'https://bugs.chromium.org/p/gerrit/issues/detail?id=3650';
- assert.equal(linkEl.target, '_blank');
- assert.equal(linkEl.href, url);
- assert.equal(linkEl.textContent, 'Issue 3650');
-
- element.content = 'Bug 3650';
- await element.updateComplete;
-
- linkEl = queryAndAssert(element, 'span#output')
- .childNodes[0] as HTMLAnchorElement;
- assert.equal(linkEl.target, '_blank');
- assert.equal(linkEl.rel, 'noopener');
- assert.equal(linkEl.href, url);
- assert.equal(linkEl.textContent, 'Bug 3650');
- });
-
- test('Pattern with same prefix as link was correctly parsed', async () => {
- // Pattern starts with the same prefix (`http`) as the url.
- element.content = 'httpexample 3650';
- await element.updateComplete;
-
- assert.equal(queryAndAssert(element, 'span#output').childNodes.length, 1);
- const linkEl = queryAndAssert(element, 'span#output')
- .childNodes[0] as HTMLAnchorElement;
- const url = 'https://bugs.chromium.org/p/gerrit/issues/detail?id=3650';
- assert.equal(linkEl.target, '_blank');
- assert.equal(linkEl.href, url);
- assert.equal(linkEl.textContent, 'httpexample 3650');
- });
-
- test('Change-Id pattern was parsed and linked', async () => {
- // "Change-Id:" pattern.
- const changeID = 'I11d6a37f5e9b5df0486f6c922d8836dfa780e03e';
- const prefix = 'Change-Id: ';
- element.content = prefix + changeID;
- await element.updateComplete;
-
- const textNode = queryAndAssert(element, 'span#output').childNodes[0];
- const linkEl = queryAndAssert(element, 'span#output')
- .childNodes[1] as HTMLAnchorElement;
- assert.equal(textNode.textContent, prefix);
- const url = '/q/' + changeID;
- assert.isFalse(linkEl.hasAttribute('target'));
- // Since url is a path, the host is added automatically.
- assert.isTrue(linkEl.href.endsWith(url));
- assert.equal(linkEl.textContent, changeID);
- });
-
- test('Change-Id pattern was parsed and linked with base url', async () => {
- window.CANONICAL_PATH = '/r';
-
- // "Change-Id:" pattern.
- const changeID = 'I11d6a37f5e9b5df0486f6c922d8836dfa780e03e';
- const prefix = 'Change-Id: ';
- element.content = prefix + changeID;
- await element.updateComplete;
-
- const textNode = queryAndAssert(element, 'span#output').childNodes[0];
- const linkEl = queryAndAssert(element, 'span#output')
- .childNodes[1] as HTMLAnchorElement;
- assert.equal(textNode.textContent, prefix);
- const url = '/r/q/' + changeID;
- assert.isFalse(linkEl.hasAttribute('target'));
- // Since url is a path, the host is added automatically.
- assert.isTrue(linkEl.href.endsWith(url));
- assert.equal(linkEl.textContent, changeID);
- });
-
- test('Multiple matches', async () => {
- element.content = 'Issue 3650\nIssue 3450';
- await element.updateComplete;
-
- const linkEl1 = queryAndAssert(element, 'span#output')
- .childNodes[0] as HTMLAnchorElement;
- const linkEl2 = queryAndAssert(element, 'span#output')
- .childNodes[2] as HTMLAnchorElement;
-
- assert.equal(linkEl1.target, '_blank');
- assert.equal(
- linkEl1.href,
- 'https://bugs.chromium.org/p/gerrit/issues/detail?id=3650'
- );
- assert.equal(linkEl1.textContent, 'Issue 3650');
-
- assert.equal(linkEl2.target, '_blank');
- assert.equal(
- linkEl2.href,
- 'https://bugs.chromium.org/p/gerrit/issues/detail?id=3450'
- );
- assert.equal(linkEl2.textContent, 'Issue 3450');
- });
-
- test('Change-Id pattern parsed before bug pattern', async () => {
- // "Change-Id:" pattern.
- const changeID = 'I11d6a37f5e9b5df0486f6c922d8836dfa780e03e';
- const prefix = 'Change-Id: ';
-
- // "Issue/Bug" pattern.
- const bug = 'Issue 3650';
-
- const changeUrl = '/q/' + changeID;
- const bugUrl = 'https://bugs.chromium.org/p/gerrit/issues/detail?id=3650';
-
- element.content = prefix + changeID + bug;
- await element.updateComplete;
-
- const textNode = queryAndAssert(element, 'span#output').childNodes[0];
- const changeLinkEl = queryAndAssert(element, 'span#output')
- .childNodes[1] as HTMLAnchorElement;
- const bugLinkEl = queryAndAssert(element, 'span#output')
- .childNodes[2] as HTMLAnchorElement;
-
- assert.equal(textNode.textContent, prefix);
-
- assert.isFalse(changeLinkEl.hasAttribute('target'));
- assert.isTrue(changeLinkEl.href.endsWith(changeUrl));
- assert.equal(changeLinkEl.textContent, changeID);
-
- assert.equal(bugLinkEl.target, '_blank');
- assert.equal(bugLinkEl.href, bugUrl);
- assert.equal(bugLinkEl.textContent, 'Issue 3650');
- });
-
- test('html field in link config', async () => {
- element.content = 'google:do a barrel roll';
- await element.updateComplete;
-
- const linkEl = queryAndAssert(element, 'span#output')
- .childNodes[0] as HTMLAnchorElement;
- assert.equal(
- linkEl.getAttribute('href'),
- 'https://google.com/search?q=do a barrel roll'
- );
- assert.equal(linkEl.textContent, 'do a barrel roll');
- });
-
- test('removing hash from links', async () => {
- element.content = 'hash:foo';
- await element.updateComplete;
-
- const linkEl = queryAndAssert(element, 'span#output')
- .childNodes[0] as HTMLAnchorElement;
- assert.isTrue(linkEl.href.endsWith('/awesomesauce'));
- assert.equal(linkEl.textContent, 'foo');
- });
-
- test('html with base url', async () => {
- window.CANONICAL_PATH = '/r';
-
- element.content = 'test foo';
- await element.updateComplete;
-
- const linkEl = queryAndAssert(element, 'span#output')
- .childNodes[0] as HTMLAnchorElement;
- assert.isTrue(linkEl.href.endsWith('/r/awesomesauce'));
- assert.equal(linkEl.textContent, 'foo');
- });
-
- test('a is not at start', async () => {
- window.CANONICAL_PATH = '/r';
-
- element.content = 'a test foo';
- await element.updateComplete;
-
- const linkEl = queryAndAssert(element, 'span#output')
- .childNodes[1] as HTMLAnchorElement;
- assert.isTrue(linkEl.href.endsWith('/r/awesomesauce'));
- assert.equal(linkEl.textContent, 'foo');
- });
-
- test('hash html with base url', async () => {
- window.CANONICAL_PATH = '/r';
-
- element.content = 'hash:foo';
- await element.updateComplete;
-
- const linkEl = queryAndAssert(element, 'span#output')
- .childNodes[0] as HTMLAnchorElement;
- assert.isTrue(linkEl.href.endsWith('/r/awesomesauce'));
- assert.equal(linkEl.textContent, 'foo');
- });
-
- test('disabled config', async () => {
- element.content = 'foo:baz';
- await element.updateComplete;
-
- assert.equal(queryAndAssert(element, 'span#output').innerHTML, 'foo:baz');
- });
-
- test('R=email labels link correctly', async () => {
- element.removeZeroWidthSpace = true;
- element.content = 'R=\u200Btest@google.com';
- await element.updateComplete;
-
- assert.equal(
- queryAndAssert(element, 'span#output').textContent,
- 'R=test@google.com'
- );
- assert.equal(
- queryAndAssert(element, 'span#output').innerHTML.match(/(R=<a)/g)!.length,
- 1
- );
- });
-
- test('CC=email labels link correctly', async () => {
- element.removeZeroWidthSpace = true;
- element.content = 'CC=\u200Btest@google.com';
- await element.updateComplete;
-
- assert.equal(
- queryAndAssert(element, 'span#output').textContent,
- 'CC=test@google.com'
- );
- assert.equal(
- queryAndAssert(element, 'span#output').innerHTML.match(/(CC=<a)/g)!
- .length,
- 1
- );
- });
-
- test('only {http,https,mailto} protocols are linkified', async () => {
- element.content = 'xx mailto:test@google.com yy';
- await element.updateComplete;
-
- let links = queryAndAssert(element, 'span#output').querySelectorAll('a');
- assert.equal(links.length, 1);
- assert.equal(links[0].getAttribute('href'), 'mailto:test@google.com');
- assert.equal(links[0].innerHTML, 'mailto:test@google.com');
-
- element.content = 'xx http://google.com yy';
- await element.updateComplete;
-
- links = queryAndAssert(element, 'span#output').querySelectorAll('a');
- assert.equal(links.length, 1);
- assert.equal(links[0].getAttribute('href'), 'http://google.com');
- assert.equal(links[0].innerHTML, 'http://google.com');
-
- element.content = 'xx https://google.com yy';
- await element.updateComplete;
-
- links = queryAndAssert(element, 'span#output').querySelectorAll('a');
- assert.equal(links.length, 1);
- assert.equal(links[0].getAttribute('href'), 'https://google.com');
- assert.equal(links[0].innerHTML, 'https://google.com');
-
- element.content = 'xx ssh://google.com yy';
- await element.updateComplete;
-
- links = queryAndAssert(element, 'span#output').querySelectorAll('a');
- assert.equal(links.length, 0);
-
- element.content = 'xx ftp://google.com yy';
- await element.updateComplete;
-
- links = queryAndAssert(element, 'span#output').querySelectorAll('a');
- assert.equal(links.length, 0);
- });
-
- test('links without leading whitespace are linkified', async () => {
- element.content = 'xx abcmailto:test@google.com yy';
- await element.updateComplete;
-
- assert.equal(
- queryAndAssert(element, 'span#output').innerHTML.substr(0, 6),
- 'xx abc'
- );
- let links = queryAndAssert(element, 'span#output').querySelectorAll('a');
- assert.equal(links.length, 1);
- assert.equal(links[0].getAttribute('href'), 'mailto:test@google.com');
- assert.equal(links[0].innerHTML, 'mailto:test@google.com');
-
- element.content = 'xx defhttp://google.com yy';
- await element.updateComplete;
-
- assert.equal(
- queryAndAssert(element, 'span#output').innerHTML.substr(0, 6),
- 'xx def'
- );
- links = queryAndAssert(element, 'span#output').querySelectorAll('a');
- assert.equal(links.length, 1);
- assert.equal(links[0].getAttribute('href'), 'http://google.com');
- assert.equal(links[0].innerHTML, 'http://google.com');
-
- element.content = 'xx qwehttps://google.com yy';
- await element.updateComplete;
-
- assert.equal(
- queryAndAssert(element, 'span#output').innerHTML.substr(0, 6),
- 'xx qwe'
- );
- links = queryAndAssert(element, 'span#output').querySelectorAll('a');
- assert.equal(links.length, 1);
- assert.equal(links[0].getAttribute('href'), 'https://google.com');
- assert.equal(links[0].innerHTML, 'https://google.com');
-
- // Non-latin character
- element.content = 'xx абвhttps://google.com yy';
- await element.updateComplete;
-
- assert.equal(
- queryAndAssert(element, 'span#output').innerHTML.substr(0, 6),
- 'xx абв'
- );
- links = queryAndAssert(element, 'span#output').querySelectorAll('a');
- assert.equal(links.length, 1);
- assert.equal(links[0].getAttribute('href'), 'https://google.com');
- assert.equal(links[0].innerHTML, 'https://google.com');
-
- element.content = 'xx ssh://google.com yy';
- await element.updateComplete;
-
- links = queryAndAssert(element, 'span#output').querySelectorAll('a');
- assert.equal(links.length, 0);
-
- element.content = 'xx ftp://google.com yy';
- await element.updateComplete;
-
- links = queryAndAssert(element, 'span#output').querySelectorAll('a');
- assert.equal(links.length, 0);
- });
-
- test('overlapping links', async () => {
- element.config = {
- b1: {
- match: '(B:\\s*)(\\d+)',
- html: '$1<a href="ftp://foo/$2">$2</a>',
- },
- b2: {
- match: '(B:\\s*\\d+\\s*,\\s*)(\\d+)',
- html: '$1<a href="ftp://foo/$2">$2</a>',
- },
- };
- element.content = '- B: 123, 45';
- await element.updateComplete;
-
- const links = element.querySelectorAll('a');
-
- assert.equal(links.length, 2);
- assert.equal(
- queryAndAssert<HTMLSpanElement>(element, 'span').textContent,
- '- B: 123, 45'
- );
-
- assert.equal(links[0].href, 'ftp://foo/123');
- assert.equal(links[0].textContent, '123');
-
- assert.equal(links[1].href, 'ftp://foo/45');
- assert.equal(links[1].textContent, '45');
- });
-
- test('_contentOrConfigChanged called with config', async () => {
- const contentConfigStub = sinon.stub(element, '_contentOrConfigChanged');
- element.content = 'some text';
- await element.updateComplete;
-
- assert.isTrue(contentConfigStub.called);
- });
-});
diff --git a/polygerrit-ui/app/elements/shared/gr-linked-text/link-text-parser.ts b/polygerrit-ui/app/elements/shared/gr-linked-text/link-text-parser.ts
deleted file mode 100644
index 73cf58b..0000000
--- a/polygerrit-ui/app/elements/shared/gr-linked-text/link-text-parser.ts
+++ /dev/null
@@ -1,415 +0,0 @@
-/**
- * @license
- * Copyright 2015 Google LLC
- * SPDX-License-Identifier: Apache-2.0
- */
-import 'ba-linkify/ba-linkify';
-import {getBaseUrl} from '../../../utils/url-util';
-import {CommentLinkInfo} from '../../../types/common';
-
-/**
- * Pattern describing URLs with supported protocols.
- */
-const URL_PROTOCOL_PATTERN = /^(.*)(https?:\/\/|mailto:)/;
-
-export type LinkTextParserCallback = ((text: string, href: string) => void) &
- ((text: null, href: null, fragment: DocumentFragment) => void);
-
-export interface CommentLinkItem {
- position: number;
- length: number;
- html: HTMLAnchorElement | DocumentFragment;
-}
-
-export type LinkTextParserConfig = {[name: string]: CommentLinkInfo};
-
-export class GrLinkTextParser {
- private readonly baseUrl = getBaseUrl();
-
- /**
- * Construct a parser for linkifying text. Will linkify plain URLs that appear
- * in the text as well as custom links if any are specified in the linkConfig
- * parameter.
- *
- * @param linkConfig Comment links as specified by the commentlinks field on a
- * project config.
- * @param callback The callback to be fired when an intermediate parse result
- * is emitted. The callback is passed text and href strings if a link is to
- * be created, or a document fragment otherwise.
- * @param removeZeroWidthSpace If true, zero-width spaces will be removed from
- * R=<email> and CC=<email> expressions.
- */
- constructor(
- private readonly linkConfig: LinkTextParserConfig,
- private readonly callback: LinkTextParserCallback,
- private readonly removeZeroWidthSpace?: boolean
- ) {
- Object.preventExtensions(this);
- }
-
- /**
- * Emit a callback to create a link element.
- *
- * @param text The text of the link.
- * @param href The URL to use as the href of the link.
- */
- addText(text: string, href: string) {
- if (!text) {
- return;
- }
- this.callback(text, href);
- }
-
- /**
- * Given the source text and a list of CommentLinkItem objects that were
- * generated by the commentlinks config, emit parsing callbacks.
- *
- * @param text The chuml of source text over which the outputArray items range.
- * @param outputArray The list of items to add resulting from commentlink
- * matches.
- */
- processLinks(text: string, outputArray: CommentLinkItem[]) {
- this.sortArrayReverse(outputArray);
- const fragment = document.createDocumentFragment();
- let cursor = text.length;
-
- // Start inserting linkified URLs from the end of the String. That way, the
- // string positions of the items don't change as we iterate through.
- outputArray.forEach(item => {
- // Add any text between the current linkified item and the item added
- // before if it exists.
- if (item.position + item.length !== cursor) {
- fragment.insertBefore(
- document.createTextNode(
- text.slice(item.position + item.length, cursor)
- ),
- fragment.firstChild
- );
- }
- fragment.insertBefore(item.html, fragment.firstChild);
- cursor = item.position;
- });
-
- // Add the beginning portion at the end.
- if (cursor !== 0) {
- fragment.insertBefore(
- document.createTextNode(text.slice(0, cursor)),
- fragment.firstChild
- );
- }
-
- this.callback(null, null, fragment);
- }
-
- /**
- * Sort the given array of CommentLinkItems such that the positions are in
- * reverse order.
- */
- sortArrayReverse(outputArray: CommentLinkItem[]) {
- outputArray.sort((a, b) => b.position - a.position);
- }
-
- addItem(
- text: string,
- href: string,
- html: null,
- position: number,
- length: number,
- outputArray: CommentLinkItem[]
- ): void;
-
- addItem(
- text: null,
- href: null,
- html: string,
- position: number,
- length: number,
- outputArray: CommentLinkItem[]
- ): void;
-
- /**
- * Create a CommentLinkItem and append it to the given output array. This
- * method can be called in either of two ways:
- * - With `text` and `href` parameters provided, and the `html` parameter
- * passed as `null`. In this case, the new CommentLinkItem will be a link
- * element with the given text and href value.
- * - With the `html` paremeter provided, and the `text` and `href` parameters
- * passed as `null`. In this case, the string of HTML will be parsed and the
- * first resulting node will be used as the resulting content.
- *
- * @param text The text to use if creating a link.
- * @param href The href to use as the URL if creating a link.
- * @param html The html to parse and use as the result.
- * @param position The position inside the source text where the item
- * starts.
- * @param length The number of characters in the source text
- * represented by the item.
- * @param outputArray The array to which the
- * new item is to be appended.
- */
- addItem(
- text: string | null,
- href: string | null,
- html: string | null,
- position: number,
- length: number,
- outputArray: CommentLinkItem[]
- ): void {
- if (href) {
- const a = document.createElement('a');
- a.setAttribute('href', href);
- a.textContent = text;
- a.target = '_blank';
- a.rel = 'noopener';
- outputArray.push({
- html: a,
- position,
- length,
- });
- } else if (html) {
- // addItem has 2 overloads. If href is null, then html
- // can't be null.
- // TODO(TS): remove if(html) and keep else block without condition
- const fragment = document.createDocumentFragment();
- // Create temporary div to hold the nodes in.
- const div = document.createElement('div');
- div.innerHTML = html;
- while (div.firstChild) {
- fragment.appendChild(div.firstChild);
- }
- outputArray.push({
- html: fragment,
- position,
- length,
- });
- }
- }
-
- /**
- * Create a CommentLinkItem for a link and append it to the given output
- * array.
- *
- * @param text The text for the link.
- * @param href The href to use as the URL of the link.
- * @param position The position inside the source text where the link
- * starts.
- * @param length The number of characters in the source text
- * represented by the link.
- * @param outputArray The array to which the
- * new item is to be appended.
- */
- addLink(
- text: string,
- href: string,
- position: number,
- length: number,
- outputArray: CommentLinkItem[]
- ) {
- // TODO(TS): remove !test condition
- if (!text || this.hasOverlap(position, length, outputArray)) {
- return;
- }
- if (
- !!this.baseUrl &&
- href.startsWith('/') &&
- !href.startsWith(this.baseUrl)
- ) {
- href = this.baseUrl + href;
- }
- this.addItem(text, href, null, position, length, outputArray);
- }
-
- /**
- * Create a CommentLinkItem specified by an HTMl string and append it to the
- * given output array.
- *
- * @param html The html to parse and use as the result.
- * @param position The position inside the source text where the item
- * starts.
- * @param length The number of characters in the source text
- * represented by the item.
- * @param outputArray The array to which the
- * new item is to be appended.
- */
- addHTML(
- html: string,
- position: number,
- length: number,
- outputArray: CommentLinkItem[]
- ) {
- if (this.hasOverlap(position, length, outputArray)) {
- return;
- }
- if (
- !!this.baseUrl &&
- html.match(/<a href="\//g) &&
- !new RegExp(`<a href="${this.baseUrl}`, 'g').test(html)
- ) {
- html = html.replace(/<a href="\//g, `<a href="${this.baseUrl}/`);
- }
- this.addItem(null, null, html, position, length, outputArray);
- }
-
- /**
- * Does the given range overlap with anything already in the item list.
- */
- hasOverlap(position: number, length: number, outputArray: CommentLinkItem[]) {
- const endPosition = position + length;
- for (let i = 0; i < outputArray.length; i++) {
- const arrayItemStart = outputArray[i].position;
- const arrayItemEnd = outputArray[i].position + outputArray[i].length;
- if (
- (position >= arrayItemStart && position < arrayItemEnd) ||
- (endPosition > arrayItemStart && endPosition <= arrayItemEnd) ||
- (position === arrayItemStart && position === arrayItemEnd)
- ) {
- return true;
- }
- }
- return false;
- }
-
- /**
- * Parse the given source text and emit callbacks for the items that are
- * parsed.
- */
- parse(text?: string | null) {
- if (text) {
- window.linkify(text, {
- callback: (text: string, href?: string) => this.parseChunk(text, href),
- });
- }
- }
-
- /**
- * Callback that is pased into the linkify function. ba-linkify will call this
- * method in either of two ways:
- * - With both a `text` and `href` parameter provided: this indicates that
- * ba-linkify has found a plain URL and wants it linkified.
- * - With only a `text` parameter provided: this represents the non-link
- * content that lies between the links the library has found.
- *
- */
- parseChunk(text: string, href?: string) {
- // TODO(wyatta) switch linkify sequence, see issue 5526.
- if (this.removeZeroWidthSpace) {
- // Remove the zero-width space added in gr-change-view.
- text = text.replace(/^(CC|R)=\u200B/gm, '$1=');
- }
-
- // If the href is provided then ba-linkify has recognized it as a URL. If
- // the source text does not include a protocol, the protocol will be added
- // by ba-linkify. Create the link if the href is provided and its protocol
- // matches the expected pattern.
- if (href) {
- const result = URL_PROTOCOL_PATTERN.exec(href);
- if (result) {
- const prefixText = result[1];
- if (prefixText.length > 0) {
- // Fix for simple cases from
- // https://bugs.chromium.org/p/gerrit/issues/detail?id=11697
- // When leading whitespace is missed before link,
- // linkify add this text before link as a schema name to href.
- // We suppose, that prefixText just a single word
- // before link and add this word as is, without processing
- // any patterns in it.
- this.parseLinks(prefixText, {});
- text = text.substring(prefixText.length);
- href = href.substring(prefixText.length);
- }
- this.addText(text, href);
- return;
- }
- }
- // For the sections of text that lie between the links found by
- // ba-linkify, we search for the project-config-specified link patterns.
- this.parseLinks(text, this.linkConfig);
- }
-
- /**
- * Walk over the given source text to find matches for comemntlink patterns
- * and emit parse result callbacks.
- *
- * @param text The raw source text.
- * @param config A comment links specification object.
- */
- parseLinks(text: string, config: LinkTextParserConfig) {
- // The outputArray is used to store all of the matches found for all
- // patterns.
- const outputArray: CommentLinkItem[] = [];
- for (const [configName, linkInfo] of Object.entries(config)) {
- // TODO(TS): it seems, the following line can be rewritten as:
- // if(enabled === false || enabled === 0 || enabled === '')
- // Should be double-checked before update
- // eslint-disable-next-line eqeqeq
- if (linkInfo.enabled != null && linkInfo.enabled == false) {
- continue;
- }
- // PolyGerrit doesn't use hash-based navigation like the GWT UI.
- // Account for this.
- const html = linkInfo.html;
- const link = linkInfo.link;
- if (html) {
- linkInfo.html = html.replace(/<a href="#\//g, '<a href="/');
- } else if (link) {
- if (link[0] === '#') {
- linkInfo.link = link.substr(1);
- }
- }
-
- const pattern = new RegExp(linkInfo.match, 'g');
-
- let match;
- let textToCheck = text;
- let susbtrIndex = 0;
-
- while ((match = pattern.exec(textToCheck))) {
- textToCheck = textToCheck.substr(match.index + match[0].length);
- let result = match[0].replace(
- pattern,
- // Either html or link has a value. Otherwise an exception is thrown
- // in the code below.
- (linkInfo.html || linkInfo.link)!
- );
-
- if (linkInfo.html) {
- let i;
- // Skip portion of replacement string that is equal to original to
- // allow overlapping patterns.
- for (i = 0; i < result.length; i++) {
- if (result[i] !== match[0][i]) {
- break;
- }
- }
- result = result.slice(i);
-
- this.addHTML(
- result,
- susbtrIndex + match.index + i,
- match[0].length - i,
- outputArray
- );
- } else if (linkInfo.link) {
- this.addLink(
- match[0],
- result,
- susbtrIndex + match.index,
- match[0].length,
- outputArray
- );
- } else {
- throw Error(
- 'linkconfig entry ' +
- configName +
- ' doesn’t contain a link or html attribute.'
- );
- }
-
- // Update the substring location so we know where we are in relation to
- // the initial full text string.
- susbtrIndex = susbtrIndex + match.index + match[0].length;
- }
- }
- this.processLinks(text, outputArray);
- }
-}
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-image-viewer/gr-image-viewer.ts b/polygerrit-ui/app/embed/diff/gr-diff-image-viewer/gr-image-viewer.ts
index fe4632b..8a92bcc 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff-image-viewer/gr-image-viewer.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff-image-viewer/gr-image-viewer.ts
@@ -13,6 +13,7 @@
import '@polymer/paper-listbox/paper-listbox';
import './gr-overview-image';
import './gr-zoomed-image';
+import '../../../elements/shared/gr-icons/gr-icons';
import {GrLibLoader} from '../../../elements/shared/gr-lib-loader/gr-lib-loader';
import {RESEMBLEJS_LIBRARY_CONFIG} from '../../../elements/shared/gr-lib-loader/resemblejs_config';
diff --git a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff.ts b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff.ts
index f0697df..53c2780 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff.ts
@@ -5,7 +5,6 @@
*/
import '../../../styles/shared-styles';
import '../../../elements/shared/gr-button/gr-button';
-import '../../../elements/shared/gr-icons/gr-icons';
import '../../../elements/shared/gr-icon/gr-icon';
import '../gr-diff-builder/gr-diff-builder-element';
import '../gr-diff-highlight/gr-diff-highlight';
diff --git a/polygerrit-ui/app/models/browser/browser-model.ts b/polygerrit-ui/app/models/browser/browser-model.ts
index 9b69b24..1592cd8 100644
--- a/polygerrit-ui/app/models/browser/browser-model.ts
+++ b/polygerrit-ui/app/models/browser/browser-model.ts
@@ -4,12 +4,12 @@
* SPDX-License-Identifier: Apache-2.0
*/
import {Observable, combineLatest} from 'rxjs';
-import {distinctUntilChanged, map} from 'rxjs/operators';
import {Finalizable} from '../../services/registry';
import {define} from '../dependency';
import {DiffViewMode} from '../../api/diff';
import {UserModel} from '../user/user-model';
import {Model} from '../model';
+import {select} from '../../utils/observable-util';
// This value is somewhat arbitrary and not based on research or calculations.
const MAX_UNIFIED_DEFAULT_WINDOW_WIDTH_PX = 850;
@@ -27,30 +27,24 @@
export const browserModelToken = define<BrowserModel>('browser-model');
export class BrowserModel extends Model<BrowserState> implements Finalizable {
- readonly diffViewMode$: Observable<DiffViewMode>;
+ private readonly isScreenTooSmall$ = select(
+ this.state$,
+ state =>
+ !!state.screenWidth &&
+ state.screenWidth < MAX_UNIFIED_DEFAULT_WINDOW_WIDTH_PX
+ );
+
+ readonly diffViewMode$: Observable<DiffViewMode> = select(
+ combineLatest([
+ this.isScreenTooSmall$,
+ this.userModel.preferenceDiffViewMode$,
+ ]),
+ ([isScreenTooSmall, preferenceDiffViewMode]) =>
+ isScreenTooSmall ? DiffViewMode.UNIFIED : preferenceDiffViewMode
+ );
constructor(readonly userModel: UserModel) {
super(initialState);
- const screenWidth$ = this.state$.pipe(
- map(
- state =>
- !!state.screenWidth &&
- state.screenWidth < MAX_UNIFIED_DEFAULT_WINDOW_WIDTH_PX
- ),
- distinctUntilChanged()
- );
- // TODO; Inject the UserModel once preferenceDiffViewMode$ has moved to
- // the user model.
- this.diffViewMode$ = combineLatest([
- screenWidth$,
- userModel.preferenceDiffViewMode$,
- ]).pipe(
- map(([isScreenTooSmall, preferenceDiffViewMode]) => {
- if (isScreenTooSmall) return DiffViewMode.UNIFIED;
- else return preferenceDiffViewMode;
- }),
- distinctUntilChanged()
- );
}
/* Observe the screen width so that the app can react to changes to it */
diff --git a/polygerrit-ui/app/models/change/change-model.ts b/polygerrit-ui/app/models/change/change-model.ts
index 5cfe670..35a08c6 100644
--- a/polygerrit-ui/app/models/change/change-model.ts
+++ b/polygerrit-ui/app/models/change/change-model.ts
@@ -14,20 +14,11 @@
RevisionPatchSetNum,
} from '../../types/common';
import {DefaultBase} from '../../constants/constants';
-import {
- combineLatest,
- from,
- fromEvent,
- Observable,
- Subscription,
- forkJoin,
- of,
-} from 'rxjs';
+import {combineLatest, from, fromEvent, Observable, forkJoin, of} from 'rxjs';
import {
map,
filter,
withLatestFrom,
- distinctUntilChanged,
startWith,
switchMap,
} from 'rxjs/operators';
@@ -234,35 +225,33 @@
* out inconsistent state, e.g. router changeNum already updated, change not
* yet reset to undefined.
*/
- combineLatest([this.routerModel.state$, this.state$, this.userModel.state$])
- .pipe(
+ select(
+ combineLatest([
+ this.routerModel.state$,
+ this.state$,
+ this.userModel.state$,
+ ]).pipe(
filter(([routerState, changeState, _]) => {
const changeNum = changeState.change?._number;
const routerChangeNum = routerState.changeNum;
return changeNum === undefined || changeNum === routerChangeNum;
}),
- distinctUntilChanged()
- )
- .pipe(
withLatestFrom(
this.routerModel.routerBasePatchNum$,
this.patchNum$,
this.change$,
this.userModel.preferences$
- ),
- map(([_, routerBasePatchNum, patchNum, change, preferences]) =>
- computeBase(routerBasePatchNum, patchNum, change, preferences)
- ),
- distinctUntilChanged()
- );
+ )
+ ),
+ ([_, routerBasePatchNum, patchNum, change, preferences]) =>
+ computeBase(routerBasePatchNum, patchNum, change, preferences)
+ );
public readonly isOwner$: Observable<boolean> = select(
combineLatest([this.change$, this.userModel.account$]),
([change, account]) => isOwner(change, account)
);
- private subscriptions: Subscription[] = [];
-
// For usage in `combineLatest` we need `startWith` such that reload$ has an
// initial value.
readonly reload$: Observable<unknown> = fromEvent(document, 'reload').pipe(
@@ -316,13 +305,6 @@
];
}
- override finalize() {
- for (const s of this.subscriptions) {
- s.unsubscribe();
- }
- this.subscriptions = [];
- }
-
// Temporary workaround until path is derived in the model itself.
updatePath(diffPath?: string) {
this.updateState({diffPath});
diff --git a/polygerrit-ui/app/models/change/change-model_test.ts b/polygerrit-ui/app/models/change/change-model_test.ts
index 0ee508a..fdf9e04 100644
--- a/polygerrit-ui/app/models/change/change-model_test.ts
+++ b/polygerrit-ui/app/models/change/change-model_test.ts
@@ -22,6 +22,7 @@
CommitId,
EDIT,
NumericChangeId,
+ PARENT,
PatchSetNum,
PatchSetNumber,
} from '../../types/common';
@@ -63,7 +64,7 @@
});
});
-suite('change service tests', () => {
+suite('change model tests', () => {
let changeModel: ChangeModel;
let knownChange: ParsedChangeInfo;
const testCompleted = new Subject<void>();
@@ -279,4 +280,25 @@
message: 'blah blah',
});
});
+
+ // At some point we had forgotten the `select()` wrapper for this selector.
+ // And the missing `replay` led to a bug that was hard to find. That is why
+ // we are testing this explicitly here.
+ test('basePatchNum$ selector', async () => {
+ const spy = sinon.spy();
+ changeModel.basePatchNum$.subscribe(spy);
+
+ // test replay
+ assert.equal(spy.callCount, 1);
+ assert.equal(spy.lastCall.firstArg, PARENT);
+
+ // test update
+ changeModel.routerModel.updateState({basePatchNum: 1 as PatchSetNumber});
+ assert.equal(spy.callCount, 2);
+ assert.equal(spy.lastCall.firstArg, 1 as PatchSetNumber);
+
+ // test distinctUntilChanged
+ changeModel.updateStateChange(createParsedChange());
+ assert.equal(spy.callCount, 2);
+ });
});
diff --git a/polygerrit-ui/app/models/change/files-model.ts b/polygerrit-ui/app/models/change/files-model.ts
index 31e42a6..6922f6d 100644
--- a/polygerrit-ui/app/models/change/files-model.ts
+++ b/polygerrit-ui/app/models/change/files-model.ts
@@ -12,7 +12,7 @@
PatchSetNumber,
RevisionPatchSetNum,
} from '../../types/common';
-import {combineLatest, Subscription, of, from} from 'rxjs';
+import {combineLatest, of, from} from 'rxjs';
import {switchMap, map} from 'rxjs/operators';
import {RestApiService} from '../../services/gr-rest-api/gr-rest-api';
import {Finalizable} from '../../services/registry';
@@ -131,8 +131,6 @@
state => state.filesRightBase
);
- private subscriptions: Subscription[] = [];
-
constructor(
readonly changeModel: ChangeModel,
readonly commentsModel: CommentsModel,
@@ -198,11 +196,4 @@
this.updateState(state);
});
}
-
- override finalize() {
- for (const s of this.subscriptions) {
- s.unsubscribe();
- }
- this.subscriptions = [];
- }
}
diff --git a/polygerrit-ui/app/models/checks/checks-model.ts b/polygerrit-ui/app/models/checks/checks-model.ts
index 2b92529..9c4c45c 100644
--- a/polygerrit-ui/app/models/checks/checks-model.ts
+++ b/polygerrit-ui/app/models/checks/checks-model.ts
@@ -20,7 +20,6 @@
Observable,
of,
Subject,
- Subscription,
timer,
} from 'rxjs';
import {
@@ -151,7 +150,12 @@
};
}
-const FETCH_RESULT_TIMEOUT_MS = 10000;
+/**
+ * Android's Checks Plugin has a 15s timeout internally. So we are using
+ * something slightly larger, so that we get a proper error from the plugin,
+ * if they run into timeout issues.
+ */
+const FETCH_RESULT_TIMEOUT_MS = 16000;
/**
* Can be used in `reduce()` to collect all results from all runs from all
@@ -185,9 +189,11 @@
private checkToPluginMap = new Map<string, string>();
- private changeNum?: NumericChangeId;
+ // visible for testing
+ changeNum?: NumericChangeId;
- private latestPatchNum?: PatchSetNumber;
+ // visible for testing
+ latestPatchNum?: PatchSetNumber;
private readonly documentVisibilityChange$ = new BehaviorSubject(undefined);
@@ -195,9 +201,6 @@
private readonly visibilityChangeListener: () => void;
- private subscriptions: Subscription[] = [];
-
- // TODO: Maybe consider migrating usages to `changeViewModel` directly.
public checksSelectedPatchsetNumber$ = select(
this.changeViewModel.checksPatchset$,
ps => ps
@@ -384,6 +387,9 @@
this.reporting.time(Timing.CHECKS_LOAD);
this.subscriptions = [
this.changeModel.changeNum$.subscribe(x => (this.changeNum = x)),
+ this.changeModel.latestPatchNum$.subscribe(
+ x => (this.latestPatchNum = x)
+ ),
this.pluginsModel.checksPlugins$.subscribe(plugins => {
for (const plugin of plugins) {
this.register(plugin);
@@ -461,10 +467,7 @@
'visibilitychange',
this.visibilityChangeListener
);
- for (const s of this.subscriptions) {
- s.unsubscribe();
- }
- this.subscriptions = [];
+ super.finalize();
}
// Must only be used by the checks service or whatever is in control of this
@@ -750,8 +753,10 @@
patchset === ChecksPatchset.LATEST
? this.changeModel.latestPatchNum$
: this.checksSelectedPatchsetNumber$,
- this.reloadSubjects[pluginName].pipe(throttleTime(1000)),
- timer(0, pollIntervalMs),
+ this.reloadSubjects[pluginName].pipe(
+ throttleTime(1000, undefined, {trailing: true, leading: true})
+ ),
+ pollIntervalMs === 0 ? from([0]) : timer(0, pollIntervalMs),
this.documentVisibilityChange$,
])
.pipe(
diff --git a/polygerrit-ui/app/models/checks/checks-model_test.ts b/polygerrit-ui/app/models/checks/checks-model_test.ts
index 83ed464..3489c5a 100644
--- a/polygerrit-ui/app/models/checks/checks-model_test.ts
+++ b/polygerrit-ui/app/models/checks/checks-model_test.ts
@@ -7,6 +7,7 @@
import './checks-model';
import {ChecksModel, ChecksPatchset, ChecksProviderState} from './checks-model';
import {
+ Action,
Category,
CheckRun,
ChecksApiConfig,
@@ -22,6 +23,7 @@
import {assert} from '@open-wc/testing';
import {testResolver} from '../../test/common-test-setup';
import {changeViewModelToken} from '../views/change';
+import {NumericChangeId, PatchSetNumber} from '../../api/rest-api';
const PLUGIN_NAME = 'test-plugin';
@@ -42,8 +44,12 @@
},
];
-const CONFIG: ChecksApiConfig = {
- fetchPollingIntervalSeconds: 1000,
+const CONFIG_POLLING_5S: ChecksApiConfig = {
+ fetchPollingIntervalSeconds: 5,
+};
+
+const CONFIG_POLLING_NONE: ChecksApiConfig = {
+ fetchPollingIntervalSeconds: 0,
};
function createProvider(): ChecksProvider {
@@ -82,13 +88,67 @@
const provider = createProvider();
const fetchSpy = sinon.spy(provider, 'fetch');
- model.register({pluginName: 'test-plugin', provider, config: CONFIG});
+ model.register({
+ pluginName: 'test-plugin',
+ provider,
+ config: CONFIG_POLLING_NONE,
+ });
await waitUntil(() => change === undefined);
const testChange = createParsedChange();
model.changeModel.updateStateChange(testChange);
await waitUntil(() => change === testChange);
await waitUntilCalled(fetchSpy, 'fetch');
+
+ assert.equal(
+ model.latestPatchNum,
+ testChange.revisions[testChange.current_revision]
+ ._number as PatchSetNumber
+ );
+ assert.equal(model.changeNum, testChange._number);
+ });
+
+ test('reload throttle', async () => {
+ const clock = sinon.useFakeTimers();
+ let change: ParsedChangeInfo | undefined = undefined;
+ model.changeModel.change$.subscribe(c => (change = c));
+ const provider = createProvider();
+ const fetchSpy = sinon.spy(provider, 'fetch');
+
+ model.register({
+ pluginName: 'test-plugin',
+ provider,
+ config: CONFIG_POLLING_NONE,
+ });
+ await waitUntil(() => change === undefined);
+
+ const testChange = createParsedChange();
+ model.changeModel.updateStateChange(testChange);
+ await waitUntil(() => change === testChange);
+ clock.tick(1);
+ assert.equal(fetchSpy.callCount, 1);
+
+ // The second reload call will be processed, but only after a 1s throttle.
+ model.reload('test-plugin');
+ clock.tick(100);
+ assert.equal(fetchSpy.callCount, 1);
+ // 2000 ms is greater than the 1000 ms throttle time.
+ clock.tick(2000);
+ assert.equal(fetchSpy.callCount, 2);
+ });
+
+ test('triggerAction', async () => {
+ model.changeNum = 314 as NumericChangeId;
+ model.latestPatchNum = 13 as PatchSetNumber;
+ const action: Action = {
+ name: 'test action',
+ callback: () => undefined,
+ };
+ const spy = sinon.spy(action, 'callback');
+ model.triggerAction(action, undefined, 'none');
+ assert.isTrue(spy.calledOnce);
+ assert.equal(spy.lastCall.args[0], 314);
+ assert.equal(spy.lastCall.args[1], 13);
});
test('model.updateStateSetProvider', () => {
@@ -204,4 +264,58 @@
assert.lengthOf(current.runs[0].results!, 1);
assert.equal(current.runs[0].results![0].summary, 'new');
});
+
+ test('polls for changes', async () => {
+ const clock = sinon.useFakeTimers();
+ let change: ParsedChangeInfo | undefined = undefined;
+ model.changeModel.change$.subscribe(c => (change = c));
+ const provider = createProvider();
+ const fetchSpy = sinon.spy(provider, 'fetch');
+
+ model.register({
+ pluginName: 'test-plugin',
+ provider,
+ config: CONFIG_POLLING_5S,
+ });
+ await waitUntil(() => change === undefined);
+ clock.tick(1);
+ const testChange = createParsedChange();
+ model.changeModel.updateStateChange(testChange);
+ await waitUntil(() => change === testChange);
+ await waitUntilCalled(fetchSpy, 'fetch');
+ clock.tick(1);
+ const pollCount = fetchSpy.callCount;
+
+ // polling should continue while we wait
+ clock.tick(CONFIG_POLLING_5S.fetchPollingIntervalSeconds * 1000 * 2);
+
+ assert.isTrue(fetchSpy.callCount > pollCount);
+ });
+
+ test('does not poll when config specifies 0 seconds', async () => {
+ const clock = sinon.useFakeTimers();
+ let change: ParsedChangeInfo | undefined = undefined;
+ model.changeModel.change$.subscribe(c => (change = c));
+ const provider = createProvider();
+ const fetchSpy = sinon.spy(provider, 'fetch');
+
+ model.register({
+ pluginName: 'test-plugin',
+ provider,
+ config: CONFIG_POLLING_NONE,
+ });
+ await waitUntil(() => change === undefined);
+ clock.tick(1);
+ const testChange = createParsedChange();
+ model.changeModel.updateStateChange(testChange);
+ await waitUntil(() => change === testChange);
+ await waitUntilCalled(fetchSpy, 'fetch');
+ clock.tick(1);
+ const pollCount = fetchSpy.callCount;
+
+ // polling should not happen
+ clock.tick(60 * 1000);
+
+ assert.equal(fetchSpy.callCount, pollCount);
+ });
});
diff --git a/polygerrit-ui/app/models/checks/checks-util.ts b/polygerrit-ui/app/models/checks/checks-util.ts
index 09c9273..7ccdf91 100644
--- a/polygerrit-ui/app/models/checks/checks-util.ts
+++ b/polygerrit-ui/app/models/checks/checks-util.ts
@@ -8,12 +8,16 @@
Category,
CheckResult as CheckResultApi,
CheckRun as CheckRunApi,
+ Fix,
Link,
LinkIcon,
+ Replacement,
RunStatus,
} from '../../api/checks';
import {PatchSetNumber} from '../../api/rest-api';
+import {FixSuggestionInfo, FixReplacementInfo} from '../../types/common';
import {OpenFixPreviewEventDetail} from '../../types/events';
+import {notUndefined} from '../../types/types';
import {PROVIDED_FIX_ID} from '../../utils/comment-util';
import {assert, assertNever} from '../../utils/common-util';
import {fire} from '../../utils/event-util';
@@ -86,17 +90,15 @@
target: EventTarget,
result?: RunResult
): Action | undefined {
- const fixes = result?.fixes;
- if (!fixes || fixes?.length === 0 || !result?.patchset) return;
+ if (!result?.patchset) return;
+ if (!result?.fixes) return;
+ const fixSuggestions = result.fixes
+ .map(f => rectifyFix(f, result?.checkName))
+ .filter(notUndefined);
+ if (fixSuggestions.length === 0) return;
const eventDetail: OpenFixPreviewEventDetail = {
- patchNum: result?.patchset as PatchSetNumber,
- fixSuggestions: fixes.map(fix => {
- return {
- description: `Fix provided by ${result?.checkName}`,
- fix_id: PROVIDED_FIX_ID,
- ...fix,
- };
- }),
+ patchNum: result.patchset as PatchSetNumber,
+ fixSuggestions,
};
return {
name: 'Show Fix',
@@ -107,6 +109,36 @@
};
}
+export function rectifyFix(
+ fix: Fix | undefined,
+ checkName: string
+): FixSuggestionInfo | undefined {
+ if (!fix?.replacements) return undefined;
+ const replacements = fix.replacements
+ .map(rectifyReplacement)
+ .filter(notUndefined);
+ if (replacements.length === 0) return undefined;
+
+ return {
+ description: fix.description ?? `Fix provided by ${checkName}`,
+ fix_id: PROVIDED_FIX_ID,
+ replacements,
+ };
+}
+
+export function rectifyReplacement(
+ r: Replacement | undefined
+): FixReplacementInfo | undefined {
+ if (!r?.path) return undefined;
+ if (!r?.range) return undefined;
+ if (r?.replacement === undefined) return undefined;
+ if (!Number.isInteger(r.range.start_line)) return undefined;
+ if (!Number.isInteger(r.range.end_line)) return undefined;
+ if (!Number.isInteger(r.range.start_character)) return undefined;
+ if (!Number.isInteger(r.range.end_character)) return undefined;
+ return r;
+}
+
export function worstCategory(run: CheckRun) {
if (hasResultsOf(run, Category.ERROR)) return Category.ERROR;
if (hasResultsOf(run, Category.WARNING)) return Category.WARNING;
diff --git a/polygerrit-ui/app/models/checks/checks-util_test.ts b/polygerrit-ui/app/models/checks/checks-util_test.ts
index 5a7bd75..c237c59 100644
--- a/polygerrit-ui/app/models/checks/checks-util_test.ts
+++ b/polygerrit-ui/app/models/checks/checks-util_test.ts
@@ -10,9 +10,13 @@
ALL_ATTEMPTS,
AttemptChoice,
LATEST_ATTEMPT,
+ rectifyFix,
sortAttemptChoices,
stringToAttemptChoice,
} from './checks-util';
+import {Fix, Replacement} from '../../api/checks';
+import {CommentRange} from '../../api/core';
+import {PROVIDED_FIX_ID} from '../../utils/comment-util';
suite('checks-util tests', () => {
setup(() => {});
@@ -33,6 +37,55 @@
assert.equal(stringToAttemptChoice('1x'), undefined);
});
+ test('rectifyFix', () => {
+ assert.isUndefined(rectifyFix(undefined, 'name'));
+ assert.isUndefined(rectifyFix({} as Fix, 'name'));
+ assert.isUndefined(
+ rectifyFix({description: 'asdf', replacements: []}, 'name')
+ );
+ assert.isUndefined(
+ rectifyFix(
+ {description: 'asdf', replacements: [{} as Replacement]},
+ 'test-check-name'
+ )
+ );
+ assert.isUndefined(
+ rectifyFix(
+ {
+ description: 'asdf',
+ replacements: [
+ {
+ path: 'test-path',
+ range: {} as CommentRange,
+ replacement: 'test-replacement-string',
+ },
+ ],
+ },
+ 'test-check-name'
+ )
+ );
+ const rectified = rectifyFix(
+ {
+ replacements: [
+ {
+ path: 'test-path',
+ range: {
+ start_line: 1,
+ end_line: 1,
+ start_character: 0,
+ end_character: 1,
+ } as CommentRange,
+ replacement: 'test-replacement-string',
+ },
+ ],
+ },
+ 'test-check-name'
+ );
+ assert.isDefined(rectified);
+ assert.equal(rectified?.description, 'Fix provided by test-check-name');
+ assert.equal(rectified?.fix_id, PROVIDED_FIX_ID);
+ });
+
test('sortAttemptChoices', () => {
const unsorted: (AttemptChoice | undefined)[] = [
3,
diff --git a/polygerrit-ui/app/models/comments/comments-model.ts b/polygerrit-ui/app/models/comments/comments-model.ts
index 39d9eaa..8ed4b69 100644
--- a/polygerrit-ui/app/models/comments/comments-model.ts
+++ b/polygerrit-ui/app/models/comments/comments-model.ts
@@ -30,7 +30,7 @@
import {RouterModel} from '../../services/router/router-model';
import {Finalizable} from '../../services/registry';
import {define} from '../dependency';
-import {combineLatest, forkJoin, from, Observable, Subscription} from 'rxjs';
+import {combineLatest, forkJoin, from, Observable} from 'rxjs';
import {fire, fireAlert, fireEvent} from '../../utils/event-util';
import {CURRENT} from '../../utils/patch-set-util';
import {RestApiService} from '../../services/gr-rest-api/gr-rest-api';
@@ -370,8 +370,6 @@
private readonly reloadListener: () => void;
- private readonly subscriptions: Subscription[] = [];
-
private drafts: {[path: string]: DraftInfo[]} = {};
private draftToastTask?: DelayedTask;
@@ -421,10 +419,7 @@
override finalize() {
document.removeEventListener('reload', this.reloadListener);
- for (const s of this.subscriptions) {
- s.unsubscribe();
- }
- this.subscriptions.splice(0, this.subscriptions.length);
+ super.finalize();
}
// Note that this does *not* reload ported comments.
diff --git a/polygerrit-ui/app/models/config/config-model.ts b/polygerrit-ui/app/models/config/config-model.ts
index c4bd637..6e374d1 100644
--- a/polygerrit-ui/app/models/config/config-model.ts
+++ b/polygerrit-ui/app/models/config/config-model.ts
@@ -4,7 +4,7 @@
* SPDX-License-Identifier: Apache-2.0
*/
import {ConfigInfo, RepoName, ServerInfo} from '../../types/common';
-import {from, of, Subscription} from 'rxjs';
+import {from, of} from 'rxjs';
import {switchMap} from 'rxjs/operators';
import {Finalizable} from '../../services/registry';
import {RestApiService} from '../../services/gr-rest-api/gr-rest-api';
@@ -50,8 +50,6 @@
url => url
);
- private subscriptions: Subscription[];
-
constructor(
readonly changeModel: ChangeModel,
readonly restApiService: RestApiService
@@ -83,11 +81,4 @@
updateServerConfig(serverConfig?: ServerInfo) {
this.updateState({serverConfig});
}
-
- override finalize() {
- for (const s of this.subscriptions) {
- s.unsubscribe();
- }
- this.subscriptions = [];
- }
}
diff --git a/polygerrit-ui/app/models/model.ts b/polygerrit-ui/app/models/model.ts
index 2e90a5e..19b52fc 100644
--- a/polygerrit-ui/app/models/model.ts
+++ b/polygerrit-ui/app/models/model.ts
@@ -3,7 +3,7 @@
* Copyright 2021 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
-import {BehaviorSubject, Observable} from 'rxjs';
+import {BehaviorSubject, Observable, Subscription} from 'rxjs';
import {Finalizable} from '../services/registry';
import {deepEqual} from '../utils/deep-util';
@@ -22,10 +22,19 @@
* Any new subscriber will immediately receive the current value.
*/
export abstract class Model<T> implements Finalizable {
+ /**
+ * rxjs does not like `next()` being called on a subject during processing of
+ * another `next()` call. So make sure that state updates complete before
+ * starting another one.
+ */
+ private stateUpdateInProgress = false;
+
private subject$: BehaviorSubject<T>;
public state$: Observable<T>;
+ protected subscriptions: Subscription[] = [];
+
constructor(initialState: T) {
this.subject$ = new BehaviorSubject(initialState);
this.state$ = this.subject$.asObservable();
@@ -36,15 +45,32 @@
}
setState(state: T) {
+ if (this.stateUpdateInProgress) {
+ setTimeout(() => this.setState(state));
+ return;
+ }
if (deepEqual(state, this.getState())) return;
- this.subject$.next(state);
+ try {
+ this.stateUpdateInProgress = true;
+ this.subject$.next(state);
+ } finally {
+ this.stateUpdateInProgress = false;
+ }
}
updateState(state: Partial<T>) {
+ if (this.stateUpdateInProgress) {
+ setTimeout(() => this.updateState(state));
+ return;
+ }
this.setState({...this.getState(), ...state});
}
finalize() {
this.subject$.complete();
+ for (const s of this.subscriptions) {
+ s.unsubscribe();
+ }
+ this.subscriptions = [];
}
}
diff --git a/polygerrit-ui/app/models/model_test.ts b/polygerrit-ui/app/models/model_test.ts
new file mode 100644
index 0000000..3fa88e7
--- /dev/null
+++ b/polygerrit-ui/app/models/model_test.ts
@@ -0,0 +1,65 @@
+/**
+ * @license
+ * Copyright 2022 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+import {assert, waitUntil} from '@open-wc/testing';
+import '../test/common-test-setup';
+import {Model} from './model';
+
+interface TestModelState {
+ prop1?: string;
+ prop2?: string;
+ prop3?: string;
+}
+
+export class TestModel extends Model<TestModelState> {
+ constructor() {
+ super({});
+ }
+}
+
+suite('model tests', () => {
+ test('setState update in progress', async () => {
+ const model = new TestModel();
+ let firstUpdateCompleted = false;
+ let secondUpdateCompleted = false;
+ model.state$.subscribe(s => {
+ if (s.prop2 === 'set') {
+ // Otherwise this would be a clear indication of a nested `setState()`
+ // call, which `stateUpdateInProgress` is supposed to avoid.
+ assert.isTrue(firstUpdateCompleted);
+ secondUpdateCompleted = true;
+ }
+ if (s.prop1 === 'set' && s.prop2 !== 'set')
+ model.setState({prop2: 'set'});
+ });
+
+ // This call should return before the subscriber calls `setState()` again.
+ model.setState({prop1: 'set'});
+ firstUpdateCompleted = true;
+
+ await waitUntil(() => secondUpdateCompleted);
+ });
+
+ test('updateState update in progress', async () => {
+ const model = new TestModel();
+ let completed = false;
+ model.state$.subscribe(s => {
+ if (s.prop1 !== 'go') return;
+ if (s.prop2 !== 'set' && s.prop3 !== 'set')
+ model.updateState({prop2: 'set'});
+ if (s.prop2 === 'set' && s.prop3 === 'set') completed = true;
+ });
+ model.state$.subscribe(s => {
+ if (s.prop1 !== 'go') return;
+ if (s.prop2 !== 'set' && s.prop3 !== 'set')
+ model.updateState({prop3: 'set'});
+ if (s.prop2 === 'set' && s.prop3 === 'set') completed = true;
+ });
+
+ model.updateState({prop1: 'go'});
+
+ await waitUntil(() => completed);
+ });
+});
diff --git a/polygerrit-ui/app/models/user/user-model.ts b/polygerrit-ui/app/models/user/user-model.ts
index b69c715..fa00a0b 100644
--- a/polygerrit-ui/app/models/user/user-model.ts
+++ b/polygerrit-ui/app/models/user/user-model.ts
@@ -3,7 +3,7 @@
* Copyright 2021 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
-import {from, of, Observable, Subscription} from 'rxjs';
+import {from, of, Observable} from 'rxjs';
import {switchMap} from 'rxjs/operators';
import {
DiffPreferencesInfo as DiffPreferencesInfoAPI,
@@ -83,8 +83,6 @@
preference => preference.theme
);
- private subscriptions: Subscription[] = [];
-
constructor(readonly restApiService: RestApiService) {
super({
preferences: createDefaultPreferences(),
@@ -140,13 +138,6 @@
];
}
- override finalize() {
- for (const s of this.subscriptions) {
- s.unsubscribe();
- }
- this.subscriptions = [];
- }
-
updatePreferences(prefs: Partial<PreferencesInfo>) {
return this.restApiService
.savePreferences(prefs)
diff --git a/polygerrit-ui/app/models/views/change.ts b/polygerrit-ui/app/models/views/change.ts
index 826ec66..100c46b 100644
--- a/polygerrit-ui/app/models/views/change.ts
+++ b/polygerrit-ui/app/models/views/change.ts
@@ -11,8 +11,10 @@
ChangeInfo,
PatchSetNumber,
} from '../../api/rest-api';
+import {Tab} from '../../constants/constants';
import {GerritView} from '../../services/router/router-model';
import {UrlEncodedCommentId} from '../../types/common';
+import {toggleSet} from '../../utils/common-util';
import {select} from '../../utils/observable-util';
import {
encodeURL,
@@ -33,7 +35,8 @@
patchNum?: RevisionPatchSetNum;
basePatchNum?: BasePatchSetNum;
commentId?: UrlEncodedCommentId;
- tab?: string;
+ /** This can be a string only for plugin provided tabs. */
+ tab?: Tab | string;
/** Checks related view state */
@@ -43,6 +46,10 @@
filter?: string;
/** selected attempt for check runs (undefined=latest) */
attempt?: AttemptChoice;
+ /** selected check runs identified by `checkName` */
+ checksRunsSelected?: Set<string>;
+ /** regular expression for filtering check results */
+ checksResultsFilter?: string;
/** State properties that trigger one-time actions */
@@ -107,6 +114,15 @@
if (state.filter) {
queries.push(`filter=${state.filter}`);
}
+ if (state.checksResultsFilter) {
+ queries.push(`checksResultsFilter=${state.checksResultsFilter}`);
+ }
+ if (state.checksRunsSelected && state.checksRunsSelected.size > 0) {
+ queries.push(`checksRunsSelected=${[...state.checksRunsSelected].sort()}`);
+ }
+ if (state.tab && state.tab !== Tab.FILES) {
+ queries.push(`tab=${state.tab}`);
+ }
if (state.forceReload) {
queries.push('forceReload=true');
}
@@ -130,9 +146,9 @@
}
if (state.project) {
const encodedProject = encodeURL(state.project, true);
- return getBaseUrl() + `/c/${encodedProject}/+/${state.changeNum}${suffix}`;
+ return `${getBaseUrl()}/c/${encodedProject}/+/${state.changeNum}${suffix}`;
} else {
- return getBaseUrl() + `/c/${state.changeNum}${suffix}`;
+ return `${getBaseUrl()}/c/${state.changeNum}${suffix}`;
}
}
@@ -151,6 +167,16 @@
public readonly filter$ = select(this.state$, state => state?.filter);
+ public readonly checksResultsFilter$ = select(
+ this.state$,
+ state => state?.checksResultsFilter ?? ''
+ );
+
+ public readonly checksRunsSelected$ = select(
+ this.state$,
+ state => state?.checksRunsSelected ?? new Set<string>()
+ );
+
constructor() {
super(undefined);
this.state$.subscribe(s => {
@@ -163,4 +189,11 @@
}
});
}
+
+ toggleSelectedCheckRun(checkName: string) {
+ const current = this.getState()?.checksRunsSelected ?? new Set();
+ const next = new Set(current);
+ toggleSet(next, checkName);
+ this.updateState({checksRunsSelected: next});
+ }
}
diff --git a/polygerrit-ui/app/models/views/change_test.ts b/polygerrit-ui/app/models/views/change_test.ts
index 2f05e9f..24ced82 100644
--- a/polygerrit-ui/app/models/views/change_test.ts
+++ b/polygerrit-ui/app/models/views/change_test.ts
@@ -14,13 +14,15 @@
import '../../test/common-test-setup';
import {createChangeUrl, ChangeViewState} from './change';
+const STATE: ChangeViewState = {
+ view: GerritView.CHANGE,
+ changeNum: 1234 as NumericChangeId,
+ project: 'test' as RepoName,
+};
+
suite('change view state tests', () => {
test('createChangeUrl()', () => {
- const state: ChangeViewState = {
- view: GerritView.CHANGE,
- changeNum: 1234 as NumericChangeId,
- project: 'test' as RepoName,
- };
+ const state: ChangeViewState = {...STATE};
assert.equal(createChangeUrl(state), '/c/test/+/1234');
@@ -34,6 +36,37 @@
assert.equal(createChangeUrl(state), '/c/test/+/1234/5..10#123');
});
+ test('createChangeUrl() baseUrl', () => {
+ window.CANONICAL_PATH = '/base';
+ const state: ChangeViewState = {...STATE};
+ assert.equal(createChangeUrl(state).substring(0, 5), '/base');
+ window.CANONICAL_PATH = undefined;
+ });
+
+ test('createChangeUrl() checksRunsSelected', () => {
+ const state: ChangeViewState = {
+ ...STATE,
+ checksRunsSelected: new Set(['asdf']),
+ };
+
+ assert.equal(
+ createChangeUrl(state),
+ '/c/test/+/1234?checksRunsSelected=asdf'
+ );
+ });
+
+ test('createChangeUrl() checksResultsFilter', () => {
+ const state: ChangeViewState = {
+ ...STATE,
+ checksResultsFilter: 'asdf.*qwer',
+ };
+
+ assert.equal(
+ createChangeUrl(state),
+ '/c/test/+/1234?checksResultsFilter=asdf.*qwer'
+ );
+ });
+
test('createChangeUrl() with repo name encoding', () => {
const state: ChangeViewState = {
view: GerritView.CHANGE,
diff --git a/polygerrit-ui/app/models/views/dashboard.ts b/polygerrit-ui/app/models/views/dashboard.ts
index 7141637..d9ff2d2 100644
--- a/polygerrit-ui/app/models/views/dashboard.ts
+++ b/polygerrit-ui/app/models/views/dashboard.ts
@@ -7,7 +7,7 @@
import {GerritView} from '../../services/router/router-model';
import {DashboardId} from '../../types/common';
import {DashboardSection} from '../../utils/dashboard-util';
-import {encodeURL} from '../../utils/url-util';
+import {encodeURL, getBaseUrl} from '../../utils/url-util';
import {define} from '../dependency';
import {Model} from '../model';
import {ViewState} from './base';
@@ -46,14 +46,14 @@
queryParams.push('title=' + encodeURIComponent(state.title));
}
const user = state.user ? state.user : '';
- return `/dashboard/${user}?${queryParams.join('&')}`;
+ return `${getBaseUrl()}/dashboard/${user}?${queryParams.join('&')}`;
} else if (repoName) {
// Project dashboard.
const encodedRepo = encodeURL(repoName, true);
- return `/p/${encodedRepo}/+/dashboard/${state.dashboard}`;
+ return `${getBaseUrl()}/p/${encodedRepo}/+/dashboard/${state.dashboard}`;
} else {
// User dashboard.
- return `/dashboard/${state.user || 'self'}`;
+ return `${getBaseUrl()}/dashboard/${state.user || 'self'}`;
}
}
diff --git a/polygerrit-ui/app/models/views/dashboard_test.ts b/polygerrit-ui/app/models/views/dashboard_test.ts
index 9deed72..86bb5c0 100644
--- a/polygerrit-ui/app/models/views/dashboard_test.ts
+++ b/polygerrit-ui/app/models/views/dashboard_test.ts
@@ -15,6 +15,12 @@
assert.equal(createDashboardUrl({}), '/dashboard/self');
});
+ test('baseUrl', () => {
+ window.CANONICAL_PATH = '/base';
+ assert.equal(createDashboardUrl({}).substring(0, 5), '/base');
+ window.CANONICAL_PATH = undefined;
+ });
+
test('user dashboard', () => {
assert.equal(createDashboardUrl({user: 'user'}), '/dashboard/user');
});
diff --git a/polygerrit-ui/app/models/views/diff.ts b/polygerrit-ui/app/models/views/diff.ts
index 63df521..3cc107a 100644
--- a/polygerrit-ui/app/models/views/diff.ts
+++ b/polygerrit-ui/app/models/views/diff.ts
@@ -12,7 +12,11 @@
} from '../../api/rest-api';
import {GerritView} from '../../services/router/router-model';
import {UrlEncodedCommentId} from '../../types/common';
-import {encodeURL, getPatchRangeExpression} from '../../utils/url-util';
+import {
+ encodeURL,
+ getBaseUrl,
+ getPatchRangeExpression,
+} from '../../utils/url-util';
import {define} from '../dependency';
import {Model} from '../model';
import {ViewState} from './base';
@@ -85,9 +89,9 @@
if (state.project) {
const encodedProject = encodeURL(state.project, true);
- return `/c/${encodedProject}/+/${state.changeNum}${suffix}`;
+ return `${getBaseUrl()}/c/${encodedProject}/+/${state.changeNum}${suffix}`;
} else {
- return `/c/${state.changeNum}${suffix}`;
+ return `${getBaseUrl()}/c/${state.changeNum}${suffix}`;
}
}
diff --git a/polygerrit-ui/app/models/views/diff_test.ts b/polygerrit-ui/app/models/views/diff_test.ts
index ec20037..b0f91bb 100644
--- a/polygerrit-ui/app/models/views/diff_test.ts
+++ b/polygerrit-ui/app/models/views/diff_test.ts
@@ -25,6 +25,10 @@
};
assert.equal(createDiffUrl(params), '/c/42/12/x%252By/path.cpp');
+ window.CANONICAL_PATH = '/base';
+ assert.equal(createDiffUrl(params).substring(0, 5), '/base');
+ window.CANONICAL_PATH = undefined;
+
params.project = 'test' as RepoName;
assert.equal(createDiffUrl(params), '/c/test/+/42/12/x%252By/path.cpp');
diff --git a/polygerrit-ui/app/models/views/edit.ts b/polygerrit-ui/app/models/views/edit.ts
index d8f4770..c63c8ce 100644
--- a/polygerrit-ui/app/models/views/edit.ts
+++ b/polygerrit-ui/app/models/views/edit.ts
@@ -10,7 +10,11 @@
RevisionPatchSetNum,
} from '../../api/rest-api';
import {GerritView} from '../../services/router/router-model';
-import {encodeURL, getPatchRangeExpression} from '../../utils/url-util';
+import {
+ encodeURL,
+ getBaseUrl,
+ getPatchRangeExpression,
+} from '../../utils/url-util';
import {define} from '../dependency';
import {Model} from '../model';
import {ViewState} from './base';
@@ -41,9 +45,9 @@
if (state.project) {
const encodedProject = encodeURL(state.project, true);
- return `/c/${encodedProject}/+/${state.changeNum}${suffix}`;
+ return `${getBaseUrl()}/c/${encodedProject}/+/${state.changeNum}${suffix}`;
} else {
- return `/c/${state.changeNum}${suffix}`;
+ return `${getBaseUrl()}/c/${state.changeNum}${suffix}`;
}
}
diff --git a/polygerrit-ui/app/models/views/edit_test.ts b/polygerrit-ui/app/models/views/edit_test.ts
index e1a05c7..2912063 100644
--- a/polygerrit-ui/app/models/views/edit_test.ts
+++ b/polygerrit-ui/app/models/views/edit_test.ts
@@ -27,5 +27,9 @@
createEditUrl(params),
'/c/test-project/+/42/12/x%252By/path.cpp,edit#31'
);
+
+ window.CANONICAL_PATH = '/base';
+ assert.equal(createEditUrl(params).substring(0, 5), '/base');
+ window.CANONICAL_PATH = undefined;
});
});
diff --git a/polygerrit-ui/app/models/views/search.ts b/polygerrit-ui/app/models/views/search.ts
index 58cb8f7..13de8f3 100644
--- a/polygerrit-ui/app/models/views/search.ts
+++ b/polygerrit-ui/app/models/views/search.ts
@@ -6,7 +6,7 @@
import {RepoName, BranchName, TopicName} from '../../api/rest-api';
import {GerritView} from '../../services/router/router-model';
import {addQuotesWhen} from '../../utils/string-util';
-import {encodeURL} from '../../utils/url-util';
+import {encodeURL, getBaseUrl} from '../../utils/url-util';
import {define} from '../dependency';
import {Model} from '../model';
import {ViewState} from './base';
@@ -35,7 +35,7 @@
}
if (params.query) {
- return '/q/' + encodeURL(params.query, true) + offsetExpr;
+ return `${getBaseUrl()}/q/${encodeURL(params.query, true)}${offsetExpr}`;
}
const operators: string[] = [];
@@ -80,7 +80,7 @@
}
}
- return '/q/' + operators.join('+') + offsetExpr;
+ return `${getBaseUrl()}/q/${operators.join('+')}${offsetExpr}`;
}
export const searchViewModelToken =
diff --git a/polygerrit-ui/app/models/views/search_test.ts b/polygerrit-ui/app/models/views/search_test.ts
index 138ce1e..d48667b 100644
--- a/polygerrit-ui/app/models/views/search_test.ts
+++ b/polygerrit-ui/app/models/views/search_test.ts
@@ -23,6 +23,10 @@
'topic:g%2525h+status:op%2525en'
);
+ window.CANONICAL_PATH = '/base';
+ assert.equal(createSearchUrl(options).substring(0, 5), '/base');
+ window.CANONICAL_PATH = undefined;
+
options.offset = 100;
assert.equal(
createSearchUrl(options),
diff --git a/polygerrit-ui/app/rollup.config.js b/polygerrit-ui/app/rollup.config.js
index 47492a3..be60a63 100644
--- a/polygerrit-ui/app/rollup.config.js
+++ b/polygerrit-ui/app/rollup.config.js
@@ -30,6 +30,7 @@
const resolve = requirePlugin('rollup-plugin-node-resolve');
const {terser} = requirePlugin('rollup-plugin-terser');
+const define = requirePlugin('rollup-plugin-define');
// @polymer/font-roboto-local uses import.meta.url value
// as a base path to fonts. We should substitute a correct javascript
@@ -75,5 +76,11 @@
extensions: ['.js'],
moduleDirectory: 'external/ui_npm/node_modules',
},
- }), importLocalFontMetaUrlResolver()],
+ }),
+ define({
+ replacements: {
+ 'process.env.NODE_ENV': JSON.stringify('production'),
+ },
+ }),
+ importLocalFontMetaUrlResolver()],
};
diff --git a/polygerrit-ui/app/rules.bzl b/polygerrit-ui/app/rules.bzl
index 1d5e577..9ab0f64 100644
--- a/polygerrit-ui/app/rules.bzl
+++ b/polygerrit-ui/app/rules.bzl
@@ -29,6 +29,7 @@
silent = True,
sourcemap = "hidden",
deps = [
+ "@tools_npm//rollup-plugin-define",
"@tools_npm//rollup-plugin-node-resolve",
],
)
@@ -42,6 +43,7 @@
silent = True,
sourcemap = "hidden",
deps = [
+ "@tools_npm//rollup-plugin-define",
"@tools_npm//rollup-plugin-node-resolve",
],
)
@@ -55,6 +57,7 @@
silent = True,
sourcemap = "hidden",
deps = [
+ "@tools_npm//rollup-plugin-define",
"@tools_npm//rollup-plugin-node-resolve",
],
)
diff --git a/polygerrit-ui/app/services/router/router-model.ts b/polygerrit-ui/app/services/router/router-model.ts
index 99b9870..f8bc778 100644
--- a/polygerrit-ui/app/services/router/router-model.ts
+++ b/polygerrit-ui/app/services/router/router-model.ts
@@ -4,7 +4,6 @@
* SPDX-License-Identifier: Apache-2.0
*/
import {Observable} from 'rxjs';
-import {distinctUntilChanged, map} from 'rxjs/operators';
import {Finalizable} from '../registry';
import {
NumericChangeId,
@@ -12,6 +11,7 @@
BasePatchSetNum,
} from '../../types/common';
import {Model} from '../../models/model';
+import {select} from '../../utils/observable-util';
export enum GerritView {
ADMIN = 'admin',
@@ -37,31 +37,23 @@
}
export class RouterModel extends Model<RouterState> implements Finalizable {
- readonly routerView$: Observable<GerritView | undefined>;
+ readonly routerView$: Observable<GerritView | undefined> = select(
+ this.state$,
+ state => state.view
+ );
- readonly routerChangeNum$: Observable<NumericChangeId | undefined>;
+ readonly routerChangeNum$: Observable<NumericChangeId | undefined> = select(
+ this.state$,
+ state => state.changeNum
+ );
- readonly routerPatchNum$: Observable<RevisionPatchSetNum | undefined>;
+ readonly routerPatchNum$: Observable<RevisionPatchSetNum | undefined> =
+ select(this.state$, state => state.patchNum);
- readonly routerBasePatchNum$: Observable<BasePatchSetNum | undefined>;
+ readonly routerBasePatchNum$: Observable<BasePatchSetNum | undefined> =
+ select(this.state$, state => state.basePatchNum);
constructor() {
super({});
- this.routerView$ = this.state$.pipe(
- map(state => state.view),
- distinctUntilChanged()
- );
- this.routerChangeNum$ = this.state$.pipe(
- map(state => state.changeNum),
- distinctUntilChanged()
- );
- this.routerPatchNum$ = this.state$.pipe(
- map(state => state.patchNum),
- distinctUntilChanged()
- );
- this.routerBasePatchNum$ = this.state$.pipe(
- map(state => state.basePatchNum),
- distinctUntilChanged()
- );
}
}
diff --git a/polygerrit-ui/app/services/service-worker-installer.ts b/polygerrit-ui/app/services/service-worker-installer.ts
index 0eec23a..53cd325 100644
--- a/polygerrit-ui/app/services/service-worker-installer.ts
+++ b/polygerrit-ui/app/services/service-worker-installer.ts
@@ -16,6 +16,7 @@
/** Type of incoming messages for ServiceWorker. */
export enum ServiceWorkerMessageType {
TRIGGER_NOTIFICATIONS = 'TRIGGER_NOTIFICATIONS',
+ USER_PREFERENCE_CHANGE = 'USER_PREFERENCE_CHANGE',
}
export const TRIGGER_NOTIFICATION_UPDATES_MS = 5 * 60 * 1000;
@@ -25,33 +26,48 @@
account?: AccountDetailInfo;
+ allowBrowserNotificationsPreference?: boolean;
+
constructor(
private readonly flagsService: FlagsService,
private readonly userModel: UserModel
) {
+ if (!this.flagsService.isEnabled(KnownExperimentId.PUSH_NOTIFICATIONS)) {
+ return;
+ }
this.userModel.account$.subscribe(acc => (this.account = acc));
+ this.userModel.preferences$.subscribe(prefs => {
+ if (
+ this.allowBrowserNotificationsPreference !==
+ prefs.allow_browser_notifications
+ ) {
+ this.allowBrowserNotificationsPreference =
+ prefs.allow_browser_notifications;
+ navigator.serviceWorker.controller?.postMessage({
+ type: ServiceWorkerMessageType.USER_PREFERENCE_CHANGE,
+ allowBrowserNotificationsPreference:
+ this.allowBrowserNotificationsPreference,
+ });
+ }
+ });
+ Promise.all([
+ until(this.userModel.account$, account => !!account),
+ until(
+ this.userModel.preferences$,
+ prefs => !!prefs.allow_browser_notifications
+ ),
+ ]).then(() => {
+ this.init();
+ });
}
- async init() {
+ private async init() {
if (this.initialized) return;
- if (
- !this.flagsService.isEnabled(
- KnownExperimentId.PUSH_NOTIFICATIONS_DEVELOPER
- )
- ) {
- if (!this.flagsService.isEnabled(KnownExperimentId.PUSH_NOTIFICATIONS)) {
- return;
- }
- const timeout1s = new Promise(resolve => {
- setTimeout(resolve, 1000);
- });
- // We wait for account to be defined, if its not defined in 1s, it's guest
- await Promise.race([
- timeout1s,
- until(this.userModel.account$, account => !!account),
- ]);
- if (!areNotificationsEnabled(this.account)) return;
+ if (!this.flagsService.isEnabled(KnownExperimentId.PUSH_NOTIFICATIONS)) {
+ return;
}
+ if (!this.areNotificationsEnabled()) return;
+
if (!('serviceWorker' in navigator)) {
console.error('Service worker API not available');
return;
@@ -62,6 +78,21 @@
this.initialized = true;
}
+ areNotificationsEnabled() {
+ // Push Notification developer can have notification enabled even if they
+ // are disabled for this.account.
+ if (
+ !this.flagsService.isEnabled(
+ KnownExperimentId.PUSH_NOTIFICATIONS_DEVELOPER
+ ) &&
+ !areNotificationsEnabled(this.account)
+ ) {
+ return false;
+ }
+
+ return this.allowBrowserNotificationsPreference;
+ }
+
/**
* Every 5 minutes, we trigger service-worker to get
* latest updates in attention set and service-worker will create
diff --git a/polygerrit-ui/app/services/service-worker-installer_test.ts b/polygerrit-ui/app/services/service-worker-installer_test.ts
index 77dd907d..e8fd233 100644
--- a/polygerrit-ui/app/services/service-worker-installer_test.ts
+++ b/polygerrit-ui/app/services/service-worker-installer_test.ts
@@ -7,23 +7,29 @@
import '../test/common-test-setup';
import {ServiceWorkerInstaller} from './service-worker-installer';
import {assert} from '@open-wc/testing';
+import {createDefaultPreferences} from '../constants/constants';
+import {waitUntilObserved} from '../test/test-utils';
-suite('service woerker installler tests', () => {
- let serviceWorkerInstaller: ServiceWorkerInstaller;
-
- setup(() => {
+suite('service worker installer tests', () => {
+ test('init', async () => {
+ const registerStub = sinon.stub(window.navigator.serviceWorker, 'register');
const flagsService = getAppContext().flagsService;
const userModel = getAppContext().userModel;
sinon.stub(flagsService, 'isEnabled').returns(true);
- serviceWorkerInstaller = new ServiceWorkerInstaller(
- flagsService,
- userModel
+ new ServiceWorkerInstaller(flagsService, userModel);
+ const prefs = {
+ ...createDefaultPreferences(),
+ allow_browser_notifications: true,
+ };
+ userModel.setPreferences(prefs);
+ await waitUntilObserved(
+ userModel.preferences$,
+ pref => pref.allow_browser_notifications === true
);
- });
-
- test('init', () => {
- const registerStub = sinon.stub(window.navigator.serviceWorker, 'register');
- serviceWorkerInstaller.init();
+ await waitUntilObserved(
+ userModel.preferences$,
+ pref => pref.allow_browser_notifications === true
+ );
assert.isTrue(registerStub.called);
});
});
diff --git a/polygerrit-ui/app/styles/themes/app-theme.ts b/polygerrit-ui/app/styles/themes/app-theme.ts
index 117e7cc..e148da9 100644
--- a/polygerrit-ui/app/styles/themes/app-theme.ts
+++ b/polygerrit-ui/app/styles/themes/app-theme.ts
@@ -99,6 +99,8 @@
--purple-100: #e9d2fd;
--purple-50: #f3e8fd;
--purple-tonal: #523272;
+ --deep-purple-800: #4527a0;
+ --deep-purple-600: #5e35b1;
--pink-800: #b80672;
--pink-500: #f538a0;
--pink-50: #fde7f3;
diff --git a/polygerrit-ui/app/styles/themes/dark-theme.ts b/polygerrit-ui/app/styles/themes/dark-theme.ts
index d11e372..2330041 100644
--- a/polygerrit-ui/app/styles/themes/dark-theme.ts
+++ b/polygerrit-ui/app/styles/themes/dark-theme.ts
@@ -219,8 +219,8 @@
--dark-remove-highlight-color: #62110f;
--light-remove-highlight-color: #320404;
- --dark-rebased-add-highlight-color: rgba(11, 255, 155, 0.15);
- --light-rebased-add-highlight-color: #487165;
+ --dark-rebased-add-highlight-color: var(--deep-purple-800);
+ --light-rebased-add-highlight-color: var(--deep-purple-600);
--dark-rebased-remove-highlight-color: rgba(255, 139, 6, 0.15);
--light-rebased-remove-highlight-color: #2f3f2f;
diff --git a/polygerrit-ui/app/test/test-data-generators.ts b/polygerrit-ui/app/test/test-data-generators.ts
index 9337500..4e29f53 100644
--- a/polygerrit-ui/app/test/test-data-generators.ts
+++ b/polygerrit-ui/app/test/test-data-generators.ts
@@ -686,6 +686,7 @@
change_table: [],
email_strategy: EmailStrategy.ENABLED,
default_base_for_merges: DefaultBase.AUTO_MERGE,
+ allow_browser_notifications: true,
};
}
diff --git a/polygerrit-ui/app/types/common.ts b/polygerrit-ui/app/types/common.ts
index 523dadb..008a8de 100644
--- a/polygerrit-ui/app/types/common.ts
+++ b/polygerrit-ui/app/types/common.ts
@@ -1122,6 +1122,7 @@
work_in_progress_by_default?: boolean;
// The email_format doesn't mentioned in doc, but exists in Java class GeneralPreferencesInfo
email_format?: EmailFormat;
+ allow_browser_notifications?: boolean;
}
/**
diff --git a/polygerrit-ui/app/types/events.ts b/polygerrit-ui/app/types/events.ts
index 597bb6f..496513d 100644
--- a/polygerrit-ui/app/types/events.ts
+++ b/polygerrit-ui/app/types/events.ts
@@ -209,9 +209,7 @@
// Type for the custom event to switch tab.
export interface SwitchTabEventDetail {
// name of the tab to set as active, from custom event
- tab?: string;
- // index of tab to set as active, from paper-tabs event
- value?: number;
+ tab: string;
// scroll into the tab afterwards, from custom event
scrollIntoView?: boolean;
// define state of tab after opening
diff --git a/polygerrit-ui/app/utils/change-util.ts b/polygerrit-ui/app/utils/change-util.ts
index 07b63d6..9062ac7 100644
--- a/polygerrit-ui/app/utils/change-util.ts
+++ b/polygerrit-ui/app/utils/change-util.ts
@@ -282,7 +282,9 @@
change?: ChangeInfo,
reviewer?: AccountInfo
): boolean {
- if (!change?.removable_reviewers || !reviewer) return false;
+ if (!reviewer || !change) return false;
+ if (isCc(change, reviewer)) return true;
+ if (!change.removable_reviewers) return false;
return change.removable_reviewers.some(
account =>
account._account_id === reviewer._account_id ||
diff --git a/polygerrit-ui/app/utils/common-util.ts b/polygerrit-ui/app/utils/common-util.ts
index 05c054b..183d167 100644
--- a/polygerrit-ui/app/utils/common-util.ts
+++ b/polygerrit-ui/app/utils/common-util.ts
@@ -117,7 +117,7 @@
/**
* Add value, if the set does not contain it. Otherwise remove it.
*/
-export function toggleSetMembership<T>(set: Set<T>, value: T): void {
+export function toggleSet<T>(set: Set<T>, value: T): void {
if (set.has(value)) {
set.delete(value);
} else {
@@ -125,6 +125,14 @@
}
}
+export function toggle<T>(array: T[], item: T): T[] {
+ if (array.includes(item)) {
+ return array.filter(r => r !== item);
+ } else {
+ return array.concat([item]);
+ }
+}
+
export function unique<T>(item: T, index: number, array: T[]) {
return array.indexOf(item) === index;
}
diff --git a/polygerrit-ui/app/utils/common-util_test.ts b/polygerrit-ui/app/utils/common-util_test.ts
index 0d85f34..76c8a6c 100644
--- a/polygerrit-ui/app/utils/common-util_test.ts
+++ b/polygerrit-ui/app/utils/common-util_test.ts
@@ -11,6 +11,7 @@
containsAll,
intersection,
difference,
+ toggle,
} from './common-util';
suite('common-util tests', () => {
@@ -97,4 +98,11 @@
assert.deepEqual(difference([1, 2, 3], [1, 2, 3]), []);
assert.deepEqual(difference([1, 2, 3], [4, 5, 6]), [1, 2, 3]);
});
+
+ test('toggle', () => {
+ assert.deepEqual(toggle([], 1), [1]);
+ assert.deepEqual(toggle([1], 1), []);
+ assert.deepEqual(toggle([1, 2, 3], 1), [2, 3]);
+ assert.deepEqual(toggle([2, 3], 1), [2, 3, 1]);
+ });
});
diff --git a/polygerrit-ui/app/utils/link-util.ts b/polygerrit-ui/app/utils/link-util.ts
index 9079f4c..b5b9025 100644
--- a/polygerrit-ui/app/utils/link-util.ts
+++ b/polygerrit-ui/app/utils/link-util.ts
@@ -4,76 +4,164 @@
* SPDX-License-Identifier: Apache-2.0
*/
import 'ba-linkify/ba-linkify';
-import {CommentLinks} from '../types/common';
+import {CommentLinkInfo, CommentLinks} from '../types/common';
import {getBaseUrl} from './url-util';
-export function linkifyNormalUrls(base: string): string {
- // Some tools are known to look for reviewers/CCs by finding lines such as
- // "R=foo@gmail.com, bar@gmail.com". However, "=" is technically a valid email
- // character, so ba-linkify interprets the entire string "R=foo@gmail.com" as
- // an email address. To fix this, we insert a zero width space character
- // \u200B before linking that prevents ba-linkify from associating the prefix
- // with the email. After linking we remove the zero width space.
- const baseWithZeroWidthSpace = base.replace(/^(R=|CC=)/g, '$&\u200B');
+/**
+ * Finds links within the base string and convert them to HTML. Config-based
+ * rewrites are only applied on text that is not linked by the default linking
+ * library.
+ */
+export function linkifyUrlsAndApplyRewrite(
+ base: string,
+ repoCommentLinks: CommentLinks
+): string {
const parts: string[] = [];
- window.linkify(baseWithZeroWidthSpace, {
+ window.linkify(insertZeroWidthSpace(base), {
callback: (text, href) => {
- const result = href ? createLinkTemplate(href, text) : text;
- const resultWithoutZeroWidthSpace = result.replace(/\u200B/g, '');
- parts.push(resultWithoutZeroWidthSpace);
+ if (href) {
+ parts.push(removeZeroWidthSpace(createLinkTemplate(href, text)));
+ } else {
+ const rewriteResults = getRewriteResultsFromConfig(
+ text,
+ repoCommentLinks
+ );
+ parts.push(removeZeroWidthSpace(applyRewrites(text, rewriteResults)));
+ }
},
});
return parts.join('');
}
-export function applyLinkRewritesFromConfig(
+/**
+ * Generates a list of rewrites that would be applied to a base string. They are
+ * not applied immediately to the base text because one rewrite may interfere or
+ * overlap with a later rewrite. Only after all rewrites are known they are
+ * carefully merged with `applyRewrites`.
+ */
+function getRewriteResultsFromConfig(
base: string,
repoCommentLinks: CommentLinks
-) {
- const linkRewritesFromConfig = Object.values(repoCommentLinks).filter(
- commentLinkInfo => commentLinkInfo.enabled !== false && commentLinkInfo.link
+): RewriteResult[] {
+ const enabledRewrites = Object.values(repoCommentLinks).filter(
+ commentLinkInfo =>
+ commentLinkInfo.enabled !== false &&
+ (commentLinkInfo.link !== undefined || commentLinkInfo.html !== undefined)
);
- const rewrites = linkRewritesFromConfig.map(rewrite => {
- const replacementHref = rewrite.link!.startsWith('/')
- ? `${getBaseUrl()}${rewrite.link!}`
- : rewrite.link!;
- return {
- match: new RegExp(rewrite.match, 'g'),
- replace: createLinkTemplate(
+ return enabledRewrites.flatMap(rewrite => {
+ const regexp = new RegExp(rewrite.match, 'g');
+ const partialResults: RewriteResult[] = [];
+ let match: RegExpExecArray | null;
+
+ while ((match = regexp.exec(base)) !== null) {
+ const fullReplacementText = getReplacementText(match[0], rewrite);
+ // The replacement may not be changing the entire matched substring so we
+ // "trim" the replacement position and text to the part that is actually
+ // different. This makes sure that unchanged portions are still eligible
+ // for other rewrites without being rejected as overlaps during
+ // `applyRewrites`. The new `replacementText` is not eligible for other
+ // rewrites since it would introduce unexpected interactions between
+ // rewrites depending on their order of definition/execution.
+ const sharedPrefixLength = getSharedPrefixLength(
+ match[0],
+ fullReplacementText
+ );
+ const sharedSuffixLength = getSharedSuffixLength(
+ match[0],
+ fullReplacementText
+ );
+ const prefixIndex = sharedPrefixLength;
+ const matchSuffixIndex = match[0].length - sharedSuffixLength;
+ const fullReplacementSuffixIndex =
+ fullReplacementText.length - sharedSuffixLength;
+ partialResults.push({
+ replacedTextStartPosition: match.index + prefixIndex,
+ replacedTextEndPosition: match.index + matchSuffixIndex,
+ replacementText: fullReplacementText.substring(
+ prefixIndex,
+ fullReplacementSuffixIndex
+ ),
+ });
+ }
+ return partialResults;
+ });
+}
+
+/**
+ * Applies all the rewrites to the given base string. To resolve cases where
+ * multiple rewrites target overlapping pieces of the base string, the rewrite
+ * that ends latest is kept and the rest are not applied and discarded.
+ */
+function applyRewrites(base: string, rewriteResults: RewriteResult[]): string {
+ const rewritesByEndPosition = [...rewriteResults].sort((a, b) => {
+ if (b.replacedTextEndPosition !== a.replacedTextEndPosition) {
+ return b.replacedTextEndPosition - a.replacedTextEndPosition;
+ }
+ return a.replacedTextStartPosition - b.replacedTextStartPosition;
+ });
+ const filteredSortedRewrites: RewriteResult[] = [];
+ let latestReplace = base.length;
+ for (const rewrite of rewritesByEndPosition) {
+ // Only accept rewrites that do not overlap with any previously accepted
+ // rewrites.
+ if (rewrite.replacedTextEndPosition <= latestReplace) {
+ filteredSortedRewrites.push(rewrite);
+ latestReplace = rewrite.replacedTextStartPosition;
+ }
+ }
+ return filteredSortedRewrites.reduce(
+ (text, rewrite) =>
+ text
+ .substring(0, rewrite.replacedTextStartPosition)
+ .concat(rewrite.replacementText)
+ .concat(text.substring(rewrite.replacedTextEndPosition)),
+ base
+ );
+}
+
+/**
+ * For a given regexp match, apply the rewrite based on the rewrite's type and
+ * return the resulting string.
+ */
+function getReplacementText(
+ matchedText: string,
+ rewrite: CommentLinkInfo
+): string {
+ if (rewrite.link !== undefined) {
+ const replacementHref = rewrite.link.startsWith('/')
+ ? `${getBaseUrl()}${rewrite.link}`
+ : rewrite.link;
+ const regexp = new RegExp(rewrite.match, 'g');
+ return matchedText.replace(
+ regexp,
+ createLinkTemplate(
replacementHref,
rewrite.text ?? '$&',
rewrite.prefix,
rewrite.suffix
- ),
- };
- });
- return applyRewrites(base, rewrites);
+ )
+ );
+ } else if (rewrite.html !== undefined) {
+ return matchedText.replace(new RegExp(rewrite.match, 'g'), rewrite.html);
+ } else {
+ throw new Error('commentLinkInfo is not a link or html rewrite');
+ }
}
-export function applyHtmlRewritesFromConfig(
- base: string,
- repoCommentLinks: CommentLinks
-) {
- const htmlRewritesFromConfig = Object.values(repoCommentLinks).filter(
- commentLinkInfo => commentLinkInfo.enabled !== false && commentLinkInfo.html
- );
- const rewrites = htmlRewritesFromConfig.map(rewrite => {
- return {
- match: new RegExp(rewrite.match, 'g'),
- replace: rewrite.html!,
- };
- });
- return applyRewrites(base, rewrites);
+/**
+ * Some tools are known to look for reviewers/CCs by finding lines such as
+ * "R=foo@gmail.com, bar@gmail.com". However, "=" is technically a valid email
+ * character, so ba-linkify interprets the entire string "R=foo@gmail.com" as an
+ * email address. To fix this, we insert a zero width space character \u200B
+ * before linking that prevents ba-linkify from associating the prefix with the
+ * email. After linking we remove the zero width space.
+ */
+function insertZeroWidthSpace(base: string) {
+ return base.replace(/^(R=|CC=)/g, '$&\u200B');
}
-function applyRewrites(
- base: string,
- rewrites: {match: RegExp | string; replace: string}[]
-) {
- return rewrites.reduce(
- (text, rewrite) => text.replace(rewrite.match, rewrite.replace),
- base
- );
+function removeZeroWidthSpace(base: string) {
+ return base.replace(/\u200B/g, '');
}
function createLinkTemplate(
@@ -88,3 +176,41 @@
suffix ?? ''
}`;
}
+
+/**
+ * Returns the number of characters that are identical at the start of both
+ * strings.
+ *
+ * For example, `getSharedPrefixLength('12345678', '1234zz78')` would return 4
+ */
+function getSharedPrefixLength(a: string, b: string) {
+ let i = 0;
+ for (; i < a.length && i < b.length; ++i) {
+ if (a[i] !== b[i]) {
+ return i;
+ }
+ }
+ return i;
+}
+
+/**
+ * Returns the number of characters that are identical at the end of both
+ * strings.
+ *
+ * For example, `getSharedSuffixLength('12345678', '1234zz78')` would return 2
+ */
+function getSharedSuffixLength(a: string, b: string) {
+ let i = a.length;
+ for (let j = b.length; i !== 0 && j !== 0; --i, --j) {
+ if (a[i] !== b[j]) {
+ return a.length - 1 - i;
+ }
+ }
+ return a.length - i;
+}
+
+interface RewriteResult {
+ replacedTextStartPosition: number;
+ replacedTextEndPosition: number;
+ replacementText: string;
+}
diff --git a/polygerrit-ui/app/utils/link-util_test.ts b/polygerrit-ui/app/utils/link-util_test.ts
index a1ec2fa..61d6bff 100644
--- a/polygerrit-ui/app/utils/link-util_test.ts
+++ b/polygerrit-ui/app/utils/link-util_test.ts
@@ -3,11 +3,7 @@
* Copyright 2022 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
-import {
- applyHtmlRewritesFromConfig,
- applyLinkRewritesFromConfig,
- linkifyNormalUrls,
-} from './link-util';
+import {linkifyUrlsAndApplyRewrite} from './link-util';
import {assert} from '@open-wc/testing';
suite('link-util tests', () => {
@@ -15,55 +11,220 @@
return `<a href="${href}" rel="noopener" target="_blank">${text}</a>`;
}
- test('applyHtmlRewritesFromConfig', () => {
- assert.equal(
- applyHtmlRewritesFromConfig('#12345 foo', {
- 'number-emphasizer': {
- match: '#(\\d+)',
- html: '<h1>Change $1 is the best change</h1>',
- },
- 'foo-capitalizer': {
- match: 'foo',
- html: '<div>FOO</div>',
- },
- }),
- '<h1>Change 12345 is the best change</h1> <div>FOO</div>'
- );
+ suite('link rewrites', () => {
+ test('without text', () => {
+ assert.equal(
+ linkifyUrlsAndApplyRewrite('foo', {
+ fooLinkWithoutText: {
+ match: 'foo',
+ link: 'foo.gov',
+ },
+ }),
+ link('foo', 'foo.gov')
+ );
+ });
+
+ test('with text', () => {
+ assert.equal(
+ linkifyUrlsAndApplyRewrite('foo', {
+ fooLinkWithText: {
+ match: 'foo',
+ link: 'foo.gov',
+ text: 'foo site',
+ },
+ }),
+ link('foo site', 'foo.gov')
+ );
+ });
+
+ test('with prefix and suffix', () => {
+ assert.equal(
+ linkifyUrlsAndApplyRewrite('there are 12 foos here', {
+ fooLinkWithText: {
+ match: '(.*)(bug|foo)s(.*)',
+ link: '$2.gov',
+ text: '$2 list',
+ prefix: '$1on the ',
+ suffix: '$3',
+ },
+ }),
+ `there are 12 on the ${link('foo list', 'foo.gov')} here`
+ );
+ });
+
+ test('multiple matches', () => {
+ assert.equal(
+ linkifyUrlsAndApplyRewrite('foo foo', {
+ foo: {
+ match: 'foo',
+ link: 'foo.gov',
+ },
+ }),
+ `${link('foo', 'foo.gov')} ${link('foo', 'foo.gov')}`
+ );
+ });
+
+ test('does not apply within normal links', () => {
+ assert.equal(
+ linkifyUrlsAndApplyRewrite('google.com', {
+ ogle: {
+ match: 'ogle',
+ link: 'gerritcodereview.com',
+ },
+ }),
+ link('google.com', 'http://google.com')
+ );
+ });
+ });
+ suite('html rewrites', () => {
+ test('basic case', () => {
+ assert.equal(
+ linkifyUrlsAndApplyRewrite('foo', {
+ foo: {
+ match: '(foo)',
+ html: '<div>$1</div>',
+ },
+ }),
+ '<div>foo</div>'
+ );
+ });
+
+ test('only inserts', () => {
+ assert.equal(
+ linkifyUrlsAndApplyRewrite('foo', {
+ foo: {
+ match: 'foo',
+ html: 'foo bar',
+ },
+ }),
+ 'foo bar'
+ );
+ });
+
+ test('only deletes', () => {
+ assert.equal(
+ linkifyUrlsAndApplyRewrite('foo bar baz', {
+ bar: {
+ match: 'bar',
+ html: '',
+ },
+ }),
+ 'foo baz'
+ );
+ });
+
+ test('multiple matches', () => {
+ assert.equal(
+ linkifyUrlsAndApplyRewrite('foo foo', {
+ foo: {
+ match: '(foo)',
+ html: '<div>$1</div>',
+ },
+ }),
+ '<div>foo</div> <div>foo</div>'
+ );
+ });
+
+ test('does not apply within normal links', () => {
+ assert.equal(
+ linkifyUrlsAndApplyRewrite('google.com', {
+ ogle: {
+ match: 'ogle',
+ html: '<div>gerritcodereview.com<div>',
+ },
+ }),
+ link('google.com', 'http://google.com')
+ );
+ });
});
- test('applyLinkRewritesFromConfig', () => {
- const linkedNumber = link('#12345', 'google.com/12345');
- const linkedFoo = link('foo', 'foo.gov');
- const linkedBar = link('Bar Page: 300', 'bar.com/page?id=300');
+ test('for overlapping rewrites prefer the latest ending', () => {
assert.equal(
- applyLinkRewritesFromConfig('#12345 foo crowbar:12 bar:300', {
- 'number-linker': {
- match: '#(\\d+)',
- link: 'google.com/$1',
- },
- 'foo-linker': {
+ linkifyUrlsAndApplyRewrite('foobarbaz', {
+ foo: {
match: 'foo',
link: 'foo.gov',
},
- 'advanced-link': {
- match: '(^|\\s)bar:(\\d+)($|\\s)',
- link: 'bar.com/page?id=$2',
- text: 'Bar Page: $2',
- prefix: '$1',
- suffix: '$3',
+ foobarbaz: {
+ match: 'foobarbaz',
+ html: '<div>foobarbaz.gov</div>',
+ },
+ foobar: {
+ match: 'foobar',
+ link: 'foobar.gov',
},
}),
- `${linkedNumber} ${linkedFoo} crowbar:12 ${linkedBar}`
+ '<div>foobarbaz.gov</div>'
);
});
- suite('linkifyNormalUrls', () => {
+ test('overlapping rewrites with same ending prefers earliest start', () => {
+ assert.equal(
+ linkifyUrlsAndApplyRewrite('foobarbaz', {
+ foo: {
+ match: 'baz',
+ link: 'Baz.gov',
+ },
+ foobarbaz: {
+ match: 'foobarbaz',
+ html: '<div>FooBarBaz.gov</div>',
+ },
+ foobar: {
+ match: 'barbaz',
+ link: 'BarBaz.gov',
+ },
+ }),
+ '<div>FooBarBaz.gov</div>'
+ );
+ });
+
+ test('removed overlapping rewrites do not prevent other rewrites', () => {
+ assert.equal(
+ linkifyUrlsAndApplyRewrite('foobarbaz', {
+ foo: {
+ match: 'foo',
+ html: 'FOO',
+ },
+ oobarba: {
+ match: 'oobarba',
+ html: 'OOBARBA',
+ },
+ baz: {
+ match: 'baz',
+ html: 'BAZ',
+ },
+ }),
+ 'FOObarBAZ'
+ );
+ });
+
+ test('rewrites do not interfere with each other matching', () => {
+ assert.equal(
+ linkifyUrlsAndApplyRewrite('bugs: 123 234 345', {
+ bug1: {
+ match: '(bugs:) (\\d+)',
+ html: '$1 <div>bug/$2</div>',
+ },
+ bug2: {
+ match: '(bugs:) (\\d+) (\\d+)',
+ html: '$1 $2 <div>bug/$3</div>',
+ },
+ bug3: {
+ match: '(bugs:) (\\d+) (\\d+) (\\d+)',
+ html: '$1 $2 $3 <div>bug/$4</div>',
+ },
+ }),
+ 'bugs: <div>bug/123</div> <div>bug/234</div> <div>bug/345</div>'
+ );
+ });
+
+ suite('normal links', () => {
test('links urls', () => {
const googleLink = link('google.com', 'http://google.com');
const mapsLink = link('maps.google.com', 'http://maps.google.com');
assert.equal(
- linkifyNormalUrls('google.com, maps.google.com'),
+ linkifyUrlsAndApplyRewrite('google.com, maps.google.com', {}),
`${googleLink}, ${mapsLink}`
);
});
@@ -72,7 +233,7 @@
const fooEmail = link('foo@gmail.com', 'mailto:foo@gmail.com');
const barEmail = link('bar@gmail.com', 'mailto:bar@gmail.com');
assert.equal(
- linkifyNormalUrls('R=foo@gmail.com, bar@gmail.com'),
+ linkifyUrlsAndApplyRewrite('R=foo@gmail.com, bar@gmail.com', {}),
`R=${fooEmail}, ${barEmail}`
);
});
@@ -81,7 +242,7 @@
const fooEmail = link('foo@gmail.com', 'mailto:foo@gmail.com');
const barEmail = link('bar@gmail.com', 'mailto:bar@gmail.com');
assert.equal(
- linkifyNormalUrls('CC=foo@gmail.com, bar@gmail.com'),
+ linkifyUrlsAndApplyRewrite('CC=foo@gmail.com, bar@gmail.com', {}),
`CC=${fooEmail}, ${barEmail}`
);
});
@@ -92,7 +253,7 @@
'mailto:fooR=barCC=baz@gmail.com'
);
assert.equal(
- linkifyNormalUrls('fooR=barCC=baz@gmail.com'),
+ linkifyUrlsAndApplyRewrite('fooR=barCC=baz@gmail.com', {}),
fooBarBazEmail
);
});
diff --git a/polygerrit-ui/app/utils/string-util_test.ts b/polygerrit-ui/app/utils/string-util_test.ts
index 0a7ee27..c6c65b1 100644
--- a/polygerrit-ui/app/utils/string-util_test.ts
+++ b/polygerrit-ui/app/utils/string-util_test.ts
@@ -12,7 +12,7 @@
diffFilePaths,
} from './string-util';
-suite('formatter util tests', () => {
+suite('string-util tests', () => {
test('pluralize', () => {
const noun = 'comment';
assert.equal(pluralize(0, noun), '');
diff --git a/polygerrit-ui/app/workers/service-worker-class.ts b/polygerrit-ui/app/workers/service-worker-class.ts
index c257d8d..218744d 100644
--- a/polygerrit-ui/app/workers/service-worker-class.ts
+++ b/polygerrit-ui/app/workers/service-worker-class.ts
@@ -27,6 +27,8 @@
// private but used in test
latestUpdateTimestampMs = Date.now();
+ allowBrowserNotificationsPreference = false;
+
/**
* We cannot rely on a state in a service worker, because every time
* service worker starts or stops, new instance is created. So every time
@@ -44,6 +46,8 @@
saveState() {
return putServiceWorkerState({
latestUpdateTimestampMs: this.latestUpdateTimestampMs,
+ allowBrowserNotificationsPreference:
+ this.allowBrowserNotificationsPreference,
});
}
@@ -51,19 +55,27 @@
const state = await getServiceWorkerState();
if (state) {
this.latestUpdateTimestampMs = state.latestUpdateTimestampMs;
+ this.allowBrowserNotificationsPreference =
+ state.allowBrowserNotificationsPreference;
}
}
private onMessage(e: ExtendableMessageEvent) {
- if (e.data?.type !== ServiceWorkerMessageType.TRIGGER_NOTIFICATIONS) {
- // Only this notification message type exists, but we do not throw error.
- return;
+ if (e.data?.type === ServiceWorkerMessageType.TRIGGER_NOTIFICATIONS) {
+ e.waitUntil(
+ this.showLatestAttentionChangeNotification(
+ e.data?.account as AccountDetailInfo | undefined
+ )
+ );
+ } else if (
+ e.data?.type === ServiceWorkerMessageType.USER_PREFERENCE_CHANGE
+ ) {
+ e.waitUntil(
+ this.allowBrowserNotificationsPreferenceChanged(
+ e.data?.allowBrowserNotificationsPreference as boolean
+ )
+ );
}
- e.waitUntil(
- this.showLatestAttentionChangeNotification(
- e.data?.account as AccountDetailInfo | undefined
- )
- );
}
private onNotificationClick(e: NotificationEvent) {
@@ -71,10 +83,16 @@
e.waitUntil(this.openWindow(e.notification.data.url));
}
+ async allowBrowserNotificationsPreferenceChanged(preference: boolean) {
+ this.allowBrowserNotificationsPreference = preference;
+ await this.saveState();
+ }
+
// private but used in test
async showLatestAttentionChangeNotification(account?: AccountDetailInfo) {
// Message always contains account, but we do not throw error.
if (!account?._account_id) return;
+ if (!this.allowBrowserNotificationsPreference) return;
const latestAttentionChanges = await this.getChangesToNotify(account);
const numOfChangesToNotifyAbout = latestAttentionChanges.length;
if (numOfChangesToNotifyAbout === 1) {
diff --git a/polygerrit-ui/app/workers/service-worker-class_test.ts b/polygerrit-ui/app/workers/service-worker-class_test.ts
index 8b508e8..4cbd7bb 100644
--- a/polygerrit-ui/app/workers/service-worker-class_test.ts
+++ b/polygerrit-ui/app/workers/service-worker-class_test.ts
@@ -25,6 +25,7 @@
},
} as {} as ServiceWorkerGlobalScope;
serviceWorker = new ServiceWorker(moctCtx);
+ serviceWorker.allowBrowserNotificationsPreference = true;
});
test('notify about attention in t2 when time is t3', async () => {
diff --git a/polygerrit-ui/app/workers/service-worker-indexdb.ts b/polygerrit-ui/app/workers/service-worker-indexdb.ts
index f540d59..6d5ab40 100644
--- a/polygerrit-ui/app/workers/service-worker-indexdb.ts
+++ b/polygerrit-ui/app/workers/service-worker-indexdb.ts
@@ -6,6 +6,7 @@
export interface GerritServiceWorkerState {
latestUpdateTimestampMs: number;
+ allowBrowserNotificationsPreference: boolean;
}
const SERVICE_WORKER_DB = 'service-worker-db-1';
diff --git a/tools/node_tools/package.json b/tools/node_tools/package.json
index fc00451..3765575 100644
--- a/tools/node_tools/package.json
+++ b/tools/node_tools/package.json
@@ -15,6 +15,7 @@
"polymer-bundler": "^4.0.10",
"rollup": "^2.3.4",
"rollup-plugin-commonjs": "^10.1.0",
+ "rollup-plugin-define": "^1.0.1",
"rollup-plugin-node-resolve": "^5.2.0",
"rollup-plugin-terser": "^5.1.3",
"typescript": "^4.7.2"
diff --git a/tools/node_tools/yarn.lock b/tools/node_tools/yarn.lock
index 3914e17..c6b3eab 100644
--- a/tools/node_tools/yarn.lock
+++ b/tools/node_tools/yarn.lock
@@ -216,6 +216,14 @@
resolved "https://registry.yarnpkg.com/@protobufjs/utf8/-/utf8-1.1.0.tgz#a777360b5b39a1a2e5106f8e858f2fd2d060c570"
integrity sha512-Vvn3zZrhQZkkBE8LSuW3em98c0FwgO4nxzv6OdSxPKJIEKY2bGbHn+mhGIPerzI4twdxaP8/0+06HBpwf345Lw==
+"@rollup/pluginutils@^4.0.0":
+ version "4.2.1"
+ resolved "https://registry.yarnpkg.com/@rollup/pluginutils/-/pluginutils-4.2.1.tgz#e6c6c3aba0744edce3fb2074922d3776c0af2a6d"
+ integrity sha512-iKnFXr7NkdZAIHiIWE+BX5ULi/ucVFYWD6TbAV+rZctiRTY2PL6tsIKhoIOaoskiWAkgu+VsbXgUVDNLHf+InQ==
+ dependencies:
+ estree-walker "^2.0.1"
+ picomatch "^2.2.2"
+
"@sindresorhus/is@^4.0.0":
version "4.6.0"
resolved "https://registry.yarnpkg.com/@sindresorhus/is/-/is-4.6.0.tgz#3c7c9c46e678feefe7a2e5bb609d3dbd665ffb3f"
@@ -576,6 +584,11 @@
resolved "https://registry.yarnpkg.com/array-back/-/array-back-3.1.0.tgz#b8859d7a508871c9a7b2cf42f99428f65e96bfb0"
integrity sha512-TkuxA4UCOvxuDK6NZYXCalszEzj+TLszyASooky+i742l9TqsOdYCMJJupxRic61hwquNtppB3hgcuq9SVSH1Q==
+ast-matcher@^1.1.1:
+ version "1.1.1"
+ resolved "https://registry.yarnpkg.com/ast-matcher/-/ast-matcher-1.1.1.tgz#95a6dc72318319507024fff438b7839e4e280813"
+ integrity sha512-wQPAp09kPFRQsOijM2Blfg4lH6B9MIhIUrhFtDdhD/1JFhPmfg2/+WAjViVYl3N7EwleHI+q/enTHjaDrv+wEw==
+
async@^2.0.1:
version "2.6.4"
resolved "https://registry.yarnpkg.com/async/-/async-2.6.4.tgz#706b7ff6084664cd7eae713f6f965433b5504221"
@@ -985,6 +998,11 @@
resolved "https://registry.yarnpkg.com/escape-string-regexp/-/escape-string-regexp-1.0.5.tgz#1b61c0562190a8dff6ae3bb2cf0200ca130b86d4"
integrity sha512-vbRorB5FUQWvla16U8R/qgaFIya2qGzwDrNmCZuYKrbdSUMG6I1ZCGQRefkRVhuOkIGVne7BQ35DSfo1qvJqFg==
+escape-string-regexp@^4.0.0:
+ version "4.0.0"
+ resolved "https://registry.yarnpkg.com/escape-string-regexp/-/escape-string-regexp-4.0.0.tgz#14ba83a5d373e3d311e5afca29cf5bfad965bf34"
+ integrity sha512-TtpcNJ3XAzx3Gq8sWRzJaVajRs0uVxA2YAkdb1jm2YkPz4G6egUFAyA3n5vtEIZefPk5Wa4UXbKuS5fKkJWdgA==
+
espree@^3.5.2:
version "3.5.4"
resolved "https://registry.yarnpkg.com/espree/-/espree-3.5.4.tgz#b0f447187c8a8bed944b815a660bddf5deb5d1a7"
@@ -998,6 +1016,11 @@
resolved "https://registry.yarnpkg.com/estree-walker/-/estree-walker-0.6.1.tgz#53049143f40c6eb918b23671d1fe3219f3a1b362"
integrity sha512-SqmZANLWS0mnatqbSfRP5g8OXZC12Fgg1IwNtLsyHDzJizORW4khDfjPqJZsemPWBB2uqykUah5YpQ6epsqC/w==
+estree-walker@^2.0.1:
+ version "2.0.2"
+ resolved "https://registry.yarnpkg.com/estree-walker/-/estree-walker-2.0.2.tgz#52f010178c2a4c117a7757cfe942adb7d2da4cac"
+ integrity sha512-Rfkk/Mp/DL7JVje3u18FxFujQlTNR2q6QfMSMB7AvCBx91NGj/ba3kCfza0f6dVDbw7YlRf/nDrn7pQrCCyQ/w==
+
esutils@^2.0.2:
version "2.0.3"
resolved "https://registry.yarnpkg.com/esutils/-/esutils-2.0.3.tgz#74d2eb4de0b8da1293711910d50775b9b710ef64"
@@ -1325,7 +1348,7 @@
dependencies:
vlq "^0.2.2"
-magic-string@^0.25.2:
+magic-string@^0.25.2, magic-string@^0.25.7:
version "0.25.9"
resolved "https://registry.yarnpkg.com/magic-string/-/magic-string-0.25.9.tgz#de7f9faf91ef8a1c91d02c2e5314c8277dbcdd1c"
integrity sha512-RmF0AsMzgt25qzqqLc1+MbHmhdx0ojF2Fvs4XnOqz2ZOBXzzkEwc/dJQZCYHAn7v1jbVOjAZfK8msRn4BxO4VQ==
@@ -1458,6 +1481,11 @@
resolved "https://registry.yarnpkg.com/pend/-/pend-1.2.0.tgz#7a57eb550a6783f9115331fcf4663d5c8e007a50"
integrity sha512-F3asv42UuXchdzt+xXqfW1OGlVBe+mxa2mqI0pg5yAHZPvFmY3Y6drSf/GQ1A86WgWEN9Kzh/WrgKa6iGcHXLg==
+picomatch@^2.2.2:
+ version "2.3.1"
+ resolved "https://registry.yarnpkg.com/picomatch/-/picomatch-2.3.1.tgz#3ba3833733646d9d3e4995946c1365a67fb07a42"
+ integrity sha512-JU3teHTNjmE2VCGFzuY8EXzCDVwEqB2a8fsIvwaStHhAWJEeVd1o1QD80CU6+ZdEXXSLbSsuLwJjkCBWqRQUVA==
+
plist@^2.0.1:
version "2.1.0"
resolved "https://registry.yarnpkg.com/plist/-/plist-2.1.0.tgz#57ccdb7a0821df21831217a3cad54e3e146a1025"
@@ -1652,6 +1680,16 @@
resolve "^1.11.0"
rollup-pluginutils "^2.8.1"
+rollup-plugin-define@^1.0.1:
+ version "1.0.1"
+ resolved "https://registry.yarnpkg.com/rollup-plugin-define/-/rollup-plugin-define-1.0.1.tgz#45b027cec9d2e30df71118efa156170e3846fd3d"
+ integrity sha512-SM/CKFpLvWq5xBEf84ff/ooT3KodXPVITCkRliyNccuq8SZMpzthN/Bp7JkWScbGTX5lo1SF3cjxKKDjnnFCuA==
+ dependencies:
+ "@rollup/pluginutils" "^4.0.0"
+ ast-matcher "^1.1.1"
+ escape-string-regexp "^4.0.0"
+ magic-string "^0.25.7"
+
rollup-plugin-node-resolve@^5.2.0:
version "5.2.0"
resolved "https://registry.yarnpkg.com/rollup-plugin-node-resolve/-/rollup-plugin-node-resolve-5.2.0.tgz#730f93d10ed202473b1fb54a5997a7db8c6d8523"
