Merge pull request github#854 from github/fixup-asana-ref-bug

atmos · atmos · commit 2fdc300084f6 · 2014-05-14T13:40:32.000-07:00
Fix Asana exceptions when parsing non-existent Asana task IDs
diff --git a/lib/services/asana.rb b/lib/services/asana.rb
@@ -33,7 +33,7 @@ def receive_push
   end
 
   def check_commit(commit, push_msg)
-    message = " (" + commit['url'] + ")\n- " + commit['message']
+    message = "(#{commit['url']})\n- #{commit['message']}"
 
     task_list = []
     message.split("\n").each do |line|
@@ -42,16 +42,25 @@ def check_commit(commit, push_msg)
     end
 
     # post commit to every taskid found
-    task_list.each do |taskid|
+    task_list.flatten.each do |taskid|
+      deliver_story taskid, "#{push_msg} #{message}"
+    end
+  end
 
-      http.basic_auth(data['auth_token'], "")
-      http.headers['X-GitHub-Event'] = event.to_s
+  def deliver_story(task_id, text)
+    http.basic_auth(data['auth_token'], "")
+    http.headers['X-GitHub-Event'] = event.to_s
 
-      res = http_post "https://app.asana.com/api/1.0/tasks/" + taskid[0] + "/stories", "text=" + push_msg + message
-      if res.status < 200 || res.status > 299
-        raise_config_error res.message
-      end
+    res = http_post "https://app.asana.com/api/1.0/tasks/#{task_id}/stories", "text=#{text}"
+    case res.status
+    when 200..299
+      # Success
+    when 400
+      # Unknown task. Could be GitHub issue or pull request number. Ignore it.
+    else
+      # Try to pull out an error message from the Asana response
+      error_message = JSON.parse(res.body)['errors'][0]['message'] rescue nil
+      raise_config_error(error_message || "Unexpected Error")
     end
   end
-
 end
diff --git a/test/asana_test.rb b/test/asana_test.rb
@@ -71,6 +71,50 @@ def test_restricted_branch_commit_push
     svc.receive_push
   end
 
+  def test_merge_pull_request_payload
+    @stubs.post "/api/1.0/tasks/42/stories" do |env|
+      [400, {}, ''] # Asana responds with 400 for unknown tasks
+    end
+
+    @stubs.post "/api/1.0/tasks/1234/stories" do |env|
+      assert_match /#1234/, env[:body]
+      [200, {}, '']
+    end
+
+    svc = service({'auth_token' => '0000'}, merge_payload)
+    assert_nothing_raised { svc.receive_push }
+  end
+
+  def test_error_response
+    @stubs.post "/api/1.0/tasks/1234/stories" do |env|
+      [401, {"Content-Type" => "application/json; charset=UTF-8"}, '{"errors":[{"message":"Not Authorized"}]}']
+    end
+
+    svc = service( {'auth_token' => 'bad-token'}, modified_payload)
+
+    begin
+      svc.receive_push
+    rescue StandardError => e
+      assert_equal Service::ConfigurationError, e.class
+      assert_equal "Not Authorized", e.message
+    end
+  end
+
+  def test_asana_exception
+    @stubs.post "/api/1.0/tasks/1234/stories" do |env|
+      [500, {}, 'Boom!']
+    end
+
+    svc = service( {'auth_token' => '0000'}, modified_payload)
+
+    begin
+      svc.receive_push
+    rescue StandardError => e
+      assert_equal Service::ConfigurationError, e.class
+      assert_equal "Unexpected Error", e.message
+    end
+  end
+
   def service(*args)
     super Service::Asana, *args
   end
@@ -83,4 +127,10 @@ def modified_payload
     pay
   end
 
+  def merge_payload
+    pay = payload
+    pay['commits'][0]['message'] = "Merge pull request #42. Fixes Asana task #1234."
+    pay
+  end
+
  end
