Merge pull request #550 from hunner/chef-adjust-tag-section-mapping

Fix section mapping, hiding untagged PRs, and hiding untagged issues

lib/github_changelog_generator/generator/generator.rb

@@ -57,14 +57,23 @@ module GitHubChangelogGenerator
     # @param [Array] pull_requests List or PR's in new section
     # @param [Array] issues List of issues in new section
     # @param [String] newer_tag Name of the newer tag. Could be nil for `Unreleased` section
-    # @param [String] older_tag_name Older tag, used for the links. Could be nil for last tag.
+    # @param [Hash, nil] older_tag Older tag, used for the links. Could be nil for last tag.
     # @return [String] Ready and parsed section
-    def create_log_for_tag(pull_requests, issues, newer_tag, older_tag_name = nil)
+    def create_log_for_tag(pull_requests, issues, newer_tag, older_tag = nil)
       newer_tag_link, newer_tag_name, newer_tag_time = detect_link_tag_time(newer_tag)
 
       github_site = options[:github_site] || "https://github.com"
       project_url = "#{github_site}/#{options[:user]}/#{options[:project]}"
 
+      # If the older tag is nil, go back in time from the latest tag and find
+      # the SHA for the first commit.
+      older_tag_name =
+        if older_tag.nil?
+          @fetcher.commits_before(newer_tag_time).last["sha"]
+        else
+          older_tag["name"]
+        end
+
       log = generate_header(newer_tag_name, newer_tag_link, newer_tag_time, older_tag_name, project_url)
 
       if options[:issues]
@@ -72,7 +81,7 @@ module GitHubChangelogGenerator
         log += issues_to_log(issues, pull_requests)
       end
 
-      if options[:pulls]
+      if options[:pulls] && options[:add_pr_wo_labels]
         # Generate pull requests:
         log += generate_sub_section(pull_requests, options[:merge_prefix])
       end

lib/github_changelog_generator/generator/generator_generation.rb

@@ -83,14 +83,12 @@ module GitHubChangelogGenerator
     def generate_log_between_tags(older_tag, newer_tag)
       filtered_issues, filtered_pull_requests = filter_issues_for_tags(newer_tag, older_tag)
 
-      older_tag_name = older_tag.nil? ? detect_since_tag : older_tag["name"]
-
       if newer_tag.nil? && filtered_issues.empty? && filtered_pull_requests.empty?
         # do not generate empty unreleased section
         return ""
       end
 
-      create_log_for_tag(filtered_pull_requests, filtered_issues, newer_tag, older_tag_name)
+      create_log_for_tag(filtered_pull_requests, filtered_issues, newer_tag, older_tag)
     end
 
     # Apply all filters to issues and pull requests

lib/github_changelog_generator/generator/generator_processor.rb

@@ -126,19 +126,17 @@ module GitHubChangelogGenerator
     # @return [Array] filtered array of issues
     def include_issues_by_labels(issues)
       filtered_issues = filter_by_include_labels(issues)
-      filtered_issues |= filter_wo_labels(issues)
+      filtered_issues = filter_wo_labels(filtered_issues)
       filtered_issues
     end
 
     # @return [Array] issues without labels or empty array if add_issues_wo_labels is false
     def filter_wo_labels(issues)
       if options[:add_issues_wo_labels]
-        issues_wo_labels = issues.reject do |issue|
+        issues
-          issue["labels"].map { |l| l["name"] }.none?
+      else
+        issues.select { |issue| issue["labels"].map { |l| l["name"] }.any? }
       end
-        return issues_wo_labels
-      end
-      []
     end
 
     def filter_by_include_labels(issues)

lib/github_changelog_generator/generator/generator_tags.rb

@@ -8,30 +8,47 @@ module GitHubChangelogGenerator
       detect_due_tag
 
       all_tags = @fetcher.get_all_tags
-      included_tags = filter_excluded_tags(all_tags)
-
       fetch_tags_dates(all_tags) # Creates a Hash @tag_times_hash
-      @sorted_tags   = sort_tags_by_date(included_tags)
+      all_sorted_tags = sort_tags_by_date(all_tags)
-      @filtered_tags = get_filtered_tags(included_tags)
 
