Merge "CommentJson fix: URL decode comment id before requesting comment context"
diff --git a/Documentation/cmd-index.txt b/Documentation/cmd-index.txt
index 8a970c5..99ff0db 100644
--- a/Documentation/cmd-index.txt
+++ b/Documentation/cmd-index.txt
@@ -118,6 +118,9 @@
link:cmd-close-connection.html[gerrit close-connection]::
Close the specified SSH connection.
+link:cmd-convert-ref-storage.html[gerrit convert-ref-storage]::
+ Convert ref storage to reftable (experimental).
+
link:cmd-create-account.html[gerrit create-account]::
Create a new user account.
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index e750450..495a573 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -604,7 +604,14 @@
By default this is set to `LDAP` when link:#auth.type[`auth.type`] is `LDAP`
and `OAUTH` when link:#auth.type[`auth.type`] is `OAUTH`.
Otherwise, the default value is `HTTP`.
-
++
+When gitBasicAuthPolicy is set to `LDAP` or `HTTP_LDAP` and the user
+is authenticating with the LDAP username/password, the Git client config
+needs to have `http.cookieFile` set to a local file, otherwise every
+single call would trigger a full LDAP authentication and groups resolution
+which could introduce a noticeable latency on the overall execution
+and produce unwanted load to the LDAP server.
++
[[auth.gitOAuthProvider]]auth.gitOAuthProvider::
+
Selects the OAuth 2 provider to authenticate git over HTTP traffic with.
@@ -634,7 +641,9 @@
existing accounts this username is already in lower case. It is not
possible to convert the usernames of the existing accounts to lower
case because this would break the access to existing per-user
-branches and Gerrit provides no tool to do such a conversion.
+branches and Gerrit provides no tool to do such a conversion. Accounts
+created using the REST API or the `create-account` SSH command will
+be created with all lowercase characters, when this option is set.
+
Setting this parameter to `true` will prevent all users from login that
have a non-lower-case username.
diff --git a/Documentation/dev-bazel.txt b/Documentation/dev-bazel.txt
index 7db364c..61565f8 100644
--- a/Documentation/dev-bazel.txt
+++ b/Documentation/dev-bazel.txt
@@ -550,6 +550,10 @@
* ~/.gerritcodereview/bazel-cache/repository
* ~/.gerritcodereview/bazel-cache/cas
+The `downloaded-artifacts` cache can be relocated by setting the
+`GERRIT_CACHE_HOME` environment variable. The other two can be adjusted with
+`bazel build` options `--repository_cache` and `--disk_cache` respectively.
+
Currently none of these caches have a maximum size limit. See
link:https://github.com/bazelbuild/bazel/issues/5139[this bazel issue,role=external,window=_blank] for
details. Users should watch the cache sizes and clean them manually if
diff --git a/Documentation/images/user-review-ui-change-screen-change-info-actions.png b/Documentation/images/gwt-user-review-ui-change-screen-change-info-actions.png
similarity index 100%
rename from Documentation/images/user-review-ui-change-screen-change-info-actions.png
rename to Documentation/images/gwt-user-review-ui-change-screen-change-info-actions.png
Binary files differ
diff --git a/Documentation/images/user-review-ui-change-screen-change-info-cannot-merge.png b/Documentation/images/gwt-user-review-ui-change-screen-change-info-cannot-merge.png
similarity index 100%
rename from Documentation/images/user-review-ui-change-screen-change-info-cannot-merge.png
rename to Documentation/images/gwt-user-review-ui-change-screen-change-info-cannot-merge.png
Binary files differ
diff --git a/Documentation/images/user-review-ui-change-screen-change-info-labels.png b/Documentation/images/gwt-user-review-ui-change-screen-change-info-labels.png
similarity index 100%
rename from Documentation/images/user-review-ui-change-screen-change-info-labels.png
rename to Documentation/images/gwt-user-review-ui-change-screen-change-info-labels.png
Binary files differ
diff --git a/Documentation/images/user-review-ui-change-screen-change-info-last-update.png b/Documentation/images/gwt-user-review-ui-change-screen-change-info-last-update.png
similarity index 100%
rename from Documentation/images/user-review-ui-change-screen-change-info-last-update.png
rename to Documentation/images/gwt-user-review-ui-change-screen-change-info-last-update.png
Binary files differ
diff --git a/Documentation/images/user-review-ui-change-screen-change-info-owner.png b/Documentation/images/gwt-user-review-ui-change-screen-change-info-owner.png
similarity index 100%
rename from Documentation/images/user-review-ui-change-screen-change-info-owner.png
rename to Documentation/images/gwt-user-review-ui-change-screen-change-info-owner.png
Binary files differ
diff --git a/Documentation/images/user-review-ui-change-screen-change-info-project-branch-topic.png b/Documentation/images/gwt-user-review-ui-change-screen-change-info-project-branch-topic.png
similarity index 100%
rename from Documentation/images/user-review-ui-change-screen-change-info-project-branch-topic.png
rename to Documentation/images/gwt-user-review-ui-change-screen-change-info-project-branch-topic.png
Binary files differ
diff --git a/Documentation/images/user-review-ui-change-screen-change-info-reviewers.png b/Documentation/images/gwt-user-review-ui-change-screen-change-info-reviewers.png
similarity index 100%
rename from Documentation/images/user-review-ui-change-screen-change-info-reviewers.png
rename to Documentation/images/gwt-user-review-ui-change-screen-change-info-reviewers.png
Binary files differ
diff --git a/Documentation/images/user-review-ui-change-screen-change-info-submit-strategy.png b/Documentation/images/gwt-user-review-ui-change-screen-change-info-submit-strategy.png
similarity index 100%
rename from Documentation/images/user-review-ui-change-screen-change-info-submit-strategy.png
rename to Documentation/images/gwt-user-review-ui-change-screen-change-info-submit-strategy.png
Binary files differ
diff --git a/Documentation/images/user-review-ui-change-screen-change-info.png b/Documentation/images/gwt-user-review-ui-change-screen-change-info.png
similarity index 100%
rename from Documentation/images/user-review-ui-change-screen-change-info.png
rename to Documentation/images/gwt-user-review-ui-change-screen-change-info.png
Binary files differ
diff --git a/Documentation/images/user-review-ui-change-screen-change-update.png b/Documentation/images/gwt-user-review-ui-change-screen-change-update.png
similarity index 100%
rename from Documentation/images/user-review-ui-change-screen-change-update.png
rename to Documentation/images/gwt-user-review-ui-change-screen-change-update.png
Binary files differ
diff --git a/Documentation/images/user-review-ui-change-screen-cherry-picks.png b/Documentation/images/gwt-user-review-ui-change-screen-cherry-picks.png
similarity index 100%
rename from Documentation/images/user-review-ui-change-screen-cherry-picks.png
rename to Documentation/images/gwt-user-review-ui-change-screen-cherry-picks.png
Binary files differ
diff --git a/Documentation/images/user-review-ui-change-screen-commit-info-merge-commit.png b/Documentation/images/gwt-user-review-ui-change-screen-commit-info-merge-commit.png
similarity index 100%
rename from Documentation/images/user-review-ui-change-screen-commit-info-merge-commit.png
rename to Documentation/images/gwt-user-review-ui-change-screen-commit-info-merge-commit.png
Binary files differ
diff --git a/Documentation/images/user-review-ui-change-screen-commit-info.png b/Documentation/images/gwt-user-review-ui-change-screen-commit-info.png
similarity index 100%
rename from Documentation/images/user-review-ui-change-screen-commit-info.png
rename to Documentation/images/gwt-user-review-ui-change-screen-commit-info.png
Binary files differ
diff --git a/Documentation/images/user-review-ui-change-screen-commit-message.png b/Documentation/images/gwt-user-review-ui-change-screen-commit-message.png
similarity index 100%
rename from Documentation/images/user-review-ui-change-screen-commit-message.png
rename to Documentation/images/gwt-user-review-ui-change-screen-commit-message.png
Binary files differ
diff --git a/Documentation/images/user-review-ui-change-screen-conflicts-with.png b/Documentation/images/gwt-user-review-ui-change-screen-conflicts-with.png
similarity index 100%
rename from Documentation/images/user-review-ui-change-screen-conflicts-with.png
rename to Documentation/images/gwt-user-review-ui-change-screen-conflicts-with.png
Binary files differ
diff --git a/Documentation/images/user-review-ui-change-screen-download-commands-list.png b/Documentation/images/gwt-user-review-ui-change-screen-download-commands-list.png
similarity index 100%
rename from Documentation/images/user-review-ui-change-screen-download-commands-list.png
rename to Documentation/images/gwt-user-review-ui-change-screen-download-commands-list.png
Binary files differ
diff --git a/Documentation/images/user-review-ui-change-screen-download-commands.png b/Documentation/images/gwt-user-review-ui-change-screen-download-commands.png
similarity index 100%
rename from Documentation/images/user-review-ui-change-screen-download-commands.png
rename to Documentation/images/gwt-user-review-ui-change-screen-download-commands.png
Binary files differ
diff --git a/Documentation/images/user-review-ui-change-screen-file-list-comments.png b/Documentation/images/gwt-user-review-ui-change-screen-file-list-comments.png
similarity index 100%
rename from Documentation/images/user-review-ui-change-screen-file-list-comments.png
rename to Documentation/images/gwt-user-review-ui-change-screen-file-list-comments.png
Binary files differ
diff --git a/Documentation/images/user-review-ui-change-screen-file-list-header.png b/Documentation/images/gwt-user-review-ui-change-screen-file-list-header.png
similarity index 100%
rename from Documentation/images/user-review-ui-change-screen-file-list-header.png
rename to Documentation/images/gwt-user-review-ui-change-screen-file-list-header.png
Binary files differ
diff --git a/Documentation/images/user-review-ui-change-screen-file-list-mark-as-reviewed.png b/Documentation/images/gwt-user-review-ui-change-screen-file-list-mark-as-reviewed.png
similarity index 100%
rename from Documentation/images/user-review-ui-change-screen-file-list-mark-as-reviewed.png
rename to Documentation/images/gwt-user-review-ui-change-screen-file-list-mark-as-reviewed.png
Binary files differ
diff --git a/Documentation/images/user-review-ui-change-screen-file-list-modification-type.png b/Documentation/images/gwt-user-review-ui-change-screen-file-list-modification-type.png
similarity index 100%
rename from Documentation/images/user-review-ui-change-screen-file-list-modification-type.png
rename to Documentation/images/gwt-user-review-ui-change-screen-file-list-modification-type.png
Binary files differ
diff --git a/Documentation/images/user-review-ui-change-screen-file-list-rename.png b/Documentation/images/gwt-user-review-ui-change-screen-file-list-rename.png
similarity index 100%
rename from Documentation/images/user-review-ui-change-screen-file-list-rename.png
rename to Documentation/images/gwt-user-review-ui-change-screen-file-list-rename.png
Binary files differ
diff --git a/Documentation/images/user-review-ui-change-screen-file-list-repeating-paths.png b/Documentation/images/gwt-user-review-ui-change-screen-file-list-repeating-paths.png
similarity index 100%
rename from Documentation/images/user-review-ui-change-screen-file-list-repeating-paths.png
rename to Documentation/images/gwt-user-review-ui-change-screen-file-list-repeating-paths.png
Binary files differ
diff --git a/Documentation/images/user-review-ui-change-screen-file-list-size.png b/Documentation/images/gwt-user-review-ui-change-screen-file-list-size.png
similarity index 100%
rename from Documentation/images/user-review-ui-change-screen-file-list-size.png
rename to Documentation/images/gwt-user-review-ui-change-screen-file-list-size.png
Binary files differ
diff --git a/Documentation/images/user-review-ui-change-screen-file-list.png b/Documentation/images/gwt-user-review-ui-change-screen-file-list.png
similarity index 100%
rename from Documentation/images/user-review-ui-change-screen-file-list.png
rename to Documentation/images/gwt-user-review-ui-change-screen-file-list.png
Binary files differ
diff --git a/Documentation/images/user-review-ui-change-screen-history.png b/Documentation/images/gwt-user-review-ui-change-screen-history.png
similarity index 100%
rename from Documentation/images/user-review-ui-change-screen-history.png
rename to Documentation/images/gwt-user-review-ui-change-screen-history.png
Binary files differ
diff --git a/Documentation/images/user-review-ui-change-screen-included-in-list.png b/Documentation/images/gwt-user-review-ui-change-screen-included-in-list.png
similarity index 100%
rename from Documentation/images/user-review-ui-change-screen-included-in-list.png
rename to Documentation/images/gwt-user-review-ui-change-screen-included-in-list.png
Binary files differ
diff --git a/Documentation/images/user-review-ui-change-screen-included-in.png b/Documentation/images/gwt-user-review-ui-change-screen-included-in.png
similarity index 100%
rename from Documentation/images/user-review-ui-change-screen-included-in.png
rename to Documentation/images/gwt-user-review-ui-change-screen-included-in.png
Binary files differ
diff --git a/Documentation/images/user-review-ui-change-screen-inline-comments.png b/Documentation/images/gwt-user-review-ui-change-screen-inline-comments.png
similarity index 100%
rename from Documentation/images/user-review-ui-change-screen-inline-comments.png
rename to Documentation/images/gwt-user-review-ui-change-screen-inline-comments.png
Binary files differ
diff --git a/Documentation/images/user-review-ui-change-screen-not-current.png b/Documentation/images/gwt-user-review-ui-change-screen-not-current.png
similarity index 100%
rename from Documentation/images/user-review-ui-change-screen-not-current.png
rename to Documentation/images/gwt-user-review-ui-change-screen-not-current.png
Binary files differ
diff --git a/Documentation/images/user-review-ui-change-screen-patch-set-list.png b/Documentation/images/gwt-user-review-ui-change-screen-patch-set-list.png
similarity index 100%
rename from Documentation/images/user-review-ui-change-screen-patch-set-list.png
rename to Documentation/images/gwt-user-review-ui-change-screen-patch-set-list.png
Binary files differ
diff --git a/Documentation/images/user-review-ui-change-screen-patch-sets.png b/Documentation/images/gwt-user-review-ui-change-screen-patch-sets.png
similarity index 100%
rename from Documentation/images/user-review-ui-change-screen-patch-sets.png
rename to Documentation/images/gwt-user-review-ui-change-screen-patch-sets.png
Binary files differ
diff --git a/Documentation/images/user-review-ui-change-screen-permalink.png b/Documentation/images/gwt-user-review-ui-change-screen-permalink.png
similarity index 100%
rename from Documentation/images/user-review-ui-change-screen-permalink.png
rename to Documentation/images/gwt-user-review-ui-change-screen-permalink.png
Binary files differ
diff --git a/Documentation/images/user-review-ui-change-screen-plugin-extensions.png b/Documentation/images/gwt-user-review-ui-change-screen-plugin-extensions.png
similarity index 100%
rename from Documentation/images/user-review-ui-change-screen-plugin-extensions.png
rename to Documentation/images/gwt-user-review-ui-change-screen-plugin-extensions.png
Binary files differ
diff --git a/Documentation/images/user-review-ui-change-screen-quick-approve.png b/Documentation/images/gwt-user-review-ui-change-screen-quick-approve.png
similarity index 100%
rename from Documentation/images/user-review-ui-change-screen-quick-approve.png
rename to Documentation/images/gwt-user-review-ui-change-screen-quick-approve.png
Binary files differ
diff --git a/Documentation/images/user-review-ui-change-screen-related-changes-indicators.png b/Documentation/images/gwt-user-review-ui-change-screen-related-changes-indicators.png
similarity index 100%
rename from Documentation/images/user-review-ui-change-screen-related-changes-indicators.png
rename to Documentation/images/gwt-user-review-ui-change-screen-related-changes-indicators.png
Binary files differ
diff --git a/Documentation/images/user-review-ui-change-screen-related-changes.png b/Documentation/images/gwt-user-review-ui-change-screen-related-changes.png
similarity index 100%
rename from Documentation/images/user-review-ui-change-screen-related-changes.png
rename to Documentation/images/gwt-user-review-ui-change-screen-related-changes.png
Binary files differ
diff --git a/Documentation/images/user-review-ui-change-screen-reply-to-comment.png b/Documentation/images/gwt-user-review-ui-change-screen-reply-to-comment.png
similarity index 100%
rename from Documentation/images/user-review-ui-change-screen-reply-to-comment.png
rename to Documentation/images/gwt-user-review-ui-change-screen-reply-to-comment.png
Binary files differ
diff --git a/Documentation/images/user-review-ui-change-screen-reply.png b/Documentation/images/gwt-user-review-ui-change-screen-reply.png
similarity index 100%
rename from Documentation/images/user-review-ui-change-screen-reply.png
rename to Documentation/images/gwt-user-review-ui-change-screen-reply.png
Binary files differ
diff --git a/Documentation/images/user-review-ui-change-screen-replying.png b/Documentation/images/gwt-user-review-ui-change-screen-replying.png
similarity index 100%
rename from Documentation/images/user-review-ui-change-screen-replying.png
rename to Documentation/images/gwt-user-review-ui-change-screen-replying.png
Binary files differ
diff --git a/Documentation/images/user-review-ui-change-screen-same-topic.png b/Documentation/images/gwt-user-review-ui-change-screen-same-topic.png
similarity index 100%
rename from Documentation/images/user-review-ui-change-screen-same-topic.png
rename to Documentation/images/gwt-user-review-ui-change-screen-same-topic.png
Binary files differ
diff --git a/Documentation/images/user-review-ui-change-screen-star.png b/Documentation/images/gwt-user-review-ui-change-screen-star.png
similarity index 100%
rename from Documentation/images/user-review-ui-change-screen-star.png
rename to Documentation/images/gwt-user-review-ui-change-screen-star.png
Binary files differ
diff --git a/Documentation/images/user-review-ui-change-screen.png b/Documentation/images/gwt-user-review-ui-change-screen.png
similarity index 100%
rename from Documentation/images/user-review-ui-change-screen.png
rename to Documentation/images/gwt-user-review-ui-change-screen.png
Binary files differ
diff --git a/Documentation/images/user-review-ui-side-by-side-diff-screen-column.png b/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-column.png
similarity index 100%
rename from Documentation/images/user-review-ui-side-by-side-diff-screen-column.png
rename to Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-column.png
Binary files differ
diff --git a/Documentation/images/user-review-ui-side-by-side-diff-screen-comment-box.png b/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-comment-box.png
similarity index 100%
rename from Documentation/images/user-review-ui-side-by-side-diff-screen-comment-box.png
rename to Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-comment-box.png
Binary files differ
diff --git a/Documentation/images/user-review-ui-side-by-side-diff-screen-comment-edit.png b/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-comment-edit.png
similarity index 100%
rename from Documentation/images/user-review-ui-side-by-side-diff-screen-comment-edit.png
rename to Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-comment-edit.png
Binary files differ
diff --git a/Documentation/images/user-review-ui-side-by-side-diff-screen-comment-reply.png b/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-comment-reply.png
similarity index 100%
rename from Documentation/images/user-review-ui-side-by-side-diff-screen-comment-reply.png
rename to Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-comment-reply.png
Binary files differ
diff --git a/Documentation/images/user-review-ui-side-by-side-diff-screen-comment.png b/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-comment.png
similarity index 100%
rename from Documentation/images/user-review-ui-side-by-side-diff-screen-comment.png
rename to Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-comment.png
Binary files differ
diff --git a/Documentation/images/user-review-ui-side-by-side-diff-screen-commented.png b/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-commented.png
similarity index 100%
rename from Documentation/images/user-review-ui-side-by-side-diff-screen-commented.png
rename to Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-commented.png
Binary files differ
diff --git a/Documentation/images/user-review-ui-side-by-side-diff-screen-dark-theme.png b/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-dark-theme.png
similarity index 100%
rename from Documentation/images/user-review-ui-side-by-side-diff-screen-dark-theme.png
rename to Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-dark-theme.png
Binary files differ
diff --git a/Documentation/images/user-review-ui-side-by-side-diff-screen-expand-skipped-lines.png b/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-expand-skipped-lines.png
similarity index 100%
rename from Documentation/images/user-review-ui-side-by-side-diff-screen-expand-skipped-lines.png
rename to Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-expand-skipped-lines.png
Binary files differ
diff --git a/Documentation/images/user-review-ui-side-by-side-diff-screen-file-level-comment.png b/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-file-level-comment.png
similarity index 100%
rename from Documentation/images/user-review-ui-side-by-side-diff-screen-file-level-comment.png
rename to Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-file-level-comment.png
Binary files differ
diff --git a/Documentation/images/user-review-ui-side-by-side-diff-screen-file-level-commented.png b/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-file-level-commented.png
similarity index 100%
rename from Documentation/images/user-review-ui-side-by-side-diff-screen-file-level-commented.png
rename to Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-file-level-commented.png
Binary files differ
diff --git a/Documentation/images/user-review-ui-side-by-side-diff-screen-inline-comments.png b/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-inline-comments.png
similarity index 100%
rename from Documentation/images/user-review-ui-side-by-side-diff-screen-inline-comments.png
rename to Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-inline-comments.png
Binary files differ
diff --git a/Documentation/images/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
similarity index 100%
rename from Documentation/images/user-review-ui-side-by-side-diff-screen-keyboard-shortcuts.png
rename to Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-keyboard-shortcuts.png
Binary files differ
diff --git a/Documentation/images/user-review-ui-side-by-side-diff-screen-navigation.png b/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-navigation.png
similarity index 100%
rename from Documentation/images/user-review-ui-side-by-side-diff-screen-navigation.png
rename to Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-navigation.png
Binary files differ
diff --git a/Documentation/images/user-review-ui-side-by-side-diff-screen-no-differences.png b/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-no-differences.png
similarity index 100%
rename from Documentation/images/user-review-ui-side-by-side-diff-screen-no-differences.png
rename to Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-no-differences.png
Binary files differ
diff --git a/Documentation/images/user-review-ui-side-by-side-diff-screen-patch-sets.png b/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-patch-sets.png
similarity index 100%
rename from Documentation/images/user-review-ui-side-by-side-diff-screen-patch-sets.png
rename to Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-patch-sets.png
Binary files differ
diff --git a/Documentation/images/user-review-ui-side-by-side-diff-screen-preferences-popup.png b/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-preferences-popup.png
similarity index 100%
rename from Documentation/images/user-review-ui-side-by-side-diff-screen-preferences-popup.png
rename to Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-preferences-popup.png
Binary files differ
diff --git a/Documentation/images/user-review-ui-side-by-side-diff-screen-preferences.png b/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-preferences.png
similarity index 100%
rename from Documentation/images/user-review-ui-side-by-side-diff-screen-preferences.png
rename to Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-preferences.png
Binary files differ
diff --git a/Documentation/images/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
similarity index 100%
rename from Documentation/images/user-review-ui-side-by-side-diff-screen-project-and-file.png
rename to Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-project-and-file.png
Binary files differ
diff --git a/Documentation/images/user-review-ui-side-by-side-diff-screen-red-bar.png b/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-red-bar.png
similarity index 100%
rename from Documentation/images/user-review-ui-side-by-side-diff-screen-red-bar.png
rename to Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-red-bar.png
Binary files differ
diff --git a/Documentation/images/user-review-ui-side-by-side-diff-screen-rename.png b/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-rename.png
similarity index 100%
rename from Documentation/images/user-review-ui-side-by-side-diff-screen-rename.png
rename to Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-rename.png
Binary files differ
diff --git a/Documentation/images/user-review-ui-side-by-side-diff-screen-replied-done.png b/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-replied-done.png
similarity index 100%
rename from Documentation/images/user-review-ui-side-by-side-diff-screen-replied-done.png
rename to Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-replied-done.png
Binary files differ
diff --git a/Documentation/images/user-review-ui-side-by-side-diff-screen-reviewed.png b/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-reviewed.png
similarity index 100%
rename from Documentation/images/user-review-ui-side-by-side-diff-screen-reviewed.png
rename to Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-reviewed.png
Binary files differ
diff --git a/Documentation/images/user-review-ui-side-by-side-diff-screen-scrollbar.png b/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-scrollbar.png
similarity index 100%
rename from Documentation/images/user-review-ui-side-by-side-diff-screen-scrollbar.png
rename to Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-scrollbar.png
Binary files differ
diff --git a/Documentation/images/user-review-ui-side-by-side-diff-screen-search.png b/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-search.png
similarity index 100%
rename from Documentation/images/user-review-ui-side-by-side-diff-screen-search.png
rename to Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-search.png
Binary files differ
diff --git a/Documentation/images/user-review-ui-side-by-side-diff-screen-syntax-coloring.png b/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-syntax-coloring.png
similarity index 100%
rename from Documentation/images/user-review-ui-side-by-side-diff-screen-syntax-coloring.png
rename to Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-syntax-coloring.png
Binary files differ
diff --git a/Documentation/images/user-review-ui-side-by-side-diff-screen.png b/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen.png
similarity index 100%
rename from Documentation/images/user-review-ui-side-by-side-diff-screen.png
rename to Documentation/images/gwt-user-review-ui-side-by-side-diff-screen.png
Binary files differ
diff --git a/Documentation/images/inline-edit-enter-edit-mode-from-diff.png b/Documentation/images/inline-edit-enter-edit-mode-from-diff.png
deleted file mode 100644
index 46dd0ff..0000000
--- a/Documentation/images/inline-edit-enter-edit-mode-from-diff.png
+++ /dev/null
Binary files differ
diff --git a/Documentation/images/inline-edit-enter-edit-mode-from-file-list.png b/Documentation/images/inline-edit-enter-edit-mode-from-file-list.png
deleted file mode 100644
index b8c52c9..0000000
--- a/Documentation/images/inline-edit-enter-edit-mode-from-file-list.png
+++ /dev/null
Binary files differ
diff --git a/Documentation/images/inline-edit-file-list-in-edit-mode.png b/Documentation/images/inline-edit-file-list-in-edit-mode.png
deleted file mode 100644
index 8f355335..0000000
--- a/Documentation/images/inline-edit-file-list-in-edit-mode.png
+++ /dev/null
Binary files differ
diff --git a/Documentation/images/inline-edit-full-screen-editor.png b/Documentation/images/inline-edit-full-screen-editor.png
deleted file mode 100644
index 474fae5..0000000
--- a/Documentation/images/inline-edit-full-screen-editor.png
+++ /dev/null
Binary files differ
diff --git a/Documentation/images/intro-quick-hot-key-help.jpg b/Documentation/images/intro-quick-hot-key-help.jpg
deleted file mode 100644
index 41bcbe4..0000000
--- a/Documentation/images/intro-quick-hot-key-help.jpg
+++ /dev/null
Binary files differ
diff --git a/Documentation/images/user-review-ui-change-screen-keyboard-shortcuts.png b/Documentation/images/user-review-ui-change-screen-keyboard-shortcuts.png
index 734ab29..9ef8f27 100644
--- a/Documentation/images/user-review-ui-change-screen-keyboard-shortcuts.png
+++ b/Documentation/images/user-review-ui-change-screen-keyboard-shortcuts.png
Binary files differ
diff --git a/Documentation/images/user-review-ui-side-by-side-diff-screen-intraline-difference.png b/Documentation/images/user-review-ui-side-by-side-diff-screen-intraline-difference.png
deleted file mode 100644
index 044f96f..0000000
--- a/Documentation/images/user-review-ui-side-by-side-diff-screen-intraline-difference.png
+++ /dev/null
Binary files differ
diff --git a/Documentation/metrics.txt b/Documentation/metrics.txt
index 7a35d4d..7ac804c 100644
--- a/Documentation/metrics.txt
+++ b/Documentation/metrics.txt
@@ -106,6 +106,13 @@
* `http/server/jetty/threadpool/pool_size`: Current thread pool size
* `http/server/jetty/threadpool/queue_size`: Queued requests waiting for a thread
+==== LDAP
+
+* `ldap/login_latency`: Latency of logins.
+* `ldap/user_search_latency`: Latency for searching the user account.
+* `ldap/group_search_latency`: Latency for querying the group memberships of an account.
+* `ldap/group_expansion_latency`: Latency for expanding nested groups.
+
==== REST API
* `http/server/error_count`: Rate of REST API error responses.
diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt
index e0bed37..ff12af2 100644
--- a/Documentation/rest-api-changes.txt
+++ b/Documentation/rest-api-changes.txt
@@ -6921,7 +6921,10 @@
The subject of the commit (header line of the commit message).
|`message` ||The commit message.
|`web_links` |optional|
-Links to the commit in external sites as a list of
+Links to the patch set in external sites as a list of
+link:#web-link-info[WebLinkInfo] entities.
+|`resolve_conflicts_web_links` |optional|
+Links to the commit in external sites for resolving conflicts as a list of
link:#web-link-info[WebLinkInfo] entities.
|===========================
diff --git a/Documentation/user-review-ui.txt b/Documentation/user-review-ui.txt
index 952a1bf..98ec22d 100644
--- a/Documentation/user-review-ui.txt
+++ b/Documentation/user-review-ui.txt
@@ -12,7 +12,7 @@
The change screen shows the details of a single change and provides
various actions on it.
-image::images/user-review-ui-change-screen.png[width=800, link="images/user-review-ui-change-screen.png"]
+image::images/gwt-user-review-ui-change-screen.png[width=800, link="images/gwt-user-review-ui-change-screen.png"]
[[commit-message]]
=== Commit Message Block
@@ -21,14 +21,14 @@
the most important information about a change. The numeric change ID
and the change status are displayed right above the commit message.
-image::images/user-review-ui-change-screen-commit-message.png[width=800, link="images/user-review-ui-change-screen-commit-message.png"]
+image::images/gwt-user-review-ui-change-screen-commit-message.png[width=800, link="images/gwt-user-review-ui-change-screen-commit-message.png"]
[[permalink]]
The numeric change ID is a link to the change and clicking on it
refreshes the change screen. By copying the link location you can get
the permalink of the change.
-image::images/user-review-ui-change-screen-permalink.png[width=800, link="images/user-review-ui-change-screen-permalink.png"]
+image::images/gwt-user-review-ui-change-screen-permalink.png[width=800, link="images/gwt-user-review-ui-change-screen-permalink.png"]
[[change-status]]
The change status shows the state of the change:
@@ -79,11 +79,11 @@
If a Git web browser, such as gitweb or Gitiles, is configured, there
is also a link to the commit in the Git web browser.
-image::images/user-review-ui-change-screen-commit-info.png[width=800, link="images/user-review-ui-change-screen-commit-info.png"]
+image::images/gwt-user-review-ui-change-screen-commit-info.png[width=800, link="images/gwt-user-review-ui-change-screen-commit-info.png"]
If a merge commit is viewed this is highlighted by an icon.
-image::images/user-review-ui-change-screen-commit-info-merge-commit.png[width=800, link="images/user-review-ui-change-screen-commit-info-merge-commit.png"]
+image::images/gwt-user-review-ui-change-screen-commit-info-merge-commit.png[width=800, link="images/gwt-user-review-ui-change-screen-commit-info-merge-commit.png"]
[[change-info]]
=== Change Info Block
@@ -91,14 +91,14 @@
The change info block contains detailed information about the change
and offers actions on the change.
-image::images/user-review-ui-change-screen-change-info.png[width=800, link="images/user-review-ui-change-screen-change-info.png"]
+image::images/gwt-user-review-ui-change-screen-change-info.png[width=800, link="images/gwt-user-review-ui-change-screen-change-info.png"]
- [[change-owner]]Change Owner:
+
The owner of the change is displayed as a link to a list of the owner's
changes that have the same status as the currently viewed change.
+
-image::images/user-review-ui-change-screen-change-info-owner.png[width=800, link="images/user-review-ui-change-screen-change-info-owner.png"]
+image::images/gwt-user-review-ui-change-screen-change-info-owner.png[width=800, link="images/gwt-user-review-ui-change-screen-change-info-owner.png"]
- [[reviewers]]Reviewers:
+
@@ -126,7 +126,7 @@
and Gerrit administrators may remove anyone.
+
-image::images/user-review-ui-change-screen-change-info-reviewers.png[width=800, link="images/user-review-ui-change-screen-change-info-reviewers.png"]
+image::images/gwt-user-review-ui-change-screen-change-info-reviewers.png[width=800, link="images/gwt-user-review-ui-change-screen-change-info-reviewers.png"]
- [[project-branch-topic]]Project / Branch / Topic:
+
@@ -148,7 +148,7 @@
access right. To be able to set a topic on a closed change, the
`Edit Topic Name` must be assigned with the `force` flag.
+
-image::images/user-review-ui-change-screen-change-info-project-branch-topic.png[width=800, link="images/user-review-ui-change-screen-change-info-project-branch-topic.png"]
+image::images/gwt-user-review-ui-change-screen-change-info-project-branch-topic.png[width=800, link="images/gwt-user-review-ui-change-screen-change-info-project-branch-topic.png"]
- [[submit-strategy]]Submit Strategy:
+
@@ -156,16 +156,16 @@
used to submit the change. The submit strategy is only displayed for
open changes.
+
-image::images/user-review-ui-change-screen-change-info-submit-strategy.png[width=800, link="images/user-review-ui-change-screen-change-info-submit-strategy.png"]
+image::images/gwt-user-review-ui-change-screen-change-info-submit-strategy.png[width=800, link="images/gwt-user-review-ui-change-screen-change-info-submit-strategy.png"]
+
If a change cannot be merged due to path conflicts this is highlighted
by a bold red `Cannot Merge` label.
+
-image::images/user-review-ui-change-screen-change-info-cannot-merge.png[width=800, link="images/user-review-ui-change-screen-change-info-cannot-merge.png"]
+image::images/gwt-user-review-ui-change-screen-change-info-cannot-merge.png[width=800, link="images/gwt-user-review-ui-change-screen-change-info-cannot-merge.png"]
- [[update-time]]Time of Last Update:
+
-image::images/user-review-ui-change-screen-change-info-last-update.png[width=800, link="images/user-review-ui-change-screen-change-info-last-update.png"]
+image::images/gwt-user-review-ui-change-screen-change-info-last-update.png[width=800, link="images/gwt-user-review-ui-change-screen-change-info-last-update.png"]
- [[actions]]Actions:
+
@@ -266,13 +266,13 @@
** [[plugin-actions]]Further actions may be available if plugins are installed.
+
-image::images/user-review-ui-change-screen-change-info-actions.png[width=800, link="images/user-review-ui-change-screen-change-info-actions.png"]
+image::images/gwt-user-review-ui-change-screen-change-info-actions.png[width=800, link="images/gwt-user-review-ui-change-screen-change-info-actions.png"]
- [[labels]]Labels & Votes:
+
Approving votes are colored green; veto votes are colored red.
+
-image::images/user-review-ui-change-screen-change-info-labels.png[width=800, link="images/user-review-ui-change-screen-change-info-labels.png"]
+image::images/gwt-user-review-ui-change-screen-change-info-labels.png[width=800, link="images/gwt-user-review-ui-change-screen-change-info-labels.png"]
[[files]]
=== File List
@@ -280,7 +280,7 @@
The file list shows the files that are modified in the currently viewed
patch set.
-image::images/user-review-ui-change-screen-file-list.png[width=800, link="images/user-review-ui-change-screen-file-list.png"]
+image::images/gwt-user-review-ui-change-screen-file-list.png[width=800, link="images/gwt-user-review-ui-change-screen-file-list.png"]
[[magic-files]]
In addition to the modified files the file list contains magic files
@@ -302,7 +302,7 @@
[[change-screen-mark-reviewed]]
The checkboxes in front of the file names allow files to be marked as reviewed.
-image::images/user-review-ui-change-screen-file-list-mark-as-reviewed.png[width=800, link="images/user-review-ui-change-screen-file-list-mark-as-reviewed.png"]
+image::images/gwt-user-review-ui-change-screen-file-list-mark-as-reviewed.png[width=800, link="images/gwt-user-review-ui-change-screen-file-list-mark-as-reviewed.png"]
[[modification-type]]
The type of a file modification is indicated by the character in front
@@ -342,18 +342,18 @@
(Modified) if the majority of the lines have been changed so that the new file
content has a very low similarity with the old file content.
-image::images/user-review-ui-change-screen-file-list-modification-type.png[width=800, link="images/user-review-ui-change-screen-file-list-modification-type.png"]
+image::images/gwt-user-review-ui-change-screen-file-list-modification-type.png[width=800, link="images/gwt-user-review-ui-change-screen-file-list-modification-type.png"]
[[rename-or-copy]]
If a file is renamed or copied, the name of the original file is
displayed in gray below the file name.
-image::images/user-review-ui-change-screen-file-list-rename.png[width=800, link="images/user-review-ui-change-screen-file-list-rename.png"]
+image::images/gwt-user-review-ui-change-screen-file-list-rename.png[width=800, link="images/gwt-user-review-ui-change-screen-file-list-rename.png"]
[[repeating-path-segments]]
Repeating path segments are grayed out.
-image::images/user-review-ui-change-screen-file-list-repeating-paths.png[width=800, link="images/user-review-ui-change-screen-file-list-repeating-paths.png"]
+image::images/gwt-user-review-ui-change-screen-file-list-repeating-paths.png[width=800, link="images/gwt-user-review-ui-change-screen-file-list-repeating-paths.png"]
[[inline-comments-column]]
Inline comments on a file are shown in the `Comments` column.
@@ -364,7 +364,7 @@
New comments from other users, that were published after the current
user last reviewed this change, are highlighted in bold.
-image::images/user-review-ui-change-screen-file-list-comments.png[width=800, link="images/user-review-ui-change-screen-file-list-comments.png"]
+image::images/gwt-user-review-ui-change-screen-file-list-comments.png[width=800, link="images/gwt-user-review-ui-change-screen-file-list-comments.png"]
[[size]]
The size of the modifications in the files can be seen in the `Size` column. The
@@ -387,7 +387,7 @@
shows the total number of lines in the new file. No size is shown for binary
files and deleted files.
-image::images/user-review-ui-change-screen-file-list-size.png[width=800, link="images/user-review-ui-change-screen-file-list-size.png"]
+image::images/gwt-user-review-ui-change-screen-file-list-size.png[width=800, link="images/gwt-user-review-ui-change-screen-file-list-size.png"]
[[diff-against]]
In the header of the file list, the `Diff Against` selection can be
@@ -399,7 +399,7 @@
The file list header also provides an `Open All` button that opens the
diff views for all files in the file list.
-image::images/user-review-ui-change-screen-file-list-header.png[width=800, link="images/user-review-ui-change-screen-file-list-header.png"]
+image::images/gwt-user-review-ui-change-screen-file-list-header.png[width=800, link="images/gwt-user-review-ui-change-screen-file-list-header.png"]
[[patch-sets]]
=== Patch Sets
@@ -414,11 +414,11 @@
link:#not-current[Not Current] change state. Please note that some
operations are only available on the current patch set.
-image::images/user-review-ui-change-screen-patch-sets.png[width=800, link="images/user-review-ui-change-screen-patch-sets.png"]
+image::images/gwt-user-review-ui-change-screen-patch-sets.png[width=800, link="images/gwt-user-review-ui-change-screen-patch-sets.png"]
Another indication is a highlighted drop-down label.
-image::images/user-review-ui-change-screen-not-current.png[width=800, link="images/user-review-ui-change-screen-not-current.png"]
+image::images/gwt-user-review-ui-change-screen-not-current.png[width=800, link="images/gwt-user-review-ui-change-screen-not-current.png"]
[[patch-set-drop-down]]
The patch set drop-down list shows the list of patch sets and allows to
@@ -427,7 +427,7 @@
Draft patch sets are marked with `DRAFT`.
-image::images/user-review-ui-change-screen-patch-set-list.png[width=800, link="images/user-review-ui-change-screen-patch-set-list.png"]
+image::images/gwt-user-review-ui-change-screen-patch-set-list.png[width=800, link="images/gwt-user-review-ui-change-screen-patch-set-list.png"]
[[download]]
=== Download
@@ -435,7 +435,7 @@
The `Download` drop-down panel in the change header offers commands and
links for downloading the currently viewed patch set.
-image::images/user-review-ui-change-screen-download-commands.png[width=800, link="images/user-review-ui-change-screen-download-commands.png"]
+image::images/gwt-user-review-ui-change-screen-download-commands.png[width=800, link="images/gwt-user-review-ui-change-screen-download-commands.png"]
The available download commands depend on the installed Gerrit plugins.
The most popular plugin for download commands, the
@@ -462,7 +462,7 @@
formats (e.g. tar and tbz2); which formats are available depends on the
configuration of the server.
-image::images/user-review-ui-change-screen-download-commands-list.png[width=800, link="images/user-review-ui-change-screen-download-commands-list.png"]
+image::images/gwt-user-review-ui-change-screen-download-commands-list.png[width=800, link="images/gwt-user-review-ui-change-screen-download-commands-list.png"]
[[included-in]]
=== Included In
@@ -470,14 +470,14 @@
For merged changes the `Included In` drop-down panel is available in
the change header.
-image::images/user-review-ui-change-screen-included-in.png[width=800, link="images/user-review-ui-change-screen-included-in.png"]
+image::images/gwt-user-review-ui-change-screen-included-in.png[width=800, link="images/gwt-user-review-ui-change-screen-included-in.png"]
The `Included In` drop-down panel shows the branches and tags in which
the change is included. E.g. if a change fixes a bug, this allows to
quickly see in which released versions the bug-fix is contained
(assuming that every release is tagged).
-image::images/user-review-ui-change-screen-included-in-list.png[width=800, link="images/user-review-ui-change-screen-included-in-list.png"]
+image::images/gwt-user-review-ui-change-screen-included-in-list.png[width=800, link="images/gwt-user-review-ui-change-screen-included-in-list.png"]
[[star]]
=== Star Change
@@ -485,7 +485,7 @@
The star icon in the change header allows to mark the change as a
favorite. Clicking on the star icon again, unstars the change.
-image::images/user-review-ui-change-screen-star.png[width=800, link="images/user-review-ui-change-screen-star.png"]
+image::images/gwt-user-review-ui-change-screen-star.png[width=800, link="images/gwt-user-review-ui-change-screen-star.png"]
Starring a change turns on email notifications for this change.
@@ -520,7 +520,7 @@
For merged changes this tab is only shown if there are open
descendants.
+
-image::images/user-review-ui-change-screen-related-changes.png[width=800, link="images/user-review-ui-change-screen-related-changes.png"]
+image::images/gwt-user-review-ui-change-screen-related-changes.png[width=800, link="images/gwt-user-review-ui-change-screen-related-changes.png"]
+
Related changes may be decorated with an icon to signify dependencies
on outdated patch sets, or commits that are not associated to changes
@@ -567,7 +567,7 @@
through.
+
-image::images/user-review-ui-change-screen-related-changes-indicators.png[width=800, link="images/user-review-ui-change-screen-related-changes-indicators.png"]
+image::images/gwt-user-review-ui-change-screen-related-changes-indicators.png[width=800, link="images/gwt-user-review-ui-change-screen-related-changes-indicators.png"]
- [[conflicts-with]]`Conflicts With`:
+
@@ -579,14 +579,14 @@
conflicts and must be rebased. The rebase of the other changes with the
conflict resolution must then be done manually.
+
-image::images/user-review-ui-change-screen-conflicts-with.png[width=800, link="images/user-review-ui-change-screen-conflicts-with.png"]
+image::images/gwt-user-review-ui-change-screen-conflicts-with.png[width=800, link="images/gwt-user-review-ui-change-screen-conflicts-with.png"]
- [[same-topic]]`Same Topic`:
+
This tab page shows changes that have the same topic as the current
change. Only open changes are included in the list.
+
-image::images/user-review-ui-change-screen-same-topic.png[width=800, link="images/user-review-ui-change-screen-same-topic.png"]
+image::images/gwt-user-review-ui-change-screen-same-topic.png[width=800, link="images/gwt-user-review-ui-change-screen-same-topic.png"]
- [[submitted-together]]`Submitted Together`:
+
@@ -609,7 +609,7 @@
For each change in this list the destination branch is shown as a
prefix in front of the change subject.
+
-image::images/user-review-ui-change-screen-cherry-picks.png[width=800, link="images/user-review-ui-change-screen-cherry-picks.png"]
+image::images/gwt-user-review-ui-change-screen-cherry-picks.png[width=800, link="images/gwt-user-review-ui-change-screen-cherry-picks.png"]
If there are no related changes for a tab, the tab is not displayed.
@@ -620,7 +620,7 @@
currently viewed patch set; one can add a summary comment, publish
inline draft comments, and vote on the labels.
-image::images/user-review-ui-change-screen-reply.png[width=800, link="images/user-review-ui-change-screen-reply.png"]
+image::images/gwt-user-review-ui-change-screen-reply.png[width=800, link="images/gwt-user-review-ui-change-screen-reply.png"]
Clicking on the `Reply...` button opens a popup panel.
@@ -646,7 +646,7 @@
The `Post` button publishes the comments and the votes.
-image::images/user-review-ui-change-screen-replying.png[width=800, link="images/user-review-ui-change-screen-replying.png"]
+image::images/gwt-user-review-ui-change-screen-replying.png[width=800, link="images/gwt-user-review-ui-change-screen-replying.png"]
[[quick-approve]]
If a user can approve a label that is still required, a quick approve
@@ -665,7 +665,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/user-review-ui-change-screen-quick-approve.png"]
+image::images/gwt-user-review-ui-change-screen-quick-approve.png[width=800, link="images/gwt-user-review-ui-change-screen-quick-approve.png"]
[[history]]
=== History
@@ -679,7 +679,7 @@
Messages with new comments from other users, that were published after
the current user last reviewed this change, are automatically expanded.
-image::images/user-review-ui-change-screen-history.png[width=800, link="images/user-review-ui-change-screen-history.png"]
+image::images/gwt-user-review-ui-change-screen-history.png[width=800, link="images/gwt-user-review-ui-change-screen-history.png"]
[[reply-to-message]]
It is possible to directly reply to a change message by clicking on the
@@ -690,13 +690,13 @@
Please note that for a correct rendering it is important to leave a blank
line between a quoted block and the reply to it.
-image::images/user-review-ui-change-screen-reply-to-comment.png[width=800, link="images/user-review-ui-change-screen-reply-to-comment.png"]
+image::images/gwt-user-review-ui-change-screen-reply-to-comment.png[width=800, link="images/gwt-user-review-ui-change-screen-reply-to-comment.png"]
[[inline-comments-in-history]]
Inline comments are directly displayed in the change history and there
are links to navigate to the inline comments.
-image::images/user-review-ui-change-screen-inline-comments.png[width=800, link="images/user-review-ui-change-screen-inline-comments.png"]
+image::images/gwt-user-review-ui-change-screen-inline-comments.png[width=800, link="images/gwt-user-review-ui-change-screen-inline-comments.png"]
[[expand-all]]
The `Expand All` button expands all messages; the `Collapse All` button
@@ -713,7 +713,7 @@
it is 30 seconds. Polling may also be completely disabled by the
administrator.
-image::images/user-review-ui-change-screen-change-update.png[width=800, link="images/user-review-ui-change-screen-change-update.png"]
+image::images/gwt-user-review-ui-change-screen-change-update.png[width=800, link="images/gwt-user-review-ui-change-screen-change-update.png"]
[[plugin-extensions]]
=== Plugin Extensions
@@ -722,7 +722,7 @@
additional actions to the change info block and display arbitrary UI
controls below the change info block.
-image::images/user-review-ui-change-screen-plugin-extensions.png[width=800, link="images/user-review-ui-change-screen-plugin-extensions.png"]
+image::images/gwt-user-review-ui-change-screen-plugin-extensions.png[width=800, link="images/gwt-user-review-ui-change-screen-plugin-extensions.png"]
[[side-by-side]]
== Side-by-Side Diff Screen
@@ -733,7 +733,7 @@
This screen allows to review a patch and to comment on it.
-image::images/user-review-ui-side-by-side-diff-screen.png[width=800, link="images/user-review-ui-side-by-side-diff-screen.png"]
+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"]
[[side-by-side-header]]
In the screen header the project name and the name of the viewed patch
@@ -743,7 +743,7 @@
the file path are displayed as links to the project and the folder in
the Git web browser.
-image::images/user-review-ui-side-by-side-diff-screen-project-and-file.png[width=800, link="images/user-review-ui-side-by-side-diff-screen-project-and-file.png"]
+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 project name and the file name allows the
@@ -751,7 +751,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/user-review-ui-side-by-side-diff-screen-reviewed.png"]
+image::images/gwt-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"]
[[scrollbar]]
The scrollbar shows patch diffs and inline comments as annotations.
@@ -759,7 +759,7 @@
relevant for reviewing. By clicking on an annotation one can quickly
navigate to the corresponding line in the patch.
-image::images/user-review-ui-side-by-side-diff-screen-scrollbar.png[width=800, link="images/user-review-ui-side-by-side-diff-screen-scrollbar.png"]
+image::images/gwt-user-review-ui-side-by-side-diff-screen-scrollbar.png[width=800, link="images/gwt-user-review-ui-side-by-side-diff-screen-scrollbar.png"]
[[gaps]]
A gap between lines in the file content that is caused by aligning the
@@ -767,7 +767,7 @@
vertical red bar in the line number column. This prevents a gap from
being mistaken for blank lines in the file
-image::images/user-review-ui-side-by-side-diff-screen-red-bar.png[width=800, link="images/user-review-ui-side-by-side-diff-screen-red-bar.png"]
+image::images/gwt-user-review-ui-side-by-side-diff-screen-red-bar.png[width=800, link="images/gwt-user-review-ui-side-by-side-diff-screen-red-bar.png"]
[[patch-set-selection]]
In the header, on each side, the list of patch sets is shown. Clicking
@@ -787,7 +787,7 @@
version before, may see what has changed since that version by
comparing the old patch against the current patch.
-image::images/user-review-ui-side-by-side-diff-screen-patch-sets.png[width=800, link="images/user-review-ui-side-by-side-diff-screen-patch-sets.png"]
+image::images/gwt-user-review-ui-side-by-side-diff-screen-patch-sets.png[width=800, link="images/gwt-user-review-ui-side-by-side-diff-screen-patch-sets.png"]
[[download-file]]
The download icon next to the patch set list allows to download the
@@ -798,14 +798,14 @@
If the compared patches are identical, this is highlighted by a red
`No Differences` label in the screen header.
-image::images/user-review-ui-side-by-side-diff-screen-no-differences.png[width=800, link="images/user-review-ui-side-by-side-diff-screen-no-differences.png"]
+image::images/gwt-user-review-ui-side-by-side-diff-screen-no-differences.png[width=800, link="images/gwt-user-review-ui-side-by-side-diff-screen-no-differences.png"]
[[side-by-side-rename]]
If a file was renamed, the old and new file paths are shown in the
header together with a similarity index that shows how much of the file
content is unmodified.
-image::images/user-review-ui-side-by-side-diff-screen-rename.png[width=800, link="images/user-review-ui-side-by-side-diff-screen-rename.png"]
+image::images/gwt-user-review-ui-side-by-side-diff-screen-rename.png[width=800, link="images/gwt-user-review-ui-side-by-side-diff-screen-rename.png"]
[[navigation]]
For navigating between the patches in a patch set there are navigation
@@ -814,7 +814,7 @@
the next patch. The arrow up button leads back to the change screen. In
all cases the selection for the patch set comparison is kept.
-image::images/user-review-ui-side-by-side-diff-screen-navigation.png[width=800, link="images/user-review-ui-side-by-side-diff-screen-navigation.png"]
+image::images/gwt-user-review-ui-side-by-side-diff-screen-navigation.png[width=800, link="images/gwt-user-review-ui-side-by-side-diff-screen-navigation.png"]
[[inline-comments]]
=== Inline Comments
@@ -840,7 +840,7 @@
If the diff preference link:#expand-all-comments[Expand All Comments]
is set to `Expand`, all inline comments will be automatically expanded.
-image::images/user-review-ui-side-by-side-diff-screen-inline-comments.png[width=800, link="images/user-review-ui-side-by-side-diff-screen-inline-comments.png"]
+image::images/gwt-user-review-ui-side-by-side-diff-screen-inline-comments.png[width=800, link="images/gwt-user-review-ui-side-by-side-diff-screen-inline-comments.png"]
[[comment]]
In the header of the comment box, the name of the comment author and
@@ -849,7 +849,7 @@
top left corner. Below the actual comment there are buttons to reply to
the comment.
-image::images/user-review-ui-side-by-side-diff-screen-comment-box.png[width=800, link="images/user-review-ui-side-by-side-diff-screen-comment-box.png"]
+image::images/gwt-user-review-ui-side-by-side-diff-screen-comment-box.png[width=800, link="images/gwt-user-review-ui-side-by-side-diff-screen-comment-box.png"]
[[reply-inline-comment]]
Clicking on the `Reply` button opens an editor to type the reply.
@@ -868,7 +868,7 @@
Clicking on the `Discard` button deletes the inline draft comment.
-image::images/user-review-ui-side-by-side-diff-screen-comment-reply.png[width=800, link="images/user-review-ui-side-by-side-diff-screen-comment-reply.png"]
+image::images/gwt-user-review-ui-side-by-side-diff-screen-comment-reply.png[width=800, link="images/gwt-user-review-ui-side-by-side-diff-screen-comment-reply.png"]
[[draft-inline-comment]]
Draft comments are marked by the text "Draft" in the header in the
@@ -877,14 +877,14 @@
A draft comment can be edited by clicking on the `Edit` button, or
deleted by clicking on the `Discard` button.
-image::images/user-review-ui-side-by-side-diff-screen-comment-edit.png[width=800, link="images/user-review-ui-side-by-side-diff-screen-comment-edit.png"]
+image::images/gwt-user-review-ui-side-by-side-diff-screen-comment-edit.png[width=800, link="images/gwt-user-review-ui-side-by-side-diff-screen-comment-edit.png"]
[[done]]
Clicking on the `Done` button is a quick way to reply with "Done" to a
comment. This is used to mark a comment as addressed by a follow-up
patch set.
-image::images/user-review-ui-side-by-side-diff-screen-replied-done.png[width=800, link="images/user-review-ui-side-by-side-diff-screen-replied-done.png"]
+image::images/gwt-user-review-ui-side-by-side-diff-screen-replied-done.png[width=800, link="images/gwt-user-review-ui-side-by-side-diff-screen-replied-done.png"]
[[add-inline-comment]]
To add a new inline comment there are several possibilities:
@@ -910,7 +910,7 @@
** press 'V' + arrow keys (or 'j', 'k') to select a code block line-wise
** type 'bvw' to select a word
-image::images/user-review-ui-side-by-side-diff-screen-comment.png[width=800, link="images/user-review-ui-side-by-side-diff-screen-comment.png"]
+image::images/gwt-user-review-ui-side-by-side-diff-screen-comment.png[width=800, link="images/gwt-user-review-ui-side-by-side-diff-screen-comment.png"]
For typing the new comment, a new comment box is shown under the code
that is commented.
@@ -921,7 +921,7 @@
Clicking on the `Discard` button deletes the new comment.
-image::images/user-review-ui-side-by-side-diff-screen-commented.png[width=800, link="images/user-review-ui-side-by-side-diff-screen-commented.png"]
+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"]
[[file-level-comments]]
=== File Level Comments
@@ -931,12 +931,12 @@
File level comments are added by clicking on the comment icon in the
header above the file.
-image::images/user-review-ui-side-by-side-diff-screen-file-level-comment.png[width=800, link="images/user-review-ui-side-by-side-diff-screen-file-level-comment.png"]
+image::images/gwt-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"]
Clicking on the comment icon opens a comment box for typing the file
level comment.
-image::images/user-review-ui-side-by-side-diff-screen-file-level-commented.png[width=800, link="images/user-review-ui-side-by-side-diff-screen-file-level-commented.png"]
+image::images/gwt-user-review-ui-side-by-side-diff-screen-file-level-commented.png[width=800, link="images/gwt-user-review-ui-side-by-side-diff-screen-file-level-commented.png"]
[[search]]
=== Search
@@ -958,7 +958,7 @@
Searching by `Ctrl-F` finds matches only in the visible area of the
screen unless the link:#render[Render] diff preference is set to `Slow`.
-image::images/user-review-ui-side-by-side-diff-screen-search.png[width=800, link="images/user-review-ui-side-by-side-diff-screen-search.png"]
+image::images/gwt-user-review-ui-side-by-side-diff-screen-search.png[width=800, link="images/gwt-user-review-ui-side-by-side-diff-screen-search.png"]
[[key-navigation]]
=== Key Navigation
@@ -981,7 +981,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/user-review-ui-side-by-side-diff-screen-preferences.png"]
+image::images/gwt-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"]
The diff preferences popup allows to change the diff preferences.
By clicking on the `Save` button changes to the diff preferences are
@@ -990,7 +990,7 @@
screen is refreshed. The `Save` button is only available if the user is
signed in.
-image::images/user-review-ui-side-by-side-diff-screen-preferences-popup.png[width=800, link="images/user-review-ui-side-by-side-diff-screen-preferences-popup.png"]
+image::images/gwt-user-review-ui-side-by-side-diff-screen-preferences-popup.png[width=800, link="images/gwt-user-review-ui-side-by-side-diff-screen-preferences-popup.png"]
The following diff preferences can be configured:
@@ -1000,7 +1000,7 @@
+
E.g. users could choose to work with a dark theme.
+
-image::images/user-review-ui-side-by-side-diff-screen-dark-theme.png[width=800, link="images/user-review-ui-side-by-side-diff-screen-dark-theme.png"]
+image::images/gwt-user-review-ui-side-by-side-diff-screen-dark-theme.png[width=800, link="images/gwt-user-review-ui-side-by-side-diff-screen-dark-theme.png"]
- [[ignore-whitespace]]`Ignore Whitespace`:
+
@@ -1032,7 +1032,7 @@
is displayed so that one can easily detect lines the exceed the
preferred line length.
+
-image::images/user-review-ui-side-by-side-diff-screen-column.png[width=800, link="images/user-review-ui-side-by-side-diff-screen-column.png"]
+image::images/gwt-user-review-ui-side-by-side-diff-screen-column.png[width=800, link="images/gwt-user-review-ui-side-by-side-diff-screen-column.png"]
- [[lines-of-context]]`Lines Of Context`:
+
@@ -1049,13 +1049,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/user-review-ui-side-by-side-diff-screen-expand-skipped-lines.png"]
-
-- [[intraline-difference]]`Intraline Difference`:
-+
-Controls whether intraline differences should be highlighted.
-+
-image::images/user-review-ui-side-by-side-diff-screen-intraline-difference.png[width=800, link="images/user-review-ui-side-by-side-diff-screen-intraline-difference.png"]
+image::images/gwt-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"]
- [[syntax-highlighting]]`Syntax Highlighting`:
+
@@ -1065,7 +1059,7 @@
the file extension. The language can also be set manually by selecting
it from the `Language` drop-down list.
+
-image::images/user-review-ui-side-by-side-diff-screen-syntax-coloring.png[width=800, link="images/user-review-ui-side-by-side-diff-screen-syntax-coloring.png"]
+image::images/gwt-user-review-ui-side-by-side-diff-screen-syntax-coloring.png[width=800, link="images/gwt-user-review-ui-side-by-side-diff-screen-syntax-coloring.png"]
- [[whitespace-errors]]`Whitespace Errors`:
+
@@ -1138,45 +1132,15 @@
Navigation within the review UI can be completely done by keys, and
most actions can be controlled by keyboard shortcuts. Typing `?` opens
-a popup that shows a list of available keyboard shortcuts:
+a popup that shows a list of available keyboard shortcuts.
-- Change Screen
-+
-image::images/user-review-ui-change-screen-keyboard-shortcuts.png[width=800, link="images/user-review-ui-change-screen-keyboard-shortcuts.png"]
-- Side-by-Side Diff Screen
-+
-image::images/user-review-ui-side-by-side-diff-screen-keyboard-shortcuts.png[width=800, link="images/user-review-ui-side-by-side-diff-screen-keyboard-shortcuts.png"]
-+
+image::images/user-review-ui-change-screen-keyboard-shortcuts.png[width=800, link="images/gwt-user-review-ui-change-screen-keyboard-shortcuts.png"]
+
+
In addition, Vim-like commands can be used to link:#key-navigation[
navigate] and link:#search[search] within a patch file.
-[[new-vs-old]]
-== New Review UI vs. Old Review UI
-
-There are some important conceptual differences between the old and
-new review UIs:
-
-- The old change screen directly shows all patch sets of the change.
- With the new change screen only a single patch set is displayed;
- users can switch between the patch sets by choosing another patch
- set from the link:#patch-sets[Patch Sets] drop down panel in the
- screen header.
-- On the old side-by-side diff screen, new comments are inserted by
- double-clicking on a line. With the new side-by-side diff screen
- double-click is used to select a word for commenting on it; there
- are link:#add-inline-comment[several ways to insert new comments],
- e.g. by selecting a code block and clicking on the popup comment
- icon.
-
-[[limitations]]
-Limitations of the new review UI:
-
-- The new side-by-side diff screen cannot render images.
-
-- The new side-by-side diff screen isn't able to highlight line
- endings.
-
GERRIT
------
Part of link:index.html[Gerrit Code Review]
diff --git a/Documentation/user-upload.txt b/Documentation/user-upload.txt
index cdaf155..0670968 100644
--- a/Documentation/user-upload.txt
+++ b/Documentation/user-upload.txt
@@ -29,6 +29,13 @@
* Both, the HTTP and the LDAP passwords (in this order) if `gitBasicAuthPolicy`
is `HTTP_LDAP`.
+When gitBasicAuthPolicy is set to `LDAP` or `HTTP_LDAP` and the user
+is authenticating with the LDAP username/password, the Git client config
+needs to have `http.cookieFile` set to a local file, otherwise every
+single call would trigger a full LDAP authentication and groups resolution
+which could introduce a noticeable latency on the overall execution
+and produce unwanted load to the LDAP server.
+
When gitBasicAuthPolicy is not `LDAP`, the user's HTTP credentials can
be regenerated by going to `Settings`, and then accessing the `HTTP
Password` tab. Revocation can effectively be done by regenerating the
diff --git a/java/com/google/gerrit/acceptance/AbstractPredicateTest.java b/java/com/google/gerrit/acceptance/AbstractPredicateTest.java
new file mode 100644
index 0000000..c8bdb63
--- /dev/null
+++ b/java/com/google/gerrit/acceptance/AbstractPredicateTest.java
@@ -0,0 +1,109 @@
+// 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;
+
+import static com.google.common.base.Preconditions.checkArgument;
+
+import com.google.gerrit.common.Nullable;
+import com.google.gerrit.entities.Change;
+import com.google.gerrit.extensions.annotations.Exports;
+import com.google.gerrit.extensions.common.PluginDefinedInfo;
+import com.google.gerrit.index.query.Predicate;
+import com.google.gerrit.index.query.QueryParseException;
+import com.google.gerrit.json.OutputFormat;
+import com.google.gerrit.server.DynamicOptions;
+import com.google.gerrit.server.change.ChangePluginDefinedInfoFactory;
+import com.google.gerrit.server.query.change.ChangeData;
+import com.google.gerrit.server.query.change.ChangeQueryBuilder;
+import com.google.gerrit.server.restapi.change.QueryChanges;
+import com.google.gerrit.sshd.commands.Query;
+import com.google.gson.Gson;
+import com.google.gson.reflect.TypeToken;
+import com.google.inject.AbstractModule;
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.kohsuke.args4j.Option;
+
+public abstract class AbstractPredicateTest extends AbstractDaemonTest {
+ public static final String PLUGIN_NAME = "my-plugin";
+ public static final Gson GSON = OutputFormat.JSON.newGson();
+
+ protected static class PluginModule extends AbstractModule {
+ @Override
+ public void configure() {
+ bind(DynamicOptions.DynamicBean.class)
+ .annotatedWith(Exports.named(Query.class))
+ .to(MyQueryOptions.class);
+ bind(DynamicOptions.DynamicBean.class)
+ .annotatedWith(Exports.named(QueryChanges.class))
+ .to(MyQueryOptions.class);
+ bind(ChangePluginDefinedInfoFactory.class)
+ .annotatedWith(Exports.named("sample"))
+ .to(AttributeFactory.class);
+ }
+ }
+
+ public static class MyQueryOptions implements DynamicOptions.DynamicBean {
+ @Option(name = "--sample")
+ public boolean sample;
+ }
+
+ protected static class AttributeFactory implements ChangePluginDefinedInfoFactory {
+ private final Provider<ChangeQueryBuilder> queryBuilderProvider;
+
+ @Inject
+ AttributeFactory(Provider<ChangeQueryBuilder> queryBuilderProvider) {
+ this.queryBuilderProvider = queryBuilderProvider;
+ }
+
+ @Override
+ public Map<Change.Id, PluginDefinedInfo> createPluginDefinedInfos(
+ Collection<ChangeData> cds, DynamicOptions.BeanProvider beanProvider, String plugin) {
+ MyQueryOptions options = (MyQueryOptions) beanProvider.getDynamicBean(plugin);
+ Map<Change.Id, PluginDefinedInfo> res = new HashMap<>();
+ if (options.sample) {
+ try {
+ Predicate<ChangeData> predicate = queryBuilderProvider.get().parse("label:Code-Review+2");
+ for (ChangeData cd : cds) {
+ PluginDefinedInfo myInfo = new PluginDefinedInfo();
+ if (predicate.isMatchable() && predicate.asMatchable().match(cd)) {
+ myInfo.message = "matched";
+ } else {
+ myInfo.message = "not matched";
+ }
+ res.put(cd.getId(), myInfo);
+ }
+ } catch (QueryParseException e) {
+ // ignored
+ }
+ }
+ return res;
+ }
+ }
+
+ protected static List<PluginDefinedInfo> decodeRawPluginsList(@Nullable Object plugins) {
+ if (plugins == null) {
+ return Collections.emptyList();
+ }
+ checkArgument(plugins instanceof List, "not a list: %s", plugins);
+ return GSON.fromJson(
+ GSON.toJson(plugins), new TypeToken<List<PluginDefinedInfo>>() {}.getType());
+ }
+}
diff --git a/java/com/google/gerrit/acceptance/ExtensionRegistry.java b/java/com/google/gerrit/acceptance/ExtensionRegistry.java
index 6c6bab0..85c4c13 100644
--- a/java/com/google/gerrit/acceptance/ExtensionRegistry.java
+++ b/java/com/google/gerrit/acceptance/ExtensionRegistry.java
@@ -35,6 +35,7 @@
import com.google.gerrit.extensions.webui.EditWebLink;
import com.google.gerrit.extensions.webui.FileHistoryWebLink;
import com.google.gerrit.extensions.webui.PatchSetWebLink;
+import com.google.gerrit.extensions.webui.ResolveConflictsWebLink;
import com.google.gerrit.server.ExceptionHook;
import com.google.gerrit.server.account.GroupBackend;
import com.google.gerrit.server.change.ChangeETagComputation;
@@ -76,6 +77,7 @@
private final DynamicSet<GitReferenceUpdatedListener> refUpdatedListeners;
private final DynamicSet<FileHistoryWebLink> fileHistoryWebLinks;
private final DynamicSet<PatchSetWebLink> patchSetWebLinks;
+ private final DynamicSet<ResolveConflictsWebLink> resolveConflictsWebLinks;
private final DynamicSet<EditWebLink> editWebLinks;
private final DynamicSet<RevisionCreatedListener> revisionCreatedListeners;
private final DynamicSet<GroupBackend> groupBackends;
@@ -111,6 +113,7 @@
DynamicSet<GitReferenceUpdatedListener> refUpdatedListeners,
DynamicSet<FileHistoryWebLink> fileHistoryWebLinks,
DynamicSet<PatchSetWebLink> patchSetWebLinks,
+ DynamicSet<ResolveConflictsWebLink> resolveConflictsWebLinks,
DynamicSet<EditWebLink> editWebLinks,
DynamicSet<RevisionCreatedListener> revisionCreatedListeners,
DynamicSet<GroupBackend> groupBackends,
@@ -143,6 +146,7 @@
this.fileHistoryWebLinks = fileHistoryWebLinks;
this.patchSetWebLinks = patchSetWebLinks;
this.editWebLinks = editWebLinks;
+ this.resolveConflictsWebLinks = resolveConflictsWebLinks;
this.revisionCreatedListeners = revisionCreatedListeners;
this.groupBackends = groupBackends;
this.accountActivationValidationListeners = accountActivationValidationListeners;
@@ -244,6 +248,10 @@
return add(patchSetWebLinks, patchSetWebLink);
}
+ public Registration add(ResolveConflictsWebLink resolveConflictsWebLink) {
+ return add(resolveConflictsWebLinks, resolveConflictsWebLink);
+ }
+
public Registration add(EditWebLink editWebLink) {
return add(editWebLinks, editWebLink);
}
diff --git a/java/com/google/gerrit/auth/BUILD b/java/com/google/gerrit/auth/BUILD
index 609ec8a..e844696 100644
--- a/java/com/google/gerrit/auth/BUILD
+++ b/java/com/google/gerrit/auth/BUILD
@@ -20,6 +20,7 @@
"//java/com/google/gerrit/common:server",
"//java/com/google/gerrit/entities",
"//java/com/google/gerrit/extensions:api",
+ "//java/com/google/gerrit/metrics",
"//java/com/google/gerrit/proto",
"//java/com/google/gerrit/server",
"//java/com/google/gerrit/server/cache/serialize",
diff --git a/java/com/google/gerrit/auth/ldap/Helper.java b/java/com/google/gerrit/auth/ldap/Helper.java
index bf2a8c2..a939c72 100644
--- a/java/com/google/gerrit/auth/ldap/Helper.java
+++ b/java/com/google/gerrit/auth/ldap/Helper.java
@@ -20,6 +20,10 @@
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.common.data.ParameterizedString;
import com.google.gerrit.entities.AccountGroup;
+import com.google.gerrit.metrics.Description;
+import com.google.gerrit.metrics.Description.Units;
+import com.google.gerrit.metrics.MetricMaker;
+import com.google.gerrit.metrics.Timer0;
import com.google.gerrit.server.account.AccountException;
import com.google.gerrit.server.account.AuthenticationFailedException;
import com.google.gerrit.server.auth.NoSuchUserException;
@@ -81,11 +85,16 @@
private final String connectTimeoutMillis;
private final boolean useConnectionPooling;
private final boolean groupsVisibleToAll;
+ private final Timer0 loginLatencyTimer;
+ private final Timer0 userSearchLatencyTimer;
+ private final Timer0 groupSearchLatencyTimer;
+ private final Timer0 groupExpansionLatencyTimer;
@Inject
Helper(
@GerritServerConfig Config config,
- @Named(LdapModule.PARENT_GROUPS_CACHE) Cache<String, ImmutableSet<String>> parentGroups) {
+ @Named(LdapModule.PARENT_GROUPS_CACHE) Cache<String, ImmutableSet<String>> parentGroups,
+ MetricMaker metricMaker) {
this.config = config;
this.server = LdapRealm.optional(config, "server");
this.username = LdapRealm.optional(config, "username");
@@ -112,6 +121,33 @@
}
this.parentGroups = parentGroups;
this.useConnectionPooling = LdapRealm.optional(config, "useConnectionPooling", false);
+
+ this.loginLatencyTimer =
+ metricMaker.newTimer(
+ "ldap/login_latency",
+ new Description("Latency of logins").setCumulative().setUnit(Units.NANOSECONDS));
+ this.userSearchLatencyTimer =
+ metricMaker.newTimer(
+ "ldap/user_search_latency",
+ new Description("Latency for searching the user account")
+ .setCumulative()
+ .setUnit(Units.NANOSECONDS));
+ this.groupSearchLatencyTimer =
+ metricMaker.newTimer(
+ "ldap/group_search_latency",
+ new Description("Latency for querying the groups membership of an account")
+ .setCumulative()
+ .setUnit(Units.NANOSECONDS));
+ this.groupExpansionLatencyTimer =
+ metricMaker.newTimer(
+ "ldap/group_expansion_latency",
+ new Description("Latency for expanding nested groups")
+ .setCumulative()
+ .setUnit(Units.NANOSECONDS));
+ }
+
+ Timer0 getGroupSearchLatencyTimer() {
+ return groupSearchLatencyTimer;
}
private Properties createContextProperties() {
@@ -191,7 +227,9 @@
private DirContext kerberosOpen(Properties env)
throws IOException, LoginException, NamingException {
LoginContext ctx = new LoginContext("KerberosLogin");
- ctx.login();
+ try (Timer0.Context ignored = loginLatencyTimer.start()) {
+ ctx.login();
+ }
Subject subject = ctx.getSubject();
try {
return Subject.doAs(
@@ -209,7 +247,7 @@
DirContext authenticate(String dn, String password) throws AccountException {
final Properties env = createContextProperties();
- try {
+ try (Timer0.Context ignored = loginLatencyTimer.start()) {
env.put(Context.REFERRAL, referral);
if (!supportAnonymous) {
@@ -258,7 +296,7 @@
}
for (LdapQuery accountQuery : accountQueryList) {
- List<LdapQuery.Result> res = accountQuery.query(ctx, params);
+ List<LdapQuery.Result> res = accountQuery.query(ctx, params, userSearchLatencyTimer);
if (res.size() == 1) {
return res.get(0);
} else if (res.size() > 1) {
@@ -290,8 +328,10 @@
params.put(LdapRealm.USERNAME, username);
for (LdapQuery groupMemberQuery : schema.groupMemberQueryList) {
- for (LdapQuery.Result r : groupMemberQuery.query(ctx, params)) {
- recursivelyExpandGroups(groupDNs, schema, ctx, r.getDN());
+ for (LdapQuery.Result r : groupMemberQuery.query(ctx, params, groupSearchLatencyTimer)) {
+ try (Timer0.Context ignored = groupExpansionLatencyTimer.start()) {
+ recursivelyExpandGroups(groupDNs, schema, ctx, r.getDN());
+ }
}
}
}
diff --git a/java/com/google/gerrit/auth/ldap/LdapGroupBackend.java b/java/com/google/gerrit/auth/ldap/LdapGroupBackend.java
index f82523e..2947efd 100644
--- a/java/com/google/gerrit/auth/ldap/LdapGroupBackend.java
+++ b/java/com/google/gerrit/auth/ldap/LdapGroupBackend.java
@@ -205,7 +205,8 @@
Map<String, String> params = Collections.emptyMap();
for (String groupBase : schema.groupBases) {
LdapQuery query = new LdapQuery(groupBase, schema.groupScope, filter, returnAttrs);
- for (LdapQuery.Result res : query.query(ctx, params)) {
+ for (LdapQuery.Result res :
+ query.query(ctx, params, helper.getGroupSearchLatencyTimer())) {
out.add(groupReference(schema.groupName, res));
}
}
diff --git a/java/com/google/gerrit/auth/ldap/LdapQuery.java b/java/com/google/gerrit/auth/ldap/LdapQuery.java
index 2586fd4..409c9f5 100644
--- a/java/com/google/gerrit/auth/ldap/LdapQuery.java
+++ b/java/com/google/gerrit/auth/ldap/LdapQuery.java
@@ -15,6 +15,7 @@
package com.google.gerrit.auth.ldap;
import com.google.gerrit.common.data.ParameterizedString;
+import com.google.gerrit.metrics.Timer0;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
@@ -61,13 +62,16 @@
return pattern.getParameterNames();
}
- List<Result> query(DirContext ctx, Map<String, String> params) throws NamingException {
+ List<Result> query(DirContext ctx, Map<String, String> params, Timer0 queryTimer)
+ throws NamingException {
final SearchControls sc = new SearchControls();
final NamingEnumeration<SearchResult> res;
sc.setSearchScope(searchScope.scope());
sc.setReturningAttributes(returnAttributes);
- res = ctx.search(base, pattern.getRawPattern(), pattern.bind(params), sc);
+ try (Timer0.Context ignored = queryTimer.start()) {
+ res = ctx.search(base, pattern.getRawPattern(), pattern.bind(params), sc);
+ }
try {
final List<Result> r = new ArrayList<>();
try {
diff --git a/java/com/google/gerrit/extensions/common/CommitInfo.java b/java/com/google/gerrit/extensions/common/CommitInfo.java
index 1fd8755..202b829 100644
--- a/java/com/google/gerrit/extensions/common/CommitInfo.java
+++ b/java/com/google/gerrit/extensions/common/CommitInfo.java
@@ -29,6 +29,7 @@
public String subject;
public String message;
public List<WebLinkInfo> webLinks;
+ public List<WebLinkInfo> resolveConflictsWebLinks;
@Override
public boolean equals(Object o) {
@@ -42,12 +43,14 @@
&& Objects.equals(committer, c.committer)
&& Objects.equals(subject, c.subject)
&& Objects.equals(message, c.message)
- && Objects.equals(webLinks, c.webLinks);
+ && Objects.equals(webLinks, c.webLinks)
+ && Objects.equals(resolveConflictsWebLinks, c.resolveConflictsWebLinks);
}
@Override
public int hashCode() {
- return Objects.hash(commit, parents, author, committer, subject, message, webLinks);
+ return Objects.hash(
+ commit, parents, author, committer, subject, message, webLinks, resolveConflictsWebLinks);
}
@Override
@@ -64,6 +67,9 @@
if (webLinks != null) {
helper.add("webLinks", webLinks);
}
+ if (resolveConflictsWebLinks != null) {
+ helper.add("resolveConflictsWebLinks", resolveConflictsWebLinks);
+ }
return helper.toString();
}
}
diff --git a/java/com/google/gerrit/extensions/common/testing/CommitInfoSubject.java b/java/com/google/gerrit/extensions/common/testing/CommitInfoSubject.java
index d344e18..71fc564 100644
--- a/java/com/google/gerrit/extensions/common/testing/CommitInfoSubject.java
+++ b/java/com/google/gerrit/extensions/common/testing/CommitInfoSubject.java
@@ -20,6 +20,7 @@
import com.google.common.truth.Correspondence;
import com.google.common.truth.FailureMetadata;
+import com.google.common.truth.IterableSubject;
import com.google.common.truth.StringSubject;
import com.google.common.truth.Subject;
import com.google.gerrit.extensions.common.CommitInfo;
@@ -68,6 +69,16 @@
return check("message").that(commitInfo.message);
}
+ public IterableSubject webLinks() {
+ isNotNull();
+ return check("webLinks").that(commitInfo.webLinks);
+ }
+
+ public IterableSubject resolveConflictsWebLinks() {
+ isNotNull();
+ return check("resolveConflictsWebLinks").that(commitInfo.resolveConflictsWebLinks);
+ }
+
public static Correspondence<CommitInfo, String> hasCommit() {
return NullAwareCorrespondence.transforming(commitInfo -> commitInfo.commit, "hasCommit");
}
diff --git a/java/com/google/gerrit/extensions/webui/ResolveConflictsWebLink.java b/java/com/google/gerrit/extensions/webui/ResolveConflictsWebLink.java
new file mode 100644
index 0000000..19402a9
--- /dev/null
+++ b/java/com/google/gerrit/extensions/webui/ResolveConflictsWebLink.java
@@ -0,0 +1,40 @@
+// 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.extensions.webui;
+
+import com.google.gerrit.extensions.annotations.ExtensionPoint;
+import com.google.gerrit.extensions.common.WebLinkInfo;
+
+@ExtensionPoint
+public interface ResolveConflictsWebLink extends WebLink {
+
+ /**
+ * {@link com.google.gerrit.extensions.common.WebLinkInfo} describing a link from a patch set to
+ * an external service for the purpose of resolving merge conflicts.
+ *
+ * <p>In order for the web link to be visible {@link
+ * com.google.gerrit.extensions.common.WebLinkInfo#url} and {@link
+ * com.google.gerrit.extensions.common.WebLinkInfo#name} must be set.
+ *
+ * @param projectName name of the project
+ * @param commit commit of the patch set
+ * @param commitMessage the commit message of the change
+ * @param branchName target branch of the change
+ * @return WebLinkInfo that links to patch set in external service, {@code null} if there should
+ * be no link.
+ */
+ WebLinkInfo getResolveConflictsWebLink(
+ String projectName, String commit, String commitMessage, String branchName);
+}
diff --git a/java/com/google/gerrit/httpd/CacheBasedWebSession.java b/java/com/google/gerrit/httpd/CacheBasedWebSession.java
index a3a67e5..7212e3e 100644
--- a/java/com/google/gerrit/httpd/CacheBasedWebSession.java
+++ b/java/com/google/gerrit/httpd/CacheBasedWebSession.java
@@ -76,29 +76,27 @@
this.identified = identified;
this.byIdCache = byIdCache;
- if (request.getRequestURI() == null || !GitSmartHttpTools.isGitClient(request)) {
- String cookie = readCookie(request);
- if (cookie != null) {
- authFromCookie(cookie);
- } else {
- String token;
- try {
- token = ParameterParser.getQueryParams(request).accessToken();
- } catch (BadRequestException e) {
- token = null;
- }
- if (token != null) {
- authFromQueryParameter(token);
- }
+ String cookie = readCookie(request);
+ if (cookie != null) {
+ authFromCookie(cookie);
+ } else if (request.getRequestURI() == null || !GitSmartHttpTools.isGitClient(request)) {
+ String token;
+ try {
+ token = ParameterParser.getQueryParams(request).accessToken();
+ } catch (BadRequestException e) {
+ token = null;
}
- if (val != null && !checkAccountStatus(val.getAccountId())) {
- val = null;
- okPaths.clear();
+ if (token != null) {
+ authFromQueryParameter(token);
}
- if (val != null && val.needsCookieRefresh()) {
- // Session is more than half old; update cache entry with new expiration date.
- val = manager.createVal(key, val);
- }
+ }
+ if (val != null && !checkAccountStatus(val.getAccountId())) {
+ val = null;
+ okPaths.clear();
+ }
+ if (val != null && val.needsCookieRefresh()) {
+ // Session is more than half old; update cache entry with new expiration date.
+ val = manager.createVal(key, val);
}
}
diff --git a/java/com/google/gerrit/httpd/ProjectBasicAuthFilter.java b/java/com/google/gerrit/httpd/ProjectBasicAuthFilter.java
index 111cc34..7fcd4f8 100644
--- a/java/com/google/gerrit/httpd/ProjectBasicAuthFilter.java
+++ b/java/com/google/gerrit/httpd/ProjectBasicAuthFilter.java
@@ -22,6 +22,7 @@
import com.google.common.base.Strings;
import com.google.common.flogger.FluentLogger;
import com.google.common.io.BaseEncoding;
+import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.Account;
import com.google.gerrit.extensions.client.GitBasicAuthPolicy;
import com.google.gerrit.extensions.registration.DynamicItem;
@@ -99,7 +100,7 @@
HttpServletRequest req = (HttpServletRequest) request;
Response rsp = new Response((HttpServletResponse) response);
- if (verify(req, rsp)) {
+ if (session.get().isSignedIn() || verify(req, rsp)) {
chain.doFilter(req, rsp);
}
}
@@ -144,7 +145,7 @@
if (gitBasicAuthPolicy == GitBasicAuthPolicy.HTTP
|| gitBasicAuthPolicy == GitBasicAuthPolicy.HTTP_LDAP) {
if (PasswordVerifier.checkPassword(who.externalIds(), username, password)) {
- return succeedAuthentication(who);
+ return succeedAuthentication(who, null);
}
}
@@ -157,11 +158,11 @@
try {
AuthResult whoAuthResult = accountManager.authenticate(whoAuth);
- setUserIdentified(whoAuthResult.getAccountId());
+ setUserIdentified(whoAuthResult.getAccountId(), whoAuthResult);
return true;
} catch (NoSuchUserException e) {
if (PasswordVerifier.checkPassword(who.externalIds(), username, password)) {
- return succeedAuthentication(who);
+ return succeedAuthentication(who, null);
}
logger.atWarning().withCause(e).log(authenticationFailedMsg(username, req));
rsp.sendError(SC_UNAUTHORIZED);
@@ -183,8 +184,8 @@
}
}
- private boolean succeedAuthentication(AccountState who) {
- setUserIdentified(who.account().id());
+ private boolean succeedAuthentication(AccountState who, @Nullable AuthResult whoAuthResult) {
+ setUserIdentified(who.account().id(), whoAuthResult);
return true;
}
@@ -201,11 +202,15 @@
return String.format("Authentication from %s failed for %s", req.getRemoteAddr(), username);
}
- private void setUserIdentified(Account.Id id) {
+ private void setUserIdentified(Account.Id id, @Nullable AuthResult whoAuthResult) {
WebSession ws = session.get();
ws.setUserAccountId(id);
ws.setAccessPathOk(AccessPath.GIT, true);
ws.setAccessPathOk(AccessPath.REST_API, true);
+
+ if (whoAuthResult != null) {
+ ws.login(whoAuthResult, false);
+ }
}
private String encoding(HttpServletRequest req) {
diff --git a/java/com/google/gerrit/server/WebLinks.java b/java/com/google/gerrit/server/WebLinks.java
index 3b626ea..4acef06 100644
--- a/java/com/google/gerrit/server/WebLinks.java
+++ b/java/com/google/gerrit/server/WebLinks.java
@@ -33,6 +33,7 @@
import com.google.gerrit.extensions.webui.ParentWebLink;
import com.google.gerrit.extensions.webui.PatchSetWebLink;
import com.google.gerrit.extensions.webui.ProjectWebLink;
+import com.google.gerrit.extensions.webui.ResolveConflictsWebLink;
import com.google.gerrit.extensions.webui.TagWebLink;
import com.google.gerrit.extensions.webui.WebLink;
import com.google.inject.Inject;
@@ -56,6 +57,7 @@
};
private final DynamicSet<PatchSetWebLink> patchSetLinks;
+ private final DynamicSet<ResolveConflictsWebLink> resolveConflictsLinks;
private final DynamicSet<ParentWebLink> parentLinks;
private final DynamicSet<EditWebLink> editLinks;
private final DynamicSet<FileWebLink> fileLinks;
@@ -68,6 +70,7 @@
@Inject
public WebLinks(
DynamicSet<PatchSetWebLink> patchSetLinks,
+ DynamicSet<ResolveConflictsWebLink> resolveConflictsLinks,
DynamicSet<ParentWebLink> parentLinks,
DynamicSet<EditWebLink> editLinks,
DynamicSet<FileWebLink> fileLinks,
@@ -77,6 +80,7 @@
DynamicSet<BranchWebLink> branchLinks,
DynamicSet<TagWebLink> tagLinks) {
this.patchSetLinks = patchSetLinks;
+ this.resolveConflictsLinks = resolveConflictsLinks;
this.parentLinks = parentLinks;
this.editLinks = editLinks;
this.fileLinks = fileLinks;
@@ -103,6 +107,21 @@
/**
* @param project Project name.
+ * @param revision SHA1 of commit.
+ * @param commitMessage the commit message of the commit.
+ * @param branchName branch of the commit.
+ * @return Links for resolving comflicts.
+ */
+ public ImmutableList<WebLinkInfo> getResolveConflictsLinks(
+ Project.NameKey project, String commit, String commitMessage, String branchName) {
+ return filterLinks(
+ resolveConflictsLinks,
+ webLink ->
+ webLink.getResolveConflictsWebLink(project.get(), commit, commitMessage, branchName));
+ }
+
+ /**
+ * @param project Project name.
* @param revision SHA1 of the parent revision.
* @param commitMessage the commit message of the parent revision.
* @param branchName branch of the revision (and parent revision).
diff --git a/java/com/google/gerrit/server/account/AccountsUpdate.java b/java/com/google/gerrit/server/account/AccountsUpdate.java
index b46d10d..5a74047 100644
--- a/java/com/google/gerrit/server/account/AccountsUpdate.java
+++ b/java/com/google/gerrit/server/account/AccountsUpdate.java
@@ -428,6 +428,11 @@
* Updates multiple different accounts atomically. This will only store a single new value (aka
* set of all external IDs of the host) in the external ID cache, which is important for storage
* economy. All {@code updates} must be for different accounts.
+ *
+ * <p>NOTE on error handling: Since updates are executed in multiple stages, with some stages
+ * resulting from the union of all individual updates, we cannot point to the update that caused
+ * the error. Callers should be aware that a single "update of death" (or a set of updates that
+ * together have this property) will always prevent the entire batch from being executed.
*/
public ImmutableList<Optional<AccountState>> updateBatch(List<UpdateArguments> updates)
throws IOException, ConfigInvalidException {
diff --git a/java/com/google/gerrit/server/change/RevisionJson.java b/java/com/google/gerrit/server/change/RevisionJson.java
index b702440..33f3d4f 100644
--- a/java/com/google/gerrit/server/change/RevisionJson.java
+++ b/java/com/google/gerrit/server/change/RevisionJson.java
@@ -182,9 +182,14 @@
info.message = commit.getFullMessage();
if (addLinks) {
- ImmutableList<WebLinkInfo> links =
+ ImmutableList<WebLinkInfo> patchSetLinks =
webLinks.getPatchSetLinks(project, commit.name(), commit.getFullMessage(), branchName);
- info.webLinks = links.isEmpty() ? null : links;
+ info.webLinks = patchSetLinks.isEmpty() ? null : patchSetLinks;
+ ImmutableList<WebLinkInfo> resolveConflictsLinks =
+ webLinks.getResolveConflictsLinks(
+ project, commit.name(), commit.getFullMessage(), branchName);
+ info.resolveConflictsWebLinks =
+ resolveConflictsLinks.isEmpty() ? null : resolveConflictsLinks;
}
for (RevCommit parent : commit.getParents()) {
diff --git a/java/com/google/gerrit/server/config/GerritGlobalModule.java b/java/com/google/gerrit/server/config/GerritGlobalModule.java
index 339b350..4794858 100644
--- a/java/com/google/gerrit/server/config/GerritGlobalModule.java
+++ b/java/com/google/gerrit/server/config/GerritGlobalModule.java
@@ -72,6 +72,7 @@
import com.google.gerrit.extensions.webui.ParentWebLink;
import com.google.gerrit.extensions.webui.PatchSetWebLink;
import com.google.gerrit.extensions.webui.ProjectWebLink;
+import com.google.gerrit.extensions.webui.ResolveConflictsWebLink;
import com.google.gerrit.extensions.webui.TagWebLink;
import com.google.gerrit.extensions.webui.TopMenu;
import com.google.gerrit.extensions.webui.WebUiPlugin;
@@ -392,6 +393,7 @@
DynamicMap.mapOf(binder(), ProjectConfigEntry.class);
DynamicSet.setOf(binder(), PluginPushOption.class);
DynamicSet.setOf(binder(), PatchSetWebLink.class);
+ DynamicSet.setOf(binder(), ResolveConflictsWebLink.class);
DynamicSet.setOf(binder(), ParentWebLink.class);
DynamicSet.setOf(binder(), FileWebLink.class);
DynamicSet.setOf(binder(), FileHistoryWebLink.class);
diff --git a/java/com/google/gerrit/server/config/GitwebConfig.java b/java/com/google/gerrit/server/config/GitwebConfig.java
index 97cc830..f90a72e 100644
--- a/java/com/google/gerrit/server/config/GitwebConfig.java
+++ b/java/com/google/gerrit/server/config/GitwebConfig.java
@@ -33,6 +33,7 @@
import com.google.gerrit.extensions.webui.ParentWebLink;
import com.google.gerrit.extensions.webui.PatchSetWebLink;
import com.google.gerrit.extensions.webui.ProjectWebLink;
+import com.google.gerrit.extensions.webui.ResolveConflictsWebLink;
import com.google.gerrit.extensions.webui.TagWebLink;
import com.google.inject.AbstractModule;
import com.google.inject.Inject;
@@ -83,6 +84,7 @@
if (!isNullOrEmpty(type.getRevision())) {
DynamicSet.bind(binder(), PatchSetWebLink.class).to(GitwebLinks.class);
DynamicSet.bind(binder(), ParentWebLink.class).to(GitwebLinks.class);
+ DynamicSet.bind(binder(), ResolveConflictsWebLink.class).to(GitwebLinks.class);
}
if (!isNullOrEmpty(type.getProject())) {
@@ -261,6 +263,7 @@
PatchSetWebLink,
ParentWebLink,
ProjectWebLink,
+ ResolveConflictsWebLink,
TagWebLink {
private final String url;
private final GitwebType type;
@@ -350,6 +353,13 @@
}
@Override
+ public WebLinkInfo getResolveConflictsWebLink(
+ String projectName, String commit, String commitMessage, String branchName) {
+ // For Gitweb treat resolve conflicts links the same as patch set links
+ return getPatchSetWebLink(projectName, commit, commitMessage, branchName);
+ }
+
+ @Override
public WebLinkInfo getParentWebLink(
String projectName, String commit, String commitMessage, String branchName) {
// For Gitweb treat parent revision links the same as patch set links
diff --git a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
index 377718b..15bc603 100644
--- a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
+++ b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
@@ -1531,6 +1531,14 @@
@Option(name = "--remove-private", usage = "remove privacy flag from updated change")
boolean removePrivate;
+ /**
+ * The skip-validation option is defined to allow parsing it using the {@link #cmdLineParser}.
+ * However we do not allow this option for pushes to magic branches. This option is used to fail
+ * with a proper error message.
+ */
+ @Option(name = "--skip-validation", usage = "skips commit validation")
+ boolean skipValidation;
+
@Option(
name = "--wip",
aliases = {"-work-in-progress"},
@@ -1814,6 +1822,14 @@
ref = null; // never happens
}
+ if (magicBranch.skipValidation) {
+ reject(
+ cmd,
+ String.format(
+ "\"--%s\" option is only supported for direct push", PUSH_OPTION_SKIP_VALIDATION));
+ return;
+ }
+
if (magicBranch.topic != null && magicBranch.topic.length() > ChangeUtil.TOPIC_MAX_LENGTH) {
reject(
cmd, String.format("topic length exceeds the limit (%d)", ChangeUtil.TOPIC_MAX_LENGTH));
diff --git a/java/com/google/gerrit/server/query/change/EqualsLabelPredicate.java b/java/com/google/gerrit/server/query/change/EqualsLabelPredicate.java
index 4b6c964..3e9209b 100644
--- a/java/com/google/gerrit/server/query/change/EqualsLabelPredicate.java
+++ b/java/com/google/gerrit/server/query/change/EqualsLabelPredicate.java
@@ -73,6 +73,7 @@
}
boolean hasVote = false;
+ object.setStorageConstraint(ChangeData.StorageConstraint.INDEX_PRIMARY_NOTEDB_SECONDARY);
for (PatchSetApproval p : object.currentApprovals()) {
if (labelType.matches(p)) {
hasVote = true;
diff --git a/java/com/google/gerrit/server/restapi/account/CreateAccount.java b/java/com/google/gerrit/server/restapi/account/CreateAccount.java
index 1cf875a..baa2951 100644
--- a/java/com/google/gerrit/server/restapi/account/CreateAccount.java
+++ b/java/com/google/gerrit/server/restapi/account/CreateAccount.java
@@ -44,6 +44,7 @@
import com.google.gerrit.server.account.VersionedAuthorizedKeys;
import com.google.gerrit.server.account.externalids.DuplicateExternalIdKeyException;
import com.google.gerrit.server.account.externalids.ExternalId;
+import com.google.gerrit.server.config.AuthConfig;
import com.google.gerrit.server.group.GroupResolver;
import com.google.gerrit.server.group.db.GroupDelta;
import com.google.gerrit.server.group.db.GroupsUpdate;
@@ -59,6 +60,7 @@
import java.util.ArrayList;
import java.util.HashSet;
import java.util.List;
+import java.util.Locale;
import java.util.Set;
import org.eclipse.jgit.errors.ConfigInvalidException;
@@ -82,6 +84,7 @@
private final PluginSetContext<AccountExternalIdCreator> externalIdCreators;
private final Provider<GroupsUpdate> groupsUpdate;
private final OutgoingEmailValidator validator;
+ private final AuthConfig authConfig;
@Inject
CreateAccount(
@@ -93,7 +96,8 @@
AccountLoader.Factory infoLoader,
PluginSetContext<AccountExternalIdCreator> externalIdCreators,
@UserInitiated Provider<GroupsUpdate> groupsUpdate,
- OutgoingEmailValidator validator) {
+ OutgoingEmailValidator validator,
+ AuthConfig authConfig) {
this.seq = seq;
this.groupResolver = groupResolver;
this.authorizedKeys = authorizedKeys;
@@ -103,6 +107,7 @@
this.externalIdCreators = externalIdCreators;
this.groupsUpdate = groupsUpdate;
this.validator = validator;
+ this.authConfig = authConfig;
}
@Override
@@ -116,14 +121,18 @@
public Response<AccountInfo> apply(IdString id, AccountInput input)
throws BadRequestException, ResourceConflictException, UnprocessableEntityException,
IOException, ConfigInvalidException, PermissionBackendException {
- String username = id.get();
- if (input.username != null && !username.equals(input.username)) {
+ String username = applyCaseOfUsername(id.get());
+ if (input.username != null && !username.equals(applyCaseOfUsername(input.username))) {
throw new BadRequestException("username must match URL");
}
if (!ExternalId.isValidUsername(username)) {
throw new BadRequestException("Invalid username '" + username + "'");
}
+ if (input.name == null) {
+ input.name = input.username;
+ }
+
Set<AccountGroup.UUID> groups = parseGroups(input.groups);
Account.Id accountId = Account.id(seq.nextAccountId());
@@ -182,6 +191,10 @@
return Response.created(info);
}
+ private String applyCaseOfUsername(String username) {
+ return authConfig.isUserNameToLowerCase() ? username.toLowerCase(Locale.US) : username;
+ }
+
private Set<AccountGroup.UUID> parseGroups(List<String> groups)
throws UnprocessableEntityException {
Set<AccountGroup.UUID> groupUuids = new HashSet<>();
diff --git a/java/com/google/gerrit/server/restapi/change/CherryPickChange.java b/java/com/google/gerrit/server/restapi/change/CherryPickChange.java
index ee6484c..b43585d 100644
--- a/java/com/google/gerrit/server/restapi/change/CherryPickChange.java
+++ b/java/com/google/gerrit/server/restapi/change/CherryPickChange.java
@@ -448,6 +448,9 @@
if (workInProgress != null) {
inserter.setWorkInProgress(workInProgress);
}
+ if (shouldSetToReady(cherryPickCommit, destNotes, workInProgress)) {
+ inserter.setWorkInProgress(false);
+ }
bu.addOp(destChange.getId(), inserter);
PatchSet.Id sourcePatchSetId = sourceChange == null ? null : sourceChange.currentPatchSetId();
// If sourceChange is not provided, reset cherryPickOf to avoid stale value.
@@ -461,6 +464,20 @@
return destChange.getId();
}
+ /**
+ * We should set the change to be "ready for review" if: 1. workInProgress is not already set on
+ * this request. 2. The patch-set doesn't have any git conflict markers. 3. The change used to be
+ * work in progress (because of a previous patch-set).
+ */
+ private boolean shouldSetToReady(
+ CodeReviewCommit cherryPickCommit,
+ ChangeNotes destChangeNotes,
+ @Nullable Boolean workInProgress) {
+ return workInProgress == null
+ && cherryPickCommit.getFilesWithGitConflicts().isEmpty()
+ && destChangeNotes.getChange().isWorkInProgress();
+ }
+
private Change.Id createNewChange(
BatchUpdate bu,
CodeReviewCommit cherryPickCommit,
diff --git a/java/com/google/gerrit/server/submit/LocalMergeSuperSetComputation.java b/java/com/google/gerrit/server/submit/LocalMergeSuperSetComputation.java
index afb6286..547c946 100644
--- a/java/com/google/gerrit/server/submit/LocalMergeSuperSetComputation.java
+++ b/java/com/google/gerrit/server/submit/LocalMergeSuperSetComputation.java
@@ -22,6 +22,7 @@
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.common.UsedAt;
import com.google.gerrit.entities.BranchNameKey;
import com.google.gerrit.entities.Project;
import com.google.gerrit.entities.SubmitTypeRecord;
@@ -170,7 +171,8 @@
return str.type;
}
- private ChangeSet byCommitsOnBranchNotMerged(
+ @UsedAt(UsedAt.Project.GOOGLE)
+ public ChangeSet byCommitsOnBranchNotMerged(
OpenRepo or,
BranchNameKey branch,
Set<String> visibleHashes,
@@ -185,7 +187,9 @@
ChangeIsVisibleToPredicate changeIsVisibleToPredicate =
changeIsVisibleToPredicateFactory.forUser(user);
for (ChangeData cd : potentiallyVisibleChanges) {
- if (changeIsVisibleToPredicate.match(cd)) {
+ // short circuit permission checks for non-private changes, as we already checked all
+ // permissions (except for private changes).
+ if (!cd.change().isPrivate() || changeIsVisibleToPredicate.match(cd)) {
visibleChanges.add(cd);
} else {
invisibleChanges.add(cd);
@@ -210,7 +214,8 @@
return result;
}
- private Set<String> walkChangesByHashes(
+ @UsedAt(UsedAt.Project.GOOGLE)
+ public Set<String> walkChangesByHashes(
Collection<RevCommit> sourceCommits, Set<String> ignoreHashes, OpenRepo or, BranchNameKey b)
throws IOException {
Set<String> destHashes = new HashSet<>();
diff --git a/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java b/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java
index 3d2d0bb..9d0b1f4 100644
--- a/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java
@@ -93,6 +93,7 @@
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
import com.google.gerrit.extensions.webui.PatchSetWebLink;
+import com.google.gerrit.extensions.webui.ResolveConflictsWebLink;
import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.testing.FakeEmailSender;
@@ -1198,6 +1199,23 @@
}
@Test
+ public void cherryPickSetsReadyChangeOnNewPatchset() throws Exception {
+ PushOneCommit.Result result = pushTo("refs/for/master");
+ CherryPickInput input = new CherryPickInput();
+ input.destination = "foo";
+ gApi.projects().name(project.get()).branch(input.destination).create(new BranchInput());
+ ChangeApi originalChange = gApi.changes().id(project.get() + "~master~" + result.getChangeId());
+
+ ChangeApi cherryPick = originalChange.revision(result.getCommit().name()).cherryPick(input);
+ cherryPick.setWorkInProgress();
+ cherryPick = originalChange.revision(result.getCommit().name()).cherryPick(input);
+
+ ChangeInfo secondCherryPickResult = cherryPick.get(ALL_REVISIONS);
+ assertThat(secondCherryPickResult.revisions).hasSize(2);
+ assertThat(secondCherryPickResult.workInProgress).isNull();
+ }
+
+ @Test
public void canRebase() throws Exception {
PushOneCommit push = pushFactory.create(admin.newIdent(), testRepo);
PushOneCommit.Result r1 = push.to("refs/for/master");
@@ -1616,16 +1634,26 @@
@Test
public void commit() throws Exception {
- WebLinkInfo expectedWebLinkInfo = new WebLinkInfo("foo", "imageUrl", "url");
- PatchSetWebLink link =
+ WebLinkInfo expectedPatchSetLinkInfo = new WebLinkInfo("foo", "imageUrl", "url");
+ PatchSetWebLink patchSetLink =
new PatchSetWebLink() {
@Override
public WebLinkInfo getPatchSetWebLink(
String projectName, String commit, String commitMessage, String branchName) {
- return expectedWebLinkInfo;
+ return expectedPatchSetLinkInfo;
}
};
- try (Registration registration = extensionRegistry.newRegistration().add(link)) {
+ WebLinkInfo expectedResolveConflictsLinkInfo = new WebLinkInfo("bar", "img", "resolve");
+ ResolveConflictsWebLink resolveConflictsLink =
+ new ResolveConflictsWebLink() {
+ @Override
+ public WebLinkInfo getResolveConflictsWebLink(
+ String projectName, String commit, String commitMessage, String branchName) {
+ return expectedResolveConflictsLinkInfo;
+ }
+ };
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(patchSetLink).add(resolveConflictsLink)) {
PushOneCommit.Result r = createChange();
RevCommit c = r.getCommit();
@@ -1642,11 +1670,20 @@
commitInfo = gApi.changes().id(r.getChangeId()).current().commit(true);
assertThat(commitInfo.webLinks).hasSize(1);
- WebLinkInfo webLinkInfo = Iterables.getOnlyElement(commitInfo.webLinks);
- assertThat(webLinkInfo.name).isEqualTo(expectedWebLinkInfo.name);
- assertThat(webLinkInfo.imageUrl).isEqualTo(expectedWebLinkInfo.imageUrl);
- assertThat(webLinkInfo.url).isEqualTo(expectedWebLinkInfo.url);
- assertThat(webLinkInfo.target).isEqualTo(expectedWebLinkInfo.target);
+ WebLinkInfo patchSetLinkInfo = Iterables.getOnlyElement(commitInfo.webLinks);
+ assertThat(patchSetLinkInfo.name).isEqualTo(expectedPatchSetLinkInfo.name);
+ assertThat(patchSetLinkInfo.imageUrl).isEqualTo(expectedPatchSetLinkInfo.imageUrl);
+ assertThat(patchSetLinkInfo.url).isEqualTo(expectedPatchSetLinkInfo.url);
+ assertThat(patchSetLinkInfo.target).isEqualTo(expectedPatchSetLinkInfo.target);
+
+ assertThat(commitInfo.resolveConflictsWebLinks).hasSize(1);
+ WebLinkInfo resolveCommentsLinkInfo =
+ Iterables.getOnlyElement(commitInfo.resolveConflictsWebLinks);
+ assertThat(resolveCommentsLinkInfo.name).isEqualTo(expectedResolveConflictsLinkInfo.name);
+ assertThat(resolveCommentsLinkInfo.imageUrl)
+ .isEqualTo(expectedResolveConflictsLinkInfo.imageUrl);
+ assertThat(resolveCommentsLinkInfo.url).isEqualTo(expectedResolveConflictsLinkInfo.url);
+ assertThat(resolveCommentsLinkInfo.target).isEqualTo(expectedResolveConflictsLinkInfo.target);
}
}
diff --git a/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java b/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java
index a76616a..45d1b76 100644
--- a/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java
+++ b/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java
@@ -2823,6 +2823,14 @@
r.assertErrorStatus("\"--unknown\" is not a valid option");
}
+ @Test
+ public void pushForMagicBranchWithSkipValidationOptionIsNotAllowed() throws Exception {
+ PushOneCommit push = pushFactory.create(admin.newIdent(), testRepo);
+ push.setPushOptions(ImmutableList.of(PUSH_OPTION_SKIP_VALIDATION));
+ PushOneCommit.Result r = push.to("refs/for/master");
+ r.assertErrorStatus("\"--skip-validation\" option is only supported for direct push");
+ }
+
private DraftInput newDraft(String path, int line, String message) {
DraftInput d = new DraftInput();
d.path = path;
diff --git a/javatests/com/google/gerrit/acceptance/rest/account/CreateAccountIT.java b/javatests/com/google/gerrit/acceptance/rest/account/CreateAccountIT.java
index aca6c4c..8d801f1 100644
--- a/javatests/com/google/gerrit/acceptance/rest/account/CreateAccountIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/account/CreateAccountIT.java
@@ -18,6 +18,7 @@
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.RestResponse;
+import com.google.gerrit.acceptance.config.GerritConfig;
import com.google.gerrit.extensions.api.accounts.AccountInput;
import org.junit.Test;
@@ -31,4 +32,44 @@
r.assertCreated();
assertThat(accountCache.getByUsername(input.username)).isPresent();
}
+
+ @Test
+ @GerritConfig(name = "auth.userNameToLowerCase", value = "false")
+ public void createAccountRestApiUserNameToLowerCaseFalse() throws Exception {
+ AccountInput input = new AccountInput();
+ input.username = "JohnDoe";
+ assertThat(accountCache.getByUsername(input.username)).isEmpty();
+ RestResponse r = adminRestSession.put("/accounts/" + input.username, input);
+ r.assertCreated();
+ assertThat(accountCache.getByUsername(input.username)).isPresent();
+ }
+
+ @Test
+ @GerritConfig(name = "auth.userNameToLowerCase", value = "true")
+ public void createAccountRestApiUserNameToLowerCaseTrue() throws Exception {
+ testUserNameToLowerCase("John1", "John1", "john1");
+ assertThat(accountCache.getByUsername("John1")).isEmpty();
+
+ testUserNameToLowerCase("john2", "John2", "john2");
+ assertThat(accountCache.getByUsername("John2")).isEmpty();
+
+ testUserNameToLowerCase("John3", "john3", "john3");
+ assertThat(accountCache.getByUsername("John3")).isEmpty();
+
+ testUserNameToLowerCase("John4", "johN4", "john4");
+ assertThat(accountCache.getByUsername("John4")).isEmpty();
+ assertThat(accountCache.getByUsername("johN4")).isEmpty();
+
+ testUserNameToLowerCase("john5", "john5", "john5");
+ }
+
+ private void testUserNameToLowerCase(String usernameUrl, String usernameInput, String usernameDb)
+ throws Exception {
+ AccountInput input = new AccountInput();
+ input.username = usernameInput;
+ assertThat(accountCache.getByUsername(usernameDb)).isEmpty();
+ RestResponse r = adminRestSession.put("/accounts/" + usernameUrl, input);
+ r.assertCreated();
+ assertThat(accountCache.getByUsername(usernameDb)).isPresent();
+ }
}
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/PredicateIT.java b/javatests/com/google/gerrit/acceptance/rest/change/PredicateIT.java
new file mode 100644
index 0000000..d893dc7
--- /dev/null
+++ b/javatests/com/google/gerrit/acceptance/rest/change/PredicateIT.java
@@ -0,0 +1,53 @@
+// 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.rest.change;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.gerrit.acceptance.AbstractPredicateTest;
+import com.google.gerrit.acceptance.RestResponse;
+import com.google.gerrit.entities.Change;
+import com.google.gerrit.extensions.common.PluginDefinedInfo;
+import com.google.gson.reflect.TypeToken;
+import java.util.List;
+import java.util.Map;
+import org.junit.Test;
+
+public class PredicateIT extends AbstractPredicateTest {
+
+ @Test
+ public void testLabelPredicate() throws Exception {
+ try (AutoCloseable ignored = installPlugin(PLUGIN_NAME, PluginModule.class)) {
+ Change.Id changeId = createChange().getChange().getId();
+ approve(String.valueOf(changeId.get()));
+ List<PluginDefinedInfo> myInfos =
+ pluginInfoFromSingletonList(
+ adminRestSession.get("/changes/?--my-plugin--sample&q=change:" + changeId.get()));
+
+ assertThat(myInfos).hasSize(1);
+ assertThat(myInfos.get(0).name).isEqualTo(PLUGIN_NAME);
+ assertThat(myInfos.get(0).message).isEqualTo("matched");
+ }
+ }
+
+ public List<PluginDefinedInfo> pluginInfoFromSingletonList(RestResponse res) throws Exception {
+ res.assertOK();
+ List<Map<String, Object>> changeInfos =
+ GSON.fromJson(res.getReader(), new TypeToken<List<Map<String, Object>>>() {}.getType());
+
+ assertThat(changeInfos).hasSize(1);
+ return decodeRawPluginsList(changeInfos.get(0).get("plugins"));
+ }
+}
diff --git a/javatests/com/google/gerrit/acceptance/ssh/PredicateIT.java b/javatests/com/google/gerrit/acceptance/ssh/PredicateIT.java
new file mode 100644
index 0000000..cedd270
--- /dev/null
+++ b/javatests/com/google/gerrit/acceptance/ssh/PredicateIT.java
@@ -0,0 +1,67 @@
+// 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.ssh;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.common.io.CharStreams;
+import com.google.gerrit.acceptance.AbstractPredicateTest;
+import com.google.gerrit.acceptance.NoHttpd;
+import com.google.gerrit.acceptance.UseSsh;
+import com.google.gerrit.entities.Change;
+import com.google.gerrit.extensions.common.PluginDefinedInfo;
+import com.google.gson.reflect.TypeToken;
+import java.io.StringReader;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import org.junit.Test;
+
+@NoHttpd
+@UseSsh
+public class PredicateIT extends AbstractPredicateTest {
+
+ @Test
+ public void testLabelPredicate() throws Exception {
+ try (AutoCloseable ignored = installPlugin(PLUGIN_NAME, PluginModule.class)) {
+ Change.Id changeId = createChange().getChange().getId();
+ approve(String.valueOf(changeId.get()));
+ String sshOutput =
+ adminSshSession.exec(
+ "gerrit query --format json --my-plugin--sample change:" + changeId.get());
+ adminSshSession.assertSuccess();
+ List<PluginDefinedInfo> myInfos = pluginInfoFromSingletonList(sshOutput);
+
+ assertThat(myInfos).hasSize(1);
+ assertThat(myInfos.get(0).name).isEqualTo(PLUGIN_NAME);
+ assertThat(myInfos.get(0).message).isEqualTo("matched");
+ }
+ }
+
+ private static List<PluginDefinedInfo> pluginInfoFromSingletonList(String sshOutput)
+ throws Exception {
+ List<Map<String, Object>> changeAttrs = new ArrayList<>();
+ for (String line : CharStreams.readLines(new StringReader(sshOutput))) {
+ Map<String, Object> changeAttr =
+ GSON.fromJson(line, new TypeToken<Map<String, Object>>() {}.getType());
+ if (!"stats".equals(changeAttr.get("type"))) {
+ changeAttrs.add(changeAttr);
+ }
+ }
+
+ assertThat(changeAttrs).hasSize(1);
+ return decodeRawPluginsList(changeAttrs.get(0).get("plugins"));
+ }
+}
diff --git a/javatests/com/google/gerrit/httpd/BUILD b/javatests/com/google/gerrit/httpd/BUILD
index d751890..121cbc4 100644
--- a/javatests/com/google/gerrit/httpd/BUILD
+++ b/javatests/com/google/gerrit/httpd/BUILD
@@ -4,6 +4,7 @@
name = "httpd_tests",
srcs = glob(["**/*.java"]),
deps = [
+ "//java/com/google/gerrit/entities",
"//java/com/google/gerrit/extensions:api",
"//java/com/google/gerrit/httpd",
"//java/com/google/gerrit/server",
@@ -17,6 +18,7 @@
"//lib:junit",
"//lib:servlet-api-without-neverlink",
"//lib:soy",
+ "//lib/bouncycastle:bcprov",
"//lib/guice",
"//lib/guice:guice-servlet",
"//lib/mockito",
diff --git a/javatests/com/google/gerrit/httpd/ProjectBasicAuthFilterTest.java b/javatests/com/google/gerrit/httpd/ProjectBasicAuthFilterTest.java
new file mode 100644
index 0000000..735abbf
--- /dev/null
+++ b/javatests/com/google/gerrit/httpd/ProjectBasicAuthFilterTest.java
@@ -0,0 +1,266 @@
+// 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.httpd;
+
+import static com.google.common.truth.Truth.assertThat;
+import static org.mockito.Mockito.any;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.doThrow;
+import static org.mockito.Mockito.eq;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
+
+import com.google.common.collect.ImmutableSet;
+import com.google.gerrit.entities.Account;
+import com.google.gerrit.extensions.client.GitBasicAuthPolicy;
+import com.google.gerrit.extensions.registration.DynamicItem;
+import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.account.AccountCache;
+import com.google.gerrit.server.account.AccountException;
+import com.google.gerrit.server.account.AccountManager;
+import com.google.gerrit.server.account.AccountState;
+import com.google.gerrit.server.account.AuthResult;
+import com.google.gerrit.server.account.externalids.ExternalId;
+import com.google.gerrit.server.config.AuthConfig;
+import com.google.gerrit.util.http.testutil.FakeHttpServletRequest;
+import com.google.gerrit.util.http.testutil.FakeHttpServletResponse;
+import java.nio.charset.StandardCharsets;
+import java.util.Base64;
+import java.util.Optional;
+import javax.servlet.FilterChain;
+import javax.servlet.http.HttpServletResponse;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.ArgumentCaptor;
+import org.mockito.Captor;
+import org.mockito.Mock;
+import org.mockito.junit.MockitoJUnitRunner;
+
+@RunWith(MockitoJUnitRunner.class)
+public class ProjectBasicAuthFilterTest {
+ private static final Base64.Encoder B64_ENC = Base64.getEncoder();
+ private static final Account.Id AUTH_ACCOUNT_ID = Account.id(1000);
+ private static final String AUTH_USER = "johndoe";
+ private static final String AUTH_USER_B64 =
+ B64_ENC.encodeToString(AUTH_USER.getBytes(StandardCharsets.UTF_8));
+ private static final String AUTH_PASSWORD = "jd123";
+ private static final String GERRIT_COOKIE_KEY = "GerritAccount";
+ private static final String AUTH_COOKIE_VALUE = "gerritcookie";
+ private static final ExternalId AUTH_USER_PASSWORD_EXTERNAL_ID =
+ ExternalId.createWithPassword(
+ ExternalId.Key.create(ExternalId.SCHEME_USERNAME, AUTH_USER),
+ AUTH_ACCOUNT_ID,
+ null,
+ AUTH_PASSWORD);
+
+ @Mock private DynamicItem<WebSession> webSessionItem;
+
+ @Mock private AccountCache accountCache;
+
+ @Mock private AccountState accountState;
+
+ @Mock private Account account;
+
+ @Mock private AccountManager accountManager;
+
+ @Mock private AuthConfig authConfig;
+
+ @Mock private FilterChain chain;
+
+ @Captor private ArgumentCaptor<HttpServletResponse> filterResponseCaptor;
+
+ @Mock private IdentifiedUser.RequestFactory userRequestFactory;
+
+ @Mock private WebSessionManager webSessionManager;
+
+ private WebSession webSession;
+ private FakeHttpServletRequest req;
+ private HttpServletResponse res;
+ private AuthResult authSuccessful;
+
+ @Before
+ public void setUp() throws Exception {
+ req = new FakeHttpServletRequest();
+ res = new FakeHttpServletResponse();
+
+ authSuccessful =
+ new AuthResult(AUTH_ACCOUNT_ID, ExternalId.Key.create("username", AUTH_USER), false);
+ doReturn(Optional.of(accountState)).when(accountCache).getByUsername(AUTH_USER);
+ doReturn(Optional.of(accountState)).when(accountCache).get(AUTH_ACCOUNT_ID);
+ doReturn(account).when(accountState).account();
+ doReturn(ImmutableSet.builder().add(AUTH_USER_PASSWORD_EXTERNAL_ID).build())
+ .when(accountState)
+ .externalIds();
+
+ doReturn(new WebSessionManager.Key(AUTH_COOKIE_VALUE)).when(webSessionManager).createKey(any());
+ WebSessionManager.Val webSessionValue =
+ new WebSessionManager.Val(AUTH_ACCOUNT_ID, 0L, false, null, 0L, "", "");
+ doReturn(webSessionValue)
+ .when(webSessionManager)
+ .createVal(any(), any(), eq(false), any(), any(), any());
+ }
+
+ private void initWebSessionWithCookie(String cookie) {
+ req.addHeader("Cookie", cookie);
+ initWebSessionWithoutCookie();
+ }
+
+ private void initWebSessionWithoutCookie() {
+ webSession =
+ new CacheBasedWebSession(
+ req, res, webSessionManager, authConfig, null, userRequestFactory, accountCache) {};
+ doReturn(webSession).when(webSessionItem).get();
+ }
+
+ private void initMockedWebSession() {
+ webSession = mock(WebSession.class);
+ doReturn(webSession).when(webSessionItem).get();
+ }
+
+ @Test
+ public void shouldAllowAnonymousRequest() throws Exception {
+ initMockedWebSession();
+ res.setStatus(HttpServletResponse.SC_OK);
+
+ ProjectBasicAuthFilter basicAuthFilter =
+ new ProjectBasicAuthFilter(webSessionItem, accountCache, accountManager, authConfig);
+
+ basicAuthFilter.doFilter(req, res, chain);
+
+ verify(chain).doFilter(eq(req), filterResponseCaptor.capture());
+ assertThat(res.getStatus()).isEqualTo(HttpServletResponse.SC_OK);
+ }
+
+ @Test
+ public void shouldRequestAuthenticationForBasicAuthRequest() throws Exception {
+ initMockedWebSession();
+ req.addHeader("Authorization", "Basic " + AUTH_USER_B64);
+ res.setStatus(HttpServletResponse.SC_OK);
+
+ ProjectBasicAuthFilter basicAuthFilter =
+ new ProjectBasicAuthFilter(webSessionItem, accountCache, accountManager, authConfig);
+
+ basicAuthFilter.doFilter(req, res, chain);
+
+ verify(chain, never()).doFilter(any(), any());
+ assertThat(res.getStatus()).isEqualTo(HttpServletResponse.SC_UNAUTHORIZED);
+ assertThat(res.getHeader("WWW-Authenticate")).contains("Basic realm=");
+ }
+
+ @Test
+ public void shouldAuthenticateSucessfullyAgainstRealmAndReturnCookie() throws Exception {
+ initWebSessionWithoutCookie();
+ requestBasicAuth(req);
+ res.setStatus(HttpServletResponse.SC_OK);
+
+ doReturn(true).when(account).isActive();
+ doReturn(authSuccessful).when(accountManager).authenticate(any());
+ doReturn(GitBasicAuthPolicy.LDAP).when(authConfig).getGitBasicAuthPolicy();
+
+ ProjectBasicAuthFilter basicAuthFilter =
+ new ProjectBasicAuthFilter(webSessionItem, accountCache, accountManager, authConfig);
+
+ basicAuthFilter.doFilter(req, res, chain);
+
+ verify(accountManager).authenticate(any());
+
+ verify(chain).doFilter(eq(req), any());
+ assertThat(res.getStatus()).isEqualTo(HttpServletResponse.SC_OK);
+ assertThat(res.getHeader("Set-Cookie")).contains(GERRIT_COOKIE_KEY);
+ }
+
+ @Test
+ public void shouldValidateUserPasswordAndNotReturnCookie() throws Exception {
+ initWebSessionWithoutCookie();
+ requestBasicAuth(req);
+ res.setStatus(HttpServletResponse.SC_OK);
+
+ doReturn(true).when(account).isActive();
+ doReturn(GitBasicAuthPolicy.HTTP).when(authConfig).getGitBasicAuthPolicy();
+
+ ProjectBasicAuthFilter basicAuthFilter =
+ new ProjectBasicAuthFilter(webSessionItem, accountCache, accountManager, authConfig);
+
+ basicAuthFilter.doFilter(req, res, chain);
+
+ verify(accountManager, never()).authenticate(any());
+
+ verify(chain).doFilter(eq(req), any());
+ assertThat(res.getStatus()).isEqualTo(HttpServletResponse.SC_OK);
+ assertThat(res.getHeader("Set-Cookie")).isNull();
+ }
+
+ @Test
+ public void shouldNotReauthenticateIfAlreadySignedIn() throws Exception {
+ initMockedWebSession();
+ doReturn(true).when(webSession).isSignedIn();
+ requestBasicAuth(req);
+ res.setStatus(HttpServletResponse.SC_OK);
+
+ ProjectBasicAuthFilter basicAuthFilter =
+ new ProjectBasicAuthFilter(webSessionItem, accountCache, accountManager, authConfig);
+
+ basicAuthFilter.doFilter(req, res, chain);
+
+ verify(accountManager, never()).authenticate(any());
+ verify(chain).doFilter(eq(req), any());
+ assertThat(res.getStatus()).isEqualTo(HttpServletResponse.SC_OK);
+ }
+
+ @Test
+ public void shouldNotReauthenticateIfHasExistingCookie() throws Exception {
+ initWebSessionWithCookie("GerritAccount=" + AUTH_COOKIE_VALUE);
+ res.setStatus(HttpServletResponse.SC_OK);
+
+ ProjectBasicAuthFilter basicAuthFilter =
+ new ProjectBasicAuthFilter(webSessionItem, accountCache, accountManager, authConfig);
+
+ basicAuthFilter.doFilter(req, res, chain);
+
+ verify(accountManager, never()).authenticate(any());
+ verify(chain).doFilter(eq(req), any());
+ assertThat(res.getStatus()).isEqualTo(HttpServletResponse.SC_OK);
+ }
+
+ @Test
+ public void shouldFailedAuthenticationAgainstRealm() throws Exception {
+ initMockedWebSession();
+ requestBasicAuth(req);
+
+ doReturn(true).when(account).isActive();
+ doThrow(new AccountException("Authentication error")).when(accountManager).authenticate(any());
+ doReturn(GitBasicAuthPolicy.LDAP).when(authConfig).getGitBasicAuthPolicy();
+
+ ProjectBasicAuthFilter basicAuthFilter =
+ new ProjectBasicAuthFilter(webSessionItem, accountCache, accountManager, authConfig);
+
+ basicAuthFilter.doFilter(req, res, chain);
+
+ verify(accountManager).authenticate(any());
+
+ verify(chain, never()).doFilter(any(), any());
+ assertThat(res.getStatus()).isEqualTo(HttpServletResponse.SC_UNAUTHORIZED);
+ }
+
+ private void requestBasicAuth(FakeHttpServletRequest fakeReq) {
+ fakeReq.addHeader(
+ "Authorization",
+ "Basic "
+ + B64_ENC.encodeToString(
+ (AUTH_USER + ":" + AUTH_PASSWORD).getBytes(StandardCharsets.UTF_8)));
+ }
+}
diff --git a/javatests/com/google/gerrit/util/http/testutil/FakeHttpServletRequest.java b/javatests/com/google/gerrit/util/http/testutil/FakeHttpServletRequest.java
index a4175e3..2efa94b 100644
--- a/javatests/com/google/gerrit/util/http/testutil/FakeHttpServletRequest.java
+++ b/javatests/com/google/gerrit/util/http/testutil/FakeHttpServletRequest.java
@@ -17,6 +17,7 @@
import static com.google.common.base.Preconditions.checkArgument;
import static java.nio.charset.StandardCharsets.UTF_8;
import static java.util.Objects.requireNonNull;
+import static java.util.stream.Collectors.toList;
import com.google.common.base.Splitter;
import com.google.common.base.Strings;
@@ -257,7 +258,15 @@
@Override
public Cookie[] getCookies() {
- return new Cookie[0];
+ return Splitter.on(";").splitToList(Strings.nullToEmpty(getHeader("Cookie"))).stream()
+ .filter(s -> !s.isEmpty())
+ .map(
+ (String cookieValue) -> {
+ String[] kv = cookieValue.split("=");
+ return new Cookie(kv[0], kv[1]);
+ })
+ .collect(toList())
+ .toArray(new Cookie[0]);
}
@Override
diff --git a/javatests/com/google/gerrit/util/http/testutil/FakeHttpServletResponse.java b/javatests/com/google/gerrit/util/http/testutil/FakeHttpServletResponse.java
index 9a98ecd..f39b875 100644
--- a/javatests/com/google/gerrit/util/http/testutil/FakeHttpServletResponse.java
+++ b/javatests/com/google/gerrit/util/http/testutil/FakeHttpServletResponse.java
@@ -161,7 +161,7 @@
@Override
public void addCookie(Cookie cookie) {
- throw new UnsupportedOperationException();
+ addHeader("Set-Cookie", cookie.getName() + "=" + cookie.getValue());
}
@Override
diff --git a/modules/jgit b/modules/jgit
index c82818e..a9579ba 160000
--- a/modules/jgit
+++ b/modules/jgit
@@ -1 +1 @@
-Subproject commit c82818e0e02a9d1bd979d27bd342bb05661150d4
+Subproject commit a9579ba60cd2fd72179dfd8c2c37d389db5ec402
diff --git a/plugins/replication b/plugins/replication
index 0022a34..13cefb7 160000
--- a/plugins/replication
+++ b/plugins/replication
@@ -1 +1 @@
-Subproject commit 0022a34428cf8bfe4feb0935cdd20b0257bfc8a3
+Subproject commit 13cefb724df786d254ecbc24261589ab473be267
diff --git a/polygerrit-ui/app/api/checks.ts b/polygerrit-ui/app/api/checks.ts
index 62b9ad9..5d20f3f 100644
--- a/polygerrit-ui/app/api/checks.ts
+++ b/polygerrit-ui/app/api/checks.ts
@@ -298,6 +298,11 @@
externalId?: string;
/**
+ * SUCCESS: Indicates that some build, test or check is passing. A COMPLETED
+ * run without results will also be treated as "passing" and will get
+ * an artificial SUCCESS result. But you can also make this explicit,
+ * which also allows one run to have multiple "passing" results,
+ * maybe along with results of other categories.
* INFO: The user will typically not bother to look into this category,
* only for looking up something that they are searching for. Can be
* used for reporting secondary metrics and analysis, or a wider
@@ -383,6 +388,7 @@
}
export enum Category {
+ SUCCESS = 'SUCCESS',
INFO = 'INFO',
WARNING = 'WARNING',
ERROR = 'ERROR',
diff --git a/polygerrit-ui/app/api/diff.ts b/polygerrit-ui/app/api/diff.ts
index 6e142d4..f6b3aa0 100644
--- a/polygerrit-ui/app/api/diff.ts
+++ b/polygerrit-ui/app/api/diff.ts
@@ -303,12 +303,8 @@
}
export declare type ImageDiffAction =
- | {
- type: 'overview-image-clicked';
- }
- | {
- type: 'overview-frame-dragged';
- }
+ | {type: 'overview-image-clicked'}
+ | {type: 'overview-frame-dragged'}
| {type: 'magnifier-clicked'}
| {type: 'magnifier-dragged'}
| {type: 'version-switcher-clicked'; button: 'base' | 'revision' | 'switch'}
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 4f60823..c6d2ee0 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
@@ -1220,6 +1220,14 @@
return;
}
+ // Everything in the change view is tied to the change. It seems better to
+ // force the re-creation of the change view when the change number changes.
+ const changeChanged = this._changeNum !== value.changeNum;
+ if (this._changeNum !== undefined && changeChanged) {
+ fireEvent(this, EventType.RECREATE_CHANGE_VIEW);
+ return;
+ }
+
if (value.changeNum && value.project) {
this.restApiService.setInProjectLookup(value.changeNum, value.project);
}
@@ -1230,7 +1238,6 @@
value.basePatchNum !== undefined &&
(this._patchRange.patchNum !== value.patchNum ||
this._patchRange.basePatchNum !== value.basePatchNum);
- const changeChanged = this._changeNum !== value.changeNum;
let rightPatchNumChanged =
this._patchRange &&
@@ -1245,9 +1252,13 @@
this.$.fileList.collapseAllDiffs();
this._patchRange = patchRange;
+ const patchKnown =
+ !patchRange.patchNum ||
+ (this._allPatchSets ?? []).some(ps => ps.num === patchRange.patchNum);
+
// If the change has already been loaded and the parameter change is only
// in the patch range, then don't do a full reload.
- if (!changeChanged && patchChanged) {
+ if (!changeChanged && patchChanged && patchKnown) {
if (!patchRange.patchNum) {
patchRange.patchNum = computeLatestPatchNum(this._allPatchSets);
rightPatchNumChanged = true;
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 06c3ded..3d51ac6 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
@@ -380,7 +380,7 @@
stubRestApi('getDiffRobotComments').returns(Promise.resolve({}));
stubRestApi('getDiffDrafts').returns(Promise.resolve({}));
element = fixture.instantiate();
- element._changeNum = 1 as NumericChangeId;
+ element._changeNum = TEST_NUMERIC_CHANGE_ID;
sinon.stub(element.$.actions, 'reload').returns(Promise.resolve());
getPluginLoader().loadPlugins([]);
pluginApi.install(
@@ -518,7 +518,7 @@
suite('plugins adding to file tab', () => {
setup(done => {
- element._changeNum = 1 as NumericChangeId;
+ element._changeNum = TEST_NUMERIC_CHANGE_ID;
// Resolving it here instead of during setup() as other tests depend
// on flush() not being called during setup.
flush(() => done());
@@ -1633,6 +1633,13 @@
assert.isTrue(reloadStub.calledOnce);
element._initialLoadComplete = true;
+ element._change = {
+ ...createChangeViewChange(),
+ revisions: {
+ rev1: createRevision(1),
+ rev2: createRevision(2),
+ },
+ };
value.basePatchNum = 1 as BasePatchSetNum;
value.patchNum = 2 as RevisionPatchSetNum;
@@ -1661,6 +1668,13 @@
element._paramsChanged(value);
element._initialLoadComplete = true;
+ element._change = {
+ ...createChangeViewChange(),
+ revisions: {
+ rev1: createRevision(1),
+ rev2: createRevision(2),
+ },
+ };
value.basePatchNum = 1 as BasePatchSetNum;
value.patchNum = 2 as RevisionPatchSetNum;
@@ -2530,23 +2544,6 @@
});
});
- test('_paramsChanged sets in projectLookup', () => {
- flush();
- const relatedChanges = element.shadowRoot!.querySelector(
- '#relatedChanges'
- ) as GrRelatedChangesList;
- sinon.stub(relatedChanges, 'reload');
- sinon.stub(element, 'loadData').returns(Promise.resolve([]));
- const setStub = stubRestApi('setInProjectLookup');
- element._paramsChanged({
- view: GerritNav.View.CHANGE,
- changeNum: 101 as NumericChangeId,
- project: TEST_PROJECT_NAME,
- });
- assert.isTrue(setStub.calledOnce);
- assert.isTrue(setStub.calledWith(101 as never, TEST_PROJECT_NAME as never));
- });
-
test('_handleToggleStar called when star is tapped', () => {
element._change = {
...createChangeViewChange(),
@@ -2605,7 +2602,7 @@
);
element._paramsChanged({
...createAppElementChangeViewParams(),
- changeNum: 101 as NumericChangeId,
+ changeNum: TEST_NUMERIC_CHANGE_ID,
project: TEST_PROJECT_NAME,
});
flush(() => {
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 26af2a2..1711499 100644
--- a/polygerrit-ui/app/elements/change/gr-message/gr-message.ts
+++ b/polygerrit-ui/app/elements/change/gr-message/gr-message.ts
@@ -184,7 +184,7 @@
type: String,
computed:
'_computeMessageContentExpanded(message.message,' +
- ' message.accountsInMessage,' +
+ ' message.accounts_in_message,' +
' message.tag)',
})
_messageContentExpanded = '';
@@ -193,7 +193,7 @@
type: String,
computed:
'_computeMessageContentCollapsed(message.message,' +
- ' message.accountsInMessage,' +
+ ' message.accounts_in_message,' +
' message.tag,' +
' message.commentThreads)',
})
diff --git a/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.ts b/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.ts
index 8d5bc33..5595d15 100644
--- a/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.ts
+++ b/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.ts
@@ -21,13 +21,7 @@
import '../../plugins/gr-endpoint-slot/gr-endpoint-slot';
import {classMap} from 'lit-html/directives/class-map';
import {GrLitElement} from '../../lit/gr-lit-element';
-import {
- customElement,
- property,
- css,
- internalProperty,
- TemplateResult,
-} from 'lit-element';
+import {customElement, property, css, state, TemplateResult} from 'lit-element';
import {sharedStyles} from '../../../styles/shared-styles';
import {
SubmittedTogetherInfo,
@@ -76,22 +70,22 @@
@property()
mergeable?: boolean;
- @internalProperty()
+ @state()
submittedTogether?: SubmittedTogetherInfo = {
changes: [],
non_visible_changes: 0,
};
- @internalProperty()
+ @state()
relatedChanges: RelatedChangeAndCommitInfo[] = [];
- @internalProperty()
+ @state()
conflictingChanges: ChangeInfo[] = [];
- @internalProperty()
+ @state()
cherryPickChanges: ChangeInfo[] = [];
- @internalProperty()
+ @state()
sameTopicChanges: ChangeInfo[] = [];
private readonly restApiService = appContext.restApiService;
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog-it_test.js b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog-it_test.js
index 5f35fd3..2767940 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog-it_test.js
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog-it_test.js
@@ -87,9 +87,6 @@
test('_submit blocked when invalid email is supplied to ccs', () => {
const sendStub = sinon.stub(element, 'send').returns(Promise.resolve());
- // Stub the below function to avoid side effects from the send promise
- // resolving.
- sinon.stub(element, '_purgeReviewersPendingRemove');
element.$.ccs.$.entry.setText('test');
MockInteractions.tap(element.shadowRoot
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts
index 05340c8..e303dad 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts
@@ -39,7 +39,11 @@
SpecialFilePath,
} from '../../../constants/constants';
import {KeyboardShortcutMixin} from '../../../mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin';
-import {accountKey, removeServiceUsers} from '../../../utils/account-util';
+import {
+ accountKey,
+ accountOrGroupKey,
+ removeServiceUsers,
+} from '../../../utils/account-util';
import {getDisplayName} from '../../../utils/display-name-util';
import {IronA11yAnnouncer} from '@polymer/iron-a11y-announcer/iron-a11y-announcer';
import {TargetElement} from '../../../api/plugin';
@@ -64,7 +68,6 @@
GroupInfo,
isAccount,
isDetailedLabelInfo,
- isGroup,
isReviewerAccountSuggestion,
isReviewerGroupSuggestion,
LabelNameToValueMap,
@@ -83,13 +86,11 @@
import {GrLabelScoreRow} from '../gr-label-score-row/gr-label-score-row';
import {
PolymerDeepPropertyChange,
- PolymerSplice,
PolymerSpliceChange,
} from '@polymer/polymer/interfaces';
import {
areSetsEqual,
assertIsDefined,
- assertNever,
containsAll,
} from '../../../utils/common-util';
import {CommentThread, isUnresolved} from '../../../utils/comment-util';
@@ -148,15 +149,6 @@
const EMPTY_REPLY_MESSAGE = 'Cannot send an empty reply.';
-interface PendingRemovals {
- CC: (AccountInfoInput | GroupInfoInput)[];
- REVIEWER: (AccountInfoInput | GroupInfoInput)[];
-}
-const PENDING_REMOVAL_KEYS: (keyof PendingRemovals)[] = [
- ReviewerType.CC,
- ReviewerType.REVIEWER,
-];
-
export interface GrReplyDialog {
$: {
reviewers: GrAccountList;
@@ -311,12 +303,6 @@
@property({type: Boolean, observer: '_handleHeightChanged'})
_previewFormatting = false;
- @property({type: Object})
- _reviewersPendingRemove: PendingRemovals = {
- CC: [],
- REVIEWER: [],
- };
-
@property({type: String, computed: '_computeSendButtonLabel(canBeStarted)'})
_sendButtonLabel?: string;
@@ -529,7 +515,6 @@
) {
if (splices && splices.indexSplices) {
this._reviewersMutated = true;
- this._processReviewerChange(splices.indexSplices, reviewerType);
let key: AccountId | EmailAddress | GroupId | undefined;
let index;
let account;
@@ -539,10 +524,10 @@
for (const splice of splices.indexSplices) {
for (let i = 0; i < splice.addedCount; i++) {
account = splice.object[splice.index + i];
- key = this._accountOrGroupKey(account);
+ key = accountOrGroupKey(account);
const array = isReviewer ? this._ccs : this._reviewers;
index = array.findIndex(
- account => this._accountOrGroupKey(account) === key
+ account => accountOrGroupKey(account) === key
);
if (index >= 0) {
this.splice(isReviewer ? '_ccs' : '_reviewers', index, 1);
@@ -557,76 +542,6 @@
}
}
- _processReviewerChange(
- indexSplices: Array<PolymerSplice<AccountInfo[]>>,
- type: ReviewerType
- ) {
- for (const splice of indexSplices) {
- for (const account of splice.removed) {
- if (!this._reviewersPendingRemove[type]) {
- this.reporting.error(new Error(`Invalid type ${type} for reviewer.`));
- return;
- }
- this._reviewersPendingRemove[type].push(account);
- }
- }
- }
-
- /**
- * Resets the state of the _reviewersPendingRemove object, and removes
- * accounts if necessary.
- *
- * @param isCancel true if the action is a cancel.
- * @param keep map of account IDs that must
- * not be removed, because they have been readded in another state.
- */
- _purgeReviewersPendingRemove(
- isCancel: boolean,
- keep = new Map<AccountId | EmailAddress, boolean>()
- ) {
- let reviewerArr: (AccountInfoInput | GroupInfoInput)[];
- for (const type of PENDING_REMOVAL_KEYS) {
- if (!isCancel) {
- reviewerArr = this._reviewersPendingRemove[type];
- for (let i = 0; i < reviewerArr.length; i++) {
- const reviewer = reviewerArr[i];
- if (!isAccount(reviewer) || !keep.get(accountKey(reviewer))) {
- this._removeAccount(reviewer, type as ReviewerType);
- }
- }
- }
- this._reviewersPendingRemove[type] = [];
- }
- }
-
- /**
- * Removes an account from the change, both on the backend and the client.
- * Does nothing if the account is a pending addition.
- */
- _removeAccount(
- account: AccountInfoInput | GroupInfoInput,
- type: ReviewerType
- ) {
- assertIsDefined(this.change, 'change');
- if (account._pendingAdd || !isAccount(account)) {
- return;
- }
-
- return this.restApiService
- .removeChangeReviewer(this.change._number, accountKey(account))
- .then((response?: Response) => {
- if (!response?.ok || !this.change) return;
-
- const reviewers = this.change.reviewers[type] || [];
- for (let i = 0; i < reviewers.length; i++) {
- if (reviewers[i]._account_id === account._account_id) {
- this.splice(`change.reviewers.${type}`, i, 1);
- break;
- }
- }
- });
- }
-
_mapReviewer(addition: AccountAddition): ReviewerInput {
if (addition.account) {
return {reviewer: accountKey(addition.account)};
@@ -639,10 +554,7 @@
throw new Error('Reviewer must be either an account or a group.');
}
- send(
- includeComments: boolean,
- startReview: boolean
- ): Promise<Map<AccountId | EmailAddress, boolean>> {
+ send(includeComments: boolean, startReview: boolean) {
this.reporting.time(Timing.SEND_REPLY);
const labels = this.$.labelScores.getLabelValues();
@@ -689,24 +601,21 @@
};
}
- const accountAdditions = new Map<AccountId | EmailAddress, boolean>();
- reviewInput.reviewers = this.$.reviewers.additions().map(reviewer => {
- if (reviewer.account) {
- accountAdditions.set(accountKey(reviewer.account), true);
- }
- return this._mapReviewer(reviewer);
- });
- const ccsEl = this.$.ccs;
- if (ccsEl) {
- for (const addition of ccsEl.additions()) {
- if (addition.account) {
- accountAdditions.set(accountKey(addition.account), true);
- }
+ const addToReviewInput = (
+ additions: AccountAddition[],
+ state?: ReviewerState
+ ) => {
+ additions.forEach(addition => {
const reviewer = this._mapReviewer(addition);
- reviewer.state = ReviewerState.CC;
- reviewInput.reviewers.push(reviewer);
- }
- }
+ if (state) reviewer.state = state;
+ reviewInput.reviewers?.push(reviewer);
+ });
+ };
+ reviewInput.reviewers = [];
+ addToReviewInput(this.$.reviewers.additions(), ReviewerState.REVIEWER);
+ addToReviewInput(this.$.ccs.additions(), ReviewerState.CC);
+ addToReviewInput(this.$.reviewers.removals(), ReviewerState.REMOVED);
+ addToReviewInput(this.$.ccs.removals(), ReviewerState.REMOVED);
this.disabled = true;
@@ -716,11 +625,11 @@
if (!response) {
// Null or undefined response indicates that an error handler
// took responsibility, so just return.
- return new Map<AccountId | EmailAddress, boolean>();
+ return;
}
if (!response.ok) {
fireServerError(response);
- return new Map<AccountId | EmailAddress, boolean>();
+ return;
}
this.draft = '';
@@ -732,7 +641,7 @@
})
);
fireIronAnnounce(this, 'Reply sent');
- return accountAdditions;
+ return;
})
.then(result => {
this.disabled = false;
@@ -1162,12 +1071,6 @@
return rev.uploader;
}
- _accountOrGroupKey(entry: AccountInfo | GroupInfo) {
- if (isAccount(entry)) return accountKey(entry);
- if (isGroup(entry)) return entry.id;
- assertNever(entry, 'entry must be account or group');
- }
-
/**
* Generates a function to filter out reviewer/CC entries. When isCCs is
* truthy, the function filters out entries that already exist in this._ccs.
@@ -1193,9 +1096,9 @@
return false;
}
- const key = this._accountOrGroupKey(entry);
+ const key = accountOrGroupKey(entry);
const finder = (entry: AccountInfo | GroupInfo) =>
- this._accountOrGroupKey(entry) === key;
+ accountOrGroupKey(entry) === key;
if (isCCs) {
return this._ccs.find(finder) === undefined;
}
@@ -1222,7 +1125,7 @@
})
);
this.$.textarea.closeDropdown();
- this._purgeReviewersPendingRemove(true);
+ this.$.reviewers.clearPendingRemovals();
this._rebuildReviewerArrays(this.change.reviewers, this._owner);
}
@@ -1233,9 +1136,7 @@
// the text field of the CC entry.
return;
}
- this.send(this._includeComments, false).then(keepReviewers => {
- this._purgeReviewersPendingRemove(false, keepReviewers);
- });
+ this.send(this._includeComments, false);
}
_sendTapHandler(e: Event) {
@@ -1253,19 +1154,15 @@
fireAlert(this, EMPTY_REPLY_MESSAGE);
return;
}
- return this.send(this._includeComments, this.canBeStarted)
- .then(keepReviewers => {
- this._purgeReviewersPendingRemove(false, keepReviewers);
- })
- .catch(err => {
- this.dispatchEvent(
- new CustomEvent('show-error', {
- bubbles: true,
- composed: true,
- detail: {message: `Error submitting review ${err}`},
- })
- );
- });
+ return this.send(this._includeComments, this.canBeStarted).catch(err => {
+ this.dispatchEvent(
+ new CustomEvent('show-error', {
+ bubbles: true,
+ composed: true,
+ detail: {message: `Error submitting review ${err}`},
+ })
+ );
+ });
}
_saveReview(review: ReviewInput, errFn?: ErrorCallback) {
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.js b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.js
index 4a0e8d2..b51eb5f 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.js
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.js
@@ -19,7 +19,7 @@
import {IronOverlayManager} from '@polymer/iron-overlay-behavior/iron-overlay-manager.js';
import './gr-reply-dialog.js';
import {mockPromise, stubStorage} from '../../../test/test-utils.js';
-import {SpecialFilePath} from '../../../constants/constants.js';
+import {ReviewerState, SpecialFilePath} from '../../../constants/constants.js';
import {appContext} from '../../../services/app-context.js';
import {addListenerForTest} from '../../../test/test-utils.js';
import {stubRestApi} from '../../../test/test-utils.js';
@@ -57,7 +57,7 @@
let setDraftCommentStub;
let eraseDraftCommentStub;
- let lastId = 0;
+ let lastId = 1;
const makeAccount = function() { return {_account_id: lastId++}; };
const makeGroup = function() { return {id: lastId++}; };
@@ -203,17 +203,17 @@
});
function checkComputeAttention(status, userId, reviewerIds, ownerId,
- attSetIds, replyToIds, expectedIds, uploaderId, hasDraft,
+ attSetIds, replyToIds, expectedIds, uploaderId, hasDraft = true,
includeComments = true) {
const user = {_account_id: userId};
const reviewers = {base: reviewerIds.map(id => {
return {_account_id: id};
})};
- const draftThreads = [
- {comments: []},
- ];
+ let draftThreads = [];
if (hasDraft) {
- draftThreads[0].comments.push({__draft: true, unresolved: true});
+ draftThreads = [
+ {comments: [{__draft: true, unresolved: true}]},
+ ];
}
replyToIds.forEach(id => draftThreads[0].comments.push({
author: {_account_id: id},
@@ -240,7 +240,6 @@
}
test('computeNewAttention NEW', () => {
- checkComputeAttention('NEW', null, [], 999, [], [], [999]);
checkComputeAttention('NEW', 1, [], 999, [], [], [999]);
checkComputeAttention('NEW', 1, [], 999, [1], [], [999]);
checkComputeAttention('NEW', 1, [22], 999, [], [], [999]);
@@ -264,14 +263,14 @@
});
test('computeNewAttention MERGED', () => {
- checkComputeAttention('MERGED', null, [], 999, [], [], []);
- checkComputeAttention('MERGED', 1, [], 999, [], [], []);
- checkComputeAttention('MERGED', 1, [], 999, [], [], [999], undefined, true);
+ checkComputeAttention('MERGED', 1, [], 999, [], [], [], undefined, false);
+ checkComputeAttention('MERGED', 1, [], 999, [], [], [999], undefined);
checkComputeAttention(
'MERGED', 1, [], 999, [], [], [], undefined, true, false);
- checkComputeAttention('MERGED', 1, [], 999, [1], [], []);
- checkComputeAttention('MERGED', 1, [22], 999, [], [], []);
- checkComputeAttention('MERGED', 1, [22], 999, [22], [], [22]);
+ checkComputeAttention('MERGED', 1, [], 999, [1], [], [], undefined, false);
+ checkComputeAttention('MERGED', 1, [22], 999, [], [], [], undefined, false);
+ checkComputeAttention('MERGED', 1, [22], 999, [22], [], [22], undefined,
+ false);
checkComputeAttention('MERGED', 1, [22], 999, [], [22], []);
checkComputeAttention('MERGED', 1, [22, 33], 999, [33], [22], [33]);
checkComputeAttention('MERGED', 1, [], 1, [], [], []);
@@ -958,63 +957,7 @@
});
});
- test('_processReviewerChange', () => {
- const mockIndexSplices = function(toRemove) {
- return [{
- removed: [toRemove],
- }];
- };
-
- element._processReviewerChange(
- mockIndexSplices(makeAccount()), 'REVIEWER');
- assert.equal(element._reviewersPendingRemove.REVIEWER.length, 1);
- });
-
- test('_purgeReviewersPendingRemove', () => {
- const removeStub = sinon.stub(element, '_removeAccount');
- const mock = function() {
- element._reviewersPendingRemove = {
- CC: [makeAccount()],
- REVIEWER: [makeAccount(), makeAccount()],
- };
- };
- const checkObjEmpty = function(obj) {
- for (const prop of Object.keys(obj)) {
- if (obj[prop].length) { return false; }
- }
- return true;
- };
- mock();
- element._purgeReviewersPendingRemove(true); // Cancel
- assert.isFalse(removeStub.called);
- assert.isTrue(checkObjEmpty(element._reviewersPendingRemove));
-
- mock();
- element._purgeReviewersPendingRemove(false); // Submit
- assert.isTrue(removeStub.called);
- assert.isTrue(checkObjEmpty(element._reviewersPendingRemove));
- });
-
- test('_removeAccount', done => {
- stubRestApi('removeChangeReviewer')
- .returns(Promise.resolve({ok: true}));
- const arr = [makeAccount(), makeAccount()];
- element.change.reviewers = {
- REVIEWER: arr.slice(),
- };
-
- element._removeAccount(arr[1], 'REVIEWER').then(() => {
- assert.equal(element.change.reviewers.REVIEWER.length, 1);
- assert.deepEqual(element.change.reviewers.REVIEWER, arr.slice(0, 1));
- done();
- });
- });
-
test('moving from cc to reviewer', () => {
- element._reviewersPendingRemove = {
- CC: [],
- REVIEWER: [],
- };
flush();
const reviewer1 = makeAccount();
@@ -1032,7 +975,6 @@
assert.deepEqual(element._reviewers,
[reviewer1, reviewer2, reviewer3, cc1]);
assert.deepEqual(element._ccs, [cc2, cc3, cc4]);
- assert.deepEqual(element._reviewersPendingRemove.CC, [cc1]);
element.push('_reviewers', cc4, cc3);
flush();
@@ -1040,7 +982,6 @@
assert.deepEqual(element._reviewers,
[reviewer1, reviewer2, reviewer3, cc1, cc4, cc3]);
assert.deepEqual(element._ccs, [cc2]);
- assert.deepEqual(element._reviewersPendingRemove.CC, [cc1, cc4, cc3]);
});
test('update attention section when reviewers and ccs change', () => {
@@ -1096,10 +1037,6 @@
});
test('moving from reviewer to cc', () => {
- element._reviewersPendingRemove = {
- CC: [],
- REVIEWER: [],
- };
flush();
const reviewer1 = makeAccount();
@@ -1117,7 +1054,6 @@
assert.deepEqual(element._reviewers,
[reviewer2, reviewer3]);
assert.deepEqual(element._ccs, [cc1, cc2, cc3, cc4, reviewer1]);
- assert.deepEqual(element._reviewersPendingRemove.REVIEWER, [reviewer1]);
element.push('_ccs', reviewer3, reviewer2);
flush();
@@ -1125,15 +1061,9 @@
assert.deepEqual(element._reviewers, []);
assert.deepEqual(element._ccs,
[cc1, cc2, cc3, cc4, reviewer1, reviewer3, reviewer2]);
- assert.deepEqual(element._reviewersPendingRemove.REVIEWER,
- [reviewer1, reviewer3, reviewer2]);
});
test('migrate reviewers between states', async () => {
- element._reviewersPendingRemove = {
- CC: [],
- REVIEWER: [],
- };
flush();
const reviewers = element.$.reviewers;
const ccs = element.$.ccs;
@@ -1149,11 +1079,6 @@
stubSaveReview(review => mutations.push(...review.reviewers));
- sinon.stub(element, '_removeAccount').callsFake((account, type) => {
- mutations.push({state: 'REMOVED', account});
- return Promise.resolve();
- });
-
// Remove and add to other field.
reviewers.dispatchEvent(
new CustomEvent('remove', {
@@ -1202,15 +1127,24 @@
};
// Send and purge and verify moves, delete cc3.
- await element.send()
- .then(keepReviewers =>
- element._purgeReviewersPendingRemove(false, keepReviewers));
- expect(mutations).to.have.lengthOf(5);
- expect(mutations[0]).to.deep.equal(mapReviewer(cc1));
- expect(mutations[1]).to.deep.equal(mapReviewer(cc2));
- expect(mutations[2]).to.deep.equal(mapReviewer(reviewer1, 'CC'));
- expect(mutations[3]).to.deep.equal(mapReviewer(reviewer2, 'CC'));
- expect(mutations[4]).to.deep.equal({account: cc3, state: 'REMOVED'});
+ await element.send();
+ expect(mutations).to.have.lengthOf(7);
+ expect(mutations[0]).to.deep.equal(mapReviewer(cc1,
+ ReviewerState.REVIEWER));
+ expect(mutations[1]).to.deep.equal(mapReviewer(cc2,
+ ReviewerState.REVIEWER));
+ expect(mutations[2]).to.deep.equal(mapReviewer(reviewer1,
+ ReviewerState.CC));
+ expect(mutations[3]).to.deep.equal(mapReviewer(reviewer2,
+ ReviewerState.CC));
+
+ // 3 remove events stored
+ expect(mutations[4]).to.deep.equal({reviewer: 33, state:
+ ReviewerState.REMOVED});
+ expect(mutations[5]).to.deep.equal({reviewer: 35, state:
+ ReviewerState.REMOVED});
+ expect(mutations[6]).to.deep.equal({reviewer: 37, state:
+ ReviewerState.REMOVED});
});
test('emits cancel on esc key', () => {
@@ -1533,9 +1467,6 @@
test('_submit blocked when no mutations exist', async () => {
const sendStub = sinon.stub(element, 'send').returns(Promise.resolve());
- // Stub the below function to avoid side effects from the send promise
- // resolving.
- sinon.stub(element, '_purgeReviewersPendingRemove');
element.account = makeAccount();
element.draftCommentThreads = [];
await flush();
diff --git a/polygerrit-ui/app/elements/checks/gr-checks-results.ts b/polygerrit-ui/app/elements/checks/gr-checks-results.ts
index 49f51e02..c74f6f9 100644
--- a/polygerrit-ui/app/elements/checks/gr-checks-results.ts
+++ b/polygerrit-ui/app/elements/checks/gr-checks-results.ts
@@ -20,10 +20,10 @@
import {
css,
customElement,
- internalProperty,
property,
PropertyValues,
query,
+ state,
TemplateResult,
} from 'lit-element';
import {GrLitElement} from '../lit/gr-lit-element';
@@ -39,17 +39,15 @@
import {sharedStyles} from '../../styles/shared-styles';
import {
allActions$,
- checksPatchsetNumber$,
- someProvidersAreLoading$,
- RunResult,
- CheckRun,
allLinks$,
+ CheckRun,
+ checksPatchsetNumber$,
+ RunResult,
+ someProvidersAreLoading$,
} from '../../services/checks/checks-model';
import {
allResults,
fireActionTriggered,
- hasCompletedWithoutResults,
- hasResultsOf,
iconForCategory,
iconForLink,
tooltipForLink,
@@ -462,18 +460,18 @@
}
}
-const SHOW_ALL_THRESHOLDS: Map<Category | 'SUCCESS', number> = new Map();
+const SHOW_ALL_THRESHOLDS: Map<Category, number> = new Map();
SHOW_ALL_THRESHOLDS.set(Category.ERROR, 20);
SHOW_ALL_THRESHOLDS.set(Category.WARNING, 10);
SHOW_ALL_THRESHOLDS.set(Category.INFO, 5);
-SHOW_ALL_THRESHOLDS.set('SUCCESS', 5);
+SHOW_ALL_THRESHOLDS.set(Category.SUCCESS, 5);
@customElement('gr-checks-results')
export class GrChecksResults extends GrLitElement {
@query('#filterInput')
filterInput?: HTMLInputElement;
- @internalProperty()
+ @state()
filterRegExp = new RegExp('');
/** All runs. Shown should only the selected/filtered ones. */
@@ -513,22 +511,22 @@
>();
/** Maintains the state of which result sections should show all results. */
- @internalProperty()
- isShowAll: Map<Category | 'SUCCESS', boolean> = new Map();
+ @state()
+ isShowAll: Map<Category, boolean> = new Map();
/**
* This is the current state of whether a section is expanded or not. As long
* as isSectionExpandedByUser is false this will be computed by a default rule
* on every render.
*/
- private isSectionExpanded = new Map<Category | 'SUCCESS', boolean>();
+ private isSectionExpanded = new Map<Category, boolean>();
/**
* Keeps track of whether the user intentionally changed the expansion state.
* Once this is true the default rule for showing a section expanded or not
* is not applied anymore.
*/
- private isSectionExpandedByUser = new Map<Category | 'SUCCESS', boolean>();
+ private isSectionExpandedByUser = new Map<Category, boolean>();
private readonly checksService = appContext.checksService;
@@ -732,7 +730,8 @@
<div class="body">
${this.renderSection(Category.ERROR)}
${this.renderSection(Category.WARNING)}
- ${this.renderSection(Category.INFO)} ${this.renderSection('SUCCESS')}
+ ${this.renderSection(Category.INFO)}
+ ${this.renderSection(Category.SUCCESS)}
</div>
`;
}
@@ -859,16 +858,11 @@
this.filterRegExp = new RegExp(this.filterInput.value, 'i');
}
- renderSection(category: Category | 'SUCCESS') {
+ renderSection(category: Category) {
const catString = category.toString().toLowerCase();
- let allRuns = this.runs.filter(run =>
+ const allRuns = this.runs.filter(run =>
isAttemptSelected(this.selectedAttempts, run)
);
- if (category === 'SUCCESS') {
- allRuns = allRuns.filter(hasCompletedWithoutResults);
- } else {
- allRuns = allRuns.filter(r => hasResultsOf(r, category));
- }
const all = allRuns.reduce(
(results: RunResult[], run) => [
...results,
@@ -928,7 +922,7 @@
}
renderShowAllButton(
- category: Category | 'SUCCESS',
+ category: Category,
isShowAll: boolean,
showAllThreshold: number,
resultCount: number
@@ -947,7 +941,7 @@
`;
}
- toggleShowAll(category: Category | 'SUCCESS') {
+ toggleShowAll(category: Category) {
const current = this.isShowAll.get(category) ?? false;
this.isShowAll.set(category, !current);
this.requestUpdate();
@@ -1011,7 +1005,7 @@
return html`(${filtered.length} of ${all.length})`;
}
- toggleExpanded(category: Category | 'SUCCESS') {
+ toggleExpanded(category: Category) {
const expanded = this.isSectionExpanded.get(category);
assertIsDefined(expanded, 'expanded must have been set in initial render');
this.isSectionExpanded.set(category, !expanded);
@@ -1019,8 +1013,11 @@
this.requestUpdate();
}
- computeRunResults(category: Category | 'SUCCESS', run: CheckRun) {
- if (category === 'SUCCESS') return [this.computeSuccessfulRunResult(run)];
+ computeRunResults(category: Category, run: CheckRun) {
+ const noResults = (run.results ?? []).length === 0;
+ if (noResults && category === Category.SUCCESS) {
+ return [this.computeSuccessfulRunResult(run)];
+ }
return (
run.results
?.filter(result => result.category === category)
@@ -1033,7 +1030,7 @@
computeSuccessfulRunResult(run: CheckRun): RunResult {
const adaptedRun: RunResult = {
internalResultId: run.internalRunId + '-0',
- category: Category.INFO, // will not be used, but is required
+ category: Category.SUCCESS,
summary: run.statusDescription ?? '',
...run,
};
diff --git a/polygerrit-ui/app/elements/checks/gr-checks-runs.ts b/polygerrit-ui/app/elements/checks/gr-checks-runs.ts
index de9c5fb..8ff42ee 100644
--- a/polygerrit-ui/app/elements/checks/gr-checks-runs.ts
+++ b/polygerrit-ui/app/elements/checks/gr-checks-runs.ts
@@ -20,10 +20,10 @@
import {
css,
customElement,
- internalProperty,
property,
PropertyValues,
query,
+ state,
} from 'lit-element';
import {GrLitElement} from '../lit/gr-lit-element';
import {Action, Link, RunStatus} from '../../api/checks';
@@ -326,7 +326,7 @@
@query('#filterInput')
filterInput?: HTMLInputElement;
- @internalProperty()
+ @state()
filterRegExp = new RegExp('');
@property()
diff --git a/polygerrit-ui/app/elements/checks/gr-checks-tab.ts b/polygerrit-ui/app/elements/checks/gr-checks-tab.ts
index 418598b..b28596a 100644
--- a/polygerrit-ui/app/elements/checks/gr-checks-tab.ts
+++ b/polygerrit-ui/app/elements/checks/gr-checks-tab.ts
@@ -15,13 +15,7 @@
* limitations under the License.
*/
import {html} from 'lit-html';
-import {
- css,
- customElement,
- internalProperty,
- property,
- PropertyValues,
-} from 'lit-element';
+import {css, customElement, property, PropertyValues, state} from 'lit-element';
import {GrLitElement} from '../lit/gr-lit-element';
import {Action} from '../../api/checks';
import {
@@ -64,11 +58,11 @@
@property()
changeNum: NumericChangeId | undefined = undefined;
- @internalProperty()
+ @state()
selectedRuns: string[] = [];
/** Maps checkName to selected attempt number. `undefined` means `latest`. */
- @internalProperty()
+ @state()
selectedAttempts: Map<string, number | undefined> = new Map<
string,
number | undefined
diff --git a/polygerrit-ui/app/elements/diff/gr-context-controls/gr-context-controls.ts b/polygerrit-ui/app/elements/diff/gr-context-controls/gr-context-controls.ts
new file mode 100644
index 0000000..60b1a1a
--- /dev/null
+++ b/polygerrit-ui/app/elements/diff/gr-context-controls/gr-context-controls.ts
@@ -0,0 +1,463 @@
+/**
+ * @license
+ * Copyright (C) 2021 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+import '@polymer/paper-button/paper-button';
+import '@polymer/paper-card/paper-card';
+import '@polymer/paper-checkbox/paper-checkbox';
+import '@polymer/paper-dropdown-menu/paper-dropdown-menu';
+import '@polymer/paper-fab/paper-fab';
+import '@polymer/paper-icon-button/paper-icon-button';
+import '@polymer/paper-item/paper-item';
+import '@polymer/paper-listbox/paper-listbox';
+import '@polymer/paper-tooltip/paper-tooltip.js';
+
+import '../../shared/gr-button/gr-button';
+import {pluralize} from '../../../utils/string-util';
+import {fire} from '../../../utils/event-util';
+import {DiffInfo} from '../../../types/diff';
+import {assertIsDefined} from '../../../utils/common-util';
+import {css, customElement, html, LitElement, property} from 'lit-element';
+
+import {
+ ContextButtonType,
+ RenderPreferences,
+ SyntaxBlock,
+} from '../../../api/diff';
+
+import {GrDiffGroup, hideInContextControl} from '../gr-diff/gr-diff-group';
+
+const PARTIAL_CONTEXT_AMOUNT = 10;
+
+/**
+ * Traverses a hierarchical structure of syntax blocks and
+ * finds the most local/nested block that can be associated line.
+ * It finds the closest block that contains the whole line and
+ * returns the whole path from the syntax layer (blocks) sent as parameter
+ * to the most nested block - the complete path from the top to bottom layer of
+ * a syntax tree. Example: [myNamepace, MyClass, myMethod1, aLocalFunctionInsideMethod1]
+ *
+ * @param lineNum line number for the targeted line.
+ * @param blocks Blocks for a specific syntax level in the file (to allow recursive calls)
+ */
+function findBlockTreePathForLine(
+ lineNum: number,
+ blocks?: SyntaxBlock[]
+): SyntaxBlock[] {
+ const containingBlock = blocks?.find(
+ ({range}) => range.start_line < lineNum && range.end_line > lineNum
+ );
+ if (!containingBlock) return [];
+ const innerPathInChild = findBlockTreePathForLine(
+ lineNum,
+ containingBlock?.children
+ );
+ return [containingBlock].concat(innerPathInChild);
+}
+
+@customElement('gr-context-controls')
+export class GrContextControls extends LitElement {
+ @property({type: Object}) renderPreferences?: RenderPreferences;
+
+ @property({type: Object}) diff?: DiffInfo;
+
+ @property({type: Object}) section?: HTMLElement;
+
+ @property({type: Object}) contextGroups: GrDiffGroup[] = [];
+
+ @property({type: Boolean}) showAbove = false;
+
+ @property({type: Boolean}) showBelow = false;
+
+ static styles = css`
+ :host {
+ display: flex;
+ width: 100%;
+ height: 100%;
+ justify-content: center;
+ position: absolute;
+ }
+ .contextControlButton {
+ background-color: var(--default-button-background-color);
+ font: var(--context-control-button-font, inherit);
+ /* All position is relative to container, so ignore sibling buttons. */
+ position: absolute;
+ }
+ .contextControlButton:first-child {
+ /* First button needs to claim width to display without text wrapping. */
+ position: relative;
+ }
+ .centeredButton {
+ /* Center over divider. */
+ top: 50%;
+ transform: translateY(-50%);
+ }
+ .aboveBelowButtons {
+ display: flex;
+ flex-direction: column;
+ margin-left: var(--spacing-m);
+ position: relative;
+ }
+ .aboveBelowButtons:first-child {
+ margin-left: 0;
+ }
+
+ .aboveButton {
+ /* Display over preceding content / background placeholder. */
+ transform: translateY(-100%);
+ }
+ .belowButton {
+ top: calc(100% + var(--divider-border));
+ }
+ .breadcrumbTooltip {
+ white-space: nowrap;
+ }
+ `;
+
+ // To pass CSS mixins for @apply to Polymer components, they need to be
+ // wrapped in a <custom-style>.
+ static customStyles = html`
+ <custom-style>
+ <style>
+ .centeredButton {
+ --gr-button: {
+ color: var(--diff-context-control-color);
+ border-style: solid;
+ border-color: var(--border-color);
+ border-top-width: 1px;
+ border-right-width: 1px;
+ border-bottom-width: 1px;
+ border-left-width: 1px;
+
+ border-top-left-radius: var(--border-radius);
+ border-top-right-radius: var(--border-radius);
+ border-bottom-right-radius: var(--border-radius);
+ border-bottom-left-radius: var(--border-radius);
+ padding: var(--spacing-s) var(--spacing-l);
+ }
+ }
+ .aboveButton {
+ --gr-button: {
+ color: var(--diff-context-control-color);
+ border-style: solid;
+ border-color: var(--border-color);
+ border-top-width: 1px;
+ border-right-width: 1px;
+ border-bottom-width: 0;
+ border-left-width: 1px;
+
+ border-top-left-radius: var(--border-radius);
+ border-top-right-radius: var(--border-radius);
+ border-bottom-right-radius: 0;
+ border-bottom-left-radius: var(--border-radius);
+ padding: var(--spacing-xxs) var(--spacing-l);
+ }
+ }
+ .belowButton {
+ --gr-button: {
+ color: var(--diff-context-control-color);
+ border-style: solid;
+ border-color: var(--border-color);
+ border-top-width: 0;
+ border-right-width: 1px;
+ border-bottom-width: 1px;
+ border-left-width: 1px;
+
+ border-top-left-radius: 0;
+ border-top-right-radius: 0;
+ border-bottom-right-radius: var(--border-radius);
+ border-bottom-left-radius: var(--border-radius);
+ padding: var(--spacing-xxs) var(--spacing-l);
+ }
+ }
+ </style>
+ </custom-style>
+ `;
+
+ private numLines() {
+ const {leftStart, leftEnd} = this.contextRange();
+ return leftEnd - leftStart + 1;
+ }
+
+ private createExpandAllButtonContainer() {
+ return html` <div
+ class="style-scope gr-diff aboveBelowButtons fullExpansion"
+ >
+ ${this.createContextButton(ContextButtonType.ALL, this.numLines())}
+ </div>`;
+ }
+
+ /**
+ * Creates a specific expansion button (e.g. +X common lines, +10, +Block).
+ */
+ private createContextButton(
+ type: ContextButtonType,
+ linesToExpand: number,
+ tooltipText?: string
+ ) {
+ let text = '';
+ let groups: GrDiffGroup[] = []; // The groups that replace this one if tapped.
+ let ariaLabel = '';
+ let classes = 'contextControlButton showContext ';
+
+ if (type === ContextButtonType.ALL) {
+ text = `+${pluralize(linesToExpand, 'common line')}`;
+ ariaLabel = `Show ${pluralize(linesToExpand, 'common line')}`;
+ classes +=
+ this.showAbove && this.showBelow
+ ? 'centeredButton'
+ : this.showAbove
+ ? 'aboveButton'
+ : 'belowButton';
+ if (this.partialContent) {
+ // Expanding content would require load of more data
+ text += ' (too large)';
+ }
+ groups.push(...this.contextGroups);
+ } else if (type === ContextButtonType.ABOVE) {
+ groups = hideInContextControl(
+ this.contextGroups,
+ linesToExpand,
+ this.numLines()
+ );
+ text = `+${linesToExpand}`;
+ classes += 'aboveButton';
+ ariaLabel = `Show ${pluralize(linesToExpand, 'line')} above`;
+ } else if (type === ContextButtonType.BELOW) {
+ groups = hideInContextControl(
+ this.contextGroups,
+ 0,
+ this.numLines() - linesToExpand
+ );
+ text = `+${linesToExpand}`;
+ classes += 'belowButton';
+ ariaLabel = `Show ${pluralize(linesToExpand, 'line')} below`;
+ } else if (type === ContextButtonType.BLOCK_ABOVE) {
+ groups = hideInContextControl(
+ this.contextGroups,
+ linesToExpand,
+ this.numLines()
+ );
+ text = '+Block';
+ classes += 'aboveButton';
+ ariaLabel = 'Show block above';
+ } else if (type === ContextButtonType.BLOCK_BELOW) {
+ groups = hideInContextControl(
+ this.contextGroups,
+ 0,
+ this.numLines() - linesToExpand
+ );
+ text = '+Block';
+ classes += 'belowButton';
+ ariaLabel = 'Show block below';
+ }
+ const expandHandler = this.createExpansionHandler(
+ linesToExpand,
+ type,
+ groups
+ );
+
+ const tooltip = tooltipText
+ ? html`<paper-tooltip offset="10"
+ ><div class="breadcrumbTooltip">${tooltipText}</div></paper-tooltip
+ >`
+ : undefined;
+ const button = html` <gr-button
+ class="${classes}"
+ link="true"
+ no-uppercase="true"
+ aria-label="${ariaLabel}"
+ @click="${expandHandler}"
+ >
+ <span class="showContext">${text}</span>
+ ${tooltip}
+ </gr-button>`;
+ return button;
+ }
+
+ private createExpansionHandler(
+ linesToExpand: number,
+ type: ContextButtonType,
+ groups: GrDiffGroup[]
+ ) {
+ return (e: Event) => {
+ e.stopPropagation();
+ if (type === ContextButtonType.ALL && this.partialContent) {
+ const {leftStart, leftEnd, rightStart, rightEnd} = this.contextRange();
+ const lineRange = {
+ left: {
+ start_line: leftStart,
+ end_line: leftEnd,
+ },
+ right: {
+ start_line: rightStart,
+ end_line: rightEnd,
+ },
+ };
+ fire(this, 'content-load-needed', {
+ lineRange,
+ });
+ } else {
+ assertIsDefined(this.section, 'section');
+ fire(this, 'diff-context-expanded', {
+ groups,
+ section: this.section!,
+ numLines: this.numLines(),
+ buttonType: type,
+ expandedLines: linesToExpand,
+ });
+ }
+ };
+ }
+
+ private showPartialLinks() {
+ return this.numLines() > PARTIAL_CONTEXT_AMOUNT;
+ }
+
+ /**
+ * Creates a container div with partial (+10) expansion buttons (above and/or below).
+ */
+ private createPartialExpansionButtons() {
+ if (!this.showPartialLinks()) {
+ return undefined;
+ }
+ let aboveButton;
+ let belowButton;
+ if (this.showAbove) {
+ aboveButton = this.createContextButton(
+ ContextButtonType.ABOVE,
+ PARTIAL_CONTEXT_AMOUNT
+ );
+ }
+ if (this.showBelow) {
+ belowButton = this.createContextButton(
+ ContextButtonType.BELOW,
+ PARTIAL_CONTEXT_AMOUNT
+ );
+ }
+ return aboveButton || belowButton
+ ? html` <div class="aboveBelowButtons partialExpansion">
+ ${aboveButton} ${belowButton}
+ </div>`
+ : undefined;
+ }
+
+ /**
+ * Checks if the collapsed section contains unavailable content (skip chunks).
+ */
+ private get partialContent() {
+ return this.contextGroups.some(c => !!c.skip);
+ }
+
+ /**
+ * Creates a container div with block expansion buttons (above and/or below).
+ */
+ private createBlockExpansionButtons() {
+ if (
+ !this.showPartialLinks() ||
+ !this.renderPreferences?.use_block_expansion ||
+ this.partialContent
+ ) {
+ return undefined;
+ }
+ let aboveBlockButton;
+ let belowBlockButton;
+ if (this.showAbove) {
+ aboveBlockButton = this.createBlockButton(
+ ContextButtonType.BLOCK_ABOVE,
+ this.numLines(),
+ this.contextRange().rightStart - 1
+ );
+ }
+ if (this.showBelow) {
+ belowBlockButton = this.createBlockButton(
+ ContextButtonType.BLOCK_BELOW,
+ this.numLines(),
+ this.contextRange().rightEnd + 1
+ );
+ }
+ if (aboveBlockButton || belowBlockButton) {
+ return html` <div class="aboveBelowButtons blockExpansion">
+ ${aboveBlockButton} ${belowBlockButton}
+ </div>`;
+ }
+ return undefined;
+ }
+
+ private createBlockButton(
+ buttonType: ContextButtonType,
+ numLines: number,
+ referenceLine: number
+ ) {
+ assertIsDefined(this.diff, 'diff');
+ const syntaxTree = this.diff!.meta_b.syntax_tree;
+ const outlineSyntaxPath = findBlockTreePathForLine(
+ referenceLine,
+ syntaxTree
+ );
+ let linesToExpand = numLines;
+ let tooltipText = `${linesToExpand} common lines`;
+ if (outlineSyntaxPath.length) {
+ const {range} = outlineSyntaxPath[outlineSyntaxPath.length - 1];
+ // Create breadcrumb string:
+ // myNamepace > MyClass > myMethod1 > aLocalFunctionInsideMethod1 > (anonymous)
+ tooltipText = outlineSyntaxPath
+ .map(b => b.name || '(anonymous)')
+ .join(' > ');
+ const targetLine =
+ buttonType === ContextButtonType.BLOCK_ABOVE
+ ? range.end_line
+ : range.start_line;
+ const distanceToTargetLine = Math.abs(targetLine - referenceLine);
+ if (distanceToTargetLine < numLines) {
+ linesToExpand = distanceToTargetLine;
+ }
+ }
+ return this.createContextButton(buttonType, linesToExpand, tooltipText);
+ }
+
+ private contextRange() {
+ return {
+ leftStart: this.contextGroups[0].lineRange.left.start_line,
+ leftEnd: this.contextGroups[this.contextGroups.length - 1].lineRange.left
+ .end_line,
+ rightStart: this.contextGroups[0].lineRange.right.start_line,
+ rightEnd: this.contextGroups[this.contextGroups.length - 1].lineRange
+ .right.end_line,
+ };
+ }
+
+ private hasValidProperties() {
+ return !!(this.diff && this.section && this.contextGroups?.length);
+ }
+
+ render() {
+ if (!this.hasValidProperties()) {
+ console.error('Invalid properties for gr-context-controls!');
+ return html`<p>invalid properties</p>`;
+ }
+ return html`
+ ${GrContextControls.customStyles} ${this.createExpandAllButtonContainer()}
+ ${this.createPartialExpansionButtons()}
+ ${this.createBlockExpansionButtons()}
+ `;
+ }
+}
+
+declare global {
+ interface HTMLElementTagNameMap {
+ 'gr-context-controls': GrContextControls;
+ }
+}
diff --git a/polygerrit-ui/app/elements/diff/gr-context-controls/gr-context-controls_test.ts b/polygerrit-ui/app/elements/diff/gr-context-controls/gr-context-controls_test.ts
new file mode 100644
index 0000000..d866c1a
--- /dev/null
+++ b/polygerrit-ui/app/elements/diff/gr-context-controls/gr-context-controls_test.ts
@@ -0,0 +1,377 @@
+/**
+ * @license
+ * Copyright (C) 2021 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import '../../../test/common-test-setup-karma';
+import '../gr-diff/gr-diff-group';
+import './gr-context-controls';
+import {GrContextControls} from './gr-context-controls';
+import {flush} from '@polymer/polymer/lib/legacy/polymer.dom';
+
+import {GrDiffLine, GrDiffLineType} from '../gr-diff/gr-diff-line';
+import {GrDiffGroup, GrDiffGroupType} from '../gr-diff/gr-diff-group';
+import {DiffFileMetaInfo, DiffInfo, SyntaxBlock} from '../../../api/diff';
+
+const blankFixture = fixtureFromElement('div');
+
+suite('gr-context-control tests', () => {
+ let element: GrContextControls;
+
+ setup(async () => {
+ element = document.createElement('gr-context-controls');
+ element.diff = ({content: []} as any) as DiffInfo;
+ element.renderPreferences = {};
+ element.section = document.createElement('div');
+ blankFixture.instantiate().appendChild(element);
+ await flush();
+ });
+
+ function createContextGroups(options: {offset?: number; count?: number}) {
+ const offset = options.offset || 0;
+ const numLines = options.count || 10;
+ const lines = [];
+ for (let i = 0; i < numLines; i++) {
+ const line = new GrDiffLine(GrDiffLineType.BOTH);
+ line.beforeNumber = offset + i + 1;
+ line.afterNumber = offset + i + 1;
+ line.text = 'lorem upsum';
+ lines.push(line);
+ }
+
+ return [new GrDiffGroup(GrDiffGroupType.BOTH, lines)];
+ }
+
+ test('no +10 buttons for 10 or less lines', async () => {
+ element.contextGroups = createContextGroups({count: 10});
+
+ await flush();
+
+ const buttons = element.shadowRoot!.querySelectorAll(
+ 'gr-button.showContext'
+ );
+ assert.equal(buttons.length, 1);
+ assert.equal(buttons[0].textContent!.trim(), '+10 common lines');
+ });
+
+ test('context control at the top', async () => {
+ element.contextGroups = createContextGroups({offset: 0, count: 20});
+ element.showBelow = true;
+
+ await flush();
+
+ const buttons = element.shadowRoot!.querySelectorAll(
+ 'gr-button.showContext'
+ );
+
+ assert.equal(buttons.length, 2);
+ assert.equal(buttons[0].textContent!.trim(), '+20 common lines');
+ assert.equal(buttons[1].textContent!.trim(), '+10');
+
+ assert.include([...buttons[0].classList.values()], 'belowButton');
+ assert.include([...buttons[1].classList.values()], 'belowButton');
+ });
+
+ test('context control in the middle', async () => {
+ element.contextGroups = createContextGroups({offset: 10, count: 20});
+ element.showAbove = true;
+ element.showBelow = true;
+
+ await flush();
+
+ const buttons = element.shadowRoot!.querySelectorAll(
+ 'gr-button.showContext'
+ );
+
+ assert.equal(buttons.length, 3);
+ assert.equal(buttons[0].textContent!.trim(), '+20 common lines');
+ assert.equal(buttons[1].textContent!.trim(), '+10');
+ assert.equal(buttons[2].textContent!.trim(), '+10');
+
+ assert.include([...buttons[0].classList.values()], 'centeredButton');
+ assert.include([...buttons[1].classList.values()], 'aboveButton');
+ assert.include([...buttons[2].classList.values()], 'belowButton');
+ });
+
+ test('context control at the bottom', async () => {
+ element.contextGroups = createContextGroups({offset: 30, count: 20});
+ element.showAbove = true;
+
+ await flush();
+
+ const buttons = element.shadowRoot!.querySelectorAll(
+ 'gr-button.showContext'
+ );
+
+ assert.equal(buttons.length, 2);
+ assert.equal(buttons[0].textContent!.trim(), '+20 common lines');
+ assert.equal(buttons[1].textContent!.trim(), '+10');
+
+ assert.include([...buttons[0].classList.values()], 'aboveButton');
+ assert.include([...buttons[1].classList.values()], 'aboveButton');
+ });
+
+ function prepareForBlockExpansion(syntaxTree: SyntaxBlock[]) {
+ element.renderPreferences!.use_block_expansion = true;
+ element.diff!.meta_b = ({
+ syntax_tree: syntaxTree,
+ } as any) as DiffFileMetaInfo;
+ }
+
+ test('context control with block expansion at the top', async () => {
+ prepareForBlockExpansion([]);
+ element.contextGroups = createContextGroups({offset: 0, count: 20});
+ element.showBelow = true;
+
+ await flush();
+
+ const fullExpansionButtons = element.shadowRoot!.querySelectorAll(
+ '.fullExpansion gr-button'
+ );
+ const partialExpansionButtons = element.shadowRoot!.querySelectorAll(
+ '.partialExpansion gr-button'
+ );
+ const blockExpansionButtons = element.shadowRoot!.querySelectorAll(
+ '.blockExpansion gr-button'
+ );
+ assert.equal(fullExpansionButtons.length, 1);
+ assert.equal(partialExpansionButtons.length, 1);
+ assert.equal(blockExpansionButtons.length, 1);
+ assert.equal(
+ blockExpansionButtons[0].querySelector('span')!.textContent!.trim(),
+ '+Block'
+ );
+ assert.include(
+ [...blockExpansionButtons[0].classList.values()],
+ 'belowButton'
+ );
+ });
+
+ test('context control with block expansion in the middle', async () => {
+ prepareForBlockExpansion([]);
+ element.contextGroups = createContextGroups({offset: 10, count: 20});
+ element.showAbove = true;
+ element.showBelow = true;
+
+ await flush();
+
+ const fullExpansionButtons = element.shadowRoot!.querySelectorAll(
+ '.fullExpansion gr-button'
+ );
+ const partialExpansionButtons = element.shadowRoot!.querySelectorAll(
+ '.partialExpansion gr-button'
+ );
+ const blockExpansionButtons = element.shadowRoot!.querySelectorAll(
+ '.blockExpansion gr-button'
+ );
+ assert.equal(fullExpansionButtons.length, 1);
+ assert.equal(partialExpansionButtons.length, 2);
+ assert.equal(blockExpansionButtons.length, 2);
+ assert.equal(
+ blockExpansionButtons[0].querySelector('span')!.textContent!.trim(),
+ '+Block'
+ );
+ assert.equal(
+ blockExpansionButtons[1].querySelector('span')!.textContent!.trim(),
+ '+Block'
+ );
+ assert.include(
+ [...blockExpansionButtons[0].classList.values()],
+ 'aboveButton'
+ );
+ assert.include(
+ [...blockExpansionButtons[1].classList.values()],
+ 'belowButton'
+ );
+ });
+
+ test('context control with block expansion at the bottom', async () => {
+ prepareForBlockExpansion([]);
+ element.contextGroups = createContextGroups({offset: 30, count: 20});
+ element.showAbove = true;
+
+ await flush();
+
+ const fullExpansionButtons = element.shadowRoot!.querySelectorAll(
+ '.fullExpansion gr-button'
+ );
+ const partialExpansionButtons = element.shadowRoot!.querySelectorAll(
+ '.partialExpansion gr-button'
+ );
+ const blockExpansionButtons = element.shadowRoot!.querySelectorAll(
+ '.blockExpansion gr-button'
+ );
+ assert.equal(fullExpansionButtons.length, 1);
+ assert.equal(partialExpansionButtons.length, 1);
+ assert.equal(blockExpansionButtons.length, 1);
+ assert.equal(
+ blockExpansionButtons[0].querySelector('span')!.textContent!.trim(),
+ '+Block'
+ );
+ assert.include(
+ [...blockExpansionButtons[0].classList.values()],
+ 'aboveButton'
+ );
+ });
+
+ test('+ Block tooltip tooltip shows syntax block containing the target lines above and below', async () => {
+ prepareForBlockExpansion([
+ {
+ name: 'aSpecificFunction',
+ range: {start_line: 1, start_column: 0, end_line: 25, end_column: 0},
+ children: [],
+ },
+ {
+ name: 'anotherFunction',
+ range: {start_line: 26, start_column: 0, end_line: 50, end_column: 0},
+ children: [],
+ },
+ ]);
+ element.contextGroups = createContextGroups({offset: 10, count: 20});
+ element.showAbove = true;
+ element.showBelow = true;
+
+ await flush();
+
+ const blockExpansionButtons = element.shadowRoot!.querySelectorAll(
+ '.blockExpansion gr-button'
+ );
+ assert.equal(
+ blockExpansionButtons[0]
+ .querySelector('.breadcrumbTooltip')!
+ .textContent?.trim(),
+ 'aSpecificFunction'
+ );
+ assert.equal(
+ blockExpansionButtons[1]
+ .querySelector('.breadcrumbTooltip')!
+ .textContent?.trim(),
+ 'anotherFunction'
+ );
+ });
+
+ test('+Block tooltip shows nested syntax blocks as breadcrumbs', async () => {
+ prepareForBlockExpansion([
+ {
+ name: 'aSpecificNamespace',
+ range: {start_line: 1, start_column: 0, end_line: 200, end_column: 0},
+ children: [
+ {
+ name: 'MyClass',
+ range: {
+ start_line: 2,
+ start_column: 0,
+ end_line: 100,
+ end_column: 0,
+ },
+ children: [
+ {
+ name: 'aMethod',
+ range: {
+ start_line: 5,
+ start_column: 0,
+ end_line: 80,
+ end_column: 0,
+ },
+ children: [],
+ },
+ ],
+ },
+ ],
+ },
+ ]);
+ element.contextGroups = createContextGroups({offset: 10, count: 20});
+ element.showAbove = true;
+ element.showBelow = true;
+
+ await flush();
+
+ const blockExpansionButtons = element.shadowRoot!.querySelectorAll(
+ '.blockExpansion gr-button'
+ );
+ assert.equal(
+ blockExpansionButtons[0]
+ .querySelector('.breadcrumbTooltip')!
+ .textContent?.trim(),
+ 'aSpecificNamespace > MyClass > aMethod'
+ );
+ });
+
+ test('+Block tooltip shows (anonymous) for empty blocks', async () => {
+ prepareForBlockExpansion([
+ {
+ name: 'aSpecificNamespace',
+ range: {start_line: 1, start_column: 0, end_line: 200, end_column: 0},
+ children: [
+ {
+ name: '',
+ range: {
+ start_line: 2,
+ start_column: 0,
+ end_line: 100,
+ end_column: 0,
+ },
+ children: [
+ {
+ name: 'aMethod',
+ range: {
+ start_line: 5,
+ start_column: 0,
+ end_line: 80,
+ end_column: 0,
+ },
+ children: [],
+ },
+ ],
+ },
+ ],
+ },
+ ]);
+ element.contextGroups = createContextGroups({offset: 10, count: 20});
+ element.showAbove = true;
+ element.showBelow = true;
+ await flush();
+
+ const blockExpansionButtons = element.shadowRoot!.querySelectorAll(
+ '.blockExpansion gr-button'
+ );
+ assert.equal(
+ blockExpansionButtons[0]
+ .querySelector('.breadcrumbTooltip')!
+ .textContent?.trim(),
+ 'aSpecificNamespace > (anonymous) > aMethod'
+ );
+ });
+
+ test('+Block tooltip shows "all common lines" for empty syntax tree', async () => {
+ prepareForBlockExpansion([]);
+
+ element.contextGroups = createContextGroups({offset: 10, count: 20});
+ element.showAbove = true;
+ element.showBelow = true;
+ await flush();
+
+ const blockExpansionButtons = element.shadowRoot!.querySelectorAll(
+ '.blockExpansion gr-button'
+ );
+ // querySelector('.breadcrumbTooltip')!.textContent!.trim()
+ assert.equal(
+ blockExpansionButtons[0]
+ .querySelector('.breadcrumbTooltip')!
+ .textContent?.trim(),
+ '20 common lines'
+ );
+ });
+});
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-element_test.js b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-element_test.js
index 5ba3606..4455bd5 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-element_test.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-element_test.js
@@ -18,6 +18,7 @@
import '../../../test/common-test-setup-karma.js';
import '../gr-diff/gr-diff-group.js';
import './gr-diff-builder.js';
+import '../gr-context-controls/gr-context-controls.js';
import {getMockDiffResponse} from '../../../test/mocks/diff-response.js';
import './gr-diff-builder-element.js';
import {stubBaseUrl} from '../../../test/test-utils.js';
@@ -74,152 +75,6 @@
assert.isTrue(node.classList.contains('classes'));
});
- suite('context control', () => {
- function createContextGroups(options) {
- const offset = options.offset || 0;
- const numLines = options.count || 10;
- const lines = [];
- for (let i = 0; i < numLines; i++) {
- const line = new GrDiffLine(GrDiffLineType.BOTH);
- line.beforeNumber = offset + i + 1;
- line.afterNumber = offset + i + 1;
- line.text = 'lorem upsum';
- lines.push(line);
- }
-
- return [new GrDiffGroup(GrDiffGroupType.BOTH, lines)];
- }
-
- function createContextSectionForGroups(options) {
- const section = document.createElement('div');
- builder._createContextControls(
- section, createContextGroups(options), DiffViewMode.UNIFIED);
- return section;
- }
-
- let diffInfo;
- let renderPrefs;
-
- setup(() => {
- diffInfo = {content: []};
- renderPrefs = {};
- builder = new GrDiffBuilder(diffInfo, prefs, null, [], renderPrefs);
- });
-
- test('no +10 buttons for 10 or less lines', () => {
- const section = createContextSectionForGroups({count: 10});
- const buttons = section.querySelectorAll('gr-button.showContext');
-
- assert.equal(buttons.length, 1);
- assert.equal(buttons[0].textContent, '+10 common lines');
- });
-
- test('context control at the top', () => {
- builder._numLinesLeft = 50;
- const section = createContextSectionForGroups({offset: 0, count: 20});
- const buttons = section.querySelectorAll('gr-button.showContext');
-
- assert.equal(buttons.length, 2);
- assert.equal(buttons[0].textContent, '+20 common lines');
- assert.equal(buttons[1].textContent, '+10');
-
- assert.include([...buttons[0].classList.values()], 'belowButton');
- assert.include([...buttons[1].classList.values()], 'belowButton');
- });
-
- test('context control in the middle', () => {
- builder._numLinesLeft = 50;
- const section = createContextSectionForGroups({offset: 10, count: 20});
- const buttons = section.querySelectorAll('gr-button.showContext');
-
- assert.equal(buttons.length, 3);
- assert.equal(buttons[0].textContent, '+20 common lines');
- assert.equal(buttons[1].textContent, '+10');
- assert.equal(buttons[2].textContent, '+10');
-
- assert.include([...buttons[0].classList.values()], 'centeredButton');
- assert.include([...buttons[1].classList.values()], 'aboveButton');
- assert.include([...buttons[2].classList.values()], 'belowButton');
- });
-
- test('context control at the bottom', () => {
- builder._numLinesLeft = 50;
- const section = createContextSectionForGroups({offset: 30, count: 20});
- const buttons = section.querySelectorAll('gr-button.showContext');
-
- assert.equal(buttons.length, 2);
- assert.equal(buttons[0].textContent, '+20 common lines');
- assert.equal(buttons[1].textContent, '+10');
-
- assert.include([...buttons[0].classList.values()], 'aboveButton');
- assert.include([...buttons[1].classList.values()], 'aboveButton');
- });
-
- suite('with block expansion', () => {
- setup(() => {
- builder._numLinesLeft = 50;
- renderPrefs.use_block_expansion = true;
- diffInfo.meta_b = {
- syntax_tree: [],
- };
- });
-
- test('context control with block expansion at the top', () => {
- const section = createContextSectionForGroups({offset: 0, count: 20});
-
- const fullExpansionButtons = section
- .querySelectorAll('.fullExpansion gr-button');
- const partialExpansionButtons = section
- .querySelectorAll('.partialExpansion gr-button');
- const blockExpansionButtons = section
- .querySelectorAll('.blockExpansion gr-button');
- assert.equal(fullExpansionButtons.length, 1);
- assert.equal(partialExpansionButtons.length, 1);
- assert.equal(blockExpansionButtons.length, 1);
- assert.equal(blockExpansionButtons[0].textContent, '+Block');
- assert.include([...blockExpansionButtons[0].classList.values()],
- 'belowButton');
- });
-
- test('context control in the middle', () => {
- const section = createContextSectionForGroups({offset: 10, count: 20});
-
- const fullExpansionButtons = section
- .querySelectorAll('.fullExpansion gr-button');
- const partialExpansionButtons = section
- .querySelectorAll('.partialExpansion gr-button');
- const blockExpansionButtons = section
- .querySelectorAll('.blockExpansion gr-button');
- assert.equal(fullExpansionButtons.length, 1);
- assert.equal(partialExpansionButtons.length, 2);
- assert.equal(blockExpansionButtons.length, 2);
- assert.equal(blockExpansionButtons[0].textContent, '+Block');
- assert.equal(blockExpansionButtons[1].textContent, '+Block');
- assert.include([...blockExpansionButtons[0].classList.values()],
- 'aboveButton');
- assert.include([...blockExpansionButtons[1].classList.values()],
- 'belowButton');
- });
-
- test('context control at the bottom', () => {
- const section = createContextSectionForGroups({offset: 30, count: 20});
-
- const fullExpansionButtons = section
- .querySelectorAll('.fullExpansion gr-button');
- const partialExpansionButtons = section
- .querySelectorAll('.partialExpansion gr-button');
- const blockExpansionButtons = section
- .querySelectorAll('.blockExpansion gr-button');
- assert.equal(fullExpansionButtons.length, 1);
- assert.equal(partialExpansionButtons.length, 1);
- assert.equal(blockExpansionButtons.length, 1);
- assert.equal(blockExpansionButtons[0].textContent, '+Block');
- assert.include([...blockExpansionButtons[0].classList.values()],
- 'aboveButton');
- });
- });
- });
-
test('newlines 1', () => {
let text = 'abcdef';
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.ts b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.ts
index adfe75f..0ccf8bc 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.ts
@@ -16,25 +16,20 @@
*/
import {
ContentLoadNeededEventDetail,
- ContextButtonType,
DiffContextExpandedExternalDetail,
MovedLinkClickedEventDetail,
RenderPreferences,
- SyntaxBlock,
} from '../../../api/diff';
import {getBaseUrl} from '../../../utils/url-util';
import {GrDiffLine, GrDiffLineType, LineNumber} from '../gr-diff/gr-diff-line';
-import {
- GrDiffGroup,
- GrDiffGroupType,
- hideInContextControl,
-} from '../gr-diff/gr-diff-group';
+import {GrDiffGroup, GrDiffGroupType} from '../gr-diff/gr-diff-group';
+
+import '../gr-context-controls/gr-context-controls';
+import {GrContextControls} from '../gr-context-controls/gr-context-controls';
import {BlameInfo} from '../../../types/common';
import {DiffInfo, DiffPreferencesInfo} from '../../../types/diff';
import {DiffViewMode, Side} from '../../../constants/constants';
import {DiffLayer} from '../../../types/types';
-import {pluralize} from '../../../utils/string-util';
-import {fire} from '../../../utils/event-util';
/**
* In JS, unicode code points above 0xFFFF occupy two elements of a string.
@@ -58,8 +53,6 @@
*/
const REGEX_TAB_OR_SURROGATE_PAIR = /\t|[\uD800-\uDBFF][\uDC00-\uDFFF]/;
-const PARTIAL_CONTEXT_AMOUNT = 10;
-
export interface DiffContextExpandedEventDetail
extends DiffContextExpandedExternalDetail {
groups: GrDiffGroup[];
@@ -74,19 +67,6 @@
}
}
-function findMostNestedContainingBlock(
- lineNum: number,
- blocks?: SyntaxBlock[]
-): SyntaxBlock | undefined {
- const containingBlock = blocks?.find(
- ({range}) => range.start_line < lineNum && range.end_line > lineNum
- );
- const containingChildBlock = containingBlock
- ? findMostNestedContainingBlock(lineNum, containingBlock?.children)
- : undefined;
- return containingChildBlock || containingBlock;
-}
-
export abstract class GrDiffBuilder {
private readonly _diff: DiffInfo;
@@ -320,7 +300,6 @@
);
}
- // TODO(renanoliveira): Move context controls to polymer component (or at least a separate class).
_createContextControls(
section: HTMLElement,
contextGroups: GrDiffGroup[],
@@ -329,10 +308,6 @@
const leftStart = contextGroups[0].lineRange.left.start_line;
const leftEnd =
contextGroups[contextGroups.length - 1].lineRange.left.end_line;
- const rightStart = contextGroups[0].lineRange.right.start_line;
- const rightEnd =
- contextGroups[contextGroups.length - 1].lineRange.right.end_line;
-
const firstGroupIsSkipped = !!contextGroups[0].skip;
const lastGroupIsSkipped = !!contextGroups[contextGroups.length - 1].skip;
@@ -349,9 +324,7 @@
section,
contextGroups,
showAbove,
- showBelow,
- rightStart,
- rightEnd
+ showBelow
)
);
if (showBelow) {
@@ -369,13 +342,8 @@
section: HTMLElement,
contextGroups: GrDiffGroup[],
showAbove: boolean,
- showBelow: boolean,
- rightStart: number,
- rightEnd: number
+ showBelow: boolean
): HTMLElement {
- const numLines = rightEnd - rightStart + 1;
- if (numLines === 0) console.error('context group without lines');
-
const row = this._createElement('tr', 'contextDivider');
if (!(showAbove && showBelow)) {
row.classList.add('collapsed');
@@ -384,193 +352,19 @@
const element = this._createElement('td', 'dividerCell');
row.appendChild(element);
- const showAllContainer = this._createExpandAllButtonContainer(
- section,
- contextGroups,
- showAbove,
- showBelow,
- numLines
- );
- element.appendChild(showAllContainer);
-
- const showPartialLinks = numLines > PARTIAL_CONTEXT_AMOUNT;
- if (showPartialLinks) {
- const partialExpansionContainer = this._createPartialExpansionButtons(
- section,
- contextGroups,
- showAbove,
- showBelow,
- numLines
- );
- if (partialExpansionContainer) {
- element.appendChild(partialExpansionContainer);
- }
- const blockExpansionContainer = this._createBlockExpansionButtons(
- section,
- contextGroups,
- showAbove,
- showBelow,
- rightStart,
- rightEnd,
- numLines
- );
- if (blockExpansionContainer) {
- element.appendChild(blockExpansionContainer);
- }
- }
+ const contextControls = this._createElement(
+ 'gr-context-controls'
+ ) as GrContextControls;
+ contextControls.diff = this._diff;
+ contextControls.renderPreferences = this._renderPrefs;
+ contextControls.section = section;
+ contextControls.contextGroups = contextGroups;
+ contextControls.showAbove = showAbove;
+ contextControls.showBelow = showBelow;
+ element.appendChild(contextControls);
return row;
}
- private _createExpandAllButtonContainer(
- section: HTMLElement,
- contextGroups: GrDiffGroup[],
- showAbove: boolean,
- showBelow: boolean,
- numLines: number
- ) {
- const showAllButton = this._createContextButton(
- ContextButtonType.ALL,
- section,
- contextGroups,
- numLines,
- numLines
- );
- showAllButton.classList.add(
- showAbove && showBelow
- ? 'centeredButton'
- : showAbove
- ? 'aboveButton'
- : 'belowButton'
- );
- const showAllContainer = this._createElement(
- 'div',
- 'aboveBelowButtons fullExpansion'
- );
- showAllContainer.appendChild(showAllButton);
- return showAllContainer;
- }
-
- private _createPartialExpansionButtons(
- section: HTMLElement,
- contextGroups: GrDiffGroup[],
- showAbove: boolean,
- showBelow: boolean,
- numLines: number
- ) {
- let aboveButton;
- let belowButton;
- if (showAbove) {
- aboveButton = this._createContextButton(
- ContextButtonType.ABOVE,
- section,
- contextGroups,
- numLines,
- PARTIAL_CONTEXT_AMOUNT
- );
- }
- if (showBelow) {
- belowButton = this._createContextButton(
- ContextButtonType.BELOW,
- section,
- contextGroups,
- numLines,
- PARTIAL_CONTEXT_AMOUNT
- );
- }
- if (aboveButton || belowButton) {
- const partialExpansionContainer = this._createElement(
- 'div',
- 'aboveBelowButtons partialExpansion'
- );
- aboveButton && partialExpansionContainer.appendChild(aboveButton);
- belowButton && partialExpansionContainer.appendChild(belowButton);
- return partialExpansionContainer;
- }
- return undefined;
- }
-
- private _createBlockExpansionButtons(
- section: HTMLElement,
- contextGroups: GrDiffGroup[],
- showAbove: boolean,
- showBelow: boolean,
- rightStart: number,
- rightEnd: number,
- numLines: number
- ) {
- const fullContentNotAvailable =
- contextGroups.find(c => !!c.skip) !== undefined;
- if (!this._renderPrefs?.use_block_expansion || fullContentNotAvailable) {
- return undefined;
- }
- let aboveBlockButton;
- let belowBlockButton;
- const rightSyntaxTree = this._diff.meta_b.syntax_tree;
- if (showAbove) {
- aboveBlockButton = this._createBlockButton(
- section,
- contextGroups,
- ContextButtonType.BLOCK_ABOVE,
- numLines,
- rightStart - 1,
- rightSyntaxTree
- );
- }
- if (showBelow) {
- belowBlockButton = this._createBlockButton(
- section,
- contextGroups,
- ContextButtonType.BLOCK_BELOW,
- numLines,
- rightEnd + 1,
- rightSyntaxTree
- );
- }
- if (aboveBlockButton || belowBlockButton) {
- const blockExpansionContainer = this._createElement(
- 'div',
- 'blockExpansion aboveBelowButtons'
- );
- aboveBlockButton && blockExpansionContainer.appendChild(aboveBlockButton);
- belowBlockButton && blockExpansionContainer.appendChild(belowBlockButton);
- return blockExpansionContainer;
- }
- return undefined;
- }
-
- private _createBlockButton(
- section: HTMLElement,
- contextGroups: GrDiffGroup[],
- buttonType: ContextButtonType,
- numLines: number,
- referenceLine: number,
- syntaxTree?: SyntaxBlock[]
- ) {
- const containingBlock = findMostNestedContainingBlock(
- referenceLine,
- syntaxTree
- );
- let linesToExpand = numLines;
- if (containingBlock) {
- const {range} = containingBlock;
- const targetLine =
- buttonType === ContextButtonType.BLOCK_ABOVE
- ? range.end_line
- : range.start_line;
- const distanceToTargetLine = Math.abs(targetLine - referenceLine);
- if (distanceToTargetLine < numLines) {
- linesToExpand = distanceToTargetLine;
- }
- }
- return this._createContextButton(
- buttonType,
- section,
- contextGroups,
- numLines,
- linesToExpand
- );
- }
-
/**
* Creates a table row to serve as padding between code and context controls.
* Blame column, line gutters, and content area will continue visually, but
@@ -599,99 +393,6 @@
return row;
}
- _createContextButton(
- type: ContextButtonType,
- section: HTMLElement,
- contextGroups: GrDiffGroup[],
- numLines: number,
- linesToExpand: number
- ) {
- const button = this._createElement('gr-button', 'showContext');
- button.classList.add('contextControlButton');
- button.setAttribute('link', 'true');
- button.setAttribute('no-uppercase', 'true');
-
- let text = '';
- let groups: GrDiffGroup[] = []; // The groups that replace this one if tapped.
- let requiresLoad = false;
- if (type === ContextButtonType.ALL) {
- text = `+${pluralize(linesToExpand, 'common line')}`;
- button.setAttribute(
- 'aria-label',
- `Show ${pluralize(linesToExpand, 'common line')}`
- );
- requiresLoad = contextGroups.find(c => !!c.skip) !== undefined;
- if (requiresLoad) {
- // Expanding content would require load of more data
- text += ' (too large)';
- }
- groups.push(...contextGroups);
- } else if (type === ContextButtonType.ABOVE) {
- groups = hideInContextControl(contextGroups, linesToExpand, numLines);
- text = `+${linesToExpand}`;
- button.classList.add('aboveButton');
- button.setAttribute(
- 'aria-label',
- `Show ${pluralize(linesToExpand, 'line')} above`
- );
- } else if (type === ContextButtonType.BELOW) {
- groups = hideInContextControl(contextGroups, 0, numLines - linesToExpand);
- text = `+${linesToExpand}`;
- button.classList.add('belowButton');
- button.setAttribute(
- 'aria-label',
- `Show ${pluralize(linesToExpand, 'line')} below`
- );
- } else if (type === ContextButtonType.BLOCK_ABOVE) {
- groups = hideInContextControl(contextGroups, linesToExpand, numLines);
- text = '+Block';
- button.classList.add('aboveButton');
- button.setAttribute('aria-label', 'Show block above');
- } else if (type === ContextButtonType.BLOCK_BELOW) {
- groups = hideInContextControl(contextGroups, 0, numLines - linesToExpand);
- text = '+Block';
- button.classList.add('belowButton');
- button.setAttribute('aria-label', 'Show block below');
- }
- const textSpan = this._createElement('span', 'showContext');
- textSpan.textContent = text;
- button.appendChild(textSpan);
-
- if (requiresLoad) {
- button.addEventListener('click', e => {
- e.stopPropagation();
- const firstRange = groups[0].lineRange;
- const lastRange = groups[groups.length - 1].lineRange;
- const lineRange = {
- left: {
- start_line: firstRange.left.start_line,
- end_line: lastRange.left.end_line,
- },
- right: {
- start_line: firstRange.right.start_line,
- end_line: lastRange.right.end_line,
- },
- };
- fire(button, 'content-load-needed', {
- lineRange,
- });
- });
- } else {
- button.addEventListener('click', e => {
- e.stopPropagation();
- fire(button, 'diff-context-expanded', {
- groups,
- section,
- numLines,
- buttonType: type,
- expandedLines: linesToExpand,
- });
- });
- }
-
- return button;
- }
-
_createLineEl(
line: GrDiffLine,
number: LineNumber,
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor_test.js b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor_test.js
index 75439c8..901f72a 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor_test.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor_test.js
@@ -617,6 +617,7 @@
test('expand context updates stops', done => {
sinon.spy(cursorElement, '_updateStops');
MockInteractions.tap(diffElement.shadowRoot
+ .querySelector('gr-context-controls').shadowRoot
.querySelector('.showContext'));
flush(() => {
assert.isTrue(cursorElement._updateStops.called);
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-image-viewer/gr-image-viewer.ts b/polygerrit-ui/app/elements/diff/gr-diff-image-viewer/gr-image-viewer.ts
index 2a4b250..9a98a21 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-image-viewer/gr-image-viewer.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-image-viewer/gr-image-viewer.ts
@@ -29,11 +29,11 @@
css,
customElement,
html,
- internalProperty,
LitElement,
property,
PropertyValues,
query,
+ state,
} from 'lit-element';
import {classMap} from 'lit-html/directives/class-map';
import {StyleInfo, styleMap} from 'lit-html/directives/style-map';
@@ -66,23 +66,23 @@
// URL for the image to use as revision.
@property({type: String}) revisionUrl = '';
- @internalProperty() protected baseSelected = true;
+ @state() protected baseSelected = true;
- @internalProperty() protected scaledSelected = true;
+ @state() protected scaledSelected = true;
- @internalProperty() protected followMouse = false;
+ @state() protected followMouse = false;
- @internalProperty() protected scale = 1;
+ @state() protected scale = 1;
- @internalProperty() protected checkerboardSelected = true;
+ @state() protected checkerboardSelected = true;
- @internalProperty() protected backgroundColor = '';
+ @state() protected backgroundColor = '';
- @internalProperty() protected automaticBlink = false;
+ @state() protected automaticBlink = false;
- @internalProperty() protected automaticBlinkShown = false;
+ @state() protected automaticBlinkShown = false;
- @internalProperty() protected zoomedImageStyle: StyleInfo = {};
+ @state() protected zoomedImageStyle: StyleInfo = {};
@query('.imageArea') protected imageArea!: HTMLDivElement;
@@ -94,16 +94,16 @@
private imageSize: Dimensions = {width: 0, height: 0};
- @internalProperty()
+ @state()
protected magnifierSize: Dimensions = {width: 0, height: 0};
- @internalProperty()
+ @state()
protected magnifierFrame: Rect = {
origin: {x: 0, y: 0},
dimensions: {width: 0, height: 0},
};
- @internalProperty()
+ @state()
protected overviewFrame: Rect = {
origin: {x: 0, y: 0},
dimensions: {width: 0, height: 0},
@@ -118,7 +118,7 @@
2,
];
- @internalProperty() protected grabbing = false;
+ @state() protected grabbing = false;
private ownsMouseDown = false;
@@ -745,16 +745,20 @@
}
mouseupMagnifier(event: MouseEvent) {
+ if (!this.ownsMouseDown) return;
+ this.grabbing = false;
+ this.ownsMouseDown = false;
const offsetX = event.clientX - this.pointerOnDown.x;
const offsetY = event.clientY - this.pointerOnDown.y;
const distance = Math.max(Math.abs(offsetX), Math.abs(offsetY));
// Consider very short drags as clicks. These tend to happen more often on
// external mice.
- if (this.ownsMouseDown && distance < DRAG_DEAD_ZONE_PIXELS) {
+ if (distance < DRAG_DEAD_ZONE_PIXELS) {
this.toggleImage();
+ this.dispatchEvent(createEvent({type: 'magnifier-clicked'}));
+ } else {
+ this.dispatchEvent(createEvent({type: 'magnifier-dragged'}));
}
- this.grabbing = false;
- this.ownsMouseDown = false;
}
mousemoveMagnifier(event: MouseEvent) {
@@ -793,8 +797,10 @@
}
mouseleaveMagnifier() {
+ if (!this.ownsMouseDown) return;
this.grabbing = false;
this.ownsMouseDown = false;
+ this.dispatchEvent(createEvent({type: 'magnifier-dragged'}));
}
dragstartMagnifier(event: DragEvent) {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-image-viewer/gr-overview-image.ts b/polygerrit-ui/app/elements/diff/gr-diff-image-viewer/gr-overview-image.ts
index d7b6916..9439dca 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-image-viewer/gr-overview-image.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-image-viewer/gr-overview-image.ts
@@ -18,11 +18,11 @@
css,
customElement,
html,
- internalProperty,
LitElement,
property,
PropertyValues,
query,
+ state,
} from 'lit-element';
import {StyleInfo, styleMap} from 'lit-html/directives/style-map';
import {ImageDiffAction} from '../../../api/diff';
@@ -45,13 +45,13 @@
@property({type: Object})
frameRect: Rect = {origin: {x: 0, y: 0}, dimensions: {width: 0, height: 0}};
- @internalProperty() protected contentStyle: StyleInfo = {};
+ @state() protected contentStyle: StyleInfo = {};
- @internalProperty() protected contentTransformStyle: StyleInfo = {};
+ @state() protected contentTransformStyle: StyleInfo = {};
- @internalProperty() protected frameStyle: StyleInfo = {};
+ @state() protected frameStyle: StyleInfo = {};
- @internalProperty() protected dragging = false;
+ @state() protected dragging = false;
@query('.content-box') protected contentBox!: HTMLDivElement;
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-image-viewer/gr-zoomed-image.ts b/polygerrit-ui/app/elements/diff/gr-diff-image-viewer/gr-zoomed-image.ts
index a14a9cc..4558dda 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-image-viewer/gr-zoomed-image.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-image-viewer/gr-zoomed-image.ts
@@ -18,10 +18,10 @@
css,
customElement,
html,
- internalProperty,
LitElement,
property,
PropertyValues,
+ state,
} from 'lit-element';
import {StyleInfo, styleMap} from 'lit-html/directives/style-map';
import {Rect} from './util';
@@ -41,7 +41,7 @@
@property({type: Object})
frameRect: Rect = {origin: {x: 0, y: 0}, dimensions: {width: 0, height: 0}};
- @internalProperty() protected imageStyles: StyleInfo = {};
+ @state() protected imageStyles: StyleInfo = {};
static styles = css`
:host {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
index 829597d..638b49e 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
@@ -97,7 +97,11 @@
getPatchRangeForCommentUrl,
} from '../../../utils/comment-util';
import {AppElementParams} from '../../gr-app-types';
-import {CustomKeyboardEvent, OpenFixPreviewEvent} from '../../../types/events';
+import {
+ CustomKeyboardEvent,
+ EventType,
+ OpenFixPreviewEvent,
+} from '../../../types/events';
import {fireAlert, fireEvent, fireTitleChange} from '../../../utils/event-util';
import {GerritView} from '../../../services/router/router-model';
import {assertIsDefined} from '../../../utils/common-util';
@@ -1008,6 +1012,14 @@
return;
}
+ // Everything in the diff view is tied to the change. It seems better to
+ // force the re-creation of the diff view when the change number changes.
+ const changeChanged = this._changeNum !== value.changeNum;
+ if (this._changeNum !== undefined && changeChanged) {
+ fireEvent(this, EventType.RECREATE_DIFF_VIEW);
+ return;
+ }
+
this._change = undefined;
this._files = {sortedFileList: [], changeFilesByPath: {}};
this._path = undefined;
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js
index 5c3dfdc..244bafb 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js
@@ -1735,19 +1735,6 @@
});
});
- test('_paramsChanged sets in projectLookup', () => {
- sinon.stub(element, '_initLineOfInterestAndCursor');
- const setStub = stubRestApi('setInProjectLookup');
- element._paramsChanged({
- view: GerritNav.View.DIFF,
- changeNum: 101,
- project: 'test-project',
- path: '',
- });
- assert.isTrue(setStub.calledOnce);
- assert.isTrue(setStub.calledWith(101, 'test-project'));
- });
-
test('shift+m navigates to next unreviewed file', () => {
element._files = getFilesFromFileList(['file1', 'file2', 'file3']);
element._reviewedFiles = new Set(['file1', 'file2']);
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 285c942..f48d15a 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
@@ -344,81 +344,6 @@
top: 0;
left: 0;
}
- .contextControlButton {
- background-color: var(--default-button-background-color);
- font: var(--context-control-button-font, inherit);
- /* All position is relative to container, so ignore sibling buttons. */
- position: absolute;
- }
- .contextControlButton:first-child {
- /* First button needs to claim width to display without text wrapping. */
- position: relative;
- }
- .centeredButton {
- /* Center over divider. */
- top: 50%;
- transform: translateY(-50%);
- --gr-button: {
- color: var(--diff-context-control-color);
- border-style: solid;
- border-color: var(--border-color);
- border-top-width: 1px;
- border-right-width: 1px;
- border-bottom-width: 1px;
- border-left-width: 1px;
- border-top-left-radius: var(--border-radius);
- border-top-right-radius: var(--border-radius);
- border-bottom-right-radius: var(--border-radius);
- border-bottom-left-radius: var(--border-radius);
- padding: var(--spacing-s) var(--spacing-l);
- }
- }
- .aboveBelowButtons {
- display: flex;
- flex-direction: column;
- margin-left: var(--spacing-m);
- position: relative;
- }
- .aboveBelowButtons:first-child {
- margin-left: 0;
- }
- .aboveButton {
- /* Display over preceding content / background placeholder. */
- transform: translateY(-100%);
- --gr-button: {
- color: var(--diff-context-control-color);
- border-style: solid;
- border-color: var(--border-color);
- border-top-width: 1px;
- border-right-width: 1px;
- border-bottom-width: 0;
- border-left-width: 1px;
- border-top-left-radius: var(--border-radius);
- border-top-right-radius: var(--border-radius);
- border-bottom-right-radius: 0;
- border-bottom-left-radius: 0;
- padding: var(--spacing-xxs) var(--spacing-l);
- }
- }
- .belowButton {
- /* Display over following content / background placeholder. */
- top: calc(100% + var(--divider-border));
- --gr-button: {
- color: var(--diff-context-control-color);
- border-style: solid;
- border-color: var(--border-color);
- border-top-width: 0;
- border-right-width: 1px;
- border-bottom-width: 1px;
- border-left-width: 1px;
- border-top-left-radius: 0;
- border-top-right-radius: 0;
- border-bottom-right-radius: var(--border-radius);
- border-bottom-left-radius: var(--border-radius);
- padding: var(--spacing-xxs) var(--spacing-l);
- }
- }
-
.displayLine .diff-row.target-row td {
box-shadow: inset 0 -1px var(--border-color);
}
diff --git a/polygerrit-ui/app/elements/gr-app-element.ts b/polygerrit-ui/app/elements/gr-app-element.ts
index 81e4887..6e20a55 100644
--- a/polygerrit-ui/app/elements/gr-app-element.ts
+++ b/polygerrit-ui/app/elements/gr-app-element.ts
@@ -67,18 +67,19 @@
import {GrSettingsView} from './settings/gr-settings-view/gr-settings-view';
import {
CustomKeyboardEvent,
+ DialogChangeEventDetail,
+ EventType,
LocationChangeEvent,
PageErrorEventDetail,
RpcLogEvent,
ShortcutTriggeredEvent,
TitleChangeEventDetail,
- DialogChangeEventDetail,
- EventType,
} from '../types/events';
import {ViewState} from '../types/types';
import {GerritView} from '../services/router/router-model';
import {LifeCycle} from '../constants/reporting';
import {fireIronAnnounce} from '../utils/event-util';
+import {assertIsDefined} from '../utils/common-util';
interface ErrorInfo {
text: string;
@@ -95,6 +96,10 @@
};
}
+type DomIf = PolymerElement & {
+ restamp: boolean;
+};
+
// TODO(TS): implement AppElement interface from gr-app-types.ts
@customElement('gr-app-element')
export class GrAppElement extends KeyboardShortcutMixin(PolymerElement) {
@@ -235,6 +240,12 @@
this.addEventListener('location-change', e =>
this._handleLocationChange(e)
);
+ this.addEventListener(EventType.RECREATE_CHANGE_VIEW, () =>
+ this.handleRecreateView(GerritView.CHANGE)
+ );
+ this.addEventListener(EventType.RECREATE_DIFF_VIEW, () =>
+ this.handleRecreateView(GerritView.DIFF)
+ );
document.addEventListener('gr-rpc-log', e => this._handleRpcLog(e));
this.addEventListener('shortcut-triggered', e =>
this._handleShortcutTriggered(e)
@@ -458,35 +469,58 @@
(this._account && this._account._account_id) || null;
}
- @observe('params.view')
- _viewChanged(view?: GerritView) {
+ /**
+ * Throws away the view and re-creates it. The view itself fires an event, if
+ * it wants to be re-created.
+ */
+ private handleRecreateView(view: GerritView.DIFF | GerritView.CHANGE) {
+ const isDiff = view === GerritView.DIFF;
+ const domId = isDiff ? '#dom-if-diff-view' : '#dom-if-change-view';
+ const domIf = this.root!.querySelector(domId) as DomIf;
+ assertIsDefined(domIf, '<dom-if> for the view');
+ // The rendering of DomIf is debounced, so just changing _show...View and
+ // restamp properties back and forth won't work. That is why we are using
+ // timeouts.
+ // The first timeout is needed, because the _viewChanged() observer also
+ // affects _show...View and would change _show...View=false directly back to
+ // _show...View=true.
+ setTimeout(() => {
+ this._showChangeView = false;
+ this._showDiffView = false;
+ domIf.restamp = true;
+ setTimeout(() => {
+ this._showChangeView = this.params?.view === GerritView.CHANGE;
+ this._showDiffView = this.params?.view === GerritView.DIFF;
+ domIf.restamp = false;
+ }, 1);
+ }, 1);
+ }
+
+ @observe('params.*')
+ _viewChanged() {
+ const view = this.params?.view;
this.$.errorView.classList.remove('show');
- this.set('_showChangeListView', view === GerritView.SEARCH);
- this.set('_showDashboardView', view === GerritView.DASHBOARD);
- this.set('_showChangeView', view === GerritView.CHANGE);
- this.set('_showDiffView', view === GerritView.DIFF);
- this.set('_showSettingsView', view === GerritView.SETTINGS);
+ this._showChangeListView = view === GerritView.SEARCH;
+ this._showDashboardView = view === GerritView.DASHBOARD;
+ this._showChangeView = view === GerritView.CHANGE;
+ this._showDiffView = view === GerritView.DIFF;
+ this._showSettingsView = view === GerritView.SETTINGS;
// _showAdminView must be in sync with the gr-admin-view AdminViewParams type
- this.set(
- '_showAdminView',
+ this._showAdminView =
view === GerritView.ADMIN ||
- view === GerritView.GROUP ||
- view === GerritView.REPO
- );
- this.set('_showCLAView', view === GerritView.AGREEMENTS);
- this.set('_showEditorView', view === GerritView.EDIT);
+ view === GerritView.GROUP ||
+ view === GerritView.REPO;
+ this._showCLAView = view === GerritView.AGREEMENTS;
+ this._showEditorView = view === GerritView.EDIT;
const isPluginScreen = view === GerritView.PLUGIN_SCREEN;
- this.set('_showPluginScreen', false);
+ this._showPluginScreen = false;
// Navigation within plugin screens does not restamp gr-endpoint-decorator
// because _showPluginScreen value does not change. To force restamp,
// change _showPluginScreen value between true and false.
if (isPluginScreen) {
- setTimeout(() => this.set('_showPluginScreen', true), 1);
+ setTimeout(() => (this._showPluginScreen = true), 1);
}
- this.set(
- '_showDocumentationSearch',
- view === GerritView.DOCUMENTATION_SEARCH
- );
+ this._showDocumentationSearch = view === GerritView.DOCUMENTATION_SEARCH;
if (
this.params &&
isAppElementJustRegisteredParams(this.params) &&
@@ -567,7 +601,7 @@
if (pathname.startsWith('/c/') && Number(hash) > 0) {
pathname += '@' + hash;
}
- this.set('_path', pathname);
+ this._path = pathname;
}
_updateLoginUrl() {
@@ -602,7 +636,7 @@
const params = paramsRecord.base;
const viewsToCheck = [GerritView.SEARCH, GerritView.DASHBOARD];
if (params?.view && viewsToCheck.includes(params.view)) {
- this.set('_lastSearchPage', location.pathname);
+ this._lastSearchPage = location.pathname;
}
}
diff --git a/polygerrit-ui/app/elements/gr-app-element_html.ts b/polygerrit-ui/app/elements/gr-app-element_html.ts
index 81a9988..a1e6ac9 100644
--- a/polygerrit-ui/app/elements/gr-app-element_html.ts
+++ b/polygerrit-ui/app/elements/gr-app-element_html.ts
@@ -131,7 +131,9 @@
view-state="{{_viewState.dashboardView}}"
></gr-dashboard-view>
</template>
- <template is="dom-if" if="[[_showChangeView]]" restamp="true">
+ <!-- Note that the change view does not have restamp="true" set, because we
+ want to re-use it as long as the change number does not change. -->
+ <template id="dom-if-change-view" is="dom-if" if="[[_showChangeView]]">
<gr-change-view
params="[[params]]"
view-state="{{_viewState.changeView}}"
@@ -141,7 +143,9 @@
<template is="dom-if" if="[[_showEditorView]]" restamp="true">
<gr-editor-view params="[[params]]"></gr-editor-view>
</template>
- <template is="dom-if" if="[[_showDiffView]]" restamp="true">
+ <!-- Note that the diff view does not have restamp="true" set, because we
+ want to re-use it as long as the change number does not change. -->
+ <template id="dom-if-diff-view" is="dom-if" if="[[_showDiffView]]">
<gr-diff-view
params="[[params]]"
change-view-state="{{_viewState.changeView}}"
diff --git a/polygerrit-ui/app/elements/gr-app-types.ts b/polygerrit-ui/app/elements/gr-app-types.ts
index 2ae85f7..4d60274 100644
--- a/polygerrit-ui/app/elements/gr-app-types.ts
+++ b/polygerrit-ui/app/elements/gr-app-types.ts
@@ -106,6 +106,16 @@
leftSide?: boolean;
commentLink?: boolean;
}
+
+export interface AppElementDiffEditViewParam {
+ view: GerritView.EDIT;
+ changeNum: NumericChangeId;
+ project: RepoName;
+ path: string;
+ patchNum: RevisionPatchSetNum;
+ lineNum?: number;
+}
+
export interface AppElementChangeViewParams {
view: GerritView.CHANGE;
changeNum: NumericChangeId;
@@ -138,6 +148,7 @@
| AppElementSettingsParam
| AppElementAgreementParam
| AppElementDiffViewParam
+ | AppElementDiffEditViewParam
| AppElementJustRegisteredParams;
export function isAppElementJustRegisteredParams(
diff --git a/polygerrit-ui/app/elements/shared/gr-account-list/gr-account-list.ts b/polygerrit-ui/app/elements/shared/gr-account-list/gr-account-list.ts
index 1e54f69..231fc36 100644
--- a/polygerrit-ui/app/elements/shared/gr-account-list/gr-account-list.ts
+++ b/polygerrit-ui/app/elements/shared/gr-account-list/gr-account-list.ts
@@ -26,6 +26,7 @@
Suggestion,
AccountInfo,
GroupInfo,
+ EmailAddress,
} from '../../../types/common';
import {
GrReviewerSuggestionsProvider,
@@ -37,6 +38,7 @@
import {PolymerDeepPropertyChange} from '@polymer/polymer/interfaces';
import {PaperInputElementExt} from '../../../types/types';
import {fireAlert} from '../../../utils/event-util';
+import {accountOrGroupKey} from '../../../utils/account-util';
const VALID_EMAIL_ALERT = 'Please input a valid email.';
@@ -184,6 +186,8 @@
reporting: ReportingService;
+ private pendingRemoval: Set<AccountInput> = new Set();
+
constructor() {
super();
this.reporting = appContext.reportingService;
@@ -233,6 +237,7 @@
let itemTypeAdded = 'unknown';
if (isAccountObject(item)) {
const account = {...item.account, _pendingAdd: true};
+ this.removeFromPendingRemoval(account);
this.push('accounts', account);
itemTypeAdded = 'account';
} else if (isGroupObjectInput(item)) {
@@ -242,6 +247,7 @@
}
const group = {...item.group, _pendingAdd: true, _group: true};
this.push('accounts', group);
+ this.removeFromPendingRemoval(group);
itemTypeAdded = 'group';
} else if (this.allowAnyInput) {
if (!item.includes('@')) {
@@ -251,8 +257,9 @@
fireAlert(this, VALID_EMAIL_ALERT);
return false;
} else {
- const account = {email: item, _pendingAdd: true};
+ const account = {email: item as EmailAddress, _pendingAdd: true};
this.push('accounts', account);
+ this.removeFromPendingRemoval(account);
itemTypeAdded = 'email';
}
}
@@ -283,32 +290,16 @@
return classes.join(' ');
}
- _accountMatches(a: AccountInput, b: AccountInput) {
- // TODO(TS): seems a & b always exists ?
- if (a && b) {
- // both conditions are checking against AccountInfo
- // and only check a not b.. typeguard won't work very good without
- // changing logic, so keep it as inline casting
- if ((a as AccountInfoInput)._account_id) {
- return (
- (a as AccountInfoInput)._account_id ===
- (b as AccountInfoInput)._account_id
- );
- }
- if ((a as AccountInfoInput).email) {
- return (a as AccountInfoInput).email === (b as AccountInfoInput).email;
- }
- }
- return a === b;
- }
-
_computeRemovable(account: AccountInput, readonly: boolean) {
if (readonly) {
return false;
}
if (this.removableValues) {
for (let i = 0; i < this.removableValues.length; i++) {
- if (this._accountMatches(this.removableValues[i], account)) {
+ if (
+ accountOrGroupKey(this.removableValues[i]) ===
+ accountOrGroupKey(account)
+ ) {
return true;
}
}
@@ -328,16 +319,9 @@
return;
}
for (let i = 0; i < this.accounts.length; i++) {
- let matches;
- const account = this.accounts[i];
- if (toRemove._group) {
- matches =
- (toRemove as GroupInfoInput).id === (account as GroupInfoInput).id;
- } else {
- matches = this._accountMatches(toRemove, account);
- }
- if (matches) {
+ if (accountOrGroupKey(toRemove) === accountOrGroupKey(this.accounts[i])) {
this.splice('accounts', i, 1);
+ this.pendingRemoval.add(toRemove);
this.reporting.reportInteraction(`Remove from ${this.id}`);
return;
}
@@ -445,6 +429,26 @@
});
}
+ removals(): AccountAddition[] {
+ return Array.from(this.pendingRemoval).map(account => {
+ if (isGroupInfoInput(account)) {
+ return {group: account};
+ } else if (isAccountInfoInput(account)) {
+ return {account};
+ } else {
+ throw new Error('AccountInput must be either Account or Group.');
+ }
+ });
+ }
+
+ removeFromPendingRemoval(account: AccountInput) {
+ this.pendingRemoval.delete(account);
+ }
+
+ clearPendingRemovals() {
+ this.pendingRemoval.clear();
+ }
+
_computeEntryHidden(
maxCount: number,
accountsRecord: PolymerDeepPropertyChange<AccountInput[], AccountInput[]>,
diff --git a/polygerrit-ui/app/elements/shared/gr-account-list/gr-account-list_test.js b/polygerrit-ui/app/elements/shared/gr-account-list/gr-account-list_test.js
index 20e8672..693f4cb 100644
--- a/polygerrit-ui/app/elements/shared/gr-account-list/gr-account-list_test.js
+++ b/polygerrit-ui/app/elements/shared/gr-account-list/gr-account-list_test.js
@@ -441,19 +441,6 @@
});
});
- test('_accountMatches', () => {
- const acct = makeAccount();
-
- assert.isTrue(element._accountMatches(acct, acct));
- acct.email = 'test';
- assert.isTrue(element._accountMatches(acct, acct));
- assert.isTrue(element._accountMatches({email: 'test'}, acct));
-
- assert.isFalse(element._accountMatches({}, acct));
- assert.isFalse(element._accountMatches({email: 'test2'}, acct));
- assert.isFalse(element._accountMatches({_account_id: -1}, acct));
- });
-
suite('keyboard interactions', () => {
test('backspace at text input start removes last account', async () => {
const input = element.$.entry.$.input;
diff --git a/polygerrit-ui/app/mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin.ts b/polygerrit-ui/app/mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin.ts
index 837a795..62589f8 100644
--- a/polygerrit-ui/app/mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin.ts
+++ b/polygerrit-ui/app/mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin.ts
@@ -102,7 +102,7 @@
import {dedupingMixin} from '@polymer/polymer/lib/utils/mixin';
import {property} from '@polymer/decorators';
import {PolymerElement} from '@polymer/polymer';
-import {Constructor} from '../../utils/common-util';
+import {check, Constructor} from '../../utils/common-util';
import {getKeyboardEvent, isModifierPressed} from '../../utils/dom-util';
import {
CustomKeyboardEvent,
@@ -222,11 +222,6 @@
viewMap?: Map<ShortcutSection, SectionView>
) => void;
-interface ShortcutEnabledElement extends PolymerElement {
- // TODO: should replace with Map so we can have proper type here
- keyboardShortcuts(): {[shortcut: string]: string};
-}
-
interface ShortcutHelpItem {
shortcut: Shortcut;
text: string;
@@ -558,14 +553,9 @@
return this.bindings.get(shortcut);
}
- attachHost(host: PolymerElement | ShortcutEnabledElement) {
- if (!('keyboardShortcuts' in host)) {
- return;
- }
- const shortcuts = host.keyboardShortcuts();
- this.activeHosts.set(host, new Map(Object.entries(shortcuts)));
+ attachHost(host: PolymerElement, shortcuts: Map<string, string>) {
+ this.activeHosts.set(host, shortcuts);
this.notifyListeners();
- return shortcuts;
}
detachHost(host: PolymerElement) {
@@ -788,6 +778,12 @@
private readonly restApiService = appContext.restApiService;
+ /** Used to disable shortcuts when the element is not visible. */
+ private observer?: IntersectionObserver;
+
+ /** Are shortcuts currently enabled? True only when element is visible. */
+ private bindingsEnabled = false;
+
modifierPressed(event: CustomKeyboardEvent) {
/* We are checking for g/v as modifiers pressed. There are cases such as
* pressing v and then /, where we want the handler for / to be triggered.
@@ -902,21 +898,62 @@
/** @override */
connectedCallback() {
super.connectedCallback();
-
this.restApiService.getPreferences().then(prefs => {
if (prefs?.disable_keyboard_shortcuts) {
this._disableKeyboardShortcuts = true;
}
});
+ this.createVisibilityObserver();
+ this.enableBindings();
+ }
- const shortcuts = shortcutManager.attachHost(this);
- if (!shortcuts) {
- return;
- }
+ /** @override */
+ disconnectedCallback() {
+ this.destroyVisibilityObserver();
+ this.disableBindings();
+ super.disconnectedCallback();
+ }
- for (const key of Object.keys(shortcuts)) {
- // TODO(TS): not needed if convert shortcuts to Map
- this._addOwnKeyBindings(key as Shortcut, shortcuts[key]);
+ /**
+ * Creates an intersection observer that enables bindings when the
+ * element is visible and disables them when the element is hidden.
+ */
+ private createVisibilityObserver() {
+ if (!this.hasKeyboardShortcuts()) return;
+ if (this.observer) return;
+ this.observer = new IntersectionObserver(entries => {
+ check(entries.length === 1, 'Expected one observer entry.');
+ const isVisible = entries[0].isIntersecting;
+ if (isVisible) {
+ this.enableBindings();
+ } else {
+ this.disableBindings();
+ }
+ });
+ this.observer.observe(this);
+ }
+
+ private destroyVisibilityObserver() {
+ if (this.observer) this.observer.unobserve(this);
+ }
+
+ /**
+ * Enables all the shortcuts returned by keyboardShortcuts().
+ * This is a private method being called when the element becomes
+ * connected or visible.
+ */
+ private enableBindings() {
+ if (!this.hasKeyboardShortcuts()) return;
+ if (this.bindingsEnabled) return;
+ this.bindingsEnabled = true;
+
+ const shortcuts = new Map<string, string>(
+ Object.entries(this.keyboardShortcuts())
+ );
+ shortcutManager.attachHost(this, shortcuts);
+
+ for (const [key, value] of shortcuts.entries()) {
+ this._addOwnKeyBindings(key as Shortcut, value);
}
// each component that uses this behaviour must be aware if go key is
@@ -941,12 +978,21 @@
}
}
- /** @override */
- disconnectedCallback() {
+ /**
+ * Disables all the shortcuts returned by keyboardShortcuts().
+ * This is a private method being called when the element becomes
+ * disconnected or invisible.
+ */
+ private disableBindings() {
+ if (!this.bindingsEnabled) return;
+ this.bindingsEnabled = false;
if (shortcutManager.detachHost(this)) {
this.removeOwnKeyBindings();
}
- super.disconnectedCallback();
+ }
+
+ private hasKeyboardShortcuts() {
+ return Object.entries(this.keyboardShortcuts()).length > 0;
}
keyboardShortcuts() {
diff --git a/polygerrit-ui/app/mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin_test.js b/polygerrit-ui/app/mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin_test.js
index b22b8d8..d5e7fe7 100644
--- a/polygerrit-ui/app/mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin_test.js
+++ b/polygerrit-ui/app/mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin_test.js
@@ -181,13 +181,7 @@
mapToObject(mgr.activeShortcutsBySection()),
{});
- mgr.attachHost({
- keyboardShortcuts() {
- return {
- [Shortcut.NEXT_FILE]: null,
- };
- },
- });
+ mgr.attachHost({}, new Map([[Shortcut.NEXT_FILE, null]]));
assert.deepEqual(
mapToObject(mgr.activeShortcutsBySection()),
{
@@ -196,13 +190,7 @@
],
});
- mgr.attachHost({
- keyboardShortcuts() {
- return {
- [Shortcut.NEXT_LINE]: null,
- };
- },
- });
+ mgr.attachHost({}, new Map([[Shortcut.NEXT_LINE, null]]));
assert.deepEqual(
mapToObject(mgr.activeShortcutsBySection()),
{
@@ -214,14 +202,10 @@
],
});
- mgr.attachHost({
- keyboardShortcuts() {
- return {
- [Shortcut.SEARCH]: null,
- [Shortcut.GO_TO_OPENED_CHANGES]: null,
- };
- },
- });
+ mgr.attachHost({}, new Map([
+ [Shortcut.SEARCH, null],
+ [Shortcut.GO_TO_OPENED_CHANGES, null],
+ ]));
assert.deepEqual(
mapToObject(mgr.activeShortcutsBySection()),
{
@@ -254,17 +238,13 @@
assert.deepEqual(mapToObject(mgr.directoryView()), {});
- mgr.attachHost({
- keyboardShortcuts() {
- return {
- [Shortcut.GO_TO_OPENED_CHANGES]: null,
- [Shortcut.NEXT_FILE]: null,
- [Shortcut.NEXT_LINE]: null,
- [Shortcut.SAVE_COMMENT]: null,
- [Shortcut.SEARCH]: null,
- };
- },
- });
+ mgr.attachHost({}, new Map([
+ [Shortcut.GO_TO_OPENED_CHANGES, null],
+ [Shortcut.NEXT_FILE, null],
+ [Shortcut.NEXT_LINE, null],
+ [Shortcut.SAVE_COMMENT, null],
+ [Shortcut.SEARCH, null],
+ ]));
assert.deepEqual(
mapToObject(mgr.directoryView()),
{
diff --git a/polygerrit-ui/app/package.json b/polygerrit-ui/app/package.json
index d250537..6d5e475 100644
--- a/polygerrit-ui/app/package.json
+++ b/polygerrit-ui/app/package.json
@@ -36,7 +36,7 @@
"@webcomponents/webcomponentsjs": "^1.3.3",
"ba-linkify": "file:../../lib/ba-linkify/src/",
"codemirror-minified": "^5.60.0",
- "lit-element": "^2.4.0",
+ "lit-element": "^2.5.1",
"page": "^1.11.6",
"polymer-bridges": "file:../../polymer-bridges/",
"polymer-resin": "^2.0.1",
diff --git a/polygerrit-ui/app/services/checks/checks-util.ts b/polygerrit-ui/app/services/checks/checks-util.ts
index 15841f3..0a0881b 100644
--- a/polygerrit-ui/app/services/checks/checks-util.ts
+++ b/polygerrit-ui/app/services/checks/checks-util.ts
@@ -79,10 +79,11 @@
if (hasResultsOf(run, Category.ERROR)) return Category.ERROR;
if (hasResultsOf(run, Category.WARNING)) return Category.WARNING;
if (hasResultsOf(run, Category.INFO)) return Category.INFO;
+ if (hasResultsOf(run, Category.SUCCESS)) return Category.SUCCESS;
return undefined;
}
-export function iconForCategory(category: Category | 'SUCCESS') {
+export function iconForCategory(category: Category) {
switch (category) {
case Category.ERROR:
return 'error';
@@ -90,7 +91,7 @@
return 'info-outline';
case Category.WARNING:
return 'warning';
- case 'SUCCESS':
+ case Category.SUCCESS:
return 'check-circle-outline';
default:
assertNever(category, `Unsupported category: ${category}`);
@@ -210,12 +211,14 @@
export function level(cat?: Category) {
if (!cat) return -1;
switch (cat) {
- case Category.INFO:
+ case Category.SUCCESS:
return 0;
- case Category.WARNING:
+ case Category.INFO:
return 1;
- case Category.ERROR:
+ case Category.WARNING:
return 2;
+ case Category.ERROR:
+ return 3;
}
}
diff --git a/polygerrit-ui/app/types/common.ts b/polygerrit-ui/app/types/common.ts
index 65095dd..30eb658 100644
--- a/polygerrit-ui/app/types/common.ts
+++ b/polygerrit-ui/app/types/common.ts
@@ -529,7 +529,7 @@
real_author?: AccountInfo;
date: Timestamp;
message: string;
- accountsInMessage?: AccountInfo[];
+ accounts_in_message?: AccountInfo[];
tag?: ReviewInputTag;
_revision_number?: PatchSetNum;
}
@@ -638,6 +638,7 @@
subject: string;
message: string;
web_links?: WebLinkInfo[];
+ resolve_conflicts_web_links?: WebLinkInfo[];
}
export interface CommitInfoWithRequiredCommit extends CommitInfo {
diff --git a/polygerrit-ui/app/types/events.ts b/polygerrit-ui/app/types/events.ts
index 2612131..3a61437 100644
--- a/polygerrit-ui/app/types/events.ts
+++ b/polygerrit-ui/app/types/events.ts
@@ -34,6 +34,8 @@
OPEN_FIX_PREVIEW = 'open-fix-preview',
CLOSE_FIX_PREVIEW = 'close-fix-preview',
PAGE_ERROR = 'page-error',
+ RECREATE_CHANGE_VIEW = 'recreate-change-view',
+ RECREATE_DIFF_VIEW = 'recreate-diff-view',
RELOAD = 'reload',
REPLY = 'reply',
SERVER_ERROR = 'server-error',
diff --git a/polygerrit-ui/app/utils/account-util.ts b/polygerrit-ui/app/utils/account-util.ts
index bb9f328..ff37608 100644
--- a/polygerrit-ui/app/utils/account-util.ts
+++ b/polygerrit-ui/app/utils/account-util.ts
@@ -15,8 +15,16 @@
* limitations under the License.
*/
-import {AccountId, AccountInfo, EmailAddress} from '../types/common';
+import {
+ AccountId,
+ AccountInfo,
+ EmailAddress,
+ GroupInfo,
+ isAccount,
+ isGroup,
+} from '../types/common';
import {AccountTag} from '../constants/constants';
+import {assertNever} from './common-util';
export function accountKey(account: AccountInfo): AccountId | EmailAddress {
if (account._account_id) return account._account_id;
@@ -40,6 +48,12 @@
return account?.avatars?.[0]?.url === other?.avatars?.[0]?.url;
}
+export function accountOrGroupKey(entry: AccountInfo | GroupInfo) {
+ if (isAccount(entry)) return accountKey(entry);
+ if (isGroup(entry)) return entry.id;
+ assertNever(entry, 'entry must be account or group');
+}
+
export function uniqueDefinedAvatar(
account: AccountInfo,
index: number,
diff --git a/polygerrit-ui/app/yarn.lock b/polygerrit-ui/app/yarn.lock
index fa35f288..543017a 100644
--- a/polygerrit-ui/app/yarn.lock
+++ b/polygerrit-ui/app/yarn.lock
@@ -428,17 +428,17 @@
resolved "https://registry.yarnpkg.com/isarray/-/isarray-0.0.1.tgz#8a18acfca9a8f4177e09abfc6038939b05d1eedf"
integrity sha1-ihis/Kmo9Bd+Cav8YDiTmwXR7t8=
-lit-element@^2.4.0:
- version "2.4.0"
- resolved "https://registry.yarnpkg.com/lit-element/-/lit-element-2.4.0.tgz#b22607a037a8fc08f5a80736dddf7f3f5d401452"
- integrity sha512-pBGLglxyhq/Prk2H91nA0KByq/hx/wssJBQFiYqXhGDvEnY31PRGYf1RglVzyLeRysu0IHm2K0P196uLLWmwFg==
+lit-element@^2.5.1:
+ version "2.5.1"
+ resolved "https://registry.yarnpkg.com/lit-element/-/lit-element-2.5.1.tgz#3fa74b121a6cd22902409ae3859b7847d01aa6b6"
+ integrity sha512-ogu7PiJTA33bEK0xGu1dmaX5vhcRjBXCFexPja0e7P7jqLhTpNKYRPmE+GmiCaRVAbiQKGkUgkh/i6+bh++dPQ==
dependencies:
lit-html "^1.1.1"
lit-html@^1.1.1:
- version "1.3.0"
- resolved "https://registry.yarnpkg.com/lit-html/-/lit-html-1.3.0.tgz#c80f3cc5793a6dea6c07172be90a70ab20e56034"
- integrity sha512-0Q1bwmaFH9O14vycPHw8C/IeHMk/uSDldVLIefu/kfbTBGIc44KGH6A8p1bDfxUfHdc8q6Ct7kQklWoHgr4t1Q==
+ version "1.4.1"
+ resolved "https://registry.yarnpkg.com/lit-html/-/lit-html-1.4.1.tgz#0c6f3ee4ad4eb610a49831787f0478ad8e9ae5e0"
+ integrity sha512-B9btcSgPYb1q4oSOb/PrOT6Z/H+r6xuNzfH4lFli/AWhYwdtrgQkQWBbIc6mdnf6E2IL3gDXdkkqNktpU0OZQA==
page@^1.11.6:
version "1.11.6"
diff --git a/tools/download_file.py b/tools/download_file.py
index 7caeb5d..2af2c07 100755
--- a/tools/download_file.py
+++ b/tools/download_file.py
@@ -17,7 +17,7 @@
import argparse
from hashlib import sha1
-from os import link, makedirs, path, remove
+from os import environ, link, makedirs, path, remove
import shutil
from subprocess import check_call, CalledProcessError
from sys import stderr
@@ -25,7 +25,10 @@
from zipfile import ZipFile, BadZipfile, LargeZipFile
GERRIT_HOME = path.expanduser('~/.gerritcodereview')
-CACHE_DIR = path.join(GERRIT_HOME, 'bazel-cache', 'downloaded-artifacts')
+CACHE_DIR = environ.get(
+ 'GERRIT_CACHE_HOME',
+ path.join(GERRIT_HOME, 'bazel-cache', 'downloaded-artifacts'))
+
LOCAL_PROPERTIES = 'local.properties'
