CRM-18543 Deleting email address or phone of CiviMail recipient hangs job #8355
Conversation
|
Test failure is unrelated. |
| if (empty($recipients->email_id) && empty($recipients->phone_id)) { | ||
| continue; | ||
| } | ||
|
|
||
| if ($recipients->phone_id) { |
There was a problem hiding this comment.
I'm more concerned about these following lines, which set strings of 'null' D:
There was a problem hiding this comment.
The problem for me is not so much the 'null' strings (which are needed as part of the INSERT statement), but the disconnect between the calling code (which says that a mailing job is either 'sms' or 'email') and this code which allows individual recipients to be treated as either 'sms' or 'email'.
Eg, what if (for an SMS job) all but one recipient have a phone_id, and that one has an email? My thought is that there's no point creating a recipient that won't be used, or worse, is mistakenly used.
|
Jenkins retest this please |
|
Hi Ken, as Release Manager this month, I'm trying to recruit people to help pare down the backlog of almost 100 PRs, some going back to last summer. I'm wondering if you would be able to help QA another PR if I got someone to QA this PR and your other one #8432 ? |
|
Joe, I'm willing to help. This week I have 3 deadlines, and event and a cold, so not able to make much traction until next week. |
|
Great! Thanks for your time next week. Please see civicrm/release-management#3, and then fill in a bit about yourself/interests at https://docs.google.com/spreadsheets/d/1fmHFOZ83ectSPCMWvXwCeJgrw4KpYi8JEV2ZDkd6XDU/edit#gid=924271888 |
|
I did a quick QA on this and we're using the patch, so I vote to merge. |
|
@JKingsnorth the vote to merge was unfortunately a day past the deadline to merge for 4.7.9, I believe. Would you be willing to participate in the 4.7.10 working group on Mail that includes this PR? https://issues.civicrm.org/jira/secure/RapidBoard.jspa?rapidView=28&view=planning |
|
Heh @kenwest - you too - any interest in that working group? Or another? We haven't figured how to do the signup in JIRA so for now we're managing things through https://docs.google.com/document/d/1hk5FdVLD9pFzzO-m1mojdRIcKQM5qIr8amHv6b00cRE/edit?ts=577b3413# |
|
Sure I'm already signed up to the merge lot, there's actually a couple of
|
… job (civicrm#8355) * CRM-18543 Deleting email address or phone of CiviMail recipient causes job to hang * CRM-18543 Deleting email address or phone of CiviMail recipient causes job to hang
… job (civicrm#8355) * CRM-18543 Deleting email address or phone of CiviMail recipient causes job to hang * CRM-18543 Deleting email address or phone of CiviMail recipient causes job to hang
There are situations when both the email and phone are null. See CRM-18345.
Skip the recipient in this case.