-      @tag_section_mapping = build_tag_section_mapping(@filtered_tags, sorted_tags)
+      @sorted_tags   = filter_excluded_tags(all_sorted_tags)
+      @filtered_tags = get_filtered_tags(@sorted_tags)
+
+      # Because we need to properly create compare links, we need a sorted list
+      # of all filtered tags (including the excluded ones). We'll exclude those
+      # tags from section headers inside the mapping function.
+      section_tags = get_filtered_tags(all_sorted_tags)
+
+      @tag_section_mapping = build_tag_section_mapping(section_tags, @filtered_tags)
 
       @filtered_tags
     end
 
-    # @param [Array] filtered_tags are the tags that need a subsection output
+    # @param [Array] section_tags are the tags that need a subsection output
-    # @param [Array] all_tags is the list of all tags ordered from newest -> oldest
+    # @param [Array] filtered_tags is the list of filtered tags ordered from newest -> oldest
     # @return [Hash] key is the tag to output, value is an array of [Left Tag, Right Tag]
     # PRs to include in this section will be >= [Left Tag Date] and <= [Right Tag Date]
-    def build_tag_section_mapping(filtered_tags, all_tags)
+    # rubocop:disable Style/For - for allows us to be more concise
+    def build_tag_section_mapping(section_tags, filtered_tags)
       tag_mapping = {}
-      filtered_tags.each do |tag|
+      for i in 0..(section_tags.length - 1)
-        older_tag_idx = all_tags.index(tag) + 1
+        tag = section_tags[i]
-        older_tag = all_tags[older_tag_idx]
+
+        # Don't create section header for the "since" tag
+        next if @since_tag && tag["name"] == @since_tag
+
+        # Don't create a section header for the first tag in between_tags
+        next if options[:between_tags] && tag == section_tags.last
+
+        # Don't create a section header for excluded tags
+        next unless filtered_tags.include?(tag)
+
+        older_tag = section_tags[i + 1]
         tag_mapping[tag] = [older_tag, tag]
       end
       tag_mapping
     end
+    # rubocop:enable Style/For
 
     # Sort all tags by date, newest to oldest
     def sort_tags_by_date(tags)
@@ -113,7 +130,7 @@ module GitHubChangelogGenerator
         if all_tags.map { |t| t["name"] }.include? tag
           idx = all_tags.index { |t| t["name"] == tag }
           filtered_tags = if idx > 0
-                            all_tags[0..idx - 1]
+                            all_tags[0..idx]
                           else
                             []
                           end

lib/github_changelog_generator/octo_fetcher.rb

@@ -236,6 +236,17 @@ Make sure, that you push tags to remote repo via 'git push --tags'"
       end
     end
 
+    # Fetch all commits before certain point
+    #
+    # @return [String]
+    def commits_before(start_time)
+      commits = []
+      iterate_pages(@client, "commits_before", start_time.to_datetime.to_s) do |new_commits|
+        commits.concat(new_commits)
+      end
+      commits
+    end
+
     private
 
     def stringify_keys_deep(indata)

lib/github_changelog_generator/parser.rb

@@ -207,9 +207,9 @@ module GitHubChangelogGenerator
         unreleased: true,
         unreleased_label: "Unreleased",
         compare_link: true,
-        enhancement_labels: %w[enhancement Enhancement],
+        enhancement_labels: ["enhancement", "Enhancement", "Type: Enhancement"],
-        bug_labels: %w[bug Bug],
+        bug_labels: ["bug", "Bug", "Type: Bug"],
-        exclude_labels: %w[duplicate question invalid wontfix Duplicate Question Invalid Wontfix],
+        exclude_labels: ["duplicate", "question", "invalid", "wontfix", "Duplicate", "Question", "Invalid", "Wontfix", "Meta: Exclude From Changelog"],
         issue_line_labels: [],
         max_issues: nil,
         simple_list: false,

spec/unit/generator/generator_processor_spec.rb

@@ -2,19 +2,31 @@
 
 module GitHubChangelogGenerator
   describe Generator do
