Merge "docs/pgm-daemon: fix number of dashes for Inspector option, -s"
diff --git a/Documentation/metrics.txt b/Documentation/metrics.txt
index 0318cd7..d8b6250 100644
--- a/Documentation/metrics.txt
+++ b/Documentation/metrics.txt
@@ -113,7 +113,7 @@
* `receivecommits/ps_revision_missing`: errors due to patch set revision missing
* `receivecommits/push_count`: number of pushes
** `kind`:
- The push kind (direct vs. magic).
+ The push kind (magic, direct or direct_submit).
** `project`:
The name of the project for which the push is done.
** `type`:
diff --git a/Documentation/user-search.txt b/Documentation/user-search.txt
index 1543720..6e6b9d7 100644
--- a/Documentation/user-search.txt
+++ b/Documentation/user-search.txt
@@ -777,6 +777,22 @@
+
Matches changes with label voted with any score.
+`label:Code-Review=+1,count=2`::
++
+Matches changes with exactly two +1 votes to the code-review label. The {MAX,
+MIN, ANY} votes can also be used, for example `label:Code-Review=MAX,count=2` is
+equivalent to `label:Code-Review=2,count=2` (if 2 is the maximum positive vote
+for the code review label). The maximum supported value for `count` is 5.
+`count=0` is not allowed and the query request will fail with `400 Bad Request`.
+
+`label:Code-Review=+1,count>=2`::
++
+Matches changes having two or more +1 votes to the code-review label. Can also
+be used with the {MAX, MIN, ANY} label votes. All operators `>`, `>=`, `<`, `<=`
+are supported.
+Note that a query like `label:Code-Review=+1,count<x` will not match with
+changes having zero +1 votes to this label.
+
`label:Non-Author-Code-Review=need`::
+
Matches changes where the submit rules indicate that a label named
diff --git a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
index 66e7d80..f4c7a92 100644
--- a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
+++ b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
@@ -747,14 +747,19 @@
return;
}
- if (!magicCommands.isEmpty()) {
- metrics.pushCount.increment("magic", project.getName(), getUpdateType(magicCommands));
- }
- if (!regularCommands.isEmpty()) {
- metrics.pushCount.increment("direct", project.getName(), getUpdateType(regularCommands));
- }
-
try {
+ if (!magicCommands.isEmpty()) {
+ parseMagicBranch(Iterables.getLast(magicCommands));
+ // Using the submit option submits the created change(s) immediately without checking labels
+ // nor submit rules. Hence we shouldn't record such pushes as "magic" which implies that
+ // code review is being done.
+ String pushKind = magicBranch != null && magicBranch.submit ? "direct_submit" : "magic";
+ metrics.pushCount.increment(pushKind, project.getName(), getUpdateType(magicCommands));
+ }
+ if (!regularCommands.isEmpty()) {
+ metrics.pushCount.increment("direct", project.getName(), getUpdateType(regularCommands));
+ }
+
if (!regularCommands.isEmpty()) {
handleRegularCommands(regularCommands, progress);
return;
@@ -763,7 +768,6 @@
boolean first = true;
for (ReceiveCommand cmd : magicCommands) {
if (first) {
- parseMagicBranch(cmd);
first = false;
} else {
reject(cmd, "duplicate request");
diff --git a/java/com/google/gerrit/server/git/receive/ReceiveConstants.java b/java/com/google/gerrit/server/git/receive/ReceiveConstants.java
index df1888b..e50482d 100644
--- a/java/com/google/gerrit/server/git/receive/ReceiveConstants.java
+++ b/java/com/google/gerrit/server/git/receive/ReceiveConstants.java
@@ -21,7 +21,7 @@
@VisibleForTesting
public static final String ONLY_USERS_WITH_TOGGLE_WIP_STATE_PERM_CAN_MODIFY_WIP =
- "only users with Toogle-Wip-State permission can modify Work-in-Progress";
+ "only users with Toggle-Wip-State permission can modify Work-in-Progress";
static final String COMMAND_REJECTION_MESSAGE_FOOTER =
"Contact an administrator to fix the permissions";
diff --git a/java/com/google/gerrit/server/index/change/ChangeField.java b/java/com/google/gerrit/server/index/change/ChangeField.java
index 2581d89..2cdb7c8 100644
--- a/java/com/google/gerrit/server/index/change/ChangeField.java
+++ b/java/com/google/gerrit/server/index/change/ChangeField.java
@@ -33,6 +33,7 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Splitter;
+import com.google.common.collect.HashBasedTable;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
@@ -616,8 +617,10 @@
private static Iterable<String> getLabels(ChangeData cd) {
Set<String> allApprovals = new HashSet<>();
Set<String> distinctApprovals = new HashSet<>();
+ Table<String, Short, Integer> voteCounts = HashBasedTable.create();
for (PatchSetApproval a : cd.currentApprovals()) {
if (a.value() != 0 && !a.isLegacySubmit()) {
+ increment(voteCounts, a.label(), a.value());
Optional<LabelType> labelType = cd.getLabelTypes().byLabel(a.labelId());
allApprovals.add(formatLabel(a.label(), a.value(), a.accountId()));
@@ -630,9 +633,67 @@
}
}
allApprovals.addAll(distinctApprovals);
+ allApprovals.addAll(getCountLabelFormats(voteCounts, cd));
return allApprovals;
}
+ private static void increment(Table<String, Short, Integer> table, String k1, short k2) {
+ if (!table.contains(k1, k2)) {
+ table.put(k1, k2, 1);
+ } else {
+ int val = table.get(k1, k2);
+ table.put(k1, k2, val + 1);
+ }
+ }
+
+ private static List<String> getCountLabelFormats(
+ Table<String, Short, Integer> voteCounts, ChangeData cd) {
+ List<String> allFormats = new ArrayList<>();
+ for (String label : voteCounts.rowMap().keySet()) {
+ Optional<LabelType> labelType = cd.getLabelTypes().byLabel(label);
+ Map<Short, Integer> row = voteCounts.row(label);
+ for (short vote : row.keySet()) {
+ int count = row.get(vote);
+ allFormats.addAll(getCountLabelFormats(labelType, label, vote, count));
+ }
+ }
+ return allFormats;
+ }
+
+ private static List<String> getCountLabelFormats(
+ Optional<LabelType> labelType, String label, short vote, int count) {
+ List<String> formats =
+ getMagicLabelFormats(label, vote, labelType, /* accountId= */ null, /* count= */ count);
+ formats.add(formatLabel(label, vote, count));
+ return formats;
+ }
+
+ /** Get magic label formats corresponding to the {MIN, MAX, ANY} label votes. */
+ private static List<String> getMagicLabelFormats(
+ String label, short labelVal, Optional<LabelType> labelType, @Nullable Account.Id accountId) {
+ return getMagicLabelFormats(label, labelVal, labelType, accountId, /* count= */ null);
+ }
+
+ /** Get magic label formats corresponding to the {MIN, MAX, ANY} label votes. */
+ private static List<String> getMagicLabelFormats(
+ String label,
+ short labelVal,
+ Optional<LabelType> labelType,
+ @Nullable Account.Id accountId,
+ @Nullable Integer count) {
+ List<String> labels = new ArrayList<>();
+ if (labelType.isPresent()) {
+ if (labelVal == labelType.get().getMaxPositive()) {
+ labels.add(formatLabel(label, MagicLabelValue.MAX.name(), accountId, count));
+ }
+ if (labelVal == labelType.get().getMaxNegative()) {
+ labels.add(formatLabel(label, MagicLabelValue.MIN.name(), accountId, count));
+ }
+ }
+ labels.add(formatLabel(label, MagicLabelValue.ANY.name(), accountId, count));
+ return labels;
+ }
+
private static List<String> getLabelOwnerFormats(
PatchSetApproval a, ChangeData cd, Optional<LabelType> labelType) {
List<String> allFormats = new ArrayList<>();
@@ -657,22 +718,6 @@
return allFormats;
}
- /** Get magic label formats corresponding to the {MIN, MAX, ANY} label votes. */
- private static List<String> getMagicLabelFormats(
- String label, short labelVal, Optional<LabelType> labelType, @Nullable Account.Id accountId) {
- List<String> labels = new ArrayList<>();
- if (labelType.isPresent()) {
- if (labelVal == labelType.get().getMaxPositive()) {
- labels.add(formatLabel(label, MagicLabelValue.MAX.name(), accountId));
- }
- if (labelVal == labelType.get().getMaxNegative()) {
- labels.add(formatLabel(label, MagicLabelValue.MIN.name(), accountId));
- }
- }
- labels.add(formatLabel(label, MagicLabelValue.ANY.name(), accountId));
- return labels;
- }
-
public static Set<String> getAuthorParts(ChangeData cd) {
return SchemaUtil.getPersonParts(cd.getAuthor());
}
@@ -747,21 +792,33 @@
decodeProtos(field, PatchSetApprovalProtoConverter.INSTANCE)));
public static String formatLabel(String label, int value) {
- return formatLabel(label, value, null);
+ return formatLabel(label, value, /* accountId= */ null, /* count= */ null);
+ }
+
+ public static String formatLabel(String label, int value, @Nullable Integer count) {
+ return formatLabel(label, value, /* accountId= */ null, count);
}
public static String formatLabel(String label, int value, Account.Id accountId) {
+ return formatLabel(label, value, accountId, /* count= */ null);
+ }
+
+ public static String formatLabel(
+ String label, int value, @Nullable Account.Id accountId, @Nullable Integer count) {
return label.toLowerCase()
+ (value >= 0 ? "+" : "")
+ value
- + (accountId != null ? "," + formatAccount(accountId) : "");
+ + (accountId != null ? "," + formatAccount(accountId) : "")
+ + (count != null ? ",count=" + count : "");
}
- public static String formatLabel(String label, String value, @Nullable Account.Id accountId) {
+ public static String formatLabel(
+ String label, String value, @Nullable Account.Id accountId, @Nullable Integer count) {
return label.toLowerCase()
+ "="
+ value
- + (accountId != null ? "," + formatAccount(accountId) : "");
+ + (accountId != null ? "," + formatAccount(accountId) : "")
+ + (count != null ? ",count=" + count : "");
}
private static String formatAccount(Account.Id accountId) {
diff --git a/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java b/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java
index 5dfce84..ee93065 100644
--- a/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java
+++ b/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java
@@ -188,9 +188,13 @@
new Schema.Builder<ChangeData>().add(V70).add(ChangeField.UPLOADER).build();
/** Added new field {@link ChangeField#IS_PURE_REVERT}. */
+ @Deprecated
static final Schema<ChangeData> V72 =
new Schema.Builder<ChangeData>().add(V71).add(ChangeField.IS_PURE_REVERT).build();
+ /** Added new "count=$count" argument to the {@link ChangeField#LABEL} operator. */
+ static final Schema<ChangeData> V73 = schema(V72, false);
+
/**
* Name of the change index to be used when contacting index backends or loading configurations.
*/
diff --git a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
index e1cf454..d435df1 100644
--- a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
+++ b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
@@ -84,6 +84,7 @@
import com.google.gerrit.server.plugincontext.PluginSetContext;
import com.google.gerrit.server.project.ChildProjects;
import com.google.gerrit.server.project.ProjectCache;
+import com.google.gerrit.server.query.change.PredicateArgs.ValOp;
import com.google.gerrit.server.rules.SubmitRule;
import com.google.gerrit.server.submit.SubmitDryRun;
import com.google.inject.Inject;
@@ -214,6 +215,7 @@
public static final String ARG_ID_GROUP = "group";
public static final String ARG_ID_OWNER = "owner";
public static final String ARG_ID_NON_UPLOADER = "non_uploader";
+ public static final String ARG_COUNT = "count";
public static final Account.Id OWNER_ACCOUNT_ID = Account.id(0);
public static final Account.Id NON_UPLOADER_ACCOUNT_ID = Account.id(-1);
@@ -946,6 +948,8 @@
throws QueryParseException, IOException, ConfigInvalidException {
Set<Account.Id> accounts = null;
AccountGroup.UUID group = null;
+ Integer count = null;
+ PredicateArgs.Operator countOp = null;
// Parse for:
// label:Code-Review=1,user=jsmith or
@@ -956,6 +960,7 @@
// Special case: votes by owners can be tracked with ",owner":
// label:Code-Review+2,owner
// label:Code-Review+2,user=owner
+ // label:Code-Review+1,count=2
List<String> splitReviewer = Lists.newArrayList(Splitter.on(',').limit(2).split(name));
name = splitReviewer.get(0); // remove all but the vote piece, e.g.'CodeReview=1'
@@ -963,17 +968,40 @@
// process the user/group piece
PredicateArgs lblArgs = new PredicateArgs(splitReviewer.get(1));
- for (Map.Entry<String, String> pair : lblArgs.keyValue.entrySet()) {
- if (pair.getKey().equalsIgnoreCase(ARG_ID_USER)) {
- if (pair.getValue().equals(ARG_ID_OWNER)) {
+ // Disallow using the "count=" arg in conjunction with the "user=" or "group=" args. to avoid
+ // unnecessary complexity.
+ assertDisjunctive(lblArgs, ARG_COUNT, ARG_ID_USER);
+ assertDisjunctive(lblArgs, ARG_COUNT, ARG_ID_GROUP);
+
+ for (Map.Entry<String, ValOp> pair : lblArgs.keyValue.entrySet()) {
+ String key = pair.getKey();
+ String value = pair.getValue().value();
+ PredicateArgs.Operator operator = pair.getValue().operator();
+ if (key.equalsIgnoreCase(ARG_ID_USER)) {
+ if (value.equals(ARG_ID_OWNER)) {
accounts = Collections.singleton(OWNER_ACCOUNT_ID);
- } else if (pair.getValue().equals(ARG_ID_NON_UPLOADER)) {
+ } else if (value.equals(ARG_ID_NON_UPLOADER)) {
accounts = Collections.singleton(NON_UPLOADER_ACCOUNT_ID);
} else {
- accounts = parseAccount(pair.getValue());
+ accounts = parseAccount(value);
}
- } else if (pair.getKey().equalsIgnoreCase(ARG_ID_GROUP)) {
- group = parseGroup(pair.getValue()).getUUID();
+ } else if (key.equalsIgnoreCase(ARG_ID_GROUP)) {
+ group = parseGroup(value).getUUID();
+ } else if (key.equalsIgnoreCase(ARG_COUNT)) {
+ if (!isInt(value)) {
+ throw new QueryParseException("Invalid count argument. Value should be an integer");
+ }
+ count = Integer.parseInt(value);
+ countOp = operator;
+ if (count == 0) {
+ throw new QueryParseException("Argument count=0 is not allowed.");
+ }
+ if (count > LabelPredicate.MAX_COUNT) {
+ throw new QueryParseException(
+ String.format(
+ "count=%d is not allowed. Maximum allowed value for count is %d.",
+ count, LabelPredicate.MAX_COUNT));
+ }
} else {
throw new QueryParseException("Invalid argument identifier '" + pair.getKey() + "'");
}
@@ -1022,7 +1050,18 @@
}
}
- return new LabelPredicate(args, name, accounts, group);
+ return new LabelPredicate(args, name, accounts, group, count, countOp);
+ }
+
+ /** Assert that keys {@code k1} and {@code k2} do not exist in {@code labelArgs} together. */
+ private void assertDisjunctive(PredicateArgs labelArgs, String k1, String k2)
+ throws QueryParseException {
+ Map<String, ValOp> keyValArgs = labelArgs.keyValue;
+ if (keyValArgs.containsKey(k1) && keyValArgs.containsKey(k2)) {
+ throw new QueryParseException(
+ String.format(
+ "Cannot use the '%s' argument in conjunction with the '%s' argument", k1, k2));
+ }
}
private static boolean isInt(String s) {
@@ -1345,7 +1384,7 @@
try (Repository git = args.repoManager.openRepository(args.allUsersName)) {
// [name=]<name>
if (inputArgs.keyValue.containsKey(ARG_ID_NAME)) {
- name = inputArgs.keyValue.get(ARG_ID_NAME);
+ name = inputArgs.keyValue.get(ARG_ID_NAME).value();
} else if (inputArgs.positional.size() == 1) {
name = Iterables.getOnlyElement(inputArgs.positional);
} else if (inputArgs.positional.size() > 1) {
@@ -1354,7 +1393,7 @@
// [,user=<user>]
if (inputArgs.keyValue.containsKey(ARG_ID_USER)) {
- Set<Account.Id> accounts = parseAccount(inputArgs.keyValue.get(ARG_ID_USER));
+ Set<Account.Id> accounts = parseAccount(inputArgs.keyValue.get(ARG_ID_USER).value());
if (accounts != null && accounts.size() > 1) {
throw error(
String.format(
@@ -1396,7 +1435,7 @@
try (Repository git = args.repoManager.openRepository(args.allUsersName)) {
// [name=]<name>
if (inputArgs.keyValue.containsKey(ARG_ID_NAME)) {
- name = inputArgs.keyValue.get(ARG_ID_NAME);
+ name = inputArgs.keyValue.get(ARG_ID_NAME).value();
} else if (inputArgs.positional.size() == 1) {
name = Iterables.getOnlyElement(inputArgs.positional);
} else if (inputArgs.positional.size() > 1) {
@@ -1405,7 +1444,7 @@
// [,user=<user>]
if (inputArgs.keyValue.containsKey(ARG_ID_USER)) {
- Set<Account.Id> accounts = parseAccount(inputArgs.keyValue.get(ARG_ID_USER));
+ Set<Account.Id> accounts = parseAccount(inputArgs.keyValue.get(ARG_ID_USER).value());
if (accounts != null && accounts.size() > 1) {
throw error(
String.format(
diff --git a/java/com/google/gerrit/server/query/change/EqualsLabelPredicate.java b/java/com/google/gerrit/server/query/change/EqualsLabelPredicate.java
index 12efecb..b2bc6aa 100644
--- a/java/com/google/gerrit/server/query/change/EqualsLabelPredicate.java
+++ b/java/com/google/gerrit/server/query/change/EqualsLabelPredicate.java
@@ -14,6 +14,7 @@
package com.google.gerrit.server.query.change;
+import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.AccountGroup;
import com.google.gerrit.entities.Change;
@@ -34,17 +35,34 @@
protected final ProjectCache projectCache;
protected final PermissionBackend permissionBackend;
protected final IdentifiedUser.GenericFactory userFactory;
+ /** label name to be matched. */
protected final String label;
+
+ /** Expected vote value for the label. */
protected final int expVal;
+
+ /**
+ * Number of times the value {@link #expVal} for label {@link #label} should occur. If null, match
+ * with any count greater or equal to 1.
+ */
+ @Nullable protected final Integer count;
+
+ /** Account ID that has voted on the label. */
protected final Account.Id account;
+
protected final AccountGroup.UUID group;
public EqualsLabelPredicate(
- LabelPredicate.Args args, String label, int expVal, Account.Id account) {
- super(ChangeField.LABEL, ChangeField.formatLabel(label, expVal, account));
+ LabelPredicate.Args args,
+ String label,
+ int expVal,
+ Account.Id account,
+ @Nullable Integer count) {
+ super(ChangeField.LABEL, ChangeField.formatLabel(label, expVal, account, count));
this.permissionBackend = args.permissionBackend;
this.projectCache = args.projectCache;
this.userFactory = args.userFactory;
+ this.count = count;
this.group = args.group;
this.label = label;
this.expVal = expVal;
@@ -60,6 +78,14 @@
return false;
}
+ if (Integer.valueOf(0).equals(count)) {
+ // We don't match against count=0 so that the computation is identical to the stored values
+ // in the index. We do that since computing count=0 requires looping on all {label_type,
+ // vote_value} for the change and storing a {count=0} format for it in the change index which
+ // is computationally expensive.
+ return false;
+ }
+
Optional<ProjectState> project = projectCache.get(c.getDest().project());
if (!project.isPresent()) {
// The project has disappeared.
@@ -73,12 +99,13 @@
}
boolean hasVote = false;
+ int matchingVotes = 0;
object.setStorageConstraint(ChangeData.StorageConstraint.INDEX_PRIMARY_NOTEDB_SECONDARY);
for (PatchSetApproval p : object.currentApprovals()) {
if (labelType.matches(p)) {
hasVote = true;
if (match(object, p.value(), p.accountId())) {
- return true;
+ matchingVotes += 1;
}
}
}
@@ -87,7 +114,7 @@
return true;
}
- return false;
+ return count == null ? matchingVotes >= 1 : matchingVotes == count;
}
protected static LabelType type(LabelTypes types, String toFind) {
diff --git a/java/com/google/gerrit/server/query/change/LabelPredicate.java b/java/com/google/gerrit/server/query/change/LabelPredicate.java
index 5f017fb..2e09075 100644
--- a/java/com/google/gerrit/server/query/change/LabelPredicate.java
+++ b/java/com/google/gerrit/server/query/change/LabelPredicate.java
@@ -14,8 +14,8 @@
package com.google.gerrit.server.query.change;
-import com.google.common.collect.ImmutableList;
import com.google.common.collect.Lists;
+import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.AccountGroup;
import com.google.gerrit.index.query.OrPredicate;
@@ -29,9 +29,11 @@
import java.util.ArrayList;
import java.util.List;
import java.util.Set;
+import java.util.stream.IntStream;
public class LabelPredicate extends OrPredicate<ChangeData> {
protected static final int MAX_LABEL_VALUE = 4;
+ protected static final int MAX_COUNT = 5; // inclusive
protected static class Args {
protected final ProjectCache projectCache;
@@ -40,6 +42,8 @@
protected final String value;
protected final Set<Account.Id> accounts;
protected final AccountGroup.UUID group;
+ protected final Integer count;
+ protected final PredicateArgs.Operator countOp;
protected Args(
ProjectCache projectCache,
@@ -47,13 +51,17 @@
IdentifiedUser.GenericFactory userFactory,
String value,
Set<Account.Id> accounts,
- AccountGroup.UUID group) {
+ AccountGroup.UUID group,
+ @Nullable Integer count,
+ @Nullable PredicateArgs.Operator countOp) {
this.projectCache = projectCache;
this.permissionBackend = permissionBackend;
this.userFactory = userFactory;
this.value = value;
this.accounts = accounts;
this.group = group;
+ this.count = count;
+ this.countOp = countOp;
}
}
@@ -75,19 +83,35 @@
ChangeQueryBuilder.Arguments a,
String value,
Set<Account.Id> accounts,
- AccountGroup.UUID group) {
+ AccountGroup.UUID group,
+ @Nullable Integer count,
+ @Nullable PredicateArgs.Operator countOp) {
super(
predicates(
- new Args(a.projectCache, a.permissionBackend, a.userFactory, value, accounts, group)));
+ new Args(
+ a.projectCache,
+ a.permissionBackend,
+ a.userFactory,
+ value,
+ accounts,
+ group,
+ count,
+ countOp)));
this.value = value;
}
protected static List<Predicate<ChangeData>> predicates(Args args) {
String v = args.value;
-
+ List<Integer> counts = getCounts(args.count, args.countOp);
try {
MagicLabelVote mlv = MagicLabelVote.parseWithEquals(v);
- return ImmutableList.of(magicLabelPredicate(args, mlv));
+ List<Predicate<ChangeData>> result = Lists.newArrayListWithCapacity(counts.size());
+ if (counts.isEmpty()) {
+ result.add(magicLabelPredicate(args, mlv, /* count= */ null));
+ } else {
+ counts.forEach(count -> result.add(magicLabelPredicate(args, mlv, count)));
+ }
+ return result;
} catch (IllegalArgumentException e) {
// Try next format.
}
@@ -123,16 +147,24 @@
int min = range.min;
int max = range.max;
- List<Predicate<ChangeData>> r = Lists.newArrayListWithCapacity(max - min + 1);
+ List<Predicate<ChangeData>> r =
+ Lists.newArrayListWithCapacity((counts.isEmpty() ? 1 : counts.size()) * (max - min + 1));
for (int i = min; i <= max; i++) {
- r.add(onePredicate(args, prefix, i));
+ if (counts.isEmpty()) {
+ r.add(onePredicate(args, prefix, i, /* count= */ null));
+ } else {
+ for (int count : counts) {
+ r.add(onePredicate(args, prefix, i, count));
+ }
+ }
}
return r;
}
- protected static Predicate<ChangeData> onePredicate(Args args, String label, int expVal) {
+ protected static Predicate<ChangeData> onePredicate(
+ Args args, String label, int expVal, @Nullable Integer count) {
if (expVal != 0) {
- return equalsLabelPredicate(args, label, expVal);
+ return equalsLabelPredicate(args, label, expVal, count);
}
return noLabelQuery(args, label);
}
@@ -140,34 +172,66 @@
protected static Predicate<ChangeData> noLabelQuery(Args args, String label) {
List<Predicate<ChangeData>> r = Lists.newArrayListWithCapacity(2 * MAX_LABEL_VALUE);
for (int i = 1; i <= MAX_LABEL_VALUE; i++) {
- r.add(equalsLabelPredicate(args, label, i));
- r.add(equalsLabelPredicate(args, label, -i));
+ r.add(equalsLabelPredicate(args, label, i, /* count= */ null));
+ r.add(equalsLabelPredicate(args, label, -i, /* count= */ null));
}
return not(or(r));
}
- protected static Predicate<ChangeData> equalsLabelPredicate(Args args, String label, int expVal) {
+ protected static Predicate<ChangeData> equalsLabelPredicate(
+ Args args, String label, int expVal, @Nullable Integer count) {
if (args.accounts == null || args.accounts.isEmpty()) {
- return new EqualsLabelPredicate(args, label, expVal, null);
+ return new EqualsLabelPredicate(args, label, expVal, null, count);
}
List<Predicate<ChangeData>> r = new ArrayList<>();
for (Account.Id a : args.accounts) {
- r.add(new EqualsLabelPredicate(args, label, expVal, a));
+ r.add(new EqualsLabelPredicate(args, label, expVal, a, count));
}
return or(r);
}
- protected static Predicate<ChangeData> magicLabelPredicate(Args args, MagicLabelVote mlv) {
+ protected static Predicate<ChangeData> magicLabelPredicate(
+ Args args, MagicLabelVote mlv, @Nullable Integer count) {
if (args.accounts == null || args.accounts.isEmpty()) {
- return new MagicLabelPredicate(args, mlv, /* account= */ null);
+ return new MagicLabelPredicate(args, mlv, /* account= */ null, count);
}
List<Predicate<ChangeData>> r = new ArrayList<>();
for (Account.Id a : args.accounts) {
- r.add(new MagicLabelPredicate(args, mlv, a));
+ r.add(new MagicLabelPredicate(args, mlv, a, count));
}
return or(r);
}
+ private static List<Integer> getCounts(
+ @Nullable Integer count, @Nullable PredicateArgs.Operator countOp) {
+ List<Integer> result = new ArrayList<>();
+ if (count == null) {
+ return result;
+ }
+ switch (countOp) {
+ case EQUAL:
+ case GREATER_EQUAL:
+ case LESS_EQUAL:
+ result.add(count);
+ break;
+ default:
+ break;
+ }
+ switch (countOp) {
+ case GREATER:
+ case GREATER_EQUAL:
+ IntStream.range(count + 1, MAX_COUNT + 1).forEach(result::add);
+ break;
+ case LESS:
+ case LESS_EQUAL:
+ IntStream.range(0, count).forEach(result::add);
+ break;
+ default:
+ break;
+ }
+ return result;
+ }
+
@Override
public String toString() {
return ChangeQueryBuilder.FIELD_LABEL + ":" + value;
diff --git a/java/com/google/gerrit/server/query/change/MagicLabelPredicate.java b/java/com/google/gerrit/server/query/change/MagicLabelPredicate.java
index 3917c79..5a81ca1 100644
--- a/java/com/google/gerrit/server/query/change/MagicLabelPredicate.java
+++ b/java/com/google/gerrit/server/query/change/MagicLabelPredicate.java
@@ -14,6 +14,7 @@
package com.google.gerrit.server.query.change;
+import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.LabelType;
@@ -30,13 +31,21 @@
protected final LabelPredicate.Args args;
private final MagicLabelVote magicLabelVote;
private final Account.Id account;
+ @Nullable private final Integer count;
public MagicLabelPredicate(
- LabelPredicate.Args args, MagicLabelVote magicLabelVote, Account.Id account) {
- super(ChangeField.LABEL, magicLabelVote.formatLabel());
+ LabelPredicate.Args args,
+ MagicLabelVote magicLabelVote,
+ Account.Id account,
+ @Nullable Integer count) {
+ super(
+ ChangeField.LABEL,
+ ChangeField.formatLabel(
+ magicLabelVote.label(), magicLabelVote.value().name(), account, count));
this.account = account;
this.args = args;
this.magicLabelVote = magicLabelVote;
+ this.count = count;
}
@Override
@@ -87,7 +96,7 @@
}
private EqualsLabelPredicate numericPredicate(String label, short value) {
- return new EqualsLabelPredicate(args, label, value, account);
+ return new EqualsLabelPredicate(args, label, value, account, count);
}
protected static LabelType type(LabelTypes types, String toFind) {
diff --git a/java/com/google/gerrit/server/query/change/PredicateArgs.java b/java/com/google/gerrit/server/query/change/PredicateArgs.java
index d82b9bc..9f0dffb 100644
--- a/java/com/google/gerrit/server/query/change/PredicateArgs.java
+++ b/java/com/google/gerrit/server/query/change/PredicateArgs.java
@@ -14,12 +14,15 @@
package com.google.gerrit.server.query.change;
+import com.google.auto.value.AutoValue;
import com.google.common.base.Splitter;
import com.google.gerrit.index.query.QueryParseException;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
/**
* This class is used to extract comma separated values in a predicate.
@@ -30,8 +33,35 @@
* appear in the map and others in the positional list (e.g. "vote=approved,jb_2.3).
*/
public class PredicateArgs {
+ private static final Pattern SPLIT_PATTERN = Pattern.compile("(>|>=|=|<|<=)([^=].*)$");
+
public List<String> positional;
- public Map<String, String> keyValue;
+ public Map<String, ValOp> keyValue;
+
+ enum Operator {
+ EQUAL("="),
+ GREATER_EQUAL(">="),
+ GREATER(">"),
+ LESS_EQUAL("<="),
+ LESS("<");
+
+ final String op;
+
+ Operator(String op) {
+ this.op = op;
+ }
+ };
+
+ @AutoValue
+ public abstract static class ValOp {
+ abstract String value();
+
+ abstract Operator operator();
+
+ static ValOp create(String value, Operator operator) {
+ return new AutoValue_PredicateArgs_ValOp(value, operator);
+ }
+ }
/**
* Parses query arguments into {@link #keyValue} and/or {@link #positional}..
@@ -46,19 +76,39 @@
keyValue = new HashMap<>();
for (String arg : Splitter.on(',').split(args)) {
- List<String> splitKeyValue = Splitter.on('=').splitToList(arg);
+ Matcher m = SPLIT_PATTERN.matcher(arg);
- if (splitKeyValue.size() == 1) {
- positional.add(splitKeyValue.get(0));
- } else if (splitKeyValue.size() == 2) {
- if (!keyValue.containsKey(splitKeyValue.get(0))) {
- keyValue.put(splitKeyValue.get(0), splitKeyValue.get(1));
+ if (!m.find()) {
+ positional.add(arg);
+ } else if (m.groupCount() == 2) {
+ String key = arg.substring(0, m.start());
+ String op = m.group(1);
+ String val = m.group(2);
+ if (!keyValue.containsKey(key)) {
+ keyValue.put(key, ValOp.create(val, getOperator(op)));
} else {
- throw new QueryParseException("Duplicate key " + splitKeyValue.get(0));
+ throw new QueryParseException("Duplicate key " + key);
}
} else {
- throw new QueryParseException("invalid arg " + arg);
+ throw new QueryParseException("Invalid arg " + arg);
}
}
}
+
+ private Operator getOperator(String operator) {
+ switch (operator) {
+ case "<":
+ return Operator.LESS;
+ case "<=":
+ return Operator.LESS_EQUAL;
+ case "=":
+ return Operator.EQUAL;
+ case ">=":
+ return Operator.GREATER_EQUAL;
+ case ">":
+ return Operator.GREATER;
+ default:
+ throw new IllegalArgumentException("Invalid Operator " + operator);
+ }
+ }
}
diff --git a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
index 26bb7a0..5253a5b 100644
--- a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
+++ b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
@@ -41,6 +41,9 @@
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
+import com.google.common.collect.Multimap;
+import com.google.common.collect.Multimaps;
import com.google.common.collect.Streams;
import com.google.common.truth.ThrowableSubject;
import com.google.gerrit.acceptance.ExtensionRegistry;
@@ -143,7 +146,6 @@
import java.util.Arrays;
import java.util.Collection;
import java.util.Iterator;
-import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.concurrent.TimeUnit;
@@ -1040,6 +1042,7 @@
ChangeInserter ins3 = newChange(repo);
ChangeInserter ins4 = newChange(repo);
ChangeInserter ins5 = newChange(repo);
+ ChangeInserter ins6 = newChange(repo);
Change reviewMinus2Change = insert(repo, ins);
gApi.changes().id(reviewMinus2Change.getId().get()).current().review(ReviewInput.reject());
@@ -1052,7 +1055,13 @@
Change reviewPlus1Change = insert(repo, ins4);
gApi.changes().id(reviewPlus1Change.getId().get()).current().review(ReviewInput.recommend());
- Change reviewPlus2Change = insert(repo, ins5);
+ Change reviewTwoPlus1Change = insert(repo, ins5);
+ gApi.changes().id(reviewTwoPlus1Change.getId().get()).current().review(ReviewInput.recommend());
+ requestContext.setContext(newRequestContext(createAccount("user1")));
+ gApi.changes().id(reviewTwoPlus1Change.getId().get()).current().review(ReviewInput.recommend());
+ requestContext.setContext(newRequestContext(userId));
+
+ Change reviewPlus2Change = insert(repo, ins6);
gApi.changes().id(reviewPlus2Change.getId().get()).current().review(ReviewInput.approve());
Map<String, Short> m =
@@ -1063,8 +1072,10 @@
assertThat(m).hasSize(1);
assertThat(m).containsEntry("Code-Review", Short.valueOf((short) 1));
- Map<Integer, Change> changes = new LinkedHashMap<>(5);
+ Multimap<Integer, Change> changes =
+ Multimaps.newListMultimap(Maps.newLinkedHashMap(), () -> Lists.newArrayList());
changes.put(2, reviewPlus2Change);
+ changes.put(1, reviewTwoPlus1Change);
changes.put(1, reviewPlus1Change);
changes.put(0, noLabelChange);
changes.put(-1, reviewMinus1Change);
@@ -1076,9 +1087,9 @@
assertQuery("label:Code-Review=-1", reviewMinus1Change);
assertQuery("label:Code-Review-1", reviewMinus1Change);
assertQuery("label:Code-Review=0", noLabelChange);
- assertQuery("label:Code-Review=+1", reviewPlus1Change);
- assertQuery("label:Code-Review=1", reviewPlus1Change);
- assertQuery("label:Code-Review+1", reviewPlus1Change);
+ assertQuery("label:Code-Review=+1", reviewTwoPlus1Change, reviewPlus1Change);
+ assertQuery("label:Code-Review=1", reviewTwoPlus1Change, reviewPlus1Change);
+ assertQuery("label:Code-Review+1", reviewTwoPlus1Change, reviewPlus1Change);
assertQuery("label:Code-Review=+2", reviewPlus2Change);
assertQuery("label:Code-Review=2", reviewPlus2Change);
assertQuery("label:Code-Review+2", reviewPlus2Change);
@@ -1086,6 +1097,7 @@
assertQuery(
"label:Code-Review=ANY",
reviewPlus2Change,
+ reviewTwoPlus1Change,
reviewPlus1Change,
reviewMinus1Change,
reviewMinus2Change);
@@ -1114,14 +1126,70 @@
assertQuery("label:Code-Review<-2");
assertQuery("label:Code-Review=+1,anotheruser");
- assertQuery("label:Code-Review=+1,user", reviewPlus1Change);
- assertQuery("label:Code-Review=+1,user=user", reviewPlus1Change);
- assertQuery("label:Code-Review=+1,Administrators", reviewPlus1Change);
- assertQuery("label:Code-Review=+1,group=Administrators", reviewPlus1Change);
- assertQuery("label:Code-Review=+1,user=owner", reviewPlus1Change);
- assertQuery("label:Code-Review=+1,owner", reviewPlus1Change);
+ assertQuery("label:Code-Review=+1,user", reviewTwoPlus1Change, reviewPlus1Change);
+ assertQuery("label:Code-Review=+1,user=user", reviewTwoPlus1Change, reviewPlus1Change);
+ assertQuery("label:Code-Review=+1,Administrators", reviewTwoPlus1Change, reviewPlus1Change);
+ assertQuery(
+ "label:Code-Review=+1,group=Administrators", reviewTwoPlus1Change, reviewPlus1Change);
+ assertQuery("label:Code-Review=+1,user=owner", reviewTwoPlus1Change, reviewPlus1Change);
+ assertQuery("label:Code-Review=+1,owner", reviewTwoPlus1Change, reviewPlus1Change);
assertQuery("label:Code-Review=+2,owner", reviewPlus2Change);
assertQuery("label:Code-Review=-2,owner", reviewMinus2Change);
+
+ // count=0 is not allowed
+ Exception thrown =
+ assertThrows(BadRequestException.class, () -> assertQuery("label:Code-Review=+2,count=0"));
+ assertThat(thrown).hasMessageThat().isEqualTo("Argument count=0 is not allowed.");
+ assertQuery("label:Code-Review=1,count=1", reviewPlus1Change);
+ assertQuery("label:Code-Review=1,count=2", reviewTwoPlus1Change);
+ assertQuery("label:Code-Review=1,count>=2", reviewTwoPlus1Change);
+ assertQuery("label:Code-Review=1,count>1", reviewTwoPlus1Change);
+ assertQuery("label:Code-Review=1,count>=1", reviewTwoPlus1Change, reviewPlus1Change);
+ assertQuery("label:Code-Review=1,count=3");
+ thrown =
+ assertThrows(BadRequestException.class, () -> assertQuery("label:Code-Review=1,count=7"));
+ assertThat(thrown)
+ .hasMessageThat()
+ .isEqualTo("count=7 is not allowed. Maximum allowed value for count is 5.");
+
+ // Less than operator does not match with changes having count=0 for a specific vote value (i.e.
+ // no votes for that specific value). We do that deliberately since the computation of count=0
+ // for label values is expensive when the change is re-indexed. This is because the operation
+ // requires generating all formats for all {label-type, vote}=0 values that are non-voted for
+ // the change and storing them with the 'count=0' format.
+ assertQuery("label:Code-Review=1,count<5", reviewTwoPlus1Change, reviewPlus1Change);
+ assertQuery("label:Code-Review=1,count<=5", reviewTwoPlus1Change, reviewPlus1Change);
+ assertQuery(
+ "label:Code-Review=1,count<=1", // reviewTwoPlus1Change is not matched since its count=2
+ reviewPlus1Change);
+ assertQuery(
+ "label:Code-Review=1,count<5 label:Code-Review=1,count>=1",
+ reviewTwoPlus1Change,
+ reviewPlus1Change);
+ assertQuery(
+ "label:Code-Review=1,count<=5 label:Code-Review=1,count>=1",
+ reviewTwoPlus1Change,
+ reviewPlus1Change);
+ assertQuery("label:Code-Review=1,count<=1 label:Code-Review=1,count>=1", reviewPlus1Change);
+
+ assertQuery("label:Code-Review=MAX,count=1", reviewPlus2Change);
+ assertQuery("label:Code-Review=MAX,count=2");
+ assertQuery("label:Code-Review=MIN,count=1", reviewMinus2Change);
+ assertQuery("label:Code-Review=MIN,count>1");
+ assertQuery("label:Code-Review=MAX,count<2", reviewPlus2Change);
+ assertQuery("label:Code-Review=MIN,count<1");
+ assertQuery("label:Code-Review=MAX,count<2 label:Code-Review=MAX,count>=1", reviewPlus2Change);
+ assertQuery("label:Code-Review=MIN,count<1 label:Code-Review=MIN,count>=1");
+ assertQuery("label:Code-Review>=+1,count=2", reviewTwoPlus1Change);
+
+ // "count" and "user" args cannot be used simultaneously.
+ assertThrows(
+ BadRequestException.class,
+ () -> assertQuery("label:Code-Review=+1,user=non_uploader,count=2"));
+
+ // "count" and "group" args cannot be used simultaneously.
+ assertThrows(
+ BadRequestException.class, () -> assertQuery("label:Code-Review=+1,group=gerrit,count=2"));
}
@Test
@@ -1226,16 +1294,15 @@
assertQuery("label:Code-Review=+1,non_uploader", reviewPlus1Change);
}
- private Change[] codeReviewInRange(Map<Integer, Change> changes, int start, int end) {
- int size = 0;
- Change[] range = new Change[end - start + 1];
- for (int i : changes.keySet()) {
+ private Change[] codeReviewInRange(Multimap<Integer, Change> changes, int start, int end) {
+ List<Change> range = new ArrayList<>();
+ for (Map.Entry<Integer, Change> entry : changes.entries()) {
+ int i = entry.getKey();
if (i >= start && i <= end) {
- range[size] = changes.get(i);
- size++;
+ range.add(entry.getValue());
}
}
- return range;
+ return range.toArray(new Change[0]);
}
private String createGroup(String name, String owner) throws Exception {
diff --git a/polygerrit-ui/app/elements/admin/gr-create-repo-dialog/gr-create-repo-dialog.ts b/polygerrit-ui/app/elements/admin/gr-create-repo-dialog/gr-create-repo-dialog.ts
index a493747..63f6601 100644
--- a/polygerrit-ui/app/elements/admin/gr-create-repo-dialog/gr-create-repo-dialog.ts
+++ b/polygerrit-ui/app/elements/admin/gr-create-repo-dialog/gr-create-repo-dialog.ts
@@ -20,6 +20,8 @@
import '../../shared/gr-autocomplete/gr-autocomplete';
import '../../shared/gr-button/gr-button';
import '../../shared/gr-select/gr-select';
+import {GrAutocomplete} from '../../shared/gr-autocomplete/gr-autocomplete';
+import {GrSelect} from '../../shared/gr-select/gr-select';
import {PolymerElement} from '@polymer/polymer/polymer-element';
import {htmlTemplate} from './gr-create-repo-dialog_html';
import {encodeURL, getBaseUrl} from '../../../utils/url-util';
@@ -40,6 +42,15 @@
}
}
+export interface GrCreateRepoDialog {
+ $: {
+ initialCommit: GrSelect;
+ parentRepo: GrSelect;
+ repoNameInput: HTMLInputElement;
+ rightsInheritFromInput: GrAutocomplete;
+ };
+}
+
@customElement('gr-create-repo-dialog')
export class GrCreateRepoDialog extends PolymerElement {
static get template() {
diff --git a/polygerrit-ui/app/elements/admin/gr-create-repo-dialog/gr-create-repo-dialog_html.ts b/polygerrit-ui/app/elements/admin/gr-create-repo-dialog/gr-create-repo-dialog_html.ts
index f529ac6..d0a6b7f 100644
--- a/polygerrit-ui/app/elements/admin/gr-create-repo-dialog/gr-create-repo-dialog_html.ts
+++ b/polygerrit-ui/app/elements/admin/gr-create-repo-dialog/gr-create-repo-dialog_html.ts
@@ -36,24 +36,14 @@
<div id="form">
<section>
<span class="title">Repository name</span>
- <iron-input autocomplete="on" bind-value="{{_repoConfig.name}}">
- <input
- is="iron-input"
- id="repoNameInput"
- autocomplete="on"
- bind-value="{{_repoConfig.name}}"
- />
+ <iron-input bind-value="{{_repoConfig.name}}">
+ <input id="repoNameInput" autocomplete="on" />
</iron-input>
</section>
<section>
<span class="title">Default Branch</span>
- <iron-input autocomplete="off" bind-value="{{_defaultBranch}}">
- <input
- is="iron-input"
- id="defaultBranchNameInput"
- autocomplete="off"
- bind-value="{{_defaultBranch}}"
- />
+ <iron-input bind-value="{{_defaultBranch}}">
+ <input id="defaultBranchNameInput" autocomplete="off" />
</iron-input>
</section>
<section>
diff --git a/polygerrit-ui/app/elements/admin/gr-create-repo-dialog/gr-create-repo-dialog_test.js b/polygerrit-ui/app/elements/admin/gr-create-repo-dialog/gr-create-repo-dialog_test.js
deleted file mode 100644
index f1babee..0000000
--- a/polygerrit-ui/app/elements/admin/gr-create-repo-dialog/gr-create-repo-dialog_test.js
+++ /dev/null
@@ -1,80 +0,0 @@
-/**
- * @license
- * Copyright (C) 2017 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-import '../../../test/common-test-setup-karma.js';
-import './gr-create-repo-dialog.js';
-import {stubRestApi} from '../../../test/test-utils.js';
-
-const basicFixture = fixtureFromElement('gr-create-repo-dialog');
-
-suite('gr-create-repo-dialog tests', () => {
- let element;
-
- setup(() => {
- element = basicFixture.instantiate();
- });
-
- test('default values are populated', () => {
- assert.isTrue(element.$.initialCommit.bindValue);
- assert.isFalse(element.$.parentRepo.bindValue);
- });
-
- test('repo created', async () => {
- const configInputObj = {
- name: 'test-repo',
- create_empty_commit: true,
- parent: 'All-Project',
- permissions_only: false,
- };
-
- const saveStub = stubRestApi('createRepo').returns(Promise.resolve({}));
-
- assert.isFalse(element.hasNewRepoName);
-
- element._repoConfig = {
- name: 'test-repo',
- create_empty_commit: true,
- parent: 'All-Project',
- permissions_only: false,
- };
-
- element._repoOwner = 'test';
- element._repoOwnerId = 'testId';
- element._defaultBranch = 'main';
-
- element.$.repoNameInput.bindValue = configInputObj.name;
- element.$.rightsInheritFromInput.bindValue = configInputObj.parent;
- element.$.initialCommit.bindValue =
- configInputObj.create_empty_commit;
- element.$.parentRepo.bindValue =
- configInputObj.permissions_only;
-
- assert.isTrue(element.hasNewRepoName);
-
- assert.deepEqual(element._repoConfig, configInputObj);
-
- await element.handleCreateRepo();
- assert.isTrue(saveStub.lastCall.calledWithExactly(
- {
- ...configInputObj,
- owners: ['testId'],
- branches: ['main'],
- }
- ));
- });
-});
-
diff --git a/polygerrit-ui/app/elements/admin/gr-create-repo-dialog/gr-create-repo-dialog_test.ts b/polygerrit-ui/app/elements/admin/gr-create-repo-dialog/gr-create-repo-dialog_test.ts
new file mode 100644
index 0000000..6485bae
--- /dev/null
+++ b/polygerrit-ui/app/elements/admin/gr-create-repo-dialog/gr-create-repo-dialog_test.ts
@@ -0,0 +1,81 @@
+/**
+ * @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 '../../../test/common-test-setup-karma';
+import './gr-create-repo-dialog';
+import {GrCreateRepoDialog} from './gr-create-repo-dialog';
+import {stubRestApi} from '../../../test/test-utils';
+import {BranchName, GroupId, RepoName} from '../../../types/common';
+
+const basicFixture = fixtureFromElement('gr-create-repo-dialog');
+
+suite('gr-create-repo-dialog tests', () => {
+ let element: GrCreateRepoDialog;
+
+ setup(() => {
+ element = basicFixture.instantiate();
+ });
+
+ test('default values are populated', () => {
+ assert.isTrue(element.$.initialCommit.bindValue);
+ assert.isFalse(element.$.parentRepo.bindValue);
+ });
+
+ test('repo created', async () => {
+ const configInputObj = {
+ name: 'test-repo' as RepoName,
+ create_empty_commit: true,
+ parent: 'All-Project' as RepoName,
+ permissions_only: false,
+ };
+
+ const saveStub = stubRestApi('createRepo').returns(
+ Promise.resolve(new Response())
+ );
+
+ assert.isFalse(element.hasNewRepoName);
+
+ element._repoConfig = {
+ name: 'test-repo' as RepoName,
+ create_empty_commit: true,
+ parent: 'All-Project' as RepoName,
+ permissions_only: false,
+ };
+
+ element._repoOwner = 'test';
+ element._repoOwnerId = 'testId' as GroupId;
+ element._defaultBranch = 'main' as BranchName;
+
+ element.$.repoNameInput.value = configInputObj.name;
+ element.$.rightsInheritFromInput.value = configInputObj.parent;
+ element.$.initialCommit.bindValue = configInputObj.create_empty_commit;
+ element.$.parentRepo.bindValue = configInputObj.permissions_only;
+
+ assert.isTrue(element.hasNewRepoName);
+
+ assert.deepEqual(element._repoConfig, configInputObj);
+
+ await element.handleCreateRepo();
+ assert.isTrue(
+ saveStub.lastCall.calledWithExactly({
+ ...configInputObj,
+ owners: ['testId' as GroupId],
+ branches: ['main' as BranchName],
+ })
+ );
+ });
+});
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_html.ts b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_html.ts
index c59b2e3..cd55e15 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_html.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_html.ts
@@ -46,7 +46,8 @@
width: 100%;
}
.comments,
- .reviewers {
+ .reviewers,
+ .requirements {
white-space: nowrap;
}
.reviewers {
@@ -120,7 +121,7 @@
</style>
<td aria-hidden="true" class="cell leftPadding"></td>
<td class="cell star" hidden$="[[!showStar]]" hidden="">
- <gr-change-star change="{{change}}"></gr-change-star>
+ <gr-change-star change="[[change]]"></gr-change-star>
</td>
<td class="cell number" hidden$="[[!showNumber]]" hidden="">
<a href$="[[changeURL]]">[[change._number]]</a>
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 f0a90f4..ca237d1 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
@@ -43,6 +43,7 @@
import '../gr-reply-dialog/gr-reply-dialog';
import '../gr-thread-list/gr-thread-list';
import '../../checks/gr-checks-tab';
+import {ChangeStarToggleStarDetail} from '../../shared/gr-change-star/gr-change-star';
import {flush} from '@polymer/polymer/lib/legacy/polymer.dom';
import {PolymerElement} from '@polymer/polymer/polymer-element';
import {htmlTemplate} from './gr-change-view_html';
@@ -622,6 +623,12 @@
private diffViewMode?: DiffViewMode;
+ /**
+ * If the user comes back to the change page we want to remember the scroll
+ * position when we re-render the page as is.
+ */
+ private scrollPosition?: number;
+
override ready() {
super.ready();
aPluginHasRegistered$.pipe(takeUntil(this.disconnected$)).subscribe(b => {
@@ -710,6 +717,7 @@
this.addEventListener('open-fix-preview', e => this._onOpenFixPreview(e));
this.addEventListener('close-fix-preview', e => this._onCloseFixPreview(e));
document.addEventListener('visibilitychange', this.handleVisibilityChange);
+ document.addEventListener('scroll', this.handleScroll);
this.addEventListener(EventType.SHOW_PRIMARY_TAB, e =>
this._setActivePrimaryTab(e)
@@ -728,6 +736,7 @@
'visibilitychange',
this.handleVisibilityChange
);
+ document.removeEventListener('scroll', this.handleScroll);
this.replyRefitTask?.cancel();
this.scrollTask?.cancel();
@@ -745,6 +754,15 @@
return this.shadowRoot!.querySelector<GrThreadList>('gr-thread-list');
}
+ private readonly handleScroll = () => {
+ if (!this.isViewCurrent) return;
+ this.scrollTask = debounce(
+ this.scrollTask,
+ () => (this.scrollPosition = document.documentElement.scrollTop),
+ 150
+ );
+ };
+
_onOpenFixPreview(e: OpenFixPreviewEvent) {
this.$.applyFixDialog.open(e);
}
@@ -1250,6 +1268,7 @@
// If there is no change in patchset or changeNum, such as when user goes
// to the diff view and then comes back to change page then there is no
// need to reload anything and we render the change view component as is.
+ document.documentElement.scrollTop = this.scrollPosition ?? 0;
return;
}
@@ -2531,7 +2550,7 @@
this.$.replyOverlay.setFocusStops(dialog.getFocusStops());
}
- _handleToggleStar(e: CustomEvent<{change: ChangeInfo; starred: boolean}>) {
+ _handleToggleStar(e: CustomEvent<ChangeStarToggleStarDetail>) {
if (e.detail.starred) {
this.reporting.reportInteraction('change-starred-from-change-view');
this.lastStarredTimestamp = Date.now();
@@ -2609,6 +2628,9 @@
}
declare global {
+ interface HTMLElementEventMap {
+ 'toggle-star': CustomEvent<ChangeStarToggleStarDetail>;
+ }
interface HTMLElementTagNameMap {
'gr-change-view': GrChangeView;
}
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.ts
index 155d817..9341b18 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.ts
@@ -338,7 +338,7 @@
</div>
<gr-change-star
id="changeStar"
- change="{{_change}}"
+ change="[[_change]]"
on-toggle-star="_handleToggleStar"
hidden$="[[!_loggedIn]]"
></gr-change-star>
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts
index 591aa41..fd9ce1e 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts
@@ -106,6 +106,7 @@
import {_testOnly_setState} from '../../../services/user/user-model';
import {FocusTarget, GrReplyDialog} from '../gr-reply-dialog/gr-reply-dialog';
import {GrOverlay} from '../../shared/gr-overlay/gr-overlay';
+import {GrChangeStar} from '../../shared/gr-change-star/gr-change-star';
const pluginApi = _testOnly_initGerritPluginApi();
const fixture = fixtureFromElement('gr-change-view');
@@ -2168,17 +2169,19 @@
});
});
- test('_handleToggleStar called when star is tapped', () => {
+ test('_handleToggleStar called when star is tapped', async () => {
element._change = {
...createChangeViewChange(),
owner: {_account_id: 1 as AccountId},
starred: false,
};
element._loggedIn = true;
- const stub = sinon.stub(element, '_handleToggleStar');
- flush();
+ await flush();
- tap(element.$.changeStar.shadowRoot!.querySelector('button')!);
+ const stub = sinon.stub(element, '_handleToggleStar');
+
+ const changeStar = queryAndAssert<GrChangeStar>(element, '#changeStar');
+ tap(queryAndAssert<HTMLButtonElement>(changeStar, 'button')!);
assert.isTrue(stub.called);
});
diff --git a/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-change.ts b/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-change.ts
index e4703df..744db3b 100644
--- a/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-change.ts
+++ b/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-change.ts
@@ -35,6 +35,9 @@
href?: string;
@property()
+ label?: string;
+
+ @property()
showSubmittableCheck = false;
@property()
@@ -110,7 +113,12 @@
const linkClass = this._computeLinkClass(change);
return html`
<div class="changeContainer">
- <a href="${ifDefined(this.href)}" class="${linkClass}"><slot></slot></a>
+ <a
+ href="${ifDefined(this.href)}"
+ aria-label="${ifDefined(this.label)}"
+ class="${linkClass}"
+ ><slot></slot
+ ></a>
${this.showSubmittableCheck
? html`<span
tabindex="-1"
diff --git a/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.ts b/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.ts
index 7493e2f..963c009 100644
--- a/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.ts
+++ b/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.ts
@@ -33,6 +33,7 @@
import {appContext} from '../../../services/app-context';
import {ParsedChangeInfo} from '../../../types/types';
import {GerritNav} from '../../core/gr-navigation/gr-navigation';
+import {truncatePath} from '../../../utils/path-list-util';
import {pluralize} from '../../../utils/string-util';
import {
changeIsOpen,
@@ -146,6 +147,45 @@
this.conflictingChanges.length,
this.cherryPickChanges.length
);
+
+ const sectionRenderers = [
+ this.renderRelationChain,
+ this.renderSubmittedTogether,
+ this.renderSameTopic,
+ this.renderMergeConflicts,
+ this.renderCherryPicks,
+ ];
+
+ let firstNonEmptySectionFound = false;
+ const sections = [];
+ for (const renderer of sectionRenderers) {
+ const section: TemplateResult<1> | undefined = renderer.call(
+ this,
+ !firstNonEmptySectionFound,
+ sectionSize
+ );
+ firstNonEmptySectionFound = firstNonEmptySectionFound || !!section;
+ sections.push(section);
+ }
+
+ return html`<gr-endpoint-decorator name="related-changes-section">
+ <gr-endpoint-param
+ name="change"
+ .value=${this.change}
+ ></gr-endpoint-param>
+ <gr-endpoint-slot name="top"></gr-endpoint-slot>
+ ${sections}
+ <gr-endpoint-slot name="bottom"></gr-endpoint-slot>
+ </gr-endpoint-decorator>`;
+ }
+
+ private renderRelationChain(
+ isFirst: boolean,
+ sectionSize: (section: Section) => number
+ ) {
+ if (this.relatedChanges.length === 0) {
+ return undefined;
+ }
const relatedChangesMarkersPredicate = this.markersPredicateFactory(
this.relatedChanges.length,
this.relatedChanges.findIndex(relatedChange =>
@@ -158,17 +198,11 @@
this.patchNum,
this.relatedChanges
);
- let firstNonEmptySectionFound = false;
- let isFirstNonEmpty =
- !firstNonEmptySectionFound && !!this.relatedChanges.length;
- firstNonEmptySectionFound = firstNonEmptySectionFound || isFirstNonEmpty;
- const relatedChangeSection = html` <section
- id="relatedChanges"
- ?hidden=${!this.relatedChanges.length}
- >
+
+ return html`<section id="relatedChanges">
<gr-related-collapse
title="Relation chain"
- class="${classMap({first: isFirstNonEmpty})}"
+ class="${classMap({first: isFirst})}"
.length=${this.relatedChanges.length}
.numChangesWhenCollapsed=${sectionSize(Section.RELATED_CHANGES)}
>
@@ -200,8 +234,19 @@
)}
</gr-related-collapse>
</section>`;
+ }
+ private renderSubmittedTogether(
+ isFirst: boolean,
+ sectionSize: (section: Section) => number
+ ) {
const submittedTogetherChanges = this.submittedTogether?.changes ?? [];
+ if (
+ !submittedTogetherChanges.length &&
+ !this.submittedTogether?.non_visible_changes
+ ) {
+ return undefined;
+ }
const countNonVisibleChanges =
this.submittedTogether?.non_visible_changes ?? 0;
const submittedTogetherMarkersPredicate = this.markersPredicateFactory(
@@ -211,19 +256,10 @@
),
sectionSize(Section.SUBMITTED_TOGETHER)
);
- isFirstNonEmpty =
- !firstNonEmptySectionFound &&
- (!!submittedTogetherChanges?.length ||
- !!this.submittedTogether?.non_visible_changes);
- firstNonEmptySectionFound = firstNonEmptySectionFound || isFirstNonEmpty;
- const submittedTogetherSection = html`<section
- id="submittedTogether"
- ?hidden=${!submittedTogetherChanges?.length &&
- !this.submittedTogether?.non_visible_changes}
- >
+ return html`<section id="submittedTogether">
<gr-related-collapse
title="Submitted together"
- class="${classMap({first: isFirstNonEmpty})}"
+ class="${classMap({first: isFirst})}"
.length=${submittedTogetherChanges.length}
.numChangesWhenCollapsed=${sectionSize(Section.SUBMITTED_TOGETHER)}
>
@@ -239,14 +275,14 @@
${this.renderMarkers(
submittedTogetherMarkersPredicate(index)
)}<gr-related-change
+ .label="${this.renderChangeTitle(change)}"
.change="${change}"
.href="${GerritNav.getUrlForChangeById(
change._number,
change.project
)}"
.showSubmittableCheck=${true}
- >${change.project}: ${change.branch}:
- ${change.subject}</gr-related-change
+ >${this.renderChangeLine(change)}</gr-related-change
>
</div>`
)}
@@ -255,22 +291,25 @@
(+ ${pluralize(countNonVisibleChanges, 'non-visible change')})
</div>
</section>`;
+ }
+
+ private renderSameTopic(
+ isFirst: boolean,
+ sectionSize: (section: Section) => number
+ ) {
+ if (!this.sameTopicChanges?.length) {
+ return undefined;
+ }
const sameTopicMarkersPredicate = this.markersPredicateFactory(
this.sameTopicChanges.length,
-1,
sectionSize(Section.SAME_TOPIC)
);
- isFirstNonEmpty =
- !firstNonEmptySectionFound && !!this.sameTopicChanges?.length;
- firstNonEmptySectionFound = firstNonEmptySectionFound || isFirstNonEmpty;
- const sameTopicSection = html`<section
- id="sameTopic"
- ?hidden=${!this.sameTopicChanges?.length}
- >
+ return html`<section id="sameTopic">
<gr-related-collapse
title="Same topic"
- class="${classMap({first: isFirstNonEmpty})}"
+ class="${classMap({first: isFirst})}"
.length=${this.sameTopicChanges.length}
.numChangesWhenCollapsed=${sectionSize(Section.SAME_TOPIC)}
>
@@ -287,33 +326,35 @@
sameTopicMarkersPredicate(index)
)}<gr-related-change
.change="${change}"
+ .label="${this.renderChangeTitle(change)}"
.href="${GerritNav.getUrlForChangeById(
change._number,
change.project
)}"
- >${change.project}: ${change.branch}:
- ${change.subject}</gr-related-change
+ >${this.renderChangeLine(change)}</gr-related-change
>
</div>`
)}
</gr-related-collapse>
</section>`;
+ }
+ private renderMergeConflicts(
+ isFirst: boolean,
+ sectionSize: (section: Section) => number
+ ) {
+ if (!this.conflictingChanges?.length) {
+ return undefined;
+ }
const mergeConflictsMarkersPredicate = this.markersPredicateFactory(
this.conflictingChanges.length,
-1,
sectionSize(Section.MERGE_CONFLICTS)
);
- isFirstNonEmpty =
- !firstNonEmptySectionFound && !!this.conflictingChanges?.length;
- firstNonEmptySectionFound = firstNonEmptySectionFound || isFirstNonEmpty;
- const mergeConflictsSection = html`<section
- id="mergeConflicts"
- ?hidden=${!this.conflictingChanges?.length}
- >
+ return html`<section id="mergeConflicts">
<gr-related-collapse
title="Merge conflicts"
- class="${classMap({first: isFirstNonEmpty})}"
+ class="${classMap({first: isFirst})}"
.length=${this.conflictingChanges.length}
.numChangesWhenCollapsed=${sectionSize(Section.MERGE_CONFLICTS)}
>
@@ -340,22 +381,24 @@
)}
</gr-related-collapse>
</section>`;
+ }
+ private renderCherryPicks(
+ isFirst: boolean,
+ sectionSize: (section: Section) => number
+ ) {
+ if (!this.cherryPickChanges.length) {
+ return undefined;
+ }
const cherryPicksMarkersPredicate = this.markersPredicateFactory(
this.cherryPickChanges.length,
-1,
sectionSize(Section.CHERRY_PICKS)
);
- isFirstNonEmpty =
- !firstNonEmptySectionFound && !!this.cherryPickChanges?.length;
- firstNonEmptySectionFound = firstNonEmptySectionFound || isFirstNonEmpty;
- const cherryPicksSection = html`<section
- id="cherryPicks"
- ?hidden=${!this.cherryPickChanges?.length}
- >
+ return html`<section id="cherryPicks">
<gr-related-collapse
title="Cherry picks"
- class="${classMap({first: isFirstNonEmpty})}"
+ class="${classMap({first: isFirst})}"
.length=${this.cherryPickChanges.length}
.numChangesWhenCollapsed=${sectionSize(Section.CHERRY_PICKS)}
>
@@ -382,17 +425,17 @@
)}
</gr-related-collapse>
</section>`;
+ }
- return html`<gr-endpoint-decorator name="related-changes-section">
- <gr-endpoint-param
- name="change"
- .value=${this.change}
- ></gr-endpoint-param>
- <gr-endpoint-slot name="top"></gr-endpoint-slot>
- ${relatedChangeSection} ${submittedTogetherSection} ${sameTopicSection}
- ${mergeConflictsSection} ${cherryPicksSection}
- <gr-endpoint-slot name="bottom"></gr-endpoint-slot>
- </gr-endpoint-decorator>`;
+ private renderChangeTitle(change: ChangeInfo) {
+ return `${change.project}: ${change.branch}: ${change.subject}`;
+ }
+
+ private renderChangeLine(change: ChangeInfo) {
+ const truncatedRepo = truncatePath(change.project, 2);
+ return html`<span class="truncatedRepo" .title="${change.project}"
+ >${truncatedRepo}</span
+ >: ${change.branch}: ${change.subject}`;
}
sectionSizeFactory(
diff --git a/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list_test.ts b/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list_test.ts
index a6dc338f..d0b56fb 100644
--- a/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list_test.ts
@@ -30,6 +30,7 @@
createSubmittedTogetherInfo,
} from '../../../test/test-data-generators';
import {
+ query,
queryAndAssert,
resetPlugins,
stubRestApi,
@@ -227,11 +228,8 @@
Promise.resolve(submittedTogether)
);
await element.reload();
- const relatedChanges = queryAndAssert<GrRelatedCollapse>(
- queryAndAssert<HTMLElement>(element, '#relatedChanges'),
- 'gr-related-collapse'
- );
- assert.isFalse(relatedChanges!.classList.contains('first'));
+ const relatedChanges = query<HTMLElement>(element, '#relatedChanges');
+ assert.notExists(relatedChanges);
const submittedTogetherSection = queryAndAssert<GrRelatedCollapse>(
queryAndAssert<HTMLElement>(element, '#submittedTogether'),
'gr-related-collapse'
@@ -255,11 +253,11 @@
'gr-related-collapse'
);
assert.isTrue(relatedChanges!.classList.contains('first'));
- const submittedTogetherSection = queryAndAssert<GrRelatedCollapse>(
- queryAndAssert<HTMLElement>(element, '#submittedTogether'),
- 'gr-related-collapse'
+ const submittedTogetherSection = query<HTMLElement>(
+ element,
+ '#submittedTogether'
);
- assert.isFalse(submittedTogetherSection!.classList.contains('first'));
+ assert.notExists(submittedTogetherSection);
const cherryPicks = queryAndAssert<GrRelatedCollapse>(
queryAndAssert<HTMLElement>(element, '#cherryPicks'),
'gr-related-collapse'
diff --git a/polygerrit-ui/app/elements/diff/gr-syntax-layer/gr-syntax-layer.ts b/polygerrit-ui/app/elements/diff/gr-syntax-layer/gr-syntax-layer.ts
index ad671da..dddd6a4 100644
--- a/polygerrit-ui/app/elements/diff/gr-syntax-layer/gr-syntax-layer.ts
+++ b/polygerrit-ui/app/elements/diff/gr-syntax-layer/gr-syntax-layer.ts
@@ -54,6 +54,7 @@
['text/x-erlang', 'erlang'],
['text/x-fortran', 'fortran'],
['text/x-fsharp', 'fsharp'],
+ ['text/x-gherkin', 'gherkin'],
['text/x-go', 'go'],
['text/x-groovy', 'groovy'],
['text/x-haml', 'haml'],
diff --git a/polygerrit-ui/app/elements/shared/gr-change-star/gr-change-star.ts b/polygerrit-ui/app/elements/shared/gr-change-star/gr-change-star.ts
index a23621e..c6fd01c 100644
--- a/polygerrit-ui/app/elements/shared/gr-change-star/gr-change-star.ts
+++ b/polygerrit-ui/app/elements/shared/gr-change-star/gr-change-star.ts
@@ -15,10 +15,6 @@
* limitations under the License.
*/
import '../gr-icons/gr-icons';
-import '../../../styles/shared-styles';
-import {PolymerElement} from '@polymer/polymer/polymer-element';
-import {htmlTemplate} from './gr-change-star_html';
-import {customElement, property} from '@polymer/decorators';
import {ChangeInfo} from '../../../types/common';
import {fireAlert} from '../../../utils/event-util';
import {
@@ -26,6 +22,9 @@
ShortcutSection,
} from '../../../services/shortcuts/shortcuts-config';
import {appContext} from '../../../services/app-context';
+import {sharedStyles} from '../../../styles/shared-styles';
+import {LitElement, css, html} from 'lit';
+import {customElement, property} from 'lit/decorators';
declare global {
interface HTMLElementTagNameMap {
@@ -39,44 +38,78 @@
}
@customElement('gr-change-star')
-export class GrChangeStar extends PolymerElement {
- static get template() {
- return htmlTemplate;
- }
-
+export class GrChangeStar extends LitElement {
/**
* Fired when star state is toggled.
*
* @event toggle-star
*/
- @property({type: Object, notify: true})
+ @property({type: Object})
change?: ChangeInfo;
private readonly shortcuts = appContext.shortcutsService;
- _computeStarClass(starred?: boolean) {
- return starred ? 'active' : '';
+ static override get styles() {
+ return [
+ sharedStyles,
+ css`
+ button {
+ background-color: transparent;
+ cursor: pointer;
+ }
+ iron-icon.active {
+ fill: var(--link-color);
+ }
+ iron-icon {
+ vertical-align: top;
+ --iron-icon-height: var(
+ --gr-change-star-size,
+ var(--line-height-normal, 20px)
+ );
+ --iron-icon-width: var(
+ --gr-change-star-size,
+ var(--line-height-normal, 20px)
+ );
+ }
+ :host([hidden]) {
+ visibility: hidden;
+ display: block !important;
+ }
+ `,
+ ];
}
- _computeStarIcon(starred?: boolean) {
- // Hollow star is used to indicate inactive state.
- return `gr-icons:star${starred ? '' : '-border'}`;
- }
-
- _computeAriaLabel(starred?: boolean) {
- return starred ? 'Unstar this change' : 'Star this change';
+ override render() {
+ return html`
+ <button
+ role="checkbox"
+ title=${this.shortcuts.createTitle(
+ Shortcut.TOGGLE_CHANGE_STAR,
+ ShortcutSection.ACTIONS
+ )}
+ aria-label=${this.change?.starred
+ ? 'Unstar this change'
+ : 'Star this change'}
+ @click=${this.toggleStar}
+ >
+ <iron-icon
+ class=${this.change?.starred ? 'active' : ''}
+ .icon=${`gr-icons:star${this.change?.starred ? '' : '-border'}`}
+ ></iron-icon>
+ </button>
+ `;
}
toggleStar() {
// Note: change should always be defined when use gr-change-star
// but since we don't have a good way to enforce usage to always
// set the change, we still check it here.
- if (!this.change) {
- return;
- }
+ if (!this.change) return;
+
const newVal = !this.change.starred;
- this.set('change.starred', newVal);
+ this.change.starred = newVal;
+ this.requestUpdate('change');
const detail: ChangeStarToggleStarDetail = {
change: this.change,
starred: newVal,
@@ -90,8 +123,4 @@
})
);
}
-
- createTitle(shortcutName: Shortcut, section: ShortcutSection) {
- return this.shortcuts.createTitle(shortcutName, section);
- }
}
diff --git a/polygerrit-ui/app/elements/shared/gr-change-star/gr-change-star_html.ts b/polygerrit-ui/app/elements/shared/gr-change-star/gr-change-star_html.ts
deleted file mode 100644
index d404795..0000000
--- a/polygerrit-ui/app/elements/shared/gr-change-star/gr-change-star_html.ts
+++ /dev/null
@@ -1,56 +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="shared-styles">
- button {
- background-color: transparent;
- cursor: pointer;
- }
- iron-icon.active {
- fill: var(--link-color);
- }
- iron-icon {
- vertical-align: top;
- --iron-icon-height: var(
- --gr-change-star-size,
- var(--line-height-normal, 20px)
- );
- --iron-icon-width: var(
- --gr-change-star-size,
- var(--line-height-normal, 20px)
- );
- }
- :host([hidden]) {
- visibility: hidden;
- display: block !important;
- }
- </style>
- <button
- role="checkbox"
- title="[[createTitle(Shortcut.TOGGLE_CHANGE_STAR,
- ShortcutSection.ACTIONS)]]"
- aria-label="[[_computeAriaLabel(change.starred)]]"
- on-click="toggleStar"
- >
- <iron-icon
- class$="[[_computeStarClass(change.starred)]]"
- icon$="[[_computeStarIcon(change.starred)]]"
- ></iron-icon>
- </button>
-`;
diff --git a/polygerrit-ui/app/elements/shared/gr-change-star/gr-change-star_test.ts b/polygerrit-ui/app/elements/shared/gr-change-star/gr-change-star_test.ts
index 8f411ae..2c5d7a2 100644
--- a/polygerrit-ui/app/elements/shared/gr-change-star/gr-change-star_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-change-star/gr-change-star_test.ts
@@ -27,45 +27,47 @@
suite('gr-change-star tests', () => {
let element: GrChangeStar;
- setup(() => {
+ setup(async () => {
element = basicFixture.instantiate();
element.change = {
...createChange(),
starred: true,
};
+ await element.updateComplete;
});
test('star visibility states', async () => {
- element.set('change.starred', true);
- await flush();
+ element.change!.starred = true;
+ await element.updateComplete;
let icon = queryAndAssert<IronIconElement>(element, 'iron-icon');
assert.isTrue(icon.classList.contains('active'));
assert.equal(icon.icon, 'gr-icons:star');
- element.set('change.starred', false);
- await flush();
+ element.change!.starred = false;
+ element.requestUpdate('change');
+ await element.updateComplete;
icon = queryAndAssert<IronIconElement>(element, 'iron-icon');
assert.isFalse(icon.classList.contains('active'));
assert.equal(icon.icon, 'gr-icons:star-border');
});
test('starring', async () => {
- element.set('change.starred', false);
- await flush();
+ element.change!.starred = false;
+ await element.updateComplete;
assert.equal(element.change!.starred, false);
- MockInteractions.tap(queryAndAssert(element, 'button'));
- await flush();
+ MockInteractions.tap(queryAndAssert<HTMLButtonElement>(element, 'button'));
+ await element.updateComplete;
assert.equal(element.change!.starred, true);
});
test('unstarring', async () => {
- element.set('change.starred', true);
- await flush();
+ element.change!.starred = true;
+ await element.updateComplete;
assert.equal(element.change!.starred, true);
- MockInteractions.tap(queryAndAssert(element, 'button'));
- await flush();
+ MockInteractions.tap(queryAndAssert<HTMLButtonElement>(element, 'button'));
+ await element.updateComplete;
assert.equal(element.change!.starred, false);
});
});
diff --git a/polygerrit-ui/app/elements/shared/gr-select/gr-select.ts b/polygerrit-ui/app/elements/shared/gr-select/gr-select.ts
index 571272d..47295ab 100644
--- a/polygerrit-ui/app/elements/shared/gr-select/gr-select.ts
+++ b/polygerrit-ui/app/elements/shared/gr-select/gr-select.ts
@@ -34,7 +34,7 @@
}
@property({type: String, notify: true})
- bindValue?: string | number;
+ bindValue?: string | number | boolean;
get nativeSelect() {
// gr-select is not a shadow component
diff --git a/polygerrit-ui/app/elements/shared/gr-tooltip-content/gr-tooltip-content.ts b/polygerrit-ui/app/elements/shared/gr-tooltip-content/gr-tooltip-content.ts
index 0585aec8..4315071 100644
--- a/polygerrit-ui/app/elements/shared/gr-tooltip-content/gr-tooltip-content.ts
+++ b/polygerrit-ui/app/elements/shared/gr-tooltip-content/gr-tooltip-content.ts
@@ -210,13 +210,9 @@
const left = rect.left - parentRect.left + (rect.width - boxRect.width) / 2;
const right = parentRect.width - left - boxRect.width;
if (left < 0) {
- tooltip.updateStyles({
- '--gr-tooltip-arrow-center-offset': `${left}px`,
- });
+ tooltip.arrowCenterOffset = `${left}px`;
} else if (right < 0) {
- tooltip.updateStyles({
- '--gr-tooltip-arrow-center-offset': `${-0.5 * right}px`,
- });
+ tooltip.arrowCenterOffset = `${-0.5 * right}px`;
}
tooltip.style.left = `${Math.max(0, left)}px`;
diff --git a/polygerrit-ui/app/elements/shared/gr-tooltip-content/gr-tooltip-content_test.js b/polygerrit-ui/app/elements/shared/gr-tooltip-content/gr-tooltip-content_test.js
index 8d3bbb0..3b81f46 100644
--- a/polygerrit-ui/app/elements/shared/gr-tooltip-content/gr-tooltip-content_test.js
+++ b/polygerrit-ui/app/elements/shared/gr-tooltip-content/gr-tooltip-content_test.js
@@ -25,11 +25,15 @@
function makeTooltip(tooltipRect, parentRect) {
return {
- getBoundingClientRect() { return tooltipRect; },
- updateStyles: sinon.stub(),
+ arrowCenterOffset: '0',
+ getBoundingClientRect() {
+ return tooltipRect;
+ },
style: {left: 0, top: 0},
parentElement: {
- getBoundingClientRect() { return parentRect; },
+ getBoundingClientRect() {
+ return parentRect;
+ },
},
};
}
@@ -66,12 +70,12 @@
{top: 0, left: 0, width: 1000});
element._positionTooltip(tooltip);
- assert.isFalse(tooltip.updateStyles.called);
+ assert.equal(tooltip.arrowCenterOffset, '0');
assert.equal(tooltip.style.left, '175px');
assert.equal(tooltip.style.top, '100px');
});
- test('left side position', () => {
+ test('left side position', async () => {
sinon.stub(element, 'getBoundingClientRect').callsFake(() => {
return {top: 100, left: 10, width: 50};
});
@@ -80,10 +84,8 @@
{top: 0, left: 0, width: 1000});
element._positionTooltip(tooltip);
- assert.isTrue(tooltip.updateStyles.called);
- const offset = tooltip.updateStyles
- .lastCall.args[0]['--gr-tooltip-arrow-center-offset'];
- assert.isBelow(parseFloat(offset.replace(/px$/, '')), 0);
+ await element.updateComplete;
+ assert.isBelow(parseFloat(tooltip.arrowCenterOffset.replace(/px$/, '')), 0);
assert.equal(tooltip.style.left, '0px');
assert.equal(tooltip.style.top, '100px');
});
@@ -97,10 +99,7 @@
{top: 0, left: 0, width: 1000});
element._positionTooltip(tooltip);
- assert.isTrue(tooltip.updateStyles.called);
- const offset = tooltip.updateStyles
- .lastCall.args[0]['--gr-tooltip-arrow-center-offset'];
- assert.isAbove(parseFloat(offset.replace(/px$/, '')), 0);
+ assert.isAbove(parseFloat(tooltip.arrowCenterOffset.replace(/px$/, '')), 0);
assert.equal(tooltip.style.left, '915px');
assert.equal(tooltip.style.top, '100px');
});
@@ -115,19 +114,16 @@
element.positionBelow = true;
element._positionTooltip(tooltip);
- assert.isTrue(tooltip.updateStyles.called);
- const offset = tooltip.updateStyles
- .lastCall.args[0]['--gr-tooltip-arrow-center-offset'];
- assert.isAbove(parseFloat(offset.replace(/px$/, '')), 0);
+ assert.isAbove(parseFloat(tooltip.arrowCenterOffset.replace(/px$/, '')), 0);
assert.equal(tooltip.style.left, '915px');
assert.equal(tooltip.style.top, '157.2px');
});
test('hides tooltip when detached', async () => {
- sinon.stub(element, '_handleHideTooltip');
+ const handleHideTooltipStub = sinon.stub(element, '_handleHideTooltip');
element.remove();
await element.updateComplete;
- assert.isTrue(element._handleHideTooltip.called);
+ assert.isTrue(handleHideTooltipStub.called);
});
test('sets up listeners when has-tooltip is changed', async () => {
diff --git a/polygerrit-ui/app/elements/shared/gr-tooltip/gr-tooltip.ts b/polygerrit-ui/app/elements/shared/gr-tooltip/gr-tooltip.ts
index cab05b4..0e41891 100644
--- a/polygerrit-ui/app/elements/shared/gr-tooltip/gr-tooltip.ts
+++ b/polygerrit-ui/app/elements/shared/gr-tooltip/gr-tooltip.ts
@@ -14,14 +14,11 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-import '../../../styles/shared-styles';
-import {PolymerElement} from '@polymer/polymer/polymer-element';
-import {htmlTemplate} from './gr-tooltip_html';
-import {customElement, property, observe} from '@polymer/decorators';
-export interface GrTooltip {
- $: {};
-}
+import {sharedStyles} from '../../../styles/shared-styles';
+import {LitElement, css, html} from 'lit';
+import {customElement, property} from 'lit/decorators';
+import {styleMap} from 'lit/directives/style-map';
declare global {
interface HTMLElementTagNameMap {
@@ -30,22 +27,78 @@
}
@customElement('gr-tooltip')
-export class GrTooltip extends PolymerElement {
- static get template() {
- return htmlTemplate;
- }
-
+export class GrTooltip extends LitElement {
@property({type: String})
text = '';
@property({type: String})
maxWidth = '';
- @property({type: Boolean, reflectToAttribute: true})
+ @property({type: String})
+ arrowCenterOffset = '0';
+
+ @property({type: Boolean, reflect: true, attribute: 'position-below'})
positionBelow = false;
- @observe('maxWidth')
- _updateWidth(maxWidth: string) {
- this.updateStyles({'--tooltip-max-width': maxWidth});
+ static override get styles() {
+ return [
+ sharedStyles,
+ css`
+ :host {
+ --gr-tooltip-arrow-size: 0.5em;
+
+ background-color: var(--tooltip-background-color);
+ box-shadow: var(--elevation-level-2);
+ color: var(--tooltip-text-color);
+ font-size: var(--font-size-small);
+ position: absolute;
+ z-index: 1000;
+ }
+ :host .tooltip {
+ padding: var(--spacing-m) var(--spacing-l);
+ }
+ :host .arrowPositionBelow,
+ :host([position-below]) .arrowPositionAbove {
+ display: none;
+ }
+ :host([position-below]) .arrowPositionBelow {
+ display: initial;
+ }
+ .arrow {
+ border-left: var(--gr-tooltip-arrow-size) solid transparent;
+ border-right: var(--gr-tooltip-arrow-size) solid transparent;
+ height: 0;
+ position: absolute;
+ left: calc(50% - var(--gr-tooltip-arrow-size));
+ width: 0;
+ }
+ .arrowPositionAbove {
+ border-top: var(--gr-tooltip-arrow-size) solid
+ var(--tooltip-background-color);
+ bottom: calc(-1 * var(--gr-tooltip-arrow-size));
+ }
+ .arrowPositionBelow {
+ border-bottom: var(--gr-tooltip-arrow-size) solid
+ var(--tooltip-background-color);
+ top: calc(-1 * var(--gr-tooltip-arrow-size));
+ }
+ `,
+ ];
+ }
+
+ override render() {
+ this.style.maxWidth = this.maxWidth;
+
+ return html` <div class="tooltip">
+ <i
+ class="arrowPositionBelow arrow"
+ style="${styleMap({marginLeft: this.arrowCenterOffset})}"
+ ></i>
+ ${this.text}
+ <i
+ class="arrowPositionAbove arrow"
+ style="${styleMap({marginLeft: this.arrowCenterOffset})}"
+ ></i>
+ </div>`;
}
}
diff --git a/polygerrit-ui/app/elements/shared/gr-tooltip/gr-tooltip_html.ts b/polygerrit-ui/app/elements/shared/gr-tooltip/gr-tooltip_html.ts
deleted file mode 100644
index d59a6c3..0000000
--- a/polygerrit-ui/app/elements/shared/gr-tooltip/gr-tooltip_html.ts
+++ /dev/null
@@ -1,68 +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="shared-styles">
- :host {
- --gr-tooltip-arrow-size: 0.5em;
- --gr-tooltip-arrow-center-offset: 0;
-
- background-color: var(--tooltip-background-color);
- box-shadow: var(--elevation-level-2);
- color: var(--tooltip-text-color);
- font-size: var(--font-size-small);
- position: absolute;
- z-index: 1000;
- max-width: var(--tooltip-max-width);
- }
- :host .tooltip {
- padding: var(--spacing-m) var(--spacing-l);
- }
- :host .arrowPositionBelow,
- :host([position-below]) .arrowPositionAbove {
- display: none;
- }
- :host([position-below]) .arrowPositionBelow {
- display: initial;
- }
- .arrow {
- border-left: var(--gr-tooltip-arrow-size) solid transparent;
- border-right: var(--gr-tooltip-arrow-size) solid transparent;
- height: 0;
- position: absolute;
- left: calc(50% - var(--gr-tooltip-arrow-size));
- margin-left: var(--gr-tooltip-arrow-center-offset);
- width: 0;
- }
- .arrowPositionAbove {
- border-top: var(--gr-tooltip-arrow-size) solid
- var(--tooltip-background-color);
- bottom: calc(-1 * var(--gr-tooltip-arrow-size));
- }
- .arrowPositionBelow {
- border-bottom: var(--gr-tooltip-arrow-size) solid
- var(--tooltip-background-color);
- top: calc(-1 * var(--gr-tooltip-arrow-size));
- }
- </style>
- <div class="tooltip">
- <i class="arrowPositionBelow arrow"></i>
- [[text]]
- <i class="arrowPositionAbove arrow"></i>
- </div>
-`;
diff --git a/polygerrit-ui/app/elements/shared/gr-tooltip/gr-tooltip_test.ts b/polygerrit-ui/app/elements/shared/gr-tooltip/gr-tooltip_test.ts
index 8b44047..b693a9e 100644
--- a/polygerrit-ui/app/elements/shared/gr-tooltip/gr-tooltip_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-tooltip/gr-tooltip_test.ts
@@ -17,49 +17,45 @@
import '../../../test/common-test-setup-karma';
import './gr-tooltip';
-import {html} from '@polymer/polymer/lib/utils/html-tag';
import {GrTooltip} from './gr-tooltip';
+import {queryAndAssert} from '../../../test/test-utils';
-const basicFixture = fixtureFromTemplate(html` <gr-tooltip> </gr-tooltip> `);
+const basicFixture = fixtureFromElement('gr-tooltip');
suite('gr-tooltip tests', () => {
let element: GrTooltip;
- setup(() => {
+
+ setup(async () => {
element = basicFixture.instantiate() as GrTooltip;
+ await element.updateComplete;
});
- test('max-width is respected if set', () => {
+ test('max-width is respected if set', async () => {
element.text =
'Lorem ipsum dolor sit amet, consectetur adipiscing elit' +
', sed do eiusmod tempor incididunt ut labore et dolore magna aliqua';
element.maxWidth = '50px';
+ await element.updateComplete;
assert.equal(getComputedStyle(element).width, '50px');
});
- test('the correct arrow is displayed', () => {
+ test('the correct arrow is displayed', async () => {
assert.equal(
- getComputedStyle(
- element.shadowRoot!.querySelector('.arrowPositionBelow')!
- ).display,
+ getComputedStyle(queryAndAssert(element, '.arrowPositionBelow')!).display,
'none'
);
assert.notEqual(
- getComputedStyle(
- element.shadowRoot!.querySelector('.arrowPositionAbove')!
- ).display,
+ getComputedStyle(queryAndAssert(element, '.arrowPositionAbove')!).display,
'none'
);
element.positionBelow = true;
+ await element.updateComplete;
assert.notEqual(
- getComputedStyle(
- element.shadowRoot!.querySelector('.arrowPositionBelow')!
- ).display,
+ getComputedStyle(queryAndAssert(element, '.arrowPositionBelow')!).display,
'none'
);
assert.equal(
- getComputedStyle(
- element.shadowRoot!.querySelector('.arrowPositionAbove')!
- ).display,
+ getComputedStyle(queryAndAssert(element, '.arrowPositionAbove')!).display,
'none'
);
});
diff --git a/resources/com/google/gerrit/server/mime/mime-types.properties b/resources/com/google/gerrit/server/mime/mime-types.properties
index fd2280c..01ca71c 100644
--- a/resources/com/google/gerrit/server/mime/mime-types.properties
+++ b/resources/com/google/gerrit/server/mime/mime-types.properties
@@ -67,6 +67,7 @@
f = text/x-fortran
factor = text/x-factor
feathre = text/x-feature
+feature = text/x-gherkin
fcl = text/x-fcl
for = text/x-fortran
formula = text/x-spreadsheet
