Vantiv: Read saleResponse duplicate attribute #4413
Conversation
| doc = Nokogiri::XML(xml).remove_namespaces! | ||
|
|
||
| parsed['duplicate'] = doc.at_xpath('//saleResponse')['duplicate'] == 'true' if kind == :sale |
There was a problem hiding this comment.
One worry I have here is that if doc.at_xpath('//saleResponse') returns nil then calling ['duplicate'] on it will cause ruby to say NoMethodError: undefined method '[]' for nil:NilClass and then we are returning a sad 500 to the customer.
It looks like we make assumptions like this elsewhere in the parse method, and it could very well be that saleResponse is ALWAYS there when kind == :sale, but I would still feel better if we protected against a nil here. Something like:
parsed['duplicate'] = doc.at_xpath('//saleResponse').try(:[], 'duplicate') == 'true' if kind == :sale
| @@ -727,6 +743,25 @@ def successful_purchase_response | |||
| ) | |||
| end | |||
|
|
|||
| def duplicate_purchase_response | |||
|
I sure wish there was a better way to check for xml validation errors than |
Agree 100%. The |
There was a problem hiding this comment.
When I run bundle exec rake test:local I get a linting error now.
Once that is fixed, rebase with master and squash down to one commit. Then add an entry to the changelog (at the bottom of the == HEAD section), and at that point you are good to merge.
I am approving so that you don't have to tag me for re-review on a linting fix, but don't hesitate to reach out if you have questions about the merge process 🙂 Please don't forget to fix the linting error before merging
For posterity: I am getting 13 failing remote tests on this branch, same as master
mashton
force-pushed
the
CE-2506_vantiv_gsrf_duplicate
branch
2 times, most recently
from
407ded0 to
abf7cfd
Compare
The LitleGateway parse method reads elements and their text values, but does not read any of the elements attributes. Here we read one very specific attribute (probably not worth generalizing just yet). Here we're interested in a litle saleResponse that may or may not have the attribute "duplicate='true'". All AM litle saleResposnes will include duplicate: true/false. Note on testing: Remote test for this feature is missing. It looks like the testvantivcnp api does not return the duplicate attribute the same way the production api does. Additionally, something seems to have changed in the testvantivcnp api such that existing remote tests fail. PR coming soon with fixes for existing tests. CE-2506 Local: 5175 tests, 75684 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications 100% passed Remote: 13 failed tests, same as master. See note above. Linting: 739 files inspected, no offenses detected
mashton
force-pushed
the
CE-2506_vantiv_gsrf_duplicate
branch
from
abf7cfd to
35c00e4
Compare
The LitleGateway parse method reads elements and their text values, but does not read any of the elements attributes. Here we read one very specific attribute (probably not worth generalizing just yet). Here we're interested in a Litle saleResponse that may or may not have the attribute
duplicate='true'. All Litle saleResponses will include duplicate: true/false.Note on testing: Remote test for this feature is missing. It looks like the
testvantivcnpapi does not return the duplicate attribute the same way the production api does. Additionally, something seems to have changed in thetestvantivcnpapi such that existing remote tests fail. PR coming soon with fixes for existing tests.CE-2506
Local:
5175 tests, 75684 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
100% passed
Remote:
See note.
Linting:
739 files inspected, no offenses detected