Merge "Cleanup of gr-hovercard"
diff --git a/java/com/google/gerrit/entities/SubmitRequirementResult.java b/java/com/google/gerrit/entities/SubmitRequirementResult.java
index 4f2f2f6..b7fa398 100644
--- a/java/com/google/gerrit/entities/SubmitRequirementResult.java
+++ b/java/com/google/gerrit/entities/SubmitRequirementResult.java
@@ -64,6 +64,13 @@
}
}
+ /** Returns true if the submit requirement is fulfilled and can allow change submission. */
+ @Memoized
+ public boolean fulfilled() {
+ Status s = status();
+ return s == Status.SATISFIED || s == Status.OVERRIDDEN || s == Status.NOT_APPLICABLE;
+ }
+
public static Builder builder() {
return new AutoValue_SubmitRequirementResult.Builder();
}
diff --git a/java/com/google/gerrit/server/approval/ApprovalInference.java b/java/com/google/gerrit/server/approval/ApprovalInference.java
index 4cb080a..695997a 100644
--- a/java/com/google/gerrit/server/approval/ApprovalInference.java
+++ b/java/com/google/gerrit/server/approval/ApprovalInference.java
@@ -54,6 +54,7 @@
import java.util.Map;
import java.util.Optional;
import org.eclipse.jgit.lib.Config;
+import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.revwalk.RevWalk;
/**
@@ -134,8 +135,9 @@
PatchSet.Id psId,
ChangeKind kind,
LabelType type,
- @Nullable Map<String, FileDiffOutput> modifiedFiles,
- @Nullable Map<String, FileDiffOutput> modifiedFilesLastPatchset) {
+ @Nullable Map<String, FileDiffOutput> baseVsCurrentDiff,
+ @Nullable Map<String, FileDiffOutput> baseVsPriorDiff,
+ @Nullable Map<String, FileDiffOutput> priorVsCurrentDiff) {
int n = psa.key().patchSetId().get();
checkArgument(n != psId.get());
@@ -185,7 +187,8 @@
project.getName());
return true;
} else if (type.isCopyAllScoresIfListOfFilesDidNotChange()
- && listOfFilesUnchangedPredicate.match(modifiedFiles, modifiedFilesLastPatchset)) {
+ && listOfFilesUnchangedPredicate.match(
+ baseVsCurrentDiff, baseVsPriorDiff, priorVsCurrentDiff)) {
logger.atFine().log(
"approval %d on label %s of patch set %d of change %d can be copied"
+ " to patch set %d because the label has set "
@@ -402,8 +405,9 @@
priorPatchSet.getValue().id().changeId(),
changeKind);
- Map<String, FileDiffOutput> modifiedFiles = null;
- Map<String, FileDiffOutput> modifiedFilesLastPatchSet = null;
+ Map<String, FileDiffOutput> baseVsCurrent = null;
+ Map<String, FileDiffOutput> baseVsPrior = null;
+ Map<String, FileDiffOutput> priorVsCurrent = null;
LabelTypes labelTypes = project.getLabelTypes();
for (PatchSetApproval psa : priorApprovals) {
if (resultByUser.contains(psa.label(), psa.accountId())) {
@@ -411,11 +415,13 @@
}
Optional<LabelType> type = labelTypes.byLabel(psa.labelId());
// Only compute modified files if there is a relevant label, since this is expensive.
- if (modifiedFiles == null
+ if (baseVsCurrent == null
&& type.isPresent()
&& type.get().isCopyAllScoresIfListOfFilesDidNotChange()) {
- modifiedFiles = listModifiedFiles(project, patchSet);
- modifiedFilesLastPatchSet = listModifiedFiles(project, priorPatchSet.getValue());
+ baseVsCurrent = listModifiedFiles(project, patchSet);
+ baseVsPrior = listModifiedFiles(project, priorPatchSet.getValue());
+ priorVsCurrent =
+ listModifiedFiles(project, priorPatchSet.getValue().commitId(), patchSet.commitId());
}
if (!type.isPresent()) {
logger.atFine().log(
@@ -435,8 +441,9 @@
patchSet.id(),
changeKind,
type.get(),
- modifiedFiles,
- modifiedFilesLastPatchSet)
+ baseVsCurrent,
+ baseVsPrior,
+ priorVsCurrent)
&& !canCopyBasedOnCopyCondition(notes, psa, patchSet, type.get(), changeKind)) {
continue;
}
@@ -465,4 +472,21 @@
ex);
}
}
+
+ /**
+ * Gets the modified files between two commits corresponding to different patchsets of the same
+ * change.
+ */
+ private Map<String, FileDiffOutput> listModifiedFiles(
+ ProjectState project, ObjectId sourceCommit, ObjectId targetCommit) {
+ try {
+ return diffOperations.listModifiedFiles(project.getNameKey(), sourceCommit, targetCommit);
+ } catch (DiffNotAvailableException ex) {
+ throw new StorageException(
+ "failed to compute difference in files, so won't copy"
+ + " votes on labels even if list of files is the same and "
+ + "copyAllIfListOfFilesDidNotChange",
+ ex);
+ }
+ }
}
diff --git a/java/com/google/gerrit/server/notedb/CommitRewriter.java b/java/com/google/gerrit/server/notedb/CommitRewriter.java
index 3cbe546..e940b1e 100644
--- a/java/com/google/gerrit/server/notedb/CommitRewriter.java
+++ b/java/com/google/gerrit/server/notedb/CommitRewriter.java
@@ -543,12 +543,16 @@
Matcher assigneeDeletedMatcher = ASSIGNEE_DELETED_PATTERN.matcher(originalChangeMessage);
if (assigneeDeletedMatcher.matches()) {
if (!NON_REPLACE_ACCOUNT_PATTERN.matcher(assigneeDeletedMatcher.group(1)).matches()) {
+ Optional<String> assigneeReplacement =
+ getPossibleAccountReplacement(
+ changeFixProgress,
+ oldAssignee,
+ getAccountInfoFromNameEmail(assigneeDeletedMatcher.group(1)));
+
return Optional.of(
- "Assignee deleted: "
- + getPossibleAccountReplacement(
- changeFixProgress,
- oldAssignee,
- ParsedAccountInfo.create(assigneeDeletedMatcher.group(1))));
+ assigneeReplacement.isPresent()
+ ? "Assignee deleted: " + assigneeReplacement.get()
+ : "Assignee was deleted.");
}
return Optional.empty();
}
@@ -556,12 +560,15 @@
Matcher assigneeAddedMatcher = ASSIGNEE_ADDED_PATTERN.matcher(originalChangeMessage);
if (assigneeAddedMatcher.matches()) {
if (!NON_REPLACE_ACCOUNT_PATTERN.matcher(assigneeAddedMatcher.group(1)).matches()) {
+ Optional<String> assigneeReplacement =
+ getPossibleAccountReplacement(
+ changeFixProgress,
+ newAssignee,
+ getAccountInfoFromNameEmail(assigneeAddedMatcher.group(1)));
return Optional.of(
- "Assignee added: "
- + getPossibleAccountReplacement(
- changeFixProgress,
- newAssignee,
- ParsedAccountInfo.create(assigneeAddedMatcher.group(1))));
+ assigneeReplacement.isPresent()
+ ? "Assignee added: " + assigneeReplacement.get()
+ : "Assignee was added.");
}
return Optional.empty();
}
@@ -569,17 +576,22 @@
Matcher assigneeChangedMatcher = ASSIGNEE_CHANGED_PATTERN.matcher(originalChangeMessage);
if (assigneeChangedMatcher.matches()) {
if (!NON_REPLACE_ACCOUNT_PATTERN.matcher(assigneeChangedMatcher.group(1)).matches()) {
+ Optional<String> oldAssigneeReplacement =
+ getPossibleAccountReplacement(
+ changeFixProgress,
+ oldAssignee,
+ getAccountInfoFromNameEmail(assigneeChangedMatcher.group(1)));
+ Optional<String> newAssigneeReplacement =
+ getPossibleAccountReplacement(
+ changeFixProgress,
+ newAssignee,
+ getAccountInfoFromNameEmail(assigneeChangedMatcher.group(2)));
return Optional.of(
- String.format(
- "Assignee changed from: %s to: %s",
- getPossibleAccountReplacement(
- changeFixProgress,
- oldAssignee,
- ParsedAccountInfo.create(assigneeChangedMatcher.group(1))),
- getPossibleAccountReplacement(
- changeFixProgress,
- newAssignee,
- ParsedAccountInfo.create(assigneeChangedMatcher.group(2)))));
+ oldAssigneeReplacement.isPresent() && newAssigneeReplacement.isPresent()
+ ? String.format(
+ "Assignee changed from: %s to: %s",
+ oldAssigneeReplacement.get(), newAssigneeReplacement.get())
+ : "Assignee was changed.");
}
return Optional.empty();
}
@@ -610,12 +622,15 @@
Matcher matcher = REMOVED_VOTE_PATTERN.matcher(originalChangeMessage);
if (matcher.matches() && !NON_REPLACE_ACCOUNT_PATTERN.matcher(matcher.group(2)).matches()) {
- return Optional.of(
- String.format(
- "Removed %s by %s",
- matcher.group(1),
- getPossibleAccountReplacement(
- changeFixProgress, reviewer, getAccountInfoFromNameEmail(matcher.group(2)))));
+ Optional<String> reviewerReplacement =
+ getPossibleAccountReplacement(
+ changeFixProgress, reviewer, getAccountInfoFromNameEmail(matcher.group(2)));
+ StringBuilder replacement = new StringBuilder();
+ replacement.append("Removed ").append(matcher.group(1));
+ if (reviewerReplacement.isPresent()) {
+ replacement.append(" by ").append(reviewerReplacement.get());
+ }
+ return Optional.of(replacement.toString());
}
return Optional.empty();
}
@@ -637,14 +652,14 @@
String replacementLine = lines[i];
if (matcher.matches() && !NON_REPLACE_ACCOUNT_PATTERN.matcher(matcher.group(2)).matches()) {
anyFixed = true;
- replacementLine =
- String.format(
- "* %s by %s\n",
- matcher.group(1),
- getPossibleAccountReplacement(
- changeFixProgress,
- Optional.empty(),
- getAccountInfoFromNameEmail(matcher.group(2))));
+ Optional<String> reviewerReplacement =
+ getPossibleAccountReplacement(
+ changeFixProgress, Optional.empty(), getAccountInfoFromNameEmail(matcher.group(2)));
+ replacementLine = "* " + matcher.group(1);
+ if (reviewerReplacement.isPresent()) {
+ replacementLine += " by " + reviewerReplacement.get();
+ }
+ replacementLine += "\n";
}
fixedLines.append(replacementLine);
}
@@ -708,11 +723,14 @@
StringBuffer sb = new StringBuffer();
while (onAddReviewerMatcher.find()) {
String reviewerName = normalizeOnCodeOwnerAddReviewerMatch(onAddReviewerMatcher.group(1));
- String replacementName =
+ Optional<String> replacementName =
getPossibleAccountReplacement(
changeFixProgress, Optional.empty(), ParsedAccountInfo.create(reviewerName));
onAddReviewerMatcher.appendReplacement(
- sb, replacementName + ", who was added as reviewer owns the following files");
+ sb,
+ replacementName.isPresent()
+ ? replacementName.get() + ", who was added as reviewer owns the following files"
+ : "Added reviewer owns the following files");
}
onAddReviewerMatcher.appendTail(sb);
sb.append("\n");
@@ -1071,19 +1089,20 @@
* <p>If {@code account} is known, replace with {@link AccountTemplateUtil#getAccountTemplate}.
* Otherwise, try to guess the correct replacement account for {@code accountName} among {@link
* ChangeFixProgress#parsedAccounts} that appeared in the change. If this fails {@link
- * #DEFAULT_ACCOUNT_REPLACEMENT} is applied.
+ * Optional#empty} is returned.
*
* @param changeFixProgress see {@link ChangeFixProgress}
* @param account account that should be used for replacement, if known
* @param accountInfo {@link ParsedAccountInfo} to replace.
- * @return replacement for {@code accountName}
+ * @return replacement for {@code accountName} or {@link Optional#empty}, if the replacement could
+ * not be determined.
*/
- private String getPossibleAccountReplacement(
+ private Optional<String> getPossibleAccountReplacement(
ChangeFixProgress changeFixProgress,
Optional<Account.Id> account,
ParsedAccountInfo accountInfo) {
if (account.isPresent()) {
- return AccountTemplateUtil.getAccountTemplate(account.get());
+ return Optional.of(AccountTemplateUtil.getAccountTemplate(account.get()));
}
// Retrieve reviewer accounts from cache and try to match by their name.
Map<Account.Id, AccountState> missingAccountStateReviewers =
@@ -1129,7 +1148,7 @@
e.getValue().get().account().getName(), accountInfo.name()))
.collect(ImmutableMap.toImmutableMap(Map.Entry::getKey, e -> e.getValue().get()));
}
- String replacementName = DEFAULT_ACCOUNT_REPLACEMENT;
+ Optional<String> replacementName = Optional.empty();
if (possibleReplacements.isEmpty()) {
logger.atWarning().log(
"Fixing ref %s, could not find reviewer account matching name %s",
@@ -1140,8 +1159,9 @@
changeFixProgress.changeMetaRef, accountInfo);
} else {
replacementName =
- AccountTemplateUtil.getAccountTemplate(
- Iterables.getOnlyElement(possibleReplacements.keySet()));
+ Optional.of(
+ AccountTemplateUtil.getAccountTemplate(
+ Iterables.getOnlyElement(possibleReplacements.keySet())));
}
return replacementName;
}
diff --git a/java/com/google/gerrit/server/project/SubmitRequirementsEvaluatorImpl.java b/java/com/google/gerrit/server/project/SubmitRequirementsEvaluatorImpl.java
index 151ee7b..de637b4 100644
--- a/java/com/google/gerrit/server/project/SubmitRequirementsEvaluatorImpl.java
+++ b/java/com/google/gerrit/server/project/SubmitRequirementsEvaluatorImpl.java
@@ -77,12 +77,17 @@
@Override
public Map<SubmitRequirement, SubmitRequirementResult> evaluateAllRequirements(
ChangeData cd, boolean includeLegacy) {
- Map<SubmitRequirement, SubmitRequirementResult> result = getRequirements(cd);
+ Map<SubmitRequirement, SubmitRequirementResult> projectConfigRequirements = getRequirements(cd);
+ Map<SubmitRequirement, SubmitRequirementResult> result = projectConfigRequirements;
if (includeLegacy
&& experimentFeatures.isFeatureEnabled(
ExperimentFeaturesConstants
.GERRIT_BACKEND_REQUEST_FEATURE_ENABLE_LEGACY_SUBMIT_REQUIREMENTS)) {
- result.putAll(SubmitRequirementsAdapter.getLegacyRequirements(legacyEvaluator, cd));
+ Map<SubmitRequirement, SubmitRequirementResult> legacyReqs =
+ SubmitRequirementsAdapter.getLegacyRequirements(legacyEvaluator, cd);
+ result =
+ SubmitRequirementsUtil.mergeLegacyAndNonLegacyRequirements(
+ projectConfigRequirements, legacyReqs);
}
return ImmutableMap.copyOf(result);
}
diff --git a/java/com/google/gerrit/server/project/SubmitRequirementsUtil.java b/java/com/google/gerrit/server/project/SubmitRequirementsUtil.java
new file mode 100644
index 0000000..2e43eac
--- /dev/null
+++ b/java/com/google/gerrit/server/project/SubmitRequirementsUtil.java
@@ -0,0 +1,71 @@
+// Copyright (C) 2021 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.project;
+
+import com.google.gerrit.entities.SubmitRequirement;
+import com.google.gerrit.entities.SubmitRequirementResult;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.stream.Collectors;
+
+/**
+ * A utility class for different operations related to {@link
+ * com.google.gerrit.entities.SubmitRequirement}s.
+ */
+public class SubmitRequirementsUtil {
+
+ private SubmitRequirementsUtil() {}
+
+ /**
+ * Merge legacy and non-legacy submit requirement results. If both input maps have submit
+ * requirements with the same name and fulfillment status (according to {@link
+ * SubmitRequirementResult#fulfilled()}), we eliminate the entry from the {@code
+ * legacyRequirements} input map and only include the one from the {@code
+ * projectConfigRequirements} in the result.
+ *
+ * @param projectConfigRequirements map of {@link SubmitRequirement} to {@link
+ * SubmitRequirementResult} containing results for submit requirements stored in the
+ * project.config.
+ * @param legacyRequirements map of {@link SubmitRequirement} to {@link SubmitRequirementResult}
+ * containing the results of converting legacy submit records to submit requirements.
+ * @return a map that is the result of merging both input maps, while eliminating requirements
+ * with the same name and status.
+ */
+ public static Map<SubmitRequirement, SubmitRequirementResult> mergeLegacyAndNonLegacyRequirements(
+ Map<SubmitRequirement, SubmitRequirementResult> projectConfigRequirements,
+ Map<SubmitRequirement, SubmitRequirementResult> legacyRequirements) {
+ Map<SubmitRequirement, SubmitRequirementResult> result = new HashMap<>();
+ result.putAll(projectConfigRequirements);
+ Map<String, SubmitRequirementResult> requirementsByName =
+ projectConfigRequirements.entrySet().stream()
+ .collect(Collectors.toMap(sr -> sr.getKey().name(), sr -> sr.getValue()));
+ for (Map.Entry<SubmitRequirement, SubmitRequirementResult> legacy :
+ legacyRequirements.entrySet()) {
+ String name = legacy.getKey().name();
+ SubmitRequirementResult projectConfigResult = requirementsByName.get(name);
+ SubmitRequirementResult legacyResult = legacy.getValue();
+ if (projectConfigResult != null && matchByStatus(projectConfigResult, legacyResult)) {
+ continue;
+ }
+ result.put(legacy.getKey(), legacy.getValue());
+ }
+ return result;
+ }
+
+ /** Returns true if both input results are equal in allowing/disallowing change submission. */
+ private static boolean matchByStatus(SubmitRequirementResult r1, SubmitRequirementResult r2) {
+ return r1.fulfilled() == r2.fulfilled();
+ }
+}
diff --git a/java/com/google/gerrit/server/query/approval/ListOfFilesUnchangedPredicate.java b/java/com/google/gerrit/server/query/approval/ListOfFilesUnchangedPredicate.java
index 55c27be..de7dd0a 100644
--- a/java/com/google/gerrit/server/query/approval/ListOfFilesUnchangedPredicate.java
+++ b/java/com/google/gerrit/server/query/approval/ListOfFilesUnchangedPredicate.java
@@ -58,13 +58,18 @@
Integer parentNum =
isInitialCommit(ctx.changeNotes().getProjectName(), targetPatchSet.commitId()) ? 0 : 1;
try {
- Map<String, FileDiffOutput> modifiedTargetPatchSet =
+ Map<String, FileDiffOutput> baseVsCurrent =
diffOperations.listModifiedFilesAgainstParent(
ctx.changeNotes().getProjectName(), targetPatchSet.commitId(), parentNum);
- Map<String, FileDiffOutput> modifiedSourcePatchSet =
+ Map<String, FileDiffOutput> baseVsPrior =
diffOperations.listModifiedFilesAgainstParent(
ctx.changeNotes().getProjectName(), sourcePatchSet.commitId(), parentNum);
- return match(modifiedTargetPatchSet, modifiedSourcePatchSet);
+ Map<String, FileDiffOutput> priorVsCurrent =
+ diffOperations.listModifiedFiles(
+ ctx.changeNotes().getProjectName(),
+ sourcePatchSet.commitId(),
+ targetPatchSet.commitId());
+ return match(baseVsCurrent, baseVsPrior, priorVsCurrent);
} catch (DiffNotAvailableException ex) {
throw new StorageException(
"failed to compute difference in files, so won't copy"
@@ -79,16 +84,23 @@
* {@link ChangeType} matches for each modified file.
*/
public boolean match(
- Map<String, FileDiffOutput> modifiedFiles1, Map<String, FileDiffOutput> modifiedFiles2) {
+ Map<String, FileDiffOutput> baseVsCurrent,
+ Map<String, FileDiffOutput> baseVsPrior,
+ Map<String, FileDiffOutput> priorVsCurrent) {
Set<String> allFiles = new HashSet<>();
- allFiles.addAll(modifiedFiles1.keySet());
- allFiles.addAll(modifiedFiles2.keySet());
+ allFiles.addAll(baseVsCurrent.keySet());
+ allFiles.addAll(baseVsPrior.keySet());
for (String file : allFiles) {
if (Patch.isMagic(file)) {
continue;
}
- FileDiffOutput fileDiffOutput1 = modifiedFiles1.get(file);
- FileDiffOutput fileDiffOutput2 = modifiedFiles2.get(file);
+ FileDiffOutput fileDiffOutput1 = baseVsCurrent.get(file);
+ FileDiffOutput fileDiffOutput2 = baseVsPrior.get(file);
+ if (!priorVsCurrent.containsKey(file)) {
+ // If the file is not modified between prior and current patchsets, then scan safely skip
+ // it. The file might has been modified due to rebase.
+ continue;
+ }
if (fileDiffOutput1 == null || fileDiffOutput2 == null) {
return false;
}
diff --git a/java/com/google/gerrit/server/query/change/ChangeData.java b/java/com/google/gerrit/server/query/change/ChangeData.java
index 4112579..b8c8c07 100644
--- a/java/com/google/gerrit/server/query/change/ChangeData.java
+++ b/java/com/google/gerrit/server/query/change/ChangeData.java
@@ -91,6 +91,7 @@
import com.google.gerrit.server.project.ProjectState;
import com.google.gerrit.server.project.SubmitRequirementsAdapter;
import com.google.gerrit.server.project.SubmitRequirementsEvaluator;
+import com.google.gerrit.server.project.SubmitRequirementsUtil;
import com.google.gerrit.server.project.SubmitRuleEvaluator;
import com.google.gerrit.server.project.SubmitRuleOptions;
import com.google.gerrit.server.util.time.TimeUtil;
@@ -969,16 +970,11 @@
submitRequirements = projectConfigRequirements;
return submitRequirements;
}
- // Get legacy submit requirements, i.e. those created from submit records.
Map<SubmitRequirement, SubmitRequirementResult> legacyRequirements =
SubmitRequirementsAdapter.getLegacyRequirements(submitRuleEvaluatorFactory, this);
- // Combine projectConfigRequirements with legacyRequirements
submitRequirements =
- Stream.of(projectConfigRequirements, legacyRequirements)
- .flatMap(map -> map.entrySet().stream())
- .collect(
- ImmutableMap.toImmutableMap(
- Map.Entry::getKey, Map.Entry::getValue, (value1, value2) -> value1));
+ SubmitRequirementsUtil.mergeLegacyAndNonLegacyRequirements(
+ projectConfigRequirements, legacyRequirements);
}
return submitRequirements;
}
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
index ed9f2f3..52202d7 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -4539,6 +4539,129 @@
value =
ExperimentFeaturesConstants
.GERRIT_BACKEND_REQUEST_FEATURE_ENABLE_LEGACY_SUBMIT_REQUIREMENTS)
+ public void
+ submitRequirements_returnOneEntryForMatchingLegacyAndNonLegacyResultsWithTheSameName_ifLegacySubmitRecordsAreEnabled()
+ throws Exception {
+ // Configure a legacy submit requirement: label with a max with block function
+ configLabel("build-cop-override", LabelFunction.MAX_WITH_BLOCK);
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(
+ allowLabel("build-cop-override")
+ .ref("refs/heads/master")
+ .group(REGISTERED_USERS)
+ .range(-1, 1))
+ .update();
+
+ // Configure a submit requirement with the same name.
+ configSubmitRequirement(
+ project,
+ SubmitRequirement.builder()
+ .setName("build-cop-override")
+ .setSubmittabilityExpression(
+ SubmitRequirementExpression.create(
+ "label:build-cop-override=MAX -label:build-cop-override=MIN"))
+ .setAllowOverrideInChildProjects(false)
+ .build());
+
+ // Create a change. Vote to fulfill all requirements.
+ PushOneCommit.Result r = createChange();
+ String changeId = r.getChangeId();
+ voteLabel(changeId, "build-cop-override", 1);
+ voteLabel(changeId, "Code-Review", 2);
+
+ // Project has two legacy requirements: Code-Review and bco, and a non-legacy requirement: bco.
+ // Only non-legacy bco is returned.
+ ChangeInfo change = gApi.changes().id(changeId).get();
+ assertThat(change.submitRequirements).hasSize(2);
+ assertSubmitRequirementStatus(
+ change.submitRequirements, "Code-Review", Status.SATISFIED, /* isLegacy= */ true);
+ assertSubmitRequirementStatus(
+ change.submitRequirements,
+ "build-cop-override",
+ Status.SATISFIED,
+ /* isLegacy= */ false,
+ /* submittabilityCondition= */ "label:build-cop-override=MAX -label:build-cop-override=MIN");
+
+ // Merge the change. Submit requirements are still the same.
+ gApi.changes().id(changeId).current().submit();
+ change = gApi.changes().id(changeId).get();
+ assertThat(change.submitRequirements).hasSize(2);
+ assertSubmitRequirementStatus(
+ change.submitRequirements, "Code-Review", Status.SATISFIED, /* isLegacy= */ true);
+ assertSubmitRequirementStatus(
+ change.submitRequirements,
+ "build-cop-override",
+ Status.SATISFIED,
+ /* isLegacy= */ false,
+ /* submittabilityCondition= */ "label:build-cop-override=MAX -label:build-cop-override=MIN");
+ }
+
+ @Test
+ @GerritConfig(
+ name = "experiments.enabled",
+ value =
+ ExperimentFeaturesConstants
+ .GERRIT_BACKEND_REQUEST_FEATURE_ENABLE_LEGACY_SUBMIT_REQUIREMENTS)
+ public void
+ submitRequirements_returnTwoEntriesForMismatchingLegacyAndNonLegacyResultsWithTheSameName_ifLegacySubmitRecordsAreEnabled()
+ throws Exception {
+ // Configure a legacy submit requirement: label with a max with block function
+ configLabel("build-cop-override", LabelFunction.MAX_WITH_BLOCK);
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(
+ allowLabel("build-cop-override")
+ .ref("refs/heads/master")
+ .group(REGISTERED_USERS)
+ .range(-1, 1))
+ .update();
+
+ // Configure a submit requirement with the same name.
+ configSubmitRequirement(
+ project,
+ SubmitRequirement.builder()
+ .setName("build-cop-override")
+ .setSubmittabilityExpression(
+ SubmitRequirementExpression.create("label:build-cop-override=MIN"))
+ .setAllowOverrideInChildProjects(false)
+ .build());
+
+ // Create a change
+ PushOneCommit.Result r = createChange();
+ String changeId = r.getChangeId();
+ voteLabel(changeId, "build-cop-override", 1);
+ voteLabel(changeId, "Code-Review", 2);
+
+ // Project has two legacy requirements: Code-Review and bco, and a non-legacy requirement: bco.
+ // Two instances of bco will be returned since their status is not matching.
+ ChangeInfo change = gApi.changes().id(changeId).get();
+ assertThat(change.submitRequirements).hasSize(3);
+ assertSubmitRequirementStatus(
+ change.submitRequirements, "Code-Review", Status.SATISFIED, /* isLegacy= */ true);
+ assertSubmitRequirementStatus(
+ change.submitRequirements,
+ "build-cop-override",
+ Status.SATISFIED,
+ /* isLegacy= */ true,
+ // MAX_WITH_BLOCK function was translated to a submittability expression.
+ /* submittabilityCondition= */ "label:build-cop-override=MAX -label:build-cop-override=MIN");
+ assertSubmitRequirementStatus(
+ change.submitRequirements,
+ "build-cop-override",
+ Status.UNSATISFIED,
+ /* isLegacy= */ false,
+ /* submittabilityCondition= */ "label:build-cop-override=MIN");
+ }
+
+ @Test
+ @GerritConfig(
+ name = "experiments.enabled",
+ value =
+ ExperimentFeaturesConstants
+ .GERRIT_BACKEND_REQUEST_FEATURE_ENABLE_LEGACY_SUBMIT_REQUIREMENTS)
public void submitRequirements_returnForLegacySubmitRecords_ifEnabled() throws Exception {
configLabel("build-cop-override", LabelFunction.MAX_WITH_BLOCK);
projectOperations
@@ -5197,6 +5320,30 @@
Collection<SubmitRequirementResultInfo> results,
String requirementName,
SubmitRequirementResultInfo.Status status,
+ boolean isLegacy,
+ String submittabilityCondition) {
+ for (SubmitRequirementResultInfo result : results) {
+ if (result.name.equals(requirementName)
+ && result.status == status
+ && result.isLegacy == isLegacy
+ && result.submittabilityExpressionResult.expression.equals(submittabilityCondition)) {
+ return;
+ }
+ }
+ throw new AssertionError(
+ String.format(
+ "Could not find submit requirement %s with status %s (results = %s)",
+ requirementName,
+ status,
+ results.stream()
+ .map(r -> String.format("%s=%s", r.name, r.status))
+ .collect(toImmutableList())));
+ }
+
+ private void assertSubmitRequirementStatus(
+ Collection<SubmitRequirementResultInfo> results,
+ String requirementName,
+ SubmitRequirementResultInfo.Status status,
boolean isLegacy) {
for (SubmitRequirementResultInfo result : results) {
if (result.name.equals(requirementName)
diff --git a/javatests/com/google/gerrit/acceptance/api/change/StickyApprovalsIT.java b/javatests/com/google/gerrit/acceptance/api/change/StickyApprovalsIT.java
index cd9e876..3888679 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/StickyApprovalsIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/StickyApprovalsIT.java
@@ -43,6 +43,7 @@
import com.google.gerrit.acceptance.testsuite.change.ChangeOperations;
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
+import com.google.gerrit.common.RawInputUtil;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.LabelId;
import com.google.gerrit.entities.LabelType;
@@ -557,6 +558,46 @@
}
@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)) {
@@ -1072,17 +1113,9 @@
.sorted(comparing(a -> a.patchSetId().get()))
.collect(toImmutableList());
PatchSetApproval nonCopied = patchSetApprovals.get(0);
-
- assertThat(nonCopied.patchSetId().get()).isEqualTo(1);
- 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.label()).isEqualTo(LabelId.CODE_REVIEW);
- assertThat(copied.value()).isEqualTo((short) 1);
- assertThat(copied.copied()).isTrue();
+ assertCopied(nonCopied, 1, LabelId.CODE_REVIEW, (short) 1, /* copied= */ false);
+ assertCopied(copied, 2, LabelId.CODE_REVIEW, (short) 1, /* copied= */ true);
}
@Test
@@ -1311,4 +1344,12 @@
}
assertWithMessage(name).that(vote).isEqualTo(expectedVote);
}
+
+ private void assertCopied(
+ PatchSetApproval approval, int psId, String label, short value, boolean copied) {
+ assertThat(approval.patchSetId().get()).isEqualTo(psId);
+ assertThat(approval.label()).isEqualTo(label);
+ assertThat(approval.value()).isEqualTo(value);
+ assertThat(approval.copied()).isEqualTo(copied);
+ }
}
diff --git a/javatests/com/google/gerrit/server/notedb/CommitRewriterTest.java b/javatests/com/google/gerrit/server/notedb/CommitRewriterTest.java
index 19c2bcf..7f16cc4 100644
--- a/javatests/com/google/gerrit/server/notedb/CommitRewriterTest.java
+++ b/javatests/com/google/gerrit/server/notedb/CommitRewriterTest.java
@@ -785,7 +785,7 @@
.containsExactly(
"@@ -6 +6 @@\n"
+ "-Removed Verified+2 by Other Account <other@account.com>\n"
- + "+Removed Verified+2 by Gerrit Account\n");
+ + "+Removed Verified+2\n");
BackfillResult secondRunResult = rewriter.backfillProject(project, repo, options);
assertThat(secondRunResult.fixedRefDiff.keySet()).isEmpty();
assertThat(secondRunResult.refsFailedToFix).isEmpty();
@@ -1671,10 +1671,9 @@
+ " * file1.java\n"
+ "\n<GERRIT_ACCOUNT_2>, who was added as reviewer owns the following files:\n"
+ " * file3.js\n"
- + "\nGerrit Account, who was added as reviewer owns the following files:\n"
+ + "\nAdded reviewer owns the following files:\n"
+ " * file4.java\n",
- "Gerrit Account, who was added as reviewer owns the following files:\n"
- + " * file6.java\n",
+ "Added reviewer owns the following files:\n" + " * file6.java\n",
"Gerrit Account who was added as reviewer owns the following files:\n"
+ " * file1.java\n"
+ "\n<GERRIT_ACCOUNT_1> who was added as reviewer owns the following files:\n"
@@ -1701,10 +1700,10 @@
+ "+<GERRIT_ACCOUNT_2>, who was added as reviewer owns the following files:\n"
+ "@@ -12 +12 @@\n"
+ "-Missing Reviewer who was added as reviewer owns the following files:\n"
- + "+Gerrit Account, who was added as reviewer owns the following files:\n",
+ + "+Added reviewer owns the following files:\n",
"@@ -6 +6 @@\n"
+ "-Reviewer User who was added as reviewer owns the following files:\n"
- + "+Gerrit Account, who was added as reviewer owns the following files:\n");
+ + "+Added reviewer owns the following files:\n");
BackfillResult secondRunResult = rewriter.backfillProject(project, repo, options);
assertThat(secondRunResult.fixedRefDiff.keySet()).isEmpty();
assertThat(secondRunResult.refsFailedToFix).isEmpty();
@@ -2051,7 +2050,8 @@
getChangeUpdateBody(
c,
String.format(
- "Assignee changed from: %s to: %s", changeOwner.getName(), otherUser.getName())),
+ "Assignee changed from: %s to: %s",
+ changeOwner.getNameEmail(), otherUser.getNameEmail())),
getAuthorIdent(otherUser.getAccount()));
writeUpdate(
RefNames.changeMetaRef(c.getId()),
@@ -2086,14 +2086,12 @@
+ "-Assignee added: Change Owner\n"
+ "+Assignee added: <GERRIT_ACCOUNT_1>\n",
"@@ -6 +6 @@\n"
- + "-Assignee changed from: Change Owner to: Other Account\n"
+ + "-Assignee changed from: Change Owner <change@owner.com> to: Other Account <other@account.com>\n"
+ "+Assignee changed from: <GERRIT_ACCOUNT_1> to: <GERRIT_ACCOUNT_2>\n",
"@@ -6 +6 @@\n"
+ "-Assignee deleted: Other Account\n"
+ "+Assignee deleted: <GERRIT_ACCOUNT_2>\n",
- "@@ -6 +6 @@\n"
- + "-Assignee added: Reviewer User\n"
- + "+Assignee added: Gerrit Account\n");
+ "@@ -6 +6 @@\n" + "-Assignee added: Reviewer User\n" + "+Assignee was added.\n");
BackfillResult secondRunResult = rewriter.backfillProject(project, repo, options);
assertThat(secondRunResult.fixedRefDiff.keySet()).isEmpty();
assertThat(secondRunResult.refsFailedToFix).isEmpty();
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
index fd7b5d1..fc2cbe5 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
@@ -1824,7 +1824,7 @@
changeIsOpen(change)
) {
fireAlert(this, 'Change edit not found. Please create a change edit.');
- GerritNav.navigateToChange(change);
+ fireReload(this, true);
return;
}
@@ -1837,7 +1837,7 @@
this,
'Change edits cannot be created if change is merged or abandoned. Redirected to non edit mode.'
);
- GerritNav.navigateToChange(change);
+ fireReload(this, true);
return;
}
diff --git a/polygerrit-ui/app/elements/change/gr-submit-requirements/gr-submit-requirements.ts b/polygerrit-ui/app/elements/change/gr-submit-requirements/gr-submit-requirements.ts
index 21e8093..11dc9d9 100644
--- a/polygerrit-ui/app/elements/change/gr-submit-requirements/gr-submit-requirements.ts
+++ b/polygerrit-ui/app/elements/change/gr-submit-requirements/gr-submit-requirements.ts
@@ -21,6 +21,7 @@
import {
AccountInfo,
isDetailedLabelInfo,
+ isQuickLabelInfo,
LabelInfo,
LabelNameToInfoMap,
SubmitRequirementResultInfo,
@@ -225,18 +226,23 @@
renderLabelVote(label: string, labels: LabelNameToInfoMap) {
const labelInfo = labels[label];
- if (!isDetailedLabelInfo(labelInfo)) return;
- const uniqueApprovals = getAllUniqueApprovals(labelInfo);
- return uniqueApprovals.map(
- approvalInfo =>
- html`<gr-vote-chip
- .vote="${approvalInfo}"
- .label="${labelInfo}"
- .more="${(labelInfo.all ?? []).filter(
- other => other.value === approvalInfo.value
- ).length > 1}"
- ></gr-vote-chip>`
- );
+ if (isDetailedLabelInfo(labelInfo)) {
+ const uniqueApprovals = getAllUniqueApprovals(labelInfo);
+ return uniqueApprovals.map(
+ approvalInfo =>
+ html`<gr-vote-chip
+ .vote="${approvalInfo}"
+ .label="${labelInfo}"
+ .more="${(labelInfo.all ?? []).filter(
+ other => other.value === approvalInfo.value
+ ).length > 1}"
+ ></gr-vote-chip>`
+ );
+ } else if (isQuickLabelInfo(labelInfo)) {
+ return [html`<gr-vote-chip .label="${labelInfo}"></gr-vote-chip>`];
+ } else {
+ return html``;
+ }
}
renderChecks(requirement: SubmitRequirementResultInfo) {
@@ -364,19 +370,30 @@
}
override render() {
- const uniqueApprovals = getAllUniqueApprovals(this.labelInfo);
+ if (!this.labelInfo) return;
return html`
<div class="container">
<span class="label">${this.label}</span>
- ${uniqueApprovals.map(
- approvalInfo => html`<gr-vote-chip
- .vote="${approvalInfo}"
- .label="${this.labelInfo}"
- ></gr-vote-chip>`
- )}
+ ${this.renderVotes()}
</div>
`;
}
+
+ private renderVotes() {
+ if (!this.labelInfo) return;
+ if (isDetailedLabelInfo(this.labelInfo)) {
+ return getAllUniqueApprovals(this.labelInfo).map(
+ approvalInfo => html`<gr-vote-chip
+ .vote="${approvalInfo}"
+ .label="${this.labelInfo}"
+ ></gr-vote-chip>`
+ );
+ } else if (isQuickLabelInfo(this.labelInfo)) {
+ return [html`<gr-vote-chip .label="${this.labelInfo}"></gr-vote-chip>`];
+ } else {
+ return html``;
+ }
+ }
}
declare global {
diff --git a/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls.ts b/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls.ts
index d87b573..1615a23 100644
--- a/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls.ts
+++ b/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls.ts
@@ -36,7 +36,7 @@
} from '../../shared/gr-autocomplete/gr-autocomplete';
import {appContext} from '../../../services/app-context';
import {IronInputElement} from '@polymer/iron-input';
-import {fireAlert} from '../../../utils/event-util';
+import {fireAlert, fireReload} from '../../../utils/event-util';
export interface GrEditControls {
$: {
@@ -237,7 +237,7 @@
return;
}
this._closeDialog(this.$.openDialog);
- GerritNav.navigateToChange(this.change);
+ fireReload(this, true);
});
}
@@ -257,7 +257,7 @@
return;
}
this._closeDialog(dialog);
- GerritNav.navigateToChange(this.change);
+ fireReload(this);
});
}
@@ -275,7 +275,7 @@
return;
}
this._closeDialog(dialog);
- GerritNav.navigateToChange(this.change);
+ fireReload(this);
});
}
@@ -293,7 +293,7 @@
return;
}
this._closeDialog(dialog);
- GerritNav.navigateToChange(this.change);
+ fireReload(this, true);
});
}
diff --git a/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls_test.ts b/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls_test.ts
index 0ba68e2..6198f17 100644
--- a/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls_test.ts
+++ b/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls_test.ts
@@ -122,12 +122,12 @@
});
suite('delete button CUJ', () => {
- let navStub: sinon.SinonStub;
+ let eventStub: sinon.SinonStub;
let deleteStub: sinon.SinonStub;
let deleteAutocomplete: GrAutocomplete;
setup(() => {
- navStub = sinon.stub(GerritNav, 'navigateToChange');
+ eventStub = sinon.stub(element, 'dispatchEvent');
deleteStub = stubRestApi('deleteFileInChangeEdit');
deleteAutocomplete =
element.$.deleteDialog!.querySelector('gr-autocomplete')!;
@@ -155,7 +155,7 @@
assert.isTrue(deleteStub.called);
await deleteStub.lastCall.returnValue;
assert.equal(element._path, '');
- assert.isTrue(navStub.called);
+ assert.equal(eventStub.firstCall.args[0].type, 'reload');
assert.isTrue(closeDialogSpy.called);
});
@@ -181,7 +181,7 @@
assert.isTrue(deleteStub.called);
await deleteStub.lastCall.returnValue;
- assert.isFalse(navStub.called);
+ assert.isFalse(eventStub.called);
assert.isFalse(closeDialogSpy.called);
});
@@ -195,7 +195,7 @@
MockInteractions.tap(
queryAndAssert(element.$.deleteDialog, 'gr-button')
);
- assert.isFalse(navStub.called);
+ assert.isFalse(eventStub.called);
assert.isTrue(closeDialogSpy.called);
assert.equal(element._path, '');
});
@@ -203,12 +203,12 @@
});
suite('rename button CUJ', () => {
- let navStub: sinon.SinonStub;
+ let eventStub: sinon.SinonStub;
let renameStub: sinon.SinonStub;
let renameAutocomplete: GrAutocomplete;
setup(() => {
- navStub = sinon.stub(GerritNav, 'navigateToChange');
+ eventStub = sinon.stub(element, 'dispatchEvent');
renameStub = stubRestApi('renameFileInChangeEdit');
renameAutocomplete =
element.$.renameDialog!.querySelector('gr-autocomplete')!;
@@ -241,7 +241,7 @@
await renameStub.lastCall.returnValue;
assert.equal(element._path, '');
- assert.isTrue(navStub.called);
+ assert.equal(eventStub.firstCall.args[0].type, 'reload');
assert.isTrue(closeDialogSpy.called);
});
@@ -272,7 +272,7 @@
assert.isTrue(renameStub.called);
await renameStub.lastCall.returnValue;
- assert.isFalse(navStub.called);
+ assert.isFalse(eventStub.called);
assert.isFalse(closeDialogSpy.called);
});
@@ -287,7 +287,7 @@
MockInteractions.tap(
queryAndAssert(element.$.renameDialog, 'gr-button')
);
- assert.isFalse(navStub.called);
+ assert.isFalse(eventStub.called);
assert.isTrue(closeDialogSpy.called);
assert.equal(element._path, '');
assert.equal(element._newPath, '');
@@ -296,11 +296,11 @@
});
suite('restore button CUJ', () => {
- let navStub: sinon.SinonStub;
+ let eventStub: sinon.SinonStub;
let restoreStub: sinon.SinonStub;
setup(() => {
- navStub = sinon.stub(GerritNav, 'navigateToChange');
+ eventStub = sinon.stub(element, 'dispatchEvent');
restoreStub = stubRestApi('restoreFileInChangeEdit');
});
@@ -324,7 +324,7 @@
assert.equal(restoreStub.lastCall.args[1], 'src/test.cpp');
return restoreStub.lastCall.returnValue.then(() => {
assert.equal(element._path, '');
- assert.isTrue(navStub.called);
+ assert.equal(eventStub.firstCall.args[0].type, 'reload');
assert.isTrue(closeDialogSpy.called);
});
});
@@ -343,7 +343,7 @@
assert.isTrue(restoreStub.called);
assert.equal(restoreStub.lastCall.args[1], 'src/test.cpp');
return restoreStub.lastCall.returnValue.then(() => {
- assert.isFalse(navStub.called);
+ assert.isFalse(eventStub.called);
assert.isFalse(closeDialogSpy.called);
});
});
@@ -356,7 +356,7 @@
MockInteractions.tap(
queryAndAssert(element.$.restoreDialog, 'gr-button')
);
- assert.isFalse(navStub.called);
+ assert.isFalse(eventStub.called);
assert.isTrue(closeDialogSpy.called);
assert.equal(element._path, '');
});
diff --git a/polygerrit-ui/app/elements/gr-app-element.ts b/polygerrit-ui/app/elements/gr-app-element.ts
index 3b93bea..38f55bd 100644
--- a/polygerrit-ui/app/elements/gr-app-element.ts
+++ b/polygerrit-ui/app/elements/gr-app-element.ts
@@ -242,7 +242,7 @@
this.addEventListener(EventType.DIALOG_CHANGE, e => {
this._handleDialogChange(e as CustomEvent<DialogChangeEventDetail>);
});
- this.addEventListener('location-change', e =>
+ this.addEventListener(EventType.LOCATION_CHANGE, e =>
this._handleLocationChange(e)
);
this.addEventListener(EventType.RECREATE_CHANGE_VIEW, () =>
@@ -251,7 +251,7 @@
this.addEventListener(EventType.RECREATE_DIFF_VIEW, () =>
this.handleRecreateView(GerritView.DIFF)
);
- document.addEventListener('gr-rpc-log', e => this._handleRpcLog(e));
+ document.addEventListener(EventType.GR_RPC_LOG, e => this._handleRpcLog(e));
}
override ready() {
diff --git a/polygerrit-ui/app/elements/shared/gr-label-info/gr-label-info.ts b/polygerrit-ui/app/elements/shared/gr-label-info/gr-label-info.ts
index 5a6d821..2df2ccb 100644
--- a/polygerrit-ui/app/elements/shared/gr-label-info/gr-label-info.ts
+++ b/polygerrit-ui/app/elements/shared/gr-label-info/gr-label-info.ts
@@ -24,7 +24,6 @@
import '../gr-label/gr-label';
import '../gr-tooltip-content/gr-tooltip-content';
import {dom, EventApi} from '@polymer/polymer/lib/legacy/polymer.dom';
-import {GerritNav} from '../../core/gr-navigation/gr-navigation';
import {
AccountInfo,
LabelInfo,
@@ -44,6 +43,7 @@
import {sharedStyles} from '../../../styles/shared-styles';
import {votingStyles} from '../../../styles/gr-voting-styles';
import {ifDefined} from 'lit/directives/if-defined';
+import {fireReload} from '../../../utils/event-util';
declare global {
interface HTMLElementTagNameMap {
@@ -349,7 +349,7 @@
return;
}
if (this.change) {
- GerritNav.navigateToChange(this.change);
+ fireReload(this);
}
})
.catch(err => {
diff --git a/polygerrit-ui/app/elements/shared/gr-vote-chip/gr-vote-chip.ts b/polygerrit-ui/app/elements/shared/gr-vote-chip/gr-vote-chip.ts
index 3762d8d..9013088 100644
--- a/polygerrit-ui/app/elements/shared/gr-vote-chip/gr-vote-chip.ts
+++ b/polygerrit-ui/app/elements/shared/gr-vote-chip/gr-vote-chip.ts
@@ -16,7 +16,12 @@
*/
import {LitElement, css, html} from 'lit';
import {customElement, property} from 'lit/decorators';
-import {ApprovalInfo, LabelInfo} from '../../../api/rest-api';
+import {
+ ApprovalInfo,
+ isDetailedLabelInfo,
+ isQuickLabelInfo,
+ LabelInfo,
+} from '../../../api/rest-api';
import {appContext} from '../../../services/app-context';
import {KnownExperimentId} from '../../../services/flags/flags';
import {
@@ -109,22 +114,51 @@
override render() {
if (!this.flagsService.isEnabled(KnownExperimentId.SUBMIT_REQUIREMENTS_UI))
return;
- if (!this.vote?.value) return;
- const className = this.computeClass(this.vote.value, this.label);
+
+ const renderValue = this.renderValue();
+ if (!renderValue) return;
+
return html`<span class="container">
- <div class="vote-chip ${className} ${this.more ? 'more' : ''}">
- ${valueString(this.vote.value)}
+ <div class="vote-chip ${this.computeClass()} ${this.more ? 'more' : ''}">
+ ${renderValue}
</div>
${this.more
- ? html`<div class="chip-angle ${className}">
- ${valueString(this.vote.value)}
+ ? html`<div class="chip-angle ${this.computeClass()}">
+ ${renderValue}
</div>`
: ''}
</span>`;
}
- computeClass(vote: number, label?: LabelInfo) {
- const status = getLabelStatus(label, vote);
- return classForLabelStatus(status);
+ private renderValue() {
+ if (!this.label) {
+ return '';
+ } else if (isDetailedLabelInfo(this.label)) {
+ if (this.vote?.value) {
+ return valueString(this.vote.value);
+ }
+ } else if (isQuickLabelInfo(this.label)) {
+ if (this.label.approved) {
+ return '👍️';
+ } else if (this.label.rejected) {
+ return '👎️';
+ }
+ }
+ return '';
+ }
+
+ private computeClass() {
+ if (!this.label) {
+ return '';
+ } else if (isDetailedLabelInfo(this.label)) {
+ if (this.vote?.value) {
+ const status = getLabelStatus(this.label, this.vote.value);
+ return classForLabelStatus(status);
+ }
+ } else if (isQuickLabelInfo(this.label)) {
+ const status = getLabelStatus(this.label);
+ return classForLabelStatus(status);
+ }
+ return '';
}
}
diff --git a/polygerrit-ui/app/utils/label-util.ts b/polygerrit-ui/app/utils/label-util.ts
index 884afd7..7c94892 100644
--- a/polygerrit-ui/app/utils/label-util.ts
+++ b/polygerrit-ui/app/utils/label-util.ts
@@ -70,17 +70,23 @@
return max > -min ? max : min;
}
-export function getLabelStatus(
- label?: DetailedLabelInfo,
- vote?: number
-): LabelStatus {
- const value = vote ?? getRepresentativeValue(label);
- const range = getVotingRangeOrDefault(label);
- if (value < 0) {
- return value === range.min ? LabelStatus.REJECTED : LabelStatus.DISLIKED;
+export function getLabelStatus(label?: LabelInfo, vote?: number): LabelStatus {
+ if (!label) return LabelStatus.NEUTRAL;
+ if (isDetailedLabelInfo(label)) {
+ const value = vote ?? getRepresentativeValue(label);
+ const range = getVotingRangeOrDefault(label);
+ if (value < 0) {
+ return value === range.min ? LabelStatus.REJECTED : LabelStatus.DISLIKED;
+ }
+ if (value > 0) {
+ return value === range.max
+ ? LabelStatus.APPROVED
+ : LabelStatus.RECOMMENDED;
+ }
}
- if (value > 0) {
- return value === range.max ? LabelStatus.APPROVED : LabelStatus.RECOMMENDED;
+ if (isQuickLabelInfo(label)) {
+ if (label.approved) return LabelStatus.RECOMMENDED;
+ if (label.rejected) return LabelStatus.DISLIKED;
}
return LabelStatus.NEUTRAL;
}
diff --git a/polygerrit-ui/app/utils/label-util_test.ts b/polygerrit-ui/app/utils/label-util_test.ts
index 196c5e9..ec27758 100644
--- a/polygerrit-ui/app/utils/label-util_test.ts
+++ b/polygerrit-ui/app/utils/label-util_test.ts
@@ -32,8 +32,10 @@
AccountInfo,
ApprovalInfo,
DetailedLabelInfo,
+ QuickLabelInfo,
} from '../types/common';
import {
+ createAccountWithEmail,
createSubmitRequirementExpressionInfo,
createSubmitRequirementResultInfo,
} from '../test/test-data-generators';
@@ -171,6 +173,15 @@
assert.equal(getLabelStatus(labelInfo), LabelStatus.REJECTED);
});
+ test('getLabelStatus - quicklabelinfo', () => {
+ let labelInfo: QuickLabelInfo = {};
+ assert.equal(getLabelStatus(labelInfo), LabelStatus.NEUTRAL);
+ labelInfo = {approved: createAccountWithEmail()};
+ assert.equal(getLabelStatus(labelInfo), LabelStatus.RECOMMENDED);
+ labelInfo = {rejected: createAccountWithEmail()};
+ assert.equal(getLabelStatus(labelInfo), LabelStatus.DISLIKED);
+ });
+
test('getRepresentativeValue', () => {
let labelInfo: DetailedLabelInfo = {all: []};
assert.equal(getRepresentativeValue(labelInfo), 0);
