Merge "Wiki-like formatting for PolyGerrit comments"
diff --git a/Documentation/config-labels.txt b/Documentation/config-labels.txt
index 1f9dd33..5a82d5a 100644
--- a/Documentation/config-labels.txt
+++ b/Documentation/config-labels.txt
@@ -230,6 +230,19 @@
Allowed range of values are 0 (Patch Set Unlocked) to 1 (Patch Set
Locked).
+[[label_allowPostSubmit]]
+=== `label.Label-Name.allowPostSubmit`
+
+If true, the label may be voted on for changes that have already been
+submitted. If false, the label will not appear in the UI and will not
+be accepted when reviewing a closed change.
+
+In either case, voting on a label after submission is only permitted if
+the new vote is at least as high as the old vote by that user. This
+avoids creating the false impression that a post-submit vote can change
+the past and affect submission somehow.
+
+Defaults to true.
[[label_copyMinScore]]
=== `label.Label-Name.copyMinScore`
diff --git a/Documentation/database-setup.txt b/Documentation/database-setup.txt
index 08d0a54..0f73bd3 100644
--- a/Documentation/database-setup.txt
+++ b/Documentation/database-setup.txt
@@ -25,12 +25,13 @@
database as backend so no set up or configuration is necessary.
Currently only support for embedded mode is added. There are two other
-deployment options for Apache Derby that can be added later [1]:
-+
-* Derby Network Server (standalone mode)
-* Embedded Server (hybrid mode)
-+
-[1] http://db.apache.org/derby/papers/DerbyTut/ns_intro.html#ns
+deployment options for Apache Derby that can be added later:
+
+* link:http://db.apache.org/derby/papers/DerbyTut/ns_intro.html#Network+Server+Options[
+Derby Network Server (standalone mode)]
+
+* link:http://db.apache.org/derby/papers/DerbyTut/ns_intro.html#Embedded+Server[
+Embedded Server (hybrid mode)]
[[createdb_postgres]]
=== PostgreSQL
diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt
index 11b2729..3137e16 100644
--- a/Documentation/rest-api-changes.txt
+++ b/Documentation/rest-api-changes.txt
@@ -2792,6 +2792,64 @@
Adding query parameter `links` (for example `/changes/.../commit?links`)
returns a link:#commit-info[CommitInfo] with the additional field `web_links`.
+[[get-description]]
+=== Get Description
+--
+'GET /changes/link:#change-id[\{change-id\}]/revisions/link:#revision-id[\{revision-id\}]/description'
+--
+
+Retrieves the description of a patch set.
+
+.Request
+----
+ GET /changes/myProject~master~I8473b95934b5732ac55d26311a706c9c2bde9940/revisions/674ac754f91e64a0efb8087e59a176484bd534d1/description HTTP/1.0
+----
+
+.Response
+----
+ HTTP/1.1 200 OK
+ Content-Disposition: attachment
+ Content-Type: application/json; charset=UTF-8
+
+ )]}'
+ "Added Documentation"
+----
+
+If the patch set does not have a description an empty string is returned.
+
+[[set-description]]
+=== Set Description
+--
+'PUT /changes/link:#change-id[\{change-id\}]/revisions/link:#revision-id[\{revision-id\}]/description'
+--
+
+Sets the description of a patch set.
+
+The new description must be provided in the request body inside a
+link:#description-input[DescriptionInput] entity.
+
+.Request
+----
+ PUT /changes/myProject~master~I8473b95934b5732ac55d26311a706c9c2bde9940/revisions/674ac754f91e64a0efb8087e59a176484bd534d1/description HTTP/1.0
+ Content-Type: application/json; charset=UTF-8
+
+ {
+ "description": "Added Documentation"
+ }
+----
+
+As response the new description is returned.
+
+.Response
+----
+ HTTP/1.1 200 OK
+ Content-Disposition: attachment
+ Content-Type: application/json; charset=UTF-8
+
+ )]}'
+ "Added Documentation"
+----
+
[[get-merge-list]]
=== Get Merge List
--
@@ -5097,6 +5155,16 @@
If not set, the default is `ALL`.
|=======================
+[[description-input]]
+=== DescriptionInput
+The `DescriptionInput` entity contains information for setting a description.
+
+[options="header",cols="1,6"]
+|===========================
+|Field Name |Description
+|`description` |The description text.
+|===========================
+
[[diff-content]]
=== DiffContent
The `DiffContent` entity contains information about the content differences
diff --git a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
index 1910049..9cc687b 100644
--- a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
+++ b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
@@ -20,6 +20,7 @@
import static com.google.gerrit.reviewdb.client.Patch.COMMIT_MSG;
import static com.google.gerrit.reviewdb.client.Patch.MERGE_LIST;
import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
+import static java.util.stream.Collectors.toList;
import static org.eclipse.jgit.lib.Constants.HEAD;
import com.google.common.base.Strings;
@@ -30,6 +31,7 @@
import com.google.common.collect.Sets;
import com.google.common.primitives.Chars;
import com.google.gerrit.acceptance.AcceptanceTestRequestScope.Context;
+import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.data.AccessSection;
import com.google.gerrit.common.data.ContributorAgreement;
import com.google.gerrit.common.data.GroupReference;
@@ -138,6 +140,7 @@
import java.io.InputStream;
import java.util.ArrayList;
import java.util.Arrays;
+import java.util.Collection;
import java.util.Collections;
import java.util.EnumSet;
import java.util.HashMap;
@@ -1138,4 +1141,27 @@
assertThat(contentEntry.editB).isNull();
assertThat(contentEntry.skip).isNull();
}
+
+ protected TestRepository<?> createProjectWithPush(String name,
+ @Nullable Project.NameKey parent,
+ SubmitType submitType) throws Exception {
+ Project.NameKey project = createProject(name, parent, true, submitType);
+ grant(Permission.PUSH, project, "refs/heads/*");
+ grant(Permission.SUBMIT, project, "refs/for/refs/heads/*");
+ return cloneProject(project);
+ }
+
+ protected void assertPermitted(ChangeInfo info, String label,
+ Integer... expected) {
+ assertThat(info.permittedLabels).isNotNull();
+ Collection<String> strs = info.permittedLabels.get(label);
+ if (expected.length == 0) {
+ assertThat(strs).isNull();
+ } else {
+ assertThat(
+ strs.stream().map(s -> Integer.valueOf(s.trim()))
+ .collect(toList()))
+ .containsExactlyElementsIn(Arrays.asList(expected));
+ }
+ }
}
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java
index 234d2fa..e7557fd 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -2304,6 +2304,8 @@
.containsExactly("Code-Review", "Verified");
assertThat(change.permittedLabels.keySet())
.containsExactly("Code-Review", "Verified");
+ assertPermitted(change, "Code-Review", -2, -1, 0, 1, 2);
+ assertPermitted(change, "Verified", -1, 0, 1);
// add an approval on the new label
gApi.changes()
@@ -2344,6 +2346,7 @@
assertThat(change.status).isEqualTo(ChangeStatus.MERGED);
assertThat(change.labels.keySet()).containsExactly("Code-Review");
assertThat(change.permittedLabels.keySet()).containsExactly("Code-Review");
+ assertPermitted(change, "Code-Review", 2);
// add new label and assert that it's returned for existing changes
ProjectConfig cfg = projectCache.checkedGet(project).getConfig();
@@ -2363,6 +2366,8 @@
.containsExactly("Code-Review", "Verified");
assertThat(change.permittedLabels.keySet())
.containsExactly("Code-Review", "Verified");
+ assertPermitted(change, "Code-Review", 2);
+ assertPermitted(change, "Verified", 0, 1);
// ignore the new label by Prolog submit rule and assert that the label is
// no longer returned
@@ -2378,8 +2383,8 @@
change = gApi.changes()
.id(r.getChangeId())
.get();
- assertThat(change.labels.keySet()).containsExactly("Code-Review");
- assertThat(change.permittedLabels.keySet()).containsExactly("Code-Review");
+ assertPermitted(change, "Code-Review", 2);
+ assertPermitted(change, "Verified");
// add an approval on the new label and assert that the label is now
// returned although it is ignored by the Prolog submit rule and hence not
@@ -2395,7 +2400,8 @@
.get();
assertThat(change.labels.keySet())
.containsExactly("Code-Review", "Verified");
- assertThat(change.permittedLabels.keySet()).containsExactly("Code-Review");
+ assertPermitted(change, "Code-Review", 2);
+ assertPermitted(change, "Verified");
// remove label and assert that it's no longer returned for existing
// changes, even if there is an approval for it
@@ -2410,6 +2416,7 @@
.get();
assertThat(change.labels.keySet()).containsExactly("Code-Review");
assertThat(change.permittedLabels.keySet()).containsExactly("Code-Review");
+ assertPermitted(change, "Code-Review", 2);
}
@Test
@@ -2425,7 +2432,7 @@
.get();
assertThat(change.status).isEqualTo(ChangeStatus.MERGED);
assertThat(change.labels.keySet()).containsExactly("Code-Review");
- assertThat(change.permittedLabels.keySet()).containsExactly("Code-Review");
+ assertPermitted(change, "Code-Review", 0, 1, 2);
}
@Test
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/revision/RevisionIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/revision/RevisionIT.java
index 2925c1d..ab4daf2 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/revision/RevisionIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/revision/RevisionIT.java
@@ -20,6 +20,7 @@
import static com.google.gerrit.acceptance.PushOneCommit.PATCH;
import static com.google.gerrit.acceptance.PushOneCommit.PATCH_FILE_ONLY;
import static com.google.gerrit.acceptance.PushOneCommit.SUBJECT;
+import static com.google.gerrit.extensions.client.ListChangesOption.DETAILED_LABELS;
import static com.google.gerrit.reviewdb.client.Patch.COMMIT_MSG;
import static com.google.gerrit.reviewdb.client.Patch.MERGE_LIST;
import static java.nio.charset.StandardCharsets.UTF_8;
@@ -44,7 +45,6 @@
import com.google.gerrit.extensions.api.changes.RevisionApi;
import com.google.gerrit.extensions.api.projects.BranchInput;
import com.google.gerrit.extensions.client.ChangeStatus;
-import com.google.gerrit.extensions.client.ListChangesOption;
import com.google.gerrit.extensions.client.SubmitType;
import com.google.gerrit.extensions.common.ApprovalInfo;
import com.google.gerrit.extensions.common.ChangeInfo;
@@ -87,6 +87,7 @@
import java.util.List;
import java.util.Locale;
import java.util.Map;
+import java.util.Optional;
public class RevisionIT extends AbstractDaemonTest {
@@ -166,6 +167,9 @@
approval = getApproval(changeId, label);
assertThat(approval.value).isEqualTo(1);
assertThat(approval.postSubmit).isNull();
+ assertPermitted(
+ gApi.changes().id(changeId).get(EnumSet.of(DETAILED_LABELS)),
+ "Code-Review", 1, 2);
// Repeating the current label is allowed. Does not flip the postSubmit bit
// due to deduplication codepath.
@@ -200,6 +204,9 @@
approval = getApproval(changeId, label);
assertThat(approval.value).isEqualTo(2);
assertThat(approval.postSubmit).isTrue();
+ assertPermitted(
+ gApi.changes().id(changeId).get(EnumSet.of(DETAILED_LABELS)),
+ "Code-Review", 2);
// Decreasing to previous post-submit vote is still not allowed.
try {
@@ -217,6 +224,53 @@
assertThat(approval.postSubmit).isTrue();
}
+ @Test
+ public void postSubmitApprovalAfterVoteRemoved() throws Exception {
+ PushOneCommit.Result r = createChange();
+ String changeId = project.get() + "~master~" + r.getChangeId();
+
+ setApiUser(admin);
+ revision(r).review(ReviewInput.approve());
+
+ setApiUser(user);
+ revision(r).review(ReviewInput.recommend());
+
+ setApiUser(admin);
+ gApi.changes()
+ .id(changeId)
+ .reviewer(user.username)
+ .deleteVote("Code-Review");
+ Optional<ApprovalInfo> crUser = get(changeId, DETAILED_LABELS)
+ .labels.get("Code-Review").all.stream()
+ .filter(a -> a._accountId == user.id.get()).findFirst();
+ assertThat(crUser.isPresent()).isTrue();
+ assertThat(crUser.get().value).isEqualTo(0);
+
+ revision(r).submit();
+
+ setApiUser(user);
+ ReviewInput in = new ReviewInput();
+ in.label("Code-Review", 0);
+ in.message = "Still LGTM";
+ revision(r).review(in);
+ }
+
+ @Test
+ public void postSubmitDeleteApprovalNotAllowed() throws Exception {
+ PushOneCommit.Result r = createChange();
+
+ revision(r).review(ReviewInput.approve());
+ revision(r).submit();
+
+ ReviewInput in = new ReviewInput();
+ in.label("Code-Review", 0);
+
+ exception.expect(ResourceConflictException.class);
+ exception.expectMessage(
+ "Cannot reduce vote on labels for closed change: Code-Review");
+ revision(r).review(in);
+ }
+
@TestProjectInput(submitType = SubmitType.CHERRY_PICK)
@Test
public void approvalCopiedDuringSubmitIsNotPostSubmit() throws Exception {
@@ -771,6 +825,31 @@
}
@Test
+ public void description() throws Exception {
+ PushOneCommit.Result r = createChange();
+ assertThat(gApi.changes()
+ .id(r.getChangeId())
+ .revision(r.getCommit().name())
+ .description()).isEqualTo("");
+ gApi.changes()
+ .id(r.getChangeId())
+ .revision(r.getCommit().name())
+ .description("test");
+ assertThat(gApi.changes()
+ .id(r.getChangeId())
+ .revision(r.getCommit().name())
+ .description()).isEqualTo("test");
+ gApi.changes()
+ .id(r.getChangeId())
+ .revision(r.getCommit().name())
+ .description("");
+ assertThat(gApi.changes()
+ .id(r.getChangeId())
+ .revision(r.getCommit().name())
+ .description()).isEqualTo("");
+ }
+
+ @Test
public void content() throws Exception {
PushOneCommit.Result r = createChange();
assertContent(r, FILE_NAME, FILE_CONTENT);
@@ -946,11 +1025,11 @@
public void actions() throws Exception {
PushOneCommit.Result r = createChange();
assertThat(current(r).actions().keySet())
- .containsExactly("cherrypick", "rebase");
+ .containsExactly("cherrypick", "description", "rebase");
current(r).review(ReviewInput.approve());
assertThat(current(r).actions().keySet())
- .containsExactly("submit", "cherrypick", "rebase");
+ .containsExactly("submit", "cherrypick", "description", "rebase");
current(r).submit();
assertThat(current(r).actions().keySet())
@@ -1090,7 +1169,7 @@
throws Exception {
ChangeInfo info = gApi.changes()
.id(changeId)
- .get(EnumSet.of(ListChangesOption.DETAILED_LABELS));
+ .get(EnumSet.of(DETAILED_LABELS));
LabelInfo li = info.labels.get(label);
assertThat(li).isNotNull();
int accountId = atrScope.get().getUser().getAccountId().get();
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java
index 9927c15..cfdc226 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java
@@ -26,6 +26,7 @@
import static java.util.concurrent.TimeUnit.SECONDS;
import static org.junit.Assert.fail;
+import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterables;
import com.google.common.collect.Lists;
import com.google.gerrit.acceptance.AbstractDaemonTest;
@@ -34,6 +35,7 @@
import com.google.gerrit.acceptance.TestProjectInput;
import com.google.gerrit.common.data.Permission;
import com.google.gerrit.extensions.api.changes.SubmitInput;
+import com.google.gerrit.extensions.api.projects.BranchInput;
import com.google.gerrit.extensions.api.projects.ProjectInput;
import com.google.gerrit.extensions.client.ChangeStatus;
import com.google.gerrit.extensions.client.InheritableBoolean;
@@ -81,6 +83,7 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
+import java.util.stream.Collectors;
@NoHttpd
public abstract class AbstractSubmit extends AbstractDaemonTest {
@@ -347,26 +350,126 @@
}
@Test
+ public void submitWholeTopicMultipleProjects() throws Exception {
+ assume().that(isSubmitWholeTopicEnabled()).isTrue();
+ String topic = "test-topic";
+
+ // Create test projects
+ TestRepository<?> repoA = createProjectWithPush(
+ "project-a", null, getSubmitType());
+ TestRepository<?> repoB = createProjectWithPush(
+ "project-b", null, getSubmitType());
+
+ // Create changes on project-a
+ PushOneCommit.Result change1 =
+ createChange(repoA, "master", "Change 1", "a.txt", "content", topic);
+ PushOneCommit.Result change2 =
+ createChange(repoA, "master", "Change 2", "b.txt", "content", topic);
+
+ // Create changes on project-b
+ PushOneCommit.Result change3 =
+ createChange(repoB, "master", "Change 3", "a.txt", "content", topic);
+ PushOneCommit.Result change4 =
+ createChange(repoB, "master", "Change 4", "b.txt", "content", topic);
+
+ approve(change1.getChangeId());
+ approve(change2.getChangeId());
+ approve(change3.getChangeId());
+ approve(change4.getChangeId());
+ submit(change4.getChangeId());
+
+ String expectedTopic = name(topic);
+ change1.assertChange(Change.Status.MERGED, expectedTopic, admin);
+ change2.assertChange(Change.Status.MERGED, expectedTopic, admin);
+ change3.assertChange(Change.Status.MERGED, expectedTopic, admin);
+ change4.assertChange(Change.Status.MERGED, expectedTopic, admin);
+ }
+
+ @Test
+ public void submitWholeTopicMultipleBranchesOnSameProject() throws Exception {
+ assume().that(isSubmitWholeTopicEnabled()).isTrue();
+ String topic = "test-topic";
+
+ // Create test project
+ String projectName = "project-a";
+ TestRepository<?> repoA = createProjectWithPush(
+ projectName, null, getSubmitType());
+
+ RevCommit initialHead =
+ getRemoteHead(new Project.NameKey(name(projectName)), "master");
+
+ // Create the dev branch on the test project
+ BranchInput in = new BranchInput();
+ in.revision = initialHead.name();
+ gApi.projects().name(name(projectName)).branch("dev").create(in);
+
+ // Create changes on master
+ PushOneCommit.Result change1 =
+ createChange(repoA, "master", "Change 1", "a.txt", "content", topic);
+ PushOneCommit.Result change2 =
+ createChange(repoA, "master", "Change 2", "b.txt", "content", topic);
+
+ // Create changes on dev
+ repoA.reset(initialHead);
+ PushOneCommit.Result change3 =
+ createChange(repoA, "dev", "Change 3", "a.txt", "content", topic);
+ PushOneCommit.Result change4 =
+ createChange(repoA, "dev", "Change 4", "b.txt", "content", topic);
+
+ approve(change1.getChangeId());
+ approve(change2.getChangeId());
+ approve(change3.getChangeId());
+ approve(change4.getChangeId());
+ submit(change4.getChangeId());
+
+ String expectedTopic = name(topic);
+ change1.assertChange(Change.Status.MERGED, expectedTopic, admin);
+ change2.assertChange(Change.Status.MERGED, expectedTopic, admin);
+ change3.assertChange(Change.Status.MERGED, expectedTopic, admin);
+ change4.assertChange(Change.Status.MERGED, expectedTopic, admin);
+ }
+
+ @Test
public void submitWholeTopic() throws Exception {
assume().that(isSubmitWholeTopicEnabled()).isTrue();
+ String topic = "test-topic";
PushOneCommit.Result change1 =
- createChange("Change 1", "a.txt", "content", "test-topic");
+ createChange("Change 1", "a.txt", "content", topic);
PushOneCommit.Result change2 =
- createChange("Change 2", "b.txt", "content", "test-topic");
+ createChange("Change 2", "b.txt", "content", topic);
PushOneCommit.Result change3 =
- createChange("Change 3", "c.txt", "content", "test-topic");
+ createChange("Change 3", "c.txt", "content", topic);
approve(change1.getChangeId());
approve(change2.getChangeId());
approve(change3.getChangeId());
submit(change3.getChangeId());
- change1.assertChange(Change.Status.MERGED, name("test-topic"), admin);
- change2.assertChange(Change.Status.MERGED, name("test-topic"), admin);
- change3.assertChange(Change.Status.MERGED, name("test-topic"), admin);
+ String expectedTopic = name(topic);
+ change1.assertChange(Change.Status.MERGED, expectedTopic, admin);
+ change2.assertChange(Change.Status.MERGED, expectedTopic, admin);
+ change3.assertChange(Change.Status.MERGED, expectedTopic, admin);
+
// Check for the exact change to have the correct submitter.
assertSubmitter(change3);
// Also check submitters for changes submitted via the topic relationship.
assertSubmitter(change1);
assertSubmitter(change2);
+
+ // Check that the repo has the expected commits
+ List<RevCommit> log = getRemoteLog();
+ List<String> commitsInRepo = log.stream()
+ .map(c -> c.getShortMessage())
+ .collect(Collectors.toList());
+ int expectedCommitCount = getSubmitType() == SubmitType.MERGE_ALWAYS
+ ? 5 // initial commit + 3 commits + merge commit
+ : 4; // initial commit + 3 commits
+ assertThat(log).hasSize(expectedCommitCount);
+
+ assertThat(commitsInRepo).containsAllOf(
+ "Initial empty repository", "Change 1", "Change 2", "Change 3");
+ if (getSubmitType() == SubmitType.MERGE_ALWAYS) {
+ assertThat(commitsInRepo).contains(
+ "Merge changes from topic '" + expectedTopic + "'");
+ }
}
@Test
@@ -407,6 +510,66 @@
"A change to be submitted with " + num + " is not visible");
}
+ @Test
+ public void submitChangeWhenParentOfOtherBranchTip() throws Exception {
+ // Chain of two commits
+ // Push both to topic-branch
+ // Push the first commit for review and submit
+ //
+ // C2 -- tip of topic branch
+ // |
+ // C1 -- pushed for review
+ // |
+ // C0 -- Master
+ //
+ ProjectConfig config = projectCache.checkedGet(project).getConfig();
+ config.getProject().setCreateNewChangeForAllNotInTarget(
+ InheritableBoolean.TRUE);
+ saveProjectConfig(project, config);
+
+ PushOneCommit push1 = pushFactory.create(db, admin.getIdent(), testRepo,
+ PushOneCommit.SUBJECT, "a.txt", "content");
+ PushOneCommit.Result c1 = push1.to("refs/heads/topic");
+ c1.assertOkStatus();
+ PushOneCommit push2 = pushFactory.create(db, admin.getIdent(), testRepo,
+ PushOneCommit.SUBJECT, "b.txt", "anotherContent");
+ PushOneCommit.Result c2 = push2.to("refs/heads/topic");
+ c2.assertOkStatus();
+
+ PushOneCommit.Result change1 = push1.to("refs/for/master");
+ change1.assertOkStatus();
+
+ approve(change1.getChangeId());
+ submit(change1.getChangeId());
+ }
+
+ @Test
+ public void submitMergeOfNonChangeBranchTip() throws Exception {
+ // Merge a branch with commits that have not been submitted as
+ // changes.
+ //
+ // M -- mergeCommit (pushed for review and submitted)
+ // | \
+ // | S -- stable (pushed directly to refs/heads/stable)
+ // | /
+ // I -- master
+ //
+ RevCommit master = getRemoteHead(project, "master");
+ PushOneCommit stableTip = pushFactory.create(db, admin.getIdent(), testRepo,
+ "Tip of branch stable", "stable.txt", "");
+ PushOneCommit.Result stable = stableTip.to("refs/heads/stable");
+ PushOneCommit mergeCommit = pushFactory.create(db, admin.getIdent(),
+ testRepo, "The merge commit", "merge.txt", "");
+ mergeCommit.setParents(ImmutableList.of(master, stable.getCommit()));
+ PushOneCommit.Result mergeReview = mergeCommit.to("refs/for/master");
+ approve(mergeReview.getChangeId());
+ submit(mergeReview.getChangeId());
+
+ List<RevCommit> log = getRemoteLog();
+ assertThat(log).contains(stable.getCommit());
+ assertThat(log).contains(mergeReview.getCommit());
+ }
+
private void assertSubmitter(PushOneCommit.Result change) throws Exception {
ChangeInfo info = get(change.getChangeId(), ListChangesOption.MESSAGES);
assertThat(info.messages).isNotNull();
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/ActionsIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/ActionsIT.java
index 4bbc159..98a94b1 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/ActionsIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/ActionsIT.java
@@ -84,9 +84,10 @@
public void revisionActionsOneChangePerTopicUnapproved() throws Exception {
String changeId = createChangeWithTopic().getChangeId();
Map<String, ActionInfo> actions = getActions(changeId);
+ assertThat(actions).hasSize(3);
assertThat(actions).containsKey("cherrypick");
assertThat(actions).containsKey("rebase");
- assertThat(actions).hasSize(2);
+ assertThat(actions).containsKey("description");
}
@Test
@@ -436,9 +437,10 @@
}
private void commonActionsAssertions(Map<String, ActionInfo> actions) {
- assertThat(actions).hasSize(3);
+ assertThat(actions).hasSize(4);
assertThat(actions).containsKey("cherrypick");
assertThat(actions).containsKey("submit");
+ assertThat(actions).containsKey("description");
assertThat(actions).containsKey("rebase");
}
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/MailMetadataIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/MailMetadataIT.java
index d724653..7d59fb1 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/MailMetadataIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/MailMetadataIT.java
@@ -15,6 +15,7 @@
package com.google.gerrit.acceptance.server.mail;
import static com.google.common.truth.Truth.assertThat;
+import static java.util.concurrent.TimeUnit.SECONDS;
import com.google.common.collect.Iterables;
import com.google.gerrit.acceptance.AbstractDaemonTest;
@@ -24,7 +25,10 @@
import com.google.gerrit.server.mail.MailUtil;
import com.google.gerrit.server.mail.send.EmailHeader;
import com.google.gerrit.testutil.FakeEmailSender;
+import com.google.gerrit.testutil.TestTimeUtil;
+import org.junit.After;
+import org.junit.Before;
import org.junit.Test;
import java.sql.Timestamp;
@@ -38,6 +42,20 @@
/** Tests the presence of required metadata in email headers, text and html. */
public class MailMetadataIT extends AbstractDaemonTest {
+ private String systemTimeZone;
+
+ @Before
+ public void setTimeForTesting() {
+ systemTimeZone = System.setProperty("user.timezone", "US/Eastern");
+ TestTimeUtil.resetWithClockStep(1, SECONDS);
+ }
+
+ @After
+ public void resetTime() {
+ TestTimeUtil.useSystemTime();
+ System.setProperty("user.timezone", systemTimeZone);
+ }
+
@Test
public void metadataOnNewChange() throws Exception {
PushOneCommit.Result newChange = createChange();
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/project/CustomLabelIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/project/CustomLabelIT.java
index 6f4cc45..8b1690f 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/project/CustomLabelIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/project/CustomLabelIT.java
@@ -26,11 +26,13 @@
import com.google.gerrit.common.data.Permission;
import com.google.gerrit.extensions.api.changes.AddReviewerInput;
import com.google.gerrit.extensions.api.changes.ReviewInput;
+import com.google.gerrit.extensions.client.ListChangesOption;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.LabelInfo;
import com.google.gerrit.extensions.events.CommentAddedListener;
import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.extensions.registration.RegistrationHandle;
+import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.server.git.ProjectConfig;
import com.google.gerrit.server.group.SystemGroupBackend;
@@ -175,6 +177,34 @@
assertThat(q.blocking).isTrue();
}
+ @Test
+ public void customLabel_DisallowPostSubmit() throws Exception {
+ label.setFunctionName("NoOp");
+ label.setAllowPostSubmit(false);
+ P.setFunctionName("NoOp");
+ saveLabelConfig();
+
+ PushOneCommit.Result r = createChange();
+ revision(r).review(ReviewInput.approve());
+ revision(r).submit();
+
+ ChangeInfo info = get(r.getChangeId(), ListChangesOption.DETAILED_LABELS);
+ assertPermitted(info, "Code-Review", 2);
+ assertPermitted(info, P.getName(), 0, 1);
+ assertPermitted(info, label.getName());
+
+ ReviewInput in = new ReviewInput();
+ in.label(P.getName(), P.getMax().getValue());
+ revision(r).review(in);
+
+ in = new ReviewInput();
+ in.label(label.getName(), label.getMax().getValue());
+ exception.expect(ResourceConflictException.class);
+ exception.expectMessage(
+ "Voting on labels disallowed after submit: " + label.getName());
+ revision(r).review(in);
+ }
+
private void saveLabelConfig() throws Exception {
ProjectConfig cfg = projectCache.checkedGet(project).getConfig();
cfg.getLabelSections().put(label.getName(), label);
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/project/ProjectWatchIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/project/ProjectWatchIT.java
index c999c9c..e6efc0a 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/project/ProjectWatchIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/project/ProjectWatchIT.java
@@ -19,6 +19,8 @@
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.NoHttpd;
import com.google.gerrit.acceptance.PushOneCommit;
+import com.google.gerrit.acceptance.Sandboxed;
+import com.google.gerrit.acceptance.TestAccount;
import com.google.gerrit.extensions.client.ProjectWatchInfo;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.reviewdb.client.AccountProjectWatch.NotifyType;
@@ -37,6 +39,7 @@
import java.util.List;
@NoHttpd
+@Sandboxed
public class ProjectWatchIT extends AbstractDaemonTest {
@Test
public void newPatchSetsNotifyConfig() throws Exception {
@@ -111,12 +114,18 @@
@Test
public void watchFile() throws Exception {
- // watch file in project
String watchedProject = createProject("watchedProject").get();
+ String otherWatchedProject = createProject("otherWatchedProject").get();
setApiUser(user);
+
+ // watch file in project as user
watch(watchedProject, "file:a.txt");
- // push a change to watched file -> should trigger email notification
+ // watch other project as user
+ watch(otherWatchedProject, null);
+
+ // push a change to watched file -> should trigger email notification for
+ // user
setApiUser(admin);
TestRepository<InMemoryRepository> watchedRepo =
cloneProject(new Project.NameKey(watchedProject), admin);
@@ -125,10 +134,87 @@
.to("refs/for/master");
r.assertOkStatus();
+ // assert email notification for user
+ List<Message> messages = sender.getMessages();
+ assertThat(messages).hasSize(1);
+ Message m = messages.get(0);
+ assertThat(m.rcpt()).containsExactly(user.emailAddress);
+ assertThat(m.body()).contains("Change subject: TRIGGER\n");
+ assertThat(m.body()).contains("Gerrit-PatchSet: 1\n");
+ sender.clear();
+
+ // watch project as user2
+ TestAccount user2 = accounts.create("user2", "user2@test.com", "User2");
+ setApiUser(user2);
+ watch(watchedProject, null);
+
// push a change to non-watched file -> should not trigger email
- // notification
- r = pushFactory.create(db, admin.getIdent(), testRepo,
- "DONT_TRIGGER", "b.txt", "b1").to("refs/for/master");
+ // notification for user, only for user2
+ r = pushFactory.create(db, admin.getIdent(), watchedRepo,
+ "TRIGGER_USER2", "b.txt", "b1").to("refs/for/master");
+ r.assertOkStatus();
+
+ // assert email notification
+ messages = sender.getMessages();
+ assertThat(messages).hasSize(1);
+ m = messages.get(0);
+ assertThat(m.rcpt()).containsExactly(user2.emailAddress);
+ assertThat(m.body()).contains("Change subject: TRIGGER_USER2\n");
+ assertThat(m.body()).contains("Gerrit-PatchSet: 1\n");
+ }
+
+ @Test
+ public void watchKeyword() throws Exception {
+ String watchedProject = createProject("watchedProject").get();
+ setApiUser(user);
+
+ // watch keyword in project as user
+ watch(watchedProject, "multimaster");
+
+ // push a change with keyword -> should trigger email notification
+ setApiUser(admin);
+ TestRepository<InMemoryRepository> watchedRepo =
+ cloneProject(new Project.NameKey(watchedProject), admin);
+ PushOneCommit.Result r = pushFactory
+ .create(db, admin.getIdent(), watchedRepo,
+ "Document multimaster setup", "a.txt", "a1")
+ .to("refs/for/master");
+ r.assertOkStatus();
+
+ // assert email notification for user
+ List<Message> messages = sender.getMessages();
+ assertThat(messages).hasSize(1);
+ Message m = messages.get(0);
+ assertThat(m.rcpt()).containsExactly(user.emailAddress);
+ assertThat(m.body())
+ .contains("Change subject: Document multimaster setup\n");
+ assertThat(m.body()).contains("Gerrit-PatchSet: 1\n");
+ sender.clear();
+
+ // push a change without keyword -> should not trigger email notification
+ r = pushFactory.create(db, admin.getIdent(), watchedRepo,
+ "Cleanup cache implementation", "b.txt", "b1").to("refs/for/master");
+ r.assertOkStatus();
+
+ // assert email notification
+ assertThat(sender.getMessages()).hasSize(0);
+ }
+
+ @Test
+ public void watchAllProjects() throws Exception {
+ String anyProject = createProject("anyProject").get();
+ setApiUser(user);
+
+ // watch the All-Projects project to watch all projects
+ watch(allProjects.get(), null);
+
+ // push a change to any project -> should trigger email notification
+ setApiUser(admin);
+ TestRepository<InMemoryRepository> anyRepo =
+ cloneProject(new Project.NameKey(anyProject), admin);
+ PushOneCommit.Result r = pushFactory
+ .create(db, admin.getIdent(), anyRepo, "TRIGGER", "a", "a1")
+ .to("refs/for/master");
r.assertOkStatus();
// assert email notification
@@ -140,6 +226,93 @@
assertThat(m.body()).contains("Gerrit-PatchSet: 1\n");
}
+ @Test
+ public void watchFileAllProjects() throws Exception {
+ String anyProject = createProject("anyProject").get();
+ setApiUser(user);
+
+ // watch file in All-Projects project as user to watch the file in all
+ // projects
+ watch(allProjects.get(), "file:a.txt");
+
+ // push a change to watched file in any project -> should trigger email
+ // notification for user
+ setApiUser(admin);
+ TestRepository<InMemoryRepository> anyRepo =
+ cloneProject(new Project.NameKey(anyProject), admin);
+ PushOneCommit.Result r = pushFactory
+ .create(db, admin.getIdent(), anyRepo, "TRIGGER", "a.txt", "a1")
+ .to("refs/for/master");
+ r.assertOkStatus();
+
+ // assert email notification for user
+ List<Message> messages = sender.getMessages();
+ assertThat(messages).hasSize(1);
+ Message m = messages.get(0);
+ assertThat(m.rcpt()).containsExactly(user.emailAddress);
+ assertThat(m.body()).contains("Change subject: TRIGGER\n");
+ assertThat(m.body()).contains("Gerrit-PatchSet: 1\n");
+ sender.clear();
+
+ // watch project as user2
+ TestAccount user2 = accounts.create("user2", "user2@test.com", "User2");
+ setApiUser(user2);
+ watch(anyProject, null);
+
+ // push a change to non-watched file in any project -> should not trigger
+ // email notification for user, only for user2
+ r = pushFactory.create(db, admin.getIdent(), anyRepo,
+ "TRIGGER_USER2", "b.txt", "b1").to("refs/for/master");
+ r.assertOkStatus();
+
+ // assert email notification
+ messages = sender.getMessages();
+ assertThat(messages).hasSize(1);
+ m = messages.get(0);
+ assertThat(m.rcpt()).containsExactly(user2.emailAddress);
+ assertThat(m.body()).contains("Change subject: TRIGGER_USER2\n");
+ assertThat(m.body()).contains("Gerrit-PatchSet: 1\n");
+ }
+
+ @Test
+ public void watchKeywordAllProjects() throws Exception {
+ String anyProject = createProject("anyProject").get();
+ setApiUser(user);
+
+ // watch keyword in project as user
+ watch(allProjects.get(), "multimaster");
+
+ // push a change with keyword to any project -> should trigger email
+ // notification
+ setApiUser(admin);
+ TestRepository<InMemoryRepository> anyRepo =
+ cloneProject(new Project.NameKey(anyProject), admin);
+ PushOneCommit.Result r = pushFactory
+ .create(db, admin.getIdent(), anyRepo,
+ "Document multimaster setup", "a.txt", "a1")
+ .to("refs/for/master");
+ r.assertOkStatus();
+
+ // assert email notification for user
+ List<Message> messages = sender.getMessages();
+ assertThat(messages).hasSize(1);
+ Message m = messages.get(0);
+ assertThat(m.rcpt()).containsExactly(user.emailAddress);
+ assertThat(m.body())
+ .contains("Change subject: Document multimaster setup\n");
+ assertThat(m.body()).contains("Gerrit-PatchSet: 1\n");
+ sender.clear();
+
+ // push a change without keyword to any project -> should not trigger email
+ // notification
+ r = pushFactory.create(db, admin.getIdent(), anyRepo,
+ "Cleanup cache implementation", "b.txt", "b1").to("refs/for/master");
+ r.assertOkStatus();
+
+ // assert email notification
+ assertThat(sender.getMessages()).hasSize(0);
+ }
+
private void watch(String project, String filter)
throws RestApiException {
List<ProjectWatchInfo> projectsToWatch = new ArrayList<>();
diff --git a/gerrit-common/src/main/java/com/google/gerrit/common/data/LabelType.java b/gerrit-common/src/main/java/com/google/gerrit/common/data/LabelType.java
index b1e1243..8135c91 100644
--- a/gerrit-common/src/main/java/com/google/gerrit/common/data/LabelType.java
+++ b/gerrit-common/src/main/java/com/google/gerrit/common/data/LabelType.java
@@ -25,6 +25,7 @@
import java.util.Map;
public class LabelType {
+ public static final boolean DEF_ALLOW_POST_SUBMIT = true;
public static final boolean DEF_CAN_OVERRIDE = true;
public static final boolean DEF_COPY_ALL_SCORES_IF_NO_CHANGE = true;
public static final boolean DEF_COPY_ALL_SCORES_IF_NO_CODE_CHANGE = false;
@@ -104,6 +105,7 @@
protected boolean copyAllScoresOnTrivialRebase;
protected boolean copyAllScoresIfNoCodeChange;
protected boolean copyAllScoresIfNoChange;
+ protected boolean allowPostSubmit;
protected short defaultValue;
protected List<LabelValue> values;
@@ -144,6 +146,7 @@
DEF_COPY_ALL_SCORES_ON_MERGE_FIRST_PARENT_UPDATE);
setCopyMaxScore(DEF_COPY_MAX_SCORE);
setCopyMinScore(DEF_COPY_MIN_SCORE);
+ setAllowPostSubmit(DEF_ALLOW_POST_SUBMIT);
}
public String getName() {
@@ -174,6 +177,14 @@
this.canOverride = canOverride;
}
+ public boolean allowPostSubmit() {
+ return allowPostSubmit;
+ }
+
+ public void setAllowPostSubmit(boolean allowPostSubmit) {
+ this.allowPostSubmit = allowPostSubmit;
+ }
+
public void setRefPatterns(List<String> refPatterns) {
this.refPatterns = refPatterns;
}
diff --git a/gerrit-elasticsearch/BUCK b/gerrit-elasticsearch/BUCK
index 83f373f..f3ccd01 100644
--- a/gerrit-elasticsearch/BUCK
+++ b/gerrit-elasticsearch/BUCK
@@ -35,6 +35,7 @@
deps = [
':elasticsearch',
'//gerrit-extension-api:api',
+ '//gerrit-reviewdb:server',
'//gerrit-server:server',
'//gerrit-server:testutil',
'//gerrit-server:query_tests',
diff --git a/gerrit-elasticsearch/src/test/java/com/google/gerrit/elasticsearch/ElasticQueryChangesTest.java b/gerrit-elasticsearch/src/test/java/com/google/gerrit/elasticsearch/ElasticQueryChangesTest.java
index 78590a1..9a6755f 100644
--- a/gerrit-elasticsearch/src/test/java/com/google/gerrit/elasticsearch/ElasticQueryChangesTest.java
+++ b/gerrit-elasticsearch/src/test/java/com/google/gerrit/elasticsearch/ElasticQueryChangesTest.java
@@ -27,12 +27,14 @@
import com.google.gerrit.server.index.change.ChangeSchemaDefinitions;
import com.google.gerrit.server.query.change.AbstractQueryChangesTest;
import com.google.gerrit.testutil.InMemoryModule;
+import com.google.gerrit.testutil.InMemoryRepositoryManager.Repo;
import com.google.gson.FieldNamingPolicy;
import com.google.gson.Gson;
import com.google.gson.GsonBuilder;
import com.google.inject.Guice;
import com.google.inject.Injector;
+import org.eclipse.jgit.junit.TestRepository;
import org.eclipse.jgit.lib.Config;
import org.elasticsearch.action.admin.cluster.node.info.NodesInfoRequest;
import org.elasticsearch.common.settings.Settings;
@@ -41,6 +43,7 @@
import org.junit.After;
import org.junit.AfterClass;
import org.junit.BeforeClass;
+import org.junit.Test;
import java.io.File;
import java.nio.file.Path;
@@ -179,4 +182,13 @@
return httpAddress.substring(httpAddress.indexOf(':') + 1,
httpAddress.length());
}
+
+ @Test
+ public void byOwnerInvalidQuery() throws Exception {
+ TestRepository<Repo> repo = createProject("repo");
+ insert(repo, newChange(repo), userId);
+ String nameEmail = user.asIdentifiedUser().getNameEmail();
+ assertQuery("owner: \"" + nameEmail + "\"\\");
+ }
+
}
diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/RevisionApi.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/RevisionApi.java
index 8ada55a..2375b7e 100644
--- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/RevisionApi.java
+++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/RevisionApi.java
@@ -33,6 +33,9 @@
public interface RevisionApi {
void delete() throws RestApiException;
+ String description() throws RestApiException;
+ void description(String description) throws RestApiException;
+
void review(ReviewInput in) throws RestApiException;
void submit() throws RestApiException;
@@ -283,5 +286,15 @@
public MergeListRequest getMergeList() throws RestApiException {
throw new NotImplementedException();
}
+
+ @Override
+ public void description(String description) throws RestApiException {
+ throw new NotImplementedException();
+ }
+
+ @Override
+ public String description() throws RestApiException {
+ throw new NotImplementedException();
+ }
}
}
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/CommitBox.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/CommitBox.java
index 9ec1356..8f00a45 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/CommitBox.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/CommitBox.java
@@ -56,7 +56,6 @@
String collapsed();
String expanded();
String clippy();
- String parentWebLink();
}
@UiField Style style;
@@ -67,7 +66,6 @@
@UiField FlowPanel webLinkPanel;
@UiField TableRowElement firstParent;
@UiField FlowPanel parentCommits;
- @UiField FlowPanel parentWebLinks;
@UiField InlineHyperlink authorNameEmail;
@UiField Element authorDate;
@UiField InlineHyperlink committerNameEmail;
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/CommitBox.ui.xml b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/CommitBox.ui.xml
index 5f476be..19f115d 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/CommitBox.ui.xml
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/CommitBox.ui.xml
@@ -88,13 +88,6 @@
margin-left:2px;
}
- .parentWebLink a:first-child {
- margin-left:16px;
- }
- .parentWebLink>a {
- margin-left:2px;
- }
-
.commit {
margin-right: 3px;
float: left;
@@ -185,9 +178,6 @@
<td>
<g:FlowPanel ui:field='parentCommits'/>
</td>
- <td>
- <g:FlowPanel ui:field='parentWebLinks' styleName='{style.parentWebLink}'/>
- </td>
</tr>
<tr>
<th><ui:msg>Change-Id</ui:msg></th>
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/ChangeMessagesUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/ChangeMessagesUtil.java
index 83e238f..55c7e21 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/ChangeMessagesUtil.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/ChangeMessagesUtil.java
@@ -64,6 +64,8 @@
"autogenerated:gerrit:revert";
public static final String TAG_SET_ASSIGNEE =
"autogenerated:gerrit:setAssignee";
+ public static final String TAG_SET_DESCRIPTION =
+ "autogenerated:gerrit:setPsDescription";
public static final String TAG_SET_HASHTAGS =
"autogenerated:gerrit:setHashtag";
public static final String TAG_SET_TOPIC =
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/RevisionApiImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/RevisionApiImpl.java
index 5864a39..c5fb188 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/RevisionApiImpl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/RevisionApiImpl.java
@@ -47,6 +47,7 @@
import com.google.gerrit.server.change.DraftComments;
import com.google.gerrit.server.change.FileResource;
import com.google.gerrit.server.change.Files;
+import com.google.gerrit.server.change.GetDescription;
import com.google.gerrit.server.change.GetMergeList;
import com.google.gerrit.server.change.GetPatch;
import com.google.gerrit.server.change.GetRevisionActions;
@@ -57,6 +58,7 @@
import com.google.gerrit.server.change.PostReview;
import com.google.gerrit.server.change.PreviewSubmit;
import com.google.gerrit.server.change.PublishDraftPatchSet;
+import com.google.gerrit.server.change.PutDescription;
import com.google.gerrit.server.change.Rebase;
import com.google.gerrit.server.change.RebaseUtil;
import com.google.gerrit.server.change.Reviewed;
@@ -118,6 +120,8 @@
private final TestSubmitType testSubmitType;
private final TestSubmitType.Get getSubmitType;
private final Provider<GetMergeList> getMergeList;
+ private final PutDescription putDescription;
+ private final GetDescription getDescription;
@Inject
RevisionApiImpl(GitRepositoryManager repoManager,
@@ -151,6 +155,8 @@
TestSubmitType testSubmitType,
TestSubmitType.Get getSubmitType,
Provider<GetMergeList> getMergeList,
+ PutDescription putDescription,
+ GetDescription getDescription,
@Assisted RevisionResource r) {
this.repoManager = repoManager;
this.changes = changes;
@@ -183,6 +189,8 @@
this.testSubmitType = testSubmitType;
this.getSubmitType = getSubmitType;
this.getMergeList = getMergeList;
+ this.putDescription = putDescription;
+ this.getDescription = getDescription;
this.revision = r;
}
@@ -515,4 +523,20 @@
}
};
}
+
+ @Override
+ public void description(String description) throws RestApiException {
+ PutDescription.Input in = new PutDescription.Input();
+ in.description = description;
+ try {
+ putDescription.apply(revision, in);
+ } catch (UpdateException e) {
+ throw new RestApiException("Cannot set description", e);
+ }
+ }
+
+ @Override
+ public String description() throws RestApiException {
+ return getDescription.apply(revision);
+ }
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java
index 15d39b5..e4326a8 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java
@@ -36,6 +36,7 @@
import static com.google.gerrit.extensions.client.ListChangesOption.SUBMITTABLE;
import static com.google.gerrit.extensions.client.ListChangesOption.WEB_LINKS;
import static com.google.gerrit.server.CommonConverters.toGitPerson;
+
import static java.util.stream.Collectors.toList;
import com.google.auto.value.AutoValue;
@@ -89,6 +90,7 @@
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.AnonymousUser;
+import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.GpgException;
@@ -192,6 +194,7 @@
private final ChangeResource.Factory changeResourceFactory;
private final ChangeKindCache changeKindCache;
private final ChangeIndexCollection indexes;
+ private final ApprovalsUtil approvalsUtil;
private boolean lazyLoad = true;
private AccountLoader accountLoader;
@@ -221,6 +224,7 @@
ChangeResource.Factory changeResourceFactory,
ChangeKindCache changeKindCache,
ChangeIndexCollection indexes,
+ ApprovalsUtil approvalsUtil,
@Assisted Set<ListChangesOption> options) {
this.db = db;
this.labelNormalizer = ln;
@@ -244,6 +248,7 @@
this.changeResourceFactory = changeResourceFactory;
this.changeKindCache = changeKindCache;
this.indexes = indexes;
+ this.approvalsUtil = approvalsUtil;
this.options = options.isEmpty()
? EnumSet.noneOf(ListChangesOption.class)
: EnumSet.copyOf(options);
@@ -891,10 +896,12 @@
private Map<String, Collection<String>> permittedLabels(ChangeControl ctl, ChangeData cd)
throws OrmException {
- if (ctl == null) {
+ if (ctl == null || !ctl.getUser().isIdentifiedUser()) {
return null;
}
+ Map<String, Short> labels = null;
+ boolean isMerged = ctl.getChange().getStatus() == Change.Status.MERGED;
LabelTypes labelTypes = ctl.getLabelTypes();
SetMultimap<String, String> permitted = LinkedHashMultimap.create();
for (SubmitRecord rec : submitRecords(cd)) {
@@ -903,12 +910,20 @@
}
for (SubmitRecord.Label r : rec.labels) {
LabelType type = labelTypes.byLabel(r.label);
- if (type == null) {
+ if (type == null || (isMerged && !type.allowPostSubmit())) {
continue;
}
PermissionRange range = ctl.getRange(Permission.forLabel(r.label));
for (LabelValue v : type.getValues()) {
- if (range.contains(v.getValue())) {
+ boolean ok = range.contains(v.getValue());
+ if (isMerged) {
+ if (labels == null) {
+ labels = currentLabels(ctl);
+ }
+ short prev = labels.getOrDefault(type.getName(), (short) 0);
+ ok &= v.getValue() >= prev;
+ }
+ if (ok) {
permitted.put(r.label, v.formatValue());
}
}
@@ -928,6 +943,17 @@
return permitted.asMap();
}
+ private Map<String, Short> currentLabels(ChangeControl ctl)
+ throws OrmException {
+ Map<String, Short> result = new HashMap<>();
+ for (PatchSetApproval psa : approvalsUtil.byPatchSetUser(
+ db.get(), ctl, ctl.getChange().currentPatchSetId(),
+ ctl.getUser().getAccountId())) {
+ result.put(psa.getLabel(), psa.getValue());
+ }
+ return result;
+ }
+
private Collection<ChangeMessageInfo> messages(ChangeControl ctl, ChangeData cd,
Map<PatchSet.Id, PatchSet> map)
throws OrmException {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/GetDescription.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/GetDescription.java
new file mode 100644
index 0000000..b8a34d2
--- /dev/null
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/GetDescription.java
@@ -0,0 +1,27 @@
+// Copyright (C) 2016 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.change;
+
+import com.google.common.base.Strings;
+import com.google.gerrit.extensions.restapi.RestReadView;
+import com.google.inject.Singleton;
+
+@Singleton
+public class GetDescription implements RestReadView<RevisionResource> {
+ @Override
+ public String apply(RevisionResource rsrc) {
+ return Strings.nullToEmpty(rsrc.getPatchSet().getDescription());
+ }
+}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Module.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Module.java
index 9ff9833..4d32222 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Module.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Module.java
@@ -104,6 +104,8 @@
get(REVISION_KIND, "preview_submit").to(PreviewSubmit.class);
post(REVISION_KIND, "submit").to(Submit.class);
post(REVISION_KIND, "rebase").to(Rebase.class);
+ put(REVISION_KIND, "description").to(PutDescription.class);
+ get(REVISION_KIND, "description").to(GetDescription.class);
get(REVISION_KIND, "patch").to(GetPatch.class);
get(REVISION_KIND, "submit_type").to(TestSubmitType.Get.class);
post(REVISION_KIND, "test.submit_rule").to(TestSubmitRule.class);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java
index e711181..cd62e45 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java
@@ -900,10 +900,27 @@
// make it possible to take a merged change and make it no longer
// submittable.
List<PatchSetApproval> reduced = new ArrayList<>(ups.size() + del.size());
- reduced.addAll(del);
+ List<String> disallowed =
+ new ArrayList<>(labelTypes.getLabelTypes().size());
+
+ for (PatchSetApproval psa : del) {
+ LabelType lt = checkNotNull(labelTypes.byLabel(psa.getLabel()));
+ String normName = lt.getName();
+ if (!lt.allowPostSubmit()) {
+ disallowed.add(normName);
+ }
+ Short prev = previous.get(normName);
+ if (prev != null && prev != 0) {
+ reduced.add(psa);
+ }
+ }
+
for (PatchSetApproval psa : ups) {
LabelType lt = checkNotNull(labelTypes.byLabel(psa.getLabel()));
String normName = lt.getName();
+ if (!lt.allowPostSubmit()) {
+ disallowed.add(normName);
+ }
Short prev = previous.get(normName);
if (prev == null) {
continue;
@@ -918,6 +935,12 @@
}
}
+ if (!disallowed.isEmpty()) {
+ throw new ResourceConflictException(
+ "Voting on labels disallowed after submit: "
+ + disallowed.stream().distinct().sorted()
+ .collect(joining(", ")));
+ }
if (!reduced.isEmpty()) {
throw new ResourceConflictException(
"Cannot reduce vote on labels for closed change: "
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PutDescription.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PutDescription.java
new file mode 100644
index 0000000..b8224a8
--- /dev/null
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PutDescription.java
@@ -0,0 +1,132 @@
+// Copyright (C) 2016 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.change;
+
+import com.google.common.base.Strings;
+import com.google.gerrit.common.TimeUtil;
+import com.google.gerrit.extensions.restapi.AuthException;
+import com.google.gerrit.extensions.restapi.DefaultInput;
+import com.google.gerrit.extensions.restapi.Response;
+import com.google.gerrit.extensions.restapi.RestApiException;
+import com.google.gerrit.extensions.restapi.RestModifyView;
+import com.google.gerrit.extensions.webui.UiAction;
+import com.google.gerrit.reviewdb.client.ChangeMessage;
+import com.google.gerrit.reviewdb.client.PatchSet;
+import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.ChangeMessagesUtil;
+import com.google.gerrit.server.git.BatchUpdate;
+import com.google.gerrit.server.git.BatchUpdate.ChangeContext;
+import com.google.gerrit.server.git.UpdateException;
+import com.google.gerrit.server.notedb.ChangeUpdate;
+import com.google.gwtorm.server.OrmException;
+import com.google.gerrit.server.PatchSetUtil;
+import com.google.gerrit.server.project.ChangeControl;
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import com.google.inject.Singleton;
+
+import java.util.Collections;
+
+@Singleton
+public class PutDescription implements RestModifyView<RevisionResource,
+ PutDescription.Input>, UiAction<RevisionResource> {
+ private final Provider<ReviewDb> dbProvider;
+ private final ChangeMessagesUtil cmUtil;
+ private final BatchUpdate.Factory batchUpdateFactory;
+ private final PatchSetUtil psUtil;
+
+ public static class Input {
+ @DefaultInput
+ public String description;
+ }
+
+ @Inject
+ PutDescription(Provider<ReviewDb> dbProvider,
+ ChangeMessagesUtil cmUtil,
+ BatchUpdate.Factory batchUpdateFactory,
+ PatchSetUtil psUtil) {
+ this.dbProvider = dbProvider;
+ this.cmUtil = cmUtil;
+ this.batchUpdateFactory = batchUpdateFactory;
+ this.psUtil = psUtil;
+ }
+
+ @Override
+ public Response<String> apply(RevisionResource rsrc, Input input)
+ throws UpdateException, RestApiException {
+ ChangeControl ctl = rsrc.getControl();
+ if (!ctl.canEditDescription()) {
+ throw new AuthException("changing description not permitted");
+ }
+ Op op =
+ new Op(input != null ? input : new Input(), rsrc.getPatchSet().getId());
+ try (BatchUpdate u = batchUpdateFactory.create(dbProvider.get(),
+ rsrc.getChange().getProject(), ctl.getUser(), TimeUtil.nowTs())) {
+ u.addOp(rsrc.getChange().getId(), op);
+ u.execute();
+ }
+ return Strings.isNullOrEmpty(op.newDescription) ? Response.none()
+ : Response.ok(op.newDescription);
+ }
+
+ private class Op extends BatchUpdate.Op {
+ private final Input input;
+ private final PatchSet.Id psId;
+
+ private String oldDescription;
+ private String newDescription;
+
+ Op(Input input, PatchSet.Id psId) {
+ this.input = input;
+ this.psId = psId;
+ }
+
+ @Override
+ public boolean updateChange(ChangeContext ctx) throws OrmException {
+ PatchSet ps = psUtil.get(ctx.getDb(), ctx.getNotes(), psId);
+ ChangeUpdate update = ctx.getUpdate(psId);
+ newDescription = Strings.nullToEmpty(input.description);
+ oldDescription = Strings.nullToEmpty(ps.getDescription());
+ if (oldDescription.equals(newDescription)) {
+ return false;
+ }
+ String summary;
+ if (oldDescription.isEmpty()) {
+ summary = "Description set to \"" + newDescription + "\"";
+ } else if (newDescription.isEmpty()) {
+ summary = "Description \"" + oldDescription + "\" removed";
+ } else {
+ summary = "Description changed to \"" + newDescription + "\"";
+ }
+
+ ps.setDescription(newDescription);
+ update.setPsDescription(newDescription);
+
+ ctx.getDb().patchSets().update(Collections.singleton(ps));
+
+ ChangeMessage cmsg =
+ ChangeMessagesUtil.newMessage(ctx.getDb(), psId, ctx.getUser(),
+ ctx.getWhen(), summary, ChangeMessagesUtil.TAG_SET_DESCRIPTION);
+ cmUtil.addChangeMessage(ctx.getDb(), update, cmsg);
+ return true;
+ }
+ }
+
+ @Override
+ public UiAction.Description getDescription(RevisionResource rsrc) {
+ return new UiAction.Description().setLabel("Edit Description")
+ .setVisible(rsrc.getControl().canEditDescription());
+ }
+}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/BatchUpdate.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/BatchUpdate.java
index 3953f27..bc2b4df 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/BatchUpdate.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/BatchUpdate.java
@@ -1029,10 +1029,14 @@
}
private ChangeContext newChangeContext(ReviewDb db, Repository repo,
- RevWalk rw, Change.Id id) throws Exception {
+ RevWalk rw, Change.Id id) throws OrmException, NoSuchChangeException {
Change c = newChanges.get(id);
if (c == null) {
c = ChangeNotes.readOneReviewDbChange(db, id);
+ if (c == null) {
+ logDebug("Failed to get change {} from unwrapped db", id);
+ throw new NoSuchChangeException(id);
+ }
}
// Pass in preloaded change to controlFor, to avoid:
// - reading from a db that does not belong to this update
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/LockFailureException.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/LockFailureException.java
new file mode 100644
index 0000000..7380b0a
--- /dev/null
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/LockFailureException.java
@@ -0,0 +1,26 @@
+// Copyright (C) 2016 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.git;
+
+import java.io.IOException;
+
+/** Thrown when updating a ref in Git fails with LOCK_FAILURE. */
+public class LockFailureException extends IOException {
+ private static final long serialVersionUID = 1L;
+
+ public LockFailureException(String message) {
+ super(message);
+ }
+}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeUtil.java
index 90edfb1..025db40 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeUtil.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeUtil.java
@@ -679,7 +679,10 @@
rw.sort(RevSort.REVERSE, true);
rw.markStart(mergeTip);
for (RevCommit c : alreadyAccepted) {
- rw.markUninteresting(c);
+ // If branch was not created by this submit.
+ if (c != mergeTip) {
+ rw.markUninteresting(c);
+ }
}
CodeReviewCommit c;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ProjectConfig.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ProjectConfig.java
index 5421421..f3ed9f2 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ProjectConfig.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ProjectConfig.java
@@ -144,6 +144,7 @@
private static final String KEY_FUNCTION = "function";
private static final String KEY_DEFAULT_VALUE = "defaultValue";
private static final String KEY_COPY_MIN_SCORE = "copyMinScore";
+ private static final String KEY_ALLOW_POST_SUBMIT = "allowPostSubmit";
private static final String KEY_COPY_MAX_SCORE = "copyMaxScore";
private static final String KEY_COPY_ALL_SCORES_ON_MERGE_FIRST_PARENT_UPDATE = "copyAllScoresOnMergeFirstParentUpdate";
private static final String KEY_COPY_ALL_SCORES_ON_TRIVIAL_REBASE = "copyAllScoresOnTrivialRebase";
@@ -802,6 +803,9 @@
KEY_DEFAULT_VALUE, dv, name)));
}
}
+ label.setAllowPostSubmit(
+ rc.getBoolean(LABEL, name, KEY_ALLOW_POST_SUBMIT,
+ LabelType.DEF_ALLOW_POST_SUBMIT));
label.setCopyMinScore(
rc.getBoolean(LABEL, name, KEY_COPY_MIN_SCORE,
LabelType.DEF_COPY_MIN_SCORE));
@@ -1197,6 +1201,8 @@
rc.setString(LABEL, name, KEY_FUNCTION, label.getFunctionName());
rc.setInt(LABEL, name, KEY_DEFAULT_VALUE, label.getDefaultValue());
+ setBooleanConfigKey(rc, name, KEY_ALLOW_POST_SUBMIT, label.allowPostSubmit(),
+ LabelType.DEF_ALLOW_POST_SUBMIT);
setBooleanConfigKey(rc, name, KEY_COPY_MIN_SCORE, label.isCopyMinScore(),
LabelType.DEF_COPY_MIN_SCORE);
setBooleanConfigKey(rc, name, KEY_COPY_MAX_SCORE, label.isCopyMaxScore(),
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/RebaseSorter.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/RebaseSorter.java
index 5952602..6448d06 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/RebaseSorter.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/RebaseSorter.java
@@ -32,13 +32,15 @@
public class RebaseSorter {
private final CodeReviewRevWalk rw;
private final RevFlag canMergeFlag;
- private final Set<RevCommit> accepted;
+ private final RevCommit initialTip;
+ private final Set<RevCommit> alreadyAccepted;
- public RebaseSorter(CodeReviewRevWalk rw, Set<RevCommit> alreadyAccepted,
- RevFlag canMergeFlag) {
+ public RebaseSorter(CodeReviewRevWalk rw, RevCommit initialTip,
+ Set<RevCommit> alreadyAccepted, RevFlag canMergeFlag) {
this.rw = rw;
this.canMergeFlag = canMergeFlag;
- this.accepted = alreadyAccepted;
+ this.initialTip = initialTip;
+ this.alreadyAccepted = alreadyAccepted;
}
public List<CodeReviewCommit> sort(Collection<CodeReviewCommit> incoming)
@@ -50,17 +52,18 @@
rw.resetRetain(canMergeFlag);
rw.markStart(n);
- for (RevCommit c : accepted) {
- // n also tip of directly pushed branch => n remains 'interesting' here
- if (!c.equals(n)) {
- rw.markUninteresting(c);
- }
+ if (initialTip != null) {
+ rw.markUninteresting(initialTip);
}
CodeReviewCommit c;
final List<CodeReviewCommit> contents = new ArrayList<>();
while ((c = rw.next()) != null) {
if (!c.has(canMergeFlag) || !incoming.contains(c)) {
+ if (isAlreadyMerged(c)) {
+ rw.markUninteresting(c);
+ break;
+ }
// We cannot merge n as it would bring something we
// aren't permitted to merge at this time. Drop n.
//
@@ -86,6 +89,21 @@
return sorted;
}
+ private boolean isAlreadyMerged(CodeReviewCommit commit) throws IOException {
+ try (CodeReviewRevWalk mirw =
+ CodeReviewCommit.newRevWalk(rw.getObjectReader())) {
+ mirw.reset();
+ mirw.markStart(commit);
+ for (RevCommit accepted : alreadyAccepted) {
+ if (mirw.isMergedInto(mirw.parseCommit(accepted),
+ mirw.parseCommit(commit))) {
+ return true;
+ }
+ }
+ }
+ return false;
+ }
+
private static <T> T removeOne(final Collection<T> c) {
final Iterator<T> i = c.iterator();
final T r = i.next();
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/VersionedMetaData.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/VersionedMetaData.java
index 6334cd2..f7752dc 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/VersionedMetaData.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/VersionedMetaData.java
@@ -347,9 +347,11 @@
case FORCED:
update.fireGitRefUpdatedEvent(ru);
return;
+ case LOCK_FAILURE:
+ throw new LockFailureException("Cannot delete " + ru.getName()
+ + " in " + db.getDirectory() + ": " + ru.getResult());
case FAST_FORWARD:
case IO_FAILURE:
- case LOCK_FAILURE:
case NEW:
case NOT_ATTEMPTED:
case NO_CHANGE:
@@ -424,9 +426,11 @@
revision = rw.parseCommit(ru.getNewObjectId());
update.fireGitRefUpdatedEvent(ru);
return revision;
+ case LOCK_FAILURE:
+ throw new LockFailureException("Cannot update " + ru.getName()
+ + " in " + db.getDirectory() + ": " + ru.getResult());
case FORCED:
case IO_FAILURE:
- case LOCK_FAILURE:
case NOT_ATTEMPTED:
case NO_CHANGE:
case REJECTED:
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/RebaseSubmitStrategy.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/RebaseSubmitStrategy.java
index c93cf6d..9beae1f 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/RebaseSubmitStrategy.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/RebaseSubmitStrategy.java
@@ -18,6 +18,7 @@
import static com.google.gerrit.server.git.strategy.CommitMergeStatus.SKIPPED_IDENTICAL_TREE;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
import com.google.gerrit.extensions.restapi.MergeConflictException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.RestApiException;
@@ -39,6 +40,7 @@
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.PersonIdent;
+import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.transport.ReceiveCommand;
import java.io.IOException;
@@ -60,7 +62,7 @@
@Override
public List<SubmitStrategyOp> buildOps(
Collection<CodeReviewCommit> toMerge) throws IntegrationException {
- List<CodeReviewCommit> sorted = sort(toMerge);
+ List<CodeReviewCommit> sorted = sort(toMerge, args.mergeTip.getCurrentTip());
List<SubmitStrategyOp> ops = new ArrayList<>(sorted.size());
boolean first = true;
@@ -256,8 +258,10 @@
args.inserter, args.destBranch, mergeTip.getCurrentTip(), toMerge);
mergeTip.moveTipTo(amendGitlink(newTip), toMerge);
}
+ RevCommit initialTip = mergeTip.getInitialTip();
args.mergeUtil.markCleanMerges(args.rw, args.canMergeFlag,
- mergeTip.getCurrentTip(), args.alreadyAccepted);
+ mergeTip.getCurrentTip(), initialTip == null ?
+ ImmutableSet.<RevCommit>of() : ImmutableSet.of(initialTip));
acceptMergeTip(mergeTip);
}
}
@@ -266,11 +270,11 @@
args.alreadyAccepted.add(mergeTip.getCurrentTip());
}
- private List<CodeReviewCommit> sort(Collection<CodeReviewCommit> toSort)
- throws IntegrationException {
+ private List<CodeReviewCommit> sort(Collection<CodeReviewCommit> toSort,
+ RevCommit initialTip) throws IntegrationException {
try {
- return new RebaseSorter(
- args.rw, args.alreadyAccepted, args.canMergeFlag).sort(toSort);
+ return new RebaseSorter(args.rw, initialTip, args.alreadyAccepted,
+ args.canMergeFlag).sort(toSort);
} catch (IOException e) {
throw new IntegrationException("Commit sorting failed", e);
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/group/ListGroups.java b/gerrit-server/src/main/java/com/google/gerrit/server/group/ListGroups.java
index 85c3d15..a53829e 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/group/ListGroups.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/group/ListGroups.java
@@ -326,12 +326,6 @@
continue;
}
}
- if (!isAdmin) {
- GroupControl c = groupControlFactory.controlFor(group);
- if (!c.isVisible()) {
- continue;
- }
- }
if (visibleToAll && !group.isVisibleToAll()) {
continue;
}
@@ -339,6 +333,12 @@
&& !groupsToInspect.contains(group.getGroupUUID())) {
continue;
}
+ if (!isAdmin) {
+ GroupControl c = groupControlFactory.controlFor(group);
+ if (!c.isVisible()) {
+ continue;
+ }
+ }
filteredGroups.add(group);
}
Collections.sort(filteredGroups, new GroupComparator());
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java
index f7ebd77..145d1c8 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java
@@ -441,6 +441,18 @@
return getRefControl().canForceEditTopicName();
}
+ /** Can this user edit the description? */
+ public boolean canEditDescription() {
+ if (getChange().getStatus().isOpen()) {
+ return isOwner() // owner (aka creator) of the change can edit desc
+ || getRefControl().isOwner() // branch owner can edit desc
+ || getProjectControl().isOwner() // project owner can edit desc
+ || getUser().getCapabilities().canAdministrateServer() // site administers are god
+ ;
+ }
+ return false;
+ }
+
public boolean canEditAssignee() {
return isOwner()
|| getProjectControl().isOwner()
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java
index 3387f06..21896f2 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java
@@ -1129,7 +1129,7 @@
return Collections.emptySet();
}
draftsByUser = new HashSet<>();
- for (Comment sc : commentsUtil.draftByChange(db, notes)) {
+ for (Comment sc : commentsUtil.draftByChange(db, notes())) {
draftsByUser.add(sc.author.getId());
}
}
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
index 3e3ec13..a1a5fbc 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
@@ -374,6 +374,9 @@
assertQuery("owner:" + userId.get(), change1);
assertQuery("owner:" + user2, change2);
+
+ String nameEmail = user.asIdentifiedUser().getNameEmail();
+ assertQuery("owner: \"" + nameEmail + "\"", change1);
}
@Test
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/LuceneQueryChangesTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/LuceneQueryChangesTest.java
index 038abda..70493e8 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/LuceneQueryChangesTest.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/LuceneQueryChangesTest.java
@@ -14,6 +14,7 @@
package com.google.gerrit.server.query.change;
+import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.testutil.InMemoryModule;
import com.google.gerrit.testutil.InMemoryRepositoryManager.Repo;
@@ -52,4 +53,15 @@
assertQuery("message:one.two", change2);
assertQuery("message:one two", change2);
}
+
+ @Test
+ public void byOwnerInvalidQuery() throws Exception {
+ TestRepository<Repo> repo = createProject("repo");
+ Change change1 = insert(repo, newChange(repo), userId);
+ String nameEmail = user.asIdentifiedUser().getNameEmail();
+
+ exception.expect(BadRequestException.class);
+ exception.expectMessage("Cannot create full-text query with value: \\");
+ assertQuery("owner: \"" + nameEmail + "\"\\", change1);
+ }
}
diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/ChangeArgumentParser.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/ChangeArgumentParser.java
index d658b7f..93a508c 100644
--- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/ChangeArgumentParser.java
+++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/ChangeArgumentParser.java
@@ -96,8 +96,8 @@
}
private List<ChangeControl> changeFromNotesFactory(String id, CurrentUser currentUser)
- throws OrmException {
- return changeNotesFactory.create(db, Arrays.asList(Change.Id.parse(id)))
+ throws OrmException, UnloggedFailure {
+ return changeNotesFactory.create(db, parseId(id))
.stream()
.map(changeNote -> controlForChange(changeNote, currentUser))
.filter(changeControl -> changeControl.isPresent())
@@ -105,7 +105,16 @@
.collect(toList());
}
- private Optional<ChangeControl> controlForChange(ChangeNotes change, CurrentUser user) {
+ private List<Change.Id> parseId(String id) throws UnloggedFailure {
+ try {
+ return Arrays.asList(new Change.Id(Integer.parseInt(id)));
+ } catch (NumberFormatException e) {
+ throw new UnloggedFailure(2, "Invalid change ID " + id, e);
+ }
+ }
+
+ private Optional<ChangeControl> controlForChange(ChangeNotes change,
+ CurrentUser user) {
try {
return Optional.of(changeControlFactory.controlFor(change, user));
} catch (NoSuchChangeException e) {
diff --git a/polygerrit-ui/app/behaviors/keyboard-shortcut-behavior.html b/polygerrit-ui/app/behaviors/keyboard-shortcut-behavior.html
index f636650..ba7d3de1 100644
--- a/polygerrit-ui/app/behaviors/keyboard-shortcut-behavior.html
+++ b/polygerrit-ui/app/behaviors/keyboard-shortcut-behavior.html
@@ -14,69 +14,30 @@
limitations under the License.
-->
<link rel="import" href="../bower_components/polymer/polymer.html">
+<link rel="import" href="../bower_components/iron-a11y-keys-behavior/iron-a11y-keys-behavior.html">
+
<script>
(function(window) {
'use strict';
- /** @polymerBehavior Gerrit.KeyboardShortcutBehavior */
- var KeyboardShortcutBehavior = {
- // Set of identifiers currently blocking keyboard shortcuts. Stored as
- // a map of string to the value of true.
- _disablers: {},
-
- properties: {
- keyEventTarget: {
- type: Object,
- value: function() { return this; },
- },
-
- _boundKeyHandler: {
- type: Function,
- readonly: true,
- value: function() { return this._handleKey.bind(this); },
- },
- },
-
- attached: function() {
- this.keyEventTarget.addEventListener('keydown', this._boundKeyHandler);
- },
-
- detached: function() {
- this.keyEventTarget.removeEventListener('keydown', this._boundKeyHandler);
- },
-
+ var KeyboardShortcutBehaviorImpl = {
shouldSuppressKeyboardShortcut: function(e) {
- for (var c in KeyboardShortcutBehavior._disablers) {
- if (KeyboardShortcutBehavior._disablers[c] === true) {
- return true;
- }
+ e = Polymer.dom(e.detail ? e.detail.keyboardEvent : e);
+ if (e.path[0].tagName === 'INPUT' || e.path[0].tagName === 'TEXTAREA') {
+ return true;
}
- var getModifierState = e.getModifierState ?
- e.getModifierState.bind(e) :
- function() { return false; };
- var target = e.detail ? e.detail.keyboardEvent : e.target;
- return getModifierState('Control') ||
- getModifierState('Alt') ||
- getModifierState('Meta') ||
- getModifierState('Fn') ||
- target.tagName == 'INPUT' ||
- target.tagName == 'TEXTAREA' ||
- target.tagName == 'SELECT' ||
- target.tagName == 'BUTTON' ||
- target.tagName == 'A' ||
- target.tagName == 'GR-BUTTON';
- },
-
- disable: function(id) {
- KeyboardShortcutBehavior._disablers[id] = true;
- },
-
- enable: function(id) {
- delete KeyboardShortcutBehavior._disablers[id];
+ for (var i = 0; i < e.path.length; i++) {
+ if (e.path[i].tagName === 'GR-OVERLAY') { return true; }
+ }
+ return false;
},
};
window.Gerrit = window.Gerrit || {};
- window.Gerrit.KeyboardShortcutBehavior = KeyboardShortcutBehavior;
+ /** @polymerBehavior Gerrit.KeyboardShortcutBehavior */
+ window.Gerrit.KeyboardShortcutBehavior = [
+ Polymer.IronA11yKeysBehavior,
+ KeyboardShortcutBehaviorImpl,
+ ];
})(window);
</script>
diff --git a/polygerrit-ui/app/behaviors/keyboard-shortcut-behavior_test.html b/polygerrit-ui/app/behaviors/keyboard-shortcut-behavior_test.html
index 5ec4145..457c5f25 100644
--- a/polygerrit-ui/app/behaviors/keyboard-shortcut-behavior_test.html
+++ b/polygerrit-ui/app/behaviors/keyboard-shortcut-behavior_test.html
@@ -30,49 +30,76 @@
</template>
</test-fixture>
+<test-fixture id="within-overlay">
+ <template>
+ <gr-overlay>
+ <test-element></test-element>
+ </gr-overlay>
+ </template>
+</test-fixture>
+
<script>
suite('keyboard-shortcut-behavior tests', function() {
var element;
+ var overlay;
suiteSetup(function() {
// Define a Polymer element that uses this behavior.
Polymer({
is: 'test-element',
behaviors: [Gerrit.KeyboardShortcutBehavior],
- properties: {
- keyEventTarget: {
- value: function() { return document.body; },
- },
- log: {
- value: function() { return []; },
- },
+ keyBindings: {
+ 'k': '_handleKey'
},
-
- _handleKey: function(e) {
- if (!this.shouldSuppressKeyboardShortcut(e)) {
- this.log.push(e.keyCode);
- }
- },
+ _handleKey: function() {},
});
});
setup(function() {
element = fixture('basic');
+ overlay = fixture('within-overlay');
});
- test('blocks keydown events iff one or more disablers', function() {
- MockInteractions.pressAndReleaseKeyOn(document.body, 97); // 'a'
- Gerrit.KeyboardShortcutBehavior.enable('x'); // should have no effect
- MockInteractions.pressAndReleaseKeyOn(document.body, 98); // 'b'
- Gerrit.KeyboardShortcutBehavior.disable('x'); // blocking starts here
- MockInteractions.pressAndReleaseKeyOn(document.body, 99); // 'c'
- Gerrit.KeyboardShortcutBehavior.disable('y');
- MockInteractions.pressAndReleaseKeyOn(document.body, 100); // 'd'
- Gerrit.KeyboardShortcutBehavior.enable('x');
- MockInteractions.pressAndReleaseKeyOn(document.body, 101); // 'e'
- Gerrit.KeyboardShortcutBehavior.enable('y'); // blocking ends here
- MockInteractions.pressAndReleaseKeyOn(document.body, 102); // 'f'
- assert.deepEqual(element.log, [97, 98, 102]);
+ test('doesn’t block kb shortcuts for non-whitelisted els', function(done) {
+ var divEl = document.createElement('div');
+ element.appendChild(divEl);
+ element._handleKey = function(e) {
+ assert.isFalse(element.shouldSuppressKeyboardShortcut(e));
+ done();
+ };
+ MockInteractions.keyDownOn(divEl, 75, null, 'k');
});
+
+ test('blocks kb shortcuts for input els', function(done) {
+ var inputEl = document.createElement('input');
+ element.appendChild(inputEl);
+ element._handleKey = function(e) {
+ assert.isTrue(element.shouldSuppressKeyboardShortcut(e));
+ done();
+ };
+ MockInteractions.keyDownOn(inputEl, 75, null, 'k');
+ });
+
+ test('blocks kb shortcuts for textarea els', function(done) {
+ var textareaEl = document.createElement('textarea');
+ element.appendChild(textareaEl);
+ element._handleKey = function(e) {
+ assert.isTrue(element.shouldSuppressKeyboardShortcut(e));
+ done();
+ };
+ MockInteractions.keyDownOn(textareaEl, 75, null, 'k');
+ });
+
+ test('blocks kb shortcuts for anything in a gr-overlay', function(done) {
+ var divEl = document.createElement('div');
+ var element = overlay.querySelector('test-element');
+ element.appendChild(divEl);
+ element._handleKey = function(e) {
+ assert.isTrue(element.shouldSuppressKeyboardShortcut(e));
+ done();
+ };
+ MockInteractions.keyDownOn(divEl, 75, null, 'k');
+ });
+
});
</script>
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.js b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.js
index 2be0afc..51626cc 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.js
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.js
@@ -78,6 +78,12 @@
Gerrit.RESTClientBehavior,
],
+ keyBindings: {
+ 'j': '_handleJKey',
+ 'k': '_handleKKey',
+ 'o enter': '_handleEnterKey',
+ },
+
attached: function() {
this._loadPreferences();
},
@@ -149,31 +155,37 @@
account._account_id != change.owner._account_id;
},
- _handleKey: function(e) {
- if (this.shouldSuppressKeyboardShortcut(e)) { return; }
-
- if (this.groups == null) { return; }
+ _getAggregateGroupsLen: function(groups) {
+ groups = groups || [];
var len = 0;
this.groups.forEach(function(group) {
len += group.length;
});
- switch (e.keyCode) {
- case 74: // 'j'
- e.preventDefault();
- if (this.selectedIndex == len - 1) { return; }
- this.selectedIndex += 1;
- break;
- case 75: // 'k'
- e.preventDefault();
- if (this.selectedIndex == 0) { return; }
- this.selectedIndex -= 1;
- break;
- case 79: // 'o'
- case 13: // 'enter'
- e.preventDefault();
- page.show(this._changeURLForIndex(this.selectedIndex));
- break;
- }
+ return len;
+ },
+
+ _handleJKey: function(e) {
+ if (this.shouldSuppressKeyboardShortcut(e)) { return; }
+
+ e.preventDefault();
+ var len = this._getAggregateGroupsLen(this.groups);
+ if (this.selectedIndex === len - 1) { return; }
+ this.selectedIndex += 1;
+ },
+
+ _handleKKey: function(e) {
+ if (this.shouldSuppressKeyboardShortcut(e)) { return; }
+
+ e.preventDefault();
+ if (this.selectedIndex === 0) { return; }
+ this.selectedIndex -= 1;
+ },
+
+ _handleEnterKey: function(e) {
+ if (this.shouldSuppressKeyboardShortcut(e)) { return; }
+
+ e.preventDefault();
+ page.show(this._changeURLForIndex(this.selectedIndex));
},
_changeURLForIndex: function(index) {
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.html b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.html
index 718cfb5..33b4279 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.html
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.html
@@ -131,25 +131,25 @@
flush(function() {
assert.isTrue(elementItems[0].selected);
- MockInteractions.pressAndReleaseKeyOn(element, 74); // 'j'
+ MockInteractions.pressAndReleaseKeyOn(element, 74, null, 'j');
assert.equal(element.selectedIndex, 1);
- MockInteractions.pressAndReleaseKeyOn(element, 74); // 'j'
+ MockInteractions.pressAndReleaseKeyOn(element, 74, null, 'j');
var showStub = sinon.stub(page, 'show');
assert.equal(element.selectedIndex, 2);
- MockInteractions.pressAndReleaseKeyOn(element, 13); // 'enter'
+ MockInteractions.pressAndReleaseKeyOn(element, 13, null, 'enter');
assert(showStub.lastCall.calledWithExactly('/c/2/'),
'Should navigate to /c/2/');
- MockInteractions.pressAndReleaseKeyOn(element, 75); // 'k'
+ MockInteractions.pressAndReleaseKeyOn(element, 75, null, 'k');
assert.equal(element.selectedIndex, 1);
- MockInteractions.pressAndReleaseKeyOn(element, 13); // 'enter'
+ MockInteractions.pressAndReleaseKeyOn(element, 13, null, 'enter');
assert(showStub.lastCall.calledWithExactly('/c/1/'),
'Should navigate to /c/1/');
- MockInteractions.pressAndReleaseKeyOn(element, 75); // 'k'
- MockInteractions.pressAndReleaseKeyOn(element, 75); // 'k'
- MockInteractions.pressAndReleaseKeyOn(element, 75); // 'k'
+ MockInteractions.pressAndReleaseKeyOn(element, 75, null, 'k');
+ MockInteractions.pressAndReleaseKeyOn(element, 75, null, 'k');
+ MockInteractions.pressAndReleaseKeyOn(element, 75, null, 'k');
assert.equal(element.selectedIndex, 0);
showStub.restore();
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.html b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.html
index 30e9e86..9c591db 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.html
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.html
@@ -76,7 +76,7 @@
on-tap="_handleActionTap"></gr-button>
</template>
</section>
- <section hidden$="[[!_actionCount(_revisionActions.*, _additionalActions.*)]]">
+ <section hidden$="[[!_actionCount(revisionActions.*, _additionalActions.*)]]">
<template is="dom-repeat" items="[[_revisionActionValues]]" as="action">
<gr-button title$="[[action.title]]"
hidden$="[[_computeActionHidden(action.__key, _hiddenRevisionActions.*)]]"
@@ -98,7 +98,9 @@
hidden></gr-confirm-rebase-dialog>
<gr-confirm-cherrypick-dialog id="confirmCherrypick"
class="confirmDialog"
- message="[[commitMessage]]"
+ change-status="[[changeStatus]]"
+ commit-message="[[commitMessage]]"
+ commit-num="[[commitNum]]"
on-confirm="_handleCherrypickConfirm"
on-cancel="_handleConfirmDialogCancel"
hidden></gr-confirm-cherrypick-dialog>
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js
index ef2a3b4..2d5c0a7 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js
@@ -74,23 +74,25 @@
},
},
changeNum: String,
+ changeStatus: String,
+ commitNum: String,
patchNum: String,
commitMessage: {
type: String,
value: '',
},
+ revisionActions: {
+ type: Object,
+ value: function() { return {}; },
+ },
_loading: {
type: Boolean,
value: true,
},
- _revisionActions: {
- type: Object,
- value: function() { return {}; },
- },
_revisionActionValues: {
type: Array,
- computed: '_computeRevisionActionValues(_revisionActions.*, ' +
+ computed: '_computeRevisionActionValues(revisionActions.*, ' +
'primaryActionKeys.*, _additionalActions.*)',
},
_changeActionValues: {
@@ -121,7 +123,7 @@
],
observers: [
- '_actionsChanged(actions.*, _revisionActions.*, _additionalActions.*)',
+ '_actionsChanged(actions.*, revisionActions.*, _additionalActions.*)',
],
ready: function() {
@@ -137,7 +139,7 @@
return this._getRevisionActions().then(function(revisionActions) {
if (!revisionActions) { return; }
- this._revisionActions = revisionActions;
+ this.revisionActions = revisionActions;
this._loading = false;
}.bind(this)).catch(function(err) {
alert('Couldn’t load revision actions. Check the console ' +
@@ -363,7 +365,7 @@
/* falls through */ // required by JSHint
default:
this._fireAction(this._prependSlash(key),
- this._revisionActions[key], true);
+ this.revisionActions[key], true);
}
},
@@ -400,7 +402,7 @@
}
this.$.overlay.close();
el.hidden = true;
- this._fireAction('/rebase', this._revisionActions.rebase, true, payload);
+ this._fireAction('/rebase', this.revisionActions.rebase, true, payload);
},
_handleCherrypickConfirm: function() {
@@ -418,7 +420,7 @@
el.hidden = true;
this._fireAction(
'/cherrypick',
- this._revisionActions.cherrypick,
+ this.revisionActions.cherrypick,
true,
{
destination: el.branch,
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
index af666b0..15fcd8e 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
@@ -248,7 +248,10 @@
<gr-change-actions id="actions"
change="[[_change]]"
actions="[[_change.actions]]"
+ revision-actions="[[_currentRevisionActions]]"
change-num="[[_changeNum]]"
+ change-status="[[_change.status]]"
+ commit-num="[[_commitInfo.commit]]"
patch-num="[[_computeLatestPatchNum(_allPatchSets)]]"
commit-message="[[_latestCommitMessage]]"
on-reload-change="_handleReloadChange"></gr-change-actions>
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
index bbd4d2d..8ce963e 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
@@ -55,6 +55,7 @@
observer: '_changeChanged',
},
_commitInfo: Object,
+ _files: Object,
_changeNum: String,
_diffDrafts: {
type: Object,
@@ -77,6 +78,7 @@
type: Object,
observer: '_updateSelected',
},
+ _currentRevisionActions: Object,
_allPatchSets: {
type: Array,
computed: '_computeAllPatchSets(_change)',
@@ -110,6 +112,13 @@
'_paramsAndChangeChanged(params, _change)',
],
+ keyBindings: {
+ 'shift+r': '_handleCapitalRKey',
+ 'a': '_handleAKey',
+ 'd': '_handleDKey',
+ 'u': '_handleUKey',
+ },
+
attached: function() {
this._getLoggedIn().then(function(loggedIn) {
this._loggedIn = loggedIn;
@@ -579,30 +588,29 @@
}.bind(this));
},
- _handleKey: function(e) {
+ _handleAKey: function(e) {
if (this.shouldSuppressKeyboardShortcut(e)) { return; }
- switch (e.keyCode) {
- case 65: // 'a'
- if (this._loggedIn && !e.shiftKey) {
- e.preventDefault();
- this._openReplyDialog();
- }
- break;
- case 68: // 'd'
- e.preventDefault();
- this.$.downloadOverlay.open();
- break;
- case 82: // 'r'
- if (e.shiftKey) {
- e.preventDefault();
- this._switchToMostRecentPatchNum();
- }
- break;
- case 85: // 'u'
- e.preventDefault();
- this._determinePageBack();
- break;
- }
+ if (!this._loggedIn || e.detail.keyboardEvent.shiftKey) { return; }
+ e.preventDefault();
+ this._openReplyDialog();
+ },
+
+ _handleDKey: function(e) {
+ if (this.shouldSuppressKeyboardShortcut(e)) { return; }
+ e.preventDefault();
+ this.$.downloadOverlay.open();
+ },
+
+ _handleCapitalRKey: function(e) {
+ if (this.shouldSuppressKeyboardShortcut(e)) { return; }
+ e.preventDefault();
+ this._switchToMostRecentPatchNum();
+ },
+
+ _handleUKey: function(e) {
+ if (this.shouldSuppressKeyboardShortcut(e)) { return; }
+ e.preventDefault();
+ this._determinePageBack();
},
_determinePageBack: function() {
@@ -681,7 +689,16 @@
if (!change.reviewer_updates) {
change.reviewer_updates = null;
}
+ var currentRevision = change.revisions &&
+ change.revisions[change.current_revision];
+ this._latestCommitMessage = currentRevision.commit.message;
this._change = change;
+ if (!this._patchRange || !this._patchRange.patchNum ||
+ this._patchRange.patchNum === currentRevision._number) {
+ this._commitInfo = currentRevision.commit;
+ this._currentRevisionActions = currentRevision.actions;
+ // TODO: Fetch and process files.
+ }
}.bind(this));
},
@@ -728,41 +745,26 @@
var detailCompletes = this._getChangeDetail().then(function() {
this._loading = false;
+ this._getProjectConfig();
}.bind(this));
this._getComments();
if (this._patchRange.patchNum) {
- return this._reloadPatchNumDependentResources().then(function() {
- return detailCompletes;
- }).then(function() {
- return this._reloadDetailDependentResources();
+ return Promise.all([
+ this._reloadPatchNumDependentResources(),
+ detailCompletes,
+ ]).then(function() {
+ return this.$.actions.reload();
}.bind(this));
} else {
// The patch number is reliant on the change detail request.
return detailCompletes.then(function() {
- return this._reloadPatchNumDependentResources();
- }.bind(this)).then(function() {
- return this._reloadDetailDependentResources();
+ this.$.fileList.reload();
}.bind(this));
}
},
/**
- * Kicks off requests for resources that rely on the change detail
- * (`this._change`) being loaded.
- */
- _reloadDetailDependentResources: function() {
- if (!this._change) { return Promise.resolve(); }
-
- return this._getProjectConfig().then(function() {
- return Promise.all([
- this._getLatestCommitMessage(),
- this.$.actions.reload(),
- ]);
- }.bind(this));
- },
-
- /**
* Kicks off requests for resources that rely on the patch range
* (`this._patchRange`) being defined.
*/
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
index 7c8a329..c40062b 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
@@ -53,27 +53,27 @@
suite('keyboard shortcuts', function() {
test('U should navigate to / if no backPage set', function() {
var showStub = sandbox.stub(page, 'show');
- MockInteractions.pressAndReleaseKeyOn(element, 85); // 'U'
+ MockInteractions.pressAndReleaseKeyOn(element, 85, null, 'u');
assert(showStub.lastCall.calledWithExactly('/'));
});
test('U should navigate to backPage if set', function() {
element.backPage = '/dashboard/self';
var showStub = sandbox.stub(page, 'show');
- MockInteractions.pressAndReleaseKeyOn(element, 85); // 'U'
+ MockInteractions.pressAndReleaseKeyOn(element, 85, null, 'u');
assert(showStub.lastCall.calledWithExactly('/dashboard/self'));
});
test('A should toggle overlay', function() {
- MockInteractions.pressAndReleaseKeyOn(element, 65); // 'A'
+ MockInteractions.pressAndReleaseKeyOn(element, 65, null, 'a');
var overlayEl = element.$.replyOverlay;
assert.isFalse(overlayEl.opened);
element._loggedIn = true;
- MockInteractions.pressAndReleaseKeyOn(element, 65, 'shift'); // 'A'
+ MockInteractions.pressAndReleaseKeyOn(element, 65, 'shift', 'a');
assert.isFalse(overlayEl.opened);
- MockInteractions.pressAndReleaseKeyOn(element, 65); // 'A'
+ MockInteractions.pressAndReleaseKeyOn(element, 65, null, 'a');
assert.isTrue(overlayEl.opened);
overlayEl.close();
assert.isFalse(overlayEl.opened);
@@ -102,7 +102,7 @@
return Promise.resolve({
change_id: 'Iad9dc96274af6946f3632be53b106ef80f7ba6ca',
revisions: {
- rev1: {_number: 1},
+ rev1: {_number: 1, commit: {}},
rev13: {_number: 13},
},
current_revision: 'rev1',
@@ -117,13 +117,12 @@
done();
});
- // 'shift + R'
- MockInteractions.pressAndReleaseKeyOn(element, 82, 'shift');
+ MockInteractions.pressAndReleaseKeyOn(element, 82, 'shift', 'r');
});
test('d should open download overlay', function() {
var stub = sandbox.stub(element.$.downloadOverlay, 'open');
- MockInteractions.pressAndReleaseKeyOn(element, 68); // 'd'
+ MockInteractions.pressAndReleaseKeyOn(element, 68, null, 'd');
assert.isTrue(stub.called);
});
});
@@ -468,7 +467,12 @@
test('topic is coalesced to null', function(done) {
sandbox.stub(element, '_changeChanged');
sandbox.stub(element.$.restAPI, 'getChangeDetail', function() {
- return Promise.resolve({id: '123456789', labels: {}});
+ return Promise.resolve({
+ id: '123456789',
+ labels: {},
+ current_revision: 'foo',
+ revisions: {foo: {commit: {}}},
+ });
});
element._getChangeDetail().then(function() {
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-cherrypick-dialog/gr-confirm-cherrypick-dialog.html b/polygerrit-ui/app/elements/change/gr-confirm-cherrypick-dialog/gr-confirm-cherrypick-dialog.html
index ebc6533..5d2a9ac 100644
--- a/polygerrit-ui/app/elements/change/gr-confirm-cherrypick-dialog/gr-confirm-cherrypick-dialog.html
+++ b/polygerrit-ui/app/elements/change/gr-confirm-cherrypick-dialog/gr-confirm-cherrypick-dialog.html
@@ -74,7 +74,7 @@
autocomplete="on"
rows="4"
max-rows="15"
- bind-value="{{message}}"></iron-autogrow-textarea>
+ bind-value="{{_message}}"></iron-autogrow-textarea>
</div>
</gr-confirm-dialog>
</template>
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-cherrypick-dialog/gr-confirm-cherrypick-dialog.js b/polygerrit-ui/app/elements/change/gr-confirm-cherrypick-dialog/gr-confirm-cherrypick-dialog.js
index f27e4e2..f87af1c 100644
--- a/polygerrit-ui/app/elements/change/gr-confirm-cherrypick-dialog/gr-confirm-cherrypick-dialog.js
+++ b/polygerrit-ui/app/elements/change/gr-confirm-cherrypick-dialog/gr-confirm-cherrypick-dialog.js
@@ -31,7 +31,22 @@
properties: {
branch: String,
- message: String,
+ changeStatus: String,
+ commitMessage: String,
+ commitNum: String,
+ _message: {
+ type: String,
+ computed: '_computeMessage(changeStatus, commitNum, commitMessage)',
+ },
+ },
+
+ _computeMessage: function(changeStatus, commitNum, commitMessage) {
+ var newMessage = commitMessage;
+
+ if (changeStatus === 'MERGED') {
+ newMessage += '(cherry picked from commit ' + commitNum + ')';
+ }
+ return newMessage;
},
_handleConfirmTap: function(e) {
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-cherrypick-dialog/gr-confirm-cherrypick-dialog_test.html b/polygerrit-ui/app/elements/change/gr-confirm-cherrypick-dialog/gr-confirm-cherrypick-dialog_test.html
new file mode 100644
index 0000000..edf7d7a
--- /dev/null
+++ b/polygerrit-ui/app/elements/change/gr-confirm-cherrypick-dialog/gr-confirm-cherrypick-dialog_test.html
@@ -0,0 +1,61 @@
+<!DOCTYPE html>
+<!--
+Copyright (C) 2016 The Android Open Source Project
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+-->
+
+<meta name="viewport" content="width=device-width, minimum-scale=1.0, initial-scale=1.0, user-scalable=yes">
+<title>gr-confirm-cherrypick-dialog</title>
+
+<script src="../../../bower_components/webcomponentsjs/webcomponents.min.js"></script>
+<script src="../../../bower_components/web-component-tester/browser.js"></script>
+
+<link rel="import" href="../../../bower_components/iron-test-helpers/iron-test-helpers.html">
+<link rel="import" href="gr-confirm-cherrypick-dialog.html">
+
+<test-fixture id="basic">
+ <template>
+ <gr-confirm-cherrypick-dialog></gr-confirm-cherrypick-dialog>
+ </template>
+</test-fixture>
+
+<script>
+ suite('gr-confirm-cherrypick-dialog tests', function() {
+ var element;
+
+ setup(function() {
+ element = fixture('basic');
+ });
+
+ test('with merged change', function() {
+ element.changeStatus = 'MERGED';
+ element.commitMessage = 'message\n';
+ element.commitNum = '123';
+ element.branch = 'master';
+ flushAsynchronousOperations();
+ var expectedMessage = 'message\n(cherry picked from commit 123)';
+ assert.equal(element._message, expectedMessage);
+ });
+
+ test('with unmerged change', function() {
+ element.changeStatus = 'OPEN';
+ element.commitMessage = 'message\n';
+ element.commitNum = '123';
+ element.branch = 'master';
+ flushAsynchronousOperations();
+ var expectedMessage = 'message\n';
+ assert.equal(element._message, expectedMessage);
+ });
+ });
+</script>
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
index 873ab3b..4e68559 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
@@ -179,8 +179,7 @@
<select
id="modeSelect"
is="gr-select"
- bind-value="{{diffViewMode}}"
- on-change="_handleDropdownChange">
+ bind-value="{{diffViewMode}}">
<option value="SIDE_BY_SIDE">Side By Side</option>
<option value="UNIFIED_DIFF">Unified</option>
</select>
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
index 98c4284..ab889ac 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
@@ -106,6 +106,22 @@
Gerrit.URLEncodingBehavior,
],
+ keyBindings: {
+ 'shift+left': '_handleShiftLeftKey',
+ 'shift+right': '_handleShiftRightKey',
+ 'i': '_handleIKey',
+ 'shift+i': '_handleCapitalIKey',
+ 'down j': '_handleDownKey',
+ 'up k': '_handleUpKey',
+ 'c': '_handleCKey',
+ '[': '_handleLeftBracketKey',
+ ']': '_handleRightBracketKey',
+ 'o enter': '_handleEnterKey',
+ 'n': '_handleNKey',
+ 'p': '_handlePKey',
+ 'shift+a': '_handleCapitalAKey',
+ },
+
reload: function() {
if (!this.changeNum || !this.patchRange.patchNum) {
return Promise.resolve();
@@ -216,9 +232,6 @@
this.set(['_shownFiles', i, '__expanded'], true);
this.set(['_files', i, '__expanded'], true);
}
- if (e && e.target) {
- e.target.blur();
- }
},
_collapseAllDiffs: function(e) {
@@ -228,9 +241,6 @@
this.set(['_files', i, '__expanded'], false);
}
this.$.cursor.handleDiffUpdate();
- if (e && e.target) {
- e.target.blur();
- }
},
_computeCommentsString: function(comments, patchNum, path) {
@@ -298,113 +308,137 @@
});
},
- _handleKey: function(e) {
+ _handleShiftLeftKey: function(e) {
if (this.shouldSuppressKeyboardShortcut(e)) { return; }
- switch (e.keyCode) {
- case 37: // left
- if (e.shiftKey && this._showInlineDiffs) {
- e.preventDefault();
- this.$.cursor.moveLeft();
- }
- break;
- case 39: // right
- if (e.shiftKey && this._showInlineDiffs) {
- e.preventDefault();
- this.$.cursor.moveRight();
- }
- break;
- case 73: // 'i'
- if (e.shiftKey) {
- e.preventDefault();
- this._toggleInlineDiffs();
- } else if (this.selectedIndex !== undefined) {
- e.preventDefault();
- var expanded = this._files[this.selectedIndex].__expanded;
- // Until Polymer 2.0, manual management of reflection between _files
- // and _shownFiles is necessary.
- this.set(['_shownFiles', this.selectedIndex, '__expanded'],
- !expanded);
- this.set(['_files', this.selectedIndex, '__expanded'], !expanded);
- }
- break;
- case 40: // down
- case 74: // 'j'
- e.preventDefault();
- if (this._showInlineDiffs) {
- this.$.cursor.moveDown();
- } else {
- this.selectedIndex =
- Math.min(this._numFilesShown, this.selectedIndex + 1);
- this._scrollToSelectedFile();
- }
- break;
- case 38: // up
- case 75: // 'k'
- e.preventDefault();
- if (this._showInlineDiffs) {
- this.$.cursor.moveUp();
- } else {
- this.selectedIndex = Math.max(0, this.selectedIndex - 1);
- this._scrollToSelectedFile();
- }
- break;
- case 67: // 'c'
- var isRangeSelected = this.diffs.some(function(diff) {
- return diff.isRangeSelected();
- }, this);
- if (this._showInlineDiffs && !isRangeSelected) {
- e.preventDefault();
- this._addDraftAtTarget();
- }
- break;
- case 219: // '['
- e.preventDefault();
- this._openSelectedFile(this._files.length - 1);
- break;
- case 221: // ']'
- e.preventDefault();
- this._openSelectedFile(0);
- break;
- case 13: // <enter>
- case 79: // 'o'
- e.preventDefault();
- if (this._showInlineDiffs) {
- this._openCursorFile();
- } else {
- this._openSelectedFile();
- }
- break;
- case 78: // 'n'
- if (this._showInlineDiffs) {
- e.preventDefault();
- if (e.shiftKey) {
- this.$.cursor.moveToNextCommentThread();
- } else {
- this.$.cursor.moveToNextChunk();
- }
- }
- break;
- case 80: // 'p'
- if (this._showInlineDiffs) {
- e.preventDefault();
- if (e.shiftKey) {
- this.$.cursor.moveToPreviousCommentThread();
- } else {
- this.$.cursor.moveToPreviousChunk();
- }
- }
- break;
- case 65: // 'a'
- if (e.shiftKey) { // Hide left diff.
- e.preventDefault();
- this._forEachDiff(function(diff) {
- diff.toggleLeftDiff();
- });
- }
- break;
+ if (!this._showInlineDiffs) { return; }
+
+ e.preventDefault();
+ this.$.cursor.moveLeft();
+ },
+
+ _handleShiftRightKey: function(e) {
+ if (this.shouldSuppressKeyboardShortcut(e)) { return; }
+ if (!this._showInlineDiffs) { return; }
+
+ e.preventDefault();
+ this.$.cursor.moveRight();
+ },
+
+ _handleIKey: function(e) {
+ if (this.shouldSuppressKeyboardShortcut(e)) { return; }
+ if (this.selectedIndex === undefined) { return; }
+
+ e.preventDefault();
+ var expanded = this._files[this.selectedIndex].__expanded;
+ // Until Polymer 2.0, manual management of reflection between _files
+ // and _shownFiles is necessary.
+ this.set(['_shownFiles', this.selectedIndex, '__expanded'],
+ !expanded);
+ this.set(['_files', this.selectedIndex, '__expanded'], !expanded);
+ },
+
+ _handleCapitalIKey: function(e) {
+ if (this.shouldSuppressKeyboardShortcut(e)) { return; }
+
+ e.preventDefault();
+ this._toggleInlineDiffs();
+ },
+
+ _handleDownKey: function(e) {
+ if (this.shouldSuppressKeyboardShortcut(e)) { return; }
+
+ e.preventDefault();
+ if (this._showInlineDiffs) {
+ this.$.cursor.moveDown();
+ } else {
+ this.selectedIndex =
+ Math.min(this._numFilesShown, this.selectedIndex + 1);
+ this._scrollToSelectedFile();
}
},
+ _handleUpKey: function(e) {
+ if (this.shouldSuppressKeyboardShortcut(e)) { return; }
+
+ e.preventDefault();
+ if (this._showInlineDiffs) {
+ this.$.cursor.moveUp();
+ } else {
+ this.selectedIndex = Math.max(0, this.selectedIndex - 1);
+ this._scrollToSelectedFile();
+ }
+ },
+
+ _handleCKey: function(e) {
+ if (this.shouldSuppressKeyboardShortcut(e)) { return; }
+
+ var isRangeSelected = this.diffs.some(function(diff) {
+ return diff.isRangeSelected();
+ }, this);
+ if (this._showInlineDiffs && !isRangeSelected) {
+ e.preventDefault();
+ this._addDraftAtTarget();
+ }
+ },
+
+ _handleLeftBracketKey: function(e) {
+ if (this.shouldSuppressKeyboardShortcut(e)) { return; }
+
+ e.preventDefault();
+ this._openSelectedFile(this._files.length - 1);
+ },
+
+ _handleRightBracketKey: function(e) {
+ if (this.shouldSuppressKeyboardShortcut(e)) { return; }
+
+ e.preventDefault();
+ this._openSelectedFile(0);
+ },
+
+ _handleEnterKey: function(e) {
+ if (this.shouldSuppressKeyboardShortcut(e)) { return; }
+
+ e.preventDefault();
+ if (this._showInlineDiffs) {
+ this._openCursorFile();
+ } else {
+ this._openSelectedFile();
+ }
+ },
+
+ _handleNKey: function(e) {
+ if (this.shouldSuppressKeyboardShortcut(e)) { return; }
+ if (!this._showInlineDiffs) { return; }
+
+ e.preventDefault();
+ if (e.shiftKey) {
+ this.$.cursor.moveToNextCommentThread();
+ } else {
+ this.$.cursor.moveToNextChunk();
+ }
+ },
+
+ _handlePKey: function(e) {
+ if (this.shouldSuppressKeyboardShortcut(e)) { return; }
+ if (!this._showInlineDiffs) { return; }
+
+ e.preventDefault();
+ if (e.shiftKey) {
+ this.$.cursor.moveToPreviousCommentThread();
+ } else {
+ this.$.cursor.moveToPreviousChunk();
+ }
+ },
+
+ _handleCapitalAKey: function(e) {
+ if (this.shouldSuppressKeyboardShortcut(e)) { return; }
+
+ e.preventDefault();
+ this._forEachDiff(function(diff) {
+ diff.toggleLeftDiff();
+ });
+ },
+
_toggleInlineDiffs: function() {
if (this._showInlineDiffs) {
this._collapseAllDiffs();
@@ -555,10 +589,6 @@
return DiffViewMode.SIDE_BY_SIDE;
},
- _handleDropdownChange: function(e) {
- e.target.blur();
- },
-
_fileListActionsVisible: function(numFilesShown, maxFilesForBulkActions) {
return numFilesShown <= maxFilesForBulkActions;
},
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 b530bae..1e146b5 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
@@ -155,7 +155,7 @@
return [{toggleLeftDiff: toggleLeftDiffStub}];
},
});
- MockInteractions.pressAndReleaseKeyOn(element, 65, 'shift'); // 'A'
+ MockInteractions.pressAndReleaseKeyOn(element, 65, 'shift', 'a');
assert.isTrue(toggleLeftDiffStub.calledOnce);
diffsStub.restore();
});
@@ -168,25 +168,25 @@
assert.isTrue(elementItems[0].hasAttribute('selected'));
assert.isFalse(elementItems[1].hasAttribute('selected'));
assert.isFalse(elementItems[2].hasAttribute('selected'));
- MockInteractions.pressAndReleaseKeyOn(element, 74); // 'J'
+ MockInteractions.pressAndReleaseKeyOn(element, 74, null, 'j');
assert.equal(element.selectedIndex, 1);
- MockInteractions.pressAndReleaseKeyOn(element, 74); // 'J'
+ MockInteractions.pressAndReleaseKeyOn(element, 74, null, 'j');
var showStub = sandbox.stub(page, 'show');
assert.equal(element.selectedIndex, 2);
- MockInteractions.pressAndReleaseKeyOn(element, 13); // 'ENTER'
+ MockInteractions.pressAndReleaseKeyOn(element, 13, null, 'enter');
assert(showStub.lastCall.calledWith('/c/42/2/myfile.txt'),
'Should navigate to /c/42/2/myfile.txt');
- MockInteractions.pressAndReleaseKeyOn(element, 75); // 'K'
+ MockInteractions.pressAndReleaseKeyOn(element, 75, null, 'k');
assert.equal(element.selectedIndex, 1);
- MockInteractions.pressAndReleaseKeyOn(element, 79); // 'O'
+ MockInteractions.pressAndReleaseKeyOn(element, 79, null, 'o');
assert(showStub.lastCall.calledWith('/c/42/2/file_added_in_rev2.txt'),
'Should navigate to /c/42/2/file_added_in_rev2.txt');
- MockInteractions.pressAndReleaseKeyOn(element, 75); // 'K'
- MockInteractions.pressAndReleaseKeyOn(element, 75); // 'K'
- MockInteractions.pressAndReleaseKeyOn(element, 75); // 'K'
+ MockInteractions.pressAndReleaseKeyOn(element, 75, null, 'k');
+ MockInteractions.pressAndReleaseKeyOn(element, 75, null, 'k');
+ MockInteractions.pressAndReleaseKeyOn(element, 75, null, 'k');
assert.equal(element.selectedIndex, 0);
showStub.restore();
@@ -194,23 +194,23 @@
test('i key shows/hides selected inline diff', function() {
element.selectedIndex = 0;
- MockInteractions.pressAndReleaseKeyOn(element, 73); // 'I'
+ MockInteractions.pressAndReleaseKeyOn(element, 73, null, 'i');
flushAsynchronousOperations();
assert.isFalse(element.diffs[0].hasAttribute('hidden'));
- MockInteractions.pressAndReleaseKeyOn(element, 73); // 'I'
+ MockInteractions.pressAndReleaseKeyOn(element, 73, null, 'i');
flushAsynchronousOperations();
assert.isTrue(element.diffs[0].hasAttribute('hidden'));
element.selectedIndex = 1;
- MockInteractions.pressAndReleaseKeyOn(element, 73); // 'I'
+ MockInteractions.pressAndReleaseKeyOn(element, 73, null, 'i');
flushAsynchronousOperations();
assert.isFalse(element.diffs[1].hasAttribute('hidden'));
- MockInteractions.pressAndReleaseKeyOn(element, 73, 'shift'); // 'I'
+ MockInteractions.pressAndReleaseKeyOn(element, 73, 'shift', 'i');
flushAsynchronousOperations();
for (var index in element.diffs) {
assert.isFalse(element.diffs[index].hasAttribute('hidden'));
}
- MockInteractions.pressAndReleaseKeyOn(element, 73, 'shift'); // 'I'
+ MockInteractions.pressAndReleaseKeyOn(element, 73, 'shift', 'i');
flushAsynchronousOperations();
for (var index in element.diffs) {
assert.isTrue(element.diffs[index].hasAttribute('hidden'));
diff --git a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.js b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.js
index cee2a69..aa0703f 100644
--- a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.js
+++ b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.js
@@ -113,17 +113,11 @@
for (var i = 0; i < messageEls.length; i++) {
messageEls[i].expanded = this._expanded;
}
- if (e && e.target) {
- e.target.blur();
- }
},
_handleAutomatedMessageToggleTap: function(e) {
e.preventDefault();
this._hideAutomated = !this._hideAutomated;
- if (e && e.target) {
- e.target.blur();
- }
},
_handleScrollTo: function(e) {
diff --git a/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar.js b/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar.js
index 2c8549d..61eb280 100644
--- a/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar.js
+++ b/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar.js
@@ -94,6 +94,10 @@
'searchButton.tap': '_preventDefaultAndNavigateToInputVal',
},
+ keyBindings: {
+ '/': '_handleForwardSlashKey',
+ },
+
properties: {
value: {
type: String,
@@ -292,18 +296,12 @@
});
},
- _handleKey: function(e) {
+ _handleForwardSlashKey: function(e) {
if (this.shouldSuppressKeyboardShortcut(e)) { return; }
- switch (e.keyCode) {
- case 191: // '/' or '?' with shift key.
- // TODO(andybons): Localization using e.key/keypress event.
- if (e.shiftKey) { break; }
- e.preventDefault();
- var s = this.$.searchInput;
- s.focus();
- s.setSelectionRange(0, s.value.length);
- break;
- }
+
+ e.preventDefault();
+ this.$.searchInput.focus();
+ this.$.searchInput.selectAll();
},
});
})();
diff --git a/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar_test.html b/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar_test.html
index cd218d4..621511f 100644
--- a/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar_test.html
+++ b/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar_test.html
@@ -70,13 +70,15 @@
done();
});
element.value = 'test';
- MockInteractions.pressAndReleaseKeyOn(element.$.searchInput.$.input, 13);
+ MockInteractions.pressAndReleaseKeyOn(element.$.searchInput.$.input, 13,
+ null, 'enter');
});
test('search query should be double-escaped', function() {
var showStub = sinon.stub(page, 'show');
element.$.searchInput.text = 'fate/stay';
- MockInteractions.pressAndReleaseKeyOn(element.$.searchInput.$.input, 13);
+ MockInteractions.pressAndReleaseKeyOn(element.$.searchInput.$.input, 13,
+ null, 'enter');
assert.equal(showStub.lastCall.args[0], '/q/fate%252Fstay');
showStub.restore();
});
@@ -85,7 +87,8 @@
var showStub = sinon.stub(page, 'show');
var blurSpy = sinon.spy(element.$.searchInput.$.input, 'blur');
element.$.searchInput.text = 'fate/stay';
- MockInteractions.pressAndReleaseKeyOn(element.$.searchInput.$.input, 13);
+ MockInteractions.pressAndReleaseKeyOn(element.$.searchInput.$.input, 13,
+ null, 'enter');
assert.isTrue(blurSpy.called);
showStub.restore();
blurSpy.restore();
@@ -94,10 +97,19 @@
test('empty search query does not trigger nav', function() {
var showSpy = sinon.spy(page, 'show');
element.value = '';
- MockInteractions.pressAndReleaseKeyOn(element.$.searchInput.$.input, 13);
+ MockInteractions.pressAndReleaseKeyOn(element.$.searchInput.$.input, 13,
+ null, 'enter');
assert.isFalse(showSpy.called);
});
+ test('keyboard shortcuts', function() {
+ var focusSpy = sinon.spy(element.$.searchInput, 'focus');
+ var selectAllSpy = sinon.spy(element.$.searchInput, 'selectAll');
+ MockInteractions.pressAndReleaseKeyOn(document.body, 191, null, '/');
+ assert.isTrue(focusSpy.called);
+ assert.isTrue(selectAllSpy.called);
+ });
+
suite('_getSearchSuggestions',
function() {
setup(function() {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread.js b/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread.js
index 20b032d..a24100f 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread.js
@@ -57,6 +57,10 @@
'_commentsChanged(comments.splices)',
],
+ keyBindings: {
+ 'e shift+e': '_handleEKey',
+ },
+
attached: function() {
this._getLoggedIn().then(function(loggedIn) {
this._showActions = loggedIn;
@@ -88,12 +92,12 @@
this._orderedComments = this._sortedComments(this.comments);
},
- _handleKey: function(e) {
+ _handleEKey: function(e) {
if (this.shouldSuppressKeyboardShortcut(e)) { return; }
- if (e.keyCode === 69) { // 'e'
- e.preventDefault();
- this._expandCollapseComments(e.shiftKey);
- }
+
+ // Don’t preventDefault in this case because it will render the event
+ // useless for other handlers (other gr-diff-comment-thread elements).
+ this._expandCollapseComments(e.detail.keyboardEvent.shiftKey);
},
_expandCollapseComments: function(actionIsCollapse) {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread_test.html b/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread_test.html
index 73e4983..7b85b62 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread_test.html
@@ -280,10 +280,10 @@
}];
element.comments = comments;
var expandCollapseStub = sinon.stub(element, '_expandCollapseComments');
- MockInteractions.pressAndReleaseKeyOn(element, 69); // 'e'
+ MockInteractions.pressAndReleaseKeyOn(element, 69, null, 'e');
assert.isTrue(expandCollapseStub.lastCall.calledWith(false));
- MockInteractions.pressAndReleaseKeyOn(element, 69, 'shift'); // 'e'
+ MockInteractions.pressAndReleaseKeyOn(element, 69, 'shift', 'e');
assert.isTrue(expandCollapseStub.lastCall.calledWith(true));
expandCollapseStub.restore();
});
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.js b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.js
index 410a813..720e6542 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.js
@@ -279,34 +279,34 @@
},
_handleReply: function(e) {
- this._preventDefaultAndBlur(e);
+ e.preventDefault();
this.fire('reply', this._getEventPayload(), {bubbles: false});
},
_handleQuote: function(e) {
- this._preventDefaultAndBlur(e);
+ e.preventDefault();
this.fire(
'reply', this._getEventPayload({quote: true}), {bubbles: false});
},
_handleDone: function(e) {
- this._preventDefaultAndBlur(e);
+ e.preventDefault();
this.fire('done', this._getEventPayload(), {bubbles: false});
},
_handleEdit: function(e) {
- this._preventDefaultAndBlur(e);
+ e.preventDefault();
this._messageText = this.comment.message;
this.editing = true;
},
_handleSave: function(e) {
- this._preventDefaultAndBlur(e);
+ e.preventDefault();
this.save();
},
_handleCancel: function(e) {
- this._preventDefaultAndBlur(e);
+ e.preventDefault();
if (this.comment.message == null || this.comment.message.length == 0) {
this._fireDiscard();
return;
@@ -321,7 +321,7 @@
},
_handleDiscard: function(e) {
- this._preventDefaultAndBlur(e);
+ e.preventDefault();
if (!this.comment.__draft) {
throw Error('Cannot discard a non-draft comment.');
}
@@ -345,11 +345,6 @@
}.bind(this));
},
- _preventDefaultAndBlur: function(e) {
- e.preventDefault();
- Polymer.dom(e).rootTarget.blur();
- },
-
_saveDraft: function(draft) {
return this.$.restAPI.saveDiffDraft(this.changeNum, this.patchNum, draft);
},
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html
index 9dbb25b..b6471e7 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html
@@ -217,8 +217,7 @@
id="modeSelect"
is="gr-select"
bind-value="{{changeViewState.diffMode}}"
- hidden$="[[_computeModeSelectHidden(_isImageDiff)]]"
- on-change="_handleDropdownChange">
+ hidden$="[[_computeModeSelectHidden(_isImageDiff)]]">
<option value="SIDE_BY_SIDE">Side By Side</option>
<option value="UNIFIED_DIFF">Unified</option>
</select>
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
index 44204fd..1d63b47 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
@@ -100,6 +100,22 @@
'_getFiles(_changeNum, _patchRange.*)',
],
+ keyBindings: {
+ 'esc': '_handleEscKey',
+ 'shift+left': '_handleShiftLeftKey',
+ 'shift+right': '_handleShiftRightKey',
+ 'up k': '_handleUpKey',
+ 'down j': '_handleDownKey',
+ 'c': '_handleCKey',
+ '[': '_handleLeftBracketKey',
+ ']': '_handleRightBracketKey',
+ 'n shift+n': '_handleNKey',
+ 'p shift+p': '_handlePKey',
+ 'a shift+a': '_handleAKey',
+ 'u': '_handleUKey',
+ ',': '_handleCommaKey',
+ },
+
attached: function() {
this._getLoggedIn().then(function(loggedIn) {
this._loggedIn = loggedIn;
@@ -185,103 +201,129 @@
this._patchRange.patchNum, this._path, reviewed);
},
- _checkForModifiers: function(e) {
- return e.altKey || e.ctrlKey || e.metaKey || e.shiftKey || false;
- },
-
- _handleKey: function(e) {
+ _handleEscKey: function(e) {
if (this.shouldSuppressKeyboardShortcut(e)) { return; }
- switch (e.keyCode) {
- case 27: // escape
- e.preventDefault();
- this.$.diff.displayLine = false;
- break;
- case 37: // left
- if (e.shiftKey) {
- e.preventDefault();
- this.$.cursor.moveLeft();
- }
- break;
- case 39: // right
- if (e.shiftKey) {
- e.preventDefault();
- this.$.cursor.moveRight();
- }
- break;
- case 40: // down
- case 74: // 'j'
- e.preventDefault();
- this.$.diff.displayLine = true;
- this.$.cursor.moveDown();
- break;
- case 38: // up
- case 75: // 'k'
- e.preventDefault();
- this.$.diff.displayLine = true;
- this.$.cursor.moveUp();
- break;
- case 67: // 'c'
- if (this._checkForModifiers(e)) { return; }
- if (!this.$.diff.isRangeSelected()) {
- e.preventDefault();
- var line = this.$.cursor.getTargetLineElement();
- if (line) {
- this.$.diff.addDraftAtLine(line);
- }
- }
- break;
- case 219: // '['
- e.preventDefault();
- this._navToFile(this._path, this._fileList, -1);
- break;
- case 221: // ']'
- e.preventDefault();
- this._navToFile(this._path, this._fileList, 1);
- break;
- case 78: // 'n'
- e.preventDefault();
- if (e.shiftKey) {
- this.$.cursor.moveToNextCommentThread();
- } else {
- this.$.cursor.moveToNextChunk();
- }
- break;
- case 80: // 'p'
- e.preventDefault();
- if (e.shiftKey) {
- this.$.cursor.moveToPreviousCommentThread();
- } else {
- this.$.cursor.moveToPreviousChunk();
- }
- break;
- case 65: // 'a'
- if (e.shiftKey) { // Hide left diff.
- e.preventDefault();
- this.$.diff.toggleLeftDiff();
- break;
- }
+ e.preventDefault();
+ this.$.diff.displayLine = false;
+ },
- if (!this._loggedIn) { break; }
+ _handleShiftLeftKey: function(e) {
+ if (this.shouldSuppressKeyboardShortcut(e)) { return; }
- this.set('changeViewState.showReplyDialog', true);
- /* falls through */ // required by JSHint
- case 85: // 'u'
- if (this._changeNum && this._patchRange.patchNum) {
- e.preventDefault();
- page.show(this._getChangePath(
- this._changeNum,
- this._patchRange,
- this._change && this._change.revisions));
- }
- break;
- case 188: // ','
- e.preventDefault();
- this._openPrefs();
- break;
+ e.preventDefault();
+ this.$.cursor.moveLeft();
+ },
+
+ _handleShiftRightKey: function(e) {
+ if (this.shouldSuppressKeyboardShortcut(e)) { return; }
+
+ e.preventDefault();
+ this.$.cursor.moveRight();
+ },
+
+ _handleUpKey: function(e) {
+ if (this.shouldSuppressKeyboardShortcut(e)) { return; }
+
+ e.preventDefault();
+ this.$.diff.displayLine = true;
+ this.$.cursor.moveUp();
+ },
+
+ _handleDownKey: function(e) {
+ if (this.shouldSuppressKeyboardShortcut(e)) { return; }
+
+ e.preventDefault();
+ this.$.diff.displayLine = true;
+ this.$.cursor.moveDown();
+ },
+
+ _handleCKey: function(e) {
+ if (this.shouldSuppressKeyboardShortcut(e)) { return; }
+ if (this.$.diff.isRangeSelected()) { return; }
+
+ e.preventDefault();
+ var line = this.$.cursor.getTargetLineElement();
+ if (line) {
+ this.$.diff.addDraftAtLine(line);
}
},
+ _handleLeftBracketKey: function(e) {
+ if (this.shouldSuppressKeyboardShortcut(e)) { return; }
+
+ e.preventDefault();
+ this._navToFile(this._path, this._fileList, -1);
+ },
+
+ _handleRightBracketKey: function(e) {
+ if (this.shouldSuppressKeyboardShortcut(e)) { return; }
+
+ e.preventDefault();
+ this._navToFile(this._path, this._fileList, 1);
+ },
+
+ _handleNKey: function(e) {
+ if (this.shouldSuppressKeyboardShortcut(e)) { return; }
+
+ e.preventDefault();
+ if (e.detail.keyboardEvent.shiftKey) {
+ this.$.cursor.moveToNextCommentThread();
+ } else {
+ this.$.cursor.moveToNextChunk();
+ }
+ },
+
+ _handlePKey: function(e) {
+ if (this.shouldSuppressKeyboardShortcut(e)) { return; }
+
+ e.preventDefault();
+ if (e.detail.keyboardEvent.shiftKey) {
+ this.$.cursor.moveToPreviousCommentThread();
+ } else {
+ this.$.cursor.moveToPreviousChunk();
+ }
+ },
+
+ _handleAKey: function(e) {
+ if (this.shouldSuppressKeyboardShortcut(e)) { return; }
+
+ if (e.detail.keyboardEvent.shiftKey) { // Hide left diff.
+ e.preventDefault();
+ this.$.diff.toggleLeftDiff();
+ return;
+ }
+
+ if (!this._loggedIn) { return; }
+
+ this.set('changeViewState.showReplyDialog', true);
+ e.preventDefault();
+ this._navToChangeView();
+ },
+
+ _handleUKey: function(e) {
+ if (this.shouldSuppressKeyboardShortcut(e)) { return; }
+
+ e.preventDefault();
+ this._navToChangeView();
+ },
+
+ _handleCommaKey: function(e) {
+ if (this.shouldSuppressKeyboardShortcut(e)) { return; }
+
+ e.preventDefault();
+ this._openPrefs();
+ },
+
+ _navToChangeView: function() {
+ if (!this._changeNum || !this._patchRange.patchNum) { return; }
+
+ page.show(this._getChangePath(
+ this._changeNum,
+ this._patchRange,
+ this._change && this._change.revisions));
+ },
+
_navToFile: function(path, fileList, direction) {
var url = this._computeNavLinkURL(path, fileList, direction);
if (!url) { return; }
@@ -556,10 +598,6 @@
history.replaceState(null, null, '#' + this.$.cursor.getAddress());
},
- _handleDropdownChange: function(e) {
- e.target.blur();
- },
-
_computeDownloadLink: function(changeNum, patchRange, path) {
var url = this.changeBaseURL(changeNum, patchRange.patchNum);
url += '/patch?zip&path=' + encodeURIComponent(path);
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.html b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.html
index e423e45..c6ccb1b 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.html
@@ -62,7 +62,7 @@
test('toggle left diff with a hotkey', function() {
var toggleLeftDiffStub = sandbox.stub(element.$.diff, 'toggleLeftDiff');
- MockInteractions.pressAndReleaseKeyOn(element, 65, 'shift'); // 'a'
+ MockInteractions.pressAndReleaseKeyOn(element, 65, 'shift', 'a');
assert.isTrue(toggleLeftDiffStub.calledOnce);
});
@@ -82,29 +82,29 @@
element.changeViewState.selectedFileIndex = 1;
var showStub = sandbox.stub(page, 'show');
- MockInteractions.pressAndReleaseKeyOn(element, 85); // 'u'
+ MockInteractions.pressAndReleaseKeyOn(element, 85, null, 'u');
assert(showStub.lastCall.calledWithExactly('/c/42/'),
'Should navigate to /c/42/');
- MockInteractions.pressAndReleaseKeyOn(element, 221); // ']'
+ MockInteractions.pressAndReleaseKeyOn(element, 221, null, ']');
assert(showStub.lastCall.calledWithExactly('/c/42/10/wheatley.md'),
'Should navigate to /c/42/10/wheatley.md');
element._path = 'wheatley.md';
assert.equal(element.changeViewState.selectedFileIndex, 2);
- MockInteractions.pressAndReleaseKeyOn(element, 219); // '['
+ MockInteractions.pressAndReleaseKeyOn(element, 219, null, '[');
assert(showStub.lastCall.calledWithExactly('/c/42/10/glados.txt'),
'Should navigate to /c/42/10/glados.txt');
element._path = 'glados.txt';
assert.equal(element.changeViewState.selectedFileIndex, 1);
- MockInteractions.pressAndReleaseKeyOn(element, 219); // '['
+ MockInteractions.pressAndReleaseKeyOn(element, 219, null, '[');
assert(showStub.lastCall.calledWithExactly('/c/42/10/chell.go'),
'Should navigate to /c/42/10/chell.go');
element._path = 'chell.go';
assert.equal(element.changeViewState.selectedFileIndex, 0);
- MockInteractions.pressAndReleaseKeyOn(element, 219); // '['
+ MockInteractions.pressAndReleaseKeyOn(element, 219, null, '[');
assert(showStub.lastCall.calledWithExactly('/c/42/'),
'Should navigate to /c/42/');
assert.equal(element.changeViewState.selectedFileIndex, 0);
@@ -112,33 +112,33 @@
var showPrefsStub = sandbox.stub(element.$.prefsOverlay, 'open',
function() { return Promise.resolve({}); });
- MockInteractions.pressAndReleaseKeyOn(element, 188); // ','
+ MockInteractions.pressAndReleaseKeyOn(element, 188, null, ',');
assert(showPrefsStub.calledOnce);
var scrollStub = sandbox.stub(element.$.cursor, 'moveToNextChunk');
- MockInteractions.pressAndReleaseKeyOn(element, 78); // 'n'
+ MockInteractions.pressAndReleaseKeyOn(element, 78, null, 'n');
assert(scrollStub.calledOnce);
scrollStub = sandbox.stub(element.$.cursor, 'moveToPreviousChunk');
- MockInteractions.pressAndReleaseKeyOn(element, 80); // 'p'
+ MockInteractions.pressAndReleaseKeyOn(element, 80, null, 'p');
assert(scrollStub.calledOnce);
scrollStub = sandbox.stub(element.$.cursor, 'moveToNextCommentThread');
- MockInteractions.pressAndReleaseKeyOn(element, 78, ['shift']); // 'N'
+ MockInteractions.pressAndReleaseKeyOn(element, 78, 'shift', 'n');
assert(scrollStub.calledOnce);
scrollStub = sandbox.stub(element.$.cursor,
'moveToPreviousCommentThread');
- MockInteractions.pressAndReleaseKeyOn(element, 80, ['shift']); // 'P'
+ MockInteractions.pressAndReleaseKeyOn(element, 80, 'shift', 'p');
assert(scrollStub.calledOnce);
var computeContainerClassStub = sandbox.stub(element.$.diff,
'_computeContainerClass');
- MockInteractions.pressAndReleaseKeyOn(element, 74); // 'j'
+ MockInteractions.pressAndReleaseKeyOn(element, 74, null, 'j');
assert(computeContainerClassStub.lastCall.calledWithExactly(
false, 'SIDE_BY_SIDE', true));
- MockInteractions.pressAndReleaseKeyOn(element, 27); // 'escape'
+ MockInteractions.pressAndReleaseKeyOn(element, 27, null, 'esc');
assert(computeContainerClassStub.lastCall.calledWithExactly(
false, 'SIDE_BY_SIDE', false));
});
@@ -175,39 +175,39 @@
var showStub = sandbox.stub(page, 'show');
- MockInteractions.pressAndReleaseKeyOn(element, 65); // 'a'
+ MockInteractions.pressAndReleaseKeyOn(element, 65, null, 'a');
assert.isTrue(showStub.notCalled, 'The `a` keyboard shortcut should ' +
'only work when the user is logged in.');
assert.isNull(window.sessionStorage.getItem(
'changeView.showReplyDialog'));
element._loggedIn = true;
- MockInteractions.pressAndReleaseKeyOn(element, 65); // 'a'
+ MockInteractions.pressAndReleaseKeyOn(element, 65, null, 'a');
assert.isTrue(element.changeViewState.showReplyDialog);
assert(showStub.lastCall.calledWithExactly('/c/42/5..10'),
'Should navigate to /c/42/5..10');
- MockInteractions.pressAndReleaseKeyOn(element, 85); // 'u'
+ MockInteractions.pressAndReleaseKeyOn(element, 85, null, 'u');
assert(showStub.lastCall.calledWithExactly('/c/42/5..10'),
'Should navigate to /c/42/5..10');
- MockInteractions.pressAndReleaseKeyOn(element, 221); // ']'
+ MockInteractions.pressAndReleaseKeyOn(element, 221, null, ']');
assert(showStub.lastCall.calledWithExactly('/c/42/5..10/wheatley.md'),
'Should navigate to /c/42/5..10/wheatley.md');
element._path = 'wheatley.md';
- MockInteractions.pressAndReleaseKeyOn(element, 219); // '['
+ MockInteractions.pressAndReleaseKeyOn(element, 219, null, '[');
assert(showStub.lastCall.calledWithExactly('/c/42/5..10/glados.txt'),
'Should navigate to /c/42/5..10/glados.txt');
element._path = 'glados.txt';
- MockInteractions.pressAndReleaseKeyOn(element, 219); // '['
+ MockInteractions.pressAndReleaseKeyOn(element, 219, null, '[');
assert(showStub.lastCall.calledWithExactly('/c/42/5..10/chell.go'),
'Should navigate to /c/42/5..10/chell.go');
element._path = 'chell.go';
- MockInteractions.pressAndReleaseKeyOn(element, 219); // '['
+ MockInteractions.pressAndReleaseKeyOn(element, 219, null, '[');
assert(showStub.lastCall.calledWithExactly('/c/42/5..10'),
'Should navigate to /c/42/5..10');
});
@@ -229,39 +229,39 @@
var showStub = sandbox.stub(page, 'show');
- MockInteractions.pressAndReleaseKeyOn(element, 65); // 'a'
+ MockInteractions.pressAndReleaseKeyOn(element, 65, null, 'a');
assert.isTrue(showStub.notCalled, 'The `a` keyboard shortcut should ' +
'only work when the user is logged in.');
assert.isNull(window.sessionStorage.getItem(
'changeView.showReplyDialog'));
element._loggedIn = true;
- MockInteractions.pressAndReleaseKeyOn(element, 65); // 'a'
+ MockInteractions.pressAndReleaseKeyOn(element, 65, null, 'a');
assert.isTrue(element.changeViewState.showReplyDialog);
assert(showStub.lastCall.calledWithExactly('/c/42/1'),
'Should navigate to /c/42/1');
- MockInteractions.pressAndReleaseKeyOn(element, 85); // 'u'
+ MockInteractions.pressAndReleaseKeyOn(element, 85, null, 'u');
assert(showStub.lastCall.calledWithExactly('/c/42/1'),
'Should navigate to /c/42/1');
- MockInteractions.pressAndReleaseKeyOn(element, 221); // ']'
+ MockInteractions.pressAndReleaseKeyOn(element, 221, null, ']');
assert(showStub.lastCall.calledWithExactly('/c/42/1/wheatley.md'),
'Should navigate to /c/42/1/wheatley.md');
element._path = 'wheatley.md';
- MockInteractions.pressAndReleaseKeyOn(element, 219); // '['
+ MockInteractions.pressAndReleaseKeyOn(element, 219, null, '[');
assert(showStub.lastCall.calledWithExactly('/c/42/1/glados.txt'),
'Should navigate to /c/42/1/glados.txt');
element._path = 'glados.txt';
- MockInteractions.pressAndReleaseKeyOn(element, 219); // '['
+ MockInteractions.pressAndReleaseKeyOn(element, 219, null, '[');
assert(showStub.lastCall.calledWithExactly('/c/42/1/chell.go'),
'Should navigate to /c/42/1/chell.go');
element._path = 'chell.go';
- MockInteractions.pressAndReleaseKeyOn(element, 219); // '['
+ MockInteractions.pressAndReleaseKeyOn(element, 219, null, '[');
assert(showStub.lastCall.calledWithExactly('/c/42/1'),
'Should navigate to /c/42/1');
});
@@ -278,39 +278,39 @@
var showStub = sandbox.stub(page, 'show');
- MockInteractions.pressAndReleaseKeyOn(element, 65); // 'a'
+ MockInteractions.pressAndReleaseKeyOn(element, 65, null, 'a');
assert.isTrue(showStub.notCalled, 'The `a` keyboard shortcut should ' +
'only work when the user is logged in.');
assert.isNull(window.sessionStorage.getItem(
'changeView.showReplyDialog'));
element._loggedIn = true;
- MockInteractions.pressAndReleaseKeyOn(element, 65); // 'a'
+ MockInteractions.pressAndReleaseKeyOn(element, 65, null, 'a');
assert.isTrue(element.changeViewState.showReplyDialog);
assert(showStub.lastCall.calledWithExactly('/c/42/1'),
'Should navigate to /c/42/1');
- MockInteractions.pressAndReleaseKeyOn(element, 85); // 'u'
+ MockInteractions.pressAndReleaseKeyOn(element, 85, null, 'u');
assert(showStub.lastCall.calledWithExactly('/c/42/1'),
'Should navigate to /c/42/1');
- MockInteractions.pressAndReleaseKeyOn(element, 221); // ']'
+ MockInteractions.pressAndReleaseKeyOn(element, 221, null, ']');
assert(showStub.lastCall.calledWithExactly('/c/42/1/wheatley.md'),
'Should navigate to /c/42/1/wheatley.md');
element._path = 'wheatley.md';
- MockInteractions.pressAndReleaseKeyOn(element, 219); // '['
+ MockInteractions.pressAndReleaseKeyOn(element, 219, null, '[');
assert(showStub.lastCall.calledWithExactly('/c/42/1/glados.txt'),
'Should navigate to /c/42/1/glados.txt');
element._path = 'glados.txt';
- MockInteractions.pressAndReleaseKeyOn(element, 219); // '['
+ MockInteractions.pressAndReleaseKeyOn(element, 219, null, '[');
assert(showStub.lastCall.calledWithExactly('/c/42/1/chell.go'),
'Should navigate to /c/42/1/chell.go');
element._path = 'chell.go';
- MockInteractions.pressAndReleaseKeyOn(element, 219); // '['
+ MockInteractions.pressAndReleaseKeyOn(element, 219, null, '[');
assert(showStub.lastCall.calledWithExactly('/c/42/1'),
'Should navigate to /c/42/1');
});
@@ -457,7 +457,6 @@
test('diff mode selector correctly toggles the diff', function() {
var select = element.$.modeSelect;
var diffDisplay = element.$.diff;
- var blurSpy = sandbox.spy(select, 'blur');
element._userPrefs = {diff_view: 'SIDE_BY_SIDE'};
// The mode selected in the view state reflects the selected option.
@@ -477,7 +476,6 @@
assert.equal(element._getDiffViewMode(), newMode);
assert.equal(element._getDiffViewMode(), select.value);
assert.equal(element._getDiffViewMode(), diffDisplay.viewMode);
- assert(blurSpy.called, 'select should be blurred after selection');
});
test('diff mode selector initializes from preferences', function() {
@@ -557,14 +555,6 @@
assert.equal(element.$.cursor.side, 'left');
});
- test('_checkForModifiers', function() {
- assert.isTrue(element._checkForModifiers({altKey: true}));
- assert.isTrue(element._checkForModifiers({ctrlKey: true}));
- assert.isTrue(element._checkForModifiers({metaKey: true}));
- assert.isTrue(element._checkForModifiers({shiftKey: true}));
- assert.isFalse(element._checkForModifiers({}));
- });
-
test('_shortenPath with long path should add ellipsis', function() {
var path =
'level1/level2/level3/level4/file.js';
diff --git a/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box.js b/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box.js
index 568b5e0..c610073 100644
--- a/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box.js
+++ b/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box.js
@@ -51,6 +51,10 @@
'mousedown': '_handleMouseDown', // See https://crbug.com/gerrit/4767
},
+ keyBindings: {
+ 'c': '_handleCKey',
+ },
+
placeAbove: function(el) {
var rect = this._getTargetBoundingRect(el);
var boxRect = this.getBoundingClientRect();
@@ -74,17 +78,11 @@
return rect;
},
- _checkForModifiers: function(e) {
- return e.altKey || e.ctrlKey || e.metaKey || e.shiftKey || false;
- },
-
- _handleKey: function(e) {
+ _handleCKey: function(e) {
if (this.shouldSuppressKeyboardShortcut(e)) { return; }
- if (e.keyCode === 67) { // 'c'
- if (this._checkForModifiers(e)) { return; }
- e.preventDefault();
- this._fireCreateComment();
- }
+
+ e.preventDefault();
+ this._fireCreateComment();
},
_handleMouseDown: function(e) {
diff --git a/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box_test.html b/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box_test.html
index c12966d..79ff2a5 100644
--- a/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box_test.html
@@ -49,12 +49,12 @@
});
test('ignores regular keys', function() {
- MockInteractions.pressAndReleaseKeyOn(document.body, 27); // 'esc'
+ MockInteractions.pressAndReleaseKeyOn(document.body, 27, null, 'esc');
assert.isFalse(element.fire.called);
});
test('reacts to hotkey', function() {
- MockInteractions.pressAndReleaseKeyOn(document.body, 67); // 'c'
+ MockInteractions.pressAndReleaseKeyOn(document.body, 67, null, 'c');
assert.isTrue(element.fire.called);
});
@@ -68,7 +68,7 @@
};
element.side = 'left';
element.range = range;
- MockInteractions.pressAndReleaseKeyOn(document.body, 67); // 'c'
+ MockInteractions.pressAndReleaseKeyOn(document.body, 67, null, 'c');
assert(element.fire.calledWithExactly(
'create-comment',
{
@@ -117,13 +117,5 @@
document.createRange.restore();
});
});
-
- test('_checkForModifiers', function() {
- assert.isTrue(element._checkForModifiers({altKey: true}));
- assert.isTrue(element._checkForModifiers({ctrlKey: true}));
- assert.isTrue(element._checkForModifiers({metaKey: true}));
- assert.isTrue(element._checkForModifiers({shiftKey: true}));
- assert.isFalse(element._checkForModifiers({}));
- });
});
</script>
diff --git a/polygerrit-ui/app/elements/gr-app.js b/polygerrit-ui/app/elements/gr-app.js
index b758ff0..1ef6c2d 100644
--- a/polygerrit-ui/app/elements/gr-app.js
+++ b/polygerrit-ui/app/elements/gr-app.js
@@ -62,6 +62,10 @@
Gerrit.KeyboardShortcutBehavior,
],
+ keyBindings: {
+ '?': '_showKeyboardShortcuts',
+ },
+
attached: function() {
this.$.restAPI.getAccount().then(function(account) {
this._account = account;
@@ -194,12 +198,9 @@
}
},
- _handleKey: function(e) {
+ _showKeyboardShortcuts(e) {
if (this.shouldSuppressKeyboardShortcut(e)) { return; }
-
- if (e.keyCode === 191 && e.shiftKey) { // '/' or '?' with shift key.
- this.$.keyboardShortcuts.open();
- }
+ this.$.keyboardShortcuts.open();
},
_handleKeyboardShortcutDialogClose: function() {
diff --git a/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.html b/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.html
index 864114f..5de54bb 100644
--- a/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.html
+++ b/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.html
@@ -14,6 +14,7 @@
limitations under the License.
-->
<link rel="import" href="../../../bower_components/polymer/polymer.html">
+<link rel="import" href="../../../bower_components/iron-a11y-keys-behavior/iron-a11y-keys-behavior.html">
<link rel="import" href="../../../bower_components/iron-input/iron-input.html">
<link rel="import" href="../../shared/gr-cursor-manager/gr-cursor-manager.html">
diff --git a/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.js b/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.js
index 1c9e010..0626288 100644
--- a/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.js
+++ b/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.js
@@ -140,6 +140,10 @@
this.$.input.focus();
},
+ selectAll: function() {
+ this.$.input.setSelectionRange(0, this.$.input.value.length);
+ },
+
clear: function() {
this.text = '';
},
diff --git a/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete_test.html b/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete_test.html
index 03fa8e43..394b2c6 100644
--- a/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete_test.html
@@ -94,7 +94,7 @@
var cancelHandler = sinon.spy();
element.addEventListener('cancel', cancelHandler);
- MockInteractions.pressAndReleaseKeyOn(element.$.input, 27); // Esc
+ MockInteractions.pressAndReleaseKeyOn(element.$.input, 27, null, 'esc');
assert.isTrue(cancelHandler.called);
assert.isTrue(element.$.suggestions.hasAttribute('hidden'));
@@ -128,19 +128,22 @@
assert.equal(element.$.cursor.index, 0);
- MockInteractions.pressAndReleaseKeyOn(element.$.input, 40); // Down
+ MockInteractions.pressAndReleaseKeyOn(element.$.input, 40, null,
+ 'down');
assert.equal(element.$.cursor.index, 1);
- MockInteractions.pressAndReleaseKeyOn(element.$.input, 40); // Down
+ MockInteractions.pressAndReleaseKeyOn(element.$.input, 40, null,
+ 'down');
assert.equal(element.$.cursor.index, 2);
- MockInteractions.pressAndReleaseKeyOn(element.$.input, 38); // Up
+ MockInteractions.pressAndReleaseKeyOn(element.$.input, 38, null, 'up');
assert.equal(element.$.cursor.index, 1);
- MockInteractions.pressAndReleaseKeyOn(element.$.input, 13); // Enter
+ MockInteractions.pressAndReleaseKeyOn(element.$.input, 13, null,
+ 'enter');
assert.equal(element.value, 1);
assert.isTrue(commitHandler.called);
@@ -163,7 +166,8 @@
var commitHandler = sinon.spy();
element.addEventListener('commit', commitHandler);
- MockInteractions.pressAndReleaseKeyOn(element.$.input, 13); // Enter
+ MockInteractions.pressAndReleaseKeyOn(element.$.input, 13, null,
+ 'enter');
assert.isTrue(commitHandler.called);
assert.equal(element.text, 'suggestion');
@@ -184,7 +188,8 @@
var commitHandler = sinon.spy();
element.addEventListener('commit', commitHandler);
- MockInteractions.pressAndReleaseKeyOn(element.$.input, 13); // Enter
+ MockInteractions.pressAndReleaseKeyOn(element.$.input, 13, null,
+ 'enter');
assert.isTrue(commitHandler.called);
assert.equal(element.text, '');
@@ -234,7 +239,8 @@
var commitHandler = sinon.spy();
element.addEventListener('commit', commitHandler);
- MockInteractions.pressAndReleaseKeyOn(element.$.input, 13); // Enter
+ MockInteractions.pressAndReleaseKeyOn(element.$.input, 13, null,
+ 'enter');
assert.isTrue(commitHandler.called);
assert.equal(element.text, 'blah 0');
@@ -245,10 +251,10 @@
test('tab key completes only when suggestions exist', function() {
var commitStub = sinon.stub(element, '_commit');
element._suggestions = [];
- MockInteractions.pressAndReleaseKeyOn(element.$.input, 9); // tab
+ MockInteractions.pressAndReleaseKeyOn(element.$.input, 9, null, 'tab');
assert.isFalse(commitStub.called);
element._suggestions = ['tunnel snakes rule!'];
- MockInteractions.pressAndReleaseKeyOn(element.$.input, 9); // tab
+ MockInteractions.pressAndReleaseKeyOn(element.$.input, 9, null, 'tab');
assert.isTrue(commitStub.called);
commitStub.restore();
});
@@ -258,11 +264,11 @@
element.addEventListener('commit', commitHandler);
element._suggestions = ['tunnel snakes rule!'];
element.tabCompleteWithoutCommit = true;
- MockInteractions.pressAndReleaseKeyOn(element.$.input, 9); // tab
+ MockInteractions.pressAndReleaseKeyOn(element.$.input, 9, null, 'tab');
assert.isFalse(commitHandler.called);
element.tabCompleteWithoutCommit = false;
element._suggestions = ['tunnel snakes rule!'];
- MockInteractions.pressAndReleaseKeyOn(element.$.input, 9); // tab
+ MockInteractions.pressAndReleaseKeyOn(element.$.input, 9, null, 'tab');
assert.isTrue(commitHandler.called);
});
@@ -301,7 +307,7 @@
test('input-keydown event fired', function() {
var listener = sinon.spy();
element.addEventListener('input-keydown', listener);
- MockInteractions.pressAndReleaseKeyOn(element.$.input, 9); // tab
+ MockInteractions.pressAndReleaseKeyOn(element.$.input, 9, null, 'tab');
flushAsynchronousOperations();
assert.isTrue(listener.called);
});
diff --git a/polygerrit-ui/app/elements/shared/gr-button/gr-button.js b/polygerrit-ui/app/elements/shared/gr-button/gr-button.js
index e109896..01d2585 100644
--- a/polygerrit-ui/app/elements/shared/gr-button/gr-button.js
+++ b/polygerrit-ui/app/elements/shared/gr-button/gr-button.js
@@ -39,6 +39,10 @@
tabindex: '0',
},
+ keyBindings: {
+ 'space enter': '_handleCommitKey',
+ },
+
_disabledChanged: function(disabled) {
if (disabled) {
this._enabledTabindex = this.getAttribute('tabindex');
@@ -46,13 +50,9 @@
this.setAttribute('tabindex', disabled ? '-1' : this._enabledTabindex);
},
- _handleKey: function(e) {
- switch (e.keyCode) {
- case 32: // 'spacebar'
- case 13: // 'enter'
- e.preventDefault();
- this.click();
- }
+ _handleCommitKey: function(e) {
+ e.preventDefault();
+ this.click();
},
});
})();
diff --git a/polygerrit-ui/app/elements/shared/gr-overlay/gr-overlay.html b/polygerrit-ui/app/elements/shared/gr-overlay/gr-overlay.html
index 817d8c5..9aa80b5 100644
--- a/polygerrit-ui/app/elements/shared/gr-overlay/gr-overlay.html
+++ b/polygerrit-ui/app/elements/shared/gr-overlay/gr-overlay.html
@@ -16,7 +16,6 @@
<link rel="import" href="../../../bower_components/polymer/polymer.html">
<link rel="import" href="../../../bower_components/iron-overlay-behavior/iron-overlay-behavior.html">
-<link rel="import" href="../../../behaviors/keyboard-shortcut-behavior.html">
<dom-module id="gr-overlay">
<template>
diff --git a/polygerrit-ui/app/elements/shared/gr-overlay/gr-overlay.js b/polygerrit-ui/app/elements/shared/gr-overlay/gr-overlay.js
index f4a389a..9f271ed 100644
--- a/polygerrit-ui/app/elements/shared/gr-overlay/gr-overlay.js
+++ b/polygerrit-ui/app/elements/shared/gr-overlay/gr-overlay.js
@@ -24,28 +24,13 @@
Polymer.IronOverlayBehavior,
],
- detached: function() {
- Gerrit.KeyboardShortcutBehavior.enable(this._id());
- },
-
open: function() {
return new Promise(function(resolve) {
- Gerrit.KeyboardShortcutBehavior.disable(this._id());
Polymer.IronOverlayBehaviorImpl.open.apply(this, arguments);
this._awaitOpen(resolve);
}.bind(this));
},
- close: function() {
- Gerrit.KeyboardShortcutBehavior.enable(this._id());
- Polymer.IronOverlayBehaviorImpl.close.apply(this, arguments);
- },
-
- cancel: function() {
- Gerrit.KeyboardShortcutBehavior.enable(this._id());
- Polymer.IronOverlayBehaviorImpl.cancel.apply(this, arguments);
- },
-
/**
* Override the focus stops that iron-overlay-behavior tries to find.
*/
diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.html b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.html
index f34ffcf..b5c9f0c 100644
--- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.html
+++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.html
@@ -22,4 +22,3 @@
<dom-module id="gr-rest-api-interface">
<script src="gr-rest-api-interface.js"></script>
</dom-module>
-
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 96a8bca..07ccf2b 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
@@ -388,11 +388,13 @@
var options = this._listChangesOptionsToHex(
ListChangesOption.ALL_REVISIONS,
ListChangesOption.CHANGE_ACTIONS,
+ ListChangesOption.CURRENT_ACTIONS,
+ ListChangesOption.CURRENT_COMMIT,
ListChangesOption.DOWNLOAD_COMMANDS,
ListChangesOption.SUBMITTABLE
);
- return this._getChangeDetail(changeNum, options, opt_errFn,
- opt_cancelCondition);
+ return this._getChangeDetail(
+ changeNum, options, opt_errFn, opt_cancelCondition);
},
getDiffChangeDetail: function(changeNum, opt_errFn, opt_cancelCondition) {
