Credorax: Enable selecting a processor

[leila-alderman]

Adds gateway specific fields to the Credorax gateway to enable the user
to select a different processor, including adding a unit test and a
(failing) remote test.

Gateway specific fields added:

• R1: processor name
• R2: processor merchant ID

CE-35

Unit:
23 tests, 106 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions,
0 notifications
100% passed

Remote:
26 tests, 37 assertions, 21 failures, 0 errors, 0 pendings, 0 omissions,
0 notifications
19.2308% passed

The Credorax gateway integration was recently updated, and now many of
the remote tests that were previously passing are failing. It is
currently unknown why these tests are all failing. We have reached
out to Credorax to inquire about this new behavior.

[jasonxp]

One general comment about the test coverage. The implementation covers purchase, authorize, capture, void, refund, and credit, but the tests appear to only cover purchase. As a litmus test for coverage, I would expect test failures if we commented out any of the add_processor calls to intentionally break support for one of the other operations. If that doesn't happen, it would imply that there is no test coverage for the other methods to guard against regressions. The common code for the tests can be factored out into a helper test method, similar to what we did in the CyberSource tests that you reviewed recently @leila-alderman.

[leila-alderman]

Similar to my comment above, I was trying to follow the convention in this test file, which only tests purchase transactions in the tests for the gateway specific fields.

I have the same questions about this recommendation as I wrote above. Perhaps this is a higher level discussion that would be beneficial to have outside of GitHub.

[jasonxp]

Looks good. 👍 for filling in the test coverage gaps. It sounds like Credorax has fixed some issues on their end that should allow the tests to pass. pending a test run and updating the commit message to indicate the final state of test results.

[leila-alderman]

@jasonxp Despite the email from Credorax saying that the issues were fixed, I'm still getting the same remote test output that I did originally (19% of tests passing and many "Internal server error. Please contact Credorax support." error messages). Is it okay to ship these changes with so many test failures or should I wait to get the remote tests to pass?

[jasonxp]

@leila-alderman I think we can go ahead and ship the change since it should be very low risk. There shouldn't be any impact unless the processor or processor_merchant_id option is specified. Additionally, it would be helpful to note the issue with the failing tests in their file, to save time for whoever works on this gateway next. I also think it would be worthwhile to reply to Credorax's latest e-mail letting them know of the problem, even though we're going ahead with shipping it. We can give the Spreedly customer who is planning to use the new fields a heads up that we'll need feedback from them as to whether it is working properly.

[therufs]

🎉 :partyparrot:

[leila-alderman]

I'm re-requesting review for this one now that the tests are working. (!!!!!!) With the 3DS2 updates for Credorax, the implementation is slightly different.

The fixture for credorax_with_processor has been added to the Spreedly 1Password note for Credorax.

[jasonxp]

It looks like with the previous implementation, no r1 parameter would have been set for a non-3DS transaction, whereas now r1 will be set to CREDORAX for a non-3DS transaction that does not have the new processor option. This seems like it would be okay, but I'm not specifically familiar with the option. Is that difference fine?

[leila-alderman]

My understanding is that r1 needs to be sent through for all transactions, not just 3DS transactions, whenever selecting a processor is enabled in the gateway settings.

What I don't know (and sort of assume is okay) is the effect of sending r1 when selecting a processor is not enabled in the gateway settings.

[jasonxp]

👍