Merge "Fix ba-linkify on emails with url-like names"
diff --git a/java/com/google/gerrit/acceptance/AbstractDaemonTest.java b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
index ca105f6..cfee970 100644
--- a/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
+++ b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
@@ -1282,7 +1282,7 @@
}
protected void assertNotifyCc(TestAccount expected) {
- assertNotifyCc(expected.getEmailAddress());
+ assertNotifyCc(expected.getNameEmail());
}
protected void assertNotifyCc(String expectedEmail, String expectedFullname) {
@@ -1302,7 +1302,7 @@
protected void assertNotifyBcc(TestAccount expected) {
assertThat(sender.getMessages()).hasSize(1);
Message m = sender.getMessages().get(0);
- assertThat(m.rcpt()).containsExactly(expected.getEmailAddress());
+ assertThat(m.rcpt()).containsExactly(expected.getNameEmail());
assertThat(m.headers().get("To").isEmpty()).isTrue();
assertThat(m.headers().get("Cc").isEmpty()).isTrue();
}
diff --git a/java/com/google/gerrit/acceptance/TestAccount.java b/java/com/google/gerrit/acceptance/TestAccount.java
index 07bb739..8b39c9e 100644
--- a/java/com/google/gerrit/acceptance/TestAccount.java
+++ b/java/com/google/gerrit/acceptance/TestAccount.java
@@ -79,7 +79,7 @@
.toString();
}
- public Address getEmailAddress() {
+ public Address getNameEmail() {
// Address is weird enough that it's safer and clearer to create a new instance in a
// non-abstract method rather than, say, having an abstract emailAddress() as part of this
// AutoValue class. Specifically:
diff --git a/java/com/google/gerrit/entities/AttentionStatus.java b/java/com/google/gerrit/entities/AttentionStatus.java
new file mode 100644
index 0000000..c488ccd
--- /dev/null
+++ b/java/com/google/gerrit/entities/AttentionStatus.java
@@ -0,0 +1,72 @@
+// 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 com.google.auto.value.AutoValue;
+import com.google.gerrit.common.Nullable;
+import java.time.Instant;
+
+/**
+ * A single update to the attention set. To reconstruct the attention set these instances are parsed
+ * in reverse chronological order. Since each update contains all required information and
+ * invalidates all previous state (hence the name -Status rather than -Update), only the most recent
+ * record is relevant for each user.
+ *
+ * <p>See <a href="https://www.gerritcodereview.com/design-docs/attention-set.html">here</a> for
+ * details.
+ */
+@AutoValue
+public abstract class AttentionStatus {
+
+ /** Users can be added to or removed from the attention set. */
+ public enum Operation {
+ ADD,
+ REMOVE
+ }
+
+ /**
+ * The time at which this status was set. This is null for instances to be written because the
+ * timestamp in the commit message will be used.
+ */
+ @Nullable
+ public abstract Instant timestamp();
+
+ /** The user included in or excluded from the attention set. */
+ public abstract Account.Id account();
+
+ /** Indicates whether the user is added to or removed from the attention set. */
+ public abstract Operation operation();
+
+ /** A short human readable reason that explains this status (e.g. "manual"). */
+ public abstract String reason();
+
+ /**
+ * Create an instance from data read from NoteDB. This includes the timestamp taken from the
+ * commit.
+ */
+ public static AttentionStatus createFromRead(
+ Instant timestamp, Account.Id account, Operation operation, String reason) {
+ return new AutoValue_AttentionStatus(timestamp, account, operation, reason);
+ }
+
+ /**
+ * Create an instance to be written to NoteDB. This has no timestamp because the timestamp of the
+ * commit will be used.
+ */
+ public static AttentionStatus createForWrite(
+ Account.Id account, Operation operation, String reason) {
+ return new AutoValue_AttentionStatus(null, account, operation, reason);
+ }
+}
diff --git a/java/com/google/gerrit/mail/Address.java b/java/com/google/gerrit/mail/Address.java
index 24ab353..455cc90 100644
--- a/java/com/google/gerrit/mail/Address.java
+++ b/java/com/google/gerrit/mail/Address.java
@@ -16,6 +16,7 @@
import com.google.gerrit.common.Nullable;
+/** Represents an address (name + email) in an email message. */
public class Address {
public static Address parse(String in) {
final int lt = in.indexOf('<');
diff --git a/java/com/google/gerrit/pgm/http/jetty/BUILD b/java/com/google/gerrit/pgm/http/jetty/BUILD
index 6ceb242..cd188f5 100644
--- a/java/com/google/gerrit/pgm/http/jetty/BUILD
+++ b/java/com/google/gerrit/pgm/http/jetty/BUILD
@@ -10,10 +10,10 @@
"//java/com/google/gerrit/lifecycle",
"//java/com/google/gerrit/metrics",
"//java/com/google/gerrit/server",
- "//java/com/google/gerrit/server/logging",
"//java/com/google/gerrit/server/util/time",
"//java/com/google/gerrit/sshd",
"//java/com/google/gerrit/util/http",
+ "//java/com/google/gerrit/util/logging",
"//lib:gson",
"//lib:guava",
"//lib:jgit",
diff --git a/java/com/google/gerrit/pgm/http/jetty/HttpLogJsonLayout.java b/java/com/google/gerrit/pgm/http/jetty/HttpLogJsonLayout.java
index 33916ac..73d9ee4 100644
--- a/java/com/google/gerrit/pgm/http/jetty/HttpLogJsonLayout.java
+++ b/java/com/google/gerrit/pgm/http/jetty/HttpLogJsonLayout.java
@@ -24,8 +24,8 @@
import static com.google.gerrit.pgm.http.jetty.HttpLog.P_USER;
import static com.google.gerrit.pgm.http.jetty.HttpLog.P_USER_AGENT;
-import com.google.gerrit.server.logging.JsonLayout;
-import com.google.gerrit.server.logging.JsonLogEntry;
+import com.google.gerrit.util.logging.JsonLayout;
+import com.google.gerrit.util.logging.JsonLogEntry;
import java.time.format.DateTimeFormatter;
import org.apache.log4j.spi.LoggingEvent;
diff --git a/java/com/google/gerrit/server/git/MergeUtil.java b/java/com/google/gerrit/server/git/MergeUtil.java
index 39fd103..89a2036 100644
--- a/java/com/google/gerrit/server/git/MergeUtil.java
+++ b/java/com/google/gerrit/server/git/MergeUtil.java
@@ -21,7 +21,6 @@
import static com.google.gerrit.git.ObjectIds.abbreviateName;
import static java.nio.charset.StandardCharsets.UTF_8;
import static java.util.Comparator.naturalOrder;
-import static java.util.Objects.requireNonNull;
import static java.util.stream.Collectors.joining;
import com.google.common.base.Strings;
@@ -44,8 +43,6 @@
import com.google.gerrit.exceptions.MergeWithConflictsNotSupportedException;
import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.extensions.registration.DynamicItem;
-import com.google.gerrit.extensions.registration.DynamicSet;
-import com.google.gerrit.extensions.registration.Extension;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.MergeConflictException;
import com.google.gerrit.extensions.restapi.MethodNotAllowedException;
@@ -62,7 +59,6 @@
import com.google.gerrit.server.submit.IntegrationException;
import com.google.gerrit.server.submit.MergeIdenticalTreeException;
import com.google.gerrit.server.submit.MergeSorter;
-import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
import com.google.inject.assistedinject.AssistedInject;
import java.io.IOException;
@@ -128,44 +124,6 @@
*/
private static final int NAME_ABBREV_LEN = 6;
- static class PluggableCommitMessageGenerator {
- private final DynamicSet<ChangeMessageModifier> changeMessageModifiers;
-
- @Inject
- PluggableCommitMessageGenerator(DynamicSet<ChangeMessageModifier> changeMessageModifiers) {
- this.changeMessageModifiers = changeMessageModifiers;
- }
-
- public String generate(
- RevCommit original, RevCommit mergeTip, BranchNameKey dest, String originalMessage) {
- requireNonNull(original.getRawBuffer());
- if (mergeTip != null) {
- requireNonNull(mergeTip.getRawBuffer());
- }
-
- int count = 0;
- String current = originalMessage;
- for (Extension<ChangeMessageModifier> ext : changeMessageModifiers.entries()) {
- ChangeMessageModifier changeMessageModifier = ext.get();
- String className = changeMessageModifier.getClass().getName();
- current = changeMessageModifier.onSubmit(current, original, mergeTip, dest);
- checkState(
- current != null,
- "%s.onSubmit from plugin %s returned null instead of new commit message",
- className,
- ext.getPluginName());
- count++;
- logger.atFine().log(
- "Invoked %s from plugin %s, message length now %d",
- className, ext.getPluginName(), current.length());
- }
- logger.atFine().log(
- "Invoked %d ChangeMessageModifiers on message with original length %d",
- count, originalMessage.length());
- return current;
- }
- }
-
private static final String R_HEADS_MASTER = Constants.R_HEADS + Constants.MASTER;
public static boolean useRecursiveMerge(Config cfg) {
@@ -432,7 +390,33 @@
RevCommit originalCommit,
String mergeStrategy,
boolean allowConflicts,
- PersonIdent committerIndent,
+ PersonIdent committerIdent,
+ String commitMsg,
+ CodeReviewRevWalk rw)
+ throws IOException, MergeIdenticalTreeException, MergeConflictException,
+ InvalidMergeStrategyException {
+ return createMergeCommit(
+ inserter,
+ repoConfig,
+ mergeTip,
+ originalCommit,
+ mergeStrategy,
+ allowConflicts,
+ committerIdent,
+ committerIdent,
+ commitMsg,
+ rw);
+ }
+
+ public static CodeReviewCommit createMergeCommit(
+ ObjectInserter inserter,
+ Config repoConfig,
+ RevCommit mergeTip,
+ RevCommit originalCommit,
+ String mergeStrategy,
+ boolean allowConflicts,
+ PersonIdent authorIdent,
+ PersonIdent committerIdent,
String commitMsg,
CodeReviewRevWalk rw)
throws IOException, MergeIdenticalTreeException, MergeConflictException,
@@ -496,8 +480,8 @@
CommitBuilder mergeCommit = new CommitBuilder();
mergeCommit.setTreeId(tree);
mergeCommit.setParentIds(mergeTip, originalCommit);
- mergeCommit.setAuthor(committerIndent);
- mergeCommit.setCommitter(committerIndent);
+ mergeCommit.setAuthor(authorIdent);
+ mergeCommit.setCommitter(committerIdent);
mergeCommit.setMessage(commitMsg);
CodeReviewCommit commit = rw.parseCommit(inserter.insert(mergeCommit));
commit.setFilesWithGitConflicts(filesWithGitConflicts);
diff --git a/java/com/google/gerrit/server/git/PluggableCommitMessageGenerator.java b/java/com/google/gerrit/server/git/PluggableCommitMessageGenerator.java
new file mode 100644
index 0000000..804a218
--- /dev/null
+++ b/java/com/google/gerrit/server/git/PluggableCommitMessageGenerator.java
@@ -0,0 +1,71 @@
+// 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.git;
+
+import static com.google.common.base.Preconditions.checkState;
+import static java.util.Objects.requireNonNull;
+
+import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.entities.BranchNameKey;
+import com.google.gerrit.extensions.registration.DynamicSet;
+import com.google.gerrit.extensions.registration.Extension;
+import com.google.inject.Inject;
+import org.eclipse.jgit.revwalk.RevCommit;
+
+/** Helper to call plugins that want to change the commit message before a change is merged. */
+public class PluggableCommitMessageGenerator {
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+
+ private final DynamicSet<ChangeMessageModifier> changeMessageModifiers;
+
+ @Inject
+ PluggableCommitMessageGenerator(DynamicSet<ChangeMessageModifier> changeMessageModifiers) {
+ this.changeMessageModifiers = changeMessageModifiers;
+ }
+
+ /**
+ * Returns the commit message as modified by plugins. The returned message can be equal to {@code
+ * originalMessage} in case no plugins are registered or the registered plugins decided not to
+ * modify the message.
+ */
+ public String generate(
+ RevCommit original, RevCommit mergeTip, BranchNameKey dest, String originalMessage) {
+ requireNonNull(original.getRawBuffer());
+ if (mergeTip != null) {
+ requireNonNull(mergeTip.getRawBuffer());
+ }
+
+ int count = 0;
+ String current = originalMessage;
+ for (Extension<ChangeMessageModifier> ext : changeMessageModifiers.entries()) {
+ ChangeMessageModifier changeMessageModifier = ext.get();
+ String className = changeMessageModifier.getClass().getName();
+ current = changeMessageModifier.onSubmit(current, original, mergeTip, dest);
+ checkState(
+ current != null,
+ "%s.onSubmit from plugin %s returned null instead of new commit message",
+ className,
+ ext.getPluginName());
+ count++;
+ logger.atFine().log(
+ "Invoked %s from plugin %s, message length now %d",
+ className, ext.getPluginName(), current.length());
+ }
+ logger.atFine().log(
+ "Invoked %d ChangeMessageModifiers on message with original length %d",
+ count, originalMessage.length());
+ return current;
+ }
+}
diff --git a/java/com/google/gerrit/server/notedb/AbstractChangeUpdate.java b/java/com/google/gerrit/server/notedb/AbstractChangeUpdate.java
index 1f0c58a..7136d2b 100644
--- a/java/com/google/gerrit/server/notedb/AbstractChangeUpdate.java
+++ b/java/com/google/gerrit/server/notedb/AbstractChangeUpdate.java
@@ -110,7 +110,7 @@
ChangeNoteUtil noteUtil, PersonIdent serverIdent, CurrentUser u, Date when) {
checkUserType(u);
if (u instanceof IdentifiedUser) {
- return noteUtil.newIdent(u.asIdentifiedUser().getAccount(), when, serverIdent);
+ return noteUtil.newIdent(u.asIdentifiedUser().getAccount().id(), when, serverIdent);
} else if (u instanceof InternalUser) {
return serverIdent;
}
@@ -164,10 +164,6 @@
return accountId;
}
- protected PersonIdent newIdent(Account.Id authorId, Date when) {
- return noteUtil.newIdent(authorId, when, serverIdent);
- }
-
/** Whether no updates have been done. */
public abstract boolean isEmpty();
diff --git a/java/com/google/gerrit/server/notedb/ChangeNoteUtil.java b/java/com/google/gerrit/server/notedb/ChangeNoteUtil.java
index cebb67d..287f3e7 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNoteUtil.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNoteUtil.java
@@ -15,10 +15,13 @@
package com.google.gerrit.server.notedb;
import com.google.auto.value.AutoValue;
-import com.google.common.annotations.VisibleForTesting;
import com.google.gerrit.entities.Account;
+import com.google.gerrit.entities.AttentionStatus;
+import com.google.gerrit.json.OutputFormat;
import com.google.gerrit.server.config.GerritServerId;
+import com.google.gson.Gson;
import com.google.inject.Inject;
+import java.time.Instant;
import java.util.Date;
import java.util.Optional;
import org.eclipse.jgit.lib.PersonIdent;
@@ -27,42 +30,31 @@
import org.eclipse.jgit.util.RawParseUtils;
public class ChangeNoteUtil {
- public static final FooterKey FOOTER_ASSIGNEE = new FooterKey("Assignee");
- public static final FooterKey FOOTER_BRANCH = new FooterKey("Branch");
- public static final FooterKey FOOTER_CHANGE_ID = new FooterKey("Change-id");
- public static final FooterKey FOOTER_COMMIT = new FooterKey("Commit");
- public static final FooterKey FOOTER_CURRENT = new FooterKey("Current");
- public static final FooterKey FOOTER_GROUPS = new FooterKey("Groups");
- public static final FooterKey FOOTER_HASHTAGS = new FooterKey("Hashtags");
- public static final FooterKey FOOTER_LABEL = new FooterKey("Label");
- public static final FooterKey FOOTER_PATCH_SET = new FooterKey("Patch-set");
- public static final FooterKey FOOTER_PATCH_SET_DESCRIPTION =
- new FooterKey("Patch-set-description");
- public static final FooterKey FOOTER_PRIVATE = new FooterKey("Private");
- public static final FooterKey FOOTER_REAL_USER = new FooterKey("Real-user");
- public static final FooterKey FOOTER_STATUS = new FooterKey("Status");
- public static final FooterKey FOOTER_SUBJECT = new FooterKey("Subject");
- public static final FooterKey FOOTER_SUBMISSION_ID = new FooterKey("Submission-id");
- public static final FooterKey FOOTER_SUBMITTED_WITH = new FooterKey("Submitted-with");
- public static final FooterKey FOOTER_TOPIC = new FooterKey("Topic");
- public static final FooterKey FOOTER_TAG = new FooterKey("Tag");
- public static final FooterKey FOOTER_WORK_IN_PROGRESS = new FooterKey("Work-in-progress");
- public static final FooterKey FOOTER_REVERT_OF = new FooterKey("Revert-of");
- public static final FooterKey FOOTER_CHERRY_PICK_OF = new FooterKey("Cherry-pick-of");
- static final String AUTHOR = "Author";
- static final String BASE_PATCH_SET = "Base-for-patch-set";
- static final String COMMENT_RANGE = "Comment-range";
- static final String FILE = "File";
- static final String LENGTH = "Bytes";
- static final String PARENT = "Parent";
- static final String PARENT_NUMBER = "Parent-number";
- static final String PATCH_SET = "Patch-set";
- static final String REAL_AUTHOR = "Real-author";
- static final String REVISION = "Revision";
- static final String UUID = "UUID";
- static final String UNRESOLVED = "Unresolved";
- static final String TAG = FOOTER_TAG.getName();
+ static final FooterKey FOOTER_ATTENTION = new FooterKey("Attention");
+ static final FooterKey FOOTER_ASSIGNEE = new FooterKey("Assignee");
+ static final FooterKey FOOTER_BRANCH = new FooterKey("Branch");
+ static final FooterKey FOOTER_CHANGE_ID = new FooterKey("Change-id");
+ static final FooterKey FOOTER_COMMIT = new FooterKey("Commit");
+ static final FooterKey FOOTER_CURRENT = new FooterKey("Current");
+ static final FooterKey FOOTER_GROUPS = new FooterKey("Groups");
+ static final FooterKey FOOTER_HASHTAGS = new FooterKey("Hashtags");
+ static final FooterKey FOOTER_LABEL = new FooterKey("Label");
+ static final FooterKey FOOTER_PATCH_SET = new FooterKey("Patch-set");
+ static final FooterKey FOOTER_PATCH_SET_DESCRIPTION = new FooterKey("Patch-set-description");
+ static final FooterKey FOOTER_PRIVATE = new FooterKey("Private");
+ static final FooterKey FOOTER_REAL_USER = new FooterKey("Real-user");
+ static final FooterKey FOOTER_STATUS = new FooterKey("Status");
+ static final FooterKey FOOTER_SUBJECT = new FooterKey("Subject");
+ static final FooterKey FOOTER_SUBMISSION_ID = new FooterKey("Submission-id");
+ static final FooterKey FOOTER_SUBMITTED_WITH = new FooterKey("Submitted-with");
+ static final FooterKey FOOTER_TOPIC = new FooterKey("Topic");
+ static final FooterKey FOOTER_TAG = new FooterKey("Tag");
+ static final FooterKey FOOTER_WORK_IN_PROGRESS = new FooterKey("Work-in-progress");
+ static final FooterKey FOOTER_REVERT_OF = new FooterKey("Revert-of");
+ static final FooterKey FOOTER_CHERRY_PICK_OF = new FooterKey("Cherry-pick-of");
+
+ private static final Gson gson = OutputFormat.JSON_COMPACT.newGson();
private final ChangeNoteJson changeNoteJson;
private final String serverId;
@@ -77,21 +69,17 @@
return changeNoteJson;
}
- public PersonIdent newIdent(Account.Id authorId, Date when, PersonIdent serverIdent) {
+ public PersonIdent newIdent(Account.Id accountId, Date when, PersonIdent serverIdent) {
return new PersonIdent(
- "Gerrit User " + authorId.toString(),
- authorId.get() + "@" + serverId,
- when,
- serverIdent.getTimeZone());
+ getUsername(accountId), getEmailAddress(accountId), when, serverIdent.getTimeZone());
}
- @VisibleForTesting
- public PersonIdent newIdent(Account author, Date when, PersonIdent serverIdent) {
- return new PersonIdent(
- "Gerrit User " + author.id(),
- author.id().get() + "@" + serverId,
- when,
- serverIdent.getTimeZone());
+ private static String getUsername(Account.Id accountId) {
+ return "Gerrit User " + accountId.toString();
+ }
+
+ private String getEmailAddress(Account.Id accountId) {
+ return accountId.get() + "@" + serverId;
}
public static Optional<CommitMessageRange> parseCommitMessageRange(RevCommit commit) {
@@ -151,6 +139,7 @@
@AutoValue
public abstract static class CommitMessageRange {
+
public abstract int subjectStart();
public abstract int subjectEnd();
@@ -165,6 +154,7 @@
@AutoValue.Builder
public abstract static class Builder {
+
abstract Builder subjectStart(int subjectStart);
abstract Builder subjectEnd(int subjectEnd);
@@ -176,4 +166,51 @@
abstract CommitMessageRange build();
}
}
+
+ /** Helper class for JSON serialization. Timestamp is taken from the commit. */
+ private static class AttentionStatusInNoteDb {
+
+ final String personIdent;
+ final AttentionStatus.Operation operation;
+ final String reason;
+
+ AttentionStatusInNoteDb(
+ String personIndent, AttentionStatus.Operation operation, String reason) {
+ this.personIdent = personIndent;
+ this.operation = operation;
+ this.reason = reason;
+ }
+ }
+
+ /** The returned {@link Optional} holds the parsed entity or is empty if parsing failed. */
+ static Optional<AttentionStatus> attentionStatusFromJson(
+ Instant timestamp, String attentionString) {
+ AttentionStatusInNoteDb inNoteDb =
+ gson.fromJson(attentionString, AttentionStatusInNoteDb.class);
+ PersonIdent personIdent = RawParseUtils.parsePersonIdent(inNoteDb.personIdent);
+ if (personIdent == null) {
+ return Optional.empty();
+ }
+ Optional<Account.Id> account = NoteDbUtil.parseIdent(personIdent);
+ return account.map(
+ id -> AttentionStatus.createFromRead(timestamp, id, inNoteDb.operation, inNoteDb.reason));
+ }
+
+ String attentionStatusToJson(AttentionStatus attentionStatus) {
+ PersonIdent personIdent =
+ new PersonIdent(
+ getUsername(attentionStatus.account()), getEmailAddress(attentionStatus.account()));
+ StringBuilder stringBuilder = new StringBuilder();
+ appendIdentString(stringBuilder, personIdent.getName(), personIdent.getEmailAddress());
+ return gson.toJson(
+ new AttentionStatusInNoteDb(
+ stringBuilder.toString(), attentionStatus.operation(), attentionStatus.reason()));
+ }
+
+ static void appendIdentString(StringBuilder stringBuilder, String name, String emailAddress) {
+ PersonIdent.appendSanitized(stringBuilder, name);
+ stringBuilder.append(" <");
+ PersonIdent.appendSanitized(stringBuilder, emailAddress);
+ stringBuilder.append('>');
+ }
}
diff --git a/java/com/google/gerrit/server/notedb/ChangeNotes.java b/java/com/google/gerrit/server/notedb/ChangeNotes.java
index b7ce2a8..2de2195 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNotes.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNotes.java
@@ -40,6 +40,7 @@
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.data.SubmitRecord;
import com.google.gerrit.entities.Account;
+import com.google.gerrit.entities.AttentionStatus;
import com.google.gerrit.entities.BranchNameKey;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.ChangeMessage;
@@ -375,6 +376,10 @@
return state.reviewerUpdates();
}
+ public ImmutableList<AttentionStatus> getAttentionUpdates() {
+ return state.attentionUpdates();
+ }
+
/**
* @return an ImmutableSet of Account.Ids of all users that have been assigned to this change. The
* order of the set is the order in which they were assigned.
diff --git a/java/com/google/gerrit/server/notedb/ChangeNotesParser.java b/java/com/google/gerrit/server/notedb/ChangeNotesParser.java
index 428df16..ed6039f 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNotesParser.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNotesParser.java
@@ -16,6 +16,7 @@
import static com.google.common.base.MoreObjects.firstNonNull;
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_ASSIGNEE;
+import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_ATTENTION;
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_BRANCH;
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_CHANGE_ID;
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_CHERRY_PICK_OF;
@@ -43,6 +44,7 @@
import com.google.common.base.Enums;
import com.google.common.base.Splitter;
import com.google.common.collect.HashBasedTable;
+import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ImmutableTable;
import com.google.common.collect.ListMultimap;
@@ -57,6 +59,7 @@
import com.google.gerrit.common.data.LabelType;
import com.google.gerrit.common.data.SubmitRecord;
import com.google.gerrit.entities.Account;
+import com.google.gerrit.entities.AttentionStatus;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.ChangeMessage;
import com.google.gerrit.entities.Comment;
@@ -75,6 +78,7 @@
import java.io.IOException;
import java.nio.charset.Charset;
import java.sql.Timestamp;
+import java.time.Instant;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
@@ -102,7 +106,6 @@
// Private final members initialized in the constructor.
private final ChangeNoteJson changeNoteJson;
-
private final NoteDbMetrics metrics;
private final Change.Id id;
private final ObjectId tip;
@@ -114,6 +117,9 @@
private final Table<Address, ReviewerStateInternal, Timestamp> reviewersByEmail;
private final List<Account.Id> allPastReviewers;
private final List<ReviewerStatusUpdate> reviewerUpdates;
+ /** Holds only the most recent update per user. Older updates are discarded. */
+ private final Map<Account.Id, AttentionStatus> latestAttentionStatus;
+
private final List<AssigneeStatusUpdate> assigneeUpdates;
private final List<SubmitRecord> submitRecords;
private final ListMultimap<ObjectId, Comment> comments;
@@ -169,6 +175,7 @@
pendingReviewersByEmail = ReviewerByEmailSet.empty();
allPastReviewers = new ArrayList<>();
reviewerUpdates = new ArrayList<>();
+ latestAttentionStatus = new HashMap<>();
assigneeUpdates = new ArrayList<>();
submitRecords = Lists.newArrayListWithExpectedSize(1);
allChangeMessages = new ArrayList<>();
@@ -239,6 +246,7 @@
pendingReviewersByEmail,
allPastReviewers,
buildReviewerUpdates(),
+ ImmutableList.copyOf(latestAttentionStatus.values()),
assigneeUpdates,
submitRecords,
buildAllMessages(),
@@ -359,6 +367,7 @@
}
parseHashtags(commit);
+ parseAttentionUpdates(commit);
parseAssigneeUpdates(ts, commit);
if (submissionId == null) {
@@ -569,6 +578,21 @@
}
}
+ private void parseAttentionUpdates(ChangeNotesCommit commit) throws ConfigInvalidException {
+ List<String> attentionStrings = commit.getFooterLineValues(FOOTER_ATTENTION);
+ for (String attentionString : attentionStrings) {
+
+ Optional<AttentionStatus> attentionStatus =
+ ChangeNoteUtil.attentionStatusFromJson(
+ Instant.ofEpochSecond(commit.getCommitTime()), attentionString);
+ if (!attentionStatus.isPresent()) {
+ throw invalidFooter(FOOTER_ATTENTION, attentionString);
+ }
+ // Processing is in reverse chronological order. Keep only the latest update.
+ latestAttentionStatus.putIfAbsent(attentionStatus.get().account(), attentionStatus.get());
+ }
+ }
+
private void parseAssigneeUpdates(Timestamp ts, ChangeNotesCommit commit)
throws ConfigInvalidException {
String assigneeValue = parseOneFooter(commit, FOOTER_ASSIGNEE);
diff --git a/java/com/google/gerrit/server/notedb/ChangeNotesState.java b/java/com/google/gerrit/server/notedb/ChangeNotesState.java
index 064e43b..1931e7e 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNotesState.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNotesState.java
@@ -35,6 +35,7 @@
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.data.SubmitRecord;
import com.google.gerrit.entities.Account;
+import com.google.gerrit.entities.AttentionStatus;
import com.google.gerrit.entities.BranchNameKey;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.ChangeMessage;
@@ -55,6 +56,7 @@
import com.google.gerrit.server.ReviewerStatusUpdate;
import com.google.gerrit.server.cache.proto.Cache.ChangeNotesStateProto;
import com.google.gerrit.server.cache.proto.Cache.ChangeNotesStateProto.AssigneeStatusUpdateProto;
+import com.google.gerrit.server.cache.proto.Cache.ChangeNotesStateProto.AttentionStatusProto;
import com.google.gerrit.server.cache.proto.Cache.ChangeNotesStateProto.ChangeColumnsProto;
import com.google.gerrit.server.cache.proto.Cache.ChangeNotesStateProto.ReviewerByEmailSetEntryProto;
import com.google.gerrit.server.cache.proto.Cache.ChangeNotesStateProto.ReviewerSetEntryProto;
@@ -66,6 +68,7 @@
import com.google.protobuf.ByteString;
import com.google.protobuf.MessageLite;
import java.sql.Timestamp;
+import java.time.Instant;
import java.util.List;
import java.util.Map;
import java.util.Optional;
@@ -83,11 +86,12 @@
*/
@AutoValue
public abstract class ChangeNotesState {
+
static ChangeNotesState empty(Change change) {
return Builder.empty(change.getId()).build();
}
- static Builder builder() {
+ private static Builder builder() {
return new AutoValue_ChangeNotesState.Builder();
}
@@ -115,6 +119,7 @@
ReviewerByEmailSet pendingReviewersByEmail,
List<Account.Id> allPastReviewers,
List<ReviewerStatusUpdate> reviewerUpdates,
+ List<AttentionStatus> attentionStatusUpdates,
List<AssigneeStatusUpdate> assigneeUpdates,
List<SubmitRecord> submitRecords,
List<ChangeMessage> changeMessages,
@@ -165,6 +170,7 @@
.pendingReviewersByEmail(pendingReviewersByEmail)
.allPastReviewers(allPastReviewers)
.reviewerUpdates(reviewerUpdates)
+ .attentionUpdates(attentionStatusUpdates)
.assigneeUpdates(assigneeUpdates)
.submitRecords(submitRecords)
.changeMessages(changeMessages)
@@ -180,6 +186,7 @@
*/
@AutoValue
abstract static class ChangeColumns {
+
static Builder builder() {
return new AutoValue_ChangeNotesState_ChangeColumns.Builder();
}
@@ -229,6 +236,7 @@
@AutoValue.Builder
abstract static class Builder {
+
abstract Builder changeKey(Change.Key changeKey);
abstract Builder createdOn(Timestamp createdOn);
@@ -297,6 +305,8 @@
abstract ImmutableList<ReviewerStatusUpdate> reviewerUpdates();
+ abstract ImmutableList<AttentionStatus> attentionUpdates();
+
abstract ImmutableList<AssigneeStatusUpdate> assigneeUpdates();
abstract ImmutableList<SubmitRecord> submitRecords();
@@ -361,6 +371,7 @@
@AutoValue.Builder
abstract static class Builder {
+
static Builder empty(Change.Id changeId) {
return new AutoValue_ChangeNotesState.Builder()
.changeId(changeId)
@@ -373,6 +384,7 @@
.pendingReviewersByEmail(ReviewerByEmailSet.empty())
.allPastReviewers(ImmutableList.of())
.reviewerUpdates(ImmutableList.of())
+ .attentionUpdates(ImmutableList.of())
.assigneeUpdates(ImmutableList.of())
.submitRecords(ImmutableList.of())
.changeMessages(ImmutableList.of())
@@ -406,6 +418,8 @@
abstract Builder reviewerUpdates(List<ReviewerStatusUpdate> reviewerUpdates);
+ abstract Builder attentionUpdates(List<AttentionStatus> attentionUpdates);
+
abstract Builder assigneeUpdates(List<AssigneeStatusUpdate> assigneeUpdates);
abstract Builder submitRecords(List<SubmitRecord> submitRecords);
@@ -473,6 +487,7 @@
object.allPastReviewers().forEach(a -> b.addPastReviewer(a.get()));
object.reviewerUpdates().forEach(u -> b.addReviewerUpdate(toReviewerStatusUpdateProto(u)));
+ object.attentionUpdates().forEach(u -> b.addAttentionStatus(toAttentionStatusProto(u)));
object.assigneeUpdates().forEach(u -> b.addAssigneeUpdate(toAssigneeStatusUpdateProto(u)));
object
.submitRecords()
@@ -556,6 +571,15 @@
.build();
}
+ private static AttentionStatusProto toAttentionStatusProto(AttentionStatus attentionStatus) {
+ return AttentionStatusProto.newBuilder()
+ .setDate(attentionStatus.timestamp().toEpochMilli())
+ .setAccount(attentionStatus.account().get())
+ .setOperation(attentionStatus.operation().name())
+ .setReason(attentionStatus.reason())
+ .build();
+ }
+
private static AssigneeStatusUpdateProto toAssigneeStatusUpdateProto(AssigneeStatusUpdate u) {
AssigneeStatusUpdateProto.Builder builder =
AssigneeStatusUpdateProto.newBuilder()
@@ -596,6 +620,7 @@
.allPastReviewers(
proto.getPastReviewerList().stream().map(Account::id).collect(toImmutableList()))
.reviewerUpdates(toReviewerStatusUpdateList(proto.getReviewerUpdateList()))
+ .attentionUpdates(toAttentionUpdateList(proto.getAttentionStatusList()))
.assigneeUpdates(toAssigneeStatusUpdateList(proto.getAssigneeUpdateList()))
.submitRecords(
proto.getSubmitRecordList().stream()
@@ -694,6 +719,20 @@
return b.build();
}
+ private static ImmutableList<AttentionStatus> toAttentionUpdateList(
+ List<AttentionStatusProto> protos) {
+ ImmutableList.Builder<AttentionStatus> b = ImmutableList.builder();
+ for (AttentionStatusProto proto : protos) {
+ b.add(
+ AttentionStatus.createFromRead(
+ Instant.ofEpochMilli(proto.getDate()),
+ Account.id(proto.getAccount()),
+ AttentionStatus.Operation.valueOf(proto.getOperation()),
+ proto.getReason()));
+ }
+ return b.build();
+ }
+
private static ImmutableList<AssigneeStatusUpdate> toAssigneeStatusUpdateList(
List<AssigneeStatusUpdateProto> protos) {
ImmutableList.Builder<AssigneeStatusUpdate> b = ImmutableList.builder();
diff --git a/java/com/google/gerrit/server/notedb/ChangeUpdate.java b/java/com/google/gerrit/server/notedb/ChangeUpdate.java
index 2822f61..3ef631b 100644
--- a/java/com/google/gerrit/server/notedb/ChangeUpdate.java
+++ b/java/com/google/gerrit/server/notedb/ChangeUpdate.java
@@ -19,6 +19,7 @@
import static com.google.common.base.Preconditions.checkState;
import static com.google.gerrit.entities.RefNames.changeMetaRef;
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_ASSIGNEE;
+import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_ATTENTION;
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_BRANCH;
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_CHANGE_ID;
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_CHERRY_PICK_OF;
@@ -52,6 +53,7 @@
import com.google.common.collect.TreeBasedTable;
import com.google.gerrit.common.data.SubmitRecord;
import com.google.gerrit.entities.Account;
+import com.google.gerrit.entities.AttentionStatus;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.Comment;
import com.google.gerrit.entities.Project;
@@ -125,6 +127,7 @@
private String submissionId;
private String topic;
private String commit;
+ private List<AttentionStatus> attentionUpdates;
private Optional<Account.Id> assignee;
private Set<String> hashtags;
private String changeMessage;
@@ -368,6 +371,21 @@
this.hashtags = hashtags;
}
+ /**
+ * 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.
+ */
+ void setAttentionUpdates(List<AttentionStatus> attentionUpdates) {
+ checkArgument(
+ attentionUpdates.stream().noneMatch(x -> x.timestamp() != null),
+ "must not specify timestamp for write");
+ checkArgument(
+ attentionUpdates.stream().map(AttentionStatus::account).distinct().count()
+ == attentionUpdates.size(),
+ "must not specify multiple updates for single user");
+ this.attentionUpdates = attentionUpdates;
+ }
+
public void setAssignee(Account.Id assignee) {
checkArgument(assignee != null, "use removeAssignee");
this.assignee = Optional.of(assignee);
@@ -578,6 +596,12 @@
addFooter(msg, FOOTER_COMMIT, commit);
}
+ if (attentionUpdates != null) {
+ for (AttentionStatus attentionUpdate : attentionUpdates) {
+ addFooter(msg, FOOTER_ATTENTION, noteUtil.attentionStatusToJson(attentionUpdate));
+ }
+ }
+
if (assignee != null) {
if (assignee.isPresent()) {
addFooter(msg, FOOTER_ASSIGNEE);
@@ -713,6 +737,7 @@
&& status == null
&& submissionId == null
&& submitRecords == null
+ && attentionUpdates == null
&& assignee == null
&& hashtags == null
&& topic == null
@@ -774,12 +799,8 @@
}
private StringBuilder addIdent(StringBuilder sb, Account.Id accountId) {
- PersonIdent ident = newIdent(accountId, when);
-
- PersonIdent.appendSanitized(sb, ident.getName());
- sb.append(" <");
- PersonIdent.appendSanitized(sb, ident.getEmailAddress());
- sb.append('>');
+ PersonIdent ident = noteUtil.newIdent(accountId, when, serverIdent);
+ ChangeNoteUtil.appendIdentString(sb, ident.getName(), ident.getEmailAddress());
return sb;
}
}
diff --git a/java/com/google/gerrit/server/restapi/change/CreateChange.java b/java/com/google/gerrit/server/restapi/change/CreateChange.java
index a6a1eef..9e792d0 100644
--- a/java/com/google/gerrit/server/restapi/change/CreateChange.java
+++ b/java/com/google/gerrit/server/restapi/change/CreateChange.java
@@ -335,9 +335,10 @@
Timestamp now = TimeUtil.nowTs();
+ PersonIdent committer = me.newCommitterIdent(now, serverTimeZone);
PersonIdent author =
input.author == null
- ? me.newCommitterIdent(now, serverTimeZone)
+ ? committer
: new PersonIdent(input.author.name, input.author.email, now, serverTimeZone);
String commitMessage = getCommitMessage(input.subject, me);
@@ -345,7 +346,9 @@
CodeReviewCommit c;
if (input.merge != null) {
// create a merge commit
- c = newMergeCommit(git, oi, rw, projectState, mergeTip, input.merge, author, commitMessage);
+ c =
+ newMergeCommit(
+ git, oi, rw, projectState, mergeTip, input.merge, author, committer, commitMessage);
if (!c.getFilesWithGitConflicts().isEmpty()) {
logger.atFine().log(
"merge commit has conflicts in the following files: %s",
@@ -353,7 +356,7 @@
}
} else {
// create an empty commit
- c = newCommit(oi, rw, author, mergeTip, commitMessage);
+ c = newCommit(oi, rw, author, committer, mergeTip, commitMessage);
}
Change.Id changeId = Change.id(seq.nextChangeId());
@@ -495,6 +498,7 @@
ObjectInserter oi,
CodeReviewRevWalk rw,
PersonIdent authorIdent,
+ PersonIdent committerIdent,
RevCommit mergeTip,
String commitMessage)
throws IOException {
@@ -507,7 +511,7 @@
commit.setParentId(mergeTip);
}
commit.setAuthor(authorIdent);
- commit.setCommitter(authorIdent);
+ commit.setCommitter(committerIdent);
commit.setMessage(commitMessage);
return rw.parseCommit(insert(oi, commit));
}
@@ -520,6 +524,7 @@
RevCommit mergeTip,
MergeInput merge,
PersonIdent authorIdent,
+ PersonIdent committerIdent,
String commitMessage)
throws RestApiException, IOException {
logger.atFine().log(
@@ -556,6 +561,7 @@
mergeStrategy,
merge.allowConflicts,
authorIdent,
+ committerIdent,
commitMessage,
rw);
} catch (NoMergeBaseException e) {
diff --git a/java/com/google/gerrit/sshd/BUILD b/java/com/google/gerrit/sshd/BUILD
index f567a3a..a5b88b4 100644
--- a/java/com/google/gerrit/sshd/BUILD
+++ b/java/com/google/gerrit/sshd/BUILD
@@ -22,6 +22,7 @@
"//java/com/google/gerrit/server/restapi",
"//java/com/google/gerrit/server/util/time",
"//java/com/google/gerrit/util/cli",
+ "//java/com/google/gerrit/util/logging",
"//lib:args4j",
"//lib:gson",
"//lib:guava",
diff --git a/java/com/google/gerrit/sshd/SshLogJsonLayout.java b/java/com/google/gerrit/sshd/SshLogJsonLayout.java
index 8495ed1..5ece5af 100644
--- a/java/com/google/gerrit/sshd/SshLogJsonLayout.java
+++ b/java/com/google/gerrit/sshd/SshLogJsonLayout.java
@@ -23,8 +23,8 @@
import static com.google.gerrit.sshd.SshLog.P_USER_NAME;
import static com.google.gerrit.sshd.SshLog.P_WAIT;
-import com.google.gerrit.server.logging.JsonLayout;
-import com.google.gerrit.server.logging.JsonLogEntry;
+import com.google.gerrit.util.logging.JsonLayout;
+import com.google.gerrit.util.logging.JsonLogEntry;
import java.time.format.DateTimeFormatter;
import org.apache.log4j.spi.LoggingEvent;
diff --git a/java/com/google/gerrit/util/logging/BUILD b/java/com/google/gerrit/util/logging/BUILD
new file mode 100644
index 0000000..b8db49bd
--- /dev/null
+++ b/java/com/google/gerrit/util/logging/BUILD
@@ -0,0 +1,13 @@
+load("@rules_java//java:defs.bzl", "java_library")
+
+java_library(
+ name = "logging",
+ srcs = glob(
+ ["*.java"],
+ ),
+ visibility = ["//visibility:public"],
+ deps = [
+ "//lib:gson",
+ "//lib/log:log4j",
+ ],
+)
diff --git a/java/com/google/gerrit/server/logging/JsonLayout.java b/java/com/google/gerrit/util/logging/JsonLayout.java
similarity index 94%
rename from java/com/google/gerrit/server/logging/JsonLayout.java
rename to java/com/google/gerrit/util/logging/JsonLayout.java
index 3eb4515..45fbd32 100644
--- a/java/com/google/gerrit/server/logging/JsonLayout.java
+++ b/java/com/google/gerrit/util/logging/JsonLayout.java
@@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.
-package com.google.gerrit.server.logging;
+package com.google.gerrit.util.logging;
import com.google.gson.FieldNamingPolicy;
import com.google.gson.Gson;
@@ -34,7 +34,7 @@
public JsonLayout() {
dateFormatter = createDateTimeFormatter();
- timeOffset = OffsetDateTime.now().getOffset();
+ timeOffset = OffsetDateTime.now(ZoneId.systemDefault()).getOffset();
gson = newGson();
}
diff --git a/java/com/google/gerrit/server/logging/JsonLogEntry.java b/java/com/google/gerrit/util/logging/JsonLogEntry.java
similarity index 94%
rename from java/com/google/gerrit/server/logging/JsonLogEntry.java
rename to java/com/google/gerrit/util/logging/JsonLogEntry.java
index bc16c70..ceac427 100644
--- a/java/com/google/gerrit/server/logging/JsonLogEntry.java
+++ b/java/com/google/gerrit/util/logging/JsonLogEntry.java
@@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.
-package com.google.gerrit.server.logging;
+package com.google.gerrit.util.logging;
import org.apache.log4j.spi.LoggingEvent;
diff --git a/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java b/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java
index 8ef49a1..e2d2501 100644
--- a/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java
@@ -857,7 +857,7 @@
gApi.changes().id(r.getChangeId()).abandon();
List<Message> messages = sender.getMessages();
assertThat(messages).hasSize(1);
- assertThat(messages.get(0).rcpt()).containsExactly(user2.getEmailAddress());
+ assertThat(messages.get(0).rcpt()).containsExactly(user2.getNameEmail());
accountIndexedCounter.assertNoReindex();
}
}
@@ -883,7 +883,7 @@
List<Message> messages = sender.getMessages();
assertThat(messages).hasSize(1);
Message message = messages.get(0);
- assertThat(message.rcpt()).containsExactly(user.getEmailAddress());
+ assertThat(message.rcpt()).containsExactly(user.getNameEmail());
assertMailReplyTo(message, admin.email());
accountIndexedCounter.assertNoReindex();
}
@@ -1840,7 +1840,7 @@
assertThat(sender.getMessages()).hasSize(1);
Message message = sender.getMessages().get(0);
- assertThat(message.rcpt()).containsExactly(user.getEmailAddress());
+ assertThat(message.rcpt()).containsExactly(user.getNameEmail());
assertThat(message.body()).contains("new SSH keys have been added");
// Delete key
@@ -1852,7 +1852,7 @@
assertThat(sender.getMessages()).hasSize(1);
message = sender.getMessages().get(0);
- assertThat(message.rcpt()).containsExactly(user.getEmailAddress());
+ assertThat(message.rcpt()).containsExactly(user.getNameEmail());
assertThat(message.body()).contains("SSH keys have been deleted");
}
}
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
index 0eefe02..9399c3b 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -1518,7 +1518,7 @@
assertThat(messages).hasSize(1);
Message m = messages.get(0);
assertThat(m.from().getName()).isEqualTo("Administrator (Code Review)");
- assertThat(m.rcpt()).containsExactly(user.getEmailAddress());
+ assertThat(m.rcpt()).containsExactly(user.getNameEmail());
assertThat(m.body()).contains("I'd like you to do a code review");
assertThat(m.body()).contains("Change subject: " + PushOneCommit.SUBJECT + "\n");
assertMailReplyTo(m, admin.email());
@@ -1587,7 +1587,7 @@
List<Message> messages = sender.getMessages();
assertThat(messages).hasSize(1);
Message m = messages.get(0);
- assertThat(m.rcpt()).containsExactly(user.getEmailAddress());
+ assertThat(m.rcpt()).containsExactly(user.getNameEmail());
assertThat(m.body()).contains("Hello " + user.fullName() + ",\n");
assertThat(m.body()).contains("I'd like you to do a code review.");
assertThat(m.body()).contains("Change subject: " + PushOneCommit.SUBJECT + "\n");
@@ -1771,7 +1771,7 @@
List<Message> messages = sender.getMessages();
assertThat(messages).hasSize(1);
Message m = messages.get(0);
- assertThat(m.rcpt()).containsExactly(user.getEmailAddress());
+ assertThat(m.rcpt()).containsExactly(user.getNameEmail());
assertThat(m.body()).contains("Hello " + user.fullName() + ",\n");
assertThat(m.body()).contains("I'd like you to do a code review.");
assertThat(m.body()).contains("Change subject: " + PushOneCommit.SUBJECT + "\n");
@@ -2162,7 +2162,7 @@
assertThat(sender.getMessages()).hasSize(1);
Message m = sender.getMessages().get(0);
- assertThat(m.rcpt()).containsExactly(user.getEmailAddress());
+ assertThat(m.rcpt()).containsExactly(user.getNameEmail());
}
@Test
@@ -2411,7 +2411,7 @@
List<Message> messages = sender.getMessages();
assertThat(messages).hasSize(1);
Message msg = messages.get(0);
- assertThat(msg.rcpt()).containsExactly(user.getEmailAddress());
+ assertThat(msg.rcpt()).containsExactly(user.getNameEmail());
assertThat(msg.body()).contains(admin.fullName() + " has removed a vote from this change.");
assertThat(msg.body())
.contains("Removed Code-Review+1 by " + user.fullName() + " <" + user.email() + ">\n");
@@ -3065,7 +3065,7 @@
assertThat(commitPatchSetCreation.getShortMessage()).isEqualTo("Create patch set 2");
PersonIdent expectedAuthor =
- changeNoteUtil.newIdent(getAccount(admin.id()), c.updated, serverIdent.get());
+ changeNoteUtil.newIdent(getAccount(admin.id()).id(), c.updated, serverIdent.get());
assertThat(commitPatchSetCreation.getAuthorIdent()).isEqualTo(expectedAuthor);
assertThat(commitPatchSetCreation.getCommitterIdent())
.isEqualTo(new PersonIdent(serverIdent.get(), c.updated));
@@ -3074,7 +3074,7 @@
RevCommit commitChangeCreation = rw.parseCommit(commitPatchSetCreation.getParent(0));
assertThat(commitChangeCreation.getShortMessage()).isEqualTo("Create change");
expectedAuthor =
- changeNoteUtil.newIdent(getAccount(admin.id()), c.created, serverIdent.get());
+ changeNoteUtil.newIdent(getAccount(admin.id()).id(), c.created, serverIdent.get());
assertThat(commitChangeCreation.getAuthorIdent()).isEqualTo(expectedAuthor);
assertThat(commitChangeCreation.getCommitterIdent())
.isEqualTo(new PersonIdent(serverIdent.get(), c.created));
@@ -4527,7 +4527,7 @@
List<Message> messages = sender.getMessages();
assertThat(messages).hasSize(1);
- assertThat(messages.get(0).rcpt()).containsExactly(user.getEmailAddress());
+ assertThat(messages.get(0).rcpt()).containsExactly(user.getNameEmail());
}
@Test
diff --git a/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java b/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java
index 51dee72..4ee1f35 100644
--- a/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java
+++ b/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java
@@ -545,7 +545,7 @@
r.assertOkStatus();
assertThat(sender.getMessages()).hasSize(1);
Message m = sender.getMessages().get(0);
- assertThat(m.rcpt()).containsExactly(user.getEmailAddress());
+ assertThat(m.rcpt()).containsExactly(user.getNameEmail());
sender.clear();
r = pushTo(pushSpec + ",notify=" + NotifyHandling.ALL);
@@ -553,7 +553,7 @@
assertThat(sender.getMessages()).hasSize(1);
m = sender.getMessages().get(0);
assertThat(m.rcpt())
- .containsExactly(user.getEmailAddress(), user2.getEmailAddress(), user3.getEmailAddress());
+ .containsExactly(user.getNameEmail(), user2.getNameEmail(), user3.getNameEmail());
sender.clear();
r = pushTo(pushSpec + ",notify=" + NotifyHandling.NONE + ",notify-to=" + user3.email());
@@ -1116,7 +1116,7 @@
assertThat(sender.getMessages()).hasSize(1);
assertThat(sender.getMessages().get(0).rcpt())
- .containsExactly(user.getEmailAddress(), user2.getEmailAddress());
+ .containsExactly(user.getNameEmail(), user2.getNameEmail());
}
@Test
@@ -1141,7 +1141,7 @@
assertThat(sender.getMessages()).hasSize(1);
assertThat(sender.getMessages().get(0).rcpt())
- .containsExactly(user.getEmailAddress(), user2.getEmailAddress());
+ .containsExactly(user.getNameEmail(), user2.getNameEmail());
}
/**
@@ -1838,7 +1838,7 @@
@Test
public void pushWithEmailInFooter() throws Exception {
- pushWithReviewerInFooter(user.getEmailAddress().toString(), user);
+ pushWithReviewerInFooter(user.getNameEmail().toString(), user);
}
@Test
diff --git a/javatests/com/google/gerrit/acceptance/git/RefAdvertisementIT.java b/javatests/com/google/gerrit/acceptance/git/RefAdvertisementIT.java
index 56d9ff2..1083377 100644
--- a/javatests/com/google/gerrit/acceptance/git/RefAdvertisementIT.java
+++ b/javatests/com/google/gerrit/acceptance/git/RefAdvertisementIT.java
@@ -1071,7 +1071,7 @@
PersonIdent committer = serverIdent.get();
PersonIdent author =
- noteUtil.newIdent(getAccount(admin.id()), committer.getWhen(), committer);
+ noteUtil.newIdent(getAccount(admin.id()).id(), committer.getWhen(), committer);
tr.branch(RefNames.changeMetaRef(cd3.getId()))
.commit()
.author(author)
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/AssigneeIT.java b/javatests/com/google/gerrit/acceptance/rest/change/AssigneeIT.java
index 0f5def6..420ddda 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/AssigneeIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/AssigneeIT.java
@@ -61,7 +61,7 @@
assertThat(sender.getMessages()).hasSize(1);
Message m = sender.getMessages().get(0);
- assertThat(m.rcpt()).containsExactly(user.getEmailAddress());
+ assertThat(m.rcpt()).containsExactly(user.getNameEmail());
}
@Test
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/ChangeReviewersByEmailIT.java b/javatests/com/google/gerrit/acceptance/rest/change/ChangeReviewersByEmailIT.java
index ac00e38..090146e 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/ChangeReviewersByEmailIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/ChangeReviewersByEmailIT.java
@@ -209,7 +209,7 @@
List<Message> messages = sender.getMessages();
assertThat(messages).hasSize(1);
assertThat(messages.get(0).rcpt())
- .containsExactly(Address.parse(addInput.reviewer), user.getEmailAddress());
+ .containsExactly(Address.parse(addInput.reviewer), user.getNameEmail());
sender.clear();
}
}
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/ChangeReviewersIT.java b/javatests/com/google/gerrit/acceptance/rest/change/ChangeReviewersIT.java
index fbe6533..0a006fa 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/ChangeReviewersIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/ChangeReviewersIT.java
@@ -150,7 +150,7 @@
List<Message> messages = sender.getMessages();
assertThat(messages).hasSize(1);
Message m = messages.get(0);
- assertThat(m.rcpt()).containsExactly(user.getEmailAddress());
+ assertThat(m.rcpt()).containsExactly(user.getNameEmail());
assertThat(m.body()).contains(admin.fullName() + " has uploaded this change for review.");
}
@@ -188,7 +188,7 @@
Message m = messages.get(0);
List<Address> expectedAddresses = new ArrayList<>(firstUsers.size());
for (TestAccount u : firstUsers) {
- expectedAddresses.add(u.getEmailAddress());
+ expectedAddresses.add(u.getNameEmail());
}
assertThat(m.rcpt()).containsExactlyElementsIn(expectedAddresses);
@@ -216,9 +216,9 @@
m = messages.get(0);
expectedAddresses = new ArrayList<>(4);
for (int i = 0; i < 3; i++) {
- expectedAddresses.add(users.get(users.size() - i - 1).getEmailAddress());
+ expectedAddresses.add(users.get(users.size() - i - 1).getNameEmail());
}
- expectedAddresses.add(reviewer.getEmailAddress());
+ expectedAddresses.add(reviewer.getNameEmail());
assertThat(m.rcpt()).containsExactlyElementsIn(expectedAddresses);
}
@@ -398,13 +398,13 @@
assertThat(messages).hasSize(2);
Message m = messages.get(0);
- assertThat(m.rcpt()).containsExactly(user.getEmailAddress(), observer.getEmailAddress());
+ assertThat(m.rcpt()).containsExactly(user.getNameEmail(), observer.getNameEmail());
assertThat(m.body()).contains(admin.fullName() + " has posted comments on this change.");
assertThat(m.body()).contains("Change subject: " + PushOneCommit.SUBJECT + "\n");
assertThat(m.body()).contains("Patch Set 1: Code-Review+2");
m = messages.get(1);
- assertThat(m.rcpt()).containsExactly(user.getEmailAddress(), observer.getEmailAddress());
+ assertThat(m.rcpt()).containsExactly(user.getNameEmail(), observer.getNameEmail());
assertThat(m.body()).contains("Hello " + user.fullName() + ",\n");
assertThat(m.body()).contains("I'd like you to do a code review.");
}
@@ -635,7 +635,7 @@
sender.clear();
gApi.changes().id(r.getChangeId()).current().review(reviewInput);
assertThat(sender.getMessages()).hasSize(1);
- assertThat(sender.getMessages().get(0).rcpt()).containsExactly(userToNotify.getEmailAddress());
+ assertThat(sender.getMessages().get(0).rcpt()).containsExactly(userToNotify.getNameEmail());
}
@Test
@@ -652,7 +652,7 @@
sender.clear();
gApi.changes().id(r.getChangeId()).addReviewer(addReviewer);
assertThat(sender.getMessages()).hasSize(1);
- assertThat(sender.getMessages().get(0).rcpt()).containsExactly(userToNotify.getEmailAddress());
+ assertThat(sender.getMessages().get(0).rcpt()).containsExactly(userToNotify.getNameEmail());
}
@Test
@@ -829,7 +829,7 @@
}
private void assertThatUserIsOnlyReviewer(String changeId) throws Exception {
- AccountInfo userInfo = new AccountInfo(user.fullName(), user.getEmailAddress().getEmail());
+ AccountInfo userInfo = new AccountInfo(user.fullName(), user.getNameEmail().getEmail());
userInfo._accountId = user.id().get();
userInfo.username = user.username();
assertThat(gApi.changes().id(changeId).get().reviewers)
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java b/javatests/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java
index 0af1295..d831b62 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java
@@ -216,7 +216,7 @@
List<Message> messages = sender.getMessages();
assertThat(messages).hasSize(1);
Message m = messages.get(0);
- assertThat(m.rcpt()).containsExactly(user.getEmailAddress());
+ assertThat(m.rcpt()).containsExactly(user.getNameEmail());
assertThat(m.body()).contains(admin.fullName() + " has uploaded this change for review.");
// check that watcher is not notified if notify=NONE
@@ -299,9 +299,11 @@
ChangeInfo info = assertCreateSucceeds(input);
RevisionApi rApi = gApi.changes().id(info.id).current();
- GitPerson person = rApi.commit(false).author;
- assertThat(person).email().isEqualTo(input.author.email);
- assertThat(person).name().isEqualTo(input.author.name);
+ GitPerson author = rApi.commit(false).author;
+ assertThat(author).email().isEqualTo(input.author.email);
+ assertThat(author).name().isEqualTo(input.author.name);
+ GitPerson committer = rApi.commit(false).committer;
+ assertThat(committer).email().isEqualTo(admin.getNameEmail().getEmail());
}
@Test
@@ -331,7 +333,19 @@
changeInTwoBranches("foo", "foo.txt", "bar", "bar.txt");
ChangeInput input = newChangeInput(ChangeStatus.NEW);
input.baseCommit = setup.get("master").getCommit().getId().name();
- assertCreateSucceeds(input);
+ ChangeInfo result = assertCreateSucceeds(input);
+ assertThat(gApi.changes().id(result.id).current().commit(false).parents.get(0).commit)
+ .isEqualTo(input.baseCommit);
+ }
+
+ @Test
+ public void createChangeWithParentChange() throws Exception {
+ Result change = createChange();
+ ChangeInput input = newChangeInput(ChangeStatus.NEW);
+ input.baseChange = change.getChangeId();
+ ChangeInfo result = assertCreateSucceeds(input);
+ assertThat(gApi.changes().id(result.id).current().commit(false).parents.get(0).commit)
+ .isEqualTo(change.getCommit().getId().name());
}
@Test
@@ -415,7 +429,7 @@
assertThat(commit.getShortMessage()).isEqualTo("Create change");
PersonIdent expectedAuthor =
- changeNoteUtil.newIdent(getAccount(admin.id()), c.created, serverIdent.get());
+ changeNoteUtil.newIdent(getAccount(admin.id()).id(), c.created, serverIdent.get());
assertThat(commit.getAuthorIdent()).isEqualTo(expectedAuthor);
assertThat(commit.getCommitterIdent())
@@ -437,6 +451,22 @@
}
@Test
+ public void createMergeChangeAuthor() throws Exception {
+ changeInTwoBranches("branchA", "a.txt", "branchB", "b.txt");
+ ChangeInput in = newMergeChangeInput("branchA", "branchB", "");
+ in.author = new AccountInput();
+ in.author.name = "Gerritless Jane";
+ in.author.email = "gerritlessjane@invalid";
+ ChangeInfo change = assertCreateSucceeds(in);
+
+ RevisionApi rApi = gApi.changes().id(change.id).current();
+ GitPerson author = rApi.commit(false).author;
+ assertThat(author).email().isEqualTo(in.author.email);
+ GitPerson committer = rApi.commit(false).committer;
+ assertThat(committer).email().isEqualTo(admin.getNameEmail().getEmail());
+ }
+
+ @Test
public void createMergeChange_Conflicts() throws Exception {
changeInTwoBranches("branchA", "shared.txt", "branchB", "shared.txt");
ChangeInput in = newMergeChangeInput("branchA", "branchB", "");
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/DeleteVoteIT.java b/javatests/com/google/gerrit/acceptance/rest/change/DeleteVoteIT.java
index 37fa2ce..25e5647 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/DeleteVoteIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/DeleteVoteIT.java
@@ -73,7 +73,7 @@
List<FakeEmailSender.Message> messages = sender.getMessages();
assertThat(messages).hasSize(1);
FakeEmailSender.Message msg = messages.get(0);
- assertThat(msg.rcpt()).containsExactly(user.getEmailAddress());
+ assertThat(msg.rcpt()).containsExactly(user.getNameEmail());
assertThat(msg.body()).contains(admin.fullName() + " has removed a vote from this change.");
assertThat(msg.body())
.contains("Removed Code-Review+1 by " + user.fullName() + " <" + user.email() + ">\n");
diff --git a/javatests/com/google/gerrit/acceptance/server/change/ConsistencyCheckerIT.java b/javatests/com/google/gerrit/acceptance/server/change/ConsistencyCheckerIT.java
index ee5f117..069387c 100644
--- a/javatests/com/google/gerrit/acceptance/server/change/ConsistencyCheckerIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/change/ConsistencyCheckerIT.java
@@ -830,7 +830,8 @@
private void addNoteDbCommit(Change.Id id, String commitMessage) throws Exception {
PersonIdent committer = serverIdent.get();
- PersonIdent author = noteUtil.newIdent(getAccount(admin.id()), committer.getWhen(), committer);
+ PersonIdent author =
+ noteUtil.newIdent(getAccount(admin.id()).id(), committer.getWhen(), committer);
serverSideTestRepo
.branch(RefNames.changeMetaRef(id))
.commit()
diff --git a/javatests/com/google/gerrit/acceptance/server/mail/AbstractMailIT.java b/javatests/com/google/gerrit/acceptance/server/mail/AbstractMailIT.java
index 3b0e27b..9e0de92 100644
--- a/javatests/com/google/gerrit/acceptance/server/mail/AbstractMailIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/mail/AbstractMailIT.java
@@ -38,8 +38,8 @@
MailMessage.Builder messageBuilderWithDefaultFields() {
MailMessage.Builder b = MailMessage.builder();
b.id("some id");
- b.from(user.getEmailAddress());
- b.addTo(user.getEmailAddress()); // Not evaluated
+ b.from(user.getNameEmail());
+ b.addTo(user.getNameEmail()); // Not evaluated
b.subject("");
b.dateReceived(Instant.now());
return b;
diff --git a/javatests/com/google/gerrit/acceptance/server/mail/MailProcessorIT.java b/javatests/com/google/gerrit/acceptance/server/mail/MailProcessorIT.java
index 502194a..49f25e4 100644
--- a/javatests/com/google/gerrit/acceptance/server/mail/MailProcessorIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/mail/MailProcessorIT.java
@@ -237,7 +237,7 @@
newPlaintextBody(getChangeUrl(changeInfo) + "/1", "Test Message", null, null, null);
MailMessage.Builder b =
messageBuilderWithDefaultFields()
- .from(user.getEmailAddress())
+ .from(user.getNameEmail())
.textContent(txt + textFooterForChange(changeInfo._number, ts));
sender.clear();
@@ -259,7 +259,7 @@
newPlaintextBody(getChangeUrl(changeInfo) + "/1", "Test Message", null, null, null);
MailMessage.Builder b =
messageBuilderWithDefaultFields()
- .from(user.getEmailAddress())
+ .from(user.getNameEmail())
.textContent(txt + textFooterForChange(changeInfo._number, ts));
sender.clear();
diff --git a/javatests/com/google/gerrit/acceptance/server/project/ProjectWatchIT.java b/javatests/com/google/gerrit/acceptance/server/project/ProjectWatchIT.java
index 29574c4..8b12b9d 100644
--- a/javatests/com/google/gerrit/acceptance/server/project/ProjectWatchIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/project/ProjectWatchIT.java
@@ -241,7 +241,7 @@
List<Message> messages = sender.getMessages();
assertThat(messages).hasSize(1);
Message m = messages.get(0);
- assertThat(m.rcpt()).containsExactly(user.getEmailAddress());
+ assertThat(m.rcpt()).containsExactly(user.getNameEmail());
assertThat(m.body()).contains("Change subject: TRIGGER\n");
assertThat(m.body()).contains("Gerrit-PatchSet: 1\n");
}
@@ -273,7 +273,7 @@
List<Message> messages = sender.getMessages();
assertThat(messages).hasSize(1);
Message m = messages.get(0);
- assertThat(m.rcpt()).containsExactly(user.getEmailAddress());
+ assertThat(m.rcpt()).containsExactly(user.getNameEmail());
assertThat(m.body()).contains("Change subject: TRIGGER\n");
assertThat(m.body()).contains("Gerrit-PatchSet: 1\n");
sender.clear();
@@ -295,7 +295,7 @@
messages = sender.getMessages();
assertThat(messages).hasSize(1);
m = messages.get(0);
- assertThat(m.rcpt()).containsExactly(user2.getEmailAddress());
+ assertThat(m.rcpt()).containsExactly(user2.getNameEmail());
assertThat(m.body()).contains("Change subject: TRIGGER_USER2\n");
assertThat(m.body()).contains("Gerrit-PatchSet: 1\n");
}
@@ -322,7 +322,7 @@
List<Message> messages = sender.getMessages();
assertThat(messages).hasSize(1);
Message m = messages.get(0);
- assertThat(m.rcpt()).containsExactly(user.getEmailAddress());
+ assertThat(m.rcpt()).containsExactly(user.getNameEmail());
assertThat(m.body()).contains("Change subject: Document multiprimary setup\n");
assertThat(m.body()).contains("Gerrit-PatchSet: 1\n");
sender.clear();
@@ -357,7 +357,7 @@
List<Message> messages = sender.getMessages();
assertThat(messages).hasSize(1);
Message m = messages.get(0);
- assertThat(m.rcpt()).containsExactly(user.getEmailAddress());
+ assertThat(m.rcpt()).containsExactly(user.getNameEmail());
assertThat(m.body()).contains("Change subject: TRIGGER\n");
assertThat(m.body()).contains("Gerrit-PatchSet: 1\n");
}
@@ -385,7 +385,7 @@
List<Message> messages = sender.getMessages();
assertThat(messages).hasSize(1);
Message m = messages.get(0);
- assertThat(m.rcpt()).containsExactly(user.getEmailAddress());
+ assertThat(m.rcpt()).containsExactly(user.getNameEmail());
assertThat(m.body()).contains("Change subject: TRIGGER\n");
assertThat(m.body()).contains("Gerrit-PatchSet: 1\n");
sender.clear();
@@ -407,7 +407,7 @@
messages = sender.getMessages();
assertThat(messages).hasSize(1);
m = messages.get(0);
- assertThat(m.rcpt()).containsExactly(user2.getEmailAddress());
+ assertThat(m.rcpt()).containsExactly(user2.getNameEmail());
assertThat(m.body()).contains("Change subject: TRIGGER_USER2\n");
assertThat(m.body()).contains("Gerrit-PatchSet: 1\n");
}
@@ -434,7 +434,7 @@
List<Message> messages = sender.getMessages();
assertThat(messages).hasSize(1);
Message m = messages.get(0);
- assertThat(m.rcpt()).containsExactly(user.getEmailAddress());
+ assertThat(m.rcpt()).containsExactly(user.getNameEmail());
assertThat(m.body()).contains("Change subject: Document multiprimary setup\n");
assertThat(m.body()).contains("Gerrit-PatchSet: 1\n");
sender.clear();
@@ -547,7 +547,7 @@
List<Message> messages = sender.getMessages();
assertThat(messages).hasSize(1);
Message m = messages.get(0);
- assertThat(m.rcpt()).containsExactly(userThatCanViewPrivateChanges.getEmailAddress());
+ assertThat(m.rcpt()).containsExactly(userThatCanViewPrivateChanges.getNameEmail());
assertThat(m.body()).contains("Change subject: TRIGGER\n");
assertThat(m.body()).contains("Gerrit-PatchSet: 1\n");
}
diff --git a/javatests/com/google/gerrit/server/notedb/ChangeNotesParserTest.java b/javatests/com/google/gerrit/server/notedb/ChangeNotesParserTest.java
index 013939a..8ffcc8b 100644
--- a/javatests/com/google/gerrit/server/notedb/ChangeNotesParserTest.java
+++ b/javatests/com/google/gerrit/server/notedb/ChangeNotesParserTest.java
@@ -518,7 +518,9 @@
private RevCommit writeCommit(String body) throws Exception {
ChangeNoteUtil noteUtil = injector.getInstance(ChangeNoteUtil.class);
return writeCommit(
- body, noteUtil.newIdent(changeOwner.getAccount(), TimeUtil.nowTs(), serverIdent), false);
+ body,
+ noteUtil.newIdent(changeOwner.getAccount().id(), TimeUtil.nowTs(), serverIdent),
+ false);
}
private RevCommit writeCommit(String body, PersonIdent author) throws Exception {
@@ -529,7 +531,7 @@
ChangeNoteUtil noteUtil = injector.getInstance(ChangeNoteUtil.class);
return writeCommit(
body,
- noteUtil.newIdent(changeOwner.getAccount(), TimeUtil.nowTs(), serverIdent),
+ noteUtil.newIdent(changeOwner.getAccount().id(), TimeUtil.nowTs(), serverIdent),
initWorkInProgress);
}
diff --git a/javatests/com/google/gerrit/server/notedb/ChangeNotesStateTest.java b/javatests/com/google/gerrit/server/notedb/ChangeNotesStateTest.java
index 16981dc..68c7883 100644
--- a/javatests/com/google/gerrit/server/notedb/ChangeNotesStateTest.java
+++ b/javatests/com/google/gerrit/server/notedb/ChangeNotesStateTest.java
@@ -28,6 +28,7 @@
import com.google.gerrit.common.data.SubmitRecord;
import com.google.gerrit.common.data.SubmitRequirement;
import com.google.gerrit.entities.Account;
+import com.google.gerrit.entities.AttentionStatus;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.ChangeMessage;
import com.google.gerrit.entities.Comment;
@@ -44,6 +45,7 @@
import com.google.gerrit.server.ReviewerStatusUpdate;
import com.google.gerrit.server.cache.proto.Cache.ChangeNotesStateProto;
import com.google.gerrit.server.cache.proto.Cache.ChangeNotesStateProto.AssigneeStatusUpdateProto;
+import com.google.gerrit.server.cache.proto.Cache.ChangeNotesStateProto.AttentionStatusProto;
import com.google.gerrit.server.cache.proto.Cache.ChangeNotesStateProto.ChangeColumnsProto;
import com.google.gerrit.server.cache.proto.Cache.ChangeNotesStateProto.ReviewerByEmailSetEntryProto;
import com.google.gerrit.server.cache.proto.Cache.ChangeNotesStateProto.ReviewerSetEntryProto;
@@ -55,6 +57,7 @@
import com.google.protobuf.ByteString;
import java.lang.reflect.Type;
import java.sql.Timestamp;
+import java.time.Instant;
import java.util.List;
import java.util.Map;
import java.util.Optional;
@@ -64,6 +67,7 @@
import org.junit.Test;
public class ChangeNotesStateTest {
+
private static final Change.Id ID = Change.id(123);
private static final ObjectId SHA =
ObjectId.fromString("1234567812345678123456781234567812345678");
@@ -95,6 +99,13 @@
return ChangeNotesState.Builder.empty(ID).metaId(SHA).columns(cols);
}
+ private ChangeNotesStateProto.Builder newProtoBuilder() {
+ return ChangeNotesStateProto.newBuilder()
+ .setMetaId(SHA_BYTES)
+ .setChangeId(ID.get())
+ .setColumns(colsProto);
+ }
+
@Test
public void serializeChangeKey() throws Exception {
assertRoundTrip(
@@ -589,6 +600,39 @@
}
@Test
+ public void serializeAttentionUpdates() throws Exception {
+ assertRoundTrip(
+ newBuilder()
+ .attentionUpdates(
+ ImmutableList.of(
+ AttentionStatus.createFromRead(
+ Instant.EPOCH.plusSeconds(23),
+ Account.id(1000),
+ AttentionStatus.Operation.ADD,
+ "reason 1"),
+ AttentionStatus.createFromRead(
+ Instant.EPOCH.plusSeconds(42),
+ Account.id(2000),
+ AttentionStatus.Operation.REMOVE,
+ "reason 2")))
+ .build(),
+ newProtoBuilder()
+ .addAttentionStatus(
+ AttentionStatusProto.newBuilder()
+ .setDate(23_000) // epoch millis
+ .setAccount(1000)
+ .setOperation("ADD")
+ .setReason("reason 1"))
+ .addAttentionStatus(
+ AttentionStatusProto.newBuilder()
+ .setDate(42_000) // epoch millis
+ .setAccount(2000)
+ .setOperation("REMOVE")
+ .setReason("reason 2"))
+ .build());
+ }
+
+ @Test
public void serializeAssigneeUpdates() throws Exception {
assertRoundTrip(
newBuilder()
@@ -745,6 +789,9 @@
"reviewerUpdates",
new TypeLiteral<ImmutableList<ReviewerStatusUpdate>>() {}.getType())
.put(
+ "attentionUpdates",
+ new TypeLiteral<ImmutableList<AttentionStatus>>() {}.getType())
+ .put(
"assigneeUpdates",
new TypeLiteral<ImmutableList<AssigneeStatusUpdate>>() {}.getType())
.put("submitRecords", new TypeLiteral<ImmutableList<SubmitRecord>>() {}.getType())
@@ -824,10 +871,10 @@
.hasFields(
ImmutableMap.of(
"table",
- new TypeLiteral<
- ImmutableTable<
- ReviewerStateInternal, Account.Id, Timestamp>>() {}.getType(),
- "accounts", new TypeLiteral<ImmutableSet<Account.Id>>() {}.getType()));
+ new TypeLiteral<
+ ImmutableTable<ReviewerStateInternal, Account.Id, Timestamp>>() {}.getType(),
+ "accounts",
+ new TypeLiteral<ImmutableSet<Account.Id>>() {}.getType()));
}
@Test
@@ -836,9 +883,10 @@
.hasFields(
ImmutableMap.of(
"table",
- new TypeLiteral<
- ImmutableTable<ReviewerStateInternal, Address, Timestamp>>() {}.getType(),
- "users", new TypeLiteral<ImmutableSet<Address>>() {}.getType()));
+ new TypeLiteral<
+ ImmutableTable<ReviewerStateInternal, Address, Timestamp>>() {}.getType(),
+ "users",
+ new TypeLiteral<ImmutableSet<Address>>() {}.getType()));
}
@Test
diff --git a/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java b/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java
index db0dec8..4e068ba 100644
--- a/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java
+++ b/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java
@@ -38,6 +38,8 @@
import com.google.common.collect.Lists;
import com.google.gerrit.common.data.SubmitRecord;
import com.google.gerrit.entities.Account;
+import com.google.gerrit.entities.AttentionStatus;
+import com.google.gerrit.entities.AttentionStatus.Operation;
import com.google.gerrit.entities.BranchNameKey;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.ChangeMessage;
@@ -58,6 +60,7 @@
import com.google.gerrit.testing.TestChanges;
import com.google.inject.Inject;
import java.sql.Timestamp;
+import java.time.Instant;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
@@ -688,6 +691,92 @@
}
@Test
+ public void defaultAttentionSetIsEmpty() throws Exception {
+ Change c = newChange();
+ ChangeNotes notes = newNotes(c);
+ assertThat(notes.getAttentionUpdates()).isEmpty();
+ }
+
+ @Test
+ public void addAttentionStatus() throws Exception {
+ Change c = newChange();
+ ChangeUpdate update = newUpdate(c, changeOwner);
+ AttentionStatus attentionStatus =
+ AttentionStatus.createForWrite(changeOwner.getAccountId(), Operation.ADD, "test");
+ update.setAttentionUpdates(ImmutableList.of(attentionStatus));
+ update.commit();
+
+ ChangeNotes notes = newNotes(c);
+ assertThat(notes.getAttentionUpdates()).containsExactly(addTimestamp(attentionStatus, c));
+ }
+
+ @Test
+ public void filterLatestAttentionStatus() throws Exception {
+ Change c = newChange();
+ ChangeUpdate update = newUpdate(c, changeOwner);
+ AttentionStatus attentionStatus =
+ AttentionStatus.createForWrite(changeOwner.getAccountId(), Operation.ADD, "test");
+ update.setAttentionUpdates(ImmutableList.of(attentionStatus));
+ update.commit();
+ update = newUpdate(c, changeOwner);
+ attentionStatus =
+ AttentionStatus.createForWrite(changeOwner.getAccountId(), Operation.REMOVE, "test");
+ update.setAttentionUpdates(ImmutableList.of(attentionStatus));
+ update.commit();
+
+ ChangeNotes notes = newNotes(c);
+ assertThat(notes.getAttentionUpdates()).containsExactly(addTimestamp(attentionStatus, c));
+ }
+
+ @Test
+ public void addAttentionStatus_rejectTimestamp() throws Exception {
+ Change c = newChange();
+ ChangeUpdate update = newUpdate(c, changeOwner);
+ AttentionStatus attentionStatus =
+ AttentionStatus.createFromRead(
+ Instant.now(), changeOwner.getAccountId(), Operation.ADD, "test");
+ IllegalArgumentException thrown =
+ assertThrows(
+ IllegalArgumentException.class,
+ () -> update.setAttentionUpdates(ImmutableList.of(attentionStatus)));
+ assertThat(thrown).hasMessageThat().contains("must not specify timestamp for write");
+ }
+
+ @Test
+ public void addAttentionStatus_rejectMultiplePerUser() throws Exception {
+ Change c = newChange();
+ ChangeUpdate update = newUpdate(c, changeOwner);
+ AttentionStatus attentionStatus0 =
+ AttentionStatus.createForWrite(changeOwner.getAccountId(), Operation.ADD, "test 0");
+ AttentionStatus attentionStatus1 =
+ AttentionStatus.createForWrite(changeOwner.getAccountId(), Operation.ADD, "test 1");
+ IllegalArgumentException thrown =
+ assertThrows(
+ IllegalArgumentException.class,
+ () -> update.setAttentionUpdates(ImmutableList.of(attentionStatus0, attentionStatus1)));
+ assertThat(thrown)
+ .hasMessageThat()
+ .contains("must not specify multiple updates for single user");
+ }
+
+ @Test
+ public void addAttentionStatusForMultipleUsers() throws Exception {
+ Change c = newChange();
+ ChangeUpdate update = newUpdate(c, changeOwner);
+ AttentionStatus attentionStatus0 =
+ AttentionStatus.createForWrite(changeOwner.getAccountId(), Operation.ADD, "test");
+ AttentionStatus attentionStatus1 =
+ AttentionStatus.createForWrite(otherUser.getAccountId(), Operation.ADD, "test");
+
+ update.setAttentionUpdates(ImmutableList.of(attentionStatus0, attentionStatus1));
+ update.commit();
+
+ ChangeNotes notes = newNotes(c);
+ assertThat(notes.getAttentionUpdates())
+ .containsExactly(addTimestamp(attentionStatus0, c), addTimestamp(attentionStatus1, c));
+ }
+
+ @Test
public void assigneeCommit() throws Exception {
Change c = newChange();
ChangeUpdate update = newUpdate(c, changeOwner);
@@ -3140,4 +3229,13 @@
update.commit();
return tr.parseBody(commit);
}
+
+ private AttentionStatus addTimestamp(AttentionStatus attentionStatus, Change c) {
+ Timestamp timestamp = newNotes(c).getChange().getLastUpdatedOn();
+ return AttentionStatus.createFromRead(
+ timestamp.toInstant(),
+ attentionStatus.account(),
+ attentionStatus.operation(),
+ attentionStatus.reason());
+ }
}
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
index bfe5ddc..bd21015 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
@@ -535,25 +535,24 @@
</div>
</section>
- <template is="dom-if" if="[[_showPrimaryTabs]]">
- <paper-tabs id="primaryTabs" on-selected-changed="_handleFileTabChange">
- <paper-tab>Files</paper-tab>
- <template is="dom-repeat" items="[[_dynamicTabHeaderEndpoints]]"
- as="tabHeader">
- <paper-tab>
- <gr-endpoint-decorator name$="[[tabHeader]]">
- <gr-endpoint-param name="change" value="[[_change]]">
- </gr-endpoint-param>
- <gr-endpoint-param name="revision" value="[[_selectedRevision]]">
- </gr-endpoint-param>
- </gr-endpoint-decorator>
- </paper-tab>
- </template>
- </paper-tabs>
- </template>
-
<section class="patchInfo">
- <div hidden$="[[!_showFileTabContent]]">
+ <template is="dom-if" if="[[_showPrimaryTabs]]">
+ <paper-tabs id="primaryTabs" on-selected-changed="_handleFileTabChange">
+ <paper-tab data-name$="[[_files_tab_name]]">Files</paper-tab>
+ <template is="dom-repeat" items="[[_dynamicTabHeaderEndpoints]]"
+ as="tabHeader">
+ <paper-tab data-name$="[[tabHeader]]">
+ <gr-endpoint-decorator name$="[[tabHeader]]">
+ <gr-endpoint-param name="change" value="[[_change]]">
+ </gr-endpoint-param>
+ <gr-endpoint-param name="revision" value="[[_selectedRevision]]">
+ </gr-endpoint-param>
+ </gr-endpoint-decorator>
+ </paper-tab>
+ </template>
+ </paper-tabs>
+ </template>
+ <div hidden$="[[!_findIfTabMatches(_currentTabName, _files_tab_name)]]">
<gr-file-list-header
id="fileListHeader"
account="[[_account]]"
@@ -604,13 +603,13 @@
on-reload-drafts="_reloadDraftsWithCallback"></gr-file-list>
</div>
- <template is="dom-if" if="[[!_showFileTabContent]]">
- <gr-endpoint-decorator name$="[[_selectedFilesTabPluginEndpoint]]">
- <gr-endpoint-param name="change" value="[[_change]]">
- </gr-endpoint-param>
- <gr-endpoint-param name="revision" value="[[_selectedRevision]]">
- </gr-endpoint-param>
- </gr-endpoint-decorator>
+ <template is="dom-if" if="[[_findIfTabMatches(_currentTabName, _selectedTabPluginHeader)]]">
+ <gr-endpoint-decorator name$="[[_selectedTabPluginEndpoint]]">
+ <gr-endpoint-param name="change" value="[[_change]]">
+ </gr-endpoint-param>
+ <gr-endpoint-param name="revision" value="[[_selectedRevision]]">
+ </gr-endpoint-param>
+ </gr-endpoint-decorator>
</template>
</section>
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
index a90fe46..b55d964 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
@@ -68,6 +68,7 @@
const CHANGE_DATA_TIMING_LABEL = 'ChangeDataLoaded';
const CHANGE_RELOAD_TIMING_LABEL = 'ChangeReloaded';
const SEND_REPLY_TIMING_LABEL = 'SendReply';
+ const FILES_TAB_NAME = 'files';
/**
* @appliesMixin Gerrit.FireMixin
@@ -322,7 +323,12 @@
_dynamicTabContentEndpoints: {
type: Array,
},
- _selectedFilesTabPluginEndpoint: {
+ // The dynamic content of the plugin added tab
+ _selectedTabPluginEndpoint: {
+ type: String,
+ },
+ // The dynamic heading of the plugin added tab
+ _selectedTabPluginHeader: {
type: String,
},
_robotCommentsPatchSetDropdownItems: {
@@ -333,6 +339,14 @@
_currentRobotCommentsPatchSet: {
type: Number,
},
+ _files_tab_name: {
+ type: String,
+ value: FILES_TAB_NAME,
+ },
+ _currentTabName: {
+ type: String,
+ value: FILES_TAB_NAME,
+ },
};
}
@@ -503,30 +517,44 @@
return currentView === view;
}
- _handleFileTabChange(e) {
- const selectedIndex = this.shadowRoot
- .querySelector('#primaryTabs').selected;
- this._showFileTabContent = selectedIndex === 0;
- // Initial tab is the static files list.
- const newSelectedTab =
- this._dynamicTabContentEndpoints[selectedIndex - 1];
- if (newSelectedTab !== this._selectedFilesTabPluginEndpoint) {
- this._selectedFilesTabPluginEndpoint = newSelectedTab;
+ _findIfTabMatches(currentTab, tab) {
+ return currentTab === tab;
+ }
- const tabName = this._selectedFilesTabPluginEndpoint || 'files';
- const source = e && e.type ? e.type : '';
- this.$.reporting.reportInteraction('tab-changed', {tabName, source});
+ _handleFileTabChange(e) {
+ const selectedIndex = e.target.selected;
+ const tabs = e.target.querySelectorAll('paper-tab');
+ this._currentTabName = tabs[selectedIndex] &&
+ tabs[selectedIndex].dataset.name;
+ const source = e && e.type ? e.type : '';
+ const pluginIndex = this._dynamicTabHeaderEndpoints.indexOf(
+ this._currentTabName);
+ if (pluginIndex !== -1) {
+ this._selectedTabPluginEndpoint = this._dynamicTabContentEndpoints[
+ pluginIndex];
+ this._selectedTabPluginHeader = this._dynamicTabHeaderEndpoints[
+ pluginIndex];
+ } else {
+ this._selectedTabPluginEndpoint = '';
+ this._selectedTabPluginHeader = '';
}
+ this.$.reporting.reportInteraction('tab-changed',
+ {tabName: this._currentTabName, source});
}
_handleShowTab(e) {
- const idx = this._dynamicTabContentEndpoints.indexOf(e.detail.tab);
+ const primaryTabs = this.shadowRoot.querySelector('#primaryTabs');
+ const tabs = primaryTabs.querySelectorAll('paper-tab');
+ let idx = -1;
+ tabs.forEach((tab, index) => {
+ if (tab.dataset.name === e.detail.tab) idx = index;
+ });
if (idx === -1) {
- console.warn(e.detail.tab + ' tab not found');
+ console.error(e.detail.tab + ' tab not found');
return;
}
- this.shadowRoot.querySelector('#primaryTabs').selected = idx + 1;
- this.shadowRoot.querySelector('#primaryTabs').scrollIntoView();
+ primaryTabs.selected = idx;
+ primaryTabs.scrollIntoView();
this.$.reporting.reportInteraction('show-tab', {tabName: e.detail.tab});
}
@@ -1442,8 +1470,14 @@
} else {
this._selectedRevision =
Object.values(this._change.revisions).find(
- revision => revision._number ===
- parseInt(this._patchRange.patchNum, 10));
+ revision => {
+ // edit patchset is a special one
+ const thePatchNum = this._patchRange.patchNum;
+ if (thePatchNum === 'edit') {
+ return revision._number === thePatchNum;
+ }
+ return revision._number === parseInt(thePatchNum, 10);
+ });
}
});
}
@@ -1935,8 +1969,12 @@
if (!this._selectedRevision) {
return;
}
- // If patchNumStr is"edit", then patchNum is undefined hence an OR
- const patchNum = parseInt(patchNumStr, 10) || patchNumStr;
+
+ let patchNum = parseInt(patchNumStr, 10);
+ if (patchNumStr === 'edit') {
+ patchNum = patchNumStr;
+ }
+
if (patchNum === this._selectedRevision._number) {
return;
}
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
index 934eeaf..3fe8faf 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
@@ -280,6 +280,20 @@
element = fixture('basic');
sandbox.stub(element.$.actions, 'reload').returns(Promise.resolve());
Gerrit._loadPlugins([]);
+ Gerrit.install(
+ plugin => {
+ plugin.registerDynamicCustomComponent(
+ 'change-view-tab-header',
+ 'gr-checks-change-view-tab-header-view'
+ );
+ plugin.registerDynamicCustomComponent(
+ 'change-view-tab-content',
+ 'gr-checks-view'
+ );
+ },
+ '0.1',
+ 'http://some/plugins/url.html'
+ );
});
teardown(done => {
@@ -305,6 +319,44 @@
assert.isTrue(replaceStateStub.called);
});
+ suite('plugins adding to file tab', () => {
+ setup(done => {
+ // Resolving it here instead of during setup() as other tests depend
+ // on flush() not being called during setup.
+ flush(() => done());
+ });
+
+ test('plugin added tab shows up as a dynamic endpoint', () => {
+ assert(element._dynamicTabHeaderEndpoints.includes(
+ 'change-view-tab-header-url'));
+ const paperTabs = element.shadowRoot.querySelector('#primaryTabs');
+ assert.equal(paperTabs.querySelectorAll('paper-tab').length, 2);
+ assert.equal(paperTabs.querySelectorAll('paper-tab')[1].dataset.name,
+ 'change-view-tab-header-url');
+ });
+
+ test('handleShowTab switched tab correctly', done => {
+ const paperTabs = element.shadowRoot.querySelector('#primaryTabs');
+ assert.equal(paperTabs.selected, 0);
+ element._handleShowTab({detail:
+ {tab: 'change-view-tab-header-url'}});
+ flush(() => {
+ assert.equal(paperTabs.selected, 1);
+ done();
+ });
+ });
+
+ test('switching tab sets _selectedTabPluginEndpoint', done => {
+ const paperTabs = element.shadowRoot.querySelector('#primaryTabs');
+ MockInteractions.tap(paperTabs.querySelectorAll('paper-tab')[1]);
+ flush(() => {
+ assert.equal(element._selectedTabPluginEndpoint,
+ 'change-view-tab-content-url');
+ done();
+ });
+ });
+ });
+
suite('keyboard shortcuts', () => {
test('t to add topic', () => {
const editStub = sandbox.stub(element.$.metadata, 'editTopic');
diff --git a/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog.html b/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog.html
index 06edb9b..c650bb5 100644
--- a/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog.html
+++ b/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog.html
@@ -76,10 +76,10 @@
</div>
<div slot="footer" class="fix-picker" hidden$="[[hasSingleFix(_fixSuggestions)]]">
<span>Suggested fix [[addOneTo(_selectedFixIdx)]] of [[_fixSuggestions.length]]</span>
- <gr-button on-click="_onPrevFixClick" disabled$="[[_noPrevFix(_selectedFixIdx)]]">
+ <gr-button id="prevFix" on-click="_onPrevFixClick" disabled$="[[_noPrevFix(_selectedFixIdx)]]">
<iron-icon icon="gr-icons:chevron-left"></iron-icon>
</gr-button>
- <gr-button on-click="_onNextFixClick" disabled$="[[_noNextFix(_selectedFixIdx)]]">
+ <gr-button id="nextFix" on-click="_onNextFixClick" disabled$="[[_noNextFix(_selectedFixIdx, _fixSuggestions)]]">
<iron-icon icon="gr-icons:chevron-right"></iron-icon>
</gr-button>
</div>
diff --git a/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog.js b/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog.js
index 033ef02..94549ee 100644
--- a/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog.js
+++ b/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog.js
@@ -140,8 +140,8 @@
_onNextFixClick(e) {
if (e) e.stopPropagation();
- if (this._selectedFixIdx < this._fixSuggestions.length &&
- this._fixSuggestions != null) {
+ if (this._fixSuggestions &&
+ this._selectedFixIdx < this._fixSuggestions.length) {
this._selectedFixIdx += 1;
return this._showSelectedFixSuggestion(
this._fixSuggestions[this._selectedFixIdx]);
@@ -152,9 +152,9 @@
return _selectedFixIdx === 0;
},
- _noNextFix(_selectedFixIdx) {
- if (this._fixSuggestions == null) return true;
- return _selectedFixIdx === this._fixSuggestions.length - 1;
+ _noNextFix(_selectedFixIdx, fixSuggestions) {
+ if (fixSuggestions == null) return true;
+ return _selectedFixIdx === fixSuggestions.length - 1;
},
_close() {
diff --git a/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog_test.html b/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog_test.html
index f9bbd88..f3f748f 100644
--- a/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog_test.html
@@ -17,7 +17,7 @@
-->
<meta name='viewport' content='width=device-width, minimum-scale=1.0, initial-scale=1.0, user-scalable=yes'>
<title>gr-apply-fix-dialog</title>
-<script src='/test/common-test-setup.js'></script>
+<link rel='import' href='../../../test/common-test-setup.html'>
<script src='/bower_components/webcomponentsjs/custom-elements-es5-adapter.js'></script>
<script src='/bower_components/webcomponentsjs/webcomponents-lite.js'></script>
@@ -40,11 +40,16 @@
await readyToTest();
let element;
let sandbox;
- const ROBOT_COMMENT = {
+ const ROBOT_COMMENT_WITH_TWO_FIXES = {
robot_id: 'robot_1',
fix_suggestions: [{fix_id: 'fix_1'}, {fix_id: 'fix_2'}],
};
+ const ROBOT_COMMENT_WITH_ONE_FIX = {
+ robot_id: 'robot_1',
+ fix_suggestions: [{fix_id: 'fix_1'}],
+ };
+
setup(() => {
sandbox = sinon.sandbox.create();
element = fixture('basic');
@@ -101,7 +106,7 @@
}));
sandbox.stub(element.$.applyFixOverlay, 'open').returns(Promise.resolve());
- element.open({detail: {patchNum: 2, comment: ROBOT_COMMENT}})
+ element.open({detail: {patchNum: 2, comment: ROBOT_COMMENT_WITH_TWO_FIXES}})
.then(() => {
assert.equal(element._currentFix.fix_id, 'fix_1');
assert.equal(element._currentPreviews.length, 2);
@@ -110,6 +115,22 @@
});
});
+ test('next button state updated when suggestions changed', done => {
+ sandbox.stub(element.$.restAPI, 'getRobotCommentFixPreview')
+ .returns(Promise.resolve({}));
+ sandbox.stub(element.$.applyFixOverlay, 'open').returns(Promise.resolve());
+
+ element.open({detail: {patchNum: 2, comment: ROBOT_COMMENT_WITH_ONE_FIX}})
+ .then(() => assert.isTrue(element.$.nextFix.disabled))
+ .then(() =>
+ element.open({detail: {patchNum: 2,
+ comment: ROBOT_COMMENT_WITH_TWO_FIXES}}))
+ .then(() => {
+ assert.isFalse(element.$.nextFix.disabled);
+ done();
+ });
+ });
+
test('preview endpoint throws error should reset dialog', done => {
sandbox.stub(window, 'fetch', (url => {
if (url.endsWith('/preview')) {
@@ -123,7 +144,8 @@
}));
const errorStub = sinon.stub();
document.addEventListener('network-error', errorStub);
- element.open({detail: {patchNum: 2, comment: ROBOT_COMMENT}});
+ element.open({detail: {patchNum: 2,
+ comment: ROBOT_COMMENT_WITH_TWO_FIXES}});
flush(() => {
assert.isTrue(errorStub.called);
assert.deepEqual(element._currentFix, {});
@@ -189,7 +211,7 @@
}));
sandbox.stub(element.$.applyFixOverlay, 'open').returns(Promise.resolve());
- element.open({detail: {patchNum: 2, comment: ROBOT_COMMENT}})
+ element.open({detail: {patchNum: 2, comment: ROBOT_COMMENT_WITH_TWO_FIXES}})
.then(() => {
element._onNextFixClick();
assert.equal(element._currentFix.fix_id, 'fix_2');
diff --git a/polygerrit-ui/app/elements/shared/gr-event-interface/gr-event-interface_test.html b/polygerrit-ui/app/elements/shared/gr-event-interface/gr-event-interface_test.html
index 6e62b15..305702a 100644
--- a/polygerrit-ui/app/elements/shared/gr-event-interface/gr-event-interface_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-event-interface/gr-event-interface_test.html
@@ -19,7 +19,7 @@
<meta name="viewport" content="width=device-width, minimum-scale=1.0, initial-scale=1.0, user-scalable=yes">
<title>gr-api-interface</title>
-<script src="../../../bower_components/webcomponentsjs/webcomponents-lite.min.js"></script>
+<script src="../../../bower_components/webcomponentsjs/webcomponents-lite.js"></script>
<script src="../../../bower_components/web-component-tester/browser.js"></script>
<script src="../../../test/test-pre-setup.js"></script>
<link rel="import" href="../../../test/common-test-setup.html"/>
diff --git a/polygerrit-ui/app/scripts/gr-email-suggestions-provider/gr-email-suggestions-provider_test.html b/polygerrit-ui/app/scripts/gr-email-suggestions-provider/gr-email-suggestions-provider_test.html
index 5fad3f4..f64d9ef 100644
--- a/polygerrit-ui/app/scripts/gr-email-suggestions-provider/gr-email-suggestions-provider_test.html
+++ b/polygerrit-ui/app/scripts/gr-email-suggestions-provider/gr-email-suggestions-provider_test.html
@@ -23,8 +23,8 @@
<script src="/bower_components/webcomponentsjs/webcomponents-lite.js"></script>
<script src="/bower_components/web-component-tester/browser.js"></script>
-<script src="../../../test/test-pre-setup.js"></script>
-<link rel="import" href="../../../test/common-test-setup.html"/>
+<script src="../../test/test-pre-setup.js"></script>
+<link rel="import" href="../../test/common-test-setup.html"/>
<link rel="import" href="../../elements/shared/gr-rest-api-interface/gr-rest-api-interface.html"/>
<script src="../gr-display-name-utils/gr-display-name-utils.js"></script>
<script src="gr-email-suggestions-provider.js"></script>
diff --git a/polygerrit-ui/app/scripts/gr-group-suggestions-provider/gr-group-suggestions-provider_test.html b/polygerrit-ui/app/scripts/gr-group-suggestions-provider/gr-group-suggestions-provider_test.html
index 7d89c2a..c46b443 100644
--- a/polygerrit-ui/app/scripts/gr-group-suggestions-provider/gr-group-suggestions-provider_test.html
+++ b/polygerrit-ui/app/scripts/gr-group-suggestions-provider/gr-group-suggestions-provider_test.html
@@ -23,8 +23,8 @@
<script src="/bower_components/webcomponentsjs/webcomponents-lite.js"></script>
<script src="/bower_components/web-component-tester/browser.js"></script>
-<script src="../../../test/test-pre-setup.js"></script>
-<link rel="import" href="../../../test/common-test-setup.html"/>
+<script src="../../test/test-pre-setup.js"></script>
+<link rel="import" href="../../test/common-test-setup.html"/>
<link rel="import" href="../../elements/shared/gr-rest-api-interface/gr-rest-api-interface.html"/>
<script src="../gr-display-name-utils/gr-display-name-utils.js"></script>
<script src="gr-group-suggestions-provider.js"></script>
diff --git a/polygerrit-ui/app/scripts/gr-reviewer-suggestions-provider/gr-reviewer-suggestions-provider_test.html b/polygerrit-ui/app/scripts/gr-reviewer-suggestions-provider/gr-reviewer-suggestions-provider_test.html
index af7b4bd..9696c2e 100644
--- a/polygerrit-ui/app/scripts/gr-reviewer-suggestions-provider/gr-reviewer-suggestions-provider_test.html
+++ b/polygerrit-ui/app/scripts/gr-reviewer-suggestions-provider/gr-reviewer-suggestions-provider_test.html
@@ -23,8 +23,8 @@
<script src="/bower_components/webcomponentsjs/webcomponents-lite.js"></script>
<script src="/bower_components/web-component-tester/browser.js"></script>
-<script src="../../../test/test-pre-setup.js"></script>
-<link rel="import" href="../../../test/common-test-setup.html"/>
+<script src="../../test/test-pre-setup.js"></script>
+<link rel="import" href="../../test/common-test-setup.html"/>
<link rel="import" href="../../elements/shared/gr-rest-api-interface/gr-rest-api-interface.html"/>
<script src="../gr-display-name-utils/gr-display-name-utils.js"></script>
<script src="gr-reviewer-suggestions-provider.js"></script>
diff --git a/proto/cache.proto b/proto/cache.proto
index 8f73388..b881e3d 100644
--- a/proto/cache.proto
+++ b/proto/cache.proto
@@ -75,7 +75,7 @@
// Instead, we just take the tedious yet simple approach of having a "has_foo"
// field for each nullable field "foo", indicating whether or not foo is null.
//
-// Next ID: 23
+// Next ID: 24
message ChangeNotesStateProto {
// Effectively required, even though the corresponding ChangeNotesState field
// is optional, since the field is only absent when NoteDb is disabled, in
@@ -133,7 +133,7 @@
// which case attempting to use the ChangeNotesCache is programmer error.
ChangeColumnsProto columns = 3;
- reserved 4; // past_assignee
+ reserved 4; // past_assignee
repeated string hashtag = 5;
@@ -167,6 +167,7 @@
// Next ID: 5
message ReviewerStatusUpdateProto {
+ // Epoch millis.
int64 date = 1;
int32 updated_by = 2;
int32 reviewer = 3;
@@ -194,14 +195,26 @@
bool has_server_id = 21;
message AssigneeStatusUpdateProto {
+ // Epoch millis.
int64 date = 1;
int32 updated_by = 2;
int32 current_assignee = 3;
bool has_current_assignee = 4;
}
repeated AssigneeStatusUpdateProto assignee_update = 22;
-}
+ // An update to the attention set of the change. See class AttentionStatus for
+ // context.
+ message AttentionStatusProto {
+ // Epoch millis.
+ int64 date = 1;
+ int32 account = 2;
+ // Maps to enum AttentionStatus.Operation
+ string operation = 3;
+ string reason = 4;
+ }
+ repeated AttentionStatusProto attention_status = 23;
+}
// Serialized form of com.google.gerrit.server.query.change.ConflictKey
message ConflictKeyProto {
