Merge "Change OutgoingEmail to use a soy instead of string concatenation."
diff --git a/Documentation/metrics.txt b/Documentation/metrics.txt
index 974e897..6c9dfef 100644
--- a/Documentation/metrics.txt
+++ b/Documentation/metrics.txt
@@ -131,6 +131,7 @@
* `proc/cpu/usage`: CPU time used by the Gerrit process.
* `proc/cpu/system_load`: System load average for the last minute.
* `proc/num_open_fds`: Number of open file descriptors.
+* `proc/jvm/memory/allocated`: Total memory allocated by all threads since Gerrit process started.
* `proc/jvm/memory/heap_committed`: Amount of memory guaranteed for user objects.
* `proc/jvm/memory/heap_used`: Amount of memory holding user objects.
* `proc/jvm/memory/non_heap_committed`: Amount of memory guaranteed for classes,
diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt
index bf51252..d9ab64d 100644
--- a/Documentation/rest-api-changes.txt
+++ b/Documentation/rest-api-changes.txt
@@ -379,6 +379,11 @@
as link:#tracking-id-info[TrackingIdInfo].
--
+[[custom-keyed-values]]
+--
+* `CUSTOM_KEYED_VALUES`: include the custom key-value map
+---
+
.Request
----
GET /changes/?q=97&o=CURRENT_REVISION&o=CURRENT_COMMIT&o=CURRENT_FILES&o=DOWNLOAD_COMMANDS HTTP/1.0
@@ -6919,6 +6924,11 @@
|==================================
|Field Name ||Description
|`id` ||
+The ID of the change. Subject to a 'GerritBackendFeature__return_new_change_info_id'
+ experiment, the format is either "'<project>\~<_number>'" (new format),
+or `triplet_id`. The experiment is needed to allow callers to migrate.
+'project' and '_number' are URL encoded. The callers must not rely on the format.
+|`triplet_id` ||
The ID of the change in the format "'<project>\~<branch>~<Change-Id>'",
where 'project', 'branch' and 'Change-Id' are URL encoded. For 'branch' the
`refs/heads/` prefix is omitted.
@@ -6939,6 +6949,10 @@
of the account from the attention set.
|`hashtags` |optional|
List of hashtags that are set on the change.
+|`custom_keyed_values` |optional|
+A map that maps custom keys to custom values that are tied to a specific change,
+both in the form of strings. Only set if link:#custom-keyed-values[custom keyed
+values] are requested.
|`change_id` ||The Change-Id of the change.
|`subject` ||
The subject of the change (header line of the commit message).
diff --git a/Documentation/rest-api-config.txt b/Documentation/rest-api-config.txt
index 1faffb2..fe9b13c 100644
--- a/Documentation/rest-api-config.txt
+++ b/Documentation/rest-api-config.txt
@@ -2089,10 +2089,11 @@
|`start_time` ||The start time of the task.
|`delay` ||The remaining delay of the task.
|`command` ||The command of the task.
+|`queue_name` ||The work queue the task is associated with.
|`remote_name`|optional|
The remote name. May only be set for tasks that are associated with a
project.
-|`project` |optional|The project the task is associated with.
+|`project_name` |optional|The project the task is associated with.
|====================================
[[task-summary-info]]
diff --git a/java/Main.java b/java/Main.java
index 09c8c76..c04db2c 100644
--- a/java/Main.java
+++ b/java/Main.java
@@ -16,6 +16,7 @@
public final class Main {
private static final String FLOGGER_BACKEND_PROPERTY = "flogger.backend_factory";
private static final String FLOGGER_LOGGING_CONTEXT = "flogger.logging_context";
+ private static final Runtime.Version MIN_JAVA_VERSION = Runtime.Version.parse("11.0.10");
// We don't do any real work here because we need to import
// the archive lookup code and we cannot import a class in
@@ -34,11 +35,11 @@
}
private static boolean onSupportedJavaVersion() {
- final String version = System.getProperty("java.specification.version");
- if (1.8 <= parse(version)) {
+ Runtime.Version version = Runtime.version();
+ if (version.compareTo(MIN_JAVA_VERSION) >= 0) {
return true;
}
- System.err.println("fatal: Gerrit Code Review requires Java 8 or later");
+ System.err.println("fatal: Gerrit Code Review requires Java " + MIN_JAVA_VERSION + " or later");
System.err.println(" (trying to run on Java " + version + ")");
return false;
}
@@ -58,22 +59,5 @@
"com.google.common.flogger.backend.log4j.Log4jBackendFactory#getInstance");
}
- private static double parse(String version) {
- if (version == null || version.length() == 0) {
- return 0.0;
- }
-
- try {
- final int fd = version.indexOf('.');
- final int sd = version.indexOf('.', fd + 1);
- if (0 < sd) {
- version = version.substring(0, sd);
- }
- return Double.parseDouble(version);
- } catch (NumberFormatException e) {
- return 0.0;
- }
- }
-
private Main() {}
}
diff --git a/java/com/google/gerrit/auth/BUILD b/java/com/google/gerrit/auth/BUILD
index e844696..f04334d 100644
--- a/java/com/google/gerrit/auth/BUILD
+++ b/java/com/google/gerrit/auth/BUILD
@@ -12,8 +12,6 @@
srcs = glob(
["**/*.java"],
),
- resource_strip_prefix = "resources",
- resources = ["//resources/com/google/gerrit/server"],
visibility = ["//visibility:public"],
deps = [
"//java/com/google/gerrit/common:annotations",
diff --git a/java/com/google/gerrit/entities/BUILD b/java/com/google/gerrit/entities/BUILD
index dbbcf71..c0f5de6 100644
--- a/java/com/google/gerrit/entities/BUILD
+++ b/java/com/google/gerrit/entities/BUILD
@@ -10,7 +10,6 @@
deps = [
"//java/com/google/gerrit/common:annotations",
"//java/com/google/gerrit/extensions:api",
- "//java/com/google/gerrit/server:project_util",
"//lib:gson",
"//lib:guava",
"//lib:jgit",
diff --git a/java/com/google/gerrit/entities/Project.java b/java/com/google/gerrit/entities/Project.java
index b43650b..72ca6a9 100644
--- a/java/com/google/gerrit/entities/Project.java
+++ b/java/com/google/gerrit/entities/Project.java
@@ -23,7 +23,6 @@
import com.google.gerrit.extensions.client.InheritableBoolean;
import com.google.gerrit.extensions.client.ProjectState;
import com.google.gerrit.extensions.client.SubmitType;
-import com.google.gerrit.server.ProjectUtil;
import java.io.Serializable;
import java.util.Arrays;
import java.util.HashMap;
diff --git a/java/com/google/gerrit/server/ProjectUtil.java b/java/com/google/gerrit/entities/ProjectUtil.java
similarity index 97%
rename from java/com/google/gerrit/server/ProjectUtil.java
rename to java/com/google/gerrit/entities/ProjectUtil.java
index e87c8bf..98ce67a 100644
--- a/java/com/google/gerrit/server/ProjectUtil.java
+++ b/java/com/google/gerrit/entities/ProjectUtil.java
@@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.
-package com.google.gerrit.server;
+package com.google.gerrit.entities;
public class ProjectUtil {
public static String sanitizeProjectName(String name) {
diff --git a/java/com/google/gerrit/extensions/client/ListChangesOption.java b/java/com/google/gerrit/extensions/client/ListChangesOption.java
index f1f7831..de4326e 100644
--- a/java/com/google/gerrit/extensions/client/ListChangesOption.java
+++ b/java/com/google/gerrit/extensions/client/ListChangesOption.java
@@ -88,7 +88,10 @@
SKIP_DIFFSTAT(23),
/** Include the evaluated submit requirements for the caller. */
- SUBMIT_REQUIREMENTS(24);
+ SUBMIT_REQUIREMENTS(24),
+
+ /** Include custom keyed values. */
+ CUSTOM_KEYED_VALUES(25);
private final int value;
diff --git a/java/com/google/gerrit/extensions/common/ChangeInfo.java b/java/com/google/gerrit/extensions/common/ChangeInfo.java
index dc9bc32..a2e2e8f 100644
--- a/java/com/google/gerrit/extensions/common/ChangeInfo.java
+++ b/java/com/google/gerrit/extensions/common/ChangeInfo.java
@@ -37,6 +37,8 @@
// protected by any ListChangesOption.
public String id;
+ public String tripletId;
+
public String project;
public String branch;
public String topic;
@@ -49,6 +51,8 @@
public Map<Integer, AttentionSetInfo> removedFromAttentionSet;
+ public Map<String, String> customKeyedValues;
+
public Collection<String> hashtags;
public String changeId;
public String subject;
diff --git a/java/com/google/gerrit/gpg/BUILD b/java/com/google/gerrit/gpg/BUILD
index b2173c4..fcf4f0f 100644
--- a/java/com/google/gerrit/gpg/BUILD
+++ b/java/com/google/gerrit/gpg/BUILD
@@ -11,7 +11,6 @@
"//java/com/google/gerrit/extensions:api",
"//java/com/google/gerrit/git",
"//java/com/google/gerrit/server",
- "//java/com/google/gerrit/server/api",
"//lib:guava",
"//lib:jgit",
"//lib/auto:auto-factory",
diff --git a/java/com/google/gerrit/gpg/PublicKeyStoreUtil.java b/java/com/google/gerrit/gpg/PublicKeyStoreUtil.java
new file mode 100644
index 0000000..5333826
--- /dev/null
+++ b/java/com/google/gerrit/gpg/PublicKeyStoreUtil.java
@@ -0,0 +1,80 @@
+// Copyright (C) 2023 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.gpg;
+
+import static com.google.gerrit.server.account.externalids.ExternalId.SCHEME_GPGKEY;
+
+import com.google.common.flogger.FluentLogger;
+import com.google.common.io.BaseEncoding;
+import com.google.gerrit.entities.Account;
+import com.google.gerrit.server.account.externalids.ExternalId;
+import com.google.gerrit.server.account.externalids.ExternalIds;
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import org.bouncycastle.openpgp.PGPException;
+import org.bouncycastle.openpgp.PGPPublicKey;
+import org.bouncycastle.openpgp.PGPPublicKeyRing;
+import org.eclipse.jgit.util.NB;
+
+public class PublicKeyStoreUtil {
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+
+ private final ExternalIds externalIds;
+ private final Provider<PublicKeyStore> storeProvider;
+
+ @Inject
+ public PublicKeyStoreUtil(ExternalIds externalIds, Provider<PublicKeyStore> storeProvider) {
+ this.externalIds = externalIds;
+ this.storeProvider = storeProvider;
+ }
+
+ public static byte[] parseFingerprint(ExternalId gpgKeyExtId) {
+ return BaseEncoding.base16().decode(gpgKeyExtId.key().id());
+ }
+
+ public static long keyIdFromFingerprint(byte[] fp) {
+ return NB.decodeInt64(fp, fp.length - 8);
+ }
+
+ public List<PGPPublicKey> listGpgKeysForUser(Account.Id id) throws PGPException, IOException {
+ List<PGPPublicKey> keys = new ArrayList<>();
+ try (PublicKeyStore store = storeProvider.get()) {
+ for (ExternalId extId : getGpgExtIds(id)) {
+ byte[] fp = parseFingerprint(extId);
+ boolean found = false;
+ for (PGPPublicKeyRing keyRing : store.get(keyIdFromFingerprint(fp))) {
+ if (Arrays.equals(keyRing.getPublicKey().getFingerprint(), fp)) {
+ found = true;
+ keys.add(keyRing.getPublicKey());
+ break;
+ }
+ }
+ if (!found) {
+ logger.atWarning().log(
+ "No public key stored for fingerprint %s", Fingerprint.toString(fp));
+ }
+ }
+ }
+ return keys;
+ }
+
+ public Iterable<ExternalId> getGpgExtIds(Account.Id id) throws IOException {
+ return externalIds.byAccount(id, SCHEME_GPGKEY);
+ }
+}
diff --git a/java/com/google/gerrit/gpg/api/GpgApiAdapterImpl.java b/java/com/google/gerrit/gpg/api/GpgApiAdapterImpl.java
index 6ae0334..57fda5b 100644
--- a/java/com/google/gerrit/gpg/api/GpgApiAdapterImpl.java
+++ b/java/com/google/gerrit/gpg/api/GpgApiAdapterImpl.java
@@ -14,8 +14,6 @@
package com.google.gerrit.gpg.api;
-import static com.google.gerrit.server.api.ApiUtil.asRestApiException;
-
import com.google.gerrit.extensions.api.accounts.GpgKeyApi;
import com.google.gerrit.extensions.api.accounts.GpgKeysInput;
import com.google.gerrit.extensions.common.GpgKeyInfo;
@@ -70,8 +68,10 @@
return gpgKeys.get().list().apply(account).value();
} catch (PGPException | IOException e) {
throw new GpgException(e);
+ } catch (RestApiException e) {
+ throw e;
} catch (Exception e) {
- throw asRestApiException("Cannot list GPG keys", e);
+ throw RestApiException.wrap("Cannot list GPG keys", e);
}
}
@@ -86,8 +86,10 @@
return postGpgKeys.get().apply(account, in).value();
} catch (PGPException | IOException | ConfigInvalidException e) {
throw new GpgException(e);
+ } catch (RestApiException e) {
+ throw e;
} catch (Exception e) {
- throw asRestApiException("Cannot put GPG keys", e);
+ throw RestApiException.wrap("Cannot put GPG keys", e);
}
}
diff --git a/java/com/google/gerrit/gpg/api/GpgKeyApiImpl.java b/java/com/google/gerrit/gpg/api/GpgKeyApiImpl.java
index 0ff12e8..2a05f35 100644
--- a/java/com/google/gerrit/gpg/api/GpgKeyApiImpl.java
+++ b/java/com/google/gerrit/gpg/api/GpgKeyApiImpl.java
@@ -14,8 +14,6 @@
package com.google.gerrit.gpg.api;
-import static com.google.gerrit.server.api.ApiUtil.asRestApiException;
-
import com.google.gerrit.extensions.api.accounts.GpgKeyApi;
import com.google.gerrit.extensions.common.GpgKeyInfo;
import com.google.gerrit.extensions.common.Input;
@@ -50,7 +48,7 @@
try {
return get.apply(rsrc).value();
} catch (Exception e) {
- throw asRestApiException("Cannot get GPG key", e);
+ throw RestApiException.wrap("Cannot get GPG key", e);
}
}
@@ -58,8 +56,10 @@
public void delete() throws RestApiException {
try {
delete.apply(rsrc, new Input());
+ } catch (RestApiException e) {
+ throw e;
} catch (PGPException | IOException | ConfigInvalidException e) {
- throw asRestApiException("Cannot delete GPG key", e);
+ throw RestApiException.wrap("Cannot delete GPG key", e);
}
}
}
diff --git a/java/com/google/gerrit/gpg/server/GpgKeys.java b/java/com/google/gerrit/gpg/server/GpgKeys.java
index 00a0f57..9fb8286 100644
--- a/java/com/google/gerrit/gpg/server/GpgKeys.java
+++ b/java/com/google/gerrit/gpg/server/GpgKeys.java
@@ -14,13 +14,10 @@
package com.google.gerrit.gpg.server;
-import static com.google.gerrit.server.account.externalids.ExternalId.SCHEME_GPGKEY;
import static java.nio.charset.StandardCharsets.UTF_8;
import com.google.common.base.CharMatcher;
import com.google.common.collect.ImmutableList;
-import com.google.common.flogger.FluentLogger;
-import com.google.common.io.BaseEncoding;
import com.google.gerrit.extensions.common.GpgKeyInfo;
import com.google.gerrit.extensions.registration.DynamicMap;
import com.google.gerrit.extensions.restapi.AuthException;
@@ -36,10 +33,10 @@
import com.google.gerrit.gpg.GerritPublicKeyChecker;
import com.google.gerrit.gpg.PublicKeyChecker;
import com.google.gerrit.gpg.PublicKeyStore;
+import com.google.gerrit.gpg.PublicKeyStoreUtil;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.account.AccountResource;
import com.google.gerrit.server.account.externalids.ExternalId;
-import com.google.gerrit.server.account.externalids.ExternalIds;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
@@ -48,36 +45,35 @@
import java.util.Arrays;
import java.util.HashMap;
import java.util.Iterator;
+import java.util.List;
import java.util.Locale;
import java.util.Map;
import org.bouncycastle.bcpg.ArmoredOutputStream;
import org.bouncycastle.openpgp.PGPException;
import org.bouncycastle.openpgp.PGPPublicKey;
import org.bouncycastle.openpgp.PGPPublicKeyRing;
-import org.eclipse.jgit.util.NB;
@Singleton
public class GpgKeys implements ChildCollection<AccountResource, GpgKey> {
- private static final FluentLogger logger = FluentLogger.forEnclosingClass();
private final DynamicMap<RestView<GpgKey>> views;
private final Provider<CurrentUser> self;
+ private final PublicKeyStoreUtil publicKeyStoreUtil;
private final Provider<PublicKeyStore> storeProvider;
private final GerritPublicKeyChecker.Factory checkerFactory;
- private final ExternalIds externalIds;
@Inject
GpgKeys(
DynamicMap<RestView<GpgKey>> views,
Provider<CurrentUser> self,
+ PublicKeyStoreUtil publicKeyStoreUtil,
Provider<PublicKeyStore> storeProvider,
- GerritPublicKeyChecker.Factory checkerFactory,
- ExternalIds externalIds) {
+ GerritPublicKeyChecker.Factory checkerFactory) {
this.views = views;
this.self = self;
+ this.publicKeyStoreUtil = publicKeyStoreUtil;
this.storeProvider = storeProvider;
this.checkerFactory = checkerFactory;
- this.externalIds = externalIds;
}
@Override
@@ -90,10 +86,11 @@
throws ResourceNotFoundException, PGPException, IOException {
checkVisible(self, parent);
- ExternalId gpgKeyExtId = findGpgKey(id.get(), getGpgExtIds(parent));
- byte[] fp = parseFingerprint(gpgKeyExtId);
+ ExternalId gpgKeyExtId =
+ findGpgKey(id.get(), publicKeyStoreUtil.getGpgExtIds(parent.getUser().getAccountId()));
+ byte[] fp = PublicKeyStoreUtil.parseFingerprint(gpgKeyExtId);
try (PublicKeyStore store = storeProvider.get()) {
- long keyId = keyId(fp);
+ long keyId = PublicKeyStoreUtil.keyIdFromFingerprint(fp);
for (PGPPublicKeyRing keyRing : store.get(keyId)) {
PGPPublicKey key = keyRing.getPublicKey();
if (Arrays.equals(key.getFingerprint(), fp)) {
@@ -131,10 +128,6 @@
return gpgKeyExtId;
}
- static byte[] parseFingerprint(ExternalId gpgKeyExtId) {
- return BaseEncoding.base16().decode(gpgKeyExtId.key().id());
- }
-
@Override
public DynamicMap<RestView<GpgKey>> views() {
return views;
@@ -145,29 +138,17 @@
public Response<Map<String, GpgKeyInfo>> apply(AccountResource rsrc)
throws PGPException, IOException, ResourceNotFoundException {
checkVisible(self, rsrc);
- Map<String, GpgKeyInfo> keys = new HashMap<>();
+ List<PGPPublicKey> keys =
+ publicKeyStoreUtil.listGpgKeysForUser(rsrc.getUser().getAccountId());
+ Map<String, GpgKeyInfo> res = new HashMap<>();
try (PublicKeyStore store = storeProvider.get()) {
- for (ExternalId extId : getGpgExtIds(rsrc)) {
- byte[] fp = parseFingerprint(extId);
- boolean found = false;
- for (PGPPublicKeyRing keyRing : store.get(keyId(fp))) {
- if (Arrays.equals(keyRing.getPublicKey().getFingerprint(), fp)) {
- found = true;
- GpgKeyInfo info =
- toJson(
- keyRing.getPublicKey(), checkerFactory.create(rsrc.getUser(), store), store);
- keys.put(info.id, info);
- info.id = null;
- break;
- }
- }
- if (!found) {
- logger.atWarning().log(
- "No public key stored for fingerprint %s", Fingerprint.toString(fp));
- }
+ for (PGPPublicKey key : keys) {
+ GpgKeyInfo info = toJson(key, checkerFactory.create(rsrc.getUser(), store), store);
+ res.put(info.id, info);
+ info.id = null;
}
}
- return Response.ok(keys);
+ return Response.ok(res);
}
}
@@ -194,14 +175,6 @@
}
}
- private Iterable<ExternalId> getGpgExtIds(AccountResource rsrc) throws IOException {
- return externalIds.byAccount(rsrc.getUser().getAccountId(), SCHEME_GPGKEY);
- }
-
- private static long keyId(byte[] fp) {
- return NB.decodeInt64(fp, fp.length - 8);
- }
-
static void checkVisible(Provider<CurrentUser> self, AccountResource rsrc)
throws ResourceNotFoundException {
if (!BouncyCastleUtil.havePGP()) {
diff --git a/java/com/google/gerrit/gpg/server/PostGpgKeys.java b/java/com/google/gerrit/gpg/server/PostGpgKeys.java
index 7c7efdc..edd5a58 100644
--- a/java/com/google/gerrit/gpg/server/PostGpgKeys.java
+++ b/java/com/google/gerrit/gpg/server/PostGpgKeys.java
@@ -46,6 +46,7 @@
import com.google.gerrit.gpg.GerritPublicKeyChecker;
import com.google.gerrit.gpg.PublicKeyChecker;
import com.google.gerrit.gpg.PublicKeyStore;
+import com.google.gerrit.gpg.PublicKeyStoreUtil;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.IdentifiedUser;
@@ -175,7 +176,8 @@
for (String id : input.delete) {
try {
ExternalId gpgKeyExtId = GpgKeys.findGpgKey(id, existingExtIds);
- fingerprints.put(gpgKeyExtId, new Fingerprint(GpgKeys.parseFingerprint(gpgKeyExtId)));
+ fingerprints.put(
+ gpgKeyExtId, new Fingerprint(PublicKeyStoreUtil.parseFingerprint(gpgKeyExtId)));
} catch (ResourceNotFoundException e) {
// Skip removal.
}
diff --git a/java/com/google/gerrit/httpd/restapi/RestApiServlet.java b/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
index 210ba7b..9056732 100644
--- a/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
+++ b/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
@@ -1615,7 +1615,9 @@
private void checkUserSession(HttpServletRequest req) throws AuthException {
CurrentUser user = globals.currentUser.get();
if (isRead(req)) {
- user.setAccessPath(AccessPath.REST_API);
+ if (user.getAccessPath().equals(AccessPath.UNKNOWN)) {
+ user.setAccessPath(AccessPath.REST_API);
+ }
} else if (user instanceof AnonymousUser) {
throw new AuthException("Authentication required");
} else if (!globals.webSession.get().isAccessPathOk(AccessPath.REST_API)) {
diff --git a/java/com/google/gerrit/index/project/ProjectSchemaDefinitions.java b/java/com/google/gerrit/index/project/ProjectSchemaDefinitions.java
index 3ac594e..dac1012 100644
--- a/java/com/google/gerrit/index/project/ProjectSchemaDefinitions.java
+++ b/java/com/google/gerrit/index/project/ProjectSchemaDefinitions.java
@@ -62,7 +62,10 @@
@Deprecated static final Schema<ProjectData> V4 = schema(V3);
// Upgrade Lucene to 7.x requires reindexing.
- static final Schema<ProjectData> V5 = schema(V4);
+ @Deprecated static final Schema<ProjectData> V5 = schema(V4);
+
+ // Upgrade Lucene to 8.x requires reindexing.
+ static final Schema<ProjectData> V6 = schema(V5);
/**
* Name of the project index to be used when contacting index backends or loading configurations.
diff --git a/java/com/google/gerrit/lucene/AbstractLuceneIndex.java b/java/com/google/gerrit/lucene/AbstractLuceneIndex.java
index f9dc31a..78ee128 100644
--- a/java/com/google/gerrit/lucene/AbstractLuceneIndex.java
+++ b/java/com/google/gerrit/lucene/AbstractLuceneIndex.java
@@ -544,8 +544,7 @@
int realLimit = opts.start() + opts.pageSize();
TopFieldDocs docs =
opts.searchAfter() != null
- ? searcher.searchAfter(
- (ScoreDoc) opts.searchAfter(), query, realLimit, sort, false, false)
+ ? searcher.searchAfter((ScoreDoc) opts.searchAfter(), query, realLimit, sort, false)
: searcher.search(query, realLimit, sort);
ImmutableList.Builder<T> b = ImmutableList.builderWithExpectedSize(docs.scoreDocs.length);
for (int i = opts.start(); i < docs.scoreDocs.length; i++) {
diff --git a/java/com/google/gerrit/lucene/LuceneChangeIndex.java b/java/com/google/gerrit/lucene/LuceneChangeIndex.java
index 6365260..728606d 100644
--- a/java/com/google/gerrit/lucene/LuceneChangeIndex.java
+++ b/java/com/google/gerrit/lucene/LuceneChangeIndex.java
@@ -415,12 +415,7 @@
if (maxRemainingHits > 0) {
TopFieldDocs subIndexHits =
searchers[i].searchAfter(
- searchAfter,
- query,
- maxRemainingHits,
- sort,
- /* doDocScores= */ false,
- /* doMaxScore= */ false);
+ searchAfter, query, maxRemainingHits, sort, /* doDocScores= */ false);
searchAfterHitsCount += subIndexHits.scoreDocs.length;
hits.add(subIndexHits);
searchAfterBySubIndex.put(
diff --git a/java/com/google/gerrit/metrics/proc/ProcMetricModule.java b/java/com/google/gerrit/metrics/proc/ProcMetricModule.java
index 09e40c1..301ec85 100644
--- a/java/com/google/gerrit/metrics/proc/ProcMetricModule.java
+++ b/java/com/google/gerrit/metrics/proc/ProcMetricModule.java
@@ -34,6 +34,8 @@
import java.util.concurrent.TimeUnit;
public class ProcMetricModule extends MetricModule {
+ private static final ThreadMXBeanInterface threadMxBean = ThreadMXBeanFactory.create();
+
@Override
protected void configure(MetricMaker metrics) {
buildLabel(metrics);
@@ -167,6 +169,14 @@
objectPendingFinalizationCount.set(memory.getObjectPendingFinalizationCount());
});
+
+ if (threadMxBean.supportsAllocatedBytes()) {
+ metrics.newCallbackMetric(
+ "proc/jvm/memory/allocated",
+ Long.class,
+ new Description("Allocated memory").setCumulative().setUnit(Units.BYTES),
+ () -> getTotalAllocatedBytes());
+ }
}
private void procJvmMemoryPool(MetricMaker metrics) {
@@ -312,4 +322,19 @@
});
}
}
+
+ private static long getTotalAllocatedBytes() {
+ if (!threadMxBean.supportsAllocatedBytes()) {
+ return -1;
+ }
+
+ long[] ids = threadMxBean.getAllThreadIds();
+ long[] allocatedBytes = threadMxBean.getAllThreadsAllocatedBytes(ids);
+ long total = 0;
+
+ for (long a : allocatedBytes) {
+ total += a;
+ }
+ return total;
+ }
}
diff --git a/java/com/google/gerrit/metrics/proc/ThreadMXBeanInterface.java b/java/com/google/gerrit/metrics/proc/ThreadMXBeanInterface.java
index 546924f..03e59a0 100644
--- a/java/com/google/gerrit/metrics/proc/ThreadMXBeanInterface.java
+++ b/java/com/google/gerrit/metrics/proc/ThreadMXBeanInterface.java
@@ -19,4 +19,12 @@
long getCurrentThreadUserTime();
long getCurrentThreadAllocatedBytes();
+
+ boolean supportsAllocatedBytes();
+
+ long getThreadAllocatedBytes(long threadId);
+
+ long[] getAllThreadsAllocatedBytes(long[] threadIds);
+
+ long[] getAllThreadIds();
}
diff --git a/java/com/google/gerrit/metrics/proc/ThreadMXBeanJava.java b/java/com/google/gerrit/metrics/proc/ThreadMXBeanJava.java
index 29dd42a..361aba0 100644
--- a/java/com/google/gerrit/metrics/proc/ThreadMXBeanJava.java
+++ b/java/com/google/gerrit/metrics/proc/ThreadMXBeanJava.java
@@ -37,4 +37,24 @@
public long getCurrentThreadAllocatedBytes() {
return -1;
}
+
+ @Override
+ public boolean supportsAllocatedBytes() {
+ return false;
+ }
+
+ @Override
+ public long getThreadAllocatedBytes(long threadId) {
+ return -1;
+ }
+
+ @Override
+ public long[] getAllThreadsAllocatedBytes(long[] threadIds) {
+ return new long[0];
+ }
+
+ @Override
+ public long[] getAllThreadIds() {
+ return sys.getAllThreadIds();
+ }
}
diff --git a/java/com/google/gerrit/metrics/proc/ThreadMXBeanSun.java b/java/com/google/gerrit/metrics/proc/ThreadMXBeanSun.java
index f5555b5..ca750cd 100644
--- a/java/com/google/gerrit/metrics/proc/ThreadMXBeanSun.java
+++ b/java/com/google/gerrit/metrics/proc/ThreadMXBeanSun.java
@@ -37,4 +37,24 @@
public long getCurrentThreadAllocatedBytes() {
return sys.getCurrentThreadAllocatedBytes();
}
+
+ @Override
+ public boolean supportsAllocatedBytes() {
+ return true;
+ }
+
+ @Override
+ public long getThreadAllocatedBytes(long threadId) {
+ return sys.getThreadAllocatedBytes(threadId);
+ }
+
+ @Override
+ public long[] getAllThreadsAllocatedBytes(long[] threadIds) {
+ return sys.getThreadAllocatedBytes(threadIds);
+ }
+
+ @Override
+ public long[] getAllThreadIds() {
+ return sys.getAllThreadIds();
+ }
}
diff --git a/java/com/google/gerrit/server/BUILD b/java/com/google/gerrit/server/BUILD
index 104a44a..d68f809 100644
--- a/java/com/google/gerrit/server/BUILD
+++ b/java/com/google/gerrit/server/BUILD
@@ -14,10 +14,6 @@
"account/externalids/testing/ExternalIdTestUtil.java",
]
-PROJECT_UTIL_SRC = [
- "ProjectUtil.java",
-]
-
PROLOG_SRC = ["rules/prolog/*.java"]
java_library(
@@ -26,12 +22,6 @@
visibility = ["//visibility:public"],
)
-java_library(
- name = "project_util",
- srcs = PROJECT_UTIL_SRC,
- visibility = ["//visibility:public"],
-)
-
# Giant kitchen-sink target.
#
# The only reason this hasn't been split up further is because we have too many
@@ -43,14 +33,13 @@
srcs = glob(
["**/*.java"],
exclude = CONSTANTS_SRC + GERRIT_GLOBAL_MODULE_SRC + TESTING_SRC +
- PROJECT_UTIL_SRC + PROLOG_SRC,
+ PROLOG_SRC,
),
resource_strip_prefix = "resources",
resources = ["//resources/com/google/gerrit/server"],
visibility = ["//visibility:public"],
deps = [
":constants",
- ":project_util",
"//java/com/google/gerrit/common:annotations",
"//java/com/google/gerrit/common:server",
"//java/com/google/gerrit/entities",
diff --git a/java/com/google/gerrit/server/args4j/ProjectHandler.java b/java/com/google/gerrit/server/args4j/ProjectHandler.java
index a1e45e9..dc7fa24 100644
--- a/java/com/google/gerrit/server/args4j/ProjectHandler.java
+++ b/java/com/google/gerrit/server/args4j/ProjectHandler.java
@@ -18,8 +18,8 @@
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.entities.Project;
+import com.google.gerrit.entities.ProjectUtil;
import com.google.gerrit.extensions.restapi.AuthException;
-import com.google.gerrit.server.ProjectUtil;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.permissions.ProjectPermission;
diff --git a/java/com/google/gerrit/server/audit/BUILD b/java/com/google/gerrit/server/audit/BUILD
index bde7404..39cec8b 100644
--- a/java/com/google/gerrit/server/audit/BUILD
+++ b/java/com/google/gerrit/server/audit/BUILD
@@ -5,8 +5,6 @@
srcs = glob(
["**/*.java"],
),
- resource_strip_prefix = "resources",
- resources = ["//resources/com/google/gerrit/server"],
visibility = ["//visibility:public"],
deps = [
"//java/com/google/gerrit/entities",
diff --git a/java/com/google/gerrit/server/cache/PerThreadRefDbCache.java b/java/com/google/gerrit/server/cache/PerThreadRefDbCache.java
index a0526e1..82c2856b 100644
--- a/java/com/google/gerrit/server/cache/PerThreadRefDbCache.java
+++ b/java/com/google/gerrit/server/cache/PerThreadRefDbCache.java
@@ -12,9 +12,8 @@
// See the License for the specific language governing permissions and
// limitations under the License.
-package com.google.gerrit.server.git;
+package com.google.gerrit.server.cache;
-import com.google.gerrit.server.cache.PerThreadCache;
import java.io.File;
import java.util.HashMap;
import java.util.Map;
diff --git a/java/com/google/gerrit/server/change/ActionJson.java b/java/com/google/gerrit/server/change/ActionJson.java
index e5a9534..00673d5 100644
--- a/java/com/google/gerrit/server/change/ActionJson.java
+++ b/java/com/google/gerrit/server/change/ActionJson.java
@@ -123,6 +123,10 @@
changeInfo.removedFromAttentionSet == null
? null
: ImmutableMap.copyOf(changeInfo.removedFromAttentionSet);
+ copy.customKeyedValues =
+ changeInfo.customKeyedValues == null
+ ? null
+ : ImmutableMap.copyOf(changeInfo.customKeyedValues);
copy.hashtags = changeInfo.hashtags;
copy.changeId = changeInfo.changeId;
copy.submitType = changeInfo.submitType;
@@ -147,6 +151,7 @@
copy.unresolvedCommentCount = changeInfo.unresolvedCommentCount;
copy.workInProgress = changeInfo.workInProgress;
copy.id = changeInfo.id;
+ copy.tripletId = changeInfo.tripletId;
copy.cherryPickOfChange = changeInfo.cherryPickOfChange;
copy.cherryPickOfPatchSet = changeInfo.cherryPickOfPatchSet;
return copy;
diff --git a/java/com/google/gerrit/server/change/ChangeJson.java b/java/com/google/gerrit/server/change/ChangeJson.java
index f733a7b..839629e 100644
--- a/java/com/google/gerrit/server/change/ChangeJson.java
+++ b/java/com/google/gerrit/server/change/ChangeJson.java
@@ -23,6 +23,7 @@
import static com.google.gerrit.extensions.client.ListChangesOption.CURRENT_ACTIONS;
import static com.google.gerrit.extensions.client.ListChangesOption.CURRENT_COMMIT;
import static com.google.gerrit.extensions.client.ListChangesOption.CURRENT_REVISION;
+import static com.google.gerrit.extensions.client.ListChangesOption.CUSTOM_KEYED_VALUES;
import static com.google.gerrit.extensions.client.ListChangesOption.DETAILED_ACCOUNTS;
import static com.google.gerrit.extensions.client.ListChangesOption.DETAILED_LABELS;
import static com.google.gerrit.extensions.client.ListChangesOption.LABELS;
@@ -101,6 +102,8 @@
import com.google.gerrit.server.cancellation.RequestCancelledException;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.config.TrackingFooters;
+import com.google.gerrit.server.experiments.ExperimentFeatures;
+import com.google.gerrit.server.experiments.ExperimentFeaturesConstants;
import com.google.gerrit.server.index.change.ChangeField;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.ReviewerStateInternal;
@@ -240,9 +243,11 @@
private AccountLoader accountLoader;
private FixInput fix;
+ private ExperimentFeatures experimentFeatures;
@Inject
ChangeJson(
+ ExperimentFeatures experimentFeatures,
Provider<CurrentUser> user,
PermissionBackend permissionBackend,
ChangeData.Factory cdf,
@@ -259,6 +264,7 @@
@GerritServerConfig Config cfg,
@Assisted Iterable<ListChangesOption> options,
@Assisted Optional<PluginDefinedInfosFactory> pluginDefinedInfosFactory) {
+ this.experimentFeatures = experimentFeatures;
this.userProvider = user;
this.changeDataFactory = cdf;
this.permissionBackend = permissionBackend;
@@ -422,10 +428,17 @@
return info;
}
- private static void finish(ChangeInfo info) {
- info.id =
+ private static void finish(ChangeInfo info, ExperimentFeatures experimentFeatures) {
+ info.tripletId =
Joiner.on('~')
.join(Url.encode(info.project), Url.encode(info.branch), Url.encode(info.changeId));
+ if (experimentFeatures.isFeatureEnabled(
+ ExperimentFeaturesConstants.GERRIT_BACKEND_FEATURE_RETURN_NEW_CHANGE_INFO_ID)) {
+ info.id =
+ Joiner.on('~').join(Url.encode(info.project), Url.encode(String.valueOf(info._number)));
+ } else {
+ info.id = info.tripletId;
+ }
}
private static boolean containsAnyOf(
@@ -563,7 +576,7 @@
info.isPrivate = c.isPrivate() ? true : null;
info.workInProgress = c.isWorkInProgress() ? true : null;
info.hasReviewStarted = c.hasReviewStarted();
- finish(info);
+ finish(info, experimentFeatures);
} else {
info._number = result.id().get();
info.problems = result.problems();
@@ -617,6 +630,9 @@
a -> a.account().get(),
a -> AttentionSetUtil.createAttentionSetInfo(a, accountLoader)));
}
+ if (has(CUSTOM_KEYED_VALUES)) {
+ out.customKeyedValues = cd.customKeyedValues();
+ }
out.hashtags = cd.hashtags();
out.changeId = in.getKey().get();
if (in.isNew()) {
@@ -727,7 +743,7 @@
if (needMessages) {
out.messages = messages(cd);
}
- finish(out);
+ finish(out, experimentFeatures);
// This block must come after the ChangeInfo is mostly populated, since
// it will be passed to ActionVisitors as-is.
diff --git a/java/com/google/gerrit/server/documentation/QueryDocumentationExecutor.java b/java/com/google/gerrit/server/documentation/QueryDocumentationExecutor.java
index cd49ea6..093b87c 100644
--- a/java/com/google/gerrit/server/documentation/QueryDocumentationExecutor.java
+++ b/java/com/google/gerrit/server/documentation/QueryDocumentationExecutor.java
@@ -34,6 +34,7 @@
import org.apache.lucene.search.Query;
import org.apache.lucene.search.ScoreDoc;
import org.apache.lucene.search.TopDocs;
+import org.apache.lucene.search.TotalHits;
import org.apache.lucene.store.ByteBuffersDirectory;
import org.apache.lucene.store.Directory;
import org.apache.lucene.store.IndexOutput;
@@ -84,10 +85,10 @@
// We don't have much documentation, so we just use MAX_VALUE here and skip paging.
TopDocs results = searcher.search(query, Integer.MAX_VALUE);
ScoreDoc[] hits = results.scoreDocs;
- long totalHits = results.totalHits;
+ TotalHits totalHits = results.totalHits;
List<DocResult> out = new ArrayList<>();
- for (int i = 0; i < totalHits; i++) {
+ for (int i = 0; i < totalHits.value; i++) {
DocResult result = new DocResult();
Document doc = searcher.doc(hits[i].doc);
result.url = doc.get(Constants.URL_FIELD);
diff --git a/java/com/google/gerrit/server/experiments/ConfigExperimentFeatures.java b/java/com/google/gerrit/server/experiments/ConfigExperimentFeatures.java
index 227deb5..8de5db3 100644
--- a/java/com/google/gerrit/server/experiments/ConfigExperimentFeatures.java
+++ b/java/com/google/gerrit/server/experiments/ConfigExperimentFeatures.java
@@ -15,6 +15,7 @@
package com.google.gerrit.server.experiments;
import com.google.common.collect.ImmutableSet;
+import com.google.gerrit.entities.Project;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.inject.AbstractModule;
import com.google.inject.Inject;
@@ -57,6 +58,11 @@
}
@Override
+ public boolean isFeatureEnabled(String featureFlag, Project.NameKey project) {
+ return isFeatureEnabled(featureFlag);
+ }
+
+ @Override
public ImmutableSet<String> getEnabledExperimentFeatures() {
return enabledExperimentFeatures;
}
diff --git a/java/com/google/gerrit/server/experiments/ExperimentFeatures.java b/java/com/google/gerrit/server/experiments/ExperimentFeatures.java
index dc9148a..fd885ed 100644
--- a/java/com/google/gerrit/server/experiments/ExperimentFeatures.java
+++ b/java/com/google/gerrit/server/experiments/ExperimentFeatures.java
@@ -15,6 +15,7 @@
package com.google.gerrit.server.experiments;
import com.google.common.collect.ImmutableSet;
+import com.google.gerrit.entities.Project;
/**
* Features that can be enabled/disabled on Gerrit (e. g. experiments to research new behavior in
@@ -37,6 +38,12 @@
boolean isFeatureEnabled(String featureFlag);
/**
+ * Same {@link #isFeatureEnabled}, but takes into account {@code project}, when evaluating the
+ * experiment.
+ */
+ boolean isFeatureEnabled(String featureFlag, Project.NameKey project);
+
+ /**
* Returns the names of the features that are enabled on Gerrit instance (either by default or via
* gerrit.config).
*/
diff --git a/java/com/google/gerrit/server/experiments/ExperimentFeaturesConstants.java b/java/com/google/gerrit/server/experiments/ExperimentFeaturesConstants.java
index e294d55..fd7d504 100644
--- a/java/com/google/gerrit/server/experiments/ExperimentFeaturesConstants.java
+++ b/java/com/google/gerrit/server/experiments/ExperimentFeaturesConstants.java
@@ -29,4 +29,11 @@
/** On BatchUpdate, do not await index completion before returning to the user */
public static String GERRIT_BACKEND_FEATURE_DO_NOT_AWAIT_CHANGE_INDEXING =
"GerritBackendFeature__do_not_await_change_indexing";
+
+ /**
+ * Sets ChangeInfo.id to "'<project>~<_number>'", instead of "'<project>~<branch>~<Change-Id>'",
+ * spearing an index lookup if the id is used in the follow-up API calls.
+ */
+ public static String GERRIT_BACKEND_FEATURE_RETURN_NEW_CHANGE_INFO_ID =
+ "GerritBackendFeature__return_new_change_info_id";
}
diff --git a/java/com/google/gerrit/server/git/LocalDiskRepositoryManager.java b/java/com/google/gerrit/server/git/LocalDiskRepositoryManager.java
index 5cf7de5..5eb913d 100644
--- a/java/com/google/gerrit/server/git/LocalDiskRepositoryManager.java
+++ b/java/com/google/gerrit/server/git/LocalDiskRepositoryManager.java
@@ -20,6 +20,7 @@
import com.google.gerrit.entities.RefNames;
import com.google.gerrit.extensions.events.LifecycleListener;
import com.google.gerrit.lifecycle.LifecycleModule;
+import com.google.gerrit.server.cache.PerThreadRefDbCache;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.config.SitePaths;
import com.google.inject.Inject;
diff --git a/java/com/google/gerrit/server/index/account/AccountSchemaDefinitions.java b/java/com/google/gerrit/server/index/account/AccountSchemaDefinitions.java
index 8e7d964..fd264a1 100644
--- a/java/com/google/gerrit/server/index/account/AccountSchemaDefinitions.java
+++ b/java/com/google/gerrit/server/index/account/AccountSchemaDefinitions.java
@@ -85,7 +85,10 @@
.build();
// Upgrade Lucene to 7.x requires reindexing.
- static final Schema<AccountState> V12 = schema(V11);
+ @Deprecated static final Schema<AccountState> V12 = schema(V11);
+
+ // Upgrade Lucene to 8.x requires reindexing.
+ static final Schema<AccountState> V13 = schema(V12);
/**
* Name of the account index to be used when contacting index backends or loading configurations.
diff --git a/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java b/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java
index e74ce8f..93521f3 100644
--- a/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java
+++ b/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java
@@ -239,7 +239,7 @@
.build();
/** Remove assignee field. */
- @SuppressWarnings("deprecation")
+ @Deprecated
static final Schema<ChangeData> V82 =
new Schema.Builder<ChangeData>()
.add(V81)
@@ -247,6 +247,10 @@
.remove(ChangeField.ASSIGNEE_FIELD)
.build();
+ /** Upgrade Lucene to 8.x requires reindexing. */
+ @SuppressWarnings("deprecation")
+ static final Schema<ChangeData> V83 = schema(V82);
+
/**
* Name of the change index to be used when contacting index backends or loading configurations.
*/
diff --git a/java/com/google/gerrit/server/index/group/GroupSchemaDefinitions.java b/java/com/google/gerrit/server/index/group/GroupSchemaDefinitions.java
index f0f3510..1b87d27 100644
--- a/java/com/google/gerrit/server/index/group/GroupSchemaDefinitions.java
+++ b/java/com/google/gerrit/server/index/group/GroupSchemaDefinitions.java
@@ -69,7 +69,10 @@
@Deprecated static final Schema<InternalGroup> V8 = schema(V7);
// Upgrade Lucene to 7.x requires reindexing.
- static final Schema<InternalGroup> V9 = schema(V8);
+ @Deprecated static final Schema<InternalGroup> V9 = schema(V8);
+
+ // Upgrade Lucene to 8.x requires reindexing.
+ static final Schema<InternalGroup> V10 = schema(V9);
/** Singleton instance of the schema definitions. This is one per JVM. */
public static final GroupSchemaDefinitions INSTANCE = new GroupSchemaDefinitions();
diff --git a/java/com/google/gerrit/server/notedb/ChangeNoteFooters.java b/java/com/google/gerrit/server/notedb/ChangeNoteFooters.java
index 3be55ea..eb1f692 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNoteFooters.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNoteFooters.java
@@ -23,6 +23,7 @@
public static final FooterKey FOOTER_CHANGE_ID = new FooterKey("Change-id");
public static final FooterKey FOOTER_COMMIT = new FooterKey("Commit");
public static final FooterKey FOOTER_CURRENT = new FooterKey("Current");
+ public static final FooterKey FOOTER_CUSTOM_KEYED_VALUE = new FooterKey("Custom-Keyed-Value");
public static final FooterKey FOOTER_GROUPS = new FooterKey("Groups");
public static final FooterKey FOOTER_HASHTAGS = new FooterKey("Hashtags");
public static final FooterKey FOOTER_LABEL = new FooterKey("Label");
diff --git a/java/com/google/gerrit/server/notedb/ChangeNotes.java b/java/com/google/gerrit/server/notedb/ChangeNotes.java
index c75fd29..4a31e23 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNotes.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNotes.java
@@ -394,6 +394,7 @@
// Lazy defensive copies of mutable ReviewDb types, to avoid polluting the
// ChangeNotesCache from handlers.
+ private ImmutableSortedMap<String, String> customKeyedValues;
private ImmutableSortedMap<PatchSet.Id, PatchSet> patchSets;
private PatchSetApprovals approvals;
private ImmutableSet<Comment.Key> commentKeys;
@@ -477,6 +478,16 @@
return state.allAttentionSetUpdates();
}
+ /** Returns the key-value pairs that are attached to this change */
+ public ImmutableSortedMap<String, String> getCustomKeyedValues() {
+ if (customKeyedValues == null) {
+ ImmutableSortedMap.Builder<String, String> b = ImmutableSortedMap.naturalOrder();
+ b.putAll(state.customKeyedValues());
+ customKeyedValues = b.build();
+ }
+ return customKeyedValues;
+ }
+
/**
* Returns the evaluated submit requirements for the change. We only intend to store submit
* requirements in NoteDb for closed changes. For closed changes, the results represent the state
diff --git a/java/com/google/gerrit/server/notedb/ChangeNotesCache.java b/java/com/google/gerrit/server/notedb/ChangeNotesCache.java
index 2d3902c..37729b8 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNotesCache.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNotesCache.java
@@ -62,7 +62,7 @@
.weigher(Weigher.class)
.maximumWeight(10 << 20)
.diskLimit(-1)
- .version(5)
+ .version(6)
.keySerializer(Key.Serializer.INSTANCE)
.valueSerializer(ChangeNotesState.Serializer.INSTANCE);
}
diff --git a/java/com/google/gerrit/server/notedb/ChangeNotesParser.java b/java/com/google/gerrit/server/notedb/ChangeNotesParser.java
index 951a478..95e528b 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNotesParser.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNotesParser.java
@@ -22,6 +22,7 @@
import static com.google.gerrit.server.notedb.ChangeNoteFooters.FOOTER_COMMIT;
import static com.google.gerrit.server.notedb.ChangeNoteFooters.FOOTER_COPIED_LABEL;
import static com.google.gerrit.server.notedb.ChangeNoteFooters.FOOTER_CURRENT;
+import static com.google.gerrit.server.notedb.ChangeNoteFooters.FOOTER_CUSTOM_KEYED_VALUE;
import static com.google.gerrit.server.notedb.ChangeNoteFooters.FOOTER_GROUPS;
import static com.google.gerrit.server.notedb.ChangeNoteFooters.FOOTER_HASHTAGS;
import static com.google.gerrit.server.notedb.ChangeNoteFooters.FOOTER_LABEL;
@@ -47,6 +48,7 @@
import com.google.common.collect.HashBasedTable;
import com.google.common.collect.ImmutableListMultimap;
import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.ImmutableSortedMap;
import com.google.common.collect.ImmutableTable;
import com.google.common.collect.ListMultimap;
import com.google.common.collect.Lists;
@@ -98,6 +100,7 @@
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
+import java.util.TreeMap;
import java.util.TreeSet;
import java.util.function.Function;
import java.util.stream.Collectors;
@@ -166,6 +169,7 @@
private final Set<PatchSet.Id> deletedPatchSets;
private final Map<PatchSet.Id, PatchSetState> patchSetStates;
private final List<PatchSet.Id> currentPatchSets;
+ private final TreeMap<String, String> customKeyedValues;
private final Map<PatchSetApproval.Key, PatchSetApproval.Builder> approvals;
private final List<PatchSetApproval.Builder> bufferedApprovals;
private final List<ChangeMessage> allChangeMessages;
@@ -234,6 +238,7 @@
deletedPatchSets = new HashSet<>();
patchSetStates = new HashMap<>();
currentPatchSets = new ArrayList<>();
+ customKeyedValues = new TreeMap<>();
}
ChangeNotesState parseAll() throws ConfigInvalidException, IOException {
@@ -264,6 +269,7 @@
checkMandatoryFooters();
}
+ pruneEmptyCustomKeyedValues();
return buildState();
}
@@ -288,6 +294,7 @@
submissionId,
status,
firstNonNull(hashtags, ImmutableSet.of()),
+ ImmutableSortedMap.copyOfSorted(customKeyedValues),
buildPatchSets(),
buildApprovals(),
ReviewerSet.fromTable(Tables.transpose(reviewers)),
@@ -491,6 +498,7 @@
}
parseHashtags(commit);
+ parseCustomKeyedValues(commit);
parseAttentionSetUpdates(commit);
parseSubmission(commit, commitTimestamp);
@@ -722,6 +730,30 @@
}
}
+ private void parseCustomKeyedValues(ChangeNotesCommit commit) throws ConfigInvalidException {
+ for (String customKeyedValueLine : commit.getFooterLineValues(FOOTER_CUSTOM_KEYED_VALUE)) {
+ String[] parts = customKeyedValueLine.split("=", 2);
+ String key = parts[0];
+ String value = parts[1];
+ // Commits are parsed in reverse order and only the last set of values
+ // should be used. An empty value for a key means it's a deletion.
+ customKeyedValues.putIfAbsent(key, value);
+ }
+ }
+
+ private void pruneEmptyCustomKeyedValues() {
+ List<String> toRemove = new ArrayList<>();
+ for (Map.Entry<String, String> entry : customKeyedValues.entrySet()) {
+ if (entry.getValue().length() == 0) {
+ toRemove.add(entry.getKey());
+ }
+ }
+
+ for (String key : toRemove) {
+ customKeyedValues.remove(key);
+ }
+ }
+
private void parseAttentionSetUpdates(ChangeNotesCommit commit) throws ConfigInvalidException {
List<String> attentionStrings = commit.getFooterLineValues(FOOTER_ATTENTION);
for (String attentionString : attentionStrings) {
diff --git a/java/com/google/gerrit/server/notedb/ChangeNotesState.java b/java/com/google/gerrit/server/notedb/ChangeNotesState.java
index 1715b43..304b54d 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNotesState.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNotesState.java
@@ -28,6 +28,7 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableListMultimap;
import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.ImmutableSortedMap;
import com.google.common.collect.ImmutableTable;
import com.google.common.collect.ListMultimap;
import com.google.common.collect.Maps;
@@ -111,6 +112,7 @@
@Nullable String submissionId,
@Nullable Change.Status status,
Set<String> hashtags,
+ ImmutableSortedMap<String, String> customKeyedValues,
Map<PatchSet.Id, PatchSet> patchSets,
ListMultimap<PatchSet.Id, PatchSetApproval> approvals,
ReviewerSet reviewers,
@@ -163,6 +165,7 @@
.cherryPickOf(cherryPickOf)
.build())
.hashtags(hashtags)
+ .customKeyedValues(customKeyedValues.entrySet())
.serverId(serverId)
.patchSets(patchSets.entrySet())
.approvals(approvals.entries())
@@ -290,6 +293,16 @@
// Other related to this Change.
abstract ImmutableSet<String> hashtags();
+ /*
+ Custom values are small key value pairs. They can be used to associate the
+ change with external, potentially proprietary systems (e.g. Bug trackers)
+ without requiring dedicated fields in Gerrit-core.
+
+ This data is visible to everyone who can see the change. It must not contain
+ personally identify-able information.
+ */
+ abstract ImmutableList<Map.Entry<String, String>> customKeyedValues();
+
@Nullable
abstract String serverId();
@@ -384,6 +397,7 @@
return new AutoValue_ChangeNotesState.Builder()
.changeId(changeId)
.hashtags(ImmutableSet.of())
+ .customKeyedValues(ImmutableList.of())
.patchSets(ImmutableList.of())
.approvals(ImmutableList.of())
.reviewers(ReviewerSet.empty())
@@ -411,6 +425,8 @@
abstract Builder hashtags(Iterable<String> hashtags);
+ abstract Builder customKeyedValues(Iterable<Map.Entry<String, String>> customKeyedValues);
+
abstract Builder patchSets(Iterable<Map.Entry<PatchSet.Id, PatchSet>> patchSets);
abstract Builder approvals(Iterable<Map.Entry<PatchSet.Id, PatchSetApproval>> approvals);
@@ -475,6 +491,14 @@
b.setHasServerId(true);
}
object.hashtags().forEach(b::addHashtag);
+
+ object
+ .customKeyedValues()
+ .forEach(
+ entry -> {
+ b.putCustomKeyedValues(entry.getKey(), entry.getValue());
+ });
+
object
.patchSets()
.forEach(e -> b.addPatchSet(PatchSetProtoConverter.INSTANCE.toProto(e.getValue())));
@@ -614,6 +638,7 @@
.columns(toChangeColumns(changeId, proto.getColumns()))
.serverId(proto.getHasServerId() ? proto.getServerId() : null)
.hashtags(proto.getHashtagList())
+ .customKeyedValues(proto.getCustomKeyedValuesMap().entrySet())
.patchSets(
proto.getPatchSetList().stream()
.map(msg -> PatchSetProtoConverter.INSTANCE.fromProto(msg))
diff --git a/java/com/google/gerrit/server/notedb/ChangeUpdate.java b/java/com/google/gerrit/server/notedb/ChangeUpdate.java
index 0a895fb..be755ea 100644
--- a/java/com/google/gerrit/server/notedb/ChangeUpdate.java
+++ b/java/com/google/gerrit/server/notedb/ChangeUpdate.java
@@ -25,6 +25,7 @@
import static com.google.gerrit.server.notedb.ChangeNoteFooters.FOOTER_COMMIT;
import static com.google.gerrit.server.notedb.ChangeNoteFooters.FOOTER_COPIED_LABEL;
import static com.google.gerrit.server.notedb.ChangeNoteFooters.FOOTER_CURRENT;
+import static com.google.gerrit.server.notedb.ChangeNoteFooters.FOOTER_CUSTOM_KEYED_VALUE;
import static com.google.gerrit.server.notedb.ChangeNoteFooters.FOOTER_GROUPS;
import static com.google.gerrit.server.notedb.ChangeNoteFooters.FOOTER_HASHTAGS;
import static com.google.gerrit.server.notedb.ChangeNoteFooters.FOOTER_LABEL;
@@ -100,6 +101,7 @@
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
+import java.util.TreeMap;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import org.eclipse.jgit.errors.ConfigInvalidException;
@@ -163,6 +165,7 @@
private Map<Account.Id, AttentionSetUpdate> plannedAttentionSetUpdates;
private boolean ignoreFurtherAttentionSetUpdates;
private Set<String> hashtags;
+ private TreeMap<String, String> customKeyedValues = new TreeMap<>();
private String changeMessage;
private String tag;
private PatchSetState psState;
@@ -462,6 +465,14 @@
this.hashtags = hashtags;
}
+ public void addCustomKeyedValue(String key, String value) {
+ this.customKeyedValues.put(key, value);
+ }
+
+ public void deleteCustomKeyedValue(String key) {
+ this.customKeyedValues.put(key, "");
+ }
+
/**
* Adds attention set updates that should be stored in NoteDb.
*
@@ -764,6 +775,10 @@
addFooter(msg, FOOTER_HASHTAGS, comma.join(hashtags));
}
+ for (Map.Entry<String, String> entry : customKeyedValues.entrySet()) {
+ addFooter(msg, FOOTER_CUSTOM_KEYED_VALUE, entry.getKey() + "=" + entry.getValue());
+ }
+
if (tag != null) {
addFooter(msg, FOOTER_TAG, tag);
}
@@ -1159,6 +1174,7 @@
&& submissionId == null
&& submitRecords == null
&& hashtags == null
+ && customKeyedValues.isEmpty()
&& topic == null
&& commit == null
&& psState == null
diff --git a/java/com/google/gerrit/server/query/change/ChangeData.java b/java/com/google/gerrit/server/query/change/ChangeData.java
index 929aa94..46b85b7 100644
--- a/java/com/google/gerrit/server/query/change/ChangeData.java
+++ b/java/com/google/gerrit/server/query/change/ChangeData.java
@@ -331,6 +331,7 @@
private SubmitTypeRecord submitTypeRecord;
private Boolean mergeable;
private Set<String> hashtags;
+ private Map<String, String> customKeyedValues;
/**
* Map from {@link com.google.gerrit.entities.Account.Id} to the tip of the edit ref for this
* change and a given user.
@@ -1183,6 +1184,16 @@
this.hashtags = hashtags;
}
+ public Map<String, String> customKeyedValues() {
+ if (customKeyedValues == null) {
+ if (!lazyload()) {
+ return Collections.emptyMap();
+ }
+ customKeyedValues = notes().getCustomKeyedValues();
+ }
+ return customKeyedValues;
+ }
+
public ImmutableListMultimap<Account.Id, String> stars() {
if (stars == null) {
if (!lazyload()) {
diff --git a/java/com/google/gerrit/server/restapi/BUILD b/java/com/google/gerrit/server/restapi/BUILD
index 4ce8c42..dd0ec78d 100644
--- a/java/com/google/gerrit/server/restapi/BUILD
+++ b/java/com/google/gerrit/server/restapi/BUILD
@@ -21,7 +21,6 @@
"//java/com/google/gerrit/json",
"//java/com/google/gerrit/metrics",
"//java/com/google/gerrit/server",
- "//java/com/google/gerrit/server:project_util",
"//java/com/google/gerrit/server/ioutil",
"//java/com/google/gerrit/server/logging",
"//java/com/google/gerrit/server/util/time",
diff --git a/java/com/google/gerrit/server/restapi/change/RebaseChain.java b/java/com/google/gerrit/server/restapi/change/RebaseChain.java
index b8afcb7..343fb72 100644
--- a/java/com/google/gerrit/server/restapi/change/RebaseChain.java
+++ b/java/com/google/gerrit/server/restapi/change/RebaseChain.java
@@ -317,10 +317,18 @@
private List<PatchSetData> getChainForCurrentPatchSet(ChangeResource rsrc)
throws PermissionBackendException, IOException {
- return Lists.reverse(
- getRelatedChangesUtil.getAncestors(
- changeDataFactory.create(rsrc.getNotes()),
- patchSetUtil.current(rsrc.getNotes()),
- true));
+ List<PatchSetData> ancestors =
+ Lists.reverse(
+ getRelatedChangesUtil.getAncestors(
+ changeDataFactory.create(rsrc.getNotes()),
+ patchSetUtil.current(rsrc.getNotes()),
+ true));
+ int eldestOpenAncestor = 0;
+ for (PatchSetData ps : ancestors) {
+ if (ps.data().change().isMerged()) {
+ eldestOpenAncestor++;
+ }
+ }
+ return ancestors.subList(eldestOpenAncestor, ancestors.size());
}
}
diff --git a/java/com/google/gerrit/server/restapi/project/CreateChange.java b/java/com/google/gerrit/server/restapi/project/CreateChange.java
index 2f1153e..e30759d 100644
--- a/java/com/google/gerrit/server/restapi/project/CreateChange.java
+++ b/java/com/google/gerrit/server/restapi/project/CreateChange.java
@@ -15,6 +15,7 @@
package com.google.gerrit.server.restapi.project;
import com.google.common.base.Strings;
+import com.google.gerrit.entities.ProjectUtil;
import com.google.gerrit.exceptions.InvalidNameException;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.ChangeInput;
@@ -24,7 +25,6 @@
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.server.CurrentUser;
-import com.google.gerrit.server.ProjectUtil;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.InvalidChangeOperationException;
import com.google.gerrit.server.project.ProjectResource;
diff --git a/java/com/google/gerrit/server/restapi/project/CreateProject.java b/java/com/google/gerrit/server/restapi/project/CreateProject.java
index 8203346..cfdadd9 100644
--- a/java/com/google/gerrit/server/restapi/project/CreateProject.java
+++ b/java/com/google/gerrit/server/restapi/project/CreateProject.java
@@ -21,6 +21,7 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Lists;
import com.google.gerrit.common.data.GlobalCapability;
+import com.google.gerrit.entities.ProjectUtil;
import com.google.gerrit.entities.RefNames;
import com.google.gerrit.extensions.annotations.RequiresCapability;
import com.google.gerrit.extensions.api.projects.ConfigInput;
@@ -35,7 +36,6 @@
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestCollectionCreateView;
import com.google.gerrit.extensions.restapi.TopLevelResource;
-import com.google.gerrit.server.ProjectUtil;
import com.google.gerrit.server.config.AllProjectsName;
import com.google.gerrit.server.config.AllUsersName;
import com.google.gerrit.server.config.GerritServerConfig;
diff --git a/java/com/google/gerrit/server/restapi/project/ProjectsCollection.java b/java/com/google/gerrit/server/restapi/project/ProjectsCollection.java
index 6174798..f9602bc 100644
--- a/java/com/google/gerrit/server/restapi/project/ProjectsCollection.java
+++ b/java/com/google/gerrit/server/restapi/project/ProjectsCollection.java
@@ -18,6 +18,7 @@
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.Project;
+import com.google.gerrit.entities.ProjectUtil;
import com.google.gerrit.extensions.registration.DynamicMap;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
@@ -32,7 +33,6 @@
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
import com.google.gerrit.json.OutputFormat;
import com.google.gerrit.server.CurrentUser;
-import com.google.gerrit.server.ProjectUtil;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.permissions.ProjectPermission;
diff --git a/java/com/google/gerrit/server/update/BatchUpdate.java b/java/com/google/gerrit/server/update/BatchUpdate.java
index 9250513..14d781d 100644
--- a/java/com/google/gerrit/server/update/BatchUpdate.java
+++ b/java/com/google/gerrit/server/update/BatchUpdate.java
@@ -19,6 +19,7 @@
import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.common.collect.ImmutableMultiset.toImmutableMultiset;
import static com.google.common.flogger.LazyArgs.lazy;
+import static com.google.gerrit.common.UsedAt.Project.GOOGLE;
import static java.util.Comparator.comparing;
import static java.util.Objects.requireNonNull;
import static java.util.stream.Collectors.toMap;
@@ -39,6 +40,7 @@
import com.google.common.util.concurrent.ListenableFuture;
import com.google.errorprone.annotations.CanIgnoreReturnValue;
import com.google.gerrit.common.Nullable;
+import com.google.gerrit.common.UsedAt;
import com.google.gerrit.entities.AttentionSetUpdate;
import com.google.gerrit.entities.BranchNameKey;
import com.google.gerrit.entities.Change;
@@ -52,6 +54,7 @@
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.RestApiException;
+import com.google.gerrit.server.AccessPath;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.RefLogIdentityProvider;
@@ -666,9 +669,14 @@
}
}
+ // For upstream implementation, AccessPath.WEB_BROWSER is never set, so the method will always
+ // return false.
+ @UsedAt(GOOGLE)
private boolean indexAsync() {
- return experimentFeatures.isFeatureEnabled(
- ExperimentFeaturesConstants.GERRIT_BACKEND_FEATURE_DO_NOT_AWAIT_CHANGE_INDEXING);
+ return user.getAccessPath().equals(AccessPath.WEB_BROWSER)
+ && experimentFeatures.isFeatureEnabled(
+ ExperimentFeaturesConstants.GERRIT_BACKEND_FEATURE_DO_NOT_AWAIT_CHANGE_INDEXING,
+ project);
}
private void fireRefChangeEvent() {
@@ -751,6 +759,8 @@
}
}
if (indexAsync) {
+ logger.atFine().log(
+ "Asynchronously reindexing changes, %s in project %s", results.keySet(), project.get());
// We want to index asynchronously. However, the callers will await all
// index futures. This allows us to - even in synchronous case -
// parallelize indexing changes.
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
index 118b31b..96c86d4 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -246,11 +246,14 @@
private Cache<DiffSummaryKey, DiffSummary> diffSummaryCache;
@Test
+ @GerritConfig(
+ name = "experiments.enabled",
+ value = "GerritBackendFeature__return_new_change_info_id")
public void get() throws Exception {
PushOneCommit.Result r = createChange();
- String triplet = project.get() + "~master~" + r.getChangeId();
- ChangeInfo c = info(triplet);
- assertThat(c.id).isEqualTo(triplet);
+ String id = project.get() + "~" + r.getChange().getId().get();
+ ChangeInfo c = info(id);
+ assertThat(c.id).isEqualTo(id);
assertThat(c.project).isEqualTo(project.get());
assertThat(c.branch).isEqualTo("master");
assertThat(c.status).isEqualTo(ChangeStatus.NEW);
diff --git a/javatests/com/google/gerrit/acceptance/api/change/RebaseIT.java b/javatests/com/google/gerrit/acceptance/api/change/RebaseIT.java
index fd37fb0..5ecb5a7 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/RebaseIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/RebaseIT.java
@@ -986,7 +986,7 @@
public void rebaseChain() throws Exception {
// Create changes with the following hierarchy:
// * HEAD
- // * r1
+ // * r (merged)
// * r2
// * r3
// * r4
@@ -1043,7 +1043,7 @@
final String newContent = "new content";
// Create changes with the following revision hierarchy:
// * HEAD
- // * r1
+ // * r (merged)
// * r2
// * r3/1 r3/2
// * r4
@@ -1079,6 +1079,71 @@
}
@Test
+ public void rebaseChainWithMergedAncestor() throws Exception {
+ final String file = "modified_file.txt";
+ final String newContent = "new content";
+
+ // Create changes with the following hierarchy:
+ // * HEAD
+ // * r (merged)
+ // * r2.1 r2.2 (merged)
+ // * r3
+ // * r4
+ // *r5
+ PushOneCommit.Result r = createChange();
+ PushOneCommit.Result r2 = createChange();
+ PushOneCommit.Result r3 = createChange();
+ PushOneCommit.Result r4 = createChange();
+ PushOneCommit.Result r5 = createChange();
+
+ // Approve and submit the first change
+ RevisionApi revision = gApi.changes().id(r.getChangeId()).current();
+ revision.review(ReviewInput.approve());
+ revision.submit();
+ testRepo.reset("HEAD~1");
+
+ // Create r2.2
+ gApi.changes()
+ .id(r2.getChangeId())
+ .edit()
+ .modifyFile(file, RawInputUtil.create(newContent.getBytes(UTF_8)));
+ gApi.changes().id(r2.getChangeId()).edit().publish();
+ // Approve and submit r2.2
+ revision = gApi.changes().id(r2.getChangeId()).current();
+ revision.review(ReviewInput.approve());
+ revision.submit();
+
+ // Add an approval whose score should be copied on trivial rebase
+ gApi.changes().id(r3.getChangeId()).current().review(ReviewInput.recommend());
+
+ // Rebase the chain through r4.
+ verifyRebaseChainResponse(gApi.changes().id(r4.getChangeId()).rebaseChain(), false, r3, r4);
+
+ // Only r3 and r4 are rebased.
+ verifyRebaseForChange(r3.getChange().getId(), r2.getChange().getId(), true);
+ verifyRebaseForChange(r4.getChange().getId(), r3.getChange().getId(), false);
+
+ verifyChangeIsUpToDate(r2);
+ verifyChangeIsUpToDate(r3);
+ verifyChangeIsUpToDate(r4);
+
+ // r5 wasn't rebased.
+ assertThat(
+ gApi.changes()
+ .id(r5.getChangeId())
+ .get(CURRENT_REVISION)
+ .getCurrentRevision()
+ ._number)
+ .isEqualTo(1);
+
+ // Rebasing r5
+ verifyRebaseChainResponse(
+ gApi.changes().id(r5.getChangeId()).rebaseChain(), false, r3, r4, r5);
+
+ verifyRebaseForChange(r5.getChange().getId(), r4.getChange().getId(), false);
+ }
+
+ @Test
public void rebaseChainWithConflicts_conflictsForbidden() throws Exception {
PushOneCommit.Result r1 = createChange();
gApi.changes()
diff --git a/javatests/com/google/gerrit/auth/BUILD b/javatests/com/google/gerrit/auth/BUILD
index 6a41d01..fa30f80a 100644
--- a/javatests/com/google/gerrit/auth/BUILD
+++ b/javatests/com/google/gerrit/auth/BUILD
@@ -7,8 +7,6 @@
srcs = glob(
["**/*.java"],
),
- resource_strip_prefix = "resources",
- resources = ["//resources/com/google/gerrit/server"],
tags = ["no_windows"],
visibility = ["//visibility:public"],
runtime_deps = [
diff --git a/javatests/com/google/gerrit/server/BUILD b/javatests/com/google/gerrit/server/BUILD
index b5c6149..4821f20 100644
--- a/javatests/com/google/gerrit/server/BUILD
+++ b/javatests/com/google/gerrit/server/BUILD
@@ -25,8 +25,6 @@
["**/*.java"],
exclude = CUSTOM_TRUTH_SUBJECTS,
),
- resource_strip_prefix = "resources",
- resources = ["//resources/com/google/gerrit/server"],
tags = ["no_windows"],
visibility = ["//visibility:public"],
runtime_deps = [
diff --git a/javatests/com/google/gerrit/server/ioutil/BUILD b/javatests/com/google/gerrit/server/ioutil/BUILD
index ac9530f..8654522 100644
--- a/javatests/com/google/gerrit/server/ioutil/BUILD
+++ b/javatests/com/google/gerrit/server/ioutil/BUILD
@@ -6,7 +6,6 @@
srcs = glob(
["**/*.java"],
),
- resource_strip_prefix = "resources",
visibility = ["//visibility:public"],
deps = [
"//java/com/google/gerrit/server/ioutil",
diff --git a/javatests/com/google/gerrit/server/notedb/ChangeNotesStateTest.java b/javatests/com/google/gerrit/server/notedb/ChangeNotesStateTest.java
index 9a29230..d04964b 100644
--- a/javatests/com/google/gerrit/server/notedb/ChangeNotesStateTest.java
+++ b/javatests/com/google/gerrit/server/notedb/ChangeNotesStateTest.java
@@ -65,6 +65,7 @@
import java.lang.reflect.Type;
import java.sql.Timestamp;
import java.time.Instant;
+import java.util.AbstractMap.SimpleEntry;
import java.util.List;
import java.util.Map;
import java.util.Optional;
@@ -334,6 +335,24 @@
}
@Test
+ public void serializeCustomKeyedValues() throws Exception {
+ assertRoundTrip(
+ newBuilder()
+ .customKeyedValues(
+ ImmutableList.of(
+ new SimpleEntry<String, String>("key1", "value1"),
+ new SimpleEntry<String, String>("key2", "value2")))
+ .build(),
+ ChangeNotesStateProto.newBuilder()
+ .setMetaId(SHA_BYTES)
+ .setChangeId(ID.get())
+ .setColumns(colsProto)
+ .putCustomKeyedValues("key1", "value1")
+ .putCustomKeyedValues("key2", "value2")
+ .build());
+ }
+
+ @Test
public void serializePatchSets() throws Exception {
PatchSet ps1 =
PatchSet.builder()
@@ -918,6 +937,9 @@
.put("columns", ChangeColumns.class)
.put("hashtags", new TypeLiteral<ImmutableSet<String>>() {}.getType())
.put(
+ "customKeyedValues",
+ new TypeLiteral<ImmutableList<Map.Entry<String, String>>>() {}.getType())
+ .put(
"patchSets",
new TypeLiteral<ImmutableList<Map.Entry<PatchSet.Id, PatchSet>>>() {}.getType())
.put(
diff --git a/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java b/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java
index 15eefcd..69b1870 100644
--- a/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java
+++ b/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java
@@ -34,6 +34,7 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableListMultimap;
import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.ImmutableSortedMap;
import com.google.common.collect.ImmutableTable;
import com.google.common.collect.Iterables;
import com.google.common.collect.ListMultimap;
@@ -67,6 +68,7 @@
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
+import java.util.TreeMap;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Ref;
@@ -1504,6 +1506,78 @@
}
@Test
+ public void customKeyedValuesCommit() throws Exception {
+ Change c = newChange();
+ ChangeUpdate update = newUpdate(c, changeOwner);
+ update.addCustomKeyedValue("key1", "value1");
+ update.addCustomKeyedValue("key2", "value2");
+ update.commit();
+ try (RevWalk walk = new RevWalk(repo)) {
+ RevCommit commit = walk.parseCommit(update.getResult());
+ walk.parseBody(commit);
+ assertThat(commit.getFullMessage()).contains("Custom-Keyed-Value: key1=value1\n");
+ assertThat(commit.getFullMessage()).contains("Custom-Keyed-Value: key2=value2\n");
+ }
+ }
+
+ @Test
+ public void customKeyedValuesChangeNotes() throws Exception {
+ Change c = newChange();
+ ChangeUpdate update = newUpdate(c, changeOwner);
+ update.addCustomKeyedValue("key1", "value\n1");
+ update.addCustomKeyedValue("key2", "value2=value3");
+ update.addCustomKeyedValue("key3", "value3: value4");
+ update.commit();
+
+ TreeMap<String, String> customKeyedValues = new TreeMap<>();
+ customKeyedValues.put("key1", "value 1");
+ customKeyedValues.put("key2", "value2=value3");
+ customKeyedValues.put("key3", "value3: value4");
+ ChangeNotes notes = newNotes(c);
+ assertThat(notes.getCustomKeyedValues())
+ .isEqualTo(ImmutableSortedMap.copyOfSorted(customKeyedValues));
+ }
+
+ @Test
+ public void customKeyedValuesChangeNotes_Override() throws Exception {
+ Change c = newChange();
+ ChangeUpdate update = newUpdate(c, changeOwner);
+ update.addCustomKeyedValue("key1", "value1");
+ update.addCustomKeyedValue("key2", "value2");
+ update.commit();
+
+ update = newUpdate(c, changeOwner);
+ update.addCustomKeyedValue("key1", "value3");
+ update.commit();
+
+ TreeMap<String, String> customKeyedValues = new TreeMap<>();
+ customKeyedValues.put("key1", "value3");
+ customKeyedValues.put("key2", "value2");
+ ChangeNotes notes = newNotes(c);
+ assertThat(notes.getCustomKeyedValues())
+ .isEqualTo(ImmutableSortedMap.copyOfSorted(customKeyedValues));
+ }
+
+ @Test
+ public void customKeyedValuesChangeNotes_Deletion() throws Exception {
+ Change c = newChange();
+ ChangeUpdate update = newUpdate(c, changeOwner);
+ update.addCustomKeyedValue("key1", "value1");
+ update.addCustomKeyedValue("key2", "value2");
+ update.commit();
+
+ update = newUpdate(c, changeOwner);
+ update.deleteCustomKeyedValue("key1");
+ update.commit();
+
+ TreeMap<String, String> customKeyedValues = new TreeMap<>();
+ customKeyedValues.put("key2", "value2");
+ ChangeNotes notes = newNotes(c);
+ assertThat(notes.getCustomKeyedValues())
+ .isEqualTo(ImmutableSortedMap.copyOfSorted(customKeyedValues));
+ }
+
+ @Test
public void topicChangeNotes() throws Exception {
Change c = newChange();
diff --git a/plugins/codemirror-editor b/plugins/codemirror-editor
index e36ecdd..be8e04b 160000
--- a/plugins/codemirror-editor
+++ b/plugins/codemirror-editor
@@ -1 +1 @@
-Subproject commit e36ecddbc99dc434f3259814d9d84fa96e509772
+Subproject commit be8e04b1a6de091a63c9bc79b56508f2ad56a830
diff --git a/plugins/package.json b/plugins/package.json
index 5323c9c..504fc17 100644
--- a/plugins/package.json
+++ b/plugins/package.json
@@ -13,11 +13,12 @@
"@codemirror/commands": "^6.2.3",
"@codemirror/legacy-modes": "^6.3.2",
"@codemirror/lang-cpp": "^6.0.2",
- "@codemirror/lang-css": "^6.1.1",
+ "@codemirror/lang-css": "^6.2.0",
"@codemirror/lang-html": "^6.4.3",
"@codemirror/lang-java": "^6.0.1",
"@codemirror/lang-javascript": "^6.1.7",
"@codemirror/lang-json": "^6.0.1",
+ "@codemirror/lang-less": "^6.0.0",
"@codemirror/lang-markdown": "^6.1.1",
"@codemirror/lang-php": "^6.0.1",
"@codemirror/lang-python": "^6.1.2",
@@ -28,9 +29,9 @@
"@codemirror/language": "^6.6.0",
"@codemirror/language-data": "^6.3.0",
"@codemirror/lint": "^6.2.1",
- "@codemirror/search": "^6.3.0",
+ "@codemirror/search": "^6.4.0",
"@codemirror/state": "^6.2.0",
- "@codemirror/view": "^6.9.6",
+ "@codemirror/view": "^6.10.0",
"lit": "^2.2.3",
"rxjs": "^6.6.7",
"sinon": "^13.0.0"
diff --git a/plugins/yarn.lock b/plugins/yarn.lock
index cde18ce..3f53453 100644
--- a/plugins/yarn.lock
+++ b/plugins/yarn.lock
@@ -62,14 +62,15 @@
"@codemirror/language" "^6.0.0"
"@lezer/cpp" "^1.0.0"
-"@codemirror/lang-css@^6.0.0", "@codemirror/lang-css@^6.1.1":
- version "6.1.1"
- resolved "https://registry.yarnpkg.com/@codemirror/lang-css/-/lang-css-6.1.1.tgz#8c4414d399df14e796f9891a8152e411264ef535"
- integrity sha512-P6jdNEHyRcqqDgbvHYyC9Wxkek0rnG3a9aVSRi4a7WrjPbQtBTaOmvYpXmm13zZMAatO4Oqpac+0QZs7sy+LnQ==
+"@codemirror/lang-css@^6.0.0", "@codemirror/lang-css@^6.1.1", "@codemirror/lang-css@^6.2.0":
+ version "6.2.0"
+ resolved "https://registry.yarnpkg.com/@codemirror/lang-css/-/lang-css-6.2.0.tgz#f84f9da392099432445c75e32fdac63ae572315f"
+ integrity sha512-oyIdJM29AyRPM3+PPq1I2oIk8NpUfEN3kAM05XWDDs6o3gSneIKaVJifT2P+fqONLou2uIgXynFyMUDQvo/szA==
dependencies:
"@codemirror/autocomplete" "^6.0.0"
"@codemirror/language" "^6.0.0"
"@codemirror/state" "^6.0.0"
+ "@lezer/common" "^1.0.2"
"@lezer/css" "^1.0.0"
"@codemirror/lang-html@^6.0.0", "@codemirror/lang-html@^6.4.3":
@@ -116,6 +117,16 @@
"@codemirror/language" "^6.0.0"
"@lezer/json" "^1.0.0"
+"@codemirror/lang-less@^6.0.0":
+ version "6.0.0"
+ resolved "https://registry.yarnpkg.com/@codemirror/lang-less/-/lang-less-6.0.0.tgz#47ac36242f45bcc211dbcbce11e10f3b249519c9"
+ integrity sha512-hQVj+AxcUW/LybRkwaOope8K8+U6bjWH91t0tW8MMok33Y65xo+Wx0t1BaXi3Iuo6CgJ4tW7Rz09cfNwloIdNA==
+ dependencies:
+ "@codemirror/lang-css" "^6.2.0"
+ "@codemirror/language" "^6.0.0"
+ "@lezer/highlight" "^1.0.0"
+ "@lezer/lr" "^1.0.0"
+
"@codemirror/lang-markdown@^6.0.0", "@codemirror/lang-markdown@^6.1.1":
version "6.1.1"
resolved "https://registry.yarnpkg.com/@codemirror/lang-markdown/-/lang-markdown-6.1.1.tgz#ff3cdd339c277f6a02d08eb12f1090977873e771"
@@ -262,10 +273,10 @@
"@codemirror/view" "^6.0.0"
crelt "^1.0.5"
-"@codemirror/search@^6.3.0":
- version "6.3.0"
- resolved "https://registry.yarnpkg.com/@codemirror/search/-/search-6.3.0.tgz#d48b2d636fc7474613e240ba8e56c4c5e6e51822"
- integrity sha512-rBhZxzT34CarfhgCZGhaLBScABDN3iqJxixzNuINp9lrb3lzm0nTpR77G1VrxGO3HOGK7j62jcJftQM7eCOIuw==
+"@codemirror/search@^6.4.0":
+ version "6.4.0"
+ resolved "https://registry.yarnpkg.com/@codemirror/search/-/search-6.4.0.tgz#2b256a9e0eaa9317fb48e3cc81eb2735360a59b4"
+ integrity sha512-zMDgaBXah+nMLK2dHz9GdCnGbQu+oaGRXS1qviqNZkvOCv/whp5XZFyoikLp/23PM9RBcbuKUUISUmQHM1eRHw==
dependencies:
"@codemirror/state" "^6.0.0"
"@codemirror/view" "^6.0.0"
@@ -276,10 +287,10 @@
resolved "https://registry.yarnpkg.com/@codemirror/state/-/state-6.2.0.tgz#a0fb08403ced8c2a68d1d0acee926bd20be922f2"
integrity sha512-69QXtcrsc3RYtOtd+GsvczJ319udtBf1PTrr2KbLWM/e2CXUPnh0Nz9AUo8WfhSQ7GeL8dPVNUmhQVgpmuaNGA==
-"@codemirror/view@^6.0.0", "@codemirror/view@^6.2.2", "@codemirror/view@^6.6.0", "@codemirror/view@^6.9.6":
- version "6.9.6"
- resolved "https://registry.yarnpkg.com/@codemirror/view/-/view-6.9.6.tgz#6bf302608e03566688d5138c50b38d07356e11a1"
- integrity sha512-g68PxS3RkHpxfYS6DTWCy1jeA5/oHzmdWjMVPkOzqQyxpElHEcPncUd4EeMVSa4okt0sS3hNXVaRnJqO/7MeJw==
+"@codemirror/view@^6.0.0", "@codemirror/view@^6.10.0", "@codemirror/view@^6.2.2", "@codemirror/view@^6.6.0":
+ version "6.10.0"
+ resolved "https://registry.yarnpkg.com/@codemirror/view/-/view-6.10.0.tgz#40bb39f391955db8960337a9e80fd7564f8915e2"
+ integrity sha512-Oea3rvE4JQLMmLsy2b54yxXQJgJM9xKpUQIpF/LGgKUTH2lA06GAmEtKKWn5OUnbW3jrH1hHeUd8DJEgePMOeQ==
dependencies:
"@codemirror/state" "^6.1.4"
style-mod "^4.0.0"
@@ -348,9 +359,9 @@
"@lezer/lr" "^1.0.0"
"@lezer/javascript@^1.0.0":
- version "1.4.2"
- resolved "https://registry.yarnpkg.com/@lezer/javascript/-/javascript-1.4.2.tgz#aee4e18f573b496756294c558965d36320bfca47"
- integrity sha512-77qdAD4zanmImPiAu4ibrMUzRc79UHoccdPa+Ey5iwS891TAkhnMAodUe17T7zV7tnF7e9HXM0pfmjoGEhrppg==
+ version "1.4.3"
+ resolved "https://registry.yarnpkg.com/@lezer/javascript/-/javascript-1.4.3.tgz#f59e764a0578184c6fb86abb5279a9679777c3ba"
+ integrity sha512-k7Eo9z9B1supZ5cCD4ilQv/RZVN30eUQL+gGbr6ybrEY3avBAL5MDiYi2aa23Aj0A79ry4rJRvPAwE2TM8bd+A==
dependencies:
"@lezer/highlight" "^1.1.3"
"@lezer/lr" "^1.3.0"
@@ -364,9 +375,9 @@
"@lezer/lr" "^1.0.0"
"@lezer/lr@^1.0.0", "@lezer/lr@^1.1.0", "@lezer/lr@^1.3.0", "@lezer/lr@^1.3.1":
- version "1.3.3"
- resolved "https://registry.yarnpkg.com/@lezer/lr/-/lr-1.3.3.tgz#0ac6c889f1235874f33c45a1b9785d7054f60708"
- integrity sha512-JPQe3mwJlzEVqy67iQiiGozhcngbO8QBgpqZM6oL1Wj/dXckrEexpBLeFkq0edtW5IqnPRFxA24BHJni8Js69w==
+ version "1.3.4"
+ resolved "https://registry.yarnpkg.com/@lezer/lr/-/lr-1.3.4.tgz#8795bf2ba4f69b998e8fb4b5a7c57ea68753474c"
+ integrity sha512-7o+e4og/QoC/6btozDPJqnzBhUaD1fMfmvnEKQO1wRRiTse1WxaJ3OMEXZJnkgT6HCcTVOctSoXK9jGJw2oe9g==
dependencies:
"@lezer/common" "^1.0.0"
diff --git a/polygerrit-ui/app/constants/reporting.ts b/polygerrit-ui/app/constants/reporting.ts
index fb350fd..ef2f63e 100644
--- a/polygerrit-ui/app/constants/reporting.ts
+++ b/polygerrit-ui/app/constants/reporting.ts
@@ -10,6 +10,7 @@
STARTED_AS_GUEST = 'Started as guest',
VISIBILILITY_HIDDEN = 'Visibility changed to hidden',
VISIBILILITY_VISIBLE = 'Visibility changed to visible',
+ FOCUS = 'Focus changed',
EXTENSION_DETECTED = 'Extension detected',
PLUGINS_INSTALLED = 'Plugins installed',
PLUGINS_FAILED = 'Some plugins failed to load',
@@ -133,4 +134,6 @@
CHANGE_ACTION_FIRED = 'change-action-fired',
BUTTON_CLICK = 'button-click',
LINK_CLICK = 'link-click',
+ USER_ACTIVE = 'user-active',
+ USER_PASSIVE = 'user-passive',
}
diff --git a/polygerrit-ui/app/elements/admin/gr-access-section/gr-access-section.ts b/polygerrit-ui/app/elements/admin/gr-access-section/gr-access-section.ts
index 4d0d3f1..e15c240 100644
--- a/polygerrit-ui/app/elements/admin/gr-access-section/gr-access-section.ts
+++ b/polygerrit-ui/app/elements/admin/gr-access-section/gr-access-section.ts
@@ -39,8 +39,6 @@
// The name that gets automatically input when a new reference is added.
const NEW_NAME = 'refs/heads/*';
const REFS_NAME = 'refs/';
-const ON_BEHALF_OF = '(On Behalf Of)';
-const LABEL = 'Label';
@customElement('gr-access-section')
export class GrAccessSection extends LitElement {
@@ -360,14 +358,14 @@
labelOptions.push({
id: 'label-' + labelName,
value: {
- name: `${LABEL} ${labelName}`,
+ name: `Label ${labelName}`,
id: 'label-' + labelName,
},
});
labelOptions.push({
id: 'labelAs-' + labelName,
value: {
- name: `${LABEL} ${labelName} ${ON_BEHALF_OF}`,
+ name: `Label ${labelName} (On Behalf Of)`,
id: 'labelAs-' + labelName,
},
});
@@ -384,11 +382,13 @@
} else if (AccessPermissions[permission.id]) {
return AccessPermissions[permission.id]?.name;
} else if (permission.value.label) {
- let behalfOf = '';
if (permission.id.startsWith('labelAs-')) {
- behalfOf = ON_BEHALF_OF;
+ return `Label ${permission.value.label} (On Behalf Of)`;
+ } else if (permission.id.startsWith('removeLabel-')) {
+ return `Remove Label ${permission.value.label}`;
+ } else {
+ return `Label ${permission.value.label}`;
}
- return `${LABEL} ${permission.value.label}${behalfOf}`;
}
return undefined;
}
diff --git a/polygerrit-ui/app/elements/admin/gr-access-section/gr-access-section_test.ts b/polygerrit-ui/app/elements/admin/gr-access-section/gr-access-section_test.ts
index 593a1ed..2c397e0 100644
--- a/polygerrit-ui/app/elements/admin/gr-access-section/gr-access-section_test.ts
+++ b/polygerrit-ui/app/elements/admin/gr-access-section/gr-access-section_test.ts
@@ -355,7 +355,20 @@
assert.equal(
element.computePermissionName(permission),
- 'Label Code-Review(On Behalf Of)'
+ 'Label Code-Review (On Behalf Of)'
+ );
+
+ permission = {
+ id: 'removeLabel-Code-Review' as GitRef,
+ value: {
+ label: 'Code-Review',
+ rules: {},
+ },
+ };
+
+ assert.equal(
+ element.computePermissionName(permission),
+ 'Remove Label Code-Review'
);
});
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 4eee3cb..e0c28bc 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
@@ -42,7 +42,6 @@
BranchName,
ChangeActionDialog,
ChangeInfo,
- ChangeViewChangeInfo,
CherryPickInput,
CommitId,
InheritedBooleanInfo,
@@ -100,7 +99,7 @@
import {changeModelToken} from '../../../models/change/change-model';
import {sharedStyles} from '../../../styles/shared-styles';
import {LitElement, PropertyValues, css, html, nothing} from 'lit';
-import {customElement, property, query, state} from 'lit/decorators.js';
+import {customElement, query, state} from 'lit/decorators.js';
import {ifDefined} from 'lit/directives/if-defined.js';
import {assertIsDefined, queryAll, uuid} from '../../../utils/common-util';
import {Interaction} from '../../../constants/reporting';
@@ -113,6 +112,9 @@
import {pluginLoaderToken} from '../../shared/gr-js-api-interface/gr-plugin-loader';
import {modalStyles} from '../../../styles/gr-modal-styles';
import {subscribe} from '../../lit/subscription-controller';
+import {userModelToken} from '../../../models/user/user-model';
+import {ParsedChangeInfo} from '../../../types/types';
+import {configModelToken} from '../../../models/config/config-model';
const ERR_BRANCH_EMPTY = 'The destination branch can’t be empty.';
const ERR_COMMIT_EMPTY = 'The commit message can’t be empty.';
@@ -377,76 +379,54 @@
RevisionActions = RevisionActions;
- @property({type: Object})
- change?: ChangeViewChangeInfo;
+ @state() change?: ParsedChangeInfo;
- @state()
- actions: ActionNameToActionInfoMap = {};
+ @state() actions: ActionNameToActionInfoMap = {};
- @property({type: Array})
- primaryActionKeys: PrimaryActionKey[] = [
+ @state() primaryActionKeys: PrimaryActionKey[] = [
ChangeActions.READY,
RevisionActions.SUBMIT,
];
- @property({type: Boolean})
- disableEdit = false;
-
- // private but used in test
@state() _hideQuickApproveAction = false;
- @property({type: Object})
- account?: AccountInfo;
+ @state() account?: AccountInfo;
- @property({type: String})
- changeNum?: NumericChangeId;
+ @state() changeNum?: NumericChangeId;
- @property({type: String})
- changeStatus?: ChangeStatus;
+ @state() changeStatus?: ChangeStatus;
- @property({type: String})
- commitNum?: CommitId;
+ @state() commitNum?: CommitId;
@state() latestPatchNum?: PatchSetNumber;
- @property({type: String})
- commitMessage = '';
+ @state() commitMessage = '';
- @property({type: Object})
- revisionActions: ActionNameToActionInfoMap = {};
+ @state() revisionActions: ActionNameToActionInfoMap = {};
- @state() private revisionSubmitAction?: ActionInfo | null;
+ @state() revisionSubmitAction?: ActionInfo | null;
- // used as a proprty type so cannot be private
@state() revisionRebaseAction?: ActionInfo | null;
- @property({type: String})
- privateByDefault?: InheritedBooleanInfo;
+ @state() privateByDefault?: InheritedBooleanInfo;
- // private but used in test
@state() loading = true;
- // private but used in test
@state() actionLoadingMessage = '';
- @state() private inProgressActionKeys = new Set<string>();
+ @state() inProgressActionKeys = new Set<string>();
- // _computeAllActions always returns an array
- // private but used in test
@state() allActionValues: UIActionInfo[] = [];
- // private but used in test
@state() topLevelActions?: UIActionInfo[];
- // private but used in test
@state() topLevelPrimaryActions?: UIActionInfo[];
- // private but used in test
@state() topLevelSecondaryActions?: UIActionInfo[];
- @state() private menuActions?: MenuAction[];
+ @state() menuActions?: MenuAction[];
- @state() private overflowActions: OverflowAction[] = [
+ @state() overflowActions: OverflowAction[] = [
{
type: ActionType.CHANGE,
key: ChangeActions.WIP,
@@ -493,29 +473,21 @@
},
];
- @state() private actionPriorityOverrides: ActionPriorityOverride[] = [];
+ @state() actionPriorityOverrides: ActionPriorityOverride[] = [];
- @state() private additionalActions: UIActionInfo[] = [];
+ @state() additionalActions: UIActionInfo[] = [];
- // private but used in test
@state() hiddenActions: string[] = [];
- // private but used in test
@state() disabledMenuActions: string[] = [];
- // private but used in test
- @state()
- editPatchsetLoaded = false;
+ @state() editPatchsetLoaded = false;
- @property({type: Boolean})
- editMode = false;
+ @state() editMode = false;
- // private but used in test
- @state()
- editBasedOnCurrentPatchSet = true;
+ @state() editBasedOnCurrentPatchSet = true;
- @property({type: Boolean})
- loggedIn = false;
+ @state() loggedIn = false;
private readonly restApiService = getAppContext().restApiService;
@@ -523,6 +495,10 @@
private readonly getPluginLoader = resolve(this, pluginLoaderToken);
+ private readonly getUserModel = resolve(this, userModelToken);
+
+ private readonly getConfigModel = resolve(this, configModelToken);
+
private readonly getChangeModel = resolve(this, changeModelToken);
private readonly getStorage = resolve(this, storageServiceToken);
@@ -546,6 +522,51 @@
() => this.getChangeModel().patchNum$,
x => (this.editPatchsetLoaded = x === 'edit')
);
+ subscribe(
+ this,
+ () => this.getChangeModel().changeNum$,
+ x => (this.changeNum = x)
+ );
+ subscribe(
+ this,
+ () => this.getChangeModel().change$,
+ x => (this.change = x)
+ );
+ subscribe(
+ this,
+ () => this.getChangeModel().status$,
+ x => (this.changeStatus = x)
+ );
+ subscribe(
+ this,
+ () => this.getChangeModel().editMode$,
+ x => (this.editMode = x)
+ );
+ subscribe(
+ this,
+ () => this.getChangeModel().revision$,
+ rev => (this.commitNum = rev?.commit?.commit)
+ );
+ subscribe(
+ this,
+ () => this.getChangeModel().latestRevision$,
+ rev => (this.commitMessage = rev?.commit?.message ?? '')
+ );
+ subscribe(
+ this,
+ () => this.getUserModel().account$,
+ x => (this.account = x)
+ );
+ subscribe(
+ this,
+ () => this.getUserModel().loggedIn$,
+ x => (this.loggedIn = x)
+ );
+ subscribe(
+ this,
+ () => this.getConfigModel().repoConfig$,
+ config => (this.privateByDefault = config?.private_by_default)
+ );
}
override connectedCallback() {
@@ -865,7 +886,7 @@
this.revisionActions = revisionActions;
this.sendShowRevisionActions({
- change,
+ change: change as ChangeInfo,
revisionActions,
});
this.handleLoadingComplete();
@@ -1031,18 +1052,7 @@
}
private editStatusChanged() {
- // Hide change edits if not logged in
- if (this.change === undefined || !this.loggedIn) {
- return;
- }
- if (this.disableEdit) {
- delete this.actions.rebaseEdit;
- delete this.actions.publishEdit;
- delete this.actions.deleteEdit;
- delete this.actions.stopEdit;
- delete this.actions.edit;
- return;
- }
+ if (!this.change || !this.loggedIn) return;
if (this.editPatchsetLoaded) {
// Only show actions that mutate an edit if an actual edit patch set
// is loaded.
@@ -1121,7 +1131,7 @@
if (!this.change || !this.change.labels || !this.change.permitted_labels) {
return null;
}
- if (this.change && this.change.status === ChangeStatus.MERGED) {
+ if (this.change?.status === ChangeStatus.MERGED) {
return null;
}
let result;
@@ -1323,18 +1333,18 @@
// private but used in test
canSubmitChange() {
- if (!this.change) {
- return false;
- }
+ if (!this.change) return false;
+ const change = this.change as ChangeInfo;
+ const revision = this.getRevision(change, this.latestPatchNum);
return this.getPluginLoader().jsApiService.canSubmitChange(
- this.change,
- this.getRevision(this.change, this.latestPatchNum)
+ change,
+ revision
);
}
// private but used in test
- getRevision(change: ChangeViewChangeInfo, patchNum?: PatchSetNumber) {
- for (const rev of Object.values(change.revisions)) {
+ getRevision(change: ChangeInfo, patchNum?: PatchSetNumber) {
+ for (const rev of Object.values(change.revisions ?? {})) {
if (rev._number === patchNum) {
return rev;
}
@@ -1805,7 +1815,7 @@
if (dialog.init) dialog.init();
dialog.hidden = false;
assertIsDefined(this.actionsModal, 'actionsModal');
- this.actionsModal.showModal();
+ if (this.actionsModal.isConnected) this.actionsModal.showModal();
whenVisible(dialog, () => {
if (dialog.resetFocus) {
dialog.resetFocus();
@@ -1818,7 +1828,7 @@
// private but used in test
setReviewOnRevert(newChangeId: NumericChangeId) {
const review = this.getPluginLoader().jsApiService.getReviewPostRevert(
- this.change
+ this.change as ChangeInfo
);
if (!review) {
return Promise.resolve(undefined);
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.ts b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.ts
index 946191b..b628cc1 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.ts
@@ -129,6 +129,7 @@
element = await fixture<GrChangeActions>(html`
<gr-change-actions></gr-change-actions>
`);
+ element.changeStatus = ChangeStatus.NEW;
element.change = {
...createChangeViewChange(),
actions: {
@@ -155,7 +156,7 @@
await element.reload();
});
- test('render', () => {
+ test('render', async () => {
assert.shadowDom.equal(
element,
/* HTML */ `
@@ -200,6 +201,24 @@
Rebase
</gr-button>
</gr-tooltip-content>
+ <gr-tooltip-content
+ has-tooltip=""
+ position-below=""
+ title="Edit this change"
+ >
+ <gr-button
+ aria-disabled="false"
+ class="edit"
+ data-action-key="edit"
+ data-label="Edit"
+ link=""
+ role="button"
+ tabindex="0"
+ >
+ <gr-icon filled="" icon="edit"> </gr-icon>
+ Edit
+ </gr-button>
+ </gr-tooltip-content>
</section>
<gr-button
aria-disabled="false"
@@ -705,29 +724,6 @@
});
suite('change edits', () => {
- test('disableEdit', async () => {
- element.editMode = false;
- element.editBasedOnCurrentPatchSet = false;
- element.change = {
- ...createChangeViewChange(),
- status: ChangeStatus.NEW,
- };
- element.disableEdit = true;
- await element.updateComplete;
-
- assert.isNotOk(
- query(element, 'gr-button[data-action-key="publishEdit"]')
- );
- assert.isNotOk(
- query(element, 'gr-button[data-action-key="rebaseEdit"]')
- );
- assert.isNotOk(
- query(element, 'gr-button[data-action-key="deleteEdit"]')
- );
- assert.isNotOk(query(element, 'gr-button[data-action-key="edit"]'));
- assert.isNotOk(query(element, 'gr-button[data-action-key="stopEdit"]'));
- });
-
test('shows confirm dialog for delete edit', async () => {
element.loggedIn = true;
element.editMode = true;
diff --git a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.ts b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.ts
index b7851a6..1cb25ea 100644
--- a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.ts
@@ -82,6 +82,12 @@
import {createChangeUrl} from '../../../models/views/change';
import {getChangeWeblinks} from '../../../utils/weblink-util';
import {throwingErrorCallback} from '../../shared/gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper';
+import {subscribe} from '../../lit/subscription-controller';
+import {userModelToken} from '../../../models/user/user-model';
+import {resolve} from '../../../models/dependency';
+import {configModelToken} from '../../../models/config/config-model';
+import {changeModelToken} from '../../../models/change/change-model';
+import {relatedChangesModelToken} from '../../../models/change/related-changes-model';
const HASHTAG_ADD_MESSAGE = 'Add Hashtag';
@@ -118,54 +124,90 @@
export class GrChangeMetadata extends LitElement {
@query('#webLinks') webLinks?: HTMLElement;
- @property({type: Object}) change?: ParsedChangeInfo;
-
- @property({type: Object}) revertedChange?: ChangeInfo;
-
- @property({type: Object}) account?: AccountDetailInfo;
-
- @property({type: Object}) revision?: RevisionInfo | EditRevisionInfo;
-
- // TODO: Just use `revision.commit` instead.
- @property({type: Object}) commitInfo?: CommitInfoWithRequiredCommit;
-
- @property({type: Object}) serverConfig?: ServerInfo;
-
+ // TODO: Convert to @state. That requires the change model to keep track of
+ // current revision actions. Then we can also get rid of the
+ // `revision-actions-changed` event.
@property({type: Boolean}) parentIsCurrent?: boolean;
- @property({type: Object}) repoConfig?: ConfigInfo;
+ @state() change?: ParsedChangeInfo;
- // private but used in test
+ @state() revertedChange?: ChangeInfo;
+
+ @state() account?: AccountDetailInfo;
+
+ @state() revision?: RevisionInfo | EditRevisionInfo;
+
+ @state() serverConfig?: ServerInfo;
+
+ @state() repoConfig?: ConfigInfo;
+
@state() mutable = false;
- @state() private readonly notCurrentMessage = NOT_CURRENT_MESSAGE;
+ @state() readonly notCurrentMessage = NOT_CURRENT_MESSAGE;
- // private but used in test
@state() topicReadOnly = true;
- // private but used in test
@state() hashtagReadOnly = true;
- @state() private pushCertificateValidation?: PushCertificateValidationInfo;
+ @state() pushCertificateValidation?: PushCertificateValidationInfo;
- // private but used in test
@state() settingTopic = false;
- // private but used in test
@state() currentParents: ParentCommitInfo[] = [];
- @state() private showAllSections = false;
+ @state() showAllSections = false;
- @state() private queryTopic?: AutocompleteQuery;
+ @state() queryTopic?: AutocompleteQuery;
- @state() private queryHashtag?: AutocompleteQuery;
+ @state() queryHashtag?: AutocompleteQuery;
private restApiService = getAppContext().restApiService;
private readonly reporting = getAppContext().reportingService;
+ private readonly getUserModel = resolve(this, userModelToken);
+
+ private readonly getConfigModel = resolve(this, configModelToken);
+
+ private readonly getChangeModel = resolve(this, changeModelToken);
+
+ private readonly getRelatedChangesModel = resolve(
+ this,
+ relatedChangesModelToken
+ );
+
constructor() {
super();
+ subscribe(
+ this,
+ () => this.getConfigModel().serverConfig$,
+ serverConfig => (this.serverConfig = serverConfig)
+ );
+ subscribe(
+ this,
+ () => this.getConfigModel().repoConfig$,
+ repoConfig => (this.repoConfig = repoConfig)
+ );
+ subscribe(
+ this,
+ () => this.getUserModel().account$,
+ account => (this.account = account)
+ );
+ subscribe(
+ this,
+ () => this.getChangeModel().change$,
+ change => (this.change = change)
+ );
+ subscribe(
+ this,
+ () => this.getChangeModel().revision$,
+ revision => (this.revision = revision)
+ );
+ subscribe(
+ this,
+ () => this.getRelatedChangesModel().revertingChange$,
+ revertingChange => (this.revertedChange = revertingChange)
+ );
this.queryTopic = (input: string) => this.getTopicSuggestions(input);
this.queryHashtag = (input: string) => this.getHashtagSuggestions(input);
}
@@ -735,7 +777,10 @@
// private but used in test
computeWebLinks(): WebLinkInfo[] {
- return getChangeWeblinks(this.commitInfo?.web_links, this.serverConfig);
+ return getChangeWeblinks(
+ this.revision?.commit?.web_links,
+ this.serverConfig
+ );
}
private computeStrategy() {
diff --git a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_test.ts b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_test.ts
index c46fe24..e0e1364 100644
--- a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_test.ts
@@ -9,7 +9,6 @@
import {ChangeRole, GrChangeMetadata} from './gr-change-metadata';
import {
createServerInfo,
- createUserConfig,
createParsedChange,
createAccountWithId,
createCommitInfoWithRequiredCommit,
@@ -61,18 +60,9 @@
let element: GrChangeMetadata;
setup(async () => {
- stubRestApi('getLoggedIn').returns(Promise.resolve(false));
- stubRestApi('getConfig').returns(
- Promise.resolve({
- ...createServerInfo(),
- user: {
- ...createUserConfig(),
- anonymouscowardname: 'test coward name',
- },
- })
- );
element = await fixture(html`<gr-change-metadata></gr-change-metadata>`);
element.change = createParsedChange();
+ element.account = undefined;
await element.updateComplete;
});
@@ -251,16 +241,22 @@
});
test('weblinks hidden when no weblinks', async () => {
- element.commitInfo = createCommitInfoWithRequiredCommit();
+ element.revision = {
+ ...createRevision(),
+ commit: createCommitInfoWithRequiredCommit(),
+ };
element.serverConfig = createServerInfo();
await element.updateComplete;
assert.isNull(element.webLinks);
});
test('weblinks hidden when only gitiles weblink', async () => {
- element.commitInfo = {
- ...createCommitInfoWithRequiredCommit(),
- web_links: [{...createWebLinkInfo(), name: 'gitiles', url: '#'}],
+ element.revision = {
+ ...createRevision(),
+ commit: {
+ ...createCommitInfoWithRequiredCommit(),
+ web_links: [{...createWebLinkInfo(), name: 'gitiles', url: '#'}],
+ },
};
element.serverConfig = createServerInfo();
await element.updateComplete;
@@ -270,9 +266,12 @@
test('weblinks hidden when sole weblink is set as primary', async () => {
const browser = 'browser';
- element.commitInfo = {
- ...createCommitInfoWithRequiredCommit(),
- web_links: [{...createWebLinkInfo(), name: browser, url: '#'}],
+ element.revision = {
+ ...createRevision(),
+ commit: {
+ ...createCommitInfoWithRequiredCommit(),
+ web_links: [{...createWebLinkInfo(), name: browser, url: '#'}],
+ },
};
element.serverConfig = {
...createServerInfo(),
@@ -286,9 +285,12 @@
});
test('weblinks are visible when other weblinks', async () => {
- element.commitInfo = {
- ...createCommitInfoWithRequiredCommit(),
- web_links: [{...createWebLinkInfo(), name: 'test', url: '#'}],
+ element.revision = {
+ ...createRevision(),
+ commit: {
+ ...createCommitInfoWithRequiredCommit(),
+ web_links: [{...createWebLinkInfo(), name: 'test', url: '#'}],
+ },
};
await element.updateComplete;
const webLinks = element.webLinks!;
@@ -297,12 +299,15 @@
});
test('weblinks are visible when gitiles and other weblinks', async () => {
- element.commitInfo = {
- ...createCommitInfoWithRequiredCommit(),
- web_links: [
- {...createWebLinkInfo(), name: 'test', url: '#'},
- {...createWebLinkInfo(), name: 'gitiles', url: '#'},
- ],
+ element.revision = {
+ ...createRevision(),
+ commit: {
+ ...createCommitInfoWithRequiredCommit(),
+ web_links: [
+ {...createWebLinkInfo(), name: 'test', url: '#'},
+ {...createWebLinkInfo(), name: 'gitiles', url: '#'},
+ ],
+ },
};
await element.updateComplete;
const webLinks = element.webLinks!;
diff --git a/polygerrit-ui/app/elements/change/gr-change-summary/gr-change-summary.ts b/polygerrit-ui/app/elements/change/gr-change-summary/gr-change-summary.ts
index 531994c90..f6a40a2 100644
--- a/polygerrit-ui/app/elements/change/gr-change-summary/gr-change-summary.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-summary/gr-change-summary.ts
@@ -63,6 +63,9 @@
@customElement('gr-change-summary')
export class GrChangeSummary extends LitElement {
@state()
+ commentsLoading = true;
+
+ @state()
commentThreads?: CommentThread[];
@state()
@@ -161,6 +164,11 @@
);
subscribe(
this,
+ () => this.getCommentsModel().commentsLoading$,
+ x => (this.commentsLoading = x)
+ );
+ subscribe(
+ this,
() =>
combineLatest([
this.getUserModel().account$,
@@ -530,6 +538,10 @@
<tr>
<td class="key">Comments</td>
<td class="value">
+ ${when(
+ this.commentsLoading,
+ () => html`<span class="loadingSpin"></span>`
+ )}
<gr-comments-summary
.commentThreads=${this.commentThreads}
.draftCount=${this.draftCount}
diff --git a/polygerrit-ui/app/elements/change/gr-change-summary/gr-change-summary_test.ts b/polygerrit-ui/app/elements/change/gr-change-summary/gr-change-summary_test.ts
index 3cb63f9..c3d9774 100644
--- a/polygerrit-ui/app/elements/change/gr-change-summary/gr-change-summary_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-summary/gr-change-summary_test.ts
@@ -50,6 +50,7 @@
},
discardedDrafts: [],
});
+ element.commentsLoading = false;
element.commentThreads = [
createCommentThread([createComment()]),
createCommentThread([{...createComment(), unresolved: true}]),
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 a64dcd2..02fdb34 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
@@ -335,10 +335,7 @@
@state()
private updateCheckTimerHandle?: number | null;
- // Private but used in tests.
- getEditMode(): boolean {
- return !!this.viewState?.edit || this.patchNum === EDIT;
- }
+ @state() editMode = false;
isSubmitEnabled(): boolean {
return !!(
@@ -664,6 +661,11 @@
);
subscribe(
this,
+ () => this.getChangeModel().editMode$,
+ editMode => (this.editMode = editMode)
+ );
+ subscribe(
+ this,
() => this.getChangeModel().patchNum$,
patchNum => (this.patchNum = patchNum)
);
@@ -1134,23 +1136,16 @@
${this.renderTabHeaders()} ${this.renderTabContent()}
${this.renderChangeLog()}
</div>
- <gr-apply-fix-dialog
- id="applyFixDialog"
- .change=${this.change}
- .changeNum=${this.changeNum}
- ></gr-apply-fix-dialog>
+ <gr-apply-fix-dialog id="applyFixDialog"></gr-apply-fix-dialog>
<dialog id="downloadModal" tabindex="-1">
<gr-download-dialog
id="downloadDialog"
- .change=${this.change}
- .config=${this.serverConfig?.download}
@close=${this.handleDownloadDialogClose}
></gr-download-dialog>
</dialog>
<dialog id="includedInModal" tabindex="-1">
<gr-included-in-dialog
id="includedInDialog"
- .changeNum=${this.changeNum}
@close=${this.handleIncludedInDialogClose}
></gr-included-in-dialog>
</dialog>
@@ -1287,27 +1282,18 @@
}
private renderCommitActions() {
- return html` <div class="commitActions">
- <!-- always show gr-change-actions regardless if logged in or not -->
- <gr-change-actions
- id="actions"
- .change=${this.change}
- .disableEdit=${false}
- .account=${this.account}
- .changeNum=${this.changeNum}
- .changeStatus=${this.change?.status}
- .commitNum=${this.revision?.commit?.commit}
- .commitMessage=${this.latestCommitMessage}
- .editMode=${this.getEditMode()}
- .privateByDefault=${this.projectConfig?.private_by_default}
- .loggedIn=${this.loggedIn}
- @edit-tap=${() => this.handleEditTap()}
- @stop-edit-tap=${() => this.handleStopEditTap()}
- @download-tap=${() => this.handleOpenDownloadDialog()}
- @included-tap=${() => this.handleOpenIncludedInDialog()}
- @revision-actions-changed=${this.handleRevisionActionsChanged}
- ></gr-change-actions>
- </div>`;
+ return html`
+ <div class="commitActions">
+ <gr-change-actions
+ id="actions"
+ @edit-tap=${() => this.handleEditTap()}
+ @stop-edit-tap=${() => this.handleStopEditTap()}
+ @download-tap=${() => this.handleOpenDownloadDialog()}
+ @included-tap=${() => this.handleOpenIncludedInDialog()}
+ @revision-actions-changed=${this.handleRevisionActionsChanged}
+ ></gr-change-actions>
+ </div>
+ `;
}
private renderChangeInfo() {
@@ -1315,20 +1301,13 @@
this.loggedIn,
this.editingCommitMessage,
this.change,
- this.getEditMode()
+ this.editMode
);
return html` <div class="changeInfo">
<div class="changeInfo-column changeMetadata">
<gr-change-metadata
id="metadata"
- .change=${this.change}
- .revertedChange=${this.revertingChange}
- .account=${this.account}
- .revision=${this.revision}
- .commitInfo=${this.revision?.commit}
- .serverConfig=${this.serverConfig}
.parentIsCurrent=${this.isParentCurrent()}
- .repoConfig=${this.projectConfig}
@show-reply-dialog=${this.handleShowReplyDialog}
>
</gr-change-metadata>
@@ -1419,7 +1398,7 @@
)}
${this.pluginTabsHeaderEndpoints.map(
tabHeader => html`
- <paper-tab data-name=${tabHeader}>
+ <paper-tab data-name=${tabHeader} @click=${this.onPaperTabClick}>
<gr-endpoint-decorator name=${tabHeader}>
<gr-endpoint-param name="change" .value=${this.change}>
</gr-endpoint-param>
@@ -1461,7 +1440,7 @@
.changeNum=${this.changeNum}
.commitInfo=${this.revision?.commit}
.changeUrl=${this.computeChangeUrl()}
- .editMode=${this.getEditMode()}
+ .editMode=${this.editMode}
.loggedIn=${this.loggedIn}
.shownFileCount=${this.shownFileCount}
.filesExpanded=${this.fileList?.filesExpanded}
@@ -1475,7 +1454,7 @@
id="fileList"
.change=${this.change}
.changeNum=${this.changeNum}
- .editMode=${this.getEditMode()}
+ .editMode=${this.editMode}
@files-shown-changed=${(e: CustomEvent<{length: number}>) => {
this.shownFileCount = e.detail.length;
}}
@@ -1727,7 +1706,6 @@
const options = {
mergeable: this.mergeable,
- submitEnabled: !!this.isSubmitEnabled(),
revertingChangeStatus: this.revertingChange?.status,
};
return changeStatuses(this.change as ChangeInfo, options);
@@ -1957,7 +1935,7 @@
change: this.change,
patchNum: this.patchNum,
basePatchNum: this.basePatchNum,
- edit: this.getEditMode(),
+ edit: this.editMode,
messageHash: hash,
});
history.replaceState(null, '', url);
@@ -1967,7 +1945,7 @@
async maybeScrollToMessage(hash: string) {
if (hash.startsWith(PREFIX)) {
await waitUntil(() => !!this.messagesList);
- this.messagesList!.scrollToMessage(hash.substr(PREFIX.length));
+ await this.messagesList!.scrollToMessage(hash.substr(PREFIX.length));
}
}
@@ -2405,7 +2383,7 @@
// Private but used in tests.
computeHeaderClass() {
const classes = ['header'];
- if (this.getEditMode()) {
+ if (this.editMode) {
classes.push('editMode');
}
return classes.join(' ');
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 5331558..ae60449 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
@@ -73,7 +73,7 @@
import {Modifier} from '../../../utils/dom-util';
import {GrButton} from '../../shared/gr-button/gr-button';
import {GrCopyLinks} from '../gr-copy-links/gr-copy-links';
-import {ChangeChildView, ChangeViewState} from '../../../models/views/change';
+import {ChangeChildView} from '../../../models/views/change';
import {rootUrl} from '../../../utils/url-util';
import {testResolver} from '../../../test/common-test-setup';
import {UserModel, userModelToken} from '../../../models/user/user-model';
@@ -1318,10 +1318,9 @@
});
});
- test('header class computation', () => {
+ test('header class computation', async () => {
assert.equal(element.computeHeaderClass(), 'header');
- assertIsDefined(element.viewState);
- element.viewState.edit = true;
+ element.editMode = true;
assert.equal(element.computeHeaderClass(), 'header editMode');
});
@@ -1342,38 +1341,6 @@
assert.equal(scrollStub.lastCall.args[0], 'TEST');
});
- test('computeEditMode', async () => {
- const callCompute = async (viewState: ChangeViewState) => {
- element.viewState = viewState;
- element.patchNum = viewState.patchNum;
- element.basePatchNum = viewState.basePatchNum ?? PARENT;
- await element.updateComplete;
- return element.getEditMode();
- };
- assert.isTrue(
- await callCompute({
- ...createChangeViewState(),
- edit: true,
- basePatchNum: PARENT,
- patchNum: 1 as RevisionPatchSetNum,
- })
- );
- assert.isFalse(
- await callCompute({
- ...createChangeViewState(),
- basePatchNum: PARENT,
- patchNum: 1 as RevisionPatchSetNum,
- })
- );
- assert.isTrue(
- await callCompute({
- ...createChangeViewState(),
- basePatchNum: 1 as BasePatchSetNum,
- patchNum: EDIT,
- })
- );
- });
-
test('file-action-tap handling', async () => {
element.patchNum = 1 as RevisionPatchSetNum;
element.change = {
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog.ts b/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog.ts
index fedc377..e421c9c 100644
--- a/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog.ts
+++ b/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog.ts
@@ -15,6 +15,7 @@
import {resolve} from '../../../models/dependency';
import {pluginLoaderToken} from '../../shared/gr-js-api-interface/gr-plugin-loader';
import {createSearchUrl} from '../../../models/views/search';
+import {ParsedChangeInfo} from '../../../types/types';
const ERR_COMMIT_NOT_FOUND = 'Unable to find the commit hash of this change.';
const INSERT_REASON_STRING = '<INSERT REASONING HERE>';
@@ -170,15 +171,23 @@
return this.revertType === RevertType.REVERT_SUBMISSION;
}
- modifyRevertMsg(change: ChangeInfo, commitMessage: string, message: string) {
+ modifyRevertMsg(
+ change: ParsedChangeInfo,
+ commitMessage: string,
+ message: string
+ ) {
return this.getPluginLoader().jsApiService.modifyRevertMsg(
- change,
+ change as ChangeInfo,
message,
commitMessage
);
}
- populate(change: ChangeInfo, commitMessage: string, changesCount: number) {
+ populate(
+ change: ParsedChangeInfo,
+ commitMessage: string,
+ changesCount: number
+ ) {
this.changesCount = changesCount;
// The option to revert a single change is always available
this.populateRevertSingleChangeMessage(
@@ -190,7 +199,7 @@
}
populateRevertSingleChangeMessage(
- change: ChangeInfo,
+ change: ParsedChangeInfo,
commitMessage: string,
commitHash?: CommitId
) {
@@ -215,18 +224,21 @@
}
private modifyRevertSubmissionMsg(
- change: ChangeInfo,
+ change: ParsedChangeInfo,
msg: string,
commitMessage: string
) {
return this.getPluginLoader().jsApiService.modifyRevertSubmissionMsg(
- change,
+ change as ChangeInfo,
msg,
commitMessage
);
}
- populateRevertSubmissionMessage(change: ChangeInfo, commitMessage: string) {
+ populateRevertSubmissionMessage(
+ change: ParsedChangeInfo,
+ commitMessage: string
+ ) {
// Follow the same convention of the revert
const commitHash = change.current_revision;
if (!commitHash) {
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog_test.ts b/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog_test.ts
index 904285f..8d71e15 100644
--- a/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog_test.ts
@@ -5,7 +5,7 @@
*/
import {fixture, html, assert} from '@open-wc/testing';
import '../../../test/common-test-setup';
-import {createChange} from '../../../test/test-data-generators';
+import {createParsedChange} from '../../../test/test-data-generators';
import {ChangeSubmissionId, CommitId} from '../../../types/common';
import './gr-confirm-revert-dialog';
import {GrConfirmRevertDialog} from './gr-confirm-revert-dialog';
@@ -48,7 +48,7 @@
const alertStub = sinon.stub();
element.addEventListener('show-alert', alertStub);
element.populateRevertSingleChangeMessage(
- createChange(),
+ createParsedChange(),
'not a commitHash in sight',
undefined
);
@@ -58,7 +58,7 @@
test('single line', () => {
assert.isNotOk(element.message);
element.populateRevertSingleChangeMessage(
- createChange(),
+ createParsedChange(),
'one line commit\n\nChange-Id: abcdefg\n',
'abcd123' as CommitId
);
@@ -72,7 +72,7 @@
test('multi line', () => {
assert.isNotOk(element.message);
element.populateRevertSingleChangeMessage(
- createChange(),
+ createParsedChange(),
'many lines\ncommit\n\nmessage\n\nChange-Id: abcdefg\n',
'abcd123' as CommitId
);
@@ -86,7 +86,7 @@
test('issue above change id', () => {
assert.isNotOk(element.message);
element.populateRevertSingleChangeMessage(
- createChange(),
+ createParsedChange(),
'much lines\nvery\n\ncommit\n\nBug: Issue 42\nChange-Id: abcdefg\n',
'abcd123' as CommitId
);
@@ -100,7 +100,7 @@
test('revert a revert', () => {
assert.isNotOk(element.message);
element.populateRevertSingleChangeMessage(
- createChange(),
+ createParsedChange(),
'Revert "one line commit"\n\nChange-Id: abcdefg\n',
'abcd123' as CommitId
);
@@ -115,7 +115,7 @@
element.changesCount = 3;
element.populateRevertSubmissionMessage(
{
- ...createChange(),
+ ...createParsedChange(),
submission_id: '5545' as ChangeSubmissionId,
current_revision: 'abcd123' as CommitId,
},
diff --git a/polygerrit-ui/app/elements/change/gr-download-dialog/gr-download-dialog.ts b/polygerrit-ui/app/elements/change/gr-download-dialog/gr-download-dialog.ts
index 11dc890..e1808bf 100644
--- a/polygerrit-ui/app/elements/change/gr-download-dialog/gr-download-dialog.ts
+++ b/polygerrit-ui/app/elements/change/gr-download-dialog/gr-download-dialog.ts
@@ -5,7 +5,7 @@
*/
import '../../shared/gr-download-commands/gr-download-commands';
import {changeBaseURL, getRevisionKey} from '../../../utils/change-util';
-import {ChangeInfo, DownloadInfo, PatchSetNum} from '../../../types/common';
+import {DownloadInfo, PatchSetNum} from '../../../types/common';
import {GrDownloadCommands} from '../../shared/gr-download-commands/gr-download-commands';
import {GrButton} from '../../shared/gr-button/gr-button';
import {copyToClipbard, hasOwnProperty} from '../../../utils/common-util';
@@ -13,13 +13,15 @@
import {fontStyles} from '../../../styles/gr-font-styles';
import {sharedStyles} from '../../../styles/shared-styles';
import {LitElement, PropertyValues, html, css} from 'lit';
-import {customElement, property, state, query} from 'lit/decorators.js';
+import {customElement, state, query} from 'lit/decorators.js';
import {assertIsDefined} from '../../../utils/common-util';
import {BindValueChangeEvent} from '../../../types/events';
import {ShortcutController} from '../../lit/shortcut-controller';
import {subscribe} from '../../lit/subscription-controller';
import {resolve} from '../../../models/dependency';
import {changeModelToken} from '../../../models/change/change-model';
+import {ParsedChangeInfo} from '../../../types/types';
+import {configModelToken} from '../../../models/config/config-model';
@customElement('gr-download-dialog')
export class GrDownloadDialog extends LitElement {
@@ -35,27 +37,37 @@
@query('#closeButton') protected closeButton?: GrButton;
- @property({type: Object})
- change: ChangeInfo | undefined;
+ @state() change?: ParsedChangeInfo;
- @property({type: Object})
- config?: DownloadInfo;
+ @state() config?: DownloadInfo;
@state() patchNum?: PatchSetNum;
- @state() private selectedScheme?: string;
+ @state() selectedScheme?: string;
private readonly shortcuts = new ShortcutController(this);
private readonly getChangeModel = resolve(this, changeModelToken);
+ private readonly getConfigModel = resolve(this, configModelToken);
+
constructor() {
super();
subscribe(
this,
+ () => this.getChangeModel().change$,
+ x => (this.change = x)
+ );
+ subscribe(
+ this,
() => this.getChangeModel().patchNum$,
x => (this.patchNum = x)
);
+ subscribe(
+ this,
+ () => this.getConfigModel().download$,
+ x => (this.config = x)
+ );
for (const key of ['1', '2', '3', '4', '5']) {
this.shortcuts.addLocal({key}, e => this.handleNumberKey(e));
}
diff --git a/polygerrit-ui/app/elements/change/gr-download-dialog/gr-download-dialog_test.ts b/polygerrit-ui/app/elements/change/gr-download-dialog/gr-download-dialog_test.ts
index e5f40a6..73d7618 100644
--- a/polygerrit-ui/app/elements/change/gr-download-dialog/gr-download-dialog_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-download-dialog/gr-download-dialog_test.ts
@@ -8,6 +8,7 @@
createChange,
createCommit,
createDownloadInfo,
+ createParsedChange,
createRevision,
} from '../../../test/test-data-generators';
import {
@@ -160,7 +161,7 @@
suite('gr-download-dialog tests with no fetch options', () => {
setup(async () => {
element.change = {
- ...createChange(),
+ ...createParsedChange(),
revisions: {
r1: {
...createRevision(),
@@ -204,7 +205,7 @@
test('computed fields', () => {
element.change = {
- ...createChange(),
+ ...createParsedChange(),
project: 'test/project' as RepoName,
_number: 123 as NumericChangeId,
};
@@ -233,7 +234,7 @@
element.patchNum = 1 as PatchSetNum;
element.change = {
- ...createChange(),
+ ...createParsedChange(),
revisions: {
r1: {...createRevision(), commit: createCommit()},
},
@@ -241,7 +242,7 @@
assert.isTrue(element.computeHidePatchFile());
element.change = {
- ...createChange(),
+ ...createParsedChange(),
revisions: {
r1: {
...createRevision(),
@@ -255,7 +256,7 @@
assert.isFalse(element.computeHidePatchFile());
element.change = {
- ...createChange(),
+ ...createParsedChange(),
revisions: {
r1: {
...createRevision(),
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
index 62693ea..9665b03 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
@@ -86,7 +86,6 @@
import {userModelToken} from '../../../models/user/user-model';
import {pluginLoaderToken} from '../../shared/gr-js-api-interface/gr-plugin-loader';
import {FileMode, fileModeToString} from '../../../utils/file-util';
-import {KnownExperimentId} from '../../../services/flags/flags';
export const DEFAULT_NUM_FILES_SHOWN = 200;
@@ -309,8 +308,6 @@
private readonly getBrowserModel = resolve(this, browserModelToken);
- private readonly flagsService = getAppContext().flagsService;
-
shortcutsController = new ShortcutController(this);
private readonly getNavigation = resolve(this, navigationToken);
@@ -1286,16 +1283,7 @@
private renderFileComments(file: NormalizedFileInfo) {
return html` <div role="gridcell">
<div class="comments desktop">
- ${when(
- this.flagsService.isEnabled(
- KnownExperimentId.COMMENTS_CHIPS_IN_FILE_LIST
- ),
- () => html`<span>${this.renderCommentsChips(file)}</span>`,
- () => html`<span class="drafts"
- >${this.computeDraftsString(file)}</span
- >
- <span>${this.computeCommentsString(file)}</span>`
- )}
+ <span>${this.renderCommentsChips(file)}</span>
<span class="noCommentsScreenReaderText">
<!-- Screen readers read the following content only if 2 other
spans in the parent div is empty. The content is not visible on
@@ -1824,37 +1812,6 @@
}
/**
- * Computes a string with the number of comments and unresolved comments.
- */
- computeCommentsString(file?: NormalizedFileInfo) {
- if (
- this.changeComments === undefined ||
- this.patchRange === undefined ||
- file?.__path === undefined
- ) {
- return '';
- }
- return this.changeComments.computeCommentsString(
- this.patchRange,
- file.__path,
- file
- );
- }
-
- /**
- * Computes a string with the number of drafts.
- */
- computeDraftsString(file?: NormalizedFileInfo) {
- if (this.changeComments === undefined) return '';
- const draftCount = this.changeComments.computeDraftCountForFile(
- this.patchRange,
- file
- );
- if (draftCount === 0) return '';
- return pluralize(Number(draftCount), 'draft');
- }
-
- /**
* Computes a shortened string with the number of drafts.
* Private but used in tests.
*/
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.ts b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.ts
index 8fb5622..939ad06 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.ts
@@ -192,7 +192,11 @@
</span>
<div role="gridcell">
<div class="comments desktop">
- <span class="drafts"> </span> <span> </span>
+ <span
+ ><gr-comments-summary
+ emptywhennocomments=""
+ ></gr-comments-summary
+ ></span>
<span class="noCommentsScreenReaderText"> No comments </span>
</div>
<div class="comments mobile">
@@ -719,28 +723,6 @@
element.basePatchNum = PARENT;
element.patchNum = 1 as RevisionPatchSetNum;
assert.equal(
- element.computeDraftsString({
- __path: 'unresolved.file',
- size: 0,
- size_delta: 0,
- }),
- '1 draft'
- );
-
- element.basePatchNum = 1 as BasePatchSetNum;
- element.patchNum = 2 as RevisionPatchSetNum;
- assert.equal(
- element.computeDraftsString({
- __path: 'unresolved.file',
- size: 0,
- size_delta: 0,
- }),
- '1 draft'
- );
-
- element.basePatchNum = PARENT;
- element.patchNum = 1 as RevisionPatchSetNum;
- assert.equal(
element.computeDraftsStringMobile({
__path: 'unresolved.file',
size: 0,
@@ -785,28 +767,6 @@
element.basePatchNum = PARENT;
element.patchNum = 1 as RevisionPatchSetNum;
assert.equal(
- element.computeDraftsString({
- __path: 'myfile.txt',
- size: 0,
- size_delta: 0,
- }),
- ''
- );
-
- element.basePatchNum = 1 as BasePatchSetNum;
- element.patchNum = 2 as RevisionPatchSetNum;
- assert.equal(
- element.computeDraftsString({
- __path: 'myfile.txt',
- size: 0,
- size_delta: 0,
- }),
- ''
- );
-
- element.basePatchNum = PARENT;
- element.patchNum = 1 as RevisionPatchSetNum;
- assert.equal(
element.computeDraftsStringMobile({
__path: 'myfile.txt',
size: 0,
@@ -851,28 +811,6 @@
element.basePatchNum = PARENT;
element.patchNum = 1 as RevisionPatchSetNum;
assert.equal(
- element.computeDraftsString({
- __path: 'file_added_in_rev2.txt',
- size: 0,
- size_delta: 0,
- }),
- ''
- );
-
- element.basePatchNum = 1 as BasePatchSetNum;
- element.patchNum = 2 as RevisionPatchSetNum;
- assert.equal(
- element.computeDraftsString({
- __path: 'file_added_in_rev2.txt',
- size: 0,
- size_delta: 0,
- }),
- ''
- );
-
- element.basePatchNum = PARENT;
- element.patchNum = 1 as RevisionPatchSetNum;
- assert.equal(
element.computeDraftsStringMobile({
__path: 'file_added_in_rev2.txt',
size: 0,
@@ -917,28 +855,6 @@
element.basePatchNum = PARENT;
element.patchNum = 1 as RevisionPatchSetNum;
assert.equal(
- element.computeDraftsString({
- __path: '/COMMIT_MSG',
- size: 0,
- size_delta: 0,
- }),
- '2 drafts'
- );
-
- element.basePatchNum = 1 as BasePatchSetNum;
- element.patchNum = 2 as RevisionPatchSetNum;
- assert.equal(
- element.computeDraftsString({
- __path: '/COMMIT_MSG',
- size: 0,
- size_delta: 0,
- }),
- '2 drafts'
- );
-
- element.basePatchNum = PARENT;
- element.patchNum = 1 as RevisionPatchSetNum;
- assert.equal(
element.computeDraftsStringMobile({
__path: '/COMMIT_MSG',
size: 0,
diff --git a/polygerrit-ui/app/elements/change/gr-included-in-dialog/gr-included-in-dialog.ts b/polygerrit-ui/app/elements/change/gr-included-in-dialog/gr-included-in-dialog.ts
index 33dfe82..32da400 100644
--- a/polygerrit-ui/app/elements/change/gr-included-in-dialog/gr-included-in-dialog.ts
+++ b/polygerrit-ui/app/elements/change/gr-included-in-dialog/gr-included-in-dialog.ts
@@ -10,9 +10,12 @@
import {fontStyles} from '../../../styles/gr-font-styles';
import {sharedStyles} from '../../../styles/shared-styles';
import {LitElement, PropertyValues, html, css} from 'lit';
-import {customElement, property, state} from 'lit/decorators.js';
+import {customElement, state} from 'lit/decorators.js';
import {BindValueChangeEvent} from '../../../types/events';
import {fireNoBubble} from '../../../utils/event-util';
+import {resolve} from '../../../models/dependency';
+import {changeModelToken} from '../../../models/change/change-model';
+import {subscribe} from '../../lit/subscription-controller';
interface DisplayGroup {
title: string;
@@ -27,19 +30,18 @@
* @event close
*/
- @property({type: Object})
- changeNum?: NumericChangeId;
+ @state() changeNum?: NumericChangeId;
- // private but used in test
@state() includedIn?: IncludedInInfo;
@state() private loaded = false;
- // private but used in test
@state() filterText = '';
private readonly restApiService = getAppContext().restApiService;
+ private readonly getChangeModel = resolve(this, changeModelToken);
+
static override get styles() {
return [
fontStyles,
@@ -93,6 +95,15 @@
];
}
+ constructor() {
+ super();
+ subscribe(
+ this,
+ () => this.getChangeModel().changeNum$,
+ changeNum => (this.changeNum = changeNum)
+ );
+ }
+
override render() {
return html`
<header>
diff --git a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.ts b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.ts
index ac087cef..d5da9c9 100644
--- a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.ts
+++ b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.ts
@@ -440,7 +440,7 @@
}
async scrollToMessage(messageID: string) {
- await waitUntil(() => this.messages.length > 0);
+ await waitUntil(() => this.messages && this.messages.length > 0);
await this.updateComplete;
const selector = `[data-message-id="${messageID}"]`;
diff --git a/polygerrit-ui/app/elements/core/gr-router/gr-page.ts b/polygerrit-ui/app/elements/core/gr-router/gr-page.ts
index 1d2a272..264c6e0 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-page.ts
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-page.ts
@@ -4,6 +4,8 @@
* SPDX-License-Identifier: Apache-2.0
*/
+import {sameOrigin} from '../../../utils/url-util';
+
/**
* This file was originally a copy of https://github.com/visionmedia/page.js.
* It was converted to TypeScript and stripped off lots of code that we don't
@@ -252,17 +254,6 @@
};
}
-function sameOrigin(href: string) {
- if (!href) return false;
- const url = new URL(href, window.location.toString());
- const loc = window.location;
- return (
- loc.protocol === url.protocol &&
- loc.hostname === url.hostname &&
- loc.port === url.port
- );
-}
-
function samePath(url: HTMLAnchorElement) {
const loc = window.location;
return url.pathname === loc.pathname && url.search === loc.search;
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 8bcbb2b..4f1ce7a 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
@@ -24,7 +24,7 @@
import {GrButton} from '../../shared/gr-button/gr-button';
import {TokenHighlightLayer} from '../../../embed/diff/gr-diff-builder/token-highlight-layer';
import {css, html, LitElement, nothing} from 'lit';
-import {customElement, property, query, state} from 'lit/decorators.js';
+import {customElement, query, state} from 'lit/decorators.js';
import {sharedStyles} from '../../../styles/shared-styles';
import {subscribe} from '../../lit/subscription-controller';
import {assert} from '../../../utils/common-util';
@@ -39,6 +39,7 @@
import {fireReload} from '../../../utils/event-util';
import {when} from 'lit/directives/when.js';
import {Timing} from '../../../constants/reporting';
+import {changeModelToken} from '../../../models/change/change-model';
interface FilePreview {
filepath: string;
@@ -62,10 +63,10 @@
@query('#nextFix')
nextFix?: GrButton;
- @property({type: Object})
+ @state()
change?: ParsedChangeInfo;
- @property({type: Number})
+ @state()
changeNum?: NumericChangeId;
@state()
@@ -102,6 +103,8 @@
private readonly getUserModel = resolve(this, userModelToken);
+ private readonly getChangeModel = resolve(this, changeModelToken);
+
private readonly getNavigation = resolve(this, navigationToken);
private readonly reporting = getAppContext().reportingService;
@@ -133,6 +136,16 @@
this.syntaxLayer.setEnabled(!!this.diffPrefs.syntax_highlighting);
}
);
+ subscribe(
+ this,
+ () => this.getChangeModel().change$,
+ change => (this.change = change)
+ );
+ subscribe(
+ this,
+ () => this.getChangeModel().changeNum$,
+ changeNum => (this.changeNum = changeNum)
+ );
}
static override styles = [
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
index bdb634b..0f54ad1 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
@@ -46,7 +46,6 @@
PreferencesInfo,
RepoName,
RevisionPatchSetNum,
- ServerInfo,
CommentMap,
} from '../../../types/common';
import {DiffInfo, DiffPreferencesInfo, WebLinkInfo} from '../../../types/diff';
@@ -80,7 +79,6 @@
import {ShortcutController} from '../../lit/shortcut-controller';
import {subscribe} from '../../lit/subscription-controller';
import {customElement, property, query, state} from 'lit/decorators.js';
-import {configModelToken} from '../../../models/config/config-model';
import {a11yStyles} from '../../../styles/gr-a11y-styles';
import {sharedStyles} from '../../../styles/shared-styles';
import {ifDefined} from 'lit/directives/if-defined.js';
@@ -194,9 +192,6 @@
@property({type: Object})
prefs?: DiffPreferencesInfo;
- @state()
- private serverConfig?: ServerInfo;
-
// Private but used in tests.
@state()
userPrefs?: PreferencesInfo;
@@ -241,8 +236,6 @@
private readonly getShortcutsService = resolve(this, shortcutsServiceToken);
- private readonly getConfigModel = resolve(this, configModelToken);
-
private readonly getViewModel = resolve(this, changeViewModelToken);
private throttledToggleFileReviewed?: (e: KeyboardEvent) => void;
@@ -340,13 +333,6 @@
);
subscribe(
this,
- () => this.getConfigModel().serverConfig$,
- config => {
- this.serverConfig = config;
- }
- );
- subscribe(
- this,
() => this.getCommentsModel().changeComments$,
changeComments => {
this.changeComments = changeComments;
@@ -966,12 +952,8 @@
}
private renderDialogs() {
- return html` <gr-apply-fix-dialog
- id="applyFixDialog"
- .change=${this.change}
- .changeNum=${this.changeNum}
- >
- </gr-apply-fix-dialog>
+ return html`
+ <gr-apply-fix-dialog id="applyFixDialog"></gr-apply-fix-dialog>
<gr-diff-preferences-dialog
id="diffPreferencesDialog"
@reload-diff-preference=${this.handleReloadingDiffPreference}
@@ -980,12 +962,10 @@
<dialog id="downloadModal" tabindex="-1">
<gr-download-dialog
id="downloadDialog"
- .change=${this.change}
- .patchNum=${this.patchNum}
- .config=${this.serverConfig?.download}
@close=${this.handleDownloadDialogClose}
></gr-download-dialog>
- </dialog>`;
+ </dialog>
+ `;
}
/**
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.ts b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.ts
index e527633..737e964 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.ts
@@ -1168,7 +1168,7 @@
});
test(
- '_prefs.manual_review true means set reviewed is not ' +
+ 'prefs.manual_review true means set reviewed is not ' +
'automatically called',
async () => {
const setReviewedFileStatusStub = sinon
@@ -1204,7 +1204,7 @@
}
);
- test('_prefs.manual_review false means set reviewed is called', async () => {
+ test('prefs.manual_review false means set reviewed is called', async () => {
const setReviewedFileStatusStub = sinon
.stub(changeModel, 'setReviewedFilesStatus')
.callsFake(() => Promise.resolve());
diff --git a/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select.ts b/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select.ts
index 3082979..76d67af 100644
--- a/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select.ts
+++ b/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select.ts
@@ -47,7 +47,6 @@
import {changeModelToken} from '../../../models/change/change-model';
import {changeViewModelToken} from '../../../models/views/change';
import {fireNoBubbleNoCompose} from '../../../utils/event-util';
-import {KnownExperimentId} from '../../../services/flags/flags';
// Maximum length for patch set descriptions.
const PATCH_DESC_MAX_LENGTH = 500;
@@ -126,8 +125,6 @@
private readonly getViewModel = resolve(this, changeViewModelToken);
- private readonly flagsService = getAppContext().flagsService;
-
constructor() {
super();
subscribe(
@@ -326,16 +323,7 @@
}
private computeText(patchNum: PatchSetNum, prefix: string, sha: string) {
- if (
- this.flagsService.isEnabled(KnownExperimentId.COMMENTS_CHIPS_IN_FILE_LIST)
- ) {
- return `${prefix}${patchNum} | ${sha}`;
- }
- return (
- `${prefix}${patchNum}` +
- `${this.computePatchSetCommentsString(patchNum)}` +
- ` | ${sha}`
- );
+ return `${prefix}${patchNum} | ${sha}`;
}
private createDropdownEntry(
@@ -349,17 +337,13 @@
mobileText: this.computeMobileText(patchNum),
bottomText: `${this.computePatchSetDescription(patchNum)}`,
value: patchNum,
- };
- if (
- this.flagsService.isEnabled(KnownExperimentId.COMMENTS_CHIPS_IN_FILE_LIST)
- ) {
- entry.commentThreads = this.changeComments?.computeCommentThreads(
+ commentThreads: this.changeComments?.computeCommentThreads(
{
patchNum,
},
true
- );
- }
+ ),
+ };
const date = this.computePatchSetDate(patchNum);
if (date) {
entry.date = date;
diff --git a/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select_test.ts b/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select_test.ts
index 6051131..b4ab043 100644
--- a/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select_test.ts
+++ b/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select_test.ts
@@ -147,6 +147,7 @@
mobileText: EDIT,
bottomText: '',
value: EDIT,
+ commentThreads: [],
},
{
disabled: true,
@@ -156,6 +157,7 @@
bottomText: '',
value: 3,
date: '2020-02-01 01:02:03.000000000' as Timestamp,
+ commentThreads: [],
} as DropdownItem,
{
disabled: true,
@@ -165,6 +167,7 @@
bottomText: '',
value: 2,
date: '2020-02-01 01:02:03.000000000' as Timestamp,
+ commentThreads: [],
} as DropdownItem,
{
disabled: true,
@@ -174,6 +177,7 @@
bottomText: '',
value: 1,
date: '2020-02-01 01:02:03.000000000' as Timestamp,
+ commentThreads: [],
} as DropdownItem,
{
text: 'Base',
@@ -287,6 +291,7 @@
mobileText: EDIT,
bottomText: '',
value: EDIT,
+ commentThreads: [],
},
{
disabled: false,
@@ -296,6 +301,7 @@
bottomText: '',
value: 3,
date: '2020-02-01 01:02:03.000000000' as Timestamp,
+ commentThreads: [],
} as DropdownItem,
{
disabled: false,
@@ -305,6 +311,7 @@
bottomText: 'description',
value: 2,
date: '2020-02-01 01:02:03.000000000' as Timestamp,
+ commentThreads: [],
} as DropdownItem,
{
disabled: true,
@@ -314,6 +321,7 @@
bottomText: '',
value: 1,
date: '2020-02-01 01:02:03.000000000' as Timestamp,
+ commentThreads: [],
} as DropdownItem,
];
diff --git a/polygerrit-ui/app/elements/gr-app-element.ts b/polygerrit-ui/app/elements/gr-app-element.ts
index 58d7105..75bfca2 100644
--- a/polygerrit-ui/app/elements/gr-app-element.ts
+++ b/polygerrit-ui/app/elements/gr-app-element.ts
@@ -118,7 +118,7 @@
@state() private version?: string;
- @state() private view?: GerritView;
+ @state() view?: GerritView;
// TODO: Introduce a wrapper element for CHANGE, DIFF, EDIT view.
@state() private childView?: ChangeChildView;
@@ -549,15 +549,21 @@
private renderPluginScreen() {
if (this.view !== GerritView.PLUGIN_SCREEN) return nothing;
+ if (!this.params) return nothing;
const pluginViewState = this.params as PluginViewState;
- return html`
- <gr-endpoint-decorator .name=${this.computePluginScreenName()}>
- <gr-endpoint-param
- name="token"
- .value=${pluginViewState.screen}
- ></gr-endpoint-param>
- </gr-endpoint-decorator>
- `;
+ const pluginScreenName = this.computePluginScreenName();
+
+ return keyed(
+ pluginScreenName,
+ html`
+ <gr-endpoint-decorator .name=${pluginScreenName}>
+ <gr-endpoint-param
+ name="token"
+ .value=${pluginViewState.screen}
+ ></gr-endpoint-param>
+ </gr-endpoint-decorator>
+ `
+ );
}
private renderCLAView() {
diff --git a/polygerrit-ui/app/elements/gr-app-element_test.ts b/polygerrit-ui/app/elements/gr-app-element_test.ts
new file mode 100644
index 0000000..ec415ff
--- /dev/null
+++ b/polygerrit-ui/app/elements/gr-app-element_test.ts
@@ -0,0 +1,100 @@
+/**
+ * @license
+ * Copyright 2023 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+import '../test/common-test-setup';
+import './gr-app';
+import {fixture, html, assert} from '@open-wc/testing';
+import {GrAppElement} from './gr-app-element';
+import {queryAndAssert} from '../utils/common-util';
+import {GerritView} from '../services/router/router-model';
+import {PluginViewState} from '../models/views/plugin';
+import {GrEndpointDecorator} from './plugins/gr-endpoint-decorator/gr-endpoint-decorator';
+
+suite('gr-app-element tests', () => {
+ let element: GrAppElement;
+
+ setup(async () => {
+ element = await fixture<GrAppElement>(
+ html`<gr-app-element></gr-app-element>`
+ );
+ await element.updateComplete;
+ });
+
+ test('renders', () => {
+ assert.shadowDom.equal(
+ element,
+ /* HTML */ `
+ <gr-css-mixins> </gr-css-mixins>
+ <gr-endpoint-decorator name="banner"> </gr-endpoint-decorator>
+ <gr-main-header loggedin id="mainHeader" role="banner">
+ </gr-main-header>
+ <main>
+ <div class="errorView" id="errorView">
+ <div class="errorEmoji"></div>
+ <div class="errorText"></div>
+ <div class="errorMoreInfo"></div>
+ </div>
+ </main>
+ <footer>
+ <div>
+ Powered by
+ <a
+ href="https://www.gerritcodereview.com/"
+ rel="noopener"
+ target="_blank"
+ >
+ Gerrit Code Review
+ </a>
+ ()
+ <gr-endpoint-decorator name="footer-left"> </gr-endpoint-decorator>
+ </div>
+ <div>
+ Press “?” for keyboard shortcuts
+ <gr-endpoint-decorator name="footer-right"> </gr-endpoint-decorator>
+ </div>
+ </footer>
+ <gr-notifications-prompt> </gr-notifications-prompt>
+ <gr-endpoint-decorator name="plugin-overlay"> </gr-endpoint-decorator>
+ <gr-error-manager id="errorManager"> </gr-error-manager>
+ <gr-plugin-host id="plugins"> </gr-plugin-host>
+ `
+ );
+ });
+
+ test('renders plugin screen, changes endpoint instance', async () => {
+ element.view = GerritView.PLUGIN_SCREEN;
+ element.params = {
+ view: GerritView.PLUGIN_SCREEN,
+ screen: 'test-screen-1',
+ plugin: 'test-plugin',
+ } as PluginViewState;
+ await element.updateComplete;
+
+ const main1 = queryAndAssert(element, 'main');
+ const endpoint1 = queryAndAssert<GrEndpointDecorator>(
+ main1,
+ 'gr-endpoint-decorator'
+ );
+ assert.equal(endpoint1.name, 'test-plugin-screen-test-screen-1');
+
+ element.params = {
+ view: GerritView.PLUGIN_SCREEN,
+ screen: 'test-screen-2',
+ plugin: 'test-plugin',
+ } as PluginViewState;
+ await element.updateComplete;
+
+ const main2 = queryAndAssert(element, 'main');
+ const endpoint2 = queryAndAssert<GrEndpointDecorator>(
+ main2,
+ 'gr-endpoint-decorator'
+ );
+ assert.equal(endpoint2.name, 'test-plugin-screen-test-screen-2');
+
+ // Plugin screen endpoints have a variable name. Lit must not re-use the
+ // same element instance. (Issue 16884)
+ assert.isFalse(endpoint1 === endpoint2);
+ });
+});
diff --git a/polygerrit-ui/app/elements/gr-app-global-var-init.ts b/polygerrit-ui/app/elements/gr-app-global-var-init.ts
index d6a14ed..6589ee8 100644
--- a/polygerrit-ui/app/elements/gr-app-global-var-init.ts
+++ b/polygerrit-ui/app/elements/gr-app-global-var-init.ts
@@ -21,6 +21,7 @@
initErrorReporter,
initWebVitals,
initClickReporter,
+ initInteractionReporter,
} from '../services/gr-reporting/gr-reporting_impl';
import {Finalizable} from '../services/registry';
@@ -36,6 +37,7 @@
initWebVitals(reportingService);
initErrorReporter(reportingService);
initClickReporter(reportingService);
+ initInteractionReporter(reportingService);
}
window.GrAnnotation = GrAnnotation;
window.GrPluginActionContext = GrPluginActionContext;
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 e230faf..e880531 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
@@ -21,7 +21,11 @@
import '../gr-user-suggestion-fix/gr-user-suggestion-fix';
import {KnownExperimentId} from '../../../services/flags/flags';
import {getAppContext} from '../../../services/app-context';
-import {USER_SUGGESTION_INFO_STRING} from '../../../utils/comment-util';
+import {
+ getUserSuggestionFromString,
+ USER_SUGGESTION_INFO_STRING,
+} from '../../../utils/comment-util';
+import {sameOrigin} from '../../../utils/url-util';
/**
* This element optionally renders markdown and also applies some regex
@@ -200,9 +204,8 @@
/* HTML */
`<a
href="${href}"
- target="_blank"
+ ${sameOrigin(href) ? '' : 'target="_blank" rel="noopener"'}
${title ? `title="${title}"` : ''}
- rel="noopener"
>${text}</a
>`;
renderer['image'] = (href: string, _title: string, text: string) =>
@@ -298,9 +301,16 @@
}
private convertCodeToSuggestions() {
- for (const userSuggestionMark of this.renderRoot.querySelectorAll('mark')) {
+ const marks = this.renderRoot.querySelectorAll('mark');
+ if (marks.length > 0) {
+ const userSuggestionMark = marks[0];
const userSuggestion = document.createElement('gr-user-suggestion-fix');
- userSuggestion.textContent = userSuggestionMark.textContent ?? '';
+ // Temporary workaround for bug - tabs replacement
+ if (this.content.includes('\t')) {
+ userSuggestion.textContent = getUserSuggestionFromString(this.content);
+ } else {
+ userSuggestion.textContent = userSuggestionMark.textContent ?? '';
+ }
userSuggestionMark.parentNode?.replaceChild(
userSuggestion,
userSuggestionMark
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 81048a3..9d7f06e 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
@@ -502,7 +502,11 @@
});
test('renders inline links into <a> tags', async () => {
- element.content = '[myLink](https://www.google.com)';
+ const origin = window.location.origin;
+ element.content = `[myLink1](https://www.google.com)
+ [myLink2](/destiny)
+ [myLink3](${origin}/destiny)
+ `;
await element.updateComplete;
assert.shadowDom.equal(
@@ -512,8 +516,12 @@
<div slot="markdown-html" class="markdown-html">
<p>
<a href="https://www.google.com" rel="noopener" target="_blank"
- >myLink</a
+ >myLink1</a
>
+ <br />
+ <a href="/destiny">myLink2</a>
+ <br />
+ <a href="${origin}/destiny">myLink3</a>
</p>
</div>
</marked-element>
diff --git a/polygerrit-ui/app/elements/shared/gr-weblink/gr-weblink.ts b/polygerrit-ui/app/elements/shared/gr-weblink/gr-weblink.ts
index 63a1dc4..fb6372c 100644
--- a/polygerrit-ui/app/elements/shared/gr-weblink/gr-weblink.ts
+++ b/polygerrit-ui/app/elements/shared/gr-weblink/gr-weblink.ts
@@ -30,6 +30,7 @@
display: inline-block;
vertical-align: top;
line-height: var(--line-height-normal);
+ margin-right: var(--spacing-s);
}
a {
color: var(--link-color);
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-binary.ts b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-binary.ts
index 5267f307..cc45e1e 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-binary.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-binary.ts
@@ -3,16 +3,13 @@
* Copyright 2017 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
-import {GrDiffBuilderUnified} from './gr-diff-builder-unified';
+import {GrDiffBuilder} from './gr-diff-builder';
import {DiffInfo, DiffPreferencesInfo} from '../../../types/diff';
import {createElementDiff} from '../gr-diff/gr-diff-utils';
import {GrDiffGroup} from '../gr-diff/gr-diff-group';
-import {GrDiffLine} from '../gr-diff/gr-diff-line';
-import {GrDiffLineType} from '../../../api/diff';
-import {queryAndAssert} from '../../../utils/common-util';
import {html, render} from 'lit';
-export class GrDiffBuilderBinary extends GrDiffBuilderUnified {
+export class GrDiffBuilderBinary extends GrDiffBuilder {
constructor(
diff: DiffInfo,
prefs: DiffPreferencesInfo,
@@ -25,17 +22,21 @@
const section = createElementDiff('tbody', 'binary-diff');
// Do not create a diff row for 'LOST'.
if (group.lines[0].beforeNumber !== 'FILE') return section;
+ return super.buildSectionElement(group);
+ }
- const line = new GrDiffLine(GrDiffLineType.BOTH, 'FILE', 'FILE');
- const fileRow = this.createRow(line);
- const contentTd = queryAndAssert<HTMLTableCellElement>(
- fileRow,
- 'td.both.file'
- )!;
- const div = document.createElement('div');
- render(html`<span>Difference in binary files</span>`, div);
- contentTd.insertBefore(div, contentTd.firstChild);
- section.appendChild(fileRow);
- return section;
+ public renderBinaryDiff() {
+ render(
+ html`
+ <tbody class="gr-diff binary-diff">
+ <tr class="gr-diff">
+ <td colspan="5" class="gr-diff">
+ <span>Difference in binary files</span>
+ </td>
+ </tr>
+ </tbody>
+ `,
+ this.outputEl
+ );
}
}
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-element.ts b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-element.ts
index 7fa6d72..396e9e2 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-element.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-element.ts
@@ -5,17 +5,15 @@
*/
import '../gr-diff-processor/gr-diff-processor';
import '../../../elements/shared/gr-hovercard/gr-hovercard';
-import './gr-diff-builder-side-by-side';
import {GrAnnotation} from '../gr-diff-highlight/gr-annotation';
import {
- DiffBuilder,
- ImageDiffBuilder,
+ GrDiffBuilder,
DiffContextExpandedEventDetail,
isImageDiffBuilder,
+ isBinaryDiffBuilder,
} from './gr-diff-builder';
import {GrDiffBuilderImage} from './gr-diff-builder-image';
import {GrDiffBuilderBinary} from './gr-diff-builder-binary';
-import {GrDiffBuilderLit} from './gr-diff-builder-lit';
import {CancelablePromise, makeCancelable} from '../../../utils/async-util';
import {BlameInfo, ImageInfo} from '../../../types/common';
import {DiffInfo, DiffPreferencesInfo} from '../../../types/diff';
@@ -117,7 +115,7 @@
layers: DiffLayer[] = [];
// visible for testing
- builder?: DiffBuilder | ImageDiffBuilder;
+ builder?: GrDiffBuilder;
/**
* All layers, both from the outside and the default ones. See `layers` for
@@ -211,6 +209,8 @@
.then(async () => {
if (isImageDiffBuilder(this.builder)) {
this.builder.renderImageDiff();
+ } else if (isBinaryDiffBuilder(this.builder)) {
+ this.builder.renderBinaryDiff();
}
await this.untilGroupsRendered();
fire(this.diffElement, 'render-content', {});
@@ -261,31 +261,21 @@
this.layersInternal = layers;
}
- getContentTdByLine(lineNumber: LineNumber, side?: Side, root?: Element) {
- if (!this.builder) return null;
- return this.builder.getContentTdByLine(lineNumber, side, root);
+ getContentTdByLine(lineNumber: LineNumber, side?: Side) {
+ if (!this.builder) return undefined;
+ return this.builder.getContentTdByLine(lineNumber, side);
}
- private getDiffRowByChild(child: Element) {
- while (!child.classList.contains('diff-row') && child.parentElement) {
- child = child.parentElement;
- }
- return child;
- }
-
- getContentTdByLineEl(lineEl?: Element): Element | null {
- if (!lineEl) return null;
+ getContentTdByLineEl(lineEl?: Element): Element | undefined {
+ if (!lineEl) return undefined;
const line = getLineNumber(lineEl);
- if (!line) return null;
+ if (!line) return undefined;
const side = getSideByLineEl(lineEl);
- // Performance optimization because we already have an element in the
- // correct row
- const row = this.getDiffRowByChild(lineEl);
- return this.getContentTdByLine(line, side, row);
+ return this.getContentTdByLine(line, side);
}
getLineElByNumber(lineNumber: LineNumber, side?: Side) {
- if (!this.builder) return null;
+ if (!this.builder) return undefined;
return this.builder.getLineElByNumber(lineNumber, side);
}
@@ -407,7 +397,7 @@
}
// visible for testing
- getDiffBuilder(): DiffBuilder {
+ getDiffBuilder(): GrDiffBuilder {
assertIsDefined(this.diff, 'diff');
assertIsDefined(this.diffElement, 'diff table');
if (isNaN(this.prefs.tab_size) || this.prefs.tab_size <= 0) {
@@ -437,14 +427,13 @@
this.useNewImageDiffUi
);
} else if (this.diff.binary) {
- // If the diff is binary, but not an image.
return new GrDiffBuilderBinary(this.diff, localPrefs, this.diffElement);
} else if (this.viewMode === DiffViewMode.SIDE_BY_SIDE) {
this.renderPrefs = {
...this.renderPrefs,
view_mode: DiffViewMode.SIDE_BY_SIDE,
};
- builder = new GrDiffBuilderLit(
+ builder = new GrDiffBuilder(
this.diff,
localPrefs,
this.diffElement,
@@ -456,7 +445,7 @@
...this.renderPrefs,
view_mode: DiffViewMode.UNIFIED,
};
- builder = new GrDiffBuilderLit(
+ builder = new GrDiffBuilder(
this.diff,
localPrefs,
this.diffElement,
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-element_test.ts b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-element_test.ts
index ba6acfd..d776164 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-element_test.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-element_test.ts
@@ -12,7 +12,6 @@
import {stubBaseUrl, waitUntil} from '../../../test/test-utils';
import {GrAnnotation} from '../gr-diff-highlight/gr-annotation';
import {GrDiffLine, GrDiffLineType} from '../gr-diff/gr-diff-line';
-import {GrDiffBuilderSideBySide} from './gr-diff-builder-side-by-side';
import {
DiffContent,
DiffLayer,
@@ -21,26 +20,23 @@
Side,
} from '../../../api/diff';
import {stubRestApi} from '../../../test/test-utils';
-import {GrDiffBuilderLegacy} from './gr-diff-builder-legacy';
import {waitForEventOnce} from '../../../utils/event-util';
import {GrDiffBuilderElement} from './gr-diff-builder-element';
import {createDefaultDiffPrefs} from '../../../constants/constants';
import {KeyLocations} from '../gr-diff-processor/gr-diff-processor';
-import {BlameInfo} from '../../../types/common';
import {fixture, html, assert} from '@open-wc/testing';
import {GrDiffRow} from './gr-diff-row';
+import {GrDiffBuilder} from './gr-diff-builder';
const DEFAULT_PREFS = createDefaultDiffPrefs();
suite('gr-diff-builder tests', () => {
let element: GrDiffBuilderElement;
- let builder: GrDiffBuilderLegacy;
+ let builder: GrDiffBuilder;
let diffTable: HTMLTableElement;
- const LINE_BREAK_HTML = '<span class="gr-diff br"></span>';
-
const setBuilderPrefs = (prefs: Partial<DiffPreferencesInfo>) => {
- builder = new GrDiffBuilderSideBySide(
+ builder = new GrDiffBuilder(
createEmptyDiff(),
{...createDefaultDiffPrefs(), ...prefs},
diffTable
@@ -63,15 +59,6 @@
setBuilderPrefs({});
});
- test('line_length applied with line break if line_wrapping is false', () => {
- setBuilderPrefs({line_wrapping: false, tab_size: 4, line_length: 50});
- const text = 'a'.repeat(51);
- const expected = 'a'.repeat(50) + LINE_BREAK_HTML + 'a';
- const result = builder.createTextEl(null, line(text)).firstElementChild
- ?.firstElementChild?.innerHTML;
- assert.equal(result, expected);
- });
-
[DiffViewMode.UNIFIED, DiffViewMode.SIDE_BY_SIDE].forEach(mode => {
test(`line_length used for regular files under ${mode}`, () => {
element.path = '/a.txt';
@@ -82,8 +69,8 @@
tab_size: 4,
line_length: 50,
};
- builder = element.getDiffBuilder() as GrDiffBuilderLegacy;
- assert.equal(builder._prefs.line_length, 50);
+ builder = element.getDiffBuilder();
+ assert.equal(builder.prefs.line_length, 50);
});
test(`line_length ignored for commit msg under ${mode}`, () => {
@@ -95,26 +82,11 @@
tab_size: 4,
line_length: 50,
};
- builder = element.getDiffBuilder() as GrDiffBuilderLegacy;
- assert.equal(builder._prefs.line_length, 72);
+ builder = element.getDiffBuilder();
+ assert.equal(builder.prefs.line_length, 72);
});
});
- test('createTextEl linewrap with tabs', () => {
- setBuilderPrefs({tab_size: 4, line_length: 10});
- const text = '\t'.repeat(7) + '!';
- const el = builder.createTextEl(null, line(text));
- assert.equal(el.innerText, text);
- // With line length 10 and tab size 4, there should be a line break
- // after every two tabs.
- const newlineEl = el.querySelector('.contentText .br');
- assert.isOk(newlineEl);
- assert.equal(
- el.querySelector('.contentText .tab:nth-child(2)')?.nextSibling,
- newlineEl
- );
- });
-
test('_handlePreferenceError throws with invalid preference', () => {
element.prefs = {...createDefaultDiffPrefs(), tab_size: 0};
assert.throws(() => element.getDiffBuilder());
@@ -553,95 +525,6 @@
});
});
- suite('rendering', () => {
- let content: DiffContent[];
- let outputEl: HTMLTableElement;
- let keyLocations: KeyLocations;
- let addColumnsStub: sinon.SinonStub;
- let dispatchStub: sinon.SinonStub;
- let builder: GrDiffBuilderSideBySide;
-
- setup(() => {
- const prefs = {...DEFAULT_PREFS};
- content = [
- {
- a: ['all work and no play make andybons a dull boy'],
- b: ['elgoog elgoog elgoog'],
- },
- {
- ab: [
- 'Non eram nescius, Brute, cum, quae summis ingeniis ',
- 'exquisitaque doctrina philosophi Graeco sermone tractavissent',
- ],
- },
- ];
- dispatchStub = sinon.stub(diffTable, 'dispatchEvent');
- outputEl = element.diffElement!;
- keyLocations = {left: {}, right: {}};
- sinon.stub(element, 'getDiffBuilder').callsFake(() => {
- builder = new GrDiffBuilderSideBySide(
- {...createEmptyDiff(), content},
- prefs,
- outputEl
- );
- addColumnsStub = sinon.stub(builder, 'addColumns');
- builder.buildSectionElement = function (group) {
- const section = document.createElement('stub');
- section.style.display = 'block';
- section.textContent = group.lines.reduce(
- (acc, line) => acc + line.text,
- ''
- );
- return section;
- };
- return builder;
- });
- element.diff = {...createEmptyDiff(), content};
- element.prefs = prefs;
- element.render(keyLocations);
- });
-
- test('addColumns is called', () => {
- assert.isTrue(addColumnsStub.called);
- });
-
- test('getGroupsByLineRange one line', () => {
- const section = outputEl.querySelector<HTMLElement>(
- 'stub:nth-of-type(3)'
- );
- const groups = builder.getGroupsByLineRange(1, 1, Side.LEFT);
- assert.equal(groups.length, 1);
- assert.strictEqual(groups[0].element, section);
- });
-
- test('getGroupsByLineRange over diff', () => {
- const section = [
- outputEl.querySelector<HTMLElement>('stub:nth-of-type(3)'),
- outputEl.querySelector<HTMLElement>('stub:nth-of-type(4)'),
- ];
- const groups = builder.getGroupsByLineRange(1, 2, Side.LEFT);
- assert.equal(groups.length, 2);
- assert.strictEqual(groups[0].element, section[0]);
- assert.strictEqual(groups[1].element, section[1]);
- });
-
- test('render-start and render-content are fired', async () => {
- await waitUntil(() => dispatchStub.callCount >= 1);
- let firedEventTypes = dispatchStub.getCalls().map(c => c.args[0].type);
- assert.include(firedEventTypes, 'render-start');
-
- await waitUntil(() => dispatchStub.callCount >= 2);
- firedEventTypes = dispatchStub.getCalls().map(c => c.args[0].type);
- assert.include(firedEventTypes, 'render-content');
- });
-
- test('cancel cancels the processor', () => {
- const processorCancelStub = sinon.stub(element.processor, 'cancel');
- element.cleanup();
- assert.isTrue(processorCancelStub.called);
- });
- });
-
suite('context hiding and expanding', () => {
let dispatchStub: sinon.SinonStub;
@@ -747,101 +630,4 @@
assert.include(firedEventTypes, 'render-content');
});
});
-
- suite('blame', () => {
- let mockBlame: BlameInfo[];
-
- setup(() => {
- mockBlame = [
- {
- author: 'test-author',
- time: 314,
- commit_msg: 'test-commit-message',
- id: 'commit 1',
- ranges: [
- {start: 1, end: 2},
- {start: 10, end: 16},
- ],
- },
- {
- author: 'test-author',
- time: 314,
- commit_msg: 'test-commit-message',
- id: 'commit 2',
- ranges: [
- {start: 4, end: 10},
- {start: 17, end: 32},
- ],
- },
- ];
- });
-
- test('setBlame attempts to render each blamed line', () => {
- const getBlameStub = sinon
- .stub(builder, 'getBlameTdByLine')
- .returns(undefined);
- builder.setBlame(mockBlame);
- assert.equal(getBlameStub.callCount, 32);
- });
-
- test('getBlameCommitForBaseLine', () => {
- sinon.stub(builder, 'getBlameTdByLine').returns(undefined);
- builder.setBlame(mockBlame);
- assert.isOk(builder.getBlameCommitForBaseLine(1));
- assert.equal(builder.getBlameCommitForBaseLine(1)?.id, 'commit 1');
-
- assert.isOk(builder.getBlameCommitForBaseLine(11));
- assert.equal(builder.getBlameCommitForBaseLine(11)?.id, 'commit 1');
-
- assert.isOk(builder.getBlameCommitForBaseLine(32));
- assert.equal(builder.getBlameCommitForBaseLine(32)?.id, 'commit 2');
-
- assert.isUndefined(builder.getBlameCommitForBaseLine(33));
- });
-
- test('getBlameCommitForBaseLine w/o blame returns null', () => {
- assert.isUndefined(builder.getBlameCommitForBaseLine(1));
- assert.isUndefined(builder.getBlameCommitForBaseLine(11));
- assert.isUndefined(builder.getBlameCommitForBaseLine(31));
- });
-
- test('createBlameCell', () => {
- const mockBlameInfo = {
- time: 1576155200,
- id: '1234567890',
- author: 'Clark Kent',
- commit_msg: 'Testing Commit',
- ranges: [{start: 4, end: 10}],
- };
- const getBlameStub = sinon
- .stub(builder, 'getBlameCommitForBaseLine')
- .returns(mockBlameInfo);
- const line = new GrDiffLine(GrDiffLineType.BOTH);
- line.beforeNumber = 3;
- line.afterNumber = 5;
-
- const result = builder.createBlameCell(line.beforeNumber);
-
- assert.isTrue(getBlameStub.calledWithExactly(3));
- assert.equal(result.getAttribute('data-line-number'), '3');
- assert.dom.equal(
- result,
- /* HTML */ `
- <span class="gr-diff">
- <a class="blameDate gr-diff" href="/r/q/1234567890"> 12/12/2019 </a>
- <span class="blameAuthor gr-diff">Clark</span>
- <gr-hovercard class="gr-diff">
- <span class="blameHoverCard gr-diff">
- Commit 1234567890<br />
- Author: Clark Kent<br />
- Date: 12/12/2019<br />
- <br />
- Testing Commit
- </span>
- </gr-hovercard>
- </span>
- `
- );
- });
- });
});
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-image.ts b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-image.ts
index 0ea904a..3bffe08 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-image.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-image.ts
@@ -3,23 +3,19 @@
* Copyright 2016 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
-import {GrDiffBuilderSideBySide} from './gr-diff-builder-side-by-side';
import {ImageInfo} from '../../../types/common';
import {DiffInfo, DiffPreferencesInfo} from '../../../types/diff';
import {RenderPreferences, Side} from '../../../api/diff';
import '../gr-diff-image-viewer/gr-image-viewer';
-import {ImageDiffBuilder} from './gr-diff-builder';
import {html, LitElement, nothing} from 'lit';
import {customElement, property, query, state} from 'lit/decorators.js';
+import {GrDiffBuilder} from './gr-diff-builder';
// MIME types for images we allow showing. Do not include SVG, it can contain
// arbitrary JavaScript.
const IMAGE_MIME_PATTERN = /^image\/(bmp|gif|x-icon|jpeg|jpg|png|tiff|webp)$/;
-export class GrDiffBuilderImage
- extends GrDiffBuilderSideBySide
- implements ImageDiffBuilder
-{
+export class GrDiffBuilderImage extends GrDiffBuilder {
constructor(
diff: DiffInfo,
prefs: DiffPreferencesInfo,
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-legacy.ts b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-legacy.ts
deleted file mode 100644
index e786cf4..0000000
--- a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-legacy.ts
+++ /dev/null
@@ -1,518 +0,0 @@
-/**
- * @license
- * Copyright 2016 Google LLC
- * SPDX-License-Identifier: Apache-2.0
- */
-import {RenderPreferences} from '../../../api/diff';
-import {fire} from '../../../utils/event-util';
-import {GrDiffLine, GrDiffLineType, LineNumber} from '../gr-diff/gr-diff-line';
-import {GrDiffGroup, GrDiffGroupType} from '../gr-diff/gr-diff-group';
-import '../gr-context-controls/gr-context-controls';
-import {
- GrContextControls,
- GrContextControlsShowConfig,
-} from '../gr-context-controls/gr-context-controls';
-import {DiffInfo, DiffPreferencesInfo} from '../../../types/diff';
-import {DiffViewMode, Side} from '../../../constants/constants';
-import {DiffLayer} from '../../../types/types';
-import {
- createBlameElement,
- createElementDiff,
- createElementDiffWithText,
- formatText,
- getResponsiveMode,
-} from '../gr-diff/gr-diff-utils';
-import {GrDiffBuilder} from './gr-diff-builder';
-import {BlameInfo} from '../../../types/common';
-
-function lineTdSelector(lineNumber: LineNumber, side?: Side): string {
- const sideSelector = side ? `.${side}` : '';
- return `td.lineNum[data-value="${lineNumber}"]${sideSelector}`;
-}
-/**
- * Base class for builders that are creating the DOM elements programmatically
- * by calling `document.createElement()` and such. We are calling such builders
- * "legacy", because we want to create (Lit) component based diff elements.
- *
- * TODO: Do not subclass `GrDiffBuilder`. Use composition and interfaces.
- */
-export abstract class GrDiffBuilderLegacy extends GrDiffBuilder {
- constructor(
- diff: DiffInfo,
- prefs: DiffPreferencesInfo,
- outputEl: HTMLElement,
- layers: DiffLayer[] = [],
- renderPrefs?: RenderPreferences
- ) {
- super(diff, prefs, outputEl, layers, renderPrefs);
- }
-
- override getContentTdByLine(
- lineNumber: LineNumber,
- side?: Side,
- root: Element = this.outputEl
- ): HTMLTableCellElement | null {
- return root.querySelector<HTMLTableCellElement>(
- `${lineTdSelector(lineNumber, side)} ~ td.content`
- );
- }
-
- override getLineElByNumber(
- lineNumber: LineNumber,
- side?: Side
- ): HTMLTableCellElement | null {
- return this.outputEl.querySelector<HTMLTableCellElement>(
- lineTdSelector(lineNumber, side)
- );
- }
-
- override getLineNumberRows() {
- return Array.from(
- this.outputEl.querySelectorAll<HTMLTableRowElement>(
- ':not(.contextControl) > .diff-row'
- ) ?? []
- ).filter(tr => tr.querySelector('button'));
- }
-
- override getLineNumEls(side: Side): HTMLTableCellElement[] {
- return Array.from(
- this.outputEl.querySelectorAll<HTMLTableCellElement>(
- `td.lineNum.${side}`
- ) ?? []
- );
- }
-
- override getBlameTdByLine(lineNum: number): Element | undefined {
- return (
- this.outputEl.querySelector(`td.blame[data-line-number="${lineNum}"]`) ??
- undefined
- );
- }
-
- override getContentByLine(
- lineNumber: LineNumber,
- side?: Side,
- root?: HTMLElement
- ): HTMLElement | null {
- const td = this.getContentTdByLine(lineNumber, side, root);
- return td ? td.querySelector('.contentText') : null;
- }
-
- override renderContentByRange(
- start: LineNumber,
- end: LineNumber,
- side: Side
- ) {
- const lines: GrDiffLine[] = [];
- const elements: HTMLElement[] = [];
- let line;
- let el;
- this.findLinesByRange(start, end, side, lines, elements);
- for (let i = 0; i < lines.length; i++) {
- line = lines[i];
- el = elements[i];
- if (!el || !el.parentElement) {
- // Cannot re-render an element if it does not exist. This can happen
- // if lines are collapsed and not visible on the page yet.
- continue;
- }
- const lineNumberEl = this.getLineNumberEl(el, side);
- const newContent = this.createTextEl(lineNumberEl, line, side)
- .firstChild as HTMLElement;
- // Note that ${el.id} ${newContent.id} might actually mismatch: In unified
- // diff we are rendering the same content twice for all the diff chunk
- // that are unchanged from left to right. TODO: Be smarter about this.
- el.parentElement.replaceChild(newContent, el);
- }
- }
-
- override renderBlameByRange(blame: BlameInfo, start: number, end: number) {
- for (let i = start; i <= end; i++) {
- // TODO(wyatta): this query is expensive, but, when traversing a
- // range, the lines are consecutive, and given the previous blame
- // cell, the next one can be reached cheaply.
- const blameCell = this.getBlameTdByLine(i);
- if (!blameCell) continue;
-
- // Remove the element's children (if any).
- while (blameCell.hasChildNodes()) {
- blameCell.removeChild(blameCell.lastChild!);
- }
- const blameEl = createBlameElement(i, blame);
- if (blameEl) blameCell.appendChild(blameEl);
- }
- }
-
- /**
- * Finds the line number element given the content element by walking up the
- * DOM tree to the diff row and then querying for a .lineNum element on the
- * requested side.
- *
- * TODO(brohlfs): Consolidate this with getLineEl... methods in html file.
- */
- // visible for testing
- getLineNumberEl(content: HTMLElement, side: Side): HTMLElement | null {
- let row: HTMLElement | null = content;
- while (row && !row.classList.contains('diff-row')) row = row.parentElement;
- return row ? (row.querySelector('.lineNum.' + side) as HTMLElement) : null;
- }
-
- /**
- * Adds <tr> table rows to a <tbody> section for allowing the user to expand
- * collapsed of lines. Called by subclasses.
- */
- protected createContextControls(
- section: HTMLElement,
- group: GrDiffGroup,
- viewMode: DiffViewMode
- ) {
- const leftStart = group.lineRange.left.start_line;
- const leftEnd = group.lineRange.left.end_line;
- const firstGroupIsSkipped = !!group.contextGroups[0].skip;
- const lastGroupIsSkipped =
- !!group.contextGroups[group.contextGroups.length - 1].skip;
-
- const containsWholeFile = this.numLinesLeft === leftEnd - leftStart + 1;
- const showAbove =
- (leftStart > 1 && !firstGroupIsSkipped) || containsWholeFile;
- const showBelow = leftEnd < this.numLinesLeft && !lastGroupIsSkipped;
-
- if (showAbove) {
- const paddingRow = this.createContextControlPaddingRow(viewMode);
- paddingRow.classList.add('above');
- section.appendChild(paddingRow);
- }
- section.appendChild(
- this.createContextControlRow(group, showAbove, showBelow, viewMode)
- );
- if (showBelow) {
- const paddingRow = this.createContextControlPaddingRow(viewMode);
- paddingRow.classList.add('below');
- section.appendChild(paddingRow);
- }
- }
-
- /**
- * Creates a context control <tr> table row for with buttons the allow the
- * user to expand collapsed lines. Buttons extend from the gap created by this
- * method up or down into the area of code that they affect.
- */
- private createContextControlRow(
- group: GrDiffGroup,
- showAbove: boolean,
- showBelow: boolean,
- viewMode: DiffViewMode
- ): HTMLElement {
- const row = createElementDiff('tr', 'dividerRow');
- let showConfig: GrContextControlsShowConfig;
- if (showAbove && !showBelow) {
- showConfig = 'above';
- } else if (!showAbove && showBelow) {
- showConfig = 'below';
- } else {
- // Note that !showAbove && !showBelow also intentionally creates
- // "show-both". This means the file is completely collapsed, which is
- // unusual, but at least happens in one test.
- showConfig = 'both';
- }
- row.classList.add(`show-${showConfig}`);
-
- row.appendChild(this.createBlameCell(0));
- if (viewMode === DiffViewMode.SIDE_BY_SIDE) {
- row.appendChild(createElementDiff('td'));
- }
-
- const cell = createElementDiff('td', 'dividerCell');
- // Note that <td> table cells that have `display: none` don't count!
- const colspan = this.renderPrefs?.show_sign_col ? '5' : '3';
- cell.setAttribute('colspan', colspan);
- row.appendChild(cell);
-
- const contextControls = createElementDiff(
- 'gr-context-controls'
- ) as GrContextControls;
- contextControls.diff = this._diff;
- contextControls.renderPreferences = this.renderPrefs;
- contextControls.group = group;
- contextControls.showConfig = showConfig;
- cell.appendChild(contextControls);
- return row;
- }
-
- /**
- * Creates a table row to serve as padding between code and context controls.
- * Blame column, line gutters, and content area will continue visually, but
- * context controls can render over this background to map more clearly to
- * the area of code they expand.
- */
- private createContextControlPaddingRow(viewMode: DiffViewMode) {
- const row = createElementDiff('tr', 'contextBackground');
-
- if (viewMode === DiffViewMode.SIDE_BY_SIDE) {
- row.classList.add('side-by-side');
- row.setAttribute('left-type', GrDiffGroupType.CONTEXT_CONTROL);
- row.setAttribute('right-type', GrDiffGroupType.CONTEXT_CONTROL);
- } else {
- row.classList.add('unified');
- }
-
- row.appendChild(this.createBlameCell(0));
- row.appendChild(createElementDiff('td', 'contextLineNum'));
- if (viewMode === DiffViewMode.SIDE_BY_SIDE) {
- row.appendChild(createElementDiff('td', 'sign'));
- row.appendChild(createElementDiff('td'));
- }
- row.appendChild(createElementDiff('td', 'contextLineNum'));
- if (viewMode === DiffViewMode.SIDE_BY_SIDE) {
- row.appendChild(createElementDiff('td', 'sign'));
- }
- row.appendChild(createElementDiff('td'));
-
- return row;
- }
-
- protected createLineEl(
- line: GrDiffLine,
- number: LineNumber,
- type: GrDiffLineType,
- side: Side
- ) {
- const td = createElementDiff('td');
- td.classList.add(side);
- if (line.type === GrDiffLineType.BLANK) {
- td.classList.add('blankLineNum');
- return td;
- }
- if (line.type === GrDiffLineType.BOTH || line.type === type) {
- td.classList.add('lineNum');
- td.dataset['value'] = number.toString();
-
- if (
- ((this._prefs.show_file_comment_button === false ||
- this.renderPrefs?.show_file_comment_button === false) &&
- number === 'FILE') ||
- number === 'LOST'
- ) {
- return td;
- }
-
- const button = createElementDiff('button');
- td.appendChild(button);
- button.tabIndex = -1;
- button.classList.add('lineNumButton');
- button.classList.add(side);
- button.dataset['value'] = number.toString();
- button.id =
- side === Side.LEFT ? `left-button-${number}` : `right-button-${number}`;
- button.textContent = number === 'FILE' ? 'File' : number.toString();
- if (number === 'FILE') {
- button.setAttribute('aria-label', 'Add file comment');
- }
-
- // Add aria-labels for valid line numbers.
- // For unified diff, this method will be called with number set to 0 for
- // the empty line number column for added/removed lines. This should not
- // be announced to the screenreader.
- if (number !== 'FILE' && number > 0) {
- if (line.type === GrDiffLineType.REMOVE) {
- button.setAttribute('aria-label', `${number} removed`);
- } else if (line.type === GrDiffLineType.ADD) {
- button.setAttribute('aria-label', `${number} added`);
- } else {
- button.setAttribute('aria-label', `${number} unmodified`);
- }
- }
- this.addLineNumberMouseEvents(td, number, side);
- }
- return td;
- }
-
- private addLineNumberMouseEvents(
- el: HTMLElement,
- number: LineNumber,
- side: Side
- ) {
- el.addEventListener('mouseenter', () => {
- fire(el, 'line-mouse-enter', {lineNum: number, side});
- });
- el.addEventListener('mouseleave', () => {
- fire(el, 'line-mouse-leave', {lineNum: number, side});
- });
- }
-
- // visible for testing
- createTextEl(
- lineNumberEl: HTMLElement | null,
- line: GrDiffLine,
- side?: Side,
- twoSlots?: boolean
- ) {
- const td = createElementDiff('td');
- if (line.type !== GrDiffLineType.BLANK) {
- td.classList.add('content');
- }
- if (side) {
- td.classList.add(side);
- }
-
- // If intraline info is not available, the entire line will be
- // considered as changed and marked as dark red / green color
- if (!line.hasIntralineInfo) {
- td.classList.add('no-intraline-info');
- }
- td.classList.add(line.type);
-
- const lineNumber = side ? line.lineNumber(side) : 0;
- if (lineNumber === 'FILE') {
- td.classList.add('file');
- } else if (lineNumber === 'LOST') {
- td.classList.add('lost');
- } else {
- const responsiveMode = getResponsiveMode(this._prefs, this.renderPrefs);
- const contentId =
- side && lineNumber > 0 ? `${side}-content-${lineNumber}` : '';
- const contentText = formatText(
- line.text,
- responsiveMode,
- this._prefs.tab_size,
- this._prefs.line_length,
- contentId
- );
-
- if (side) {
- contentText.setAttribute('data-side', side);
- this.addLineNumberMouseEvents(td, lineNumber, side);
- }
-
- if (lineNumberEl && side) {
- for (const layer of this.layers) {
- if (typeof layer.annotate === 'function') {
- layer.annotate(contentText, lineNumberEl, line, side);
- }
- }
- } else {
- console.error('lineNumberEl or side not set, skipping layer.annotate');
- }
-
- td.appendChild(contentText);
- }
-
- if (side && lineNumber) {
- const threadGroupEl = document.createElement('div');
- threadGroupEl.className = 'thread-group';
- threadGroupEl.setAttribute('data-side', side);
-
- const slot = document.createElement('slot');
- slot.name = `${side}-${lineNumber}`;
- threadGroupEl.appendChild(slot);
-
- // For line.type === BOTH in unified diff we want two slots.
- if (twoSlots) {
- const slot = document.createElement('slot');
- const otherSide = side === Side.LEFT ? Side.RIGHT : Side.LEFT;
- slot.name = `${otherSide}-${line.lineNumber(otherSide)}`;
- threadGroupEl.appendChild(slot);
- }
-
- td.appendChild(threadGroupEl);
- }
-
- return td;
- }
-
- private createMovedLineAnchor(line: number, side: Side) {
- const anchor = createElementDiffWithText('a', `${line}`);
-
- // href is not actually used but important for Screen Readers
- anchor.setAttribute('href', `#${line}`);
- anchor.addEventListener('click', e => {
- e.preventDefault();
- fire(anchor, 'moved-link-clicked', {
- lineNum: line,
- side,
- });
- });
- return anchor;
- }
-
- private createMoveDescriptionDiv(movedIn: boolean, group: GrDiffGroup) {
- const div = createElementDiff('div');
- if (group.moveDetails?.range) {
- const {changed, range} = group.moveDetails;
- const otherSide = movedIn ? Side.LEFT : Side.RIGHT;
- const andChangedLabel = changed ? 'and changed ' : '';
- const direction = movedIn ? 'from' : 'to';
- const textLabel = `Moved ${andChangedLabel}${direction} lines `;
- div.appendChild(createElementDiffWithText('span', textLabel));
- div.appendChild(this.createMovedLineAnchor(range.start, otherSide));
- div.appendChild(createElementDiffWithText('span', ' - '));
- div.appendChild(this.createMovedLineAnchor(range.end, otherSide));
- } else {
- div.appendChild(
- createElementDiffWithText('span', movedIn ? 'Moved in' : 'Moved out')
- );
- }
- return div;
- }
-
- protected buildMoveControls(group: GrDiffGroup) {
- const movedIn = group.adds.length > 0;
- const {
- numberOfCells,
- movedOutIndex,
- movedInIndex,
- lineNumberCols,
- signCols,
- } = this.getMoveControlsConfig();
-
- let controlsClass;
- let descriptionIndex;
- const descriptionTextDiv = this.createMoveDescriptionDiv(movedIn, group);
- if (movedIn) {
- controlsClass = 'movedIn';
- descriptionIndex = movedInIndex;
- } else {
- controlsClass = 'movedOut';
- descriptionIndex = movedOutIndex;
- }
-
- const controls = createElementDiff('tr', `moveControls ${controlsClass}`);
- const cells = [...Array(numberOfCells).keys()].map(() =>
- createElementDiff('td')
- );
- lineNumberCols.forEach(index => {
- cells[index].classList.add('moveControlsLineNumCol');
- });
-
- if (signCols) {
- cells[signCols.left].classList.add('sign', 'left');
- cells[signCols.right].classList.add('sign', 'right');
- }
- const moveRangeHeader = createElementDiff('gr-range-header');
- moveRangeHeader.setAttribute('icon', 'move_item');
- moveRangeHeader.appendChild(descriptionTextDiv);
- cells[descriptionIndex].classList.add('moveHeader');
- cells[descriptionIndex].appendChild(moveRangeHeader);
- cells.forEach(c => {
- controls.appendChild(c);
- });
- return controls;
- }
-
- /**
- * Create a blame cell for the given base line. Blame information will be
- * included in the cell if available.
- */
- // visible for testing
- createBlameCell(lineNumber: LineNumber): HTMLTableCellElement {
- const blameTd = createElementDiff('td', 'blame') as HTMLTableCellElement;
- blameTd.setAttribute('data-line-number', lineNumber.toString());
- if (!lineNumber) return blameTd;
-
- const blameInfo = this.getBlameCommitForBaseLine(lineNumber);
- if (!blameInfo) return blameTd;
-
- blameTd.appendChild(createBlameElement(lineNumber, blameInfo));
- return blameTd;
- }
-}
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-lit.ts b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-lit.ts
deleted file mode 100644
index 2a61ef2..0000000
--- a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-lit.ts
+++ /dev/null
@@ -1,214 +0,0 @@
-/**
- * @license
- * Copyright 2022 Google LLC
- * SPDX-License-Identifier: Apache-2.0
- */
-import {DiffViewMode, RenderPreferences} from '../../../api/diff';
-import {LineNumber} from '../gr-diff/gr-diff-line';
-import {GrDiffGroup} from '../gr-diff/gr-diff-group';
-import {DiffInfo, DiffPreferencesInfo} from '../../../types/diff';
-import {Side} from '../../../constants/constants';
-import {DiffLayer, isDefined} from '../../../types/types';
-import {diffClasses} from '../gr-diff/gr-diff-utils';
-import {GrDiffBuilder} from './gr-diff-builder';
-import {BlameInfo} from '../../../types/common';
-import {html, nothing, render} from 'lit';
-import {GrDiffSection} from './gr-diff-section';
-import '../gr-context-controls/gr-context-controls';
-import './gr-diff-section';
-import {GrDiffRow} from './gr-diff-row';
-
-/**
- * Base class for builders that are creating the diff using Lit elements.
- */
-export class GrDiffBuilderLit extends GrDiffBuilder {
- constructor(
- diff: DiffInfo,
- prefs: DiffPreferencesInfo,
- outputEl: HTMLElement,
- layers: DiffLayer[] = [],
- renderPrefs?: RenderPreferences
- ) {
- super(diff, prefs, outputEl, layers, renderPrefs);
- }
-
- override getContentTdByLine(
- lineNumber: LineNumber,
- side?: Side,
- _root: Element = this.outputEl
- ): HTMLTableCellElement | null {
- if (!side) return null;
- const row = this.findRow(lineNumber, side);
- return row?.getContentCell(side) ?? null;
- }
-
- override getLineElByNumber(lineNumber: LineNumber, side: Side) {
- const row = this.findRow(lineNumber, side);
- return row?.getLineNumberCell(side) ?? null;
- }
-
- private findRow(lineNumber?: LineNumber, side?: Side): GrDiffRow | undefined {
- if (!side || !lineNumber) return undefined;
- const group = this.findGroup(side, lineNumber);
- if (!group) return undefined;
- const section = this.findSection(group);
- if (!section) return undefined;
- return section.findRow(side, lineNumber);
- }
-
- private getDiffRows() {
- const sections = [
- ...this.outputEl.querySelectorAll<GrDiffSection>('gr-diff-section'),
- ];
- return sections.map(s => s.getDiffRows()).flat();
- }
-
- override getLineNumberRows(): HTMLTableRowElement[] {
- const rows = this.getDiffRows();
- return rows.map(r => r.getTableRow()).filter(isDefined);
- }
-
- override getLineNumEls(side: Side): HTMLTableCellElement[] {
- const rows = this.getDiffRows();
- return rows.map(r => r.getLineNumberCell(side)).filter(isDefined);
- }
-
- override getBlameTdByLine(lineNumber: number): Element | undefined {
- return this.findRow(lineNumber, Side.LEFT)?.getBlameCell();
- }
-
- override getContentByLine(
- lineNumber: LineNumber,
- side?: Side,
- _root?: HTMLElement
- ): HTMLElement | null {
- const cell = this.getContentTdByLine(lineNumber, side);
- return (cell?.firstChild ?? null) as HTMLElement | null;
- }
-
- /** This is used when layers initiate an update. */
- override renderContentByRange(
- start: LineNumber,
- end: LineNumber,
- side: Side
- ) {
- const groups = this.getGroupsByLineRange(start, end, side);
- for (const group of groups) {
- const section = this.findSection(group);
- for (const row of section?.getDiffRows() ?? []) {
- row.requestUpdate();
- }
- }
- }
-
- private findSection(group?: GrDiffGroup): GrDiffSection | undefined {
- if (!group) return undefined;
- const leftClass = `left-${group.startLine(Side.LEFT)}`;
- const rightClass = `right-${group.startLine(Side.RIGHT)}`;
- return (
- this.outputEl.querySelector<GrDiffSection>(
- `gr-diff-section.${leftClass}.${rightClass}`
- ) ?? undefined
- );
- }
-
- override renderBlameByRange(
- blameInfo: BlameInfo,
- start: number,
- end: number
- ) {
- for (let lineNumber = start; lineNumber <= end; lineNumber++) {
- const row = this.findRow(lineNumber, Side.LEFT);
- if (!row) continue;
- row.blameInfo = blameInfo;
- }
- }
-
- // TODO: Refactor this such that adding the move controls becomes part of the
- // lit element.
- protected override getMoveControlsConfig() {
- return {
- numberOfCells: 6, // How many cells does the diff table have?
- movedOutIndex: 2, // Index of left content column in diff table.
- movedInIndex: 5, // Index of right content column in diff table.
- lineNumberCols: [0, 3], // Indices of line number columns in diff table.
- signCols: {left: 1, right: 4},
- };
- }
-
- protected override buildSectionElement(group: GrDiffGroup) {
- const leftCl = `left-${group.startLine(Side.LEFT)}`;
- const rightCl = `right-${group.startLine(Side.RIGHT)}`;
- const section = html`
- <gr-diff-section
- class="${leftCl} ${rightCl}"
- .group=${group}
- .diff=${this._diff}
- .layers=${this.layers}
- .diffPrefs=${this._prefs}
- .renderPrefs=${this.renderPrefs}
- ></gr-diff-section>
- `;
- // When using Lit's `render()` method it wants to be in full control of the
- // element that it renders into, so we let it render into a temp element.
- // Rendering into the diff table directly would interfere with
- // `clearDiffContent()`for example.
- // TODO: Remove legacy diff builder, then convert <gr-diff> to be fully lit
- // controlled, then this code will become part of the standard `render()` of
- // <gr-diff> as a LitElement.
- const tempEl = document.createElement('div');
- render(section, tempEl);
- const sectionEl = tempEl.firstElementChild as GrDiffSection;
- return sectionEl;
- }
-
- override addColumns(outputEl: HTMLElement, lineNumberWidth: number): void {
- const colgroup = html`
- <colgroup>
- <col class=${diffClasses('blame')}></col>
- ${this.renderUnifiedColumns(lineNumberWidth)}
- ${this.renderSideBySideColumns(Side.LEFT, lineNumberWidth)}
- ${this.renderSideBySideColumns(Side.RIGHT, lineNumberWidth)}
- </colgroup>
- `;
- // When using Lit's `render()` method it wants to be in full control of the
- // element that it renders into, so we let it render into a temp element.
- // Rendering into the diff table directly would interfere with
- // `clearDiffContent()`for example.
- // TODO: Remove legacy diff builder, then convert <gr-diff> to be fully lit
- // controlled, then this code will become part of the standard `render()` of
- // <gr-diff> as a LitElement.
- const tempEl = document.createElement('div');
- render(colgroup, tempEl);
- const colgroupEl = tempEl.firstElementChild as HTMLElement;
- outputEl.appendChild(colgroupEl);
- }
-
- private renderUnifiedColumns(lineNumberWidth: number) {
- if (this.renderPrefs?.view_mode !== DiffViewMode.UNIFIED) return nothing;
- return html`
- <col class=${diffClasses()} width=${lineNumberWidth}></col>
- <col class=${diffClasses()} width=${lineNumberWidth}></col>
- <col class=${diffClasses()}></col>
- `;
- }
-
- private renderSideBySideColumns(side: Side, lineNumberWidth: number) {
- if (this.renderPrefs?.view_mode === DiffViewMode.UNIFIED) return nothing;
- return html`
- <col class=${diffClasses(side)} width=${lineNumberWidth}></col>
- <col class=${diffClasses(side, 'sign')}></col>
- <col class=${diffClasses(side)}></col>
- `;
- }
-
- protected override getNextContentOnSide(
- _content: HTMLElement,
- _side: Side
- ): HTMLElement | null {
- // TODO: getNextContentOnSide() is not required by lit based rendering.
- // So let's refactor it to be moved into gr-diff-builder-legacy.
- console.warn('unimplemented method getNextContentOnSide() called');
- return null;
- }
-}
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-side-by-side.ts b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-side-by-side.ts
deleted file mode 100644
index 6ae7d4cd..0000000
--- a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-side-by-side.ts
+++ /dev/null
@@ -1,174 +0,0 @@
-/**
- * @license
- * Copyright 2016 Google LLC
- * SPDX-License-Identifier: Apache-2.0
- */
-import {GrDiffGroup, GrDiffGroupType} from '../gr-diff/gr-diff-group';
-import {DiffInfo, DiffPreferencesInfo} from '../../../types/diff';
-import {GrDiffLine, GrDiffLineType, LineNumber} from '../gr-diff/gr-diff-line';
-import {DiffViewMode, Side} from '../../../constants/constants';
-import {DiffLayer} from '../../../types/types';
-import {RenderPreferences} from '../../../api/diff';
-import {createElementDiff} from '../gr-diff/gr-diff-utils';
-import {GrDiffBuilderLegacy} from './gr-diff-builder-legacy';
-
-export class GrDiffBuilderSideBySide extends GrDiffBuilderLegacy {
- constructor(
- diff: DiffInfo,
- prefs: DiffPreferencesInfo,
- outputEl: HTMLElement,
- layers: DiffLayer[] = [],
- renderPrefs?: RenderPreferences
- ) {
- super(diff, prefs, outputEl, layers, renderPrefs);
- }
-
- protected override getMoveControlsConfig() {
- return {
- numberOfCells: 6,
- movedOutIndex: 2,
- movedInIndex: 5,
- lineNumberCols: [0, 3],
- signCols: {left: 1, right: 4},
- };
- }
-
- // visible for testing
- override buildSectionElement(group: GrDiffGroup) {
- const sectionEl = createElementDiff('tbody', 'section');
- sectionEl.classList.add(group.type);
- if (group.isTotal()) {
- sectionEl.classList.add('total');
- }
- if (group.dueToRebase) {
- sectionEl.classList.add('dueToRebase');
- }
- if (group.moveDetails) {
- sectionEl.classList.add('dueToMove');
- if (group.moveDetails.changed) {
- sectionEl.classList.add('changed');
- }
- sectionEl.appendChild(this.buildMoveControls(group));
- }
- if (group.ignoredWhitespaceOnly) {
- sectionEl.classList.add('ignoredWhitespaceOnly');
- }
- if (group.type === GrDiffGroupType.CONTEXT_CONTROL) {
- this.createContextControls(sectionEl, group, DiffViewMode.SIDE_BY_SIDE);
- return sectionEl;
- }
-
- const pairs = group.getSideBySidePairs();
- for (let i = 0; i < pairs.length; i++) {
- sectionEl.appendChild(this.createRow(pairs[i].left, pairs[i].right));
- }
- return sectionEl;
- }
-
- override addColumns(outputEl: HTMLElement, lineNumberWidth: number): void {
- const colgroup = document.createElement('colgroup');
-
- // Add the blame column.
- let col = createElementDiff('col', 'blame');
- colgroup.appendChild(col);
-
- // Add left-side line number.
- col = createElementDiff('col', 'left');
- col.setAttribute('width', lineNumberWidth.toString());
- colgroup.appendChild(col);
-
- colgroup.appendChild(createElementDiff('col', 'sign left'));
-
- // Add left-side content.
- colgroup.appendChild(createElementDiff('col', 'left'));
-
- // Add right-side line number.
- col = createElementDiff('col', 'right');
- col.setAttribute('width', lineNumberWidth.toString());
- colgroup.appendChild(col);
-
- colgroup.appendChild(createElementDiff('col', 'sign right'));
-
- // Add right-side content.
- colgroup.appendChild(createElementDiff('col', 'right'));
-
- outputEl.appendChild(colgroup);
- }
-
- private createRow(leftLine: GrDiffLine, rightLine: GrDiffLine) {
- const row = createElementDiff('tr');
- row.classList.add('diff-row', 'side-by-side');
- row.setAttribute('left-type', leftLine.type);
- row.setAttribute('right-type', rightLine.type);
- // TabIndex makes screen reader read a row when navigating with j/k
- row.tabIndex = -1;
- // Before Chrome 102, Chrome was able to compute a11y label from children
- // content. Now Chrome 102 and Firefox are not computing a11y label because
- // tr is not expected to have aria label. Adding aria role button is
- // pushing browser to compute aria even for tr. This can be removed, once
- // browsers will again compute a11y label even for tr when it is focused.
- // TODO: Remove when Chrome 102 is out of date for 1 year.
- row.setAttribute(
- 'aria-labelledby',
- [
- leftLine.beforeNumber ? `left-button-${leftLine.beforeNumber}` : '',
- leftLine.beforeNumber ? `left-content-${leftLine.beforeNumber}` : '',
- rightLine.afterNumber ? `right-button-${rightLine.afterNumber}` : '',
- rightLine.afterNumber ? `right-content-${rightLine.afterNumber}` : '',
- ]
- .join(' ')
- .trim()
- );
-
- row.appendChild(this.createBlameCell(leftLine.beforeNumber));
-
- this.appendPair(row, leftLine, leftLine.beforeNumber, Side.LEFT);
- this.appendPair(row, rightLine, rightLine.afterNumber, Side.RIGHT);
- return row;
- }
-
- private appendPair(
- row: HTMLElement,
- line: GrDiffLine,
- lineNumber: LineNumber,
- side: Side
- ) {
- const lineNumberEl = this.createLineEl(line, lineNumber, line.type, side);
- row.appendChild(lineNumberEl);
- row.appendChild(this.createSignEl(line, side));
- row.appendChild(this.createTextEl(lineNumberEl, line, side));
- }
-
- private createSignEl(line: GrDiffLine, side: Side): HTMLElement {
- const td = createElementDiff('td', 'sign');
- td.classList.add(side);
- if (line.type === GrDiffLineType.BLANK) {
- td.classList.add('blank');
- } else if (line.type === GrDiffLineType.ADD && side === Side.RIGHT) {
- td.classList.add('add');
- td.innerText = '+';
- } else if (line.type === GrDiffLineType.REMOVE && side === Side.LEFT) {
- td.classList.add('remove');
- td.innerText = '-';
- }
- if (!line.hasIntralineInfo) {
- td.classList.add('no-intraline-info');
- }
- return td;
- }
-
- // visible for testing
- override getNextContentOnSide(
- content: HTMLElement,
- side: Side
- ): HTMLElement | null {
- let tr: HTMLElement = content.parentElement!.parentElement!;
- while ((tr = tr.nextSibling as HTMLElement)) {
- const nextContent = tr.querySelector(
- 'td.content .contentText[data-side="' + side + '"]'
- );
- if (nextContent) return nextContent as HTMLElement;
- }
- return null;
- }
-}
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-unified.ts b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-unified.ts
deleted file mode 100644
index ffc2dcd..0000000
--- a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-unified.ts
+++ /dev/null
@@ -1,168 +0,0 @@
-/**
- * @license
- * Copyright 2016 Google LLC
- * SPDX-License-Identifier: Apache-2.0
- */
-import {GrDiffLine, GrDiffLineType} from '../gr-diff/gr-diff-line';
-import {GrDiffGroup, GrDiffGroupType} from '../gr-diff/gr-diff-group';
-import {DiffInfo, DiffPreferencesInfo} from '../../../types/diff';
-import {DiffViewMode, Side} from '../../../constants/constants';
-import {DiffLayer} from '../../../types/types';
-import {RenderPreferences} from '../../../api/diff';
-import {createElementDiff} from '../gr-diff/gr-diff-utils';
-import {GrDiffBuilderLegacy} from './gr-diff-builder-legacy';
-
-export class GrDiffBuilderUnified extends GrDiffBuilderLegacy {
- constructor(
- diff: DiffInfo,
- prefs: DiffPreferencesInfo,
- outputEl: HTMLElement,
- layers: DiffLayer[] = [],
- renderPrefs?: RenderPreferences
- ) {
- super(diff, prefs, outputEl, layers, renderPrefs);
- }
-
- protected override getMoveControlsConfig() {
- return {
- numberOfCells: 3,
- movedOutIndex: 2,
- movedInIndex: 2,
- lineNumberCols: [0, 1],
- };
- }
-
- // visible for testing
- override buildSectionElement(group: GrDiffGroup): HTMLElement {
- const sectionEl = createElementDiff('tbody', 'section');
- sectionEl.classList.add(group.type);
- if (group.isTotal()) {
- sectionEl.classList.add('total');
- }
- if (group.dueToRebase) {
- sectionEl.classList.add('dueToRebase');
- }
- if (group.moveDetails) {
- sectionEl.classList.add('dueToMove');
- if (group.moveDetails.changed) {
- sectionEl.classList.add('changed');
- }
- sectionEl.appendChild(this.buildMoveControls(group));
- }
- if (group.ignoredWhitespaceOnly) {
- sectionEl.classList.add('ignoredWhitespaceOnly');
- }
- if (group.type === GrDiffGroupType.CONTEXT_CONTROL) {
- this.createContextControls(sectionEl, group, DiffViewMode.UNIFIED);
- return sectionEl;
- }
-
- for (let i = 0; i < group.lines.length; ++i) {
- const line = group.lines[i];
- // If only whitespace has changed and the settings ask for whitespace to
- // be ignored, only render the right-side line in unified diff mode.
- if (group.ignoredWhitespaceOnly && line.type === GrDiffLineType.REMOVE) {
- continue;
- }
- sectionEl.appendChild(this.createRow(line));
- }
- return sectionEl;
- }
-
- override addColumns(outputEl: HTMLElement, lineNumberWidth: number): void {
- const colgroup = document.createElement('colgroup');
-
- // Add the blame column.
- let col = createElementDiff('col', 'blame');
- colgroup.appendChild(col);
-
- // Add left-side line number.
- col = createElementDiff('col');
- col.setAttribute('width', lineNumberWidth.toString());
- colgroup.appendChild(col);
-
- // Add right-side line number.
- col = createElementDiff('col');
- col.setAttribute('width', lineNumberWidth.toString());
- colgroup.appendChild(col);
-
- // Add the content.
- colgroup.appendChild(createElementDiff('col'));
-
- outputEl.appendChild(colgroup);
- }
-
- protected createRow(line: GrDiffLine) {
- const row = createElementDiff('tr', line.type);
- row.classList.add('diff-row', 'unified');
- // TabIndex makes screen reader read a row when navigating with j/k
- row.tabIndex = -1;
- row.appendChild(this.createBlameCell(line.beforeNumber));
- let lineNumberEl = this.createLineEl(
- line,
- line.beforeNumber,
- GrDiffLineType.REMOVE,
- Side.LEFT
- );
- row.appendChild(lineNumberEl);
- lineNumberEl = this.createLineEl(
- line,
- line.afterNumber,
- GrDiffLineType.ADD,
- Side.RIGHT
- );
- row.appendChild(lineNumberEl);
- let side = undefined;
- if (line.type === GrDiffLineType.ADD || line.type === GrDiffLineType.BOTH) {
- side = Side.RIGHT;
- }
- if (line.type === GrDiffLineType.REMOVE) {
- side = Side.LEFT;
- }
-
- // Before Chrome 102, Chrome was able to compute a11y label from children
- // content. Now Chrome 102 and Firefox are not computing a11y label because
- // tr is not expected to have aria label. Adding aria role button is
- // pushing browser to compute aria even for tr. This can be removed, once
- // browsers will again compute a11y label even for tr when it is focused.
- // TODO: Remove when Chrome 102 is out of date for 1 year.
- row.setAttribute(
- 'aria-labelledby',
- [
- line.beforeNumber ? `left-button-${line.beforeNumber}` : '',
- side === Side.LEFT && line.beforeNumber
- ? `left-content-${line.beforeNumber}`
- : '',
- line.afterNumber ? `right-button-${line.afterNumber}` : '',
- side === Side.RIGHT && line.afterNumber
- ? `right-content-${line.afterNumber}`
- : '',
- ]
- .filter(id => !!id)
- .join(' ')
- .trim()
- );
- const twoSlots = line.type === GrDiffLineType.BOTH;
- row.appendChild(this.createTextEl(lineNumberEl, line, side, twoSlots));
- return row;
- }
-
- getNextContentOnSide(content: HTMLElement, side: Side): HTMLElement | null {
- let tr: HTMLElement = content.parentElement!.parentElement!;
- while ((tr = tr.nextSibling as HTMLElement)) {
- // Note that this does not work when there is a "common" chunk in the
- // diff (different content only because of whitespace). Such chunks are
- // rendered with class "add", so these rows will be skipped for the
- // 'left' side.
- // TODO: Fix this when writing a Lit component for unified diff.
- if (
- tr.classList.contains('both') ||
- (side === 'left' && tr.classList.contains('remove')) ||
- (side === 'right' && tr.classList.contains('add'))
- ) {
- return tr.querySelector('.contentText');
- }
- }
- return null;
- }
-}
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-unified_test.ts b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-unified_test.ts
deleted file mode 100644
index 8c44727..0000000
--- a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-unified_test.ts
+++ /dev/null
@@ -1,283 +0,0 @@
-/**
- * @license
- * Copyright 2016 Google LLC
- * SPDX-License-Identifier: Apache-2.0
- */
-import '../../../test/common-test-setup';
-import '../gr-diff/gr-diff-group';
-import './gr-diff-builder';
-import './gr-diff-builder-unified';
-import {GrDiffLine, GrDiffLineType} from '../gr-diff/gr-diff-line';
-import {GrDiffGroup, GrDiffGroupType} from '../gr-diff/gr-diff-group';
-import {GrDiffBuilderUnified} from './gr-diff-builder-unified';
-import {DiffPreferencesInfo} from '../../../api/diff';
-import {createDefaultDiffPrefs} from '../../../constants/constants';
-import {createDiff} from '../../../test/test-data-generators';
-import {queryAndAssert} from '../../../utils/common-util';
-import {assert} from '@open-wc/testing';
-
-suite('GrDiffBuilderUnified tests', () => {
- let prefs: DiffPreferencesInfo;
- let outputEl: HTMLElement;
- let diffBuilder: GrDiffBuilderUnified;
-
- setup(() => {
- prefs = {
- ...createDefaultDiffPrefs(),
- line_length: 10,
- show_tabs: true,
- tab_size: 4,
- };
- outputEl = document.createElement('div');
- diffBuilder = new GrDiffBuilderUnified(createDiff(), prefs, outputEl, []);
- });
-
- suite('buildSectionElement for BOTH group', () => {
- let lines: GrDiffLine[];
- let group: GrDiffGroup;
-
- setup(() => {
- lines = [
- new GrDiffLine(GrDiffLineType.BOTH, 1, 2),
- new GrDiffLine(GrDiffLineType.BOTH, 2, 3),
- new GrDiffLine(GrDiffLineType.BOTH, 3, 4),
- ];
- lines[0].text = 'def hello_world():';
- lines[1].text = ' print "Hello World";';
- lines[2].text = ' return True';
-
- group = new GrDiffGroup({type: GrDiffGroupType.BOTH, lines});
- });
-
- test('creates the section', () => {
- const sectionEl = diffBuilder.buildSectionElement(group);
- assert.isTrue(sectionEl.classList.contains('section'));
- assert.isTrue(sectionEl.classList.contains('both'));
- });
-
- test('creates each unchanged row once', () => {
- const sectionEl = diffBuilder.buildSectionElement(group);
- const rowEls = sectionEl.querySelectorAll('.diff-row');
-
- assert.equal(rowEls.length, 3);
-
- assert.equal(
- queryAndAssert(rowEls[0], '.lineNum.left').textContent,
- lines[0].beforeNumber.toString()
- );
- assert.equal(
- queryAndAssert(rowEls[0], '.lineNum.right').textContent,
- lines[0].afterNumber.toString()
- );
- assert.equal(
- queryAndAssert(rowEls[0], '.content').textContent,
- lines[0].text
- );
-
- assert.equal(
- queryAndAssert(rowEls[1], '.lineNum.left').textContent,
- lines[1].beforeNumber.toString()
- );
- assert.equal(
- queryAndAssert(rowEls[1], '.lineNum.right').textContent,
- lines[1].afterNumber.toString()
- );
- assert.equal(
- queryAndAssert(rowEls[1], '.content').textContent,
- lines[1].text
- );
-
- assert.equal(
- queryAndAssert(rowEls[2], '.lineNum.left').textContent,
- lines[2].beforeNumber.toString()
- );
- assert.equal(
- queryAndAssert(rowEls[2], '.lineNum.right').textContent,
- lines[2].afterNumber.toString()
- );
- assert.equal(
- queryAndAssert(rowEls[2], '.content').textContent,
- lines[2].text
- );
- });
- });
-
- suite('buildSectionElement for moved chunks', () => {
- test('creates a moved out group', () => {
- const lines = [
- new GrDiffLine(GrDiffLineType.REMOVE, 15),
- new GrDiffLine(GrDiffLineType.REMOVE, 16),
- ];
- lines[0].text = 'def hello_world():';
- lines[1].text = ' print "Hello World"';
- const group = new GrDiffGroup({
- type: GrDiffGroupType.DELTA,
- lines,
- moveDetails: {changed: false},
- });
-
- const sectionEl = diffBuilder.buildSectionElement(group);
-
- const rowEls = sectionEl.querySelectorAll('tr');
- const moveControlsRow = rowEls[0];
- const cells = moveControlsRow.querySelectorAll('td');
- assert.isTrue(sectionEl.classList.contains('dueToMove'));
- assert.equal(rowEls.length, 3);
- assert.isTrue(moveControlsRow.classList.contains('movedOut'));
- assert.equal(cells.length, 3);
- assert.isTrue(cells[2].classList.contains('moveHeader'));
- assert.equal(cells[2].textContent, 'Moved out');
- });
-
- test('creates a moved in group', () => {
- const lines = [
- new GrDiffLine(GrDiffLineType.ADD, 37),
- new GrDiffLine(GrDiffLineType.ADD, 38),
- ];
- lines[0].text = 'def hello_world():';
- lines[1].text = ' print "Hello World"';
- const group = new GrDiffGroup({
- type: GrDiffGroupType.DELTA,
- lines,
- moveDetails: {changed: false},
- });
-
- const sectionEl = diffBuilder.buildSectionElement(group);
-
- const rowEls = sectionEl.querySelectorAll('tr');
- const moveControlsRow = rowEls[0];
- const cells = moveControlsRow.querySelectorAll('td');
- assert.isTrue(sectionEl.classList.contains('dueToMove'));
- assert.equal(rowEls.length, 3);
- assert.isTrue(moveControlsRow.classList.contains('movedIn'));
- assert.equal(cells.length, 3);
- assert.isTrue(cells[2].classList.contains('moveHeader'));
- assert.equal(cells[2].textContent, 'Moved in');
- });
- });
-
- suite('buildSectionElement for DELTA group', () => {
- let lines: GrDiffLine[];
- let group: GrDiffGroup;
-
- setup(() => {
- lines = [
- new GrDiffLine(GrDiffLineType.REMOVE, 1),
- new GrDiffLine(GrDiffLineType.REMOVE, 2),
- new GrDiffLine(GrDiffLineType.ADD, 2),
- new GrDiffLine(GrDiffLineType.ADD, 3),
- ];
- lines[0].text = 'def hello_world():';
- lines[1].text = ' print "Hello World"';
- lines[2].text = 'def hello_universe()';
- lines[3].text = ' print "Hello Universe"';
- });
-
- test('creates the section', () => {
- group = new GrDiffGroup({type: GrDiffGroupType.DELTA, lines});
- const sectionEl = diffBuilder.buildSectionElement(group);
- assert.isTrue(sectionEl.classList.contains('section'));
- assert.isTrue(sectionEl.classList.contains('delta'));
- });
-
- test('creates the section with class if ignoredWhitespaceOnly', () => {
- group = new GrDiffGroup({
- type: GrDiffGroupType.DELTA,
- lines,
- ignoredWhitespaceOnly: true,
- });
- const sectionEl = diffBuilder.buildSectionElement(group);
- assert.isTrue(sectionEl.classList.contains('ignoredWhitespaceOnly'));
- });
-
- test('creates the section with class if dueToRebase', () => {
- group = new GrDiffGroup({
- type: GrDiffGroupType.DELTA,
- lines,
- dueToRebase: true,
- });
- const sectionEl = diffBuilder.buildSectionElement(group);
- assert.isTrue(sectionEl.classList.contains('dueToRebase'));
- });
-
- test('creates first the removed and then the added rows', () => {
- group = new GrDiffGroup({type: GrDiffGroupType.DELTA, lines});
- const sectionEl = diffBuilder.buildSectionElement(group);
- const rowEls = sectionEl.querySelectorAll('.diff-row');
-
- assert.equal(rowEls.length, 4);
-
- assert.equal(
- queryAndAssert(rowEls[0], '.lineNum.left').textContent,
- lines[0].beforeNumber.toString()
- );
- assert.isNotOk(rowEls[0].querySelector('.lineNum.right'));
- assert.equal(
- queryAndAssert(rowEls[0], '.content').textContent,
- lines[0].text
- );
-
- assert.equal(
- queryAndAssert(rowEls[1], '.lineNum.left').textContent,
- lines[1].beforeNumber.toString()
- );
- assert.isNotOk(rowEls[1].querySelector('.lineNum.right'));
- assert.equal(
- queryAndAssert(rowEls[1], '.content').textContent,
- lines[1].text
- );
-
- assert.isNotOk(rowEls[2].querySelector('.lineNum.left'));
- assert.equal(
- queryAndAssert(rowEls[2], '.lineNum.right').textContent,
- lines[2].afterNumber.toString()
- );
- assert.equal(
- queryAndAssert(rowEls[2], '.content').textContent,
- lines[2].text
- );
-
- assert.isNotOk(rowEls[3].querySelector('.lineNum.left'));
- assert.equal(
- queryAndAssert(rowEls[3], '.lineNum.right').textContent,
- lines[3].afterNumber.toString()
- );
- assert.equal(
- queryAndAssert(rowEls[3], '.content').textContent,
- lines[3].text
- );
- });
-
- test('creates only the added rows if only ignored whitespace', () => {
- group = new GrDiffGroup({
- type: GrDiffGroupType.DELTA,
- lines,
- ignoredWhitespaceOnly: true,
- });
- const sectionEl = diffBuilder.buildSectionElement(group);
- const rowEls = sectionEl.querySelectorAll('.diff-row');
-
- assert.equal(rowEls.length, 2);
-
- assert.isNotOk(rowEls[0].querySelector('.lineNum.left'));
- assert.equal(
- queryAndAssert(rowEls[0], '.lineNum.right').textContent,
- lines[2].afterNumber.toString()
- );
- assert.equal(
- queryAndAssert(rowEls[0], '.content').textContent,
- lines[2].text
- );
-
- assert.isNotOk(rowEls[1].querySelector('.lineNum.left'));
- assert.equal(
- queryAndAssert(rowEls[1], '.lineNum.right').textContent,
- lines[3].afterNumber.toString()
- );
- assert.equal(
- queryAndAssert(rowEls[1], '.content').textContent,
- lines[3].text
- );
- });
- });
-});
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder.ts b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder.ts
index 5ca5197..f38ba5c 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder.ts
@@ -3,19 +3,27 @@
* Copyright 2016 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
+import './gr-diff-section';
+import '../gr-context-controls/gr-context-controls';
import {
ContentLoadNeededEventDetail,
DiffContextExpandedExternalDetail,
+ DiffViewMode,
RenderPreferences,
} from '../../../api/diff';
-import {GrDiffLine, GrDiffLineType, LineNumber} from '../gr-diff/gr-diff-line';
+import {LineNumber} from '../gr-diff/gr-diff-line';
import {GrDiffGroup} from '../gr-diff/gr-diff-group';
-import {assert} from '../../../utils/common-util';
-import '../gr-context-controls/gr-context-controls';
import {BlameInfo} from '../../../types/common';
import {DiffInfo, DiffPreferencesInfo} from '../../../types/diff';
import {Side} from '../../../constants/constants';
-import {DiffLayer} from '../../../types/types';
+import {DiffLayer, isDefined} from '../../../types/types';
+import {GrDiffRow} from './gr-diff-row';
+import {GrDiffSection} from './gr-diff-section';
+import {html, render} from 'lit';
+import {diffClasses} from '../gr-diff/gr-diff-utils';
+import {when} from 'lit/directives/when.js';
+import {GrDiffBuilderImage} from './gr-diff-builder-image';
+import {GrDiffBuilderBinary} from './gr-diff-builder-binary';
export interface DiffContextExpandedEventDetail
extends DiffContextExpandedExternalDetail {
@@ -32,73 +40,34 @@
}
}
-/**
- * Given that GrDiffBuilder has ~1,000 lines of code, this interface is just
- * making refactorings easier by emphasizing what the public facing "contract"
- * of this class is. There are no plans for adding separate implementations.
- */
-export interface DiffBuilder {
- init(): void;
- cleanup(): void;
- addGroups(groups: readonly GrDiffGroup[]): void;
- clearGroups(): void;
- replaceGroup(
- contextControl: GrDiffGroup,
- groups: readonly GrDiffGroup[]
- ): void;
- findGroup(side: Side, line: LineNumber): GrDiffGroup | undefined;
- addColumns(outputEl: HTMLElement, fontSize: number): void;
- // TODO: Change `null` to `undefined`.
- getContentTdByLine(
- lineNumber: LineNumber,
- side?: Side,
- root?: Element
- ): HTMLTableCellElement | null;
- getLineElByNumber(
- lineNumber: LineNumber,
- side?: Side
- ): HTMLTableCellElement | null;
- getLineNumberRows(): HTMLTableRowElement[];
- getLineNumEls(side: Side): HTMLTableCellElement[];
- setBlame(blame: BlameInfo[]): void;
- updateRenderPrefs(renderPrefs: RenderPreferences): void;
+export function isImageDiffBuilder<T extends GrDiffBuilder>(
+ x: T | GrDiffBuilderImage | undefined
+): x is GrDiffBuilderImage {
+ return !!x && !!(x as GrDiffBuilderImage).renderImageDiff;
}
-export interface ImageDiffBuilder extends DiffBuilder {
- renderImageDiff(): void;
-}
-
-export function isImageDiffBuilder(
- x: DiffBuilder | ImageDiffBuilder | undefined
-): x is ImageDiffBuilder {
- return !!x && !!(x as ImageDiffBuilder).renderImageDiff;
+export function isBinaryDiffBuilder<T extends GrDiffBuilder>(
+ x: T | GrDiffBuilderBinary | undefined
+): x is GrDiffBuilderBinary {
+ return !!x && !!(x as GrDiffBuilderBinary).renderBinaryDiff;
}
/**
- * Base class for different diff builders, like side-by-side, unified etc.
- *
* The builder takes GrDiffGroups, and builds the corresponding DOM elements,
* called sections. Only the builder should add or remove sections from the
* DOM. Callers can use the ...group() methods to modify groups and thus cause
* rendering changes.
- *
- * TODO: Do not subclass `GrDiffBuilder`. Use composition and interfaces.
*/
-export abstract class GrDiffBuilder implements DiffBuilder {
- protected readonly _diff: DiffInfo;
+export class GrDiffBuilder {
+ private readonly diff: DiffInfo;
- protected readonly numLinesLeft: number;
+ readonly prefs: DiffPreferencesInfo;
- // visible for testing
- readonly _prefs: DiffPreferencesInfo;
+ renderPrefs?: RenderPreferences;
- protected renderPrefs?: RenderPreferences;
+ readonly outputEl: HTMLElement;
- protected readonly outputEl: HTMLElement;
-
- protected groups: GrDiffGroup[];
-
- private blameInfo: BlameInfo[] = [];
+ private groups: GrDiffGroup[];
private readonly layerUpdateListener: (
start: LineNumber,
@@ -113,14 +82,8 @@
readonly layers: DiffLayer[] = [],
renderPrefs?: RenderPreferences
) {
- this._diff = diff;
- this.numLinesLeft = this._diff.content
- ? this._diff.content.reduce((sum, chunk) => {
- const left = chunk.a || chunk.ab;
- return sum + (left?.length || chunk.skip || 0);
- }, 0)
- : 0;
- this._prefs = prefs;
+ this.diff = diff;
+ this.prefs = prefs;
this.renderPrefs = renderPrefs;
this.outputEl = outputEl;
this.groups = [];
@@ -141,6 +104,138 @@
this.init();
}
+ getContentTdByLine(
+ lineNumber: LineNumber,
+ side?: Side
+ ): HTMLTableCellElement | undefined {
+ if (!side) return undefined;
+ const row = this.findRow(lineNumber, side);
+ return row?.getContentCell(side);
+ }
+
+ getLineElByNumber(
+ lineNumber: LineNumber,
+ side?: Side
+ ): HTMLTableCellElement | undefined {
+ if (!side) return undefined;
+ const row = this.findRow(lineNumber, side);
+ return row?.getLineNumberCell(side);
+ }
+
+ private findRow(lineNumber?: LineNumber, side?: Side): GrDiffRow | undefined {
+ if (!side || !lineNumber) return undefined;
+ const group = this.findGroup(side, lineNumber);
+ if (!group) return undefined;
+ const section = this.findSection(group);
+ if (!section) return undefined;
+ return section.findRow(side, lineNumber);
+ }
+
+ private getDiffRows() {
+ const sections = [
+ ...this.outputEl.querySelectorAll<GrDiffSection>('gr-diff-section'),
+ ];
+ return sections.map(s => s.getDiffRows()).flat();
+ }
+
+ getLineNumberRows(): HTMLTableRowElement[] {
+ const rows = this.getDiffRows();
+ return rows.map(r => r.getTableRow()).filter(isDefined);
+ }
+
+ getLineNumEls(side: Side): HTMLTableCellElement[] {
+ const rows = this.getDiffRows();
+ return rows.map(r => r.getLineNumberCell(side)).filter(isDefined);
+ }
+
+ /** This is used when layers initiate an update. */
+ renderContentByRange(start: LineNumber, end: LineNumber, side: Side) {
+ const groups = this.getGroupsByLineRange(start, end, side);
+ for (const group of groups) {
+ const section = this.findSection(group);
+ for (const row of section?.getDiffRows() ?? []) {
+ row.requestUpdate();
+ }
+ }
+ }
+
+ private findSection(group: GrDiffGroup): GrDiffSection | undefined {
+ const leftClass = `left-${group.startLine(Side.LEFT)}`;
+ const rightClass = `right-${group.startLine(Side.RIGHT)}`;
+ return (
+ this.outputEl.querySelector<GrDiffSection>(
+ `gr-diff-section.${leftClass}.${rightClass}`
+ ) ?? undefined
+ );
+ }
+
+ buildSectionElement(group: GrDiffGroup): HTMLElement {
+ const leftCl = `left-${group.startLine(Side.LEFT)}`;
+ const rightCl = `right-${group.startLine(Side.RIGHT)}`;
+ const section = html`
+ <gr-diff-section
+ class="${leftCl} ${rightCl}"
+ .group=${group}
+ .diff=${this.diff}
+ .layers=${this.layers}
+ .diffPrefs=${this.prefs}
+ .renderPrefs=${this.renderPrefs}
+ ></gr-diff-section>
+ `;
+ // When using Lit's `render()` method it wants to be in full control of the
+ // element that it renders into, so we let it render into a temp element.
+ // Rendering into the diff table directly would interfere with
+ // `clearDiffContent()`for example.
+ // TODO: Convert <gr-diff> to be fully lit controlled and incorporate this
+ // method into Lit's `render()` cycle.
+ const tempEl = document.createElement('div');
+ render(section, tempEl);
+ const sectionEl = tempEl.firstElementChild as GrDiffSection;
+ return sectionEl;
+ }
+
+ addColumns(outputEl: HTMLElement, lineNumberWidth: number): void {
+ const colgroup = html`
+ <colgroup>
+ <col class=${diffClasses('blame')}></col>
+ ${when(
+ this.renderPrefs?.view_mode === DiffViewMode.UNIFIED,
+ () => html` ${this.renderUnifiedColumns(lineNumberWidth)} `,
+ () => html`
+ ${this.renderSideBySideColumns(Side.LEFT, lineNumberWidth)}
+ ${this.renderSideBySideColumns(Side.RIGHT, lineNumberWidth)}
+ `
+ )}
+ </colgroup>
+ `;
+ // When using Lit's `render()` method it wants to be in full control of the
+ // element that it renders into, so we let it render into a temp element.
+ // Rendering into the diff table directly would interfere with
+ // `clearDiffContent()`for example.
+ // TODO: Convert <gr-diff> to be fully lit controlled and incorporate this
+ // method into Lit's `render()` cycle.
+ const tempEl = document.createElement('div');
+ render(colgroup, tempEl);
+ const colgroupEl = tempEl.firstElementChild as HTMLElement;
+ outputEl.appendChild(colgroupEl);
+ }
+
+ private renderUnifiedColumns(lineNumberWidth: number) {
+ return html`
+ <col class=${diffClasses()} width=${lineNumberWidth}></col>
+ <col class=${diffClasses()} width=${lineNumberWidth}></col>
+ <col class=${diffClasses()}></col>
+ `;
+ }
+
+ private renderSideBySideColumns(side: Side, lineNumberWidth: number) {
+ return html`
+ <col class=${diffClasses(side)} width=${lineNumberWidth}></col>
+ <col class=${diffClasses(side, 'sign')}></col>
+ <col class=${diffClasses(side)}></col>
+ `;
+ }
+
/**
* This is meant to be called when the gr-diff component re-connects, or when
* the diff is (re-)rendered.
@@ -172,10 +267,6 @@
}
}
- abstract addColumns(outputEl: HTMLElement, fontSize: number): void;
-
- protected abstract buildSectionElement(group: GrDiffGroup): HTMLElement;
-
addGroups(groups: readonly GrDiffGroup[]) {
for (const group of groups) {
this.groups.push(group);
@@ -238,151 +329,19 @@
.filter(group => group.lines.length > 0);
}
- // TODO: Change `null` to `undefined`.
- abstract getContentTdByLine(
- lineNumber: LineNumber,
- side?: Side,
- root?: Element
- ): HTMLTableCellElement | null;
-
- // TODO: Change `null` to `undefined`.
- abstract getLineElByNumber(
- lineNumber: LineNumber,
- side?: Side
- ): HTMLTableCellElement | null;
-
- abstract getLineNumberRows(): HTMLTableRowElement[];
-
- abstract getLineNumEls(side: Side): HTMLTableCellElement[];
-
- protected abstract getBlameTdByLine(lineNum: number): Element | undefined;
-
- // TODO: Change `null` to `undefined`.
- protected abstract getContentByLine(
- lineNumber: LineNumber,
- side?: Side,
- root?: HTMLElement
- ): HTMLElement | null;
-
- /**
- * Find line elements or line objects by a range of line numbers and a side.
- *
- * @param start The first line number
- * @param end The last line number
- * @param side The side of the range. Either 'left' or 'right'.
- * @param out_lines The output list of line objects.
- * TODO: Change to camelCase.
- * @param out_elements The output list of line elements.
- * TODO: Change to camelCase.
- */
- // visible for testing
- findLinesByRange(
- start: LineNumber,
- end: LineNumber,
- side: Side,
- out_lines: GrDiffLine[],
- out_elements: HTMLElement[]
- ) {
- const groups = this.getGroupsByLineRange(start, end, side);
- for (const group of groups) {
- let content: HTMLElement | null = null;
- for (const line of group.lines) {
- if (
- (side === 'left' && line.type === GrDiffLineType.ADD) ||
- (side === 'right' && line.type === GrDiffLineType.REMOVE)
- ) {
- continue;
- }
- const lineNumber =
- side === 'left' ? line.beforeNumber : line.afterNumber;
- if (lineNumber < start || lineNumber > end) {
- continue;
- }
-
- if (content) {
- content = this.getNextContentOnSide(content, side);
- } else {
- content = this.getContentByLine(lineNumber, side, group.element);
- }
- if (content) {
- // out_lines and out_elements must match. So if we don't have an
- // element to push, then also don't push a line.
- out_lines.push(line);
- out_elements.push(content);
- }
- }
- }
- assert(
- out_lines.length === out_elements.length,
- 'findLinesByRange: lines and elements arrays must have same length'
- );
- }
-
- protected abstract renderContentByRange(
- start: LineNumber,
- end: LineNumber,
- side: Side
- ): void;
-
- protected abstract renderBlameByRange(
- blame: BlameInfo,
- start: number,
- end: number
- ): void;
-
- /**
- * Finds the next DIV.contentText element following the given element, and on
- * the same side. Will only search within a group.
- *
- * TODO: Change `null` to `undefined`.
- */
- protected abstract getNextContentOnSide(
- content: HTMLElement,
- side: Side
- ): HTMLElement | null;
-
- /**
- * Gets configuration for creating move controls for chunks marked with
- * dueToMove
- */
- protected abstract getMoveControlsConfig(): {
- numberOfCells: number;
- movedOutIndex: number;
- movedInIndex: number;
- lineNumberCols: number[];
- signCols?: {left: number; right: number};
- };
-
/**
* Set the blame information for the diff. For any already-rendered line,
* re-render its blame cell content.
*/
setBlame(blame: BlameInfo[]) {
- this.blameInfo = blame;
- for (const commit of blame) {
- for (const range of commit.ranges) {
- this.renderBlameByRange(commit, range.start, range.end);
- }
- }
- }
-
- /**
- * Given a base line number, return the commit containing that line in the
- * current set of blame information. If no blame information has been
- * provided, null is returned.
- *
- * @return The commit information.
- */
- // visible for testing
- getBlameCommitForBaseLine(lineNum: LineNumber): BlameInfo | undefined {
- for (const blameCommit of this.blameInfo) {
- for (const range of blameCommit.ranges) {
- if (range.start <= lineNum && range.end >= lineNum) {
- return blameCommit;
+ for (const blameInfo of blame) {
+ for (const range of blameInfo.ranges) {
+ for (let line = range.start; line <= range.end; line++) {
+ const row = this.findRow(line, Side.LEFT);
+ if (row) row.blameInfo = blameInfo;
}
}
}
- return undefined;
}
/**
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-highlight/gr-diff-highlight.ts b/polygerrit-ui/app/embed/diff/gr-diff-highlight/gr-diff-highlight.ts
index 02f2233..69c0f5c 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff-highlight/gr-diff-highlight.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff-highlight/gr-diff-highlight.ts
@@ -44,7 +44,7 @@
* fully blown dependency on GrDiffBuilderElement.
*/
export interface DiffBuilderInterface {
- getContentTdByLineEl(lineEl?: Element): Element | null;
+ getContentTdByLineEl(lineEl?: Element): Element | undefined;
}
/**
diff --git a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff-group.ts b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff-group.ts
index 3b2fa81..2ec0a2e 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff-group.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff-group.ts
@@ -506,16 +506,8 @@
return Promise.resolve();
}
assertIsDefined(this.element);
- // This is a temporary hack while migration to lit based diff rendering:
- // Elements with 'display: contents;' do not have a height, so they
- // won't work as intended with `untilRendered()`.
- const isLitDiff = this.element.tagName === 'GR-DIFF-SECTION';
- if (isLitDiff) {
- await (this.element as LitElement).updateComplete;
- await untilRendered(this.element.firstElementChild as HTMLElement);
- } else {
- await untilRendered(this.element);
- }
+ await (this.element as LitElement).updateComplete;
+ await untilRendered(this.element.firstElementChild as HTMLElement);
}
/**
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 1c5d594..3d54690 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff.ts
@@ -413,11 +413,16 @@
.image-diff .right.lineNumButton {
border-left: 1px solid var(--border-color);
}
- .image-diff label,
- .binary-diff label {
+ .image-diff label {
font-family: var(--font-family);
font-style: italic;
}
+ tbody.binary-diff td {
+ font-family: var(--font-family);
+ font-style: italic;
+ text-align: center;
+ padding: var(--spacing-s) 0;
+ }
.diff-row {
outline: none;
user-select: none;
diff --git a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff_test.ts b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff_test.ts
index 227ac2d..f0826ad 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff_test.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff_test.ts
@@ -3142,18 +3142,25 @@
element,
/* HTML */ `
<div class="diffContainer sideBySide">
+ <gr-diff-section class="left-FILE right-FILE"> </gr-diff-section>
+ <gr-diff-row class="left-FILE right-FILE"> </gr-diff-row>
<table class="selected-right" id="diffTable">
<colgroup>
<col class="blame gr-diff" />
- <col class="gr-diff" width="48" />
- <col class="gr-diff" width="48" />
- <col class="gr-diff" />
+ <col class="gr-diff left" width="48" />
+ <col class="gr-diff left sign" />
+ <col class="gr-diff left" />
+ <col class="gr-diff right" width="48" />
+ <col class="gr-diff right sign" />
+ <col class="gr-diff right" />
</colgroup>
<tbody class="binary-diff gr-diff"></tbody>
- <tbody class="binary-diff gr-diff">
+ <tbody class="both gr-diff section">
<tr
- aria-labelledby="left-button-FILE right-button-FILE right-content-FILE"
- class="both diff-row gr-diff unified"
+ aria-labelledby="left-button-FILE left-content-FILE right-button-FILE right-content-FILE"
+ class="diff-row gr-diff side-by-side"
+ left-type="both"
+ right-type="both"
tabindex="-1"
>
<td class="blame gr-diff" data-line-number="FILE"></td>
@@ -3168,6 +3175,14 @@
File
</button>
</td>
+ <td class="gr-diff left no-intraline-info sign"></td>
+ <td
+ class="both content file gr-diff left no-intraline-info"
+ >
+ <div class="thread-group" data-side="left">
+ <slot name="left-FILE"> </slot>
+ </div>
+ </td>
<td class="gr-diff lineNum right" data-value="FILE">
<button
aria-label="Add file comment"
@@ -3179,19 +3194,23 @@
File
</button>
</td>
+ <td class="gr-diff no-intraline-info right sign"></td>
<td
class="both content file gr-diff no-intraline-info right"
>
- <div>
- <span> Difference in binary files </span>
- </div>
<div class="thread-group" data-side="right">
<slot name="right-FILE"> </slot>
- <slot name="left-FILE"> </slot>
</div>
</td>
</tr>
</tbody>
+ <tbody class="binary-diff gr-diff">
+ <tr class="gr-diff">
+ <td class="gr-diff" colspan="5">
+ <span> Difference in binary files </span>
+ </td>
+ </tr>
+ </tbody>
</table>
</div>
`
diff --git a/polygerrit-ui/app/models/change/change-model.ts b/polygerrit-ui/app/models/change/change-model.ts
index 6d111aa..f5d7fef 100644
--- a/polygerrit-ui/app/models/change/change-model.ts
+++ b/polygerrit-ui/app/models/change/change-model.ts
@@ -281,6 +281,12 @@
viewModelState?.patchNum || latestPatchN
);
+ /** The user can enter edit mode without an `EDIT` patchset existing yet. */
+ public readonly editMode$ = select(
+ combineLatest([this.viewModel.edit$, this.patchNum$]),
+ ([edit, patchNum]) => !!edit || patchNum === EDIT
+ );
+
/**
* Emits the base patchset number. This is identical to the
* `viewModel.basePatchNum$`, but has some special logic for merges.
diff --git a/polygerrit-ui/app/models/config/config-model.ts b/polygerrit-ui/app/models/config/config-model.ts
index 168e0f4..4c0df60 100644
--- a/polygerrit-ui/app/models/config/config-model.ts
+++ b/polygerrit-ui/app/models/config/config-model.ts
@@ -34,6 +34,11 @@
configState => configState.serverConfig
);
+ public download$ = select(
+ this.serverConfig$,
+ serverConfig => serverConfig?.download
+ );
+
public mergeabilityComputationBehavior$ = select(
this.serverConfig$,
serverConfig => serverConfig?.change?.mergeability_computation_behavior
diff --git a/polygerrit-ui/app/services/flags/flags.ts b/polygerrit-ui/app/services/flags/flags.ts
index 973960e..7488e79 100644
--- a/polygerrit-ui/app/services/flags/flags.ts
+++ b/polygerrit-ui/app/services/flags/flags.ts
@@ -19,5 +19,4 @@
PUSH_NOTIFICATIONS_DEVELOPER = 'UiFeature__push_notifications_developer',
PUSH_NOTIFICATIONS = 'UiFeature__push_notifications',
SUGGEST_EDIT = 'UiFeature__suggest_edit',
- COMMENTS_CHIPS_IN_FILE_LIST = 'UiFeature__comments_chips_in_file_list',
}
diff --git a/polygerrit-ui/app/services/gr-reporting/gr-reporting.ts b/polygerrit-ui/app/services/gr-reporting/gr-reporting.ts
index 49259b4..9b08a6e 100644
--- a/polygerrit-ui/app/services/gr-reporting/gr-reporting.ts
+++ b/polygerrit-ui/app/services/gr-reporting/gr-reporting.ts
@@ -34,6 +34,7 @@
appStarted(): void;
onVisibilityChange(): void;
+ onFocusChange(): void;
beforeLocationChanged(): void;
locationChanged(page: string): void;
dashboardDisplayed(): void;
diff --git a/polygerrit-ui/app/services/gr-reporting/gr-reporting_impl.ts b/polygerrit-ui/app/services/gr-reporting/gr-reporting_impl.ts
index 55bb64c..b8758e2 100644
--- a/polygerrit-ui/app/services/gr-reporting/gr-reporting_impl.ts
+++ b/polygerrit-ui/app/services/gr-reporting/gr-reporting_impl.ts
@@ -185,6 +185,12 @@
document.addEventListener('visibilitychange', () => {
reportingService.onVisibilityChange();
});
+ window.addEventListener('blur', () => {
+ reportingService.onFocusChange();
+ });
+ window.addEventListener('focus', () => {
+ reportingService.onFocusChange();
+ });
}
export function initClickReporter(reportingService: ReportingService) {
@@ -203,6 +209,62 @@
});
}
+/**
+ * Reports generic user interaction every x seconds to detect, if the user is
+ * present and is using the application somehow. If you just look at
+ * `document.visibilityState`, then the user may have left the browser open
+ * without locking the screen. So it helps to know whether some interaction is
+ * actually happening.
+ */
+export class InteractionReporter implements Finalizable {
+ /** Accumulates event names until the next round of interaction reporting. */
+ private interactionEvents = new Set<string>();
+
+ /** Allows clearing the interval timer. Mostly useful for tests. */
+ private intervalId?: number;
+
+ constructor(
+ private readonly reportingService: ReportingService,
+ private readonly reportingIntervalMs = 10 * 1000
+ ) {
+ const events = ['mousemove', 'scroll', 'wheel', 'keydown', 'pointerdown'];
+ for (const eventName of events) {
+ document.addEventListener(eventName, () =>
+ this.interactionEvents.add(eventName)
+ );
+ }
+
+ this.intervalId = window.setInterval(
+ () => this.report(),
+ this.reportingIntervalMs
+ );
+ }
+
+ finalize() {
+ window.clearInterval(this.intervalId);
+ }
+
+ private report() {
+ const active = this.interactionEvents.size > 0;
+ if (active) {
+ this.reportingService.reportInteraction(Interaction.USER_ACTIVE, {
+ events: [...this.interactionEvents],
+ });
+ } else if (document.visibilityState === 'visible') {
+ this.reportingService.reportInteraction(Interaction.USER_PASSIVE, {});
+ }
+ this.interactionEvents.clear();
+ }
+}
+
+let interactionReporter: InteractionReporter;
+
+export function initInteractionReporter(reportingService: ReportingService) {
+ if (!interactionReporter) {
+ interactionReporter = new InteractionReporter(reportingService);
+ }
+}
+
export function initWebVitals(reportingService: ReportingService) {
function reportWebVitalMetric(name: Timing, metric: Metric) {
let score = metric.value;
@@ -470,6 +532,20 @@
this._reportNavResTimes();
}
+ onFocusChange() {
+ this.reporter(
+ LIFECYCLE.TYPE,
+ LIFECYCLE.CATEGORY.VISIBILITY,
+ LifeCycle.FOCUS,
+ undefined,
+ {
+ isVisible: document.visibilityState === 'visible',
+ hasFocus: document.hasFocus(),
+ },
+ false
+ );
+ }
+
onVisibilityChange() {
this.hiddenDurationTimer.onVisibilityChange();
let eventName;
@@ -486,6 +562,8 @@
undefined,
{
hiddenDurationMs: this.hiddenDurationTimer.hiddenDurationMs,
+ isVisible: document.visibilityState === 'visible',
+ hasFocus: document.hasFocus(),
},
false
);
diff --git a/polygerrit-ui/app/services/gr-reporting/gr-reporting_mock.ts b/polygerrit-ui/app/services/gr-reporting/gr-reporting_mock.ts
index d4efbcc..fd1b88c 100644
--- a/polygerrit-ui/app/services/gr-reporting/gr-reporting_mock.ts
+++ b/polygerrit-ui/app/services/gr-reporting/gr-reporting_mock.ts
@@ -44,6 +44,9 @@
onVisibilityChange: () => {
log('onVisibilityChange');
},
+ onFocusChange: () => {
+ log('onFocusChange');
+ },
pluginLoaded: () => {},
pluginsLoaded: () => {},
pluginsFailed: () => {},
diff --git a/polygerrit-ui/app/services/gr-reporting/gr-reporting_test.ts b/polygerrit-ui/app/services/gr-reporting/gr-reporting_test.ts
index 9c5e20d..2d3dfe2 100644
--- a/polygerrit-ui/app/services/gr-reporting/gr-reporting_test.ts
+++ b/polygerrit-ui/app/services/gr-reporting/gr-reporting_test.ts
@@ -8,11 +8,14 @@
GrReporting,
DEFAULT_STARTUP_TIMERS,
initErrorReporter,
+ InteractionReporter,
} from './gr-reporting_impl';
import {getAppContext} from '../app-context';
import {Deduping} from '../../api/reporting';
-import {SinonFakeTimers} from 'sinon';
+import {SinonFakeTimers, SinonStub} from 'sinon';
import {assert} from '@open-wc/testing';
+import {grReportingMock} from './gr-reporting_mock';
+import {Interaction} from '../../constants/reporting';
suite('gr-reporting tests', () => {
// We have to type as any because we access
@@ -563,3 +566,62 @@
});
});
});
+
+suite('InteractionReporter', () => {
+ let interactionReporter: InteractionReporter;
+ let clock: SinonFakeTimers;
+ let stub: SinonStub;
+ let activeCalls: number[] = [];
+ let passiveCalls: number[] = [];
+
+ setup(() => {
+ clock = sinon.useFakeTimers(0);
+ activeCalls = [];
+ passiveCalls = [];
+ const reporting = grReportingMock;
+ stub = sinon
+ .stub(reporting, 'reportInteraction')
+ .callsFake((interaction: string | Interaction) => {
+ if (interaction === Interaction.USER_ACTIVE) {
+ activeCalls.push(clock.now);
+ }
+ if (interaction === Interaction.USER_PASSIVE) {
+ passiveCalls.push(clock.now);
+ }
+ });
+ interactionReporter = new InteractionReporter(reporting, 1000);
+ });
+
+ teardown(() => {
+ clock.restore();
+ interactionReporter.finalize();
+ });
+
+ test('interaction example', () => {
+ clock.tick(500);
+ clock.tick(1000);
+ document.dispatchEvent(new MouseEvent('mousemove'));
+ clock.tick(1000);
+ document.dispatchEvent(new MouseEvent('mousemove'));
+ document.dispatchEvent(new MouseEvent('scroll'));
+ document.dispatchEvent(new MouseEvent('wheel'));
+ clock.tick(1000);
+ clock.tick(1000);
+ clock.tick(1000);
+ document.dispatchEvent(new MouseEvent('mousemove'));
+ clock.tick(1000);
+ document.dispatchEvent(new MouseEvent('mousemove'));
+ clock.tick(1000);
+
+ assert.sameOrderedMembers(activeCalls, [2000, 3000, 6000, 7000]);
+ assert.sameOrderedMembers(passiveCalls, [1000, 4000, 5000]);
+
+ assert.isUndefined(stub.getCall(0).args[1].events);
+ assert.sameMembers(stub.getCall(1).args[1].events, ['mousemove']);
+ assert.sameMembers(stub.getCall(2).args[1].events, [
+ 'mousemove',
+ 'scroll',
+ 'wheel',
+ ]);
+ });
+});
diff --git a/polygerrit-ui/app/utils/change-util.ts b/polygerrit-ui/app/utils/change-util.ts
index 4490afa..7de5e7e 100644
--- a/polygerrit-ui/app/utils/change-util.ts
+++ b/polygerrit-ui/app/utils/change-util.ts
@@ -16,10 +16,8 @@
import {ParsedChangeInfo} from '../types/types';
import {getUserId, isServiceUser} from './account-util';
-// This can be wrong! See WARNING above
interface ChangeStatusesOptions {
- mergeable: boolean; // This can be wrong! See WARNING above
- submitEnabled: boolean; // This can be wrong! See WARNING above
+ mergeable: boolean;
/** Is there a reverting change and if so, what status has it? */
revertingChangeStatus?: ChangeStatus;
}
@@ -190,11 +188,9 @@
return states;
}
- // If no missing requirements, either active or ready to submit.
- if (change.submittable && options.submitEnabled) {
+ if (change.submittable) {
states.push(ChangeStates.READY_TO_SUBMIT);
} else {
- // Otherwise it is active.
states.push(ChangeStates.ACTIVE);
}
return states;
diff --git a/polygerrit-ui/app/utils/change-util_test.ts b/polygerrit-ui/app/utils/change-util_test.ts
index f768145..1782d80 100644
--- a/polygerrit-ui/app/utils/change-util_test.ts
+++ b/polygerrit-ui/app/utils/change-util_test.ts
@@ -66,29 +66,24 @@
assert.deepEqual(statuses, []);
change.submittable = false;
- statuses = changeStatuses(change, {mergeable: true, submitEnabled: false});
+ statuses = changeStatuses(change, {mergeable: true});
assert.deepEqual(statuses, [ChangeStates.ACTIVE]);
- // With no missing labels but no submitEnabled option.
change.submittable = true;
- statuses = changeStatuses(change, {mergeable: true, submitEnabled: false});
- assert.deepEqual(statuses, [ChangeStates.ACTIVE]);
-
- // Without missing labels and enabled submit
- statuses = changeStatuses(change, {mergeable: true, submitEnabled: true});
+ statuses = changeStatuses(change, {mergeable: true});
assert.deepEqual(statuses, [ChangeStates.READY_TO_SUBMIT]);
change.mergeable = false;
change.submittable = true;
- statuses = changeStatuses(change, {mergeable: false, submitEnabled: false});
+ statuses = changeStatuses(change, {mergeable: false});
assert.deepEqual(statuses, [ChangeStates.MERGE_CONFLICT]);
change.mergeable = true;
- statuses = changeStatuses(change, {mergeable: true, submitEnabled: true});
+ statuses = changeStatuses(change, {mergeable: true});
assert.deepEqual(statuses, [ChangeStates.READY_TO_SUBMIT]);
change.submittable = true;
- statuses = changeStatuses(change, {mergeable: false, submitEnabled: false});
+ statuses = changeStatuses(change, {mergeable: false});
assert.deepEqual(statuses, [ChangeStates.MERGE_CONFLICT]);
});
@@ -141,7 +136,6 @@
changeStatuses(change, {
revertingChangeStatus: ChangeStatus.NEW,
mergeable: true,
- submitEnabled: true,
}),
[ChangeStates.MERGED, ChangeStates.REVERT_CREATED]
);
@@ -149,7 +143,6 @@
changeStatuses(change, {
revertingChangeStatus: ChangeStatus.MERGED,
mergeable: true,
- submitEnabled: true,
}),
[ChangeStates.MERGED, ChangeStates.REVERT_SUBMITTED]
);
diff --git a/polygerrit-ui/app/utils/comment-util.ts b/polygerrit-ui/app/utils/comment-util.ts
index 4646bf6..ee1a44c 100644
--- a/polygerrit-ui/app/utils/comment-util.ts
+++ b/polygerrit-ui/app/utils/comment-util.ts
@@ -476,13 +476,17 @@
return comment.message?.includes(USER_SUGGESTION_START_PATTERN) ?? false;
}
+export function getUserSuggestionFromString(content: string) {
+ const start =
+ content.indexOf(USER_SUGGESTION_START_PATTERN) +
+ USER_SUGGESTION_START_PATTERN.length;
+ const end = content.indexOf('\n```', start);
+ return content.substring(start, end);
+}
+
export function getUserSuggestion(comment: Comment) {
if (!comment.message) return;
- const start =
- comment.message.indexOf(USER_SUGGESTION_START_PATTERN) +
- USER_SUGGESTION_START_PATTERN.length;
- const end = comment.message.indexOf('\n```', start);
- return comment.message.substring(start, end);
+ return getUserSuggestionFromString(comment.message);
}
export function getContentInCommentRange(
diff --git a/polygerrit-ui/app/utils/url-util.ts b/polygerrit-ui/app/utils/url-util.ts
index 4fa0e63..6ceaa4f 100644
--- a/polygerrit-ui/app/utils/url-util.ts
+++ b/polygerrit-ui/app/utils/url-util.ts
@@ -149,3 +149,9 @@
export function generateAbsoluteUrl(url: string) {
return new URL(url, window.location.href).toString();
}
+
+export function sameOrigin(href: string) {
+ if (!href) return false;
+ const url = new URL(href, window.location.origin);
+ return url.origin === window.location.origin;
+}
diff --git a/polygerrit-ui/app/utils/url-util_test.ts b/polygerrit-ui/app/utils/url-util_test.ts
index c82cd70..e2ff837 100644
--- a/polygerrit-ui/app/utils/url-util_test.ts
+++ b/polygerrit-ui/app/utils/url-util_test.ts
@@ -14,6 +14,7 @@
toSearchParams,
getPatchRangeExpression,
PatchRangeParams,
+ sameOrigin,
} from './url-util';
import {assert} from '@open-wc/testing';
@@ -69,6 +70,12 @@
});
});
+ test('sameOrigin', () => {
+ assert.isTrue(sameOrigin('/asdf'));
+ assert.isTrue(sameOrigin(window.location.origin + '/asdf'));
+ assert.isFalse(sameOrigin('http://www.goole.com/asdf'));
+ });
+
test('toPathname', () => {
assert.equal(toPathname('asdf'), 'asdf');
assert.equal(toPathname('asdf?qwer=zxcv'), 'asdf');
diff --git a/proto/cache.proto b/proto/cache.proto
index 7063ee5..79bfa9e 100644
--- a/proto/cache.proto
+++ b/proto/cache.proto
@@ -78,7 +78,7 @@
// Instead, we just take the tedious yet simple approach of having a "has_foo"
// field for each nullable field "foo", indicating whether or not foo is null.
//
-// Next ID: 28
+// Next ID: 29
message ChangeNotesStateProto {
// Effectively required, even though the corresponding ChangeNotesState field
// is optional, since the field is only absent when NoteDb is disabled, in
@@ -142,6 +142,8 @@
repeated string hashtag = 5;
+ map<string, string> custom_keyed_values = 28;
+
repeated devtools.gerritcodereview.PatchSet patch_set = 6;
repeated devtools.gerritcodereview.PatchSetApproval approval = 7;
diff --git a/tools/nongoogle.bzl b/tools/nongoogle.bzl
index 7f26ef3..a03ccf0 100644
--- a/tools/nongoogle.bzl
+++ b/tools/nongoogle.bzl
@@ -171,24 +171,24 @@
sha1 = GUAVA_TESTLIB_BIN_SHA1,
)
- GUICE_VERS = "5.0.1"
+ GUICE_VERS = "5.1.0"
maven_jar(
name = "guice-library",
artifact = "com.google.inject:guice:" + GUICE_VERS,
- sha1 = "0dae7556b441cada2b4f0a2314eb68e1ff423429",
+ sha1 = "da25056c694c54ba16e78e4fc35f17fc60f0d1b4",
)
maven_jar(
name = "guice-assistedinject",
artifact = "com.google.inject.extensions:guice-assistedinject:" + GUICE_VERS,
- sha1 = "62e02f2aceb7d90ba354584dacc018c1e94ff01c",
+ sha1 = "58a8956f00d6939978d7da735f393d7af7db5c02",
)
maven_jar(
name = "guice-servlet",
artifact = "com.google.inject.extensions:guice-servlet:" + GUICE_VERS,
- sha1 = "f527009d51f172a2e6937bfb55fcb827e2e2386b",
+ sha1 = "cb89ddec4246a469698a3461e69de1f245016c5d",
)
# Keep this version of Soy synchronized with the version used in Gitiles.
@@ -243,36 +243,36 @@
sha1 = "64cba89cf87c1d84cb8c81d06f0b9c482f10b4dc",
)
- LUCENE_VERS = "7.7.3"
+ LUCENE_VERS = "8.11.2"
maven_jar(
name = "lucene-core",
artifact = "org.apache.lucene:lucene-core:" + LUCENE_VERS,
- sha1 = "5faa5ae56f7599019fce6184accc6c968b7519e7",
+ sha1 = "57438c3f31e0e440de149294890eee88e030ea6d",
)
maven_jar(
name = "lucene-analyzers-common",
artifact = "org.apache.lucene:lucene-analyzers-common:" + LUCENE_VERS,
- sha1 = "0a76cbf5e21bbbb0c2d6288b042450236248214e",
+ sha1 = "07a74c5c2dd082b08c644a9016bc6ff66c8f27cc",
)
maven_jar(
name = "backward-codecs",
artifact = "org.apache.lucene:lucene-backward-codecs:" + LUCENE_VERS,
- sha1 = "40207d0dd023a0e2868a23dd87d72f1a3cdbb893",
+ sha1 = "a5d0f0db405d607cc13265819b8d2ef0c81c0819",
)
maven_jar(
name = "lucene-misc",
artifact = "org.apache.lucene:lucene-misc:" + LUCENE_VERS,
- sha1 = "3aca078edf983059722fe61a81b7b7bd5ecdb222",
+ sha1 = "9c7204f923465a96a20ac9e49cdca0cfcde64851",
)
maven_jar(
name = "lucene-queryparser",
artifact = "org.apache.lucene:lucene-queryparser:" + LUCENE_VERS,
- sha1 = "685fc6166d29eb3e3441ae066873bb442aa02df1",
+ sha1 = "1886e3a27a8d4a73eb8fad54ea93a160b099bc60",
)
# JGit's transitive dependencies
