Fix Mailing_Event_BAO_Reply to use token processor #24446
Conversation
|
(Standard links)
|
eileenmcnaughton
force-pushed
the
dom_tokens
branch
4 times, most recently
from
f2e96dc to
10e75b2
Compare
eileenmcnaughton
force-pushed
the
dom_tokens
branch
from
10e75b2 to
a8e9ed2
Compare
|
test this please |
|
|
||
| CRM_Mailing_BAO_Mailing::addMessageIdHeader($params, 'a', $eq->job_id, $queue_id, $eq->hash); | ||
| if (CRM_Core_BAO_MailSettings::includeMessageId()) { | ||
| $params['messageId'] = $params['Message-ID']; | ||
| } | ||
| CRM_Utils_Mail::send($params); | ||
| \CRM_Core_BAO_MessageTemplate::sendTemplate([ | ||
| 'envelope' => $params, |
There was a problem hiding this comment.
@eileenmcnaughton I'm not seeing how envelope key is being used I would have thought that this should be
\CRM_Core_BAO_MessageTemplate::sentTemplate(array_merge($params, [
'tokenContext => ...
]));
There was a problem hiding this comment.
yeah - that function is horrible - really confusing. We really need a sane api wrapper for it
There was a problem hiding this comment.
Well writing the unit test helped me prove that thing works so yeh bloody confusing that is for sure
| $eq = CRM_Core_DAO::executeQuery( | ||
| 'SELECT civicrm_email.email as email, | ||
| civicrm_mailing_event_queue.job_id as job_id, | ||
| civicrm_mailing_event_queuee.hash as hash |
There was a problem hiding this comment.
@eileenmcnaughton missing a comma at the end of this one and also a typo in the table name
There was a problem hiding this comment.
thanks - will try to get to back to this before the rc is cut - otherwise will revert #24235 or do a temp patch on that
|
Jenkins re test this please (this should fail given the mysql issues I commented above about) |
|
I'm gonna close this & come back to it - should be easy now @seamuslee001 has written a test but I'm not really hapy with that awful BAO function & would like to discuss getting working api for it. In the meantime I'll revert the regressive part of #24235 |
|
actually I think those others are OK / non-regressive. So putting this on the back burner for now - but it will be great when we finally do get rid of those old domain token calls & the test is there now... |
Overview
Fix Mailing_Event_BAO_Reply to use token processor
Before
Uses legacy domain token replacement
After
Uses token processor
Technical Details
See #24235
Comments
We really should add an api for this stuff - there is the
sendTemplatefunction but it is really hard to understand how to use it & I think there is another 'light weight' version but we just need an api with sensible params not a bunch of internal functionsI think there was some sort of blocker around permissions - but we actually have a permission we use for the v3 api ....