Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[REF] Fix tax_amount to be consistent & load from the templateContribution #19274

Merged
merged 1 commit into from
Dec 24, 2020

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented Dec 24, 2020

Overview

[REF] Fix tax_amount to be consistent & load from the templateContribution

This leverages the work done in https://github.com/civicrm/civicrm-core/pull/15517/files (the test in particular)
but alters it to be more consistent. The original fix relied on the tax_amount on the contribution object
passed in - this fixes it to get the amount from the templateContribution (which is more consistent
now than when the patch was likely written

Before

tax_amount taken from the contribution object that is passed in which is likely to be the same as the template contribution but might not be - if there were a difference template contribution would be correct

After

template contribution value used

Technical Details

Comments

…ution

This leverages the work done in https://github.com/civicrm/civicrm-core/pull/15517/files (the test in particular)
but alters it to be more consistent. The original fix relied on the tax_amount on the contribution object
passed in - this fixes it to get the amount from the templateContribution (which is more consistent
now than when the patch was likely written
@civibot
Copy link

civibot bot commented Dec 24, 2020

(Standard links)

@civibot civibot bot added the master label Dec 24, 2020
@eileenmcnaughton eileenmcnaughton changed the title [REF] Fix tax_amount to be consistent & load from the templateContrib… [REF] Fix tax_amount to be consistent & load from the templateContribution Dec 24, 2020
@mattwire
Copy link
Contributor

I suspect we might need to revisit tax_amount at some point in the future as it could be different if amounts change etc. But we'd need to look into calculating it again / updating the template contribution so I'm happy with this change as-is.

@mattwire mattwire merged commit e4e20a1 into civicrm:master Dec 24, 2020
@eileenmcnaughton eileenmcnaughton deleted the tax branch December 27, 2020 04:48
@eileenmcnaughton
Copy link
Contributor Author

@mattwire yeah - I suspect that an override amount (only available for 1 line item contributions) would work incorrectly before and after this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants