Skip to content
Permalink
Browse files
Use quoting to direct comment threading
Reviewed-by: ehelin
  • Loading branch information
rwestberg committed Mar 5, 2020
1 parent de5f4b5 commit eb543319fc9989df449cef264419f1a8961aa71f
Showing 3 changed files with 101 additions and 2 deletions.
@@ -151,6 +151,28 @@ private static Optional<ArchiveItem> findLastMention(String commentText, List<Ar
return Optional.empty();
}

static boolean containsQuote(String quote, String body) {
var compactQuote = quote.lines()
.takeWhile(line -> line.startsWith(">"))
.map(line -> line.replaceAll("\\W", ""))
.collect(Collectors.joining());
if (!compactQuote.isBlank()) {
var compactBody = body.replaceAll("\\W", "");
return compactBody.contains(compactQuote);
} else {
return false;
}
}

private static Optional<ArchiveItem> findLastQuoted(String commentText, List<ArchiveItem> eligibleParents) {
for (int i = eligibleParents.size() - 1; i != 0; --i) {
if (containsQuote(commentText, eligibleParents.get(i).body())) {
return Optional.of(eligibleParents.get(i));
}
}
return Optional.empty();
}

static ArchiveItem findParent(List<ArchiveItem> generated, Comment comment) {
ArchiveItem lastCommentOrReview = generated.get(0);
var eligible = new ArrayList<ArchiveItem>();
@@ -168,6 +190,10 @@ static ArchiveItem findParent(List<ArchiveItem> generated, Comment comment) {
if (lastMention.isPresent()) {
return lastMention.get();
}
var lastQuoted = findLastQuoted(comment.body(), eligible);
if (lastQuoted.isPresent()) {
return lastQuoted.get();
}

return lastCommentOrReview;
}
@@ -160,7 +160,15 @@ private List<ArchiveItem> parentsToQuote(ArchiveItem item, int quoteLevel, Set<A
return ret;
}

private String quoteSelectedParents(List<ArchiveItem> parentsToQuote) {
// Parents to quote are provided with the newest item first. If the item already has quoted
// a parent, use that as the quote and return an empty string.
private String quoteSelectedParents(List<ArchiveItem> parentsToQuote, ArchiveItem first) {
if (parentsToQuote.isEmpty()) {
return "";
}
if (ArchiveItem.containsQuote(first.body(), parentsToQuote.get(0).body())) {
return "";
}
Collections.reverse(parentsToQuote);
var ret = "";
for (var parent : parentsToQuote) {
@@ -233,7 +241,7 @@ List<Email> generateNewEmails(List<Email> sentEmails, Duration cooldown, Reposit
body.append("\n\n");
}
var newQuotes = parentsToQuote(item, 2, quotedParents);
var quote = quoteSelectedParents(newQuotes);
var quote = quoteSelectedParents(newQuotes, item);
if (!quote.isBlank()) {
body.append(quote);
body.append("\n\n");
@@ -779,6 +779,71 @@ void reviewCommentWithMention(TestInfo testInfo) throws IOException {
}
}

@Test
void commentWithQuote(TestInfo testInfo) throws IOException {
try (var credentials = new HostCredentials(testInfo);
var tempFolder = new TemporaryDirectory();
var archiveFolder = new TemporaryDirectory();
var listServer = new TestMailmanServer();
var webrevServer = new TestWebrevServer()) {
var author = credentials.getHostedRepository();
var reviewer = credentials.getHostedRepository();
var archive = credentials.getHostedRepository();
var listAddress = EmailAddress.parse(listServer.createList("test"));
var censusBuilder = credentials.getCensusBuilder()
.addReviewer(reviewer.forge().currentUser().id())
.addAuthor(author.forge().currentUser().id());
var from = EmailAddress.from("test", "test@test.mail");
var mlBot = MailingListBridgeBot.newBuilder()
.from(from)
.repo(author)
.archive(archive)
.censusRepo(censusBuilder.build())
.list(listAddress)
.listArchive(listServer.getArchive())
.smtpServer(listServer.getSMTP())
.webrevStorageRepository(archive)
.webrevStorageRef("webrev")
.webrevStorageBase(Path.of("test"))
.webrevStorageBaseUri(webrevServer.uri())
.issueTracker(URIBuilder.base("http://issues.test/browse/").build())
.build();

// Populate the projects repository
var reviewFile = Path.of("reviewfile.txt");
var localRepo = CheckableRepository.init(tempFolder.path(), author.repositoryType(), reviewFile);
var masterHash = localRepo.resolve("master").orElseThrow();
localRepo.push(masterHash, author.url(), "master", true);
localRepo.push(masterHash, archive.url(), "webrev", true);

// Make a change with a corresponding PR
var editHash = CheckableRepository.appendAndCommit(localRepo);
localRepo.push(editHash, author.url(), "edit", true);
var pr = credentials.createPullRequest(archive, "master", "edit", "This is a pull request");
pr.setBody("This is now ready");
TestBotRunner.runPeriodicItems(mlBot);
listServer.processIncoming();

// Make two comments from different authors
var reviewPr = reviewer.pullRequest(pr.id());
reviewPr.addComment("First comment\nsecond line");
pr.addComment("Second comment\nfourth line");

TestBotRunner.runPeriodicItems(mlBot);
listServer.processIncoming();

pr.addComment(">First comm\n\nreply to first");
TestBotRunner.runPeriodicItems(mlBot);
listServer.processIncoming();

// The first comment should be quoted more often than the second
Repository.materialize(archiveFolder.path(), archive.url(), "master");
assertEquals(2, archiveContainsCount(archiveFolder.path(), "First comment"));
assertEquals(3, archiveContainsCount(archiveFolder.path(), "First comm"));
assertEquals(1, archiveContainsCount(archiveFolder.path(), "Second comment"));
}
}

@Test
void reviewContext(TestInfo testInfo) throws IOException {
try (var credentials = new HostCredentials(testInfo);

0 comments on commit eb54331

Please sign in to comment.