-
-
Notifications
You must be signed in to change notification settings - Fork 825
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
Payment.sendconfirmation api - add further tpl variables. #13610
Conversation
(Standard links)
|
@eileenmcnaughton now when #13609 is merged, can you please rebase this PR? |
1adc7c3
to
565920d
Compare
@monishdeb done |
'isRefund' => $entities['payment']['total_amount'] < 0, | ||
'isAmountzero' => $entities['payment']['total_amount'] === 0, | ||
'refundAmount' => ($entities['payment']['total_amount'] < 0 ? $entities['payment']['total_amount'] : NULL), | ||
'paymentsComplete' => ($entities['payment']['balance'] == 0), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
:-)
'billingName', | ||
'address', | ||
'credit_card_type', | ||
'credit_card_number', | ||
'credit_card_exp_date', | ||
'eventEmail', | ||
'$event.participant_role', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@eileenmcnaughton should we remove these two event parameters (as an indicator)? I didn't found any occurrence in form where its been assigned
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@monishdeb so that was my list of things to add to meet the tpl's requirements but when I reviewed the tpl I decided they were derived parameters (with the tpl) not assigned parameters
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok I see
Tested the patch, working fine. Also the added UT successfully asserts the presence of new template variables. Merging now. |
Overview
Payment.sendconfirmation is an api added in 5.12 - this is part of that. The api will replace the form email confirmation logic once mature
Before
Less complete
After
More complete
Technical Details
Incorporates & extends #13609
Comments
There are a couple of parameters assigned to the template i the form - but they are not used
@jitendrapurohit I think once this is merged we could replace emailReceipt with an api call to Payment.sendconfirmation. There is nothing else in that fn that is done AND used in the tpl from what I can see