Merge "ProjectsCollection: remove @Singleton"
diff --git a/WORKSPACE b/WORKSPACE
index 2244dca..0767907 100644
--- a/WORKSPACE
+++ b/WORKSPACE
@@ -66,17 +66,17 @@
http_archive(
name = "build_bazel_rules_nodejs",
- sha256 = "dd4dc46066e2ce034cba0c81aa3e862b27e8e8d95871f567359f7a534cccb666",
- urls = ["https://github.com/bazelbuild/rules_nodejs/releases/download/3.1.0/rules_nodejs-3.1.0.tar.gz"],
+ sha256 = "fcc6dccb39ca88d481224536eb8f9fa754619676c6163f87aa6af94059b02b12",
+ urls = ["https://github.com/bazelbuild/rules_nodejs/releases/download/3.2.0/rules_nodejs-3.2.0.tar.gz"],
)
# Golang support for PolyGerrit local dev server.
http_archive(
name = "io_bazel_rules_go",
- sha256 = "a8d6b1b354d371a646d2f7927319974e0f9e52f73a2452d2b3877118169eb6bb",
+ sha256 = "4d838e2d70b955ef9dd0d0648f673141df1bc1d7ecf5c2d621dcc163f47dd38a",
urls = [
- "https://mirror.bazel.build/github.com/bazelbuild/rules_go/releases/download/v0.23.3/rules_go-v0.23.3.tar.gz",
- "https://github.com/bazelbuild/rules_go/releases/download/v0.23.3/rules_go-v0.23.3.tar.gz",
+ "https://mirror.bazel.build/github.com/bazelbuild/rules_go/releases/download/v0.24.12/rules_go-v0.24.12.tar.gz",
+ "https://github.com/bazelbuild/rules_go/releases/download/v0.24.12/rules_go-v0.24.12.tar.gz",
],
)
@@ -88,10 +88,10 @@
http_archive(
name = "bazel_gazelle",
- sha256 = "cdb02a887a7187ea4d5a27452311a75ed8637379a1287d8eeb952138ea485f7d",
+ sha256 = "222e49f034ca7a1d1231422cdb67066b885819885c356673cb1f72f748a3c9d4",
urls = [
- "https://mirror.bazel.build/github.com/bazelbuild/bazel-gazelle/releases/download/v0.21.1/bazel-gazelle-v0.21.1.tar.gz",
- "https://github.com/bazelbuild/bazel-gazelle/releases/download/v0.21.1/bazel-gazelle-v0.21.1.tar.gz",
+ "https://mirror.bazel.build/github.com/bazelbuild/bazel-gazelle/releases/download/v0.22.3/bazel-gazelle-v0.22.3.tar.gz",
+ "https://github.com/bazelbuild/bazel-gazelle/releases/download/v0.22.3/bazel-gazelle-v0.22.3.tar.gz",
],
)
diff --git a/java/com/google/gerrit/entities/RefNames.java b/java/com/google/gerrit/entities/RefNames.java
index b6efcbf..4272960 100644
--- a/java/com/google/gerrit/entities/RefNames.java
+++ b/java/com/google/gerrit/entities/RefNames.java
@@ -140,7 +140,7 @@
}
/**
- * Warning: Change refs have to manually be advertised in {@link
+ * Warning: Change refs have to manually be advertised in {@code
* com.google.gerrit.server.permissions.DefaultRefFilter}; this should be done when adding new
* change refs.
*/
@@ -180,6 +180,16 @@
return ref.startsWith(REFS_CHANGES);
}
+ /** True if the provided ref is in {@code refs/changes/../meta}. */
+ public static boolean isRefsMetaChanges(String ref) {
+ return ref.startsWith(REFS_CHANGES) && ref.endsWith(META_SUFFIX);
+ }
+
+ /** True if the provided ref is in {@code refs/changes/../robot-comments}. */
+ public static boolean isRobotCommentMetaRef(String ref) {
+ return ref.startsWith(REFS_CHANGES) && ref.endsWith(ROBOT_COMMENTS_SUFFIX);
+ }
+
public static String refsGroups(AccountGroup.UUID groupUuid) {
return REFS_GROUPS + shardUuid(groupUuid.get());
}
diff --git a/java/com/google/gerrit/exceptions/InternalServerWithUserMessageException.java b/java/com/google/gerrit/exceptions/InternalServerWithUserMessageException.java
index 9f74e9d..452192c 100644
--- a/java/com/google/gerrit/exceptions/InternalServerWithUserMessageException.java
+++ b/java/com/google/gerrit/exceptions/InternalServerWithUserMessageException.java
@@ -15,6 +15,8 @@
package com.google.gerrit.exceptions;
public class InternalServerWithUserMessageException extends RuntimeException {
+ private static final long serialVersionUID = 1L;
+
public InternalServerWithUserMessageException(String msg, Throwable cause) {
super(msg, cause);
}
diff --git a/java/com/google/gerrit/server/change/FileInfoJsonNewImpl.java b/java/com/google/gerrit/server/change/FileInfoJsonNewImpl.java
index 0dd71507..ed046826 100644
--- a/java/com/google/gerrit/server/change/FileInfoJsonNewImpl.java
+++ b/java/com/google/gerrit/server/change/FileInfoJsonNewImpl.java
@@ -48,9 +48,8 @@
if (base == null) {
return asFileInfo(
diffs.listModifiedFilesAgainstParent(change.getProject(), objectId, null));
- } else {
- return asFileInfo(diffs.listModifiedFiles(change.getProject(), base.commitId(), objectId));
}
+ return asFileInfo(diffs.listModifiedFiles(change.getProject(), base.commitId(), objectId));
} catch (DiffNotAvailableException e) {
convertException(e);
return null; // unreachable. handleAndThrow will throw an exception anyway
diff --git a/java/com/google/gerrit/server/change/RebaseChangeOp.java b/java/com/google/gerrit/server/change/RebaseChangeOp.java
index a548262..b43996e 100644
--- a/java/com/google/gerrit/server/change/RebaseChangeOp.java
+++ b/java/com/google/gerrit/server/change/RebaseChangeOp.java
@@ -219,8 +219,13 @@
.setFireRevisionCreated(fireRevisionCreated)
.setCheckAddPatchSetPermission(checkAddPatchSetPermission)
.setValidate(validate)
- .setSendEmail(sendEmail)
- .setWorkInProgress(!rebasedCommit.getFilesWithGitConflicts().isEmpty());
+ .setSendEmail(sendEmail);
+
+ if (!rebasedCommit.getFilesWithGitConflicts().isEmpty()
+ && !notes.getChange().isWorkInProgress()) {
+ patchSetInserter.setWorkInProgress(true);
+ }
+
if (postMessage) {
patchSetInserter.setMessage(
messageForRebasedChange(rebasedPatchSetId, originalPatchSet.id(), rebasedCommit));
diff --git a/java/com/google/gerrit/server/git/SearchingChangeCacheImpl.java b/java/com/google/gerrit/server/git/SearchingChangeCacheImpl.java
index fed6541..c3d4777 100644
--- a/java/com/google/gerrit/server/git/SearchingChangeCacheImpl.java
+++ b/java/com/google/gerrit/server/git/SearchingChangeCacheImpl.java
@@ -17,10 +17,12 @@
import com.google.auto.value.AutoValue;
import com.google.common.cache.CacheLoader;
import com.google.common.cache.LoadingCache;
+import com.google.common.collect.ImmutableList;
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.UsedAt;
import com.google.gerrit.entities.Change;
+import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.entities.Project;
import com.google.gerrit.entities.RefNames;
import com.google.gerrit.extensions.events.GitReferenceUpdatedListener;
@@ -42,6 +44,7 @@
import com.google.inject.name.Named;
import com.google.inject.util.Providers;
import java.util.ArrayList;
+import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.concurrent.ExecutionException;
@@ -96,6 +99,10 @@
@Nullable
abstract ReviewerSet reviewers();
+
+ abstract Collection<PatchSet> patchSets();
+
+ abstract ImmutableList<byte[]> refStates();
}
private final LoadingCache<Project.NameKey, List<CachedChange>> cache;
@@ -125,6 +132,8 @@
for (CachedChange cc : cached) {
ChangeData cd = changeDataFactory.create(cc.change());
cd.setReviewers(cc.reviewers());
+ cd.setPatchSets(cc.patchSets());
+ cd.setRefStates(cc.refStates());
cds.add(cd);
}
return Collections.unmodifiableList(cds);
@@ -160,12 +169,17 @@
List<ChangeData> cds =
queryProvider
.get()
- .setRequestedFields(ChangeField.CHANGE, ChangeField.REVIEWER)
+ .setRequestedFields(
+ ChangeField.CHANGE,
+ ChangeField.REVIEWER,
+ ChangeField.REF_STATE,
+ ChangeField.PATCH_SET)
.byProject(key);
List<CachedChange> result = new ArrayList<>(cds.size());
for (ChangeData cd : cds) {
result.add(
- new AutoValue_SearchingChangeCacheImpl_CachedChange(cd.change(), cd.reviewers()));
+ new AutoValue_SearchingChangeCacheImpl_CachedChange(
+ cd.change(), cd.reviewers(), cd.patchSets(), cd.getRefStates()));
}
return Collections.unmodifiableList(result);
}
diff --git a/java/com/google/gerrit/server/patch/filediff/FileDiffCacheImpl.java b/java/com/google/gerrit/server/patch/filediff/FileDiffCacheImpl.java
index ad61753..d945da3 100644
--- a/java/com/google/gerrit/server/patch/filediff/FileDiffCacheImpl.java
+++ b/java/com/google/gerrit/server/patch/filediff/FileDiffCacheImpl.java
@@ -217,7 +217,7 @@
: createMergeListEntry(
reader, aCommit, bCommit, comparisonType, cmp, key.diffAlgorithm());
} catch (IOException e) {
- logger.atWarning().log("Failed to compute commit entry for key " + key);
+ logger.atWarning().log("Failed to compute commit entry for key %s", key);
}
return FileDiffOutput.empty(key.newFilePath());
}
diff --git a/java/com/google/gerrit/server/permissions/DefaultRefFilter.java b/java/com/google/gerrit/server/permissions/DefaultRefFilter.java
index e88a840..88f91be 100644
--- a/java/com/google/gerrit/server/permissions/DefaultRefFilter.java
+++ b/java/com/google/gerrit/server/permissions/DefaultRefFilter.java
@@ -189,35 +189,42 @@
}
if (opts.returnMostRecentRefChanges()) {
- visibleChangesCache.cachedVisibleChanges().values().stream()
- .forEach(n -> addAllChangeAndPatchsetRefs(visibleRefs, n));
+ visibleChangesCache.cachedVisibleChanges().keySet().stream()
+ .forEach(c -> addAllChangeAndPatchsetRefs(visibleRefs, c));
}
logger.atFinest().log("visible refs = %s", visibleRefs);
return visibleRefs;
}
- private void addAllChangeAndPatchsetRefs(Collection<Ref> refs, ChangeNotes changeNotes) {
- refs.add(
- new ObjectIdRef.PeeledNonTag(
- Storage.PACKED,
- RefNames.changeMetaRef(changeNotes.getChangeId()),
- changeNotes.getMetaId()));
- changeNotes
- .getPatchSets()
- .values()
- .forEach(
- p ->
- refs.add(
- new ObjectIdRef.PeeledNonTag(
- Storage.PACKED, RefNames.patchSetRef(p.id()), p.commitId())));
- if (changeNotes.getRobotCommentNotes() != null
- && changeNotes.getRobotCommentNotes().getMetaId() != null) {
+ private void addAllChangeAndPatchsetRefs(Collection<Ref> refs, Change.Id changeId) {
+ try {
refs.add(
new ObjectIdRef.PeeledNonTag(
Storage.PACKED,
- RefNames.robotCommentsRef(changeNotes.getChangeId()),
- changeNotes.getRobotCommentNotes().getMetaId()));
+ RefNames.changeMetaRef(changeId),
+ visibleChangesCache.getMetaId(changeId)));
+ visibleChangesCache
+ .getPatchSets(changeId)
+ .forEach(
+ p ->
+ refs.add(
+ new ObjectIdRef.PeeledNonTag(
+ Storage.PACKED, RefNames.patchSetRef(p.id()), p.commitId())));
+ if (visibleChangesCache.getRobotCommentsMetaId(changeId) != null) {
+ refs.add(
+ new ObjectIdRef.PeeledNonTag(
+ Storage.PACKED,
+ RefNames.robotCommentsRef(changeId),
+ visibleChangesCache.getRobotCommentsMetaId(changeId)));
+ }
+ } catch (PermissionBackendException ex) {
+ // this can't happen since the changes are all already in the cache, so we don't need to
+ // perform permission checks at all to get those changes.
+ throw new IllegalStateException(
+ "this shouldn't happen since all permission checks should have been "
+ + "done previously, exception: %s",
+ ex);
}
}
@@ -359,7 +366,7 @@
try {
// Default to READ_PRIVATE_CHANGES as there is no special permission for reading edits.
permissionBackendForProject
- .ref(visibleChangesCache.cachedVisibleChanges().get(id).getChange().getDest().branch())
+ .ref(visibleChangesCache.getChange(id).getDest().branch())
.check(RefPermission.READ_PRIVATE_CHANGES);
logger.atFinest().log("Foreign change edit ref is visible: %s", name);
return true;
diff --git a/java/com/google/gerrit/server/permissions/VisibleChangesCache.java b/java/com/google/gerrit/server/permissions/VisibleChangesCache.java
index 6284442..926dde6 100644
--- a/java/com/google/gerrit/server/permissions/VisibleChangesCache.java
+++ b/java/com/google/gerrit/server/permissions/VisibleChangesCache.java
@@ -16,14 +16,18 @@
import static com.google.common.collect.ImmutableList.toImmutableList;
+import com.google.common.collect.HashMultimap;
import com.google.common.collect.ImmutableList;
-import com.google.common.collect.Maps;
+import com.google.common.collect.Multimap;
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.Change;
+import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.entities.Project;
+import com.google.gerrit.entities.RefNames;
import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.extensions.restapi.AuthException;
+import com.google.gerrit.index.RefState;
import com.google.gerrit.server.git.SearchingChangeCacheImpl;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.ChangeNotes.Factory.ChangeNotesResult;
@@ -32,9 +36,10 @@
import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
import java.io.IOException;
-import java.util.Collections;
+import java.util.Collection;
import java.util.HashMap;
import java.util.Map;
+import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Repository;
/**
@@ -54,7 +59,10 @@
private final PermissionBackend.ForProject permissionBackendForProject;
private final Repository repository;
- private Map<Change.Id, ChangeNotes> visibleChanges;
+ private Map<Change.Id, Change> visibleChanges;
+ private Map<Change.Id, ObjectId> metaIds = new HashMap<>();
+ private Map<Change.Id, ObjectId> robotCommentsMetaIds = new HashMap<>();
+ private Multimap<Change.Id, PatchSet> patchSets = HashMultimap.create();
@Inject
VisibleChangesCache(
@@ -76,49 +84,103 @@
* by looking at the cached visible changes.
*/
public boolean isVisible(Change.Id changeId) throws PermissionBackendException {
- return cachedVisibleChanges().containsKey(changeId);
+ cachedVisibleChanges();
+ return visibleChanges.containsKey(changeId);
}
/**
* Returns the visible changes in the repository {@code repo}. If not cached, computes the visible
* changes and caches them.
*/
- public Map<Change.Id, ChangeNotes> cachedVisibleChanges() throws PermissionBackendException {
+ public Map<Change.Id, Change> cachedVisibleChanges() throws PermissionBackendException {
if (visibleChanges == null) {
if (changeCache == null) {
- visibleChanges = visibleChangesByScan();
+ visibleChangesByScan();
} else {
- visibleChanges = visibleChangesBySearch();
+ visibleChangesBySearch();
}
logger.atFinest().log("Visible changes: %s", visibleChanges.keySet());
}
return visibleChanges;
}
- private Map<Change.Id, ChangeNotes> visibleChangesBySearch() throws PermissionBackendException {
+ /**
+ * Returns the change for {@code changeId}. If not cached, computes *all* visible changes and
+ * caches them before returning this specific change. If not visible or not found, returns null.
+ */
+ @Nullable
+ public Change getChange(Change.Id changeId) throws PermissionBackendException {
+ return cachedVisibleChanges().get(changeId);
+ }
+
+ /**
+ * Returns the change's meta id for {@code changeId}. If not cached, computes *all* visible
+ * changes and caches them before returning this specific meta id. If not visible or not found,
+ * returns null.
+ */
+ @Nullable
+ public ObjectId getMetaId(Change.Id changeId) throws PermissionBackendException {
+ cachedVisibleChanges();
+ return metaIds.get(changeId);
+ }
+
+ /**
+ * Returns the change's robot comment meta id for {@code changeId}. If not cached, computes *all*
+ * visible changes and caches them before returning this specific robot comments meta id. If not
+ * visible, not found or there are no robot comments on this change, returns null.
+ */
+ @Nullable
+ public ObjectId getRobotCommentsMetaId(Change.Id changeId) throws PermissionBackendException {
+ cachedVisibleChanges();
+ return robotCommentsMetaIds.get(changeId);
+ }
+
+ /**
+ * Returns the change's patch-sets for {@code changeId}. If not cached, computes *all* visible
+ * changes and caches them before returning this collection of patch-sets. If not visible or not
+ * found, returns an empty collection.
+ */
+ public Collection<PatchSet> getPatchSets(Change.Id changeId) throws PermissionBackendException {
+ cachedVisibleChanges();
+ return patchSets.get(changeId);
+ }
+
+ private void visibleChangesBySearch() throws PermissionBackendException {
+ visibleChanges = new HashMap<>();
Project.NameKey project = projectState.getNameKey();
try {
- Map<Change.Id, ChangeNotes> visibleChanges = new HashMap<>();
for (ChangeData cd : changeCache.getChangeData(project)) {
if (!projectState.statePermitsRead()) {
continue;
}
try {
permissionBackendForProject.change(cd).check(ChangePermission.READ);
- visibleChanges.put(cd.getId(), cd.notes());
+ visibleChanges.put(cd.getId(), cd.change());
+ Collection<RefState> refStates = RefState.parseStates(cd.getRefStates()).values();
+ for (RefState refState : refStates) {
+ if (RefNames.isRobotCommentMetaRef(refState.ref())) {
+ if (!refState.id().equals(ObjectId.zeroId())) {
+ robotCommentsMetaIds.put(cd.getId(), refState.id());
+ }
+ }
+ if (RefNames.isRefsMetaChanges(refState.ref())) {
+ metaIds.put(cd.getId(), refState.id());
+ }
+ }
+ cd.patchSets().stream().forEach(ps -> patchSets.put(cd.getId(), ps));
+
} catch (AuthException e) {
// Do nothing.
}
}
- return visibleChanges;
} catch (StorageException e) {
logger.atSevere().withCause(e).log(
"Cannot load changes for project %s, assuming no changes are visible", project);
- return Collections.emptyMap();
}
}
- private Map<Change.Id, ChangeNotes> visibleChangesByScan() throws PermissionBackendException {
+ private void visibleChangesByScan() throws PermissionBackendException {
+ visibleChanges = new HashMap<>();
Project.NameKey p = projectState.getNameKey();
ImmutableList<ChangeNotesResult> changes;
try {
@@ -126,17 +188,22 @@
} catch (IOException e) {
logger.atSevere().withCause(e).log(
"Cannot load changes for project %s, assuming no changes are visible", p);
- return Collections.emptyMap();
+ return;
}
- Map<Change.Id, ChangeNotes> result = Maps.newHashMapWithExpectedSize(changes.size());
for (ChangeNotesResult notesResult : changes) {
ChangeNotes notes = toNotes(notesResult);
if (notes != null) {
- result.put(notes.getChangeId(), notes);
+ visibleChanges.put(notes.getChangeId(), notes.getChange());
+ metaIds.put(notes.getChangeId(), notes.getMetaId());
+ if (notes.getRobotCommentNotes() != null
+ && notes.getRobotCommentNotes().getMetaId() != null) {
+ robotCommentsMetaIds.put(notes.getChangeId(), notes.getRobotCommentNotes().getMetaId());
+ }
+ notes.getPatchSets().values().stream()
+ .forEach(ps -> patchSets.put(notes.getChangeId(), ps));
}
}
- return result;
}
@Nullable
diff --git a/java/com/google/gerrit/server/restapi/change/PutMessage.java b/java/com/google/gerrit/server/restapi/change/PutMessage.java
index 37318d0..1ed7fd7 100644
--- a/java/com/google/gerrit/server/restapi/change/PutMessage.java
+++ b/java/com/google/gerrit/server/restapi/change/PutMessage.java
@@ -20,7 +20,6 @@
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.extensions.api.changes.NotifyHandling;
import com.google.gerrit.extensions.common.CommitMessageInput;
-import com.google.gerrit.extensions.registration.DynamicItem;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
@@ -34,7 +33,6 @@
import com.google.gerrit.server.change.ChangeResource;
import com.google.gerrit.server.change.NotifyResolver;
import com.google.gerrit.server.change.PatchSetInserter;
-import com.google.gerrit.server.config.UrlFormatter;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.permissions.ChangePermission;
@@ -72,7 +70,6 @@
private final PatchSetUtil psUtil;
private final NotifyResolver notifyResolver;
private final ProjectCache projectCache;
- private final DynamicItem<UrlFormatter> urlFormatter;
@Inject
PutMessage(
@@ -84,8 +81,7 @@
@GerritPersonIdent PersonIdent gerritIdent,
PatchSetUtil psUtil,
NotifyResolver notifyResolver,
- ProjectCache projectCache,
- DynamicItem<UrlFormatter> urlFormatter) {
+ ProjectCache projectCache) {
this.updateFactory = updateFactory;
this.repositoryManager = repositoryManager;
this.userProvider = userProvider;
@@ -95,7 +91,6 @@
this.psUtil = psUtil;
this.notifyResolver = notifyResolver;
this.projectCache = projectCache;
- this.urlFormatter = urlFormatter;
}
@Override
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
index 1c7b54b..50b7a7c 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -1360,6 +1360,39 @@
}
@Test
+ public void rebaseDoesNotAddWorkInProgress() throws Exception {
+ PushOneCommit.Result r = createChange();
+
+ // create an unrelated change so that we can rebase
+ testRepo.reset("HEAD~1");
+ PushOneCommit.Result unrelated = createChange();
+ gApi.changes().id(unrelated.getChangeId()).current().review(ReviewInput.approve());
+ gApi.changes().id(unrelated.getChangeId()).current().submit();
+
+ gApi.changes().id(r.getChangeId()).rebase();
+
+ // change is still ready for review after rebase
+ assertThat(gApi.changes().id(r.getChangeId()).get().workInProgress).isNull();
+ }
+
+ @Test
+ public void rebaseDoesNotRemoveWorkInProgress() throws Exception {
+ PushOneCommit.Result r = createChange();
+ change(r).setWorkInProgress();
+
+ // create an unrelated change so that we can rebase
+ testRepo.reset("HEAD~1");
+ PushOneCommit.Result unrelated = createChange();
+ gApi.changes().id(unrelated.getChangeId()).current().review(ReviewInput.approve());
+ gApi.changes().id(unrelated.getChangeId()).current().submit();
+
+ gApi.changes().id(r.getChangeId()).rebase();
+
+ // change is still work in progress after rebase
+ assertThat(gApi.changes().id(r.getChangeId()).get().workInProgress).isTrue();
+ }
+
+ @Test
public void rebaseConflict_conflictsAllowed() throws Exception {
String patchSetSubject = "patch set change";
String patchSetContent = "patch set content";
diff --git a/javatests/com/google/gerrit/acceptance/git/AbstractGitOverHttpServlet.java b/javatests/com/google/gerrit/acceptance/git/AbstractGitOverHttpServlet.java
index 7f01fb9..a22759f 100644
--- a/javatests/com/google/gerrit/acceptance/git/AbstractGitOverHttpServlet.java
+++ b/javatests/com/google/gerrit/acceptance/git/AbstractGitOverHttpServlet.java
@@ -19,6 +19,7 @@
import com.google.common.collect.ImmutableList;
import com.google.gerrit.acceptance.FakeGroupAuditService;
import com.google.gerrit.acceptance.Sandboxed;
+import com.google.gerrit.acceptance.TestProjectInput;
import com.google.gerrit.entities.Account;
import com.google.gerrit.pgm.http.jetty.JettyServer;
import com.google.gerrit.server.audit.HttpAuditEvent;
@@ -80,6 +81,7 @@
}
@Test
+ @TestProjectInput(createEmptyCommit = false)
public void authenticatedUploadPackAuditEventLog() throws Exception {
String remote = "authenticated";
Config cfg = testRepo.git().getRepository().getConfig();
@@ -92,6 +94,7 @@
}
@Test
+ @TestProjectInput(createEmptyCommit = false)
public void anonymousUploadPackAuditEventLog() throws Exception {
String remote = "anonymous";
Config cfg = testRepo.git().getRepository().getConfig();
@@ -110,16 +113,18 @@
*/
private void uploadPackAuditEventLog(String remote, Optional<Account.Id> accountId)
throws Exception {
+ // Make a server-side change to have a common base.
+ createCommit("foo");
+ testRepo.git().fetch().call();
+
+ // Make a server-side change so we have something to fetch.
+ createCommit("bar");
+
auditService.drainHttpAuditEvents();
- // testRepo is already a clone. Make a server-side change so we have something to fetch.
- try (Repository repo = repoManager.openRepository(project);
- TestRepository<?> testRepo = new TestRepository<>(repo)) {
- testRepo.branch("master").commit().create();
- }
testRepo.git().fetch().setRemote(remote).call();
ImmutableList<HttpAuditEvent> auditEvents = auditService.drainHttpAuditEvents();
- assertThat(auditEvents).hasSize(4);
+ assertThat(auditEvents).hasSize(3);
// Protocol V2 Capability advertisement
// https://git-scm.com/docs/protocol-v2#_capability_advertisement
@@ -147,11 +152,13 @@
assertThat(uploadPackFetch.what).endsWith("/git-upload-pack");
assertThat(uploadPackFetch.params).isEmpty();
assertThat(uploadPackFetch.httpStatus).isEqualTo(HttpServletResponse.SC_OK);
- HttpAuditEvent uploadPackDone = auditEvents.get(3);
-
- assertThat(uploadPackDone.what).endsWith("/git-upload-pack");
- assertThat(uploadPackDone.params).isEmpty();
- assertThat(uploadPackDone.httpStatus).isEqualTo(HttpServletResponse.SC_OK);
assertThat(jettyServer.numActiveSessions()).isEqualTo(0);
}
+
+ private void createCommit(String message) throws Exception {
+ try (Repository repo = repoManager.openRepository(project);
+ TestRepository<Repository> tr = new TestRepository<>(repo)) {
+ tr.branch("master").commit().message(message).create();
+ }
+ }
}
diff --git a/javatests/com/google/gerrit/acceptance/git/PushPermissionsIT.java b/javatests/com/google/gerrit/acceptance/git/PushPermissionsIT.java
index 78be4ab..385780b 100644
--- a/javatests/com/google/gerrit/acceptance/git/PushPermissionsIT.java
+++ b/javatests/com/google/gerrit/acceptance/git/PushPermissionsIT.java
@@ -82,17 +82,14 @@
.update();
}
- @SuppressWarnings("TruthIncompatibleType")
@Test
public void mixingMagicAndRegularPush() throws Exception {
testRepo.branch("HEAD").commit().create();
PushResult r = push("HEAD:refs/heads/master", "HEAD:refs/for/master");
String msg = "cannot combine normal pushes and magic pushes";
- assertThat(r.getRemoteUpdate("refs/heads/master"))
- .isNotEqualTo(/* expected: RemoteRefUpdate, actual: Status */ Status.OK);
- assertThat(r.getRemoteUpdate("refs/for/master"))
- .isNotEqualTo(/* expected: RemoteRefUpdate, actual: Status */ Status.OK);
+ assertThat(r.getRemoteUpdate("refs/heads/master").getStatus()).isNotEqualTo(Status.OK);
+ assertThat(r.getRemoteUpdate("refs/for/master").getStatus()).isNotEqualTo(Status.OK);
assertThat(r.getRemoteUpdate("refs/for/master").getMessage()).isEqualTo(msg);
}
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/AttentionSetIT.java b/javatests/com/google/gerrit/acceptance/rest/change/AttentionSetIT.java
index 9e944a2..7e6a822 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/AttentionSetIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/AttentionSetIT.java
@@ -495,6 +495,24 @@
}
@Test
+ public void rebaseDoesNotAddToAttentionSet() throws Exception {
+ PushOneCommit.Result r = createChange();
+ change(r).setWorkInProgress();
+ change(r).addReviewer(user.email());
+
+ // create an unrelated change so that we can rebase
+ testRepo.reset("HEAD~1");
+ PushOneCommit.Result unrelated = createChange();
+ gApi.changes().id(unrelated.getChangeId()).current().review(ReviewInput.approve());
+ gApi.changes().id(unrelated.getChangeId()).current().submit();
+
+ gApi.changes().id(r.getChangeId()).rebase();
+
+ // rebase has no impact on the attention set
+ assertThat(r.getChange().attentionSet()).isEmpty();
+ }
+
+ @Test
public void readyForReviewWhileRemovingReviewerRemovesThemToAttentionSet() throws Exception {
PushOneCommit.Result r = createChange();
change(r).setWorkInProgress();
diff --git a/javatests/com/google/gerrit/server/restapi/change/ListChangeCommentsTest.java b/javatests/com/google/gerrit/server/restapi/change/ListChangeCommentsTest.java
index 5cefe74..b3e0c56 100644
--- a/javatests/com/google/gerrit/server/restapi/change/ListChangeCommentsTest.java
+++ b/javatests/com/google/gerrit/server/restapi/change/ListChangeCommentsTest.java
@@ -25,6 +25,7 @@
import java.sql.Timestamp;
import java.util.ArrayList;
import java.util.List;
+import java.util.Set;
import java.util.stream.Collectors;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -33,7 +34,6 @@
@RunWith(JUnit4.class)
public class ListChangeCommentsTest {
- @SuppressWarnings("TruthIncompatibleType")
@Test
public void commentsLinkedToChangeMessagesIgnoreGerritAutoGenTaggedMessages() {
/* Comments should not be linked to Gerrit's autogenerated messages */
@@ -55,10 +55,10 @@
.isEqualTo(getChangeMessage(changeMessages, "cm3").getKey().uuid());
// Make sure no comment is linked to the auto-gen message
- assertThat(comments.stream().map(c -> c.changeMessageId).collect(Collectors.toSet()))
- .doesNotContain(
- /* expected: String, actual: ChangeMessage */ getChangeMessage(
- changeMessages, "cmAutoGenByGerrit"));
+ Set<String> changeMessageIds =
+ comments.stream().map(c -> c.changeMessageId).collect(Collectors.toSet());
+ assertThat(changeMessageIds)
+ .doesNotContain(getChangeMessage(changeMessages, "cmAutoGenByGerrit").getKey().uuid());
}
@Test
diff --git a/package.json b/package.json
index ab23092..fc4161b 100644
--- a/package.json
+++ b/package.json
@@ -3,9 +3,9 @@
"version": "3.1.0-SNAPSHOT",
"description": "Gerrit Code Review",
"dependencies": {
- "@bazel/rollup": "^3.1.0",
- "@bazel/terser": "^3.1.0",
- "@bazel/typescript": "^3.1.0"
+ "@bazel/rollup": "^3.2.0",
+ "@bazel/terser": "^3.2.0",
+ "@bazel/typescript": "^3.2.0"
},
"devDependencies": {
"@typescript-eslint/eslint-plugin": "^4.11.0",
diff --git a/polygerrit-ui/app/.eslintrc.js b/polygerrit-ui/app/.eslintrc.js
index 764d5e8..a3da3cf 100644
--- a/polygerrit-ui/app/.eslintrc.js
+++ b/polygerrit-ui/app/.eslintrc.js
@@ -55,6 +55,7 @@
}],
// https://eslint.org/docs/rules/eol-last
'eol-last': 'off',
+ 'guard-for-in': 'error',
// https://eslint.org/docs/rules/indent
'indent': ['error', 2, {
MemberExpression: 2,
diff --git a/polygerrit-ui/app/api/diff.ts b/polygerrit-ui/app/api/diff.ts
index 7e619c2..5d7125c 100644
--- a/polygerrit-ui/app/api/diff.ts
+++ b/polygerrit-ui/app/api/diff.ts
@@ -225,6 +225,9 @@
code_range: LineRange;
}
+/** LOST LineNumber is for ported comments without a range, they have their own
+ * line number and are added on top of the FILE row in gr-diff
+ */
export declare type LineNumber = number | 'FILE' | 'LOST';
/** The detail of the 'create-comment' event dispatched by gr-diff. */
diff --git a/polygerrit-ui/app/elements/admin/gr-create-change-dialog/gr-create-change-dialog.ts b/polygerrit-ui/app/elements/admin/gr-create-change-dialog/gr-create-change-dialog.ts
index 0b1e27e..2124949 100644
--- a/polygerrit-ui/app/elements/admin/gr-create-change-dialog/gr-create-change-dialog.ts
+++ b/polygerrit-ui/app/elements/admin/gr-create-change-dialog/gr-create-change-dialog.ts
@@ -34,7 +34,6 @@
InheritedBooleanInfo,
} from '../../../types/common';
import {InheritedBooleanInfoConfiguredValue} from '../../../constants/constants';
-import {hasOwnProperty} from '../../../utils/common-util';
import {GrAutocomplete} from '../../shared/gr-autocomplete/gr-autocomplete';
import {IronAutogrowTextareaElement} from '@polymer/iron-autogrow-textarea/iron-autogrow-textarea';
import {appContext} from '../../../services/app-context';
@@ -174,19 +173,12 @@
.then(response => {
if (!response) return [];
const branches = [];
- let branch;
- for (const key in response) {
- if (!hasOwnProperty(response, key)) {
- continue;
+ for (const branchInfo of response) {
+ let name: string = branchInfo.ref;
+ if (name.startsWith('refs/heads/')) {
+ name = name.substring('refs/heads/'.length);
}
- if (response[key].ref.startsWith('refs/heads/')) {
- branch = response[key].ref.substring('refs/heads/'.length);
- } else {
- branch = response[key].ref;
- }
- branches.push({
- name: branch,
- });
+ branches.push({name});
}
return branches;
});
diff --git a/polygerrit-ui/app/elements/admin/gr-create-repo-dialog/gr-create-repo-dialog.ts b/polygerrit-ui/app/elements/admin/gr-create-repo-dialog/gr-create-repo-dialog.ts
index 1b16052..c9fd241 100644
--- a/polygerrit-ui/app/elements/admin/gr-create-repo-dialog/gr-create-repo-dialog.ts
+++ b/polygerrit-ui/app/elements/admin/gr-create-repo-dialog/gr-create-repo-dialog.ts
@@ -28,7 +28,6 @@
import {page} from '../../../utils/page-wrapper-utils';
import {customElement, observe, property} from '@polymer/decorators';
import {ProjectInput, RepoName} from '../../../types/common';
-import {hasOwnProperty} from '../../../utils/common-util';
import {AutocompleteQuery} from '../../shared/gr-autocomplete/gr-autocomplete';
import {appContext} from '../../../services/app-context';
@@ -115,14 +114,8 @@
_getRepoSuggestions(input: string) {
return this.restApiService.getSuggestedProjects(input).then(response => {
const repos = [];
- for (const key in response) {
- if (!hasOwnProperty(response, key)) {
- continue;
- }
- repos.push({
- name: key,
- value: response[key].id,
- });
+ for (const [name, project] of Object.entries(response ?? {})) {
+ repos.push({name, value: project.id});
}
return repos;
});
@@ -131,14 +124,8 @@
_getGroupSuggestions(input: string) {
return this.restApiService.getSuggestedGroups(input).then(response => {
const groups = [];
- for (const key in response) {
- if (!hasOwnProperty(response, key)) {
- continue;
- }
- groups.push({
- name: key,
- value: decodeURIComponent(response[key].id),
- });
+ for (const [name, group] of Object.entries(response ?? {})) {
+ groups.push({name, value: decodeURIComponent(group.id)});
}
return groups;
});
diff --git a/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members.ts b/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members.ts
index 888647f..f5602a3 100644
--- a/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members.ts
+++ b/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members.ts
@@ -39,9 +39,11 @@
GroupInfo,
GroupName,
} from '../../../types/common';
-import {AutocompleteQuery} from '../../shared/gr-autocomplete/gr-autocomplete';
+import {
+ AutocompleteQuery,
+ AutocompleteSuggestion,
+} from '../../shared/gr-autocomplete/gr-autocomplete';
import {PolymerDomRepeatEvent} from '../../../types/types';
-import {hasOwnProperty} from '../../../utils/common-util';
import {
fireAlert,
firePageError,
@@ -339,23 +341,18 @@
return this.restApiService
.getSuggestedAccounts(input, SUGGESTIONS_LIMIT)
.then(accounts => {
+ if (!accounts) return [];
const accountSuggestions = [];
- let nameAndEmail;
- if (!accounts) {
- return [];
- }
- for (const key in accounts) {
- if (!hasOwnProperty(accounts, key)) {
- continue;
- }
- if (accounts[key].email !== undefined) {
- nameAndEmail = `${accounts[key].name} <${accounts[key].email}>`;
+ for (const account of accounts) {
+ let nameAndEmail;
+ if (account.email !== undefined) {
+ nameAndEmail = `${account.name} <${account.email}>`;
} else {
- nameAndEmail = accounts[key].name;
+ nameAndEmail = account.name;
}
accountSuggestions.push({
name: nameAndEmail,
- value: accounts[key]._account_id?.toString(),
+ value: account._account_id?.toString(),
});
}
return accountSuggestions;
@@ -364,15 +361,9 @@
_getGroupSuggestions(input: string) {
return this.restApiService.getSuggestedGroups(input).then(response => {
- const groups = [];
- for (const key in response) {
- if (!hasOwnProperty(response, key)) {
- continue;
- }
- groups.push({
- name: key,
- value: decodeURIComponent(response[key].id),
- });
+ const groups: AutocompleteSuggestion[] = [];
+ for (const [name, group] of Object.entries(response ?? {})) {
+ groups.push({name, value: decodeURIComponent(group.id)});
}
return groups;
});
diff --git a/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members_test.js b/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members_test.js
index 654d24a..672ac07 100644
--- a/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members_test.js
+++ b/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members_test.js
@@ -100,7 +100,7 @@
},
]);
} else {
- return Promise.resolve({});
+ return Promise.resolve([]);
}
});
stubRestApi('getSuggestedGroups').callsFake(input => {
diff --git a/polygerrit-ui/app/elements/admin/gr-group/gr-group.ts b/polygerrit-ui/app/elements/admin/gr-group/gr-group.ts
index cee803c..058f86b 100644
--- a/polygerrit-ui/app/elements/admin/gr-group/gr-group.ts
+++ b/polygerrit-ui/app/elements/admin/gr-group/gr-group.ts
@@ -33,7 +33,6 @@
} from '../../shared/gr-autocomplete/gr-autocomplete';
import {GroupId, GroupInfo, GroupName} from '../../../types/common';
import {ErrorCallback} from '../../../services/gr-rest-api/gr-rest-api';
-import {hasOwnProperty} from '../../../utils/common-util';
import {
fireEvent,
firePageError,
@@ -301,14 +300,8 @@
_getGroupSuggestions(input: string) {
return this.restApiService.getSuggestedGroups(input).then(response => {
const groups: AutocompleteSuggestion[] = [];
- for (const key in response) {
- if (!hasOwnProperty(response, key)) {
- continue;
- }
- groups.push({
- name: key,
- value: decodeURIComponent(response[key].id),
- });
+ for (const [name, group] of Object.entries(response ?? {})) {
+ groups.push({name, value: decodeURIComponent(group.id)});
}
return groups;
});
diff --git a/polygerrit-ui/app/elements/admin/gr-permission/gr-permission.ts b/polygerrit-ui/app/elements/admin/gr-permission/gr-permission.ts
index 65a3b00..5702bfb 100644
--- a/polygerrit-ui/app/elements/admin/gr-permission/gr-permission.ts
+++ b/polygerrit-ui/app/elements/admin/gr-permission/gr-permission.ts
@@ -33,7 +33,6 @@
PermissionArray,
} from '../../../utils/access-util';
import {customElement, property, observe} from '@polymer/decorators';
-import {hasOwnProperty} from '../../../utils/common-util';
import {
LabelNameToLabelTypeInfoMap,
LabelTypeInfoValues,
@@ -333,14 +332,8 @@
.getSuggestedGroups(this._groupFilter || '', MAX_AUTOCOMPLETE_RESULTS)
.then(response => {
const groups: GroupSuggestion[] = [];
- for (const key in response) {
- if (!hasOwnProperty(response, key)) {
- continue;
- }
- groups.push({
- name: key,
- value: response[key],
- });
+ for (const [name, value] of Object.entries(response ?? {})) {
+ groups.push({name, value});
}
// Does not return groups in which we already have rules for.
return groups
diff --git a/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access.ts b/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access.ts
index 9fb79d3..935d091 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access.ts
+++ b/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access.ts
@@ -37,7 +37,6 @@
UrlEncodedRepoName,
ProjectAccessGroups,
} from '../../../types/common';
-import {hasOwnProperty} from '../../../utils/common-util';
import {GrButton} from '../../shared/gr-button/gr-button';
import {GrAccessSection} from '../gr-access-section/gr-access-section';
import {
@@ -370,11 +369,9 @@
/**
* Used to recursively remove any objects with a 'deleted' bit.
*/
- _recursivelyRemoveDeleted(obj: PropertyTreeNode) {
- for (const k in obj) {
- if (!hasOwnProperty(obj, k)) {
- continue;
- }
+ _recursivelyRemoveDeleted(obj?: PropertyTreeNode) {
+ if (!obj) return;
+ for (const k of Object.keys(obj)) {
const node = obj[k];
if (typeof node === 'object') {
if (node.deleted) {
@@ -387,17 +384,15 @@
}
_recursivelyUpdateAddRemoveObj(
- obj: PropertyTreeNode,
+ obj: PropertyTreeNode | undefined,
addRemoveObj: {
add: PropertyTreeNode;
remove: PropertyTreeNode;
},
path: string[] = []
) {
- for (const k in obj) {
- if (!hasOwnProperty(obj, k)) {
- continue;
- }
+ if (!obj) return;
+ for (const k of Object.keys(obj)) {
const node = obj[k];
if (typeof node === 'object') {
const updatedId = node.updatedId;
diff --git a/polygerrit-ui/app/elements/admin/gr-repo/gr-repo.ts b/polygerrit-ui/app/elements/admin/gr-repo/gr-repo.ts
index 1c30e96..3543e3b 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo/gr-repo.ts
+++ b/polygerrit-ui/app/elements/admin/gr-repo/gr-repo.ts
@@ -392,21 +392,14 @@
schemesObj?: SchemesInfoMap,
_selectedScheme?: string
) {
- if (!schemesObj || !repo || !_selectedScheme) {
- return [];
- }
+ if (!schemesObj || !repo || !_selectedScheme) return [];
+ if (!hasOwnProperty(schemesObj, _selectedScheme)) return [];
+ const commandObj = schemesObj[_selectedScheme].clone_commands;
const commands = [];
- let commandObj: {[title: string]: string} = {};
- if (hasOwnProperty(schemesObj, _selectedScheme)) {
- commandObj = schemesObj[_selectedScheme].clone_commands;
- }
- for (const title in commandObj) {
- if (!hasOwnProperty(commandObj, title)) {
- continue;
- }
+ for (const [title, command] of Object.entries(commandObj)) {
commands.push({
title,
- command: commandObj[title]
+ command: command
.replace(/\${project}/gi, encodeURI(repo))
.replace(
/\${project-base-name}/gi,
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-view/gr-change-list-view.ts b/polygerrit-ui/app/elements/change-list/gr-change-list-view/gr-change-list-view.ts
index 973ccc8..441d514 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-view/gr-change-list-view.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-view/gr-change-list-view.ts
@@ -37,17 +37,16 @@
} from '../../../types/common';
import {ChangeListToggleReviewedDetail} from '../gr-change-list-item/gr-change-list-item';
import {ChangeStarToggleStarDetail} from '../../shared/gr-change-star/gr-change-star';
-import {hasOwnProperty} from '../../../utils/common-util';
import {ChangeListViewState} from '../../../types/types';
import {fireTitleChange} from '../../../utils/event-util';
import {appContext} from '../../../services/app-context';
import {GerritView} from '../../../services/router/router-model';
-const LookupQueryPatterns = {
- CHANGE_ID: /^\s*i?[0-9a-f]{7,40}\s*$/i,
- CHANGE_NUM: /^\s*[1-9][0-9]*\s*$/g,
- COMMIT: /[0-9a-f]{40}/,
-};
+const LOOKUP_QUERY_PATTERNS: RegExp[] = [
+ /^\s*i?[0-9a-f]{7,40}\s*$/i, // CHANGE_ID
+ /^\s*[1-9][0-9]*\s*$/g, // CHANGE_NUM
+ /[0-9a-f]{40}/, // COMMIT
+];
const USER_QUERY_PATTERN = /^owner:\s?("[^"]+"|[^ ]+)$/;
@@ -159,12 +158,8 @@
.then(changes => {
changes = changes || [];
if (this._query && changes.length === 1) {
- let query: keyof typeof LookupQueryPatterns;
- for (query in LookupQueryPatterns) {
- if (
- hasOwnProperty(LookupQueryPatterns, query) &&
- this._query.match(LookupQueryPatterns[query])
- ) {
+ for (const queryPattern of LOOKUP_QUERY_PATTERNS) {
+ if (this._query.match(queryPattern)) {
// "Back"/"Forward" buttons work correctly only with
// opt_redirect options
GerritNav.navigateToChange(
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.ts b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.ts
index 9ad0b05f..f36df84 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.ts
@@ -55,6 +55,7 @@
} from '../../../utils/attention-set-util';
import {CustomKeyboardEvent} from '../../../types/events';
import {fireEvent} from '../../../utils/event-util';
+import {windowLocationReload} from '../../../utils/dom-util';
const NUMBER_FIXED_COLUMNS = 3;
const CLOSED_STATUS = ['MERGED', 'ABANDONED'];
@@ -478,7 +479,7 @@
}
_reloadWindow() {
- window.location.reload();
+ windowLocationReload();
}
_toggleChangeStar(e: CustomKeyboardEvent) {
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 96714e3..903afaa 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
@@ -940,14 +940,14 @@
return null;
}
let result;
- for (const label in this.change.labels) {
+ for (const [label, labelInfo] of Object.entries(this.change.labels)) {
if (!(label in this.change.permitted_labels)) {
continue;
}
if (this.change.permitted_labels[label].length === 0) {
continue;
}
- const status = this._getLabelStatus(this.change.labels[label]);
+ const status = this._getLabelStatus(labelInfo);
if (status === LabelStatus.NEED) {
if (result) {
// More than one label is missing, so it's unclear which to quick
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 4a92869..5afcafc 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
@@ -55,7 +55,10 @@
} from '../../../mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin';
import {GrEditConstants} from '../../edit/gr-edit-constants';
import {pluralize} from '../../../utils/string-util';
-import {getComputedStyleValue} from '../../../utils/dom-util';
+import {
+ getComputedStyleValue,
+ windowLocationReload,
+} from '../../../utils/dom-util';
import {GerritNav} from '../../core/gr-navigation/gr-navigation';
import {getPluginEndpoints} from '../../shared/gr-js-api-interface/gr-plugin-endpoints';
import {getPluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader';
@@ -236,6 +239,10 @@
export type ChangeViewPatchRange = Partial<PatchRange>;
+const DEBOUNCER_REPLY_OVERLAY_REFIT = 'reply-overlay-refit';
+
+const DEBOUNCER_SCROLL = 'scroll';
+
@customElement('gr-change-view')
export class GrChangeView extends KeyboardShortcutMixin(
GestureEventListeners(LegacyElementMixin(PolymerElement))
@@ -707,6 +714,8 @@
super.detached();
this.unlisten(window, 'scroll', '_handleScroll');
this.unlisten(document, 'visibilitychange', '_handleVisibilityChange');
+ this.cancelDebouncer(DEBOUNCER_REPLY_OVERLAY_REFIT);
+ this.cancelDebouncer(DEBOUNCER_SCROLL);
if (this._updateCheckTimerHandle) {
this._cancelUpdateCheckTimer();
@@ -896,7 +905,7 @@
}
_reloadWindow() {
- window.location.reload();
+ windowLocationReload();
}
_handleCommitMessageCancel() {
@@ -1227,7 +1236,7 @@
_handleReplyAutogrow() {
// If the textarea resizes, we need to re-fit the overlay.
this.debounce(
- 'reply-overlay-refit',
+ DEBOUNCER_REPLY_OVERLAY_REFIT,
() => {
this.$.replyOverlay.refit();
},
@@ -1245,7 +1254,7 @@
_handleScroll() {
this.debounce(
- 'scroll',
+ DEBOUNCER_SCROLL,
() => {
this.viewState.scrollTop = document.body.scrollTop;
},
@@ -2074,21 +2083,15 @@
}
_getLatestRevisionSHA(change: ChangeInfo | ParsedChangeInfo) {
- if (change.current_revision) {
- return change.current_revision;
- }
+ if (change.current_revision) return change.current_revision;
// current_revision may not be present in the case where the latest rev is
// a draft and the user doesn’t have permission to view that rev.
let latestRev = null;
let latestPatchNum = -1 as PatchSetNum;
- for (const rev in change.revisions) {
- if (!hasOwnProperty(change.revisions, rev)) {
- continue;
- }
-
- if (change.revisions[rev]._number > latestPatchNum) {
+ for (const [rev, revInfo] of Object.entries(change.revisions ?? {})) {
+ if (revInfo._number > latestPatchNum) {
latestRev = rev;
- latestPatchNum = change.revisions[rev]._number;
+ latestPatchNum = revInfo._number;
}
}
return latestRev;
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-cherrypick-dialog/gr-confirm-cherrypick-dialog.ts b/polygerrit-ui/app/elements/change/gr-confirm-cherrypick-dialog/gr-confirm-cherrypick-dialog.ts
index 2855716..10cffba 100644
--- a/polygerrit-ui/app/elements/change/gr-confirm-cherrypick-dialog/gr-confirm-cherrypick-dialog.ts
+++ b/polygerrit-ui/app/elements/change/gr-confirm-cherrypick-dialog/gr-confirm-cherrypick-dialog.ts
@@ -37,7 +37,6 @@
import {customElement, property, observe} from '@polymer/decorators';
import {AutocompleteSuggestion} from '../../shared/gr-autocomplete/gr-autocomplete';
import {HttpMethod, ChangeStatus} from '../../../constants/constants';
-import {hasOwnProperty} from '../../../utils/common-util';
import {dom, EventApi} from '@polymer/polymer/lib/legacy/polymer.dom';
const SUGGESTIONS_LIMIT = 15;
@@ -399,21 +398,16 @@
return this.restApiService
.getRepoBranches(input, this.project, SUGGESTIONS_LIMIT)
.then((response: BranchInfo[] | undefined) => {
- const branches = [];
if (!response) return [];
- let branch;
- for (const key in response) {
- if (!hasOwnProperty(response, key)) {
- continue;
- }
- if (response[key].ref.startsWith('refs/heads/')) {
- branch = response[key].ref.substring('refs/heads/'.length);
+ const branches = [];
+ for (const branchInfo of response) {
+ let branch;
+ if (branchInfo.ref.startsWith('refs/heads/')) {
+ branch = branchInfo.ref.substring('refs/heads/'.length);
} else {
- branch = response[key].ref;
+ branch = branchInfo.ref;
}
- branches.push({
- name: branch,
- });
+ branches.push({name: branch});
}
return branches;
});
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-cherrypick-dialog/gr-confirm-cherrypick-dialog_test.js b/polygerrit-ui/app/elements/change/gr-confirm-cherrypick-dialog/gr-confirm-cherrypick-dialog_test.js
index e4ed533..536a4ab 100644
--- a/polygerrit-ui/app/elements/change/gr-confirm-cherrypick-dialog/gr-confirm-cherrypick-dialog_test.js
+++ b/polygerrit-ui/app/elements/change/gr-confirm-cherrypick-dialog/gr-confirm-cherrypick-dialog_test.js
@@ -39,7 +39,7 @@
},
]);
} else {
- return Promise.resolve({});
+ return Promise.resolve([]);
}
});
element = basicFixture.instantiate();
@@ -77,11 +77,9 @@
assert.equal(element.message, myNewMessage);
});
- test('_getProjectBranchesSuggestions empty', done => {
- element._getProjectBranchesSuggestions('nonexistent').then(branches => {
- assert.equal(branches.length, 0);
- done();
- });
+ test('_getProjectBranchesSuggestions empty', async () => {
+ const branches = await element._getProjectBranchesSuggestions('asdf');
+ assert.isEmpty(branches);
});
suite('cherry pick topic', () => {
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-rebase-dialog/gr-confirm-rebase-dialog.ts b/polygerrit-ui/app/elements/change/gr-confirm-rebase-dialog/gr-confirm-rebase-dialog.ts
index 9e7bdb4..9dcd849 100644
--- a/polygerrit-ui/app/elements/change/gr-confirm-rebase-dialog/gr-confirm-rebase-dialog.ts
+++ b/polygerrit-ui/app/elements/change/gr-confirm-rebase-dialog/gr-confirm-rebase-dialog.ts
@@ -22,7 +22,6 @@
import {PolymerElement} from '@polymer/polymer/polymer-element';
import {htmlTemplate} from './gr-confirm-rebase-dialog_html';
import {customElement, property, observe} from '@polymer/decorators';
-import {hasOwnProperty} from '../../../utils/common-util';
import {NumericChangeId, BranchName} from '../../../types/common';
import {
GrAutocomplete,
@@ -109,13 +108,10 @@
.then(response => {
if (!response) return [];
const changes: RebaseChange[] = [];
- for (const key in response) {
- if (!hasOwnProperty(response, key)) {
- continue;
- }
+ for (const change of response) {
changes.push({
- name: `${response[key]._number}: ${response[key].subject}`,
- value: response[key]._number,
+ name: `${change._number}: ${change.subject}`,
+ value: change._number,
});
}
this._recentChanges = changes;
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 7fbe4b5..ab1e4e6 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
@@ -122,14 +122,8 @@
}
}
const commands = [];
- for (const title in commandObj) {
- if (!commandObj || !hasOwnProperty(commandObj, title)) {
- continue;
- }
- commands.push({
- title,
- command: commandObj[title],
- });
+ for (const [title, command] of Object.entries(commandObj ?? {})) {
+ commands.push({title, command});
}
return commands;
}
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 d41a662..a5c0624 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
@@ -67,7 +67,6 @@
import {DiffPreferencesInfo} from '../../../types/diff';
import {GrDiffHost} from '../../diff/gr-diff-host/gr-diff-host';
import {GrDiffPreferencesDialog} from '../../diff/gr-diff-preferences-dialog/gr-diff-preferences-dialog';
-import {hasOwnProperty} from '../../../utils/common-util';
import {GrDiffCursor} from '../../diff/gr-diff-cursor/gr-diff-cursor';
import {GrCursorManager} from '../../shared/gr-cursor-manager/gr-cursor-manager';
import {PolymerSpliceChange} from '@polymer/polymer/interfaces';
@@ -153,6 +152,8 @@
export type FileNameToReviewedFileInfoMap = {[name: string]: ReviewedFileInfo};
+const DEBOUNCER_LOADING_CHANGE = 'loading-change';
+
/**
* Type for FileInfo
*
@@ -407,6 +408,7 @@
detached() {
super.detached();
this._cancelDiffs();
+ this.cancelDebouncer(DEBOUNCER_LOADING_CHANGE);
}
/**
@@ -1241,13 +1243,9 @@
const files: FileNameToReviewedFileInfoMap = {...filesByPath};
addUnmodifiedFiles(files, commentedPaths);
const reviewedSet = new Set(reviewed || []);
- for (const filePath in files) {
- if (!hasOwnProperty(files, filePath)) {
- continue;
- }
- files[filePath].isReviewed = reviewedSet.has(filePath);
+ for (const [filePath, reviewedFileInfo] of Object.entries(files)) {
+ reviewedFileInfo.isReviewed = reviewedSet.has(filePath);
}
-
this._files = this._normalizeChangeFilesResponse(files);
}
@@ -1598,7 +1596,7 @@
*/
_loadingChanged(loading?: boolean) {
this.debounce(
- 'loading-change',
+ DEBOUNCER_LOADING_CHANGE,
() => {
// Only show set the loading if there have been files loaded to show. In
// this way, the gray loading style is not shown on initial loads.
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.js b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.js
index 77855df..2d38dcc 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.js
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.js
@@ -306,11 +306,8 @@
'-1073741824': '-1 GiB',
'0': '+/-0 B',
};
-
- for (const bytes in table) {
- if (table.hasOwnProperty(bytes)) {
- assert.equal(element._formatBytes(Number(bytes)), table[bytes]);
- }
+ for (const [bytes, expected] of Object.entries(table)) {
+ assert.equal(element._formatBytes(Number(bytes)), expected);
}
});
@@ -590,12 +587,8 @@
flush();
assert.equal(element.diffs.length, paths.length);
assert.equal(element._expandedFiles.length, paths.length);
- for (const index in element.diffs) {
- if (!element.diffs.hasOwnProperty(index)) { continue; }
- assert.isTrue(
- element._expandedFiles
- .some(f => f.path === element.diffs[index].path)
- );
+ for (const diff of element.diffs) {
+ assert.isTrue(element._expandedFiles.some(f => f.path === diff.path));
}
MockInteractions.keyUpOn(element, 73, 'shift', 'i');
diff --git a/polygerrit-ui/app/elements/change/gr-label-scores/gr-label-scores.ts b/polygerrit-ui/app/elements/change/gr-label-scores/gr-label-scores.ts
index b994fa5..f8cd42c 100644
--- a/polygerrit-ui/app/elements/change/gr-label-scores/gr-label-scores.ts
+++ b/polygerrit-ui/app/elements/change/gr-label-scores/gr-label-scores.ts
@@ -65,11 +65,7 @@
if (this.shadowRoot === null || !this.change) {
return labels;
}
- for (const label in this.permittedLabels) {
- if (!hasOwnProperty(this.permittedLabels, label)) {
- continue;
- }
-
+ for (const label of Object.keys(this.permittedLabels ?? {})) {
const selectorEl = this.shadowRoot.querySelector(
`gr-label-score-row[name="${label}"]`
) as null | GrLabelScoreRow;
@@ -104,9 +100,10 @@
labelName: string,
numberValue?: number
) {
- for (const k in (labels[labelName] as DetailedLabelInfo).values) {
- if (Number(k) === numberValue) {
- return k;
+ const detailedInfo = labels[labelName] as DetailedLabelInfo;
+ for (const labelValue of Object.keys(detailedInfo.values)) {
+ if (Number(labelValue) === numberValue) {
+ return labelValue;
}
}
return numberValue;
diff --git a/polygerrit-ui/app/elements/change/gr-label-scores/gr-label-scores_test.js b/polygerrit-ui/app/elements/change/gr-label-scores/gr-label-scores_test.js
index 5135e439..ef123c9 100644
--- a/polygerrit-ui/app/elements/change/gr-label-scores/gr-label-scores_test.js
+++ b/polygerrit-ui/app/elements/change/gr-label-scores/gr-label-scores_test.js
@@ -85,12 +85,10 @@
});
test('get and set label scores', () => {
- for (const label in element.permittedLabels) {
- if (element.permittedLabels.hasOwnProperty(label)) {
- const row = element.shadowRoot
- .querySelector('gr-label-score-row[name="' + label + '"]');
- row.setSelectedValue(-1);
- }
+ for (const label of Object.keys(element.permittedLabels)) {
+ const row = element.shadowRoot
+ .querySelector('gr-label-score-row[name="' + label + '"]');
+ row.setSelectedValue(-1);
}
assert.deepEqual(element.getLabelValues(), {
'Code-Review': -1,
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts
index ca3161f..e5396c3 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts
@@ -168,6 +168,8 @@
};
}
+const DEBOUNCER_STORE = 'store';
+
@customElement('gr-reply-dialog')
export class GrReplyDialog extends KeyboardShortcutMixin(
GestureEventListeners(LegacyElementMixin(PolymerElement))
@@ -425,6 +427,11 @@
this.jsAPI.addElement(TargetElement.REPLY_DIALOG, this);
}
+ /** @override */
+ detached() {
+ this.cancelDebouncer(DEBOUNCER_STORE);
+ }
+
open(focusTarget?: FocusTarget) {
if (!this.change) throw new Error('missing required change property');
this.knownLatestState = LatestPatchState.CHECKING;
@@ -1332,7 +1339,7 @@
_draftChanged(newDraft: string, oldDraft?: string) {
this.debounce(
- 'store',
+ DEBOUNCER_STORE,
() => {
if (!newDraft.length && oldDraft) {
// If the draft has been modified to be empty, then erase the storage
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.js b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.js
index 2b92ca6..6682bfb 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.js
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.js
@@ -1001,8 +1001,8 @@
};
};
const checkObjEmpty = function(obj) {
- for (const prop in obj) {
- if (obj.hasOwnProperty(prop) && obj[prop].length) { return false; }
+ for (const prop of Object.keys(obj)) {
+ if (obj[prop].length) { return false; }
}
return true;
};
diff --git a/polygerrit-ui/app/elements/change/gr-reviewer-list/gr-reviewer-list.ts b/polygerrit-ui/app/elements/change/gr-reviewer-list/gr-reviewer-list.ts
index 254ecca..30931c3 100644
--- a/polygerrit-ui/app/elements/change/gr-reviewer-list/gr-reviewer-list.ts
+++ b/polygerrit-ui/app/elements/change/gr-reviewer-list/gr-reviewer-list.ts
@@ -231,7 +231,7 @@
}
let result: AccountInfo[] = [];
const reviewers = changeRecord.base;
- for (const key in reviewers) {
+ for (const key of Object.keys(reviewers)) {
if (this.reviewersOnly && key !== 'REVIEWER') {
continue;
}
diff --git a/polygerrit-ui/app/elements/core/gr-error-manager/gr-error-manager.ts b/polygerrit-ui/app/elements/core/gr-error-manager/gr-error-manager.ts
index 264034c..4a86005 100644
--- a/polygerrit-ui/app/elements/core/gr-error-manager/gr-error-manager.ts
+++ b/polygerrit-ui/app/elements/core/gr-error-manager/gr-error-manager.ts
@@ -38,6 +38,7 @@
ServerErrorEvent,
ShowAlertEvent,
} from '../../../types/events';
+import {windowLocationReload} from '../../../utils/dom-util';
const HIDE_ALERT_TIMEOUT_MS = 5000;
const CHECK_SIGN_IN_INTERVAL_MS = 60 * 1000;
@@ -72,6 +73,9 @@
errorOverlay: GrOverlay;
};
}
+
+const DEBOUNCER_CHECK_LOGGED_IN = 'checkLoggedIn';
+
@customElement('gr-error-manager')
export class GrErrorManager extends GestureEventListeners(
LegacyElementMixin(PolymerElement)
@@ -147,6 +151,7 @@
this.unlisten(document, 'show-error', '_handleShowErrorDialog');
this.unlisten(document, 'visibilitychange', '_handleVisibilityChange');
this.unlisten(document, 'show-auth-required', '_handleAuthRequired');
+ this.cancelDebouncer(DEBOUNCER_CHECK_LOGGED_IN);
if (this._authErrorHandlerDeregistrationHook) {
this._authErrorHandlerDeregistrationHook();
@@ -363,6 +368,7 @@
this._createLoginPopup()
);
this.fire('iron-announce', {text: errorText}, {bubbles: true});
+ this.reporting.reportInteraction('show-auth-error', {text: errorText});
this._refreshingCredentials = true;
this._requestCheckLoggedIn();
if (!document.hidden) {
@@ -378,11 +384,8 @@
}
_handleVisibilityChange() {
- // Ignore when the page is transitioning to hidden (or hidden is
- // undefined).
- if (document.hidden !== false) {
- return;
- }
+ // Ignore when the page is transitioning to hidden (or hidden is undefined).
+ if (document.hidden !== false) return;
// If not currently refreshing credentials and the credentials are old,
// request them to confirm their validity or (display an auth toast if it
@@ -393,6 +396,7 @@
this.knownAccountId !== undefined &&
timeSinceLastCheck > STALE_CREDENTIAL_THRESHOLD_MS
) {
+ this.reporting.reportInteraction('visibility-sign-in-check');
this._lastCredentialCheck = Date.now();
// check auth status in case:
@@ -404,7 +408,7 @@
_requestCheckLoggedIn() {
this.debounce(
- 'checkLoggedIn',
+ DEBOUNCER_CHECK_LOGGED_IN,
this._checkSignedIn,
CHECK_SIGN_IN_INTERVAL_MS
);
@@ -418,7 +422,6 @@
this._authService.clearCache();
this.restApiService.getLoggedIn().then(isLoggedIn => {
- // do nothing if its refreshing
if (!this._refreshingCredentials) return;
if (!isLoggedIn) {
@@ -428,12 +431,15 @@
// in case #2, auth-error is taken care of separately
this._requestCheckLoggedIn();
} else {
- // check account
this.restApiService.getAccount().then(account => {
if (this._refreshingCredentials) {
- // If the credentials were refreshed but the account is different
+ // If the credentials were refreshed but the account is different,
// then reload the page completely.
if (account?._account_id !== this.knownAccountId) {
+ this.reporting.reportInteraction('sign-in-window-reload', {
+ oldAccount: !!this.knownAccountId,
+ newAccount: !!account?._account_id,
+ });
this._reloadPage();
return;
}
@@ -446,7 +452,7 @@
}
_reloadPage() {
- window.location.reload();
+ windowLocationReload();
}
_createLoginPopup() {
@@ -478,7 +484,7 @@
}
_handleWindowFocus() {
- this.flushDebouncer('checkLoggedIn');
+ this.flushDebouncer(DEBOUNCER_CHECK_LOGGED_IN);
}
_handleShowErrorDialog(e: CustomEvent) {
diff --git a/polygerrit-ui/app/elements/core/gr-router/gr-router.ts b/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
index 8a23004..3a76112 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
@@ -67,6 +67,7 @@
import {GerritView, updateState} from '../../../services/router/router-model';
import {firePageError} from '../../../utils/event-util';
import {addQuotesWhen} from '../../../utils/string-util';
+import {windowLocationReload} from '../../../utils/dom-util';
const RoutePattern = {
ROOT: '/',
@@ -1725,7 +1726,7 @@
* by the catchall _handleDefaultRoute handler.
*/
_handlePassThroughRoute() {
- location.reload();
+ windowLocationReload();
}
/**
@@ -1762,7 +1763,7 @@
_handleDocumentationRedirectRoute(data: PageContextWithQueryMap) {
if (data.params[1]) {
- location.reload();
+ windowLocationReload();
} else {
// Redirect /Documentation to /Documentation/index.html
this._redirect('/Documentation/index.html');
diff --git a/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api.ts b/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api.ts
index 1085fbc..468c8ca 100644
--- a/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api.ts
+++ b/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api.ts
@@ -32,7 +32,6 @@
FileInfo,
ParentPatchSetNum,
} from '../../../types/common';
-import {hasOwnProperty} from '../../../utils/common-util';
import {
Comment,
CommentMap,
@@ -149,15 +148,11 @@
];
const commentMap: CommentMap = {};
for (const response of responses) {
- for (const path in response) {
+ for (const [path, comments] of Object.entries(response)) {
if (
- hasOwnProperty(response, path) &&
- response[path].some(c => {
+ comments.some(c => {
// If don't care about patch range, we know that the path exists.
- if (!patchRange) {
- return true;
- }
- return isInPatchRange(c, patchRange);
+ return !patchRange || isInPatchRange(c, patchRange);
})
) {
commentMap[path] = true;
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight.ts b/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight.ts
index d5c7d8e..094f2d7 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight.ts
@@ -54,6 +54,8 @@
rootId: string;
}
+const DEBOUNCER_SELECTION_CHANGE = 'selectionChange';
+
@customElement('gr-diff-highlight')
export class GrDiffHighlight extends GestureEventListeners(
LegacyElementMixin(PolymerElement)
@@ -88,6 +90,11 @@
);
}
+ /** @override */
+ detached() {
+ this.cancelDebouncer(DEBOUNCER_SELECTION_CHANGE);
+ }
+
get diffBuilder() {
if (!this._cachedDiffBuilder) {
this._cachedDiffBuilder = this.querySelector(
@@ -123,7 +130,7 @@
// ms, then you will have about 50 _handleSelection calls when doing a
// simple drag for select.
this.debounce(
- 'selectionChange',
+ DEBOUNCER_SELECTION_CHANGE,
() => this._handleSelection(selection, isMouseUp),
10
);
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts
index 1b1e71c..ae1e438 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts
@@ -72,7 +72,6 @@
import {FilesWebLinks} from '../gr-patch-range-select/gr-patch-range-select';
import {LineNumber, FILE} from '../gr-diff/gr-diff-line';
import {GrCommentThread} from '../../shared/gr-comment-thread/gr-comment-thread';
-import {KnownExperimentId} from '../../../services/flags/flags';
import {
firePageError,
fireAlert,
@@ -266,8 +265,6 @@
private readonly reporting = appContext.reportingService;
- private readonly flags = appContext.flagsService;
-
private readonly restApiService = appContext.restApiService;
private readonly jsAPI = appContext.jsApiService;
@@ -1147,10 +1144,6 @@
_showNewlineWarningRight(diff?: DiffInfo) {
return this._hasTrailingNewlines(diff, false) === false;
}
-
- _useNewContextControls() {
- return this.flags.isEnabled(KnownExperimentId.NEW_CONTEXT_CONTROLS);
- }
}
declare global {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_html.ts b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_html.ts
index 9d5555d..84a2e4a 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_html.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_html.ts
@@ -40,7 +40,6 @@
diff="[[diff]]"
show-newline-warning-left="[[_showNewlineWarningLeft(diff)]]"
show-newline-warning-right="[[_showNewlineWarningRight(diff)]]"
- use-new-context-controls="[[_useNewContextControls()]]"
>
</gr-diff>
<gr-syntax-layer
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-processor/gr-diff-processor.ts b/polygerrit-ui/app/elements/diff/gr-diff-processor/gr-diff-processor.ts
index 034081d..9fafcfa 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-processor/gr-diff-processor.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-processor/gr-diff-processor.ts
@@ -64,6 +64,8 @@
*/
const MAX_GROUP_SIZE = 120;
+const DEBOUNCER_RESET_IS_SCROLLING = 'resetIsScrolling';
+
/**
* Converts the API's `DiffContent`s to `GrDiffGroup`s for rendering.
*
@@ -123,6 +125,7 @@
/** @override */
detached() {
super.detached();
+ this.cancelDebouncer(DEBOUNCER_RESET_IS_SCROLLING);
this.cancel();
this.unlisten(window, 'scroll', '_handleWindowScroll');
}
@@ -130,7 +133,7 @@
_handleWindowScroll() {
this._isScrolling = true;
this.debounce(
- 'resetIsScrolling',
+ DEBOUNCER_RESET_IS_SCROLLING,
() => {
this._isScrolling = false;
},
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 1e9b1cc..62b34e9 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
@@ -85,7 +85,6 @@
import {PolymerDeepPropertyChange} from '@polymer/polymer/interfaces';
import {GrDiffCursor} from '../gr-diff-cursor/gr-diff-cursor';
import {CommentSide, DiffViewMode, Side} from '../../../constants/constants';
-import {hasOwnProperty} from '../../../utils/common-util';
import {GrApplyFixDialog} from '../gr-apply-fix-dialog/gr-apply-fix-dialog';
import {LineOfInterest} from '../gr-diff/gr-diff';
import {RevisionInfo as RevisionInfoObj} from '../../shared/revision-info/revision-info';
@@ -896,9 +895,8 @@
let baseCommit: CommitId | undefined;
if (!this._change) return;
if (!this._patchRange || !this._patchRange.patchNum) return;
- for (const commitSha in this._change.revisions) {
- if (!hasOwnProperty(this._change.revisions, commitSha)) continue;
- const revision = this._change.revisions[commitSha];
+ const revisions = this._change.revisions ?? {};
+ for (const [commitSha, revision] of Object.entries(revisions)) {
const patchNum = revision._number;
if (patchNum === this._patchRange.patchNum) {
commit = commitSha as CommitId;
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js
index 8fe06f8..34e74b6 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js
@@ -812,10 +812,7 @@
}
test('edit visible only when logged and status NEW', async () => {
- for (const changeStatus in ChangeStatus) {
- if (!ChangeStatus.hasOwnProperty(changeStatus)) {
- continue;
- }
+ for (const changeStatus of Object.keys(ChangeStatus)) {
assert.isFalse(await isEditVisibile({loggedIn: false, changeStatus}),
`loggedIn: false, changeStatus: ${changeStatus}`);
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.ts b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.ts
index 9fb5e74..7946061 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.ts
@@ -67,6 +67,7 @@
import * as shadow from 'shadow-selection-polyfill/shadow.js';
import {CreateCommentEventDetail as CreateCommentEventDetailApi} from '../../../api/diff';
+import {isSafari} from '../../../utils/dom-util';
const NO_NEWLINE_BASE = 'No newline at end of base file.';
const NO_NEWLINE_REVISION = 'No newline at end of revision file.';
@@ -290,6 +291,7 @@
/** @override */
detached() {
+ this.cancelDebouncer(RENDER_DIFF_TABLE_DEBOUNCE_NAME);
super.detached();
this._unobserveIncrementalNodes();
this._unobserveNodes();
@@ -355,11 +357,13 @@
_getShadowOrDocumentSelection() {
// When using native shadow DOM, the selection returned by
// document.getSelection() cannot reference the actual DOM elements making
- // up the diff, because they are in the shadow DOM of the gr-diff element.
- // This takes the shadow DOM selection if one exists.
+ // up the diff in Safari because they are in the shadow DOM of the gr-diff
+ // element. This takes the shadow DOM selection if one exists.
return this.root instanceof ShadowRoot && this.root.getSelection
? this.root.getSelection()
- : shadow.getRange(this.root);
+ : isSafari()
+ ? shadow.getRange(this.root)
+ : document.getSelection();
}
_observeNodes() {
diff --git a/polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view.ts b/polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view.ts
index 84ca10a..e9637af 100644
--- a/polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view.ts
+++ b/polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view.ts
@@ -55,6 +55,8 @@
const STORAGE_DEBOUNCE_INTERVAL_MS = 100;
+const DEBOUNCER_STORE = 'store';
+
@customElement('gr-editor-view')
export class GrEditorView extends KeyboardShortcutMixin(
GestureEventListeners(LegacyElementMixin(PolymerElement))
@@ -145,6 +147,11 @@
});
}
+ /** @override */
+ detached() {
+ this.cancelDebouncer(DEBOUNCER_STORE);
+ }
+
get storageKey() {
return `c${this._changeNum}_ps${this._patchNum}_${this._path}`;
}
@@ -348,7 +355,7 @@
_handleContentChange(e: CustomEvent<{value: string}>) {
this.debounce(
- 'store',
+ DEBOUNCER_STORE,
() => {
const content = e.detail.value;
if (content) {
diff --git a/polygerrit-ui/app/elements/gr-app-element.ts b/polygerrit-ui/app/elements/gr-app-element.ts
index 72e24f7..cdc9f98 100644
--- a/polygerrit-ui/app/elements/gr-app-element.ts
+++ b/polygerrit-ui/app/elements/gr-app-element.ts
@@ -79,6 +79,7 @@
import {ViewState} from '../types/types';
import {EventType} from '../utils/event-util';
import {GerritView} from '../services/router/router-model';
+import {windowLocationReload} from '../utils/dom-util';
interface ErrorInfo {
text: string;
@@ -241,7 +242,7 @@
// Ideally individual views should handle this event and respond with a soft
// reload. This is a catch-all for all views that cannot or have not
// implemented that.
- this.addEventListener('reload', () => window.location.reload());
+ this.addEventListener('reload', () => windowLocationReload());
}
/** @override */
diff --git a/polygerrit-ui/app/elements/plugins/gr-styles-api/gr-styles-api_test.js b/polygerrit-ui/app/elements/plugins/gr-styles-api/gr-styles-api_test.js
index c41b551..1c606cd 100644
--- a/polygerrit-ui/app/elements/plugins/gr-styles-api/gr-styles-api_test.js
+++ b/polygerrit-ui/app/elements/plugins/gr-styles-api/gr-styles-api_test.js
@@ -172,12 +172,10 @@
}
function assertDisplayPropertyValues(elements, expectedDisplayValues) {
- for (const key in elements) {
- if (elements.hasOwnProperty(key)) {
- assert.equal(
- getComputedStyle(elements[key]).getPropertyValue('display'),
- expectedDisplayValues[key]);
- }
+ for (let i = 0; i < elements.length; i++) {
+ assert.equal(
+ getComputedStyle(elements[i]).getPropertyValue('display'),
+ expectedDisplayValues[i]);
}
}
});
diff --git a/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog_test.js b/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog_test.js
index ce9c106..1088585 100644
--- a/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog_test.js
+++ b/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog_test.js
@@ -61,10 +61,8 @@
});
teardown(() => {
- for (const eventType in _listeners) {
- if (_listeners.hasOwnProperty(eventType)) {
- element.removeEventListener(eventType, _listeners[eventType]);
- }
+ for (const [eventType, listeners] of Object.entries(_listeners)) {
+ element.removeEventListener(eventType, listeners);
}
});
diff --git a/polygerrit-ui/app/elements/settings/gr-watched-projects-editor/gr-watched-projects-editor.ts b/polygerrit-ui/app/elements/settings/gr-watched-projects-editor/gr-watched-projects-editor.ts
index 36444a3..a45e160 100644
--- a/polygerrit-ui/app/elements/settings/gr-watched-projects-editor/gr-watched-projects-editor.ts
+++ b/polygerrit-ui/app/elements/settings/gr-watched-projects-editor/gr-watched-projects-editor.ts
@@ -121,14 +121,8 @@
_getProjectSuggestions(input: string) {
return this.restApiService.getSuggestedProjects(input).then(response => {
const projects: AutocompleteSuggestion[] = [];
- for (const key in response) {
- if (!hasOwnProperty(response, key)) {
- continue;
- }
- projects.push({
- name: key,
- value: response[key].id,
- });
+ for (const [name, project] of Object.entries(response ?? {})) {
+ projects.push({name, value: project.id});
}
return projects;
});
diff --git a/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.ts b/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.ts
index 6151a1d9..f4eb053 100644
--- a/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.ts
+++ b/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.ts
@@ -68,6 +68,8 @@
AutocompleteCommitEventDetail
>;
+const DEBOUNCER_UPDATE_SUGGESTIONS = 'update-suggestions';
+
@customElement('gr-autocomplete')
export class GrAutocomplete extends KeyboardShortcutMixin(
GestureEventListeners(LegacyElementMixin(PolymerElement))
@@ -217,7 +219,7 @@
detached() {
super.detached();
this.unlisten(document.body, 'click', '_handleBodyClick');
- this.cancelDebouncer('update-suggestions');
+ this.cancelDebouncer(DEBOUNCER_UPDATE_SUGGESTIONS);
}
get focusStart() {
@@ -330,7 +332,7 @@
if (noDebounce) {
update();
} else {
- this.debounce('update-suggestions', update, DEBOUNCE_WAIT_MS);
+ this.debounce(DEBOUNCER_UPDATE_SUGGESTIONS, update, DEBOUNCE_WAIT_MS);
}
}
diff --git a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
index 5d5f4f3..898aff3 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
@@ -100,6 +100,12 @@
};
}
+const DEBOUNCER_FIRE_UPDATE = 'fire-update';
+
+const DEBOUNCER_STORE = 'store';
+
+const DEBOUNCER_DRAFT_TOAST = 'draft-toast';
+
@customElement('gr-comment')
export class GrComment extends KeyboardShortcutMixin(
GestureEventListeners(LegacyElementMixin(PolymerElement))
@@ -293,7 +299,9 @@
/** @override */
detached() {
super.detached();
- this.cancelDebouncer('fire-update');
+ this.cancelDebouncer(DEBOUNCER_FIRE_UPDATE);
+ this.cancelDebouncer(DEBOUNCER_STORE);
+ this.cancelDebouncer(DEBOUNCER_DRAFT_TOAST);
if (this.textarea) {
this.textarea.closeDropdown();
}
@@ -486,7 +494,7 @@
_eraseDraftComment() {
// Prevents a race condition in which removing the draft comment occurs
// prior to it being saved.
- this.cancelDebouncer('store');
+ this.cancelDebouncer(DEBOUNCER_STORE);
if (!this.comment?.path) throw new Error('Cannot erase Draft Comment');
if (this.changeNum === undefined) {
@@ -538,7 +546,7 @@
}
_fireUpdate() {
- this.debounce('fire-update', () => {
+ this.debounce(DEBOUNCER_FIRE_UPDATE, () => {
this.dispatchEvent(
new CustomEvent('comment-update', {
detail: this._getEventPayload(),
@@ -647,7 +655,7 @@
const {path, line, range} = this.comment;
if (path) {
this.debounce(
- 'store',
+ DEBOUNCER_STORE,
() => {
const message = this._messageText;
if (this.changeNum === undefined) {
@@ -729,7 +737,7 @@
}
_fireDiscard() {
- this.cancelDebouncer('fire-update');
+ this.cancelDebouncer(DEBOUNCER_FIRE_UPDATE);
this.dispatchEvent(
new CustomEvent('comment-discard', {
detail: this._getEventPayload(),
@@ -852,7 +860,7 @@
// Cancel the debouncer so that error toasts from the error-manager will
// not be overridden.
- this.cancelDebouncer('draft-toast');
+ this.cancelDebouncer(DEBOUNCER_DRAFT_TOAST);
this._updateRequestToast(
this._numPendingDraftRequests.number,
/* requestFailed=*/ true
@@ -862,7 +870,7 @@
_updateRequestToast(numPending: number, requestFailed?: boolean) {
const message = this._getSavingMessage(numPending, requestFailed);
this.debounce(
- 'draft-toast',
+ DEBOUNCER_DRAFT_TOAST,
() => {
// Note: the event is fired on the body rather than this element because
// this element may not be attached by the time this executes, in which
diff --git a/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content.ts b/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content.ts
index e825d21..c744eab 100644
--- a/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content.ts
+++ b/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content.ts
@@ -37,6 +37,8 @@
}
}
+const DEBOUNCER_STORE = 'store';
+
@customElement('gr-editable-content')
export class GrEditableContent extends GestureEventListeners(
LegacyElementMixin(PolymerElement)
@@ -126,6 +128,11 @@
);
}
+ /** @override */
+ detached() {
+ this.cancelDebouncer(DEBOUNCER_STORE);
+ }
+
_contentChanged() {
/* A changed content means that either a different change has been loaded
* or new content was saved. Either way, let's reset the component.
@@ -143,7 +150,7 @@
const storageKey = this.storageKey;
this.debounce(
- 'store',
+ DEBOUNCER_STORE,
() => {
if (newContent.length) {
this.storage.setEditableContentItem(storageKey, newContent);
diff --git a/polygerrit-ui/app/elements/shared/gr-hovercard/gr-hovercard-behavior.ts b/polygerrit-ui/app/elements/shared/gr-hovercard/gr-hovercard-behavior.ts
index 94f99d3..e67e1f6 100644
--- a/polygerrit-ui/app/elements/shared/gr-hovercard/gr-hovercard-behavior.ts
+++ b/polygerrit-ui/app/elements/shared/gr-hovercard/gr-hovercard-behavior.ts
@@ -143,6 +143,8 @@
detached() {
super.detached();
+ this.cancelShowDebouncer();
+ this.cancelHideDebouncer();
this.unlock();
}
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-action-context.ts b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-action-context.ts
index 2149fe4..d4a2bd6 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-action-context.ts
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-action-context.ts
@@ -19,6 +19,7 @@
import {ShowAlertEventDetail} from '../../../types/events';
import {PluginApi} from '../../plugins/gr-plugin-types';
import {UIActionInfo} from './gr-change-actions-js-api';
+import {windowLocationReload} from '../../../utils/dom-util';
interface GrPopupInterface {
close(): void;
@@ -57,7 +58,7 @@
}
refresh() {
- window.location.reload();
+ windowLocationReload();
}
textfield(): HTMLElement {
diff --git a/polygerrit-ui/app/elements/shared/gr-linked-text/link-text-parser.ts b/polygerrit-ui/app/elements/shared/gr-linked-text/link-text-parser.ts
index 9066911..36f518b 100644
--- a/polygerrit-ui/app/elements/shared/gr-linked-text/link-text-parser.ts
+++ b/polygerrit-ui/app/elements/shared/gr-linked-text/link-text-parser.ts
@@ -350,27 +350,27 @@
// The outputArray is used to store all of the matches found for all
// patterns.
const outputArray: CommentLinkItem[] = [];
- for (const p in config) {
+ for (const [configName, linkInfo] of Object.entries(config)) {
// TODO(TS): it seems, the following line can be rewritten as:
// if(enabled === false || enabled === 0 || enabled === '')
// Should be double-checked before update
// eslint-disable-next-line eqeqeq
- if (config[p].enabled != null && config[p].enabled == false) {
+ if (linkInfo.enabled != null && linkInfo.enabled == false) {
continue;
}
// PolyGerrit doesn't use hash-based navigation like the GWT UI.
// Account for this.
- const html = config[p].html;
- const link = config[p].link;
+ const html = linkInfo.html;
+ const link = linkInfo.link;
if (html) {
- config[p].html = html.replace(/<a href="#\//g, '<a href="/');
+ linkInfo.html = html.replace(/<a href="#\//g, '<a href="/');
} else if (link) {
if (link[0] === '#') {
- config[p].link = link.substr(1);
+ linkInfo.link = link.substr(1);
}
}
- const pattern = new RegExp(config[p].match, 'g');
+ const pattern = new RegExp(linkInfo.match, 'g');
let match;
let textToCheck = text;
@@ -382,10 +382,10 @@
pattern,
// Either html or link has a value. Otherwise an exception is thrown
// in the code below.
- (config[p].html || config[p].link)!
+ (linkInfo.html || linkInfo.link)!
);
- if (config[p].html) {
+ if (linkInfo.html) {
let i;
// Skip portion of replacement string that is equal to original to
// allow overlapping patterns.
@@ -402,7 +402,7 @@
match[0].length - i,
outputArray
);
- } else if (config[p].link) {
+ } else if (linkInfo.link) {
this.addLink(
match[0],
result,
@@ -413,7 +413,7 @@
} else {
throw Error(
'linkconfig entry ' +
- p +
+ configName +
' doesn’t contain a link or html attribute.'
);
}
diff --git a/polygerrit-ui/app/elements/shared/gr-list-view/gr-list-view.ts b/polygerrit-ui/app/elements/shared/gr-list-view/gr-list-view.ts
index 97b45ac..bf532ef 100644
--- a/polygerrit-ui/app/elements/shared/gr-list-view/gr-list-view.ts
+++ b/polygerrit-ui/app/elements/shared/gr-list-view/gr-list-view.ts
@@ -35,6 +35,8 @@
}
}
+const DEBOUNCER_RELOAD = 'reload';
+
@customElement('gr-list-view')
class GrListView extends GestureEventListeners(
LegacyElementMixin(PolymerElement)
@@ -67,7 +69,7 @@
/** @override */
detached() {
super.detached();
- this.cancelDebouncer('reload');
+ this.cancelDebouncer(DEBOUNCER_RELOAD);
}
_filterChanged(newFilter?: string, oldFilter?: string) {
@@ -81,7 +83,7 @@
_debounceReload(filter?: string) {
this.debounce(
- 'reload',
+ DEBOUNCER_RELOAD,
() => {
if (this.path) {
if (filter) {
diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.ts b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.ts
index 355691f..84d84b1 100644
--- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.ts
+++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.ts
@@ -259,27 +259,6 @@
type SendChangeRequest = SendRawChangeRequest | SendJSONChangeRequest;
export function _testOnlyResetGrRestApiSharedObjects() {
- // TODO(TS): The commented code below didn't do anything.
- // It is impossible to reject an existing promise. Should be rewritten in a
- // different way
- // const fetchPromisesCacheData = fetchPromisesCache.testOnlyGetData();
- // for (const key in fetchPromisesCacheData) {
- // if (hasOwnProperty(fetchPromisesCacheData, key)) {
- // // reject already fulfilled promise does nothing
- // fetchPromisesCacheData[key]!.reject();
- // }
- // }
- //
- // for (const key in pendingRequest) {
- // if (!hasOwnProperty(pendingRequest, key)) {
- // continue;
- // }
- // for (const req of pendingRequest[key]) {
- // // reject already fulfilled promise does nothing
- // req.reject();
- // }
- // }
-
siteBasedCache = new SiteBasedCache();
fetchPromisesCache = new FetchPromisesCache();
pendingRequest = {};
diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper.ts b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper.ts
index d6b6645..710445c 100644
--- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper.ts
+++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper.ts
@@ -23,7 +23,6 @@
AuthRequestInit,
AuthService,
} from '../../../../services/gr-auth/gr-auth';
-import {hasOwnProperty} from '../../../../utils/common-util';
import {
AccountDetailInfo,
EmailInfo,
@@ -379,11 +378,7 @@
}
const params: Array<string | number | boolean> = [];
- for (const p in fetchParams) {
- if (!hasOwnProperty(fetchParams, p)) {
- continue;
- }
- const paramValue = fetchParams[p];
+ for (const [p, paramValue] of Object.entries(fetchParams)) {
// TODO(TS): Replace == null with === and check for null and undefined
// eslint-disable-next-line eqeqeq
if (paramValue == null) {
@@ -482,11 +477,8 @@
if (!options.headers) {
options.headers = new Headers();
}
- for (const header in req.headers) {
- if (!hasOwnProperty(req.headers, header)) {
- continue;
- }
- options.headers.set(header, req.headers[header]);
+ for (const [name, value] of Object.entries(req.headers)) {
+ options.headers.set(name, value);
}
}
const url = req.url.startsWith('http') ? req.url : getBaseUrl() + req.url;
diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-reviewer-updates-parser.ts b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-reviewer-updates-parser.ts
index 809b78b..1a1062c 100644
--- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-reviewer-updates-parser.ts
+++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-reviewer-updates-parser.ts
@@ -25,7 +25,6 @@
ReviewerUpdateInfo,
Timestamp,
} from '../../../types/common';
-import {hasOwnProperty} from '../../../utils/common-util';
import {accountKey} from '../../../utils/account-util';
import {
FormattedReviewerUpdateInfo,
@@ -122,12 +121,10 @@
*/
private _completeBatch(batch: ParserBatch) {
const items = [];
- for (const accountId in this._updateItems) {
- if (!hasOwnProperty(this._updateItems, accountId)) continue;
- const updateItem = this._updateItems[accountId];
- if (this._lastState[accountId] !== updateItem.state) {
- this._lastState[accountId] = updateItem.state;
- items.push(updateItem);
+ for (const [accountId, item] of Object.entries(this._updateItems ?? {})) {
+ if (this._lastState[accountId] !== item.state) {
+ this._lastState[accountId] = item.state;
+ items.push(item);
}
}
if (items.length) {
@@ -233,15 +230,10 @@
const reviewerUpdates = (this.result
.reviewer_updates as unknown) as ParserBatchWithNonEmptyUpdates[];
for (const update of reviewerUpdates) {
- const grouppedReviewers = this._groupUpdatesByMessage(update.updates);
+ const groupedReviewers = this._groupUpdatesByMessage(update.updates);
const newUpdates: {message: string; reviewers: AccountInfo[]}[] = [];
- for (const message in grouppedReviewers) {
- if (hasOwnProperty(grouppedReviewers, message)) {
- newUpdates.push({
- message,
- reviewers: grouppedReviewers[message],
- });
- }
+ for (const [message, reviewers] of Object.entries(groupedReviewers)) {
+ newUpdates.push({message, reviewers});
}
((update as unknown) as FormattedReviewerUpdateInfo).updates = newUpdates;
}
diff --git a/polygerrit-ui/app/node_modules_licenses/BUILD b/polygerrit-ui/app/node_modules_licenses/BUILD
index ad5c676..44b9811 100644
--- a/polygerrit-ui/app/node_modules_licenses/BUILD
+++ b/polygerrit-ui/app/node_modules_licenses/BUILD
@@ -15,7 +15,7 @@
tsconfig = "tsconfig.json",
deps = [
"//tools/node_tools/node_modules_licenses:licenses-map",
- "@tools_npm//:node_modules",
+ "@tools_npm//@bazel/typescript",
"@tools_npm//@types/node",
],
)
diff --git a/polygerrit-ui/app/services/flags/flags.ts b/polygerrit-ui/app/services/flags/flags.ts
index bf62d0d0..891716b 100644
--- a/polygerrit-ui/app/services/flags/flags.ts
+++ b/polygerrit-ui/app/services/flags/flags.ts
@@ -24,7 +24,6 @@
* @desc Experiment ids used in Gerrit.
*/
export enum KnownExperimentId {
- NEW_CONTEXT_CONTROLS = 'UiFeature__new_context_controls',
// Note that this flag is not supposed to be used by Gerrit itself, but can
// be used by plugins. The new Checks UI will show up, if a plugin registers
// with the new Checks plugin API.
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 a6472d1..2b42a6b 100644
--- a/polygerrit-ui/app/services/gr-reporting/gr-reporting_mock.ts
+++ b/polygerrit-ui/app/services/gr-reporting/gr-reporting_mock.ts
@@ -14,7 +14,7 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-import {ReportingService, Timer} from './gr-reporting';
+import {EventDetails, ReportingService, Timer} from './gr-reporting';
export class MockTimer implements Timer {
end(): this {
@@ -30,6 +30,10 @@
}
}
+const log = function (msg: string) {
+ console.info(`ReportingMock.${msg}`);
+};
+
export const grReportingMock: ReportingService = {
appStarted: () => {},
beforeLocationChanged: () => {},
@@ -43,17 +47,29 @@
getTimer: () => {
return new MockTimer();
},
- locationChanged: () => {},
- onVisibilityChange: () => {},
+ locationChanged: (page: string) => {
+ log(`locationChanged: ${page}`);
+ },
+ onVisibilityChange: () => {
+ log('onVisibilityChange');
+ },
pluginLoaded: () => {},
pluginsLoaded: () => {},
recordDraftInteraction: () => {},
reporter: () => {},
- reportErrorDialog: () => {},
- error: () => {},
- reportExecution: () => {},
+ reportErrorDialog: (message: string) => {
+ log(`reportErrorDialog: ${message}`);
+ },
+ error: () => {
+ log('error');
+ },
+ reportExecution: (id: string, details: EventDetails) => {
+ log(`reportExecution '${id}': ${JSON.stringify(details)}`);
+ },
reportExtension: () => {},
- reportInteraction: () => {},
+ reportInteraction: (eventName: string, details?: EventDetails) => {
+ log(`reportInteraction '${eventName}': ${JSON.stringify(details)}`);
+ },
reportLifeCycle: () => {},
reportRpcTiming: () => {},
setRepoName: () => {},
diff --git a/polygerrit-ui/app/test/common-test-setup.ts b/polygerrit-ui/app/test/common-test-setup.ts
index beba88e..0f394be 100644
--- a/polygerrit-ui/app/test/common-test-setup.ts
+++ b/polygerrit-ui/app/test/common-test-setup.ts
@@ -43,7 +43,6 @@
_testOnly_defaultResinReportHandler,
installPolymerResin,
} from '../scripts/polymer-resin-install';
-import {hasOwnProperty} from '../utils/common-util';
declare global {
interface Window {
@@ -132,12 +131,9 @@
// This method is inspired by web-component-tester method
const proto = document.createElement(tagName).constructor
.prototype as HTMLElementTagNameMap[T];
- let key: keyof HTMLElementTagNameMap[T];
const stubs: SinonSpy[] = [];
- for (key in implementation) {
- if (hasOwnProperty(implementation, key)) {
- stubs.push(sinon.stub(proto, key).callsFake(implementation[key]));
- }
+ for (const [key, value] of Object.entries(implementation)) {
+ stubs.push(sinon.stub(proto, key).callsFake(value));
}
registerTestCleanup(() => {
stubs.forEach(stub => {
@@ -199,5 +195,10 @@
checkGlobalSpace();
removeIronOverlayBackdropStyleEl();
// Clean Polymer debouncer queue, so next tests will not be affected.
+ // WARNING! This will most likely not do what you expect. `flushDebouncers()`
+ // will only flush debouncers that were added using `enqueueDebouncer()`. So
+ // this will not affect "normal" debouncers that were added using
+ // `this.debounce()`. For those please be careful and cancel them using
+ // `this.cancelDebouncer()` in the `detached()` lifecycle hook.
flushDebouncers();
});
diff --git a/polygerrit-ui/app/utils/dom-util.ts b/polygerrit-ui/app/utils/dom-util.ts
index c9a7d6b..aa83173 100644
--- a/polygerrit-ui/app/utils/dom-util.ts
+++ b/polygerrit-ui/app/utils/dom-util.ts
@@ -153,6 +153,12 @@
return [...results];
}
+export function windowLocationReload() {
+ const e = new Error();
+ console.info(`Calling window.location.realod(): ${e.stack}`);
+ window.location.reload();
+}
+
/**
* Retrieves the dom path of the current event.
*
@@ -244,3 +250,11 @@
}
return root.activeElement as HTMLElement;
}
+
+// Whether the browser is Safari. Used for polyfilling unique browser behavior.
+export function isSafari() {
+ return (
+ /^((?!chrome|android).)*safari/i.test(navigator.userAgent) ||
+ (/iPad|iPhone|iPod/.test(navigator.userAgent) && !window.MSStream)
+ );
+}
diff --git a/tools/node_tools/node_modules_licenses/BUILD b/tools/node_tools/node_modules_licenses/BUILD
index 051cfe8..b88ec24 100644
--- a/tools/node_tools/node_modules_licenses/BUILD
+++ b/tools/node_tools/node_modules_licenses/BUILD
@@ -10,7 +10,7 @@
compiler = "//tools/node_tools:tsc_wrapped-bin",
tsconfig = "tsconfig.json",
deps = [
- "@tools_npm//:node_modules",
+ "@tools_npm//@bazel/typescript",
"@tools_npm//@types/node",
],
)
diff --git a/tools/node_tools/package.json b/tools/node_tools/package.json
index ba93c94..9acbd07 100644
--- a/tools/node_tools/package.json
+++ b/tools/node_tools/package.json
@@ -3,8 +3,8 @@
"description": "Gerrit Build Tools",
"browser": false,
"dependencies": {
- "@bazel/rollup": "^3.1.0",
- "@bazel/typescript": "^3.1.0",
+ "@bazel/rollup": "^3.2.0",
+ "@bazel/typescript": "^3.2.0",
"@types/node": "^10.17.12",
"@types/parse5": "^4.0.0",
"@types/parse5-html-rewriting-stream": "^5.1.2",
diff --git a/tools/node_tools/yarn.lock b/tools/node_tools/yarn.lock
index 28d787e..45a0c89 100644
--- a/tools/node_tools/yarn.lock
+++ b/tools/node_tools/yarn.lock
@@ -492,15 +492,15 @@
lodash "^4.17.13"
to-fast-properties "^2.0.0"
-"@bazel/rollup@^3.1.0":
- version "3.1.0"
- resolved "https://registry.yarnpkg.com/@bazel/rollup/-/rollup-3.1.0.tgz#36346f052b2ce3c1e31e5ebb05ed80464548eb00"
- integrity sha512-lmgPhlR1VsJRsSE83Jlv+WT26Lso0/0FqXknlVuOmvCWFwSUKlriws729fqJZsvV5O2muAgJKuQl/zk+gqGCug==
+"@bazel/rollup@^3.2.0":
+ version "3.2.0"
+ resolved "https://registry.yarnpkg.com/@bazel/rollup/-/rollup-3.2.0.tgz#4241a5767e12e57b01279a539af2537c2d01924a"
+ integrity sha512-Wkw6L+hor/+FzpDswri7IlWAbKyShnUZRx59fG06+qqVhpNaS3V3lnZqVytMlLLT4oSP8YSIzoXC5GkXgLI2/Q==
-"@bazel/typescript@^3.1.0":
- version "3.1.0"
- resolved "https://registry.yarnpkg.com/@bazel/typescript/-/typescript-3.1.0.tgz#a07999ad7956b8c624604a521e653570bba32025"
- integrity sha512-sEWuvkUGIDeRhjLENHtJyop7wu4UqKN8h/nSgUwc5gkpWXQiT2wGH5jKVxBqODOBHB+IInEMtAjyRmCT+HbSHA==
+"@bazel/typescript@^3.2.0":
+ version "3.2.0"
+ resolved "https://registry.yarnpkg.com/@bazel/typescript/-/typescript-3.2.0.tgz#299bd173fe04f98407ab9be4f654662c1c28470e"
+ integrity sha512-RKdy9ThbcUAqZR3AJK7AR/nxbJqdHi7pPayIGUSMIpxVkeTxVRQpf1aGe2H02HdZ9fR/uk1xXhO/Ff9TLvTgHQ==
dependencies:
protobufjs "6.8.8"
semver "5.6.0"
diff --git a/tools/nongoogle.bzl b/tools/nongoogle.bzl
index 62db566..e8f12c8 100644
--- a/tools/nongoogle.bzl
+++ b/tools/nongoogle.bzl
@@ -1,8 +1,8 @@
load("//tools/bzl:maven_jar.bzl", "maven_jar")
-GUAVA_VERSION = "29.0-jre"
+GUAVA_VERSION = "30.1-jre"
-GUAVA_BIN_SHA1 = "801142b4c3d0f0770dd29abea50906cacfddd447"
+GUAVA_BIN_SHA1 = "00d0c3ce2311c9e36e73228da25a6e99b2ab826f"
GUAVA_DOC_URL = "https://google.github.io/guava/releases/" + GUAVA_VERSION + "/api/docs/"
diff --git a/yarn.lock b/yarn.lock
index 170b18a..a424d79 100644
--- a/yarn.lock
+++ b/yarn.lock
@@ -485,20 +485,20 @@
lodash "^4.17.11"
to-fast-properties "^2.0.0"
-"@bazel/rollup@^3.1.0":
- version "3.1.0"
- resolved "https://registry.yarnpkg.com/@bazel/rollup/-/rollup-3.1.0.tgz#36346f052b2ce3c1e31e5ebb05ed80464548eb00"
- integrity sha512-lmgPhlR1VsJRsSE83Jlv+WT26Lso0/0FqXknlVuOmvCWFwSUKlriws729fqJZsvV5O2muAgJKuQl/zk+gqGCug==
+"@bazel/rollup@^3.2.0":
+ version "3.2.0"
+ resolved "https://registry.yarnpkg.com/@bazel/rollup/-/rollup-3.2.0.tgz#4241a5767e12e57b01279a539af2537c2d01924a"
+ integrity sha512-Wkw6L+hor/+FzpDswri7IlWAbKyShnUZRx59fG06+qqVhpNaS3V3lnZqVytMlLLT4oSP8YSIzoXC5GkXgLI2/Q==
-"@bazel/terser@^3.1.0":
- version "3.1.0"
- resolved "https://registry.yarnpkg.com/@bazel/terser/-/terser-3.1.0.tgz#5801e83d4ac648fb1a8824a77a1a1f32c3af0c1e"
- integrity sha512-8oXZwy5G5dbr4zltBzLjfPw4ZARDEysB2E25dCqAo64XJ26ptS+D3/UnE3uZU9KuM/3ka1U+YIpit+f9SqCgTA==
+"@bazel/terser@^3.2.0":
+ version "3.2.0"
+ resolved "https://registry.yarnpkg.com/@bazel/terser/-/terser-3.2.0.tgz#e53ad32733a0b231323b9eb55ebc2a3c65b10223"
+ integrity sha512-/yq4gST3t1mETkP6NjC05yEyIIL//4mbfLI56hE3CC/mm/xJ6UeooFVpUdlJREQEDRAdNWoiMesQ1ZtgpNPzFg==
-"@bazel/typescript@^3.1.0":
- version "3.1.0"
- resolved "https://registry.yarnpkg.com/@bazel/typescript/-/typescript-3.1.0.tgz#a07999ad7956b8c624604a521e653570bba32025"
- integrity sha512-sEWuvkUGIDeRhjLENHtJyop7wu4UqKN8h/nSgUwc5gkpWXQiT2wGH5jKVxBqODOBHB+IInEMtAjyRmCT+HbSHA==
+"@bazel/typescript@^3.2.0":
+ version "3.2.0"
+ resolved "https://registry.yarnpkg.com/@bazel/typescript/-/typescript-3.2.0.tgz#299bd173fe04f98407ab9be4f654662c1c28470e"
+ integrity sha512-RKdy9ThbcUAqZR3AJK7AR/nxbJqdHi7pPayIGUSMIpxVkeTxVRQpf1aGe2H02HdZ9fR/uk1xXhO/Ff9TLvTgHQ==
dependencies:
protobufjs "6.8.8"
semver "5.6.0"
