Merge "Evaluate and return submit requirements with ChangeInfo"
diff --git a/Documentation/config-labels.txt b/Documentation/config-labels.txt
index f5346c1..b4e65b0 100644
--- a/Documentation/config-labels.txt
+++ b/Documentation/config-labels.txt
@@ -280,7 +280,7 @@
Gerrit currently supports the following predicates:
-==== change-kind:{REWORK,TRIVIAL_REBASE,MERGE_FIRST_PARENT_UPDATE,NO_CODE_CHANGE,NO_CHANGE}
+==== changekind:{REWORK,TRIVIAL_REBASE,MERGE_FIRST_PARENT_UPDATE,NO_CODE_CHANGE,NO_CHANGE}
Matches if the diff between two patch sets was of a certain change kind.
diff --git a/java/com/google/gerrit/server/CommentsUtil.java b/java/com/google/gerrit/server/CommentsUtil.java
index b752791..344549e 100644
--- a/java/com/google/gerrit/server/CommentsUtil.java
+++ b/java/com/google/gerrit/server/CommentsUtil.java
@@ -42,8 +42,9 @@
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.ChangeUpdate;
-import com.google.gerrit.server.patch.PatchListCache;
-import com.google.gerrit.server.patch.PatchListNotAvailableException;
+import com.google.gerrit.server.patch.DiffNotAvailableException;
+import com.google.gerrit.server.patch.DiffOperations;
+import com.google.gerrit.server.patch.filediff.FileDiffOutput;
import com.google.gerrit.server.update.ChangeContext;
import com.google.inject.Inject;
import com.google.inject.Singleton;
@@ -52,6 +53,7 @@
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
+import java.util.Map;
import java.util.Optional;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Ref;
@@ -108,21 +110,21 @@
private static final Ordering<Comparable<?>> NULLS_FIRST = Ordering.natural().nullsFirst();
+ private final DiffOperations diffOperations;
private final GitRepositoryManager repoManager;
private final AllUsersName allUsers;
private final String serverId;
- private final PatchListCache patchListCache;
@Inject
CommentsUtil(
+ DiffOperations diffOperations,
GitRepositoryManager repoManager,
AllUsersName allUsers,
- @GerritServerId String serverId,
- PatchListCache patchListCache) {
+ @GerritServerId String serverId) {
+ this.diffOperations = diffOperations;
this.repoManager = repoManager;
this.allUsers = allUsers;
this.serverId = serverId;
- this.patchListCache = patchListCache;
}
public HumanComment newHumanComment(
@@ -411,7 +413,7 @@
int parentNumber = Math.abs(side);
return resolveParentCommit(change.getProject(), patchset, parentNumber);
}
- return Optional.of(resolveAutoMergeCommit(change, patchset));
+ return Optional.ofNullable(resolveAutoMergeCommit(change, patchset));
}
return Optional.of(patchset.commitId());
}
@@ -429,12 +431,18 @@
}
}
+ @Nullable
private ObjectId resolveAutoMergeCommit(Change change, PatchSet patchset) {
try {
// TODO(ghareeb): Adjust after the auto-merge code was moved out of the diff caches. Also
// unignore the test in PortedCommentsIT.
- return patchListCache.getOldId(change, patchset, null);
- } catch (PatchListNotAvailableException e) {
+ Map<String, FileDiffOutput> modifiedFiles =
+ diffOperations.listModifiedFilesAgainstParent(
+ change.getProject(), patchset.commitId(), /* parentNum= */ null);
+ return modifiedFiles.isEmpty()
+ ? null
+ : modifiedFiles.values().iterator().next().oldCommitId();
+ } catch (DiffNotAvailableException e) {
throw new StorageException(e);
}
}
diff --git a/java/com/google/gerrit/server/patch/AutoMerger.java b/java/com/google/gerrit/server/patch/AutoMerger.java
index b99e2d2..8d1e0ff 100644
--- a/java/com/google/gerrit/server/patch/AutoMerger.java
+++ b/java/com/google/gerrit/server/patch/AutoMerger.java
@@ -31,6 +31,8 @@
import com.google.gerrit.server.logging.Metadata;
import com.google.gerrit.server.update.RepoView;
import com.google.inject.Inject;
+import com.google.inject.Provider;
+import com.google.inject.Singleton;
import java.io.IOException;
import java.util.Optional;
import org.eclipse.jgit.dircache.DirCache;
@@ -70,6 +72,7 @@
* <p>The second point means that these commits are referenced from NoteDb. The consequence of this
* is that these refs should never be deleted.
*/
+@Singleton
public class AutoMerger {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
@@ -88,7 +91,7 @@
private final Counter1<OperationType> counter;
private final Timer1<OperationType> latency;
- private final PersonIdent gerritIdent;
+ private final Provider<PersonIdent> gerritIdentProvider;
private final boolean save;
private final ThreeWayMergeStrategy configuredMergeStrategy;
@@ -96,7 +99,7 @@
AutoMerger(
MetricMaker metricMaker,
@GerritServerConfig Config cfg,
- @GerritPersonIdent PersonIdent gerritIdent) {
+ @GerritPersonIdent Provider<PersonIdent> gerritIdentProvider) {
this.counter =
metricMaker.newCounter(
"git/auto-merge/num_operations",
@@ -110,7 +113,7 @@
.setUnit("milliseconds"),
Field.ofEnum(OperationType.class, "type", Metadata.Builder::operationName).build());
this.save = cacheAutomerge(cfg);
- this.gerritIdent = gerritIdent;
+ this.gerritIdentProvider = gerritIdentProvider;
this.configuredMergeStrategy = MergeUtil.getMergeStrategy(cfg);
}
@@ -224,7 +227,9 @@
// the input commit, using the server name and timezone.
PersonIdent ident =
new PersonIdent(
- gerritIdent, merge.getCommitterIdent().getWhen(), gerritIdent.getTimeZone());
+ gerritIdentProvider.get(),
+ merge.getCommitterIdent().getWhen(),
+ gerritIdentProvider.get().getTimeZone());
CommitBuilder cb = new CommitBuilder();
cb.setAuthor(ident);
cb.setCommitter(ident);
diff --git a/java/com/google/gerrit/server/patch/BaseCommitUtil.java b/java/com/google/gerrit/server/patch/BaseCommitUtil.java
index 7c06a62..dd930378 100644
--- a/java/com/google/gerrit/server/patch/BaseCommitUtil.java
+++ b/java/com/google/gerrit/server/patch/BaseCommitUtil.java
@@ -21,6 +21,7 @@
import com.google.gerrit.server.git.InMemoryInserter;
import com.google.gerrit.server.git.MergeUtil;
import com.google.inject.Inject;
+import com.google.inject.Singleton;
import java.io.IOException;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.Constants;
@@ -34,6 +35,7 @@
import org.eclipse.jgit.revwalk.RevWalk;
/** A utility class for computing the base commit / parent for a specific patchset commit. */
+@Singleton
class BaseCommitUtil {
private final AutoMerger autoMerger;
private final ThreeWayMergeStrategy mergeStrategy;
diff --git a/java/com/google/gerrit/server/patch/DiffOperationsImpl.java b/java/com/google/gerrit/server/patch/DiffOperationsImpl.java
index dbbb7a6..fd30868 100644
--- a/java/com/google/gerrit/server/patch/DiffOperationsImpl.java
+++ b/java/com/google/gerrit/server/patch/DiffOperationsImpl.java
@@ -44,6 +44,7 @@
import com.google.gerrit.server.patch.gitfilediff.GitFileDiffCacheImpl.DiffAlgorithm;
import com.google.inject.Inject;
import com.google.inject.Module;
+import com.google.inject.Singleton;
import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
@@ -60,6 +61,7 @@
* Provides different file diff operations. Uses the underlying Git/Gerrit caches to speed up the
* diff computation.
*/
+@Singleton
public class DiffOperationsImpl implements DiffOperations {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
diff --git a/java/com/google/gerrit/server/patch/diff/ModifiedFilesCacheImpl.java b/java/com/google/gerrit/server/patch/diff/ModifiedFilesCacheImpl.java
index 6023c0e..e168386 100644
--- a/java/com/google/gerrit/server/patch/diff/ModifiedFilesCacheImpl.java
+++ b/java/com/google/gerrit/server/patch/diff/ModifiedFilesCacheImpl.java
@@ -34,6 +34,7 @@
import com.google.gerrit.server.patch.gitdiff.ModifiedFile;
import com.google.inject.Inject;
import com.google.inject.Module;
+import com.google.inject.Singleton;
import com.google.inject.TypeLiteral;
import com.google.inject.name.Named;
import java.io.IOException;
@@ -56,6 +57,7 @@
* and the result will be exactly the same as the caller can get from {@link
* GitModifiedFilesCache#get(GitModifiedFilesCacheKey)}
*/
+@Singleton
public class ModifiedFilesCacheImpl implements ModifiedFilesCache {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
diff --git a/java/com/google/gerrit/server/patch/filediff/FileDiffCacheImpl.java b/java/com/google/gerrit/server/patch/filediff/FileDiffCacheImpl.java
index 2133474..0794775 100644
--- a/java/com/google/gerrit/server/patch/filediff/FileDiffCacheImpl.java
+++ b/java/com/google/gerrit/server/patch/filediff/FileDiffCacheImpl.java
@@ -44,6 +44,7 @@
import com.google.gerrit.server.patch.gitfilediff.GitFileDiffCacheImpl.DiffAlgorithmFactory;
import com.google.inject.Inject;
import com.google.inject.Module;
+import com.google.inject.Singleton;
import com.google.inject.name.Named;
import java.io.IOException;
import java.util.ArrayList;
@@ -76,6 +77,7 @@
* org.eclipse.jgit.lib.Constants#EMPTY_TREE_ID}, the git diff will be evaluated against the empty
* tree.
*/
+@Singleton
public class FileDiffCacheImpl implements FileDiffCache {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
diff --git a/java/com/google/gerrit/server/patch/gitdiff/GitModifiedFilesCacheImpl.java b/java/com/google/gerrit/server/patch/gitdiff/GitModifiedFilesCacheImpl.java
index b3b82bb..9b96da1 100644
--- a/java/com/google/gerrit/server/patch/gitdiff/GitModifiedFilesCacheImpl.java
+++ b/java/com/google/gerrit/server/patch/gitdiff/GitModifiedFilesCacheImpl.java
@@ -29,6 +29,7 @@
import com.google.gerrit.server.patch.DiffNotAvailableException;
import com.google.inject.Inject;
import com.google.inject.Module;
+import com.google.inject.Singleton;
import com.google.inject.TypeLiteral;
import com.google.inject.name.Named;
import java.io.IOException;
@@ -43,6 +44,7 @@
import org.eclipse.jgit.util.io.DisabledOutputStream;
/** Implementation of the {@link GitModifiedFilesCache} */
+@Singleton
public class GitModifiedFilesCacheImpl implements GitModifiedFilesCache {
private static final String GIT_MODIFIED_FILES = "git_modified_files";
private static final ImmutableMap<ChangeType, Patch.ChangeType> changeTypeMap =
diff --git a/java/com/google/gerrit/server/patch/gitfilediff/GitFileDiffCacheImpl.java b/java/com/google/gerrit/server/patch/gitfilediff/GitFileDiffCacheImpl.java
index 97cf37d32..4bc6b87 100644
--- a/java/com/google/gerrit/server/patch/gitfilediff/GitFileDiffCacheImpl.java
+++ b/java/com/google/gerrit/server/patch/gitfilediff/GitFileDiffCacheImpl.java
@@ -31,6 +31,7 @@
import com.google.gerrit.server.patch.DiffNotAvailableException;
import com.google.inject.Inject;
import com.google.inject.Module;
+import com.google.inject.Singleton;
import com.google.inject.name.Named;
import java.io.IOException;
import java.util.Collection;
@@ -52,6 +53,7 @@
import org.eclipse.jgit.util.io.DisabledOutputStream;
/** Implementation of the {@link GitFileDiffCache} */
+@Singleton
public class GitFileDiffCacheImpl implements GitFileDiffCache {
private static final String GIT_DIFF = "git_file_diff";
diff --git a/java/com/google/gerrit/server/query/approval/ApprovalQueryBuilder.java b/java/com/google/gerrit/server/query/approval/ApprovalQueryBuilder.java
index c3594f5..f412cc7 100644
--- a/java/com/google/gerrit/server/query/approval/ApprovalQueryBuilder.java
+++ b/java/com/google/gerrit/server/query/approval/ApprovalQueryBuilder.java
@@ -51,7 +51,7 @@
}
@Operator
- public Predicate<ApprovalContext> changeKind(String term) throws QueryParseException {
+ public Predicate<ApprovalContext> changekind(String term) throws QueryParseException {
return new ChangeKindPredicate(toEnumValue(ChangeKind.class, term));
}
diff --git a/java/com/google/gerrit/server/restapi/change/CommentPorter.java b/java/com/google/gerrit/server/restapi/change/CommentPorter.java
index d1d4544..35d512a 100644
--- a/java/com/google/gerrit/server/restapi/change/CommentPorter.java
+++ b/java/com/google/gerrit/server/restapi/change/CommentPorter.java
@@ -29,7 +29,6 @@
import com.google.gerrit.entities.Patch;
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.entities.Project;
-import com.google.gerrit.extensions.client.DiffPreferencesInfo.Whitespace;
import com.google.gerrit.metrics.Counter0;
import com.google.gerrit.metrics.Description;
import com.google.gerrit.metrics.MetricMaker;
@@ -39,16 +38,17 @@
import com.google.gerrit.server.logging.TraceContext.TraceTimer;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.patch.DiffMappings;
+import com.google.gerrit.server.patch.DiffNotAvailableException;
+import com.google.gerrit.server.patch.DiffOperations;
import com.google.gerrit.server.patch.GitPositionTransformer;
import com.google.gerrit.server.patch.GitPositionTransformer.BestPositionOnConflict;
import com.google.gerrit.server.patch.GitPositionTransformer.FileMapping;
import com.google.gerrit.server.patch.GitPositionTransformer.Mapping;
import com.google.gerrit.server.patch.GitPositionTransformer.Position;
import com.google.gerrit.server.patch.GitPositionTransformer.PositionedEntity;
-import com.google.gerrit.server.patch.PatchList;
-import com.google.gerrit.server.patch.PatchListCache;
-import com.google.gerrit.server.patch.PatchListKey;
-import com.google.gerrit.server.patch.PatchListNotAvailableException;
+import com.google.gerrit.server.patch.filediff.FileDiffOutput;
+import com.google.gerrit.server.patch.filediff.FileEdits;
+import com.google.gerrit.server.patch.filediff.TaggedEdit;
import com.google.inject.Inject;
import com.google.inject.Singleton;
import java.util.List;
@@ -102,15 +102,15 @@
}
}
+ private final DiffOperations diffOperations;
private final GitPositionTransformer positionTransformer =
new GitPositionTransformer(BestPositionOnConflict.INSTANCE);
- private final PatchListCache patchListCache;
private final CommentsUtil commentsUtil;
private final Metrics metrics;
@Inject
- public CommentPorter(PatchListCache patchListCache, CommentsUtil commentsUtil, Metrics metrics) {
- this.patchListCache = patchListCache;
+ public CommentPorter(DiffOperations diffOperations, CommentsUtil commentsUtil, Metrics metrics) {
+ this.diffOperations = diffOperations;
this.commentsUtil = commentsUtil;
this.metrics = metrics;
}
@@ -283,7 +283,7 @@
PatchSet originalPatchset,
PatchSet targetPatchset,
short side)
- throws PatchListNotAvailableException {
+ throws DiffNotAvailableException {
try (TraceTimer ignored =
TraceContext.newTimer(
"Loading commit mappings",
@@ -311,18 +311,26 @@
private ImmutableSet<Mapping> loadCommitMappings(
Project.NameKey project, ObjectId originalCommit, ObjectId targetCommit)
- throws PatchListNotAvailableException {
+ throws DiffNotAvailableException {
try (TraceTimer ignored =
TraceContext.newTimer(
"Computing diffs", Metadata.builder().commit(originalCommit.name()).build())) {
- PatchList patchList =
- patchListCache.get(
- PatchListKey.againstCommit(originalCommit, targetCommit, Whitespace.IGNORE_NONE),
- project);
- return patchList.getPatches().stream().map(DiffMappings::toMapping).collect(toImmutableSet());
+ Map<String, FileDiffOutput> modifiedFiles =
+ diffOperations.listModifiedFiles(project, originalCommit, targetCommit);
+ return modifiedFiles.values().stream()
+ .map(CommentPorter::getFileEdits)
+ .map(DiffMappings::toMapping)
+ .collect(toImmutableSet());
}
}
+ private static FileEdits getFileEdits(FileDiffOutput fileDiffOutput) {
+ return FileEdits.create(
+ fileDiffOutput.edits().stream().map(TaggedEdit::edit).collect(toImmutableList()),
+ fileDiffOutput.oldPath(),
+ fileDiffOutput.newPath());
+ }
+
private ImmutableSet<Mapping> getFallbackMappings(List<HumanComment> comments) {
// Consider all files as deleted. -> Comments will be ported to the fallback destination, which
// currently are patchset-level comments.
diff --git a/javatests/com/google/gerrit/server/restapi/change/CommentPorterTest.java b/javatests/com/google/gerrit/server/restapi/change/CommentPorterTest.java
index ebb2f38..46f9c5a 100644
--- a/javatests/com/google/gerrit/server/restapi/change/CommentPorterTest.java
+++ b/javatests/com/google/gerrit/server/restapi/change/CommentPorterTest.java
@@ -22,6 +22,7 @@
import static org.mockito.Mockito.when;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSortedMap;
import com.google.common.truth.Correspondence;
import com.google.gerrit.entities.Account;
@@ -35,12 +36,8 @@
import com.google.gerrit.metrics.DisabledMetricMaker;
import com.google.gerrit.server.CommentsUtil;
import com.google.gerrit.server.notedb.ChangeNotes;
-import com.google.gerrit.server.patch.ComparisonType;
-import com.google.gerrit.server.patch.PatchList;
-import com.google.gerrit.server.patch.PatchListCache;
-import com.google.gerrit.server.patch.PatchListEntry;
-import com.google.gerrit.server.patch.PatchListKey;
-import com.google.gerrit.server.patch.PatchListNotAvailableException;
+import com.google.gerrit.server.patch.DiffNotAvailableException;
+import com.google.gerrit.server.patch.DiffOperations;
import com.google.gerrit.server.restapi.change.CommentPorter.Metrics;
import com.google.gerrit.truth.NullAwareCorrespondence;
import java.sql.Timestamp;
@@ -60,7 +57,7 @@
@Rule public final MockitoRule mockito = MockitoJUnit.rule();
- @Mock private PatchListCache patchListCache;
+ @Mock private DiffOperations diffOperations;
@Mock private CommentsUtil commentsUtil;
private static final CommentPorter.Metrics metrics = new Metrics(new DisabledMetricMaker());
@@ -76,12 +73,13 @@
PatchSet patchset2 = createPatchset(PatchSet.id(changeId, 2));
ChangeNotes changeNotes = mockChangeNotes(project, change, patchset1, patchset2);
- CommentPorter commentPorter = new CommentPorter(patchListCache, commentsUtil, metrics);
+ CommentPorter commentPorter = new CommentPorter(diffOperations, commentsUtil, metrics);
HumanComment comment = createComment(patchset1.id(), "myFile");
when(commentsUtil.determineCommitId(any(), any(), anyShort()))
.thenReturn(Optional.of(dummyObjectId));
- when(patchListCache.get(any(PatchListKey.class), any(Project.NameKey.class)))
- .thenThrow(PatchListNotAvailableException.class);
+ when(diffOperations.listModifiedFiles(
+ any(Project.NameKey.class), any(ObjectId.class), any(ObjectId.class)))
+ .thenThrow(DiffNotAvailableException.class);
ImmutableList<HumanComment> portedComments =
commentPorter.portComments(
changeNotes, patchset2, ImmutableList.of(comment), ImmutableList.of());
@@ -98,11 +96,12 @@
PatchSet patchset2 = createPatchset(PatchSet.id(changeId, 2));
ChangeNotes changeNotes = mockChangeNotes(project, change, patchset1, patchset2);
- CommentPorter commentPorter = new CommentPorter(patchListCache, commentsUtil, metrics);
+ CommentPorter commentPorter = new CommentPorter(diffOperations, commentsUtil, metrics);
HumanComment comment = createComment(patchset1.id(), "myFile");
when(commentsUtil.determineCommitId(any(), any(), anyShort()))
.thenReturn(Optional.of(dummyObjectId));
- when(patchListCache.get(any(PatchListKey.class), any(Project.NameKey.class)))
+ when(diffOperations.listModifiedFiles(
+ any(Project.NameKey.class), any(ObjectId.class), any(ObjectId.class)))
.thenThrow(IllegalStateException.class);
ImmutableList<HumanComment> portedComments =
commentPorter.portComments(
@@ -120,7 +119,7 @@
PatchSet patchset2 = createPatchset(PatchSet.id(changeId, 2));
ChangeNotes changeNotes = mockChangeNotes(project, change, patchset1, patchset2);
- CommentPorter commentPorter = new CommentPorter(patchListCache, commentsUtil, metrics);
+ CommentPorter commentPorter = new CommentPorter(diffOperations, commentsUtil, metrics);
HumanComment comment = createComment(patchset1.id(), "myFile");
when(commentsUtil.determineCommitId(any(), any(), anyShort()))
.thenThrow(IllegalStateException.class);
@@ -140,11 +139,12 @@
PatchSet patchset2 = createPatchset(PatchSet.id(changeId, 2));
ChangeNotes changeNotes = mockChangeNotes(project, change, patchset1, patchset2);
- CommentPorter commentPorter = new CommentPorter(patchListCache, commentsUtil, metrics);
+ CommentPorter commentPorter = new CommentPorter(diffOperations, commentsUtil, metrics);
HumanComment comment = createComment(patchset1.id(), "myFile");
when(commentsUtil.determineCommitId(any(), any(), anyShort()))
.thenReturn(Optional.of(dummyObjectId));
- when(patchListCache.get(any(PatchListKey.class), any(Project.NameKey.class)))
+ when(diffOperations.listModifiedFiles(
+ any(Project.NameKey.class), any(ObjectId.class), any(ObjectId.class)))
.thenThrow(IllegalStateException.class);
ImmutableList<HumanComment> portedComments =
commentPorter.portComments(
@@ -165,17 +165,17 @@
PatchSet patchset3 = createPatchset(PatchSet.id(changeId, 3));
ChangeNotes changeNotes = mockChangeNotes(project, change, patchset1, patchset2, patchset3);
- CommentPorter commentPorter = new CommentPorter(patchListCache, commentsUtil, metrics);
+ CommentPorter commentPorter = new CommentPorter(diffOperations, commentsUtil, metrics);
// Place the comments on different patchsets to have two different diff requests.
HumanComment comment1 = createComment(patchset1.id(), "myFile");
HumanComment comment2 = createComment(patchset2.id(), "myFile");
when(commentsUtil.determineCommitId(any(), any(), anyShort()))
.thenReturn(Optional.of(dummyObjectId));
- PatchList emptyDiff = getEmptyDiff();
// Throw an exception on the first diff request but return an actual value on the second.
- when(patchListCache.get(any(PatchListKey.class), any(Project.NameKey.class)))
+ when(diffOperations.listModifiedFiles(
+ any(Project.NameKey.class), any(ObjectId.class), any(ObjectId.class)))
.thenThrow(IllegalStateException.class)
- .thenReturn(emptyDiff);
+ .thenReturn(ImmutableMap.of());
ImmutableList<HumanComment> portedComments =
commentPorter.portComments(
changeNotes, patchset3, ImmutableList.of(comment1, comment2), ImmutableList.of());
@@ -195,13 +195,13 @@
// Leave out patchset 1 (e.g. reserved for draft patchsets in the past).
ChangeNotes changeNotes = mockChangeNotes(project, change, patchset2);
- CommentPorter commentPorter = new CommentPorter(patchListCache, commentsUtil, metrics);
+ CommentPorter commentPorter = new CommentPorter(diffOperations, commentsUtil, metrics);
HumanComment comment = createComment(patchset1.id(), "myFile");
when(commentsUtil.determineCommitId(any(), any(), anyShort()))
.thenReturn(Optional.of(dummyObjectId));
- PatchList emptyDiff = getEmptyDiff();
- when(patchListCache.get(any(PatchListKey.class), any(Project.NameKey.class)))
- .thenReturn(emptyDiff);
+ when(diffOperations.listModifiedFiles(
+ any(Project.NameKey.class), any(ObjectId.class), any(ObjectId.class)))
+ .thenReturn(ImmutableMap.of());
ImmutableList<HumanComment> portedComments =
commentPorter.portComments(
changeNotes, patchset2, ImmutableList.of(comment), ImmutableList.of());
@@ -260,13 +260,4 @@
private Correspondence<HumanComment, String> hasFilePath() {
return NullAwareCorrespondence.transforming(comment -> comment.key.filename, "hasFilePath");
}
-
- private PatchList getEmptyDiff() {
- return new PatchList(
- dummyObjectId,
- dummyObjectId,
- false,
- ComparisonType.againstOtherPatchSet(),
- new PatchListEntry[0]);
- }
}
diff --git a/polygerrit-ui/app/api/diff.ts b/polygerrit-ui/app/api/diff.ts
index ad83fb8..724141f 100644
--- a/polygerrit-ui/app/api/diff.ts
+++ b/polygerrit-ui/app/api/diff.ts
@@ -406,17 +406,26 @@
): void;
}
-// The current setup requires API users to register GrDiff instances with the
-// cursor, but we do not at this point want to expose the API that GrDiffCursor
-// uses to the public as it is likely to change. So for now, we allow any type
-// and cast. This works fine so long as API users do provide whatever the
-// gr-diff tag creates.
-export type GrDiff = unknown;
+/** An instance of the GrDiff Webcomponent */
+export interface GrDiff extends HTMLElement {
+ /**
+ * Return line number element for reading only,
+ *
+ * This is useful e.g. to determine where on screen certain lines are,
+ * whether they are covered up etc.
+ */
+ getLineNumEls(side: Side): readonly HTMLElement[];
+}
/** A service to interact with the line cursor in gr-diff instances. */
export declare interface GrDiffCursor {
- replaceDiffs(diffs: GrDiff[]): void;
- unregisterDiff(diff: GrDiff): void;
+ // The current setup requires API users to register GrDiff instances with the
+ // cursor, but we do not at this point want to expose the API that GrDiffCursor
+ // uses to the public as it is likely to change. So for now, we allow any type
+ // and cast. This works fine so long as API users do provide whatever the
+ // gr-diff tag creates.
+ replaceDiffs(diffs: unknown[]): void;
+ unregisterDiff(diff: unknown): void;
isAtStart(): boolean;
isAtEnd(): boolean;
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_html.ts b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_html.ts
index 06087ed..8ee6a3f 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_html.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_html.ts
@@ -158,7 +158,7 @@
hidden$="[[isColumnHidden('Owner', visibleChangeTableColumns)]]"
>
<gr-account-link
- highlight-attention
+ highlightAttention
change="[[change]]"
account="[[change.owner]]"
></gr-account-link>
@@ -189,10 +189,10 @@
indexAs="index"
>
<gr-account-link
- hide-avatar=""
- hide-status=""
- first-name
- highlight-attention
+ hideAvatar=""
+ hideStatus=""
+ firstName
+ highlightAttention
change="[[change]]"
account="[[reviewer]]"
></gr-account-link
diff --git a/polygerrit-ui/app/elements/change-list/gr-create-commands-dialog/gr-create-commands-dialog_test.js b/polygerrit-ui/app/elements/change-list/gr-create-commands-dialog/gr-create-commands-dialog_test.ts
similarity index 77%
rename from polygerrit-ui/app/elements/change-list/gr-create-commands-dialog/gr-create-commands-dialog_test.js
rename to polygerrit-ui/app/elements/change-list/gr-create-commands-dialog/gr-create-commands-dialog_test.ts
index 9dbcd29..42aed07 100644
--- a/polygerrit-ui/app/elements/change-list/gr-create-commands-dialog/gr-create-commands-dialog_test.js
+++ b/polygerrit-ui/app/elements/change-list/gr-create-commands-dialog/gr-create-commands-dialog_test.ts
@@ -17,11 +17,12 @@
import '../../../test/common-test-setup-karma.js';
import './gr-create-commands-dialog.js';
+import {GrCreateCommandsDialog} from './gr-create-commands-dialog.js';
const basicFixture = fixtureFromElement('gr-create-commands-dialog');
suite('gr-create-commands-dialog tests', () => {
- let element;
+ let element: GrCreateCommandsDialog;
setup(() => {
element = basicFixture.instantiate();
@@ -29,12 +30,12 @@
test('_computePushCommand', () => {
element.branch = 'master';
- assert.equal(element._pushCommand,
- 'git push origin HEAD:refs/for/master');
+ assert.equal(element._pushCommand, 'git push origin HEAD:refs/for/master');
element.branch = 'stable-2.15';
- assert.equal(element._pushCommand,
- 'git push origin HEAD:refs/for/stable-2.15');
+ assert.equal(
+ element._pushCommand,
+ 'git push origin HEAD:refs/for/stable-2.15'
+ );
});
});
-
diff --git a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_test.ts b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_test.ts
index ab8e404..516a31e 100644
--- a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_test.ts
@@ -939,7 +939,7 @@
});
suite('plugin endpoints', () => {
- test('endpoint params', done => {
+ test('endpoint params', async () => {
element.change = createParsedChange();
element.revision = createRevision();
interface MetadataGrEndpointDecorator extends GrEndpointDecorator {
@@ -961,12 +961,10 @@
'http://some/plugins/url.js'
);
getPluginLoader().loadPlugins([]);
- flush(() => {
- assert.strictEqual(hookEl!.plugin, plugin);
- assert.strictEqual(hookEl!.change, element.change);
- assert.strictEqual(hookEl!.revision, element.revision);
- done();
- });
+ await flush();
+ assert.strictEqual(hookEl!.plugin, plugin!);
+ assert.strictEqual(hookEl!.change, element.change);
+ assert.strictEqual(hookEl!.revision, element.revision);
});
});
});
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 58cf489..8b37cf0 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
@@ -171,7 +171,7 @@
import {takeUntil} from 'rxjs/operators';
import {aPluginHasRegistered$} from '../../../services/checks/checks-model';
import {Subject} from 'rxjs';
-import {debounce, DelayedTask} from '../../../utils/async-util';
+import {debounce, DelayedTask, throttleWrap} from '../../../utils/async-util';
import {Interaction, Timing} from '../../../constants/reporting';
import {ChangeStates} from '../../shared/gr-change-status/gr-change-status';
import {getRevertCreatedChangeIds} from '../../../utils/message-util';
@@ -603,7 +603,7 @@
/** @override */
connectedCallback() {
super.connectedCallback();
- this._throttledToggleChangeStar = this._throttleWrap(e =>
+ this._throttledToggleChangeStar = throttleWrap(e =>
this._handleToggleChangeStar(e as CustomKeyboardEvent)
);
this._getServerConfig().then(config => {
diff --git a/polygerrit-ui/app/elements/change/gr-download-dialog/gr-download-dialog_test.ts b/polygerrit-ui/app/elements/change/gr-download-dialog/gr-download-dialog_test.ts
index 52ec8af..ea3c01d 100644
--- a/polygerrit-ui/app/elements/change/gr-download-dialog/gr-download-dialog_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-download-dialog/gr-download-dialog_test.ts
@@ -31,6 +31,7 @@
RepoName,
} from '../../../types/common';
import {GrDownloadDialog} from './gr-download-dialog';
+import {mockPromise} from '../../../test/test-utils';
const basicFixture = fixtureFromElement('gr-download-dialog');
@@ -168,14 +169,16 @@
);
});
- test('close event', done => {
+ test('close event', async () => {
+ const closeCalled = mockPromise();
element.addEventListener('close', () => {
- done();
+ closeCalled.resolve();
});
const closeButton = element.shadowRoot!.querySelector(
'.closeButtonContainer gr-button'
);
tap(closeButton!);
+ await closeCalled;
});
});
diff --git a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header_html.ts b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header_html.ts
index 8b9b80c..be3a0be 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header_html.ts
+++ b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header_html.ts
@@ -167,6 +167,30 @@
<span class="separator"></span>
</span>
</template>
+ <div class="fileViewActions">
+ <span class="fileViewActionsLabel">Diff view:</span>
+ <gr-diff-mode-selector
+ id="modeSelect"
+ mode="{{diffViewMode}}"
+ save-on-change="[[!diffPrefsDisabled]]"
+ ></gr-diff-mode-selector>
+ <span
+ id="diffPrefsContainer"
+ class="hideOnEdit"
+ hidden$="[[_computePrefsButtonHidden(diffPrefs, diffPrefsDisabled)]]"
+ hidden=""
+ >
+ <gr-button
+ link=""
+ has-tooltip=""
+ title="Diff preferences"
+ class="prefsButton desktop"
+ on-click="_handlePrefsTap"
+ ><iron-icon icon="gr-icons:settings"></iron-icon
+ ></gr-button>
+ </span>
+ </div>
+ <span class="separator"></span>
<span class="downloadContainer desktop">
<gr-button
link=""
@@ -209,30 +233,6 @@
Bulk actions disabled because there are too many files.
</div>
</template>
- <div class="fileViewActions">
- <span class="separator"></span>
- <span class="fileViewActionsLabel">Diff view:</span>
- <gr-diff-mode-selector
- id="modeSelect"
- mode="{{diffViewMode}}"
- save-on-change="[[!diffPrefsDisabled]]"
- ></gr-diff-mode-selector>
- <span
- id="diffPrefsContainer"
- class="hideOnEdit"
- hidden$="[[_computePrefsButtonHidden(diffPrefs, diffPrefsDisabled)]]"
- hidden=""
- >
- <gr-button
- link=""
- has-tooltip=""
- title="Diff preferences"
- class="prefsButton desktop"
- on-click="_handlePrefsTap"
- ><iron-icon icon="gr-icons:settings"></iron-icon
- ></gr-button>
- </span>
- </div>
</div>
</div>
`;
diff --git a/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list_test.ts b/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list_test.ts
index ae9af4a..15bc6bf 100644
--- a/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list_test.ts
@@ -599,7 +599,7 @@
resetPlugins();
});
- test('endpoint params', done => {
+ test('endpoint params', async () => {
element.change = {...createParsedChange(), labels: {}};
interface RelatedChangesListGrEndpointDecorator
extends GrEndpointDecorator {
@@ -620,11 +620,9 @@
'http://some/plugins/url1.js'
);
getPluginLoader().loadPlugins([]);
- flush(() => {
- assert.strictEqual(hookEl.plugin, plugin);
- assert.strictEqual(hookEl.change, element.change);
- done();
- });
+ await flush();
+ assert.strictEqual(hookEl!.plugin, plugin!);
+ assert.strictEqual(hookEl!.change, element.change);
});
});
});
diff --git a/polygerrit-ui/app/elements/change/gr-reviewer-list/gr-reviewer-list_test.ts b/polygerrit-ui/app/elements/change/gr-reviewer-list/gr-reviewer-list_test.ts
index 4b57651..354d360 100644
--- a/polygerrit-ui/app/elements/change/gr-reviewer-list/gr-reviewer-list_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-reviewer-list/gr-reviewer-list_test.ts
@@ -17,7 +17,11 @@
import '../../../test/common-test-setup-karma';
import './gr-reviewer-list';
-import {queryAndAssert, stubRestApi} from '../../../test/test-utils';
+import {
+ mockPromise,
+ queryAndAssert,
+ stubRestApi,
+} from '../../../test/test-utils';
import {GrReviewerList} from './gr-reviewer-list';
import {
createAccountDetailWithId,
@@ -53,12 +57,14 @@
);
});
- test('add reviewer button opens reply dialog', done => {
+ test('add reviewer button opens reply dialog', async () => {
+ const dialogShown = mockPromise();
element.addEventListener('show-reply-dialog', () => {
- done();
+ dialogShown.resolve();
});
- flush();
+ await flush();
tap(queryAndAssert(element, '.addReviewer'));
+ await dialogShown;
});
test('only show remove for removable reviewers', () => {
diff --git a/polygerrit-ui/app/elements/checks/gr-hovercard-run_test.js b/polygerrit-ui/app/elements/checks/gr-hovercard-run_test.ts
similarity index 78%
rename from polygerrit-ui/app/elements/checks/gr-hovercard-run_test.js
rename to polygerrit-ui/app/elements/checks/gr-hovercard-run_test.ts
index 9c77654..80d8e5e 100644
--- a/polygerrit-ui/app/elements/checks/gr-hovercard-run_test.js
+++ b/polygerrit-ui/app/elements/checks/gr-hovercard-run_test.ts
@@ -18,24 +18,25 @@
import '../../test/common-test-setup-karma.js';
import './gr-hovercard-run.js';
import {html} from '@polymer/polymer/lib/utils/html-tag.js';
+import {GrHovercardRun} from './gr-hovercard-run.js';
const basicFixture = fixtureFromTemplate(html`
-<gr-hovercard-run class="hovered"></gr-hovercard-run>
+ <gr-hovercard-run class="hovered"></gr-hovercard-run>
`);
suite('gr-hovercard-run tests', () => {
- let element;
+ let element: GrHovercardRun;
setup(async () => {
- element = basicFixture.instantiate();
+ element = basicFixture.instantiate() as GrHovercardRun;
await flush();
});
teardown(() => {
- element.hide({});
+ element.hide();
});
test('hovercard is shown', () => {
+ assert.equal(element.computeIcon(), '');
});
});
-
diff --git a/polygerrit-ui/app/elements/core/gr-error-dialog/gr-error-dialog_test.ts b/polygerrit-ui/app/elements/core/gr-error-dialog/gr-error-dialog_test.ts
index fa57a23..2dec8d2 100644
--- a/polygerrit-ui/app/elements/core/gr-error-dialog/gr-error-dialog_test.ts
+++ b/polygerrit-ui/app/elements/core/gr-error-dialog/gr-error-dialog_test.ts
@@ -17,7 +17,7 @@
import * as MockInteractions from '@polymer/iron-test-helpers/mock-interactions';
import '../../../test/common-test-setup-karma';
-import {queryAndAssert} from '../../../test/test-utils';
+import {mockPromise, queryAndAssert} from '../../../test/test-utils';
import {GrDialog} from '../../shared/gr-dialog/gr-dialog';
import {GrErrorDialog} from './gr-error-dialog';
@@ -30,10 +30,12 @@
element = basicFixture.instantiate();
});
- test('dismiss tap fires event', done => {
- element.addEventListener('dismiss', () => done());
+ test('dismiss tap fires event', async () => {
+ const dismissCalled = mockPromise();
+ element.addEventListener('dismiss', () => dismissCalled.resolve());
MockInteractions.tap(
(queryAndAssert(element, '#dialog') as GrDialog).$.confirm
);
+ await dismissCalled;
});
});
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 8f9dfe2..8352319 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
@@ -75,6 +75,37 @@
};
}
+export function constructServerErrorMsg({
+ errorText,
+ status,
+ statusText,
+ url,
+ trace,
+ tip,
+}: ErrorMsg) {
+ let err = '';
+ if (tip) {
+ err += `${tip}\n\n`;
+ }
+ err += `Error ${status}`;
+ if (statusText) {
+ err += ` (${statusText})`;
+ }
+ if (errorText || url) {
+ err += ': ';
+ }
+ if (errorText) {
+ err += errorText;
+ }
+ if (url) {
+ err += `\nEndpoint: ${url}`;
+ }
+ if (trace) {
+ err += `\nTrace Id: ${trace}`;
+ }
+ return err;
+}
+
@customElement('gr-error-manager')
export class GrErrorManager extends PolymerElement {
static get template() {
@@ -221,7 +252,7 @@
this._showQuotaExceeded({status, statusText});
} else {
this._showErrorDialog(
- this._constructServerErrorMsg({
+ constructServerErrorMsg({
status,
statusText,
errorText,
@@ -247,7 +278,7 @@
? 'You might have not enough privileges.'
: 'You might have not enough privileges. Sign in and try again.';
this._showErrorDialog(
- this._constructServerErrorMsg({
+ constructServerErrorMsg({
status,
statusText,
errorText,
@@ -266,7 +297,7 @@
const tip = 'Try again later';
const errorText = 'Too many requests from this client';
this._showErrorDialog(
- this._constructServerErrorMsg({
+ constructServerErrorMsg({
status,
statusText,
errorText,
@@ -275,37 +306,6 @@
);
}
- _constructServerErrorMsg({
- errorText,
- status,
- statusText,
- url,
- trace,
- tip,
- }: ErrorMsg) {
- let err = '';
- if (tip) {
- err += `${tip}\n\n`;
- }
- err += `Error ${status}`;
- if (statusText) {
- err += ` (${statusText})`;
- }
- if (errorText || url) {
- err += ': ';
- }
- if (errorText) {
- err += errorText;
- }
- if (url) {
- err += `\nEndpoint: ${url}`;
- }
- if (trace) {
- err += `\nTrace Id: ${trace}`;
- }
- return err;
- }
-
private readonly handleShowAlert = (e: CustomEvent<ShowAlertEventDetail>) => {
this._showAlert(
e.detail.message,
diff --git a/polygerrit-ui/app/elements/core/gr-error-manager/gr-error-manager_test.js b/polygerrit-ui/app/elements/core/gr-error-manager/gr-error-manager_test.js
index fe4d9da..4cf15b4 100644
--- a/polygerrit-ui/app/elements/core/gr-error-manager/gr-error-manager_test.js
+++ b/polygerrit-ui/app/elements/core/gr-error-manager/gr-error-manager_test.js
@@ -18,7 +18,7 @@
import '../../../test/common-test-setup-karma.js';
import './gr-error-manager.js';
import {_testOnly_initGerritPluginApi} from '../../shared/gr-js-api-interface/gr-gerrit.js';
-import {__testOnly_ErrorType} from './gr-error-manager.js';
+import {constructServerErrorMsg, __testOnly_ErrorType} from './gr-error-manager.js';
import {stubRestApi} from '../../../test/test-utils.js';
import {appContext} from '../../../services/app-context.js';
import {createPreferences} from '../../../test/test-data-generators.js';
@@ -132,27 +132,26 @@
});
});
- test('_constructServerErrorMsg', () => {
+ test('constructServerErrorMsg', () => {
const errorText = 'change conflicts';
const status = 409;
const statusText = 'Conflict';
const url = '/my/test/url';
- assert.equal(element._constructServerErrorMsg({status}),
+ assert.equal(constructServerErrorMsg({status}),
'Error 409');
- assert.equal(element._constructServerErrorMsg({status, url}),
+ assert.equal(constructServerErrorMsg({status, url}),
'Error 409: \nEndpoint: /my/test/url');
- assert.equal(element.
- _constructServerErrorMsg({status, statusText, url}),
- 'Error 409 (Conflict): \nEndpoint: /my/test/url');
- assert.equal(element._constructServerErrorMsg({
+ assert.equal(constructServerErrorMsg({status, statusText, url}),
+ 'Error 409 (Conflict): \nEndpoint: /my/test/url');
+ assert.equal(constructServerErrorMsg({
status,
statusText,
errorText,
url,
}), 'Error 409 (Conflict): change conflicts' +
'\nEndpoint: /my/test/url');
- assert.equal(element._constructServerErrorMsg({
+ assert.equal(constructServerErrorMsg({
status,
statusText,
errorText,
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 dacd8e7..9004517 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
@@ -106,6 +106,7 @@
import {assertIsDefined} from '../../../utils/common-util';
import {toggleClass, getKeyboardEvent} from '../../../utils/dom-util';
import {CursorMoveResult} from '../../../api/core';
+import {throttleWrap} from '../../../utils/async-util';
const ERR_REVIEW_STATUS = 'Couldn’t change file review status.';
const MSG_LOADING_BLAME = 'Loading blame...';
@@ -342,7 +343,7 @@
/** @override */
connectedCallback() {
super.connectedCallback();
- this._throttledToggleFileReviewed = this._throttleWrap(e =>
+ this._throttledToggleFileReviewed = throttleWrap(e =>
this._handleToggleFileReviewed(e as CustomKeyboardEvent)
);
this._getLoggedIn().then(loggedIn => {
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 7c437fd..0b42f9f 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.ts
@@ -69,6 +69,7 @@
import {
CreateCommentEventDetail as CreateCommentEventDetailApi,
RenderPreferences,
+ GrDiff as GrDiffApi,
} from '../../../api/diff';
import {isSafari, toggleClass} from '../../../utils/dom-util';
import {assertIsDefined} from '../../../utils/common-util';
@@ -109,7 +110,7 @@
}
@customElement('gr-diff')
-export class GrDiff extends PolymerElement {
+export class GrDiff extends PolymerElement implements GrDiffApi {
static get template() {
return htmlTemplate;
}
@@ -322,6 +323,12 @@
super.disconnectedCallback();
}
+ getLineNumEls(side: Side): HTMLElement[] {
+ return Array.from(
+ this.root?.querySelectorAll<HTMLElement>(`.lineNum.${side}`) ?? []
+ );
+ }
+
showNoChangeMessage(
loading?: boolean,
prefs?: DiffPreferencesInfo,
diff --git a/polygerrit-ui/app/elements/gr-app-element.ts b/polygerrit-ui/app/elements/gr-app-element.ts
index be9c7c5..c9f0506 100644
--- a/polygerrit-ui/app/elements/gr-app-element.ts
+++ b/polygerrit-ui/app/elements/gr-app-element.ts
@@ -55,7 +55,10 @@
ElementPropertyDeepChange,
ServerInfo,
} from '../types/common';
-import {GrErrorManager} from './core/gr-error-manager/gr-error-manager';
+import {
+ constructServerErrorMsg,
+ GrErrorManager,
+} from './core/gr-error-manager/gr-error-manager';
import {GrOverlay} from './shared/gr-overlay/gr-overlay';
import {GrRegistrationDialog} from './settings/gr-registration-dialog/gr-registration-dialog';
import {
@@ -565,7 +568,15 @@
err.emoji = 'o_O';
if (response) {
response.text().then(text => {
- err.moreInfo = text;
+ const trace =
+ response.headers && response.headers.get('X-Gerrit-Trace');
+ const {status, statusText} = response;
+ err.moreInfo = constructServerErrorMsg({
+ status,
+ statusText,
+ errorText: text,
+ trace,
+ });
this._lastError = err;
});
}
diff --git a/polygerrit-ui/app/elements/settings/gr-agreements-list/gr-agreements-list_test.ts b/polygerrit-ui/app/elements/settings/gr-agreements-list/gr-agreements-list_test.ts
index f08976f..4891759 100644
--- a/polygerrit-ui/app/elements/settings/gr-agreements-list/gr-agreements-list_test.ts
+++ b/polygerrit-ui/app/elements/settings/gr-agreements-list/gr-agreements-list_test.ts
@@ -25,7 +25,7 @@
suite('gr-agreements-list tests', () => {
let element: GrAgreementsList;
- setup(done => {
+ setup(async () => {
const agreements: ContributorAgreementInfo[] = [
{
url: 'some url',
@@ -38,9 +38,8 @@
element = basicFixture.instantiate();
- element.loadData().then(() => {
- flush(done);
- });
+ await element.loadData();
+ await flush();
});
test('renders', () => {
diff --git a/polygerrit-ui/app/elements/settings/gr-cla-view/gr-cla-view_test.ts b/polygerrit-ui/app/elements/settings/gr-cla-view/gr-cla-view_test.ts
index 9f54cd1..d634483 100644
--- a/polygerrit-ui/app/elements/settings/gr-cla-view/gr-cla-view_test.ts
+++ b/polygerrit-ui/app/elements/settings/gr-cla-view/gr-cla-view_test.ts
@@ -125,16 +125,15 @@
},
];
- setup(done => {
+ setup(async () => {
stubRestApi('getConfig').returns(Promise.resolve(config));
stubRestApi('getAccountGroups').returns(Promise.resolve(groups));
stubRestApi('getAccountAgreements').returns(
Promise.resolve(signedAgreements)
);
element = basicFixture.instantiate();
- element.loadData().then(() => {
- flush(done);
- });
+ await element.loadData();
+ await flush();
});
test('renders as expected with signed agreement', () => {
diff --git a/polygerrit-ui/app/elements/shared/gr-account-link/gr-account-link.ts b/polygerrit-ui/app/elements/shared/gr-account-link/gr-account-link.ts
index 5c4b76a..944c3df 100644
--- a/polygerrit-ui/app/elements/shared/gr-account-link/gr-account-link.ts
+++ b/polygerrit-ui/app/elements/shared/gr-account-link/gr-account-link.ts
@@ -16,19 +16,12 @@
*/
import '../gr-account-label/gr-account-label';
-import '../../../styles/shared-styles';
-import {PolymerElement} from '@polymer/polymer/polymer-element';
-import {htmlTemplate} from './gr-account-link_html';
import {GerritNav} from '../../core/gr-navigation/gr-navigation';
-import {customElement, property} from '@polymer/decorators';
import {AccountInfo, ChangeInfo} from '../../../types/common';
+import {css, customElement, html, LitElement, property} from 'lit-element';
@customElement('gr-account-link')
-class GrAccountLink extends PolymerElement {
- static get template() {
- return htmlTemplate;
- }
-
+class GrAccountLink extends LitElement {
@property({type: String})
voteableText?: string;
@@ -70,6 +63,45 @@
@property({type: Boolean})
firstName = false;
+ static get styles() {
+ return [
+ css`
+ :host {
+ display: inline-block;
+ vertical-align: top;
+ }
+ a {
+ color: var(--primary-text-color);
+ text-decoration: none;
+ }
+ gr-account-label {
+ --gr-account-label-text-hover-style: {
+ text-decoration: underline;
+ }
+ }
+ `,
+ ];
+ }
+
+ render() {
+ if (!this.account) return;
+ return html`<span>
+ <a href="${this._computeOwnerLink(this.account)}">
+ <gr-account-label
+ .account="${this.account}"
+ .change="${this.change}"
+ ?force-attention=${this.forceAttention}
+ ?highlight-attention=${this.highlightAttention}
+ ?hide-avatar=${this.hideAvatar}
+ ?hide-status=${this.hideStatus}
+ ?first-name=${this.firstName}
+ .voteable-text=${this.voteableText}
+ >
+ </gr-account-label>
+ </a>
+ </span>`;
+ }
+
_computeOwnerLink(account?: AccountInfo) {
if (!account) {
return;
diff --git a/polygerrit-ui/app/elements/shared/gr-account-link/gr-account-link_html.ts b/polygerrit-ui/app/elements/shared/gr-account-link/gr-account-link_html.ts
deleted file mode 100644
index 98a6e77..0000000
--- a/polygerrit-ui/app/elements/shared/gr-account-link/gr-account-link_html.ts
+++ /dev/null
@@ -1,50 +0,0 @@
-/**
- * @license
- * Copyright (C) 2020 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-import {html} from '@polymer/polymer/lib/utils/html-tag';
-
-export const htmlTemplate = html`
- <style>
- :host {
- display: inline-block;
- vertical-align: top;
- }
- a {
- color: var(--primary-text-color);
- text-decoration: none;
- }
- gr-account-label {
- --gr-account-label-text-hover-style: {
- text-decoration: underline;
- }
- }
- </style>
- <span>
- <a href$="[[_computeOwnerLink(account)]]">
- <gr-account-label
- account="[[account]]"
- change="[[change]]"
- force-attention="[[forceAttention]]"
- highlight-attention="[[highlightAttention]]"
- hide-avatar="[[hideAvatar]]"
- hide-status="[[hideStatus]]"
- first-name="[[firstName]]"
- voteable-text="[[voteableText]]"
- >
- </gr-account-label>
- </a>
- </span>
-`;
diff --git a/polygerrit-ui/app/elements/shared/gr-alert/gr-alert_test.js b/polygerrit-ui/app/elements/shared/gr-alert/gr-alert_test.ts
similarity index 82%
rename from polygerrit-ui/app/elements/shared/gr-alert/gr-alert_test.js
rename to polygerrit-ui/app/elements/shared/gr-alert/gr-alert_test.ts
index 11ec496..cf0c34a 100644
--- a/polygerrit-ui/app/elements/shared/gr-alert/gr-alert_test.js
+++ b/polygerrit-ui/app/elements/shared/gr-alert/gr-alert_test.ts
@@ -17,8 +17,11 @@
import '../../../test/common-test-setup-karma.js';
import './gr-alert.js';
+import {GrAlert} from './gr-alert.js';
+import * as MockInteractions from '@polymer/iron-test-helpers/mock-interactions';
+
suite('gr-alert tests', () => {
- let element;
+ let element: GrAlert;
setup(() => {
element = document.createElement('gr-alert');
@@ -32,7 +35,7 @@
test('show/hide', () => {
assert.isNull(element.parentNode);
- element.show();
+ element.show('Alert text');
assert.equal(element.parentNode, document.body);
element.updateStyles({'--gr-alert-transition-duration': '0ms'});
element.hide();
@@ -41,11 +44,10 @@
test('action event', () => {
const spy = sinon.spy();
- element.show();
+ element.show('Alert text');
element._actionCallback = spy;
assert.isFalse(spy.called);
- MockInteractions.tap(element.shadowRoot.querySelector('.action'));
+ MockInteractions.tap(element.shadowRoot!.querySelector('.action')!);
assert.isTrue(spy.called);
});
});
-
diff --git a/polygerrit-ui/app/elements/shared/gr-linked-chip/gr-linked-chip_test.js b/polygerrit-ui/app/elements/shared/gr-linked-chip/gr-linked-chip_test.ts
similarity index 80%
rename from polygerrit-ui/app/elements/shared/gr-linked-chip/gr-linked-chip_test.js
rename to polygerrit-ui/app/elements/shared/gr-linked-chip/gr-linked-chip_test.ts
index dd2b98a..972e02c 100644
--- a/polygerrit-ui/app/elements/shared/gr-linked-chip/gr-linked-chip_test.js
+++ b/polygerrit-ui/app/elements/shared/gr-linked-chip/gr-linked-chip_test.ts
@@ -15,13 +15,15 @@
* limitations under the License.
*/
-import '../../../test/common-test-setup-karma.js';
-import './gr-linked-chip.js';
+import '../../../test/common-test-setup-karma';
+import './gr-linked-chip';
+import {GrLinkedChip} from './gr-linked-chip';
+import * as MockInteractions from '@polymer/iron-test-helpers/mock-interactions';
const basicFixture = fixtureFromElement('gr-linked-chip');
suite('gr-linked-chip tests', () => {
- let element;
+ let element: GrLinkedChip;
setup(() => {
element = basicFixture.instantiate();
@@ -35,4 +37,3 @@
assert.isTrue(spy.called);
});
});
-
diff --git a/polygerrit-ui/app/elements/shared/gr-shell-command/gr-shell-command_test.js b/polygerrit-ui/app/elements/shared/gr-shell-command/gr-shell-command_test.ts
similarity index 78%
rename from polygerrit-ui/app/elements/shared/gr-shell-command/gr-shell-command_test.js
rename to polygerrit-ui/app/elements/shared/gr-shell-command/gr-shell-command_test.ts
index de9f243..1a7d2cd 100644
--- a/polygerrit-ui/app/elements/shared/gr-shell-command/gr-shell-command_test.js
+++ b/polygerrit-ui/app/elements/shared/gr-shell-command/gr-shell-command_test.ts
@@ -17,25 +17,26 @@
import '../../../test/common-test-setup-karma.js';
import './gr-shell-command.js';
+import {GrShellCommand} from './gr-shell-command.js';
const basicFixture = fixtureFromElement('gr-shell-command');
suite('gr-shell-command tests', () => {
- let element;
+ let element: GrShellCommand;
setup(() => {
element = basicFixture.instantiate();
- element.text = `git fetch http://gerrit@localhost:8080/a/test-project
+ element.command = `git fetch http://gerrit@localhost:8080/a/test-project
refs/changes/05/5/1 && git checkout FETCH_HEAD`;
flush();
});
test('focusOnCopy', () => {
- const focusStub = sinon.stub(element.shadowRoot
- .querySelector('gr-copy-clipboard'),
- 'focusOnCopy');
+ const focusStub = sinon.stub(
+ element.shadowRoot!.querySelector('gr-copy-clipboard')!,
+ 'focusOnCopy'
+ );
element.focusOnCopy();
assert.isTrue(focusStub.called);
});
});
-
diff --git a/polygerrit-ui/app/mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin.ts b/polygerrit-ui/app/mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin.ts
index 555a256..3d5a208 100644
--- a/polygerrit-ui/app/mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin.ts
+++ b/polygerrit-ui/app/mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin.ts
@@ -120,8 +120,6 @@
const V_KEY_TIMEOUT_MS = 1000;
-const THROTTLE_INTERVAL_MS = 500;
-
/**
* Enum for all shortcut sections, where that shortcut should be applied to.
*/
@@ -675,8 +673,6 @@
if (!bindings) {
return null;
}
- // TODO(TS): should check base on length to differentiate two
- // cases
if (bindings[0] === SPECIAL_SHORTCUT.GO_KEY) {
return bindings
.slice(1)
@@ -851,13 +847,6 @@
return getKeyboardEvent(e);
}
- // TODO(TS): maybe remove, no reference in the code base
- getRootTarget(e: CustomKeyboardEvent) {
- // TODO(TS): worth checking if we can limit this to EventApi only
- // dom currently returns DomNativeApi|EventApi
- return (dom(getKeyboardEvent(e)) as EventApi).rootTarget;
- }
-
bindShortcut(shortcut: Shortcut, ...bindings: string[]) {
shortcutManager.bindShortcut(shortcut, ...bindings);
}
@@ -868,20 +857,6 @@
return desc && shortcut ? `${desc} (shortcut: ${shortcut})` : '';
}
- _throttleWrap(fn: (e: Event) => void) {
- let lastCall: number | undefined;
- return (e: Event) => {
- if (
- lastCall !== undefined &&
- Date.now() - lastCall < THROTTLE_INTERVAL_MS
- ) {
- return;
- }
- lastCall = Date.now();
- fn(e);
- };
- }
-
_addOwnKeyBindings(shortcut: Shortcut, handler: string) {
const bindings = shortcutManager.getBindingsForShortcut(shortcut);
if (!bindings) {
@@ -1128,8 +1103,6 @@
modifierPressed(event: CustomKeyboardEvent): boolean;
addKeyboardShortcutDirectoryListener(listener: ShortcutListener): void;
removeKeyboardShortcutDirectoryListener(listener: ShortcutListener): void;
- // TODO(TS): Remove underscore. Apparently not a private method.
- _throttleWrap(eventListener: EventListener): EventListener;
}
export function _testOnly_getShortcutManagerInstance() {
diff --git a/polygerrit-ui/app/services/checks/checks-model.ts b/polygerrit-ui/app/services/checks/checks-model.ts
index 5d3da42..22c2381 100644
--- a/polygerrit-ui/app/services/checks/checks-model.ts
+++ b/polygerrit-ui/app/services/checks/checks-model.ts
@@ -261,10 +261,13 @@
// Must only be used by the checks service or whatever is in control of this
// model.
-export function updateStateSetProvider(pluginName: string) {
+export function updateStateSetProvider(
+ pluginName: string,
+ patchset: ChecksPatchset
+) {
const nextState = {...privateState$.getValue()};
- nextState.pluginStateLatest = {...nextState.pluginStateLatest};
- nextState.pluginStateLatest[pluginName] = {
+ const pluginState = getPluginState(nextState, patchset);
+ pluginState[pluginName] = {
pluginName,
loading: false,
runs: [],
diff --git a/polygerrit-ui/app/services/checks/checks-service.ts b/polygerrit-ui/app/services/checks/checks-service.ts
index e1d435b..d683514 100644
--- a/polygerrit-ui/app/services/checks/checks-service.ts
+++ b/polygerrit-ui/app/services/checks/checks-service.ts
@@ -118,7 +118,8 @@
) {
this.providers[pluginName] = provider;
this.reloadSubjects[pluginName] = new BehaviorSubject<void>(undefined);
- updateStateSetProvider(pluginName);
+ updateStateSetProvider(pluginName, ChecksPatchset.LATEST);
+ updateStateSetProvider(pluginName, ChecksPatchset.SELECTED);
this.initFetchingOfData(pluginName, config, ChecksPatchset.LATEST);
this.initFetchingOfData(pluginName, config, ChecksPatchset.SELECTED);
}
diff --git a/polygerrit-ui/app/services/flags/flags_test.js b/polygerrit-ui/app/services/flags/flags_test.ts
similarity index 85%
rename from polygerrit-ui/app/services/flags/flags_test.js
rename to polygerrit-ui/app/services/flags/flags_test.ts
index 33508af..8827368 100644
--- a/polygerrit-ui/app/services/flags/flags_test.js
+++ b/polygerrit-ui/app/services/flags/flags_test.ts
@@ -15,12 +15,12 @@
* limitations under the License.
*/
-import '../../test/common-test-setup-karma.js';
-import {FlagsServiceImplementation} from './flags_impl.js';
+import '../../test/common-test-setup-karma';
+import {FlagsServiceImplementation} from './flags_impl';
suite('flags tests', () => {
- let originalEnabledExperiments;
- let flags;
+ let originalEnabledExperiments: string[];
+ let flags: FlagsServiceImplementation;
suiteSetup(() => {
originalEnabledExperiments = window.ENABLED_EXPERIMENTS;
@@ -41,4 +41,3 @@
assert.deepEqual(flags.enabledExperiments, ['a']);
});
});
-
diff --git a/polygerrit-ui/app/utils/account-util_test.js b/polygerrit-ui/app/utils/account-util_test.ts
similarity index 75%
rename from polygerrit-ui/app/utils/account-util_test.js
rename to polygerrit-ui/app/utils/account-util_test.ts
index 0628f2d..835cd6d 100644
--- a/polygerrit-ui/app/utils/account-util_test.js
+++ b/polygerrit-ui/app/utils/account-util_test.ts
@@ -15,13 +15,12 @@
* limitations under the License.
*/
-import '../test/common-test-setup-karma.js';
-import {isServiceUser, removeServiceUsers} from './account-util.js';
-import {AccountTag} from '../constants/constants.js';
+import '../test/common-test-setup-karma';
+import {isServiceUser, removeServiceUsers} from './account-util';
+import {AccountTag} from '../constants/constants';
const EMPTY = {};
const ERNIE = {name: 'Ernie'};
-const KERMIT = {name: 'Kermit', tags: ['FROG']};
const SERVY = {name: 'Servy', tags: [AccountTag.SERVICE_USER]};
const BOTTY = {name: 'Botty', tags: [AccountTag.SERVICE_USER]};
@@ -30,17 +29,17 @@
assert.isFalse(isServiceUser());
assert.isFalse(isServiceUser(EMPTY));
assert.isFalse(isServiceUser(ERNIE));
- assert.isFalse(isServiceUser(KERMIT));
assert.isTrue(isServiceUser(SERVY));
assert.isTrue(isServiceUser(BOTTY));
});
test('removeServiceUsers', () => {
assert.sameMembers(removeServiceUsers([]), []);
- assert.sameMembers(removeServiceUsers([EMPTY, ERNIE, KERMIT]),
- [EMPTY, ERNIE, KERMIT]);
+ assert.sameMembers(removeServiceUsers([EMPTY, ERNIE]), [EMPTY, ERNIE]);
assert.sameMembers(removeServiceUsers([SERVY, BOTTY]), []);
- assert.sameMembers(removeServiceUsers([EMPTY, SERVY, ERNIE, BOTTY, KERMIT]),
- [EMPTY, ERNIE, KERMIT]);
+ assert.sameMembers(removeServiceUsers([EMPTY, SERVY, ERNIE, BOTTY]), [
+ EMPTY,
+ ERNIE,
+ ]);
});
});
diff --git a/polygerrit-ui/app/utils/async-util.ts b/polygerrit-ui/app/utils/async-util.ts
index 2b36fee..c82f5e4 100644
--- a/polygerrit-ui/app/utils/async-util.ts
+++ b/polygerrit-ui/app/utils/async-util.ts
@@ -110,3 +110,23 @@
existingTask?.cancel();
return new DelayedTask(callback, waitMs);
}
+
+const THROTTLE_INTERVAL_MS = 500;
+
+/**
+ * Ensure only one call is made within THROTTLE_INTERVAL_MS and any call within
+ * this interval is ignored
+ */
+export function throttleWrap(fn: (e: Event) => void) {
+ let lastCall: number | undefined;
+ return (e: Event) => {
+ if (
+ lastCall !== undefined &&
+ Date.now() - lastCall < THROTTLE_INTERVAL_MS
+ ) {
+ return;
+ }
+ lastCall = Date.now();
+ fn(e);
+ };
+}
diff --git a/polygerrit-ui/app/utils/safe-types-util_test.js b/polygerrit-ui/app/utils/safe-types-util_test.ts
similarity index 83%
rename from polygerrit-ui/app/utils/safe-types-util_test.js
rename to polygerrit-ui/app/utils/safe-types-util_test.ts
index e3968d0..03253e0 100644
--- a/polygerrit-ui/app/utils/safe-types-util_test.js
+++ b/polygerrit-ui/app/utils/safe-types-util_test.ts
@@ -15,12 +15,12 @@
* limitations under the License.
*/
-import '../test/common-test-setup-karma.js';
-import {safeTypesBridge, _testOnly_SafeUrl} from './safe-types-util.js';
+import '../test/common-test-setup-karma';
+import {safeTypesBridge, _testOnly_SafeUrl} from './safe-types-util';
suite('safe-types-util tests', () => {
test('SafeUrl accepts valid urls', () => {
- function accepts(url) {
+ function accepts(url: string) {
const safeUrl = new _testOnly_SafeUrl(url);
assert.isOk(safeUrl);
assert.equal(url, safeUrl.toString());
@@ -35,8 +35,10 @@
});
test('SafeUrl rejects invalid urls', () => {
- function rejects(url) {
- assert.throws(() => { new _testOnly_SafeUrl(url); });
+ function rejects(url: string) {
+ assert.throws(() => {
+ new _testOnly_SafeUrl(url);
+ });
}
rejects('javascript://alert("evil");');
rejects('ftp:example.com');
@@ -44,13 +46,14 @@
});
suite('safeTypesBridge', () => {
- function acceptsString(value, type) {
- assert.equal(safeTypesBridge(value, type),
- value);
+ function acceptsString(value: string, type: string) {
+ assert.equal(safeTypesBridge(value, type), value);
}
- function rejects(value, type) {
- assert.throws(() => { safeTypesBridge(value, type); });
+ function rejects(value: unknown, type: string) {
+ assert.throws(() => {
+ safeTypesBridge(value, type);
+ });
}
test('accepts valid URL strings', () => {
