From 84f6779a2f95a1ac05f721a42b2b1b107b1deac2 Mon Sep 17 00:00:00 2001 From: eileen Date: Thu, 23 Apr 2020 14:58:22 +1200 Subject: [PATCH] [REF] Minor cleanup around actionn schedule code. CRM_Core_BAO_ActionSchedule::processQueue is only called from one place. It either succeeds or throws an exception so there is no value in returning anything & the parsing of what it does return assumes that it could return false Some slight tightenig around typing as buildRecipientContact is also only called from one place --- CRM/Core/BAO/ActionSchedule.php | 31 +++++++++++++++++++------------ CRM/Utils/Time.php | 4 ++-- Civi/Api4/ActionSchedule.php | 2 -- api/v3/Job.php | 22 +++++++++------------- 4 files changed, 30 insertions(+), 29 deletions(-) diff --git a/CRM/Core/BAO/ActionSchedule.php b/CRM/Core/BAO/ActionSchedule.php index 11d9b209c80a..7a5b1a06af9e 100644 --- a/CRM/Core/BAO/ActionSchedule.php +++ b/CRM/Core/BAO/ActionSchedule.php @@ -25,8 +25,11 @@ class CRM_Core_BAO_ActionSchedule extends CRM_Core_DAO_ActionSchedule { /** * @param array $filters * Filter by property (e.g. 'id'). + * * @return array * Array(scalar $id => Mapping $mapping). + * + * @throws \CRM_Core_Exception */ public static function getMappings($filters = NULL) { static $_action_mapping; @@ -320,14 +323,21 @@ public static function sendMailings($mappingID, $now) { } /** - * @param int $mappingID - * @param $now + * Build a list of the contacts to send to. + * + * @param string $mappingID + * Value from the mapping_id field in the civicrm_action_schedule able. It might be a string like + * 'contribpage' for an older class like CRM_Contribute_ActionMapping_ByPage of for ones following + * more recent patterns, an integer. + * @param string $now * @param array $params * * @throws API_Exception + * @throws \CRM_Core_Exception */ - public static function buildRecipientContacts($mappingID, $now, $params = []) { + public static function buildRecipientContacts(string $mappingID, $now, $params = []) { $actionSchedule = new CRM_Core_DAO_ActionSchedule(); + $actionSchedule->mapping_id = $mappingID; $actionSchedule->is_active = 1; if (!empty($params)) { @@ -346,25 +356,22 @@ public static function buildRecipientContacts($mappingID, $now, $params = []) { } /** - * @param null $now + * Main processing callback for sending out scheduled reminders. + * + * @param string $now * @param array $params * - * @return array + * @throws \API_Exception + * @throws \CRM_Core_Exception */ public static function processQueue($now = NULL, $params = []) { $now = $now ? CRM_Utils_Time::setTime($now) : CRM_Utils_Time::getTime(); $mappings = CRM_Core_BAO_ActionSchedule::getMappings(); foreach ($mappings as $mappingID => $mapping) { - CRM_Core_BAO_ActionSchedule::buildRecipientContacts($mappingID, $now, $params); + CRM_Core_BAO_ActionSchedule::buildRecipientContacts((string) $mappingID, $now, $params); CRM_Core_BAO_ActionSchedule::sendMailings($mappingID, $now); } - - $result = [ - 'is_error' => 0, - 'messages' => ts('Sent all scheduled reminders successfully'), - ]; - return $result; } /** diff --git a/CRM/Utils/Time.php b/CRM/Utils/Time.php index 6ab662da767f..dd4b4e2cca61 100644 --- a/CRM/Utils/Time.php +++ b/CRM/Utils/Time.php @@ -32,7 +32,7 @@ class CRM_Utils_Time { * @param string $returnFormat * Format in which date is to be retrieved. * - * @return date + * @return string */ public static function getTime($returnFormat = 'YmdHis') { return date($returnFormat, self::getTimeRaw()); @@ -56,7 +56,7 @@ public static function getTimeRaw() { * @param string $returnFormat * Format in which date is to be retrieved. * - * @return date + * @return string */ public static function setTime($newDateTime, $returnFormat = 'YmdHis') { self::$_delta = strtotime($newDateTime) - time(); diff --git a/Civi/Api4/ActionSchedule.php b/Civi/Api4/ActionSchedule.php index 7ddcb4ea6578..86cf74fc9db7 100644 --- a/Civi/Api4/ActionSchedule.php +++ b/Civi/Api4/ActionSchedule.php @@ -14,8 +14,6 @@ * * @package CRM * @copyright CiviCRM LLC https://civicrm.org/licensing - * $Id$ - * */ diff --git a/api/v3/Job.php b/api/v3/Job.php index 252726d7878e..f34d7d48a224 100644 --- a/api/v3/Job.php +++ b/api/v3/Job.php @@ -182,14 +182,16 @@ function _civicrm_api3_job_geocode_spec(&$params) { } /** - * Send the scheduled reminders for all contacts (either for activities or events). + * Send the scheduled reminders as configured. * * @param array $params - * (reference ) input parameters. - * now - the time to use, in YmdHis format - * - makes testing a bit simpler since we can simulate past/future time + * - now - the time to use, in YmdHis format + * - makes testing a bit simpler since we can simulate past/future time * * @return array + * @throws \API_Exception + * @throws \CRM_Core_Exception + * @throws \CiviCRM_API3_Exception */ function civicrm_api3_job_send_reminder($params) { //note that $params['rowCount' can be overridden by one of the preferred syntaxes ($options['limit'] = x @@ -199,18 +201,12 @@ function civicrm_api3_job_send_reminder($params) { $params['rowCount'] = 0; $lock = Civi::lockManager()->acquire('worker.core.ActionSchedule'); if (!$lock->isAcquired()) { - return civicrm_api3_create_error('Could not acquire lock, another ActionSchedule process is running'); + throw new API_Exception('Could not acquire lock, another ActionSchedule process is running'); } - $result = CRM_Core_BAO_ActionSchedule::processQueue(CRM_Utils_Array::value('now', $params), $params); + CRM_Core_BAO_ActionSchedule::processQueue($params['now'] ?? NULL, $params); $lock->release(); - - if ($result['is_error'] == 0) { - return civicrm_api3_create_success(); - } - else { - return civicrm_api3_create_error($result['messages']); - } + return civicrm_api3_create_success(1, $params, 'ActionSchedule', 'send_reminder'); } /**