Refactor

docs/bulk_delivery_methods/slack.md

@@ -13,7 +13,9 @@ class CommentNotification
         # ...
       }
     }
-    config.raise_on_failure = true # fail if response is 2xx but body['ok'] is false
+
+    # Slack's chat.postMessage endpoint returns a 200 with {ok: true/false}. Disable this check by setting to false
+    # config.raise_if_not_ok = true
   end
 end
 ```

docs/delivery_methods/slack.md

@@ -13,7 +13,9 @@ class CommentNotification
         # ...
       }
     }
-    config.raise_on_failure = true # fail if response is 2xx but body['ok'] is false
+
+    # Slack's chat.postMessage endpoint returns a 200 with {ok: true/false}. Disable this check by setting to false
+    # config.raise_if_not_ok = true
   end
 end
 ```

lib/noticed/bulk_delivery_methods/slack.rb

@@ -8,12 +8,10 @@ class Slack < BulkDeliveryMethod
       def deliver
         headers = evaluate_option(:headers)
         json = evaluate_option(:json)
-
         response = post_request url, headers: headers, json: json
 
-        if raise_on_failure? && response.body
-          parsed_response = JSON.parse(response.body)
-          raise ResponseUnsuccessful.new(response, url, {headers: headers, json: json}) unless parsed_response["ok"]
+        if raise_if_not_ok?
+          raise ResponseUnsuccessful.new(response, url, {headers: headers, json: json}) unless JSON.parse(response.body)["ok"]
         end
 
         response
@@ -23,8 +21,9 @@ def url
         evaluate_option(:url) || DEFAULT_URL
       end
 
-      def raise_on_failure?
-        evaluate_option(:raise_on_failure) || false
+      def raise_if_not_ok?
+        value = evaluate_option(:raise_if_not_ok)
+        value.nil? ? true : value
       end
     end
   end

lib/noticed/delivery_methods/slack.rb

@@ -8,12 +8,10 @@ class Slack < DeliveryMethod
       def deliver
         headers = evaluate_option(:headers)
         json = evaluate_option(:json)
-
         response = post_request url, headers: headers, json: json
 
-        if raise_on_failure? && response.body
-          parsed_response = JSON.parse(response.body)
-          raise ResponseUnsuccessful.new(response, url, {headers: headers, json: json}) unless parsed_response["ok"]
+        if raise_if_not_ok?
+          raise ResponseUnsuccessful.new(response, url, {headers: headers, json: json}) unless JSON.parse(response.body)["ok"]
         end
 
         response
@@ -23,8 +21,9 @@ def url
         evaluate_option(:url) || DEFAULT_URL
       end
 
-      def raise_on_failure?
-        evaluate_option(:raise_on_failure) || false
+      def raise_if_not_ok?
+        value = evaluate_option(:raise_if_not_ok)
+        value.nil? ? true : value
       end
     end
   end

test/delivery_methods/slack_test.rb

@@ -7,7 +7,7 @@ class SlackTest < ActiveSupport::TestCase
   end
 
   test "sends a slack message" do
-    stub_request(:post, Noticed::DeliveryMethods::Slack::DEFAULT_URL).with(body: "{\"foo\":\"bar\"}")
+    stub_request(:post, Noticed::DeliveryMethods::Slack::DEFAULT_URL).with(body: "{\"foo\":\"bar\"}").to_return(status: 200, body: {ok: true}.to_json)
     assert_nothing_raised do
       @delivery_method.deliver
     end
@@ -20,17 +20,17 @@ class SlackTest < ActiveSupport::TestCase
     end
   end
 
-  test "doesnt raise error on failed 200 status code request with raise_on_failure false" do
-    @delivery_method.config[:raise_on_failure] = false
-    stub_request(:post, Noticed::DeliveryMethods::Slack::DEFAULT_URL).with(body: "{\"foo\":\"bar\"}").to_return(status: 200, body: "{\"ok\": false}")
+  test "doesnt raise error on failed 200 status code request with raise_if_not_ok false" do
+    @delivery_method.config[:raise_if_not_ok] = false
+    stub_request(:post, Noticed::DeliveryMethods::Slack::DEFAULT_URL).with(body: "{\"foo\":\"bar\"}").to_return(status: 200, body: {ok: false}.to_json)
     assert_nothing_raised do
       @delivery_method.deliver
     end
   end
 
-  test "raises error on 200 status code request with raise_on_failure true" do
-    @delivery_method.config[:raise_on_failure] = true
-    stub_request(:post, Noticed::DeliveryMethods::Slack::DEFAULT_URL).with(body: "{\"foo\":\"bar\"}").to_return(status: 200, body: "{\"ok\": false}")
+  test "raises error on 200 status code request with raise_if_not_ok true" do
+    @delivery_method.config[:raise_if_not_ok] = true
+    stub_request(:post, Noticed::DeliveryMethods::Slack::DEFAULT_URL).with(body: "{\"foo\":\"bar\"}").to_return(status: 200, body: {ok: false}.to_json)
     assert_raises Noticed::ResponseUnsuccessful do
       @delivery_method.deliver
     end