Add test for mixed Order data

[eileenmcnaughton]

Overview

This test is to try to capture & replicate an issue @christianwach is having with order api on taxes with more than one row - @christianwach I feel like this should be failing from your experience but it didn't locally

Before

Less tests

After

More tests

Technical Details

This didn't replicate the issue but it's useful as a test I think & I can build on it

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).

[christianwach]

@eileenmcnaughton I've added a Gist with complete details of the mismatch:

https://gist.github.com/christianwach/ac8e8460c7f488a9a31f30a2c74eefc6

There isn't a Taxable Financial Type in the CiviCRM Sample Data by default but I've added one (ID: 5) as per instructions from @kcristiano

[christianwach]

@eileenmcnaughton A bit more info... so... turns out my Order API call wasn't as clean as I thought it was. Elsewhere, just after the Order.create, there was an attempt to update the Contribution with a contribution_note. Seems this triggers the recalculation.

I can now confirm that with a Non-taxable Financial Type applied to the Contribution and mixed Line Item Financial Types all seems to be well.

I'll keep digging with other combinations, but it seems updating a Contribution with a contribution_note is big no-no.

[JoeMurray]

@monishdeb could you work to ensure that a contribution update does not result in inappropriate recalculation of sales taxes, which I imagine involves calculating it differently on subsequent saves.

[christianwach]

Recalculation seems to happen any time a Contribution is updated regardless of whether the Financial Type is taxable or not e.g. a trivial update to the "Source":

$result = civicrm_api3('Contribution', 'create', [
  'financial_type_id' => 5, // Happens to be a Taxable Financial Type but need not be.
  'contact_id' => 206,
  'id' => 103,
  'source' => "Something",
]);

All taxes will be recalculated and the errors in the Gist can be observed. I've updated it to reflect the situation better:

https://gist.github.com/christianwach/ac8e8460c7f488a9a31f30a2c74eefc6

[eileenmcnaughton]

@christianwach thanks for narrowing it down - that helps

[eileenmcnaughton]

OK - I can replicate this now - @seamuslee001 we might as well merge this as an extra test & I can build on it to fix Christian's actual problem

[christianwach]

I can build on it to fix Christian's actual problem

@eileenmcnaughton That would be wonderful 🏅

[eileenmcnaughton]

@christianwach I'm working on it over here - #20357 at this stage my fix assumes a lack of weird data being passed in which I am not sure is true :-)