CRM-16460 PayPal Standard needs two digits of cents

Submitted PR at https://github.com/civicrm/civicrm-core/pull/5767

I think this has caused a bug - we are doing formatting in a deep BAO function as opposed to a form layer function & when I run tests locally I see 

testEnterNegativeContribution()

fails because total_amount of -5 has been transformed to the invalid '(5.00)'

probably hitting this line 

if (is_numeric($amount) and function_exists('money_format')) {
$amount = money_format($valueFormat, $amount);
}

Eileen, thanks for your comment here and in the code. I've been wrestling with this for a bit because it seems to be causing intermittent issues on some of our sites, and I can confirm that at least in my testing, during the money format call, there is no locale defined, and so the money_format function will sometimes generate round brackets instead of a negative sign. I considered trying to fix the format function when called with the "only number" parameter set to true, but it'd be difficult to figure out all the places that might be used that could be broken if I changed it. As far as I can tell, the "only number" option seems to just be a way to remove the currency symbol, so that may be called with or without any locale-expectations.

So I think the way to fix this is either:

1. by replacing the patch from this issue with one that is simpler and locale-agnostic (i.e. a simple format that ensures 2 digits).
2. revert the patch and let paypal and/or other payment processors handle the formatting if still necessary.

My reading of the original issue is that it was solving a problem that could better be handled in the payment processor code, since each processor has different requirements of what format the money coming in requires. Actually, Karin thinks that Paypal will now handle amounts with arbitrary numbers of digits.

Additionally, rounding to two digits at this stage is also a potential problem if taxes are going to be applied later on in the code, as per some upcoming work from Matt.

Conclusion: I'm leaning strongly towards option 2., feedback encouraged. I'll create a new issue if anyone bites.

Alan Dixon When I look at Omnipay code I see

```

// Contribution page in 4.4 passes amount - not sure which passes total_amount if any.
if (isset($params['total_amount'])) {
   $amount = (float) CRM_Utils_Rule::cleanMoney($params['total_amount']);
}
else {
   $amount = (float) CRM_Utils_Rule::cleanMoney($params['amount']);
}

```

This suggests to me that in the past we have not been even fully consistent about setting our parameters as what amount to use - perhaps it has become more reliable since that code was written of course!

I also lean towards reverting because I think that is the wrong place in the code to do any formatting at all. It may also make sense to add some formatting helpers to the CRM_Core_Payment such as 

getAmountInCents() and getFormattedAmount(). (Omnipay offers the following 

getAmountInteger(), getAmount() - the latter is formatted to the number of decimal places & separator for the currency), the former maps to the concept of getAmountInCents() - without the locale-specific term). If we grandfather in some helpers then it probably is easier over time to deal with any changes.

Alan Dixon Eileen McNaughton So with the PR on CRM-20772 that hasn't been merged yet (the bit that allows the UI to handle the extra decimals) I've refactored CRM_Utils_Money to create a set of formatting helpers (replacing CRM_Utils_Money::format).

It makes sense to me that the formatters are in CRM_Utils_Money rather than CRM_Core_Payment, but I'm not too concerned about the function names - did not realise Omnipay already had some so that's a good place to look.  Eileen McNaughton Alan Dixon If you could give your thoughts on those changes, we could break the formatting functions out into a separate PR and get that merged first, so they are then available for core to use.

As for the specific line that Alan Dixon is having trouble with, you'll see here (https://github.com/civicrm/civicrm-core/pull/10641/files#diff-9bb0aa4010a43713521df361cca72fe6R841) that it also stops me doing the extra decimal places and it needs to change.  KarinG has also had problems with the negative brackets I think, which only happens with certain combinations of locales etc (Canadian?) and stuff breaks!

Thanks for your comments.

1. Eileen McNaughton, yes to comment about not doing formatting here. Premature rounding and the round-bracket notation for negatives are just two of the issues that have arisen from premature formatting, and I can imagine there could be others. In an ideal world, it seems to me that any money amount that gets passed around needs at least a currency and never any formatting. Some kind of more elaborate "money" object seems like a worthwhile direction, where you could keep track of other potential meta-properties that could be of value (e.g. tax-status?). Not an original idea of course: http://moneyphp.org/en/latest/index.html

2. Matthew Wire so it sounds like you've already got a good solution for the immediate problem, and more, so thanks. Since money is super sensitive, we'll have to test it pretty thoroughly and encourage all the payment processor maintainers to do testing as well.

I'll stop dragging this issue around and try being constructive over on Matthew's.