Merge "Add api portion of resolvable comments"
This commit is contained in:
@@ -5315,6 +5315,10 @@ Value of the `tag` field from link:#review-input[ReviewInput] set
|
|||||||
while posting the review.
|
while posting the review.
|
||||||
NOTE: To apply different tags on on different votes/comments multiple
|
NOTE: To apply different tags on on different votes/comments multiple
|
||||||
invocations of the REST call are required.
|
invocations of the REST call are required.
|
||||||
|
|`unresolved` |optional|
|
||||||
|
Whether or not the comment must be addressed by the user. The state of
|
||||||
|
resolution of a comment thread is stored in the last comment in that thread
|
||||||
|
chronologically.
|
||||||
|===========================
|
|===========================
|
||||||
|
|
||||||
[[comment-input]]
|
[[comment-input]]
|
||||||
@@ -5357,6 +5361,10 @@ comment is deleted.
|
|||||||
Value of the `tag` field. Only allowed on link:#create-draft[draft comment] +
|
Value of the `tag` field. Only allowed on link:#create-draft[draft comment] +
|
||||||
inputs; for published comments, use the `tag` field in +
|
inputs; for published comments, use the `tag` field in +
|
||||||
link#review-input[ReviewInput]
|
link#review-input[ReviewInput]
|
||||||
|
|`unresolved` |optional|
|
||||||
|
Whether or not the comment must be addressed by the user. This value will
|
||||||
|
default to false if the comment is an orphan, or the value of the `in_reply_to`
|
||||||
|
comment if it is supplied.
|
||||||
|===========================
|
|===========================
|
||||||
|
|
||||||
[[comment-range]]
|
[[comment-range]]
|
||||||
|
|||||||
@@ -135,7 +135,33 @@ public class CommentsIT extends AbstractDaemonTest {
|
|||||||
String changeId = r.getChangeId();
|
String changeId = r.getChangeId();
|
||||||
String revId = r.getCommit().getName();
|
String revId = r.getCommit().getName();
|
||||||
ReviewInput input = new ReviewInput();
|
ReviewInput input = new ReviewInput();
|
||||||
CommentInput comment = newComment(file, Side.REVISION, line, "comment 1");
|
CommentInput comment =
|
||||||
|
newComment(file, Side.REVISION, line, "comment 1", false);
|
||||||
|
input.comments = new HashMap<>();
|
||||||
|
input.comments.put(comment.path, Lists.newArrayList(comment));
|
||||||
|
revision(r).review(input);
|
||||||
|
Map<String, List<CommentInfo>> result = getPublishedComments(changeId, revId);
|
||||||
|
assertThat(result).isNotEmpty();
|
||||||
|
CommentInfo actual = Iterables.getOnlyElement(result.get(comment.path));
|
||||||
|
assertThat(comment).isEqualTo(infoToInput(file).apply(actual));
|
||||||
|
assertThat(comment).isEqualTo(infoToInput(file).apply(
|
||||||
|
getPublishedComment(changeId, revId, actual.id)));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void postCommentWithUnresolved() throws Exception {
|
||||||
|
for (Integer line : lines) {
|
||||||
|
String file = "file";
|
||||||
|
String contents = "contents " + line;
|
||||||
|
PushOneCommit push = pushFactory.create(db, admin.getIdent(), testRepo,
|
||||||
|
"first subject", file, contents);
|
||||||
|
PushOneCommit.Result r = push.to("refs/for/master");
|
||||||
|
String changeId = r.getChangeId();
|
||||||
|
String revId = r.getCommit().getName();
|
||||||
|
ReviewInput input = new ReviewInput();
|
||||||
|
CommentInput comment =
|
||||||
|
newComment(file, Side.REVISION, line, "comment 1", true);
|
||||||
input.comments = new HashMap<>();
|
input.comments = new HashMap<>();
|
||||||
input.comments.put(comment.path, Lists.newArrayList(comment));
|
input.comments.put(comment.path, Lists.newArrayList(comment));
|
||||||
revision(r).review(input);
|
revision(r).review(input);
|
||||||
@@ -156,8 +182,9 @@ public class CommentsIT extends AbstractDaemonTest {
|
|||||||
String changeId = r.getChangeId();
|
String changeId = r.getChangeId();
|
||||||
String revId = r.getCommit().getName();
|
String revId = r.getCommit().getName();
|
||||||
ReviewInput input = new ReviewInput();
|
ReviewInput input = new ReviewInput();
|
||||||
CommentInput c1 = newComment(file, Side.REVISION, line, "ps-1");
|
CommentInput c1 = newComment(file, Side.REVISION, line, "ps-1", false);
|
||||||
CommentInput c2 = newComment(file, Side.PARENT, line, "auto-merge of ps-1");
|
CommentInput c2 =
|
||||||
|
newComment(file, Side.PARENT, line, "auto-merge of ps-1", false);
|
||||||
CommentInput c3 = newCommentOnParent(file, 1, line, "parent-1 of ps-1");
|
CommentInput c3 = newCommentOnParent(file, 1, line, "parent-1 of ps-1");
|
||||||
CommentInput c4 = newCommentOnParent(file, 2, line, "parent-2 of ps-1");
|
CommentInput c4 = newCommentOnParent(file, 2, line, "parent-2 of ps-1");
|
||||||
input.comments = new HashMap<>();
|
input.comments = new HashMap<>();
|
||||||
@@ -176,7 +203,7 @@ public class CommentsIT extends AbstractDaemonTest {
|
|||||||
String changeId = r.getChangeId();
|
String changeId = r.getChangeId();
|
||||||
String revId = r.getCommit().getName();
|
String revId = r.getCommit().getName();
|
||||||
ReviewInput input = new ReviewInput();
|
ReviewInput input = new ReviewInput();
|
||||||
CommentInput c1 = newComment(file, Side.REVISION, line, "ps-1");
|
CommentInput c1 = newComment(file, Side.REVISION, line, "ps-1", false);
|
||||||
CommentInput c2 = newCommentOnParent(file, 1, line, "parent-1 of ps-1");
|
CommentInput c2 = newCommentOnParent(file, 1, line, "parent-1 of ps-1");
|
||||||
CommentInput c3 = newCommentOnParent(file, 2, line, "parent-2 of ps-1");
|
CommentInput c3 = newCommentOnParent(file, 2, line, "parent-2 of ps-1");
|
||||||
input.comments = new HashMap<>();
|
input.comments = new HashMap<>();
|
||||||
@@ -193,8 +220,8 @@ public class CommentsIT extends AbstractDaemonTest {
|
|||||||
public void postCommentOnCommitMessageOnAutoMerge() throws Exception {
|
public void postCommentOnCommitMessageOnAutoMerge() throws Exception {
|
||||||
PushOneCommit.Result r = createMergeCommitChange("refs/for/master");
|
PushOneCommit.Result r = createMergeCommitChange("refs/for/master");
|
||||||
ReviewInput input = new ReviewInput();
|
ReviewInput input = new ReviewInput();
|
||||||
CommentInput c =
|
CommentInput c = newComment(Patch.COMMIT_MSG, Side.PARENT, 0,
|
||||||
newComment(Patch.COMMIT_MSG, Side.PARENT, 0, "comment on auto-merge");
|
"comment on auto-merge", false);
|
||||||
input.comments = new HashMap<>();
|
input.comments = new HashMap<>();
|
||||||
input.comments.put(Patch.COMMIT_MSG, ImmutableList.of(c));
|
input.comments.put(Patch.COMMIT_MSG, ImmutableList.of(c));
|
||||||
exception.expect(BadRequestException.class);
|
exception.expect(BadRequestException.class);
|
||||||
@@ -216,7 +243,8 @@ public class CommentsIT extends AbstractDaemonTest {
|
|||||||
List<CommentInput> expectedComments = new ArrayList<>();
|
List<CommentInput> expectedComments = new ArrayList<>();
|
||||||
for (Integer line : lines) {
|
for (Integer line : lines) {
|
||||||
ReviewInput input = new ReviewInput();
|
ReviewInput input = new ReviewInput();
|
||||||
CommentInput comment = newComment(file, Side.REVISION, line, "comment " + line);
|
CommentInput comment =
|
||||||
|
newComment(file, Side.REVISION, line, "comment " + line, false);
|
||||||
expectedComments.add(comment);
|
expectedComments.add(comment);
|
||||||
input.comments = new HashMap<>();
|
input.comments = new HashMap<>();
|
||||||
input.comments.put(comment.path, Lists.newArrayList(comment));
|
input.comments.put(comment.path, Lists.newArrayList(comment));
|
||||||
@@ -326,7 +354,8 @@ public class CommentsIT extends AbstractDaemonTest {
|
|||||||
Timestamp origLastUpdated = r.getChange().change().getLastUpdatedOn();
|
Timestamp origLastUpdated = r.getChange().change().getLastUpdatedOn();
|
||||||
|
|
||||||
ReviewInput input = new ReviewInput();
|
ReviewInput input = new ReviewInput();
|
||||||
CommentInput comment = newComment(file, Side.REVISION, line, "comment 1");
|
CommentInput comment =
|
||||||
|
newComment(file, Side.REVISION, line, "comment 1", false);
|
||||||
comment.updated = timestamp;
|
comment.updated = timestamp;
|
||||||
input.comments = new HashMap<>();
|
input.comments = new HashMap<>();
|
||||||
input.comments.put(comment.path, Lists.newArrayList(comment));
|
input.comments.put(comment.path, Lists.newArrayList(comment));
|
||||||
@@ -554,12 +583,12 @@ public class CommentsIT extends AbstractDaemonTest {
|
|||||||
+ url + "#/c/" + c + "/1/a.txt\n"
|
+ url + "#/c/" + c + "/1/a.txt\n"
|
||||||
+ "File a.txt:\n"
|
+ "File a.txt:\n"
|
||||||
+ "\n"
|
+ "\n"
|
||||||
+ url + "#/c/12/1/a.txt@a2\n"
|
+ url + "#/c/" + c + "/1/a.txt@a2\n"
|
||||||
+ "PS1, Line 2: \n"
|
+ "PS1, Line 2: \n"
|
||||||
+ "what happened to this?\n"
|
+ "what happened to this?\n"
|
||||||
+ "\n"
|
+ "\n"
|
||||||
+ "\n"
|
+ "\n"
|
||||||
+ url + "#/c/12/1/a.txt@1\n"
|
+ url + "#/c/" + c + "/1/a.txt@1\n"
|
||||||
+ "PS1, Line 1: ew\n"
|
+ "PS1, Line 1: ew\n"
|
||||||
+ "nit: trailing whitespace\n"
|
+ "nit: trailing whitespace\n"
|
||||||
+ "\n"
|
+ "\n"
|
||||||
@@ -567,22 +596,22 @@ public class CommentsIT extends AbstractDaemonTest {
|
|||||||
+ url + "#/c/" + c + "/2/a.txt\n"
|
+ url + "#/c/" + c + "/2/a.txt\n"
|
||||||
+ "File a.txt:\n"
|
+ "File a.txt:\n"
|
||||||
+ "\n"
|
+ "\n"
|
||||||
+ url + "#/c/12/2/a.txt@a1\n"
|
+ url + "#/c/" + c + "/2/a.txt@a1\n"
|
||||||
+ "PS2, Line 1: \n"
|
+ "PS2, Line 1: \n"
|
||||||
+ "comment 1 on base\n"
|
+ "comment 1 on base\n"
|
||||||
+ "\n"
|
+ "\n"
|
||||||
+ "\n"
|
+ "\n"
|
||||||
+ url + "#/c/12/2/a.txt@a2\n"
|
+ url + "#/c/" + c + "/2/a.txt@a2\n"
|
||||||
+ "PS2, Line 2: \n"
|
+ "PS2, Line 2: \n"
|
||||||
+ "comment 2 on base\n"
|
+ "comment 2 on base\n"
|
||||||
+ "\n"
|
+ "\n"
|
||||||
+ "\n"
|
+ "\n"
|
||||||
+ url + "#/c/12/2/a.txt@1\n"
|
+ url + "#/c/" + c + "/2/a.txt@1\n"
|
||||||
+ "PS2, Line 1: ew\n"
|
+ "PS2, Line 1: ew\n"
|
||||||
+ "join lines\n"
|
+ "join lines\n"
|
||||||
+ "\n"
|
+ "\n"
|
||||||
+ "\n"
|
+ "\n"
|
||||||
+ url + "#/c/12/2/a.txt@2\n"
|
+ url + "#/c/" + c + "/2/a.txt@2\n"
|
||||||
+ "PS2, Line 2: nten\n"
|
+ "PS2, Line 2: nten\n"
|
||||||
+ "typo: content\n"
|
+ "typo: content\n"
|
||||||
+ "\n"
|
+ "\n"
|
||||||
@@ -688,36 +717,39 @@ public class CommentsIT extends AbstractDaemonTest {
|
|||||||
}
|
}
|
||||||
|
|
||||||
private static CommentInput newComment(String path, Side side, int line,
|
private static CommentInput newComment(String path, Side side, int line,
|
||||||
String message) {
|
String message, Boolean unresolved) {
|
||||||
CommentInput c = new CommentInput();
|
CommentInput c = new CommentInput();
|
||||||
return populate(c, path, side, null, line, message);
|
return populate(c, path, side, null, line, message, unresolved);
|
||||||
}
|
}
|
||||||
|
|
||||||
private static CommentInput newCommentOnParent(String path, int parent,
|
private static CommentInput newCommentOnParent(String path, int parent,
|
||||||
int line, String message) {
|
int line, String message) {
|
||||||
CommentInput c = new CommentInput();
|
CommentInput c = new CommentInput();
|
||||||
return populate(c, path, Side.PARENT, Integer.valueOf(parent), line, message);
|
return populate(c, path, Side.PARENT, Integer.valueOf(parent), line,
|
||||||
|
message, false);
|
||||||
}
|
}
|
||||||
|
|
||||||
private DraftInput newDraft(String path, Side side, int line,
|
private DraftInput newDraft(String path, Side side, int line,
|
||||||
String message) {
|
String message) {
|
||||||
DraftInput d = new DraftInput();
|
DraftInput d = new DraftInput();
|
||||||
return populate(d, path, side, null, line, message);
|
return populate(d, path, side, null, line, message, false);
|
||||||
}
|
}
|
||||||
|
|
||||||
private DraftInput newDraftOnParent(String path, int parent, int line,
|
private DraftInput newDraftOnParent(String path, int parent, int line,
|
||||||
String message) {
|
String message) {
|
||||||
DraftInput d = new DraftInput();
|
DraftInput d = new DraftInput();
|
||||||
return populate(d, path, Side.PARENT, Integer.valueOf(parent), line, message);
|
return populate(d, path, Side.PARENT, Integer.valueOf(parent), line,
|
||||||
|
message, false);
|
||||||
}
|
}
|
||||||
|
|
||||||
private static <C extends Comment> C populate(C c, String path, Side side,
|
private static <C extends Comment> C populate(C c, String path, Side side,
|
||||||
Integer parent, int line, String message) {
|
Integer parent, int line, String message, Boolean unresolved) {
|
||||||
c.path = path;
|
c.path = path;
|
||||||
c.side = side;
|
c.side = side;
|
||||||
c.parent = parent;
|
c.parent = parent;
|
||||||
c.line = line != 0 ? line : null;
|
c.line = line != 0 ? line : null;
|
||||||
c.message = message;
|
c.message = message;
|
||||||
|
c.unresolved = unresolved;
|
||||||
if (line != 0) {
|
if (line != 0) {
|
||||||
Comment.Range range = new Comment.Range();
|
Comment.Range range = new Comment.Range();
|
||||||
range.startLine = line;
|
range.startLine = line;
|
||||||
@@ -753,5 +785,6 @@ public class CommentsIT extends AbstractDaemonTest {
|
|||||||
to.line = from.line;
|
to.line = from.line;
|
||||||
to.message = from.message;
|
to.message = from.message;
|
||||||
to.range = from.range;
|
to.range = from.range;
|
||||||
|
to.unresolved = from.unresolved;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -38,6 +38,7 @@ import com.google.gerrit.extensions.api.changes.ReviewInput;
|
|||||||
import com.google.gerrit.extensions.api.changes.ReviewInput.CommentInput;
|
import com.google.gerrit.extensions.api.changes.ReviewInput.CommentInput;
|
||||||
import com.google.gerrit.extensions.api.changes.ReviewInput.DraftHandling;
|
import com.google.gerrit.extensions.api.changes.ReviewInput.DraftHandling;
|
||||||
import com.google.gerrit.extensions.client.Side;
|
import com.google.gerrit.extensions.client.Side;
|
||||||
|
import com.google.gerrit.extensions.common.CommentInfo;
|
||||||
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
|
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
|
||||||
import com.google.gerrit.extensions.restapi.RestApiException;
|
import com.google.gerrit.extensions.restapi.RestApiException;
|
||||||
import com.google.gerrit.reviewdb.client.Account;
|
import com.google.gerrit.reviewdb.client.Account;
|
||||||
@@ -227,7 +228,7 @@ public class ChangeRebuilderIT extends AbstractDaemonTest {
|
|||||||
public void draftComment() throws Exception {
|
public void draftComment() throws Exception {
|
||||||
PushOneCommit.Result r = createChange();
|
PushOneCommit.Result r = createChange();
|
||||||
Change.Id id = r.getPatchSetId().getParentKey();
|
Change.Id id = r.getPatchSetId().getParentKey();
|
||||||
putDraft(user, id, 1, "comment");
|
putDraft(user, id, 1, "comment", null);
|
||||||
checker.rebuildAndCheckChanges(id);
|
checker.rebuildAndCheckChanges(id);
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -235,7 +236,7 @@ public class ChangeRebuilderIT extends AbstractDaemonTest {
|
|||||||
public void draftAndPublishedComment() throws Exception {
|
public void draftAndPublishedComment() throws Exception {
|
||||||
PushOneCommit.Result r = createChange();
|
PushOneCommit.Result r = createChange();
|
||||||
Change.Id id = r.getPatchSetId().getParentKey();
|
Change.Id id = r.getPatchSetId().getParentKey();
|
||||||
putDraft(user, id, 1, "draft comment");
|
putDraft(user, id, 1, "draft comment", null);
|
||||||
putComment(user, id, 1, "published comment");
|
putComment(user, id, 1, "published comment");
|
||||||
checker.rebuildAndCheckChanges(id);
|
checker.rebuildAndCheckChanges(id);
|
||||||
}
|
}
|
||||||
@@ -244,7 +245,7 @@ public class ChangeRebuilderIT extends AbstractDaemonTest {
|
|||||||
public void publishDraftComment() throws Exception {
|
public void publishDraftComment() throws Exception {
|
||||||
PushOneCommit.Result r = createChange();
|
PushOneCommit.Result r = createChange();
|
||||||
Change.Id id = r.getPatchSetId().getParentKey();
|
Change.Id id = r.getPatchSetId().getParentKey();
|
||||||
putDraft(user, id, 1, "draft comment");
|
putDraft(user, id, 1, "draft comment", null);
|
||||||
publishDrafts(user, id);
|
publishDrafts(user, id);
|
||||||
checker.rebuildAndCheckChanges(id);
|
checker.rebuildAndCheckChanges(id);
|
||||||
}
|
}
|
||||||
@@ -371,14 +372,14 @@ public class ChangeRebuilderIT extends AbstractDaemonTest {
|
|||||||
assertThat(getUnwrappedDb().changes().get(id).getNoteDbState()).isEqualTo(
|
assertThat(getUnwrappedDb().changes().get(id).getNoteDbState()).isEqualTo(
|
||||||
changeMetaId.name());
|
changeMetaId.name());
|
||||||
|
|
||||||
putDraft(user, id, 1, "comment by user");
|
putDraft(user, id, 1, "comment by user", null);
|
||||||
ObjectId userDraftsId = getMetaRef(
|
ObjectId userDraftsId = getMetaRef(
|
||||||
allUsers, refsDraftComments(id, user.getId()));
|
allUsers, refsDraftComments(id, user.getId()));
|
||||||
assertThat(getUnwrappedDb().changes().get(id).getNoteDbState()).isEqualTo(
|
assertThat(getUnwrappedDb().changes().get(id).getNoteDbState()).isEqualTo(
|
||||||
changeMetaId.name()
|
changeMetaId.name()
|
||||||
+ "," + user.getId() + "=" + userDraftsId.name());
|
+ "," + user.getId() + "=" + userDraftsId.name());
|
||||||
|
|
||||||
putDraft(admin, id, 2, "comment by admin");
|
putDraft(admin, id, 2, "comment by admin", null);
|
||||||
ObjectId adminDraftsId = getMetaRef(
|
ObjectId adminDraftsId = getMetaRef(
|
||||||
allUsers, refsDraftComments(id, admin.getId()));
|
allUsers, refsDraftComments(id, admin.getId()));
|
||||||
assertThat(admin.getId().get()).isLessThan(user.getId().get());
|
assertThat(admin.getId().get()).isLessThan(user.getId().get());
|
||||||
@@ -387,7 +388,7 @@ public class ChangeRebuilderIT extends AbstractDaemonTest {
|
|||||||
+ "," + admin.getId() + "=" + adminDraftsId.name()
|
+ "," + admin.getId() + "=" + adminDraftsId.name()
|
||||||
+ "," + user.getId() + "=" + userDraftsId.name());
|
+ "," + user.getId() + "=" + userDraftsId.name());
|
||||||
|
|
||||||
putDraft(admin, id, 2, "revised comment by admin");
|
putDraft(admin, id, 2, "revised comment by admin", null);
|
||||||
adminDraftsId = getMetaRef(
|
adminDraftsId = getMetaRef(
|
||||||
allUsers, refsDraftComments(id, admin.getId()));
|
allUsers, refsDraftComments(id, admin.getId()));
|
||||||
assertThat(getUnwrappedDb().changes().get(id).getNoteDbState()).isEqualTo(
|
assertThat(getUnwrappedDb().changes().get(id).getNoteDbState()).isEqualTo(
|
||||||
@@ -565,7 +566,7 @@ public class ChangeRebuilderIT extends AbstractDaemonTest {
|
|||||||
|
|
||||||
PushOneCommit.Result r = createChange();
|
PushOneCommit.Result r = createChange();
|
||||||
Change.Id id = r.getPatchSetId().getParentKey();
|
Change.Id id = r.getPatchSetId().getParentKey();
|
||||||
putDraft(user, id, 1, "comment by user");
|
putDraft(user, id, 1, "comment by user", null);
|
||||||
assertChangeUpToDate(true, id);
|
assertChangeUpToDate(true, id);
|
||||||
|
|
||||||
ObjectId oldMetaId =
|
ObjectId oldMetaId =
|
||||||
@@ -573,7 +574,7 @@ public class ChangeRebuilderIT extends AbstractDaemonTest {
|
|||||||
|
|
||||||
// Add a draft behind NoteDb's back.
|
// Add a draft behind NoteDb's back.
|
||||||
setNotesMigration(false, false);
|
setNotesMigration(false, false);
|
||||||
putDraft(user, id, 1, "second comment by user");
|
putDraft(user, id, 1, "second comment by user", null);
|
||||||
setInvalidNoteDbState(id);
|
setInvalidNoteDbState(id);
|
||||||
assertDraftsUpToDate(false, id, user);
|
assertDraftsUpToDate(false, id, user);
|
||||||
assertThat(getMetaRef(allUsers, refsDraftComments(id, user.getId())))
|
assertThat(getMetaRef(allUsers, refsDraftComments(id, user.getId())))
|
||||||
@@ -608,7 +609,7 @@ public class ChangeRebuilderIT extends AbstractDaemonTest {
|
|||||||
|
|
||||||
PushOneCommit.Result r = createChange();
|
PushOneCommit.Result r = createChange();
|
||||||
Change.Id id = r.getPatchSetId().getParentKey();
|
Change.Id id = r.getPatchSetId().getParentKey();
|
||||||
putDraft(user, id, 1, "comment by user");
|
putDraft(user, id, 1, "comment by user", null);
|
||||||
assertChangeUpToDate(true, id);
|
assertChangeUpToDate(true, id);
|
||||||
|
|
||||||
ObjectId oldMetaId =
|
ObjectId oldMetaId =
|
||||||
@@ -616,7 +617,7 @@ public class ChangeRebuilderIT extends AbstractDaemonTest {
|
|||||||
|
|
||||||
// Add a draft behind NoteDb's back.
|
// Add a draft behind NoteDb's back.
|
||||||
setNotesMigration(false, false);
|
setNotesMigration(false, false);
|
||||||
putDraft(user, id, 1, "second comment by user");
|
putDraft(user, id, 1, "second comment by user", null);
|
||||||
|
|
||||||
ReviewDb db = getUnwrappedDb();
|
ReviewDb db = getUnwrappedDb();
|
||||||
Change c = db.changes().get(id);
|
Change c = db.changes().get(id);
|
||||||
@@ -668,12 +669,12 @@ public class ChangeRebuilderIT extends AbstractDaemonTest {
|
|||||||
|
|
||||||
PushOneCommit.Result r = createChange();
|
PushOneCommit.Result r = createChange();
|
||||||
Change.Id id = r.getPatchSetId().getParentKey();
|
Change.Id id = r.getPatchSetId().getParentKey();
|
||||||
putDraft(user, id, 1, "comment");
|
putDraft(user, id, 1, "comment", null);
|
||||||
assertDraftsUpToDate(true, id, user);
|
assertDraftsUpToDate(true, id, user);
|
||||||
|
|
||||||
// Make a ReviewDb change behind NoteDb's back and ensure it's detected.
|
// Make a ReviewDb change behind NoteDb's back and ensure it's detected.
|
||||||
setNotesMigration(false, false);
|
setNotesMigration(false, false);
|
||||||
putDraft(user, id, 1, "comment");
|
putDraft(user, id, 1, "comment", null);
|
||||||
setInvalidNoteDbState(id);
|
setInvalidNoteDbState(id);
|
||||||
assertDraftsUpToDate(false, id, user);
|
assertDraftsUpToDate(false, id, user);
|
||||||
|
|
||||||
@@ -900,7 +901,7 @@ public class ChangeRebuilderIT extends AbstractDaemonTest {
|
|||||||
public void rebuildDeletesOldDraftRefs() throws Exception {
|
public void rebuildDeletesOldDraftRefs() throws Exception {
|
||||||
PushOneCommit.Result r = createChange();
|
PushOneCommit.Result r = createChange();
|
||||||
Change.Id id = r.getPatchSetId().getParentKey();
|
Change.Id id = r.getPatchSetId().getParentKey();
|
||||||
putDraft(user, id, 1, "comment");
|
putDraft(user, id, 1, "comment", null);
|
||||||
|
|
||||||
Account.Id otherAccountId = new Account.Id(user.getId().get() + 1234);
|
Account.Id otherAccountId = new Account.Id(user.getId().get() + 1234);
|
||||||
String otherDraftRef = refsDraftComments(id, otherAccountId);
|
String otherDraftRef = refsDraftComments(id, otherAccountId);
|
||||||
@@ -1162,6 +1163,23 @@ public class ChangeRebuilderIT extends AbstractDaemonTest {
|
|||||||
assertThat(notes.getChange().currentPatchSetId()).isEqualTo(psId1);
|
assertThat(notes.getChange().currentPatchSetId()).isEqualTo(psId1);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void resolveCommentsInheritsValueFromParentWhenUnspecified()
|
||||||
|
throws Exception {
|
||||||
|
PushOneCommit.Result r = createChange();
|
||||||
|
Change.Id id = r.getPatchSetId().getParentKey();
|
||||||
|
putDraft(user, id, 1, "comment", true);
|
||||||
|
putDraft(user, id, 1, "newComment", null);
|
||||||
|
|
||||||
|
Map<String, List<CommentInfo>> comments =
|
||||||
|
gApi.changes().id(id.get()).current().drafts();
|
||||||
|
for (List<CommentInfo> cList: comments.values()) {
|
||||||
|
for (CommentInfo ci: cList) {
|
||||||
|
assertThat(ci.unresolved).isEqualTo(true);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
private void assertChangesReadOnly(RestApiException e) throws Exception {
|
private void assertChangesReadOnly(RestApiException e) throws Exception {
|
||||||
Throwable cause = e.getCause();
|
Throwable cause = e.getCause();
|
||||||
assertThat(cause).isInstanceOf(UpdateException.class);
|
assertThat(cause).isInstanceOf(UpdateException.class);
|
||||||
@@ -1213,12 +1231,13 @@ public class ChangeRebuilderIT extends AbstractDaemonTest {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
private void putDraft(TestAccount account, Change.Id id, int line, String msg)
|
private void putDraft(TestAccount account, Change.Id id, int line, String msg,
|
||||||
throws Exception {
|
Boolean unresolved) throws Exception {
|
||||||
DraftInput in = new DraftInput();
|
DraftInput in = new DraftInput();
|
||||||
in.line = line;
|
in.line = line;
|
||||||
in.message = msg;
|
in.message = msg;
|
||||||
in.path = PushOneCommit.FILE_NAME;
|
in.path = PushOneCommit.FILE_NAME;
|
||||||
|
in.unresolved = unresolved;
|
||||||
AcceptanceTestRequestScope.Context old = setApiUser(account);
|
AcceptanceTestRequestScope.Context old = setApiUser(account);
|
||||||
try {
|
try {
|
||||||
gApi.changes().id(id.get()).current().createDraft(in);
|
gApi.changes().id(id.get()).current().createDraft(in);
|
||||||
|
|||||||
@@ -34,7 +34,7 @@ public abstract class Comment {
|
|||||||
public String inReplyTo;
|
public String inReplyTo;
|
||||||
public Timestamp updated;
|
public Timestamp updated;
|
||||||
public String message;
|
public String message;
|
||||||
public boolean unresolved;
|
public Boolean unresolved;
|
||||||
|
|
||||||
public static class Range {
|
public static class Range {
|
||||||
public int startLine;
|
public int startLine;
|
||||||
|
|||||||
@@ -124,6 +124,7 @@ public final class PatchLineComment {
|
|||||||
plc.setRevId(new RevId(c.revId));
|
plc.setRevId(new RevId(c.revId));
|
||||||
plc.setStatus(status);
|
plc.setStatus(status);
|
||||||
plc.setRealAuthor(c.getRealAuthor().getId());
|
plc.setRealAuthor(c.getRealAuthor().getId());
|
||||||
|
plc.setUnresolved(c.unresolved);
|
||||||
return plc;
|
return plc;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -323,6 +324,14 @@ public final class PatchLineComment {
|
|||||||
return tag;
|
return tag;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public void setUnresolved(Boolean unresolved) {
|
||||||
|
this.unresolved = unresolved;
|
||||||
|
}
|
||||||
|
|
||||||
|
public Boolean getUnresolved() {
|
||||||
|
return unresolved;
|
||||||
|
}
|
||||||
|
|
||||||
public Comment asComment(String serverId) {
|
public Comment asComment(String serverId) {
|
||||||
Comment c = new Comment(key.asCommentKey(), author, writtenOn, side,
|
Comment c = new Comment(key.asCommentKey(), author, writtenOn, side,
|
||||||
message, serverId, unresolved);
|
message, serverId, unresolved);
|
||||||
@@ -349,7 +358,8 @@ public final class PatchLineComment {
|
|||||||
&& Objects.equals(parentUuid, c.getParentUuid())
|
&& Objects.equals(parentUuid, c.getParentUuid())
|
||||||
&& Objects.equals(range, c.getRange())
|
&& Objects.equals(range, c.getRange())
|
||||||
&& Objects.equals(revId, c.getRevId())
|
&& Objects.equals(revId, c.getRevId())
|
||||||
&& Objects.equals(tag, c.getTag());
|
&& Objects.equals(tag, c.getTag())
|
||||||
|
&& Objects.equals(unresolved, c.getUnresolved());
|
||||||
}
|
}
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -27,6 +27,7 @@ import com.google.common.collect.Ordering;
|
|||||||
import com.google.gerrit.common.Nullable;
|
import com.google.gerrit.common.Nullable;
|
||||||
import com.google.gerrit.extensions.client.Side;
|
import com.google.gerrit.extensions.client.Side;
|
||||||
import com.google.gerrit.extensions.common.CommentInfo;
|
import com.google.gerrit.extensions.common.CommentInfo;
|
||||||
|
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
|
||||||
import com.google.gerrit.reviewdb.client.Account;
|
import com.google.gerrit.reviewdb.client.Account;
|
||||||
import com.google.gerrit.reviewdb.client.Change;
|
import com.google.gerrit.reviewdb.client.Change;
|
||||||
import com.google.gerrit.reviewdb.client.Comment;
|
import com.google.gerrit.reviewdb.client.Comment;
|
||||||
@@ -141,12 +142,33 @@ public class CommentsUtil {
|
|||||||
this.serverId = serverId;
|
this.serverId = serverId;
|
||||||
}
|
}
|
||||||
|
|
||||||
public Comment newComment(ChangeContext ctx, String path, PatchSet.Id psId,
|
public Comment newComment(ChangeContext ctx,
|
||||||
short side, String message) throws OrmException {
|
String path,
|
||||||
|
PatchSet.Id psId,
|
||||||
|
short side,
|
||||||
|
String message,
|
||||||
|
@Nullable Boolean unresolved,
|
||||||
|
@Nullable String parentUuid)
|
||||||
|
throws OrmException, UnprocessableEntityException {
|
||||||
|
if (unresolved == null) {
|
||||||
|
if (parentUuid == null) {
|
||||||
|
// Default to false if comment is not descended from another.
|
||||||
|
unresolved = false;
|
||||||
|
} else {
|
||||||
|
// Inherit unresolved value from inReplyTo comment if not specified.
|
||||||
|
Comment.Key key = new Comment.Key(parentUuid, path, psId.patchSetId);
|
||||||
|
Optional<Comment> parent = get(ctx.getDb(), ctx.getNotes(), key);
|
||||||
|
if (!parent.isPresent()) {
|
||||||
|
throw new UnprocessableEntityException(
|
||||||
|
"Invalid parentUuid supplied for comment");
|
||||||
|
}
|
||||||
|
unresolved = parent.get().unresolved;
|
||||||
|
}
|
||||||
|
}
|
||||||
Comment c = new Comment(
|
Comment c = new Comment(
|
||||||
new Comment.Key(ChangeUtil.messageUUID(ctx.getDb()), path, psId.get()),
|
new Comment.Key(ChangeUtil.messageUUID(ctx.getDb()), path, psId.get()),
|
||||||
ctx.getUser().getAccountId(), ctx.getWhen(), side, message, serverId,
|
ctx.getUser().getAccountId(), ctx.getWhen(), side, message, serverId,
|
||||||
false);
|
unresolved);
|
||||||
ctx.getUser().updateRealAccountId(c::setRealAuthor);
|
ctx.getUser().updateRealAccountId(c::setRealAuthor);
|
||||||
return c;
|
return c;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -25,6 +25,7 @@ import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
|
|||||||
import com.google.gerrit.extensions.restapi.Response;
|
import com.google.gerrit.extensions.restapi.Response;
|
||||||
import com.google.gerrit.extensions.restapi.RestApiException;
|
import com.google.gerrit.extensions.restapi.RestApiException;
|
||||||
import com.google.gerrit.extensions.restapi.RestModifyView;
|
import com.google.gerrit.extensions.restapi.RestModifyView;
|
||||||
|
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
|
||||||
import com.google.gerrit.extensions.restapi.Url;
|
import com.google.gerrit.extensions.restapi.Url;
|
||||||
import com.google.gerrit.reviewdb.client.Comment;
|
import com.google.gerrit.reviewdb.client.Comment;
|
||||||
import com.google.gerrit.reviewdb.client.PatchLineComment.Status;
|
import com.google.gerrit.reviewdb.client.PatchLineComment.Status;
|
||||||
@@ -103,16 +104,20 @@ public class CreateDraftComment implements RestModifyView<RevisionResource, Draf
|
|||||||
|
|
||||||
@Override
|
@Override
|
||||||
public boolean updateChange(ChangeContext ctx)
|
public boolean updateChange(ChangeContext ctx)
|
||||||
throws ResourceNotFoundException, OrmException {
|
throws ResourceNotFoundException, OrmException,
|
||||||
|
UnprocessableEntityException {
|
||||||
PatchSet ps = psUtil.get(ctx.getDb(), ctx.getNotes(), psId);
|
PatchSet ps = psUtil.get(ctx.getDb(), ctx.getNotes(), psId);
|
||||||
if (ps == null) {
|
if (ps == null) {
|
||||||
throw new ResourceNotFoundException("patch set not found: " + psId);
|
throw new ResourceNotFoundException("patch set not found: " + psId);
|
||||||
}
|
}
|
||||||
|
String parentUuid = Url.decode(in.inReplyTo);
|
||||||
|
|
||||||
comment = commentsUtil.newComment(
|
comment = commentsUtil.newComment(
|
||||||
ctx, in.path, ps.getId(), in.side(), in.message.trim());
|
ctx, in.path, ps.getId(), in.side(), in.message.trim(),
|
||||||
comment.parentUuid = Url.decode(in.inReplyTo);
|
in.unresolved, parentUuid);
|
||||||
comment.setLineNbrAndRange(in.line, in.range);
|
comment.setLineNbrAndRange(in.line, in.range);
|
||||||
comment.tag = in.tag;
|
comment.tag = in.tag;
|
||||||
|
|
||||||
setCommentRevId(
|
setCommentRevId(
|
||||||
comment, patchListCache, ctx.getChange(), ps);
|
comment, patchListCache, ctx.getChange(), ps);
|
||||||
|
|
||||||
|
|||||||
@@ -694,7 +694,8 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
|
|||||||
|
|
||||||
@Override
|
@Override
|
||||||
public boolean updateChange(ChangeContext ctx)
|
public boolean updateChange(ChangeContext ctx)
|
||||||
throws OrmException, ResourceConflictException {
|
throws OrmException, ResourceConflictException,
|
||||||
|
UnprocessableEntityException {
|
||||||
user = ctx.getIdentifiedUser();
|
user = ctx.getIdentifiedUser();
|
||||||
notes = ctx.getNotes();
|
notes = ctx.getNotes();
|
||||||
ps = psUtil.get(ctx.getDb(), ctx.getNotes(), psId);
|
ps = psUtil.get(ctx.getDb(), ctx.getNotes(), psId);
|
||||||
@@ -729,7 +730,8 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
|
|||||||
approvals, oldApprovals, ctx.getWhen());
|
approvals, oldApprovals, ctx.getWhen());
|
||||||
}
|
}
|
||||||
|
|
||||||
private boolean insertComments(ChangeContext ctx) throws OrmException {
|
private boolean insertComments(ChangeContext ctx)
|
||||||
|
throws OrmException, UnprocessableEntityException {
|
||||||
Map<String, List<CommentInput>> map = in.comments;
|
Map<String, List<CommentInput>> map = in.comments;
|
||||||
if (map == null) {
|
if (map == null) {
|
||||||
map = Collections.emptyMap();
|
map = Collections.emptyMap();
|
||||||
@@ -757,16 +759,14 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
|
|||||||
String parent = Url.decode(c.inReplyTo);
|
String parent = Url.decode(c.inReplyTo);
|
||||||
Comment e = drafts.remove(Url.decode(c.id));
|
Comment e = drafts.remove(Url.decode(c.id));
|
||||||
if (e == null) {
|
if (e == null) {
|
||||||
e = commentsUtil.newComment(ctx, path, psId, c.side(), c.message);
|
e = commentsUtil.newComment(ctx, path, psId, c.side(), c.message,
|
||||||
|
c.unresolved, parent);
|
||||||
} else {
|
} else {
|
||||||
e.writtenOn = ctx.getWhen();
|
e.writtenOn = ctx.getWhen();
|
||||||
e.side = c.side();
|
e.side = c.side();
|
||||||
e.message = c.message;
|
e.message = c.message;
|
||||||
}
|
}
|
||||||
|
|
||||||
if (parent != null) {
|
|
||||||
e.parentUuid = parent;
|
|
||||||
}
|
|
||||||
setCommentRevId(e, patchListCache, ctx.getChange(), ps);
|
setCommentRevId(e, patchListCache, ctx.getChange(), ps);
|
||||||
e.setLineNbrAndRange(c.line, c.range);
|
e.setLineNbrAndRange(c.line, c.range);
|
||||||
e.tag = in.tag;
|
e.tag = in.tag;
|
||||||
|
|||||||
@@ -164,6 +164,9 @@ public class PutDraftComment implements RestModifyView<DraftCommentResource, Dra
|
|||||||
// TODO(dborowitz): Can we support changing tags via PUT?
|
// TODO(dborowitz): Can we support changing tags via PUT?
|
||||||
e.tag = in.tag;
|
e.tag = in.tag;
|
||||||
}
|
}
|
||||||
|
if (in.unresolved != null) {
|
||||||
|
e.unresolved = in.unresolved;
|
||||||
|
}
|
||||||
return e;
|
return e;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -18,6 +18,7 @@ import com.google.common.base.Strings;
|
|||||||
import com.google.gerrit.common.TimeUtil;
|
import com.google.gerrit.common.TimeUtil;
|
||||||
import com.google.gerrit.extensions.client.Side;
|
import com.google.gerrit.extensions.client.Side;
|
||||||
import com.google.gerrit.extensions.restapi.RestApiException;
|
import com.google.gerrit.extensions.restapi.RestApiException;
|
||||||
|
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
|
||||||
import com.google.gerrit.reviewdb.client.Account;
|
import com.google.gerrit.reviewdb.client.Account;
|
||||||
import com.google.gerrit.reviewdb.client.Change;
|
import com.google.gerrit.reviewdb.client.Change;
|
||||||
import com.google.gerrit.reviewdb.client.ChangeMessage;
|
import com.google.gerrit.reviewdb.client.ChangeMessage;
|
||||||
@@ -179,7 +180,8 @@ public class MailProcessor {
|
|||||||
}
|
}
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
public boolean updateChange(ChangeContext ctx) throws OrmException {
|
public boolean updateChange(ChangeContext ctx) throws OrmException,
|
||||||
|
UnprocessableEntityException {
|
||||||
PatchSet ps = psUtil.get(ctx.getDb(), ctx.getNotes(), psId);
|
PatchSet ps = psUtil.get(ctx.getDb(), ctx.getNotes(), psId);
|
||||||
if (ps == null) {
|
if (ps == null) {
|
||||||
throw new OrmException("patch set not found: " + psId);
|
throw new OrmException("patch set not found: " + psId);
|
||||||
@@ -224,12 +226,14 @@ public class MailProcessor {
|
|||||||
}
|
}
|
||||||
|
|
||||||
Comment comment = commentsUtil.newComment(ctx, fileName,
|
Comment comment = commentsUtil.newComment(ctx, fileName,
|
||||||
psForComment.getId(), (short) side.ordinal(), c.message);
|
psForComment.getId(), (short) side.ordinal(), c.message,
|
||||||
|
false, null);
|
||||||
comment.tag = tag;
|
comment.tag = tag;
|
||||||
if (c.inReplyTo != null) {
|
if (c.inReplyTo != null) {
|
||||||
comment.parentUuid = c.inReplyTo.key.uuid;
|
comment.parentUuid = c.inReplyTo.key.uuid;
|
||||||
comment.lineNbr = c.inReplyTo.lineNbr;
|
comment.lineNbr = c.inReplyTo.lineNbr;
|
||||||
comment.range = c.inReplyTo.range;
|
comment.range = c.inReplyTo.range;
|
||||||
|
comment.unresolved = c.inReplyTo.unresolved;
|
||||||
}
|
}
|
||||||
CommentsUtil.setCommentRevId(comment, patchListCache,
|
CommentsUtil.setCommentRevId(comment, patchListCache,
|
||||||
ctx.getChange(), psForComment);
|
ctx.getChange(), psForComment);
|
||||||
|
|||||||
@@ -434,7 +434,7 @@ public class CommentSender extends ReplyToChangeSender {
|
|||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Retrieve the file lines refered to by a comment.
|
* Retrieve the file lines referred to by a comment.
|
||||||
* @param comment The comment that refers to some file contents. The comment
|
* @param comment The comment that refers to some file contents. The comment
|
||||||
* may be a line comment or a ranged comment.
|
* may be a line comment or a ranged comment.
|
||||||
* @param fileData The file on which the comment appears.
|
* @param fileData The file on which the comment appears.
|
||||||
|
|||||||
Reference in New Issue
Block a user