[IOSP-590] Restore Queue order on bot reboot (#57)

* Order PRs on boot based on previous bot comments

* Unit Test

* Add info in bot comment when booting

* Updated code + test about order by comment date

Thanks @NSMyself

* Simplify decoding of IssueComment + add test for it

thx @ilyapuchka for pointing this out

* Naming

* Add trap comment to Unit Test

.gitignore

@@ -6,3 +6,6 @@ xcuserdata
 !WallEView.xcodeproj
 Config/secrets
 .DS_Store
+
+# Local files we don't want to commit, like scripts containing secrets
+_local

Sources/Bot/API/GitHubAPIProtocol.swift

@@ -29,5 +29,10 @@ public protocol GitHubAPIProtocol {
 
     func postComment(_ comment: String, in pullRequest: PullRequest) -> SignalProducer<Void, GitHubClient.Error>
 
+    /// Note: only fetches issue comments, not Pull Request review comments
+    func fetchIssueComments(in pullRequest: PullRequest) -> SignalProducer<[IssueComment], GitHubClient.Error>
+
     func removeLabel(_ label: PullRequest.Label, from pullRequest: PullRequest) -> SignalProducer<Void, GitHubClient.Error>
+
+    func fetchCurrentUser() -> SignalProducer<GitHubUser, GitHubClient.Error>
 }

Sources/Bot/API/Repository.swift

@@ -92,6 +92,14 @@ extension Repository {
         )
     }
 
+    func issueComments(in pullRequest: PullRequest) -> Resource<[IssueComment]> {
+        return Resource(
+            method: .GET,
+            path: path(for: "issues/\(pullRequest.number)/comments"),
+            decoder: decode
+        )
+    }
+
     func merge(head: PullRequest.Branch, into base: PullRequest.Branch) -> Resource<MergeResult> {
         return Resource(
             method: .POST,
@@ -105,7 +113,7 @@ extension Repository {
                 default:
                     return .failure(.api(response))
                 }
-        }
+            }
         )
     }
 
@@ -118,6 +126,14 @@ extension Repository {
         )
     }
 
