PostReviewOp: check all comments for stream events, including drafts
When checking what type of comments notification to perform to
stream-events, consider *ALL* published comments and not just the
ones passed as input.
This allows to fix a regression in v3.7 where the GUI stores
the patchset-level comments first as drafts and then publishes them.
Bug: Issue 16475
Release-Notes: Fix stream events message including comments created as draft
Change-Id: I2ac40bd8235fea5c0059e8ca4111d3b6bf266f5a
diff --git a/java/com/google/gerrit/server/restapi/change/PostReviewOp.java b/java/com/google/gerrit/server/restapi/change/PostReviewOp.java
index 5ff0968..9274f52 100644
--- a/java/com/google/gerrit/server/restapi/change/PostReviewOp.java
+++ b/java/com/google/gerrit/server/restapi/change/PostReviewOp.java
@@ -193,14 +193,14 @@
// 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);
- }
- }
+ String patchSetLevelComment =
+ comments.stream()
+ .filter(c -> c.key.filename.equals(PATCHSET_LEVEL))
+ .map(c -> Strings.nullToEmpty(c.message))
+ .collect(Collectors.joining("\n"))
+ .trim();
+ if (!patchSetLevelComment.isEmpty()) {
+ comment = String.format("Patch Set %s:\n\n%s", psId.get(), patchSetLevelComment);
}
}
commentAdded.fire(
diff --git a/javatests/com/google/gerrit/acceptance/ssh/StreamEventsIT.java b/javatests/com/google/gerrit/acceptance/ssh/StreamEventsIT.java
index 13a9e0c..80b8ff0 100644
--- a/javatests/com/google/gerrit/acceptance/ssh/StreamEventsIT.java
+++ b/javatests/com/google/gerrit/acceptance/ssh/StreamEventsIT.java
@@ -15,6 +15,7 @@
package com.google.gerrit.acceptance.ssh;
import static com.google.gerrit.acceptance.WaitUtil.waitUntil;
+import static com.google.gerrit.entities.Patch.PATCHSET_LEVEL;
import com.google.common.base.Splitter;
import com.google.gerrit.acceptance.AbstractDaemonTest;
@@ -22,13 +23,17 @@
import com.google.gerrit.acceptance.Sandboxed;
import com.google.gerrit.acceptance.UseSsh;
import com.google.gerrit.extensions.api.changes.ChangeApi;
+import com.google.gerrit.extensions.api.changes.DraftInput;
import com.google.gerrit.extensions.api.changes.ReviewInput;
+import com.google.gerrit.extensions.api.changes.ReviewInput.DraftHandling;
+import com.google.gerrit.server.query.change.ChangeData;
import java.io.IOException;
import java.io.Reader;
import java.time.Duration;
import java.util.List;
import java.util.function.Supplier;
import java.util.stream.Collectors;
+import java.util.stream.Stream;
import java.util.stream.StreamSupport;
import org.junit.After;
import org.junit.Before;
@@ -40,7 +45,9 @@
public class StreamEventsIT extends AbstractDaemonTest {
private static final Duration MAX_DURATION_FOR_RECEIVING_EVENTS = Duration.ofSeconds(2);
private static final String TEST_REVIEW_COMMENT = "any comment";
+ private static final String TEST_REVIEW_DRAFT_COMMENT = "any draft comment";
private Reader streamEventsReader;
+ private ChangeData change;
@Before
public void setup() throws Exception {
@@ -59,6 +66,23 @@
}
@Test
+ public void publishedDraftPatchSetLevelCommentShowsUpInStreamEvents() throws Exception {
+ change = createChange().getChange();
+
+ String firstDraftComment = String.format("%s 1", TEST_REVIEW_DRAFT_COMMENT);
+ String secondDraftComment = String.format("%s 2", TEST_REVIEW_DRAFT_COMMENT);
+
+ draftReviewChange(PATCHSET_LEVEL, firstDraftComment);
+ draftReviewChange(PATCHSET_LEVEL, secondDraftComment);
+ publishDraftReviews();
+
+ waitForEvent(
+ () ->
+ pollEventsContaining("comment-added", firstDraftComment, secondDraftComment).size()
+ == 1);
+ }
+
+ @Test
public void batchRefsUpdatedShowSeparatelyInStreamEvents() throws Exception {
String refName = createChange().getChange().currentPatchSet().refName();
waitForEvent(
@@ -77,7 +101,22 @@
changeApi.current().review(reviewInput);
}
- private List<String> pollEventsContaining(String eventType, String expectedContent) {
+ private void draftReviewChange(String path, String reviewMessage) throws Exception {
+ DraftInput draftInput = new DraftInput();
+ draftInput.message = reviewMessage;
+ draftInput.path = path;
+ ChangeApi changeApi = gApi.changes().id(change.getId().get());
+ changeApi.current().createDraft(draftInput).get();
+ }
+
+ private void publishDraftReviews() throws Exception {
+ ReviewInput reviewInput = new ReviewInput();
+ reviewInput.tag = "new_tag";
+ reviewInput.drafts = DraftHandling.PUBLISH;
+ gApi.changes().id(change.getId().get()).current().review(reviewInput);
+ }
+
+ private List<String> pollEventsContaining(String eventType, String... expectedContent) {
try {
char[] cbuf = new char[2048];
StringBuilder eventsOutput = new StringBuilder();
@@ -90,7 +129,7 @@
.filter(
event ->
event.contains(String.format("\"type\":\"%s\"", eventType))
- && event.contains(expectedContent))
+ && Stream.of(expectedContent).allMatch(event::contains))
.collect(Collectors.toList());
} catch (IOException e) {
throw new IllegalStateException(e);
