Merge "Set patchset description to 'Rebase' when rebasing"
diff --git a/Documentation/dev-intellij.txt b/Documentation/dev-intellij.txt
index f7e0987..1fe912a 100644
--- a/Documentation/dev-intellij.txt
+++ b/Documentation/dev-intellij.txt
@@ -1,7 +1,7 @@
= Gerrit Code Review - IntelliJ Setup
== Prerequisites
-You need an installation of IntelliJ of version 2016.2 or newer.
+You need an installation of IntelliJ of version 2016.2.
In addition, Java 8 must be specified on your path or via `JAVA_HOME` so that
building with Bazel via the Bazel plugin is possible.
diff --git a/gerrit-elasticsearch/BUILD b/gerrit-elasticsearch/BUILD
index 9323bf0..c0ffd5b 100644
--- a/gerrit-elasticsearch/BUILD
+++ b/gerrit-elasticsearch/BUILD
@@ -34,6 +34,7 @@
testonly = 1,
srcs = glob(["src/test/java/**/ElasticTestUtils.java"]),
deps = [
+ ":elasticsearch",
"//gerrit-extension-api:api",
"//gerrit-server:server",
"//lib:gson",
@@ -51,11 +52,9 @@
size = "large",
srcs = glob(["src/test/java/**/*Test.java"]),
flaky = 1,
- #TODO(davido): Re-enable the tests on the CI when they are fixed
tags = [
"elastic",
"flaky",
- "noci",
],
deps = [
":elasticsearch",
diff --git a/gerrit-elasticsearch/src/test/java/com/google/gerrit/elasticsearch/ElasticQueryAccountsTest.java b/gerrit-elasticsearch/src/test/java/com/google/gerrit/elasticsearch/ElasticQueryAccountsTest.java
index 62c1a57..68759df 100644
--- a/gerrit-elasticsearch/src/test/java/com/google/gerrit/elasticsearch/ElasticQueryAccountsTest.java
+++ b/gerrit-elasticsearch/src/test/java/com/google/gerrit/elasticsearch/ElasticQueryAccountsTest.java
@@ -14,11 +14,7 @@
package com.google.gerrit.elasticsearch;
-import static com.google.gerrit.elasticsearch.ElasticAccountIndex.ACCOUNTS_PREFIX;
-
-import com.google.gerrit.elasticsearch.ElasticAccountIndex.AccountMapping;
import com.google.gerrit.elasticsearch.ElasticTestUtils.ElasticNodeInfo;
-import com.google.gerrit.server.index.account.AccountSchemaDefinitions;
import com.google.gerrit.server.query.account.AbstractQueryAccountsTest;
import com.google.gerrit.testutil.InMemoryModule;
import com.google.inject.Guice;
@@ -32,9 +28,6 @@
import java.util.concurrent.ExecutionException;
public class ElasticQueryAccountsTest extends AbstractQueryAccountsTest {
- private static final String INDEX_NAME =
- String.format("%s%04d", ACCOUNTS_PREFIX,
- AccountSchemaDefinitions.INSTANCE.getLatest().getVersion());
private static ElasticNodeInfo nodeInfo;
@BeforeClass
@@ -45,21 +38,7 @@
return;
}
nodeInfo = ElasticTestUtils.startElasticsearchNode();
- createIndexes();
- }
-
- private static void createIndexes() {
- AccountMapping accountMapping =
- new AccountMapping(AccountSchemaDefinitions.INSTANCE.getLatest());
- nodeInfo.node
- .client()
- .admin()
- .indices()
- .prepareCreate(INDEX_NAME)
- .addMapping(ElasticAccountIndex.ACCOUNTS,
- ElasticTestUtils.gson.toJson(accountMapping))
- .execute()
- .actionGet();
+ ElasticTestUtils.createAllIndexes(nodeInfo);
}
@AfterClass
@@ -74,8 +53,8 @@
@After
public void cleanupIndex() {
if (nodeInfo != null) {
- ElasticTestUtils.deleteIndexes(nodeInfo.node, INDEX_NAME);
- createIndexes();
+ ElasticTestUtils.deleteAllIndexes(nodeInfo);
+ ElasticTestUtils.createAllIndexes(nodeInfo);
}
}
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 4b76e4b..95dbe5b 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
@@ -14,13 +14,7 @@
package com.google.gerrit.elasticsearch;
-import static com.google.gerrit.elasticsearch.ElasticChangeIndex.CHANGES_PREFIX;
-import static com.google.gerrit.elasticsearch.ElasticChangeIndex.CLOSED_CHANGES;
-import static com.google.gerrit.elasticsearch.ElasticChangeIndex.OPEN_CHANGES;
-
-import com.google.gerrit.elasticsearch.ElasticChangeIndex.ChangeMapping;
import com.google.gerrit.elasticsearch.ElasticTestUtils.ElasticNodeInfo;
-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;
@@ -37,9 +31,6 @@
import java.util.concurrent.ExecutionException;
public class ElasticQueryChangesTest extends AbstractQueryChangesTest {
- private static final String INDEX_NAME =
- String.format("%s%04d", CHANGES_PREFIX,
- ChangeSchemaDefinitions.INSTANCE.getLatest().getVersion());
private static ElasticNodeInfo nodeInfo;
@BeforeClass
@@ -51,14 +42,14 @@
}
nodeInfo = ElasticTestUtils.startElasticsearchNode();
- createIndexes();
+ ElasticTestUtils.createAllIndexes(nodeInfo);
}
@After
public void cleanupIndex() {
if (nodeInfo != null) {
- ElasticTestUtils.deleteIndexes(nodeInfo.node, INDEX_NAME);
- createIndexes();
+ ElasticTestUtils.deleteAllIndexes(nodeInfo);
+ ElasticTestUtils.createAllIndexes(nodeInfo);
}
}
@@ -80,26 +71,6 @@
new InMemoryModule(elasticsearchConfig, notesMigration));
}
- private static void createIndexes() {
- ChangeMapping openChangesMapping =
- new ChangeMapping(ChangeSchemaDefinitions.INSTANCE.getLatest());
- ChangeMapping closedChangesMapping =
- new ChangeMapping(ChangeSchemaDefinitions.INSTANCE.getLatest());
- openChangesMapping.closedChanges = null;
- closedChangesMapping.openChanges = null;
- nodeInfo.node
- .client()
- .admin()
- .indices()
- .prepareCreate(INDEX_NAME)
- .addMapping(OPEN_CHANGES,
- ElasticTestUtils.gson.toJson(openChangesMapping))
- .addMapping(CLOSED_CHANGES,
- ElasticTestUtils.gson.toJson(closedChangesMapping))
- .execute()
- .actionGet();
- }
-
@Test
public void byOwnerInvalidQuery() throws Exception {
TestRepository<Repo> repo = createProject("repo");
diff --git a/gerrit-elasticsearch/src/test/java/com/google/gerrit/elasticsearch/ElasticTestUtils.java b/gerrit-elasticsearch/src/test/java/com/google/gerrit/elasticsearch/ElasticTestUtils.java
index 7c26edc..9d6c808 100644
--- a/gerrit-elasticsearch/src/test/java/com/google/gerrit/elasticsearch/ElasticTestUtils.java
+++ b/gerrit-elasticsearch/src/test/java/com/google/gerrit/elasticsearch/ElasticTestUtils.java
@@ -15,10 +15,21 @@
package com.google.gerrit.elasticsearch;
import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.elasticsearch.ElasticAccountIndex.ACCOUNTS_PREFIX;
+import static com.google.gerrit.elasticsearch.ElasticChangeIndex.CHANGES_PREFIX;
+import static com.google.gerrit.elasticsearch.ElasticChangeIndex.CLOSED_CHANGES;
+import static com.google.gerrit.elasticsearch.ElasticChangeIndex.OPEN_CHANGES;
import com.google.common.base.Strings;
import com.google.common.io.Files;
+import com.google.gerrit.elasticsearch.ElasticAccountIndex.AccountMapping;
+import com.google.gerrit.elasticsearch.ElasticChangeIndex.ChangeMapping;
+import com.google.gerrit.server.account.AccountState;
import com.google.gerrit.server.index.IndexModule.IndexType;
+import com.google.gerrit.server.index.Schema;
+import com.google.gerrit.server.index.account.AccountSchemaDefinitions;
+import com.google.gerrit.server.index.change.ChangeSchemaDefinitions;
+import com.google.gerrit.server.query.change.ChangeData;
import com.google.gson.FieldNamingPolicy;
import com.google.gson.Gson;
import com.google.gson.GsonBuilder;
@@ -100,8 +111,8 @@
return new ElasticNodeInfo(node, elasticDir, getHttpPort(node));
}
- static void deleteIndexes(Node node, String index) {
- node.client().admin().indices().prepareDelete(index).execute();
+ static void deleteAllIndexes(ElasticNodeInfo nodeInfo) {
+ nodeInfo.node.client().admin().indices().prepareDelete("_all").execute();
}
static class NodeInfo {
@@ -112,6 +123,39 @@
Map<String, NodeInfo> nodes;
}
+ static void createAllIndexes(ElasticNodeInfo nodeInfo) {
+ Schema<ChangeData> changeSchema =
+ ChangeSchemaDefinitions.INSTANCE.getLatest();
+ ChangeMapping openChangesMapping = new ChangeMapping(changeSchema);
+ ChangeMapping closedChangesMapping = new ChangeMapping(changeSchema);
+ openChangesMapping.closedChanges = null;
+ closedChangesMapping.openChanges = null;
+ nodeInfo.node
+ .client()
+ .admin()
+ .indices()
+ .prepareCreate(
+ String.format("%s%04d", CHANGES_PREFIX, changeSchema.getVersion()))
+ .addMapping(OPEN_CHANGES, gson.toJson(openChangesMapping))
+ .addMapping(CLOSED_CHANGES, gson.toJson(closedChangesMapping))
+ .execute()
+ .actionGet();
+
+ Schema<AccountState> accountSchema =
+ AccountSchemaDefinitions.INSTANCE.getLatest();
+ AccountMapping accountMapping = new AccountMapping(accountSchema);
+ nodeInfo.node
+ .client()
+ .admin()
+ .indices()
+ .prepareCreate(
+ String.format(
+ "%s%04d", ACCOUNTS_PREFIX, accountSchema.getVersion()))
+ .addMapping(ElasticAccountIndex.ACCOUNTS, gson.toJson(accountMapping))
+ .execute()
+ .actionGet();
+ }
+
private static String getHttpPort(Node node)
throws InterruptedException, ExecutionException {
String nodes = node.client().admin().cluster()
diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/client/Comment.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/client/Comment.java
index f03b1f7..24e6094 100644
--- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/client/Comment.java
+++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/client/Comment.java
@@ -34,6 +34,7 @@
public String inReplyTo;
public Timestamp updated;
public String message;
+ public boolean unresolved;
public static class Range {
public int startLine;
@@ -101,7 +102,8 @@
&& Objects.equals(range, c.range)
&& Objects.equals(inReplyTo, c.inReplyTo)
&& Objects.equals(updated, c.updated)
- && Objects.equals(message, c.message);
+ && Objects.equals(message, c.message)
+ && Objects.equals(unresolved, c.unresolved);
}
return false;
}
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/LocalComments.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/LocalComments.java
index f6022f9..44316bc 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/LocalComments.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/LocalComments.java
@@ -216,7 +216,7 @@
} else {
line = Integer.parseInt(elements[offset + 4]);
}
- CommentInfo info = CommentInfo.create(path, side, line, range);
+ CommentInfo info = CommentInfo.create(path, side, line, range, false);
info.message(storage.getItem(key));
if (key.startsWith("patchReply-")) {
info.inReplyTo(elements[1]);
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/CommentInfo.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/CommentInfo.java
index d42c344..2800b0b 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/CommentInfo.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/CommentInfo.java
@@ -24,12 +24,12 @@
public class CommentInfo extends JavaScriptObject {
public static CommentInfo create(String path, Side side,
- int line, CommentRange range) {
- return create(path, side, 0, line, range);
+ int line, CommentRange range, Boolean unresolved) {
+ return create(path, side, 0, line, range, unresolved);
}
public static CommentInfo create(String path, Side side, int parent,
- int line, CommentRange range) {
+ int line, CommentRange range, boolean unresolved) {
CommentInfo n = createObject().cast();
n.path(path);
n.side(side);
@@ -40,6 +40,7 @@
} else if (line > 0) {
n.line(line);
}
+ n.unresolved(unresolved);
return n;
}
@@ -55,6 +56,7 @@
} else if (r.hasLine()) {
n.line(r.line());
}
+ n.unresolved(r.unresolved());
return n;
}
@@ -72,6 +74,7 @@
} else if (s.hasLine()) {
n.line(s.line());
}
+ n.unresolved(s.unresolved());
return n;
}
@@ -81,6 +84,7 @@
public final native void range(CommentRange r) /*-{ this.range = r }-*/;
public final native void inReplyTo(String i) /*-{ this.in_reply_to = i }-*/;
public final native void message(String m) /*-{ this.message = m }-*/;
+ public final native void unresolved(boolean b) /*-{ this.unresolved = b }-*/;
public final void side(Side side) {
sideRaw(side.toString());
@@ -93,6 +97,7 @@
public final native String id() /*-{ return this.id }-*/;
public final native String inReplyTo() /*-{ return this.in_reply_to }-*/;
public final native int patchSet() /*-{ return this.patch_set }-*/;
+ public final native boolean unresolved() /*-{ return this.unresolved }-*/;
public final Side side() {
String s = sideRaw();
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/CommentManager.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/CommentManager.java
index 4c01941..1d1ead9 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/CommentManager.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/CommentManager.java
@@ -196,7 +196,8 @@
getStoredSideFromDisplaySide(side),
getParentNumFromDisplaySide(side),
line,
- null)).setEdit(true);
+ null,
+ false)).setEdit(true);
}
}
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SideBySideCommentManager.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SideBySideCommentManager.java
index fab6e6b..1981ed0 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SideBySideCommentManager.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SideBySideCommentManager.java
@@ -87,7 +87,8 @@
getStoredSideFromDisplaySide(cm.side()),
getParentNumFromDisplaySide(cm.side()),
line,
- CommentRange.create(fromTo))).setEdit(true);
+ CommentRange.create(fromTo),
+ false)).setEdit(true);
cm.setCursor(fromTo.to());
cm.setSelection(cm.getCursor());
} else {
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/UnifiedCommentManager.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/UnifiedCommentManager.java
index 21356fc..cc23aca 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/UnifiedCommentManager.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/UnifiedCommentManager.java
@@ -176,7 +176,8 @@
getPath(),
getStoredSideFromDisplaySide(side),
to.line() + 1,
- CommentRange.create(fromTo))).setEdit(true);
+ CommentRange.create(fromTo),
+ false)).setEdit(true);
cm.setCursor(Pos.create(host.getCmLine(to.line(), side), to.ch()));
cm.setSelection(cm.getCursor());
} else {
diff --git a/gerrit-plugin-api/BUCK b/gerrit-plugin-api/BUCK
index 4b2d677..a949d03 100644
--- a/gerrit-plugin-api/BUCK
+++ b/gerrit-plugin-api/BUCK
@@ -57,6 +57,7 @@
'//lib/guice:javax-inject',
'//lib/guice:multibindings',
'//lib/guice:guice-servlet',
+ "//lib/httpcomponents:httpclient",
'//lib/jgit/org.eclipse.jgit:jgit',
'//lib/jgit/org.eclipse.jgit.http.server:jgit-servlet',
'//lib/joda:joda-time',
diff --git a/gerrit-plugin-api/BUILD b/gerrit-plugin-api/BUILD
index 78f065b..c117eeb 100644
--- a/gerrit-plugin-api/BUILD
+++ b/gerrit-plugin-api/BUILD
@@ -26,6 +26,7 @@
"//lib/guice:guice-servlet",
"//lib/guice:javax-inject",
"//lib/guice:multibindings",
+ "//lib/httpcomponents:httpclient",
"//lib/jgit/org.eclipse.jgit.http.server:jgit-servlet",
"//lib/jgit/org.eclipse.jgit:jgit",
"//lib/joda:joda-time",
@@ -45,7 +46,7 @@
"//lib:jsch",
"//lib:mime-util",
"//lib:protobuf",
- "//lib:servlet-api-3_1",
+ "//lib:servlet-api-3_1-without-neverlink",
"//lib:soy",
"//lib:velocity",
]
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/CommentsUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/CommentsUtil.java
index aac8c66..5d160d9 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/CommentsUtil.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/CommentsUtil.java
@@ -148,7 +148,7 @@
Comment c = new Comment(
new Comment.Key(ChangeUtil.messageUUID(ctx.getDb()), path, psId.get()),
ctx.getUser().getAccountId(), ctx.getWhen(), side, message, serverId,
- true);
+ false);
ctx.getUser().updateRealAccountId(c::setRealAuthor);
return c;
}
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 d7c0110..4ad9f15 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
@@ -66,6 +66,7 @@
import com.google.gerrit.common.data.SubmitTypeRecord;
import com.google.gerrit.extensions.api.changes.FixInput;
import com.google.gerrit.extensions.client.ListChangesOption;
+import com.google.gerrit.extensions.client.ReviewerState;
import com.google.gerrit.extensions.common.AccountInfo;
import com.google.gerrit.extensions.common.ApprovalInfo;
import com.google.gerrit.extensions.common.ChangeInfo;
@@ -517,7 +518,6 @@
? permittedLabels(ctl, cd)
: ImmutableMap.of();
}
- out.removableReviewers = removableReviewers(ctl, out.labels.values());
out.reviewers = new HashMap<>();
for (Map.Entry<ReviewerStateInternal, Map<Account.Id, Timestamp>> e
@@ -525,6 +525,8 @@
out.reviewers.put(e.getKey().asReviewerState(),
toAccountInfo(e.getValue().keySet()));
}
+
+ out.removableReviewers = removableReviewers(ctl, out);
}
if (has(REVIEWER_UPDATES)) {
@@ -1030,7 +1032,18 @@
}
private Collection<AccountInfo> removableReviewers(ChangeControl ctl,
- Collection<LabelInfo> labels) {
+ ChangeInfo out) {
+ // Although this is called removableReviewers, this method also determines
+ // which CCs are removable.
+ //
+ // For reviewers, we need to look at each approval, because the reviewer
+ // should only be considered removable if *all* of their approvals can be
+ // removed. First, add all reviewers with *any* removable approval to the
+ // "removable" set. Along the way, if we encounter a non-removable approval,
+ // add the reviewer to the "fixed" set. Before we return, remove all members
+ // of "fixed" from "removable", because not all of their approvals can be
+ // removed.
+ Collection<LabelInfo> labels = out.labels.values();
Set<Account.Id> fixed = Sets.newHashSetWithExpectedSize(labels.size());
Set<Account.Id> removable = Sets.newHashSetWithExpectedSize(labels.size());
for (LabelInfo label : labels) {
@@ -1046,6 +1059,24 @@
}
}
}
+
+ // CCs are simpler than reviewers. They are removable if the ChangeControl
+ // would permit a non-negative approval by that account to be removed, in
+ // which case add them to removable. We don't need to add unremovable CCs to
+ // "fixed" because we only visit each CC once here.
+ Collection<AccountInfo> ccs = out.reviewers.get(ReviewerState.CC);
+ if (ccs != null) {
+ for (AccountInfo ai : ccs) {
+ Account.Id id = new Account.Id(ai._accountId);
+ if (ctl.canRemoveReviewer(id, 0)) {
+ removable.add(id);
+ }
+ }
+ }
+
+ // Subtract any reviewers with non-removable approvals from the "removable"
+ // set. This also subtracts any CCs that for some reason also hold
+ // unremovable approvals.
removable.removeAll(fixed);
List<AccountInfo> result = Lists.newArrayListWithCapacity(removable.size());
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/CommentJson.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/CommentJson.java
index 82d932c..cd5195f 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/CommentJson.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/CommentJson.java
@@ -18,6 +18,7 @@
import com.google.common.base.Strings;
import com.google.common.collect.FluentIterable;
+import com.google.gerrit.common.Nullable;
import com.google.gerrit.extensions.client.Comment.Range;
import com.google.gerrit.extensions.client.Side;
import com.google.gerrit.extensions.common.CommentInfo;
@@ -147,6 +148,7 @@
r.updated = c.writtenOn;
r.range = toRange(c.range);
r.tag = c.tag;
+ r.unresolved = c.unresolved;
if (loader != null) {
r.author = loader.get(c.author.getId());
}
@@ -192,8 +194,8 @@
}
private List<FixSuggestionInfo> toFixSuggestionInfos(
- List<FixSuggestion> fixSuggestions) {
- if (fixSuggestions.isEmpty()) {
+ @Nullable List<FixSuggestion> fixSuggestions) {
+ if (fixSuggestions == null || fixSuggestions.isEmpty()) {
return null;
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/EmailReviewComments.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/EmailReviewComments.java
index 2c65e4d..9994085 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/EmailReviewComments.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/EmailReviewComments.java
@@ -17,6 +17,7 @@
import static com.google.gerrit.server.CommentsUtil.COMMENT_ORDER;
import com.google.common.collect.Multimap;
+import com.google.gerrit.common.Nullable;
import com.google.gerrit.extensions.api.changes.NotifyHandling;
import com.google.gerrit.extensions.api.changes.RecipientType;
import com.google.gerrit.reviewdb.client.Account;
@@ -30,6 +31,7 @@
import com.google.gerrit.server.mail.send.CommentSender;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.patch.PatchSetInfoFactory;
+import com.google.gerrit.server.util.LabelVote;
import com.google.gerrit.server.util.RequestContext;
import com.google.gerrit.server.util.ThreadLocalRequestContext;
import com.google.gwtorm.server.OrmException;
@@ -56,7 +58,9 @@
PatchSet patchSet,
IdentifiedUser user,
ChangeMessage message,
- List<Comment> comments);
+ List<Comment> comments,
+ String patchSetComment,
+ List<LabelVote> labels);
}
private final ExecutorService sendEmailsExecutor;
@@ -72,6 +76,8 @@
private final IdentifiedUser user;
private final ChangeMessage message;
private final List<Comment> comments;
+ private final String patchSetComment;
+ private final List<LabelVote> labels;
private ReviewDb db;
@Inject
@@ -87,7 +93,9 @@
@Assisted PatchSet patchSet,
@Assisted IdentifiedUser user,
@Assisted ChangeMessage message,
- @Assisted List<Comment> comments) {
+ @Assisted List<Comment> comments,
+ @Nullable @Assisted String patchSetComment,
+ @Assisted List<LabelVote> labels) {
this.sendEmailsExecutor = executor;
this.patchSetInfoFactory = patchSetInfoFactory;
this.commentSenderFactory = commentSenderFactory;
@@ -100,6 +108,8 @@
this.user = user;
this.message = message;
this.comments = COMMENT_ORDER.sortedCopy(comments);
+ this.patchSetComment = patchSetComment;
+ this.labels = labels;
}
void sendAsync() {
@@ -118,6 +128,8 @@
patchSetInfoFactory.get(notes.getProjectName(), patchSet));
cm.setChangeMessage(message.getMessage(), message.getWrittenOn());
cm.setComments(comments);
+ cm.setPatchSetComment(patchSetComment);
+ cm.setLabels(labels);
cm.setNotify(notify);
cm.setAccountsToNotify(accountsToNotify);
cm.send();
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 21783af..b71d771 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
@@ -658,7 +658,7 @@
private PatchSet ps;
private ChangeMessage message;
private List<Comment> comments = new ArrayList<>();
- private List<String> labelDelta = new ArrayList<>();
+ private List<LabelVote> labelDelta = new ArrayList<>();
private Map<String, Short> approvals = new HashMap<>();
private Map<String, Short> oldApprovals = new HashMap<>();
@@ -699,7 +699,9 @@
ps,
user,
message,
- comments).sendAsync();
+ comments,
+ in.message,
+ labelDelta).sendAsync();
}
commentAdded.fire(
notes.getChange(), ps, user.getAccount(), message.getMessage(),
@@ -1198,8 +1200,8 @@
String msg = Strings.nullToEmpty(in.message).trim();
StringBuilder buf = new StringBuilder();
- for (String d : labelDelta) {
- buf.append(" ").append(d);
+ for (LabelVote d : labelDelta) {
+ buf.append(" ").append(d.format());
}
if (comments.size() == 1) {
buf.append("\n\n(1 comment)");
@@ -1221,7 +1223,7 @@
}
private void addLabelDelta(String name, short value) {
- labelDelta.add(LabelVote.create(name, value).format());
+ labelDelta.add(LabelVote.create(name, value));
}
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/CommentSender.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/CommentSender.java
index 0cc4152..a4d0e24 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/CommentSender.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/CommentSender.java
@@ -31,6 +31,7 @@
import com.google.gerrit.server.patch.PatchFile;
import com.google.gerrit.server.patch.PatchList;
import com.google.gerrit.server.patch.PatchListNotAvailableException;
+import com.google.gerrit.server.util.LabelVote;
import com.google.gwtorm.client.KeyUtil;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
@@ -103,6 +104,8 @@
}
private List<Comment> inlineComments = Collections.emptyList();
+ private String patchSetComment = null;
+ private List<LabelVote> labels = Collections.emptyList();
private final CommentsUtil commentsUtil;
@Inject
@@ -126,6 +129,14 @@
changeData.setCurrentFilePaths(Ordering.natural().sortedCopy(paths));
}
+ public void setPatchSetComment(String comment) {
+ this.patchSetComment = comment;
+ }
+
+ public void setLabels(List<LabelVote> labels) {
+ this.labels = labels;
+ }
+
@Override
protected void init() throws EmailException {
super.init();
@@ -595,6 +606,10 @@
hasComments = !files.isEmpty();
}
+ soyContext.put("patchSetCommentBlocks",
+ commentBlocksToSoyData(CommentFormatter.parse(patchSetComment)));
+ soyContext.put("labels", getLabelVoteSoyData(labels));
+ soyContext.put("commentCount", inlineComments.size());
soyContext.put("commentTimestamp", getCommentTimestamp());
soyContext.put("coverLetterBlocks",
commentBlocksToSoyData(CommentFormatter.parse(getCoverLetter())));
@@ -623,6 +638,20 @@
}
}
+ private List<Map<String, Object>> getLabelVoteSoyData(List<LabelVote> votes) {
+ List<Map<String, Object>> result = new ArrayList<>();
+ for (LabelVote vote : votes) {
+ Map<String, Object> data = new HashMap<>();
+ data.put("label", vote.label());
+
+ // Soy needs the short to be cast as an int for it to get converted to the
+ // correct tamplate type.
+ data.put("value", (int) vote.value());
+ result.add(data);
+ }
+ return result;
+ }
+
private String getCommentTimestamp() {
// Grouping is currently done by timestamp.
return MailUtil.rfcDateformatter.format(
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNoteUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNoteUtil.java
index ff5e8ee..ce73142 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNoteUtil.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNoteUtil.java
@@ -249,9 +249,16 @@
boolean hasParent =
(RawParseUtils.match(note, curr.value, PARENT.getBytes(UTF_8))) != -1;
String parentUUID = null;
+ boolean unresolved = false;
if (hasParent) {
parentUUID = parseStringField(note, curr, changeId, PARENT);
}
+ boolean hasUnresolved =
+ (RawParseUtils.match(note, curr.value,
+ UNRESOLVED.getBytes(UTF_8))) != -1;
+ if (hasUnresolved) {
+ unresolved = parseBooleanField(note, curr, changeId, UNRESOLVED);
+ }
String uuid = parseStringField(note, curr, changeId, UUID);
@@ -277,7 +284,7 @@
: (short) 1,
message,
serverId,
- false);
+ unresolved);
c.lineNbr = range.getEndLine();
c.parentUuid = parentUUID;
c.tag = tag;
@@ -458,6 +465,17 @@
return checkResult(commentLength, "comment length", changeId);
}
+ private boolean parseBooleanField(byte[] note, MutableInteger curr,
+ Change.Id changeId, String fieldName) throws ConfigInvalidException {
+ String str = parseStringField(note, curr, changeId, fieldName);
+ if ("true".equalsIgnoreCase(str)) {
+ return true;
+ } else if ("false".equalsIgnoreCase(str)) {
+ return false;
+ }
+ throw parseException(changeId, "invalid boolean for %s: %s", fieldName, str);
+ }
+
private static <T> T checkResult(T o, String fieldName,
Change.Id changeId) throws ConfigInvalidException {
if (o == null) {
@@ -590,6 +608,7 @@
appendHeaderField(writer, PARENT, parent);
}
+ appendHeaderField(writer, UNRESOLVED, Boolean.toString(c.unresolved));
appendHeaderField(writer, UUID, c.key.uuid);
if (c.tag != null) {
diff --git a/gerrit-server/src/main/resources/com/google/gerrit/server/mail/CommentHtml.soy b/gerrit-server/src/main/resources/com/google/gerrit/server/mail/CommentHtml.soy
index 75492d6..4e67d4e 100644
--- a/gerrit-server/src/main/resources/com/google/gerrit/server/mail/CommentHtml.soy
+++ b/gerrit-server/src/main/resources/com/google/gerrit/server/mail/CommentHtml.soy
@@ -18,10 +18,12 @@
/**
* @param commentFiles
- * @param coverLetter
- * @param coverLetterBlocks
+ * @param commentCount
* @param email
* @param fromName
+ * @param labels
+ * @param patchSet
+ * @param patchSetCommentBlocks
*/
{template .CommentHtml autoescape="strict" kind="html"}
{let $commentHeaderStyle kind="css"}
@@ -39,6 +41,28 @@
padding-left: 20px;
{/let}
+ {let $voteStyle kind="css"}
+ border-radius: 3px;
+ display: inline-block;
+ margin: 0 2px;
+ padding: 4px;
+ {/let}
+
+ {let $positiveVoteStyle kind="css"}
+ {$voteStyle}
+ background-color: #d4ffd4;
+ {/let}
+
+ {let $negativeVoteStyle kind="css"}
+ {$voteStyle}
+ background-color: #ffd4d4;
+ {/let}
+
+ {let $neutralVoteStyle kind="css"}
+ {$voteStyle}
+ background-color: #ddd;
+ {/let}
+
<p>
{$fromName} <strong>posted comments</strong> on this change.
</p>
@@ -49,8 +73,33 @@
</p>
{/if}
- {if $coverLetter}
- {call .WikiFormat}{param content: $coverLetterBlocks /}{/call}
+ <p>
+ Patch set {$patchSet.patchSetId}:
+ {foreach $label in $labels}
+ {if $label.value > 0}
+ <span style="{$positiveVoteStyle}">
+ {$label.label}{sp}+{$label.value}
+ </span>
+ {elseif $label.value < 0}
+ <span style="{$negativeVoteStyle}">
+ {$label.label}{sp}{$label.value}
+ </span>
+ {else}
+ <span style="{$neutralVoteStyle}">
+ -{$label.label}
+ </span>
+ {/if}
+ {/foreach}
+ </p>
+
+ {if $patchSetCommentBlocks}
+ {call .WikiFormat}{param content: $patchSetCommentBlocks /}{/call}
+ {/if}
+
+ {if $commentCount == 1}
+ <p>(1 comment)</p>
+ {elseif $commentCount > 1}
+ <p>({$commentCount} comments)</p>
{/if}
<ul style="{$ulStyle}">
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/mail/receive/AbstractParserTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/mail/receive/AbstractParserTest.java
index 8b6e55e..dc1c054 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/mail/receive/AbstractParserTest.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/mail/receive/AbstractParserTest.java
@@ -59,7 +59,7 @@
protected static Comment newComment(String uuid, String file,
String message, int line) {
Comment c = new Comment(new Comment.Key(uuid, file, 1),
- new Account.Id(0), new Timestamp(0l), (short) 0, message, "", false);
+ new Account.Id(0), new Timestamp(0L), (short) 0, message, "", false);
c.lineNbr = line;
return c;
}
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java
index fdf58d1..d827e6c 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java
@@ -251,7 +251,8 @@
protected Comment newComment(PatchSet.Id psId, String filename, String UUID,
CommentRange range, int line, IdentifiedUser commenter, String parentUUID,
- Timestamp t, String message, short side, String commitSHA1) {
+ Timestamp t, String message, short side, String commitSHA1,
+ boolean unresolved) {
Comment c = new Comment(
new Comment.Key(UUID, filename, psId.get()),
commenter.getAccountId(),
@@ -259,7 +260,7 @@
side,
message,
serverId,
- false);
+ unresolved);
c.lineNbr = line;
c.parentUuid = parentUUID;
c.revId = commitSHA1;
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java
index a648276..f43eeb3 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java
@@ -128,7 +128,7 @@
update.putComment(Status.PUBLISHED,
newComment(c.currentPatchSetId(), "a.txt", "uuid1",
new CommentRange(1, 2, 3, 4), 1, changeOwner, null,
- TimeUtil.nowTs(), "Comment", (short) 1, commit.name()));
+ TimeUtil.nowTs(), "Comment", (short) 1, commit.name(), false));
update.setTag(tag);
update.commit();
@@ -183,7 +183,7 @@
update.putComment(Status.PUBLISHED,
newComment(c.currentPatchSetId(), "a.txt", "uuid1",
new CommentRange(1, 2, 3, 4), 1, changeOwner, null,
- TimeUtil.nowTs(), "Comment", (short) 1, commit.name()));
+ TimeUtil.nowTs(), "Comment", (short) 1, commit.name(), false));
update.setChangeMessage("coverage verification");
update.setTag(coverageTag);
update.commit();
@@ -1039,7 +1039,7 @@
update.putComment(Status.PUBLISHED,
newComment(c.currentPatchSetId(), "a.txt", "uuid1",
new CommentRange(1, 2, 3, 4), 1, changeOwner, null,
- TimeUtil.nowTs(), "Comment", (short) 1, commit.name()));
+ TimeUtil.nowTs(), "Comment", (short) 1, commit.name(), false));
update.commit();
ChangeNotes notes = newNotes(c);
@@ -1137,7 +1137,7 @@
Timestamp ts = TimeUtil.nowTs();
update.putComment(Status.PUBLISHED,
newComment(psId2, "a.txt", "uuid1", new CommentRange(1, 2, 3, 4), 1,
- changeOwner, null, ts, "Comment", (short) 1, commit.name()));
+ changeOwner, null, ts, "Comment", (short) 1, commit.name(), false));
update.commit();
notes = newNotes(c);
@@ -1157,6 +1157,7 @@
+ "1:2-3:4\n"
+ ChangeNoteUtil.formatTime(serverIdent, ts) + "\n"
+ "Author: Change Owner <1@gerrit>\n"
+ + "Unresolved: false\n"
+ "UUID: uuid1\n"
+ "Bytes: 7\n"
+ "Comment\n"
@@ -1217,7 +1218,7 @@
updateManagerFactory.create(project)) {
Comment comment1 = newComment(psId, "file1",
uuid1, range1, range1.getEndLine(), otherUser, null, time1, message1,
- (short) 0, "abcd1234abcd1234abcd1234abcd1234abcd1234");
+ (short) 0, "abcd1234abcd1234abcd1234abcd1234abcd1234", false);
update1.setPatchSetId(psId);
update1.putComment(Status.PUBLISHED, comment1);
updateManager.add(update1);
@@ -1444,7 +1445,7 @@
Comment comment = newComment(psId, "file1",
"uuid", null, 0, otherUser, null,
- TimeUtil.nowTs(), "message", (short) 1, revId.get());
+ TimeUtil.nowTs(), "message", (short) 1, revId.get(), false);
update.setPatchSetId(psId);
update.putComment(Status.PUBLISHED, comment);
update.commit();
@@ -1464,7 +1465,7 @@
Comment comment = newComment(psId, "file1",
"uuid", range, range.getEndLine(), otherUser, null,
- TimeUtil.nowTs(), "message", (short) 1, revId.get());
+ TimeUtil.nowTs(), "message", (short) 1, revId.get(), false);
update.setPatchSetId(psId);
update.putComment(Status.PUBLISHED, comment);
update.commit();
@@ -1484,7 +1485,7 @@
Comment comment = newComment(psId, "file",
"uuid", range, range.getEndLine(), otherUser, null,
- TimeUtil.nowTs(), "message", (short) 1, revId.get());
+ TimeUtil.nowTs(), "message", (short) 1, revId.get(), false);
update.setPatchSetId(psId);
update.putComment(Status.PUBLISHED, comment);
update.commit();
@@ -1503,7 +1504,8 @@
CommentRange range = new CommentRange(1, 2, 3, 4);
Comment comment = newComment(psId, "", "uuid", range, range.getEndLine(),
- otherUser, null, TimeUtil.nowTs(), "message", (short) 1, revId.get());
+ otherUser, null, TimeUtil.nowTs(), "message", (short) 1, revId.get(),
+ false);
update.setPatchSetId(psId);
update.putComment(Status.PUBLISHED, comment);
update.commit();
@@ -1531,7 +1533,7 @@
Comment comment1 = newComment(psId, "file1", uuid1, range1,
range1.getEndLine(), otherUser, null, time1, message1, (short) 1,
- "abcd1234abcd1234abcd1234abcd1234abcd1234");
+ "abcd1234abcd1234abcd1234abcd1234abcd1234", false);
update.setPatchSetId(psId);
update.putComment(Status.PUBLISHED, comment1);
update.commit();
@@ -1540,7 +1542,7 @@
CommentRange range2 = new CommentRange(2, 1, 3, 1);
Comment comment2 = newComment(psId, "file1", uuid2, range2,
range2.getEndLine(), otherUser, null, time2, message2, (short) 1,
- "abcd1234abcd1234abcd1234abcd1234abcd1234");
+ "abcd1234abcd1234abcd1234abcd1234abcd1234", false);
update.setPatchSetId(psId);
update.putComment(Status.PUBLISHED, comment2);
update.commit();
@@ -1549,7 +1551,7 @@
CommentRange range3 = new CommentRange(3, 0, 4, 1);
Comment comment3 = newComment(psId, "file2", uuid3, range3,
range3.getEndLine(), otherUser, null, time3, message3, (short) 1,
- "abcd1234abcd1234abcd1234abcd1234abcd1234");
+ "abcd1234abcd1234abcd1234abcd1234abcd1234", false);
update.setPatchSetId(psId);
update.putComment(Status.PUBLISHED, comment3);
update.commit();
@@ -1575,6 +1577,7 @@
+ "1:1-2:1\n"
+ ChangeNoteUtil.formatTime(serverIdent, time1) + "\n"
+ "Author: Other Account <2@gerrit>\n"
+ + "Unresolved: false\n"
+ "UUID: uuid1\n"
+ "Bytes: 9\n"
+ "comment 1\n"
@@ -1582,6 +1585,7 @@
+ "2:1-3:1\n"
+ ChangeNoteUtil.formatTime(serverIdent, time2) + "\n"
+ "Author: Other Account <2@gerrit>\n"
+ + "Unresolved: false\n"
+ "UUID: uuid2\n"
+ "Bytes: 9\n"
+ "comment 2\n"
@@ -1591,6 +1595,7 @@
+ "3:0-4:1\n"
+ ChangeNoteUtil.formatTime(serverIdent, time3) + "\n"
+ "Author: Other Account <2@gerrit>\n"
+ + "Unresolved: false\n"
+ "UUID: uuid3\n"
+ "Bytes: 9\n"
+ "comment 3\n"
@@ -1614,7 +1619,7 @@
Comment comment1 = newComment(psId, "file1",
uuid1, range1, range1.getEndLine(), otherUser, null, time1, message1,
- (short) 0, "abcd1234abcd1234abcd1234abcd1234abcd1234");
+ (short) 0, "abcd1234abcd1234abcd1234abcd1234abcd1234", false);
update.setPatchSetId(psId);
update.putComment(Status.PUBLISHED, comment1);
update.commit();
@@ -1623,7 +1628,7 @@
CommentRange range2 = new CommentRange(2, 1, 3, 1);
Comment comment2 = newComment(psId, "file1",
uuid2, range2, range2.getEndLine(), otherUser, null, time2, message2,
- (short) 0, "abcd1234abcd1234abcd1234abcd1234abcd1234");
+ (short) 0, "abcd1234abcd1234abcd1234abcd1234abcd1234", false);
update.setPatchSetId(psId);
update.putComment(Status.PUBLISHED, comment2);
update.commit();
@@ -1649,6 +1654,7 @@
+ "1:1-2:1\n"
+ ChangeNoteUtil.formatTime(serverIdent, time1) + "\n"
+ "Author: Other Account <2@gerrit>\n"
+ + "Unresolved: false\n"
+ "UUID: uuid1\n"
+ "Bytes: 9\n"
+ "comment 1\n"
@@ -1656,6 +1662,74 @@
+ "2:1-3:1\n"
+ ChangeNoteUtil.formatTime(serverIdent, time2) + "\n"
+ "Author: Other Account <2@gerrit>\n"
+ + "Unresolved: false\n"
+ + "UUID: uuid2\n"
+ + "Bytes: 9\n"
+ + "comment 2\n"
+ + "\n");
+ }
+ }
+ }
+
+ @Test
+ public void patchLineCommentNotesResolvedChangesValue() throws Exception {
+ Change c = newChange();
+ ChangeUpdate update = newUpdate(c, otherUser);
+ String uuid1 = "uuid1";
+ String uuid2 = "uuid2";
+ String message1 = "comment 1";
+ String message2 = "comment 2";
+ CommentRange range1 = new CommentRange(1, 1, 2, 1);
+ Timestamp time1 = TimeUtil.nowTs();
+ Timestamp time2 = TimeUtil.nowTs();
+ PatchSet.Id psId = c.currentPatchSetId();
+
+ Comment comment1 = newComment(psId, "file1",
+ uuid1, range1, range1.getEndLine(), otherUser, null, time1, message1,
+ (short) 0, "abcd1234abcd1234abcd1234abcd1234abcd1234", false);
+ update.setPatchSetId(psId);
+ update.putComment(Status.PUBLISHED, comment1);
+ update.commit();
+
+ update = newUpdate(c, otherUser);
+ Comment comment2 = newComment(psId, "file1",
+ uuid2, range1, range1.getEndLine(), otherUser, uuid1, time2, message2,
+ (short) 0, "abcd1234abcd1234abcd1234abcd1234abcd1234", true);
+ update.setPatchSetId(psId);
+ update.putComment(Status.PUBLISHED, comment2);
+ update.commit();
+
+ ChangeNotes notes = newNotes(c);
+
+ try (RevWalk walk = new RevWalk(repo)) {
+ ArrayList<Note> notesInTree =
+ Lists.newArrayList(notes.revisionNoteMap.noteMap.iterator());
+ Note note = Iterables.getOnlyElement(notesInTree);
+
+ byte[] bytes =
+ walk.getObjectReader().open(
+ note.getData(), Constants.OBJ_BLOB).getBytes();
+ String noteString = new String(bytes, UTF_8);
+
+ if (!testJson()) {
+ assertThat(noteString).isEqualTo(
+ "Revision: abcd1234abcd1234abcd1234abcd1234abcd1234\n"
+ + "Base-for-patch-set: 1\n"
+ + "File: file1\n"
+ + "\n"
+ + "1:1-2:1\n"
+ + ChangeNoteUtil.formatTime(serverIdent, time1) + "\n"
+ + "Author: Other Account <2@gerrit>\n"
+ + "Unresolved: false\n"
+ + "UUID: uuid1\n"
+ + "Bytes: 9\n"
+ + "comment 1\n"
+ + "\n"
+ + "1:1-2:1\n"
+ + ChangeNoteUtil.formatTime(serverIdent, time2) + "\n"
+ + "Author: Other Account <2@gerrit>\n"
+ + "Parent: uuid1\n"
+ + "Unresolved: true\n"
+ "UUID: uuid2\n"
+ "Bytes: 9\n"
+ "comment 2\n"
@@ -1684,13 +1758,13 @@
Comment comment1 =
newComment(psId1, "file1", uuid1, range1, range1.getEndLine(),
- otherUser, null, time, message1, (short) 0, revId.get());
+ otherUser, null, time, message1, (short) 0, revId.get(), false);
Comment comment2 =
newComment(psId1, "file1", uuid2, range2, range2.getEndLine(),
- otherUser, null, time, message2, (short) 0, revId.get());
+ otherUser, null, time, message2, (short) 0, revId.get(), false);
Comment comment3 =
newComment(psId2, "file1", uuid3, range1, range1.getEndLine(),
- otherUser, null, time, message3, (short) 0, revId.get());
+ otherUser, null, time, message3, (short) 0, revId.get(), false);
ChangeUpdate update = newUpdate(c, otherUser);
update.setPatchSetId(psId2);
@@ -1721,6 +1795,7 @@
+ "1:1-2:1\n"
+ timeStr + "\n"
+ "Author: Other Account <2@gerrit>\n"
+ + "Unresolved: false\n"
+ "UUID: uuid1\n"
+ "Bytes: 9\n"
+ "comment 1\n"
@@ -1728,6 +1803,7 @@
+ "2:1-3:1\n"
+ timeStr + "\n"
+ "Author: Other Account <2@gerrit>\n"
+ + "Unresolved: false\n"
+ "UUID: uuid2\n"
+ "Bytes: 9\n"
+ "comment 2\n"
@@ -1738,6 +1814,7 @@
+ "1:1-2:1\n"
+ timeStr + "\n"
+ "Author: Other Account <2@gerrit>\n"
+ + "Unresolved: false\n"
+ "UUID: uuid3\n"
+ "Bytes: 9\n"
+ "comment 3\n"
@@ -1766,7 +1843,7 @@
Comment comment = newComment(psId, "file", uuid, range,
range.getEndLine(), otherUser, null, time, message, (short) 1,
- revId.get());
+ revId.get(), false);
comment.setRealAuthor(changeOwner.getAccountId());
update.setPatchSetId(psId);
update.putComment(Status.PUBLISHED, comment);
@@ -1794,6 +1871,7 @@
+ ChangeNoteUtil.formatTime(serverIdent, time) + "\n"
+ "Author: Other Account <2@gerrit>\n"
+ "Real-author: Change Owner <1@gerrit>\n"
+ + "Unresolved: false\n"
+ "UUID: uuid\n"
+ "Bytes: 7\n"
+ "comment\n"
@@ -1821,7 +1899,7 @@
Comment comment = newComment(psId, "file1", uuid, range, range.getEndLine(),
user, null, time, "comment", (short) 1,
- "abcd1234abcd1234abcd1234abcd1234abcd1234");
+ "abcd1234abcd1234abcd1234abcd1234abcd1234", false);
update.setPatchSetId(psId);
update.putComment(Status.PUBLISHED, comment);
update.commit();
@@ -1848,6 +1926,7 @@
+ "1:1-2:1\n"
+ timeStr + "\n"
+ "Author: Weird\u0002User <3@gerrit>\n"
+ + "Unresolved: false\n"
+ "UUID: uuid\n"
+ "Bytes: 7\n"
+ "comment\n"
@@ -1875,7 +1954,7 @@
Comment commentForBase =
newComment(psId, "filename", uuid1, range, range.getEndLine(),
- otherUser, null, now, messageForBase, (short) 0, rev1);
+ otherUser, null, now, messageForBase, (short) 0, rev1, false);
update.setPatchSetId(psId);
update.putComment(Status.PUBLISHED, commentForBase);
update.commit();
@@ -1884,7 +1963,7 @@
Comment commentForPS =
newComment(psId, "filename", uuid2, range, range.getEndLine(),
otherUser, null, now, messageForPS,
- (short) 1, rev2);
+ (short) 1, rev2, false);
update.setPatchSetId(psId);
update.putComment(Status.PUBLISHED, commentForPS);
update.commit();
@@ -1911,7 +1990,7 @@
Timestamp timeForComment2 = TimeUtil.nowTs();
Comment comment1 =
newComment(psId, filename, uuid1, range, range.getEndLine(), otherUser,
- null, timeForComment1, "comment 1", side, rev);
+ null, timeForComment1, "comment 1", side, rev, false);
update.setPatchSetId(psId);
update.putComment(Status.PUBLISHED, comment1);
update.commit();
@@ -1919,7 +1998,7 @@
update = newUpdate(c, otherUser);
Comment comment2 =
newComment(psId, filename, uuid2, range, range.getEndLine(), otherUser,
- null, timeForComment2, "comment 2", side, rev);
+ null, timeForComment2, "comment 2", side, rev, false);
update.setPatchSetId(psId);
update.putComment(Status.PUBLISHED, comment2);
update.commit();
@@ -1946,7 +2025,7 @@
Timestamp now = TimeUtil.nowTs();
Comment comment1 = newComment(psId, filename1,
uuid, range, range.getEndLine(), otherUser, null, now, "comment 1",
- side, rev);
+ side, rev, false);
update.setPatchSetId(psId);
update.putComment(Status.PUBLISHED, comment1);
update.commit();
@@ -1954,7 +2033,7 @@
update = newUpdate(c, otherUser);
Comment comment2 = newComment(psId, filename2,
uuid, range, range.getEndLine(), otherUser, null, now, "comment 2",
- side, rev);
+ side, rev, false);
update.setPatchSetId(psId);
update.putComment(Status.PUBLISHED, comment2);
update.commit();
@@ -1979,7 +2058,8 @@
ChangeUpdate update = newUpdate(c, otherUser);
Timestamp now = TimeUtil.nowTs();
Comment comment1 = newComment(ps1, filename, uuid, range,
- range.getEndLine(), otherUser, null, now, "comment on ps1", side, rev1);
+ range.getEndLine(), otherUser, null, now, "comment on ps1",
+ side, rev1, false);
update.setPatchSetId(ps1);
update.putComment(Status.PUBLISHED, comment1);
update.commit();
@@ -1990,7 +2070,8 @@
update = newUpdate(c, otherUser);
now = TimeUtil.nowTs();
Comment comment2 = newComment(ps2, filename, uuid, range,
- range.getEndLine(), otherUser, null, now, "comment on ps2", side, rev2);
+ range.getEndLine(), otherUser, null, now, "comment on ps2",
+ side, rev2, false);
update.setPatchSetId(ps2);
update.putComment(Status.PUBLISHED, comment2);
update.commit();
@@ -2014,7 +2095,8 @@
ChangeUpdate update = newUpdate(c, otherUser);
Timestamp now = TimeUtil.nowTs();
Comment comment1 = newComment(ps1, filename, uuid, range,
- range.getEndLine(), otherUser, null, now, "comment on ps1", side, rev);
+ range.getEndLine(), otherUser, null, now, "comment on ps1",
+ side, rev, false);
update.setPatchSetId(ps1);
update.putComment(Status.DRAFT, comment1);
update.commit();
@@ -2053,9 +2135,11 @@
ChangeUpdate update = newUpdate(c, otherUser);
update.setPatchSetId(psId);
Comment comment1 = newComment(psId, filename, uuid1, range1,
- range1.getEndLine(), otherUser, null, now, "comment on ps1", side, rev);
+ range1.getEndLine(), otherUser, null, now, "comment on ps1",
+ side, rev, false);
Comment comment2 = newComment(psId, filename, uuid2, range2,
- range2.getEndLine(), otherUser, null, now, "other on ps1", side, rev);
+ range2.getEndLine(), otherUser, null, now, "other on ps1",
+ side, rev, false);
update.putComment(Status.DRAFT, comment1);
update.putComment(Status.DRAFT, comment2);
update.commit();
@@ -2099,10 +2183,10 @@
update.setPatchSetId(psId);
Comment baseComment =
newComment(psId, filename, uuid1, range1, range1.getEndLine(),
- otherUser, null, now, "comment on base", (short) 0, rev1);
+ otherUser, null, now, "comment on base", (short) 0, rev1, false);
Comment psComment =
newComment(psId, filename, uuid2, range2, range2.getEndLine(),
- otherUser, null, now, "comment on ps", (short) 1, rev2);
+ otherUser, null, now, "comment on ps", (short) 1, rev2, false);
update.putComment(Status.DRAFT, baseComment);
update.putComment(Status.DRAFT, psComment);
@@ -2145,7 +2229,8 @@
ChangeUpdate update = newUpdate(c, otherUser);
Timestamp now = TimeUtil.nowTs();
Comment comment = newComment(psId, filename, uuid, range,
- range.getEndLine(), otherUser, null, now, "comment on ps1", side, rev);
+ range.getEndLine(), otherUser, null, now, "comment on ps1",
+ side, rev, false);
update.setPatchSetId(psId);
update.putComment(Status.DRAFT, comment);
update.commit();
@@ -2183,7 +2268,8 @@
ChangeUpdate update = newUpdate(c, otherUser);
Timestamp now = TimeUtil.nowTs();
Comment comment1 = newComment(ps1, filename, uuid, range,
- range.getEndLine(), otherUser, null, now, "comment on ps1", side, rev1);
+ range.getEndLine(), otherUser, null, now, "comment on ps1",
+ side, rev1, false);
update.setPatchSetId(ps1);
update.putComment(Status.DRAFT, comment1);
update.commit();
@@ -2194,7 +2280,8 @@
update = newUpdate(c, otherUser);
now = TimeUtil.nowTs();
Comment comment2 = newComment(ps2, filename, uuid, range,
- range.getEndLine(), otherUser, null, now, "comment on ps2", side, rev2);
+ range.getEndLine(), otherUser, null, now, "comment on ps2",
+ side, rev2, false);
update.setPatchSetId(ps2);
update.putComment(Status.DRAFT, comment2);
update.commit();
@@ -2229,7 +2316,7 @@
ChangeUpdate update = newUpdate(c, otherUser);
Timestamp now = TimeUtil.nowTs();
Comment comment = newComment(ps1, filename, uuid, range, range.getEndLine(),
- otherUser, null, now, "comment on ps1", side, rev);
+ otherUser, null, now, "comment on ps1", side, rev, false);
update.putComment(Status.PUBLISHED, comment);
update.commit();
@@ -2252,7 +2339,7 @@
Timestamp now = TimeUtil.nowTs();
Comment draft =
newComment(ps1, filename, "uuid1", range, range.getEndLine(), otherUser,
- null, now, "draft comment on ps1", side, rev);
+ null, now, "draft comment on ps1", side, rev, false);
update.putComment(Status.DRAFT, draft);
update.commit();
@@ -2262,7 +2349,7 @@
update = newUpdate(c, otherUser);
Comment pub = newComment(ps1, filename, "uuid2", range, range.getEndLine(),
- otherUser, null, now, "comment on ps1", side, rev);
+ otherUser, null, now, "comment on ps1", side, rev, false);
update.putComment(Status.PUBLISHED, pub);
update.commit();
@@ -2280,7 +2367,7 @@
PatchSet.Id psId = c.currentPatchSetId();
Comment comment = newComment(psId, "filename", uuid, null, 0, otherUser,
- null, now, messageForBase, (short) 0, rev);
+ null, now, messageForBase, (short) 0, rev, false);
update.setPatchSetId(psId);
update.putComment(Status.PUBLISHED, comment);
update.commit();
@@ -2300,7 +2387,7 @@
PatchSet.Id psId = c.currentPatchSetId();
Comment comment = newComment(psId, "filename", uuid, null, 1, otherUser,
- null, now, messageForBase, (short) 0, rev);
+ null, now, messageForBase, (short) 0, rev, false);
update.setPatchSetId(psId);
update.putComment(Status.PUBLISHED, comment);
update.commit();
@@ -2327,9 +2414,11 @@
update.setPatchSetId(ps2);
Timestamp now = TimeUtil.nowTs();
Comment comment1 = newComment(ps1, filename, uuid, range,
- range.getEndLine(), otherUser, null, now, "comment on ps1", side, rev1);
+ range.getEndLine(), otherUser, null, now, "comment on ps1",
+ side, rev1, false);
Comment comment2 = newComment(ps2, filename, uuid, range,
- range.getEndLine(), otherUser, null, now, "comment on ps2", side, rev2);
+ range.getEndLine(), otherUser, null, now, "comment on ps2",
+ side, rev2, false);
update.putComment(Status.DRAFT, comment1);
update.putComment(Status.DRAFT, comment2);
update.commit();
@@ -2361,9 +2450,11 @@
update.setPatchSetId(ps1);
Timestamp now = TimeUtil.nowTs();
Comment comment1 = newComment(ps1, "file1", "uuid1", range,
- range.getEndLine(), otherUser, null, now, "comment1", side, rev1.get());
+ range.getEndLine(), otherUser, null, now, "comment1",
+ side, rev1.get(), false);
Comment comment2 = newComment(ps1, "file2", "uuid2", range,
- range.getEndLine(), otherUser, null, now, "comment2", side, rev1.get());
+ range.getEndLine(), otherUser, null, now, "comment2",
+ side, rev1.get(), false);
update.putComment(Status.DRAFT, comment1);
update.putComment(Status.DRAFT, comment2);
update.commit();
@@ -2412,10 +2503,10 @@
Timestamp now = TimeUtil.nowTs();
Comment comment1 =
newComment(ps1, "file1", "uuid1", range, range.getEndLine(), otherUser,
- null, now, "comment on ps1", side, rev1.get());
+ null, now, "comment on ps1", side, rev1.get(), false);
Comment comment2 =
newComment(ps1, "file2", "uuid2", range, range.getEndLine(), otherUser,
- null, now, "another comment", side, rev1.get());
+ null, now, "another comment", side, rev1.get(), false);
update.putComment(Status.DRAFT, comment1);
update.putComment(Status.DRAFT, comment2);
update.commit();
@@ -2472,13 +2563,15 @@
ChangeUpdate update1 = newUpdate(c, otherUser);
Comment comment1 = newComment(c.currentPatchSetId(), "filename",
"uuid1", range, range.getEndLine(), otherUser, null,
- new Timestamp(update1.getWhen().getTime()), "comment 1", (short) 1, rev);
+ new Timestamp(update1.getWhen().getTime()), "comment 1",
+ (short) 1, rev, false);
update1.putComment(Status.PUBLISHED, comment1);
ChangeUpdate update2 = newUpdate(c, otherUser);
Comment comment2 = newComment(c.currentPatchSetId(), "filename",
"uuid2", range, range.getEndLine(), otherUser, null,
- new Timestamp(update2.getWhen().getTime()), "comment 2", (short) 1, rev);
+ new Timestamp(update2.getWhen().getTime()), "comment 2",
+ (short) 1, rev, false);
update2.putComment(Status.PUBLISHED, comment2);
try (NoteDbUpdateManager manager = updateManagerFactory.create(project)) {
@@ -2527,7 +2620,7 @@
Comment comment = newComment(update.getPatchSetId(), "filename",
"uuid", range, range.getEndLine(), changeOwner, null,
new Timestamp(update.getWhen().getTime()), "comment", (short) 1,
- "abcd1234abcd1234abcd1234abcd1234abcd1234");
+ "abcd1234abcd1234abcd1234abcd1234abcd1234", false);
update.putComment(Status.PUBLISHED, comment);
update.commit();
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 9c52a27..60fd2ef 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
@@ -204,7 +204,7 @@
@Before
public void setTimeForTesting() {
- resetTimeWithClockStep(1, MILLISECONDS);
+ resetTimeWithClockStep(1, SECONDS);
}
private void resetTimeWithClockStep(long clockStep, TimeUnit clockStepUnit) {
diff --git a/polygerrit-ui/app/elements/change/gr-download-dialog/gr-download-dialog.js b/polygerrit-ui/app/elements/change/gr-download-dialog/gr-download-dialog.js
index b331899..485c8f9 100644
--- a/polygerrit-ui/app/elements/change/gr-download-dialog/gr-download-dialog.js
+++ b/polygerrit-ui/app/elements/change/gr-download-dialog/gr-download-dialog.js
@@ -30,6 +30,7 @@
loggedIn: {
type: Boolean,
value: false,
+ observer: '_loggedInChanged',
},
_schemes: {
@@ -49,15 +50,6 @@
Gerrit.RESTClientBehavior,
],
- attached: function() {
- if (!this.loggedIn) { return; }
- this.$.restAPI.getPreferences().then(function(prefs) {
- if (prefs.download_scheme) {
- this._selectedScheme = prefs.download_scheme;
- }
- }.bind(this));
- },
-
focus: function() {
this.$.download.focus();
},
@@ -70,6 +62,15 @@
};
},
+ _loggedInChanged: function(loggedIn) {
+ if (!loggedIn) { return; }
+ this.$.restAPI.getPreferences().then(function(prefs) {
+ if (prefs.download_scheme) {
+ this._selectedScheme = prefs.download_scheme;
+ }
+ }.bind(this));
+ },
+
_computeDownloadCommands: function(change, patchNum, _selectedScheme) {
var commandObj;
for (var rev in change.revisions) {
diff --git a/polygerrit-ui/app/elements/change/gr-download-dialog/gr-download-dialog_test.html b/polygerrit-ui/app/elements/change/gr-download-dialog/gr-download-dialog_test.html
index ad5086b..fdeecd7 100644
--- a/polygerrit-ui/app/elements/change/gr-download-dialog/gr-download-dialog_test.html
+++ b/polygerrit-ui/app/elements/change/gr-download-dialog/gr-download-dialog_test.html
@@ -162,6 +162,22 @@
assert.isFalse(el.hasAttribute('selected'));
});
});
+
+ test('loads scheme from preferences w/o initial login', function(done) {
+ stub('gr-rest-api-interface', {
+ getPreferences: function() {
+ return Promise.resolve({download_scheme: 'repo'});
+ },
+ });
+
+ element.loggedIn = true;
+
+ assert.isTrue(element.$.restAPI.getPreferences.called);
+ element.$.restAPI.getPreferences.lastCall.returnValue.then(function() {
+ assert.equal(element._selectedScheme, 'repo');
+ done();
+ });
+ });
});
suite('gr-download-dialog tests', function() {
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 5ab7fd6..cf66b49 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
@@ -284,7 +284,7 @@
path="[[file.__path]]"
prefs="[[_diffPrefs]]"
project-config="[[projectConfig]]"
- view-mode="[[_diffMode]]"></gr-diff>
+ view-mode="[[diffViewMode]]"></gr-diff>
</template>
<div class="row totalChanges">
<div class="total-stats" hidden$="[[_hideChangeTotals]]">
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 64b232e..1bb998f 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
@@ -16,11 +16,6 @@
var COMMIT_MESSAGE_PATH = '/COMMIT_MSG';
- var DiffViewMode = {
- SIDE_BY_SIDE: 'SIDE_BY_SIDE',
- UNIFIED: 'UNIFIED_DIFF',
- };
-
Polymer({
is: 'gr-file-list',
@@ -91,10 +86,6 @@
type: Array,
computed: '_computeFilesShown(_numFilesShown, _files.*)',
},
- _diffMode: {
- type: String,
- computed: '_getDiffViewMode(diffViewMode, _userPrefs)',
- },
// Caps the number of files that can be shown and have the 'show diffs' /
// 'hide diffs' buttons still be functional.
_maxFilesForBulkActions: {
@@ -152,16 +143,10 @@
this._diffPrefs = prefs;
}.bind(this)));
- // Initialize with user's diff mode preference. Default to
- // SIDE_BY_SIDE in the meantime.
- var setDiffViewMode = this.diffViewMode === null;
- if (setDiffViewMode) {
- this.set('diffViewMode', DiffViewMode.SIDE_BY_SIDE);
- }
promises.push(this._getPreferences().then(function(prefs) {
this._userPrefs = prefs;
- if (setDiffViewMode) {
- this.set('diffViewMode', prefs.diff_view);
+ if (!this.diffViewMode) {
+ this.set('diffViewMode', prefs.default_diff_view);
}
}.bind(this)));
},
@@ -613,29 +598,6 @@
this._diffAgainst = patchRange.basePatchNum;
},
- /**
- * _getDiffViewMode: Get the diff view (side-by-side or unified) based on
- * the current state.
- *
- * The expected behavior is to use the mode specified in the user's
- * preferences unless they have manually chosen the alternative view. If the
- * user navigates up to the change view, it should clear this choice and
- * revert to the preference the next time a diff is viewed.
- *
- * Use side-by-side if the user is not logged in.
- *
- * @return {String}
- */
- _getDiffViewMode: function() {
- if (this.diffViewMode) {
- return this.diffViewMode;
- } else if (this._userPrefs && this._userPrefs.diff_view) {
- return this.diffViewMode = this._userPrefs.diff_view;
- }
-
- return DiffViewMode.SIDE_BY_SIDE;
- },
-
_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 f472403..04070b5 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
@@ -557,12 +557,10 @@
element.$.fileCursor.setCursorAtIndex(0);
flushAsynchronousOperations();
var diffDisplay = element.diffs[0];
- element._userPrefs = {diff_view: 'SIDE_BY_SIDE'};
- assert.equal(element._getDiffViewMode(), 'SIDE_BY_SIDE');
+ element._userPrefs = {default_diff_view: 'SIDE_BY_SIDE'};
assert.equal(element.diffViewMode, 'SIDE_BY_SIDE');
assert.equal(diffDisplay.viewMode, 'SIDE_BY_SIDE');
element.set('diffViewMode', 'UNIFIED_DIFF');
- assert.equal(element._getDiffViewMode(), 'UNIFIED_DIFF');
assert.equal(diffDisplay.viewMode, 'UNIFIED_DIFF');
});
@@ -583,7 +581,7 @@
assert.equal(select.value, 'SIDE_BY_SIDE');
// Receive the overriding preference.
- resolvePrefs({diff_view: 'UNIFIED'});
+ resolvePrefs({default_diff_view: 'UNIFIED'});
flushAsynchronousOperations();
assert.equal(select.value, 'SIDE_BY_SIDE');
document.getElementById('blank').restore();
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.html b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.html
index 9b7a11f0..5832f7f 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.html
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.html
@@ -187,11 +187,11 @@
<div class="reviewerConfirmation">
Group
<span class="groupName">
- {{_reviewerPendingConfirmation.group.name}}
+ [[_pendingConfirmationDetails.group.name]]
</span>
has
<span class="groupSize">
- {{_reviewerPendingConfirmation.count}}
+ [[_pendingConfirmationDetails.count]]
</span>
members.
<br>
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js
index 3f27234..1cad8e5 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js
@@ -84,6 +84,7 @@
computed: '_computeLabels(change.labels.*, _account)',
},
_owner: Object,
+ _pendingConfirmationDetails: Object,
_reviewers: Array,
_reviewerPendingConfirmation: {
type: Object,
@@ -419,6 +420,8 @@
if (reviewer === null) {
this.$.reviewerConfirmationOverlay.close();
} else {
+ this._pendingConfirmationDetails =
+ this._ccPendingConfirmation || this._reviewerPendingConfirmation;
this.$.reviewerConfirmationOverlay.open();
}
},
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.html b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.html
index 72fbaaf..23a6c86 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.html
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.html
@@ -194,12 +194,14 @@
});
}
- test('reviewer confirmation', function(done) {
+ function testConfirmationDialog(done, cc) {
var yesButton =
element.$$('.reviewerConfirmationButtons gr-button:first-child');
var noButton =
element.$$('.reviewerConfirmationButtons gr-button:last-child');
+ element.serverConfig = {note_db_enabled: true};
+ element._ccPendingConfirmation = null;
element._reviewerPendingConfirmation = null;
flushAsynchronousOperations();
assert.isFalse(isVisible(element.$.reviewerConfirmationOverlay));
@@ -209,15 +211,37 @@
var group = {
id: 'id',
name: 'name',
- count: 10,
};
- element._reviewerPendingConfirmation = {
- group: group,
- };
+ if (cc) {
+ element._ccPendingConfirmation = {
+ group: group,
+ count: 10,
+ };
+ } else {
+ element._reviewerPendingConfirmation = {
+ group: group,
+ count: 10,
+ };
+ }
+ flushAsynchronousOperations();
+
+ if (cc) {
+ assert.deepEqual(
+ element._ccPendingConfirmation,
+ element._pendingConfirmationDetails);
+ } else {
+ assert.deepEqual(
+ element._reviewerPendingConfirmation,
+ element._pendingConfirmationDetails);
+ }
observer.then(function() {
assert.isTrue(isVisible(element.$.reviewerConfirmationOverlay));
observer = overlayObserver('closed');
+ var expected = 'Group name has 10 members';
+ assert.notEqual(
+ element.$.reviewerConfirmationOverlay.innerText.indexOf(expected),
+ -1);
MockInteractions.tap(noButton); // close the overlay
return observer;
}).then(function() {
@@ -226,14 +250,23 @@
// We should be focused on account entry input.
assert.equal(getActiveElement().id, 'input');
- // No reviewer should have been added.
- assert.deepEqual(element.$.reviewers.additions(), []);
+ // No reviewer/CC should have been added.
+ assert.equal(element.$$('#ccs').additions().length, 0);
+ assert.equal(element.$.reviewers.additions().length, 0);
// Reopen confirmation dialog.
observer = overlayObserver('opened');
- element._reviewerPendingConfirmation = {
- group: group,
- };
+ if (cc) {
+ element._ccPendingConfirmation = {
+ group: group,
+ count: 10,
+ };
+ } else {
+ element._reviewerPendingConfirmation = {
+ group: group,
+ count: 10,
+ };
+ }
return observer;
}).then(function() {
assert.isTrue(isVisible(element.$.reviewerConfirmationOverlay));
@@ -242,14 +275,16 @@
return observer;
}).then(function() {
assert.isFalse(isVisible(element.$.reviewerConfirmationOverlay));
+ var additions = cc ?
+ element.$$('#ccs').additions() :
+ element.$.reviewers.additions();
assert.deepEqual(
- element.$.reviewers.additions(),
+ additions,
[
{
group: {
id: 'id',
name: 'name',
- count: 10,
confirmed: true,
_group: true,
_pendingAdd: true,
@@ -260,6 +295,14 @@
// We should be focused on account entry input.
assert.equal(getActiveElement().id, 'input');
}).then(done);
+ };
+
+ test('cc confirmation', function(done) {
+ testConfirmationDialog(done, true);
+ });
+
+ test('reviewer confirmation', function(done) {
+ testConfirmationDialog(done, false);
});
test('_getStorageLocation', function() {
diff --git a/polygerrit-ui/app/elements/core/gr-main-header/gr-main-header.html b/polygerrit-ui/app/elements/core/gr-main-header/gr-main-header.html
index 76a1c1f..c3d9b16 100644
--- a/polygerrit-ui/app/elements/core/gr-main-header/gr-main-header.html
+++ b/polygerrit-ui/app/elements/core/gr-main-header/gr-main-header.html
@@ -46,7 +46,7 @@
cursor: default;
display: inline-block;
margin-left: 1em;
- padding: .5em 0;
+ padding: 0;
position: relative;
}
.linksTitle {
@@ -70,6 +70,7 @@
border-top-color: #666;
}
.rightItems {
+ align-items: center;
display: flex;
flex: 1;
justify-content: flex-end;
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread.html b/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread.html
index 864294b..0f00e18 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread.html
@@ -25,7 +25,6 @@
background-color: #ffd;
border: 1px solid #bbb;
display: block;
- padding: 0 .7em;
white-space: normal;
}
</style>
@@ -38,10 +37,11 @@
draft="[[comment.__draft]]"
show-actions="[[_showActions]]"
project-config="[[projectConfig]]"
- on-reply="_handleCommentReply"
on-comment-discard="_handleCommentDiscard"
- on-ack="_handleCommentAck"
- on-done="_handleCommentDone"></gr-diff-comment>
+ on-create-ack-comment="_handleCommentAck"
+ on-create-done-comment="_handleCommentDone"
+ on-create-fix-comment="_handleCommentFix"
+ on-create-reply-comment="_handleCommentReply"></gr-diff-comment>
</template>
</div>
<gr-rest-api-interface id="restAPI"></gr-rest-api-interface>
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 a094253..04dc547 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
@@ -14,6 +14,8 @@
(function() {
'use strict';
+ var NEWLINE_PATTERN = /\n/g;
+
Polymer({
is: 'gr-diff-comment-thread',
@@ -154,41 +156,50 @@
}
},
+ _createReplyComment: function(parent, content, opt_isEditing) {
+ var reply = this._newReply(parent.id, parent.line, content);
+
+ if (opt_isEditing) {
+ reply.__editing = true;
+ }
+
+ this.push('comments', reply);
+
+ if (!opt_isEditing) {
+ // Allow the reply to render in the dom-repeat.
+ this.async(function() {
+ var commentEl = this._commentElWithDraftID(reply.__draftID);
+ commentEl.save();
+ }, 1);
+ }
+ },
+
_handleCommentReply: function(e) {
var comment = e.detail.comment;
var quoteStr;
if (e.detail.quote) {
var msg = comment.message;
- var quoteStr = msg.split('\n').map(
- function(line) { return '> ' + line; }).join('\n') + '\n\n';
+ quoteStr = '> ' + msg.replace(NEWLINE_PATTERN, '\n> ') + '\n\n';
}
- var reply = this._newReply(comment.id, comment.line, quoteStr);
- reply.__editing = true;
- this.push('comments', reply);
+ this._createReplyComment(comment, quoteStr, true);
},
_handleCommentAck: function(e) {
var comment = e.detail.comment;
- var reply = this._newReply(comment.id, comment.line, 'Ack');
- this.push('comments', reply);
-
- // Allow the reply to render in the dom-repeat.
- this.async(function() {
- var commentEl = this._commentElWithDraftID(reply.__draftID);
- commentEl.save();
- }.bind(this), 1);
+ this._createReplyComment(comment, 'Ack');
},
_handleCommentDone: function(e) {
var comment = e.detail.comment;
- var reply = this._newReply(comment.id, comment.line, 'Done');
- this.push('comments', reply);
+ this._createReplyComment(comment, 'Done');
+ },
- // Allow the reply to render in the dom-repeat.
- this.async(function() {
- var commentEl = this._commentElWithDraftID(reply.__draftID);
- commentEl.save();
- }.bind(this), 1);
+ _handleCommentFix: function(e) {
+ var comment = e.detail.comment;
+ var msg = comment.message;
+ var quoteStr = '> ' + msg.replace(NEWLINE_PATTERN, '\n> ') + '\n\n';
+ var response = quoteStr + 'Please Fix';
+ this._createReplyComment(comment, response);
},
_commentElWithDraftID: function(id) {
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 b694a75..5e65bc8 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
@@ -152,7 +152,7 @@
test('reply', function(done) {
var commentEl = element.$$('gr-diff-comment');
assert.ok(commentEl);
- commentEl.addEventListener('reply', function() {
+ commentEl.addEventListener('create-reply-comment', function() {
var drafts = element._orderedComments.filter(function(c) {
return c.__draft == true;
});
@@ -161,13 +161,14 @@
assert.equal(drafts[0].in_reply_to, 'baf0414d_60047215');
done();
});
- commentEl.fire('reply', {comment: commentEl.comment}, {bubbles: false});
+ commentEl.fire('create-reply-comment', {comment: commentEl.comment},
+ {bubbles: false});
});
test('quote reply', function(done) {
var commentEl = element.$$('gr-diff-comment');
assert.ok(commentEl);
- commentEl.addEventListener('reply', function() {
+ commentEl.addEventListener('create-reply-comment', function() {
var drafts = element._orderedComments.filter(function(c) {
return c.__draft == true;
});
@@ -176,8 +177,37 @@
assert.equal(drafts[0].in_reply_to, 'baf0414d_60047215');
done();
});
- commentEl.fire('reply', {comment: commentEl.comment, quote: true},
- {bubbles: false});
+ commentEl.fire('create-reply-comment', {comment: commentEl.comment,
+ quote: true}, {bubbles: false});
+ });
+
+ test('quote reply multiline', function(done) {
+ element.comments = [{
+ author: {
+ name: 'Mr. Peanutbutter',
+ email: 'tenn1sballchaser@aol.com',
+ },
+ id: 'baf0414d_60047215',
+ line: 5,
+ message: 'is this a crossover episode!?\nIt might be!',
+ updated: '2015-12-08 19:48:33.843000000',
+ }];
+ flushAsynchronousOperations();
+
+ var commentEl = element.$$('gr-diff-comment');
+ assert.ok(commentEl);
+ commentEl.addEventListener('create-reply-comment', function() {
+ var drafts = element._orderedComments.filter(function(c) {
+ return c.__draft == true;
+ });
+ assert.equal(drafts.length, 1);
+ assert.equal(drafts[0].message,
+ '> is this a crossover episode!?\n> It might be!\n\n');
+ assert.equal(drafts[0].in_reply_to, 'baf0414d_60047215');
+ done();
+ });
+ commentEl.fire('create-reply-comment', {comment: commentEl.comment,
+ quote: true}, {bubbles: false});
});
test('ack', function(done) {
@@ -185,7 +215,7 @@
element.patchNum = '1';
var commentEl = element.$$('gr-diff-comment');
assert.ok(commentEl);
- commentEl.addEventListener('ack', function() {
+ commentEl.addEventListener('create-ack-comment', function() {
var drafts = element._orderedComments.filter(function(c) {
return c.__draft == true;
});
@@ -194,7 +224,8 @@
assert.equal(drafts[0].in_reply_to, 'baf0414d_60047215');
done();
});
- commentEl.fire('ack', {comment: commentEl.comment}, {bubbles: false});
+ commentEl.fire('create-ack-comment', {comment: commentEl.comment},
+ {bubbles: false});
});
test('done', function(done) {
@@ -202,7 +233,7 @@
element.patchNum = '1';
var commentEl = element.$$('gr-diff-comment');
assert.ok(commentEl);
- commentEl.addEventListener('done', function() {
+ commentEl.addEventListener('create-done-comment', function() {
var drafts = element._orderedComments.filter(function(c) {
return c.__draft == true;
});
@@ -211,7 +242,27 @@
assert.equal(drafts[0].in_reply_to, 'baf0414d_60047215');
done();
});
- commentEl.fire('done', {comment: commentEl.comment}, {bubbles: false});
+ commentEl.fire('create-done-comment', {comment: commentEl.comment},
+ {bubbles: false});
+ });
+
+ test('please fix', function(done) {
+ element.changeNum = '42';
+ element.patchNum = '1';
+ var commentEl = element.$$('gr-diff-comment');
+ assert.ok(commentEl);
+ commentEl.addEventListener('create-fix-comment', function() {
+ var drafts = element._orderedComments.filter(function(c) {
+ return c.__draft == true;
+ });
+ assert.equal(drafts.length, 1);
+ assert.equal(
+ drafts[0].message, '> is this a crossover episode!?\n\nPlease Fix');
+ assert.equal(drafts[0].in_reply_to, 'baf0414d_60047215');
+ done();
+ });
+ commentEl.fire('create-fix-comment', {comment: commentEl.comment},
+ {bubbles: false});
});
test('discard', function(done) {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.html b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.html
index a1c73db..2bde4e7 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.html
@@ -28,17 +28,20 @@
:host {
display: block;
font-family: var(--font-family);
- margin: .7em 0;
+ padding: .7em .7em;
--iron-autogrow-textarea: {
padding: 2px;
};
}
- :host([disabled]) {
+ :host[disabled] {
pointer-events: none;
}
- :host([disabled]) .container {
+ :host[disabled] .container {
opacity: .5;
}
+ :host[is-robot-comment] {
+ background-color: #cfe8fc;
+ }
.header {
cursor: pointer;
display: flex;
@@ -93,7 +96,7 @@
.danger .action {
margin-right: 0;
}
- .container:not(.draft) .actions :not(.reply):not(.quote):not(.ack):not(.done) {
+ .container:not(.draft) .actions .hideOnPublished {
display: none;
}
.draft .reply,
@@ -124,6 +127,20 @@
.show-hide {
margin-left: .4em;
}
+ .robotId {
+ color: #808080;
+ margin-bottom: .8em;
+ margin-top: -.4em;
+ }
+ .runIdInformation {
+ margin-bottom: .5em;
+ }
+ .robotRun {
+ margin-left: .5em;
+ }
+ .robotRunLink {
+ margin-left: .5em;
+ }
input.show-hide {
display: none;
}
@@ -178,6 +195,11 @@
</label>
</div>
</div>
+ <template is="dom-if" if="[[comment.robot_id]]">
+ <div class="robotId" hidden$="[[collapsed]]"">
+ [[comment.robot_id]]
+ </div>
+ </template>
<iron-autogrow-textarea
id="editTextarea"
class="editMessage"
@@ -190,19 +212,36 @@
content="[[comment.message]]"
collapsed="[[collapsed]]"
config="[[projectConfig.commentlinks]]"></gr-formatted-text>
- <div class="actions" hidden$="[[!showActions]]">
+ <div hidden$="[[!comment.robot_run_id]]">
+ <div class="runIdInformation" hidden$="[[collapsed]]">
+ Run ID:
+ <a class="robotRunLink" href$="[[comment.url]]">
+ <span class="robotRun">[[comment.robot_run_id]]</span>
+ </a>
+ </div>
+ </div>
+ <div class="actions humanActions" hidden$="[[!_showHumanActions]]">
<gr-button class="action reply" on-tap="_handleReply">Reply</gr-button>
<gr-button class="action quote" on-tap="_handleQuote">Quote</gr-button>
<gr-button class="action ack" on-tap="_handleAck">Ack</gr-button>
- <gr-button class="action done" on-tap="_handleDone">Done</gr-button>
- <gr-button class="action edit" on-tap="_handleEdit">Edit</gr-button>
- <gr-button class="action save" on-tap="_handleSave"
+ <gr-button class="action done" on-tap="_handleDone">
+ Done</gr-button>
+ <gr-button class="action edit hideOnPublished" on-tap="_handleEdit">
+ Edit</gr-button>
+ <gr-button class="action save hideOnPublished" on-tap="_handleSave"
disabled$="[[_computeSaveDisabled(_messageText)]]">Save</gr-button>
- <gr-button class="action cancel" on-tap="_handleCancel" hidden>Cancel</gr-button>
+ <gr-button class="action cancel hideOnPublished"
+ on-tap="_handleCancel" hidden>Cancel</gr-button>
<div class="danger">
- <gr-button class="action discard" on-tap="_handleDiscard">Discard</gr-button>
+ <gr-button class="action discard hideOnPublished"
+ on-tap="_handleDiscard">Discard</gr-button>
</div>
</div>
+ <div class="actions robotActions" hidden$="[[!_showRobotActions]]">
+ <gr-button class="action fix" on-tap="_handleFix">
+ Please Fix
+ </gr-button>
+ </div>
</div>
<gr-rest-api-interface id="restAPI"></gr-rest-api-interface>
<gr-storage id="storage"></gr-storage>
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 72138ff..cbf20e8 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
@@ -22,19 +22,25 @@
/**
* Fired when the Reply action is triggered.
*
- * @event reply
+ * @event create-reply-comment
*/
/**
* Fired when the Ack action is triggered.
*
- * @event ack
+ * @event create-ack-comment
*/
/**
* Fired when the Done action is triggered.
*
- * @event done
+ * @event create-done-comment
+ */
+
+ /**
+ * Fired when the create fix comment action is triggered.
+ *
+ * @event create-fix-comment
*/
/**
@@ -70,6 +76,11 @@
notify: true,
observer: '_commentChanged',
},
+ isRobotComment: {
+ type: Boolean,
+ value: false,
+ reflectToAttribute: true,
+ },
disabled: {
type: Boolean,
value: false,
@@ -85,8 +96,11 @@
value: false,
observer: '_editingChanged',
},
+ hasChildren: Boolean,
patchNum: String,
showActions: Boolean,
+ _showHumanActions: Boolean,
+ _showRobotActions: Boolean,
collapsed: {
type: Boolean,
value: true,
@@ -105,6 +119,8 @@
observers: [
'_commentMessageChanged(comment.message)',
'_loadLocalDraft(changeNum, patchNum, comment)',
+ '_isRobotComment(comment)',
+ '_calculateActionstoShow(showActions, isRobotComment)',
],
attached: function() {
@@ -121,6 +137,15 @@
return collapsed ? '◀' : '▼';
},
+ _calculateActionstoShow: function(showActions, isRobotComment) {
+ this._showHumanActions = showActions && !isRobotComment;
+ this._showRobotActions = showActions && isRobotComment;
+ },
+
+ _isRobotComment: function(comment) {
+ this.isRobotComment = !!comment.robot_id;
+ },
+
save: function() {
this.comment.message = this._messageText;
this.disabled = true;
@@ -290,23 +315,32 @@
_handleReply: function(e) {
e.preventDefault();
- this.fire('reply', this._getEventPayload(), {bubbles: false});
+ this.fire('create-reply-comment', this._getEventPayload(),
+ {bubbles: false});
},
_handleQuote: function(e) {
e.preventDefault();
- this.fire(
- 'reply', this._getEventPayload({quote: true}), {bubbles: false});
+ this.fire( 'create-reply-comment', this._getEventPayload({quote: true}),
+ {bubbles: false});
+ },
+
+ _handleFix: function(e) {
+ e.preventDefault();
+ this.fire('create-fix-comment', this._getEventPayload({quote: true}),
+ {bubbles: false});
},
_handleAck: function(e) {
e.preventDefault();
- this.fire('ack', this._getEventPayload(), {bubbles: false});
+ this.fire('create-ack-comment', this._getEventPayload(),
+ {bubbles: false});
},
_handleDone: function(e) {
e.preventDefault();
- this.fire('done', this._getEventPayload(), {bubbles: false});
+ this.fire('create-done-comment', this._getEventPayload(),
+ {bubbles: false});
},
_handleEdit: function(e) {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment_test.html b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment_test.html
index f562d3a..6793144 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment_test.html
@@ -99,7 +99,7 @@
});
test('proper event fires on reply', function(done) {
- element.addEventListener('reply', function(e) {
+ element.addEventListener('create-reply-comment', function(e) {
assert.ok(e.detail.comment);
done();
});
@@ -107,7 +107,7 @@
});
test('proper event fires on quote', function(done) {
- element.addEventListener('reply', function(e) {
+ element.addEventListener('create-reply-comment', function(e) {
assert.ok(e.detail.comment);
assert.isTrue(e.detail.quote);
done();
@@ -116,19 +116,26 @@
});
test('proper event fires on ack', function(done) {
- element.addEventListener('ack', function(e) {
+ element.addEventListener('create-ack-comment', function(e) {
done();
});
MockInteractions.tap(element.$$('.ack'));
});
test('proper event fires on done', function(done) {
- element.addEventListener('done', function(e) {
+ element.addEventListener('create-done-comment', function(e) {
done();
});
MockInteractions.tap(element.$$('.done'));
});
+ test('proper event fires on fix', function(done) {
+ element.addEventListener('create-fix-comment', function(e) {
+ done();
+ });
+ MockInteractions.tap(element.$$('.fix'));
+ });
+
test('clicking on date link does not trigger nav', function() {
var showStub = sinon.stub(page, 'show');
var dateEl = element.$$('.date');
@@ -229,9 +236,12 @@
test('button visibility states', function() {
element.showActions = false;
- assert.isTrue(element.$$('.actions').hasAttribute('hidden'));
+ assert.isTrue(element.$$('.humanActions').hasAttribute('hidden'));
+ assert.isTrue(element.$$('.robotActions').hasAttribute('hidden'));
+
element.showActions = true;
- assert.isFalse(element.$$('.actions').hasAttribute('hidden'));
+ assert.isFalse(element.$$('.humanActions').hasAttribute('hidden'));
+ assert.isTrue(element.$$('.robotActions').hasAttribute('hidden'));
element.draft = true;
assert.isTrue(isVisible(element.$$('.edit')), 'edit is visible');
@@ -242,6 +252,8 @@
assert.isFalse(isVisible(element.$$('.quote')), 'quote is not visible');
assert.isFalse(isVisible(element.$$('.ack')), 'ack is not visible');
assert.isFalse(isVisible(element.$$('.done')), 'done is not visible');
+ assert.isFalse(element.$$('.humanActions').hasAttribute('hidden'));
+ assert.isTrue(element.$$('.robotActions').hasAttribute('hidden'));
element.editing = true;
assert.isFalse(isVisible(element.$$('.edit')), 'edit is not visible');
@@ -252,6 +264,8 @@
assert.isFalse(isVisible(element.$$('.quote')), 'quote is not visible');
assert.isFalse(isVisible(element.$$('.ack')), 'ack is not visible');
assert.isFalse(isVisible(element.$$('.done')), 'done is not visible');
+ assert.isFalse(element.$$('.humanActions').hasAttribute('hidden'));
+ assert.isTrue(element.$$('.robotActions').hasAttribute('hidden'));
element.draft = false;
element.editing = false;
@@ -264,11 +278,26 @@
assert.isTrue(isVisible(element.$$('.quote')), 'quote is visible');
assert.isTrue(isVisible(element.$$('.ack')), 'ack is visible');
assert.isTrue(isVisible(element.$$('.done')), 'done is visible');
+ assert.isFalse(element.$$('.humanActions').hasAttribute('hidden'));
+ assert.isTrue(element.$$('.robotActions').hasAttribute('hidden'));
element.comment.id = 'foo';
element.draft = true;
element.editing = true;
assert.isTrue(isVisible(element.$$('.cancel')), 'cancel is visible');
+ assert.isFalse(element.$$('.humanActions').hasAttribute('hidden'));
+ assert.isTrue(element.$$('.robotActions').hasAttribute('hidden'));
+
+ element.isRobotComment = true;
+ element.draft = true;
+ assert.isTrue(element.$$('.humanActions').hasAttribute('hidden'));
+ assert.isFalse(element.$$('.robotActions').hasAttribute('hidden'));
+
+ // It is not expected to see Robot comment drafts, but if they appear,
+ // they will behave the same as non-drafts.
+ element.draft = false;
+ assert.isTrue(element.$$('.humanActions').hasAttribute('hidden'));
+ assert.isFalse(element.$$('.robotActions').hasAttribute('hidden'));
});
test('collapsible drafts', function() {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor_test.html b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor_test.html
index a7d98e6..f1f3810 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor_test.html
@@ -70,6 +70,10 @@
return Promise.resolve({baseComments: [], comments: []});
});
+ sinon.stub(diffElement, '_getDiffRobotComments', function() {
+ return Promise.resolve({baseComments: [], comments: []});
+ });
+
var setupDone = function() {
cursorElement.moveToFirstChunk();
done();
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 a087c57..5dc1d95 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
@@ -16,13 +16,6 @@
var COMMIT_MESSAGE_PATH = '/COMMIT_MSG';
- var MAX_UNIFIED_DEFAULT_WINDOW_WIDTH_PX = 900;
-
- var DiffViewMode = {
- SIDE_BY_SIDE: 'SIDE_BY_SIDE',
- UNIFIED: 'UNIFIED_DIFF',
- };
-
var DiffSides = {
LEFT: 'left',
RIGHT: 'right',
@@ -125,16 +118,9 @@
}.bind(this));
if (this.changeViewState.diffMode === null) {
// If screen size is small, always default to unified view.
- if (this._getWindowWidth() < MAX_UNIFIED_DEFAULT_WINDOW_WIDTH_PX) {
- this.set('changeViewState.diffMode', DiffViewMode.UNIFIED);
- } else {
- // Initialize with user's diff mode preference. Default to
- // SIDE_BY_SIDE in the meantime.
- this.set('changeViewState.diffMode', DiffViewMode.SIDE_BY_SIDE);
- this.$.restAPI.getPreferences().then(function(prefs) {
- this.set('changeViewState.diffMode', prefs.diff_view);
- }.bind(this));
- }
+ this.$.restAPI.getPreferences().then(function(prefs) {
+ this.set('changeViewState.diffMode', prefs.default_diff_view);
+ }.bind(this));
}
if (this._path) {
@@ -573,9 +559,10 @@
* the current state.
*
* The expected behavior is to use the mode specified in the user's
- * preferences unless they have manually chosen the alternative view. If the
- * user navigates up to the change view, it should clear this choice and
- * revert to the preference the next time a diff is viewed.
+ * preferences unless they have manually chosen the alternative view or they
+ * are on a mobile device. If the user navigates up to the change view, it
+ * should clear this choice and revert to the preference the next time a
+ * diff is viewed.
*
* Use side-by-side if the user is not logged in.
*
@@ -584,11 +571,12 @@
_getDiffViewMode: function() {
if (this.changeViewState.diffMode) {
return this.changeViewState.diffMode;
- } else if (this._userPrefs && this._userPrefs.diff_view) {
- return this.changeViewState.diffMode = this._userPrefs.diff_view;
+ } else if (this._userPrefs) {
+ return this.changeViewState.diffMode =
+ this._userPrefs.default_diff_view;
+ } else {
+ return 'SIDE_BY_SIDE';
}
-
- return DiffViewMode.SIDE_BY_SIDE;
},
_computeModeSelectHidden: function() {
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 cb8c095..29dd8ae 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
@@ -457,7 +457,7 @@
test('diff mode selector correctly toggles the diff', function() {
var select = element.$.modeSelect;
var diffDisplay = element.$.diff;
- element._userPrefs = {diff_view: 'SIDE_BY_SIDE'};
+ element._userPrefs = {default_diff_view: 'SIDE_BY_SIDE'};
// The mode selected in the view state reflects the selected option.
assert.equal(element._getDiffViewMode(), select.value);
@@ -496,42 +496,11 @@
assert.equal(select.value, 'SIDE_BY_SIDE');
// Receive the overriding preference.
- resolvePrefs({diff_view: 'UNIFIED'});
+ resolvePrefs({default_diff_view: 'UNIFIED'});
flushAsynchronousOperations();
assert.equal(select.value, 'SIDE_BY_SIDE');
});
- test('unified view is always default on small screens', function() {
- var resolvePrefs;
- var prefsPromise = new Promise(function(resolve) {
- resolvePrefs = resolve;
- });
-
- var getPreferencesStub = sandbox.stub(element.$.restAPI, 'getPreferences',
- function() { return prefsPromise; });
-
- // Attach a new gr-diff-view so we can intercept the preferences fetch.
- var view = document.createElement('gr-diff-view');
-
- view.changeViewState = {diffMode: null};
-
- sandbox.stub(view, '_getWindowWidth', function() { return 800; });
-
- var select = view.$.modeSelect;
- fixture('blank').appendChild(view);
- flushAsynchronousOperations();
-
- // At this point the diff mode doesn't yet have the user's preference.
- assert.equal(select.value, 'UNIFIED_DIFF');
-
- // Receive the overriding preference.
- resolvePrefs({diff_view: 'SIDE_BY_SIDE'});
- flushAsynchronousOperations();
-
- // On small screens, unified should override user perferences
- assert.equal(select.value, 'UNIFIED_DIFF');
- });
-
test('_loadHash', function() {
assert.isNotOk(element.$.cursor.initialLineNumber);
@@ -598,5 +567,18 @@
assert.equal(element._getDiffURL(changeNum, patchRange, path),
'/c/123/123..456/c%252B%252B/cpp.cpp');
});
+
+ test('_getDiffViewMode', function() {
+ // No user prefs or change view state set.
+ assert.equal(element._getDiffViewMode(), 'SIDE_BY_SIDE');
+
+ // User prefs but no change view state set.
+ element._userPrefs = {default_diff_view: 'UNIFIED_DIFF'};
+ assert.equal(element._getDiffViewMode(), 'UNIFIED_DIFF');
+
+ // User prefs and change view state set.
+ element.changeViewState = {diffMode: 'SIDE_BY_SIDE'};
+ assert.equal(element._getDiffViewMode(), 'SIDE_BY_SIDE');
+ });
});
</script>
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
index 4c9d021..4b5e381 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
@@ -453,14 +453,24 @@
}.bind(this));
},
+ _getDiffRobotComments: function() {
+ return this.$.restAPI.getDiffRobotComments(
+ this.changeNum,
+ this.patchRange.basePatchNum,
+ this.patchRange.patchNum,
+ this.path);
+ },
+
_getDiffCommentsAndDrafts: function() {
var promises = [];
promises.push(this._getDiffComments());
promises.push(this._getDiffDrafts());
+ promises.push(this._getDiffRobotComments());
return Promise.all(promises).then(function(results) {
return Promise.resolve({
comments: results[0],
drafts: results[1],
+ robotComments: results[2],
});
}).then(this._normalizeDiffCommentsAndDrafts.bind(this));
},
@@ -472,6 +482,9 @@
}
var baseDrafts = results.drafts.baseComments.map(markAsDraft);
var drafts = results.drafts.comments.map(markAsDraft);
+
+ var baseRobotComments = results.robotComments.baseComments;
+ var robotComments = results.robotComments.comments;
return Promise.resolve({
meta: {
path: this.path,
@@ -479,8 +492,10 @@
patchRange: this.patchRange,
projectConfig: this.projectConfig,
},
- left: results.comments.baseComments.concat(baseDrafts),
- right: results.comments.comments.concat(drafts),
+ left: results.comments.baseComments.concat(baseDrafts)
+ .concat(baseRobotComments),
+ right: results.comments.comments.concat(drafts)
+ .concat(robotComments),
});
},
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html
index 306f283..9e10bf9 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html
@@ -77,6 +77,19 @@
});
});
+ test('get robot comments', function(done) {
+ element.patchRange = {basePatchNum: 0, patchNum: 0};
+
+ var getDraftsStub = sinon.stub(element.$.restAPI,
+ 'getDiffRobotComments');
+ element._getDiffDrafts().then(function(result) {
+ assert.deepEqual(result, {baseComments: [], comments: []});
+ sinon.assert.notCalled(getDraftsStub);
+ getDraftsStub.restore();
+ done();
+ });
+ });
+
test('loads files weblinks', function(done) {
var diffStub = sinon.stub(element.$.restAPI, 'getDiff').returns(
Promise.resolve({
@@ -403,9 +416,24 @@
{id: 'd2'},
],
};
+
var diffDraftsStub = sinon.stub(element, '_getDiffDrafts',
function() { return Promise.resolve(drafts); });
+ var robotComments = {
+ baseComments: [
+ {id: 'br1'},
+ {id: 'br2'},
+ ],
+ comments: [
+ {id: 'r1'},
+ {id: 'r2'},
+ ],
+ };
+
+ var diffRobotCommentStub = sinon.stub(element, '_getDiffRobotComments',
+ function() { return Promise.resolve(robotComments); });
+
element.changeNum = '42';
element.patchRange = {
basePatchNum: 'PARENT',
@@ -430,17 +458,22 @@
{id: 'bc2'},
{id: 'bd1', __draft: true},
{id: 'bd2', __draft: true},
+ {id: 'br1'},
+ {id: 'br2'},
],
right: [
{id: 'c1'},
{id: 'c2'},
{id: 'd1', __draft: true},
{id: 'd2', __draft: true},
+ {id: 'r1'},
+ {id: 'r2'},
],
});
diffCommentsStub.restore();
diffDraftsStub.restore();
+ diffRobotCommentStub.restore();
done();
});
});
diff --git a/polygerrit-ui/app/elements/gr-app.html b/polygerrit-ui/app/elements/gr-app.html
index c0c967d..d62571d 100644
--- a/polygerrit-ui/app/elements/gr-app.html
+++ b/polygerrit-ui/app/elements/gr-app.html
@@ -47,13 +47,14 @@
gr-main-header,
footer {
color: var(--primary-text-color);
- padding: .5rem var(--default-horizontal-margin);
}
gr-main-header {
background-color: var(--header-background-color, #eee);
+ padding: 0 var(--default-horizontal-margin);
}
footer {
background-color: var(--footer-background-color, #eee);
+ padding: .5rem var(--default-horizontal-margin);
}
main {
flex: 1;
diff --git a/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown.html b/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown.html
index 1d99caa..55209af 100644
--- a/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown.html
+++ b/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown.html
@@ -43,6 +43,9 @@
width: 2em;
vertical-align: middle;
}
+ gr-button[link] {
+ padding: 1em 0;
+ }
ul {
list-style: none;
}
@@ -77,7 +80,7 @@
</gr-button>
<iron-dropdown id="dropdown"
vertical-align="top"
- vertical-offset="25"
+ vertical-offset="40"
allow-outside-scroll="true"
horizontal-align="[[horizontalAlign]]"
on-tap="_handleDropdownTap">
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 48682e9..dee0cad 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
@@ -14,7 +14,12 @@
(function() {
'use strict';
+ var DiffViewMode = {
+ SIDE_BY_SIDE: 'SIDE_BY_SIDE',
+ UNIFIED: 'UNIFIED_DIFF',
+ };
var JSON_PREFIX = ')]}\'';
+ var MAX_UNIFIED_DEFAULT_WINDOW_WIDTH_PX = 900;
var PARENT_PATCH_NUM = 'PARENT';
// Must be kept in sync with the ListChangesOption enum and protobuf.
@@ -295,11 +300,21 @@
getPreferences: function() {
return this.getLoggedIn().then(function(loggedIn) {
if (loggedIn) {
- return this._fetchSharedCacheURL('/accounts/self/preferences');
+ return this._fetchSharedCacheURL('/accounts/self/preferences').then(
+ function(res) {
+ if (this._isNarrowScreen()) {
+ res.default_diff_view = DiffViewMode.UNIFIED;
+ } else {
+ res.default_diff_view = res.diff_view;
+ }
+ return Promise.resolve(res);
+ }.bind(this));
}
return Promise.resolve({
changes_per_page: 25,
+ default_diff_view: this._isNarrowScreen() ?
+ DiffViewMode.UNIFIED : DiffViewMode.SIDE_BY_SIDE,
diff_view: 'SIDE_BY_SIDE',
});
}.bind(this));
@@ -344,6 +359,10 @@
return this._sharedFetchPromises[url];
},
+ _isNarrowScreen: function() {
+ return window.innerWidth < MAX_UNIFIED_DEFAULT_WINDOW_WIDTH_PX;
+ },
+
getChanges: function(changesPerPage, opt_query, opt_offset) {
var options = this._listChangesOptionsToHex(
ListChangesOption.LABELS,
@@ -671,6 +690,12 @@
opt_patchNum, opt_path);
},
+ getDiffRobotComments: function(changeNum, basePatchNum, patchNum,
+ opt_path) {
+ return this._getDiffComments(changeNum, '/robotcomments', basePatchNum,
+ patchNum, opt_path);
+ },
+
getDiffDrafts: function(changeNum, opt_basePatchNum, opt_patchNum,
opt_path) {
return this._getDiffComments(changeNum, '/drafts', opt_basePatchNum,
diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.html b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.html
index d07968f..27d5d32 100644
--- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.html
@@ -339,5 +339,78 @@
assert.isTrue(sendStub.called);
assert.notOk(element._cache[cacheKey]);
});
+
+ var preferenceSetup = function(testJSON, loggedIn, smallScreen) {
+ sandbox.stub(element, 'getLoggedIn', function() {
+ return Promise.resolve(loggedIn);
+ });
+ sandbox.stub(element, '_isNarrowScreen', function() {
+ return smallScreen;
+ });
+ sandbox.stub(element, '_fetchSharedCacheURL', function() {
+ return Promise.resolve(testJSON);
+ });
+ };
+
+ test('getPreferences returns correctly on small screens logged in',
+ function(done) {
+
+ var testJSON = {diff_view: 'SIDE_BY_SIDE'};
+ var loggedIn = true;
+ var smallScreen = true;
+
+ preferenceSetup(testJSON, loggedIn, smallScreen);
+
+ element.getPreferences().then(function(obj) {
+ assert.equal(obj.default_diff_view, 'UNIFIED_DIFF');
+ assert.equal(obj.diff_view, 'SIDE_BY_SIDE');
+ done();
+ });
+ });
+
+ test('getPreferences returns correctly on small screens not logged in',
+ function(done) {
+
+ var testJSON = {diff_view: 'SIDE_BY_SIDE'};
+ var loggedIn = false;
+ var smallScreen = true;
+
+ preferenceSetup(testJSON, loggedIn, smallScreen);
+ element.getPreferences().then(function(obj) {
+ assert.equal(obj.default_diff_view, 'UNIFIED_DIFF');
+ assert.equal(obj.diff_view, 'SIDE_BY_SIDE');
+ done();
+ });
+ });
+
+ test('getPreferences returns correctly on larger screens logged in',
+ function(done) {
+ var testJSON = {diff_view: 'UNIFIED_DIFF'};
+ var loggedIn = true;
+ var smallScreen = false;
+
+ preferenceSetup(testJSON, loggedIn, smallScreen);
+
+ element.getPreferences().then(function(obj) {
+ assert.equal(obj.default_diff_view, 'UNIFIED_DIFF');
+ assert.equal(obj.diff_view, 'UNIFIED_DIFF');
+ done();
+ });
+ });
+
+ test('getPreferences returns correctly on larger screens not logged in',
+ function(done) {
+ var testJSON = {diff_view: 'UNIFIED_DIFF'};
+ var loggedIn = false;
+ var smallScreen = false;
+
+ preferenceSetup(testJSON, loggedIn, smallScreen);
+
+ element.getPreferences().then(function(obj) {
+ assert.equal(obj.default_diff_view, 'SIDE_BY_SIDE');
+ assert.equal(obj.diff_view, 'SIDE_BY_SIDE');
+ done();
+ });
+ });
});
</script>
