From d78cc635c03d2678c6ef8e11712a009a48088013 Mon Sep 17 00:00:00 2001 From: Tim Otten Date: Fri, 23 Jan 2015 21:03:13 -0800 Subject: [PATCH] CRM-15855 - Allow mailings to be saved (but not sent) without name+subject. --- CRM/Mailing/BAO/Mailing.php | 43 ++++++++++++++++++++++---- api/v3/Mailing.php | 2 -- js/angular-crmMailing/services.js | 15 ++++++--- partials/crmMailing/edit-unified.html | 2 +- partials/crmMailing/edit-unified2.html | 2 +- partials/crmMailing/edit-wizard.html | 2 +- partials/crmMailing/edit.html | 2 +- tests/phpunit/api/v3/MailingTest.php | 32 +++++++++++++------ 8 files changed, 75 insertions(+), 25 deletions(-) diff --git a/CRM/Mailing/BAO/Mailing.php b/CRM/Mailing/BAO/Mailing.php index 43a9a07884..b14cc5218a 100644 --- a/CRM/Mailing/BAO/Mailing.php +++ b/CRM/Mailing/BAO/Mailing.php @@ -1541,7 +1541,7 @@ ORDER BY civicrm_email.is_bulkmail DESC * Reference array contains the id. * * - * @return object + * @return CRM_Mailing_DAO_Mailing */ public static function add(&$params, $ids = array()) { $id = CRM_Utils_Array::value('mailing_id', $ids, CRM_Utils_Array::value('id', $params)); @@ -1554,7 +1554,10 @@ ORDER BY civicrm_email.is_bulkmail DESC } $mailing = new static(); - $mailing->id = $id; + if ($id) { + $mailing->id = $id; + $mailing->find(TRUE); + } $mailing->domain_id = CRM_Utils_Array::value('domain_id', $params, CRM_Core_Config::domainID()); if (!isset($params['replyto_email']) && @@ -1700,12 +1703,19 @@ ORDER BY civicrm_email.is_bulkmail DESC // check and attach and files as needed CRM_Core_BAO_File::processAttachment($params, 'civicrm_mailing', $mailing->id); + // If we're going to autosend, then check validity before saving. + if (!empty($params['scheduled_date']) && $params['scheduled_date'] != 'null') { + $errors = static::checkSendable($mailing); + if (!empty($errors)) { + $fields = implode(',', array_keys($errors)); + throw new CRM_Core_Exception("Mailing cannot be sent. There are missing fields ($fields).", 'cannot-send', $errors); + } + } + $transaction->commit(); - /** - * create parent job if not yet created - * condition on the existence of a scheduled date - */ + // Create parent job if not yet created. + // Condition on the existence of a scheduled date. if (!empty($params['scheduled_date']) && $params['scheduled_date'] != 'null') { $job = new CRM_Mailing_BAO_MailingJob(); $job->mailing_id = $mailing->id; @@ -1726,6 +1736,27 @@ ORDER BY civicrm_email.is_bulkmail DESC return $mailing; } + /** + * @param CRM_Mailing_DAO_Mailing $mailing + * The mailing which may or may not be sendable. + * @return array + * List of error messages. + */ + public static function checkSendable($mailing) { + $errors = array(); + foreach (array('subject', 'name', 'from_name', 'from_email') as $field) { + if (empty($mailing->{$field})) { + $errors[$field] = ts('Field "%1" is required.', array( + 1 => $field, + )); + } + } + if (empty($mailing->body_html) && empty($mailing->body_text)) { + $errors['body'] = ts('Field "body_html" or "body_text" is required.'); + } + return $errors; + } + /** * Replace the list of recipients on a given mailing * diff --git a/api/v3/Mailing.php b/api/v3/Mailing.php index ed73b8a1ed..a9ab3a3221 100755 --- a/api/v3/Mailing.php +++ b/api/v3/Mailing.php @@ -102,8 +102,6 @@ function civicrm_api3_mailing_get_token($params) { * Array or parameters determined by getfields. */ function _civicrm_api3_mailing_create_spec(&$params) { - $params['name']['api.required'] = 1; - $params['subject']['api.required'] = 1; $params['created_id']['api.required'] = 1; $params['created_id']['api.default'] = 'user_contact_id'; $params['api.mailing_job.create']['api.default'] = 1; diff --git a/js/angular-crmMailing/services.js b/js/angular-crmMailing/services.js index 342d760941..e1eae1c2bc 100644 --- a/js/angular-crmMailing/services.js +++ b/js/angular-crmMailing/services.js @@ -153,12 +153,12 @@ create: function create() { return { jobs: {}, // {jobId: JobRecord} - name: "New Mailing", + name: "", campaign_id: null, from_name: crmFromAddresses.getDefault().author, from_email: crmFromAddresses.getDefault().email, replyto_email: "", - subject: "New Mailing", + subject: "", groups: {include: [], exclude: []}, mailings: {include: [], exclude: []}, body_html: "", @@ -274,11 +274,18 @@ // Save a (draft) mailing // @param mailing Object (per APIv3) // @return Promise - save: function (mailing) { + save: function(mailing) { var params = angular.extend({}, mailing, { 'api.mailing_job.create': 0 // note: exact match to API default }); + // Angular ngModel sometimes treats blank fields as undefined. + angular.forEach(mailing, function(value, key) { + if (value === undefined) { + mailing[key] = ''; + } + }); + // WORKAROUND: Mailing.create (aka CRM_Mailing_BAO_Mailing::create()) interprets scheduled_date // as an *intent* to schedule and creates tertiary records. Saving a draft with a scheduled_date // is therefore not allowed. Remove this after fixing Mailing.create's contract. @@ -286,7 +293,7 @@ delete params.jobs; - return crmApi('Mailing', 'create', params).then(function (result) { + return crmApi('Mailing', 'create', params).then(function(result) { if (result.id && !mailing.id) { mailing.id = result.id; } // no rollback, so update mailing.id diff --git a/partials/crmMailing/edit-unified.html b/partials/crmMailing/edit-unified.html index 55e80aaf8f..25e1d7b69c 100644 --- a/partials/crmMailing/edit-unified.html +++ b/partials/crmMailing/edit-unified.html @@ -6,7 +6,7 @@ {{ts('This mailing has been submitted.')}} -
+
diff --git a/partials/crmMailing/edit-unified2.html b/partials/crmMailing/edit-unified2.html index e5ea499445..d56c3bd1ca 100644 --- a/partials/crmMailing/edit-unified2.html +++ b/partials/crmMailing/edit-unified2.html @@ -6,7 +6,7 @@ {{ts('This mailing has been submitted.')}}
- +
diff --git a/partials/crmMailing/edit-wizard.html b/partials/crmMailing/edit-wizard.html index 4a8141db09..b9ee637196 100644 --- a/partials/crmMailing/edit-wizard.html +++ b/partials/crmMailing/edit-wizard.html @@ -6,7 +6,7 @@ {{ts('This mailing has been submitted.')}}
- +
diff --git a/partials/crmMailing/edit.html b/partials/crmMailing/edit.html index 9340e37a9f..3cdda25c78 100644 --- a/partials/crmMailing/edit.html +++ b/partials/crmMailing/edit.html @@ -6,7 +6,7 @@ {{ts('This mailing has been submitted.')}}
- +
diff --git a/tests/phpunit/api/v3/MailingTest.php b/tests/phpunit/api/v3/MailingTest.php index 4be15d85f5..c908fbd934 100755 --- a/tests/phpunit/api/v3/MailingTest.php +++ b/tests/phpunit/api/v3/MailingTest.php @@ -314,55 +314,67 @@ class api_v3_MailingTest extends CiviUnitTestCase { $cases = array(); // $useLogin, $params, $expectedFailure, $expectedJobCount $cases[] = array( TRUE, //useLogin + array(), // createParams array('scheduled_date' => '2014-12-13 10:00:00', 'approval_date' => '2014-12-13 00:00:00'), FALSE, // expectedFailure 1, // expectedJobCount ); $cases[] = array( FALSE, //useLogin + array(), // createParams array('scheduled_date' => '2014-12-13 10:00:00', 'approval_date' => '2014-12-13 00:00:00'), "/Failed to determine current user/", // expectedFailure 0, // expectedJobCount ); $cases[] = array( TRUE, //useLogin + array(), // createParams array('scheduled_date' => '2014-12-13 10:00:00'), FALSE, // expectedFailure 1, // expectedJobCount ); $cases[] = array( TRUE, //useLogin + array(), // createParams array(), "/Missing parameter scheduled_date and.or approval_date/", // expectedFailure 0, // expectedJobCount ); + $cases[] = array( + TRUE, //useLogin + array('name' => ''), // createParams + array('scheduled_date' => '2014-12-13 10:00:00', 'approval_date' => '2014-12-13 00:00:00'), + "/Mailing cannot be sent. There are missing fields \\(name\\)./", // expectedFailure + 0, // expectedJobCount + ); return $cases; } /** * @param bool $useLogin - * @param array $params + * @param array $createParams + * @param array $submitParams * @param null|string $expectedFailure * @param int $expectedJobCount * @dataProvider submitProvider */ - public function testMailerSubmit($useLogin, $params, $expectedFailure, $expectedJobCount) { + public function testMailerSubmit($useLogin, $createParams, $submitParams, $expectedFailure, $expectedJobCount) { if ($useLogin) { $this->createLoggedInUser(); } - $id = $this->createDraftMailing(); + $id = $this->createDraftMailing($createParams); - $params['id'] = $id; + $submitParams['id'] = $id; if ($expectedFailure) { - $submitResult = $this->callAPIFailure('mailing', 'submit', $params); + $submitResult = $this->callAPIFailure('mailing', 'submit', $submitParams); $this->assertRegExp($expectedFailure, $submitResult['error_message']); } else { - $submitResult = $this->callAPIAndDocument('mailing', 'submit', $params, __FUNCTION__, __FILE__); + $submitResult = $this->callAPIAndDocument('mailing', 'submit', $submitParams, __FUNCTION__, __FILE__); $this->assertTrue(is_numeric($submitResult['id'])); $this->assertTrue(is_numeric($submitResult['values'][$id]['scheduled_id'])); - $this->assertEquals($params['scheduled_date'], $submitResult['values'][$id]['scheduled_date']); + $this->assertEquals($submitParams['scheduled_date'], $submitResult['values'][$id]['scheduled_date']); } $this->assertDBQuery($expectedJobCount, 'SELECT count(*) FROM civicrm_mailing_job WHERE mailing_id = %1', array( 1 => array($id, 'Integer'), @@ -520,10 +532,12 @@ SELECT event_queue_id, time_stamp FROM mail_{$type}_temp"; } /** + * @param array $params + * Extra parameters for the draft mailing. * @return array|int */ - public function createDraftMailing() { - $createParams = $this->_params; + public function createDraftMailing($params = array()) { + $createParams = array_merge($this->_params, $params); $createParams['api.mailing_job.create'] = 0; // note: exact match to API default $createResult = $this->callAPISuccess('mailing', 'create', $createParams, __FUNCTION__, __FILE__); $this->assertTrue(is_numeric($createResult['id'])); -- 2.25.1