Allow to tag reviews

In order to be able to filter out non-human comments and votes we need
to tag them.

This change introduces new property to the ReviewInput object called
'tag'. This allows us to add some meta information about where this vote
come from. Then in the UI we can show/hide comments and votes that have
different tags.

Also ApprovalInfo, CommentInfo and ChangeMessageInfo were extended to
include value of 'tag' property read from DB.

To be able to persist those data new column (tag) was introduced to
change_messages, patch_set_comments and patch_set_approvals tables.

Change-Id: If6378c5a9f4e0673c00ab348297549f27a06110b
Signed-off-by: Dariusz Luksza <dariusz@luksza.org>
This commit is contained in:
Dariusz Luksza
2016-03-15 14:05:51 +01:00
committed by Dave Borowitz
parent b50cd86eff
commit c70e862596
25 changed files with 379 additions and 31 deletions

View File

@@ -280,14 +280,14 @@ public class ChangeBundleTest {
assertDiffs(b1, b2,
"Differing numbers of ChangeMessages for Change.Id " + id + ":\n"
+ "ChangeMessage{key=" + id + ",uuid1, author=100,"
+ " writtenOn=2009-09-30 17:00:06.0, patchset=" + id + ",1,"
+ " writtenOn=2009-09-30 17:00:06.0, patchset=" + id + ",1, tag=null,"
+ " message=[message 2]}\n"
+ "ChangeMessage{key=" + id + ",uuid2, author=100,"
+ " writtenOn=2009-09-30 17:00:12.0, patchset=" + id + ",1,"
+ " writtenOn=2009-09-30 17:00:12.0, patchset=" + id + ",1, tag=null,"
+ " message=[null]}\n"
+ "--- vs. ---\n"
+ "ChangeMessage{key=" + id + ",uuid1, author=100,"
+ " writtenOn=2009-09-30 17:00:06.0, patchset=" + id + ",1,"
+ " writtenOn=2009-09-30 17:00:06.0, patchset=" + id + ",1, tag=null,"
+ " message=[message 2]}");
}

View File

