Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Refund#perform_response and reintroduce @response ivar #3672

Merged
merged 2 commits into from
Jun 23, 2020

Conversation

spaghetticode
Copy link
Member

@spaghetticode spaghetticode commented Jun 19, 2020

Description

This PR reintroduces the instance variable @response in Spree::Refund#perform! that was recently removed in #3641.

There may be stores or extensions that rely on the existence of this ivar, so it was reintroduced
but deprecated in favor of the instance method #perform_response which has a more appropriate name.

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

Checklist:

  • I have followed Pull Request guidelines
  • I have added a detailed description into each commit message
  • I have updated Guides and README accordingly to this change (if needed)
  • I have added tests to cover this change (if needed)
  • I have attached screenshots to this PR for visual changes (if needed)

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
```
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.
Copy link
Member

@kennyadsl kennyadsl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 works for me, thanks!

@kennyadsl kennyadsl added Needs Core Team Review changelog:solidus_core Changes to the solidus_core gem labels Jun 22, 2020
@aldesantis aldesantis merged commit a98fb1b into solidusio:master Jun 23, 2020
@aldesantis aldesantis deleted the spaghetticode/refund-response branch June 23, 2020 15:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog:solidus_core Changes to the solidus_core gem
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants