Merge changes I2a63f725,I582c12ea,I476c6371,I886e4fcf * changes: Store submit requirements for merged changes in NoteDb Enable parsing submit requirements results from change meta commits Add Gson type adapter to submit requirement entities Submit requirements: refactor SubmitRequirementResult
diff --git a/Documentation/dev-bazel.txt b/Documentation/dev-bazel.txt index 61565f8..c3237ed 100644 --- a/Documentation/dev-bazel.txt +++ b/Documentation/dev-bazel.txt
@@ -365,6 +365,13 @@ bazel test --test_tag_filters=api,git //... ---- +To run the tests against a specific index backend (LUCENE, FAKE): +---- + bazel test --test_env=GERRIT_INDEX_TYPE=LUCENE //... +---- + +Elastic search is not currently supported in integration tests. + The following values are currently supported for the group name: * annotation
diff --git a/Documentation/images/gwt-user-review-ui-change-screen-change-info-cannot-merge.png b/Documentation/images/gwt-user-review-ui-change-screen-change-info-cannot-merge.png deleted file mode 100644 index 69a28ec..0000000 --- a/Documentation/images/gwt-user-review-ui-change-screen-change-info-cannot-merge.png +++ /dev/null Binary files differ
diff --git a/Documentation/images/gwt-user-review-ui-change-screen-change-info.png b/Documentation/images/gwt-user-review-ui-change-screen-change-info.png deleted file mode 100644 index e92b49d..0000000 --- a/Documentation/images/gwt-user-review-ui-change-screen-change-info.png +++ /dev/null Binary files differ
diff --git a/Documentation/images/gwt-user-review-ui-change-screen-change-update.png b/Documentation/images/gwt-user-review-ui-change-screen-change-update.png deleted file mode 100644 index 227db40..0000000 --- a/Documentation/images/gwt-user-review-ui-change-screen-change-update.png +++ /dev/null Binary files differ
diff --git a/Documentation/images/gwt-user-review-ui-change-screen-commit-info-merge-commit.png b/Documentation/images/gwt-user-review-ui-change-screen-commit-info-merge-commit.png deleted file mode 100644 index 097637e..0000000 --- a/Documentation/images/gwt-user-review-ui-change-screen-commit-info-merge-commit.png +++ /dev/null Binary files differ
diff --git a/Documentation/images/gwt-user-review-ui-change-screen-commit-info.png b/Documentation/images/gwt-user-review-ui-change-screen-commit-info.png deleted file mode 100644 index fe0c1d1..0000000 --- a/Documentation/images/gwt-user-review-ui-change-screen-commit-info.png +++ /dev/null Binary files differ
diff --git a/Documentation/images/gwt-user-review-ui-change-screen-included-in-list.png b/Documentation/images/gwt-user-review-ui-change-screen-included-in-list.png deleted file mode 100644 index ad30fe2..0000000 --- a/Documentation/images/gwt-user-review-ui-change-screen-included-in-list.png +++ /dev/null Binary files differ
diff --git a/Documentation/images/gwt-user-review-ui-change-screen-not-current.png b/Documentation/images/gwt-user-review-ui-change-screen-not-current.png deleted file mode 100644 index 9a87c67..0000000 --- a/Documentation/images/gwt-user-review-ui-change-screen-not-current.png +++ /dev/null Binary files differ
diff --git a/Documentation/images/gwt-user-review-ui-change-screen-permalink.png b/Documentation/images/gwt-user-review-ui-change-screen-permalink.png deleted file mode 100644 index a1aede9..0000000 --- a/Documentation/images/gwt-user-review-ui-change-screen-permalink.png +++ /dev/null Binary files differ
diff --git a/Documentation/images/gwt-user-review-ui-change-screen-plugin-extensions.png b/Documentation/images/gwt-user-review-ui-change-screen-plugin-extensions.png deleted file mode 100644 index 120b99c..0000000 --- a/Documentation/images/gwt-user-review-ui-change-screen-plugin-extensions.png +++ /dev/null Binary files differ
diff --git a/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-comment.png b/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-comment.png deleted file mode 100644 index 2ecc47e..0000000 --- a/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-comment.png +++ /dev/null Binary files differ
diff --git a/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-commented.png b/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-commented.png deleted file mode 100644 index 598d18d..0000000 --- a/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-commented.png +++ /dev/null Binary files differ
diff --git a/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-keyboard-shortcuts.png b/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-keyboard-shortcuts.png deleted file mode 100644 index 6f63f0e4..0000000 --- a/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-keyboard-shortcuts.png +++ /dev/null Binary files differ
diff --git a/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-project-and-file.png b/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-project-and-file.png deleted file mode 100644 index 836964b..0000000 --- a/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-project-and-file.png +++ /dev/null Binary files differ
diff --git a/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen.png b/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen.png deleted file mode 100644 index d76ecef..0000000 --- a/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen.png +++ /dev/null Binary files differ
diff --git a/Documentation/images/user-review-ui-change-screen-change-update.png b/Documentation/images/user-review-ui-change-screen-change-update.png new file mode 100644 index 0000000..fe07ef9 --- /dev/null +++ b/Documentation/images/user-review-ui-change-screen-change-update.png Binary files differ
diff --git a/Documentation/images/user-review-ui-change-screen-plugin-extensions.png b/Documentation/images/user-review-ui-change-screen-plugin-extensions.png new file mode 100644 index 0000000..5d6fee7 --- /dev/null +++ b/Documentation/images/user-review-ui-change-screen-plugin-extensions.png Binary files differ
diff --git a/Documentation/images/user-review-ui-side-by-side-diff-screen.png b/Documentation/images/user-review-ui-side-by-side-diff-screen.png new file mode 100644 index 0000000..74d02e3 --- /dev/null +++ b/Documentation/images/user-review-ui-side-by-side-diff-screen.png Binary files differ
diff --git a/Documentation/js_licenses.txt b/Documentation/js_licenses.txt index 0d8da89..813ff44 100644 --- a/Documentation/js_licenses.txt +++ b/Documentation/js_licenses.txt
@@ -1077,6 +1077,38 @@ ---- +[[immer]] +immer + +* immer + +[[immer_license]] +---- +MIT License + +Copyright (c) 2017 Michel Weststrate + +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. + +---- + + [[isarray]] isarray
diff --git a/Documentation/licenses.txt b/Documentation/licenses.txt index ed1a336..11f9ff3 100644 --- a/Documentation/licenses.txt +++ b/Documentation/licenses.txt
@@ -4036,6 +4036,38 @@ ---- +[[immer]] +immer + +* immer + +[[immer_license]] +---- +MIT License + +Copyright (c) 2017 Michel Weststrate + +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. + +---- + + [[isarray]] isarray
diff --git a/Documentation/user-review-ui.txt b/Documentation/user-review-ui.txt index d4fe6b5..6f5f7297 100644 --- a/Documentation/user-review-ui.txt +++ b/Documentation/user-review-ui.txt
@@ -218,7 +218,7 @@ ** [[plugin-actions]]Further actions may be available if plugins are installed. + -image::images/user-review-ui-change-screen-change-info-actions.png[width=600, link="images/user-review-ui-change-screen-change-info-actions.png"] +image::images/user-review-ui-change-screen-change-info-actions.png[width=400, link="images/user-review-ui-change-screen-change-info-actions.png"] - [[labels]]Labels & Votes: + @@ -259,7 +259,7 @@ set is currently viewed can be seen from the `Patch Sets` drop-down panel in the change header. -image::images/user-review-ui-change-screen-patch-sets.png[width=487, link="images/user-review-ui-change-screen-patch-sets.png"] +image::images/user-review-ui-change-screen-patch-sets.png[width=300, link="images/user-review-ui-change-screen-patch-sets.png"] [[download]] @@ -458,7 +458,7 @@ comments; a summary comment is only added if the reply popup panel is open when the quick approve button is clicked. -image::images/user-review-ui-change-screen-quick-approve.png[width=800, link="images/gwt-user-review-ui-change-screen-quick-approve.png"] +image::images/user-review-ui-change-screen-quick-approve.png[width=800, link="images/user-review-ui-change-screen-quick-approve.png"] [[history]] === History @@ -480,16 +480,16 @@ it is 30 seconds. Polling may also be completely disabled by the administrator. -image::images/gwt-user-review-ui-change-screen-change-update.png[width=800, link="images/gwt-user-review-ui-change-screen-change-update.png"] +image::images/user-review-ui-change-screen-change-update.png[width=400, link="images/user-review-ui-change-screen-change-update.png"] [[plugin-extensions]] === Plugin Extensions -Gerrit plugins may extend the change screen; they can add buttons for -additional actions to the change info block and display arbitrary UI -controls below the change info block. +Gerrit plugins may extend the change screen. Java plugins in the +backend can add additional actions to the triple-dot menu block. +Frontend plugins can change the UI controls in arbitrary ways. -image::images/gwt-user-review-ui-change-screen-plugin-extensions.png[width=800, link="images/gwt-user-review-ui-change-screen-plugin-extensions.png"] +image::images/user-review-ui-change-screen-plugin-extensions.png[width=300, link="images/user-review-ui-change-screen-plugin-extensions.png"] [[side-by-side]] == Side-by-Side Diff Screen @@ -500,17 +500,8 @@ This screen allows to review a patch and to comment on it. -image::images/gwt-user-review-ui-side-by-side-diff-screen.png[width=800, link="images/gwt-user-review-ui-side-by-side-diff-screen.png"] +image::images/user-review-ui-side-by-side-diff-screen.png[width=800, link="images/user-review-ui-side-by-side-diff-screen.png"] -[[side-by-side-header]] -In the screen header the project name and the name of the viewed patch -file are shown. - -If a Git web browser is configured on the server, the project name and -the file path are displayed as links to the project and the folder in -the Git web browser. - -image::images/gwt-user-review-ui-side-by-side-diff-screen-project-and-file.png[width=800, link="images/gwt-user-review-ui-side-by-side-diff-screen-project-and-file.png"] [[side-by-side-mark-reviewed]] The checkbox in front of the file name allows the @@ -518,7 +509,7 @@ diff preference allows to control whether the files should be automatically marked as reviewed when they are viewed. -image::images/user-review-ui-side-by-side-diff-screen-reviewed.png[width=800, link="images/gwt-user-review-ui-side-by-side-diff-screen-reviewed.png"] +image::images/user-review-ui-side-by-side-diff-screen-reviewed.png[width=800, link="images/user-review-ui-side-by-side-diff-screen-reviewed.png"] [[patch-set-selection]] In the header, on each side, the list of patch sets is shown. Clicking @@ -615,13 +606,9 @@ For typing the new comment, a new comment box is shown under the code that is commented. -Clicking on the `Save` button saves the new comment as a draft. To make -it visible to other users it must be published from the change screen -by link:#reply[replying] to the change. - -Clicking on the `Discard` button deletes the new comment. - -image::images/gwt-user-review-ui-side-by-side-diff-screen-commented.png[width=800, link="images/gwt-user-review-ui-side-by-side-diff-screen-commented.png"] +Clicking on the `Save` button saves the new comment as a draft. To +make it visible to other users it must be published from the change +screen by link:#reply[replying] to the change. [[file-level-comments]] === File Level Comments @@ -629,7 +616,7 @@ File level comments are added by clicking the 'File' header at the top of the file. -image::images/user-review-ui-side-by-side-diff-screen-file-level-comment.png[width=800, link="images/gwt-user-review-ui-side-by-side-diff-screen-file-level-comment.png"] +image::images/user-review-ui-side-by-side-diff-screen-file-level-comment.png[width=400, link="images/user-review-ui-side-by-side-diff-screen-file-level-comment.png"] [[diff-preferences]] === Diff Preferences @@ -639,7 +626,7 @@ preferences. The diff preferences can be accessed by clicking on the settings icon in the screen header. -image::images/user-review-ui-side-by-side-diff-screen-preferences.png[width=800, link="images/gwt-user-review-ui-side-by-side-diff-screen-preferences.png"] +image::images/user-review-ui-side-by-side-diff-screen-preferences.png[width=800, link="images/user-review-ui-side-by-side-diff-screen-preferences.png"] The following diff preferences can be configured: @@ -686,7 +673,7 @@ If many lines are skipped there are additional links to expand the context by ten lines before and after the skipped block. + -image::images/user-review-ui-side-by-side-diff-screen-expand-skipped-lines.png[width=800, link="images/gwt-user-review-ui-side-by-side-diff-screen-expand-skipped-lines.png"] +image::images/user-review-ui-side-by-side-diff-screen-expand-skipped-lines.png[width=800, link="images/user-review-ui-side-by-side-diff-screen-expand-skipped-lines.png"] - [[syntax-highlighting]]`Syntax Highlighting`: + @@ -716,7 +703,7 @@ a popup that shows a list of available keyboard shortcuts. -image::images/user-review-ui-change-screen-keyboard-shortcuts.png[width=800, link="images/gwt-user-review-ui-change-screen-keyboard-shortcuts.png"] +image::images/user-review-ui-change-screen-keyboard-shortcuts.png[width=800, link="images/user-review-ui-change-screen-keyboard-shortcuts.png"] In addition, Vim-like commands can be used to link:#key-navigation[
diff --git a/Documentation/user-search.txt b/Documentation/user-search.txt index a2dc31f..a9779b1 100644 --- a/Documentation/user-search.txt +++ b/Documentation/user-search.txt
@@ -702,6 +702,18 @@ to one of the fields in the link:rest-api-changes.html#submit-record[SubmitRecord] REST API entity. +`label:Code-Review=MAX`:: ++ +Matches changes with label voted with the highest possible score. + +`label:Code-Review=MIN`:: ++ +Matches changes with label voted with the lowest possible score. + +`label:Code-Review=ANY`:: ++ +Matches changes with label voted with any score. + `label:Non-Author-Code-Review=need`:: + Matches changes where the submit rules indicate that a label named
diff --git a/java/com/google/gerrit/acceptance/BUILD b/java/com/google/gerrit/acceptance/BUILD index 5ee1a08..fa62cd9 100644 --- a/java/com/google/gerrit/acceptance/BUILD +++ b/java/com/google/gerrit/acceptance/BUILD
@@ -75,6 +75,7 @@ "//java/com/google/gerrit/extensions/restapi/testing:restapi-test-util", "//java/com/google/gerrit/gpg/testing:gpg-test-util", "//java/com/google/gerrit/git/testing", + "//java/com/google/gerrit/index/testing", ] PGM_DEPLOY_ENV = [
diff --git a/java/com/google/gerrit/acceptance/GerritServer.java b/java/com/google/gerrit/acceptance/GerritServer.java index 93c1237..085fef5 100644 --- a/java/com/google/gerrit/acceptance/GerritServer.java +++ b/java/com/google/gerrit/acceptance/GerritServer.java
@@ -45,6 +45,8 @@ import com.google.gerrit.common.Nullable; import com.google.gerrit.extensions.annotations.Exports; import com.google.gerrit.extensions.config.FactoryModule; +import com.google.gerrit.index.IndexType; +import com.google.gerrit.index.testing.FakeIndexModule; import com.google.gerrit.lucene.LuceneIndexModule; import com.google.gerrit.pgm.Daemon; import com.google.gerrit.pgm.Init; @@ -449,9 +451,29 @@ cfg.setString("gitweb", null, "cgi", ""); cfg.setString( "accountPatchReviewDb", null, "url", JdbcAccountPatchReviewStore.TEST_IN_MEMORY_URL); + + String configuredIndexBackend = cfg.getString("index", null, "type"); + IndexType indexType; + if (configuredIndexBackend != null) { + // Explicitly configured index backend from gerrit.config trumps any other ways to configure + // index backends so that Reindex tests can be explicit about the backend they want to test + // against. + indexType = new IndexType(configuredIndexBackend); + } else { + // Allow configuring the index backend based on sys/env variables so that integration tests + // can be run against different index backends. + indexType = IndexType.fromEnvironment().orElse(new IndexType("fake")); + } + if (indexType.isLucene()) { + daemon.setIndexModule( + LuceneIndexModule.singleVersionAllLatest(0, ReplicaUtil.isReplica(baseConfig))); + } else { + daemon.setIndexModule(FakeIndexModule.latestVersion(false)); + } + // Elastic search is not supported in integration tests yet. + daemon.setEnableHttpd(desc.httpd()); - daemon.setLuceneModule( - LuceneIndexModule.singleVersionAllLatest(0, ReplicaUtil.isReplica(baseConfig))); + daemon.setInMemory(true); daemon.setDatabaseForTesting( ImmutableList.of( new InMemoryTestingDatabaseModule(cfg, site, inMemoryRepoManager), @@ -476,6 +498,8 @@ String[] additionalArgs) throws Exception { requireNonNull(site); + daemon.addAdditionalSysModuleForTesting( + new ReindexProjectsAtStartup.Module(), new ReindexGroupsAtStartup.Module()); ExecutorService daemonService = Executors.newSingleThreadExecutor(); String[] args = Stream.concat(
diff --git a/java/com/google/gerrit/entities/SubscribeSection.java b/java/com/google/gerrit/entities/SubscribeSection.java index b95517c..574cae8 100644 --- a/java/com/google/gerrit/entities/SubscribeSection.java +++ b/java/com/google/gerrit/entities/SubscribeSection.java
@@ -99,9 +99,10 @@ public ImmutableSet<BranchNameKey> getDestinationBranches( BranchNameKey src, Collection<Ref> allRefsInRefsHeads) { Set<BranchNameKey> ret = new HashSet<>(); - logger.atFine().log("Inspecting SubscribeSection %s", this); - for (RefSpec r : matchingRefSpecs()) { - logger.atFine().log("Inspecting [matching] ref %s", r); + + ImmutableList<RefSpec> matching = matchingRefSpecs(); + ImmutableList<RefSpec> multiMatch = multiMatchRefSpecs(); + for (RefSpec r : matching) { if (!r.matchSource(src.branch())) { continue; } @@ -118,8 +119,7 @@ } } - for (RefSpec r : multiMatchRefSpecs()) { - logger.atFine().log("Inspecting [all] ref %s", r); + for (RefSpec r : multiMatch) { if (!r.matchSource(src.branch())) { continue; } @@ -133,7 +133,9 @@ } } } - logger.atFine().log("Returning possible branches: %s for project %s", ret, project()); + logger.atFine().log( + "getDestinationBranches(%s): %s. matching refs: %s, multimatch refs: %s", + this, ret, matching, multiMatch); return ImmutableSet.copyOf(ret); }
diff --git a/java/com/google/gerrit/httpd/init/WebAppInitializer.java b/java/com/google/gerrit/httpd/init/WebAppInitializer.java index 079f306..91d032e 100644 --- a/java/com/google/gerrit/httpd/init/WebAppInitializer.java +++ b/java/com/google/gerrit/httpd/init/WebAppInitializer.java
@@ -122,6 +122,8 @@ import com.google.inject.spi.Message; import com.google.inject.util.Providers; import java.io.IOException; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; import java.nio.file.Path; import java.nio.file.Paths; import java.util.ArrayList; @@ -365,6 +367,19 @@ return LuceneIndexModule.latestVersion(false); } else if (indexType.isElasticsearch()) { return ElasticIndexModule.latestVersion(false); + } else if (indexType.isFake()) { + // Use Reflection so that we can omit the fake index binary in production code. Test code does + // compile the component in. + try { + Class<?> clazz = Class.forName("com.google.gerrit.index.testing.FakeIndexModule"); + Method m = clazz.getMethod("latestVersion", boolean.class); + return (Module) m.invoke(null, false); + } catch (NoSuchMethodException + | ClassNotFoundException + | IllegalAccessException + | InvocationTargetException e) { + throw new IllegalStateException("can't create index", e); + } } else { throw new IllegalStateException("unsupported index.type = " + indexType); }
diff --git a/java/com/google/gerrit/index/IndexType.java b/java/com/google/gerrit/index/IndexType.java index cade439..0c3a76a 100644 --- a/java/com/google/gerrit/index/IndexType.java +++ b/java/com/google/gerrit/index/IndexType.java
@@ -14,8 +14,12 @@ package com.google.gerrit.index; +import static com.google.common.base.Preconditions.checkArgument; + +import com.google.common.base.Strings; import com.google.common.collect.ImmutableSet; import com.google.gerrit.common.Nullable; +import java.util.Optional; /** * Index types supported by the secondary index. @@ -28,12 +32,42 @@ * allows to not break that case upon core implementation changes. */ public class IndexType { + public static final String SYS_PROP = "gerrit.index.type"; + private static final String ENV_VAR = "GERRIT_INDEX_TYPE"; + private static final String LUCENE = "lucene"; private static final String ELASTICSEARCH = "elasticsearch"; private static final String FAKE = "fake"; private final String type; + /** + * Returns the index type in case it was set by an environment variable. This is useful to run + * tests against a certain index backend. + */ + public static Optional<IndexType> fromEnvironment() { + String value = System.getenv(ENV_VAR); + if (Strings.isNullOrEmpty(value)) { + value = System.getProperty(SYS_PROP); + } + if (Strings.isNullOrEmpty(value)) { + return Optional.empty(); + } + value = value.toUpperCase().replace("-", "_"); + IndexType type = new IndexType(value); + if (!Strings.isNullOrEmpty(System.getenv(ENV_VAR))) { + checkArgument( + type != null, "Invalid value for env variable %s: %s", ENV_VAR, System.getenv(ENV_VAR)); + } else { + checkArgument( + type != null, + "Invalid value for system property %s: %s", + SYS_PROP, + System.getProperty(SYS_PROP)); + } + return Optional.of(type); + } + public IndexType(@Nullable String type) { this.type = type == null ? getDefault() : type.toLowerCase(); }
diff --git a/java/com/google/gerrit/index/testing/FakeIndexModuleOnInit.java b/java/com/google/gerrit/index/testing/FakeIndexModuleOnInit.java new file mode 100644 index 0000000..75d8de2 --- /dev/null +++ b/java/com/google/gerrit/index/testing/FakeIndexModuleOnInit.java
@@ -0,0 +1,37 @@ +// Copyright (C) 2021 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.gerrit.index.testing; + +import com.google.gerrit.index.testing.AbstractFakeIndex.FakeAccountIndex; +import com.google.gerrit.index.testing.AbstractFakeIndex.FakeGroupIndex; +import com.google.gerrit.server.index.account.AccountIndex; +import com.google.gerrit.server.index.group.GroupIndex; +import com.google.inject.AbstractModule; +import com.google.inject.assistedinject.FactoryModuleBuilder; + +public class FakeIndexModuleOnInit extends AbstractModule { + @Override + protected void configure() { + install( + new FactoryModuleBuilder() + .implement(AccountIndex.class, FakeAccountIndex.class) + .build(AccountIndex.Factory.class)); + + install( + new FactoryModuleBuilder() + .implement(GroupIndex.class, FakeGroupIndex.class) + .build(GroupIndex.Factory.class)); + } +}
diff --git a/java/com/google/gerrit/pgm/Daemon.java b/java/com/google/gerrit/pgm/Daemon.java index 29c5788..2b4cfef 100644 --- a/java/com/google/gerrit/pgm/Daemon.java +++ b/java/com/google/gerrit/pgm/Daemon.java
@@ -87,6 +87,7 @@ import com.google.gerrit.server.git.SearchingChangeCacheImpl; import com.google.gerrit.server.git.WorkQueue; import com.google.gerrit.server.group.PeriodicGroupIndexer; +import com.google.gerrit.server.index.AbstractIndexModule; import com.google.gerrit.server.index.IndexModule; import com.google.gerrit.server.index.OnlineUpgrader; import com.google.gerrit.server.index.VersionManager; @@ -128,6 +129,8 @@ import com.google.inject.Provider; import com.google.inject.Stage; import java.io.IOException; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; import java.nio.file.Files; import java.nio.file.Path; import java.util.ArrayList; @@ -207,7 +210,7 @@ private Injector httpdInjector; private Path runFile; private boolean inMemoryTest; - private AbstractModule luceneModule; + private AbstractModule indexModule; private Module emailModule; private List<Module> testSysModules = new ArrayList<>(); private List<Module> testSshModules = new ArrayList<>(); @@ -336,9 +339,13 @@ } @VisibleForTesting - public void setLuceneModule(LuceneIndexModule m) { - luceneModule = m; - inMemoryTest = true; + public void setIndexModule(AbstractIndexModule m) { + indexModule = m; + } + + @VisibleForTesting + public void setInMemory(boolean inMemory) { + this.inMemoryTest = inMemory; } @VisibleForTesting @@ -523,8 +530,8 @@ } private Module createIndexModule() { - if (luceneModule != null) { - return luceneModule; + if (indexModule != null) { + return indexModule; } if (indexType.isLucene()) { return LuceneIndexModule.latestVersion(replica); @@ -532,6 +539,20 @@ if (indexType.isElasticsearch()) { return ElasticIndexModule.latestVersion(replica); } + if (indexType.isFake()) { + // Use Reflection so that we can omit the fake index binary in production code. Test code does + // compile the component in. + try { + Class<?> clazz = Class.forName("com.google.gerrit.index.testing.FakeIndexModule"); + Method m = clazz.getMethod("latestVersion", boolean.class); + return (Module) m.invoke(null, replica); + } catch (NoSuchMethodException + | ClassNotFoundException + | IllegalAccessException + | InvocationTargetException e) { + throw new IllegalStateException("can't create index", e); + } + } throw new IllegalStateException("unsupported index.type = " + indexType); }
diff --git a/java/com/google/gerrit/pgm/Reindex.java b/java/com/google/gerrit/pgm/Reindex.java index 3935268..6e99007 100644 --- a/java/com/google/gerrit/pgm/Reindex.java +++ b/java/com/google/gerrit/pgm/Reindex.java
@@ -39,6 +39,8 @@ import com.google.inject.Injector; import com.google.inject.Key; import com.google.inject.Module; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; import java.util.ArrayList; import java.util.Collection; import java.util.HashMap; @@ -154,6 +156,21 @@ } else if (indexType.isElasticsearch()) { indexModule = ElasticIndexModule.singleVersionWithExplicitVersions(versions, threads, replica); + } else if (indexType.isFake()) { + // Use Reflection so that we can omit the fake index binary in production code. Test code does + // compile the component in. + try { + Class<?> clazz = Class.forName("com.google.gerrit.index.testing.FakeIndexModule"); + Method m = + clazz.getMethod( + "singleVersionWithExplicitVersions", Map.class, int.class, boolean.class); + indexModule = (Module) m.invoke(null, versions, threads, replica); + } catch (NoSuchMethodException + | ClassNotFoundException + | IllegalAccessException + | InvocationTargetException e) { + throw new IllegalStateException("can't create index", e); + } } else { throw new IllegalStateException("unsupported index.type = " + indexType); }
diff --git a/java/com/google/gerrit/pgm/init/BaseInit.java b/java/com/google/gerrit/pgm/init/BaseInit.java index 62ff66a..c4b0040 100644 --- a/java/com/google/gerrit/pgm/init/BaseInit.java +++ b/java/com/google/gerrit/pgm/init/BaseInit.java
@@ -32,6 +32,7 @@ import com.google.gerrit.pgm.init.api.InstallPlugins; import com.google.gerrit.pgm.init.api.LibraryDownload; import com.google.gerrit.pgm.init.index.IndexManagerOnInit; +import com.google.gerrit.pgm.init.index.IndexModuleOnInit; import com.google.gerrit.pgm.init.index.elasticsearch.ElasticIndexModuleOnInit; import com.google.gerrit.pgm.init.index.lucene.LuceneIndexModuleOnInit; import com.google.gerrit.pgm.util.SiteProgram; @@ -57,6 +58,7 @@ import com.google.inject.util.Providers; import java.io.FileNotFoundException; import java.io.IOException; +import java.lang.reflect.InvocationTargetException; import java.nio.file.FileVisitResult; import java.nio.file.Files; import java.nio.file.Path; @@ -418,6 +420,19 @@ modules.add(new LuceneIndexModuleOnInit()); } else if (indexType.isElasticsearch()) { modules.add(new ElasticIndexModuleOnInit()); + } else if (indexType.isFake()) { + try { + Class<?> clazz = Class.forName("com.google.gerrit.index.testing.FakeIndexModuleOnInit"); + Module indexOnInitModule = (Module) clazz.getDeclaredConstructor().newInstance(); + modules.add(indexOnInitModule); + } catch (InstantiationException + | IllegalAccessException + | ClassNotFoundException + | NoSuchMethodException + | InvocationTargetException e) { + throw new IllegalStateException("unable to create fake index", e); + } + modules.add(new IndexModuleOnInit()); } else { throw new IllegalStateException("unsupported index.type = " + indexType); }
diff --git a/java/com/google/gerrit/server/config/PluginConfigFactory.java b/java/com/google/gerrit/server/config/PluginConfigFactory.java index 2d0f9a5..c49e928 100644 --- a/java/com/google/gerrit/server/config/PluginConfigFactory.java +++ b/java/com/google/gerrit/server/config/PluginConfigFactory.java
@@ -28,13 +28,11 @@ import com.google.inject.Inject; import com.google.inject.Provider; import com.google.inject.Singleton; -import java.io.File; import java.io.IOException; import java.nio.file.Path; import java.util.HashMap; import java.util.Map; import org.eclipse.jgit.errors.ConfigInvalidException; -import org.eclipse.jgit.internal.storage.file.FileSnapshot; import org.eclipse.jgit.lib.Config; import org.eclipse.jgit.storage.file.FileBasedConfig; import org.eclipse.jgit.util.FS; @@ -52,7 +50,6 @@ private final SecureStore secureStore; private final Map<String, Config> pluginConfigs; - private volatile FileSnapshot cfgSnapshot; private volatile Config cfg; @Inject @@ -69,7 +66,6 @@ this.secureStore = secureStore; this.pluginConfigs = new HashMap<>(); - this.cfgSnapshot = FileSnapshot.save(site.gerrit_config.toFile()); this.cfg = cfgProvider.get(); } @@ -103,12 +99,10 @@ * @return the plugin configuration from the 'gerrit.config' file */ public PluginConfig getFromGerritConfig(String pluginName, boolean refresh) { - if (refresh && secureStore.isOutdated()) { - secureStore.reload(); - } - File configFile = site.gerrit_config.toFile(); - if (refresh && cfgSnapshot.isModified(configFile)) { - cfgSnapshot = FileSnapshot.save(configFile); + if (refresh) { + if (secureStore.isOutdated()) { + secureStore.reload(); + } cfg = cfgProvider.get(); } return PluginConfig.createFromGerritConfig(pluginName, cfg);
diff --git a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java index 005aebb..454df66 100644 --- a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java +++ b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
@@ -659,7 +659,9 @@ // Process as many commands as possible, but may leave some commands in state NOT_ATTEMPTED. private void processCommandsUnsafe( Collection<ReceiveCommand> commands, MultiProgressMonitor progress) { - logger.atFine().log("Calling user: %s", user.getLoggableName()); + logger.atFine().log("Calling user: %s, commands: %d", user.getLoggableName(), commands.size()); + + // If the list of groups is large, the log entry may get dropped, so separate out. logger.atFine().log("Groups: %s", lazy(() -> user.getEffectiveGroups().getKnownGroups())); if (!projectState.getProject().getState().permitsWrite()) { @@ -669,8 +671,6 @@ return; } - logger.atFine().log("Parsing %d commands", commands.size()); - List<ReceiveCommand> magicCommands = new ArrayList<>(); List<ReceiveCommand> regularCommands = new ArrayList<>();
diff --git a/java/com/google/gerrit/server/index/change/ChangeField.java b/java/com/google/gerrit/server/index/change/ChangeField.java index 9c39c6e..810cd4d 100644 --- a/java/com/google/gerrit/server/index/change/ChangeField.java +++ b/java/com/google/gerrit/server/index/change/ChangeField.java
@@ -43,11 +43,13 @@ import com.google.common.flogger.FluentLogger; import com.google.common.io.Files; import com.google.common.primitives.Longs; +import com.google.gerrit.common.Nullable; import com.google.gerrit.entities.Account; import com.google.gerrit.entities.Address; import com.google.gerrit.entities.AttentionSetUpdate; import com.google.gerrit.entities.Change; import com.google.gerrit.entities.ChangeMessage; +import com.google.gerrit.entities.LabelType; import com.google.gerrit.entities.LegacySubmitRequirement; import com.google.gerrit.entities.PatchSetApproval; import com.google.gerrit.entities.Project; @@ -72,6 +74,7 @@ import com.google.gerrit.server.query.change.ChangeData; import com.google.gerrit.server.query.change.ChangeQueryBuilder; import com.google.gerrit.server.query.change.ChangeStatusPredicate; +import com.google.gerrit.server.query.change.MagicLabelValue; import com.google.gson.Gson; import com.google.protobuf.MessageLite; import java.sql.Timestamp; @@ -602,16 +605,35 @@ for (PatchSetApproval a : cd.currentApprovals()) { if (a.value() != 0 && !a.isLegacySubmit()) { allApprovals.add(formatLabel(a.label(), a.value(), a.accountId())); + LabelType labelType = cd.getLabelTypes().byLabel(a.labelId()); + allApprovals.addAll(getMaxMinAnyLabels(a.label(), a.value(), labelType, a.accountId())); if (owners && cd.change().getOwner().equals(a.accountId())) { allApprovals.add(formatLabel(a.label(), a.value(), ChangeQueryBuilder.OWNER_ACCOUNT_ID)); + allApprovals.addAll( + getMaxMinAnyLabels( + a.label(), a.value(), labelType, ChangeQueryBuilder.OWNER_ACCOUNT_ID)); } distinctApprovals.add(formatLabel(a.label(), a.value())); + distinctApprovals.addAll(getMaxMinAnyLabels(a.label(), a.value(), labelType, null)); } } allApprovals.addAll(distinctApprovals); return allApprovals; } + private static List<String> getMaxMinAnyLabels( + String label, short labelVal, LabelType labelType, @Nullable Account.Id accountId) { + List<String> labels = new ArrayList<>(); + if (labelVal == labelType.getMaxPositive()) { + labels.add(formatLabel(label, MagicLabelValue.MAX.name(), accountId)); + } + if (labelVal == labelType.getMaxNegative()) { + labels.add(formatLabel(label, MagicLabelValue.MIN.name(), accountId)); + } + labels.add(formatLabel(label, MagicLabelValue.ANY.name(), accountId)); + return labels; + } + public static Set<String> getAuthorParts(ChangeData cd) { return SchemaUtil.getPersonParts(cd.getAuthor()); } @@ -696,6 +718,17 @@ + (accountId != null ? "," + formatAccount(accountId) : ""); } + public static String formatLabel(String label, String value) { + return formatLabel(label, value, null); + } + + public static String formatLabel(String label, String value, @Nullable Account.Id accountId) { + return label.toLowerCase() + + "=" + + value + + (accountId != null ? "," + formatAccount(accountId) : ""); + } + private static String formatAccount(Account.Id accountId) { if (ChangeQueryBuilder.OWNER_ACCOUNT_ID.equals(accountId)) { return ChangeQueryBuilder.ARG_ID_OWNER;
diff --git a/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java b/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java index 6355674..879da4f 100644 --- a/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java +++ b/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java
@@ -150,6 +150,9 @@ */ static final Schema<ChangeData> V63 = schema(V62, false); + /** Added support for MIN/MAX/ANY for {@link ChangeField#LABEL} */ + static final Schema<ChangeData> V64 = schema(V63, false); + /** * Name of the change index to be used when contacting index backends or loading configurations. */
diff --git a/java/com/google/gerrit/server/project/ProjectConfig.java b/java/com/google/gerrit/server/project/ProjectConfig.java index 3253282..0d710b9 100644 --- a/java/com/google/gerrit/server/project/ProjectConfig.java +++ b/java/com/google/gerrit/server/project/ProjectConfig.java
@@ -126,12 +126,12 @@ public static final String KEY_CAN_OVERRIDE = "canOverride"; public static final String KEY_BRANCH = "branch"; - public static final String SUBMIT_REQUIREMENT = "submitRequirement"; + public static final String SUBMIT_REQUIREMENT = "submit-requirement"; public static final String KEY_SR_NAME = "name"; public static final String KEY_SR_DESCRIPTION = "description"; - public static final String KEY_SR_APPLICABILITY_EXPRESSION = "applicabilityExpression"; - public static final String KEY_SR_SUBMITTABILITY_EXPRESSION = "submittabilityExpression"; - public static final String KEY_SR_OVERRIDE_EXPRESSION = "overrideExpression"; + public static final String KEY_SR_APPLICABILITY_EXPRESSION = "applicableIf"; + public static final String KEY_SR_SUBMITTABILITY_EXPRESSION = "submittableIf"; + public static final String KEY_SR_OVERRIDE_EXPRESSION = "overrideIf"; public static final String KEY_SR_OVERRIDE_IN_CHILD_PROJECTS = "canOverrideInChildProjects"; public static final String KEY_MATCH = "match";
diff --git a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java index 94b5442..131de74 100644 --- a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java +++ b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
@@ -973,7 +973,7 @@ int eq = name.indexOf('='); if (args.getSchema().hasField(ChangeField.SUBMIT_RECORD) && eq > 0) { String statusName = name.substring(eq + 1).toUpperCase(); - if (!isInt(statusName)) { + if (!isInt(statusName) && !MagicLabelValue.tryParse(statusName).isPresent()) { SubmitRecord.Label.Status status = Enums.getIfPresent(SubmitRecord.Label.Status.class, statusName).orNull(); if (status == null) {
diff --git a/java/com/google/gerrit/server/query/change/LabelPredicate.java b/java/com/google/gerrit/server/query/change/LabelPredicate.java index 38d1dbe..989b4bb 100644 --- a/java/com/google/gerrit/server/query/change/LabelPredicate.java +++ b/java/com/google/gerrit/server/query/change/LabelPredicate.java
@@ -14,6 +14,7 @@ package com.google.gerrit.server.query.change; +import com.google.common.collect.ImmutableList; import com.google.common.collect.Lists; import com.google.gerrit.entities.Account; import com.google.gerrit.entities.AccountGroup; @@ -59,12 +60,12 @@ protected static class Parsed { protected final String label; protected final String test; - protected final int expVal; + protected final int numericValue; - protected Parsed(String label, String test, int expVal) { + protected Parsed(String label, String test, int numericValue) { this.label = label; this.test = test; - this.expVal = expVal; + this.numericValue = numericValue; } } @@ -83,6 +84,14 @@ protected static List<Predicate<ChangeData>> predicates(Args args) { String v = args.value; + + try { + MagicLabelVote mlv = MagicLabelVote.parseWithEquals(v); + return ImmutableList.of(new MagicLabelPredicate(args, mlv)); + } catch (IllegalArgumentException e) { + // Try next format. + } + Parsed parsed = null; try { @@ -108,7 +117,7 @@ } else { range = RangeUtil.getRange( - parsed.label, parsed.test, parsed.expVal, -MAX_LABEL_VALUE, MAX_LABEL_VALUE); + parsed.label, parsed.test, parsed.numericValue, -MAX_LABEL_VALUE, MAX_LABEL_VALUE); } String prefix = range.prefix; int min = range.min;
diff --git a/java/com/google/gerrit/server/query/change/MagicLabelPredicate.java b/java/com/google/gerrit/server/query/change/MagicLabelPredicate.java new file mode 100644 index 0000000..e3c58e47 --- /dev/null +++ b/java/com/google/gerrit/server/query/change/MagicLabelPredicate.java
@@ -0,0 +1,101 @@ +// Copyright (C) 2021 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.gerrit.server.query.change; + +import com.google.gerrit.entities.Change; +import com.google.gerrit.entities.LabelType; +import com.google.gerrit.entities.LabelTypes; +import com.google.gerrit.entities.LabelValue; +import com.google.gerrit.index.query.Predicate; +import com.google.gerrit.server.index.change.ChangeField; +import com.google.gerrit.server.project.ProjectState; +import java.util.ArrayList; +import java.util.List; +import java.util.Optional; + +public class MagicLabelPredicate extends ChangeIndexPredicate { + protected final LabelPredicate.Args args; + private final MagicLabelVote magicLabelVote; + + public MagicLabelPredicate(LabelPredicate.Args args, MagicLabelVote magicLabelVote) { + super(ChangeField.LABEL, magicLabelVote.formatLabel()); + this.args = args; + this.magicLabelVote = magicLabelVote; + } + + @Override + public boolean match(ChangeData changeData) { + Change change = changeData.change(); + if (change == null) { + // The change has disappeared. + // + return false; + } + + Optional<ProjectState> project = args.projectCache.get(change.getDest().project()); + if (!project.isPresent()) { + // The project has disappeared. + // + return false; + } + + LabelType labelType = type(project.get().getLabelTypes(), magicLabelVote.label()); + if (labelType == null) { + return false; // Label is not defined by this project. + } + + switch (magicLabelVote.value()) { + case ANY: + return matchAny(changeData, labelType); + case MIN: + return matchNumeric(changeData, magicLabelVote.label(), labelType.getMin().getValue()); + case MAX: + return matchNumeric(changeData, magicLabelVote.label(), labelType.getMax().getValue()); + } + + throw new IllegalStateException("Unsupported magic label value: " + magicLabelVote.value()); + } + + private boolean matchAny(ChangeData changeData, LabelType labelType) { + List<Predicate<ChangeData>> predicates = new ArrayList<>(); + for (LabelValue labelValue : labelType.getValues()) { + if (labelValue.getValue() != 0) { + predicates.add(numericPredicate(labelType.getName(), labelValue.getValue())); + } + } + return or(predicates).asMatchable().match(changeData); + } + + private boolean matchNumeric(ChangeData changeData, String label, short value) { + return numericPredicate(label, value).match(changeData); + } + + private EqualsLabelPredicate numericPredicate(String label, short value) { + return new EqualsLabelPredicate(args, label, value, /* account= */ null); + } + + protected static LabelType type(LabelTypes types, String toFind) { + if (types.byLabel(toFind) != null) { + return types.byLabel(toFind); + } + + for (LabelType lt : types.getLabelTypes()) { + if (toFind.equalsIgnoreCase(lt.getName())) { + return lt; + } + } + return null; + } +}
diff --git a/java/com/google/gerrit/server/query/change/MagicLabelValue.java b/java/com/google/gerrit/server/query/change/MagicLabelValue.java new file mode 100644 index 0000000..c4bcbe3 --- /dev/null +++ b/java/com/google/gerrit/server/query/change/MagicLabelValue.java
@@ -0,0 +1,31 @@ +// Copyright (C) 2021 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.gerrit.server.query.change; + +import java.util.Optional; + +public enum MagicLabelValue { + ANY, + MIN, + MAX; + + public static Optional<MagicLabelValue> tryParse(String value) { + try { + return Optional.of(MagicLabelValue.valueOf(value)); + } catch (IllegalArgumentException e) { + return Optional.empty(); + } + } +}
diff --git a/java/com/google/gerrit/server/query/change/MagicLabelVote.java b/java/com/google/gerrit/server/query/change/MagicLabelVote.java new file mode 100644 index 0000000..c29ac72 --- /dev/null +++ b/java/com/google/gerrit/server/query/change/MagicLabelVote.java
@@ -0,0 +1,47 @@ +// Copyright (C) 2021 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.gerrit.server.query.change; + +import static com.google.common.base.Preconditions.checkArgument; + +import com.google.auto.value.AutoValue; +import com.google.common.base.Strings; +import com.google.gerrit.entities.LabelType; +import java.util.Locale; + +/** An entity representing a special label vote that's not numeric, e.g. MAX, MIN, etc... */ +@AutoValue +public abstract class MagicLabelVote { + public static MagicLabelVote parseWithEquals(String text) { + checkArgument(!Strings.isNullOrEmpty(text), "Empty label vote"); + int e = text.lastIndexOf('='); + checkArgument(e >= 0, "Label vote missing '=': %s", text); + String label = text.substring(0, e); + String voteValue = text.substring(e + 1); + return create(label, MagicLabelValue.valueOf(voteValue)); + } + + public static MagicLabelVote create(String label, MagicLabelValue value) { + return new AutoValue_MagicLabelVote(LabelType.checkNameInternal(label), value); + } + + public abstract String label(); + + public abstract MagicLabelValue value(); + + public String formatLabel() { + return label().toLowerCase(Locale.US) + "=" + value().name(); + } +}
diff --git a/java/com/google/gerrit/server/restapi/change/ReplyAttentionSetUpdates.java b/java/com/google/gerrit/server/restapi/change/ReplyAttentionSetUpdates.java index 0356cdd..53d0f18 100644 --- a/java/com/google/gerrit/server/restapi/change/ReplyAttentionSetUpdates.java +++ b/java/com/google/gerrit/server/restapi/change/ReplyAttentionSetUpdates.java
@@ -19,7 +19,9 @@ import com.google.common.collect.ImmutableSet; import com.google.common.collect.Sets; import com.google.common.collect.Sets.SetView; +import com.google.common.flogger.FluentLogger; import com.google.gerrit.entities.Account; +import com.google.gerrit.entities.AttentionSetUpdate; import com.google.gerrit.entities.HumanComment; import com.google.gerrit.entities.PatchSet; import com.google.gerrit.extensions.api.changes.AttentionSetInput; @@ -60,6 +62,7 @@ * This class is used to update the attention set when performing a review or replying on a change. */ public class ReplyAttentionSetUpdates { + private static final FluentLogger logger = FluentLogger.forEnclosingClass(); private final PermissionBackend permissionBackend; private final AddToAttentionSetOp.Factory addToAttentionSetOpFactory; @@ -316,11 +319,15 @@ AttentionSetUtil.validateInput(add); try { Account.Id attentionUserId = - getAccountIdAndValidateUser(changeNotes, add.user, accountsChangedInCommit); + getAccountIdAndValidateUser( + changeNotes, add.user, accountsChangedInCommit, AttentionSetUpdate.Operation.ADD); addToAttentionSet(bu, changeNotes, attentionUserId, add.reason, false); } catch (AccountResolver.UnresolvableAccountException ex) { // This happens only when the account doesn't exist. Silently ignore it. If we threw an error // message here, then it would be possible to probe whether an account exists. + } catch (AuthException ex) { + // adding users without permission to the attention set should fail silently. + logger.atFine().log(ex.getMessage()); } } @@ -334,17 +341,25 @@ AttentionSetUtil.validateInput(remove); try { Account.Id attentionUserId = - getAccountIdAndValidateUser(changeNotes, remove.user, accountsChangedInCommit); + getAccountIdAndValidateUser( + changeNotes, + remove.user, + accountsChangedInCommit, + AttentionSetUpdate.Operation.REMOVE); removeFromAttentionSet(bu, changeNotes, attentionUserId, remove.reason, false); } catch (AccountResolver.UnresolvableAccountException ex) { // This happens only when the account doesn't exist. Silently ignore it. If we threw an error // message here, then it would be possible to probe whether an account exists. + } catch (AuthException ex) { + // this should never happen since removing users with permissions should work. + logger.atSevere().log(ex.getMessage()); } } - private Account.Id getAccountId(ChangeNotes changeNotes, String user) + private Account.Id getAccountId( + ChangeNotes changeNotes, String user, AttentionSetUpdate.Operation operation) throws ConfigInvalidException, IOException, UnprocessableEntityException, - PermissionBackendException { + PermissionBackendException, AuthException { Account.Id attentionUserId = accountResolver.resolve(user).asUnique().account().id(); try { permissionBackend @@ -352,22 +367,29 @@ .change(changeNotes) .check(ChangePermission.READ); } catch (AuthException e) { + // If the change is private, it is okay to add the user to the attention set since that + // person will be granted visibility when a reviewer. if (!changeNotes.getChange().isPrivate()) { - // If the change is private, it is okay to add the user to the attention set since that - // person will be granted visibility when a reviewer. - throw new UnprocessableEntityException( - "Can't add to attention set: Read not permitted for " + attentionUserId, e); + + // Removing users without access is allowed, adding is not allowed + if (operation == AttentionSetUpdate.Operation.ADD) { + throw new AuthException( + "Can't modify attention set: Read not permitted for " + attentionUserId, e); + } } } return attentionUserId; } private Account.Id getAccountIdAndValidateUser( - ChangeNotes changeNotes, String user, Set<Account.Id> accountsChangedInCommit) + ChangeNotes changeNotes, + String user, + Set<Account.Id> accountsChangedInCommit, + AttentionSetUpdate.Operation operation) throws ConfigInvalidException, IOException, PermissionBackendException, - UnprocessableEntityException, BadRequestException { + UnprocessableEntityException, BadRequestException, AuthException { try { - Account.Id attentionUserId = getAccountId(changeNotes, user); + Account.Id attentionUserId = getAccountId(changeNotes, user, operation); if (accountsChangedInCommit.contains(attentionUserId)) { throw new BadRequestException( String.format(
diff --git a/java/com/google/gerrit/server/rules/PrologRuleEvaluator.java b/java/com/google/gerrit/server/rules/PrologRuleEvaluator.java index 3dc5be0..179a3d0 100644 --- a/java/com/google/gerrit/server/rules/PrologRuleEvaluator.java +++ b/java/com/google/gerrit/server/rules/PrologRuleEvaluator.java
@@ -158,6 +158,8 @@ return ruleError("Error looking up change " + cd.getId(), e); } + logger.atFine().log("input approvals: %s", cd.approvals()); + List<Term> results; try { results = @@ -178,7 +180,9 @@ getSubmitRuleName(), cd.getId(), projectState.getName())); } - return resultsToSubmitRecord(getSubmitRule(), results); + SubmitRecord submitRecord = resultsToSubmitRecord(getSubmitRule(), results); + logger.atFine().log("submit record: %s", submitRecord); + return submitRecord; } private String getSubmitRuleName() { @@ -320,6 +324,7 @@ logger.atSevere().withCause(e).log(err); return createRuleError(DEFAULT_MSG); } + logger.atFine().log("rule error: %s", err); return createRuleError(err); }
diff --git a/java/com/google/gerrit/server/submit/SubscriptionGraph.java b/java/com/google/gerrit/server/submit/SubscriptionGraph.java index ad16cb0..26c8ac9 100644 --- a/java/com/google/gerrit/server/submit/SubscriptionGraph.java +++ b/java/com/google/gerrit/server/submit/SubscriptionGraph.java
@@ -231,7 +231,6 @@ Map<BranchNameKey, GitModules> branchGitModules, MergeOpRepoManager orm) throws SubmoduleConflictException { - logger.atFine().log("Calculating superprojects - submodules map"); LinkedHashSet<BranchNameKey> allVisited = new LinkedHashSet<>(); for (BranchNameKey updatedBranch : updatedBranches) { if (allVisited.contains(updatedBranch)) { @@ -332,7 +331,6 @@ Map<BranchNameKey, GitModules> branchGitModules, MergeOpRepoManager orm) throws IOException { - logger.atFine().log("Calculating possible superprojects for %s", srcBranch); Collection<SubmoduleSubscription> ret = new ArrayList<>(); Project.NameKey srcProject = srcBranch.project(); for (SubscribeSection s : @@ -340,7 +338,6 @@ .get(srcProject) .orElseThrow(illegalState(srcProject)) .getSubscribeSections(srcBranch)) { - logger.atFine().log("Checking subscribe section %s", s); Collection<BranchNameKey> branches = getDestinationBranches(srcBranch, s, orm); for (BranchNameKey targetBranch : branches) { Project.NameKey targetProject = targetBranch.project(); @@ -348,11 +345,11 @@ OpenRepo or = orm.getRepo(targetProject); ObjectId id = or.repo.resolve(targetBranch.branch()); if (id == null) { - logger.atFine().log("The branch %s doesn't exist.", targetBranch); + logger.atFine().log("SubscribeSection %s: branch %s doesn't exist.", s, targetBranch); continue; } } catch (NoSuchProjectException e) { - logger.atFine().log("The project %s doesn't exist", targetProject); + logger.atFine().log("SubscribeSection %s: project %s doesn't exist", s, targetProject); continue; }
diff --git a/java/com/google/gerrit/server/update/BatchUpdate.java b/java/com/google/gerrit/server/update/BatchUpdate.java index 3b0cd9a..f558d30 100644 --- a/java/com/google/gerrit/server/update/BatchUpdate.java +++ b/java/com/google/gerrit/server/update/BatchUpdate.java
@@ -75,6 +75,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Map.Entry; import java.util.Objects; import java.util.Optional; import java.util.TimeZone; @@ -553,11 +554,15 @@ try { logDebug("Executing updateRepo on %d ops", ops.size()); RepoContextImpl ctx = new RepoContextImpl(); - for (BatchUpdateOp op : ops.values()) { + for (Entry<Change.Id, BatchUpdateOp> op : ops.entries()) { try (TraceContext.TraceTimer ignored = TraceContext.newTimer( - op.getClass().getSimpleName() + "#updateRepo", Metadata.empty())) { - op.updateRepo(ctx); + op.getClass().getSimpleName() + "#updateRepo", + Metadata.builder() + .projectName(project.get()) + .changeId(op.getKey().get()) + .build())) { + op.getValue().updateRepo(ctx); } } @@ -672,7 +677,8 @@ for (BatchUpdateOp op : e.getValue()) { try (TraceContext.TraceTimer ignored = TraceContext.newTimer( - op.getClass().getSimpleName() + "#updateChange", Metadata.empty())) { + op.getClass().getSimpleName() + "#updateChange", + Metadata.builder().projectName(project.get()).changeId(id.get()).build())) { dirty |= op.updateChange(ctx); } }
diff --git a/java/com/google/gerrit/truth/NullAwareCorrespondence.java b/java/com/google/gerrit/truth/NullAwareCorrespondence.java index 687ad94..5b107a6 100644 --- a/java/com/google/gerrit/truth/NullAwareCorrespondence.java +++ b/java/com/google/gerrit/truth/NullAwareCorrespondence.java
@@ -7,15 +7,6 @@ // http://www.apache.org/licenses/LICENSE-2.0 // // Unless required by applicable law or agreed to in writing, software -// 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
diff --git a/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java b/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java index d091ae1..9e51054 100644 --- a/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java +++ b/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java
@@ -47,8 +47,8 @@ import static org.eclipse.jgit.lib.Constants.OBJ_BLOB; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.verifyNoMoreInteractions; -import static org.mockito.Mockito.verifyZeroInteractions; import com.github.rholder.retry.StopStrategies; import com.google.common.collect.FluentIterable; @@ -686,7 +686,7 @@ () -> gApi.accounts().id(activatableAccountId.get()).setActive(false)); assertThat(thrown).hasMessageThat().isEqualTo("account not active"); assertThat(accountOperations.account(activatableAccountId).get().active()).isFalse(); - verifyZeroInteractions(listener); + verifyNoInteractions(listener); // Activate account that can be activated gApi.accounts().id(activatableAccountId.get()).setActive(true); @@ -697,7 +697,7 @@ // Activate account that is already active gApi.accounts().id(activatableAccountId.get()).setActive(true); assertThat(accountOperations.account(activatableAccountId).get().active()).isTrue(); - verifyZeroInteractions(listener); + verifyNoMoreInteractions(listener); // Try deactivating account that cannot be deactivated thrown = @@ -706,13 +706,13 @@ () -> gApi.accounts().id(activatableAccountId.get()).setActive(false)); assertThat(thrown).hasMessageThat().isEqualTo("not allowed to deactive account"); assertThat(accountOperations.account(activatableAccountId).get().active()).isTrue(); - verifyZeroInteractions(listener); + verifyNoMoreInteractions(listener); /* Test account that can be deactivated, but not activated */ // Activate account that is already inactive gApi.accounts().id(deactivatableAccountId.get()).setActive(true); assertThat(accountOperations.account(deactivatableAccountId).get().active()).isTrue(); - verifyZeroInteractions(listener); + verifyNoMoreInteractions(listener); // Deactivate account that can be deactivated gApi.accounts().id(deactivatableAccountId.get()).setActive(false); @@ -727,7 +727,7 @@ () -> gApi.accounts().id(deactivatableAccountId.get()).setActive(false)); assertThat(thrown).hasMessageThat().isEqualTo("account not active"); assertThat(accountOperations.account(deactivatableAccountId).get().active()).isFalse(); - verifyZeroInteractions(listener); + verifyNoMoreInteractions(listener); // Try activating account that cannot be activated thrown = @@ -736,7 +736,7 @@ () -> gApi.accounts().id(deactivatableAccountId.get()).setActive(true)); assertThat(thrown).hasMessageThat().isEqualTo("not allowed to active account"); assertThat(accountOperations.account(deactivatableAccountId).get().active()).isFalse(); - verifyZeroInteractions(listener); + verifyNoMoreInteractions(listener); } }
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java index fcc0dec..c08aa7f 100644 --- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java +++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -4024,6 +4024,86 @@ } @Test + public void submitRequirement_withLabelEqualsMax() throws Exception { + configSubmitRequirement( + project, + SubmitRequirement.builder() + .setName("code-review") + .setSubmittabilityExpression( + SubmitRequirementExpression.create("label:code-review=MAX")) + .setAllowOverrideInChildProjects(false) + .build()); + + PushOneCommit.Result r = createChange(); + String changeId = r.getChangeId(); + + ChangeInfo change = gApi.changes().id(changeId).get(); + assertThat(change.submitRequirements).hasSize(1); + assertSubmitRequirementStatus(change.submitRequirements, "code-review", Status.UNSATISFIED); + + voteLabel(changeId, "code-review", 2); + change = gApi.changes().id(changeId).get(); + assertThat(change.submitRequirements).hasSize(1); + assertSubmitRequirementStatus(change.submitRequirements, "code-review", Status.SATISFIED); + } + + @Test + public void submitRequirement_withLabelEqualsMinBlockingSubmission() throws Exception { + configSubmitRequirement( + project, + SubmitRequirement.builder() + .setName("code-review") + .setSubmittabilityExpression( + SubmitRequirementExpression.create("-label:code-review=MIN")) + .setAllowOverrideInChildProjects(false) + .build()); + + PushOneCommit.Result r = createChange(); + String changeId = r.getChangeId(); + + ChangeInfo change = gApi.changes().id(changeId).get(); + assertThat(change.submitRequirements).hasSize(1); + // Requirement is satisfied because there are no votes + assertSubmitRequirementStatus(change.submitRequirements, "code-review", Status.SATISFIED); + + voteLabel(changeId, "code-review", -1); + change = gApi.changes().id(changeId).get(); + assertThat(change.submitRequirements).hasSize(1); + // Requirement is still satisfied because -1 is not the max negative value + assertSubmitRequirementStatus(change.submitRequirements, "code-review", Status.SATISFIED); + + voteLabel(changeId, "code-review", -2); + change = gApi.changes().id(changeId).get(); + assertThat(change.submitRequirements).hasSize(1); + // Requirement is now unsatisfied because -2 is the max negative value + assertSubmitRequirementStatus(change.submitRequirements, "code-review", Status.UNSATISFIED); + } + + @Test + public void submitRequirement_withLabelEqualsAny() throws Exception { + configSubmitRequirement( + project, + SubmitRequirement.builder() + .setName("code-review") + .setSubmittabilityExpression( + SubmitRequirementExpression.create("label:code-review=ANY")) + .setAllowOverrideInChildProjects(false) + .build()); + + PushOneCommit.Result r = createChange(); + String changeId = r.getChangeId(); + + ChangeInfo change = gApi.changes().id(changeId).get(); + assertThat(change.submitRequirements).hasSize(1); + assertSubmitRequirementStatus(change.submitRequirements, "code-review", Status.UNSATISFIED); + + voteLabel(changeId, "code-review", 1); + change = gApi.changes().id(changeId).get(); + assertThat(change.submitRequirements).hasSize(1); + assertSubmitRequirementStatus(change.submitRequirements, "code-review", Status.SATISFIED); + } + + @Test public void submitRequirementIsSatisfied_whenSubmittabilityExpressionIsFulfilled() throws Exception { configSubmitRequirement(
diff --git a/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java b/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java index 9d0b1f4..4590d34 100644 --- a/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java +++ b/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java
@@ -1207,10 +1207,15 @@ ChangeApi originalChange = gApi.changes().id(project.get() + "~master~" + result.getChangeId()); ChangeApi cherryPick = originalChange.revision(result.getCommit().name()).cherryPick(input); + String firstCherryPickChangeId = cherryPick.id(); cherryPick.setWorkInProgress(); - cherryPick = originalChange.revision(result.getCommit().name()).cherryPick(input); + gApi.changes() + .id(project.get() + "~master~" + result.getChangeId()) + .revision(result.getCommit().name()) + .cherryPick(input); - ChangeInfo secondCherryPickResult = cherryPick.get(ALL_REVISIONS); + ChangeInfo secondCherryPickResult = + gApi.changes().id(firstCherryPickChangeId).get(ALL_REVISIONS); assertThat(secondCherryPickResult.revisions).hasSize(2); assertThat(secondCherryPickResult.workInProgress).isNull(); } @@ -1416,22 +1421,12 @@ BadRequestException thrown = assertThrows( BadRequestException.class, - () -> - gApi.changes() - .id(r.getChangeId()) - .revision(r.getCommit().name()) - .files(3) - .keySet()); + () -> gApi.changes().id(r.getChangeId()).revision(r.getCommit().name()).files(3)); assertThat(thrown).hasMessageThat().isEqualTo("invalid parent number: 3"); thrown = assertThrows( BadRequestException.class, - () -> - gApi.changes() - .id(r.getChangeId()) - .revision(r.getCommit().name()) - .files(-1) - .keySet()); + () -> gApi.changes().id(r.getChangeId()).revision(r.getCommit().name()).files(-1)); assertThat(thrown).hasMessageThat().isEqualTo("invalid parent number: -1"); } @@ -1444,14 +1439,13 @@ BadRequestException thrown = assertThrows( - BadRequestException.class, - () -> gApi.changes().id(changeId).revision(revId2).files(2).keySet()); + BadRequestException.class, () -> gApi.changes().id(changeId).revision(revId2).files(2)); assertThat(thrown).hasMessageThat().isEqualTo("invalid parent number: 2"); thrown = assertThrows( BadRequestException.class, - () -> gApi.changes().id(changeId).revision(revId2).files(-1).keySet()); + () -> gApi.changes().id(changeId).revision(revId2).files(-1)); assertThat(thrown).hasMessageThat().isEqualTo("invalid parent number: -1"); }
diff --git a/javatests/com/google/gerrit/acceptance/git/RefAdvertisementIT.java b/javatests/com/google/gerrit/acceptance/git/RefAdvertisementIT.java index b7acbe2..92770ba 100644 --- a/javatests/com/google/gerrit/acceptance/git/RefAdvertisementIT.java +++ b/javatests/com/google/gerrit/acceptance/git/RefAdvertisementIT.java
@@ -38,7 +38,6 @@ import com.google.gerrit.common.data.GlobalCapability; import com.google.gerrit.entities.AccessSection; import com.google.gerrit.entities.AccountGroup; -import com.google.gerrit.entities.Change; import com.google.gerrit.entities.Patch; import com.google.gerrit.entities.PatchSet; import com.google.gerrit.entities.Permission; @@ -52,7 +51,6 @@ import com.google.gerrit.server.config.AllUsersName; import com.google.gerrit.server.git.receive.ReceiveCommitsAdvertiseRefsHookChain; import com.google.gerrit.server.git.receive.testing.TestRefAdvertiser; -import com.google.gerrit.server.notedb.ChangeNoteUtil; import com.google.gerrit.server.notedb.Sequences; import com.google.gerrit.server.permissions.PermissionBackend; import com.google.gerrit.server.permissions.PermissionBackend.RefFilterOptions; @@ -69,7 +67,6 @@ import org.eclipse.jgit.lib.Config; import org.eclipse.jgit.lib.Constants; import org.eclipse.jgit.lib.ObjectId; -import org.eclipse.jgit.lib.PersonIdent; import org.eclipse.jgit.lib.Ref; import org.eclipse.jgit.lib.RefUpdate; import org.eclipse.jgit.lib.Repository; @@ -82,7 +79,6 @@ @NoHttpd public class RefAdvertisementIT extends AbstractDaemonTest { @Inject private AllUsersName allUsersName; - @Inject private ChangeNoteUtil noteUtil; @Inject private PermissionBackend permissionBackend; @Inject private ProjectOperations projectOperations; @Inject private RequestScopeOperations requestScopeOperations; @@ -1106,37 +1102,10 @@ @Test public void receivePackOmitsMissingObject() throws Exception { - String rev = "deadbeefdeadbeefdeadbeefdeadbeefdeadbeef"; try (Repository repo = repoManager.openRepository(project); TestRepository<Repository> tr = new TestRepository<>(repo)) { - String subject = "Subject for missing commit"; - Change c = new Change(cd3.change()); - PatchSet.Id psId = PatchSet.id(cd3.getId(), 2); - c.setCurrentPatchSet(psId, subject, c.getOriginalSubject()); - - PersonIdent committer = serverIdent.get(); - PersonIdent author = - noteUtil.newAccountIdIdent(getAccount(admin.id()).id(), committer.getWhen(), committer); - tr.branch(RefNames.changeMetaRef(cd3.getId())) - .commit() - .author(author) - .committer(committer) - .message( - "Update patch set " - + psId.get() - + "\n" - + "\n" - + "Patch-set: " - + psId.get() - + "\n" - + "Commit: " - + rev - + "\n" - + "Subject: " - + subject - + "\n") - .create(); - indexer.index(c.getProject(), c.getId()); + PatchSet.Id psId = PatchSet.id(cd3.getId(), 1); + tr.delete(psId.toRefName()); } assertThat(getReceivePackRefs().additionalHaves()).containsExactly(obj(cd4, 1));
diff --git a/javatests/com/google/gerrit/acceptance/pgm/AbstractReindexTests.java b/javatests/com/google/gerrit/acceptance/pgm/AbstractReindexTests.java index cad0b83..cac376f 100644 --- a/javatests/com/google/gerrit/acceptance/pgm/AbstractReindexTests.java +++ b/javatests/com/google/gerrit/acceptance/pgm/AbstractReindexTests.java
@@ -31,14 +31,18 @@ import com.google.gerrit.entities.Project; import com.google.gerrit.extensions.api.GerritApi; import com.google.gerrit.extensions.common.ChangeInput; +import com.google.gerrit.index.IndexDefinition; import com.google.gerrit.launcher.GerritLauncher; import com.google.gerrit.server.index.GerritIndexStatus; import com.google.gerrit.server.index.change.ChangeIndexCollection; import com.google.gerrit.server.index.change.ChangeSchemaDefinitions; import com.google.gerrit.server.query.change.InternalChangeQuery; import com.google.inject.Injector; +import com.google.inject.Key; import com.google.inject.Provider; +import com.google.inject.TypeLiteral; import java.nio.file.Files; +import java.util.Collection; import java.util.Set; import java.util.function.Consumer; import org.eclipse.jgit.lib.Config; @@ -48,9 +52,6 @@ @NoHttpd public abstract class AbstractReindexTests extends StandaloneSiteTest { - /** @param injector injector */ - public abstract void configureIndex(Injector injector) throws Exception; - private static final String CHANGES = ChangeSchemaDefinitions.NAME; private Project.NameKey project; @@ -223,10 +224,18 @@ } } + protected static void createAllIndexes(Injector injector) { + Collection<IndexDefinition<?, ?, ?>> indexDefs = + injector.getInstance(Key.get(new TypeLiteral<Collection<IndexDefinition<?, ?, ?>>>() {})); + for (IndexDefinition<?, ?, ?> indexDef : indexDefs) { + indexDef.getIndexCollection().getSearchIndex().deleteAll(); + } + } + private void setUpChange() throws Exception { project = Project.nameKey("reindex-project-test"); try (ServerContext ctx = startServer()) { - configureIndex(ctx.getInjector()); + createAllIndexes(ctx.getInjector()); GerritApi gApi = ctx.getInjector().getInstance(GerritApi.class); gApi.projects().create(project.get());
diff --git a/javatests/com/google/gerrit/acceptance/pgm/ElasticReindexIT.java b/javatests/com/google/gerrit/acceptance/pgm/ElasticReindexIT.java index f23cc10..0632241 100644 --- a/javatests/com/google/gerrit/acceptance/pgm/ElasticReindexIT.java +++ b/javatests/com/google/gerrit/acceptance/pgm/ElasticReindexIT.java
@@ -14,23 +14,15 @@ package com.google.gerrit.acceptance.pgm; -import static com.google.gerrit.elasticsearch.ElasticTestUtils.createAllIndexes; import static com.google.gerrit.elasticsearch.ElasticTestUtils.getConfig; import com.google.gerrit.elasticsearch.ElasticVersion; import com.google.gerrit.testing.ConfigSuite; -import com.google.inject.Injector; import org.eclipse.jgit.lib.Config; public class ElasticReindexIT extends AbstractReindexTests { - @ConfigSuite.Default public static Config elasticsearchV7() { return getConfig(ElasticVersion.V7_8); } - - @Override - public void configureIndex(Injector injector) { - createAllIndexes(injector); - } }
diff --git a/javatests/com/google/gerrit/acceptance/pgm/ReindexIT.java b/javatests/com/google/gerrit/acceptance/pgm/LuceneReindexIT.java similarity index 68% rename from javatests/com/google/gerrit/acceptance/pgm/ReindexIT.java rename to javatests/com/google/gerrit/acceptance/pgm/LuceneReindexIT.java index 223851e..e630bca 100644 --- a/javatests/com/google/gerrit/acceptance/pgm/ReindexIT.java +++ b/javatests/com/google/gerrit/acceptance/pgm/LuceneReindexIT.java
@@ -14,9 +14,14 @@ package com.google.gerrit.acceptance.pgm; -import com.google.inject.Injector; +import com.google.gerrit.testing.ConfigSuite; +import org.eclipse.jgit.lib.Config; -public class ReindexIT extends AbstractReindexTests { - @Override - public void configureIndex(Injector injector) {} +public class LuceneReindexIT extends AbstractReindexTests { + @ConfigSuite.Default + public static Config luceneConfig() { + Config cfg = new Config(); + cfg.setString("index", null, "type", "lucene"); + return cfg; + } }
diff --git a/javatests/com/google/gerrit/acceptance/rest/TraceIT.java b/javatests/com/google/gerrit/acceptance/rest/TraceIT.java index 4b780f8..530f2ec 100644 --- a/javatests/com/google/gerrit/acceptance/rest/TraceIT.java +++ b/javatests/com/google/gerrit/acceptance/rest/TraceIT.java
@@ -24,7 +24,7 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.timeout; import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyZeroInteractions; +import static org.mockito.Mockito.verifyNoInteractions; import com.google.auto.value.AutoValue; import com.google.common.collect.ImmutableList; @@ -409,7 +409,7 @@ PushOneCommit.Result r = push.to("refs/heads/master"); r.assertOkStatus(); - verifyZeroInteractions(testPerformanceLogger); + verifyNoInteractions(testPerformanceLogger); } }
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/AttentionSetIT.java b/javatests/com/google/gerrit/acceptance/rest/change/AttentionSetIT.java index 800ee42..4b45476 100644 --- a/javatests/com/google/gerrit/acceptance/rest/change/AttentionSetIT.java +++ b/javatests/com/google/gerrit/acceptance/rest/change/AttentionSetIT.java
@@ -15,6 +15,7 @@ package com.google.gerrit.acceptance.rest.change; import static com.google.common.truth.Truth.assertThat; +import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.block; import static com.google.gerrit.extensions.restapi.testing.AttentionSetUpdateSubject.assertThat; import static com.google.gerrit.testing.GerritJUnit.assertThrows; @@ -32,23 +33,29 @@ import com.google.gerrit.acceptance.config.GerritConfig; import com.google.gerrit.acceptance.testsuite.account.AccountOperations; import com.google.gerrit.acceptance.testsuite.change.ChangeOperations; +import com.google.gerrit.acceptance.testsuite.project.ProjectOperations; import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations; import com.google.gerrit.common.Nullable; import com.google.gerrit.entities.Account; +import com.google.gerrit.entities.AccountGroup; import com.google.gerrit.entities.AttentionSetUpdate; import com.google.gerrit.entities.AttentionSetUpdate.Operation; import com.google.gerrit.entities.Change; import com.google.gerrit.entities.LabelId; import com.google.gerrit.entities.Patch; +import com.google.gerrit.entities.Permission; import com.google.gerrit.extensions.api.changes.AttentionSetInput; import com.google.gerrit.extensions.api.changes.DeleteReviewerInput; import com.google.gerrit.extensions.api.changes.HashtagsInput; import com.google.gerrit.extensions.api.changes.ReviewInput; import com.google.gerrit.extensions.api.changes.ReviewerInput; +import com.google.gerrit.extensions.api.groups.GroupInput; import com.google.gerrit.extensions.client.GeneralPreferencesInfo; import com.google.gerrit.extensions.client.GeneralPreferencesInfo.EmailStrategy; import com.google.gerrit.extensions.client.ReviewerState; import com.google.gerrit.extensions.client.Side; +import com.google.gerrit.extensions.common.GroupInfo; +import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.extensions.restapi.BadRequestException; import com.google.gerrit.server.account.ServiceUserClassifier; import com.google.gerrit.server.query.change.ChangeData; @@ -82,6 +89,7 @@ @Inject private FakeEmailSender email; @Inject private TestCommentHelper testCommentHelper; @Inject private Provider<InternalChangeQuery> changeQueryProvider; + @Inject private ProjectOperations projectOperations; /** Simulates a fake clock. Uses second granularity. */ private static class FakeClock implements LongSupplier { @@ -622,7 +630,7 @@ assertThat(attentionSet).hasReasonThat().isEqualTo("reason"); // No emails for adding to attention set were sent. - email.getMessages().isEmpty(); + assertThat(email.getMessages()).isEmpty(); } @Test @@ -631,6 +639,7 @@ // implictly adds the user to the attention set when adding as reviewer change(r).addReviewer(user.email()); requestScopeOperations.setApiUser(user.id()); + email.clear(); ReviewInput reviewInput = ReviewInput.create().removeUserFromAttentionSet(user.email(), "reason"); @@ -643,7 +652,7 @@ assertThat(attentionSet).hasReasonThat().isEqualTo("reason"); // No emails for removing from attention set were sent. - email.getMessages().isEmpty(); + assertThat(email.getMessages()).isEmpty(); } @Test @@ -1834,6 +1843,44 @@ assertThat(attentionSet).hasReasonThat().isEqualTo("Their vote was deleted"); } + @Test + public void accountsWithNoReadPermissionIgnoredOnReply() throws Exception { + // Create a group with user. + GroupInput groupInput = new GroupInput(); + groupInput.name = name("User"); + groupInput.members = ImmutableList.of(String.valueOf(user.id())); + GroupInfo group = gApi.groups().create(groupInput).get(); + + PushOneCommit.Result r = createChange(); + gApi.changes().id(r.getChangeId()).addReviewer(user.email()); + + // remove read permission for user. + projectOperations + .project(project) + .forUpdate() + .add(block(Permission.READ).ref("refs/*").group(AccountGroup.uuid(group.id))) + .update(); + + // removing user without permissions from attention set is allowed on reply. + gApi.changes() + .id(r.getChangeId()) + .current() + .review(new ReviewInput().removeUserFromAttentionSet(user.email(), "reason")); + + // Add user to attention throws an exception. + assertThrows( + AuthException.class, + () -> change(r).addToAttentionSet(new AttentionSetInput(user.email(), "reason"))); + + // Add user to attention set is ignored on reply. + gApi.changes() + .id(r.getChangeId()) + .current() + .review(new ReviewInput().addUserToAttentionSet(user.email(), "reason")); + assertThat(Iterables.getOnlyElement(getAttentionSetUpdatesForUser(r, user)).operation()) + .isEqualTo(Operation.REMOVE); + } + private List<AttentionSetUpdate> getAttentionSetUpdatesForUser( PushOneCommit.Result r, TestAccount account) { return getAttentionSetUpdates(r.getChange().getId()).stream()
diff --git a/javatests/com/google/gerrit/acceptance/server/change/ConsistencyCheckerIT.java b/javatests/com/google/gerrit/acceptance/server/change/ConsistencyCheckerIT.java index e39f967..9d821b7 100644 --- a/javatests/com/google/gerrit/acceptance/server/change/ConsistencyCheckerIT.java +++ b/javatests/com/google/gerrit/acceptance/server/change/ConsistencyCheckerIT.java
@@ -257,7 +257,6 @@ + "Groups: " + rev + "\n"); - indexer.index(c.getProject(), c.getId()); ChangeNotes notes = changeNotesFactory.create(c.getProject(), c.getId()); FixInput fix = new FixInput(); @@ -817,8 +816,6 @@ + "Subject: " + subject + "\n"); - indexer.index(c.getProject(), c.getId()); - return ps; }
diff --git a/javatests/com/google/gerrit/acceptance/testsuite/index/DefaultIndexBindingIT.java b/javatests/com/google/gerrit/acceptance/testsuite/index/DefaultIndexBindingIT.java new file mode 100644 index 0000000..f5e4e09 --- /dev/null +++ b/javatests/com/google/gerrit/acceptance/testsuite/index/DefaultIndexBindingIT.java
@@ -0,0 +1,51 @@ +// Copyright (C) 2021 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.gerrit.acceptance.testsuite.index; + +import static com.google.common.truth.Truth.assertThat; + +import com.google.gerrit.acceptance.AbstractDaemonTest; +import com.google.gerrit.index.IndexType; +import com.google.gerrit.index.testing.AbstractFakeIndex; +import com.google.gerrit.server.index.change.ChangeIndexCollection; +import javax.inject.Inject; +import org.junit.AfterClass; +import org.junit.BeforeClass; +import org.junit.Test; + +/** Test to check that the expected index backend was bound depending on sys/env properties. */ +public class DefaultIndexBindingIT extends AbstractDaemonTest { + + @Inject private ChangeIndexCollection changeIndex; + + private static String propertyBeforeTest; + + @BeforeClass + public static void setup() { + propertyBeforeTest = System.getProperty(IndexType.SYS_PROP); + System.setProperty(IndexType.SYS_PROP, ""); + } + + @AfterClass + public static void teardown() { + System.setProperty(IndexType.SYS_PROP, propertyBeforeTest); + } + + @Test + public void fakeIsBoundByDefault() throws Exception { + assertThat(System.getProperty(IndexType.SYS_PROP)).isEmpty(); + assertThat(changeIndex.getSearchIndex()).isInstanceOf(AbstractFakeIndex.FakeChangeIndex.class); + } +}
diff --git a/javatests/com/google/gerrit/acceptance/testsuite/index/FakeIndexBindingIT.java b/javatests/com/google/gerrit/acceptance/testsuite/index/FakeIndexBindingIT.java new file mode 100644 index 0000000..4122426 --- /dev/null +++ b/javatests/com/google/gerrit/acceptance/testsuite/index/FakeIndexBindingIT.java
@@ -0,0 +1,51 @@ +// Copyright (C) 2021 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.gerrit.acceptance.testsuite.index; + +import static com.google.common.truth.Truth.assertThat; + +import com.google.gerrit.acceptance.AbstractDaemonTest; +import com.google.gerrit.index.IndexType; +import com.google.gerrit.index.testing.AbstractFakeIndex; +import com.google.gerrit.server.index.change.ChangeIndexCollection; +import javax.inject.Inject; +import org.junit.AfterClass; +import org.junit.BeforeClass; +import org.junit.Test; + +/** Test to check that the expected index backend was bound depending on sys/env properties. */ +public class FakeIndexBindingIT extends AbstractDaemonTest { + + @Inject private ChangeIndexCollection changeIndex; + + private static String propertyBeforeTest; + + @BeforeClass + public static void setup() { + propertyBeforeTest = System.getProperty(IndexType.SYS_PROP); + System.setProperty(IndexType.SYS_PROP, "fake"); + } + + @AfterClass + public static void teardown() { + System.setProperty(IndexType.SYS_PROP, propertyBeforeTest); + } + + @Test + public void fakeIsBoundWhenConfigured() throws Exception { + assertThat(System.getProperty(IndexType.SYS_PROP)).isEqualTo("fake"); + assertThat(changeIndex.getSearchIndex()).isInstanceOf(AbstractFakeIndex.FakeChangeIndex.class); + } +}
diff --git a/javatests/com/google/gerrit/acceptance/testsuite/index/LuceneIndexBindingIT.java b/javatests/com/google/gerrit/acceptance/testsuite/index/LuceneIndexBindingIT.java new file mode 100644 index 0000000..31e31fd --- /dev/null +++ b/javatests/com/google/gerrit/acceptance/testsuite/index/LuceneIndexBindingIT.java
@@ -0,0 +1,51 @@ +// Copyright (C) 2021 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.gerrit.acceptance.testsuite.index; + +import static com.google.common.truth.Truth.assertThat; + +import com.google.gerrit.acceptance.AbstractDaemonTest; +import com.google.gerrit.index.IndexType; +import com.google.gerrit.lucene.LuceneChangeIndex; +import com.google.gerrit.server.index.change.ChangeIndexCollection; +import javax.inject.Inject; +import org.junit.AfterClass; +import org.junit.BeforeClass; +import org.junit.Test; + +/** Test to check that the expected index backend was bound depending on sys/env properties. */ +public class LuceneIndexBindingIT extends AbstractDaemonTest { + + @Inject private ChangeIndexCollection changeIndex; + + private static String propertyBeforeTest; + + @BeforeClass + public static void setup() { + propertyBeforeTest = System.getProperty(IndexType.SYS_PROP); + System.setProperty(IndexType.SYS_PROP, "lucene"); + } + + @AfterClass + public static void teardown() { + System.setProperty(IndexType.SYS_PROP, propertyBeforeTest); + } + + @Test + public void luceneIsBoundWhenConfigured() throws Exception { + assertThat(System.getProperty(IndexType.SYS_PROP)).isEqualTo("lucene"); + assertThat(changeIndex.getSearchIndex()).isInstanceOf(LuceneChangeIndex.class); + } +}
diff --git a/javatests/com/google/gerrit/server/account/externalids/ExternalIDCacheLoaderTest.java b/javatests/com/google/gerrit/server/account/externalids/ExternalIDCacheLoaderTest.java index 2f64ed0..2b80601 100644 --- a/javatests/com/google/gerrit/server/account/externalids/ExternalIDCacheLoaderTest.java +++ b/javatests/com/google/gerrit/server/account/externalids/ExternalIDCacheLoaderTest.java
@@ -17,7 +17,7 @@ import static com.google.common.truth.Truth.assertThat; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyZeroInteractions; +import static org.mockito.Mockito.verifyNoInteractions; import com.google.common.cache.Cache; import com.google.common.cache.CacheBuilder; @@ -80,7 +80,7 @@ externalIdCache.put(firstState, allFromGit(firstState)); assertThat(loader.load(head)).isEqualTo(allFromGit(head)); - verifyZeroInteractions(externalIdReaderSpy); + verifyNoInteractions(externalIdReaderSpy); } @Test @@ -92,7 +92,7 @@ externalIdCache.put(firstState, allFromGit(firstState)); assertThat(loader.load(head)).isEqualTo(allFromGit(head)); - verifyZeroInteractions(externalIdReaderSpy); + verifyNoInteractions(externalIdReaderSpy); } @Test @@ -142,7 +142,7 @@ externalIdCache.put(firstState, allFromGit(firstState)); assertThat(loader.load(head)).isEqualTo(allFromGit(head)); - verifyZeroInteractions(externalIdReaderSpy); + verifyNoInteractions(externalIdReaderSpy); } @Test @@ -156,7 +156,7 @@ externalIdCache.put(firstState, allFromGit(firstState)); assertThat(loader.load(head)).isEqualTo(allFromGit(head)); - verifyZeroInteractions(externalIdReaderSpy); + verifyNoInteractions(externalIdReaderSpy); } @Test @@ -173,7 +173,7 @@ externalIdCache.put(firstState, allFromGit(firstState)); assertThat(loader.load(head)).isEqualTo(allFromGit(head)); - verifyZeroInteractions(externalIdReaderSpy); + verifyNoInteractions(externalIdReaderSpy); } @Test @@ -186,7 +186,7 @@ externalIdCache.put(oldState, allFromGit(oldState)); assertThat(loader.load(head)).isEqualTo(allFromGit(head)); - verifyZeroInteractions(externalIdReaderSpy); + verifyNoInteractions(externalIdReaderSpy); } @Test @@ -200,7 +200,7 @@ externalIdCache.put(oldState, allFromGit(oldState)); assertThat(loader.load(head)).isEqualTo(allFromGit(head)); - verifyZeroInteractions(externalIdReaderSpy); + verifyNoInteractions(externalIdReaderSpy); } private ExternalIdCacheLoader createLoader(boolean allowPartial) {
diff --git a/javatests/com/google/gerrit/server/mail/send/FromAddressGeneratorProviderTest.java b/javatests/com/google/gerrit/server/mail/send/FromAddressGeneratorProviderTest.java index f10a281..5980071 100644 --- a/javatests/com/google/gerrit/server/mail/send/FromAddressGeneratorProviderTest.java +++ b/javatests/com/google/gerrit/server/mail/send/FromAddressGeneratorProviderTest.java
@@ -18,7 +18,7 @@ import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyZeroInteractions; +import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.when; import com.google.gerrit.entities.Account; @@ -125,7 +125,7 @@ assertThat(r).isNotNull(); assertThat(r.name()).isEqualTo(ident.getName()); assertThat(r.email()).isEqualTo(ident.getEmailAddress()); - verifyZeroInteractions(accountCache); + verifyNoInteractions(accountCache); } @Test @@ -229,7 +229,7 @@ assertThat(r).isNotNull(); assertThat(r.name()).isEqualTo(ident.getName()); assertThat(r.email()).isEqualTo(ident.getEmailAddress()); - verifyZeroInteractions(accountCache); + verifyNoInteractions(accountCache); } @Test @@ -239,7 +239,7 @@ assertThat(r).isNotNull(); assertThat(r.name()).isEqualTo(ident.getName()); assertThat(r.email()).isEqualTo(ident.getEmailAddress()); - verifyZeroInteractions(accountCache); + verifyNoInteractions(accountCache); } @Test @@ -304,7 +304,7 @@ assertThat(r).isNotNull(); assertThat(r.name()).isEqualTo(ident.getName()); assertThat(r.email()).isEqualTo(ident.getEmailAddress()); - verifyZeroInteractions(accountCache); + verifyNoInteractions(accountCache); } @Test
diff --git a/javatests/com/google/gerrit/server/project/ProjectConfigTest.java b/javatests/com/google/gerrit/server/project/ProjectConfigTest.java index 2e934e9..9130d3e 100644 --- a/javatests/com/google/gerrit/server/project/ProjectConfigTest.java +++ b/javatests/com/google/gerrit/server/project/ProjectConfigTest.java
@@ -209,15 +209,15 @@ .add("groups", group(developers)) .add( "project.config", - "[submitRequirement \"Code-review\"]\n" + "[submit-requirement \"Code-review\"]\n" + " description = At least one Code Review +2\n" - + " applicabilityExpression = branch(refs/heads/master)\n" - + " submittabilityExpression = label(code-review, +2)\n" - + "[submitRequirement \"api-review\"]\n" + + " applicableIf =branch(refs/heads/master)\n" + + " submittableIf = label(code-review, +2)\n" + + "[submit-requirement \"api-review\"]\n" + " description = Additional review required for API modifications\n" - + " applicabilityExpression = commit_filepath_contains(\\\"/api/.*\\\")\n" - + " submittabilityExpression = label(api-review, +2)\n" - + " overrideExpression = label(build-cop-override, +1)\n" + + " applicableIf =commit_filepath_contains(\\\"/api/.*\\\")\n" + + " submittableIf = label(api-review, +2)\n" + + " overrideIf = label(build-cop-override, +1)\n" + " canOverrideInChildProjects = true\n") .create(); @@ -257,8 +257,8 @@ .add("groups", group(developers)) .add( "project.config", - "[submitRequirement \"code-review\"]\n" - + " submittabilityExpression = label(code-review, +2)\n") + "[submit-requirement \"code-review\"]\n" + + " submittableIf = label(code-review, +2)\n") .create(); ProjectConfig cfg = read(rev); @@ -281,12 +281,12 @@ .add("groups", group(developers)) .add( "project.config", - "[submitRequirement \"code-review\"]\n" + "[submit-requirement \"code-review\"]\n" + " description = At least one Code Review +2\n" - + " submittabilityExpression = label(code-review, +2)\n" - + "[submitRequirement \"Code-Review\"]\n" + + " submittableIf = label(code-review, +2)\n" + + "[submit-requirement \"Code-Review\"]\n" + " description = Another code review label\n" - + " submittabilityExpression = label(code-review, +2)\n" + + " submittableIf = label(code-review, +2)\n" + " canOverrideInChildProjects = true\n") .create(); @@ -317,8 +317,8 @@ .add("groups", group(developers)) .add( "project.config", - "[submitRequirement \"code-review\"]\n" - + " applicabilityExpression = label(code-review, +2)\n") + "[submit-requirement \"code-review\"]\n" + + " applicableIf =label(code-review, +2)\n") .create(); ProjectConfig cfg = read(rev); @@ -943,10 +943,10 @@ tr.commit() .add( "project.config", - "[submitRequirement \"code-review\"]\n" + "[submit-requirement \"code-review\"]\n" + " description = At least one Code Review +2\n" - + " applicabilityExpression = branch(refs/heads/master)\n" - + " submittabilityExpression = label(code-review, +2)\n" + + " applicableIf =branch(refs/heads/master)\n" + + " submittableIf = label(code-review, +2)\n" + "[notify \"name\"]\n" + " email = example@example.com\n") .create();
diff --git a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java index 3c10fbc..1f29f45 100644 --- a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java +++ b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
@@ -1007,6 +1007,7 @@ changes.put(-1, reviewMinus1Change); changes.put(-2, reviewMinus2Change); + assertQuery("label:Code-Review=MIN", reviewMinus2Change); assertQuery("label:Code-Review=-2", reviewMinus2Change); assertQuery("label:Code-Review-2", reviewMinus2Change); assertQuery("label:Code-Review=-1", reviewMinus1Change); @@ -1018,6 +1019,13 @@ assertQuery("label:Code-Review=+2", reviewPlus2Change); assertQuery("label:Code-Review=2", reviewPlus2Change); assertQuery("label:Code-Review+2", reviewPlus2Change); + assertQuery("label:Code-Review=MAX", reviewPlus2Change); + assertQuery( + "label:Code-Review=ANY", + reviewPlus2Change, + reviewPlus1Change, + reviewMinus1Change, + reviewMinus2Change); assertQuery("label:Code-Review>-3", codeReviewInRange(changes, -2, 2)); assertQuery("label:Code-Review>=-2", codeReviewInRange(changes, -2, 2));
diff --git a/polygerrit-ui/app/BUILD b/polygerrit-ui/app/BUILD index 4a81f38..fcf1cf4 100644 --- a/polygerrit-ui/app/BUILD +++ b/polygerrit-ui/app/BUILD
@@ -119,14 +119,8 @@ "elements/diff/gr-diff/gr-diff_html.ts", "elements/diff/gr-patch-range-select/gr-patch-range-select_html.ts", "elements/gr-app-element_html.ts", - "elements/settings/gr-edit-preferences/gr-edit-preferences_html.ts", - "elements/settings/gr-email-editor/gr-email-editor_html.ts", - "elements/settings/gr-identities/gr-identities_html.ts", - "elements/settings/gr-registration-dialog/gr-registration-dialog_html.ts", "elements/settings/gr-settings-view/gr-settings-view_html.ts", "elements/settings/gr-watched-projects-editor/gr-watched-projects-editor_html.ts", - "elements/shared/gr-account-entry/gr-account-entry_html.ts", - "elements/shared/gr-account-label/gr-account-label_html.ts", "elements/shared/gr-account-list/gr-account-list_html.ts", "elements/shared/gr-autocomplete/gr-autocomplete_html.ts", "elements/shared/gr-change-status/gr-change-status_html.ts",
diff --git a/polygerrit-ui/app/api/checks.ts b/polygerrit-ui/app/api/checks.ts index aed6f8d..b64cd91 100644 --- a/polygerrit-ui/app/api/checks.ts +++ b/polygerrit-ui/app/api/checks.ts
@@ -62,7 +62,7 @@ patchsetNumber: number; patchsetSha: string; repo: string; - commmitMessage?: string; + commitMessage?: string; changeInfo: ChangeInfo; }
diff --git a/polygerrit-ui/app/api/diff.ts b/polygerrit-ui/app/api/diff.ts index 98cd4dc..7400295 100644 --- a/polygerrit-ui/app/api/diff.ts +++ b/polygerrit-ui/app/api/diff.ts
@@ -206,6 +206,7 @@ font_size: number; // TODO: Missing documentation show_file_comment_button?: boolean; + line_wrapping?: boolean; } export declare interface ImageDiffPreferences {
diff --git a/polygerrit-ui/app/api/hook.ts b/polygerrit-ui/app/api/hook.ts index deb8566..f8a6cc1 100644 --- a/polygerrit-ui/app/api/hook.ts +++ b/polygerrit-ui/app/api/hook.ts
@@ -16,7 +16,7 @@ */ import {ChangeInfo, ConfigInfo, RevisionInfo} from './rest-api'; -interface GerritElementExtensions { +export interface GerritElementExtensions { content?: HTMLElement & {hidden?: boolean}; change?: ChangeInfo; revision?: RevisionInfo; @@ -25,17 +25,21 @@ config?: ConfigInfo; } -export type HookCallback = (el: HTMLElement & GerritElementExtensions) => void; +export type PluginElement = HTMLElement & GerritElementExtensions; + +export type HookCallback<T extends PluginElement> = (el: T) => void; export declare interface RegisterOptions { + /** Defaults to empty string. */ slot?: string; - replace: boolean; + /** Defaults to false. */ + replace?: boolean; } -export declare interface HookApi { - onAttached(callback: HookCallback): HookApi; +export declare interface HookApi<T extends PluginElement> { + onAttached(callback: HookCallback<T>): HookApi<T>; - onDetached(callback: HookCallback): HookApi; + onDetached(callback: HookCallback<T>): HookApi<T>; getAllAttached(): HTMLElement[];
diff --git a/polygerrit-ui/app/api/plugin.ts b/polygerrit-ui/app/api/plugin.ts index 0c91546..7a56ff7 100644 --- a/polygerrit-ui/app/api/plugin.ts +++ b/polygerrit-ui/app/api/plugin.ts
@@ -58,22 +58,25 @@ checks(): ChecksPluginApi; eventHelper(element: Node): EventHelperPluginApi; getPluginName(): string; - hook(endpointName: string, opt_options?: RegisterOptions): HookApi; + hook<T extends HTMLElement>( + endpointName: string, + opt_options?: RegisterOptions + ): HookApi<T>; // eslint-disable-next-line @typescript-eslint/no-explicit-any on(eventName: EventType, target: any): void; popup(): Promise<PopupPluginApi>; popup(moduleName: string): Promise<PopupPluginApi>; popup(moduleName?: string): Promise<PopupPluginApi | null>; - registerCustomComponent( + registerCustomComponent<T extends HTMLElement>( endpointName: string, moduleName?: string, options?: RegisterOptions - ): HookApi; - registerDynamicCustomComponent( + ): HookApi<T>; + registerDynamicCustomComponent<T extends HTMLElement>( endpointName: string, moduleName?: string, options?: RegisterOptions - ): HookApi; + ): HookApi<T>; registerStyleModule(endpoint: string, moduleName: string): void; reporting(): ReportingPluginApi; restApi(): RestPluginApi;
diff --git a/polygerrit-ui/app/api/rest.ts b/polygerrit-ui/app/api/rest.ts index 2b91bf6..4c93ef0 100644 --- a/polygerrit-ui/app/api/rest.ts +++ b/polygerrit-ui/app/api/rest.ts
@@ -78,29 +78,29 @@ /** * Fetch and parse REST API response, if request succeeds. */ - send( + send<T>( method: HttpMethod, url: string, payload?: RequestPayload, errFn?: ErrorCallback, contentType?: string - ): Promise<unknown>; + ): Promise<T>; - get(url: string): Promise<unknown>; + get<T>(url: string): Promise<T>; - post( + post<T>( url: string, payload?: RequestPayload, errFn?: ErrorCallback, contentType?: string - ): Promise<unknown>; + ): Promise<T>; - put( + put<T>( url: string, payload?: RequestPayload, errFn?: ErrorCallback, contentType?: string - ): Promise<unknown>; + ): Promise<T>; delete(url: string): Promise<Response>; }
diff --git a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_html.ts b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_html.ts index ca68926..07f9a4a 100644 --- a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_html.ts +++ b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_html.ts
@@ -184,7 +184,7 @@ <gr-account-chip account="[[change.owner]]" change="[[change]]" - highlight-attention + highlightAttention ></gr-account-chip> <template is="dom-if" if="[[_pushCertificateValidation]]"> <gr-tooltip-content @@ -213,7 +213,7 @@ <gr-account-chip account="[[_getNonOwnerRole(change, _CHANGE_ROLE.UPLOADER)]]" change="[[change]]" - highlight-attention + highlightAttention ></gr-account-chip> </span> </section>
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 39a80e6..53bdb91 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
@@ -76,8 +76,9 @@ PatchSet, } from '../../../utils/patch-set-util'; import { - changeIsMerged, changeIsAbandoned, + changeIsMerged, + changeIsOpen, changeStatuses, isCc, isOwner, @@ -676,7 +677,6 @@ ); this.addEventListener('open-fix-preview', e => this._onOpenFixPreview(e)); this.addEventListener('close-fix-preview', e => this._onCloseFixPreview(e)); - window.addEventListener('scroll', this.handleScroll); document.addEventListener('visibilitychange', this.handleVisibilityChange); this.addEventListener(EventType.SHOW_PRIMARY_TAB, e => @@ -693,7 +693,6 @@ /** @override */ disconnectedCallback() { this.disconnected$.next(); - window.removeEventListener('scroll', this.handleScroll); document.removeEventListener( 'visibilitychange', this.handleVisibilityChange @@ -1139,14 +1138,6 @@ this._openReplyDialog(target); } - readonly handleScroll = () => { - this.scrollTask = debounce( - this.scrollTask, - () => (this.viewState.scrollTop = document.body.scrollTop), - 150 - ); - }; - _setShownFiles(e: CustomEvent<{length: number}>) { this._shownFileCount = e.detail.length; } @@ -1283,11 +1274,7 @@ this._sendShowChangeEvent(); setTimeout(() => { - if (this.viewState.scrollTop) { - document.documentElement.scrollTop = document.body.scrollTop = this.viewState.scrollTop; - } else { - this._maybeScrollToMessage(window.location.hash); - } + this._maybeScrollToMessage(window.location.hash); this._initialLoadComplete = true; }); } @@ -1407,7 +1394,6 @@ _resetFileListViewState() { this.set('viewState.selectedFileIndex', 0); - this.set('viewState.scrollTop', 0); if ( !!this.viewState.changeNum && this.viewState.changeNum !== this._changeNum @@ -1768,6 +1754,17 @@ */ _processEdit(change: ParsedChangeInfo, edit?: EditInfo | false) { if ( + !edit && + this._patchRange?.patchNum === EditPatchSetNum && + changeIsOpen(change) + ) { + fireAlert(this, 'Change edit not found. Please create a change edit.'); + GerritNav.navigateToChange(change); + return; + } + + if ( + !edit && (changeIsMerged(change) || changeIsAbandoned(change)) && this._editMode ) { @@ -1970,13 +1967,32 @@ _getCommitInfo() { if (!this._changeNum) - throw new Error('missing required changeNum property'); + throw new Error('missing required _changeNum property'); if (!this._patchRange) throw new Error('missing required _patchRange property'); if (this._patchRange.patchNum === undefined) throw new Error('missing required patchNum property'); + + // We only call _getEdit if the patchset number is an edit. + // We have to do this to ensure we can tell if an edit + // exists or not. + // This safely works even if a edit does not exist. + if (this._patchRange!.patchNum! === EditPatchSetNum) { + return this._getEdit().then(edit => { + if (!edit) { + return Promise.resolve(); + } + + return this._getChangeCommitInfo(); + }); + } + + return this._getChangeCommitInfo(); + } + + _getChangeCommitInfo() { return this.restApiService - .getChangeCommitInfo(this._changeNum, this._patchRange.patchNum) + .getChangeCommitInfo(this._changeNum!, this._patchRange!.patchNum!) .then(commitInfo => { this._commitInfo = commitInfo; });
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 5b940a8..c6975e6 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
@@ -344,7 +344,7 @@ <span class="headerSubject">[[_change.subject]]</span> <gr-copy-clipboard class="changeCopyClipboard" - hide-input="" + hideInput="" text="[[_computeCopyTextForTitle(_change)]]" > </gr-copy-clipboard>
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 e882920..867b3a7 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
@@ -128,8 +128,6 @@ TestKeyboardShortcutBinder.pop(); }); - const TEST_SCROLL_TOP_PX = 100; - const ROBOT_COMMENTS_LIMIT = 10; // TODO: should have a mock service to generate VALID fake data @@ -1805,42 +1803,6 @@ assert.isTrue(awaitPluginsLoadedStub.called); }); - suite('scroll related tests', () => { - test('document scrolling calls function to set scroll height', done => { - const originalHeight = document.body.scrollHeight; - const scrollStub = sinon.stub(element, 'handleScroll').callsFake(() => { - assert.isTrue(scrollStub.called); - document.body.style.height = `${originalHeight}px`; - scrollStub.restore(); - done(); - }); - document.body.style.height = '10000px'; - element.handleScroll(); - }); - - test('scrollTop is set correctly', async () => { - element.viewState = {scrollTop: TEST_SCROLL_TOP_PX}; - - sinon.stub(element, 'loadData').callsFake(() => { - // When element is reloaded, ensure that the history - // state has the scrollTop set earlier. This will then - // be reset. - assert.isTrue(element.viewState.scrollTop === TEST_SCROLL_TOP_PX); - return Promise.resolve([]); - }); - - // simulate reloading component, which is done when route - // changes to match a regex of change view type. - element.params = {...createAppElementChangeViewParams()}; - await flush(); - }); - - test('scrollTop is reset when new change is loaded', () => { - element._resetFileListViewState(); - assert.equal(element.viewState.scrollTop, 0); - }); - }); - suite('reply dialog tests', () => { setup(() => { sinon.stub(element.$.replyDialog, '_draftChanged');
diff --git a/polygerrit-ui/app/elements/change/gr-commit-info/gr-commit-info_html.ts b/polygerrit-ui/app/elements/change/gr-commit-info/gr-commit-info_html.ts index 65ca8b5..02fa090 100644 --- a/polygerrit-ui/app/elements/change/gr-commit-info/gr-commit-info_html.ts +++ b/polygerrit-ui/app/elements/change/gr-commit-info/gr-commit-info_html.ts
@@ -31,9 +31,9 @@ >[[_computeShortHash(change, commitInfo, serverConfig)]]</a > <gr-copy-clipboard - has-tooltip="" - button-title="Copy full SHA to clipboard" - hide-input="" + hasTooltip="" + buttonTitle="Copy full SHA to clipboard" + hideInput="" text="[[commitInfo.commit]]" > </gr-copy-clipboard>
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 71b4ac4..ff303e3 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
@@ -31,8 +31,6 @@ import { AccountInfo, ChangeInfo, - EditPatchSetNum, - ParentPatchSetNum, PatchSetNum, CommitInfo, ServerInfo, @@ -189,10 +187,6 @@ ) { return; } - if (patchNum === EditPatchSetNum && basePatchNum === ParentPatchSetNum) { - GerritNav.navigateToChange(this.change, undefined, undefined, true); - return; - } GerritNav.navigateToChange(this.change, patchNum, basePatchNum); }
diff --git a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header_test.js b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header_test.js index 9bc243d..dd70678 100644 --- a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header_test.js +++ b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header_test.js
@@ -22,7 +22,6 @@ import 'lodash/lodash.js'; import {createRevisions} from '../../../test/test-data-generators.js'; import {stubRestApi} from '../../../test/test-utils.js'; -import {EditPatchSetNum, ParentPatchSetNum} from '../../../types/common.js'; const basicFixture = fixtureFromElement('gr-file-list-header'); @@ -162,34 +161,6 @@ .calledWithExactly(element.change, 3, 1)); }); - test('navigateToChange called when range select changes with edit', () => { - const navigateToChangeStub = sinon.stub(GerritNav, 'navigateToChange'); - element.change = { - change_id: 'Iad9dc96274af6946f3632be53b106ef80f7ba6ca', - revisions: { - rev2: {_number: 2}, - rev1: {_number: 1}, - rev13: {_number: 13}, - rev3: {_number: 3}, - }, - status: 'NEW', - labels: {}, - }; - element.basePatchNum = 1; - element.patchNum = EditPatchSetNum; - - const detail = { - detail: { - basePatchNum: ParentPatchSetNum, - patchNum: EditPatchSetNum, - }, - }; - element._handlePatchChange(detail); - assert.equal(navigateToChangeStub.callCount, 1); - assert.isTrue(navigateToChangeStub.lastCall - .calledWithExactly(element.change, undefined, undefined, true)); - }); - test('class is applied to file list on old patch set', () => { const allPatchSets = [{num: 4}, {num: 2}, {num: 1}]; assert.equal(element._computePatchInfoClass(1, allPatchSets),
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 106fe27..9dcb67e 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
@@ -63,6 +63,7 @@ import {customElement, observe, property} from '@polymer/decorators'; import { BasePatchSetNum, + EditPatchSetNum, ElementPropertyDeepChange, FileInfo, FileNameToFileInfoMap, @@ -500,12 +501,7 @@ } @observe('_filesByPath') - async _updateCleanlyMergedPaths( - // changeNum?: NumericChangeId, - // change?: ParsedChangeInfo, - // patchRange?: PatchRange, - filesByPath?: FileNameToFileInfoMap - ) { + async _updateCleanlyMergedPaths(filesByPath?: FileNameToFileInfoMap) { // When viewing Auto Merge base vs a patchset, add an additional row that // knows how many files were cleanly merged. This requires an additional RPC // for the diffs between target parent and the patch set. The cleanly merged @@ -516,7 +512,8 @@ this.changeNum && this.patchRange?.patchNum && new RevisionInfo(this.change).isMergeCommit(this.patchRange.patchNum) && - this.patchRange.basePatchNum === 'PARENT' + this.patchRange.basePatchNum === 'PARENT' && + this.patchRange.patchNum !== EditPatchSetNum ) { const allFilesByPath = await this.restApiService.getChangeOrEditFiles( this.changeNum, @@ -680,7 +677,8 @@ patchRange?: PatchRange, file?: NormalizedFileInfo ) { - const draftCount = changeComments?.computeDraftCountForFile( + if (changeComments === undefined) return ''; + const draftCount = changeComments.computeDraftCountForFile( patchRange, file ); @@ -696,7 +694,8 @@ patchRange?: PatchRange, file?: NormalizedFileInfo ) { - const draftCount = changeComments?.computeDraftCountForFile( + if (changeComments === undefined) return ''; + const draftCount = changeComments.computeDraftCountForFile( patchRange, file );
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_html.ts b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_html.ts index 3b2cacf..4d04744 100644 --- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_html.ts +++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_html.ts
@@ -410,7 +410,7 @@ </span> <gr-file-status-chip file="[[file]]"></gr-file-status-chip> <gr-copy-clipboard - hide-input="" + hideInput="" text="[[file.__path]]" ></gr-copy-clipboard> </a> @@ -418,7 +418,7 @@ <div class="oldPath" title$="[[file.old_path]]"> [[file.old_path]] <gr-copy-clipboard - hide-input="" + hideInput="" text="[[file.old_path]]" ></gr-copy-clipboard> </div>
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 c32e3e5..0655721 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
@@ -33,6 +33,7 @@ mockPromise, query, } from '../../../test/test-utils.js'; +import {EditPatchSetNum} from '../../../types/common.js'; import {Shortcut} from '../../../mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin.js'; import {createCommentThreads} from '../../../utils/comment-util.js'; import { @@ -1232,6 +1233,15 @@ assert.notOk(query(element, '.cleanlyMergedText')); assert.notOk(query(element, '.showParentButton')); }); + + test('not shown in edit mode', async () => { + element.patchRange = {basePatchNum: 1, patchNum: EditPatchSetNum}; + await element.reload(); + await flush(); + + assert.notOk(query(element, '.cleanlyMergedText')); + assert.notOk(query(element, '.showParentButton')); + }); }); });
diff --git a/polygerrit-ui/app/elements/change/gr-message/gr-message.ts b/polygerrit-ui/app/elements/change/gr-message/gr-message.ts index 1711499..b097340 100644 --- a/polygerrit-ui/app/elements/change/gr-message/gr-message.ts +++ b/polygerrit-ui/app/elements/change/gr-message/gr-message.ts
@@ -121,6 +121,9 @@ @property({type: Object}) message: ChangeMessage | undefined; + @property({type: Array}) + commentThreads: CommentThread[] = []; + @computed('message') get author() { return this.message?.author || this.message?.updated_by; @@ -195,13 +198,13 @@ '_computeMessageContentCollapsed(message.message,' + ' message.accounts_in_message,' + ' message.tag,' + - ' message.commentThreads)', + ' commentThreads)', }) _messageContentCollapsed = ''; @property({ type: String, - computed: '_computeCommentCountText(message.commentThreads.length)', + computed: '_computeCommentCountText(commentThreads)', }) _commentCountText = ''; @@ -234,12 +237,12 @@ } } - _computeCommentCountText(threadsLength?: number) { - if (!threadsLength) { + _computeCommentCountText(commentThreads?: CommentThread[]) { + if (!commentThreads?.length) { return undefined; } - return pluralize(threadsLength, 'comment'); + return pluralize(commentThreads.length, 'comment'); } _computeMessageContentExpanded(
diff --git a/polygerrit-ui/app/elements/change/gr-message/gr-message_html.ts b/polygerrit-ui/app/elements/change/gr-message/gr-message_html.ts index e58b8a8..9e24a09 100644 --- a/polygerrit-ui/app/elements/change/gr-message/gr-message_html.ts +++ b/polygerrit-ui/app/elements/change/gr-message/gr-message_html.ts
@@ -195,10 +195,8 @@ padding-bottom: 1px; color: var(--vote-text-color); } - gr-account-label { - --gr-account-label-text-style: { - font-weight: var(--font-weight-bold); - } + gr-account-label::part(gr-account-label-text) { + font-weight: var(--font-weight-bold); } iron-icon { --iron-icon-height: 20px; @@ -251,7 +249,7 @@ <div class="content messageContent"> <div class="message hideOnOpen">[[_messageContentCollapsed]]</div> <gr-formatted-text - no-trailing-margin="" + noTrailingMargin class="message hideOnCollapsed" content="[[_messageContentExpanded]]" config="[[_projectConfig.commentlinks]]"
diff --git a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.ts b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.ts index 4e2aaf1..fae624e 100644 --- a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.ts +++ b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.ts
@@ -90,9 +90,9 @@ */ function computeThreads( message: CombinedMessage, - changeComments: ChangeComments + changeComments?: ChangeComments ): CommentThread[] { - if (message._index === undefined) { + if (message._index === undefined || changeComments === undefined) { return []; } const messageId = getMessageId(message); @@ -369,6 +369,10 @@ return combinedMessages; } + getCommentThreads(message: CombinedMessage, changeComments?: ChangeComments) { + return computeThreads(message, changeComments); + } + _updateExpandedStateOfAllMessages(exp: boolean) { if (this._combinedMessages) { for (let i = 0; i < this._combinedMessages.length; i++) {
diff --git a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list_html.ts b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list_html.ts index 8fa8eab..93df77e 100644 --- a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list_html.ts +++ b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list_html.ts
@@ -93,6 +93,7 @@ change="[[change]]" change-num="[[changeNum]]" message="[[message]]" + comment-threads="[[getCommentThreads(message, changeComments)]]" project-name="[[projectName]]" show-reply-button="[[showReplyButtons]]" on-message-anchor-tap="_handleAnchorClick"
diff --git a/polygerrit-ui/app/elements/change/gr-reviewer-list/gr-reviewer-list_html.ts b/polygerrit-ui/app/elements/change/gr-reviewer-list/gr-reviewer-list_html.ts index 25d8517..ca8bf87 100644 --- a/polygerrit-ui/app/elements/change/gr-reviewer-list/gr-reviewer-list_html.ts +++ b/polygerrit-ui/app/elements/change/gr-reviewer-list/gr-reviewer-list_html.ts
@@ -66,7 +66,7 @@ account="[[reviewer]]" change="[[change]]" on-remove="_handleRemove" - highlight-attention + highlightAttention voteable-text="[[_computeVoteableText(reviewer, change)]]" removable="[[_computeCanRemoveReviewer(reviewer, mutable)]]" >
diff --git a/polygerrit-ui/app/elements/change/gr-reviewer-list/gr-reviewer-list_test.ts b/polygerrit-ui/app/elements/change/gr-reviewer-list/gr-reviewer-list_test.ts index 354d360..d3f23ad 100644 --- a/polygerrit-ui/app/elements/change/gr-reviewer-list/gr-reviewer-list_test.ts +++ b/polygerrit-ui/app/elements/change/gr-reviewer-list/gr-reviewer-list_test.ts
@@ -67,7 +67,7 @@ await dialogShown; }); - test('only show remove for removable reviewers', () => { + test('only show remove for removable reviewers', async () => { element.mutable = true; element.change = { ...createChange(), @@ -112,7 +112,7 @@ }, ], }; - flush(); + await flush(); const chips = element.root!.querySelectorAll('gr-account-chip'); assert.equal(chips.length, 4);
diff --git a/polygerrit-ui/app/elements/checks/gr-checks-results.ts b/polygerrit-ui/app/elements/checks/gr-checks-results.ts index b348eb5..f27a383 100644 --- a/polygerrit-ui/app/elements/checks/gr-checks-results.ts +++ b/polygerrit-ui/app/elements/checks/gr-checks-results.ts
@@ -588,10 +588,10 @@ .value="${this.result}" ></gr-endpoint-param> <gr-formatted-text - no-trailing-margin="" + noTrailingMargin class="message" - content="${this.result.message}" - config="${this.repoConfig}" + .content="${this.result.message}" + .config="${this.repoConfig?.commentlinks}" ></gr-formatted-text> </gr-endpoint-decorator> `;
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 015fdf3..7532101 100644 --- a/polygerrit-ui/app/elements/core/gr-router/gr-router.ts +++ b/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
@@ -51,7 +51,6 @@ import { BasePatchSetNum, DashboardId, - EditPatchSetNum, GroupId, NumericChangeId, PatchSetNum, @@ -1601,15 +1600,6 @@ queryMap: ctx.queryMap, }; - // We do not want to allow "edit" to be used as a - // patch number. Instead redirect to ,edit. - if (ctx.params[4] === EditPatchSetNum && !ctx.params[6]) { - params.basePatchNum = undefined; - params.edit = true; - this._redirect(this._generateUrl(params)); - return; - } - this.reporting.setRepoName(params.project); this.reporting.setChangeId(changeNum); this._redirectOrNavigate(params);
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 c4674d2..6e80a35 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
@@ -22,7 +22,6 @@ import {stubBaseUrl, stubRestApi, addListenerForTest} from '../../../test/test-utils.js'; import {_testOnly_RoutePattern} from './gr-router.js'; import {GerritView} from '../../../services/router/router-model.js'; -import {EditPatchSetNum} from '../../../types/common.js'; const basicFixture = fixtureFromElement('gr-router'); @@ -1430,16 +1429,16 @@ suite('_handleChangeRoute', () => { let normalizeRangeStub; - function makeParams(path, hash, baseNum, patchNum) { + function makeParams(path, hash) { return { params: [ 'foo/bar', // 0 Project 1234, // 1 Change number null, // 2 Unused null, // 3 Unused - baseNum ? baseNum : 4, // 4 Base patch number + 4, // 4 Base patch number null, // 5 Unused - patchNum ? patchNum : 7, // 6 Patch number + 7, // 6 Patch number ], queryMap: new Map(), }; @@ -1477,23 +1476,6 @@ assert.isFalse(redirectStub.called); assert.isTrue(normalizeRangeStub.called); }); - - test('redirect due to patchNum being an edit', () => { - normalizeRangeStub.returns(true); - const ctx = makeParams(null, ''); - element._handleChangeRoute(ctx, undefined, EditPatchSetNum, false); - assert.isTrue(normalizeRangeStub.called); - assert.isFalse(setParamsStub.called); - assert.isTrue(redirectStub.calledOnce); - - const params = { - view: GerritView.CHANGE, - changeNum: '1234', - project: 'test', - edit: true, - }; - assert.equal(element._generateUrl(params), '/c/test/+/1234,edit'); - }); }); suite('_handleDiffRoute', () => {
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 0929301..315fbfb 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
@@ -42,6 +42,7 @@ isInBaseOfPatchRange, isInRevisionOfPatchRange, isPatchsetLevel, + addPath, } from '../../../utils/comment-util'; import {PatchSetFile, PatchNumOnly, isPatchSetFile} from '../../../types/types'; import {appContext} from '../../../services/app-context'; @@ -75,35 +76,13 @@ portedComments: PathToCommentsInfoMap | undefined, portedDrafts: PathToCommentsInfoMap | undefined ) { - this._comments = this._addPath(comments); - this._robotComments = this._addPath(robotComments); - this._drafts = this._addPath(drafts); + this._comments = addPath(comments); + this._robotComments = addPath(robotComments); + this._drafts = addPath(drafts); this._portedComments = portedComments || {}; this._portedDrafts = portedDrafts || {}; } - /** - * Add path info to every comment as CommentInfo returned - * from server does not have that. - * - * TODO(taoalpha): should consider changing BE to send path - * back within CommentInfo - */ - _addPath<T>( - comments: {[path: string]: T[]} = {} - ): {[path: string]: Array<T & {path: string}>} { - const updatedComments: {[path: string]: Array<T & {path: string}>} = {}; - for (const filePath of Object.keys(comments)) { - const allCommentsForPath = comments[filePath] || []; - if (allCommentsForPath.length) { - updatedComments[filePath] = allCommentsForPath.map(comment => { - return {...comment, path: filePath}; - }); - } - } - return updatedComments; - } - get drafts() { return this._drafts; }
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_html.ts b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_html.ts index 9da3bf1..d01943c 100644 --- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_html.ts +++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_html.ts
@@ -429,9 +429,16 @@ } /** Support the line length indicator **/ .full-width td.content .contentText { - background-image: var(--line-length-indicator); + /* + Same strategy as in https://stackoverflow.com/questions/1179928/how-can-i-put-a-vertical-line-down-the-center-of-a-div + */ + background-image: linear-gradient( + var(--line-length-indicator-color), + var(--line-length-indicator-color) + ); + background-size: 1px 100%; background-position: var(--line-limit) 0; - background-repeat: repeat-y; + background-repeat: no-repeat; } .newlineWarning { color: var(--deemphasized-text-color);
diff --git a/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls.ts b/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls.ts index e61d0b8..f0a3ca1 100644 --- a/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls.ts +++ b/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls.ts
@@ -36,6 +36,7 @@ } from '../../shared/gr-autocomplete/gr-autocomplete'; import {appContext} from '../../../services/app-context'; import {IronInputElement} from '@polymer/iron-input'; +import {fireAlert} from '../../../utils/event-util'; export interface GrEditControls { $: { @@ -176,11 +177,15 @@ '.dialog' ) as NodeListOf<GrDialog>; for (const dialog of dialogs) { - this._closeDialog(dialog); + // We set the second param to false, because this function + // is called by _showDialog which when you open either restore, + // delete or rename dialogs, it reseted the automatically + // set input. + this._closeDialog(dialog, false); } } - _closeDialog(dialog?: GrDialog, clearInputs = false) { + _closeDialog(dialog?: GrDialog, clearInputs = true) { if (!dialog) return; if (clearInputs) { @@ -205,18 +210,24 @@ } _handleOpenConfirm(e: Event) { + if (!this.change || !this._path) { + fireAlert(this, 'You must enter a path.'); + this._closeDialog(this.$.openDialog); + return; + } const url = GerritNav.getEditUrlForDiff( this.change, this._path, this.patchNum ); GerritNav.navigateToRelativeUrl(url); - this._closeDialog(this._getDialogFromEvent(e), true); + this._closeDialog(this._getDialogFromEvent(e)); } _handleUploadConfirm(path: string, fileData: string) { if (!this.change || !path || !fileData) { - this._closeDialog(this.$.openDialog, true); + fireAlert(this, 'You must enter a path and data.'); + this._closeDialog(this.$.openDialog); return Promise.resolve(); } return this.restApiService @@ -225,7 +236,7 @@ if (!res || !res.ok) { return; } - this._closeDialog(this.$.openDialog, true); + this._closeDialog(this.$.openDialog); GerritNav.navigateToChange(this.change); }); } @@ -234,39 +245,54 @@ // Get the dialog before the api call as the event will change during bubbling // which will make Polymer.dom(e).path an empty array in polymer 2 const dialog = this._getDialogFromEvent(e); + if (!this.change || !this._path) { + fireAlert(this, 'You must enter a path.'); + this._closeDialog(dialog); + return; + } this.restApiService .deleteFileInChangeEdit(this.change._number, this._path) .then(res => { if (!res || !res.ok) { return; } - this._closeDialog(dialog, true); + this._closeDialog(dialog); GerritNav.navigateToChange(this.change); }); } _handleRestoreConfirm(e: Event) { const dialog = this._getDialogFromEvent(e); + if (!this.change || !this._path) { + fireAlert(this, 'You must enter a path.'); + this._closeDialog(dialog); + return; + } this.restApiService .restoreFileInChangeEdit(this.change._number, this._path) .then(res => { if (!res || !res.ok) { return; } - this._closeDialog(dialog, true); + this._closeDialog(dialog); GerritNav.navigateToChange(this.change); }); } _handleRenameConfirm(e: Event) { const dialog = this._getDialogFromEvent(e); + if (!this.change || !this._path || !this._newPath) { + fireAlert(this, 'You must enter a old path and a new path.'); + this._closeDialog(dialog); + return; + } return this.restApiService .renameFileInChangeEdit(this.change._number, this._path, this._newPath) .then(res => { if (!res || !res.ok) { return; } - this._closeDialog(dialog, true); + this._closeDialog(dialog); GerritNav.navigateToChange(this.change); }); }
diff --git a/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls_test.ts b/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls_test.ts index d9f5cd7..a0b5392 100644 --- a/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls_test.ts +++ b/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls_test.ts
@@ -117,7 +117,7 @@ assert.isFalse(editDiffStub.called); assert.isFalse(navStub.called); assert.isTrue(closeDialogSpy.called); - assert.equal(element._path, 'src/test.cpp'); + assert.equal(element._path, ''); }); }); }); @@ -202,7 +202,7 @@ ); assert.isFalse(navStub.called); assert.isTrue(closeDialogSpy.called); - assert.equal(element._path, 'src/test.cpp'); + assert.equal(element._path, ''); }); }); }); @@ -296,8 +296,8 @@ ); assert.isFalse(navStub.called); assert.isTrue(closeDialogSpy.called); - assert.equal(element._path, 'src/test.cpp'); - assert.equal(element._newPath, 'src/test.newPath'); + assert.equal(element._path, ''); + assert.equal(element._newPath, ''); }); }); }); @@ -365,7 +365,7 @@ ); assert.isFalse(navStub.called); assert.isTrue(closeDialogSpy.called); - assert.equal(element._path, 'src/test.cpp'); + assert.equal(element._path, ''); }); }); });
diff --git a/polygerrit-ui/app/elements/gr-app-element.ts b/polygerrit-ui/app/elements/gr-app-element.ts index c9f0506..ebd143c 100644 --- a/polygerrit-ui/app/elements/gr-app-element.ts +++ b/polygerrit-ui/app/elements/gr-app-element.ts
@@ -295,7 +295,6 @@ showDownloadDialog: false, diffMode: null, numFilesShown: null, - scrollTop: 0, }, changeListView: { query: null,
diff --git a/polygerrit-ui/app/elements/plugins/gr-dom-hooks/gr-dom-hooks.ts b/polygerrit-ui/app/elements/plugins/gr-dom-hooks/gr-dom-hooks.ts index 3e8f0a4..3613f4c 100644 --- a/polygerrit-ui/app/elements/plugins/gr-dom-hooks/gr-dom-hooks.ts +++ b/polygerrit-ui/app/elements/plugins/gr-dom-hooks/gr-dom-hooks.ts
@@ -16,10 +16,10 @@ */ import {PolymerElement} from '@polymer/polymer/polymer-element'; import {PluginApi} from '../../../api/plugin'; -import {HookApi, HookCallback} from '../../../api/hook'; +import {HookApi, HookCallback, PluginElement} from '../../../api/hook'; export class GrDomHooksManager { - private hooks: Record<string, GrDomHook>; + private hooks: Record<string, GrDomHook<PluginElement>>; private plugin: PluginApi; @@ -41,23 +41,33 @@ } } - getDomHook(endpointName: string, moduleName?: string) { + getDomHook<T extends PluginElement>( + endpointName: string, + moduleName?: string + ): HookApi<T> { const hookName = this._getHookName(endpointName, moduleName); if (!this.hooks[hookName]) { - this.hooks[hookName] = new GrDomHook(hookName, moduleName); + this.hooks[hookName] = (new GrDomHook<T>( + hookName, + moduleName + ) as unknown) as GrDomHook<PluginElement>; } - return this.hooks[hookName]; + return (this.hooks[hookName] as unknown) as GrDomHook<T>; } } -export class GrDomHook implements HookApi { +export class GrDomHook<T extends PluginElement> implements HookApi<T> { private instances: HTMLElement[] = []; - private attachCallbacks: HookCallback[] = []; + private attachCallbacks: HookCallback<T>[] = []; - private detachCallbacks: HookCallback[] = []; + private detachCallbacks: HookCallback<T>[] = []; - private moduleName: string; + /** + * The name of the (custom) element that is going to be created. Matches the T + * type parameter. + */ + private readonly moduleName: string; private lastAttachedPromise: Promise<HTMLElement> | null = null; @@ -87,7 +97,7 @@ customElements.define(HookPlaceholder.is, HookPlaceholder); } - handleInstanceDetached(instance: HTMLElement) { + handleInstanceDetached(instance: T) { const index = this.instances.indexOf(instance); if (index !== -1) { this.instances.splice(index, 1); @@ -95,7 +105,7 @@ this.detachCallbacks.forEach(callback => callback(instance)); } - handleInstanceAttached(instance: HTMLElement) { + handleInstanceAttached(instance: T) { this.instances.push(instance); this.attachCallbacks.forEach(callback => callback(instance)); } @@ -109,7 +119,7 @@ return Promise.resolve(this.instances.slice(-1)[0]); } if (!this.lastAttachedPromise) { - let resolve: HookCallback; + let resolve: HookCallback<T>; const promise = new Promise<HTMLElement>(r => { resolve = r; this.attachCallbacks.push(resolve); @@ -137,7 +147,7 @@ * Install a new callback to invoke when a new instance of DOM hook element * is attached. */ - onAttached(callback: HookCallback) { + onAttached(callback: HookCallback<T>) { this.attachCallbacks.push(callback); return this; } @@ -147,7 +157,7 @@ * is detached. * */ - onDetached(callback: HookCallback) { + onDetached(callback: HookCallback<T>) { this.detachCallbacks.push(callback); return this; }
diff --git a/polygerrit-ui/app/elements/plugins/gr-endpoint-decorator/gr-endpoint-decorator.ts b/polygerrit-ui/app/elements/plugins/gr-endpoint-decorator/gr-endpoint-decorator.ts index 0e8cbcb..4776eac 100644 --- a/polygerrit-ui/app/elements/plugins/gr-endpoint-decorator/gr-endpoint-decorator.ts +++ b/polygerrit-ui/app/elements/plugins/gr-endpoint-decorator/gr-endpoint-decorator.ts
@@ -23,7 +23,7 @@ import {getPluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader'; import {customElement, property} from '@polymer/decorators'; import {PluginApi} from '../../../api/plugin'; -import {HookApi} from '../../../api/hook'; +import {HookApi, PluginElement} from '../../../api/hook'; const INIT_PROPERTIES_TIMEOUT_MS = 10000; @@ -37,7 +37,7 @@ name!: string; @property({type: Object}) - _domHooks = new Map<HTMLElement, HookApi>(); + _domHooks = new Map<PluginElement, HookApi<PluginElement>>(); @property({type: Object}) _initializedPlugins = new Map<string, boolean>();
diff --git a/polygerrit-ui/app/elements/settings/gr-edit-preferences/gr-edit-preferences.ts b/polygerrit-ui/app/elements/settings/gr-edit-preferences/gr-edit-preferences.ts index 5e2c5cb..4909bef 100644 --- a/polygerrit-ui/app/elements/settings/gr-edit-preferences/gr-edit-preferences.ts +++ b/polygerrit-ui/app/elements/settings/gr-edit-preferences/gr-edit-preferences.ts
@@ -26,6 +26,9 @@ export interface GrEditPreferences { $: { + editTabWidth: HTMLInputElement; + editColumns: HTMLInputElement; + editIndentUnit: HTMLInputElement; editSyntaxHighlighting: HTMLInputElement; showAutoCloseBrackets: HTMLInputElement; showIndentWithTabs: HTMLInputElement; @@ -58,6 +61,21 @@ this.hasUnsavedChanges = true; } + _handleEditTabWidthChanged() { + this.set('editPrefs.tab_size', Number(this.$.editTabWidth.value)); + this._handleEditPrefsChanged(); + } + + _handleEditLineLengthChanged() { + this.set('editPrefs.line_length', Number(this.$.editColumns.value)); + this._handleEditPrefsChanged(); + } + + _handleEditIndentUnitChanged() { + this.set('editPrefs.indent_unit', Number(this.$.editIndentUnit.value)); + this._handleEditPrefsChanged(); + } + _handleEditSyntaxHighlightingChanged() { this.set( 'editPrefs.syntax_highlighting', @@ -101,6 +119,16 @@ this.hasUnsavedChanges = false; }); } + + /** + * bind-value has type string so we have to convert + * anything inputed to string. + * + * This is so typescript checker doesn't fail. + */ + _convertToString(key?: number) { + return key !== undefined ? String(key) : ''; + } } declare global {
diff --git a/polygerrit-ui/app/elements/settings/gr-edit-preferences/gr-edit-preferences_html.ts b/polygerrit-ui/app/elements/settings/gr-edit-preferences/gr-edit-preferences_html.ts index f6344f0..a51deaf 100644 --- a/polygerrit-ui/app/elements/settings/gr-edit-preferences/gr-edit-preferences_html.ts +++ b/polygerrit-ui/app/elements/settings/gr-edit-preferences/gr-edit-preferences_html.ts
@@ -28,23 +28,11 @@ <label for="editTabWidth" class="title">Tab width</label> <span class="value"> <iron-input - type="number" - prevent-invalid-input="" allowed-pattern="[0-9]" - bind-value="{{editPrefs.tab_size}}" - on-keypress="_handleEditPrefsChanged" - on-change="_handleEditPrefsChanged" + bind-value="[[_convertToString(editPrefs.tab_size)]]" + on-change="_handleEditTabWidthChanged" > - <input - is="iron-input" - id="editTabWidth" - type="number" - prevent-invalid-input="" - allowed-pattern="[0-9]" - bind-value="{{editPrefs.tab_size}}" - on-keypress="_handleEditPrefsChanged" - on-change="_handleEditPrefsChanged" - /> + <input id="editTabWidth" type="number" /> </iron-input> </span> </section> @@ -52,47 +40,23 @@ <label for="editColumns" class="title">Columns</label> <span class="value"> <iron-input - type="number" - prevent-invalid-input="" allowed-pattern="[0-9]" - bind-value="{{editPrefs.line_length}}" - on-keypress="_handleEditPrefsChanged" - on-change="_handleEditPrefsChanged" + bind-value="[[_convertToString(editPrefs.line_length)]]" + on-change="_handleEditLineLengthChanged" > - <input - id="editColumns" - is="iron-input" - type="number" - prevent-invalid-input="" - allowed-pattern="[0-9]" - bind-value="{{editPrefs.line_length}}" - on-keypress="_handleEditPrefsChanged" - on-change="_handleEditPrefsChanged" - /> + <input id="editColumns" type="number" /> </iron-input> </span> </section> <section> - <label for="indentUnit" class="title">Indent unit</label> + <label for="editIndentUnit" class="title">Indent unit</label> <span class="value"> <iron-input - type="number" - prevent-invalid-input="" allowed-pattern="[0-9]" - bind-value="{{editPrefs.indent_unit}}" - on-keypress="_handleEditPrefsChanged" - on-change="_handleEditPrefsChanged" + bind-value="[[_convertToString(editPrefs.indent_unit)]]" + on-change="_handleEditIndentUnitChanged" > - <input - is="iron-input" - id="indentUnit" - type="number" - prevent-invalid-input="" - allowed-pattern="[0-9]" - bind-value="{{editPrefs.indent_unit}}" - on-keypress="_handleEditPrefsChanged" - on-change="_handleEditPrefsChanged" - /> + <input id="indentUnit" type="number" /> </iron-input> </span> </section>
diff --git a/polygerrit-ui/app/elements/settings/gr-email-editor/gr-email-editor.ts b/polygerrit-ui/app/elements/settings/gr-email-editor/gr-email-editor.ts index 74c6ac0..7f25a86 100644 --- a/polygerrit-ui/app/elements/settings/gr-email-editor/gr-email-editor.ts +++ b/polygerrit-ui/app/elements/settings/gr-email-editor/gr-email-editor.ts
@@ -105,6 +105,10 @@ } } } + + _checkPreferred(preferred?: boolean) { + return preferred ?? false; + } } declare global {
diff --git a/polygerrit-ui/app/elements/settings/gr-email-editor/gr-email-editor_html.ts b/polygerrit-ui/app/elements/settings/gr-email-editor/gr-email-editor_html.ts index 0591e4b..666afb7 100644 --- a/polygerrit-ui/app/elements/settings/gr-email-editor/gr-email-editor_html.ts +++ b/polygerrit-ui/app/elements/settings/gr-email-editor/gr-email-editor_html.ts
@@ -71,12 +71,10 @@ checked$="[[item.preferred]]" > <input - is="iron-input" class="preferredRadio" type="radio" on-change="_handlePreferredChange" name="preferred" - value="[[item.email]]" checked$="[[item.preferred]]" /> </iron-input> @@ -85,7 +83,7 @@ <gr-button data-index$="[[index]]" on-click="_handleDeleteButton" - disabled="[[item.preferred]]" + disabled="[[_checkPreferred(item.preferred)]]" class="remove-button" >Delete</gr-button >
diff --git a/polygerrit-ui/app/elements/settings/gr-gpg-editor/gr-gpg-editor_html.ts b/polygerrit-ui/app/elements/settings/gr-gpg-editor/gr-gpg-editor_html.ts index ab24168..f4641c2 100644 --- a/polygerrit-ui/app/elements/settings/gr-gpg-editor/gr-gpg-editor_html.ts +++ b/polygerrit-ui/app/elements/settings/gr-gpg-editor/gr-gpg-editor_html.ts
@@ -70,9 +70,9 @@ </td> <td> <gr-copy-clipboard - has-tooltip="" - button-title="Copy GPG public key to clipboard" - hide-input="" + hasTooltip="" + buttonTitle="Copy GPG public key to clipboard" + hideInput="" text="[[key.key]]" > </gr-copy-clipboard>
diff --git a/polygerrit-ui/app/elements/settings/gr-http-password/gr-http-password_html.ts b/polygerrit-ui/app/elements/settings/gr-http-password/gr-http-password_html.ts index 549fc93..811b85c 100644 --- a/polygerrit-ui/app/elements/settings/gr-http-password/gr-http-password_html.ts +++ b/polygerrit-ui/app/elements/settings/gr-http-password/gr-http-password_html.ts
@@ -78,9 +78,9 @@ <span class="title">New Password:</span> <span class="value">[[_generatedPassword]]</span> <gr-copy-clipboard - has-tooltip="" - button-title="Copy password to clipboard" - hide-input="" + hasTooltip="" + buttonTitle="Copy password to clipboard" + hideInput="" text="[[_generatedPassword]]" > </gr-copy-clipboard>
diff --git a/polygerrit-ui/app/elements/settings/gr-identities/gr-identities_test.js b/polygerrit-ui/app/elements/settings/gr-identities/gr-identities_test.js deleted file mode 100644 index 867473f..0000000 --- a/polygerrit-ui/app/elements/settings/gr-identities/gr-identities_test.js +++ /dev/null
@@ -1,168 +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-identities.js'; -import {stubRestApi} from '../../../test/test-utils.js'; - -const basicFixture = fixtureFromElement('gr-identities'); - -suite('gr-identities tests', () => { - let element; - - const ids = [ - { - identity: 'username:john', - email_address: 'john.doe@example.com', - trusted: true, - }, { - identity: 'gerrit:gerrit', - email_address: 'gerrit@example.com', - }, { - identity: 'mailto:gerrit2@example.com', - email_address: 'gerrit2@example.com', - trusted: true, - can_delete: true, - }, - ]; - - setup(async () => { - stubRestApi('getExternalIds').returns(Promise.resolve(ids)); - - element = basicFixture.instantiate(); - await element.loadData(); - await flush(); - }); - - test('renders', () => { - const rows = Array.from( - element.root.querySelectorAll('tbody tr')); - - assert.equal(rows.length, 2); - - const nameCells = rows.map(row => - row.querySelectorAll('td')[2].textContent - ); - - assert.equal(nameCells[0].trim(), 'gerrit:gerrit'); - assert.equal(nameCells[1].trim(), ''); - }); - - test('renders email', () => { - const rows = Array.from( - element.root.querySelectorAll('tbody tr')); - - assert.equal(rows.length, 2); - - const nameCells = rows.map(row => - row.querySelectorAll('td')[1].textContent - ); - - assert.equal(nameCells[0], 'gerrit@example.com'); - assert.equal(nameCells[1], 'gerrit2@example.com'); - }); - - test('_computeIdentity', () => { - assert.equal( - element._computeIdentity(ids[0].identity), 'username:john'); - assert.equal(element._computeIdentity(ids[2].identity), ''); - }); - - test('filterIdentities', () => { - assert.isFalse(element.filterIdentities(ids[0])); - - assert.isTrue(element.filterIdentities(ids[1])); - }); - - test('delete id', done => { - element._idName = 'mailto:gerrit2@example.com'; - const loadDataStub = sinon.stub(element, 'loadData'); - element._handleDeleteItemConfirm().then(() => { - assert.isTrue(loadDataStub.called); - done(); - }); - }); - - test('_handleDeleteItem opens modal', () => { - const deleteBtn = - element.root.querySelector('.deleteButton'); - const deleteItem = sinon.stub(element, '_handleDeleteItem'); - MockInteractions.tap(deleteBtn); - assert.isTrue(deleteItem.called); - }); - - test('_computeShowLinkAnotherIdentity', () => { - let serverConfig; - - serverConfig = { - auth: { - git_basic_auth_policy: 'OAUTH', - }, - }; - assert.isTrue(element._computeShowLinkAnotherIdentity(serverConfig)); - - serverConfig = { - auth: { - git_basic_auth_policy: 'OpenID', - }, - }; - assert.isTrue(element._computeShowLinkAnotherIdentity(serverConfig)); - - serverConfig = { - auth: { - git_basic_auth_policy: 'HTTP_LDAP', - }, - }; - assert.isFalse(element._computeShowLinkAnotherIdentity(serverConfig)); - - serverConfig = { - auth: { - git_basic_auth_policy: 'LDAP', - }, - }; - assert.isFalse(element._computeShowLinkAnotherIdentity(serverConfig)); - - serverConfig = { - auth: { - git_basic_auth_policy: 'HTTP', - }, - }; - assert.isFalse(element._computeShowLinkAnotherIdentity(serverConfig)); - - serverConfig = {}; - assert.isFalse(element._computeShowLinkAnotherIdentity(serverConfig)); - }); - - test('_showLinkAnotherIdentity', () => { - element.serverConfig = { - auth: { - git_basic_auth_policy: 'OAUTH', - }, - }; - - assert.isTrue(element._showLinkAnotherIdentity); - - element.serverConfig = { - auth: { - git_basic_auth_policy: 'LDAP', - }, - }; - - assert.isFalse(element._showLinkAnotherIdentity); - }); -}); -
diff --git a/polygerrit-ui/app/elements/settings/gr-identities/gr-identities_test.ts b/polygerrit-ui/app/elements/settings/gr-identities/gr-identities_test.ts new file mode 100644 index 0000000..d1a4f8f --- /dev/null +++ b/polygerrit-ui/app/elements/settings/gr-identities/gr-identities_test.ts
@@ -0,0 +1,148 @@ +/** + * @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-identities'; +import {GrIdentities} from './gr-identities'; +import {stubRestApi} from '../../../test/test-utils'; +import {ServerInfo} from '../../../types/common'; +import {createServerInfo} from '../../../test/test-data-generators'; +import {queryAll, queryAndAssert} from '../../../test/test-utils'; +import * as MockInteractions from '@polymer/iron-test-helpers/mock-interactions'; + +const basicFixture = fixtureFromElement('gr-identities'); + +suite('gr-identities tests', () => { + let element: GrIdentities; + + const ids = [ + { + identity: 'username:john', + email_address: 'john.doe@example.com', + trusted: true, + }, + { + identity: 'gerrit:gerrit', + email_address: 'gerrit@example.com', + }, + { + identity: 'mailto:gerrit2@example.com', + email_address: 'gerrit2@example.com', + trusted: true, + can_delete: true, + }, + ]; + + setup(async () => { + stubRestApi('getExternalIds').returns(Promise.resolve(ids)); + + element = basicFixture.instantiate(); + await element.loadData(); + await flush(); + }); + + test('renders', () => { + const rows = Array.from(queryAll(element, 'tbody tr')); + + assert.equal(rows.length, 2); + + const nameCells = rows.map(row => queryAll(row, 'td')[2].textContent); + + assert.equal(nameCells[0]!.trim(), 'gerrit:gerrit'); + assert.equal(nameCells[1]!.trim(), ''); + }); + + test('renders email', () => { + const rows = Array.from(queryAll(element, 'tbody tr')); + + assert.equal(rows.length, 2); + + const nameCells = rows.map(row => queryAll(row, 'td')[1]!.textContent); + + assert.equal(nameCells[0]!, 'gerrit@example.com'); + assert.equal(nameCells[1]!, 'gerrit2@example.com'); + }); + + test('_computeIdentity', () => { + assert.equal(element._computeIdentity(ids[0].identity), 'username:john'); + assert.equal(element._computeIdentity(ids[2].identity), ''); + }); + + test('filterIdentities', () => { + assert.isFalse(element.filterIdentities(ids[0])); + + assert.isTrue(element.filterIdentities(ids[1])); + }); + + test('delete id', done => { + element._idName = 'mailto:gerrit2@example.com'; + const loadDataStub = sinon.stub(element, 'loadData'); + element._handleDeleteItemConfirm().then(() => { + assert.isTrue(loadDataStub.called); + done(); + }); + }); + + test('_handleDeleteItem opens modal', () => { + const deleteBtn = queryAndAssert(element, '.deleteButton'); + const deleteItem = sinon.stub(element, '_handleDeleteItem'); + MockInteractions.tap(deleteBtn); + assert.isTrue(deleteItem.called); + }); + + test('_computeShowLinkAnotherIdentity', () => { + const config: ServerInfo = { + ...createServerInfo(), + }; + + config.auth.git_basic_auth_policy = 'OAUTH'; + assert.isTrue(element._computeShowLinkAnotherIdentity(config)); + + config.auth.git_basic_auth_policy = 'OpenID'; + assert.isTrue(element._computeShowLinkAnotherIdentity(config)); + + config.auth.git_basic_auth_policy = 'HTTP_LDAP'; + assert.isFalse(element._computeShowLinkAnotherIdentity(config)); + + config.auth.git_basic_auth_policy = 'LDAP'; + assert.isFalse(element._computeShowLinkAnotherIdentity(config)); + + config.auth.git_basic_auth_policy = 'HTTP'; + assert.isFalse(element._computeShowLinkAnotherIdentity(config)); + + assert.isFalse(element._computeShowLinkAnotherIdentity(undefined)); + }); + + test('_showLinkAnotherIdentity', () => { + let config: ServerInfo = { + ...createServerInfo(), + }; + config.auth.git_basic_auth_policy = 'OAUTH'; + + element.serverConfig = config; + + assert.isTrue(element._showLinkAnotherIdentity); + + config = { + ...createServerInfo(), + }; + config.auth.git_basic_auth_policy = 'LDAP'; + element.serverConfig = config; + + assert.isFalse(element._showLinkAnotherIdentity); + }); +});
diff --git a/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog_html.ts b/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog_html.ts index 4b86709..6f270f5 100644 --- a/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog_html.ts +++ b/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog_html.ts
@@ -75,12 +75,7 @@ <span class="title">Full Name</span> <span class="value"> <iron-input bind-value="{{_account.name}}"> - <input - is="iron-input" - id="name" - bind-value="{{_account.name}}" - disabled="[[_saving]]" - /> + <input id="name" disabled="[[_saving]]" /> </iron-input> </span> </section> @@ -92,12 +87,7 @@ > <span hidden$="[[!_usernameMutable]]" class="value"> <iron-input bind-value="{{_username}}"> - <input - is="iron-input" - id="username" - bind-value="{{_username}}" - disabled="[[_saving]]" - /> + <input id="username" disabled="[[_saving]]" /> </iron-input> </span> </section>
diff --git a/polygerrit-ui/app/elements/settings/gr-ssh-editor/gr-ssh-editor_html.ts b/polygerrit-ui/app/elements/settings/gr-ssh-editor/gr-ssh-editor_html.ts index 0bee1d3..e853b58 100644 --- a/polygerrit-ui/app/elements/settings/gr-ssh-editor/gr-ssh-editor_html.ts +++ b/polygerrit-ui/app/elements/settings/gr-ssh-editor/gr-ssh-editor_html.ts
@@ -76,9 +76,9 @@ </td> <td> <gr-copy-clipboard - has-tooltip="" - button-title="Copy SSH public key to clipboard" - hide-input="" + hasTooltip="" + buttonTitle="Copy SSH public key to clipboard" + hideInput="" text="[[key.ssh_public_key]]" > </gr-copy-clipboard>
diff --git a/polygerrit-ui/app/elements/settings/gr-watched-projects-editor/gr-watched-projects-editor.ts b/polygerrit-ui/app/elements/settings/gr-watched-projects-editor/gr-watched-projects-editor.ts index 6178e7a..4381a59 100644 --- a/polygerrit-ui/app/elements/settings/gr-watched-projects-editor/gr-watched-projects-editor.ts +++ b/polygerrit-ui/app/elements/settings/gr-watched-projects-editor/gr-watched-projects-editor.ts
@@ -31,6 +31,7 @@ import {hasOwnProperty} from '../../../utils/common-util'; import {ProjectWatchInfo} from '../../../types/common'; import {appContext} from '../../../services/app-context'; +import {IronInputElement} from '@polymer/iron-input'; const NOTIFICATION_TYPES = [ {name: 'Changes', key: 'notify_new_changes'}, @@ -43,9 +44,11 @@ export interface GrWatchedProjectsEditor { $: { newFilter: HTMLInputElement; + newFilterInput: IronInputElement; newProject: GrAutocomplete; }; } + @customElement('gr-watched-projects-editor') export class GrWatchedProjectsEditor extends PolymerElement { static get template() { @@ -62,7 +65,7 @@ _projectsToRemove: ProjectWatchInfo[] = []; @property({type: Object}) - _query?: AutocompleteQuery; + _query: AutocompleteQuery; private readonly restApiService = appContext.restApiService;
diff --git a/polygerrit-ui/app/elements/settings/gr-watched-projects-editor/gr-watched-projects-editor_html.ts b/polygerrit-ui/app/elements/settings/gr-watched-projects-editor/gr-watched-projects-editor_html.ts index edc8fb2..fb65a03 100644 --- a/polygerrit-ui/app/elements/settings/gr-watched-projects-editor/gr-watched-projects-editor_html.ts +++ b/polygerrit-ui/app/elements/settings/gr-watched-projects-editor/gr-watched-projects-editor_html.ts
@@ -102,13 +102,13 @@ </th> <th colspan$="[[_getTypeCount()]]"> <iron-input + id="newFilterInput" class="newFilterInput" placeholder="branch:name, or other search expression" > <input id="newFilter" class="newFilterInput" - is="iron-input" placeholder="branch:name, or other search expression" /> </iron-input>
diff --git a/polygerrit-ui/app/elements/settings/gr-watched-projects-editor/gr-watched-projects-editor_test.js b/polygerrit-ui/app/elements/settings/gr-watched-projects-editor/gr-watched-projects-editor_test.ts similarity index 76% rename from polygerrit-ui/app/elements/settings/gr-watched-projects-editor/gr-watched-projects-editor_test.js rename to polygerrit-ui/app/elements/settings/gr-watched-projects-editor/gr-watched-projects-editor_test.ts index aac8995..cb4b86d 100644 --- a/polygerrit-ui/app/elements/settings/gr-watched-projects-editor/gr-watched-projects-editor_test.js +++ b/polygerrit-ui/app/elements/settings/gr-watched-projects-editor/gr-watched-projects-editor_test.ts
@@ -15,14 +15,18 @@ * limitations under the License. */ -import '../../../test/common-test-setup-karma.js'; -import './gr-watched-projects-editor.js'; -import {stubRestApi} from '../../../test/test-utils.js'; +import '../../../test/common-test-setup-karma'; +import './gr-watched-projects-editor'; +import {GrWatchedProjectsEditor} from './gr-watched-projects-editor'; +import {stubRestApi} from '../../../test/test-utils'; +import {ProjectWatchInfo} from '../../../types/common'; +import {queryAll, queryAndAssert} from '../../../test/test-utils'; +import * as MockInteractions from '@polymer/iron-test-helpers/mock-interactions'; const basicFixture = fixtureFromElement('gr-watched-projects-editor'); suite('gr-watched-projects-editor tests', () => { - let element; + let element: GrWatchedProjectsEditor; setup(done => { const projects = [ @@ -30,29 +34,34 @@ project: 'project a', notify_submitted_changes: true, notify_abandoned_changes: true, - }, { + }, + { project: 'project b', filter: 'filter 1', notify_new_changes: true, - }, { + }, + { project: 'project b', filter: 'filter 2', - }, { + }, + { project: 'project c', notify_new_changes: true, notify_new_patch_sets: true, notify_all_comments: true, }, - ]; + ] as ProjectWatchInfo[]; stubRestApi('getWatchedProjects').returns(Promise.resolve(projects)); stubRestApi('getSuggestedProjects').callsFake(input => { if (input.startsWith('th')) { - return Promise.resolve({'the project': { - id: 'the project', - state: 'ACTIVE', - web_links: [], - }}); + return Promise.resolve({ + 'the project': { + id: 'the project', + state: 'ACTIVE', + web_links: [], + }, + }); } else { return Promise.resolve({}); } @@ -60,18 +69,18 @@ element = basicFixture.instantiate(); - element.loadData().then(() => { flush(done); }); + element.loadData().then(() => { + flush(done); + }); }); test('renders', () => { - const rows = element.shadowRoot - .querySelector('table').querySelectorAll('tbody tr'); + const rows = queryAndAssert(element, 'table').querySelectorAll('tbody tr'); assert.equal(rows.length, 4); - function getKeysOfRow(row) { - const boxes = rows[row].querySelectorAll('input[checked]'); - return Array.prototype.map.call(boxes, - e => e.getAttribute('data-key')); + function getKeysOfRow(row: number) { + const boxes = queryAll(rows[row], 'input[checked]'); + return Array.prototype.map.call(boxes, e => e.getAttribute('data-key')); } let checkedKeys = getKeysOfRow(0); @@ -157,41 +166,44 @@ test('_handleAddProject', () => { element.$.newProject.value = 'project d'; element.$.newProject.setText('project d'); - element.$.newFilter.bindValue = ''; + element.$.newFilterInput.bindValue = ''; element._handleAddProject(); - assert.equal(element._projects.length, 5); - assert.equal(element._projects[4].project, 'project d'); - assert.isNotOk(element._projects[4].filter); - assert.isTrue(element._projects[4]._is_local); + const projects = element._projects!; + assert.equal(projects.length, 5); + assert.equal(projects[4].project, 'project d'); + assert.isNotOk(projects[4].filter); + assert.isTrue(projects[4]._is_local); }); test('_handleAddProject with invalid inputs', () => { element.$.newProject.value = 'project b'; element.$.newProject.setText('project b'); - element.$.newFilter.bindValue = 'filter 1'; + element.$.newFilterInput.bindValue = 'filter 1'; element.$.newFilter.value = 'filter 1'; element._handleAddProject(); - assert.equal(element._projects.length, 4); + assert.equal(element._projects!.length, 4); }); test('_handleRemoveProject', () => { - assert.equal(element._projectsToRemove, 0); - const button = element.shadowRoot - .querySelector('table tbody tr:nth-child(2) gr-button'); + assert.deepEqual(element._projectsToRemove, []); + + const button = queryAndAssert( + element, + 'table tbody tr:nth-child(2) gr-button' + ); MockInteractions.tap(button); flush(); - const rows = element.shadowRoot - .querySelector('table tbody').querySelectorAll('tr'); + const rows = queryAndAssert(element, 'table tbody').querySelectorAll('tr'); + assert.equal(rows.length, 3); assert.equal(element._projectsToRemove.length, 1); assert.equal(element._projectsToRemove[0].project, 'project b'); }); }); -
diff --git a/polygerrit-ui/app/elements/shared/gr-account-chip/gr-account-chip.ts b/polygerrit-ui/app/elements/shared/gr-account-chip/gr-account-chip.ts index dab778b..f703037 100644 --- a/polygerrit-ui/app/elements/shared/gr-account-chip/gr-account-chip.ts +++ b/polygerrit-ui/app/elements/shared/gr-account-chip/gr-account-chip.ts
@@ -17,19 +17,14 @@ import '../gr-account-link/gr-account-link'; import '../gr-button/gr-button'; import '../gr-icons/gr-icons'; -import '../../../styles/shared-styles'; -import {PolymerElement} from '@polymer/polymer/polymer-element'; -import {htmlTemplate} from './gr-account-chip_html'; -import {customElement, property} from '@polymer/decorators'; import {AccountInfo, ChangeInfo} from '../../../types/common'; import {appContext} from '../../../services/app-context'; +import {GrLitElement} from '../../lit/gr-lit-element'; +import {css, customElement, html, property} from 'lit-element'; +import {classMap} from 'lit-html/directives/class-map'; @customElement('gr-account-chip') -export class GrAccountChip extends PolymerElement { - static get template() { - return htmlTemplate; - } - +export class GrAccountChip extends GrLitElement { /** * Fired to indicate a key was pressed while this chip was focused. * @@ -64,10 +59,10 @@ @property({type: String}) voteableText?: string; - @property({type: Boolean, reflectToAttribute: true}) + @property({type: Boolean, reflect: true}) disabled = false; - @property({type: Boolean, reflectToAttribute: true}) + @property({type: Boolean, reflect: true}) removable = false; /** @@ -78,7 +73,7 @@ @property({type: Boolean}) highlightAttention = false; - @property({type: Boolean, reflectToAttribute: true}) + @property({type: Boolean, reflect: true}) showAvatar?: boolean; @property({type: Boolean}) @@ -86,18 +81,126 @@ private readonly restApiService = appContext.restApiService; - /** @override */ - ready() { - super.ready(); + static get styles() { + return [ + css` + :host { + display: block; + overflow: hidden; + } + .container { + align-items: center; + background-color: var(--background-color-primary); + /** round */ + border-radius: var(--account-chip-border-radius, 20px); + border: 1px solid var(--border-color); + display: inline-flex; + padding: 0 1px; + } + :host:focus { + border-color: transparent; + box-shadow: none; + outline: none; + } + :host:focus .container, + :host:focus gr-button { + background: #ccc; + } + .transparentBackground, + gr-button.transparentBackground { + background-color: transparent; + } + :host([disabled]) { + opacity: 0.6; + pointer-events: none; + } + iron-icon { + height: 1.2rem; + width: 1.2rem; + } + .container gr-account-link::part(gr-account-link-text) { + color: var(--deemphasized-text-color); + } + `, + ]; + } + + render() { + // To pass CSS mixins for @apply to Polymer components, they need to appear + // in <style> inside the template. + const customStyle = html` + <style> + .container { + --account-label-padding-horizontal: 6px; + } + gr-button.remove { + --gr-remove-button-style: { + border-top-width: 0; + border-right-width: 0; + border-bottom-width: 0; + border-left-width: 0; + color: var(--deemphasized-text-color); + font-weight: var(--font-weight-normal); + height: 0.6em; + line-height: 10px; + /* This cancels most of the --account-label-padding-horizontal. */ + margin-left: -4px; + padding: 0 2px 0 0; + text-decoration: none; + } + } + + gr-button.remove:hover, + gr-button.remove:focus { + --gr-button: { + @apply --gr-remove-button-style; + } + } + gr-button.remove { + --gr-button: { + @apply --gr-remove-button-style; + } + } + </style> + `; + return html`${customStyle} + <div + class="${classMap({ + container: true, + transparentBackground: this.transparentBackground, + })}" + > + <gr-account-link + .account="${this.account}" + .change="${this.change}" + ?forceAttention=${this.forceAttention} + ?highlightAttention=${this.highlightAttention} + .voteableText=${this.voteableText} + > + </gr-account-link> + <gr-button + id="remove" + link="" + ?hidden=${!this.removable} + aria-label="Remove" + class="${classMap({ + remove: true, + transparentBackground: this.transparentBackground, + })}" + @click=${this._handleRemoveTap} + > + <iron-icon icon="gr-icons:close"></iron-icon> + </gr-button> + </div>`; + } + + constructor() { + super(); this._getHasAvatars().then(hasAvatars => { this.showAvatar = hasAvatars; }); } - _getBackgroundClass(transparent: boolean) { - return transparent ? 'transparentBackground' : ''; - } - _handleRemoveTap(e: MouseEvent) { e.preventDefault(); this.dispatchEvent(
diff --git a/polygerrit-ui/app/elements/shared/gr-account-chip/gr-account-chip_html.ts b/polygerrit-ui/app/elements/shared/gr-account-chip/gr-account-chip_html.ts deleted file mode 100644 index e5efebb..0000000 --- a/polygerrit-ui/app/elements/shared/gr-account-chip/gr-account-chip_html.ts +++ /dev/null
@@ -1,114 +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> - :host { - display: block; - overflow: hidden; - } - .container { - align-items: center; - background-color: var(--background-color-primary); - /** round */ - border-radius: var(--account-chip-border-radius, 20px); - border: 1px solid var(--border-color); - display: inline-flex; - padding: 0 1px; - - --account-label-padding-horizontal: 6px; - --gr-account-label-text-style: { - color: var(--deemphasized-text-color); - } - } - :host([show-avatar]) .container { - } - :host([removable]) .container { - } - gr-button.remove { - --gr-remove-button-style: { - border-top-width: 0; - border-right-width: 0; - border-bottom-width: 0; - border-left-width: 0; - color: var(--deemphasized-text-color); - font-weight: var(--font-weight-normal); - height: 0.6em; - line-height: 10px; - /* This cancels most of the --account-label-padding-horizontal. */ - margin-left: -4px; - padding: 0 2px 0 0; - text-decoration: none; - } - } - - gr-button.remove:hover, - gr-button.remove:focus { - --gr-button: { - @apply --gr-remove-button-style; - } - } - gr-button.remove { - --gr-button: { - @apply --gr-remove-button-style; - } - } - :host:focus { - border-color: transparent; - box-shadow: none; - outline: none; - } - :host:focus .container, - :host:focus gr-button { - background: #ccc; - } - .transparentBackground, - gr-button.transparentBackground { - background-color: transparent; - } - :host([disabled]) { - opacity: 0.6; - pointer-events: none; - } - iron-icon { - height: 1.2rem; - width: 1.2rem; - } - </style> - <div class$="container [[_getBackgroundClass(transparentBackground)]]"> - <gr-account-link - account="[[account]]" - change="[[change]]" - force-attention="[[forceAttention]]" - highlight-attention="[[highlightAttention]]" - voteable-text="[[voteableText]]" - > - </gr-account-link> - <gr-button - id="remove" - link="" - hidden$="[[!removable]]" - hidden="" - aria-label="Remove" - class$="remove [[_getBackgroundClass(transparentBackground)]]" - on-click="_handleRemoveTap" - > - <iron-icon icon="gr-icons:close"></iron-icon> - </gr-button> - </div> -`;
diff --git a/polygerrit-ui/app/elements/shared/gr-account-entry/gr-account-entry.ts b/polygerrit-ui/app/elements/shared/gr-account-entry/gr-account-entry.ts index 944054e..c250428 100644 --- a/polygerrit-ui/app/elements/shared/gr-account-entry/gr-account-entry.ts +++ b/polygerrit-ui/app/elements/shared/gr-account-entry/gr-account-entry.ts
@@ -54,13 +54,13 @@ */ @property({type: Boolean}) - allowAnyInput?: boolean; + allowAnyInput = false; @property({type: Boolean}) - borderless?: boolean; + borderless = false; @property({type: String}) - placeholder?: string; + placeholder = ''; @property({type: Number}) suggestFrom = 0; @@ -69,7 +69,7 @@ querySuggestions: AutocompleteQuery = () => Promise.resolve([]); @property({type: String, observer: '_inputTextChanged'}) - _inputText?: string; + _inputText = ''; get focusStart() { return this.$.input.focusStart;
diff --git a/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label.ts b/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label.ts index d7078ed..cc66734 100644 --- a/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label.ts +++ b/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label.ts
@@ -190,11 +190,7 @@ : ''; } - _computeName( - account?: AccountInfo, - config?: ServerInfo, - firstName?: boolean - ) { + _computeName(account: AccountInfo, firstName: boolean, config?: ServerInfo) { return getDisplayName(config, account, firstName); } @@ -264,12 +260,12 @@ highlight: boolean, account: AccountInfo, change: ChangeInfo, - selfAccount: AccountInfo, - selected: boolean + selected: boolean, + selfAccount?: AccountInfo ) { if (selected) return true; return ( - this._hasUnforcedAttention(highlight, account, change) && + !!this._hasUnforcedAttention(highlight, account, change) && (isInvolved(change, selfAccount) || isSelf(account, selfAccount)) ); } @@ -278,16 +274,16 @@ highlight: boolean, account: AccountInfo, change: ChangeInfo, - selfAccount: AccountInfo, force: boolean, - selected: boolean + selected: boolean, + selfAccount?: AccountInfo ) { const enabled = this._computeAttentionButtonEnabled( highlight, account, change, - selfAccount, - selected + selected, + selfAccount ); return enabled ? 'Click to remove the user from the attention set'
diff --git a/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label_html.ts b/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label_html.ts index a642337..352763b 100644 --- a/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label_html.ts +++ b/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label_html.ts
@@ -59,12 +59,6 @@ position: relative; top: 1px; } - .text { - @apply --gr-account-label-text-style; - } - .text:hover { - @apply --gr-account-label-text-hover-style; - } #attentionButton { /* This negates the 4px horizontal padding, which we appreciate as a larger click target, but which we don't want to consume space. :-) */ @@ -87,6 +81,7 @@ } .name { display: inline-block; + text-decoration: inherit; vertical-align: top; overflow: hidden; text-overflow: ellipsis; @@ -116,9 +111,9 @@ link="" aria-label="Remove user from attention set" on-click="_handleRemoveAttentionClick" - disabled="[[!_computeAttentionButtonEnabled(highlightAttention, account, change, _selfAccount, selected)]]" - has-tooltip="[[_computeAttentionButtonEnabled(highlightAttention, account, change, _selfAccount, false)]]" - title="[[_computeAttentionIconTitle(highlightAttention, account, change, _selfAccount, forceAttention, selected)]]" + disabled="[[!_computeAttentionButtonEnabled(highlightAttention, account, change, selected, _selfAccount)]]" + has-tooltip="[[_computeAttentionButtonEnabled(highlightAttention, account, change, false, _selfAccount)]]" + title="[[_computeAttentionIconTitle(highlightAttention, account, change, forceAttention, selected, _selfAccount)]]" ><iron-icon class="attention" icon="gr-icons:attention"></iron-icon> </gr-button> </template> @@ -130,8 +125,8 @@ <template is="dom-if" if="[[!hideAvatar]]"> <gr-avatar account="[[account]]" imageSize="32"></gr-avatar> </template> - <span class="text"> - <span class="name">[[_computeName(account, _config, firstName)]]</span> + <span class="text" part="gr-account-label-text"> + <span class="name">[[_computeName(account, firstName, _config)]]</span> <template is="dom-if" if="[[!hideStatus]]"> <template is="dom-if" if="[[account.status]]"> <iron-icon class="status" icon="gr-icons:calendar"></iron-icon>
diff --git a/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label_test.ts b/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label_test.ts index efaa9f7..574e450 100644 --- a/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label_test.ts +++ b/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label_test.ts
@@ -55,12 +55,12 @@ suite('_computeName', () => { test('not showing anonymous', () => { const account = {name: 'Wyatt'}; - assert.deepEqual(element._computeName(account), 'Wyatt'); + assert.deepEqual(element._computeName(account, false), 'Wyatt'); }); test('showing anonymous but no config', () => { const account = {}; - assert.deepEqual(element._computeName(account), 'Anonymous'); + assert.deepEqual(element._computeName(account, false), 'Anonymous'); }); test('test for Anonymous Coward user and replace with Anonymous', () => { @@ -71,7 +71,10 @@ }, }; const account = {}; - assert.deepEqual(element._computeName(account, config), 'Anonymous'); + assert.deepEqual( + element._computeName(account, false, config), + 'Anonymous' + ); }); test('test for anonymous_coward_name', () => { @@ -82,7 +85,10 @@ }, }; const account = {}; - assert.deepEqual(element._computeName(account, config), 'TestAnon'); + assert.deepEqual( + element._computeName(account, false, config), + 'TestAnon' + ); }); });
diff --git a/polygerrit-ui/app/elements/shared/gr-account-link/gr-account-link.ts b/polygerrit-ui/app/elements/shared/gr-account-link/gr-account-link.ts index 94a24e1..317806c 100644 --- a/polygerrit-ui/app/elements/shared/gr-account-link/gr-account-link.ts +++ b/polygerrit-ui/app/elements/shared/gr-account-link/gr-account-link.ts
@@ -18,10 +18,11 @@ import '../gr-account-label/gr-account-label'; import {GerritNav} from '../../core/gr-navigation/gr-navigation'; import {AccountInfo, ChangeInfo} from '../../../types/common'; -import {css, customElement, html, LitElement, property} from 'lit-element'; +import {css, customElement, html, property} from 'lit-element'; +import {GrLitElement} from '../../lit/gr-lit-element'; @customElement('gr-account-link') -export class GrAccountLink extends LitElement { +export class GrAccountLink extends GrLitElement { @property({type: String}) voteableText?: string; @@ -74,10 +75,8 @@ color: var(--primary-text-color); text-decoration: none; } - gr-account-label { - --gr-account-label-text-hover-style: { - text-decoration: underline; - } + gr-account-label::part(gr-account-label-text):hover { + text-decoration: underline !important; } `, ]; @@ -96,6 +95,7 @@ ?hide-status=${this.hideStatus} ?first-name=${this.firstName} .voteable-text=${this.voteableText} + part="gr-account-link-text => gr-account-label-text" > </gr-account-label> </a>
diff --git a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_html.ts b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_html.ts index 43c949a..a848b2f 100644 --- a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_html.ts +++ b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_html.ts
@@ -236,12 +236,12 @@ margin-left: var(--spacing-s); } .headerLeft gr-account-label { - --gr-account-label-text-style: { - font-weight: var(--font-weight-bold); - } --account-max-length: 130px; width: 150px; } + .headerLeft gr-account-label::part(gr-account-label-text) { + font-weight: var(--font-weight-bold); + } .draft gr-account-label { width: unset; }
diff --git a/polygerrit-ui/app/elements/shared/gr-copy-clipboard/gr-copy-clipboard.ts b/polygerrit-ui/app/elements/shared/gr-copy-clipboard/gr-copy-clipboard.ts index 2fe6fed..4a2bcee 100644 --- a/polygerrit-ui/app/elements/shared/gr-copy-clipboard/gr-copy-clipboard.ts +++ b/polygerrit-ui/app/elements/shared/gr-copy-clipboard/gr-copy-clipboard.ts
@@ -15,16 +15,14 @@ * limitations under the License. */ import '@polymer/iron-input/iron-input'; -import '../../../styles/shared-styles'; import '../gr-button/gr-button'; import '../gr-icons/gr-icons'; -import {dom, EventApi} from '@polymer/polymer/lib/legacy/polymer.dom'; -import {PolymerElement} from '@polymer/polymer/polymer-element'; -import {htmlTemplate} from './gr-copy-clipboard_html'; -import {GrButton} from '../gr-button/gr-button'; -import {customElement, property} from '@polymer/decorators'; import {IronIconElement} from '@polymer/iron-icon'; -import {assertIsDefined} from '../../../utils/common-util'; +import {assertIsDefined, queryAndAssert} from '../../../utils/common-util'; +import {classMap} from 'lit-html/directives/class-map'; +import {css, customElement, html, property} from 'lit-element'; +import {GrLitElement} from '../../lit/gr-lit-element'; +import {GrButton} from '../gr-button/gr-button'; const COPY_TIMEOUT_MS = 1000; @@ -33,17 +31,8 @@ 'gr-copy-clipboard': GrCopyClipboard; } } - -export interface GrCopyClipboard { - $: {button: GrButton; icon: IronIconElement; input: HTMLInputElement}; -} - @customElement('gr-copy-clipboard') -export class GrCopyClipboard extends PolymerElement { - static get template() { - return htmlTemplate; - } - +export class GrCopyClipboard extends GrLitElement { @property({type: String}) text: string | undefined; @@ -56,29 +45,121 @@ @property({type: Boolean}) hideInput = false; - focusOnCopy() { - this.$.button.focus(); + static get styles() { + return [ + css` + .text { + align-items: center; + display: flex; + flex-wrap: wrap; + } + .copyText { + flex-grow: 1; + margin-right: var(--spacing-s); + } + .hideInput { + display: none; + } + input#input { + font-family: var(--monospace-font-family); + font-size: var(--font-size-mono); + line-height: var(--line-height-mono); + width: 100%; + } + /* + * Typically icons are 20px, which is the normal line-height. + * The copy icon is too prominent at 20px, so we choose 16px + * here, but add 2x2px padding below, so the entire + * component should still fit nicely into a normal inline + * layout flow. + */ + #icon { + height: 16px; + width: 16px; + } + iron-icon { + color: var(--deemphasized-text-color); + vertical-align: top; + } + `, + ]; } - _computeInputClass(hideInput: boolean) { - return hideInput ? 'hideInput' : ''; + render() { + // To pass CSS mixins for @apply to Polymer components, they need to appear + // in <style> inside the template. + const customStyle = html` + <style> + iron-icon { + --iron-icon-height: 20px; + --iron-icon-width: 20px; + } + gr-button { + --gr-button: { + padding: 2px; + } + } + </style> + `; + return html`${customStyle} + <div class="text"> + <iron-input + class="copyText" + type="text" + @click="${this._handleInputClick}" + readonly="" + bind-value=${this.text} + > + <input + id="input" + is="iron-input" + class="${classMap({hideInput: this.hideInput})}" + type="text" + @click="${this._handleInputClick}" + readonly="" + .value=${this.text} + part="text-container-style" + /> + </iron-input> + <gr-button + id="copy-clipboard-button" + link="" + ?has-tooltip=${this.hasTooltip} + class="copyToClipboard" + title="${this.buttonTitle}" + @click="${this._copyToClipboard}" + aria-label="Click to copy to clipboard" + > + <iron-icon id="icon" icon="gr-icons:content-copy"></iron-icon> + </gr-button> + </div> `; + } + + focusOnCopy() { + queryAndAssert<GrButton>(this, '#copy-clipboard-button').focus(); } _handleInputClick(e: MouseEvent) { e.preventDefault(); - ((dom(e) as EventApi).rootTarget as HTMLInputElement).select(); + const rootTarget = e.composedPath()[0]; + (rootTarget as HTMLInputElement).select(); } _copyToClipboard(e: MouseEvent) { e.preventDefault(); e.stopPropagation(); + this.text = queryAndAssert<HTMLInputElement>(this, '#input').value; assertIsDefined(this.text, 'text'); - this.$.icon.icon = 'gr-icons:check'; + this.iconEl.icon = 'gr-icons:check'; navigator.clipboard.writeText(this.text); setTimeout( - () => (this.$.icon.icon = 'gr-icons:content-copy'), + () => (this.iconEl.icon = 'gr-icons:content-copy'), COPY_TIMEOUT_MS ); } + + private get iconEl(): IronIconElement { + return queryAndAssert<IronIconElement>(this, '#icon'); + } }
diff --git a/polygerrit-ui/app/elements/shared/gr-copy-clipboard/gr-copy-clipboard_html.ts b/polygerrit-ui/app/elements/shared/gr-copy-clipboard/gr-copy-clipboard_html.ts deleted file mode 100644 index 3ccc46f..0000000 --- a/polygerrit-ui/app/elements/shared/gr-copy-clipboard/gr-copy-clipboard_html.ts +++ /dev/null
@@ -1,93 +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> - .text { - align-items: center; - display: flex; - flex-wrap: wrap; - } - .copyText { - flex-grow: 1; - margin-right: var(--spacing-s); - } - .hideInput { - display: none; - } - input#input { - font-family: var(--monospace-font-family); - font-size: var(--font-size-mono); - line-height: var(--line-height-mono); - @apply --text-container-style; - width: 100%; - } - /* - * Typically icons are 20px, which is the normal line-height. - * The copy icon is too prominent at 20px, so we choose 16px - * here, but add 2x2px padding below, so the entire - * component should still fit nicely into a normal inline - * layout flow. - */ - #icon { - height: 16px; - width: 16px; - } - iron-icon { - color: var(--deemphasized-text-color); - vertical-align: top; - --iron-icon-height: 20px; - --iron-icon-width: 20px; - } - gr-button { - --gr-button: { - padding: 2px; - } - } - </style> - <div class="text"> - <iron-input - class="copyText" - type="text" - bind-value="[[text]]" - on-click="_handleInputClick" - readonly="" - > - <input - id="input" - is="iron-input" - class$="[[_computeInputClass(hideInput)]]" - type="text" - bind-value="[[text]]" - on-click="_handleInputClick" - readonly="" - /> - </iron-input> - <gr-button - id="button" - link="" - has-tooltip="[[hasTooltip]]" - class="copyToClipboard" - title="[[buttonTitle]]" - on-click="_copyToClipboard" - aria-label="Click to copy to clipboard" - > - <iron-icon id="icon" icon="gr-icons:content-copy"></iron-icon> - </gr-button> - </div> -`;
diff --git a/polygerrit-ui/app/elements/shared/gr-copy-clipboard/gr-copy-clipboard_test.js b/polygerrit-ui/app/elements/shared/gr-copy-clipboard/gr-copy-clipboard_test.js index 55b2483..45847d7 100644 --- a/polygerrit-ui/app/elements/shared/gr-copy-clipboard/gr-copy-clipboard_test.js +++ b/polygerrit-ui/app/elements/shared/gr-copy-clipboard/gr-copy-clipboard_test.js
@@ -17,7 +17,7 @@ import '../../../test/common-test-setup-karma.js'; import './gr-copy-clipboard.js'; -import {dom} from '@polymer/polymer/lib/legacy/polymer.dom.js'; +import {queryAndAssert} from '../../../test/test-utils.js'; const basicFixture = fixtureFromElement('gr-copy-clipboard'); @@ -32,18 +32,18 @@ }); test('copy to clipboard', () => { - const clipboardSpy = sinon.spy(element, '_copyToClipboard'); + const clipboardSpy = sinon.spy(navigator.clipboard, 'writeText'); const copyBtn = element.shadowRoot .querySelector('.copyToClipboard'); - MockInteractions.tap(copyBtn); + MockInteractions.click(copyBtn); assert.isTrue(clipboardSpy.called); }); test('focusOnCopy', () => { element.focusOnCopy(); - assert.deepEqual(dom(element.root).activeElement, - element.shadowRoot - .querySelector('.copyToClipboard')); + const activeElement = element.shadowRoot.activeElement; + const button = element.shadowRoot.querySelector('.copyToClipboard'); + assert.deepEqual(activeElement, button); }); test('_handleInputClick', () => { @@ -58,16 +58,17 @@ assert.equal(inputElement.selectionEnd, element.text.length - 1); }); - test('hideInput', () => { + test('hideInput', async () => { // iron-input as parent should never be hidden as copy won't work // on nested hidden elements const ironInputElement = element.shadowRoot.querySelector('iron-input'); assert.notEqual(getComputedStyle(ironInputElement).display, 'none'); - assert.notEqual(getComputedStyle(element.$.input).display, 'none'); + const input = queryAndAssert(element, 'input'); + assert.notEqual(getComputedStyle(input).display, 'none'); element.hideInput = true; - flush(); - assert.equal(getComputedStyle(element.$.input).display, 'none'); + await flush(); + assert.equal(getComputedStyle(input).display, 'none'); }); test('stop events propagation', () => {
diff --git a/polygerrit-ui/app/elements/shared/gr-dropdown-list/gr-dropdown-list_html.ts b/polygerrit-ui/app/elements/shared/gr-dropdown-list/gr-dropdown-list_html.ts index c163924..18a46a0 100644 --- a/polygerrit-ui/app/elements/shared/gr-dropdown-list/gr-dropdown-list_html.ts +++ b/polygerrit-ui/app/elements/shared/gr-dropdown-list/gr-dropdown-list_html.ts
@@ -127,7 +127,7 @@ <span id="triggerText">[[text]]</span> <gr-copy-clipboard hidden="[[!showCopyForTriggerText]]" - hide-input="" + hideInput="" text="[[text]]" ></gr-copy-clipboard> </gr-button>
diff --git a/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text.ts b/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text.ts index 7298af7..0193197 100644 --- a/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text.ts +++ b/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text.ts
@@ -15,12 +15,10 @@ * limitations under the License. */ import '../gr-linked-text/gr-linked-text'; -import '../../../styles/shared-styles'; -import {PolymerElement} from '@polymer/polymer/polymer-element'; -import {customElement, property} from '@polymer/decorators'; -import {htmlTemplate} from './gr-formatted-text_html'; import {CommentLinks} from '../../../types/common'; import {appContext} from '../../../services/app-context'; +import {GrLitElement} from '../../lit/gr-lit-element'; +import {css, customElement, html, property} from 'lit-element'; const CODE_MARKER_PATTERN = /^(`{1,3})([^`]+?)\1$/; @@ -36,65 +34,73 @@ 'gr-formatted-text': GrFormattedText; } } - -export interface GrFormattedText { - $: { - container: HTMLElement; - }; -} - @customElement('gr-formatted-text') -export class GrFormattedText extends PolymerElement { - static get template() { - return htmlTemplate; - } - - @property({type: String, observer: '_contentChanged'}) +export class GrFormattedText extends GrLitElement { + @property({type: String}) content?: string; @property({type: Object}) config?: CommentLinks; - @property({type: Boolean}) + @property({type: Boolean, reflect: true}) noTrailingMargin = false; private readonly reporting = appContext.reportingService; - static get observers() { - return ['_contentOrConfigChanged(content, config)']; + static get styles() { + return [ + css` + :host { + display: block; + font-family: var(--font-family); + } + p, + ul, + code, + blockquote, + gr-linked-text.pre { + margin: 0 0 var(--spacing-m) 0; + } + p, + ul, + code, + blockquote { + max-width: var(--gr-formatted-text-prose-max-width, none); + } + :host([noTrailingMargin]) p:last-child, + :host([noTrailingMargin]) ul:last-child, + :host([noTrailingMargin]) blockquote:last-child, + :host([noTrailingMargin]) gr-linked-text.pre:last-child { + margin: 0; + } + code, + blockquote { + border-left: 1px solid #aaa; + padding: 0 var(--spacing-m); + } + code { + display: block; + white-space: pre-wrap; + color: var(--deemphasized-text-color); + } + li { + list-style-type: disc; + margin-left: var(--spacing-xl); + } + code, + gr-linked-text.pre { + font-family: var(--monospace-font-family); + font-size: var(--font-size-code); + /* usually 16px = 12px + 4px */ + line-height: calc(var(--font-size-code) + var(--spacing-s)); + } + `, + ]; } - /** @override */ - ready() { - super.ready(); - if (this.noTrailingMargin) { - this.classList.add('noTrailingMargin'); - } - } - - _contentChanged(content: string) { - // In the case where the config may not be set (perhaps due to the - // request for it still being in flight), set the content anyway to - // prevent waiting on the config to display the text. - if (this.config) return; - this._contentOrConfigChanged(content); - } - - /** - * Given a source string, update the DOM inside #container. - */ - _contentOrConfigChanged(content?: string) { - const container = this.$.container; - - // Remove existing content. - while (container.firstChild) { - container.removeChild(container.firstChild); - } - - // Add new content. - for (const node of this._computeNodes(this._computeBlocks(content))) { - if (node) container.appendChild(node); - } + render() { + const nodes = this._computeNodes(this._computeBlocks(this.content)); + return html`<div id="container">${nodes}</div>`; } /**
diff --git a/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text_html.ts b/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text_html.ts deleted file mode 100644 index 04e4954..0000000 --- a/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text_html.ts +++ /dev/null
@@ -1,67 +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> - :host { - display: block; - font-family: var(--font-family); - } - p, - ul, - code, - blockquote, - gr-linked-text.pre { - margin: 0 0 var(--spacing-m) 0; - } - p, - ul, - code, - blockquote { - max-width: var(--gr-formatted-text-prose-max-width, none); - } - :host(.noTrailingMargin) p:last-child, - :host(.noTrailingMargin) ul:last-child, - :host(.noTrailingMargin) blockquote:last-child, - :host(.noTrailingMargin) gr-linked-text.pre:last-child { - margin: 0; - } - code, - blockquote { - border-left: 1px solid #aaa; - padding: 0 var(--spacing-m); - } - code { - display: block; - white-space: pre-wrap; - color: var(--deemphasized-text-color); - } - li { - list-style-type: disc; - margin-left: var(--spacing-xl); - } - code, - gr-linked-text.pre { - font-family: var(--monospace-font-family); - font-size: var(--font-size-code); - /* usually 16px = 12px + 4px */ - line-height: calc(var(--font-size-code) + var(--spacing-s)); - } - </style> - <div id="container"></div> -`;
diff --git a/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text_test.js b/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text_test.js index fd5a9ba..3e05f11 100644 --- a/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text_test.js +++ b/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text_test.js
@@ -387,20 +387,5 @@ assert.equal(result[3].type, 'code'); assert.equal(result[4].type, 'quote'); }); - - test('_computeNodes called without config', () => { - const computeNodesSpy = sinon.spy(element, '_computeNodes'); - element.content = 'some text'; - assert.isTrue(computeNodesSpy.called); - }); - - test('_contentOrConfigChanged called with config', () => { - const contentStub = sinon.stub(element, '_contentChanged'); - const contentConfigStub = sinon.stub(element, '_contentOrConfigChanged'); - element.content = 'some text'; - element.config = {}; - assert.isTrue(contentStub.called); - assert.isTrue(contentConfigStub.called); - }); });
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-change-reply-js-api.ts b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-change-reply-js-api.ts index fc5a4aa..b8f5aff 100644 --- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-change-reply-js-api.ts +++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-change-reply-js-api.ts
@@ -26,6 +26,7 @@ ValueChangedDetail, } from '../../../api/change-reply'; import {appContext} from '../../../services/app-context'; +import {HookApi, PluginElement} from '../../../api/hook'; /** * GrChangeReplyInterface, provides a set of handy methods on reply dialog. @@ -58,7 +59,7 @@ addReplyTextChangedCallback(handler: ReplyChangedCallback) { this.reporting.trackApi(this.plugin, 'reply', 'addReplyTextChangedCb'); - const hookApi = this.plugin.hook('reply-text'); + const hookApi = this.plugin.hook('reply-text') as HookApi<PluginElement>; const registeredHandler = (e: Event) => { const ce = e as CustomEvent<ValueChangedDetail>; handler(ce.detail.value); @@ -79,7 +80,9 @@ addLabelValuesChangedCallback(handler: LabelsChangedCallback) { this.reporting.trackApi(this.plugin, 'reply', 'addLabelValuesChangedCb'); - const hookApi = this.plugin.hook('reply-label-scores'); + const hookApi = this.plugin.hook( + 'reply-label-scores' + ) as HookApi<PluginElement>; const registeredHandler = (e: Event) => { const ce = e as CustomEvent<LabelsChangedDetail>; handler(ce.detail);
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-endpoints.ts b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-endpoints.ts index 0cc95a3..f7475bf 100644 --- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-endpoints.ts +++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-endpoints.ts
@@ -16,7 +16,7 @@ */ import {PluginApi} from '../../../api/plugin'; import {notUndefined} from '../../../types/types'; -import {HookApi} from '../../../api/hook'; +import {HookApi, PluginElement} from '../../../api/hook'; // eslint-disable-next-line @typescript-eslint/no-explicit-any type Callback = (value: any) => void; @@ -26,7 +26,7 @@ plugin: PluginApi; pluginUrl?: URL; type?: string; - domHook?: HookApi; + domHook?: HookApi<PluginElement>; slot?: string; } @@ -36,7 +36,7 @@ slot?: string; type?: string; moduleName?: string; - domHook?: HookApi; + domHook?: HookApi<PluginElement>; } export class GrPluginEndpoints {
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-rest-api.ts b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-rest-api.ts index 150c45a..903fb2c 100644 --- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-rest-api.ts +++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-rest-api.ts
@@ -125,7 +125,7 @@ /** * Fetch and parse REST API response, if request succeeds. */ - send( + send<T>( method: HttpMethod, url: string, payload?: RequestPayload, @@ -167,7 +167,7 @@ Promise.reject(new Error(msg)) ); } else { - return this.restApi.getResponseObject(response); + return this.restApi.getResponseObject(response) as Promise<T>; } }) .catch(err => { @@ -180,29 +180,29 @@ }); } - get(url: string) { + get<T>(url: string) { this.reporting.trackApi(this.plugin, 'rest', 'get'); - return this.send(HttpMethod.GET, url); + return this.send<T>(HttpMethod.GET, url); } - post( + post<T>( url: string, payload?: RequestPayload, errFn?: ErrorCallback, contentType?: string ) { this.reporting.trackApi(this.plugin, 'rest', 'post'); - return this.send(HttpMethod.POST, url, payload, errFn, contentType); + return this.send<T>(HttpMethod.POST, url, payload, errFn, contentType); } - put( + put<T>( url: string, payload?: RequestPayload, errFn?: ErrorCallback, contentType?: string ) { this.reporting.trackApi(this.plugin, 'rest', 'put'); - return this.send(HttpMethod.PUT, url, payload, errFn, contentType); + return this.send<T>(HttpMethod.PUT, url, payload, errFn, contentType); } delete(url: string) {
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-public-js-api.ts b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-public-js-api.ts index fef471d..7d3911a 100644 --- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-public-js-api.ts +++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-public-js-api.ts
@@ -41,7 +41,7 @@ import {ChangeActionsPluginApi} from '../../../api/change-actions'; import {ChangeReplyPluginApi} from '../../../api/change-reply'; import {RestPluginApi} from '../../../api/rest'; -import {HookApi, RegisterOptions} from '../../../api/hook'; +import {HookApi, PluginElement, RegisterOptions} from '../../../api/hook'; import {AttributeHelperPluginApi} from '../../../api/attribute-helper'; /** @@ -108,11 +108,11 @@ /** * Registers an endpoint for the plugin. */ - registerCustomComponent( + registerCustomComponent<T extends PluginElement>( endpointName: string, moduleName?: string, options?: RegisterOptions - ): HookApi { + ): HookApi<T> { this.report.trackApi(this, 'plugin', 'registerCustomComponent'); return this._registerCustomComponent(endpointName, moduleName, options); } @@ -123,11 +123,11 @@ * Dynamic plugins are registered by specific prefix, such as * 'change-list-header'. */ - registerDynamicCustomComponent( + registerDynamicCustomComponent<T extends PluginElement>( endpointName: string, moduleName?: string, options?: RegisterOptions - ): HookApi { + ): HookApi<T> { this.report.trackApi(this, 'plugin', 'registerDynamicCustomComponent'); const fullEndpointName = `${endpointName}-${this.getPluginName()}`; return this._registerCustomComponent( @@ -138,16 +138,17 @@ ); } - _registerCustomComponent( + _registerCustomComponent<T extends PluginElement>( endpoint: string, moduleName?: string, options?: RegisterOptions, dynamicEndpoint?: string - ): HookApi { - const type = - options && options.replace ? EndpointType.REPLACE : EndpointType.DECORATE; - const slot = (options && options.slot) || ''; - const domHook = this.domHooks.getDomHook(endpoint, moduleName); + ): HookApi<T> { + const type = options?.replace + ? EndpointType.REPLACE + : EndpointType.DECORATE; + const slot = options?.slot ?? ''; + const domHook = this.domHooks.getDomHook<T>(endpoint, moduleName); moduleName = moduleName || domHook.getModuleName(); getPluginEndpoints().registerModule(this, { slot, @@ -164,7 +165,10 @@ * Returns instance of DOM hook API for endpoint. Creates a placeholder * element for the first call. */ - hook(endpointName: string, options?: RegisterOptions) { + hook<T extends PluginElement>( + endpointName: string, + options?: RegisterOptions + ): HookApi<T> { this.report.trackApi(this, 'plugin', 'hook'); return this.registerCustomComponent(endpointName, undefined, options); }
diff --git a/polygerrit-ui/app/elements/shared/gr-shell-command/gr-shell-command_html.ts b/polygerrit-ui/app/elements/shared/gr-shell-command/gr-shell-command_html.ts index 6c7f0d8..e43460d 100644 --- a/polygerrit-ui/app/elements/shared/gr-shell-command/gr-shell-command_html.ts +++ b/polygerrit-ui/app/elements/shared/gr-shell-command/gr-shell-command_html.ts
@@ -45,17 +45,15 @@ /* Should roughly match the height of .commandContainer without padding. */ line-height: 26px; } - .commandContainer gr-copy-clipboard { - --text-container-style: { - border: none; - } + .commandContainer gr-copy-clipboard::part(text-container-style) { + border: none; } </style> <label>[[label]]</label> <div class="commandContainer"> <gr-copy-clipboard text="[[command]]" - has-tooltip + hasTooltip button-title="[[tooltip]]" ></gr-copy-clipboard> </div>
diff --git a/polygerrit-ui/app/node_modules_licenses/licenses.ts b/polygerrit-ui/app/node_modules_licenses/licenses.ts index f04e224..bcdab0e 100644 --- a/polygerrit-ui/app/node_modules_licenses/licenses.ts +++ b/polygerrit-ui/app/node_modules_licenses/licenses.ts
@@ -405,6 +405,14 @@ packageLicenseFile: "LICENSE", } }, + { + name: "immer", + license: { + name: "immer", + type: LicenseTypes.Mit, + packageLicenseFile: "LICENSE", + } + } ]; export default packages;
diff --git a/polygerrit-ui/app/package.json b/polygerrit-ui/app/package.json index 4e18e47..d26dc97 100644 --- a/polygerrit-ui/app/package.json +++ b/polygerrit-ui/app/package.json
@@ -37,6 +37,7 @@ "@webcomponents/webcomponentsjs": "^1.3.3", "ba-linkify": "file:../../lib/ba-linkify/src/", "codemirror-minified": "^5.62.0", + "immer": "^9.0.5", "lit-element": "^2.5.1", "page": "^1.11.6", "polymer-bridges": "file:../../polymer-bridges/",
diff --git a/polygerrit-ui/app/services/checks/checks-service.ts b/polygerrit-ui/app/services/checks/checks-service.ts index ca6c6e7..164074b 100644 --- a/polygerrit-ui/app/services/checks/checks-service.ts +++ b/polygerrit-ui/app/services/checks/checks-service.ts
@@ -171,7 +171,7 @@ patchsetNumber: patchNum, patchsetSha, repo: change.project, - commmitMessage: getCurrentRevision(change)?.commit?.message, + commitMessage: getCurrentRevision(change)?.commit?.message, changeInfo: change as ChangeInfo, }; return this.fetchResults(pluginName, data, patchset);
diff --git a/polygerrit-ui/app/services/comments/comments-model.ts b/polygerrit-ui/app/services/comments/comments-model.ts index b0279d4..b26ec9b 100644 --- a/polygerrit-ui/app/services/comments/comments-model.ts +++ b/polygerrit-ui/app/services/comments/comments-model.ts
@@ -23,7 +23,7 @@ PathToCommentsInfoMap, RobotCommentInfo, } from '../../types/common'; -import {DraftInfo} from '../../utils/comment-util'; +import {addPath, DraftInfo} from '../../utils/comment-util'; interface CommentState { comments: PathToCommentsInfoMap; @@ -71,7 +71,7 @@ [path: string]: CommentInfo[]; }) { const nextState = {...privateState$.getValue()}; - nextState.comments = comments || {}; + nextState.comments = addPath(comments) || {}; privateState$.next(nextState); } @@ -79,13 +79,13 @@ [path: string]: RobotCommentInfo[]; }) { const nextState = {...privateState$.getValue()}; - nextState.robotComments = robotComments || {}; + nextState.robotComments = addPath(robotComments) || {}; privateState$.next(nextState); } export function updateStateDrafts(drafts?: {[path: string]: DraftInfo[]}) { const nextState = {...privateState$.getValue()}; - nextState.drafts = drafts || {}; + nextState.drafts = addPath(drafts) || {}; privateState$.next(nextState); } @@ -126,10 +126,11 @@ if (!draft.path) throw new Error('draft path undefined'); nextState.drafts = {...nextState.drafts}; const drafts = nextState.drafts; - const index = drafts[draft.path].findIndex( + const index = (drafts[draft.path] || []).findIndex( d => d.__draftID === draft.__draftID || d.id === draft.id ); if (index === -1) return; - drafts[draft.path] = [...drafts[draft.path]].splice(index, 1); + drafts[draft.path] = [...drafts[draft.path]]; + drafts[draft.path].splice(index, 1); privateState$.next(nextState); }
diff --git a/polygerrit-ui/app/services/gr-reporting/gr-reporting_impl.ts b/polygerrit-ui/app/services/gr-reporting/gr-reporting_impl.ts index 2cc6f41..0df7d12 100644 --- a/polygerrit-ui/app/services/gr-reporting/gr-reporting_impl.ts +++ b/polygerrit-ui/app/services/gr-reporting/gr-reporting_impl.ts
@@ -419,7 +419,10 @@ eventInfo.inBackgroundTab = isInBackgroundTab; } - if (this._flagsService.enabledExperiments.length) { + if ( + name === Timing.APP_STARTED && + this._flagsService.enabledExperiments.length + ) { eventInfo.enabledExperiments = JSON.stringify( this._flagsService.enabledExperiments );
diff --git a/polygerrit-ui/app/styles/themes/app-theme.ts b/polygerrit-ui/app/styles/themes/app-theme.ts index d768c96..1996800 100644 --- a/polygerrit-ui/app/styles/themes/app-theme.ts +++ b/polygerrit-ui/app/styles/themes/app-theme.ts
@@ -377,8 +377,7 @@ /* misc */ --border-radius: 4px; --reply-overlay-z-index: 1000; - /* Base 64 encoded 1x1px of #681da8 */ - --line-length-indicator: url('data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVQYV2PIlF2xAgAD+AHXfBDdKAAAAABJRU5ErkJggg=='); + --line-length-indicator-color: #681da8; /* paper and iron component overrides */ --iron-overlay-backdrop-background-color: black;
diff --git a/polygerrit-ui/app/styles/themes/dark-theme.ts b/polygerrit-ui/app/styles/themes/dark-theme.ts index dec438d..926b02d 100644 --- a/polygerrit-ui/app/styles/themes/dark-theme.ts +++ b/polygerrit-ui/app/styles/themes/dark-theme.ts
@@ -230,8 +230,7 @@ --syntax-variable-color: #f77669; /* misc */ - /* Base 64 encoded 1x1px of #d7aefb; */ - --line-length-indicator: url('data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVQYV2O4vu73fwAIYAOAtqAXCQAAAABJRU5ErkJggg=='); + --line-length-indicator-color: #d7aefb; /* paper and iron component overrides */ --iron-overlay-backdrop-background-color: white;
diff --git a/polygerrit-ui/app/types/common.ts b/polygerrit-ui/app/types/common.ts index 54c9899..0f2608e 100644 --- a/polygerrit-ui/app/types/common.ts +++ b/polygerrit-ui/app/types/common.ts
@@ -287,7 +287,7 @@ */ export interface AccountExternalIdInfo { identity: string; - email?: string; + email_address?: string; trusted?: boolean; can_delete?: boolean; } @@ -984,6 +984,7 @@ notify_all_comments?: boolean; notify_submitted_changes?: boolean; notify_abandoned_changes?: boolean; + _is_local?: boolean; // Added manually } /** * The DeleteDraftCommentsInput entity contains information specifying a set of draft comments that should be deleted
diff --git a/polygerrit-ui/app/types/diff.ts b/polygerrit-ui/app/types/diff.ts index b29e312..16338335 100644 --- a/polygerrit-ui/app/types/diff.ts +++ b/polygerrit-ui/app/types/diff.ts
@@ -104,7 +104,6 @@ hide_line_numbers?: boolean; hide_empty_pane?: boolean; match_brackets?: boolean; - line_wrapping?: boolean; } export declare type DiffPreferencesInfoKey = keyof DiffPreferencesInfo;
diff --git a/polygerrit-ui/app/types/events.ts b/polygerrit-ui/app/types/events.ts index 6e37697..5145527 100644 --- a/polygerrit-ui/app/types/events.ts +++ b/polygerrit-ui/app/types/events.ts
@@ -26,6 +26,7 @@ CHANGE = 'change', CHANGED = 'changed', CHANGE_MESSAGE_DELETED = 'change-message-deleted', + COMMIT = 'commit', DIALOG_CHANGE = 'dialog-change', DROP = 'drop', EDITABLE_CONTENT_SAVE = 'editable-content-save', @@ -58,6 +59,8 @@ /* prettier-ignore */ 'changed': ChangedEvent; 'change-message-deleted': ChangeMessageDeletedEvent; + /* prettier-ignore */ + 'commit': CommitEvent; 'dialog-change': DialogChangeEvent; /* prettier-ignore */ 'drop': DropEvent; @@ -109,6 +112,8 @@ } export type ChangeMessageDeletedEvent = CustomEvent<ChangeMessageDeletedEventDetail>; +export type CommitEvent = CustomEvent; + // TODO(milutin) - remove once new gr-dialog will do it out of the box // This informs gr-app-element to remove footer, header from a11y tree export interface DialogChangeEventDetail {
diff --git a/polygerrit-ui/app/types/types.ts b/polygerrit-ui/app/types/types.ts index fd41cc9..2d4d412 100644 --- a/polygerrit-ui/app/types/types.ts +++ b/polygerrit-ui/app/types/types.ts
@@ -189,7 +189,6 @@ showDownloadDialog: boolean; diffMode: DiffViewMode | null; numFilesShown: number | null; - scrollTop?: number; diffViewMode?: boolean; }
diff --git a/polygerrit-ui/app/utils/comment-util.ts b/polygerrit-ui/app/utils/comment-util.ts index 83a9174..3e85b48 100644 --- a/polygerrit-ui/app/utils/comment-util.ts +++ b/polygerrit-ui/app/utils/comment-util.ts
@@ -361,3 +361,25 @@ if (isDraft(comment)) return comment.__draftID; throw new Error('Missing id in root comment.'); } + +/** + * Add path info to every comment as CommentInfo returned + * from server does not have that. + * + * TODO(taoalpha): should consider changing BE to send path + * back within CommentInfo + */ +export function addPath<T>( + comments: {[path: string]: T[]} = {} +): {[path: string]: Array<T & {path: string}>} { + const updatedComments: {[path: string]: Array<T & {path: string}>} = {}; + for (const filePath of Object.keys(comments)) { + const allCommentsForPath = comments[filePath] || []; + if (allCommentsForPath.length) { + updatedComments[filePath] = allCommentsForPath.map(comment => { + return {...comment, path: filePath}; + }); + } + } + return updatedComments; +}
diff --git a/polygerrit-ui/app/yarn.lock b/polygerrit-ui/app/yarn.lock index d8657f7..4fb98dd 100644 --- a/polygerrit-ui/app/yarn.lock +++ b/polygerrit-ui/app/yarn.lock
@@ -600,6 +600,11 @@ agent-base "6" debug "4" +immer@^9.0.5: + version "9.0.5" + resolved "https://registry.yarnpkg.com/immer/-/immer-9.0.5.tgz#a7154f34fe7064f15f00554cc94c66cc0bf453ec" + integrity sha512-2WuIehr2y4lmYz9gaQzetPR2ECniCifk4ORaQbU3g5EalLt+0IVTosEPJ5BoYl/75ky2mivzdRzV8wWgQGOSYQ== + inflight@^1.0.4: version "1.0.6" resolved "https://registry.yarnpkg.com/inflight/-/inflight-1.0.6.tgz#49bd6331d7d02d0c09bc910a1075ba8165b56df9"
diff --git a/polygerrit-ui/server.go b/polygerrit-ui/server.go index e030878..ddfaeb4 100644 --- a/polygerrit-ui/server.go +++ b/polygerrit-ui/server.go
@@ -202,6 +202,10 @@ moduleImportRegexp = regexp.MustCompile("(?m)^((import|export).*'/node_modules/)lit-(element|html).js';$") data = moduleImportRegexp.ReplaceAll(data, []byte("${1}lit-${3}/lit-${3}.js';")) + // 'immer' imports and exports have to be resolved to 'immer/dist/immer.esm.js'. + moduleImportRegexp = regexp.MustCompile("(?m)^((import|export).*'/node_modules/)immer.js';$") + data = moduleImportRegexp.ReplaceAll(data, []byte("${1}/immer/dist/immer.esm.js';")) + if strings.HasSuffix(normalizedContentPath, "/node_modules/page/page.js") { // Can't import page.js directly, because this is undefined. // Replace it with window
