From 8a20ab897256527e89c217157773cefef18ff048 Mon Sep 17 00:00:00 2001 From: Brandur Date: Thu, 14 Jan 2016 18:16:56 -0800 Subject: [PATCH] Deprecate `#refund` helpers on `Charge` and `ApplicationFee` As discussed previously in #354 and alluded to in #363, this patch deprecates the `#refund` helpers on `Charge` and `ApplicationFee` in favor of the resource-centric approach (i.e. `charge.refunds.create`). We do this for a few reasons: 1. The new approach is far preferred and uses our modern endpoints. It's also been the mechanism suggested by the documentation for ages now. 2. The old approach is somewhat risky in that a forgotten "s" can lead to an accidental refund (i.e. `charge.refund` instead of `charge.refunds`). Follows up #354. Fixes #363. --- lib/stripe/application_fee.rb | 2 ++ lib/stripe/charge.rb | 2 ++ test/stripe/api_resource_test.rb | 6 +++--- test/stripe/application_fee_test.rb | 33 +++++++++++++++++++++++------ test/stripe/charge_test.rb | 25 ++++++++++++++++++---- 5 files changed, 54 insertions(+), 14 deletions(-) diff --git a/lib/stripe/application_fee.rb b/lib/stripe/application_fee.rb index aa7a5672..96d1d240 100644 --- a/lib/stripe/application_fee.rb +++ b/lib/stripe/application_fee.rb @@ -16,5 +16,7 @@ module Stripe # from the server self.refresh end + extend Gem::Deprecate + deprecate :refund, "application_fee.refunds.create", 2016, 07 end end diff --git a/lib/stripe/charge.rb b/lib/stripe/charge.rb index 4b93599c..03e2037d 100644 --- a/lib/stripe/charge.rb +++ b/lib/stripe/charge.rb @@ -8,6 +8,8 @@ module Stripe response, opts = request(:post, refund_url, params, opts) initialize_from(response, opts) end + extend Gem::Deprecate + deprecate :refund, "charge.refunds.create", 2016, 07 def capture(params={}, opts={}) response, opts = request(:post, capture_url, params, opts) diff --git a/test/stripe/api_resource_test.rb b/test/stripe/api_resource_test.rb index 81238986..24b43fa9 100644 --- a/test/stripe/api_resource_test.rb +++ b/test/stripe/api_resource_test.rb @@ -206,12 +206,12 @@ module Stripe opts[:headers][:authorization] == 'Bearer local' end.returns(make_response(make_charge)) Stripe.expects(:execute_request).with do |opts| - opts[:url] == "#{Stripe.api_base}/v1/charges/ch_test_charge/refund" && + opts[:url] == "#{Stripe.api_base}/v1/charges/ch_test_charge/refunds" && opts[:headers][:authorization] == 'Bearer local' - end.returns(make_response(make_charge)) + end.returns(make_response(make_refund)) ch = Stripe::Charge.retrieve('ch_test_charge', 'local') - ch.refund + ch.refunds.create end end end diff --git a/test/stripe/application_fee_test.rb b/test/stripe/application_fee_test.rb index 0552c36d..7a57c2f1 100644 --- a/test/stripe/application_fee_test.rb +++ b/test/stripe/application_fee_test.rb @@ -14,18 +14,37 @@ module Stripe should "application fees should be refundable" do fee = Stripe::ApplicationFee.construct_from(make_application_fee) - # first a post to create a refund @mock.expects(:post).once. with("#{Stripe.api_base}/v1/application_fees/#{fee.id}/refunds", nil, ''). returns(make_response(make_application_fee_refund)) - # then a get to refresh the current object - @mock.expects(:get).once. - with("#{Stripe.api_base}/v1/application_fees/#{fee.id}", nil, nil). - returns(make_response({:id => "fee_test_fee", :refunded => true})) + refund = fee.refunds.create + assert refund.is_a?(Stripe::ApplicationFeeRefund) + end - fee.refund - assert fee.refunded + should "warn that #refund is deprecated" do + old_stderr = $stderr + $stderr = StringIO.new + begin + fee = Stripe::ApplicationFee.construct_from(make_application_fee) + + # creates the refund + @mock.expects(:post).once. + with("#{Stripe.api_base}/v1/application_fees/#{fee.id}/refunds", nil, ''). + returns(make_response({})) + + # reloads the application fee to get the field updates + @mock.expects(:get).once. + with("#{Stripe.api_base}/v1/application_fees/#{fee.id}", nil, nil). + returns(make_response({:id => fee.id, :refunded => true})) + + fee.refund + message = "NOTE: Stripe::ApplicationFee#refund is deprecated; use " + + "application_fee.refunds.create instead" + assert_match Regexp.new(message), $stderr.string + ensure + $stderr = old_stderr + end end end end diff --git a/test/stripe/charge_test.rb b/test/stripe/charge_test.rb index 094d1325..ba54668a 100644 --- a/test/stripe/charge_test.rb +++ b/test/stripe/charge_test.rb @@ -12,11 +12,11 @@ module Stripe end should "charges should be refundable" do + c = Stripe::Charge.construct_from(make_charge) @mock.expects(:get).never - @mock.expects(:post).once.returns(make_response({:id => "ch_test_charge", :refunded => true})) - c = Stripe::Charge.new("test_charge") - c.refund - assert c.refunded + @mock.expects(:post).once.returns(make_response(make_refund(:charge => c))) + r = c.refunds.create + assert r.is_a?(Stripe::Refund) end should "charges should not be deletable" do @@ -114,5 +114,22 @@ module Stripe }) assert c.paid end + + should "warn that #refund is deprecated" do + old_stderr = $stderr + $stderr = StringIO.new + begin + charge = Stripe::Charge.construct_from(make_charge) + @mock.expects(:post).once. + with("#{Stripe.api_base}/v1/charges/#{charge.id}/refund", nil, ''). + returns(make_response({:id => charge.id, :refunded => true})) + charge.refund + message = "NOTE: Stripe::Charge#refund is deprecated; use " + + "charge.refunds.create instead" + assert_match Regexp.new(message), $stderr.string + ensure + $stderr = old_stderr + end + end end end