From 322bf38e5ca1d4c011674a28fafb1c583dbbb7a0 Mon Sep 17 00:00:00 2001
From: Alice Kober-Sotzek <aliceks@google.com>
Date: Thu, 13 Aug 2020 20:47:58 +0200
Subject: [PATCH] Fix: Also copy parentUuid in copy constructor of Comment

The copy constructor is currently only used by the code which updates
a draft comment. Interestingly, nobody seemed to have noticed that this
REST API actually had a bug and the parentUuid was lost when the draft
comment was updated. Users of Gerrit's API didn't run into this bug
as the frontend always sends all comment parameters as input to the
draft update along.

By the way, this issue was noticed as a side-effect of testing the
implementation of another new feature (ported comments, see I4e96af5ac).
Writing even seemingly trivial tests (e.g. that the parentUuid of a
ported comment is kept as in the original) seemed to have paid off in
this case.

Change-Id: I75a65bd57fb71cb8efe9d2460bc210b557546165
---
 java/com/google/gerrit/entities/Comment.java  |  1 +
 .../acceptance/server/change/CommentsIT.java  | 38 +++++++++++++++++++
 2 files changed, 39 insertions(+)

diff --git a/java/com/google/gerrit/entities/Comment.java b/java/com/google/gerrit/entities/Comment.java
index 2c10c87408..e7ac4e44ef 100644
--- a/java/com/google/gerrit/entities/Comment.java
+++ b/java/com/google/gerrit/entities/Comment.java
@@ -244,6 +244,7 @@ public abstract class Comment {
         c.unresolved);
     this.lineNbr = c.lineNbr;
     this.realAuthor = c.realAuthor;
+    this.parentUuid = c.parentUuid;
     this.range = c.range != null ? new Range(c.range) : null;
     this.tag = c.tag;
     this.revId = c.revId;
diff --git a/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java b/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java
index 7faae6bceb..45e14178c4 100644
--- a/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java
@@ -31,11 +31,16 @@ import com.google.common.collect.Lists;
 import com.google.gerrit.acceptance.AbstractDaemonTest;
 import com.google.gerrit.acceptance.NoHttpd;
 import com.google.gerrit.acceptance.PushOneCommit;
+import com.google.gerrit.acceptance.testsuite.account.AccountOperations;
+import com.google.gerrit.acceptance.testsuite.change.ChangeOperations;
+import com.google.gerrit.acceptance.testsuite.change.TestHumanComment;
 import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
 import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
+import com.google.gerrit.entities.Account;
 import com.google.gerrit.entities.Change;
 import com.google.gerrit.entities.HumanComment;
 import com.google.gerrit.entities.Patch;
+import com.google.gerrit.entities.PatchSet;
 import com.google.gerrit.entities.RefNames;
 import com.google.gerrit.extensions.api.changes.DeleteCommentInput;
 import com.google.gerrit.extensions.api.changes.DraftInput;
@@ -90,6 +95,8 @@ public class CommentsIT extends AbstractDaemonTest {
   @Inject private ProjectOperations projectOperations;
   @Inject private Provider<ChangesCollection> changes;
   @Inject private Provider<PostReview> postReview;
+  @Inject private ChangeOperations changeOperations;
+  @Inject private AccountOperations accountOperations;
   @Inject private RequestScopeOperations requestScopeOperations;
   @Inject private CommentsUtil commentsUtil;
   @Inject private TestCommentHelper testCommentHelper;
@@ -726,6 +733,37 @@ public class CommentsIT extends AbstractDaemonTest {
     assertThat(comment.tag).isEqualTo(tag);
   }
 
+  @Test
+  public void updatedDraftStillPointsToParentComment() throws Exception {
+    Account.Id accountId = accountOperations.newAccount().create();
+    Change.Id changeId = changeOperations.newChange().create();
+    PatchSet.Id patchsetId = changeOperations.change(changeId).currentPatchset().get().patchsetId();
+    String parentCommentUuid =
+        changeOperations.change(changeId).patchset(patchsetId).newComment().create();
+    String draftCommentUuid =
+        changeOperations
+            .change(changeId)
+            .patchset(patchsetId)
+            .newDraftComment()
+            .parentUuid(parentCommentUuid)
+            .author(accountId)
+            .create();
+
+    // Each user can only see their own drafts.
+    requestScopeOperations.setApiUser(accountId);
+    DraftInput draftInput = newDraft(FILE_NAME, Side.REVISION, 0, "bar");
+    draftInput.message = "Another comment text.";
+    gApi.changes()
+        .id(changeId.get())
+        .revision(patchsetId.get())
+        .draft(draftCommentUuid)
+        .update(draftInput);
+
+    TestHumanComment comment =
+        changeOperations.change(changeId).draftComment(draftCommentUuid).get();
+    assertThat(comment.parentUuid()).hasValue(parentCommentUuid);
+  }
+
   @Test
   public void listDrafts() throws Exception {
     String file = "file";