@@ -414,6 +414,32 @@ public class ChangeNotesParserTest extends AbstractChangeNotesTest {
assertParseFails(writeCommit(msg, serverIdent));
}
@Test
public void parseTag() throws Exception {
assertParseSucceeds("Update change\n"
+ "\n"
+ "Patch-Set: 1\n"
+ "Branch: refs/heads/master\n"
+ "Change-id: I577fb248e474018276351785930358ec0450e9f7\n"
+ "Subject: Change subject\n"
+ "Tag:\n");
assertParseSucceeds("Update change\n"
+ "\n"
+ "Patch-Set: 1\n"
+ "Branch: refs/heads/master\n"
+ "Change-id: I577fb248e474018276351785930358ec0450e9f7\n"
+ "Subject: Change subject\n"
+ "Tag: jenkins\n");
assertParseFails("Update change\n"
+ "\n"
+ "Patch-Set: 1\n"
+ "Branch: refs/heads/master\n"
+ "Change-id: I577fb248e474018276351785930358ec0450e9f7\n"
+ "Subject: Change subject\n"
+ "Tag: ci\n"
+ "Tag: jenkins\n");
}
private RevCommit writeCommit(String body) throws Exception {
return writeCommit(body, noteUtil.newIdent(
changeOwner.getAccount(), TimeUtil.nowTs(), serverIdent,

View File

@@ -69,6 +69,117 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
@Inject
private DraftCommentNotes.Factory draftNotesFactory;
@Test
public void tagChangeMessage() throws Exception {
String tag = "jenkins";
Change c = newChange();
ChangeUpdate update = newUpdate(c, changeOwner);
update.setChangeMessage("verification from jenkins");
update.setTag(tag);
update.commit();
ChangeNotes notes = newNotes(c);
assertThat(notes.getChangeMessages()).hasSize(1);
assertThat(notes.getChangeMessages().get(0).getTag()).isEqualTo(tag);
}
@Test
public void tagInlineCommenrts() throws Exception {
String tag = "jenkins";
Change c = newChange();
RevCommit commit = tr.commit().message("PS2").create();
ChangeUpdate update = newUpdate(c, changeOwner);
update.putComment(newPublishedComment(c.currentPatchSetId(), "a.txt",
"uuid1", new CommentRange(1, 2, 3, 4), 1, changeOwner, null,
TimeUtil.nowTs(), "Comment", (short) 1, commit.name()));
update.setTag(tag);
update.commit();
ChangeNotes notes = newNotes(c);
ImmutableListMultimap<RevId, PatchLineComment> comments = notes.getComments();
assertThat(comments).hasSize(1);
assertThat(
comments.entries().asList().get(0).getValue().getTag())
.isEqualTo(tag);
}
@Test
public void tagApprovals() throws Exception {
String tag1 = "jenkins";
String tag2 = "ip";
Change c = newChange();
ChangeUpdate update = newUpdate(c, changeOwner);
update.putApproval("Verified", (short) -1);
update.setTag(tag1);
update.commit();
update = newUpdate(c, changeOwner);
update.putApproval("Verified", (short) 1);
update.setTag(tag2);
update.commit();
ChangeNotes notes = newNotes(c);
ImmutableListMultimap<PatchSet.Id, PatchSetApproval> approvals =
notes.getApprovals();
assertThat(approvals).hasSize(2);
assertThat(approvals.entries().asList().get(0).getValue().getTag())
.isEqualTo(tag1);
assertThat(approvals.entries().asList().get(1).getValue().getTag())
.isEqualTo(tag2);
}
@Test
public void multipleTags() throws Exception {
String ipTag = "ip";
String coverageTag = "coverage";
String integrationTag = "integration";
Change c = newChange();
ChangeUpdate update = newUpdate(c, changeOwner);
update.putApproval("Verified", (short) -1);
update.setChangeMessage("integration verification");
update.setTag(integrationTag);
update.commit();
RevCommit commit = tr.commit().message("PS2").create();
update = newUpdate(c, changeOwner);
update.putComment(newPublishedComment(c.currentPatchSetId(), "a.txt",
"uuid1", new CommentRange(1, 2, 3, 4), 1, changeOwner, null,
TimeUtil.nowTs(), "Comment", (short) 1, commit.name()));
update.setChangeMessage("coverage verification");
update.setTag(coverageTag);
update.commit();
update = newUpdate(c, changeOwner);
update.setChangeMessage("ip clear");
update.setTag(ipTag);
update.commit();
ChangeNotes notes = newNotes(c);
ImmutableListMultimap<PatchSet.Id, PatchSetApproval> approvals =
notes.getApprovals();
assertThat(approvals).hasSize(1);
PatchSetApproval approval = approvals.entries().asList().get(0).getValue();
assertThat(approval.getTag()).isEqualTo(integrationTag);
assertThat(approval.getValue()).isEqualTo(-1);
ImmutableListMultimap<RevId, PatchLineComment> comments =
notes.getComments();
assertThat(comments).hasSize(1);
assertThat(comments.entries().asList().get(0).getValue().getTag())
.isEqualTo(coverageTag);
ImmutableList<ChangeMessage> messages = notes.getChangeMessages();
assertThat(messages).hasSize(3);
assertThat(messages.get(0).getTag()).isEqualTo(integrationTag);
assertThat(messages.get(1).getTag()).isEqualTo(coverageTag);
assertThat(messages.get(2).getTag()).isEqualTo(ipTag);
}
@Test
public void approvalsOnePatchSet() throws Exception {
Change c = newChange();

View File

@@ -272,6 +272,23 @@ public class CommitMessageOutputTest extends AbstractChangeNotesTest {
update.getResult());
}
@Test
public void changeMessageWithTag() throws Exception {
Change c = newChange();
ChangeUpdate update = newUpdate(c, changeOwner);
update.setChangeMessage("Change message with tag");
update.setTag("jenkins");
update.commit();
assertBodyEquals("Update patch set 1\n"
+ "\n"
+ "Change message with tag\n"
+ "\n"
+ "Patch-set: 1\n"
+ "Tag: jenkins\n",
update.getResult());
}
private RevCommit parseCommit(ObjectId id) throws Exception {
if (id instanceof RevCommit) {
return (RevCommit) id;