When period used as thousands separator, the total is wrong and minimum amount alert triggers

[mathetos]

Issue Overview

When period used as thousands separator, the total is wrong and minimum amount alert triggers

Current Behavior

Some countries use a period as the thousands separator. Currently, when that is set it makes the total amount incorrect and triggers the minimum donation alert falsely.

Expected Behavior

Periods should be supported as thousands separators correctly.

Steps to Reproduce (for bugs)

1. Go to "Donations > Settings > General > Currency" and set the thousands separator to a period (.). The decimal seperator and the number of decimals won't matter.
2. On your form, set a minimum $20 donation.
3. Test a donation by donating over $1,000 (in order to trigger a thousand separator). The totla will be reflected as $1 instead of $1,000. You should then see a message like this:
   

Customers reporting this issue:

• https://secure.helpscout.net/conversation/391650991/8704/?folderId=672201
• https://secure.helpscout.net/conversation/398883082/8972/?folderId=672201

[DevinWalker]

It looks like give_sanitize_amount() is not properly sanitizing when the donation amount is more than $1000 and decimals are used for the thousands separator.

[mathetos]

As a reference, here's a list of currencies and their formatting:

http://www.thefinancials.com/Default.aspx?SubSectionID=curformat

[ravinderk]

@DevinWalker @mathetos @kevinwhoffman @mehul0810 Currently we are using number_format function for formatting number. It does not respect currency, that reason behind we do not have correct number formatting for INR currency. Same thing happens in Woocommerce (check screenshot).

Check Indian number system

We currently have the options in core settings to set decimal, thousand separator, number of decimal for number formatting. I think it will be useful if number formatting becomes currency aware.

I found a library which does it. Let me know what do you guys think about this.
https://github.com/gerardojbaez/money

[ravinderk]

@DevinWalker @mathetos @kevinwhoffman @mehul0810 I reviewed my solution and I think we do not need this. I can achieve this just by simple code because except India most of the country has similar number system means only separators changes but logic remain same.

[ravinderk]

@mathetos @DevinWalker This problem is a little bit complicated.

Problem
As @mathetos said some countries have decimal separator as thousand separators, we can fix this, but then the problem started occurring with database values because we are storing already sanitized values in the database.
for example, if we store 1 with 3 decimal precision then 1.000 will store in the database. If we format this value then it's value will become 1000.
The reason behind this is we are imposing sanitization to each value even for database value which is already sanitized.
for ref:
https://github.com/WordImpress/Give/blob/master/includes/formatting.php#L145
https://github.com/WordImpress/Give/blob/master/includes/formatting.php#L210

Solution
To solve this problem we can take out give_sanitize_amount function from amount formatting function and let start use them where they need. We can handle this change in core but this will also affect all add-ons, but it will help us to solve future problems. Instead of removing give_sanitize_amount function we can add the third param to formatting function $is_sanitize by default it is set to true and we only need to set it to false for already sanitizing value like database value. We have to cross check addons in this condition also but this will save us from updating lots of code and I think it is smart way also.
And I also want to suggest you store 6 decimal precision value in the database, it should not depend upon decimal precision setting.