Ignore reporting "Do Not Merge" PRs in Discord (#293)

Author: MahdiBM

.sourcekit-lsp/config.json

@@ -1,6 +1,6 @@
 {
   "backgroundIndexing": true,
-  "backgroundPreparationMode": "build",
+  "backgroundPreparationMode": "enabled",
   "maxCoresPercentageToUseForBackgroundIndexing": 0.7,
   "experimentalFeatures": ["on-type-formatting"]
 }

Lambdas/GHHooks/Constants.swift

@@ -5,6 +5,13 @@ enum Constants {
     static let guildID: GuildSnowflake = "431917998102675485"
     static let botDevUserID: UserSnowflake = "290483761559240704"
 
+    static let trustedGitHubUserIds: Set<Int64> = [
+        69_189_821,  // Paul
+        9_938_337,  // Tim
+        1_130_717,  // Gwynne
+        54_685_446,  // Mahdi
+    ]
+
     enum GitHub {
         /// The user id of Penny.
         static let userID = 139_480_971

Lambdas/GHHooks/EventHandler/Handlers/PRCoinGiver.swift

@@ -30,8 +30,11 @@ struct PRCoinGiver {
             repoFullName: repo.fullName,
             branch: branch
         )
-        for pr in try await getPRsRelatedToCommit() {
-            guard pr.mergedAt != nil else { continue }
+        for pr in prs {
+            if pr.mergedAt == nil {
+                logger.debug("PR is not merged yet", metadata: ["pr": "\(pr)"])
+                continue
+            }
             let user = try pr.user.requireValue()
             var usersToReceiveCoins = codeOwners.contains(user: user) ? [] : [user.id]
 
@@ -88,6 +91,8 @@ struct PRCoinGiver {
     }
 
     func giveCoin(userId: Int64, pr: SimplePullRequest) async throws {
+        logger.trace("Giving a coin", metadata: ["userId": .stringConvertible(userId)])
+
         guard
             let member = try await context.requester.getDiscordMember(
                 githubID: "\(userId)"

Lambdas/GHHooks/EventHandler/Handlers/PRHandler.swift

@@ -1,10 +1,6 @@
-import AsyncHTTPClient
 import DiscordBM
 import GitHubAPI
-import Markdown
-import NIOCore
-import NIOFoundationCompat
-import SwiftSemver
+import Logging
 
 #if canImport(FoundationEssentials)
 import FoundationEssentials
@@ -19,6 +15,9 @@ struct PRHandler {
     var event: GHEvent {
         self.context.event
     }
+    var logger: Logger {
+        context.logger
+    }
 
     var repo: Repository {
         get throws {
@@ -49,17 +48,21 @@ struct PRHandler {
     }
 
     func onEdited() async throws {
-        try await self.editPRReport()
+        if self.isIgnorableDoNotMergePR() { return }
+        try await self.makeReporter().reportEdition(
+            requiresPreexistingReport: self.action == .labeled
+        )
     }
 
     func onOpened() async throws {
+        if self.isIgnorableDoNotMergePR() { return }
         try await self.makeReporter().reportCreation()
     }
 
     func onClosed() async throws {
         try await withThrowingAccumulatingVoidTaskGroup(tasks: [
             { try await self.makeRelease() },
-            { try await self.editPRReport() },
+            { try await self.onEdited() },
         ])
     }
 
@@ -71,12 +74,6 @@ struct PRHandler {
         ).handle()
     }
 
-    func editPRReport() async throws {
-        try await self.makeReporter().reportEdition(
-            requiresPreexistingReport: self.action == .labeled
-        )
-    }
-
     func makeReporter() async throws -> TicketReporter {
         try TicketReporter(
             context: self.context,
@@ -125,6 +122,19 @@ struct PRHandler {
 
         return embed
     }
+
+    func isIgnorableDoNotMergePR() -> Bool {
+        let isIgnorable = self.pr.isIgnorableDoNotMergePR
+        if isIgnorable {
+            logger.info(
+                "PR is ignorable",
+                metadata: [
+                    "pr": "\(self.pr)"
+                ]
+            )
+        }
+        return isIgnorable
+    }
 }
 
 private enum Status: String {

Lambdas/GHHooks/EventHandler/Handlers/ReleaseMaker.swift

@@ -290,7 +290,9 @@ struct ReleaseMaker {
     }
 
     func isNewContributor(codeOwners: CodeOwners, existingContributors: Set<Int64>) -> Bool {
-        pr.authorAssociation != .owner && !pr.user.isBot && !codeOwners.contains(user: pr.user)
+        pr.authorAssociation != .owner
+            && !pr.user.isBot
+            && !codeOwners.contains(user: pr.user)
             && !existingContributors.contains(pr.user.id)
     }
 
@@ -305,19 +307,18 @@ struct ReleaseMaker {
             )
         )
 
-        guard case let .ok(ok) = response,
-            case let .json(json) = ok.body
-        else {
+        do {
+            return try response.ok.body.json
+        } catch {
             logger.warning(
                 "Could not find reviews",
                 metadata: [
-                    "response": "\(response)"
+                    "response": "\(response)",
+                    "error": "\(String(reflecting: error))",
                 ]
             )
             return []
         }
-
-        return json
     }
 
     func getExistingContributorIDs() async throws -> Set<Int64> {
@@ -357,9 +358,9 @@ struct ReleaseMaker {
             )
         )
 
-        if case let .ok(ok) = response,
-            case let .json(json) = ok.body
-        {
+        do {
+            let ok = try response.ok
+            let json = try ok.body.json
             /// Example of a `link` header: `<https://api.github.com/repositories/49910095/contributors?page=6>; rel="prev", <https://api.github.com/repositories/49910095/contributors?page=8>; rel="next", <https://api.github.com/repositories/49910095/contributors?page=8>; rel="last", <https://api.github.com/repositories/49910095/contributors?page=1>; rel="first"`
             /// If the header contains `rel="next"` then we'll have a next page to fetch.
             let hasNext =
@@ -378,12 +379,13 @@ struct ReleaseMaker {
                 ]
             )
             return (ids, hasNext)
-        } else {
+        } catch {
             logger.error(
                 "Error when fetching contributors but will continue",
                 metadata: [
                     "page": .stringConvertible(page),
                     "response": "\(response)",
+                    "error": "\(String(reflecting: error))",
                 ]
             )
             return ([], false)

Lambdas/GHHooks/EventHandler/Handlers/TicketReporter.swift

@@ -128,7 +128,8 @@ struct TicketReporter {
             )
         case let .some(error):
             throw error
-        default: break
+        case .none:
+            break
         }
     }
 }

Lambdas/GHHooks/Extensions/+PR.Label.swift renamed to Lambdas/GHHooks/Extensions/+PR.swift

@@ -1,5 +1,11 @@
 import GitHubAPI
 
+#if canImport(FoundationEssentials)
+import FoundationEssentials
+#else
+import Foundation
+#endif
+
 extension PullRequest {
 
     enum KnownLabel: String {
@@ -28,6 +34,14 @@ extension PullRequest {
             KnownLabel(rawValue: $0.name)
         }
     }
+
+    var isIgnorableDoNotMergePR: Bool {
+        let isTrustedUser = Constants.trustedGitHubUserIds.contains(self.user.id)
+        guard isTrustedUser || self.authorAssociation.isContributorOrHigher else {
+            return false
+        }
+        return self.title.hasDoNotMergePrefix
+    }
 }
 
 extension SimplePullRequest {
@@ -37,3 +51,22 @@ extension SimplePullRequest {
         }
     }
 }
+
+extension AuthorAssociation {
+    fileprivate var isContributorOrHigher: Bool {
+        switch self {
+        case .collaborator, .contributor, .owner: return true
+        case .member, .firstTimer, .firstTimeContributor, .mannequin, .none: return false
+        }
+    }
+}
+
+extension String {
+    fileprivate var hasDoNotMergePrefix: Bool {
+        let folded = self.lowercased()
+            .filter { !$0.isPunctuation }
+            .folding(options: .caseInsensitive, locale: nil)
+            .folding(options: .diacriticInsensitive, locale: nil)
+        return folded.hasPrefix("dnm") || folded.hasPrefix("do not merge")
+    }
+}

Lambdas/GitHubAPI/Aliases.swift

@@ -2,6 +2,7 @@ package typealias Repository = Components.Schemas.Repository
 package typealias User = Components.Schemas.SimpleUser
 package typealias PullRequest = Components.Schemas.PullRequest
 package typealias SimplePullRequest = Components.Schemas.PullRequestSimple
+package typealias AuthorAssociation = Components.Schemas.AuthorAssociation
 package typealias Issue = Components.Schemas.Issue
 package typealias Label = Components.Schemas.Label
 package typealias Release = Components.Schemas.Release

Tests/PennyTests/Tests/GHHooksTests.swift

@@ -864,6 +864,15 @@ actor GHHooksTests {
         )
     }
 
+    @Test("PR with [DNM] prefix where author_association is CONTRIBUTOR should not be posted to Discord")
+    func handlePREvent14() async throws {
+        try await handleEvent(
+            key: "pr14",
+            eventName: .pull_request,
+            expect: .noResponse
+        )
+    }
+
     @Test
     func handlePushEvent1() async throws {
         try await handleEvent(