Merge "Introduce RefUpdateContext in gerrit."
diff --git a/Documentation/config-submit-requirements.txt b/Documentation/config-submit-requirements.txt
index ba20cea..5ff9425 100644
--- a/Documentation/config-submit-requirements.txt
+++ b/Documentation/config-submit-requirements.txt
@@ -246,6 +246,15 @@
name contains the file pattern, or the edits of the file diff contain the edit
pattern.
+[[operator_label]]
+label:labelName=+1,user=non_contributor::
++
+Submit requirements support an additional `user=non_contributor` argument for
+labels that returns true if the change has a label vote matching the specified
+value and the vote is applied from a gerrit account that's not the uploader,
+author or committer of the latest patchset. See the documentation for the labels
+operator in the link:user-search.html[user search] page.
+
[[unsupported_operators]]
=== Unsupported Operators
diff --git a/Documentation/js_licenses.txt b/Documentation/js_licenses.txt
index 114aa3a..f685af9 100644
--- a/Documentation/js_licenses.txt
+++ b/Documentation/js_licenses.txt
@@ -603,39 +603,6 @@
----
-[[ba-linkify]]
-ba-linkify
-
-* ba-linkify
-
-[[ba-linkify_license]]
-----
-Copyright (c) 2009 "Cowboy" Ben Alman
-
-Permission is hereby granted, free of charge, to any person
-obtaining a copy of this software and associated documentation
-files (the "Software"), to deal in the Software without
-restriction, including without limitation the rights to use,
-copy, modify, merge, publish, distribute, sublicense, and/or sell
-copies of the Software, and to permit persons to whom the
-Software is furnished to do so, subject to the following
-conditions:
-
-The above copyright notice and this permission notice shall be
-included in all copies or substantial portions of the Software.
-
-THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
-EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
-OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
-NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
-HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
-WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
-FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
-OTHER DEALINGS IN THE SOFTWARE.
-
-----
-
-
[[codemirror-minified]]
codemirror-minified
diff --git a/Documentation/licenses.txt b/Documentation/licenses.txt
index f8ca85b..be41d0c 100644
--- a/Documentation/licenses.txt
+++ b/Documentation/licenses.txt
@@ -3507,39 +3507,6 @@
----
-[[ba-linkify]]
-ba-linkify
-
-* ba-linkify
-
-[[ba-linkify_license]]
-----
-Copyright (c) 2009 "Cowboy" Ben Alman
-
-Permission is hereby granted, free of charge, to any person
-obtaining a copy of this software and associated documentation
-files (the "Software"), to deal in the Software without
-restriction, including without limitation the rights to use,
-copy, modify, merge, publish, distribute, sublicense, and/or sell
-copies of the Software, and to permit persons to whom the
-Software is furnished to do so, subject to the following
-conditions:
-
-The above copyright notice and this permission notice shall be
-included in all copies or substantial portions of the Software.
-
-THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
-EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
-OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
-NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
-HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
-WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
-FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
-OTHER DEALINGS IN THE SOFTWARE.
-
-----
-
-
[[codemirror-minified]]
codemirror-minified
diff --git a/contrib/git-gc-preserve b/contrib/git-gc-preserve
index a886721..33c8f5b 100755
--- a/contrib/git-gc-preserve
+++ b/contrib/git-gc-preserve
@@ -49,10 +49,10 @@
[2] https://git.eclipse.org/r/c/jgit/jgit/+/122288
CONFIGURATION
- "gc.prunepreserved": if set to "true" preserved packs from the last gc run
+ "pack.prunepreserved": if set to "true" preserved packs from the last gc run
are pruned before current packs are preserved.
- "gc.preserveoldpacks": if set to "true" current packs will be hard linked
+ "pack.preserveoldpacks": if set to "true" current packs will be hard linked
to objects/pack/preserved before git gc is executed. JGit will
fallback to the preserved packs in this directory in case it comes
across missing objects which might be caused by a concurrent run of
@@ -84,9 +84,9 @@
exec 9>&-
}
-# prune preserved packs if gc.prunepreserved == true
+# prune preserved packs if pack.prunepreserved == true
prune_preserved() { # repo
- configured=$(git --git-dir="$1" config --get gc.prunepreserved)
+ configured=$(git --git-dir="$1" config --get pack.prunepreserved)
if [ "$configured" != "true" ]; then
return 0
fi
@@ -99,9 +99,9 @@
fi
}
-# preserve packs if gc.preserveoldpacks == true
+# preserve packs if pack.preserveoldpacks == true
preserve_packs() { # repo
- configured=$(git --git-dir="$1" config --get gc.preserveoldpacks)
+ configured=$(git --git-dir="$1" config --get pack.preserveoldpacks)
if [ "$configured" != "true" ]; then
return 0
fi
diff --git a/java/com/google/gerrit/acceptance/BUILD b/java/com/google/gerrit/acceptance/BUILD
index 3c7ec2b..4238563 100644
--- a/java/com/google/gerrit/acceptance/BUILD
+++ b/java/com/google/gerrit/acceptance/BUILD
@@ -53,6 +53,7 @@
"//lib/bouncycastle:bcpg",
"//lib/bouncycastle:bcpkix",
"//lib/bouncycastle:bcprov",
+ "//lib/bouncycastle:bcutil",
"//prolog:gerrit-prolog-common",
]
diff --git a/java/com/google/gerrit/server/mail/send/MailSoySauceLoader.java b/java/com/google/gerrit/server/mail/send/MailSoySauceLoader.java
index ce54708..0eaafb8 100644
--- a/java/com/google/gerrit/server/mail/send/MailSoySauceLoader.java
+++ b/java/com/google/gerrit/server/mail/send/MailSoySauceLoader.java
@@ -90,7 +90,7 @@
"RevertedHtml.soy",
};
- private static final SoySauce DEFAULT = getDefault().build().compileTemplates();
+ private static final SoySauce DEFAULT = getDefault(null).build().compileTemplates();
private final SitePaths site;
private final PluginSetContext<MailSoyTemplateProvider> templateProviders;
@@ -106,7 +106,7 @@
return DEFAULT;
}
- SoyFileSet.Builder builder = getDefault();
+ SoyFileSet.Builder builder = getDefault(site);
templateProviders.runEach(
e -> e.getFileNames().forEach(p -> addTemplate(builder, site, e.getPath(), p)));
return builder.build().compileTemplates();
@@ -124,10 +124,10 @@
}
}
- private static SoyFileSet.Builder getDefault() {
+ private static SoyFileSet.Builder getDefault(@Nullable SitePaths site) {
SoyFileSet.Builder builder = SoyFileSet.builder();
for (String name : TEMPLATES) {
- addTemplate(builder, null, "com/google/gerrit/server/mail/", name);
+ addTemplate(builder, site, "com/google/gerrit/server/mail/", name);
}
return builder;
}
diff --git a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
index 738eab3..f6fc8db 100644
--- a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
+++ b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
@@ -225,9 +225,11 @@
public static final String ARG_ID_GROUP = "group";
public static final String ARG_ID_OWNER = "owner";
public static final String ARG_ID_NON_UPLOADER = "non_uploader";
+ public static final String ARG_ID_NON_CONTRIBUTOR = "non_contributor";
public static final String ARG_COUNT = "count";
public static final Account.Id OWNER_ACCOUNT_ID = Account.id(0);
public static final Account.Id NON_UPLOADER_ACCOUNT_ID = Account.id(-1);
+ public static final Account.Id NON_CONTRIBUTOR_ACCOUNT_ID = Account.id(-2);
public static final String OPERATOR_MERGED_BEFORE = "mergedbefore";
public static final String OPERATOR_MERGED_AFTER = "mergedafter";
@@ -485,13 +487,13 @@
}
}
- private final Arguments args;
+ protected final Arguments args;
protected Map<String, String> hasOperandAliases = Collections.emptyMap();
private Map<Account.Id, DestinationList> destinationListByAccount = new HashMap<>();
private static final Splitter RULE_SPLITTER = Splitter.on("=");
private static final Splitter PLUGIN_SPLITTER = Splitter.on("_");
- private static final Splitter LABEL_SPLITTER = Splitter.on(",");
+ protected static final Splitter LABEL_SPLITTER = Splitter.on(",");
@Inject
protected ChangeQueryBuilder(Arguments args) {
@@ -1038,6 +1040,8 @@
accounts = Collections.singleton(OWNER_ACCOUNT_ID);
} else if (value.equals(ARG_ID_NON_UPLOADER)) {
accounts = Collections.singleton(NON_UPLOADER_ACCOUNT_ID);
+ } else if (value.equals(ARG_ID_NON_CONTRIBUTOR)) {
+ accounts = Collections.singleton(NON_CONTRIBUTOR_ACCOUNT_ID);
} else {
accounts = parseAccount(value);
}
@@ -1072,6 +1076,8 @@
accounts = Collections.singleton(OWNER_ACCOUNT_ID);
} else if (value.equals(ARG_ID_NON_UPLOADER)) {
accounts = Collections.singleton(NON_UPLOADER_ACCOUNT_ID);
+ } else if (value.equals(ARG_ID_NON_CONTRIBUTOR)) {
+ accounts = Collections.singleton(NON_CONTRIBUTOR_ACCOUNT_ID);
} else {
accounts = parseAccount(value);
}
@@ -1106,9 +1112,16 @@
}
}
+ validateLabelArgs(accounts);
return new LabelPredicate(args, name, accounts, group, count, countOp);
}
+ protected void validateLabelArgs(Set<Account.Id> accounts) throws QueryParseException {
+ if (accounts != null && accounts.contains(NON_CONTRIBUTOR_ACCOUNT_ID)) {
+ throw new QueryParseException("non_contributor arg is not allowed in change queries");
+ }
+ }
+
/** Assert that keys {@code k1} and {@code k2} do not exist in {@code labelArgs} together. */
private void assertDisjunctive(PredicateArgs labelArgs, String k1, String k2)
throws QueryParseException {
diff --git a/java/com/google/gerrit/server/query/change/EqualsLabelPredicate.java b/java/com/google/gerrit/server/query/change/EqualsLabelPredicate.java
index 5662e4d..83dd5ba 100644
--- a/java/com/google/gerrit/server/query/change/EqualsLabelPredicate.java
+++ b/java/com/google/gerrit/server/query/change/EqualsLabelPredicate.java
@@ -14,6 +14,7 @@
package com.google.gerrit.server.query.change;
+import com.google.common.flogger.FluentLogger;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.AccountGroup;
@@ -23,6 +24,8 @@
import com.google.gerrit.entities.PatchSetApproval;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.account.AccountResolver;
+import com.google.gerrit.server.account.AccountState;
import com.google.gerrit.server.index.change.ChangeField;
import com.google.gerrit.server.permissions.ChangePermission;
import com.google.gerrit.server.permissions.PermissionBackend;
@@ -30,9 +33,15 @@
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectState;
import com.google.gerrit.server.query.change.ChangeData.StorageConstraint;
+import java.io.IOException;
+import java.util.List;
import java.util.Optional;
+import org.eclipse.jgit.errors.ConfigInvalidException;
public class EqualsLabelPredicate extends ChangeIndexPostFilterPredicate {
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+
+ protected final AccountResolver accountResolver;
protected final ProjectCache projectCache;
protected final PermissionBackend permissionBackend;
protected final IdentifiedUser.GenericFactory userFactory;
@@ -61,6 +70,7 @@
@Nullable Integer count) {
super(ChangeField.LABEL_SPEC, ChangeField.formatLabel(label, expVal, account, count));
this.permissionBackend = args.permissionBackend;
+ this.accountResolver = args.accountResolver;
this.projectCache = args.projectCache;
this.userFactory = args.userFactory;
this.count = count;
@@ -155,6 +165,14 @@
&& cd.currentPatchSet().uploader().equals(approver)) {
return false;
}
+
+ if (account.equals(ChangeQueryBuilder.NON_CONTRIBUTOR_ACCOUNT_ID)) {
+ if ((cd.currentPatchSet().uploader().equals(approver)
+ || matchAccount(cd.getCommitter().getEmailAddress(), approver)
+ || matchAccount(cd.getAuthor().getEmailAddress(), approver))) {
+ return false;
+ }
+ }
}
IdentifiedUser reviewer = userFactory.create(approver);
@@ -176,9 +194,24 @@
}
}
+ /**
+ * Returns true if the {@code email} parameter belongs to the account identified by the {@code
+ * accountId} parameter.
+ */
+ private boolean matchAccount(String email, Account.Id accountId) {
+ try {
+ List<AccountState> accountsList = accountResolver.resolve(email).asList();
+ return accountsList.stream().anyMatch(c -> c.account().id().equals(accountId));
+ } catch (ConfigInvalidException | IOException e) {
+ logger.atWarning().withCause(e).log("Failed to resolve account %s", email);
+ }
+ return false;
+ }
+
private boolean isMagicUser() {
return account.equals(ChangeQueryBuilder.OWNER_ACCOUNT_ID)
- || account.equals(ChangeQueryBuilder.NON_UPLOADER_ACCOUNT_ID);
+ || account.equals(ChangeQueryBuilder.NON_UPLOADER_ACCOUNT_ID)
+ || account.equals(ChangeQueryBuilder.NON_CONTRIBUTOR_ACCOUNT_ID);
}
@Override
diff --git a/java/com/google/gerrit/server/query/change/LabelPredicate.java b/java/com/google/gerrit/server/query/change/LabelPredicate.java
index 2a5a47d..d89940d 100644
--- a/java/com/google/gerrit/server/query/change/LabelPredicate.java
+++ b/java/com/google/gerrit/server/query/change/LabelPredicate.java
@@ -23,6 +23,7 @@
import com.google.gerrit.index.query.RangeUtil;
import com.google.gerrit.index.query.RangeUtil.Range;
import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.account.AccountResolver;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.util.LabelVote;
@@ -36,6 +37,7 @@
protected static final int MAX_COUNT = 5; // inclusive
protected static class Args {
+ protected final AccountResolver accountResolver;
protected final ProjectCache projectCache;
protected final PermissionBackend permissionBackend;
protected final IdentifiedUser.GenericFactory userFactory;
@@ -46,6 +48,7 @@
protected final PredicateArgs.Operator countOp;
protected Args(
+ AccountResolver accountResolver,
ProjectCache projectCache,
PermissionBackend permissionBackend,
IdentifiedUser.GenericFactory userFactory,
@@ -54,6 +57,7 @@
AccountGroup.UUID group,
@Nullable Integer count,
@Nullable PredicateArgs.Operator countOp) {
+ this.accountResolver = accountResolver;
this.projectCache = projectCache;
this.permissionBackend = permissionBackend;
this.userFactory = userFactory;
@@ -89,6 +93,7 @@
super(
predicates(
new Args(
+ a.accountResolver,
a.projectCache,
a.permissionBackend,
a.userFactory,
diff --git a/java/com/google/gerrit/server/query/change/SubmitRequirementChangeQueryBuilder.java b/java/com/google/gerrit/server/query/change/SubmitRequirementChangeQueryBuilder.java
index 5632c14..cb92ddd 100644
--- a/java/com/google/gerrit/server/query/change/SubmitRequirementChangeQueryBuilder.java
+++ b/java/com/google/gerrit/server/query/change/SubmitRequirementChangeQueryBuilder.java
@@ -15,6 +15,7 @@
package com.google.gerrit.server.query.change;
import com.google.common.base.Splitter;
+import com.google.gerrit.entities.Account;
import com.google.gerrit.index.SchemaFieldDefs.SchemaField;
import com.google.gerrit.index.query.Predicate;
import com.google.gerrit.index.query.QueryBuilder;
@@ -30,6 +31,7 @@
import com.google.inject.Inject;
import java.util.List;
import java.util.Locale;
+import java.util.Set;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import java.util.regex.PatternSyntaxException;
@@ -179,6 +181,9 @@
return fileEditsPredicateFactory.create(FileEditsArgs.create(filePattern, contentPattern));
}
+ @Override
+ protected void validateLabelArgs(Set<Account.Id> accountIds) throws QueryParseException {}
+
private static void validateRegularExpression(String pattern, String errorMessage)
throws QueryParseException {
try {
diff --git a/javatests/com/google/gerrit/acceptance/api/change/SubmitRequirementPredicateIT.java b/javatests/com/google/gerrit/acceptance/api/change/SubmitRequirementPredicateIT.java
index 56e23a4..2fe7038 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/SubmitRequirementPredicateIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/SubmitRequirementPredicateIT.java
@@ -17,6 +17,8 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.allowLabel;
import static com.google.gerrit.server.group.SystemGroupBackend.ANONYMOUS_USERS;
+import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
+import static com.google.gerrit.server.project.testing.TestLabels.codeReview;
import static com.google.gerrit.server.project.testing.TestLabels.label;
import static com.google.gerrit.server.project.testing.TestLabels.value;
import static org.eclipse.jgit.lib.Constants.HEAD;
@@ -26,6 +28,8 @@
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.NoHttpd;
import com.google.gerrit.acceptance.PushOneCommit;
+import com.google.gerrit.acceptance.Sandboxed;
+import com.google.gerrit.acceptance.TestAccount;
import com.google.gerrit.acceptance.UseTimezone;
import com.google.gerrit.acceptance.VerifyNoPiiInChangeNotes;
import com.google.gerrit.acceptance.testsuite.account.AccountOperations;
@@ -34,15 +38,21 @@
import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.Account;
+import com.google.gerrit.entities.AccountGroup;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.LabelType;
+import com.google.gerrit.entities.RefNames;
import com.google.gerrit.entities.SubmitRequirementExpression;
import com.google.gerrit.entities.SubmitRequirementExpressionResult;
import com.google.gerrit.extensions.api.changes.ReviewInput;
+import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.server.project.SubmitRequirementsEvaluator;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.inject.Inject;
+import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
+import org.eclipse.jgit.junit.TestRepository;
import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.merge.MergeStrategy;
import org.eclipse.jgit.merge.ThreeWayMerger;
import org.eclipse.jgit.revwalk.RevCommit;
@@ -294,6 +304,162 @@
"unexpected base value format");
}
+ @Test
+ public void nonContributorLabelVote_match() throws Exception {
+ requestScopeOperations.setApiUser(user.id());
+ TestRepository<InMemoryRepository> clonedRepo = cloneProject(project, user);
+ PushOneCommit.Result r1 =
+ pushFactory
+ .create(user.newIdent(), clonedRepo, "Subject", "file.txt", "text")
+ .to("refs/for/master");
+
+ Change.Id cId = r1.getChange().getId();
+
+ ChangeInfo changeInfo = gApi.changes().id(r1.getChangeId()).get();
+
+ // Assert on uploader, committer and author
+ assertUploader(changeInfo, user.email());
+ assertCommitter(changeInfo, user.email());
+ assertAuthor(changeInfo, user.email());
+
+ // Vote from admin (a.k.a. non uploader/committer/author) matches
+ requestScopeOperations.setApiUser(admin.id());
+ approve(cId.toString());
+ assertMatching("label:Code-Review=+2,user=non_contributor", cId);
+ // Also make sure magic label votes and > operator work
+ assertMatching("label:Code-Review=MAX,user=non_contributor", cId);
+ assertMatching("label:Code-Review>+1,user=non_contributor", cId);
+ }
+
+ @Test
+ public void nonContributorLabelVote_voteFromUploader_doesNotMatch() throws Exception {
+ PushOneCommit.Result r1 = createNormalCommit(user.newIdent(), "refs/for/master", "file1");
+
+ ChangeInfo changeInfo = gApi.changes().id(r1.getChangeId()).get();
+ assertUploader(changeInfo, admin.email());
+
+ // Vote from admin (a.k.a. uploader) does not match
+ requestScopeOperations.setApiUser(admin.id());
+ approve(r1.getChangeId());
+ assertNotMatching("label:Code-Review=+2,user=non_contributor", r1.getChange().getId());
+ }
+
+ @Test
+ @Sandboxed
+ public void nonContributorLabelVote_voteFromAuthor_doesNotMatch() throws Exception {
+ Account.Id authorId =
+ accountOperations
+ .newAccount()
+ .fullname("author")
+ .preferredEmail("authoremail@example.com")
+ .create();
+ Account.Id committerId =
+ accountOperations
+ .newAccount()
+ .fullname("committer")
+ .preferredEmail("committeremail@example.com")
+ .create();
+
+ Change.Id changeId =
+ changeOperations.newChange().author(authorId).committer(committerId).create();
+ ChangeInfo changeInfo = gApi.changes().id(changeId.get()).get();
+ assertAuthor(changeInfo, "authoremail@example.com");
+
+ allowLabelPermission(
+ codeReview().getName(), RefNames.REFS_HEADS + "*", REGISTERED_USERS, -2, +2);
+
+ // Vote from author does not match
+ requestScopeOperations.setApiUser(authorId);
+ approve(changeId.toString());
+ assertNotMatching("label:Code-Review=+2,user=non_contributor", changeId);
+ }
+
+ @Test
+ public void nonContributorLabelVote_voteFromCommitter_doesNotMatch() throws Exception {
+ Account.Id authorId =
+ accountOperations
+ .newAccount()
+ .fullname("author")
+ .preferredEmail("authoremail@example.com")
+ .create();
+ Account.Id committerId =
+ accountOperations
+ .newAccount()
+ .fullname("committer")
+ .preferredEmail("committeremail@example.com")
+ .create();
+
+ Change.Id changeId =
+ changeOperations.newChange().author(authorId).committer(committerId).create();
+ ChangeInfo changeInfo = gApi.changes().id(changeId.get()).get();
+ assertCommitter(changeInfo, "committeremail@example.com");
+
+ allowLabelPermission(
+ codeReview().getName(), RefNames.REFS_HEADS + "*", REGISTERED_USERS, -2, +2);
+
+ // Vote from committer does not match
+ requestScopeOperations.setApiUser(committerId);
+ approve(changeId.toString());
+ assertNotMatching("label:Code-Review=+2,user=non_contributor", changeId);
+ }
+
+ @Test
+ public void nonContributorLabelVote_uploaderAndAuthorDifferent() throws Exception {
+ TestRepository<InMemoryRepository> clonedRepo = cloneProject(project, admin);
+ PushOneCommit.Result r1 =
+ pushFactory
+ .create(user.newIdent(), clonedRepo, "Subject", "file.txt", "text")
+ .to("refs/for/master");
+
+ requestScopeOperations.setApiUser(admin.id());
+ ChangeInfo changeInfo = gApi.changes().id(r1.getChangeId()).get();
+ assertUploader(changeInfo, admin.email());
+ assertAuthor(changeInfo, user.email());
+
+ allowLabelPermission(
+ codeReview().getName(), RefNames.REFS_HEADS + "*", REGISTERED_USERS, -2, +2);
+
+ // Vote from admin (a.k.a. uploader) does not match
+ requestScopeOperations.setApiUser(user.id());
+ approve(r1.getChangeId());
+ assertNotMatching("label:Code-Review=+2,user=non_contributor", r1.getChange().getId());
+
+ // Vote from user (a.k.a. author) does not match
+ requestScopeOperations.setApiUser(admin.id());
+ approve(r1.getChangeId());
+ assertNotMatching("label:Code-Review=+2,user=non_contributor", r1.getChange().getId());
+
+ // Vote from user2 (a.k.a. non-author and non-uploader) matches
+ TestAccount user2 = accountCreator.create();
+ requestScopeOperations.setApiUser(user2.id());
+ approve(r1.getChangeId());
+ assertMatching("label:Code-Review=+2,user=non_contributor", r1.getChange().getId());
+ }
+
+ private static void assertUploader(ChangeInfo changeInfo, String email) {
+ assertThat(changeInfo.revisions.get(changeInfo.currentRevision).uploader.email)
+ .isEqualTo(email);
+ }
+
+ private static void assertCommitter(ChangeInfo changeInfo, String email) {
+ assertThat(changeInfo.revisions.get(changeInfo.currentRevision).commit.committer.email)
+ .isEqualTo(email);
+ }
+
+ private static void assertAuthor(ChangeInfo changeInfo, String email) {
+ assertThat(changeInfo.revisions.get(changeInfo.currentRevision).commit.author.email)
+ .isEqualTo(email);
+ }
+
+ private void allowLabelPermission(
+ String labelName, String refPattern, AccountGroup.UUID group, int minVote, int maxVote) {
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(allowLabel(labelName).ref(refPattern).group(group).range(minVote, maxVote))
+ .update();
+ }
+
private PushOneCommit.Result createGitSubmoduleCommit(String ref) throws Exception {
return pushFactory
.create(admin.newIdent(), testRepo, "subject", ImmutableMap.of())
@@ -302,6 +468,13 @@
.to(ref);
}
+ private PushOneCommit.Result createNormalCommit(
+ PersonIdent personIdent, String ref, String fileName) throws Exception {
+ return pushFactory
+ .create(personIdent, testRepo, "subject", ImmutableMap.of(fileName, fileName))
+ .to(ref);
+ }
+
private PushOneCommit.Result createNormalCommit(String ref, String fileName) throws Exception {
return pushFactory
.create(admin.newIdent(), testRepo, "subject", ImmutableMap.of(fileName, fileName))
diff --git a/javatests/com/google/gerrit/acceptance/server/mail/MailSenderIT.java b/javatests/com/google/gerrit/acceptance/server/mail/MailSenderIT.java
index 45a471b..f728995 100644
--- a/javatests/com/google/gerrit/acceptance/server/mail/MailSenderIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/mail/MailSenderIT.java
@@ -15,16 +15,25 @@
package com.google.gerrit.acceptance.server.mail;
import static com.google.common.truth.Truth.assertThat;
+import static java.nio.charset.StandardCharsets.UTF_8;
+import com.google.gerrit.acceptance.Sandboxed;
+import com.google.gerrit.acceptance.UseLocalDisk;
import com.google.gerrit.acceptance.config.GerritConfig;
import com.google.gerrit.entities.EmailHeader;
import com.google.gerrit.entities.EmailHeader.StringEmailHeader;
+import com.google.gerrit.server.config.SitePaths;
import java.net.URI;
+import java.nio.file.Files;
import java.util.Map;
+import javax.inject.Inject;
import org.junit.Test;
+@UseLocalDisk
public class MailSenderIT extends AbstractMailIT {
+ @Inject private SitePaths sitePaths;
+
@Test
@GerritConfig(name = "sendemail.replyToAddress", value = "custom@gerritcodereview.com")
@GerritConfig(name = "receiveemail.protocol", value = "POP3")
@@ -63,6 +72,20 @@
assertThat(headerString(headers, "In-Reply-To")).isEqualTo(threadId);
}
+ @Test
+ @Sandboxed
+ public void useCustomTemplates() throws Exception {
+ String customTemplate =
+ "{namespace com.google.gerrit.server.mail.template.ChangeSubject}\n"
+ + "\n"
+ + "{template ChangeSubject kind=\"text\"}CUSTOM-TEMPLATE{/template}\n";
+ Files.write(sitePaths.mail_dir.resolve("ChangeSubject.soy"), customTemplate.getBytes(UTF_8));
+
+ createChangeWithReview(user);
+ String subject = headerString(sender.getMessages().iterator().next().headers(), "Subject");
+ assertThat(subject).isEqualTo("CUSTOM-TEMPLATE");
+ }
+
private String headerString(Map<String, EmailHeader> headers, String name) {
EmailHeader header = headers.get(name);
assertThat(header).isInstanceOf(StringEmailHeader.class);
diff --git a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
index 96a8dea..0533f3f 100644
--- a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
+++ b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
@@ -1435,6 +1435,15 @@
// "count" and "group" args cannot be used simultaneously.
assertThrows(
BadRequestException.class, () -> assertQuery("label:Code-Review=+1,group=gerrit,count=2"));
+
+ // "non_contributor arg for the label operator is not allowed in change queries
+ thrown =
+ assertThrows(
+ BadRequestException.class,
+ () -> assertQuery("label:Code-Review=+2,user=non_contributor"));
+ assertThat(thrown)
+ .hasMessageThat()
+ .isEqualTo("non_contributor arg is not allowed in change queries");
}
@Test
diff --git a/lib/bouncycastle/BUILD b/lib/bouncycastle/BUILD
index 43ba6e1..6a87d73 100644
--- a/lib/bouncycastle/BUILD
+++ b/lib/bouncycastle/BUILD
@@ -22,6 +22,13 @@
)
java_library(
+ name = "bcutil",
+ data = ["//lib:LICENSE-bouncycastle"],
+ visibility = ["//visibility:public"],
+ exports = ["@bcutil//jar"],
+)
+
+java_library(
name = "bcprov-neverlink",
data = ["//lib:LICENSE-bouncycastle"],
neverlink = 1,
@@ -44,3 +51,11 @@
visibility = ["//visibility:public"],
exports = ["@bcpkix//jar"],
)
+
+java_library(
+ name = "bcutil-neverlink",
+ data = ["//lib:LICENSE-bouncycastle"],
+ neverlink = 1,
+ visibility = ["//visibility:public"],
+ exports = ["@bcutil//jar"],
+)
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 72a6a3a..721d650 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
@@ -115,7 +115,6 @@
import {GrFileList} from '../gr-file-list/gr-file-list';
import {EditRevisionInfo, ParsedChangeInfo} from '../../../types/types';
import {
- CloseFixPreviewEvent,
EditableContentSaveEvent,
EventType,
OpenFixPreviewEvent,
@@ -590,8 +589,9 @@
this.addEventListener('editable-content-cancel', () =>
this.handleCommitMessageCancel()
);
- this.addEventListener('open-fix-preview', e => this.onOpenFixPreview(e));
- this.addEventListener('close-fix-preview', e => this.onCloseFixPreview(e));
+ this.addEventListener(EventType.OPEN_FIX_PREVIEW, e =>
+ this.onOpenFixPreview(e)
+ );
this.addEventListener(EventType.SHOW_TAB, e => this.setActiveTab(e));
this.addEventListener('reload', e => {
@@ -1675,10 +1675,6 @@
this.applyFixDialog.open(e);
}
- private onCloseFixPreview(e: CloseFixPreviewEvent) {
- if (e.detail.fixApplied) fireReload(this);
- }
-
// Private but used in tests.
handleToggleDiffMode() {
if (this.diffViewMode === DiffViewMode.SIDE_BY_SIDE) {
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 d4defcb..6eaf7ae 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
@@ -258,10 +258,6 @@
// Private but used in tests.
@state()
- displayLine?: boolean;
-
- // Private but used in tests.
- @state()
showSizeBars = true;
// For merge commits vs Auto Merge, an extra file row is shown detailing the
@@ -733,9 +729,6 @@
this.shortcutsController.addAbstract(Shortcut.TOGGLE_LEFT_PANE, _ =>
this.handleToggleLeftPane()
);
- this.shortcutsController.addGlobal({key: Key.ESC}, _ =>
- this.handleEscKey()
- );
this.shortcutsController.addAbstract(
Shortcut.EXPAND_ALL_COMMENT_THREADS,
_ => {}
@@ -1079,7 +1072,6 @@
<gr-diff-host
?noAutoRender=${true}
?showLoadFailure=${true}
- .displayLine=${this.displayLine}
.changeNum=${this.changeNum}
.change=${this.change}
.patchRange=${this.patchRange}
@@ -2032,7 +2024,6 @@
e.stopPropagation();
if (this.filesExpanded === FilesExpandedState.ALL) {
this.diffCursor?.moveDown();
- this.displayLine = true;
} else {
this.fileCursor.next({circular: true});
this.selectedIndex = this.fileCursor.index;
@@ -2052,7 +2043,6 @@
e.stopPropagation();
if (this.filesExpanded === FilesExpandedState.ALL) {
this.diffCursor?.moveUp();
- this.displayLine = true;
} else {
this.fileCursor.previous({circular: true});
this.selectedIndex = this.fileCursor.index;
@@ -2504,11 +2494,6 @@
return undefined;
}
- // Private but used in tests.
- handleEscKey() {
- this.displayLine = false;
- }
-
/**
* Compute size bar layout values from the file list.
* Private but used in tests.
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.ts b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.ts
index 247b7c3..c79be96 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.ts
@@ -2299,22 +2299,6 @@
assert.isTrue(setUrlStub.calledOnce);
});
- test('displayLine', () => {
- element.filesExpanded = FilesExpandedState.ALL;
-
- element.displayLine = false;
- element.handleCursorNext(new KeyboardEvent('keydown'));
- assert.isTrue(element.displayLine);
-
- element.displayLine = false;
- element.handleCursorPrev(new KeyboardEvent('keydown'));
- assert.isTrue(element.displayLine);
-
- element.displayLine = true;
- element.handleEscKey();
- assert.isFalse(element.displayLine);
- });
-
suite('editMode behavior', () => {
test('reviewed checkbox', async () => {
reviewFileStub.restore();
diff --git a/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog.ts b/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog.ts
index b146e93..1608c22 100644
--- a/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog.ts
+++ b/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog.ts
@@ -20,7 +20,6 @@
import {PROVIDED_FIX_ID} from '../../../utils/comment-util';
import {OpenFixPreviewEvent} from '../../../types/events';
import {getAppContext} from '../../../services/app-context';
-import {fireCloseFixPreview} from '../../../utils/event-util';
import {DiffLayer, ParsedChangeInfo} from '../../../types/types';
import {GrButton} from '../../shared/gr-button/gr-button';
import {TokenHighlightLayer} from '../../../embed/diff/gr-diff-builder/token-highlight-layer';
@@ -37,6 +36,8 @@
import {GrSyntaxLayerWorker} from '../../../embed/diff/gr-syntax-layer/gr-syntax-layer-worker';
import {highlightServiceToken} from '../../../services/highlight/highlight-service';
import {anyLineTooLong} from '../../../embed/diff/gr-diff/gr-diff-utils';
+import {changeModelToken} from '../../../models/change/change-model';
+import {fireReload} from '../../../utils/event-util';
interface FilePreview {
filepath: string;
@@ -90,12 +91,20 @@
@state()
diffPrefs?: DiffPreferencesInfo;
+ @state()
+ isOwner = false;
+
+ @state()
+ onCloseFixPreviewCallbacks: ((fixapplied: boolean) => void)[] = [];
+
private readonly restApiService = getAppContext().restApiService;
private readonly getUserModel = resolve(this, userModelToken);
private readonly getNavigation = resolve(this, navigationToken);
+ private readonly getChangeModel = resolve(this, changeModelToken);
+
private readonly syntaxLayer = new GrSyntaxLayerWorker(
resolve(this, highlightServiceToken),
() => getAppContext().reportingService
@@ -105,6 +114,11 @@
super();
subscribe(
this,
+ () => this.getChangeModel().isOwner$,
+ x => (this.isOwner = x)
+ );
+ subscribe(
+ this,
() => this.getUserModel().preferences$,
preferences => {
const layers: DiffLayer[] = [this.syntaxLayer];
@@ -234,6 +248,7 @@
open(e: OpenFixPreviewEvent) {
this.patchNum = e.detail.patchNum;
this.fixSuggestions = e.detail.fixSuggestions;
+ this.onCloseFixPreviewCallbacks = e.detail.onCloseFixPreviewCallbacks;
assert(this.fixSuggestions.length > 0, 'no fix in the event');
this.selectedFixIdx = 0;
this.applyFixModal?.showModal();
@@ -319,12 +334,14 @@
this.currentPreviews = [];
this.isApplyFixLoading = false;
- fireCloseFixPreview(this, fixApplied);
+ this.onCloseFixPreviewCallbacks.forEach(fn => fn(fixApplied));
this.applyFixModal?.close();
+ if (fixApplied) fireReload(this);
}
private computeTooltip() {
if (!this.change || !this.patchNum) return '';
+ if (!this.isOwner) return 'Fix can only be applied by author';
const latestPatchNum =
this.change.revisions[this.change.current_revision]._number;
return latestPatchNum !== this.patchNum
@@ -334,6 +351,7 @@
private computeDisableApplyFixButton() {
if (!this.change || !this.patchNum) return true;
+ if (!this.isOwner) return true;
const latestPatchNum =
this.change.revisions[this.change.current_revision]._number;
return this.patchNum !== latestPatchNum || this.isApplyFixLoading;
diff --git a/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog_test.ts b/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog_test.ts
index 24dadf7..9284fb2 100644
--- a/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog_test.ts
+++ b/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog_test.ts
@@ -17,11 +17,7 @@
} from '../../../test/test-data-generators';
import {createDefaultDiffPrefs} from '../../../constants/constants';
import {DiffInfo} from '../../../types/diff';
-import {
- CloseFixPreviewEventDetail,
- EventType,
- OpenFixPreviewEventDetail,
-} from '../../../types/events';
+import {EventType, OpenFixPreviewEventDetail} from '../../../types/events';
import {GrButton} from '../../shared/gr-button/gr-button';
import {fixture, html, assert} from '@open-wc/testing';
import {SinonStub} from 'sinon';
@@ -37,11 +33,13 @@
createFixSuggestionInfo('fix_1'),
createFixSuggestionInfo('fix_2'),
],
+ onCloseFixPreviewCallbacks: [],
};
const ONE_FIX: OpenFixPreviewEventDetail = {
patchNum: 2 as PatchSetNum,
fixSuggestions: [createFixSuggestionInfo('fix_1')],
+ onCloseFixPreviewCallbacks: [],
};
function getConfirmButton(): GrButton {
@@ -73,6 +71,7 @@
element.changeNum = change._number;
element.patchNum = change.revisions[change.current_revision]._number;
element.change = change;
+ element.isOwner = true;
element.diffPrefs = {
...createDefaultDiffPrefs(),
font_size: 12,
@@ -162,8 +161,22 @@
assert.equal(button.getAttribute('title'), '');
});
+ test('apply fix button is disabled for non-author', async () => {
+ element.isOwner = false;
+ await element.updateComplete;
+ await open(TWO_FIXES);
+ assert.equal(element.currentFix!.fix_id, 'fix_1');
+ assert.equal(element.currentPreviews.length, 2);
+ const button = getConfirmButton();
+ assert.isTrue(button.hasAttribute('disabled'));
+ assert.equal(
+ button.getAttribute('title'),
+ 'Fix can only be applied by author'
+ );
+ });
+
test('apply fix button is disabled on older patchset', async () => {
- element.change = element.change = {
+ element.change = {
...createParsedChange(),
revisions: createRevisions(2),
current_revision: getCurrentRevision(0),
@@ -246,11 +259,7 @@
element.currentFix = createFixSuggestionInfo('123');
const closeFixPreviewEventSpy = sinon.spy();
- // Element is recreated after each test, removeEventListener isn't required
- element.addEventListener(
- EventType.CLOSE_FIX_PREVIEW,
- closeFixPreviewEventSpy
- );
+ element.onCloseFixPreviewCallbacks.push(closeFixPreviewEventSpy);
await element.handleApplyFix(new CustomEvent('confirm'));
@@ -263,14 +272,7 @@
assert.isTrue(setUrlStub.called);
assert.equal(setUrlStub.lastCall.firstArg, '/c/test-project/+/42/2..edit');
- sinon.assert.calledOnceWithExactly(
- closeFixPreviewEventSpy,
- new CustomEvent<CloseFixPreviewEventDetail>(EventType.CLOSE_FIX_PREVIEW, {
- detail: {
- fixApplied: true,
- },
- })
- );
+ sinon.assert.calledOnceWithExactly(closeFixPreviewEventSpy, true);
// reset gr-apply-fix-dialog and close
assert.equal(element.currentFix, undefined);
assert.equal(element.currentPreviews.length, 0);
@@ -311,11 +313,7 @@
element.currentFix = createFixSuggestionInfo('fix_123');
const closeFixPreviewEventSpy = sinon.spy();
- // Element is recreated after each test, removeEventListener isn't required
- element.addEventListener(
- EventType.CLOSE_FIX_PREVIEW,
- closeFixPreviewEventSpy
- );
+ element.onCloseFixPreviewCallbacks.push(closeFixPreviewEventSpy);
let expectedError;
await element.handleApplyFix(new CustomEvent('click')).catch(e => {
@@ -328,19 +326,8 @@
test('onCancel fires close with correct parameters', () => {
const closeFixPreviewEventSpy = sinon.spy();
- // Element is recreated after each test, removeEventListener isn't required
- element.addEventListener(
- EventType.CLOSE_FIX_PREVIEW,
- closeFixPreviewEventSpy
- );
+ element.onCloseFixPreviewCallbacks.push(closeFixPreviewEventSpy);
element.onCancel(new CustomEvent('cancel'));
- sinon.assert.calledOnceWithExactly(
- closeFixPreviewEventSpy,
- new CustomEvent<CloseFixPreviewEventDetail>(EventType.CLOSE_FIX_PREVIEW, {
- detail: {
- fixApplied: false,
- },
- })
- );
+ sinon.assert.calledOnceWithExactly(closeFixPreviewEventSpy, false);
});
});
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts
index 23dce97..fd0e5d2 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts
@@ -171,9 +171,6 @@
@property({type: String})
projectName?: RepoName;
- @property({type: Boolean})
- displayLine = false;
-
@state()
private _isImageDiff = false;
@@ -522,7 +519,6 @@
.noAutoRender=${this.noAutoRender}
.path=${this.path}
.prefs=${this.prefs}
- .displayLine=${this.displayLine}
.isImageDiff=${this.isImageDiff}
.noRenderOnPrefsChange=${this.noRenderOnPrefsChange}
.renderPrefs=${this.renderPrefs}
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.ts b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.ts
index be03afd..7a32c27 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.ts
@@ -774,14 +774,6 @@
assert.equal(element.diffElement.prefs, value);
});
- test('passes in displayLine', async () => {
- const value = true;
- element.displayLine = value;
- await element.updateComplete;
- assertIsDefined(element.diffElement);
- assert.equal(element.diffElement.displayLine, value);
- });
-
test('passes in hidden', async () => {
const value = true;
element.hidden = value;
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 c7d6948..19186ec 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
@@ -56,7 +56,7 @@
import {OpenFixPreviewEvent, ValueChangedEvent} from '../../../types/events';
import {fireAlert, fireEvent, fireTitleChange} from '../../../utils/event-util';
import {assertIsDefined, queryAndAssert} from '../../../utils/common-util';
-import {Key, toggleClass, whenVisible} from '../../../utils/dom-util';
+import {toggleClass, whenVisible} from '../../../utils/dom-util';
import {CursorMoveResult} from '../../../api/core';
import {throttleWrap} from '../../../utils/async-util';
import {filter, take, switchMap} from 'rxjs/operators';
@@ -344,10 +344,6 @@
);
listen(Shortcut.EXPAND_ALL_COMMENT_THREADS, _ => {}); // docOnly
listen(Shortcut.COLLAPSE_ALL_COMMENT_THREADS, _ => {}); // docOnly
- this.shortcutsController.addGlobal({key: Key.ESC}, _ => {
- assertIsDefined(this.diffHost, 'diffHost');
- this.diffHost.displayLine = false;
- });
}
private setupSubscriptions() {
@@ -1067,7 +1063,6 @@
private handlePrevLine() {
assertIsDefined(this.diffHost, 'diffHost');
- this.diffHost.displayLine = true;
this.cursor?.moveUp();
}
@@ -1102,7 +1097,6 @@
private handleNextLine() {
assertIsDefined(this.diffHost, 'diffHost');
- this.diffHost.displayLine = true;
this.cursor?.moveDown();
}
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.ts b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.ts
index 889e9dd..6507046 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.ts
@@ -61,7 +61,6 @@
import {GrDiffModeSelector} from '../../../embed/diff/gr-diff-mode-selector/gr-diff-mode-selector';
import {fixture, html, assert} from '@open-wc/testing';
import {EventType} from '../../../types/events';
-import {Key} from '../../../utils/dom-util';
import {GrButton} from '../../shared/gr-button/gr-button';
import {testResolver} from '../../../test/common-test-setup';
import {UserModel, userModelToken} from '../../../models/user/user-model';
@@ -459,15 +458,6 @@
element.diffHost.diffElement.viewMode,
DiffViewMode.SIDE_BY_SIDE
);
- assert.isTrue(element.diffHost.diffElement.displayLine);
-
- pressKey(element, Key.ESC);
- await element.updateComplete;
- assert.equal(
- element.diffHost.diffElement.viewMode,
- DiffViewMode.SIDE_BY_SIDE
- );
- assert.isFalse(element.diffHost.diffElement.displayLine);
const setReviewedStub = sinon.stub(element, 'setReviewed');
const handleToggleSpy = sinon.spy(element, 'handleToggleFileReviewed');
diff --git a/polygerrit-ui/app/elements/gr-app-global-var-init.ts b/polygerrit-ui/app/elements/gr-app-global-var-init.ts
index da9c0c9..d66fec0 100644
--- a/polygerrit-ui/app/elements/gr-app-global-var-init.ts
+++ b/polygerrit-ui/app/elements/gr-app-global-var-init.ts
@@ -14,11 +14,27 @@
import {GrAnnotation} from '../embed/diff/gr-diff-highlight/gr-annotation';
import {GrPluginActionContext} from './shared/gr-js-api-interface/gr-plugin-action-context';
import {AppContext, injectAppContext} from '../services/app-context';
-import {Finalizable} from '../services/registry';
import {PluginLoader} from './shared/gr-js-api-interface/gr-plugin-loader';
+import {
+ initVisibilityReporter,
+ initPerformanceReporter,
+ initErrorReporter,
+ initWebVitals,
+} from '../services/gr-reporting/gr-reporting_impl';
+import {Finalizable} from '../services/registry';
-export function initGlobalVariables(appContext: AppContext & Finalizable) {
+export function initGlobalVariables(
+ appContext: AppContext & Finalizable,
+ initializeReporting: boolean
+) {
injectAppContext(appContext);
+ if (initializeReporting) {
+ const reportingService = appContext.reportingService;
+ initVisibilityReporter(reportingService);
+ initPerformanceReporter(reportingService);
+ initWebVitals(reportingService);
+ initErrorReporter(reportingService);
+ }
window.GrAnnotation = GrAnnotation;
window.GrPluginActionContext = GrPluginActionContext;
}
diff --git a/polygerrit-ui/app/elements/gr-app.ts b/polygerrit-ui/app/elements/gr-app.ts
index 645f94a..ded0626 100644
--- a/polygerrit-ui/app/elements/gr-app.ts
+++ b/polygerrit-ui/app/elements/gr-app.ts
@@ -37,12 +37,6 @@
createAppDependencies,
Creator,
} from '../services/app-context-init';
-import {
- initVisibilityReporter,
- initPerformanceReporter,
- initErrorReporter,
- initWebVitals,
-} from '../services/gr-reporting/gr-reporting_impl';
import {html, LitElement} from 'lit';
import {customElement} from 'lit/decorators.js';
import {
@@ -50,14 +44,9 @@
serviceWorkerInstallerToken,
} from '../services/service-worker-installer';
import {pluginLoaderToken} from './shared/gr-js-api-interface/gr-plugin-loader';
+import {getAppContext} from '../services/app-context';
-const appContext = createAppContext();
-initGlobalVariables(appContext);
-const reportingService = appContext.reportingService;
-initVisibilityReporter(reportingService);
-initPerformanceReporter(reportingService);
-initWebVitals(reportingService);
-initErrorReporter(reportingService);
+initGlobalVariables(createAppContext(), true);
installPolymerResin(safeTypesBridge);
@@ -97,7 +86,7 @@
};
for (const [token, creator] of createAppDependencies(
- appContext,
+ getAppContext(),
resolver
)) {
injectDependency(token, creator);
diff --git a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
index 1ac89d6..c844d42 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
@@ -1053,6 +1053,11 @@
replacement
),
patchNum: this.comment.patch_set,
+ onCloseFixPreviewCallbacks: [
+ fixApplied => {
+ if (fixApplied) this.handleAppliedFix();
+ },
+ ],
};
}
if (isRobot(this.comment) && this.comment.fix_suggestions.length > 0) {
@@ -1065,6 +1070,7 @@
};
}),
patchNum: this.comment.patch_set,
+ onCloseFixPreviewCallbacks: [],
};
}
throw new Error('unable to create preview fix event');
@@ -1132,6 +1138,18 @@
fire(this, 'reply-to-comment', eventDetail);
}
+ private handleAppliedFix() {
+ const message = this.comment?.message;
+ assert(!!message, 'empty message');
+ const eventDetail: ReplyToCommentEventDetail = {
+ content: 'Fix applied.',
+ userWantsToEdit: false,
+ unresolved: false,
+ };
+ // Handled by <gr-comment-thread>.
+ fire(this, 'reply-to-comment', eventDetail);
+ }
+
private async handleShowFix() {
// Handled top-level in the diff and change view components.
fire(this, 'open-fix-preview', await this.createFixPreview());
diff --git a/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text_test.ts b/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text_test.ts
index 3187ada..fcebeea 100644
--- a/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text_test.ts
@@ -69,7 +69,7 @@
});
test('does not apply rewrites within links', async () => {
- element.content = 'google.com/LinkRewriteMe';
+ element.content = 'http://google.com/LinkRewriteMe';
await element.updateComplete;
assert.shadowDom.equal(
@@ -81,7 +81,7 @@
rel="noopener"
target="_blank"
>
- google.com/LinkRewriteMe
+ http://google.com/LinkRewriteMe
</a>
</pre>
`
@@ -149,20 +149,25 @@
});
test('renders text with links and rewrites', async () => {
- element.content = `text with plain link: google.com
- \ntext with config link: LinkRewriteMe
- \ntext with complex link: A Link 12`;
+ element.content = `
+ text with plain link: http://google.com
+ text with config link: LinkRewriteMe
+ text with complex link: A Link 12`;
await element.updateComplete;
assert.shadowDom.equal(
element,
/* HTML */ `
<pre class="plaintext">
- text with plain link:
- <a href="http://google.com" rel="noopener" target="_blank">
- google.com
- </a>
- text with config link:
+ text with plain link:
+ <a
+ href="http://google.com"
+ rel="noopener"
+ target="_blank"
+ >
+ http://google.com
+ </a>
+ text with config link:
<a
href="http://google.com/LinkRewriteMe"
rel="noopener"
@@ -212,7 +217,7 @@
});
test('renders text with links and rewrites', async () => {
element.content = `text
- \ntext with plain link: google.com
+ \ntext with plain link: http://google.com
\ntext with config link: LinkRewriteMe
\ntext without a link: NotA Link 15 cats
\ntext with complex link: A Link 12`;
@@ -227,7 +232,7 @@
<p>
text with plain link:
<a href="http://google.com" rel="noopener" target="_blank">
- google.com
+ http://google.com
</a>
</p>
<p>
@@ -259,7 +264,7 @@
test('does not render if too long', async () => {
element.content = `text
- text with plain link: google.com
+ text with plain link: http://google.com
text with config link: LinkRewriteMe
text without a link: NotA Link 15 cats
text with complex link: A Link 12`;
@@ -272,8 +277,14 @@
<pre class="plaintext">
text
text with plain link:
- <a href="http://google.com" rel="noopener" target="_blank">google.com</a>
- text with config link:
+ <a
+ href="http://google.com"
+ rel="noopener"
+ target="_blank"
+ >
+ http://google.com
+ </a>
+ text with config link:
<a
href="http://google.com/LinkRewriteMe"
rel="noopener"
@@ -302,7 +313,7 @@
\n#### h4-heading
\n##### h5-heading
\n###### h6-heading
- \n# heading with plain link: google.com
+ \n# heading with plain link: http://google.com
\n# heading with config link: LinkRewriteMe`;
await element.updateComplete;
@@ -320,7 +331,7 @@
<h1>
heading with plain link:
<a href="http://google.com" rel="noopener" target="_blank">
- google.com
+ http://google.com
</a>
</h1>
<h1>
@@ -480,7 +491,7 @@
test('renders block quotes with links and rewrites', async () => {
element.content = `> block quote
- \n> block quote with plain link: google.com
+ \n> block quote with plain link: http://google.com
\n> block quote with config link: LinkRewriteMe`;
await element.updateComplete;
@@ -496,7 +507,7 @@
<p>
block quote with plain link:
<a href="http://google.com" rel="noopener" target="_blank">
- google.com
+ http://google.com
</a>
</p>
</blockquote>
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-section.ts b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-section.ts
index b952a3d..d40fdda 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-section.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-section.ts
@@ -26,6 +26,7 @@
import '../gr-context-controls/gr-context-controls';
import '../gr-range-header/gr-range-header';
import './gr-diff-row';
+import {when} from 'lit/directives/when.js';
@customElement('gr-diff-section')
export class GrDiffSection extends LitElement {
@@ -170,7 +171,7 @@
`;
const moveCell = html`
<td class=${diffClasses('moveHeader')}>
- <gr-range-header class=${diffClasses()} icon="gr-icons:move-item">
+ <gr-range-header class=${diffClasses()} icon="move_item">
${this.renderMoveDescription(movedIn)}
</gr-range-header>
</td>
@@ -179,8 +180,13 @@
<tr
class=${diffClasses('moveControls', movedIn ? 'movedIn' : 'movedOut')}
>
- ${lineNumberCell} ${signCell} ${movedIn ? plainCell : moveCell}
- ${lineNumberCell} ${signCell} ${movedIn ? moveCell : plainCell}
+ ${when(
+ this.isUnifiedDiff(),
+ () => html`${lineNumberCell} ${lineNumberCell} ${moveCell}`,
+ () => html`${lineNumberCell} ${signCell}
+ ${movedIn ? plainCell : moveCell} ${lineNumberCell} ${signCell}
+ ${movedIn ? moveCell : plainCell}`
+ )}
</tr>
`;
}
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-section_test.ts b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-section_test.ts
index 33b3df0..14f4536 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-section_test.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-section_test.ts
@@ -9,7 +9,8 @@
import {fixture, html, assert} from '@open-wc/testing';
import {GrDiffGroup, GrDiffGroupType} from '../gr-diff/gr-diff-group';
import {GrDiffLine} from '../gr-diff/gr-diff-line';
-import {GrDiffLineType} from '../../../api/diff';
+import {DiffViewMode, GrDiffLineType} from '../../../api/diff';
+import {waitQueryAndAssert} from '../../../test/test-utils';
suite('gr-diff-section test', () => {
let element: GrDiffSection;
@@ -22,6 +23,93 @@
await element.updateComplete;
});
+ suite('move controls', async () => {
+ setup(async () => {
+ const lines = [new GrDiffLine(GrDiffLineType.BOTH, 1, 1)];
+ lines[0].text = 'asdf';
+ const group = new GrDiffGroup({
+ type: GrDiffGroupType.BOTH,
+ lines,
+ moveDetails: {changed: false, range: {start: 1, end: 2}},
+ });
+ element.group = group;
+ await element.updateComplete;
+ });
+
+ test('side-by-side', async () => {
+ const row = await waitQueryAndAssert(element, 'tr.moveControls');
+ // Semantic dom diff has a problem with just comparing table rows or
+ // cells directly. So as a workaround put the row into an empty test
+ // table.
+ const testTable = document.createElement('table');
+ testTable.appendChild(row);
+ assert.dom.equal(
+ testTable,
+ /* HTML */ `
+ <table>
+ <tbody>
+ <tr class="gr-diff moveControls movedOut">
+ <td class="gr-diff moveControlsLineNumCol"></td>
+ <td class="gr-diff sign"></td>
+ <td class="gr-diff moveHeader">
+ <gr-range-header class="gr-diff" icon="move_item">
+ <div class="gr-diff">
+ <span class="gr-diff"> Moved to lines </span>
+ <a class="gr-diff" href="#1"> 1 </a>
+ <span class="gr-diff"> - </span>
+ <a class="gr-diff" href="#2"> 2 </a>
+ </div>
+ </gr-range-header>
+ </td>
+ <td class="gr-diff moveControlsLineNumCol"></td>
+ <td class="gr-diff sign"></td>
+ <td class="gr-diff"></td>
+ </tr>
+ </tbody>
+ </table>
+ `,
+ {}
+ );
+ });
+
+ test('unified', async () => {
+ element.renderPrefs = {
+ ...element.renderPrefs,
+ view_mode: DiffViewMode.UNIFIED,
+ };
+ const row = await waitQueryAndAssert(element, 'tr.moveControls');
+ // Semantic dom diff has a problem with just comparing table rows or
+ // cells directly. So as a workaround put the row into an empty test
+ // table.
+ const testTable = document.createElement('table');
+ testTable.appendChild(row);
+ assert.dom.equal(
+ testTable,
+ /* HTML */ `
+ <table>
+ <tbody>
+ <tr class="gr-diff moveControls movedOut">
+ <td class="gr-diff moveControlsLineNumCol"></td>
+ <td class="gr-diff moveControlsLineNumCol"></td>
+ <td class="gr-diff moveHeader">
+ <gr-range-header class="gr-diff" icon="move_item">
+ <div class="gr-diff">
+ <span class="gr-diff"> Moved to lines </span>
+ <a class="gr-diff" href="#1"> 1 </a>
+ <span class="gr-diff"> - </span>
+ <a class="gr-diff" href="#2"> 2 </a>
+ </div>
+ </gr-range-header>
+ </td>
+ </tr>
+ </tbody>
+ </table>
+ `,
+ {}
+ );
+ });
+ });
+
test('3 normal unchanged rows', async () => {
const lines = [
new GrDiffLine(GrDiffLineType.BOTH, 1, 1),
diff --git a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff.ts b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff.ts
index a0526ed..54bc46d 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff.ts
@@ -141,9 +141,6 @@
renderPrefs?: RenderPreferences;
@property({type: Boolean})
- displayLine = false;
-
- @property({type: Boolean})
isImageDiff?: boolean;
@property({type: Boolean, reflect: true})
@@ -1081,7 +1078,6 @@
unified: this.viewMode === DiffViewMode.UNIFIED,
sideBySide: this.viewMode === DiffViewMode.SIDE_BY_SIDE,
canComment: this.loggedIn,
- displayLine: this.displayLine,
};
return html`
<div class=${classMap(cssClasses)} @click=${this.handleTap}>
diff --git a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff_test.ts b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff_test.ts
index ce1393d..2855ae6 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff_test.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff_test.ts
@@ -3141,18 +3141,6 @@
assert.isFalse(element.classList.contains('no-left'));
});
- test('view does not start with displayLine classList', () => {
- const container = queryAndAssert(element, '.diffContainer');
- assert.isFalse(container.classList.contains('displayLine'));
- });
-
- test('displayLine class added when displayLine is true', async () => {
- element.displayLine = true;
- await element.updateComplete;
- const container = queryAndAssert(element, '.diffContainer');
- assert.isTrue(container.classList.contains('displayLine'));
- });
-
suite('binary diffs', () => {
test('render binary diff', async () => {
element.prefs = {
diff --git a/polygerrit-ui/app/embed/diff/gr-range-header/gr-range-header_test.ts b/polygerrit-ui/app/embed/diff/gr-range-header/gr-range-header_test.ts
new file mode 100644
index 0000000..b31197d16
--- /dev/null
+++ b/polygerrit-ui/app/embed/diff/gr-range-header/gr-range-header_test.ts
@@ -0,0 +1,40 @@
+/**
+ * @license
+ * Copyright 2023 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+import '../../../test/common-test-setup';
+import './gr-range-header';
+import {GrRangeHeader} from './gr-range-header';
+import {fixture, html, assert} from '@open-wc/testing';
+
+suite('gr-range-header test', () => {
+ let element: GrRangeHeader;
+
+ setup(async () => {
+ element = await fixture<GrRangeHeader>(
+ html`<gr-range-header></gr-range-header>`
+ );
+ await element.updateComplete;
+ });
+
+ test('renders', async () => {
+ element.filled = true;
+ element.icon = 'test-icon';
+ await element.updateComplete;
+ assert.shadowDom.equal(
+ element,
+ /* HTML */ `
+ <div class="row">
+ <gr-icon
+ aria-hidden="true"
+ class="icon"
+ filled
+ icon="test-icon"
+ ></gr-icon>
+ <slot></slot>
+ </div>
+ `
+ );
+ });
+});
diff --git a/polygerrit-ui/app/models/checks/checks-util.ts b/polygerrit-ui/app/models/checks/checks-util.ts
index ba43eb4..da05bbe 100644
--- a/polygerrit-ui/app/models/checks/checks-util.ts
+++ b/polygerrit-ui/app/models/checks/checks-util.ts
@@ -121,6 +121,7 @@
const eventDetail: OpenFixPreviewEventDetail = {
patchNum: result.patchset as PatchSetNumber,
fixSuggestions,
+ onCloseFixPreviewCallbacks: [],
};
return {
name: 'Show Fix',
diff --git a/polygerrit-ui/app/node_modules_licenses/licenses.ts b/polygerrit-ui/app/node_modules_licenses/licenses.ts
index c65e7a31..155dd6c 100644
--- a/polygerrit-ui/app/node_modules_licenses/licenses.ts
+++ b/polygerrit-ui/app/node_modules_licenses/licenses.ts
@@ -329,14 +329,6 @@
license: SharedLicenses.Polymer2018,
},
{
- name: 'ba-linkify',
- license: {
- name: 'ba-linkify',
- type: LicenseTypes.Mit,
- packageLicenseFile: 'LICENSE-MIT',
- },
- },
- {
name: 'codemirror-minified',
license: {
name: 'codemirror-minified',
diff --git a/polygerrit-ui/app/package.json b/polygerrit-ui/app/package.json
index 0df1eda..41f4043 100644
--- a/polygerrit-ui/app/package.json
+++ b/polygerrit-ui/app/package.json
@@ -34,7 +34,6 @@
"@types/resize-observer-browser": "^0.1.5",
"@webcomponents/shadycss": "^1.10.2",
"@webcomponents/webcomponentsjs": "^1.3.3",
- "ba-linkify": "^1.0.1",
"codemirror-minified": "^5.65.0",
"highlight.js": "^11.5.0",
"highlightjs-closure-templates": "https://github.com/highlightjs/highlightjs-closure-templates",
@@ -50,4 +49,4 @@
},
"license": "Apache-2.0",
"private": true
-}
\ No newline at end of file
+}
diff --git a/polygerrit-ui/app/test/common-test-setup.ts b/polygerrit-ui/app/test/common-test-setup.ts
index aed58d8..365bb16 100644
--- a/polygerrit-ui/app/test/common-test-setup.ts
+++ b/polygerrit-ui/app/test/common-test-setup.ts
@@ -6,7 +6,7 @@
// TODO(dmfilippov): remove bundled-polymer.js imports when the following issue
// https://github.com/Polymer/polymer-resin/issues/9 is resolved.
import '../scripts/bundled-polymer';
-import {AppContext} from '../services/app-context';
+import {getAppContext} from '../services/app-context';
import {Finalizable} from '../services/registry';
import {
createTestAppContext,
@@ -60,7 +60,6 @@
});
let testSetupTimestampMs = 0;
-let appContext: AppContext & Finalizable;
const injectedDependencies: Map<
DependencyToken<unknown>,
@@ -101,11 +100,10 @@
// If the following asserts fails - then window.stub is
// overwritten by some other code.
assert.equal(getCleanupsCount(), 0);
- appContext = createTestAppContext();
- initGlobalVariables(appContext);
+ initGlobalVariables(createTestAppContext(), false);
- finalizers.push(appContext);
- const dependencies = createTestDependencies(appContext, testResolver);
+ finalizers.push(getAppContext());
+ const dependencies = createTestDependencies(getAppContext(), testResolver);
for (const [token, provider] of dependencies) {
injectDependency(token, provider);
}
@@ -124,7 +122,7 @@
// `awaitPluginsLoaded` will rely on that to kick off,
// in testing, we want to kick start this earlier.
testResolver(pluginLoaderToken).loadPlugins([]);
- testOnlyResetGrRestApiSharedObjects(appContext.authService);
+ testOnlyResetGrRestApiSharedObjects(getAppContext().authService);
});
export function removeRequestDependencyListener() {
diff --git a/polygerrit-ui/app/types/events.ts b/polygerrit-ui/app/types/events.ts
index 2cc6742..e2612b0 100644
--- a/polygerrit-ui/app/types/events.ts
+++ b/polygerrit-ui/app/types/events.ts
@@ -26,7 +26,6 @@
MOVED_LINK_CLICKED = 'moved-link-clicked',
NETWORK_ERROR = 'network-error',
OPEN_FIX_PREVIEW = 'open-fix-preview',
- CLOSE_FIX_PREVIEW = 'close-fix-preview',
PAGE_ERROR = 'page-error',
RELOAD = 'reload',
REPLY = 'reply',
@@ -63,7 +62,6 @@
'line-cursor-moved-out': LineNumberEvent;
'moved-link-clicked': MovedLinkClickedEvent;
'open-fix-preview': OpenFixPreviewEvent;
- 'close-fix-preview': CloseFixPreviewEvent;
'reply-to-comment': ReplyToCommentEvent;
/* prettier-ignore */
'reload': ReloadEvent;
@@ -156,13 +154,10 @@
export interface OpenFixPreviewEventDetail {
patchNum: PatchSetNum;
fixSuggestions: FixSuggestionInfo[];
+ onCloseFixPreviewCallbacks: ((fixapplied: boolean) => void)[];
}
export type OpenFixPreviewEvent = CustomEvent<OpenFixPreviewEventDetail>;
-export interface CloseFixPreviewEventDetail {
- fixApplied: boolean;
-}
-export type CloseFixPreviewEvent = CustomEvent<CloseFixPreviewEventDetail>;
export interface ReplyToCommentEventDetail {
content: string;
userWantsToEdit: boolean;
diff --git a/polygerrit-ui/app/utils/event-util.ts b/polygerrit-ui/app/utils/event-util.ts
index 714955b..49d5382 100644
--- a/polygerrit-ui/app/utils/event-util.ts
+++ b/polygerrit-ui/app/utils/event-util.ts
@@ -103,10 +103,6 @@
fire(target, EventType.SHOW_TAB, detail);
}
-export function fireCloseFixPreview(target: EventTarget, fixApplied: boolean) {
- fire(target, EventType.CLOSE_FIX_PREVIEW, {fixApplied});
-}
-
export function fireReload(target: EventTarget, clearPatchset?: boolean) {
fire(target, EventType.RELOAD, {clearPatchset: !!clearPatchset});
}
diff --git a/polygerrit-ui/app/utils/link-util.ts b/polygerrit-ui/app/utils/link-util.ts
index ec1e7e7..ccafa5a 100644
--- a/polygerrit-ui/app/utils/link-util.ts
+++ b/polygerrit-ui/app/utils/link-util.ts
@@ -3,7 +3,6 @@
* Copyright 2022 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
-import 'ba-linkify/ba-linkify';
import {CommentLinkInfo, CommentLinks} from '../types/common';
import {getBaseUrl} from './url-util';
@@ -16,21 +15,8 @@
base: string,
repoCommentLinks: CommentLinks
): string {
- const parts: string[] = [];
- window.linkify(insertZeroWidthSpace(base), {
- callback: (text, href) => {
- if (href) {
- parts.push(removeZeroWidthSpace(createLinkTemplate(href, text)));
- } else {
- const rewriteResults = getRewriteResultsFromConfig(
- text,
- repoCommentLinks
- );
- parts.push(removeZeroWidthSpace(applyRewrites(text, rewriteResults)));
- }
- },
- });
- return parts.join('');
+ const rewriteResults = getRewriteResultsFromConfig(base, repoCommentLinks);
+ return applyRewrites(base, rewriteResults);
}
/**
@@ -47,6 +33,11 @@
commentLinkInfo =>
commentLinkInfo.enabled !== false && commentLinkInfo.link !== undefined
);
+ // Always linkify URLs starting with https?://
+ enabledRewrites.push({
+ match: '(https?://\\S+[\\w/])',
+ link: '$1',
+ });
return enabledRewrites.flatMap(rewrite => {
const regexp = new RegExp(rewrite.match, 'g');
const partialResults: RewriteResult[] = [];
@@ -141,22 +132,6 @@
);
}
-/**
- * Some tools are known to look for reviewers/CCs by finding lines such as
- * "R=foo@gmail.com, bar@gmail.com". However, "=" is technically a valid email
- * character, so ba-linkify interprets the entire string "R=foo@gmail.com" as an
- * email address. To fix this, we insert a zero width space character \u200B
- * before linking that prevents ba-linkify from associating the prefix with the
- * email. After linking we remove the zero width space.
- */
-function insertZeroWidthSpace(base: string) {
- return base.replace(/^(R=|CC=)/g, '$&\u200B');
-}
-
-function removeZeroWidthSpace(base: string) {
- return base.replace(/\u200B/g, '');
-}
-
function createLinkTemplate(
href: string,
displayText: string,
diff --git a/polygerrit-ui/app/utils/link-util_test.ts b/polygerrit-ui/app/utils/link-util_test.ts
index f5c13e8..52b8288 100644
--- a/polygerrit-ui/app/utils/link-util_test.ts
+++ b/polygerrit-ui/app/utils/link-util_test.ts
@@ -12,6 +12,17 @@
}
suite('link rewrites', () => {
+ test('default linking', () => {
+ assert.equal(
+ linkifyUrlsAndApplyRewrite('http://www.google.com', {}),
+ link('http://www.google.com', 'http://www.google.com')
+ );
+ assert.equal(
+ linkifyUrlsAndApplyRewrite('https://www.google.com', {}),
+ link('https://www.google.com', 'https://www.google.com')
+ );
+ });
+
test('without text', () => {
assert.equal(
linkifyUrlsAndApplyRewrite('foo', {
@@ -63,18 +74,6 @@
`${link('foo', 'foo.gov')} ${link('foo', 'foo.gov')}`
);
});
-
- test('does not apply within normal links', () => {
- assert.equal(
- linkifyUrlsAndApplyRewrite('google.com', {
- ogle: {
- match: 'ogle',
- link: 'gerritcodereview.com',
- },
- }),
- link('google.com', 'http://google.com')
- );
- });
});
test('for overlapping rewrites prefer the latest ending', () => {
@@ -165,45 +164,4 @@
)}`
);
});
-
- suite('normal links', () => {
- test('links urls', () => {
- const googleLink = link('google.com', 'http://google.com');
- const mapsLink = link('maps.google.com', 'http://maps.google.com');
-
- assert.equal(
- linkifyUrlsAndApplyRewrite('google.com, maps.google.com', {}),
- `${googleLink}, ${mapsLink}`
- );
- });
-
- test('links emails without including R= prefix', () => {
- const fooEmail = link('foo@gmail.com', 'mailto:foo@gmail.com');
- const barEmail = link('bar@gmail.com', 'mailto:bar@gmail.com');
- assert.equal(
- linkifyUrlsAndApplyRewrite('R=foo@gmail.com, bar@gmail.com', {}),
- `R=${fooEmail}, ${barEmail}`
- );
- });
-
- test('links emails without including CC= prefix', () => {
- const fooEmail = link('foo@gmail.com', 'mailto:foo@gmail.com');
- const barEmail = link('bar@gmail.com', 'mailto:bar@gmail.com');
- assert.equal(
- linkifyUrlsAndApplyRewrite('CC=foo@gmail.com, bar@gmail.com', {}),
- `CC=${fooEmail}, ${barEmail}`
- );
- });
-
- test('links emails maintains R= and CC= within addresses', () => {
- const fooBarBazEmail = link(
- 'fooR=barCC=baz@gmail.com',
- 'mailto:fooR=barCC=baz@gmail.com'
- );
- assert.equal(
- linkifyUrlsAndApplyRewrite('fooR=barCC=baz@gmail.com', {}),
- fooBarBazEmail
- );
- });
- });
});
diff --git a/polygerrit-ui/app/yarn.lock b/polygerrit-ui/app/yarn.lock
index 2edce7e..7f8168e 100644
--- a/polygerrit-ui/app/yarn.lock
+++ b/polygerrit-ui/app/yarn.lock
@@ -488,11 +488,6 @@
delegates "^1.0.0"
readable-stream "^2.0.6"
-ba-linkify@^1.0.1:
- version "1.0.1"
- resolved "https://registry.yarnpkg.com/ba-linkify/-/ba-linkify-1.0.1.tgz#664cf5744947c6e8611f1fbbaf7d9f315f982f4c"
- integrity sha1-Zkz1dElHxuhhHx+7r32fMV+YL0w=
-
balanced-match@^1.0.0:
version "1.0.2"
resolved "https://registry.yarnpkg.com/balanced-match/-/balanced-match-1.0.2.tgz#e83e3a7e3f300b34cb9d87f615fa0cbf357690ee"
diff --git a/tools/deps.bzl b/tools/deps.bzl
index 28904a9..4c21ae6 100644
--- a/tools/deps.bzl
+++ b/tools/deps.bzl
@@ -18,7 +18,7 @@
GITILES_REPO = GERRIT
# When updating Bouncy Castle, also update it in bazlets.
-BC_VERS = "1.64"
+BC_VERS = "1.72"
HTTPCOMP_VERS = "4.5.2"
JETTY_VERS = "9.4.49.v20220914"
BYTE_BUDDY_VERSION = "1.10.7"
@@ -558,20 +558,26 @@
maven_jar(
name = "bcprov",
- artifact = "org.bouncycastle:bcprov-jdk15on:" + BC_VERS,
- sha1 = "1467dac1b787b5ad2a18201c0c281df69882259e",
+ artifact = "org.bouncycastle:bcprov-jdk18on:" + BC_VERS,
+ sha1 = "d8dc62c28a3497d29c93fee3e71c00b27dff41b4",
)
maven_jar(
name = "bcpg",
- artifact = "org.bouncycastle:bcpg-jdk15on:" + BC_VERS,
- sha1 = "56956a8c63ccadf62e7c678571cf86f30bd84441",
+ artifact = "org.bouncycastle:bcpg-jdk18on:" + BC_VERS,
+ sha1 = "1a36a1740d07869161f6f0d01fae8d72dd1d8320",
)
maven_jar(
name = "bcpkix",
- artifact = "org.bouncycastle:bcpkix-jdk15on:" + BC_VERS,
- sha1 = "3dac163e20110817d850d17e0444852a6d7d0bd7",
+ artifact = "org.bouncycastle:bcpkix-jdk18on:" + BC_VERS,
+ sha1 = "bb3fdb5162ccd5085e8d7e57fada4d8eaa571f5a",
+ )
+
+ maven_jar(
+ name = "bcutil",
+ artifact = "org.bouncycastle:bcutil-jdk18on:" + BC_VERS,
+ sha1 = "41f19a69ada3b06fa48781120d8bebe1ba955c77",
)
maven_jar(