CRM-20676 Add tax_amount to contribution create params so it doesn't get added …

[mattwire]

…to calculation

Overview

Follow on from #11455. This fixes an issue where tax is repeatedly added on contribution edit as it's not passed in the params to contribution create.

[eileenmcnaughton]

@mattwire can you explain the circumstances under which this is still required?

[mattwire]

@eileenmcnaughton

1. Create a membership price set containing a membership that has sales tax enabled.
2. Click on "Add new membership" via backend and record payment.
3. Find the newly created contribution and click "Edit".
4. Now click "Save". You'll see that an additional line item has been created with a newly calculated tax amount. This happens each time you click "Edit" with latest master.

[eileenmcnaughton]

@monishdeb ^^

[monishdeb]

Yup, it fixed the issue, here's the before/after screencast:

Before

After

[monishdeb]

@eileenmcnaughton @mattwire now I am thinking how we could possibly add a unit test to capture this use-case. So here's my thought, I have added a unit-test testLineItemAmountOnSaleTax() here that has a initial steps for this issue to replicate, those are :

1. Create a membership price set containing a membership that has sales tax enabled.
2. Do a backoffice membership.

Now after #11521 is merged, we need to extend the UT to do these steps:
3. Simply save backoffice contribution in Edit mode.
4. Check the line-item and contribution details to ensure there is no additional record / surplus total amount respectively.

What do you think?

[monishdeb]

I have added 'merged ready' tag as the fix is good to merge, but then we need unit-test to avoid any recurrence.

[mattwire]

I just triggered the issue again running the Contribution.repeattransaction API - this issue fixes that too.

[eileenmcnaughton]

sounds like repeattransaction will be the easiest place to do the test - there are a tonne already in api_v3_ContributionTest

[monishdeb]

@mattwire I have extended a existing unit-test for this fix and this is the patch https://gist.github.com/monishdeb/ab5d62fb6799f4e3d99f8e75432a64ca

In addition to that, can you add a unit test with repeattransaction API?

[monishdeb]

I have submitted a PR #11655 that has the UT for it, hence merging.