core/issues/72 fix payflow bug for amount

[yashodha]

Overview

PayflowPro throws error due to more than 4 decimal places

Payment Processor Error message: 9013: Error - from payment processor: [4 Invalid amount]
9013: Error - from payment processor: [4 Invalid amount]

This was introduced in
#11016

where database changes supported decimal upto 9 places.

Before

After

[mattwire]

@yashodha Confirming that this is the correct fix. Do any of the other core processors need the same fix?

[mattwire]

Every payment processor should be validating the "amount" is in the right format to be sent upstream as different processor API have different requirements. The "standard" validator is CRM_Core_Payment::getAmount() which @eileenmcnaughton created a few versions back and indicated that payment processors should be updated to use it. This PR does that for the payflow processor in core. Checking other processors is out of scope for this PR, but any chance of creating a basic unit test for this? Could use one of the existing CRM/Core/Payment tests as a template.

[eileenmcnaughton]

ouch - this wasn't linked from the gitlab so I assumed no work had been done & did #12042 - just comparing now

[eileenmcnaughton]

OK so thoughts

1. we should aim to get this merged to the 5.1 rc (if we can feel confident quickly enough)
2. the key difference between mine Fix paypal to round amount to 2 decimal places before passing to gateway #12042 & this one concerns handling of the thousand separator. My reading is that the getAmount function will, at least on some systems, use a comma for the decimal separator. (this seems to depend on locale). Further to that I think it would be bad if it did - per links like https://stackoverflow.com/questions/36818316/paypal-error-currency-amount-must-be-non-negative-number
3. We should test on a system with the comma decimal separator - although I'm not sure that will get all the variants in when the comma might burst through - I think if we have doubts my approach is probably safer

Re unit tests the way forwards here would be to get the guzzle mock handler in play so we can start to test what is sent to paypal. I won't delay merging this until we have a path on that though.

[mattwire]

@eileenmcnaughton Yep, sorry you are right. See my comments on #12042

[eileenmcnaughton]

I've been confused all this time! I thought we were dealing with Paypal here - but it's Payflow Pro - that's a pretty obscure gateway & one that has been deprecated by Paypal for a very long time so I don't believe anyone else will be able to test this (or likely is using this).

I'm still a little worried about the euro-thousands separator issue but my risk assessment has changed now I realise we are just dealing with PayFlow - if there are any issues on Euro sites at least with this it will hard-fail quickly, rather than occassionally fail. Probably would have been better in 5.1 but needed to have had a lot more discussion over the last 7 days for that to make sense - ie. @yashodha has not been around since submitting this - so merging to master now