dev/core#2493 Default to not cleaning money for order.create api

[eileenmcnaughton]

Overview

dev/core#2493 Default to not cleaning money for order.create api

Before

Per https://lab.civicrm.org/dev/core/-/issues/2493#note_57387 there seem to be a lot of issues now with money being truncated which is what the clean function does if called twice on the same string. By default Order v3 create api gets the same default for skipCleanMoney (FALSE) as Contribution.create api.

However, it seems implicated in some of the cases and also one that can be more easily changed since Order api always involves some wrangling (and is less used) and the most broadly used cases appear to be implicated

After

Order v3 api defaults to skipCleanMoney

Technical Details

@kcristiano does this fix the case you can replicate? @agileware-justin may fix some of the case/s you hit

Comments

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[eileenmcnaughton]

I've moved the PR to master since I don't think we will get confirmation in time to merge this to 5.36.0 - will consider which branch once we have more feedback

[agileware-fj]

Just a couple of notes on this one from my testing. The use case which I hoped this would fix involved values that had been passed through CRM_Utils_Money::format once already.

• The total_amount does not appear to already being cleaned twice in the first place - worked even without this change.
• This had no effect on the line items, which I would assume is the more common use case for Order API.

In both cases if the amount hadn't already been formatted, everything worked fine both before and after the change.

Adding a skipCleanMoney parameter explicitly to the Order API can't be a bad thing though, right?

[agileware-justin]

@eileenmcnaughton thanks for the PR

[eileenmcnaughton]

@agileware-fj @agileware-justin -ok so not a strong case for this being helpful - hopefully we can gather a bit more information & then decide if it's part of the fix or not then

[eileenmcnaughton]

@seamuslee001 what did you decide on this?

[seamuslee001]

I thought about this but I think on the balance of probabilities this is better in than out

[eileenmcnaughton]

Thanks @seamuslee001 - that was my conclusion too