Merge pull request #12634 from seamuslee001/dev_core_273

dev/core/#273 Fix issue where sending an SMS with the To Field in the…
civicrm · Aug 14, 2018 · 5fce4a2 · 5fce4a2
2 parents ac22bdf + a9b7ee4
commit 5fce4a2
Show file tree

Hide file tree

Showing 3 changed files with 61 additions and 18 deletions.
diff --git a/CRM/Activity/BAO/Activity.php b/CRM/Activity/BAO/Activity.php
@@ -1772,20 +1772,27 @@ public static function sendSMS(
         $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 @@ public static function sendSMSMessage(
     $activityID,
     $sourceContactID = NULL
   ) {
-    $doNotSms = TRUE;
     $toPhoneNumber = NULL;
-
     if ($smsProviderParams['To']) {
       // If phone number is specified use it
       $toPhoneNumber = trim($smsProviderParams['To']);
@@ -1842,21 +1847,20 @@ public static function sendSMSMessage(
         $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,
         PEAR_ERROR_RETURN
       );
     }
 
-    $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
@@ -231,6 +231,7 @@ public static function getRecipients($mailingID) {
       $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
@@ -1234,10 +1234,36 @@ public function testSendSmsMobilePhoneNumber() {
     $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 @@ public function createSendSmsTest($phoneType = 0) {
       "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 @@ public function createSendSmsTest($phoneType = 0) {
           '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 @@ public function createSendSmsTest($phoneType = 0) {
           '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;
     }