diff --git a/src/main/java/io/quarkus/github/lottery/draw/LotteryReport.java b/src/main/java/io/quarkus/github/lottery/draw/LotteryReport.java index d8e289c..ef640cf 100644 --- a/src/main/java/io/quarkus/github/lottery/draw/LotteryReport.java +++ b/src/main/java/io/quarkus/github/lottery/draw/LotteryReport.java @@ -5,8 +5,10 @@ import java.util.List; import java.util.Optional; import java.util.stream.Collectors; +import java.util.stream.Stream; import com.fasterxml.jackson.annotation.JsonAlias; + import io.quarkus.github.lottery.github.Issue; import io.quarkus.runtime.annotations.RegisterForReflection; @@ -21,9 +23,23 @@ public record LotteryReport( Optional stale, Optional stewardship) { + Stream buckets() { + return Stream.of(triage, feedbackNeeded, feedbackProvided, stale, stewardship) + .filter(Optional::isPresent) + .map(Optional::get); + } + + public boolean hasContent() { + return buckets().anyMatch(Bucket::hasContent); + } + public record Bucket( List issues) { + public boolean hasContent() { + return !issues().isEmpty(); + } + public Serialized serialized() { return new Serialized(issues.stream().map(Issue::number).collect(Collectors.toList())); } diff --git a/src/main/java/io/quarkus/github/lottery/github/GitHubRepository.java b/src/main/java/io/quarkus/github/lottery/github/GitHubRepository.java index 8df87c7..58b9f0c 100644 --- a/src/main/java/io/quarkus/github/lottery/github/GitHubRepository.java +++ b/src/main/java/io/quarkus/github/lottery/github/GitHubRepository.java @@ -13,7 +13,6 @@ import java.util.Optional; import java.util.Set; import java.util.function.Function; -import java.util.function.Predicate; import java.util.stream.Stream; import org.kohsuke.github.GHDirection; @@ -234,7 +233,7 @@ private Topic(TopicRef ref) { * * @throws IOException If a GitHub API call fails. * @throws java.io.UncheckedIOException If a GitHub API call fails. - * @see #comment(String, String) + * @see #update(String, String, Optional) */ public boolean isClosed() throws IOException { var existingIssue = getDedicatedIssues().findFirst(); @@ -242,7 +241,8 @@ public boolean isClosed() throws IOException { } /** - * Adds a comment to an issue identified by its assignee and topic (title prefix). + * Updates an issue identified by its assignee and topic (title prefix), + * changing the summary in its description and commenting if necessary. * * @param topicSuffix A string that should be appended to the topic in the issue title. * Each time that suffix changes for a new comment, @@ -251,12 +251,13 @@ public boolean isClosed() throws IOException { * In conversation-based email clients such as GMail, * this will result in the comment appearing in a new conversation, * which can be useful to avoid huge conversations. - * @param markdownBody The body of the comment to add. + * @param markdownBody The body of the description to update. + * @param comment Whether The body should also be added as a comment, triggering a GitHub notification. * * @throws IOException If a GitHub API call fails. * @throws java.io.UncheckedIOException If a GitHub API call fails. */ - public void comment(String topicSuffix, String markdownBody) + public void update(String topicSuffix, String markdownBody, boolean comment) throws IOException { var dedicatedIssue = getDedicatedIssues().findFirst(); if (ref.expectedSuffixStart() != null && !topicSuffix.startsWith(ref.expectedSuffixStart()) @@ -297,7 +298,9 @@ public void comment(String topicSuffix, String markdownBody) issue = createDedicatedIssue(targetTitle, markdownBody); } - issue.comment(markdownBody); + if (comment) { + issue.comment(markdownBody); + } } private Stream getDedicatedIssues() throws IOException { diff --git a/src/main/java/io/quarkus/github/lottery/history/HistoryService.java b/src/main/java/io/quarkus/github/lottery/history/HistoryService.java index f0525d9..a75b540 100644 --- a/src/main/java/io/quarkus/github/lottery/history/HistoryService.java +++ b/src/main/java/io/quarkus/github/lottery/history/HistoryService.java @@ -4,8 +4,8 @@ import java.io.IOException; import java.util.List; +import java.util.Optional; -import io.quarkus.github.lottery.github.TopicRef; import jakarta.enterprise.context.ApplicationScoped; import jakarta.inject.Inject; @@ -14,6 +14,7 @@ import io.quarkus.github.lottery.draw.LotteryReport; import io.quarkus.github.lottery.github.GitHubRepository; import io.quarkus.github.lottery.github.GitHubService; +import io.quarkus.github.lottery.github.TopicRef; import io.quarkus.github.lottery.message.MessageFormatter; import io.quarkus.github.lottery.notification.NotificationService; @@ -36,8 +37,8 @@ public LotteryHistory fetch(DrawRef drawRef, LotteryConfig config) throws IOExce public void append(DrawRef drawRef, LotteryConfig config, List reports) throws IOException { var persistenceRepo = persistenceRepo(drawRef, config); - String commentBody = messageFormatter.formatHistoryBodyMarkdown(drawRef, reports); - persistenceRepo.topic(historyTopic(drawRef)).comment("", commentBody); + String body = messageFormatter.formatHistoryBodyMarkdown(drawRef, reports); + persistenceRepo.topic(historyTopic(drawRef)).update("", body, true); } private TopicRef historyTopic(DrawRef drawRef) { diff --git a/src/main/java/io/quarkus/github/lottery/notification/Notifier.java b/src/main/java/io/quarkus/github/lottery/notification/Notifier.java index 50cd505..5912a4a 100644 --- a/src/main/java/io/quarkus/github/lottery/notification/Notifier.java +++ b/src/main/java/io/quarkus/github/lottery/notification/Notifier.java @@ -1,6 +1,7 @@ package io.quarkus.github.lottery.notification; import java.io.IOException; +import java.util.Optional; import io.quarkus.github.lottery.draw.DrawRef; import io.quarkus.github.lottery.draw.LotteryReport; @@ -38,7 +39,10 @@ public void send(LotteryReport report) throws IOException { String topicSuffix = formatter.formatNotificationTopicSuffixText(report); String body = formatter.formatNotificationBodyMarkdown(report, notificationRepository.ref()); notificationRepository.topic(notificationTopic(report.username())) - .comment(topicSuffix, body); + .update(topicSuffix, body, + // When the report has no content, we update the topic's description, + // but we don't comment, because that would trigger an unnecessary notification. + report.hasContent()); } private TopicRef notificationTopic(String username) { diff --git a/src/test/java/io/quarkus/github/lottery/GitHubServiceTest.java b/src/test/java/io/quarkus/github/lottery/GitHubServiceTest.java index 682e3d2..0a3b3cd 100644 --- a/src/test/java/io/quarkus/github/lottery/GitHubServiceTest.java +++ b/src/test/java/io/quarkus/github/lottery/GitHubServiceTest.java @@ -972,7 +972,7 @@ void topic_extractComments_dedicatedIssueExists_appCommentsExist_withConfusingOt @SuppressWarnings("unchecked") @Test - void topic_comment_dedicatedIssueExists_open() throws Exception { + void topic_update_dedicatedIssueExists_open() throws Exception { var repoRef = new GitHubRepositoryRef(installationRef, "quarkusio/quarkus-lottery-reports"); var commentToMinimizeNodeId = "MDM6Qm90NzUwNjg0Mzg="; @@ -1019,7 +1019,7 @@ void topic_comment_dedicatedIssueExists_open() throws Exception { var repo = gitHubService.repository(repoRef); repo.topic(TopicRef.notification("yrodiere", "yrodiere's report for quarkusio/quarkus")) - .comment(" (updated 2017-11-06T06:00:00Z)", "Some content"); + .update(" (updated 2017-11-06T06:00:00Z)", "Some content", true); }) .then().github(mocks -> { verify(searchIssuesBuilderMock).q("repo:" + repoRef.repositoryName()); @@ -1045,7 +1045,7 @@ void topic_comment_dedicatedIssueExists_open() throws Exception { @SuppressWarnings("unchecked") @Test - void topic_comment_dedicatedIssueExists_noTopicSuffix() throws Exception { + void topic_update_dedicatedIssueExists_noTopicSuffix() throws Exception { var repoRef = new GitHubRepositoryRef(installationRef, "quarkusio/quarkus-lottery-reports"); var commentToMinimizeNodeId = "MDM6Qm90NzUwNjg0Mzg="; @@ -1091,7 +1091,7 @@ void topic_comment_dedicatedIssueExists_noTopicSuffix() throws Exception { var repo = gitHubService.repository(repoRef); repo.topic(TopicRef.history("Lottery history for quarkusio/quarkus")) - .comment("", "Some content"); + .update("", "Some content", true); }) .then().github(mocks -> { verify(searchIssuesBuilderMock).q("repo:" + repoRef.repositoryName()); @@ -1115,7 +1115,7 @@ void topic_comment_dedicatedIssueExists_noTopicSuffix() throws Exception { @SuppressWarnings("unchecked") @Test - void topic_comment_dedicatedIssueExists_withConfusingOther() throws Exception { + void topic_update_dedicatedIssueExists_withConfusingOther() throws Exception { var repoRef = new GitHubRepositoryRef(installationRef, "quarkusio/quarkus-lottery-reports"); var commentToMinimizeNodeId = "MDM6Qm90NzUwNjg0Mzg="; @@ -1161,7 +1161,7 @@ void topic_comment_dedicatedIssueExists_withConfusingOther() throws Exception { var repo = gitHubService.repository(repoRef); repo.topic(TopicRef.history("Lottery history for quarkusio/quarkus")) - .comment("", "Some content"); + .update("", "Some content", true); }) .then().github(mocks -> { verify(searchIssuesBuilderMock).q("repo:" + repoRef.repositoryName()); @@ -1185,7 +1185,7 @@ void topic_comment_dedicatedIssueExists_withConfusingOther() throws Exception { @SuppressWarnings("unchecked") @Test - void topic_comment_dedicatedIssueExists_closed() throws Exception { + void topic_update_dedicatedIssueExists_closed() throws Exception { var repoRef = new GitHubRepositoryRef(installationRef, "quarkusio/quarkus-lottery-reports"); var commentToMinimizeNodeId = "MDM6Qm90NzUwNjg0Mzg="; @@ -1232,7 +1232,7 @@ void topic_comment_dedicatedIssueExists_closed() throws Exception { var repo = gitHubService.repository(repoRef); repo.topic(TopicRef.notification("yrodiere", "yrodiere's report for quarkusio/quarkus")) - .comment(" (updated 2017-11-06T06:00:00Z)", "Some content"); + .update(" (updated 2017-11-06T06:00:00Z)", "Some content", true); }) .then().github(mocks -> { verify(searchIssuesBuilderMock).q("repo:" + repoRef.repositoryName()); @@ -1259,7 +1259,7 @@ void topic_comment_dedicatedIssueExists_closed() throws Exception { } @Test - void topic_comment_dedicatedIssueDoesNotExist() throws IOException { + void topic_update_dedicatedIssueDoesNotExist() throws IOException { var repoRef = new GitHubRepositoryRef(installationRef, "quarkusio/quarkus-lottery-reports"); var searchIssuesBuilderMock = Mockito.mock(GHIssueSearchBuilder.class, withSettings().defaultAnswer(Answers.RETURNS_SELF)); @@ -1288,7 +1288,7 @@ void topic_comment_dedicatedIssueDoesNotExist() throws IOException { var repo = gitHubService.repository(repoRef); repo.topic(TopicRef.notification("yrodiere", "yrodiere's report for quarkusio/quarkus")) - .comment(" (updated 2017-11-06T06:00:00Z)", "Some content"); + .update(" (updated 2017-11-06T06:00:00Z)", "Some content", true); }) .then().github(mocks -> { var repositoryMock = mocks.repository(repoRef.repositoryName()); @@ -1309,7 +1309,7 @@ void topic_comment_dedicatedIssueDoesNotExist() throws IOException { } @Test - void topic_comment_dedicatedIssueDoesNotExist_withConfusingOther() throws IOException { + void topic_update_dedicatedIssueDoesNotExist_withConfusingOther() throws IOException { var repoRef = new GitHubRepositoryRef(installationRef, "quarkusio/quarkus-lottery-reports"); var searchIssuesBuilderMock = Mockito.mock(GHIssueSearchBuilder.class, withSettings().defaultAnswer(Answers.RETURNS_SELF)); @@ -1338,7 +1338,7 @@ void topic_comment_dedicatedIssueDoesNotExist_withConfusingOther() throws IOExce var repo = gitHubService.repository(repoRef); repo.topic(TopicRef.notification("yrodiere", "yrodiere's report for quarkusio/quarkus")) - .comment(" (updated 2017-11-06T06:00:00Z)", "Some content"); + .update(" (updated 2017-11-06T06:00:00Z)", "Some content", true); }) .then().github(mocks -> { var repositoryMock = mocks.repository(repoRef.repositoryName()); diff --git a/src/test/java/io/quarkus/github/lottery/NotificationServiceTest.java b/src/test/java/io/quarkus/github/lottery/NotificationServiceTest.java index ec19710..f0c3f01 100644 --- a/src/test/java/io/quarkus/github/lottery/NotificationServiceTest.java +++ b/src/test/java/io/quarkus/github/lottery/NotificationServiceTest.java @@ -11,23 +11,23 @@ import java.time.ZoneOffset; import java.util.Optional; -import io.quarkus.github.lottery.github.TopicRef; import jakarta.inject.Inject; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; -import io.quarkus.github.lottery.github.GitHubInstallationRef; import org.mockito.Mockito; import org.mockito.junit.jupiter.MockitoExtension; import io.quarkus.github.lottery.config.LotteryConfig; import io.quarkus.github.lottery.draw.DrawRef; import io.quarkus.github.lottery.draw.LotteryReport; +import io.quarkus.github.lottery.github.GitHubInstallationRef; import io.quarkus.github.lottery.github.GitHubRepository; import io.quarkus.github.lottery.github.GitHubRepositoryRef; import io.quarkus.github.lottery.github.GitHubService; +import io.quarkus.github.lottery.github.TopicRef; import io.quarkus.github.lottery.message.MessageFormatter; import io.quarkus.github.lottery.notification.NotificationService; import io.quarkus.github.lottery.notification.Notifier; @@ -103,12 +103,15 @@ void send() throws IOException { GitHubRepository.Topic notificationTopicYrodiereMock = Mockito.mock(GitHubRepository.Topic.class); GitHubRepository.Topic notificationTopicGsmetMock = Mockito.mock(GitHubRepository.Topic.class); GitHubRepository.Topic notificationTopicGeoandMock = Mockito.mock(GitHubRepository.Topic.class); + GitHubRepository.Topic notificationTopicJsmithMock = Mockito.mock(GitHubRepository.Topic.class); when(notificationRepoMock.topic(TopicRef.notification("yrodiere", "yrodiere's report for quarkusio/quarkus"))) .thenReturn(notificationTopicYrodiereMock); when(notificationRepoMock.topic(TopicRef.notification("gsmet", "gsmet's report for quarkusio/quarkus"))) .thenReturn(notificationTopicGsmetMock); when(notificationRepoMock.topic(TopicRef.notification("geoand", "geoand's report for quarkusio/quarkus"))) .thenReturn(notificationTopicGeoandMock); + when(notificationRepoMock.topic(TopicRef.notification("jsmith", "jsmith's report for quarkusio/quarkus"))) + .thenReturn(notificationTopicJsmithMock); Notifier notifier = notificationService.notifier(drawRef, config); verifyNoMoreInteractions(gitHubServiceMock, notificationRepoMock, messageFormatterMock); @@ -125,7 +128,8 @@ void send() throws IOException { when(messageFormatterMock.formatNotificationBodyMarkdown(lotteryReport1, notificationRepoRef)) .thenReturn(markdownNotification1); notifier.send(lotteryReport1); - verify(notificationTopicYrodiereMock).comment(" (updated 2017-11-06T06:00:00Z)", markdownNotification1); + verify(notificationTopicYrodiereMock) + .update(" (updated 2017-11-06T06:00:00Z)", markdownNotification1, true); verifyNoMoreInteractions(gitHubServiceMock, notificationRepoMock, messageFormatterMock); var lotteryReport2 = new LotteryReport(drawRef, "gsmet", Optional.empty(), @@ -142,7 +146,8 @@ void send() throws IOException { when(messageFormatterMock.formatNotificationBodyMarkdown(lotteryReport2, notificationRepoRef)) .thenReturn(markdownNotification2); notifier.send(lotteryReport2); - verify(notificationTopicGsmetMock).comment(" (updated 2017-11-06T06:00:00Z)", markdownNotification2); + verify(notificationTopicGsmetMock) + .update(" (updated 2017-11-06T06:00:00Z)", markdownNotification2, true); verifyNoMoreInteractions(gitHubServiceMock, notificationRepoMock, messageFormatterMock); var lotteryReport3 = new LotteryReport(drawRef, "geoand", Optional.empty(), @@ -151,7 +156,7 @@ void send() throws IOException { Optional.empty(), Optional.empty(), Optional.of(new LotteryReport.Bucket(stubIssueList(13, 14)))); - var markdownNotification3 = "Notif 2"; + var markdownNotification3 = "Notif 3"; when(messageFormatterMock.formatNotificationTopicText(drawRef, "geoand")) .thenReturn("geoand's report for quarkusio/quarkus"); when(messageFormatterMock.formatNotificationTopicSuffixText(lotteryReport3)) @@ -159,7 +164,26 @@ void send() throws IOException { when(messageFormatterMock.formatNotificationBodyMarkdown(lotteryReport3, notificationRepoRef)) .thenReturn(markdownNotification3); notifier.send(lotteryReport3); - verify(notificationTopicGeoandMock).comment(" (updated 2017-11-06T06:00:00Z)", markdownNotification3); + verify(notificationTopicGeoandMock) + .update(" (updated 2017-11-06T06:00:00Z)", markdownNotification3, true); + verifyNoMoreInteractions(gitHubServiceMock, notificationRepoMock, messageFormatterMock); + + var lotteryReport4 = new LotteryReport(drawRef, "jsmith", Optional.empty(), + Optional.of(new LotteryReport.Bucket(stubIssueList())), + Optional.empty(), + Optional.empty(), + Optional.empty(), + Optional.of(new LotteryReport.Bucket(stubIssueList()))); + var markdownNotification4 = "Notif 4"; + when(messageFormatterMock.formatNotificationTopicText(drawRef, "jsmith")) + .thenReturn("jsmith's report for quarkusio/quarkus"); + when(messageFormatterMock.formatNotificationTopicSuffixText(lotteryReport4)) + .thenReturn(" (updated 2017-11-06T06:00:00Z)"); + when(messageFormatterMock.formatNotificationBodyMarkdown(lotteryReport4, notificationRepoRef)) + .thenReturn(markdownNotification4); + notifier.send(lotteryReport4); + verify(notificationTopicJsmithMock) + .update(" (updated 2017-11-06T06:00:00Z)", markdownNotification4, false); verifyNoMoreInteractions(gitHubServiceMock, notificationRepoMock, messageFormatterMock); } diff --git a/src/test/java/io/quarkus/github/lottery/draw/LotteryReportTest.java b/src/test/java/io/quarkus/github/lottery/draw/LotteryReportTest.java new file mode 100644 index 0000000..845021e --- /dev/null +++ b/src/test/java/io/quarkus/github/lottery/draw/LotteryReportTest.java @@ -0,0 +1,55 @@ +package io.quarkus.github.lottery.draw; + +import static io.quarkus.github.lottery.util.MockHelper.stubIssueList; +import static org.assertj.core.api.Assertions.assertThat; + +import java.time.LocalDateTime; +import java.time.ZoneOffset; +import java.util.ArrayList; +import java.util.List; +import java.util.Optional; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import io.quarkus.github.lottery.github.GitHubInstallationRef; +import io.quarkus.github.lottery.github.GitHubRepositoryRef; + +class LotteryReportTest { + + GitHubInstallationRef installationRef; + GitHubRepositoryRef repoRef; + DrawRef drawRef; + + @BeforeEach + void setup() { + installationRef = new GitHubInstallationRef("quarkus-github-lottery", 1L); + repoRef = new GitHubRepositoryRef(installationRef, "quarkusio/quarkus"); + drawRef = new DrawRef(repoRef, LocalDateTime.of(2017, 11, 6, 8, 0).toInstant(ZoneOffset.UTC)); + } + + // This simply tests that all buckets are returned by buckets(), + // which should guarantee that hasContent() works correctly. + // This should start failing when we add a new bucket to LotteryReport but forget + // to reference it in buckets(). + @Test + void buckets() { + List buckets = new ArrayList<>(); + var report = new LotteryReport(drawRef, "geoand", Optional.empty(), + newBucket(buckets), + newBucket(buckets), + newBucket(buckets), + newBucket(buckets), + newBucket(buckets)); + assertThat(report.buckets()).containsExactlyInAnyOrderElementsOf(buckets); + } + + private Optional newBucket(List buckets) { + var bucket = new LotteryReport.Bucket(stubIssueList( + // Using a different issue number for each bucket so that it's different according to equals(). + buckets.size())); + buckets.add(bucket); + return Optional.of(bucket); + } + +} \ No newline at end of file