Worldpay: extract issuer response code

[dsmcclain]

The data is embedded in XML attributes and was therefore not included in the parsed response.

There is no remote test to describe this behavior because Worldpay's Sandbox does not return IssuerResponseCode. An example of the XML response we should expect has been included in the unit tests.

CE-2546

Rubocop:
739 files inspected, no offenses detected

Unit:
5175 tests, 75682 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
100% passed

Remote:
Loaded suite test/remote/gateways/remote_worldpay_test
94 tests, 392 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
100% passed

[drkjc]

@dsmcclain Approved. I looked a little more into required_status_message and wondering if it's necessary to display "A transaction status of 'AUTHORISED' or 'CAPTURED' is required." as a message in a purchase response. The initial work around that method indicates to me that the status message is really only relevant on failed void or refund transactions.

What if your change was an option for the message field instead of exposing it as a GSRF?

[dsmcclain]

@dsmcclain Approved. I looked a little more into required_status_message and wondering if it's necessary to display "A transaction status of 'AUTHORISED' or 'CAPTURED' is required." as a message in a purchase response. The initial work around that method indicates to me that the status message is really only relevant on failed void or refund transactions.

What if your change was an option for the message field instead of exposing it as a GSRF?

@drkjc Nice work looking deeply at the issue. You are right - in this particular case we have a failed purchase transaction and the message returned is "A transaction status of 'AUTHORISED' or 'CAPTURED' is required." which is obviously unhelpful.

I agree with you that it appears required_status_message is custom tailored for refunds & voids.

I think you are suggesting two separate but related things:

1. change the logic of required_status_message to only return the message on voids and refunds. Something like

def required_status_message(raw, success_criteria, action)
  return unless success_criteria.include?(raw[:last_event]) && %w[cancel refund].include?(action)

  "A transaction status of #{success_criteria.collect { |c| "'#{c}'" }.join(' or ')} is required."
end

2. return issuer_response_code['description'] in message_from

I don't really feel confident in either of these changes.

I think change 1 is a great idea, I just don't know the gateway enough. What is raw[:last_event]? I would like to know what sort of filter is in place at the moment before I modify the logic of that filter, in case I end up missing some important cases and returning something confusing or wrong in the message field for those cases. I think this change could definitely improve this gateway integration, but I would need to spend more time researching and testing it.

I'm not sure I agree with change 2. Isn't it possible that there is valuable information in raw[:error], for example, that is different from the information in issuer_response? I wouldn't want to drop one in favor of the other. Also, I have no idea what types of descriptions could appear in issuer_response, so I would rather label it clearly as the description & code provided by the issuer versus adopting it as the "official" message of the transaction.

Thoughts?

[dsmcclain]

Moving ahead with merging in the interests of expedience. Still open to hearing your thoughts about additional changes, @drkjc .

[drkjc]

@dsmcclain Apologies, I missed your reply. I agree with your assessment of both options.

In regards to 1 I created a Jira ticket in our backlog, CE-2620. My research was cursory and I agree a thorough vetting of the current behavior is needed. Your example code is what I was imagining.

For 2 I think you are also correct, the issuer response while sometimes helpful, should not be the official message we display in the response. What would be great is if we could expose the issuer_response on all of our gateways as a GSRF, but that's another project in itself.