Allow changelog to be split into separate sections (#40)

* Allow changelog to be split into separate sections * Refine wording in comments in IssuesFetcher * Update Sources/GitBuddyCore/Models/Changelog.swift Co-Authored-By: Antoine van der Lee <[email protected]> * Fix linter issues * Fix remaining tests, update comments and README.md * Update README.md and tests * Add doc comment * Update IssuesJSON.swift * Update Package.swift * Update Sources/GitBuddyCore/Models/Changelog.swift * Update Package.swift Co-authored-by: Antoine van der Lee <[email protected]>
WeTransfer · Mar 30, 2020 · 1cae532 · 1cae532
1 parent f73fc1d
commit 1cae532
Show file tree

Hide file tree

Showing 12 changed files with 4,565 additions and 25 deletions.
diff --git a/README.md b/README.md
@@ -21,6 +21,7 @@ OVERVIEW: Create a changelog for GitHub repositories
 OPTIONS:
   --base-branch, -b   The base branch to compare with. Defaults to master.
   --help              Display available options
+  --sections          Whether the changelog should be split into sections. Defaults to false. 
   --since-tag, -s     The tag to use as a base. Defaults to the latest tag.
   --verbose           Show extra logging for debugging purposes
 ```
@@ -39,10 +40,25 @@ This is an example taken from [Mocker](https://github.com/WeTransfer/Mocker/rele
 
 ----
 
+If you'd like a changelog to link to issues closed before a release was tagged, pass the `--sections` argument, then it's going to look like this:
+
+----
+
+**Closed issues:**
+
+- Can SPM support be merged branch add-spm-support be merged to master? ([#33](https://github.com/WeTransfer/Mocker/pull/33))
+- migrate 2.0.0 changes to spm compatible branch `feature/add-spm-support`? ([#32](https://github.com/WeTransfer/Mocker/pull/32))
+
+**Merged pull requests:**
+
+- Switch over to Danger-Swift & Bitrise ([#34](https://github.com/WeTransfer/Mocker/pull/34)) via @AvdLee
+- Fix important mismatch for getting the right mock ([#31](https://github.com/WeTransfer/Mocker/pull/31)) via @AvdLee
+
+----
+
 ### Generating a release
 ```
 $ gitbuddy release --help
-[112/112] Linking GitBuddy
 OVERVIEW: Create a new release including a changelog and publish comments on related issues
 
 OPTIONS:
@@ -51,6 +67,7 @@ OPTIONS:
   --help                   Display available options
   --last-release-tag, -l   The last release tag to use as a base for the changelog creation. Default: previous tag
   --release-title, -r      The title of the release. Default: uses the tag name.
+  --sections               Whether the changelog should be split into sections. Defaults to false.
   --skip-comments, -s      Disable commenting on issues and PRs about the new release
   --tag-name, -n           The name of the tag. Default: takes the last created tag to publish as a GitHub release.
   --target-commitish, -t   Specifies the commitish value that determines where the Git tag is created from. Can be any branch or commit SHA. Unused if the Git tag already exists. Default: the repository's default branch (usually master).

diff --git a/Sources/GitBuddyCore/Changelog/ChangelogProducer.swift b/Sources/GitBuddyCore/Changelog/ChangelogProducer.swift
@@ -16,17 +16,19 @@ struct ChangelogCommand: Command {
     static let description = "Create a changelog for GitHub repositories"
     let sinceTag: OptionArgument<String>
     let baseBranch: OptionArgument<String>
+    let isSectioned: OptionArgument<Bool>
 
     init(subparser: ArgumentParser) {
         sinceTag = subparser.add(option: "--since-tag", shortName: "-s", kind: String.self, usage: "The tag to use as a base. Defaults to the latest tag.")
         baseBranch = subparser.add(option: "--base-branch", shortName: "-b", kind: String.self, usage: "The base branch to compare with. Defaults to master.")
+        isSectioned = subparser.add(option: "--sections", kind: Bool.self, usage: "Whether the changelog should be split into sections. Defaults to false.")
     }
 
     @discardableResult func run(using arguments: ArgumentParser.Result) throws -> String {
         let sinceTag = arguments.get(self.sinceTag).map { ChangelogProducer.Since.tag(tag: $0) }
         let changelogProducer = try ChangelogProducer(since: sinceTag ?? .latestTag,
                                                       baseBranch: arguments.get(baseBranch))
-        return try changelogProducer.run().description
+        return try changelogProducer.run(isSectioned: arguments.get(isSectioned) ?? false).description
     }
 }
 
@@ -78,7 +80,7 @@ final class ChangelogProducer: URLSessionInjectable {
         project = GITProject.current()
     }
 
-    @discardableResult public func run() throws -> Changelog {
+    @discardableResult public func run(isSectioned: Bool) throws -> Changelog {
         let pullRequestsFetcher = PullRequestFetcher(octoKit: octoKit, baseBranch: baseBranch, project: project)
         let pullRequests = try pullRequestsFetcher.fetchAllBetween(from, and: to, using: urlSession)
 
@@ -90,9 +92,28 @@ final class ChangelogProducer: URLSessionInjectable {
             }
         }
 
-        let items = ChangelogItemsFactory(octoKit: octoKit, pullRequests: pullRequests, project: project).items(using: urlSession)
+        if isSectioned {
+            let issuesFetcher = IssuesFetcher(octoKit: octoKit, project: project)
+            let issues = try issuesFetcher.fetchAllBetween(from, and: to, using: urlSession)
 
-        Log.debug("Result of creating the changelog:")
-        return Changelog(items: items)
+            if Log.isVerbose {
+                Log.debug("\nChangelog will use the following issues as input:")
+                issues.forEach { issue in
+                    guard let title = issue.title, let closedAt = issue.closedAt else { return }
+                    Log.debug("- #\(issue.number): \(title), closed at: \(closedAt)\n")
+                }
+            }
+
+            return SectionedChangelog(issues: issues, pullRequests: pullRequests)
+        } else {
+            let items = ChangelogItemsFactory(
+                octoKit: octoKit,
+                pullRequests: pullRequests,
+                project: project
+            ).items(using: urlSession)
+
+            Log.debug("Result of creating the changelog:")
+            return SingleSectionChangelog(items: items)
+        }
     }
 }
diff --git a/Sources/GitBuddyCore/GitHub/IssuesFetcher.swift b/Sources/GitBuddyCore/GitHub/IssuesFetcher.swift
@@ -0,0 +1,46 @@
+//
+//  IssuesFetcher.swift
+//
+//
+//  Created by Max Desiatov on 26/03/2020.
+//
+
+import Foundation
+import OctoKit
+
+struct IssuesFetcher {
+
+    let octoKit: Octokit
+    let project: GITProject
+
+    /// Fetches issues and filters them by a given `fromDate` and `toDate`.
+    func fetchAllBetween(_ fromDate: Date, and toDate: Date, using session: URLSession = URLSession.shared) throws -> [Issue] {
+        let group = DispatchGroup()
+        group.enter()
+
+        var result: Result<[Issue], Swift.Error>!
+
+        octoKit.issues(session, owner: project.organisation, repository: project.repository, state: .Closed) { (response) in
+            switch response {
+            case .success(let issues):
+                result = .success(issues)
+            case .failure(let error):
+                result = .failure(error)
+            }
+            group.leave()
+        }
+        group.wait()
+
+        return try result.get().filter { issue -> Bool in
+            guard
+                // It looks like OctoKit.swift doesn't support `pull_request` property that helps in
+                // distinguishing between issues and pull requests, as the issues endpoint returns both.
+                // See https://developer.github.com/v3/issues/ for more details.
+                issue.htmlURL?.pathComponents.contains("issues") ?? false,
+                let closedAt = issue.closedAt
+            else { return false }
+            return closedAt > fromDate && closedAt < toDate
+        }
+    }
+
+}
diff --git a/Sources/GitBuddyCore/Models/Changelog.swift b/Sources/GitBuddyCore/Models/Changelog.swift
@@ -9,12 +9,21 @@
 import Foundation
 import OctoKit
 
