Fix calculation and assignment of taxAmount on contribution page confirmation

[mattwire]

Overview

When using a contribution page with "Separate Membership Payment" we get an error if both membership and contribution amount have a tax component. It was trying to use $params['tax_amount'] for each contribution which was incorrect and came from the form submission and was the sum of both.

Before

Incorrect usage of tax_amount

After

Calculated tax_amount

Technical Details

Comments

Tested using a contribution page with Separate Membership Payment enabled, recurring set to every 1 day and installments=10. Using "quick config" so amounts configured directly on contribution page configuration and not via pricesets.

[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]

test this please

[eileenmcnaughton]

If this passes r-run then I think it's all good - I took a look at the code & I think it makes sense

[demeritcowboy]

I tried running this but I'm not sure I'm setting it up correctly. It does get rid of a deprecation warning (User deprecated function: passing in incorrect tax amounts is deprecated Caller: CRM_Contribute_Form_Contribution_Confirm::processFormContribution), but doesn't seem to include the tax in the final amount for either item.

My setup:

• Turn on tax/invoicing.
• Add financial accounts:
  • Tax stuff - liability 5%
  • More tax stuff - liability 8%
• Assign the first to Member Dues fintype. Assign the second to Donation fintype.
• Make contribution page
  • Title tab has fintype Donation.
  • Membership tab is the stock General/Student types, with the box checked for "Separate Membership".
  • Amounts tab using quick config with two entries $10 and $20 and the test processor.

On the contribution page first page, it looks fine. On the review page, it shows this, without tax:

General Membership: $100.00
Additional Contribution: $10.00
-------------------------------------------
Total: $110.00

When I confirm, it also displays without the tax on the thankyou page, and it has Warning: Undefined array key "tax_amount" in CRM_Contribute_Form_Contribution_ThankYou->buildQuickForm() (line 114

The receipt does not include tax.

The contributions appear to have the tax recorded, but show that only the principal is paid, so show a balance still owing.

[eileenmcnaughton]

I guess this is cos we have line items 'handy' - but I'd rather grab it from the db contribution than rely on whatever got through the form wrangling to this point - ie cleanest is ALWAYs assign tax_amount & grab the value from the DB - it will be zero if not a tax situation

[mattwire]

I think there are a number of issues with "Separate membership payment" and this PR only scratches the surface.

If you look in confirm.tpl and thankyou.tpl you'll see there is special handling for $is_separate_payment which does not include displaying tax. But the tax is also not assigned properly to the form. In my view we should be getting rid of all that special handling and relying on the "lineitems" display in all cases (as we've already done with contributions in the backend).

@demeritcowboy The problems that you found, are they caused by this PR or pre-existing?

[demeritcowboy]

Ok it looks pre-existing, so I see now this is just about replacing the $params part.

I don't have an opinion on where it should come from - maybe just if there's any hooks that would make a difference and one would handle that better.

[mattwire]

Merging as there are more pre-existing issues to resolve in future PRs and this helps us towards that goal