opendev/gerrit
Code Issues Proposed changes

DeleteDraftComment: Don't bump 'lastUpdatedOn'

Browse Source 
According to the implementation in change I3d8b13f70, posting a draft
inline comment to a change does not cause the change's 'lastUpdatedOn'
field to be updated.

However, the implementation of deleting a draft comment is inconsistent
with this, and does cause the 'lastUpdatedOn' to be updated.

Fix this inconsistency. Also add tests for both operations to confirm
that the timestamp is not updated.

Bug: Issue 4150
Change-Id: I222a702dd1dc529d4668a1dad6614a5839f63625
This commit is contained in:
David Pursehouse
2016-05-30 14:58:33 +09:00
parent ad6f15b4a7
commit fafc650b0f
2 changed files with 11 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files

10
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/change/CommentsIT.java
View File

@@ -152,6 +152,7 @@ public class CommentsIT extends AbstractDaemonTest {
public void putDraft() throws Exception {
for (Integer line : lines) {
PushOneCommit.Result r = createChange();
Timestamp origLastUpdated = r.getChange().change().getLastUpdatedOn();
String changeId = r.getChangeId();
String revId = r.getCommit().getName();
DraftInput comment = newDraft("file1", Side.REVISION, line, "comment 1");
@@ -165,6 +166,10 @@ public class CommentsIT extends AbstractDaemonTest {
result = getDraftComments(changeId, revId);
actual = Iterables.getOnlyElement(result.get(comment.path));
assertCommentInfo(comment, actual);
// Posting a draft comment doesn't cause lastUpdatedOn to change.
assertThat(r.getChange().change().getLastUpdatedOn())
.isEqualTo(origLastUpdated);
}
}
@@ -210,6 +215,7 @@ public class CommentsIT extends AbstractDaemonTest {
public void deleteDraft() throws Exception {
for (Integer line : lines) {
PushOneCommit.Result r = createChange();
Timestamp origLastUpdated = r.getChange().change().getLastUpdatedOn();
String changeId = r.getChangeId();
String revId = r.getCommit().getName();
DraftInput draft = newDraft("file1", Side.REVISION, line, "comment 1");
@@ -217,6 +223,10 @@ public class CommentsIT extends AbstractDaemonTest {
deleteDraft(changeId, revId, returned.id);
Map<String, List<CommentInfo>> drafts = getDraftComments(changeId, revId);
assertThat(drafts).isEmpty();
// Deleting a draft comment doesn't cause lastUpdatedOn to change.
assertThat(r.getChange().change().getLastUpdatedOn())
.isEqualTo(origLastUpdated);
}
}

1
gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteDraftComment.java
View File

@@ -102,6 +102,7 @@ public class DeleteDraftComment
setCommentRevId(c, patchListCache, ctx.getChange(), ps);
plcUtil.deleteComments(
ctx.getDb(), ctx.getUpdate(psId), Collections.singleton(c));
ctx.bumpLastUpdatedOn(false);
return true;
}
}