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.
This commit is contained in:
Brandur 2016-01-14 18:16:56 -08:00
parent 0c14e6ec1f
commit 8a20ab8972
5 changed files with 54 additions and 14 deletions

View File

@ -16,5 +16,7 @@ module Stripe
# from the server # from the server
self.refresh self.refresh
end end
extend Gem::Deprecate
deprecate :refund, "application_fee.refunds.create", 2016, 07
end end
end end

View File

@ -8,6 +8,8 @@ module Stripe
response, opts = request(:post, refund_url, params, opts) response, opts = request(:post, refund_url, params, opts)
initialize_from(response, opts) initialize_from(response, opts)
end end
extend Gem::Deprecate
deprecate :refund, "charge.refunds.create", 2016, 07
def capture(params={}, opts={}) def capture(params={}, opts={})
response, opts = request(:post, capture_url, params, opts) response, opts = request(:post, capture_url, params, opts)

View File

@ -206,12 +206,12 @@ module Stripe
opts[:headers][:authorization] == 'Bearer local' opts[:headers][:authorization] == 'Bearer local'
end.returns(make_response(make_charge)) end.returns(make_response(make_charge))
Stripe.expects(:execute_request).with do |opts| 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' 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 = Stripe::Charge.retrieve('ch_test_charge', 'local')
ch.refund ch.refunds.create
end end
end end
end end

View File

@ -14,18 +14,37 @@ module Stripe
should "application fees should be refundable" do should "application fees should be refundable" do
fee = Stripe::ApplicationFee.construct_from(make_application_fee) fee = Stripe::ApplicationFee.construct_from(make_application_fee)
# first a post to create a refund
@mock.expects(:post).once. @mock.expects(:post).once.
with("#{Stripe.api_base}/v1/application_fees/#{fee.id}/refunds", nil, ''). with("#{Stripe.api_base}/v1/application_fees/#{fee.id}/refunds", nil, '').
returns(make_response(make_application_fee_refund)) returns(make_response(make_application_fee_refund))
# then a get to refresh the current object refund = fee.refunds.create
@mock.expects(:get).once. assert refund.is_a?(Stripe::ApplicationFeeRefund)
with("#{Stripe.api_base}/v1/application_fees/#{fee.id}", nil, nil). end
returns(make_response({:id => "fee_test_fee", :refunded => true}))
fee.refund should "warn that #refund is deprecated" do
assert fee.refunded 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 end
end end

View File

@ -12,11 +12,11 @@ module Stripe
end end
should "charges should be refundable" do should "charges should be refundable" do
c = Stripe::Charge.construct_from(make_charge)
@mock.expects(:get).never @mock.expects(:get).never
@mock.expects(:post).once.returns(make_response({:id => "ch_test_charge", :refunded => true})) @mock.expects(:post).once.returns(make_response(make_refund(:charge => c)))
c = Stripe::Charge.new("test_charge") r = c.refunds.create
c.refund assert r.is_a?(Stripe::Refund)
assert c.refunded
end end
should "charges should not be deletable" do should "charges should not be deletable" do
@ -114,5 +114,22 @@ module Stripe
}) })
assert c.paid assert c.paid
end 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
end end