Merge changes from topic "ts-gr-diff"
* changes:
Convert files to typescript
Rename files to preserve history
diff --git a/Documentation/cmd-hook-commit-msg.txt b/Documentation/cmd-hook-commit-msg.txt
index 2b6d7af..e547822 100644
--- a/Documentation/cmd-hook-commit-msg.txt
+++ b/Documentation/cmd-hook-commit-msg.txt
@@ -56,6 +56,26 @@
The `Change-Id` will not be added if `gerrit.createChangeId` is set
to `false` in the git config.
+If `gerrit.reviewUrl` is set to the base URL of the Gerrit server that
+changes are uploaded to (e.g. `https://gerrit-review.googlesource.com/`)
+in the git config, then instead of adding a `Change-Id` trailer, a `Link`
+trailer will be inserted that will look like this:
+
+----
+Improve foo widget by attaching a bar.
+
+We want a bar, because it improves the foo by providing more
+wizbangery to the dowhatimeanery.
+
+Link: https://gerrit-review.googlesource.com/id/Ic8aaa0728a43936cd4c6e1ed590e01ba8f0fbf5b
+Signed-off-by: A. U. Thor <author@example.com>
+----
+
+This link will become a valid link to the review page once the change is
+uploaded to the Gerrit server. Newer versions of the Gerrit server will read
+the change identifier out of the appropriate `Link` trailer and treat it in
+the same way as the change identifier in a `Change-Id` trailer.
+
== OBTAINING
To obtain the `commit-msg` script use `scp`, `wget` or `curl` to download
diff --git a/Documentation/concept-changes.txt b/Documentation/concept-changes.txt
index 1d275b4..762fb43 100644
--- a/Documentation/concept-changes.txt
+++ b/Documentation/concept-changes.txt
@@ -203,6 +203,34 @@
method uses git's link:cmd-hook-commit-msg.html[commit-msg hook]
to automatically add the Change-Id to each new commit.
+== The Link footer
+
+Gerrit also supports the Link footer as an alternative to the Change-Id
+footer. A Link footer looks like this:
+
+....
+ Link: https://gerrit-review.googlesource.com/id/Ic8aaa0728a43936cd4c6e1ed590e01ba8f0fbf5b
+....
+
+The advantage of this style of footer is that it usually acts
+as a link directly to the change's review page, provided that
+the change has been uploaded to Gerrit. Projects such as the
+link:https://www.kernel.org/doc/html/latest/maintainer/configure-git.html#creating-commit-links-to-lore-kernel-org[Linux kernel]
+have a convention of adding links to commit messages using the
+Link footer.
+
+If multiple changes have been uploaded to Gerrit with the same
+change ID, for example if a change has been cherry-picked to multiple
+branches, the link will take the user to a list of changes.
+
+The base URL in the footer is required to match the server's base URL.
+If the URL does not match, the server will not recognize the footer
+as a change ID footer.
+
+The link:cmd-hook-commit-msg.html[commit-msg hook] can be configured
+to insert Link footers instead of Change-Id footers by setting the
+property `gerrit.reviewUrl` to the base URL of the Gerrit server.
+
GERRIT
------
Part of link:index.html[Gerrit Code Review]
diff --git a/java/com/google/gerrit/acceptance/testsuite/change/ChangeOperationsImpl.java b/java/com/google/gerrit/acceptance/testsuite/change/ChangeOperationsImpl.java
index 13392d8..f04de17 100644
--- a/java/com/google/gerrit/acceptance/testsuite/change/ChangeOperationsImpl.java
+++ b/java/com/google/gerrit/acceptance/testsuite/change/ChangeOperationsImpl.java
@@ -25,6 +25,7 @@
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.entities.Project;
import com.google.gerrit.entities.RefNames;
+import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.server.ChangeUtil;
@@ -52,12 +53,16 @@
import java.util.Objects;
import java.util.Optional;
import org.eclipse.jgit.errors.ConfigInvalidException;
+import org.eclipse.jgit.lib.AnyObjectId;
import org.eclipse.jgit.lib.CommitBuilder;
+import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.ObjectInserter;
import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.merge.MergeStrategy;
+import org.eclipse.jgit.merge.Merger;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevWalk;
import org.eclipse.jgit.util.ChangeIdUtil;
@@ -196,52 +201,154 @@
TestChangeCreation changeCreation,
PersonIdent authorAndCommitter)
throws IOException, BadRequestException {
- Optional<ObjectId> branchTip = getTip(repository, changeCreation.branch());
-
- ObjectId tree =
- createNewTree(
- repository,
- revWalk,
- branchTip.orElse(ObjectId.zeroId()),
- changeCreation.treeModifications());
-
+ ImmutableList<ObjectId> parentCommits = getParentCommits(repository, revWalk, changeCreation);
+ TreeCreator treeCreator =
+ getTreeCreator(objectInserter, parentCommits, changeCreation.mergeStrategy());
+ ObjectId tree = createNewTree(repository, treeCreator, changeCreation.treeModifications());
String commitMessage = correctCommitMessage(changeCreation.commitMessage());
-
- ImmutableList<ObjectId> parentCommitIds = Streams.stream(branchTip).collect(toImmutableList());
return createCommit(
- objectInserter,
- tree,
- parentCommitIds,
- authorAndCommitter,
- authorAndCommitter,
- commitMessage);
+ objectInserter, tree, parentCommits, authorAndCommitter, authorAndCommitter, commitMessage);
}
- private Optional<ObjectId> getTip(Repository repository, String branch) throws IOException {
- Optional<Ref> ref = Optional.ofNullable(repository.findRef(branch));
- return ref.map(Ref::getObjectId);
+ private ImmutableList<ObjectId> getParentCommits(
+ Repository repository, RevWalk revWalk, TestChangeCreation changeCreation) {
+
+ return changeCreation
+ .parents()
+ .map(parents -> resolveParents(repository, revWalk, parents))
+ .orElseGet(() -> asImmutableList(getTip(repository, changeCreation.branch())));
+ }
+
+ private ImmutableList<ObjectId> resolveParents(
+ Repository repository, RevWalk revWalk, ImmutableList<TestCommitIdentifier> parents) {
+ return parents.stream()
+ .map(parent -> resolveCommit(repository, revWalk, parent))
+ .collect(toImmutableList());
+ }
+
+ private ObjectId resolveCommit(
+ Repository repository, RevWalk revWalk, TestCommitIdentifier parentCommit) {
+ switch (parentCommit.getKind()) {
+ case BRANCH:
+ return resolveBranchTip(repository, parentCommit.branch());
+ case CHANGE_ID:
+ return resolveChange(parentCommit.changeId());
+ case COMMIT_SHA_1:
+ return resolveCommitFromSha1(revWalk, parentCommit.commitSha1());
+ case PATCHSET_ID:
+ return resolvePatchset(parentCommit.patchsetId());
+ default:
+ throw new IllegalStateException(
+ String.format("No parent behavior implemented for %s.", parentCommit.getKind()));
+ }
+ }
+
+ private static ObjectId resolveBranchTip(Repository repository, String branchName) {
+ return getTip(repository, branchName)
+ .orElseThrow(
+ () ->
+ new IllegalStateException(
+ String.format(
+ "Tip of branch %s not found and hence can't be used as parent.",
+ branchName)));
+ }
+
+ private static Optional<ObjectId> getTip(Repository repository, String branch) {
+ try {
+ Optional<Ref> ref = Optional.ofNullable(repository.findRef(branch));
+ return ref.map(Ref::getObjectId);
+ } catch (IOException e) {
+ throw new StorageException(e);
+ }
+ }
+
+ private ObjectId resolveChange(Change.Id changeId) {
+ Optional<ChangeNotes> changeNotes = changeFinder.findOne(changeId);
+ return changeNotes
+ .map(ChangeNotes::getCurrentPatchSet)
+ .map(PatchSet::commitId)
+ .orElseThrow(
+ () ->
+ new IllegalStateException(
+ String.format(
+ "Change %s not found and hence can't be used as parent.", changeId)));
+ }
+
+ private static RevCommit resolveCommitFromSha1(RevWalk revWalk, ObjectId commitSha1) {
+ try {
+ return revWalk.parseCommit(commitSha1);
+ } catch (Exception e) {
+ throw new IllegalStateException(
+ String.format("Commit %s not found and hence can't be used as parent/base.", commitSha1),
+ e);
+ }
+ }
+
+ private ObjectId resolvePatchset(PatchSet.Id patchsetId) {
+ Optional<ChangeNotes> changeNotes = changeFinder.findOne(patchsetId.changeId());
+ return changeNotes
+ .map(ChangeNotes::getPatchSets)
+ .map(patchsets -> patchsets.get(patchsetId))
+ .map(PatchSet::commitId)
+ .orElseThrow(
+ () ->
+ new IllegalStateException(
+ String.format(
+ "Patchset %s not found and hence can't be used as parent.", patchsetId)));
+ }
+
+ private static <T> ImmutableList<T> asImmutableList(Optional<T> value) {
+ return Streams.stream(value).collect(toImmutableList());
+ }
+
+ private static TreeCreator getTreeCreator(
+ RevWalk revWalk, ObjectId customBaseCommit, ImmutableList<ObjectId> parentCommits) {
+ RevCommit commit = resolveCommitFromSha1(revWalk, customBaseCommit);
+ // Use actual parents; relevant for example when a file is restored (->
+ // RestoreFileModification).
+ return TreeCreator.basedOnTree(commit.getTree(), parentCommits);
+ }
+
+ private static TreeCreator getTreeCreator(
+ ObjectInserter objectInserter,
+ ImmutableList<ObjectId> parentCommits,
+ MergeStrategy mergeStrategy) {
+ if (parentCommits.isEmpty()) {
+ return TreeCreator.basedOnEmptyTree();
+ }
+ ObjectId baseTreeId = merge(objectInserter, parentCommits, mergeStrategy);
+ return TreeCreator.basedOnTree(baseTreeId, parentCommits);
+ }
+
+ private static ObjectId merge(
+ ObjectInserter objectInserter,
+ ImmutableList<ObjectId> parentCommits,
+ MergeStrategy mergeStrategy) {
+ try {
+ Merger merger = mergeStrategy.newMerger(objectInserter, new Config());
+ boolean mergeSuccessful = merger.merge(parentCommits.toArray(new AnyObjectId[0]));
+ if (!mergeSuccessful) {
+ throw new IllegalStateException(
+ "Conflicts encountered while merging the specified parents. Use"
+ + " mergeOfButBaseOnFirst() instead to avoid these conflicts and define any"
+ + " other desired file contents with file().content().");
+ }
+ return merger.getResultTreeId();
+ } catch (IOException e) {
+ throw new IllegalStateException(
+ "Creating the merge commits of the specified parents failed for an unknown reason.", e);
+ }
}
private static ObjectId createNewTree(
Repository repository,
- RevWalk revWalk,
- ObjectId baseCommitId,
+ TreeCreator treeCreator,
ImmutableList<TreeModification> treeModifications)
throws IOException {
- TreeCreator treeCreator = getTreeCreator(revWalk, baseCommitId);
treeCreator.addTreeModifications(treeModifications);
return treeCreator.createNewTreeAndGetId(repository);
}
- private static TreeCreator getTreeCreator(RevWalk revWalk, ObjectId baseCommitId)
- throws IOException {
- if (ObjectId.zeroId().equals(baseCommitId)) {
- return TreeCreator.basedOnEmptyTree();
- }
- RevCommit baseCommit = revWalk.parseCommit(baseCommitId);
- return TreeCreator.basedOn(baseCommit);
- }
-
private String correctCommitMessage(String desiredCommitMessage) throws BadRequestException {
String commitMessage = CommitMessageUtil.checkAndSanitizeCommitMessage(desiredCommitMessage);
@@ -352,16 +459,16 @@
ObjectId oldPatchsetCommitId = changeNotes.getCurrentPatchSet().commitId();
RevCommit oldPatchsetCommit = repository.parseCommit(oldPatchsetCommitId);
- ObjectId tree =
- createNewTree(
- repository, revWalk, oldPatchsetCommitId, patchsetCreation.treeModifications());
+ ImmutableList<ObjectId> parentCommitIds =
+ getParents(repository, revWalk, patchsetCreation, oldPatchsetCommit);
+ TreeCreator treeCreator = getTreeCreator(revWalk, oldPatchsetCommit, parentCommitIds);
+ ObjectId tree = createNewTree(repository, treeCreator, patchsetCreation.treeModifications());
String commitMessage =
correctCommitMessage(
changeNotes.getChange().getKey().get(),
patchsetCreation.commitMessage().orElseGet(oldPatchsetCommit::getFullMessage));
- ImmutableList<ObjectId> parentCommitIds = getParents(oldPatchsetCommit);
PersonIdent author = getAuthor(oldPatchsetCommit);
PersonIdent committer = getCommitter(oldPatchsetCommit, now);
return createCommit(objectInserter, tree, parentCommitIds, author, committer, commitMessage);
@@ -404,10 +511,16 @@
return date.getTime() / 1000;
}
- private ImmutableList<ObjectId> getParents(RevCommit oldPatchsetCommit) {
- return Arrays.stream(oldPatchsetCommit.getParents())
- .map(ObjectId::toObjectId)
- .collect(toImmutableList());
+ private ImmutableList<ObjectId> getParents(
+ Repository repository,
+ RevWalk revWalk,
+ TestPatchsetCreation patchsetCreation,
+ RevCommit oldPatchsetCommit) {
+ return patchsetCreation
+ .parents()
+ .map(parents -> resolveParents(repository, revWalk, parents))
+ .orElseGet(
+ () -> Arrays.stream(oldPatchsetCommit.getParents()).collect(toImmutableList()));
}
private PatchSetInserter getPatchSetInserter(
diff --git a/java/com/google/gerrit/acceptance/testsuite/change/MultipleParentBuilder.java b/java/com/google/gerrit/acceptance/testsuite/change/MultipleParentBuilder.java
new file mode 100644
index 0000000..63d8c0a
--- /dev/null
+++ b/java/com/google/gerrit/acceptance/testsuite/change/MultipleParentBuilder.java
@@ -0,0 +1,47 @@
+// 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.testsuite.change;
+
+import com.google.common.collect.ImmutableList;
+import java.util.function.Function;
+
+/** Builder to simplify specifying multiple parents for a change. */
+public class MultipleParentBuilder<T> {
+ private final Function<ImmutableList<TestCommitIdentifier>, T> parentsToBuilderAdder;
+ private final ImmutableList.Builder<TestCommitIdentifier> parents;
+
+ public MultipleParentBuilder(
+ Function<ImmutableList<TestCommitIdentifier>, T> parentsToBuilderAdder,
+ TestCommitIdentifier firstParent) {
+ this.parentsToBuilderAdder = parentsToBuilderAdder;
+ parents = ImmutableList.builder();
+ parents.add(firstParent);
+ }
+
+ /** Adds an intermediate parent. */
+ public ParentBuilder<MultipleParentBuilder<T>> followedBy() {
+ return new ParentBuilder<>(
+ parent -> {
+ parents.add(parent);
+ return this;
+ });
+ }
+
+ /** Adds the last parent. */
+ public ParentBuilder<T> and() {
+ return new ParentBuilder<>(
+ (parent) -> parentsToBuilderAdder.apply(parents.add(parent).build()));
+ }
+}
diff --git a/java/com/google/gerrit/acceptance/testsuite/change/ParentBuilder.java b/java/com/google/gerrit/acceptance/testsuite/change/ParentBuilder.java
new file mode 100644
index 0000000..b57aa6d
--- /dev/null
+++ b/java/com/google/gerrit/acceptance/testsuite/change/ParentBuilder.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.acceptance.testsuite.change;
+
+import com.google.gerrit.entities.Change;
+import com.google.gerrit.entities.PatchSet;
+import java.util.function.Function;
+import org.eclipse.jgit.lib.ObjectId;
+
+/** Builder to simplify parent specification of a change. */
+public class ParentBuilder<T> {
+ private final Function<TestCommitIdentifier, T> parentToBuilderAdder;
+
+ public ParentBuilder(Function<TestCommitIdentifier, T> parentToBuilderAdder) {
+ this.parentToBuilderAdder = parentToBuilderAdder;
+ }
+
+ /** Use the commit identified by the specified SHA-1. */
+ public T commit(ObjectId commitSha1) {
+ return parentToBuilderAdder.apply(TestCommitIdentifier.ofCommitSha1(commitSha1));
+ }
+
+ /**
+ * Use the commit which is at the tip of the specified branch. Short branch names (without
+ * refs/heads) are automatically expanded.
+ */
+ public T tipOfBranch(String branchName) {
+ return parentToBuilderAdder.apply(TestCommitIdentifier.ofBranch(branchName));
+ }
+
+ /** Use the current patchset commit of the indicated change. */
+ public T change(Change.Id changeId) {
+ return parentToBuilderAdder.apply(TestCommitIdentifier.ofChangeId(changeId));
+ }
+
+ /** Use the commit identified by the specified patchset. */
+ public T patchset(PatchSet.Id patchsetId) {
+ return parentToBuilderAdder.apply(TestCommitIdentifier.ofPatchsetId(patchsetId));
+ }
+}
diff --git a/java/com/google/gerrit/acceptance/testsuite/change/TestChangeCreation.java b/java/com/google/gerrit/acceptance/testsuite/change/TestChangeCreation.java
index 65db967..5871e17 100644
--- a/java/com/google/gerrit/acceptance/testsuite/change/TestChangeCreation.java
+++ b/java/com/google/gerrit/acceptance/testsuite/change/TestChangeCreation.java
@@ -23,6 +23,7 @@
import com.google.gerrit.server.edit.tree.TreeModification;
import java.util.Optional;
import org.eclipse.jgit.lib.Constants;
+import org.eclipse.jgit.merge.MergeStrategy;
/** Initial attributes of the change. If not provided, arbitrary values will be used. */
@AutoValue
@@ -37,13 +38,19 @@
public abstract ImmutableList<TreeModification> treeModifications();
+ public abstract Optional<ImmutableList<TestCommitIdentifier>> parents();
+
+ public abstract MergeStrategy mergeStrategy();
+
abstract ThrowingFunction<TestChangeCreation, Change.Id> changeCreator();
public static Builder builder(ThrowingFunction<TestChangeCreation, Change.Id> changeCreator) {
return new AutoValue_TestChangeCreation.Builder()
.changeCreator(changeCreator)
.branch(Constants.R_HEADS + Constants.MASTER)
- .commitMessage("A test change");
+ .commitMessage("A test change")
+ // Which value we choose here doesn't matter. All relevant code paths set the desired value.
+ .mergeStrategy(MergeStrategy.OURS);
}
@AutoValue.Builder
@@ -72,6 +79,53 @@
abstract ImmutableList.Builder<TreeModification> treeModificationsBuilder();
+ /**
+ * Parent commit of the change. The commit can be specified via various means in the returned
+ * builder.
+ */
+ public ParentBuilder<Builder> childOf() {
+ return new ParentBuilder<>(parentCommit -> parents(ImmutableList.of(parentCommit)));
+ }
+
+ /**
+ * Parent commits of the change. Each parent commit can be specified via various means in the
+ * returned builder. The order of the parents matters and is preserved (first parent commit in
+ * fluent change -> first parent of the change).
+ *
+ * <p>This method will automatically merge the parent commits and use the resulting commit as
+ * base for the change. Use {@link #file(String)} for additional file adjustments on top of that
+ * merge commit.
+ *
+ * <p><strong>Note:</strong> If this method fails with a merge conflict, use {@link
+ * #mergeOfButBaseOnFirst()} instead and specify all other necessary file contents manually via
+ * {@link #file(String)}.
+ */
+ public ParentBuilder<MultipleParentBuilder<Builder>> mergeOf() {
+ return new ParentBuilder<>(parent -> mergeBuilder(MergeStrategy.RECURSIVE, parent));
+ }
+
+ /**
+ * Parent commits of the change. Each parent commit can be specified via various means in the
+ * returned builder. The order of the parents matters and is preserved (first parent commit in
+ * fluent change -> first parent of the change).
+ *
+ * <p>This method will use the first specified parent commit as base for the resulting change.
+ * This approach is especially useful if merging the parents is not possible.
+ */
+ public ParentBuilder<MultipleParentBuilder<Builder>> mergeOfButBaseOnFirst() {
+ return new ParentBuilder<>(parent -> mergeBuilder(MergeStrategy.OURS, parent));
+ }
+
+ MultipleParentBuilder<Builder> mergeBuilder(
+ MergeStrategy mergeStrategy, TestCommitIdentifier parent) {
+ mergeStrategy(mergeStrategy);
+ return new MultipleParentBuilder<>(this::parents, parent);
+ }
+
+ abstract Builder parents(ImmutableList<TestCommitIdentifier> parents);
+
+ abstract Builder mergeStrategy(MergeStrategy mergeStrategy);
+
abstract Builder changeCreator(ThrowingFunction<TestChangeCreation, Change.Id> changeCreator);
abstract TestChangeCreation autoBuild();
diff --git a/java/com/google/gerrit/acceptance/testsuite/change/TestCommitIdentifier.java b/java/com/google/gerrit/acceptance/testsuite/change/TestCommitIdentifier.java
new file mode 100644
index 0000000..a352607
--- /dev/null
+++ b/java/com/google/gerrit/acceptance/testsuite/change/TestCommitIdentifier.java
@@ -0,0 +1,61 @@
+// 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.testsuite.change;
+
+import com.google.auto.value.AutoOneOf;
+import com.google.gerrit.entities.Change;
+import com.google.gerrit.entities.PatchSet;
+import org.eclipse.jgit.lib.ObjectId;
+
+/** Attributes, each one uniquely identifying a commit. */
+@AutoOneOf(TestCommitIdentifier.Kind.class)
+public abstract class TestCommitIdentifier {
+ public enum Kind {
+ COMMIT_SHA_1,
+ BRANCH,
+ CHANGE_ID,
+ PATCHSET_ID
+ }
+
+ public abstract Kind getKind();
+
+ /** SHA-1 of the commit. */
+ public abstract ObjectId commitSha1();
+
+ /** Branch whose tip points to the desired commit. */
+ public abstract String branch();
+
+ /** Numeric ID of the change whose current patchset points to the desired commit. */
+ public abstract Change.Id changeId();
+
+ /** ID of the patchset representing the desired commit. */
+ public abstract PatchSet.Id patchsetId();
+
+ public static TestCommitIdentifier ofCommitSha1(ObjectId commitSha1) {
+ return AutoOneOf_TestCommitIdentifier.commitSha1(commitSha1);
+ }
+
+ public static TestCommitIdentifier ofBranch(String branchName) {
+ return AutoOneOf_TestCommitIdentifier.branch(branchName);
+ }
+
+ public static TestCommitIdentifier ofChangeId(Change.Id changeId) {
+ return AutoOneOf_TestCommitIdentifier.changeId(changeId);
+ }
+
+ public static TestCommitIdentifier ofPatchsetId(PatchSet.Id patchsetId) {
+ return AutoOneOf_TestCommitIdentifier.patchsetId(patchsetId);
+ }
+}
diff --git a/java/com/google/gerrit/acceptance/testsuite/change/TestPatchsetCreation.java b/java/com/google/gerrit/acceptance/testsuite/change/TestPatchsetCreation.java
index 0152cd1..fe9d909 100644
--- a/java/com/google/gerrit/acceptance/testsuite/change/TestPatchsetCreation.java
+++ b/java/com/google/gerrit/acceptance/testsuite/change/TestPatchsetCreation.java
@@ -29,6 +29,8 @@
public abstract ImmutableList<TreeModification> treeModifications();
+ public abstract Optional<ImmutableList<TestCommitIdentifier>> parents();
+
abstract ThrowingFunction<TestPatchsetCreation, PatchSet.Id> patchsetCreator();
public static TestPatchsetCreation.Builder builder(
@@ -48,6 +50,37 @@
abstract ImmutableList.Builder<TreeModification> treeModificationsBuilder();
+ /**
+ * Parent commit of the change. The commit can be specified via various means in the returned
+ * builder.
+ *
+ * <p>This method will just change the parent but not influence the contents of the patchset
+ * commit.
+ *
+ * <p>It's possible to switch from a change representing a merge commit to a change not being a
+ * merge commit with this method.
+ */
+ public ParentBuilder<Builder> parent() {
+ return new ParentBuilder<>(parent -> parents(ImmutableList.of(parent)));
+ }
+
+ /**
+ * Parent commits of the change. Each parent commit can be specified via various means in the
+ * returned builder. The order of the parents matters and is preserved (first parent commit in
+ * fluent change -> first parent of the change).
+ *
+ * <p>This method will just change the parents but not influence the contents of the patchset
+ * commit.
+ *
+ * <p>It's possible to switch from a change representing a non-merge commit to a change which is
+ * a merge commit with this method.
+ */
+ public ParentBuilder<MultipleParentBuilder<Builder>> parents() {
+ return new ParentBuilder<>(parent -> new MultipleParentBuilder<>(this::parents, parent));
+ }
+
+ abstract Builder parents(ImmutableList<TestCommitIdentifier> value);
+
abstract TestPatchsetCreation.Builder patchsetCreator(
ThrowingFunction<TestPatchsetCreation, PatchSet.Id> patchsetCreator);
diff --git a/java/com/google/gerrit/common/FooterConstants.java b/java/com/google/gerrit/common/FooterConstants.java
index 3ec809c..656d850 100644
--- a/java/com/google/gerrit/common/FooterConstants.java
+++ b/java/com/google/gerrit/common/FooterConstants.java
@@ -20,6 +20,9 @@
/** The change ID as used to track patch sets. */
public static final FooterKey CHANGE_ID = new FooterKey("Change-Id");
+ /** Link is an alternative footer that may be used to track patch sets. */
+ public static final FooterKey LINK = new FooterKey("Link");
+
/** The footer telling us who reviewed the change. */
public static final FooterKey REVIEWED_BY = new FooterKey("Reviewed-by");
diff --git a/java/com/google/gerrit/extensions/restapi/testing/AttentionSetUpdateSubject.java b/java/com/google/gerrit/extensions/restapi/testing/AttentionSetUpdateSubject.java
index 2dc94b0..4033c3e 100644
--- a/java/com/google/gerrit/extensions/restapi/testing/AttentionSetUpdateSubject.java
+++ b/java/com/google/gerrit/extensions/restapi/testing/AttentionSetUpdateSubject.java
@@ -66,11 +66,11 @@
}
/**
- * Returns a {@link ComparableSubject} for the {@link AttentionSetUpdate.Operation} of attention
- * set update.
+ * Returns a {@link ComparableSubject} for the {@link
+ * com.google.gerrit.entities.AttentionSetUpdate.Operation} of attention set update.
*
- * @return {@link ComparableSubject} for the {@link AttentionSetUpdate.Operation} of attention set
- * update
+ * @return {@link ComparableSubject} for the {@link
+ * com.google.gerrit.entities.AttentionSetUpdate.Operation} of attention set update.
*/
public ComparableSubject<AttentionSetUpdate.Operation> hasOperationThat() {
return check("operation()").that(attentionSetUpdate().operation());
diff --git a/java/com/google/gerrit/httpd/raw/StaticModule.java b/java/com/google/gerrit/httpd/raw/StaticModule.java
index 414a120..4b2c8a9 100644
--- a/java/com/google/gerrit/httpd/raw/StaticModule.java
+++ b/java/com/google/gerrit/httpd/raw/StaticModule.java
@@ -69,6 +69,7 @@
ImmutableList.of(
"/",
"/c/*",
+ "/id/*",
"/p/*",
"/q/*",
"/x/*",
diff --git a/java/com/google/gerrit/server/ChangeUtil.java b/java/com/google/gerrit/server/ChangeUtil.java
index a166d97..eea1052 100644
--- a/java/com/google/gerrit/server/ChangeUtil.java
+++ b/java/com/google/gerrit/server/ChangeUtil.java
@@ -19,17 +19,24 @@
import com.google.common.collect.Ordering;
import com.google.common.io.BaseEncoding;
+import com.google.gerrit.common.FooterConstants;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.PatchSet;
+import com.google.gerrit.server.config.UrlFormatter;
import com.google.inject.Singleton;
import java.io.IOException;
import java.security.SecureRandom;
import java.util.Collection;
+import java.util.List;
+import java.util.Optional;
import java.util.Random;
import java.util.Set;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
import java.util.stream.Stream;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.revwalk.RevCommit;
@Singleton
public class ChangeUtil {
@@ -106,5 +113,29 @@
return c != null ? c.getStatus().name().toLowerCase() : "deleted";
}
+ private static final Pattern LINK_CHANGE_ID_PATTERN = Pattern.compile("I[0-9a-f]{40}");
+
+ public static List<String> getChangeIdsFromFooter(RevCommit c, UrlFormatter urlFormatter) {
+ List<String> changeIds = c.getFooterLines(FooterConstants.CHANGE_ID);
+ Optional<String> webUrl = urlFormatter.getWebUrl();
+ if (!webUrl.isPresent()) {
+ return changeIds;
+ }
+
+ String prefix = webUrl.get() + "id/";
+ for (String link : c.getFooterLines(FooterConstants.LINK)) {
+ if (!link.startsWith(prefix)) {
+ continue;
+ }
+ String changeId = link.substring(prefix.length());
+ Matcher m = LINK_CHANGE_ID_PATTERN.matcher(changeId);
+ if (m.matches()) {
+ changeIds.add(changeId);
+ }
+ }
+
+ return changeIds;
+ }
+
private ChangeUtil() {}
}
diff --git a/java/com/google/gerrit/server/CommentsUtil.java b/java/com/google/gerrit/server/CommentsUtil.java
index 81969f0..30913f7 100644
--- a/java/com/google/gerrit/server/CommentsUtil.java
+++ b/java/com/google/gerrit/server/CommentsUtil.java
@@ -57,6 +57,7 @@
import java.util.Optional;
import java.util.Set;
import java.util.stream.Collectors;
+import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.Repository;
@@ -411,15 +412,33 @@
ps.id(),
c);
if (c.getCommitId() == null) {
- if (Side.fromShort(c.side) == Side.PARENT) {
- if (c.side < 0) {
- c.setCommitId(cache.getOldId(change, ps, -c.side));
- } else {
- c.setCommitId(cache.getOldId(change, ps, null));
- }
+ c.setCommitId(determineCommitId(cache, change, ps, c.side));
+ }
+ }
+
+ /**
+ * Determines the SHA-1 of the commit referenced by the (change, patchset, side) triple.
+ *
+ * @param patchListCache the cache to use for SHA-1 lookups
+ * @param change the change to which the commit belongs
+ * @param patchset the patchset to which the commit belongs
+ * @param side the side indicating which commit of the patchset to take. 1 is the patchset commit,
+ * 0 the parent commit (or auto-merge for changes representing merge commits); -x the xth
+ * parent commit of a merge commit
+ * @return the commit SHA-1
+ * @throws PatchListNotAvailableException if the SHA-1 is unavailable for an unknown reason
+ */
+ public static ObjectId determineCommitId(
+ PatchListCache patchListCache, Change change, PatchSet patchset, short side)
+ throws PatchListNotAvailableException {
+ if (Side.fromShort(side) == Side.PARENT) {
+ if (side < 0) {
+ return patchListCache.getOldId(change, patchset, -side);
} else {
- c.setCommitId(ps.commitId());
+ return patchListCache.getOldId(change, patchset, null);
}
+ } else {
+ return patchset.commitId();
}
}
diff --git a/java/com/google/gerrit/server/change/ChangeInserter.java b/java/com/google/gerrit/server/change/ChangeInserter.java
index f49a21e..a086cb1 100644
--- a/java/com/google/gerrit/server/change/ChangeInserter.java
+++ b/java/com/google/gerrit/server/change/ChangeInserter.java
@@ -28,7 +28,6 @@
import com.google.common.collect.Iterables;
import com.google.common.collect.Streams;
import com.google.common.flogger.FluentLogger;
-import com.google.gerrit.common.FooterConstants;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.BranchNameKey;
import com.google.gerrit.entities.Change;
@@ -41,17 +40,20 @@
import com.google.gerrit.entities.SubmissionId;
import com.google.gerrit.extensions.api.changes.NotifyHandling;
import com.google.gerrit.extensions.client.ReviewerState;
+import com.google.gerrit.extensions.registration.DynamicItem;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.ChangeMessagesUtil;
+import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.PatchSetUtil;
import com.google.gerrit.server.change.ReviewerAdder.InternalAddReviewerInput;
import com.google.gerrit.server.change.ReviewerAdder.ReviewerAddition;
import com.google.gerrit.server.change.ReviewerAdder.ReviewerAdditionList;
import com.google.gerrit.server.config.SendEmailExecutor;
+import com.google.gerrit.server.config.UrlFormatter;
import com.google.gerrit.server.events.CommitReceivedEvent;
import com.google.gerrit.server.extensions.events.CommentAdded;
import com.google.gerrit.server.extensions.events.RevisionCreated;
@@ -110,6 +112,7 @@
private final CommentAdded commentAdded;
private final ReviewerAdder reviewerAdder;
private final MessageIdGenerator messageIdGenerator;
+ private final DynamicItem<UrlFormatter> urlFormatter;
private final Change.Id changeId;
private final PatchSet.Id psId;
@@ -159,6 +162,7 @@
RevisionCreated revisionCreated,
ReviewerAdder reviewerAdder,
MessageIdGenerator messageIdGenerator,
+ DynamicItem<UrlFormatter> urlFormatter,
@Assisted Change.Id changeId,
@Assisted ObjectId commitId,
@Assisted String refName) {
@@ -175,6 +179,7 @@
this.commentAdded = commentAdded;
this.reviewerAdder = reviewerAdder;
this.messageIdGenerator = messageIdGenerator;
+ this.urlFormatter = urlFormatter;
this.changeId = changeId;
this.psId = PatchSet.id(changeId, INITIAL_PATCH_SET_ID);
@@ -191,7 +196,7 @@
public Change createChange(Context ctx) throws IOException {
change =
new Change(
- getChangeKey(ctx.getRevWalk(), commitId),
+ getChangeKey(ctx.getRevWalk()),
changeId,
ctx.getAccountId(),
BranchNameKey.create(ctx.getProject(), refName),
@@ -206,10 +211,10 @@
return change;
}
- private static Change.Key getChangeKey(RevWalk rw, ObjectId id) throws IOException {
- RevCommit commit = rw.parseCommit(id);
+ private Change.Key getChangeKey(RevWalk rw) throws IOException {
+ RevCommit commit = rw.parseCommit(commitId);
rw.parseBody(commit);
- List<String> idList = commit.getFooterLines(FooterConstants.CHANGE_ID);
+ List<String> idList = ChangeUtil.getChangeIdsFromFooter(commit, urlFormatter.get());
if (!idList.isEmpty()) {
return Change.key(idList.get(idList.size() - 1).trim());
}
diff --git a/java/com/google/gerrit/server/change/ConsistencyChecker.java b/java/com/google/gerrit/server/change/ConsistencyChecker.java
index 61616c0..7d0bda1 100644
--- a/java/com/google/gerrit/server/change/ConsistencyChecker.java
+++ b/java/com/google/gerrit/server/change/ConsistencyChecker.java
@@ -28,7 +28,6 @@
import com.google.common.collect.MultimapBuilder;
import com.google.common.collect.SetMultimap;
import com.google.common.flogger.FluentLogger;
-import com.google.gerrit.common.FooterConstants;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.PatchSet;
@@ -38,12 +37,14 @@
import com.google.gerrit.extensions.api.changes.FixInput;
import com.google.gerrit.extensions.common.ProblemInfo;
import com.google.gerrit.extensions.common.ProblemInfo.Status;
+import com.google.gerrit.extensions.registration.DynamicItem;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.PatchSetUtil;
import com.google.gerrit.server.account.Accounts;
+import com.google.gerrit.server.config.UrlFormatter;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.PatchSetState;
@@ -115,6 +116,7 @@
private final Provider<CurrentUser> user;
private final Provider<PersonIdent> serverIdent;
private final RetryHelper retryHelper;
+ private final DynamicItem<UrlFormatter> urlFormatter;
private BatchUpdate.Factory updateFactory;
private FixInput fix;
@@ -141,7 +143,8 @@
PatchSetInserter.Factory patchSetInserterFactory,
PatchSetUtil psUtil,
Provider<CurrentUser> user,
- RetryHelper retryHelper) {
+ RetryHelper retryHelper,
+ DynamicItem<UrlFormatter> urlFormatter) {
this.accounts = accounts;
this.accountPatchReviewStore = accountPatchReviewStore;
this.notesFactory = notesFactory;
@@ -152,6 +155,7 @@
this.retryHelper = retryHelper;
this.serverIdent = serverIdent;
this.user = user;
+ this.urlFormatter = urlFormatter;
reset();
}
@@ -456,7 +460,8 @@
// No patch set for this commit; insert one.
rw.parseBody(commit);
String changeId =
- Iterables.getFirst(commit.getFooterLines(FooterConstants.CHANGE_ID), null);
+ Iterables.getFirst(
+ ChangeUtil.getChangeIdsFromFooter(commit, urlFormatter.get()), null);
// Missing Change-Id footer is ok, but mismatched is not.
if (changeId != null && !changeId.equals(change().getKey().get())) {
problem(
diff --git a/java/com/google/gerrit/server/edit/tree/TreeCreator.java b/java/com/google/gerrit/server/edit/tree/TreeCreator.java
index 45f877e..dfc1ffb 100644
--- a/java/com/google/gerrit/server/edit/tree/TreeCreator.java
+++ b/java/com/google/gerrit/server/edit/tree/TreeCreator.java
@@ -46,6 +46,12 @@
return new TreeCreator(baseCommit.getTree(), ImmutableList.copyOf(baseCommit.getParents()));
}
+ public static TreeCreator basedOnTree(
+ ObjectId baseTreeId, ImmutableList<? extends ObjectId> baseParents) {
+ requireNonNull(baseTreeId, "baseTreeId is required");
+ return new TreeCreator(baseTreeId, baseParents);
+ }
+
public static TreeCreator basedOnEmptyTree() {
return new TreeCreator(ObjectId.zeroId(), ImmutableList.of());
}
diff --git a/java/com/google/gerrit/server/git/MergeUtil.java b/java/com/google/gerrit/server/git/MergeUtil.java
index 7518a14..8666f26 100644
--- a/java/com/google/gerrit/server/git/MergeUtil.java
+++ b/java/com/google/gerrit/server/git/MergeUtil.java
@@ -48,6 +48,7 @@
import com.google.gerrit.extensions.restapi.MethodNotAllowedException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.server.ApprovalsUtil;
+import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.config.UrlFormatter;
@@ -531,7 +532,7 @@
msgbuf.append('\n');
}
- if (!contains(footers, FooterConstants.CHANGE_ID, c.getKey().get())) {
+ if (ChangeUtil.getChangeIdsFromFooter(n, urlFormatter.get()).isEmpty()) {
msgbuf.append(FooterConstants.CHANGE_ID.getName());
msgbuf.append(": ");
msgbuf.append(c.getKey().get());
diff --git a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
index 1d10045..015ed0b 100644
--- a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
+++ b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
@@ -60,7 +60,6 @@
import com.google.common.collect.SortedSetMultimap;
import com.google.common.collect.Streams;
import com.google.common.flogger.FluentLogger;
-import com.google.gerrit.common.FooterConstants;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.UsedAt;
import com.google.gerrit.entities.Account;
@@ -117,6 +116,7 @@
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.config.PluginConfig;
import com.google.gerrit.server.config.ProjectConfigEntry;
+import com.google.gerrit.server.config.UrlFormatter;
import com.google.gerrit.server.edit.ChangeEdit;
import com.google.gerrit.server.edit.ChangeEditUtil;
import com.google.gerrit.server.git.BanCommit;
@@ -347,6 +347,7 @@
private final ProjectConfig.Factory projectConfigFactory;
private final SetPrivateOp.Factory setPrivateOpFactory;
private final ReplyAttentionSetUpdates replyAttentionSetUpdates;
+ private final DynamicItem<UrlFormatter> urlFormatter;
// Assisted injected fields.
private final ProjectState projectState;
@@ -427,6 +428,7 @@
TagCache tagCache,
SetPrivateOp.Factory setPrivateOpFactory,
ReplyAttentionSetUpdates replyAttentionSetUpdates,
+ DynamicItem<UrlFormatter> urlFormatter,
@Assisted ProjectState projectState,
@Assisted IdentifiedUser user,
@Assisted ReceivePack rp,
@@ -475,6 +477,7 @@
this.projectConfigFactory = projectConfigFactory;
this.setPrivateOpFactory = setPrivateOpFactory;
this.replyAttentionSetUpdates = replyAttentionSetUpdates;
+ this.urlFormatter = urlFormatter;
// Assisted injected fields.
this.projectState = projectState;
@@ -2084,7 +2087,7 @@
} catch (IOException e) {
throw new StorageException("Can't parse commit", e);
}
- List<String> idList = create.commit.getFooterLines(FooterConstants.CHANGE_ID);
+ List<String> idList = ChangeUtil.getChangeIdsFromFooter(create.commit, urlFormatter.get());
if (idList.isEmpty()) {
messages.add(
@@ -2176,7 +2179,7 @@
}
}
- List<String> idList = c.getFooterLines(FooterConstants.CHANGE_ID);
+ List<String> idList = ChangeUtil.getChangeIdsFromFooter(c, urlFormatter.get());
if (!idList.isEmpty()) {
pending.put(c, lookupByChangeKey(c, Change.key(idList.get(idList.size() - 1).trim())));
} else {
@@ -3315,7 +3318,8 @@
}
}
- for (String changeId : c.getFooterLines(FooterConstants.CHANGE_ID)) {
+ for (String changeId :
+ ChangeUtil.getChangeIdsFromFooter(c, urlFormatter.get())) {
if (byKey == null) {
byKey =
retryHelper
diff --git a/java/com/google/gerrit/server/git/receive/ReplaceOp.java b/java/com/google/gerrit/server/git/receive/ReplaceOp.java
index cdec2cd..ce62d7a 100644
--- a/java/com/google/gerrit/server/git/receive/ReplaceOp.java
+++ b/java/com/google/gerrit/server/git/receive/ReplaceOp.java
@@ -16,7 +16,6 @@
import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.common.collect.ImmutableSet.toImmutableSet;
-import static com.google.gerrit.common.FooterConstants.CHANGE_ID;
import static com.google.gerrit.server.change.ReviewerAdder.newAddReviewerInputFromCommitIdentity;
import static com.google.gerrit.server.mail.MailUtil.getRecipientsFromFooters;
import static com.google.gerrit.server.mail.MailUtil.getRecipientsFromReviewers;
@@ -41,11 +40,13 @@
import com.google.gerrit.extensions.api.changes.NotifyHandling;
import com.google.gerrit.extensions.client.ChangeKind;
import com.google.gerrit.extensions.client.ReviewerState;
+import com.google.gerrit.extensions.registration.DynamicItem;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.ChangeMessagesUtil;
+import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.PatchSetUtil;
import com.google.gerrit.server.account.AccountResolver;
import com.google.gerrit.server.change.AddReviewersOp;
@@ -56,6 +57,7 @@
import com.google.gerrit.server.change.ReviewerAdder.ReviewerAddition;
import com.google.gerrit.server.change.ReviewerAdder.ReviewerAdditionList;
import com.google.gerrit.server.config.SendEmailExecutor;
+import com.google.gerrit.server.config.UrlFormatter;
import com.google.gerrit.server.extensions.events.CommentAdded;
import com.google.gerrit.server.extensions.events.RevisionCreated;
import com.google.gerrit.server.git.MergedByPushOp;
@@ -131,6 +133,7 @@
private final ReviewerAdder reviewerAdder;
private final Change change;
private final MessageIdGenerator messageIdGenerator;
+ private final DynamicItem<UrlFormatter> urlFormatter;
private final ProjectState projectState;
private final BranchNameKey dest;
@@ -175,6 +178,7 @@
ReviewerAdder reviewerAdder,
Change change,
MessageIdGenerator messageIdGenerator,
+ DynamicItem<UrlFormatter> urlFormatter,
@Assisted ProjectState projectState,
@Assisted BranchNameKey dest,
@Assisted boolean checkMergedInto,
@@ -202,6 +206,7 @@
this.reviewerAdder = reviewerAdder;
this.change = change;
this.messageIdGenerator = messageIdGenerator;
+ this.urlFormatter = urlFormatter;
this.projectState = projectState;
this.dest = dest;
@@ -483,7 +488,7 @@
change.setStatus(Change.Status.NEW);
change.setCurrentPatchSet(info);
- List<String> idList = commit.getFooterLines(CHANGE_ID);
+ List<String> idList = ChangeUtil.getChangeIdsFromFooter(commit, urlFormatter.get());
change.setKey(Change.key(idList.get(idList.size() - 1).trim()));
}
diff --git a/java/com/google/gerrit/server/git/validators/CommitValidators.java b/java/com/google/gerrit/server/git/validators/CommitValidators.java
index 923ba68..c67df8b 100644
--- a/java/com/google/gerrit/server/git/validators/CommitValidators.java
+++ b/java/com/google/gerrit/server/git/validators/CommitValidators.java
@@ -34,6 +34,7 @@
import com.google.gerrit.extensions.api.config.ConsistencyCheckInfo.ConsistencyProblemInfo;
import com.google.gerrit.extensions.registration.DynamicItem;
import com.google.gerrit.extensions.restapi.AuthException;
+import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.externalids.ExternalIdsConsistencyChecker;
@@ -302,7 +303,7 @@
}
RevCommit commit = receiveEvent.commit;
List<CommitValidationMessage> messages = new ArrayList<>();
- List<String> idList = commit.getFooterLines(FooterConstants.CHANGE_ID);
+ List<String> idList = ChangeUtil.getChangeIdsFromFooter(commit, urlFormatter);
if (idList.isEmpty()) {
String shortMsg = commit.getShortMessage();
diff --git a/java/com/google/gerrit/server/project/ProjectsConsistencyChecker.java b/java/com/google/gerrit/server/project/ProjectsConsistencyChecker.java
index 7ed2491..812d98d 100644
--- a/java/com/google/gerrit/server/project/ProjectsConsistencyChecker.java
+++ b/java/com/google/gerrit/server/project/ProjectsConsistencyChecker.java
@@ -14,7 +14,6 @@
package com.google.gerrit.server.project;
-import static com.google.gerrit.common.FooterConstants.CHANGE_ID;
import static com.google.gerrit.index.query.Predicate.and;
import static com.google.gerrit.index.query.Predicate.or;
import static com.google.gerrit.server.query.change.ChangeStatusPredicate.open;
@@ -36,13 +35,16 @@
import com.google.gerrit.extensions.api.projects.CheckProjectResultInfo.AutoCloseableChangesCheckResult;
import com.google.gerrit.extensions.client.ListChangesOption;
import com.google.gerrit.extensions.common.ChangeInfo;
+import com.google.gerrit.extensions.registration.DynamicItem;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
import com.google.gerrit.index.IndexConfig;
import com.google.gerrit.index.query.Predicate;
+import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.change.ChangeJson;
+import com.google.gerrit.server.config.UrlFormatter;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.index.change.ChangeField;
import com.google.gerrit.server.query.change.ChangeData;
@@ -75,17 +77,20 @@
private final RetryHelper retryHelper;
private final ChangeJson.Factory changeJsonFactory;
private final IndexConfig indexConfig;
+ private final DynamicItem<UrlFormatter> urlFormatter;
@Inject
ProjectsConsistencyChecker(
GitRepositoryManager repoManager,
RetryHelper retryHelper,
ChangeJson.Factory changeJsonFactory,
- IndexConfig indexConfig) {
+ IndexConfig indexConfig,
+ DynamicItem<UrlFormatter> urlFormatter) {
this.repoManager = repoManager;
this.retryHelper = retryHelper;
this.changeJsonFactory = changeJsonFactory;
this.indexConfig = indexConfig;
+ this.urlFormatter = urlFormatter;
}
public CheckProjectResultInfo check(Project.NameKey projectName, CheckProjectInput input)
@@ -172,7 +177,7 @@
mergedSha1s.add(commitId);
// Consider all Change-Id lines since this is what ReceiveCommits#autoCloseChanges does.
- List<String> changeIds = commit.getFooterLines(CHANGE_ID);
+ List<String> changeIds = ChangeUtil.getChangeIdsFromFooter(commit, urlFormatter.get());
// Number of predicates that we need to add for this commit, 1 per Change-Id plus one for
// the commit.
diff --git a/java/com/google/gerrit/server/restapi/change/CherryPickChange.java b/java/com/google/gerrit/server/restapi/change/CherryPickChange.java
index 44dc6e1..5fab976 100644
--- a/java/com/google/gerrit/server/restapi/change/CherryPickChange.java
+++ b/java/com/google/gerrit/server/restapi/change/CherryPickChange.java
@@ -20,7 +20,6 @@
import com.google.auto.value.AutoValue;
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableSet;
-import com.google.gerrit.common.FooterConstants;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.BranchNameKey;
@@ -29,6 +28,7 @@
import com.google.gerrit.entities.Project;
import com.google.gerrit.extensions.api.changes.CherryPickInput;
import com.google.gerrit.extensions.api.changes.NotifyHandling;
+import com.google.gerrit.extensions.registration.DynamicItem;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.MergeConflictException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
@@ -43,6 +43,7 @@
import com.google.gerrit.server.change.NotifyResolver;
import com.google.gerrit.server.change.PatchSetInserter;
import com.google.gerrit.server.change.SetCherryPickOp;
+import com.google.gerrit.server.config.UrlFormatter;
import com.google.gerrit.server.git.CodeReviewCommit;
import com.google.gerrit.server.git.CodeReviewCommit.CodeReviewRevWalk;
import com.google.gerrit.server.git.GitRepositoryManager;
@@ -112,6 +113,7 @@
private final ApprovalsUtil approvalsUtil;
private final NotifyResolver notifyResolver;
private final BatchUpdate.Factory batchUpdateFactory;
+ private final DynamicItem<UrlFormatter> urlFormatter;
@Inject
CherryPickChange(
@@ -128,7 +130,8 @@
ProjectCache projectCache,
ApprovalsUtil approvalsUtil,
NotifyResolver notifyResolver,
- BatchUpdate.Factory batchUpdateFactory) {
+ BatchUpdate.Factory batchUpdateFactory,
+ DynamicItem<UrlFormatter> urlFormatter) {
this.seq = seq;
this.queryProvider = queryProvider;
this.gitManager = gitManager;
@@ -143,6 +146,7 @@
this.approvalsUtil = approvalsUtil;
this.notifyResolver = notifyResolver;
this.batchUpdateFactory = batchUpdateFactory;
+ this.urlFormatter = urlFormatter;
}
/**
@@ -312,7 +316,8 @@
input.allowConflicts);
Change.Key changeKey;
- final List<String> idList = cherryPickCommit.getFooterLines(FooterConstants.CHANGE_ID);
+ final List<String> idList =
+ ChangeUtil.getChangeIdsFromFooter(cherryPickCommit, urlFormatter.get());
if (!idList.isEmpty()) {
final String idStr = idList.get(idList.size() - 1).trim();
changeKey = Change.key(idStr);
diff --git a/java/com/google/gerrit/server/restapi/change/PutMessage.java b/java/com/google/gerrit/server/restapi/change/PutMessage.java
index 2e9d21a..190deb5 100644
--- a/java/com/google/gerrit/server/restapi/change/PutMessage.java
+++ b/java/com/google/gerrit/server/restapi/change/PutMessage.java
@@ -21,6 +21,7 @@
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.extensions.api.changes.NotifyHandling;
import com.google.gerrit.extensions.common.CommitMessageInput;
+import com.google.gerrit.extensions.registration.DynamicItem;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
@@ -34,6 +35,7 @@
import com.google.gerrit.server.change.ChangeResource;
import com.google.gerrit.server.change.NotifyResolver;
import com.google.gerrit.server.change.PatchSetInserter;
+import com.google.gerrit.server.config.UrlFormatter;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.permissions.ChangePermission;
@@ -73,6 +75,7 @@
private final PatchSetUtil psUtil;
private final NotifyResolver notifyResolver;
private final ProjectCache projectCache;
+ private final DynamicItem<UrlFormatter> urlFormatter;
@Inject
PutMessage(
@@ -84,7 +87,8 @@
@GerritPersonIdent PersonIdent gerritIdent,
PatchSetUtil psUtil,
NotifyResolver notifyResolver,
- ProjectCache projectCache) {
+ ProjectCache projectCache,
+ DynamicItem<UrlFormatter> urlFormatter) {
this.updateFactory = updateFactory;
this.repositoryManager = repositoryManager;
this.userProvider = userProvider;
@@ -94,6 +98,7 @@
this.psUtil = psUtil;
this.notifyResolver = notifyResolver;
this.projectCache = projectCache;
+ this.urlFormatter = urlFormatter;
}
@Override
@@ -200,7 +205,7 @@
}
}
- private static String ensureChangeIdIsCorrect(
+ private String ensureChangeIdIsCorrect(
boolean requireChangeId, String currentChangeId, String newCommitMessage)
throws ResourceConflictException, BadRequestException {
RevCommit revCommit =
@@ -210,7 +215,7 @@
// Check that the commit message without footers is not empty
CommitMessageUtil.checkAndSanitizeCommitMessage(revCommit.getShortMessage());
- List<String> changeIdFooters = revCommit.getFooterLines(FooterConstants.CHANGE_ID);
+ List<String> changeIdFooters = ChangeUtil.getChangeIdsFromFooter(revCommit, urlFormatter.get());
if (!changeIdFooters.isEmpty() && !changeIdFooters.get(0).equals(currentChangeId)) {
throw new ResourceConflictException("wrong Change-Id footer");
}
diff --git a/javatests/com/google/gerrit/acceptance/api/change/PluginOperatorsIT.java b/javatests/com/google/gerrit/acceptance/api/change/PluginOperatorsIT.java
index 68307cf..f4cf96d 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/PluginOperatorsIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/PluginOperatorsIT.java
@@ -49,10 +49,11 @@
assertThat(getChanges(queryChanges)).hasSize(0);
try (AutoCloseable ignored = installPlugin("myplugin", IsOperatorModule.class)) {
- List<ChangeInfo> changes = getChanges(queryChanges);
+ List<?> changes = getChanges(queryChanges);
assertThat(changes).hasSize(1);
- String outputChangeId = ((ChangeInfo) changes.get(0)).changeId;
+ ChangeInfo c = (ChangeInfo) changes.get(0);
+ String outputChangeId = c.changeId;
assertThat(outputChangeId).isEqualTo(evenChangeId);
assertThat(outputChangeId).isNotEqualTo(oddChangeId);
}
@@ -95,8 +96,8 @@
}
}
- private List<ChangeInfo> getChanges(QueryChanges queryChanges)
+ private List<?> getChanges(QueryChanges queryChanges)
throws AuthException, PermissionBackendException, BadRequestException {
- return (List<ChangeInfo>) queryChanges.apply(TopLevelResource.INSTANCE).value();
+ return queryChanges.apply(TopLevelResource.INSTANCE).value();
}
}
diff --git a/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java b/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java
index 6fb444f..e994d03 100644
--- a/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java
+++ b/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java
@@ -1557,6 +1557,27 @@
}
@Test
+ public void pushWithLinkFooter() throws Exception {
+ String changeId = "I0123456789abcdef0123456789abcdef01234567";
+ String url = cfg.getString("gerrit", null, "canonicalWebUrl");
+ if (!url.endsWith("/")) {
+ url += "/";
+ }
+ RevCommit c = createCommit(testRepo, "test commit\n\nLink: " + url + "id/" + changeId);
+ pushForReviewOk(testRepo);
+
+ List<ChangeMessageInfo> messages = getMessages(changeId);
+ assertThat(messages.get(0).message).isEqualTo("Uploaded patch set 1.");
+ }
+
+ @Test
+ public void pushWithWrongHostLinkFooter() throws Exception {
+ String changeId = "I0123456789abcdef0123456789abcdef01234567";
+ RevCommit c = createCommit(testRepo, "test commit\n\nLink: https://wronghost/id/" + changeId);
+ pushForReviewRejected(testRepo, "missing Change-Id in message footer");
+ }
+
+ @Test
public void pushWithChangeIdAboveFooterWithCreateNewChangeForAllNotInTarget() throws Exception {
enableCreateNewChangeForAllNotInTarget();
testPushWithChangeIdAboveFooter();
diff --git a/javatests/com/google/gerrit/acceptance/testsuite/change/ChangeOperationsImplTest.java b/javatests/com/google/gerrit/acceptance/testsuite/change/ChangeOperationsImplTest.java
index e293edf..975d7ec 100644
--- a/javatests/com/google/gerrit/acceptance/testsuite/change/ChangeOperationsImplTest.java
+++ b/javatests/com/google/gerrit/acceptance/testsuite/change/ChangeOperationsImplTest.java
@@ -25,6 +25,7 @@
import static com.google.gerrit.truth.OptionalSubject.assertThat;
import com.google.common.collect.ImmutableSet;
+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.project.ProjectOperations;
@@ -43,6 +44,7 @@
import com.google.gerrit.extensions.common.RevisionInfo;
import com.google.gerrit.extensions.restapi.BinaryResult;
import com.google.gerrit.extensions.restapi.RestApiException;
+import com.google.gerrit.truth.NullAwareCorrespondence;
import com.google.inject.Inject;
import java.util.Map;
import org.eclipse.jgit.lib.ObjectId;
@@ -154,7 +156,417 @@
.parents()
.onlyElement()
.commit()
- .isEqualTo(parentCommitId.getName());
+ .isEqualTo(parentCommitId.name());
+ }
+
+ @Test
+ public void createdChangeUsesSpecifiedBranchTipAsParent() throws Exception {
+ Project.NameKey project = projectOperations.newProject().branches("test-branch").create();
+
+ Change.Id changeId =
+ changeOperations
+ .newChange()
+ .project(project)
+ .childOf()
+ .tipOfBranch("refs/heads/test-branch")
+ .create();
+
+ ChangeInfo change = getChangeFromServer(changeId);
+ CommitInfo currentPatchsetCommit = change.revisions.get(change.currentRevision).commit;
+ ObjectId parentCommitId = projectOperations.project(project).getHead("test-branch").getId();
+ assertThat(currentPatchsetCommit)
+ .parents()
+ .onlyElement()
+ .commit()
+ .isEqualTo(parentCommitId.name());
+ }
+
+ @Test
+ public void specifiedParentBranchMayHaveShortName() throws Exception {
+ Project.NameKey project = projectOperations.newProject().branches("test-branch").create();
+
+ Change.Id changeId =
+ changeOperations.newChange().project(project).childOf().tipOfBranch("test-branch").create();
+
+ ChangeInfo change = getChangeFromServer(changeId);
+ CommitInfo currentPatchsetCommit = change.revisions.get(change.currentRevision).commit;
+ ObjectId parentCommitId = projectOperations.project(project).getHead("test-branch").getId();
+ assertThat(currentPatchsetCommit)
+ .parents()
+ .onlyElement()
+ .commit()
+ .isEqualTo(parentCommitId.name());
+ }
+
+ @Test
+ public void specifiedParentBranchMustExist() {
+ IllegalStateException exception =
+ assertThrows(
+ IllegalStateException.class,
+ () ->
+ changeOperations.newChange().childOf().tipOfBranch("not-existing-branch").create());
+ assertThat(exception).hasMessageThat().ignoringCase().contains("parent");
+ }
+
+ @Test
+ public void createdChangeUsesSpecifiedChangeAsParent() throws Exception {
+ Change.Id parentChangeId = changeOperations.newChange().create();
+
+ Change.Id changeId = changeOperations.newChange().childOf().change(parentChangeId).create();
+
+ ChangeInfo change = getChangeFromServer(changeId);
+ CommitInfo currentPatchsetCommit = change.revisions.get(change.currentRevision).commit;
+ ObjectId parentCommitId =
+ changeOperations.change(parentChangeId).currentPatchset().get().commitId();
+ assertThat(currentPatchsetCommit)
+ .parents()
+ .onlyElement()
+ .commit()
+ .isEqualTo(parentCommitId.name());
+ }
+
+ @Test
+ public void specifiedParentChangeMustExist() {
+ IllegalStateException exception =
+ assertThrows(
+ IllegalStateException.class,
+ () -> changeOperations.newChange().childOf().change(Change.id(987654321)).create());
+ assertThat(exception).hasMessageThat().ignoringCase().contains("parent");
+ }
+
+ @Test
+ public void createdChangeUsesSpecifiedPatchsetAsParent() throws Exception {
+ Change.Id parentChangeId = changeOperations.newChange().create();
+ TestPatchset parentPatchset = changeOperations.change(parentChangeId).currentPatchset().get();
+
+ Change.Id changeId =
+ changeOperations.newChange().childOf().patchset(parentPatchset.patchsetId()).create();
+
+ ChangeInfo change = getChangeFromServer(changeId);
+ CommitInfo currentPatchsetCommit = change.revisions.get(change.currentRevision).commit;
+ assertThat(currentPatchsetCommit)
+ .parents()
+ .onlyElement()
+ .commit()
+ .isEqualTo(parentPatchset.commitId().name());
+ }
+
+ @Test
+ public void changeOfSpecifiedParentPatchsetMustExist() {
+ IllegalStateException exception =
+ assertThrows(
+ IllegalStateException.class,
+ () ->
+ changeOperations
+ .newChange()
+ .childOf()
+ .patchset(PatchSet.id(Change.id(987654321), 1))
+ .create());
+ assertThat(exception).hasMessageThat().ignoringCase().contains("parent");
+ }
+
+ @Test
+ public void specifiedParentPatchsetMustExist() {
+ Change.Id parentChangeId = changeOperations.newChange().create();
+
+ IllegalStateException exception =
+ assertThrows(
+ IllegalStateException.class,
+ () ->
+ changeOperations
+ .newChange()
+ .childOf()
+ .patchset(PatchSet.id(parentChangeId, 1000))
+ .create());
+ assertThat(exception).hasMessageThat().ignoringCase().contains("parent");
+ }
+
+ @Test
+ public void createdChangeUsesSpecifiedCommitAsParent() throws Exception {
+ // Currently, the easiest way to create a commit is by creating another change.
+ Change.Id anotherChangeId = changeOperations.newChange().create();
+ ObjectId parentCommitId =
+ changeOperations.change(anotherChangeId).currentPatchset().get().commitId();
+
+ Change.Id changeId = changeOperations.newChange().childOf().commit(parentCommitId).create();
+
+ ChangeInfo change = getChangeFromServer(changeId);
+ CommitInfo currentPatchsetCommit = change.revisions.get(change.currentRevision).commit;
+ assertThat(currentPatchsetCommit)
+ .parents()
+ .onlyElement()
+ .commit()
+ .isEqualTo(parentCommitId.name());
+ }
+
+ @Test
+ public void specifiedParentCommitMustExist() {
+ IllegalStateException exception =
+ assertThrows(
+ IllegalStateException.class,
+ () ->
+ changeOperations
+ .newChange()
+ .childOf()
+ .commit(ObjectId.fromString("0123456789012345678901234567890123456789"))
+ .create());
+ assertThat(exception).hasMessageThat().ignoringCase().contains("parent");
+ }
+
+ @Test
+ public void createdChangeUsesSpecifiedChangesInGivenOrderAsParents() throws Exception {
+ Change.Id parent1ChangeId = changeOperations.newChange().create();
+ Change.Id parent2ChangeId = changeOperations.newChange().create();
+
+ Change.Id changeId =
+ changeOperations
+ .newChange()
+ .mergeOf()
+ .change(parent1ChangeId)
+ .and()
+ .change(parent2ChangeId)
+ .create();
+
+ ChangeInfo change = getChangeFromServer(changeId);
+ CommitInfo currentPatchsetCommit = change.revisions.get(change.currentRevision).commit;
+ ObjectId parent1CommitId =
+ changeOperations.change(parent1ChangeId).currentPatchset().get().commitId();
+ ObjectId parent2CommitId =
+ changeOperations.change(parent2ChangeId).currentPatchset().get().commitId();
+ assertThat(currentPatchsetCommit)
+ .parents()
+ .comparingElementsUsing(hasSha1())
+ .containsExactly(parent1CommitId.name(), parent2CommitId.name())
+ .inOrder();
+ }
+
+ @Test
+ public void createdChangeUsesMergedParentsAsBaseCommit() throws Exception {
+ Change.Id parent1ChangeId =
+ changeOperations.newChange().file("file1").content("Line 1").create();
+ Change.Id parent2ChangeId =
+ changeOperations.newChange().file("file2").content("Some other content").create();
+
+ Change.Id changeId =
+ changeOperations
+ .newChange()
+ .mergeOf()
+ .change(parent1ChangeId)
+ .and()
+ .change(parent2ChangeId)
+ .create();
+
+ PatchSet.Id patchsetId = changeOperations.change(changeId).currentPatchset().get().patchsetId();
+ BinaryResult file1Content = getFileContent(changeId, patchsetId, "file1");
+ assertThat(file1Content).asString().isEqualTo("Line 1");
+ BinaryResult file2Content = getFileContent(changeId, patchsetId, "file2");
+ assertThat(file2Content).asString().isEqualTo("Some other content");
+ }
+
+ @Test
+ public void mergeConflictsOfParentsAreReported() {
+ Change.Id parent1ChangeId =
+ changeOperations.newChange().file("file1").content("Content 1").create();
+ Change.Id parent2ChangeId =
+ changeOperations.newChange().file("file1").content("Content 2").create();
+
+ IllegalStateException exception =
+ assertThrows(
+ IllegalStateException.class,
+ () ->
+ changeOperations
+ .newChange()
+ .mergeOf()
+ .change(parent1ChangeId)
+ .and()
+ .change(parent2ChangeId)
+ .create());
+
+ assertThat(exception).hasMessageThat().ignoringCase().contains("conflict");
+ }
+
+ @Test
+ public void mergeConflictsCanBeAvoidedByUsingTheFirstParentAsBase() throws Exception {
+ Change.Id parent1ChangeId =
+ changeOperations.newChange().file("file1").content("Content 1").create();
+ Change.Id parent2ChangeId =
+ changeOperations.newChange().file("file1").content("Content 2").create();
+
+ Change.Id changeId =
+ changeOperations
+ .newChange()
+ .mergeOfButBaseOnFirst()
+ .change(parent1ChangeId)
+ .and()
+ .change(parent2ChangeId)
+ .create();
+
+ PatchSet.Id patchsetId = changeOperations.change(changeId).currentPatchset().get().patchsetId();
+ BinaryResult file1Content = getFileContent(changeId, patchsetId, "file1");
+ assertThat(file1Content).asString().isEqualTo("Content 1");
+ }
+
+ @Test
+ public void createdChangeHasAllParentsEvenWhenBasedOnFirst() throws Exception {
+ Change.Id parent1ChangeId = changeOperations.newChange().create();
+ Change.Id parent2ChangeId = changeOperations.newChange().create();
+
+ Change.Id changeId =
+ changeOperations
+ .newChange()
+ .mergeOfButBaseOnFirst()
+ .change(parent1ChangeId)
+ .and()
+ .change(parent2ChangeId)
+ .create();
+
+ ChangeInfo change = getChangeFromServer(changeId);
+ CommitInfo currentPatchsetCommit = change.revisions.get(change.currentRevision).commit;
+ ObjectId parent1CommitId =
+ changeOperations.change(parent1ChangeId).currentPatchset().get().commitId();
+ ObjectId parent2CommitId =
+ changeOperations.change(parent2ChangeId).currentPatchset().get().commitId();
+ assertThat(currentPatchsetCommit)
+ .parents()
+ .comparingElementsUsing(hasSha1())
+ .containsExactly(parent1CommitId.name(), parent2CommitId.name())
+ .inOrder();
+ }
+
+ @Test
+ public void automaticMergeOfMoreThanTwoParentsIsNotPossible() {
+ Change.Id parent1ChangeId =
+ changeOperations.newChange().file("file1").content("Content 1").create();
+ Change.Id parent2ChangeId =
+ changeOperations.newChange().file("file2").content("Content 2").create();
+ Change.Id parent3ChangeId =
+ changeOperations.newChange().file("file3").content("Content 3").create();
+
+ IllegalStateException exception =
+ assertThrows(
+ IllegalStateException.class,
+ () ->
+ changeOperations
+ .newChange()
+ .mergeOf()
+ .change(parent1ChangeId)
+ .followedBy()
+ .change(parent2ChangeId)
+ .and()
+ .change(parent3ChangeId)
+ .create());
+
+ assertThat(exception).hasMessageThat().ignoringCase().contains("conflict");
+ }
+
+ @Test
+ public void createdChangeCanHaveMoreThanTwoParentsWhenBasedOnFirst() throws Exception {
+ Change.Id parent1ChangeId =
+ changeOperations.newChange().file("file1").content("Content 1").create();
+ Change.Id parent2ChangeId =
+ changeOperations.newChange().file("file2").content("Content 2").create();
+ Change.Id parent3ChangeId =
+ changeOperations.newChange().file("file3").content("Content 3").create();
+
+ Change.Id changeId =
+ changeOperations
+ .newChange()
+ .mergeOfButBaseOnFirst()
+ .change(parent1ChangeId)
+ .followedBy()
+ .change(parent2ChangeId)
+ .and()
+ .change(parent3ChangeId)
+ .create();
+
+ ChangeInfo change = getChangeFromServer(changeId);
+ CommitInfo currentPatchsetCommit = change.revisions.get(change.currentRevision).commit;
+ ObjectId parent1CommitId =
+ changeOperations.change(parent1ChangeId).currentPatchset().get().commitId();
+ ObjectId parent2CommitId =
+ changeOperations.change(parent2ChangeId).currentPatchset().get().commitId();
+ ObjectId parent3CommitId =
+ changeOperations.change(parent3ChangeId).currentPatchset().get().commitId();
+ assertThat(currentPatchsetCommit)
+ .parents()
+ .comparingElementsUsing(hasSha1())
+ .containsExactly(parent1CommitId.name(), parent2CommitId.name(), parent3CommitId.name())
+ .inOrder();
+ }
+
+ @Test
+ public void changeBasedOnParentMayHaveAdditionalFileModifications() throws Exception {
+ Change.Id parentChangeId =
+ changeOperations
+ .newChange()
+ .file("file1")
+ .content("Content 1")
+ .file("file2")
+ .content("Content 2")
+ .create();
+
+ Change.Id changeId =
+ changeOperations
+ .newChange()
+ .childOf()
+ .change(parentChangeId)
+ .file("file1")
+ .content("Different content")
+ .create();
+
+ PatchSet.Id patchsetId = changeOperations.change(changeId).currentPatchset().get().patchsetId();
+ BinaryResult file1Content = getFileContent(changeId, patchsetId, "file1");
+ assertThat(file1Content).asString().isEqualTo("Different content");
+ BinaryResult file2Content = getFileContent(changeId, patchsetId, "file2");
+ assertThat(file2Content).asString().isEqualTo("Content 2");
+ }
+
+ @Test
+ public void changeFromMergedParentsMayHaveAdditionalFileModifications() throws Exception {
+ Change.Id parent1ChangeId =
+ changeOperations.newChange().file("file1").content("Content 1").create();
+ Change.Id parent2ChangeId =
+ changeOperations.newChange().file("file2").content("Content 2").create();
+
+ Change.Id changeId =
+ changeOperations
+ .newChange()
+ .mergeOf()
+ .change(parent1ChangeId)
+ .and()
+ .change(parent2ChangeId)
+ .file("file1")
+ .content("Different content")
+ .create();
+
+ PatchSet.Id patchsetId = changeOperations.change(changeId).currentPatchset().get().patchsetId();
+ BinaryResult file1Content = getFileContent(changeId, patchsetId, "file1");
+ assertThat(file1Content).asString().isEqualTo("Different content");
+ BinaryResult file2Content = getFileContent(changeId, patchsetId, "file2");
+ assertThat(file2Content).asString().isEqualTo("Content 2");
+ }
+
+ @Test
+ public void changeBasedOnFirstOfMultipleParentsMayHaveAdditionalFileModifications()
+ throws Exception {
+ Change.Id parent1ChangeId =
+ changeOperations.newChange().file("file1").content("Content 1").create();
+ Change.Id parent2ChangeId = changeOperations.newChange().create();
+
+ Change.Id changeId =
+ changeOperations
+ .newChange()
+ .mergeOfButBaseOnFirst()
+ .change(parent1ChangeId)
+ .and()
+ .change(parent2ChangeId)
+ .file("file1")
+ .content("Different content")
+ .create();
+
+ PatchSet.Id patchsetId = changeOperations.change(changeId).currentPatchset().get().patchsetId();
+ BinaryResult file1Content = getFileContent(changeId, patchsetId, "file1");
+ assertThat(file1Content).asString().isEqualTo("Different content");
}
@Test
@@ -626,6 +1038,105 @@
}
@Test
+ public void newPatchsetCanHaveADifferentParent() throws Exception {
+ Change.Id originalParentChange = changeOperations.newChange().create();
+ Change.Id changeId =
+ changeOperations.newChange().childOf().change(originalParentChange).create();
+ Change.Id newParentChange = changeOperations.newChange().create();
+
+ changeOperations.change(changeId).newPatchset().parent().change(newParentChange).create();
+
+ ChangeInfo change = getChangeFromServer(changeId);
+ CommitInfo currentPatchsetCommit = change.revisions.get(change.currentRevision).commit;
+ ObjectId newParentCommitId =
+ changeOperations.change(newParentChange).currentPatchset().get().commitId();
+ assertThat(currentPatchsetCommit)
+ .parents()
+ .onlyElement()
+ .commit()
+ .isEqualTo(newParentCommitId.name());
+ }
+
+ @Test
+ public void newPatchsetCanHaveDifferentParents() throws Exception {
+ Change.Id originalParent1Change = changeOperations.newChange().create();
+ Change.Id originalParent2Change = changeOperations.newChange().create();
+ Change.Id changeId =
+ changeOperations
+ .newChange()
+ .mergeOf()
+ .change(originalParent1Change)
+ .and()
+ .change(originalParent2Change)
+ .create();
+ Change.Id newParent1Change = changeOperations.newChange().create();
+ Change.Id newParent2Change = changeOperations.newChange().create();
+
+ changeOperations
+ .change(changeId)
+ .newPatchset()
+ .parents()
+ .change(newParent1Change)
+ .and()
+ .change(newParent2Change)
+ .create();
+
+ ChangeInfo change = getChangeFromServer(changeId);
+ CommitInfo currentPatchsetCommit = change.revisions.get(change.currentRevision).commit;
+ ObjectId newParent1CommitId =
+ changeOperations.change(newParent1Change).currentPatchset().get().commitId();
+ ObjectId newParent2CommitId =
+ changeOperations.change(newParent2Change).currentPatchset().get().commitId();
+ assertThat(currentPatchsetCommit)
+ .parents()
+ .comparingElementsUsing(hasSha1())
+ .containsExactly(newParent1CommitId.name(), newParent2CommitId.name());
+ }
+
+ @Test
+ public void newPatchsetCanHaveADifferentNumberOfParents() throws Exception {
+ Change.Id originalParentChange = changeOperations.newChange().create();
+ Change.Id changeId =
+ changeOperations.newChange().childOf().change(originalParentChange).create();
+ Change.Id newParent1Change = changeOperations.newChange().create();
+ Change.Id newParent2Change = changeOperations.newChange().create();
+
+ changeOperations
+ .change(changeId)
+ .newPatchset()
+ .parents()
+ .change(newParent1Change)
+ .and()
+ .change(newParent2Change)
+ .create();
+
+ ChangeInfo change = getChangeFromServer(changeId);
+ CommitInfo currentPatchsetCommit = change.revisions.get(change.currentRevision).commit;
+ ObjectId newParent1CommitId =
+ changeOperations.change(newParent1Change).currentPatchset().get().commitId();
+ ObjectId newParent2CommitId =
+ changeOperations.change(newParent2Change).currentPatchset().get().commitId();
+ assertThat(currentPatchsetCommit)
+ .parents()
+ .comparingElementsUsing(hasSha1())
+ .containsExactly(newParent1CommitId.name(), newParent2CommitId.name());
+ }
+
+ @Test
+ public void newPatchsetKeepsFileContentsWithDifferentParent() throws Exception {
+ Change.Id changeId =
+ changeOperations.newChange().file("file1").content("Actual change content").create();
+ Change.Id newParentChange =
+ changeOperations.newChange().file("file1").content("Parent content").create();
+
+ changeOperations.change(changeId).newPatchset().parent().change(newParentChange).create();
+
+ PatchSet.Id patchsetId = changeOperations.change(changeId).currentPatchset().get().patchsetId();
+ BinaryResult file1Content = getFileContent(changeId, patchsetId, "file1");
+ assertThat(file1Content).asString().isEqualTo("Actual change content");
+ }
+
+ @Test
public void publishedCommentCanBeRetrieved() {
Change.Id changeId = changeOperations.newChange().create();
String commentUuid = changeOperations.change(changeId).currentPatchset().newComment().create();
@@ -721,4 +1232,8 @@
throws RestApiException {
return gApi.changes().id(changeId.get()).revision(patchsetId.get()).file(filePath).content();
}
+
+ private Correspondence<CommitInfo, String> hasSha1() {
+ return NullAwareCorrespondence.transforming(commitInfo -> commitInfo.commit, "hasSha1");
+ }
}
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 7bac47d..2174791 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
@@ -78,7 +78,8 @@
MISMATCH: 'mismatch',
MISSING: 'missing',
};
-const CHANGE_ID_REGEX_PATTERN = /^Change-Id\:\s(I[0-9a-f]{8,40})/gm;
+const CHANGE_ID_REGEX_PATTERN =
+ /^(Change-Id\:\s|Link:.*\/id\/)(I[0-9a-f]{8,40})/gm;
const MIN_LINES_FOR_COMMIT_COLLAPSE = 30;
const DEFAULT_NUM_FILES_SHOWN = 200;
@@ -1325,7 +1326,7 @@
let changeIdArr;
while (changeIdArr = CHANGE_ID_REGEX_PATTERN.exec(commitMessage)) {
- changeId = changeIdArr[1];
+ changeId = changeIdArr[2];
}
if (changeId) {
diff --git a/polygerrit-ui/app/elements/change/gr-file-list-constants.ts b/polygerrit-ui/app/elements/change/gr-file-list-constants.ts
index 7c07e68..0e55494 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list-constants.ts
+++ b/polygerrit-ui/app/elements/change/gr-file-list-constants.ts
@@ -15,10 +15,8 @@
* limitations under the License.
*/
-export const GrFileListConstants = {
- FilesExpandedState: {
- ALL: 'all',
- NONE: 'none',
- SOME: 'some',
- },
-};
+export enum FilesExpandedState {
+ ALL = 'all',
+ NONE = 'none',
+ SOME = 'some',
+}
diff --git a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.js b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.js
deleted file mode 100644
index 05f2ab0..0000000
--- a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.js
+++ /dev/null
@@ -1,302 +0,0 @@
-/**
- * @license
- * Copyright (C) 2017 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 '../../diff/gr-diff-mode-selector/gr-diff-mode-selector.js';
-import '../../diff/gr-patch-range-select/gr-patch-range-select.js';
-import '../../edit/gr-edit-controls/gr-edit-controls.js';
-import '../../shared/gr-editable-label/gr-editable-label.js';
-import '../../shared/gr-linked-chip/gr-linked-chip.js';
-import '../../shared/gr-rest-api-interface/gr-rest-api-interface.js';
-import '../../shared/gr-select/gr-select.js';
-import '../../shared/gr-button/gr-button.js';
-import '../../shared/gr-icons/gr-icons.js';
-import '../gr-commit-info/gr-commit-info.js';
-import {dom} from '@polymer/polymer/lib/legacy/polymer.dom.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-file-list-header_html.js';
-import {KeyboardShortcutMixin} from '../../../mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin.js';
-import {GrFileListConstants} from '../gr-file-list-constants.js';
-import {GerritNav} from '../../core/gr-navigation/gr-navigation.js';
-import {
- computeLatestPatchNum,
- getRevisionByPatchNum,
- patchNumEquals,
-} from '../../../utils/patch-set-util.js';
-
-// Maximum length for patch set descriptions.
-const PATCH_DESC_MAX_LENGTH = 500;
-const MERGED_STATUS = 'MERGED';
-
-/**
- * @extends PolymerElement
- */
-class GrFileListHeader extends KeyboardShortcutMixin(
- GestureEventListeners(
- LegacyElementMixin(
- PolymerElement))) {
- static get template() { return htmlTemplate; }
-
- static get is() { return 'gr-file-list-header'; }
- /**
- * @event expand-diffs
- */
-
- /**
- * @event collapse-diffs
- */
-
- /**
- * @event open-diff-prefs
- */
-
- /**
- * @event open-included-in-dialog
- */
-
- /**
- * @event open-download-dialog
- */
-
- /**
- * @event open-upload-help-dialog
- */
-
- static get properties() {
- return {
- account: Object,
- allPatchSets: Array,
- /** @type {?} */
- change: Object,
- changeNum: String,
- changeUrl: String,
- changeComments: Object,
- commitInfo: Object,
- editMode: Boolean,
- loggedIn: Boolean,
- serverConfig: Object,
- shownFileCount: Number,
- diffPrefs: Object,
- diffPrefsDisabled: Boolean,
- diffViewMode: {
- type: String,
- notify: true,
- },
- patchNum: String,
- basePatchNum: String,
- filesExpanded: String,
- // Caps the number of files that can be shown and have the 'show diffs' /
- // 'hide diffs' buttons still be functional.
- _maxFilesForBulkActions: {
- type: Number,
- readOnly: true,
- value: 225,
- },
- _patchsetDescription: {
- type: String,
- value: '',
- },
- _descriptionReadOnly: {
- type: Boolean,
- computed: '_computeDescriptionReadOnly(loggedIn, change, account)',
- },
- revisionInfo: Object,
- };
- }
-
- static get observers() {
- return [
- '_computePatchSetDescription(change, patchNum)',
- ];
- }
-
- setDiffViewMode(mode) {
- this.$.modeSelect.setMode(mode);
- }
-
- _expandAllDiffs() {
- this._expanded = true;
- this.dispatchEvent(new CustomEvent('expand-diffs', {
- composed: true, bubbles: true,
- }));
- }
-
- _collapseAllDiffs() {
- this._expanded = false;
- this.dispatchEvent(new CustomEvent('collapse-diffs', {
- composed: true, bubbles: true,
- }));
- }
-
- _computeExpandedClass(filesExpanded) {
- const classes = [];
- if (filesExpanded === GrFileListConstants.FilesExpandedState.ALL) {
- classes.push('expanded');
- }
- if (filesExpanded === GrFileListConstants.FilesExpandedState.SOME ||
- filesExpanded === GrFileListConstants.FilesExpandedState.ALL) {
- classes.push('openFile');
- }
- return classes.join(' ');
- }
-
- _computeDescriptionPlaceholder(readOnly) {
- return (readOnly ? 'No' : 'Add') + ' patchset description';
- }
-
- _computeDescriptionReadOnly(loggedIn, change, account) {
- // Polymer 2: check for undefined
- if ([loggedIn, change, account].includes(undefined)) {
- return undefined;
- }
-
- return !(loggedIn && (account._account_id === change.owner._account_id));
- }
-
- _computePatchSetDescription(change, patchNum) {
- // Polymer 2: check for undefined
- if ([change, patchNum].includes(undefined)) {
- return;
- }
-
- const rev = getRevisionByPatchNum(change.revisions, patchNum);
- this._patchsetDescription = (rev && rev.description) ?
- rev.description.substring(0, PATCH_DESC_MAX_LENGTH) : '';
- }
-
- _handleDescriptionRemoved(e) {
- return this._updateDescription('', e);
- }
-
- /**
- * @param {!Object} revisions The revisions object keyed by revision hashes
- * @param {?Object} patchSet A revision already fetched from {revisions}
- * @return {string|undefined} the SHA hash corresponding to the revision.
- */
- _getPatchsetHash(revisions, patchSet) {
- for (const rev in revisions) {
- if (revisions.hasOwnProperty(rev) &&
- revisions[rev] === patchSet) {
- return rev;
- }
- }
- }
-
- _handleDescriptionChanged(e) {
- const desc = e.detail.trim();
- this._updateDescription(desc, e);
- }
-
- /**
- * Update the patchset description with the rest API.
- *
- * @param {string} desc
- * @param {?(Event|Node)} e
- * @return {!Promise}
- */
- _updateDescription(desc, e) {
- const target = dom(e).rootTarget;
- if (target) { target.disabled = true; }
- const rev = getRevisionByPatchNum(this.change.revisions,
- this.patchNum);
- const sha = this._getPatchsetHash(this.change.revisions, rev);
- return this.$.restAPI.setDescription(this.changeNum, this.patchNum, desc)
- .then(res => {
- if (res.ok) {
- if (target) { target.disabled = false; }
- this.set(['change', 'revisions', sha, 'description'], desc);
- this._patchsetDescription = desc;
- }
- })
- .catch(err => {
- if (target) { target.disabled = false; }
- return;
- });
- }
-
- _computePrefsButtonHidden(prefs, diffPrefsDisabled) {
- return diffPrefsDisabled || !prefs;
- }
-
- _fileListActionsVisible(shownFileCount, maxFilesForBulkActions) {
- return shownFileCount <= maxFilesForBulkActions;
- }
-
- _handlePatchChange(e) {
- const {basePatchNum, patchNum} = e.detail;
- if (patchNumEquals(basePatchNum, this.basePatchNum) &&
- patchNumEquals(patchNum, this.patchNum)) { return; }
- GerritNav.navigateToChange(this.change, patchNum, basePatchNum);
- }
-
- _handlePrefsTap(e) {
- e.preventDefault();
- this.dispatchEvent(new CustomEvent('open-diff-prefs', {
- composed: true, bubbles: true,
- }));
- }
-
- _handleIncludedInTap(e) {
- e.preventDefault();
- this.dispatchEvent(new CustomEvent('open-included-in-dialog', {
- composed: true, bubbles: true,
- }));
- }
-
- _handleDownloadTap(e) {
- e.preventDefault();
- e.stopPropagation();
- this.dispatchEvent(
- new CustomEvent('open-download-dialog', {bubbles: false}));
- }
-
- _computeEditModeClass(editMode) {
- return editMode ? 'editMode' : '';
- }
-
- _computePatchInfoClass(patchNum, allPatchSets) {
- const latestNum = computeLatestPatchNum(allPatchSets);
- if (patchNumEquals(patchNum, latestNum)) {
- return '';
- }
- return 'patchInfoOldPatchSet';
- }
-
- _hideIncludedIn(change) {
- return change && change.status === MERGED_STATUS ? '' : 'hide';
- }
-
- _handleUploadTap(e) {
- e.preventDefault();
- e.stopPropagation();
- this.dispatchEvent(
- new CustomEvent('open-upload-help-dialog', {bubbles: false}));
- }
-
- _computeUploadHelpContainerClass(change, account) {
- const changeIsMerged = change && change.status === MERGED_STATUS;
- const ownerId = change && change.owner && change.owner._account_id ?
- change.owner._account_id : null;
- const userId = account && account._account_id;
- const userIsOwner = ownerId && userId && ownerId === userId;
- const hideContainer = !userIsOwner || changeIsMerged;
- return 'uploadContainer desktop' + (hideContainer ? ' hide' : '');
- }
-}
-
-customElements.define(GrFileListHeader.is, GrFileListHeader);
diff --git a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.ts b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.ts
new file mode 100644
index 0000000..fb91806
--- /dev/null
+++ b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.ts
@@ -0,0 +1,403 @@
+/**
+ * @license
+ * Copyright (C) 2017 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 '../../diff/gr-diff-mode-selector/gr-diff-mode-selector';
+import '../../diff/gr-patch-range-select/gr-patch-range-select';
+import '../../edit/gr-edit-controls/gr-edit-controls';
+import '../../shared/gr-editable-label/gr-editable-label';
+import '../../shared/gr-linked-chip/gr-linked-chip';
+import '../../shared/gr-rest-api-interface/gr-rest-api-interface';
+import '../../shared/gr-select/gr-select';
+import '../../shared/gr-button/gr-button';
+import '../../shared/gr-icons/gr-icons';
+import '../gr-commit-info/gr-commit-info';
+import {dom, EventApi} from '@polymer/polymer/lib/legacy/polymer.dom';
+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-file-list-header_html';
+import {KeyboardShortcutMixin} from '../../../mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin';
+import {FilesExpandedState} from '../gr-file-list-constants';
+import {GerritNav} from '../../core/gr-navigation/gr-navigation';
+import {
+ computeLatestPatchNum,
+ getRevisionByPatchNum,
+ patchNumEquals,
+ PatchSet,
+} from '../../../utils/patch-set-util';
+import {property, computed, observe, customElement} from '@polymer/decorators';
+import {
+ AccountInfo,
+ ChangeInfo,
+ PatchSetNum,
+ CommitInfo,
+ ServerInfo,
+ DiffPreferencesInfo,
+ RevisionInfo,
+} from '../../../types/common';
+import {ChangeComments} from '../../diff/gr-comment-api/gr-comment-api';
+import {GrDiffModeSelector} from '../../diff/gr-diff-mode-selector/gr-diff-mode-selector';
+import {DiffViewMode} from '../../../constants/constants';
+import {RestApiService} from '../../../services/services/gr-rest-api/gr-rest-api';
+import {ChangeNum} from '../../shared/gr-rest-api-interface/gr-rest-api-interface';
+import {GrButton} from '../../shared/gr-button/gr-button';
+
+// Maximum length for patch set descriptions.
+const PATCH_DESC_MAX_LENGTH = 500;
+const MERGED_STATUS = 'MERGED';
+
+declare global {
+ interface HTMLElementTagNameMap {
+ 'gr-file-list-header': GrFileListHeader;
+ }
+}
+
+export interface GrFileListHeader {
+ $: {
+ modeSelect: GrDiffModeSelector;
+ restAPI: RestApiService & Element;
+ expandBtn: GrButton;
+ collapseBtn: GrButton;
+ };
+}
+
+@customElement('gr-file-list-header')
+export class GrFileListHeader extends KeyboardShortcutMixin(
+ GestureEventListeners(LegacyElementMixin(PolymerElement))
+) {
+ static get template() {
+ return htmlTemplate;
+ }
+
+ /**
+ * @event expand-diffs
+ */
+
+ /**
+ * @event collapse-diffs
+ */
+
+ /**
+ * @event open-diff-prefs
+ */
+
+ /**
+ * @event open-included-in-dialog
+ */
+
+ /**
+ * @event open-download-dialog
+ */
+
+ /**
+ * @event open-upload-help-dialog
+ */
+
+ @property({type: Object})
+ account: AccountInfo | undefined;
+
+ @property({type: Array})
+ allPatchSets?: PatchSet[];
+
+ @property({type: Object})
+ change: ChangeInfo | undefined;
+
+ @property({type: String})
+ changeNum?: ChangeNum;
+
+ @property({type: String})
+ changeUrl?: string;
+
+ @property({type: Object})
+ changeComments?: ChangeComments;
+
+ @property({type: Object})
+ commitInfo?: CommitInfo;
+
+ @property({type: Boolean})
+ editMode?: boolean;
+
+ @property({type: Boolean})
+ loggedIn: boolean | undefined;
+
+ @property({type: Object})
+ serverConfig?: ServerInfo;
+
+ @property({type: Number})
+ shownFileCount?: number;
+
+ @property({type: Object})
+ diffPrefs?: DiffPreferencesInfo;
+
+ @property({type: Boolean})
+ diffPrefsDisabled?: boolean;
+
+ @property({type: String, notify: true})
+ diffViewMode?: DiffViewMode;
+
+ @property({type: String})
+ patchNum?: PatchSetNum;
+
+ @property({type: String})
+ basePatchNum?: PatchSetNum;
+
+ @property({type: String})
+ filesExpanded?: FilesExpandedState;
+
+ // Caps the number of files that can be shown and have the 'show diffs' /
+ // 'hide diffs' buttons still be functional.
+ @property({type: Number})
+ readonly _maxFilesForBulkActions = 225;
+
+ @property({type: String})
+ _patchsetDescription = '';
+
+ @property({type: Object})
+ revisionInfo?: RevisionInfo;
+
+ @computed('loggedIn', 'change', 'account')
+ get _descriptionReadOnly(): boolean {
+ // Polymer 2: check for undefined
+ if (
+ this.loggedIn === undefined ||
+ this.change === undefined ||
+ this.account === undefined
+ ) {
+ return false;
+ }
+
+ return !(
+ this.loggedIn &&
+ this.account._account_id === this.change.owner._account_id
+ );
+ }
+
+ setDiffViewMode(mode: DiffViewMode) {
+ this.$.modeSelect.setMode(mode);
+ }
+
+ _expandAllDiffs() {
+ this.dispatchEvent(
+ new CustomEvent('expand-diffs', {
+ composed: true,
+ bubbles: true,
+ })
+ );
+ }
+
+ _collapseAllDiffs() {
+ this.dispatchEvent(
+ new CustomEvent('collapse-diffs', {
+ composed: true,
+ bubbles: true,
+ })
+ );
+ }
+
+ _computeExpandedClass(filesExpanded: FilesExpandedState) {
+ const classes = [];
+ if (filesExpanded === FilesExpandedState.ALL) {
+ classes.push('expanded');
+ }
+ if (
+ filesExpanded === FilesExpandedState.SOME ||
+ filesExpanded === FilesExpandedState.ALL
+ ) {
+ classes.push('openFile');
+ }
+ return classes.join(' ');
+ }
+
+ _computeDescriptionPlaceholder(readOnly: boolean) {
+ return (readOnly ? 'No' : 'Add') + ' patchset description';
+ }
+
+ @observe('change', 'patchNum')
+ _computePatchSetDescription(change: ChangeInfo, patchNum: PatchSetNum) {
+ // Polymer 2: check for undefined
+ if (
+ change === undefined ||
+ change.revisions === undefined ||
+ patchNum === undefined
+ ) {
+ return;
+ }
+
+ const rev = getRevisionByPatchNum(
+ Object.values(change.revisions),
+ patchNum
+ );
+ this._patchsetDescription = rev?.description
+ ? rev.description.substring(0, PATCH_DESC_MAX_LENGTH)
+ : '';
+ }
+
+ _handleDescriptionRemoved(e: CustomEvent) {
+ return this._updateDescription('', e);
+ }
+
+ /**
+ * @param revisions The revisions object keyed by revision hashes
+ * @param patchSet A revision already fetched from {revisions}
+ * @return the SHA hash corresponding to the revision.
+ */
+ _getPatchsetHash(
+ revisions: {[revisionId: string]: RevisionInfo},
+ patchSet: RevisionInfo
+ ) {
+ for (const sha of Object.keys(revisions)) {
+ if (revisions[sha] === patchSet) {
+ return sha;
+ }
+ }
+ throw new Error('patchset hash not found');
+ }
+
+ _handleDescriptionChanged(e: CustomEvent) {
+ const desc = e.detail.trim();
+ this._updateDescription(desc, e);
+ }
+
+ /**
+ * Update the patchset description with the rest API.
+ */
+ _updateDescription(desc: string, e: CustomEvent) {
+ if (
+ !this.change ||
+ !this.change.revisions ||
+ !this.patchNum ||
+ !this.changeNum
+ )
+ return;
+ // target can be either gr-editable-label or gr-linked-chip
+ const target = (dom(e) as EventApi).rootTarget as HTMLElement & {
+ disabled: boolean;
+ };
+ if (target) {
+ target.disabled = true;
+ }
+ const rev = getRevisionByPatchNum(
+ Object.values(this.change.revisions),
+ this.patchNum
+ )!;
+ const sha = this._getPatchsetHash(this.change.revisions, rev);
+ return this.$.restAPI
+ .setDescription(this.changeNum, this.patchNum, desc)
+ .then((res: Response) => {
+ if (res.ok) {
+ if (target) {
+ target.disabled = false;
+ }
+ this.set(['change', 'revisions', sha, 'description'], desc);
+ this._patchsetDescription = desc;
+ }
+ })
+ .catch(() => {
+ if (target) {
+ target.disabled = false;
+ }
+ return;
+ });
+ }
+
+ _computePrefsButtonHidden(
+ prefs: DiffPreferencesInfo,
+ diffPrefsDisabled: boolean
+ ) {
+ return diffPrefsDisabled || !prefs;
+ }
+
+ _fileListActionsVisible(
+ shownFileCount: number,
+ maxFilesForBulkActions: number
+ ) {
+ return shownFileCount <= maxFilesForBulkActions;
+ }
+
+ _handlePatchChange(e: CustomEvent) {
+ const {basePatchNum, patchNum} = e.detail;
+ if (
+ (patchNumEquals(basePatchNum, this.basePatchNum) &&
+ patchNumEquals(patchNum, this.patchNum)) ||
+ !this.change
+ ) {
+ return;
+ }
+ GerritNav.navigateToChange(this.change, patchNum, basePatchNum);
+ }
+
+ _handlePrefsTap(e: Event) {
+ e.preventDefault();
+ this.dispatchEvent(
+ new CustomEvent('open-diff-prefs', {
+ composed: true,
+ bubbles: true,
+ })
+ );
+ }
+
+ _handleIncludedInTap(e: Event) {
+ e.preventDefault();
+ this.dispatchEvent(
+ new CustomEvent('open-included-in-dialog', {
+ composed: true,
+ bubbles: true,
+ })
+ );
+ }
+
+ _handleDownloadTap(e: Event) {
+ e.preventDefault();
+ e.stopPropagation();
+ this.dispatchEvent(
+ new CustomEvent('open-download-dialog', {bubbles: false})
+ );
+ }
+
+ _computeEditModeClass(editMode?: boolean) {
+ return editMode ? 'editMode' : '';
+ }
+
+ _computePatchInfoClass(patchNum?: PatchSetNum, allPatchSets?: PatchSet[]) {
+ const latestNum = computeLatestPatchNum(allPatchSets);
+ if (patchNumEquals(patchNum, latestNum)) {
+ return '';
+ }
+ return 'patchInfoOldPatchSet';
+ }
+
+ _hideIncludedIn(change?: ChangeInfo) {
+ return change?.status === MERGED_STATUS ? '' : 'hide';
+ }
+
+ _handleUploadTap(e: Event) {
+ e.preventDefault();
+ e.stopPropagation();
+ this.dispatchEvent(
+ new CustomEvent('open-upload-help-dialog', {bubbles: false})
+ );
+ }
+
+ _computeUploadHelpContainerClass(change: ChangeInfo, account: AccountInfo) {
+ const changeIsMerged = change?.status === MERGED_STATUS;
+ const ownerId = change?.owner?._account_id || null;
+ const userId = account && account._account_id;
+ const userIsOwner = ownerId && userId && ownerId === userId;
+ const hideContainer = !userIsOwner || changeIsMerged;
+ return 'uploadContainer desktop' + (hideContainer ? ' hide' : '');
+ }
+}
diff --git a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header_test.js b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header_test.js
index dd05b9d..af59616 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header_test.js
+++ b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header_test.js
@@ -17,9 +17,10 @@
import '../../../test/common-test-setup-karma.js';
import './gr-file-list-header.js';
-import {GrFileListConstants} from '../gr-file-list-constants.js';
+import {FilesExpandedState} from '../gr-file-list-constants.js';
import {GerritNav} from '../../core/gr-navigation/gr-navigation.js';
import {generateChange} from '../../../test/test-utils.js';
+import 'lodash/lodash.js';
const basicFixture = fixtureFromElement('gr-file-list-header');
@@ -61,12 +62,20 @@
});
test('_computeDescriptionReadOnly', () => {
- assert.equal(element._computeDescriptionReadOnly(false,
- {owner: {_account_id: 1}}, {_account_id: 1}), true);
- assert.equal(element._computeDescriptionReadOnly(true,
- {owner: {_account_id: 0}}, {_account_id: 1}), true);
- assert.equal(element._computeDescriptionReadOnly(true,
- {owner: {_account_id: 1}}, {_account_id: 1}), false);
+ element.loggedIn = false;
+ element.change = {owner: {_account_id: 1}};
+ element.account = {_account_id: 1};
+ assert.equal(element._descriptionReadOnly, true);
+
+ element.loggedIn = true;
+ element.change = {owner: {_account_id: 0}};
+ element.account = {_account_id: 1};
+ assert.equal(element._descriptionReadOnly, true);
+
+ element.loggedIn = true;
+ element.change = {owner: {_account_id: 1}};
+ element.account = {_account_id: 1};
+ assert.equal(element._descriptionReadOnly, false);
});
test('_computeDescriptionPlaceholder', () => {
@@ -96,11 +105,13 @@
owner: {_account_id: 1},
};
element.account = {_account_id: 1};
+ element.owner = {_account_id: 1};
element.loggedIn = true;
flushAsynchronousOperations();
// The element has a description, so the account chip should be visible
+ element.owner = {_account_id: 1};
// and the description label should not exist.
const chip = element.root.querySelector('#descriptionChip');
let label = element.root.querySelector('#descriptionLabel');
@@ -182,13 +193,13 @@
const actions = element.shadowRoot
.querySelector('.fileViewActions');
assert.equal(getComputedStyle(actions).display, 'none');
- element.filesExpanded = GrFileListConstants.FilesExpandedState.SOME;
+ element.filesExpanded = FilesExpandedState.SOME;
flushAsynchronousOperations();
assert.notEqual(getComputedStyle(actions).display, 'none');
- element.filesExpanded = GrFileListConstants.FilesExpandedState.ALL;
+ element.filesExpanded = FilesExpandedState.ALL;
flushAsynchronousOperations();
assert.notEqual(getComputedStyle(actions).display, 'none');
- element.filesExpanded = GrFileListConstants.FilesExpandedState.NONE;
+ element.filesExpanded = FilesExpandedState.NONE;
flushAsynchronousOperations();
assert.equal(getComputedStyle(actions).display, 'none');
});
@@ -200,15 +211,15 @@
const collapseBtn = element.shadowRoot.querySelector('#collapseBtn');
assert.notEqual(getComputedStyle(expandBtn).display, 'none');
assert.equal(getComputedStyle(collapseBtn).display, 'none');
- element.filesExpanded = GrFileListConstants.FilesExpandedState.SOME;
+ element.filesExpanded = FilesExpandedState.SOME;
flushAsynchronousOperations();
assert.notEqual(getComputedStyle(expandBtn).display, 'none');
assert.equal(getComputedStyle(collapseBtn).display, 'none');
- element.filesExpanded = GrFileListConstants.FilesExpandedState.ALL;
+ element.filesExpanded = FilesExpandedState.ALL;
flushAsynchronousOperations();
assert.equal(getComputedStyle(expandBtn).display, 'none');
assert.notEqual(getComputedStyle(collapseBtn).display, 'none');
- element.filesExpanded = GrFileListConstants.FilesExpandedState.NONE;
+ element.filesExpanded = FilesExpandedState.NONE;
flushAsynchronousOperations();
assert.notEqual(getComputedStyle(expandBtn).display, 'none');
assert.equal(getComputedStyle(collapseBtn).display, 'none');
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 aaf0205..bf45eb6 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
@@ -34,7 +34,7 @@
import {htmlTemplate} from './gr-file-list_html.js';
import {asyncForeach} from '../../../utils/async-util.js';
import {KeyboardShortcutMixin, Shortcut} from '../../../mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin.js';
-import {GrFileListConstants} from '../gr-file-list-constants.js';
+import {FilesExpandedState} from '../gr-file-list-constants.js';
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';
@@ -42,7 +42,6 @@
import {appContext} from '../../../services/app-context.js';
import {SpecialFilePath} from '../../../constants/constants.js';
import {descendedFromClass} from '../../../utils/dom-util.js';
-import {getRevisionByPatchNum} from '../../../utils/patch-set-util.js';
import {
addUnmodifiedFiles,
computeDisplayPath,
@@ -51,8 +50,6 @@
specialFilePathCompare,
} from '../../../utils/path-list-util.js';
-// Maximum length for patch set descriptions.
-const PATCH_DESC_MAX_LENGTH = 500;
const WARN_SHOW_ALL_THRESHOLD = 1000;
const LOADING_DEBOUNCE_INTERVAL = 100;
@@ -139,7 +136,7 @@
},
filesExpanded: {
type: String,
- value: GrFileListConstants.FilesExpandedState.NONE,
+ value: FilesExpandedState.NONE,
notify: true,
},
_filesByPath: Object,
@@ -1161,17 +1158,6 @@
this.numFilesShown = this._files.length;
}
- _computePatchSetDescription(revisions, patchNum) {
- // Polymer 2: check for undefined
- if ([revisions, patchNum].includes(undefined)) {
- return '';
- }
-
- const rev = getRevisionByPatchNum(revisions, patchNum);
- return (rev && rev.description) ?
- rev.description.substring(0, PATCH_DESC_MAX_LENGTH) : '';
- }
-
/**
* Get a descriptive label for use in the status indicator's tooltip and
* ARIA label.
@@ -1210,11 +1196,11 @@
_computeExpandedFiles(expandedCount, totalCount) {
if (expandedCount === 0) {
- return GrFileListConstants.FilesExpandedState.NONE;
+ return FilesExpandedState.NONE;
} else if (expandedCount === totalCount) {
- return GrFileListConstants.FilesExpandedState.ALL;
+ return FilesExpandedState.ALL;
}
- return GrFileListConstants.FilesExpandedState.SOME;
+ return FilesExpandedState.SOME;
}
/**
@@ -1575,7 +1561,7 @@
* @return {boolean}
*/
_noDiffsExpanded() {
- return this.filesExpanded === GrFileListConstants.FilesExpandedState.NONE;
+ return this.filesExpanded === FilesExpandedState.NONE;
}
/**
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.js b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.js
index aaa4499..0343b39 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.js
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.js
@@ -21,7 +21,7 @@
import './gr-file-list.js';
import {createCommentApiMockWithTemplateElement} from '../../../test/mocks/comment-api.js';
import {dom} from '@polymer/polymer/lib/legacy/polymer.dom.js';
-import {GrFileListConstants} from '../gr-file-list-constants.js';
+import {FilesExpandedState} from '../gr-file-list-constants.js';
import {GerritNav} from '../../core/gr-navigation/gr-navigation.js';
import {runA11yAudit} from '../../../test/a11y-test-utils.js';
import {html} from '@polymer/polymer/lib/utils/html-tag.js';
@@ -1107,23 +1107,23 @@
};
flushAsynchronousOperations();
assert.equal(element.filesExpanded,
- GrFileListConstants.FilesExpandedState.NONE);
+ FilesExpandedState.NONE);
element.push('_expandedFiles', {path: 'baz.bar'});
flushAsynchronousOperations();
assert.equal(element.filesExpanded,
- GrFileListConstants.FilesExpandedState.SOME);
+ FilesExpandedState.SOME);
element.push('_expandedFiles', {path: 'foo.bar'});
flushAsynchronousOperations();
assert.equal(element.filesExpanded,
- GrFileListConstants.FilesExpandedState.ALL);
+ FilesExpandedState.ALL);
element.collapseAllDiffs();
flushAsynchronousOperations();
assert.equal(element.filesExpanded,
- GrFileListConstants.FilesExpandedState.NONE);
+ FilesExpandedState.NONE);
element.expandAllDiffs();
flushAsynchronousOperations();
assert.equal(element.filesExpanded,
- GrFileListConstants.FilesExpandedState.ALL);
+ FilesExpandedState.ALL);
});
test('_renderInOrder', done => {
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js
index c1ab0280..d7a41f1 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js
@@ -657,14 +657,14 @@
}
if (section === FocusTarget.BODY) {
const textarea = this.$.textarea;
- textarea.async(textarea.getNativeTextarea()
- .focus.bind(textarea.getNativeTextarea()));
+ textarea.async(() => textarea.getNativeTextarea()
+ .focus());
} else if (section === FocusTarget.REVIEWERS) {
const reviewerEntry = this.$.reviewers.focusStart;
- reviewerEntry.async(reviewerEntry.focus);
+ reviewerEntry.async(() => reviewerEntry.focus());
} else if (section === FocusTarget.CCS) {
const ccEntry = this.$.ccs.focusStart;
- ccEntry.async(ccEntry.focus);
+ ccEntry.async(() => ccEntry.focus());
}
}
diff --git a/polygerrit-ui/app/elements/core/gr-router/gr-router.ts b/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
index dad7d4b..6f1bc60 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
@@ -160,6 +160,8 @@
*/
QUERY_LEGACY_SUFFIX: /^\/q\/.+,n,z$/,
+ CHANGE_ID_QUERY: /^\/id\/(I[0-9a-f]{40})$/,
+
// Matches /c/<changeNum>/[<basePatchNum>..][<patchNum>][/].
CHANGE_LEGACY: /^\/c\/(\d+)\/?(((-?\d+|edit)(\.\.(\d+|edit))?))?\/?$/,
CHANGE_NUMBER_LEGACY: /^\/(\d+)\/?/,
@@ -1006,6 +1008,8 @@
this._mapRoute(RoutePattern.QUERY, '_handleQueryRoute');
+ this._mapRoute(RoutePattern.CHANGE_ID_QUERY, '_handleChangeIdQueryRoute');
+
this._mapRoute(RoutePattern.DIFF_LEGACY_LINENUM, '_handleLegacyLinenum');
this._mapRoute(
@@ -1498,6 +1502,16 @@
});
}
+ _handleChangeIdQueryRoute(data: PageContextWithQueryMap) {
+ // TODO(pcc): This will need to indicate that this was a change ID query if
+ // standard queries gain the ability to search places like commit messages
+ // for change IDs.
+ this._setParams({
+ view: GerritNav.View.SEARCH,
+ query: data.params[0],
+ });
+ }
+
_handleQueryLegacySuffixRoute(ctx: PageContextWithQueryMap) {
this._redirect(ctx.path.replace(LEGACY_QUERY_SUFFIX_PATTERN, ''));
}
diff --git a/polygerrit-ui/app/elements/core/gr-router/gr-router_test.js b/polygerrit-ui/app/elements/core/gr-router/gr-router_test.js
index a931aac..5ee58c9 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router_test.js
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router_test.js
@@ -182,6 +182,7 @@
'_handleBranchListFilterOffsetRoute',
'_handleBranchListFilterRoute',
'_handleBranchListOffsetRoute',
+ '_handleChangeIdQueryRoute',
'_handleChangeNumberLegacyRoute',
'_handleChangeRoute',
'_handleCommentRoute',
@@ -739,6 +740,14 @@
});
});
+ test('_handleChangeIdQueryRoute', () => {
+ const data = {params: ['I0123456789abcdef0123456789abcdef01234567']};
+ assertDataToParams(data, '_handleChangeIdQueryRoute', {
+ view: GerritNav.View.SEARCH,
+ query: 'I0123456789abcdef0123456789abcdef01234567',
+ });
+ });
+
suite('_handleRegisterRoute', () => {
test('happy path', () => {
const ctx = {params: ['/foo/bar']};
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-image.ts b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-image.ts
index f3de371..15264ea 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-image.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-image.ts
@@ -52,8 +52,7 @@
// TODO(kaspern): Support blame for image diffs and remove the hardcoded 4
// column limit.
td.setAttribute('colspan', '4');
- const endpoint = this._createElement('gr-endpoint-decorator');
- const endpointDomApi = endpoint;
+ const endpointDomApi = this._createElement('gr-endpoint-decorator');
endpointDomApi.setAttribute('name', 'image-diff');
endpointDomApi.appendChild(
this._createEndpointParam('baseImage', this._baseImage)
@@ -61,7 +60,7 @@
endpointDomApi.appendChild(
this._createEndpointParam('revisionImage', this._revisionImage)
);
- td.appendChild(endpoint);
+ td.appendChild(endpointDomApi);
tr.appendChild(td);
tbody.appendChild(tr);
return tbody;
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 73ad899..314c322 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
@@ -52,7 +52,6 @@
isMagicPath, specialFilePathCompare,
} from '../../../utils/path-list-util.js';
import {changeBaseURL, changeIsOpen} from '../../../utils/change-util.js';
-import {KnownExperimentId} from '../../../services/flags/flags.js';
const ERR_REVIEW_STATUS = 'Couldn’t change file review status.';
const MSG_LOADING_BLAME = 'Loading blame...';
@@ -308,8 +307,6 @@
/** @override */
attached() {
super.attached();
- this._isChangeCommentsLinkExperimentEnabled = this.flagsService
- .isEnabled(KnownExperimentId.PATCHSET_CHOICE_FOR_COMMENT_LINKS);
this._getLoggedIn().then(loggedIn => {
this._loggedIn = loggedIn;
});
@@ -960,8 +957,7 @@
this._change, this._path, this._patchRange.basePatchNum);
return;
}
- if (this._isChangeCommentsLinkExperimentEnabled &&
- !!value.commentLink) {
+ if (value.commentLink) {
this._displayToasts();
}
// If the blame was loaded for a previous file and user navigates to
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 f0b0dcd..e4dd52e 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
@@ -207,7 +207,6 @@
sinon.spy(element, '_paramsChanged');
sinon.stub(element, '_getChangeDetail').returns(Promise.resolve(
generateChange({revisionsCount: 11})));
- element._isChangeCommentsLinkExperimentEnabled = true;
element.params = {
view: GerritNav.View.DIFF,
changeNum: '42',
@@ -269,7 +268,6 @@
sinon.spy(element, '_paramsChanged');
sinon.stub(element, '_getChangeDetail').returns(Promise.resolve(
generateChange({revisionsCount: 11})));
- element._isChangeCommentsLinkExperimentEnabled = true;
element.params = {
view: GerritNav.View.DIFF,
changeNum: '42',
diff --git a/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select.ts b/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select.ts
index 3e5cfb5..b6e1885 100644
--- a/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select.ts
+++ b/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select.ts
@@ -405,7 +405,7 @@
addFrontSpace?: boolean
) {
const rev = getRevisionByPatchNum(revisions, patchNum);
- return rev && rev.description
+ return rev?.description
? (addFrontSpace ? ' ' : '') +
rev.description.substring(0, PATCH_DESC_MAX_LENGTH)
: '';
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 30ff123..07d37fa 100644
--- a/polygerrit-ui/app/elements/gr-app-global-var-init.js
+++ b/polygerrit-ui/app/elements/gr-app-global-var-init.js
@@ -35,7 +35,6 @@
import {GrChangeActionsInterface} from './shared/gr-js-api-interface/gr-change-actions-js-api.js';
import {GrChangeReplyInterface} from './shared/gr-js-api-interface/gr-change-reply-js-api.js';
import {GrEditConstants} from './edit/gr-edit-constants.js';
-import {GrFileListConstants} from './change/gr-file-list-constants.js';
import {GrDomHooksManager, GrDomHook} from './plugins/gr-dom-hooks/gr-dom-hooks.js';
import {GrEtagDecorator} from './shared/gr-rest-api-interface/gr-etag-decorator.js';
import {GrThemeApi} from './plugins/gr-theme-api/gr-theme-api.js';
@@ -92,7 +91,6 @@
window.GrChangeActionsInterface = GrChangeActionsInterface;
window.GrChangeReplyInterface = GrChangeReplyInterface;
window.GrEditConstants = GrEditConstants;
- window.GrFileListConstants = GrFileListConstants;
window.GrDomHooksManager = GrDomHooksManager;
window.GrDomHook = GrDomHook;
window.GrEtagDecorator = GrEtagDecorator;
diff --git a/polygerrit-ui/app/elements/plugins/gr-endpoint-decorator/gr-endpoint-decorator.ts b/polygerrit-ui/app/elements/plugins/gr-endpoint-decorator/gr-endpoint-decorator.ts
index 674297d..3a83729 100644
--- a/polygerrit-ui/app/elements/plugins/gr-endpoint-decorator/gr-endpoint-decorator.ts
+++ b/polygerrit-ui/app/elements/plugins/gr-endpoint-decorator/gr-endpoint-decorator.ts
@@ -114,6 +114,7 @@
}
const expectProperties = this._getEndpointParams().map(paramEl => {
const helper = plugin.attributeHelper(paramEl);
+ // TODO: this should be replaced by accessing the property directly
const paramName = paramEl.getAttribute('name');
if (!paramName) throw Error('plugin endpoint parameter missing a name');
return helper
diff --git a/polygerrit-ui/app/elements/plugins/gr-endpoint-param/gr-endpoint-param.ts b/polygerrit-ui/app/elements/plugins/gr-endpoint-param/gr-endpoint-param.ts
index 41964d5..9e9f348 100644
--- a/polygerrit-ui/app/elements/plugins/gr-endpoint-param/gr-endpoint-param.ts
+++ b/polygerrit-ui/app/elements/plugins/gr-endpoint-param/gr-endpoint-param.ts
@@ -29,7 +29,7 @@
export class GrEndpointParam extends GestureEventListeners(
LegacyElementMixin(PolymerElement)
) {
- @property({type: String})
+ @property({type: String, reflectToAttribute: true})
name = '';
@property({
diff --git a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.js b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.js
index 3b7dbfe..5de7a66 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.js
+++ b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.js
@@ -29,7 +29,6 @@
import {appContext} from '../../../services/app-context.js';
import {SpecialFilePath} from '../../../constants/constants.js';
import {computeDisplayPath} from '../../../utils/path-list-util.js';
-import {KnownExperimentId} from '../../../services/flags/flags.js';
const UNRESOLVED_EXPAND_COUNT = 5;
const NEWLINE_PATTERN = /\n/g;
@@ -192,8 +191,6 @@
this._getLoggedIn().then(loggedIn => {
this._showActions = loggedIn;
});
- this._isChangeCommentsLinkExperimentEnabled = this.flagsService
- .isEnabled(KnownExperimentId.PATCHSET_CHOICE_FOR_COMMENT_LINKS);
this._setInitialExpandedState();
}
@@ -228,8 +225,7 @@
}
_getDiffUrlForPath(path) {
- if (!this._isChangeCommentsLinkExperimentEnabled ||
- this.comments[0].__draft) {
+ if (this.comments[0].__draft) {
return GerritNav.getUrlForDiffById(this.changeNum,
this.projectName, path, this.patchNum);
}
@@ -238,8 +234,7 @@
}
_getDiffUrlForComment(projectName, changeNum, path, patchNum) {
- if (!this._isChangeCommentsLinkExperimentEnabled ||
- (this.comments.length && this.comments[0].side === 'PARENT') ||
+ if ((this.comments.length && this.comments[0].side === 'PARENT') ||
this.comments[0].__draft) {
return GerritNav.getUrlForDiffById(changeNum,
projectName, path, patchNum, null, this.lineNum);
diff --git a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_test.js b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_test.js
index ab93cb6..a25a4c7 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_test.js
+++ b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_test.js
@@ -186,7 +186,6 @@
element.lineNum = 5;
element.comments = [{id: 'comment_id'}];
element.showFilePath = true;
- element._isChangeCommentsLinkExperimentEnabled = true;
flushAsynchronousOperations();
assert.isOk(element.shadowRoot
.querySelector('.pathInfo'));
diff --git a/polygerrit-ui/app/elements/shared/gr-label-info/gr-label-info.js b/polygerrit-ui/app/elements/shared/gr-label-info/gr-label-info.js
deleted file mode 100644
index a6020cf..0000000
--- a/polygerrit-ui/app/elements/shared/gr-label-info/gr-label-info.js
+++ /dev/null
@@ -1,187 +0,0 @@
-/**
- * @license
- * Copyright (C) 2018 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/gr-voting-styles.js';
-import '../../../styles/shared-styles.js';
-import '../gr-account-label/gr-account-label.js';
-import '../gr-account-link/gr-account-link.js';
-import '../gr-button/gr-button.js';
-import '../gr-icons/gr-icons.js';
-import '../gr-label/gr-label.js';
-import '../gr-rest-api-interface/gr-rest-api-interface.js';
-import {dom} from '@polymer/polymer/lib/legacy/polymer.dom.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-label-info_html.js';
-import {GerritNav} from '../../core/gr-navigation/gr-navigation.js';
-
-/** @extends PolymerElement */
-class GrLabelInfo extends GestureEventListeners(
- LegacyElementMixin(
- PolymerElement)) {
- static get template() { return htmlTemplate; }
-
- static get is() { return 'gr-label-info'; }
-
- static get properties() {
- return {
- labelInfo: Object,
- label: String,
- /** @type {?} */
- change: Object,
- account: Object,
- mutable: Boolean,
- };
- }
-
- /**
- * @param {!Object} labelInfo
- * @param {!Object} account
- * @param {Object} changeLabelsRecord not used, but added as a parameter in
- * order to trigger computation when a label is removed from the change.
- */
- _mapLabelInfo(labelInfo, account, changeLabelsRecord) {
- const result = [];
- if (!labelInfo || !account) { return result; }
- if (!labelInfo.values) {
- if (labelInfo.rejected || labelInfo.approved) {
- const ok = labelInfo.approved || !labelInfo.rejected;
- return [{
- value: ok ? '👍️' : '👎️',
- className: ok ? 'positive' : 'negative',
- account: ok ? labelInfo.approved : labelInfo.rejected,
- }];
- }
- return result;
- }
- // Sort votes by positivity.
- const votes = (labelInfo.all || []).sort((a, b) => a.value - b.value);
- const values = Object.keys(labelInfo.values);
- for (const label of votes) {
- if (label.value && label.value != labelInfo.default_value) {
- let labelClassName;
- let labelValPrefix = '';
- if (label.value > 0) {
- labelValPrefix = '+';
- if (parseInt(label.value, 10) ===
- parseInt(values[values.length - 1], 10)) {
- labelClassName = 'max';
- } else {
- labelClassName = 'positive';
- }
- } else if (label.value < 0) {
- if (parseInt(label.value, 10) === parseInt(values[0], 10)) {
- labelClassName = 'min';
- } else {
- labelClassName = 'negative';
- }
- }
- const formattedLabel = {
- value: labelValPrefix + label.value,
- className: labelClassName,
- account: label,
- };
- if (label._account_id === account._account_id) {
- // Put self-votes at the top.
- result.unshift(formattedLabel);
- } else {
- result.push(formattedLabel);
- }
- }
- }
- return result;
- }
-
- /**
- * A user is able to delete a vote iff the mutable property is true and the
- * reviewer that left the vote exists in the list of removable_reviewers
- * received from the backend.
- *
- * @param {!Object} reviewer An object describing the reviewer that left the
- * vote.
- * @param {boolean} mutable
- * @param {!Object} change
- */
- _computeDeleteClass(reviewer, mutable, change) {
- if (!mutable || !change || !change.removable_reviewers) {
- return 'hidden';
- }
- const removable = change.removable_reviewers;
- if (removable.find(r => r._account_id === reviewer._account_id)) {
- return '';
- }
- return 'hidden';
- }
-
- /**
- * Closure annotation for Polymer.prototype.splice is off.
- * For now, suppressing annotations.
- *
- * @suppress {checkTypes} */
- _onDeleteVote(e) {
- e.preventDefault();
- let target = dom(e).rootTarget;
- while (!target.classList.contains('deleteBtn')) {
- if (!target.parentElement) { return; }
- target = target.parentElement;
- }
-
- target.disabled = true;
- const accountID = parseInt(target.getAttribute('data-account-id'), 10);
- this._xhrPromise =
- this.$.restAPI.deleteVote(this.change._number, accountID, this.label)
- .then(response => {
- target.disabled = false;
- if (!response.ok) { return; }
- GerritNav.navigateToChange(this.change);
- })
- .catch(err => {
- target.disabled = false;
- return;
- });
- }
-
- _computeValueTooltip(labelInfo, score) {
- if (!labelInfo || !labelInfo.values || !labelInfo.values[score]) {
- return '';
- }
- return labelInfo.values[score];
- }
-
- /**
- * @param {!Object} labelInfo
- * @param {Object} changeLabelsRecord not used, but added as a parameter in
- * order to trigger computation when a label is removed from the change.
- */
- _computeShowPlaceholder(labelInfo, changeLabelsRecord) {
- if (labelInfo &&
- !labelInfo.values && (labelInfo.rejected || labelInfo.approved)) {
- return 'hidden';
- }
-
- if (labelInfo && labelInfo.all) {
- for (const label of labelInfo.all) {
- if (label.value && label.value != labelInfo.default_value) {
- return 'hidden';
- }
- }
- }
- return '';
- }
-}
-
-customElements.define(GrLabelInfo.is, GrLabelInfo);
diff --git a/polygerrit-ui/app/elements/shared/gr-label-info/gr-label-info.ts b/polygerrit-ui/app/elements/shared/gr-label-info/gr-label-info.ts
new file mode 100644
index 0000000..b6e664e
--- /dev/null
+++ b/polygerrit-ui/app/elements/shared/gr-label-info/gr-label-info.ts
@@ -0,0 +1,262 @@
+/**
+ * @license
+ * Copyright (C) 2018 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/gr-voting-styles';
+import '../../../styles/shared-styles';
+import '../gr-account-label/gr-account-label';
+import '../gr-account-link/gr-account-link';
+import '../gr-button/gr-button';
+import '../gr-icons/gr-icons';
+import '../gr-label/gr-label';
+import '../gr-rest-api-interface/gr-rest-api-interface';
+import {dom, EventApi} from '@polymer/polymer/lib/legacy/polymer.dom';
+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-label-info_html';
+import {GerritNav} from '../../core/gr-navigation/gr-navigation';
+import {customElement, property} from '@polymer/decorators';
+import {
+ ChangeInfo,
+ AccountInfo,
+ LabelInfo,
+ DetailedLabelInfo,
+ QuickLabelInfo,
+ ApprovalInfo,
+ AccountId,
+} from '../../../types/common';
+import {RestApiService} from '../../../services/services/gr-rest-api/gr-rest-api';
+import {GrButton} from '../gr-button/gr-button';
+
+export interface GrLabelInfo {
+ $: {
+ restAPI: RestApiService & Element;
+ };
+}
+
+declare global {
+ interface HTMLElementTagNameMap {
+ 'gr-label-info': GrLabelInfo;
+ }
+}
+
+enum LabelClassName {
+ NEGATIVE = 'negative',
+ POSITIVE = 'positive',
+ MIN = 'min',
+ MAX = 'max',
+}
+
+interface FormattedLabel {
+ className?: LabelClassName;
+ account: ApprovalInfo;
+ value: string;
+}
+
+// type guard to check if label is QuickLabelInfo
+function isQuickLabelInfo(label: LabelInfo): label is QuickLabelInfo {
+ return !(label as DetailedLabelInfo).values;
+}
+
+@customElement('gr-label-info')
+export class GrLabelInfo extends GestureEventListeners(
+ LegacyElementMixin(PolymerElement)
+) {
+ static get template() {
+ return htmlTemplate;
+ }
+
+ @property({type: Object})
+ labelInfo?: LabelInfo;
+
+ @property({type: String})
+ label = '';
+
+ @property({type: Object})
+ change?: ChangeInfo;
+
+ @property({type: Object})
+ account?: AccountInfo;
+
+ @property({type: Boolean})
+ mutable = false;
+
+ // TODO(TS): not used, remove later
+ _xhrPromise?: Promise<void>;
+
+ /**
+ * This method also listens on change.labels.*,
+ * to trigger computation when a label is removed from the change.
+ */
+ _mapLabelInfo(labelInfo: LabelInfo, account: AccountInfo) {
+ const result: FormattedLabel[] = [];
+ if (!labelInfo || !account) {
+ return result;
+ }
+ if (isQuickLabelInfo(labelInfo)) {
+ if (labelInfo.rejected || labelInfo.approved) {
+ const ok = labelInfo.approved || !labelInfo.rejected;
+ return [
+ {
+ value: ok ? '👍️' : '👎️',
+ className: ok ? LabelClassName.POSITIVE : LabelClassName.NEGATIVE,
+ account: ok ? labelInfo.approved : labelInfo.rejected,
+ },
+ ];
+ }
+ return result;
+ }
+
+ // Sort votes by positivity.
+ // TODO(TS): maybe mark value as required if always present
+ const votes = (labelInfo.all || []).sort(
+ (a, b) => (a.value || 0) - (b.value || 0)
+ );
+ const values = Object.keys(labelInfo.values || {});
+ for (const label of votes) {
+ if (label.value && label.value !== labelInfo.default_value) {
+ let labelClassName;
+ let labelValPrefix = '';
+ if (label.value > 0) {
+ labelValPrefix = '+';
+ if (
+ parseInt(`${label.value}`, 10) ===
+ parseInt(values[values.length - 1], 10)
+ ) {
+ labelClassName = LabelClassName.MAX;
+ } else {
+ labelClassName = LabelClassName.POSITIVE;
+ }
+ } else if (label.value < 0) {
+ if (parseInt(`${label.value}`, 10) === parseInt(values[0], 10)) {
+ labelClassName = LabelClassName.MIN;
+ } else {
+ labelClassName = LabelClassName.NEGATIVE;
+ }
+ }
+ const formattedLabel = {
+ value: `${labelValPrefix}${label.value}`,
+ className: labelClassName,
+ account: label,
+ };
+ if (label._account_id === account._account_id) {
+ // Put self-votes at the top.
+ result.unshift(formattedLabel);
+ } else {
+ result.push(formattedLabel);
+ }
+ }
+ }
+ return result;
+ }
+
+ /**
+ * A user is able to delete a vote iff the mutable property is true and the
+ * reviewer that left the vote exists in the list of removable_reviewers
+ * received from the backend.
+ *
+ * @param reviewer An object describing the reviewer that left the
+ * vote.
+ */
+ _computeDeleteClass(
+ reviewer: ApprovalInfo,
+ mutable: boolean,
+ change: ChangeInfo
+ ) {
+ if (!mutable || !change || !change.removable_reviewers) {
+ return 'hidden';
+ }
+ const removable = change.removable_reviewers;
+ if (removable.find(r => r._account_id === reviewer._account_id)) {
+ return '';
+ }
+ return 'hidden';
+ }
+
+ /**
+ * Closure annotation for Polymer.prototype.splice is off.
+ * For now, suppressing annotations.
+ */
+ _onDeleteVote(e: MouseEvent) {
+ if (!this.change) return;
+
+ e.preventDefault();
+ let target = (dom(e) as EventApi).rootTarget as GrButton;
+ while (!target.classList.contains('deleteBtn')) {
+ if (!target.parentElement) {
+ return;
+ }
+ target = target.parentElement as GrButton;
+ }
+
+ target.disabled = true;
+ const accountID = parseInt(
+ `${target.getAttribute('data-account-id')}`,
+ 10
+ ) as AccountId;
+ this._xhrPromise = this.$.restAPI
+ .deleteVote(this.change._number, accountID, this.label)
+ .then(response => {
+ target.disabled = false;
+ if (!response.ok) {
+ return;
+ }
+ if (this.change) {
+ GerritNav.navigateToChange(this.change);
+ }
+ })
+ .catch(err => {
+ console.warn(err);
+ target.disabled = false;
+ return;
+ });
+ }
+
+ _computeValueTooltip(labelInfo: LabelInfo, score: string) {
+ if (
+ !labelInfo ||
+ isQuickLabelInfo(labelInfo) ||
+ !labelInfo.values?.[score]
+ ) {
+ return '';
+ }
+ return labelInfo.values[score];
+ }
+
+ /**
+ * This method also listens change.labels.* in
+ * order to trigger computation when a label is removed from the change.
+ */
+ _computeShowPlaceholder(labelInfo: LabelInfo) {
+ if (
+ labelInfo &&
+ isQuickLabelInfo(labelInfo) &&
+ (labelInfo.rejected || labelInfo.approved)
+ ) {
+ return 'hidden';
+ }
+
+ // TODO(TS): might replace with hasOwnProperty instead
+ if (labelInfo && (labelInfo as DetailedLabelInfo).all) {
+ for (const label of (labelInfo as DetailedLabelInfo).all || []) {
+ if (label.value && label.value !== labelInfo.default_value) {
+ return 'hidden';
+ }
+ }
+ }
+ return '';
+ }
+}
diff --git a/polygerrit-ui/app/elements/shared/gr-label-info/gr-label-info_test.js b/polygerrit-ui/app/elements/shared/gr-label-info/gr-label-info_test.js
index 0fbaba2..b936237 100644
--- a/polygerrit-ui/app/elements/shared/gr-label-info/gr-label-info_test.js
+++ b/polygerrit-ui/app/elements/shared/gr-label-info/gr-label-info_test.js
@@ -21,7 +21,7 @@
const basicFixture = fixtureFromElement('gr-label-info');
-suite('gr-account-link tests', () => {
+suite('gr-label-info tests', () => {
let element;
setup(() => {
diff --git a/polygerrit-ui/app/services/services/gr-rest-api/gr-rest-api.ts b/polygerrit-ui/app/services/services/gr-rest-api/gr-rest-api.ts
index c6555d0..9fb2882 100644
--- a/polygerrit-ui/app/services/services/gr-rest-api/gr-rest-api.ts
+++ b/polygerrit-ui/app/services/services/gr-rest-api/gr-rest-api.ts
@@ -585,4 +585,14 @@
): Promise<void>;
setAccountStatus(status: string, errFn?: ErrorCallback): Promise<void>;
getAvatarChangeUrl(): Promise<string | undefined>;
+ setDescription(
+ changeNum: ChangeNum,
+ patchNum: PatchSetNum,
+ desc: string
+ ): Promise<Response>;
+ deleteVote(
+ changeNum: ChangeNum,
+ account: AccountId,
+ label: string
+ ): Promise<Response>;
}
diff --git a/polygerrit-ui/app/types/common.ts b/polygerrit-ui/app/types/common.ts
index f4ffcb2..52adb7b 100644
--- a/polygerrit-ui/app/types/common.ts
+++ b/polygerrit-ui/app/types/common.ts
@@ -140,7 +140,7 @@
* The LabelInfo entity contains information about a label on a change, always corresponding to the current patch set.
* https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#label-info
*/
-type LabelInfo = QuickLabelInfo | DetailedLabelInfo;
+export type LabelInfo = QuickLabelInfo | DetailedLabelInfo;
interface LabelCommonInfo {
optional?: boolean; // not set if false
@@ -159,6 +159,7 @@
export interface DetailedLabelInfo extends LabelCommonInfo {
all?: ApprovalInfo[];
values?: LabelValueToDescriptionMap; // A map of all values that are allowed for this label
+ default_value?: number;
}
// https://gerrit-review.googlesource.com/Documentation/rest-api-accounts.html#contributor-agreement-input
diff --git a/polygerrit-ui/app/utils/change-util.ts b/polygerrit-ui/app/utils/change-util.ts
index ba95690..298ae9d 100644
--- a/polygerrit-ui/app/utils/change-util.ts
+++ b/polygerrit-ui/app/utils/change-util.ts
@@ -133,7 +133,7 @@
}
export function changeIsOpen(change?: Change) {
- return change && change.status === ChangeStatus.NEW;
+ return change?.status === ChangeStatus.NEW;
}
export function changeStatuses(
diff --git a/polygerrit-ui/app/utils/patch-set-util.ts b/polygerrit-ui/app/utils/patch-set-util.ts
index 0cef352..ff55e1f 100644
--- a/polygerrit-ui/app/utils/patch-set-util.ts
+++ b/polygerrit-ui/app/utils/patch-set-util.ts
@@ -91,7 +91,7 @@
revisions: RevisionInfo[],
patchNum: PatchSetNum
) {
- for (const rev of Object.values(revisions || {})) {
+ for (const rev of revisions) {
if (patchNumEquals(rev._number, patchNum)) {
return rev;
}
diff --git a/polygerrit-ui/server.go b/polygerrit-ui/server.go
index 2b788a1..12b3516 100644
--- a/polygerrit-ui/server.go
+++ b/polygerrit-ui/server.go
@@ -482,7 +482,7 @@
// Any path prefixes that should resolve to index.html.
var (
- fePaths = []string{"/q/", "/c/", "/p/", "/x/", "/dashboard/", "/admin/", "/settings/"}
+ fePaths = []string{"/q/", "/c/", "/id/", "/p/", "/x/", "/dashboard/", "/admin/", "/settings/"}
issueNumRE = regexp.MustCompile(`^\/\d+\/?$`)
)
diff --git a/resources/com/google/gerrit/server/commit-msg_test.sh b/resources/com/google/gerrit/server/commit-msg_test.sh
index d797be3..4f1a3f7 100755
--- a/resources/com/google/gerrit/server/commit-msg_test.sh
+++ b/resources/com/google/gerrit/server/commit-msg_test.sh
@@ -110,6 +110,25 @@
fi
}
+# gerrit.reviewUrl causes us to create Link instead of Change-Id.
+function test_link {
+ cat << EOF > input
+bla bla
+EOF
+
+ git config gerrit.reviewUrl https://myhost/
+ ${hook} input || fail "failed hook execution"
+ git config --unset gerrit.reviewUrl
+ found=$(grep -c '^Change-Id' input || true)
+ if [[ "${found}" != "0" ]]; then
+ fail "got ${found} Change-Ids, want 0"
+ fi
+ found=$(grep -c '^Link: https://myhost/id/I' input || true)
+ if [[ "${found}" != "1" ]]; then
+ fail "got ${found} Link footers, want 1"
+ fi
+}
+
# Change-Id goes after existing trailers.
function test_at_end {
cat << EOF > input
diff --git a/resources/com/google/gerrit/server/tools/root/hooks/commit-msg b/resources/com/google/gerrit/server/tools/root/hooks/commit-msg
index 2901232..3a40d22 100755
--- a/resources/com/google/gerrit/server/tools/root/hooks/commit-msg
+++ b/resources/com/google/gerrit/server/tools/root/hooks/commit-msg
@@ -48,12 +48,23 @@
exit 1
fi
-# Avoid the --in-place option which only appeared in Git 2.8
-# Avoid the --if-exists option which only appeared in Git 2.15
-if ! git -c trailer.ifexists=doNothing interpret-trailers \
- --trailer "Change-Id: I${random}" < "$1" > "${dest}" ; then
- echo "cannot insert change-id line in $1"
- exit 1
+reviewurl="$(git config --get gerrit.reviewUrl)"
+if test -n "${reviewurl}" ; then
+ if ! git interpret-trailers --parse < "$1" | grep -q '^Link:.*/id/I[0-9a-f]\{40\}$' ; then
+ if ! git interpret-trailers \
+ --trailer "Link: ${reviewurl%/}/id/I${random}" < "$1" > "${dest}" ; then
+ echo "cannot insert link footer in $1"
+ exit 1
+ fi
+ fi
+else
+ # Avoid the --in-place option which only appeared in Git 2.8
+ # Avoid the --if-exists option which only appeared in Git 2.15
+ if ! git -c trailer.ifexists=doNothing interpret-trailers \
+ --trailer "Change-Id: I${random}" < "$1" > "${dest}" ; then
+ echo "cannot insert change-id line in $1"
+ exit 1
+ fi
fi
if ! mv "${dest}" "$1" ; then
