Merge "Submit requirements: Add a custom serializer for ObjectId"
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..58a3724 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.
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/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/git/ChangeMessageModifier.java b/java/com/google/gerrit/server/git/ChangeMessageModifier.java
index 580c0b9..3424477 100644
--- a/java/com/google/gerrit/server/git/ChangeMessageModifier.java
+++ b/java/com/google/gerrit/server/git/ChangeMessageModifier.java
@@ -14,6 +14,7 @@
package com.google.gerrit.server.git;
+import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.BranchNameKey;
import com.google.gerrit.extensions.annotations.ExtensionPoint;
import org.eclipse.jgit.revwalk.RevCommit;
@@ -42,10 +43,13 @@
* @param original the commit of the change being submitted. <b>Note that its commit message may
* be different than newCommitMessage argument.</b>
* @param mergeTip the current HEAD of the destination branch, which will be a parent of a new
- * commit being generated
+ * commit being generated. mergeTip can be null if the destination branch does not yet exist.
* @param destination the branch onto which the change is being submitted
* @return a new not null commit message.
*/
String onSubmit(
- String newCommitMessage, RevCommit original, RevCommit mergeTip, BranchNameKey destination);
+ String newCommitMessage,
+ RevCommit original,
+ @Nullable RevCommit mergeTip,
+ BranchNameKey destination);
}
diff --git a/java/com/google/gerrit/server/git/MergeUtil.java b/java/com/google/gerrit/server/git/MergeUtil.java
index 3a4d407..99cb9b0 100644
--- a/java/com/google/gerrit/server/git/MergeUtil.java
+++ b/java/com/google/gerrit/server/git/MergeUtil.java
@@ -31,6 +31,7 @@
import com.google.common.collect.Sets;
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.common.FooterConstants;
+import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.BooleanProjectConfig;
import com.google.gerrit.entities.BranchNameKey;
@@ -630,7 +631,7 @@
* @return new message
*/
public String createCommitMessageOnSubmit(
- RevCommit n, RevCommit mergeTip, ChangeNotes notes, PatchSet.Id id) {
+ RevCommit n, @Nullable RevCommit mergeTip, ChangeNotes notes, PatchSet.Id id) {
return commitMessageGenerator.generate(
n, mergeTip, notes.getChange().getDest(), createDetailedCommitMessage(n, notes, id));
}
diff --git a/java/com/google/gerrit/server/git/PluggableCommitMessageGenerator.java b/java/com/google/gerrit/server/git/PluggableCommitMessageGenerator.java
index 804a218..1f5a330 100644
--- a/java/com/google/gerrit/server/git/PluggableCommitMessageGenerator.java
+++ b/java/com/google/gerrit/server/git/PluggableCommitMessageGenerator.java
@@ -18,6 +18,7 @@
import static java.util.Objects.requireNonNull;
import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.BranchNameKey;
import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.extensions.registration.Extension;
@@ -41,7 +42,10 @@
* modify the message.
*/
public String generate(
- RevCommit original, RevCommit mergeTip, BranchNameKey dest, String originalMessage) {
+ RevCommit original,
+ @Nullable RevCommit mergeTip,
+ BranchNameKey dest,
+ String originalMessage) {
requireNonNull(original.getRawBuffer());
if (mergeTip != null) {
requireNonNull(mergeTip.getRawBuffer());
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/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..45d1f5a 100644
--- a/java/com/google/gerrit/server/restapi/change/DeleteVote.java
+++ b/java/com/google/gerrit/server/restapi/change/DeleteVote.java
@@ -81,7 +81,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 +108,7 @@
this.removeReviewerControl = removeReviewerControl;
this.projectCache = projectCache;
this.messageIdGenerator = messageIdGenerator;
- this.attentionSetOpfactory = attentionSetOpFactory;
+ this.attentionSetOpFactory = attentionSetOpFactory;
this.currentUserProvider = currentUserProvider;
}
@@ -149,7 +149,7 @@
if (!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));
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..4bce5d8 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/AttentionSetIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/AttentionSetIT.java
@@ -261,6 +261,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"));
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/polygerrit-ui/app/BUILD b/polygerrit-ui/app/BUILD
index 980abb4..47c820a 100644
--- a/polygerrit-ui/app/BUILD
+++ b/polygerrit-ui/app/BUILD
@@ -95,7 +95,6 @@
# TODO: fix problems reported by template checker in these files.
ignore_templates_list = [
"elements/admin/gr-admin-view/gr-admin-view_html.ts",
- "elements/admin/gr-create-repo-dialog/gr-create-repo-dialog_html.ts",
"elements/admin/gr-group-members/gr-group-members_html.ts",
"elements/admin/gr-permission/gr-permission_html.ts",
"elements/admin/gr-plugin-list/gr-plugin-list_html.ts",
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 63f6601..934e3fb 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
@@ -15,18 +15,11 @@
* limitations under the License.
*/
import '@polymer/iron-input/iron-input';
-import '../../../styles/gr-form-styles';
-import '../../../styles/shared-styles';
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';
import {page} from '../../../utils/page-wrapper-utils';
-import {customElement, observe, property} from '@polymer/decorators';
import {
BranchName,
GroupId,
@@ -35,63 +28,173 @@
} from '../../../types/common';
import {AutocompleteQuery} from '../../shared/gr-autocomplete/gr-autocomplete';
import {appContext} from '../../../services/app-context';
+import {convertToString} from '../../../utils/string-util';
+import {formStyles} from '../../../styles/gr-form-styles';
+import {sharedStyles} from '../../../styles/shared-styles';
+import {LitElement, css, html} from 'lit';
+import {customElement, query, property, state} from 'lit/decorators';
+import {fireEvent} from '../../../utils/event-util';
declare global {
+ interface HTMLElementEventMap {
+ 'text-changed': CustomEvent;
+ 'value-changed': CustomEvent;
+ }
interface HTMLElementTagNameMap {
'gr-create-repo-dialog': GrCreateRepoDialog;
}
}
-export interface GrCreateRepoDialog {
- $: {
- initialCommit: GrSelect;
- parentRepo: GrSelect;
- repoNameInput: HTMLInputElement;
- rightsInheritFromInput: GrAutocomplete;
- };
-}
-
@customElement('gr-create-repo-dialog')
-export class GrCreateRepoDialog extends PolymerElement {
- static get template() {
- return htmlTemplate;
- }
+export class GrCreateRepoDialog extends LitElement {
+ /**
+ * Fired when repostiory name is entered.
+ *
+ * @event new-repo-name
+ */
- @property({type: Boolean, notify: true})
- hasNewRepoName = false;
+ @query('input')
+ input?: HTMLInputElement;
- @property({type: Object})
- _repoConfig: ProjectInput & {name: RepoName} = {
+ @property({type: Boolean})
+ nameChanged = false;
+
+ /* private but used in test */
+ @state() repoConfig: ProjectInput & {name: RepoName} = {
create_empty_commit: true,
permissions_only: false,
name: '' as RepoName,
branches: [],
};
- @property({type: String})
- _defaultBranch?: BranchName;
+ /* private but used in test */
+ @state() defaultBranch?: BranchName;
- @property({type: Boolean})
- _repoCreated = false;
+ /* private but used in test */
+ @state() repoCreated = false;
- @property({type: String})
- _repoOwner?: string;
+ /* private but used in test */
+ @state() repoOwner?: string;
- @property({type: String})
- _repoOwnerId?: GroupId;
+ /* private but used in test */
+ @state() repoOwnerId?: GroupId;
- @property({type: Object})
- _query: AutocompleteQuery;
+ private readonly query: AutocompleteQuery;
- @property({type: Object})
- _queryGroups: AutocompleteQuery;
+ private readonly queryGroups: AutocompleteQuery;
private readonly restApiService = appContext.restApiService;
constructor() {
super();
- this._query = (input: string) => this._getRepoSuggestions(input);
- this._queryGroups = (input: string) => this._getGroupSuggestions(input);
+ this.query = (input: string) => this.getRepoSuggestions(input);
+ this.queryGroups = (input: string) => this.getGroupSuggestions(input);
+ }
+
+ static override get styles() {
+ return [
+ formStyles,
+ sharedStyles,
+ css`
+ :host {
+ display: inline-block;
+ }
+ input {
+ width: 20em;
+ }
+ gr-autocomplete {
+ width: 20em;
+ }
+ `,
+ ];
+ }
+
+ override render() {
+ return html`
+ <div class="gr-form-styles">
+ <div id="form">
+ <section>
+ <span class="title">Repository name</span>
+ <iron-input
+ .bindValue=${convertToString(this.repoConfig.name)}
+ @bind-value-changed=${this.handleNameBindValueChanged}
+ >
+ <input id="repoNameInput" autocomplete="on" />
+ </iron-input>
+ </section>
+ <section>
+ <span class="title">Default Branch</span>
+ <iron-input
+ .bindValue=${convertToString(this.defaultBranch)}
+ @bind-value-changed=${this.handleBranchNameBindValueChanged}
+ >
+ <input id="defaultBranchNameInput" autocomplete="off" />
+ </iron-input>
+ </section>
+ <section>
+ <span class="title">Rights inherit from</span>
+ <span class="value">
+ <gr-autocomplete
+ id="rightsInheritFromInput"
+ .text=${convertToString(this.repoConfig.parent)}
+ .query=${this.query}
+ .placeholder="Optional, defaults to 'All-Projects'"
+ @text-changed=${this.handleRightsTextChanged}
+ >
+ </gr-autocomplete>
+ </span>
+ </section>
+ <section>
+ <span class="title">Owner</span>
+ <span class="value">
+ <gr-autocomplete
+ id="ownerInput"
+ .text=${convertToString(this.repoOwner)}
+ .value=${convertToString(this.repoOwnerId)}
+ .query=${this.queryGroups}
+ @text-changed=${this.handleOwnerTextChanged}
+ @value-changed=${this.handleOwnerValueChanged}
+ >
+ </gr-autocomplete>
+ </span>
+ </section>
+ <section>
+ <span class="title">Create initial empty commit</span>
+ <span class="value">
+ <gr-select
+ id="initialCommit"
+ .bindValue=${this.repoConfig.create_empty_commit}
+ @bind-value-changed=${this
+ .handleCreateEmptyCommitBindValueChanged}
+ >
+ <select>
+ <option value="false">False</option>
+ <option value="true">True</option>
+ </select>
+ </gr-select>
+ </span>
+ </section>
+ <section>
+ <span class="title"
+ >Only serve as parent for other repositories</span
+ >
+ <span class="value">
+ <gr-select
+ id="parentRepo"
+ .bindValue=${this.repoConfig.permissions_only}
+ @bind-value-changed=${this
+ .handlePermissionsOnlyBindValueChanged}
+ >
+ <select>
+ <option value="false">False</option>
+ <option value="true">True</option>
+ </select>
+ </gr-select>
+ </span>
+ </section>
+ </div>
+ </div>
+ `;
}
_computeRepoUrl(repoName: string) {
@@ -99,44 +202,76 @@
}
override focus() {
- this.shadowRoot?.querySelector('input')?.focus();
+ this.input?.focus();
}
- @observe('_repoConfig.name')
- _updateRepoName(name: string) {
- this.hasNewRepoName = !!name;
+ async handleCreateRepo() {
+ if (this.defaultBranch) this.repoConfig.branches = [this.defaultBranch];
+ if (this.repoOwnerId) this.repoConfig.owners = [this.repoOwnerId];
+ const repoRegistered = await this.restApiService.createRepo(
+ this.repoConfig
+ );
+ if (repoRegistered.status === 201) {
+ this.repoCreated = true;
+ page.show(this._computeRepoUrl(this.repoConfig.name));
+ }
+ return repoRegistered;
}
- handleCreateRepo() {
- if (this._defaultBranch) this._repoConfig.branches = [this._defaultBranch];
- if (this._repoOwnerId) this._repoConfig.owners = [this._repoOwnerId];
- return this.restApiService
- .createRepo(this._repoConfig)
- .then(repoRegistered => {
- if (repoRegistered.status === 201) {
- this._repoCreated = true;
- page.show(this._computeRepoUrl(this._repoConfig.name));
- }
- });
+ private async getRepoSuggestions(input: string) {
+ const response = await this.restApiService.getSuggestedProjects(input);
+
+ const repos = [];
+ for (const [name, project] of Object.entries(response ?? {})) {
+ repos.push({name, value: project.id});
+ }
+ return repos;
}
- _getRepoSuggestions(input: string) {
- return this.restApiService.getSuggestedProjects(input).then(response => {
- const repos = [];
- for (const [name, project] of Object.entries(response ?? {})) {
- repos.push({name, value: project.id});
- }
- return repos;
- });
+ private async getGroupSuggestions(input: string) {
+ const response = await this.restApiService.getSuggestedGroups(input);
+
+ const groups = [];
+ for (const [name, group] of Object.entries(response ?? {})) {
+ groups.push({name, value: decodeURIComponent(group.id)});
+ }
+ return groups;
}
- _getGroupSuggestions(input: string) {
- return this.restApiService.getSuggestedGroups(input).then(response => {
- const groups = [];
- for (const [name, group] of Object.entries(response ?? {})) {
- groups.push({name, value: decodeURIComponent(group.id)});
- }
- return groups;
- });
+ private handleRightsTextChanged(e: CustomEvent) {
+ this.repoConfig.parent = e.detail.value as RepoName;
+ this.requestUpdate();
+ }
+
+ private handleOwnerTextChanged(e: CustomEvent) {
+ this.repoOwner = e.detail.value;
+ }
+
+ private handleOwnerValueChanged(e: CustomEvent) {
+ this.repoOwnerId = e.detail.value as GroupId;
+ }
+
+ private handleNameBindValueChanged(e: CustomEvent) {
+ this.repoConfig.name = e.detail.value as RepoName;
+ // nameChanged needs to be set before the event is fired,
+ // because when the event is fired, gr-repo-list gets
+ // the nameChanged value.
+ this.nameChanged = !!e.detail.value;
+ fireEvent(this, 'new-repo-name');
+ this.requestUpdate();
+ }
+
+ private handleBranchNameBindValueChanged(e: CustomEvent) {
+ this.defaultBranch = e.detail.value as BranchName;
+ }
+
+ private handleCreateEmptyCommitBindValueChanged(e: CustomEvent) {
+ this.repoConfig.create_empty_commit = e.detail.value;
+ this.requestUpdate();
+ }
+
+ private handlePermissionsOnlyBindValueChanged(e: CustomEvent) {
+ this.repoConfig.permissions_only = e.detail.value;
+ this.requestUpdate();
}
}
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
deleted file mode 100644
index d0a6b7f..0000000
--- a/polygerrit-ui/app/elements/admin/gr-create-repo-dialog/gr-create-repo-dialog_html.ts
+++ /dev/null
@@ -1,103 +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">
- /* Workaround for empty style block - see https://github.com/Polymer/tools/issues/408 */
- </style>
- <style include="gr-form-styles">
- :host {
- display: inline-block;
- }
- input {
- width: 20em;
- }
- gr-autocomplete {
- width: 20em;
- }
- </style>
-
- <div class="gr-form-styles">
- <div id="form">
- <section>
- <span class="title">Repository name</span>
- <iron-input bind-value="{{_repoConfig.name}}">
- <input id="repoNameInput" autocomplete="on" />
- </iron-input>
- </section>
- <section>
- <span class="title">Default Branch</span>
- <iron-input bind-value="{{_defaultBranch}}">
- <input id="defaultBranchNameInput" autocomplete="off" />
- </iron-input>
- </section>
- <section>
- <span class="title">Rights inherit from</span>
- <span class="value">
- <gr-autocomplete
- id="rightsInheritFromInput"
- text="{{_repoConfig.parent}}"
- query="[[_query]]"
- placeholder="Optional, defaults to 'All-Projects'"
- >
- </gr-autocomplete>
- </span>
- </section>
- <section>
- <span class="title">Owner</span>
- <span class="value">
- <gr-autocomplete
- id="ownerInput"
- text="{{_repoOwner}}"
- value="{{_repoOwnerId}}"
- query="[[_queryGroups]]"
- >
- </gr-autocomplete>
- </span>
- </section>
- <section>
- <span class="title">Create initial empty commit</span>
- <span class="value">
- <gr-select
- id="initialCommit"
- bind-value="{{_repoConfig.create_empty_commit}}"
- >
- <select>
- <option value="false">False</option>
- <option value="true">True</option>
- </select>
- </gr-select>
- </span>
- </section>
- <section>
- <span class="title">Only serve as parent for other repositories</span>
- <span class="value">
- <gr-select
- id="parentRepo"
- bind-value="{{_repoConfig.permissions_only}}"
- >
- <select>
- <option value="false">False</option>
- <option value="true">True</option>
- </select>
- </gr-select>
- </span>
- </section>
- </div>
- </div>
-`;
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
index 6485bae..d3e2171 100644
--- 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
@@ -18,26 +18,35 @@
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 {
+ mockPromise,
+ queryAndAssert,
+ stubRestApi,
+} from '../../../test/test-utils';
import {BranchName, GroupId, RepoName} from '../../../types/common';
+import {GrAutocomplete} from '../../shared/gr-autocomplete/gr-autocomplete';
+import {GrSelect} from '../../shared/gr-select/gr-select';
const basicFixture = fixtureFromElement('gr-create-repo-dialog');
suite('gr-create-repo-dialog tests', () => {
let element: GrCreateRepoDialog;
- setup(() => {
+ setup(async () => {
element = basicFixture.instantiate();
+ await element.updateComplete;
});
test('default values are populated', () => {
- assert.isTrue(element.$.initialCommit.bindValue);
- assert.isFalse(element.$.parentRepo.bindValue);
+ assert.isTrue(
+ queryAndAssert<GrSelect>(element, '#initialCommit').bindValue
+ );
+ assert.isFalse(queryAndAssert<GrSelect>(element, '#parentRepo').bindValue);
});
test('repo created', async () => {
const configInputObj = {
- name: 'test-repo' as RepoName,
+ name: 'test-repo-new' as RepoName,
create_empty_commit: true,
parent: 'All-Project' as RepoName,
permissions_only: false,
@@ -47,27 +56,38 @@
Promise.resolve(new Response())
);
- assert.isFalse(element.hasNewRepoName);
+ const promise = mockPromise();
+ element.addEventListener('new-repo-name', () => {
+ promise.resolve();
+ });
- element._repoConfig = {
+ 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.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;
+ const repoNameInput = queryAndAssert<HTMLInputElement>(
+ element,
+ '#repoNameInput'
+ );
+ repoNameInput.value = configInputObj.name;
+ repoNameInput.dispatchEvent(
+ new Event('input', {bubbles: true, composed: true})
+ );
+ queryAndAssert<GrAutocomplete>(element, '#rightsInheritFromInput').value =
+ configInputObj.parent;
+ queryAndAssert<GrSelect>(element, '#initialCommit').bindValue =
+ configInputObj.create_empty_commit;
+ queryAndAssert<GrSelect>(element, '#parentRepo').bindValue =
+ configInputObj.permissions_only;
- assert.isTrue(element.hasNewRepoName);
-
- assert.deepEqual(element._repoConfig, configInputObj);
+ assert.deepEqual(element.repoConfig, configInputObj);
await element.handleCreateRepo();
assert.isTrue(
@@ -77,5 +97,10 @@
branches: ['main' as BranchName],
})
);
+
+ await promise;
+
+ assert.equal(element.repoConfig.name, configInputObj.name);
+ assert.equal(element.nameChanged, true);
});
});
diff --git a/polygerrit-ui/app/elements/admin/gr-repo-list/gr-repo-list.ts b/polygerrit-ui/app/elements/admin/gr-repo-list/gr-repo-list.ts
index adcfb64..8e50b5f 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo-list/gr-repo-list.ts
+++ b/polygerrit-ui/app/elements/admin/gr-repo-list/gr-repo-list.ts
@@ -14,24 +14,27 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-import '../../../styles/gr-table-styles';
-import '../../../styles/shared-styles';
import '../../shared/gr-dialog/gr-dialog';
import '../../shared/gr-list-view/gr-list-view';
import '../../shared/gr-overlay/gr-overlay';
import '../gr-create-repo-dialog/gr-create-repo-dialog';
-import {PolymerElement} from '@polymer/polymer/polymer-element';
-import {htmlTemplate} from './gr-repo-list_html';
import {GerritNav} from '../../core/gr-navigation/gr-navigation';
-import {customElement, property, observe, computed} from '@polymer/decorators';
import {AppElementAdminParams} from '../../gr-app-types';
import {GrOverlay} from '../../shared/gr-overlay/gr-overlay';
-import {RepoName, ProjectInfoWithName} from '../../../types/common';
+import {
+ RepoName,
+ ProjectInfoWithName,
+ WebLinkInfo,
+} from '../../../types/common';
import {GrCreateRepoDialog} from '../gr-create-repo-dialog/gr-create-repo-dialog';
import {ProjectState, SHOWN_ITEMS_COUNT} from '../../../constants/constants';
import {fireTitleChange} from '../../../utils/event-util';
import {appContext} from '../../../services/app-context';
import {encodeURL, getBaseUrl} from '../../../utils/url-util';
+import {tableStyles} from '../../../styles/gr-table-styles';
+import {sharedStyles} from '../../../styles/shared-styles';
+import {LitElement, PropertyValues, css, html} from 'lit';
+import {customElement, property, query, state} from 'lit/decorators';
declare global {
interface HTMLElementTagNameMap {
@@ -39,151 +42,259 @@
}
}
-export interface GrRepoList {
- $: {
- createOverlay: GrOverlay;
- createNewModal: GrCreateRepoDialog;
- };
-}
-
@customElement('gr-repo-list')
-export class GrRepoList extends PolymerElement {
- static get template() {
- return htmlTemplate;
- }
+export class GrRepoList extends LitElement {
+ @query('#createOverlay')
+ createOverlay?: GrOverlay;
+
+ @query('#createNewModal')
+ createNewModal?: GrCreateRepoDialog;
@property({type: Object})
params?: AppElementAdminParams;
- @property({type: Number})
- _offset = 0;
+ @state() offset = 0;
- @property({type: String})
- readonly _path = '/admin/repos';
+ @state() newRepoName = false;
- @property({type: Boolean})
- _hasNewRepoName = false;
+ @state() createNewCapability = false;
- @property({type: Boolean})
- _createNewCapability = false;
+ @state() repos: ProjectInfoWithName[] = [];
- @property({type: Array})
- _repos: ProjectInfoWithName[] = [];
+ @state() reposPerPage = 25;
- @property({type: Number})
- _reposPerPage = 25;
+ @state() loading = true;
- @property({type: Boolean})
- _loading = true;
+ @state() filter = '';
- @property({type: String})
- _filter = '';
-
- @computed('_repos')
- get _shownRepos() {
- return this._repos.slice(0, SHOWN_ITEMS_COUNT);
- }
+ @state() readonly path = '/admin/repos';
private readonly restApiService = appContext.restApiService;
- override connectedCallback() {
+ override async connectedCallback() {
super.connectedCallback();
- this._getCreateRepoCapability();
+ await this.getCreateRepoCapability();
fireTitleChange(this, 'Repos');
- this._maybeOpenCreateOverlay(this.params);
+ this.maybeOpenCreateOverlay(this.params);
}
- @observe('params')
- _paramsChanged(params: AppElementAdminParams) {
- this._loading = true;
- this._filter = params?.filter ?? '';
- this._offset = Number(params?.offset ?? 0);
+ static override get styles() {
+ return [
+ tableStyles,
+ sharedStyles,
+ css`
+ .genericList tr td:last-of-type {
+ text-align: left;
+ }
+ .genericList tr th:last-of-type {
+ text-align: left;
+ }
+ .readOnly {
+ text-align: center;
+ }
+ .changesLink,
+ .name,
+ .repositoryBrowser,
+ .readOnly {
+ white-space: nowrap;
+ }
+ `,
+ ];
+ }
- return this._getRepos(this._filter, this._reposPerPage, this._offset);
+ override render() {
+ return html`
+ <gr-list-view
+ .createNew=${this.createNewCapability}
+ .filter=${this.filter}
+ .itemsPerPage=${this.reposPerPage}
+ .items=${this.repos}
+ .loading=${this.loading}
+ .offset=${this.offset}
+ .path=${this.path}
+ @create-clicked=${this.handleCreateClicked}
+ >
+ <table id="list" class="genericList">
+ <tbody>
+ <tr class="headerRow">
+ <th class="name topHeader">Repository Name</th>
+ <th class="repositoryBrowser topHeader">Repository Browser</th>
+ <th class="changesLink topHeader">Changes</th>
+ <th class="topHeader readOnly">Read only</th>
+ <th class="description topHeader">Repository Description</th>
+ </tr>
+ <tr
+ id="loading"
+ class="loadingMsg ${this.computeLoadingClass(this.loading)}"
+ >
+ <td>Loading...</td>
+ </tr>
+ </tbody>
+ <tbody class="${this.computeLoadingClass(this.loading)}">
+ ${this.renderRepoList()}
+ </tbody>
+ </table>
+ </gr-list-view>
+ <gr-overlay id="createOverlay" with-backdrop>
+ <gr-dialog
+ id="createDialog"
+ class="confirmDialog"
+ ?disabled=${!this.newRepoName}
+ confirm-label="Create"
+ @confirm=${this.handleCreateRepo}
+ @cancel=${this.handleCloseCreate}
+ >
+ <div class="header" slot="header">Create Repository</div>
+ <div class="main" slot="main">
+ <gr-create-repo-dialog
+ id="createNewModal"
+ @new-repo-name=${this.handleNewRepoName}
+ ></gr-create-repo-dialog>
+ </div>
+ </gr-dialog>
+ </gr-overlay>
+ `;
+ }
+
+ private renderRepoList() {
+ const shownRepos = this.repos.slice(0, SHOWN_ITEMS_COUNT);
+ return shownRepos.map(item => this.renderRepo(item));
+ }
+
+ private renderRepo(item: ProjectInfoWithName) {
+ return html`
+ <tr class="table">
+ <td class="name">
+ <a href="${this.computeRepoUrl(item.name)}">${item.name}</a>
+ </td>
+ <td class="repositoryBrowser">${this.renderWebLinks(item)}</td>
+ <td class="changesLink">
+ <a href="${this.computeChangesLink(item.name)}">view all</a>
+ </td>
+ <td class="readOnly">
+ ${item.state === ProjectState.READ_ONLY ? 'Y' : ''}
+ </td>
+ <td class="description">${item.description}</td>
+ </tr>
+ `;
+ }
+
+ private renderWebLinks(links: ProjectInfoWithName) {
+ const webLinks = links.web_links ? links.web_links : [];
+ return webLinks.map(link => this.renderWebLink(link));
+ }
+
+ private renderWebLink(link: WebLinkInfo) {
+ return html`
+ <a href="${link.url}" class="webLink" rel="noopener" target="_blank">
+ ${link.name}
+ </a>
+ `;
+ }
+
+ override willUpdate(changedProperties: PropertyValues) {
+ if (changedProperties.has('params')) {
+ this._paramsChanged();
+ }
+ }
+
+ async _paramsChanged() {
+ const params = this.params;
+ this.loading = true;
+ this.filter = params?.filter ?? '';
+ this.offset = Number(params?.offset ?? 0);
+
+ return await this.getRepos();
}
/**
- * Opens the create overlay if the route has a hash 'create'
+ * Opens the create overlay if the route has a hash 'create'.
+ *
+ * private but used in test
*/
- _maybeOpenCreateOverlay(params?: AppElementAdminParams) {
+ maybeOpenCreateOverlay(params?: AppElementAdminParams) {
if (params?.openCreateModal) {
- this.$.createOverlay.open();
+ this.createOverlay?.open();
}
}
- _computeRepoUrl(name: string) {
- return getBaseUrl() + this._path + '/' + encodeURL(name, true);
+ private computeRepoUrl(name: string) {
+ return `${getBaseUrl()}${this.path}/${encodeURL(name, true)}`;
}
- _computeChangesLink(name: string) {
+ private computeChangesLink(name: string) {
return GerritNav.getUrlForProjectChanges(name as RepoName);
}
- _getCreateRepoCapability() {
- return this.restApiService.getAccount().then(account => {
- if (!account) {
- return;
- }
- return this.restApiService
- .getAccountCapabilities(['createProject'])
- .then(capabilities => {
- if (capabilities?.createProject) {
- this._createNewCapability = true;
- }
- });
- });
- }
+ private async getCreateRepoCapability() {
+ const account = await this.restApiService.getAccount();
- _getRepos(filter: string, reposPerPage: number, offset?: number) {
- this._repos = [];
- return this.restApiService
- .getRepos(filter, reposPerPage, offset)
- .then(repos => {
- // Late response.
- if (filter !== this._filter || !repos) {
- return;
- }
- this._repos = repos.filter(repo =>
- repo.name.toLowerCase().includes(filter.toLowerCase())
- );
- this._loading = false;
- });
- }
+ if (!account) return;
- _refreshReposList() {
- this.restApiService.invalidateReposCache();
- return this._getRepos(this._filter, this._reposPerPage, this._offset);
- }
-
- _handleCreateRepo() {
- this.$.createNewModal.handleCreateRepo().then(() => {
- this._refreshReposList();
- });
- }
-
- _handleCloseCreate() {
- this.$.createOverlay.close();
- }
-
- _handleCreateClicked() {
- this.$.createOverlay.open().then(() => {
- this.$.createNewModal.focus();
- });
- }
-
- _readOnly(repo: ProjectInfoWithName) {
- return repo.state === ProjectState.READ_ONLY ? 'Y' : '';
- }
-
- _computeWeblink(repo: ProjectInfoWithName) {
- if (!repo.web_links) {
- return '';
+ const accountCapabilities =
+ await this.restApiService.getAccountCapabilities(['createProject']);
+ if (accountCapabilities?.createProject) {
+ this.createNewCapability = true;
}
- const webLinks = repo.web_links;
- return webLinks.length ? webLinks : null;
+
+ return account;
}
+ /* private but used in test */
+ async getRepos() {
+ this.repos = [];
+
+ // We save the filter before getting the repos
+ // and then we check the value hasn't changed aftwards.
+ const filter = this.filter;
+
+ const repos = await this.restApiService.getRepos(
+ this.filter,
+ this.reposPerPage,
+ this.offset
+ );
+
+ // Late response.
+ if (filter !== this.filter || !repos) return;
+
+ this.repos = repos.filter(repo =>
+ repo.name.toLowerCase().includes(filter.toLowerCase())
+ );
+ this.loading = false;
+
+ return repos;
+ }
+
+ private async refreshReposList() {
+ this.restApiService.invalidateReposCache();
+ return await this.getRepos();
+ }
+
+ /* private but used in test */
+ async handleCreateRepo() {
+ await this.createNewModal?.handleCreateRepo();
+ await this.refreshReposList();
+ }
+
+ /* private but used in test */
+ handleCloseCreate() {
+ this.createOverlay?.close();
+ }
+
+ /* private but used in test */
+ handleCreateClicked() {
+ this.createOverlay?.open().then(() => {
+ this.createNewModal?.focus();
+ });
+ }
+
+ /* private but used in test */
computeLoadingClass(loading: boolean) {
return loading ? 'loading' : '';
}
+
+ private handleNewRepoName() {
+ if (!this.createNewModal) return;
+ this.newRepoName = this.createNewModal.nameChanged;
+ }
}
diff --git a/polygerrit-ui/app/elements/admin/gr-repo-list/gr-repo-list_html.ts b/polygerrit-ui/app/elements/admin/gr-repo-list/gr-repo-list_html.ts
deleted file mode 100644
index e1a7f489..0000000
--- a/polygerrit-ui/app/elements/admin/gr-repo-list/gr-repo-list_html.ts
+++ /dev/null
@@ -1,116 +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">
- /* Workaround for empty style block - see https://github.com/Polymer/tools/issues/408 */
- </style>
- <style include="gr-table-styles">
- /* Workaround for empty style block - see https://github.com/Polymer/tools/issues/408 */
- </style>
- <style>
- .genericList tr td:last-of-type {
- text-align: left;
- }
- .genericList tr th:last-of-type {
- text-align: left;
- }
- .readOnly {
- text-align: center;
- }
- .changesLink,
- .name,
- .repositoryBrowser,
- .readOnly {
- white-space: nowrap;
- }
- </style>
- <gr-list-view
- create-new="[[_createNewCapability]]"
- filter="[[_filter]]"
- items-per-page="[[_reposPerPage]]"
- items="[[_repos]]"
- loading="[[_loading]]"
- offset="[[_offset]]"
- on-create-clicked="_handleCreateClicked"
- path="[[_path]]"
- >
- <table id="list" class="genericList">
- <tbody>
- <tr class="headerRow">
- <th class="name topHeader">Repository Name</th>
- <th class="repositoryBrowser topHeader">Repository Browser</th>
- <th class="changesLink topHeader">Changes</th>
- <th class="topHeader readOnly">Read only</th>
- <th class="description topHeader">Repository Description</th>
- </tr>
- <tr id="loading" class$="loadingMsg [[computeLoadingClass(_loading)]]">
- <td>Loading...</td>
- </tr>
- </tbody>
- <tbody class$="[[computeLoadingClass(_loading)]]">
- <template is="dom-repeat" items="[[_shownRepos]]">
- <tr class="table">
- <td class="name">
- <a href$="[[_computeRepoUrl(item.name)]]">[[item.name]]</a>
- </td>
- <td class="repositoryBrowser">
- <template
- is="dom-repeat"
- items="[[_computeWeblink(item)]]"
- as="link"
- >
- <a
- href$="[[link.url]]"
- class="webLink"
- rel="noopener"
- target="_blank"
- >
- [[link.name]]
- </a>
- </template>
- </td>
- <td class="changesLink">
- <a href$="[[_computeChangesLink(item.name)]]">view all</a>
- </td>
- <td class="readOnly">[[_readOnly(item)]]</td>
- <td class="description">[[item.description]]</td>
- </tr>
- </template>
- </tbody>
- </table>
- </gr-list-view>
- <gr-overlay id="createOverlay" with-backdrop="">
- <gr-dialog
- id="createDialog"
- class="confirmDialog"
- disabled="[[!_hasNewRepoName]]"
- confirm-label="Create"
- on-confirm="_handleCreateRepo"
- on-cancel="_handleCloseCreate"
- >
- <div class="header" slot="header">Create Repository</div>
- <div class="main" slot="main">
- <gr-create-repo-dialog
- has-new-repo-name="{{_hasNewRepoName}}"
- id="createNewModal"
- ></gr-create-repo-dialog>
- </div>
- </gr-dialog>
- </gr-overlay>
-`;
diff --git a/polygerrit-ui/app/elements/admin/gr-repo-list/gr-repo-list_test.js b/polygerrit-ui/app/elements/admin/gr-repo-list/gr-repo-list_test.js
deleted file mode 100644
index 8fef4d0..0000000
--- a/polygerrit-ui/app/elements/admin/gr-repo-list/gr-repo-list_test.js
+++ /dev/null
@@ -1,189 +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-repo-list.js';
-import {page} from '../../../utils/page-wrapper-utils.js';
-import 'lodash/lodash.js';
-import {stubRestApi} from '../../../test/test-utils.js';
-
-const basicFixture = fixtureFromElement('gr-repo-list');
-
-function createRepo(name, counter) {
- return {
- id: `${name}${counter}`,
- name: `${name}`,
- state: 'ACTIVE',
- web_links: [
- {
- name: 'diffusion',
- url: `https://phabricator.example.org/r/project/${name}${counter}`,
- },
- ],
- };
-}
-
-let counter;
-const repoGenerator = () => createRepo('test', ++counter);
-
-suite('gr-repo-list tests', () => {
- let element;
- let repos;
-
- let value;
-
- setup(() => {
- sinon.stub(page, 'show');
- element = basicFixture.instantiate();
- counter = 0;
- });
-
- suite('list with repos', () => {
- setup(async () => {
- repos = _.times(26, repoGenerator);
- stubRestApi('getRepos').returns(Promise.resolve(repos));
- await element._paramsChanged(value);
- await flush();
- });
-
- test('test for test repo in the list', async () => {
- await flush();
- assert.equal(element._repos[1].id, 'test2');
- });
-
- test('_shownRepos', () => {
- assert.equal(element._shownRepos.length, 25);
- });
-
- test('_maybeOpenCreateOverlay', () => {
- const overlayOpen = sinon.stub(element.$.createOverlay, 'open');
- element._maybeOpenCreateOverlay();
- assert.isFalse(overlayOpen.called);
- const params = {};
- element._maybeOpenCreateOverlay(params);
- assert.isFalse(overlayOpen.called);
- params.openCreateModal = true;
- element._maybeOpenCreateOverlay(params);
- assert.isTrue(overlayOpen.called);
- });
- });
-
- suite('list with less then 25 repos', () => {
- setup(async () => {
- repos = _.times(25, repoGenerator);
- stubRestApi('getRepos').returns(Promise.resolve(repos));
- await element._paramsChanged(value);
- await flush();
- });
-
- test('_shownRepos', () => {
- assert.equal(element._shownRepos.length, 25);
- });
- });
-
- suite('filter', () => {
- let reposFiltered;
- setup(() => {
- repos = _.times(25, repoGenerator);
- reposFiltered = _.times(1, repoGenerator);
- });
-
- test('_paramsChanged', async () => {
- const repoStub = stubRestApi('getRepos');
- repoStub.returns(Promise.resolve(repos));
- const value = {
- filter: 'test',
- offset: 25,
- };
- await element._paramsChanged(value);
- assert.isTrue(repoStub.lastCall.calledWithExactly('test', 25, 25));
- });
-
- test('latest repos requested are always set', async () => {
- const repoStub = stubRestApi('getRepos');
- repoStub.withArgs('test').returns(Promise.resolve(repos));
- repoStub.withArgs('filter').returns(Promise.resolve(reposFiltered));
- element._filter = 'test';
-
- // Repos are not set because the element._filter differs.
- await element._getRepos('filter', 25, 0);
- assert.deepEqual(element._repos, []);
- });
-
- test('filter is case insensitive', async () => {
- const repoStub = stubRestApi('getRepos');
- const repos = [createRepo('aSDf', 0)];
- repoStub.withArgs('asdf').returns(Promise.resolve(repos));
- element._filter = 'asdf';
- await element._getRepos('asdf', 25, 0);
- assert.equal(element._repos.length, 1);
- });
- });
-
- suite('loading', () => {
- test('correct contents are displayed', () => {
- assert.isTrue(element._loading);
- assert.equal(element.computeLoadingClass(element._loading), 'loading');
- assert.equal(getComputedStyle(element.$.loading).display, 'block');
-
- element._loading = false;
- element._repos = _.times(25, repoGenerator);
-
- flush();
- assert.equal(element.computeLoadingClass(element._loading), '');
- assert.equal(getComputedStyle(element.$.loading).display, 'none');
- });
- });
-
- suite('create new', () => {
- test('_handleCreateClicked called when create-click fired', () => {
- sinon.stub(element, '_handleCreateClicked');
- element.shadowRoot
- .querySelector('gr-list-view').dispatchEvent(
- new CustomEvent('create-clicked', {
- composed: true, bubbles: true,
- }));
- assert.isTrue(element._handleCreateClicked.called);
- });
-
- test('_handleCreateClicked opens modal', () => {
- const openStub = sinon.stub(element.$.createOverlay, 'open').returns(
- Promise.resolve());
- element._handleCreateClicked();
- assert.isTrue(openStub.called);
- });
-
- test('_handleCreateRepo called when confirm fired', () => {
- sinon.stub(element, '_handleCreateRepo');
- element.$.createDialog.dispatchEvent(
- new CustomEvent('confirm', {
- composed: true, bubbles: true,
- }));
- assert.isTrue(element._handleCreateRepo.called);
- });
-
- test('_handleCloseCreate called when cancel fired', () => {
- sinon.stub(element, '_handleCloseCreate');
- element.$.createDialog.dispatchEvent(
- new CustomEvent('cancel', {
- composed: true, bubbles: true,
- }));
- assert.isTrue(element._handleCloseCreate.called);
- });
- });
-});
-
diff --git a/polygerrit-ui/app/elements/admin/gr-repo-list/gr-repo-list_test.ts b/polygerrit-ui/app/elements/admin/gr-repo-list/gr-repo-list_test.ts
new file mode 100644
index 0000000..142a838
--- /dev/null
+++ b/polygerrit-ui/app/elements/admin/gr-repo-list/gr-repo-list_test.ts
@@ -0,0 +1,246 @@
+/**
+ * @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-repo-list';
+import {GrRepoList} from './gr-repo-list';
+import {page} from '../../../utils/page-wrapper-utils';
+import {
+ mockPromise,
+ queryAndAssert,
+ stubRestApi,
+} from '../../../test/test-utils';
+import {
+ UrlEncodedRepoName,
+ ProjectInfoWithName,
+ RepoName,
+} from '../../../types/common';
+import {AppElementAdminParams} from '../../gr-app-types';
+import {ProjectState, SHOWN_ITEMS_COUNT} from '../../../constants/constants';
+import {GerritView} from '../../../services/router/router-model';
+import {GrOverlay} from '../../shared/gr-overlay/gr-overlay';
+import {GrDialog} from '../../shared/gr-dialog/gr-dialog';
+import {GrListView} from '../../shared/gr-list-view/gr-list-view';
+
+const basicFixture = fixtureFromElement('gr-repo-list');
+
+function createRepo(name: string, counter: number) {
+ return {
+ id: `${name}${counter}` as UrlEncodedRepoName,
+ name: `${name}` as RepoName,
+ state: 'ACTIVE' as ProjectState,
+ web_links: [
+ {
+ name: 'diffusion',
+ url: `https://phabricator.example.org/r/project/${name}${counter}`,
+ },
+ ],
+ };
+}
+
+function createRepoList(name: string, n: number) {
+ const repos = [];
+ for (let i = 0; i < n; ++i) {
+ repos.push(createRepo(name, i));
+ }
+ return repos;
+}
+
+suite('gr-repo-list tests', () => {
+ let element: GrRepoList;
+ let repos: ProjectInfoWithName[];
+
+ setup(async () => {
+ sinon.stub(page, 'show');
+ element = basicFixture.instantiate();
+ await element.updateComplete;
+ });
+
+ suite('list with repos', () => {
+ setup(async () => {
+ repos = createRepoList('test', 26);
+ stubRestApi('getRepos').returns(Promise.resolve(repos));
+ await element._paramsChanged();
+ await element.updateComplete;
+ });
+
+ test('test for test repo in the list', async () => {
+ await element.updateComplete;
+ assert.equal(element.repos[0].id, 'test0');
+ assert.equal(element.repos[1].id, 'test1');
+ assert.equal(element.repos[2].id, 'test2');
+ });
+
+ test('shownRepos', () => {
+ assert.equal(element.repos.slice(0, SHOWN_ITEMS_COUNT).length, 25);
+ });
+
+ test('maybeOpenCreateOverlay', () => {
+ const overlayOpen = sinon.stub(
+ queryAndAssert<GrOverlay>(element, '#createOverlay'),
+ 'open'
+ );
+ element.maybeOpenCreateOverlay();
+ assert.isFalse(overlayOpen.called);
+ element.maybeOpenCreateOverlay(undefined);
+ assert.isFalse(overlayOpen.called);
+ const params: AppElementAdminParams = {
+ view: GerritView.ADMIN,
+ adminView: '',
+ openCreateModal: true,
+ };
+ element.maybeOpenCreateOverlay(params);
+ assert.isTrue(overlayOpen.called);
+ });
+ });
+
+ suite('list with less then 25 repos', () => {
+ setup(async () => {
+ repos = createRepoList('test', 25);
+ stubRestApi('getRepos').returns(Promise.resolve(repos));
+ await element._paramsChanged();
+ await element.updateComplete;
+ });
+
+ test('shownRepos', () => {
+ assert.equal(element.repos.slice(0, SHOWN_ITEMS_COUNT).length, 25);
+ });
+ });
+
+ suite('filter', () => {
+ let reposFiltered: ProjectInfoWithName[];
+
+ setup(() => {
+ repos = createRepoList('test', 25);
+ reposFiltered = createRepoList('filter', 1);
+ });
+
+ test('_paramsChanged', async () => {
+ const repoStub = stubRestApi('getRepos');
+ repoStub.returns(Promise.resolve(repos));
+ element.params = {
+ view: GerritView.ADMIN,
+ adminView: '',
+ filter: 'test',
+ offset: 25,
+ } as AppElementAdminParams;
+ await element._paramsChanged();
+ assert.isTrue(repoStub.lastCall.calledWithExactly('test', 25, 25));
+ });
+
+ test('latest repos requested are always set', async () => {
+ const repoStub = stubRestApi('getRepos');
+ const promise = mockPromise<ProjectInfoWithName[]>();
+ repoStub.withArgs('filter', 25).returns(promise);
+
+ element.filter = 'test';
+ element.reposPerPage = 25;
+ element.offset = 0;
+
+ // Repos are not set because the element.filter differs.
+ const p = element.getRepos();
+ element.filter = 'filter';
+ promise.resolve(reposFiltered);
+ await p;
+ assert.deepEqual(element.repos, []);
+ });
+
+ test('filter is case insensitive', async () => {
+ const repoStub = stubRestApi('getRepos');
+ const repos = [createRepo('aSDf', 0)];
+ repoStub.withArgs('asdf', 25).returns(Promise.resolve(repos));
+
+ element.filter = 'asdf';
+ element.reposPerPage = 25;
+ element.offset = 0;
+
+ await element.getRepos();
+ assert.equal(element.repos.length, 1);
+ });
+ });
+
+ suite('loading', () => {
+ test('correct contents are displayed', async () => {
+ assert.isTrue(element.loading);
+ assert.equal(element.computeLoadingClass(element.loading), 'loading');
+ assert.equal(
+ getComputedStyle(
+ queryAndAssert<HTMLTableRowElement>(element, '#loading')
+ ).display,
+ 'block'
+ );
+
+ element.loading = false;
+ element.repos = createRepoList('test', 25);
+
+ await element.updateComplete;
+ assert.equal(element.computeLoadingClass(element.loading), '');
+ assert.equal(
+ getComputedStyle(
+ queryAndAssert<HTMLTableRowElement>(element, '#loading')
+ ).display,
+ 'none'
+ );
+ });
+ });
+
+ suite('create new', () => {
+ test('handleCreateClicked called when create-clicked fired', () => {
+ const handleCreateClickedStub = sinon.stub();
+ element.addEventListener('create-clicked', handleCreateClickedStub);
+ queryAndAssert<GrListView>(element, 'gr-list-view').dispatchEvent(
+ new CustomEvent('create-clicked', {
+ composed: true,
+ bubbles: true,
+ })
+ );
+ assert.isTrue(handleCreateClickedStub.called);
+ });
+
+ test('handleCreateClicked opens modal', () => {
+ const openStub = sinon
+ .stub(queryAndAssert<GrOverlay>(element, '#createOverlay'), 'open')
+ .returns(Promise.resolve());
+ element.handleCreateClicked();
+ assert.isTrue(openStub.called);
+ });
+
+ test('handleCreateRepo called when confirm fired', () => {
+ const handleCreateRepoStub = sinon.stub();
+ element.addEventListener('confirm', handleCreateRepoStub);
+ queryAndAssert<GrDialog>(element, '#createDialog').dispatchEvent(
+ new CustomEvent('confirm', {
+ composed: true,
+ bubbles: false,
+ })
+ );
+ assert.isTrue(handleCreateRepoStub.called);
+ });
+
+ test('handleCloseCreate called when cancel fired', () => {
+ const handleCloseCreateStub = sinon.stub();
+ element.addEventListener('cancel', handleCloseCreateStub);
+ queryAndAssert<GrDialog>(element, '#createDialog').dispatchEvent(
+ new CustomEvent('cancel', {
+ composed: true,
+ bubbles: false,
+ })
+ );
+ assert.isTrue(handleCloseCreateStub.called);
+ });
+ });
+});
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-change-actions/gr-change-actions.ts b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts
index b10c17e..536ef92 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts
@@ -99,7 +99,6 @@
getVotingRange,
StandardLabels,
} from '../../../utils/label-util';
-import {CommentThread} from '../../../utils/comment-util';
import {ShowAlertEventDetail} from '../../../types/events';
import {
ActionPriority,
@@ -448,9 +447,6 @@
@property({type: String})
_actionLoadingMessage = '';
- @property({type: Array})
- commentThreads: CommentThread[] = [];
-
@property({
type: Array,
computed:
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_html.ts b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_html.ts
index d21c29f..17ca7cf 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_html.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_html.ts
@@ -213,11 +213,9 @@
<gr-confirm-submit-dialog
id="confirmSubmitDialog"
class="confirmDialog"
- change="[[change]]"
action="[[_revisionSubmitAction]]"
on-cancel="_handleConfirmDialogCancel"
on-confirm="_handleSubmitConfirm"
- comment-threads="[[commentThreads]]"
hidden=""
></gr-confirm-submit-dialog>
<gr-dialog
diff --git a/polygerrit-ui/app/elements/change/gr-change-summary/gr-change-summary.ts b/polygerrit-ui/app/elements/change/gr-change-summary/gr-change-summary.ts
index e4b466b..c4ccb7d 100644
--- a/polygerrit-ui/app/elements/change/gr-change-summary/gr-change-summary.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-summary/gr-change-summary.ts
@@ -15,7 +15,7 @@
* limitations under the License.
*/
import {LitElement, css, html} from 'lit';
-import {customElement, property} from 'lit/decorators';
+import {customElement, property, state} from 'lit/decorators';
import {subscribe} from '../../lit/subscription-controller';
import {sharedStyles} from '../../../styles/shared-styles';
import {appContext} from '../../../services/app-context';
@@ -65,6 +65,11 @@
import {modifierPressed} from '../../../utils/dom-util';
import {DropdownLink} from '../../shared/gr-dropdown/gr-dropdown';
import {fontStyles} from '../../../styles/gr-font-styles';
+import {account$} from '../../../services/user/user-model';
+import {
+ changeComments$,
+ threads$,
+} from '../../../services/comments/comments-model';
export enum SummaryChipStyles {
INFO = 'info',
@@ -378,31 +383,31 @@
@customElement('gr-change-summary')
export class GrChangeSummary extends LitElement {
- @property({type: Object})
+ @state()
changeComments?: ChangeComments;
- @property({type: Array})
+ @state()
commentThreads?: CommentThread[];
- @property({type: Object})
+ @state()
selfAccount?: AccountInfo;
- @property()
+ @state()
runs: CheckRun[] = [];
- @property()
+ @state()
showChecksSummary = false;
- @property()
+ @state()
someProvidersAreLoading = false;
- @property()
+ @state()
errorMessages: ErrorMessages = {};
- @property()
+ @state()
loginCallback?: () => void;
- @property()
+ @state()
actions: Action[] = [];
private showAllChips = new Map<RunStatus | Category, boolean>();
@@ -421,6 +426,9 @@
subscribe(this, errorMessagesLatest$, x => (this.errorMessages = x));
subscribe(this, loginCallbackLatest$, x => (this.loginCallback = x));
subscribe(this, topLevelActionsLatest$, x => (this.actions = x));
+ subscribe(this, changeComments$, x => (this.changeComments = x));
+ subscribe(this, threads$, x => (this.commentThreads = x));
+ subscribe(this, account$, x => (this.selfAccount = x));
}
static override get styles() {
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 8d64bc0..8a28cb1 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
@@ -18,7 +18,6 @@
import '../../../styles/gr-a11y-styles';
import '../../../styles/gr-paper-styles';
import '../../../styles/shared-styles';
-import '../../diff/gr-comment-api/gr-comment-api';
import '../../plugins/gr-endpoint-decorator/gr-endpoint-decorator';
import '../../plugins/gr-endpoint-param/gr-endpoint-param';
import '../../shared/gr-account-link/gr-account-link';
@@ -129,10 +128,7 @@
import {GrIncludedInDialog} from '../gr-included-in-dialog/gr-included-in-dialog';
import {GrDownloadDialog} from '../gr-download-dialog/gr-download-dialog';
import {GrChangeMetadata} from '../gr-change-metadata/gr-change-metadata';
-import {
- ChangeComments,
- GrCommentApi,
-} from '../../diff/gr-comment-api/gr-comment-api';
+import {ChangeComments} from '../../diff/gr-comment-api/gr-comment-api';
import {
assertIsDefined,
hasOwnProperty,
@@ -230,7 +226,6 @@
export interface GrChangeView {
$: {
- commentAPI: GrCommentApi;
applyFixDialog: GrApplyFixDialog;
fileList: GrFileList & Element;
fileListHeader: GrFileListHeader;
@@ -2026,23 +2021,6 @@
});
}
- /**
- * Fetches a new changeComment object, and data for all types of comments
- * (comments, robot comments, draft comments) is requested.
- */
- _reloadComments() {
- // We are resetting all comment related properties, because we want to avoid
- // a new change being loaded and then paired with outdated comments.
- this._changeComments = undefined;
- this._commentThreads = undefined;
- this._draftCommentThreads = undefined;
- this._robotCommentThreads = undefined;
- if (!this._changeNum)
- throw new Error('missing required changeNum property');
-
- this.commentsService.loadAll(this._changeNum, this._patchRange?.patchNum);
- }
-
@observe('_changeComments')
changeCommentsChanged(comments?: ChangeComments) {
if (!comments) return;
@@ -2124,8 +2102,6 @@
});
allDataPromises.push(projectConfigLoaded);
- this._reloadComments();
-
let coreDataPromise;
// If the patch number is specified
@@ -2230,13 +2206,20 @@
assertIsDefined(this._changeNum, '_changeNum');
if (!this._patchRange?.patchNum) throw new Error('missing patchNum');
const promises = [this._getCommitInfo(), this.$.fileList.reload()];
- if (patchNumChanged)
+ if (patchNumChanged) {
promises.push(
- this.$.commentAPI.reloadPortedComments(
+ this.commentsService.reloadPortedComments(
this._changeNum,
this._patchRange?.patchNum
)
);
+ promises.push(
+ this.commentsService.reloadPortedDrafts(
+ this._changeNum,
+ this._patchRange?.patchNum
+ )
+ );
+ }
return Promise.all(promises);
}
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 9341b18..9181ca0 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
@@ -381,7 +381,6 @@
on-stop-edit-tap="_handleStopEditTap"
on-download-tap="_handleOpenDownloadDialog"
on-included-tap="_handleOpenIncludedInDialog"
- comment-threads="[[_commentThreads]]"
></gr-change-actions>
</div>
<!-- end commit actions -->
@@ -439,12 +438,7 @@
</gr-editable-content>
</div>
<h3 class="assistive-tech-only">Comments and Checks Summary</h3>
- <gr-change-summary
- change-comments="[[_changeComments]]"
- comment-threads="[[_commentThreads]]"
- self-account="[[_account]]"
- >
- </gr-change-summary>
+ <gr-change-summary></gr-change-summary>
<gr-endpoint-decorator name="commit-container">
<gr-endpoint-param name="change" value="[[_change]]">
</gr-endpoint-param>
@@ -528,7 +522,6 @@
change="[[_change]]"
change-num="[[_changeNum]]"
revision-info="[[_revisionInfo]]"
- change-comments="[[_changeComments]]"
commit-info="[[_commitInfo]]"
change-url="[[_computeChangeUrl(_change)]]"
edit-mode="[[_editMode]]"
@@ -720,5 +713,4 @@
</gr-reply-dialog>
</template>
</gr-overlay>
- <gr-comment-api id="commentAPI"></gr-comment-api>
`;
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 ab17f47..72cd91e 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
@@ -40,6 +40,7 @@
import {
mockPromise,
queryAndAssert,
+ stubComments,
stubRestApi,
stubUsers,
waitQueryAndAssert,
@@ -1346,10 +1347,8 @@
sinon.stub(element, '_getCommitInfo');
sinon.stub(element.$.fileList, 'reload');
flush();
- const reloadPortedCommentsStub = sinon.stub(
- element.$.commentAPI,
- 'reloadPortedComments'
- );
+ const reloadPortedCommentsStub = stubComments('reloadPortedComments');
+ const reloadPortedDraftsStub = stubComments('reloadPortedDrafts');
sinon.stub(element.$.fileList, 'collapseAllDiffs');
const value: AppElementChangeViewParams = {
@@ -1374,6 +1373,7 @@
element.params = {...value};
await flush();
assert.isTrue(reloadPortedCommentsStub.calledOnce);
+ assert.isTrue(reloadPortedDraftsStub.calledOnce);
});
test('do not reload entire page when patchRange doesnt change', async () => {
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-submit-dialog/gr-confirm-submit-dialog.ts b/polygerrit-ui/app/elements/change/gr-confirm-submit-dialog/gr-confirm-submit-dialog.ts
index 9d371d3..a9b7b81 100644
--- a/polygerrit-ui/app/elements/change/gr-confirm-submit-dialog/gr-confirm-submit-dialog.ts
+++ b/polygerrit-ui/app/elements/change/gr-confirm-submit-dialog/gr-confirm-submit-dialog.ts
@@ -20,14 +20,18 @@
import '../../plugins/gr-endpoint-decorator/gr-endpoint-decorator';
import '../../plugins/gr-endpoint-param/gr-endpoint-param';
import '../gr-thread-list/gr-thread-list';
-import {ChangeInfo, ActionInfo} from '../../../types/common';
+import {ActionInfo} from '../../../types/common';
import {GrDialog} from '../../shared/gr-dialog/gr-dialog';
import {pluralize} from '../../../utils/string-util';
import {CommentThread, isUnresolved} from '../../../utils/comment-util';
import {sharedStyles} from '../../../styles/shared-styles';
import {LitElement, css, html} from 'lit';
-import {customElement, property, query} from 'lit/decorators';
+import {customElement, property, query, state} from 'lit/decorators';
import {fontStyles} from '../../../styles/gr-font-styles';
+import {subscribe} from '../../lit/subscription-controller';
+import {change$} from '../../../services/change/change-model';
+import {threads$} from '../../../services/comments/comments-model';
+import {ParsedChangeInfo} from '../../../types/types';
@customElement('gr-confirm-submit-dialog')
export class GrConfirmSubmitDialog extends LitElement {
@@ -47,16 +51,16 @@
*/
@property({type: Object})
- change?: ChangeInfo;
-
- @property({type: Object})
action?: ActionInfo;
- @property({type: Array})
- commentThreads?: CommentThread[] = [];
+ @state()
+ change?: ParsedChangeInfo;
- @property({type: Boolean})
- _initialised = false;
+ @state()
+ unresolvedThreads: CommentThread[] = [];
+
+ @state()
+ initialised = false;
static override get styles() {
return [
@@ -84,6 +88,16 @@
];
}
+ constructor() {
+ super();
+ subscribe(this, change$, x => (this.change = x));
+ subscribe(
+ this,
+ threads$,
+ x => (this.unresolvedThreads = x.filter(isUnresolved))
+ );
+ }
+
private renderPrivate() {
if (!this.change?.is_private) return '';
return html`
@@ -106,11 +120,11 @@
icon="gr-icons:warning"
class="warningBeforeSubmit"
></iron-icon>
- ${this._computeUnresolvedCommentsWarning(this.change)}
+ ${this.computeUnresolvedCommentsWarning()}
</p>
<gr-thread-list
id="commentList"
- .threads="${this._computeUnresolvedThreads(this.commentThreads)}"
+ .threads="${this.unresolvedThreads}"
.change="${this.change}"
.changeNum="${this.change?._number}"
logged-in
@@ -121,7 +135,7 @@
}
private renderChangeEdit() {
- if (!this._computeHasChangeEdit(this.change)) return '';
+ if (!this.computeHasChangeEdit()) return '';
return html`
<iron-icon
icon="gr-icons:warning"
@@ -133,7 +147,7 @@
}
private renderInitialised() {
- if (!this._initialised) return '';
+ if (!this.initialised) return '';
return html`
<div class="header" slot="header">${this.action?.label}</div>
<div class="main" slot="main">
@@ -159,48 +173,43 @@
id="dialog"
confirm-label="Continue"
confirm-on-enter=""
- @cancel=${this._handleCancelTap}
- @confirm=${this._handleConfirmTap}
+ @cancel=${this.handleCancelTap}
+ @confirm=${this.handleConfirmTap}
>
${this.renderInitialised()}
</gr-dialog>`;
}
init() {
- this._initialised = true;
+ this.initialised = true;
}
resetFocus() {
this.dialog?.resetFocus();
}
- _computeHasChangeEdit(change?: ChangeInfo) {
- return (
- !!change &&
- !!change.revisions &&
- Object.values(change.revisions).some(rev => rev._number === 'edit')
+ // Private method, but visible for testing.
+ computeHasChangeEdit() {
+ return Object.values(this.change?.revisions ?? {}).some(
+ rev => rev._number === 'edit'
);
}
- _computeUnresolvedThreads(commentThreads?: CommentThread[]) {
- if (!commentThreads) return [];
- return commentThreads.filter(thread => isUnresolved(thread));
- }
-
- _computeUnresolvedCommentsWarning(change?: ChangeInfo) {
- if (!change) return '';
- const unresolvedCount = change.unresolved_comment_count;
+ // Private method, but visible for testing.
+ computeUnresolvedCommentsWarning() {
+ if (!this.change) return '';
+ const unresolvedCount = this.change.unresolved_comment_count;
if (!unresolvedCount) throw new Error('unresolved comments undefined or 0');
return `Heads Up! ${pluralize(unresolvedCount, 'unresolved comment')}.`;
}
- _handleConfirmTap(e: Event) {
+ private handleConfirmTap(e: Event) {
e.preventDefault();
e.stopPropagation();
this.dispatchEvent(new CustomEvent('confirm', {bubbles: false}));
}
- _handleCancelTap(e: Event) {
+ private handleCancelTap(e: Event) {
e.preventDefault();
e.stopPropagation();
this.dispatchEvent(new CustomEvent('cancel', {bubbles: false}));
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-submit-dialog/gr-confirm-submit-dialog_test.ts b/polygerrit-ui/app/elements/change/gr-confirm-submit-dialog/gr-confirm-submit-dialog_test.ts
index e1823b1..0426cb6 100644
--- a/polygerrit-ui/app/elements/change/gr-confirm-submit-dialog/gr-confirm-submit-dialog_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-confirm-submit-dialog/gr-confirm-submit-dialog_test.ts
@@ -16,7 +16,10 @@
*/
import '../../../test/common-test-setup-karma';
-import {createChange, createRevision} from '../../../test/test-data-generators';
+import {
+ createParsedChange,
+ createRevision,
+} from '../../../test/test-data-generators';
import {queryAndAssert} from '../../../test/test-utils';
import {PatchSetNum} from '../../../types/common';
import {GrConfirmSubmitDialog} from './gr-confirm-submit-dialog';
@@ -28,13 +31,13 @@
setup(() => {
element = basicFixture.instantiate();
- element._initialised = true;
+ element.initialised = true;
});
test('display', async () => {
element.action = {label: 'my-label'};
element.change = {
- ...createChange(),
+ ...createParsedChange(),
subject: 'my-subject',
revisions: {},
};
@@ -47,23 +50,23 @@
assert.notEqual(message.textContent!.indexOf('my-subject'), -1);
});
- test('_computeUnresolvedCommentsWarning', () => {
- const change = {...createChange(), unresolved_comment_count: 1};
+ test('computeUnresolvedCommentsWarning', () => {
+ element.change = {...createParsedChange(), unresolved_comment_count: 1};
assert.equal(
- element._computeUnresolvedCommentsWarning(change),
+ element.computeUnresolvedCommentsWarning(),
'Heads Up! 1 unresolved comment.'
);
- const change2 = {...createChange(), unresolved_comment_count: 2};
+ element.change = {...createParsedChange(), unresolved_comment_count: 2};
assert.equal(
- element._computeUnresolvedCommentsWarning(change2),
+ element.computeUnresolvedCommentsWarning(),
'Heads Up! 2 unresolved comments.'
);
});
- test('_computeHasChangeEdit', () => {
- const change = {
- ...createChange(),
+ test('computeHasChangeEdit', () => {
+ element.change = {
+ ...createParsedChange(),
revisions: {
d442ff05d6c4f2a3af0eeca1f67374b39f9dc3d8: {
...createRevision(),
@@ -73,10 +76,10 @@
unresolved_comment_count: 0,
};
- assert.isTrue(element._computeHasChangeEdit(change));
+ assert.isTrue(element.computeHasChangeEdit());
- const change2 = {
- ...createChange(),
+ element.change = {
+ ...createParsedChange(),
revisions: {
d442ff05d6c4f2a3af0eeca1f67374b39f9dc3d8: {
...createRevision(),
@@ -84,6 +87,6 @@
},
},
};
- assert.isFalse(element._computeHasChangeEdit(change2));
+ assert.isFalse(element.computeHasChangeEdit());
});
});
diff --git a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.ts b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.ts
index 1b44e35..920844b 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.ts
+++ b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.ts
@@ -39,7 +39,6 @@
BasePatchSetNum,
} from '../../../types/common';
import {DiffPreferencesInfo} from '../../../types/diff';
-import {ChangeComments} from '../../diff/gr-comment-api/gr-comment-api';
import {GrDiffModeSelector} from '../../diff/gr-diff-mode-selector/gr-diff-mode-selector';
import {GrButton} from '../../shared/gr-button/gr-button';
import {fireEvent} from '../../../utils/event-util';
@@ -101,9 +100,6 @@
changeUrl?: string;
@property({type: Object})
- changeComments?: ChangeComments;
-
- @property({type: Object})
commitInfo?: CommitInfo;
@property({type: Boolean})
diff --git a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header_html.ts b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header_html.ts
index 5a85531..fbba2fc 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header_html.ts
+++ b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header_html.ts
@@ -132,7 +132,6 @@
<div class="patchInfoContent">
<gr-patch-range-select
id="rangeSelect"
- change-comments="[[changeComments]]"
change-num="[[changeNum]]"
patch-num="[[patchNum]]"
base-patch-num="[[basePatchNum]]"
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
index 95984b8..d02f09b 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
@@ -18,7 +18,6 @@
import '../../../styles/shared-styles';
import '../../diff/gr-diff-cursor/gr-diff-cursor';
import '../../diff/gr-diff-host/gr-diff-host';
-import '../../diff/gr-comment-api/gr-comment-api';
import '../../diff/gr-diff-preferences-dialog/gr-diff-preferences-dialog';
import '../../edit/gr-edit-file-controls/gr-edit-file-controls';
import '../../shared/gr-button/gr-button';
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.js b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.js
index 0db7690..ee65837 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.js
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.js
@@ -16,7 +16,6 @@
*/
import '../../../test/common-test-setup-karma.js';
-import '../../diff/gr-comment-api/gr-comment-api.js';
import '../../shared/gr-date-formatter/gr-date-formatter.js';
import {getMockDiffResponse} from '../../../test/mocks/diff-response.js';
import './gr-file-list.js';
@@ -49,7 +48,6 @@
'gr-file-list-comment-api-mock', html`
<gr-file-list id="fileList"
change-comments="[[_changeComments]]"></gr-file-list>
- <gr-comment-api id="commentAPI"></gr-comment-api>
`);
const basicFixture = fixtureFromElement(commentApiMock.is);
diff --git a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list_test.js b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list_test.js
index 0939daa..a3b8873 100644
--- a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list_test.js
+++ b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list_test.js
@@ -16,7 +16,6 @@
*/
import '../../../test/common-test-setup-karma.js';
-import '../../diff/gr-comment-api/gr-comment-api.js';
import './gr-messages-list.js';
import {createCommentApiMockWithTemplateElement} from '../../../test/mocks/comment-api.js';
import {TEST_ONLY} from './gr-messages-list.js';
@@ -30,7 +29,6 @@
<gr-messages-list
id="messagesList"
change-comments="[[_changeComments]]"></gr-messages-list>
- <gr-comment-api id="commentAPI"></gr-comment-api>
`);
const basicFixture = fixtureFromTemplate(html`
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..fcc853d 100644
--- a/polygerrit-ui/app/elements/core/gr-navigation/gr-navigation.ts
+++ b/polygerrit-ui/app/elements/core/gr-navigation/gr-navigation.ts
@@ -316,11 +316,17 @@
commentLink?: boolean;
}
+export interface GenerateUrlTopicViewParams {
+ view: GerritView.TOPIC;
+ topic?: string;
+}
+
export type GenerateUrlParameters =
| GenerateUrlSearchViewParameters
| GenerateUrlChangeViewParameters
| GenerateUrlRepoViewParameters
| GenerateUrlDashboardViewParameters
+ | GenerateUrlTopicViewParams
| GenerateUrlGroupViewParameters
| GenerateUrlEditViewParameters
| GenerateUrlRootViewParameters
@@ -675,6 +681,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-comment-api/gr-comment-api.ts b/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api.ts
index 7e7e507..32c732e 100644
--- a/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api.ts
+++ b/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api.ts
@@ -14,16 +14,12 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-import {PolymerElement} from '@polymer/polymer/polymer-element';
-import {htmlTemplate} from './gr-comment-api_html';
-import {customElement, property} from '@polymer/decorators';
import {
CommentBasics,
PatchRange,
PatchSetNum,
RobotCommentInfo,
UrlEncodedCommentId,
- NumericChangeId,
PathToCommentsInfoMap,
FileInfo,
ParentPatchSetNum,
@@ -45,7 +41,6 @@
addPath,
} from '../../../utils/comment-util';
import {PatchSetFile, PatchNumOnly, isPatchSetFile} from '../../../types/types';
-import {appContext} from '../../../services/app-context';
import {CommentSide, Side} from '../../../constants/constants';
import {pluralize} from '../../../utils/string-util';
import {NormalizedFileInfo} from '../../change/gr-file-list/gr-file-list';
@@ -611,38 +606,3 @@
return createCommentThreads(comments);
}
}
-
-@customElement('gr-comment-api')
-export class GrCommentApi extends PolymerElement {
- static get template() {
- return htmlTemplate;
- }
-
- @property({type: Object})
- _changeComments?: ChangeComments;
-
- private readonly restApiService = appContext.restApiService;
-
- private readonly commentsService = appContext.commentsService;
-
- reloadPortedComments(changeNum: NumericChangeId, patchNum: PatchSetNum) {
- if (!this._changeComments) {
- this.commentsService.loadAll(changeNum);
- return Promise.resolve();
- }
- return Promise.all([
- this.restApiService.getPortedComments(changeNum, patchNum),
- this.restApiService.getPortedDrafts(changeNum, patchNum),
- ]).then(res => {
- if (!this._changeComments) return;
- this._changeComments =
- this._changeComments.cloneWithUpdatedPortedComments(res[0], res[1]);
- });
- }
-}
-
-declare global {
- interface HTMLElementTagNameMap {
- 'gr-comment-api': GrCommentApi;
- }
-}
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight.ts b/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight.ts
index 3e292fe..a47db20 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight.ts
@@ -408,6 +408,7 @@
range: Text | Element | Range
) {
if (startLine > 1) {
+ actionBox.positionBelow = false;
actionBox.placeAbove(range);
return;
}
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.js b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.js
index 6facdca..8d75230 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.js
@@ -20,7 +20,6 @@
import {dom} from '@polymer/polymer/lib/legacy/polymer.dom.js';
import {createDefaultDiffPrefs, Side} from '../../../constants/constants.js';
-import {_testOnly_resetState} from '../../../services/comments/comments-model.js';
import {createChange, createComment, createCommentThread} from '../../../test/test-data-generators.js';
import {addListenerForTest, mockPromise, stubRestApi} from '../../../test/test-utils.js';
import {EditPatchSetNum, ParentPatchSetNum} from '../../../types/common.js';
@@ -43,7 +42,6 @@
element.path = 'some/path';
sinon.stub(element.reporting, 'time');
sinon.stub(element.reporting, 'timeEnd');
- _testOnly_resetState();
await flush();
});
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
index 0ffe61f..64186f4 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
@@ -65,7 +65,7 @@
GrDropdownList,
} from '../../shared/gr-dropdown-list/gr-dropdown-list';
import {GrOverlay} from '../../shared/gr-overlay/gr-overlay';
-import {ChangeComments, GrCommentApi} from '../gr-comment-api/gr-comment-api';
+import {ChangeComments} from '../gr-comment-api/gr-comment-api';
import {GrDiffModeSelector} from '../gr-diff-mode-selector/gr-diff-mode-selector';
import {
BasePatchSetNum,
@@ -134,7 +134,6 @@
export interface GrDiffView {
$: {
- commentAPI: GrCommentApi;
diffHost: GrDiffHost;
reviewed: HTMLInputElement;
dropdown: GrDropdownList;
@@ -348,8 +347,6 @@
private readonly userService = appContext.userService;
- private readonly commentsService = appContext.commentsService;
-
private readonly shortcuts = appContext.shortcutsService;
_throttledToggleFileReviewed?: (e: KeyboardEvent) => void;
@@ -1074,8 +1071,6 @@
if (!this._change) promises.push(this._getChangeDetail(this._changeNum));
- if (!this._changeComments) this._loadComments(value.patchNum);
-
promises.push(this._getChangeEdit());
this.$.diffHost.cancel();
@@ -1464,11 +1459,6 @@
return url;
}
- _loadComments(patchSet?: PatchSetNum) {
- assertIsDefined(this._changeNum, '_changeNum');
- return this.commentsService.loadAll(this._changeNum, patchSet);
- }
-
@observe(
'_changeComments',
'_files.changeFilesByPath',
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_html.ts b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_html.ts
index 16adb45..d87d192 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_html.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_html.ts
@@ -426,5 +426,4 @@
on-reload-diff-preference="_handleReloadingDiffPreference"
>
</gr-diff-preferences-dialog>
- <gr-comment-api id="commentAPI"></gr-comment-api>
`;
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js
index 46ec5d0..5c04ab0 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js
@@ -30,7 +30,7 @@
import {EditPatchSetNum} from '../../../types/common.js';
import {CursorMoveResult} from '../../../api/core.js';
import {EventType} from '../../../types/events.js';
-import {_testOnly_resetState, _testOnly_setState} from '../../../services/browser/browser-model.js';
+import {_testOnly_setState} from '../../../services/browser/browser-model.js';
const basicFixture = fixtureFromElement('gr-diff-view');
@@ -69,8 +69,6 @@
stubRestApi('getDiffDrafts').returns(Promise.resolve({}));
stubRestApi('getPortedComments').returns(Promise.resolve({}));
- _testOnly_resetState();
-
element = basicFixture.instantiate();
element._changeNum = '42';
element._path = 'some/path.txt';
@@ -2040,7 +2038,6 @@
Promise.resolve([]));
element = basicFixture.instantiate();
element._changeNum = '42';
- return element._loadComments();
});
test('_getFiles add files with comments without changes', () => {
diff --git a/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select.ts b/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select.ts
index 501f688..5d4405f 100644
--- a/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select.ts
+++ b/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select.ts
@@ -50,10 +50,16 @@
import {sharedStyles} from '../../../styles/shared-styles';
import {LitElement, PropertyValues, css, html} from 'lit';
import {customElement, property, query, state} from 'lit/decorators';
+import {subscribe} from '../../lit/subscription-controller';
+import {changeComments$} from '../../../services/comments/comments-model';
// Maximum length for patch set descriptions.
const PATCH_DESC_MAX_LENGTH = 500;
+function getShaForPatch(patch: PatchSet) {
+ return patch.sha.substring(0, 10);
+}
+
export interface PatchRangeChangeDetail {
patchNum?: PatchSetNum;
basePatchNum?: BasePatchSetNum;
@@ -95,9 +101,6 @@
changeNum?: string;
@property({type: Object})
- changeComments?: ChangeComments;
-
- @property({type: Object})
filesWeblinks?: FilesWebLinks;
@property({type: String})
@@ -106,18 +109,28 @@
@property({type: String})
basePatchNum?: BasePatchSetNum;
+ /** Not used directly. Translated into `sortedRevisions` in willUpdate(). */
@property({type: Object})
- revisions?: RevisionInfo[];
+ revisions: (RevisionInfo | EditRevisionInfo)[] = [];
@property({type: Object})
revisionInfo?: RevisionInfoClass;
- @property({type: Array})
+ /** Private internal state, derived from `revisions` in willUpdate(). */
@state()
- protected sortedRevisions?: RevisionInfo[];
+ private sortedRevisions: (RevisionInfo | EditRevisionInfo)[] = [];
+
+ /** Private internal state, visible for testing. */
+ @state()
+ changeComments?: ChangeComments;
private readonly reporting: ReportingService = appContext.reportingService;
+ constructor() {
+ super();
+ subscribe(this, changeComments$, x => (this.changeComments = x));
+ }
+
static override get styles() {
return [
a11yStyles,
@@ -152,20 +165,6 @@
];
}
- private renderWeblinks(fileLink?: GeneratedWebLink[]) {
- if (!fileLink) return;
-
- return html`<span class="filesWeblinks">
- ${fileLink.map(
- weblink => html`
- <a target="_blank" rel="noopener" href="${weblink.url}">
- ${weblink.name}
- </a>
- `
- )}</span
- > `;
- }
-
override render() {
return html`
<h3 class="assistive-tech-only">Patchset Range Selection</h3>
@@ -173,14 +172,8 @@
<gr-dropdown-list
id="basePatchDropdown"
.value="${convertToString(this.basePatchNum)}"
- .items="${this._computeBaseDropdownContent(
- this.availablePatches,
- this.patchNum,
- this.sortedRevisions,
- this.changeComments,
- this.revisionInfo
- )}"
- @value-change=${this._handlePatchChange}
+ .items="${this.computeBaseDropdownContent()}"
+ @value-change=${this.handlePatchChange}
>
</gr-dropdown-list>
</span>
@@ -190,13 +183,8 @@
<gr-dropdown-list
id="patchNumDropdown"
.value="${convertToString(this.patchNum)}"
- .items="${this._computePatchDropdownContent(
- this.availablePatches,
- this.basePatchNum,
- this.sortedRevisions,
- this.changeComments
- )}"
- @value-change=${this._handlePatchChange}
+ .items="${this.computePatchDropdownContent()}"
+ @value-change=${this.handlePatchChange}
>
</gr-dropdown-list>
${this.renderWeblinks(this.filesWeblinks?.meta_b)}
@@ -204,63 +192,54 @@
`;
}
- override updated(changedProperties: PropertyValues) {
+ private renderWeblinks(fileLinks?: GeneratedWebLink[]) {
+ if (!fileLinks) return;
+ return html`<span class="filesWeblinks">
+ ${fileLinks.map(
+ weblink => html`
+ <a target="_blank" rel="noopener" href="${weblink.url}">
+ ${weblink.name}
+ </a>
+ `
+ )}</span
+ > `;
+ }
+
+ override willUpdate(changedProperties: PropertyValues) {
if (changedProperties.has('revisions')) {
- this._updateSortedRevisions(this.revisions);
+ this.sortedRevisions = sortRevisions(Object.values(this.revisions || {}));
}
}
- _updateSortedRevisions(revisions?: RevisionInfo[]) {
- if (!revisions) return;
- this.sortedRevisions = sortRevisions(Object.values(revisions));
- }
-
- _getShaForPatch(patch: PatchSet) {
- return patch.sha.substring(0, 10);
- }
-
- _computeBaseDropdownContent(
- availablePatches?: PatchSet[],
- patchNum?: PatchSetNum,
- sortedRevisions?: (RevisionInfo | EditRevisionInfo)[],
- changeComments?: ChangeComments,
- revisionInfo?: RevisionInfoClass
- ): DropdownItem[] {
- // Polymer 2: check for undefined
+ // Private method, but visible for testing.
+ computeBaseDropdownContent(): DropdownItem[] {
if (
- availablePatches === undefined ||
- patchNum === undefined ||
- sortedRevisions === undefined ||
- changeComments === undefined ||
- revisionInfo === undefined
+ this.availablePatches === undefined ||
+ this.patchNum === undefined ||
+ this.changeComments === undefined ||
+ this.revisionInfo === undefined
) {
return [];
}
- const parentCounts = revisionInfo.getParentCountMap();
- const currentParentCount = hasOwnProperty(parentCounts, patchNum)
- ? parentCounts[patchNum as number]
+ const parentCounts = this.revisionInfo.getParentCountMap();
+ const currentParentCount = hasOwnProperty(parentCounts, this.patchNum)
+ ? parentCounts[this.patchNum as number]
: 1;
- const maxParents = revisionInfo.getMaxParents();
+ const maxParents = this.revisionInfo.getMaxParents();
const isMerge = currentParentCount > 1;
const dropdownContent: DropdownItem[] = [];
- for (const basePatch of availablePatches) {
+ for (const basePatch of this.availablePatches) {
const basePatchNum = basePatch.num;
- const entry: DropdownItem = this._createDropdownEntry(
+ const entry: DropdownItem = this.createDropdownEntry(
basePatchNum,
'Patchset ',
- sortedRevisions,
- changeComments,
- this._getShaForPatch(basePatch)
+ getShaForPatch(basePatch)
);
dropdownContent.push({
...entry,
- disabled: this._computeLeftDisabled(
- basePatch.num,
- patchNum,
- sortedRevisions
- ),
+ disabled: this.computeLeftDisabled(basePatch.num, this.patchNum),
});
}
@@ -282,91 +261,61 @@
return dropdownContent;
}
- _computeMobileText(
- patchNum: PatchSetNum,
- changeComments: ChangeComments,
- revisions: (RevisionInfo | EditRevisionInfo)[]
- ) {
+ private computeMobileText(patchNum: PatchSetNum) {
return (
`${patchNum}` +
- `${this._computePatchSetCommentsString(changeComments, patchNum)}` +
- `${this._computePatchSetDescription(revisions, patchNum, true)}`
+ `${this.computePatchSetCommentsString(patchNum)}` +
+ `${this.computePatchSetDescription(patchNum, true)}`
);
}
- _computePatchDropdownContent(
- availablePatches?: PatchSet[],
- basePatchNum?: BasePatchSetNum,
- sortedRevisions?: (RevisionInfo | EditRevisionInfo)[],
- changeComments?: ChangeComments
- ): DropdownItem[] {
- // Polymer 2: check for undefined
+ // Private method, but visible for testing.
+ computePatchDropdownContent(): DropdownItem[] {
if (
- availablePatches === undefined ||
- basePatchNum === undefined ||
- sortedRevisions === undefined ||
- changeComments === undefined
+ this.availablePatches === undefined ||
+ this.basePatchNum === undefined ||
+ this.changeComments === undefined
) {
return [];
}
const dropdownContent: DropdownItem[] = [];
- for (const patch of availablePatches) {
+ for (const patch of this.availablePatches) {
const patchNum = patch.num;
- const entry = this._createDropdownEntry(
+ const entry = this.createDropdownEntry(
patchNum,
patchNum === 'edit' ? '' : 'Patchset ',
- sortedRevisions,
- changeComments,
- this._getShaForPatch(patch)
+ getShaForPatch(patch)
);
dropdownContent.push({
...entry,
- disabled: this._computeRightDisabled(
- basePatchNum,
- patchNum,
- sortedRevisions
- ),
+ disabled: this.computeRightDisabled(this.basePatchNum, patchNum),
});
}
return dropdownContent;
}
- _computeText(
- patchNum: PatchSetNum,
- prefix: string,
- changeComments: ChangeComments,
- sha: string
- ) {
+ private computeText(patchNum: PatchSetNum, prefix: string, sha: string) {
return (
`${prefix}${patchNum}` +
- `${this._computePatchSetCommentsString(changeComments, patchNum)}` +
+ `${this.computePatchSetCommentsString(patchNum)}` +
` | ${sha}`
);
}
- _createDropdownEntry(
+ private createDropdownEntry(
patchNum: PatchSetNum,
prefix: string,
- sortedRevisions: (RevisionInfo | EditRevisionInfo)[],
- changeComments: ChangeComments,
sha: string
) {
const entry: DropdownItem = {
triggerText: `${prefix}${patchNum}`,
- text: this._computeText(patchNum, prefix, changeComments, sha),
- mobileText: this._computeMobileText(
- patchNum,
- changeComments,
- sortedRevisions
- ),
- bottomText: `${this._computePatchSetDescription(
- sortedRevisions,
- patchNum
- )}`,
+ text: this.computeText(patchNum, prefix, sha),
+ mobileText: this.computeMobileText(patchNum),
+ bottomText: `${this.computePatchSetDescription(patchNum)}`,
value: patchNum,
};
- const date = this._computePatchSetDate(sortedRevisions, patchNum);
+ const date = this.computePatchSetDate(patchNum);
if (date) {
entry.date = date;
}
@@ -378,17 +327,18 @@
* is sorted in reverse order (higher patchset nums first), invalid base
* patch nums have an index greater than the index of patchNum.
*
+ * Private method, but visible for testing.
+ *
* @param basePatchNum The possible base patch num.
* @param patchNum The current selected patch num.
*/
- _computeLeftDisabled(
+ computeLeftDisabled(
basePatchNum: PatchSetNum,
- patchNum: PatchSetNum,
- sortedRevisions: (RevisionInfo | EditRevisionInfo)[]
+ patchNum: PatchSetNum
): boolean {
return (
- findSortedIndex(basePatchNum, sortedRevisions) <=
- findSortedIndex(patchNum, sortedRevisions)
+ findSortedIndex(basePatchNum, this.sortedRevisions) <=
+ findSortedIndex(patchNum, this.sortedRevisions)
);
}
@@ -403,13 +353,14 @@
* If the current basePatchNum is a parent index, then only patches that have
* at least that many parents are valid.
*
+ * Private method, but visible for testing.
+ *
* @param basePatchNum The current selected base patch num.
* @param patchNum The possible patch num.
*/
- _computeRightDisabled(
+ computeRightDisabled(
basePatchNum: PatchSetNum,
- patchNum: PatchSetNum,
- sortedRevisions: (RevisionInfo | EditRevisionInfo)[]
+ patchNum: PatchSetNum
): boolean {
if (basePatchNum === ParentPatchSetNum) {
return false;
@@ -427,21 +378,17 @@
}
return (
- findSortedIndex(basePatchNum, sortedRevisions) <=
- findSortedIndex(patchNum, sortedRevisions)
+ findSortedIndex(basePatchNum, this.sortedRevisions) <=
+ findSortedIndex(patchNum, this.sortedRevisions)
);
}
// TODO(dhruvsri): have ported comments contribute to this count
- _computePatchSetCommentsString(
- changeComments: ChangeComments,
- patchNum: PatchSetNum
- ) {
- if (!changeComments) {
- return;
- }
+ // Private method, but visible for testing.
+ computePatchSetCommentsString(patchNum: PatchSetNum): string {
+ if (!this.changeComments) return '';
- const commentThreadCount = changeComments.computeCommentThreadCount(
+ const commentThreadCount = this.changeComments.computeCommentThreadCount(
{
patchNum,
},
@@ -449,7 +396,7 @@
);
const commentThreadString = pluralize(commentThreadCount, 'comment');
- const unresolvedCount = changeComments.computeUnresolvedNum(
+ const unresolvedCount = this.changeComments.computeUnresolvedNum(
{patchNum},
true
);
@@ -468,23 +415,19 @@
);
}
- _computePatchSetDescription(
- revisions: (RevisionInfo | EditRevisionInfo)[],
+ private computePatchSetDescription(
patchNum: PatchSetNum,
addFrontSpace?: boolean
) {
- const rev = getRevisionByPatchNum(revisions, patchNum);
+ const rev = getRevisionByPatchNum(this.sortedRevisions, patchNum);
return rev?.description
? (addFrontSpace ? ' ' : '') +
rev.description.substring(0, PATCH_DESC_MAX_LENGTH)
: '';
}
- _computePatchSetDate(
- revisions: (RevisionInfo | EditRevisionInfo)[],
- patchNum: PatchSetNum
- ): Timestamp | undefined {
- const rev = getRevisionByPatchNum(revisions, patchNum);
+ private computePatchSetDate(patchNum: PatchSetNum): Timestamp | undefined {
+ const rev = getRevisionByPatchNum(this.sortedRevisions, patchNum);
return rev ? rev.created : undefined;
}
@@ -492,7 +435,7 @@
* Catches value-change events from the patchset dropdowns and determines
* whether or not a patch change event should be fired.
*/
- _handlePatchChange(e: DropDownValueChangeEvent) {
+ private handlePatchChange(e: DropDownValueChangeEvent) {
const detail: PatchRangeChangeDetail = {
patchNum: this.patchNum,
basePatchNum: this.basePatchNum,
diff --git a/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select_test.ts b/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select_test.ts
index a47b685..342fe3a 100644
--- a/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select_test.ts
+++ b/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select_test.ts
@@ -55,7 +55,7 @@
suite('gr-patch-range-select tests', () => {
let element: GrPatchRangeSelect;
- function getInfo(revisions: RevisionInfo[]) {
+ function getInfo(revisions: (RevisionInfo | EditRevisionInfo)[]) {
const revisionObj: Partial<RevIdToRevisionInfo> = {};
for (let i = 0; i < revisions.length; i++) {
revisionObj[i] = revisions[i];
@@ -78,97 +78,54 @@
await element.updateComplete;
});
- test('enabled/disabled options', () => {
- const patchRange = {
- basePatchNum: 'PARENT' as PatchSetNum,
- patchNum: 3 as PatchSetNum,
- };
- const sortedRevisions = [
+ test('enabled/disabled options', async () => {
+ element.revisions = [
createRevision(3) as RevisionInfo,
createEditRevision(2) as EditRevisionInfo,
createRevision(2) as RevisionInfo,
createRevision(1) as RevisionInfo,
];
+ await element.updateComplete;
+
+ const parent = 'PARENT' as PatchSetNum;
+ const edit = EditPatchSetNum;
+
for (const patchNum of [1, 2, 3]) {
assert.isFalse(
- element._computeRightDisabled(
- patchRange.basePatchNum,
- patchNum as PatchSetNum,
- sortedRevisions
- )
+ element.computeRightDisabled(parent, patchNum as PatchSetNum)
);
}
for (const basePatchNum of [1, 2]) {
- assert.isFalse(
- element._computeLeftDisabled(
- basePatchNum as PatchSetNum,
- patchRange.patchNum,
- sortedRevisions
- )
- );
+ const base = basePatchNum as PatchSetNum;
+ assert.isFalse(element.computeLeftDisabled(base, 3 as PatchSetNum));
}
assert.isTrue(
- element._computeLeftDisabled(3 as PatchSetNum, patchRange.patchNum, [])
+ element.computeLeftDisabled(3 as PatchSetNum, 3 as PatchSetNum)
);
- patchRange.basePatchNum = EditPatchSetNum;
assert.isTrue(
- element._computeLeftDisabled(
- 3 as PatchSetNum,
- patchRange.patchNum,
- sortedRevisions
- )
+ element.computeLeftDisabled(3 as PatchSetNum, 3 as PatchSetNum)
);
- assert.isTrue(
- element._computeRightDisabled(
- patchRange.basePatchNum,
- 1 as PatchSetNum,
- sortedRevisions
- )
- );
- assert.isTrue(
- element._computeRightDisabled(
- patchRange.basePatchNum,
- 2 as PatchSetNum,
- sortedRevisions
- )
- );
- assert.isFalse(
- element._computeRightDisabled(
- patchRange.basePatchNum,
- 3 as PatchSetNum,
- sortedRevisions
- )
- );
- assert.isTrue(
- element._computeRightDisabled(
- patchRange.basePatchNum,
- EditPatchSetNum,
- sortedRevisions
- )
- );
+ assert.isTrue(element.computeRightDisabled(edit, 1 as PatchSetNum));
+ assert.isTrue(element.computeRightDisabled(edit, 2 as PatchSetNum));
+ assert.isFalse(element.computeRightDisabled(edit, 3 as PatchSetNum));
+ assert.isTrue(element.computeRightDisabled(edit, edit));
});
- test('_computeBaseDropdownContent', () => {
- const availablePatches = [
+ test('computeBaseDropdownContent', async () => {
+ element.availablePatches = [
{num: 'edit', sha: '1'} as PatchSet,
{num: 3, sha: '2'} as PatchSet,
{num: 2, sha: '3'} as PatchSet,
{num: 1, sha: '4'} as PatchSet,
];
- const revisions: RevisionInfo[] = [
+ element.revisions = [
createRevision(2),
createRevision(3),
createRevision(1),
createRevision(4),
];
- element.revisionInfo = getInfo(revisions);
- const sortedRevisions = [
- createRevision(3) as RevisionInfo,
- createEditRevision(2) as EditRevisionInfo,
- createRevision(2) as RevisionInfo,
- createRevision(1) as RevisionInfo,
- ];
+ element.revisionInfo = getInfo(element.revisions);
const expectedResult: DropdownItem[] = [
{
disabled: true,
@@ -210,19 +167,14 @@
value: 'PARENT',
} as DropdownItem,
];
- assert.deepEqual(
- element._computeBaseDropdownContent(
- availablePatches,
- 1 as PatchSetNum,
- sortedRevisions,
- element.changeComments,
- element.revisionInfo
- ),
- expectedResult
- );
+ element.patchNum = 1 as PatchSetNum;
+ element.basePatchNum = 'PARENT' as BasePatchSetNum;
+ await element.updateComplete;
+
+ assert.deepEqual(element.computeBaseDropdownContent(), expectedResult);
});
- test('_computeBaseDropdownContent called when patchNum updates', async () => {
+ test('computeBaseDropdownContent called when patchNum updates', async () => {
element.revisions = [
createRevision(2),
createRevision(3),
@@ -240,7 +192,7 @@
element.basePatchNum = 'PARENT' as BasePatchSetNum;
await element.updateComplete;
- const baseDropDownStub = sinon.stub(element, '_computeBaseDropdownContent');
+ const baseDropDownStub = sinon.stub(element, 'computeBaseDropdownContent');
// Should be recomputed for each available patch
element.patchNum = 1 as PatchSetNum;
@@ -248,7 +200,7 @@
assert.equal(baseDropDownStub.callCount, 1);
});
- test('_computeBaseDropdownContent called when changeComments update', async () => {
+ test('computeBaseDropdownContent called when changeComments update', async () => {
element.revisions = [
createRevision(2),
createRevision(3),
@@ -266,14 +218,14 @@
await element.updateComplete;
// Should be recomputed for each available patch
- const baseDropDownStub = sinon.stub(element, '_computeBaseDropdownContent');
+ const baseDropDownStub = sinon.stub(element, 'computeBaseDropdownContent');
assert.equal(baseDropDownStub.callCount, 0);
element.changeComments = new ChangeComments();
await element.updateComplete;
assert.equal(baseDropDownStub.callCount, 1);
});
- test('_computePatchDropdownContent called when basePatchNum updates', async () => {
+ test('computePatchDropdownContent called when basePatchNum updates', async () => {
element.revisions = [
createRevision(2),
createRevision(3),
@@ -292,29 +244,27 @@
await element.updateComplete;
// Should be recomputed for each available patch
- const baseDropDownStub = sinon.stub(
- element,
- '_computePatchDropdownContent'
- );
+ const baseDropDownStub = sinon.stub(element, 'computePatchDropdownContent');
element.basePatchNum = 1 as BasePatchSetNum;
await element.updateComplete;
assert.equal(baseDropDownStub.callCount, 1);
});
- test('_computePatchDropdownContent', () => {
- const availablePatches: PatchSet[] = [
+ test('computePatchDropdownContent', async () => {
+ element.availablePatches = [
{num: 'edit', sha: '1'} as PatchSet,
{num: 3, sha: '2'} as PatchSet,
{num: 2, sha: '3'} as PatchSet,
{num: 1, sha: '4'} as PatchSet,
];
- const basePatchNum = 1;
- const sortedRevisions = [
+ element.basePatchNum = 1 as BasePatchSetNum;
+ element.revisions = [
createRevision(3) as RevisionInfo,
createEditRevision(2) as EditRevisionInfo,
createRevision(2, 'description') as RevisionInfo,
createRevision(1) as RevisionInfo,
];
+ await element.updateComplete;
const expectedResult: DropdownItem[] = [
{
@@ -354,15 +304,7 @@
} as DropdownItem,
];
- assert.deepEqual(
- element._computePatchDropdownContent(
- availablePatches,
- basePatchNum as BasePatchSetNum,
- sortedRevisions,
- element.changeComments
- ),
- expectedResult
- );
+ assert.deepEqual(element.computePatchDropdownContent(), expectedResult);
});
test('filesWeblinks', async () => {
@@ -391,7 +333,7 @@
);
});
- test('_computePatchSetCommentsString', () => {
+ test('computePatchSetCommentsString', () => {
// Test string with unresolved comments.
const comments: PathToCommentsInfoMap = {
foo: [
@@ -432,10 +374,7 @@
element.changeComments = new ChangeComments(comments);
assert.equal(
- element._computePatchSetCommentsString(
- element.changeComments,
- 1 as PatchSetNum
- ),
+ element.computePatchSetCommentsString(1 as PatchSetNum),
' (3 comments, 1 unresolved)'
);
@@ -443,23 +382,14 @@
delete comments['foo'];
element.changeComments = new ChangeComments(comments);
assert.equal(
- element._computePatchSetCommentsString(
- element.changeComments,
- 1 as PatchSetNum
- ),
+ element.computePatchSetCommentsString(1 as PatchSetNum),
' (2 comments)'
);
// Test string with no comments.
delete comments['bar'];
element.changeComments = new ChangeComments(comments);
- assert.equal(
- element._computePatchSetCommentsString(
- element.changeComments,
- 1 as PatchSetNum
- ),
- ''
- );
+ assert.equal(element.computePatchSetCommentsString(1 as PatchSetNum), '');
});
test('patch-range-change fires', () => {
diff --git a/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box.ts b/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box.ts
index 551889f..0f64d9e 100644
--- a/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box.ts
+++ b/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box.ts
@@ -18,7 +18,6 @@
import '../../shared/gr-tooltip/gr-tooltip';
import {GrTooltip} from '../../shared/gr-tooltip/gr-tooltip';
import {customElement, property} from '@polymer/decorators';
-import {flush} from '@polymer/polymer/lib/legacy/polymer.dom';
import {PolymerElement} from '@polymer/polymer/polymer-element';
import {htmlTemplate} from './gr-selection-action-box_html';
import {fireEvent} from '../../../utils/event-util';
@@ -56,8 +55,8 @@
this.addEventListener('mousedown', e => this._handleMouseDown(e));
}
- placeAbove(el: Text | Element | Range) {
- flush();
+ async placeAbove(el: Text | Element | Range) {
+ await this.$.tooltip.updateComplete;
const rect = this._getTargetBoundingRect(el);
const boxRect = this.$.tooltip.getBoundingClientRect();
const parentRect = this._getParentBoundingClientRect();
@@ -70,8 +69,8 @@
}px`;
}
- placeBelow(el: Text | Element | Range) {
- flush();
+ async placeBelow(el: Text | Element | Range) {
+ await this.$.tooltip.updateComplete;
const rect = this._getTargetBoundingRect(el);
const boxRect = this.$.tooltip.getBoundingClientRect();
const parentRect = this._getParentBoundingClientRect();
diff --git a/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box_test.js b/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box_test.js
index 81cf0d6..c978c37 100644
--- a/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box_test.js
+++ b/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box_test.js
@@ -83,35 +83,35 @@
{width: 10, height: 10});
});
- test('placeAbove for Element argument', () => {
- element.placeAbove(target);
+ test('placeAbove for Element argument', async () => {
+ await element.placeAbove(target);
assert.equal(element.style.top, '25px');
assert.equal(element.style.left, '72px');
});
- test('placeAbove for Text Node argument', () => {
- element.placeAbove(target.firstChild);
+ test('placeAbove for Text Node argument', async () => {
+ await element.placeAbove(target.firstChild);
assert.equal(element.style.top, '25px');
assert.equal(element.style.left, '72px');
});
- test('placeBelow for Element argument', () => {
- element.placeBelow(target);
+ test('placeBelow for Element argument', async () => {
+ await element.placeBelow(target);
assert.equal(element.style.top, '45px');
assert.equal(element.style.left, '72px');
});
- test('placeBelow for Text Node argument', () => {
- element.placeBelow(target.firstChild);
+ test('placeBelow for Text Node argument', async () => {
+ await element.placeBelow(target.firstChild);
assert.equal(element.style.top, '45px');
assert.equal(element.style.left, '72px');
});
- test('uses document.createRange', () => {
+ test('uses document.createRange', async () => {
sinon.spy(document, 'createRange');
element._getTargetBoundingRect.restore();
sinon.spy(element, '_getTargetBoundingRect');
- element.placeAbove(target.firstChild);
+ await element.placeAbove(target.firstChild);
assert.isTrue(document.createRange.called);
});
});
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-comment-thread/gr-comment-thread_test.ts b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_test.ts
index 595de34..9e0027a 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_test.ts
@@ -50,7 +50,6 @@
stubReporting,
stubRestApi,
} from '../../../test/test-utils';
-import {_testOnly_resetState} from '../../../services/comments/comments-model';
import {SinonStub} from 'sinon';
const basicFixture = fixtureFromElement('gr-comment-thread');
@@ -63,7 +62,6 @@
setup(() => {
stubRestApi('getLoggedIn').returns(Promise.resolve(false));
- _testOnly_resetState();
element = basicFixture.instantiate();
element.patchNum = 3 as PatchSetNum;
element.changeNum = 1 as NumericChangeId;
diff --git a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.ts b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.ts
index 00edc07..09ac95b 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.ts
@@ -384,11 +384,7 @@
test('delete comment', async () => {
const stub = stubRestApi('deleteComment').returns(
- Promise.resolve({
- id: '1' as UrlEncodedCommentId,
- updated: '1' as Timestamp,
- ...createComment(),
- })
+ Promise.resolve(createComment())
);
const openSpy = sinon.spy(element.confirmDeleteOverlay!, 'open');
element.changeNum = 42 as NumericChangeId;
@@ -1183,6 +1179,7 @@
test('draft prevent save when disabled', async () => {
const saveStub = sinon.stub(element, 'save').returns(Promise.resolve());
+ sinon.stub(element, '_fireEdit');
element.showActions = true;
element.draft = true;
await flush();
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/browser/browser-model.ts b/polygerrit-ui/app/services/browser/browser-model.ts
index db790f6..8cb6575 100644
--- a/polygerrit-ui/app/services/browser/browser-model.ts
+++ b/polygerrit-ui/app/services/browser/browser-model.ts
@@ -33,11 +33,13 @@
const initialState: BrowserState = {};
-// Mutable for testing
-let privateState$ = new BehaviorSubject(initialState);
+const privateState$ = new BehaviorSubject(initialState);
export function _testOnly_resetState() {
- privateState$ = new BehaviorSubject(initialState);
+ // We cannot assign a new subject to privateState$, because all the selectors
+ // have already subscribed to the original subject. So we have to emit the
+ // initial state on the existing subject.
+ privateState$.next({...initialState});
}
export function _testOnly_setState(state: BrowserState) {
diff --git a/polygerrit-ui/app/services/change/change-model.ts b/polygerrit-ui/app/services/change/change-model.ts
index 962ef4d..7df0c22 100644
--- a/polygerrit-ui/app/services/change/change-model.ts
+++ b/polygerrit-ui/app/services/change/change-model.ts
@@ -40,6 +40,13 @@
const privateState$ = new BehaviorSubject(initialState);
+export function _testOnly_resetState() {
+ // We cannot assign a new subject to privateState$, because all the selectors
+ // have already subscribed to the original subject. So we have to emit the
+ // initial state on the existing subject.
+ privateState$.next({...initialState});
+}
+
// Re-exporting as Observable so that you can only subscribe, but not emit.
export const changeState$: Observable<ChangeState> = privateState$;
diff --git a/polygerrit-ui/app/services/checks/checks-model.ts b/polygerrit-ui/app/services/checks/checks-model.ts
index 75c24b6d..6435252 100644
--- a/polygerrit-ui/app/services/checks/checks-model.ts
+++ b/polygerrit-ui/app/services/checks/checks-model.ts
@@ -126,11 +126,13 @@
pluginStateSelected: {},
};
-// Mutable for testing
-let privateState$ = new BehaviorSubject(initialState);
+const privateState$ = new BehaviorSubject(initialState);
export function _testOnly_resetState() {
- privateState$ = new BehaviorSubject(initialState);
+ // We cannot assign a new subject to privateState$, because all the selectors
+ // have already subscribed to the original subject. So we have to emit the
+ // initial state on the existing subject.
+ privateState$.next({...initialState});
}
export function _testOnly_setState(state: ChecksState) {
diff --git a/polygerrit-ui/app/services/checks/checks-model_test.ts b/polygerrit-ui/app/services/checks/checks-model_test.ts
index dbd3f86..0be0451 100644
--- a/polygerrit-ui/app/services/checks/checks-model_test.ts
+++ b/polygerrit-ui/app/services/checks/checks-model_test.ts
@@ -18,7 +18,6 @@
import './checks-model';
import {
_testOnly_getState,
- _testOnly_resetState,
ChecksPatchset,
updateStateSetLoading,
updateStateSetProvider,
@@ -52,7 +51,6 @@
suite('checks-model tests', () => {
test('updateStateSetProvider', () => {
- _testOnly_resetState();
updateStateSetProvider(PLUGIN_NAME, ChecksPatchset.LATEST);
assert.deepEqual(current(), {
pluginName: PLUGIN_NAME,
@@ -65,7 +63,6 @@
});
test('loading and first time load', () => {
- _testOnly_resetState();
updateStateSetProvider(PLUGIN_NAME, ChecksPatchset.LATEST);
assert.isFalse(current().loading);
assert.isTrue(current().firstTimeLoad);
@@ -84,14 +81,12 @@
});
test('updateStateSetResults', () => {
- _testOnly_resetState();
updateStateSetResults(PLUGIN_NAME, RUNS, [], [], ChecksPatchset.LATEST);
assert.lengthOf(current().runs, 1);
assert.lengthOf(current().runs[0].results!, 1);
});
test('updateStateUpdateResult', () => {
- _testOnly_resetState();
updateStateSetResults(PLUGIN_NAME, RUNS, [], [], ChecksPatchset.LATEST);
assert.equal(
current().runs[0].results![0].summary,
diff --git a/polygerrit-ui/app/services/comments/comments-model.ts b/polygerrit-ui/app/services/comments/comments-model.ts
index 850acbc..5b32465 100644
--- a/polygerrit-ui/app/services/comments/comments-model.ts
+++ b/polygerrit-ui/app/services/comments/comments-model.ts
@@ -51,7 +51,10 @@
const privateState$ = new BehaviorSubject(initialState);
export function _testOnly_resetState() {
- privateState$.next(initialState);
+ // We cannot assign a new subject to privateState$, because all the selectors
+ // have already subscribed to the original subject. So we have to emit the
+ // initial state on the existing subject.
+ privateState$.next({...initialState});
}
// Re-exporting as Observable so that you can only subscribe, but not emit.
@@ -65,11 +68,21 @@
privateState$.next(state);
}
+export const comments$ = commentState$.pipe(
+ map(commentState => commentState.comments),
+ distinctUntilChanged()
+);
+
export const drafts$ = commentState$.pipe(
map(commentState => commentState.drafts),
distinctUntilChanged()
);
+export const portedComments$ = commentState$.pipe(
+ map(commentState => commentState.portedComments),
+ distinctUntilChanged()
+);
+
export const discardedDrafts$ = commentState$.pipe(
map(commentState => commentState.discardedDrafts),
distinctUntilChanged()
@@ -87,14 +100,22 @@
commentState.portedComments,
commentState.portedDrafts
)
- ),
- distinctUntilChanged()
+ )
+);
+
+export const threads$ = changeComments$.pipe(
+ map(changeComments => changeComments.getAllThreadsForChange())
);
function publishState(state: CommentState) {
privateState$.next(state);
}
+/** Called when the change number changes. Wipes out all data from the state. */
+export function updateStateReset() {
+ publishState({...initialState});
+}
+
export function updateStateComments(comments?: {
[path: string]: CommentInfo[];
}) {
diff --git a/polygerrit-ui/app/services/comments/comments-model_test.ts b/polygerrit-ui/app/services/comments/comments-model_test.ts
index e389254..30fc7cf 100644
--- a/polygerrit-ui/app/services/comments/comments-model_test.ts
+++ b/polygerrit-ui/app/services/comments/comments-model_test.ts
@@ -22,13 +22,11 @@
import {
updateStateDeleteDraft,
_testOnly_getState,
- _testOnly_resetState,
_testOnly_setState,
} from './comments-model';
suite('comments model tests', () => {
test('updateStateDeleteDraft', () => {
- _testOnly_resetState();
const draft = createDraft();
draft.id = '1' as UrlEncodedCommentId;
_testOnly_setState({
diff --git a/polygerrit-ui/app/services/comments/comments-service.ts b/polygerrit-ui/app/services/comments/comments-service.ts
index 16ee2f7..b5745a8 100644
--- a/polygerrit-ui/app/services/comments/comments-service.ts
+++ b/polygerrit-ui/app/services/comments/comments-service.ts
@@ -31,40 +31,88 @@
updateStatePortedDrafts,
updateStateUndoDiscardedDraft,
discardedDrafts$,
+ updateStateReset,
} from './comments-model';
+import {changeNum$, currentPatchNum$} from '../change/change-model';
+import {combineLatest} from 'rxjs';
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();
+ this.reloadAllComments();
+ });
+ combineLatest([changeNum$, currentPatchNum$]).subscribe(
+ ([changeNum, patchNum]) => {
+ this.changeNum = changeNum;
+ this.patchNum = patchNum;
+ this.reloadAllPortedComments();
+ }
+ );
+ document.addEventListener('reload', () => {
+ this.reloadAllComments();
+ this.reloadAllPortedComments();
+ });
}
- /**
- * Load all comments (with drafts and robot comments) for the given change
- * number. The returned promise resolves when the comments have loaded, but
- * does not yield the comment data.
- */
- // TODO(dhruvsri): listen to changeNum changes or reload event to update
- // automatically
- loadAll(changeNum: NumericChangeId, patchNum = CURRENT as RevisionId) {
- const revision = patchNum;
- this.restApiService
+ // 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> {
+ return this.restApiService
.getDiffComments(changeNum)
.then(comments => updateStateComments(comments));
- this.restApiService
+ }
+
+ reloadRobotComments(changeNum: NumericChangeId): Promise<void> {
+ return this.restApiService
.getDiffRobotComments(changeNum)
.then(robotComments => updateStateRobotComments(robotComments));
- this.restApiService
+ }
+
+ reloadDrafts(changeNum: NumericChangeId): Promise<void> {
+ return this.restApiService
.getDiffDrafts(changeNum)
.then(drafts => updateStateDrafts(drafts));
- this.restApiService
- .getPortedComments(changeNum, revision)
+ }
+
+ reloadPortedComments(
+ changeNum: NumericChangeId,
+ patchNum = CURRENT as RevisionId
+ ): Promise<void> {
+ return this.restApiService
+ .getPortedComments(changeNum, patchNum)
.then(portedComments => updateStatePortedComments(portedComments));
- this.restApiService
- .getPortedDrafts(changeNum, revision)
+ }
+
+ reloadPortedDrafts(
+ changeNum: NumericChangeId,
+ patchNum = CURRENT as RevisionId
+ ): Promise<void> {
+ return this.restApiService
+ .getPortedDrafts(changeNum, patchNum)
.then(portedDrafts => updateStatePortedDrafts(portedDrafts));
}
diff --git a/polygerrit-ui/app/services/comments/comments-service_test.ts b/polygerrit-ui/app/services/comments/comments-service_test.ts
index 604b5c4..a35768e 100644
--- a/polygerrit-ui/app/services/comments/comments-service_test.ts
+++ b/polygerrit-ui/app/services/comments/comments-service_test.ts
@@ -18,109 +18,63 @@
import '../../test/common-test-setup-karma';
import {
createComment,
- createFixSuggestionInfo,
+ createParsedChange,
+ TEST_NUMERIC_CHANGE_ID,
} from '../../test/test-data-generators';
-import {stubRestApi} from '../../test/test-utils';
-import {
- NumericChangeId,
- RobotId,
- RobotRunId,
- Timestamp,
- UrlEncodedCommentId,
-} from '../../types/common';
+import {stubRestApi, waitUntil, waitUntilCalled} from '../../test/test-utils';
import {appContext} from '../app-context';
import {CommentsService} from './comments-service';
+import {updateState as updateChangeState} from '../change/change-model';
+import {
+ GerritView,
+ updateState as updateRouterState,
+} from '../router/router-model';
+import {comments$, portedComments$} from './comments-model';
+import {PathToCommentsInfoMap} from '../../types/common';
suite('change service tests', () => {
- let commentsService: CommentsService;
-
- test('loads logged-out', () => {
- const changeNum = 1234 as NumericChangeId;
- commentsService = new CommentsService(appContext.restApiService);
- stubRestApi('getLoggedIn').returns(Promise.resolve(false));
+ test('loads comments', async () => {
+ new CommentsService(appContext.restApiService);
const diffCommentsSpy = stubRestApi('getDiffComments').returns(
- Promise.resolve({
- 'foo.c': [
- {
- ...createComment(),
- id: '123' as UrlEncodedCommentId,
- message: 'Done',
- updated: '2017-02-08 16:40:49' as Timestamp,
- },
- ],
- })
+ Promise.resolve({'foo.c': [createComment()]})
);
const diffRobotCommentsSpy = stubRestApi('getDiffRobotComments').returns(
- Promise.resolve({
- 'foo.c': [
- {
- ...createComment(),
- id: '321' as UrlEncodedCommentId,
- message: 'Done',
- updated: '2017-02-08 16:40:49' as Timestamp,
- robot_id: 'robot_1' as RobotId,
- robot_run_id: 'run_1' as RobotRunId,
- properties: {},
- fix_suggestions: [
- createFixSuggestionInfo('fix_1'),
- createFixSuggestionInfo('fix_2'),
- ],
- },
- ],
- })
+ Promise.resolve({})
);
const diffDraftsSpy = stubRestApi('getDiffDrafts').returns(
Promise.resolve({})
);
-
- commentsService.loadAll(changeNum);
- assert.isTrue(diffCommentsSpy.calledWithExactly(changeNum));
- assert.isTrue(diffRobotCommentsSpy.calledWithExactly(changeNum));
- assert.isTrue(diffDraftsSpy.calledWithExactly(changeNum));
- });
-
- test('loads logged-in', () => {
- const changeNum = 1234 as NumericChangeId;
-
- stubRestApi('getLoggedIn').returns(Promise.resolve(true));
- const diffCommentsSpy = stubRestApi('getDiffComments').returns(
- Promise.resolve({
- 'foo.c': [
- {
- ...createComment(),
- id: '123' as UrlEncodedCommentId,
- message: 'Done',
- updated: '2017-02-08 16:40:49' as Timestamp,
- },
- ],
- })
+ const portedCommentsSpy = stubRestApi('getPortedComments').returns(
+ Promise.resolve({'foo.c': [createComment()]})
);
- const diffRobotCommentsSpy = stubRestApi('getDiffRobotComments').returns(
- Promise.resolve({
- 'foo.c': [
- {
- ...createComment(),
- id: '321' as UrlEncodedCommentId,
- message: 'Done',
- updated: '2017-02-08 16:40:49' as Timestamp,
- robot_id: 'robot_1' as RobotId,
- robot_run_id: 'run_1' as RobotRunId,
- properties: {},
- fix_suggestions: [
- createFixSuggestionInfo('fix_1'),
- createFixSuggestionInfo('fix_2'),
- ],
- },
- ],
- })
- );
- const diffDraftsSpy = stubRestApi('getDiffDrafts').returns(
+ const portedDraftsSpy = stubRestApi('getPortedDrafts').returns(
Promise.resolve({})
);
+ let comments: PathToCommentsInfoMap = {};
+ comments$.subscribe(c => (comments = c));
+ let portedComments: PathToCommentsInfoMap = {};
+ portedComments$.subscribe(c => (portedComments = c));
- commentsService.loadAll(changeNum);
- assert.isTrue(diffCommentsSpy.calledWithExactly(changeNum));
- assert.isTrue(diffRobotCommentsSpy.calledWithExactly(changeNum));
- assert.isTrue(diffDraftsSpy.calledWithExactly(changeNum));
+ updateRouterState(GerritView.CHANGE, TEST_NUMERIC_CHANGE_ID);
+ updateChangeState(createParsedChange());
+
+ await waitUntilCalled(diffCommentsSpy, 'diffCommentsSpy');
+ await waitUntilCalled(diffRobotCommentsSpy, 'diffRobotCommentsSpy');
+ await waitUntilCalled(diffDraftsSpy, 'diffDraftsSpy');
+ await waitUntilCalled(portedCommentsSpy, 'portedCommentsSpy');
+ await waitUntilCalled(portedDraftsSpy, 'portedDraftsSpy');
+ await waitUntil(
+ () => Object.keys(comments).length > 0,
+ 'comment in model not set'
+ );
+ await waitUntil(
+ () => Object.keys(portedComments).length > 0,
+ 'ported comment in model not set'
+ );
+
+ assert.equal(comments['foo.c'].length, 1);
+ assert.equal(comments['foo.c'][0].id, '12345');
+ assert.equal(portedComments['foo.c'].length, 1);
+ assert.equal(portedComments['foo.c'][0].id, '12345');
});
});
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 b3cdf9e..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 {
@@ -47,11 +48,19 @@
const privateState$ = new BehaviorSubject<RouterState>(initialState);
+export function _testOnly_resetState() {
+ // We cannot assign a new subject to privateState$, because all the selectors
+ // have already subscribed to the original subject. So we have to emit the
+ // initial state on the existing subject.
+ privateState$.next({...initialState});
+}
+
// Re-exporting as Observable so that you can only subscribe, but not emit.
export const routerState$: Observable<RouterState> = privateState$;
// 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/services/user/user-model.ts b/polygerrit-ui/app/services/user/user-model.ts
index 000887c..df307d6 100644
--- a/polygerrit-ui/app/services/user/user-model.ts
+++ b/polygerrit-ui/app/services/user/user-model.ts
@@ -37,11 +37,13 @@
diffPreferences: createDefaultDiffPrefs(),
};
-// Mutable for testing
-let privateState$ = new BehaviorSubject(initialState);
+const privateState$ = new BehaviorSubject(initialState);
export function _testOnly_resetState() {
- privateState$ = new BehaviorSubject(initialState);
+ // We cannot assign a new subject to privateState$, because all the selectors
+ // have already subscribed to the original subject. So we have to emit the
+ // initial state on the existing subject.
+ privateState$.next({...initialState});
}
export function _testOnly_setState(state: UserState) {
diff --git a/polygerrit-ui/app/test/common-test-setup.ts b/polygerrit-ui/app/test/common-test-setup.ts
index bd5504a..05adb41 100644
--- a/polygerrit-ui/app/test/common-test-setup.ts
+++ b/polygerrit-ui/app/test/common-test-setup.ts
@@ -45,6 +45,12 @@
import {updatePreferences} from '../services/user/user-model';
import {createDefaultPreferences} from '../constants/constants';
import {appContext} from '../services/app-context';
+import {_testOnly_resetState as resetBrowserState} from '../services/browser/browser-model';
+import {_testOnly_resetState as resetChangeState} from '../services/change/change-model';
+import {_testOnly_resetState as resetChecksState} from '../services/checks/checks-model';
+import {_testOnly_resetState as resetCommentsState} from '../services/comments/comments-model';
+import {_testOnly_resetState as resetRouterState} from '../services/router/router-model';
+import {_testOnly_resetState as resetUserState} from '../services/user/user-model';
declare global {
interface Window {
@@ -106,6 +112,14 @@
// tests.
initGlobalVariables();
_testOnly_initGerritPluginApi();
+
+ resetBrowserState();
+ resetChangeState();
+ resetChecksState();
+ resetCommentsState();
+ resetRouterState();
+ resetUserState();
+
const shortcuts = appContext.shortcutsService;
assert.isTrue(shortcuts._testOnly_isEmpty());
const selection = document.getSelection();
diff --git a/polygerrit-ui/app/test/test-data-generators.ts b/polygerrit-ui/app/test/test-data-generators.ts
index dd56ce2..91cd2f3 100644
--- a/polygerrit-ui/app/test/test-data-generators.ts
+++ b/polygerrit-ui/app/test/test-data-generators.ts
@@ -95,7 +95,12 @@
import {AppElementChangeViewParams} from '../elements/gr-app-types';
import {CommitInfoWithRequiredCommit} from '../elements/change/gr-change-metadata/gr-change-metadata';
import {WebLinkInfo} from '../types/diff';
-import {createCommentThreads, UIComment, UIDraft} from '../utils/comment-util';
+import {
+ createCommentThreads,
+ UIComment,
+ UIDraft,
+ UIHuman,
+} from '../utils/comment-util';
import {GerritView} from '../services/router/router-model';
import {ChangeComments} from '../elements/diff/gr-comment-api/gr-comment-api';
import {EditRevisionInfo, ParsedChangeInfo} from '../types/types';
@@ -488,7 +493,7 @@
};
}
-export function createComment(): UIComment {
+export function createComment(): UIHuman {
return {
patch_set: 1 as PatchSetNum,
id: '12345' as UrlEncodedCommentId,
diff --git a/polygerrit-ui/app/test/test-utils.ts b/polygerrit-ui/app/test/test-utils.ts
index 4a513f8..faecda0 100644
--- a/polygerrit-ui/app/test/test-utils.ts
+++ b/polygerrit-ui/app/test/test-utils.ts
@@ -19,7 +19,7 @@
import {_testOnly_resetEndpoints} from '../elements/shared/gr-js-api-interface/gr-plugin-endpoints';
import {appContext} from '../services/app-context';
import {RestApiService} from '../services/gr-rest-api/gr-rest-api';
-import {SinonSpy} from 'sinon';
+import {SinonSpy, SinonStub} from 'sinon';
import {StorageService} from '../services/storage/gr-storage';
import {AuthService} from '../services/gr-auth/gr-auth';
import {ReportingService} from '../services/gr-reporting/gr-reporting';
@@ -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>
@@ -185,6 +190,7 @@
): Promise<void> {
const start = Date.now();
let sleep = 0;
+ if (predicate()) return Promise.resolve();
return new Promise((resolve, reject) => {
const waiter = () => {
if (predicate()) {
@@ -200,6 +206,10 @@
});
}
+export function waitUntilCalled(stub: SinonStub, name: string) {
+ return waitUntil(() => stub.called, `${name} was not called`);
+}
+
/**
* Promisify an event callback to simplify async...await tests.
*
diff --git a/polygerrit-ui/app/types/common.ts b/polygerrit-ui/app/types/common.ts
index 3ac7c7b..4406a73 100644
--- a/polygerrit-ui/app/types/common.ts
+++ b/polygerrit-ui/app/types/common.ts
@@ -206,6 +206,7 @@
UrlEncodedRepoName,
UserConfigInfo,
VotingRangeInfo,
+ WebLinkInfo,
isDetailedLabelInfo,
isQuickLabelInfo,
};
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/polygerrit-ui/app/utils/patch-set-util.ts b/polygerrit-ui/app/utils/patch-set-util.ts
index ee4ed8b..921850a 100644
--- a/polygerrit-ui/app/utils/patch-set-util.ts
+++ b/polygerrit-ui/app/utils/patch-set-util.ts
@@ -103,7 +103,7 @@
return rev;
}
}
- console.warn('no revision found');
+ if (revisions.length > 0) console.warn('no revision found');
return;
}
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",
