Merge "Don't expose repository existence when user has no access" into stable-3.4
diff --git a/Documentation/images/gwt-user-review-ui-change-screen-change-info-last-update.png b/Documentation/images/gwt-user-review-ui-change-screen-change-info-last-update.png
deleted file mode 100644
index 93c296a..0000000
--- a/Documentation/images/gwt-user-review-ui-change-screen-change-info-last-update.png
+++ /dev/null
Binary files differ
diff --git a/Documentation/images/gwt-user-review-ui-change-screen-change-info-owner.png b/Documentation/images/gwt-user-review-ui-change-screen-change-info-owner.png
deleted file mode 100644
index 3d73ef7..0000000
--- a/Documentation/images/gwt-user-review-ui-change-screen-change-info-owner.png
+++ /dev/null
Binary files differ
diff --git a/Documentation/images/gwt-user-review-ui-change-screen-quick-approve.png b/Documentation/images/gwt-user-review-ui-change-screen-quick-approve.png
deleted file mode 100644
index 638fc2f..0000000
--- a/Documentation/images/gwt-user-review-ui-change-screen-quick-approve.png
+++ /dev/null
Binary files differ
diff --git a/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-column.png b/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-column.png
deleted file mode 100644
index b599f6d..0000000
--- a/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-column.png
+++ /dev/null
Binary files differ
diff --git a/Documentation/images/gwt-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
deleted file mode 100644
index c041311..0000000
--- a/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-dark-theme.png
+++ /dev/null
Binary files differ
diff --git a/Documentation/images/gwt-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
deleted file mode 100644
index ea14a21..0000000
--- a/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-expand-skipped-lines.png
+++ /dev/null
Binary files differ
diff --git a/Documentation/images/gwt-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
deleted file mode 100644
index 8406ce8..0000000
--- a/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-file-level-comment.png
+++ /dev/null
Binary files differ
diff --git a/Documentation/images/gwt-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
deleted file mode 100644
index 1fd2033..0000000
--- a/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-file-level-commented.png
+++ /dev/null
Binary files differ
diff --git a/Documentation/images/gwt-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
deleted file mode 100644
index 043c1ff..0000000
--- a/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-preferences-popup.png
+++ /dev/null
Binary files differ
diff --git a/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-preferences.png b/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-preferences.png
deleted file mode 100644
index 7373b2f..0000000
--- a/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-preferences.png
+++ /dev/null
Binary files differ
diff --git a/Documentation/images/gwt-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
deleted file mode 100644
index f817d66..0000000
--- a/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-red-bar.png
+++ /dev/null
Binary files differ
diff --git a/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-reviewed.png b/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-reviewed.png
deleted file mode 100644
index c767452..0000000
--- a/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-reviewed.png
+++ /dev/null
Binary files differ
diff --git a/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-scrollbar.png b/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-scrollbar.png
deleted file mode 100644
index cbadd26..0000000
--- a/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-scrollbar.png
+++ /dev/null
Binary files differ
diff --git a/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-search.png b/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-search.png
deleted file mode 100644
index e69bb0d..0000000
--- a/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-search.png
+++ /dev/null
Binary files differ
diff --git a/Documentation/images/gwt-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
deleted file mode 100644
index a4b019a..0000000
--- a/Documentation/images/gwt-user-review-ui-side-by-side-diff-screen-syntax-coloring.png
+++ /dev/null
Binary files differ
diff --git a/Documentation/images/user-review-ui-change-screen-quick-approve.png b/Documentation/images/user-review-ui-change-screen-quick-approve.png
new file mode 100644
index 0000000..f692d07
--- /dev/null
+++ b/Documentation/images/user-review-ui-change-screen-quick-approve.png
Binary files differ
diff --git a/Documentation/images/user-review-ui-side-by-side-diff-screen-expand-skipped-lines.png b/Documentation/images/user-review-ui-side-by-side-diff-screen-expand-skipped-lines.png
new file mode 100644
index 0000000..1eb7665
--- /dev/null
+++ b/Documentation/images/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/user-review-ui-side-by-side-diff-screen-file-level-comment.png
new file mode 100644
index 0000000..66c46b7
--- /dev/null
+++ b/Documentation/images/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-preferences.png b/Documentation/images/user-review-ui-side-by-side-diff-screen-preferences.png
new file mode 100644
index 0000000..e008f2b
--- /dev/null
+++ b/Documentation/images/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-reviewed.png b/Documentation/images/user-review-ui-side-by-side-diff-screen-reviewed.png
new file mode 100644
index 0000000..e2a7957
--- /dev/null
+++ b/Documentation/images/user-review-ui-side-by-side-diff-screen-reviewed.png
Binary files differ
diff --git a/Documentation/index.txt b/Documentation/index.txt
index e56e7ca..dc94b14 100644
--- a/Documentation/index.txt
+++ b/Documentation/index.txt
@@ -75,6 +75,7 @@
. link:config-reverseproxy.html[Reverse Proxy]
. link:config-auto-site-initialization.html[Automatic Site Initialization on Startup]
. link:pgm-index.html[Server Side Administrative Tools]
+. link:repository-maintenance.html[Repository Maintenance]
. link:user-request-tracing.html[Request Tracing]
. link:note-db.html[NoteDb]
. link:config-accounts.html[Accounts on NoteDb]
diff --git a/Documentation/repository-maintenance.txt b/Documentation/repository-maintenance.txt
new file mode 100644
index 0000000..1672436
--- /dev/null
+++ b/Documentation/repository-maintenance.txt
@@ -0,0 +1,116 @@
+= Gerrit Code Review - Repository Maintenance
+
+== Description
+
+Each project in Gerrit is stored in a bare Git repository. Gerrit uses
+the JGit library to access (read and write to) these Git repositories.
+As modifications are made to a project, Git repository maintenance will
+be needed or performance will eventually suffer. When using the Git
+command line tool to operate on a Git repository, it will run `git gc`
+every now and then on the repository to ensure that Git garbage
+collection is performed. However regular maintenance does not happen as
+a result of normal Gerrit operations, so this is something that Gerrit
+administrators need to plan for.
+
+Gerrit has a built-in feature which allows it to run Git garbage
+collection on repositories. This can be
+link:config-gerrit.html#gc[configured] to run on a regular basis, and/or
+this can be run manually with the link:cmd-gc.html[gerrit gc] ssh
+command, or with the link:rest-api-projects.html#run-gc[run-gc] REST API.
+Some administrators will opt to run `git gc` or `jgit gc` outside of
+Gerrit instead. There are many reasons this might be done, the main one
+likely being that when it is run in Gerrit it can be very resource
+intensive and scheduling an external job to run Git garbage collection
+allows administrators to finely tune the approach and resource usage of
+this maintenance.
+
+== Git Garbage Collection Impacts
+
+Unlike a typical server database, access to Git repositories is not
+marshalled through a single process or a set of inter communicating
+processes. Unfortuntatlely the design of the on-disk layout of a Git
+repository does not allow for 100% race free operations when accessed by
+multiple actors concurrently. These design shortcomings are more likely
+to impact the operations of busy repositories since racy conditions are
+more likely to occur when there are more concurrent operations. Since
+most Gerrit servers are expected to run without interruptions, Git
+garbage collection likely needs to be run during normal operational hours.
+When it runs, it adds to the concurrency of the overall accesses. Given
+that many of the operations in garbage collection involve deleting files
+and directories, it has a higher chance of impacting other ongoing
+operations than most other operations.
+
+=== Interrupted Operations
+
+When Git garbage collection deletes a file or directory that is
+currently in use by an ongoing operation, it can cause that operation to
+fail. These sorts of failures are often single shot failures, i.e. the
+operation will succeed if tried again. An example of such a failure is
+when a pack file is deleted while Gerrit is sending an object in the
+file over the network to a user performing a clone or fetch. Usually
+pack files are only deleted when the referenced objects in them have
+been repacked and thus copied to a new pack file. So performing the same
+operation again after the fetch will likely send the same object from
+the new pack instead of the deleted one, and the operation will succeed.
+
+=== Data Loss
+
+It is possible for data loss to occur when Git garbage collection runs.
+This is very rare, but it can happen. This can happen when an object is
+believed to be unreferenced when object repacking is running, and then
+garbage collection deletes it. This can happen because even though an
+object may indeed be unreferenced when object repacking begins and
+reachability of all objects is determined, it can become referenced by
+another concurrent operation after this unreferenced determination but
+before it gets deleted. When this happens, a new reference can be
+created which points to a now missing object, and this will result in a
+loss.
+
+== Reducing Git Garbage Collection Impacts
+
+JGit has a `preserved` directory feature which is intended to reduce
+some of the impacts of Git garbage collection, and Gerrit can take
+advantage of the feature too. The `preserved` directory is a
+subdirectory of a repository's `objects/pack` directory where JGit will
+move pack files that it would normally delete when `jgit gc` is invoked
+with the `--preserve-oldpacks` option. It will later delete these files
+the next time that `jgit gc` is run if it is invoked with the
+`--prune-preserved` option. Using these flags together on every `jgit gc`
+invocation means that packfiles will get an extended lifetime by one
+full garbage collection cycle. Since an atomic move is used to move these
+files, any open references to them will continue to work, even on NFS. On
+a busy repository, preserving pack files can make operations much more
+reliable, and interrupted operations should almost entirely disappear.
+
+Moving files to the `preserved` directory also has the ability to reduce
+data loss. If JGit cannot find an object it needs in its current object
+DB, it will look into the `preserved` directory as a last resort. If it
+finds the object in a pack file there, it will restore the
+slated-to-be-deleted pack file back to the original `objects/pack`
+directory effectively "undeleting" it and making all the objects in it
+available again. When this happens, data loss is prevented.
+
+One advantage of restoring preserved packfiles in this way when an
+object is referenced in them, is that it makes loosening unreferenced
+objects during Git garbage collection, which is a potentially expensive,
+wasteful, and performance impacting operation, no longer desirable. It
+is recommended that if you use Git for garbage collection, that you use
+the `-a` option to `git repack` instead of the `-A` option to no longer
+perform this loosening.
+
+When Git is used for garbage collection instead of JGit, it is fairly
+easy to wrap `git gc` or `git repack` with a small script which has a
+`--prune-preserved` option which behaves as mentioned above by deleting
+any pack files currently in the preserved directory, and also has a
+`--preserve-oldpacks` option which then hardlinks all the currently
+existing pack files from the `objects/pack` directory into the
+`preserved` directory right before calling the real Git command. This
+approach will then behave similarly to `jgit gc` with respect to
+preserving pack files.
+
+GERRIT
+------
+Part of link:index.html[Gerrit Code Review]
+
+SEARCHBOX
+---------
diff --git a/Documentation/user-review-ui.txt b/Documentation/user-review-ui.txt
index 9eacaed..156ea02 100644
--- a/Documentation/user-review-ui.txt
+++ b/Documentation/user-review-ui.txt
@@ -93,13 +93,6 @@
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/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:
+
The reviewers of the change are displayed as chip tokens.
@@ -163,10 +156,6 @@
+
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/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:
+
Depending on the change state and the permissions of the user, different
@@ -658,7 +647,7 @@
comments; a summary comment is only added if the reply popup panel is
open when the quick approve button is clicked.
-image::images/gwt-user-review-ui-change-screen-quick-approve.png[width=800, link="images/gwt-user-review-ui-change-screen-quick-approve.png"]
+image::images/user-review-ui-change-screen-quick-approve.png[width=800, link="images/gwt-user-review-ui-change-screen-quick-approve.png"]
[[history]]
=== History
@@ -739,28 +728,12 @@
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
+The checkbox in front of the file name allows the
patch to be marked as reviewed. The link:#mark-reviewed[Mark Reviewed]
diff preference allows to control whether the files should be
automatically marked as reviewed when they are viewed.
-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.
-This provides a good overview of the lines in the patch that are
-relevant for reviewing. By clicking on an annotation one can quickly
-navigate to the corresponding line in the patch.
-
-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
-left and right side or by displaying inline comments is shown as a
-vertical red bar in the line number column. This prevents a gap from
-being mistaken for blank lines in the file
-
-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"]
+image::images/user-review-ui-side-by-side-diff-screen-reviewed.png[width=800, link="images/gwt-user-review-ui-side-by-side-diff-screen-reviewed.png"]
[[patch-set-selection]]
In the header, on each side, the list of patch sets is shown. Clicking
@@ -919,52 +892,10 @@
[[file-level-comments]]
=== File Level Comments
-Comments that apply to a whole file can be added on file level.
+File level comments are added by clicking the 'File' header at the top
+of the file.
-File level comments are added by clicking on the comment icon in the
-header above the file.
-
-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/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
-
-For searching within a patch file, a Vim-like search is supported.
-Typing `/` opens the search box. Typing in the search box immediately
-highlights matches in the patch file with a yellow background. Using
-JavaScript regular expressions in the search term is supported. The
-search is case insensitive. After confirming the search by `ENTER` one
-can navigate between the matches by `n` / `N` to go to the next /
-previous match. Skipped lines are automatically expanded if they
-contain a match and one navigates to it.
-
-For additional possibilities to search please check the
-link:http://www.vim.org/docs.php[Vim documentation,role=external,window=_blank]. There are other
-useful ways to search, e.g. while the cursor is on a word, pressing `*`
-or `#` searches for the next or previous occurrence of the word.
-
-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/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
-
-Vim-like commands can be used to navigate within a patch file:
-
-- `h` / `j` / `k` / `l` moves the cursor left / down / up / right
-- `0` / `$` moves the cursor to the start / end of the line
-- `gg` / `G` moves to cursor to the start / end of the file
-- `Ctrl-D` / `Ctrl-U` scrolls downwards / upwards
-
-Please check the link:http://www.vim.org/docs.php[Vim documentation,role=external,window=_blank]
-for further information.
+image::images/user-review-ui-side-by-side-diff-screen-file-level-comment.png[width=800, link="images/gwt-user-review-ui-side-by-side-diff-screen-file-level-comment.png"]
[[diff-preferences]]
=== Diff Preferences
@@ -974,27 +905,10 @@
preferences. The diff preferences can be accessed by clicking on the
settings icon in the screen header.
-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
-saved permanently. Clicking on the `Apply` button applies the new
-diff preferences to the current screen, but they are discarded when the
-screen is refreshed. The `Save` button is only available if the user is
-signed in.
-
-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"]
+image::images/user-review-ui-side-by-side-diff-screen-preferences.png[width=800, link="images/gwt-user-review-ui-side-by-side-diff-screen-preferences.png"]
The following diff preferences can be configured:
-- [[theme]]`Theme`:
-+
-Controls the theme that is used to render the file content.
-+
-E.g. users could choose to work with a dark theme.
-+
-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`:
+
Controls whether differences in whitespace should be ignored or not.
@@ -1003,11 +917,11 @@
+
All differences in whitespace are highlighted.
+
-** `At Line End`:
+** `Trailing`:
+
Whitespace differences at the end of lines are ignored.
+
-** `Leading, At Line End`:
+** `Leading, Trailing`:
+
Whitespace differences at the beginning and end of lines are ignored.
+
@@ -1021,11 +935,7 @@
- [[columns]]`Columns`:
+
-Sets the preferred line length. At this position a vertical dashed line
-is displayed so that one can easily detect lines the exceed the
-preferred line length.
-+
-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"]
+Sets the preferred line length. At this position, lines are wrapped.
- [[lines-of-context]]`Lines Of Context`:
+
@@ -1042,84 +952,28 @@
If many lines are skipped there are additional links to expand the
context by ten lines before and after the skipped block.
+
-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"]
+image::images/user-review-ui-side-by-side-diff-screen-expand-skipped-lines.png[width=800, link="images/gwt-user-review-ui-side-by-side-diff-screen-expand-skipped-lines.png"]
- [[syntax-highlighting]]`Syntax Highlighting`:
+
Controls whether syntax highlighting should be enabled.
+
The language for the syntax highlighting is automatically detected from
-the file extension. The language can also be set manually by selecting
-it from the `Language` drop-down list.
-+
-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"]
+the file extension.
-- [[whitespace-errors]]`Whitespace Errors`:
+- [[whitespace-errors]]`Show trailing whitespace`:
+
-Controls whether whitespace errors are highlighted.
+Controls whether trailing whitespace is highlighted.
- [[show-tabs]]`Show Tabs`:
+
Controls whether tabs are highlighted.
-- [[line-numbers]]`Line Numbers`:
-+
-Controls whether line numbers are shown.
-
-- [[empty-pane]]`Empty Pane`:
-+
-Controls whether empty panes are shown or not. The Left pane is empty when a
-file was added; the right pane is empty when a file was deleted.
-
-- [[left-side]]`Left Side`:
-+
-Controls whether the left side is shown. This preference is not
-persistent and is ignored by the `Save` button. Every time a
-patch diff is opened, this preference is reset to `Show`.
-
-- [[top-menu]]`Top Menu`:
-+
-Controls whether the top menu is shown.
-
-- [[auto-hide-diff-table-header]]`Auto Hide Diff Table Header`:
-+
-Controls whether the diff table header should be automatically hidden
-when scrolling down more than half of a page.
-
- [[mark-reviewed]]`Mark Reviewed`:
+
Controls whether the files of the patch set should be automatically
marked as reviewed when they are viewed.
-- [[expand-all-comments]]`Expand All Comments`:
-+
-Controls whether all comments should be automatically expanded.
-
-- [[render]]`Render`:
-+
-Controls how patch files that exceed the screen size are rendered.
-+
-If `Fast` is selected file contents which are outside of the visible
-area are not attached to the browser's DOM tree. This makes the
-rendering fast, but searching by `Ctrl+F` only finds content which is
-in the visible area.
-+
-If `Slow` is selected all file contents are attached to the browser's
-DOM tree, which makes the rendering slow for large files. The advantage
-of this setting is that `Ctrl+F` can be used to search in the complete
-file.
-+
-Large files that exceed 4000 lines will not be fully rendered.
-
-- [[line-wrapping]]`Line Wrapping`:
-+
-Controls whether to enable line wrapping or not.
-+
-If `false` is selected then line wrapping is disabled.
-This is the default option.
-+
-If `true` is selected then line wrapping is enabled.
-
[[keyboard-shortcuts]]
== Keyboard Shortcuts
diff --git a/java/com/google/gerrit/server/change/ActionJson.java b/java/com/google/gerrit/server/change/ActionJson.java
index 6f28dad..54ebf40 100644
--- a/java/com/google/gerrit/server/change/ActionJson.java
+++ b/java/com/google/gerrit/server/change/ActionJson.java
@@ -31,7 +31,7 @@
import com.google.gerrit.extensions.webui.UiAction;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.extensions.webui.UiActions;
-import com.google.gerrit.server.notedb.ChangeNotes;
+import com.google.gerrit.server.query.change.ChangeData;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
@@ -89,9 +89,9 @@
return Lists.newArrayList(visitorSet);
}
- void addChangeActions(ChangeInfo to, ChangeNotes notes) {
+ void addChangeActions(ChangeInfo to, ChangeData changeData) {
List<ActionVisitor> visitors = visitors();
- to.actions = toActionMap(notes, visitors, copy(visitors, to));
+ to.actions = toActionMap(changeData, visitors, copy(visitors, to));
}
void addRevisionActions(@Nullable ChangeInfo changeInfo, RevisionInfo to, RevisionResource rsrc) {
@@ -167,7 +167,7 @@
}
private Map<String, ActionInfo> toActionMap(
- ChangeNotes notes, List<ActionVisitor> visitors, ChangeInfo changeInfo) {
+ ChangeData changeData, List<ActionVisitor> visitors, ChangeInfo changeInfo) {
CurrentUser user = userProvider.get();
Map<String, ActionInfo> out = new LinkedHashMap<>();
if (!user.isIdentifiedUser()) {
@@ -175,12 +175,12 @@
}
Iterable<UiAction.Description> descs =
- uiActions.from(changeViews, changeResourceFactory.create(notes, user));
+ uiActions.from(changeViews, changeResourceFactory.create(changeData, user));
// The followup action is a client-side only operation that does not
// have a server side handler. It must be manually registered into the
// resulting action map.
- if (!notes.getChange().isAbandoned()) {
+ if (!changeData.change().isAbandoned()) {
UiAction.Description descr = new UiAction.Description();
PrivateInternals_UiActionDescription.setId(descr, "followup");
PrivateInternals_UiActionDescription.setMethod(descr, "POST");
diff --git a/java/com/google/gerrit/server/change/ChangeJson.java b/java/com/google/gerrit/server/change/ChangeJson.java
index 1e9bc5f3..029f231 100644
--- a/java/com/google/gerrit/server/change/ChangeJson.java
+++ b/java/com/google/gerrit/server/change/ChangeJson.java
@@ -678,7 +678,7 @@
}
if (has(CURRENT_ACTIONS) || has(CHANGE_ACTIONS)) {
- actionJson.addChangeActions(out, cd.notes());
+ actionJson.addChangeActions(out, cd);
}
if (has(TRACKING_IDS)) {
diff --git a/java/com/google/gerrit/server/change/ChangeResource.java b/java/com/google/gerrit/server/change/ChangeResource.java
index 5a1798d..3729b59 100644
--- a/java/com/google/gerrit/server/change/ChangeResource.java
+++ b/java/com/google/gerrit/server/change/ChangeResource.java
@@ -44,9 +44,10 @@
import com.google.gerrit.server.plugincontext.PluginSetContext;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectState;
-import com.google.inject.Inject;
+import com.google.gerrit.server.query.change.ChangeData;
import com.google.inject.TypeLiteral;
import com.google.inject.assistedinject.Assisted;
+import com.google.inject.assistedinject.AssistedInject;
import java.util.HashSet;
import java.util.Optional;
import java.util.Set;
@@ -66,6 +67,8 @@
public interface Factory {
ChangeResource create(ChangeNotes notes, CurrentUser user);
+
+ ChangeResource create(ChangeData changeData, CurrentUser user);
}
private static final String ZERO_ID_STRING = ObjectId.zeroId().name();
@@ -77,10 +80,10 @@
private final StarredChangesUtil starredChangesUtil;
private final ProjectCache projectCache;
private final PluginSetContext<ChangeETagComputation> changeETagComputation;
- private final ChangeNotes notes;
+ private final ChangeData changeData;
private final CurrentUser user;
- @Inject
+ @AssistedInject
ChangeResource(
AccountCache accountCache,
ApprovalsUtil approvalUtil,
@@ -89,6 +92,7 @@
StarredChangesUtil starredChangesUtil,
ProjectCache projectCache,
PluginSetContext<ChangeETagComputation> changeETagComputation,
+ ChangeData.Factory changeDataFactory,
@Assisted ChangeNotes notes,
@Assisted CurrentUser user) {
this.accountCache = accountCache;
@@ -98,12 +102,34 @@
this.starredChangesUtil = starredChangesUtil;
this.projectCache = projectCache;
this.changeETagComputation = changeETagComputation;
- this.notes = notes;
+ this.changeData = changeDataFactory.create(notes);
+ this.user = user;
+ }
+
+ @AssistedInject
+ ChangeResource(
+ AccountCache accountCache,
+ ApprovalsUtil approvalUtil,
+ PatchSetUtil patchSetUtil,
+ PermissionBackend permissionBackend,
+ StarredChangesUtil starredChangesUtil,
+ ProjectCache projectCache,
+ PluginSetContext<ChangeETagComputation> changeETagComputation,
+ @Assisted ChangeData changeData,
+ @Assisted CurrentUser user) {
+ this.accountCache = accountCache;
+ this.approvalUtil = approvalUtil;
+ this.patchSetUtil = patchSetUtil;
+ this.permissionBackend = permissionBackend;
+ this.starredChangesUtil = starredChangesUtil;
+ this.projectCache = projectCache;
+ this.changeETagComputation = changeETagComputation;
+ this.changeData = changeData;
this.user = user;
}
public PermissionBackend.ForChange permissions() {
- return permissionBackend.user(user).change(notes);
+ return permissionBackend.user(user).change(getNotes());
}
public CurrentUser getUser() {
@@ -111,7 +137,7 @@
}
public Change.Id getId() {
- return notes.getChangeId();
+ return changeData.getId();
}
/** @return true if {@link #getUser()} is the change's owner. */
@@ -121,7 +147,7 @@
}
public Change getChange() {
- return notes.getChange();
+ return changeData.change();
}
public Project.NameKey getProject() {
@@ -129,7 +155,11 @@
}
public ChangeNotes getNotes() {
- return notes;
+ return changeData.notes();
+ }
+
+ public ChangeData getChangeData() {
+ return changeData;
}
// This includes all information relevant for ETag computation
@@ -153,7 +183,7 @@
accounts.add(getChange().getAssignee());
}
try {
- patchSetUtil.byChange(notes).stream().map(PatchSet::uploader).forEach(accounts::add);
+ patchSetUtil.byChange(getNotes()).stream().map(PatchSet::uploader).forEach(accounts::add);
// It's intentional to include the states for *all* reviewers into the ETag computation.
// We need the states of all current reviewers and CCs because they are part of ChangeInfo.
@@ -162,7 +192,7 @@
// set of accounts that posted a message is too expensive. However everyone who posts a
// message is automatically added as reviewer. Hence if we include removed reviewers we can
// be sure that we have all accounts that posted messages on the change.
- accounts.addAll(approvalUtil.getReviewers(notes).all());
+ accounts.addAll(approvalUtil.getReviewers(getNotes()).all());
} catch (StorageException e) {
// This ETag will be invalidated if it loads next time.
}
@@ -178,7 +208,7 @@
ObjectId noteId;
try {
- noteId = notes.loadRevision();
+ noteId = getNotes().loadRevision();
} catch (StorageException e) {
noteId = null; // This ETag will be invalidated if it loads next time.
}
@@ -194,7 +224,7 @@
changeETagComputation.runEach(
c -> {
- String pluginETag = c.getETag(notes.getProjectName(), notes.getChangeId());
+ String pluginETag = c.getETag(changeData.project(), changeData.getId());
if (pluginETag != null) {
h.putString(pluginETag, UTF_8);
}
@@ -207,8 +237,8 @@
TraceContext.newTimer(
"Compute change ETag",
Metadata.builder()
- .changeId(notes.getChangeId().get())
- .projectName(notes.getProjectName().get())
+ .changeId(changeData.getId().get())
+ .projectName(changeData.project().get())
.build())) {
Hasher h = Hashing.murmur3_128().newHasher();
if (user.isIdentifiedUser()) {
diff --git a/java/com/google/gerrit/server/change/RevisionJson.java b/java/com/google/gerrit/server/change/RevisionJson.java
index 558bdba..b702440 100644
--- a/java/com/google/gerrit/server/change/RevisionJson.java
+++ b/java/com/google/gerrit/server/change/RevisionJson.java
@@ -327,7 +327,7 @@
actionJson.addRevisionActions(
changeInfo,
out,
- new RevisionResource(changeResourceFactory.create(cd.notes(), userProvider.get()), in));
+ new RevisionResource(changeResourceFactory.create(cd, userProvider.get()), in));
}
if (gpgApi.isEnabled() && has(PUSH_CERTIFICATES)) {
diff --git a/java/com/google/gerrit/server/patch/FilePathAdapter.java b/java/com/google/gerrit/server/patch/FilePathAdapter.java
index 7f34cf1..ccd1466 100644
--- a/java/com/google/gerrit/server/patch/FilePathAdapter.java
+++ b/java/com/google/gerrit/server/patch/FilePathAdapter.java
@@ -1,3 +1,17 @@
+// Copyright (C) 2021 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
package com.google.gerrit.server.patch;
import com.google.gerrit.entities.Patch.ChangeType;
diff --git a/java/com/google/gerrit/server/restapi/change/Submit.java b/java/com/google/gerrit/server/restapi/change/Submit.java
index 790b2db..ba0720a 100644
--- a/java/com/google/gerrit/server/restapi/change/Submit.java
+++ b/java/com/google/gerrit/server/restapi/change/Submit.java
@@ -112,7 +112,6 @@
private final GitRepositoryManager repoManager;
private final PermissionBackend permissionBackend;
- private final ChangeData.Factory changeDataFactory;
private final Provider<MergeOp> mergeOpProvider;
private final Provider<MergeSuperSet> mergeSuperSet;
private final AccountResolver accountResolver;
@@ -131,7 +130,6 @@
Submit(
GitRepositoryManager repoManager,
PermissionBackend permissionBackend,
- ChangeData.Factory changeDataFactory,
Provider<MergeOp> mergeOpProvider,
Provider<MergeSuperSet> mergeSuperSet,
AccountResolver accountResolver,
@@ -141,7 +139,6 @@
ProjectCache projectCache) {
this.repoManager = repoManager;
this.permissionBackend = permissionBackend;
- this.changeDataFactory = changeDataFactory;
this.mergeOpProvider = mergeOpProvider;
this.mergeSuperSet = mergeSuperSet;
this.accountResolver = accountResolver;
@@ -311,7 +308,7 @@
throw new StorageException("Could not determine problems for the change", e);
}
- ChangeData cd = changeDataFactory.create(resource.getNotes());
+ ChangeData cd = resource.getChangeResource().getChangeData();
try {
MergeOp.checkSubmitRule(cd, false);
} catch (ResourceConflictException e) {
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeSubmitRequirementIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeSubmitRequirementIT.java
index 9cb9058..c6b57da 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeSubmitRequirementIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeSubmitRequirementIT.java
@@ -24,6 +24,7 @@
import com.google.gerrit.extensions.annotations.Exports;
import com.google.gerrit.extensions.api.changes.ChangeApi;
import com.google.gerrit.extensions.api.changes.ReviewInput;
+import com.google.gerrit.extensions.client.ListChangesOption;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.LegacySubmitRequirementInfo;
import com.google.gerrit.extensions.config.FactoryModule;
@@ -35,6 +36,7 @@
import java.util.ArrayList;
import java.util.List;
import java.util.Optional;
+import java.util.concurrent.atomic.AtomicInteger;
import org.junit.Test;
public class ChangeSubmitRequirementIT extends AbstractDaemonTest {
@@ -58,7 +60,7 @@
};
}
- @Inject CustomSubmitRule rule;
+ @Inject private CustomSubmitRule rule;
@Test
public void submitRequirementIsPropagated() throws Exception {
@@ -170,6 +172,35 @@
assertThat(result.get(0).changeId).isEqualTo(change.info().changeId);
}
+ @Test
+ public void submitRuleIsInvokedOnlyOnceWhenGettingChangeDetails() throws Exception {
+ PushOneCommit.Result r = createChange("Some Change", "foo.txt", "some content");
+ String changeId = r.getChangeId();
+
+ rule.numberOfEvaluations.set(0);
+ gApi.changes()
+ .id(changeId)
+ .get(ListChangesOption.ALL_REVISIONS, ListChangesOption.CURRENT_ACTIONS);
+
+ // Submit rules are computed freshly, but only once.
+ assertThat(rule.numberOfEvaluations.get()).isEqualTo(1);
+ }
+
+ @Test
+ public void submitRuleIsNotInvokedWhenQueryingChange() throws Exception {
+ PushOneCommit.Result r = createChange("Some Change", "foo.txt", "some content");
+ String changeId = r.getChangeId();
+
+ rule.numberOfEvaluations.set(0);
+ gApi.changes()
+ .query(changeId)
+ .withOptions(ListChangesOption.ALL_REVISIONS, ListChangesOption.CURRENT_ACTIONS)
+ .get();
+
+ // Submit rule evaluation results from the change index are reused
+ assertThat(rule.numberOfEvaluations.get()).isEqualTo(0);
+ }
+
private List<ChangeInfo> queryIsSubmittable() throws Exception {
return gApi.changes().query("is:submittable project:" + project.get()).get();
}
@@ -189,6 +220,7 @@
@Singleton
private static class CustomSubmitRule implements SubmitRule {
private Optional<SubmitRecord.Status> recordStatus = Optional.empty();
+ private AtomicInteger numberOfEvaluations = new AtomicInteger();
public void block(boolean block) {
this.status(block ? Optional.of(SubmitRecord.Status.NOT_READY) : Optional.empty());
@@ -200,6 +232,7 @@
@Override
public Optional<SubmitRecord> evaluate(ChangeData changeData) {
+ numberOfEvaluations.incrementAndGet();
if (this.recordStatus.isPresent()) {
SubmitRecord record = new SubmitRecord();
record.labels = new ArrayList<>();
diff --git a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_html.ts b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_html.ts
index 8c8a886..ed38d6f 100644
--- a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_html.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_html.ts
@@ -153,7 +153,14 @@
<section
class$="[[_computeDisplayState(_showAllSections, change, _SECTION.UPDATED)]]"
>
- <span class="title">Updated</span>
+ <span class="title">
+ <gr-tooltip-content
+ has-tooltip=""
+ title="Last update of (meta)data for this change."
+ >
+ Updated
+ </gr-tooltip-content>
+ </span>
<span class="value">
<gr-date-formatter
has-tooltip=""
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 874f572..eb943f7 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
@@ -345,7 +345,8 @@
contextGroups,
showAbove,
showBelow,
- numLines
+ numLines,
+ viewMode
)
);
if (showBelow) {
@@ -364,19 +365,36 @@
contextGroups: GrDiffGroup[],
showAbove: boolean,
showBelow: boolean,
- numLines: number
+ numLines: number,
+ viewMode: DiffViewMode
): HTMLElement {
- const row = this._createElement('tr', 'contextDivider');
- if (!(showAbove && showBelow)) {
- row.classList.add('collapsed');
+ const row = this._createElement('tr', 'dividerRow');
+ if (showAbove && !showBelow) {
+ row.classList.add('showAboveOnly');
+ } else if (!showAbove && showBelow) {
+ row.classList.add('showBelowOnly');
+ } else {
+ // Note that !showAbove && !showBelow also intentionally creates
+ // "showBoth". This means the file is completely collapsed, which is
+ // unusual, but at least happens in one test.
+ row.classList.add('showBoth');
}
- const element = this._createElement('td', 'dividerCell');
- row.appendChild(element);
+ row.appendChild(this._createBlameCell(0));
+ if (viewMode === DiffViewMode.SIDE_BY_SIDE) {
+ row.appendChild(this._createElement('td'));
+ }
+
+ const cell = this._createElement('td', 'dividerCell');
+ cell.setAttribute('colspan', '3');
+ row.appendChild(cell);
+ const verticalFlex = this._createElement('div', 'verticalFlex');
+ cell.appendChild(verticalFlex);
+ const horizontalFlex = this._createElement('div', 'horizontalFlex');
+ verticalFlex.appendChild(horizontalFlex);
const showAllContainer = this._createElement('div', 'aboveBelowButtons');
- element.appendChild(showAllContainer);
-
+ horizontalFlex.appendChild(showAllContainer);
const showAllButton = this._createContextButton(
ContextButtonType.ALL,
section,
@@ -415,7 +433,7 @@
)
);
}
- element.appendChild(container);
+ horizontalFlex.appendChild(container);
}
return row;
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 a93d5b5..e254728 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
@@ -318,38 +318,64 @@
height: calc(var(--line-height-normal) + var(--spacing-s));
}
- .contextDivider {
- height: var(--divider-height);
- /* Create a positioning context. */
- transform: translateX(0px);
+ .dividerCell {
+ vertical-align: top;
}
- .contextDivider.collapsed {
- /* Hide divider gap, but still show child elements (expansion buttons). */
+ .dividerRow.showBoth .dividerCell {
+ height: var(--divider-height);
+ }
+ .dividerRow.showAboveOnly .dividerCell,
+ .dividerRow.showBelowOnly .dividerCell {
height: 0;
}
- .dividerCell {
- width: 100%;
- height: 100%;
+
+ .verticalFlex {
+ display: flex;
+ flex-direction: column;
+ position: relative;
+ }
+ .dividerRow.showBoth .verticalFlex {
+ justify-content: center;
+ margin-top: calc(0px - var(--line-height-normal) - var(--spacing-s));
+ margin-bottom: calc(0px - var(--line-height-normal) - var(--spacing-s));
+ height: calc(
+ 2 * var(--line-height-normal) + 2 * var(--spacing-s) +
+ var(--divider-height) - 1px
+ );
+ }
+ .dividerRow.showAboveOnly .verticalFlex {
+ justify-content: flex-end;
+ /* margin-top has to make room for height+1px. */
+ margin-top: calc(-1px - var(--line-height-normal) - var(--spacing-s));
+ height: calc(var(--line-height-normal) + var(--spacing-s));
+ }
+ .dividerRow.showBelowOnly .verticalFlex {
+ justify-content: flex-start;
+ /* This just pushes the container down 1 pixel as to render below the
+ 1px border-top of the padding row below. The same could be achieved
+ by position:relative; top:1px.*/
+ margin-top: 1px;
+ margin-bottom: calc(0px - var(--line-height-normal) - var(--spacing-s));
+ }
+
+ .horizontalFlex {
display: flex;
justify-content: center;
- position: absolute;
- top: 0;
- left: 0;
+ }
+ .dividerRow.showBoth .horizontalFlex {
+ align-items: center;
+ }
+ .dividerRow.showAboveOnly .horizontalFlex {
+ align-items: end;
+ }
+ .dividerRow.showBelowOnly .horizontalFlex {
+ align-items: start;
}
.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;
@@ -368,15 +394,17 @@
.aboveBelowButtons {
display: flex;
flex-direction: column;
+ justify-content: center;
margin-left: var(--spacing-m);
- position: relative;
}
.aboveBelowButtons:first-child {
margin-left: 0;
}
+ .dividerRow.showBoth .aboveButton {
+ /* The size of the gap between the above and below button. */
+ margin-bottom: calc(var(--divider-height) + 1px);
+ }
.aboveButton {
- /* Display over preceding content / background placeholder. */
- transform: translateY(-100%);
--gr-button: {
color: var(--diff-context-control-color);
border-style: solid;
@@ -393,8 +421,6 @@
}
}
.belowButton {
- /* Display over following content / background placeholder. */
- top: calc(100% + var(--divider-border));
--gr-button: {
color: var(--diff-context-control-color);
border-style: solid;
diff --git a/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog.ts b/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog.ts
index 39a5d54..3763fbf 100644
--- a/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog.ts
+++ b/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog.ts
@@ -30,7 +30,6 @@
$: {
name: HTMLInputElement;
username: HTMLInputElement;
- email: HTMLSelectElement;
};
}
@@ -73,11 +72,17 @@
_serverConfig?: ServerInfo;
@property({
- computed: '_computeUsernameMutable(_serverConfig,_account.username)',
+ computed: '_computeUsernameMutable(_account.username)',
type: Boolean,
})
_usernameMutable = false;
+ @property({type: Boolean})
+ _hasUsernameChange?: boolean;
+
+ @property({type: String, observer: '_usernameChanged'})
+ _username?: string;
+
private readonly restApiService = appContext.restApiService;
/** @override */
@@ -86,26 +91,17 @@
this._ensureAttribute('role', 'dialog');
}
- _computeUsernameMutable(config?: ServerInfo, username?: string) {
- // Polymer 2: check for undefined
- // username is not being checked for undefined as we want to avoid
- // setting it null explicitly to trigger the computation
- if (config === undefined) {
- return false;
- }
-
- return (
- config.auth.editable_account_fields.includes(
- EditableAccountField.USER_NAME
- ) && !username
- );
- }
-
loadData() {
this._loading = true;
const loadAccount = this.restApiService.getAccount().then(account => {
- this._account = {...this._account, ...account};
+ if (!account) return;
+ this._hasUsernameChange = false;
+ // Provide predefined value for username to trigger computation of
+ // username mutability.
+ account.username = account.username || '';
+ this._account = account;
+ this._username = account.username;
});
const loadConfig = this.restApiService.getConfig().then(config => {
@@ -117,17 +113,34 @@
});
}
+ _usernameChanged() {
+ if (this._loading || !this._account) {
+ return;
+ }
+ this._hasUsernameChange =
+ (this._account.username || '') !== (this._username || '');
+ }
+
+ _computeUsernameMutable(username?: string) {
+ // Username may not be changed once it is set.
+ return !username;
+ }
+
+ _computeUsernameEditable(config?: ServerInfo) {
+ return !!config?.auth.editable_account_fields.includes(
+ EditableAccountField.USER_NAME
+ );
+ }
+
_save() {
this._saving = true;
- const promises = [
- this.restApiService.setAccountName(this.$.name.value),
- this.restApiService.setPreferredAccountEmail(this.$.email.value || ''),
- ];
- if (this._usernameMutable) {
- promises.push(
- this.restApiService.setAccountUsername(this.$.username.value)
- );
+ const promises = [this.restApiService.setAccountName(this.$.name.value)];
+
+ // Note that we are intentionally not acting on this._username being the
+ // empty string (which is falsy).
+ if (this._hasUsernameChange && this._usernameMutable && this._username) {
+ promises.push(this.restApiService.setAccountUsername(this._username));
}
return Promise.all(promises).then(() => {
@@ -151,12 +164,8 @@
fireEvent(this, 'close');
}
- _computeSaveDisabled(name?: string, email?: string, saving?: boolean) {
- return !name || !email || saving;
- }
-
- _computeUsernameClass(usernameMutable: boolean) {
- return usernameMutable ? '' : 'hide';
+ _computeSaveDisabled(name?: string, username?: string, saving?: boolean) {
+ return saving || (!name && !username);
}
@observe('_loading')
diff --git a/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog_html.ts b/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog_html.ts
index a1d6a5c..4b86709 100644
--- a/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog_html.ts
+++ b/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog_html.ts
@@ -59,9 +59,6 @@
input {
width: 20em;
}
- section.hide {
- display: none;
- }
</style>
<div class="container gr-form-styles">
<header>Please confirm your contact information</header>
@@ -75,36 +72,36 @@
</p>
<hr />
<section>
- <div class="title">Full Name</div>
- <iron-input bind-value="{{_account.name}}">
- <input
- is="iron-input"
- id="name"
- bind-value="{{_account.name}}"
- disabled="[[_saving]]"
- />
- </iron-input>
+ <span class="title">Full Name</span>
+ <span class="value">
+ <iron-input bind-value="{{_account.name}}">
+ <input
+ is="iron-input"
+ id="name"
+ bind-value="{{_account.name}}"
+ disabled="[[_saving]]"
+ />
+ </iron-input>
+ </span>
</section>
- <section class$="[[_computeUsernameClass(_usernameMutable)]]">
- <div class="title">Username</div>
- <iron-input bind-value="{{_account.username}}">
- <input
- is="iron-input"
- id="username"
- bind-value="{{_account.username}}"
- disabled="[[_saving]]"
- />
- </iron-input>
- </section>
- <section>
- <div class="title">Preferred Email</div>
- <select id="email" disabled="[[_saving]]">
- <option value="[[_account.email]]">[[_account.email]]</option>
- <template is="dom-repeat" items="[[_account.secondary_emails]]">
- <option value="[[item]]">[[item]]</option>
- </template>
- </select>
- </section>
+ <template is="dom-if" if="[[_computeUsernameEditable(_serverConfig)]]">
+ <section>
+ <span class="title">Username</span>
+ <span hidden$="[[_usernameMutable]]" class="value"
+ >[[_username]]</span
+ >
+ <span hidden$="[[!_usernameMutable]]" class="value">
+ <iron-input bind-value="{{_username}}">
+ <input
+ is="iron-input"
+ id="username"
+ bind-value="{{_username}}"
+ disabled="[[_saving]]"
+ />
+ </iron-input>
+ </span>
+ </section>
+ </template>
<hr />
<p>
More configuration options for Gerrit may be found in the
@@ -123,7 +120,7 @@
id="saveButton"
primary=""
link=""
- disabled="[[_computeSaveDisabled(_account.name, _account.email, _saving)]]"
+ disabled="[[_computeSaveDisabled(_account.name, _username, _saving)]]"
on-click="_handleSave"
>Save</gr-button
>
diff --git a/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog_test.ts b/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog_test.ts
index ccb7404..6c21e58 100644
--- a/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog_test.ts
+++ b/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog_test.ts
@@ -17,11 +17,7 @@
import '../../../test/common-test-setup-karma';
import {GrRegistrationDialog} from './gr-registration-dialog';
import {queryAndAssert, stubRestApi} from '../../../test/test-utils';
-import {
- AccountDetailInfo,
- EmailAddress,
- Timestamp,
-} from '../../../types/common';
+import {AccountDetailInfo, Timestamp} from '../../../types/common';
import * as MockInteractions from '@polymer/iron-test-helpers/mock-interactions';
import {AuthType, EditableAccountField} from '../../../constants/constants';
import {createServerInfo} from '../../../test/test-data-generators';
@@ -39,8 +35,6 @@
account = {
name: 'name',
- email: 'email' as EmailAddress,
- secondary_emails: ['email2', 'email3'],
registered_on: '2018-02-08 18:49:18.000000000' as Timestamp,
};
@@ -53,10 +47,6 @@
account.username = username;
return Promise.resolve();
});
- stubRestApi('setPreferredAccountEmail').callsFake(email => {
- account.email = email as EmailAddress;
- return Promise.resolve();
- });
stubRestApi('getConfig').returns(
Promise.resolve({
...createServerInfo(),
@@ -116,93 +106,59 @@
test('saves account details', done => {
flush(() => {
element.$.name.value = 'new name';
- element.$.username.value = 'new username';
- element.$.email.value = 'email3';
+
+ element.set('_account.username', '');
+ element._hasUsernameChange = false;
+ assert.isTrue(element._usernameMutable);
+
+ element.set('_username', 'new username');
// Nothing should be committed yet.
assert.equal(account.name, 'name');
assert.isNotOk(account.username);
- assert.equal(account.email, 'email' as EmailAddress);
// Save and verify new values are committed.
save()
.then(() => {
assert.equal(account.name, 'new name');
assert.equal(account.username, 'new username');
- assert.equal(account.email, 'email3' as EmailAddress);
})
.then(done);
});
});
- test('email select properly populated', done => {
- element._account = {
- email: 'foo' as EmailAddress,
- secondary_emails: ['bar', 'baz'],
- };
- flush(() => {
- assert.equal(element.$.email.value, 'foo');
- done();
- });
- });
-
test('save btn disabled', () => {
const compute = element._computeSaveDisabled;
assert.isTrue(compute('', '', false));
- assert.isTrue(compute('', 'test', false));
- assert.isTrue(compute('test', '', false));
+ assert.isFalse(compute('', 'test', false));
+ assert.isFalse(compute('test', '', false));
assert.isTrue(compute('test', 'test', true));
assert.isFalse(compute('test', 'test', false));
});
test('_computeUsernameMutable', () => {
+ assert.isTrue(element._computeUsernameMutable(undefined));
+ assert.isFalse(element._computeUsernameMutable('abc'));
+ });
+
+ test('_computeUsernameEditable', () => {
assert.isTrue(
- element._computeUsernameMutable(
- {
- ...createServerInfo(),
- auth: {
- auth_type: AuthType.HTTP,
- editable_account_fields: [EditableAccountField.USER_NAME],
- },
+ element._computeUsernameEditable({
+ ...createServerInfo(),
+ auth: {
+ auth_type: AuthType.HTTP,
+ editable_account_fields: [EditableAccountField.USER_NAME],
},
- undefined
- )
+ })
);
assert.isFalse(
- element._computeUsernameMutable(
- {
- ...createServerInfo(),
- auth: {
- auth_type: AuthType.HTTP,
- editable_account_fields: [EditableAccountField.USER_NAME],
- },
+ element._computeUsernameEditable({
+ ...createServerInfo(),
+ auth: {
+ auth_type: AuthType.HTTP,
+ editable_account_fields: [],
},
- 'abc'
- )
- );
- assert.isFalse(
- element._computeUsernameMutable(
- {
- ...createServerInfo(),
- auth: {
- auth_type: AuthType.HTTP,
- editable_account_fields: [],
- },
- },
- undefined
- )
- );
- assert.isFalse(
- element._computeUsernameMutable(
- {
- ...createServerInfo(),
- auth: {
- auth_type: AuthType.HTTP,
- editable_account_fields: [],
- },
- },
- 'abc'
- )
+ })
);
});
});
