Merge "Fix post submit diff size limit"
diff --git a/Documentation/config-themes.txt b/Documentation/config-themes.txt
index a83c747..73fb1bc 100644
--- a/Documentation/config-themes.txt
+++ b/Documentation/config-themes.txt
@@ -4,11 +4,12 @@
the browser, allowing organizations to alter the look and
feel of the application to fit with their general scheme.
-== HTML Header/Footer and CSS
+== HTML Header/Footer and CSS for login screens
-The HTML header, footer and CSS may be customized for login
-screens (LDAP, OAuth, OpenId) and the internally managed
-Gitweb servlet.
+The HTML header, footer, and CSS may be customized for login screens (LDAP,
+OAuth, OpenId) and the internally managed Gitweb servlet. See
+link:pg-plugin-dev.txt[JavaScript Plugin Development and API] for documentation
+on modifying styles for the rest of Gerrit (not login screens).
At startup Gerrit reads the following files (if they exist) and
uses them to customize the HTML page it sends to clients:
diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt
index 33e7804..64aa6e0 100644
--- a/Documentation/rest-api-changes.txt
+++ b/Documentation/rest-api-changes.txt
@@ -6520,7 +6520,14 @@
|`topic` |optional|The topic to which this change belongs.
|`attention_set` |optional|
The map that maps link:rest-api-accounts.html#account-id[account IDs]
-to link:#attention-set-info[AttentionSetInfo] of that account.
+to link:#attention-set-info[AttentionSetInfo] of that account. Those are all
+accounts that are currently in the attention set.
+|`removed_from_attention_set` |optional|
+The map that maps link:rest-api-accounts.html#account-id[account IDs]
+to link:#attention-set-info[AttentionSetInfo] of that account. Those are all
+accounts that were in the attention set but were removed. The
+link:#attention-set-info[AttentionSetInfo] is the latest and most recent removal
+ of the account from the attention set.
|`assignee` |optional|
The assignee of the change as an link:rest-api-accounts.html#account-info[
AccountInfo] entity.
@@ -7104,6 +7111,9 @@
Additional information about whom to notify about the update as a map
of link:user-notify.html#recipient-types[recipient type] to
link:#notify-info[NotifyInfo] entity.
+|`ignore_automatic_attention_set_rules`|optional|
+If set to true, ignore all automatic attention set rules described in the
+link:#attention-set[attention set]. When not set, the default is false.
|=============================
[[description-input]]
diff --git a/java/com/google/gerrit/entities/Account.java b/java/com/google/gerrit/entities/Account.java
index cd3b27a..18fcef3 100644
--- a/java/com/google/gerrit/entities/Account.java
+++ b/java/com/google/gerrit/entities/Account.java
@@ -196,9 +196,9 @@
* <p>Example output:
*
* <ul>
- * <li>{@code A U. Thor <author@example.com>}: full populated
+ * <li>{@code A U. Thor <author@example.com>}: full populated
* <li>{@code A U. Thor (12)}: missing email address
- * <li>{@code Anonymous Coward <author@example.com>}: missing name
+ * <li>{@code Anonymous Coward <author@example.com>}: missing name
* <li>{@code Anonymous Coward (12)}: missing name and email address
* </ul>
*/
diff --git a/java/com/google/gerrit/extensions/api/changes/DeleteVoteInput.java b/java/com/google/gerrit/extensions/api/changes/DeleteVoteInput.java
index ee10a1d..8432c8f 100644
--- a/java/com/google/gerrit/extensions/api/changes/DeleteVoteInput.java
+++ b/java/com/google/gerrit/extensions/api/changes/DeleteVoteInput.java
@@ -25,4 +25,10 @@
public NotifyHandling notify = NotifyHandling.ALL;
public Map<RecipientType, NotifyInfo> notifyDetails;
+
+ /**
+ * Users in the attention set will not be added/removed from this endpoint call. Normally, users
+ * are added to the attention set upon deletion of their vote by other users.
+ */
+ public boolean ignoreAutomaticAttentionSetRules;
}
diff --git a/java/com/google/gerrit/extensions/common/ChangeInfo.java b/java/com/google/gerrit/extensions/common/ChangeInfo.java
index 2bb3dd7..55e4670 100644
--- a/java/com/google/gerrit/extensions/common/ChangeInfo.java
+++ b/java/com/google/gerrit/extensions/common/ChangeInfo.java
@@ -46,6 +46,8 @@
*/
public Map<Integer, AttentionSetInfo> attentionSet;
+ public Map<Integer, AttentionSetInfo> removedFromAttentionSet;
+
public AccountInfo assignee;
public Collection<String> hashtags;
public String changeId;
diff --git a/java/com/google/gerrit/httpd/raw/StaticModule.java b/java/com/google/gerrit/httpd/raw/StaticModule.java
index aa32169..460ad60 100644
--- a/java/com/google/gerrit/httpd/raw/StaticModule.java
+++ b/java/com/google/gerrit/httpd/raw/StaticModule.java
@@ -79,6 +79,7 @@
"/dashboard/*",
"/groups/self",
"/settings/*",
+ "/topic/*",
"/Documentation/q/*");
/**
diff --git a/java/com/google/gerrit/server/account/AuthRequest.java b/java/com/google/gerrit/server/account/AuthRequest.java
index 50ed532..cceda70 100644
--- a/java/com/google/gerrit/server/account/AuthRequest.java
+++ b/java/com/google/gerrit/server/account/AuthRequest.java
@@ -49,20 +49,20 @@
}
/** Create a request for a local username, such as from LDAP. */
- public AuthRequest createForUser(String username) {
+ public AuthRequest createForUser(String userName) {
AuthRequest r =
new AuthRequest(
- externalIdKeyFactory.create(SCHEME_GERRIT, username), externalIdKeyFactory);
- r.setUserName(username);
+ externalIdKeyFactory.create(SCHEME_GERRIT, userName), externalIdKeyFactory);
+ r.setUserName(userName);
return r;
}
/** Create a request for an external username. */
- public AuthRequest createForExternalUser(String username) {
+ public AuthRequest createForExternalUser(String userName) {
AuthRequest r =
new AuthRequest(
- externalIdKeyFactory.create(SCHEME_EXTERNAL, username), externalIdKeyFactory);
- r.setUserName(username);
+ externalIdKeyFactory.create(SCHEME_EXTERNAL, userName), externalIdKeyFactory);
+ r.setUserName(userName);
return r;
}
diff --git a/java/com/google/gerrit/server/change/ActionJson.java b/java/com/google/gerrit/server/change/ActionJson.java
index 54ebf40..ffbd30b 100644
--- a/java/com/google/gerrit/server/change/ActionJson.java
+++ b/java/com/google/gerrit/server/change/ActionJson.java
@@ -118,6 +118,10 @@
copy.topic = changeInfo.topic;
copy.attentionSet =
changeInfo.attentionSet == null ? null : ImmutableMap.copyOf(changeInfo.attentionSet);
+ copy.removedFromAttentionSet =
+ changeInfo.removedFromAttentionSet == null
+ ? null
+ : ImmutableMap.copyOf(changeInfo.removedFromAttentionSet);
copy.assignee = changeInfo.assignee;
copy.hashtags = changeInfo.hashtags;
copy.changeId = changeInfo.changeId;
diff --git a/java/com/google/gerrit/server/change/ChangeJson.java b/java/com/google/gerrit/server/change/ChangeJson.java
index 328c5de..e57238b 100644
--- a/java/com/google/gerrit/server/change/ChangeJson.java
+++ b/java/com/google/gerrit/server/change/ChangeJson.java
@@ -35,6 +35,7 @@
import static com.google.gerrit.extensions.client.ListChangesOption.TRACKING_IDS;
import static com.google.gerrit.server.ChangeMessagesUtil.createChangeMessageInfo;
import static com.google.gerrit.server.util.AttentionSetUtil.additionsOnly;
+import static com.google.gerrit.server.util.AttentionSetUtil.removalsOnly;
import static java.util.stream.Collectors.toList;
import com.google.common.base.Joiner;
@@ -603,6 +604,12 @@
out.branch = in.getDest().shortName();
out.topic = in.getTopic();
if (!cd.attentionSet().isEmpty()) {
+ out.removedFromAttentionSet =
+ removalsOnly(cd.attentionSet()).stream()
+ .collect(
+ toImmutableMap(
+ a -> a.account().get(),
+ a -> AttentionSetUtil.createAttentionSetInfo(a, accountLoader)));
out.attentionSet =
// This filtering should match GetAttentionSet.
additionsOnly(cd.attentionSet()).stream()
diff --git a/java/com/google/gerrit/server/change/LabelsJson.java b/java/com/google/gerrit/server/change/LabelsJson.java
index 5ce121b..77f6278 100644
--- a/java/com/google/gerrit/server/change/LabelsJson.java
+++ b/java/com/google/gerrit/server/change/LabelsJson.java
@@ -95,53 +95,44 @@
return ImmutableMap.copyOf(Maps.transformValues(withStatus, LabelWithStatus::label));
}
- /** Returns all labels that the provided user has permission to vote on. */
+ /**
+ * Returns A map of all label names and the values that the provided user has permission to vote
+ * on.
+ *
+ * @param filterApprovalsBy a Gerrit user ID.
+ * @param cd {@link ChangeData} corresponding to a specific gerrit change.
+ * @return A Map where the key contain a label name, and the value is a list of the permissible
+ * vote values that the user can vote on.
+ */
Map<String, Collection<String>> permittedLabels(Account.Id filterApprovalsBy, ChangeData cd)
throws PermissionBackendException {
- boolean isMerged = cd.change().isMerged();
- LabelTypes labelTypes = cd.getLabelTypes();
- Map<String, LabelType> toCheck = new HashMap<>();
- for (SubmitRecord rec : submitRecords(cd)) {
- if (rec.labels != null) {
- for (SubmitRecord.Label r : rec.labels) {
- Optional<LabelType> type = labelTypes.byLabel(r.label);
- if (type.isPresent() && (!isMerged || type.get().isAllowPostSubmit())) {
- toCheck.put(type.get().getName(), type.get());
- }
- }
- }
- }
-
- Map<String, Short> labels = null;
- Set<LabelPermission.WithValue> can =
- permissionBackend.absentUser(filterApprovalsBy).change(cd).testLabels(toCheck.values());
SetMultimap<String, String> permitted = LinkedHashMultimap.create();
- for (SubmitRecord rec : submitRecords(cd)) {
- if (rec.labels == null) {
+ boolean isMerged = cd.change().isMerged();
+ Map<String, Short> currentUserVotes = currentLabels(filterApprovalsBy, cd);
+ for (LabelType labelType : cd.getLabelTypes().getLabelTypes()) {
+ if (isMerged && !labelType.isAllowPostSubmit()) {
continue;
}
- for (SubmitRecord.Label r : rec.labels) {
- Optional<LabelType> type = labelTypes.byLabel(r.label);
- if (!type.isPresent() || (isMerged && !type.get().isAllowPostSubmit())) {
- continue;
+ Set<LabelPermission.WithValue> can =
+ permissionBackend.absentUser(filterApprovalsBy).change(cd).test(labelType);
+ for (LabelValue v : labelType.getValues()) {
+ boolean ok = can.contains(new LabelPermission.WithValue(labelType, v));
+ if (isMerged) {
+ // Votes cannot be decreased if the change is merged. Only accept the label value if it's
+ // greater or equal than the user's latest vote.
+ short prev = currentUserVotes.getOrDefault(labelType.getName(), (short) 0);
+ ok &= v.getValue() >= prev;
}
-
- for (LabelValue v : type.get().getValues()) {
- boolean ok = can.contains(new LabelPermission.WithValue(type.get(), v));
- if (isMerged) {
- if (labels == null) {
- labels = currentLabels(filterApprovalsBy, cd);
- }
- short prev = labels.getOrDefault(type.get().getName(), (short) 0);
- ok &= v.getValue() >= prev;
- }
- if (ok) {
- permitted.put(r.label, v.formatValue());
- }
+ if (ok) {
+ permitted.put(labelType.getName(), v.formatValue());
}
}
}
+ clearOnlyZerosEntries(permitted);
+ return permitted.asMap();
+ }
+ private static void clearOnlyZerosEntries(SetMultimap<String, String> permitted) {
List<String> toClear = Lists.newArrayListWithCapacity(permitted.keySet().size());
for (Map.Entry<String, Collection<String>> e : permitted.asMap().entrySet()) {
if (isOnlyZero(e.getValue())) {
@@ -151,7 +142,6 @@
for (String label : toClear) {
permitted.removeAll(label);
}
- return permitted.asMap();
}
private static boolean isOnlyZero(Collection<String> values) {
diff --git a/java/com/google/gerrit/server/edit/ModificationTarget.java b/java/com/google/gerrit/server/edit/ModificationTarget.java
index 0de0149..86de812 100644
--- a/java/com/google/gerrit/server/edit/ModificationTarget.java
+++ b/java/com/google/gerrit/server/edit/ModificationTarget.java
@@ -52,21 +52,21 @@
/** A specific patchset commit is the target of the modification. */
class PatchsetCommit implements ModificationTarget {
- private final PatchSet patchset;
+ private final PatchSet patchSet;
- PatchsetCommit(PatchSet patchset) {
- this.patchset = patchset;
+ PatchsetCommit(PatchSet patchSet) {
+ this.patchSet = patchSet;
}
@Override
public void ensureTargetMayBeModifiedDespiteExistingEdit(ChangeEdit changeEdit)
throws InvalidChangeOperationException {
- if (!isBasedOn(changeEdit, patchset)) {
+ if (!isBasedOn(changeEdit, patchSet)) {
throw new InvalidChangeOperationException(
String.format(
"Only the patch set %s on which the existing change edit is based may be modified "
+ "(specified patch set: %s)",
- changeEdit.getBasePatchSet().id(), patchset.id()));
+ changeEdit.getBasePatchSet().id(), patchSet.id()));
}
}
@@ -78,7 +78,7 @@
@Override
public void ensureNewEditMayBeBasedOnTarget(Change change)
throws InvalidChangeOperationException {
- PatchSet.Id patchSetId = patchset.id();
+ PatchSet.Id patchSetId = patchSet.id();
PatchSet.Id currentPatchSetId = change.currentPatchSetId();
if (!patchSetId.equals(currentPatchSetId)) {
throw new InvalidChangeOperationException(
@@ -91,13 +91,13 @@
@Override
public RevCommit getCommit(Repository repository) throws IOException {
try (RevWalk revWalk = new RevWalk(repository)) {
- return revWalk.parseCommit(patchset.commitId());
+ return revWalk.parseCommit(patchSet.commitId());
}
}
@Override
public PatchSet getBasePatchset() {
- return patchset;
+ return patchSet;
}
}
diff --git a/java/com/google/gerrit/server/mail/send/ChangeEmail.java b/java/com/google/gerrit/server/mail/send/ChangeEmail.java
index 1a2e150..606fb28 100644
--- a/java/com/google/gerrit/server/mail/send/ChangeEmail.java
+++ b/java/com/google/gerrit/server/mail/send/ChangeEmail.java
@@ -595,6 +595,11 @@
try {
ObjectId oldId = modifiedFiles.values().iterator().next().oldCommitId();
ObjectId newId = modifiedFiles.values().iterator().next().newCommitId();
+ if (oldId.equals(ObjectId.zeroId())) {
+ // DiffOperations returns ObjectId.zeroId if newCommit is a root commit, i.e. has no
+ // parents.
+ oldId = null;
+ }
fmt.setRepository(git);
fmt.setDetectRenames(true);
fmt.format(oldId, newId);
diff --git a/java/com/google/gerrit/server/notedb/ChangeNoteJson.java b/java/com/google/gerrit/server/notedb/ChangeNoteJson.java
index 4c41a12..0d45abf 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNoteJson.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNoteJson.java
@@ -19,6 +19,9 @@
import com.google.gerrit.json.EnumTypeAdapterFactory;
import com.google.gson.Gson;
import com.google.gson.GsonBuilder;
+import com.google.gson.JsonElement;
+import com.google.gson.JsonObject;
+import com.google.gson.JsonParser;
import com.google.gson.TypeAdapter;
import com.google.gson.stream.JsonReader;
import com.google.gson.stream.JsonWriter;
@@ -26,6 +29,9 @@
import com.google.inject.TypeLiteral;
import java.io.IOException;
import java.sql.Timestamp;
+import java.util.Arrays;
+import java.util.List;
+import org.eclipse.jgit.lib.ObjectId;
@Singleton
public class ChangeNoteJson {
@@ -39,6 +45,7 @@
.registerTypeAdapter(
new TypeLiteral<ImmutableList<String>>() {}.getType(),
new ImmutableListAdapter().nullSafe())
+ .registerTypeAdapter(ObjectId.class, new ObjectIdAdapter())
.setPrettyPrinting()
.create();
}
@@ -47,6 +54,37 @@
return gson;
}
+ /** Json serializer for the {@link ObjectId} class. */
+ static class ObjectIdAdapter extends TypeAdapter<ObjectId> {
+ private static final List<String> legacyFields = Arrays.asList("w1", "w2", "w3", "w4", "w5");
+
+ @Override
+ public void write(JsonWriter out, ObjectId value) throws IOException {
+ out.value(value.name());
+ }
+
+ @Override
+ public ObjectId read(JsonReader in) throws IOException {
+ JsonElement parsed = new JsonParser().parse(in);
+ if (parsed.isJsonObject() && isJGitFormat(parsed)) {
+ // Some object IDs may have been serialized using the JGit format using the five integers
+ // w1, w2, w3, w4, w5. Detect this case so that we can deserialize properly.
+ int[] raw =
+ legacyFields.stream()
+ .mapToInt(field -> parsed.getAsJsonObject().get(field).getAsInt())
+ .toArray();
+ return ObjectId.fromRaw(raw);
+ }
+ return ObjectId.fromString(parsed.getAsString());
+ }
+
+ /** Return true if the json element contains the JGit serialized format of the Object ID. */
+ private boolean isJGitFormat(JsonElement elem) {
+ JsonObject asObj = elem.getAsJsonObject();
+ return legacyFields.stream().allMatch(field -> asObj.has(field));
+ }
+ }
+
static class ImmutableListAdapter extends TypeAdapter<ImmutableList<String>> {
@Override
diff --git a/java/com/google/gerrit/server/patch/PatchFile.java b/java/com/google/gerrit/server/patch/PatchFile.java
index 81355cc..31ca3c33 100644
--- a/java/com/google/gerrit/server/patch/PatchFile.java
+++ b/java/com/google/gerrit/server/patch/PatchFile.java
@@ -96,7 +96,13 @@
bTree = null;
} else {
if (diff.oldCommitId() != null) {
- aTree = rw.parseTree(diff.oldCommitId());
+ if (diff.oldCommitId().equals(ObjectId.zeroId())) {
+ // DiffOperations returns ObjectId.zeroId if newCommit is a root commit, i.e. has no
+ // parents.
+ aTree = null;
+ } else {
+ aTree = rw.parseTree(diff.oldCommitId());
+ }
} else {
final RevCommit p = bCommit.getParent(0);
rw.parseHeaders(p);
diff --git a/java/com/google/gerrit/server/plugins/TestServerPlugin.java b/java/com/google/gerrit/server/plugins/TestServerPlugin.java
index 3751c3f..cd5d5e3 100644
--- a/java/com/google/gerrit/server/plugins/TestServerPlugin.java
+++ b/java/com/google/gerrit/server/plugins/TestServerPlugin.java
@@ -28,7 +28,7 @@
String name,
String pluginCanonicalWebUrl,
PluginUser user,
- ClassLoader classloader,
+ ClassLoader classLoader,
String sysName,
String httpName,
String sshName,
@@ -42,10 +42,10 @@
null,
null,
dataDir,
- classloader,
+ classLoader,
null,
GerritRuntime.DAEMON);
- this.classLoader = classloader;
+ this.classLoader = classLoader;
this.sysName = sysName;
this.httpName = httpName;
this.sshName = sshName;
diff --git a/java/com/google/gerrit/server/restapi/change/DeleteVote.java b/java/com/google/gerrit/server/restapi/change/DeleteVote.java
index 7ee38d4..4387524 100644
--- a/java/com/google/gerrit/server/restapi/change/DeleteVote.java
+++ b/java/com/google/gerrit/server/restapi/change/DeleteVote.java
@@ -40,6 +40,7 @@
import com.google.gerrit.server.account.AccountState;
import com.google.gerrit.server.approval.ApprovalsUtil;
import com.google.gerrit.server.change.AddToAttentionSetOp;
+import com.google.gerrit.server.change.AttentionSetUnchangedOp;
import com.google.gerrit.server.change.NotifyResolver;
import com.google.gerrit.server.change.ReviewerResource;
import com.google.gerrit.server.change.VoteResource;
@@ -81,7 +82,7 @@
private final RemoveReviewerControl removeReviewerControl;
private final ProjectCache projectCache;
private final MessageIdGenerator messageIdGenerator;
- private final AddToAttentionSetOp.Factory attentionSetOpfactory;
+ private final AddToAttentionSetOp.Factory attentionSetOpFactory;
private final Provider<CurrentUser> currentUserProvider;
@Inject
@@ -108,7 +109,7 @@
this.removeReviewerControl = removeReviewerControl;
this.projectCache = projectCache;
this.messageIdGenerator = messageIdGenerator;
- this.attentionSetOpfactory = attentionSetOpFactory;
+ this.attentionSetOpFactory = attentionSetOpFactory;
this.currentUserProvider = currentUserProvider;
}
@@ -146,14 +147,18 @@
r.getReviewerUser().state(),
rsrc.getLabel(),
input));
- if (!r.getReviewerUser().getAccountId().equals(currentUserProvider.get().getAccountId())) {
+ if (!input.ignoreAutomaticAttentionSetRules
+ && !r.getReviewerUser().getAccountId().equals(currentUserProvider.get().getAccountId())) {
bu.addOp(
change.getId(),
- attentionSetOpfactory.create(
+ attentionSetOpFactory.create(
r.getReviewerUser().getAccountId(),
/* reason= */ "Their vote was deleted",
/* notify= */ false));
}
+ if (input.ignoreAutomaticAttentionSetRules) {
+ bu.addOp(change.getId(), new AttentionSetUnchangedOp());
+ }
bu.execute();
}
diff --git a/java/com/google/gerrit/server/util/AttentionSetUtil.java b/java/com/google/gerrit/server/util/AttentionSetUtil.java
index 9238b44..98200fd 100644
--- a/java/com/google/gerrit/server/util/AttentionSetUtil.java
+++ b/java/com/google/gerrit/server/util/AttentionSetUtil.java
@@ -43,6 +43,14 @@
.collect(ImmutableSet.toImmutableSet());
}
+ /** Returns only updates where the user was removed. */
+ public static ImmutableSet<AttentionSetUpdate> removalsOnly(
+ Collection<AttentionSetUpdate> updates) {
+ return updates.stream()
+ .filter(u -> u.operation() == Operation.REMOVE)
+ .collect(ImmutableSet.toImmutableSet());
+ }
+
/**
* Validates the input for AttentionSetInput. This must be called for all inputs that relate to
* adding or removing attention set entries, except for {@link
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
index 58c222a..f60f1f1 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -3575,8 +3575,8 @@
assertPermitted(change, LabelId.CODE_REVIEW, 2);
assertPermitted(change, LabelId.VERIFIED, 0, 1);
- // ignore the new label by Prolog submit rule and assert that the label is
- // no longer returned
+ // Ignore the new label by Prolog submit rule. Permitted ranges are still going to be
+ // returned for the label.
GitUtil.fetch(testRepo, RefNames.REFS_CONFIG + ":config");
testRepo.reset("config");
PushOneCommit push2 =
@@ -3590,11 +3590,10 @@
change = gApi.changes().id(r.getChangeId()).get();
assertPermitted(change, LabelId.CODE_REVIEW, 2);
- assertPermitted(change, LabelId.VERIFIED);
+ assertPermitted(change, LabelId.VERIFIED, 0, 1);
- // add an approval on the new label and assert that the label is now
- // returned although it is ignored by the Prolog submit rule and hence not
- // included in the submit records
+ // add an approval on the new label. The label can still be voted +1 although it is ignored
+ // in Prolog. 0 is not permitted because votes cannot be decreased.
gApi.changes()
.id(r.getChangeId())
.revision(r.getCommit().name())
@@ -3603,7 +3602,7 @@
change = gApi.changes().id(r.getChangeId()).get();
assertThat(change.labels.keySet()).containsExactly(LabelId.CODE_REVIEW, LabelId.VERIFIED);
assertPermitted(change, LabelId.CODE_REVIEW, 2);
- assertPermitted(change, LabelId.VERIFIED);
+ assertPermitted(change, LabelId.VERIFIED, 1);
// remove label and assert that it's no longer returned for existing
// changes, even if there is an approval for it
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/AttentionSetIT.java b/javatests/com/google/gerrit/acceptance/rest/change/AttentionSetIT.java
index 0a11b15..9246442 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/AttentionSetIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/AttentionSetIT.java
@@ -46,6 +46,7 @@
import com.google.gerrit.entities.Permission;
import com.google.gerrit.extensions.api.changes.AttentionSetInput;
import com.google.gerrit.extensions.api.changes.DeleteReviewerInput;
+import com.google.gerrit.extensions.api.changes.DeleteVoteInput;
import com.google.gerrit.extensions.api.changes.HashtagsInput;
import com.google.gerrit.extensions.api.changes.ReviewInput;
import com.google.gerrit.extensions.api.changes.ReviewerInput;
@@ -261,6 +262,12 @@
fakeClock.now(), user.id(), AttentionSetUpdate.Operation.REMOVE, "removed");
assertThat(r.getChange().attentionSet()).containsExactly(expectedAttentionSetUpdate);
+ // The removal also shows up in AttentionSetInfo.
+ AttentionSetInfo attentionSetInfo =
+ Iterables.getOnlyElement(change(r).get().removedFromAttentionSet.values());
+ assertThat(attentionSetInfo.reason).isEqualTo("removed");
+ assertThat(attentionSetInfo.account).isEqualTo(getAccountInfo(user.id()));
+
// Second removal is ignored.
fakeClock.advance(Duration.ofSeconds(42));
change(r).attention(user.id().toString()).remove(new AttentionSetInput("removed again"));
@@ -1935,6 +1942,30 @@
}
@Test
+ public void deleteVotesDoesNotAffectAttentionSetWhenIgnoreAutomaticRulesIsSet() throws Exception {
+ PushOneCommit.Result r = createChange();
+
+ requestScopeOperations.setApiUser(user.id());
+ recommend(r.getChangeId());
+
+ requestScopeOperations.setApiUser(admin.id());
+
+ DeleteVoteInput deleteVoteInput = new DeleteVoteInput();
+ deleteVoteInput.label = LabelId.CODE_REVIEW;
+
+ // set this to true to not change the attention set.
+ deleteVoteInput.ignoreAutomaticAttentionSetRules = true;
+
+ gApi.changes()
+ .id(r.getChangeId())
+ .current()
+ .reviewer(user.id().toString())
+ .deleteVote(deleteVoteInput);
+
+ assertThat(getAttentionSetUpdatesForUser(r, user)).isEmpty();
+ }
+
+ @Test
public void deleteVotesOfOthersAddThemToAttentionSet() throws Exception {
PushOneCommit.Result r = createChange();
diff --git a/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java b/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java
index 80cdad8..517be98 100644
--- a/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java
@@ -36,6 +36,7 @@
import com.google.gerrit.acceptance.testsuite.change.TestHumanComment;
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.Account;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.HumanComment;
@@ -47,9 +48,11 @@
import com.google.gerrit.extensions.api.changes.ReviewInput;
import com.google.gerrit.extensions.api.changes.ReviewInput.CommentInput;
import com.google.gerrit.extensions.api.changes.ReviewInput.DraftHandling;
+import com.google.gerrit.extensions.api.changes.ReviewerInput;
import com.google.gerrit.extensions.client.Comment;
import com.google.gerrit.extensions.client.Side;
import com.google.gerrit.extensions.common.ChangeInfo;
+import com.google.gerrit.extensions.common.ChangeInput;
import com.google.gerrit.extensions.common.CommentInfo;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
@@ -1853,6 +1856,42 @@
assertThat(exception.getMessage()).contains(String.format("%s not found", comment.inReplyTo));
}
+ @Test
+ public void commentsOnRootCommitsAreIncludedInEmails() throws Exception {
+ // Create a change in a new branch, making the patch-set commit a root commit.
+ ChangeInfo changeInfo = createChangeInNewBranch("newBranch");
+ Change.Id changeId = Change.Id.tryParse(Integer.toString(changeInfo._number)).get();
+
+ // Add a file.
+ gApi.changes().id(changeId.get()).edit().modifyFile("f1.txt", RawInputUtil.create("content"));
+ gApi.changes().id(changeId.get()).edit().publish();
+ email.clear();
+
+ ReviewerInput reviewerInput = new ReviewerInput();
+ reviewerInput.reviewer = admin.email();
+ gApi.changes().id(changeId.get()).addReviewer(reviewerInput);
+ changeInfo = gApi.changes().id(changeId.get()).get();
+ assertThat(email.getMessages()).hasSize(1);
+ Message message = email.getMessages().get(0);
+ assertThat(message.body()).contains("f1.txt");
+ email.clear();
+
+ // Send a comment. Make sure the email that is sent includes the comment text.
+ CommentInput c1 =
+ CommentsUtil.newComment(
+ "f1.txt",
+ Side.REVISION,
+ /* line= */ 1,
+ /* message= */ "Comment text",
+ /* unresolved= */ false);
+ CommentsUtil.addComments(gApi, changeId.toString(), changeInfo.currentRevision, c1);
+ assertThat(email.getMessages()).hasSize(1);
+ Message commentMessage = email.getMessages().get(0);
+ assertThat(commentMessage.body())
+ .contains("Patch Set 2:\n" + "\n" + "(1 comment)\n" + "\n" + "File f1.txt:");
+ assertThat(commentMessage.body()).contains("PS2, Line 1: content\n" + "Comment text");
+ }
+
private List<CommentInfo> getRevisionComments(String changeId, String revId) throws Exception {
return getPublishedComments(changeId, revId).values().stream()
.flatMap(List::stream)
@@ -2017,4 +2056,13 @@
reviewInput.draftIdsToPublish = draftIdsToPublish;
return reviewInput;
}
+
+ private ChangeInfo createChangeInNewBranch(String branchName) throws Exception {
+ ChangeInput in = new ChangeInput();
+ in.project = project.get();
+ in.branch = branchName;
+ in.newBranch = true;
+ in.subject = "New changes";
+ return gApi.changes().create(in).get();
+ }
}
diff --git a/javatests/com/google/gerrit/server/cache/serialize/entities/BUILD b/javatests/com/google/gerrit/server/cache/serialize/entities/BUILD
index 8df9292..d68a5c1 100644
--- a/javatests/com/google/gerrit/server/cache/serialize/entities/BUILD
+++ b/javatests/com/google/gerrit/server/cache/serialize/entities/BUILD
@@ -16,5 +16,6 @@
"//lib/truth:truth-proto-extension",
"//proto:cache_java_proto",
"//proto/testing:test_java_proto",
+ "@gson//jar",
],
)
diff --git a/javatests/com/google/gerrit/server/cache/serialize/entities/SubmitRequirementJsonSerializerTest.java b/javatests/com/google/gerrit/server/cache/serialize/entities/SubmitRequirementJsonSerializerTest.java
new file mode 100644
index 0000000..1df409a
--- /dev/null
+++ b/javatests/com/google/gerrit/server/cache/serialize/entities/SubmitRequirementJsonSerializerTest.java
@@ -0,0 +1,219 @@
+// 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.cache.serialize.entities;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.common.collect.ImmutableList;
+import com.google.gerrit.entities.SubmitRequirement;
+import com.google.gerrit.entities.SubmitRequirementExpression;
+import com.google.gerrit.entities.SubmitRequirementExpressionResult;
+import com.google.gerrit.entities.SubmitRequirementExpressionResult.Status;
+import com.google.gerrit.entities.SubmitRequirementResult;
+import com.google.gerrit.server.notedb.ChangeNoteJson;
+import com.google.gson.Gson;
+import com.google.gson.TypeAdapter;
+import java.util.Optional;
+import org.eclipse.jgit.lib.ObjectId;
+import org.junit.Test;
+
+public class SubmitRequirementJsonSerializerTest {
+ private static final SubmitRequirementExpression srReqExp =
+ SubmitRequirementExpression.create("label:Code-Review=+2");
+
+ private static final String srReqExpSerial = "{\"expressionString\":\"label:Code-Review=+2\"}";
+
+ private static final SubmitRequirement sr =
+ SubmitRequirement.builder()
+ .setName("CR")
+ .setDescription(Optional.of("CR description"))
+ .setApplicabilityExpression(SubmitRequirementExpression.of("branch:refs/heads/master"))
+ .setSubmittabilityExpression(SubmitRequirementExpression.create("label:Code-Review=+2"))
+ .setAllowOverrideInChildProjects(true)
+ .build();
+
+ private static final String srSerial =
+ "{\"name\":\"CR\","
+ + "\"description\":{\"value\":\"CR description\"},"
+ + "\"applicabilityExpression\":{\"value\":"
+ + "{\"expressionString\":\"branch:refs/heads/master\"}},"
+ + "\"submittabilityExpression\":{"
+ + "\"expressionString\":\"label:Code-Review=+2\"},"
+ + "\"overrideExpression\":{\"value\":null},"
+ + "\"allowOverrideInChildProjects\":true}";
+
+ private static final SubmitRequirementExpressionResult srExpResult =
+ SubmitRequirementExpressionResult.create(
+ SubmitRequirementExpression.create("label:Code-Review=MAX AND -label:Code-Review=MIN"),
+ Status.FAIL,
+ /* passingAtoms= */ ImmutableList.of("label:Code-Review=MAX"),
+ /* failingAtoms= */ ImmutableList.of("label:Code-Review=MIN"));
+
+ private static final String srExpResultSerial =
+ "{\"expression\":{\"expressionString\":"
+ + "\"label:Code-Review=MAX AND -label:Code-Review=MIN\"},"
+ + "\"status\":\"FAIL\","
+ + "\"errorMessage\":{\"value\":null},"
+ + "\"passingAtoms\":[\"label:Code-Review=MAX\"],"
+ + "\"failingAtoms\":[\"label:Code-Review=MIN\"]}";
+
+ private static final SubmitRequirementResult srReqResult =
+ SubmitRequirementResult.builder()
+ .submitRequirement(
+ SubmitRequirement.builder()
+ .setName("CR")
+ .setDescription(Optional.of("CR Description"))
+ .setApplicabilityExpression(
+ SubmitRequirementExpression.of("branch:refs/heads/master"))
+ .setSubmittabilityExpression(
+ SubmitRequirementExpression.create("label:\"Code-Review=+2\""))
+ .setOverrideExpression(SubmitRequirementExpression.of("label:Override=+1"))
+ .setAllowOverrideInChildProjects(false)
+ .build())
+ .patchSetCommitId(ObjectId.fromString("4663ab9e9eb49a214e68e60f0fe5d0b6f44f763e"))
+ .applicabilityExpressionResult(
+ Optional.of(
+ SubmitRequirementExpressionResult.create(
+ SubmitRequirementExpression.create("branch:refs/heads/master"),
+ Status.PASS,
+ ImmutableList.of("refs/heads/master"),
+ ImmutableList.of())))
+ .submittabilityExpressionResult(
+ SubmitRequirementExpressionResult.create(
+ SubmitRequirementExpression.create("label:\"Code-Review=+2\""),
+ Status.PASS,
+ /* passingAtoms= */ ImmutableList.of("label:\"Code-Review=+2\""),
+ /* failingAtoms= */ ImmutableList.of()))
+ .overrideExpressionResult(
+ Optional.of(
+ SubmitRequirementExpressionResult.create(
+ SubmitRequirementExpression.create("label:Override=+1"),
+ Status.PASS,
+ /* passingAtoms= */ ImmutableList.of(),
+ /* failingAtoms= */ ImmutableList.of("label:Override=+1"))))
+ .legacy(Optional.of(true))
+ .build();
+
+ private static final String srReqResultSerial =
+ "{\"submitRequirement\":{\"name\":\"CR\",\"description\":{\"value\":\"CR Description\"},"
+ + "\"applicabilityExpression\":{\"value\":{"
+ + "\"expressionString\":\"branch:refs/heads/master\"}},"
+ + "\"submittabilityExpression\":{\"expressionString\":\"label:\\\"Code-Review=+2\\\"\"},"
+ + "\"overrideExpression\":{\"value\":{\"expressionString\":\"label:Override=+1\"}},"
+ + "\"allowOverrideInChildProjects\":false},"
+ + "\"applicabilityExpressionResult\":{\"value\":{"
+ + "\"expression\":{\"expressionString\":\"branch:refs/heads/master\"},"
+ + "\"status\":\"PASS\",\"errorMessage\":{\"value\":null},"
+ + "\"passingAtoms\":[\"refs/heads/master\"],"
+ + "\"failingAtoms\":[]}},"
+ + "\"submittabilityExpressionResult\":{"
+ + "\"expression\":{\"expressionString\":\"label:\\\"Code-Review=+2\\\"\"},"
+ + "\"status\":\"PASS\",\"errorMessage\":{\"value\":null},"
+ + "\"passingAtoms\":[\"label:\\\"Code-Review=+2\\\"\"],"
+ + "\"failingAtoms\":[]},"
+ + "\"overrideExpressionResult\":{\"value\":{"
+ + "\"expression\":{\"expressionString\":\"label:Override=+1\"},"
+ + "\"status\":\"PASS\",\"errorMessage\":{\"value\":null},"
+ + "\"passingAtoms\":[],"
+ + "\"failingAtoms\":[\"label:Override=+1\"]}},"
+ + "\"patchSetCommitId\":\"4663ab9e9eb49a214e68e60f0fe5d0b6f44f763e\","
+ + "\"legacy\":{\"value\":true}}";
+
+ private static final Gson gson = new ChangeNoteJson().getGson();
+
+ @Test
+ public void submitRequirementExpression_serialize() {
+ assertThat(SubmitRequirementExpression.typeAdapter(gson).toJson(srReqExp))
+ .isEqualTo(srReqExpSerial);
+ }
+
+ @Test
+ public void submitRequirementExpression_deserialize() throws Exception {
+ assertThat(SubmitRequirementExpression.typeAdapter(gson).fromJson(srReqExpSerial))
+ .isEqualTo(srReqExp);
+ }
+
+ @Test
+ public void submitRequirementExpression_roundTrip() throws Exception {
+ SubmitRequirementExpression exp = SubmitRequirementExpression.create("label:Code-Review=+2");
+ TypeAdapter<SubmitRequirementExpression> adapter =
+ SubmitRequirementExpression.typeAdapter(gson);
+ assertThat(adapter.fromJson(adapter.toJson(exp))).isEqualTo(exp);
+ }
+
+ @Test
+ public void submitRequirement_serialize() throws Exception {
+ assertThat(SubmitRequirement.typeAdapter(gson).toJson(sr)).isEqualTo(srSerial);
+ }
+
+ @Test
+ public void submitRequirement_deserialize() throws Exception {
+ assertThat(SubmitRequirement.typeAdapter(gson).fromJson(srSerial)).isEqualTo(sr);
+ }
+
+ @Test
+ public void submitRequirement_roundTrip() throws Exception {
+ TypeAdapter<SubmitRequirement> adapter = SubmitRequirement.typeAdapter(gson);
+ assertThat(adapter.fromJson(adapter.toJson(sr))).isEqualTo(sr);
+ }
+
+ @Test
+ public void submitRequirementExpressionResult_serialize() throws Exception {
+ assertThat(SubmitRequirementExpressionResult.typeAdapter(gson).toJson(srExpResult))
+ .isEqualTo(srExpResultSerial);
+ }
+
+ @Test
+ public void submitRequirementExpressionResult_deserialize() throws Exception {
+ assertThat(SubmitRequirementExpressionResult.typeAdapter(gson).fromJson(srExpResultSerial))
+ .isEqualTo(srExpResult);
+ }
+
+ @Test
+ public void submitRequirementExpressionResult_roundtrip() throws Exception {
+ TypeAdapter<SubmitRequirementExpressionResult> adapter =
+ SubmitRequirementExpressionResult.typeAdapter(gson);
+ assertThat(adapter.fromJson(adapter.toJson(srExpResult))).isEqualTo(srExpResult);
+ }
+
+ @Test
+ public void submitRequirementResult_serialize() throws Exception {
+ assertThat(SubmitRequirementResult.typeAdapter(gson).toJson(srReqResult))
+ .isEqualTo(srReqResultSerial);
+ }
+
+ @Test
+ public void submitRequirementResult_deserialize() throws Exception {
+ assertThat(SubmitRequirementResult.typeAdapter(gson).fromJson(srReqResultSerial))
+ .isEqualTo(srReqResult);
+ }
+
+ @Test
+ public void submitRequirementResult_roundTrip() throws Exception {
+ TypeAdapter<SubmitRequirementResult> adapter = SubmitRequirementResult.typeAdapter(gson);
+ assertThat(adapter.fromJson(adapter.toJson(srReqResult))).isEqualTo(srReqResult);
+ }
+
+ @Test
+ public void deserializeSubmitRequirementResult_withJGitPatchsetIdFormat() throws Exception {
+ String srResultSerialJgitFormat =
+ srReqResultSerial.replace(
+ "\"4663ab9e9eb49a214e68e60f0fe5d0b6f44f763e\"",
+ "{\"w1\":1180937118,\"w2\":-1632331231,\"w3\":1315497487,"
+ + "\"w4\":266719414,\"w5\":-196118978}");
+ assertThat(SubmitRequirementResult.typeAdapter(gson).fromJson(srResultSerialJgitFormat))
+ .isEqualTo(srReqResult);
+ }
+}
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.ts b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.ts
index 43f6730..8ffc0aa 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.ts
@@ -47,9 +47,12 @@
QuickLabelInfo,
Timestamp,
} from '../../../types/common';
-import {hasOwnProperty} from '../../../utils/common-util';
+import {assertNever, hasOwnProperty} from '../../../utils/common-util';
import {pluralize} from '../../../utils/string-util';
import {ChangeStates} from '../../shared/gr-change-status/gr-change-status';
+import {KnownExperimentId} from '../../../services/flags/flags';
+import {getRequirements, iconForStatus} from '../../../utils/label-util';
+import {SubmitRequirementStatus} from '../../../api/rest-api';
enum ChangeSize {
XS = 10,
@@ -121,8 +124,20 @@
@property({type: Array})
_dynamicCellEndpoints?: string[];
+ @property({type: Boolean})
+ _isSubmitRequirementsUiEnabled = false;
+
reporting: ReportingService = appContext.reportingService;
+ private readonly flagsService = appContext.flagsService;
+
+ override ready() {
+ super.ready();
+ this._isSubmitRequirementsUiEnabled = this.flagsService.isEnabled(
+ KnownExperimentId.SUBMIT_REQUIREMENTS_UI
+ );
+ }
+
override connectedCallback() {
super.connectedCallback();
getPluginLoader()
@@ -167,8 +182,33 @@
}
_computeLabelClass(change: ChangeInfo | undefined, labelName: string) {
- const category = this._computeLabelCategory(change, labelName);
const classes = ['cell', 'label'];
+ if (this._isSubmitRequirementsUiEnabled) {
+ const requirements = getRequirements(change).filter(
+ sr => sr.name === labelName
+ );
+ if (requirements.length === 1) {
+ const status = requirements[0].status;
+ switch (status) {
+ case SubmitRequirementStatus.SATISFIED:
+ classes.push('u-green');
+ break;
+ case SubmitRequirementStatus.UNSATISFIED:
+ classes.push('u-red');
+ break;
+ case SubmitRequirementStatus.OVERRIDDEN:
+ classes.push('u-green');
+ break;
+ case SubmitRequirementStatus.NOT_APPLICABLE:
+ classes.push('u-gray-background');
+ break;
+ default:
+ assertNever(status, `Unsupported status: ${status}`);
+ }
+ return classes.sort().join(' ');
+ }
+ }
+ const category = this._computeLabelCategory(change, labelName);
switch (category) {
case LabelCategory.NOT_APPLICABLE:
classes.push('u-gray-background');
@@ -196,6 +236,14 @@
}
_computeLabelIcon(change: ChangeInfo | undefined, labelName: string): string {
+ if (this._isSubmitRequirementsUiEnabled) {
+ const requirements = getRequirements(change).filter(
+ sr => sr.name === labelName
+ );
+ if (requirements.length === 1) {
+ return `gr-icons:${iconForStatus(requirements[0].status)}`;
+ }
+ }
const category = this._computeLabelCategory(change, labelName);
switch (category) {
case LabelCategory.APPROVED:
diff --git a/polygerrit-ui/app/elements/change/gr-submit-requirement-dashboard-hovercard/gr-submit-requirement-dashboard-hovercard.ts b/polygerrit-ui/app/elements/change/gr-submit-requirement-dashboard-hovercard/gr-submit-requirement-dashboard-hovercard.ts
index ebe3ce3..c31be2e 100644
--- a/polygerrit-ui/app/elements/change/gr-submit-requirement-dashboard-hovercard/gr-submit-requirement-dashboard-hovercard.ts
+++ b/polygerrit-ui/app/elements/change/gr-submit-requirement-dashboard-hovercard/gr-submit-requirement-dashboard-hovercard.ts
@@ -44,6 +44,7 @@
return html`<div id="container" role="tooltip" tabindex="-1">
<gr-submit-requirements
.change=${this.change}
+ disable-hovercards
suppress-title
></gr-submit-requirements>
</div>`;
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 002ac56..74f430c 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
@@ -236,7 +236,7 @@
name: string,
expression?: SubmitRequirementExpressionInfo
) {
- if (!expression) return '';
+ if (!expression?.expression) return '';
return html`
<div class="section">
<div class="sectionIcon"></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 6406e3c..2e1f5fd 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
@@ -64,6 +64,9 @@
@property({type: Boolean})
mutable?: boolean;
+ @property({type: Boolean, attribute: 'disable-hovercards'})
+ disableHovercards = false;
+
@state()
runs: CheckRun[] = [];
@@ -164,17 +167,19 @@
)}
</tbody>
</table>
- ${submit_requirements.map(
- requirement => html`
- <gr-submit-requirement-hovercard
- for="requirement-${charsOnly(requirement.name)}"
- .requirement="${requirement}"
- .change="${this.change}"
- .account="${this.account}"
- .mutable="${this.mutable ?? false}"
- ></gr-submit-requirement-hovercard>
- `
- )}
+ ${this.disableHovercards
+ ? ''
+ : submit_requirements.map(
+ requirement => html`
+ <gr-submit-requirement-hovercard
+ for="requirement-${charsOnly(requirement.name)}"
+ .requirement="${requirement}"
+ .change="${this.change}"
+ .account="${this.account}"
+ .mutable="${this.mutable ?? false}"
+ ></gr-submit-requirement-hovercard>
+ `
+ )}
${this.renderTriggerVotes()}`;
}
@@ -192,7 +197,9 @@
<td>
<gr-endpoint-decorator
class="votes-cell"
- name="${`submit-requirement-${charsOnly(requirement.name)}`}"
+ name="${`submit-requirement-${charsOnly(
+ requirement.name
+ ).toLowerCase()}`}"
>
<gr-endpoint-param
name="change"
diff --git a/polygerrit-ui/app/elements/core/gr-account-dropdown/gr-account-dropdown.ts b/polygerrit-ui/app/elements/core/gr-account-dropdown/gr-account-dropdown.ts
index ac3d5f4..03fb4d5 100644
--- a/polygerrit-ui/app/elements/core/gr-account-dropdown/gr-account-dropdown.ts
+++ b/polygerrit-ui/app/elements/core/gr-account-dropdown/gr-account-dropdown.ts
@@ -83,6 +83,7 @@
gr-dropdown {
padding: 0 var(--spacing-m);
--gr-button-text-color: var(--header-text-color);
+ --gr-dropdown-item-color: var(--primary-text-color);
}
gr-avatar {
height: 2em;
@@ -94,36 +95,23 @@
}
override render() {
- // To pass CSS mixins for @apply to Polymer components, they need to appear
- // in <style> inside the template.
- /* eslint-disable lit/prefer-static-styles */
- const customStyle = html`
- <style>
- gr-dropdown {
- --gr-dropdown-item: {
- color: var(--primary-text-color);
- }
- }
- </style>
- `;
- return html`${customStyle}
- <gr-dropdown
- link=""
- .items="${this.links}"
- .topContent="${this.topContent}"
- @tap-item-shortcuts=${this._handleShortcutsTap}
- .horizontalAlign=${'right'}
+ return html`<gr-dropdown
+ link=""
+ .items="${this.links}"
+ .topContent="${this.topContent}"
+ @tap-item-shortcuts=${this._handleShortcutsTap}
+ .horizontalAlign=${'right'}
+ >
+ <span ?hidden="${this._hasAvatars}"
+ >${this._accountName(this.account)}</span
>
- <span ?hidden="${this._hasAvatars}"
- >${this._accountName(this.account)}</span
- >
- <gr-avatar
- .account="${this.account}"
- ?hidden=${!this._hasAvatars}
- .imageSize=${56}
- aria-label="Account avatar"
- ></gr-avatar>
- </gr-dropdown>`;
+ <gr-avatar
+ .account="${this.account}"
+ ?hidden=${!this._hasAvatars}
+ .imageSize=${56}
+ aria-label="Account avatar"
+ ></gr-avatar>
+ </gr-dropdown>`;
}
get links(): DropdownLink[] | undefined {
diff --git a/polygerrit-ui/app/elements/core/gr-key-binding-display/gr-key-binding-display.ts b/polygerrit-ui/app/elements/core/gr-key-binding-display/gr-key-binding-display.ts
index 1ae0992..2a3936f 100644
--- a/polygerrit-ui/app/elements/core/gr-key-binding-display/gr-key-binding-display.ts
+++ b/polygerrit-ui/app/elements/core/gr-key-binding-display/gr-key-binding-display.ts
@@ -25,6 +25,9 @@
@customElement('gr-key-binding-display')
export class GrKeyBindingDisplay extends LitElement {
+ @property({type: Array})
+ binding: string[][] = [];
+
static override get styles() {
return [
css`
@@ -53,9 +56,6 @@
return html`${items}`;
}
- @property({type: Array})
- binding: string[][] = [];
-
_computeModifiers(binding: string[]) {
return binding.slice(0, binding.length - 1);
}
diff --git a/polygerrit-ui/app/elements/core/gr-keyboard-shortcuts-dialog/gr-keyboard-shortcuts-dialog.ts b/polygerrit-ui/app/elements/core/gr-keyboard-shortcuts-dialog/gr-keyboard-shortcuts-dialog.ts
index 8610999..2e51e64 100644
--- a/polygerrit-ui/app/elements/core/gr-keyboard-shortcuts-dialog/gr-keyboard-shortcuts-dialog.ts
+++ b/polygerrit-ui/app/elements/core/gr-keyboard-shortcuts-dialog/gr-keyboard-shortcuts-dialog.ts
@@ -16,15 +16,14 @@
*/
import '../../shared/gr-button/gr-button';
import '../gr-key-binding-display/gr-key-binding-display';
-import '../../../styles/shared-styles';
-import '../../../styles/gr-font-styles';
-import {PolymerElement} from '@polymer/polymer/polymer-element';
-import {htmlTemplate} from './gr-keyboard-shortcuts-dialog_html';
+import {sharedStyles} from '../../../styles/shared-styles';
+import {fontStyles} from '../../../styles/gr-font-styles';
+import {LitElement, css, html} from 'lit';
+import {customElement, property} from 'lit/decorators';
import {
ShortcutSection,
SectionView,
} from '../../../mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin';
-import {property, customElement} from '@polymer/decorators';
import {appContext} from '../../../services/app-context';
import {ShortcutViewListener} from '../../../services/shortcuts/shortcuts-service';
@@ -40,11 +39,7 @@
}
@customElement('gr-keyboard-shortcuts-dialog')
-export class GrKeyboardShortcutsDialog extends PolymerElement {
- static get template() {
- return htmlTemplate;
- }
-
+export class GrKeyboardShortcutsDialog extends LitElement {
/**
* Fired when the user presses the close button.
*
@@ -67,9 +62,107 @@
this._onDirectoryUpdated(d);
}
- override ready() {
- super.ready();
- this._ensureAttribute('role', 'dialog');
+ static override get styles() {
+ return [
+ sharedStyles,
+ fontStyles,
+ css`
+ :host {
+ display: block;
+ max-height: 100vh;
+ overflow-y: auto;
+ }
+ header {
+ padding: var(--spacing-l);
+ }
+ main {
+ display: flex;
+ padding: 0 var(--spacing-xxl) var(--spacing-xxl);
+ }
+ .column {
+ flex: 50%;
+ }
+ header {
+ align-items: center;
+ border-bottom: 1px solid var(--border-color);
+ display: flex;
+ justify-content: space-between;
+ }
+ table caption {
+ font-weight: var(--font-weight-bold);
+ padding-top: var(--spacing-l);
+ text-align: left;
+ }
+ tr {
+ height: 32px;
+ }
+ td {
+ padding: var(--spacing-xs) 0;
+ }
+ td:first-child,
+ th:first-child {
+ padding-right: var(--spacing-m);
+ text-align: right;
+ width: 160px;
+ color: var(--deemphasized-text-color);
+ }
+ td:second-child {
+ min-width: 200px;
+ }
+ th {
+ color: var(--deemphasized-text-color);
+ text-align: left;
+ }
+ .header {
+ font-weight: var(--font-weight-bold);
+ padding-top: var(--spacing-l);
+ }
+ .modifier {
+ font-weight: var(--font-weight-normal);
+ }
+ `,
+ ];
+ }
+
+ override render() {
+ return html`<header>
+ <h3 class="heading-3">Keyboard shortcuts</h3>
+ <gr-button link="" @click=${this.handleCloseTap}>Close</gr-button>
+ </header>
+ <main>
+ <div class="column">
+ ${this._left?.map(section => this.renderSection(section))}
+ </div>
+ <div class="column">
+ ${this._right?.map(section => this.renderSection(section))}
+ </div>
+ </main>
+ <footer></footer>`;
+ }
+
+ private renderSection(section: SectionShortcut) {
+ return html`<table>
+ <caption>
+ ${section.section}
+ </caption>
+ <thead>
+ <tr>
+ <th>Key</th>
+ <th>Action</th>
+ </tr>
+ </thead>
+ <tbody>
+ ${section.shortcuts?.map(
+ shortcut => html`<tr>
+ <td>
+ <gr-key-binding-display .binding=${shortcut.binding}>
+ </gr-key-binding-display>
+ </td>
+ <td>${shortcut.text}</td>
+ </tr>`
+ )}
+ </tbody>
+ </table>`;
}
override connectedCallback() {
@@ -82,7 +175,7 @@
super.disconnectedCallback();
}
- _handleCloseTap(e: MouseEvent) {
+ private handleCloseTap(e: MouseEvent) {
e.preventDefault();
e.stopPropagation();
this.dispatchEvent(
@@ -142,7 +235,7 @@
});
}
- this.set('_left', left);
- this.set('_right', right);
+ this._right = right;
+ this._left = left;
}
}
diff --git a/polygerrit-ui/app/elements/core/gr-keyboard-shortcuts-dialog/gr-keyboard-shortcuts-dialog_html.ts b/polygerrit-ui/app/elements/core/gr-keyboard-shortcuts-dialog/gr-keyboard-shortcuts-dialog_html.ts
deleted file mode 100644
index 4992daa..0000000
--- a/polygerrit-ui/app/elements/core/gr-keyboard-shortcuts-dialog/gr-keyboard-shortcuts-dialog_html.ts
+++ /dev/null
@@ -1,137 +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-font-styles">
- /* Workaround for empty style block - see https://github.com/Polymer/tools/issues/408 */
- </style>
- <style include="shared-styles">
- :host {
- display: block;
- max-height: 100vh;
- overflow-y: auto;
- }
- header {
- padding: var(--spacing-l);
- }
- main {
- display: flex;
- padding: 0 var(--spacing-xxl) var(--spacing-xxl);
- }
- .column {
- flex: 50%;
- }
- header {
- align-items: center;
- border-bottom: 1px solid var(--border-color);
- display: flex;
- justify-content: space-between;
- }
- table caption {
- font-weight: var(--font-weight-bold);
- padding-top: var(--spacing-l);
- text-align: left;
- }
- tr {
- height: 32px;
- }
- td {
- padding: var(--spacing-xs) 0;
- }
- td:first-child,
- th:first-child {
- padding-right: var(--spacing-m);
- text-align: right;
- width: 160px;
- color: var(--deemphasized-text-color);
- }
- td:second-child {
- min-width: 200px;
- }
- th {
- color: var(--deemphasized-text-color);
- text-align: left;
- }
- .header {
- font-weight: var(--font-weight-bold);
- padding-top: var(--spacing-l);
- }
- .modifier {
- font-weight: var(--font-weight-normal);
- }
- </style>
- <header>
- <h3 class="heading-3">Keyboard shortcuts</h3>
- <gr-button link="" on-click="_handleCloseTap">Close</gr-button>
- </header>
- <main>
- <div class="column">
- <template is="dom-repeat" items="[[_left]]">
- <table>
- <caption>
- [[item.section]]
- </caption>
- <thead>
- <tr>
- <th>Key</th>
- <th>Action</th>
- </tr>
- </thead>
- <tbody>
- <template is="dom-repeat" items="[[item.shortcuts]]" as="shortcut">
- <tr>
- <td>
- <gr-key-binding-display binding="[[shortcut.binding]]">
- </gr-key-binding-display>
- </td>
- <td>[[shortcut.text]]</td>
- </tr>
- </template>
- </tbody>
- </table>
- </template>
- </div>
- <div class="column">
- <template is="dom-repeat" items="[[_right]]">
- <table>
- <caption>
- [[item.section]]
- </caption>
- <thead>
- <tr>
- <th>Key</th>
- <th>Action</th>
- </tr>
- </thead>
- <tbody>
- <template is="dom-repeat" items="[[item.shortcuts]]" as="shortcut">
- <tr>
- <td>
- <gr-key-binding-display binding="[[shortcut.binding]]">
- </gr-key-binding-display>
- </td>
- <td>[[shortcut.text]]</td>
- </tr>
- </template>
- </tbody>
- </table>
- </template>
- </div>
- </main>
- <footer></footer>
-`;
diff --git a/polygerrit-ui/app/elements/core/gr-keyboard-shortcuts-dialog/gr-keyboard-shortcuts-dialog_test.ts b/polygerrit-ui/app/elements/core/gr-keyboard-shortcuts-dialog/gr-keyboard-shortcuts-dialog_test.ts
index 2c76704..7fc52f5 100644
--- a/polygerrit-ui/app/elements/core/gr-keyboard-shortcuts-dialog/gr-keyboard-shortcuts-dialog_test.ts
+++ b/polygerrit-ui/app/elements/core/gr-keyboard-shortcuts-dialog/gr-keyboard-shortcuts-dialog_test.ts
@@ -16,6 +16,7 @@
*/
import '../../../test/common-test-setup-karma';
+import './gr-keyboard-shortcuts-dialog';
import {GrKeyboardShortcutsDialog} from './gr-keyboard-shortcuts-dialog';
import {
SectionView,
@@ -27,8 +28,9 @@
suite('gr-keyboard-shortcuts-dialog tests', () => {
let element: GrKeyboardShortcutsDialog;
- setup(() => {
+ setup(async () => {
element = basicFixture.instantiate();
+ await flush();
});
function update(directory: Map<ShortcutSection, SectionView>) {
diff --git a/polygerrit-ui/app/elements/core/gr-main-header/gr-main-header_html.ts b/polygerrit-ui/app/elements/core/gr-main-header/gr-main-header_html.ts
index b745c3d..b623e8e 100644
--- a/polygerrit-ui/app/elements/core/gr-main-header/gr-main-header_html.ts
+++ b/polygerrit-ui/app/elements/core/gr-main-header/gr-main-header_html.ts
@@ -86,9 +86,7 @@
padding: var(--spacing-m);
}
gr-dropdown {
- --gr-dropdown-item: {
- color: var(--primary-text-color);
- }
+ --gr-dropdown-item-color: var(--primary-text-color);
}
.settingsButton {
margin-left: var(--spacing-m);
diff --git a/polygerrit-ui/app/elements/core/gr-navigation/gr-navigation.ts b/polygerrit-ui/app/elements/core/gr-navigation/gr-navigation.ts
index 6b4006c..e5340fa 100644
--- a/polygerrit-ui/app/elements/core/gr-navigation/gr-navigation.ts
+++ b/polygerrit-ui/app/elements/core/gr-navigation/gr-navigation.ts
@@ -40,65 +40,7 @@
//
// Each object has a `view` property with a value from GerritNav.View. The
// remaining properties depend on the value used for view.
-//
-// - GerritNav.View.CHANGE:
-// - `changeNum`, required, String: the numeric ID of the change.
-// - `project`, optional, String: the project name.
-// - `patchNum`, optional, Number: the patch for the right-hand-side of
-// the diff.
-// - `basePatchNum`, optional, Number: the patch for the left-hand-side
-// of the diff. If `basePatchNum` is provided, then `patchNum` must
-// also be provided.
-// - `edit`, optional, Boolean: whether or not to load the file list with
-// edit controls.
-// - `messageHash`, optional, String: the hash of the change message to
-// scroll to.
-//
-// - GerritNav.View.SEARCH:
-// - `query`, optional, String: the literal search query. If provided,
-// the string will be used as the query, and all other params will be
-// ignored.
-// - `owner`, optional, String: the owner name.
-// - `project`, optional, String: the project name.
-// - `branch`, optional, String: the branch name.
-// - `topic`, optional, String: the topic name.
-// - `hashtag`, optional, String: the hashtag name.
-// - `statuses`, optional, Array<String>: the list of change statuses to
-// search for. If more than one is provided, the search will OR them
-// together.
-// - `offset`, optional, Number: the offset for the query.
-//
-// - GerritNav.View.DIFF:
-// - `changeNum`, required, String: the numeric ID of the change.
-// - `path`, required, String: the filepath of the diff.
-// - `patchNum`, required, Number: the patch for the right-hand-side of
-// the diff.
-// - `basePatchNum`, optional, Number: the patch for the left-hand-side
-// of the diff. If `basePatchNum` is provided, then `patchNum` must
-// also be provided.
-// - `lineNum`, optional, Number: the line number to be selected on load.
-// - `leftSide`, optional, Boolean: if a `lineNum` is provided, a value
-// of true selects the line from base of the patch range. False by
-// default.
-//
-// - GerritNav.View.GROUP:
-// - `groupId`, required, String: the ID of the group.
-// - `detail`, optional, String: the name of the group detail view.
-// Takes any value from GerritNav.GroupDetailView.
-//
-// - GerritNav.View.REPO:
-// - `repoName`, required, String: the name of the repo
-// - `detail`, optional, String: the name of the repo detail view.
-// Takes any value from GerritNav.RepoDetailView.
-//
-// - GerritNav.View.DASHBOARD
-// - `repo`, optional, String.
-// - `sections`, optional, Array of objects with `title` and `query`
-// strings.
-// - `user`, optional, String.
-//
-// - GerritNav.View.ROOT:
-// - no possible parameters.
+// GenerateUrlParameters lists all the possible view parameters.
const uninitialized = () => {
console.warn('Use of uninitialized routing');
@@ -316,11 +258,17 @@
commentLink?: boolean;
}
+export interface GenerateUrlTopicViewParams {
+ view: GerritView.TOPIC;
+ topic?: string;
+}
+
export type GenerateUrlParameters =
| GenerateUrlSearchViewParameters
| GenerateUrlChangeViewParameters
| GenerateUrlRepoViewParameters
| GenerateUrlDashboardViewParameters
+ | GenerateUrlTopicViewParams
| GenerateUrlGroupViewParameters
| GenerateUrlEditViewParameters
| GenerateUrlRootViewParameters
@@ -675,6 +623,15 @@
);
},
+ navigateToTopicPage(topic: string) {
+ this._navigate(
+ this._getUrlFor({
+ view: GerritView.TOPIC,
+ topic,
+ })
+ );
+ },
+
/**
* @param basePatchNum The string 'PARENT' can be used for none.
*/
diff --git a/polygerrit-ui/app/elements/core/gr-router/gr-router.ts b/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
index 65ac9df..5fac268 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
@@ -43,6 +43,7 @@
isGenerateUrlDiffViewParameters,
RepoDetailView,
WeblinkType,
+ GenerateUrlTopicViewParams,
} from '../gr-navigation/gr-navigation';
import {appContext} from '../../../services/app-context';
import {convertToPatchSetNum} from '../../../utils/patch-set-util';
@@ -77,11 +78,14 @@
toSearchParams,
} from '../../../utils/url-util';
import {Execution, LifeCycle, Timing} from '../../../constants/reporting';
+import {KnownExperimentId} from '../../../services/flags/flags';
const RoutePattern = {
ROOT: '/',
DASHBOARD: /^\/dashboard\/(.+)$/,
+ // TODO(dhruvsri): remove /c once Change 322894 lands
+ TOPIC: /^\/c\/topic\/(\w+)\/?$/,
CUSTOM_DASHBOARD: /^\/dashboard\/?$/,
PROJECT_DASHBOARD: /^\/p\/(.+)\/\+\/dashboard\/(.+)/,
LEGACY_PROJECT_DASHBOARD: /^\/projects\/(.+),dashboards\/(.+)/,
@@ -309,6 +313,8 @@
private readonly restApiService = appContext.restApiService;
+ private readonly flagsService = appContext.flagsService;
+
start() {
if (!this._app) {
return;
@@ -355,6 +361,8 @@
url = this._generateChangeUrl(params);
} else if (params.view === GerritView.DASHBOARD) {
url = this._generateDashboardUrl(params);
+ } else if (params.view === GerritView.TOPIC) {
+ url = this._generateTopicPageUrl(params);
} else if (
params.view === GerritView.DIFF ||
params.view === GerritView.EDIT
@@ -577,6 +585,10 @@
}
}
+ _generateTopicPageUrl(params: GenerateUrlTopicViewParams) {
+ return `/c/topic/${params.topic ?? ''}`;
+ }
+
_sectionsToEncodedParams(sections: DashboardSection[], repoName?: RepoName) {
return sections.map(section => {
// If there is a repo name provided, make sure to substitute it into the
@@ -893,6 +905,8 @@
this._mapRoute(RoutePattern.DASHBOARD, '_handleDashboardRoute');
+ this._mapRoute(RoutePattern.TOPIC, '_handleTopicRoute');
+
this._mapRoute(
RoutePattern.CUSTOM_DASHBOARD,
'_handleCustomDashboardRoute'
@@ -1217,6 +1231,13 @@
});
}
+ _handleTopicRoute(data: PageContextWithQueryMap) {
+ this._setParams({
+ view: GerritView.TOPIC,
+ topic: data.params[0],
+ });
+ }
+
/**
* Handle custom dashboard routes.
*
@@ -1534,6 +1555,16 @@
}
_handleQueryRoute(data: PageContextWithQueryMap) {
+ if (this.flagsService.isEnabled(KnownExperimentId.TOPICS_PAGE)) {
+ const query = data.params[0];
+ const terms = query.split(' ');
+ if (terms.length === 1) {
+ const tokens = terms[0].split(':');
+ if (tokens[0] === 'topic') {
+ return GerritNav.navigateToTopicPage(tokens[1]);
+ }
+ }
+ }
this._setParams({
view: GerritView.SEARCH,
query: data.params[0],
diff --git a/polygerrit-ui/app/elements/core/gr-router/gr-router_test.js b/polygerrit-ui/app/elements/core/gr-router/gr-router_test.js
index 7f1a40b..6462816 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router_test.js
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router_test.js
@@ -19,10 +19,11 @@
import './gr-router.js';
import {page} from '../../../utils/page-wrapper-utils.js';
import {GerritNav} from '../gr-navigation/gr-navigation.js';
-import {stubBaseUrl, stubRestApi, addListenerForTest} from '../../../test/test-utils.js';
+import {stubBaseUrl, stubRestApi, addListenerForTest, stubFlags} from '../../../test/test-utils.js';
import {_testOnly_RoutePattern} from './gr-router.js';
import {GerritView} from '../../../services/router/router-model.js';
import {ParentPatchSetNum} from '../../../types/common.js';
+import {KnownExperimentId} from '../../../services/flags/flags.js';
const basicFixture = fixtureFromElement('gr-router');
@@ -214,6 +215,7 @@
'_handleTagListFilterOffsetRoute',
'_handleTagListFilterRoute',
'_handleTagListOffsetRoute',
+ '_handleTopicRoute',
'_handlePluginScreen',
];
@@ -259,6 +261,15 @@
});
suite('generateUrl', () => {
+ test('topic page', () => {
+ const params = {
+ view: GerritView.TOPIC,
+ topic: 'ggh',
+ };
+ assert.equal(element._generateUrl(params),
+ '/c/topic/ggh');
+ });
+
test('search', () => {
let params = {
view: GerritNav.View.SEARCH,
@@ -664,28 +675,27 @@
});
});
+ test('_handleQueryRoute to topic page', () => {
+ stubFlags('isEnabled').withArgs(KnownExperimentId.TOPICS_PAGE)
+ .returns(true);
+ const navStub = sinon.stub(GerritNav, 'navigateToTopicPage');
+ let data = {params: ['topic:abcd']};
+ element._handleQueryRoute(data);
+
+ assert.isTrue(navStub.called);
+
+ // multiple terms so topic page is not loaded
+ data = {params: ['topic:abcd owner:self']};
+ element._handleQueryRoute(data);
+ assert.isTrue(navStub.calledOnce);
+ });
+
test('_handleQueryLegacySuffixRoute', () => {
element._handleQueryLegacySuffixRoute({path: '/q/foo+bar,n,z'});
assert.isTrue(redirectStub.calledOnce);
assert.equal(redirectStub.lastCall.args[0], '/q/foo+bar');
});
- test('_handleQueryRoute', () => {
- const data = {params: ['project:foo/bar/baz']};
- assertDataToParams(data, '_handleQueryRoute', {
- view: GerritNav.View.SEARCH,
- query: 'project:foo/bar/baz',
- offset: undefined,
- });
-
- data.params.push(',123', '123');
- assertDataToParams(data, '_handleQueryRoute', {
- view: GerritNav.View.SEARCH,
- query: 'project:foo/bar/baz',
- offset: '123',
- });
- });
-
test('_handleChangeIdQueryRoute', () => {
const data = {params: ['I0123456789abcdef0123456789abcdef01234567']};
assertDataToParams(data, '_handleChangeIdQueryRoute', {
@@ -1230,6 +1240,19 @@
});
});
+ suite('topic routes', () => {
+ test('_handleTopicRoute', () => {
+ const url = '/c/topic/random/';
+ const groups = url.match(_testOnly_RoutePattern.TOPIC);
+
+ const data = {params: groups.slice(1)};
+ assertDataToParams(data, '_handleTopicRoute', {
+ view: GerritView.TOPIC,
+ topic: 'random',
+ });
+ });
+ });
+
suite('plugin routes', () => {
test('_handlePluginListOffsetRoute', () => {
const data = {params: {}};
diff --git a/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box_html.ts b/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box_html.ts
index 558742a..24d63b3 100644
--- a/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box_html.ts
+++ b/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box_html.ts
@@ -23,9 +23,6 @@
font-family: var(--font-family);
position: absolute;
white-space: nowrap;
- /* This prevents the mouse over the tooltip from interfering with the
- selection. */
- pointer-events: none;
}
</style>
<gr-tooltip
diff --git a/polygerrit-ui/app/elements/edit/gr-edit-file-controls/gr-edit-file-controls.ts b/polygerrit-ui/app/elements/edit/gr-edit-file-controls/gr-edit-file-controls.ts
index 418c368..1b854b4 100644
--- a/polygerrit-ui/app/elements/edit/gr-edit-file-controls/gr-edit-file-controls.ts
+++ b/polygerrit-ui/app/elements/edit/gr-edit-file-controls/gr-edit-file-controls.ts
@@ -50,6 +50,7 @@
justify-content: flex-end;
}
gr-dropdown {
+ --gr-dropdown-item-color: var(--link-color);
--gr-button-padding: var(--spacing-xs) var(--spacing-s);
}
#actions {
@@ -69,7 +70,6 @@
--gr-dropdown-item: {
background-color: transparent;
border: none;
- color: var(--link-color);
text-transform: uppercase;
}
}
diff --git a/polygerrit-ui/app/elements/gr-app-element.ts b/polygerrit-ui/app/elements/gr-app-element.ts
index 7f7749a..be26fcd 100644
--- a/polygerrit-ui/app/elements/gr-app-element.ts
+++ b/polygerrit-ui/app/elements/gr-app-element.ts
@@ -21,6 +21,7 @@
import './documentation/gr-documentation-search/gr-documentation-search';
import './change-list/gr-change-list-view/gr-change-list-view';
import './change-list/gr-dashboard-view/gr-dashboard-view';
+import './topic/gr-topic-view';
import './change/gr-change-view/gr-change-view';
import './core/gr-error-manager/gr-error-manager';
import './core/gr-keyboard-shortcuts-dialog/gr-keyboard-shortcuts-dialog';
@@ -140,6 +141,9 @@
_showDashboardView?: boolean;
@property({type: Boolean})
+ _showTopicView?: boolean;
+
+ @property({type: Boolean})
_showChangeView?: boolean;
@property({type: Boolean})
@@ -355,6 +359,7 @@
this.$.errorView.classList.remove('show');
this._showChangeListView = view === GerritView.SEARCH;
this._showDashboardView = view === GerritView.DASHBOARD;
+ this._showTopicView = view === GerritView.TOPIC;
this._showChangeView = view === GerritView.CHANGE;
this._showDiffView = view === GerritView.DIFF;
this._showSettingsView = view === GerritView.SETTINGS;
diff --git a/polygerrit-ui/app/elements/gr-app-element_html.ts b/polygerrit-ui/app/elements/gr-app-element_html.ts
index a1e6ac9..34c6a35 100644
--- a/polygerrit-ui/app/elements/gr-app-element_html.ts
+++ b/polygerrit-ui/app/elements/gr-app-element_html.ts
@@ -131,6 +131,9 @@
view-state="{{_viewState.dashboardView}}"
></gr-dashboard-view>
</template>
+ <template is="dom-if" if="[[_showTopicView]]">
+ <gr-topic-view params="[[params]]"></gr-topic-view>
+ </template>
<!-- Note that the change view does not have restamp="true" set, because we
want to re-use it as long as the change number does not change. -->
<template id="dom-if-change-view" is="dom-if" if="[[_showChangeView]]">
diff --git a/polygerrit-ui/app/elements/gr-app-types.ts b/polygerrit-ui/app/elements/gr-app-types.ts
index 39070bd..8ff7734 100644
--- a/polygerrit-ui/app/elements/gr-app-types.ts
+++ b/polygerrit-ui/app/elements/gr-app-types.ts
@@ -46,6 +46,11 @@
title?: string;
}
+export interface AppElementTopicParams {
+ view: GerritView.TOPIC;
+ topic?: string;
+}
+
export interface AppElementGroupParams {
view: GerritView.GROUP;
detail?: GroupDetailView;
@@ -141,6 +146,7 @@
export type AppElementParams =
| AppElementDashboardParams
+ | AppElementTopicParams
| AppElementGroupParams
| AppElementAdminParams
| AppElementChangeViewParams
diff --git a/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown_html.ts b/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown_html.ts
index 3c07d94..082a10b 100644
--- a/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown_html.ts
+++ b/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown_html.ts
@@ -58,6 +58,7 @@
padding: var(--spacing-m) var(--spacing-l);
}
li .itemAction {
+ color: var(--gr-dropdown-item-color);
@apply --gr-dropdown-item;
}
li .itemAction.disabled {
@@ -83,6 +84,7 @@
.topContent {
display: block;
padding: var(--spacing-m) var(--spacing-l);
+ color: var(--gr-dropdown-item-color);
@apply --gr-dropdown-item;
}
.bold-text {
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 9013088..1738aaa 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
@@ -93,6 +93,7 @@
padding: 1px;
border-radius: var(--border-radius);
line-height: var(--gr-vote-chip-width, 16px);
+ color: var(--vote-text-color);
}
.vote-chip {
position: relative;
diff --git a/polygerrit-ui/app/elements/topic/gr-topic-view.ts b/polygerrit-ui/app/elements/topic/gr-topic-view.ts
new file mode 100644
index 0000000..be57885
--- /dev/null
+++ b/polygerrit-ui/app/elements/topic/gr-topic-view.ts
@@ -0,0 +1,62 @@
+/**
+ * @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 {customElement, property, state} from 'lit/decorators';
+import {LitElement, html, PropertyValues} from 'lit';
+import {AppElementTopicParams} from '../gr-app-types';
+import {appContext} from '../../services/app-context';
+import {KnownExperimentId} from '../../services/flags/flags';
+import {GerritNav} from '../core/gr-navigation/gr-navigation';
+import {GerritView} from '../../services/router/router-model';
+
+@customElement('gr-topic-view')
+export class GrTopicView extends LitElement {
+ @property()
+ params?: AppElementTopicParams;
+
+ @state()
+ topic?: string;
+
+ private readonly flagsService = appContext.flagsService;
+
+ override willUpdate(changedProperties: PropertyValues) {
+ if (changedProperties.has('params')) {
+ this.paramsChanged();
+ }
+ }
+
+ override render() {
+ return html`<div>Topic page for ${this.topic ?? ''}</div>`;
+ }
+
+ paramsChanged() {
+ if (this.params?.view !== GerritView.TOPIC) return;
+ this.topic = this.params?.topic;
+ if (
+ !this.flagsService.isEnabled(KnownExperimentId.TOPICS_PAGE) &&
+ this.topic
+ ) {
+ GerritNav.navigateToSearchQuery(`topic:${this.topic}`);
+ }
+ }
+}
+
+declare global {
+ interface HTMLElementTagNameMap {
+ 'gr-topic-view': GrTopicView;
+ }
+}
diff --git a/polygerrit-ui/app/services/comments/comments-service.ts b/polygerrit-ui/app/services/comments/comments-service.ts
index 5896b52..b5745a8 100644
--- a/polygerrit-ui/app/services/comments/comments-service.ts
+++ b/polygerrit-ui/app/services/comments/comments-service.ts
@@ -39,24 +39,45 @@
export class CommentsService {
private discardedDrafts?: UIDraft[] = [];
+ private changeNum?: NumericChangeId;
+
+ private patchNum?: PatchSetNum;
+
constructor(readonly restApiService: RestApiService) {
discardedDrafts$.subscribe(
discardedDrafts => (this.discardedDrafts = discardedDrafts)
);
changeNum$.subscribe(changeNum => {
+ this.changeNum = changeNum;
updateStateReset();
- if (!changeNum) return;
- this.reloadComments(changeNum);
- this.reloadRobotComments(changeNum);
- this.reloadDrafts(changeNum);
+ this.reloadAllComments();
});
combineLatest([changeNum$, currentPatchNum$]).subscribe(
- ([changeNum, currentPatchNum]) => {
- if (!changeNum || !currentPatchNum) return;
- this.reloadPortedComments(changeNum, currentPatchNum);
- this.reloadPortedDrafts(changeNum, currentPatchNum);
+ ([changeNum, patchNum]) => {
+ this.changeNum = changeNum;
+ this.patchNum = patchNum;
+ this.reloadAllPortedComments();
}
);
+ document.addEventListener('reload', () => {
+ this.reloadAllComments();
+ this.reloadAllPortedComments();
+ });
+ }
+
+ // Note that this does *not* reload ported comments.
+ reloadAllComments() {
+ if (!this.changeNum) return;
+ this.reloadComments(this.changeNum);
+ this.reloadRobotComments(this.changeNum);
+ this.reloadDrafts(this.changeNum);
+ }
+
+ reloadAllPortedComments() {
+ if (!this.changeNum) return;
+ if (!this.patchNum) return;
+ this.reloadPortedComments(this.changeNum, this.patchNum);
+ this.reloadPortedDrafts(this.changeNum, this.patchNum);
}
reloadComments(changeNum: NumericChangeId): Promise<void> {
diff --git a/polygerrit-ui/app/services/flags/flags.ts b/polygerrit-ui/app/services/flags/flags.ts
index 863f95f..318ea35 100644
--- a/polygerrit-ui/app/services/flags/flags.ts
+++ b/polygerrit-ui/app/services/flags/flags.ts
@@ -27,4 +27,5 @@
NEW_IMAGE_DIFF_UI = 'UiFeature__new_image_diff_ui',
CHECKS_DEVELOPER = 'UiFeature__checks_developer',
SUBMIT_REQUIREMENTS_UI = 'UiFeature__submit_requirements_ui',
+ TOPICS_PAGE = 'UiFeature__topics_page',
}
diff --git a/polygerrit-ui/app/services/router/router-model.ts b/polygerrit-ui/app/services/router/router-model.ts
index ae3d848..584b8d7 100644
--- a/polygerrit-ui/app/services/router/router-model.ts
+++ b/polygerrit-ui/app/services/router/router-model.ts
@@ -33,6 +33,7 @@
ROOT = 'root',
SEARCH = 'search',
SETTINGS = 'settings',
+ TOPIC = 'topic',
}
export interface RouterState {
@@ -59,6 +60,7 @@
// Must only be used by the router service or whatever is in control of this
// model.
+// TODO: Consider keeping params of type AppElementParams entirely in the state
export function updateState(
view?: GerritView,
changeNum?: NumericChangeId,
diff --git a/polygerrit-ui/app/services/shortcuts/shortcuts-config.ts b/polygerrit-ui/app/services/shortcuts/shortcuts-config.ts
index 3c9e058..18b321a 100644
--- a/polygerrit-ui/app/services/shortcuts/shortcuts-config.ts
+++ b/polygerrit-ui/app/services/shortcuts/shortcuts-config.ts
@@ -180,13 +180,13 @@
Shortcut.CURSOR_NEXT_CHANGE,
ShortcutSection.ACTIONS,
'Select next change',
- {key: 'j'}
+ {key: 'j', allowRepeat: true}
);
describe(
Shortcut.CURSOR_PREV_CHANGE,
ShortcutSection.ACTIONS,
'Select previous change',
- {key: 'k'}
+ {key: 'k', allowRepeat: true}
);
describe(
Shortcut.OPEN_CHANGE,
@@ -316,15 +316,15 @@
Shortcut.NEXT_LINE,
ShortcutSection.DIFFS,
'Go to next line',
- {key: 'j'},
- {key: Key.DOWN}
+ {key: 'j', allowRepeat: true},
+ {key: Key.DOWN, allowRepeat: true}
);
describe(
Shortcut.PREV_LINE,
ShortcutSection.DIFFS,
'Go to previous line',
- {key: 'k'},
- {key: Key.UP}
+ {key: 'k', allowRepeat: true},
+ {key: Key.UP, allowRepeat: true}
);
describe(
Shortcut.VISIBLE_LINE,
@@ -480,15 +480,15 @@
Shortcut.CURSOR_NEXT_FILE,
ShortcutSection.FILE_LIST,
'Select next file',
- {key: 'j'},
- {key: Key.DOWN}
+ {key: 'j', allowRepeat: true},
+ {key: Key.DOWN, allowRepeat: true}
);
describe(
Shortcut.CURSOR_PREV_FILE,
ShortcutSection.FILE_LIST,
'Select previous file',
- {key: 'k'},
- {key: Key.UP}
+ {key: 'k', allowRepeat: true},
+ {key: Key.UP, allowRepeat: true}
);
describe(
Shortcut.OPEN_FILE,
diff --git a/polygerrit-ui/app/services/shortcuts/shortcuts-service.ts b/polygerrit-ui/app/services/shortcuts/shortcuts-service.ts
index a26fa08..f2e9e98 100644
--- a/polygerrit-ui/app/services/shortcuts/shortcuts-service.ts
+++ b/polygerrit-ui/app/services/shortcuts/shortcuts-service.ts
@@ -137,7 +137,7 @@
listener: (e: KeyboardEvent) => void
) {
const wrappedListener = (e: KeyboardEvent) => {
- if (e.repeat) return;
+ if (e.repeat && !shortcut.allowRepeat) return;
if (!eventMatchesShortcut(e, shortcut)) return;
if (shortcut.combo) {
if (!this.isInSpecificComboKeyMode(shortcut.combo)) return;
diff --git a/polygerrit-ui/app/services/shortcuts/shortcuts-service_test.ts b/polygerrit-ui/app/services/shortcuts/shortcuts-service_test.ts
index 05c4f53..a024159 100644
--- a/polygerrit-ui/app/services/shortcuts/shortcuts-service_test.ts
+++ b/polygerrit-ui/app/services/shortcuts/shortcuts-service_test.ts
@@ -213,7 +213,10 @@
{
shortcut: Shortcut.NEXT_LINE,
text: 'Go to next line',
- bindings: [{key: 'j'}, {key: 'ArrowDown'}],
+ bindings: [
+ {allowRepeat: true, key: 'j'},
+ {allowRepeat: true, key: 'ArrowDown'},
+ ],
},
],
[ShortcutSection.NAVIGATION]: [
@@ -234,7 +237,10 @@
{
shortcut: Shortcut.NEXT_LINE,
text: 'Go to next line',
- bindings: [{key: 'j'}, {key: 'ArrowDown'}],
+ bindings: [
+ {allowRepeat: true, key: 'j'},
+ {allowRepeat: true, key: 'ArrowDown'},
+ ],
},
],
[ShortcutSection.EVERYWHERE]: [
diff --git a/polygerrit-ui/app/test/test-utils.ts b/polygerrit-ui/app/test/test-utils.ts
index 63c125e..faecda0 100644
--- a/polygerrit-ui/app/test/test-utils.ts
+++ b/polygerrit-ui/app/test/test-utils.ts
@@ -27,6 +27,7 @@
import {UserService} from '../services/user/user-service';
import {ShortcutsService} from '../services/shortcuts/shortcuts-service';
import {queryAndAssert, query} from '../utils/common-util';
+import {FlagsService} from '../services/flags/flags';
export {query, queryAll, queryAndAssert} from '../utils/common-util';
export interface MockPromise<T> extends Promise<T> {
@@ -138,6 +139,10 @@
return sinon.stub(appContext.reportingService, method);
}
+export function stubFlags<K extends keyof FlagsService>(method: K) {
+ return sinon.stub(appContext.flagsService, method);
+}
+
export type SinonSpyMember<F extends (...args: any) => any> = SinonSpy<
Parameters<F>,
ReturnType<F>
diff --git a/polygerrit-ui/app/utils/dom-util.ts b/polygerrit-ui/app/utils/dom-util.ts
index e2fa8fe..bd0f742 100644
--- a/polygerrit-ui/app/utils/dom-util.ts
+++ b/polygerrit-ui/app/utils/dom-util.ts
@@ -338,6 +338,8 @@
combo?: ComboKey;
/** Defaults to no modifiers. */
modifiers?: Modifier[];
+ /** Defaults to false. If true, then `event.repeat === true` is allowed. */
+ allowRepeat?: boolean;
}
const ALPHA_NUM = new RegExp(/^[A-Za-z0-9]$/);
@@ -406,7 +408,7 @@
}
) {
const wrappedListener = (e: KeyboardEvent) => {
- if (e.repeat) return;
+ if (e.repeat && !shortcut.allowRepeat) return;
if (options.shouldSuppress && shouldSuppress(e)) return;
if (eventMatchesShortcut(e, shortcut)) {
listener(e);
diff --git a/tools/BUILD b/tools/BUILD
index 5d8491a..3fd2a0f 100644
--- a/tools/BUILD
+++ b/tools/BUILD
@@ -180,7 +180,7 @@
"-Xep:EqualsUsingHashCode:ERROR",
"-Xep:EqualsWrongThing:ERROR",
"-Xep:ErroneousThreadPoolConstructorChecker:ERROR",
- # "-Xep:EscapedEntity:WARN",
+ "-Xep:EscapedEntity:WARN",
"-Xep:ExpectedExceptionChecker:ERROR",
"-Xep:ExtendingJUnitAssert:ERROR",
"-Xep:ExtendsAutoValue:ERROR",
@@ -223,7 +223,7 @@
"-Xep:Incomparable:ERROR",
"-Xep:IncompatibleArgumentType:ERROR",
"-Xep:IncompatibleModifiers:ERROR",
- # "-Xep:InconsistentCapitalization:WARN",
+ "-Xep:InconsistentCapitalization:ERROR",
"-Xep:InconsistentHashCode:ERROR",
"-Xep:IncrementInForLoopAndHeader:ERROR",
"-Xep:IndexOfChar:ERROR",
