Merge changes Iad9c0692,I4af5946c,I9d8091c3,Ie2713ebd,I4d347d65, ...
* changes:
Document ported comments endpoints
Only port unresolved comment threads
Don't reimplement comment threads for attention set
Extract and rewrite the logic for comment threads
PortedCommentsIT: Harden a test against execution speed issues
Allow comments created via the test API to have a specific creation time
diff --git a/.bazelversion b/.bazelversion
index 47b322c..1545d96 100644
--- a/.bazelversion
+++ b/.bazelversion
@@ -1 +1 @@
-3.4.1
+3.5.0
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index d96ae46..6b89d67 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -2828,7 +2828,8 @@
+
Enable (or disable) the `'$site_path'/logs/httpd_log` request log.
If enabled, an NCSA combined log format request log file is written
-out by the internal HTTP daemon.
+out by the internal HTTP daemon. The httpd log format is documented
+link:logs.html#_httpd_log[here].
+
`log4j.appender` with the name `httpd_log` can be configured to overwrite
programmatic configuration.
@@ -4881,6 +4882,21 @@
+
By default, 30s.
+[[sshd.gracefulStopTimeout]]sshd.gracefulStopTimeout::
++
+Set a graceful stop time. If set, Gerrit ensures that all open SSH
+sessions are preserved for a maximum period of time, before forcing the
+shutdown of the SSH daemon. During this period, no new requests
+will be accepted. This option is meant to be used in setups performing
+rolling restarts.
++
+Values should use common unit suffixes to express their setting:
++
+* s, sec, second, seconds
+* m, min, minute, minutes
++
+By default, 0 seconds (immediate shutdown).
+
[[sshd.maxConnectionsPerUser]]sshd.maxConnectionsPerUser::
+
Maximum number of concurrent SSH sessions that a user account
@@ -5006,6 +5022,7 @@
+
Enable (or disable) the `'$site_path'/logs/sshd_log` request log.
If enabled, a request log file is written out by the SSH daemon.
+The sshd log format is documented link:logs.html#_sshd_log[here].
+
`log4j.appender` with the name `sshd_log` can be configured to overwrite
programmatic configuration.
diff --git a/Documentation/config-reverseproxy.txt b/Documentation/config-reverseproxy.txt
index eff777b..3a9bcc7 100644
--- a/Documentation/config-reverseproxy.txt
+++ b/Documentation/config-reverseproxy.txt
@@ -21,6 +21,13 @@
listenUrl = proxy-http://127.0.0.1:8081/r/
----
+== Reverse proxy and client IPs
+
+When behind a reverse proxy the http_log will log the IP of the reverse proxy
+as client.ip. To log the correct client IP you must provide the
+'X-Forwarded-For' header from the reverse proxy.
+See the nginx configuration example below.
+
== Apache 2 Configuration
diff --git a/Documentation/dev-bazel.txt b/Documentation/dev-bazel.txt
index 01cd494..ee2f4a1 100644
--- a/Documentation/dev-bazel.txt
+++ b/Documentation/dev-bazel.txt
@@ -90,12 +90,12 @@
```
$ cat << EOF > ~/.bazelrc
-> build --define=ABSOLUTE_JAVABASE=<path-to-java-13>
-> build --javabase=@bazel_tools//tools/jdk:absolute_javabase
-> build --host_javabase=@bazel_tools//tools/jdk:absolute_javabase
-> build --host_java_toolchain=@bazel_tools//tools/jdk:toolchain_vanilla
-> build --java_toolchain=@bazel_tools//tools/jdk:toolchain_vanilla
-> EOF
+build --define=ABSOLUTE_JAVABASE=<path-to-java-13>
+build --javabase=@bazel_tools//tools/jdk:absolute_javabase
+build --host_javabase=@bazel_tools//tools/jdk:absolute_javabase
+build --host_java_toolchain=@bazel_tools//tools/jdk:toolchain_vanilla
+build --java_toolchain=@bazel_tools//tools/jdk:toolchain_vanilla
+EOF
```
Now, invoking Bazel with just `bazel build :release` would include
diff --git a/Documentation/dev-e2e-tests.txt b/Documentation/dev-e2e-tests.txt
index 78b2c15..c5b3bfc 100644
--- a/Documentation/dev-e2e-tests.txt
+++ b/Documentation/dev-e2e-tests.txt
@@ -27,14 +27,33 @@
leveraged to run tests at the Git protocol level.
Gatling is written in Scala, but the abstraction provided by the Gatling DSL makes the scenarios
-implementation easy even without any Scala knowledge. The
-link:https://gitenterprise.me/2019/12/20/stress-your-gerrit-with-gatling/[Stress your Gerrit with Gatling,role=external,window=_blank]
-blog post has more introductory information.
+implementation easy even without any Scala knowledge. The online `End-to-end tests`
+link:https://www.gerritcodereview.com/presentations.html#list-of-presentations[presentation,role=external,window=_blank]
+links posted on the homepage have more introductory information.
+
+== IDE: IntelliJ
Examples of scenarios can be found in the `e2e-tests` directory. The files in that directory should
be formatted using the mainstream
link:https://plugins.jetbrains.com/plugin/1347-scala[Scala plugin for IntelliJ,role=external,window=_blank].
The latter is not mandatory but preferred for `sbt` and Scala IDE purposes in this project.
+So, Eclipse can also be used alongside as a development IDE; this is described below.
+
+=== Eclipse
+
+1. Install the link:http://scala-ide.org/docs/user/gettingstarted.html[Scala plugin for Eclipse,role=external,window=_blank].
+1. Run `sbt eclipse` from the `e2e-tests` root directory.
+1. Import the resulting `e2e-tests` eclipse file inside the Gerrit project, in Eclipse.
+1. You should see errors in Eclipse telling you there are missing packages.
+1. This is due to the sbt-eclipse plugin not properly linking the Gerrit Gatling e2e tests with
+ Gatling Git plugin.
+1. You then have to right-click on the root directory and choose the build path->link source option.
+1. Then you have to browse to `.sbt/1.0/staging`, find the folder where gatling-git is contained,
+ and choose that.
+1. That last step should link the gatling-git plugin to the project; e2e tests should not show
+ errors anymore.
+1. You may get errors in the gatling-git directory; these should not affect Gerrit Gatling
+ development and can be ignored.
== How to build the tests
@@ -163,10 +182,11 @@
Plugin or otherwise non-core scenarios may do so just as well. The core java package
`com.google.gerrit.scenarios` from the example above has to be replaced with the one under which
those scenario classes are. Such extending scenarios can also add extension-specific properties.
-Early examples of this can be found in the Gerrit
-`link:https://gerrit.googlesource.com/plugins/high-availability[high-availability,role=external,window=_blank]`
-and `link:https://gerrit.googlesource.com/plugins/multi-site[multi-site,role=external,window=_blank]`
-plugins test code.
+Examples of this can be found in these Gerrit plugins test code:
+
+* `link:https://gerrit.googlesource.com/plugins/high-availability[high-availability,role=external,window=_blank]`
+* `link:https://gerrit.googlesource.com/plugins/multi-site[multi-site,role=external,window=_blank]`
+* `link:https://gerrit.googlesource.com/plugins/rename-project[rename-project,role=external,window=_blank]`
Further above, the `_PROJECT` keyword is prefixed with an underscore, which means that its value
gets automatically generated by the scenario. Any property setting for it is therefore not
diff --git a/Documentation/dev-eclipse.txt b/Documentation/dev-eclipse.txt
index 9596a55..742cf42 100644
--- a/Documentation/dev-eclipse.txt
+++ b/Documentation/dev-eclipse.txt
@@ -41,6 +41,37 @@
Filters on a folder, they will be overwritten the next time you run
`tools/eclipse/project.py`.
+=== Eclipse project on MacOS
+
+By default, bazel uses `/private/var/tmp` as the
+link:https://docs.bazel.build/versions/master/output_directories.html[outputRoot on MacOS].
+This means that the eclipse project will reference libraries stored under that directory.
+However, MacOS runs periodic cleanup task which deletes the content under `/private/var/tmp`
+which wasn't accessed or modified for some days, by default 3 days. This can lead to a broken
+Eclipse project as referenced libraries get deleted.
+
+There are two possibilities to mitigate this issue.
+
+==== Change the location of the bazel output directory
+On Linux, the output directory defaults to `$HOME/.cache/bazel` and the same can be configured
+on Mac too. Edit, or create, the `$HOME/.bazelrc` file and add the following line:
+----
+startup --output_user_root=/Users/johndoe/.cache/bazel
+----
+
+==== Increase the treshold for the cleanup of temporary files
+The default treshold for the cleanup can be overriden by creating a configuration file under
+`/etc/periodic.conf` and setting a larger value for the `daily_clean_tmps_days`.
+
+An example `/etc/periodic.conf` file:
+
+----
+# This file overrides the settings from /etc/defaults/periodic.conf
+daily_clean_tmps_days="45" # If not accessed for
+----
+
+For more details about the proposed workaround see link:https://superuser.com/a/187105[this post]
+
=== Eclipse project with custom plugins ===
To add custom plugins to the eclipse project add them to `tools/bzl/plugins.bzl`
diff --git a/Documentation/images/inline-edit-create-change-project-screen-dialog.png b/Documentation/images/inline-edit-create-change-project-screen-dialog.png
deleted file mode 100644
index ea5daa9..0000000
--- a/Documentation/images/inline-edit-create-change-project-screen-dialog.png
+++ /dev/null
Binary files differ
diff --git a/Documentation/images/inline-edit-create-change-project-screen.png b/Documentation/images/inline-edit-create-change-project-screen.png
deleted file mode 100644
index e9c7033..0000000
--- a/Documentation/images/inline-edit-create-change-project-screen.png
+++ /dev/null
Binary files differ
diff --git a/Documentation/images/inline-edit-create-follow-up-change.png b/Documentation/images/inline-edit-create-follow-up-change.png
deleted file mode 100644
index 3e81eee..0000000
--- a/Documentation/images/inline-edit-create-follow-up-change.png
+++ /dev/null
Binary files differ
diff --git a/Documentation/images/inline-edit-edit-in-diff-screen-patch-list.png b/Documentation/images/inline-edit-edit-in-diff-screen-patch-list.png
deleted file mode 100644
index bdbc59d..0000000
--- a/Documentation/images/inline-edit-edit-in-diff-screen-patch-list.png
+++ /dev/null
Binary files differ
diff --git a/Documentation/images/inline-edit-edit-in-patch-list.png b/Documentation/images/inline-edit-edit-in-patch-list.png
deleted file mode 100644
index 9a31e02..0000000
--- a/Documentation/images/inline-edit-edit-in-patch-list.png
+++ /dev/null
Binary files differ
diff --git a/Documentation/index.txt b/Documentation/index.txt
index 4fb977a..8f36ecc 100644
--- a/Documentation/index.txt
+++ b/Documentation/index.txt
@@ -69,6 +69,7 @@
. link:cmd-index.html[Command Line Tools]
. link:config-plugins.html#replication[Replication]
. link:config-plugins.html[Plugins]
+. link:logs.html[Log Files]
. link:metrics.html[Metrics]
. link:config-reverseproxy.html[Reverse Proxy]
. link:config-auto-site-initialization.html[Automatic Site Initialization on Startup]
diff --git a/Documentation/logs.txt b/Documentation/logs.txt
new file mode 100644
index 0000000..43a8e62
--- /dev/null
+++ b/Documentation/logs.txt
@@ -0,0 +1,174 @@
+= Gerrit Code Review - Logs
+
+Gerrit writes log files in the `$site_path/logs/` folder tracking requests,
+background and plugin activity and errors. By default logs are written in
+link:config-gerrit.html#log.textLogging[text format], optionally in
+link:config-gerrit.html#log.jsonLogging[JSON format].
+By default log files are link:config-gerrit.html#log.compress[compressed]
+at server startup and then daily at 11pm and
+link:config-gerrit.html#log.rotate[rotated] every midnight.
+
+== Logs
+
+The following logs can be written.
+
+=== HTTPD Log
+
+The httpd log tracks HTTP requests processed by Gerrit's http daemon
+and is written to `$site_path/logs/httpd_log`. Enabled or disabled via the
+link:config-gerrit.html#httpd.requestLog[httpd.requestLog] option.
+
+Format is an enhanced
+link:https://httpd.apache.org/docs/2.4/logs.html#combined[NCSA combined log],
+if a log field is not present, a "-" is substituted:
+
+* `host`: The IP address of the HTTP client that made the HTTP resource request.
+ If you are using a reverse proxy it depends on the proxy configuration if the
+ proxy IP address or the client IP address is logged.
+* `[thread name]`: name of the Java thread executing the request.
+* `remote logname`: the identifier used to
+ link: https://tools.ietf.org/html/rfc1413[identify the client making the HTTP request],
+ Gerrit always logs a dash `-`.
+* `username`: the username used by the client for authentication. "-" for
+ anonymous requests.
+* `[date:time]`: The date and time stamp of the HTTP request.
+ The time that the request was received. The format is until Gerrit 3.1
+ `[dd/MMM/yyyy:HH:mm:ss.SSS ZZZZ]`. For Gerrit 3.2 or newer
+ link:https://www.w3.org/TR/NOTE-datetime[ISO 8601 format] `[yyyy-MM-dd'T'HH:mm:ss,SSSZ]`
+ is used for all timestamps.
+* `request`: The request line from the client is given in double quotes.
+** the HTTP method used by the client.
+** the resource the client requested.
+** the protocol/version used by the client.
+* `statuscode`: the link:https://tools.ietf.org/html/rfc2616#section-10[HTTP status code]
+ that the server sent back to the client.
+* `response size`: the number of bytes of data transferred as part of the HTTP
+ response, not including the HTTP header.
+* `latency`: response time in milliseconds.
+* `referer`: the `Referer` HTTP request header. This gives the site that
+ the client reports having been referred from.
+* `client agent`: the client agent which sent the request.
+
+Example:
+```
+12.34.56.78 [HTTP-4136374] - johndoe [28/Aug/2020:10:02:20 +0200] "GET /a/plugins/metrics-reporter-prometheus/metrics HTTP/1.1" 200 1247498 1900 - "Prometheus/2.13.1"
+```
+
+=== SSHD Log
+
+The sshd log tracks ssh requests processed by Gerrit's ssh daemon
+and is written to `$site_path/logs/sshd_log`. Enabled or disabled
+via option link:config-gerrit.html#sshd.requestLog[sshd.requestLog].
+
+Log format:
+
+* `[date time]`: The time that the request was received.
+ The format is until Gerrit 3.1 `[yyyy-mm-dd HH:mm:ss.SSS ZZZZ]`.
+ For Gerrit 3.2 or newer
+ link:https://www.w3.org/TR/NOTE-datetime[ISO 8601 format] `[yyyy-MM-dd'T'HH:mm:ss,SSSZ]`
+ is used for all timestamps.
+* `sessionid`: hexadecimal session identifier, all requests of the
+ same connection share the same sessionid. Gerrit does not support multiplexing multiple
+ sessions on the same connection. Grep the log file using the sessionid as filter to
+ get all requests from that session.
+* `[thread name]`: name of the Java thread executing the request.
+* `username`: the username used by the client for authentication.
+* `a/accountid`: identifier of the Gerrit account which is logged on.
+* `operation`: the operation being executed via ssh.
+** `LOGIN FROM <host>`: login and start new SSH session from the given host.
+** `AUTH FAILURE FROM <host> <message>`: failed authentication from given host and cause of failure.
+** `LOGOUT`: logout and terminate SSH session.
+** `git-upload-pack.<projectname>`: git fetch or clone command for given project.
+** `git-receive-pack.<projectname>`: git push command for given project.
+** Gerrit ssh commands which may be logged in this field are documented
+ link:cmd-index.html#_server[here].
+* `wait`: command wait time, time in milliseconds the command waited for an execution thread.
+* `exec`: command execution time, time in milliseconds to execute the command.
+* `status`: status code. 0 means success, any other value is an error.
+
+The `git-upload-pack` command provides the following additional fields after the `exec`
+and before the `status` field. All times are in milliseconds. Fields are -1 if not available
+when the upload-pack request returns an empty result since the client's repository was up to date:
+
+* `time negotiating`: time for negotiating which objects need to be transferred.
+* `time searching for reuse`: time jgit searched for deltas which can be reused.
+ That is the time spent matching existing representations against objects that
+ will be transmitted, or that the client can be assumed to already have.
+* `time searching for sizes`: time jgit was searching for sizes of all objects that
+ will enter the delta compression search window. The sizes need to
+ be known to better match similar objects together and improve
+ delta compression ratios.
+* `time counting`: time jgit spent enumerating the objects that need to
+ be included in the output. This time includes any restarts that
+ occur when a cached pack is selected for reuse.
+* `time compressing`: time jgit was compressing objects. This is observed
+ wall-clock time and does not accurately track CPU time used when
+ multiple threads were used to perform the delta compression.
+* `time writing`: time jgit needed to write packfile, from start of
+ header until end of trailer. The transfer speed can be
+ approximated by dividing `total bytes` by this value.
+* `total time in UploadPack`: total time jgit spent in upload-pack.
+* `bitmap index misses`: number of misses when trying to use bitmap index,
+ -1 means no bitmap index available. This is the count of objects that
+ needed to be discovered through an object walk because they were not found
+ in bitmap indices.
+* `total deltas`: total number of deltas transferred. This may be lower than the actual
+ number of deltas if a cached pack was reused.
+* `total objects`: total number of objects transferred. This total includes
+ the value of `total deltas`.
+* `total bytes`: total number of bytes transferred. This size includes the pack
+ header, trailer, thin pack, and reused cached packs.
+* `client agent`: the client agent and version which sent the request.
+
+Example: a CI system established a SSH connection, sent an upload-pack command (git fetch) and closed the connection:
+```
+[2020-08-28 11:00:01,391 +0200] 8a154cae [sshd-SshServer[570fc452]-nio2-thread-299] voter a/1000023 LOGIN FROM 12.34.56.78
+[2020-08-28 11:00:01,556 +0200] 8a154cae [SSH git-upload-pack /AP/ajs/jpaas-msg-svc.git (voter)] voter a/1000056 git-upload-pack./demo/project.git 0ms 115ms 92ms 1ms 0ms 6ms 0ms 0ms 7ms 3 10 26 2615 0 git/2.26.2
+[2020-08-28 11:00:01,583 +0200] 8a154cae [sshd-SshServer[570fc452]-nio2-thread-168] voter a/1000023 LOGOUT
+```
+
+=== Error Log
+
+The error log tracks errors and stack traces and is written to
+`$site_path/logs/error_log`.
+
+Log format:
+
+* `[date time]`: The time that the request was received.
+ The format is until Gerrit 3.1 `[yyyy-mm-dd HH:mm:ss.SSS ZZZZ]`.
+ For Gerrit 3.2 or newer
+ link:https://www.w3.org/TR/NOTE-datetime[ISO 8601 format] `[yyyy-MM-dd'T'HH:mm:ss,SSSZ]`
+ is used for all timestamps.
+* `[thread name]`: : name of the Java thread executing the request.
+* `level`: log level (ERROR, WARN, INFO, DEBUG).
+* `logger`: name of the logger.
+* `message`: log message.
+* `stacktrace`: Java stacktrace when an execption was caught, usually spans multiple lines.
+
+=== GC Log
+
+The gc log tracks git garbage collection running in a background thread
+if enabled and is written to `$site_path/logs/gc_log`.
+
+Log format:
+
+* `[date time]`: The time that the request was received.
+ The format is until Gerrit 3.1 `[yyyy-mm-dd HH:mm:ss.SSS ZZZZ]`.
+ For Gerrit 3.2 or newer
+ link:https://www.w3.org/TR/NOTE-datetime[ISO 8601 format] `[yyyy-MM-dd'T'HH:mm:ss,SSSZ]`
+ is used for all timestamps.
+* `level`: log level (ERROR, WARN, INFO, DEBUG).
+* `message`: log message.
+
+=== Plugin Logs
+
+Some plugins write their own log file.
+E.g. the replication plugin writes its log to `$site_path/logs/replication_log`.
+Refer to each plugin's documentation for more details on their logs.
+
+GERRIT
+------
+Part of link:index.html[Gerrit Code Review]
+
+SEARCHBOX
+---------
diff --git a/WORKSPACE b/WORKSPACE
index 6be35cf..4c2fe35 100644
--- a/WORKSPACE
+++ b/WORKSPACE
@@ -219,15 +219,15 @@
sha1 = GUAVA_BIN_SHA1,
)
-CAFFEINE_VERS = "2.8.0"
+CAFFEINE_VERS = "2.8.5"
maven_jar(
name = "caffeine",
artifact = "com.github.ben-manes.caffeine:caffeine:" + CAFFEINE_VERS,
- sha1 = "6000774d7f8412ced005a704188ced78beeed2bb",
+ sha1 = "f0eafef6e1529a44e36549cd9d1fc06d3a57f384",
)
-CAFFEINE_GUAVA_SHA256 = "3a66ee3ec70971dee0bae6e56bda7b8742bc4bedd7489161bfbbaaf7137d89e1"
+CAFFEINE_GUAVA_SHA256 = "a7ce6d29c40bccd688815a6734070c55b20cd326351a06886a6144005aa32299"
# TODO(davido): Rename guava.jar to caffeine-guava.jar on fetch to prevent potential
# naming collision between caffeine guava adapter and guava library itself.
@@ -758,8 +758,8 @@
# Keep this version of Soy synchronized with the version used in Gitiles.
maven_jar(
name = "soy",
- artifact = "com.google.template:soy:2019-10-08",
- sha1 = "4518bf8bac2dbbed684849bc209c39c4cb546237",
+ artifact = "com.google.template:soy:2020-08-24",
+ sha1 = "e774bf5cc95923d2685292883fe219e231346e50",
)
maven_jar(
diff --git a/e2e-tests/build.sbt b/e2e-tests/build.sbt
index a322970..294212c 100644
--- a/e2e-tests/build.sbt
+++ b/e2e-tests/build.sbt
@@ -2,7 +2,6 @@
enablePlugins(GatlingPlugin)
-lazy val gatlingGitExtension = RootProject(uri("git://github.com/GerritForge/gatling-git.git"))
lazy val root = (project in file("."))
.settings(
inThisBuild(List(
@@ -12,8 +11,8 @@
)),
name := "gerrit",
libraryDependencies ++=
- gatling ++
+ gatling ++ gatlingGit ++
Seq("io.gatling" % "gatling-core" % GatlingVersion) ++
Seq("io.gatling" % "gatling-app" % GatlingVersion),
scalacOptions += "-language:postfixOps"
- ) dependsOn gatlingGitExtension
+ )
diff --git a/e2e-tests/project/Dependencies.scala b/e2e-tests/project/Dependencies.scala
index 63328f9..56ef740 100644
--- a/e2e-tests/project/Dependencies.scala
+++ b/e2e-tests/project/Dependencies.scala
@@ -2,9 +2,17 @@
object Dependencies {
val GatlingVersion = "3.2.0"
+ val GatlingGitVersion = "1.0.12"
lazy val gatling = Seq(
"io.gatling.highcharts" % "gatling-charts-highcharts",
"io.gatling" % "gatling-test-framework",
).map(_ % GatlingVersion % Test)
+
+ lazy val gatlingGit = Seq(
+ "com.gerritforge" %% "gatling-git" % GatlingGitVersion excludeAll(
+ ExclusionRule(organization = "io.gatling"),
+ ExclusionRule(organization = "io.gatling.highcharts")
+ )
+ )
}
diff --git a/e2e-tests/project/plugins.sbt b/e2e-tests/project/plugins.sbt
index 36cd201..9ed0f89 100644
--- a/e2e-tests/project/plugins.sbt
+++ b/e2e-tests/project/plugins.sbt
@@ -1 +1,2 @@
addSbtPlugin("io.gatling" % "gatling-sbt" % "3.0.0")
+addSbtPlugin("com.typesafe.sbteclipse" % "sbteclipse-plugin" % "5.2.4")
diff --git a/e2e-tests/src/test/resources/data/com/google/gerrit/scenarios/CreateProject-body.json b/e2e-tests/src/test/resources/data/com/google/gerrit/scenarios/CreateProject-body.json
index bcf4708..282ac99 100644
--- a/e2e-tests/src/test/resources/data/com/google/gerrit/scenarios/CreateProject-body.json
+++ b/e2e-tests/src/test/resources/data/com/google/gerrit/scenarios/CreateProject-body.json
@@ -1,3 +1,4 @@
{
- "create_empty_commit": "true"
+ "create_empty_commit": "true",
+ "parent": "${parent}"
}
diff --git a/e2e-tests/src/test/resources/data/com/google/gerrit/scenarios/CreateProject.json b/e2e-tests/src/test/resources/data/com/google/gerrit/scenarios/CreateProject.json
index cd90739..c141bb8 100644
--- a/e2e-tests/src/test/resources/data/com/google/gerrit/scenarios/CreateProject.json
+++ b/e2e-tests/src/test/resources/data/com/google/gerrit/scenarios/CreateProject.json
@@ -1,5 +1,6 @@
[
{
- "url": "HTTP_SCHEME://HOSTNAME:HTTP_PORT/a/projects/PROJECT"
+ "url": "HTTP_SCHEME://HOSTNAME:HTTP_PORT/a/projects/PROJECT",
+ "parent": "PARENT"
}
]
diff --git a/e2e-tests/src/test/scala/com/google/gerrit/scenarios/CreateProject.scala b/e2e-tests/src/test/scala/com/google/gerrit/scenarios/CreateProject.scala
index d631292..f2b3d12 100644
--- a/e2e-tests/src/test/scala/com/google/gerrit/scenarios/CreateProject.scala
+++ b/e2e-tests/src/test/scala/com/google/gerrit/scenarios/CreateProject.scala
@@ -28,7 +28,7 @@
val test: ScenarioBuilder = scenario(unique)
.feed(data)
- .exec(httpRequest.body(RawFileBody(body)).asJson)
+ .exec(httpRequest.body(ElFileBody(body)).asJson)
setUp(
test.inject(
diff --git a/e2e-tests/src/test/scala/com/google/gerrit/scenarios/GerritSimulation.scala b/e2e-tests/src/test/scala/com/google/gerrit/scenarios/GerritSimulation.scala
index 4832392..679320d 100644
--- a/e2e-tests/src/test/scala/com/google/gerrit/scenarios/GerritSimulation.scala
+++ b/e2e-tests/src/test/scala/com/google/gerrit/scenarios/GerritSimulation.scala
@@ -67,6 +67,8 @@
case ("number", number) =>
val precedes = replaceKeyWith("_number", 0, number.toString)
replaceProperty("number", 1, precedes)
+ case ("parent", parent) =>
+ replaceProperty("parent", "All-Projects", parent.toString)
case ("project", project) =>
var precedes = replaceKeyWith("_project", name, project.toString)
precedes = replaceOverride(precedes)
diff --git a/java/com/google/gerrit/acceptance/testsuite/change/PerPatchsetOperationsImpl.java b/java/com/google/gerrit/acceptance/testsuite/change/PerPatchsetOperationsImpl.java
index 89cfa16..465c419 100644
--- a/java/com/google/gerrit/acceptance/testsuite/change/PerPatchsetOperationsImpl.java
+++ b/java/com/google/gerrit/acceptance/testsuite/change/PerPatchsetOperationsImpl.java
@@ -131,11 +131,11 @@
return userFactory.create(authorId);
}
- /**
- * Both this and {@code toEntitiesCommentRange} is needed since there are two Comment.Range
- * entities, in different packages: {@code com.google.gerrit.entities.Comment.Range}, and {@code
- * com.google.gerrit.extensions.Comment.Range}
- */
+ private IdentifiedUser getAuthor(TestRobotCommentCreation robotCommentCreation) {
+ Account.Id authorId = robotCommentCreation.author().orElse(changeNotes.getChange().getOwner());
+ return userFactory.create(authorId);
+ }
+
private static Comment.Range toCommentRange(TestRange range) {
Comment.Range commentRange = new Range();
commentRange.startLine = range.start().line();
@@ -145,19 +145,6 @@
return commentRange;
}
- /**
- * Both this and {@code toCommentRange} is needed since there are two Comment.Range entities, in
- * different packages: {@code com.google.gerrit.entities.Comment.Range}, and {@code
- * com.google.gerrit.extensions.Comment.Range}
- */
- private static com.google.gerrit.entities.Comment.Range toEntitiesCommentRange(TestRange range) {
- return new com.google.gerrit.entities.Comment.Range(
- range.start().line(),
- range.start().charOffset(),
- range.end().line(),
- range.end().charOffset());
- }
-
private class CommentAdditionOp implements BatchUpdateOp {
private String createdCommentUuid;
private final TestCommentCreation commentCreation;
@@ -240,11 +227,6 @@
}
}
- private IdentifiedUser getAuthor(TestRobotCommentCreation robotCommentCreation) {
- Account.Id authorId = robotCommentCreation.author().orElse(changeNotes.getChange().getOwner());
- return userFactory.create(authorId);
- }
-
private class RobotCommentAdditionOp implements BatchUpdateOp {
private String createdRobotCommentUuid;
private final TestRobotCommentCreation robotCommentCreation;
diff --git a/java/com/google/gerrit/server/restapi/change/PutDraftComment.java b/java/com/google/gerrit/server/restapi/change/PutDraftComment.java
index 17a8ff4..39de43d 100644
--- a/java/com/google/gerrit/server/restapi/change/PutDraftComment.java
+++ b/java/com/google/gerrit/server/restapi/change/PutDraftComment.java
@@ -31,7 +31,6 @@
import com.google.gerrit.server.CommentsUtil;
import com.google.gerrit.server.PatchSetUtil;
import com.google.gerrit.server.change.DraftCommentResource;
-import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.patch.PatchListCache;
import com.google.gerrit.server.patch.PatchListNotAvailableException;
@@ -56,7 +55,6 @@
private final PatchSetUtil psUtil;
private final Provider<CommentJson> commentJson;
private final PatchListCache patchListCache;
- private final ChangeNotes.Factory changeNotesFactory;
@Inject
PutDraftComment(
@@ -65,15 +63,13 @@
CommentsUtil commentsUtil,
PatchSetUtil psUtil,
Provider<CommentJson> commentJson,
- PatchListCache patchListCache,
- ChangeNotes.Factory changeNotesFactory) {
+ PatchListCache patchListCache) {
this.updateFactory = updateFactory;
this.delete = delete;
this.commentsUtil = commentsUtil;
this.psUtil = psUtil;
this.commentJson = commentJson;
this.patchListCache = patchListCache;
- this.changeNotesFactory = changeNotesFactory;
}
@Override
diff --git a/java/com/google/gerrit/sshd/SshDaemon.java b/java/com/google/gerrit/sshd/SshDaemon.java
index 5145c13..c43bf91 100644
--- a/java/com/google/gerrit/sshd/SshDaemon.java
+++ b/java/com/google/gerrit/sshd/SshDaemon.java
@@ -62,7 +62,9 @@
import java.util.Iterator;
import java.util.List;
import java.util.Set;
+import java.util.concurrent.CountDownLatch;
import java.util.concurrent.ExecutorService;
+import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicInteger;
import org.apache.mina.transport.socket.SocketSessionConfig;
import org.apache.sshd.common.BaseBuilder;
@@ -72,6 +74,8 @@
import org.apache.sshd.common.compression.BuiltinCompressions;
import org.apache.sshd.common.compression.Compression;
import org.apache.sshd.common.forward.DefaultForwarderFactory;
+import org.apache.sshd.common.future.CloseFuture;
+import org.apache.sshd.common.future.SshFutureListener;
import org.apache.sshd.common.io.AbstractIoServiceFactory;
import org.apache.sshd.common.io.IoAcceptor;
import org.apache.sshd.common.io.IoServiceFactory;
@@ -142,6 +146,7 @@
private final List<HostKey> hostKeys;
private volatile IoAcceptor daemonAcceptor;
private final Config cfg;
+ private final long gracefulStopTimeout;
@Inject
SshDaemon(
@@ -212,6 +217,8 @@
SshSessionBackend backend = cfg.getEnum("sshd", null, "backend", SshSessionBackend.NIO2);
boolean channelIdTracking = cfg.getBoolean("sshd", "enableChannelIdTracking", true);
+ gracefulStopTimeout = cfg.getTimeUnit("sshd", null, "gracefulStopTimeout", 0, TimeUnit.SECONDS);
+
System.setProperty(
IoServiceFactoryFactory.class.getName(),
backend == SshSessionBackend.MINA
@@ -341,6 +348,12 @@
public synchronized void stop() {
if (daemonAcceptor != null) {
try {
+ if (gracefulStopTimeout > 0) {
+ logger.atInfo().log(
+ "Stopping SSHD sessions gracefully with %d seconds timeout.", gracefulStopTimeout);
+ daemonAcceptor.unbind(daemonAcceptor.getBoundAddresses());
+ waitForSessionClose();
+ }
daemonAcceptor.close(true).await();
shutdownExecutors();
logger.atInfo().log("Stopped Gerrit SSHD");
@@ -352,6 +365,30 @@
}
}
+ private void waitForSessionClose() {
+ Collection<IoSession> ioSessions = daemonAcceptor.getManagedSessions().values();
+ CountDownLatch allSessionsClosed = new CountDownLatch(ioSessions.size());
+ for (IoSession io : ioSessions) {
+ logger.atFine().log("Waiting for session %s to stop.", io.getId());
+ io.addCloseFutureListener(
+ new SshFutureListener<CloseFuture>() {
+ @Override
+ public void operationComplete(CloseFuture future) {
+ allSessionsClosed.countDown();
+ }
+ });
+ }
+ try {
+ if (!allSessionsClosed.await(gracefulStopTimeout, TimeUnit.SECONDS)) {
+ logger.atWarning().log(
+ "Timeout waiting for SSH session to close. SSHD will be shut down immediately.");
+ }
+ } catch (InterruptedException e) {
+ logger.atWarning().withCause(e).log(
+ "Interrupted waiting for SSH-sessions to close. SSHD will be shut down immediately.");
+ }
+ }
+
private void shutdownExecutors() {
if (executor != null) {
executor.shutdownNow();
diff --git a/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java b/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java
index 0cdff4e..4f61d79 100644
--- a/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java
@@ -58,7 +58,6 @@
import com.google.gerrit.extensions.restapi.IdString;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.TopLevelResource;
-import com.google.gerrit.server.CommentsUtil;
import com.google.gerrit.server.change.ChangeResource;
import com.google.gerrit.server.change.RevisionResource;
import com.google.gerrit.server.notedb.ChangeNoteUtil;
@@ -67,7 +66,6 @@
import com.google.gerrit.server.restapi.change.PostReview;
import com.google.gerrit.testing.FakeEmailSender;
import com.google.gerrit.testing.FakeEmailSender.Message;
-import com.google.gerrit.testing.TestCommentHelper;
import com.google.inject.Inject;
import com.google.inject.Provider;
import java.sql.Timestamp;
@@ -99,8 +97,6 @@
@Inject private ChangeOperations changeOperations;
@Inject private AccountOperations accountOperations;
@Inject private RequestScopeOperations requestScopeOperations;
- @Inject private CommentsUtil commentsUtil;
- @Inject private TestCommentHelper testCommentHelper;
private final Integer[] lines = {0, 1};
@@ -1649,12 +1645,6 @@
gApi.changes().id(changeId).revision(revision).review(input);
}
- private void addComments(String changeId, CommentInput... commentInputs) throws Exception {
- ReviewInput input = new ReviewInput();
- input.comments = Arrays.stream(commentInputs).collect(groupingBy(c -> c.path));
- gApi.changes().id(changeId).current().review(input);
- }
-
/**
* All the commits, which contain the target comment before, should still contain the comment with
* the updated message. All the other metas of the commits should be exactly the same.
@@ -1748,14 +1738,6 @@
return gApi.changes().id(changeId).revision(revId).createDraft(in).get();
}
- private CommentInfo addDraft(Change.Id changeId, String revId, DraftInput in) throws Exception {
- return gApi.changes().id(changeId.get()).revision(revId).createDraft(in).get();
- }
-
- private CommentInfo addDraft(String changeId, DraftInput in) throws Exception {
- return gApi.changes().id(changeId).current().createDraft(in).get();
- }
-
private CommentInfo addDraft(Change.Id changeId, DraftInput in) throws Exception {
return gApi.changes().id(changeId.get()).current().createDraft(in).get();
}
@@ -1765,10 +1747,6 @@
gApi.changes().id(changeId).revision(revId).draft(uuid).update(in);
}
- private void updateDraft(String changeId, DraftInput in, String uuid) throws Exception {
- gApi.changes().id(changeId).current().draft(uuid).update(in);
- }
-
private void updateDraft(Change.Id changeId, DraftInput in, String uuid) throws Exception {
gApi.changes().id(changeId.get()).current().draft(uuid).update(in);
}
diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java b/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java
index 60bf64c..1a43269 100644
--- a/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java
+++ b/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java
@@ -41,7 +41,7 @@
case V6_7:
return "blacktop/elasticsearch:6.7.2";
case V6_8:
- return "blacktop/elasticsearch:6.8.11";
+ return "blacktop/elasticsearch:6.8.12";
case V7_0:
return "blacktop/elasticsearch:7.0.1";
case V7_1:
diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticVersionTest.java b/javatests/com/google/gerrit/elasticsearch/ElasticVersionTest.java
index 9296a6b..b9d5313 100644
--- a/javatests/com/google/gerrit/elasticsearch/ElasticVersionTest.java
+++ b/javatests/com/google/gerrit/elasticsearch/ElasticVersionTest.java
@@ -72,15 +72,15 @@
public void atLeastMinorVersion() throws Exception {
assertThat(ElasticVersion.V6_7.isAtLeastMinorVersion(ElasticVersion.V6_7)).isTrue();
assertThat(ElasticVersion.V6_8.isAtLeastMinorVersion(ElasticVersion.V6_8)).isTrue();
- assertThat(ElasticVersion.V7_0.isAtLeastMinorVersion(ElasticVersion.V6_7)).isFalse();
- assertThat(ElasticVersion.V7_1.isAtLeastMinorVersion(ElasticVersion.V6_7)).isFalse();
- assertThat(ElasticVersion.V7_2.isAtLeastMinorVersion(ElasticVersion.V6_7)).isFalse();
- assertThat(ElasticVersion.V7_3.isAtLeastMinorVersion(ElasticVersion.V6_7)).isFalse();
- assertThat(ElasticVersion.V7_4.isAtLeastMinorVersion(ElasticVersion.V6_7)).isFalse();
- assertThat(ElasticVersion.V7_5.isAtLeastMinorVersion(ElasticVersion.V6_7)).isFalse();
- assertThat(ElasticVersion.V7_6.isAtLeastMinorVersion(ElasticVersion.V6_7)).isFalse();
- assertThat(ElasticVersion.V7_7.isAtLeastMinorVersion(ElasticVersion.V6_7)).isFalse();
- assertThat(ElasticVersion.V7_8.isAtLeastMinorVersion(ElasticVersion.V6_7)).isFalse();
+ assertThat(ElasticVersion.V7_0.isAtLeastMinorVersion(ElasticVersion.V6_8)).isFalse();
+ assertThat(ElasticVersion.V7_1.isAtLeastMinorVersion(ElasticVersion.V6_8)).isFalse();
+ assertThat(ElasticVersion.V7_2.isAtLeastMinorVersion(ElasticVersion.V6_8)).isFalse();
+ assertThat(ElasticVersion.V7_3.isAtLeastMinorVersion(ElasticVersion.V6_8)).isFalse();
+ assertThat(ElasticVersion.V7_4.isAtLeastMinorVersion(ElasticVersion.V6_8)).isFalse();
+ assertThat(ElasticVersion.V7_5.isAtLeastMinorVersion(ElasticVersion.V6_8)).isFalse();
+ assertThat(ElasticVersion.V7_6.isAtLeastMinorVersion(ElasticVersion.V6_8)).isFalse();
+ assertThat(ElasticVersion.V7_7.isAtLeastMinorVersion(ElasticVersion.V6_8)).isFalse();
+ assertThat(ElasticVersion.V7_8.isAtLeastMinorVersion(ElasticVersion.V6_8)).isFalse();
}
@Test
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog.js b/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog.js
deleted file mode 100644
index 9489b94..0000000
--- a/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog.js
+++ /dev/null
@@ -1,211 +0,0 @@
-/**
- * @license
- * Copyright (C) 2016 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 '../../shared/gr-dialog/gr-dialog.js';
-import '../../../styles/shared-styles.js';
-import '../../plugins/gr-endpoint-decorator/gr-endpoint-decorator.js';
-import '../../shared/gr-js-api-interface/gr-js-api-interface.js';
-import {GestureEventListeners} from '@polymer/polymer/lib/mixins/gesture-event-listeners.js';
-import {LegacyElementMixin} from '@polymer/polymer/lib/legacy/legacy-element-mixin.js';
-import {PolymerElement} from '@polymer/polymer/polymer-element.js';
-import {htmlTemplate} from './gr-confirm-revert-dialog_html.js';
-
-const ERR_COMMIT_NOT_FOUND =
- 'Unable to find the commit hash of this change.';
-const CHANGE_SUBJECT_LIMIT = 50;
-
-// TODO(dhruvsri): clean up repeated definitions after moving to js modules
-const REVERT_TYPES = {
- REVERT_SINGLE_CHANGE: 1,
- REVERT_SUBMISSION: 2,
-};
-
-/**
- * @extends PolymerElement
- */
-class GrConfirmRevertDialog extends GestureEventListeners(
- LegacyElementMixin(PolymerElement)) {
- static get template() { return htmlTemplate; }
-
- static get is() { return 'gr-confirm-revert-dialog'; }
- /**
- * Fired when the confirm button is pressed.
- *
- * @event confirm
- */
-
- /**
- * Fired when the cancel button is pressed.
- *
- * @event cancel
- */
-
- static get properties() {
- return {
- /* The revert message updated by the user
- The default value is set by the dialog */
- _message: String,
- _revertType: {
- type: Number,
- value: REVERT_TYPES.REVERT_SINGLE_CHANGE,
- },
- _showRevertSubmission: {
- type: Boolean,
- value: false,
- },
- _changesCount: Number,
- _showErrorMessage: {
- type: Boolean,
- value: false,
- },
- /* store the default revert messages per revert type so that we can
- check if user has edited the revert message or not
- Set when populate() is called */
- _originalRevertMessages: {
- type: Array,
- value() { return []; },
- },
- // Store the actual messages that the user has edited
- _revertMessages: {
- type: Array,
- value() { return []; },
- },
- };
- }
-
- _computeIfSingleRevert(revertType) {
- return revertType === REVERT_TYPES.REVERT_SINGLE_CHANGE;
- }
-
- _computeIfRevertSubmission(revertType) {
- return revertType === REVERT_TYPES.REVERT_SUBMISSION;
- }
-
- _modifyRevertMsg(change, commitMessage, message) {
- return this.$.jsAPI.modifyRevertMsg(change,
- message, commitMessage);
- }
-
- populate(change, commitMessage, changes) {
- this._changesCount = changes.length;
- // The option to revert a single change is always available
- this._populateRevertSingleChangeMessage(
- change, commitMessage, change.current_revision);
- this._populateRevertSubmissionMessage(change, changes, commitMessage);
- }
-
- _populateRevertSingleChangeMessage(change, commitMessage, commitHash) {
- // Figure out what the revert title should be.
- const originalTitle = (commitMessage || '').split('\n')[0];
- const revertTitle = `Revert "${originalTitle}"`;
- if (!commitHash) {
- this.dispatchEvent(new CustomEvent('show-alert', {
- detail: {message: ERR_COMMIT_NOT_FOUND},
- composed: true, bubbles: true,
- }));
- return;
- }
- const revertCommitText = `This reverts commit ${commitHash}.`;
-
- this._message = `${revertTitle}\n\n${revertCommitText}\n\n` +
- `Reason for revert: <INSERT REASONING HERE>\n`;
- // This is to give plugins a chance to update message
- this._message = this._modifyRevertMsg(change, commitMessage,
- this._message);
- this._revertType = REVERT_TYPES.REVERT_SINGLE_CHANGE;
- this._showRevertSubmission = false;
- this._revertMessages[this._revertType] = this._message;
- this._originalRevertMessages[this._revertType] = this._message;
- }
-
- _getTrimmedChangeSubject(subject) {
- if (!subject) return '';
- if (subject.length < CHANGE_SUBJECT_LIMIT) return subject;
- return subject.substring(0, CHANGE_SUBJECT_LIMIT) + '...';
- }
-
- _modifyRevertSubmissionMsg(change, msg, commitMessage) {
- return this.$.jsAPI.modifyRevertSubmissionMsg(change, msg,
- commitMessage);
- }
-
- _populateRevertSubmissionMessage(change, changes, commitMessage) {
- // Follow the same convention of the revert
- const commitHash = change.current_revision;
- if (!commitHash) {
- this.dispatchEvent(new CustomEvent('show-alert', {
- detail: {message: ERR_COMMIT_NOT_FOUND},
- composed: true, bubbles: true,
- }));
- return;
- }
- if (!changes || changes.length <= 1) return;
- const submissionId = change.submission_id;
- const revertTitle = 'Revert submission ' + submissionId;
- this._message = revertTitle + '\n\n' + 'Reason for revert: <INSERT ' +
- 'REASONING HERE>\n';
- this._message += 'Reverted Changes:\n';
- changes.forEach(change => {
- this._message += change.change_id.substring(0, 10) + ':'
- + this._getTrimmedChangeSubject(change.subject) + '\n';
- });
- this._message = this._modifyRevertSubmissionMsg(change, this._message,
- commitMessage);
- this._revertType = REVERT_TYPES.REVERT_SUBMISSION;
- this._revertMessages[this._revertType] = this._message;
- this._originalRevertMessages[this._revertType] = this._message;
- this._showRevertSubmission = true;
- }
-
- _handleRevertSingleChangeClicked() {
- this._showErrorMessage = false;
- this._revertMessages[REVERT_TYPES.REVERT_SUBMISSION] = this._message;
- this._message = this._revertMessages[REVERT_TYPES.REVERT_SINGLE_CHANGE];
- this._revertType = REVERT_TYPES.REVERT_SINGLE_CHANGE;
- }
-
- _handleRevertSubmissionClicked() {
- this._showErrorMessage = false;
- this._revertType = REVERT_TYPES.REVERT_SUBMISSION;
- this._revertMessages[REVERT_TYPES.REVERT_SINGLE_CHANGE] = this._message;
- this._message = this._revertMessages[REVERT_TYPES.REVERT_SUBMISSION];
- }
-
- _handleConfirmTap(e) {
- e.preventDefault();
- e.stopPropagation();
- if (this._message === this._originalRevertMessages[this._revertType]) {
- this._showErrorMessage = true;
- return;
- }
- this.dispatchEvent(new CustomEvent('confirm', {
- detail: {revertType: this._revertType,
- message: this._message},
- composed: true, bubbles: false,
- }));
- }
-
- _handleCancelTap(e) {
- e.preventDefault();
- e.stopPropagation();
- this.dispatchEvent(new CustomEvent('cancel', {
- detail: {revertType: this._revertType},
- composed: true, bubbles: false,
- }));
- }
-}
-
-customElements.define(GrConfirmRevertDialog.is, GrConfirmRevertDialog);
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog.ts b/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog.ts
new file mode 100644
index 0000000..beaf0f8
--- /dev/null
+++ b/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog.ts
@@ -0,0 +1,248 @@
+/**
+ * @license
+ * Copyright (C) 2016 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 '../../shared/gr-dialog/gr-dialog';
+import '../../../styles/shared-styles';
+import '../../plugins/gr-endpoint-decorator/gr-endpoint-decorator';
+import '../../shared/gr-js-api-interface/gr-js-api-interface';
+import {GestureEventListeners} from '@polymer/polymer/lib/mixins/gesture-event-listeners';
+import {LegacyElementMixin} from '@polymer/polymer/lib/legacy/legacy-element-mixin';
+import {PolymerElement} from '@polymer/polymer/polymer-element';
+import {htmlTemplate} from './gr-confirm-revert-dialog_html';
+import {customElement, property} from '@polymer/decorators';
+import {JsApiService} from '../../shared/gr-js-api-interface/gr-js-api-types';
+import {ChangeInfo, CommitId} from '../../../types/common';
+
+const ERR_COMMIT_NOT_FOUND = 'Unable to find the commit hash of this change.';
+const CHANGE_SUBJECT_LIMIT = 50;
+
+// TODO(dhruvsri): clean up repeated definitions after moving to js modules
+const REVERT_TYPES = {
+ REVERT_SINGLE_CHANGE: 1,
+ REVERT_SUBMISSION: 2,
+};
+
+export interface GrConfirmRevertDialog {
+ $: {
+ jsAPI: JsApiService & Element;
+ };
+}
+@customElement('gr-confirm-revert-dialog')
+export class GrConfirmRevertDialog extends GestureEventListeners(
+ LegacyElementMixin(PolymerElement)
+) {
+ static get template() {
+ return htmlTemplate;
+ }
+
+ /**
+ * Fired when the confirm button is pressed.
+ *
+ * @event confirm
+ */
+
+ /**
+ * Fired when the cancel button is pressed.
+ *
+ * @event cancel
+ */
+
+ /* The revert message updated by the user
+ The default value is set by the dialog */
+ @property({type: String})
+ _message?: string;
+
+ @property({type: Number})
+ _revertType = REVERT_TYPES.REVERT_SINGLE_CHANGE;
+
+ @property({type: Boolean})
+ _showRevertSubmission = false;
+
+ @property({type: Number})
+ _changesCount?: number;
+
+ @property({type: Boolean})
+ _showErrorMessage = false;
+
+ /* store the default revert messages per revert type so that we can
+ check if user has edited the revert message or not
+ Set when populate() is called */
+ @property({type: Array})
+ _originalRevertMessages: string[] = [];
+
+ // Store the actual messages that the user has edited
+ @property({type: Array})
+ _revertMessages: string[] = [];
+
+ _computeIfSingleRevert(revertType: number) {
+ return revertType === REVERT_TYPES.REVERT_SINGLE_CHANGE;
+ }
+
+ _computeIfRevertSubmission(revertType: number) {
+ return revertType === REVERT_TYPES.REVERT_SUBMISSION;
+ }
+
+ _modifyRevertMsg(change: ChangeInfo, commitMessage: string, message: string) {
+ return this.$.jsAPI.modifyRevertMsg(change, message, commitMessage);
+ }
+
+ populate(change: ChangeInfo, commitMessage: string, changes: ChangeInfo[]) {
+ this._changesCount = changes.length;
+ // The option to revert a single change is always available
+ this._populateRevertSingleChangeMessage(
+ change,
+ commitMessage,
+ change.current_revision
+ );
+ this._populateRevertSubmissionMessage(change, changes, commitMessage);
+ }
+
+ _populateRevertSingleChangeMessage(
+ change: ChangeInfo,
+ commitMessage: string,
+ commitHash?: CommitId
+ ) {
+ // Figure out what the revert title should be.
+ const originalTitle = (commitMessage || '').split('\n')[0];
+ const revertTitle = `Revert "${originalTitle}"`;
+ if (!commitHash) {
+ this.dispatchEvent(
+ new CustomEvent('show-alert', {
+ detail: {message: ERR_COMMIT_NOT_FOUND},
+ composed: true,
+ bubbles: true,
+ })
+ );
+ return;
+ }
+ const revertCommitText = `This reverts commit ${commitHash}.`;
+
+ const message =
+ `${revertTitle}\n\n${revertCommitText}\n\n` +
+ 'Reason for revert: <INSERT REASONING HERE>\n';
+ // This is to give plugins a chance to update message
+ this._message = this._modifyRevertMsg(change, commitMessage, message);
+ this._revertType = REVERT_TYPES.REVERT_SINGLE_CHANGE;
+ this._showRevertSubmission = false;
+ this._revertMessages[this._revertType] = this._message;
+ this._originalRevertMessages[this._revertType] = this._message;
+ }
+
+ _getTrimmedChangeSubject(subject: string) {
+ if (!subject) return '';
+ if (subject.length < CHANGE_SUBJECT_LIMIT) return subject;
+ return subject.substring(0, CHANGE_SUBJECT_LIMIT) + '...';
+ }
+
+ _modifyRevertSubmissionMsg(
+ change: ChangeInfo,
+ msg: string,
+ commitMessage: string
+ ) {
+ return this.$.jsAPI.modifyRevertSubmissionMsg(change, msg, commitMessage);
+ }
+
+ _populateRevertSubmissionMessage(
+ change: ChangeInfo,
+ changes: ChangeInfo[],
+ commitMessage: string
+ ) {
+ // Follow the same convention of the revert
+ const commitHash = change.current_revision;
+ if (!commitHash) {
+ this.dispatchEvent(
+ new CustomEvent('show-alert', {
+ detail: {message: ERR_COMMIT_NOT_FOUND},
+ composed: true,
+ bubbles: true,
+ })
+ );
+ return;
+ }
+ if (!changes || changes.length <= 1) return;
+ const revertTitle = `Revert submission ${change.submission_id}`;
+ let message =
+ revertTitle +
+ '\n\n' +
+ 'Reason for revert: <INSERT ' +
+ 'REASONING HERE>\n';
+ message += 'Reverted Changes:\n';
+ changes.forEach(change => {
+ message +=
+ `${change.change_id.substring(0, 10)}:` +
+ `${this._getTrimmedChangeSubject(change.subject)}\n`;
+ });
+ this._message = this._modifyRevertSubmissionMsg(
+ change,
+ message,
+ commitMessage
+ );
+ this._revertType = REVERT_TYPES.REVERT_SUBMISSION;
+ this._revertMessages[this._revertType] = this._message;
+ this._originalRevertMessages[this._revertType] = this._message;
+ this._showRevertSubmission = true;
+ }
+
+ _handleRevertSingleChangeClicked() {
+ this._showErrorMessage = false;
+ if (this._message)
+ this._revertMessages[REVERT_TYPES.REVERT_SUBMISSION] = this._message;
+ this._message = this._revertMessages[REVERT_TYPES.REVERT_SINGLE_CHANGE];
+ this._revertType = REVERT_TYPES.REVERT_SINGLE_CHANGE;
+ }
+
+ _handleRevertSubmissionClicked() {
+ this._showErrorMessage = false;
+ this._revertType = REVERT_TYPES.REVERT_SUBMISSION;
+ if (this._message)
+ this._revertMessages[REVERT_TYPES.REVERT_SINGLE_CHANGE] = this._message;
+ this._message = this._revertMessages[REVERT_TYPES.REVERT_SUBMISSION];
+ }
+
+ _handleConfirmTap(e: MouseEvent) {
+ e.preventDefault();
+ e.stopPropagation();
+ if (this._message === this._originalRevertMessages[this._revertType]) {
+ this._showErrorMessage = true;
+ return;
+ }
+ this.dispatchEvent(
+ new CustomEvent('confirm', {
+ detail: {revertType: this._revertType, message: this._message},
+ composed: true,
+ bubbles: false,
+ })
+ );
+ }
+
+ _handleCancelTap(e: MouseEvent) {
+ e.preventDefault();
+ e.stopPropagation();
+ this.dispatchEvent(
+ new CustomEvent('cancel', {
+ detail: {revertType: this._revertType},
+ composed: true,
+ bubbles: false,
+ })
+ );
+ }
+}
+
+declare global {
+ interface HTMLElementTagNameMap {
+ 'gr-confirm-revert-dialog': GrConfirmRevertDialog;
+ }
+}
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-submit-dialog/gr-confirm-submit-dialog.js b/polygerrit-ui/app/elements/change/gr-confirm-submit-dialog/gr-confirm-submit-dialog.js
deleted file mode 100644
index 42afcc2..0000000
--- a/polygerrit-ui/app/elements/change/gr-confirm-submit-dialog/gr-confirm-submit-dialog.js
+++ /dev/null
@@ -1,92 +0,0 @@
-/**
- * @license
- * Copyright (C) 2018 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/iron-icon/iron-icon.js';
-import '../../shared/gr-icons/gr-icons.js';
-import '../../shared/gr-dialog/gr-dialog.js';
-import '../../shared/gr-rest-api-interface/gr-rest-api-interface.js';
-import '../../plugins/gr-endpoint-decorator/gr-endpoint-decorator.js';
-import '../../plugins/gr-endpoint-param/gr-endpoint-param.js';
-import '../../../styles/shared-styles.js';
-import {GestureEventListeners} from '@polymer/polymer/lib/mixins/gesture-event-listeners.js';
-import {LegacyElementMixin} from '@polymer/polymer/lib/legacy/legacy-element-mixin.js';
-import {PolymerElement} from '@polymer/polymer/polymer-element.js';
-import {htmlTemplate} from './gr-confirm-submit-dialog_html.js';
-
-/** @extends PolymerElement */
-class GrConfirmSubmitDialog extends GestureEventListeners(
- LegacyElementMixin(
- PolymerElement)) {
- static get template() { return htmlTemplate; }
-
- static get is() { return 'gr-confirm-submit-dialog'; }
- /**
- * Fired when the confirm button is pressed.
- *
- * @event confirm
- */
-
- /**
- * Fired when the cancel button is pressed.
- *
- * @event cancel
- */
-
- static get properties() {
- return {
- /**
- * @type {Gerrit.Change}
- */
- change: Object,
-
- /**
- * @type {{
- * label: string,
- * }}
- */
- action: Object,
- };
- }
-
- resetFocus(e) {
- this.$.dialog.resetFocus();
- }
-
- _computeHasChangeEdit(change) {
- return !!change.revisions &&
- Object.values(change.revisions).some(rev => rev._number == 'edit');
- }
-
- _computeUnresolvedCommentsWarning(change) {
- const unresolvedCount = change.unresolved_comment_count;
- const plural = unresolvedCount > 1 ? 's' : '';
- return `Heads Up! ${unresolvedCount} unresolved comment${plural}.`;
- }
-
- _handleConfirmTap(e) {
- e.preventDefault();
- e.stopPropagation();
- this.dispatchEvent(new CustomEvent('confirm', {bubbles: false}));
- }
-
- _handleCancelTap(e) {
- e.preventDefault();
- e.stopPropagation();
- this.dispatchEvent(new CustomEvent('cancel', {bubbles: false}));
- }
-}
-
-customElements.define(GrConfirmSubmitDialog.is, GrConfirmSubmitDialog);
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-submit-dialog/gr-confirm-submit-dialog.ts b/polygerrit-ui/app/elements/change/gr-confirm-submit-dialog/gr-confirm-submit-dialog.ts
new file mode 100644
index 0000000..666f95d
--- /dev/null
+++ b/polygerrit-ui/app/elements/change/gr-confirm-submit-dialog/gr-confirm-submit-dialog.ts
@@ -0,0 +1,98 @@
+/**
+ * @license
+ * Copyright (C) 2018 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/iron-icon/iron-icon';
+import '../../shared/gr-icons/gr-icons';
+import '../../shared/gr-dialog/gr-dialog';
+import '../../shared/gr-rest-api-interface/gr-rest-api-interface';
+import '../../plugins/gr-endpoint-decorator/gr-endpoint-decorator';
+import '../../plugins/gr-endpoint-param/gr-endpoint-param';
+import '../../../styles/shared-styles';
+import {GestureEventListeners} from '@polymer/polymer/lib/mixins/gesture-event-listeners';
+import {LegacyElementMixin} from '@polymer/polymer/lib/legacy/legacy-element-mixin';
+import {PolymerElement} from '@polymer/polymer/polymer-element';
+import {htmlTemplate} from './gr-confirm-submit-dialog_html';
+import {customElement, property} from '@polymer/decorators';
+import {ChangeInfo, ActionInfo} from '../../../types/common';
+import {GrDialog} from '../../shared/gr-dialog/gr-dialog';
+
+export interface GrConfirmSubmitDialog {
+ $: {
+ dialog: GrDialog;
+ };
+}
+@customElement('gr-confirm-submit-dialog')
+export class GrConfirmSubmitDialog extends GestureEventListeners(
+ LegacyElementMixin(PolymerElement)
+) {
+ static get template() {
+ return htmlTemplate;
+ }
+
+ /**
+ * Fired when the confirm button is pressed.
+ *
+ * @event confirm
+ */
+
+ /**
+ * Fired when the cancel button is pressed.
+ *
+ * @event cancel
+ */
+
+ @property({type: Object})
+ change?: ChangeInfo;
+
+ @property({type: Object})
+ action?: ActionInfo;
+
+ resetFocus() {
+ this.$.dialog.resetFocus();
+ }
+
+ _computeHasChangeEdit(change?: ChangeInfo) {
+ return (
+ !!change &&
+ !!change.revisions &&
+ Object.values(change.revisions).some(rev => rev._number === 'edit')
+ );
+ }
+
+ _computeUnresolvedCommentsWarning(change: ChangeInfo) {
+ const unresolvedCount = change.unresolved_comment_count;
+ const plural = unresolvedCount && unresolvedCount > 1 ? 's' : '';
+ return `Heads Up! ${unresolvedCount} unresolved comment${plural}.`;
+ }
+
+ _handleConfirmTap(e: MouseEvent) {
+ e.preventDefault();
+ e.stopPropagation();
+ this.dispatchEvent(new CustomEvent('confirm', {bubbles: false}));
+ }
+
+ _handleCancelTap(e: MouseEvent) {
+ e.preventDefault();
+ e.stopPropagation();
+ this.dispatchEvent(new CustomEvent('cancel', {bubbles: false}));
+ }
+}
+
+declare global {
+ interface HTMLElementTagNameMap {
+ 'gr-confirm-submit-dialog': GrConfirmSubmitDialog;
+ }
+}
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js
index 6ecdabb..630681c 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js
@@ -540,7 +540,7 @@
if (reviewer.account) {
reviewerId = reviewer.account._account_id || reviewer.account.email;
} else if (reviewer.group) {
- reviewerId = reviewer.group.id;
+ reviewerId = decodeURIComponent(reviewer.group.id);
confirmed = reviewer.group.confirmed;
}
return {reviewer: reviewerId, confirmed};
diff --git a/polygerrit-ui/app/elements/change/gr-reviewer-list/gr-reviewer-list.js b/polygerrit-ui/app/elements/change/gr-reviewer-list/gr-reviewer-list.js
deleted file mode 100644
index 174bbbb..0000000
--- a/polygerrit-ui/app/elements/change/gr-reviewer-list/gr-reviewer-list.js
+++ /dev/null
@@ -1,303 +0,0 @@
-/**
- * @license
- * Copyright (C) 2015 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 '../../shared/gr-account-chip/gr-account-chip.js';
-import '../../shared/gr-button/gr-button.js';
-import '../../shared/gr-rest-api-interface/gr-rest-api-interface.js';
-import '../../../styles/shared-styles.js';
-import {dom} from '@polymer/polymer/lib/legacy/polymer.dom.js';
-import {GestureEventListeners} from '@polymer/polymer/lib/mixins/gesture-event-listeners.js';
-import {LegacyElementMixin} from '@polymer/polymer/lib/legacy/legacy-element-mixin.js';
-import {PolymerElement} from '@polymer/polymer/polymer-element.js';
-import {htmlTemplate} from './gr-reviewer-list_html.js';
-import {
- hasAttention,
- isServiceUser,
-} from '../../../utils/account-util.js';
-
-/**
- * @extends PolymerElement
- */
-class GrReviewerList extends GestureEventListeners(
- LegacyElementMixin(PolymerElement)) {
- static get template() { return htmlTemplate; }
-
- static get is() { return 'gr-reviewer-list'; }
- /**
- * Fired when the "Add reviewer..." button is tapped.
- *
- * @event show-reply-dialog
- */
-
- static get properties() {
- return {
- change: Object,
- serverConfig: Object,
- disabled: {
- type: Boolean,
- value: false,
- reflectToAttribute: true,
- },
- mutable: {
- type: Boolean,
- value: false,
- },
- reviewersOnly: {
- type: Boolean,
- value: false,
- },
- ccsOnly: {
- type: Boolean,
- value: false,
- },
-
- _displayedReviewers: {
- type: Array,
- value() { return []; },
- },
- _reviewers: {
- type: Array,
- value() { return []; },
- },
- _showInput: {
- type: Boolean,
- value: false,
- },
- _addLabel: {
- type: String,
- computed: '_computeAddLabel(ccsOnly)',
- },
- _hiddenReviewerCount: {
- type: Number,
- computed: '_computeHiddenCount(_reviewers, _displayedReviewers)',
- },
-
- // Used for testing.
- _lastAutocompleteRequest: Object,
- _xhrPromise: Object,
- };
- }
-
- static get observers() {
- return [
- '_reviewersChanged(change.reviewers.*, change.owner, serverConfig)',
- ];
- }
-
- /**
- * Converts change.permitted_labels to an array of hashes of label keys to
- * numeric scores.
- * Example:
- * [{
- * 'Code-Review': ['-1', ' 0', '+1']
- * }]
- * will be converted to
- * [{
- * label: 'Code-Review',
- * scores: [-1, 0, 1]
- * }]
- */
- _permittedLabelsToNumericScores(labels) {
- if (!labels) return [];
- return Object.keys(labels).map(label => {
- return {
- label,
- scores: labels[label].map(v => parseInt(v, 10)),
- };
- });
- }
-
- /**
- * Returns hash of labels to max permitted score.
- *
- * @param {!Object} change
- * @returns {!Object} labels to max permitted scores hash
- */
- _getMaxPermittedScores(change) {
- return this._permittedLabelsToNumericScores(change.permitted_labels)
- .map(({label, scores}) => {
- return {
- [label]: scores
- .map(v => parseInt(v, 10))
- .reduce((a, b) => Math.max(a, b))};
- })
- .reduce((acc, i) => Object.assign(acc, i), {});
- }
-
- /**
- * Returns max permitted score for reviewer.
- *
- * @param {!Object} reviewer
- * @param {!Object} change
- * @param {string} label
- * @return {number}
- */
- _getReviewerPermittedScore(reviewer, change, label) {
- // Note (issue 7874): sometimes the "all" list is not included in change
- // detail responses, even when DETAILED_LABELS is included in options.
- if (!change.labels[label].all) { return NaN; }
- const detailed = change.labels[label].all.filter(
- ({_account_id}) => reviewer._account_id === _account_id).pop();
- if (!detailed) {
- return NaN;
- }
- if (detailed.hasOwnProperty('permitted_voting_range')) {
- return detailed.permitted_voting_range.max;
- } else if (detailed.hasOwnProperty('value')) {
- // If preset, user can vote on the label.
- return 0;
- }
- return NaN;
- }
-
- _computeVoteableText(reviewer, change) {
- if (!change || !change.labels) { return ''; }
- const maxScores = [];
- const maxPermitted = this._getMaxPermittedScores(change);
- for (const label of Object.keys(change.labels)) {
- const maxScore =
- this._getReviewerPermittedScore(reviewer, change, label);
- if (isNaN(maxScore) || maxScore < 0) { continue; }
- if (maxScore > 0 && maxScore === maxPermitted[label]) {
- maxScores.push(`${label}: +${maxScore}`);
- } else {
- maxScores.push(`${label}`);
- }
- }
- return maxScores.join(', ');
- }
-
- _reviewersChanged(changeRecord, owner, serverConfig) {
- // Polymer 2: check for undefined
- if ([changeRecord, owner, serverConfig].includes(undefined)) {
- return;
- }
-
- let result = [];
- const reviewers = changeRecord.base;
- for (const key in reviewers) {
- if (this.reviewersOnly && key !== 'REVIEWER') {
- continue;
- }
- if (this.ccsOnly && key !== 'CC') {
- continue;
- }
- if (key === 'REVIEWER' || key === 'CC') {
- result = result.concat(reviewers[key]);
- }
- }
- this._reviewers = result
- .filter(reviewer => reviewer._account_id != owner._account_id)
- // Sort order:
- // 1. Human users in the attention set.
- // 2. Other human users.
- // 3. Service users.
- .sort((r1, r2) => {
- const a1 = hasAttention(serverConfig, r1, this.change) ? 1 : 0;
- const a2 = hasAttention(serverConfig, r2, this.change) ? 1 : 0;
- const s1 = isServiceUser(r1) ? -2 : 0;
- const s2 = isServiceUser(r2) ? -2 : 0;
- return a2 - a1 + s2 - s1;
- });
-
- if (this._reviewers.length > 8) {
- this._displayedReviewers = this._reviewers.slice(0, 6);
- } else {
- this._displayedReviewers = this._reviewers;
- }
- }
-
- _computeHiddenCount(reviewers, displayedReviewers) {
- // Polymer 2: check for undefined
- if ([reviewers, displayedReviewers].includes(undefined)) {
- return undefined;
- }
-
- return reviewers.length - displayedReviewers.length;
- }
-
- _computeCanRemoveReviewer(reviewer, mutable) {
- if (!mutable) { return false; }
-
- let current;
- for (let i = 0; i < this.change.removable_reviewers.length; i++) {
- current = this.change.removable_reviewers[i];
- if (current._account_id === reviewer._account_id ||
- (!reviewer._account_id && current.email === reviewer.email)) {
- return true;
- }
- }
- return false;
- }
-
- _handleRemove(e) {
- e.preventDefault();
- const target = dom(e).rootTarget;
- if (!target.account) { return; }
- const accountID = target.account._account_id || target.account.email;
- this.disabled = true;
- this._xhrPromise = this._removeReviewer(accountID).then(response => {
- this.disabled = false;
- if (!response.ok) { return response; }
-
- const reviewers = this.change.reviewers;
-
- for (const type of ['REVIEWER', 'CC']) {
- reviewers[type] = reviewers[type] || [];
- for (let i = 0; i < reviewers[type].length; i++) {
- if (reviewers[type][i]._account_id == accountID ||
- reviewers[type][i].email == accountID) {
- this.splice('change.reviewers.' + type, i, 1);
- break;
- }
- }
- }
- })
- .catch(err => {
- this.disabled = false;
- throw err;
- });
- }
-
- _handleAddTap(e) {
- e.preventDefault();
- const value = {};
- if (this.reviewersOnly) {
- value.reviewersOnly = true;
- }
- if (this.ccsOnly) {
- value.ccsOnly = true;
- }
- this.dispatchEvent(new CustomEvent('show-reply-dialog', {
- detail: {value},
- composed: true, bubbles: true,
- }));
- }
-
- _handleViewAll(e) {
- this._displayedReviewers = this._reviewers;
- }
-
- _removeReviewer(id) {
- return this.$.restAPI.removeChangeReviewer(this.change._number, id);
- }
-
- _computeAddLabel(ccsOnly) {
- return ccsOnly ? 'Add CC' : 'Add reviewer';
- }
-}
-
-customElements.define(GrReviewerList.is, GrReviewerList);
diff --git a/polygerrit-ui/app/elements/change/gr-reviewer-list/gr-reviewer-list.ts b/polygerrit-ui/app/elements/change/gr-reviewer-list/gr-reviewer-list.ts
new file mode 100644
index 0000000..70e7ba7
--- /dev/null
+++ b/polygerrit-ui/app/elements/change/gr-reviewer-list/gr-reviewer-list.ts
@@ -0,0 +1,339 @@
+/**
+ * @license
+ * Copyright (C) 2015 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 '../../shared/gr-account-chip/gr-account-chip';
+import '../../shared/gr-button/gr-button';
+import '../../shared/gr-rest-api-interface/gr-rest-api-interface';
+import '../../../styles/shared-styles';
+import {dom, EventApi} from '@polymer/polymer/lib/legacy/polymer.dom';
+import {GestureEventListeners} from '@polymer/polymer/lib/mixins/gesture-event-listeners';
+import {LegacyElementMixin} from '@polymer/polymer/lib/legacy/legacy-element-mixin';
+import {PolymerElement} from '@polymer/polymer/polymer-element';
+import {htmlTemplate} from './gr-reviewer-list_html';
+import {hasAttention, isServiceUser} from '../../../utils/account-util';
+import {customElement, property, computed, observe} from '@polymer/decorators';
+import {
+ ChangeInfo,
+ ServerInfo,
+ LabelNameToValueMap,
+ AccountInfo,
+ ApprovalInfo,
+ Reviewers,
+ AccountId,
+ DetailedLabelInfo,
+} from '../../../types/common';
+import {PolymerDeepPropertyChange} from '@polymer/polymer/interfaces';
+import {GrAccountChip} from '../../shared/gr-account-chip/gr-account-chip';
+import {RestApiService} from '../../../services/services/gr-rest-api/gr-rest-api';
+import {hasOwnProperty} from '../../../utils/common-util';
+
+export interface GrReviewerList {
+ $: {
+ restAPI: RestApiService & Element;
+ };
+}
+
+@customElement('gr-reviewer-list')
+export class GrReviewerList extends GestureEventListeners(
+ LegacyElementMixin(PolymerElement)
+) {
+ static get template() {
+ return htmlTemplate;
+ }
+
+ /**
+ * Fired when the "Add reviewer..." button is tapped.
+ *
+ * @event show-reply-dialog
+ */
+
+ @property({type: Object})
+ change?: ChangeInfo;
+
+ @property({type: Object})
+ serverConfig?: ServerInfo;
+
+ @property({type: Boolean, reflectToAttribute: true})
+ disabled = false;
+
+ @property({type: Boolean})
+ mutable = false;
+
+ @property({type: Boolean})
+ reviewersOnly = false;
+
+ @property({type: Boolean})
+ ccsOnly = false;
+
+ @property({type: Array})
+ _displayedReviewers: AccountInfo[] = [];
+
+ @property({type: Array})
+ _reviewers: AccountInfo[] = [];
+
+ @property({type: Boolean})
+ _showInput = false;
+
+ @property({type: Object})
+ _xhrPromise?: Promise<Response | undefined>;
+
+ @computed('ccsOnly')
+ get _addLabel() {
+ return this.ccsOnly ? 'Add CC' : 'Add reviewer';
+ }
+
+ @computed('_reviewers', '_displayedReviewers')
+ get _hiddenReviewerCount() {
+ // Polymer 2: check for undefined
+ if (
+ this._reviewers === undefined ||
+ this._displayedReviewers === undefined
+ ) {
+ return undefined;
+ }
+ return this._reviewers.length - this._displayedReviewers.length;
+ }
+
+ /**
+ * Converts change.permitted_labels to an array of hashes of label keys to
+ * numeric scores.
+ * Example:
+ * [{
+ * 'Code-Review': ['-1', ' 0', '+1']
+ * }]
+ * will be converted to
+ * [{
+ * label: 'Code-Review',
+ * scores: [-1, 0, 1]
+ * }]
+ */
+ _permittedLabelsToNumericScores(labels: LabelNameToValueMap | undefined) {
+ if (!labels) return [];
+ return Object.keys(labels).map(label => {
+ return {
+ label,
+ scores: labels[label].map(v => parseInt(v, 10)),
+ };
+ });
+ }
+
+ /**
+ * Returns hash of labels to max permitted score.
+ *
+ * @returns labels to max permitted scores hash
+ */
+ _getMaxPermittedScores(change: ChangeInfo) {
+ return this._permittedLabelsToNumericScores(change.permitted_labels)
+ .map(({label, scores}) => {
+ return {
+ [label]: scores.reduce((a, b) => Math.max(a, b)),
+ };
+ })
+ .reduce((acc, i) => Object.assign(acc, i), {});
+ }
+
+ /**
+ * Returns max permitted score for reviewer.
+ */
+ _getReviewerPermittedScore(
+ reviewer: AccountInfo,
+ change: ChangeInfo,
+ label: string
+ ) {
+ // Note (issue 7874): sometimes the "all" list is not included in change
+ // detail responses, even when DETAILED_LABELS is included in options.
+ if (!change.labels) {
+ return NaN;
+ }
+ const detailedLabel = change.labels[label] as DetailedLabelInfo;
+ if (!detailedLabel.all) {
+ return NaN;
+ }
+ const detailed = detailedLabel.all
+ .filter(
+ (approval: ApprovalInfo) =>
+ reviewer._account_id === approval._account_id
+ )
+ .pop();
+ if (!detailed) {
+ return NaN;
+ }
+ if (hasOwnProperty(detailed, 'permitted_voting_range')) {
+ if (!detailed.permitted_voting_range) return NaN;
+ return detailed.permitted_voting_range.max;
+ } else if (hasOwnProperty(detailed, 'value')) {
+ // If preset, user can vote on the label.
+ return 0;
+ }
+ return NaN;
+ }
+
+ _computeVoteableText(reviewer: AccountInfo, change: ChangeInfo) {
+ if (!change || !change.labels) {
+ return '';
+ }
+ const maxScores = [];
+ const maxPermitted = this._getMaxPermittedScores(change);
+ for (const label of Object.keys(change.labels)) {
+ const maxScore = this._getReviewerPermittedScore(reviewer, change, label);
+ if (isNaN(maxScore) || maxScore < 0) {
+ continue;
+ }
+ if (maxScore > 0 && maxScore === maxPermitted[label]) {
+ maxScores.push(`${label}: +${maxScore}`);
+ } else {
+ maxScores.push(`${label}`);
+ }
+ }
+ return maxScores.join(', ');
+ }
+
+ @observe('change.reviewers.*', 'change.owner', 'serverConfig')
+ _reviewersChanged(
+ changeRecord: PolymerDeepPropertyChange<Reviewers, Reviewers>,
+ owner: AccountInfo,
+ serverConfig: ServerInfo
+ ) {
+ // Polymer 2: check for undefined
+ if (
+ changeRecord === undefined ||
+ owner === undefined ||
+ serverConfig === undefined ||
+ this.change === undefined
+ ) {
+ return;
+ }
+ let result: AccountInfo[] = [];
+ const reviewers = changeRecord.base;
+ for (const key in reviewers) {
+ if (this.reviewersOnly && key !== 'REVIEWER') {
+ continue;
+ }
+ if (this.ccsOnly && key !== 'CC') {
+ continue;
+ }
+ if (key === 'REVIEWER' || key === 'CC') {
+ result = result.concat(reviewers[key]!);
+ }
+ }
+ this._reviewers = result
+ .filter(reviewer => reviewer._account_id !== owner._account_id)
+ // Sort order:
+ // 1. Human users in the attention set.
+ // 2. Other human users.
+ // 3. Service users.
+ .sort((r1, r2) => {
+ const a1 = hasAttention(serverConfig, r1, this.change!) ? 1 : 0;
+ const a2 = hasAttention(serverConfig, r2, this.change!) ? 1 : 0;
+ const s1 = isServiceUser(r1) ? -2 : 0;
+ const s2 = isServiceUser(r2) ? -2 : 0;
+ return a2 - a1 + s2 - s1;
+ });
+
+ if (this._reviewers.length > 8) {
+ this._displayedReviewers = this._reviewers.slice(0, 6);
+ } else {
+ this._displayedReviewers = this._reviewers;
+ }
+ }
+
+ _computeCanRemoveReviewer(reviewer: AccountInfo, mutable: boolean) {
+ if (
+ !mutable ||
+ this.change === undefined ||
+ this.change.removable_reviewers === undefined
+ ) {
+ return false;
+ }
+
+ let current;
+ for (let i = 0; i < this.change.removable_reviewers.length; i++) {
+ current = this.change.removable_reviewers[i];
+ if (
+ current._account_id === reviewer._account_id ||
+ (!reviewer._account_id && current.email === reviewer.email)
+ ) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ _handleRemove(e: Event) {
+ e.preventDefault();
+ const target = (dom(e) as EventApi).rootTarget as GrAccountChip;
+ if (!target.account || !this.change) {
+ return;
+ }
+ const accountID = target.account._account_id;
+ this.disabled = true;
+ if (!accountID) return;
+ this._xhrPromise = this._removeReviewer(accountID)
+ .then((response: Response | undefined) => {
+ this.disabled = false;
+ if (!response || !response.ok) {
+ return response;
+ }
+ if (!this.change || !this.change.reviewers) return;
+ const reviewers: {[type: string]: AccountInfo[] | undefined} = this
+ .change!.reviewers;
+ for (const type of ['REVIEWER', 'CC']) {
+ reviewers[type] = reviewers[type] || [];
+ for (let i = 0; i < reviewers[type]!.length; i++) {
+ if (reviewers[type]![i]._account_id === accountID) {
+ this.splice('change.reviewers.' + type, i, 1);
+ break;
+ }
+ }
+ }
+ return;
+ })
+ .catch((err: Error) => {
+ this.disabled = false;
+ throw err;
+ });
+ }
+
+ _handleAddTap(e: Event) {
+ e.preventDefault();
+ const value = {
+ reviewersOnly: false,
+ ccsOnly: false,
+ };
+ if (this.reviewersOnly) {
+ value.reviewersOnly = true;
+ }
+ if (this.ccsOnly) {
+ value.ccsOnly = true;
+ }
+ this.dispatchEvent(
+ new CustomEvent('show-reply-dialog', {
+ detail: {value},
+ composed: true,
+ bubbles: true,
+ })
+ );
+ }
+
+ _handleViewAll() {
+ this._displayedReviewers = this._reviewers;
+ }
+
+ _removeReviewer(id: AccountId): Promise<Response | undefined> {
+ if (!this.change) return Promise.resolve(undefined);
+ return this.$.restAPI.removeChangeReviewer(this.change._number, id);
+ }
+}
diff --git a/polygerrit-ui/app/elements/change/gr-reviewer-list/gr-reviewer-list_test.js b/polygerrit-ui/app/elements/change/gr-reviewer-list/gr-reviewer-list_test.js
index 521945f..834cc43 100644
--- a/polygerrit-ui/app/elements/change/gr-reviewer-list/gr-reviewer-list_test.js
+++ b/polygerrit-ui/app/elements/change/gr-reviewer-list/gr-reviewer-list_test.js
@@ -163,21 +163,24 @@
element.reviewersOnly = false;
element._handleAddTap(e);
assert.equal(fireStub.lastCall.args[0].type, 'show-reply-dialog');
- assert.deepEqual(fireStub.lastCall.args[0].detail, {value: {}});
+ assert.deepEqual(fireStub.lastCall.args[0].detail, {value: {
+ reviewersOnly: false,
+ ccsOnly: false,
+ }});
element.reviewersOnly = true;
element._handleAddTap(e);
assert.equal(fireStub.lastCall.args[0].type, 'show-reply-dialog');
assert.deepEqual(
fireStub.lastCall.args[0].detail,
- {value: {reviewersOnly: true}});
+ {value: {reviewersOnly: true, ccsOnly: false}});
element.ccsOnly = true;
element.reviewersOnly = false;
element._handleAddTap(e);
assert.equal(fireStub.lastCall.args[0].type, 'show-reply-dialog');
assert.deepEqual(fireStub.lastCall.args[0].detail,
- {value: {ccsOnly: true}});
+ {value: {ccsOnly: true, reviewersOnly: false}});
});
test('dont show all reviewers button with 4 reviewers', () => {
diff --git a/polygerrit-ui/app/elements/core/gr-router/gr-router.ts b/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
index 6f1bc60..aa2e4ce 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
@@ -72,6 +72,7 @@
DASHBOARD: /^\/dashboard\/(.+)$/,
CUSTOM_DASHBOARD: /^\/dashboard\/?$/,
PROJECT_DASHBOARD: /^\/p\/(.+)\/\+\/dashboard\/(.+)/,
+ LEGACY_PROJECT_DASHBOARD: /^\/projects\/(.+),dashboards\/(.+)/,
AGREEMENTS: /^\/settings\/agreements\/?/,
NEW_AGREEMENTS: /^\/settings\/new-agreement\/?/,
@@ -880,6 +881,11 @@
'_handleProjectDashboardRoute'
);
+ this._mapRoute(
+ RoutePattern.LEGACY_PROJECT_DASHBOARD,
+ '_handleLegacyProjectDashboardRoute'
+ );
+
this._mapRoute(RoutePattern.GROUP_INFO, '_handleGroupInfoRoute', true);
this._mapRoute(
@@ -1255,6 +1261,10 @@
this.reporting.setRepoName(project);
}
+ _handleLegacyProjectDashboardRoute(data: PageContextWithQueryMap) {
+ this._redirect('/p/' + data.params[0] + '/+/dashboard/' + data.params[1]);
+ }
+
_handleGroupInfoRoute(data: PageContextWithQueryMap) {
this._redirect('/admin/groups/' + encodeURIComponent(data.params[0]));
}
diff --git a/polygerrit-ui/app/elements/core/gr-router/gr-router_test.js b/polygerrit-ui/app/elements/core/gr-router/gr-router_test.js
index 5ee58c9..927434b 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router_test.js
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router_test.js
@@ -197,6 +197,7 @@
'_handleImproperlyEncodedPlusRoute',
'_handlePassThroughRoute',
'_handleProjectDashboardRoute',
+ '_handleLegacyProjectDashboardRoute',
'_handleProjectsOldRoute',
'_handleRepoAccessRoute',
'_handleRepoDashboardsRoute',
@@ -617,6 +618,14 @@
handlePassThroughRoute = sinon.stub(element, '_handlePassThroughRoute');
});
+ test('_handleLegacyProjectDashboardRoute', () => {
+ const params = {0: 'gerrit/project', 1: 'dashboard:main'};
+ element._handleLegacyProjectDashboardRoute({params});
+ assert.isTrue(redirectStub.calledOnce);
+ assert.equal(redirectStub.lastCall.args[0],
+ '/p/gerrit/project/+/dashboard/dashboard:main');
+ });
+
test('_handleAgreementsRoute', () => {
const data = {params: {}};
element._handleAgreementsRoute(data);
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-types.ts b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-types.ts
index 7455cad..0a5dbc9 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-types.ts
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-types.ts
@@ -39,5 +39,10 @@
origMsg: string
): string;
handleEvent(eventName: EventType, detail: any): void;
+ modifyRevertMsg(
+ change: ChangeInfo,
+ revertMsg: string,
+ origMsg: string
+ ): string;
// TODO(TS): Add more methods when needed for the TS conversion.
}
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-reporting-js-api.ts b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-reporting-js-api.ts
index ddf4c21..0bf6676 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-reporting-js-api.ts
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-reporting-js-api.ts
@@ -37,4 +37,11 @@
details
);
}
+
+ reportLifeCycle(eventName: string, details?: EventDetails) {
+ return this.reporting.reportLifeCycle(
+ `${this.plugin.getPluginName()}-${eventName}`,
+ details
+ );
+ }
}
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-reporting-js-api_test.js b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-reporting-js-api_test.js
index e05dff3..1229641 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-reporting-js-api_test.js
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-reporting-js-api_test.js
@@ -57,5 +57,19 @@
{}
);
});
+
+ test('redirect reportLifeCycle call to reportingService', () => {
+ sinon.spy(appContext.reportingService, 'reportLifeCycle');
+ reporting.reportLifeCycle('test', {});
+ assert.isTrue(appContext.reportingService.reportLifeCycle.called);
+ assert.equal(
+ appContext.reportingService.reportLifeCycle.lastCall.args[0],
+ 'testplugin-test'
+ );
+ assert.deepEqual(
+ appContext.reportingService.reportLifeCycle.lastCall.args[1],
+ {}
+ );
+ });
});
});
\ No newline at end of file
diff --git a/polygerrit-ui/app/services/services/gr-rest-api/gr-rest-api.ts b/polygerrit-ui/app/services/services/gr-rest-api/gr-rest-api.ts
index 90112b9..e3c2ce8 100644
--- a/polygerrit-ui/app/services/services/gr-rest-api/gr-rest-api.ts
+++ b/polygerrit-ui/app/services/services/gr-rest-api/gr-rest-api.ts
@@ -605,4 +605,8 @@
changeNum: ChangeNum,
messageId: ChangeMessageId
): Promise<Response>;
+ removeChangeReviewer(
+ changeNum: ChangeNum,
+ reviewerID: AccountId | GroupId
+ ): Promise<Response | undefined>;
}
diff --git a/polygerrit-ui/app/types/common.ts b/polygerrit-ui/app/types/common.ts
index e131b4f..0c4bc8b 100644
--- a/polygerrit-ui/app/types/common.ts
+++ b/polygerrit-ui/app/types/common.ts
@@ -142,6 +142,10 @@
*/
export type LabelInfo = QuickLabelInfo | DetailedLabelInfo;
+export type Reviewers = {
+ REVIEWER?: AccountInfo[];
+ CC?: AccountInfo[];
+};
interface LabelCommonInfo {
optional?: boolean; // not set if false
}
@@ -211,10 +215,7 @@
labels?: LabelNameToInfoMap;
permitted_labels?: LabelNameToValueMap;
removable_reviewers?: AccountInfo[];
- reviewers?: {
- REVIEWER?: AccountInfo[];
- CC?: AccountInfo[];
- };
+ reviewers?: Reviewers;
pending_reviewers?: AccountInfo[];
reviewer_updates?: ReviewerUpdateInfo[];
messages?: ChangeMessageInfo[];
