From 43ceab3f7541e855228e2cf3ae91304bed9dea86 Mon Sep 17 00:00:00 2001 From: Tim Otten Date: Thu, 23 Jul 2015 19:18:22 -0700 Subject: [PATCH] CRM-13244 - CRM_Core_BAO_ActionSchedule - Simplify `sendReminder` The old `sendReminder()` had too many responsibilities (loading token data and processing token data and sending emails and sending SMS's), contained lots of duplicate code, and did pointless variable shuffling. The Smarty and HTML parameters were enforced inconsistently, and the "From" address had weird side-effects, ad nauseum. This commit splits `sendReminder()` into three pieces: * `TokenCompatSubscriber` for filling in token data. * `sendReminderSms()` for sending SMS. * `sendReminderEmail()` for sending email. --- CRM/Core/BAO/ActionSchedule.php | 327 ++++++++---------- CRM/Utils/Token.php | 2 +- Civi/Core/Container.php | 7 +- Civi/Token/TokenCompatSubscriber.php | 116 +++++++ .../CRM/Core/BAO/ActionScheduleTest.php | 139 ++++++++ 5 files changed, 397 insertions(+), 194 deletions(-) create mode 100644 Civi/Token/TokenCompatSubscriber.php diff --git a/CRM/Core/BAO/ActionSchedule.php b/CRM/Core/BAO/ActionSchedule.php index 66cdea2dd3..d27b6fae9a 100755 --- a/CRM/Core/BAO/ActionSchedule.php +++ b/CRM/Core/BAO/ActionSchedule.php @@ -420,179 +420,6 @@ AND cas.entity_value = $id AND return $list; } - /** - * @param int $contactId - * @param $to - * @param int $scheduleID - * @param $from - * @param array $tokenParams - * - * @return bool|null - * @throws CRM_Core_Exception - */ - public static function sendReminder($contactId, $to, $scheduleID, $from, $tokenParams) { - $email = $to['email']; - $phoneNumber = $to['phone']; - $schedule = new CRM_Core_DAO_ActionSchedule(); - $schedule->id = $scheduleID; - - $domain = CRM_Core_BAO_Domain::getDomain(); - $result = NULL; - $hookTokens = array(); - - if ($schedule->find(TRUE)) { - $body_text = $schedule->body_text; - $body_html = $schedule->body_html; - $sms_body_text = $schedule->sms_body_text; - $body_subject = $schedule->subject; - if (!$body_text) { - $body_text = CRM_Utils_String::htmlToText($body_html); - } - - $params = array(array('contact_id', '=', $contactId, 0, 0)); - list($contact, $_) = CRM_Contact_BAO_Query::apiQuery($params); - - //CRM-4524 - $contact = reset($contact); - - if (!$contact || is_a($contact, 'CRM_Core_Error')) { - return NULL; - } - - // merge activity tokens with contact array - $contact = array_merge($contact, $tokenParams); - - //CRM-5734 - CRM_Utils_Hook::tokenValues($contact, $contactId); - - CRM_Utils_Hook::tokens($hookTokens); - $categories = array_keys($hookTokens); - - $type = array('body_html' => 'html', 'body_text' => 'text', 'sms_body_text' => 'text'); - - foreach ($type as $bodyType => $value) { - $dummy_mail = new CRM_Mailing_BAO_Mailing(); - if ($bodyType == 'sms_body_text') { - $dummy_mail->body_text = $$bodyType; - } - else { - $dummy_mail->$bodyType = $$bodyType; - } - $tokens = $dummy_mail->getTokens(); - - if ($$bodyType) { - CRM_Utils_Token::replaceGreetingTokens($$bodyType, NULL, $contact['contact_id']); - $$bodyType = CRM_Utils_Token::replaceDomainTokens($$bodyType, $domain, TRUE, $tokens[$value], TRUE); - $$bodyType = CRM_Utils_Token::replaceContactTokens($$bodyType, $contact, FALSE, $tokens[$value], FALSE, TRUE); - $$bodyType = CRM_Utils_Token::replaceComponentTokens($$bodyType, $contact, $tokens[$value], TRUE, FALSE); - $$bodyType = CRM_Utils_Token::replaceHookTokens($$bodyType, $contact, $categories, TRUE); - } - } - $html = $body_html; - $text = $body_text; - $sms_text = $sms_body_text; - - $smarty = CRM_Core_Smarty::singleton(); - foreach (array( - 'text', - 'html', - 'sms_text', - ) as $elem) { - $$elem = $smarty->fetch("string:{$$elem}"); - } - - //@todo - this next section is a duplicate of function CRM_Utils_Token::getTokens - $matches = array(); - preg_match_all('/(?fetch("string:{$messageSubject}"); - - if ($schedule->mode == 'SMS' or $schedule->mode == 'User_Preference') { - $session = CRM_Core_Session::singleton(); - $userID = $session->get('userID') ? $session->get('userID') : $contactId; - $smsParams = array( - 'To' => $phoneNumber, - 'provider_id' => $schedule->sms_provider_id, - 'activity_subject' => $messageSubject, - ); - $activityTypeID = CRM_Core_OptionGroup::getValue('activity_type', - 'SMS', - 'name' - ); - $activityParams = array( - 'source_contact_id' => $userID, - 'activity_type_id' => $activityTypeID, - 'activity_date_time' => date('YmdHis'), - 'subject' => $messageSubject, - 'details' => $sms_text, - 'status_id' => CRM_Core_OptionGroup::getValue('activity_status', 'Completed', 'name'), - ); - - $activity = CRM_Activity_BAO_Activity::create($activityParams); - - CRM_Activity_BAO_Activity::sendSMSMessage($contactId, - $sms_text, - $smsParams, - $activity->id, - $userID - ); - } - - if ($schedule->mode == 'Email' or $schedule->mode == 'User_Preference') { - // set up the parameters for CRM_Utils_Mail::send - $mailParams = array( - 'groupName' => 'Scheduled Reminder Sender', - 'from' => $from, - 'toName' => $contact['display_name'], - 'toEmail' => $email, - 'subject' => $messageSubject, - 'entity' => 'action_schedule', - 'entity_id' => $scheduleID, - ); - - if (!$html || $contact['preferred_mail_format'] == 'Text' || - $contact['preferred_mail_format'] == 'Both' - ) { - // render the & entities in text mode, so that the links work - $mailParams['text'] = str_replace('&', '&', $text); - } - if ($html && ($contact['preferred_mail_format'] == 'HTML' || - $contact['preferred_mail_format'] == 'Both' - ) - ) { - $mailParams['html'] = $html; - } - $result = CRM_Utils_Mail::send($mailParams); - } - } - $schedule->free(); - - return $result; - } - /** * Add the schedules reminders in the db. * @@ -682,9 +509,6 @@ AND cas.entity_value = $id AND * @throws CRM_Core_Exception */ public static function sendMailings($mappingID, $now) { - $domainValues = CRM_Core_BAO_Domain::getNameAndEmail(); - $fromEmailAddress = "$domainValues[0] <$domainValues[1]>"; - $mapping = new CRM_Core_DAO_ActionMapping(); $mapping->id = $mappingID; $mapping->find(TRUE); @@ -695,10 +519,6 @@ AND cas.entity_value = $id AND $actionSchedule->find(FALSE); while ($actionSchedule->fetch()) { - if ($actionSchedule->from_email) { - $fromEmailAddress = "$actionSchedule->from_name <$actionSchedule->from_email>"; - } - list($tokenEntity, $tokenFields) = CRM_Core_BAO_ActionSchedule::listMailingTokens($mapping); $query = CRM_Core_BAO_ActionSchedule::prepareMailingQuery($mapping, $actionSchedule); $dao = CRM_Core_DAO::executeQuery($query, @@ -733,20 +553,31 @@ AND cas.entity_value = $id AND $toEmail = CRM_Contact_BAO_Contact::getPrimaryEmail($dao->contactID); } if ($toEmail || !(empty($toPhoneNumber) or $toDoNotSms)) { - $to['email'] = $toEmail; - $to['phone'] = $toPhoneNumber; - $result - = CRM_Core_BAO_ActionSchedule::sendReminder( - $dao->contactID, - $to, - $actionSchedule->id, - $fromEmailAddress, - $entityTokenParams - ); + // FIXME: Shouldn't SMS & email errors be independent+equal? + try { + $result = NULL; + $tokenProcessor = self::createTokenProcessor($actionSchedule); + $tokenProcessor->addRow() + ->context('contactId', $dao->contactID) + ->context('tmpTokenParams', $entityTokenParams); + foreach ($tokenProcessor->evaluate()->getRows() as $tokenRow) { + if ($actionSchedule->mode == 'SMS' or $actionSchedule->mode == 'User_Preference') { + self::sendReminderSms($tokenRow, $actionSchedule, $toPhoneNumber); + } + + if ($actionSchedule->mode == 'Email' or $actionSchedule->mode == 'User_Preference') { + $result = self::sendReminderEmail($tokenRow, $actionSchedule, $toEmail); + } + } - if (!$result || is_a($result, 'PEAR_Error')) { - // we could not send an email, for now we ignore, CRM-3406 + if (!$result || is_a($result, 'PEAR_Error')) { + // we could not send an email, for now we ignore, CRM-3406 + $isError = 1; + } + } + catch (\Civi\Token\TokenException $e) { $isError = 1; + $errorMsg = $e->getMessage(); } } else { @@ -1645,4 +1476,116 @@ WHERE reminder.action_schedule_id = %1 AND reminder.action_date_time IS NULL return array($tokenEntity, $tokenFields); } + /** + * @param $tokenRow + * @param $toPhoneNumber + * @param $schedule + * @throws CRM_Core_Exception + */ + protected static function sendReminderSms($tokenRow, $schedule, $toPhoneNumber) { + $messageSubject = $tokenRow->render('subject'); + $sms_body_text = $tokenRow->render('sms_body_text'); + + $session = CRM_Core_Session::singleton(); + $userID = $session->get('userID') ? $session->get('userID') : $tokenRow->context['contactId']; + $smsParams = array( + 'To' => $toPhoneNumber, + 'provider_id' => $schedule->sms_provider_id, + 'activity_subject' => $messageSubject, + ); + $activityTypeID = CRM_Core_OptionGroup::getValue('activity_type', + 'SMS', + 'name' + ); + $activityParams = array( + 'source_contact_id' => $userID, + 'activity_type_id' => $activityTypeID, + 'activity_date_time' => date('YmdHis'), + 'subject' => $messageSubject, + 'details' => $sms_body_text, + 'status_id' => CRM_Core_OptionGroup::getValue('activity_status', 'Completed', 'name'), + ); + + $activity = CRM_Activity_BAO_Activity::create($activityParams); + + CRM_Activity_BAO_Activity::sendSMSMessage($tokenRow->context['contactId'], + $sms_body_text, + $smsParams, + $activity->id, + $userID + ); + } + + /** + * @param CRM_Core_DAO_ActionSchedule $actionSchedule + * @return string + * Ex: "Alice ". + */ + protected static function pickFromEmail($actionSchedule) { + $domainValues = CRM_Core_BAO_Domain::getNameAndEmail(); + $fromEmailAddress = "$domainValues[0] <$domainValues[1]>"; + if ($actionSchedule->from_email) { + $fromEmailAddress = "$actionSchedule->from_name <$actionSchedule->from_email>"; + return $fromEmailAddress; + } + return $fromEmailAddress; + } + + /** + * @param $tokenRow + * @param $schedule + * @param $toEmail + * @return bool + */ + protected static function sendReminderEmail($tokenRow, $schedule, $toEmail) { + $body_text = $tokenRow->render('body_text'); + $body_html = $tokenRow->render('body_html'); + if (!$schedule->body_text) { + $body_text = CRM_Utils_String::htmlToText($body_html); + } + + // set up the parameters for CRM_Utils_Mail::send + $mailParams = array( + 'groupName' => 'Scheduled Reminder Sender', + 'from' => self::pickFromEmail($schedule), + 'toName' => $tokenRow->context['contact']['display_name'], + 'toEmail' => $toEmail, + 'subject' => $tokenRow->render('subject'), + 'entity' => 'action_schedule', + 'entity_id' => $schedule->id, + ); + + if (!$body_html || $tokenRow->context['contact']['preferred_mail_format'] == 'Text' || + $tokenRow->context['contact']['preferred_mail_format'] == 'Both' + ) { + // render the & entities in text mode, so that the links work + $mailParams['text'] = str_replace('&', '&', $body_text); + } + if ($body_html && ($tokenRow->context['contact']['preferred_mail_format'] == 'HTML' || + $tokenRow->context['contact']['preferred_mail_format'] == 'Both' + ) + ) { + $mailParams['html'] = $body_html; + } + $result = CRM_Utils_Mail::send($mailParams); + return $result; + } + + /** + * @param CRM_Core_DAO_ActionSchedule $schedule + * @return \Civi\Token\TokenProcessor + */ + protected static function createTokenProcessor($schedule) { + $tp = new \Civi\Token\TokenProcessor(\Civi\Core\Container::singleton()->get('dispatcher'), array( + 'controller' => __CLASS__, + 'actionSchedule' => $schedule, + 'smarty' => TRUE, + )); + $tp->addMessage('body_text', $schedule->body_text, 'text/plain'); + $tp->addMessage('body_html', $schedule->body_html, 'text/html'); + $tp->addMessage('sms_body_text', $schedule->sms_body_text, 'text/plain'); + $tp->addMessage('subject', $schedule->subject, 'text/plain'); + return $tp; + } + } diff --git a/CRM/Utils/Token.php b/CRM/Utils/Token.php index 192fc5b97b..cb3c0fc67b 100644 --- a/CRM/Utils/Token.php +++ b/CRM/Utils/Token.php @@ -216,7 +216,7 @@ class CRM_Utils_Token { * @return string * the escaped string */ - private static function tokenEscapeSmarty($string) { + public static function tokenEscapeSmarty($string) { // need to use negative look-behind, as both str_replace() and preg_replace() are sequential return preg_replace(array('/{/', '/(?setFactoryClass($class)->setFactoryMethod('singleton'); } + $container->setDefinition('civi_token_compat', new Definition( + 'Civi\Token\TokenCompatSubscriber', + array() + ))->addTag('kernel.event_subscriber'); + \CRM_Utils_Hook::container($container); return $container; @@ -208,7 +213,7 @@ class Container { /** * @param ContainerInterface $container - * @return \Symfony\Component\EventDispatcher\EventDispatcher + * @return \Symfony\Component\EventDispatcher\ContainerAwareEventDispatcher */ public function createEventDispatcher($container) { $dispatcher = new ContainerAwareEventDispatcher($container); diff --git a/Civi/Token/TokenCompatSubscriber.php b/Civi/Token/TokenCompatSubscriber.php new file mode 100644 index 0000000000..1ce2d9ef1d --- /dev/null +++ b/Civi/Token/TokenCompatSubscriber.php @@ -0,0 +1,116 @@ + 'onEvaluate', + Events::TOKEN_RENDER => 'onRender', + ); + } + + /** + * Load token data. + * + * @param TokenValueEvent $e + * @throws TokenException + */ + public function onEvaluate(TokenValueEvent $e) { + // For reasons unknown, replaceHookTokens requires a pre-computed list of + // hook *categories* (aka entities aka namespaces). We'll cache + // this in the TokenProcessor's context. + + $hookTokens = array(); + \CRM_Utils_Hook::tokens($hookTokens); + $categories = array_keys($hookTokens); + $e->getTokenProcessor()->context['hookTokenCategories'] = $categories; + + $messageTokens = $e->getTokenProcessor()->getMessageTokens(); + + foreach ($e->getRows() as $row) { + if (empty($row->context['contact'])) { + $params = array( + array('contact_id', '=', $row->context['contactId'], 0, 0), + ); + list($contact, $_) = \CRM_Contact_BAO_Query::apiQuery($params); + $contact = reset($contact); //CRM-4524 + if (!$contact || is_a($contact, 'CRM_Core_Error')) { + // FIXME: Need to differentiate errors which kill the batch vs the individual row. + throw new TokenException("Failed to generate token data. Invalid contact ID: " . $row->context['contactId']); + } + } + else { + $contact = $row->context['contact']; + } + + if (!empty($row->context['tmpTokenParams'])) { + // merge activity tokens with contact array + // this is pretty weird. + $contact = array_merge($contact, $row->context['tmpTokenParams']); + } + + // Note: This is a small contract change from the past; data should be missing + // less randomly. + //\CRM_Utils_Hook::tokenValues($contact, $row->context['contactId']); + \CRM_Utils_Hook::tokenValues($contact, + $row->context['contactId'], + empty($row->context['mailingJob']) ? NULL : $row->context['mailingJob']->id, + $messageTokens, + $row->context['controller'] + ); + + $row->context('contact', $contact); + } + } + + /** + * Apply the various CRM_Utils_Token helpers. + * + * @param TokenRenderEvent $e + */ + public function onRender(TokenRenderEvent $e) { + $isHtml = ($e->message['format'] == 'text/html'); + $useSmarty = !empty($e->context['smarty']); + + if (!empty($e->context['contact'])) { + \CRM_Utils_Token::replaceGreetingTokens($e->string, NULL, $e->context['contact']['contact_id'], NULL, $useSmarty); + } + + $e->string = \CRM_Utils_Token::replaceDomainTokens($e->string, \CRM_Core_BAO_Domain::getDomain(), $isHtml, $e->message['tokens'], $useSmarty); + + if (!empty($e->context['contact'])) { + $e->string = \CRM_Utils_Token::replaceContactTokens($e->string, $e->context['contact'], $isHtml, $e->message['tokens'], FALSE, $useSmarty); + $e->string = \CRM_Utils_Token::replaceComponentTokens($e->string, $e->context['contact'], $e->message['tokens'], $useSmarty, FALSE); + + // FIXME: This may depend on $contact being merged with hook values. + $e->string = \CRM_Utils_Token::replaceHookTokens($e->string, $e->context['contact'], $e->context['hookTokenCategories'], $isHtml, $useSmarty); + } + + if ($useSmarty) { + $smarty = \CRM_Core_Smarty::singleton(); + $e->string = $smarty->fetch("string:" . $e->string); + } + } + +} diff --git a/tests/phpunit/CRM/Core/BAO/ActionScheduleTest.php b/tests/phpunit/CRM/Core/BAO/ActionScheduleTest.php index 17d7203d2d..cde0bd5e4e 100755 --- a/tests/phpunit/CRM/Core/BAO/ActionScheduleTest.php +++ b/tests/phpunit/CRM/Core/BAO/ActionScheduleTest.php @@ -81,6 +81,7 @@ class CRM_Core_BAO_ActionScheduleTest extends CiviUnitTestCase { 'is_deceased' => 0, 'contact_type' => 'Individual', 'email' => 'test-member@example.com', + 'gender_id' => 'Female', ); $this->fixtures['contact_birthdate'] = array( 'is_deceased' => 0, @@ -525,6 +526,144 @@ class CRM_Core_BAO_ActionScheduleTest extends CiviUnitTestCase { $this->_tearDown(); } + public function mailerExamples() { + $cases = array(); + + $manyTokensTmpl = implode(';;', array( + '{contact.display_name}', // basic contact token + '{contact.gender}', // funny legacy contact token + '{contact.gender_id}', // funny legacy contact token + '{domain.name}', // domain token + '{activity.activity_type}', // action-scheduler token + )); + $manyTokensExpected = 'test-member@example.com;;Female;;Female;;Second Domain;;Phone Call'; + + // In this example, we use a lot of tokens cutting across multiple components.. + $cases[0] = array( + // Schedule definition. + array( + 'subject' => "subj $manyTokensTmpl", + 'body_html' => "html $manyTokensTmpl", + 'body_text' => "text $manyTokensTmpl", + ), + // Assertions (regex). + array( + 'from_name' => "/^FIXME\$/", + 'from_email' => "/^info@EXAMPLE.ORG\$/", + 'subject' => "/^subj $manyTokensExpected\$/", + 'body_html' => "/^html $manyTokensExpected\$/", + 'body_text' => "/^text $manyTokensExpected\$/", + ), + ); + + // In this example, we customize the from address. + $cases[1] = array( + // Schedule definition. + array( + 'from_name' => 'Bob', + 'from_email' => 'bob@example.org', + ), + // Assertions (regex). + array( + 'from_name' => "/^Bob\$/", + 'from_email' => "/^bob@example.org\$/", + ), + ); + + // In this example, we autoconvert HTML to text + $cases[2] = array( + // Schedule definition. + array( + 'body_html' => '

Hello & stuff.

', + 'body_text' => '', + ), + // Assertions (regex). + array( + 'body_html' => '/^' . preg_quote('

Hello & stuff.

', '/') . '/', + 'body_text' => '/^' . preg_quote('Hello & stuff.', '/') . '/', + ), + ); + + // In this example, we autoconvert HTML to text + $cases[3] = array( + // Schedule definition. + array( + 'body_html' => '', + 'body_text' => 'Hello world', + ), + // Assertions (regex). + array( + 'body_html' => '/^--UNDEFINED--$/', + 'body_text' => '/^Hello world$/', + ), + ); + + return $cases; + } + + /** + * This generates a single mailing through the scheduled-reminder + * system (using an activity-reminder as a baseline) and + * checks that the resulting message satisfies various + * regular expressions. + * + * @param array $schedule + * Values to set/override in the schedule. + * Ex: array('subject' => 'Hello, {contact.first_name}!'). + * @param array $patterns + * A list of regexes to compare with the actual email. + * Ex: array('subject' => '/^Hello, Alice!/'). + * Keys: subject, body_text, body_html, from_name, from_email. + * @dataProvider mailerExamples + */ + public function testMailer($schedule, $patterns) { + $actionSchedule = array_merge($this->fixtures['sched_activity_1day'], $schedule); + $actionScheduleDao = CRM_Core_BAO_ActionSchedule::add($actionSchedule); + $this->assertTrue(is_numeric($actionScheduleDao->id)); + + $activity = $this->createTestObject('CRM_Activity_DAO_Activity', $this->fixtures['phonecall']); + $this->assertTrue(is_numeric($activity->id)); + $contact = $this->callAPISuccess('contact', 'create', $this->fixtures['contact']); + $activity->save(); + + $source['contact_id'] = $contact['id']; + $source['activity_id'] = $activity->id; + $source['record_type_id'] = 2; + $activityContact = $this->createTestObject('CRM_Activity_DAO_ActivityContact', $source); + $activityContact->save(); + + CRM_Utils_Time::setTime('2012-06-14 15:00:00'); + $this->callAPISuccess('job', 'send_reminder', array()); + $this->mut->assertRecipients(array(array('test-member@example.com'))); + foreach ($this->mut->getAllMessages('ezc') as $message) { + /** @var ezcMail $message */ + + $messageArray = array(); + $messageArray['subject'] = $message->subject; + $messageArray['from_name'] = $message->from->name; + $messageArray['from_email'] = $message->from->email; + $messageArray['body_text'] = '--UNDEFINED--'; + $messageArray['body_html'] = '--UNDEFINED--'; + + foreach ($message->fetchParts() as $part) { + /** @var ezcMailText ezcMailText */ + if ($part instanceof ezcMailText && $part->subType == 'html') { + $messageArray['body_html'] = $part->text; + } + if ($part instanceof ezcMailText && $part->subType == 'plain') { + $messageArray['body_text'] = $part->text; + } + } + + foreach ($patterns as $field => $pattern) { + $this->assertRegExp($pattern, $messageArray[$field], + "Check that '$field'' matches regex. " . print_r(array('expected' => $patterns, 'actual' => $messageArray), 1)); + } + } + $this->mut->clearMessages(); + + } + public function testActivityDateTimeMatchNonRepeatableSchedule() { $actionScheduleDao = CRM_Core_BAO_ActionSchedule::add($this->fixtures['sched_activity_1day']); $this->assertTrue(is_numeric($actionScheduleDao->id)); -- 2.25.1