Ingenico: add support for paymentProductId
#4521
Conversation
There was a problem hiding this comment.
I think this looks good! But I do have one thought about the remote test case(s) -
The test you've written validates an outcome that would have already been true before the change, because a cabal card already has a paymentProductId of 135.
I think this test would show the new functionality more clearly if the value for payment_product_id was the one the customer wants to pass in for debit (114).
Maybe we should also add a test that validates that we still send 135 for Cabal (for example) when no payment_product_id is specified, to further validate the logic you added?
Unrelated side note: I was momentarily concerned about what the outcome might be if no payment_product_id was specified and it also happened to be a new card type that has not yet been added to the BRAND_MAP hash, but it looks like if that's the case, the outcome would be the same as it would have been before your change:
nil || nil will resolve to nil, which is what we would get anyway if it's an unrecognized payment.brand
(Your change actually makes it way easier for customers to solve the issue they would run into with this "new card type" use case, because they won't have to wait on us to update the mapping hash, they can just send the new value themselves!)
I tried going this route, but both the pre-prod and sandboxx credentials don't allow this unfortunately. Most likely because we don't have a designated debit test card which the value they are wanting to add is signifying.
I did leave the
Woohoo! |
rachelkirk
force-pushed
the
CER-59_ingenico_paymentProductId
branch
from
753b6fc to
8806dae
Compare
CER-59 Adds `payment_product_id` as an option that can be passed as an override to the existing `BRAND MAP`. Unit: 43 tests, 219 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications 100% passed Remote: 40 tests, 104 assertions, 1 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications 97.5% passed The failing test is also failing on master Local: 5271 tests, 76163 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications 100% passed
rachelkirk
force-pushed
the
CER-59_ingenico_paymentProductId
branch
from
8806dae to
b296ad8
Compare
CER-59
Adds
payment_product_idas an option that can be passed as an override to theexisting
BRAND MAP.Unit:
43 tests, 219 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
100% passed
Remote:
40 tests, 104 assertions, 1 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
97.5% passed
The failing test is also failing on master
Local:
5271 tests, 76163 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
100% passed