410: Update mailing list bridge subject decorations

Reviewed-by: ehelin

bots/mlbridge/src/main/java/org/openjdk/skara/bots/mlbridge/ArchiveItem.java

@@ -156,7 +156,7 @@ static ArchiveItem from(PullRequest pr, Repository localRepo, HostUserToEmailAut
                             ZonedDateTime created, ZonedDateTime updated, Hash lastBase, Hash lastHead, Hash base,
                             Hash head, int index, ArchiveItem parent, String subjectPrefix, String threadPrefix) {
         return new ArchiveItem(parent,"ha" + head.hex(), created, updated, pr.author(), Map.of("PR-Head-Hash", head.hex(), "PR-Base-Hash", base.hex()),
-                               () -> String.format("Re: %s[Rev %02d] %s%s", subjectPrefix, index, threadPrefix + (threadPrefix.isEmpty() ? "" : ": "), pr.title()),
+                               () -> String.format("Re: %s%s%s [v%d]", subjectPrefix, threadPrefix + (threadPrefix.isEmpty() ? "" : ": "), pr.title(), index + 1),
                                () -> "",
                                () -> {
                                    if (lastBase.equals(base)) {
@@ -214,17 +214,17 @@ static ArchiveItem from(PullRequest pr, ReviewComment reviewComment, HostUserToE
                                () -> ArchiveMessages.composeReplyFooter(pr));
     }
 
-    static ArchiveItem closedNotice(PullRequest pr, HostUserToEmailAuthor hostUserToEmailAuthor, ArchiveItem parent, String subjectPrefix, String threadPrefix) {
+    static ArchiveItem closedNotice(PullRequest pr, HostUserToEmailAuthor hostUserToEmailAuthor, ArchiveItem parent, String subjectPrefix) {
         return new ArchiveItem(parent, "cn", pr.updatedAt(), pr.updatedAt(), pr.author(), Map.of("PR-Closed-Notice", "0"),
-                               () -> String.format("Re: [Closed] %s%s%s", subjectPrefix, threadPrefix + (threadPrefix.isEmpty() ? "" : ": "), pr.title()),
+                               () -> String.format("%sWithdrawn: %s", subjectPrefix, pr.title()),
                                () -> ArchiveMessages.composeReplyHeader(parent.createdAt(), hostUserToEmailAuthor.author(parent.author())),
                                () -> ArchiveMessages.composeClosedNotice(pr),
                                () -> ArchiveMessages.composeReplyFooter(pr));
     }
 
-    static ArchiveItem integratedNotice(PullRequest pr, Repository localRepo, Commit commit, HostUserToEmailAuthor hostUserToEmailAuthor, ArchiveItem parent, String subjectPrefix, String threadPrefix) {
+    static ArchiveItem integratedNotice(PullRequest pr, Repository localRepo, Commit commit, HostUserToEmailAuthor hostUserToEmailAuthor, ArchiveItem parent, String subjectPrefix) {
         return new ArchiveItem(parent, "in", pr.updatedAt(), pr.updatedAt(), pr.author(), Map.of("PR-Integrated-Notice", "0"),
-                               () -> String.format("Re: [Integrated] %s%s%s", subjectPrefix, threadPrefix + (threadPrefix.isEmpty() ? "" : ": "), pr.title()),
+                               () -> String.format("%sIntegrated: %s", subjectPrefix, pr.title()),
                                () -> ArchiveMessages.composeReplyHeader(parent.createdAt(), hostUserToEmailAuthor.author(parent.author())),
                                () -> ArchiveMessages.composeIntegratedNotice(pr, localRepo, commit),
                                () -> ArchiveMessages.composeReplyFooter(pr));

bots/mlbridge/src/main/java/org/openjdk/skara/bots/mlbridge/ReviewArchive.java

@@ -89,7 +89,7 @@ private List<ArchiveItem> generateArchiveItems(List<Email> sentEmails, Repositor
             }
         } else {
             if (pr.state() != Issue.State.OPEN) {
-                threadPrefix = "FYI";
+                threadPrefix = "Integrated";
             }
         }
 
@@ -152,18 +152,18 @@ private List<ArchiveItem> generateArchiveItems(List<Email> sentEmails, Repositor
                 if (hash.isPresent()) {
                     var commit = localRepo.lookup(hash.get());
                     if (!hasLegacyIntegrationNotice(localRepo, commit.orElseThrow())) {
-                        var reply = ArchiveItem.integratedNotice(pr, localRepo, commit.orElseThrow(), hostUserToEmailAuthor, parent, subjectPrefix, threadPrefix);
+                        var reply = ArchiveItem.integratedNotice(pr, localRepo, commit.orElseThrow(), hostUserToEmailAuthor, parent, subjectPrefix);
                         generated.add(reply);
                     }
                 } else {
                     throw new RuntimeException("PR " + pr.webUrl() + " has integrated label but no integration comment");
                 }
             } else if (localRepo.isAncestor(pr.headHash(), pr.targetHash())) {
                 var commit = localRepo.lookup(pr.headHash());
-                var reply = ArchiveItem.integratedNotice(pr, localRepo, commit.orElseThrow(), hostUserToEmailAuthor, parent, subjectPrefix, threadPrefix);
+                var reply = ArchiveItem.integratedNotice(pr, localRepo, commit.orElseThrow(), hostUserToEmailAuthor, parent, subjectPrefix);
                 generated.add(reply);
             } else if (threadPrefix.equals("RFR")) {
-                var reply = ArchiveItem.closedNotice(pr, hostUserToEmailAuthor, parent, subjectPrefix, threadPrefix);
+                var reply = ArchiveItem.closedNotice(pr, hostUserToEmailAuthor, parent, subjectPrefix);
                 generated.add(reply);
             }
         }

bots/mlbridge/src/test/java/org/openjdk/skara/bots/mlbridge/MailingListBridgeBotTests.java

@@ -335,7 +335,7 @@ void archiveIntegrated(TestInfo testInfo) throws IOException {
 
             // The archive should now contain another entry
             Repository.materialize(archiveFolder.path(), archive.url(), "master");
-            assertTrue(archiveContains(archiveFolder.path(), "Subject: Re: \\[Integrated\\] RFR: 1234: This is a pull request"));
+            assertTrue(archiveContains(archiveFolder.path(), "Subject: Integrated: 1234: This is a pull request"));
             assertFalse(archiveContains(archiveFolder.path(), "\\[Closed\\]"));
         }
     }
@@ -478,7 +478,7 @@ void archiveDirectToIntegrated(TestInfo testInfo) throws IOException {
 
             // The archive should now contain an entry
             Repository.materialize(archiveFolder.path(), archive.url(), "master");
-            assertTrue(archiveContains(archiveFolder.path(), "Subject: FYI: 1234: This is a pull request"));
+            assertTrue(archiveContains(archiveFolder.path(), "Subject: Integrated: 1234: This is a pull request"));
 
             var updatedHash = CheckableRepository.appendAndCommit(localRepo, "Another change");
             localRepo.push(updatedHash, author.url(), "edit");
@@ -488,8 +488,8 @@ void archiveDirectToIntegrated(TestInfo testInfo) throws IOException {
 
             // The archive should now contain another entry
             Repository.materialize(archiveFolder.path(), archive.url(), "master");
-            assertTrue(archiveContains(archiveFolder.path(), "Subject: Re: \\[Rev 01\\] FYI: 1234: This is a pull request"));
-            assertFalse(archiveContains(archiveFolder.path(), "\\[Closed\\]"));
+            assertTrue(archiveContains(archiveFolder.path(), "Subject: Re: Integrated: 1234: This is a pull request \\[v2\\]"));
+            assertFalse(archiveContains(archiveFolder.path(), "Withdrawn"));
         }
     }
 
@@ -566,7 +566,7 @@ void archiveIntegratedRetainPrefix(TestInfo testInfo) throws IOException {
 
             // The archive should now contain another entry - should retain the RFR thread prefix
             Repository.materialize(archiveFolder.path(), archive.url(), "master");
-            assertTrue(archiveContains(archiveFolder.path(), "Subject: Re: \\[Rev 01\\] RFR: 1234: This is a pull request"));
+            assertTrue(archiveContains(archiveFolder.path(), "Subject: Re: RFR: 1234: This is a pull request \\[v2\\]"));
         }
     }
 
@@ -641,14 +641,15 @@ void archiveClosed(TestInfo testInfo) throws IOException {
 
             // The archive should now contain another entry - should say that it is closed
             Repository.materialize(archiveFolder.path(), archive.url(), "master");
-            assertEquals(1, archiveContainsCount(archiveFolder.path(), "Subject: Re: \\[Closed\\] RFR: 1234: This is a pull request"));
+            assertEquals(1, archiveContainsCount(archiveFolder.path(), "Subject: Withdrawn: 1234: This is a pull request"));
 
             pr.addComment("Fair enough");
 
             // Run another archive pass - only a single close notice should have been posted
             TestBotRunner.runPeriodicItems(mlBot);
             Repository.materialize(archiveFolder.path(), archive.url(), "master");
-            assertEquals(1, archiveContainsCount(archiveFolder.path(), "Subject: Re: \\[Closed\\] RFR: 1234: This is a pull request"));
+            assertEquals(1, archiveContainsCount(archiveFolder.path(), "Subject: Withdrawn: 1234: This is a pull request"));
+            assertEquals(1, archiveContainsCount(archiveFolder.path(), "Subject: Re: RFR: 1234: This is a pull request"));
         }
     }
 
@@ -1605,9 +1606,9 @@ void incrementalChanges(TestInfo testInfo) throws IOException {
             assertEquals(1, updatedConversations.size());
             var conversation = updatedConversations.get(0);
             assertEquals(6, conversation.allMessages().size());
-            assertEquals("[Rev 01] RFR: This is a pull request", conversation.allMessages().get(1).subject());
-            assertEquals("[Rev 01] RFR: This is a pull request", conversation.allMessages().get(2).subject(), conversation.allMessages().get(2).toString());
-            assertEquals("[Rev 04] RFR: This is a pull request", conversation.allMessages().get(5).subject());
+            assertEquals("RFR: This is a pull request [v2]", conversation.allMessages().get(1).subject());
+            assertEquals("RFR: This is a pull request [v2]", conversation.allMessages().get(2).subject(), conversation.allMessages().get(2).toString());
+            assertEquals("RFR: This is a pull request [v5]", conversation.allMessages().get(5).subject());
         }
     }
 
@@ -1703,7 +1704,7 @@ void rebased(TestInfo testInfo) throws IOException {
                 assertEquals(listAddress, newMail.sender());
                 assertFalse(newMail.hasHeader("PR-Head-Hash"));
             }
-            assertEquals("[Rev 01] RFR: This is a pull request", conversations.get(0).allMessages().get(1).subject());
+            assertEquals("RFR: This is a pull request [v2]", conversations.get(0).allMessages().get(1).subject());
         }
     }