Merge "Allow to set Code-Review label in AllProjects(Users)Creator"
diff --git a/Documentation/dev-polygerrit.txt b/Documentation/dev-polygerrit.txt
index 79049fc..5621d32 100644
--- a/Documentation/dev-polygerrit.txt
+++ b/Documentation/dev-polygerrit.txt
@@ -1,12 +1,15 @@
= PolyGerrit - GUI
-[IMPORTANT]
-PolyGerrit is still a beta feature. Some features may be missing.
-
== Configuring
By default both GWT and PolyGerrit UI are available to users.
+To make PolyGerrit the default UI but keep GWT as a secondary UI:
+----
+[gerrit]
+ ui = POLYGERRIT
+----
+
To disable GWT but not PolyGerrit:
----
[gerrit]
diff --git a/Documentation/images/inline-edit-add-file-page.png b/Documentation/images/inline-edit-add-file-page.png
new file mode 100644
index 0000000..1a761b4
--- /dev/null
+++ b/Documentation/images/inline-edit-add-file-page.png
Binary files differ
diff --git a/Documentation/images/inline-edit-create-change-form.png b/Documentation/images/inline-edit-create-change-form.png
new file mode 100644
index 0000000..7a93460
--- /dev/null
+++ b/Documentation/images/inline-edit-create-change-form.png
Binary files differ
diff --git a/Documentation/images/inline-edit-create-change.png b/Documentation/images/inline-edit-create-change.png
new file mode 100644
index 0000000..1df0421
--- /dev/null
+++ b/Documentation/images/inline-edit-create-change.png
Binary files differ
diff --git a/Documentation/images/inline-edit-delete-file.png b/Documentation/images/inline-edit-delete-file.png
new file mode 100644
index 0000000..1634e0f
--- /dev/null
+++ b/Documentation/images/inline-edit-delete-file.png
Binary files differ
diff --git a/Documentation/images/inline-edit-diff-screen.png b/Documentation/images/inline-edit-diff-screen.png
new file mode 100644
index 0000000..228484a
--- /dev/null
+++ b/Documentation/images/inline-edit-diff-screen.png
Binary files differ
diff --git a/Documentation/images/inline-edit-home-page.png b/Documentation/images/inline-edit-home-page.png
new file mode 100644
index 0000000..a1b8eb4
--- /dev/null
+++ b/Documentation/images/inline-edit-home-page.png
Binary files differ
diff --git a/Documentation/images/inline-edit-new-change-page.png b/Documentation/images/inline-edit-new-change-page.png
new file mode 100644
index 0000000..8a33dd6
--- /dev/null
+++ b/Documentation/images/inline-edit-new-change-page.png
Binary files differ
diff --git a/Documentation/images/inline-edit-open-file.png b/Documentation/images/inline-edit-open-file.png
new file mode 100644
index 0000000..a5422f5
--- /dev/null
+++ b/Documentation/images/inline-edit-open-file.png
Binary files differ
diff --git a/Documentation/images/inline-edit-prefill-files.png b/Documentation/images/inline-edit-prefill-files.png
new file mode 100644
index 0000000..0b2b766
--- /dev/null
+++ b/Documentation/images/inline-edit-prefill-files.png
Binary files differ
diff --git a/Documentation/images/inline-edit-review-message.png b/Documentation/images/inline-edit-review-message.png
new file mode 100644
index 0000000..bd76fad
--- /dev/null
+++ b/Documentation/images/inline-edit-review-message.png
Binary files differ
diff --git a/Documentation/images/inline-edit-start-review-button.png b/Documentation/images/inline-edit-start-review-button.png
new file mode 100644
index 0000000..df6350b
--- /dev/null
+++ b/Documentation/images/inline-edit-start-review-button.png
Binary files differ
diff --git a/Documentation/user-inline-edit.txt b/Documentation/user-inline-edit.txt
index bce8183..ada2560 100644
--- a/Documentation/user-inline-edit.txt
+++ b/Documentation/user-inline-edit.txt
@@ -1,191 +1,236 @@
-= Inline Edit
+= Creating and Editing Changes in the Gerrit Web Interface
-This page explains the workflow for creating and amending changes in the
-browser.
+== Overview
+
+The following content explains how to use the Gerrit web interface to create
+and edit changes. Use the web interface to make minor changes to files. When
+you create a change in the Gerrit user interface, you don't clone a Gerrit
+repository or use the CLI to issue Git commands — you perform your work
+directly in the Gerrit web interface.
+
+To learn more, see the link:intro-user.html[Gerrit User's Guide].
[[create-change]]
-== Creating a New Change
+== Creating a Change
-A new change can be created directly in the browser, meaning it is not necessary
-to clone the whole repository to make trivial changes.
+To create a change in the Gerrit web interface:
-The new change is created as a public
-link:user-upload.html#wip[work-in-progress change].
+. From the link:http://gerrit-review.googlesource.com[Gerrit Code Review]
+ dashboard, select Browse > Repositories.
-There are two different ways to create a new change:
+. Under Repository Name, click the name of the repository you want to work
+ on. For example, Public-Projects. To find a specific repository, enter all
+ or part of its name next to Filter:
++
+image::images/inline-edit-home-page.png[width=600]
-By clicking on the 'Create Change' button in the project screen:
+. In the left navigation panel for the repository you selected, click
+ Commands:
++
+image::images/inline-edit-create-change.png[width=350]
-[[create-change-from-project-info-screen]]
+. Under Repository Commands, click Create Change.
-image::images/inline-edit-create-change-project-screen.png[width=800, link="images/inline-edit-create-change-project-screen.png"]
+. In the Create Change window, enter the following information:
-The user can select the branch on which the new change should be created:
+ * Select branch for new change: Specify the destination branch of the
+ change.
-image::images/inline-edit-create-change-project-screen-dialog.png[width=800, link="images/inline-edit-create-change-project-screen-dialog.png"]
+ * Provide base commit SHA1 for change: Leave this field blank.
-By clicking the 'Follow-Up' button on the change screen, to create a new change
-based on the selected change.
++
+IMPORTANT: Git uses a unique SHA1 value to identify each and every commit (in
+other words, each Git commit generates a new SHA1 hash). This value differs
+from a Gerrit Change-Id, which is used by Gerrit to uniquely identify a
+change. The Gerrit Change-Id remains static throughout the life of a Gerrit
+change.
-[[create-change-from-change-screen]]
+ - Description: Briefly describe the change. Be sure to use the
+ link:dev-contributing.html#commit-message[Commit Message] format.
+ The first line becomes the subject of the change and is included in
+ the Commit Message. Because the message also appears on its own in
+ dashboards and in the results of `git log --pretty=oneline output`,
+ make the message informative and brief.
-image::images/inline-edit-create-follow-up-change.png[width=800, link="images/inline-edit-create-follow-up-change.png"]
+ - Private change: Select this option to designate this change as private.
+ Only you (and any reviewers you add) can see your private changes.
+
+. On the Create Change window, click Create. Gerrit creates a public Work
+ In Progress (WIP) change. Until the change is sent for review, it remains a
+ WIP and appears in _your_ dashboard only. In addition, all email
+ notifications are turned off.
+
+. Add the files you want to be reviewed.
+
+
+[[add-files]]
+== Adding a File to a Change
+
+Files can only be added to changes that have not been merged into the code
+base.
+
+To add a file to the change:
+
+. In the top left corner of the change, click Edit.
+. Next to Files, click Open:
+
++
+image::images/inline-edit-open-file.png[width=600]
+
+. In the Open File window, do one of the following:
+
+* To add an existing file:
+
+ ** Enter all or part of the file name in the text box. Gerrit automatically
+ populates a list of possible matching files:
++
+image::images/inline-edit-prefill-files.png[width=500]
++
+ ** Select the file you want to add to the change.
+ ** Click Open.
++
+_or,_
+
+* To create a new file, enter the name of the new file you want to add to the
+change and then click Open.
+
[[editing-change]]
-== Editing Changes
+== Modifying a Change
-To switch to edit mode, press the 'Edit' button at the top of the file list:
+To work on a file you've added to a change:
-[[switch-to-edit-mode]]
-image::images/inline-edit-enter-edit-mode-from-file-list.png[width=800, link="images/inline-edit-enter-edit-mode-from-file-list.png"]
+. On the change page, click the file name. When you add a new file to a
+ change, a blank page is displayed. When you add an existing file to a
+ change, the entire file is displayed.
-While in edit mode, it is possible to add new files to the change by clicking
-the 'Add...' button at the top of the file list.
+. Update the file and then click Save. You _must_ click Save to add the
+ file to the change.
-File changes can be reverted or files can be removed from the change or
-deleted files can be restored, by clicking the icons to the left of the file
-name.
+. To close the text editor and display the change page, click Close.
++
+When you save your work and close the file, the file is added to the change
+and the file name is listed in the Files section. The letter displayed to the
+left of the file name denotes the action performed on the file. In this case,
+one file was modified:
-To switch from edit mode back to review mode, click the 'Done Editing' button.
+- M: Modified
+- A: Added
+- D: Deleted
++
+image::images/inline-edit-add-file-page.png[width=650]
-image::images/inline-edit-file-list-in-edit-mode.png[width=800, link="images/inline-edit-file-list-in-edit-mode.png"]
+. When you're done editing and adding files, click Stop Editing.
-[[open-full-screen-editor]]
-While in edit mode, clicking on a file name in the file list opens a full
-screen editor for that file.
+. Click Publish Edit. When you publish an edit, you promote it to a regular
+ patch set. The special ref that represents the change is deleted when the
+ change is published.
-To save edits, click the 'Save' button or press `CTRL-S`. To return to the
-change screen, click the 'Close' button.
+Not happy with your edits? Click Delete Edit.
-Note that when editing the commit message, trailing blank lines will be stripped.
-image::images/inline-edit-full-screen-editor.png[width=800, link="images/inline-edit-full-screen-editor.png"]
+[[submit-change]]
+== Starting the Review
-If there are unsaved edits when the 'Close' button is pressed, a dialog will
-pop up asking to confirm the edits.
+When you start a review, Gerrit removes the WIP designation and submits
+the change to code review. The change appears in other Gerrit dashboards and
+reviewers are notified when the change is updated.
-image::images/inline-edit-confirm-unsaved-edits.png[width=800, link="images/inline-edit-confirm-unsaved-edits.png"]
+To start a review:
-To discard the unsaved edits and return to the change screen, click the 'OK'
-button. To continue editing, click 'Cancel'.
+. Open the change and then click Start Review:
++
+image::images/inline-edit-start-review-button.png[width=400]
-[[switch-to-edit-mode-from-side-by-side]]
+. In the change notification form:
-While in review mode, it is possible to switch directly to edit mode and into an
-editor for a file under review by clicking on the edit icon in the patch set list
-on the side-by-side diff view.
+ ** Add the names of the reviewers and anyone else you want to copy.
+ ** Describe the change.
+ ** Click Start Review:
++
+image::images/inline-edit-review-message.png[width=550]
-image::images/inline-edit-enter-edit-mode-from-diff.png[width=800, link="images/inline-edit-enter-edit-mode-from-diff.png"]
+The change is now displayed in other Gerrit dashboards and reviewers are
+notified that the change is available for code review.
-[[reviewing-changes-made-in-change-edit]]
-== Reviewing Change Edits
-Change edits are reviewed in the same way as regular patch sets, using the
-side-by-side diff screen. Change edits are shown as 'edit' in the patch list
-on the diff screen:
+[[review-edits]]
+== Reviewing Changes
-image::images/inline-edit-edit-in-diff-screen-patch-list.png[width=800, link="images/inline-edit-edit-in-diff-screen-patch-list.png"]
+Use the side-by-side diff screen.
-and on the change screen:
+image::images/inline-edit-diff-screen.png[width=800]
-image::images/inline-edit-edit-in-patch-list.png[width=800, link="images/inline-edit-edit-in-patch-list.png"]
+It's possible that subsequent patch sets may exist. For example, this sequence
+means that the change was created on top of patch set 9 while a regular
+patchset was uploaded later:
-Note that patch sets may exist that were created after the change edit was created.
+1 2 3 4 5 6 7 8 9 edit 10
-For example this sequence:
-`1 2 3 4 5 6 7 8 9 edit 10`
+[[search-for-changes]]
+== Searching for Changes with Pending Edits
-means that the change edit was created on top of patch set number 9 and a regular
-patch set was uploaded later.
+To find changes with pending edits:
-[[change-edit-actions]]
-== Change Edit Actions
+* From the Gerrit dashboard, select Your > Changes. All your changes are
+listed, according to Work in progress, Outgoing reviews, Incoming reviews,
+CCed on, and Recently closed.
-Change edits can be deleted, published and rebased, and a patch set that
-represents a change edit can be downloaded like a regular patch set.
+For more information about Search operators, see
+link:user-search.html[Searching Changes]. For example, to find only
+those changes that contain edits, see link:user-search.html#has[has:edit].
-[[delete-change-edit]]
-There is a special ref for a change edit. When the change edit is deleted, this
-ref is deleted as well. To delete a change edit click on the "Delete Edit"
-button.
+[change-edit-actions]
+== Modifying Changes
-[[publish-change-edit]]
-
-When a change edit is based on the current patch set, it can be published. By
-publishing a change edit it is promoted to a regular patch set. The special ref
-that represents the change edit is deleted on publish. To publish a change edit
-click on the "Publish Edit" button. This button is only shown when the change
-edit is based on the current patch set. Otherwise the change edit must first be
-rebased onto the current patch set.
[[rebase-change-edit]]
+=== Rebasing a Change Edit
-Only change edits that are based on the current patch set can be published. If
-in the meantime a new patch set was uploaded, the change edit must be rebased on
-top of the current patch set before it can be published. Rebasing a change
-edit is done by clicking on the "Rebase Edit" button. If the rebase results in
-conflicts, these conflicts cannot be resolved in the browser. In this case the
-change edit must be downloaded (see below) and the conflicts must be resolved in
-the local environment. The commit that contains the conflict resolution can then
-be uploaded by setting `edit` as option on the target ref:
+Only when a change is based on the current patch set can the change be
+published. In the meantime, if a new patch set has been uploaded, the change
+must be rebased on top of the current patch set before the change can be
+published.
-----
- $ git push host HEAD:refs/for/master%edit
-----
+To rebase a change:
+
+- Open the change and then click Rebase Edit.
+
+If the rebase generates conflicts, the conflicts can't be resolved in the web
+interface. Instead, the change must be downloaded (see below) and the conflicts
+resolved in the local environment.
+
+When the conflicts are resolved in the local environment, the commit that
+contains the conflict resolution can be uploaded by setting `edit` as an
+option on the target ref. For example:
+
+....
+$ git push host HEAD:refs/for/master%edit
+....
+
[[download-change-edit-patch]]
+=== Downloading a Patch
-Like regular patch sets, change edits can be downloaded by the download
-commands (e.g. provided by the `download-commands` plugin). To download a
-change edit, select the desired scheme from the "Download" dropdown and copy the
-command to your terminal. Note: only change edit owners and users that were
-granted the link:access-control.html#capability_accessDatabase[accessDatabase]
-global capability are able to access change edit refs.
+As with regular patch sets, you can download changes. For example, as provided
+by the `download-commands` plugin. Only the owners of a change and those
+users granted the
+link:access-control.html#capability_accessDatabase[accessDatabase] global
+capability can access change refs.
-[[search-for-change-edits]]
+To download a change:
-To search change edits from the UI the link:user-search.html#has[has:edit]
-predicate can be used.
+. Open the change, click the More icon, and then select Download patch.
+. Copy the desired scheme from the Download drop-down.
+. Paste the command into a terminal window.
-Alternatively change edits can be accessed through "My => Edits" dashboard.
-
-[[not-implemented-features]]
-== Not Implemented Features
-
-* Support default configuration options for inline editor that an
-administrator has set in `refs/users/default:preferences.config` file.
-
-* Allow to rename files that are already contained in the change (from the file table).
-The same rename file dialog can be used with preselected and disabled original file
-name.
-
-* Changed files in change edit should be marked as changed in file table in edit mode.
-One option is to use dirty icon or "*" char in front of changed files, another option
-is to use different hyperlink color for changed files (red?), to avoid adding yet another
-column to the file table
-
-* Add navigation icons in header area of edit screen. When dozen files need to be changed
-in context of change edit, this is not the best workflow to open one file in edit screen,
-change it, save it, close edit screen and select next file from the file table to edit.
-"<-" | "->" icons in header of edit screen could be used to navigate to the next file to
-change from the file table. This would behave like the navigation icons in side by side
-with the following logic on click:
-
-** "save-when-file-was-changed" or
-** "close-when-no-changes"
-
-* Implement conflict resolution during rebase of change edit using inline edit
-feature by creating new edit on top of current patch set with auto merge content
-
-* Similarly, reuse inline edit feature for conflict resolution during rebase of regular
-patch sets
+image::images/inline-edit-actions-download.png[width=600]
GERRIT
-------
+
Part of link:index.html[Gerrit Code Review]
-SEARCHBOX
----------
+SEARCHBOX
\ No newline at end of file
diff --git a/WORKSPACE b/WORKSPACE
index a2ee457..0a6caa2 100644
--- a/WORKSPACE
+++ b/WORKSPACE
@@ -816,13 +816,6 @@
sha1 = "42a25dc3219429f0e5d060061f71acb49bf010a0",
)
-# Only needed when jgit is built from the development tree
-maven_jar(
- name = "hamcrest-library",
- artifact = "org.hamcrest:hamcrest-library:1.3",
- sha1 = "4785a3c21320980282f9f33d0d1264a69040538f",
-)
-
TRUTH_VERS = "0.42"
maven_jar(
diff --git a/contrib/abandon_stale.py b/contrib/abandon_stale.py
index 3501b8b..2e01131 100755
--- a/contrib/abandon_stale.py
+++ b/contrib/abandon_stale.py
@@ -103,6 +103,9 @@
default=None,
action='store',
help='only abandon changes owned by the given user')
+ parser.add_option('--exclude-wip', dest='exclude_wip',
+ action='store_true',
+ help='Exclude changes that are Work-in-Progress')
parser.add_option('-v', '--verbose', dest='verbose',
action='store_true',
help='enable verbose (debug) logging')
@@ -148,7 +151,9 @@
if options.testmode:
query_terms = ["status:new", "owner:self", "topic:test-abandon"]
else:
- query_terms = ["status:new", "-is:wip", "age:%s" % options.age]
+ query_terms = ["status:new", "age:%s" % options.age]
+ if options.exclude_wip:
+ query_terms += ["-is:wip"]
if options.branches:
query_terms += ["branch:%s" % b for b in options.branches]
elif options.exclude_branches:
diff --git a/java/com/google/gerrit/acceptance/testsuite/ThrowingConsumer.java b/java/com/google/gerrit/acceptance/testsuite/ThrowingConsumer.java
new file mode 100644
index 0000000..5efdc81
--- /dev/null
+++ b/java/com/google/gerrit/acceptance/testsuite/ThrowingConsumer.java
@@ -0,0 +1,20 @@
+// 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.
+
+package com.google.gerrit.acceptance.testsuite;
+
+@FunctionalInterface
+public interface ThrowingConsumer<T> {
+ void accept(T t) throws Exception;
+}
diff --git a/java/com/google/gerrit/acceptance/testsuite/account/AccountOperationsImpl.java b/java/com/google/gerrit/acceptance/testsuite/account/AccountOperationsImpl.java
index 3d741b0..94b511b 100644
--- a/java/com/google/gerrit/acceptance/testsuite/account/AccountOperationsImpl.java
+++ b/java/com/google/gerrit/acceptance/testsuite/account/AccountOperationsImpl.java
@@ -138,13 +138,12 @@
return TestAccountUpdate.builder(this::updateAccount);
}
- private TestAccount updateAccount(TestAccountUpdate accountUpdate)
+ private void updateAccount(TestAccountUpdate accountUpdate)
throws OrmException, IOException, ConfigInvalidException {
AccountsUpdate.AccountUpdater accountUpdater =
(account, updateBuilder) -> fillBuilder(updateBuilder, accountUpdate, accountId);
Optional<AccountState> updatedAccount = updateAccount(accountUpdater);
checkState(updatedAccount.isPresent(), "Tried to update non-existing test account");
- return toTestAccount(updatedAccount.get());
}
private Optional<AccountState> updateAccount(AccountsUpdate.AccountUpdater accountUpdater)
diff --git a/java/com/google/gerrit/acceptance/testsuite/account/TestAccountUpdate.java b/java/com/google/gerrit/acceptance/testsuite/account/TestAccountUpdate.java
index 517e4b5..251f452 100644
--- a/java/com/google/gerrit/acceptance/testsuite/account/TestAccountUpdate.java
+++ b/java/com/google/gerrit/acceptance/testsuite/account/TestAccountUpdate.java
@@ -15,7 +15,7 @@
package com.google.gerrit.acceptance.testsuite.account;
import com.google.auto.value.AutoValue;
-import com.google.gerrit.acceptance.testsuite.ThrowingFunction;
+import com.google.gerrit.acceptance.testsuite.ThrowingConsumer;
import java.util.Optional;
@AutoValue
@@ -32,9 +32,9 @@
public abstract Optional<Boolean> active();
- abstract ThrowingFunction<TestAccountUpdate, TestAccount> accountUpdater();
+ abstract ThrowingConsumer<TestAccountUpdate> accountUpdater();
- public static Builder builder(ThrowingFunction<TestAccountUpdate, TestAccount> accountUpdater) {
+ public static Builder builder(ThrowingConsumer<TestAccountUpdate> accountUpdater) {
return new AutoValue_TestAccountUpdate.Builder()
.accountUpdater(accountUpdater)
.httpPassword("http-pass");
@@ -82,14 +82,13 @@
return active(false);
}
- abstract Builder accountUpdater(
- ThrowingFunction<TestAccountUpdate, TestAccount> accountUpdater);
+ abstract Builder accountUpdater(ThrowingConsumer<TestAccountUpdate> accountUpdater);
abstract TestAccountUpdate autoBuild();
- public TestAccount update() throws Exception {
+ public void update() throws Exception {
TestAccountUpdate accountUpdate = autoBuild();
- return accountUpdate.accountUpdater().apply(accountUpdate);
+ accountUpdate.accountUpdater().accept(accountUpdate);
}
}
}
diff --git a/java/com/google/gerrit/common/data/LabelType.java b/java/com/google/gerrit/common/data/LabelType.java
index 7bfd22e..29fc9c7 100644
--- a/java/com/google/gerrit/common/data/LabelType.java
+++ b/java/com/google/gerrit/common/data/LabelType.java
@@ -14,16 +14,18 @@
package com.google.gerrit.common.data;
+import static java.util.Comparator.comparing;
+import static java.util.stream.Collectors.collectingAndThen;
+import static java.util.stream.Collectors.toList;
+import static java.util.stream.Collectors.toMap;
+
import com.google.gerrit.common.Nullable;
import com.google.gerrit.reviewdb.client.LabelId;
import com.google.gerrit.reviewdb.client.PatchSetApproval;
import java.util.ArrayList;
import java.util.Collections;
-import java.util.Comparator;
-import java.util.HashMap;
import java.util.List;
import java.util.Map;
-import java.util.Optional;
public class LabelType {
public static final boolean DEF_ALLOW_POST_SUBMIT = true;
@@ -70,22 +72,13 @@
private static List<LabelValue> sortValues(List<LabelValue> values) {
values = new ArrayList<>(values);
- if (values.size() <= 1) {
- return Collections.unmodifiableList(values);
+ if (values.isEmpty()) {
+ return Collections.emptyList();
}
- Collections.sort(
- values,
- new Comparator<LabelValue>() {
- @Override
- public int compare(LabelValue o1, LabelValue o2) {
- return o1.getValue() - o2.getValue();
- }
- });
- short min = values.get(0).getValue();
- short max = values.get(values.size() - 1).getValue();
- short v = min;
+ values = values.stream().sorted(comparing(LabelValue::getValue)).collect(toList());
+ short v = values.get(0).getValue();
short i = 0;
- List<LabelValue> result = new ArrayList<>(max - min + 1);
+ ArrayList<LabelValue> result = new ArrayList<>();
// Fill in any missing values with empty text.
while (i < values.size()) {
while (v < values.get(i).getValue()) {
@@ -94,6 +87,7 @@
v++;
result.add(values.get(i++));
}
+ result.trimToSize();
return Collections.unmodifiableList(result);
}
@@ -117,7 +111,6 @@
private transient boolean canOverride;
private transient List<String> refPatterns;
- private transient List<Integer> intList;
private transient Map<Short, LabelValue> byValue;
protected LabelType() {}
@@ -148,6 +141,13 @@
setCopyMaxScore(DEF_COPY_MAX_SCORE);
setCopyMinScore(DEF_COPY_MIN_SCORE);
setAllowPostSubmit(DEF_ALLOW_POST_SUBMIT);
+
+ byValue =
+ values
+ .stream()
+ .collect(
+ collectingAndThen(
+ toMap(LabelValue::getValue, v -> v), Collections::unmodifiableMap));
}
public String getName() {
@@ -162,11 +162,8 @@
if (functionName == null) {
return null;
}
- Optional<LabelFunction> f = LabelFunction.parse(functionName);
- if (!f.isPresent()) {
- throw new IllegalStateException("Unsupported functionName: " + functionName);
- }
- return f.get();
+ return LabelFunction.parse(functionName)
+ .orElseThrow(() -> new IllegalStateException("Unsupported functionName: " + functionName));
}
public void setFunction(@Nullable LabelFunction function) {
@@ -194,7 +191,12 @@
}
public void setRefPatterns(List<String> refPatterns) {
- this.refPatterns = refPatterns;
+ if (refPatterns != null) {
+ this.refPatterns =
+ refPatterns.stream().collect(collectingAndThen(toList(), Collections::unmodifiableList));
+ } else {
+ this.refPatterns = null;
+ }
}
public List<LabelValue> getValues() {
@@ -281,36 +283,13 @@
}
public LabelValue getValue(short value) {
- initByValue();
return byValue.get(value);
}
public LabelValue getValue(PatchSetApproval ca) {
- initByValue();
return byValue.get(ca.getValue());
}
- private void initByValue() {
- if (byValue == null) {
- byValue = new HashMap<>();
- for (LabelValue v : values) {
- byValue.put(v.getValue(), v);
- }
- }
- }
-
- public List<Integer> getValuesAsList() {
- if (intList == null) {
- intList = new ArrayList<>(values.size());
- for (LabelValue v : values) {
- intList.add(Integer.valueOf(v.getValue()));
- }
- Collections.sort(intList);
- Collections.reverse(intList);
- }
- return intList;
- }
-
public LabelId getLabelId() {
return new LabelId(name);
}
diff --git a/java/com/google/gerrit/common/data/LabelValue.java b/java/com/google/gerrit/common/data/LabelValue.java
index 811e751..c0ba781 100644
--- a/java/com/google/gerrit/common/data/LabelValue.java
+++ b/java/com/google/gerrit/common/data/LabelValue.java
@@ -14,6 +14,8 @@
package com.google.gerrit.common.data;
+import java.util.Objects;
+
public class LabelValue {
public static String formatValue(short value) {
if (value < 0) {
@@ -56,6 +58,20 @@
}
@Override
+ public boolean equals(Object o) {
+ if (!(o instanceof LabelValue)) {
+ return false;
+ }
+ LabelValue v = (LabelValue) o;
+ return value == v.value && Objects.equals(text, v.text);
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(value, text);
+ }
+
+ @Override
public String toString() {
return format();
}
diff --git a/java/com/google/gerrit/server/change/ChangeJson.java b/java/com/google/gerrit/server/change/ChangeJson.java
index 04b649b..e02f666 100644
--- a/java/com/google/gerrit/server/change/ChangeJson.java
+++ b/java/com/google/gerrit/server/change/ChangeJson.java
@@ -1207,6 +1207,12 @@
Collection<LabelInfo> labels = out.labels.values();
Set<Account.Id> fixed = Sets.newHashSetWithExpectedSize(labels.size());
Set<Account.Id> removable = Sets.newHashSetWithExpectedSize(labels.size());
+
+ // Check if the user has the permission to remove a reviewer. This means we can bypass the
+ // testRemoveReviewer check for a specific reviewer in the loop saving potentially many
+ // permission checks.
+ boolean canRemoveAnyReviewer =
+ permissionBackendForChange(userProvider.get(), cd).test(ChangePermission.REMOVE_REVIEWER);
for (LabelInfo label : labels) {
if (label.all == null) {
continue;
@@ -1214,8 +1220,9 @@
for (ApprovalInfo ai : label.all) {
Account.Id id = new Account.Id(ai._accountId);
- if (removeReviewerControl.testRemoveReviewer(
- cd, userProvider.get(), id, MoreObjects.firstNonNull(ai.value, 0))) {
+ if (canRemoveAnyReviewer
+ || removeReviewerControl.testRemoveReviewer(
+ cd, userProvider.get(), id, MoreObjects.firstNonNull(ai.value, 0))) {
removable.add(id);
} else {
fixed.add(id);
@@ -1232,7 +1239,8 @@
for (AccountInfo ai : ccs) {
if (ai._accountId != null) {
Account.Id id = new Account.Id(ai._accountId);
- if (removeReviewerControl.testRemoveReviewer(cd, userProvider.get(), id, 0)) {
+ if (canRemoveAnyReviewer
+ || removeReviewerControl.testRemoveReviewer(cd, userProvider.get(), id, 0)) {
removable.add(id);
}
}
diff --git a/java/com/google/gerrit/server/extensions/events/EventUtil.java b/java/com/google/gerrit/server/extensions/events/EventUtil.java
index 74fba9a..5116708 100644
--- a/java/com/google/gerrit/server/extensions/events/EventUtil.java
+++ b/java/com/google/gerrit/server/extensions/events/EventUtil.java
@@ -121,8 +121,11 @@
public void logEventListenerError(Object event, Object listener, Exception error) {
logger.atWarning().log(
- "Error in event listener %s for event %s: %s",
- listener.getClass().getName(), event.getClass().getName(), error.getMessage());
+ "Error in event listener %s for event %s: %s - %s",
+ listener.getClass().getName(),
+ event.getClass().getName(),
+ error.getClass().getName(),
+ error.getMessage());
logger.atFine().withCause(error).log(
"Cause of error in event listener %s:", listener.getClass().getName());
}
diff --git a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
index d04ef32..4632083 100644
--- a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
+++ b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
@@ -59,6 +59,7 @@
import com.google.common.collect.Sets;
import com.google.common.collect.SortedSetMultimap;
import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.common.FooterConstants;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.TimeUtil;
import com.google.gerrit.common.data.LabelType;
@@ -653,6 +654,10 @@
if (magicBranch != null && magicBranch.cmd.getResult() == NOT_ATTEMPTED) {
newChanges = selectNewAndReplacedChangesFromMagicBranch(newProgress);
}
+
+ // Commit validation has already happened, so any changes without Change-Id are for the
+ // deprecated feature.
+ warnAboutMissingChangeId(newChanges);
preparePatchSetsForReplace(newChanges);
insertChangesAndPatchSets(newChanges, replaceProgress);
newProgress.end();
@@ -974,6 +979,7 @@
// The referenced change must exist and must still be open.
Change.Id changeId = Change.Id.parse(m.group(1));
parseReplaceCommand(cmd, changeId);
+ messages.add(new ValidationMessage("warning: pushes to refs/changes are deprecated", false));
} else {
reject(cmd, "upload to refs/changes not allowed");
}
@@ -1941,6 +1947,23 @@
return true;
}
+ private void warnAboutMissingChangeId(List<CreateRequest> newChanges) {
+ for (CreateRequest create : newChanges) {
+ try {
+ receivePack.getRevWalk().parseBody(create.commit);
+ } catch (IOException e) {
+ continue;
+ }
+ List<String> idList = create.commit.getFooterLines(FooterConstants.CHANGE_ID);
+
+ if (idList.isEmpty()) {
+ messages.add(
+ new ValidationMessage("warning: pushing without Change-Id is deprecated", false));
+ break;
+ }
+ }
+ }
+
private List<CreateRequest> selectNewAndReplacedChangesFromMagicBranch(Task newProgress) {
logger.atFine().log("Finding new and replaced changes");
List<CreateRequest> newChanges = new ArrayList<>();
diff --git a/java/com/google/gerrit/util/cli/BUILD b/java/com/google/gerrit/util/cli/BUILD
index b3ee09d..c94fc1d 100644
--- a/java/com/google/gerrit/util/cli/BUILD
+++ b/java/com/google/gerrit/util/cli/BUILD
@@ -7,8 +7,8 @@
"//java/com/google/gerrit/common:server",
"//lib:args4j",
"//lib:guava",
+ "//lib/flogger:api",
"//lib/guice",
"//lib/guice:guice-assistedinject",
- "@flogger//jar",
],
)
diff --git a/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java b/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java
index eab76c3..7ad34a6 100644
--- a/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java
+++ b/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java
@@ -103,6 +103,7 @@
import org.eclipse.jgit.api.errors.GitAPIException;
import org.eclipse.jgit.junit.TestRepository;
import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.lib.RefUpdate;
import org.eclipse.jgit.lib.RefUpdate.Result;
import org.eclipse.jgit.lib.Repository;
@@ -340,6 +341,20 @@
}
@Test
+ public void pushWithoutChangeIdDeprecated() throws Exception {
+ setRequireChangeId(InheritableBoolean.FALSE);
+ testRepo
+ .branch("HEAD")
+ .commit()
+ .message("A change")
+ .author(admin.getIdent())
+ .committer(new PersonIdent(admin.getIdent(), testRepo.getDate()))
+ .create();
+ PushResult result = pushHead(testRepo, "refs/for/master");
+ assertThat(result.getMessages()).contains("warning: pushing without Change-Id is deprecated");
+ }
+
+ @Test
public void autocloseByChangeId() throws Exception {
// Create a change
PushOneCommit.Result r = pushTo("refs/for/master");
diff --git a/javatests/com/google/gerrit/common/data/LabelTypeTest.java b/javatests/com/google/gerrit/common/data/LabelTypeTest.java
new file mode 100644
index 0000000..6c3befb
--- /dev/null
+++ b/javatests/com/google/gerrit/common/data/LabelTypeTest.java
@@ -0,0 +1,48 @@
+// 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.
+
+package com.google.gerrit.common.data;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.common.collect.ImmutableList;
+import org.junit.Test;
+
+public class LabelTypeTest {
+ @Test
+ public void sortLabelValues() {
+ LabelValue v0 = new LabelValue((short) 0, "Zero");
+ LabelValue v1 = new LabelValue((short) 1, "One");
+ LabelValue v2 = new LabelValue((short) 2, "Two");
+ LabelType types = new LabelType("Label", ImmutableList.of(v2, v0, v1));
+ assertThat(types.getValues()).containsExactly(v0, v1, v2).inOrder();
+ }
+
+ @Test
+ public void insertMissingLabelValues() {
+ LabelValue v0 = new LabelValue((short) 0, "Zero");
+ LabelValue v2 = new LabelValue((short) 2, "Two");
+ LabelValue v5 = new LabelValue((short) 5, "Five");
+ LabelType types = new LabelType("Label", ImmutableList.of(v2, v5, v0));
+ assertThat(types.getValues())
+ .containsExactly(
+ v0,
+ new LabelValue((short) 1, ""),
+ v2,
+ new LabelValue((short) 3, ""),
+ new LabelValue((short) 4, ""),
+ v5)
+ .inOrder();
+ }
+}
diff --git a/javatests/com/google/gerrit/server/schema/SchemaCreatorTest.java b/javatests/com/google/gerrit/server/schema/SchemaCreatorTest.java
index 7f8b6f3..d3f69982 100644
--- a/javatests/com/google/gerrit/server/schema/SchemaCreatorTest.java
+++ b/javatests/com/google/gerrit/server/schema/SchemaCreatorTest.java
@@ -15,8 +15,8 @@
package com.google.gerrit.server.schema;
import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.Truth8.assertThat;
-import com.google.common.base.Strings;
import com.google.common.collect.ImmutableList;
import com.google.gerrit.common.data.LabelFunction;
import com.google.gerrit.common.data.LabelType;
@@ -36,6 +36,7 @@
import java.sql.SQLException;
import java.util.ArrayList;
import java.util.Arrays;
+import java.util.Collections;
import java.util.List;
import org.eclipse.jgit.lib.Repository;
import org.junit.After;
@@ -108,15 +109,22 @@
assertThat(codeReview.getDefaultValue()).isEqualTo(0);
assertThat(codeReview.getFunction()).isEqualTo(LabelFunction.MAX_WITH_BLOCK);
assertThat(codeReview.isCopyMinScore()).isTrue();
- assertValueRange(codeReview, 2, 1, 0, -1, -2);
+ assertValueRange(codeReview, -2, -1, 0, 1, 2);
}
private void assertValueRange(LabelType label, Integer... range) {
- assertThat(label.getValuesAsList()).containsExactlyElementsIn(Arrays.asList(range)).inOrder();
- assertThat(label.getMax().getValue()).isEqualTo(range[0]);
- assertThat(label.getMin().getValue()).isEqualTo(range[range.length - 1]);
+ List<Integer> rangeList = Arrays.asList(range);
+ assertThat(rangeList).isNotEmpty();
+ assertThat(rangeList).isStrictlyOrdered();
+
+ assertThat(label.getValues().stream().map(v -> (int) v.getValue()))
+ .containsExactlyElementsIn(rangeList)
+ .inOrder();
+ assertThat(label.getMax().getValue()).isEqualTo(Collections.max(rangeList));
+ assertThat(label.getMin().getValue()).isEqualTo(Collections.min(rangeList));
for (LabelValue v : label.getValues()) {
- assertThat(Strings.isNullOrEmpty(v.getText())).isFalse();
+ assertThat(v.getText()).isNotNull();
+ assertThat(v.getText()).isNotEmpty();
}
}
}
diff --git a/lib/jgit/jgit.bzl b/lib/jgit/jgit.bzl
index ba34750..6ada5bd 100644
--- a/lib/jgit/jgit.bzl
+++ b/lib/jgit/jgit.bzl
@@ -18,9 +18,23 @@
name = "jgit",
path = LOCAL_JGIT_REPO,
)
+ jgit_maven_repos_dev()
else:
jgit_maven_repos()
+def jgit_maven_repos_dev():
+ # Transitive dependencies from JGit's WORKSPACE.
+ maven_jar(
+ name = "hamcrest-library",
+ artifact = "org.hamcrest:hamcrest-library:1.3",
+ sha1 = "4785a3c21320980282f9f33d0d1264a69040538f",
+ )
+ maven_jar(
+ name = "jzlib",
+ artifact = "com.jcraft:jzlib:1.1.1",
+ sha1 = "a1551373315ffc2f96130a0e5704f74e151777ba",
+ )
+
def jgit_maven_repos():
maven_jar(
name = "jgit-lib",
diff --git a/polygerrit-ui/app/elements/admin/gr-group/gr-group.html b/polygerrit-ui/app/elements/admin/gr-group/gr-group.html
index 1e19107..349cadc 100644
--- a/polygerrit-ui/app/elements/admin/gr-group/gr-group.html
+++ b/polygerrit-ui/app/elements/admin/gr-group/gr-group.html
@@ -23,7 +23,6 @@
<link rel="import" href="../../../styles/shared-styles.html">
<link rel="import" href="../../shared/gr-autocomplete/gr-autocomplete.html">
<link rel="import" href="../../shared/gr-copy-clipboard/gr-copy-clipboard.html">
-<link rel="import" href="../../shared/gr-download-commands/gr-download-commands.html">
<link rel="import" href="../../shared/gr-rest-api-interface/gr-rest-api-interface.html">
<link rel="import" href="../../shared/gr-select/gr-select.html">
diff --git a/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access.js b/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access.js
index ce0392d..799b831 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access.js
+++ b/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access.js
@@ -213,7 +213,7 @@
for (const key in response) {
if (!response.hasOwnProperty(key)) { continue; }
projects.push({
- name: key,
+ name: response[key].name,
value: response[key].id,
});
}
diff --git a/polygerrit-ui/app/elements/admin/gr-repo-command/gr-repo-command.html b/polygerrit-ui/app/elements/admin/gr-repo-command/gr-repo-command.html
index f42652d..7db4e4c 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo-command/gr-repo-command.html
+++ b/polygerrit-ui/app/elements/admin/gr-repo-command/gr-repo-command.html
@@ -27,7 +27,12 @@
}
</style>
<h3>[[title]]</h3>
- <gr-button on-tap="_onCommandTap">[[title]]</gr-button>
+ <gr-button
+ title$="[[tooltip]]"
+ disabled$="[[disabled]]"
+ on-tap="_onCommandTap">
+ [[title]]
+ </gr-button>
</template>
<script src="gr-repo-command.js"></script>
</dom-module>
diff --git a/polygerrit-ui/app/elements/admin/gr-repo-command/gr-repo-command.js b/polygerrit-ui/app/elements/admin/gr-repo-command/gr-repo-command.js
index e49c4de..bcdb7f6 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo-command/gr-repo-command.js
+++ b/polygerrit-ui/app/elements/admin/gr-repo-command/gr-repo-command.js
@@ -22,6 +22,8 @@
properties: {
title: String,
+ disabled: Boolean,
+ tooltip: String,
},
/**
diff --git a/polygerrit-ui/app/elements/admin/gr-repo-commands/gr-repo-commands.html b/polygerrit-ui/app/elements/admin/gr-repo-commands/gr-repo-commands.html
index 935c967..dba01aa 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo-commands/gr-repo-commands.html
+++ b/polygerrit-ui/app/elements/admin/gr-repo-commands/gr-repo-commands.html
@@ -50,7 +50,8 @@
on-command-tap="_handleEditRepoConfig">
</gr-repo-command>
<gr-repo-command
- title="Run GC"
+ title="[[_repoConfig.actions.gc.label]]"
+ tooltip="[[_repoConfig.actions.gc.title]]"
hidden$="[[!_repoConfig.actions.gc.enabled]]"
on-command-tap="_handleRunningGC">
</gr-repo-command>
diff --git a/polygerrit-ui/app/elements/admin/gr-repo-list/gr-repo-list.js b/polygerrit-ui/app/elements/admin/gr-repo-list/gr-repo-list.js
index 5560972..5a8b5b1 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo-list/gr-repo-list.js
+++ b/polygerrit-ui/app/elements/admin/gr-repo-list/gr-repo-list.js
@@ -120,12 +120,7 @@
.then(repos => {
// Late response.
if (filter !== this._filter || !repos) { return; }
- this._repos = Object.keys(repos)
- .map(key => {
- const repo = repos[key];
- repo.name = key;
- return repo;
- });
+ this._repos = repos;
this._loading = false;
});
},
diff --git a/polygerrit-ui/app/elements/change/gr-commit-info/gr-commit-info.html b/polygerrit-ui/app/elements/change/gr-commit-info/gr-commit-info.html
index 96a5454..b6c8fcc 100644
--- a/polygerrit-ui/app/elements/change/gr-commit-info/gr-commit-info.html
+++ b/polygerrit-ui/app/elements/change/gr-commit-info/gr-commit-info.html
@@ -39,7 +39,6 @@
has-tooltip
button-title="Copy full SHA to clipboard"
hide-input
- hide-label
text="[[commitInfo.commit]]">
</gr-copy-clipboard>
</div>
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
index b2857a5..8e32ea3 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
@@ -1351,7 +1351,6 @@
const setupDiff = function(diff) {
const mock = document.createElement('mock-diff-response');
- diff.$.diff._diff = mock.diffResponse;
diff.comments = {
left: diff.path === '/COMMIT_MSG' ? commitMsgComments : [],
right: [],
@@ -1379,7 +1378,7 @@
theme: 'DEFAULT',
ignore_whitespace: 'IGNORE_NONE',
};
- diff.$.diff._renderDiffTable();
+ diff._diff = mock.diffResponse;
};
const renderAndGetNewDiffs = function(index) {
diff --git a/polygerrit-ui/app/elements/change/gr-upload-help-dialog/gr-upload-help-dialog.html b/polygerrit-ui/app/elements/change/gr-upload-help-dialog/gr-upload-help-dialog.html
index 1e77d5d..a9843a3 100644
--- a/polygerrit-ui/app/elements/change/gr-upload-help-dialog/gr-upload-help-dialog.html
+++ b/polygerrit-ui/app/elements/change/gr-upload-help-dialog/gr-upload-help-dialog.html
@@ -17,6 +17,7 @@
<link rel="import" href="../../../bower_components/polymer/polymer.html">
<link rel="import" href="../../shared/gr-dialog/gr-dialog.html">
+<link rel="import" href="../../shared/gr-shell-command/gr-shell-command.html">
<link rel="import" href="../../../styles/shared-styles.html">
<dom-module id="gr-upload-help-dialog">
@@ -33,33 +34,9 @@
margin-left: 1em;
list-style: decimal;
}
- p,
- .commandContainer {
+ p {
margin-bottom: .75em;
}
- .commandContainer {
- background: #f5f5f5;
- padding: .5em .5em .5em 2.5em;
- position: relative;
- width: 100%;
- }
- .commandContainer:before {
- background: #ebebeb;
- bottom: 0;
- box-sizing: border-box;
- content: '$';
- display: block;
- left: 0;
- padding: .8em;
- position: absolute;
- top: 0;
- width: 2em;
- }
- .commandContainer gr-copy-clipboard {
- --text-container-style: {
- border: none;
- }
- }
</style>
<gr-dialog
confirm-label="Done"
@@ -79,18 +56,14 @@
Update the local commit with your modifications using the following
command.
</p>
- <div class="commandContainer">
- <gr-copy-clipboard text="[[_commitCommand]]"></gr-copy-clipboard>
- </div>
+ <gr-shell-command command="[[_commitCommand]]"></gr-shell-command>
<p>
Leave the "Change-Id:" line of the commit message as is.
</p>
</li>
<li>
<p>Push the updated commit to Gerrit.</p>
- <div class="commandContainer">
- <gr-copy-clipboard text="[[_pushCommand]]"></gr-copy-clipboard>
- </div>
+ <gr-shell-command command="[[_pushCommand]]"></gr-shell-command>
</li>
<li>
<p>Refresh this page to view the the update.</p>
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-image.js b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-image.js
index cb768ef..88ff79b 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-image.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-image.js
@@ -35,7 +35,6 @@
GrDiffBuilderImage.prototype.constructor = GrDiffBuilderImage;
GrDiffBuilderImage.prototype.renderDiff = function() {
- this._outputEl.classList.add('image-diff');
const section = this._createElement('tbody', 'image-diff');
this._emitImagePair(section);
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor_test.html b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor_test.html
index 2a00425..9668a54 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor_test.html
@@ -35,6 +35,7 @@
<mock-diff-response></mock-diff-response>
<gr-diff></gr-diff>
<gr-diff-cursor></gr-diff-cursor>
+ <gr-rest-api-interface></gr-rest-api-interface>
</template>
</test-fixture>
@@ -48,27 +49,18 @@
setup(done => {
sandbox = sinon.sandbox.create();
- stub('gr-rest-api-interface', {
- getLoggedIn() { return Promise.resolve(false); },
- getDiff() {
- return Promise.resolve(mockDiffResponse.diffResponse);
- },
- });
-
const fixtureElems = fixture('basic');
mockDiffResponse = fixtureElems[0];
diffElement = fixtureElems[1];
cursorElement = fixtureElems[2];
+ const restAPI = fixtureElems[3];
// Register the diff with the cursor.
cursorElement.push('diffs', diffElement);
+ diffElement.loggedIn = false;
diffElement.patchRange = {basePatchNum: 1, patchNum: 2};
diffElement.comments = {left: [], right: []};
- diffElement.$.restAPI.getDiffPreferences().then(prefs => {
- diffElement.prefs = prefs;
- });
-
const setupDone = () => {
cursorElement._updateStops();
cursorElement.moveToFirstChunk();
@@ -77,7 +69,10 @@
};
diffElement.addEventListener('render', setupDone);
- diffElement.reload();
+ restAPI.getDiffPreferences().then(prefs => {
+ diffElement.prefs = prefs;
+ diffElement.diff = mockDiffResponse.diffResponse;
+ });
});
teardown(() => sandbox.restore());
@@ -219,7 +214,7 @@
done();
}
diffElement.addEventListener('render', renderHandler);
- diffElement.reload();
+ diffElement._diffChanged(mockDiffResponse.diffResponse);
});
test('initialLineNumber enabled', done => {
@@ -239,7 +234,7 @@
cursorElement.initialLineNumber = 10;
cursorElement.side = 'right';
- diffElement.reload();
+ diffElement._diffChanged(mockDiffResponse.diffResponse);
});
test('getAddress', () => {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.html b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.html
index ff74fba..e3bf866 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.html
@@ -16,6 +16,9 @@
-->
<link rel="import" href="../../../bower_components/polymer/polymer.html">
+<link rel="import" href="../../core/gr-reporting/gr-reporting.html">
+<link rel="import" href="../../shared/gr-rest-api-interface/gr-rest-api-interface.html">
+
<link rel="import" href="../gr-diff/gr-diff.html">
<dom-module id="gr-diff-host">
@@ -30,17 +33,23 @@
project-config="[[projectConfig]]"
project-name="[[projectName]]"
display-line="[[displayLine]]"
- is-image-diff="{{isImageDiff}}"
+ is-image-diff="[[isImageDiff]]"
commit-range="[[commitRange]]"
- files-weblinks="{{filesWeblinks}}"
hidden$="[[hidden]]"
no-render-on-prefs-change="[[noRenderOnPrefsChange]]"
comments="[[comments]]"
line-wrapping="[[lineWrapping]]"
view-mode="[[viewMode]]"
line-of-interest="[[lineOfInterest]]"
- show-load-failure="[[showLoadFailure]]"
- is-blame-loaded="{{isBlameLoaded}}"></gr-diff>
+ logged-in="[[_loggedIn]]"
+ loading="[[_loading]]"
+ error-message="[[_errorMessage]]"
+ base-image="[[_baseImage]]"
+ revision-image=[[_revisionImage]]
+ blame="[[_blame]]"
+ diff="[[_diff]]"></gr-diff>
+ <gr-rest-api-interface id="restAPI"></gr-rest-api-interface>
+ <gr-reporting id="reporting" category="diff"></gr-reporting>
</template>
<script src="gr-diff-host.js"></script>
</dom-module>
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.js b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.js
index 84b920a..3e9e796 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.js
@@ -17,12 +17,33 @@
(function() {
'use strict';
+ const MSG_EMPTY_BLAME = 'No blame information for this diff.';
+
+ const EVENT_AGAINST_PARENT = 'diff-against-parent';
+ const EVENT_ZERO_REBASE = 'rebase-percent-zero';
+ const EVENT_NONZERO_REBASE = 'rebase-percent-nonzero';
+
const DiffViewMode = {
SIDE_BY_SIDE: 'SIDE_BY_SIDE',
UNIFIED: 'UNIFIED_DIFF',
};
/**
+ * @param {Object} diff
+ * @return {boolean}
+ */
+ function isImageDiff(diff) {
+ if (!diff) { return false; }
+
+ const isA = diff.meta_a &&
+ diff.meta_a.content_type.startsWith('image/');
+ const isB = diff.meta_b &&
+ diff.meta_b.content_type.startsWith('image/');
+
+ return !!(diff.binary && (isA || isB));
+ }
+
+ /**
* Wrapper around gr-diff.
*
* Webcomponent fetching diffs and related data from restAPI and passing them
@@ -71,11 +92,13 @@
},
isImageDiff: {
type: Boolean,
+ computed: '_computeIsImageDiff(_diff)',
notify: true,
},
commitRange: Object,
filesWeblinks: {
type: Object,
+ value() { return {}; },
notify: true,
},
hidden: {
@@ -113,12 +136,109 @@
isBlameLoaded: {
type: Boolean,
notify: true,
+ computed: '_computeIsBlameLoaded(_blame)',
},
+
+ _loggedIn: {
+ type: Boolean,
+ value: false,
+ },
+
+ _loading: {
+ type: Boolean,
+ value: false,
+ },
+
+ /** @type {?string} */
+ _errorMessage: {
+ type: String,
+ value: null,
+ },
+
+ /** @type {?Object} */
+ _baseImage: Object,
+ /** @type {?Object} */
+ _revisionImage: Object,
+
+ _diff: Object,
+
+ /** @type {?Object} */
+ _blame: {
+ type: Object,
+ value: null,
+ },
+ },
+
+ listeners: {
+ 'draft-interaction': '_handleDraftInteraction',
+ },
+
+ ready() {
+ if (this._canReload()) {
+ this.reload();
+ }
+ },
+
+ attached() {
+ this._getLoggedIn().then(loggedIn => {
+ this._loggedIn = loggedIn;
+ });
},
/** @return {!Promise} */
reload() {
- return this.$.diff.reload();
+ this._loading = true;
+ this._errorMessage = null;
+
+ const diffRequest = this._getDiff()
+ .then(diff => {
+ this._reportDiff(diff);
+ return diff;
+ })
+ .catch(e => {
+ this._handleGetDiffError(e);
+ return null;
+ });
+
+ const assetRequest = diffRequest.then(diff => {
+ // If the diff is null, then it's failed to load.
+ if (!diff) { return null; }
+
+ return this._loadDiffAssets(diff);
+ });
+
+ return Promise.all([diffRequest, assetRequest])
+ .then(results => {
+ const diff = results[0];
+ if (!diff) {
+ return Promise.resolve();
+ }
+ this.filesWeblinks = this._getFilesWeblinks(diff);
+ return new Promise(resolve => {
+ const callback = () => {
+ resolve();
+ this.removeEventListener('render', callback);
+ };
+ this.addEventListener('render', callback);
+ this._diff = diff;
+ });
+ })
+ .catch(err => {
+ console.warn('Error encountered loading diff:', err);
+ })
+ .then(() => { this._loading = false; });
+ },
+
+ _getFilesWeblinks(diff) {
+ if (!this.commitRange) { return {}; }
+ return {
+ meta_a: Gerrit.Nav.getFileWebLinks(
+ this.projectName, this.commitRange.baseCommit, this.path,
+ {weblinks: diff && diff.meta_a && diff.meta_a.web_links}),
+ meta_b: Gerrit.Nav.getFileWebLinks(
+ this.projectName, this.commitRange.commit, this.path,
+ {weblinks: diff && diff.meta_b && diff.meta_b.web_links}),
+ };
},
/** Cancel any remaining diff builder rendering work. */
@@ -145,12 +265,21 @@
* @return {Promise} A promise that resolves when blame finishes rendering.
*/
loadBlame() {
- return this.$.diff.loadBlame();
+ return this.$.restAPI.getBlame(this.changeNum, this.patchRange.patchNum,
+ this.path, true)
+ .then(blame => {
+ if (!blame.length) {
+ this.fire('show-alert', {message: MSG_EMPTY_BLAME});
+ return Promise.reject(MSG_EMPTY_BLAME);
+ }
+
+ this._blame = blame;
+ });
},
/** Unload blame information for the diff. */
clearBlame() {
- this.$.diff.clearBlame();
+ this._blame = null;
},
/** @return {!Array<!HTMLElement>} */
@@ -170,5 +299,136 @@
expandAllContext() {
this.$.diff.expandAllContext();
},
+
+ /** @return {!Promise} */
+ _getLoggedIn() {
+ return this.$.restAPI.getLoggedIn();
+ },
+
+ /** @return {boolean}} */
+ _canReload() {
+ return !!this.changeNum && !!this.patchRange && !!this.path &&
+ !this.noAutoRender;
+ },
+
+ /** @return {!Promise<!Object>} */
+ _getDiff() {
+ // Wrap the diff request in a new promise so that the error handler
+ // rejects the promise, allowing the error to be handled in the .catch.
+ return new Promise((resolve, reject) => {
+ this.$.restAPI.getDiff(
+ this.changeNum,
+ this.patchRange.basePatchNum,
+ this.patchRange.patchNum,
+ this.path,
+ reject)
+ .then(resolve);
+ });
+ },
+
+ _handleGetDiffError(response) {
+ // Loading the diff may respond with 409 if the file is too large. In this
+ // case, use a toast error..
+ if (response.status === 409) {
+ this.fire('server-error', {response});
+ return;
+ }
+
+ if (this.showLoadFailure) {
+ this._errorMessage = [
+ 'Encountered error when loading the diff:',
+ response.status,
+ response.statusText,
+ ].join(' ');
+ return;
+ }
+
+ this.fire('page-error', {response});
+ },
+
+ /**
+ * Report info about the diff response.
+ */
+ _reportDiff(diff) {
+ if (!diff || !diff.content) { return; }
+
+ // Count the delta lines stemming from normal deltas, and from
+ // due_to_rebase deltas.
+ let nonRebaseDelta = 0;
+ let rebaseDelta = 0;
+ diff.content.forEach(chunk => {
+ if (chunk.ab) { return; }
+ const deltaSize = Math.max(
+ chunk.a ? chunk.a.length : 0, chunk.b ? chunk.b.length : 0);
+ if (chunk.due_to_rebase) {
+ rebaseDelta += deltaSize;
+ } else {
+ nonRebaseDelta += deltaSize;
+ }
+ });
+
+ // Find the percent of the delta from due_to_rebase chunks rounded to two
+ // digits. Diffs with no delta are considered 0%.
+ const totalDelta = rebaseDelta + nonRebaseDelta;
+ const percentRebaseDelta = !totalDelta ? 0 :
+ Math.round(100 * rebaseDelta / totalDelta);
+
+ // Report the due_to_rebase percentage in the "diff" category when
+ // applicable.
+ if (this.patchRange.basePatchNum === 'PARENT') {
+ this.$.reporting.reportInteraction(EVENT_AGAINST_PARENT);
+ } else if (percentRebaseDelta === 0) {
+ this.$.reporting.reportInteraction(EVENT_ZERO_REBASE);
+ } else {
+ this.$.reporting.reportInteraction(EVENT_NONZERO_REBASE,
+ percentRebaseDelta);
+ }
+ },
+
+ /**
+ * @param {Object} diff
+ * @return {!Promise}
+ */
+ _loadDiffAssets(diff) {
+ if (isImageDiff(diff)) {
+ return this._getImages(diff).then(images => {
+ this._baseImage = images.baseImage;
+ this._revisionImage = images.revisionImage;
+ });
+ } else {
+ this._baseImage = null;
+ this._revisionImage = null;
+ return Promise.resolve();
+ }
+ },
+
+ /**
+ * @param {Object} diff
+ * @return {boolean}
+ */
+ _computeIsImageDiff(diff) {
+ return isImageDiff(diff);
+ },
+
+ /**
+ * @param {Object} blame
+ * @return {boolean}
+ */
+ _computeIsBlameLoaded(blame) {
+ return !!blame;
+ },
+
+ /**
+ * @param {Object} diff
+ * @return {!Promise}
+ */
+ _getImages(diff) {
+ return this.$.restAPI.getImagesForDiff(this.changeNum, diff,
+ this.patchRange);
+ },
+
+ _handleDraftInteraction() {
+ this.$.reporting.recordDraftInteraction();
+ },
});
})();
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.html b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.html
index 7c2dd7b..a05d44f 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.html
@@ -40,23 +40,437 @@
setup(() => {
sandbox = sinon.sandbox.create();
+ element = fixture('basic');
});
teardown(() => {
sandbox.restore();
});
- test('delegates reload()', () => {
- element = fixture('basic');
- const returnValue = Promise.resolve();
- const stub = sandbox.stub(element.$.diff, 'reload').returns(returnValue);
- assert.equal(element.reload(), returnValue);
- assert.isTrue(stub.calledOnce);
- assert.equal(stub.lastCall.args.length, 0);
+ test('reload() cancels before network resolves', () => {
+ const cancelStub = sandbox.stub(element.$.diff, 'cancel');
+
+ // Stub the network calls into requests that never resolve.
+ sandbox.stub(element, '_getDiff', () => new Promise(() => {}));
+
+ element.reload();
+ assert.isTrue(cancelStub.called);
+ });
+
+ suite('not logged in', () => {
+ setup(() => {
+ const getLoggedInPromise = Promise.resolve(false);
+ stub('gr-rest-api-interface', {
+ getLoggedIn() { return getLoggedInPromise; },
+ });
+ element = fixture('basic');
+ return getLoggedInPromise;
+ });
+
+ test('reload() loads files weblinks', () => {
+ const weblinksStub = sandbox.stub(Gerrit.Nav, '_generateWeblinks')
+ .returns({name: 'stubb', url: '#s'});
+ sandbox.stub(element.$.restAPI, 'getDiff').returns(Promise.resolve({
+ content: [],
+ }));
+ element.projectName = 'test-project';
+ element.path = 'test-path';
+ element.commitRange = {baseCommit: 'test-base', commit: 'test-commit'};
+ element.patchRange = {};
+ return element.reload().then(() => {
+ assert.isTrue(weblinksStub.calledTwice);
+ assert.isTrue(weblinksStub.firstCall.calledWith({
+ commit: 'test-base',
+ file: 'test-path',
+ options: {
+ weblinks: undefined,
+ },
+ repo: 'test-project',
+ type: Gerrit.Nav.WeblinkType.FILE}));
+ assert.isTrue(weblinksStub.secondCall.calledWith({
+ commit: 'test-commit',
+ file: 'test-path',
+ options: {
+ weblinks: undefined,
+ },
+ repo: 'test-project',
+ type: Gerrit.Nav.WeblinkType.FILE}));
+ assert.deepEqual(element.filesWeblinks, {
+ meta_a: [{name: 'stubb', url: '#s'}],
+ meta_b: [{name: 'stubb', url: '#s'}],
+ });
+ });
+ });
+
+ test('_getDiff handles null diff responses', done => {
+ stub('gr-rest-api-interface', {
+ getDiff() { return Promise.resolve(null); },
+ });
+ element.changeNum = 123;
+ element.patchRange = {basePatchNum: 1, patchNum: 2};
+ element.path = 'file.txt';
+ element._getDiff().then(done);
+ });
+
+ test('reload resolves on error', () => {
+ const onErrStub = sandbox.stub(element, '_handleGetDiffError');
+ const error = {ok: false, status: 500};
+ sandbox.stub(element.$.restAPI, 'getDiff',
+ (changeNum, basePatchNum, patchNum, path, onErr) => {
+ onErr(error);
+ });
+ return element.reload().then(() => {
+ assert.isTrue(onErrStub.calledOnce);
+ });
+ });
+
+ suite('_handleGetDiffError', () => {
+ let serverErrorStub;
+ let pageErrorStub;
+
+ setup(() => {
+ serverErrorStub = sinon.stub();
+ element.addEventListener('server-error', serverErrorStub);
+ pageErrorStub = sinon.stub();
+ element.addEventListener('page-error', pageErrorStub);
+ });
+
+ test('page error on HTTP-409', () => {
+ element._handleGetDiffError({status: 409});
+ assert.isTrue(serverErrorStub.calledOnce);
+ assert.isFalse(pageErrorStub.called);
+ assert.isNotOk(element._errorMessage);
+ });
+
+ test('server error on non-HTTP-409', () => {
+ element._handleGetDiffError({status: 500});
+ assert.isFalse(serverErrorStub.called);
+ assert.isTrue(pageErrorStub.calledOnce);
+ assert.isNotOk(element._errorMessage);
+ });
+
+ test('error message if showLoadFailure', () => {
+ element.showLoadFailure = true;
+ element._handleGetDiffError({status: 500, statusText: 'Failure!'});
+ assert.isFalse(serverErrorStub.called);
+ assert.isFalse(pageErrorStub.called);
+ assert.equal(element._errorMessage,
+ 'Encountered error when loading the diff: 500 Failure!');
+ });
+ });
+
+ suite('image diffs', () => {
+ let mockFile1;
+ let mockFile2;
+ setup(() => {
+ mockFile1 = {
+ body: 'Qk06AAAAAAAAADYAAAAoAAAAAQAAAP////8BACAAAAAAAAAAAAATCwAAE' +
+ 'wsAAAAAAAAAAAAAAAAA/w==',
+ type: 'image/bmp',
+ };
+ mockFile2 = {
+ body: 'Qk06AAAAAAAAADYAAAAoAAAAAQAAAP////8BACAAAAAAAAAAAAATCwAAE' +
+ 'wsAAAAAAAAAAAAA/////w==',
+ type: 'image/bmp',
+ };
+ sandbox.stub(element.$.restAPI,
+ 'getB64FileContents',
+ (changeId, patchNum, path, opt_parentIndex) => {
+ return Promise.resolve(opt_parentIndex === 1 ? mockFile1 :
+ mockFile2);
+ });
+
+ element.patchRange = {basePatchNum: 'PARENT', patchNum: 1};
+ element.comments = {left: [], right: []};
+ });
+
+ test('renders image diffs with same file name', done => {
+ const mockDiff = {
+ meta_a: {name: 'carrot.jpg', content_type: 'image/jpeg', lines: 66},
+ meta_b: {name: 'carrot.jpg', content_type: 'image/jpeg',
+ lines: 560},
+ intraline_status: 'OK',
+ change_type: 'MODIFIED',
+ diff_header: [
+ 'diff --git a/carrot.jpg b/carrot.jpg',
+ 'index 2adc47d..f9c2f2c 100644',
+ '--- a/carrot.jpg',
+ '+++ b/carrot.jpg',
+ 'Binary files differ',
+ ],
+ content: [{skip: 66}],
+ binary: true,
+ };
+ sandbox.stub(element.$.restAPI, 'getDiff')
+ .returns(Promise.resolve(mockDiff));
+
+ const rendered = () => {
+ // Recognizes that it should be an image diff.
+ assert.isTrue(element.isImageDiff);
+ assert.instanceOf(
+ element.$.diff.$.diffBuilder._builder, GrDiffBuilderImage);
+
+ // Left image rendered with the parent commit's version of the file.
+ const leftImage =
+ element.$.diff.$.diffTable.querySelector('td.left img');
+ const leftLabel =
+ element.$.diff.$.diffTable.querySelector('td.left label');
+ const leftLabelContent = leftLabel.querySelector('.label');
+ const leftLabelName = leftLabel.querySelector('.name');
+
+ const rightImage =
+ element.$.diff.$.diffTable.querySelector('td.right img');
+ const rightLabel = element.$.diff.$.diffTable.querySelector(
+ 'td.right label');
+ const rightLabelContent = rightLabel.querySelector('.label');
+ const rightLabelName = rightLabel.querySelector('.name');
+
+ assert.isNotOk(rightLabelName);
+ assert.isNotOk(leftLabelName);
+
+ let leftLoaded = false;
+ let rightLoaded = false;
+
+ leftImage.addEventListener('load', () => {
+ assert.isOk(leftImage);
+ assert.equal(leftImage.getAttribute('src'),
+ 'data:image/bmp;base64, ' + mockFile1.body);
+ assert.equal(leftLabelContent.textContent, '1×1 image/bmp');
+ leftLoaded = true;
+ if (rightLoaded) {
+ element.removeEventListener('render', rendered);
+ done();
+ }
+ });
+
+ rightImage.addEventListener('load', () => {
+ assert.isOk(rightImage);
+ assert.equal(rightImage.getAttribute('src'),
+ 'data:image/bmp;base64, ' + mockFile2.body);
+ assert.equal(rightLabelContent.textContent, '1×1 image/bmp');
+
+ rightLoaded = true;
+ if (leftLoaded) {
+ element.removeEventListener('render', rendered);
+ done();
+ }
+ });
+ };
+
+ element.addEventListener('render', rendered);
+
+ element.$.restAPI.getDiffPreferences().then(prefs => {
+ element.prefs = prefs;
+ element.reload();
+ });
+ });
+
+ test('renders image diffs with a different file name', done => {
+ const mockDiff = {
+ meta_a: {name: 'carrot.jpg', content_type: 'image/jpeg', lines: 66},
+ meta_b: {name: 'carrot2.jpg', content_type: 'image/jpeg',
+ lines: 560},
+ intraline_status: 'OK',
+ change_type: 'MODIFIED',
+ diff_header: [
+ 'diff --git a/carrot.jpg b/carrot2.jpg',
+ 'index 2adc47d..f9c2f2c 100644',
+ '--- a/carrot.jpg',
+ '+++ b/carrot2.jpg',
+ 'Binary files differ',
+ ],
+ content: [{skip: 66}],
+ binary: true,
+ };
+ sandbox.stub(element.$.restAPI, 'getDiff')
+ .returns(Promise.resolve(mockDiff));
+
+ const rendered = () => {
+ // Recognizes that it should be an image diff.
+ assert.isTrue(element.isImageDiff);
+ assert.instanceOf(
+ element.$.diff.$.diffBuilder._builder, GrDiffBuilderImage);
+
+ // Left image rendered with the parent commit's version of the file.
+ const leftImage =
+ element.$.diff.$.diffTable.querySelector('td.left img');
+ const leftLabel =
+ element.$.diff.$.diffTable.querySelector('td.left label');
+ const leftLabelContent = leftLabel.querySelector('.label');
+ const leftLabelName = leftLabel.querySelector('.name');
+
+ const rightImage =
+ element.$.diff.$.diffTable.querySelector('td.right img');
+ const rightLabel = element.$.diff.$.diffTable.querySelector(
+ 'td.right label');
+ const rightLabelContent = rightLabel.querySelector('.label');
+ const rightLabelName = rightLabel.querySelector('.name');
+
+ assert.isOk(rightLabelName);
+ assert.isOk(leftLabelName);
+ assert.equal(leftLabelName.textContent, mockDiff.meta_a.name);
+ assert.equal(rightLabelName.textContent, mockDiff.meta_b.name);
+
+ let leftLoaded = false;
+ let rightLoaded = false;
+
+ leftImage.addEventListener('load', () => {
+ assert.isOk(leftImage);
+ assert.equal(leftImage.getAttribute('src'),
+ 'data:image/bmp;base64, ' + mockFile1.body);
+ assert.equal(leftLabelContent.textContent, '1×1 image/bmp');
+ leftLoaded = true;
+ if (rightLoaded) {
+ element.removeEventListener('render', rendered);
+ done();
+ }
+ });
+
+ rightImage.addEventListener('load', () => {
+ assert.isOk(rightImage);
+ assert.equal(rightImage.getAttribute('src'),
+ 'data:image/bmp;base64, ' + mockFile2.body);
+ assert.equal(rightLabelContent.textContent, '1×1 image/bmp');
+
+ rightLoaded = true;
+ if (leftLoaded) {
+ element.removeEventListener('render', rendered);
+ done();
+ }
+ });
+ };
+
+ element.addEventListener('render', rendered);
+
+ element.$.restAPI.getDiffPreferences().then(prefs => {
+ element.prefs = prefs;
+ element.reload();
+ });
+ });
+
+ test('renders added image', done => {
+ const mockDiff = {
+ meta_b: {name: 'carrot.jpg', content_type: 'image/jpeg',
+ lines: 560},
+ intraline_status: 'OK',
+ change_type: 'ADDED',
+ diff_header: [
+ 'diff --git a/carrot.jpg b/carrot.jpg',
+ 'index 0000000..f9c2f2c 100644',
+ '--- /dev/null',
+ '+++ b/carrot.jpg',
+ 'Binary files differ',
+ ],
+ content: [{skip: 66}],
+ binary: true,
+ };
+ sandbox.stub(element.$.restAPI, 'getDiff')
+ .returns(Promise.resolve(mockDiff));
+
+ element.addEventListener('render', () => {
+ // Recognizes that it should be an image diff.
+ assert.isTrue(element.isImageDiff);
+ assert.instanceOf(
+ element.$.diff.$.diffBuilder._builder, GrDiffBuilderImage);
+
+ const leftImage =
+ element.$.diff.$.diffTable.querySelector('td.left img');
+ const rightImage =
+ element.$.diff.$.diffTable.querySelector('td.right img');
+
+ assert.isNotOk(leftImage);
+ assert.isOk(rightImage);
+ done();
+ });
+
+ element.$.restAPI.getDiffPreferences().then(prefs => {
+ element.prefs = prefs;
+ element.reload();
+ });
+ });
+
+ test('renders removed image', done => {
+ const mockDiff = {
+ meta_a: {name: 'carrot.jpg', content_type: 'image/jpeg',
+ lines: 560},
+ intraline_status: 'OK',
+ change_type: 'DELETED',
+ diff_header: [
+ 'diff --git a/carrot.jpg b/carrot.jpg',
+ 'index f9c2f2c..0000000 100644',
+ '--- a/carrot.jpg',
+ '+++ /dev/null',
+ 'Binary files differ',
+ ],
+ content: [{skip: 66}],
+ binary: true,
+ };
+ sandbox.stub(element.$.restAPI, 'getDiff')
+ .returns(Promise.resolve(mockDiff));
+
+ element.addEventListener('render', () => {
+ // Recognizes that it should be an image diff.
+ assert.isTrue(element.isImageDiff);
+ assert.instanceOf(
+ element.$.diff.$.diffBuilder._builder, GrDiffBuilderImage);
+
+ const leftImage =
+ element.$.diff.$.diffTable.querySelector('td.left img');
+ const rightImage =
+ element.$.diff.$.diffTable.querySelector('td.right img');
+
+ assert.isOk(leftImage);
+ assert.isNotOk(rightImage);
+ done();
+ });
+
+ element.$.restAPI.getDiffPreferences().then(prefs => {
+ element.prefs = prefs;
+ element.reload();
+ });
+ });
+
+ test('does not render disallowed image type', done => {
+ const mockDiff = {
+ meta_a: {name: 'carrot.jpg', content_type: 'image/jpeg-evil',
+ lines: 560},
+ intraline_status: 'OK',
+ change_type: 'DELETED',
+ diff_header: [
+ 'diff --git a/carrot.jpg b/carrot.jpg',
+ 'index f9c2f2c..0000000 100644',
+ '--- a/carrot.jpg',
+ '+++ /dev/null',
+ 'Binary files differ',
+ ],
+ content: [{skip: 66}],
+ binary: true,
+ };
+ mockFile1.type = 'image/jpeg-evil';
+
+ sandbox.stub(element.$.restAPI, 'getDiff')
+ .returns(Promise.resolve(mockDiff));
+
+ element.addEventListener('render', () => {
+ // Recognizes that it should be an image diff.
+ assert.isTrue(element.isImageDiff);
+ assert.instanceOf(
+ element.$.diff.$.diffBuilder._builder, GrDiffBuilderImage);
+ const leftImage =
+ element.$.diff.$.diffTable.querySelector('td.left img');
+ assert.isNotOk(leftImage);
+ done();
+ });
+
+ element.$.restAPI.getDiffPreferences().then(prefs => {
+ element.prefs = prefs;
+ element.reload();
+ });
+ });
+ });
});
test('delegates cancel()', () => {
- element = fixture('basic');
const stub = sandbox.stub(element.$.diff, 'cancel');
element.reload();
assert.isTrue(stub.calledOnce);
@@ -64,7 +478,6 @@
});
test('delegates getCursorStops()', () => {
- element = fixture('basic');
const returnValue = [document.createElement('b')];
const stub = sandbox.stub(element.$.diff, 'getCursorStops')
.returns(returnValue);
@@ -74,7 +487,6 @@
});
test('delegates isRangeSelected()', () => {
- element = fixture('basic');
const returnValue = true;
const stub = sandbox.stub(element.$.diff, 'isRangeSelected')
.returns(returnValue);
@@ -84,33 +496,66 @@
});
test('delegates toggleLeftDiff()', () => {
- element = fixture('basic');
const stub = sandbox.stub(element.$.diff, 'toggleLeftDiff');
element.toggleLeftDiff();
assert.isTrue(stub.calledOnce);
assert.equal(stub.lastCall.args.length, 0);
});
- test('delegates loadBlame()', () => {
- element = fixture('basic');
- const returnValue = Promise.resolve();
- const stub = sandbox.stub(element.$.diff, 'loadBlame')
- .returns(returnValue);
- assert.equal(element.loadBlame(), returnValue);
- assert.isTrue(stub.calledOnce);
- assert.equal(stub.lastCall.args.length, 0);
- });
+ suite('blame', () => {
+ setup(() => {
+ element = fixture('basic');
+ });
- test('delegates clearBlame()', () => {
- element = fixture('basic');
- const stub = sandbox.stub(element.$.diff, 'clearBlame');
- element.clearBlame();
- assert.isTrue(stub.calledOnce);
- assert.equal(stub.lastCall.args.length, 0);
+ test('clearBlame', () => {
+ element._blame = [];
+ const setBlameSpy = sandbox.spy(element.$.diff.$.diffBuilder, 'setBlame');
+ element.clearBlame();
+ assert.isNull(element._blame);
+ assert.isTrue(setBlameSpy.calledWithExactly(null));
+ assert.equal(element.isBlameLoaded, false);
+ });
+
+ test('loadBlame', () => {
+ const mockBlame = [{id: 'commit id', ranges: [{start: 1, end: 2}]}];
+ const showAlertStub = sinon.stub();
+ element.addEventListener('show-alert', showAlertStub);
+ const getBlameStub = sandbox.stub(element.$.restAPI, 'getBlame')
+ .returns(Promise.resolve(mockBlame));
+ element.changeNum = 42;
+ element.patchRange = {patchNum: 5, basePatchNum: 4};
+ element.path = 'foo/bar.baz';
+ return element.loadBlame().then(() => {
+ assert.isTrue(getBlameStub.calledWithExactly(
+ 42, 5, 'foo/bar.baz', true));
+ assert.isFalse(showAlertStub.called);
+ assert.equal(element._blame, mockBlame);
+ assert.equal(element.isBlameLoaded, true);
+ });
+ });
+
+ test('loadBlame empty', () => {
+ const mockBlame = [];
+ const showAlertStub = sinon.stub();
+ element.addEventListener('show-alert', showAlertStub);
+ sandbox.stub(element.$.restAPI, 'getBlame')
+ .returns(Promise.resolve(mockBlame));
+ element.changeNum = 42;
+ element.patchRange = {patchNum: 5, basePatchNum: 4};
+ element.path = 'foo/bar.baz';
+ return element.loadBlame()
+ .then(() => {
+ assert.isTrue(false, 'Promise should not resolve');
+ })
+ .catch(() => {
+ assert.isTrue(showAlertStub.calledOnce);
+ assert.isNull(element._blame);
+ assert.equal(element.isBlameLoaded, false);
+ });
+ });
});
test('delegates getThreadEls()', () => {
- element = fixture('basic');
const returnValue = [document.createElement('b')];
const stub = sandbox.stub(element.$.diff, 'getThreadEls')
.returns(returnValue);
@@ -120,7 +565,6 @@
});
test('delegates addDraftAtLine(el)', () => {
- element = fixture('basic');
const param0 = document.createElement('b');
const stub = sandbox.stub(element.$.diff, 'addDraftAtLine');
element.addDraftAtLine(param0);
@@ -130,7 +574,6 @@
});
test('delegates clearDiffContent()', () => {
- element = fixture('basic');
const stub = sandbox.stub(element.$.diff, 'clearDiffContent');
element.clearDiffContent();
assert.isTrue(stub.calledOnce);
@@ -138,7 +581,6 @@
});
test('delegates expandAllContext()', () => {
- element = fixture('basic');
const stub = sandbox.stub(element.$.diff, 'expandAllContext');
element.expandAllContext();
assert.isTrue(stub.calledOnce);
@@ -146,101 +588,66 @@
});
test('passes in changeNum', () => {
- element = fixture('basic');
const value = '12345';
element.changeNum = value;
assert.equal(element.$.diff.changeNum, value);
});
test('passes in noAutoRender', () => {
- element = fixture('basic');
const value = true;
element.noAutoRender = value;
assert.equal(element.$.diff.noAutoRender, value);
});
test('passes in patchRange', () => {
- element = fixture('basic');
const value = {patchNum: 'foo', basePatchNum: 'bar'};
element.patchRange = value;
assert.equal(element.$.diff.patchRange, value);
});
test('passes in path', () => {
- element = fixture('basic');
const value = 'some/file/path';
element.path = value;
assert.equal(element.$.diff.path, value);
});
test('passes in prefs', () => {
- element = fixture('basic');
const value = {};
element.prefs = value;
assert.equal(element.$.diff.prefs, value);
});
test('passes in projectConfig', () => {
- element = fixture('basic');
const value = {};
element.projectConfig = value;
assert.equal(element.$.diff.projectConfig, value);
});
test('passes in changeNum', () => {
- element = fixture('basic');
const value = '12345';
element.changeNum = value;
assert.equal(element.$.diff.changeNum, value);
});
test('passes in projectName', () => {
- element = fixture('basic');
const value = 'Gerrit';
element.projectName = value;
assert.equal(element.$.diff.projectName, value);
});
test('passes in displayLine', () => {
- element = fixture('basic');
const value = true;
element.displayLine = value;
assert.equal(element.$.diff.displayLine, value);
});
- test('passes out isImageDiff', () => {
- element = fixture('basic');
- const value = true;
- // isImageDiff is computed, so we cannot just set it.
- sandbox.stub(element.$.diff, '_computeIsImageDiff').returns(value);
- element.$.diff._diff = {left: [], right: [], content: []};
-
- assert.equal(element.isImageDiff, value);
- });
-
test('passes in commitRange', () => {
- element = fixture('basic');
const value = {};
element.commitRange = value;
assert.equal(element.$.diff.commitRange, value);
});
- test('passes in filesWeblinks', () => {
- element = fixture('basic');
- const value = {};
- element.filesWeblinks = value;
- assert.equal(element.$.diff.filesWeblinks, value);
- });
-
- test('passes out filesWeblinks', () => {
- element = fixture('basic');
- const value = {};
- element.$.diff.filesWeblinks = value;
- assert.equal(element.filesWeblinks, value);
- });
-
test('passes in hidden', () => {
- element = fixture('basic');
const value = true;
element.hidden = value;
assert.equal(element.$.diff.hidden, value);
@@ -248,53 +655,125 @@
});
test('passes in noRenderOnPrefsChange', () => {
- element = fixture('basic');
const value = true;
element.noRenderOnPrefsChange = value;
assert.equal(element.$.diff.noRenderOnPrefsChange, value);
});
test('passes in comments', () => {
- element = fixture('basic');
const value = {left: [], right: []};
element.comments = value;
assert.equal(element.$.diff.comments, value);
});
test('passes in lineWrapping', () => {
- element = fixture('basic');
const value = true;
element.lineWrapping = value;
assert.equal(element.$.diff.lineWrapping, value);
});
test('passes in viewMode', () => {
- element = fixture('basic');
const value = 'SIDE_BY_SIDE';
element.viewMode = value;
assert.equal(element.$.diff.viewMode, value);
});
test('passes in lineOfInterest', () => {
- element = fixture('basic');
const value = {number: 123, leftSide: true};
element.lineOfInterest = value;
assert.equal(element.$.diff.lineOfInterest, value);
});
- test('passes in showLoadFailure', () => {
- element = fixture('basic');
- const value = true;
- element.showLoadFailure = value;
- assert.equal(element.$.diff.showLoadFailure, value);
- });
+ suite('_reportDiff', () => {
+ let reportStub;
- test('passes out isBlameLoaded', () => {
- element = fixture('basic');
- const value = true;
- sandbox.stub(element.$.diff, '_computeIsBlameLoaded').returns(value);
- element.$.diff._blame = {};
- assert.equal(element.isBlameLoaded, value);
+ setup(() => {
+ element = fixture('basic');
+ element.patchRange = {basePatchNum: 1};
+ reportStub = sandbox.stub(element.$.reporting, 'reportInteraction');
+ });
+
+ test('null and content-less', () => {
+ element._reportDiff(null);
+ assert.isFalse(reportStub.called);
+
+ element._reportDiff({});
+ assert.isFalse(reportStub.called);
+ });
+
+ test('diff w/ no delta', () => {
+ const diff = {
+ content: [
+ {ab: ['foo', 'bar']},
+ {ab: ['baz', 'foo']},
+ ],
+ };
+ element._reportDiff(diff);
+ assert.isTrue(reportStub.calledOnce);
+ assert.equal(reportStub.lastCall.args[0], 'rebase-percent-zero');
+ assert.isUndefined(reportStub.lastCall.args[1]);
+ });
+
+ test('diff w/ no rebase delta', () => {
+ const diff = {
+ content: [
+ {ab: ['foo', 'bar']},
+ {a: ['baz', 'foo']},
+ {ab: ['foo', 'bar']},
+ {a: ['baz', 'foo'], b: ['bar', 'baz']},
+ {ab: ['foo', 'bar']},
+ {b: ['baz', 'foo']},
+ {ab: ['foo', 'bar']},
+ ],
+ };
+ element._reportDiff(diff);
+ assert.isTrue(reportStub.calledOnce);
+ assert.equal(reportStub.lastCall.args[0], 'rebase-percent-zero');
+ assert.isUndefined(reportStub.lastCall.args[1]);
+ });
+
+ test('diff w/ some rebase delta', () => {
+ const diff = {
+ content: [
+ {ab: ['foo', 'bar']},
+ {a: ['baz', 'foo'], due_to_rebase: true},
+ {ab: ['foo', 'bar']},
+ {a: ['baz', 'foo'], b: ['bar', 'baz']},
+ {ab: ['foo', 'bar']},
+ {b: ['baz', 'foo'], due_to_rebase: true},
+ {ab: ['foo', 'bar']},
+ {a: ['baz', 'foo']},
+ ],
+ };
+ element._reportDiff(diff);
+ assert.isTrue(reportStub.calledOnce);
+ assert.equal(reportStub.lastCall.args[0], 'rebase-percent-nonzero');
+ assert.strictEqual(reportStub.lastCall.args[1], 50);
+ });
+
+ test('diff w/ all rebase delta', () => {
+ const diff = {content: [{
+ a: ['foo', 'bar'],
+ b: ['baz', 'foo'],
+ due_to_rebase: true,
+ }]};
+ element._reportDiff(diff);
+ assert.isTrue(reportStub.calledOnce);
+ assert.equal(reportStub.lastCall.args[0], 'rebase-percent-nonzero');
+ assert.strictEqual(reportStub.lastCall.args[1], 100);
+ });
+
+ test('diff against parent event', () => {
+ element.patchRange.basePatchNum = 'PARENT';
+ const diff = {content: [{
+ a: ['foo', 'bar'],
+ b: ['baz', 'foo'],
+ }]};
+ element._reportDiff(diff);
+ assert.isTrue(reportStub.calledOnce);
+ assert.equal(reportStub.lastCall.args[0], 'diff-against-parent');
+ assert.isUndefined(reportStub.lastCall.args[1]);
+ });
});
});
</script>
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
index 1aba403..6add75c 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
@@ -18,9 +18,7 @@
<link rel="import" href="../../../bower_components/polymer/polymer.html">
<link rel="import" href="../../../behaviors/gr-patch-set-behavior/gr-patch-set-behavior.html">
<link rel="import" href="../../../styles/shared-styles.html">
-<link rel="import" href="../../core/gr-reporting/gr-reporting.html">
<link rel="import" href="../../shared/gr-button/gr-button.html">
-<link rel="import" href="../../shared/gr-rest-api-interface/gr-rest-api-interface.html">
<link rel="import" href="../gr-diff-builder/gr-diff-builder.html">
<link rel="import" href="../gr-diff-comment-thread/gr-diff-comment-thread.html">
<link rel="import" href="../gr-diff-highlight/gr-diff-highlight.html">
@@ -55,7 +53,6 @@
background-color: var(--table-header-background-color);
}
.image-diff .gr-diff {
- background-color: var(--table-header-background-color);
text-align: center;
}
.image-diff img {
@@ -83,6 +80,9 @@
.content {
background-color: var(--view-background-color);
}
+ .image-diff .content {
+ background-color: var(--table-header-background-color);
+ }
.full-width {
width: 100%;
}
@@ -270,26 +270,26 @@
<div>[[item]]</div>
</template>
</div>
- <div class$="[[_computeContainerClass(_loggedIn, viewMode, displayLine)]]"
+ <div class$="[[_computeContainerClass(loggedIn, viewMode, displayLine)]]"
on-tap="_handleTap">
- <gr-diff-selection diff="[[_diff]]">
+ <gr-diff-selection diff="[[diff]]">
<gr-diff-highlight
id="highlights"
- logged-in="[[_loggedIn]]"
+ logged-in="[[loggedIn]]"
comments="{{comments}}">
<gr-diff-builder
id="diffBuilder"
comments="[[comments]]"
project-name="[[projectName]]"
- diff="[[_diff]]"
+ diff="[[diff]]"
diff-path="[[path]]"
change-num="[[changeNum]]"
patch-num="[[patchRange.patchNum]]"
view-mode="[[viewMode]]"
line-wrapping="[[lineWrapping]]"
is-image-diff="[[isImageDiff]]"
- base-image="[[_baseImage]]"
- revision-image="[[_revisionImage]]"
+ base-image="[[baseImage]]"
+ revision-image="[[revisionImage]]"
parent-index="[[_parentIndex]]"
create-comment-fn="[[_createThreadGroupFn]]"
line-of-interest="[[lineOfInterest]]">
@@ -301,16 +301,16 @@
</gr-diff-highlight>
</gr-diff-selection>
</div>
- <div class$="[[_computeNewlineWarningClass(_newlineWarning, _loading)]]">
+ <div class$="[[_computeNewlineWarningClass(_newlineWarning, loading)]]">
[[_newlineWarning]]
</div>
- <div id="loadingError" class$="[[_computeErrorClass(_errorMessage)]]">
- [[_errorMessage]]
+ <div id="loadingError" class$="[[_computeErrorClass(errorMessage)]]">
+ [[errorMessage]]
</div>
<div id="sizeWarning" class$="[[_computeWarningClass(_showWarning)]]">
<p>
Prevented render because "Whole file" is enabled and this diff is very
- large (about [[_diffLength(_diff)]] lines).
+ large (about [[_diffLength(diff)]] lines).
</p>
<gr-button on-tap="_handleLimitedBypass">
Render with limited context
@@ -319,8 +319,6 @@
Render anyway (may be slow)
</gr-button>
</div>
- <gr-rest-api-interface id="restAPI"></gr-rest-api-interface>
- <gr-reporting id="reporting" category="diff"></gr-reporting>
</template>
<script src="gr-diff-line.js"></script>
<script src="gr-diff-group.js"></script>
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
index 4f9de6f..4f0a73b 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
@@ -21,11 +21,6 @@
const ERR_COMMENT_ON_EDIT_BASE = 'You cannot comment on the base patch set ' +
'of an edit.';
const ERR_INVALID_LINE = 'Invalid line number: ';
- const MSG_EMPTY_BLAME = 'No blame information for this diff.';
-
- const EVENT_AGAINST_PARENT = 'diff-against-parent';
- const EVENT_ZERO_REBASE = 'rebase-percent-zero';
- const EVENT_NONZERO_REBASE = 'rebase-percent-nonzero';
const NO_NEWLINE_BASE = 'No newline at end of base file.';
const NO_NEWLINE_REVISION = 'No newline at end of revision file.';
@@ -64,6 +59,12 @@
* @event diff-comments-modified
*/
+ /**
+ * Fired when a draft is added or edited.
+ *
+ * @event draft-interaction
+ */
+
properties: {
changeNum: String,
noAutoRender: {
@@ -88,15 +89,8 @@
},
isImageDiff: {
type: Boolean,
- computed: '_computeIsImageDiff(_diff)',
- notify: true,
},
commitRange: Object,
- filesWeblinks: {
- type: Object,
- value() { return {}; },
- notify: true,
- },
hidden: {
type: Boolean,
reflectToAttribute: true,
@@ -123,38 +117,33 @@
*/
lineOfInterest: Object,
- /**
- * If the diff fails to load, show the failure message in the diff rather
- * than bubbling the error up to the whole page. This is useful for when
- * loading inline diffs because one diff failing need not mark the whole
- * page with a failure.
- */
- showLoadFailure: Boolean,
-
- _loading: {
+ loading: {
type: Boolean,
value: false,
observer: '_loadingChanged',
},
- _loggedIn: {
+ loggedIn: {
type: Boolean,
value: false,
},
- _diff: Object,
+ diff: {
+ type: Object,
+ observer: '_diffChanged',
+ },
_diffHeaderItems: {
type: Array,
value: [],
- computed: '_computeDiffHeaderItems(_diff.*)',
+ computed: '_computeDiffHeaderItems(diff.*)',
},
_diffTableClass: {
type: String,
value: '',
},
/** @type {?Object} */
- _baseImage: Object,
+ baseImage: Object,
/** @type {?Object} */
- _revisionImage: Object,
+ revisionImage: Object,
/**
* Whether the safety check for large diffs when whole-file is set has
@@ -172,20 +161,16 @@
_showWarning: Boolean,
/** @type {?string} */
- _errorMessage: {
+ errorMessage: {
type: String,
value: null,
},
/** @type {?Object} */
- _blame: {
+ blame: {
type: Object,
value: null,
- },
- isBlameLoaded: {
- type: Boolean,
- notify: true,
- computed: '_computeIsBlameLoaded(_blame)',
+ observer: '_blameChanged',
},
_parentIndex: {
@@ -195,7 +180,7 @@
_newlineWarning: {
type: String,
- computed: '_computeNewlineWarning(_diff)',
+ computed: '_computeNewlineWarning(diff)',
},
/**
@@ -220,56 +205,6 @@
'create-comment': '_handleCreateComment',
},
- attached() {
- this._getLoggedIn().then(loggedIn => {
- this._loggedIn = loggedIn;
- });
- },
-
- ready() {
- if (this._canRender()) {
- this.reload();
- }
- },
-
- /** @return {!Promise} */
- reload() {
- this._loading = true;
- this._errorMessage = null;
-
- const diffRequest = this._getDiff()
- .then(diff => {
- this._reportDiff(diff);
- return diff;
- })
- .catch(e => {
- this._handleGetDiffError(e);
- return null;
- });
-
- const assetRequest = diffRequest.then(diff => {
- // If the diff is null, then it's failed to load.
- if (!diff) { return null; }
-
- return this._loadDiffAssets(diff);
- });
-
- return Promise.all([diffRequest, assetRequest])
- .then(results => {
- const diff = results[0];
- if (!diff) {
- return Promise.resolve();
- }
- this.filesWeblinks = this._getFilesWeblinks(diff);
- this._diff = diff;
- return this._renderDiffTable();
- })
- .catch(err => {
- console.warn('Error encountered loading diff:', err);
- })
- .then(() => { this._loading = false; });
- },
-
/** Cancel any remaining diff builder rendering work. */
cancel() {
this.$.diffBuilder.cancel();
@@ -293,37 +228,13 @@
this.toggleClass('no-left');
},
- /**
- * Load and display blame information for the base of the diff.
- * @return {Promise} A promise that resolves when blame finishes rendering.
- */
- loadBlame() {
- return this.$.restAPI.getBlame(this.changeNum, this.patchRange.patchNum,
- this.path, true)
- .then(blame => {
- if (!blame.length) {
- this.fire('show-alert', {message: MSG_EMPTY_BLAME});
- return Promise.reject(MSG_EMPTY_BLAME);
- }
-
- this._blame = blame;
-
- this.$.diffBuilder.setBlame(blame);
- this.classList.add('showBlame');
- });
- },
-
- _computeIsBlameLoaded(blame) {
- return !!blame;
- },
-
- /**
- * Unload blame information for the diff.
- */
- clearBlame() {
- this._blame = null;
- this.$.diffBuilder.setBlame(null);
- this.classList.remove('showBlame');
+ _blameChanged(newValue) {
+ this.$.diffBuilder.setBlame(newValue);
+ if (newValue) {
+ this.classList.add('showBlame');
+ } else {
+ this.classList.remove('showBlame');
+ }
},
_handleCommentSaveOrDiscard() {
@@ -331,12 +242,6 @@
{bubbles: true}));
},
- /** @return {boolean}} */
- _canRender() {
- return !!this.changeNum && !!this.patchRange && !!this.path &&
- !this.noAutoRender;
- },
-
/** @return {!Array<!HTMLElement>} */
getThreadEls() {
let threads = [];
@@ -425,7 +330,7 @@
/** @return {boolean} */
_isValidElForComment(el) {
- if (!this._loggedIn) {
+ if (!this.loggedIn) {
this.fire('show-auth-required');
return false;
}
@@ -454,7 +359,7 @@
* @param {!Object=} opt_range
*/
_createComment(lineEl, opt_lineNum, opt_side, opt_range) {
- this.$.reporting.recordDraftInteraction();
+ this.dispatchEvent(new CustomEvent('draft-interaction', {bubbles: true}));
const contentText = this.$.diffBuilder.getContentByLineEl(lineEl);
const contentEl = contentText.parentElement;
const side = opt_side ||
@@ -674,7 +579,7 @@
_loadingChanged(newValue) {
if (newValue) {
this.cancel();
- this.clearBlame();
+ this._blame = null;
this._safetyBypass = null;
this._showWarning = false;
this.clearDiffContent();
@@ -688,7 +593,7 @@
_prefsChanged(prefs) {
if (!prefs) { return; }
- this.clearBlame();
+ this._blame = null;
const stylesToUpdate = {};
@@ -709,22 +614,32 @@
this.updateStyles(stylesToUpdate);
- if (this._diff && this.comments && !this.noRenderOnPrefsChange) {
+ if (this.diff && this.comments && !this.noRenderOnPrefsChange) {
+ this._renderDiffTable();
+ }
+ },
+
+ _diffChanged(newValue) {
+ if (newValue) {
this._renderDiffTable();
}
},
_renderDiffTable() {
- if (!this.prefs) { return Promise.resolve(); }
+ if (!this.prefs) {
+ this.dispatchEvent(new CustomEvent('render', {bubbles: true}));
+ return;
+ }
if (this.prefs.context === -1 &&
- this._diffLength(this._diff) >= LARGE_DIFF_THRESHOLD_LINES &&
+ this._diffLength(this.diff) >= LARGE_DIFF_THRESHOLD_LINES &&
this._safetyBypass === null) {
this._showWarning = true;
- return Promise.resolve();
+ this.dispatchEvent(new CustomEvent('render', {bubbles: true}));
+ return;
}
this._showWarning = false;
- return this.$.diffBuilder.render(this.comments, this._getBypassPrefs());
+ this.$.diffBuilder.render(this.comments, this._getBypassPrefs());
},
/**
@@ -741,138 +656,6 @@
this.$.diffTable.innerHTML = null;
},
- _handleGetDiffError(response) {
- // Loading the diff may respond with 409 if the file is too large. In this
- // case, use a toast error..
- if (response.status === 409) {
- this.fire('server-error', {response});
- return;
- }
-
- if (this.showLoadFailure) {
- this._errorMessage = [
- 'Encountered error when loading the diff:',
- response.status,
- response.statusText,
- ].join(' ');
- return;
- }
-
- this.fire('page-error', {response});
- },
-
- /** @return {!Promise<!Object>} */
- _getDiff() {
- // Wrap the diff request in a new promise so that the error handler
- // rejects the promise, allowing the error to be handled in the .catch.
- return new Promise((resolve, reject) => {
- this.$.restAPI.getDiff(
- this.changeNum,
- this.patchRange.basePatchNum,
- this.patchRange.patchNum,
- this.path,
- reject)
- .then(resolve);
- });
- },
-
- _getFilesWeblinks(diff) {
- if (!this.commitRange) { return {}; }
- return {
- meta_a: Gerrit.Nav.getFileWebLinks(
- this.projectName, this.commitRange.baseCommit, this.path,
- {weblinks: diff && diff.meta_a && diff.meta_a.web_links}),
- meta_b: Gerrit.Nav.getFileWebLinks(
- this.projectName, this.commitRange.commit, this.path,
- {weblinks: diff && diff.meta_b && diff.meta_b.web_links}),
- };
- },
-
- /**
- * Report info about the diff response.
- */
- _reportDiff(diff) {
- if (!diff || !diff.content) { return; }
-
- // Count the delta lines stemming from normal deltas, and from
- // due_to_rebase deltas.
- let nonRebaseDelta = 0;
- let rebaseDelta = 0;
- diff.content.forEach(chunk => {
- if (chunk.ab) { return; }
- const deltaSize = Math.max(
- chunk.a ? chunk.a.length : 0, chunk.b ? chunk.b.length : 0);
- if (chunk.due_to_rebase) {
- rebaseDelta += deltaSize;
- } else {
- nonRebaseDelta += deltaSize;
- }
- });
-
- // Find the percent of the delta from due_to_rebase chunks rounded to two
- // digits. Diffs with no delta are considered 0%.
- const totalDelta = rebaseDelta + nonRebaseDelta;
- const percentRebaseDelta = !totalDelta ? 0 :
- Math.round(100 * rebaseDelta / totalDelta);
-
- // Report the due_to_rebase percentage in the "diff" category when
- // applicable.
- if (this.patchRange.basePatchNum === 'PARENT') {
- this.$.reporting.reportInteraction(EVENT_AGAINST_PARENT);
- } else if (percentRebaseDelta === 0) {
- this.$.reporting.reportInteraction(EVENT_ZERO_REBASE);
- } else {
- this.$.reporting.reportInteraction(EVENT_NONZERO_REBASE,
- percentRebaseDelta);
- }
- },
-
- /** @return {!Promise} */
- _getLoggedIn() {
- return this.$.restAPI.getLoggedIn();
- },
-
- /**
- * @param {Object} diff
- * @return {boolean}
- */
- _computeIsImageDiff(diff) {
- if (!diff) { return false; }
-
- const isA = diff.meta_a &&
- diff.meta_a.content_type.startsWith('image/');
- const isB = diff.meta_b &&
- diff.meta_b.content_type.startsWith('image/');
-
- return !!(diff.binary && (isA || isB));
- },
-
- /**
- * @param {Object} diff
- * @return {!Promise}
- */
- _loadDiffAssets(diff) {
- if (this._computeIsImageDiff(diff)) {
- return this._getImages(diff).then(images => {
- this._baseImage = images.baseImage;
- this._revisionImage = images.revisionImage;
- });
- } else {
- this._baseImage = null;
- this._revisionImage = null;
- return Promise.resolve();
- }
- },
-
- /**
- * @param {Object} diff
- * @return {!Promise}
- */
- _getImages(diff) {
- return this.$.restAPI.getImagesForDiff(this.changeNum, diff,
- this.patchRange);
- },
-
_projectConfigChanged(projectConfig) {
const threadEls = this.getThreadEls();
for (let i = 0; i < threadEls.length; i++) {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html
index 97338e2..0274fae 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html
@@ -48,18 +48,8 @@
sandbox.restore();
});
- test('reload cancels before network resolves', () => {
- element = fixture('basic');
- const cancelStub = sandbox.stub(element, 'cancel');
-
- // Stub the network calls into requests that never resolve.
- sandbox.stub(element, '_getDiff', () => new Promise(() => {}));
-
- element.reload();
- assert.isTrue(cancelStub.called);
- });
-
test('cancel', () => {
+ element = fixture('basic');
const cancelStub = sandbox.stub(element.$.diffBuilder, 'cancel');
element.cancel();
assert.isTrue(cancelStub.calledOnce);
@@ -208,41 +198,6 @@
element.$$('.diffContainer').classList.contains('displayLine'));
});
- test('loads files weblinks', () => {
- const weblinksStub = sandbox.stub(Gerrit.Nav, '_generateWeblinks')
- .returns({name: 'stubb', url: '#s'});
- sandbox.stub(element.$.restAPI, 'getDiff').returns(Promise.resolve({
- content: [],
- }));
- element.projectName = 'test-project';
- element.path = 'test-path';
- element.commitRange = {baseCommit: 'test-base', commit: 'test-commit'};
- element.patchRange = {};
- return element.reload().then(() => {
- assert.isTrue(weblinksStub.calledTwice);
- assert.isTrue(weblinksStub.firstCall.calledWith({
- commit: 'test-base',
- file: 'test-path',
- options: {
- weblinks: undefined,
- },
- repo: 'test-project',
- type: Gerrit.Nav.WeblinkType.FILE}));
- assert.isTrue(weblinksStub.secondCall.calledWith({
- commit: 'test-commit',
- file: 'test-path',
- options: {
- weblinks: undefined,
- },
- repo: 'test-project',
- type: Gerrit.Nav.WeblinkType.FILE}));
- assert.deepEqual(element.filesWeblinks, {
- meta_a: [{name: 'stubb', url: '#s'}],
- meta_b: [{name: 'stubb', url: '#s'}],
- });
- });
- });
-
test('remove comment', () => {
element.comments = {
meta: {
@@ -408,66 +363,29 @@
'wsAAAAAAAAAAAAA/////w==',
type: 'image/bmp',
};
- const mockCommit = {
- commit: '9a1a1d10baece5efbba10bc4ccf808a67a50ac0a',
- parents: [{
- commit: '7338aa9adfe57909f1fdaf88975cdea467d3382f',
- subject: 'Added a carrot',
- }],
- author: {
- name: 'Wyatt Allen',
- email: 'wyatta@google.com',
- date: '2016-05-23 21:44:51.000000000',
- tz: -420,
- },
- committer: {
- name: 'Wyatt Allen',
- email: 'wyatta@google.com',
- date: '2016-05-25 00:25:41.000000000',
- tz: -420,
- },
- subject: 'Updated the carrot',
- message: 'Updated the carrot\n\nChange-Id: Iabcd123\n',
- };
- const mockComments = {baseComments: [], comments: []};
-
- sandbox.stub(element.$.restAPI, 'getCommitInfo')
- .returns(Promise.resolve(mockCommit));
- sandbox.stub(element.$.restAPI,
- 'getB64FileContents',
- (changeId, patchNum, path, opt_parentIndex) => {
- return Promise.resolve(opt_parentIndex === 1 ? mockFile1 :
- mockFile2);
- });
- sandbox.stub(element.$.restAPI, '_getDiffComments')
- .returns(Promise.resolve(mockComments));
- sandbox.stub(element.$.restAPI, 'getDiffDrafts')
- .returns(Promise.resolve(mockComments));
element.patchRange = {basePatchNum: 'PARENT', patchNum: 1};
element.comments = {left: [], right: []};
+ element.isImageDiff = true;
+ element.prefs = {
+ auto_hide_diff_table_header: true,
+ context: 10,
+ cursor_blink_rate: 0,
+ font_size: 12,
+ ignore_whitespace: 'IGNORE_NONE',
+ intraline_difference: true,
+ line_length: 100,
+ line_wrapping: false,
+ show_line_endings: true,
+ show_tabs: true,
+ show_whitespace_errors: true,
+ syntax_highlighting: true,
+ tab_size: 8,
+ theme: 'DEFAULT',
+ };
});
test('renders image diffs with same file name', done => {
- const mockDiff = {
- meta_a: {name: 'carrot.jpg', content_type: 'image/jpeg', lines: 66},
- meta_b: {name: 'carrot.jpg', content_type: 'image/jpeg',
- lines: 560},
- intraline_status: 'OK',
- change_type: 'MODIFIED',
- diff_header: [
- 'diff --git a/carrot.jpg b/carrot.jpg',
- 'index 2adc47d..f9c2f2c 100644',
- '--- a/carrot.jpg',
- '+++ b/carrot.jpg',
- 'Binary files differ',
- ],
- content: [{skip: 66}],
- binary: true,
- };
- sandbox.stub(element.$.restAPI, 'getDiff')
- .returns(Promise.resolve(mockDiff));
-
const rendered = () => {
// Recognizes that it should be an image diff.
assert.isTrue(element.isImageDiff);
@@ -522,10 +440,24 @@
element.addEventListener('render', rendered);
- element.$.restAPI.getDiffPreferences().then(prefs => {
- element.prefs = prefs;
- element.reload();
- });
+ element.baseImage = mockFile1;
+ element.revisionImage = mockFile2;
+ element.diff = {
+ meta_a: {name: 'carrot.jpg', content_type: 'image/jpeg', lines: 66},
+ meta_b: {name: 'carrot.jpg', content_type: 'image/jpeg',
+ lines: 560},
+ intraline_status: 'OK',
+ change_type: 'MODIFIED',
+ diff_header: [
+ 'diff --git a/carrot.jpg b/carrot.jpg',
+ 'index 2adc47d..f9c2f2c 100644',
+ '--- a/carrot.jpg',
+ '+++ b/carrot.jpg',
+ 'Binary files differ',
+ ],
+ content: [{skip: 66}],
+ binary: true,
+ };
});
test('renders image diffs with a different file name', done => {
@@ -545,8 +477,6 @@
content: [{skip: 66}],
binary: true,
};
- sandbox.stub(element.$.restAPI, 'getDiff')
- .returns(Promise.resolve(mockDiff));
const rendered = () => {
// Recognizes that it should be an image diff.
@@ -604,10 +534,11 @@
element.addEventListener('render', rendered);
- element.$.restAPI.getDiffPreferences().then(prefs => {
- element.prefs = prefs;
- element.reload();
- });
+ element.baseImage = mockFile1;
+ element.baseImage._name = mockDiff.meta_a.name;
+ element.revisionImage = mockFile2;
+ element.revisionImage._name = mockDiff.meta_b.name;
+ element.diff = mockDiff;
});
test('renders added image', done => {
@@ -626,8 +557,6 @@
content: [{skip: 66}],
binary: true,
};
- sandbox.stub(element.$.restAPI, 'getDiff')
- .returns(Promise.resolve(mockDiff));
element.addEventListener('render', () => {
// Recognizes that it should be an image diff.
@@ -643,10 +572,8 @@
done();
});
- element.$.restAPI.getDiffPreferences().then(prefs => {
- element.prefs = prefs;
- element.reload();
- });
+ element.revisionImage = mockFile2;
+ element.diff = mockDiff;
});
test('renders removed image', done => {
@@ -665,8 +592,6 @@
content: [{skip: 66}],
binary: true,
};
- sandbox.stub(element.$.restAPI, 'getDiff')
- .returns(Promise.resolve(mockDiff));
element.addEventListener('render', () => {
// Recognizes that it should be an image diff.
@@ -682,10 +607,8 @@
done();
});
- element.$.restAPI.getDiffPreferences().then(prefs => {
- element.prefs = prefs;
- element.reload();
- });
+ element.baseImage = mockFile1;
+ element.diff = mockDiff;
});
test('does not render disallowed image type', done => {
@@ -706,9 +629,6 @@
};
mockFile1.type = 'image/jpeg-evil';
- sandbox.stub(element.$.restAPI, 'getDiff')
- .returns(Promise.resolve(mockDiff));
-
element.addEventListener('render', () => {
// Recognizes that it should be an image diff.
assert.isTrue(element.isImageDiff);
@@ -719,10 +639,8 @@
done();
});
- element.$.restAPI.getDiffPreferences().then(prefs => {
- element.prefs = prefs;
- element.reload();
- });
+ element.baseImage = mockFile1;
+ element.diff = mockDiff;
});
});
@@ -769,67 +687,10 @@
content.click();
});
- test('_getDiff handles null diff responses', done => {
- stub('gr-rest-api-interface', {
- getDiff() { return Promise.resolve(null); },
- });
- element.changeNum = 123;
- element.patchRange = {basePatchNum: 1, patchNum: 2};
- element.path = 'file.txt';
- element._getDiff().then(done);
- });
-
- test('reload resolves on error', () => {
- const onErrStub = sandbox.stub(element, '_handleGetDiffError');
- const error = {ok: false, status: 500};
- sandbox.stub(element.$.restAPI, 'getDiff',
- (changeNum, basePatchNum, patchNum, path, onErr) => {
- onErr(error);
- });
- return element.reload().then(() => {
- assert.isTrue(onErrStub.calledOnce);
- });
- });
-
- suite('_handleGetDiffError', () => {
- let serverErrorStub;
- let pageErrorStub;
-
- setup(() => {
- serverErrorStub = sinon.stub();
- element.addEventListener('server-error', serverErrorStub);
- pageErrorStub = sinon.stub();
- element.addEventListener('page-error', pageErrorStub);
- });
-
- test('page error on HTTP-409', () => {
- element._handleGetDiffError({status: 409});
- assert.isTrue(serverErrorStub.calledOnce);
- assert.isFalse(pageErrorStub.called);
- assert.isNotOk(element._errorMessage);
- });
-
- test('server error on non-HTTP-409', () => {
- element._handleGetDiffError({status: 500});
- assert.isFalse(serverErrorStub.called);
- assert.isTrue(pageErrorStub.calledOnce);
- assert.isNotOk(element._errorMessage);
- });
-
- test('error message if showLoadFailure', () => {
- element.showLoadFailure = true;
- element._handleGetDiffError({status: 500, statusText: 'Failure!'});
- assert.isFalse(serverErrorStub.called);
- assert.isFalse(pageErrorStub.called);
- assert.equal(element._errorMessage,
- 'Encountered error when loading the diff: 500 Failure!');
- });
- });
-
suite('getCursorStops', () => {
const setupDiff = function() {
const mock = document.createElement('mock-diff-response');
- element._diff = mock.diffResponse;
+ element.diff = mock.diffResponse;
element.comments = {
left: [],
right: [],
@@ -878,15 +739,8 @@
suite('logged in', () => {
let fakeLineEl;
setup(() => {
- const getLoggedInPromise = Promise.resolve(true);
- stub('gr-rest-api-interface', {
- getLoggedIn() { return getLoggedInPromise; },
- getPreferences() {
- return Promise.resolve({time_format: 'HHMM_12'});
- },
- getAccountCapabilities() { return Promise.resolve(); },
- });
element = fixture('basic');
+ element.loggedIn = true;
element.patchRange = {};
fakeLineEl = {
@@ -895,15 +749,11 @@
contains: sandbox.stub().returns(true),
},
};
- return getLoggedInPromise;
});
test('addDraftAtLine', () => {
sandbox.stub(element, '_selectLine');
sandbox.stub(element, '_createComment');
- const loggedInErrorSpy = sandbox.spy();
- element.addEventListener('show-auth-required', loggedInErrorSpy);
- assert.isFalse(loggedInErrorSpy.called);
element.addDraftAtLine(fakeLineEl);
assert.isTrue(element._createComment
.calledWithExactly(fakeLineEl, 42));
@@ -913,12 +763,9 @@
element.patchRange.basePatchNum = element.EDIT_NAME;
sandbox.stub(element, '_selectLine');
sandbox.stub(element, '_createComment');
- const loggedInErrorSpy = sandbox.spy();
const alertSpy = sandbox.spy();
- element.addEventListener('show-auth-required', loggedInErrorSpy);
element.addEventListener('show-alert', alertSpy);
element.addDraftAtLine(fakeLineEl);
- assert.isFalse(loggedInErrorSpy.called);
assert.isTrue(alertSpy.called);
assert.isFalse(element._createComment.called);
});
@@ -928,19 +775,16 @@
element.patchRange.basePatchNum = element.PARENT_NAME;
sandbox.stub(element, '_selectLine');
sandbox.stub(element, '_createComment');
- const loggedInErrorSpy = sandbox.spy();
const alertSpy = sandbox.spy();
- element.addEventListener('show-auth-required', loggedInErrorSpy);
element.addEventListener('show-alert', alertSpy);
element.addDraftAtLine(fakeLineEl);
- assert.isFalse(loggedInErrorSpy.called);
assert.isTrue(alertSpy.called);
assert.isFalse(element._createComment.called);
});
suite('change in preferences', () => {
setup(() => {
- element._diff = {
+ element.diff = {
meta_a: {name: 'carrot.jpg', content_type: 'image/jpeg', lines: 66},
meta_b: {name: 'carrot.jpg', content_type: 'image/jpeg',
lines: 560},
@@ -1074,7 +918,8 @@
suite('diff header', () => {
setup(() => {
- element._diff = {
+ element = fixture('basic');
+ element.diff = {
meta_a: {name: 'carrot.jpg', content_type: 'image/jpeg', lines: 66},
meta_b: {name: 'carrot.jpg', content_type: 'image/jpeg',
lines: 560},
@@ -1087,15 +932,15 @@
test('hidden', () => {
assert.equal(element._diffHeaderItems.length, 0);
- element.push('_diff.diff_header', 'diff --git a/test.jpg b/test.jpg');
+ element.push('diff.diff_header', 'diff --git a/test.jpg b/test.jpg');
assert.equal(element._diffHeaderItems.length, 0);
- element.push('_diff.diff_header', 'index 2adc47d..f9c2f2c 100644');
+ element.push('diff.diff_header', 'index 2adc47d..f9c2f2c 100644');
assert.equal(element._diffHeaderItems.length, 0);
- element.push('_diff.diff_header', '--- a/test.jpg');
+ element.push('diff.diff_header', '--- a/test.jpg');
assert.equal(element._diffHeaderItems.length, 0);
- element.push('_diff.diff_header', '+++ b/test.jpg');
+ element.push('diff.diff_header', '+++ b/test.jpg');
assert.equal(element._diffHeaderItems.length, 0);
- element.push('_diff.diff_header', 'test');
+ element.push('diff.diff_header', 'test');
assert.equal(element._diffHeaderItems.length, 1);
flushAsynchronousOperations();
@@ -1103,13 +948,13 @@
});
test('binary files', () => {
- element._diff.binary = true;
+ element.diff.binary = true;
assert.equal(element._diffHeaderItems.length, 0);
- element.push('_diff.diff_header', 'diff --git a/test.jpg b/test.jpg');
+ element.push('diff.diff_header', 'diff --git a/test.jpg b/test.jpg');
assert.equal(element._diffHeaderItems.length, 0);
- element.push('_diff.diff_header', 'test');
+ element.push('diff.diff_header', 'test');
assert.equal(element._diffHeaderItems.length, 1);
- element.push('_diff.diff_header', 'Binary files differ');
+ element.push('diff.diff_header', 'Binary files differ');
assert.equal(element._diffHeaderItems.length, 1);
});
});
@@ -1120,39 +965,49 @@
setup(() => {
element = fixture('basic');
renderStub = sandbox.stub(element.$.diffBuilder, 'render',
- () => Promise.resolve());
+ () => {
+ Promise.resolve();
+ element.$.diffBuilder.dispatchEvent(
+ new CustomEvent('render', {bubbles: true}));
+ });
const mock = document.createElement('mock-diff-response');
- element._diff = mock.diffResponse;
+ element.diff = mock.diffResponse;
element.comments = {left: [], right: []};
element.noRenderOnPrefsChange = true;
});
- test('lage render w/ context = 10', () => {
+ test('large render w/ context = 10', done => {
element.prefs = {context: 10};
sandbox.stub(element, '_diffLength', () => 10000);
- return element._renderDiffTable().then(() => {
+ element.addEventListener('render', () => {
assert.isTrue(renderStub.called);
assert.isFalse(element._showWarning);
+ done();
});
+ element._renderDiffTable();
});
- test('lage render w/ whole file and bypass', () => {
+ test('large render w/ whole file and bypass', done => {
element.prefs = {context: -1};
element._safetyBypass = 10;
sandbox.stub(element, '_diffLength', () => 10000);
- return element._renderDiffTable().then(() => {
+ element.addEventListener('render', () => {
assert.isTrue(renderStub.called);
assert.isFalse(element._showWarning);
+ done();
});
+ element._renderDiffTable();
});
- test('lage render w/ whole file and no bypass', () => {
+ test('large render w/ whole file and no bypass', done => {
element.prefs = {context: -1};
sandbox.stub(element, '_diffLength', () => 10000);
- return element._renderDiffTable().then(() => {
+ element.addEventListener('render', () => {
assert.isFalse(renderStub.called);
assert.isTrue(element._showWarning);
+ done();
});
+ element._renderDiffTable();
});
});
@@ -1161,144 +1016,19 @@
element = fixture('basic');
});
- test('clearBlame', () => {
- element._blame = [];
+ test('unsetting', () => {
+ element.blame = [];
const setBlameSpy = sandbox.spy(element.$.diffBuilder, 'setBlame');
element.classList.add('showBlame');
- element.clearBlame();
- assert.isNull(element._blame);
+ element.blame = null;
assert.isTrue(setBlameSpy.calledWithExactly(null));
assert.isFalse(element.classList.contains('showBlame'));
});
- test('loadBlame', () => {
+ test('setting', () => {
const mockBlame = [{id: 'commit id', ranges: [{start: 1, end: 2}]}];
- const showAlertStub = sinon.stub();
- element.addEventListener('show-alert', showAlertStub);
- const getBlameStub = sandbox.stub(element.$.restAPI, 'getBlame')
- .returns(Promise.resolve(mockBlame));
- element.changeNum = 42;
- element.patchRange = {patchNum: 5, basePatchNum: 4};
- element.path = 'foo/bar.baz';
- return element.loadBlame().then(() => {
- assert.isTrue(getBlameStub.calledWithExactly(
- 42, 5, 'foo/bar.baz', true));
- assert.isFalse(showAlertStub.called);
- assert.equal(element._blame, mockBlame);
- assert.isTrue(element.classList.contains('showBlame'));
- });
- });
-
- test('loadBlame empty', () => {
- const mockBlame = [];
- const showAlertStub = sinon.stub();
- element.addEventListener('show-alert', showAlertStub);
- sandbox.stub(element.$.restAPI, 'getBlame')
- .returns(Promise.resolve(mockBlame));
- element.changeNum = 42;
- element.patchRange = {patchNum: 5, basePatchNum: 4};
- element.path = 'foo/bar.baz';
- return element.loadBlame()
- .then(() => {
- assert.isTrue(false, 'Promise should not resolve');
- })
- .catch(() => {
- assert.isTrue(showAlertStub.calledOnce);
- assert.isNull(element._blame);
- assert.isFalse(element.classList.contains('showBlame'));
- });
- });
- });
-
- suite('_reportDiff', () => {
- let reportStub;
-
- setup(() => {
- element = fixture('basic');
- element.patchRange = {basePatchNum: 1};
- reportStub = sandbox.stub(element.$.reporting, 'reportInteraction');
- });
-
- test('null and content-less', () => {
- element._reportDiff(null);
- assert.isFalse(reportStub.called);
-
- element._reportDiff({});
- assert.isFalse(reportStub.called);
- });
-
- test('diff w/ no delta', () => {
- const diff = {
- content: [
- {ab: ['foo', 'bar']},
- {ab: ['baz', 'foo']},
- ],
- };
- element._reportDiff(diff);
- assert.isTrue(reportStub.calledOnce);
- assert.equal(reportStub.lastCall.args[0], 'rebase-percent-zero');
- assert.isUndefined(reportStub.lastCall.args[1]);
- });
-
- test('diff w/ no rebase delta', () => {
- const diff = {
- content: [
- {ab: ['foo', 'bar']},
- {a: ['baz', 'foo']},
- {ab: ['foo', 'bar']},
- {a: ['baz', 'foo'], b: ['bar', 'baz']},
- {ab: ['foo', 'bar']},
- {b: ['baz', 'foo']},
- {ab: ['foo', 'bar']},
- ],
- };
- element._reportDiff(diff);
- assert.isTrue(reportStub.calledOnce);
- assert.equal(reportStub.lastCall.args[0], 'rebase-percent-zero');
- assert.isUndefined(reportStub.lastCall.args[1]);
- });
-
- test('diff w/ some rebase delta', () => {
- const diff = {
- content: [
- {ab: ['foo', 'bar']},
- {a: ['baz', 'foo'], due_to_rebase: true},
- {ab: ['foo', 'bar']},
- {a: ['baz', 'foo'], b: ['bar', 'baz']},
- {ab: ['foo', 'bar']},
- {b: ['baz', 'foo'], due_to_rebase: true},
- {ab: ['foo', 'bar']},
- {a: ['baz', 'foo']},
- ],
- };
- element._reportDiff(diff);
- assert.isTrue(reportStub.calledOnce);
- assert.equal(reportStub.lastCall.args[0], 'rebase-percent-nonzero');
- assert.strictEqual(reportStub.lastCall.args[1], 50);
- });
-
- test('diff w/ all rebase delta', () => {
- const diff = {content: [{
- a: ['foo', 'bar'],
- b: ['baz', 'foo'],
- due_to_rebase: true,
- }]};
- element._reportDiff(diff);
- assert.isTrue(reportStub.calledOnce);
- assert.equal(reportStub.lastCall.args[0], 'rebase-percent-nonzero');
- assert.strictEqual(reportStub.lastCall.args[1], 100);
- });
-
- test('diff against parent event', () => {
- element.patchRange.basePatchNum = 'PARENT';
- const diff = {content: [{
- a: ['foo', 'bar'],
- b: ['baz', 'foo'],
- }]};
- element._reportDiff(diff);
- assert.isTrue(reportStub.calledOnce);
- assert.equal(reportStub.lastCall.args[0], 'diff-against-parent');
- assert.isUndefined(reportStub.lastCall.args[1]);
+ element.blame = mockBlame;
+ assert.isTrue(element.classList.contains('showBlame'));
});
});
diff --git a/polygerrit-ui/app/elements/shared/gr-copy-clipboard/gr-copy-clipboard.html b/polygerrit-ui/app/elements/shared/gr-copy-clipboard/gr-copy-clipboard.html
index 52785ff..32ca557 100644
--- a/polygerrit-ui/app/elements/shared/gr-copy-clipboard/gr-copy-clipboard.html
+++ b/polygerrit-ui/app/elements/shared/gr-copy-clipboard/gr-copy-clipboard.html
@@ -20,7 +20,6 @@
<link rel="import" href="../../../styles/shared-styles.html">
<link rel="import" href="../../shared/gr-button/gr-button.html">
<link rel="import" href="../../shared/gr-icons/gr-icons.html">
-<link rel="import" href="../../shared/gr-rest-api-interface/gr-rest-api-interface.html">
<dom-module id="gr-copy-clipboard">
<template>
@@ -30,15 +29,11 @@
display: flex;
flex-wrap: wrap;
}
- .text label {
- flex: 0 0 100%;
- }
.copyText {
flex-grow: 1;
margin-right: .3em;
}
- .hideInput,
- .hideLabel label {
+ .hideInput {
display: none;
}
input {
@@ -51,8 +46,7 @@
width: 1.2em;
}
</style>
- <div class$="text [[_computeLabelClass(hideLabel)]]">
- <label>[[title]]</label>
+ <div class="text">
<input id="input" is="iron-input"
class$="copyText [[_computeInputClass(hideInput)]]"
type="text"
@@ -67,8 +61,7 @@
on-tap="_copyToClipboard">
<iron-icon id="icon" icon="gr-icons:content-copy"></iron-icon>
</gr-button>
- </div>
- <gr-rest-api-interface id="restAPI"></gr-rest-api-interface>
+ </div>
</template>
<script src="gr-copy-clipboard.js"></script>
</dom-module>
diff --git a/polygerrit-ui/app/elements/shared/gr-copy-clipboard/gr-copy-clipboard.js b/polygerrit-ui/app/elements/shared/gr-copy-clipboard/gr-copy-clipboard.js
index cd8cb00..cabee36 100644
--- a/polygerrit-ui/app/elements/shared/gr-copy-clipboard/gr-copy-clipboard.js
+++ b/polygerrit-ui/app/elements/shared/gr-copy-clipboard/gr-copy-clipboard.js
@@ -24,7 +24,6 @@
properties: {
text: String,
- title: String,
buttonTitle: String,
hasTooltip: {
type: Boolean,
@@ -34,10 +33,6 @@
type: Boolean,
value: false,
},
- hideLabel: {
- type: Boolean,
- value: false,
- },
},
focusOnCopy() {
@@ -48,10 +43,6 @@
return hideInput ? 'hideInput' : '';
},
- _computeLabelClass(hideLabel) {
- return hideLabel ? 'hideLabel' : '';
- },
-
_handleInputTap(e) {
e.preventDefault();
Polymer.dom(e).rootTarget.select();
diff --git a/polygerrit-ui/app/elements/shared/gr-copy-clipboard/gr-copy-clipboard_test.html b/polygerrit-ui/app/elements/shared/gr-copy-clipboard/gr-copy-clipboard_test.html
index c865917..d6e9dca 100644
--- a/polygerrit-ui/app/elements/shared/gr-copy-clipboard/gr-copy-clipboard_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-copy-clipboard/gr-copy-clipboard_test.html
@@ -38,12 +38,8 @@
let sandbox;
setup(() => {
- stub('gr-rest-api-interface', {
- saveChangeStarred() { return Promise.resolve({ok: true}); },
- });
sandbox = sinon.sandbox.create();
element = fixture('basic');
- element.title = 'Checkout';
element.text = `git fetch http://gerrit@localhost:8080/a/test-project
refs/changes/05/5/1 && git checkout FETCH_HEAD`;
flushAsynchronousOperations();
@@ -79,12 +75,5 @@
flushAsynchronousOperations();
assert.equal(getComputedStyle(element.$.input).display, 'none');
});
-
- test('hideLabel', () => {
- assert.notEqual(getComputedStyle(element.$$('label')).display, 'none');
- element.hideLabel = true;
- flushAsynchronousOperations();
- assert.equal(getComputedStyle(element.$$('label')).display, 'none');
- });
});
</script>
diff --git a/polygerrit-ui/app/elements/shared/gr-download-commands/gr-download-commands.html b/polygerrit-ui/app/elements/shared/gr-download-commands/gr-download-commands.html
index 83e99be..bfa7885 100644
--- a/polygerrit-ui/app/elements/shared/gr-download-commands/gr-download-commands.html
+++ b/polygerrit-ui/app/elements/shared/gr-download-commands/gr-download-commands.html
@@ -20,7 +20,7 @@
<link rel="import" href="../../../behaviors/rest-client-behavior/rest-client-behavior.html">
<link rel="import" href="../../../bower_components/paper-tabs/paper-tabs.html">
-<link rel="import" href="../../shared/gr-copy-clipboard/gr-copy-clipboard.html">
+<link rel="import" href="../../shared/gr-shell-command/gr-shell-command.html">
<link rel="import" href="../../shared/gr-rest-api-interface/gr-rest-api-interface.html">
<link rel="import" href="../../../styles/shared-styles.html">
@@ -52,7 +52,7 @@
display: flex;
flex-direction: column;
}
- gr-copy-clipboard {
+ gr-shell-command {
width: 60em;
margin-bottom: .5em;
}
@@ -75,9 +75,9 @@
<template is="dom-repeat"
items="[[commands]]"
as="command">
- <gr-copy-clipboard
- title=[[command.title]]
- text=[[command.command]]></gr-copy-clipboard>
+ <gr-shell-command
+ label=[[command.title]]
+ command=[[command.command]]></gr-shell-command>
</template>
</div>
<gr-rest-api-interface id="restAPI"></gr-rest-api-interface>
diff --git a/polygerrit-ui/app/elements/shared/gr-download-commands/gr-download-commands.js b/polygerrit-ui/app/elements/shared/gr-download-commands/gr-download-commands.js
index 319cd04..ca77a30 100644
--- a/polygerrit-ui/app/elements/shared/gr-download-commands/gr-download-commands.js
+++ b/polygerrit-ui/app/elements/shared/gr-download-commands/gr-download-commands.js
@@ -44,7 +44,7 @@
},
focusOnCopy() {
- this.$$('gr-copy-clipboard').focusOnCopy();
+ this.$$('gr-shell-command').focusOnCopy();
},
_getLoggedIn() {
diff --git a/polygerrit-ui/app/elements/shared/gr-download-commands/gr-download-commands_test.html b/polygerrit-ui/app/elements/shared/gr-download-commands/gr-download-commands_test.html
index 47219a7..c59e56a 100644
--- a/polygerrit-ui/app/elements/shared/gr-download-commands/gr-download-commands_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-download-commands/gr-download-commands_test.html
@@ -74,7 +74,7 @@
});
test('focusOnCopy', () => {
- const focusStub = sandbox.stub(element.$$('gr-copy-clipboard'),
+ const focusStub = sandbox.stub(element.$$('gr-shell-command'),
'focusOnCopy');
element.focusOnCopy();
assert.isTrue(focusStub.called);
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-rest-api.js b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-rest-api.js
index 57cbc85..c18f753 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-rest-api.js
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-rest-api.js
@@ -43,10 +43,14 @@
* @param {string} method HTTP Method (GET, POST, etc)
* @param {string} url URL without base path or plugin prefix
* @param {Object=} payload Respected for POST and PUT only.
+ * @param {?function(?Response, string=)=} opt_errFn
+ * passed as null sometimes.
* @return {!Promise}
*/
- GrPluginRestApi.prototype.fetch = function(method, url, opt_payload) {
- return getRestApi().send(method, this.opt_prefix + url, opt_payload);
+ GrPluginRestApi.prototype.fetch = function(method, url, opt_payload,
+ opt_errFn) {
+ return getRestApi().send(method, this.opt_prefix + url, opt_payload,
+ opt_errFn);
};
/**
@@ -54,10 +58,13 @@
* @param {string} method HTTP Method (GET, POST, etc)
* @param {string} url URL without base path or plugin prefix
* @param {Object=} payload Respected for POST and PUT only.
+ * @param {?function(?Response, string=)=} opt_errFn
+ * passed as null sometimes.
* @return {!Promise} resolves on success, rejects on error.
*/
- GrPluginRestApi.prototype.send = function(method, url, opt_payload) {
- return this.fetch(method, url, opt_payload).then(response => {
+ GrPluginRestApi.prototype.send = function(method, url, opt_payload,
+ opt_errFn) {
+ return this.fetch(method, url, opt_payload, opt_errFn).then(response => {
if (response.status < 200 || response.status >= 300) {
return response.text().then(text => {
if (text) {
diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
index 8d714eb..84c1803 100644
--- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
+++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
@@ -1497,13 +1497,20 @@
* @return {!Promise<?Object>}
*/
getRepos(filter, reposPerPage, opt_offset) {
+ const defaultFilter = 'state:active OR state:read-only';
const offset = opt_offset || 0;
+ if (!filter) {
+ filter = defaultFilter;
+ }
+ filter = filter.trim();
+ const encodedFilter = encodeURIComponent(filter);
+
// TODO(kaspern): Rename rest api from /projects/ to /repos/ once backend
// supports it.
return this._fetchSharedCacheURL({
- url: `/projects/?d&n=${reposPerPage + 1}&S=${offset}` +
- this._computeFilter(filter),
+ url: `/projects/?n=${reposPerPage + 1}&S=${offset}` +
+ `&query=${encodedFilter}`,
anonymizedUrl: '/projects/?*',
});
},
diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.html b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.html
index 193d306..b7466ef 100644
--- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.html
@@ -930,33 +930,38 @@
});
});
- test('getRepos', () => {
- sandbox.stub(element, '_fetchSharedCacheURL');
- element.getRepos('test', 25);
- assert.equal(element._fetchSharedCacheURL.lastCall.args[0].url,
- '/projects/?d&n=26&S=0&m=test');
+ suite('getRepos', () => {
+ const defaultQuery = 'state%3Aactive%20OR%20state%3Aread-only';
- element.getRepos(null, 25);
- assert.equal(element._fetchSharedCacheURL.lastCall.args[0].url,
- '/projects/?d&n=26&S=0');
+ setup(() => {
+ sandbox.stub(element, '_fetchSharedCacheURL');
+ });
- element.getRepos('test', 25, 25);
- assert.equal(element._fetchSharedCacheURL.lastCall.args[0].url,
- '/projects/?d&n=26&S=25&m=test');
- });
+ test('normal use', () => {
+ element.getRepos('test', 25);
+ assert.equal(element._fetchSharedCacheURL.lastCall.args[0].url,
+ '/projects/?n=26&S=0&query=test');
- test('getRepos filter', () => {
- sandbox.stub(element, '_fetchSharedCacheURL');
- element.getRepos('test/test/test', 25);
- assert.equal(element._fetchSharedCacheURL.lastCall.args[0].url,
- '/projects/?d&n=26&S=0&m=test%2Ftest%2Ftest');
- });
+ element.getRepos(null, 25);
+ assert.equal(element._fetchSharedCacheURL.lastCall.args[0].url,
+ `/projects/?n=26&S=0&query=${defaultQuery}`);
- test('getRepos filter regex', () => {
- sandbox.stub(element, '_fetchSharedCacheURL');
- element.getRepos('^test.*', 25);
- assert.equal(element._fetchSharedCacheURL.lastCall.args[0].url,
- '/projects/?d&n=26&S=0&r=%5Etest.*');
+ element.getRepos('test', 25, 25);
+ assert.equal(element._fetchSharedCacheURL.lastCall.args[0].url,
+ '/projects/?n=26&S=25&query=test');
+ });
+
+ test('with filter', () => {
+ element.getRepos('test/test/test', 25);
+ assert.equal(element._fetchSharedCacheURL.lastCall.args[0].url,
+ '/projects/?n=26&S=0&query=test%2Ftest%2Ftest');
+ });
+
+ test('with regex filter', () => {
+ element.getRepos('^test.*', 25);
+ assert.equal(element._fetchSharedCacheURL.lastCall.args[0].url,
+ '/projects/?n=26&S=0&query=%5Etest.*');
+ });
});
test('getGroups filter regex', () => {
diff --git a/polygerrit-ui/app/elements/shared/gr-shell-command/gr-shell-command.html b/polygerrit-ui/app/elements/shared/gr-shell-command/gr-shell-command.html
new file mode 100644
index 0000000..fe6ed88
--- /dev/null
+++ b/polygerrit-ui/app/elements/shared/gr-shell-command/gr-shell-command.html
@@ -0,0 +1,58 @@
+<!--
+@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.
+-->
+
+<link rel="import" href="../../../bower_components/polymer/polymer.html">
+<link rel="import" href="../../../styles/shared-styles.html">
+<link rel="import" href="../../shared/gr-copy-clipboard/gr-copy-clipboard.html">
+
+<dom-module id="gr-shell-command">
+ <template>
+ <style include="shared-styles">
+ .commandContainer {
+ margin-bottom: .75em;
+ }
+ .commandContainer {
+ background-color: var(--shell-command-background-color);
+ padding: .5em .5em .5em 2.5em;
+ position: relative;
+ width: 100%;
+ }
+ .commandContainer:before {
+ background: var(--shell-command-decoration-background-color);
+ bottom: 0;
+ box-sizing: border-box;
+ content: '$';
+ display: block;
+ left: 0;
+ padding: .8em;
+ position: absolute;
+ top: 0;
+ width: 2em;
+ }
+ .commandContainer gr-copy-clipboard {
+ --text-container-style: {
+ border: none;
+ }
+ }
+ </style>
+ <label>[[label]]</label>
+ <div class="commandContainer">
+ <gr-copy-clipboard text="[[command]]"></gr-copy-clipboard>
+ </div>
+ </template>
+ <script src="gr-shell-command.js"></script>
+</dom-module>
diff --git a/polygerrit-ui/app/elements/shared/gr-shell-command/gr-shell-command.js b/polygerrit-ui/app/elements/shared/gr-shell-command/gr-shell-command.js
new file mode 100644
index 0000000..2c546cc
--- /dev/null
+++ b/polygerrit-ui/app/elements/shared/gr-shell-command/gr-shell-command.js
@@ -0,0 +1,32 @@
+/**
+ * @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.
+ */
+(function() {
+ 'use strict';
+
+ Polymer({
+ is: 'gr-shell-command',
+
+ properties: {
+ command: String,
+ label: String,
+ },
+
+ focusOnCopy() {
+ this.$$('gr-copy-clipboard').focusOnCopy();
+ },
+ });
+})();
diff --git a/polygerrit-ui/app/elements/shared/gr-shell-command/gr-shell-command_test.html b/polygerrit-ui/app/elements/shared/gr-shell-command/gr-shell-command_test.html
new file mode 100644
index 0000000..a49f76f
--- /dev/null
+++ b/polygerrit-ui/app/elements/shared/gr-shell-command/gr-shell-command_test.html
@@ -0,0 +1,59 @@
+<!DOCTYPE html>
+<!--
+@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.
+-->
+
+<meta name="viewport" content="width=device-width, minimum-scale=1.0, initial-scale=1.0, user-scalable=yes">
+<title>gr-shell-command</title>
+
+<script src="../../../bower_components/webcomponentsjs/webcomponents-lite.min.js"></script>
+<script src="../../../bower_components/web-component-tester/browser.js"></script>
+<link rel="import" href="../../../test/common-test-setup.html"/>
+<link rel="import" href="gr-shell-command.html">
+
+<script>void(0);</script>
+
+<test-fixture id="basic">
+ <template>
+ <gr-shell-command></gr-shell-command>
+ </template>
+</test-fixture>
+
+<script>
+ suite('gr-shell-command tests', () => {
+ let element;
+ let sandbox;
+
+ setup(() => {
+ sandbox = sinon.sandbox.create();
+ element = fixture('basic');
+ element.text = `git fetch http://gerrit@localhost:8080/a/test-project
+ refs/changes/05/5/1 && git checkout FETCH_HEAD`;
+ flushAsynchronousOperations();
+ });
+
+ teardown(() => {
+ sandbox.restore();
+ });
+
+ test('focusOnCopy', () => {
+ const focusStub = sandbox.stub(element.$$('gr-copy-clipboard'),
+ 'focusOnCopy');
+ element.focusOnCopy();
+ assert.isTrue(focusStub.called);
+ });
+ });
+</script>
diff --git a/polygerrit-ui/app/styles/themes/app-theme.html b/polygerrit-ui/app/styles/themes/app-theme.html
index 81c195a..ea81796 100644
--- a/polygerrit-ui/app/styles/themes/app-theme.html
+++ b/polygerrit-ui/app/styles/themes/app-theme.html
@@ -96,6 +96,9 @@
--diff-highlight-range-color: rgba(255, 213, 0, 0.5);
--diff-highlight-range-hover-color: rgba(255, 255, 0, 0.5);
+ --shell-command-background-color: #f5f5f5;
+ --shell-command-decoration-background-color: #ebebeb;
+
--comment-text-color: #000;
--comment-background-color: #fcfad6;
--unresolved-comment-background-color: #fcfaa6;
diff --git a/polygerrit-ui/app/styles/themes/dark-theme.html b/polygerrit-ui/app/styles/themes/dark-theme.html
index 1f473da..8ade9ba 100644
--- a/polygerrit-ui/app/styles/themes/dark-theme.html
+++ b/polygerrit-ui/app/styles/themes/dark-theme.html
@@ -29,6 +29,7 @@
--diff-selection-background-color: #3A71D8;
--light-remove-highlight-color: rgb(53, 27, 27);
--light-add-highlight-color: rgb(24, 45, 24);
+ --light-remove-add-highlight-color: #2f3f2f;
--light-rebased-remove-highlight-color: rgb(60, 37, 8);
--light-rebased-add-highlight-color: rgb(72, 113, 101);
--dark-remove-highlight-color: rgba(255, 0, 0, 0.15);
@@ -39,6 +40,8 @@
--diff-context-control-border-color: var(--border-color);
--diff-highlight-range-color: rgba(0, 100, 200, 0.5);
--diff-highlight-range-hover-color: rgba(0, 150, 255, 0.5);
+ --shell-command-background-color: #5f5f5f;
+ --shell-command-decoration-background-color: #999999;
--comment-text-color: var(--primary-text-color);
--comment-background-color: #0B162B;
--unresolved-comment-background-color: rgb(56, 90, 154);
diff --git a/resources/com/google/gerrit/pgm/init/gerrit.sh b/resources/com/google/gerrit/pgm/init/gerrit.sh
index 5571e7c..d3f3666 100755
--- a/resources/com/google/gerrit/pgm/init/gerrit.sh
+++ b/resources/com/google/gerrit/pgm/init/gerrit.sh
@@ -443,6 +443,11 @@
echo -16 > "/proc/${PID}/oom_adj"
fi
fi
+ elif [ "$(uname -s)"=="Linux" ] && test -d "/proc/${PID}"; then
+ echo "WARNING: Could not adjust Gerrit's process for the kernel's out-of-memory killer."
+ echo " This may be caused by ${0} not being run as root."
+ echo " Consider changing the OOM score adjustment manually for Gerrit's PID=${PID} with e.g.:"
+ echo " echo '-1000' | sudo tee /proc/${PID}/oom_score_adj"
fi
TIMEOUT="$GERRIT_STARTUP_TIMEOUT"
