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

Remove isset from online contribution receipt #22615

Merged
merged 2 commits into from
Jan 26, 2022

Conversation

eileenmcnaughton
Copy link
Contributor

Overview

Remove isset from online contribution receipt

Before

fatal errors in api_v3_ContributionTest with grumpy smarty (due to isset)

After

isset fails fixed....

Technical Details

This removes the isset checks that re failing around
totalTaxAmount (solved the same way as offline
https://github.com/civicrm/civicrm-core/pull/22560/files#diff-fd5668d5492e5f0ec55b7f8a10eccfa4ea9de249e4cf2383a2e0d36dbd92ebe0R134
)

It also removes the isset around contribution.amount_level (switching
to a token) and the enotice on isQuickConfig - this
switches to loading that information & also using a more meaningful variable.
Note that line items should always be assigned so that check is silly.

Comments

still some notices around soft credits - but will leave for a follow up

@civibot civibot bot added the master label Jan 25, 2022
@civibot
Copy link

civibot bot commented Jan 25, 2022

(Standard links)

This removes the isset checks that re failing around
totalTaxAmount (solved the same way as offline
https://github.com/civicrm/civicrm-core/pull/22560/files#diff-fd5668d5492e5f0ec55b7f8a10eccfa4ea9de249e4cf2383a2e0d36dbd92ebe0R134
)

It also removes the isset around contribution.amount_level (switching
to a token) and the enotice on isQuickConfig - this
switches to loading that information & also using a more meaningful variable.
Note that line items should always be assigned so that check is silly.
@eileenmcnaughton
Copy link
Contributor Author

@monishdeb if you are able to squeeze this into your review efforts it would be great.... (sql goes stale a lot)

@monishdeb
Copy link
Member

on it..

@monishdeb
Copy link
Member

r-run passed on a new install and upgrade. There is no breakage on the online contribution receipt mail. Hence merging this PR.

@monishdeb monishdeb merged commit 2892e54 into civicrm:master Jan 26, 2022
@eileenmcnaughton eileenmcnaughton deleted the online branch January 26, 2022 05:12
@eileenmcnaughton
Copy link
Contributor Author

yay - thanks @monishdeb

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