Braintree Blue: Refactor and add payment details to failed transactions

[yunnydang]

This refactors a slight portion of the transaction hash regarding the payment details object that is being returned depending on which payment method is used. Also returns the these objects on failing transactions and added small fixes to previously failing tests. Also fixes some rubocop related issues.

Local:
5816 tests, 78913 assertions, 2 failures, 30 errors, 0 pendings, 0 omissions, 0 notifications
99.4498% passed

Unit:
103 tests, 218 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
100% passed

Remote:
120 tests, 634 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
100% passed

[jcreiff]

This looks good to me, although I left one comment for your consideration.

The remote and unit tests all run successfully for me, so it appears this minor refactor should be compatible with the way the gateway has been working to this point. We'll just be getting more data surfaced on failed transactions.

Out of curiosity, what did you learn in your testing that suggested the reason for the narrow scope of the original return block?

[yunnydang]

This looks good to me, although I left one comment for your consideration.

The remote and unit tests all run successfully for me, so it appears this minor refactor should be compatible with the way the gateway has been working to this point. We'll just be getting more data surfaced on failed transactions.

Out of curiosity, what did you learn in your testing that suggested the reason for the narrow scope of the original return block?

Thanks for the review Joe! To answer your question regarding narrowing the scope of the return block:

Essentially it was written that way so that none of the rest of the transaction hash can be built if there are failure transactions. Braintree returns a slightly different structured response on said transactions and it results in noMethodErrors on actions such as credit, void, and refund I believe when I run the entire test suite

[yunnydang]

hey @jcreiff I know you approved, but I just wanted some quick re-review on this as I tweaked a little bit of things rubocop related. Thanks!

[jcreiff]

Thanks for cleaning up the rubocop offenses - looks good!

[yunnydang]

heya @jcreiff I added some more stuff to this as I remembered that we're also expecting the payment_instrument_type to be returned so it can dig into the correct object depending on payment methods. This way the response object can be actually surfaced, thanks!

also reran all the tests and its working as intended

[jcreiff]

🚢