Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

715: Generated email wrongly includes previous unrelated comments #1108

Closed
wants to merge 2 commits into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -297,14 +297,11 @@ private static Optional<ArchiveItem> findLastQuoted(String commentText, List<Arc
}

static ArchiveItem findParent(List<ArchiveItem> generated, Comment comment) {
ArchiveItem lastCommentOrReview = generated.get(0);
var eligible = new ArrayList<ArchiveItem>();
eligible.add(lastCommentOrReview);
for (var item : generated) {
if (item.id().startsWith("pc") || item.id().startsWith("rv")) {
if (item.createdAt().isBefore(comment.createdAt()) && item.createdAt().isAfter(lastCommentOrReview.createdAt())) {
lastCommentOrReview = item;
eligible.add(lastCommentOrReview);
if (item.createdAt().isBefore(comment.createdAt())) {
eligible.add(item);
}
}
}
@@ -318,7 +315,15 @@ static ArchiveItem findParent(List<ArchiveItem> generated, Comment comment) {
return lastQuoted.get();
}

return lastCommentOrReview;
ArchiveItem lastRevisionItem = generated.get(0);
for (var item : generated) {
if (item.id().startsWith("ha")) {
if (item.createdAt().isBefore(comment.createdAt())) {
lastRevisionItem = item;
}
}
}
return lastRevisionItem;
}

static ArchiveItem findRevisionItem(List<ArchiveItem> generated, Hash hash) {
@@ -23,12 +23,14 @@
package org.openjdk.skara.bots.mlbridge;

import org.junit.jupiter.api.*;
import org.openjdk.skara.forge.PullRequest;
import org.openjdk.skara.forge.*;
import org.openjdk.skara.host.HostUser;
import org.openjdk.skara.issuetracker.Comment;
import org.openjdk.skara.test.HostCredentials;
import org.openjdk.skara.test.*;
import org.openjdk.skara.vcs.Repository;

import java.io.IOException;
import java.net.URI;
import java.time.ZonedDateTime;
import java.util.List;

@@ -41,14 +43,21 @@ private Comment createComment(HostUser user, String body) {
return new Comment(Integer.toString(curId++), body, user, ZonedDateTime.now(), ZonedDateTime.now());
}

private ArchiveItem fromPullRequest(PullRequest pr, Repository repo) throws IOException {
var base = repo.resolve("master").orElseThrow();
return ArchiveItem.from(pr, repo, null, URI.create("http://www.example.com"), "", null, null, ZonedDateTime.now(), ZonedDateTime.now(), base, base, "", "");
}

private ArchiveItem fromComment(PullRequest pr, Comment comment) {
return ArchiveItem.from(pr, comment, null, null);
}

@Test
void simple(TestInfo testInfo) throws IOException {
try (var credentials = new HostCredentials(testInfo)) {
try (var credentials = new HostCredentials(testInfo);
var tempFolder = new TemporaryDirectory()) {
var repo = credentials.getHostedRepository();
var localRepo = CheckableRepository.init(tempFolder.path(), repo.repositoryType());
var pr = credentials.createPullRequest(repo, "master", "master", "Test");

var user1 = HostUser.create("1", "user1", "User Uno");
@@ -58,17 +67,18 @@ void simple(TestInfo testInfo) throws IOException {
var c1 = createComment(user1, "First comment\nwith two lines");
var c2 = createComment(user2, "Second comment");

var a0 = fromPullRequest(pr, localRepo);
var a1 = fromComment(pr, c1);
var a2 = fromComment(pr, c2);

assertEquals(a2, ArchiveItem.findParent(List.of(a1, a2), createComment(user3, "Plain reply")));
assertEquals(a0, ArchiveItem.findParent(List.of(a0, a1, a2), createComment(user3, "Plain unrelated reply")));

assertEquals(a1, ArchiveItem.findParent(List.of(a1, a2), createComment(user3, "> First comment\n\nI agree")));
assertEquals(a1, ArchiveItem.findParent(List.of(a1, a2), createComment(user3, "> First comment\n>with two lines\n\nI agree")));
assertEquals(a1, ArchiveItem.findParent(List.of(a1, a2), createComment(user3, "\n> First comment\n\nI agree")));
assertEquals(a1, ArchiveItem.findParent(List.of(a0, a1, a2), createComment(user3, "> First comment\n\nI agree")));
assertEquals(a1, ArchiveItem.findParent(List.of(a0, a1, a2), createComment(user3, "> First comment\n>with two lines\n\nI agree")));
assertEquals(a1, ArchiveItem.findParent(List.of(a0, a1, a2), createComment(user3, "\n> First comment\n\nI agree")));

assertEquals(a1, ArchiveItem.findParent(List.of(a1, a2), createComment(user3, "@user1 I agree")));
assertEquals(a1, ArchiveItem.findParent(List.of(a1, a2), createComment(user3, "@user1\nI agree")));
assertEquals(a1, ArchiveItem.findParent(List.of(a0, a1, a2), createComment(user3, "@user1 I agree")));
assertEquals(a1, ArchiveItem.findParent(List.of(a0, a1, a2), createComment(user3, "@user1\nI agree")));
}
}
}
@@ -22,16 +22,15 @@
*/
package org.openjdk.skara.bots.mlbridge;

import org.junit.jupiter.api.*;
import org.openjdk.skara.email.EmailAddress;
import org.openjdk.skara.forge.*;
import org.openjdk.skara.issuetracker.Issue;
import org.openjdk.skara.json.JSON;
import org.openjdk.skara.mailinglist.*;
import org.openjdk.skara.network.URIBuilder;
import org.openjdk.skara.mailinglist.MailingListServerFactory;
import org.openjdk.skara.test.*;
import org.openjdk.skara.vcs.Repository;
import org.openjdk.skara.json.JSON;

import org.junit.jupiter.api.*;

import java.io.*;
import java.nio.charset.StandardCharsets;
@@ -259,7 +258,7 @@ void simpleArchive(TestInfo testInfo) throws IOException {

// Remove the rfr flag and post another comment
pr.addLabel("rfr");
pr.addComment("This is another comment");
pr.addComment("@" + pr.author().username() +" This is another comment");

// Run another archive pass
TestBotRunner.runPeriodicItems(mlBot);
@@ -1178,7 +1177,7 @@ void commentWithMention(TestInfo testInfo) throws IOException {

// The first comment should be quoted more often than the second
Repository.materialize(archiveFolder.path(), archive.url(), "master");
assertEquals(3, archiveContainsCount(archiveFolder.path(), "First comment"));
assertEquals(2, archiveContainsCount(archiveFolder.path(), "First comment"));
assertEquals(1, archiveContainsCount(archiveFolder.path(), "Second comment"));
}
}
@@ -1296,7 +1295,8 @@ void commentWithQuote(TestInfo testInfo) throws IOException {
// Make two comments from different authors
var reviewPr = reviewer.pullRequest(pr.id());
reviewPr.addComment("First comment\nsecond line");
pr.addComment("Second comment\nfourth line");
var authorPr = author.pullRequest(pr.id());
authorPr.addComment("Second comment\nfourth line");

TestBotRunner.runPeriodicItems(mlBot);
listServer.processIncoming();
@@ -1305,11 +1305,10 @@ void commentWithQuote(TestInfo testInfo) throws IOException {
TestBotRunner.runPeriodicItems(mlBot);
listServer.processIncoming();

// The first comment should be quoted more often than the second
// The first comment should be replied to once, and the original post once
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"));
assertEquals(1, archiveContainsCount(archiveFolder.path(), Pattern.quote(reviewPr.author().fullName()) + ".* wrote"));
assertEquals(1, archiveContainsCount(archiveFolder.path(), Pattern.quote(pr.author().fullName()) + ".* wrote"));
}
}

@@ -2352,7 +2351,7 @@ void replyToEmptyReview(TestInfo testInfo) throws IOException {
TestBotRunner.runPeriodicItems(mlBot);
listServer.processIncoming();

pr.addComment("Thanks for the review!");
pr.addComment("@" + reviewer.forge().currentUser().username() + " Thanks for the review!");
TestBotRunner.runPeriodicItems(mlBot);
listServer.processIncoming();

@@ -3107,7 +3106,7 @@ void jsonArchive(TestInfo testInfo) throws IOException {

// Remove the rfr flag and post another comment
pr.addLabel("rfr");
pr.addComment("This is another comment");
pr.addComment("@" + pr.author().username() + " This is another comment");

// Run another archive pass
TestBotRunner.runPeriodicItems(mlBot);