Merge "Fix the vertical spacing of gr-account-chip"
diff --git a/Documentation/config-labels.txt b/Documentation/config-labels.txt
index 4a6b2f3..b808e5c 100644
--- a/Documentation/config-labels.txt
+++ b/Documentation/config-labels.txt
@@ -304,6 +304,12 @@
Matches votes that are equal to the minimal or maximal voting range. Or any votes.
+==== is:'VALUE'
+
+Matches approvals that have a voting value that is equal to 'VALUE'.
+
+Negative values need to be quoted, e.g.: is:"-1"
+
[[approverin]]
==== approverin:link:#group-id[\{group-id\}]
@@ -467,6 +473,9 @@
[[label_copyValue]]
=== `label.Label-Name.copyValue`
+*DEPRECATED: use `is:<value>` predicate in
+link:config-labels.html#label_copyCondition[copyCondition] instead*
+
Value that should be copied forward when a new patch set is uploaded.
This can be used to enable sticky votes. Can be specified multiple
times. By default not set.
diff --git a/java/com/google/gerrit/index/query/QueryBuilder.java b/java/com/google/gerrit/index/query/QueryBuilder.java
index d6a43b9a..987c7d3 100644
--- a/java/com/google/gerrit/index/query/QueryBuilder.java
+++ b/java/com/google/gerrit/index/query/QueryBuilder.java
@@ -366,7 +366,7 @@
* @throws QueryParseException the parser does not recognize this value.
*/
protected Predicate<T> defaultField(String value) throws QueryParseException {
- throw error("Unsupported query:" + value);
+ throw error("Unsupported query: " + value);
}
private List<Predicate<T>> children(Tree r) throws QueryParseException, IllegalArgumentException {
diff --git a/java/com/google/gerrit/server/query/approval/ApprovalQueryBuilder.java b/java/com/google/gerrit/server/query/approval/ApprovalQueryBuilder.java
index 9576b91..11749cc 100644
--- a/java/com/google/gerrit/server/query/approval/ApprovalQueryBuilder.java
+++ b/java/com/google/gerrit/server/query/approval/ApprovalQueryBuilder.java
@@ -14,8 +14,10 @@
package com.google.gerrit.server.query.approval;
+import static java.util.stream.Collectors.joining;
+
import com.google.common.base.Enums;
-import com.google.common.base.Optional;
+import com.google.common.primitives.Ints;
import com.google.gerrit.entities.AccountGroup;
import com.google.gerrit.entities.GroupDescription;
import com.google.gerrit.extensions.client.ChangeKind;
@@ -26,6 +28,7 @@
import com.google.gerrit.server.group.GroupResolver;
import com.google.inject.Inject;
import java.util.Arrays;
+import java.util.Optional;
public class ApprovalQueryBuilder extends QueryBuilder<ApprovalContext, ApprovalQueryBuilder> {
private static final QueryBuilder.Definition<ApprovalContext, ApprovalQueryBuilder> mydef =
@@ -53,13 +56,37 @@
}
@Operator
- public Predicate<ApprovalContext> changekind(String term) throws QueryParseException {
- return new ChangeKindPredicate(toEnumValue(ChangeKind.class, term));
+ public Predicate<ApprovalContext> changekind(String value) throws QueryParseException {
+ return parseEnumValue(ChangeKind.class, value)
+ .map(ChangeKindPredicate::new)
+ .orElseThrow(
+ () ->
+ new QueryParseException(
+ String.format(
+ "%s is not a valid value for operator 'changekind'. Valid values: %s",
+ value, formatEnumValues(ChangeKind.class))));
}
@Operator
- public Predicate<ApprovalContext> is(String term) throws QueryParseException {
- return magicValuePredicate.create(toEnumValue(MagicValuePredicate.MagicValue.class, term));
+ public Predicate<ApprovalContext> is(String value) throws QueryParseException {
+ // try to parse exact value
+ Optional<Integer> exactValue = Optional.ofNullable(Ints.tryParse(value));
+ if (exactValue.isPresent()) {
+ return new ExactValuePredicate(exactValue.get().shortValue());
+ }
+
+ // try to parse magic value
+ Optional<MagicValuePredicate.MagicValue> magicValue =
+ parseEnumValue(MagicValuePredicate.MagicValue.class, value);
+ if (magicValue.isPresent()) {
+ return magicValuePredicate.create(magicValue.get());
+ }
+
+ // it's neither an exact value nor a magic value
+ throw new QueryParseException(
+ String.format(
+ "%s is not a valid value for operator 'is'. Valid values: %s or integer",
+ value, formatEnumValues(MagicValuePredicate.MagicValue.class)));
}
@Operator
@@ -79,20 +106,21 @@
}
throw error(
String.format(
- "'%s' is not a supported argument for has. only 'unchanged-files' is supported",
+ "'%s' is not a valid value for operator 'has'."
+ + " The only valid value is 'unchanged-files'.",
value));
}
- private static <T extends Enum<T>> T toEnumValue(Class<T> clazz, String term)
- throws QueryParseException {
- Optional<T> maybeEnum = Enums.getIfPresent(clazz, term.toUpperCase().replace('-', '_'));
- if (!maybeEnum.isPresent()) {
- throw new QueryParseException(
- String.format(
- "%s is not a valid term. valid options: %s",
- term, Arrays.asList(clazz.getEnumConstants())));
- }
- return maybeEnum.get();
+ private static <T extends Enum<T>> Optional<T> parseEnumValue(Class<T> clazz, String value) {
+ return Optional.ofNullable(
+ Enums.getIfPresent(clazz, value.toUpperCase().replace('-', '_')).orNull());
+ }
+
+ private <T extends Enum<T>> String formatEnumValues(Class<T> clazz) {
+ return Arrays.stream(clazz.getEnumConstants())
+ .map(Object::toString)
+ .sorted()
+ .collect(joining(", "));
}
private AccountGroup.UUID parseGroupOrThrow(String maybeUUID) throws QueryParseException {
diff --git a/java/com/google/gerrit/server/query/approval/ExactValuePredicate.java b/java/com/google/gerrit/server/query/approval/ExactValuePredicate.java
new file mode 100644
index 0000000..1f36f8a
--- /dev/null
+++ b/java/com/google/gerrit/server/query/approval/ExactValuePredicate.java
@@ -0,0 +1,49 @@
+// Copyright (C) 2022 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.query.approval;
+
+import com.google.gerrit.index.query.Predicate;
+import java.util.Collection;
+
+/** Predicate that matches patch set approvals that have a given voting value. */
+public class ExactValuePredicate extends ApprovalPredicate {
+ private final short votingValue;
+
+ public ExactValuePredicate(short votingValue) {
+ this.votingValue = votingValue;
+ }
+
+ @Override
+ public boolean match(ApprovalContext approvalContext) {
+ return votingValue == approvalContext.patchSetApproval().value();
+ }
+
+ @Override
+ public Predicate<ApprovalContext> copy(
+ Collection<? extends Predicate<ApprovalContext>> children) {
+ return new ExactValuePredicate(votingValue);
+ }
+
+ @Override
+ public int hashCode() {
+ return Short.valueOf(votingValue).hashCode();
+ }
+
+ @Override
+ public boolean equals(Object other) {
+ return (other instanceof ExactValuePredicate)
+ && votingValue == ((ExactValuePredicate) other).votingValue;
+ }
+}
diff --git a/javatests/com/google/gerrit/acceptance/api/change/StickyApprovalsIT.java b/javatests/com/google/gerrit/acceptance/api/change/StickyApprovalsIT.java
index 12e3658..dee133b 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/StickyApprovalsIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/StickyApprovalsIT.java
@@ -29,6 +29,7 @@
import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
import static com.google.gerrit.server.project.testing.TestLabels.labelBuilder;
import static com.google.gerrit.server.project.testing.TestLabels.value;
+import static com.google.gerrit.testing.GerritJUnit.assertThrows;
import static java.util.Comparator.comparing;
import com.google.common.cache.Cache;
@@ -38,6 +39,7 @@
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.ExtensionRegistry;
import com.google.gerrit.acceptance.ExtensionRegistry.Registration;
+import com.google.gerrit.acceptance.GitUtil;
import com.google.gerrit.acceptance.NoHttpd;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.TestAccount;
@@ -62,6 +64,7 @@
import com.google.gerrit.extensions.common.ApprovalInfo;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.FileInfo;
+import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.server.change.ChangeKindCacheImpl;
import com.google.gerrit.server.project.testing.TestLabels;
import com.google.gerrit.server.query.change.ChangeData;
@@ -75,7 +78,10 @@
import java.util.Map;
import java.util.Optional;
import java.util.Set;
+import java.util.function.Consumer;
import java.util.stream.Collectors;
+import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
+import org.eclipse.jgit.junit.TestRepository;
import org.eclipse.jgit.lib.ObjectId;
import org.junit.Before;
import org.junit.Test;
@@ -139,33 +145,8 @@
}
@Test
- public void stickyOnAnyScore() throws Exception {
- try (ProjectConfigUpdate u = updateProject(project)) {
- u.getConfig().updateLabelType(LabelId.CODE_REVIEW, b -> b.setCopyAnyScore(true));
- u.save();
- }
-
- for (ChangeKind changeKind :
- EnumSet.of(REWORK, TRIVIAL_REBASE, NO_CODE_CHANGE, MERGE_FIRST_PARENT_UPDATE, NO_CHANGE)) {
- testRepo.reset(projectOperations.project(project).getHead("master"));
-
- String changeId = changeKindCreator.createChange(changeKind, testRepo, admin);
- vote(admin, changeId, 2, 1);
- vote(user, changeId, 1, -1);
-
- changeKindCreator.updateChange(changeId, changeKind, testRepo, admin, project);
- ChangeInfo c = detailedChange(changeId);
- assertVotes(c, admin, 2, 0, changeKind);
- assertVotes(c, user, 1, 0, changeKind);
- }
- }
-
- @Test
public void stickyWhenCopyConditionIsTrue() throws Exception {
- try (ProjectConfigUpdate u = updateProject(project)) {
- u.getConfig().updateLabelType(LabelId.CODE_REVIEW, b -> b.setCopyCondition("is:ANY"));
- u.save();
- }
+ updateCodeReviewLabel(b -> b.setCopyCondition("is:ANY"));
for (ChangeKind changeKind :
EnumSet.of(REWORK, TRIVIAL_REBASE, NO_CODE_CHANGE, MERGE_FIRST_PARENT_UPDATE, NO_CHANGE)) {
@@ -183,32 +164,27 @@
}
@Test
- public void stickyEvenWhenUserCantSeeUploaderInGroup() throws Exception {
- // user can't see admin group
- try (ProjectConfigUpdate u = updateProject(project)) {
- String administratorsUUID = gApi.groups().query("name:Administrators").get().get(0).id;
- u.getConfig()
- .updateLabelType(
- LabelId.CODE_REVIEW, b -> b.setCopyCondition("approverin:" + administratorsUUID));
- u.save();
- }
+ public void stickyOnAnyScore() throws Exception {
+ updateCodeReviewLabel(b -> b.setCopyAnyScore(true));
- String changeId = createChange().getChangeId();
- approve(changeId);
- amendChange(changeId);
- vote(user, changeId, 1, -1); // Invalidate cache
- requestScopeOperations.setApiUser(user.id());
- ChangeInfo c = detailedChange(changeId);
- assertVotes(c, admin, 2, 0);
- assertVotes(c, user, 1, -1);
+ for (ChangeKind changeKind :
+ EnumSet.of(REWORK, TRIVIAL_REBASE, NO_CODE_CHANGE, MERGE_FIRST_PARENT_UPDATE, NO_CHANGE)) {
+ testRepo.reset(projectOperations.project(project).getHead("master"));
+
+ String changeId = changeKindCreator.createChange(changeKind, testRepo, admin);
+ vote(admin, changeId, 2, 1);
+ vote(user, changeId, 1, -1);
+
+ changeKindCreator.updateChange(changeId, changeKind, testRepo, admin, project);
+ ChangeInfo c = detailedChange(changeId);
+ assertVotes(c, admin, 2, 0, changeKind);
+ assertVotes(c, user, 1, 0, changeKind);
+ }
}
@Test
public void stickyOnMinScore() throws Exception {
- try (ProjectConfigUpdate u = updateProject(project)) {
- u.getConfig().updateLabelType(LabelId.CODE_REVIEW, b -> b.setCopyMinScore(true));
- u.save();
- }
+ updateCodeReviewLabel(b -> b.setCopyMinScore(true));
for (ChangeKind changeKind :
EnumSet.of(REWORK, TRIVIAL_REBASE, NO_CODE_CHANGE, MERGE_FIRST_PARENT_UPDATE, NO_CHANGE)) {
@@ -226,35 +202,8 @@
}
@Test
- public void stickyWhenEitherBooleanConfigsOrCopyConditionAreTrue() throws Exception {
- try (ProjectConfigUpdate u = updateProject(project)) {
- u.getConfig()
- .updateLabelType(
- LabelId.CODE_REVIEW, b -> b.setCopyCondition("is:MAX").setCopyMinScore(true));
- u.save();
- }
-
- for (ChangeKind changeKind :
- EnumSet.of(REWORK, TRIVIAL_REBASE, NO_CODE_CHANGE, MERGE_FIRST_PARENT_UPDATE, NO_CHANGE)) {
- testRepo.reset(projectOperations.project(project).getHead("master"));
-
- String changeId = changeKindCreator.createChange(changeKind, testRepo, admin);
- vote(admin, changeId, 2, 1);
- vote(user, changeId, -2, -1);
-
- changeKindCreator.updateChange(changeId, changeKind, testRepo, admin, project);
- ChangeInfo c = detailedChange(changeId);
- assertVotes(c, admin, 2, 0, changeKind);
- assertVotes(c, user, -2, 0, changeKind);
- }
- }
-
- @Test
public void stickyOnMaxScore() throws Exception {
- try (ProjectConfigUpdate u = updateProject(project)) {
- u.getConfig().updateLabelType(LabelId.CODE_REVIEW, b -> b.setCopyMaxScore(true));
- u.save();
- }
+ updateCodeReviewLabel(b -> b.setCopyMaxScore(true));
for (ChangeKind changeKind :
EnumSet.of(REWORK, TRIVIAL_REBASE, NO_CODE_CHANGE, MERGE_FIRST_PARENT_UPDATE, NO_CHANGE)) {
@@ -272,11 +221,953 @@
}
@Test
- public void sticky_copiedToLatestPatchSetFromSubmitRecords() throws Exception {
- try (ProjectConfigUpdate u = updateProject(project)) {
- u.getConfig().updateLabelType(LabelId.VERIFIED, b -> b.setFunction(LabelFunction.NO_BLOCK));
- u.save();
+ public void stickyOnCopyValues() throws Exception {
+ TestAccount user2 = accountCreator.user2();
+ updateCodeReviewLabel(b -> b.setCopyValues(ImmutableList.of((short) -1, (short) 1)));
+
+ for (ChangeKind changeKind :
+ EnumSet.of(REWORK, TRIVIAL_REBASE, NO_CODE_CHANGE, MERGE_FIRST_PARENT_UPDATE, NO_CHANGE)) {
+ testRepo.reset(projectOperations.project(project).getHead("master"));
+
+ String changeId = changeKindCreator.createChange(changeKind, testRepo, admin);
+ vote(admin, changeId, -1, 1);
+ vote(user, changeId, -2, -1);
+ vote(user2, changeId, 1, -1);
+
+ changeKindCreator.updateChange(changeId, changeKind, testRepo, admin, project);
+ ChangeInfo c = detailedChange(changeId);
+ assertVotes(c, admin, -1, 0, changeKind);
+ assertVotes(c, user, 0, 0, changeKind);
+ assertVotes(c, user2, 1, 0, changeKind);
}
+ }
+
+ @Test
+ public void stickyOnTrivialRebase() throws Exception {
+ updateCodeReviewLabel(b -> b.setCopyAllScoresOnTrivialRebase(true));
+
+ String changeId = changeKindCreator.createChange(TRIVIAL_REBASE, testRepo, admin);
+ vote(admin, changeId, 2, 1);
+ vote(user, changeId, -2, -1);
+
+ changeKindCreator.updateChange(changeId, NO_CHANGE, testRepo, admin, project);
+ ChangeInfo c = detailedChange(changeId);
+ assertVotes(c, admin, 2, 0, NO_CHANGE);
+ assertVotes(c, user, -2, 0, NO_CHANGE);
+
+ changeKindCreator.updateChange(changeId, TRIVIAL_REBASE, testRepo, admin, project);
+ c = detailedChange(changeId);
+ assertVotes(c, admin, 2, 0, TRIVIAL_REBASE);
+ assertVotes(c, user, -2, 0, TRIVIAL_REBASE);
+
+ assertNotSticky(EnumSet.of(REWORK, NO_CODE_CHANGE, MERGE_FIRST_PARENT_UPDATE));
+
+ // check that votes are sticky when trivial rebase is done by cherry-pick
+ testRepo.reset(projectOperations.project(project).getHead("master"));
+ changeId = createChange().getChangeId();
+ vote(admin, changeId, 2, 1);
+ vote(user, changeId, -2, -1);
+
+ String cherryPickChangeId =
+ changeKindCreator.cherryPick(changeId, TRIVIAL_REBASE, testRepo, admin, project);
+ c = detailedChange(cherryPickChangeId);
+ assertVotes(c, admin, 2, 0);
+ assertVotes(c, user, -2, 0);
+
+ // check that votes are not sticky when rework is done by cherry-pick
+ testRepo.reset(projectOperations.project(project).getHead("master"));
+ changeId = createChange().getChangeId();
+ vote(admin, changeId, 2, 1);
+ vote(user, changeId, -2, -1);
+
+ cherryPickChangeId = changeKindCreator.cherryPick(changeId, REWORK, testRepo, admin, project);
+ c = detailedChange(cherryPickChangeId);
+ assertVotes(c, admin, 0, 0);
+ assertVotes(c, user, 0, 0);
+ }
+
+ @Test
+ public void stickyOnNoCodeChange() throws Exception {
+ updateVerifiedLabel(b -> b.setCopyAllScoresIfNoCodeChange(true));
+
+ String changeId = changeKindCreator.createChange(NO_CODE_CHANGE, testRepo, admin);
+ vote(admin, changeId, 2, 1);
+ vote(user, changeId, -2, -1);
+
+ changeKindCreator.updateChange(changeId, NO_CHANGE, testRepo, admin, project);
+ ChangeInfo c = detailedChange(changeId);
+ assertVotes(c, admin, 0, 1, NO_CHANGE);
+ assertVotes(c, user, 0, -1, NO_CHANGE);
+
+ changeKindCreator.updateChange(changeId, NO_CODE_CHANGE, testRepo, admin, project);
+ c = detailedChange(changeId);
+ assertVotes(c, admin, 0, 1, NO_CODE_CHANGE);
+ assertVotes(c, user, 0, -1, NO_CODE_CHANGE);
+
+ assertNotSticky(EnumSet.of(REWORK, TRIVIAL_REBASE, MERGE_FIRST_PARENT_UPDATE));
+ }
+
+ @Test
+ public void stickyOnMergeFirstParentUpdate() throws Exception {
+ updateCodeReviewLabel(b -> b.setCopyAllScoresOnMergeFirstParentUpdate(true));
+
+ String changeId = changeKindCreator.createChange(MERGE_FIRST_PARENT_UPDATE, testRepo, admin);
+ vote(admin, changeId, 2, 1);
+ vote(user, changeId, -2, -1);
+
+ changeKindCreator.updateChange(changeId, NO_CHANGE, testRepo, admin, project);
+ ChangeInfo c = detailedChange(changeId);
+ assertVotes(c, admin, 2, 0, NO_CHANGE);
+ assertVotes(c, user, -2, 0, NO_CHANGE);
+
+ changeKindCreator.updateChange(changeId, MERGE_FIRST_PARENT_UPDATE, testRepo, admin, project);
+ c = detailedChange(changeId);
+ assertVotes(c, admin, 2, 0, MERGE_FIRST_PARENT_UPDATE);
+ assertVotes(c, user, -2, 0, MERGE_FIRST_PARENT_UPDATE);
+
+ assertNotSticky(EnumSet.of(REWORK, NO_CODE_CHANGE, TRIVIAL_REBASE));
+ }
+
+ @Test
+ public void notStickyWithCopyOnNoChangeWhenSecondParentIsUpdated() throws Exception {
+ updateCodeReviewLabel(b -> b.setCopyAllScoresIfNoChange(true));
+
+ String changeId = changeKindCreator.createChangeForMergeCommit(testRepo, admin);
+ vote(admin, changeId, 2, 1);
+ vote(user, changeId, -2, -1);
+
+ changeKindCreator.updateSecondParent(changeId, testRepo, admin);
+ ChangeInfo c = detailedChange(changeId);
+ assertVotes(c, admin, 0, 0, null);
+ assertVotes(c, user, 0, 0, null);
+ }
+
+ @Test
+ public void
+ notStickyWithCopyAllScoresIfListOfFilesDidNotChangeWhenFileIsAdded_withoutCopyCondition()
+ throws Exception {
+ updateCodeReviewLabel(b -> b.setCopyAllScoresIfListOfFilesDidNotChange(true));
+ notStickyWithCopyAllScoresIfListOfFilesDidNotChangeWhenFileIsAdded();
+ }
+
+ @Test
+ public void notStickyWithCopyAllScoresIfListOfFilesDidNotChangeWhenFileIsAdded_withCopyCondition()
+ throws Exception {
+ updateCodeReviewLabel(b -> b.setCopyCondition("has:unchanged-files"));
+ notStickyWithCopyAllScoresIfListOfFilesDidNotChangeWhenFileIsAdded();
+ }
+
+ private void notStickyWithCopyAllScoresIfListOfFilesDidNotChangeWhenFileIsAdded()
+ throws Exception {
+ Change.Id changeId =
+ changeOperations.newChange().project(project).file("file").content("content").create();
+ vote(admin, changeId.toString(), 2, 1);
+ vote(user, changeId.toString(), -2, -1);
+
+ changeOperations
+ .change(changeId)
+ .newPatchset()
+ .file("new file")
+ .content("new content")
+ .create();
+ ChangeInfo c = detailedChange(changeId.toString());
+
+ // no votes are copied since the list of files changed.
+ assertVotes(c, admin, 0, 0);
+ assertVotes(c, user, 0, 0);
+ }
+
+ @Test
+ public void
+ notStickyWithCopyAllScoresIfListOfFilesDidNotChangeWhenFileAlreadyExists_withoutCopyCondition()
+ throws Exception {
+ updateCodeReviewLabel(b -> b.setCopyAllScoresIfListOfFilesDidNotChange(true));
+ notStickyWithCopyAllScoresIfListOfFilesDidNotChangeWhenFileAlreadyExists();
+ }
+
+ @Test
+ public void
+ notStickyWithCopyAllScoresIfListOfFilesDidNotChangeWhenFileAlreadyExists_withCopyCondition()
+ throws Exception {
+ updateCodeReviewLabel(b -> b.setCopyCondition("has:unchanged-files"));
+ notStickyWithCopyAllScoresIfListOfFilesDidNotChangeWhenFileAlreadyExists();
+ }
+
+ private void notStickyWithCopyAllScoresIfListOfFilesDidNotChangeWhenFileAlreadyExists()
+ throws Exception {
+ // create "existing file" and submit it.
+ String existingFile = "existing file";
+ Change.Id prep =
+ changeOperations
+ .newChange()
+ .project(project)
+ .file(existingFile)
+ .content("content")
+ .create();
+ vote(admin, prep.toString(), 2, 1);
+ gApi.changes().id(prep.get()).current().submit();
+
+ Change.Id changeId = changeOperations.newChange().project(project).create();
+ vote(admin, changeId.toString(), 2, 1);
+ vote(user, changeId.toString(), -2, -1);
+
+ changeOperations
+ .change(changeId)
+ .newPatchset()
+ .file(existingFile)
+ .content("new content")
+ .create();
+ ChangeInfo c = detailedChange(changeId.toString());
+
+ // no votes are copied since the list of files changed ("existing file" was added to the
+ // change).
+ assertVotes(c, admin, 0, 0);
+ assertVotes(c, user, 0, 0);
+ }
+
+ @Test
+ public void
+ notStickyWithCopyAllScoresIfListOfFilesDidNotChangeWhenFileIsDeleted_withoutCopyCondition()
+ throws Exception {
+ updateCodeReviewLabel(b -> b.setCopyAllScoresIfListOfFilesDidNotChange(true));
+ notStickyWithCopyAllScoresIfListOfFilesDidNotChangeWhenFileIsDeleted();
+ }
+
+ @Test
+ public void
+ notStickyWithCopyAllScoresIfListOfFilesDidNotChangeWhenFileIsDeleted_withCopyCondition()
+ throws Exception {
+ updateCodeReviewLabel(b -> b.setCopyCondition("has:unchanged-files"));
+ notStickyWithCopyAllScoresIfListOfFilesDidNotChangeWhenFileIsDeleted();
+ }
+
+ private void notStickyWithCopyAllScoresIfListOfFilesDidNotChangeWhenFileIsDeleted()
+ throws Exception {
+ Change.Id changeId =
+ changeOperations.newChange().project(project).file("file").content("content").create();
+ vote(admin, changeId.toString(), 2, 1);
+ vote(user, changeId.toString(), -2, -1);
+
+ changeOperations.change(changeId).newPatchset().file("file").delete().create();
+ ChangeInfo c = detailedChange(changeId.toString());
+
+ // no votes are copied since the list of files changed.
+ assertVotes(c, admin, 0, 0);
+ assertVotes(c, user, 0, 0);
+ }
+
+ @Test
+ public void
+ stickyWithCopyAllScoresIfListOfFilesDidNotChangeWhenFileIsModified_withoutCopyCondition()
+ throws Exception {
+ updateCodeReviewLabel(b -> b.setCopyAllScoresIfListOfFilesDidNotChange(true));
+ stickyWithCopyAllScoresIfListOfFilesDidNotChangeWhenFileIsModified();
+ }
+
+ @Test
+ public void
+ stickyWithCopyAllScoresIfListOfFilesDidNotChangeWhenFileIsModifiedDueToRebase_withoutCopyCondition()
+ throws Exception {
+ updateCodeReviewLabel(b -> b.setCopyAllScoresIfListOfFilesDidNotChange(true));
+
+ // Create two changes both with the same parent
+ PushOneCommit.Result r = createChange();
+ testRepo.reset("HEAD~1");
+ PushOneCommit.Result r2 = createChange();
+
+ // Modify f.txt in change 1. Approve and submit the first change
+ gApi.changes().id(r.getChangeId()).edit().modifyFile("f.txt", RawInputUtil.create("content"));
+ gApi.changes().id(r.getChangeId()).edit().publish();
+ RevisionApi revision = gApi.changes().id(r.getChangeId()).current();
+ revision.review(ReviewInput.approve().label(LabelId.VERIFIED, 1));
+ revision.submit();
+
+ // Add an approval whose score should be copied on change 2.
+ gApi.changes().id(r2.getChangeId()).current().review(ReviewInput.recommend());
+
+ // Rebase the second change. The rebase adds f1.txt.
+ gApi.changes().id(r2.getChangeId()).rebase();
+
+ // The code-review approval is copied for the second change between PS1 and PS2 since the only
+ // modified file is due to rebase.
+ List<PatchSetApproval> patchSetApprovals =
+ r2.getChange().notes().getApprovalsWithCopied().values().stream()
+ .sorted(comparing(a -> a.patchSetId().get()))
+ .collect(toImmutableList());
+ PatchSetApproval nonCopied = patchSetApprovals.get(0);
+ PatchSetApproval copied = patchSetApprovals.get(1);
+ assertCopied(nonCopied, /* psId= */ 1, LabelId.CODE_REVIEW, (short) 1, false);
+ assertCopied(copied, /* psId= */ 2, LabelId.CODE_REVIEW, (short) 1, true);
+ }
+
+ @Test
+ public void stickyWithCopyAllScoresIfListOfFilesDidNotChangeWhenFileIsModified_withCopyCondition()
+ throws Exception {
+ updateCodeReviewLabel(b -> b.setCopyCondition("has:unchanged-files"));
+ stickyWithCopyAllScoresIfListOfFilesDidNotChangeWhenFileIsModified();
+ }
+
+ private void stickyWithCopyAllScoresIfListOfFilesDidNotChangeWhenFileIsModified()
+ throws Exception {
+ Change.Id changeId =
+ changeOperations.newChange().project(project).file("file").content("content").create();
+ vote(admin, changeId.toString(), 2, 1);
+ vote(user, changeId.toString(), -2, -1);
+
+ changeOperations.change(changeId).newPatchset().file("file").content("new content").create();
+ ChangeInfo c = detailedChange(changeId.toString());
+
+ // only code review votes are copied since copyAllScoresIfListOfFilesDidNotChange is
+ // configured for that label, and list of files didn't change.
+ assertVotes(c, admin, 2, 0);
+ assertVotes(c, user, -2, 0);
+ }
+
+ @Test
+ @TestProjectInput(createEmptyCommit = false)
+ public void
+ stickyWithCopyAllScoresIfListOfFilesDidNotChangeWhenFileIsModifiedAsInitialCommit_withoutCopyCondition()
+ throws Exception {
+ updateCodeReviewLabel(b -> b.setCopyAllScoresIfListOfFilesDidNotChange(true));
+ stickyWithCopyAllScoresIfListOfFilesDidNotChangeWhenFileIsModifiedAsInitialCommit();
+ }
+
+ @Test
+ @TestProjectInput(createEmptyCommit = false)
+ public void
+ stickyWithCopyAllScoresIfListOfFilesDidNotChangeWhenFileIsModifiedAsInitialCommit_withCopyCondition()
+ throws Exception {
+ updateCodeReviewLabel(b -> b.setCopyCondition("has:unchanged-files"));
+ stickyWithCopyAllScoresIfListOfFilesDidNotChangeWhenFileIsModifiedAsInitialCommit();
+ }
+
+ private void stickyWithCopyAllScoresIfListOfFilesDidNotChangeWhenFileIsModifiedAsInitialCommit()
+ throws Exception {
+ Change.Id changeId =
+ changeOperations.newChange().project(project).file("file").content("content").create();
+ vote(admin, changeId.toString(), 2, 1);
+ vote(user, changeId.toString(), -2, -1);
+
+ changeOperations.change(changeId).newPatchset().file("file").content("new content").create();
+ ChangeInfo c = detailedChange(changeId.toString());
+
+ // only code review votes are copied since copyAllScoresIfListOfFilesDidNotChange is
+ // configured for that label, and list of files didn't change.
+ assertVotes(c, admin, 2, 0);
+ assertVotes(c, user, -2, 0);
+ }
+
+ @Test
+ public void
+ notStickyWithCopyAllScoresIfListOfFilesDidNotChangeWhenFileIsModifiedOnEarlierPatchset_withoutCopyCondition()
+ throws Exception {
+ updateCodeReviewLabel(b -> b.setCopyAllScoresIfListOfFilesDidNotChange(true));
+ notStickyWithCopyAllScoresIfListOfFilesDidNotChangeWhenFileIsModifiedOnEarlierPatchset();
+ }
+
+ @Test
+ public void
+ notStickyWithCopyAllScoresIfListOfFilesDidNotChangeWhenFileIsModifiedOnEarlierPatchset_withCopyCondition()
+ throws Exception {
+ updateCodeReviewLabel(b -> b.setCopyCondition("has:unchanged-files"));
+ notStickyWithCopyAllScoresIfListOfFilesDidNotChangeWhenFileIsModifiedOnEarlierPatchset();
+ }
+
+ private void
+ notStickyWithCopyAllScoresIfListOfFilesDidNotChangeWhenFileIsModifiedOnEarlierPatchset()
+ throws Exception {
+ Change.Id changeId =
+ changeOperations.newChange().project(project).file("file").content("content").create();
+ vote(admin, changeId.toString(), 2, 1);
+ vote(user, changeId.toString(), -2, -1);
+
+ changeOperations.change(changeId).newPatchset().file("new file").content("content").create();
+ changeOperations
+ .change(changeId)
+ .newPatchset()
+ .file("new file")
+ .content("new content")
+ .create();
+ ChangeInfo c = detailedChange(changeId.toString());
+
+ // Don't copy over votes since ps1->ps2 should copy over, but ps2->ps3 should not.
+ assertVotes(c, admin, 0, 0);
+ assertVotes(c, user, 0, 0);
+ }
+
+ @Test
+ public void
+ notStickyWithCopyAllScoresIfListOfFilesDidNotChangeWhenFileIsRenamed_withoutCopyCondition()
+ throws Exception {
+ updateCodeReviewLabel(b -> b.setCopyAllScoresIfListOfFilesDidNotChange(true));
+ notStickyWithCopyAllScoresIfListOfFilesDidNotChangeWhenFileIsRenamed();
+ }
+
+ @Test
+ public void
+ notStickyWithCopyAllScoresIfListOfFilesDidNotChangeWhenFileIsRenamed_withCopyCondition()
+ throws Exception {
+ updateCodeReviewLabel(b -> b.setCopyCondition("has:unchanged-files"));
+ notStickyWithCopyAllScoresIfListOfFilesDidNotChangeWhenFileIsRenamed();
+ }
+
+ private void notStickyWithCopyAllScoresIfListOfFilesDidNotChangeWhenFileIsRenamed()
+ throws Exception {
+ Change.Id changeId =
+ changeOperations.newChange().project(project).file("file").content("content").create();
+ vote(admin, changeId.toString(), 2, 1);
+ vote(user, changeId.toString(), -2, -1);
+
+ changeOperations.change(changeId).newPatchset().file("file").renameTo("new_file").create();
+ ChangeInfo c = detailedChange(changeId.toString());
+
+ // no votes are copied since the list of files changed (rename).
+ assertVotes(c, admin, 0, 0);
+ assertVotes(c, user, 0, 0);
+ }
+
+ @Test
+ public void copyWithListOfFilesUnchanged_withoutCopyCondition() throws Exception {
+ updateCodeReviewLabel(b -> b.setCopyAllScoresIfListOfFilesDidNotChange(true));
+ copyWithListOfFilesUnchanged();
+ }
+
+ @Test
+ public void copyWithListOfFilesUnchanged_withCopyCondition() throws Exception {
+ updateCodeReviewLabel(b -> b.setCopyCondition("has:unchanged-files"));
+ copyWithListOfFilesUnchanged();
+ }
+
+ private void copyWithListOfFilesUnchanged() throws Exception {
+ Change.Id changeId =
+ changeOperations.newChange().project(project).file("file").content("content").create();
+ vote(admin, changeId.toString(), 2, 1);
+ vote(user, changeId.toString(), -2, -1);
+
+ changeOperations.change(changeId).newPatchset().file("file").content("new content").create();
+ ChangeInfo c = detailedChange(changeId.toString());
+
+ // Code-Review votes are copied over from ps1-> ps2 since the list of files were unchanged.
+ assertVotes(c, admin, 2, 0);
+ assertVotes(c, user, -2, 0);
+
+ changeOperations
+ .change(changeId)
+ .newPatchset()
+ .file("file")
+ .content("very new content")
+ .create();
+ c = detailedChange(changeId.toString());
+
+ // Code-Review votes are copied over from ps1-> ps3 since the list of files were unchanged.
+ assertVotes(c, admin, 2, 0);
+ assertVotes(c, user, -2, 0);
+
+ changeOperations
+ .change(changeId)
+ .newPatchset()
+ .file("new file")
+ .content("new content")
+ .create();
+
+ c = detailedChange(changeId.toString());
+ // Code-Review votes are not copied over from ps1-> ps4 since a file was added.
+ assertVotes(c, admin, 0, 0);
+ assertVotes(c, user, 0, 0);
+
+ changeOperations.change(changeId).newPatchset().file("file").content("content").create();
+
+ c = detailedChange(changeId.toString());
+ // Code-Review votes are not copied over from ps1 -> ps5 since a file was added on ps4.
+ // Although the list of files is the same between ps4->ps5, we don't copy votes from before
+ // ps4.
+ assertVotes(c, admin, 0, 0);
+ assertVotes(c, user, 0, 0);
+ }
+
+ @Test
+ public void copyWithListOfFilesUnchangedButAddedMergeList() throws Exception {
+ updateCodeReviewLabel(b -> b.setCopyCondition("has:unchanged-files"));
+
+ Change.Id parent1ChangeId = changeOperations.newChange().create();
+ Change.Id parent2ChangeId = changeOperations.newChange().create();
+ Change.Id dummyParentChangeId = changeOperations.newChange().create();
+ Change.Id changeId =
+ changeOperations
+ .newChange()
+ .mergeOf()
+ .change(parent1ChangeId)
+ .and()
+ .change(parent2ChangeId)
+ .create();
+
+ Map<String, FileInfo> changedFilesFirstPatchset =
+ gApi.changes().id(changeId.get()).current().files();
+
+ assertThat(changedFilesFirstPatchset.keySet()).containsExactly("/COMMIT_MSG", "/MERGE_LIST");
+
+ // Make a Code-Review vote that should be sticky.
+ gApi.changes().id(changeId.get()).current().review(ReviewInput.approve());
+
+ changeOperations
+ .change(changeId)
+ .newPatchset()
+ .parent()
+ .patchset(PatchSet.id(dummyParentChangeId, 1))
+ .create();
+
+ Map<String, FileInfo> changedFilesSecondPatchset =
+ gApi.changes().id(changeId.get()).current().files();
+
+ // Only "/MERGE_LIST" was removed.
+ assertThat(changedFilesSecondPatchset.keySet()).containsExactly("/COMMIT_MSG");
+ ApprovalInfo approvalInfo =
+ Iterables.getOnlyElement(
+ gApi.changes().id(changeId.get()).current().votes().get(LabelId.CODE_REVIEW));
+ assertThat(approvalInfo._accountId).isEqualTo(admin.id().get());
+ assertThat(approvalInfo.value).isEqualTo(2);
+ }
+
+ @Test
+ public void approvalsThatMatchApproverinConditionAreStickyEvenIfUserCantSeeTheGroup()
+ throws Exception {
+ String administratorsUUID = gApi.groups().query("name:Administrators").get().get(0).id;
+
+ // Verify that user can't see the admin group.
+ requestScopeOperations.setApiUser(user.id());
+ ResourceNotFoundException notFound =
+ assertThrows(
+ ResourceNotFoundException.class, () -> gApi.groups().id(administratorsUUID).get());
+ assertThat(notFound).hasMessageThat().isEqualTo("Not found: " + administratorsUUID);
+
+ requestScopeOperations.setApiUser(admin.id());
+ updateCodeReviewLabel(b -> b.setCopyCondition("approverin:" + administratorsUUID));
+
+ PushOneCommit.Result r = createChange();
+
+ // Add Code-Review+2 by the admin user.
+ approve(r.getChangeId());
+
+ // Create a new patch set by user.
+ // Approvals are copied on creation of the new patch set. The approval of the admin user is
+ // expected to be sticky although the group that is configured for the 'approverin' predicate is
+ // not visible to the user.
+ TestRepository<InMemoryRepository> userTestRepo = cloneProject(project, user);
+ GitUtil.fetch(userTestRepo, r.getPatchSet().refName() + ":ps");
+ userTestRepo.reset("ps");
+ amendChange(r.getChangeId(), "refs/for/master", user, testRepo);
+
+ // Assert that the approval of the admin user was copied to the new patch set.
+ ChangeInfo c = detailedChange(r.getChangeId());
+ assertVotes(c, admin, 2, 0);
+ }
+
+ @Test
+ public void removedVotesNotSticky() throws Exception {
+ updateCodeReviewLabel(b -> b.setCopyAllScoresOnTrivialRebase(true));
+ updateVerifiedLabel(b -> b.setCopyAllScoresIfNoCodeChange(true));
+
+ for (ChangeKind changeKind :
+ EnumSet.of(REWORK, TRIVIAL_REBASE, NO_CODE_CHANGE, MERGE_FIRST_PARENT_UPDATE, NO_CHANGE)) {
+ testRepo.reset(projectOperations.project(project).getHead("master"));
+
+ String changeId = changeKindCreator.createChange(changeKind, testRepo, admin);
+ vote(admin, changeId, 2, 1);
+ vote(user, changeId, -2, -1);
+
+ // Remove votes by re-voting with 0
+ vote(admin, changeId, 0, 0);
+ vote(user, changeId, 0, 0);
+ ChangeInfo c = detailedChange(changeId);
+ assertVotes(c, admin, 0, 0, null);
+ assertVotes(c, user, 0, 0, null);
+
+ changeKindCreator.updateChange(changeId, changeKind, testRepo, admin, project);
+ c = detailedChange(changeId);
+ assertVotes(c, admin, 0, 0, changeKind);
+ assertVotes(c, user, 0, 0, changeKind);
+ }
+ }
+
+ @Test
+ public void stickyAcrossMultiplePatchSets() throws Exception {
+ updateCodeReviewLabel(b -> b.setCopyMaxScore(true));
+ updateVerifiedLabel(b -> b.setCopyAllScoresIfNoCodeChange(true));
+
+ String changeId = changeKindCreator.createChange(REWORK, testRepo, admin);
+ vote(admin, changeId, 2, 1);
+
+ for (int i = 0; i < 5; i++) {
+ changeKindCreator.updateChange(changeId, NO_CODE_CHANGE, testRepo, admin, project);
+ ChangeInfo c = detailedChange(changeId);
+ assertVotes(c, admin, 2, 1, NO_CODE_CHANGE);
+ }
+
+ changeKindCreator.updateChange(changeId, REWORK, testRepo, admin, project);
+ ChangeInfo c = detailedChange(changeId);
+ assertVotes(c, admin, 2, 0, REWORK);
+ }
+
+ @Test
+ public void stickyAcrossMultiplePatchSetsDoNotRegressPerformance() throws Exception {
+ // The purpose of this test is to make sure that we compute change kind only against the parent
+ // patch set. Change kind is a heavy operation. In prior version of Gerrit, we computed the
+ // change kind against all prior patch sets. This is a regression that made Gerrit do expensive
+ // work in O(num-patch-sets). This test ensures that we aren't regressing.
+ updateCodeReviewLabel(b -> b.setCopyMaxScore(true));
+ updateVerifiedLabel(b -> b.setCopyAllScoresIfNoCodeChange(true));
+
+ String changeId = changeKindCreator.createChange(REWORK, testRepo, admin);
+ vote(admin, changeId, 2, 1);
+ changeKindCreator.updateChange(changeId, NO_CODE_CHANGE, testRepo, admin, project);
+ changeKindCreator.updateChange(changeId, NO_CODE_CHANGE, testRepo, admin, project);
+ changeKindCreator.updateChange(changeId, NO_CODE_CHANGE, testRepo, admin, project);
+
+ Map<Integer, ObjectId> revisions = new HashMap<>();
+ gApi.changes()
+ .id(changeId)
+ .get()
+ .revisions
+ .forEach(
+ (revId, revisionInfo) ->
+ revisions.put(revisionInfo._number, ObjectId.fromString(revId)));
+ assertThat(revisions.size()).isEqualTo(4);
+ assertChangeKindCacheContains(revisions.get(3), revisions.get(4));
+ assertChangeKindCacheContains(revisions.get(2), revisions.get(3));
+ assertChangeKindCacheContains(revisions.get(1), revisions.get(2));
+
+ assertChangeKindCacheDoesNotContain(revisions.get(1), revisions.get(4));
+ assertChangeKindCacheDoesNotContain(revisions.get(2), revisions.get(4));
+ assertChangeKindCacheDoesNotContain(revisions.get(1), revisions.get(3));
+ }
+
+ @Test
+ public void copyMinMaxAcrossMultiplePatchSets() throws Exception {
+ updateCodeReviewLabel(b -> b.setCopyMaxScore(true).setCopyMinScore(true));
+
+ // Vote max score on PS1
+ String changeId = changeKindCreator.createChange(REWORK, testRepo, admin);
+ vote(admin, changeId, 2, 1);
+
+ // Have someone else vote min score on PS2
+ changeKindCreator.updateChange(changeId, REWORK, testRepo, admin, project);
+ vote(user, changeId, -2, 0);
+ ChangeInfo c = detailedChange(changeId);
+ assertVotes(c, admin, 2, 0, REWORK);
+ assertVotes(c, user, -2, 0, REWORK);
+
+ // No vote changes on PS3
+ changeKindCreator.updateChange(changeId, REWORK, testRepo, admin, project);
+ c = detailedChange(changeId);
+ assertVotes(c, admin, 2, 0, REWORK);
+ assertVotes(c, user, -2, 0, REWORK);
+
+ // Both users revote on PS4
+ changeKindCreator.updateChange(changeId, REWORK, testRepo, admin, project);
+ vote(admin, changeId, 1, 1);
+ vote(user, changeId, 1, 1);
+ c = detailedChange(changeId);
+ assertVotes(c, admin, 1, 1, REWORK);
+ assertVotes(c, user, 1, 1, REWORK);
+
+ // New approvals shouldn't carry through to PS5
+ changeKindCreator.updateChange(changeId, REWORK, testRepo, admin, project);
+ c = detailedChange(changeId);
+ assertVotes(c, admin, 0, 0, REWORK);
+ assertVotes(c, user, 0, 0, REWORK);
+ }
+
+ @Test
+ public void deleteStickyVote() throws Exception {
+ updateCodeReviewLabel(b -> b.setCopyMaxScore(true));
+
+ String label = LabelId.CODE_REVIEW;
+
+ // Vote max score on PS1
+ String changeId = changeKindCreator.createChange(REWORK, testRepo, admin);
+ vote(admin, changeId, label, 2);
+ assertVotes(detailedChange(changeId), admin, label, 2, null);
+ changeKindCreator.updateChange(changeId, REWORK, testRepo, admin, project);
+ assertVotes(detailedChange(changeId), admin, label, 2, REWORK);
+
+ // Delete vote that was copied via sticky approval
+ deleteVote(admin, changeId, label);
+ assertVotes(detailedChange(changeId), admin, label, 0, REWORK);
+ }
+
+ @Test
+ public void canVoteMultipleTimesOnNewPatchsets() throws Exception {
+ // Code-Review will be sticky.
+ updateCodeReviewLabel(b -> b.setCopyAnyScore(true));
+
+ PushOneCommit.Result r = createChange();
+
+ // Add a new vote.
+ ReviewInput input = new ReviewInput().label(LabelId.CODE_REVIEW, 2);
+ gApi.changes().id(r.getChangeId()).current().review(input);
+
+ // Make a new patchset, keeping the Code-Review +2 vote.
+ amendChange(r.getChangeId());
+
+ // Post without changing the vote.
+ input = new ReviewInput().label(LabelId.CODE_REVIEW, 2);
+ gApi.changes().id(r.getChangeId()).current().review(input);
+
+ // There is a vote both on patchset 1 and on patchset 2, although both votes are Code-Review +2.
+ assertThat(r.getChange().approvals().get(PatchSet.id(r.getChange().getId(), 1))).hasSize(1);
+ assertThat(r.getChange().approvals().get(PatchSet.id(r.getChange().getId(), 2))).hasSize(1);
+ }
+
+ @Test
+ public void stickyVoteStoredOnUpload() throws Exception {
+ // Code-Review will be sticky.
+ updateCodeReviewLabel(b -> b.setCopyAnyScore(true));
+
+ PushOneCommit.Result r = createChange();
+ // Add a new vote.
+ ReviewInput input = new ReviewInput().label(LabelId.CODE_REVIEW, 2);
+ input.tag = "tag";
+ gApi.changes().id(r.getChangeId()).current().review(input);
+
+ // Make new patchsets, keeping the Code-Review +2 vote.
+ for (int i = 0; i < 9; i++) {
+ amendChange(r.getChangeId());
+ }
+
+ List<PatchSetApproval> patchSetApprovals =
+ r.getChange().notes().getApprovalsWithCopied().values().stream()
+ .sorted(comparing(a -> a.patchSetId().get()))
+ .collect(toImmutableList());
+
+ for (int i = 0; i < 10; i++) {
+ int patchSet = i + 1;
+ assertThat(patchSetApprovals.get(i).patchSetId().get()).isEqualTo(patchSet);
+ assertThat(patchSetApprovals.get(i).accountId().get()).isEqualTo(admin.id().get());
+ assertThat(patchSetApprovals.get(i).realAccountId().get()).isEqualTo(admin.id().get());
+ assertThat(patchSetApprovals.get(i).label()).isEqualTo(LabelId.CODE_REVIEW);
+ assertThat(patchSetApprovals.get(i).value()).isEqualTo((short) 2);
+ assertThat(patchSetApprovals.get(i).tag().get()).isEqualTo("tag");
+ if (patchSet == 1) {
+ assertThat(patchSetApprovals.get(i).copied()).isFalse();
+ } else {
+ assertThat(patchSetApprovals.get(i).copied()).isTrue();
+ }
+ }
+ }
+
+ @Test
+ public void stickyVoteStoredOnRebase() throws Exception {
+ // Code-Review will be sticky.
+ updateCodeReviewLabel(b -> b.setCopyAnyScore(true));
+
+ // Create two changes both with the same parent
+ PushOneCommit.Result r = createChange();
+ testRepo.reset("HEAD~1");
+ PushOneCommit.Result r2 = createChange();
+
+ // Approve and submit the first change
+ RevisionApi revision = gApi.changes().id(r.getChangeId()).current();
+ revision.review(ReviewInput.approve().label(LabelId.VERIFIED, 1));
+ revision.submit();
+
+ // Add an approval whose score should be copied.
+ gApi.changes().id(r2.getChangeId()).current().review(ReviewInput.recommend());
+
+ // Rebase the second change
+ gApi.changes().id(r2.getChangeId()).rebase();
+
+ List<PatchSetApproval> patchSetApprovals =
+ r2.getChange().notes().getApprovalsWithCopied().values().stream()
+ .sorted(comparing(a -> a.patchSetId().get()))
+ .collect(toImmutableList());
+ PatchSetApproval nonCopied = patchSetApprovals.get(0);
+ PatchSetApproval copied = patchSetApprovals.get(1);
+ assertCopied(nonCopied, 1, LabelId.CODE_REVIEW, (short) 1, /* copied= */ false);
+ assertCopied(copied, 2, LabelId.CODE_REVIEW, (short) 1, /* copied= */ true);
+ }
+
+ @Test
+ public void stickyVoteStoredOnUploadWithRealAccount() throws Exception {
+ // Give "user" permission to vote on behalf of other users.
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(
+ allowLabel(TestLabels.codeReview().getName())
+ .impersonation(true)
+ .ref("refs/heads/*")
+ .group(REGISTERED_USERS)
+ .range(-1, 1))
+ .update();
+
+ // Code-Review will be sticky.
+ updateCodeReviewLabel(b -> b.setCopyAnyScore(true));
+
+ PushOneCommit.Result r = createChange();
+
+ // Add a new vote as user
+ requestScopeOperations.setApiUser(user.id());
+ ReviewInput input = new ReviewInput().label(LabelId.CODE_REVIEW, 1);
+ input.onBehalfOf = admin.email();
+ gApi.changes().id(r.getChangeId()).current().review(input);
+
+ // Make a new patchset, keeping the Code-Review +1 vote.
+ amendChange(r.getChangeId());
+
+ List<PatchSetApproval> patchSetApprovals =
+ r.getChange().notes().getApprovalsWithCopied().values().stream()
+ .sorted(comparing(a -> a.patchSetId().get()))
+ .collect(toImmutableList());
+
+ PatchSetApproval nonCopied = patchSetApprovals.get(0);
+ assertThat(nonCopied.patchSetId().get()).isEqualTo(1);
+ assertThat(nonCopied.accountId().get()).isEqualTo(admin.id().get());
+ assertThat(nonCopied.realAccountId().get()).isEqualTo(user.id().get());
+ assertThat(nonCopied.label()).isEqualTo(LabelId.CODE_REVIEW);
+ assertThat(nonCopied.value()).isEqualTo((short) 1);
+ assertThat(nonCopied.copied()).isFalse();
+
+ PatchSetApproval copied = patchSetApprovals.get(1);
+ assertThat(copied.patchSetId().get()).isEqualTo(2);
+ assertThat(copied.accountId().get()).isEqualTo(admin.id().get());
+ assertThat(copied.realAccountId().get()).isEqualTo(user.id().get());
+ assertThat(copied.label()).isEqualTo(LabelId.CODE_REVIEW);
+ assertThat(copied.value()).isEqualTo((short) 1);
+ assertThat(copied.copied()).isTrue();
+ }
+
+ @Test
+ public void stickyVoteStoredOnUploadWithRealAccountAndTag() throws Exception {
+ // Give "user" permission to vote on behalf of other users.
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(
+ allowLabel(TestLabels.codeReview().getName())
+ .impersonation(true)
+ .ref("refs/heads/*")
+ .group(REGISTERED_USERS)
+ .range(-1, 1))
+ .update();
+
+ // Code-Review will be sticky.
+ updateCodeReviewLabel(b -> b.setCopyAnyScore(true));
+
+ PushOneCommit.Result r = createChange();
+
+ // Add a new vote as user
+ requestScopeOperations.setApiUser(user.id());
+ ReviewInput input = new ReviewInput().label(LabelId.CODE_REVIEW, 1);
+ input.onBehalfOf = admin.email();
+ input.tag = "tag";
+ gApi.changes().id(r.getChangeId()).current().review(input);
+
+ // Make a new patchset, keeping the Code-Review +1 vote.
+ amendChange(r.getChangeId());
+
+ List<PatchSetApproval> patchSetApprovals =
+ r.getChange().notes().getApprovalsWithCopied().values().stream()
+ .sorted(comparing(a -> a.patchSetId().get()))
+ .collect(toImmutableList());
+
+ PatchSetApproval nonCopied = patchSetApprovals.get(0);
+ assertThat(nonCopied.patchSetId().get()).isEqualTo(1);
+ assertThat(nonCopied.accountId().get()).isEqualTo(admin.id().get());
+ assertThat(nonCopied.realAccountId().get()).isEqualTo(user.id().get());
+ assertThat(nonCopied.label()).isEqualTo(LabelId.CODE_REVIEW);
+ assertThat(nonCopied.value()).isEqualTo((short) 1);
+ assertThat(nonCopied.tag().get()).isEqualTo("tag");
+ assertThat(nonCopied.copied()).isFalse();
+
+ PatchSetApproval copied = patchSetApprovals.get(1);
+ assertThat(copied.patchSetId().get()).isEqualTo(2);
+ assertThat(copied.accountId().get()).isEqualTo(admin.id().get());
+ assertThat(copied.realAccountId().get()).isEqualTo(user.id().get());
+ assertThat(copied.label()).isEqualTo(LabelId.CODE_REVIEW);
+ assertThat(copied.value()).isEqualTo((short) 1);
+ assertThat(nonCopied.tag().get()).isEqualTo("tag");
+ assertThat(copied.copied()).isTrue();
+ }
+
+ @Test
+ public void stickyVoteStoredCanBeRemoved() throws Exception {
+ // Code-Review will be sticky.
+ updateCodeReviewLabel(b -> b.setCopyAnyScore(true));
+
+ PushOneCommit.Result r = createChange();
+
+ // Add a new vote
+ ReviewInput input = new ReviewInput().label(LabelId.CODE_REVIEW, 2);
+ gApi.changes().id(r.getChangeId()).current().review(input);
+
+ // Make a new patchset, keeping the Code-Review +2 vote.
+ amendChange(r.getChangeId());
+ assertVotes(detailedChange(r.getChangeId()), admin, LabelId.CODE_REVIEW, 2, null);
+
+ gApi.changes().id(r.getChangeId()).current().review(ReviewInput.noScore());
+
+ PatchSetApproval nonCopiedSecondPatchsetRemovedVote =
+ Iterables.getOnlyElement(
+ r.getChange()
+ .notes()
+ .getApprovalsWithCopied()
+ .get(r.getChange().change().currentPatchSetId()));
+
+ assertThat(nonCopiedSecondPatchsetRemovedVote.patchSetId().get()).isEqualTo(2);
+ assertThat(nonCopiedSecondPatchsetRemovedVote.accountId().get()).isEqualTo(admin.id().get());
+ assertThat(nonCopiedSecondPatchsetRemovedVote.label()).isEqualTo(LabelId.CODE_REVIEW);
+ // The vote got removed since the latest patch-set only has one vote and it's "0".
+ assertThat(nonCopiedSecondPatchsetRemovedVote.value()).isEqualTo((short) 0);
+ assertThat(nonCopiedSecondPatchsetRemovedVote.copied()).isFalse();
+ }
+
+ @Test
+ public void reviewerStickyVotingCanBeRemoved() throws Exception {
+ // Code-Review will be sticky.
+ updateCodeReviewLabel(b -> b.setCopyAnyScore(true));
+
+ PushOneCommit.Result r = createChange();
+
+ // Add a new vote by user
+ requestScopeOperations.setApiUser(user.id());
+ gApi.changes().id(r.getChangeId()).current().review(ReviewInput.recommend());
+ requestScopeOperations.setApiUser(admin.id());
+
+ // Make a new patchset, keeping the Code-Review +1 vote.
+ amendChange(r.getChangeId());
+ assertVotes(detailedChange(r.getChangeId()), user, LabelId.CODE_REVIEW, 1, null);
+
+ gApi.changes().id(r.getChangeId()).reviewer(user.email()).remove();
+
+ assertThat(r.getChange().notes().getApprovalsWithCopied()).isEmpty();
+
+ // Changes message has info about vote removed.
+ assertThat(Iterables.getLast(gApi.changes().id(r.getChangeId()).messages()).message)
+ .contains("Code-Review+1 by User");
+ }
+
+ @Test
+ public void stickyWhenEitherBooleanConfigsOrCopyConditionAreTrue() throws Exception {
+ updateCodeReviewLabel(b -> b.setCopyCondition("is:MAX").setCopyMinScore(true));
+
+ for (ChangeKind changeKind :
+ EnumSet.of(REWORK, TRIVIAL_REBASE, NO_CODE_CHANGE, MERGE_FIRST_PARENT_UPDATE, NO_CHANGE)) {
+ testRepo.reset(projectOperations.project(project).getHead("master"));
+
+ String changeId = changeKindCreator.createChange(changeKind, testRepo, admin);
+ vote(admin, changeId, 2, 1);
+ vote(user, changeId, -2, -1);
+
+ changeKindCreator.updateChange(changeId, changeKind, testRepo, admin, project);
+ ChangeInfo c = detailedChange(changeId);
+ assertVotes(c, admin, 2, 0, changeKind);
+ assertVotes(c, user, -2, 0, changeKind);
+ }
+ }
+
+ @Test
+ public void sticky_copiedToLatestPatchSetFromSubmitRecords() throws Exception {
+ updateVerifiedLabel(b -> b.setFunction(LabelFunction.NO_BLOCK));
// This test is covering the backfilling logic for changes which have been submitted, based on
// copied approvals, before Gerrit persisted copied votes as Copied-Label footers in NoteDb. It
@@ -378,1041 +1269,6 @@
}
}
- @Test
- public void stickyOnCopyValues() throws Exception {
- TestAccount user2 = accountCreator.user2();
-
- try (ProjectConfigUpdate u = updateProject(project)) {
- u.getConfig()
- .updateLabelType(
- LabelId.CODE_REVIEW, b -> b.setCopyValues(ImmutableList.of((short) -1, (short) 1)));
- u.save();
- }
-
- for (ChangeKind changeKind :
- EnumSet.of(REWORK, TRIVIAL_REBASE, NO_CODE_CHANGE, MERGE_FIRST_PARENT_UPDATE, NO_CHANGE)) {
- testRepo.reset(projectOperations.project(project).getHead("master"));
-
- String changeId = changeKindCreator.createChange(changeKind, testRepo, admin);
- vote(admin, changeId, -1, 1);
- vote(user, changeId, -2, -1);
- vote(user2, changeId, 1, -1);
-
- changeKindCreator.updateChange(changeId, changeKind, testRepo, admin, project);
- ChangeInfo c = detailedChange(changeId);
- assertVotes(c, admin, -1, 0, changeKind);
- assertVotes(c, user, 0, 0, changeKind);
- assertVotes(c, user2, 1, 0, changeKind);
- }
- }
-
- @Test
- public void stickyOnTrivialRebase() throws Exception {
- try (ProjectConfigUpdate u = updateProject(project)) {
- u.getConfig()
- .updateLabelType(LabelId.CODE_REVIEW, b -> b.setCopyAllScoresOnTrivialRebase(true));
- u.save();
- }
-
- String changeId = changeKindCreator.createChange(TRIVIAL_REBASE, testRepo, admin);
- vote(admin, changeId, 2, 1);
- vote(user, changeId, -2, -1);
-
- changeKindCreator.updateChange(changeId, NO_CHANGE, testRepo, admin, project);
- ChangeInfo c = detailedChange(changeId);
- assertVotes(c, admin, 2, 0, NO_CHANGE);
- assertVotes(c, user, -2, 0, NO_CHANGE);
-
- changeKindCreator.updateChange(changeId, TRIVIAL_REBASE, testRepo, admin, project);
- c = detailedChange(changeId);
- assertVotes(c, admin, 2, 0, TRIVIAL_REBASE);
- assertVotes(c, user, -2, 0, TRIVIAL_REBASE);
-
- assertNotSticky(EnumSet.of(REWORK, NO_CODE_CHANGE, MERGE_FIRST_PARENT_UPDATE));
-
- // check that votes are sticky when trivial rebase is done by cherry-pick
- testRepo.reset(projectOperations.project(project).getHead("master"));
- changeId = createChange().getChangeId();
- vote(admin, changeId, 2, 1);
- vote(user, changeId, -2, -1);
-
- String cherryPickChangeId =
- changeKindCreator.cherryPick(changeId, TRIVIAL_REBASE, testRepo, admin, project);
- c = detailedChange(cherryPickChangeId);
- assertVotes(c, admin, 2, 0);
- assertVotes(c, user, -2, 0);
-
- // check that votes are not sticky when rework is done by cherry-pick
- testRepo.reset(projectOperations.project(project).getHead("master"));
- changeId = createChange().getChangeId();
- vote(admin, changeId, 2, 1);
- vote(user, changeId, -2, -1);
-
- cherryPickChangeId = changeKindCreator.cherryPick(changeId, REWORK, testRepo, admin, project);
- c = detailedChange(cherryPickChangeId);
- assertVotes(c, admin, 0, 0);
- assertVotes(c, user, 0, 0);
- }
-
- @Test
- public void stickyOnNoCodeChange() throws Exception {
- try (ProjectConfigUpdate u = updateProject(project)) {
- u.getConfig().updateLabelType(LabelId.VERIFIED, b -> b.setCopyAllScoresIfNoCodeChange(true));
- u.save();
- }
-
- String changeId = changeKindCreator.createChange(NO_CODE_CHANGE, testRepo, admin);
- vote(admin, changeId, 2, 1);
- vote(user, changeId, -2, -1);
-
- changeKindCreator.updateChange(changeId, NO_CHANGE, testRepo, admin, project);
- ChangeInfo c = detailedChange(changeId);
- assertVotes(c, admin, 0, 1, NO_CHANGE);
- assertVotes(c, user, 0, -1, NO_CHANGE);
-
- changeKindCreator.updateChange(changeId, NO_CODE_CHANGE, testRepo, admin, project);
- c = detailedChange(changeId);
- assertVotes(c, admin, 0, 1, NO_CODE_CHANGE);
- assertVotes(c, user, 0, -1, NO_CODE_CHANGE);
-
- assertNotSticky(EnumSet.of(REWORK, TRIVIAL_REBASE, MERGE_FIRST_PARENT_UPDATE));
- }
-
- @Test
- public void stickyOnMergeFirstParentUpdate() throws Exception {
- try (ProjectConfigUpdate u = updateProject(project)) {
- u.getConfig()
- .updateLabelType(
- LabelId.CODE_REVIEW, b -> b.setCopyAllScoresOnMergeFirstParentUpdate(true));
- u.save();
- }
-
- String changeId = changeKindCreator.createChange(MERGE_FIRST_PARENT_UPDATE, testRepo, admin);
- vote(admin, changeId, 2, 1);
- vote(user, changeId, -2, -1);
-
- changeKindCreator.updateChange(changeId, NO_CHANGE, testRepo, admin, project);
- ChangeInfo c = detailedChange(changeId);
- assertVotes(c, admin, 2, 0, NO_CHANGE);
- assertVotes(c, user, -2, 0, NO_CHANGE);
-
- changeKindCreator.updateChange(changeId, MERGE_FIRST_PARENT_UPDATE, testRepo, admin, project);
- c = detailedChange(changeId);
- assertVotes(c, admin, 2, 0, MERGE_FIRST_PARENT_UPDATE);
- assertVotes(c, user, -2, 0, MERGE_FIRST_PARENT_UPDATE);
-
- assertNotSticky(EnumSet.of(REWORK, NO_CODE_CHANGE, TRIVIAL_REBASE));
- }
-
- @Test
- public void notStickyWithCopyOnNoChangeWhenSecondParentIsUpdated() throws Exception {
- try (ProjectConfigUpdate u = updateProject(project)) {
- u.getConfig().updateLabelType(LabelId.CODE_REVIEW, b -> b.setCopyAllScoresIfNoChange(true));
- u.save();
- }
-
- String changeId = changeKindCreator.createChangeForMergeCommit(testRepo, admin);
- vote(admin, changeId, 2, 1);
- vote(user, changeId, -2, -1);
-
- changeKindCreator.updateSecondParent(changeId, testRepo, admin);
- ChangeInfo c = detailedChange(changeId);
- assertVotes(c, admin, 0, 0, null);
- assertVotes(c, user, 0, 0, null);
- }
-
- @Test
- public void
- notStickyWithCopyAllScoresIfListOfFilesDidNotChangeWhenFileIsAdded_withoutCopyCondition()
- throws Exception {
- try (ProjectConfigUpdate u = updateProject(project)) {
- u.getConfig()
- .updateLabelType(
- LabelId.CODE_REVIEW, b -> b.setCopyAllScoresIfListOfFilesDidNotChange(true));
- u.save();
- }
- notStickyWithCopyAllScoresIfListOfFilesDidNotChangeWhenFileIsAdded();
- }
-
- @Test
- public void notStickyWithCopyAllScoresIfListOfFilesDidNotChangeWhenFileIsAdded_withCopyCondition()
- throws Exception {
- try (ProjectConfigUpdate u = updateProject(project)) {
- u.getConfig()
- .updateLabelType(LabelId.CODE_REVIEW, b -> b.setCopyCondition("has:unchanged-files"));
- u.save();
- }
- notStickyWithCopyAllScoresIfListOfFilesDidNotChangeWhenFileIsAdded();
- }
-
- private void notStickyWithCopyAllScoresIfListOfFilesDidNotChangeWhenFileIsAdded()
- throws Exception {
- Change.Id changeId =
- changeOperations.newChange().project(project).file("file").content("content").create();
- vote(admin, changeId.toString(), 2, 1);
- vote(user, changeId.toString(), -2, -1);
-
- changeOperations
- .change(changeId)
- .newPatchset()
- .file("new file")
- .content("new content")
- .create();
- ChangeInfo c = detailedChange(changeId.toString());
-
- // no votes are copied since the list of files changed.
- assertVotes(c, admin, 0, 0);
- assertVotes(c, user, 0, 0);
- }
-
- @Test
- public void
- notStickyWithCopyAllScoresIfListOfFilesDidNotChangeWhenFileAlreadyExists_withoutCopyCondition()
- throws Exception {
- try (ProjectConfigUpdate u = updateProject(project)) {
- u.getConfig()
- .updateLabelType(
- LabelId.CODE_REVIEW, b -> b.setCopyAllScoresIfListOfFilesDidNotChange(true));
- u.save();
- }
- notStickyWithCopyAllScoresIfListOfFilesDidNotChangeWhenFileAlreadyExists();
- }
-
- @Test
- public void
- notStickyWithCopyAllScoresIfListOfFilesDidNotChangeWhenFileAlreadyExists_withCopyCondition()
- throws Exception {
- try (ProjectConfigUpdate u = updateProject(project)) {
- u.getConfig()
- .updateLabelType(LabelId.CODE_REVIEW, b -> b.setCopyCondition("has:unchanged-files"));
- u.save();
- }
-
- notStickyWithCopyAllScoresIfListOfFilesDidNotChangeWhenFileAlreadyExists();
- }
-
- private void notStickyWithCopyAllScoresIfListOfFilesDidNotChangeWhenFileAlreadyExists()
- throws Exception {
- // create "existing file" and submit it.
- String existingFile = "existing file";
- Change.Id prep =
- changeOperations
- .newChange()
- .project(project)
- .file(existingFile)
- .content("content")
- .create();
- vote(admin, prep.toString(), 2, 1);
- gApi.changes().id(prep.get()).current().submit();
-
- Change.Id changeId = changeOperations.newChange().project(project).create();
- vote(admin, changeId.toString(), 2, 1);
- vote(user, changeId.toString(), -2, -1);
-
- changeOperations
- .change(changeId)
- .newPatchset()
- .file(existingFile)
- .content("new content")
- .create();
- ChangeInfo c = detailedChange(changeId.toString());
-
- // no votes are copied since the list of files changed ("existing file" was added to the
- // change).
- assertVotes(c, admin, 0, 0);
- assertVotes(c, user, 0, 0);
- }
-
- @Test
- public void
- notStickyWithCopyAllScoresIfListOfFilesDidNotChangeWhenFileIsDeleted_withoutCopyCondition()
- throws Exception {
- try (ProjectConfigUpdate u = updateProject(project)) {
- u.getConfig()
- .updateLabelType(
- LabelId.CODE_REVIEW, b -> b.setCopyAllScoresIfListOfFilesDidNotChange(true));
- u.save();
- }
- notStickyWithCopyAllScoresIfListOfFilesDidNotChangeWhenFileIsDeleted();
- }
-
- @Test
- public void
- notStickyWithCopyAllScoresIfListOfFilesDidNotChangeWhenFileIsDeleted_withCopyCondition()
- throws Exception {
- try (ProjectConfigUpdate u = updateProject(project)) {
- u.getConfig()
- .updateLabelType(LabelId.CODE_REVIEW, b -> b.setCopyCondition("has:unchanged-files"));
- u.save();
- }
- notStickyWithCopyAllScoresIfListOfFilesDidNotChangeWhenFileIsDeleted();
- }
-
- private void notStickyWithCopyAllScoresIfListOfFilesDidNotChangeWhenFileIsDeleted()
- throws Exception {
- Change.Id changeId =
- changeOperations.newChange().project(project).file("file").content("content").create();
- vote(admin, changeId.toString(), 2, 1);
- vote(user, changeId.toString(), -2, -1);
-
- changeOperations.change(changeId).newPatchset().file("file").delete().create();
- ChangeInfo c = detailedChange(changeId.toString());
-
- // no votes are copied since the list of files changed.
- assertVotes(c, admin, 0, 0);
- assertVotes(c, user, 0, 0);
- }
-
- @Test
- public void
- stickyWithCopyAllScoresIfListOfFilesDidNotChangeWhenFileIsModified_withoutCopyCondition()
- throws Exception {
- try (ProjectConfigUpdate u = updateProject(project)) {
- u.getConfig()
- .updateLabelType(
- LabelId.CODE_REVIEW, b -> b.setCopyAllScoresIfListOfFilesDidNotChange(true));
- u.save();
- }
- stickyWithCopyAllScoresIfListOfFilesDidNotChangeWhenFileIsModified();
- }
-
- @Test
- public void
- stickyWithCopyAllScoresIfListOfFilesDidNotChangeWhenFileIsModifiedDueToRebase_withoutCopyCondition()
- throws Exception {
- try (ProjectConfigUpdate u = updateProject(project)) {
- u.getConfig()
- .updateLabelType(
- LabelId.CODE_REVIEW, b -> b.setCopyAllScoresIfListOfFilesDidNotChange(true));
- u.save();
- }
- // Create two changes both with the same parent
- PushOneCommit.Result r = createChange();
- testRepo.reset("HEAD~1");
- PushOneCommit.Result r2 = createChange();
-
- // Modify f.txt in change 1. Approve and submit the first change
- gApi.changes().id(r.getChangeId()).edit().modifyFile("f.txt", RawInputUtil.create("content"));
- gApi.changes().id(r.getChangeId()).edit().publish();
- RevisionApi revision = gApi.changes().id(r.getChangeId()).current();
- revision.review(ReviewInput.approve().label(LabelId.VERIFIED, 1));
- revision.submit();
-
- // Add an approval whose score should be copied on change 2.
- gApi.changes().id(r2.getChangeId()).current().review(ReviewInput.recommend());
-
- // Rebase the second change. The rebase adds f1.txt.
- gApi.changes().id(r2.getChangeId()).rebase();
-
- // The code-review approval is copied for the second change between PS1 and PS2 since the only
- // modified file is due to rebase.
- List<PatchSetApproval> patchSetApprovals =
- r2.getChange().notes().getApprovalsWithCopied().values().stream()
- .sorted(comparing(a -> a.patchSetId().get()))
- .collect(toImmutableList());
- PatchSetApproval nonCopied = patchSetApprovals.get(0);
- PatchSetApproval copied = patchSetApprovals.get(1);
- assertCopied(nonCopied, /* psId= */ 1, LabelId.CODE_REVIEW, (short) 1, false);
- assertCopied(copied, /* psId= */ 2, LabelId.CODE_REVIEW, (short) 1, true);
- }
-
- @Test
- public void stickyWithCopyAllScoresIfListOfFilesDidNotChangeWhenFileIsModified_withCopyCondition()
- throws Exception {
- try (ProjectConfigUpdate u = updateProject(project)) {
- u.getConfig()
- .updateLabelType(LabelId.CODE_REVIEW, b -> b.setCopyCondition("has:unchanged-files"));
- u.save();
- }
- stickyWithCopyAllScoresIfListOfFilesDidNotChangeWhenFileIsModified();
- }
-
- private void stickyWithCopyAllScoresIfListOfFilesDidNotChangeWhenFileIsModified()
- throws Exception {
- Change.Id changeId =
- changeOperations.newChange().project(project).file("file").content("content").create();
- vote(admin, changeId.toString(), 2, 1);
- vote(user, changeId.toString(), -2, -1);
-
- changeOperations.change(changeId).newPatchset().file("file").content("new content").create();
- ChangeInfo c = detailedChange(changeId.toString());
-
- // only code review votes are copied since copyAllScoresIfListOfFilesDidNotChange is
- // configured for that label, and list of files didn't change.
- assertVotes(c, admin, 2, 0);
- assertVotes(c, user, -2, 0);
- }
-
- @TestProjectInput(createEmptyCommit = false)
- public void
- stickyWithCopyAllScoresIfListOfFilesDidNotChangeWhenFileIsModifiedAsInitialCommit_withoutCopyCondition()
- throws Exception {
- try (ProjectConfigUpdate u = updateProject(project)) {
- u.getConfig()
- .updateLabelType(
- LabelId.CODE_REVIEW, b -> b.setCopyAllScoresIfListOfFilesDidNotChange(true));
- u.save();
- }
- stickyWithCopyAllScoresIfListOfFilesDidNotChangeWhenFileIsModifiedAsInitialCommit();
- }
-
- @TestProjectInput(createEmptyCommit = false)
- public void
- stickyWithCopyAllScoresIfListOfFilesDidNotChangeWhenFileIsModifiedAsInitialCommit_withCopyCondition()
- throws Exception {
- try (ProjectConfigUpdate u = updateProject(project)) {
- u.getConfig()
- .updateLabelType(LabelId.CODE_REVIEW, b -> b.setCopyCondition("has:unchanged-files"));
- u.save();
- }
- stickyWithCopyAllScoresIfListOfFilesDidNotChangeWhenFileIsModifiedAsInitialCommit();
- }
-
- private void stickyWithCopyAllScoresIfListOfFilesDidNotChangeWhenFileIsModifiedAsInitialCommit()
- throws Exception {
- Change.Id changeId =
- changeOperations.newChange().project(project).file("file").content("content").create();
- vote(admin, changeId.toString(), 2, 1);
- vote(user, changeId.toString(), -2, -1);
-
- changeOperations.change(changeId).newPatchset().file("file").content("new content").create();
- ChangeInfo c = detailedChange(changeId.toString());
-
- // only code review votes are copied since copyAllScoresIfListOfFilesDidNotChange is
- // configured for that label, and list of files didn't change.
- assertVotes(c, admin, 2, 0);
- assertVotes(c, user, -2, 0);
- }
-
- @Test
- public void
- notStickyWithCopyAllScoresIfListOfFilesDidNotChangeWhenFileIsModifiedOnEarlierPatchset_withoutCopyCondition()
- throws Exception {
- try (ProjectConfigUpdate u = updateProject(project)) {
- u.getConfig()
- .updateLabelType(
- LabelId.CODE_REVIEW, b -> b.setCopyAllScoresIfListOfFilesDidNotChange(true));
- u.save();
- }
- notStickyWithCopyAllScoresIfListOfFilesDidNotChangeWhenFileIsModifiedOnEarlierPatchset();
- }
-
- @Test
- public void
- notStickyWithCopyAllScoresIfListOfFilesDidNotChangeWhenFileIsModifiedOnEarlierPatchset_withCopyCondition()
- throws Exception {
- try (ProjectConfigUpdate u = updateProject(project)) {
- u.getConfig()
- .updateLabelType(LabelId.CODE_REVIEW, b -> b.setCopyCondition("has:unchanged-files"));
- u.save();
- }
- notStickyWithCopyAllScoresIfListOfFilesDidNotChangeWhenFileIsModifiedOnEarlierPatchset();
- }
-
- private void
- notStickyWithCopyAllScoresIfListOfFilesDidNotChangeWhenFileIsModifiedOnEarlierPatchset()
- throws Exception {
- Change.Id changeId =
- changeOperations.newChange().project(project).file("file").content("content").create();
- vote(admin, changeId.toString(), 2, 1);
- vote(user, changeId.toString(), -2, -1);
-
- changeOperations.change(changeId).newPatchset().file("new file").content("content").create();
- changeOperations
- .change(changeId)
- .newPatchset()
- .file("new file")
- .content("new content")
- .create();
- ChangeInfo c = detailedChange(changeId.toString());
-
- // Don't copy over votes since ps1->ps2 should copy over, but ps2->ps3 should not.
- assertVotes(c, admin, 0, 0);
- assertVotes(c, user, 0, 0);
- }
-
- @Test
- public void
- notStickyWithCopyAllScoresIfListOfFilesDidNotChangeWhenFileIsRenamed_withoutCopyCondition()
- throws Exception {
- try (ProjectConfigUpdate u = updateProject(project)) {
- u.getConfig()
- .updateLabelType(
- LabelId.CODE_REVIEW, b -> b.setCopyAllScoresIfListOfFilesDidNotChange(true));
- u.save();
- }
- notStickyWithCopyAllScoresIfListOfFilesDidNotChangeWhenFileIsRenamed();
- }
-
- @Test
- public void
- notStickyWithCopyAllScoresIfListOfFilesDidNotChangeWhenFileIsRenamed_withCopyCondition()
- throws Exception {
- try (ProjectConfigUpdate u = updateProject(project)) {
- u.getConfig()
- .updateLabelType(LabelId.CODE_REVIEW, b -> b.setCopyCondition("has:unchanged-files"));
- u.save();
- }
- notStickyWithCopyAllScoresIfListOfFilesDidNotChangeWhenFileIsRenamed();
- }
-
- private void notStickyWithCopyAllScoresIfListOfFilesDidNotChangeWhenFileIsRenamed()
- throws Exception {
- Change.Id changeId =
- changeOperations.newChange().project(project).file("file").content("content").create();
- vote(admin, changeId.toString(), 2, 1);
- vote(user, changeId.toString(), -2, -1);
-
- changeOperations.change(changeId).newPatchset().file("file").renameTo("new_file").create();
- ChangeInfo c = detailedChange(changeId.toString());
-
- // no votes are copied since the list of files changed (rename).
- assertVotes(c, admin, 0, 0);
- assertVotes(c, user, 0, 0);
- }
-
- @Test
- public void removedVotesNotSticky() throws Exception {
- try (ProjectConfigUpdate u = updateProject(project)) {
- u.getConfig()
- .updateLabelType(LabelId.CODE_REVIEW, b -> b.setCopyAllScoresOnTrivialRebase(true));
- u.getConfig().updateLabelType(LabelId.VERIFIED, b -> b.setCopyAllScoresIfNoCodeChange(true));
- u.save();
- }
-
- for (ChangeKind changeKind :
- EnumSet.of(REWORK, TRIVIAL_REBASE, NO_CODE_CHANGE, MERGE_FIRST_PARENT_UPDATE, NO_CHANGE)) {
- testRepo.reset(projectOperations.project(project).getHead("master"));
-
- String changeId = changeKindCreator.createChange(changeKind, testRepo, admin);
- vote(admin, changeId, 2, 1);
- vote(user, changeId, -2, -1);
-
- // Remove votes by re-voting with 0
- vote(admin, changeId, 0, 0);
- vote(user, changeId, 0, 0);
- ChangeInfo c = detailedChange(changeId);
- assertVotes(c, admin, 0, 0, null);
- assertVotes(c, user, 0, 0, null);
-
- changeKindCreator.updateChange(changeId, changeKind, testRepo, admin, project);
- c = detailedChange(changeId);
- assertVotes(c, admin, 0, 0, changeKind);
- assertVotes(c, user, 0, 0, changeKind);
- }
- }
-
- @Test
- public void stickyAcrossMultiplePatchSets() throws Exception {
- try (ProjectConfigUpdate u = updateProject(project)) {
- u.getConfig().updateLabelType(LabelId.CODE_REVIEW, b -> b.setCopyMaxScore(true));
- u.getConfig().updateLabelType(LabelId.VERIFIED, b -> b.setCopyAllScoresIfNoCodeChange(true));
- u.save();
- }
-
- String changeId = changeKindCreator.createChange(REWORK, testRepo, admin);
- vote(admin, changeId, 2, 1);
-
- for (int i = 0; i < 5; i++) {
- changeKindCreator.updateChange(changeId, NO_CODE_CHANGE, testRepo, admin, project);
- ChangeInfo c = detailedChange(changeId);
- assertVotes(c, admin, 2, 1, NO_CODE_CHANGE);
- }
-
- changeKindCreator.updateChange(changeId, REWORK, testRepo, admin, project);
- ChangeInfo c = detailedChange(changeId);
- assertVotes(c, admin, 2, 0, REWORK);
- }
-
- @Test
- public void stickyAcrossMultiplePatchSetsDoNotRegressPerformance() throws Exception {
- // The purpose of this test is to make sure that we compute change kind only against the parent
- // patch set. Change kind is a heavy operation. In prior version of Gerrit, we computed the
- // change kind against all prior patch sets. This is a regression that made Gerrit do expensive
- // work in O(num-patch-sets). This test ensures that we aren't regressing.
- try (ProjectConfigUpdate u = updateProject(project)) {
- u.getConfig().updateLabelType(LabelId.CODE_REVIEW, b -> b.setCopyMaxScore(true));
- u.getConfig().updateLabelType(LabelId.VERIFIED, b -> b.setCopyAllScoresIfNoCodeChange(true));
- u.save();
- }
-
- String changeId = changeKindCreator.createChange(REWORK, testRepo, admin);
- vote(admin, changeId, 2, 1);
- changeKindCreator.updateChange(changeId, NO_CODE_CHANGE, testRepo, admin, project);
- changeKindCreator.updateChange(changeId, NO_CODE_CHANGE, testRepo, admin, project);
- changeKindCreator.updateChange(changeId, NO_CODE_CHANGE, testRepo, admin, project);
-
- Map<Integer, ObjectId> revisions = new HashMap<>();
- gApi.changes()
- .id(changeId)
- .get()
- .revisions
- .forEach(
- (revId, revisionInfo) ->
- revisions.put(revisionInfo._number, ObjectId.fromString(revId)));
- assertThat(revisions.size()).isEqualTo(4);
- assertChangeKindCacheContains(revisions.get(3), revisions.get(4));
- assertChangeKindCacheContains(revisions.get(2), revisions.get(3));
- assertChangeKindCacheContains(revisions.get(1), revisions.get(2));
-
- assertChangeKindCacheDoesNotContain(revisions.get(1), revisions.get(4));
- assertChangeKindCacheDoesNotContain(revisions.get(2), revisions.get(4));
- assertChangeKindCacheDoesNotContain(revisions.get(1), revisions.get(3));
- }
-
- @Test
- public void copyMinMaxAcrossMultiplePatchSets() throws Exception {
- try (ProjectConfigUpdate u = updateProject(project)) {
- u.getConfig().updateLabelType(LabelId.CODE_REVIEW, b -> b.setCopyMaxScore(true));
- u.getConfig().updateLabelType(LabelId.CODE_REVIEW, b -> b.setCopyMinScore(true));
- u.save();
- }
-
- // Vote max score on PS1
- String changeId = changeKindCreator.createChange(REWORK, testRepo, admin);
- vote(admin, changeId, 2, 1);
-
- // Have someone else vote min score on PS2
- changeKindCreator.updateChange(changeId, REWORK, testRepo, admin, project);
- vote(user, changeId, -2, 0);
- ChangeInfo c = detailedChange(changeId);
- assertVotes(c, admin, 2, 0, REWORK);
- assertVotes(c, user, -2, 0, REWORK);
-
- // No vote changes on PS3
- changeKindCreator.updateChange(changeId, REWORK, testRepo, admin, project);
- c = detailedChange(changeId);
- assertVotes(c, admin, 2, 0, REWORK);
- assertVotes(c, user, -2, 0, REWORK);
-
- // Both users revote on PS4
- changeKindCreator.updateChange(changeId, REWORK, testRepo, admin, project);
- vote(admin, changeId, 1, 1);
- vote(user, changeId, 1, 1);
- c = detailedChange(changeId);
- assertVotes(c, admin, 1, 1, REWORK);
- assertVotes(c, user, 1, 1, REWORK);
-
- // New approvals shouldn't carry through to PS5
- changeKindCreator.updateChange(changeId, REWORK, testRepo, admin, project);
- c = detailedChange(changeId);
- assertVotes(c, admin, 0, 0, REWORK);
- assertVotes(c, user, 0, 0, REWORK);
- }
-
- @Test
- public void copyWithListOfFilesUnchanged_withoutCopyCondition() throws Exception {
- try (ProjectConfigUpdate u = updateProject(project)) {
- u.getConfig()
- .updateLabelType(
- LabelId.CODE_REVIEW, b -> b.setCopyAllScoresIfListOfFilesDidNotChange(true));
- u.save();
- }
- copyWithListOfFilesUnchanged();
- }
-
- @Test
- public void copyWithListOfFilesUnchanged_withCopyCondition() throws Exception {
- try (ProjectConfigUpdate u = updateProject(project)) {
- u.getConfig()
- .updateLabelType(LabelId.CODE_REVIEW, b -> b.setCopyCondition("has:unchanged-files"));
- u.save();
- }
- copyWithListOfFilesUnchanged();
- }
-
- private void copyWithListOfFilesUnchanged() throws Exception {
- Change.Id changeId =
- changeOperations.newChange().project(project).file("file").content("content").create();
- vote(admin, changeId.toString(), 2, 1);
- vote(user, changeId.toString(), -2, -1);
-
- changeOperations.change(changeId).newPatchset().file("file").content("new content").create();
- ChangeInfo c = detailedChange(changeId.toString());
-
- // Code-Review votes are copied over from ps1-> ps2 since the list of files were unchanged.
- assertVotes(c, admin, 2, 0);
- assertVotes(c, user, -2, 0);
-
- changeOperations
- .change(changeId)
- .newPatchset()
- .file("file")
- .content("very new content")
- .create();
- c = detailedChange(changeId.toString());
-
- // Code-Review votes are copied over from ps1-> ps3 since the list of files were unchanged.
- assertVotes(c, admin, 2, 0);
- assertVotes(c, user, -2, 0);
-
- changeOperations
- .change(changeId)
- .newPatchset()
- .file("new file")
- .content("new content")
- .create();
-
- c = detailedChange(changeId.toString());
- // Code-Review votes are not copied over from ps1-> ps4 since a file was added.
- assertVotes(c, admin, 0, 0);
- assertVotes(c, user, 0, 0);
-
- changeOperations.change(changeId).newPatchset().file("file").content("content").create();
-
- c = detailedChange(changeId.toString());
- // Code-Review votes are not copied over from ps1 -> ps5 since a file was added on ps4.
- // Although the list of files is the same between ps4->ps5, we don't copy votes from before
- // ps4.
- assertVotes(c, admin, 0, 0);
- assertVotes(c, user, 0, 0);
- }
-
- @Test
- public void copyWithListOfFilesUnchangedButAddedMergeList() throws Exception {
- try (ProjectConfigUpdate u = updateProject(project)) {
- u.getConfig()
- .updateLabelType(LabelId.CODE_REVIEW, b -> b.setCopyCondition("has:unchanged-files"));
- u.save();
- }
- Change.Id parent1ChangeId = changeOperations.newChange().create();
- Change.Id parent2ChangeId = changeOperations.newChange().create();
- Change.Id dummyParentChangeId = changeOperations.newChange().create();
- Change.Id changeId =
- changeOperations
- .newChange()
- .mergeOf()
- .change(parent1ChangeId)
- .and()
- .change(parent2ChangeId)
- .create();
-
- Map<String, FileInfo> changedFilesFirstPatchset =
- gApi.changes().id(changeId.get()).current().files();
-
- assertThat(changedFilesFirstPatchset.keySet()).containsExactly("/COMMIT_MSG", "/MERGE_LIST");
-
- // Make a Code-Review vote that should be sticky.
- gApi.changes().id(changeId.get()).current().review(ReviewInput.approve());
-
- changeOperations
- .change(changeId)
- .newPatchset()
- .parent()
- .patchset(PatchSet.id(dummyParentChangeId, 1))
- .create();
-
- Map<String, FileInfo> changedFilesSecondPatchset =
- gApi.changes().id(changeId.get()).current().files();
-
- // Only "/MERGE_LIST" was removed.
- assertThat(changedFilesSecondPatchset.keySet()).containsExactly("/COMMIT_MSG");
- ApprovalInfo approvalInfo =
- Iterables.getOnlyElement(
- gApi.changes().id(changeId.get()).current().votes().get(LabelId.CODE_REVIEW));
- assertThat(approvalInfo._accountId).isEqualTo(admin.id().get());
- assertThat(approvalInfo.value).isEqualTo(2);
- }
-
- @Test
- public void deleteStickyVote() throws Exception {
- String label = LabelId.CODE_REVIEW;
- try (ProjectConfigUpdate u = updateProject(project)) {
- u.getConfig().updateLabelType(label, b -> b.setCopyMaxScore(true));
- u.save();
- }
-
- // Vote max score on PS1
- String changeId = changeKindCreator.createChange(REWORK, testRepo, admin);
- vote(admin, changeId, label, 2);
- assertVotes(detailedChange(changeId), admin, label, 2, null);
- changeKindCreator.updateChange(changeId, REWORK, testRepo, admin, project);
- assertVotes(detailedChange(changeId), admin, label, 2, REWORK);
-
- // Delete vote that was copied via sticky approval
- deleteVote(admin, changeId, label);
- assertVotes(detailedChange(changeId), admin, label, 0, REWORK);
- }
-
- @Test
- public void canVoteMultipleTimesOnNewPatchsets() throws Exception {
- // Code-Review will be sticky.
- String label = LabelId.CODE_REVIEW;
- try (ProjectConfigUpdate u = updateProject(project)) {
- u.getConfig().updateLabelType(label, b -> b.setCopyAnyScore(true));
- u.save();
- }
-
- PushOneCommit.Result r = createChange();
-
- // Add a new vote.
- ReviewInput input = new ReviewInput().label(LabelId.CODE_REVIEW, 2);
- gApi.changes().id(r.getChangeId()).current().review(input);
-
- // Make a new patchset, keeping the Code-Review +2 vote.
- amendChange(r.getChangeId());
-
- // Post without changing the vote.
- input = new ReviewInput().label(LabelId.CODE_REVIEW, 2);
- gApi.changes().id(r.getChangeId()).current().review(input);
-
- // There is a vote both on patchset 1 and on patchset 2, although both votes are Code-Review +2.
- assertThat(r.getChange().approvals().get(PatchSet.id(r.getChange().getId(), 1))).hasSize(1);
- assertThat(r.getChange().approvals().get(PatchSet.id(r.getChange().getId(), 2))).hasSize(1);
- }
-
- @Test
- public void stickyVoteStoredOnUpload() throws Exception {
- // Code-Review will be sticky.
- String label = LabelId.CODE_REVIEW;
- try (ProjectConfigUpdate u = updateProject(project)) {
- u.getConfig().updateLabelType(label, b -> b.setCopyAnyScore(true));
- u.save();
- }
-
- PushOneCommit.Result r = createChange();
- // Add a new vote.
- ReviewInput input = new ReviewInput().label(LabelId.CODE_REVIEW, 2);
- input.tag = "tag";
- gApi.changes().id(r.getChangeId()).current().review(input);
-
- // Make new patchsets, keeping the Code-Review +2 vote.
- for (int i = 0; i < 9; i++) {
- amendChange(r.getChangeId());
- }
-
- List<PatchSetApproval> patchSetApprovals =
- r.getChange().notes().getApprovalsWithCopied().values().stream()
- .sorted(comparing(a -> a.patchSetId().get()))
- .collect(toImmutableList());
-
- for (int i = 0; i < 10; i++) {
- int patchSet = i + 1;
- assertThat(patchSetApprovals.get(i).patchSetId().get()).isEqualTo(patchSet);
- assertThat(patchSetApprovals.get(i).accountId().get()).isEqualTo(admin.id().get());
- assertThat(patchSetApprovals.get(i).realAccountId().get()).isEqualTo(admin.id().get());
- assertThat(patchSetApprovals.get(i).label()).isEqualTo(LabelId.CODE_REVIEW);
- assertThat(patchSetApprovals.get(i).value()).isEqualTo((short) 2);
- assertThat(patchSetApprovals.get(i).tag().get()).isEqualTo("tag");
- if (patchSet == 1) {
- assertThat(patchSetApprovals.get(i).copied()).isFalse();
- } else {
- assertThat(patchSetApprovals.get(i).copied()).isTrue();
- }
- }
- }
-
- @Test
- public void stickyVoteStoredOnRebase() throws Exception {
- // Code-Review will be sticky.
- String label = LabelId.CODE_REVIEW;
- try (ProjectConfigUpdate u = updateProject(project)) {
- u.getConfig().updateLabelType(label, b -> b.setCopyAnyScore(true));
- u.save();
- }
-
- // Create two changes both with the same parent
- PushOneCommit.Result r = createChange();
- testRepo.reset("HEAD~1");
- PushOneCommit.Result r2 = createChange();
-
- // Approve and submit the first change
- RevisionApi revision = gApi.changes().id(r.getChangeId()).current();
- revision.review(ReviewInput.approve().label(LabelId.VERIFIED, 1));
- revision.submit();
-
- // Add an approval whose score should be copied.
- gApi.changes().id(r2.getChangeId()).current().review(ReviewInput.recommend());
-
- // Rebase the second change
- gApi.changes().id(r2.getChangeId()).rebase();
-
- List<PatchSetApproval> patchSetApprovals =
- r2.getChange().notes().getApprovalsWithCopied().values().stream()
- .sorted(comparing(a -> a.patchSetId().get()))
- .collect(toImmutableList());
- PatchSetApproval nonCopied = patchSetApprovals.get(0);
- PatchSetApproval copied = patchSetApprovals.get(1);
- assertCopied(nonCopied, 1, LabelId.CODE_REVIEW, (short) 1, /* copied= */ false);
- assertCopied(copied, 2, LabelId.CODE_REVIEW, (short) 1, /* copied= */ true);
- }
-
- @Test
- public void stickyVoteStoredOnUploadWithRealAccount() throws Exception {
- // Give "user" permission to vote on behalf of other users.
- projectOperations
- .project(project)
- .forUpdate()
- .add(
- allowLabel(TestLabels.codeReview().getName())
- .impersonation(true)
- .ref("refs/heads/*")
- .group(REGISTERED_USERS)
- .range(-1, 1))
- .update();
-
- // Code-Review will be sticky.
- String label = LabelId.CODE_REVIEW;
- try (ProjectConfigUpdate u = updateProject(project)) {
- u.getConfig().updateLabelType(label, b -> b.setCopyAnyScore(true));
- u.save();
- }
-
- PushOneCommit.Result r = createChange();
-
- // Add a new vote as user
- requestScopeOperations.setApiUser(user.id());
- ReviewInput input = new ReviewInput().label(LabelId.CODE_REVIEW, 1);
- input.onBehalfOf = admin.email();
- gApi.changes().id(r.getChangeId()).current().review(input);
-
- // Make a new patchset, keeping the Code-Review +1 vote.
- amendChange(r.getChangeId());
-
- List<PatchSetApproval> patchSetApprovals =
- r.getChange().notes().getApprovalsWithCopied().values().stream()
- .sorted(comparing(a -> a.patchSetId().get()))
- .collect(toImmutableList());
-
- PatchSetApproval nonCopied = patchSetApprovals.get(0);
- assertThat(nonCopied.patchSetId().get()).isEqualTo(1);
- assertThat(nonCopied.accountId().get()).isEqualTo(admin.id().get());
- assertThat(nonCopied.realAccountId().get()).isEqualTo(user.id().get());
- assertThat(nonCopied.label()).isEqualTo(LabelId.CODE_REVIEW);
- assertThat(nonCopied.value()).isEqualTo((short) 1);
- assertThat(nonCopied.copied()).isFalse();
-
- PatchSetApproval copied = patchSetApprovals.get(1);
- assertThat(copied.patchSetId().get()).isEqualTo(2);
- assertThat(copied.accountId().get()).isEqualTo(admin.id().get());
- assertThat(copied.realAccountId().get()).isEqualTo(user.id().get());
- assertThat(copied.label()).isEqualTo(LabelId.CODE_REVIEW);
- assertThat(copied.value()).isEqualTo((short) 1);
- assertThat(copied.copied()).isTrue();
- }
-
- @Test
- public void stickyVoteStoredOnUploadWithRealAccountAndTag() throws Exception {
- // Give "user" permission to vote on behalf of other users.
- projectOperations
- .project(project)
- .forUpdate()
- .add(
- allowLabel(TestLabels.codeReview().getName())
- .impersonation(true)
- .ref("refs/heads/*")
- .group(REGISTERED_USERS)
- .range(-1, 1))
- .update();
-
- // Code-Review will be sticky.
- String label = LabelId.CODE_REVIEW;
- try (ProjectConfigUpdate u = updateProject(project)) {
- u.getConfig().updateLabelType(label, b -> b.setCopyAnyScore(true));
- u.save();
- }
-
- PushOneCommit.Result r = createChange();
-
- // Add a new vote as user
- requestScopeOperations.setApiUser(user.id());
- ReviewInput input = new ReviewInput().label(LabelId.CODE_REVIEW, 1);
- input.onBehalfOf = admin.email();
- input.tag = "tag";
- gApi.changes().id(r.getChangeId()).current().review(input);
-
- // Make a new patchset, keeping the Code-Review +1 vote.
- amendChange(r.getChangeId());
-
- List<PatchSetApproval> patchSetApprovals =
- r.getChange().notes().getApprovalsWithCopied().values().stream()
- .sorted(comparing(a -> a.patchSetId().get()))
- .collect(toImmutableList());
-
- PatchSetApproval nonCopied = patchSetApprovals.get(0);
- assertThat(nonCopied.patchSetId().get()).isEqualTo(1);
- assertThat(nonCopied.accountId().get()).isEqualTo(admin.id().get());
- assertThat(nonCopied.realAccountId().get()).isEqualTo(user.id().get());
- assertThat(nonCopied.label()).isEqualTo(LabelId.CODE_REVIEW);
- assertThat(nonCopied.value()).isEqualTo((short) 1);
- assertThat(nonCopied.tag().get()).isEqualTo("tag");
- assertThat(nonCopied.copied()).isFalse();
-
- PatchSetApproval copied = patchSetApprovals.get(1);
- assertThat(copied.patchSetId().get()).isEqualTo(2);
- assertThat(copied.accountId().get()).isEqualTo(admin.id().get());
- assertThat(copied.realAccountId().get()).isEqualTo(user.id().get());
- assertThat(copied.label()).isEqualTo(LabelId.CODE_REVIEW);
- assertThat(copied.value()).isEqualTo((short) 1);
- assertThat(nonCopied.tag().get()).isEqualTo("tag");
- assertThat(copied.copied()).isTrue();
- }
-
- @Test
- public void stickyVoteStoredCanBeRemoved() throws Exception {
- // Code-Review will be sticky.
- String label = LabelId.CODE_REVIEW;
- try (ProjectConfigUpdate u = updateProject(project)) {
- u.getConfig().updateLabelType(label, b -> b.setCopyAnyScore(true));
- u.save();
- }
-
- PushOneCommit.Result r = createChange();
-
- // Add a new vote
- ReviewInput input = new ReviewInput().label(LabelId.CODE_REVIEW, 2);
- gApi.changes().id(r.getChangeId()).current().review(input);
-
- // Make a new patchset, keeping the Code-Review +2 vote.
- amendChange(r.getChangeId());
- assertVotes(detailedChange(r.getChangeId()), admin, label, 2, null);
-
- gApi.changes().id(r.getChangeId()).current().review(ReviewInput.noScore());
-
- PatchSetApproval nonCopiedSecondPatchsetRemovedVote =
- Iterables.getOnlyElement(
- r.getChange()
- .notes()
- .getApprovalsWithCopied()
- .get(r.getChange().change().currentPatchSetId()));
-
- assertThat(nonCopiedSecondPatchsetRemovedVote.patchSetId().get()).isEqualTo(2);
- assertThat(nonCopiedSecondPatchsetRemovedVote.accountId().get()).isEqualTo(admin.id().get());
- assertThat(nonCopiedSecondPatchsetRemovedVote.label()).isEqualTo(LabelId.CODE_REVIEW);
- // The vote got removed since the latest patch-set only has one vote and it's "0".
- assertThat(nonCopiedSecondPatchsetRemovedVote.value()).isEqualTo((short) 0);
- assertThat(nonCopiedSecondPatchsetRemovedVote.copied()).isFalse();
- }
-
- @Test
- public void reviewerStickyVotingCanBeRemoved() throws Exception {
- // Code-Review will be sticky.
- String label = LabelId.CODE_REVIEW;
- try (ProjectConfigUpdate u = updateProject(project)) {
- u.getConfig().updateLabelType(label, b -> b.setCopyAnyScore(true));
- u.save();
- }
-
- PushOneCommit.Result r = createChange();
-
- // Add a new vote by user
- requestScopeOperations.setApiUser(user.id());
- gApi.changes().id(r.getChangeId()).current().review(ReviewInput.recommend());
- requestScopeOperations.setApiUser(admin.id());
-
- // Make a new patchset, keeping the Code-Review +1 vote.
- amendChange(r.getChangeId());
- assertVotes(detailedChange(r.getChangeId()), user, label, 1, null);
-
- gApi.changes().id(r.getChangeId()).reviewer(user.email()).remove();
-
- assertThat(r.getChange().notes().getApprovalsWithCopied()).isEmpty();
-
- // Changes message has info about vote removed.
- assertThat(Iterables.getLast(gApi.changes().id(r.getChangeId()).messages()).message)
- .contains("Code-Review+1 by User");
- }
-
private void assertChangeKindCacheContains(ObjectId prior, ObjectId next) {
ChangeKind kind =
changeKindCache.getIfPresent(ChangeKindCacheImpl.Key.create(prior, next, "recursive"));
@@ -1501,6 +1357,21 @@
assertThat(approval.copied()).isEqualTo(copied);
}
+ private void updateCodeReviewLabel(Consumer<LabelType.Builder> update) throws Exception {
+ updateLabel(LabelId.CODE_REVIEW, update);
+ }
+
+ private void updateVerifiedLabel(Consumer<LabelType.Builder> update) throws Exception {
+ updateLabel(LabelId.VERIFIED, update);
+ }
+
+ private void updateLabel(String labelName, Consumer<LabelType.Builder> update) throws Exception {
+ try (ProjectConfigUpdate u = updateProject(project)) {
+ u.getConfig().updateLabelType(labelName, update);
+ u.save();
+ }
+ }
+
/**
* Test submit rule that always return a passing record with a "Verified" label applied by {@link
* TestSubmitRule#userAccountId}.
diff --git a/javatests/com/google/gerrit/acceptance/server/query/ApprovalQueryIT.java b/javatests/com/google/gerrit/acceptance/server/query/ApprovalQueryIT.java
index 925c855..44b59bf 100644
--- a/javatests/com/google/gerrit/acceptance/server/query/ApprovalQueryIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/query/ApprovalQueryIT.java
@@ -67,6 +67,29 @@
}
@Test
+ public void exactValuePredicate() throws Exception {
+ ApprovalContext approvalContextCodeReviewPlusOne = contextForCodeReviewLabel(1);
+ assertFalse(
+ queryBuilder.parse("is:\"-2\"").asMatchable().match(approvalContextCodeReviewPlusOne));
+ assertFalse(
+ queryBuilder.parse("is:\"-1\"").asMatchable().match(approvalContextCodeReviewPlusOne));
+ assertFalse(queryBuilder.parse("is:0").asMatchable().match(approvalContextCodeReviewPlusOne));
+ assertTrue(queryBuilder.parse("is:1").asMatchable().match(approvalContextCodeReviewPlusOne));
+ assertFalse(queryBuilder.parse("is:2").asMatchable().match(approvalContextCodeReviewPlusOne));
+ }
+
+ @Test
+ public void isPredicate_invalidValue() throws Exception {
+ QueryParseException thrown =
+ assertThrows(QueryParseException.class, () -> queryBuilder.parse("is:INVALID"));
+ assertThat(thrown)
+ .hasMessageThat()
+ .contains(
+ "INVALID is not a valid value for operator 'is'. Valid values: ANY, MAX, MIN"
+ + " or integer");
+ }
+
+ @Test
public void changeKindPredicate_noCodeChange() throws Exception {
String change = changeKindCreator.createChange(ChangeKind.NO_CODE_CHANGE, testRepo, admin);
changeKindCreator.updateChange(change, ChangeKind.NO_CODE_CHANGE, testRepo, admin, project);
@@ -127,6 +150,17 @@
}
@Test
+ public void changeKindPredicate_invalidValue() throws Exception {
+ QueryParseException thrown =
+ assertThrows(QueryParseException.class, () -> queryBuilder.parse("changekind:INVALID"));
+ assertThat(thrown)
+ .hasMessageThat()
+ .contains(
+ "INVALID is not a valid value for operator 'changekind'. Valid values:"
+ + " MERGE_FIRST_PARENT_UPDATE, NO_CHANGE, NO_CODE_CHANGE, REWORK, TRIVIAL_REBASE");
+ }
+
+ @Test
public void uploaderInPredicate() throws Exception {
String administratorsUUID = gApi.groups().query("name:Administrators").get().get(0).id;
@@ -241,7 +275,15 @@
assertThat(thrown)
.hasMessageThat()
.contains(
- "'invalid' is not a supported argument for has. only 'unchanged-files' is supported");
+ "'invalid' is not a valid value for operator 'has'."
+ + " The only valid value is 'unchanged-files'.");
+ }
+
+ @Test
+ public void invalidQuery() throws Exception {
+ QueryParseException thrown =
+ assertThrows(QueryParseException.class, () -> queryBuilder.parse("INVALID"));
+ assertThat(thrown).hasMessageThat().contains("Unsupported query: INVALID");
}
private ApprovalContext contextForCodeReviewLabel(int value) throws Exception {
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
index 2456170..7625756 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
@@ -581,14 +581,6 @@
}, createDefaultPatchChange());
}
- _getDiffPreferences() {
- return this.restApiService.getDiffPreferences();
- }
-
- _getPreferences() {
- return this.restApiService.getPreferences();
- }
-
// private but used in test
_toggleFileExpanded(file: PatchSetFile) {
// Is the path in the list of expanded diffs? If so, remove it, otherwise
diff --git a/polygerrit-ui/app/elements/change/gr-label-scores/gr-label-scores.ts b/polygerrit-ui/app/elements/change/gr-label-scores/gr-label-scores.ts
index 6035b6f..993f811 100644
--- a/polygerrit-ui/app/elements/change/gr-label-scores/gr-label-scores.ts
+++ b/polygerrit-ui/app/elements/change/gr-label-scores/gr-label-scores.ts
@@ -24,7 +24,6 @@
ChangeInfo,
AccountInfo,
DetailedLabelInfo,
- LabelNameToInfoMap,
LabelNameToValuesMap,
} from '../../../types/common';
import {
@@ -35,8 +34,8 @@
import {getAppContext} from '../../../services/app-context';
import {
getTriggerVotes,
- labelCompare,
showNewSubmitRequirements,
+ computeLabels,
} from '../../../utils/label-util';
import {ChangeStatus} from '../../../constants/constants';
import {fontStyles} from '../../../styles/gr-font-styles';
@@ -106,7 +105,7 @@
}
private renderOldSubmitRequirements() {
- const labels = this._computeLabels();
+ const labels = computeLabels(this.account, this.change);
return html`${this.renderLabels(labels)}${this.renderErrorMessages()}`;
}
@@ -117,7 +116,7 @@
private renderSubmitReqsLabels() {
const triggerVotes = getTriggerVotes(this.change);
- const labels = this._computeLabels().filter(
+ const labels = computeLabels(this.account, this.change).filter(
label => !triggerVotes.includes(label.name)
);
if (!labels.length) return;
@@ -135,7 +134,7 @@
private renderTriggerVotes() {
const triggerVotes = getTriggerVotes(this.change);
- const labels = this._computeLabels().filter(label =>
+ const labels = computeLabels(this.account, this.change).filter(label =>
triggerVotes.includes(label.name)
);
if (!labels.length) return;
@@ -214,27 +213,6 @@
return labels;
}
- private getStringLabelValue(
- labels: LabelNameToInfoMap,
- labelName: string,
- numberValue?: number
- ): string {
- const detailedInfo = labels[labelName] as DetailedLabelInfo;
- if (detailedInfo.values) {
- for (const labelValue of Object.keys(detailedInfo.values)) {
- if (Number(labelValue) === numberValue) {
- return labelValue;
- }
- }
- }
- // TODO: This code is sometimes executed with numberValue taking the
- // values 0 and undefined.
- // For now it is unclear how this is happening, ideally this code should
- // never be executed.
- const stringVal = `${numberValue}`;
- return stringVal;
- }
-
private getDefaultValue(labelName?: string) {
const labels = this.change?.labels;
if (!labelName || !labels?.[labelName]) return undefined;
@@ -242,43 +220,6 @@
return labelInfo.default_value;
}
- _getVoteForAccount(labelName: string): string | null {
- const labels = this.change?.labels;
- if (!labels) return null;
- const votes = labels[labelName] as DetailedLabelInfo;
- if (votes.all && votes.all.length > 0) {
- for (let i = 0; i < votes.all.length; i++) {
- if (
- this.account &&
- // TODO(TS): Replace == with === and check code can assign string to _account_id instead of number
- // eslint-disable-next-line eqeqeq
- votes.all[i]._account_id == this.account._account_id
- ) {
- return this.getStringLabelValue(
- labels,
- labelName,
- votes.all[i].value
- );
- }
- }
- }
- return null;
- }
-
- _computeLabels(): Label[] {
- if (!this.account) return [];
- const labelsObj = this.change?.labels;
- if (!labelsObj) return [];
- return Object.keys(labelsObj)
- .sort(labelCompare)
- .map(key => {
- return {
- name: key,
- value: this._getVoteForAccount(key),
- };
- });
- }
-
_computeColumns() {
if (!this.permittedLabels) return;
const labels = Object.keys(this.permittedLabels);
diff --git a/polygerrit-ui/app/elements/change/gr-label-scores/gr-label-scores_test.ts b/polygerrit-ui/app/elements/change/gr-label-scores/gr-label-scores_test.ts
index 0310424..5b743e2 100644
--- a/polygerrit-ui/app/elements/change/gr-label-scores/gr-label-scores_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-label-scores/gr-label-scores_test.ts
@@ -26,6 +26,7 @@
createChange,
} from '../../../test/test-data-generators';
import {ChangeStatus} from '../../../constants/constants';
+import {getVoteForAccount} from '../../../utils/label-util';
const basicFixture = fixtureFromElement('gr-label-scores');
@@ -118,7 +119,10 @@
test('_getVoteForAccount', () => {
const labelName = 'Code-Review';
- assert.strictEqual(element._getVoteForAccount(labelName), '+1');
+ assert.strictEqual(
+ getVoteForAccount(labelName, element.account, element.change),
+ '+1'
+ );
});
test('_computeColumns', () => {
@@ -132,61 +136,6 @@
});
});
- test('changes in label score are reflected in _labels', async () => {
- const change = {
- ...createChange(),
- labels: {
- 'Code-Review': {
- values: {
- '0': 'No score',
- '+1': 'good',
- '+2': 'excellent',
- '-1': 'bad',
- '-2': 'terrible',
- },
- default_value: 0,
- },
- Verified: {
- values: {
- '0': 'No score',
- '+1': 'good',
- '+2': 'excellent',
- '-1': 'bad',
- '-2': 'terrible',
- },
- default_value: 0,
- },
- },
- };
- element.change = change;
- await flush();
- let labels = element._computeLabels();
- assert.deepEqual(labels, [
- {name: 'Code-Review', value: null},
- {name: 'Verified', value: null},
- ]);
- element.change = {
- ...change,
- labels: {
- ...change.labels,
- Verified: {
- ...change.labels.Verified,
- all: [
- {
- _account_id: accountId,
- value: 1,
- },
- ],
- },
- },
- };
- await flush();
- labels = element._computeLabels();
- assert.deepEqual(labels, [
- {name: 'Code-Review', value: null},
- {name: 'Verified', value: '+1'},
- ]);
- });
suite('message', () => {
test('shown when change is abandoned', async () => {
element.change = {
diff --git a/polygerrit-ui/app/embed/diff/gr-syntax-layer/gr-syntax-layer-worker.ts b/polygerrit-ui/app/embed/diff/gr-syntax-layer/gr-syntax-layer-worker.ts
index aa2ad07..3c6a498 100644
--- a/polygerrit-ui/app/embed/diff/gr-syntax-layer/gr-syntax-layer-worker.ts
+++ b/polygerrit-ui/app/embed/diff/gr-syntax-layer/gr-syntax-layer-worker.ts
@@ -122,8 +122,10 @@
'template-tag',
'template-variable',
'title',
+ 'title function_',
'type',
'variable',
+ 'variable language_',
]);
export class GrSyntaxLayerWorker implements DiffLayer {
diff --git a/polygerrit-ui/app/embed/diff/gr-syntax-themes/gr-syntax-theme.ts b/polygerrit-ui/app/embed/diff/gr-syntax-themes/gr-syntax-theme.ts
index 267aaaf..b74cbb3 100644
--- a/polygerrit-ui/app/embed/diff/gr-syntax-themes/gr-syntax-theme.ts
+++ b/polygerrit-ui/app/embed/diff/gr-syntax-themes/gr-syntax-theme.ts
@@ -52,6 +52,9 @@
.gr-syntax-variable {
color: var(--syntax-variable-color);
}
+ .gr-syntax-variable.language_ {
+ color: var(--syntax-variable-language-color);
+ }
.gr-syntax-template-variable {
color: var(--syntax-template-variable-color);
}
@@ -82,6 +85,9 @@
.gr-syntax-title {
color: var(--syntax-title-color);
}
+ .gr-syntax-title.function_ {
+ color: var(--syntax-title-function-color);
+ }
.gr-syntax-attr {
color: var(--syntax-attr-color);
}
diff --git a/polygerrit-ui/app/styles/themes/app-theme.ts b/polygerrit-ui/app/styles/themes/app-theme.ts
index 4e719b7..f31af21 100644
--- a/polygerrit-ui/app/styles/themes/app-theme.ts
+++ b/polygerrit-ui/app/styles/themes/app-theme.ts
@@ -409,19 +409,21 @@
--syntax-meta-keyword-color: #219;
--syntax-number-color: #164;
--syntax-params-color: var(--primary-text-color);
+ --syntax-property-color: var(--primary-text-color);
--syntax-regexp-color: #fa8602;
--syntax-selector-attr-color: #fa8602;
--syntax-selector-class-color: #164;
--syntax-selector-id-color: #2a00ff;
- --syntax-property-color: #fa8602;
--syntax-selector-pseudo-color: #fa8602;
--syntax-string-color: #2a00ff;
--syntax-tag-color: #170;
--syntax-template-tag-color: #fa8602;
--syntax-template-variable-color: #0000c0;
--syntax-title-color: #0000c0;
+ --syntax-title-function-color: var(--syntax-title-color);
--syntax-type-color: var(--blue-700);
--syntax-variable-color: var(--primary-text-color);
+ --syntax-variable-language-color: var(--syntax-built_in-color);
/* elevation */
--elevation-level-1: 0px 1px 2px 0px rgba(60, 64, 67, 0.3),
diff --git a/polygerrit-ui/app/styles/themes/dark-theme.ts b/polygerrit-ui/app/styles/themes/dark-theme.ts
index c8f0d47..6856df8 100644
--- a/polygerrit-ui/app/styles/themes/dark-theme.ts
+++ b/polygerrit-ui/app/styles/themes/dark-theme.ts
@@ -246,19 +246,21 @@
--syntax-meta-keyword-color: #eefff7;
--syntax-number-color: #00998a;
--syntax-params-color: var(--primary-text-color);
+ --syntax-property-color: #c792ea;
--syntax-regexp-color: #f77669;
--syntax-selector-attr-color: #80cbbf;
--syntax-selector-class-color: #ffcb68;
--syntax-selector-id-color: #f77669;
--syntax-selector-pseudo-color: #c792ea;
- --syntax-property-color: #c792ea;
--syntax-string-color: #c3e88d;
--syntax-tag-color: #f77669;
--syntax-template-tag-color: #c792ea;
--syntax-template-variable-color: #f77669;
--syntax-title-color: #75a5ff;
+ --syntax-title-function-color: var(--syntax-title-color);
--syntax-type-color: #dd5f5f;
--syntax-variable-color: #f77669;
+ --syntax-variable-language-color: var(--syntax-built_in-color);
/* misc */
--line-length-indicator-color: #d7aefb;
diff --git a/polygerrit-ui/app/utils/label-util.ts b/polygerrit-ui/app/utils/label-util.ts
index 563e864..aa55ea2 100644
--- a/polygerrit-ui/app/utils/label-util.ts
+++ b/polygerrit-ui/app/utils/label-util.ts
@@ -32,6 +32,7 @@
} from '../types/common';
import {ParsedChangeInfo} from '../types/types';
import {assertNever, unique} from './common-util';
+import {Label} from '../elements/change/gr-label-score-row/gr-label-score-row';
// Name of the standard Code-Review label.
export enum StandardLabels {
@@ -294,6 +295,60 @@
return priorityRequirementList.concat(nonPriorityRequirements);
}
+function getStringLabelValue(
+ labels: LabelNameToInfoMap,
+ labelName: string,
+ numberValue?: number
+): string {
+ const detailedInfo = labels[labelName] as DetailedLabelInfo;
+ if (detailedInfo.values) {
+ for (const labelValue of Object.keys(detailedInfo.values)) {
+ if (Number(labelValue) === numberValue) {
+ return labelValue;
+ }
+ }
+ }
+ // TODO: This code is sometimes executed with numberValue taking the
+ // values 0 and undefined.
+ // For now it is unclear how this is happening, ideally this code should
+ // never be executed.
+ return `${numberValue}`;
+}
+
+export function getVoteForAccount(
+ labelName: string,
+ account?: AccountInfo,
+ change?: ChangeInfo
+): string | null {
+ const labels = change?.labels;
+ if (!account || !labels) return null;
+ const votes = labels[labelName] as DetailedLabelInfo;
+ if (!votes.all?.length) return null;
+ for (let i = 0; i < votes.all.length; i++) {
+ if (votes.all[i]._account_id === account._account_id) {
+ return getStringLabelValue(labels, labelName, votes.all[i].value);
+ }
+ }
+ return null;
+}
+
+export function computeLabels(
+ account?: AccountInfo,
+ change?: ChangeInfo
+): Label[] {
+ if (!account) return [];
+ const labelsObj = change?.labels;
+ if (!labelsObj) return [];
+ return Object.keys(labelsObj)
+ .sort(labelCompare)
+ .map(key => {
+ return {
+ name: key,
+ value: getVoteForAccount(key, account, change),
+ };
+ });
+}
+
export function getTriggerVotes(change?: ParsedChangeInfo | ChangeInfo) {
const allLabels = Object.keys(change?.labels ?? {});
// Normally there is utility method getRequirements, which filter out
diff --git a/polygerrit-ui/app/utils/label-util_test.ts b/polygerrit-ui/app/utils/label-util_test.ts
index f4e1da9..56fb58c 100644
--- a/polygerrit-ui/app/utils/label-util_test.ts
+++ b/polygerrit-ui/app/utils/label-util_test.ts
@@ -29,6 +29,7 @@
hasNeutralStatus,
labelCompare,
LabelStatus,
+ computeLabels,
} from './label-util';
import {
AccountId,
@@ -45,10 +46,12 @@
createSubmitRequirementResultInfo,
createNonApplicableSubmitRequirementResultInfo,
createDetailedLabelInfo,
+ createAccountWithId,
} from '../test/test-data-generators';
import {
SubmitRequirementResultInfo,
SubmitRequirementStatus,
+ LabelNameToInfoMap,
} from '../api/rest-api';
const VALUES_0 = {
@@ -237,6 +240,58 @@
assert.equal(getRepresentativeValue(labelInfo), -2);
});
+ test('computeLabels', async () => {
+ const accountId = 123 as AccountId;
+ const account = createAccountWithId(accountId);
+ const change = {
+ ...createChange(),
+ labels: {
+ 'Code-Review': {
+ values: {
+ '0': 'No score',
+ '+1': 'good',
+ '+2': 'excellent',
+ '-1': 'bad',
+ '-2': 'terrible',
+ },
+ default_value: 0,
+ } as DetailedLabelInfo,
+ Verified: {
+ values: {
+ '0': 'No score',
+ '+1': 'good',
+ '+2': 'excellent',
+ '-1': 'bad',
+ '-2': 'terrible',
+ },
+ default_value: 0,
+ } as DetailedLabelInfo,
+ } as LabelNameToInfoMap,
+ };
+ let labels = computeLabels(account, change);
+ assert.deepEqual(labels, [
+ {name: 'Code-Review', value: null},
+ {name: 'Verified', value: null},
+ ]);
+ change.labels = {
+ ...change.labels,
+ Verified: {
+ ...change.labels.Verified,
+ all: [
+ {
+ _account_id: accountId,
+ value: 1,
+ },
+ ],
+ } as DetailedLabelInfo,
+ } as LabelNameToInfoMap;
+ labels = computeLabels(account, change);
+ assert.deepEqual(labels, [
+ {name: 'Code-Review', value: null},
+ {name: 'Verified', value: '+1'},
+ ]);
+ });
+
suite('extractAssociatedLabels()', () => {
function createSubmitRequirementExpressionInfoWith(expression: string) {
return {
