Merge "ProjectWatchInfo: Suppress false positive OrphanedFormatString warning"
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/CommentsUtil.java b/java/com/google/gerrit/server/CommentsUtil.java
index 3d3603f..ba9f6d6 100644
--- a/java/com/google/gerrit/server/CommentsUtil.java
+++ b/java/com/google/gerrit/server/CommentsUtil.java
@@ -450,6 +450,10 @@
/**
* Get NoteDb draft refs for a change.
*
+ * <p>This is just a simple ref scan, so the results may potentially include refs for zombie draft
+ * comments. A zombie draft is one which has been published but the write to delete the draft ref
+ * from All-Users failed.
+ *
* @param changeId change ID.
* @return raw refs from All-Users repo.
*/
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/ChangeNotes.java b/java/com/google/gerrit/server/notedb/ChangeNotes.java
index f2034af..2d9b014 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNotes.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNotes.java
@@ -32,6 +32,7 @@
import com.google.common.collect.ListMultimap;
import com.google.common.collect.Lists;
import com.google.common.collect.MultimapBuilder;
+import com.google.common.collect.Multimaps;
import com.google.common.collect.Ordering;
import com.google.common.collect.Sets;
import com.google.common.collect.Sets.SetView;
@@ -522,7 +523,12 @@
public ImmutableListMultimap<ObjectId, HumanComment> getDraftComments(
Account.Id author, @Nullable Ref ref) {
loadDraftComments(author, ref);
- return draftCommentNotes.getComments();
+ // Filter out any zombie draft comments. These are drafts that are also in
+ // the published map, and arise when the update to All-Users to delete them
+ // during the publish operation failed.
+ return ImmutableListMultimap.copyOf(
+ Multimaps.filterEntries(
+ draftCommentNotes.getComments(), e -> !getCommentKeys().contains(e.getValue().key)));
}
public ImmutableListMultimap<ObjectId, RobotComment> getRobotComments() {
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/notedb/StoreSubmitRequirementsOp.java b/java/com/google/gerrit/server/notedb/StoreSubmitRequirementsOp.java
index 57a3cd7..1a7d5af 100644
--- a/java/com/google/gerrit/server/notedb/StoreSubmitRequirementsOp.java
+++ b/java/com/google/gerrit/server/notedb/StoreSubmitRequirementsOp.java
@@ -50,7 +50,9 @@
// patchset to the user before it was merged.
ChangeData changeData = changeDataFactory.create(ctx.getProject(), ctx.getChange().getId());
ChangeUpdate update = ctx.getUpdate(ctx.getChange().currentPatchSetId());
- update.putSubmitRequirementResults(evaluator.evaluateAllRequirements(changeData).values());
+ // We do not want to store submit requirements in NoteDb for legacy submit records
+ update.putSubmitRequirementResults(
+ evaluator.evaluateAllRequirements(changeData, /* includeLegacy= */ false).values());
return !changeData.submitRequirements().isEmpty();
}
}
diff --git a/java/com/google/gerrit/server/project/SubmitRequirementsAdapter.java b/java/com/google/gerrit/server/project/SubmitRequirementsAdapter.java
index f028def..63a29cc 100644
--- a/java/com/google/gerrit/server/project/SubmitRequirementsAdapter.java
+++ b/java/com/google/gerrit/server/project/SubmitRequirementsAdapter.java
@@ -25,8 +25,12 @@
import com.google.gerrit.entities.SubmitRequirementExpressionResult;
import com.google.gerrit.entities.SubmitRequirementExpressionResult.Status;
import com.google.gerrit.entities.SubmitRequirementResult;
+import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.query.change.ChangeQueryBuilder;
import java.util.List;
+import java.util.Map;
+import java.util.function.Function;
+import java.util.stream.Collectors;
import org.eclipse.jgit.lib.ObjectId;
/**
@@ -38,7 +42,25 @@
private SubmitRequirementsAdapter() {}
- public static List<SubmitRequirementResult> createResult(
+ /**
+ * Retrieve legacy submit records (created by label functions and other {@link
+ * com.google.gerrit.server.rules.SubmitRule}s) and convert them to submit requirement results.
+ */
+ public static Map<SubmitRequirement, SubmitRequirementResult> getLegacyRequirements(
+ SubmitRuleEvaluator.Factory evaluator, ChangeData cd) {
+ // We use SubmitRuleOptions.defaults() which does not recompute submit rules for closed changes.
+ // This doesn't have an effect since we never call this class (i.e. to evaluate submit
+ // requirements) for closed changes.
+ List<SubmitRecord> records = evaluator.create(SubmitRuleOptions.defaults()).evaluate(cd);
+ List<LabelType> labelTypes = cd.getLabelTypes().getLabelTypes();
+ ObjectId commitId = cd.currentPatchSet().commitId();
+ return records.stream()
+ .map(r -> createResult(r, labelTypes, commitId))
+ .flatMap(List::stream)
+ .collect(Collectors.toMap(sr -> sr.submitRequirement(), Function.identity()));
+ }
+
+ static List<SubmitRequirementResult> createResult(
SubmitRecord record, List<LabelType> labelTypes, ObjectId psCommitId) {
List<SubmitRequirementResult> results;
if (record.ruleName.equals("gerrit~DefaultSubmitRule")) {
diff --git a/java/com/google/gerrit/server/project/SubmitRequirementsEvaluator.java b/java/com/google/gerrit/server/project/SubmitRequirementsEvaluator.java
index b3ac380..402bb51 100644
--- a/java/com/google/gerrit/server/project/SubmitRequirementsEvaluator.java
+++ b/java/com/google/gerrit/server/project/SubmitRequirementsEvaluator.java
@@ -26,8 +26,13 @@
/**
* Evaluate and return all submit requirement results for a change. Submit requirements are read
* from the project config of the project containing the change as well as parent projects.
+ *
+ * @param cd change data corresponding to a specific gerrit change
+ * @param includeLegacy if set to true, evaluate legacy {@link
+ * com.google.gerrit.entities.SubmitRecord}s and convert them to submit requirements.
*/
- Map<SubmitRequirement, SubmitRequirementResult> evaluateAllRequirements(ChangeData cd);
+ Map<SubmitRequirement, SubmitRequirementResult> evaluateAllRequirements(
+ ChangeData cd, boolean includeLegacy);
/** Evaluate a single {@link SubmitRequirement} using change data. */
SubmitRequirementResult evaluateRequirement(SubmitRequirement sr, ChangeData cd);
diff --git a/java/com/google/gerrit/server/project/SubmitRequirementsEvaluatorImpl.java b/java/com/google/gerrit/server/project/SubmitRequirementsEvaluatorImpl.java
index 9be50c7..de637b4 100644
--- a/java/com/google/gerrit/server/project/SubmitRequirementsEvaluatorImpl.java
+++ b/java/com/google/gerrit/server/project/SubmitRequirementsEvaluatorImpl.java
@@ -17,8 +17,6 @@
import static com.google.gerrit.server.project.ProjectCache.illegalState;
import com.google.common.collect.ImmutableMap;
-import com.google.gerrit.entities.LabelType;
-import com.google.gerrit.entities.SubmitRecord;
import com.google.gerrit.entities.SubmitRequirement;
import com.google.gerrit.entities.SubmitRequirementExpression;
import com.google.gerrit.entities.SubmitRequirementExpressionResult;
@@ -36,12 +34,8 @@
import com.google.inject.Provider;
import com.google.inject.Scopes;
import java.util.HashMap;
-import java.util.List;
import java.util.Map;
import java.util.Optional;
-import java.util.function.Function;
-import java.util.stream.Collectors;
-import org.eclipse.jgit.lib.ObjectId;
/** Evaluates submit requirements for different change data. */
public class SubmitRequirementsEvaluatorImpl implements SubmitRequirementsEvaluator {
@@ -81,12 +75,19 @@
}
@Override
- public Map<SubmitRequirement, SubmitRequirementResult> evaluateAllRequirements(ChangeData cd) {
- Map<SubmitRequirement, SubmitRequirementResult> result = getRequirements(cd);
- if (experimentFeatures.isFeatureEnabled(
- ExperimentFeaturesConstants
- .GERRIT_BACKEND_REQUEST_FEATURE_ENABLE_LEGACY_SUBMIT_REQUIREMENTS)) {
- result.putAll(getLegacyRequirements(cd));
+ public Map<SubmitRequirement, SubmitRequirementResult> evaluateAllRequirements(
+ ChangeData cd, boolean includeLegacy) {
+ Map<SubmitRequirement, SubmitRequirementResult> projectConfigRequirements = getRequirements(cd);
+ Map<SubmitRequirement, SubmitRequirementResult> result = projectConfigRequirements;
+ if (includeLegacy
+ && experimentFeatures.isFeatureEnabled(
+ ExperimentFeaturesConstants
+ .GERRIT_BACKEND_REQUEST_FEATURE_ENABLE_LEGACY_SUBMIT_REQUIREMENTS)) {
+ Map<SubmitRequirement, SubmitRequirementResult> legacyReqs =
+ SubmitRequirementsAdapter.getLegacyRequirements(legacyEvaluator, cd);
+ result =
+ SubmitRequirementsUtil.mergeLegacyAndNonLegacyRequirements(
+ projectConfigRequirements, legacyReqs);
}
return ImmutableMap.copyOf(result);
}
@@ -140,23 +141,6 @@
return result;
}
- /**
- * Convert and return legacy submit records (created by label functions and other {@link
- * com.google.gerrit.server.rules.SubmitRule}s to submit requirement results.
- */
- private Map<SubmitRequirement, SubmitRequirementResult> getLegacyRequirements(ChangeData cd) {
- // We use SubmitRuleOptions.defaults() which does not recompute submit rules for closed changes.
- // This doesn't have an effect since we never call this class (i.e. to evaluate submit
- // requirements) for closed changes.
- List<SubmitRecord> records = legacyEvaluator.create(SubmitRuleOptions.defaults()).evaluate(cd);
- List<LabelType> labelTypes = cd.getLabelTypes().getLabelTypes();
- ObjectId commitId = cd.currentPatchSet().commitId();
- return records.stream()
- .map(r -> SubmitRequirementsAdapter.createResult(r, labelTypes, commitId))
- .flatMap(List::stream)
- .collect(Collectors.toMap(sr -> sr.submitRequirement(), Function.identity()));
- }
-
/** Evaluate the predicate recursively using change data. */
private PredicateResult evaluatePredicateTree(
Predicate<ChangeData> predicate, ChangeData changeData) {
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 c551cd2..b8c8c07 100644
--- a/java/com/google/gerrit/server/query/change/ChangeData.java
+++ b/java/com/google/gerrit/server/query/change/ChangeData.java
@@ -75,6 +75,8 @@
import com.google.gerrit.server.change.PureRevert;
import com.google.gerrit.server.config.AllUsersName;
import com.google.gerrit.server.config.TrackingFooters;
+import com.google.gerrit.server.experiments.ExperimentFeatures;
+import com.google.gerrit.server.experiments.ExperimentFeaturesConstants;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.MergeUtil;
import com.google.gerrit.server.notedb.ChangeNotes;
@@ -87,7 +89,9 @@
import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gerrit.server.project.ProjectCache;
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;
@@ -268,7 +272,7 @@
ChangeData cd =
new ChangeData(
null, null, null, null, null, null, null, null, null, null, null, null, null, null,
- null, null, project, id, null, null);
+ null, null, null, project, id, null, null);
cd.currentPatchSet =
PatchSet.builder()
.id(PatchSet.id(id, currentPatchSetId))
@@ -286,6 +290,7 @@
private final ChangeMessagesUtil cmUtil;
private final ChangeNotes.Factory notesFactory;
private final CommentsUtil commentsUtil;
+ private final ExperimentFeatures experimentFeatures;
private final GitRepositoryManager repoManager;
private final MergeUtil.Factory mergeUtilFactory;
private final MergeabilityCache mergeabilityCache;
@@ -367,6 +372,7 @@
ChangeMessagesUtil cmUtil,
ChangeNotes.Factory notesFactory,
CommentsUtil commentsUtil,
+ ExperimentFeatures experimentFeatures,
GitRepositoryManager repoManager,
MergeUtil.Factory mergeUtilFactory,
MergeabilityCache mergeabilityCache,
@@ -386,6 +392,7 @@
this.cmUtil = cmUtil;
this.notesFactory = notesFactory;
this.commentsUtil = commentsUtil;
+ this.experimentFeatures = experimentFeatures;
this.repoManager = repoManager;
this.mergeUtilFactory = mergeUtilFactory;
this.mergeabilityCache = mergeabilityCache;
@@ -946,13 +953,28 @@
return Collections.emptyMap();
}
Change c = change();
- if (c != null && c.isClosed()) {
+ if (c == null || !c.isClosed()) {
+ // Open changes: Evaluate submit requirements online.
submitRequirements =
- notes().getSubmitRequirementsResult().stream()
- .collect(Collectors.toMap(r -> r.submitRequirement(), Function.identity()));
- } else {
- submitRequirements = submitRequirementsEvaluator.evaluateAllRequirements(this);
+ submitRequirementsEvaluator.evaluateAllRequirements(this, /* includeLegacy= */ true);
+ return submitRequirements;
}
+ // Closed changes: Load submit requirement results from NoteDb.
+ Map<SubmitRequirement, SubmitRequirementResult> projectConfigRequirements =
+ notes().getSubmitRequirementsResult().stream()
+ .filter(r -> !r.legacy())
+ .collect(Collectors.toMap(r -> r.submitRequirement(), Function.identity()));
+ if (!experimentFeatures.isFeatureEnabled(
+ ExperimentFeaturesConstants
+ .GERRIT_BACKEND_REQUEST_FEATURE_ENABLE_LEGACY_SUBMIT_REQUIREMENTS)) {
+ submitRequirements = projectConfigRequirements;
+ return submitRequirements;
+ }
+ Map<SubmitRequirement, SubmitRequirementResult> legacyRequirements =
+ SubmitRequirementsAdapter.getLegacyRequirements(submitRuleEvaluatorFactory, this);
+ submitRequirements =
+ SubmitRequirementsUtil.mergeLegacyAndNonLegacyRequirements(
+ projectConfigRequirements, legacyRequirements);
}
return submitRequirements;
}
@@ -1347,7 +1369,14 @@
draftsByUser = new HashMap<>();
for (Ref ref : commentsUtil.getDraftRefs(notes().getChangeId())) {
Account.Id account = Account.Id.fromRefSuffix(ref.getName());
- if (account != null) {
+ if (account != null
+ // Double-check that any drafts exist for this user after
+ // filtering out zombies. If some but not all drafts in the ref
+ // were zombies, the returned Ref still includes those zombies;
+ // this is suboptimal, but is ok for the purposes of
+ // draftsByUser(), and easier than trying to rebuild the change at
+ // this point.
+ && !notes().getDraftComments(account, ref).isEmpty()) {
draftsByUser.put(account, ref.getObjectId());
}
}
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
index 4c17800..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
@@ -4582,6 +4705,7 @@
// 4. Merge the change. Submit requirements status is presented from NoteDb.
gApi.changes().id(changeId).current().submit();
change = gApi.changes().id(changeId).get();
+ // Legacy submit records are returned as submit requirements.
assertThat(change.submitRequirements).hasSize(2);
assertSubmitRequirementStatus(
change.submitRequirements, "Code-Review", Status.SATISFIED, /* isLegacy= */ true);
@@ -5196,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/ChangeNotesTest.java b/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java
index 4e7b3f3..c524c94 100644
--- a/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java
+++ b/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java
@@ -76,6 +76,8 @@
import org.junit.Test;
public class ChangeNotesTest extends AbstractChangeNotesTest {
+ @Inject private DraftCommentNotes.Factory draftNotesFactory;
+
@Inject private ChangeNoteJson changeNoteJson;
@Test
@@ -2978,6 +2980,86 @@
}
@Test
+ public void filterOutAndFixUpZombieDraftComments() throws Exception {
+ Change c = newChange();
+ ObjectId commitId1 = ObjectId.fromString("abcd1234abcd1234abcd1234abcd1234abcd1234");
+ CommentRange range = new CommentRange(1, 1, 2, 1);
+ PatchSet.Id ps1 = c.currentPatchSetId();
+ short side = (short) 1;
+
+ ChangeUpdate update = newUpdate(c, otherUser);
+ Timestamp now = TimeUtil.nowTs();
+ HumanComment comment1 =
+ newComment(
+ ps1,
+ "file1",
+ "uuid1",
+ range,
+ range.getEndLine(),
+ otherUser,
+ null,
+ now,
+ "comment on ps1",
+ side,
+ commitId1,
+ false);
+ HumanComment comment2 =
+ newComment(
+ ps1,
+ "file2",
+ "uuid2",
+ range,
+ range.getEndLine(),
+ otherUser,
+ null,
+ now,
+ "another comment",
+ side,
+ commitId1,
+ false);
+ update.putComment(HumanComment.Status.DRAFT, comment1);
+ update.putComment(HumanComment.Status.DRAFT, comment2);
+ update.commit();
+
+ String refName = refsDraftComments(c.getId(), otherUserId);
+ ObjectId oldDraftId = exactRefAllUsers(refName);
+
+ update = newUpdate(c, otherUser);
+ update.setPatchSetId(ps1);
+ update.putComment(HumanComment.Status.PUBLISHED, comment2);
+ update.commit();
+ assertThat(exactRefAllUsers(refName)).isNotNull();
+ assertThat(exactRefAllUsers(refName)).isNotEqualTo(oldDraftId);
+
+ // Re-add draft version of comment2 back to draft ref without updating
+ // change ref. Simulates the case where deleting the draft failed
+ // non-atomically after adding the published comment succeeded.
+ ChangeDraftUpdate draftUpdate = newUpdate(c, otherUser).createDraftUpdateIfNull();
+ draftUpdate.putComment(comment2);
+ try (NoteDbUpdateManager manager = updateManagerFactory.create(c.getProject())) {
+ manager.add(draftUpdate);
+ manager.execute();
+ }
+
+ // Looking at drafts directly shows the zombie comment.
+ DraftCommentNotes draftNotes = draftNotesFactory.create(c.getId(), otherUserId);
+ assertThat(draftNotes.load().getComments().get(commitId1)).containsExactly(comment1, comment2);
+
+ // Zombie comment is filtered out of drafts via ChangeNotes.
+ ChangeNotes notes = newNotes(c);
+ assertThat(notes.getDraftComments(otherUserId).get(commitId1)).containsExactly(comment1);
+ assertThat(notes.getHumanComments().get(commitId1)).containsExactly(comment2);
+
+ update = newUpdate(c, otherUser);
+ update.setPatchSetId(ps1);
+ update.putComment(HumanComment.Status.PUBLISHED, comment1);
+ update.commit();
+
+ // Updating an unrelated comment causes the zombie comment to get fixed up.
+ assertThat(exactRefAllUsers(refName)).isNull();
+ }
+
+ @Test
public void updateCommentsInSequentialUpdates() throws Exception {
Change c = newChange();
CommentRange range = new CommentRange(1, 1, 2, 1);
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/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
index 2663853..9ebee9c 100644
--- a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
+++ b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
@@ -69,6 +69,7 @@
import com.google.gerrit.extensions.api.changes.DraftInput;
import com.google.gerrit.extensions.api.changes.HashtagsInput;
import com.google.gerrit.extensions.api.changes.ReviewInput;
+import com.google.gerrit.extensions.api.changes.ReviewInput.DraftHandling;
import com.google.gerrit.extensions.api.changes.ReviewerInput;
import com.google.gerrit.extensions.api.groups.GroupInput;
import com.google.gerrit.extensions.api.projects.ConfigInput;
@@ -2333,6 +2334,44 @@
}
@Test
+ public void byHasDraftExcludesZombieDrafts() throws Exception {
+ Project.NameKey project = Project.nameKey("repo");
+ TestRepository<Repo> repo = createProject(project.get());
+ Change change = insert(repo, newChange(repo));
+ Change.Id id = change.getId();
+
+ DraftInput in = new DraftInput();
+ in.line = 1;
+ in.message = "nit: trailing whitespace";
+ in.path = Patch.COMMIT_MSG;
+ gApi.changes().id(id.get()).current().createDraft(in);
+
+ assertQuery("has:draft", change);
+ assertQuery("commentby:" + userId);
+
+ try (TestRepository<Repo> allUsers =
+ new TestRepository<>(repoManager.openRepository(allUsersName))) {
+ Ref draftsRef = allUsers.getRepository().exactRef(RefNames.refsDraftComments(id, userId));
+ assertThat(draftsRef).isNotNull();
+
+ ReviewInput rin = ReviewInput.dislike();
+ rin.drafts = DraftHandling.PUBLISH_ALL_REVISIONS;
+ gApi.changes().id(id.get()).current().review(rin);
+
+ assertQuery("has:draft");
+ assertQuery("commentby:" + userId, change);
+ assertThat(allUsers.getRepository().exactRef(draftsRef.getName())).isNull();
+
+ // Re-add drafts ref and ensure it gets filtered out during indexing.
+ allUsers.update(draftsRef.getName(), draftsRef.getObjectId());
+ assertThat(allUsers.getRepository().exactRef(draftsRef.getName())).isNotNull();
+ }
+
+ indexer.index(project, id);
+ assertQuery("has:draft");
+ }
+
+ @Test
public void byStarredBy() throws Exception {
TestRepository<Repo> repo = createProject("repo");
Change change1 = insert(repo, newChange(repo));
diff --git a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.ts b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.ts
index ba62ec3..8b46cd8 100644
--- a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.ts
@@ -752,6 +752,16 @@
})
);
}
+
+ _showNewSubmitRequirements(change?: ParsedChangeInfo) {
+ if (!this._isSubmitRequirementsUiEnabled) return false;
+ return (change?.submit_requirements ?? []).length > 0;
+ }
+
+ _showNewSubmitRequirementWarning(change?: ParsedChangeInfo) {
+ if (!this._isSubmitRequirementsUiEnabled) return false;
+ return (change?.submit_requirements ?? []).length === 0;
+ }
}
declare global {
diff --git a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_html.ts b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_html.ts
index 26d1277..97101f6 100644
--- a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_html.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_html.ts
@@ -113,6 +113,10 @@
--iron-icon-height: 18px;
--iron-icon-width: 18px;
}
+ .submit-requirement-error {
+ color: var(--deemphasized-text-color);
+ padding-left: var(--metadata-horizontal-padding);
+ }
</style>
<gr-external-style id="externalStyle" name="change-metadata">
<div class="metadata-header">
@@ -480,20 +484,25 @@
</span>
</section>
<div class="separatedSection">
- <template is="dom-if" if="[[_isSubmitRequirementsUiEnabled]]">
+ <template is="dom-if" if="[[_showNewSubmitRequirements(change)]]">
<gr-submit-requirements
change="[[change]]"
account="[[account]]"
mutable="[[_mutable]]"
></gr-submit-requirements>
</template>
- <template is="dom-if" if="[[!_isSubmitRequirementsUiEnabled]]">
+ <template is="dom-if" if="[[!_showNewSubmitRequirements(change)]]">
<gr-change-requirements
change="{{change}}"
account="[[account]]"
mutable="[[_mutable]]"
></gr-change-requirements>
</template>
+ <template is="dom-if" if="[[_showNewSubmitRequirementWarning(change)]]">
+ <div class="submit-requirement-error">
+ New Submit Requirements don't work on this change.
+ </div>
+ </template>
</div>
<section
id="webLinks"
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-requirement-hovercard/gr-submit-requirement-hovercard.ts b/polygerrit-ui/app/elements/change/gr-submit-requirement-hovercard/gr-submit-requirement-hovercard.ts
index 2e00034..3c9f54c 100644
--- a/polygerrit-ui/app/elements/change/gr-submit-requirement-hovercard/gr-submit-requirement-hovercard.ts
+++ b/polygerrit-ui/app/elements/change/gr-submit-requirement-hovercard/gr-submit-requirement-hovercard.ts
@@ -14,18 +14,14 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-import '../../../styles/gr-font-styles';
-import '../../shared/gr-hovercard/gr-hovercard-shared-style';
import '../../shared/gr-button/gr-button';
-import {PolymerElement} from '@polymer/polymer/polymer-element';
-import {customElement, property} from '@polymer/decorators';
-import {HovercardBehaviorMixin} from '../../shared/gr-hovercard/gr-hovercard-behavior';
-import {htmlTemplate} from './gr-submit-requirement-hovercard_html';
+import '../../shared/gr-label-info/gr-label-info';
+import '../../shared/gr-limited-text/gr-limited-text';
+import {customElement, property} from 'lit/decorators';
import {
AccountInfo,
SubmitRequirementExpressionInfo,
SubmitRequirementResultInfo,
- SubmitRequirementStatus,
} from '../../../api/rest-api';
import {
extractAssociatedLabels,
@@ -33,16 +29,15 @@
} from '../../../utils/label-util';
import {ParsedChangeInfo} from '../../../types/types';
import {Label} from '../gr-change-requirements/gr-change-requirements';
+import {css, html, LitElement} from 'lit';
+import {HovercardMixin} from '../../../mixins/hovercard-mixin/hovercard-mixin';
+import {fontStyles} from '../../../styles/gr-font-styles';
// This avoids JSC_DYNAMIC_EXTENDS_WITHOUT_JSDOC closure compiler error.
-const base = HovercardBehaviorMixin(PolymerElement);
+const base = HovercardMixin(LitElement);
@customElement('gr-submit-requirement-hovercard')
export class GrHovercardRun extends base {
- static get template() {
- return htmlTemplate;
- }
-
@property({type: Object})
requirement?: SubmitRequirementResultInfo;
@@ -58,16 +53,176 @@
@property({type: Boolean})
expanded = false;
- @property({type: Array, computed: 'computeLabels(change, requirement)'})
- _labels: Label[] = [];
+ static override get styles() {
+ return [
+ fontStyles,
+ base.styles || [],
+ css`
+ #container {
+ min-width: 356px;
+ max-width: 356px;
+ padding: var(--spacing-xl) 0 var(--spacing-m) 0;
+ }
+ section.label {
+ display: table-row;
+ }
+ .label-title {
+ min-width: 10em;
+ padding-top: var(--spacing-s);
+ }
+ .label-value {
+ padding-top: var(--spacing-s);
+ }
+ .label-title,
+ .label-value {
+ display: table-cell;
+ vertical-align: top;
+ }
+ .row {
+ display: flex;
+ }
+ .title {
+ color: var(--deemphasized-text-color);
+ margin-right: var(--spacing-m);
+ }
+ div.section {
+ margin: 0 var(--spacing-xl) var(--spacing-m) var(--spacing-xl);
+ display: flex;
+ align-items: center;
+ }
+ div.sectionIcon {
+ flex: 0 0 30px;
+ }
+ div.sectionIcon iron-icon {
+ position: relative;
+ width: 20px;
+ height: 20px;
+ }
+ .condition {
+ background-color: var(--gray-background);
+ padding: var(--spacing-m);
+ flex-grow: 1;
+ }
+ .expression {
+ color: var(--gray-foreground);
+ }
+ iron-icon.check {
+ color: var(--success-foreground);
+ }
+ iron-icon.close {
+ color: var(--warning-foreground);
+ }
+ .showConditions iron-icon {
+ color: inherit;
+ }
+ div.showConditions {
+ border-top: 1px solid var(--border-color);
+ margin-top: var(--spacing-m);
+ padding: var(--spacing-m) var(--spacing-xl) 0;
+ }
+ `,
+ ];
+ }
- computeLabels(
- change?: ParsedChangeInfo,
- requirement?: SubmitRequirementResultInfo
- ) {
- if (!requirement) return [];
- const requirementLabels = extractAssociatedLabels(requirement);
- const labels = change?.labels ?? {};
+ override render() {
+ if (!this.requirement) return;
+ const icon = iconForStatus(this.requirement.status);
+ return html` <div id="container" role="tooltip" tabindex="-1">
+ <div class="section">
+ <div class="sectionIcon">
+ <iron-icon class="${icon}" icon="gr-icons:${icon}"></iron-icon>
+ </div>
+ <div class="sectionContent">
+ <h3 class="name heading-3">
+ <span>${this.requirement.name}</span>
+ </h3>
+ </div>
+ </div>
+ <div class="section">
+ <div class="sectionIcon">
+ <iron-icon class="small" icon="gr-icons:info-outline"></iron-icon>
+ </div>
+ <div class="sectionContent">
+ <div class="row">
+ <div class="title">Status</div>
+ <div>${this.requirement.status}</div>
+ </div>
+ </div>
+ </div>
+ ${this.renderLabelSection()} ${this.renderConditionSection()}
+ </div>`;
+ }
+
+ private renderLabelSection() {
+ const labels = this.computeLabels();
+ return html` <div class="section">
+ ${labels.map(l => this.renderLabel(l))}
+ </div>`;
+ }
+
+ private renderLabel(label: Label) {
+ return html`
+ <section class="label">
+ <div class="label-title">
+ <gr-limited-text
+ class="name"
+ limit="25"
+ text="${label.labelName}"
+ ></gr-limited-text>
+ </div>
+ <div class="label-value">
+ <gr-label-info
+ .change=${this.change}
+ .account=${this.account}
+ .mutable=${this.mutable}
+ .label="${label.labelName}"
+ .labelInfo="${label.labelInfo}"
+ ></gr-label-info>
+ </div>
+ </section>
+ `;
+ }
+
+ private renderConditionSection() {
+ if (!this.expanded) {
+ return html` <div class="showConditions">
+ <gr-button
+ link=""
+ class="showConditions"
+ @click="${(_: MouseEvent) => this.handleShowConditions()}"
+ >
+ View condition
+ <iron-icon icon="gr-icons:expand-more"></iron-icon
+ ></gr-button>
+ </div>`;
+ } else {
+ return html`
+ <div class="section">
+ <div class="sectionIcon">
+ <iron-icon icon="gr-icons:description"></iron-icon>
+ </div>
+ <div class="sectionContent">${this.requirement?.description}</div>
+ </div>
+ ${this.renderCondition(
+ 'Blocking condition',
+ this.requirement?.submittability_expression_result
+ )}
+ ${this.renderCondition(
+ 'Application condition',
+ this.requirement?.applicability_expression_result
+ )}
+ ${this.renderCondition(
+ 'Override condition',
+ this.requirement?.override_expression_result
+ )}
+ `;
+ }
+ }
+
+ private computeLabels() {
+ if (!this.requirement) return [];
+ const requirementLabels = extractAssociatedLabels(this.requirement);
+ const labels = this.change?.labels ?? {};
const allLabels: Label[] = [];
@@ -84,17 +239,23 @@
return allLabels;
}
- computeIcon(status: SubmitRequirementStatus) {
- return iconForStatus(status);
- }
-
- renderCondition(expression?: SubmitRequirementExpressionInfo) {
+ private renderCondition(
+ name: string,
+ expression?: SubmitRequirementExpressionInfo
+ ) {
if (!expression) return '';
-
- return expression.expression;
+ return html`
+ <div class="section">
+ <div class="sectionIcon"></div>
+ <div class="sectionContent condition">
+ ${name}:<br />
+ <span class="expression"> ${expression.expression} </span>
+ </div>
+ </div>
+ `;
}
- _handleShowConditions() {
+ private handleShowConditions() {
this.expanded = true;
}
}
diff --git a/polygerrit-ui/app/elements/change/gr-submit-requirement-hovercard/gr-submit-requirement-hovercard_html.ts b/polygerrit-ui/app/elements/change/gr-submit-requirement-hovercard/gr-submit-requirement-hovercard_html.ts
deleted file mode 100644
index b7b4d9c..0000000
--- a/polygerrit-ui/app/elements/change/gr-submit-requirement-hovercard/gr-submit-requirement-hovercard_html.ts
+++ /dev/null
@@ -1,189 +0,0 @@
-/**
- * @license
- * 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.
- */
-import {html} from '@polymer/polymer/lib/utils/html-tag';
-
-export const htmlTemplate = html`
- <style include="gr-font-styles">
- /* Workaround for empty style block - see https://github.com/Polymer/tools/issues/408 */
- </style>
- <style include="gr-hovercard-shared-style">
- #container {
- min-width: 356px;
- max-width: 356px;
- padding: var(--spacing-xl) 0 var(--spacing-m) 0;
- }
- section.label {
- display: table-row;
- }
- .label-title {
- min-width: 10em;
- padding-top: var(--spacing-s);
- }
- .label-value {
- padding-top: var(--spacing-s);
- }
- .label-title,
- .label-value {
- display: table-cell;
- vertical-align: top;
- }
- .row {
- display: flex;
- }
- .title {
- color: var(--deemphasized-text-color);
- margin-right: var(--spacing-m);
- }
- div.section {
- margin: 0 var(--spacing-xl) var(--spacing-m) var(--spacing-xl);
- display: flex;
- align-items: center;
- }
- div.sectionIcon {
- flex: 0 0 30px;
- }
- div.sectionIcon iron-icon {
- position: relative;
- width: 20px;
- height: 20px;
- }
- .condition {
- background-color: var(--gray-background);
- padding: var(--spacing-m);
- flex-grow: 1;
- }
- .expression {
- color: var(--gray-foreground);
- }
- iron-icon.check {
- color: var(--success-foreground);
- }
- iron-icon.close {
- color: var(--warning-foreground);
- }
- .showConditions iron-icon {
- color: inherit;
- }
- div.showConditions {
- border-top: 1px solid var(--border-color);
- margin-top: var(--spacing-m);
- padding: var(--spacing-m) var(--spacing-xl) 0;
- }
- </style>
- <div id="container" role="tooltip" tabindex="-1">
- <div class="section">
- <div class="sectionIcon">
- <iron-icon
- class$="[[computeIcon(requirement.status)]]"
- icon="gr-icons:[[computeIcon(requirement.status)]]"
- ></iron-icon>
- </div>
- <div class="sectionContent">
- <h3 class="name heading-3">
- <span>[[requirement.name]]</span>
- </h3>
- </div>
- </div>
- <div class="section">
- <div class="sectionIcon">
- <iron-icon class="small" icon="gr-icons:info-outline"></iron-icon>
- </div>
- <div class="sectionContent">
- <div class="row">
- <div class="title">Status</div>
- <div>[[requirement.status]]</div>
- </div>
- </div>
- </div>
- <div class="section">
- <template is="dom-repeat" items="[[_labels]]">
- <section class="label">
- <div class="label-title">
- <gr-limited-text
- class="name"
- limit="25"
- text="[[item.labelName]]"
- ></gr-limited-text>
- </div>
- <div class="label-value">
- <gr-label-info
- change="{{change}}"
- account="[[account]]"
- mutable="[[mutable]]"
- label="[[item.labelName]]"
- label-info="[[item.labelInfo]]"
- ></gr-label-info>
- </div>
- </section>
- </template>
- </div>
- <template is="dom-if" if="[[!expanded]]">
- <div class="showConditions">
- <gr-button
- link=""
- class="showConditions"
- on-click="_handleShowConditions"
- >
- View condition
- <iron-icon icon="gr-icons:expand-more"></iron-icon
- ></gr-button>
- </div>
- </template>
- <template is="dom-if" if="[[expanded]]">
- <div class="section">
- <div class="sectionIcon">
- <iron-icon icon="gr-icons:description"></iron-icon>
- </div>
- <div class="sectionContent">[[requirement.description]]</div>
- </div>
- <div class="section">
- <div class="sectionIcon"></div>
- <div class="sectionContent condition">
- Blocking condition:<br />
- <span class="expression">
- [[renderCondition(requirement.submittability_expression_result)]]
- </span>
- </div>
- </div>
- <template
- is="dom-if"
- if="[[requirement.applicability_expression_result]]"
- >
- <div class="section">
- <div class="sectionIcon"></div>
- <div class="sectionContent condition">
- Application condition:<br />
- <span class="expression">
- [[renderCondition(requirement.applicability_expression_result)]]
- </span>
- </div>
- </div>
- </template>
- <template is="dom-if" if="[[requirement.override_expression_result]]">
- <div class="section">
- <div class="sectionIcon"></div>
- <div class="sectionContent condition">
- Override condition:<br />
- <span class="expression">
- [[renderCondition(requirement.override_expression_result)]]
- </span>
- </div>
- </div>
- </template>
- </template>
- </div>
-`;
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/checks/gr-checks-results.ts b/polygerrit-ui/app/elements/checks/gr-checks-results.ts
index 48ccf2c..9c27cdb 100644
--- a/polygerrit-ui/app/elements/checks/gr-checks-results.ts
+++ b/polygerrit-ui/app/elements/checks/gr-checks-results.ts
@@ -293,7 +293,13 @@
override firstUpdated() {
const loading = this.shadowRoot?.querySelector('.container');
assertIsDefined(loading, '"Loading" element');
- whenVisible(loading, () => this.setAttribute('shouldRender', 'true'), 200);
+ whenVisible(
+ loading,
+ () => {
+ this.shouldRender = true;
+ },
+ 200
+ );
}
override render() {
diff --git a/polygerrit-ui/app/elements/checks/gr-hovercard-run.ts b/polygerrit-ui/app/elements/checks/gr-hovercard-run.ts
index d26856c..95b7157 100644
--- a/polygerrit-ui/app/elements/checks/gr-hovercard-run.ts
+++ b/polygerrit-ui/app/elements/checks/gr-hovercard-run.ts
@@ -14,12 +14,8 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-import './gr-checks-styles';
-import '../../styles/gr-font-styles';
-import {HovercardBehaviorMixin} from '../shared/gr-hovercard/gr-hovercard-behavior';
-import {PolymerElement} from '@polymer/polymer/polymer-element';
-import {htmlTemplate} from './gr-hovercard-run_html';
-import {customElement, property} from '@polymer/decorators';
+import {fontStyles} from '../../styles/gr-font-styles';
+import {customElement, property} from 'lit/decorators';
import './gr-checks-action';
import {CheckRun} from '../../services/checks/checks-model';
import {
@@ -31,102 +27,346 @@
import {durationString, fromNow} from '../../utils/date-util';
import {RunStatus} from '../../api/checks';
import {ordinal} from '../../utils/string-util';
+import {HovercardMixin} from '../../mixins/hovercard-mixin/hovercard-mixin';
+import {css, html, LitElement} from 'lit';
+import {checksStyles} from './gr-checks-styles';
// This avoids JSC_DYNAMIC_EXTENDS_WITHOUT_JSDOC closure compiler error.
-const base = HovercardBehaviorMixin(PolymerElement);
+const base = HovercardMixin(LitElement);
@customElement('gr-hovercard-run')
export class GrHovercardRun extends base {
- static get template() {
- return htmlTemplate;
- }
-
@property({type: Object})
run?: CheckRun;
- computeIcon(run?: CheckRun) {
- if (!run) return '';
- const category = worstCategory(run);
+ static override get styles() {
+ return [
+ fontStyles,
+ checksStyles,
+ base.styles || [],
+ css`
+ #container {
+ min-width: 356px;
+ max-width: 356px;
+ padding: var(--spacing-xl) 0 var(--spacing-m) 0;
+ }
+ .row {
+ display: flex;
+ margin-top: var(--spacing-s);
+ }
+ .attempts.row {
+ flex-wrap: wrap;
+ }
+ .chipRow {
+ display: flex;
+ margin-top: var(--spacing-s);
+ }
+ .chip {
+ background: var(--gray-background);
+ color: var(--gray-foreground);
+ border-radius: 20px;
+ padding: var(--spacing-xs) var(--spacing-m) var(--spacing-xs)
+ var(--spacing-s);
+ }
+ .title {
+ color: var(--deemphasized-text-color);
+ margin-right: var(--spacing-m);
+ }
+ div.section {
+ margin: 0 var(--spacing-xl) var(--spacing-m) var(--spacing-xl);
+ display: flex;
+ }
+ div.sectionIcon {
+ flex: 0 0 30px;
+ }
+ div.chip iron-icon {
+ width: 16px;
+ height: 16px;
+ /* Positioning of a 16px icon in the middle of a 20px line. */
+ position: relative;
+ top: 2px;
+ }
+ div.sectionIcon iron-icon {
+ position: relative;
+ top: 2px;
+ width: 20px;
+ height: 20px;
+ }
+ div.sectionIcon iron-icon.small {
+ position: relative;
+ top: 6px;
+ width: 16px;
+ height: 16px;
+ }
+ div.sectionContent iron-icon.link {
+ color: var(--link-color);
+ }
+ div.sectionContent .attemptIcon iron-icon,
+ div.sectionContent iron-icon.small {
+ width: 16px;
+ height: 16px;
+ margin-right: var(--spacing-s);
+ /* Positioning of a 16px icon in the middle of a 20px line. */
+ position: relative;
+ top: 2px;
+ }
+ div.sectionContent .attemptIcon iron-icon {
+ margin-right: 0;
+ }
+ .attemptIcon,
+ .attemptNumber {
+ margin-right: var(--spacing-s);
+ color: var(--deemphasized-text-color);
+ text-align: center;
+ width: 24px;
+ font-size: var(--font-size-small);
+ }
+ div.action {
+ border-top: 1px solid var(--border-color);
+ margin-top: var(--spacing-m);
+ padding: var(--spacing-m) var(--spacing-xl) 0;
+ }
+ `,
+ ];
+ }
+
+ override render() {
+ if (!this.run) return '';
+ const icon = this.computeIcon();
+ return html`
+ <div id="container" role="tooltip" tabindex="-1">
+ <div class="section">
+ <div
+ ?hidden="${!this.run || this.run.status === RunStatus.RUNNABLE}"
+ class="chipRow"
+ >
+ <div class="chip">
+ <iron-icon icon="gr-icons:${this.computeChipIcon()}"></iron-icon>
+ <span>${this.run.status}</span>
+ </div>
+ </div>
+ </div>
+ <div class="section">
+ <div class="sectionIcon" ?hidden="${icon.length === 0}">
+ <iron-icon class="${icon}" icon="gr-icons:${icon}"></iron-icon>
+ </div>
+ <div class="sectionContent">
+ <h3 class="name heading-3">
+ <span>${this.run.checkName}</span>
+ </h3>
+ </div>
+ </div>
+ ${this.renderStatusSection()} ${this.renderAttemptSection()}
+ ${this.renderTimestampSection()} ${this.renderDescriptionSection()}
+ ${this.renderActions()}
+ </div>
+ `;
+ }
+
+ private renderStatusSection() {
+ if (!this.run || (!this.run.statusLink && !this.run.statusDescription))
+ return;
+
+ return html`
+ <div class="section">
+ <div class="sectionIcon">
+ <iron-icon class="small" icon="gr-icons:info-outline"></iron-icon>
+ </div>
+ <div class="sectionContent">
+ ${this.run.statusLink
+ ? html` <div class="row">
+ <div class="title">Status</div>
+ <div>
+ <a href="${this.run.statusLink}" target="_blank"
+ ><iron-icon
+ aria-label="external link to check status"
+ class="small link"
+ icon="gr-icons:launch"
+ ></iron-icon
+ >${this.computeHostName(this.run.statusLink)}
+ </a>
+ </div>
+ </div>`
+ : ''}
+ ${this.run.statusDescription
+ ? html` <div class="row">
+ <div class="title">Message</div>
+ <div>${this.run.statusDescription}</div>
+ </div>`
+ : ''}
+ </div>
+ </div>
+ `;
+ }
+
+ private renderAttemptSection() {
+ if (this.hideAttempts()) return;
+ const attempts = this.computeAttempts();
+ return html`
+ <div class="section">
+ <div class="sectionIcon">
+ <iron-icon class="small" icon="gr-icons:arrow-forward"></iron-icon>
+ </div>
+ <div class="sectionContent">
+ <div class="attempts row">
+ <div class="title">Attempt</div>
+ ${attempts.map(a => this.renderAttempt(a))}
+ </div>
+ </div>
+ </div>
+ `;
+ }
+
+ private renderAttempt(attempt: AttemptDetail) {
+ return html`
+ <div>
+ <div class="attemptIcon">
+ <iron-icon
+ class="${attempt.icon}"
+ icon="gr-icons:${attempt.icon}"
+ ></iron-icon>
+ </div>
+ <div class="attemptNumber">${ordinal(attempt.attempt)}</div>
+ </div>
+ `;
+ }
+
+ private renderTimestampSection() {
+ if (
+ !this.run ||
+ (!this.run.startedTimestamp &&
+ !this.run.scheduledTimestamp &&
+ !this.run.finishedTimestamp)
+ )
+ return;
+
+ return html`
+ <div class="section">
+ <div class="sectionIcon">
+ <iron-icon class="small" icon="gr-icons:schedule"></iron-icon>
+ </div>
+ <div class="sectionContent">
+ <div ?hidden="${this.hideScheduled()}" class="row">
+ <div class="title">Scheduled</div>
+ <div>${this.computeDuration(this.run.scheduledTimestamp)}</div>
+ </div>
+ <div ?hidden="${!this.run.startedTimestamp}" class="row">
+ <div class="title">Started</div>
+ <div>${this.computeDuration(this.run.startedTimestamp)}</div>
+ </div>
+ <div ?hidden="${!this.run.finishedTimestamp}" class="row">
+ <div class="title">Ended</div>
+ <div>${this.computeDuration(this.run.finishedTimestamp)}</div>
+ </div>
+ <div ?hidden="${this.hideCompletion()}" class="row">
+ <div class="title">Completion</div>
+ <div>${this.computeCompletionDuration()}</div>
+ </div>
+ </div>
+ </div>
+ `;
+ }
+
+ private renderDescriptionSection() {
+ if (!this.run || (!this.run.checkLink && !this.run.checkDescription))
+ return;
+ return html`
+ <div class="section">
+ <div class="sectionIcon">
+ <iron-icon class="small" icon="gr-icons:link"></iron-icon>
+ </div>
+ <div class="sectionContent">
+ ${this.run.checkDescription
+ ? html` <div class="row">
+ <div class="title">Description</div>
+ <div>${this.run.checkDescription}</div>
+ </div>`
+ : ''}
+ ${this.run.checkLink
+ ? html` <div class="row">
+ <div class="title">Documentation</div>
+ <div>
+ <a href="${this.run.checkLink}" target="_blank"
+ ><iron-icon
+ aria-label="external link to check documentation"
+ class="small link"
+ icon="gr-icons:launch"
+ ></iron-icon
+ >${this.computeHostName(this.run.checkLink)}
+ </a>
+ </div>
+ </div>`
+ : ''}
+ </div>
+ </div>
+ `;
+ }
+
+ private renderActions() {
+ const actions = runActions(this.run);
+ return actions.map(
+ action =>
+ html`
+ <div class="action">
+ <gr-checks-action
+ .eventTarget="${this._target}"
+ .action="${action}"
+ ></gr-checks-action>
+ </div>
+ `
+ );
+ }
+
+ computeIcon() {
+ if (!this.run) return '';
+ const category = worstCategory(this.run);
if (category) return iconFor(category);
- return run.status === RunStatus.COMPLETED
+ return this.run.status === RunStatus.COMPLETED
? iconFor(RunStatus.COMPLETED)
: '';
}
- computeActions(run?: CheckRun) {
- return runActions(run);
- }
-
- computeAttempt(attempt?: number) {
- return ordinal(attempt);
- }
-
- computeAttempts(run?: CheckRun): AttemptDetail[] {
- const details = run?.attemptDetails ?? [];
+ computeAttempts(): AttemptDetail[] {
+ const details = this.run?.attemptDetails ?? [];
const more =
details.length > 7 ? [{icon: 'more-horiz', attempt: undefined}] : [];
return [...more, ...details.slice(-7)];
}
- computeChipIcon(run?: CheckRun) {
- if (run?.status === RunStatus.COMPLETED) return 'check';
- if (run?.status === RunStatus.RUNNING) return 'timelapse';
+ private computeChipIcon() {
+ if (this.run?.status === RunStatus.COMPLETED) return 'check';
+ if (this.run?.status === RunStatus.RUNNING) return 'timelapse';
return '';
}
- computeCompletionDuration(run?: CheckRun) {
- if (!run?.finishedTimestamp || !run?.startedTimestamp) return '';
- return durationString(run.startedTimestamp, run.finishedTimestamp, true);
- }
-
- computeDuration(date?: Date) {
- return date ? fromNow(date) : '';
- }
-
- computeHostName(link?: string) {
- return link ? new URL(link).hostname : '';
- }
-
- hideChip(run?: CheckRun) {
- return !run || run.status === RunStatus.RUNNABLE;
- }
-
- hideHeaderSectionIcon(run?: CheckRun) {
- return this.computeIcon(run).length === 0;
- }
-
- hideStatusSection(run?: CheckRun) {
- if (!run) return true;
- return !run.statusLink && !run.statusDescription;
- }
-
- hideTimestampSection(run?: CheckRun) {
- if (!run) return true;
- return (
- !run.startedTimestamp && !run.scheduledTimestamp && !run.finishedTimestamp
+ private computeCompletionDuration() {
+ if (!this.run?.finishedTimestamp || !this.run?.startedTimestamp) return '';
+ return durationString(
+ this.run.startedTimestamp,
+ this.run.finishedTimestamp,
+ true
);
}
- hideAttempts(run?: CheckRun) {
- const attemptCount = run?.attemptDetails?.length;
+ private computeDuration(date?: Date) {
+ return date ? fromNow(date) : '';
+ }
+
+ private computeHostName(link?: string) {
+ return link ? new URL(link).hostname : '';
+ }
+
+ private hideAttempts() {
+ const attemptCount = this.run?.attemptDetails?.length;
return attemptCount === undefined || attemptCount < 2;
}
- hideScheduled(run?: CheckRun) {
- return !run?.scheduledTimestamp || !!run?.startedTimestamp;
+ private hideScheduled() {
+ return !this.run?.scheduledTimestamp || !!this.run?.startedTimestamp;
}
- hideCompletion(run?: CheckRun) {
- return !run?.startedTimestamp || !run?.finishedTimestamp;
- }
-
- hideDescriptionSection(run?: CheckRun) {
- if (!run) return true;
- return !run.checkLink && !run.checkDescription;
- }
-
- _convertUndefined(value?: string) {
- return value ?? '';
+ private hideCompletion() {
+ return !this.run?.startedTimestamp || !this.run?.finishedTimestamp;
}
}
diff --git a/polygerrit-ui/app/elements/checks/gr-hovercard-run_html.ts b/polygerrit-ui/app/elements/checks/gr-hovercard-run_html.ts
deleted file mode 100644
index 49a1416..0000000
--- a/polygerrit-ui/app/elements/checks/gr-hovercard-run_html.ts
+++ /dev/null
@@ -1,232 +0,0 @@
-/**
- * @license
- * 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.
- */
-import {html} from '@polymer/polymer/lib/utils/html-tag';
-
-export const htmlTemplate = html`
- <style include="gr-font-styles">
- /* Workaround for empty style block - see https://github.com/Polymer/tools/issues/408 */
- </style>
- <style include="gr-checks-styles">
- /* Workaround for empty style block - see https://github.com/Polymer/tools/issues/408 */
- </style>
- <style include="gr-hovercard-shared-style">
- #container {
- min-width: 356px;
- max-width: 356px;
- padding: var(--spacing-xl) 0 var(--spacing-m) 0;
- }
- .row {
- display: flex;
- margin-top: var(--spacing-s);
- }
- .attempts.row {
- flex-wrap: wrap;
- }
- .chipRow {
- display: flex;
- margin-top: var(--spacing-s);
- }
- .chip {
- background: var(--gray-background);
- color: var(--gray-foreground);
- border-radius: 20px;
- padding: var(--spacing-xs) var(--spacing-m) var(--spacing-xs)
- var(--spacing-s);
- }
- .title {
- color: var(--deemphasized-text-color);
- margin-right: var(--spacing-m);
- }
- div.section {
- margin: 0 var(--spacing-xl) var(--spacing-m) var(--spacing-xl);
- display: flex;
- }
- div.sectionIcon {
- flex: 0 0 30px;
- }
- div.chip iron-icon {
- width: 16px;
- height: 16px;
- /* Positioning of a 16px icon in the middle of a 20px line. */
- position: relative;
- top: 2px;
- }
- div.sectionIcon iron-icon {
- position: relative;
- top: 2px;
- width: 20px;
- height: 20px;
- }
- div.sectionIcon iron-icon.small {
- position: relative;
- top: 6px;
- width: 16px;
- height: 16px;
- }
- div.sectionContent iron-icon.link {
- color: var(--link-color);
- }
- div.sectionContent .attemptIcon iron-icon,
- div.sectionContent iron-icon.small {
- width: 16px;
- height: 16px;
- margin-right: var(--spacing-s);
- /* Positioning of a 16px icon in the middle of a 20px line. */
- position: relative;
- top: 2px;
- }
- div.sectionContent .attemptIcon iron-icon {
- margin-right: 0;
- }
- .attemptIcon,
- .attemptNumber {
- margin-right: var(--spacing-s);
- color: var(--deemphasized-text-color);
- text-align: center;
- width: 24px;
- font-size: var(--font-size-small);
- }
- div.action {
- border-top: 1px solid var(--border-color);
- margin-top: var(--spacing-m);
- padding: var(--spacing-m) var(--spacing-xl) 0;
- }
- </style>
- <div id="container" role="tooltip" tabindex="-1">
- <div class="section">
- <div hidden$="[[hideChip(run)]]" class="chipRow">
- <div class="chip">
- <iron-icon icon="gr-icons:[[computeChipIcon(run)]]"></iron-icon>
- <span>[[run.status]]</span>
- </div>
- </div>
- </div>
- <div class="section">
- <div class="sectionIcon" hidden$="[[hideHeaderSectionIcon(run)]]">
- <iron-icon
- class$="[[computeIcon(run)]]"
- icon="gr-icons:[[computeIcon(run)]]"
- ></iron-icon>
- </div>
- <div class="sectionContent">
- <h3 class="name heading-3">
- <span>[[run.checkName]]</span>
- </h3>
- </div>
- </div>
- <div class="section" hidden$="[[hideStatusSection(run)]]">
- <div class="sectionIcon">
- <iron-icon class="small" icon="gr-icons:info-outline"></iron-icon>
- </div>
- <div class="sectionContent">
- <div hidden$="[[!run.statusLink]]" class="row">
- <div class="title">Status</div>
- <div>
- <a href="[[_convertUndefined(run.statusLink)]]" target="_blank"
- ><iron-icon
- aria-label="external link to check status"
- class="small link"
- icon="gr-icons:launch"
- ></iron-icon
- >[[computeHostName(run.statusLink)]]
- </a>
- </div>
- </div>
- <div hidden$="[[!run.statusDescription]]" class="row">
- <div class="title">Message</div>
- <div>[[run.statusDescription]]</div>
- </div>
- </div>
- </div>
- <div class="section" hidden$="[[hideAttempts(run)]]">
- <div class="sectionIcon">
- <iron-icon class="small" icon="gr-icons:arrow-forward"></iron-icon>
- </div>
- <div class="sectionContent">
- <div hidden$="[[hideAttempts(run)]]" class="attempts row">
- <div class="title">Attempt</div>
- <template is="dom-repeat" items="[[computeAttempts(run)]]">
- <div>
- <div class="attemptIcon">
- <iron-icon
- class$="[[item.icon]]"
- icon="gr-icons:[[item.icon]]"
- ></iron-icon>
- </div>
- <div class="attemptNumber">[[computeAttempt(item.attempt)]]</div>
- </div>
- </template>
- </div>
- </div>
- </div>
- <div class="section" hidden$="[[hideTimestampSection(run)]]">
- <div class="sectionIcon">
- <iron-icon class="small" icon="gr-icons:schedule"></iron-icon>
- </div>
- <div class="sectionContent">
- <div hidden$="[[hideScheduled(run)]]" class="row">
- <div class="title">Scheduled</div>
- <div>[[computeDuration(run.scheduledTimestamp)]]</div>
- </div>
- <div hidden$="[[!run.startedTimestamp]]" class="row">
- <div class="title">Started</div>
- <div>[[computeDuration(run.startedTimestamp)]]</div>
- </div>
- <div hidden$="[[!run.finishedTimestamp]]" class="row">
- <div class="title">Ended</div>
- <div>[[computeDuration(run.finishedTimestamp)]]</div>
- </div>
- <div hidden$="[[hideCompletion(run)]]" class="row">
- <div class="title">Completion</div>
- <div>[[computeCompletionDuration(run)]]</div>
- </div>
- </div>
- </div>
- <div class="section" hidden$="[[hideDescriptionSection(run)]]">
- <div class="sectionIcon">
- <iron-icon class="small" icon="gr-icons:link"></iron-icon>
- </div>
- <div class="sectionContent">
- <div hidden$="[[!run.checkDescription]]" class="row">
- <div class="title">Description</div>
- <div>[[run.checkDescription]]</div>
- </div>
- <div hidden$="[[!run.checkLink]]" class="row">
- <div class="title">Documentation</div>
- <div>
- <a href="[[_convertUndefined(run.checkLink)]]" target="_blank"
- ><iron-icon
- aria-label="external link to check documentation"
- class="small link"
- icon="gr-icons:launch"
- ></iron-icon
- >[[computeHostName(run.checkLink)]]
- </a>
- </div>
- </div>
- </div>
- </div>
- <template is="dom-repeat" items="[[computeActions(run)]]">
- <div class="action">
- <gr-checks-action
- event-target="[[_target]]"
- action="[[item]]"
- ></gr-checks-action>
- </div>
- </template>
- </div>
-`;
diff --git a/polygerrit-ui/app/elements/checks/gr-hovercard-run_test.ts b/polygerrit-ui/app/elements/checks/gr-hovercard-run_test.ts
index 67781f5..352219a 100644
--- a/polygerrit-ui/app/elements/checks/gr-hovercard-run_test.ts
+++ b/polygerrit-ui/app/elements/checks/gr-hovercard-run_test.ts
@@ -33,7 +33,7 @@
});
teardown(() => {
- element.hide();
+ element.hide(new MouseEvent('click'));
});
test('hovercard is shown', () => {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts
index 4641897..b1bad1c 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts
@@ -56,6 +56,7 @@
PatchRange,
PatchSetNum,
RepoName,
+ UrlEncodedCommentId,
} from '../../../types/common';
import {
DiffInfo,
@@ -730,11 +731,24 @@
}
_threadsChanged(threads: CommentThread[]) {
- const threadEls = new Set<Object>();
+ const threadEls = new Set<GrCommentThread>();
+ const rootIdToThreadEl = new Map<UrlEncodedCommentId, GrCommentThread>();
+ for (const threadEl of this.getThreadEls()) {
+ if (threadEl.rootId) {
+ rootIdToThreadEl.set(threadEl.rootId, threadEl);
+ }
+ }
for (const thread of threads) {
- const threadEl = this._createThreadElement(thread);
- this._attachThreadElement(threadEl);
- threadEls.add(threadEl);
+ const existingThreadEl =
+ thread.rootId && rootIdToThreadEl.get(thread.rootId);
+ if (existingThreadEl) {
+ this._updateThreadElement(existingThreadEl, thread);
+ threadEls.add(existingThreadEl);
+ } else {
+ const threadEl = this._createThreadElement(thread);
+ this._attachThreadElement(threadEl);
+ threadEls.add(threadEl);
+ }
}
// Remove all threads that are no longer existing.
for (const threadEl of this.getThreadEls()) {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.js b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.js
index ed3ffe0..8901636 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.js
@@ -1145,11 +1145,19 @@
.queryDistributedElements('gr-comment-thread');
assert.equal(threads.length, 1);
-
element.threads= [...element.threads, thread];
threads = dom(element.$.diff)
.queryDistributedElements('gr-comment-thread');
+ // Threads have same rootId so element is reused
+ assert.equal(threads.length, 1);
+
+ const newThread = {...thread};
+ newThread.rootId = 'differentRootId';
+ element.threads= [...element.threads, newThread];
+ threads = dom(element.$.diff)
+ .queryDistributedElements('gr-comment-thread');
+ // New thread has a different rootId
assert.equal(threads.length, 2);
});
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-image-viewer/gr-image-viewer.ts b/polygerrit-ui/app/elements/diff/gr-diff-image-viewer/gr-image-viewer.ts
index 3a05e7d..32f5f39 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-image-viewer/gr-image-viewer.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-image-viewer/gr-image-viewer.ts
@@ -422,25 +422,23 @@
/>
`;
- const sourceImageWithHighlight = this.diffHighlightSrc
- ? html`
- <div id="source-plus-highlight-container">
- ${sourceImage}
- <img
- id="highlight-image"
- style="${styleMap({
- opacity: this.showHighlight ? '1' : '0',
- // When the highlight layer is not being shown, saving the image or
- // opening it in a new tab from the context menu, e.g. for external
- // comparison, should give back the source image, not the highlight
- // layer.
- 'pointer-events': this.showHighlight ? 'auto' : 'none',
- })}"
- src="${this.diffHighlightSrc}"
- />
- </div>
- `
- : '';
+ const sourceImageWithHighlight = html`
+ <div id="source-plus-highlight-container">
+ ${sourceImage}
+ <img
+ id="highlight-image"
+ style="${styleMap({
+ opacity: this.showHighlight ? '1' : '0',
+ // When the highlight layer is not being shown, saving the image or
+ // opening it in a new tab from the context menu, e.g. for external
+ // comparison, should give back the source image, not the highlight
+ // layer.
+ 'pointer-events': this.showHighlight ? 'auto' : 'none',
+ })}"
+ src="${this.diffHighlightSrc}"
+ />
+ </div>
+ `;
const versionExplanation = html`
<div id="version-explanation">
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-account-label/gr-account-label.ts b/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label.ts
index 9897a9f..dabf761 100644
--- a/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label.ts
+++ b/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label.ts
@@ -199,9 +199,9 @@
${!this.hideHovercard
? html`<gr-hovercard-account
for="hovercardTarget"
- .account="${account}"
- .change="${change}"
- ?highlight-attention=${highlightAttention}
+ .account=${account}
+ .change=${change}
+ .highlightAttention=${highlightAttention}
.voteableText=${this.voteableText}
></gr-hovercard-account>`
: ''}
diff --git a/polygerrit-ui/app/elements/shared/gr-hovercard-account/gr-hovercard-account.ts b/polygerrit-ui/app/elements/shared/gr-hovercard-account/gr-hovercard-account.ts
index 06272ed..6a34fbb 100644
--- a/polygerrit-ui/app/elements/shared/gr-hovercard-account/gr-hovercard-account.ts
+++ b/polygerrit-ui/app/elements/shared/gr-hovercard-account/gr-hovercard-account.ts
@@ -16,16 +16,11 @@
*/
import '@polymer/iron-icon/iron-icon';
-import '../../../styles/gr-font-styles';
-import '../../../styles/shared-styles';
import '../gr-avatar/gr-avatar';
import '../gr-button/gr-button';
-import {HovercardBehaviorMixin} from '../gr-hovercard/gr-hovercard-behavior';
-import {PolymerElement} from '@polymer/polymer/polymer-element';
-import {htmlTemplate} from './gr-hovercard-account_html';
import {appContext} from '../../../services/app-context';
import {accountKey, isSelf} from '../../../utils/account-util';
-import {customElement, property} from '@polymer/decorators';
+import {customElement, property} from 'lit/decorators';
import {
AccountInfo,
ChangeInfo,
@@ -45,16 +40,15 @@
import {CURRENT} from '../../../utils/patch-set-util';
import {isInvolved, isRemovableReviewer} from '../../../utils/change-util';
import {assertIsDefined} from '../../../utils/common-util';
+import {fontStyles} from '../../../styles/gr-font-styles';
+import {css, html, LitElement} from 'lit';
+import {HovercardMixin} from '../../../mixins/hovercard-mixin/hovercard-mixin';
// This avoids JSC_DYNAMIC_EXTENDS_WITHOUT_JSDOC closure compiler error.
-const base = HovercardBehaviorMixin(PolymerElement);
+const base = HovercardMixin(LitElement);
@customElement('gr-hovercard-account')
export class GrHovercardAccount extends base {
- static get template() {
- return htmlTemplate;
- }
-
@property({type: Object})
account!: AccountInfo;
@@ -106,9 +100,214 @@
});
}
- _computeText(account?: AccountInfo, selfAccount?: AccountInfo) {
- if (!account || !selfAccount) return '';
- return isSelf(account, selfAccount) ? 'Your' : 'Their';
+ static override get styles() {
+ return [
+ fontStyles,
+ base.styles || [],
+ css`
+ .top,
+ .attention,
+ .status,
+ .voteable {
+ padding: var(--spacing-s) var(--spacing-l);
+ }
+ .top {
+ display: flex;
+ padding-top: var(--spacing-xl);
+ min-width: 300px;
+ }
+ gr-avatar {
+ height: 48px;
+ width: 48px;
+ margin-right: var(--spacing-l);
+ }
+ .title,
+ .email {
+ color: var(--deemphasized-text-color);
+ }
+ .action {
+ border-top: 1px solid var(--border-color);
+ padding: var(--spacing-s) var(--spacing-l);
+ --gr-button-padding: var(--spacing-s) var(--spacing-m);
+ }
+ .attention {
+ background-color: var(--emphasis-color);
+ }
+ .attention a {
+ text-decoration: none;
+ }
+ iron-icon {
+ vertical-align: top;
+ }
+ .status iron-icon {
+ width: 14px;
+ height: 14px;
+ position: relative;
+ top: 2px;
+ }
+ iron-icon.attentionIcon {
+ width: 14px;
+ height: 14px;
+ position: relative;
+ top: 3px;
+ }
+ .reason {
+ padding-top: var(--spacing-s);
+ }
+ `,
+ ];
+ }
+
+ override render() {
+ return html`
+ <div id="container" role="tooltip" tabindex="-1">
+ ${this.renderContent()}
+ </div>
+ `;
+ }
+
+ private renderContent() {
+ if (!this._isShowing) return;
+ return html`
+ <div class="top">
+ <div class="avatar">
+ <gr-avautar .account=${this.account} imageSize="56"></gr-avatar>
+ </div>
+ <div class="account">
+ <h3 class="name heading-3">${this.account.name}</h3>
+ <div class="email">${this.account.email}</div>
+ </div>
+ </div>
+ ${this.renderAccountStatus()}
+ ${
+ this.voteableText
+ ? html`
+ <div class="voteable">
+ <span class="title">Voteable:</span>
+ <span class="value">${this.voteableText}</span>
+ </div>
+ `
+ : ''
+ }
+ ${this.renderNeedsAttention()} ${this.renderAddToAttention()}
+ ${this.renderRemoveFromAttention()} ${this.renderReviewerOrCcActions()}
+ `;
+ }
+
+ private renderReviewerOrCcActions() {
+ if (!this._selfAccount || !isRemovableReviewer(this.change, this.account))
+ return;
+ return html`
+ <div class="action">
+ <gr-button
+ class="removeReviewerOrCC"
+ link=""
+ no-uppercase
+ @click="${this.handleRemoveReviewerOrCC}"
+ >
+ Remove ${this.computeReviewerOrCCText()}
+ </gr-button>
+ </div>
+ <div class="action">
+ <gr-button
+ class="changeReviewerOrCC"
+ link=""
+ no-uppercase
+ @click="${this.handleChangeReviewerOrCCStatus}"
+ >
+ ${this.computeChangeReviewerOrCCText()}
+ </gr-button>
+ </div>
+ `;
+ }
+
+ private renderAccountStatus() {
+ if (!this.account.status) return;
+ return html`
+ <div class="status">
+ <span class="title">
+ <iron-icon icon="gr-icons:calendar"></iron-icon>
+ Status:
+ </span>
+ <span class="value">${this.account.status}</span>
+ </div>
+ `;
+ }
+
+ private renderNeedsAttention() {
+ if (!(this.isAttentionEnabled && this.hasUserAttention)) return;
+ const lastUpdate = getLastUpdate(this.account, this.change);
+ return html`
+ <div class="attention">
+ <div>
+ <iron-icon
+ class="attentionIcon"
+ icon="gr-icons:attention"
+ ></iron-icon>
+ <span> ${this.computePronoun()} turn to take this action. </span>
+ <a
+ href="https://gerrit-review.googlesource.com/Documentation/user-attention-set.html"
+ target="_blank"
+ >
+ <iron-icon
+ icon="gr-icons:help-outline"
+ title="read documentation"
+ ></iron-icon>
+ </a>
+ </div>
+ <div class="reason">
+ <span class="title">Reason:</span>
+ <span class="value">
+ ${getReason(this._config, this.account, this.change)}
+ </span>
+ ${lastUpdate
+ ? html` (<gr-date-formatter
+ withTooltip
+ .dateStr="${lastUpdate}"
+ ></gr-date-formatter
+ >)`
+ : ''}
+ </div>
+ </div>
+ `;
+ }
+
+ private renderAddToAttention() {
+ if (!this.computeShowActionAddToAttentionSet()) return;
+ return html`
+ <div class="action">
+ <gr-button
+ class="addToAttentionSet"
+ link=""
+ no-uppercase
+ @click="${this.handleClickAddToAttentionSet}"
+ >
+ Add to attention set
+ </gr-button>
+ </div>
+ `;
+ }
+
+ private renderRemoveFromAttention() {
+ if (!this.computeShowActionRemoveFromAttentionSet()) return;
+ return html`
+ <div class="action">
+ <gr-button
+ class="removeFromAttentionSet"
+ link=""
+ no-uppercase
+ @click="${this.handleClickRemoveFromAttentionSet}"
+ >
+ Remove from attention set
+ </gr-button>
+ </div>
+ `;
+ }
+
+ // private but used by tests
+ computePronoun() {
+ if (!this.account || !this._selfAccount) return '';
+ return isSelf(this.account, this._selfAccount) ? 'Your' : 'Their';
}
get isAttentionEnabled() {
@@ -123,27 +322,11 @@
return hasAttention(this.account, this.change);
}
- _computeReason(change?: ChangeInfo) {
- return getReason(this._config, this.account, change);
- }
-
- _computeLastUpdate(change?: ChangeInfo) {
- return getLastUpdate(this.account, change);
- }
-
- /** 3rd parameter is just for *triggering* re-computation. */
- _showReviewerOrCCActions(
- account?: AccountInfo,
- change?: ChangeInfo,
- _?: unknown
- ) {
- return !!this._selfAccount && isRemovableReviewer(change, account);
- }
-
- _getReviewerState(account: AccountInfo, change: ChangeInfo) {
+ private getReviewerState() {
if (
- change.reviewers[ReviewerState.REVIEWER]?.some(
- (reviewer: AccountInfo) => reviewer._account_id === account._account_id
+ this.change!.reviewers[ReviewerState.REVIEWER]?.some(
+ (reviewer: AccountInfo) =>
+ reviewer._account_id === this.account._account_id
)
) {
return ReviewerState.REVIEWER;
@@ -151,21 +334,21 @@
return ReviewerState.CC;
}
- _computeReviewerOrCCText(account?: AccountInfo, change?: ChangeInfo) {
- if (!change || !account) return '';
- return this._getReviewerState(account, change) === ReviewerState.REVIEWER
+ private computeReviewerOrCCText() {
+ if (!this.change || !this.account) return '';
+ return this.getReviewerState() === ReviewerState.REVIEWER
? 'Reviewer'
: 'CC';
}
- _computeChangeReviewerOrCCText(account?: AccountInfo, change?: ChangeInfo) {
- if (!change || !account) return '';
- return this._getReviewerState(account, change) === ReviewerState.REVIEWER
+ private computeChangeReviewerOrCCText() {
+ if (!this.change || !this.account) return '';
+ return this.getReviewerState() === ReviewerState.REVIEWER
? 'Move Reviewer to CC'
: 'Move CC to Reviewer';
}
- _handleChangeReviewerOrCCStatus() {
+ private handleChangeReviewerOrCCStatus() {
assertIsDefined(this.change, 'change');
// accountKey() throws an error if _account_id & email is not found, which
// we want to check before showing reloading toast
@@ -178,7 +361,7 @@
{
reviewer: _accountKey,
state:
- this._getReviewerState(this.account, this.change) === ReviewerState.CC
+ this.getReviewerState() === ReviewerState.CC
? ReviewerState.REVIEWER
: ReviewerState.CC,
},
@@ -189,15 +372,14 @@
.then(response => {
if (!response || !response.ok) {
throw new Error(
- 'something went wrong when toggling' +
- this._getReviewerState(this.account, this.change!)
+ 'something went wrong when toggling' + this.getReviewerState()
);
}
this.dispatchEventThroughTarget('reload', {clearPatchset: true});
});
}
- _handleRemoveReviewerOrCC() {
+ private handleRemoveReviewerOrCC() {
if (!this.change || !(this.account?._account_id || this.account?.email))
throw new Error('Missing change or account.');
this.dispatchEventThroughTarget('show-alert', {
@@ -217,45 +399,21 @@
});
}
- /** Parameters are just for *triggering* re-computation. */
- _computeShowLabelNeedsAttention(
- _1: unknown,
- _2: unknown,
- _3: unknown,
- _4: unknown
- ) {
- return this.isAttentionEnabled && this.hasUserAttention;
- }
-
- /** Parameters are just for *triggering* re-computation. */
- _computeShowActionAddToAttentionSet(
- _1: unknown,
- _2: unknown,
- _3: unknown,
- _4: unknown,
- _5: unknown
- ) {
+ private computeShowActionAddToAttentionSet() {
const involvedOrSelf =
isInvolved(this.change, this._selfAccount) ||
isSelf(this.account, this._selfAccount);
return involvedOrSelf && this.isAttentionEnabled && !this.hasUserAttention;
}
- /** Parameters are just for *triggering* re-computation. */
- _computeShowActionRemoveFromAttentionSet(
- _1: unknown,
- _2: unknown,
- _3: unknown,
- _4: unknown,
- _5: unknown
- ) {
+ private computeShowActionRemoveFromAttentionSet() {
const involvedOrSelf =
isInvolved(this.change, this._selfAccount) ||
isSelf(this.account, this._selfAccount);
return involvedOrSelf && this.isAttentionEnabled && this.hasUserAttention;
}
- _handleClickAddToAttentionSet() {
+ private handleClickAddToAttentionSet(e: MouseEvent) {
if (!this.change || !this.account._account_id) return;
this.dispatchEventThroughTarget('show-alert', {
message: 'Saving attention set update ...',
@@ -276,17 +434,17 @@
this.reporting.reportInteraction(
'attention-hovercard-add',
- this._reportingDetails()
+ this.reportingDetails()
);
this.restApiService
.addToAttentionSet(this.change._number, this.account._account_id, reason)
.then(() => {
this.dispatchEventThroughTarget('hide-alert');
});
- this.hide();
+ this.hide(e);
}
- _handleClickRemoveFromAttentionSet() {
+ private handleClickRemoveFromAttentionSet(e: MouseEvent) {
if (!this.change || !this.account._account_id) return;
this.dispatchEventThroughTarget('show-alert', {
message: 'Saving attention set update ...',
@@ -303,7 +461,7 @@
this.reporting.reportInteraction(
'attention-hovercard-remove',
- this._reportingDetails()
+ this.reportingDetails()
);
this.restApiService
.removeFromAttentionSet(
@@ -314,10 +472,10 @@
.then(() => {
this.dispatchEventThroughTarget('hide-alert');
});
- this.hide();
+ this.hide(e);
}
- _reportingDetails() {
+ private reportingDetails() {
const targetId = this.account._account_id;
const ownerId =
(this.change && this.change.owner && this.change.owner._account_id) || -1;
diff --git a/polygerrit-ui/app/elements/shared/gr-hovercard-account/gr-hovercard-account_html.ts b/polygerrit-ui/app/elements/shared/gr-hovercard-account/gr-hovercard-account_html.ts
deleted file mode 100644
index adca888..0000000
--- a/polygerrit-ui/app/elements/shared/gr-hovercard-account/gr-hovercard-account_html.ts
+++ /dev/null
@@ -1,194 +0,0 @@
-/**
- * @license
- * Copyright (C) 2020 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-import '../gr-hovercard/gr-hovercard-shared-style';
-import {html} from '@polymer/polymer/lib/utils/html-tag';
-
-export const htmlTemplate = html`
- <style include="gr-font-styles">
- /* Workaround for empty style block - see https://github.com/Polymer/tools/issues/408 */
- </style>
- <style include="gr-hovercard-shared-style">
- .top,
- .attention,
- .status,
- .voteable {
- padding: var(--spacing-s) var(--spacing-l);
- }
- .top {
- display: flex;
- padding-top: var(--spacing-xl);
- min-width: 300px;
- }
- gr-avatar {
- height: 48px;
- width: 48px;
- margin-right: var(--spacing-l);
- }
- .title,
- .email {
- color: var(--deemphasized-text-color);
- }
- .action {
- border-top: 1px solid var(--border-color);
- padding: var(--spacing-s) var(--spacing-l);
- --gr-button-padding: var(--spacing-s) var(--spacing-m);
- }
- .attention {
- background-color: var(--emphasis-color);
- }
- .attention a {
- text-decoration: none;
- }
- iron-icon {
- vertical-align: top;
- }
- .status iron-icon {
- width: 14px;
- height: 14px;
- position: relative;
- top: 2px;
- }
- iron-icon.attentionIcon {
- width: 14px;
- height: 14px;
- position: relative;
- top: 3px;
- }
- .reason {
- padding-top: var(--spacing-s);
- }
- </style>
- <div id="container" role="tooltip" tabindex="-1">
- <template is="dom-if" if="[[_isShowing]]">
- <div class="top">
- <div class="avatar">
- <gr-avatar account="[[account]]" imageSize="56"></gr-avatar>
- </div>
- <div class="account">
- <h3 class="name heading-3">[[account.name]]</h3>
- <div class="email">[[account.email]]</div>
- </div>
- </div>
- <template is="dom-if" if="[[account.status]]">
- <div class="status">
- <span class="title">
- <iron-icon icon="gr-icons:calendar"></iron-icon>
- Status:
- </span>
- <span class="value">[[account.status]]</span>
- </div>
- </template>
- <template is="dom-if" if="[[voteableText]]">
- <div class="voteable">
- <span class="title">Voteable:</span>
- <span class="value">[[voteableText]]</span>
- </div>
- </template>
- <template
- is="dom-if"
- if="[[_computeShowLabelNeedsAttention(_config, highlightAttention, account, change)]]"
- >
- <div class="attention">
- <div>
- <iron-icon
- class="attentionIcon"
- icon="gr-icons:attention"
- ></iron-icon>
- <span>
- [[_computeText(account, _selfAccount)]] turn to take action.
- </span>
- <a
- href="https://gerrit-review.googlesource.com/Documentation/user-attention-set.html"
- target="_blank"
- >
- <iron-icon
- icon="gr-icons:help-outline"
- title="read documentation"
- ></iron-icon>
- </a>
- </div>
- <div class="reason">
- <span class="title">Reason:</span>
- <span class="value">[[_computeReason(change)]]</span>
- <template is="dom-if" if="[[_computeLastUpdate(change)]]">
- (<gr-date-formatter
- withTooltip
- date-str="[[_computeLastUpdate(change)]]"
- ></gr-date-formatter
- >)
- </template>
- </div>
- </div>
- </template>
- <template
- is="dom-if"
- if="[[_computeShowActionAddToAttentionSet(_config, highlightAttention, account, change, _selfAccount)]]"
- >
- <div class="action">
- <gr-button
- class="addToAttentionSet"
- link=""
- no-uppercase=""
- on-click="_handleClickAddToAttentionSet"
- >
- Add to attention set
- </gr-button>
- </div>
- </template>
- <template
- is="dom-if"
- if="[[_computeShowActionRemoveFromAttentionSet(_config, highlightAttention, account, change, _selfAccount)]]"
- >
- <div class="action">
- <gr-button
- class="removeFromAttentionSet"
- link=""
- no-uppercase=""
- on-click="_handleClickRemoveFromAttentionSet"
- >
- Remove from attention set
- </gr-button>
- </div>
- </template>
- <template
- is="dom-if"
- if="[[_showReviewerOrCCActions(account, change, _selfAccount)]]"
- >
- <div class="action">
- <gr-button
- class="removeReviewerOrCC"
- link=""
- no-uppercase=""
- on-click="_handleRemoveReviewerOrCC"
- >
- Remove [[_computeReviewerOrCCText(account, change)]]
- </gr-button>
- </div>
- <div class="action">
- <gr-button
- class="changeReviewerOrCC"
- link=""
- no-uppercase=""
- on-click="_handleChangeReviewerOrCCStatus"
- >
- [[_computeChangeReviewerOrCCText(account, change)]]
- </gr-button>
- </div>
- </template>
- </template>
- </div>
-`;
diff --git a/polygerrit-ui/app/elements/shared/gr-hovercard-account/gr-hovercard-account_test.js b/polygerrit-ui/app/elements/shared/gr-hovercard-account/gr-hovercard-account_test.js
index 82f64d0..5530d7c 100644
--- a/polygerrit-ui/app/elements/shared/gr-hovercard-account/gr-hovercard-account_test.js
+++ b/polygerrit-ui/app/elements/shared/gr-hovercard-account/gr-hovercard-account_test.js
@@ -19,7 +19,7 @@
import './gr-hovercard-account.js';
import {html} from '@polymer/polymer/lib/utils/html-tag.js';
import {ReviewerState} from '../../../constants/constants.js';
-import {stubRestApi} from '../../../test/test-utils.js';
+import {mockPromise, stubRestApi} from '../../../test/test-utils.js';
const basicFixture = fixtureFromTemplate(html`
<gr-hovercard-account class="hovered"></gr-hovercard-account>
@@ -57,33 +57,21 @@
'Kermit The Frog');
});
- test('_computeLastUpdate', () => {
- const last_update = '2019-07-17 19:39:02.000000000';
- const change = {
- attention_set: {
- 31415926535: {
- last_update,
- },
- },
- };
- assert.equal(element._computeLastUpdate(change), last_update);
- });
-
- test('_computeText', () => {
- let account = {_account_id: '1'};
- const selfAccount = {_account_id: '1'};
- assert.equal(element._computeText(account, selfAccount), 'Your');
- account = {_account_id: '2'};
- assert.equal(element._computeText(account, selfAccount), 'Their');
+ test('computePronoun', () => {
+ element.account = {_account_id: '1'};
+ element._selfAccount = {_account_id: '1'};
+ assert.equal(element.computePronoun(), 'Your');
+ element.account = {_account_id: '2'};
+ assert.equal(element.computePronoun(), 'Their');
});
test('account status is not shown if the property is not set', () => {
assert.isNull(element.shadowRoot.querySelector('.status'));
});
- test('account status is displayed', () => {
+ test('account status is displayed', async () => {
element.account = {status: 'OOO', ...ACCOUNT};
- flush();
+ await element.updateComplete;
assert.equal(element.shadowRoot.querySelector('.status .value').innerText,
'OOO');
});
@@ -92,9 +80,9 @@
assert.isNull(element.shadowRoot.querySelector('.voteable'));
});
- test('voteable div is displayed', () => {
+ test('voteable div is displayed', async () => {
element.voteableText = 'CodeReview: +2';
- flush();
+ await element.updateComplete;
assert.equal(element.shadowRoot.querySelector('.voteable .value').innerText,
element.voteableText);
});
@@ -106,15 +94,15 @@
[ReviewerState.REVIEWER]: [ACCOUNT],
},
};
+ await element.updateComplete;
stubRestApi('removeChangeReviewer').returns(Promise.resolve({ok: true}));
const reloadListener = sinon.spy();
element._target.addEventListener('reload', reloadListener);
- await flush();
const button = element.shadowRoot.querySelector('.removeReviewerOrCC');
assert.isOk(button);
assert.equal(button.innerText, 'Remove Reviewer');
MockInteractions.tap(button);
- await flush();
+ await element.updateComplete;
assert.isTrue(reloadListener.called);
});
@@ -125,6 +113,7 @@
[ReviewerState.REVIEWER]: [ACCOUNT],
},
};
+ await element.updateComplete;
const saveReviewStub = stubRestApi(
'saveChangeReview').returns(
Promise.resolve({ok: true}));
@@ -132,14 +121,12 @@
const reloadListener = sinon.spy();
element._target.addEventListener('reload', reloadListener);
- await flush();
const button = element.shadowRoot.querySelector('.changeReviewerOrCC');
assert.isOk(button);
assert.equal(button.innerText, 'Move Reviewer to CC');
MockInteractions.tap(button);
- await flush();
-
+ await element.updateComplete;
assert.isTrue(saveReviewStub.called);
assert.isTrue(reloadListener.called);
});
@@ -151,20 +138,19 @@
[ReviewerState.REVIEWER]: [],
},
};
+ await element.updateComplete;
const saveReviewStub = stubRestApi(
'saveChangeReview').returns(Promise.resolve({ok: true}));
stubRestApi('removeChangeReviewer').returns(Promise.resolve({ok: true}));
const reloadListener = sinon.spy();
element._target.addEventListener('reload', reloadListener);
- await flush();
const button = element.shadowRoot.querySelector('.changeReviewerOrCC');
assert.isOk(button);
assert.equal(button.innerText, 'Move CC to Reviewer');
MockInteractions.tap(button);
- await flush();
-
+ await element.updateComplete;
assert.isTrue(saveReviewStub.called);
assert.isTrue(reloadListener.called);
});
@@ -176,31 +162,26 @@
[ReviewerState.REVIEWER]: [],
},
};
+ await element.updateComplete;
stubRestApi('removeChangeReviewer').returns(Promise.resolve({ok: true}));
const reloadListener = sinon.spy();
element._target.addEventListener('reload', reloadListener);
- await flush();
const button = element.shadowRoot.querySelector('.removeReviewerOrCC');
assert.equal(button.innerText, 'Remove CC');
assert.isOk(button);
MockInteractions.tap(button);
-
- await flush();
-
+ await element.updateComplete;
assert.isTrue(reloadListener.called);
});
test('add to attention set', async () => {
- let apiResolve;
- const apiPromise = new Promise(r => {
- apiResolve = r;
- });
+ const apiPromise = mockPromise();
const apiSpy = stubRestApi('addToAttentionSet').returns(apiPromise);
element.highlightAttention = true;
element._target = document.createElement('div');
- flush();
+ await element.updateComplete;
const showAlertListener = sinon.spy();
const hideAlertListener = sinon.spy();
const updatedListener = sinon.spy();
@@ -224,8 +205,8 @@
assert.isTrue(updatedListener.called, 'updatedListener was called');
assert.isFalse(element._isShowing, 'hovercard is hidden');
- apiResolve({});
- await flush();
+ apiPromise.resolve({});
+ await element.updateComplete;
assert.isTrue(apiSpy.calledOnce);
assert.equal(apiSpy.lastCall.args[2],
`Added by <GERRIT_ACCOUNT_${ACCOUNT._account_id}>`
@@ -234,10 +215,7 @@
});
test('remove from attention set', async () => {
- let apiResolve;
- const apiPromise = new Promise(r => {
- apiResolve = r;
- });
+ const apiPromise = mockPromise();
const apiSpy = stubRestApi('removeFromAttentionSet').returns(apiPromise);
element.highlightAttention = true;
element.change = {
@@ -246,7 +224,7 @@
owner: {...ACCOUNT},
};
element._target = document.createElement('div');
- flush();
+ await element.updateComplete;
const showAlertListener = sinon.spy();
const hideAlertListener = sinon.spy();
const updatedListener = sinon.spy();
@@ -264,8 +242,8 @@
assert.isTrue(updatedListener.called, 'updatedListener was called');
assert.isFalse(element._isShowing, 'hovercard is hidden');
- apiResolve({});
- await flush();
+ apiPromise.resolve({});
+ await element.updateComplete;
assert.isTrue(apiSpy.calledOnce);
assert.equal(apiSpy.lastCall.args[2],
diff --git a/polygerrit-ui/app/elements/shared/gr-hovercard/gr-hovercard-shared-style.ts b/polygerrit-ui/app/elements/shared/gr-hovercard/gr-hovercard-shared-style.ts
deleted file mode 100644
index aa92654..0000000
--- a/polygerrit-ui/app/elements/shared/gr-hovercard/gr-hovercard-shared-style.ts
+++ /dev/null
@@ -1,51 +0,0 @@
-/**
- * @license
- * Copyright (C) 2020 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-// Mark the file as a module. Otherwise typescript assumes this is a script
-// and $_documentContainer is a global variable.
-// See: https://www.typescriptlang.org/docs/handbook/modules.html
-export {};
-
-/** The shared styles for all hover cards. */
-const GrHoverCardSharedStyle = document.createElement('dom-module');
-GrHoverCardSharedStyle.innerHTML = `<template>
- <style include="shared-styles">
- :host {
- position: absolute;
- display: none;
- z-index: 200;
- max-width: 600px;
- outline: none;
- }
- :host(.hovered) {
- display: block;
- }
- :host(.hide) {
- visibility: hidden;
- }
- /* You have to use a <div class="container"> in your hovercard in order
- to pick up this consistent styling. */
- #container {
- background: var(--dialog-background-color);
- border: 1px solid var(--border-color);
- border-radius: var(--border-radius);
- box-shadow: var(--elevation-level-5);
- }
- </style>
- </template>`;
-
-GrHoverCardSharedStyle.register('gr-hovercard-shared-style');
diff --git a/polygerrit-ui/app/elements/shared/gr-hovercard/gr-hovercard.ts b/polygerrit-ui/app/elements/shared/gr-hovercard/gr-hovercard.ts
index acc5e15..bf35c06 100644
--- a/polygerrit-ui/app/elements/shared/gr-hovercard/gr-hovercard.ts
+++ b/polygerrit-ui/app/elements/shared/gr-hovercard/gr-hovercard.ts
@@ -15,20 +15,32 @@
* limitations under the License.
*/
-import '../../../styles/shared-styles';
-import {PolymerElement} from '@polymer/polymer/polymer-element';
-import {htmlTemplate} from './gr-hovercard_html';
-import {HovercardBehaviorMixin} from './gr-hovercard-behavior';
-import './gr-hovercard-shared-style';
-import {customElement} from '@polymer/decorators';
+import {customElement} from 'lit/decorators';
+import {HovercardMixin} from '../../../mixins/hovercard-mixin/hovercard-mixin';
+import {css, html, LitElement} from 'lit';
// This avoids JSC_DYNAMIC_EXTENDS_WITHOUT_JSDOC closure compiler error.
-const base = HovercardBehaviorMixin(PolymerElement);
+const base = HovercardMixin(LitElement);
@customElement('gr-hovercard')
export class GrHovercard extends base {
- static get template() {
- return htmlTemplate;
+ static override get styles() {
+ return [
+ base.styles ?? [],
+ css`
+ #container {
+ padding: var(--spacing-l);
+ }
+ `,
+ ];
+ }
+
+ override render() {
+ return html`
+ <div id="container" role="tooltip" tabindex="-1">
+ <slot></slot>
+ </div>
+ `;
}
}
diff --git a/polygerrit-ui/app/elements/shared/gr-hovercard/gr-hovercard_html.ts b/polygerrit-ui/app/elements/shared/gr-hovercard/gr-hovercard_html.ts
deleted file mode 100644
index 830cbd878..0000000
--- a/polygerrit-ui/app/elements/shared/gr-hovercard/gr-hovercard_html.ts
+++ /dev/null
@@ -1,28 +0,0 @@
-/**
- * @license
- * Copyright (C) 2020 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-import {html} from '@polymer/polymer/lib/utils/html-tag';
-
-export const htmlTemplate = html`
- <style include="gr-hovercard-shared-style">
- #container {
- padding: var(--spacing-l);
- }
- </style>
- <div id="container" role="tooltip" tabindex="-1">
- <slot></slot>
- </div>
-`;
diff --git a/polygerrit-ui/app/elements/shared/gr-hovercard/gr-hovercard_test.js b/polygerrit-ui/app/elements/shared/gr-hovercard/gr-hovercard_test.js
deleted file mode 100644
index d5e0061..0000000
--- a/polygerrit-ui/app/elements/shared/gr-hovercard/gr-hovercard_test.js
+++ /dev/null
@@ -1,168 +0,0 @@
-/**
- * @license
- * Copyright (C) 2018 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-import '../../../test/common-test-setup-karma.js';
-import './gr-hovercard.js';
-import {html} from '@polymer/polymer/lib/utils/html-tag.js';
-
-const basicFixture = fixtureFromTemplate(html`
-<gr-hovercard for="foo" id="bar"></gr-hovercard>
-`);
-
-suite('gr-hovercard tests', () => {
- let element;
-
- let button;
- let testResolve;
- let testPromise;
-
- setup(() => {
- testResolve = undefined;
- testPromise = new Promise(r => testResolve = r);
- button = document.createElement('button');
- button.innerHTML = 'Hello';
- button.setAttribute('id', 'foo');
- document.body.appendChild(button);
-
- element = basicFixture.instantiate();
- });
-
- teardown(() => {
- element.hide({});
- button.remove();
- });
-
- test('updatePosition', () => {
- // Test that the correct style properties have at least been set.
- element.position = 'bottom';
- element.updatePosition();
- assert.typeOf(element.style.getPropertyValue('left'), 'string');
- assert.typeOf(element.style.getPropertyValue('top'), 'string');
- assert.typeOf(element.style.getPropertyValue('paddingTop'), 'string');
- assert.typeOf(element.style.getPropertyValue('marginTop'), 'string');
-
- const parentRect = document.documentElement.getBoundingClientRect();
- const targetRect = element._target.getBoundingClientRect();
- const thisRect = element.getBoundingClientRect();
-
- const targetLeft = targetRect.left - parentRect.left;
- const targetTop = targetRect.top - parentRect.top;
-
- const pixelCompare = pixel =>
- Math.round(parseInt(pixel.substring(0, pixel.length - 1)), 10);
-
- assert.equal(
- pixelCompare(element.style.left),
- pixelCompare(
- (targetLeft + (targetRect.width - thisRect.width) / 2) + 'px'));
- assert.equal(
- pixelCompare(element.style.top),
- pixelCompare(
- (targetTop + targetRect.height + element.offset) + 'px'));
- });
-
- test('hide', () => {
- element.hide({});
- const style = getComputedStyle(element);
- assert.isFalse(element._isShowing);
- assert.isFalse(element.classList.contains('hovered'));
- assert.equal(style.display, 'none');
- assert.notEqual(element.container, element.parentNode);
- });
-
- test('show', async () => {
- await element.show({});
- const style = getComputedStyle(element);
- assert.isTrue(element._isShowing);
- assert.isTrue(element.classList.contains('hovered'));
- assert.equal(style.opacity, '1');
- assert.equal(style.visibility, 'visible');
- });
-
- test('debounceShow does not show immediately', async () => {
- element.debounceShowBy(100);
- setTimeout(testResolve, 0);
- await testPromise;
- assert.isFalse(element._isShowing);
- });
-
- test('debounceShow shows after delay', async () => {
- element.debounceShowBy(1);
- setTimeout(testResolve, 10);
- await testPromise;
- assert.isTrue(element._isShowing);
- });
-
- test('card is scheduled to show on enter and hides on leave', async () => {
- const button = document.querySelector('button');
- let enterResolve = undefined;
- const enterPromise = new Promise(r => enterResolve = r);
- button.addEventListener('mouseenter', enterResolve);
- let leaveResolve = undefined;
- const leavePromise = new Promise(r => leaveResolve = r);
- button.addEventListener('mouseleave', leaveResolve);
-
- assert.isFalse(element._isShowing);
- button.dispatchEvent(new CustomEvent('mouseenter'));
-
- await enterPromise;
- await flush();
- assert.isTrue(element.isScheduledToShow);
- element.showTask.flush();
- assert.isTrue(element._isShowing);
- assert.isFalse(element.isScheduledToShow);
-
- button.dispatchEvent(new CustomEvent('mouseleave'));
-
- await leavePromise;
- assert.isTrue(element.isScheduledToHide);
- assert.isTrue(element._isShowing);
- element.hideTask.flush();
- assert.isFalse(element.isScheduledToShow);
- assert.isFalse(element._isShowing);
-
- button.removeEventListener('mouseenter', enterResolve);
- button.removeEventListener('mouseleave', leaveResolve);
- });
-
- test('card should disappear on click', async () => {
- const button = document.querySelector('button');
- let enterResolve = undefined;
- const enterPromise = new Promise(r => enterResolve = r);
- button.addEventListener('mouseenter', enterResolve);
- let clickResolve = undefined;
- const clickPromise = new Promise(r => clickResolve = r);
- button.addEventListener('click', clickResolve);
-
- assert.isFalse(element._isShowing);
-
- button.dispatchEvent(new CustomEvent('mouseenter'));
-
- await enterPromise;
- await flush();
- assert.isTrue(element.isScheduledToShow);
- MockInteractions.tap(button);
-
- await clickPromise;
- assert.isFalse(element.isScheduledToShow);
- assert.isFalse(element._isShowing);
-
- button.removeEventListener('mouseenter', enterResolve);
- button.removeEventListener('click', clickResolve);
- });
-});
-
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-overlay/gr-overlay.ts b/polygerrit-ui/app/elements/shared/gr-overlay/gr-overlay.ts
index 1b84089..3d6b5ed 100644
--- a/polygerrit-ui/app/elements/shared/gr-overlay/gr-overlay.ts
+++ b/polygerrit-ui/app/elements/shared/gr-overlay/gr-overlay.ts
@@ -22,7 +22,7 @@
import {IronOverlayBehavior} from '@polymer/iron-overlay-behavior/iron-overlay-behavior';
import {findActiveElement} from '../../../utils/dom-util';
import {fireEvent} from '../../../utils/event-util';
-import {getHovercardContainer} from '../gr-hovercard/gr-hovercard-behavior';
+import {getHovercardContainer} from '../../../mixins/hovercard-mixin/hovercard-mixin';
const AWAIT_MAX_ITERS = 10;
const AWAIT_STEP = 5;
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/elements/shared/gr-hovercard/gr-hovercard-behavior.ts b/polygerrit-ui/app/mixins/hovercard-mixin/hovercard-mixin.ts
similarity index 83%
rename from polygerrit-ui/app/elements/shared/gr-hovercard/gr-hovercard-behavior.ts
rename to polygerrit-ui/app/mixins/hovercard-mixin/hovercard-mixin.ts
index 82af365..793e5d6 100644
--- a/polygerrit-ui/app/elements/shared/gr-hovercard/gr-hovercard-behavior.ts
+++ b/polygerrit-ui/app/mixins/hovercard-mixin/hovercard-mixin.ts
@@ -14,18 +14,15 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-import '../../../styles/shared-styles';
-import {flush} from '@polymer/polymer/lib/legacy/polymer.dom';
-import {getRootElement} from '../../../scripts/rootElement';
-import {Constructor} from '../../../utils/common-util';
-import {PolymerElement} from '@polymer/polymer/polymer-element';
-import {observe, property} from '@polymer/decorators';
-import {
- pushScrollLock,
- removeScrollLock,
-} from '@polymer/iron-overlay-behavior/iron-scroll-manager';
-import {ShowAlertEventDetail} from '../../../types/events';
-import {debounce, DelayedTask} from '../../../utils/async-util';
+import {getRootElement} from '../../scripts/rootElement';
+import {Constructor} from '../../utils/common-util';
+import {LitElement, PropertyValues} from 'lit';
+import {property, query} from 'lit/decorators';
+import {ShowAlertEventDetail} from '../../types/events';
+import {debounce, DelayedTask} from '../../utils/async-util';
+import {hovercardStyles} from '../../styles/gr-hovercard-styles';
+import {sharedStyles} from '../../styles/shared-styles';
+
interface ReloadEventDetail {
clearPatchset?: boolean;
}
@@ -68,27 +65,30 @@
const HIDE_DELAY_MS = 500;
/**
- * The mixin for gr-hovercard-behavior.
+ * The mixin for hovercard behavior.
*
* @example
*
* class YourComponent extends hovercardBehaviorMixin(
- * PolymerElement
+ * LitElement)
*
* @see gr-hovercard.ts
*
* // following annotations are required for polylint
- * @polymer
+ * @lit
* @mixinFunction
*/
-export const HovercardBehaviorMixin = <T extends Constructor<PolymerElement>>(
+export const HovercardMixin = <T extends Constructor<LitElement>>(
superClass: T
) => {
/**
- * @polymer
+ * @lit
* @mixinClass
*/
class Mixin extends superClass {
+ @query('#container')
+ topElement?: HTMLElement;
+
@property({type: Object})
_target: HTMLElement | null = null;
@@ -117,50 +117,74 @@
@property({type: Object})
container: HTMLElement | null = null;
- private hideTask?: DelayedTask;
+ // Private but used in tests.
+ hideTask?: DelayedTask;
- private showTask?: DelayedTask;
+ showTask?: DelayedTask;
- private isScheduledToShow?: boolean;
+ isScheduledToShow?: boolean;
- private isScheduledToHide?: boolean;
+ isScheduledToHide?: boolean;
- override connectedCallback() {
- super.connectedCallback();
- if (!this._target) {
- this._target = this.target;
- }
- this._target.addEventListener('mouseenter', this.debounceShow);
- this._target.addEventListener('focus', this.debounceShow);
- this._target.addEventListener('mouseleave', this.debounceHide);
- this._target.addEventListener('blur', this.debounceHide);
- this._target.addEventListener('click', this.hide);
+ static get styles() {
+ return [sharedStyles, hovercardStyles];
+ }
+ // eslint-disable-next-line @typescript-eslint/no-explicit-any
+ constructor(...args: any[]) {
+ super(...args);
// show the hovercard if mouse moves to hovercard
// this will cancel pending hide as well
this.addEventListener('mouseenter', this.show);
- this.addEventListener('mouseenter', this.lock);
// when leave hovercard, hide it immediately
this.addEventListener('mouseleave', this.hide);
- this.addEventListener('mouseleave', this.unlock);
+ }
+
+ override connectedCallback() {
+ super.connectedCallback();
+ // We have to cache the target because when we this.container.appendChild
+ // in show we can not pick the container as target when we reconnect.
+ if (!this._target) {
+ this._target = this.target;
+ this.addTargetEventListeners();
+ }
+
+ this.container = getHovercardContainer({createIfNotExists: true});
}
override disconnectedCallback() {
this.cancelShowTask();
this.cancelHideTask();
- this.unlock();
+ super.disconnectedCallback();
+ }
+
+ private addTargetEventListeners() {
+ this._target?.addEventListener('mouseenter', this.debounceShow);
+ this._target?.addEventListener('focus', this.debounceShow);
+ this._target?.addEventListener('mouseleave', this.debounceHide);
+ this._target?.addEventListener('blur', this.debounceHide);
+ this._target?.addEventListener('click', this.hide);
+ }
+
+ private removeTargetEventListeners() {
this._target?.removeEventListener('mouseenter', this.debounceShow);
this._target?.removeEventListener('focus', this.debounceShow);
this._target?.removeEventListener('mouseleave', this.debounceHide);
this._target?.removeEventListener('blur', this.debounceHide);
this._target?.removeEventListener('click', this.hide);
- super.disconnectedCallback();
}
- override ready() {
- super.ready();
- // First, check to see if the container has already been created.
- this.container = getHovercardContainer({createIfNotExists: true});
+ /**
+ * Responds to a change in the `for` value and gets the updated `target`
+ * element for the hovercard.
+ */
+ override updated(changedProperties: PropertyValues) {
+ super.updated(changedProperties);
+ if (changedProperties.has('for')) {
+ this.removeTargetEventListeners();
+ this._target = this.target;
+ this.addTargetEventListeners();
+ }
}
readonly debounceHide = () => {
@@ -236,13 +260,6 @@
}
/**
- * unlock scroll, this will resume the scroll outside of the hovercard.
- */
- readonly unlock = () => {
- removeScrollLock(this);
- };
-
- /**
* Hides/closes the hovercard. This occurs when the user triggers the
* `mouseleave` event on the hovercard's `target` element (as long as the
* user is not hovering over the hovercard).
@@ -275,7 +292,7 @@
// Reset and remove the hovercard from the DOM
this.style.cssText = '';
- this.$['container'].setAttribute('tabindex', '-1');
+ this.topElement?.setAttribute('tabindex', '-1');
// Remove the hovercard from the container, given that it is still a child
// of the container.
@@ -317,13 +334,6 @@
}
/**
- * Lock background scroll but enable scroll inside of current hovercard.
- */
- readonly lock = () => {
- pushScrollLock(this);
- };
-
- /**
* Shows/opens the hovercard. This occurs when the user triggers the
* `mousenter` event on the hovercard's `target` element.
*/
@@ -347,7 +357,9 @@
// Make sure that the hovercard actually rendered and all dom-if
// statements processed, so that we can measure the (invisible)
// hovercard properly in updatePosition().
- await flush();
+ await new Promise<void>(r => {
+ setTimeout(r, 0);
+ });
this.updatePosition();
this.classList.remove(HIDE_CLASS);
};
@@ -450,29 +462,27 @@
this.style.left = `${hovercardLeft}px`;
this.style.top = `${hovercardTop}px`;
}
-
- /**
- * Responds to a change in the `for` value and gets the updated `target`
- * element for the hovercard.
- */
- @observe('for')
- _forChanged() {
- this._target = this.target;
- }
}
- return Mixin as T & Constructor<GrHovercardBehaviorInterface>;
+ return Mixin as T & Constructor<HovercardMixinInterface>;
};
-export interface GrHovercardBehaviorInterface {
+export interface HovercardMixinInterface {
+ for?: string;
+ offset: number;
_target: HTMLElement | null;
_isShowing: boolean;
- ready(): void;
dispatchEventThroughTarget(eventName: string, detail?: unknown): void;
- hide(e?: MouseEvent): void;
- debounceShow(): void;
- debounceShowBy(delayMs: number): void;
- cancelShowTask(): void;
show(): void;
+
+ // Used for tests
+ hide(e: MouseEvent): void;
+ container: HTMLElement | null;
+ hideTask?: DelayedTask;
+ showTask?: DelayedTask;
+ position: string;
+ debounceShowBy(delayMs: number): void;
updatePosition(): void;
+ isScheduledToShow?: boolean;
+ isScheduledToHide?: boolean;
}
diff --git a/polygerrit-ui/app/mixins/hovercard-mixin/hovercard-mixin_test.ts b/polygerrit-ui/app/mixins/hovercard-mixin/hovercard-mixin_test.ts
new file mode 100644
index 0000000..bd12789
--- /dev/null
+++ b/polygerrit-ui/app/mixins/hovercard-mixin/hovercard-mixin_test.ts
@@ -0,0 +1,177 @@
+/**
+ * @license
+ * Copyright (C) 2018 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import '../../test/common-test-setup-karma.js';
+import {HovercardMixin} from './hovercard-mixin.js';
+import {html, LitElement} from 'lit';
+import {customElement} from 'lit/decorators';
+import {MockPromise, mockPromise} from '../../test/test-utils.js';
+
+const base = HovercardMixin(LitElement);
+
+declare global {
+ interface HTMLElementTagNameMap {
+ 'hovercard-mixin-test': HovercardMixinTest;
+ }
+}
+
+@customElement('hovercard-mixin-test')
+class HovercardMixinTest extends base {
+ constructor() {
+ super();
+ this.for = 'foo';
+ }
+
+ override render() {
+ return html`<div id="container"><slot></slot></div>`;
+ }
+}
+
+const basicFixture = fixtureFromElement('hovercard-mixin-test');
+
+suite('gr-hovercard tests', () => {
+ let element: HovercardMixinTest;
+
+ let button: HTMLElement;
+ let testPromise: MockPromise;
+
+ setup(() => {
+ testPromise = mockPromise();
+ button = document.createElement('button');
+ button.innerHTML = 'Hello';
+ button.setAttribute('id', 'foo');
+ document.body.appendChild(button);
+
+ element = basicFixture.instantiate();
+ });
+
+ teardown(() => {
+ element.hide(new MouseEvent('click'));
+ button?.remove();
+ });
+
+ test('updatePosition', async () => {
+ // Test that the correct style properties have at least been set.
+ element.position = 'bottom';
+ element.updatePosition();
+ await element.updateComplete;
+ assert.typeOf(element.style.getPropertyValue('left'), 'string');
+ assert.typeOf(element.style.getPropertyValue('top'), 'string');
+ assert.typeOf(element.style.getPropertyValue('paddingTop'), 'string');
+ assert.typeOf(element.style.getPropertyValue('marginTop'), 'string');
+
+ const parentRect = document.documentElement.getBoundingClientRect();
+ const targetRect = element!._target!.getBoundingClientRect();
+ const thisRect = element.getBoundingClientRect();
+
+ const targetLeft = targetRect.left - parentRect.left;
+ const targetTop = targetRect.top - parentRect.top;
+
+ const pixelCompare = (pixel: string) =>
+ Math.round(parseInt(pixel.substring(0, pixel.length - 1), 10));
+
+ assert.equal(
+ pixelCompare(element.style.left),
+ pixelCompare(`${targetLeft + (targetRect.width - thisRect.width) / 2}px`)
+ );
+ assert.equal(
+ pixelCompare(element.style.top),
+ pixelCompare(`${targetTop + targetRect.height + element.offset}px`)
+ );
+ });
+
+ test('hide', () => {
+ element.hide(new MouseEvent('click'));
+ const style = getComputedStyle(element);
+ assert.isFalse(element._isShowing);
+ assert.isFalse(element.classList.contains('hovered'));
+ assert.equal(style.display, 'none');
+ assert.notEqual(element.container, element.parentNode);
+ });
+
+ test('show', async () => {
+ await element.show();
+ await element.updateComplete;
+ const style = getComputedStyle(element);
+ assert.isTrue(element._isShowing);
+ assert.isTrue(element.classList.contains('hovered'));
+ assert.equal(style.opacity, '1');
+ assert.equal(style.visibility, 'visible');
+ });
+
+ test('debounceShow does not show immediately', async () => {
+ element.debounceShowBy(100);
+ setTimeout(() => testPromise.resolve(), 0);
+ await testPromise;
+ assert.isFalse(element._isShowing);
+ });
+
+ test('debounceShow shows after delay', async () => {
+ element.debounceShowBy(1);
+ setTimeout(() => testPromise.resolve(), 10);
+ await testPromise;
+ assert.isTrue(element._isShowing);
+ });
+
+ test('card is scheduled to show on enter and hides on leave', async () => {
+ const button = document.querySelector('button');
+ const enterPromise = mockPromise();
+ button!.addEventListener('mouseenter', () => enterPromise.resolve());
+ const leavePromise = mockPromise();
+ button!.addEventListener('mouseleave', () => leavePromise.resolve());
+
+ assert.isFalse(element._isShowing);
+ button!.dispatchEvent(new CustomEvent('mouseenter'));
+
+ await enterPromise;
+ await flush();
+ assert.isTrue(element.isScheduledToShow);
+ element!.showTask!.flush();
+ assert.isTrue(element._isShowing);
+ assert.isFalse(element.isScheduledToShow);
+
+ button!.dispatchEvent(new CustomEvent('mouseleave'));
+
+ await leavePromise;
+ assert.isTrue(element.isScheduledToHide);
+ assert.isTrue(element._isShowing);
+ element!.hideTask!.flush();
+ assert.isFalse(element.isScheduledToShow);
+ assert.isFalse(element._isShowing);
+ });
+
+ test('card should disappear on click', async () => {
+ const button = document.querySelector('button');
+ const enterPromise = mockPromise();
+ const clickPromise = mockPromise();
+ button!.addEventListener('mouseenter', () => enterPromise.resolve());
+ button!.addEventListener('click', () => clickPromise.resolve());
+
+ assert.isFalse(element._isShowing);
+
+ button!.dispatchEvent(new CustomEvent('mouseenter'));
+
+ await enterPromise;
+ await flush();
+ assert.isTrue(element.isScheduledToShow);
+ button!.click();
+
+ await clickPromise;
+ assert.isFalse(element.isScheduledToShow);
+ assert.isFalse(element._isShowing);
+ });
+});
diff --git a/polygerrit-ui/app/package.json b/polygerrit-ui/app/package.json
index 4be6241..2ad4e79 100644
--- a/polygerrit-ui/app/package.json
+++ b/polygerrit-ui/app/package.json
@@ -38,7 +38,7 @@
"ba-linkify": "^1.0.1",
"codemirror-minified": "^5.62.2",
"immer": "^9.0.5",
- "lit": "2.0.0-rc.3",
+ "lit": "2.0.2",
"page": "^1.11.6",
"polymer-bridges": "file:../../polymer-bridges/",
"polymer-resin": "^2.0.1",
diff --git a/polygerrit-ui/app/styles/gr-hovercard-styles.ts b/polygerrit-ui/app/styles/gr-hovercard-styles.ts
new file mode 100644
index 0000000..f214a9c
--- /dev/null
+++ b/polygerrit-ui/app/styles/gr-hovercard-styles.ts
@@ -0,0 +1,51 @@
+/**
+ * @license
+ * Copyright (C) 2017 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+import {css} from 'lit';
+
+export const hovercardStyles = css`
+ :host {
+ position: absolute;
+ display: none;
+ z-index: 200;
+ max-width: 600px;
+ outline: none;
+ }
+ :host(.hovered) {
+ display: block;
+ }
+ :host(.hide) {
+ visibility: hidden;
+ }
+ /* You have to use a <div class="container"> in your hovercard in order
+ to pick up this consistent styling. */
+ #container {
+ background: var(--dialog-background-color);
+ border: 1px solid var(--border-color);
+ border-radius: var(--border-radius);
+ box-shadow: var(--elevation-level-5);
+ }
+`;
+
+const $_documentContainer = document.createElement('template');
+$_documentContainer.innerHTML = `<dom-module id="gr-hovercard-styles">
+ <template>
+ <style>
+ ${hovercardStyles.cssText}
+ </style>
+ </template>
+</dom-module>`;
+document.head.appendChild($_documentContainer.content);
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);
diff --git a/polygerrit-ui/app/yarn.lock b/polygerrit-ui/app/yarn.lock
index 395ef64..bfca566 100644
--- a/polygerrit-ui/app/yarn.lock
+++ b/polygerrit-ui/app/yarn.lock
@@ -2,10 +2,10 @@
# yarn lockfile v1
-"@lit/reactive-element@^1.0.0-rc.2":
- version "1.0.0-rc.3"
- resolved "https://registry.yarnpkg.com/@lit/reactive-element/-/reactive-element-1.0.0-rc.3.tgz#5032f493fbf39781b187a7e2dd5d256537c8760c"
- integrity sha512-Rs2px1keOQUNJUo5B+WExl5v244ZNCiN/iMVNO9evFdJjAdWCIupR/p14zRPkNHsciRBELLTcOZ379cI9O6PDg==
+"@lit/reactive-element@^1.0.0":
+ version "1.0.1"
+ resolved "https://registry.yarnpkg.com/@lit/reactive-element/-/reactive-element-1.0.1.tgz#853cacd4d78d79059f33f66f8e7b0e5c34bee294"
+ integrity sha512-nSD5AA2AZkKuXuvGs8IK7K5ZczLAogfDd26zT9l6S7WzvqALdVWcW5vMUiTnZyj5SPcNwNNANj0koeV1ieqTFQ==
"@mapbox/node-pre-gyp@^1.0.0":
version "1.0.5"
@@ -425,10 +425,10 @@
resolved "https://registry.yarnpkg.com/@types/resize-observer-browser/-/resize-observer-browser-0.1.6.tgz#d8e6c2f830e2650dc06fe74464472ff64b54a302"
integrity sha512-61IfTac0s9jvNtBCpyo86QeaN8qqpMGHdK0uGKCCIy2dt5/Yk84VduHIdWAcmkC5QvdkPL0p5eWYgUZtHKKUVg==
-"@types/trusted-types@^1.0.1":
- version "1.0.6"
- resolved "https://registry.yarnpkg.com/@types/trusted-types/-/trusted-types-1.0.6.tgz#569b8a08121d3203398290d602d84d73c8dcf5da"
- integrity sha512-230RC8sFeHoT6sSUlRO6a8cAnclO06eeiq1QDfiv2FGCLWFvvERWgwIQD4FWqD9A69BN7Lzee4OXwoMVnnsWDw==
+"@types/trusted-types@^2.0.2":
+ version "2.0.2"
+ resolved "https://registry.yarnpkg.com/@types/trusted-types/-/trusted-types-2.0.2.tgz#fc25ad9943bcac11cceb8168db4f275e0e72e756"
+ integrity sha512-F5DIZ36YVLE+PN+Zwws4kJogq47hNgX3Nx6WyDJ3kcplxyke3XIzB8uK5n/Lpm1HBsbGzd6nmGehL8cPekP+Tg==
"@webcomponents/shadycss@^1.10.2", "@webcomponents/shadycss@^1.9.1":
version "1.11.0"
@@ -473,9 +473,9 @@
integrity sha512-Y9J6ZjXtoYh8RnXVCMOU/ttDmk1aBjunq9vO0ta5x85WDQiQfUF9sIPBITdbiiIVcBo03Hi3jMxigBtsddlXRw==
are-we-there-yet@~1.1.2:
- version "1.1.5"
- resolved "https://registry.yarnpkg.com/are-we-there-yet/-/are-we-there-yet-1.1.5.tgz#4b35c2944f062a8bfcda66410760350fe9ddfc21"
- integrity sha512-5hYdAkZlcG8tOLujVDTgCT+uPX0VnpAH28gWsLfzpXYm7wP6mp5Q/gYyR7YQ0cKVJcXJnl3j2kpBan13PtQf6w==
+ version "1.1.7"
+ resolved "https://registry.yarnpkg.com/are-we-there-yet/-/are-we-there-yet-1.1.7.tgz#b15474a932adab4ff8a50d9adfa7e4e926f21146"
+ integrity sha512-nxwy40TuMiUGqMyRHgCSWZ9FM4VAoRP4xUYSTv5ImRog+h9yISPbVH7H8fASCIzYn9wlEv4zvFL7uKDMCFQm3g==
dependencies:
delegates "^1.0.0"
readable-stream "^2.0.6"
@@ -518,9 +518,9 @@
integrity sha1-DQcLTQQ6W+ozovGkDi7bPZpMz3c=
codemirror-minified@^5.62.2:
- version "5.62.2"
- resolved "https://registry.yarnpkg.com/codemirror-minified/-/codemirror-minified-5.62.2.tgz#37d866f5f39bbd4482c60b1607c669bcb7190388"
- integrity sha512-lQpyiEaqyEln1YDiHqq8lJcX8GkTJamecZAn0DkgdteFIVCRHnVmllOXPF+d159OSNkMi1UcKRObcU6ueBHe1A==
+ version "5.63.0"
+ resolved "https://registry.yarnpkg.com/codemirror-minified/-/codemirror-minified-5.63.0.tgz#29d1a78713a633c933a27853679afdc0bfea49cc"
+ integrity sha512-dMN2w0Qg5Zwn2p7UW3sYAoyrJ+QRBkiF5bfbQAvQ1bfqhEjGnZ++/zvOG7NivfnUbYRhSULz8lsFtzt4ldBNyQ==
concat-map@0.0.1:
version "0.0.1"
@@ -533,9 +533,9 @@
integrity sha1-PXz0Rk22RG6mRL9LOVB/mFEAjo4=
core-util-is@~1.0.0:
- version "1.0.2"
- resolved "https://registry.yarnpkg.com/core-util-is/-/core-util-is-1.0.2.tgz#b5fd54220aa2bc5ab57aab7140c940754503c1a7"
- integrity sha1-tf1UIgqivFq1eqtxQMlAdUUDwac=
+ version "1.0.3"
+ resolved "https://registry.yarnpkg.com/core-util-is/-/core-util-is-1.0.3.tgz#a6042d3634c2b27e9328f837b965fac83808db85"
+ integrity sha512-ZQBvi1DcpJ4GDqanjucZ2Hj3wEO5pZDS89BWbkcrvdxksJorwUDDZamX9ldFkp9aw2lmBDLgkObEA4DWNJ9FYQ==
debug@4:
version "4.3.2"
@@ -588,9 +588,9 @@
wide-align "^1.1.0"
glob@^7.1.3:
- version "7.1.7"
- resolved "https://registry.yarnpkg.com/glob/-/glob-7.1.7.tgz#3b193e9233f01d42d0b3f78294bbeeb418f94a90"
- integrity sha512-OvD9ENzPLbegENnYP5UUfJIirTg4+XwMWGaQfQTY0JenxNvvIKP3U3/tAQSPIu/lHxXYSZmpXlUHeqAIdKzBLQ==
+ version "7.2.0"
+ resolved "https://registry.yarnpkg.com/glob/-/glob-7.2.0.tgz#d15535af7732e02e948f4c41628bd910293f6023"
+ integrity sha512-lmLf6gtyrPq8tTjSmrO94wBeQbFR3HbLHbuyD69wuyQkImp2hWqMGB47OX65FBkPffO641IP9jWa1z4ivqG26Q==
dependencies:
fs.realpath "^1.0.0"
inflight "^1.0.4"
@@ -613,9 +613,9 @@
debug "4"
immer@^9.0.5:
- version "9.0.5"
- resolved "https://registry.yarnpkg.com/immer/-/immer-9.0.5.tgz#a7154f34fe7064f15f00554cc94c66cc0bf453ec"
- integrity sha512-2WuIehr2y4lmYz9gaQzetPR2ECniCifk4ORaQbU3g5EalLt+0IVTosEPJ5BoYl/75ky2mivzdRzV8wWgQGOSYQ==
+ version "9.0.6"
+ resolved "https://registry.yarnpkg.com/immer/-/immer-9.0.6.tgz#7a96bf2674d06c8143e327cbf73539388ddf1a73"
+ integrity sha512-G95ivKpy+EvVAnAab4fVa4YGYn24J1SpEktnJX7JJ45Bd7xqME/SCplFzYFmTbrkwZbQ4xJK1xMTUYBkN6pWsQ==
inflight@^1.0.4:
version "1.0.6"
@@ -652,29 +652,29 @@
resolved "https://registry.yarnpkg.com/isarray/-/isarray-1.0.0.tgz#bb935d48582cba168c06834957a54a3e07124f11"
integrity sha1-u5NdSFgsuhaMBoNJV6VKPgcSTxE=
-lit-element@^3.0.0-rc.2:
- version "3.0.0-rc.3"
- resolved "https://registry.yarnpkg.com/lit-element/-/lit-element-3.0.0-rc.3.tgz#cece8f092d28eb6f9c6b23e4138ff5d7260897ef"
- integrity sha512-NDe7yjW18gfYQb1GIEQr1T8sB1GUAb1HB62pdAEw+SK6lUW7OFPKQqCOlRhZ6qJXsw9KxMnyYIprLZT4FZdYdQ==
+lit-element@^3.0.0:
+ version "3.0.1"
+ resolved "https://registry.yarnpkg.com/lit-element/-/lit-element-3.0.1.tgz#3c545af17d8a46268bc1dd5623a47486e6ff76f4"
+ integrity sha512-vs9uybH9ORyK49CFjoNGN85HM9h5bmisU4TQ63phe/+GYlwvY/3SIFYKdjV6xNvzz8v2MnVC+9+QOkPqh+Q3Ew==
dependencies:
- "@lit/reactive-element" "^1.0.0-rc.2"
- lit-html "^2.0.0-rc.4"
+ "@lit/reactive-element" "^1.0.0"
+ lit-html "^2.0.0"
-lit-html@^2.0.0-rc.4:
- version "2.0.0-rc.4"
- resolved "https://registry.yarnpkg.com/lit-html/-/lit-html-2.0.0-rc.4.tgz#1015fa8f1f7c8c5b79999ed0bc11c3b79ff1aab5"
- integrity sha512-WSLGu3vxq7y8q/oOd9I3zxyBELNLLiDk6gAYoKK4PGctI5fbh6lhnO/jVBdy0PV/vTc+cLJCA/occzx3YoNPeg==
+lit-html@^2.0.0:
+ version "2.0.1"
+ resolved "https://registry.yarnpkg.com/lit-html/-/lit-html-2.0.1.tgz#63241015efa07bc9259b6f96f04abd052d2a1f95"
+ integrity sha512-KF5znvFdXbxTYM/GjpdOOnMsjgRcFGusTnB54ixnCTya5zUR0XqrDRj29ybuLS+jLXv1jji6Y8+g4W7WP8uL4w==
dependencies:
- "@types/trusted-types" "^1.0.1"
+ "@types/trusted-types" "^2.0.2"
-lit@2.0.0-rc.3:
- version "2.0.0-rc.3"
- resolved "https://registry.yarnpkg.com/lit/-/lit-2.0.0-rc.3.tgz#8b6a85268aba287c11125dfe57e88e0bc09beaff"
- integrity sha512-UZDLWuspl7saA+WvS0e+TE3NdGGE05hOIwUPTWiibs34c5QupcEzpjB/aElt79V9bELQVNbUUwa0Ow7D1Wuszw==
+lit@2.0.2:
+ version "2.0.2"
+ resolved "https://registry.yarnpkg.com/lit/-/lit-2.0.2.tgz#5e6f422924e0732258629fb379556b6d23f7179c"
+ integrity sha512-hKA/1YaSB+P+DvKWuR2q1Xzy/iayhNrJ3aveD0OQ9CKn6wUjsdnF/7LavDOJsKP/K5jzW/kXsuduPgRvTFrFJw==
dependencies:
- "@lit/reactive-element" "^1.0.0-rc.2"
- lit-element "^3.0.0-rc.2"
- lit-html "^2.0.0-rc.4"
+ "@lit/reactive-element" "^1.0.0"
+ lit-element "^3.0.0"
+ lit-html "^2.0.0"
lru-cache@^6.0.0:
version "6.0.0"
@@ -703,9 +703,9 @@
brace-expansion "^1.1.7"
minipass@^3.0.0:
- version "3.1.3"
- resolved "https://registry.yarnpkg.com/minipass/-/minipass-3.1.3.tgz#7d42ff1f39635482e15f9cdb53184deebd5815fd"
- integrity sha512-Mgd2GdMVzY+x3IJ+oHnVM+KG3lA5c8tnabyJKmHSaG2kAGpudxuOf8ToDkhumF7UzME7DecbQE9uOZhNm7PuJg==
+ version "3.1.5"
+ resolved "https://registry.yarnpkg.com/minipass/-/minipass-3.1.5.tgz#71f6251b0a33a49c01b3cf97ff77eda030dff732"
+ integrity sha512-+8NzxD82XQoNKNrl1d/FSi+X8wAEWR+sbYAfIvub4Nz0d22plFG72CEVVaufV8PNf4qSslFTD8VMOxNVhHCjTw==
dependencies:
yallist "^4.0.0"
@@ -733,9 +733,11 @@
integrity sha512-8ZtvEnA2c5aYCZYd1cvgdnU6cqwixRoYg70xPLWUws5ORTa/lnw+u4amixRS/Ac5U5mQVgp9pnlSUnbNWFaWZQ==
node-fetch@^2.6.1:
- version "2.6.1"
- resolved "https://registry.yarnpkg.com/node-fetch/-/node-fetch-2.6.1.tgz#045bd323631f76ed2e2b55573394416b639a0052"
- integrity sha512-V4aYg89jEoVRxRb2fJdAg8FHvI7cEyYdVAh94HH0UIK8oJxUfkjlDQN9RbMx+bEjP7+ggMiFRprSti032Oipxw==
+ version "2.6.5"
+ resolved "https://registry.yarnpkg.com/node-fetch/-/node-fetch-2.6.5.tgz#42735537d7f080a7e5f78b6c549b7146be1742fd"
+ integrity sha512-mmlIVHJEu5rnIxgEgez6b9GgWXbkZj5YZ7fx+2r94a2E+Uirsp6HsPTPlomfdHtpt/B0cdKviwkoaM6pyvUOpQ==
+ dependencies:
+ whatwg-url "^5.0.0"
nopt@^5.0.0:
version "5.0.0"
@@ -863,9 +865,9 @@
integrity sha1-BF+XgtARrppoA93TgrJDkrPYkPc=
signal-exit@^3.0.0:
- version "3.0.3"
- resolved "https://registry.yarnpkg.com/signal-exit/-/signal-exit-3.0.3.tgz#a1410c2edd8f077b08b4e253c8eacfcaf057461c"
- integrity sha512-VUJ49FC8U1OxwZLxIbTTrDvLnf/6TDgxZcK8wxR8zs13xpx7xbG60ndBlhNrFi2EMuFRoeDoJO7wthSLq42EjA==
+ version "3.0.5"
+ resolved "https://registry.yarnpkg.com/signal-exit/-/signal-exit-3.0.5.tgz#9e3e8cc0c75a99472b44321033a7702e7738252f"
+ integrity sha512-KWcOiKeQj6ZyXx7zq4YxSMgHRlod4czeBQZrPb8OKcohcqAXShm7E20kEMle9WBt26hFcAf0qLOcp5zmY7kOqQ==
simple-concat@^1.0.0:
version "1.0.1"
@@ -920,9 +922,9 @@
ansi-regex "^3.0.0"
tar@^6.1.0:
- version "6.1.8"
- resolved "https://registry.yarnpkg.com/tar/-/tar-6.1.8.tgz#4fc50cfe56511c538ce15b71e05eebe66530cbd4"
- integrity sha512-sb9b0cp855NbkMJcskdSYA7b11Q8JsX4qe4pyUAfHp+Y6jBjJeek2ZVlwEfWayshEIwlIzXx0Fain3QG9JPm2A==
+ version "6.1.11"
+ resolved "https://registry.yarnpkg.com/tar/-/tar-6.1.11.tgz#6760a38f003afa1b2ffd0ffe9e9abbd0eab3d621"
+ integrity sha512-an/KZQzQUkZCkuoAA64hM92X0Urb6VpRhAFllDzz44U2mcD5scmT3zBc4VgVpkugF580+DQn8eAFSyoQt0tznA==
dependencies:
chownr "^2.0.0"
fs-minipass "^2.0.0"
@@ -931,6 +933,11 @@
mkdirp "^1.0.3"
yallist "^4.0.0"
+tr46@~0.0.3:
+ version "0.0.3"
+ resolved "https://registry.yarnpkg.com/tr46/-/tr46-0.0.3.tgz#8184fd347dac9cdc185992f3a6622e14b9d9ab6a"
+ integrity sha1-gYT9NH2snNwYWZLzpmIuFLnZq2o=
+
tslib@^1.9.0:
version "1.14.1"
resolved "https://registry.yarnpkg.com/tslib/-/tslib-1.14.1.tgz#cf2d38bdc34a134bcaf1091c41f6619e2f672d00"
@@ -941,6 +948,19 @@
resolved "https://registry.yarnpkg.com/util-deprecate/-/util-deprecate-1.0.2.tgz#450d4dc9fa70de732762fbd2d4a28981419a0ccf"
integrity sha1-RQ1Nyfpw3nMnYvvS1KKJgUGaDM8=
+webidl-conversions@^3.0.0:
+ version "3.0.1"
+ resolved "https://registry.yarnpkg.com/webidl-conversions/-/webidl-conversions-3.0.1.tgz#24534275e2a7bc6be7bc86611cc16ae0a5654871"
+ integrity sha1-JFNCdeKnvGvnvIZhHMFq4KVlSHE=
+
+whatwg-url@^5.0.0:
+ version "5.0.0"
+ resolved "https://registry.yarnpkg.com/whatwg-url/-/whatwg-url-5.0.0.tgz#966454e8765462e37644d3626f6742ce8b70965d"
+ integrity sha1-lmRU6HZUYuN2RNNib2dCzotwll0=
+ dependencies:
+ tr46 "~0.0.3"
+ webidl-conversions "^3.0.0"
+
wide-align@^1.1.0:
version "1.1.3"
resolved "https://registry.yarnpkg.com/wide-align/-/wide-align-1.1.3.tgz#ae074e6bdc0c14a431e804e624549c633b000457"
