Merge changes from topic "gr-confirm-revert-submission-dialog-to-ts"
* changes:
Convert gr-confirm-revert-submission-dialog to typescript
Rename files to preserve history
diff --git a/java/com/google/gerrit/extensions/api/changes/RevisionApi.java b/java/com/google/gerrit/extensions/api/changes/RevisionApi.java
index ff9fb3c..9064b97 100644
--- a/java/com/google/gerrit/extensions/api/changes/RevisionApi.java
+++ b/java/com/google/gerrit/extensions/api/changes/RevisionApi.java
@@ -106,6 +106,8 @@
List<RobotCommentInfo> robotCommentsAsList() throws RestApiException;
+ Map<String, List<CommentInfo>> portedComments() throws RestApiException;
+
/**
* Applies the indicated fix by creating a new change edit or integrating the fix with the
* existing change edit. If no change edit exists before this call, the fix must refer to the
@@ -294,6 +296,11 @@
}
@Override
+ public Map<String, List<CommentInfo>> portedComments() throws RestApiException {
+ throw new NotImplementedException();
+ }
+
+ @Override
public EditInfo applyFix(String fixId) throws RestApiException {
throw new NotImplementedException();
}
diff --git a/java/com/google/gerrit/extensions/common/testing/CommentInfoSubject.java b/java/com/google/gerrit/extensions/common/testing/CommentInfoSubject.java
index 923cfc3..c34e439 100644
--- a/java/com/google/gerrit/extensions/common/testing/CommentInfoSubject.java
+++ b/java/com/google/gerrit/extensions/common/testing/CommentInfoSubject.java
@@ -27,6 +27,7 @@
import com.google.gerrit.extensions.client.Side;
import com.google.gerrit.extensions.common.CommentInfo;
import com.google.gerrit.truth.ListSubject;
+import java.sql.Timestamp;
import java.util.List;
public class CommentInfoSubject extends Subject {
@@ -91,6 +92,10 @@
return check("inReplyTo").that(commentInfo().inReplyTo);
}
+ public StringSubject commitId() {
+ return check("commitId").that(commentInfo().commitId);
+ }
+
public AccountInfoSubject author() {
return check("author").about(accounts()).that(commentInfo().author);
}
@@ -99,6 +104,14 @@
return check("tag").that(commentInfo().tag);
}
+ public ComparableSubject<Timestamp> updated() {
+ return check("updated").that(commentInfo().updated);
+ }
+
+ public StringSubject changeMessageId() {
+ return check("changeMessageId").that(commentInfo().changeMessageId);
+ }
+
private CommentInfo commentInfo() {
isNotNull();
return commentInfo;
diff --git a/java/com/google/gerrit/server/api/changes/RevisionApiImpl.java b/java/com/google/gerrit/server/api/changes/RevisionApiImpl.java
index b515dfe..df01d8a 100644
--- a/java/com/google/gerrit/server/api/changes/RevisionApiImpl.java
+++ b/java/com/google/gerrit/server/api/changes/RevisionApiImpl.java
@@ -79,6 +79,7 @@
import com.google.gerrit.server.restapi.change.GetPatch;
import com.google.gerrit.server.restapi.change.GetRelated;
import com.google.gerrit.server.restapi.change.GetRevisionActions;
+import com.google.gerrit.server.restapi.change.ListPortedComments;
import com.google.gerrit.server.restapi.change.ListRevisionComments;
import com.google.gerrit.server.restapi.change.ListRevisionDrafts;
import com.google.gerrit.server.restapi.change.ListRobotComments;
@@ -130,6 +131,7 @@
private final FileApiImpl.Factory fileApi;
private final ListRevisionComments listComments;
private final ListRobotComments listRobotComments;
+ private final ListPortedComments listPortedComments;
private final ApplyFix applyFix;
private final GetFixPreview getFixPreview;
private final Fixes fixes;
@@ -175,6 +177,7 @@
FileApiImpl.Factory fileApi,
ListRevisionComments listComments,
ListRobotComments listRobotComments,
+ ListPortedComments listPortedComments,
ApplyFix applyFix,
GetFixPreview getFixPreview,
Fixes fixes,
@@ -219,6 +222,7 @@
this.listComments = listComments;
this.robotComments = robotComments;
this.listRobotComments = listRobotComments;
+ this.listPortedComments = listPortedComments;
this.applyFix = applyFix;
this.getFixPreview = getFixPreview;
this.fixes = fixes;
@@ -454,6 +458,15 @@
}
@Override
+ public Map<String, List<CommentInfo>> portedComments() throws RestApiException {
+ try {
+ return listPortedComments.apply(revision).value();
+ } catch (Exception e) {
+ throw asRestApiException("Cannot retrieve ported comments", e);
+ }
+ }
+
+ @Override
public EditInfo applyFix(String fixId) throws RestApiException {
try {
return applyFix.apply(fixes.parse(revision, IdString.fromDecoded(fixId)), null).value();
diff --git a/java/com/google/gerrit/server/patch/DiffMappings.java b/java/com/google/gerrit/server/patch/DiffMappings.java
new file mode 100644
index 0000000..e215e77
--- /dev/null
+++ b/java/com/google/gerrit/server/patch/DiffMappings.java
@@ -0,0 +1,52 @@
+// 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.
+
+package com.google.gerrit.server.patch;
+
+import static com.google.common.collect.ImmutableSet.toImmutableSet;
+
+import com.google.common.base.MoreObjects;
+import com.google.common.collect.ImmutableSet;
+import com.google.gerrit.server.patch.GitPositionTransformer.FileMapping;
+import com.google.gerrit.server.patch.GitPositionTransformer.Mapping;
+import com.google.gerrit.server.patch.GitPositionTransformer.Range;
+import com.google.gerrit.server.patch.GitPositionTransformer.RangeMapping;
+
+/** Mappings derived from diffs. */
+public class DiffMappings {
+
+ private DiffMappings() {}
+
+ public static Mapping toMapping(PatchListEntry patchListEntry) {
+ // This is just a direct translation of the former logic in EditTransformer. It doesn't
+ // work for file deletions, though. As file deletions aren't relevant for 'edits due to rebase'
+ // situations, we didn't notice this in the past.
+ // TODO(aliceks): Fix for file deletions in another change.
+ FileMapping fileMapping =
+ FileMapping.create(getOldFilePath(patchListEntry), patchListEntry.getNewName());
+ ImmutableSet<RangeMapping> rangeMappings =
+ patchListEntry.getEdits().stream()
+ .map(
+ edit ->
+ RangeMapping.create(
+ Range.create(edit.getBeginA(), edit.getEndA()),
+ Range.create(edit.getBeginB(), edit.getEndB())))
+ .collect(toImmutableSet());
+ return Mapping.create(fileMapping, rangeMappings);
+ }
+
+ private static String getOldFilePath(PatchListEntry patchListEntry) {
+ return MoreObjects.firstNonNull(patchListEntry.getOldName(), patchListEntry.getNewName());
+ }
+}
diff --git a/java/com/google/gerrit/server/patch/EditTransformer.java b/java/com/google/gerrit/server/patch/EditTransformer.java
index 90f442e..facfad59 100644
--- a/java/com/google/gerrit/server/patch/EditTransformer.java
+++ b/java/com/google/gerrit/server/patch/EditTransformer.java
@@ -15,19 +15,20 @@
package com.google.gerrit.server.patch;
import static com.google.common.collect.ImmutableList.toImmutableList;
+import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static com.google.common.collect.Multimaps.toMultimap;
-import static java.util.Comparator.comparing;
-import static java.util.stream.Collectors.groupingBy;
-import static java.util.stream.Collectors.toList;
import com.google.auto.value.AutoValue;
import com.google.common.base.MoreObjects;
import com.google.common.collect.ArrayListMultimap;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Multimap;
-import java.util.ArrayList;
+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.GitPositionTransformer.Range;
import java.util.List;
-import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.function.Function;
@@ -105,76 +106,23 @@
}
private void transformEdits(List<PatchListEntry> transformingEntries, SideStrategy sideStrategy) {
- Map<String, List<ContextAwareEdit>> editsPerFilePath =
- edits.stream().collect(groupingBy(sideStrategy::getFilePath));
- Map<String, List<PatchListEntry>> transEntriesPerPath =
- transformingEntries.stream().collect(groupingBy(EditTransformer::getOldFilePath));
+ ImmutableList<PositionedEntity<ContextAwareEdit>> positionedEdits =
+ edits.stream()
+ .map(edit -> toPositionedEntity(edit, sideStrategy))
+ .collect(toImmutableList());
+ ImmutableSet<Mapping> mappings =
+ transformingEntries.stream().map(DiffMappings::toMapping).collect(toImmutableSet());
edits =
- editsPerFilePath.entrySet().stream()
- .flatMap(
- pathAndEdits -> {
- List<PatchListEntry> transEntries =
- transEntriesPerPath.getOrDefault(pathAndEdits.getKey(), ImmutableList.of());
- return transformEdits(sideStrategy, pathAndEdits.getValue(), transEntries);
- })
- .collect(toList());
+ GitPositionTransformer.transform(positionedEdits, mappings).stream()
+ .map(PositionedEntity::getEntityAtUpdatedPosition)
+ .collect(toImmutableList());
}
- private static String getOldFilePath(PatchListEntry patchListEntry) {
- return MoreObjects.firstNonNull(patchListEntry.getOldName(), patchListEntry.getNewName());
- }
-
- private static Stream<ContextAwareEdit> transformEdits(
- SideStrategy sideStrategy,
- List<ContextAwareEdit> originalEdits,
- List<PatchListEntry> transformingEntries) {
- if (transformingEntries.isEmpty()) {
- return originalEdits.stream();
- }
-
- // TODO(aliceks): Find a way to prevent an explosion of the number of entries.
- return transformingEntries.stream()
- .flatMap(
- transEntry ->
- transformEdits(
- sideStrategy, originalEdits, transEntry.getEdits(), transEntry.getNewName())
- .stream());
- }
-
- private static List<ContextAwareEdit> transformEdits(
- SideStrategy sideStrategy,
- List<ContextAwareEdit> unorderedOriginalEdits,
- List<Edit> unorderedTransformingEdits,
- String adjustedFilePath) {
- List<ContextAwareEdit> originalEdits = new ArrayList<>(unorderedOriginalEdits);
- originalEdits.sort(comparing(sideStrategy::getBegin).thenComparing(sideStrategy::getEnd));
- List<Edit> transformingEdits = new ArrayList<>(unorderedTransformingEdits);
- transformingEdits.sort(comparing(Edit::getBeginA).thenComparing(Edit::getEndA));
-
- int shiftedAmount = 0;
- int transIndex = 0;
- int origIndex = 0;
- List<ContextAwareEdit> resultingEdits = new ArrayList<>(originalEdits.size());
- while (origIndex < originalEdits.size() && transIndex < transformingEdits.size()) {
- ContextAwareEdit originalEdit = originalEdits.get(origIndex);
- Edit transformingEdit = transformingEdits.get(transIndex);
- if (transformingEdit.getEndA() <= sideStrategy.getBegin(originalEdit)) {
- shiftedAmount = transformingEdit.getEndB() - transformingEdit.getEndA();
- transIndex++;
- } else if (sideStrategy.getEnd(originalEdit) <= transformingEdit.getBeginA()) {
- resultingEdits.add(sideStrategy.create(originalEdit, shiftedAmount, adjustedFilePath));
- origIndex++;
- } else {
- // Overlapping -> ignore.
- origIndex++;
- }
- }
- for (int i = origIndex; i < originalEdits.size(); i++) {
- resultingEdits.add(
- sideStrategy.create(originalEdits.get(i), shiftedAmount, adjustedFilePath));
- }
- return resultingEdits;
+ private static PositionedEntity<ContextAwareEdit> toPositionedEntity(
+ ContextAwareEdit edit, SideStrategy sideStrategy) {
+ return PositionedEntity.create(
+ edit, sideStrategy::extractPosition, sideStrategy::createEditAtNewPosition);
}
@AutoValue
@@ -234,44 +182,32 @@
}
private interface SideStrategy {
- String getFilePath(ContextAwareEdit edit);
+ Position extractPosition(ContextAwareEdit edit);
- int getBegin(ContextAwareEdit edit);
-
- int getEnd(ContextAwareEdit edit);
-
- ContextAwareEdit create(ContextAwareEdit edit, int shiftedAmount, String adjustedFilePath);
+ ContextAwareEdit createEditAtNewPosition(ContextAwareEdit edit, Position newPosition);
}
private enum SideAStrategy implements SideStrategy {
INSTANCE;
@Override
- public String getFilePath(ContextAwareEdit edit) {
- return edit.getOldFilePath();
+ public Position extractPosition(ContextAwareEdit edit) {
+ return Position.builder()
+ .filePath(edit.getOldFilePath())
+ .lineRange(Range.create(edit.getBeginA(), edit.getEndA()))
+ .build();
}
@Override
- public int getBegin(ContextAwareEdit edit) {
- return edit.getBeginA();
- }
-
- @Override
- public int getEnd(ContextAwareEdit edit) {
- return edit.getEndA();
- }
-
- @Override
- public ContextAwareEdit create(
- ContextAwareEdit edit, int shiftedAmount, String adjustedFilePath) {
+ public ContextAwareEdit createEditAtNewPosition(ContextAwareEdit edit, Position newPosition) {
return ContextAwareEdit.create(
- adjustedFilePath,
+ newPosition.filePath(),
edit.getNewFilePath(),
- edit.getBeginA() + shiftedAmount,
- edit.getEndA() + shiftedAmount,
+ newPosition.lineRange().start(),
+ newPosition.lineRange().end(),
edit.getBeginB(),
edit.getEndB(),
- !Objects.equals(edit.getOldFilePath(), adjustedFilePath));
+ !Objects.equals(edit.getOldFilePath(), newPosition.filePath()));
}
}
@@ -279,31 +215,23 @@
INSTANCE;
@Override
- public String getFilePath(ContextAwareEdit edit) {
- return edit.getNewFilePath();
+ public Position extractPosition(ContextAwareEdit edit) {
+ return Position.builder()
+ .filePath(edit.getNewFilePath())
+ .lineRange(Range.create(edit.getBeginB(), edit.getEndB()))
+ .build();
}
@Override
- public int getBegin(ContextAwareEdit edit) {
- return edit.getBeginB();
- }
-
- @Override
- public int getEnd(ContextAwareEdit edit) {
- return edit.getEndB();
- }
-
- @Override
- public ContextAwareEdit create(
- ContextAwareEdit edit, int shiftedAmount, String adjustedFilePath) {
+ public ContextAwareEdit createEditAtNewPosition(ContextAwareEdit edit, Position newPosition) {
return ContextAwareEdit.create(
edit.getOldFilePath(),
- adjustedFilePath,
+ newPosition.filePath(),
edit.getBeginA(),
edit.getEndA(),
- edit.getBeginB() + shiftedAmount,
- edit.getEndB() + shiftedAmount,
- !Objects.equals(edit.getNewFilePath(), adjustedFilePath));
+ newPosition.lineRange().start(),
+ newPosition.lineRange().end(),
+ !Objects.equals(edit.getNewFilePath(), newPosition.filePath()));
}
}
}
diff --git a/java/com/google/gerrit/server/patch/GitPositionTransformer.java b/java/com/google/gerrit/server/patch/GitPositionTransformer.java
new file mode 100644
index 0000000..64241d8
--- /dev/null
+++ b/java/com/google/gerrit/server/patch/GitPositionTransformer.java
@@ -0,0 +1,455 @@
+// 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.
+
+package com.google.gerrit.server.patch;
+
+import static com.google.common.collect.ImmutableList.toImmutableList;
+import static com.google.common.collect.ImmutableSet.toImmutableSet;
+import static java.util.Comparator.comparing;
+import static java.util.stream.Collectors.collectingAndThen;
+import static java.util.stream.Collectors.groupingBy;
+
+import com.google.auto.value.AutoValue;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Sets;
+import java.util.Collection;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.function.BiFunction;
+import java.util.function.Function;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+/**
+ * Transformer of {@link Position}s in one Git tree to {@link Position}s in another Git tree given
+ * the {@link Mapping}s between the trees.
+ *
+ * <p>The base idea is that a {@link Position} in the source tree can be translated/mapped to a
+ * corresponding {@link Position} in the target tree when we know how the target tree changed
+ * compared to the source tree. As long as {@link Position}s are only defined via file path and line
+ * range, we only need to know which file path in the source tree corresponds to which file path in
+ * the target tree and how the lines within that file changed from the source to the target tree.
+ *
+ * <p>The algorithm is roughly:
+ *
+ * <ol>
+ * <li>Go over all positions and replace the file path for each of them with the corresponding one
+ * in the target tree. If a file path maps to two file paths in the target tree (copied file),
+ * duplicate the position entry and use each of the new file paths with it. If a file path
+ * maps to no file in the target tree (deleted file), drop the position.
+ * <li>Per file path, go through the file from top to bottom and keep track of how the range
+ * mappings for that file shift the lines. Derive the shifted amount by comparing the number
+ * of lines between source and target in the range mapping. While going through the file,
+ * shift each encountered position by the currently tracked amount. If a position overlaps
+ * with the lines of a range mapping, drop the position.
+ * </ol>
+ */
+public class GitPositionTransformer {
+
+ // This is currently only a utility class but it won't stay like that.
+ private GitPositionTransformer() {}
+
+ /**
+ * Transforms the {@link Position}s of the specified entities as indicated via the {@link
+ * Mapping}s.
+ *
+ * <p>This is typically used to transform the {@link Position}s in one Git tree (source) to the
+ * corresponding {@link Position}s in another Git tree (target). The {@link Mapping}s need to
+ * indicate all relevant changes between the source and target tree. {@link Mapping}s for files
+ * not referenced by the given {@link Position}s need not be specified. They can be included,
+ * though, as they aren't harmful.
+ *
+ * @param entities the entities whose {@link Position} should be mapped to the target tree
+ * @param mappings the mappings describing all relevant changes between the source and the target
+ * tree
+ * @param <T> an entity which has a {@link Position}
+ * @return a list of entities with transformed positions
+ */
+ public static <T> ImmutableList<PositionedEntity<T>> transform(
+ Collection<PositionedEntity<T>> entities, Set<Mapping> mappings) {
+ // Update the file paths first as copied files might exist. For copied files, this operation
+ // will duplicate the PositionedEntity instances of the original file.
+ List<PositionedEntity<T>> filePathUpdatedEntities = updateFilePaths(entities, mappings);
+
+ return shiftRanges(filePathUpdatedEntities, mappings);
+ }
+
+ private static <T> ImmutableList<PositionedEntity<T>> updateFilePaths(
+ Collection<PositionedEntity<T>> entities, Set<Mapping> mappings) {
+ Map<String, ImmutableSet<String>> newFilesPerOldFile = groupNewFilesByOldFiles(mappings);
+ return entities.stream()
+ .flatMap(entity -> mapToNewFileIfChanged(newFilesPerOldFile, entity))
+ .collect(toImmutableList());
+ }
+
+ private static Map<String, ImmutableSet<String>> groupNewFilesByOldFiles(Set<Mapping> mappings) {
+ return mappings.stream()
+ .map(Mapping::file)
+ .collect(
+ groupingBy(
+ FileMapping::oldPath, Collectors.mapping(FileMapping::newPath, toImmutableSet())));
+ }
+
+ private static <T> Stream<PositionedEntity<T>> mapToNewFileIfChanged(
+ Map<String, ? extends Set<String>> newFilesPerOldFile, PositionedEntity<T> entity) {
+ String oldFilePath = entity.position().filePath();
+ if (!newFilesPerOldFile.containsKey(oldFilePath)) {
+ // Unchanged files don't have a mapping. -> Keep existing entries.
+ return Stream.of(entity);
+ }
+ Set<String> newFiles = newFilesPerOldFile.get(oldFilePath);
+ return newFiles.stream().map(entity::withFilePath);
+ }
+
+ private static <T> ImmutableList<PositionedEntity<T>> shiftRanges(
+ List<PositionedEntity<T>> filePathUpdatedEntities, Set<Mapping> mappings) {
+ Map<String, ImmutableSet<RangeMapping>> mappingsPerNewFilePath =
+ groupRangeMappingsByNewFilePath(mappings);
+ Map<String, ImmutableList<PositionedEntity<T>>> entitiesPerNewFilePath =
+ groupByFilePath(filePathUpdatedEntities);
+ return entitiesPerNewFilePath.entrySet().stream()
+ .flatMap(
+ newFilePathAndEntities ->
+ shiftRangesInOneFileIfChanged(
+ mappingsPerNewFilePath,
+ newFilePathAndEntities.getKey(),
+ newFilePathAndEntities.getValue())
+ .stream())
+ .collect(toImmutableList());
+ }
+
+ private static Map<String, ImmutableSet<RangeMapping>> groupRangeMappingsByNewFilePath(
+ Set<Mapping> mappings) {
+ return mappings.stream()
+ .collect(
+ groupingBy(
+ mapping -> mapping.file().newPath(),
+ collectingAndThen(
+ Collectors.<Mapping, Set<RangeMapping>>reducing(
+ new HashSet<>(), Mapping::ranges, Sets::union),
+ ImmutableSet::copyOf)));
+ }
+
+ private static <T> Map<String, ImmutableList<PositionedEntity<T>>> groupByFilePath(
+ List<PositionedEntity<T>> fileUpdatedEntities) {
+ return fileUpdatedEntities.stream()
+ .collect(groupingBy(entity -> entity.position().filePath(), toImmutableList()));
+ }
+
+ private static <T> ImmutableList<PositionedEntity<T>> shiftRangesInOneFileIfChanged(
+ Map<String, ImmutableSet<RangeMapping>> mappingsPerNewFilePath,
+ String newFilePath,
+ ImmutableList<PositionedEntity<T>> sameFileEntities) {
+ ImmutableSet<RangeMapping> sameFileRangeMappings =
+ mappingsPerNewFilePath.getOrDefault(newFilePath, ImmutableSet.of());
+ if (sameFileRangeMappings.isEmpty()) {
+ // Unchanged files and pure renames/copies don't have range mappings. -> Keep existing
+ // entries.
+ return sameFileEntities;
+ }
+ return shiftRangesInOneFile(sameFileEntities, sameFileRangeMappings);
+ }
+
+ private static <T> ImmutableList<PositionedEntity<T>> shiftRangesInOneFile(
+ List<PositionedEntity<T>> sameFileEntities, Set<RangeMapping> sameFileRangeMappings) {
+ ImmutableList<PositionedEntity<T>> sortedEntities = sortByStartEnd(sameFileEntities);
+ ImmutableList<RangeMapping> sortedMappings = sortByOldStartEnd(sameFileRangeMappings);
+
+ int shiftedAmount = 0;
+ int mappingIndex = 0;
+ int entityIndex = 0;
+ ImmutableList.Builder<PositionedEntity<T>> resultingEntities =
+ ImmutableList.builderWithExpectedSize(sortedEntities.size());
+ while (entityIndex < sortedEntities.size() && mappingIndex < sortedMappings.size()) {
+ PositionedEntity<T> entity = sortedEntities.get(entityIndex);
+ RangeMapping mapping = sortedMappings.get(mappingIndex);
+ if (mapping.oldLineRange().end() <= entity.position().lineRange().start()) {
+ shiftedAmount = mapping.newLineRange().end() - mapping.oldLineRange().end();
+ mappingIndex++;
+ } else if (entity.position().lineRange().end() <= mapping.oldLineRange().start()) {
+ resultingEntities.add(entity.shiftPositionBy(shiftedAmount));
+ entityIndex++;
+ } else {
+ // Overlapping -> ignore.
+ entityIndex++;
+ }
+ }
+ for (int i = entityIndex; i < sortedEntities.size(); i++) {
+ resultingEntities.add(sortedEntities.get(i).shiftPositionBy(shiftedAmount));
+ }
+ return resultingEntities.build();
+ }
+
+ private static <T> ImmutableList<PositionedEntity<T>> sortByStartEnd(
+ List<PositionedEntity<T>> entities) {
+ return entities.stream()
+ .sorted(
+ comparing(
+ entity -> entity.position().lineRange(),
+ comparing(Range::start).thenComparing(Range::end)))
+ .collect(toImmutableList());
+ }
+
+ private static ImmutableList<RangeMapping> sortByOldStartEnd(Set<RangeMapping> mappings) {
+ return mappings.stream()
+ .sorted(
+ comparing(
+ RangeMapping::oldLineRange, comparing(Range::start).thenComparing(Range::end)))
+ .collect(toImmutableList());
+ }
+
+ /**
+ * A mapping from a {@link Position} in one Git commit/tree (source) to a {@link Position} in
+ * another Git commit/tree (target).
+ */
+ @AutoValue
+ public abstract static class Mapping {
+
+ /** A mapping describing how the attributes of one file are mapped from source to target. */
+ public abstract FileMapping file();
+
+ /**
+ * Mappings describing how line ranges within the file indicated by {@link #file()} are mapped
+ * from source to target.
+ */
+ public abstract ImmutableSet<RangeMapping> ranges();
+
+ public static Mapping create(FileMapping fileMapping, Iterable<RangeMapping> rangeMappings) {
+ return new AutoValue_GitPositionTransformer_Mapping(
+ fileMapping, ImmutableSet.copyOf(rangeMappings));
+ }
+ }
+
+ /**
+ * A mapping of attributes from a file in one Git tree (source) to a file in another Git tree
+ * (target).
+ *
+ * <p>At the moment, only the file path is considered. Other attributes like file mode would be
+ * imaginable too but are currently not supported.
+ */
+ @AutoValue
+ public abstract static class FileMapping {
+
+ /** File path in the source tree. */
+ public abstract String oldPath();
+
+ /** File path in the target tree. Can be the same as {@link #oldPath()}. */
+ public abstract String newPath();
+
+ /**
+ * Creates a new {@code FileMapping}.
+ *
+ * @param oldPath see {@link #oldPath()}
+ * @param newPath see {@link #newPath()}
+ */
+ public static FileMapping create(String oldPath, String newPath) {
+ return new AutoValue_GitPositionTransformer_FileMapping(oldPath, newPath);
+ }
+ }
+
+ /**
+ * A mapping of a line range in one Git tree (source) to the corresponding line range in another
+ * Git tree (target).
+ */
+ @AutoValue
+ public abstract static class RangeMapping {
+
+ /** Range in the source tree. */
+ public abstract Range oldLineRange();
+
+ /** Range in the target tree. */
+ public abstract Range newLineRange();
+
+ /**
+ * Creates a new {@code RangeMapping}.
+ *
+ * @param oldRange see {@link #oldLineRange()}
+ * @param newRange see {@link #newLineRange()}
+ */
+ public static RangeMapping create(Range oldRange, Range newRange) {
+ return new AutoValue_GitPositionTransformer_RangeMapping(oldRange, newRange);
+ }
+ }
+
+ /**
+ * A position within the tree of a Git commit.
+ *
+ * <p>The term 'position' is our own invention. The underlying idea is that a Gerrit comment is at
+ * a specific position within the commit of a patchset. That position is defined by the attributes
+ * defined in this class.
+ *
+ * <p>The same thinking can be applied to diff hunks (= JGit edits). Each diff hunk maps a
+ * position in one commit (e.g. in the parent of the patchset) to a position in another commit
+ * (e.g. in the commit of the patchset).
+ *
+ * <p>We only refer to lines and not character offsets within the lines here as Git only works
+ * with line precision. In theory, we could do better in Gerrit as we also have intraline diffs.
+ * Incorporating those requires careful considerations, though.
+ */
+ @AutoValue
+ public abstract static class Position {
+
+ /** Absolute file path. */
+ public abstract String filePath();
+
+ /** Affected lines. */
+ public abstract Range lineRange();
+
+ /**
+ * Creates a copy of this {@code Position} whose range is shifted by the indicated amount.
+ *
+ * @param amount number of lines to shift. Negative values mean moving the range up, positive
+ * values mean moving the range down.
+ * @return a new {@code Position} instance with an updated range
+ */
+ public Position shiftBy(int amount) {
+ return toBuilder().lineRange(lineRange().shiftBy(amount)).build();
+ }
+
+ /**
+ * Creates a copy of this {@code Position} whose file path is adjusted to the indicated value.
+ *
+ * @param filePath the new file path to use
+ * @return a new {@code Position} instance with an update file path
+ */
+ public Position withFilePath(String filePath) {
+ return toBuilder().filePath(filePath).build();
+ }
+
+ abstract Builder toBuilder();
+
+ public static Builder builder() {
+ return new AutoValue_GitPositionTransformer_Position.Builder();
+ }
+
+ /** Builder of a {@link Position}. */
+ @AutoValue.Builder
+ public abstract static class Builder {
+
+ /** See {@link #filePath()}. */
+ public abstract Builder filePath(String filePath);
+
+ /** See {@link #lineRange()}. */
+ public abstract Builder lineRange(Range lineRange);
+
+ public abstract Position build();
+ }
+ }
+
+ /** A range. In the context of {@link GitPositionTransformer}, this is a line range. */
+ @AutoValue
+ public abstract static class Range {
+
+ /** Start of the range. (inclusive) */
+ public abstract int start();
+
+ /** End of the range. (exclusive) */
+ public abstract int end();
+
+ /**
+ * Creates a copy of this {@code Range} which is shifted by the indicated amount. A shift
+ * equally applies to both {@link #start()} end {@link #end()}.
+ *
+ * @param amount amount to shift. Negative values mean moving the range up, positive values mean
+ * moving the range down.
+ * @return a new {@code Range} instance with updated start/end
+ */
+ public Range shiftBy(int amount) {
+ return create(start() + amount, end() + amount);
+ }
+
+ public static Range create(int start, int end) {
+ return new AutoValue_GitPositionTransformer_Range(start, end);
+ }
+ }
+
+ /**
+ * Wrapper around an instance of {@code T} which annotates it with a {@link Position}. Methods
+ * such as {@link #shiftPositionBy(int)} and {@link #withFilePath(String)} allow to update the
+ * associated {@link Position}. Afterwards, use {@link #getEntityAtUpdatedPosition()} to get an
+ * updated version of the {@code T} instance.
+ *
+ * @param <T> an object/entity type which has a {@link Position}
+ */
+ public static class PositionedEntity<T> {
+
+ private final T entity;
+ private final Position position;
+ private final BiFunction<T, Position, T> updatedEntityCreator;
+
+ /**
+ * Creates a new {@code PositionedEntity}.
+ *
+ * @param entity an instance which should be annotated with a {@link Position}
+ * @param positionExtractor a function describing how a {@link Position} can be derived from the
+ * given entity
+ * @param updatedEntityCreator a function to create a new entity of type {@code T} from an
+ * existing entity and a given {@link Position}. This must return a new instance of type
+ * {@code T}! The existing instance must not be modified!
+ * @param <T> an object/entity type which has a {@link Position}
+ */
+ public static <T> PositionedEntity<T> create(
+ T entity,
+ Function<T, Position> positionExtractor,
+ BiFunction<T, Position, T> updatedEntityCreator) {
+ Position position = positionExtractor.apply(entity);
+ return new PositionedEntity<>(entity, position, updatedEntityCreator);
+ }
+
+ private PositionedEntity(
+ T entity, Position position, BiFunction<T, Position, T> updatedEntityCreator) {
+ this.entity = entity;
+ this.position = position;
+ this.updatedEntityCreator = updatedEntityCreator;
+ }
+
+ /**
+ * Returns an updated version of the entity to which the internally stored {@link Position} was
+ * written back to.
+ *
+ * @return an updated instance of {@code T}
+ */
+ public T getEntityAtUpdatedPosition() {
+ return updatedEntityCreator.apply(entity, position);
+ }
+
+ Position position() {
+ return position;
+ }
+
+ /**
+ * Shifts the tracked {@link Position} by the specified amount.
+ *
+ * @param amount number of lines to shift. Negative values mean moving the range up, positive
+ * values mean moving the range down.
+ * @return a {@code PositionedEntity} with updated {@link Position}
+ */
+ public PositionedEntity<T> shiftPositionBy(int amount) {
+ return new PositionedEntity<>(entity, position.shiftBy(amount), updatedEntityCreator);
+ }
+
+ /**
+ * Updates the file path of the tracked {@link Position}.
+ *
+ * @param filePath the new file path to use
+ * @return a {@code PositionedEntity} with updated {@link Position}
+ */
+ public PositionedEntity<T> withFilePath(String filePath) {
+ return new PositionedEntity<>(entity, position.withFilePath(filePath), updatedEntityCreator);
+ }
+ }
+}
diff --git a/java/com/google/gerrit/server/restapi/change/ListPortedComments.java b/java/com/google/gerrit/server/restapi/change/ListPortedComments.java
new file mode 100644
index 0000000..3f9e127
--- /dev/null
+++ b/java/com/google/gerrit/server/restapi/change/ListPortedComments.java
@@ -0,0 +1,189 @@
+// 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.
+
+package com.google.gerrit.server.restapi.change;
+
+import static com.google.common.collect.ImmutableList.toImmutableList;
+import static com.google.common.collect.ImmutableSet.toImmutableSet;
+import static java.util.stream.Collectors.groupingBy;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
+import com.google.gerrit.entities.Comment;
+import com.google.gerrit.entities.Comment.Range;
+import com.google.gerrit.entities.HumanComment;
+import com.google.gerrit.entities.PatchSet;
+import com.google.gerrit.entities.Project;
+import com.google.gerrit.extensions.client.DiffPreferencesInfo.Whitespace;
+import com.google.gerrit.extensions.common.CommentInfo;
+import com.google.gerrit.extensions.restapi.Response;
+import com.google.gerrit.extensions.restapi.RestReadView;
+import com.google.gerrit.server.CommentsUtil;
+import com.google.gerrit.server.change.RevisionResource;
+import com.google.gerrit.server.notedb.ChangeNotes;
+import com.google.gerrit.server.patch.DiffMappings;
+import com.google.gerrit.server.patch.GitPositionTransformer;
+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.permissions.PermissionBackendException;
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import com.google.inject.Singleton;
+import java.util.List;
+import java.util.Map;
+
+@Singleton
+public class ListPortedComments implements RestReadView<RevisionResource> {
+
+ private final CommentsUtil commentsUtil;
+ private final Provider<CommentJson> commentJson;
+ private final PatchListCache patchListCache;
+
+ @Inject
+ public ListPortedComments(
+ Provider<CommentJson> commentJson, CommentsUtil commentsUtil, PatchListCache patchListCache) {
+ this.commentJson = commentJson;
+ this.commentsUtil = commentsUtil;
+ this.patchListCache = patchListCache;
+ }
+
+ @Override
+ public Response<Map<String, List<CommentInfo>>> apply(RevisionResource revisionResource)
+ throws PermissionBackendException, PatchListNotAvailableException {
+ PatchSet targetPatchset = revisionResource.getPatchSet();
+
+ List<HumanComment> allComments = loadAllPublishedComments(revisionResource);
+ ImmutableList<HumanComment> relevantComments = filterToRelevant(allComments, targetPatchset);
+ ImmutableList<HumanComment> portedComments =
+ portToTargetPatchset(
+ revisionResource.getChangeResource().getNotes(), targetPatchset, relevantComments);
+ return Response.ok(format(portedComments));
+ }
+
+ private List<HumanComment> loadAllPublishedComments(RevisionResource revisionResource) {
+ return commentsUtil.publishedHumanCommentsByChange(revisionResource.getNotes());
+ }
+
+ private ImmutableList<HumanComment> filterToRelevant(
+ List<HumanComment> allComments, PatchSet targetPatchset) {
+ return allComments.stream()
+ .filter(comment -> comment.key.patchSetId < targetPatchset.number())
+ // TODO(aliceks): Also support comments on parent revisions.
+ .filter(comment -> comment.side > 0)
+ .collect(toImmutableList());
+ }
+
+ private ImmutableList<HumanComment> portToTargetPatchset(
+ ChangeNotes notes, PatchSet targetPatchset, List<HumanComment> comments)
+ throws PatchListNotAvailableException {
+ Map<Integer, ImmutableList<HumanComment>> commentsPerPatchset =
+ comments.stream().collect(groupingBy(comment -> comment.key.patchSetId, toImmutableList()));
+
+ ImmutableList.Builder<HumanComment> portedComments =
+ ImmutableList.builderWithExpectedSize(comments.size());
+ for (Integer originalPatchsetId : commentsPerPatchset.keySet()) {
+ ImmutableList<HumanComment> patchsetComments = commentsPerPatchset.get(originalPatchsetId);
+ PatchSet originalPatchset =
+ notes.getPatchSets().get(PatchSet.id(notes.getChangeId(), originalPatchsetId));
+ portedComments.addAll(
+ portToTargetPatchset(
+ notes.getProjectName(), originalPatchset, targetPatchset, patchsetComments));
+ }
+ return portedComments.build();
+ }
+
+ private ImmutableList<HumanComment> portToTargetPatchset(
+ Project.NameKey project,
+ PatchSet originalPatchset,
+ PatchSet targetPatchset,
+ ImmutableList<HumanComment> comments)
+ throws PatchListNotAvailableException {
+ ImmutableSet<Mapping> mappings =
+ loadPatchsetMappings(project, originalPatchset, targetPatchset);
+ ImmutableList<PositionedEntity<HumanComment>> positionedComments =
+ comments.stream().map(this::toPositionedEntity).collect(toImmutableList());
+ return GitPositionTransformer.transform(positionedComments, mappings).stream()
+ .map(PositionedEntity::getEntityAtUpdatedPosition)
+ .collect(toImmutableList());
+ }
+
+ private ImmutableSet<Mapping> loadPatchsetMappings(
+ Project.NameKey project, PatchSet originalPatchset, PatchSet targetPatchset)
+ throws PatchListNotAvailableException {
+ PatchList patchList =
+ patchListCache.get(
+ PatchListKey.againstCommit(
+ originalPatchset.commitId(), targetPatchset.commitId(), Whitespace.IGNORE_NONE),
+ project);
+ return patchList.getPatches().stream().map(DiffMappings::toMapping).collect(toImmutableSet());
+ }
+
+ private PositionedEntity<HumanComment> toPositionedEntity(HumanComment comment) {
+ return PositionedEntity.create(
+ comment,
+ ListPortedComments::extractPosition,
+ ListPortedComments::createCommentAtNewPosition);
+ }
+
+ private static Position extractPosition(HumanComment comment) {
+ Position.Builder positionBuilder = Position.builder().filePath(comment.key.filename);
+ return positionBuilder.lineRange(extractLineRange(comment)).build();
+ }
+
+ private static GitPositionTransformer.Range extractLineRange(HumanComment comment) {
+ // Line specifications in comment are 1-based. Line specifications in Position are 0-based.
+ if (comment.range != null) {
+ // The combination of (line, charOffset) is exclusive and must be mapped to an exclusive line.
+ int exclusiveEndLine =
+ comment.range.endChar > 0 ? comment.range.endLine : comment.range.endLine - 1;
+ return GitPositionTransformer.Range.create(comment.range.startLine - 1, exclusiveEndLine);
+ }
+ return GitPositionTransformer.Range.create(comment.lineNbr - 1, comment.lineNbr);
+ }
+
+ private static HumanComment createCommentAtNewPosition(
+ HumanComment originalComment, Position newPosition) {
+ HumanComment portedComment = new HumanComment(originalComment);
+ portedComment.key.filename = newPosition.filePath();
+ portedComment.lineNbr = newPosition.lineRange().start() + 1;
+ if (portedComment.range != null) {
+ portedComment.range =
+ toRange(
+ newPosition.lineRange(), portedComment.range.startChar, portedComment.range.endChar);
+ portedComment.lineNbr = portedComment.range.endLine;
+ }
+ return portedComment;
+ }
+
+ private static Comment.Range toRange(
+ GitPositionTransformer.Range lineRange, int originalStartChar, int originalEndChar) {
+ int adjustedEndLine = originalEndChar > 0 ? lineRange.end() : lineRange.end() + 1;
+ return new Range(lineRange.start() + 1, originalStartChar, adjustedEndLine, originalEndChar);
+ }
+
+ private Map<String, List<CommentInfo>> format(List<HumanComment> comments)
+ throws PermissionBackendException {
+ return commentJson
+ .get()
+ .setFillAccounts(true)
+ .setFillPatchSet(true)
+ .newHumanCommentFormatter()
+ .format(comments);
+ }
+}
diff --git a/java/com/google/gerrit/server/restapi/change/Module.java b/java/com/google/gerrit/server/restapi/change/Module.java
index 466ea3c..1a4f270 100644
--- a/java/com/google/gerrit/server/restapi/change/Module.java
+++ b/java/com/google/gerrit/server/restapi/change/Module.java
@@ -172,6 +172,8 @@
post(FIX_KIND, "apply").to(ApplyFix.class);
get(FIX_KIND, "preview").to(GetFixPreview.class);
+ get(REVISION_KIND, "ported_comments").to(ListPortedComments.class);
+
child(REVISION_KIND, "files").to(Files.class);
put(FILE_KIND, "reviewed").to(PutReviewed.class);
delete(FILE_KIND, "reviewed").to(DeleteReviewed.class);
diff --git a/java/com/google/gerrit/sshd/commands/DefaultCommandModule.java b/java/com/google/gerrit/sshd/commands/DefaultCommandModule.java
index 4d77c60..c7fe74e 100644
--- a/java/com/google/gerrit/sshd/commands/DefaultCommandModule.java
+++ b/java/com/google/gerrit/sshd/commands/DefaultCommandModule.java
@@ -82,6 +82,16 @@
command(git, "upload-pack").to(Upload.class);
command("git-upload-archive").to(Commands.key(git, "upload-archive"));
command(git, "upload-archive").to(UploadArchive.class);
+ if (slaveMode) {
+ command("git-receive-pack").to(ReceiveSlaveMode.class);
+ command("gerrit-receive-pack").to(ReceiveSlaveMode.class);
+ command(git, "receive-pack").to(ReceiveSlaveMode.class);
+ } else {
+ command("git-receive-pack").to(Commands.key(git, "receive-pack"));
+ command("gerrit-receive-pack").to(Commands.key(git, "receive-pack"));
+ command(git, "receive-pack").to(Commands.key(gerrit, "receive-pack"));
+ command(gerrit, "test-submit").toProvider(new DispatchCommandProvider(testSubmit));
+ }
}
command("suexec").to(SuExec.class);
listener().to(ShowCaches.StartupListener.class);
@@ -91,18 +101,6 @@
command(gerrit, CreateProjectCommand.class);
command(gerrit, SetHeadCommand.class);
- if (slaveMode) {
- command("git-receive-pack").to(ReceiveSlaveMode.class);
- command("gerrit-receive-pack").to(ReceiveSlaveMode.class);
- command(git, "receive-pack").to(ReceiveSlaveMode.class);
- } else {
- if (sshEnabled()) {
- command("git-receive-pack").to(Commands.key(git, "receive-pack"));
- command("gerrit-receive-pack").to(Commands.key(git, "receive-pack"));
- command(git, "receive-pack").to(Commands.key(gerrit, "receive-pack"));
- }
- command(gerrit, "test-submit").toProvider(new DispatchCommandProvider(testSubmit));
- }
command(gerrit, Receive.class);
command(gerrit, RenameGroupCommand.class);
diff --git a/javatests/com/google/gerrit/acceptance/api/revision/PortedCommentsIT.java b/javatests/com/google/gerrit/acceptance/api/revision/PortedCommentsIT.java
new file mode 100644
index 0000000..7e844b6
--- /dev/null
+++ b/javatests/com/google/gerrit/acceptance/api/revision/PortedCommentsIT.java
@@ -0,0 +1,1547 @@
+// 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.
+
+package com.google.gerrit.acceptance.api.revision;
+
+import static com.google.common.collect.ImmutableList.toImmutableList;
+import static com.google.common.collect.Iterables.getOnlyElement;
+import static com.google.common.collect.MoreCollectors.onlyElement;
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.extensions.common.testing.CommentInfoSubject.assertThat;
+import static com.google.gerrit.extensions.common.testing.CommentInfoSubject.assertThatList;
+import static com.google.gerrit.truth.MapSubject.assertThatMap;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.truth.Correspondence;
+import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.testsuite.account.AccountOperations;
+import com.google.gerrit.acceptance.testsuite.change.ChangeOperations;
+import com.google.gerrit.acceptance.testsuite.change.TestPatchset;
+import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
+import com.google.gerrit.entities.Account;
+import com.google.gerrit.entities.Change;
+import com.google.gerrit.entities.Patch;
+import com.google.gerrit.entities.PatchSet;
+import com.google.gerrit.extensions.common.CommentInfo;
+import com.google.gerrit.extensions.restapi.RestApiException;
+import com.google.gerrit.truth.NullAwareCorrespondence;
+import com.google.inject.Inject;
+import java.util.Collection;
+import java.util.List;
+import java.util.Map;
+import org.junit.Ignore;
+import org.junit.Test;
+
+public class PortedCommentsIT extends AbstractDaemonTest {
+
+ @Inject private ChangeOperations changeOps;
+ @Inject private AccountOperations accountOps;
+ @Inject private RequestScopeOperations requestScopeOps;
+
+ @Test
+ public void onlyCommentsBeforeTargetPatchsetArePorted() throws Exception {
+ // Set up change and patchsets.
+ Change.Id changeId = changeOps.newChange().create();
+ PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId();
+ PatchSet.Id patchset2Id = changeOps.change(changeId).newPatchset().create();
+ PatchSet.Id patchset3Id = changeOps.change(changeId).newPatchset().create();
+ // Add comments.
+ String comment1Uuid = changeOps.change(changeId).patchset(patchset1Id).newComment().create();
+ changeOps.change(changeId).patchset(patchset2Id).newComment().create();
+ changeOps.change(changeId).patchset(patchset3Id).newComment().create();
+
+ List<CommentInfo> portedComments = flatten(getPortedComments(patchset2Id));
+
+ assertThatList(portedComments).comparingElementsUsing(hasUuid()).containsExactly(comment1Uuid);
+ }
+
+ @Test
+ public void commentsOnAnyPatchsetBeforeTargetPatchsetArePorted() throws Exception {
+ // Set up change and patchsets.
+ Change.Id changeId = changeOps.newChange().create();
+ PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId();
+ changeOps.change(changeId).newPatchset().create();
+ PatchSet.Id patchset3Id = changeOps.change(changeId).newPatchset().create();
+ PatchSet.Id patchset4Id = changeOps.change(changeId).newPatchset().create();
+ // Add comments.
+ String comment1Uuid = changeOps.change(changeId).patchset(patchset1Id).newComment().create();
+ String comment3Uuid = changeOps.change(changeId).patchset(patchset3Id).newComment().create();
+
+ List<CommentInfo> portedComments = flatten(getPortedComments(patchset4Id));
+
+ assertThat(portedComments)
+ .comparingElementsUsing(hasUuid())
+ .containsExactly(comment1Uuid, comment3Uuid);
+ }
+
+ @Test
+ public void severalCommentsFromEarlierPatchsetArePorted() throws Exception {
+ // Set up change and patchsets.
+ Change.Id changeId = changeOps.newChange().create();
+ PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId();
+ PatchSet.Id patchset2Id = changeOps.change(changeId).newPatchset().create();
+ // Add comments.
+ String comment1Uuid = changeOps.change(changeId).patchset(patchset1Id).newComment().create();
+ String comment2Uuid = changeOps.change(changeId).patchset(patchset1Id).newComment().create();
+
+ List<CommentInfo> portedComments = flatten(getPortedComments(patchset2Id));
+
+ assertThat(portedComments)
+ .comparingElementsUsing(hasUuid())
+ .containsExactly(comment1Uuid, comment2Uuid);
+ }
+
+ @Test
+ public void completeCommentThreadIsPorted() throws Exception {
+ // Set up change and patchsets.
+ Change.Id changeId = changeOps.newChange().create();
+ PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId();
+ PatchSet.Id patchset2Id = changeOps.change(changeId).newPatchset().create();
+ // Add comments.
+ String rootCommentUuid = changeOps.change(changeId).patchset(patchset1Id).newComment().create();
+ String child1CommentUuid =
+ changeOps
+ .change(changeId)
+ .patchset(patchset1Id)
+ .newComment()
+ .parentUuid(rootCommentUuid)
+ .create();
+ String child2CommentUuid =
+ changeOps
+ .change(changeId)
+ .patchset(patchset1Id)
+ .newComment()
+ .parentUuid(child1CommentUuid)
+ .create();
+
+ List<CommentInfo> portedComments = flatten(getPortedComments(patchset2Id));
+
+ assertThat(portedComments)
+ .comparingElementsUsing(hasUuid())
+ .containsExactly(rootCommentUuid, child1CommentUuid, child2CommentUuid);
+ }
+
+ @Test
+ // TODO(aliceks): Filter out unresolved comment threads.
+ @Ignore
+ public void onlyUnresolvedCommentsArePorted() throws Exception {
+ // Set up change and patchsets.
+ Change.Id changeId = changeOps.newChange().create();
+ PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId();
+ PatchSet.Id patchset2Id = changeOps.change(changeId).newPatchset().create();
+ // Add comments.
+ changeOps.change(changeId).patchset(patchset1Id).newComment().resolved().create();
+ String comment2Uuid =
+ changeOps.change(changeId).patchset(patchset1Id).newComment().unresolved().create();
+
+ List<CommentInfo> portedComments = flatten(getPortedComments(patchset2Id));
+
+ assertThat(portedComments).comparingElementsUsing(hasUuid()).containsExactly(comment2Uuid);
+ }
+
+ @Test
+ // TODO(aliceks): Filter out unresolved comment threads.
+ @Ignore
+ public void unresolvedStateOfLastCommentInThreadMatters() throws Exception {
+ // Set up change and patchsets.
+ Change.Id changeId = changeOps.newChange().create();
+ PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId();
+ PatchSet.Id patchset2Id = changeOps.change(changeId).newPatchset().create();
+ // Add comments.
+ String rootComment1Uuid =
+ changeOps.change(changeId).patchset(patchset1Id).newComment().resolved().create();
+ String childComment1Uuid =
+ changeOps
+ .change(changeId)
+ .patchset(patchset1Id)
+ .newComment()
+ .parentUuid(rootComment1Uuid)
+ .unresolved()
+ .create();
+ String rootComment2Uuid =
+ changeOps.change(changeId).patchset(patchset1Id).newComment().unresolved().create();
+ changeOps
+ .change(changeId)
+ .patchset(patchset1Id)
+ .newComment()
+ .parentUuid(rootComment2Uuid)
+ .resolved()
+ .create();
+
+ List<CommentInfo> portedComments = flatten(getPortedComments(patchset2Id));
+
+ assertThat(portedComments)
+ .comparingElementsUsing(hasUuid())
+ .containsExactly(rootComment1Uuid, childComment1Uuid);
+ }
+
+ @Test
+ public void unresolvedStateOfLastCommentByDateMattersForBranchedThreads() throws Exception {
+ // Set up change and patchsets.
+ Change.Id changeId = changeOps.newChange().create();
+ PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId();
+ PatchSet.Id patchset2Id = changeOps.change(changeId).newPatchset().create();
+ // Add comments.
+ String rootCommentUuid =
+ changeOps.change(changeId).patchset(patchset1Id).newComment().resolved().create();
+ String childComment1Uuid =
+ changeOps
+ .change(changeId)
+ .patchset(patchset1Id)
+ .newComment()
+ .parentUuid(rootCommentUuid)
+ .resolved()
+ .create();
+ String childComment2Uuid =
+ changeOps
+ .change(changeId)
+ .patchset(patchset1Id)
+ .newComment()
+ .parentUuid(rootCommentUuid)
+ .unresolved()
+ .create();
+
+ List<CommentInfo> portedComments = flatten(getPortedComments(patchset2Id));
+
+ assertThat(portedComments)
+ .comparingElementsUsing(hasUuid())
+ .containsExactly(rootCommentUuid, childComment1Uuid, childComment2Uuid);
+ }
+
+ @Test
+ public void draftCommentsAreNotPortedViaApiForPublishedComments() throws Exception {
+ Account.Id accountId = accountOps.newAccount().create();
+ // Set up change and patchsets.
+ Change.Id changeId = changeOps.newChange().create();
+ PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId();
+ PatchSet.Id patchset2Id = changeOps.change(changeId).newPatchset().create();
+ // Add draft comment.
+ changeOps.change(changeId).patchset(patchset1Id).newDraftComment().author(accountId).create();
+
+ // Draft comments are only visible to their author.
+ requestScopeOps.setApiUser(accountId);
+ List<CommentInfo> portedComments = flatten(getPortedComments(patchset2Id));
+
+ assertThatList(portedComments).isEmpty();
+ }
+
+ @Test
+ public void publishedCommentsOfAllTypesArePorted() throws Exception {
+ // Set up change and patchsets.
+ Change.Id changeId = changeOps.newChange().file("myFile").content("Line 1\nLine 2\n").create();
+ PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId();
+ PatchSet.Id patchset2Id = changeOps.change(changeId).newPatchset().create();
+ // Add comments.
+ String rangeCommentUuid =
+ changeOps
+ .change(changeId)
+ .patchset(patchset1Id)
+ .newComment()
+ .message("Range comment")
+ .fromLine(1)
+ .charOffset(2)
+ .toLine(2)
+ .charOffset(1)
+ .ofFile("myFile")
+ .create();
+ String lineCommentUuid =
+ changeOps
+ .change(changeId)
+ .patchset(patchset1Id)
+ .newComment()
+ .message("Line comment")
+ .onLine(1)
+ .ofFile("myFile")
+ .create();
+ String fileCommentUuid =
+ changeOps
+ .change(changeId)
+ .patchset(patchset1Id)
+ .newComment()
+ .message("File comment")
+ .onFileLevelOf("myFile")
+ .create();
+ String patchsetLevelCommentUuid =
+ changeOps
+ .change(changeId)
+ .patchset(patchset1Id)
+ .newComment()
+ .message("Patchset-level comment")
+ .onPatchsetLevel()
+ .create();
+
+ List<CommentInfo> portedComments = flatten(getPortedComments(patchset2Id));
+
+ assertThat(portedComments)
+ .comparingElementsUsing(hasUuid())
+ .containsExactly(
+ rangeCommentUuid, lineCommentUuid, fileCommentUuid, patchsetLevelCommentUuid);
+ }
+
+ // This is not the desired behavior but at least a current state which doesn't throw exceptions
+ // or has wrong behavior.
+ @Test
+ public void commentOnParentCommitIsIgnored() throws Exception {
+ // Set up change and patchsets.
+ Change.Id changeId = changeOps.newChange().create();
+ PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId();
+ PatchSet.Id patchset2Id = changeOps.change(changeId).newPatchset().create();
+ // Add comments.
+ changeOps.change(changeId).patchset(patchset1Id).newComment().onParentCommit().create();
+
+ List<CommentInfo> portedComments = flatten(getPortedComments(patchset2Id));
+
+ assertThat(portedComments).isEmpty();
+ }
+
+ @Test
+ public void portedCommentHasOriginalUuid() throws Exception {
+ // Set up change and patchsets.
+ Change.Id changeId = changeOps.newChange().create();
+ PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId();
+ PatchSet.Id patchset2Id = changeOps.change(changeId).newPatchset().create();
+ // Add comment.
+ String commentUuid = changeOps.change(changeId).patchset(patchset1Id).newComment().create();
+
+ List<CommentInfo> portedComments = flatten(getPortedComments(patchset2Id));
+
+ assertThatList(portedComments).onlyElement().uuid().isEqualTo(commentUuid);
+ }
+
+ @Test
+ public void portedCommentHasOriginalPatchset() throws Exception {
+ // Set up change and patchsets.
+ Change.Id changeId = changeOps.newChange().create();
+ PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId();
+ PatchSet.Id patchset2Id = changeOps.change(changeId).newPatchset().create();
+ // Add comment.
+ String commentUuid = changeOps.change(changeId).patchset(patchset1Id).newComment().create();
+
+ CommentInfo portedComment = getPortedComment(patchset2Id, commentUuid);
+
+ assertThat(portedComment).patchSet().isEqualTo(patchset1Id.get());
+ }
+
+ @Test
+ public void portedCommentHasOriginalPatchsetCommitId() throws Exception {
+ // Set up change and patchsets.
+ Change.Id changeId = changeOps.newChange().create();
+ TestPatchset patchset1 = changeOps.change(changeId).currentPatchset().get();
+ PatchSet.Id patchset2Id = changeOps.change(changeId).newPatchset().create();
+ // Add comment.
+ String commentUuid =
+ changeOps.change(changeId).patchset(patchset1.patchsetId()).newComment().create();
+
+ CommentInfo portedComment = getPortedComment(patchset2Id, commentUuid);
+
+ assertThat(portedComment).commitId().isEqualTo(patchset1.commitId().name());
+ }
+
+ @Test
+ public void portedCommentHasOriginalMessage() throws Exception {
+ // Set up change and patchsets.
+ Change.Id changeId = changeOps.newChange().create();
+ TestPatchset patchset1 = changeOps.change(changeId).currentPatchset().get();
+ PatchSet.Id patchset2Id = changeOps.change(changeId).newPatchset().create();
+ // Add comment.
+ String commentUuid =
+ changeOps
+ .change(changeId)
+ .patchset(patchset1.patchsetId())
+ .newComment()
+ .message("My comment text")
+ .create();
+
+ CommentInfo portedComment = getPortedComment(patchset2Id, commentUuid);
+
+ assertThat(portedComment).message().isEqualTo("My comment text");
+ }
+
+ @Test
+ public void portedReplyStillRefersToParentComment() throws Exception {
+ // Set up change and patchsets.
+ Change.Id changeId = changeOps.newChange().create();
+ TestPatchset patchset1 = changeOps.change(changeId).currentPatchset().get();
+ PatchSet.Id patchset2Id = changeOps.change(changeId).newPatchset().create();
+ // Add comments.
+ String rootCommentUuid =
+ changeOps.change(changeId).patchset(patchset1.patchsetId()).newComment().create();
+ String childCommentUuid =
+ changeOps
+ .change(changeId)
+ .patchset(patchset1.patchsetId())
+ .newComment()
+ .parentUuid(rootCommentUuid)
+ .create();
+
+ CommentInfo portedComment = getPortedComment(patchset2Id, childCommentUuid);
+
+ assertThat(portedComment).inReplyTo().isEqualTo(rootCommentUuid);
+ }
+
+ @Test
+ public void portedCommentHasOriginalAuthor() throws Exception {
+ // Set up change and patchsets.
+ Account.Id authorId = accountOps.newAccount().create();
+ Change.Id changeId = changeOps.newChange().create();
+ PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId();
+ PatchSet.Id patchset2Id = changeOps.change(changeId).newPatchset().create();
+ // Add comment.
+ String commentUuid =
+ changeOps.change(changeId).patchset(patchset1Id).newComment().author(authorId).create();
+
+ CommentInfo portedComment = getPortedComment(patchset2Id, commentUuid);
+
+ assertThat(portedComment).author().id().isEqualTo(authorId.get());
+ }
+
+ @Test
+ public void portedCommentHasOriginalTag() throws Exception {
+ // Set up change and patchsets.
+ Change.Id changeId = changeOps.newChange().create();
+ TestPatchset patchset1 = changeOps.change(changeId).currentPatchset().get();
+ PatchSet.Id patchset2Id = changeOps.change(changeId).newPatchset().create();
+ // Add comment.
+ String commentUuid =
+ changeOps
+ .change(changeId)
+ .patchset(patchset1.patchsetId())
+ .newComment()
+ .tag("My comment tag")
+ .create();
+
+ CommentInfo portedComment = getPortedComment(patchset2Id, commentUuid);
+
+ assertThat(portedComment).tag().isEqualTo("My comment tag");
+ }
+
+ @Test
+ public void portedCommentHasUpdatedTimestamp() throws Exception {
+ // Set up change and patchsets.
+ Change.Id changeId = changeOps.newChange().create();
+ PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId();
+ PatchSet.Id patchset2Id = changeOps.change(changeId).newPatchset().create();
+ // Add comment.
+ String commentUuid = changeOps.change(changeId).patchset(patchset1Id).newComment().create();
+
+ CommentInfo portedComment = getPortedComment(patchset2Id, commentUuid);
+
+ assertThat(portedComment).updated().isNotNull();
+ }
+
+ @Test
+ public void portedCommentDoesNotHaveChangeMessageId() throws Exception {
+ // Set up change and patchsets.
+ Change.Id changeId = changeOps.newChange().create();
+ PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId();
+ PatchSet.Id patchset2Id = changeOps.change(changeId).newPatchset().create();
+ // Add comment.
+ String commentUuid = changeOps.change(changeId).patchset(patchset1Id).newComment().create();
+
+ CommentInfo portedComment = getPortedComment(patchset2Id, commentUuid);
+
+ // There's currently no use case for linking ported comments to specific change messages. Hence,
+ // there's no reason to fill this field, which requires additional computations.
+ // Besides, we also don't fill this field for the comments requested for a specific patchset.
+ assertThat(portedComment).changeMessageId().isNull();
+ }
+
+ @Test
+ public void pathOfPortedCommentIsOnlyIndicatedInMap() throws Exception {
+ // Set up change and patchsets.
+ Change.Id changeId = changeOps.newChange().file("myFile").content("Line 1").create();
+ PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId();
+ PatchSet.Id patchset2Id = changeOps.change(changeId).newPatchset().create();
+ // Add comment.
+ String commentUuid =
+ changeOps
+ .change(changeId)
+ .patchset(patchset1Id)
+ .newComment()
+ .onFileLevelOf("myFile")
+ .create();
+
+ Map<String, List<CommentInfo>> portedComments = getPortedComments(patchset2Id);
+
+ assertThatMap(portedComments).keys().containsExactly("myFile");
+ CommentInfo portedComment = extractSpecificComment(portedComments, commentUuid);
+ assertThat(portedComment).path().isNull();
+ }
+
+ @Test
+ public void portedRangeCommentCanHandleAddedLines() throws Exception {
+ // Set up change and patchsets.
+ Change.Id changeId =
+ changeOps.newChange().file("myFile").content("Line 1\nLine 2\nLine 3\nLine 4\n").create();
+ PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId();
+ PatchSet.Id patchset2Id =
+ changeOps
+ .change(changeId)
+ .newPatchset()
+ .file("myFile")
+ .content("Line 1\nLine 1.1\nLine 1.2\nLine 2\nLine 3\nLine 4\n")
+ .create();
+ // Add comment.
+ String commentUuid =
+ changeOps
+ .change(changeId)
+ .patchset(patchset1Id)
+ .newComment()
+ .fromLine(3)
+ .charOffset(2)
+ .toLine(4)
+ .charOffset(5)
+ .ofFile("myFile")
+ .create();
+
+ CommentInfo portedComment = getPortedComment(patchset2Id, commentUuid);
+
+ assertThat(portedComment).range().startLine().isEqualTo(5);
+ assertThat(portedComment).range().startCharacter().isEqualTo(2);
+ assertThat(portedComment).range().endLine().isEqualTo(6);
+ assertThat(portedComment).range().endCharacter().isEqualTo(5);
+ }
+
+ @Test
+ public void portedRangeCommentCanHandleDeletedLines() throws Exception {
+ // Set up change and patchsets.
+ Change.Id changeId =
+ changeOps.newChange().file("myFile").content("Line 1\nLine 2\nLine 3\nLine 4\n").create();
+ PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId();
+ PatchSet.Id patchset2Id =
+ changeOps
+ .change(changeId)
+ .newPatchset()
+ .file("myFile")
+ .content("Line 2\nLine 3\nLine 4\n")
+ .create();
+ // Add comment.
+ String commentUuid =
+ changeOps
+ .change(changeId)
+ .patchset(patchset1Id)
+ .newComment()
+ .fromLine(3)
+ .charOffset(2)
+ .toLine(4)
+ .charOffset(5)
+ .ofFile("myFile")
+ .create();
+
+ CommentInfo portedComment = getPortedComment(patchset2Id, commentUuid);
+
+ assertThat(portedComment).range().startLine().isEqualTo(2);
+ assertThat(portedComment).range().startCharacter().isEqualTo(2);
+ assertThat(portedComment).range().endLine().isEqualTo(3);
+ assertThat(portedComment).range().endCharacter().isEqualTo(5);
+ }
+
+ @Test
+ public void portedRangeCommentCanHandlePureRename() throws Exception {
+ // Set up change and patchsets.
+ Change.Id changeId =
+ changeOps.newChange().file("myFile").content("Line 1\nLine 2\nLine 3\nLine 4\n").create();
+ PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId();
+ PatchSet.Id patchset2Id =
+ changeOps.change(changeId).newPatchset().file("myFile").renameTo("newFileName").create();
+ // Add comment.
+ String commentUuid =
+ changeOps
+ .change(changeId)
+ .patchset(patchset1Id)
+ .newComment()
+ .fromLine(3)
+ .charOffset(2)
+ .toLine(4)
+ .charOffset(5)
+ .ofFile("myFile")
+ .create();
+
+ Map<String, List<CommentInfo>> portedComments = getPortedComments(patchset2Id);
+
+ assertThatMap(portedComments).keys().containsExactly("newFileName");
+ CommentInfo portedComment = extractSpecificComment(portedComments, commentUuid);
+ assertThat(portedComment).range().startLine().isEqualTo(3);
+ assertThat(portedComment).range().startCharacter().isEqualTo(2);
+ assertThat(portedComment).range().endLine().isEqualTo(4);
+ assertThat(portedComment).range().endCharacter().isEqualTo(5);
+ }
+
+ @Test
+ public void portedRangeCommentCanHandleRenameWithLineShift() throws Exception {
+ // Set up change and patchsets.
+ Change.Id changeId =
+ changeOps.newChange().file("myFile").content("Line 1\nLine 2\nLine 3\nLine 4\n").create();
+ PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId();
+ PatchSet.Id patchset2Id =
+ changeOps
+ .change(changeId)
+ .newPatchset()
+ .file("myFile")
+ .delete()
+ .file("newFileName")
+ .content("Line 1\nLine 1.1\nLine 1.2\nLine 2\nLine 3\nLine 4\n")
+ .create();
+ // Add comment.
+ String commentUuid =
+ changeOps
+ .change(changeId)
+ .patchset(patchset1Id)
+ .newComment()
+ .fromLine(3)
+ .charOffset(2)
+ .toLine(4)
+ .charOffset(5)
+ .ofFile("myFile")
+ .create();
+
+ Map<String, List<CommentInfo>> portedComments = getPortedComments(patchset2Id);
+
+ assertThatMap(portedComments).keys().containsExactly("newFileName");
+ CommentInfo portedComment = extractSpecificComment(portedComments, commentUuid);
+ assertThat(portedComment).range().startLine().isEqualTo(5);
+ assertThat(portedComment).range().startCharacter().isEqualTo(2);
+ assertThat(portedComment).range().endLine().isEqualTo(6);
+ assertThat(portedComment).range().endCharacter().isEqualTo(5);
+ }
+
+ @Test
+ public void portedRangeCommentAdditionallyAppearsOnCopyAtIndependentPosition() throws Exception {
+ // Set up change and patchsets.
+ Change.Id changeId =
+ changeOps.newChange().file("myFile").content("Line 1\nLine 2\nLine 3\nLine 4\n").create();
+ PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId();
+ // Gerrit currently only identifies a copy if a rename also happens at the same time. Modify the
+ // renamed file slightly different than the copied file so that the end location of the comment
+ // is different. Modify the renamed file less so that Gerrit/Git picks it as the renamed one.
+ PatchSet.Id patchset2Id =
+ changeOps
+ .change(changeId)
+ .newPatchset()
+ .file("myFile")
+ .delete()
+ .file("renamedFiled")
+ .content("Line 1\nLine 1.1\nLine 2\nLine 3\nLine 4\n")
+ .file("copiedFile")
+ .content("Line 1\nLine 1.1\nLine 1.2\nLine 2\nLine 3\nLine 4\n")
+ .create();
+ // Add comment.
+ String commentUuid =
+ changeOps
+ .change(changeId)
+ .patchset(patchset1Id)
+ .newComment()
+ .fromLine(3)
+ .charOffset(2)
+ .toLine(4)
+ .charOffset(5)
+ .ofFile("myFile")
+ .create();
+
+ Map<String, List<CommentInfo>> portedComments = getPortedComments(patchset2Id);
+
+ assertThatMap(portedComments).keys().containsExactly("renamedFiled", "copiedFile");
+ CommentInfo portedCommentOnRename = getOnlyElement(portedComments.get("renamedFiled"));
+ assertThat(portedCommentOnRename).uuid().isEqualTo(commentUuid);
+ assertThat(portedCommentOnRename).range().startLine().isEqualTo(4);
+ assertThat(portedCommentOnRename).range().startCharacter().isEqualTo(2);
+ assertThat(portedCommentOnRename).range().endLine().isEqualTo(5);
+ assertThat(portedCommentOnRename).range().endCharacter().isEqualTo(5);
+ CommentInfo portedCommentOnCopy = getOnlyElement(portedComments.get("copiedFile"));
+ assertThat(portedCommentOnCopy).uuid().isEqualTo(commentUuid);
+ assertThat(portedCommentOnCopy).range().startLine().isEqualTo(5);
+ assertThat(portedCommentOnCopy).range().startCharacter().isEqualTo(2);
+ assertThat(portedCommentOnCopy).range().endLine().isEqualTo(6);
+ assertThat(portedCommentOnCopy).range().endCharacter().isEqualTo(5);
+ }
+
+ @Test
+ public void lineOfPortedRangeCommentFollowsContract() throws Exception {
+ // Set up change and patchsets.
+ Change.Id changeId =
+ changeOps.newChange().file("myFile").content("Line 1\nLine 2\nLine 3\nLine 4\n").create();
+ PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId();
+ PatchSet.Id patchset2Id =
+ changeOps
+ .change(changeId)
+ .newPatchset()
+ .file("myFile")
+ .content("Line 1\nLine 1.1\nLine 1.2\nLine 2\nLine 3\nLine 4\n")
+ .create();
+ // Add comment.
+ String commentUuid =
+ changeOps
+ .change(changeId)
+ .patchset(patchset1Id)
+ .newComment()
+ .fromLine(3)
+ .charOffset(2)
+ .toLine(4)
+ .charOffset(5)
+ .ofFile("myFile")
+ .create();
+
+ CommentInfo portedComment = getPortedComment(patchset2Id, commentUuid);
+
+ // Line is equal to the end line, which is at 6 when ported.
+ assertThat(portedComment).line().isEqualTo(6);
+ }
+
+ @Test
+ // TODO(aliceks): Adjust code to not ignore conflicts and add mapping to file comments.
+ @Ignore
+ public void portedRangeCommentBecomesFileCommentOnConflict() throws Exception {
+ // Set up change and patchsets.
+ Change.Id changeId =
+ changeOps.newChange().file("myFile").content("Line 1\nLine 2\nLine 3\nLine 4\n").create();
+ PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId();
+ PatchSet.Id patchset2Id =
+ changeOps
+ .change(changeId)
+ .newPatchset()
+ .file("myFile")
+ .content("Line 1\nLine two\nLine three\nLine 4\n")
+ .create();
+ // Add comment.
+ String commentUuid =
+ changeOps
+ .change(changeId)
+ .patchset(patchset1Id)
+ .newComment()
+ .fromLine(2)
+ .charOffset(2)
+ .toLine(3)
+ .charOffset(5)
+ .ofFile("myFile")
+ .create();
+
+ Map<String, List<CommentInfo>> portedComments = getPortedComments(patchset2Id);
+
+ assertThatMap(portedComments).keys().containsExactly("myFile");
+ CommentInfo portedComment = extractSpecificComment(portedComments, commentUuid);
+ assertThat(portedComment).range().isNull();
+ assertThat(portedComment).line().isNull();
+ }
+
+ @Test
+ public void portedRangeCommentEndingOnLineJustBeforeModificationCanBePorted() throws Exception {
+ // Set up change and patchsets.
+ Change.Id changeId =
+ changeOps.newChange().file("myFile").content("Line 1\nLine 2\nLine 3\nLine 4\n").create();
+ PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId();
+ PatchSet.Id patchset2Id =
+ changeOps
+ .change(changeId)
+ .newPatchset()
+ .file("myFile")
+ .content("Line 1\nLine 2\nLine three\nLine 4\n")
+ .create();
+ // Add comment.
+ String commentUuid =
+ changeOps
+ .change(changeId)
+ .patchset(patchset1Id)
+ .newComment()
+ .fromLine(1)
+ .charOffset(2)
+ .toLine(2)
+ .charOffset(5)
+ .ofFile("myFile")
+ .create();
+
+ CommentInfo portedComment = getPortedComment(patchset2Id, commentUuid);
+
+ assertThat(portedComment).range().startLine().isEqualTo(1);
+ assertThat(portedComment).range().startCharacter().isEqualTo(2);
+ assertThat(portedComment).range().endLine().isEqualTo(2);
+ assertThat(portedComment).range().endCharacter().isEqualTo(5);
+ }
+
+ @Test
+ public void portedRangeCommentEndingAtStartOfModifiedLineCanBePorted() throws Exception {
+ // Set up change and patchsets.
+ Change.Id changeId =
+ changeOps.newChange().file("myFile").content("Line 1\nLine 2\nLine 3\nLine 4\n").create();
+ PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId();
+ PatchSet.Id patchset2Id =
+ changeOps
+ .change(changeId)
+ .newPatchset()
+ .file("myFile")
+ .content("Line 1\nLine 2\nLine three\nLine 4\n")
+ .create();
+ // Add comment.
+ String commentUuid =
+ changeOps
+ .change(changeId)
+ .patchset(patchset1Id)
+ .newComment()
+ .fromLine(1)
+ .charOffset(2)
+ .toLine(3)
+ .charOffset(0)
+ .ofFile("myFile")
+ .create();
+
+ CommentInfo portedComment = getPortedComment(patchset2Id, commentUuid);
+
+ assertThat(portedComment).range().startLine().isEqualTo(1);
+ assertThat(portedComment).range().startCharacter().isEqualTo(2);
+ assertThat(portedComment).range().endLine().isEqualTo(3);
+ assertThat(portedComment).range().endCharacter().isEqualTo(0);
+ }
+
+ @Test
+ // TODO(aliceks): Adjust code to not ignore conflicts and add mapping to file comments.
+ @Ignore
+ public void portedRangeCommentEndingWithinModifiedLineBecomesFileComment() throws Exception {
+ // Set up change and patchsets.
+ Change.Id changeId =
+ changeOps.newChange().file("myFile").content("Line 1\nLine 2\nLine 3\nLine 4\n").create();
+ PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId();
+ PatchSet.Id patchset2Id =
+ changeOps
+ .change(changeId)
+ .newPatchset()
+ .file("myFile")
+ .content("Line 1\nLine 2\nLine three\nLine 4\n")
+ .create();
+ // Add comment.
+ String commentUuid =
+ changeOps
+ .change(changeId)
+ .patchset(patchset1Id)
+ .newComment()
+ .fromLine(1)
+ .charOffset(2)
+ .toLine(3)
+ .charOffset(4)
+ .ofFile("myFile")
+ .create();
+
+ CommentInfo portedComment = getPortedComment(patchset2Id, commentUuid);
+
+ assertThat(portedComment).range().isNull();
+ assertThat(portedComment).line().isNull();
+ }
+
+ @Test
+ // TODO(aliceks): Adjust code to not ignore conflicts and add mapping to file comments.
+ @Ignore
+ public void portedRangeCommentWithinModifiedLineBecomesFileComment() throws Exception {
+ // Set up change and patchsets.
+ Change.Id changeId =
+ changeOps.newChange().file("myFile").content("Line 1\nLine 2\nLine 3\nLine 4\n").create();
+ PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId();
+ PatchSet.Id patchset2Id =
+ changeOps
+ .change(changeId)
+ .newPatchset()
+ .file("myFile")
+ .content("Line 1\nLine 2\nLine three\nLine 4\n")
+ .create();
+ // Add comment.
+ String commentUuid =
+ changeOps
+ .change(changeId)
+ .patchset(patchset1Id)
+ .newComment()
+ .fromLine(3)
+ .charOffset(2)
+ .toLine(3)
+ .charOffset(5)
+ .ofFile("myFile")
+ .create();
+
+ CommentInfo portedComment = getPortedComment(patchset2Id, commentUuid);
+
+ assertThat(portedComment).range().isNull();
+ assertThat(portedComment).line().isNull();
+ }
+
+ @Test
+ // TODO(aliceks): Adjust code to not ignore conflicts and add mapping to file comments.
+ @Ignore
+ public void portedRangeCommentStartingWithinLastModifiedLineBecomesFileComment()
+ throws Exception {
+ // Set up change and patchsets.
+ Change.Id changeId =
+ changeOps.newChange().file("myFile").content("Line 1\nLine 2\nLine 3\nLine 4\n").create();
+ PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId();
+ PatchSet.Id patchset2Id =
+ changeOps
+ .change(changeId)
+ .newPatchset()
+ .file("myFile")
+ .content("Line one\nLine two\nLine 3\nLine 4\n")
+ .create();
+ // Add comment.
+ String commentUuid =
+ changeOps
+ .change(changeId)
+ .patchset(patchset1Id)
+ .newComment()
+ .fromLine(2)
+ .charOffset(2)
+ .toLine(4)
+ .charOffset(5)
+ .ofFile("myFile")
+ .create();
+
+ CommentInfo portedComment = getPortedComment(patchset2Id, commentUuid);
+
+ assertThat(portedComment).range().isNull();
+ assertThat(portedComment).line().isNull();
+ }
+
+ @Test
+ public void portedRangeCommentStartingOnLineJustAfterModificationCanBePorted() throws Exception {
+ // Set up change and patchsets.
+ Change.Id changeId =
+ changeOps.newChange().file("myFile").content("Line 1\nLine 2\nLine 3\nLine 4\n").create();
+ PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId();
+ PatchSet.Id patchset2Id =
+ changeOps
+ .change(changeId)
+ .newPatchset()
+ .file("myFile")
+ .content("Line 1\nLine two\nLine 3\nLine 4\n")
+ .create();
+ // Add comment.
+ String commentUuid =
+ changeOps
+ .change(changeId)
+ .patchset(patchset1Id)
+ .newComment()
+ .fromLine(3)
+ .charOffset(2)
+ .toLine(4)
+ .charOffset(5)
+ .ofFile("myFile")
+ .create();
+
+ CommentInfo portedComment = getPortedComment(patchset2Id, commentUuid);
+
+ assertThat(portedComment).range().startLine().isEqualTo(3);
+ assertThat(portedComment).range().startCharacter().isEqualTo(2);
+ assertThat(portedComment).range().endLine().isEqualTo(4);
+ assertThat(portedComment).range().endCharacter().isEqualTo(5);
+ }
+
+ @Test
+ // TODO(aliceks): Adjust code to not ignore conflicts and only map start/end conflicts to file
+ // comments.
+ @Ignore
+ public void portedRangeCommentStartingBeforeButEndingAfterModifiedLineCanBePorted()
+ throws Exception {
+ // Set up change and patchsets.
+ Change.Id changeId =
+ changeOps.newChange().file("myFile").content("Line 1\nLine 2\nLine 3\nLine 4\n").create();
+ PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId();
+ PatchSet.Id patchset2Id =
+ changeOps
+ .change(changeId)
+ .newPatchset()
+ .file("myFile")
+ .content("Line 1\nLine 2\nLine three\nLine 4\n")
+ .create();
+ // Add comment.
+ String commentUuid =
+ changeOps
+ .change(changeId)
+ .patchset(patchset1Id)
+ .newComment()
+ .fromLine(2)
+ .charOffset(2)
+ .toLine(4)
+ .charOffset(5)
+ .ofFile("myFile")
+ .create();
+
+ CommentInfo portedComment = getPortedComment(patchset2Id, commentUuid);
+
+ assertThat(portedComment).range().startLine().isEqualTo(3);
+ assertThat(portedComment).range().startCharacter().isEqualTo(2);
+ assertThat(portedComment).range().endLine().isEqualTo(4);
+ assertThat(portedComment).range().endCharacter().isEqualTo(5);
+ }
+
+ @Test
+ // TODO(aliceks): Correct handling of deleted files.
+ @Ignore
+ public void portedRangeCommentBecomesPatchsetLevelCommentOnFileDeletion() throws Exception {
+ // Set up change and patchsets.
+ Change.Id changeId =
+ changeOps.newChange().file("myFile").content("Line 1\nLine 2\nLine 3\nLine 4\n").create();
+ PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId();
+ PatchSet.Id patchset2Id =
+ changeOps.change(changeId).newPatchset().file("myFile").delete().create();
+ // Add comment.
+ String commentUuid =
+ changeOps
+ .change(changeId)
+ .patchset(patchset1Id)
+ .newComment()
+ .fromLine(3)
+ .charOffset(2)
+ .toLine(4)
+ .charOffset(5)
+ .ofFile("myFile")
+ .create();
+
+ Map<String, List<CommentInfo>> portedComments = getPortedComments(patchset2Id);
+ assertThatMap(portedComments).keys().containsExactly(Patch.PATCHSET_LEVEL);
+ CommentInfo portedComment = extractSpecificComment(portedComments, commentUuid);
+ assertThat(portedComment).range().isNull();
+ assertThat(portedComment).line().isNull();
+ }
+
+ @Test
+ public void portedLineCommentCanHandleAddedLines() throws Exception {
+ // Set up change and patchsets.
+ Change.Id changeId =
+ changeOps.newChange().file("myFile").content("Line 1\nLine 2\nLine 3\nLine 4\n").create();
+ PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId();
+ PatchSet.Id patchset2Id =
+ changeOps
+ .change(changeId)
+ .newPatchset()
+ .file("myFile")
+ .content("Line 1\nLine 1.1\nLine 1.2\nLine 2\nLine 3\nLine 4\n")
+ .create();
+ // Add comment.
+ String commentUuid =
+ changeOps
+ .change(changeId)
+ .patchset(patchset1Id)
+ .newComment()
+ .onLine(3)
+ .ofFile("myFile")
+ .create();
+
+ CommentInfo portedComment = getPortedComment(patchset2Id, commentUuid);
+
+ assertThat(portedComment).line().isEqualTo(5);
+ }
+
+ @Test
+ public void portedLineCommentCanHandleDeletedLines() throws Exception {
+ // Set up change and patchsets.
+ Change.Id changeId =
+ changeOps.newChange().file("myFile").content("Line 1\nLine 2\nLine 3\nLine 4\n").create();
+ PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId();
+ PatchSet.Id patchset2Id =
+ changeOps
+ .change(changeId)
+ .newPatchset()
+ .file("myFile")
+ .content("Line 2\nLine 3\nLine 4\n")
+ .create();
+ // Add comment.
+ String commentUuid =
+ changeOps
+ .change(changeId)
+ .patchset(patchset1Id)
+ .newComment()
+ .onLine(3)
+ .ofFile("myFile")
+ .create();
+
+ CommentInfo portedComment = getPortedComment(patchset2Id, commentUuid);
+
+ assertThat(portedComment).line().isEqualTo(2);
+ }
+
+ @Test
+ public void portedLineCommentCanHandlePureRename() throws Exception {
+ // Set up change and patchsets.
+ Change.Id changeId =
+ changeOps.newChange().file("myFile").content("Line 1\nLine 2\nLine 3\nLine 4\n").create();
+ PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId();
+ PatchSet.Id patchset2Id =
+ changeOps.change(changeId).newPatchset().file("myFile").renameTo("newFileName").create();
+ // Add comment.
+ String commentUuid =
+ changeOps
+ .change(changeId)
+ .patchset(patchset1Id)
+ .newComment()
+ .onLine(3)
+ .ofFile("myFile")
+ .create();
+
+ Map<String, List<CommentInfo>> portedComments = getPortedComments(patchset2Id);
+
+ assertThatMap(portedComments).keys().containsExactly("newFileName");
+ CommentInfo portedComment = extractSpecificComment(portedComments, commentUuid);
+ assertThat(portedComment).line().isEqualTo(3);
+ }
+
+ @Test
+ public void portedLineCommentCanHandleRenameWithLineShift() throws Exception {
+ // Set up change and patchsets.
+ Change.Id changeId =
+ changeOps.newChange().file("myFile").content("Line 1\nLine 2\nLine 3\nLine 4\n").create();
+ PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId();
+ changeOps
+ .change(changeId)
+ .newPatchset()
+ .file("myFile")
+ .content("Line 1\nLine 1.1\nLine 1.2\nLine 2\nLine 3\nLine 4\n")
+ .create();
+ PatchSet.Id patchset3Id =
+ changeOps.change(changeId).newPatchset().file("myFile").renameTo("newFileName").create();
+ // Add comment.
+ String commentUuid =
+ changeOps
+ .change(changeId)
+ .patchset(patchset1Id)
+ .newComment()
+ .onLine(3)
+ .ofFile("myFile")
+ .create();
+
+ Map<String, List<CommentInfo>> portedComments = getPortedComments(patchset3Id);
+
+ assertThatMap(portedComments).keys().containsExactly("newFileName");
+ CommentInfo portedComment = extractSpecificComment(portedComments, commentUuid);
+ assertThat(portedComment).line().isEqualTo(5);
+ }
+
+ @Test
+ public void portedLineCommentAdditionallyAppearsOnCopyAtIndependentPosition() throws Exception {
+ // Set up change and patchsets.
+ Change.Id changeId =
+ changeOps.newChange().file("myFile").content("Line 1\nLine 2\nLine 3\nLine 4\n").create();
+ PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId();
+ // Gerrit currently only identifies a copy if a rename also happens at the same time. Modify the
+ // renamed file slightly different than the copied file so that the end location of the comment
+ // is different. Modify the renamed file less so that Gerrit/Git picks it as the renamed one.
+ PatchSet.Id patchset2Id =
+ changeOps
+ .change(changeId)
+ .newPatchset()
+ .file("myFile")
+ .delete()
+ .file("renamedFiled")
+ .content("Line 1\nLine 1.1\nLine 2\nLine 3\nLine 4\n")
+ .file("copiedFile")
+ .content("Line 1\nLine 1.1\nLine 1.2\nLine 2\nLine 3\nLine 4\n")
+ .create();
+ // Add comment.
+ String commentUuid =
+ changeOps
+ .change(changeId)
+ .patchset(patchset1Id)
+ .newComment()
+ .onLine(3)
+ .ofFile("myFile")
+ .create();
+
+ Map<String, List<CommentInfo>> portedComments = getPortedComments(patchset2Id);
+
+ assertThatMap(portedComments).keys().containsExactly("renamedFiled", "copiedFile");
+ CommentInfo portedCommentOnRename = getOnlyElement(portedComments.get("renamedFiled"));
+ assertThat(portedCommentOnRename).uuid().isEqualTo(commentUuid);
+ assertThat(portedCommentOnRename).line().isEqualTo(4);
+ CommentInfo portedCommentOnCopy = getOnlyElement(portedComments.get("copiedFile"));
+ assertThat(portedCommentOnCopy).uuid().isEqualTo(commentUuid);
+ assertThat(portedCommentOnCopy).line().isEqualTo(5);
+ }
+
+ @Test
+ // TODO(aliceks): Adjust code to not ignore conflicts and add mapping to file comments.
+ @Ignore
+ public void portedLineCommentBecomesFileCommentOnConflict() throws Exception {
+ // Set up change and patchsets.
+ Change.Id changeId =
+ changeOps.newChange().file("myFile").content("Line 1\nLine 2\nLine 3\nLine 4\n").create();
+ PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId();
+ PatchSet.Id patchset2Id =
+ changeOps
+ .change(changeId)
+ .newPatchset()
+ .file("myFile")
+ .content("Line 1\nLine two\nLine three\nLine 4\n")
+ .create();
+ // Add comment.
+ String commentUuid =
+ changeOps
+ .change(changeId)
+ .patchset(patchset1Id)
+ .newComment()
+ .onLine(2)
+ .ofFile("myFile")
+ .create();
+
+ Map<String, List<CommentInfo>> portedComments = getPortedComments(patchset2Id);
+
+ assertThatMap(portedComments).keys().containsExactly("myFile");
+ CommentInfo portedComment = extractSpecificComment(portedComments, commentUuid);
+ assertThat(portedComment).line().isNull();
+ }
+
+ @Test
+ public void portedLineCommentOnLineJustBeforeModificationCanBePorted() throws Exception {
+ // Set up change and patchsets.
+ Change.Id changeId =
+ changeOps.newChange().file("myFile").content("Line 1\nLine 2\nLine 3\nLine 4\n").create();
+ PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId();
+ PatchSet.Id patchset2Id =
+ changeOps
+ .change(changeId)
+ .newPatchset()
+ .file("myFile")
+ .content("Line 1\nLine 2\nLine three\nLine 4\n")
+ .create();
+ // Add comment.
+ String commentUuid =
+ changeOps
+ .change(changeId)
+ .patchset(patchset1Id)
+ .newComment()
+ .onLine(2)
+ .ofFile("myFile")
+ .create();
+
+ CommentInfo portedComment = getPortedComment(patchset2Id, commentUuid);
+
+ assertThat(portedComment).line().isEqualTo(2);
+ }
+
+ @Test
+ // TODO(aliceks): Adjust code to not ignore conflicts and add mapping to file comments.
+ @Ignore
+ public void portedLineCommentOnStartLineOfModificationBecomesFileComment() throws Exception {
+ // Set up change and patchsets.
+ Change.Id changeId =
+ changeOps.newChange().file("myFile").content("Line 1\nLine 2\nLine 3\nLine 4\n").create();
+ PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId();
+ PatchSet.Id patchset2Id =
+ changeOps
+ .change(changeId)
+ .newPatchset()
+ .file("myFile")
+ .content("Line 1\nLine 2\nSome completely\ndifferent\ncontent\n")
+ .create();
+ // Add comment.
+ String commentUuid =
+ changeOps
+ .change(changeId)
+ .patchset(patchset1Id)
+ .newComment()
+ .onLine(3)
+ .ofFile("myFile")
+ .create();
+
+ CommentInfo portedComment = getPortedComment(patchset2Id, commentUuid);
+
+ assertThat(portedComment).line().isNull();
+ }
+
+ @Test
+ // TODO(aliceks): Adjust code to not ignore conflicts and add mapping to file comments.
+ @Ignore
+ public void portedLineCommentOnLastLineOfModificationBecomesFileComment() throws Exception {
+ // Set up change and patchsets.
+ Change.Id changeId =
+ changeOps.newChange().file("myFile").content("Line 1\nLine 2\nLine 3\nLine 4\n").create();
+ PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId();
+ PatchSet.Id patchset2Id =
+ changeOps
+ .change(changeId)
+ .newPatchset()
+ .file("myFile")
+ .content("Line 1\nLine 2\nSome completely\ndifferent\ncontent\n")
+ .create();
+ // Add comment.
+ String commentUuid =
+ changeOps
+ .change(changeId)
+ .patchset(patchset1Id)
+ .newComment()
+ .onLine(4)
+ .ofFile("myFile")
+ .create();
+
+ CommentInfo portedComment = getPortedComment(patchset2Id, commentUuid);
+
+ assertThat(portedComment).line().isNull();
+ }
+
+ @Test
+ public void portedLineCommentOnLineJustAfterModificationCanBePorted() throws Exception {
+ // Set up change and patchsets.
+ Change.Id changeId =
+ changeOps.newChange().file("myFile").content("Line 1\nLine 2\nLine 3\nLine 4\n").create();
+ PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId();
+ PatchSet.Id patchset2Id =
+ changeOps
+ .change(changeId)
+ .newPatchset()
+ .file("myFile")
+ .content("Line 1\nLine 2\nLine three\nLine 4\n")
+ .create();
+ // Add comment.
+ String commentUuid =
+ changeOps
+ .change(changeId)
+ .patchset(patchset1Id)
+ .newComment()
+ .onLine(4)
+ .ofFile("myFile")
+ .create();
+
+ CommentInfo portedComment = getPortedComment(patchset2Id, commentUuid);
+
+ assertThat(portedComment).line().isEqualTo(4);
+ }
+
+ @Test
+ // TODO(aliceks): Correct handling of deleted files.
+ @Ignore
+ public void portedLineCommentBecomesPatchsetLevelCommentOnFileDeletion() throws Exception {
+ // Set up change and patchsets.
+ Change.Id changeId =
+ changeOps.newChange().file("myFile").content("Line 1\nLine 2\nLine 3\nLine 4\n").create();
+ PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId();
+ PatchSet.Id patchset2Id =
+ changeOps.change(changeId).newPatchset().file("myFile").delete().create();
+ // Add comment.
+ String commentUuid =
+ changeOps
+ .change(changeId)
+ .patchset(patchset1Id)
+ .newComment()
+ .onLine(3)
+ .ofFile("myFile")
+ .create();
+
+ Map<String, List<CommentInfo>> portedComments = getPortedComments(patchset2Id);
+ assertThatMap(portedComments).keys().containsExactly(Patch.PATCHSET_LEVEL);
+ CommentInfo portedComment = extractSpecificComment(portedComments, commentUuid);
+ assertThat(portedComment).range().isNull();
+ assertThat(portedComment).line().isNull();
+ }
+
+ @Test
+ public void portedFileCommentIsObliviousToAdjustedFileContent() throws Exception {
+ // Set up change and patchsets.
+ Change.Id changeId =
+ changeOps.newChange().file("myFile").content("Line 1\nLine 2\nLine 3\nLine 4\n").create();
+ PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId();
+ PatchSet.Id patchset2Id =
+ changeOps
+ .change(changeId)
+ .newPatchset()
+ .file("myFile")
+ .content("Line 1\nLine 1.1\nLine 1.2\nLine 2\nLine 3\nLine 4\n")
+ .create();
+ // Add comment.
+ String commentUuid =
+ changeOps
+ .change(changeId)
+ .patchset(patchset1Id)
+ .newComment()
+ .onFileLevelOf("myFile")
+ .create();
+
+ CommentInfo portedComment = getPortedComment(patchset2Id, commentUuid);
+
+ assertThat(portedComment).line().isNull();
+ }
+
+ @Test
+ public void portedFileCommentCanHandleRename() throws Exception {
+ // Set up change and patchsets.
+ Change.Id changeId =
+ changeOps.newChange().file("myFile").content("Line 1\nLine 2\nLine 3\nLine 4\n").create();
+ PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId();
+ PatchSet.Id patchset2Id =
+ changeOps.change(changeId).newPatchset().file("myFile").renameTo("newFileName").create();
+ // Add comment.
+ String commentUuid =
+ changeOps
+ .change(changeId)
+ .patchset(patchset1Id)
+ .newComment()
+ .onFileLevelOf("myFile")
+ .create();
+
+ Map<String, List<CommentInfo>> portedComments = getPortedComments(patchset2Id);
+
+ assertThatMap(portedComments).keys().containsExactly("newFileName");
+ CommentInfo portedComment = extractSpecificComment(portedComments, commentUuid);
+ assertThat(portedComment).line().isNull();
+ }
+
+ @Test
+ public void portedFileCommentAdditionallyAppearsOnCopy() throws Exception {
+ // Set up change and patchsets.
+ Change.Id changeId =
+ changeOps.newChange().file("myFile").content("Line 1\nLine 2\nLine 3\nLine 4\n").create();
+ PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId();
+ PatchSet.Id patchset2Id =
+ changeOps
+ .change(changeId)
+ .newPatchset()
+ .file("myFile")
+ .renameTo("renamedFiled")
+ .file("copiedFile")
+ .content("Line 1\nLine 2\nLine 3\nLine 4\n")
+ .create();
+ // Add comment.
+ changeOps.change(changeId).patchset(patchset1Id).newComment().onFileLevelOf("myFile").create();
+
+ Map<String, List<CommentInfo>> portedComments = getPortedComments(patchset2Id);
+
+ assertThatMap(portedComments).keys().containsExactly("renamedFiled", "copiedFile");
+ CommentInfo portedCommentOnCopy = getOnlyElement(portedComments.get("copiedFile"));
+ assertThat(portedCommentOnCopy).line().isNull();
+ }
+
+ @Test
+ // TODO(aliceks): Correct handling of deleted files.
+ @Ignore
+ public void portedFileCommentBecomesPatchsetLevelCommentOnFileDeletion() throws Exception {
+ // Set up change and patchsets.
+ Change.Id changeId =
+ changeOps.newChange().file("myFile").content("Line 1\nLine 2\nLine 3\nLine 4\n").create();
+ PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId();
+ PatchSet.Id patchset2Id =
+ changeOps.change(changeId).newPatchset().file("myFile").delete().create();
+ // Add comment.
+ String commentUuid =
+ changeOps
+ .change(changeId)
+ .patchset(patchset1Id)
+ .newComment()
+ .onFileLevelOf("myFile")
+ .create();
+
+ Map<String, List<CommentInfo>> portedComments = getPortedComments(patchset2Id);
+ assertThatMap(portedComments).keys().containsExactly(Patch.PATCHSET_LEVEL);
+ CommentInfo portedComment = extractSpecificComment(portedComments, commentUuid);
+ assertThat(portedComment).range().isNull();
+ assertThat(portedComment).line().isNull();
+ }
+
+ @Test
+ public void portedPatchsetLevelCommentIsObliviousToAdjustedFileContent() throws Exception {
+ // Set up change and patchsets.
+ Change.Id changeId =
+ changeOps.newChange().file("myFile").content("Line 1\nLine 2\nLine 3\nLine 4\n").create();
+ PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId();
+ PatchSet.Id patchset2Id =
+ changeOps
+ .change(changeId)
+ .newPatchset()
+ .file("myFile")
+ .content("Line 1\nLine 1.1\nLine 1.2\nLine 2\nLine 3\nLine 4\n")
+ .create();
+ // Add comment.
+ changeOps.change(changeId).patchset(patchset1Id).newComment().onPatchsetLevel().create();
+
+ Map<String, List<CommentInfo>> portedComments = getPortedComments(patchset2Id);
+
+ assertThatMap(portedComments).keys().containsExactly(Patch.PATCHSET_LEVEL);
+ }
+
+ @Test
+ public void portedPatchsetLevelCommentIsObliviousToRename() throws Exception {
+ // Set up change and patchsets.
+ Change.Id changeId =
+ changeOps.newChange().file("myFile").content("Line 1\nLine 2\nLine 3\nLine 4\n").create();
+ PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId();
+ PatchSet.Id patchset2Id =
+ changeOps.change(changeId).newPatchset().file("myFile").renameTo("newFileName").create();
+ // Add comment.
+ changeOps.change(changeId).patchset(patchset1Id).newComment().onPatchsetLevel().create();
+
+ Map<String, List<CommentInfo>> portedComments = getPortedComments(patchset2Id);
+
+ assertThatMap(portedComments).keys().containsExactly(Patch.PATCHSET_LEVEL);
+ }
+
+ @Test
+ public void lineCommentOnCommitMessageIsPortedToNewPosition() throws Exception {
+ // Set up change and patchsets.
+ Change.Id changeId =
+ changeOps.newChange().commitMessage("Summary line\n\nText 1\nText 2").create();
+ PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId();
+ PatchSet.Id patchset2Id =
+ changeOps
+ .change(changeId)
+ .newPatchset()
+ .commitMessage("Summary line\n\nText 1\nText 1.1\nText 2")
+ .create();
+ // Add comment.
+ String commentUuid =
+ changeOps
+ .change(changeId)
+ .patchset(patchset1Id)
+ .newComment()
+ // The /COMMIT_MSG file has a header of 6 lines, so the summary line is in line 7.
+ // Place comment on 'Text 2' which is line 10.
+ .onLine(10)
+ .ofFile(Patch.COMMIT_MSG)
+ .create();
+
+ CommentInfo portedComment = getPortedComment(patchset2Id, commentUuid);
+
+ assertThat(portedComment).line().isEqualTo(11);
+ }
+
+ private CommentInfo getPortedComment(PatchSet.Id patchsetId, String commentUuid)
+ throws RestApiException {
+ Map<String, List<CommentInfo>> portedComments = getPortedComments(patchsetId);
+ return extractSpecificComment(portedComments, commentUuid);
+ }
+
+ private Map<String, List<CommentInfo>> getPortedComments(PatchSet.Id patchsetId)
+ throws RestApiException {
+ return gApi.changes()
+ .id(patchsetId.changeId().get())
+ .revision(patchsetId.get())
+ .portedComments();
+ }
+
+ private static CommentInfo extractSpecificComment(
+ Map<String, List<CommentInfo>> portedComments, String commentUuid) {
+ return portedComments.values().stream()
+ .flatMap(Collection::stream)
+ .filter(comment -> comment.id.equals(commentUuid))
+ .collect(onlyElement());
+ }
+
+ /**
+ * Returns all comments in one list. The map keys (= file paths) are simply ignored. The returned
+ * comments won't have the file path attribute set for them as they came from a map with that
+ * attribute as key (= established Gerrit behavior).
+ */
+ private static ImmutableList<CommentInfo> flatten(
+ Map<String, List<CommentInfo>> commentsPerFile) {
+ return commentsPerFile.values().stream()
+ .flatMap(Collection::stream)
+ .collect((toImmutableList()));
+ }
+
+ // Unfortunately, we don't get an absolutely helpful error message when using this correspondence
+ // as CommentInfo doesn't have a toString() implementation. Even if we added it, the string
+ // representation would be quite unwieldy due to the huge number of comment attributes.
+ // Interestingly, using Correspondence#formattingDiffsUsing didn't improve anything.
+ private static Correspondence<CommentInfo, String> hasUuid() {
+ return NullAwareCorrespondence.transforming(comment -> comment.id, "hasUuid");
+ }
+}
diff --git a/javatests/com/google/gerrit/acceptance/server/project/CustomLabelIT.java b/javatests/com/google/gerrit/acceptance/server/project/CustomLabelIT.java
index df5bfca..6aa5878 100644
--- a/javatests/com/google/gerrit/acceptance/server/project/CustomLabelIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/project/CustomLabelIT.java
@@ -39,6 +39,7 @@
import com.google.gerrit.acceptance.NoHttpd;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
+import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
import com.google.gerrit.entities.CachedProjectConfig;
import com.google.gerrit.entities.LabelFunction;
import com.google.gerrit.entities.LabelType;
@@ -63,6 +64,7 @@
@Inject private ProjectOperations projectOperations;
@Inject private ExtensionRegistry extensionRegistry;
+ @Inject private RequestScopeOperations requestScopeOperations;
@Before
public void setUp() throws Exception {
@@ -204,6 +206,28 @@
}
@Test
+ public void customLabelMaxWithBlock_DeletedVoteDoesNotTriggerNegativeBlock() throws Exception {
+ saveLabelConfig(P.toBuilder().setFunction(MAX_WITH_BLOCK));
+ PushOneCommit.Result r = createChange();
+ requestScopeOperations.setApiUser(user.id());
+ revision(r).review(new ReviewInput().label(P_LABEL_NAME, 1));
+ requestScopeOperations.setApiUser(admin.id());
+ revision(r).review(new ReviewInput().label(P_LABEL_NAME, 1));
+
+ LabelInfo labelInfo = getWithLabels(r).labels.get(P_LABEL_NAME);
+ assertThat(labelInfo.all).hasSize(2);
+ assertThat(labelInfo.approved).isNotNull();
+ assertThat(labelInfo.blocking).isNull();
+
+ revision(r).reviewer(admin.email()).deleteVote(P_LABEL_NAME);
+ labelInfo = getWithLabels(r).labels.get(P_LABEL_NAME);
+ assertThat(labelInfo.all).hasSize(2); // 0 vote still delivered
+ assertThat(labelInfo.approved).isNotNull();
+ assertThat(labelInfo.rejected).isNull();
+ assertThat(labelInfo.blocking).isNull(); // label is not blocking the change submission
+ }
+
+ @Test
public void customLabelMaxWithBlock_MaxVoteSubmittable() throws Exception {
saveLabelConfig(
LABEL.toBuilder().setFunction(MAX_WITH_BLOCK), P.toBuilder().setFunction(NO_OP));
diff --git a/plugins/replication b/plugins/replication
index 9a07d19..7ad27c7 160000
--- a/plugins/replication
+++ b/plugins/replication
@@ -1 +1 @@
-Subproject commit 9a07d19326cab1dccbab5696c31f89dc8cb2e8a6
+Subproject commit 7ad27c7ac49731e70dd5ad3691b0b410492792f0
diff --git a/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view.js b/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view.js
index e555583..9affc1b 100644
--- a/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view.js
+++ b/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view.js
@@ -39,7 +39,7 @@
import {htmlTemplate} from './gr-admin-view_html.js';
import {getBaseUrl} from '../../../utils/url-util.js';
import {GerritNav} from '../../core/gr-navigation/gr-navigation.js';
-import {pluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader.js';
+import {getPluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader.js';
import {getAdminLinks} from '../../../utils/admin-nav-util.js';
const INTERNAL_GROUP_REGEX = /^[\da-f]{40}$/;
@@ -112,7 +112,7 @@
reload() {
const promises = [
this.$.restAPI.getAccount(),
- pluginLoader.awaitPluginsLoaded(),
+ getPluginLoader().awaitPluginsLoaded(),
];
return Promise.all(promises).then(result => {
this._account = result[0];
diff --git a/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view_test.js b/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view_test.js
index 9ce4296..d255c68 100644
--- a/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view_test.js
+++ b/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view_test.js
@@ -19,7 +19,7 @@
import './gr-admin-view.js';
import {dom} from '@polymer/polymer/lib/legacy/polymer.dom.js';
import {GerritNav} from '../../core/gr-navigation/gr-navigation.js';
-import {pluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader.js';
+import {getPluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader.js';
import {stubBaseUrl} from '../../../test/test-utils.js';
const basicFixture = fixtureFromElement('gr-admin-view');
@@ -35,7 +35,7 @@
},
});
const pluginsLoaded = Promise.resolve();
- sinon.stub(pluginLoader, 'awaitPluginsLoaded').returns(pluginsLoaded);
+ sinon.stub(getPluginLoader(), 'awaitPluginsLoaded').returns(pluginsLoaded);
pluginsLoaded.then(() => flush(done));
});
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.js b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.js
index e657795..673332b 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.js
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.js
@@ -34,7 +34,7 @@
import {GerritNav} from '../../core/gr-navigation/gr-navigation.js';
import {getDisplayName} from '../../../utils/display-name-util.js';
import {getPluginEndpoints} from '../../shared/gr-js-api-interface/gr-plugin-endpoints.js';
-import {pluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader.js';
+import {getPluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader.js';
import {appContext} from '../../../services/app-context.js';
import {truncatePath} from '../../../utils/path-list-util.js';
import {changeStatuses} from '../../../utils/change-util.js';
@@ -107,10 +107,11 @@
/** @override */
attached() {
super.attached();
- pluginLoader.awaitPluginsLoaded().then(() => {
- this._dynamicCellEndpoints = getPluginEndpoints().getDynamicEndpoints(
- 'change-list-item-cell');
- });
+ getPluginLoader().awaitPluginsLoaded()
+ .then(() => {
+ this._dynamicCellEndpoints = getPluginEndpoints().getDynamicEndpoints(
+ 'change-list-item-cell');
+ });
}
_changeStatuses(change) {
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.js b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.js
index 87d1b78..2230c38 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.js
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.js
@@ -31,7 +31,7 @@
import {KeyboardShortcutMixin, Shortcut} from '../../../mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin.js';
import {GerritNav} from '../../core/gr-navigation/gr-navigation.js';
import {getPluginEndpoints} from '../../shared/gr-js-api-interface/gr-plugin-endpoints.js';
-import {pluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader.js';
+import {getPluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader.js';
import {changeIsOpen} from '../../../utils/change-util.js';
const NUMBER_FIXED_COLUMNS = 3;
@@ -166,10 +166,11 @@
/** @override */
attached() {
super.attached();
- pluginLoader.awaitPluginsLoaded().then(() => {
- this._dynamicHeaderEndpoints = getPluginEndpoints().getDynamicEndpoints(
- 'change-list-header');
- });
+ getPluginLoader().awaitPluginsLoaded()
+ .then(() => {
+ this._dynamicHeaderEndpoints = getPluginEndpoints().
+ getDynamicEndpoints('change-list-header');
+ });
}
/**
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js
index 5f83a14..0285f26 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js
@@ -37,7 +37,7 @@
import {PolymerElement} from '@polymer/polymer/polymer-element.js';
import {htmlTemplate} from './gr-change-actions_html.js';
import {GerritNav} from '../../core/gr-navigation/gr-navigation.js';
-import {pluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader.js';
+import {getPluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader.js';
import {appContext} from '../../../services/app-context.js';
import {
fetchChangeUpdates,
@@ -551,7 +551,8 @@
}
_handleLoadingComplete() {
- pluginLoader.awaitPluginsLoaded().then(() => this._loading = false);
+ getPluginLoader().awaitPluginsLoaded()
+ .then(() => this._loading = false);
}
_sendShowRevisionActions(detail) {
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.js b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.js
index f5d2080..e163479 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.js
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.js
@@ -19,7 +19,7 @@
import './gr-change-actions.js';
import {dom} from '@polymer/polymer/lib/legacy/polymer.dom.js';
import {GerritNav} from '../../core/gr-navigation/gr-navigation.js';
-import {pluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader.js';
+import {getPluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader.js';
import {generateChange} from '../../../test/test-utils.js';
const basicFixture = fixtureFromElement('gr-change-actions');
@@ -85,7 +85,7 @@
getProjectConfig() { return Promise.resolve({}); },
});
- sinon.stub(pluginLoader, 'awaitPluginsLoaded')
+ sinon.stub(getPluginLoader(), 'awaitPluginsLoaded')
.returns(Promise.resolve());
element = basicFixture.instantiate();
@@ -2013,7 +2013,7 @@
getProjectConfig() { return Promise.resolve({}); },
});
- sinon.stub(pluginLoader, 'awaitPluginsLoaded')
+ sinon.stub(getPluginLoader(), 'awaitPluginsLoaded')
.returns(Promise.resolve());
element = basicFixture.instantiate();
diff --git a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata-it_test.js b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata-it_test.js
index 0f9e3b1..e812657 100644
--- a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata-it_test.js
+++ b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata-it_test.js
@@ -19,7 +19,7 @@
import {html} from '@polymer/polymer/lib/utils/html-tag.js';
import './gr-change-metadata.js';
import {resetPlugins} from '../../../test/test-utils.js';
-import {pluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader.js';
+import {getPluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader.js';
import {_testOnly_initGerritPluginApi} from '../../shared/gr-js-api-interface/gr-gerrit.js';
const testHtmlPlugin = document.createElement('dom-module');
@@ -117,10 +117,11 @@
plugin.registerStyleModule('change-metadata', 'my-plugin-style');
}, undefined, 'http://test.com/plugins/style.js');
element = createElement();
- pluginLoader.loadPlugins([]);
- pluginLoader.awaitPluginsLoaded().then(() => {
- flush(done);
- });
+ getPluginLoader().loadPlugins([]);
+ getPluginLoader().awaitPluginsLoaded()
+ .then(() => {
+ flush(done);
+ });
});
teardown(() => {
@@ -143,8 +144,8 @@
plugin = p;
plugin.registerStyleModule('change-metadata', 'my-plugin-style');
}, undefined, 'http://test.com/plugins/style.js');
- sinon.stub(pluginLoader, 'arePluginsLoaded').returns(true);
- pluginLoader.loadPlugins([]);
+ sinon.stub(getPluginLoader(), 'arePluginsLoaded').returns(true);
+ getPluginLoader().loadPlugins([]);
element = createElement();
});
diff --git a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.js b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.js
index ca0205c..b038bf6 100644
--- a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.js
+++ b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.js
@@ -177,6 +177,7 @@
_changeChanged(change) {
this._assignee = change.assignee ? [change.assignee] : [];
+ this._settingTopic = false;
}
_assigneeChanged(assigneeRecord) {
@@ -236,8 +237,12 @@
const lastTopic = this.change.topic;
if (!topic.length) { topic = null; }
this._settingTopic = true;
- this.$.restAPI.setChangeTopic(this.change._number, topic)
+ const topicChangedForChangeNumber = this.change._number;
+ this.$.restAPI.setChangeTopic(topicChangedForChangeNumber, topic)
.then(newTopic => {
+ if (this.change._number !== topicChangedForChangeNumber) {
+ return;
+ }
this._settingTopic = false;
this.set(['change', 'topic'], newTopic);
if (newTopic !== lastTopic) {
diff --git a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_test.js b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_test.js
index 8ec3121..f7a080f 100644
--- a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_test.js
+++ b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_test.js
@@ -19,7 +19,7 @@
import '../../core/gr-router/gr-router.js';
import './gr-change-metadata.js';
import {GerritNav} from '../../core/gr-navigation/gr-navigation.js';
-import {pluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader.js';
+import {getPluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader.js';
import {_testOnly_initGerritPluginApi} from '../../shared/gr-js-api-interface/gr-gerrit.js';
const basicFixture = fixtureFromElement('gr-change-metadata');
@@ -763,7 +763,7 @@
},
'0.1',
'http://some/plugins/url.html');
- pluginLoader.loadPlugins([]);
+ getPluginLoader().loadPlugins([]);
flush(() => {
assert.strictEqual(hookEl.plugin, plugin);
assert.strictEqual(hookEl.change, element.change);
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
index b1b5a9b..7bac47d 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
@@ -56,7 +56,7 @@
import {getComputedStyleValue} from '../../../utils/dom-util.js';
import {GerritNav} from '../../core/gr-navigation/gr-navigation.js';
import {getPluginEndpoints} from '../../shared/gr-js-api-interface/gr-plugin-endpoints.js';
-import {pluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader.js';
+import {getPluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader.js';
import {RevisionInfo} from '../../shared/revision-info/revision-info.js';
import {PrimaryTab, SecondaryTab} from '../../../constants/constants.js';
import {NO_ROBOT_COMMENTS_THREADS_MSG} from '../../../constants/messages.js';
@@ -498,7 +498,7 @@
this._setDiffViewMode();
});
- pluginLoader.awaitPluginsLoaded()
+ getPluginLoader().awaitPluginsLoaded()
.then(() => {
this._dynamicTabHeaderEndpoints =
getPluginEndpoints().getDynamicEndpoints('change-view-tab-header');
@@ -1075,9 +1075,10 @@
this._performPostLoadTasks();
});
- pluginLoader.awaitPluginsLoaded().then(() => {
- this._initActiveTabs(value);
- });
+ getPluginLoader().awaitPluginsLoaded()
+ .then(() => {
+ this._initActiveTabs(value);
+ });
}
_initActiveTabs(params = {}) {
@@ -1108,6 +1109,7 @@
_performPostLoadTasks() {
this._maybeShowReplyDialog();
this._maybeShowRevertDialog();
+ this._maybeShowDownloadDialog();
this._sendShowChangeEvent();
@@ -1179,7 +1181,7 @@
}
_maybeShowRevertDialog() {
- pluginLoader.awaitPluginsLoaded()
+ getPluginLoader().awaitPluginsLoaded()
.then(this._getLoggedIn.bind(this))
.then(loggedIn => {
if (!loggedIn || !this._change ||
@@ -1208,6 +1210,13 @@
});
}
+ _maybeShowDownloadDialog() {
+ if (this.viewState.showDownloadDialog) {
+ this._handleOpenDownloadDialog();
+ this.set('viewState.showDownloadDialog', false);
+ }
+ }
+
_resetFileListViewState() {
this.set('viewState.selectedFileIndex', 0);
this.set('viewState.scrollTop', 0);
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.ts
index 6352238..0f80b608 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.ts
@@ -466,6 +466,7 @@
<gr-button
link=""
class="editCommitMessage"
+ title="Edit commit message"
on-click="_handleEditCommitMessage"
hidden$="[[_hideEditCommitMessage]]"
>Edit</gr-button
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.js b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.js
index 3975be7..3d530b5 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.js
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.js
@@ -25,7 +25,7 @@
import {_testOnly_resetEndpoints} from '../../shared/gr-js-api-interface/gr-plugin-endpoints.js';
import {getComputedStyleValue} from '../../../utils/dom-util.js';
import {GerritNav} from '../../core/gr-navigation/gr-navigation.js';
-import {pluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader.js';
+import {getPluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader.js';
import {_testOnly_initGerritPluginApi} from '../../shared/gr-js-api-interface/gr-gerrit.js';
import {EventType} from '../../plugins/gr-plugin-types.js';
@@ -301,7 +301,7 @@
});
element = fixture.instantiate();
sinon.stub(element.$.actions, 'reload').returns(Promise.resolve());
- pluginLoader.loadPlugins([]);
+ getPluginLoader().loadPlugins([]);
pluginApi.install(
plugin => {
plugin.registerDynamicCustomComponent(
@@ -1642,7 +1642,7 @@
test('revert dialog opened with revert param', done => {
sinon.stub(element.$.restAPI, 'getLoggedIn')
.callsFake(() => Promise.resolve(true));
- sinon.stub(pluginLoader, 'awaitPluginsLoaded')
+ sinon.stub(getPluginLoader(), 'awaitPluginsLoaded')
.callsFake(() => Promise.resolve());
element._patchRange = {
@@ -1671,7 +1671,7 @@
done);
element._maybeShowRevertDialog();
- assert.isTrue(pluginLoader.awaitPluginsLoaded.called);
+ assert.isTrue(getPluginLoader().awaitPluginsLoaded.called);
});
suite('scroll related tests', () => {
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 c7cab1e..9f2ff51 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
@@ -236,10 +236,7 @@
if (schemes.length === 0) {
return;
}
- if (!this._selectedScheme) {
- return;
- }
- if (!schemes.includes(this._selectedScheme)) {
+ if (!this._selectedScheme || !schemes.includes(this._selectedScheme)) {
this._selectedScheme = schemes.sort()[0];
}
}
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 beabeef..1355412 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
@@ -225,12 +225,17 @@
<gr-button
id="expandBtn"
link=""
- title="[[createTitle(Shortcut.EXPAND_ALL_DIFF_CONTEXT,
- ShortcutSection.DIFFS)]]"
+ title="[[createTitle(Shortcut.TOGGLE_ALL_INLINE_DIFFS,
+ ShortcutSection.FILE_LIST)]]"
on-click="_expandAllDiffs"
>Expand All</gr-button
>
- <gr-button id="collapseBtn" link="" on-click="_collapseAllDiffs"
+ <gr-button
+ id="collapseBtn"
+ link=""
+ on-click="_collapseAllDiffs"
+ title="[[createTitle(Shortcut.TOGGLE_ALL_INLINE_DIFFS,
+ ShortcutSection.FILE_LIST)]]"
>Collapse All</gr-button
>
</template>
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
index 06cd18a..aaf0205 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
@@ -38,7 +38,7 @@
import {GrCountStringFormatter} from '../../shared/gr-count-string-formatter/gr-count-string-formatter.js';
import {GerritNav} from '../../core/gr-navigation/gr-navigation.js';
import {getPluginEndpoints} from '../../shared/gr-js-api-interface/gr-plugin-endpoints.js';
-import {pluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader.js';
+import {getPluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader.js';
import {appContext} from '../../../services/app-context.js';
import {SpecialFilePath} from '../../../constants/constants.js';
import {descendedFromClass} from '../../../utils/dom-util.js';
@@ -310,34 +310,35 @@
/** @override */
attached() {
super.attached();
- pluginLoader.awaitPluginsLoaded().then(() => {
- this._dynamicHeaderEndpoints = getPluginEndpoints()
- .getDynamicEndpoints('change-view-file-list-header');
- this._dynamicContentEndpoints = getPluginEndpoints()
- .getDynamicEndpoints('change-view-file-list-content');
- this._dynamicPrependedHeaderEndpoints = getPluginEndpoints()
- .getDynamicEndpoints('change-view-file-list-header-prepend');
- this._dynamicPrependedContentEndpoints = getPluginEndpoints()
- .getDynamicEndpoints('change-view-file-list-content-prepend');
- this._dynamicSummaryEndpoints = getPluginEndpoints()
- .getDynamicEndpoints('change-view-file-list-summary');
+ getPluginLoader().awaitPluginsLoaded()
+ .then(() => {
+ this._dynamicHeaderEndpoints = getPluginEndpoints()
+ .getDynamicEndpoints('change-view-file-list-header');
+ this._dynamicContentEndpoints = getPluginEndpoints()
+ .getDynamicEndpoints('change-view-file-list-content');
+ this._dynamicPrependedHeaderEndpoints = getPluginEndpoints()
+ .getDynamicEndpoints('change-view-file-list-header-prepend');
+ this._dynamicPrependedContentEndpoints = getPluginEndpoints()
+ .getDynamicEndpoints('change-view-file-list-content-prepend');
+ this._dynamicSummaryEndpoints = getPluginEndpoints()
+ .getDynamicEndpoints('change-view-file-list-summary');
- if (this._dynamicHeaderEndpoints.length !==
+ if (this._dynamicHeaderEndpoints.length !==
this._dynamicContentEndpoints.length) {
- console.warn(
- 'Different number of dynamic file-list header and content.');
- }
- if (this._dynamicPrependedHeaderEndpoints.length !==
+ console.warn(
+ 'Different number of dynamic file-list header and content.');
+ }
+ if (this._dynamicPrependedHeaderEndpoints.length !==
this._dynamicPrependedContentEndpoints.length) {
- console.warn(
- 'Different number of dynamic file-list header and content.');
- }
- if (this._dynamicHeaderEndpoints.length !==
+ console.warn(
+ 'Different number of dynamic file-list header and content.');
+ }
+ if (this._dynamicHeaderEndpoints.length !==
this._dynamicSummaryEndpoints.length) {
- console.warn(
- 'Different number of dynamic file-list headers and summary.');
- }
- });
+ console.warn(
+ 'Different number of dynamic file-list headers and summary.');
+ }
+ });
}
/** @override */
diff --git a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list_html.ts b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list_html.ts
index 9c4ef04..d3a72072 100644
--- a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list_html.ts
+++ b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list_html.ts
@@ -68,7 +68,7 @@
role="switch"
on-tap="_onTapShowAllActivityToggle"
></paper-toggle-button>
- <div id="showAllEntriesLabel">
+ <div id="showAllEntriesLabel" aria-hidden="true">
<span>Show all entries</span>
<span class="hiddenEntries" hidden$="[[_showAllActivity]]">
([[_computeHiddenEntriesCount(_combinedMessages)]] hidden)
diff --git a/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list_test.js b/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list_test.js
index 08921b4..7cce9e2 100644
--- a/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list_test.js
+++ b/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list_test.js
@@ -18,7 +18,7 @@
import '../../../test/common-test-setup-karma.js';
import './gr-related-changes-list.js';
import {GerritNav} from '../../core/gr-navigation/gr-navigation.js';
-import {pluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader.js';
+import {getPluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader.js';
import {_testOnly_initGerritPluginApi} from '../../shared/gr-js-api-interface/gr-gerrit.js';
import {resetPlugins} from '../../../test/test-utils.js';
@@ -581,7 +581,7 @@
},
'0.1',
'http://some/plugins/url1.html');
- pluginLoader.loadPlugins([]);
+ getPluginLoader().loadPlugins([]);
flush(() => {
assert.strictEqual(hookEl.plugin, plugin);
assert.strictEqual(hookEl.change, element.change);
@@ -605,7 +605,7 @@
},
'0.1',
'http://some/plugins/url2.html');
- pluginLoader.loadPlugins([]);
+ getPluginLoader().loadPlugins([]);
flush(() => {
// No changes, and plugin without hidden attribute. So it's visible.
element._resultsChanged({}, {}, [], [], []);
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog-it_test.js b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog-it_test.js
index 56612ff..c1272c6 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog-it_test.js
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog-it_test.js
@@ -19,7 +19,7 @@
import {resetPlugins} from '../../../test/test-utils.js';
import './gr-reply-dialog.js';
import {dom} from '@polymer/polymer/lib/legacy/polymer.dom.js';
-import {pluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader.js';
+import {getPluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader.js';
import {_testOnly_initGerritPluginApi} from '../../shared/gr-js-api-interface/gr-gerrit.js';
const basicFixture = fixtureFromElement('gr-reply-dialog');
const pluginApi = _testOnly_initGerritPluginApi();
@@ -122,20 +122,21 @@
}, null, 'http://test.com/plugins/lgtm.js');
element = basicFixture.instantiate();
setupElement(element);
- pluginLoader.loadPlugins([]);
- pluginLoader.awaitPluginsLoaded().then(() => {
- flush(() => {
- const textarea = element.$.textarea.getNativeTextarea();
- textarea.value = 'LGTM';
- textarea.dispatchEvent(new CustomEvent(
- 'input', {bubbles: true, composed: true}));
- const labelScoreRows = dom(element.$.labelScores.root)
- .querySelector('gr-label-score-row[name="Code-Review"]');
- const selectedBtn = dom(labelScoreRows.root)
- .querySelector('gr-button[data-value="+1"].iron-selected');
- assert.isOk(selectedBtn);
- done();
- });
- });
+ getPluginLoader().loadPlugins([]);
+ getPluginLoader().awaitPluginsLoaded()
+ .then(() => {
+ flush(() => {
+ const textarea = element.$.textarea.getNativeTextarea();
+ textarea.value = 'LGTM';
+ textarea.dispatchEvent(new CustomEvent(
+ 'input', {bubbles: true, composed: true}));
+ const labelScoreRows = dom(element.$.labelScores.root)
+ .querySelector('gr-label-score-row[name="Code-Review"]');
+ const selectedBtn = dom(labelScoreRows.root)
+ .querySelector('gr-button[data-value="+1"].iron-selected');
+ assert.isOk(selectedBtn);
+ done();
+ });
+ });
});
});
diff --git a/polygerrit-ui/app/elements/core/gr-main-header/gr-main-header.js b/polygerrit-ui/app/elements/core/gr-main-header/gr-main-header.js
index b36435e..e355e04 100644
--- a/polygerrit-ui/app/elements/core/gr-main-header/gr-main-header.js
+++ b/polygerrit-ui/app/elements/core/gr-main-header/gr-main-header.js
@@ -26,7 +26,7 @@
import {PolymerElement} from '@polymer/polymer/polymer-element.js';
import {htmlTemplate} from './gr-main-header_html.js';
import {getBaseUrl, getDocsBaseUrl} from '../../../utils/url-util.js';
-import {pluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader.js';
+import {getPluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader.js';
import {getAdminLinks} from '../../../utils/admin-nav-util.js';
const DEFAULT_LINKS = [{
@@ -259,7 +259,7 @@
const promises = [
this.$.restAPI.getAccount(),
this.$.restAPI.getTopMenus(),
- pluginLoader.awaitPluginsLoaded(),
+ getPluginLoader().awaitPluginsLoaded(),
];
return Promise.all(promises).then(result => {
diff --git a/polygerrit-ui/app/elements/custom-dark-theme_test.js b/polygerrit-ui/app/elements/custom-dark-theme_test.js
index d4f790f..ad12e14 100644
--- a/polygerrit-ui/app/elements/custom-dark-theme_test.js
+++ b/polygerrit-ui/app/elements/custom-dark-theme_test.js
@@ -19,7 +19,7 @@
import {getComputedStyleValue} from '../utils/dom-util.js';
import './shared/gr-rest-api-interface/gr-rest-api-interface.js';
import './gr-app.js';
-import {pluginLoader} from './shared/gr-js-api-interface/gr-plugin-loader.js';
+import {getPluginLoader} from './shared/gr-js-api-interface/gr-plugin-loader.js';
import {removeTheme} from '../styles/themes/dark-theme.js';
const basicFixture = fixtureFromElement('gr-app');
@@ -30,8 +30,9 @@
window.localStorage.setItem('dark-theme', 'true');
element = basicFixture.instantiate();
- pluginLoader.loadPlugins([]);
- pluginLoader.awaitPluginsLoaded().then(() => flush(done));
+ getPluginLoader().loadPlugins([]);
+ getPluginLoader().awaitPluginsLoaded()
+ .then(() => flush(done));
});
teardown(() => {
diff --git a/polygerrit-ui/app/elements/custom-light-theme_test.js b/polygerrit-ui/app/elements/custom-light-theme_test.js
index 5c0cf28..6d5b61e 100644
--- a/polygerrit-ui/app/elements/custom-light-theme_test.js
+++ b/polygerrit-ui/app/elements/custom-light-theme_test.js
@@ -19,7 +19,7 @@
import {getComputedStyleValue} from '../utils/dom-util.js';
import './shared/gr-rest-api-interface/gr-rest-api-interface.js';
import './gr-app.js';
-import {pluginLoader} from './shared/gr-js-api-interface/gr-plugin-loader.js';
+import {getPluginLoader} from './shared/gr-js-api-interface/gr-plugin-loader.js';
const basicFixture = fixtureFromElement('gr-app');
@@ -36,8 +36,9 @@
_fetchSharedCacheURL() { return Promise.resolve({}); },
});
element = basicFixture.instantiate();
- pluginLoader.loadPlugins([]);
- pluginLoader.awaitPluginsLoaded().then(() => flush(done));
+ getPluginLoader().loadPlugins([]);
+ getPluginLoader().awaitPluginsLoaded()
+ .then(() => flush(done));
});
teardown(() => {
// The app sends requests to server. This can lead to
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
index 189de4a..73ad899 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
@@ -274,6 +274,8 @@
'_handleOpenReplyDialogOrToggleLeftPane',
[Shortcut.TOGGLE_LEFT_PANE]:
'_handleOpenReplyDialogOrToggleLeftPane',
+ [Shortcut.OPEN_DOWNLOAD_DIALOG]:
+ '_handleOpenDownloadDialog',
[Shortcut.UP_TO_CHANGE]: '_handleUpToChange',
[Shortcut.OPEN_DIFF_PREFS]: '_handleCommaKey',
[Shortcut.TOGGLE_DIFF_MODE]: '_handleToggleDiffMode',
@@ -592,6 +594,14 @@
this._navToChangeView();
}
+ _handleOpenDownloadDialog(e) {
+ if (this.shouldSuppressKeyboardShortcut(e)) { return; }
+ if (this.modifierPressed(e)) { return; }
+ this.set('changeViewState.showDownloadDialog', true);
+ e.preventDefault();
+ this._navToChangeView();
+ }
+
_handleUpToChange(e) {
if (this.shouldSuppressKeyboardShortcut(e) ||
this.modifierPressed(e)) { return; }
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 01cfe48..f0b0dcd 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
@@ -50,6 +50,7 @@
kb.bindShortcut(Shortcut.PREV_CHUNK, 'p');
kb.bindShortcut(Shortcut.PREV_COMMENT_THREAD, 'shift+p');
kb.bindShortcut(Shortcut.OPEN_REPLY_DIALOG, 'a');
+ kb.bindShortcut(Shortcut.OPEN_DOWNLOAD_DIALOG, 'd');
kb.bindShortcut(Shortcut.TOGGLE_LEFT_PANE, 'shift+a');
kb.bindShortcut(Shortcut.UP_TO_CHANGE, 'u');
kb.bindShortcut(Shortcut.OPEN_DIFF_PREFS, ',');
@@ -539,6 +540,10 @@
assert(changeNavStub.lastCall.calledWithExactly(element._change, '10',
'5'),
'Should navigate to /c/42/5..10');
+
+ assert.isUndefined(element.changeViewState.showDownloadDialog);
+ MockInteractions.pressAndReleaseKeyOn(element, 68, null, 'd');
+ assert.isTrue(element.changeViewState.showDownloadDialog);
});
test('keyboard shortcuts with old patch number', () => {
diff --git a/polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view.js b/polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view.js
index 058f476..12cd971 100644
--- a/polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view.js
+++ b/polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view.js
@@ -188,8 +188,9 @@
return this.$.restAPI.getFileContent(changeNum, path, patchNum)
.then(res => {
+ const content = (res && res.content) || '';
if (storedContent && storedContent.message &&
- storedContent.message !== res.content) {
+ storedContent.message !== content) {
this.dispatchEvent(new CustomEvent('show-alert', {
detail: {message: RESTORED_MESSAGE},
bubbles: true,
@@ -198,14 +199,14 @@
this._newContent = storedContent.message;
} else {
- this._newContent = res.content || '';
+ this._newContent = content;
}
- this._content = res.content || '';
+ this._content = content;
// A non-ok response may result if the file does not yet exist.
// The `type` field of the response is only valid when the file
// already exists.
- if (res.ok && res.type) {
+ if (res && res.ok && res.type) {
this._type = res.type;
} else {
this._type = '';
diff --git a/polygerrit-ui/app/elements/gr-app-element.js b/polygerrit-ui/app/elements/gr-app-element.js
index 2ee3a66..a304a1b 100644
--- a/polygerrit-ui/app/elements/gr-app-element.js
+++ b/polygerrit-ui/app/elements/gr-app-element.js
@@ -230,6 +230,7 @@
patchRange: null,
selectedFileIndex: 0,
showReplyDialog: false,
+ showDownloadDialog: false,
diffMode: null,
numFilesShown: null,
scrollTop: 0,
diff --git a/polygerrit-ui/app/elements/gr-app-global-var-init.js b/polygerrit-ui/app/elements/gr-app-global-var-init.js
index 7d8539d..30ff123 100644
--- a/polygerrit-ui/app/elements/gr-app-global-var-init.js
+++ b/polygerrit-ui/app/elements/gr-app-global-var-init.js
@@ -60,7 +60,7 @@
import {GrRepoApi} from './plugins/gr-repo-api/gr-repo-api.js';
import {GrSettingsApi} from './plugins/gr-settings-api/gr-settings-api.js';
import {GrStylesApi} from './plugins/gr-styles-api/gr-styles-api.js';
-import {pluginLoader, PluginLoader} from './shared/gr-js-api-interface/gr-plugin-loader.js';
+import {getPluginLoader, PluginLoader} from './shared/gr-js-api-interface/gr-plugin-loader.js';
import {GrPluginActionContext} from './shared/gr-js-api-interface/gr-plugin-action-context.js';
import {getPluginNameFromUrl, getRestAPI, PLUGIN_LOADING_TIMEOUT_MS, PRELOADED_PROTOCOL, send} from './shared/gr-js-api-interface/gr-api-utils.js';
import {getBaseUrl} from '../utils/url-util.js';
@@ -138,7 +138,7 @@
window.Gerrit.getRootElement = getRootElement;
window.Gerrit.Auth = appContext.authService;
- window.Gerrit._pluginLoader = pluginLoader;
+ window.Gerrit._pluginLoader = getPluginLoader();
// TODO: should define as a getter
window.Gerrit._endpoints = getPluginEndpoints();
diff --git a/polygerrit-ui/app/elements/plugins/gr-admin-api/gr-admin-api_test.js b/polygerrit-ui/app/elements/plugins/gr-admin-api/gr-admin-api_test.js
index c3552cb..9a8f75e 100644
--- a/polygerrit-ui/app/elements/plugins/gr-admin-api/gr-admin-api_test.js
+++ b/polygerrit-ui/app/elements/plugins/gr-admin-api/gr-admin-api_test.js
@@ -17,7 +17,7 @@
import '../../../test/common-test-setup-karma.js';
import '../../shared/gr-js-api-interface/gr-js-api-interface.js';
-import {pluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader.js';
+import {getPluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader.js';
import {_testOnly_initGerritPluginApi} from '../../shared/gr-js-api-interface/gr-gerrit.js';
const pluginApi = _testOnly_initGerritPluginApi();
@@ -29,7 +29,7 @@
let plugin;
pluginApi.install(p => { plugin = p; }, '0.1',
'http://test.com/plugins/testplugin/static/test.js');
- pluginLoader.loadPlugins([]);
+ getPluginLoader().loadPlugins([]);
adminApi = plugin.admin();
});
diff --git a/polygerrit-ui/app/elements/plugins/gr-endpoint-decorator/gr-endpoint-decorator.js b/polygerrit-ui/app/elements/plugins/gr-endpoint-decorator/gr-endpoint-decorator.js
index 058f022..72cdc46 100644
--- a/polygerrit-ui/app/elements/plugins/gr-endpoint-decorator/gr-endpoint-decorator.js
+++ b/polygerrit-ui/app/elements/plugins/gr-endpoint-decorator/gr-endpoint-decorator.js
@@ -20,7 +20,7 @@
import {PolymerElement} from '@polymer/polymer/polymer-element.js';
import {htmlTemplate} from './gr-endpoint-decorator_html.js';
import {getPluginEndpoints} from '../../shared/gr-js-api-interface/gr-plugin-endpoints.js';
-import {pluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader.js';
+import {getPluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader.js';
const INIT_PROPERTIES_TIMEOUT_MS = 10000;
@@ -162,7 +162,7 @@
this._endpointCallBack = this._initModule.bind(this);
getPluginEndpoints().onNewEndpoint(this.name, this._endpointCallBack);
if (this.name) {
- pluginLoader.awaitPluginsLoaded()
+ getPluginLoader().awaitPluginsLoaded()
.then(() => getPluginEndpoints().getAndImportPlugins(this.name))
.then(() =>
getPluginEndpoints()
diff --git a/polygerrit-ui/app/elements/plugins/gr-endpoint-decorator/gr-endpoint-decorator_test.js b/polygerrit-ui/app/elements/plugins/gr-endpoint-decorator/gr-endpoint-decorator_test.js
index d7d53cf..c48258d 100644
--- a/polygerrit-ui/app/elements/plugins/gr-endpoint-decorator/gr-endpoint-decorator_test.js
+++ b/polygerrit-ui/app/elements/plugins/gr-endpoint-decorator/gr-endpoint-decorator_test.js
@@ -21,7 +21,7 @@
import '../gr-endpoint-slot/gr-endpoint-slot.js';
import {html} from '@polymer/polymer/lib/utils/html-tag.js';
import {resetPlugins} from '../../../test/test-utils.js';
-import {pluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader.js';
+import {getPluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader.js';
import {getPluginEndpoints} from '../../shared/gr-js-api-interface/gr-plugin-endpoints.js';
import {_testOnly_initGerritPluginApi} from '../../shared/gr-js-api-interface/gr-gerrit.js';
@@ -71,7 +71,7 @@
replacementHook = plugin.registerCustomComponent(
'second', 'other-module', {replace: true});
// Mimic all plugins loaded.
- pluginLoader.loadPlugins([]);
+ getPluginLoader().loadPlugins([]);
flush(done);
});
diff --git a/polygerrit-ui/app/elements/plugins/gr-external-style/gr-external-style.js b/polygerrit-ui/app/elements/plugins/gr-external-style/gr-external-style.js
index 5d29a4c..0e51116 100644
--- a/polygerrit-ui/app/elements/plugins/gr-external-style/gr-external-style.js
+++ b/polygerrit-ui/app/elements/plugins/gr-external-style/gr-external-style.js
@@ -21,7 +21,7 @@
import {PolymerElement} from '@polymer/polymer/polymer-element.js';
import {htmlTemplate} from './gr-external-style_html.js';
import {getPluginEndpoints} from '../../shared/gr-js-api-interface/gr-plugin-endpoints.js';
-import {pluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader.js';
+import {getPluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader.js';
/** @extends PolymerElement */
class GrExternalStyle extends GestureEventListeners(
@@ -75,7 +75,8 @@
/** @override */
ready() {
super.ready();
- pluginLoader.awaitPluginsLoaded().then(() => this._importAndApply());
+ getPluginLoader().awaitPluginsLoaded()
+ .then(() => this._importAndApply());
}
}
diff --git a/polygerrit-ui/app/elements/plugins/gr-external-style/gr-external-style_test.js b/polygerrit-ui/app/elements/plugins/gr-external-style/gr-external-style_test.js
index 6e57994..ad30f48 100644
--- a/polygerrit-ui/app/elements/plugins/gr-external-style/gr-external-style_test.js
+++ b/polygerrit-ui/app/elements/plugins/gr-external-style/gr-external-style_test.js
@@ -18,7 +18,7 @@
import '../../../test/common-test-setup-karma.js';
import {resetPlugins} from '../../../test/test-utils.js';
import './gr-external-style.js';
-import {pluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader.js';
+import {getPluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader.js';
import {getPluginEndpoints} from '../../shared/gr-js-api-interface/gr-plugin-endpoints.js';
import {_testOnly_initGerritPluginApi} from '../../shared/gr-js-api-interface/gr-gerrit.js';
import {html} from '@polymer/polymer/lib/utils/html-tag.js';
@@ -68,7 +68,7 @@
setup(() => {
sinon.stub(getPluginEndpoints(), 'importUrl')
.callsFake( url => Promise.resolve());
- sinon.stub(pluginLoader, 'awaitPluginsLoaded')
+ sinon.stub(getPluginLoader(), 'awaitPluginsLoaded')
.returns(Promise.resolve());
});
diff --git a/polygerrit-ui/app/elements/plugins/gr-plugin-host/gr-plugin-host.js b/polygerrit-ui/app/elements/plugins/gr-plugin-host/gr-plugin-host.js
index 46e37e1..6c10c7c 100644
--- a/polygerrit-ui/app/elements/plugins/gr-plugin-host/gr-plugin-host.js
+++ b/polygerrit-ui/app/elements/plugins/gr-plugin-host/gr-plugin-host.js
@@ -18,7 +18,7 @@
import {GestureEventListeners} from '@polymer/polymer/lib/mixins/gesture-event-listeners.js';
import {LegacyElementMixin} from '@polymer/polymer/lib/legacy/legacy-element-mixin.js';
import {PolymerElement} from '@polymer/polymer/polymer-element.js';
-import {pluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader.js';
+import {getPluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader.js';
/** @extends PolymerElement */
class GrPluginHost extends GestureEventListeners(
@@ -42,7 +42,7 @@
plugins && plugins.js_resource_paths || [], htmlPlugins
);
const shouldLoadTheme = config.default_theme &&
- !pluginLoader.isPluginPreloaded('preloaded:gerrit-theme');
+ !getPluginLoader().isPluginPreloaded('preloaded:gerrit-theme');
const themeToLoad =
shouldLoadTheme ? [config.default_theme] : [];
@@ -57,7 +57,7 @@
pluginOpts[config.default_theme] = {sync: true};
}
- pluginLoader.loadPlugins(pluginsPending, pluginOpts);
+ getPluginLoader().loadPlugins(pluginsPending, pluginOpts);
}
/**
diff --git a/polygerrit-ui/app/elements/plugins/gr-plugin-host/gr-plugin-host_test.js b/polygerrit-ui/app/elements/plugins/gr-plugin-host/gr-plugin-host_test.js
index 83e0a84..7a99dc4 100644
--- a/polygerrit-ui/app/elements/plugins/gr-plugin-host/gr-plugin-host_test.js
+++ b/polygerrit-ui/app/elements/plugins/gr-plugin-host/gr-plugin-host_test.js
@@ -17,7 +17,7 @@
import '../../../test/common-test-setup-karma.js';
import './gr-plugin-host.js';
-import {pluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader.js';
+import {getPluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader.js';
const basicFixture = fixtureFromElement('gr-plugin-host');
@@ -31,21 +31,21 @@
});
test('load plugins should be called', () => {
- sinon.stub(pluginLoader, 'loadPlugins');
+ sinon.stub(getPluginLoader(), 'loadPlugins');
element.config = {
plugin: {
html_resource_paths: ['plugins/foo/bar', 'plugins/baz'],
js_resource_paths: ['plugins/42'],
},
};
- assert.isTrue(pluginLoader.loadPlugins.calledOnce);
- assert.isTrue(pluginLoader.loadPlugins.calledWith([
+ assert.isTrue(getPluginLoader().loadPlugins.calledOnce);
+ assert.isTrue(getPluginLoader().loadPlugins.calledWith([
'plugins/42', 'plugins/foo/bar', 'plugins/baz',
], {}));
});
test('theme plugins should be loaded if enabled', () => {
- sinon.stub(pluginLoader, 'loadPlugins');
+ sinon.stub(getPluginLoader(), 'loadPlugins');
element.config = {
default_theme: 'gerrit-theme.html',
plugin: {
@@ -53,23 +53,23 @@
js_resource_paths: ['plugins/42'],
},
};
- assert.isTrue(pluginLoader.loadPlugins.calledOnce);
- assert.isTrue(pluginLoader.loadPlugins.calledWith([
+ assert.isTrue(getPluginLoader().loadPlugins.calledOnce);
+ assert.isTrue(getPluginLoader().loadPlugins.calledWith([
'gerrit-theme.html', 'plugins/42', 'plugins/foo/bar', 'plugins/baz',
], {'gerrit-theme.html': {sync: true}}));
});
test('skip theme if preloaded', () => {
- sinon.stub(pluginLoader, 'isPluginPreloaded')
+ sinon.stub(getPluginLoader(), 'isPluginPreloaded')
.withArgs('preloaded:gerrit-theme')
.returns(true);
- sinon.stub(pluginLoader, 'loadPlugins');
+ sinon.stub(getPluginLoader(), 'loadPlugins');
element.config = {
default_theme: '/oof',
plugin: {},
};
- assert.isTrue(pluginLoader.loadPlugins.calledOnce);
- assert.isTrue(pluginLoader.loadPlugins.calledWith([], {}));
+ assert.isTrue(getPluginLoader().loadPlugins.calledOnce);
+ assert.isTrue(getPluginLoader().loadPlugins.calledWith([], {}));
});
});
diff --git a/polygerrit-ui/app/elements/plugins/gr-repo-api/gr-repo-api_test.js b/polygerrit-ui/app/elements/plugins/gr-repo-api/gr-repo-api_test.js
index c7f1b06..9e24fda 100644
--- a/polygerrit-ui/app/elements/plugins/gr-repo-api/gr-repo-api_test.js
+++ b/polygerrit-ui/app/elements/plugins/gr-repo-api/gr-repo-api_test.js
@@ -17,7 +17,7 @@
import '../../../test/common-test-setup-karma.js';
import '../gr-endpoint-decorator/gr-endpoint-decorator.js';
-import {pluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader.js';
+import {getPluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader.js';
import {_testOnly_initGerritPluginApi} from '../../shared/gr-js-api-interface/gr-gerrit.js';
import {html} from '@polymer/polymer/lib/utils/html-tag.js';
@@ -35,7 +35,7 @@
let plugin;
pluginApi.install(p => { plugin = p; }, '0.1',
'http://test.com/plugins/testplugin/static/test.js');
- pluginLoader.loadPlugins([]);
+ getPluginLoader().loadPlugins([]);
repoApi = plugin.project();
});
diff --git a/polygerrit-ui/app/elements/plugins/gr-settings-api/gr-settings-api_test.js b/polygerrit-ui/app/elements/plugins/gr-settings-api/gr-settings-api_test.js
index 82d58fe..893f3e4 100644
--- a/polygerrit-ui/app/elements/plugins/gr-settings-api/gr-settings-api_test.js
+++ b/polygerrit-ui/app/elements/plugins/gr-settings-api/gr-settings-api_test.js
@@ -17,7 +17,7 @@
import '../../../test/common-test-setup-karma.js';
import '../gr-endpoint-decorator/gr-endpoint-decorator.js';
-import {pluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader.js';
+import {getPluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader.js';
import {_testOnly_initGerritPluginApi} from '../../shared/gr-js-api-interface/gr-gerrit.js';
import {html} from '@polymer/polymer/lib/utils/html-tag.js';
@@ -37,7 +37,7 @@
let plugin;
pluginApi.install(p => { plugin = p; }, '0.1',
'http://test.com/plugins/testplugin/static/test.js');
- pluginLoader.loadPlugins([]);
+ getPluginLoader().loadPlugins([]);
settingsApi = plugin.settings();
});
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 94d8b82..c41b551 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
@@ -18,7 +18,7 @@
import '../../../test/common-test-setup-karma.js';
import '../../shared/gr-js-api-interface/gr-js-api-interface.js';
import {PolymerElement} from '@polymer/polymer/polymer-element.js';
-import {pluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader.js';
+import {getPluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader.js';
import {_testOnly_initGerritPluginApi} from '../../shared/gr-js-api-interface/gr-gerrit.js';
import {html} from '@polymer/polymer/lib/utils/html-tag.js';
@@ -41,7 +41,7 @@
let plugin;
pluginApi.install(p => { plugin = p; }, '0.1',
'http://test.com/plugins/testplugin/static/test.js');
- pluginLoader.loadPlugins([]);
+ getPluginLoader().loadPlugins([]);
stylesApi = plugin.styles();
});
@@ -68,7 +68,7 @@
let plugin;
pluginApi.install(p => { plugin = p; }, '0.1',
'http://test.com/plugins/testplugin/static/test.js');
- pluginLoader.loadPlugins([]);
+ getPluginLoader().loadPlugins([]);
stylesApi = plugin.styles();
displayInlineStyle = stylesApi.css('display: inline');
displayNoneStyle = stylesApi.css('display: none');
diff --git a/polygerrit-ui/app/elements/plugins/gr-theme-api/gr-theme-api_test.js b/polygerrit-ui/app/elements/plugins/gr-theme-api/gr-theme-api_test.js
index 8a70303..787ab0b 100644
--- a/polygerrit-ui/app/elements/plugins/gr-theme-api/gr-theme-api_test.js
+++ b/polygerrit-ui/app/elements/plugins/gr-theme-api/gr-theme-api_test.js
@@ -17,7 +17,7 @@
import '../../../test/common-test-setup-karma.js';
import '../gr-endpoint-decorator/gr-endpoint-decorator.js';
-import {pluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader.js';
+import {getPluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader.js';
import {_testOnly_initGerritPluginApi} from '../../shared/gr-js-api-interface/gr-gerrit.js';
import {html} from '@polymer/polymer/lib/utils/html-tag.js';
@@ -56,7 +56,7 @@
/** @override */
ready() { customHeader = this; },
});
- pluginLoader.loadPlugins([]);
+ getPluginLoader().loadPlugins([]);
});
test('sets logo and title', done => {
diff --git a/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label.js b/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label.js
index aa2edbf..9122a87 100644
--- a/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label.js
+++ b/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label.js
@@ -136,7 +136,11 @@
}
_hasAttention(config, highlight, account, change, force) {
- if (force) return true;
+ return force || this._hasUnforcedAttention(config, highlight, account,
+ change);
+ }
+
+ _hasUnforcedAttention(config, highlight, account, change) {
return this._isAttentionSetEnabled(config, highlight, account, change)
&& change.attention_set
&& change.attention_set.hasOwnProperty(account._account_id);
@@ -196,6 +200,12 @@
targetIsSelf: targetId === selfId,
};
}
+
+ _computeAttentionIconTtitle(config, highlight, account, change) {
+ return this._hasUnforcedAttention(config, highlight, account, change)
+ ? 'Click to remove the user from the attention set'
+ : 'Disabled. Use "Modify" to make changes.';
+ }
}
customElements.define(GrAccountLabel.is, GrAccountLabel);
diff --git a/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label_html.ts b/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label_html.ts
index a721734..e3c0e14 100644
--- a/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label_html.ts
+++ b/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label_html.ts
@@ -106,8 +106,9 @@
link=""
aria-label="Remove user from attention set"
on-click="_handleRemoveAttentionClick"
- has-tooltip=""
- title="Click to remove the user from the attention set"
+ disabled="[[!_hasUnforcedAttention(_config, highlightAttention, account, change)]]"
+ has-tooltip="[[_hasUnforcedAttention(_config, highlightAttention, account, change)]]"
+ title="[[_computeAttentionIconTtitle(_config, highlightAttention, account, change)]]"
><iron-icon class="attention" icon="gr-icons:attention"></iron-icon>
</gr-button>
</template>
diff --git a/polygerrit-ui/app/elements/shared/gr-avatar/gr-avatar.js b/polygerrit-ui/app/elements/shared/gr-avatar/gr-avatar.js
deleted file mode 100644
index 0d30179..0000000
--- a/polygerrit-ui/app/elements/shared/gr-avatar/gr-avatar.js
+++ /dev/null
@@ -1,107 +0,0 @@
-/**
- * @license
- * Copyright (C) 2015 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-import '../../../styles/shared-styles.js';
-import '../gr-js-api-interface/gr-js-api-interface.js';
-import '../gr-rest-api-interface/gr-rest-api-interface.js';
-import {GestureEventListeners} from '@polymer/polymer/lib/mixins/gesture-event-listeners.js';
-import {LegacyElementMixin} from '@polymer/polymer/lib/legacy/legacy-element-mixin.js';
-import {PolymerElement} from '@polymer/polymer/polymer-element.js';
-import {htmlTemplate} from './gr-avatar_html.js';
-import {getBaseUrl} from '../../../utils/url-util.js';
-import {pluginLoader} from '../gr-js-api-interface/gr-plugin-loader.js';
-
-/**
- * @extends PolymerElement
- */
-class GrAvatar extends GestureEventListeners(
- LegacyElementMixin(
- PolymerElement)) {
- static get template() { return htmlTemplate; }
-
- static get is() { return 'gr-avatar'; }
-
- static get properties() {
- return {
- account: {
- type: Object,
- observer: '_accountChanged',
- },
- imageSize: {
- type: Number,
- value: 16,
- },
- _hasAvatars: {
- type: Boolean,
- value: false,
- },
- };
- }
-
- /** @override */
- attached() {
- super.attached();
- Promise.all([
- this._getConfig(),
- pluginLoader.awaitPluginsLoaded(),
- ]).then(([cfg]) => {
- this._hasAvatars = !!(cfg && cfg.plugin && cfg.plugin.has_avatars);
-
- this._updateAvatarURL();
- });
- }
-
- _getConfig() {
- return this.$.restAPI.getConfig();
- }
-
- _accountChanged(account) {
- this._updateAvatarURL();
- }
-
- _updateAvatarURL() {
- if (!this._hasAvatars || !this.account) {
- this.hidden = true;
- return;
- }
- this.hidden = false;
-
- const url = this._buildAvatarURL(this.account);
- if (url) {
- this.style.backgroundImage = 'url("' + url + '")';
- }
- }
-
- _getAccounts(account) {
- return account._account_id || account.email || account.username ||
- account.name;
- }
-
- _buildAvatarURL(account) {
- if (!account) { return ''; }
- const avatars = account.avatars || [];
- for (let i = 0; i < avatars.length; i++) {
- if (avatars[i].height === this.imageSize) {
- return avatars[i].url;
- }
- }
- return getBaseUrl() + '/accounts/' +
- encodeURIComponent(this._getAccounts(account)) +
- '/avatar?s=' + this.imageSize;
- }
-}
-
-customElements.define(GrAvatar.is, GrAvatar);
diff --git a/polygerrit-ui/app/elements/shared/gr-avatar/gr-avatar.ts b/polygerrit-ui/app/elements/shared/gr-avatar/gr-avatar.ts
new file mode 100644
index 0000000..45bac9f
--- /dev/null
+++ b/polygerrit-ui/app/elements/shared/gr-avatar/gr-avatar.ts
@@ -0,0 +1,118 @@
+/**
+ * @license
+ * Copyright (C) 2015 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+import '../../../styles/shared-styles';
+import '../gr-js-api-interface/gr-js-api-interface';
+import '../gr-rest-api-interface/gr-rest-api-interface';
+import {GestureEventListeners} from '@polymer/polymer/lib/mixins/gesture-event-listeners';
+import {LegacyElementMixin} from '@polymer/polymer/lib/legacy/legacy-element-mixin';
+import {PolymerElement} from '@polymer/polymer/polymer-element';
+import {htmlTemplate} from './gr-avatar_html';
+import {getBaseUrl} from '../../../utils/url-util';
+import {getPluginLoader} from '../gr-js-api-interface/gr-plugin-loader';
+import {customElement, property} from '@polymer/decorators';
+import {AccountInfo} from '../../../types/common';
+import {RestApiService} from '../../../services/services/gr-rest-api/gr-rest-api';
+
+export interface GrAvatar {
+ $: {
+ restAPI: RestApiService & Element;
+ };
+}
+@customElement('gr-avatar')
+export class GrAvatar extends GestureEventListeners(
+ LegacyElementMixin(PolymerElement)
+) {
+ static get template() {
+ return htmlTemplate;
+ }
+
+ @property({type: Object, observer: '_accountChanged'})
+ account?: AccountInfo;
+
+ @property({type: Number})
+ imageSize = 16;
+
+ @property({type: Boolean})
+ _hasAvatars = false;
+
+ /** @override */
+ attached() {
+ super.attached();
+ Promise.all([
+ this._getConfig(),
+ getPluginLoader().awaitPluginsLoaded(),
+ ]).then(([cfg]) => {
+ this._hasAvatars = !!(cfg && cfg.plugin && cfg.plugin.has_avatars);
+
+ this._updateAvatarURL();
+ });
+ }
+
+ _getConfig() {
+ return this.$.restAPI.getConfig();
+ }
+
+ _accountChanged() {
+ this._updateAvatarURL();
+ }
+
+ _updateAvatarURL() {
+ if (!this._hasAvatars || !this.account) {
+ this.hidden = true;
+ return;
+ }
+ this.hidden = false;
+
+ const url = this._buildAvatarURL(this.account);
+ if (url) {
+ this.style.backgroundImage = 'url("' + url + '")';
+ }
+ }
+
+ _getAccounts(account: AccountInfo) {
+ return (
+ account._account_id || account.email || account.username || account.name
+ );
+ }
+
+ _buildAvatarURL(account: AccountInfo) {
+ if (!account) {
+ return '';
+ }
+ const avatars = account.avatars || [];
+ for (let i = 0; i < avatars.length; i++) {
+ if (avatars[i].height === this.imageSize) {
+ return avatars[i].url;
+ }
+ }
+ const accountID = this._getAccounts(account);
+ if (!accountID) {
+ return '';
+ }
+ return (
+ `${getBaseUrl()}/accounts/` +
+ encodeURIComponent(`${this._getAccounts(account)}`) +
+ `/avatar?s=${this.imageSize}`
+ );
+ }
+}
+
+declare global {
+ interface HTMLElementTagNameMap {
+ 'gr-avatar': GrAvatar;
+ }
+}
diff --git a/polygerrit-ui/app/elements/shared/gr-avatar/gr-avatar_test.js b/polygerrit-ui/app/elements/shared/gr-avatar/gr-avatar_test.js
index 5e43f90..261d59c 100644
--- a/polygerrit-ui/app/elements/shared/gr-avatar/gr-avatar_test.js
+++ b/polygerrit-ui/app/elements/shared/gr-avatar/gr-avatar_test.js
@@ -17,7 +17,7 @@
import '../../../test/common-test-setup-karma.js';
import './gr-avatar.js';
-import {pluginLoader} from '../gr-js-api-interface/gr-plugin-loader.js';
+import {getPluginLoader} from '../gr-js-api-interface/gr-plugin-loader.js';
const basicFixture = fixtureFromElement('gr-avatar');
@@ -101,11 +101,11 @@
assert.strictEqual(element.style.backgroundImage, '');
// Emulate plugins loaded.
- pluginLoader.loadPlugins([]);
+ getPluginLoader().loadPlugins([]);
return Promise.all([
element.$.restAPI.getConfig(),
- pluginLoader.awaitPluginsLoaded(),
+ getPluginLoader().awaitPluginsLoaded(),
]).then(() => {
assert.isFalse(element.hasAttribute('hidden'));
@@ -131,11 +131,11 @@
assert.isFalse(element.hasAttribute('hidden'));
// Emulate plugins loaded.
- pluginLoader.loadPlugins([]);
+ getPluginLoader().loadPlugins([]);
return Promise.all([
element.$.restAPI.getConfig(),
- pluginLoader.awaitPluginsLoaded(),
+ getPluginLoader().awaitPluginsLoaded(),
]).then(() => {
assert.isTrue(element.hasAttribute('hidden'));
@@ -164,11 +164,11 @@
_account_id: 123,
};
// Emulate plugins loaded.
- pluginLoader.loadPlugins([]);
+ getPluginLoader().loadPlugins([]);
return Promise.all([
element.$.restAPI.getConfig(),
- pluginLoader.awaitPluginsLoaded(),
+ getPluginLoader().awaitPluginsLoaded(),
]).then(() => {
assert.isTrue(element.hasAttribute('hidden'));
});
diff --git a/polygerrit-ui/app/elements/shared/gr-change-star/gr-change-star.ts b/polygerrit-ui/app/elements/shared/gr-change-star/gr-change-star.ts
index 7474f7b..0aa077e 100644
--- a/polygerrit-ui/app/elements/shared/gr-change-star/gr-change-star.ts
+++ b/polygerrit-ui/app/elements/shared/gr-change-star/gr-change-star.ts
@@ -22,6 +22,7 @@
import {htmlTemplate} from './gr-change-star_html';
import {customElement, property} from '@polymer/decorators';
import {ChangeInfo} from '../../../types/common';
+import {KeyboardShortcutMixin} from '../../../mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin';
declare global {
interface HTMLElementTagNameMap {
@@ -30,8 +31,8 @@
}
@customElement('gr-change-star')
-export class GrChangeStar extends GestureEventListeners(
- LegacyElementMixin(PolymerElement)
+export class GrChangeStar extends KeyboardShortcutMixin(
+ GestureEventListeners(LegacyElementMixin(PolymerElement))
) {
static get template() {
return htmlTemplate;
diff --git a/polygerrit-ui/app/elements/shared/gr-change-star/gr-change-star_html.ts b/polygerrit-ui/app/elements/shared/gr-change-star/gr-change-star_html.ts
index c7930c0..6c0f6f7 100644
--- a/polygerrit-ui/app/elements/shared/gr-change-star/gr-change-star_html.ts
+++ b/polygerrit-ui/app/elements/shared/gr-change-star/gr-change-star_html.ts
@@ -39,7 +39,9 @@
</style>
<button
role="checkbox"
- aria-label="[[_computeAriaLabel(change.starred)]]]"
+ title="[[createTitle(Shortcut.TOGGLE_CHANGE_STAR,
+ ShortcutSection.ACTIONS)]]"
+ aria-label="[[_computeAriaLabel(change.starred)]]"
on-click="toggleStar"
>
<iron-icon
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-change-actions-js-api_test.js b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-change-actions-js-api_test.js
index 1ee81ac..1ffdf7a 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-change-actions-js-api_test.js
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-change-actions-js-api_test.js
@@ -18,7 +18,7 @@
import '../../../test/common-test-setup-karma.js';
import '../../change/gr-change-actions/gr-change-actions.js';
import {resetPlugins} from '../../../test/test-utils.js';
-import {pluginLoader} from './gr-plugin-loader.js';
+import {getPluginLoader} from './gr-plugin-loader.js';
import {_testOnly_initGerritPluginApi} from './gr-gerrit.js';
const basicFixture = fixtureFromElement('gr-change-actions');
@@ -44,7 +44,7 @@
pluginApi.install(p => { plugin = p; }, '0.1',
'http://test.com/plugins/testplugin/static/test.js');
// Mimic all plugins loaded.
- pluginLoader.loadPlugins([]);
+ getPluginLoader().loadPlugins([]);
changeActions = plugin.changeActions();
element = basicFixture.instantiate();
});
@@ -72,7 +72,7 @@
'http://test.com/plugins/testplugin/static/test.js');
changeActions = plugin.changeActions();
// Mimic all plugins loaded.
- pluginLoader.loadPlugins([]);
+ getPluginLoader().loadPlugins([]);
});
teardown(() => {
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-gerrit.ts b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-gerrit.ts
index e5e6069..7f0f5b7 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-gerrit.ts
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-gerrit.ts
@@ -19,7 +19,7 @@
* This defines the Gerrit instance. All methods directly attached to Gerrit
* should be defined or linked here.
*/
-import {pluginLoader, PluginOptionMap} from './gr-plugin-loader';
+import {getPluginLoader, PluginOptionMap} from './gr-plugin-loader';
import {getRestAPI, send} from './gr-api-utils';
import {appContext} from '../../../services/app-context';
import {PluginApi} from '../../plugins/gr-plugin-types';
@@ -81,7 +81,7 @@
// Preloaded plugins should be installed after Gerrit.install() is set,
// since plugin preloader substitutes Gerrit.install() temporarily.
// (Gerrit.install() is set in initGerritPluginsMethods)
- pluginLoader.installPreloadedPlugins();
+ getPluginLoader().installPreloadedPlugins();
}
export function _testOnly_initGerritPluginApi(): GerritGlobal {
@@ -136,7 +136,7 @@
};
globalGerritObj.install = (callback, opt_version, opt_src) => {
- pluginLoader.install(callback, opt_version, opt_src);
+ getPluginLoader().install(callback, opt_version, opt_src);
};
globalGerritObj.getLoggedIn = () => {
@@ -181,29 +181,29 @@
};
globalGerritObj.awaitPluginsLoaded = () => {
- return pluginLoader.awaitPluginsLoaded();
+ return getPluginLoader().awaitPluginsLoaded();
};
// TODO(taoalpha): consider removing these proxy methods
- // and using pluginLoader directly
+ // and using getPluginLoader() directly
globalGerritObj._loadPlugins = (plugins, opt_option) => {
- pluginLoader.loadPlugins(plugins, opt_option);
+ getPluginLoader().loadPlugins(plugins, opt_option);
};
globalGerritObj._arePluginsLoaded = () => {
- return pluginLoader.arePluginsLoaded();
+ return getPluginLoader().arePluginsLoaded();
};
globalGerritObj._isPluginPreloaded = url => {
- return pluginLoader.isPluginPreloaded(url);
+ return getPluginLoader().isPluginPreloaded(url);
};
globalGerritObj._isPluginEnabled = pathOrUrl => {
- return pluginLoader.isPluginEnabled(pathOrUrl);
+ return getPluginLoader().isPluginEnabled(pathOrUrl);
};
globalGerritObj._isPluginLoaded = pathOrUrl => {
- return pluginLoader.isPluginLoaded(pathOrUrl);
+ return getPluginLoader().isPluginLoaded(pathOrUrl);
};
const eventEmitter = appContext.eventEmitter;
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-gerrit_test.js b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-gerrit_test.js
index e5b32f7..39e3d1b 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-gerrit_test.js
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-gerrit_test.js
@@ -17,7 +17,7 @@
import '../../../test/common-test-setup-karma.js';
import './gr-js-api-interface.js';
-import {pluginLoader} from './gr-plugin-loader.js';
+import {getPluginLoader} from './gr-plugin-loader.js';
import {resetPlugins} from '../../../test/test-utils.js';
import {_testOnly_initGerritPluginApi} from './gr-gerrit.js';
@@ -52,10 +52,10 @@
});
suite('proxy methods', () => {
- test('Gerrit._isPluginEnabled proxy to pluginLoader', () => {
+ test('Gerrit._isPluginEnabled proxy to getPluginLoader()', () => {
const stubFn = sinon.stub();
sinon.stub(
- pluginLoader,
+ getPluginLoader(),
'isPluginEnabled')
.callsFake((...args) => stubFn(...args)
);
@@ -63,20 +63,20 @@
assert.isTrue(stubFn.calledWith('test_plugin'));
});
- test('Gerrit._isPluginLoaded proxy to pluginLoader', () => {
+ test('Gerrit._isPluginLoaded proxy to getPluginLoader()', () => {
const stubFn = sinon.stub();
sinon.stub(
- pluginLoader,
+ getPluginLoader(),
'isPluginLoaded')
.callsFake((...args) => stubFn(...args));
pluginApi._isPluginLoaded('test_plugin');
assert.isTrue(stubFn.calledWith('test_plugin'));
});
- test('Gerrit._isPluginPreloaded proxy to pluginLoader', () => {
+ test('Gerrit._isPluginPreloaded proxy to getPluginLoader()', () => {
const stubFn = sinon.stub();
sinon.stub(
- pluginLoader,
+ getPluginLoader(),
'isPluginPreloaded')
.callsFake((...args) => stubFn(...args));
pluginApi._isPluginPreloaded('test_plugin');
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface-element.ts b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface-element.ts
index 4973594..d2bb8fc 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface-element.ts
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface-element.ts
@@ -17,7 +17,7 @@
import {GestureEventListeners} from '@polymer/polymer/lib/mixins/gesture-event-listeners';
import {LegacyElementMixin} from '@polymer/polymer/lib/legacy/legacy-element-mixin';
import {PolymerElement} from '@polymer/polymer/polymer-element';
-import {pluginLoader} from './gr-plugin-loader';
+import {getPluginLoader} from './gr-plugin-loader';
import {patchNumEquals} from '../../../utils/patch-set-util';
import {customElement} from '@polymer/decorators';
import {ChangeInfo, RevisionInfo} from '../../../types/common';
@@ -42,31 +42,36 @@
extends GestureEventListeners(LegacyElementMixin(PolymerElement))
implements JsApiService {
handleEvent(type: EventType, detail: any) {
- pluginLoader.awaitPluginsLoaded().then(() => {
- switch (type) {
- case EventType.HISTORY:
- this._handleHistory(detail);
- break;
- case EventType.SHOW_CHANGE:
- this._handleShowChange(detail);
- break;
- case EventType.COMMENT:
- this._handleComment(detail);
- break;
- case EventType.LABEL_CHANGE:
- this._handleLabelChange(detail);
- break;
- case EventType.SHOW_REVISION_ACTIONS:
- this._handleShowRevisionActions(detail);
- break;
- case EventType.HIGHLIGHTJS_LOADED:
- this._handleHighlightjsLoaded(detail);
- break;
- default:
- console.warn('handleEvent called with unsupported event type:', type);
- break;
- }
- });
+ getPluginLoader()
+ .awaitPluginsLoaded()
+ .then(() => {
+ switch (type) {
+ case EventType.HISTORY:
+ this._handleHistory(detail);
+ break;
+ case EventType.SHOW_CHANGE:
+ this._handleShowChange(detail);
+ break;
+ case EventType.COMMENT:
+ this._handleComment(detail);
+ break;
+ case EventType.LABEL_CHANGE:
+ this._handleLabelChange(detail);
+ break;
+ case EventType.SHOW_REVISION_ACTIONS:
+ this._handleShowRevisionActions(detail);
+ break;
+ case EventType.HIGHLIGHTJS_LOADED:
+ this._handleHighlightjsLoaded(detail);
+ break;
+ default:
+ console.warn(
+ 'handleEvent called with unsupported event type:',
+ type
+ );
+ break;
+ }
+ });
}
addElement(key: TargetElement, el: HTMLElement) {
@@ -274,13 +279,15 @@
* will resolve to null.
*/
getCoverageAnnotationApi(): Promise<CoverageProvider | undefined> {
- return pluginLoader.awaitPluginsLoaded().then(
- () =>
- this._getEventCallbacks(EventType.ANNOTATE_DIFF).find(cb => {
- const annotationApi = (cb as unknown) as GrAnnotationActionsInterface;
- return annotationApi.getCoverageProvider();
- }) as CoverageProvider | undefined
- );
+ return getPluginLoader()
+ .awaitPluginsLoaded()
+ .then(
+ () =>
+ this._getEventCallbacks(EventType.ANNOTATE_DIFF).find(cb => {
+ const annotationApi = (cb as unknown) as GrAnnotationActionsInterface;
+ return annotationApi.getCoverageProvider();
+ }) as CoverageProvider | undefined
+ );
}
getAdminMenuLinks() {
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface_test.js b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface_test.js
index 272a13b..e856788 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface_test.js
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface_test.js
@@ -22,7 +22,7 @@
import {EventType} from '../../plugins/gr-plugin-types.js';
import {GrPluginActionContext} from './gr-plugin-action-context.js';
import {PLUGIN_LOADING_TIMEOUT_MS} from './gr-api-utils.js';
-import {pluginLoader} from './gr-plugin-loader.js';
+import {getPluginLoader} from './gr-plugin-loader.js';
import {_testOnly_initGerritPluginApi} from './gr-gerrit.js';
import {stubBaseUrl} from '../../../test/test-utils.js';
@@ -60,7 +60,7 @@
errorStub = sinon.stub(console, 'error');
pluginApi.install(p => { plugin = p; }, '0.1',
'http://test.com/plugins/testplugin/static/test.js');
- pluginLoader.loadPlugins([]);
+ getPluginLoader().loadPlugins([]);
});
teardown(() => {
@@ -234,7 +234,7 @@
revisions: {def: {_number: 2}, abc: {_number: 1}},
};
const spy = sinon.spy();
- pluginLoader.loadPlugins(['plugins/test.html']);
+ getPluginLoader().loadPlugins(['plugins/test.html']);
plugin.on(EventType.SHOW_CHANGE, spy);
element.handleEvent(EventType.SHOW_CHANGE,
{change: testChange, patchNum: 1});
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-loader.ts b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-loader.ts
index 31254ce..1334763 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-loader.ts
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-loader.ts
@@ -444,8 +444,12 @@
}
// TODO(dmfilippov): Convert to service and add to appContext
-export let pluginLoader = new PluginLoader();
+let pluginLoader = new PluginLoader();
export function _testOnly_resetPluginLoader() {
pluginLoader = new PluginLoader();
return pluginLoader;
}
+
+export function getPluginLoader() {
+ return pluginLoader;
+}
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-loader_test.js b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-loader_test.js
index 1e6938e..5accc68 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-loader_test.js
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-loader_test.js
@@ -540,4 +540,3 @@
});
});
});
-
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 ab73b59..a1703a2 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
@@ -613,7 +613,9 @@
getShortcut(shortcutName: Shortcut) {
const bindings = this.bindings.get(shortcutName);
return bindings
- ? bindings.map(binding => this.describeBinding(binding)).join(', ')
+ ? bindings
+ .map(binding => this.describeBinding(binding).join('+'))
+ .join(',')
: '';
}
diff --git a/polygerrit-ui/app/mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin_test.js b/polygerrit-ui/app/mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin_test.js
index 534ead4..defc7dc 100644
--- a/polygerrit-ui/app/mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin_test.js
+++ b/polygerrit-ui/app/mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin_test.js
@@ -79,7 +79,16 @@
assert.isUndefined(mgr.getBindingsForShortcut(NEXT_FILE));
mgr.bindShortcut(NEXT_FILE, ']', '}', 'right');
- assert.equal(mgr.getShortcut(NEXT_FILE), '], }, →');
+ assert.equal(mgr.getShortcut(NEXT_FILE), '],},→');
+ });
+
+ test('getShortcut with modifiers', () => {
+ const mgr = new ShortcutManager();
+ const NEXT_FILE = Shortcut.NEXT_FILE;
+
+ assert.isUndefined(mgr.getBindingsForShortcut(NEXT_FILE));
+ mgr.bindShortcut(NEXT_FILE, 'Shift+a:key');
+ assert.equal(mgr.getShortcut(NEXT_FILE), 'Shift+a');
});
suite('binding descriptions', () => {
