From be21d8d3efb7c86f35d168ddd30b906631cdbe64 Mon Sep 17 00:00:00 2001 From: Jared Pace Date: Wed, 14 May 2014 11:26:45 -0700 Subject: [PATCH 1/5] Flatten task list for simpler access to matches --- lib/services/asana.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/services/asana.rb b/lib/services/asana.rb index 205d95f5a..52beccb58 100644 --- a/lib/services/asana.rb +++ b/lib/services/asana.rb @@ -42,12 +42,12 @@ def check_commit(commit, push_msg) end # post commit to every taskid found - task_list.each do |taskid| + task_list.flatten.each do |taskid| 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 + res = http_post "https://app.asana.com/api/1.0/tasks/" + taskid + "/stories", "text=" + push_msg + message if res.status < 200 || res.status > 299 raise_config_error res.message end From cdfa5d3a70bfa5dc5dee472acb493b0471080860 Mon Sep 17 00:00:00 2001 From: Jared Pace Date: Wed, 14 May 2014 11:38:27 -0700 Subject: [PATCH 2/5] Refactor Asana service to use standard Ruby interpolation --- lib/services/asana.rb | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/lib/services/asana.rb b/lib/services/asana.rb index 52beccb58..c3c18b491 100644 --- 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| @@ -43,15 +43,17 @@ def check_commit(commit, push_msg) # post commit to every taskid found 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 + "/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}" + if res.status < 200 || res.status > 299 + raise_config_error res.message end end - end From 1497cdd8d4acb4b102775e7daed7d93a6a7d0453 Mon Sep 17 00:00:00 2001 From: Jared Pace Date: Wed, 14 May 2014 11:42:50 -0700 Subject: [PATCH 3/5] Ignore 400 responses for unknown tasks from Asana. --- lib/services/asana.rb | 7 ++++++- test/asana_test.rb | 20 ++++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/lib/services/asana.rb b/lib/services/asana.rb index c3c18b491..8e6bfe161 100644 --- a/lib/services/asana.rb +++ b/lib/services/asana.rb @@ -52,7 +52,12 @@ def deliver_story(task_id, text) http.headers['X-GitHub-Event'] = event.to_s res = http_post "https://app.asana.com/api/1.0/tasks/#{task_id}/stories", "text=#{text}" - if res.status < 200 || res.status > 299 + case res.status + when 200..299 + # Success + when 400 + # Unknown task. Could be GitHub issue or pull request number. Ignore it. + else raise_config_error res.message end end diff --git a/test/asana_test.rb b/test/asana_test.rb index 703a8cd39..716fe32e1 100644 --- a/test/asana_test.rb +++ b/test/asana_test.rb @@ -71,6 +71,20 @@ 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 service(*args) super Service::Asana, *args end @@ -83,4 +97,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 From cb41edcf0c5254c7d2a2ae79685d30658bb5a038 Mon Sep 17 00:00:00 2001 From: Jared Pace Date: Wed, 14 May 2014 12:04:29 -0700 Subject: [PATCH 4/5] Pull error messages from JSON response --- lib/services/asana.rb | 4 +++- test/asana_test.rb | 15 +++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/lib/services/asana.rb b/lib/services/asana.rb index 8e6bfe161..5e81e9088 100644 --- a/lib/services/asana.rb +++ b/lib/services/asana.rb @@ -58,7 +58,9 @@ def deliver_story(task_id, text) when 400 # Unknown task. Could be GitHub issue or pull request number. Ignore it. else - raise_config_error res.message + # 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 index 716fe32e1..d9a80d724 100644 --- a/test/asana_test.rb +++ b/test/asana_test.rb @@ -85,6 +85,21 @@ def test_merge_pull_request_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 service(*args) super Service::Asana, *args end From a7811da6a48a300c8ab9307fe474d9172b538fa0 Mon Sep 17 00:00:00 2001 From: Jared Pace Date: Wed, 14 May 2014 12:06:19 -0700 Subject: [PATCH 5/5] Handle cases where Asana crashes without JSON error --- test/asana_test.rb | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/test/asana_test.rb b/test/asana_test.rb index d9a80d724..aef40e9ed 100644 --- a/test/asana_test.rb +++ b/test/asana_test.rb @@ -100,6 +100,21 @@ def test_error_response 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