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

dev/mail#11 Add pre/post hook for CRM_Mailing_BAO_MailingJob #12231

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 13 additions & 9 deletions CRM/Mailing/BAO/Mailing.php
Original file line number Diff line number Diff line change
Expand Up @@ -1639,21 +1639,25 @@ public static function create(&$params, $ids = array()) {
// Create parent job if not yet created.
// Condition on the existence of a scheduled date.
if (!empty($params['scheduled_date']) && $params['scheduled_date'] != 'null' && empty($params['_skip_evil_bao_auto_schedule_'])) {
$jobParams = [
'mailing_id' => $mailing->id,
// If we are creating a new Completed mailing (e.g. import from another system) set the job to completed.
// Keeping former behaviour when an id is present is precautionary and may warrant reconsideration later.
'status' => ((empty($params['is_completed']) || !empty($params['id'])) ? 'Scheduled' : 'Complete'),
'is_test' => 0,
'_skip_evil_bao_auto_recipients_' => TRUE,
Copy link
Member Author

@monishdeb monishdeb Jun 1, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@eileenmcnaughton I need to introduce a new parameter here because there need to be some flag which should prevent mailingJob::create to trigger recipient rebuild, as required here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@monishdeb arg. @totten do you think we can start to grandfather to building recipients being opt-in not opt out?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about we go with is_create_recipients (ideally if not set it's false, but at least a deprecated warning if not set & set in the api). I feel like we could probably find the places where MailingJob::create are currently called?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a note - I checked the already merged changes & there is no case where 'mailing_id' is passed in our 'newly-calling create function' params - so we should be safe from any issues in the already-merged changes

];
$job = new CRM_Mailing_BAO_MailingJob();
$job->mailing_id = $mailing->id;
// If we are creating a new Completed mailing (e.g. import from another system) set the job to completed.
// Keeping former behaviour when an id is present is precautionary and may warrant reconsideration later.
$job->status = ((empty($params['is_completed']) || !empty($params['id'])) ? 'Scheduled' : 'Complete');
$job->is_test = 0;
$job->copyValues($jobParams);

if (!$job->find(TRUE)) {
// Don't schedule job until we populate the recipients.
$job->scheduled_date = NULL;
$job->save();
$jobParams['scheduled_date'] = NULL;
$jobParams['id'] = CRM_Mailing_BAO_MailingJob::create($jobParams)->id;
}
// Schedule the job now that it has recipients.
$job->scheduled_date = $params['scheduled_date'];
$job->save();
$jobParams['scheduled_date'] = $params['scheduled_date'];
CRM_Mailing_BAO_MailingJob::create($jobParams);
}

// Populate the recipients.
Expand Down
13 changes: 6 additions & 7 deletions CRM/Mailing/BAO/MailingJob.php
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ static public function create($params) {
$jobDAO = new CRM_Mailing_BAO_MailingJob();
$jobDAO->copyValues($params, TRUE);
$jobDAO->save();
if (!empty($params['mailing_id'])) {
if (!empty($params['mailing_id']) && empty($params['_skip_evil_bao_auto_recipients_'])) {
CRM_Mailing_BAO_Mailing::getRecipients($params['mailing_id']);
}
CRM_Utils_Hook::post($op, 'MailingJob', $jobDAO->id, $jobDAO);
Expand Down Expand Up @@ -165,7 +165,6 @@ public static function runJobs($testParams = NULL, $mode = NULL) {
}

/* Queue up recipients for the child job being launched */

if ($job->status != 'Running') {
$transaction = new CRM_Core_Transaction();

Expand Down Expand Up @@ -283,11 +282,11 @@ public static function runJobs_post($mode = NULL) {

$transaction = new CRM_Core_Transaction();

$saveJob = new CRM_Mailing_DAO_MailingJob();
$saveJob->id = $job->id;
$saveJob->end_date = date('YmdHis');
$saveJob->status = 'Complete';
$saveJob->save();
self::create([
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so this WILL generate a recipients list...

'id' => $job->id,
'end_date' => date('YmdHis'),
'status' => 'Complete',
]);

$mailing->reset();
$mailing->id = $job->mailing_id;
Expand Down
35 changes: 16 additions & 19 deletions CRM/Mailing/BAO/Spool.php
Original file line number Diff line number Diff line change
Expand Up @@ -87,26 +87,23 @@ public function send($recipient, $headers, $body, $job_id = NULL) {
return PEAR::raiseError('Unable to create spooled mailing.');
}

$job = new CRM_Mailing_BAO_MailingJob();
$job->is_test = 0; // if set to 1 it doesn't show in the UI
$job->status = 'Complete';
$job->scheduled_date = CRM_Utils_Date::processDate(date('Y-m-d'), date('H:i:s'));
$job->start_date = $job->scheduled_date;
$job->end_date = $job->scheduled_date;
$job->mailing_id = $mailing->id;
$job->save();
$job_id = $job->id; // need this for parent_id below
$saveJob = new CRM_Mailing_DAO_MailingJob();

$job = new CRM_Mailing_BAO_MailingJob();
$job->is_test = 0;
$job->status = 'Complete';
$job->scheduled_date = CRM_Utils_Date::processDate(date('Y-m-d'), date('H:i:s'));
$job->start_date = $job->scheduled_date;
$job->end_date = $job->scheduled_date;
$job->mailing_id = $mailing->id;
$job->parent_id = $job_id;
$job->job_type = 'child';
$job->save();
$jobParams = array(
'is_test' => 0,
'scheduled_date' => date('YmdHis'),
'start_date' => date('YmdHis'),
'end_date' => date('YmdHis'),
'mailing_id' => $mailing->id,
'status' => 'Complete',
);
$job = CRM_Mailing_BAO_MailingJob::create($jobParams);

$jobParams = array_merge($jobParams, array(
'parent_id' => $job->id,
'job_type' => 'child',
));
$job = CRM_Mailing_BAO_MailingJob::create($jobParams);
$job_id = $job->id; // this is the one we want for the spool

if (is_array($recipient)) {
Expand Down