From 36f5faa329f4463e17e107a01eac7447112f6c3e Mon Sep 17 00:00:00 2001 From: Matthew Wire Date: Mon, 5 Mar 2018 11:06:36 +0000 Subject: [PATCH] Cleanup SMS functions and rename variables so they are easier to understand --- CRM/Activity/BAO/Activity.php | 153 ++++++++++++++++++---------------- CRM/SMS/Provider.php | 26 +++--- CRM/Utils/Token.php | 10 +-- 3 files changed, 99 insertions(+), 90 deletions(-) diff --git a/CRM/Activity/BAO/Activity.php b/CRM/Activity/BAO/Activity.php index e3ff859b88..ab864e2500 100644 --- a/CRM/Activity/BAO/Activity.php +++ b/CRM/Activity/BAO/Activity.php @@ -1638,75 +1638,85 @@ LEFT JOIN civicrm_activity_contact src ON (src.activity_id = ac.activity_id AND } /** - * Send SMS. + * Send SMS. Returns: bool $sent, int $activityId, int $success (number of sent SMS) * * @param array $contactDetails * @param array $activityParams - * @param array $smsParams - * @param $contactIds - * @param int $userID + * @param array $smsProviderParams + * @param array $contactIds + * @param int $sourceContactId This is the source contact Id * - * @return array + * @return array(bool $sent, int $activityId, int $success) * @throws CRM_Core_Exception */ public static function sendSMS( - &$contactDetails, + &$contactDetails = NULL, &$activityParams, - &$smsParams = array(), - &$contactIds, - $userID = NULL + &$smsProviderParams = array(), + &$contactIds = NULL, + $sourceContactId = NULL ) { - if ($userID == NULL) { - $userID = CRM_Core_Session::getLoggedInContactID(); + if (!isset($contactDetails) && !isset($contactIds)) { + Throw new CRM_Core_Exception('You must specify either $contactDetails or $contactIds'); + } + // Populate $contactDetails and $contactIds if only one is set + if (is_array($contactIds) && !empty($contactIds) && empty($contactDetails)) { + foreach ($contactIds as $id) { + try { + $contactDetails[] = civicrm_api3('Contact', 'getsingle', array('contact_id' => $id)); + } + catch (Exception $e) { + // Contact Id doesn't exist + } + } } + elseif (is_array($contactDetails) && !empty($contactDetails) && empty($contactIds)) { + foreach ($contactDetails as $contact) { + $contactIds[] = $contact['contact_id']; + } + } + if (!CRM_Core_Permission::check('send SMS')) { throw new CRM_Core_Exception("You do not have the 'send SMS' permission"); } - $text = &$activityParams['sms_text_message']; - - // CRM-4575 - // token replacement of addressee/email/postal greetings - // get the tokens added in subject and message - $messageToken = CRM_Utils_Token::getTokens($text); - // Create the meta level record first ( sms activity ) - $activityTypeID = CRM_Core_PseudoConstant::getKey('CRM_Activity_BAO_Activity', - 'activity_type_id', - 'SMS' - ); + // Get logged in User Id + if (empty($sourceContactId)) { + $sourceContactId = CRM_Core_Session::getLoggedInContactID(); + } - $details = $text; + $text = &$activityParams['sms_text_message']; - $activitySubject = $activityParams['activity_subject']; + // Create the meta level record first ( sms activity ) $activityParams = array( - 'source_contact_id' => $userID, - 'activity_type_id' => $activityTypeID, + 'source_contact_id' => $sourceContactId, + 'activity_type_id' => CRM_Core_PseudoConstant::getKey('CRM_Activity_BAO_Activity', 'activity_type_id', 'SMS'), 'activity_date_time' => date('YmdHis'), - 'subject' => $activitySubject, - 'details' => $details, + 'subject' => $activityParams['activity_subject'], + 'details' => $text, 'status_id' => CRM_Core_PseudoConstant::getKey('CRM_Activity_BAO_Activity', 'status_id', 'Completed'), ); - $activity = self::create($activityParams); $activityID = $activity->id; + // Process Tokens + // token replacement of addressee/email/postal greetings + // get the tokens added in subject and message + $messageToken = CRM_Utils_Token::getTokens($text); $returnProperties = array(); - if (isset($messageToken['contact'])) { foreach ($messageToken['contact'] as $key => $value) { $returnProperties[$value] = 1; } } - - // call token hook + // Call tokens hook $tokens = array(); CRM_Utils_Hook::tokens($tokens); $categories = array_keys($tokens); - // get token details for contacts, call only if tokens are used - $details = array(); + $tokenDetails = array(); if (!empty($returnProperties) || !empty($tokens)) { - list($details) = CRM_Utils_Token::getTokenDetails($contactIds, + list($tokenDetails) = CRM_Utils_Token::getTokenDetails($contactIds, $returnProperties, NULL, NULL, FALSE, $messageToken, @@ -1715,35 +1725,34 @@ LEFT JOIN civicrm_activity_contact src ON (src.activity_id = ac.activity_id AND } $success = 0; - $escapeSmarty = FALSE; $errMsgs = array(); - foreach ($contactDetails as $values) { - $contactId = $values['contact_id']; + foreach ($contactDetails as $contact) { + $contactId = $contact['contact_id']; - if (!empty($details) && is_array($details["{$contactId}"])) { + // Replace tokens + if (!empty($tokenDetails) && is_array($tokenDetails["{$contactId}"])) { // unset phone from details since it always returns primary number - unset($details["{$contactId}"]['phone']); - unset($details["{$contactId}"]['phone_type_id']); - $values = array_merge($values, $details["{$contactId}"]); + unset($tokenDetails["{$contactId}"]['phone']); + unset($tokenDetails["{$contactId}"]['phone_type_id']); + $contact = array_merge($contact, $tokenDetails["{$contactId}"]); } - - $tokenText = CRM_Utils_Token::replaceContactTokens($text, $values, FALSE, $messageToken, FALSE, $escapeSmarty); - $tokenText = CRM_Utils_Token::replaceHookTokens($tokenText, $values, $categories, FALSE, $escapeSmarty); + $tokenText = CRM_Utils_Token::replaceContactTokens($text, $contact, FALSE, $messageToken, FALSE, FALSE); + $tokenText = CRM_Utils_Token::replaceHookTokens($tokenText, $contact, $categories, FALSE, FALSE); // Only send if the phone is of type mobile - if ($values['phone_type_id'] == CRM_Core_PseudoConstant::getKey('CRM_Core_BAO_Phone', 'phone_type_id', 'Mobile')) { - $smsParams['To'] = $values['phone']; + if ($contact['phone_type_id'] == CRM_Core_PseudoConstant::getKey('CRM_Core_BAO_Phone', 'phone_type_id', 'Mobile')) { + $smsProviderParams['To'] = $contact['phone']; } else { - $smsParams['To'] = ''; + $smsProviderParams['To'] = ''; } $sendResult = self::sendSMSMessage( $contactId, $tokenText, - $smsParams, + $smsProviderParams, $activityID, - $userID + $sourceContactId ); if (PEAR::isError($sendResult)) { @@ -1776,11 +1785,11 @@ LEFT JOIN civicrm_activity_contact src ON (src.activity_id = ac.activity_id AND * @param int $toID * The contact id of the recipient. * @param $tokenText - * @param array $smsParams + * @param array $smsProviderParams * The params used for sending sms. * @param int $activityID * The activity ID that tracks the message. - * @param int $userID + * @param int $sourceContactID * * @return bool|PEAR_Error * true on success or PEAR_Error object @@ -1788,31 +1797,33 @@ LEFT JOIN civicrm_activity_contact src ON (src.activity_id = ac.activity_id AND public static function sendSMSMessage( $toID, &$tokenText, - $smsParams = array(), + $smsProviderParams = array(), $activityID, - $userID = NULL + $sourceContactID = NULL ) { - $toDoNotSms = ""; - $toPhoneNumber = ""; + $doNotSms = TRUE; + $toPhoneNumber = NULL; - if ($smsParams['To']) { - $toPhoneNumber = trim($smsParams['To']); + if ($smsProviderParams['To']) { + // If phone number is specified use it + $toPhoneNumber = trim($smsProviderParams['To']); } elseif ($toID) { + // No phone number specified, so find a suitable one for the contact $filters = array('is_deceased' => 0, 'is_deleted' => 0, 'do_not_sms' => 0); $toPhoneNumbers = CRM_Core_BAO_Phone::allPhones($toID, FALSE, 'Mobile', $filters); - // To get primary mobile phonenumber,if not get the first mobile phonenumber + // To get primary mobile phonenumber, if not get the first mobile phonenumber if (!empty($toPhoneNumbers)) { - $toPhoneNumerDetails = reset($toPhoneNumbers); - $toPhoneNumber = CRM_Utils_Array::value('phone', $toPhoneNumerDetails); + $toPhoneNumberDetails = reset($toPhoneNumbers); + $toPhoneNumber = CRM_Utils_Array::value('phone', $toPhoneNumberDetails); // Contact allows to send sms - $toDoNotSms = 0; + $doNotSms = FALSE; } } // make sure both phone are valid // and that the recipient wants to receive sms - if (empty($toPhoneNumber) or $toDoNotSms) { + if (empty($toPhoneNumber) or $doNotSms) { return PEAR::raiseError( 'Recipient phone number is invalid or recipient does not want to receive SMS', NULL, @@ -1820,20 +1831,18 @@ LEFT JOIN civicrm_activity_contact src ON (src.activity_id = ac.activity_id AND ); } - $recipient = $smsParams['To']; - $smsParams['contact_id'] = $toID; - $smsParams['parent_activity_id'] = $activityID; + $recipient = $smsProviderParams['To']; + $smsProviderParams['contact_id'] = $toID; + $smsProviderParams['parent_activity_id'] = $activityID; - $providerObj = CRM_SMS_Provider::singleton(array('provider_id' => $smsParams['provider_id'])); - $sendResult = $providerObj->send($recipient, $smsParams, $tokenText, NULL, $userID); + $providerObj = CRM_SMS_Provider::singleton(array('provider_id' => $smsProviderParams['provider_id'])); + $sendResult = $providerObj->send($recipient, $smsProviderParams, $tokenText, NULL, $sourceContactID); if (PEAR::isError($sendResult)) { return $sendResult; } - $activityContacts = CRM_Activity_BAO_ActivityContact::buildOptions('record_type_id', 'validate'); - $targetID = CRM_Utils_Array::key('Activity Targets', $activityContacts); - - // add activity target record for every sms that is send + // add activity target record for every sms that is sent + $targetID = CRM_Core_PseudoConstant::getKey('CRM_Activity_BAO_ActivityContact', 'record_type_id', 'Activity Targets'); $activityTargetParams = array( 'activity_id' => $activityID, 'contact_id' => $toID, diff --git a/CRM/SMS/Provider.php b/CRM/SMS/Provider.php index 3ce50e9062..cbbe3f1d4f 100644 --- a/CRM/SMS/Provider.php +++ b/CRM/SMS/Provider.php @@ -72,14 +72,18 @@ abstract class CRM_SMS_Provider { if (!isset(self::$_singleton[$cacheKey]) || $force) { $ext = CRM_Extension_System::singleton()->getMapper(); if ($ext->isExtensionKey($providerName)) { - $paymentClass = $ext->keyToClass($providerName); - require_once "{$paymentClass}.php"; + $providerClass = $ext->keyToClass($providerName); + require_once "{$providerClass}.php"; } else { - CRM_Core_Error::fatal("Could not locate extension for {$providerName}."); + // If we are running unit tests we simulate an SMS provider with the name "testSmsProvider" + if ($providerName !== 'testSmsProvider') { + CRM_Core_Error::fatal("Could not locate extension for {$providerName}."); + } + $providerClass = 'testSmsProvider'; } - self::$_singleton[$cacheKey] = $paymentClass::singleton($providerParams, $force); + self::$_singleton[$cacheKey] = $providerClass::singleton($providerParams, $force); } return self::$_singleton[$cacheKey]; } @@ -158,12 +162,11 @@ INNER JOIN civicrm_mailing_job mj ON mj.mailing_id = m.id AND mj.id = %1"; return FALSE; } - $activityTypeID = CRM_Core_OptionGroup::getValue('activity_type', 'SMS delivery', 'name'); // note: lets not pass status here, assuming status will be updated by callback $activityParams = array( 'source_contact_id' => $sourceContactID, 'target_contact_id' => $headers['contact_id'], - 'activity_type_id' => $activityTypeID, + 'activity_type_id' => CRM_Core_PseudoConstant::getKey('CRM_Activity_BAO_Activity', 'activity_type_id', 'SMS delivery'), 'activity_date_time' => date('YmdHis'), 'details' => $message, 'result' => $apiMsgID, @@ -186,7 +189,7 @@ INNER JOIN civicrm_mailing_job mj ON mj.mailing_id = m.id AND mj.id = %1"; FALSE, $default, $location ); if ($abort && $value === NULL) { - CRM_Core_Error::debug_log_message("Could not find an entry for $name in $location"); + Civi::log()->warning("Could not find an entry for $name in $location"); echo "Failure: Missing Parameter

"; exit(); } @@ -259,16 +262,13 @@ INNER JOIN civicrm_mailing_job mj ON mj.mailing_id = m.id AND mj.id = %1"; } if ($message->fromContactID) { - $actStatusIDs = array_flip(CRM_Core_OptionGroup::values('activity_status')); - $activityTypeID = CRM_Core_PseudoConstant::getKey('CRM_Activity_BAO_Activity', 'activity_type_id', 'Inbound SMS'); - // note: lets not pass status here, assuming status will be updated by callback $activityParams = array( 'source_contact_id' => $message->toContactID, 'target_contact_id' => $message->fromContactID, - 'activity_type_id' => $activityTypeID, + 'activity_type_id' => CRM_Core_PseudoConstant::getKey('CRM_Activity_BAO_Activity', 'activity_type_id', 'Inbound SMS'), 'activity_date_time' => date('YmdHis'), - 'status_id' => $actStatusIDs['Completed'], + 'status_id' => CRM_Core_PseudoConstant::getKey('CRM_Activity_BAO_Activity', 'activity_status_id', 'Completed'), 'details' => $message->body, 'phone_number' => $message->from, ); @@ -277,7 +277,7 @@ INNER JOIN civicrm_mailing_job mj ON mj.mailing_id = m.id AND mj.id = %1"; } $result = CRM_Activity_BAO_Activity::create($activityParams); - CRM_Core_Error::debug_log_message("Inbound SMS recorded for cid={$message->fromContactID}."); + Civi::log()->info("Inbound SMS recorded for cid={$message->fromContactID}."); return $result; } } diff --git a/CRM/Utils/Token.php b/CRM/Utils/Token.php index c2cbd5fbfc..acf57b69e2 100644 --- a/CRM/Utils/Token.php +++ b/CRM/Utils/Token.php @@ -1157,7 +1157,7 @@ class CRM_Utils_Token { * Gives required details of contacts in an indexed array format so we * can iterate in a nice loop and do token evaluation * - * @param $contactIDs + * @param array $contactIDs * @param array $returnProperties * Of required properties. * @param bool $skipOnHold Don't return on_hold contact info also. @@ -1186,7 +1186,7 @@ class CRM_Utils_Token { ) { $params = array(); - foreach ($contactIDs as $key => $contactID) { + foreach ($contactIDs as $contactID) { $params[] = array( CRM_Core_Form::CB_PREFIX . $contactID, '=', @@ -1215,7 +1215,7 @@ class CRM_Utils_Token { $fields = array_merge(array_keys(CRM_Contact_BAO_Contact::exportableFields()), array('display_name', 'checksum', 'contact_id') ); - foreach ($fields as $key => $val) { + foreach ($fields as $val) { // The unavailable fields are not available as tokens, do not have a one-2-one relationship // with contacts and are expensive to resolve. // @todo see CRM-17253 - there are some other fields (e.g note) that should be excluded @@ -1239,12 +1239,12 @@ class CRM_Utils_Token { $contactDetails = &$details[0]; - foreach ($contactIDs as $key => $contactID) { + foreach ($contactIDs as $contactID) { if (array_key_exists($contactID, $contactDetails)) { if (!empty($contactDetails[$contactID]['preferred_communication_method']) ) { $communicationPreferences = array(); - foreach ($contactDetails[$contactID]['preferred_communication_method'] as $key => $val) { + foreach ($contactDetails[$contactID]['preferred_communication_method'] as $val) { if ($val) { $communicationPreferences[$val] = CRM_Core_PseudoConstant::getLabel('CRM_Contact_DAO_Contact', 'preferred_communication_method', $val); } -- 2.25.1