From 90934a91a6c0934a628dce1414b251084420d01b Mon Sep 17 00:00:00 2001 From: andrea longhi Date: Fri, 19 Jun 2020 14:31:58 +0200 Subject: [PATCH 1/2] Add Refund#perform_response There are instances when we need to be able to access the payment gateway response received during the refund perform process, notably in core/app/models/spree/payment/cancellation.rb:31 where it assumes that #try_void returns the gateway response: ``` if response = payment.payment_method.try_void(payment) payment.send(:handle_void_response, response) else #... end ``` the next line calls core/app/models/spree/payment/processing.rb:191 which makes super clear that we're expecting an object that responds to #success? and #authorization, ie. the gateway response. ``` def handle_void_response(response) record_response(response) if response.success? self.response_code = response.authorization void else gateway_error(response) end end ``` --- core/app/models/spree/refund.rb | 8 +++++--- core/spec/models/spree/refund_spec.rb | 7 +++++++ 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/core/app/models/spree/refund.rb b/core/app/models/spree/refund.rb index a92898ccdcb..2ebec2b74bd 100644 --- a/core/app/models/spree/refund.rb +++ b/core/app/models/spree/refund.rb @@ -14,7 +14,9 @@ class Refund < Spree::Base validate :amount_is_less_than_or_equal_to_allowed_amount, on: :create + attr_reader :perform_response attr_accessor :perform_after_create + after_create :set_perform_after_create_default after_create :perform! after_create :clear_perform_after_create @@ -48,10 +50,10 @@ def perform! credit_cents = money.cents - response = process!(credit_cents) - log_entries.build(details: response.to_yaml) + @perform_response = process!(credit_cents) - update!(transaction_id: response.authorization) + log_entries.build(details: perform_response.to_yaml) + update!(transaction_id: perform_response.authorization) update_order end diff --git a/core/spec/models/spree/refund_spec.rb b/core/spec/models/spree/refund_spec.rb index 77a0f46fc5f..49adef6b5a1 100644 --- a/core/spec/models/spree/refund_spec.rb +++ b/core/spec/models/spree/refund_spec.rb @@ -99,8 +99,15 @@ context "with perform_after_create: true" do let(:perform_after_create) { true } + it "sets #perform_response with the gateway response from the payment provider" do + expect(Spree::Deprecation).to receive(:warn) + + expect(refund.perform_response).to eq gateway_response + end + it "does nothing, perform! already happened after create" do expect(Spree::Deprecation).to receive(:warn) + refund expect(refund.transaction_id).not_to be_nil From 9a92987c2e9436f5dc50072505efdf8f5e2f4002 Mon Sep 17 00:00:00 2001 From: andrea longhi Date: Fri, 19 Jun 2020 14:35:50 +0200 Subject: [PATCH 2/2] Reintroduce Spree::Refund @response ivar This instance variable was removed recently but needs to be reintroduced as there may be some extenal code that relies on its existence. The instance variable usage is now deprecated, consumers should actually now use the more properly named method Spree::Refund#perform_response. --- core/app/models/spree/refund.rb | 8 ++++++++ core/spec/models/spree/refund_spec.rb | 8 +++++++- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/core/app/models/spree/refund.rb b/core/app/models/spree/refund.rb index 2ebec2b74bd..5ae7be4abfd 100644 --- a/core/app/models/spree/refund.rb +++ b/core/app/models/spree/refund.rb @@ -52,6 +52,14 @@ def perform! @perform_response = process!(credit_cents) + @response = Spree::DeprecatedInstanceVariableProxy.new( + self, + :perform_response, + :@response, + Spree::Deprecation, + "Please, do not use Spree::Refund @response anymore, use Spree::Refund#perform_response" + ) + log_entries.build(details: perform_response.to_yaml) update!(transaction_id: perform_response.authorization) update_order diff --git a/core/spec/models/spree/refund_spec.rb b/core/spec/models/spree/refund_spec.rb index 49adef6b5a1..f4d2de600f6 100644 --- a/core/spec/models/spree/refund_spec.rb +++ b/core/spec/models/spree/refund_spec.rb @@ -99,6 +99,13 @@ context "with perform_after_create: true" do let(:perform_after_create) { true } + it "deprecates usage of the instance variable @response" do + expect(Spree::Deprecation).to receive(:warn).twice + + response = refund.instance_variable_get("@response") + response.to_s + end + it "sets #perform_response with the gateway response from the payment provider" do expect(Spree::Deprecation).to receive(:warn) @@ -107,7 +114,6 @@ it "does nothing, perform! already happened after create" do expect(Spree::Deprecation).to receive(:warn) - refund expect(refund.transaction_id).not_to be_nil