Expose patch set level comment in stream event
Change message comment is now published as patch set level comment.
However, comment-added event wasn't extended to reflect this transition.
As the consequence some CI integrations, most notably Jenkins Gerrit
Trigger plugin and Zuul, are missing the comment content and thus the
build cannot be re-triggered with "recheck" or "reverify" change message
comment.
To rectify, introduce new configuration option that is enabled per
default to publish patch set level comment.
Bug: Issue 13800
Change-Id: I668029af2d971c88f157b237bc76b9878e751579
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index 0e7c4ec..0f4e319 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -3358,6 +3358,18 @@
config is backwards compatible with what the default was before the config
was added.
+[[event.comment-added.publishPatchSetLevelComment]][event.comment-added.publishPatchSetLevelComment::
++
+Add patch set level comment as event comment. Without this option, patch set
+level comment will not be included in the event comment attribute. Given that
+currently patch set level, file and robot comments are not exposed in the
+`comment-added` event type, those comments will be lost. One particular use
+case is to re-trigger CI build from the change screen by adding a comment with
+specific content, e.g.: `recheck`. Jenkins Gerrit Trigger plugin and Zuul CI
+depend on this feature to trigger change verification.
++
+By default, true.
+
[[experiments]]
=== Section experiments
diff --git a/java/com/google/gerrit/extensions/api/changes/ReviewInput.java b/java/com/google/gerrit/extensions/api/changes/ReviewInput.java
index 7ecc0a6..fd445b6 100644
--- a/java/com/google/gerrit/extensions/api/changes/ReviewInput.java
+++ b/java/com/google/gerrit/extensions/api/changes/ReviewInput.java
@@ -21,9 +21,11 @@
import com.google.gerrit.extensions.common.FixSuggestionInfo;
import com.google.gerrit.extensions.restapi.DefaultInput;
import java.util.ArrayList;
+import java.util.Collections;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
+import java.util.Objects;
/** Input passed to {@code POST /changes/[id]/revisions/[id]/review}. */
public class ReviewInput {
@@ -117,6 +119,15 @@
return this;
}
+ public ReviewInput patchSetLevelComment(String message) {
+ Objects.requireNonNull(message);
+ CommentInput comment = new CommentInput();
+ comment.message = message;
+ // TODO(davido): Because of cyclic dependency, we cannot use here Patch.PATCHSET_LEVEL constant
+ comments = Collections.singletonMap("/PATCHSET_LEVEL", Collections.singletonList(comment));
+ return this;
+ }
+
public ReviewInput label(String name, short value) {
if (name == null || name.isEmpty()) {
throw new IllegalArgumentException();
diff --git a/java/com/google/gerrit/server/restapi/change/PostReview.java b/java/com/google/gerrit/server/restapi/change/PostReview.java
index 604c87f..575a19d 100644
--- a/java/com/google/gerrit/server/restapi/change/PostReview.java
+++ b/java/com/google/gerrit/server/restapi/change/PostReview.java
@@ -179,6 +179,7 @@
private final PluginSetContext<CommentValidator> commentValidators;
private final ReplyAttentionSetUpdates replyAttentionSetUpdates;
private final boolean strictLabels;
+ private final boolean publishPatchSetLevelComment;
@Inject
PostReview(
@@ -224,6 +225,8 @@
this.commentValidators = commentValidators;
this.replyAttentionSetUpdates = replyAttentionSetUpdates;
this.strictLabels = gerritConfig.getBoolean("change", "strictLabels", false);
+ this.publishPatchSetLevelComment =
+ gerritConfig.getBoolean("event", "comment-added", "publishPatchSetLevelComment", true);
}
@Override
@@ -941,14 +944,23 @@
String.format("Repository %s not found", ctx.getProject().get()), ex);
}
}
+ String comment = message.getMessage();
+ if (publishPatchSetLevelComment) {
+ // TODO(davido): Remove this workaround when patch set level comments are exposed in comment
+ // added event. For backwards compatibility, patchset level comment has a higher priority
+ // than change message and should be used as comment in comment added event.
+ if (in.comments != null && in.comments.containsKey(PATCHSET_LEVEL)) {
+ List<CommentInput> patchSetLevelComments = in.comments.get(PATCHSET_LEVEL);
+ if (patchSetLevelComments != null && !patchSetLevelComments.isEmpty()) {
+ CommentInput firstComment = patchSetLevelComments.get(0);
+ if (!Strings.isNullOrEmpty(firstComment.message)) {
+ comment = String.format("Patch Set %s:\n\n%s", psId.get(), firstComment.message);
+ }
+ }
+ }
+ }
commentAdded.fire(
- notes.getChange(),
- ps,
- user.state(),
- message.getMessage(),
- approvals,
- oldApprovals,
- ctx.getWhen());
+ notes.getChange(), ps, user.state(), comment, approvals, oldApprovals, ctx.getWhen());
}
private boolean insertComments(ChangeContext ctx, List<RobotComment> newRobotComments)
diff --git a/javatests/com/google/gerrit/acceptance/server/event/CommentAddedEventIT.java b/javatests/com/google/gerrit/acceptance/server/event/CommentAddedEventIT.java
index 002b860..fb3259f 100644
--- a/javatests/com/google/gerrit/acceptance/server/event/CommentAddedEventIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/event/CommentAddedEventIT.java
@@ -26,6 +26,7 @@
import com.google.gerrit.acceptance.ExtensionRegistry.Registration;
import com.google.gerrit.acceptance.NoHttpd;
import com.google.gerrit.acceptance.PushOneCommit;
+import com.google.gerrit.acceptance.config.GerritConfig;
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
import com.google.gerrit.entities.LabelType;
import com.google.gerrit.extensions.api.changes.ReviewInput;
@@ -201,6 +202,25 @@
assertThat(attr.value).isEqualTo(-1);
assertThat(listener.getLastCommentAddedEvent().getComment())
.isEqualTo(String.format("Patch Set 1:\n\n%s", label.getName()));
+
+ // review with patch set level comment
+ reviewInput = new ReviewInput().patchSetLevelComment("a patch set level comment");
+ revision(r).review(reviewInput);
+ assertThat(listener.getLastCommentAddedEvent().getComment())
+ .isEqualTo(String.format("Patch Set 1:\n\n%s", "a patch set level comment"));
+ }
+ }
+
+ @Test
+ @GerritConfig(name = "event.comment-added.publishPatchSetLevelComment", value = "false")
+ public void publishPatchSetLevelComment() throws Exception {
+ PushOneCommit.Result r = createChange();
+ TestListener listener = new TestListener();
+ try (Registration registration = extensionRegistry.newRegistration().add(listener)) {
+ ReviewInput reviewInput = new ReviewInput().patchSetLevelComment("a patch set level comment");
+ revision(r).review(reviewInput);
+ assertThat(listener.getLastCommentAddedEvent().getComment())
+ .isEqualTo(String.format("Patch Set 1:\n\n%s", "(1 comment)"));
}
}
