diff --git a/CRM/Mailing/BAO/Mailing.php b/CRM/Mailing/BAO/Mailing.php index 08ee1f5b5a9b..e725fe3aa7ca 100644 --- a/CRM/Mailing/BAO/Mailing.php +++ b/CRM/Mailing/BAO/Mailing.php @@ -202,100 +202,75 @@ public static function getRecipients($mailingID) { ->execute(); } - $tempColumn = $isSMSmode ? 'phone_id' : 'email_id'; + $entityColumn = $isSMSmode ? 'phone_id' : 'email_id'; + $entityTable = $isSMSmode ? $phone : $email; $email = CRM_Core_DAO_Email::getTableName(); // Get all the group contacts we want to include. $mailingGroup->query( "CREATE TEMPORARY TABLE $includedTempTablename - (contact_id int primary key, $tempColumn int) + (contact_id int primary key, $entityColumn int) ENGINE=HEAP" ); - // Criterias to filter recipients that need to be included - $includeFilters = array( - "$contact.do_not_email = 0", - "$contact.is_opt_out = 0", - "$contact.is_deceased <> 1", - $location_filter, - "$email.email IS NOT NULL", - "$email.email != ''", - "mg.mailing_id = #mailingID", - 'temp.contact_id IS NULL', - ); - if ($isSMSmode) { $includeFilters = array( "mg.group_type = 'Include'", 'mg.search_id IS NULL', - "gc.status = 'Added'", - "$contact.do_not_sms", "$contact.is_opt_out = 0", "$contact.is_deceased <> 1", - "$phone.phone_type_id = " . CRM_Utils_Array::value('Mobile', CRM_Core_OptionGroup::values('phone_type', TRUE, FALSE, FALSE, NULL, 'name')), + "$phone.phone_type_id = " . CRM_Core_PseudoConstant::getKey('CRM_Core_DAO_Phone', 'phone_type_id', 'Mobile'), "$phone.phone IS NOT NULL", "$phone.phone != ''", + "$phone.is_primary = 1", "mg.mailing_id = #mailingID", 'temp.contact_id IS null', ); - CRM_Utils_SQL_Select::from($phone) - ->select(" DISTINCT $phone.contact_id, $phone.id as phone_id ") - ->join($contact, " INNER JOIN $contact ON $phone.contact_id = $contact.id ") - ->join('gc', " INNER JOIN civicrm_group_contact gc ON $contact.id = gc.contact_id ") - ->join('mg', " INNER JOIN civicrm_mailing_group mg ON gc.group_id = mg.entity_id AND mg.entity_table = 'civicrm_group' ") - ->join('temp', " LEFT JOIN $excludeTempTablename temp ON $contact.id = temp.contact_id ") - ->where($includeFilters) - ->param('#mailingID', $mailingID) - ->replaceInto($includedTempTablename, array('contact_id', 'phone_id')) - ->execute(); } else { - // Get the group contacts, but only those which are not in the - // exclusion temp table. - CRM_Utils_SQL_Select::from($email) - ->select("$contact.id as contact_id, $email.id as email_id") - ->join($contact, " INNER JOIN $contact ON $email.contact_id = $contact.id ") - ->join('gc', " INNER JOIN civicrm_group_contact gc ON gc.contact_id = $contact.id ") + // Criterias to filter recipients that need to be included + $includeFilters = array( + "$contact.do_not_email = 0", + "$contact.is_opt_out = 0", + "$contact.is_deceased <> 1", + $location_filter, + "$email.email IS NOT NULL", + "$email.email != ''", + "mg.mailing_id = #mailingID", + 'temp.contact_id IS NULL', + ); + } + + // Get the group contacts, but only those which are not in the + // exclusion temp table. + CRM_Utils_SQL_Select::from($entityTable) + ->select("$contact.id as contact_id, $entityTable.id as $entityColumn") + ->join($contact, " INNER JOIN $contact ON $entityTable.contact_id = $contact.id ") + ->join('gc', " INNER JOIN civicrm_group_contact gc ON gc.contact_id = $contact.id ") + ->join('mg', " INNER JOIN civicrm_mailing_group mg ON gc.group_id = mg.entity_id AND mg.search_id IS NULL ") + ->join('temp', " LEFT JOIN $excludeTempTablename temp ON $contact.id = temp.contact_id ") + ->where('gc.group_id IN (#groups) AND gc.status = "Added"') + ->where($includeFilters) + ->groupBy(array("$contact.id", "$entityTable.id")) + ->replaceInto($includedTempTablename, array('contact_id', $entityColumn)) + ->param('#groups', $recipientsGroup['Include']) + ->param('#mailingID', $mailingID) + ->execute(); + + if (count($includeSmartGroupIDs)) { + $query = CRM_Utils_SQL_Select::from($contact) + ->select("$contact.id as contact_id, $entityTable.id as $entityColumn") + ->join($phone, " INNER JOIN $entityTable ON $entityTable.contact_id = $contact.id ") + ->join('gc', " INNER JOIN civicrm_group_contact_cache gc ON $contact.id = gc.contact_id ") ->join('mg', " INNER JOIN civicrm_mailing_group mg ON gc.group_id = mg.entity_id AND mg.search_id IS NULL ") ->join('temp', " LEFT JOIN $excludeTempTablename temp ON $contact.id = temp.contact_id ") - ->where('gc.group_id IN (#groups) AND gc.status = "Added"') + ->where('gc.group_id IN (#groups)') ->where($includeFilters) - ->groupBy(array("$contact.id", "$email.id")) - ->replaceInto($includedTempTablename, array('contact_id', 'email_id')) - ->param('#groups', $recipientsGroup['Include']) + ->replaceInto($includedTempTablename, array('contact_id', $entityColumn)) + ->param('#groups', $includeSmartGroupIDs) ->param('#mailingID', $mailingID) ->execute(); } - if (count($includeSmartGroupIDs)) { - if ($isSMSmode) { - CRM_Utils_SQL_Select::from($contact) - ->select("$contact.id as contact_id, $phone.id as phone_id") - ->join($phone, " INNER JOIN $phone ON $phone.contact_id = $contact.id ") - ->join('gc', " INNER JOIN civicrm_group_contact_cache gc ON $contact.id = gc.contact_id ") - ->join('temp', " LEFT JOIN $excludeTempTablename temp ON $contact.id = temp.contact_id ") - ->where('gc.group_id IN (#groups)') - ->where($includeFilters) - ->replaceInto($includedTempTablename, array('contact_id', 'phone_id')) - ->param('#groups', $includeSmartGroupIDs) - ->param('#mailingID', $mailingID) - ->execute(); - } - else { - CRM_Utils_SQL_Select::from($contact) - ->select("$contact.id as contact_id, $email.id as email_id") - ->join($email, " INNER JOIN $email ON $email.contact_id = $contact.id ") - ->join('gc', " INNER JOIN civicrm_group_contact_cache gc ON gc.contact_id = $contact.id ") - ->join('temp', " LEFT JOIN $excludeTempTablename temp ON temp.contact_id = $contact.id ") - ->where('gc.group_id IN (#groups)') - ->where($includeFilters) - ->orderBy($order_by) - ->replaceInto($includedTempTablename, array('contact_id', 'email_id')) - ->param('#groups', $includeSmartGroupIDs) - ->param('#mailingID', $mailingID) - ->execute(); - } - } - // Construct the filtered search queries. $dao = CRM_Utils_SQL_Select::from('civicrm_mailing_group') ->select('search_id, search_args, entity_id') @@ -307,7 +282,7 @@ public static function getRecipients($mailingID) { $dao->search_args, $dao->entity_id ); - $query = "REPLACE INTO {$includedTempTablename} ($tempColumn, contact_id) {$customSQL} "; + $query = "REPLACE INTO {$includedTempTablename} ($entityColumn, contact_id) {$customSQL} "; $mailingGroup->query($query); } @@ -316,16 +291,16 @@ public static function getRecipients($mailingID) { // clear all the mailing recipients before populating CRM_Core_DAO::executeQuery(" DELETE FROM civicrm_mailing_recipients WHERE mailing_id = %1 ", array(1 => array($mailingID, 'Integer'))); - $selectClause = array('#mailingID', 'i.contact_id', "i.$tempColumn"); + $selectClause = array('#mailingID', 'i.contact_id', "i.$entityColumn"); // CRM-3975 - $orderBy = array("i.contact_id", "i.$tempColumn"); + $orderBy = array("i.contact_id", "i.$entityColumn"); $query = CRM_Utils_SQL_Select::from('civicrm_contact contact_a')->join('i', " INNER JOIN {$includedTempTablename} i ON contact_a.id = i.contact_id "); - if ($mailingObj->dedupe_email) { - $orderBy = array("MIN(i.contact_id)", "MIN(i.$tempColumn)"); + if (!$isSMSmode && $mailingObj->dedupe_email) { + $orderBy = array("MIN(i.contact_id)", "MIN(i.$entityColumn)"); $query = $query->join('e', " INNER JOIN civicrm_email e ON e.id = i.email_id ")->groupBy("e.email"); if (CRM_Utils_SQL::supportsFullGroupBy()) { - $selectClause = array('#mailingID', 'ANY_VALUE(i.contact_id) contact_id', "ANY_VALUE(i.$tempColumn) $tempColumn", "e.email"); + $selectClause = array('#mailingID', 'ANY_VALUE(i.contact_id) contact_id', "ANY_VALUE(i.$entityColumn) $entityColumn", "e.email"); } } @@ -345,12 +320,12 @@ public static function getRecipients($mailingID) { $sql = $query->toSQL(); CRM_Utils_SQL_Select::from("( $sql ) AS i ") ->select($selectClause) - ->insertInto('civicrm_mailing_recipients', array('mailing_id', 'contact_id', $tempColumn)) + ->insertInto('civicrm_mailing_recipients', array('mailing_id', 'contact_id', $entityColumn)) ->param('#mailingID', $mailingID) ->execute(); } else { - $query->insertInto('civicrm_mailing_recipients', array('mailing_id', 'contact_id', $tempColumn)) + $query->insertInto('civicrm_mailing_recipients', array('mailing_id', 'contact_id', $entityColumn)) ->param('#mailingID', $mailingID) ->execute(); } diff --git a/tests/phpunit/CRM/Mailing/BAO/MailingTest.php b/tests/phpunit/CRM/Mailing/BAO/MailingTest.php index 958021bfd7d3..52a072a709a3 100644 --- a/tests/phpunit/CRM/Mailing/BAO/MailingTest.php +++ b/tests/phpunit/CRM/Mailing/BAO/MailingTest.php @@ -35,15 +35,17 @@ public function setUp() { } /** - * getRecipients test + * Test CRM_Mailing_BAO_Mailing::getRecipients() on sms mode */ public function testgetRecipients() { - // Tests for SMS bulk mailing recipients // +CRM-21320 Ensure primary mobile number is selected over non-primary // Setup - $group = $this->groupCreate(); + $smartGroupParams = array( + 'formValues' => array('contact_type' => array('IN' => array('Individual'))), + ); + $group = $this->smartGroupCreate($smartGroupParams); $sms_provider = $this->callAPISuccess('SmsProvider', 'create', array( 'sequential' => 1, 'name' => 1, @@ -54,52 +56,59 @@ public function testgetRecipients() { 'is_active' => 1, )); - // Contact 1 - $contact_1 = $this->individualCreate(array(), 0); - $contact_1_primary = $this->callAPISuccess('Phone', 'create', array( - 'contact_id' => $contact_1, - 'phone' => "01 01", - 'location_type_id' => "Home", - 'phone_type_id' => "Mobile", - 'is_primary' => 1, - )); - $contact_1_other = $this->callAPISuccess('Phone', 'create', array( - 'contact_id' => $contact_1, - 'phone' => "01 02", - 'location_type_id' => "Work", - 'phone_type_id' => "Mobile", - 'is_primary' => 0, - )); + // Create Contact 1 and add in group + $contactID1 = $this->individualCreate(array(), 0); $this->callAPISuccess('GroupContact', 'Create', array( 'group_id' => $group, - 'contact_id' => $contact_1, + 'contact_id' => $contactID1, )); - // Contact 2 - $contact_2 = $this->individualCreate(array(), 1); - $contact_2_other = $this->callAPISuccess('Phone', 'create', array( - 'contact_id' => $contact_2, - 'phone' => "02 01", - 'location_type_id' => "Home", - 'phone_type_id' => "Mobile", - 'is_primary' => 0, - )); - $contact_2_primary = $this->callAPISuccess('Phone', 'create', array( - 'contact_id' => $contact_2, - 'phone' => "02 02", - 'location_type_id' => "Work", - 'phone_type_id' => "Mobile", - 'is_primary' => 1, - )); + // Create contact 2 and add in group + $contactID2 = $this->individualCreate(array(), 1); $this->callAPISuccess('GroupContact', 'Create', array( 'group_id' => $group, - 'contact_id' => $contact_2, + 'contact_id' => $contactID2, )); + $contactIDPhoneRecords = array( + $contactID1 => array( + 'primary_phone_id' => CRM_Utils_Array::value('id', $this->callAPISuccess('Phone', 'create', array( + 'contact_id' => $contactID1, + 'phone' => "01 01", + 'location_type_id' => "Home", + 'phone_type_id' => "Mobile", + 'is_primary' => 1, + ))), + 'other_phone_id' => CRM_Utils_Array::value('id', $this->callAPISuccess('Phone', 'create', array( + 'contact_id' => $contactID1, + 'phone' => "01 02", + 'location_type_id' => "Work", + 'phone_type_id' => "Mobile", + 'is_primary' => 0, + ))), + ), + $contactID2 => array( + 'primary_phone_id' => CRM_Utils_Array::value('id', $this->callAPISuccess('Phone', 'create', array( + 'contact_id' => $contactID2, + 'phone' => "02 01", + 'location_type_id' => "Home", + 'phone_type_id' => "Mobile", + 'is_primary' => 1, + ))), + 'other_phone_id' => CRM_Utils_Array::value('id', $this->callAPISuccess('Phone', 'create', array( + 'contact_id' => $contactID2, + 'phone' => "02 02", + 'location_type_id' => "Work", + 'phone_type_id' => "Mobile", + 'is_primary' => 0, + ))), + ), + ); + // Prepare expected results - $check_ids = array( - $contact_1 => $contact_1_primary['id'], - $contact_2 => $contact_2_primary['id'], + $checkPhoneIDs = array( + $contactID1 => $contactIDPhoneRecords[$contactID1]['primary_phone_id'], + $contactID2 => $contactIDPhoneRecords[$contactID2]['primary_phone_id'], ); // Create mailing @@ -112,7 +121,7 @@ public function testgetRecipients() { )); // Populate the recipients table (job id doesn't matter) - CRM_Mailing_BAO_Mailing::getRecipients('123', $mailing['id'], TRUE, FALSE, 'sms'); + CRM_Mailing_BAO_Mailing::getRecipients($mailing['id']); // Get recipients $recipients = $this->callAPISuccess('MailingRecipients', 'get', array('mailing_id' => $mailing['id'])); @@ -122,15 +131,15 @@ public function testgetRecipients() { // Check we got the 'primary' mobile for both contacts foreach ($recipients['values'] as $value) { - $this->assertEquals($value['phone_id'], $check_ids[$value['contact_id']], 'Check correct phone number for contact ' . $value['contact_id']); + $this->assertEquals($value['phone_id'], $checkPhoneIDs[$value['contact_id']], 'Check correct phone number for contact ' . $value['contact_id']); } // Tidy up $this->deleteMailing($mailing['id']); $this->callAPISuccess('SmsProvider', 'Delete', array('id' => $sms_provider['id'])); $this->groupDelete($group); - $this->contactDelete($contact_1); - $this->contactDelete($contact_2); + $this->contactDelete($contactID1); + $this->contactDelete($contactID2); } }