Add test for api money, fix net_amount calc #11801
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Overview
Go back to cleaning api money input -in the api layer
Before
money values 5.000,77 & 5,000.77 not handled & they are converted to '5' by mysql insert.
After
These values are handled. However, risk is added that double cleaning could mess them up.
Technical Details
This is problematic because if values are cleaned (converted to 5000.77) twice then they will be incorrect. The code that used to do a reasonable 'guess' at whether it was already cleaned stopped working once we messed with precision (a few point releases ago).
The fix is that all parameters should be cleaned as close to input as possible. When I last analysed the api I thought it was only accepting clean values but I now think I was wrong. This puts us back to the api doing cleaning, but at risk that it could double clean if skipCleanMoney not used. I can't quite figure out how to find the light at the end of this now because by putting this back we put back the potential for on ongoing whack-a-mole, but not sure we can justify NOT putting it back by default.
One BIG thing that has happened since this whole issue originally blew up is the addition of a LOT of unit tests - grep for $this->setCurrencySeparators($thousandSeparator); - so that might give some confidence (maybe) the form layer is not cleaning the values & then passing to the api which re-cleans / breaks them
Comments