-struct Changelog: CustomStringConvertible {
-    typealias PullRequestID = Int
-    typealias IssueID = Int
+typealias PullRequestID = Int
+typealias IssueID = Int
+
+/// Generalizes different types of changelogs with either single or multiple sections.
+protocol Changelog: CustomStringConvertible {
+    /// The pull requests ID and related issues IDs that are merged with the related release of this
+    /// changelog. It is used to post update comments on corresponding PRs and issues when a release
+    /// is published.
+    var itemIdentifiers: [PullRequestID: [IssueID]] { get }
+}
+
+/// Represents a changelog with a single section of changelog items.
+struct SingleSectionChangelog: Changelog {
     let description: String
 
-    /// The pull requests ID and related issues IDs that are merged with the related release of this changelog.
     let itemIdentifiers: [PullRequestID: [IssueID]]
 
     init(items: [ChangelogItem]) {
@@ -35,6 +44,31 @@ struct Changelog: CustomStringConvertible {
     }
 }
 
+/// Represents a changelog with at least two sections, one for closed issues, the other for
+/// merged pull requests.
+struct SectionedChangelog: Changelog {
+    let description: String
+
+    let itemIdentifiers: [PullRequestID: [IssueID]]
+
+    init(issues: [Issue], pullRequests: [PullRequest]) {
+        description =
+            """
+            **Closed issues:**
+
+            \(ChangelogBuilder(items: issues.map { ChangelogItem(input: $0, closedBy: $0) }).build())
+
+            **Merged pull requests:**
+
+            \(ChangelogBuilder(items: pullRequests.map { ChangelogItem(input: $0, closedBy: $0) }).build())
+            """
+
+        itemIdentifiers = pullRequests.reduce(into: [:]) { (result, item) in
+            result[item.number] = item.body?.resolvingIssues()
+        }
+    }
+}
+
 extension PullRequest: Hashable {
     public func hash(into hasher: inout Hasher) {
         hasher.combine(id)

diff --git a/Sources/GitBuddyCore/Models/ChangelogItem.swift b/Sources/GitBuddyCore/Models/ChangelogItem.swift
@@ -29,14 +29,14 @@ extension Issue: ChangelogIssue {
 
 struct ChangelogItem {
     let input: ChangelogInput
-    let closedBy: ChangelogPullRequest
+    let closedBy: ChangelogInput
 
     var title: String? {
         guard var title = input.title else { return nil }
         if let htmlURL = input.htmlURL {
             title += " ([#\(input.number)](\(htmlURL)))"
         }
-        if let username = closedBy.username {
+        if closedBy is ChangelogPullRequest, let username = closedBy.username {
             title += " via [@\(username)](https://github.com/\(username))"
         }
         return title

diff --git a/Sources/GitBuddyCore/Release/ReleaseProducer.swift b/Sources/GitBuddyCore/Release/ReleaseProducer.swift
@@ -21,6 +21,7 @@ struct ReleaseCommand: Command {
     let releaseTitle: OptionArgument<String>
     let lastReleaseTag: OptionArgument<String>
     let baseBranch: OptionArgument<String>
+    let isSectioned: OptionArgument<Bool>
 
     init(subparser: ArgumentParser) {
         changelogPath = subparser.add(option: "--changelog-path", shortName: "-c", kind: String.self, usage: "The path to the Changelog to update it with the latest changes")
@@ -31,6 +32,7 @@ struct ReleaseCommand: Command {
         releaseTitle = subparser.add(option: "--release-title", shortName: "-r", kind: String.self, usage: "The title of the release. Default: uses the tag name.")
         lastReleaseTag = subparser.add(option: "--last-release-tag", shortName: "-l", kind: String.self, usage: "The last release tag to use as a base for the changelog creation. Default: previous tag")
         baseBranch = subparser.add(option: "--base-branch", shortName: "-b", kind: String.self, usage: "The base branch to compare with for generating the changelog. Defaults to master.")
+        isSectioned = subparser.add(option: "--sections", kind: Bool.self, usage: "Whether the changelog should be split into sections. Defaults to false.")
     }
 
     @discardableResult func run(using arguments: ArgumentParser.Result) throws -> String {
@@ -42,7 +44,7 @@ struct ReleaseCommand: Command {
                                                   releaseTitle: arguments.get(releaseTitle),
                                                   lastReleaseTag: arguments.get(lastReleaseTag),
                                                   baseBranch: arguments.get(baseBranch))
-        return try releaseProducer.run().url.absoluteString
+        return try releaseProducer.run(isSectioned: arguments.get(isSectioned) ?? false).url.absoluteString
     }
 }
 
@@ -74,14 +76,14 @@ final class ReleaseProducer: URLSessionInjectable, ShellInjectable {
         self.baseBranch = baseBranch ?? "master"
     }
 
-    @discardableResult public func run() throws -> Release {
+    @discardableResult public func run(isSectioned: Bool) throws -> Release {
         let releasedTag = try tagName.map { try Tag(name: $0, created: Date()) } ?? Tag.latest()
         let previousTag = lastReleaseTag ?? Self.shell.execute(.previousTag)
 
         /// We're adding 60 seconds to make sure the tag commit itself is included in the changelog as well.
         let toDate = releasedTag.created.addingTimeInterval(60)
         let changelogProducer = try ChangelogProducer(since: .tag(tag: previousTag), to: toDate, baseBranch: baseBranch)
-        let changelog = try changelogProducer.run()
+        let changelog = try changelogProducer.run(isSectioned: isSectioned)
         Log.debug("\(changelog)\n")
 
         try updateChangelogFile(adding: changelog.description, for: releasedTag)

diff --git a/Tests/GitBuddyTests/Changelog/ChangelogProducerTests.swift b/Tests/GitBuddyTests/Changelog/ChangelogProducerTests.swift
@@ -48,6 +48,36 @@ final class ChangelogProducerTests: XCTestCase {
         )
     }
 
+    /// It should correctly output the changelog.
+    func testSectionedChangelogOutput() throws {
+        Mocker.mockPullRequests()
+        Mocker.mockIssues()
+        Mocker.mockForIssueNumber(39)
+        MockedShell.mockGITProject(organisation: "WeTransfer", repository: "Diagnostics")
+        let changelog = try GitBuddy.run(arguments: ["GitBuddy", "changelog", "--sections"], configuration: configuration)
+        XCTAssertEqual(
+            changelog,
+            """
+            **Closed issues:**
+
+            - Include device product names ([#60](https://github.com/WeTransfer/Diagnostics/issues/60))
+            - Change the order of reported sessions ([#54](https://github.com/WeTransfer/Diagnostics/issues/54))
+            - Encode Logging for HTML so object descriptions are visible ([#51](https://github.com/WeTransfer/Diagnostics/issues/51))
+            - Chinese characters display incorrectly in HTML output in Safari ([#48](https://github.com/WeTransfer/Diagnostics/issues/48))
+            - Get warning for file 'style.css' after building ([#39](https://github.com/WeTransfer/Diagnostics/issues/39))
+            - Crash happening when there is no space left on the device ([#37](https://github.com/WeTransfer/Diagnostics/issues/37))
+            - Add support for users without the Apple Mail app ([#36](https://github.com/WeTransfer/Diagnostics/issues/36))
+            - Support for Apple Watch App Logs ([#33](https://github.com/WeTransfer/Diagnostics/issues/33))
+            - Support different platforms/APIs ([#30](https://github.com/WeTransfer/Diagnostics/issues/30))
+            - Strongly typed HTML would be nice ([#6](https://github.com/WeTransfer/Diagnostics/issues/6))
+
+            **Merged pull requests:**
+
+            - Add charset utf-8 to html head ([#50](https://github.com/WeTransfer/Diagnostics/pull/50)) via [@AvdLee](https://github.com/AvdLee)
+            """
+        )
+    }
+
     /// It should default to master branch.
     func testDefaultBranch() throws {
         let producer = try ChangelogProducer(baseBranch: nil)

diff --git a/...GitBuddyTests/Models/ChangelogTests.swift → .../Models/SingleSectionChangelogTests.swift b/...GitBuddyTests/Models/ChangelogTests.swift → .../Models/SingleSectionChangelogTests.swift
@@ -1,5 +1,5 @@
 //
-//  ChangelogTests.swift
+//  SingleSectionChangelogTests.swift
 //  GitBuddyTests
 //
 //  Created by Antoine van der Lee on 05/02/2020.
@@ -10,14 +10,14 @@ import Foundation
 import XCTest
 @testable import GitBuddyCore
 
-final class ChangelogTests: XCTestCase {
+final class SingleSectionChangelogTests: XCTestCase {
 
     /// It should report a single issue from a pull request correctly.
     func testPullRequestSingleIssue() {
         let pullRequest = MockedPullRequest(number: 0)
         let issue = MockedIssue(number: 0)
         let changelogItem = ChangelogItem(input: issue, closedBy: pullRequest)
-        let changelog = Changelog(items: [changelogItem])
+        let changelog = SingleSectionChangelog(items: [changelogItem])
 
         XCTAssertEqual(changelog.itemIdentifiers, [0: [0]])
     }
@@ -29,7 +29,7 @@ final class ChangelogTests: XCTestCase {
             ChangelogItem(input: MockedIssue(number: 0), closedBy: pullRequest),
             ChangelogItem(input: MockedIssue(number: 1), closedBy: pullRequest)
         ]
-        let changelog = Changelog(items: changelogItems)
+        let changelog = SingleSectionChangelog(items: changelogItems)
 
         XCTAssertEqual(changelog.itemIdentifiers, [0: [0, 1]])
     }
@@ -38,7 +38,7 @@ final class ChangelogTests: XCTestCase {
     func testPullRequestNoIssues() {
         let pullRequest = MockedPullRequest(number: 0)
         let changelogItem = ChangelogItem(input: pullRequest, closedBy: pullRequest)
-        let changelog = Changelog(items: [changelogItem])
+        let changelog = SingleSectionChangelog(items: [changelogItem])
 
         XCTAssertEqual(changelog.itemIdentifiers, [0: []])
     }
@@ -48,7 +48,7 @@ final class ChangelogTests: XCTestCase {
         let pullRequest = MockedPullRequest(number: 0)
         let issue = MockedIssue(title: "Fixed something")
         let changelogItem = ChangelogItem(input: issue, closedBy: pullRequest)
-        let changelog = Changelog(items: [changelogItem])
+        let changelog = SingleSectionChangelog(items: [changelogItem])
 
         XCTAssertEqual(changelog.description, "- Fixed something")
     }