Merge "Tweak the appearance of the attention set icon"
diff --git a/WORKSPACE b/WORKSPACE
index 158a7cc..47c3e9c 100644
--- a/WORKSPACE
+++ b/WORKSPACE
@@ -650,18 +650,18 @@
sha1 = "a3ae34e57fa8a4040e28247291d0cc3d6b8c7bcf",
)
-AUTO_VALUE_VERSION = "1.7.2"
+AUTO_VALUE_VERSION = "1.7.3"
maven_jar(
name = "auto-value",
artifact = "com.google.auto.value:auto-value:" + AUTO_VALUE_VERSION,
- sha1 = "895dbc8f1764f162c1dae34cc29e300220d6d4ba",
+ sha1 = "cbd30873f839545c7c9264bed61d500bf85bd33e",
)
maven_jar(
name = "auto-value-annotations",
artifact = "com.google.auto.value:auto-value-annotations:" + AUTO_VALUE_VERSION,
- sha1 = "7eec707327ec1663b9387c8671efb6808750e039",
+ sha1 = "59ce5ee6aea918f674229f1147da95fdf7f31ce6",
)
declare_nongoogle_deps()
diff --git a/java/com/google/gerrit/acceptance/testsuite/account/AccountOperationsImpl.java b/java/com/google/gerrit/acceptance/testsuite/account/AccountOperationsImpl.java
index f64d7a2..8c1eebd 100644
--- a/java/com/google/gerrit/acceptance/testsuite/account/AccountOperationsImpl.java
+++ b/java/com/google/gerrit/acceptance/testsuite/account/AccountOperationsImpl.java
@@ -15,7 +15,10 @@
package com.google.gerrit.acceptance.testsuite.account;
import static com.google.common.base.Preconditions.checkState;
+import static com.google.common.collect.ImmutableSet.toImmutableSet;
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Sets;
import com.google.gerrit.entities.Account;
import com.google.gerrit.server.ServerInitiated;
import com.google.gerrit.server.account.AccountState;
@@ -60,8 +63,8 @@
private Account.Id createAccount(TestAccountCreation accountCreation) throws Exception {
AccountsUpdate.AccountUpdater accountUpdater =
- (account, updateBuilder) ->
- fillBuilder(updateBuilder, accountCreation, account.account().id());
+ (accountState, updateBuilder) ->
+ fillBuilder(updateBuilder, accountCreation, accountState.account().id());
AccountState createdAccount = createAccount(accountUpdater);
return createdAccount.account().id();
}
@@ -82,6 +85,11 @@
accountCreation.username().ifPresent(u -> setUsername(builder, accountId, u, httpPassword));
accountCreation.status().ifPresent(builder::setStatus);
accountCreation.active().ifPresent(builder::setActive);
+ accountCreation
+ .secondaryEmails()
+ .forEach(
+ secondaryEmail ->
+ builder.addExternalId(ExternalId.createEmail(accountId, secondaryEmail)));
}
private static InternalAccountUpdate.Builder setPreferredEmail(
@@ -136,6 +144,7 @@
.fullname(Optional.ofNullable(account.fullName()))
.username(accountState.userName())
.active(accountState.account().isActive())
+ .emails(ExternalId.getEmails(accountState.externalIds()).collect(toImmutableSet()))
.build();
}
@@ -147,7 +156,7 @@
private void updateAccount(TestAccountUpdate accountUpdate)
throws IOException, ConfigInvalidException {
AccountsUpdate.AccountUpdater accountUpdater =
- (account, updateBuilder) -> fillBuilder(updateBuilder, accountUpdate, accountId);
+ (accountState, updateBuilder) -> fillBuilder(updateBuilder, accountUpdate, accountState);
Optional<AccountState> updatedAccount = updateAccount(accountUpdater);
checkState(updatedAccount.isPresent(), "Tried to update non-existing test account");
}
@@ -160,13 +169,58 @@
private void fillBuilder(
InternalAccountUpdate.Builder builder,
TestAccountUpdate accountUpdate,
- Account.Id accountId) {
+ AccountState accountState) {
accountUpdate.fullname().ifPresent(builder::setFullName);
accountUpdate.preferredEmail().ifPresent(e -> setPreferredEmail(builder, accountId, e));
String httpPassword = accountUpdate.httpPassword().orElse(null);
accountUpdate.username().ifPresent(u -> setUsername(builder, accountId, u, httpPassword));
accountUpdate.status().ifPresent(builder::setStatus);
accountUpdate.active().ifPresent(builder::setActive);
+
+ ImmutableSet<String> secondaryEmails = getSecondaryEmails(accountUpdate, accountState);
+ ImmutableSet<String> newSecondaryEmails =
+ ImmutableSet.copyOf(accountUpdate.secondaryEmailsModification().apply(secondaryEmails));
+ if (!secondaryEmails.equals(newSecondaryEmails)) {
+ setSecondaryEmails(builder, accountUpdate, accountState, newSecondaryEmails);
+ }
+ }
+
+ private ImmutableSet<String> getSecondaryEmails(
+ TestAccountUpdate accountUpdate, AccountState accountState) {
+ ImmutableSet<String> allEmails =
+ ExternalId.getEmails(accountState.externalIds()).collect(toImmutableSet());
+ if (accountUpdate.preferredEmail().isPresent()) {
+ return ImmutableSet.copyOf(
+ Sets.difference(allEmails, ImmutableSet.of(accountUpdate.preferredEmail().get())));
+ } else if (accountState.account().preferredEmail() != null) {
+ return ImmutableSet.copyOf(
+ Sets.difference(allEmails, ImmutableSet.of(accountState.account().preferredEmail())));
+ }
+ return allEmails;
+ }
+
+ private void setSecondaryEmails(
+ InternalAccountUpdate.Builder builder,
+ TestAccountUpdate accountUpdate,
+ AccountState accountState,
+ ImmutableSet<String> newSecondaryEmails) {
+ // delete all external IDs of SCHEME_MAILTO scheme, then add back SCHEME_MAILTO external IDs
+ // for the new secondary emails and the preferred email
+ builder.deleteExternalIds(
+ accountState.externalIds().stream()
+ .filter(e -> e.isScheme(ExternalId.SCHEME_MAILTO))
+ .collect(toImmutableSet()));
+ builder.addExternalIds(
+ newSecondaryEmails.stream()
+ .map(secondaryEmail -> ExternalId.createEmail(accountId, secondaryEmail))
+ .collect(toImmutableSet()));
+ if (accountUpdate.preferredEmail().isPresent()) {
+ builder.addExternalId(
+ ExternalId.createEmail(accountId, accountUpdate.preferredEmail().get()));
+ } else if (accountState.account().preferredEmail() != null) {
+ builder.addExternalId(
+ ExternalId.createEmail(accountId, accountState.account().preferredEmail()));
+ }
}
@Override
diff --git a/java/com/google/gerrit/acceptance/testsuite/account/TestAccount.java b/java/com/google/gerrit/acceptance/testsuite/account/TestAccount.java
index 2574d55..94b1cc4 100644
--- a/java/com/google/gerrit/acceptance/testsuite/account/TestAccount.java
+++ b/java/com/google/gerrit/acceptance/testsuite/account/TestAccount.java
@@ -15,6 +15,8 @@
package com.google.gerrit.acceptance.testsuite.account;
import com.google.auto.value.AutoValue;
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Sets;
import com.google.gerrit.entities.Account;
import java.util.Optional;
@@ -30,6 +32,16 @@
public abstract boolean active();
+ public abstract ImmutableSet<String> emails();
+
+ public ImmutableSet<String> secondaryEmails() {
+ if (!preferredEmail().isPresent()) {
+ return emails();
+ }
+
+ return ImmutableSet.copyOf(Sets.difference(emails(), ImmutableSet.of(preferredEmail().get())));
+ }
+
static Builder builder() {
return new AutoValue_TestAccount.Builder();
}
@@ -46,6 +58,8 @@
abstract Builder active(boolean active);
+ abstract Builder emails(ImmutableSet<String> emails);
+
abstract TestAccount build();
}
}
diff --git a/java/com/google/gerrit/acceptance/testsuite/account/TestAccountCreation.java b/java/com/google/gerrit/acceptance/testsuite/account/TestAccountCreation.java
index 983fec0..042dc9a 100644
--- a/java/com/google/gerrit/acceptance/testsuite/account/TestAccountCreation.java
+++ b/java/com/google/gerrit/acceptance/testsuite/account/TestAccountCreation.java
@@ -14,10 +14,14 @@
package com.google.gerrit.acceptance.testsuite.account;
+import static com.google.common.base.Preconditions.checkState;
+
import com.google.auto.value.AutoValue;
+import com.google.common.collect.ImmutableSet;
import com.google.gerrit.acceptance.testsuite.ThrowingFunction;
import com.google.gerrit.entities.Account;
import java.util.Optional;
+import java.util.Set;
@AutoValue
public abstract class TestAccountCreation {
@@ -33,6 +37,8 @@
public abstract Optional<Boolean> active();
+ public abstract ImmutableSet<String> secondaryEmails();
+
abstract ThrowingFunction<TestAccountCreation, Account.Id> accountCreator();
public static Builder builder(ThrowingFunction<TestAccountCreation, Account.Id> accountCreator) {
@@ -83,14 +89,29 @@
return active(false);
}
+ public abstract Builder secondaryEmails(Set<String> secondaryEmails);
+
+ abstract ImmutableSet.Builder<String> secondaryEmailsBuilder();
+
+ public Builder addSecondaryEmail(String secondaryEmail) {
+ secondaryEmailsBuilder().add(secondaryEmail);
+ return this;
+ }
+
abstract Builder accountCreator(
ThrowingFunction<TestAccountCreation, Account.Id> accountCreator);
abstract TestAccountCreation autoBuild();
public Account.Id create() {
- TestAccountCreation accountUpdate = autoBuild();
- return accountUpdate.accountCreator().applyAndThrowSilently(accountUpdate);
+ TestAccountCreation accountCreation = autoBuild();
+ if (accountCreation.preferredEmail().isPresent()) {
+ checkState(
+ !accountCreation.secondaryEmails().contains(accountCreation.preferredEmail().get()),
+ "preferred email %s cannot be secondary email at the same time",
+ accountCreation.preferredEmail().get());
+ }
+ return accountCreation.accountCreator().applyAndThrowSilently(accountCreation);
}
}
}
diff --git a/java/com/google/gerrit/acceptance/testsuite/account/TestAccountUpdate.java b/java/com/google/gerrit/acceptance/testsuite/account/TestAccountUpdate.java
index da599e7..46988eb 100644
--- a/java/com/google/gerrit/acceptance/testsuite/account/TestAccountUpdate.java
+++ b/java/com/google/gerrit/acceptance/testsuite/account/TestAccountUpdate.java
@@ -15,8 +15,12 @@
package com.google.gerrit.acceptance.testsuite.account;
import com.google.auto.value.AutoValue;
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Sets;
import com.google.gerrit.acceptance.testsuite.ThrowingConsumer;
import java.util.Optional;
+import java.util.Set;
+import java.util.function.Function;
@AutoValue
public abstract class TestAccountUpdate {
@@ -32,11 +36,14 @@
public abstract Optional<Boolean> active();
+ public abstract Function<ImmutableSet<String>, Set<String>> secondaryEmailsModification();
+
abstract ThrowingConsumer<TestAccountUpdate> accountUpdater();
public static Builder builder(ThrowingConsumer<TestAccountUpdate> accountUpdater) {
return new AutoValue_TestAccountUpdate.Builder()
.accountUpdater(accountUpdater)
+ .secondaryEmailsModification(in -> in)
.httpPassword("http-pass");
}
@@ -82,6 +89,37 @@
return active(false);
}
+ abstract Builder secondaryEmailsModification(
+ Function<ImmutableSet<String>, Set<String>> secondaryEmailsModification);
+
+ abstract Function<ImmutableSet<String>, Set<String>> secondaryEmailsModification();
+
+ public Builder clearSecondaryEmails() {
+ return secondaryEmailsModification(originalSecondaryEmail -> ImmutableSet.of());
+ }
+
+ public Builder addSecondaryEmail(String secondaryEmail) {
+ Function<ImmutableSet<String>, Set<String>> secondaryEmailsModification =
+ secondaryEmailsModification();
+ secondaryEmailsModification(
+ originalSecondaryEmails ->
+ Sets.union(
+ secondaryEmailsModification.apply(originalSecondaryEmails),
+ ImmutableSet.of(secondaryEmail)));
+ return this;
+ }
+
+ public Builder removeSecondaryEmail(String secondaryEmail) {
+ Function<ImmutableSet<String>, Set<String>> previousModification =
+ secondaryEmailsModification();
+ secondaryEmailsModification(
+ originalSecondaryEmails ->
+ Sets.difference(
+ previousModification.apply(originalSecondaryEmails),
+ ImmutableSet.of(secondaryEmail)));
+ return this;
+ }
+
abstract Builder accountUpdater(ThrowingConsumer<TestAccountUpdate> accountUpdater);
abstract TestAccountUpdate autoBuild();
diff --git a/java/com/google/gerrit/common/data/CommentDetail.java b/java/com/google/gerrit/common/data/CommentDetail.java
index 55e0143..053764d 100644
--- a/java/com/google/gerrit/common/data/CommentDetail.java
+++ b/java/com/google/gerrit/common/data/CommentDetail.java
@@ -16,6 +16,7 @@
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.Comment;
+import com.google.gerrit.entities.HumanComment;
import com.google.gerrit.entities.PatchSet;
import java.util.ArrayList;
import java.util.List;
@@ -36,7 +37,7 @@
protected CommentDetail() {}
- public void include(Change.Id changeId, Comment p) {
+ public void include(Change.Id changeId, HumanComment p) {
PatchSet.Id psId = PatchSet.id(changeId, p.key.patchSetId);
if (p.side == 0) {
if (idA == null && idB.equals(psId)) {
diff --git a/java/com/google/gerrit/entities/AttentionSetUpdate.java b/java/com/google/gerrit/entities/AttentionSetUpdate.java
index 45588722..00af7a5 100644
--- a/java/com/google/gerrit/entities/AttentionSetUpdate.java
+++ b/java/com/google/gerrit/entities/AttentionSetUpdate.java
@@ -16,6 +16,7 @@
import com.google.auto.value.AutoValue;
import com.google.gerrit.common.Nullable;
+import com.google.gerrit.extensions.api.changes.AttentionSetInput;
import java.time.Instant;
/**
@@ -23,7 +24,7 @@
* in reverse chronological order. Since each update contains all required information and
* invalidates all previous state, only the most recent record is relevant for each user.
*
- * <p>See {@link com.google.gerrit.extensions.api.changes.AddToAttentionSetInput} and {@link
+ * <p>See {@link AttentionSetInput} and {@link
* com.google.gerrit.extensions.api.changes.RemoveFromAttentionSetInput} for the representation in
* the API.
*/
diff --git a/java/com/google/gerrit/entities/Change.java b/java/com/google/gerrit/entities/Change.java
index b36b5f9..845a9bb 100644
--- a/java/com/google/gerrit/entities/Change.java
+++ b/java/com/google/gerrit/entities/Change.java
@@ -39,7 +39,7 @@
* |
* +- {@link PatchSetApproval}: a +/- vote on the change's current state.
* |
- * +- {@link Comment}: comment about a specific line
+ * +- {@link HumanComment}: comment about a specific line
* </pre>
*
* <p>
diff --git a/java/com/google/gerrit/entities/Comment.java b/java/com/google/gerrit/entities/Comment.java
index 9c58fef..2c10c87 100644
--- a/java/com/google/gerrit/entities/Comment.java
+++ b/java/com/google/gerrit/entities/Comment.java
@@ -24,15 +24,15 @@
import org.eclipse.jgit.lib.ObjectId;
/**
- * This class represents inline comments in NoteDb. This means it determines the JSON format for
- * inline comments in the revision notes that NoteDb uses to persist inline comments.
+ * This class is a base class that can be extended by the different types of inline comment
+ * entities.
*
* <p>Changing fields in this class changes the storage format of inline comments in NoteDb and may
* require a corresponding data migration (adding new optional fields is generally okay).
*
- * <p>Consider updating {@link #getApproximateSize()} when adding/changing fields.
+ * <p>Consider updating {@link #getCommentFieldApproximateSize()} when adding/changing fields.
*/
-public class Comment {
+public abstract class Comment {
public enum Status {
DRAFT('d'),
@@ -301,11 +301,13 @@
* Returns the comment's approximate size. This is used to enforce size limits and should
* therefore include all unbounded fields (e.g. String-s).
*/
- public int getApproximateSize() {
+ protected int getCommentFieldApproximateSize() {
return nullableLength(message, parentUuid, tag, revId, serverId)
+ (key != null ? nullableLength(key.filename, key.uuid) : 0);
}
+ public abstract int getApproximateSize();
+
static int nullableLength(String... strings) {
int length = 0;
for (String s : strings) {
diff --git a/java/com/google/gerrit/entities/HumanComment.java b/java/com/google/gerrit/entities/HumanComment.java
new file mode 100644
index 0000000..8b687cc
--- /dev/null
+++ b/java/com/google/gerrit/entities/HumanComment.java
@@ -0,0 +1,67 @@
+// Copyright (C) 2020 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.entities;
+
+import java.sql.Timestamp;
+
+/**
+ * This class represents inline human comments in NoteDb. This means it determines the JSON format
+ * for inline comments in the revision notes that NoteDb uses to persist inline comments.
+ *
+ * <p>Changing fields in this class changes the storage format of inline comments in NoteDb and may
+ * require a corresponding data migration (adding new optional fields is generally okay).
+ *
+ * <p>Consider updating {@link #getApproximateSize()} when adding/changing fields.
+ */
+public class HumanComment extends Comment {
+
+ public HumanComment(
+ Key key,
+ Account.Id author,
+ Timestamp writtenOn,
+ short side,
+ String message,
+ String serverId,
+ boolean unresolved) {
+ super(key, author, writtenOn, side, message, serverId, unresolved);
+ }
+
+ public HumanComment(HumanComment comment) {
+ super(comment);
+ }
+
+ @Override
+ public int getApproximateSize() {
+ return super.getCommentFieldApproximateSize();
+ }
+
+ @Override
+ public String toString() {
+ return toStringHelper().toString();
+ }
+
+ @Override
+ public boolean equals(Object o) {
+ if (!(o instanceof HumanComment)) {
+ return false;
+ }
+ return super.equals(o);
+ }
+
+ @Override
+ public int hashCode() {
+ return super.hashCode();
+ }
+}
diff --git a/java/com/google/gerrit/entities/RobotComment.java b/java/com/google/gerrit/entities/RobotComment.java
index 9256e79..03ddad5 100644
--- a/java/com/google/gerrit/entities/RobotComment.java
+++ b/java/com/google/gerrit/entities/RobotComment.java
@@ -42,7 +42,8 @@
@Override
public int getApproximateSize() {
- int approximateSize = super.getApproximateSize() + nullableLength(robotId, robotRunId, url);
+ int approximateSize =
+ super.getCommentFieldApproximateSize() + nullableLength(robotId, robotRunId, url);
approximateSize +=
properties != null
? properties.entrySet().stream()
@@ -66,4 +67,23 @@
.add("fixSuggestions", Objects.toString(fixSuggestions, ""))
.toString();
}
+
+ @Override
+ public boolean equals(Object o) {
+ if (!(o instanceof RobotComment)) {
+ return false;
+ }
+ RobotComment c = (RobotComment) o;
+ return super.equals(o)
+ && Objects.equals(robotId, c.robotId)
+ && Objects.equals(robotRunId, c.robotRunId)
+ && Objects.equals(url, c.url)
+ && Objects.equals(properties, c.properties)
+ && Objects.equals(fixSuggestions, c.fixSuggestions);
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(super.hashCode(), robotId, robotRunId, url, properties, fixSuggestions);
+ }
}
diff --git a/java/com/google/gerrit/extensions/api/changes/AddToAttentionSetInput.java b/java/com/google/gerrit/extensions/api/changes/AttentionSetInput.java
similarity index 87%
rename from java/com/google/gerrit/extensions/api/changes/AddToAttentionSetInput.java
rename to java/com/google/gerrit/extensions/api/changes/AttentionSetInput.java
index 39efc64..1a173e9 100644
--- a/java/com/google/gerrit/extensions/api/changes/AddToAttentionSetInput.java
+++ b/java/com/google/gerrit/extensions/api/changes/AttentionSetInput.java
@@ -20,14 +20,14 @@
* @see RemoveFromAttentionSetInput
* @see com.google.gerrit.extensions.common.AttentionSetEntry
*/
-public class AddToAttentionSetInput {
+public class AttentionSetInput {
public String user;
public String reason;
- public AddToAttentionSetInput(String user, String reason) {
+ public AttentionSetInput(String user, String reason) {
this.user = user;
this.reason = reason;
}
- public AddToAttentionSetInput() {}
+ public AttentionSetInput() {}
}
diff --git a/java/com/google/gerrit/extensions/api/changes/ChangeApi.java b/java/com/google/gerrit/extensions/api/changes/ChangeApi.java
index 8df5343..e8b58f9 100644
--- a/java/com/google/gerrit/extensions/api/changes/ChangeApi.java
+++ b/java/com/google/gerrit/extensions/api/changes/ChangeApi.java
@@ -302,7 +302,7 @@
AttentionSetApi attention(String id) throws RestApiException;
/** Adds a user to the attention set. */
- AccountInfo addToAttentionSet(AddToAttentionSetInput input) throws RestApiException;
+ AccountInfo addToAttentionSet(AttentionSetInput input) throws RestApiException;
/** Set the assignee of a change. */
AccountInfo setAssignee(AssigneeInput input) throws RestApiException;
@@ -578,7 +578,7 @@
}
@Override
- public AccountInfo addToAttentionSet(AddToAttentionSetInput input) throws RestApiException {
+ public AccountInfo addToAttentionSet(AttentionSetInput input) throws RestApiException {
throw new NotImplementedException();
}
diff --git a/java/com/google/gerrit/extensions/api/changes/RemoveFromAttentionSetInput.java b/java/com/google/gerrit/extensions/api/changes/RemoveFromAttentionSetInput.java
index 9212788..b14c53d 100644
--- a/java/com/google/gerrit/extensions/api/changes/RemoveFromAttentionSetInput.java
+++ b/java/com/google/gerrit/extensions/api/changes/RemoveFromAttentionSetInput.java
@@ -19,7 +19,7 @@
/**
* Input at API level to remove a user from the attention set.
*
- * @see AddToAttentionSetInput
+ * @see AttentionSetInput
* @see com.google.gerrit.extensions.common.AttentionSetEntry
*/
public class RemoveFromAttentionSetInput {
diff --git a/java/com/google/gerrit/mail/HtmlParser.java b/java/com/google/gerrit/mail/HtmlParser.java
index 7905a0a..2fc659d 100644
--- a/java/com/google/gerrit/mail/HtmlParser.java
+++ b/java/com/google/gerrit/mail/HtmlParser.java
@@ -18,7 +18,7 @@
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterators;
import com.google.common.collect.PeekingIterator;
-import com.google.gerrit.entities.Comment;
+import com.google.gerrit.entities.HumanComment;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
@@ -60,7 +60,7 @@
* @return list of MailComments parsed from the html part of the email
*/
public static List<MailComment> parse(
- MailMessage email, Collection<Comment> comments, String changeUrl) {
+ MailMessage email, Collection<HumanComment> comments, String changeUrl) {
// TODO(hiesel) Add support for Gmail Mobile
// TODO(hiesel) Add tests for other popular email clients
@@ -71,10 +71,10 @@
// Gerrit as these are generally more reliable then the text captions.
List<MailComment> parsedComments = new ArrayList<>();
Document d = Jsoup.parse(email.htmlContent());
- PeekingIterator<Comment> iter = Iterators.peekingIterator(comments.iterator());
+ PeekingIterator<HumanComment> iter = Iterators.peekingIterator(comments.iterator());
String lastEncounteredFileName = null;
- Comment lastEncounteredComment = null;
+ HumanComment lastEncounteredComment = null;
for (Element e : d.body().getAllElements()) {
String elementName = e.tagName();
boolean isInBlockQuote =
@@ -91,7 +91,7 @@
if (!iter.hasNext()) {
continue;
}
- Comment perspectiveComment = iter.peek();
+ HumanComment perspectiveComment = iter.peek();
if (href.equals(ParserUtil.filePath(changeUrl, perspectiveComment))) {
if (lastEncounteredFileName == null
|| !lastEncounteredFileName.equals(perspectiveComment.key.filename)) {
diff --git a/java/com/google/gerrit/mail/MailComment.java b/java/com/google/gerrit/mail/MailComment.java
index f024f17..3e7da10 100644
--- a/java/com/google/gerrit/mail/MailComment.java
+++ b/java/com/google/gerrit/mail/MailComment.java
@@ -14,7 +14,7 @@
package com.google.gerrit.mail;
-import com.google.gerrit.entities.Comment;
+import com.google.gerrit.entities.HumanComment;
import java.util.Objects;
/** A comment parsed from inbound email */
@@ -26,7 +26,7 @@
}
CommentType type;
- Comment inReplyTo;
+ HumanComment inReplyTo;
String fileName;
String message;
boolean isLink;
@@ -34,7 +34,7 @@
public MailComment() {}
public MailComment(
- String message, String fileName, Comment inReplyTo, CommentType type, boolean isLink) {
+ String message, String fileName, HumanComment inReplyTo, CommentType type, boolean isLink) {
this.message = message;
this.fileName = fileName;
this.inReplyTo = inReplyTo;
@@ -46,7 +46,7 @@
return type;
}
- public Comment getInReplyTo() {
+ public HumanComment getInReplyTo() {
return inReplyTo;
}
diff --git a/java/com/google/gerrit/mail/TextParser.java b/java/com/google/gerrit/mail/TextParser.java
index dac3deb..a33c66f 100644
--- a/java/com/google/gerrit/mail/TextParser.java
+++ b/java/com/google/gerrit/mail/TextParser.java
@@ -18,7 +18,7 @@
import com.google.common.base.Strings;
import com.google.common.collect.Iterators;
import com.google.common.collect.PeekingIterator;
-import com.google.gerrit.entities.Comment;
+import com.google.gerrit.entities.HumanComment;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
@@ -31,15 +31,15 @@
* Parses comments from plaintext email.
*
* @param email @param email the message as received from the email service
- * @param comments list of {@link Comment}s previously persisted on the change that caused the
- * original notification email to be sent out. Ordering must be the same as in the outbound
- * email
+ * @param comments list of {@link HumanComment}s previously persisted on the change that caused
+ * the original notification email to be sent out. Ordering must be the same as in the
+ * outbound email
* @param changeUrl canonical change url that points to the change on this Gerrit instance.
* Example: https://go-review.googlesource.com/#/c/91570
* @return list of MailComments parsed from the plaintext part of the email
*/
public static List<MailComment> parse(
- MailMessage email, Collection<Comment> comments, String changeUrl) {
+ MailMessage email, Collection<HumanComment> comments, String changeUrl) {
String body = email.textContent();
// Replace CR-LF by \n
body = body.replace("\r\n", "\n");
@@ -62,11 +62,11 @@
body = body.replace(doubleQuotePattern, singleQuotePattern);
}
- PeekingIterator<Comment> iter = Iterators.peekingIterator(comments.iterator());
+ PeekingIterator<HumanComment> iter = Iterators.peekingIterator(comments.iterator());
MailComment currentComment = null;
String lastEncounteredFileName = null;
- Comment lastEncounteredComment = null;
+ HumanComment lastEncounteredComment = null;
for (String line : Splitter.on('\n').split(body)) {
if (line.equals(">")) {
// Skip empty lines
@@ -89,7 +89,7 @@
if (!iter.hasNext()) {
continue;
}
- Comment perspectiveComment = iter.peek();
+ HumanComment perspectiveComment = iter.peek();
if (line.equals(ParserUtil.filePath(changeUrl, perspectiveComment))) {
if (lastEncounteredFileName == null
|| !lastEncounteredFileName.equals(perspectiveComment.key.filename)) {
diff --git a/java/com/google/gerrit/server/CommentsUtil.java b/java/com/google/gerrit/server/CommentsUtil.java
index e8b44aa..0b3d1cb 100644
--- a/java/com/google/gerrit/server/CommentsUtil.java
+++ b/java/com/google/gerrit/server/CommentsUtil.java
@@ -28,6 +28,7 @@
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.ChangeMessage;
import com.google.gerrit.entities.Comment;
+import com.google.gerrit.entities.HumanComment;
import com.google.gerrit.entities.Patch;
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.entities.RefNames;
@@ -116,7 +117,7 @@
this.serverId = serverId;
}
- public Comment newComment(
+ public HumanComment newHumanComment(
ChangeContext ctx,
String path,
PatchSet.Id psId,
@@ -132,15 +133,15 @@
} else {
// Inherit unresolved value from inReplyTo comment if not specified.
Comment.Key key = new Comment.Key(parentUuid, path, psId.get());
- Optional<Comment> parent = getPublished(ctx.getNotes(), key);
+ Optional<HumanComment> parent = getPublishedHumanComment(ctx.getNotes(), key);
if (!parent.isPresent()) {
throw new UnprocessableEntityException("Invalid parentUuid supplied for comment");
}
unresolved = parent.get().unresolved;
}
}
- Comment c =
- new Comment(
+ HumanComment c =
+ new HumanComment(
new Comment.Key(ChangeUtil.messageUuid(), path, psId.get()),
ctx.getUser().getAccountId(),
ctx.getWhen(),
@@ -175,19 +176,21 @@
return c;
}
- public Optional<Comment> getPublished(ChangeNotes notes, Comment.Key key) {
- return publishedByChange(notes).stream().filter(c -> key.equals(c.key)).findFirst();
+ public Optional<HumanComment> getPublishedHumanComment(ChangeNotes notes, Comment.Key key) {
+ return publishedHumanCommentsByChange(notes).stream()
+ .filter(c -> key.equals(c.key))
+ .findFirst();
}
- public Optional<Comment> getDraft(ChangeNotes notes, IdentifiedUser user, Comment.Key key) {
+ public Optional<HumanComment> getDraft(ChangeNotes notes, IdentifiedUser user, Comment.Key key) {
return draftByChangeAuthor(notes, user.getAccountId()).stream()
.filter(c -> key.equals(c.key))
.findFirst();
}
- public List<Comment> publishedByChange(ChangeNotes notes) {
+ public List<HumanComment> publishedHumanCommentsByChange(ChangeNotes notes) {
notes.load();
- return sort(Lists.newArrayList(notes.getComments().values()));
+ return sort(Lists.newArrayList(notes.getHumanComments().values()));
}
public List<RobotComment> robotCommentsByChange(ChangeNotes notes) {
@@ -195,8 +198,8 @@
return sort(Lists.newArrayList(notes.getRobotComments().values()));
}
- public List<Comment> draftByChange(ChangeNotes notes) {
- List<Comment> comments = new ArrayList<>();
+ public List<HumanComment> draftByChange(ChangeNotes notes) {
+ List<HumanComment> comments = new ArrayList<>();
for (Ref ref : getDraftRefs(notes.getChangeId())) {
Account.Id account = Account.Id.fromRefSuffix(ref.getName());
if (account != null) {
@@ -206,8 +209,8 @@
return sort(comments);
}
- public List<Comment> byPatchSet(ChangeNotes notes, PatchSet.Id psId) {
- List<Comment> comments = new ArrayList<>();
+ public List<HumanComment> byPatchSet(ChangeNotes notes, PatchSet.Id psId) {
+ List<HumanComment> comments = new ArrayList<>();
comments.addAll(publishedByPatchSet(notes, psId));
for (Ref ref : getDraftRefs(notes.getChangeId())) {
@@ -219,13 +222,13 @@
return sort(comments);
}
- public List<Comment> publishedByChangeFile(ChangeNotes notes, String file) {
- return commentsOnFile(notes.load().getComments().values(), file);
+ public List<HumanComment> publishedByChangeFile(ChangeNotes notes, String file) {
+ return commentsOnFile(notes.load().getHumanComments().values(), file);
}
- public List<Comment> publishedByPatchSet(ChangeNotes notes, PatchSet.Id psId) {
+ public List<HumanComment> publishedByPatchSet(ChangeNotes notes, PatchSet.Id psId) {
return removeCommentsOnAncestorOfCommitMessage(
- commentsOnPatchSet(notes.load().getComments().values(), psId));
+ commentsOnPatchSet(notes.load().getHumanComments().values(), psId));
}
public List<RobotComment> robotCommentsByPatchSet(ChangeNotes notes, PatchSet.Id psId) {
@@ -288,29 +291,31 @@
* auto-merge was done. From that time there may still be comments on the auto-merge commit
* message and those we want to filter out.
*/
- private List<Comment> removeCommentsOnAncestorOfCommitMessage(List<Comment> list) {
+ private List<HumanComment> removeCommentsOnAncestorOfCommitMessage(List<HumanComment> list) {
return list.stream()
.filter(c -> c.side != 0 || !Patch.COMMIT_MSG.equals(c.key.filename))
.collect(toList());
}
- public List<Comment> draftByPatchSetAuthor(
+ public List<HumanComment> draftByPatchSetAuthor(
PatchSet.Id psId, Account.Id author, ChangeNotes notes) {
return commentsOnPatchSet(notes.load().getDraftComments(author).values(), psId);
}
- public List<Comment> draftByChangeFileAuthor(ChangeNotes notes, String file, Account.Id author) {
+ public List<HumanComment> draftByChangeFileAuthor(
+ ChangeNotes notes, String file, Account.Id author) {
return commentsOnFile(notes.load().getDraftComments(author).values(), file);
}
- public List<Comment> draftByChangeAuthor(ChangeNotes notes, Account.Id author) {
- List<Comment> comments = new ArrayList<>();
+ public List<HumanComment> draftByChangeAuthor(ChangeNotes notes, Account.Id author) {
+ List<HumanComment> comments = new ArrayList<>();
comments.addAll(notes.getDraftComments(author).values());
return sort(comments);
}
- public void putComments(ChangeUpdate update, Comment.Status status, Iterable<Comment> comments) {
- for (Comment c : comments) {
+ public void putHumanComments(
+ ChangeUpdate update, HumanComment.Status status, Iterable<HumanComment> comments) {
+ for (HumanComment c : comments) {
update.putComment(status, c);
}
}
@@ -321,8 +326,8 @@
}
}
- public void deleteComments(ChangeUpdate update, Iterable<Comment> comments) {
- for (Comment c : comments) {
+ public void deleteHumanComments(ChangeUpdate update, Iterable<HumanComment> comments) {
+ for (HumanComment c : comments) {
update.deleteComment(c);
}
}
@@ -332,9 +337,10 @@
update.deleteCommentByRewritingHistory(commentKey.uuid, newMessage);
}
- private static List<Comment> commentsOnFile(Collection<Comment> allComments, String file) {
- List<Comment> result = new ArrayList<>(allComments.size());
- for (Comment c : allComments) {
+ private static List<HumanComment> commentsOnFile(
+ Collection<HumanComment> allComments, String file) {
+ List<HumanComment> result = new ArrayList<>(allComments.size());
+ for (HumanComment c : allComments) {
String currentFilename = c.key.filename;
if (currentFilename.equals(file)) {
result.add(c);
diff --git a/java/com/google/gerrit/server/PublishCommentUtil.java b/java/com/google/gerrit/server/PublishCommentUtil.java
index 3d34d6b..658af15 100644
--- a/java/com/google/gerrit/server/PublishCommentUtil.java
+++ b/java/com/google/gerrit/server/PublishCommentUtil.java
@@ -20,8 +20,7 @@
import com.google.common.collect.ImmutableList;
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.common.Nullable;
-import com.google.gerrit.entities.Comment;
-import com.google.gerrit.entities.Comment.Status;
+import com.google.gerrit.entities.HumanComment;
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.extensions.validators.CommentForValidation;
@@ -60,7 +59,7 @@
public void publish(
ChangeContext ctx,
ChangeUpdate changeUpdate,
- Collection<Comment> draftComments,
+ Collection<HumanComment> draftComments,
@Nullable String tag) {
ChangeNotes notes = ctx.getNotes();
checkArgument(notes != null);
@@ -70,8 +69,8 @@
Map<PatchSet.Id, PatchSet> patchSets =
psUtil.getAsMap(notes, draftComments.stream().map(d -> psId(notes, d)).collect(toSet()));
- Set<Comment> commentsToPublish = new HashSet<>();
- for (Comment draftComment : draftComments) {
+ Set<HumanComment> commentsToPublish = new HashSet<>();
+ for (HumanComment draftComment : draftComments) {
PatchSet.Id psIdOfDraftComment = psId(notes, draftComment);
PatchSet ps = patchSets.get(psIdOfDraftComment);
if (ps == null) {
@@ -109,10 +108,10 @@
}
commentsToPublish.add(draftComment);
}
- commentsUtil.putComments(changeUpdate, Status.PUBLISHED, commentsToPublish);
+ commentsUtil.putHumanComments(changeUpdate, HumanComment.Status.PUBLISHED, commentsToPublish);
}
- private static PatchSet.Id psId(ChangeNotes notes, Comment c) {
+ private static PatchSet.Id psId(ChangeNotes notes, HumanComment c) {
return PatchSet.id(notes.getChangeId(), c.key.patchSetId);
}
diff --git a/java/com/google/gerrit/server/PublishCommentsOp.java b/java/com/google/gerrit/server/PublishCommentsOp.java
index 745755b..358ce92 100644
--- a/java/com/google/gerrit/server/PublishCommentsOp.java
+++ b/java/com/google/gerrit/server/PublishCommentsOp.java
@@ -16,7 +16,7 @@
import com.google.common.collect.ImmutableMap;
import com.google.gerrit.entities.ChangeMessage;
-import com.google.gerrit.entities.Comment;
+import com.google.gerrit.entities.HumanComment;
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.entities.Project;
import com.google.gerrit.exceptions.StorageException;
@@ -58,7 +58,7 @@
private final PatchSet.Id psId;
private final PublishCommentUtil publishCommentUtil;
- private List<Comment> comments = new ArrayList<>();
+ private List<HumanComment> comments = new ArrayList<>();
private ChangeMessage message;
private IdentifiedUser user;
diff --git a/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java b/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java
index 5122f8a..b4a5da7 100644
--- a/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java
+++ b/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java
@@ -23,9 +23,9 @@
import com.google.gerrit.extensions.api.changes.AbandonInput;
import com.google.gerrit.extensions.api.changes.AddReviewerInput;
import com.google.gerrit.extensions.api.changes.AddReviewerResult;
-import com.google.gerrit.extensions.api.changes.AddToAttentionSetInput;
import com.google.gerrit.extensions.api.changes.AssigneeInput;
import com.google.gerrit.extensions.api.changes.AttentionSetApi;
+import com.google.gerrit.extensions.api.changes.AttentionSetInput;
import com.google.gerrit.extensions.api.changes.ChangeApi;
import com.google.gerrit.extensions.api.changes.ChangeEditApi;
import com.google.gerrit.extensions.api.changes.ChangeMessageApi;
@@ -543,7 +543,7 @@
}
@Override
- public AccountInfo addToAttentionSet(AddToAttentionSetInput input) throws RestApiException {
+ public AccountInfo addToAttentionSet(AttentionSetInput input) throws RestApiException {
try {
return addToAttentionSet.apply(change, input).value();
} catch (Exception e) {
diff --git a/java/com/google/gerrit/server/api/changes/CommentApiImpl.java b/java/com/google/gerrit/server/api/changes/CommentApiImpl.java
index c5fcab1..35dd9c1 100644
--- a/java/com/google/gerrit/server/api/changes/CommentApiImpl.java
+++ b/java/com/google/gerrit/server/api/changes/CommentApiImpl.java
@@ -20,7 +20,7 @@
import com.google.gerrit.extensions.api.changes.DeleteCommentInput;
import com.google.gerrit.extensions.common.CommentInfo;
import com.google.gerrit.extensions.restapi.RestApiException;
-import com.google.gerrit.server.change.CommentResource;
+import com.google.gerrit.server.change.HumanCommentResource;
import com.google.gerrit.server.restapi.change.DeleteComment;
import com.google.gerrit.server.restapi.change.GetComment;
import com.google.inject.Inject;
@@ -28,16 +28,16 @@
class CommentApiImpl implements CommentApi {
interface Factory {
- CommentApiImpl create(CommentResource c);
+ CommentApiImpl create(HumanCommentResource c);
}
private final GetComment getComment;
private final DeleteComment deleteComment;
- private final CommentResource comment;
+ private final HumanCommentResource comment;
@Inject
CommentApiImpl(
- GetComment getComment, DeleteComment deleteComment, @Assisted CommentResource comment) {
+ GetComment getComment, DeleteComment deleteComment, @Assisted HumanCommentResource comment) {
this.getComment = getComment;
this.deleteComment = deleteComment;
this.comment = comment;
diff --git a/java/com/google/gerrit/server/change/AddToAttentionSetOp.java b/java/com/google/gerrit/server/change/AddToAttentionSetOp.java
index 829c290..2bc0773 100644
--- a/java/com/google/gerrit/server/change/AddToAttentionSetOp.java
+++ b/java/com/google/gerrit/server/change/AddToAttentionSetOp.java
@@ -17,7 +17,6 @@
import static com.google.common.collect.ImmutableMap.toImmutableMap;
import static java.util.Objects.requireNonNull;
-import com.google.common.collect.ImmutableSet;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.AttentionSetUpdate;
import com.google.gerrit.entities.AttentionSetUpdate.Operation;
@@ -36,24 +35,36 @@
public class AddToAttentionSetOp implements BatchUpdateOp {
public interface Factory {
- AddToAttentionSetOp create(Account.Id attentionUserId, String reason);
+ AddToAttentionSetOp create(
+ Account.Id attentionUserId, String reason, boolean withChangeMessage);
}
private final ChangeData.Factory changeDataFactory;
private final ChangeMessagesUtil cmUtil;
private final Account.Id attentionUserId;
private final String reason;
+ private final boolean withChangeMessage;
+ /**
+ * Add a specified user to the attention set.
+ *
+ * @param attentionUserId the id of the user we want to add to the attention set.
+ * @param reason The reason for adding that user.
+ * @param withChangeMessage Whether or not we wish to add a change message detailing about adding
+ * that user to the attention set.
+ */
@Inject
AddToAttentionSetOp(
ChangeData.Factory changeDataFactory,
ChangeMessagesUtil cmUtil,
@Assisted Account.Id attentionUserId,
- @Assisted String reason) {
+ @Assisted String reason,
+ @Assisted boolean withChangeMessage) {
this.changeDataFactory = changeDataFactory;
this.cmUtil = cmUtil;
this.attentionUserId = requireNonNull(attentionUserId, "user");
this.reason = requireNonNull(reason, "reason");
+ this.withChangeMessage = withChangeMessage;
}
@Override
@@ -68,15 +79,16 @@
}
ChangeUpdate update = ctx.getUpdate(ctx.getChange().currentPatchSetId());
- update.setAttentionSetUpdates(
- ImmutableSet.of(
- AttentionSetUpdate.createForWrite(
- attentionUserId, AttentionSetUpdate.Operation.ADD, reason)));
- addMessage(ctx, update);
+ update.addToPlannedAttentionSetUpdates(
+ AttentionSetUpdate.createForWrite(
+ attentionUserId, AttentionSetUpdate.Operation.ADD, reason));
+ if (withChangeMessage) {
+ addChangeMessage(ctx, update);
+ }
return true;
}
- private void addMessage(ChangeContext ctx, ChangeUpdate update) {
+ private void addChangeMessage(ChangeContext ctx, ChangeUpdate update) {
String message = "Added to attention set: " + attentionUserId;
cmUtil.addChangeMessage(
update,
diff --git a/java/com/google/gerrit/server/change/DraftCommentResource.java b/java/com/google/gerrit/server/change/DraftCommentResource.java
index 3d3e8f9..e0648cf 100644
--- a/java/com/google/gerrit/server/change/DraftCommentResource.java
+++ b/java/com/google/gerrit/server/change/DraftCommentResource.java
@@ -15,7 +15,7 @@
package com.google.gerrit.server.change;
import com.google.gerrit.entities.Change;
-import com.google.gerrit.entities.Comment;
+import com.google.gerrit.entities.HumanComment;
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.extensions.restapi.RestResource;
import com.google.gerrit.extensions.restapi.RestView;
@@ -27,9 +27,9 @@
new TypeLiteral<RestView<DraftCommentResource>>() {};
private final RevisionResource rev;
- private final Comment comment;
+ private final HumanComment comment;
- public DraftCommentResource(RevisionResource rev, Comment c) {
+ public DraftCommentResource(RevisionResource rev, HumanComment c) {
this.rev = rev;
this.comment = c;
}
@@ -46,7 +46,7 @@
return rev.getPatchSet();
}
- public Comment getComment() {
+ public HumanComment getComment() {
return comment;
}
diff --git a/java/com/google/gerrit/server/change/EmailReviewComments.java b/java/com/google/gerrit/server/change/EmailReviewComments.java
index f1eff6f..b30a6a3 100644
--- a/java/com/google/gerrit/server/change/EmailReviewComments.java
+++ b/java/com/google/gerrit/server/change/EmailReviewComments.java
@@ -65,7 +65,7 @@
PatchSet patchSet,
IdentifiedUser user,
ChangeMessage message,
- List<Comment> comments,
+ List<? extends Comment> comments,
String patchSetComment,
List<LabelVote> labels,
RepoView repoView);
@@ -82,7 +82,7 @@
private final PatchSet patchSet;
private final IdentifiedUser user;
private final ChangeMessage message;
- private final List<Comment> comments;
+ private final List<? extends Comment> comments;
private final String patchSetComment;
private final List<LabelVote> labels;
private final RepoView repoView;
@@ -99,7 +99,7 @@
@Assisted PatchSet patchSet,
@Assisted IdentifiedUser user,
@Assisted ChangeMessage message,
- @Assisted List<Comment> comments,
+ @Assisted List<? extends Comment> comments,
@Nullable @Assisted String patchSetComment,
@Assisted List<LabelVote> labels,
@Assisted RepoView repoView) {
diff --git a/java/com/google/gerrit/server/change/CommentResource.java b/java/com/google/gerrit/server/change/HumanCommentResource.java
similarity index 76%
rename from java/com/google/gerrit/server/change/CommentResource.java
rename to java/com/google/gerrit/server/change/HumanCommentResource.java
index dbe7a76..1611aaa 100644
--- a/java/com/google/gerrit/server/change/CommentResource.java
+++ b/java/com/google/gerrit/server/change/HumanCommentResource.java
@@ -15,20 +15,20 @@
package com.google.gerrit.server.change;
import com.google.gerrit.entities.Account;
-import com.google.gerrit.entities.Comment;
+import com.google.gerrit.entities.HumanComment;
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.extensions.restapi.RestResource;
import com.google.gerrit.extensions.restapi.RestView;
import com.google.inject.TypeLiteral;
-public class CommentResource implements RestResource {
- public static final TypeLiteral<RestView<CommentResource>> COMMENT_KIND =
- new TypeLiteral<RestView<CommentResource>>() {};
+public class HumanCommentResource implements RestResource {
+ public static final TypeLiteral<RestView<HumanCommentResource>> COMMENT_KIND =
+ new TypeLiteral<RestView<HumanCommentResource>>() {};
private final RevisionResource rev;
- private final Comment comment;
+ private final HumanComment comment;
- public CommentResource(RevisionResource rev, Comment c) {
+ public HumanCommentResource(RevisionResource rev, HumanComment c) {
this.rev = rev;
this.comment = c;
}
@@ -37,7 +37,7 @@
return rev.getPatchSet();
}
- public Comment getComment() {
+ public HumanComment getComment() {
return comment;
}
diff --git a/java/com/google/gerrit/server/change/RemoveFromAttentionSetOp.java b/java/com/google/gerrit/server/change/RemoveFromAttentionSetOp.java
index 07f0d78..4680629 100644
--- a/java/com/google/gerrit/server/change/RemoveFromAttentionSetOp.java
+++ b/java/com/google/gerrit/server/change/RemoveFromAttentionSetOp.java
@@ -17,7 +17,6 @@
import static com.google.common.collect.ImmutableMap.toImmutableMap;
import static java.util.Objects.requireNonNull;
-import com.google.common.collect.ImmutableSet;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.AttentionSetUpdate;
import com.google.gerrit.entities.AttentionSetUpdate.Operation;
@@ -36,24 +35,36 @@
public class RemoveFromAttentionSetOp implements BatchUpdateOp {
public interface Factory {
- RemoveFromAttentionSetOp create(Account.Id attentionUserId, String reason);
+ RemoveFromAttentionSetOp create(
+ Account.Id attentionUserId, String reason, boolean withChangeMessage);
}
private final ChangeData.Factory changeDataFactory;
private final ChangeMessagesUtil cmUtil;
private final Account.Id attentionUserId;
private final String reason;
+ private final boolean withChangeMessage;
+ /**
+ * Remove a specified user from the attention set.
+ *
+ * @param attentionUserId the id of the user we want to add to the attention set.
+ * @param reason The reason for adding that user.
+ * @param withChangeMessage Whether or not we wish to add a change message detailing about adding
+ * that user to the attention set.
+ */
@Inject
RemoveFromAttentionSetOp(
ChangeData.Factory changeDataFactory,
ChangeMessagesUtil cmUtil,
@Assisted Account.Id attentionUserId,
- @Assisted String reason) {
+ @Assisted String reason,
+ @Assisted boolean withChangeMessage) {
this.changeDataFactory = changeDataFactory;
this.cmUtil = cmUtil;
this.attentionUserId = requireNonNull(attentionUserId, "user");
this.reason = requireNonNull(reason, "reason");
+ this.withChangeMessage = withChangeMessage;
}
@Override
@@ -68,14 +79,15 @@
}
ChangeUpdate update = ctx.getUpdate(ctx.getChange().currentPatchSetId());
- update.setAttentionSetUpdates(
- ImmutableSet.of(
- AttentionSetUpdate.createForWrite(attentionUserId, Operation.REMOVE, reason)));
- addMessage(ctx, update);
+ update.addToPlannedAttentionSetUpdates(
+ AttentionSetUpdate.createForWrite(attentionUserId, Operation.REMOVE, reason));
+ if (withChangeMessage) {
+ addChangeMessage(ctx, update);
+ }
return true;
}
- private void addMessage(ChangeContext ctx, ChangeUpdate update) {
+ private void addChangeMessage(ChangeContext ctx, ChangeUpdate update) {
String message = "Removed from attention set: " + attentionUserId;
cmUtil.addChangeMessage(
update,
diff --git a/java/com/google/gerrit/server/events/EventFactory.java b/java/com/google/gerrit/server/events/EventFactory.java
index f0da560..19d2f3d 100644
--- a/java/com/google/gerrit/server/events/EventFactory.java
+++ b/java/com/google/gerrit/server/events/EventFactory.java
@@ -28,7 +28,7 @@
import com.google.gerrit.entities.BranchNameKey;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.ChangeMessage;
-import com.google.gerrit.entities.Comment;
+import com.google.gerrit.entities.HumanComment;
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.entities.PatchSetApproval;
import com.google.gerrit.entities.UserIdentity;
@@ -380,8 +380,8 @@
}
public void addPatchSetComments(
- PatchSetAttribute patchSetAttribute, Collection<Comment> comments) {
- for (Comment comment : comments) {
+ PatchSetAttribute patchSetAttribute, Collection<HumanComment> comments) {
+ for (HumanComment comment : comments) {
if (comment.key.patchSetId == patchSetAttribute.number) {
if (patchSetAttribute.comments == null) {
patchSetAttribute.comments = new ArrayList<>();
@@ -547,7 +547,7 @@
return a;
}
- public PatchSetCommentAttribute asPatchSetLineAttribute(Comment c) {
+ public PatchSetCommentAttribute asPatchSetLineAttribute(HumanComment c) {
PatchSetCommentAttribute a = new PatchSetCommentAttribute();
a.reviewer = asAccountAttribute(c.author.getId());
a.file = c.key.filename;
diff --git a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
index 404fa3c..5436db7 100644
--- a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
+++ b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
@@ -69,7 +69,7 @@
import com.google.gerrit.entities.BooleanProjectConfig;
import com.google.gerrit.entities.BranchNameKey;
import com.google.gerrit.entities.Change;
-import com.google.gerrit.entities.Comment;
+import com.google.gerrit.entities.HumanComment;
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.entities.PatchSetInfo;
import com.google.gerrit.entities.Project;
@@ -2022,7 +2022,7 @@
}
if (magicBranch != null && magicBranch.shouldPublishComments()) {
- List<Comment> drafts =
+ List<HumanComment> drafts =
commentsUtil.draftByChangeAuthor(
notesFactory.createChecked(change), user.getAccountId());
ImmutableList<CommentForValidation> draftsForValidation =
@@ -2144,9 +2144,10 @@
// A's group.
// C) Commit is a PatchSet of a pre-existing change uploaded with a
// different target branch.
- for (Ref ref : existingRefs) {
- updateGroups.add(new UpdateGroupsRequest(ref, c));
- }
+ existingRefs.stream()
+ .map(r -> PatchSet.Id.fromRef(r.getName()))
+ .filter(Objects::nonNull)
+ .forEach(i -> updateGroups.add(new UpdateGroupsRequest(i, c)));
if (!(newChangeForAllNotInTarget || magicBranch.base != null)) {
continue;
}
@@ -3019,8 +3020,8 @@
final RevCommit commit;
List<String> groups = ImmutableList.of();
- UpdateGroupsRequest(Ref ref, RevCommit commit) {
- this.psId = requireNonNull(PatchSet.Id.fromRef(ref.getName()));
+ UpdateGroupsRequest(PatchSet.Id psId, RevCommit commit) {
+ this.psId = psId;
this.commit = commit;
}
diff --git a/java/com/google/gerrit/server/git/validators/CommentCountValidator.java b/java/com/google/gerrit/server/git/validators/CommentCountValidator.java
index 67aa3bd..a554f90 100644
--- a/java/com/google/gerrit/server/git/validators/CommentCountValidator.java
+++ b/java/com/google/gerrit/server/git/validators/CommentCountValidator.java
@@ -45,7 +45,7 @@
ChangeNotes notes =
notesFactory.createChecked(Project.nameKey(ctx.getProject()), Change.id(ctx.getChangeId()));
int numExistingCommentsAndChangeMessages =
- notes.getComments().size()
+ notes.getHumanComments().size()
+ notes.getRobotComments().size()
+ notes.getChangeMessages().size();
if (!comments.isEmpty()
diff --git a/java/com/google/gerrit/server/git/validators/CommentCumulativeSizeValidator.java b/java/com/google/gerrit/server/git/validators/CommentCumulativeSizeValidator.java
index d9a1420..d507531 100644
--- a/java/com/google/gerrit/server/git/validators/CommentCumulativeSizeValidator.java
+++ b/java/com/google/gerrit/server/git/validators/CommentCumulativeSizeValidator.java
@@ -51,7 +51,7 @@
notesFactory.createChecked(Project.nameKey(ctx.getProject()), Change.id(ctx.getChangeId()));
int existingCumulativeSize =
Stream.concat(
- notes.getComments().values().stream(),
+ notes.getHumanComments().values().stream(),
notes.getRobotComments().values().stream())
.mapToInt(Comment::getApproximateSize)
.sum()
diff --git a/java/com/google/gerrit/server/mail/receive/MailProcessor.java b/java/com/google/gerrit/server/mail/receive/MailProcessor.java
index f004e4b..b38bef6 100644
--- a/java/com/google/gerrit/server/mail/receive/MailProcessor.java
+++ b/java/com/google/gerrit/server/mail/receive/MailProcessor.java
@@ -24,7 +24,7 @@
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.ChangeMessage;
-import com.google.gerrit.entities.Comment;
+import com.google.gerrit.entities.HumanComment;
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.entities.Project;
import com.google.gerrit.exceptions.StorageException;
@@ -257,7 +257,7 @@
// Get all comments; filter and sort them to get the original list of
// comments from the outbound email.
// TODO(hiesel) Also filter by original comment author.
- Collection<Comment> comments =
+ Collection<HumanComment> comments =
cd.publishedComments().stream()
.filter(c -> (c.writtenOn.getTime() / 1000) == (metadata.timestamp.getTime() / 1000))
.sorted(CommentsUtil.COMMENT_ORDER)
@@ -319,7 +319,7 @@
private final List<MailComment> parsedComments;
private final String tag;
private ChangeMessage changeMessage;
- private List<Comment> comments;
+ private List<HumanComment> comments;
private PatchSet patchSet;
private ChangeNotes notes;
@@ -349,8 +349,10 @@
comments.add(
persistentCommentFromMailComment(ctx, c, targetPatchSetForComment(ctx, c, patchSet)));
}
- commentsUtil.putComments(
- ctx.getUpdate(ctx.getChange().currentPatchSetId()), Comment.Status.PUBLISHED, comments);
+ commentsUtil.putHumanComments(
+ ctx.getUpdate(ctx.getChange().currentPatchSetId()),
+ HumanComment.Status.PUBLISHED,
+ comments);
return true;
}
@@ -416,7 +418,7 @@
return current;
}
- private Comment persistentCommentFromMailComment(
+ private HumanComment persistentCommentFromMailComment(
ChangeContext ctx, MailComment mailComment, PatchSet patchSetForComment)
throws UnprocessableEntityException, PatchListNotAvailableException {
String fileName;
@@ -431,8 +433,8 @@
side = Side.REVISION;
}
- Comment comment =
- commentsUtil.newComment(
+ HumanComment comment =
+ commentsUtil.newHumanComment(
ctx,
fileName,
patchSetForComment.id(),
diff --git a/java/com/google/gerrit/server/mail/send/CommentSender.java b/java/com/google/gerrit/server/mail/send/CommentSender.java
index f3cccf2..0ba5da51 100644
--- a/java/com/google/gerrit/server/mail/send/CommentSender.java
+++ b/java/com/google/gerrit/server/mail/send/CommentSender.java
@@ -22,6 +22,7 @@
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.Comment;
+import com.google.gerrit.entities.HumanComment;
import com.google.gerrit.entities.KeyUtil;
import com.google.gerrit.entities.Patch;
import com.google.gerrit.entities.Project;
@@ -114,7 +115,7 @@
}
}
- private List<Comment> inlineComments = Collections.emptyList();
+ private List<? extends Comment> inlineComments = Collections.emptyList();
private String patchSetComment;
private List<LabelVote> labels = Collections.emptyList();
private final CommentsUtil commentsUtil;
@@ -136,7 +137,7 @@
this.replyToAddress = cfg.getString("sendemail", null, "replyToAddress");
}
- public void setComments(List<Comment> comments) {
+ public void setComments(List<? extends Comment> comments) {
inlineComments = comments;
}
@@ -244,23 +245,21 @@
/** Get the set of accounts whose comments have been replied to in this email. */
private HashSet<Account.Id> getReplyAccounts() {
HashSet<Account.Id> replyAccounts = new HashSet<>();
-
// Track visited parent UUIDs to avoid cycles.
HashSet<String> visitedUuids = new HashSet<>();
for (Comment comment : inlineComments) {
visitedUuids.add(comment.key.uuid);
-
// Traverse the parent relation to the top of the comment thread.
Comment current = comment;
while (current.parentUuid != null && !visitedUuids.contains(current.parentUuid)) {
- Optional<Comment> optParent = getParent(current);
+ Optional<HumanComment> optParent = getParent(current);
if (!optParent.isPresent()) {
// There is a parent UUID, but it cannot be loaded, break from the comment thread.
break;
}
- Comment parent = optParent.get();
+ HumanComment parent = optParent.get();
replyAccounts.add(parent.author.getId());
visitedUuids.add(current.parentUuid);
current = parent;
@@ -307,14 +306,13 @@
* @return an optional comment that will be present if the given comment has a parent, and is
* empty if it does not.
*/
- private Optional<Comment> getParent(Comment child) {
+ private Optional<HumanComment> getParent(Comment child) {
if (child.parentUuid == null) {
return Optional.empty();
}
-
Comment.Key key = new Comment.Key(child.parentUuid, child.key.filename, child.key.patchSetId);
try {
- return commentsUtil.getPublished(changeData.notes(), key);
+ return commentsUtil.getPublishedHumanComment(changeData.notes(), key);
} catch (StorageException e) {
logger.atWarning().log("Could not find the parent of this comment: %s", child);
return Optional.empty();
@@ -448,7 +446,7 @@
// If the comment has a quote, don't bother loading the parent message.
if (!hasQuote(blocks)) {
// Set parent comment info.
- Optional<Comment> parent = getParent(comment);
+ Optional<HumanComment> parent = getParent(comment);
if (parent.isPresent()) {
commentData.put("parentMessage", getShortenedCommentMessage(parent.get()));
}
diff --git a/java/com/google/gerrit/server/notedb/ChangeDraftUpdate.java b/java/com/google/gerrit/server/notedb/ChangeDraftUpdate.java
index 07b58f2..57f6353 100644
--- a/java/com/google/gerrit/server/notedb/ChangeDraftUpdate.java
+++ b/java/com/google/gerrit/server/notedb/ChangeDraftUpdate.java
@@ -22,6 +22,7 @@
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.Comment;
+import com.google.gerrit.entities.HumanComment;
import com.google.gerrit.entities.Project;
import com.google.gerrit.entities.RefNames;
import com.google.gerrit.exceptions.StorageException;
@@ -82,13 +83,13 @@
FIXED
}
- private static Key key(Comment c) {
+ private static Key key(HumanComment c) {
return new AutoValue_ChangeDraftUpdate_Key(c.getCommitId(), c.key);
}
private final AllUsersName draftsProject;
- private List<Comment> put = new ArrayList<>();
+ private List<HumanComment> put = new ArrayList<>();
private Map<Key, DeleteReason> delete = new HashMap<>();
@AssistedInject
@@ -119,7 +120,7 @@
this.draftsProject = allUsers;
}
- public void putComment(Comment c) {
+ public void putComment(HumanComment c) {
checkState(!put.contains(c), "comment already added");
verifyComment(c);
put.add(c);
@@ -128,7 +129,7 @@
/**
* Marks a comment for deletion. Called when the comment is deleted because the user published it.
*/
- public void markCommentPublished(Comment c) {
+ public void markCommentPublished(HumanComment c) {
checkState(!delete.containsKey(key(c)), "comment already marked for deletion");
verifyComment(c);
delete.put(key(c), DeleteReason.PUBLISHED);
@@ -137,7 +138,7 @@
/**
* Marks a comment for deletion. Called when the comment is deleted because the user removed it.
*/
- public void deleteComment(Comment c) {
+ public void deleteComment(HumanComment c) {
checkState(!delete.containsKey(key(c)), "comment already marked for deletion");
verifyComment(c);
delete.put(key(c), DeleteReason.DELETED);
@@ -191,7 +192,7 @@
RevisionNoteMap<ChangeRevisionNote> rnm = getRevisionNoteMap(rw, curr);
RevisionNoteBuilder.Cache cache = new RevisionNoteBuilder.Cache(rnm);
- for (Comment c : put) {
+ for (HumanComment c : put) {
if (!delete.keySet().contains(key(c))) {
cache.get(c.getCommitId()).putComment(c);
}
@@ -259,7 +260,7 @@
// Even though reading from changes might not be enabled, we need to
// parse any existing revision notes so we can merge them.
return RevisionNoteMap.parse(
- noteUtil.getChangeNoteJson(), rw.getObjectReader(), noteMap, Comment.Status.DRAFT);
+ noteUtil.getChangeNoteJson(), rw.getObjectReader(), noteMap, HumanComment.Status.DRAFT);
}
@Override
diff --git a/java/com/google/gerrit/server/notedb/ChangeNotes.java b/java/com/google/gerrit/server/notedb/ChangeNotes.java
index 36a61cc0..cf854c7 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNotes.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNotes.java
@@ -45,6 +45,7 @@
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.ChangeMessage;
import com.google.gerrit.entities.Comment;
+import com.google.gerrit.entities.HumanComment;
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.entities.PatchSetApproval;
import com.google.gerrit.entities.Project;
@@ -435,14 +436,14 @@
}
/** @return inline comments on each revision. */
- public ImmutableListMultimap<ObjectId, Comment> getComments() {
+ public ImmutableListMultimap<ObjectId, HumanComment> getHumanComments() {
return state.publishedComments();
}
public ImmutableSet<Comment.Key> getCommentKeys() {
if (commentKeys == null) {
ImmutableSet.Builder<Comment.Key> b = ImmutableSet.builder();
- for (Comment c : getComments().values()) {
+ for (Comment c : getHumanComments().values()) {
b.add(new Comment.Key(c.key));
}
commentKeys = b.build();
@@ -454,11 +455,11 @@
return state.updateCount();
}
- public ImmutableListMultimap<ObjectId, Comment> getDraftComments(Account.Id author) {
+ public ImmutableListMultimap<ObjectId, HumanComment> getDraftComments(Account.Id author) {
return getDraftComments(author, null);
}
- public ImmutableListMultimap<ObjectId, Comment> getDraftComments(
+ public ImmutableListMultimap<ObjectId, HumanComment> getDraftComments(
Account.Id author, @Nullable Ref ref) {
loadDraftComments(author, ref);
// Filter out any zombie draft comments. These are drafts that are also in
@@ -502,7 +503,7 @@
return robotCommentNotes;
}
- public boolean containsComment(Comment c) {
+ public boolean containsComment(HumanComment c) {
if (containsCommentPublished(c)) {
return true;
}
@@ -511,7 +512,7 @@
}
public boolean containsCommentPublished(Comment c) {
- for (Comment l : getComments().values()) {
+ for (Comment l : getHumanComments().values()) {
if (c.key.equals(l.key)) {
return true;
}
diff --git a/java/com/google/gerrit/server/notedb/ChangeNotesParser.java b/java/com/google/gerrit/server/notedb/ChangeNotesParser.java
index a884b70..f639f49 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNotesParser.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNotesParser.java
@@ -61,7 +61,7 @@
import com.google.gerrit.entities.AttentionSetUpdate;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.ChangeMessage;
-import com.google.gerrit.entities.Comment;
+import com.google.gerrit.entities.HumanComment;
import com.google.gerrit.entities.LabelId;
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.entities.PatchSetApproval;
@@ -121,7 +121,7 @@
private final List<AssigneeStatusUpdate> assigneeUpdates;
private final List<SubmitRecord> submitRecords;
- private final ListMultimap<ObjectId, Comment> comments;
+ private final ListMultimap<ObjectId, HumanComment> humanComments;
private final Map<PatchSet.Id, PatchSet.Builder> patchSets;
private final Set<PatchSet.Id> deletedPatchSets;
private final Map<PatchSet.Id, PatchSetState> patchSetStates;
@@ -178,7 +178,7 @@
assigneeUpdates = new ArrayList<>();
submitRecords = Lists.newArrayListWithExpectedSize(1);
allChangeMessages = new ArrayList<>();
- comments = MultimapBuilder.hashKeys().arrayListValues().build();
+ humanComments = MultimapBuilder.hashKeys().arrayListValues().build();
patchSets = new HashMap<>();
deletedPatchSets = new HashSet<>();
patchSetStates = new HashMap<>();
@@ -249,7 +249,7 @@
assigneeUpdates,
submitRecords,
buildAllMessages(),
- comments,
+ humanComments,
firstNonNull(isPrivate, false),
firstNonNull(workInProgress, false),
firstNonNull(hasReviewStarted, true),
@@ -735,12 +735,12 @@
ChangeNotesCommit tipCommit = walk.parseCommit(tip);
revisionNoteMap =
RevisionNoteMap.parse(
- changeNoteJson, reader, NoteMap.read(reader, tipCommit), Comment.Status.PUBLISHED);
+ changeNoteJson, reader, NoteMap.read(reader, tipCommit), HumanComment.Status.PUBLISHED);
Map<ObjectId, ChangeRevisionNote> rns = revisionNoteMap.revisionNotes;
for (Map.Entry<ObjectId, ChangeRevisionNote> e : rns.entrySet()) {
- for (Comment c : e.getValue().getEntities()) {
- comments.put(e.getKey(), c);
+ for (HumanComment c : e.getValue().getEntities()) {
+ humanComments.put(e.getKey(), c);
}
}
@@ -1055,7 +1055,7 @@
pruneEntitiesForMissingPatchSets(allChangeMessages, ChangeMessage::getPatchSetId, missing);
pruned +=
pruneEntitiesForMissingPatchSets(
- comments.values(), c -> PatchSet.id(id, c.key.patchSetId), missing);
+ humanComments.values(), c -> PatchSet.id(id, c.key.patchSetId), missing);
pruned +=
pruneEntitiesForMissingPatchSets(
approvals.values(), psa -> psa.key().patchSetId(), missing);
diff --git a/java/com/google/gerrit/server/notedb/ChangeNotesState.java b/java/com/google/gerrit/server/notedb/ChangeNotesState.java
index 0f27b75..a2ca066 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNotesState.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNotesState.java
@@ -39,7 +39,7 @@
import com.google.gerrit.entities.BranchNameKey;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.ChangeMessage;
-import com.google.gerrit.entities.Comment;
+import com.google.gerrit.entities.HumanComment;
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.entities.PatchSetApproval;
import com.google.gerrit.entities.Project;
@@ -123,7 +123,7 @@
List<AssigneeStatusUpdate> assigneeUpdates,
List<SubmitRecord> submitRecords,
List<ChangeMessage> changeMessages,
- ListMultimap<ObjectId, Comment> publishedComments,
+ ListMultimap<ObjectId, HumanComment> publishedComments,
boolean isPrivate,
boolean workInProgress,
boolean reviewStarted,
@@ -314,7 +314,7 @@
abstract ImmutableList<ChangeMessage> changeMessages();
- abstract ImmutableListMultimap<ObjectId, Comment> publishedComments();
+ abstract ImmutableListMultimap<ObjectId, HumanComment> publishedComments();
abstract int updateCount();
@@ -427,7 +427,7 @@
abstract Builder changeMessages(List<ChangeMessage> changeMessages);
- abstract Builder publishedComments(ListMultimap<ObjectId, Comment> publishedComments);
+ abstract Builder publishedComments(ListMultimap<ObjectId, HumanComment> publishedComments);
abstract Builder updateCount(int updateCount);
@@ -634,8 +634,8 @@
.collect(toImmutableList()))
.publishedComments(
proto.getPublishedCommentList().stream()
- .map(r -> GSON.fromJson(r, Comment.class))
- .collect(toImmutableListMultimap(Comment::getCommitId, c -> c)))
+ .map(r -> GSON.fromJson(r, HumanComment.class))
+ .collect(toImmutableListMultimap(HumanComment::getCommitId, c -> c)))
.updateCount(proto.getUpdateCount());
return b.build();
}
diff --git a/java/com/google/gerrit/server/notedb/ChangeRevisionNote.java b/java/com/google/gerrit/server/notedb/ChangeRevisionNote.java
index 4e52093..bf2cf07 100644
--- a/java/com/google/gerrit/server/notedb/ChangeRevisionNote.java
+++ b/java/com/google/gerrit/server/notedb/ChangeRevisionNote.java
@@ -16,7 +16,7 @@
import static java.nio.charset.StandardCharsets.UTF_8;
-import com.google.gerrit.entities.Comment;
+import com.google.gerrit.entities.HumanComment;
import java.io.ByteArrayInputStream;
import java.io.IOException;
import java.io.InputStream;
@@ -29,13 +29,13 @@
import org.eclipse.jgit.util.MutableInteger;
/** Implements the parsing of comment data, handling JSON decoding and push certificates. */
-class ChangeRevisionNote extends RevisionNote<Comment> {
+class ChangeRevisionNote extends RevisionNote<HumanComment> {
private final ChangeNoteJson noteJson;
- private final Comment.Status status;
+ private final HumanComment.Status status;
private String pushCert;
ChangeRevisionNote(
- ChangeNoteJson noteJson, ObjectReader reader, ObjectId noteId, Comment.Status status) {
+ ChangeNoteJson noteJson, ObjectReader reader, ObjectId noteId, HumanComment.Status status) {
super(reader, noteId);
this.noteJson = noteJson;
this.status = status;
@@ -47,12 +47,13 @@
}
@Override
- protected List<Comment> parse(byte[] raw, int offset) throws IOException, ConfigInvalidException {
+ protected List<HumanComment> parse(byte[] raw, int offset)
+ throws IOException, ConfigInvalidException {
MutableInteger p = new MutableInteger();
p.value = offset;
- RevisionNoteData data = parseJson(noteJson, raw, p.value);
- if (status == Comment.Status.PUBLISHED) {
+ HumanCommentsRevisionNoteData data = parseJson(noteJson, raw, p.value);
+ if (status == HumanComment.Status.PUBLISHED) {
pushCert = data.pushCert;
} else {
pushCert = null;
@@ -60,11 +61,11 @@
return data.comments;
}
- private RevisionNoteData parseJson(ChangeNoteJson noteUtil, byte[] raw, int offset)
+ private HumanCommentsRevisionNoteData parseJson(ChangeNoteJson noteUtil, byte[] raw, int offset)
throws IOException {
try (InputStream is = new ByteArrayInputStream(raw, offset, raw.length - offset);
Reader r = new InputStreamReader(is, UTF_8)) {
- return noteUtil.getGson().fromJson(r, RevisionNoteData.class);
+ return noteUtil.getGson().fromJson(r, HumanCommentsRevisionNoteData.class);
}
}
}
diff --git a/java/com/google/gerrit/server/notedb/ChangeUpdate.java b/java/com/google/gerrit/server/notedb/ChangeUpdate.java
index 41b1b94..ba4ee69 100644
--- a/java/com/google/gerrit/server/notedb/ChangeUpdate.java
+++ b/java/com/google/gerrit/server/notedb/ChangeUpdate.java
@@ -50,6 +50,7 @@
import com.google.common.base.Joiner;
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Table;
import com.google.common.collect.TreeBasedTable;
import com.google.gerrit.common.data.SubmitRecord;
@@ -57,6 +58,7 @@
import com.google.gerrit.entities.AttentionSetUpdate;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.Comment;
+import com.google.gerrit.entities.HumanComment;
import com.google.gerrit.entities.Project;
import com.google.gerrit.entities.RobotComment;
import com.google.gerrit.entities.SubmissionId;
@@ -65,6 +67,7 @@
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.project.ProjectCache;
+import com.google.gerrit.server.util.AttentionSetUtil;
import com.google.gerrit.server.util.LabelVote;
import com.google.gerrit.server.validators.ValidationException;
import com.google.inject.assistedinject.Assisted;
@@ -73,6 +76,7 @@
import java.util.ArrayList;
import java.util.Comparator;
import java.util.Date;
+import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedHashMap;
import java.util.List;
@@ -80,6 +84,7 @@
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
+import java.util.stream.Collectors;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.CommitBuilder;
import org.eclipse.jgit.lib.ObjectId;
@@ -118,7 +123,7 @@
private final Table<String, Account.Id, Optional<Short>> approvals;
private final Map<Account.Id, ReviewerStateInternal> reviewers = new LinkedHashMap<>();
private final Map<Address, ReviewerStateInternal> reviewersByEmail = new LinkedHashMap<>();
- private final List<Comment> comments = new ArrayList<>();
+ private final List<HumanComment> comments = new ArrayList<>();
private String commitSubject;
private String subject;
@@ -129,7 +134,7 @@
private String submissionId;
private String topic;
private String commit;
- private Set<AttentionSetUpdate> attentionSetUpdates;
+ private Map<Account.Id, AttentionSetUpdate> plannedAttentionSetUpdates;
private Optional<Account.Id> assignee;
private Set<String> hashtags;
private String changeMessage;
@@ -285,10 +290,10 @@
this.psDescription = psDescription;
}
- public void putComment(Comment.Status status, Comment c) {
+ public void putComment(HumanComment.Status status, HumanComment c) {
verifyComment(c);
createDraftUpdateIfNull();
- if (status == Comment.Status.DRAFT) {
+ if (status == HumanComment.Status.DRAFT) {
draftUpdate.putComment(c);
} else {
comments.add(c);
@@ -302,7 +307,7 @@
robotCommentUpdate.putComment(c);
}
- public void deleteComment(Comment c) {
+ public void deleteComment(HumanComment c) {
verifyComment(c);
createDraftUpdateIfNull().deleteComment(c);
}
@@ -372,17 +377,39 @@
/**
* All updates must have a timestamp of null since we use the commit's timestamp. There also must
- * not be multiple updates for a single user.
+ * not be multiple updates for a single user. Only the first update takes place because of the
+ * different priorities: e.g, if we want to add someone to the attention set but also want to
+ * remove someone from the attention set, we should ensure to add/remove that user based on the
+ * priority of the addition and removal. If most importantly we want to remove the user, then we
+ * must first create the removal, and the addition will not take effect.
*/
- public void setAttentionSetUpdates(Set<AttentionSetUpdate> attentionSetUpdates) {
+ public void addToPlannedAttentionSetUpdates(Set<AttentionSetUpdate> updates) {
+ if (updates == null || updates.isEmpty()) {
+ return;
+ }
checkArgument(
- attentionSetUpdates.stream().noneMatch(a -> a.timestamp() != null),
+ updates.stream().noneMatch(a -> a.timestamp() != null),
"must not specify timestamp for write");
+
checkArgument(
- attentionSetUpdates.stream().map(AttentionSetUpdate::account).distinct().count()
- == attentionSetUpdates.size(),
+ updates.stream().map(AttentionSetUpdate::account).distinct().count() == updates.size(),
"must not specify multiple updates for single user");
- this.attentionSetUpdates = attentionSetUpdates;
+
+ if (plannedAttentionSetUpdates == null) {
+ plannedAttentionSetUpdates = new HashMap();
+ }
+
+ Set<Account.Id> currentAccountUpdates =
+ plannedAttentionSetUpdates.values().stream()
+ .map(u -> u.account())
+ .collect(Collectors.toSet());
+ updates.stream()
+ .filter(u -> !currentAccountUpdates.contains(u.account()))
+ .forEach(u -> plannedAttentionSetUpdates.putIfAbsent(u.account(), u));
+ }
+
+ public void addToPlannedAttentionSetUpdates(AttentionSetUpdate update) {
+ addToPlannedAttentionSetUpdates(ImmutableSet.of(update));
}
public void setAssignee(Account.Id assignee) {
@@ -449,7 +476,7 @@
RevisionNoteMap<ChangeRevisionNote> rnm = getRevisionNoteMap(rw, curr);
RevisionNoteBuilder.Cache cache = new RevisionNoteBuilder.Cache(rnm);
- for (Comment c : comments) {
+ for (HumanComment c : comments) {
c.tag = tag;
cache.get(c.getCommitId()).putComment(c);
}
@@ -486,7 +513,7 @@
// Even though reading from changes might not be enabled, we need to
// parse any existing revision notes so we can merge them.
return RevisionNoteMap.parse(
- noteUtil.getChangeNoteJson(), rw.getObjectReader(), noteMap, Comment.Status.PUBLISHED);
+ noteUtil.getChangeNoteJson(), rw.getObjectReader(), noteMap, HumanComment.Status.PUBLISHED);
}
private void checkComments(
@@ -583,6 +610,12 @@
if (status != null) {
addFooter(msg, FOOTER_STATUS, status.name().toLowerCase());
+ if (status.equals(Change.Status.ABANDONED)) {
+ clearAttentionSet("Change was abandoned");
+ }
+ if (status.equals(Change.Status.MERGED)) {
+ clearAttentionSet("Change was submitted");
+ }
}
if (topic != null) {
@@ -593,12 +626,6 @@
addFooter(msg, FOOTER_COMMIT, commit);
}
- if (attentionSetUpdates != null) {
- for (AttentionSetUpdate attentionSetUpdate : attentionSetUpdates) {
- addFooter(msg, FOOTER_ATTENTION, noteUtil.attentionSetUpdateToJson(attentionSetUpdate));
- }
- }
-
if (assignee != null) {
if (assignee.isPresent()) {
addFooter(msg, FOOTER_ASSIGNEE);
@@ -625,7 +652,7 @@
addFooter(msg, e.getValue().getFooterKey());
noteUtil.appendAccountIdIdentString(msg, e.getKey()).append('\n');
}
-
+ applyReviewerUpdatesToAttentionSet();
for (Map.Entry<Address, ReviewerStateInternal> e : reviewersByEmail.entrySet()) {
addFooter(msg, e.getValue().getByEmailFooterKey(), e.getKey().toString());
}
@@ -686,6 +713,11 @@
if (workInProgress != null) {
addFooter(msg, FOOTER_WORK_IN_PROGRESS, workInProgress);
+ if (workInProgress) {
+ clearAttentionSet("Change was marked work in progress");
+ } else {
+ addAllReviewersToAttentionSet();
+ }
}
if (revertOf != null) {
@@ -696,6 +728,10 @@
addFooter(msg, FOOTER_CHERRY_PICK_OF, cherryPickOf);
}
+ if (plannedAttentionSetUpdates != null) {
+ updateAttentionSet(msg);
+ }
+
CommitBuilder cb = new CommitBuilder();
cb.setMessage(msg.toString());
try {
@@ -709,6 +745,68 @@
return cb;
}
+ private void clearAttentionSet(String reason) {
+ if (getNotes().getAttentionSet() == null) {
+ return;
+ }
+ AttentionSetUtil.additionsOnly(getNotes().getAttentionSet()).stream()
+ .map(
+ a ->
+ AttentionSetUpdate.createForWrite(
+ a.account(), AttentionSetUpdate.Operation.REMOVE, reason))
+ .forEach(a -> addToPlannedAttentionSetUpdates(a));
+ }
+
+ private void applyReviewerUpdatesToAttentionSet() {
+ if ((workInProgress != null && workInProgress == true)
+ || getNotes().getChange().isWorkInProgress()) {
+ // Users shouldn't be added to the attention set if the change is work in progress.
+ return;
+ }
+ Set<Account.Id> currentReviewers =
+ getNotes().getReviewers().byState(ReviewerStateInternal.REVIEWER);
+ Set<AttentionSetUpdate> updates = new HashSet();
+ for (Map.Entry<Account.Id, ReviewerStateInternal> reviewer : reviewers.entrySet()) {
+ // Only add new reviewers to the attention set.
+ if (reviewer.getValue().equals(ReviewerStateInternal.REVIEWER)
+ && !currentReviewers.contains(reviewer.getKey())) {
+ updates.add(
+ AttentionSetUpdate.createForWrite(
+ reviewer.getKey(), AttentionSetUpdate.Operation.ADD, "Reviewer was added"));
+ }
+ // Treat both REMOVED and CC as "removed reviewers".
+ if (!reviewer.getValue().equals(ReviewerStateInternal.REVIEWER)
+ && currentReviewers.contains(reviewer.getKey())) {
+ updates.add(
+ AttentionSetUpdate.createForWrite(
+ reviewer.getKey(), AttentionSetUpdate.Operation.REMOVE, "Reviewer was removed"));
+ }
+ }
+ addToPlannedAttentionSetUpdates(updates);
+ }
+
+ private void addAllReviewersToAttentionSet() {
+ getNotes().getReviewers().byState(ReviewerStateInternal.REVIEWER).stream()
+ .map(
+ r ->
+ AttentionSetUpdate.createForWrite(
+ r, AttentionSetUpdate.Operation.ADD, "Change was marked ready for review"))
+ .forEach(a -> addToPlannedAttentionSetUpdates(a));
+ }
+
+ /**
+ * Any updates to the attention set must be done in {@link #addToPlannedAttentionSetUpdates}. This
+ * method is called after all the updates are finished to do the updates once and for real.
+ */
+ private void updateAttentionSet(StringBuilder msg) {
+ if (plannedAttentionSetUpdates == null) {
+ return;
+ }
+ for (AttentionSetUpdate attentionSetUpdate : plannedAttentionSetUpdates.values()) {
+ addFooter(msg, FOOTER_ATTENTION, noteUtil.attentionSetUpdateToJson(attentionSetUpdate));
+ }
+ }
+
private void addPatchSetFooter(StringBuilder sb, int ps) {
addFooter(sb, FOOTER_PATCH_SET).append(ps);
if (psState != null) {
@@ -735,7 +833,7 @@
&& status == null
&& submissionId == null
&& submitRecords == null
- && attentionSetUpdates == null
+ && plannedAttentionSetUpdates == null
&& assignee == null
&& hashtags == null
&& topic == null
diff --git a/java/com/google/gerrit/server/notedb/DeleteCommentRewriter.java b/java/com/google/gerrit/server/notedb/DeleteCommentRewriter.java
index 9c8b369..d0b6247 100644
--- a/java/com/google/gerrit/server/notedb/DeleteCommentRewriter.java
+++ b/java/com/google/gerrit/server/notedb/DeleteCommentRewriter.java
@@ -15,14 +15,13 @@
package com.google.gerrit.server.notedb;
import static com.google.common.base.Preconditions.checkArgument;
-import static com.google.gerrit.entities.Comment.Status;
import static java.util.stream.Collectors.toList;
import static java.util.stream.Collectors.toMap;
import static org.eclipse.jgit.lib.Constants.OBJ_BLOB;
import com.google.common.annotations.VisibleForTesting;
import com.google.gerrit.entities.Change;
-import com.google.gerrit.entities.Comment;
+import com.google.gerrit.entities.HumanComment;
import com.google.gerrit.entities.RefNames;
import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
@@ -94,14 +93,14 @@
ObjectReader reader = revWalk.getObjectReader();
RevCommit newTipCommit = revWalk.next(); // The first commit will not be rewritten.
- Map<String, Comment> parentComments =
+ Map<String, HumanComment> parentComments =
getPublishedComments(noteUtil, reader, NoteMap.read(reader, newTipCommit));
boolean rewrite = false;
RevCommit originalCommit;
while ((originalCommit = revWalk.next()) != null) {
NoteMap noteMap = NoteMap.read(reader, originalCommit);
- Map<String, Comment> currComments = getPublishedComments(noteUtil, reader, noteMap);
+ Map<String, HumanComment> currComments = getPublishedComments(noteUtil, reader, noteMap);
if (!rewrite && currComments.containsKey(uuid)) {
rewrite = true;
@@ -113,8 +112,8 @@
continue;
}
- List<Comment> putInComments = getPutInComments(parentComments, currComments);
- List<Comment> deletedComments = getDeletedComments(parentComments, currComments);
+ List<HumanComment> putInComments = getPutInComments(parentComments, currComments);
+ List<HumanComment> deletedComments = getDeletedComments(parentComments, currComments);
newTipCommit =
revWalk.parseCommit(
rewriteCommit(
@@ -130,16 +129,16 @@
* the previous commits.
*/
@VisibleForTesting
- public static Map<String, Comment> getPublishedComments(
+ public static Map<String, HumanComment> getPublishedComments(
ChangeNoteJson changeNoteJson, ObjectReader reader, NoteMap noteMap)
throws IOException, ConfigInvalidException {
- return RevisionNoteMap.parse(changeNoteJson, reader, noteMap, Status.PUBLISHED).revisionNotes
- .values().stream()
+ return RevisionNoteMap.parse(changeNoteJson, reader, noteMap, HumanComment.Status.PUBLISHED)
+ .revisionNotes.values().stream()
.flatMap(n -> n.getEntities().stream())
.collect(toMap(c -> c.key.uuid, Function.identity()));
}
- public static Map<String, Comment> getPublishedComments(
+ public static Map<String, HumanComment> getPublishedComments(
ChangeNoteUtil noteUtil, ObjectReader reader, NoteMap noteMap)
throws IOException, ConfigInvalidException {
return getPublishedComments(noteUtil.getChangeNoteJson(), reader, noteMap);
@@ -152,11 +151,12 @@
* @param curMap the comment map of the current commit.
* @return The comments put in by the current commit.
*/
- private List<Comment> getPutInComments(Map<String, Comment> parMap, Map<String, Comment> curMap) {
- List<Comment> comments = new ArrayList<>();
+ private List<HumanComment> getPutInComments(
+ Map<String, HumanComment> parMap, Map<String, HumanComment> curMap) {
+ List<HumanComment> comments = new ArrayList<>();
for (String key : curMap.keySet()) {
if (!parMap.containsKey(key)) {
- Comment comment = curMap.get(key);
+ HumanComment comment = curMap.get(key);
if (key.equals(uuid)) {
comment.message = newMessage;
}
@@ -173,8 +173,8 @@
* @param curMap the comment map of the current commit.
* @return The comments deleted by the current commit.
*/
- private List<Comment> getDeletedComments(
- Map<String, Comment> parMap, Map<String, Comment> curMap) {
+ private List<HumanComment> getDeletedComments(
+ Map<String, HumanComment> parMap, Map<String, HumanComment> curMap) {
return parMap.entrySet().stream()
.filter(c -> !curMap.containsKey(c.getKey()))
.map(Map.Entry::getValue)
@@ -199,22 +199,22 @@
RevCommit parentCommit,
ObjectInserter inserter,
ObjectReader reader,
- List<Comment> putInComments,
- List<Comment> deletedComments)
+ List<HumanComment> putInComments,
+ List<HumanComment> deletedComments)
throws IOException, ConfigInvalidException {
RevisionNoteMap<ChangeRevisionNote> revNotesMap =
RevisionNoteMap.parse(
noteUtil.getChangeNoteJson(),
reader,
NoteMap.read(reader, parentCommit),
- Status.PUBLISHED);
+ HumanComment.Status.PUBLISHED);
RevisionNoteBuilder.Cache cache = new RevisionNoteBuilder.Cache(revNotesMap);
- for (Comment c : putInComments) {
+ for (HumanComment c : putInComments) {
cache.get(c.getCommitId()).putComment(c);
}
- for (Comment c : deletedComments) {
+ for (HumanComment c : deletedComments) {
cache.get(c.getCommitId()).deleteComment(c.key);
}
diff --git a/java/com/google/gerrit/server/notedb/DraftCommentNotes.java b/java/com/google/gerrit/server/notedb/DraftCommentNotes.java
index 3966396..9b403e8 100644
--- a/java/com/google/gerrit/server/notedb/DraftCommentNotes.java
+++ b/java/com/google/gerrit/server/notedb/DraftCommentNotes.java
@@ -26,7 +26,7 @@
import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.Change;
-import com.google.gerrit.entities.Comment;
+import com.google.gerrit.entities.HumanComment;
import com.google.gerrit.entities.Project;
import com.google.inject.assistedinject.Assisted;
import com.google.inject.assistedinject.AssistedInject;
@@ -50,7 +50,7 @@
private final Account.Id author;
private final Ref ref;
- private ImmutableListMultimap<ObjectId, Comment> comments;
+ private ImmutableListMultimap<ObjectId, HumanComment> comments;
private RevisionNoteMap<ChangeRevisionNote> revisionNoteMap;
@AssistedInject
@@ -80,12 +80,12 @@
return author;
}
- public ImmutableListMultimap<ObjectId, Comment> getComments() {
+ public ImmutableListMultimap<ObjectId, HumanComment> getComments() {
return comments;
}
- public boolean containsComment(Comment c) {
- for (Comment existing : comments.values()) {
+ public boolean containsComment(HumanComment c) {
+ for (HumanComment existing : comments.values()) {
if (c.key.equals(existing.key)) {
return true;
}
@@ -120,10 +120,13 @@
ObjectReader reader = handle.walk().getObjectReader();
revisionNoteMap =
RevisionNoteMap.parse(
- args.changeNoteJson, reader, NoteMap.read(reader, tipCommit), Comment.Status.DRAFT);
- ListMultimap<ObjectId, Comment> cs = MultimapBuilder.hashKeys().arrayListValues().build();
+ args.changeNoteJson,
+ reader,
+ NoteMap.read(reader, tipCommit),
+ HumanComment.Status.DRAFT);
+ ListMultimap<ObjectId, HumanComment> cs = MultimapBuilder.hashKeys().arrayListValues().build();
for (ChangeRevisionNote rn : revisionNoteMap.revisionNotes.values()) {
- for (Comment c : rn.getEntities()) {
+ for (HumanComment c : rn.getEntities()) {
cs.put(c.getCommitId(), c);
}
}
diff --git a/java/com/google/gerrit/server/notedb/HumanCommentsRevisionNoteData.java b/java/com/google/gerrit/server/notedb/HumanCommentsRevisionNoteData.java
new file mode 100644
index 0000000..e570412
--- /dev/null
+++ b/java/com/google/gerrit/server/notedb/HumanCommentsRevisionNoteData.java
@@ -0,0 +1,28 @@
+// Copyright (C) 2020 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.notedb;
+
+import com.google.gerrit.entities.HumanComment;
+import java.util.List;
+
+/**
+ * Holds the raw data of a RevisionNote.
+ *
+ * <p>It is intended for deserialization from JSON only. It is used for human comments only.
+ */
+class HumanCommentsRevisionNoteData {
+ String pushCert;
+ List<HumanComment> comments;
+}
diff --git a/java/com/google/gerrit/server/notedb/RevisionNoteData.java b/java/com/google/gerrit/server/notedb/RevisionNoteData.java
index c0e09ed..da15b34 100644
--- a/java/com/google/gerrit/server/notedb/RevisionNoteData.java
+++ b/java/com/google/gerrit/server/notedb/RevisionNoteData.java
@@ -20,7 +20,8 @@
/**
* Holds the raw data of a RevisionNote.
*
- * <p>It is intended for (de)serialization to JSON only.
+ * <p>It is intended for serialization to JSON only. It is used for human comments and robot
+ * comments.
*/
class RevisionNoteData {
String pushCert;
diff --git a/java/com/google/gerrit/server/notedb/RevisionNoteMap.java b/java/com/google/gerrit/server/notedb/RevisionNoteMap.java
index 98c9873..5a0b67b 100644
--- a/java/com/google/gerrit/server/notedb/RevisionNoteMap.java
+++ b/java/com/google/gerrit/server/notedb/RevisionNoteMap.java
@@ -16,6 +16,7 @@
import com.google.common.collect.ImmutableMap;
import com.google.gerrit.entities.Comment;
+import com.google.gerrit.entities.HumanComment;
import java.io.IOException;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.ObjectId;
@@ -41,7 +42,7 @@
}
static RevisionNoteMap<ChangeRevisionNote> parse(
- ChangeNoteJson noteJson, ObjectReader reader, NoteMap noteMap, Comment.Status status)
+ ChangeNoteJson noteJson, ObjectReader reader, NoteMap noteMap, HumanComment.Status status)
throws ConfigInvalidException, IOException {
ImmutableMap.Builder<ObjectId, ChangeRevisionNote> result = ImmutableMap.builder();
for (Note note : noteMap) {
diff --git a/java/com/google/gerrit/server/notedb/RobotCommentsRevisionNote.java b/java/com/google/gerrit/server/notedb/RobotCommentsRevisionNote.java
index fc4c9fd..010206c 100644
--- a/java/com/google/gerrit/server/notedb/RobotCommentsRevisionNote.java
+++ b/java/com/google/gerrit/server/notedb/RobotCommentsRevisionNote.java
@@ -26,7 +26,11 @@
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.ObjectReader;
-/** Like {@link RevisionNote} but for robot comments. */
+/**
+ * Holds the raw data of a RevisionNote.
+ *
+ * <p>It is intended for deserialization from JSON only. It is used for robot comments only.
+ */
public class RobotCommentsRevisionNote extends RevisionNote<RobotComment> {
private final ChangeNoteJson noteUtil;
diff --git a/java/com/google/gerrit/server/patch/PatchScriptFactory.java b/java/com/google/gerrit/server/patch/PatchScriptFactory.java
index 29a89d6..30930ec 100644
--- a/java/com/google/gerrit/server/patch/PatchScriptFactory.java
+++ b/java/com/google/gerrit/server/patch/PatchScriptFactory.java
@@ -24,7 +24,7 @@
import com.google.gerrit.common.data.PatchScript;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.Change;
-import com.google.gerrit.entities.Comment;
+import com.google.gerrit.entities.HumanComment;
import com.google.gerrit.entities.Patch.ChangeType;
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.entities.Project;
@@ -381,13 +381,13 @@
}
private void loadPublished(String file) {
- for (Comment c : commentsUtil.publishedByChangeFile(notes, file)) {
+ for (HumanComment c : commentsUtil.publishedByChangeFile(notes, file)) {
comments.include(notes.getChangeId(), c);
}
}
private void loadDrafts(Account.Id me, String file) {
- for (Comment c : commentsUtil.draftByChangeFileAuthor(notes, file, me)) {
+ for (HumanComment c : commentsUtil.draftByChangeFileAuthor(notes, file, me)) {
comments.include(notes.getChangeId(), c);
}
}
diff --git a/java/com/google/gerrit/server/query/change/ChangeData.java b/java/com/google/gerrit/server/query/change/ChangeData.java
index 01d2c31..34c96dd 100644
--- a/java/com/google/gerrit/server/query/change/ChangeData.java
+++ b/java/com/google/gerrit/server/query/change/ChangeData.java
@@ -41,6 +41,7 @@
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.ChangeMessage;
import com.google.gerrit.entities.Comment;
+import com.google.gerrit.entities.HumanComment;
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.entities.PatchSetApproval;
import com.google.gerrit.entities.Project;
@@ -277,7 +278,7 @@
private List<PatchSetApproval> currentApprovals;
private List<String> currentFiles;
private Optional<DiffSummary> diffSummary;
- private Collection<Comment> publishedComments;
+ private Collection<HumanComment> publishedComments;
private Collection<RobotComment> robotComments;
private CurrentUser visibleTo;
private List<ChangeMessage> messages;
@@ -760,12 +761,12 @@
return reviewerUpdates;
}
- public Collection<Comment> publishedComments() {
+ public Collection<HumanComment> publishedComments() {
if (publishedComments == null) {
if (!lazyLoad) {
return Collections.emptyList();
}
- publishedComments = commentsUtil.publishedByChange(notes());
+ publishedComments = commentsUtil.publishedHumanCommentsByChange(notes());
}
return publishedComments;
}
diff --git a/java/com/google/gerrit/server/query/change/CommentByPredicate.java b/java/com/google/gerrit/server/query/change/CommentByPredicate.java
index bd7981c..b8cf100 100644
--- a/java/com/google/gerrit/server/query/change/CommentByPredicate.java
+++ b/java/com/google/gerrit/server/query/change/CommentByPredicate.java
@@ -16,7 +16,7 @@
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.ChangeMessage;
-import com.google.gerrit.entities.Comment;
+import com.google.gerrit.entities.HumanComment;
import com.google.gerrit.server.index.change.ChangeField;
import java.util.Objects;
@@ -39,7 +39,7 @@
return true;
}
}
- for (Comment c : cd.publishedComments()) {
+ for (HumanComment c : cd.publishedComments()) {
if (Objects.equals(c.author.getId(), id)) {
return true;
}
diff --git a/java/com/google/gerrit/server/restapi/account/DeleteDraftComments.java b/java/com/google/gerrit/server/restapi/account/DeleteDraftComments.java
index 4b505c6..f29c6e6 100644
--- a/java/com/google/gerrit/server/restapi/account/DeleteDraftComments.java
+++ b/java/com/google/gerrit/server/restapi/account/DeleteDraftComments.java
@@ -22,7 +22,7 @@
import com.google.common.collect.ImmutableList;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.Account;
-import com.google.gerrit.entities.Comment;
+import com.google.gerrit.entities.HumanComment;
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.entities.Project;
import com.google.gerrit.extensions.api.accounts.DeleteDraftCommentsInput;
@@ -48,7 +48,7 @@
import com.google.gerrit.server.query.change.HasDraftByPredicate;
import com.google.gerrit.server.query.change.InternalChangeQuery;
import com.google.gerrit.server.restapi.change.CommentJson;
-import com.google.gerrit.server.restapi.change.CommentJson.CommentFormatter;
+import com.google.gerrit.server.restapi.change.CommentJson.HumanCommentFormatter;
import com.google.gerrit.server.update.BatchUpdate;
import com.google.gerrit.server.update.BatchUpdate.Factory;
import com.google.gerrit.server.update.BatchUpdateListener;
@@ -123,7 +123,8 @@
throw new AuthException("Cannot delete drafts of other user");
}
- CommentFormatter commentFormatter = commentJsonProvider.get().newCommentFormatter();
+ HumanCommentFormatter humanCommentFormatter =
+ commentJsonProvider.get().newHumanCommentFormatter();
Account.Id accountId = rsrc.getUser().getAccountId();
Timestamp now = TimeUtil.nowTs();
Map<Project.NameKey, BatchUpdate> updates = new LinkedHashMap<>();
@@ -137,7 +138,7 @@
BatchUpdate update =
updates.computeIfAbsent(
cd.project(), p -> batchUpdateFactory.create(p, rsrc.getUser(), now));
- Op op = new Op(commentFormatter, accountId);
+ Op op = new Op(humanCommentFormatter, accountId);
update.addOp(cd.getId(), op);
ops.add(op);
}
@@ -165,12 +166,12 @@
}
private class Op implements BatchUpdateOp {
- private final CommentFormatter commentFormatter;
+ private final HumanCommentFormatter humanCommentFormatter;
private final Account.Id accountId;
private DeletedDraftCommentInfo result;
- Op(CommentFormatter commentFormatter, Account.Id accountId) {
- this.commentFormatter = commentFormatter;
+ Op(HumanCommentFormatter humanCommentFormatter, Account.Id accountId) {
+ this.humanCommentFormatter = humanCommentFormatter;
this.accountId = accountId;
}
@@ -179,12 +180,12 @@
throws PatchListNotAvailableException, PermissionBackendException {
ImmutableList.Builder<CommentInfo> comments = ImmutableList.builder();
boolean dirty = false;
- for (Comment c : commentsUtil.draftByChangeAuthor(ctx.getNotes(), accountId)) {
+ for (HumanComment c : commentsUtil.draftByChangeAuthor(ctx.getNotes(), accountId)) {
dirty = true;
PatchSet.Id psId = PatchSet.id(ctx.getChange().getId(), c.key.patchSetId);
setCommentCommitId(c, patchListCache, ctx.getChange(), psUtil.get(ctx.getNotes(), psId));
- commentsUtil.deleteComments(ctx.getUpdate(psId), Collections.singleton(c));
- comments.add(commentFormatter.format(c));
+ commentsUtil.deleteHumanComments(ctx.getUpdate(psId), Collections.singleton(c));
+ comments.add(humanCommentFormatter.format(c));
}
if (dirty) {
result = new DeletedDraftCommentInfo();
diff --git a/java/com/google/gerrit/server/restapi/change/AddToAttentionSet.java b/java/com/google/gerrit/server/restapi/change/AddToAttentionSet.java
index 5176fe9..a3d6388 100644
--- a/java/com/google/gerrit/server/restapi/change/AddToAttentionSet.java
+++ b/java/com/google/gerrit/server/restapi/change/AddToAttentionSet.java
@@ -16,7 +16,7 @@
import com.google.common.base.Strings;
import com.google.gerrit.entities.Account;
-import com.google.gerrit.extensions.api.changes.AddToAttentionSetInput;
+import com.google.gerrit.extensions.api.changes.AttentionSetInput;
import com.google.gerrit.extensions.common.AccountInfo;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
@@ -38,7 +38,7 @@
@Singleton
public class AddToAttentionSet
implements RestCollectionModifyView<
- ChangeResource, AttentionSetEntryResource, AddToAttentionSetInput> {
+ ChangeResource, AttentionSetEntryResource, AttentionSetInput> {
private final BatchUpdate.Factory updateFactory;
private final AccountResolver accountResolver;
private final AddToAttentionSetOp.Factory opFactory;
@@ -60,7 +60,7 @@
}
@Override
- public Response<AccountInfo> apply(ChangeResource changeResource, AddToAttentionSetInput input)
+ public Response<AccountInfo> apply(ChangeResource changeResource, AttentionSetInput input)
throws Exception {
input.user = Strings.nullToEmpty(input.user).trim();
if (input.user.isEmpty()) {
@@ -84,7 +84,7 @@
try (BatchUpdate bu =
updateFactory.create(
changeResource.getChange().getProject(), changeResource.getUser(), TimeUtil.nowTs())) {
- AddToAttentionSetOp op = opFactory.create(attentionUserId, input.reason);
+ AddToAttentionSetOp op = opFactory.create(attentionUserId, input.reason, true);
bu.addOp(changeResource.getId(), op);
bu.execute();
return Response.ok(accountLoaderFactory.create(true).fillOne(attentionUserId));
diff --git a/java/com/google/gerrit/server/restapi/change/CommentJson.java b/java/com/google/gerrit/server/restapi/change/CommentJson.java
index 03898b1..4bb6327 100644
--- a/java/com/google/gerrit/server/restapi/change/CommentJson.java
+++ b/java/com/google/gerrit/server/restapi/change/CommentJson.java
@@ -25,6 +25,7 @@
import com.google.gerrit.entities.Comment;
import com.google.gerrit.entities.FixReplacement;
import com.google.gerrit.entities.FixSuggestion;
+import com.google.gerrit.entities.HumanComment;
import com.google.gerrit.entities.RobotComment;
import com.google.gerrit.extensions.client.Comment.Range;
import com.google.gerrit.extensions.client.Side;
@@ -63,8 +64,8 @@
return this;
}
- public CommentFormatter newCommentFormatter() {
- return new CommentFormatter();
+ public HumanCommentFormatter newHumanCommentFormatter() {
+ return new HumanCommentFormatter();
}
public RobotCommentFormatter newRobotCommentFormatter() {
@@ -161,15 +162,15 @@
}
}
- public class CommentFormatter extends BaseCommentFormatter<Comment, CommentInfo> {
+ public class HumanCommentFormatter extends BaseCommentFormatter<HumanComment, CommentInfo> {
@Override
- protected CommentInfo toInfo(Comment c, AccountLoader loader) {
+ protected CommentInfo toInfo(HumanComment c, AccountLoader loader) {
CommentInfo ci = new CommentInfo();
fillCommentInfo(c, ci, loader);
return ci;
}
- private CommentFormatter() {}
+ private HumanCommentFormatter() {}
}
class RobotCommentFormatter extends BaseCommentFormatter<RobotComment, RobotCommentInfo> {
diff --git a/java/com/google/gerrit/server/restapi/change/Comments.java b/java/com/google/gerrit/server/restapi/change/Comments.java
index 078c239..00566f3 100644
--- a/java/com/google/gerrit/server/restapi/change/Comments.java
+++ b/java/com/google/gerrit/server/restapi/change/Comments.java
@@ -14,28 +14,28 @@
package com.google.gerrit.server.restapi.change;
-import com.google.gerrit.entities.Comment;
+import com.google.gerrit.entities.HumanComment;
import com.google.gerrit.extensions.registration.DynamicMap;
import com.google.gerrit.extensions.restapi.ChildCollection;
import com.google.gerrit.extensions.restapi.IdString;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.RestView;
import com.google.gerrit.server.CommentsUtil;
-import com.google.gerrit.server.change.CommentResource;
+import com.google.gerrit.server.change.HumanCommentResource;
import com.google.gerrit.server.change.RevisionResource;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.inject.Inject;
import com.google.inject.Singleton;
@Singleton
-public class Comments implements ChildCollection<RevisionResource, CommentResource> {
- private final DynamicMap<RestView<CommentResource>> views;
+public class Comments implements ChildCollection<RevisionResource, HumanCommentResource> {
+ private final DynamicMap<RestView<HumanCommentResource>> views;
private final ListRevisionComments list;
private final CommentsUtil commentsUtil;
@Inject
Comments(
- DynamicMap<RestView<CommentResource>> views,
+ DynamicMap<RestView<HumanCommentResource>> views,
ListRevisionComments list,
CommentsUtil commentsUtil) {
this.views = views;
@@ -44,7 +44,7 @@
}
@Override
- public DynamicMap<RestView<CommentResource>> views() {
+ public DynamicMap<RestView<HumanCommentResource>> views() {
return views;
}
@@ -54,13 +54,14 @@
}
@Override
- public CommentResource parse(RevisionResource rev, IdString id) throws ResourceNotFoundException {
+ public HumanCommentResource parse(RevisionResource rev, IdString id)
+ throws ResourceNotFoundException {
String uuid = id.get();
ChangeNotes notes = rev.getNotes();
- for (Comment c : commentsUtil.publishedByPatchSet(notes, rev.getPatchSet().id())) {
+ for (HumanComment c : commentsUtil.publishedByPatchSet(notes, rev.getPatchSet().id())) {
if (uuid.equals(c.key.uuid)) {
- return new CommentResource(rev, c);
+ return new HumanCommentResource(rev, c);
}
}
throw new ResourceNotFoundException(id);
diff --git a/java/com/google/gerrit/server/restapi/change/CreateDraftComment.java b/java/com/google/gerrit/server/restapi/change/CreateDraftComment.java
index 42032f7..d99d7014 100644
--- a/java/com/google/gerrit/server/restapi/change/CreateDraftComment.java
+++ b/java/com/google/gerrit/server/restapi/change/CreateDraftComment.java
@@ -18,7 +18,7 @@
import static com.google.gerrit.server.CommentsUtil.setCommentCommitId;
import com.google.common.base.Strings;
-import com.google.gerrit.entities.Comment;
+import com.google.gerrit.entities.HumanComment;
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.extensions.api.changes.DraftInput;
import com.google.gerrit.extensions.common.CommentInfo;
@@ -89,7 +89,7 @@
bu.addOp(rsrc.getChange().getId(), op);
bu.execute();
return Response.created(
- commentJson.get().setFillAccounts(false).newCommentFormatter().format(op.comment));
+ commentJson.get().setFillAccounts(false).newHumanCommentFormatter().format(op.comment));
}
}
@@ -97,7 +97,7 @@
private final PatchSet.Id psId;
private final DraftInput in;
- private Comment comment;
+ private HumanComment comment;
private Op(PatchSet.Id psId, DraftInput in) {
this.psId = psId;
@@ -115,15 +115,15 @@
String parentUuid = Url.decode(in.inReplyTo);
comment =
- commentsUtil.newComment(
+ commentsUtil.newHumanComment(
ctx, in.path, ps.id(), in.side(), in.message.trim(), in.unresolved, parentUuid);
comment.setLineNbrAndRange(in.line, in.range);
comment.tag = in.tag;
setCommentCommitId(comment, patchListCache, ctx.getChange(), ps);
- commentsUtil.putComments(
- ctx.getUpdate(psId), Comment.Status.DRAFT, Collections.singleton(comment));
+ commentsUtil.putHumanComments(
+ ctx.getUpdate(psId), HumanComment.Status.DRAFT, Collections.singleton(comment));
return true;
}
}
diff --git a/java/com/google/gerrit/server/restapi/change/DeleteComment.java b/java/com/google/gerrit/server/restapi/change/DeleteComment.java
index f915728..8580229 100644
--- a/java/com/google/gerrit/server/restapi/change/DeleteComment.java
+++ b/java/com/google/gerrit/server/restapi/change/DeleteComment.java
@@ -15,7 +15,7 @@
package com.google.gerrit.server.restapi.change;
import com.google.common.base.Strings;
-import com.google.gerrit.entities.Comment;
+import com.google.gerrit.entities.HumanComment;
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.extensions.api.changes.DeleteCommentInput;
import com.google.gerrit.extensions.common.CommentInfo;
@@ -26,7 +26,7 @@
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.server.CommentsUtil;
import com.google.gerrit.server.CurrentUser;
-import com.google.gerrit.server.change.CommentResource;
+import com.google.gerrit.server.change.HumanCommentResource;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.permissions.GlobalPermission;
import com.google.gerrit.server.permissions.PermissionBackend;
@@ -45,7 +45,7 @@
import org.eclipse.jgit.errors.ConfigInvalidException;
@Singleton
-public class DeleteComment implements RestModifyView<CommentResource, DeleteCommentInput> {
+public class DeleteComment implements RestModifyView<HumanCommentResource, DeleteCommentInput> {
private final Provider<CurrentUser> userProvider;
private final PermissionBackend permissionBackend;
@@ -71,7 +71,7 @@
}
@Override
- public Response<CommentInfo> apply(CommentResource rsrc, DeleteCommentInput input)
+ public Response<CommentInfo> apply(HumanCommentResource rsrc, DeleteCommentInput input)
throws RestApiException, IOException, ConfigInvalidException, PermissionBackendException,
UpdateException {
CurrentUser user = userProvider.get();
@@ -90,15 +90,15 @@
ChangeNotes updatedNotes =
notesFactory.createChecked(rsrc.getRevisionResource().getChange().getId());
- List<Comment> changeComments = commentsUtil.publishedByChange(updatedNotes);
- Optional<Comment> updatedComment =
+ List<HumanComment> changeComments = commentsUtil.publishedHumanCommentsByChange(updatedNotes);
+ Optional<HumanComment> updatedComment =
changeComments.stream().filter(c -> c.key.equals(rsrc.getComment().key)).findFirst();
if (!updatedComment.isPresent()) {
// This should not happen as this endpoint should not remove the whole comment.
throw new ResourceNotFoundException("comment not found: " + rsrc.getComment().key);
}
- return Response.ok(commentJson.get().newCommentFormatter().format(updatedComment.get()));
+ return Response.ok(commentJson.get().newHumanCommentFormatter().format(updatedComment.get()));
}
private static String getCommentNewMessage(String name, String reason) {
@@ -110,10 +110,10 @@
}
private class DeleteCommentOp implements BatchUpdateOp {
- private final CommentResource rsrc;
+ private final HumanCommentResource rsrc;
private final String newMessage;
- DeleteCommentOp(CommentResource rsrc, String newMessage) {
+ DeleteCommentOp(HumanCommentResource rsrc, String newMessage) {
this.rsrc = rsrc;
this.newMessage = newMessage;
}
diff --git a/java/com/google/gerrit/server/restapi/change/DeleteDraftComment.java b/java/com/google/gerrit/server/restapi/change/DeleteDraftComment.java
index 89fc3b7..71fd4d2 100644
--- a/java/com/google/gerrit/server/restapi/change/DeleteDraftComment.java
+++ b/java/com/google/gerrit/server/restapi/change/DeleteDraftComment.java
@@ -17,6 +17,7 @@
import static com.google.gerrit.server.CommentsUtil.setCommentCommitId;
import com.google.gerrit.entities.Comment;
+import com.google.gerrit.entities.HumanComment;
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.extensions.common.CommentInfo;
import com.google.gerrit.extensions.common.Input;
@@ -80,7 +81,7 @@
@Override
public boolean updateChange(ChangeContext ctx)
throws ResourceNotFoundException, PatchListNotAvailableException {
- Optional<Comment> maybeComment =
+ Optional<HumanComment> maybeComment =
commentsUtil.getDraft(ctx.getNotes(), ctx.getIdentifiedUser(), key);
if (!maybeComment.isPresent()) {
return false; // Nothing to do.
@@ -90,9 +91,9 @@
if (ps == null) {
throw new ResourceNotFoundException("patch set not found: " + psId);
}
- Comment c = maybeComment.get();
+ HumanComment c = maybeComment.get();
setCommentCommitId(c, patchListCache, ctx.getChange(), ps);
- commentsUtil.deleteComments(ctx.getUpdate(psId), Collections.singleton(c));
+ commentsUtil.deleteHumanComments(ctx.getUpdate(psId), Collections.singleton(c));
return true;
}
}
diff --git a/java/com/google/gerrit/server/restapi/change/DraftComments.java b/java/com/google/gerrit/server/restapi/change/DraftComments.java
index bab1ac9..ab5b9f4 100644
--- a/java/com/google/gerrit/server/restapi/change/DraftComments.java
+++ b/java/com/google/gerrit/server/restapi/change/DraftComments.java
@@ -14,7 +14,7 @@
package com.google.gerrit.server.restapi.change;
-import com.google.gerrit.entities.Comment;
+import com.google.gerrit.entities.HumanComment;
import com.google.gerrit.extensions.registration.DynamicMap;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.ChildCollection;
@@ -64,7 +64,7 @@
throws ResourceNotFoundException, AuthException {
checkIdentifiedUser();
String uuid = id.get();
- for (Comment c :
+ for (HumanComment c :
commentsUtil.draftByPatchSetAuthor(
rev.getPatchSet().id(), rev.getAccountId(), rev.getNotes())) {
if (uuid.equals(c.key.uuid)) {
diff --git a/java/com/google/gerrit/server/restapi/change/GetComment.java b/java/com/google/gerrit/server/restapi/change/GetComment.java
index 5103325..24085df 100644
--- a/java/com/google/gerrit/server/restapi/change/GetComment.java
+++ b/java/com/google/gerrit/server/restapi/change/GetComment.java
@@ -17,14 +17,14 @@
import com.google.gerrit.extensions.common.CommentInfo;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestReadView;
-import com.google.gerrit.server.change.CommentResource;
+import com.google.gerrit.server.change.HumanCommentResource;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
@Singleton
-public class GetComment implements RestReadView<CommentResource> {
+public class GetComment implements RestReadView<HumanCommentResource> {
private final Provider<CommentJson> commentJson;
@@ -34,7 +34,7 @@
}
@Override
- public Response<CommentInfo> apply(CommentResource rsrc) throws PermissionBackendException {
- return Response.ok(commentJson.get().newCommentFormatter().format(rsrc.getComment()));
+ public Response<CommentInfo> apply(HumanCommentResource rsrc) throws PermissionBackendException {
+ return Response.ok(commentJson.get().newHumanCommentFormatter().format(rsrc.getComment()));
}
}
diff --git a/java/com/google/gerrit/server/restapi/change/GetDraftComment.java b/java/com/google/gerrit/server/restapi/change/GetDraftComment.java
index 797dc9e..ba07b47 100644
--- a/java/com/google/gerrit/server/restapi/change/GetDraftComment.java
+++ b/java/com/google/gerrit/server/restapi/change/GetDraftComment.java
@@ -35,6 +35,6 @@
@Override
public Response<CommentInfo> apply(DraftCommentResource rsrc) throws PermissionBackendException {
- return Response.ok(commentJson.get().newCommentFormatter().format(rsrc.getComment()));
+ return Response.ok(commentJson.get().newHumanCommentFormatter().format(rsrc.getComment()));
}
}
diff --git a/java/com/google/gerrit/server/restapi/change/ListChangeComments.java b/java/com/google/gerrit/server/restapi/change/ListChangeComments.java
index 409b0d5..b842f55 100644
--- a/java/com/google/gerrit/server/restapi/change/ListChangeComments.java
+++ b/java/com/google/gerrit/server/restapi/change/ListChangeComments.java
@@ -18,7 +18,7 @@
import com.google.common.collect.ImmutableList;
import com.google.gerrit.entities.ChangeMessage;
-import com.google.gerrit.entities.Comment;
+import com.google.gerrit.entities.HumanComment;
import com.google.gerrit.extensions.common.CommentInfo;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.Response;
@@ -28,7 +28,6 @@
import com.google.gerrit.server.change.ChangeResource;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.query.change.ChangeData;
-import com.google.gerrit.server.restapi.change.CommentJson.CommentFormatter;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
@@ -64,12 +63,12 @@
return getAsList(listComments(rsrc), rsrc);
}
- private Iterable<Comment> listComments(ChangeResource rsrc) {
+ private Iterable<HumanComment> listComments(ChangeResource rsrc) {
ChangeData cd = changeDataFactory.create(rsrc.getNotes());
- return commentsUtil.publishedByChange(cd.notes());
+ return commentsUtil.publishedHumanCommentsByChange(cd.notes());
}
- private ImmutableList<CommentInfo> getAsList(Iterable<Comment> comments, ChangeResource rsrc)
+ private ImmutableList<CommentInfo> getAsList(Iterable<HumanComment> comments, ChangeResource rsrc)
throws PermissionBackendException {
ImmutableList<CommentInfo> commentInfos = getCommentFormatter().formatAsList(comments);
List<ChangeMessage> changeMessages = changeMessagesUtil.byChange(rsrc.getNotes());
@@ -77,8 +76,8 @@
return commentInfos;
}
- private Map<String, List<CommentInfo>> getAsMap(Iterable<Comment> comments, ChangeResource rsrc)
- throws PermissionBackendException {
+ private Map<String, List<CommentInfo>> getAsMap(
+ Iterable<HumanComment> comments, ChangeResource rsrc) throws PermissionBackendException {
Map<String, List<CommentInfo>> commentInfosMap = getCommentFormatter().format(comments);
List<CommentInfo> commentInfos =
commentInfosMap.values().stream().flatMap(List::stream).collect(toList());
@@ -87,7 +86,7 @@
return commentInfosMap;
}
- private CommentFormatter getCommentFormatter() {
- return commentJson.get().setFillAccounts(true).setFillPatchSet(true).newCommentFormatter();
+ private CommentJson.HumanCommentFormatter getCommentFormatter() {
+ return commentJson.get().setFillAccounts(true).setFillPatchSet(true).newHumanCommentFormatter();
}
}
diff --git a/java/com/google/gerrit/server/restapi/change/ListChangeDrafts.java b/java/com/google/gerrit/server/restapi/change/ListChangeDrafts.java
index 24e1d40..3841dc1 100644
--- a/java/com/google/gerrit/server/restapi/change/ListChangeDrafts.java
+++ b/java/com/google/gerrit/server/restapi/change/ListChangeDrafts.java
@@ -14,7 +14,7 @@
package com.google.gerrit.server.restapi.change;
-import com.google.gerrit.entities.Comment;
+import com.google.gerrit.entities.HumanComment;
import com.google.gerrit.extensions.common.CommentInfo;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.Response;
@@ -23,7 +23,7 @@
import com.google.gerrit.server.change.ChangeResource;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.query.change.ChangeData;
-import com.google.gerrit.server.restapi.change.CommentJson.CommentFormatter;
+import com.google.gerrit.server.restapi.change.CommentJson.HumanCommentFormatter;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
@@ -46,7 +46,7 @@
this.commentsUtil = commentsUtil;
}
- private Iterable<Comment> listComments(ChangeResource rsrc) {
+ private Iterable<HumanComment> listComments(ChangeResource rsrc) {
ChangeData cd = changeDataFactory.create(rsrc.getNotes());
return commentsUtil.draftByChangeAuthor(cd.notes(), rsrc.getUser().getAccountId());
}
@@ -68,7 +68,11 @@
return getCommentFormatter().formatAsList(listComments(rsrc));
}
- private CommentFormatter getCommentFormatter() {
- return commentJson.get().setFillAccounts(false).setFillPatchSet(true).newCommentFormatter();
+ private HumanCommentFormatter getCommentFormatter() {
+ return commentJson
+ .get()
+ .setFillAccounts(false)
+ .setFillPatchSet(true)
+ .newHumanCommentFormatter();
}
}
diff --git a/java/com/google/gerrit/server/restapi/change/ListRevisionComments.java b/java/com/google/gerrit/server/restapi/change/ListRevisionComments.java
index de05d2a..88309ed 100644
--- a/java/com/google/gerrit/server/restapi/change/ListRevisionComments.java
+++ b/java/com/google/gerrit/server/restapi/change/ListRevisionComments.java
@@ -14,7 +14,7 @@
package com.google.gerrit.server.restapi.change;
-import com.google.gerrit.entities.Comment;
+import com.google.gerrit.entities.HumanComment;
import com.google.gerrit.server.CommentsUtil;
import com.google.gerrit.server.change.RevisionResource;
import com.google.gerrit.server.notedb.ChangeNotes;
@@ -35,7 +35,7 @@
}
@Override
- protected Iterable<Comment> listComments(RevisionResource rsrc) {
+ protected Iterable<HumanComment> listComments(RevisionResource rsrc) {
ChangeNotes notes = rsrc.getNotes();
return commentsUtil.publishedByPatchSet(notes, rsrc.getPatchSet().id());
}
diff --git a/java/com/google/gerrit/server/restapi/change/ListRevisionDrafts.java b/java/com/google/gerrit/server/restapi/change/ListRevisionDrafts.java
index 199a752..a5fbd92 100644
--- a/java/com/google/gerrit/server/restapi/change/ListRevisionDrafts.java
+++ b/java/com/google/gerrit/server/restapi/change/ListRevisionDrafts.java
@@ -15,7 +15,7 @@
package com.google.gerrit.server.restapi.change;
import com.google.common.collect.ImmutableList;
-import com.google.gerrit.entities.Comment;
+import com.google.gerrit.entities.HumanComment;
import com.google.gerrit.extensions.common.CommentInfo;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestReadView;
@@ -39,7 +39,7 @@
this.commentsUtil = commentsUtil;
}
- protected Iterable<Comment> listComments(RevisionResource rsrc) {
+ protected Iterable<HumanComment> listComments(RevisionResource rsrc) {
return commentsUtil.draftByPatchSetAuthor(
rsrc.getPatchSet().id(), rsrc.getAccountId(), rsrc.getNotes());
}
@@ -55,7 +55,7 @@
commentJson
.get()
.setFillAccounts(includeAuthorInfo())
- .newCommentFormatter()
+ .newHumanCommentFormatter()
.format(listComments(rsrc)));
}
@@ -64,7 +64,7 @@
return commentJson
.get()
.setFillAccounts(includeAuthorInfo())
- .newCommentFormatter()
+ .newHumanCommentFormatter()
.formatAsList(listComments(rsrc));
}
}
diff --git a/java/com/google/gerrit/server/restapi/change/Module.java b/java/com/google/gerrit/server/restapi/change/Module.java
index 387d0a8..52a8f47 100644
--- a/java/com/google/gerrit/server/restapi/change/Module.java
+++ b/java/com/google/gerrit/server/restapi/change/Module.java
@@ -18,10 +18,10 @@
import static com.google.gerrit.server.change.ChangeEditResource.CHANGE_EDIT_KIND;
import static com.google.gerrit.server.change.ChangeMessageResource.CHANGE_MESSAGE_KIND;
import static com.google.gerrit.server.change.ChangeResource.CHANGE_KIND;
-import static com.google.gerrit.server.change.CommentResource.COMMENT_KIND;
import static com.google.gerrit.server.change.DraftCommentResource.DRAFT_COMMENT_KIND;
import static com.google.gerrit.server.change.FileResource.FILE_KIND;
import static com.google.gerrit.server.change.FixResource.FIX_KIND;
+import static com.google.gerrit.server.change.HumanCommentResource.COMMENT_KIND;
import static com.google.gerrit.server.change.ReviewerResource.REVIEWER_KIND;
import static com.google.gerrit.server.change.RevisionResource.REVISION_KIND;
import static com.google.gerrit.server.change.RobotCommentResource.ROBOT_COMMENT_KIND;
diff --git a/java/com/google/gerrit/server/restapi/change/PostReview.java b/java/com/google/gerrit/server/restapi/change/PostReview.java
index a16d4f9..f57ac9c 100644
--- a/java/com/google/gerrit/server/restapi/change/PostReview.java
+++ b/java/com/google/gerrit/server/restapi/change/PostReview.java
@@ -49,6 +49,7 @@
import com.google.gerrit.entities.Comment;
import com.google.gerrit.entities.FixReplacement;
import com.google.gerrit.entities.FixSuggestion;
+import com.google.gerrit.entities.HumanComment;
import com.google.gerrit.entities.Patch;
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.entities.PatchSetApproval;
@@ -811,8 +812,8 @@
}
/**
- * Used to compare existing {@link Comment}-s with {@link CommentInput} comments by copying only
- * the fields to compare.
+ * Used to compare existing {@link HumanComment}-s with {@link CommentInput} comments by copying
+ * only the fields to compare.
*/
@AutoValue
abstract static class CommentSetEntry {
@@ -942,7 +943,7 @@
// HashMap instead of Collections.emptyMap() avoids warning about remove() on immutable
// object.
- Map<String, Comment> drafts = new HashMap<>();
+ Map<String, HumanComment> drafts = new HashMap<>();
// If there are inputComments we need the deduplication loop below, so we have to read (and
// publish) drafts here.
if (!inputComments.isEmpty() || in.drafts != DraftHandling.KEEP) {
@@ -954,7 +955,7 @@
}
// This will be populated with Comment-s created from inputComments.
- List<Comment> toPublish = new ArrayList<>();
+ List<HumanComment> toPublish = new ArrayList<>();
Set<CommentSetEntry> existingComments =
in.omitDuplicateComments ? readExistingComments(ctx) : Collections.emptySet();
@@ -965,11 +966,11 @@
for (Map.Entry<String, List<CommentInput>> entry : inputComments.entrySet()) {
String path = entry.getKey();
for (CommentInput inputComment : entry.getValue()) {
- Comment comment = drafts.remove(Url.decode(inputComment.id));
+ HumanComment comment = drafts.remove(Url.decode(inputComment.id));
if (comment == null) {
String parent = Url.decode(inputComment.inReplyTo);
comment =
- commentsUtil.newComment(
+ commentsUtil.newHumanComment(
ctx,
path,
psId,
@@ -1014,7 +1015,7 @@
break;
}
ChangeUpdate changeUpdate = ctx.getUpdate(psId);
- commentsUtil.putComments(changeUpdate, Comment.Status.PUBLISHED, toPublish);
+ commentsUtil.putHumanComments(changeUpdate, HumanComment.Status.PUBLISHED, toPublish);
comments.addAll(toPublish);
return !toPublish.isEmpty();
}
@@ -1134,7 +1135,7 @@
}
private Set<CommentSetEntry> readExistingComments(ChangeContext ctx) {
- return commentsUtil.publishedByChange(ctx.getNotes()).stream()
+ return commentsUtil.publishedHumanCommentsByChange(ctx.getNotes()).stream()
.map(CommentSetEntry::create)
.collect(toSet());
}
@@ -1145,7 +1146,7 @@
.collect(toSet());
}
- private Map<String, Comment> changeDrafts(ChangeContext ctx) {
+ private Map<String, HumanComment> changeDrafts(ChangeContext ctx) {
return commentsUtil.draftByChangeAuthor(ctx.getNotes(), user.getAccountId()).stream()
.collect(
Collectors.toMap(
@@ -1156,7 +1157,7 @@
}));
}
- private Map<String, Comment> patchSetDrafts(ChangeContext ctx) {
+ private Map<String, HumanComment> patchSetDrafts(ChangeContext ctx) {
return commentsUtil.draftByPatchSetAuthor(psId, user.getAccountId(), ctx.getNotes()).stream()
.collect(Collectors.toMap(c -> c.key.uuid, c -> c));
}
diff --git a/java/com/google/gerrit/server/restapi/change/PutDraftComment.java b/java/com/google/gerrit/server/restapi/change/PutDraftComment.java
index ea58365..f327f16 100644
--- a/java/com/google/gerrit/server/restapi/change/PutDraftComment.java
+++ b/java/com/google/gerrit/server/restapi/change/PutDraftComment.java
@@ -18,6 +18,7 @@
import static com.google.gerrit.server.CommentsUtil.setCommentCommitId;
import com.google.gerrit.entities.Comment;
+import com.google.gerrit.entities.HumanComment;
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.extensions.api.changes.DraftInput;
import com.google.gerrit.extensions.common.CommentInfo;
@@ -93,7 +94,7 @@
bu.addOp(rsrc.getChange().getId(), op);
bu.execute();
return Response.ok(
- commentJson.get().setFillAccounts(false).newCommentFormatter().format(op.comment));
+ commentJson.get().setFillAccounts(false).newHumanCommentFormatter().format(op.comment));
}
}
@@ -101,7 +102,7 @@
private final Comment.Key key;
private final DraftInput in;
- private Comment comment;
+ private HumanComment comment;
private Op(Comment.Key key, DraftInput in) {
this.key = key;
@@ -111,15 +112,15 @@
@Override
public boolean updateChange(ChangeContext ctx)
throws ResourceNotFoundException, PatchListNotAvailableException {
- Optional<Comment> maybeComment =
+ Optional<HumanComment> maybeComment =
commentsUtil.getDraft(ctx.getNotes(), ctx.getIdentifiedUser(), key);
if (!maybeComment.isPresent()) {
// Disappeared out from under us. Can't easily fall back to insert,
// because the input might be missing required fields. Just give up.
throw new ResourceNotFoundException("comment not found: " + key);
}
- Comment origComment = maybeComment.get();
- comment = new Comment(origComment);
+ HumanComment origComment = maybeComment.get();
+ comment = new HumanComment(origComment);
// Copy constructor preserved old real author; replace with current real
// user.
ctx.getUser().updateRealAccountId(comment::setRealAuthor);
@@ -135,17 +136,19 @@
// Updating the path alters the primary key, which isn't possible.
// Delete then recreate the comment instead of an update.
- commentsUtil.deleteComments(update, Collections.singleton(origComment));
+ commentsUtil.deleteHumanComments(update, Collections.singleton(origComment));
comment.key.filename = in.path;
}
setCommentCommitId(comment, patchListCache, ctx.getChange(), ps);
- commentsUtil.putComments(
- update, Comment.Status.DRAFT, Collections.singleton(update(comment, in, ctx.getWhen())));
+ commentsUtil.putHumanComments(
+ update,
+ HumanComment.Status.DRAFT,
+ Collections.singleton(update(comment, in, ctx.getWhen())));
return true;
}
}
- private static Comment update(Comment e, DraftInput in, Timestamp when) {
+ private static HumanComment update(HumanComment e, DraftInput in, Timestamp when) {
if (in.side != null) {
e.side = in.side();
}
diff --git a/java/com/google/gerrit/server/restapi/change/RemoveFromAttentionSet.java b/java/com/google/gerrit/server/restapi/change/RemoveFromAttentionSet.java
index ccf375a..5d6ec4c 100644
--- a/java/com/google/gerrit/server/restapi/change/RemoveFromAttentionSet.java
+++ b/java/com/google/gerrit/server/restapi/change/RemoveFromAttentionSet.java
@@ -61,7 +61,7 @@
updateFactory.create(
changeResource.getProject(), changeResource.getUser(), TimeUtil.nowTs())) {
RemoveFromAttentionSetOp op =
- opFactory.create(attentionResource.getAccountId(), input.reason);
+ opFactory.create(attentionResource.getAccountId(), input.reason, true);
bu.addOp(changeResource.getId(), op);
bu.execute();
}
diff --git a/java/com/google/gerrit/truth/NullAwareCorrespondence.java b/java/com/google/gerrit/truth/NullAwareCorrespondence.java
new file mode 100644
index 0000000..687ad94
--- /dev/null
+++ b/java/com/google/gerrit/truth/NullAwareCorrespondence.java
@@ -0,0 +1,92 @@
+// Copyright (C) 2020 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// Copyright (C) 2020 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.truth;
+
+import com.google.common.base.Function;
+import com.google.common.truth.Correspondence;
+import java.util.Optional;
+
+/** Utility class for constructing null aware {@link Correspondence}s. */
+public class NullAwareCorrespondence {
+ /**
+ * Constructs a {@link Correspondence} that compares elements by transforming the actual elements
+ * using the given function and testing for equality with the expected elements.
+ *
+ * <p>If the actual element is null, it will correspond to a null expected element. This is
+ * different to {@link Correspondence#transforming(Function, String)} which would invoke the
+ * function with a {@code null} argument, requiring the function being able to handle {@code
+ * null}.
+ *
+ * @param actualTransform a {@link Function} taking an actual value and returning a new value
+ * which will be compared with an expected value to determine whether they correspond
+ * @param description should fill the gap in a failure message of the form {@code "not true that
+ * <some actual element> is an element that <description> <some expected element>"}, e.g.
+ * {@code "has an ID of"}
+ */
+ public static <A, E> Correspondence<A, E> transforming(
+ Function<A, ? extends E> actualTransform, String description) {
+ return Correspondence.transforming(
+ actualValue -> Optional.ofNullable(actualValue).map(actualTransform).orElse(null),
+ description);
+ }
+
+ /**
+ * Constructs a {@link Correspondence} that compares elements by transforming the actual elements
+ * using the given function and testing for equality with the expected elements.
+ *
+ * <p>If the actual element is null, it will correspond to a null expected element. This is
+ * different to {@link Correspondence#transforming(Function, Function, String)} which would invoke
+ * the function with a {@code null} argument, requiring the function being able to handle {@code
+ * null}.
+ *
+ * <p>If the expected element is null, it will correspond to a new null expected element. This is
+ * different to {@link Correspondence#transforming(Function, Function, String)} which would invoke
+ * the function with a {@code null} argument, requiring the function being able to handle {@code
+ * null}.
+ *
+ * @param actualTransform a {@link Function} taking an actual value and returning a new value
+ * which will be compared with an expected value to determine whether they correspond
+ * @param expectedTransform a {@link Function} taking an expected value and returning a new value
+ * which will be compared with a transformed actual value
+ * @param description should fill the gap in a failure message of the form {@code "not true that
+ * <some actual element> is an element that <description> <some expected element>"}, e.g.
+ * {@code "has an ID of"}
+ */
+ public static <A, E> Correspondence<A, E> transforming(
+ Function<A, ? extends E> actualTransform,
+ Function<E, ?> expectedTransform,
+ String description) {
+ return Correspondence.transforming(
+ actualValue -> Optional.ofNullable(actualValue).map(actualTransform).orElse(null),
+ expectedValue -> Optional.ofNullable(expectedValue).map(expectedTransform).orElse(null),
+ description);
+ }
+
+ /**
+ * Private constructor to prevent instantiation of this class.
+ *
+ * <p>This class contains only static method and hence never needs to be instantiated.
+ */
+ private NullAwareCorrespondence() {}
+}
diff --git a/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java b/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java
index f9ba8a2..0cd6182 100644
--- a/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java
@@ -148,6 +148,7 @@
import com.google.gerrit.server.validators.ValidationException;
import com.google.gerrit.testing.ConfigSuite;
import com.google.gerrit.testing.FakeEmailSender.Message;
+import com.google.gerrit.truth.NullAwareCorrespondence;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.jcraft.jsch.KeyPair;
@@ -161,7 +162,6 @@
import java.util.List;
import java.util.Locale;
import java.util.Map;
-import java.util.Objects;
import java.util.Optional;
import java.util.Set;
import java.util.concurrent.atomic.AtomicBoolean;
@@ -2901,12 +2901,7 @@
}
private static Correspondence<GroupInfo, String> getGroupToNameCorrespondence() {
- return Correspondence.from(
- (actualGroup, expectedName) -> {
- String groupName = actualGroup == null ? null : actualGroup.name;
- return Objects.equals(groupName, expectedName);
- },
- "has name");
+ return NullAwareCorrespondence.transforming(groupInfo -> groupInfo.name, "has name");
}
private void assertSequenceNumbers(List<SshKeyInfo> sshKeys) {
diff --git a/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java b/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java
index dcf2afd..8bc9cd1 100644
--- a/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java
@@ -100,6 +100,7 @@
import com.google.gerrit.server.util.MagicBranch;
import com.google.gerrit.server.util.time.TimeUtil;
import com.google.gerrit.testing.GerritJUnit.ThrowingRunnable;
+import com.google.gerrit.truth.NullAwareCorrespondence;
import com.google.inject.AbstractModule;
import com.google.inject.Inject;
import com.google.inject.Module;
@@ -111,7 +112,6 @@
import java.util.Collection;
import java.util.List;
import java.util.Map;
-import java.util.Objects;
import java.util.stream.Stream;
import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
import org.eclipse.jgit.junit.TestRepository;
@@ -1517,12 +1517,8 @@
}
private static Correspondence<AccountInfo, String> getAccountToUsernameCorrespondence() {
- return Correspondence.from(
- (actualAccount, expectedName) -> {
- String username = actualAccount == null ? null : actualAccount.username;
- return Objects.equals(username, expectedName);
- },
- "has username");
+ return NullAwareCorrespondence.transforming(
+ accountInfo -> accountInfo.username, "has username");
}
private void assertStaleGroupAndReindex(AccountGroup.UUID groupUuid) throws IOException {
diff --git a/javatests/com/google/gerrit/acceptance/rest/account/ImpersonationIT.java b/javatests/com/google/gerrit/acceptance/rest/account/ImpersonationIT.java
index a3c0295..840853c 100644
--- a/javatests/com/google/gerrit/acceptance/rest/account/ImpersonationIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/account/ImpersonationIT.java
@@ -41,7 +41,7 @@
import com.google.gerrit.common.data.Permission;
import com.google.gerrit.entities.AccountGroup;
import com.google.gerrit.entities.ChangeMessage;
-import com.google.gerrit.entities.Comment;
+import com.google.gerrit.entities.HumanComment;
import com.google.gerrit.entities.Patch;
import com.google.gerrit.entities.PatchSetApproval;
import com.google.gerrit.entities.RobotComment;
@@ -225,7 +225,8 @@
assertThat(psa.realAccountId()).isEqualTo(admin.id());
ChangeData cd = r.getChange();
- Comment c = Iterables.getOnlyElement(commentsUtil.publishedByChange(cd.notes()));
+ HumanComment c =
+ Iterables.getOnlyElement(commentsUtil.publishedHumanCommentsByChange(cd.notes()));
assertThat(c.message).isEqualTo(ci.message);
assertThat(c.author.getId()).isEqualTo(user.id());
assertThat(c.getRealAuthor().getId()).isEqualTo(admin.id());
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/AttentionSetIT.java b/javatests/com/google/gerrit/acceptance/rest/change/AttentionSetIT.java
index caa8832..01ed1b9 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/AttentionSetIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/AttentionSetIT.java
@@ -16,13 +16,21 @@
import static com.google.common.truth.Truth.assertThat;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Iterables;
+import com.google.common.collect.UnmodifiableIterator;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.NoHttpd;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.UseClockStep;
import com.google.gerrit.entities.AttentionSetUpdate;
-import com.google.gerrit.extensions.api.changes.AddToAttentionSetInput;
+import com.google.gerrit.extensions.api.changes.AddReviewerInput;
+import com.google.gerrit.extensions.api.changes.AttentionSetInput;
+import com.google.gerrit.extensions.api.changes.HashtagsInput;
import com.google.gerrit.extensions.api.changes.RemoveFromAttentionSetInput;
+import com.google.gerrit.extensions.api.changes.ReviewInput;
+import com.google.gerrit.extensions.client.ReviewerState;
import com.google.gerrit.server.util.time.TimeUtil;
import java.time.Duration;
import java.time.Instant;
@@ -69,7 +77,7 @@
public void addUser() throws Exception {
PushOneCommit.Result r = createChange();
int accountId =
- change(r).addToAttentionSet(new AddToAttentionSetInput(user.email(), "first"))._accountId;
+ change(r).addToAttentionSet(new AttentionSetInput(user.email(), "first"))._accountId;
assertThat(accountId).isEqualTo(user.id().get());
AttentionSetUpdate expectedAttentionSetUpdate =
AttentionSetUpdate.createFromRead(
@@ -78,7 +86,7 @@
// Second add is ignored.
accountId =
- change(r).addToAttentionSet(new AddToAttentionSetInput(user.email(), "second"))._accountId;
+ change(r).addToAttentionSet(new AttentionSetInput(user.email(), "second"))._accountId;
assertThat(accountId).isEqualTo(user.id().get());
assertThat(r.getChange().attentionSet()).containsExactly(expectedAttentionSetUpdate);
}
@@ -88,13 +96,13 @@
PushOneCommit.Result r = createChange();
Instant timestamp1 = fakeClock.now();
int accountId1 =
- change(r).addToAttentionSet(new AddToAttentionSetInput(user.email(), "user"))._accountId;
+ change(r).addToAttentionSet(new AttentionSetInput(user.email(), "user"))._accountId;
assertThat(accountId1).isEqualTo(user.id().get());
fakeClock.advance(Duration.ofSeconds(42));
Instant timestamp2 = fakeClock.now();
int accountId2 =
change(r)
- .addToAttentionSet(new AddToAttentionSetInput(admin.id().toString(), "admin"))
+ .addToAttentionSet(new AttentionSetInput(admin.id().toString(), "admin"))
._accountId;
assertThat(accountId2).isEqualTo(admin.id().get());
@@ -111,7 +119,7 @@
@Test
public void removeUser() throws Exception {
PushOneCommit.Result r = createChange();
- change(r).addToAttentionSet(new AddToAttentionSetInput(user.email(), "added"));
+ change(r).addToAttentionSet(new AttentionSetInput(user.email(), "added"));
fakeClock.advance(Duration.ofSeconds(42));
change(r).attention(user.id().toString()).remove(new RemoveFromAttentionSetInput("removed"));
AttentionSetUpdate expectedAttentionSetUpdate =
@@ -128,9 +136,272 @@
}
@Test
+ public void changeMessageWhenAddedAndRemovedExplicitly() throws Exception {
+ PushOneCommit.Result r = createChange();
+ change(r).addToAttentionSet(new AttentionSetInput(user.email(), "user"));
+ assertThat(Iterables.getLast(r.getChange().messages()).getMessage())
+ .contains("Added to attention set");
+
+ change(r).attention(user.id().toString()).remove(new RemoveFromAttentionSetInput("foo"));
+ assertThat(Iterables.getLast(r.getChange().messages()).getMessage())
+ .contains("Removed from attention set");
+ }
+
+ @Test
public void removeUnrelatedUser() throws Exception {
PushOneCommit.Result r = createChange();
change(r).attention(user.id().toString()).remove(new RemoveFromAttentionSetInput("foo"));
assertThat(r.getChange().attentionSet()).isEmpty();
}
+
+ @Test
+ public void abandonRemovesUsers() throws Exception {
+ PushOneCommit.Result r = createChange();
+ change(r).addToAttentionSet(new AttentionSetInput(user.email(), "user"));
+ change(r).addToAttentionSet(new AttentionSetInput(admin.email(), "admin"));
+
+ change(r).abandon();
+
+ UnmodifiableIterator<AttentionSetUpdate> attentionSets =
+ r.getChange().attentionSet().iterator();
+ AttentionSetUpdate userUpdate = attentionSets.next();
+ assertThat(userUpdate.account()).isEqualTo(user.id());
+ assertThat(userUpdate.operation()).isEqualTo(AttentionSetUpdate.Operation.REMOVE);
+ assertThat(userUpdate.reason()).isEqualTo("Change was abandoned");
+
+ AttentionSetUpdate adminUpdate = attentionSets.next();
+ assertThat(adminUpdate.account()).isEqualTo(admin.id());
+ assertThat(adminUpdate.operation()).isEqualTo(AttentionSetUpdate.Operation.REMOVE);
+ assertThat(adminUpdate.reason()).isEqualTo("Change was abandoned");
+ }
+
+ @Test
+ public void workInProgressRemovesUsers() throws Exception {
+ PushOneCommit.Result r = createChange();
+ change(r).addToAttentionSet(new AttentionSetInput(user.email(), "reason"));
+
+ change(r).setWorkInProgress();
+
+ AttentionSetUpdate attentionSet = Iterables.getOnlyElement(r.getChange().attentionSet());
+ assertThat(attentionSet.account()).isEqualTo(user.id());
+ assertThat(attentionSet.operation()).isEqualTo(AttentionSetUpdate.Operation.REMOVE);
+ assertThat(attentionSet.reason()).isEqualTo("Change was marked work in progress");
+ }
+
+ @Test
+ public void submitRemovesUsersForAllSubmittedChanges() throws Exception {
+ PushOneCommit.Result r1 = createChange("refs/heads/master", "file1", "content");
+
+ // Approval also automatically adds you as a reviewer and therefore to the attention set.
+ // TODO(paiking): This is actually not what we want to happen on reply, as the replying user
+ // should be removed from the attention set.
+ change(r1).current().review(ReviewInput.approve());
+
+ PushOneCommit.Result r2 = createChange("refs/heads/master", "file2", "content");
+
+ change(r2).current().review(ReviewInput.approve());
+
+ change(r2).current().submit();
+
+ AttentionSetUpdate attentionSet = Iterables.getOnlyElement(r1.getChange().attentionSet());
+ assertThat(attentionSet.account()).isEqualTo(admin.id());
+ assertThat(attentionSet.operation()).isEqualTo(AttentionSetUpdate.Operation.REMOVE);
+ assertThat(attentionSet.reason()).isEqualTo("Change was submitted");
+
+ attentionSet = Iterables.getOnlyElement(r2.getChange().attentionSet());
+ assertThat(attentionSet.account()).isEqualTo(admin.id());
+ assertThat(attentionSet.operation()).isEqualTo(AttentionSetUpdate.Operation.REMOVE);
+ assertThat(attentionSet.reason()).isEqualTo("Change was submitted");
+ }
+
+ @Test
+ public void reviewersAddedAndRemovedFromAttentionSet() throws Exception {
+ PushOneCommit.Result r = createChange();
+
+ change(r).addReviewer(user.id().toString());
+
+ AttentionSetUpdate attentionSet = Iterables.getOnlyElement(r.getChange().attentionSet());
+ assertThat(attentionSet.account()).isEqualTo(user.id());
+ assertThat(attentionSet.operation()).isEqualTo(AttentionSetUpdate.Operation.ADD);
+ assertThat(attentionSet.reason()).isEqualTo("Reviewer was added");
+
+ change(r).reviewer(user.email()).remove();
+
+ attentionSet = Iterables.getOnlyElement(r.getChange().attentionSet());
+ assertThat(attentionSet.account()).isEqualTo(user.id());
+ assertThat(attentionSet.operation()).isEqualTo(AttentionSetUpdate.Operation.REMOVE);
+ assertThat(attentionSet.reason()).isEqualTo("Reviewer was removed");
+ }
+
+ @Test
+ public void noChangeMessagesWhenAddedOrRemovedImplictly() throws Exception {
+ PushOneCommit.Result r = createChange();
+
+ change(r).addReviewer(user.id().toString());
+ change(r).reviewer(user.email()).remove();
+
+ assertThat(
+ r.getChange().messages().stream()
+ .noneMatch(u -> u.getMessage().contains("Added to attention set")))
+ .isTrue();
+ assertThat(
+ r.getChange().messages().stream()
+ .noneMatch(u -> u.getMessage().contains("Removed from attention set")))
+ .isTrue();
+ }
+
+ @Test
+ public void reviewersAddedAndRemovedByEmailFromAttentionSet() throws Exception {
+ PushOneCommit.Result r = createChange();
+
+ change(r).addReviewer(user.email());
+
+ AttentionSetUpdate attentionSet = Iterables.getOnlyElement(r.getChange().attentionSet());
+ assertThat(attentionSet.account()).isEqualTo(user.id());
+ assertThat(attentionSet.operation()).isEqualTo(AttentionSetUpdate.Operation.ADD);
+ assertThat(attentionSet.reason()).isEqualTo("Reviewer was added");
+
+ change(r).reviewer(user.email()).remove();
+
+ attentionSet = Iterables.getOnlyElement(r.getChange().attentionSet());
+ assertThat(attentionSet.account()).isEqualTo(user.id());
+ assertThat(attentionSet.operation()).isEqualTo(AttentionSetUpdate.Operation.REMOVE);
+ assertThat(attentionSet.reason()).isEqualTo("Reviewer was removed");
+ }
+
+ @Test
+ public void reviewersInWorkProgressNotAddedToAttentionSet() throws Exception {
+ PushOneCommit.Result r = createChange();
+ change(r).setWorkInProgress();
+ change(r).addReviewer(user.email());
+
+ assertThat(r.getChange().attentionSet()).isEmpty();
+ }
+
+ @Test
+ public void addingReviewerWhileMarkingWorkInprogressDoesntAddToAttentionSet() throws Exception {
+ PushOneCommit.Result r = createChange();
+ ReviewInput reviewInput = new ReviewInput().setWorkInProgress(true);
+ AddReviewerInput addReviewerInput = new AddReviewerInput();
+ addReviewerInput.state = ReviewerState.REVIEWER;
+ addReviewerInput.reviewer = user.email();
+ reviewInput.reviewers = ImmutableList.of(addReviewerInput);
+
+ change(r).current().review(reviewInput);
+
+ assertThat(r.getChange().attentionSet()).isEmpty();
+ }
+
+ @Test
+ public void reviewersAddedAsReviewersAgainAreNotAddedToAttentionSet() throws Exception {
+ PushOneCommit.Result r = createChange();
+
+ change(r).addReviewer(user.id().toString());
+ change(r)
+ .attention(user.id().toString())
+ .remove(
+ new RemoveFromAttentionSetInput("removed and not re-added when re-adding as reviewer"));
+
+ change(r).addReviewer(user.id().toString());
+
+ AttentionSetUpdate attentionSet = Iterables.getOnlyElement(r.getChange().attentionSet());
+ assertThat(attentionSet.account()).isEqualTo(user.id());
+ assertThat(attentionSet.operation()).isEqualTo(AttentionSetUpdate.Operation.REMOVE);
+ assertThat(attentionSet.reason())
+ .isEqualTo("removed and not re-added when re-adding as reviewer");
+ }
+
+ @Test
+ public void ccsAreIgnored() throws Exception {
+ PushOneCommit.Result r = createChange();
+ AddReviewerInput addReviewerInput = new AddReviewerInput();
+ addReviewerInput.state = ReviewerState.CC;
+ addReviewerInput.reviewer = user.email();
+
+ change(r).addReviewer(addReviewerInput);
+
+ assertThat(r.getChange().attentionSet()).isEmpty();
+ }
+
+ @Test
+ public void ccsConsideredSameAsRemovedForExistingReviewers() throws Exception {
+ PushOneCommit.Result r = createChange();
+ change(r).addReviewer(user.email());
+
+ AddReviewerInput addReviewerInput = new AddReviewerInput();
+ addReviewerInput.state = ReviewerState.CC;
+ addReviewerInput.reviewer = user.email();
+ change(r).addReviewer(addReviewerInput);
+
+ AttentionSetUpdate attentionSet = Iterables.getOnlyElement(r.getChange().attentionSet());
+ assertThat(attentionSet.account()).isEqualTo(user.id());
+ assertThat(attentionSet.operation()).isEqualTo(AttentionSetUpdate.Operation.REMOVE);
+ assertThat(attentionSet.reason()).isEqualTo("Reviewer was removed");
+ }
+
+ @Test
+ public void readyForReviewAddsAllReviewersToAttentionSet() throws Exception {
+ PushOneCommit.Result r = createChange();
+ change(r).setWorkInProgress();
+ change(r).addReviewer(user.email());
+
+ change(r).setReadyForReview();
+ AttentionSetUpdate attentionSet = Iterables.getOnlyElement(r.getChange().attentionSet());
+ assertThat(attentionSet.account()).isEqualTo(user.id());
+ assertThat(attentionSet.operation()).isEqualTo(AttentionSetUpdate.Operation.ADD);
+ assertThat(attentionSet.reason()).isEqualTo("Change was marked ready for review");
+ }
+
+ @Test
+ public void readyForReviewWhileRemovingReviewerDoesntAddThemToAttentionSet() throws Exception {
+ PushOneCommit.Result r = createChange();
+ change(r).setWorkInProgress();
+ change(r).addReviewer(user.email());
+
+ ReviewInput reviewInput = new ReviewInput().setReady(true);
+ AddReviewerInput addReviewerInput = new AddReviewerInput();
+ addReviewerInput.state = ReviewerState.CC;
+ addReviewerInput.reviewer = user.email();
+ reviewInput.reviewers = ImmutableList.of(addReviewerInput);
+ change(r).current().review(reviewInput);
+
+ AttentionSetUpdate attentionSet = Iterables.getOnlyElement(r.getChange().attentionSet());
+ assertThat(attentionSet.account()).isEqualTo(user.id());
+ assertThat(attentionSet.operation()).isEqualTo(AttentionSetUpdate.Operation.REMOVE);
+ assertThat(attentionSet.reason()).isEqualTo("Reviewer was removed");
+ }
+
+ @Test
+ public void readyForReviewWhileAddingReviewerAddsThemToAttentionSet() throws Exception {
+ PushOneCommit.Result r = createChange();
+ change(r).setWorkInProgress();
+
+ ReviewInput reviewInput = new ReviewInput().setReady(true);
+ AddReviewerInput addReviewerInput = new AddReviewerInput();
+ addReviewerInput.state = ReviewerState.REVIEWER;
+ addReviewerInput.reviewer = user.email();
+ reviewInput.reviewers = ImmutableList.of(addReviewerInput);
+ change(r).current().review(reviewInput);
+
+ AttentionSetUpdate attentionSet = Iterables.getOnlyElement(r.getChange().attentionSet());
+ assertThat(attentionSet.account()).isEqualTo(user.id());
+ assertThat(attentionSet.operation()).isEqualTo(AttentionSetUpdate.Operation.ADD);
+ assertThat(attentionSet.reason()).isEqualTo("Reviewer was added");
+ }
+
+ @Test
+ public void reviewersAreNotAddedForNoReasonBecauseOfAnUpdate() throws Exception {
+ PushOneCommit.Result r = createChange();
+ change(r).addToAttentionSet(new AttentionSetInput(user.email(), "user"));
+ change(r).attention(user.id().toString()).remove(new RemoveFromAttentionSetInput("removed"));
+
+ HashtagsInput hashtagsInput = new HashtagsInput();
+ hashtagsInput.add = ImmutableSet.of("tag");
+ change(r).setHashtags(hashtagsInput);
+
+ AttentionSetUpdate attentionSet = Iterables.getOnlyElement(r.getChange().attentionSet());
+ assertThat(attentionSet.account()).isEqualTo(user.id());
+ assertThat(attentionSet.operation()).isEqualTo(AttentionSetUpdate.Operation.REMOVE);
+ assertThat(attentionSet.reason()).isEqualTo("removed");
+ }
}
diff --git a/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java b/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java
index e48c9d5..15f1a6a 100644
--- a/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java
@@ -34,6 +34,7 @@
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
import com.google.gerrit.entities.Change;
+import com.google.gerrit.entities.HumanComment;
import com.google.gerrit.entities.Patch;
import com.google.gerrit.entities.RefNames;
import com.google.gerrit.extensions.api.changes.DeleteCommentInput;
@@ -1425,16 +1426,16 @@
RevCommit commitBefore = beforeDelete.get(i);
RevCommit commitAfter = afterDelete.get(i);
- Map<String, com.google.gerrit.entities.Comment> commentMapBefore =
+ Map<String, HumanComment> commentMapBefore =
DeleteCommentRewriter.getPublishedComments(
noteUtil, reader, NoteMap.read(reader, commitBefore));
- Map<String, com.google.gerrit.entities.Comment> commentMapAfter =
+ Map<String, HumanComment> commentMapAfter =
DeleteCommentRewriter.getPublishedComments(
noteUtil, reader, NoteMap.read(reader, commitAfter));
if (commentMapBefore.containsKey(targetCommentUuid)) {
assertThat(commentMapAfter).containsKey(targetCommentUuid);
- com.google.gerrit.entities.Comment comment = commentMapAfter.get(targetCommentUuid);
+ HumanComment comment = commentMapAfter.get(targetCommentUuid);
assertThat(comment.message).isEqualTo(expectedMessage);
comment.message = commentMapBefore.get(targetCommentUuid).message;
commentMapAfter.put(targetCommentUuid, comment);
diff --git a/javatests/com/google/gerrit/acceptance/server/change/GetRelatedIT.java b/javatests/com/google/gerrit/acceptance/server/change/GetRelatedIT.java
index b544f6e..74dfa04 100644
--- a/javatests/com/google/gerrit/acceptance/server/change/GetRelatedIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/change/GetRelatedIT.java
@@ -58,7 +58,6 @@
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
-import java.util.Objects;
import java.util.Optional;
import org.eclipse.jgit.junit.TestRepository;
import org.eclipse.jgit.lib.Config;
@@ -636,10 +635,8 @@
private static Correspondence<RelatedChangeAndCommitInfo, String>
getRelatedChangeToStatusCorrespondence() {
- return Correspondence.from(
- (relatedChangeAndCommitInfo, status) ->
- Objects.equals(relatedChangeAndCommitInfo.status, status),
- "has status");
+ return Correspondence.transforming(
+ relatedChangeAndCommitInfo -> relatedChangeAndCommitInfo.status, "has status");
}
private RevCommit parseBody(RevCommit c) throws Exception {
diff --git a/javatests/com/google/gerrit/acceptance/testsuite/group/GroupOperationsImplTest.java b/javatests/com/google/gerrit/acceptance/testsuite/group/GroupOperationsImplTest.java
index 96864d9..a003f9d 100644
--- a/javatests/com/google/gerrit/acceptance/testsuite/group/GroupOperationsImplTest.java
+++ b/javatests/com/google/gerrit/acceptance/testsuite/group/GroupOperationsImplTest.java
@@ -29,9 +29,9 @@
import com.google.gerrit.extensions.common.AccountInfo;
import com.google.gerrit.extensions.common.GroupInfo;
import com.google.gerrit.extensions.restapi.RestApiException;
+import com.google.gerrit.truth.NullAwareCorrespondence;
import com.google.inject.Inject;
import java.sql.Timestamp;
-import java.util.Objects;
import java.util.Optional;
import org.junit.Test;
@@ -616,28 +616,11 @@
}
private static Correspondence<AccountInfo, Account.Id> getAccountToIdCorrespondence() {
- return Correspondence.from(
- (actualAccount, expectedId) -> {
- Account.Id accountId =
- Optional.ofNullable(actualAccount)
- .map(account -> account._accountId)
- .map(Account::id)
- .orElse(null);
- return Objects.equals(accountId, expectedId);
- },
- "has ID");
+ return NullAwareCorrespondence.transforming(
+ account -> Account.id(account._accountId), "has ID");
}
private static Correspondence<GroupInfo, AccountGroup.UUID> getGroupToUuidCorrespondence() {
- return Correspondence.from(
- (actualGroup, expectedUuid) -> {
- AccountGroup.UUID groupUuid =
- Optional.ofNullable(actualGroup)
- .map(group -> group.id)
- .map(AccountGroup::uuid)
- .orElse(null);
- return Objects.equals(groupUuid, expectedUuid);
- },
- "has UUID");
+ return NullAwareCorrespondence.transforming(group -> AccountGroup.uuid(group.id), "has UUID");
}
}
diff --git a/javatests/com/google/gerrit/mail/AbstractParserTest.java b/javatests/com/google/gerrit/mail/AbstractParserTest.java
index f3c8671..2306449 100644
--- a/javatests/com/google/gerrit/mail/AbstractParserTest.java
+++ b/javatests/com/google/gerrit/mail/AbstractParserTest.java
@@ -18,6 +18,7 @@
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.Comment;
+import com.google.gerrit.entities.HumanComment;
import java.sql.Timestamp;
import java.time.Instant;
import java.util.ArrayList;
@@ -37,7 +38,7 @@
}
protected static void assertInlineComment(
- String message, MailComment comment, Comment inReplyTo) {
+ String message, MailComment comment, HumanComment inReplyTo) {
assertThat(comment.fileName).isNull();
assertThat(comment.message).isEqualTo(message);
assertThat(comment.inReplyTo.key).isEqualTo(inReplyTo.key);
@@ -51,9 +52,9 @@
assertThat(comment.type).isEqualTo(MailComment.CommentType.FILE_COMMENT);
}
- protected static Comment newComment(String uuid, String file, String message, int line) {
- Comment c =
- new Comment(
+ protected static HumanComment newComment(String uuid, String file, String message, int line) {
+ HumanComment c =
+ new HumanComment(
new Comment.Key(uuid, file, 1),
Account.id(0),
new Timestamp(0L),
@@ -65,9 +66,10 @@
return c;
}
- protected static Comment newRangeComment(String uuid, String file, String message, int line) {
- Comment c =
- new Comment(
+ protected static HumanComment newRangeComment(
+ String uuid, String file, String message, int line) {
+ HumanComment c =
+ new HumanComment(
new Comment.Key(uuid, file, 1),
Account.id(0),
new Timestamp(0L),
@@ -91,8 +93,8 @@
}
/** Returns a List of default comments for testing. */
- protected static List<Comment> defaultComments() {
- List<Comment> comments = new ArrayList<>();
+ protected static List<HumanComment> defaultComments() {
+ List<HumanComment> comments = new ArrayList<>();
comments.add(newComment("c1", "gerrit-server/test.txt", "comment", 0));
comments.add(newComment("c2", "gerrit-server/test.txt", "comment", 2));
comments.add(newComment("c3", "gerrit-server/test.txt", "comment", 3));
diff --git a/javatests/com/google/gerrit/mail/HtmlParserTest.java b/javatests/com/google/gerrit/mail/HtmlParserTest.java
index 345cb05..d661278 100644
--- a/javatests/com/google/gerrit/mail/HtmlParserTest.java
+++ b/javatests/com/google/gerrit/mail/HtmlParserTest.java
@@ -16,7 +16,7 @@
import static com.google.common.truth.Truth.assertThat;
-import com.google.gerrit.entities.Comment;
+import com.google.gerrit.entities.HumanComment;
import java.util.List;
import org.junit.Ignore;
import org.junit.Test;
@@ -31,7 +31,7 @@
MailMessage.Builder b = newMailMessageBuilder();
b.htmlContent(newHtmlBody("Looks good to me", null, null, null, null, null, null));
- List<Comment> comments = defaultComments();
+ List<HumanComment> comments = defaultComments();
List<MailComment> parsedComments = HtmlParser.parse(b.build(), comments, "");
assertThat(parsedComments).hasSize(1);
@@ -52,7 +52,7 @@
null,
null));
- List<Comment> comments = defaultComments();
+ List<HumanComment> comments = defaultComments();
List<MailComment> parsedComments = HtmlParser.parse(b.build(), comments, "");
assertThat(parsedComments).hasSize(1);
@@ -73,7 +73,7 @@
null,
null));
- List<Comment> comments = defaultComments();
+ List<HumanComment> comments = defaultComments();
List<MailComment> parsedComments = HtmlParser.parse(b.build(), comments, CHANGE_URL);
assertThat(parsedComments).hasSize(3);
@@ -96,7 +96,7 @@
null,
null));
- List<Comment> comments = defaultComments();
+ List<HumanComment> comments = defaultComments();
List<MailComment> parsedComments = HtmlParser.parse(b.build(), comments, CHANGE_URL);
assertThat(parsedComments).hasSize(3);
@@ -121,7 +121,7 @@
null,
null));
- List<Comment> comments = defaultComments();
+ List<HumanComment> comments = defaultComments();
List<MailComment> parsedComments = HtmlParser.parse(b.build(), comments, CHANGE_URL);
assertThat(parsedComments).hasSize(3);
@@ -135,7 +135,7 @@
MailMessage.Builder b = newMailMessageBuilder();
b.htmlContent(newHtmlBody(null, null, null, null, null, null, null));
- List<Comment> comments = defaultComments();
+ List<HumanComment> comments = defaultComments();
List<MailComment> parsedComments = HtmlParser.parse(b.build(), comments, CHANGE_URL);
assertThat(parsedComments).isEmpty();
@@ -148,7 +148,7 @@
newHtmlBody(
null, null, null, "Also have a comment here.", "This is a nice file", null, null));
- List<Comment> comments = defaultComments();
+ List<HumanComment> comments = defaultComments();
List<MailComment> parsedComments = HtmlParser.parse(b.build(), comments, CHANGE_URL);
assertThat(parsedComments).hasSize(2);
@@ -164,7 +164,7 @@
MailMessage.Builder b = newMailMessageBuilder();
b.htmlContent(newHtmlBody(htmlMessage, null, null, htmlMessage, htmlMessage, null, null));
- List<Comment> comments = defaultComments();
+ List<HumanComment> comments = defaultComments();
List<MailComment> parsedComments = HtmlParser.parse(b.build(), comments, CHANGE_URL);
assertThat(parsedComments).hasSize(3);
diff --git a/javatests/com/google/gerrit/mail/TextParserTest.java b/javatests/com/google/gerrit/mail/TextParserTest.java
index 00d5b41..f1d61792 100644
--- a/javatests/com/google/gerrit/mail/TextParserTest.java
+++ b/javatests/com/google/gerrit/mail/TextParserTest.java
@@ -16,7 +16,7 @@
import static com.google.common.truth.Truth.assertThat;
-import com.google.gerrit.entities.Comment;
+import com.google.gerrit.entities.HumanComment;
import java.util.List;
import org.junit.Test;
@@ -39,7 +39,7 @@
MailMessage.Builder b = newMailMessageBuilder();
b.textContent("Looks good to me\n" + quotedFooter);
- List<Comment> comments = defaultComments();
+ List<HumanComment> comments = defaultComments();
List<MailComment> parsedComments = TextParser.parse(b.build(), comments, CHANGE_URL);
assertThat(parsedComments).hasSize(1);
@@ -60,7 +60,7 @@
null)
+ quotedFooter);
- List<Comment> comments = defaultComments();
+ List<HumanComment> comments = defaultComments();
List<MailComment> parsedComments = TextParser.parse(b.build(), comments, CHANGE_URL);
assertThat(parsedComments).hasSize(3);
@@ -83,7 +83,7 @@
null)
+ quotedFooter);
- List<Comment> comments = defaultComments();
+ List<HumanComment> comments = defaultComments();
List<MailComment> parsedComments = TextParser.parse(b.build(), comments, CHANGE_URL);
assertThat(parsedComments).hasSize(3);
@@ -97,7 +97,7 @@
MailMessage.Builder b = newMailMessageBuilder();
b.textContent(newPlaintextBody(null, null, null, null, null, null, null) + quotedFooter);
- List<Comment> comments = defaultComments();
+ List<HumanComment> comments = defaultComments();
List<MailComment> parsedComments = TextParser.parse(b.build(), comments, CHANGE_URL);
assertThat(parsedComments).isEmpty();
@@ -111,7 +111,7 @@
null, null, null, "Also have a comment here.", "This is a nice file", null, null)
+ quotedFooter);
- List<Comment> comments = defaultComments();
+ List<HumanComment> comments = defaultComments();
List<MailComment> parsedComments = TextParser.parse(b.build(), comments, CHANGE_URL);
assertThat(parsedComments).hasSize(2);
@@ -134,7 +134,7 @@
+ quotedFooter)
.replace("> ", ">> "));
- List<Comment> comments = defaultComments();
+ List<HumanComment> comments = defaultComments();
List<MailComment> parsedComments = TextParser.parse(b.build(), comments, CHANGE_URL);
assertThat(parsedComments).hasSize(3);
@@ -157,7 +157,7 @@
"Comment in reply to file comment")
+ quotedFooter);
- List<Comment> comments = defaultComments();
+ List<HumanComment> comments = defaultComments();
List<MailComment> parsedComments = TextParser.parse(b.build(), comments, CHANGE_URL);
assertThat(parsedComments).hasSize(2);
diff --git a/javatests/com/google/gerrit/server/mail/send/CommentFormatterTest.java b/javatests/com/google/gerrit/server/mail/send/HumanCommentFormatterTest.java
similarity index 99%
rename from javatests/com/google/gerrit/server/mail/send/CommentFormatterTest.java
rename to javatests/com/google/gerrit/server/mail/send/HumanCommentFormatterTest.java
index f4fbc78..46ea8b2 100644
--- a/javatests/com/google/gerrit/server/mail/send/CommentFormatterTest.java
+++ b/javatests/com/google/gerrit/server/mail/send/HumanCommentFormatterTest.java
@@ -23,7 +23,7 @@
import java.util.List;
import org.junit.Test;
-public class CommentFormatterTest {
+public class HumanCommentFormatterTest {
private void assertBlock(
List<CommentFormatter.Block> list, int index, CommentFormatter.BlockType type, String text) {
CommentFormatter.Block block = list.get(index);
diff --git a/javatests/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java b/javatests/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java
index 7192c55..bf9b187 100644
--- a/javatests/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java
+++ b/javatests/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java
@@ -23,6 +23,7 @@
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.Comment;
import com.google.gerrit.entities.CommentRange;
+import com.google.gerrit.entities.HumanComment;
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.entities.Project;
import com.google.gerrit.extensions.config.FactoryModule;
@@ -244,7 +245,7 @@
return label;
}
- protected Comment newComment(
+ protected HumanComment newComment(
PatchSet.Id psId,
String filename,
String UUID,
@@ -257,8 +258,8 @@
short side,
ObjectId commitId,
boolean unresolved) {
- Comment c =
- new Comment(
+ HumanComment c =
+ new HumanComment(
new Comment.Key(UUID, filename, psId.get()),
commenter.getAccountId(),
t,
diff --git a/javatests/com/google/gerrit/server/notedb/ChangeNotesStateTest.java b/javatests/com/google/gerrit/server/notedb/ChangeNotesStateTest.java
index efbaed6..a5e7dce 100644
--- a/javatests/com/google/gerrit/server/notedb/ChangeNotesStateTest.java
+++ b/javatests/com/google/gerrit/server/notedb/ChangeNotesStateTest.java
@@ -32,6 +32,7 @@
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.ChangeMessage;
import com.google.gerrit.entities.Comment;
+import com.google.gerrit.entities.HumanComment;
import com.google.gerrit.entities.LabelId;
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.entities.PatchSetApproval;
@@ -714,8 +715,8 @@
@Test
public void serializePublishedComments() throws Exception {
- Comment c1 =
- new Comment(
+ HumanComment c1 =
+ new HumanComment(
new Comment.Key("uuid1", "file1", 1),
Account.id(1001),
new Timestamp(1212L),
@@ -726,8 +727,8 @@
c1.setCommitId(ObjectId.fromString("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"));
String c1Json = Serializer.GSON.toJson(c1);
- Comment c2 =
- new Comment(
+ HumanComment c2 =
+ new HumanComment(
new Comment.Key("uuid2", "file2", 2),
Account.id(1002),
new Timestamp(3434L),
@@ -798,7 +799,7 @@
.put("changeMessages", new TypeLiteral<ImmutableList<ChangeMessage>>() {}.getType())
.put(
"publishedComments",
- new TypeLiteral<ImmutableListMultimap<ObjectId, Comment>>() {}.getType())
+ new TypeLiteral<ImmutableListMultimap<ObjectId, HumanComment>>() {}.getType())
.put("updateCount", int.class)
.build());
}
@@ -970,7 +971,7 @@
"startChar", int.class,
"endLine", int.class,
"endChar", int.class));
- assertThatSerializedClass(Comment.class)
+ assertThatSerializedClass(HumanComment.class)
.hasFields(
ImmutableMap.<String, Type>builder()
.put("key", Comment.Key.class)
diff --git a/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java b/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java
index 964187c..df5903f 100644
--- a/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java
+++ b/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java
@@ -43,8 +43,8 @@
import com.google.gerrit.entities.BranchNameKey;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.ChangeMessage;
-import com.google.gerrit.entities.Comment;
import com.google.gerrit.entities.CommentRange;
+import com.google.gerrit.entities.HumanComment;
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.entities.PatchSetApproval;
import com.google.gerrit.entities.SubmissionId;
@@ -123,7 +123,7 @@
RevCommit commit = tr.commit().message("PS2").create();
ChangeUpdate update = newUpdate(c, changeOwner);
update.putComment(
- Comment.Status.PUBLISHED,
+ HumanComment.Status.PUBLISHED,
newComment(
c.currentPatchSetId(),
"a.txt",
@@ -142,7 +142,7 @@
ChangeNotes notes = newNotes(c);
- ImmutableListMultimap<ObjectId, Comment> comments = notes.getComments();
+ ImmutableListMultimap<ObjectId, HumanComment> comments = notes.getHumanComments();
assertThat(comments).hasSize(1);
assertThat(comments.entries().asList().get(0).getValue().tag).isEqualTo(tag);
}
@@ -185,7 +185,7 @@
RevCommit commit = tr.commit().message("PS2").create();
update = newUpdate(c, changeOwner);
update.putComment(
- Comment.Status.PUBLISHED,
+ HumanComment.Status.PUBLISHED,
newComment(
c.currentPatchSetId(),
"a.txt",
@@ -216,7 +216,7 @@
assertThat(approval.tag()).hasValue(integrationTag);
assertThat(approval.value()).isEqualTo(-1);
- ImmutableListMultimap<ObjectId, Comment> comments = notes.getComments();
+ ImmutableListMultimap<ObjectId, HumanComment> comments = notes.getHumanComments();
assertThat(comments).hasSize(1);
assertThat(comments.entries().asList().get(0).getValue().tag).isEqualTo(coverageTag);
@@ -704,7 +704,7 @@
ChangeUpdate update = newUpdate(c, changeOwner);
AttentionSetUpdate attentionSetUpdate =
AttentionSetUpdate.createForWrite(changeOwner.getAccountId(), Operation.ADD, "test");
- update.setAttentionSetUpdates(ImmutableSet.of(attentionSetUpdate));
+ update.addToPlannedAttentionSetUpdates(ImmutableSet.of(attentionSetUpdate));
update.commit();
ChangeNotes notes = newNotes(c);
@@ -717,12 +717,12 @@
ChangeUpdate update = newUpdate(c, changeOwner);
AttentionSetUpdate attentionSetUpdate =
AttentionSetUpdate.createForWrite(changeOwner.getAccountId(), Operation.ADD, "test");
- update.setAttentionSetUpdates(ImmutableSet.of(attentionSetUpdate));
+ update.addToPlannedAttentionSetUpdates(ImmutableSet.of(attentionSetUpdate));
update.commit();
update = newUpdate(c, changeOwner);
attentionSetUpdate =
AttentionSetUpdate.createForWrite(changeOwner.getAccountId(), Operation.REMOVE, "test");
- update.setAttentionSetUpdates(ImmutableSet.of(attentionSetUpdate));
+ update.addToPlannedAttentionSetUpdates(ImmutableSet.of(attentionSetUpdate));
update.commit();
ChangeNotes notes = newNotes(c);
@@ -739,23 +739,24 @@
IllegalArgumentException thrown =
assertThrows(
IllegalArgumentException.class,
- () -> update.setAttentionSetUpdates(ImmutableSet.of(attentionSetUpdate)));
+ () -> update.addToPlannedAttentionSetUpdates(ImmutableSet.of(attentionSetUpdate)));
assertThat(thrown).hasMessageThat().contains("must not specify timestamp for write");
}
@Test
- public void addAttentionStatus_rejectMultiplePerUser() throws Exception {
+ public void addAttentionStatus_rejectIfSameUserTwice() throws Exception {
Change c = newChange();
ChangeUpdate update = newUpdate(c, changeOwner);
AttentionSetUpdate attentionSetUpdate0 =
AttentionSetUpdate.createForWrite(changeOwner.getAccountId(), Operation.ADD, "test 0");
AttentionSetUpdate attentionSetUpdate1 =
AttentionSetUpdate.createForWrite(changeOwner.getAccountId(), Operation.ADD, "test 1");
+
IllegalArgumentException thrown =
assertThrows(
IllegalArgumentException.class,
() ->
- update.setAttentionSetUpdates(
+ update.addToPlannedAttentionSetUpdates(
ImmutableSet.of(attentionSetUpdate0, attentionSetUpdate1)));
assertThat(thrown)
.hasMessageThat()
@@ -771,7 +772,8 @@
AttentionSetUpdate attentionSetUpdate1 =
AttentionSetUpdate.createForWrite(otherUser.getAccountId(), Operation.ADD, "test");
- update.setAttentionSetUpdates(ImmutableSet.of(attentionSetUpdate0, attentionSetUpdate1));
+ update.addToPlannedAttentionSetUpdates(
+ ImmutableSet.of(attentionSetUpdate0, attentionSetUpdate1));
update.commit();
ChangeNotes notes = newNotes(c);
@@ -1186,7 +1188,7 @@
update.putApproval("Code-Review", (short) 1);
update.setChangeMessage("This is a message");
update.putComment(
- Comment.Status.PUBLISHED,
+ HumanComment.Status.PUBLISHED,
newComment(
c.currentPatchSetId(),
"a.txt",
@@ -1206,7 +1208,7 @@
assertThat(notes.getPatchSets().keySet()).containsExactly(psId1, psId2);
assertThat(notes.getApprovals()).isNotEmpty();
assertThat(notes.getChangeMessages()).isNotEmpty();
- assertThat(notes.getComments()).isNotEmpty();
+ assertThat(notes.getHumanComments()).isNotEmpty();
// publish ps2
update = newUpdate(c, changeOwner);
@@ -1222,7 +1224,7 @@
assertThat(notes.getPatchSets().keySet()).containsExactly(psId1);
assertThat(notes.getApprovals()).isEmpty();
assertThat(notes.getChangeMessages()).isEmpty();
- assertThat(notes.getComments()).isEmpty();
+ assertThat(notes.getHumanComments()).isEmpty();
}
@Test
@@ -1279,14 +1281,14 @@
Map<PatchSet.Id, PatchSet> patchSets = notes.getPatchSets();
assertThat(patchSets.get(psId1).pushCertificate()).isEmpty();
assertThat(patchSets.get(psId2).pushCertificate()).hasValue(pushCert);
- assertThat(notes.getComments()).isEmpty();
+ assertThat(notes.getHumanComments()).isEmpty();
// comment on ps2
update = newUpdate(c, changeOwner);
update.setPatchSetId(psId2);
Timestamp ts = TimeUtil.nowTs();
update.putComment(
- Comment.Status.PUBLISHED,
+ HumanComment.Status.PUBLISHED,
newComment(
psId2,
"a.txt",
@@ -1307,7 +1309,7 @@
patchSets = notes.getPatchSets();
assertThat(patchSets.get(psId1).pushCertificate()).isEmpty();
assertThat(patchSets.get(psId2).pushCertificate()).hasValue(pushCert);
- assertThat(notes.getComments()).isNotEmpty();
+ assertThat(notes.getHumanComments()).isNotEmpty();
}
@Test
@@ -1356,7 +1358,7 @@
PatchSet.Id psId = c.currentPatchSetId();
RevCommit tipCommit;
try (NoteDbUpdateManager updateManager = updateManagerFactory.create(project)) {
- Comment comment1 =
+ HumanComment comment1 =
newComment(
psId,
"file1",
@@ -1371,7 +1373,7 @@
ObjectId.fromString("abcd1234abcd1234abcd1234abcd1234abcd1234"),
false);
update1.setPatchSetId(psId);
- update1.putComment(Comment.Status.PUBLISHED, comment1);
+ update1.putComment(HumanComment.Status.PUBLISHED, comment1);
updateManager.add(update1);
ChangeUpdate update2 = newUpdate(c, otherUser);
@@ -1570,7 +1572,7 @@
PatchSet.Id psId = c.currentPatchSetId();
ObjectId commitId = ObjectId.fromString("abcd1234abcd1234abcd1234abcd1234abcd1234");
- Comment comment =
+ HumanComment comment =
newComment(
psId,
"file1",
@@ -1585,11 +1587,11 @@
commitId,
false);
update.setPatchSetId(psId);
- update.putComment(Comment.Status.PUBLISHED, comment);
+ update.putComment(HumanComment.Status.PUBLISHED, comment);
update.commit();
ChangeNotes notes = newNotes(c);
- assertThat(notes.getComments()).isEqualTo(ImmutableListMultimap.of(commitId, comment));
+ assertThat(notes.getHumanComments()).isEqualTo(ImmutableListMultimap.of(commitId, comment));
}
@Test
@@ -1600,7 +1602,7 @@
ObjectId commitId = ObjectId.fromString("abcd1234abcd1234abcd1234abcd1234abcd1234");
CommentRange range = new CommentRange(1, 0, 2, 0);
- Comment comment =
+ HumanComment comment =
newComment(
psId,
"file1",
@@ -1615,11 +1617,11 @@
commitId,
false);
update.setPatchSetId(psId);
- update.putComment(Comment.Status.PUBLISHED, comment);
+ update.putComment(HumanComment.Status.PUBLISHED, comment);
update.commit();
ChangeNotes notes = newNotes(c);
- assertThat(notes.getComments()).isEqualTo(ImmutableListMultimap.of(commitId, comment));
+ assertThat(notes.getHumanComments()).isEqualTo(ImmutableListMultimap.of(commitId, comment));
}
@Test
@@ -1630,7 +1632,7 @@
ObjectId commitId = ObjectId.fromString("abcd1234abcd1234abcd1234abcd1234abcd1234");
CommentRange range = new CommentRange(0, 0, 0, 0);
- Comment comment =
+ HumanComment comment =
newComment(
psId,
"file",
@@ -1645,11 +1647,11 @@
commitId,
false);
update.setPatchSetId(psId);
- update.putComment(Comment.Status.PUBLISHED, comment);
+ update.putComment(HumanComment.Status.PUBLISHED, comment);
update.commit();
ChangeNotes notes = newNotes(c);
- assertThat(notes.getComments()).isEqualTo(ImmutableListMultimap.of(commitId, comment));
+ assertThat(notes.getHumanComments()).isEqualTo(ImmutableListMultimap.of(commitId, comment));
}
@Test
@@ -1660,7 +1662,7 @@
ObjectId commitId = ObjectId.fromString("abcd1234abcd1234abcd1234abcd1234abcd1234");
CommentRange range = new CommentRange(1, 2, 3, 4);
- Comment comment =
+ HumanComment comment =
newComment(
psId,
"",
@@ -1675,11 +1677,11 @@
commitId,
false);
update.setPatchSetId(psId);
- update.putComment(Comment.Status.PUBLISHED, comment);
+ update.putComment(HumanComment.Status.PUBLISHED, comment);
update.commit();
ChangeNotes notes = newNotes(c);
- assertThat(notes.getComments()).isEqualTo(ImmutableListMultimap.of(commitId, comment));
+ assertThat(notes.getHumanComments()).isEqualTo(ImmutableListMultimap.of(commitId, comment));
}
@Test
@@ -1699,7 +1701,7 @@
Timestamp time = TimeUtil.nowTs();
ObjectId commitId = ObjectId.fromString("abcd1234abcd1234abcd1234abcd1234abcd1234");
- Comment comment1 =
+ HumanComment comment1 =
newComment(
psId1,
"file1",
@@ -1713,7 +1715,7 @@
(short) 0,
commitId,
false);
- Comment comment2 =
+ HumanComment comment2 =
newComment(
psId1,
"file1",
@@ -1727,7 +1729,7 @@
(short) 0,
commitId,
false);
- Comment comment3 =
+ HumanComment comment3 =
newComment(
psId2,
"file1",
@@ -1744,13 +1746,13 @@
ChangeUpdate update = newUpdate(c, otherUser);
update.setPatchSetId(psId2);
- update.putComment(Comment.Status.PUBLISHED, comment3);
- update.putComment(Comment.Status.PUBLISHED, comment2);
- update.putComment(Comment.Status.PUBLISHED, comment1);
+ update.putComment(HumanComment.Status.PUBLISHED, comment3);
+ update.putComment(HumanComment.Status.PUBLISHED, comment2);
+ update.putComment(HumanComment.Status.PUBLISHED, comment1);
update.commit();
ChangeNotes notes = newNotes(c);
- assertThat(notes.getComments())
+ assertThat(notes.getHumanComments())
.isEqualTo(
ImmutableListMultimap.of(
commitId, comment1,
@@ -1770,7 +1772,7 @@
PatchSet.Id psId = c.currentPatchSetId();
ObjectId commitId = ObjectId.fromString("abcd1234abcd1234abcd1234abcd1234abcd1234");
- Comment comment =
+ HumanComment comment =
newComment(
psId,
"file",
@@ -1786,12 +1788,12 @@
false);
comment.setRealAuthor(changeOwner.getAccountId());
update.setPatchSetId(psId);
- update.putComment(Comment.Status.PUBLISHED, comment);
+ update.putComment(HumanComment.Status.PUBLISHED, comment);
update.commit();
ChangeNotes notes = newNotes(c);
- assertThat(notes.getComments()).isEqualTo(ImmutableListMultimap.of(commitId, comment));
+ assertThat(notes.getHumanComments()).isEqualTo(ImmutableListMultimap.of(commitId, comment));
}
@Test
@@ -1809,7 +1811,7 @@
Timestamp time = TimeUtil.nowTs();
PatchSet.Id psId = c.currentPatchSetId();
- Comment comment =
+ HumanComment comment =
newComment(
psId,
"file1",
@@ -1824,12 +1826,12 @@
ObjectId.fromString("abcd1234abcd1234abcd1234abcd1234abcd1234"),
false);
update.setPatchSetId(psId);
- update.putComment(Comment.Status.PUBLISHED, comment);
+ update.putComment(HumanComment.Status.PUBLISHED, comment);
update.commit();
ChangeNotes notes = newNotes(c);
- assertThat(notes.getComments())
+ assertThat(notes.getHumanComments())
.isEqualTo(ImmutableListMultimap.of(comment.getCommitId(), comment));
}
@@ -1847,7 +1849,7 @@
Timestamp now = TimeUtil.nowTs();
PatchSet.Id psId = c.currentPatchSetId();
- Comment commentForBase =
+ HumanComment commentForBase =
newComment(
psId,
"filename",
@@ -1862,11 +1864,11 @@
commitId1,
false);
update.setPatchSetId(psId);
- update.putComment(Comment.Status.PUBLISHED, commentForBase);
+ update.putComment(HumanComment.Status.PUBLISHED, commentForBase);
update.commit();
update = newUpdate(c, otherUser);
- Comment commentForPS =
+ HumanComment commentForPS =
newComment(
psId,
"filename",
@@ -1881,10 +1883,10 @@
commitId2,
false);
update.setPatchSetId(psId);
- update.putComment(Comment.Status.PUBLISHED, commentForPS);
+ update.putComment(HumanComment.Status.PUBLISHED, commentForPS);
update.commit();
- assertThat(newNotes(c).getComments())
+ assertThat(newNotes(c).getHumanComments())
.containsExactlyEntriesIn(
ImmutableListMultimap.of(
commitId1, commentForBase,
@@ -1905,7 +1907,7 @@
ChangeUpdate update = newUpdate(c, otherUser);
Timestamp timeForComment1 = TimeUtil.nowTs();
Timestamp timeForComment2 = TimeUtil.nowTs();
- Comment comment1 =
+ HumanComment comment1 =
newComment(
psId,
filename,
@@ -1920,11 +1922,11 @@
commitId,
false);
update.setPatchSetId(psId);
- update.putComment(Comment.Status.PUBLISHED, comment1);
+ update.putComment(HumanComment.Status.PUBLISHED, comment1);
update.commit();
update = newUpdate(c, otherUser);
- Comment comment2 =
+ HumanComment comment2 =
newComment(
psId,
filename,
@@ -1939,10 +1941,10 @@
commitId,
false);
update.setPatchSetId(psId);
- update.putComment(Comment.Status.PUBLISHED, comment2);
+ update.putComment(HumanComment.Status.PUBLISHED, comment2);
update.commit();
- assertThat(newNotes(c).getComments())
+ assertThat(newNotes(c).getHumanComments())
.containsExactlyEntriesIn(
ImmutableListMultimap.of(
commitId, comment1,
@@ -1963,7 +1965,7 @@
ChangeUpdate update = newUpdate(c, otherUser);
Timestamp now = TimeUtil.nowTs();
- Comment comment1 =
+ HumanComment comment1 =
newComment(
psId,
filename1,
@@ -1978,11 +1980,11 @@
commitId,
false);
update.setPatchSetId(psId);
- update.putComment(Comment.Status.PUBLISHED, comment1);
+ update.putComment(HumanComment.Status.PUBLISHED, comment1);
update.commit();
update = newUpdate(c, otherUser);
- Comment comment2 =
+ HumanComment comment2 =
newComment(
psId,
filename2,
@@ -1997,10 +1999,10 @@
commitId,
false);
update.setPatchSetId(psId);
- update.putComment(Comment.Status.PUBLISHED, comment2);
+ update.putComment(HumanComment.Status.PUBLISHED, comment2);
update.commit();
- assertThat(newNotes(c).getComments())
+ assertThat(newNotes(c).getHumanComments())
.containsExactlyEntriesIn(
ImmutableListMultimap.of(
commitId, comment1,
@@ -2021,7 +2023,7 @@
ChangeUpdate update = newUpdate(c, otherUser);
Timestamp now = TimeUtil.nowTs();
- Comment comment1 =
+ HumanComment comment1 =
newComment(
ps1,
filename,
@@ -2036,7 +2038,7 @@
commitId1,
false);
update.setPatchSetId(ps1);
- update.putComment(Comment.Status.PUBLISHED, comment1);
+ update.putComment(HumanComment.Status.PUBLISHED, comment1);
update.commit();
incrementPatchSet(c);
@@ -2044,7 +2046,7 @@
update = newUpdate(c, otherUser);
now = TimeUtil.nowTs();
- Comment comment2 =
+ HumanComment comment2 =
newComment(
ps2,
filename,
@@ -2059,10 +2061,10 @@
commitId2,
false);
update.setPatchSetId(ps2);
- update.putComment(Comment.Status.PUBLISHED, comment2);
+ update.putComment(HumanComment.Status.PUBLISHED, comment2);
update.commit();
- assertThat(newNotes(c).getComments())
+ assertThat(newNotes(c).getHumanComments())
.containsExactlyEntriesIn(
ImmutableListMultimap.of(
commitId1, comment1,
@@ -2081,7 +2083,7 @@
ChangeUpdate update = newUpdate(c, otherUser);
Timestamp now = TimeUtil.nowTs();
- Comment comment1 =
+ HumanComment comment1 =
newComment(
ps1,
filename,
@@ -2096,22 +2098,22 @@
commitId,
false);
update.setPatchSetId(ps1);
- update.putComment(Comment.Status.DRAFT, comment1);
+ update.putComment(HumanComment.Status.DRAFT, comment1);
update.commit();
ChangeNotes notes = newNotes(c);
assertThat(notes.getDraftComments(otherUserId))
.containsExactlyEntriesIn(ImmutableListMultimap.of(commitId, comment1));
- assertThat(notes.getComments()).isEmpty();
+ assertThat(notes.getHumanComments()).isEmpty();
update = newUpdate(c, otherUser);
update.setPatchSetId(ps1);
- update.putComment(Comment.Status.PUBLISHED, comment1);
+ update.putComment(HumanComment.Status.PUBLISHED, comment1);
update.commit();
notes = newNotes(c);
assertThat(notes.getDraftComments(otherUserId)).isEmpty();
- assertThat(notes.getComments())
+ assertThat(notes.getHumanComments())
.containsExactlyEntriesIn(ImmutableListMultimap.of(commitId, comment1));
}
@@ -2131,7 +2133,7 @@
// Write two drafts on the same side of one patch set.
ChangeUpdate update = newUpdate(c, otherUser);
update.setPatchSetId(psId);
- Comment comment1 =
+ HumanComment comment1 =
newComment(
psId,
filename,
@@ -2145,7 +2147,7 @@
side,
commitId,
false);
- Comment comment2 =
+ HumanComment comment2 =
newComment(
psId,
filename,
@@ -2159,8 +2161,8 @@
side,
commitId,
false);
- update.putComment(Comment.Status.DRAFT, comment1);
- update.putComment(Comment.Status.DRAFT, comment2);
+ update.putComment(HumanComment.Status.DRAFT, comment1);
+ update.putComment(HumanComment.Status.DRAFT, comment2);
update.commit();
ChangeNotes notes = newNotes(c);
@@ -2170,18 +2172,18 @@
commitId, comment1,
commitId, comment2))
.inOrder();
- assertThat(notes.getComments()).isEmpty();
+ assertThat(notes.getHumanComments()).isEmpty();
// Publish first draft.
update = newUpdate(c, otherUser);
update.setPatchSetId(psId);
- update.putComment(Comment.Status.PUBLISHED, comment1);
+ update.putComment(HumanComment.Status.PUBLISHED, comment1);
update.commit();
notes = newNotes(c);
assertThat(notes.getDraftComments(otherUserId))
.containsExactlyEntriesIn(ImmutableListMultimap.of(commitId, comment2));
- assertThat(notes.getComments())
+ assertThat(notes.getHumanComments())
.containsExactlyEntriesIn(ImmutableListMultimap.of(commitId, comment1));
}
@@ -2201,7 +2203,7 @@
// Write two drafts, one on each side of the patchset.
ChangeUpdate update = newUpdate(c, otherUser);
update.setPatchSetId(psId);
- Comment baseComment =
+ HumanComment baseComment =
newComment(
psId,
filename,
@@ -2215,7 +2217,7 @@
(short) 0,
commitId1,
false);
- Comment psComment =
+ HumanComment psComment =
newComment(
psId,
filename,
@@ -2230,8 +2232,8 @@
commitId2,
false);
- update.putComment(Comment.Status.DRAFT, baseComment);
- update.putComment(Comment.Status.DRAFT, psComment);
+ update.putComment(HumanComment.Status.DRAFT, baseComment);
+ update.putComment(HumanComment.Status.DRAFT, psComment);
update.commit();
ChangeNotes notes = newNotes(c);
@@ -2240,19 +2242,19 @@
ImmutableListMultimap.of(
commitId1, baseComment,
commitId2, psComment));
- assertThat(notes.getComments()).isEmpty();
+ assertThat(notes.getHumanComments()).isEmpty();
// Publish both comments.
update = newUpdate(c, otherUser);
update.setPatchSetId(psId);
- update.putComment(Comment.Status.PUBLISHED, baseComment);
- update.putComment(Comment.Status.PUBLISHED, psComment);
+ update.putComment(HumanComment.Status.PUBLISHED, baseComment);
+ update.putComment(HumanComment.Status.PUBLISHED, psComment);
update.commit();
notes = newNotes(c);
assertThat(notes.getDraftComments(otherUserId)).isEmpty();
- assertThat(notes.getComments())
+ assertThat(notes.getHumanComments())
.containsExactlyEntriesIn(
ImmutableListMultimap.of(
commitId1, baseComment,
@@ -2271,7 +2273,7 @@
ChangeUpdate update = newUpdate(c, otherUser);
Timestamp now = TimeUtil.nowTs();
- Comment comment =
+ HumanComment comment =
newComment(
psId,
filename,
@@ -2286,7 +2288,7 @@
commitId,
false);
update.setPatchSetId(psId);
- update.putComment(Comment.Status.DRAFT, comment);
+ update.putComment(HumanComment.Status.DRAFT, comment);
update.commit();
ChangeNotes notes = newNotes(c);
@@ -2316,7 +2318,7 @@
ChangeUpdate update = newUpdate(c, otherUser);
Timestamp now = TimeUtil.nowTs();
- Comment comment1 =
+ HumanComment comment1 =
newComment(
ps1,
filename,
@@ -2331,7 +2333,7 @@
commitId1,
false);
update.setPatchSetId(ps1);
- update.putComment(Comment.Status.DRAFT, comment1);
+ update.putComment(HumanComment.Status.DRAFT, comment1);
update.commit();
incrementPatchSet(c);
@@ -2339,7 +2341,7 @@
update = newUpdate(c, otherUser);
now = TimeUtil.nowTs();
- Comment comment2 =
+ HumanComment comment2 =
newComment(
ps2,
filename,
@@ -2354,7 +2356,7 @@
commitId2,
false);
update.setPatchSetId(ps2);
- update.putComment(Comment.Status.DRAFT, comment2);
+ update.putComment(HumanComment.Status.DRAFT, comment2);
update.commit();
ChangeNotes notes = newNotes(c);
@@ -2384,7 +2386,7 @@
ChangeUpdate update = newUpdate(c, otherUser);
Timestamp now = TimeUtil.nowTs();
- Comment comment =
+ HumanComment comment =
newComment(
ps1,
filename,
@@ -2398,7 +2400,7 @@
side,
commitId,
false);
- update.putComment(Comment.Status.PUBLISHED, comment);
+ update.putComment(HumanComment.Status.PUBLISHED, comment);
update.commit();
assertThat(repo.exactRef(changeMetaRef(c.getId()))).isNotNull();
@@ -2417,7 +2419,7 @@
ChangeUpdate update = newUpdate(c, otherUser);
Timestamp now = TimeUtil.nowTs();
- Comment draft =
+ HumanComment draft =
newComment(
ps1,
filename,
@@ -2431,7 +2433,7 @@
side,
commitId,
false);
- update.putComment(Comment.Status.DRAFT, draft);
+ update.putComment(HumanComment.Status.DRAFT, draft);
update.commit();
String draftRef = refsDraftComments(c.getId(), otherUser.getAccountId());
@@ -2439,7 +2441,7 @@
assertThat(old).isNotNull();
update = newUpdate(c, otherUser);
- Comment pub =
+ HumanComment pub =
newComment(
ps1,
filename,
@@ -2453,7 +2455,7 @@
side,
commitId,
false);
- update.putComment(Comment.Status.PUBLISHED, pub);
+ update.putComment(HumanComment.Status.PUBLISHED, pub);
update.commit();
assertThat(exactRefAllUsers(draftRef)).isEqualTo(old);
@@ -2469,7 +2471,7 @@
Timestamp now = TimeUtil.nowTs();
PatchSet.Id psId = c.currentPatchSetId();
- Comment comment =
+ HumanComment comment =
newComment(
psId,
"filename",
@@ -2484,10 +2486,10 @@
commitId,
false);
update.setPatchSetId(psId);
- update.putComment(Comment.Status.PUBLISHED, comment);
+ update.putComment(HumanComment.Status.PUBLISHED, comment);
update.commit();
- assertThat(newNotes(c).getComments())
+ assertThat(newNotes(c).getHumanComments())
.containsExactlyEntriesIn(ImmutableListMultimap.of(commitId, comment));
}
@@ -2501,7 +2503,7 @@
Timestamp now = TimeUtil.nowTs();
PatchSet.Id psId = c.currentPatchSetId();
- Comment comment =
+ HumanComment comment =
newComment(
psId,
"filename",
@@ -2516,10 +2518,10 @@
commitId,
false);
update.setPatchSetId(psId);
- update.putComment(Comment.Status.PUBLISHED, comment);
+ update.putComment(HumanComment.Status.PUBLISHED, comment);
update.commit();
- assertThat(newNotes(c).getComments())
+ assertThat(newNotes(c).getHumanComments())
.containsExactlyEntriesIn(ImmutableListMultimap.of(commitId, comment));
}
@@ -2540,7 +2542,7 @@
ChangeUpdate update = newUpdate(c, otherUser);
update.setPatchSetId(ps2);
Timestamp now = TimeUtil.nowTs();
- Comment comment1 =
+ HumanComment comment1 =
newComment(
ps1,
filename,
@@ -2554,7 +2556,7 @@
side,
commitId1,
false);
- Comment comment2 =
+ HumanComment comment2 =
newComment(
ps2,
filename,
@@ -2568,23 +2570,23 @@
side,
commitId2,
false);
- update.putComment(Comment.Status.DRAFT, comment1);
- update.putComment(Comment.Status.DRAFT, comment2);
+ update.putComment(HumanComment.Status.DRAFT, comment1);
+ update.putComment(HumanComment.Status.DRAFT, comment2);
update.commit();
ChangeNotes notes = newNotes(c);
assertThat(notes.getDraftComments(otherUserId)).hasSize(2);
- assertThat(notes.getComments()).isEmpty();
+ assertThat(notes.getHumanComments()).isEmpty();
update = newUpdate(c, otherUser);
update.setPatchSetId(ps2);
- update.putComment(Comment.Status.PUBLISHED, comment1);
- update.putComment(Comment.Status.PUBLISHED, comment2);
+ update.putComment(HumanComment.Status.PUBLISHED, comment1);
+ update.putComment(HumanComment.Status.PUBLISHED, comment2);
update.commit();
notes = newNotes(c);
assertThat(notes.getDraftComments(otherUserId)).isEmpty();
- assertThat(notes.getComments()).hasSize(2);
+ assertThat(notes.getHumanComments()).hasSize(2);
}
@Test
@@ -2598,7 +2600,7 @@
ChangeUpdate update = newUpdate(c, otherUser);
update.setPatchSetId(ps1);
Timestamp now = TimeUtil.nowTs();
- Comment comment1 =
+ HumanComment comment1 =
newComment(
ps1,
"file1",
@@ -2612,7 +2614,7 @@
side,
commitId1,
false);
- Comment comment2 =
+ HumanComment comment2 =
newComment(
ps1,
"file2",
@@ -2626,23 +2628,23 @@
side,
commitId1,
false);
- update.putComment(Comment.Status.DRAFT, comment1);
- update.putComment(Comment.Status.DRAFT, comment2);
+ update.putComment(HumanComment.Status.DRAFT, comment1);
+ update.putComment(HumanComment.Status.DRAFT, comment2);
update.commit();
ChangeNotes notes = newNotes(c);
assertThat(notes.getDraftComments(otherUserId).get(commitId1))
.containsExactly(comment1, comment2);
- assertThat(notes.getComments()).isEmpty();
+ assertThat(notes.getHumanComments()).isEmpty();
update = newUpdate(c, otherUser);
update.setPatchSetId(ps1);
- update.putComment(Comment.Status.PUBLISHED, comment2);
+ update.putComment(HumanComment.Status.PUBLISHED, comment2);
update.commit();
notes = newNotes(c);
assertThat(notes.getDraftComments(otherUserId).get(commitId1)).containsExactly(comment1);
- assertThat(notes.getComments().get(commitId1)).containsExactly(comment2);
+ assertThat(notes.getHumanComments().get(commitId1)).containsExactly(comment2);
}
@Test
@@ -2671,7 +2673,7 @@
ChangeUpdate update = newUpdate(c, otherUser);
Timestamp now = TimeUtil.nowTs();
- Comment comment1 =
+ HumanComment comment1 =
newComment(
ps1,
"file1",
@@ -2685,7 +2687,7 @@
side,
commitId1,
false);
- Comment comment2 =
+ HumanComment comment2 =
newComment(
ps1,
"file2",
@@ -2699,8 +2701,8 @@
side,
commitId1,
false);
- update.putComment(Comment.Status.DRAFT, comment1);
- update.putComment(Comment.Status.DRAFT, comment2);
+ update.putComment(HumanComment.Status.DRAFT, comment1);
+ update.putComment(HumanComment.Status.DRAFT, comment2);
update.commit();
String refName = refsDraftComments(c.getId(), otherUserId);
@@ -2708,7 +2710,7 @@
update = newUpdate(c, otherUser);
update.setPatchSetId(ps1);
- update.putComment(Comment.Status.PUBLISHED, comment2);
+ update.putComment(HumanComment.Status.PUBLISHED, comment2);
update.commit();
assertThat(exactRefAllUsers(refName)).isNotNull();
assertThat(exactRefAllUsers(refName)).isNotEqualTo(oldDraftId);
@@ -2730,11 +2732,11 @@
// Zombie comment is filtered out of drafts via ChangeNotes.
ChangeNotes notes = newNotes(c);
assertThat(notes.getDraftComments(otherUserId).get(commitId1)).containsExactly(comment1);
- assertThat(notes.getComments().get(commitId1)).containsExactly(comment2);
+ assertThat(notes.getHumanComments().get(commitId1)).containsExactly(comment2);
update = newUpdate(c, otherUser);
update.setPatchSetId(ps1);
- update.putComment(Comment.Status.PUBLISHED, comment1);
+ update.putComment(HumanComment.Status.PUBLISHED, comment1);
update.commit();
// Updating an unrelated comment causes the zombie comment to get fixed up.
@@ -2748,7 +2750,7 @@
ObjectId commitId = ObjectId.fromString("abcd1234abcd1234abcd1234abcd1234abcd1234");
ChangeUpdate update1 = newUpdate(c, otherUser);
- Comment comment1 =
+ HumanComment comment1 =
newComment(
c.currentPatchSetId(),
"filename",
@@ -2762,10 +2764,10 @@
(short) 1,
commitId,
false);
- update1.putComment(Comment.Status.PUBLISHED, comment1);
+ update1.putComment(HumanComment.Status.PUBLISHED, comment1);
ChangeUpdate update2 = newUpdate(c, otherUser);
- Comment comment2 =
+ HumanComment comment2 =
newComment(
c.currentPatchSetId(),
"filename",
@@ -2779,7 +2781,7 @@
(short) 1,
commitId,
false);
- update2.putComment(Comment.Status.PUBLISHED, comment2);
+ update2.putComment(HumanComment.Status.PUBLISHED, comment2);
try (NoteDbUpdateManager manager = updateManagerFactory.create(project)) {
manager.add(update1);
@@ -2788,7 +2790,7 @@
}
ChangeNotes notes = newNotes(c);
- List<Comment> comments = notes.getComments().get(commitId);
+ List<HumanComment> comments = notes.getHumanComments().get(commitId);
assertThat(comments).hasSize(2);
assertThat(comments.get(0).message).isEqualTo("comment 1");
assertThat(comments.get(1).message).isEqualTo("comment 2");
@@ -2815,14 +2817,14 @@
int numMessages = notes.getChangeMessages().size();
int numPatchSets = notes.getPatchSets().size();
int numApprovals = notes.getApprovals().size();
- int numComments = notes.getComments().size();
+ int numComments = notes.getHumanComments().size();
ChangeUpdate update = newUpdate(c, changeOwner);
update.setPatchSetId(PatchSet.id(c.getId(), c.currentPatchSetId().get() + 1));
update.setChangeMessage("Should be ignored");
update.putApproval("Code-Review", (short) 2);
CommentRange range = new CommentRange(1, 1, 2, 1);
- Comment comment =
+ HumanComment comment =
newComment(
update.getPatchSetId(),
"filename",
@@ -2836,14 +2838,14 @@
(short) 1,
ObjectId.fromString("abcd1234abcd1234abcd1234abcd1234abcd1234"),
false);
- update.putComment(Comment.Status.PUBLISHED, comment);
+ update.putComment(HumanComment.Status.PUBLISHED, comment);
update.commit();
notes = newNotes(c);
assertThat(notes.getChangeMessages()).hasSize(numMessages);
assertThat(notes.getPatchSets()).hasSize(numPatchSets);
assertThat(notes.getApprovals()).hasSize(numApprovals);
- assertThat(notes.getComments()).hasSize(numComments);
+ assertThat(notes.getHumanComments()).hasSize(numComments);
}
@Test
diff --git a/javatests/com/google/gerrit/server/notedb/CommentTimestampAdapterTest.java b/javatests/com/google/gerrit/server/notedb/CommentTimestampAdapterTest.java
index c2620dc..2c1348c 100644
--- a/javatests/com/google/gerrit/server/notedb/CommentTimestampAdapterTest.java
+++ b/javatests/com/google/gerrit/server/notedb/CommentTimestampAdapterTest.java
@@ -18,6 +18,7 @@
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.Comment;
+import com.google.gerrit.entities.HumanComment;
import com.google.gson.Gson;
import com.google.gson.GsonBuilder;
import java.sql.Timestamp;
@@ -151,7 +152,7 @@
@Test
public void newAdapterRoundTripOfWholeComment() {
Comment c =
- new Comment(
+ new HumanComment(
new Comment.Key("uuid", "filename", 1),
Account.id(100),
NON_DST_TS,
@@ -165,7 +166,7 @@
String json = gson.toJson(c);
assertThat(json).contains("\"writtenOn\": \"" + NON_DST_STR_TRUNC + "\",");
- Comment result = gson.fromJson(json, Comment.class);
+ Comment result = gson.fromJson(json, HumanComment.class);
// Round-trip lossily truncates ms, but that's ok.
assertThat(result.writtenOn).isEqualTo(NON_DST_TS_TRUNC);
result.writtenOn = NON_DST_TS;
diff --git a/javatests/com/google/gerrit/server/notedb/CommitMessageOutputTest.java b/javatests/com/google/gerrit/server/notedb/CommitMessageOutputTest.java
index 6a090c1..8818d81 100644
--- a/javatests/com/google/gerrit/server/notedb/CommitMessageOutputTest.java
+++ b/javatests/com/google/gerrit/server/notedb/CommitMessageOutputTest.java
@@ -45,7 +45,6 @@
update.putReviewer(otherUser.getAccount().id(), CC);
update.commit();
assertThat(update.getRefName()).isEqualTo("refs/changes/01/1/meta");
-
RevCommit commit = parseCommit(update.getResult());
assertBodyEquals(
"Update patch set 1\n"
@@ -62,7 +61,8 @@
+ "Reviewer: Gerrit User 1 <1@gerrit>\n"
+ "CC: Gerrit User 2 <2@gerrit>\n"
+ "Label: Code-Review=-1\n"
- + "Label: Verified=+1\n",
+ + "Label: Verified=+1\n"
+ + "Attention: {\"person_ident\":\"Gerrit User 1 \\u003c1@gerrit\\u003e\",\"operation\":\"ADD\",\"reason\":\"Reviewer was added\"}\n",
commit);
PersonIdent author = commit.getAuthorIdent();
@@ -245,7 +245,8 @@
update.commit();
assertBodyEquals(
- "Update patch set 1\n\nPatch-set: 1\nReviewer: Gerrit User 1 <1@gerrit>\n",
+ "Update patch set 1\n\nPatch-set: 1\nReviewer: Gerrit User 1 <1@gerrit>\n"
+ + "Attention: {\"person_ident\":\"Gerrit User 1 \\u003c1@gerrit\\u003e\",\"operation\":\"ADD\",\"reason\":\"Reviewer was added\"}\n",
update.getResult());
}
diff --git a/javatests/com/google/gerrit/server/notedb/DraftCommentNotesTest.java b/javatests/com/google/gerrit/server/notedb/DraftCommentNotesTest.java
index bf49884..041366c 100644
--- a/javatests/com/google/gerrit/server/notedb/DraftCommentNotesTest.java
+++ b/javatests/com/google/gerrit/server/notedb/DraftCommentNotesTest.java
@@ -17,7 +17,7 @@
import static com.google.common.truth.Truth.assertThat;
import com.google.gerrit.entities.Change;
-import com.google.gerrit.entities.Comment;
+import com.google.gerrit.entities.HumanComment;
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.server.util.time.TimeUtil;
import org.eclipse.jgit.lib.ObjectId;
@@ -31,7 +31,7 @@
Change c = newChange();
ChangeUpdate update = newUpdate(c, otherUser);
update.setPatchSetId(c.currentPatchSetId());
- update.putComment(Comment.Status.PUBLISHED, comment(c.currentPatchSetId()));
+ update.putComment(HumanComment.Status.PUBLISHED, comment(c.currentPatchSetId()));
update.commit();
assertThat(newNotes(c).getDraftComments(otherUserId)).isEmpty();
@@ -44,13 +44,13 @@
ChangeUpdate update = newUpdate(c, otherUser);
update.setPatchSetId(c.currentPatchSetId());
- update.putComment(Comment.Status.DRAFT, comment(c.currentPatchSetId()));
+ update.putComment(HumanComment.Status.DRAFT, comment(c.currentPatchSetId()));
update.commit();
assertThat(newNotes(c).getDraftComments(otherUserId)).hasSize(1);
assertableFanOutExecutor.assertInteractions(0);
update = newUpdate(c, otherUser);
- update.putComment(Comment.Status.PUBLISHED, comment(c.currentPatchSetId()));
+ update.putComment(HumanComment.Status.PUBLISHED, comment(c.currentPatchSetId()));
update.commit();
assertThat(newNotes(c).getDraftComments(otherUserId)).isEmpty();
@@ -63,7 +63,7 @@
ChangeUpdate update = newUpdate(c, otherUser);
update.setPatchSetId(c.currentPatchSetId());
- update.putComment(Comment.Status.DRAFT, comment(c.currentPatchSetId()));
+ update.putComment(HumanComment.Status.DRAFT, comment(c.currentPatchSetId()));
update.commit();
ChangeNotes notes = newNotes(c);
@@ -80,7 +80,7 @@
assertableFanOutExecutor.assertInteractions(0);
}
- private Comment comment(PatchSet.Id psId) {
+ private HumanComment comment(PatchSet.Id psId) {
return newComment(
psId,
"filename",
diff --git a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
index 11e98c6..dbcac0d 100644
--- a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
+++ b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
@@ -59,8 +59,8 @@
import com.google.gerrit.entities.RefNames;
import com.google.gerrit.extensions.api.GerritApi;
import com.google.gerrit.extensions.api.changes.AddReviewerInput;
-import com.google.gerrit.extensions.api.changes.AddToAttentionSetInput;
import com.google.gerrit.extensions.api.changes.AssigneeInput;
+import com.google.gerrit.extensions.api.changes.AttentionSetInput;
import com.google.gerrit.extensions.api.changes.ChangeApi;
import com.google.gerrit.extensions.api.changes.Changes.QueryRequest;
import com.google.gerrit.extensions.api.changes.DraftInput;
@@ -3016,7 +3016,7 @@
Change change1 = insert(repo, newChange(repo));
Change change2 = insert(repo, newChange(repo));
- AddToAttentionSetInput input = new AddToAttentionSetInput(userId.toString(), "some reason");
+ AttentionSetInput input = new AttentionSetInput(userId.toString(), "some reason");
gApi.changes().id(change1.getChangeId()).addToAttentionSet(input);
assertQuery("attention:" + user.getUserName().get(), change1);
@@ -3029,11 +3029,11 @@
TestRepository<Repo> repo = createProject("repo");
Change change = insert(repo, newChange(repo));
- AddToAttentionSetInput input = new AddToAttentionSetInput(userId.toString(), "reason 1");
+ AttentionSetInput input = new AttentionSetInput(userId.toString(), "reason 1");
gApi.changes().id(change.getChangeId()).addToAttentionSet(input);
Account.Id user2Id =
accountManager.authenticate(AuthRequest.forUser("anotheruser")).getAccountId();
- input = new AddToAttentionSetInput(user2Id.toString(), "reason 2");
+ input = new AttentionSetInput(user2Id.toString(), "reason 2");
gApi.changes().id(change.getChangeId()).addToAttentionSet(input);
List<ChangeInfo> result = newQuery("attention:" + user2Id.toString()).get();
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.js b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.js
index a3fe990..172e444 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.js
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.js
@@ -105,6 +105,12 @@
});
}
+ _handleContentClick(e) {
+ e.preventDefault();
+ e.stopPropagation();
+ GerritNav.navigateToChange(this.change);
+ }
+
_computeChangeURL(change) {
return GerritNav.getUrlForChange(change);
}
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_html.js b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_html.js
index 4e826aa..36f84f7 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_html.js
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_html.js
@@ -44,6 +44,7 @@
text-overflow: ellipsis;
white-space: nowrap;
width: 100%;
+ cursor: pointer;
}
.comments,
.reviewers {
@@ -122,7 +123,7 @@
hidden$="[[isColumnHidden('Subject', visibleChangeTableColumns)]]"
>
<div class="container">
- <div class="content">
+ <div class="content" on-click="_handleContentClick">
<a title$="[[change.subject]]" href$="[[changeURL]]">
[[change.subject]]
</a>
diff --git a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list-experimental_html.js b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list-experimental_html.js
index 56cb960..6d3a064 100644
--- a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list-experimental_html.js
+++ b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list-experimental_html.js
@@ -84,16 +84,6 @@
<span class="transparent separator"></span>
</template>
</div>
- <div class="experimentMessage">
- <iron-icon icon="gr-icons:pets"></iron-icon>
- <span>You're currently viewing an experimental Change Log view.</span>
- <a
- target="_blank"
- href="https://www.gerritcodereview.com/2020-05-06-change-log-experiment.html"
- >
- Learn more
- </a>
- </div>
<gr-button
id="collapse-messages"
link=""
diff --git a/polygerrit-ui/app/elements/custom-dark-theme_test.html b/polygerrit-ui/app/elements/custom-dark-theme_test.html
deleted file mode 100644
index 6ac9c20..0000000
--- a/polygerrit-ui/app/elements/custom-dark-theme_test.html
+++ /dev/null
@@ -1,103 +0,0 @@
-<!DOCTYPE html>
-<!--
-@license
-Copyright (C) 2017 The Android Open Source Project
-
-Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file except in compliance with the License.
-You may obtain a copy of the License at
-
-http://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing, software
-distributed under the License is distributed on an "AS IS" BASIS,
-WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-See the License for the specific language governing permissions and
-limitations under the License.
--->
-
-<meta name="viewport" content="width=device-width, minimum-scale=1.0, initial-scale=1.0, user-scalable=yes">
-<meta charset="utf-8">
-<title>gr-app-it_test</title>
-
-<script src="/node_modules/@webcomponents/webcomponentsjs/custom-elements-es5-adapter.js"></script>
-
-<script src="/node_modules/@webcomponents/webcomponentsjs/webcomponents-lite.js"></script>
-<script src="/components/wct-browser-legacy/browser.js"></script>
-
-<test-fixture id="element">
- <template>
- <gr-app id="app"></gr-app>
- </template>
-</test-fixture>
-
-<script type="module">
-import '../test/common-test-setup.js';
-import './gr-app.js';
-import {util} from '../scripts/util.js';
-import {pluginLoader} from './shared/gr-js-api-interface/gr-plugin-loader.js';
-
-suite('gr-app custom dark theme tests', () => {
- let sandbox;
- let element;
-
- setup(done => {
- sandbox = sinon.sandbox.create();
- stub('gr-reporting', {
- appStarted: sandbox.stub(),
- });
- stub('gr-account-dropdown', {
- _getTopContent: sinon.stub(),
- });
- stub('gr-rest-api-interface', {
- getAccount() { return Promise.resolve(null); },
- getAccountCapabilities() { return Promise.resolve({}); },
- getConfig() {
- return Promise.resolve({
- plugin: {
- js_resource_paths: [],
- html_resource_paths: [
- new URL('test/plugin.html', window.location.href).toString(),
- ],
- },
- });
- },
- getVersion() { return Promise.resolve(42); },
- getLoggedIn() { return Promise.resolve(false); },
- });
-
- window.localStorage.setItem('dark-theme', 'true');
-
- element = fixture('element');
-
- const importSpy = sandbox.spy(
- element.$['app-element'].$.externalStyleForAll,
- '_import');
- const importForThemeSpy = sandbox.spy(
- element.$['app-element'].$.externalStyleForTheme,
- '_import');
- pluginLoader.awaitPluginsLoaded().then(() => {
- Promise.all(importSpy.returnValues.concat(importForThemeSpy.returnValues))
- .then(() => {
- flush(done);
- });
- });
- });
-
- teardown(() => {
- sandbox.restore();
- });
-
- test('applies the right theme', () => {
- assert.equal(
- util.getComputedStyleValue('--primary-text-color', element),
- 'red');
- assert.equal(
- util.getComputedStyleValue('--header-background-color', element),
- 'black');
- assert.equal(
- util.getComputedStyleValue('--footer-background-color', element),
- 'yellow');
- });
-});
-</script>
diff --git a/polygerrit-ui/app/elements/custom-dark-theme_test.js b/polygerrit-ui/app/elements/custom-dark-theme_test.js
new file mode 100644
index 0000000..31c78a1
--- /dev/null
+++ b/polygerrit-ui/app/elements/custom-dark-theme_test.js
@@ -0,0 +1,47 @@
+/**
+ * @license
+ * Copyright (C) 2020 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import '../test/common-test-setup-karma.js';
+import './shared/gr-rest-api-interface/gr-rest-api-interface.js';
+import './gr-app.js';
+import {pluginLoader} from './shared/gr-js-api-interface/gr-plugin-loader.js';
+const basicFixture = fixtureFromElement('gr-app');
+suite('gr-app custom dark theme tests', () => {
+ let sandbox;
+ setup(done => {
+ window.localStorage.setItem('dark-theme', 'true');
+ sandbox = sinon.sandbox.create();
+ basicFixture.instantiate();
+ pluginLoader.loadPlugins([]);
+ pluginLoader.awaitPluginsLoaded().then(() => flush(done));
+ });
+ teardown(() => {
+ sandbox.restore();
+ window.localStorage.removeItem('dark-theme');
+ const addedTheme = document
+ .querySelector('link[href="/styles/themes/dark-theme.html"]');
+ if (addedTheme) {
+ addedTheme.remove();
+ }
+ });
+
+ test('should tried to load dark theme', () => {
+ assert.isTrue(
+ !!document.querySelector('link[href="/styles/themes/dark-theme.html"]')
+ );
+ });
+});
\ No newline at end of file
diff --git a/polygerrit-ui/app/elements/custom-light-theme_test.html b/polygerrit-ui/app/elements/custom-light-theme_test.html
deleted file mode 100644
index f8a749c..0000000
--- a/polygerrit-ui/app/elements/custom-light-theme_test.html
+++ /dev/null
@@ -1,103 +0,0 @@
-<!DOCTYPE html>
-<!--
-@license
-Copyright (C) 2017 The Android Open Source Project
-
-Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file except in compliance with the License.
-You may obtain a copy of the License at
-
-http://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing, software
-distributed under the License is distributed on an "AS IS" BASIS,
-WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-See the License for the specific language governing permissions and
-limitations under the License.
--->
-
-<meta name="viewport" content="width=device-width, minimum-scale=1.0, initial-scale=1.0, user-scalable=yes">
-<meta charset="utf-8">
-<title>gr-app-it_test</title>
-
-<script src="/node_modules/@webcomponents/webcomponentsjs/custom-elements-es5-adapter.js"></script>
-
-<script src="/node_modules/@webcomponents/webcomponentsjs/webcomponents-lite.js"></script>
-<script src="/components/wct-browser-legacy/browser.js"></script>
-
-<test-fixture id="element">
- <template>
- <gr-app id="app"></gr-app>
- </template>
-</test-fixture>
-
-<script type="module">
-import '../test/common-test-setup.js';
-import './gr-app.js';
-import {util} from '../scripts/util.js';
-import {pluginLoader} from './shared/gr-js-api-interface/gr-plugin-loader.js';
-
-suite('gr-app custom light theme tests', () => {
- let sandbox;
- let element;
-
- setup(done => {
- sandbox = sinon.sandbox.create();
- stub('gr-reporting', {
- appStarted: sandbox.stub(),
- });
- stub('gr-account-dropdown', {
- _getTopContent: sinon.stub(),
- });
- stub('gr-rest-api-interface', {
- getAccount() { return Promise.resolve(null); },
- getAccountCapabilities() { return Promise.resolve({}); },
- getConfig() {
- return Promise.resolve({
- plugin: {
- js_resource_paths: [],
- html_resource_paths: [
- new URL('test/plugin.html', window.location.href).toString(),
- ],
- },
- });
- },
- getVersion() { return Promise.resolve(42); },
- getLoggedIn() { return Promise.resolve(false); },
- });
-
- window.localStorage.removeItem('dark-theme');
-
- element = fixture('element');
-
- const importSpy = sandbox.spy(
- element.$['app-element'].$.externalStyleForAll,
- '_import');
- const importForThemeSpy = sandbox.spy(
- element.$['app-element'].$.externalStyleForTheme,
- '_import');
- pluginLoader.awaitPluginsLoaded().then(() => {
- Promise.all(importSpy.returnValues.concat(importForThemeSpy.returnValues))
- .then(() => {
- flush(done);
- });
- });
- });
-
- teardown(() => {
- sandbox.restore();
- });
-
- test('applies the right theme', () => {
- assert.equal(
- util.getComputedStyleValue('--primary-text-color', element),
- '#F00BAA');
- assert.equal(
- util.getComputedStyleValue('--header-background-color', element),
- '#F01BAA');
- assert.equal(
- util.getComputedStyleValue('--footer-background-color', element),
- '#F02BAA');
- });
-});
-</script>
diff --git a/polygerrit-ui/app/elements/custom-light-theme_test.js b/polygerrit-ui/app/elements/custom-light-theme_test.js
new file mode 100644
index 0000000..fa1f93f
--- /dev/null
+++ b/polygerrit-ui/app/elements/custom-light-theme_test.js
@@ -0,0 +1,67 @@
+/**
+ * @license
+ * Copyright (C) 2020 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import '../test/common-test-setup-karma.js';
+import {getComputedStyleValue} from '../utils/dom-util.js';
+import './shared/gr-rest-api-interface/gr-rest-api-interface.js';
+import './gr-app.js';
+import {pluginLoader} from './shared/gr-js-api-interface/gr-plugin-loader.js';
+const basicFixture = fixtureFromElement('gr-app');
+
+suite('gr-app custom light theme tests', () => {
+ let sandbox;
+ let element;
+ setup(done => {
+ sandbox = sinon.sandbox.create();
+ window.localStorage.removeItem('dark-theme');
+ stub('gr-rest-api-interface', {
+ getConfig() { return Promise.resolve({test: 'config'}); },
+ getAccount() { return Promise.resolve({}); },
+ getDiffComments() { return Promise.resolve({}); },
+ getDiffRobotComments() { return Promise.resolve({}); },
+ getDiffDrafts() { return Promise.resolve({}); },
+ _fetchSharedCacheURL() { return Promise.resolve({}); },
+ });
+ element = basicFixture.instantiate();
+ pluginLoader.loadPlugins([]);
+ pluginLoader.awaitPluginsLoaded().then(() => flush(done));
+ });
+ teardown(() => {
+ sandbox.restore();
+ });
+
+ test('should not load dark theme', () => {
+ assert.isFalse(
+ !!document.querySelector('link[href="/styles/themes/dark-theme.html"]')
+ );
+ });
+
+ test('applies the right theme', () => {
+ assert.equal(
+ getComputedStyleValue('--primary-text-color', element)
+ .toLowerCase(),
+ 'black');
+ assert.equal(
+ getComputedStyleValue('--header-background-color', element)
+ .toLowerCase(),
+ '#f1f3f4');
+ assert.equal(
+ getComputedStyleValue('--footer-background-color', element)
+ .toLowerCase(),
+ 'transparent');
+ });
+});
\ No newline at end of file
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.js b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.js
index acb8f0c..9d68ba3 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.js
@@ -202,9 +202,10 @@
}
}
- moveToNextChunk(opt_clipToTop) {
+ moveToNextChunk(opt_clipToTop, opt_navigateToNextFile) {
this.$.cursorManager.next(this._isFirstRowOfChunk.bind(this),
- target => target.parentNode.scrollHeight, opt_clipToTop);
+ target => target.parentNode.scrollHeight, opt_clipToTop,
+ opt_navigateToNextFile);
this._fixSide();
}
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 71fc341..440b233 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
@@ -244,6 +244,21 @@
assert.equal(cursorElement.side, 'left');
});
+ test('navigate to next unreviewed file via moveToNextChunk', () => {
+ const cursor = cursorElement.shadowRoot.querySelector('#cursorManager');
+ cursor.index = cursor.stops.length - 1;
+ const dispatchEventStub = sandbox.stub(cursor, 'dispatchEvent');
+ cursorElement.moveToNextChunk(/* opt_clipToTop = */false,
+ /* opt_navigateToNextFile = */true);
+ assert.isTrue(dispatchEventStub.called);
+ assert.equal(dispatchEventStub.getCall(0).args[0].type, 'show-alert');
+
+ cursorElement.moveToNextChunk(/* opt_clipToTop = */false,
+ /* opt_navigateToNextFile = */true);
+ assert.equal(dispatchEventStub.getCall(1).args[0].type,
+ 'navigate-to-next-unreviewed-file');
+ });
+
test('initialLineNumber not provided', done => {
let scrollBehaviorDuringMove;
const moveToNumStub = sandbox.stub(cursorElement, 'moveToLineNumber');
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 19abdaf..cf89c63 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
@@ -556,7 +556,8 @@
this.$.cursor.moveToNextCommentThread();
} else {
if (this.modifierPressed(e)) { return; }
- this.$.cursor.moveToNextChunk();
+ this.$.cursor.moveToNextChunk(/* opt_clipToTop = */false,
+ /* opt_navigateToNextFile = */true);
}
}
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_html.js b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_html.js
index 3d30f77..2b7d363 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_html.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_html.js
@@ -422,6 +422,7 @@
<gr-diff-cursor
id="cursor"
scroll-top-margin="[[_scrollTopMargin]]"
+ on-navigate-to-next-unreviewed-file="_handleNextUnreviewedFile"
></gr-diff-cursor>
<gr-comment-api id="commentAPI"></gr-comment-api>
`;
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 0942646..142143d 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
@@ -828,6 +828,15 @@
const commentSide = threadEl.getAttribute('comment-side');
const lineEl = this.$.diffBuilder.getLineElByNumber(
lineNumString, commentSide);
+ // When the line the comment refers to does not exist, log an error
+ // but don't crash. This can happen e.g. if the API does not fully
+ // validate e.g. (robot) comments
+ if (lineEl == undefined) {
+ console.error(
+ 'thread attached to line ', commentSide, lineNumString,
+ ' which does not exist.');
+ continue;
+ }
const contentEl = this.$.diffBuilder.getContentTdByLineEl(lineEl);
const threadGroupEl = this._getOrCreateThreadGroup(
contentEl, commentSide);
diff --git a/polygerrit-ui/app/elements/plugins/gr-plugin-host/gr-plugin-host.js b/polygerrit-ui/app/elements/plugins/gr-plugin-host/gr-plugin-host.js
index 1833efa..46e37e1 100644
--- a/polygerrit-ui/app/elements/plugins/gr-plugin-host/gr-plugin-host.js
+++ b/polygerrit-ui/app/elements/plugins/gr-plugin-host/gr-plugin-host.js
@@ -37,9 +37,10 @@
_configChanged(config) {
const plugins = config.plugin;
- const htmlPlugins = (plugins.html_resource_paths || []);
- const jsPlugins =
- this._handleMigrations(plugins.js_resource_paths || [], htmlPlugins);
+ const htmlPlugins = (plugins && plugins.html_resource_paths || []);
+ const jsPlugins = this._handleMigrations(
+ plugins && plugins.js_resource_paths || [], htmlPlugins
+ );
const shouldLoadTheme = config.default_theme &&
!pluginLoader.isPluginPreloaded('preloaded:gerrit-theme');
const themeToLoad =
diff --git a/polygerrit-ui/app/elements/shared/gr-cursor-manager/gr-cursor-manager.js b/polygerrit-ui/app/elements/shared/gr-cursor-manager/gr-cursor-manager.js
index 647f41b..4bb79c9 100644
--- a/polygerrit-ui/app/elements/shared/gr-cursor-manager/gr-cursor-manager.js
+++ b/polygerrit-ui/app/elements/shared/gr-cursor-manager/gr-cursor-manager.js
@@ -119,11 +119,15 @@
* sometimes different, used by the diff cursor.
* @param {boolean=} opt_clipToTop When none of the next indices match, move
* back to first instead of to last.
+ * @param {boolean=} opt_navigateToNextFile Navigate to next unreviewed file
+ * if user presses next on the last diff chunk
* @private
*/
- next(opt_condition, opt_getTargetHeight, opt_clipToTop) {
- this._moveCursor(1, opt_condition, opt_getTargetHeight, opt_clipToTop);
+ next(opt_condition, opt_getTargetHeight, opt_clipToTop,
+ opt_navigateToNextFile) {
+ this._moveCursor(1, opt_condition, opt_getTargetHeight, opt_clipToTop,
+ opt_navigateToNextFile);
}
previous(opt_condition) {
@@ -265,9 +269,12 @@
* sometimes different, used by the diff cursor.
* @param {boolean=} opt_clipToTop When none of the next indices match, move
* back to first instead of to last.
+ * @param {boolean=} opt_navigateToNextFile Navigate to next unreviewed file
+ * if user presses next on the last diff chunk
* @private
*/
- _moveCursor(delta, opt_condition, opt_getTargetHeight, opt_clipToTop) {
+ _moveCursor(delta, opt_condition, opt_getTargetHeight, opt_clipToTop,
+ opt_navigateToNextFile) {
if (!this.stops.length) {
this.unsetCursor();
return;
@@ -282,6 +289,32 @@
newTarget = this.stops[newIndex];
}
+ /*
+ * If user presses n on the last diff chunk, show a toast informing user
+ * that pressing n again will navigate them to next unreviewed file
+ */
+ if (opt_navigateToNextFile && this.index === newIndex) {
+ if (newIndex === this.stops.length - 1) {
+ if (this._displayedNavigateToNextFileToast) {
+ // reset for next file
+ this._displayedNavigateToNextFileToast = false;
+ this.dispatchEvent(new CustomEvent(
+ 'navigate-to-next-unreviewed-file', {
+ composed: true, bubbles: true,
+ }));
+ return;
+ }
+ this._displayedNavigateToNextFileToast = true;
+ this.dispatchEvent(new CustomEvent('show-alert', {
+ detail: {
+ message: 'Press n again to navigate to next unreviewed file',
+ },
+ composed: true, bubbles: true,
+ }));
+ return;
+ }
+ }
+
this.index = newIndex;
this.target = newTarget;
