Merge pull request #195 from yrodiere/no-notification-if-empty

Do not send GitHub notifications on empty draws

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<Bucket> stale,
         Optional<Bucket> stewardship) {
 
+    Stream<Bucket> 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<Issue> issues) {
 
+        public boolean hasContent() {
+            return !issues().isEmpty();
+        }
+
         public Serialized serialized() {
             return new Serialized(issues.stream().map(Issue::number).collect(Collectors.toList()));
         }

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,15 +233,16 @@ 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();
             return existingIssue.isPresent() && GHIssueState.CLOSED.equals(existingIssue.get().getState());
         }
 
         /**
-         * 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<GHIssue> getDedicatedIssues() throws IOException {

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<LotteryReport.Serialized> 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) {

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) {

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());

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,15 +156,34 @@ 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))
                 .thenReturn(" (updated 2017-11-06T06:00:00Z)");
         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);
     }