-    context "#exclude_issues_by_labels" do
+    let(:default_options) { GitHubChangelogGenerator::Parser.default_options }
-      let(:label) { { "name" => "BAD" } }
+    let(:options) { {} }
-      let(:issue) { { "labels" => [label] } }
+    let(:generator) { described_class.new(default_options.merge(options)) }
+
+    let(:bad_label) { { "name" => "BAD" } }
+    let(:bad_issue) { { "labels" => [bad_label] } }
     let(:good_label) { { "name" => "GOOD" } }
     let(:good_issue) { { "labels" => [good_label] } }
-      let(:issues) { [issue, good_issue] }
+    let(:unlabeled_issue) { { "labels" => [] } }
-      subject(:generator) { described_class.new(exclude_labels: %w[BAD BOO]) }
+    let(:issues) { [bad_issue, good_issue, unlabeled_issue] }
 
-      it "removes issues with labels in the exclude_label list" do
+    describe "#exclude_issues_by_labels" do
-        result = generator.exclude_issues_by_labels(issues)
+      subject do
+        generator.exclude_issues_by_labels(issues)
+      end
 
-        expect(result).to include(good_issue)
+      let(:expected_issues) { issues }
-        expect(result).not_to include(issue)
+
+      it { is_expected.to eq(expected_issues) }
+
+      context "when 'exclude_lables' is provided" do
+        let(:options) { { exclude_labels: %w[BAD BOO] } }
+        let(:expected_issues) { [good_issue, unlabeled_issue] }
+
+        it { is_expected.to eq(expected_issues) }
       end
 
       context "with no option given" do
@@ -26,5 +38,43 @@ module GitHubChangelogGenerator
         end
       end
     end
+
+    describe "#get_filtered_issues" do
+      subject do
+        generator.get_filtered_issues(issues)
+      end
+
+      let(:expected_issues) { issues }
+
+      it { is_expected.to eq(expected_issues) }
+
+      context "when 'exclude_labels' is provided" do
+        let(:options) { { exclude_labels: %w[BAD BOO] } }
+        let(:expected_issues) { [good_issue, unlabeled_issue] }
+
+        it { is_expected.to eq(expected_issues) }
+      end
+
+      context "when 'add_issues_wo_labels' is false" do
+        let(:options) { { add_issues_wo_labels: false } }
+        let(:expected_issues) { [bad_issue, good_issue] }
+
+        it { is_expected.to eq(expected_issues) }
+
+        context "with 'exclude_labels'" do
+          let(:options) { { add_issues_wo_labels: false, exclude_labels: %w[GOOD] } }
+          let(:expected_issues) { [bad_issue] }
+
+          it { is_expected.to eq(expected_issues) }
+        end
+      end
+
+      context "when 'include_labels' is specified" do
+        let(:options) { { include_labels: %w[GOOD] } }
+        let(:expected_issues) { [good_issue] }
+
+        it { is_expected.to eq(expected_issues) }
+      end
+    end
   end
 end

spec/unit/generator/generator_tags_spec.rb

@@ -13,6 +13,123 @@ describe GitHubChangelogGenerator::Generator do
     end
   end
 
