From a9b7ee41eeae0a75d0c8bfc62caa81edf3024b83 Mon Sep 17 00:00:00 2001 From: Seamus Lee Date: Wed, 8 Aug 2018 14:38:25 +1000 Subject: [PATCH] dev/core/#273 Fix issue where sending an SMS with the To Field in the provider params set did not work and fix handling of do_not_sms Add test of error message --- CRM/Activity/BAO/Activity.php | 36 +++++++++------- CRM/Mailing/BAO/Mailing.php | 1 + .../phpunit/CRM/Activity/BAO/ActivityTest.php | 42 ++++++++++++++++++- 3 files changed, 61 insertions(+), 18 deletions(-) diff --git a/CRM/Activity/BAO/Activity.php b/CRM/Activity/BAO/Activity.php index 61568a96ed..e7921e83cc 100644 --- a/CRM/Activity/BAO/Activity.php +++ b/CRM/Activity/BAO/Activity.php @@ -1772,20 +1772,27 @@ LEFT JOIN civicrm_activity_contact src ON (src.activity_id = ac.activity_id AND $smsProviderParams['To'] = ''; } - $sendResult = self::sendSMSMessage( - $contactId, - $tokenText, - $smsProviderParams, - $activityID, - $sourceContactId - ); + $doNotSms = CRM_Utils_Array::value('do_not_sms', $contact, 0); - if (PEAR::isError($sendResult)) { - // Collect all of the PEAR_Error objects - $errMsgs[] = $sendResult; + if ($doNotSms) { + $errMsgs[] = PEAR::raiseError('Contact Does not accept SMS', NULL, PEAR_ERROR_RETURN); } else { - $success++; + $sendResult = self::sendSMSMessage( + $contactId, + $tokenText, + $smsProviderParams, + $activityID, + $sourceContactId + ); + + if (PEAR::isError($sendResult)) { + // Collect all of the PEAR_Error objects + $errMsgs[] = $sendResult; + } + else { + $success++; + } } } @@ -1826,9 +1833,7 @@ LEFT JOIN civicrm_activity_contact src ON (src.activity_id = ac.activity_id AND $activityID, $sourceContactID = NULL ) { - $doNotSms = TRUE; $toPhoneNumber = NULL; - if ($smsProviderParams['To']) { // If phone number is specified use it $toPhoneNumber = trim($smsProviderParams['To']); @@ -1842,13 +1847,12 @@ LEFT JOIN civicrm_activity_contact src ON (src.activity_id = ac.activity_id AND $toPhoneNumberDetails = reset($toPhoneNumbers); $toPhoneNumber = CRM_Utils_Array::value('phone', $toPhoneNumberDetails); // Contact allows to send sms - $doNotSms = FALSE; } } // make sure both phone are valid // and that the recipient wants to receive sms - if (empty($toPhoneNumber) or $doNotSms) { + if (empty($toPhoneNumber)) { return PEAR::raiseError( 'Recipient phone number is invalid or recipient does not want to receive SMS', NULL, @@ -1856,7 +1860,7 @@ LEFT JOIN civicrm_activity_contact src ON (src.activity_id = ac.activity_id AND ); } - $recipient = $smsProviderParams['To']; + $recipient = $toPhoneNumber; $smsProviderParams['contact_id'] = $toID; $smsProviderParams['parent_activity_id'] = $activityID; diff --git a/CRM/Mailing/BAO/Mailing.php b/CRM/Mailing/BAO/Mailing.php index f306c72770..212284f00e 100644 --- a/CRM/Mailing/BAO/Mailing.php +++ b/CRM/Mailing/BAO/Mailing.php @@ -231,6 +231,7 @@ class CRM_Mailing_BAO_Mailing extends CRM_Mailing_DAO_Mailing { $criteria = array( 'is_opt_out' => CRM_Utils_SQL_Select::fragment()->where("$contact.is_opt_out = 0"), 'is_deceased' => CRM_Utils_SQL_Select::fragment()->where("$contact.is_deceased <> 1"), + 'do_not_sms' => CRM_Utils_SQL_Select::fragment()->where("$contact.do_not_sms = 0"), 'location_filter' => CRM_Utils_SQL_Select::fragment()->where("$entityTable.phone_type_id = " . CRM_Core_PseudoConstant::getKey('CRM_Core_DAO_Phone', 'phone_type_id', 'Mobile')), 'phone_not_null' => CRM_Utils_SQL_Select::fragment()->where("$entityTable.phone IS NOT NULL"), 'phone_not_empty' => CRM_Utils_SQL_Select::fragment()->where("$entityTable.phone != ''"), diff --git a/tests/phpunit/CRM/Activity/BAO/ActivityTest.php b/tests/phpunit/CRM/Activity/BAO/ActivityTest.php index 33bcacb798..4fbf7b511d 100644 --- a/tests/phpunit/CRM/Activity/BAO/ActivityTest.php +++ b/tests/phpunit/CRM/Activity/BAO/ActivityTest.php @@ -1234,10 +1234,36 @@ $text $this->assertEquals(1, $success, "Expected success to be 1"); } + /** + * Test that when a numbe ris specified in the To Param of the SMS provider parameters that an SMS is sent + * @see dev/core/#273 + */ + public function testSendSMSMobileInToProviderParam() { + list($sent, $activityId, $success) = $this->createSendSmsTest(2, TRUE); + $this->assertEquals(TRUE, $sent, "Expected sent should be true"); + $this->assertEquals(1, $success, "Expected success to be 1"); + } + + /** + * Test that when a numbe ris specified in the To Param of the SMS provider parameters that an SMS is sent + * @see dev/core/#273 + */ + public function testSendSMSMobileInToProviderParamWithDoNotSMS() { + list($sent, $activityId, $success) = $this->createSendSmsTest(2, TRUE, ['do_not_sms' => 1]); + foreach ($sent as $error) { + $this->assertEquals('Contact Does not accept SMS', $error->getMessage()); + } + $this->assertEquals(1, count($sent), "Expected sent should a PEAR Error"); + $this->assertEquals(0, $success, "Expected success to be 0"); + } + + /** * @param int $phoneType (0=no phone, phone_type option group (1=fixed, 2=mobile) + * @param bool $passPhoneTypeInContactDetails + * @param array $additionalContactParams additional contact creation params */ - public function createSendSmsTest($phoneType = 0) { + public function createSendSmsTest($phoneType = 0, $passPhoneTypeInContactDetails = FALSE, $additionalContactParams = []) { $provider = civicrm_api3('SmsProvider', 'create', array( 'name' => "CiviTestSMSProvider", 'api_type' => "1", @@ -1250,11 +1276,15 @@ $text "is_active" => "1", "domain_id" => "1", )); + $smsProviderParams['provider_id'] = $provider['id']; // Create a contact $contactId = $this->individualCreate(); - $contactsResult = $this->civicrm_api('contact', 'get', array('id' => $contactId, 'version' => $this->_apiversion)); + if (!empty($additionalContactParams)) { + $this->callAPISuccess('contact', 'create', ['id' => $contactId] + $additionalContactParams); + } + $contactsResult = $this->callApiSuccess('contact', 'get', ['id' => $contactId, 'version' => $this->_apiversion]); $contactDetails = $contactsResult['values']; // Get contactIds from contact details @@ -1288,6 +1318,10 @@ $text 'phone' => 123456, 'phone_type_id' => "Mobile", )); + if ($passPhoneTypeInContactDetails) { + $contactDetails[$contactId]['phone'] = $phone['values'][$phone['id']]['phone']; + $contactDetails[$contactId]['phone_type_id'] = $phone['values'][$phone['id']]['phone_type_id']; + } break; case 1: @@ -1297,6 +1331,10 @@ $text 'phone' => 654321, 'phone_type_id' => "Phone", )); + if ($passPhoneTypeInContactDetails) { + $contactDetails[$contactId]['phone'] = $phone['values'][$phone['id']]['phone']; + $contactDetails[$contactId]['phone_type_id'] = $phone['values'][$phone['id']]['phone_type_id']; + } break; } -- 2.25.1