+    var currentUser: Resource<GitHubUser> {
+        Resource(
+            method: .GET,
+            path: "user",
+            decoder: decode
+        )
+    }
+
     // TODO: This should be moved to be done as part of the handling of resources in `GitHubClient`
     private func encode<T: Encodable>(_ t: T) -> Data {
         guard let data = try? JSONEncoder().encode(t)

Sources/Bot/API/RepositoryAPI.swift

@@ -49,9 +49,17 @@ public struct RepositoryAPI: GitHubAPIProtocol {
         return client.request(repository.publish(comment: comment, in: pullRequest))
     }
 
+    public func fetchIssueComments(in pullRequest: PullRequest) -> SignalProducer<[IssueComment], GitHubClient.Error> {
+        return client.request(repository.issueComments(in: pullRequest))
+    }
+
     public func removeLabel(_ label: PullRequest.Label, from pullRequest: PullRequest) -> SignalProducer<Void, GitHubClient.Error> {
         return client.request(repository.removeLabel(label: label, from: pullRequest))
     }
+
+    public func fetchCurrentUser() -> SignalProducer<GitHubUser, GitHubClient.Error> {
+        return client.request(repository.currentUser)
+    }
 }
 
 extension GitHubClient {

Sources/Bot/Models/GitHubUser.swift

@@ -0,0 +1,7 @@
+import Foundation
+
+public struct GitHubUser: Identifiable, Equatable, Decodable {
+    public let id: Int
+    let login: String
+    let name: String?
+}

Sources/Bot/Models/IssueComment.swift

@@ -0,0 +1,15 @@
+import Foundation
+
+public struct IssueComment: Equatable {
+    let user: GitHubUser
+    let body: String
+    let creationDate: Date
+}
+
+extension IssueComment: Decodable {
+    private enum CodingKeys: String, CodingKey {
+        case user
+        case body
+        case creationDate = "created_at"
+    }
+}

Sources/Bot/Services/DispatchService.swift

@@ -13,6 +13,7 @@ public final class DispatchService {
     private let logger: LoggerProtocol
     private let gitHubAPI: GitHubAPIProtocol
     private let scheduler: DateScheduler
+    private var botUser: GitHubUser? = nil
 
     /// Merge services per target branch
     private var mergeServices: Atomic<[String: MergeService]>
@@ -43,10 +44,18 @@ public final class DispatchService {
         self.mergeServices = Atomic([:])
         (mergeServiceLifecycle, mergeServiceLifecycleObserver) = Signal<DispatchService.MergeServiceLifecycleEvent, Never>.pipe()
 
-        gitHubAPI.fetchPullRequests()
-            .flatMapError { _ in .value([]) }
+        gitHubAPI
+            .fetchCurrentUser()
+            .map(Optional<GitHubUser>.some)
+            .flatMapError { _ in .value(nil) }
+            .flatMap(.latest) { botUser in
+                gitHubAPI.fetchPullRequests()
+                    .flatMapError { _ in .value([]) }
+                    .map { (botUser, $0) }
+            }
             .observe(on: scheduler)
-            .startWithValues { pullRequests in
+            .startWithValues { (botUser, pullRequests) in
+                self.botUser = botUser
                 self.dispatchInitial(pullRequests: pullRequests)
             }
 
@@ -117,6 +126,7 @@ public final class DispatchService {
             requiresAllStatusChecks: requiresAllStatusChecks,
             statusChecksTimeout: statusChecksTimeout,
             initialPullRequests: initialPullRequests,
+            botUser: botUser,
             logger: logger,
             gitHubAPI: gitHubAPI,
             scheduler: scheduler

Sources/Bot/Services/MergeService.swift

@@ -44,6 +44,7 @@ public final class MergeService {
         requiresAllStatusChecks: Bool,
         statusChecksTimeout: TimeInterval,
         initialPullRequests: [PullRequest] = [],
+        botUser: GitHubUser?,
         logger: LoggerProtocol,
         gitHubAPI: GitHubAPIProtocol,
         scheduler: DateScheduler = QueueScheduler()
@@ -71,7 +72,7 @@ public final class MergeService {
             scheduler: scheduler,
             reduce: MergeService.reduce,
             feedbacks: [
-                Feedbacks.whenStarting(initialPullRequests: pullRequestsReadyToInclude, scheduler: scheduler),
+                Feedbacks.whenStarting(initialPullRequests: pullRequestsReadyToInclude, gitHubAPI: gitHubAPI, botUser: botUser, scheduler: scheduler),
                 Feedbacks.whenReady(github: self.gitHubAPI, scheduler: scheduler),
                 Feedbacks.whenIntegrating(github: self.gitHubAPI, requiresAllStatusChecks: requiresAllStatusChecks, pullRequestChanges: pullRequestChanges, scheduler: scheduler),
                 Feedbacks.whenRunningStatusChecks(github: self.gitHubAPI, logger: logger, requiresAllStatusChecks: requiresAllStatusChecks, statusChecksCompletion: statusChecksCompletion, scheduler: scheduler),
@@ -316,9 +317,23 @@ extension MergeService {
 
 // MARK: - Feedbacks
 
+fileprivate let acceptedCommentTextIntro = "Your pull request was accepted"
+
 extension MergeService {
     fileprivate typealias Feedbacks = MergeService
 
+    static func acceptedCommentText(index: Int?, queue: String, isBooting: Bool = false) -> String {
+        let rebootInfo = isBooting ? "WallE just started after a reboot.\n" : ""
+        let prefix = "\(rebootInfo)\(acceptedCommentTextIntro)"
+        if let index = index {
+            // use Zero-width-space character so that GitHub doesn't transform `#N` into a link to Pull Request N
+            let zws = "\u{200B}"
+            return "\(prefix) and it's currently #\(zws)\(index + 1) in the `\(queue)` queue, hold tight ⏳"
+        } else {
+            return "\(prefix) and is going to be handled right away 🏎 (nothing in queue for `\(queue)`!)"
+        }
+    }
+
     fileprivate static func whenAddingPullRequests(
         github: GitHubAPIProtocol,
         scheduler: Scheduler
@@ -332,19 +347,20 @@ extension MergeService {
                     .enumerated()
                     .map { index, pullRequest -> SignalProducer<(), Never> in
 
-                        guard previous.pullRequests.firstIndex(of: pullRequest) == nil
+                        guard previous.pullRequests.contains(pullRequest) == false
                             else { return .empty }
 
+                        let isBooting = previous.status == .starting
+
                         if index == 0 && current.isIntegrationOngoing == false {
                             return github.postComment(
-                                "Your pull request was accepted and is going to be handled right away 🏎",
+                                acceptedCommentText(index: nil, queue: current.targetBranch, isBooting: isBooting),
                                 in: pullRequest
                             )
                             .flatMapError { _ in .empty }
                         } else {
-                            let zws = "\u{200B}" // Zero-width space character. Used so that GitHub doesn't transform `#n` into a link to Pull Request n
                             return github.postComment(
-                                "Your pull request was accepted and it's currently #\(zws)\(index + 1) in the `\(current.targetBranch)` queue, hold tight ⏳",
+                                acceptedCommentText(index: index, queue: current.targetBranch, isBooting: isBooting),
                                 in: pullRequest
                             )
                             .flatMapError { _ in .empty }
@@ -356,11 +372,43 @@ extension MergeService {
         })
     }
 
-    fileprivate static func whenStarting(initialPullRequests: [PullRequest], scheduler: DateScheduler) -> Feedback<State, Event> {
+    fileprivate static func whenStarting(
+        initialPullRequests: [PullRequest],
+        gitHubAPI: GitHubAPIProtocol,
+        botUser: GitHubUser?,
+        scheduler: DateScheduler
+    ) -> Feedback<State, Event> {
+        typealias DatedPullRequest = (pr: PullRequest, date: Date)
+
+        func isBotMergeComment(_ comment: IssueComment) -> Bool {
+            return (botUser.map({ comment.user.id == $0.id }) ?? true) // If botUser nil, default to true
+                && comment.body.contains(acceptedCommentTextIntro)
+        }
+
+        func datedPullRequest(for pullRequest: PullRequest) -> SignalProducer<DatedPullRequest, Never> {
+            return gitHubAPI.fetchIssueComments(in: pullRequest)
+                .flatMapError { _ in .value([]) }
+                .map { comments in
+                    let lastBotCommentDate = comments
+                        .filter(isBotMergeComment)
+                        .map { $0.creationDate }
+                        .max()
+                    return (pullRequest, lastBotCommentDate ?? .distantFuture)
+                }
+        }
+
+        func orderByBotCommentDate(_ datedPRs: [DatedPullRequest]) -> [PullRequest] {
+            return datedPRs.sorted(by: { $0.date < $1.date }).map { $0.pr }
+        }
+
         return Feedback(predicate: { $0.status == .starting }) { state -> SignalProducer<Event, Never> in
-            return SignalProducer
-                .value(Event.pullRequestsLoaded(initialPullRequests))
-                .observe(on: scheduler)
+            return SignalProducer.merge(
+                initialPullRequests.map(datedPullRequest(for:))
+            )
+            .collect()
+            .map(orderByBotCommentDate)
+            .map(Event.pullRequestsLoaded)
+            .observe(on: scheduler)
         }
     }
 

Tests/BotTests/Canned Data/GitHubIssueComment.swift

@@ -0,0 +1,52 @@
+let GitHubIssueComment = #"""
+{
+  "url": "https://api.github.com/repos/babylonhealth/Wall-E/pulls/comments/400979441",
+  "pull_request_review_id": 384816651,
+  "id": 400979441,
+  "node_id": "MDI0OlB1bGxSZXF1ZXN0UmV2aWV3Q29tbWVudDQwMDk3OTQ0MQ==",
+  "diff_hunk": "@@ -0,0 +1,34 @@\n+import Foundation\n+\n+public struct IssueComment: Equatable {\n+    let user: GitHubUser\n+    let body: String\n+    let creationDate: Date\n+}\n+\n+extension IssueComment: Decodable {\n+    private static let dateFormatter: DateFormatter = {\n+        // 2020-03-30T11:26:18\n+        let df = DateFormatter()\n+        df.locale = Locale(identifier: \"en_US_POSIX\")\n+        df.dateFormat = \"yyyy-MM-dd'T'HH:mm:ssZ\"\n+        return df\n+    }()\n+\n+    private enum CodingKeys: String, CodingKey {\n+        case user\n+        case body\n+        case created_at\n+    }\n+\n+    public init(from decoder: Decoder) throws {\n+        let container = try decoder.container(keyedBy: CodingKeys.self)\n+        self.user = try container.decode(GitHubUser.self, forKey: .user)\n+        self.body = try container.decode(String.self, forKey: .body)\n+        let dateString = try container.decode(String.self, forKey: .created_at)\n+        guard let date = Self.dateFormatter.date(from: dateString) else {",
+  "path": "Sources/Bot/Models/IssueComment.swift",
+  "position": 29,
+  "original_position": 29,
+  "commit_id": "8b6a72d20ab8f5cb6cd6d6bf9300bb14fdb9e681",
+  "original_commit_id": "8b6a72d20ab8f5cb6cd6d6bf9300bb14fdb9e681",
+  "user": {
+    "login": "AliSoftware",
+    "id": 216089,
+    "node_id": "MDQ6VXNlcjIxNjA4OQ==",
+    "avatar_url": "https://avatars2.githubusercontent.com/u/216089?v=4",
+    "gravatar_id": "",
+    "url": "https://api.github.com/users/AliSoftware",
+    "html_url": "https://github.com/AliSoftware",
+    "followers_url": "https://api.github.com/users/AliSoftware/followers",
+    "following_url": "https://api.github.com/users/AliSoftware/following{/other_user}",
+    "gists_url": "https://api.github.com/users/AliSoftware/gists{/gist_id}",
+    "starred_url": "https://api.github.com/users/AliSoftware/starred{/owner}{/repo}",
+    "subscriptions_url": "https://api.github.com/users/AliSoftware/subscriptions",
+    "organizations_url": "https://api.github.com/users/AliSoftware/orgs",
+    "repos_url": "https://api.github.com/users/AliSoftware/repos",
+    "events_url": "https://api.github.com/users/AliSoftware/events{/privacy}",
+    "received_events_url": "https://api.github.com/users/AliSoftware/received_events",
+    "type": "User",
+    "site_admin": false
+  },
+  "body": "Oh forgot about that one, good point.\r\nAnd just saw that there's actually already code for that in `Decoding.swift` so all that shouldn't even be needed at all indeed. ",
+  "created_at": "2020-03-31T14:54:28Z",
+  "updated_at": "2020-03-31T14:54:28Z",
+  "html_url": "https://github.com/babylonhealth/Wall-E/pull/57#discussion_r400979441",
+  "pull_request_url": "https://api.github.com/repos/babylonhealth/Wall-E/pulls/57",
+  "author_association": "CONTRIBUTOR",
+  "_links": {
+    "self": {
+      "href": "https://api.github.com/repos/babylonhealth/Wall-E/pulls/comments/400979441"
+    },
+    "html": {
+      "href": "https://github.com/babylonhealth/Wall-E/pull/57#discussion_r400979441"
+    },
+    "pull_request": {
+      "href": "https://api.github.com/repos/babylonhealth/Wall-E/pulls/57"
+    }
+  },
+  "in_reply_to_id": 400977181
+}
+"""#

Tests/BotTests/GitHub/GitHubDecodingTests.swift

@@ -60,5 +60,15 @@ class GitHubDecodingTests: XCTestCase {
             fail("Could not parse a pull request with error: \(error)")
         }
     }
+
+    func test_parsing_issue_comment() throws {
+        let response = Bot.Response(statusCode: 200, headers: [:], body: GitHubIssueComment.data(using: .utf8)!)
+        let commentResult: Result<IssueComment, GitHubClient.Error> = Bot.decode(response)
+        let comment = try commentResult.get()
+        expect(comment.user.id) == 216089
+        expect(comment.user.login) == "AliSoftware"
+        expect(comment.body) == "Oh forgot about that one, good point.\r\nAnd just saw that there's actually already code for that in `Decoding.swift` so all that shouldn't even be needed at all indeed. "
+        expect(comment.creationDate.timeIntervalSince1970) == 1585666468
+    }
 }