+  describe "#tag_section_mapping" do
+    let(:all_tags) { tags_from_strings(%w[8 7 6 5 4 3 2 1]) }
+    let(:sorted_tags) { all_tags }
+
+    let(:default_options) { GitHubChangelogGenerator::Parser.default_options }
+    let(:options) { {} }
+    let(:generator) { described_class.new(default_options.merge(options)) }
+
+    before do
+      allow_any_instance_of(GitHubChangelogGenerator::OctoFetcher).to receive(:get_all_tags).and_return(all_tags)
+      allow(generator).to receive(:fetch_tags_dates).with(all_tags)
+      allow(generator).to receive(:sort_tags_by_date).with(all_tags).and_return(sorted_tags)
+      generator.fetch_and_filter_tags
+    end
+
+    subject do
+      generator.tag_section_mapping
+    end
+
+    shared_examples_for "a section mapping" do
+      it { is_expected.to be_a(Hash) }
+      it { is_expected.to eq(expected_mapping) }
+    end
+
+    shared_examples_for "a full changelog" do
+      let(:expected_mapping) do
+        {
+          tag_with_name("8") => [tag_with_name("7"), tag_with_name("8")],
+          tag_with_name("7") => [tag_with_name("6"), tag_with_name("7")],
+          tag_with_name("6") => [tag_with_name("5"), tag_with_name("6")],
+          tag_with_name("5") => [tag_with_name("4"), tag_with_name("5")],
+          tag_with_name("4") => [tag_with_name("3"), tag_with_name("4")],
+          tag_with_name("3") => [tag_with_name("2"), tag_with_name("3")],
+          tag_with_name("2") => [tag_with_name("1"), tag_with_name("2")],
+          tag_with_name("1") => [nil, tag_with_name("1")]
+        }
+      end
+
+      it_behaves_like "a section mapping"
+    end
+
+    shared_examples_for "a changelog with some exclusions" do
+      let(:expected_mapping) do
+        {
+          tag_with_name("8") => [tag_with_name("7"), tag_with_name("8")],
+          tag_with_name("6") => [tag_with_name("5"), tag_with_name("6")],
+          tag_with_name("4") => [tag_with_name("3"), tag_with_name("4")],
+          tag_with_name("3") => [tag_with_name("2"), tag_with_name("3")],
+          tag_with_name("1") => [nil, tag_with_name("1")]
+        }
+      end
+
+      it_behaves_like "a section mapping"
+    end
+
+    context "with no constraints" do
+      it_behaves_like "a full changelog"
+    end
+
+    context "with since only" do
+      let(:options) { { since_tag: "6" } }
+      let(:expected_mapping) do
+        {
+          tag_with_name("8") => [tag_with_name("7"), tag_with_name("8")],
+          tag_with_name("7") => [tag_with_name("6"), tag_with_name("7")]
+        }
+      end
+
+      it_behaves_like "a section mapping"
+    end
+
+    context "with due only" do
+      let(:options) { { due_tag: "4" } }
+      let(:expected_mapping) do
+        {
+          tag_with_name("3") => [tag_with_name("2"), tag_with_name("3")],
+          tag_with_name("2") => [tag_with_name("1"), tag_with_name("2")],
+          tag_with_name("1") => [nil, tag_with_name("1")]
+        }
+      end
+
+      it_behaves_like "a section mapping"
+    end
+
+    context "with since and due" do
+      let(:options) { { since_tag: "2", due_tag: "5" } }
+      let(:expected_mapping) do
+        {
+          tag_with_name("4") => [tag_with_name("3"), tag_with_name("4")],
+          tag_with_name("3") => [tag_with_name("2"), tag_with_name("3")]
+        }
+      end
+
+      it_behaves_like "a section mapping"
+    end
+
+    context "with excluded tags" do
+      context "as a list of strings" do
+        let(:options) { { exclude_tags: %w[2 5 7] } }
+
+        it_behaves_like "a changelog with some exclusions"
+      end
+
+      context "as a regex" do
+        let(:options) { { exclude_tags: /[257]/ } }
+
+        it_behaves_like "a changelog with some exclusions"
+      end
+
+      context "as a regex string" do
+        let(:options) { { exclude_tags_regex: "[257]" } }
+
+        it_behaves_like "a changelog with some exclusions"
+      end
+    end
+  end
+
   describe "#filter_excluded_tags" do
     subject { generator.filter_excluded_tags(tags_from_strings(%w[1 2 3])) }
 
@@ -64,7 +181,7 @@ describe GitHubChangelogGenerator::Generator do
       context "with valid since tag" do
         let(:generator) { GitHubChangelogGenerator::Generator.new(since_tag: "2") }
         it { is_expected.to be_a Array }
-        it { is_expected.to match_array(tags_from_strings(%w[1])) }
+        it { is_expected.to match_array(tags_from_strings(%w[1 2])) }
       end
 
       context "with invalid since tag" do

spec/unit/octo_fetcher_spec.rb

@@ -526,4 +526,18 @@ describe GitHubChangelogGenerator::OctoFetcher do
       end
     end
   end
+
+  describe "#commits_before" do
+    context "when API is valid", :vcr do
+      let(:start_time) { Time.parse("Wed Mar 4 18:47:17 2015 +0200") }
+
+      subject do
+        fetcher.commits_before(start_time)
+      end
+
+      it "returns commits" do
+        expect(subject.last["sha"]).to eq("4c2d6d1ed58bdb24b870dcb5d9f2ceed0283d69d")
+      end
+    end
+  end
 end