commit 7d890be7b70a0f56cdc37a8793aada16efa0caf6 Author: Alice Kober-Sotzek Date: Thu Oct 1 17:41:56 2020 +0200 Remove 'unresolved' parameter from robot comments As Ied977dbac shows, the 'unresolved' field is unused for robot comments. It's very likely that it was never meant for use by robot comments and was just unintentionally inherited from human comments. Besides the field not being copied in PostReview.Op [1], which one could argue was just an oversight, the constructor of the class RobotComment passes a hard-coded 'false' for the 'unresolved' field [2]. In addition, we never added behavior for unresolved robot comments to the frontend. They would just appear as other robot comments to users (e.g. have the same color). To not confuse future developers and users of Gerrit's API, we remove this unused functionality. Before anybody complains that there might be a use case for which the 'unresolved' field of robot comments would be helpful: We can always add that field back. However, we should carefully consider whether other concepts aren't better suited than such a boolean field. For instance, a severity parameter for robot comments might make much more sense (e.g. to distinguish between informational-only and more severe robot comments). There's one location where we couldn't completely remove the 'unresolved' parameter: RobotCommentInfo inherits from CommentInfo and we don't have another class which could be used as super class. For robot comments, that field will always be unset, though, and hence it will never appear in the REST API output. If we want to make this clearer in the code, we should clean up the hierarchy in a future change. Moving the field 'unresolved' from Comment to HumanComment doesn't make a difference for the NoteDb storage format. For this reason, this change shouldn't break existing comments. A local, manual test with existing human comments succeeded. [1] https://gerrit.googlesource.com/gerrit/+/6d90ff931199fb3bf3d65b876d3dd9efc27bf2a0/java/com/google/gerrit/server/restapi/change/PostReview.java#1103 [2] https://gerrit.googlesource.com/gerrit/+/6d90ff931199fb3bf3d65b876d3dd9efc27bf2a0/java/com/google/gerrit/entities/RobotComment.java#38 Change-Id: If4e1d76a981f9fedde822238a20c8f18da6d7b25 diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt index f25e27d..061cc7d 100644 --- a/Documentation/rest-api-changes.txt +++ b/Documentation/rest-api-changes.txt @@ -7809,7 +7809,8 @@ selector menu. May be null if no description is set. The `RobotCommentInfo` entity contains information about a robot inline comment. -`RobotCommentInfo` has the same fields as <>. +`RobotCommentInfo` has the same fields as <> +except for the `unresolved` field which doesn't exist for robot comments. In addition `RobotCommentInfo` has the following fields: [options="header",cols="1,^1,5"] @@ -7829,8 +7830,35 @@ keys to values. The `RobotCommentInput` entity contains information for creating an inline robot comment. -`RobotCommentInput` has the same fields as -<>. +[options="header",cols="1,^1,5"] +|=========================== +|Field Name ||Description +|`path` || +link:#file-id[The file path] for which the inline comment should be added. +|`side` |optional| +The side on which the comment should be added. + +Allowed values are `REVISION` and `PARENT`. + +If not set, the default is `REVISION`. +|`line` |optional| +The number of the line for which the comment should be added. + +`0` if it is a file comment. + +If neither line nor range is set, a file comment is added. + +If range is set, this value is ignored in favor of the `end_line` of the range. +|`range` |optional| +The range of the comment as a link:#comment-range[CommentRange] +entity. +|`in_reply_to` |optional| +The URL encoded UUID of the comment to which this comment is a reply. +|`message` |optional| +The comment message. +|`robot_id` ||The ID of the robot that generated this comment. +|`robot_run_id` ||An ID of the run of the robot. +|`url` |optional|URL to more information. +|`properties` |optional|Robot specific properties as map that maps arbitrary +keys to values. +|`fix_suggestions`|optional|Suggested fixes for this robot comment as a list of +<> entities. +|=========================== [[rule-input]] === RuleInput diff --git a/java/com/google/gerrit/acceptance/testsuite/change/PerPatchsetOperationsImpl.java b/java/com/google/gerrit/acceptance/testsuite/change/PerPatchsetOperationsImpl.java index b8c841c..eda6c7e 100644 --- a/java/com/google/gerrit/acceptance/testsuite/change/PerPatchsetOperationsImpl.java +++ b/java/com/google/gerrit/acceptance/testsuite/change/PerPatchsetOperationsImpl.java @@ -259,9 +259,6 @@ public class PerPatchsetOperationsImpl implements PerPatchsetOperations { .ifPresent(range -> newRobotComment.setLineNbrAndRange(null, range)); robotCommentCreation - .unresolved() - .ifPresent(unresolved -> newRobotComment.unresolved = unresolved); - robotCommentCreation .parentUuid() .ifPresent(parentUuid -> newRobotComment.parentUuid = parentUuid); robotCommentCreation.url().ifPresent(url -> newRobotComment.url = url); diff --git a/java/com/google/gerrit/acceptance/testsuite/change/TestRobotCommentCreation.java b/java/com/google/gerrit/acceptance/testsuite/change/TestRobotCommentCreation.java index 809190d..558af3f 100644 --- a/java/com/google/gerrit/acceptance/testsuite/change/TestRobotCommentCreation.java +++ b/java/com/google/gerrit/acceptance/testsuite/change/TestRobotCommentCreation.java @@ -41,8 +41,6 @@ public abstract class TestRobotCommentCreation { public abstract Optional side(); - public abstract Optional unresolved(); - public abstract Optional parentUuid(); public abstract Optional tag(); @@ -152,18 +150,6 @@ public abstract class TestRobotCommentCreation { abstract Builder side(CommentSide side); - /** Indicates a resolved comment. */ - public Builder resolved() { - return unresolved(false); - } - - /** Indicates an unresolved comment. */ - public Builder unresolved() { - return unresolved(true); - } - - abstract Builder unresolved(boolean unresolved); - /** * UUID of another comment to which this comment is a reply. This comment must have similar * attributes (e.g. file, line, side) as the parent comment. The parent comment must be a diff --git a/java/com/google/gerrit/entities/Comment.java b/java/com/google/gerrit/entities/Comment.java index e7ac4e4..37b8620 100644 --- a/java/com/google/gerrit/entities/Comment.java +++ b/java/com/google/gerrit/entities/Comment.java @@ -231,7 +231,6 @@ public abstract class Comment { private String revId; public String serverId; - public boolean unresolved; public Comment(Comment c) { this( @@ -240,15 +239,13 @@ public abstract class Comment { new Timestamp(c.writtenOn.getTime()), c.side, c.message, - c.serverId, - c.unresolved); + c.serverId); 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; - this.unresolved = c.unresolved; } public Comment( @@ -257,8 +254,7 @@ public abstract class Comment { Timestamp writtenOn, short side, String message, - String serverId, - boolean unresolved) { + String serverId) { this.key = key; this.author = new Comment.Identity(author); this.realAuthor = this.author; @@ -266,7 +262,6 @@ public abstract class Comment { this.side = side; this.message = message; this.serverId = serverId; - this.unresolved = unresolved; } public void setLineNbrAndRange( @@ -334,8 +329,7 @@ public abstract class Comment { && Objects.equals(range, c.range) && Objects.equals(tag, c.tag) && Objects.equals(revId, c.revId) - && Objects.equals(serverId, c.serverId) - && unresolved == c.unresolved; + && Objects.equals(serverId, c.serverId); } @Override @@ -352,8 +346,7 @@ public abstract class Comment { range, tag, revId, - serverId, - unresolved); + serverId); } @Override @@ -373,7 +366,6 @@ public abstract class Comment { .add("parentUuid", Objects.toString(parentUuid, "")) .add("range", Objects.toString(range, "")) .add("revId", Objects.toString(revId, "")) - .add("tag", Objects.toString(tag, "")) - .add("unresolved", unresolved); + .add("tag", Objects.toString(tag, "")); } } diff --git a/java/com/google/gerrit/entities/HumanComment.java b/java/com/google/gerrit/entities/HumanComment.java index 8b687cc..50bee8d 100644 --- a/java/com/google/gerrit/entities/HumanComment.java +++ b/java/com/google/gerrit/entities/HumanComment.java @@ -15,6 +15,7 @@ package com.google.gerrit.entities; import java.sql.Timestamp; +import java.util.Objects; /** * This class represents inline human comments in NoteDb. This means it determines the JSON format @@ -27,6 +28,8 @@ import java.sql.Timestamp; */ public class HumanComment extends Comment { + public boolean unresolved; + public HumanComment( Key key, Account.Id author, @@ -35,7 +38,8 @@ public class HumanComment extends Comment { String message, String serverId, boolean unresolved) { - super(key, author, writtenOn, side, message, serverId, unresolved); + super(key, author, writtenOn, side, message, serverId); + this.unresolved = unresolved; } public HumanComment(HumanComment comment) { @@ -49,19 +53,23 @@ public class HumanComment extends Comment { @Override public String toString() { - return toStringHelper().toString(); + return toStringHelper().add("unresolved", unresolved).toString(); } @Override - public boolean equals(Object o) { - if (!(o instanceof HumanComment)) { + public boolean equals(Object otherObject) { + if (!(otherObject instanceof HumanComment)) { + return false; + } + if (!super.equals(otherObject)) { return false; } - return super.equals(o); + HumanComment otherComment = (HumanComment) otherObject; + return unresolved == otherComment.unresolved; } @Override public int hashCode() { - return super.hashCode(); + return Objects.hash(super.hashCode(), unresolved); } } diff --git a/java/com/google/gerrit/entities/RobotComment.java b/java/com/google/gerrit/entities/RobotComment.java index 03ddad5..e2e4114 100644 --- a/java/com/google/gerrit/entities/RobotComment.java +++ b/java/com/google/gerrit/entities/RobotComment.java @@ -35,7 +35,7 @@ public final class RobotComment extends Comment { String serverId, String robotId, String robotRunId) { - super(key, author, writtenOn, side, message, serverId, false); + super(key, author, writtenOn, side, message, serverId); this.robotId = robotId; this.robotRunId = robotRunId; } diff --git a/java/com/google/gerrit/extensions/api/changes/DraftInput.java b/java/com/google/gerrit/extensions/api/changes/DraftInput.java index b3c2786..74f626f 100644 --- a/java/com/google/gerrit/extensions/api/changes/DraftInput.java +++ b/java/com/google/gerrit/extensions/api/changes/DraftInput.java @@ -19,18 +19,19 @@ import java.util.Objects; public class DraftInput extends Comment { public String tag; + public Boolean unresolved; @Override public boolean equals(Object o) { if (super.equals(o)) { DraftInput di = (DraftInput) o; - return Objects.equals(tag, di.tag); + return Objects.equals(tag, di.tag) && Objects.equals(unresolved, di.unresolved); } return false; } @Override public int hashCode() { - return Objects.hash(super.hashCode(), tag); + return Objects.hash(super.hashCode(), tag, unresolved); } } diff --git a/java/com/google/gerrit/extensions/api/changes/ReviewInput.java b/java/com/google/gerrit/extensions/api/changes/ReviewInput.java index a213ed6..7ecc0a6 100644 --- a/java/com/google/gerrit/extensions/api/changes/ReviewInput.java +++ b/java/com/google/gerrit/extensions/api/changes/ReviewInput.java @@ -100,9 +100,11 @@ public class ReviewInput { PUBLISH_ALL_REVISIONS } - public static class CommentInput extends Comment {} + public static class CommentInput extends Comment { + public Boolean unresolved; + } - public static class RobotCommentInput extends CommentInput { + public static class RobotCommentInput extends Comment { public String robotId; public String robotRunId; public String url; diff --git a/java/com/google/gerrit/extensions/client/Comment.java b/java/com/google/gerrit/extensions/client/Comment.java index faa9f69..634992e 100644 --- a/java/com/google/gerrit/extensions/client/Comment.java +++ b/java/com/google/gerrit/extensions/client/Comment.java @@ -37,7 +37,6 @@ public abstract class Comment { public String inReplyTo; public Timestamp updated; public String message; - public Boolean unresolved; /** * Hex commit SHA1 (as 40 characters hex string) of the commit of the patchset to which this @@ -128,7 +127,6 @@ public abstract class Comment { && Objects.equals(inReplyTo, c.inReplyTo) && Objects.equals(updated, c.updated) && Objects.equals(message, c.message) - && Objects.equals(unresolved, c.unresolved) && Objects.equals(commitId, c.commitId); } return false; diff --git a/java/com/google/gerrit/extensions/common/CommentInfo.java b/java/com/google/gerrit/extensions/common/CommentInfo.java index 19d721b..fcce2b3 100644 --- a/java/com/google/gerrit/extensions/common/CommentInfo.java +++ b/java/com/google/gerrit/extensions/common/CommentInfo.java @@ -22,6 +22,7 @@ public class CommentInfo extends Comment { public AccountInfo author; public String tag; public String changeMessageId; + public Boolean unresolved; /** * A list of {@link ContextLineInfo}, that is, a list of pairs of {line_num, line_text} of the @@ -33,13 +34,15 @@ public class CommentInfo extends Comment { public boolean equals(Object o) { if (super.equals(o)) { CommentInfo ci = (CommentInfo) o; - return Objects.equals(author, ci.author) && Objects.equals(tag, ci.tag); + return Objects.equals(author, ci.author) + && Objects.equals(tag, ci.tag) + && Objects.equals(unresolved, ci.unresolved); } return false; } @Override public int hashCode() { - return Objects.hash(super.hashCode(), author, tag); + return Objects.hash(super.hashCode(), author, tag, unresolved); } } diff --git a/java/com/google/gerrit/extensions/common/testing/RobotCommentInfoSubject.java b/java/com/google/gerrit/extensions/common/testing/RobotCommentInfoSubject.java index a99b859..5176145 100644 --- a/java/com/google/gerrit/extensions/common/testing/RobotCommentInfoSubject.java +++ b/java/com/google/gerrit/extensions/common/testing/RobotCommentInfoSubject.java @@ -17,7 +17,6 @@ package com.google.gerrit.extensions.common.testing; import static com.google.common.truth.Truth.assertAbout; import static com.google.gerrit.truth.ListSubject.elements; -import com.google.common.truth.BooleanSubject; import com.google.common.truth.FailureMetadata; import com.google.common.truth.MapSubject; import com.google.common.truth.StringSubject; @@ -81,11 +80,6 @@ public class RobotCommentInfoSubject extends Subject { return check("property").that(robotCommentInfo.properties); } - public BooleanSubject unresolved() { - isNotNull(); - return check("unresolved").that(robotCommentInfo.unresolved); - } - public FixSuggestionInfoSubject onlyFixSuggestion() { return fixSuggestions().onlyElement(); } diff --git a/java/com/google/gerrit/server/change/CommentThread.java b/java/com/google/gerrit/server/change/CommentThread.java index 7b729d2..0265f60 100644 --- a/java/com/google/gerrit/server/change/CommentThread.java +++ b/java/com/google/gerrit/server/change/CommentThread.java @@ -17,9 +17,11 @@ package com.google.gerrit.server.change; import com.google.auto.value.AutoValue; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; -import com.google.common.collect.Iterables; +import com.google.common.collect.Streams; import com.google.gerrit.entities.Comment; +import com.google.gerrit.entities.HumanComment; import java.util.List; +import java.util.Optional; /** * Representation of a comment thread. @@ -37,7 +39,14 @@ public abstract class CommentThread { /** Whether the whole thread is considered as unresolved. */ public boolean unresolved() { - return Iterables.getLast(comments()).unresolved; + Optional lastHumanComment = + Streams.findLast( + comments().stream() + .filter(HumanComment.class::isInstance) + .map(HumanComment.class::cast)); + // We often use false == null for boolean fields. It's also a safe fall-back if no human comment + // is part of the thread. + return lastHumanComment.map(comment -> comment.unresolved).orElse(false); } public static Builder builder() { diff --git a/java/com/google/gerrit/server/restapi/change/CommentJson.java b/java/com/google/gerrit/server/restapi/change/CommentJson.java index 67049e8..4de9b63 100644 --- a/java/com/google/gerrit/server/restapi/change/CommentJson.java +++ b/java/com/google/gerrit/server/restapi/change/CommentJson.java @@ -166,7 +166,6 @@ public class CommentJson { r.updated = c.writtenOn; r.range = toRange(c.range); r.tag = c.tag; - r.unresolved = c.unresolved; if (loader != null) { r.author = loader.get(c.author.getId()); } @@ -194,6 +193,7 @@ public class CommentJson { protected CommentInfo toInfo(HumanComment c, AccountLoader loader) { CommentInfo ci = new CommentInfo(); fillCommentInfo(c, ci, loader); + ci.unresolved = c.unresolved; return ci; } diff --git a/java/com/google/gerrit/server/restapi/change/PostReview.java b/java/com/google/gerrit/server/restapi/change/PostReview.java index 3367ca6..604c87f 100644 --- a/java/com/google/gerrit/server/restapi/change/PostReview.java +++ b/java/com/google/gerrit/server/restapi/change/PostReview.java @@ -568,8 +568,8 @@ public class PostReview implements RestModifyView } } - private static Map> cleanUpComments( - Map> commentsPerPath) { + private static + Map> cleanUpComments(Map> commentsPerPath) { Map> cleanedUpCommentMap = new HashMap<>(); for (Map.Entry> e : commentsPerPath.entrySet()) { String path = e.getKey(); @@ -587,7 +587,8 @@ public class PostReview implements RestModifyView return cleanedUpCommentMap; } - private static List cleanUpComments(List comments) { + private static List cleanUpComments( + List comments) { return comments.stream() .filter(Objects::nonNull) .filter(comment -> !Strings.nullToEmpty(comment.message).trim().isEmpty()) @@ -598,7 +599,7 @@ public class PostReview implements RestModifyView return TraceContext.newTimer(getClass().getSimpleName() + "#" + method, Metadata.empty()); } - private void checkComments( + private void checkComments( RevisionResource revision, Map> commentsPerPath) throws BadRequestException, PatchListNotAvailableException { logger.atFine().log("checking comments"); @@ -646,15 +647,16 @@ public class PostReview implements RestModifyView } } - private static void ensureCommentNotOnMagicFilesOfAutoMerge( - String path, T comment) throws BadRequestException { + private static + void ensureCommentNotOnMagicFilesOfAutoMerge(String path, T comment) + throws BadRequestException { if (Patch.isMagic(path) && comment.side == Side.PARENT && comment.parent == null) { throw new BadRequestException(String.format("cannot comment on %s on auto-merge", path)); } } - private static void ensureValidPatchsetLevelComment( - String path, T comment) throws BadRequestException { + private static + void ensureValidPatchsetLevelComment(String path, T comment) throws BadRequestException { if (path.equals(PATCHSET_LEVEL) && (comment.side != null || comment.range != null || comment.line != null)) { throw new BadRequestException("Patchset-level comments can't have side, range, or line"); diff --git a/java/com/google/gerrit/testing/TestCommentHelper.java b/java/com/google/gerrit/testing/TestCommentHelper.java index 76a5521..da8f871 100644 --- a/java/com/google/gerrit/testing/TestCommentHelper.java +++ b/java/com/google/gerrit/testing/TestCommentHelper.java @@ -82,7 +82,6 @@ public class TestCommentHelper { c.parent = null; c.line = line != 0 ? line : null; c.message = message; - c.unresolved = false; c.inReplyTo = inReplyTo; if (line != 0) c.range = range; return c; @@ -96,7 +95,6 @@ public class TestCommentHelper { c.parent = null; c.line = line != 0 ? line : null; c.message = message; - c.unresolved = false; if (line != 0) c.range = range; return c; } diff --git a/javatests/com/google/gerrit/acceptance/api/revision/RobotCommentsIT.java b/javatests/com/google/gerrit/acceptance/api/revision/RobotCommentsIT.java index 4829086..1ad952c 100644 --- a/javatests/com/google/gerrit/acceptance/api/revision/RobotCommentsIT.java +++ b/javatests/com/google/gerrit/acceptance/api/revision/RobotCommentsIT.java @@ -440,17 +440,6 @@ public class RobotCommentsIT extends AbstractDaemonTest { } @Test - public void robotCommentsAreAlwaysResolved() throws Exception { - RobotCommentInput robotCommentInput = TestCommentHelper.createRobotCommentInput(FILE_NAME); - robotCommentInput.unresolved = true; - testCommentHelper.addRobotComment(changeId, robotCommentInput); - - List robotCommentInfos = getRobotComments(); - // The unresolved flag is false even though we explicitly set it to true! - assertThatList(robotCommentInfos).onlyElement().unresolved().isFalse(); - } - - @Test public void addedFixSuggestionCanBeRetrieved() throws Exception { testCommentHelper.addRobotComment(changeId, withFixRobotCommentInput); List robotCommentInfos = getRobotComments(); diff --git a/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java b/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java index 903ce1a..b4dd4b3 100644 --- a/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java +++ b/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java @@ -78,7 +78,6 @@ import java.util.List; import java.util.Map; import java.util.Optional; import java.util.function.Function; -import java.util.function.Supplier; import java.util.regex.Matcher; import java.util.regex.Pattern; import java.util.stream.Collectors; @@ -1883,39 +1882,46 @@ public class CommentsIT extends AbstractDaemonTest { private static CommentInput newCommentWithOnlyMandatoryFields(String path, String message) { CommentInput c = new CommentInput(); - return populate(c, path, null, null, null, null, message, false); + c.unresolved = false; + return populate(c, path, null, null, null, null, message); } private static CommentInput newComment( String path, Side side, int line, String message, Boolean unresolved) { CommentInput c = new CommentInput(); - return populate(c, path, side, null, line, message, unresolved); + c.unresolved = unresolved; + return populate(c, path, side, null, line, message); } private static CommentInput newCommentOnParent( String path, int parent, int line, String message) { CommentInput c = new CommentInput(); - return populate(c, path, Side.PARENT, parent, line, message, false); + c.unresolved = false; + return populate(c, path, Side.PARENT, parent, line, message); } private DraftInput newDraft(String path, Side side, int line, String message) { DraftInput d = new DraftInput(); - return populate(d, path, side, null, line, message, false); + d.unresolved = false; + return populate(d, path, side, null, line, message); } private DraftInput newDraft(String path, Side side, Comment.Range range, String message) { DraftInput d = new DraftInput(); - return populate(d, path, side, null, range.startLine, range, message, false); + d.unresolved = false; + return populate(d, path, side, null, range.startLine, range, message); } private DraftInput newDraftOnParent(String path, int parent, int line, String message) { DraftInput d = new DraftInput(); - return populate(d, path, Side.PARENT, parent, line, message, false); + d.unresolved = false; + return populate(d, path, Side.PARENT, parent, line, message); } private DraftInput newDraftWithOnlyMandatoryFields(String path, String message) { DraftInput d = new DraftInput(); - return populate(d, path, null, null, null, null, message, false); + d.unresolved = false; + return populate(d, path, null, null, null, null, message); } private static C populate( @@ -1925,14 +1931,12 @@ public class CommentsIT extends AbstractDaemonTest { Integer parent, Integer line, Comment.Range range, - String message, - Boolean unresolved) { + String message) { c.path = path; c.side = side; c.parent = parent; c.line = line != null && line != 0 ? line : null; c.message = message; - c.unresolved = unresolved; if (range != null) { c.range = range; } @@ -1940,8 +1944,8 @@ public class CommentsIT extends AbstractDaemonTest { } private static C populate( - C c, String path, Side side, Integer parent, int line, String message, Boolean unresolved) { - return populate(c, path, side, parent, line, null, message, unresolved); + C c, String path, Side side, Integer parent, int line, String message) { + return populate(c, path, side, parent, line, null, message); } private static Comment.Range createLineRange(int startChar, int endChar) { @@ -1954,20 +1958,22 @@ public class CommentsIT extends AbstractDaemonTest { } private static Function infoToInput(String path) { - return infoToInput(path, CommentInput::new); + return info -> { + CommentInput commentInput = new CommentInput(); + commentInput.path = path; + commentInput.unresolved = info.unresolved; + copy(info, commentInput); + return commentInput; + }; } private static Function infoToDraft(String path) { - return infoToInput(path, DraftInput::new); - } - - private static Function infoToInput( - String path, Supplier supplier) { return info -> { - I i = supplier.get(); - i.path = path; - copy(info, i); - return i; + DraftInput draftInput = new DraftInput(); + draftInput.path = path; + draftInput.unresolved = info.unresolved; + copy(info, draftInput); + return draftInput; }; } @@ -1977,7 +1983,6 @@ public class CommentsIT extends AbstractDaemonTest { to.line = from.line; to.message = from.message; to.range = from.range; - to.unresolved = from.unresolved; to.inReplyTo = from.inReplyTo; } } diff --git a/javatests/com/google/gerrit/acceptance/testsuite/change/PatchsetOperationsImplTest.java b/javatests/com/google/gerrit/acceptance/testsuite/change/PatchsetOperationsImplTest.java index 9161928..46d9abf 100644 --- a/javatests/com/google/gerrit/acceptance/testsuite/change/PatchsetOperationsImplTest.java +++ b/javatests/com/google/gerrit/acceptance/testsuite/change/PatchsetOperationsImplTest.java @@ -1067,28 +1067,6 @@ public class PatchsetOperationsImplTest extends AbstractDaemonTest { } @Test - public void robotCommentCanBeCreatedAsResolved() throws Exception { - Change.Id changeId = changeOperations.newChange().create(); - - String commentUuid = - changeOperations.change(changeId).currentPatchset().newRobotComment().resolved().create(); - - CommentInfo comment = getRobotCommentFromServerInCurrentPatchset(changeId, commentUuid); - assertThat(comment).unresolved().isFalse(); - } - - @Test - public void robotCommentCanBeCreatedAsUnresolved() throws Exception { - Change.Id changeId = changeOperations.newChange().create(); - - String commentUuid = - changeOperations.change(changeId).currentPatchset().newRobotComment().unresolved().create(); - - CommentInfo comment = getRobotCommentFromServerInCurrentPatchset(changeId, commentUuid); - assertThat(comment).unresolved().isTrue(); - } - - @Test public void replyToRobotCommentCanBeCreated() throws Exception { Change.Id changeId = changeOperations.newChange().create(); String parentCommentUuid =