Add test for selvsvctransfer, remove use of $contact from template
#21855
Conversation
|
(Standard links)
|
23f4f02 to
d171f73
Compare
b3a6e6b to
1ab17d9
Compare
|
rebased over #21925 |
|
@demeritcowboy if you have a chance to look at this one it's one more call to the legacy hook token function removed |
|
@eileenmcnaughton I have merged #21925 and I am expecting the current should contain the upgrade and unit-test changes, instead, it includes the patch already merged in master. Or does the PR need rebase? |
1ab17d9 to
6c43827
Compare
|
@monishdeb that PR was only a minor change - I've rebased over it though. There was an earlier change that some comments abover related to |
| $mailType | ||
| ) { | ||
| //send emails. | ||
| $mailSent = FALSE; | ||
|
|
||
| if (!$contactDetails) { | ||
| $contactDetails = civicrm_api3('Contact', 'getsingle', [ |
There was a problem hiding this comment.
it worries me whenever I see getsingle API call without safeguarding mandatory parameter that might lead to a fatal error. Can you put !empty($participantValues['contact_id']) in IF condition ?
There was a problem hiding this comment.
@monishdeb I feel like it would be an error if it were not set - so it's better to get an error
|
Tested on 5.44.x upgrade, looks good. Reviewed patch, didn't encounter any breakage. |
|
Thanks @monishdeb - we are getting close to the goal of only calling |
Overview
Add test for selvsvctransfer, remove from template
Note this incorporates #21925
Before
No test, shipped template refers to assigned smarty variable, uses legacy method to get contact details
After
Test, limited replacement of smarty variable
Uses the api to get contact details (to keep it simply I stuck with v3 api)
Technical Details
I realised that I can't go further in this PR in terms of testing for domain tokens as I need the test utility in #21847 merged before I can leverage it. Interms of regenerating the sql I also need that merged as this will just conflict.
Comments