Merge pull request #61 from codeclimate/ia-issues-counts-in-pr-messages

Show count of new, fixed issues in PR statuses

Author: iancanderson

codeclimate-services.gemspec

@@ -21,6 +21,7 @@ Gem::Specification.new do |spec|
   spec.add_dependency "virtus", "1.0.0"
   spec.add_dependency "nokogiri", "~> 1.6.0"
   spec.add_dependency "activemodel", ">= 3.0"
+  spec.add_dependency "activesupport", ">= 3.0"
   spec.add_development_dependency "bundler", ">= 1.6.2"
   spec.add_development_dependency "rake"
   spec.add_development_dependency "test-unit"

lib/cc/services/github_pull_requests.rb

@@ -70,7 +70,10 @@ def update_status_skipped
 
   def update_status_success
     add_comment
-    update_status("success", "Code Climate has analyzed this pull request.")
+    update_status(
+      "success",
+      CC::Service::GitHubPullRequests::Presenter.new(@payload).success_message
+    )
   end
 
   def update_status_error

lib/cc/services/github_pull_requests_presenter.rb

@@ -0,0 +1,52 @@
+class CC::Service::GitHubPullRequests::Presenter
+  def initialize(payload)
+    @fixed_count = payload["fixed_issue_count"]
+    @new_count = payload["new_issue_count"]
+  end
+
+  def success_message
+    if issue_counts_in_payload?
+      if both_issue_counts_zero?
+        "Code Climate didn't find any new or fixed issues."
+      else
+        "Code Climate found #{formatted_issue_counts}."
+      end
+    else
+      "Code Climate has analyzed this pull request."
+    end
+  end
+
+private
+
+  def both_issue_counts_zero?
+    issue_counts.all?(&:zero?)
+  end
+
+  def formatted_fixed_issues
+    if @fixed_count > 0
+      "#{@fixed_count} fixed #{"issue".pluralize(@fixed_count)}"
+    else
+      nil
+    end
+  end
+
+  def formatted_new_issues
+    if @new_count > 0
+      "#{@new_count} new #{"issue".pluralize(@new_count)}"
+    else
+      nil
+    end
+  end
+
+  def formatted_issue_counts
+    [formatted_new_issues, formatted_fixed_issues].compact.to_sentence
+  end
+
+  def issue_counts
+    [@new_count, @fixed_count]
+  end
+
+  def issue_counts_in_payload?
+    issue_counts.all?
+  end
+end

test/github_pull_requests_presenter_test.rb

@@ -0,0 +1,51 @@
+require File.expand_path('../helper', __FILE__)
+
+class TestGitHubPullRequestsPresenter < CC::Service::TestCase
+  def test_message_no_issue_counts_in_payload
+    assert_equal(
+      "Code Climate has analyzed this pull request.",
+      message_from_payload({})
+    )
+  end
+
+  def test_message_singular
+    assert_equal(
+      "Code Climate found 1 new issue and 1 fixed issue.",
+      message_from_payload("fixed_issue_count" => 1, "new_issue_count" => 1)
+    )
+  end
+
+  def test_message_plural
+    assert_equal(
+      "Code Climate found 2 new issues and 1 fixed issue.",
+      message_from_payload("fixed_issue_count" => 1, "new_issue_count" => 2)
+    )
+  end
+
+  def test_message_only_fixed
+    assert_equal(
+      "Code Climate found 1 fixed issue.",
+      message_from_payload("fixed_issue_count" => 1, "new_issue_count" => 0)
+    )
+  end
+
+  def test_message_only_new
+    assert_equal(
+      "Code Climate found 3 new issues.",
+      message_from_payload("fixed_issue_count" => 0, "new_issue_count" => 3)
+    )
+  end
+
+  def test_message_no_new_or_fixed
+    assert_equal(
+      "Code Climate didn't find any new or fixed issues.",
+      message_from_payload("fixed_issue_count" => 0, "new_issue_count" => 0)
+    )
+  end
+
+private
+
+  def message_from_payload(payload)
+    CC::Service::GitHubPullRequests::Presenter.new(payload).success_message
+  end
+end

test/github_pull_requests_test.rb

@@ -14,7 +14,22 @@ def test_pull_request_status_pending
     })
   end
 
-  def test_pull_request_status_success
+  def test_pull_request_status_success_detailed
+    expect_status_update("pbrisbin/foo", "abc123", {
+      "state"       => "success",
+      "description" => "Code Climate found 1 new issue and 2 fixed issues.",
+    })
+
+    receive_pull_request({ update_status: true }, {
+      github_slug: "pbrisbin/foo",
+      commit_sha:  "abc123",
+      state:       "success",
+      new_issue_count: 1,
+      fixed_issue_count: 2,
+    })
+  end
+
+  def test_pull_request_status_success_generic
     expect_status_update("pbrisbin/foo", "abc123", {
       "state"       => "success",
       "description" => /has analyzed/,