From 932ea7da0685e31b73696edce9823d351ba77658 Mon Sep 17 00:00:00 2001 From: Eileen McNaughton Date: Wed, 26 Jan 2022 13:32:05 +1300 Subject: [PATCH 1/5] dev/core#2486 Add v4 Mailing api (+ MailingJob) ro --- Civi/Api4/Mailing.php | 25 ++++++++++++++++++++++++ Civi/Api4/MailingJob.php | 29 ++++++++++++++++++++++++++++ tests/phpunit/api/v3/MailingTest.php | 21 ++++++++++++++------ 3 files changed, 69 insertions(+), 6 deletions(-) create mode 100644 Civi/Api4/Mailing.php create mode 100644 Civi/Api4/MailingJob.php diff --git a/Civi/Api4/Mailing.php b/Civi/Api4/Mailing.php new file mode 100644 index 000000000000..e9ada0ac8120 --- /dev/null +++ b/Civi/Api4/Mailing.php @@ -0,0 +1,25 @@ +_apiversion = $version; + $this->enableCiviCampaign(); $this->callAPISuccess('Campaign', 'create', ['name' => 'big campaign', 'title' => 'abc']); $result = $this->callAPIAndDocument('mailing', 'create', $this->_params + ['scheduled_date' => 'now', 'campaign_id' => 'big campaign'], __FUNCTION__, __FILE__); - $jobs = $this->callAPISuccess('mailing_job', 'get', ['mailing_id' => $result['id']]); + $jobs = $this->callAPISuccess('MailingJob', 'get', ['mailing_id' => $result['id']]); $this->assertEquals(1, $jobs['count']); // return isn't working on this in getAndCheck so lets not check it for now unset($this->_params['created_id']); - $this->getAndCheck($this->_params, $result['id'], 'mailing'); + $this->getAndCheck($this->_params, $result['id'], 'Mailing'); } /** @@ -103,9 +109,12 @@ public function testSkipAutoSchedule() { /** * Create a completed mailing (e.g when importing from a provider). * - * @throws \CRM_Core_Exception + * @dataProvider versionThreeAndFour + * + * @param int $version */ - public function testMailerCreateCompleted() { + public function testMailerCreateCompleted(int $version): void { + $this->_apiversion = $version; $this->_params['body_html'] = 'I am completed so it does not matter if there is an opt out link since I have already been sent by another system'; $this->_params['is_completed'] = 1; $result = $this->callAPIAndDocument('mailing', 'create', $this->_params + ['scheduled_date' => 'now'], __FUNCTION__, __FILE__); @@ -120,7 +129,7 @@ public function testMailerCreateCompleted() { /** * Per CRM-20316 the mailing should still create without created_id (not mandatory). */ - public function testMailerCreateSuccessNoCreatedID() { + public function testMailerCreateSuccessNoCreatedID(): void { unset($this->_params['created_id']); $result = $this->callAPIAndDocument('mailing', 'create', $this->_params + ['scheduled_date' => 'now'], __FUNCTION__, __FILE__); $this->getAndCheck($this->_params, $result['id'], 'mailing'); From 7e0fcd9d1073493a953b46caad225498e470f546 Mon Sep 17 00:00:00 2001 From: Eileen McNaughton Date: Fri, 18 Feb 2022 12:40:57 +1300 Subject: [PATCH 2/5] Move validator out of BAO into v3 api m m --- CRM/Mailing/BAO/Mailing.php | 12 ------------ api/v3/Mailing.php | 17 +++++++++++++++-- ext/flexmailer/src/Validator.php | 14 ++++++++++---- 3 files changed, 25 insertions(+), 18 deletions(-) diff --git a/CRM/Mailing/BAO/Mailing.php b/CRM/Mailing/BAO/Mailing.php index a3fc95dd6a8a..b099f6beb645 100644 --- a/CRM/Mailing/BAO/Mailing.php +++ b/CRM/Mailing/BAO/Mailing.php @@ -1478,7 +1478,6 @@ public static function add(&$params, $ids = []) { * * - _skip_evil_bao_auto_recipients_: bool * - _skip_evil_bao_auto_schedule_: bool - * - _evil_bao_validator_: string|callable * * * @@ -1612,17 +1611,6 @@ public static function create(array $params) { // 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['is_completed']) && !empty($params['scheduled_date']) && $params['scheduled_date'] != 'null' && !empty($params['_evil_bao_validator_'])) { - $cb = Civi\Core\Resolver::singleton() - ->get($params['_evil_bao_validator_']); - $errors = call_user_func($cb, $mailing); - if (!empty($errors)) { - $fields = implode(',', array_keys($errors)); - throw new CRM_Core_Exception("Mailing cannot be sent. There are missing or invalid fields ($fields).", 'cannot-send', $errors); - } - } - $transaction->commit(); // Create parent job if not yet created. diff --git a/api/v3/Mailing.php b/api/v3/Mailing.php index 83bc28b7412c..22403a74444a 100644 --- a/api/v3/Mailing.php +++ b/api/v3/Mailing.php @@ -36,9 +36,22 @@ function civicrm_api3_mailing_create($params) { if (!$timestampCheck) { throw new API_Exception("Mailing has not been saved, Content maybe out of date, please refresh the page and try again"); } + // If we're going to autosend, then check validity before saving. + if (empty($params['is_completed']) && !empty($params['scheduled_date']) + && $params['scheduled_date'] !== 'null' + // This might have been passed in as empty to prevent us validating, is set skip. + && !isset($params['_evil_bao_validator_'])) { + + // FlexMailer is a refactoring of CiviMail which provides new hooks/APIs/docs. If the sysadmin has opted to enable it, then use that instead of CiviMail. + $function = \CRM_Utils_Constant::value('CIVICRM_FLEXMAILER_HACK_SENDABLE', 'CRM_Mailing_BAO_Mailing::checkSendable'); + $validationFunction = Civi\Core\Resolver::singleton()->get($function); + $errors = call_user_func($validationFunction, $params); + if (!empty($errors)) { + $fields = implode(',', array_keys($errors)); + throw new CiviCRM_API3_Exception("Mailing cannot be sent. There are missing or invalid fields ($fields).", 'cannot-send', $errors); + } + } - // FlexMailer is a refactoring of CiviMail which provides new hooks/APIs/docs. If the sysadmin has opted to enable it, then use that instead of CiviMail. - $safeParams['_evil_bao_validator_'] = \CRM_Utils_Constant::value('CIVICRM_FLEXMAILER_HACK_SENDABLE', 'CRM_Mailing_BAO_Mailing::checkSendable'); $result = _civicrm_api3_basic_create(_civicrm_api3_get_BAO(__FUNCTION__), $safeParams, 'Mailing'); return _civicrm_api3_mailing_get_formatResult($result); } diff --git a/ext/flexmailer/src/Validator.php b/ext/flexmailer/src/Validator.php index f0818634d3b6..455d6f053881 100644 --- a/ext/flexmailer/src/Validator.php +++ b/ext/flexmailer/src/Validator.php @@ -25,14 +25,20 @@ class Validator { const EVENT_CHECK_SENDABLE = 'civi.flexmailer.checkSendable'; /** - * @param \CRM_Mailing_DAO_Mailing $mailing + * @param array $params * The mailing which may or may not be sendable. * @return array * List of error messages. */ - public static function createAndRun($mailing) { - $validator = new \Civi\FlexMailer\Validator(); - return $validator->run(array( + public static function createAndRun(array $params): array { + $mailing = new \CRM_Mailing_BAO_Mailing(); + $mailing->id = $params['id'] ?? NULL; + if ($mailing->id) { + $mailing->find(TRUE); + } + $mailing->copyValues($params); + + return (new Validator())->run(array( 'mailing' => $mailing, 'attachments' => \CRM_Core_BAO_File::getEntityFile('civicrm_mailing', $mailing->id), )); From cbbddb895c0486d464a7c214b391ed0105aa9d9d Mon Sep 17 00:00:00 2001 From: Eileen McNaughton Date: Fri, 18 Feb 2022 13:05:24 +1300 Subject: [PATCH 3/5] Block crud for v4 api --- CRM/Mailing/BAO/Mailing.php | 70 +++++++++++++++++++++++++------------ 1 file changed, 47 insertions(+), 23 deletions(-) diff --git a/CRM/Mailing/BAO/Mailing.php b/CRM/Mailing/BAO/Mailing.php index b099f6beb645..93592c8e0145 100644 --- a/CRM/Mailing/BAO/Mailing.php +++ b/CRM/Mailing/BAO/Mailing.php @@ -456,6 +456,47 @@ protected static function processWorkflowPermissions(array $params): array { return $safeParams; } + /** + * Do Submit actions. + * + * When submitting (as opposed to creating or updating) a mailing it should + * be scheduled. + * + * This function creates the initial job and the recipient list. + * + * @param array $params + * @param \CRM_Mailing_DAO_Mailing $mailing + * + * @return array + */ + protected static function doSubmitActions(array $params, CRM_Mailing_DAO_Mailing $mailing): 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_'])) { + $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; + + if (!$job->find(TRUE)) { + // Don't schedule job until we populate the recipients. + $job->scheduled_date = NULL; + $job->save(); + } + // Schedule the job now that it has recipients. + $job->scheduled_date = $params['scheduled_date']; + $job->save(); + } + + // Populate the recipients. + if (empty($params['_skip_evil_bao_auto_recipients_'])) { + self::getRecipients($mailing->id); + } + return $params; + } + /** * Returns the regex patterns that are used for preparing the text and html templates. * @@ -1613,29 +1654,12 @@ public static function create(array $params) { $transaction->commit(); - // 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_'])) { - $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; - - if (!$job->find(TRUE)) { - // Don't schedule job until we populate the recipients. - $job->scheduled_date = NULL; - $job->save(); - } - // Schedule the job now that it has recipients. - $job->scheduled_date = $params['scheduled_date']; - $job->save(); - } - - // Populate the recipients. - if (empty($params['_skip_evil_bao_auto_recipients_'])) { - self::getRecipients($mailing->id); + // These actions are really 'submit' not create actions. + // In v4 of the api they are not available via CRUD. At some + // point we will create a 'submit' function which will do the crud+submit + // but for now only CRUD is available via v4 api. + if (($params['version'] ?? '') !== 4) { + $params = self::doSubmitActions($params, $mailing); } return $mailing; From 1b9b056c5ac9936325eaecc54a648fc5e0ea3814 Mon Sep 17 00:00:00 2001 From: Coleman Watts Date: Thu, 17 Feb 2022 20:45:37 -0500 Subject: [PATCH 4/5] APIv4 - Create necessary fk entities in conformance test --- .../api/v4/Service/TestCreationParameterProvider.php | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/phpunit/api/v4/Service/TestCreationParameterProvider.php b/tests/phpunit/api/v4/Service/TestCreationParameterProvider.php index 5bdc2ce7cea7..4db5a4442356 100644 --- a/tests/phpunit/api/v4/Service/TestCreationParameterProvider.php +++ b/tests/phpunit/api/v4/Service/TestCreationParameterProvider.php @@ -150,9 +150,12 @@ private function getFkID(string $fkEntity) { $params['where'] = [['contact_type', '=', 'Individual']]; } $entityList = civicrm_api4($fkEntity, 'get', $params); + // If no existing entities, create one if ($entityList->count() < 1) { - $msg = sprintf('At least one %s is required in test', $fkEntity); - throw new \API_Exception($msg); + $entityList = civicrm_api4($fkEntity, 'create', [ + 'checkPermissions' => FALSE, + 'values' => $this->getRequired($fkEntity), + ]); } return $entityList->last()['id']; From 4eefbb313cb51c31ae3d65afb65a681e827786e5 Mon Sep 17 00:00:00 2001 From: Coleman Watts Date: Sat, 19 Feb 2022 10:28:33 -0500 Subject: [PATCH 5/5] FlexMailer - Update test --- .../tests/phpunit/Civi/FlexMailer/ValidatorTest.php | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/ext/flexmailer/tests/phpunit/Civi/FlexMailer/ValidatorTest.php b/ext/flexmailer/tests/phpunit/Civi/FlexMailer/ValidatorTest.php index 35541e5e6739..642d4527b246 100644 --- a/ext/flexmailer/tests/phpunit/Civi/FlexMailer/ValidatorTest.php +++ b/ext/flexmailer/tests/phpunit/Civi/FlexMailer/ValidatorTest.php @@ -86,9 +86,7 @@ public function getExamples() { * @dataProvider getExamples */ public function testExamples($mailingData, $expectedErrors): void { - $mailing = new \CRM_Mailing_DAO_Mailing(); - $mailing->copyValues($mailingData); - $actualErrors = Validator::createAndRun($mailing); + $actualErrors = Validator::createAndRun($mailingData); $this->assertEquals( array_keys($actualErrors), array_keys($expectedErrors)