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
This commit is contained in:
David Ostrovsky
2020-12-09 19:59:55 +01:00
committed by Luca Milanesio
parent b933eda641
commit d14db1f2cc
4 changed files with 62 additions and 7 deletions

View File

@@ -3358,6 +3358,18 @@ Defaults to all available options minus `CHANGE_ACTIONS`,
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

View File

@@ -21,9 +21,11 @@ import com.google.gerrit.extensions.client.ReviewerState;
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 @@ public class ReviewInput {
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();

View File

@@ -179,6 +179,7 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
private final PluginSetContext<CommentValidator> commentValidators;
private final ReplyAttentionSetUpdates replyAttentionSetUpdates;
private final boolean strictLabels;
private final boolean publishPatchSetLevelComment;
@Inject
PostReview(
@@ -224,6 +225,8 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
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 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
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)

View File

@@ -26,6 +26,7 @@ import com.google.gerrit.acceptance.ExtensionRegistry;
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 @@ public class CommentAddedEventIT extends AbstractDaemonTest {
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)"));
}
}