From f7ff8a704676d7a07b8fe4d1ca12100a1a8e985a Mon Sep 17 00:00:00 2001 From: "deb.monish" Date: Fri, 19 Jan 2018 23:16:20 +0530 Subject: [PATCH 1/4] CRM-21316: Add missing code to handle smart group on sms mode --- CRM/Mailing/BAO/Mailing.php | 69 +++++++++++++++++++++++-------------- 1 file changed, 44 insertions(+), 25 deletions(-) diff --git a/CRM/Mailing/BAO/Mailing.php b/CRM/Mailing/BAO/Mailing.php index 26659f55912c..5c284a5a38b1 100644 --- a/CRM/Mailing/BAO/Mailing.php +++ b/CRM/Mailing/BAO/Mailing.php @@ -122,6 +122,7 @@ public static function getRecipients($mailingID) { $mailing = CRM_Mailing_BAO_Mailing::getTableName(); $contact = CRM_Contact_DAO_Contact::getTableName(); + $phone = CRM_Core_DAO_Phone::getTableName(); $isSMSmode = (!CRM_Utils_System::isNull($mailingObj->sms_provider_id)); $mailingGroup = new CRM_Mailing_DAO_MailingGroup(); @@ -218,30 +219,31 @@ public static function getRecipients($mailingID) { $location_filter, "$email.email IS NOT NULL", "$email.email != ''", + "mg.mailing_id = #mailingID", 'temp.contact_id IS NULL', ); if ($isSMSmode) { - $phone = CRM_Core_DAO_Phone::getTableName(); + $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 IS NOT NULL", + "$phone.phone != ''", + "mg.mailing_id = #mailingID", + 'temp.contact_id IS null', + ); CRM_Utils_SQL_Select::from($phone) ->select(" DISTINCT $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(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 IS NOT NULL", - "$phone.phone != ''", - "mg.mailing_id = #mailingID", - 'temp.contact_id IS null', - )) + ->where($includeFilters) ->param('#mailingID', $mailingID) ->replaceInto($includedTempTablename, array('contact_id', 'phone_id')) ->execute(); @@ -260,21 +262,38 @@ public static function getRecipients($mailingID) { ->groupBy(array("$contact.id", "$email.id")) ->replaceInto($includedTempTablename, array('contact_id', 'email_id')) ->param('#groups', $recipientsGroup['Include']) + ->param('#mailingID', $mailingID) ->execute(); } if (count($includeSmartGroupIDs)) { - 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) - ->execute(); + 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. From e624bc9e6b469fc2c5466f6c01a6427fc5fb7788 Mon Sep 17 00:00:00 2001 From: Michael McAndrew Date: Fri, 26 Jan 2018 12:16:43 +0000 Subject: [PATCH 2/4] fix select SMS recipients query --- CRM/Mailing/BAO/Mailing.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CRM/Mailing/BAO/Mailing.php b/CRM/Mailing/BAO/Mailing.php index 5c284a5a38b1..08ee1f5b5a9b 100644 --- a/CRM/Mailing/BAO/Mailing.php +++ b/CRM/Mailing/BAO/Mailing.php @@ -238,7 +238,7 @@ public static function getRecipients($mailingID) { 'temp.contact_id IS null', ); CRM_Utils_SQL_Select::from($phone) - ->select(" DISTINCT $phone.id as phone_id ") + ->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' ") From 9f0a25d7b7b9efbdfe2ed64e3ad5113174111048 Mon Sep 17 00:00:00 2001 From: JKingsnorth Date: Wed, 18 Oct 2017 15:36:16 +0100 Subject: [PATCH 3/4] CRM-21320 Test for mass SMS recipients --- tests/phpunit/CRM/Mailing/BAO/MailingTest.php | 136 ++++++++++++++++++ 1 file changed, 136 insertions(+) create mode 100644 tests/phpunit/CRM/Mailing/BAO/MailingTest.php diff --git a/tests/phpunit/CRM/Mailing/BAO/MailingTest.php b/tests/phpunit/CRM/Mailing/BAO/MailingTest.php new file mode 100644 index 000000000000..958021bfd7d3 --- /dev/null +++ b/tests/phpunit/CRM/Mailing/BAO/MailingTest.php @@ -0,0 +1,136 @@ +groupCreate(); + $sms_provider = $this->callAPISuccess('SmsProvider', 'create', array( + 'sequential' => 1, + 'name' => 1, + 'title' => "Test", + 'username' => "Test", + 'password' => "Test", + 'api_type' => 1, + '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, + )); + $this->callAPISuccess('GroupContact', 'Create', array( + 'group_id' => $group, + 'contact_id' => $contact_1, + )); + + // 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, + )); + $this->callAPISuccess('GroupContact', 'Create', array( + 'group_id' => $group, + 'contact_id' => $contact_2, + )); + + // Prepare expected results + $check_ids = array( + $contact_1 => $contact_1_primary['id'], + $contact_2 => $contact_2_primary['id'], + ); + + // Create mailing + $mailing = $this->callAPISuccess('Mailing', 'create', array('sms_provider_id' => $sms_provider['id'])); + $mailing_include_group = $this->callAPISuccess('MailingGroup', 'create', array( + 'mailing_id' => $mailing['id'], + 'group_type' => "Include", + 'entity_table' => "civicrm_group", + 'entity_id' => $group, + )); + + // Populate the recipients table (job id doesn't matter) + CRM_Mailing_BAO_Mailing::getRecipients('123', $mailing['id'], TRUE, FALSE, 'sms'); + + // Get recipients + $recipients = $this->callAPISuccess('MailingRecipients', 'get', array('mailing_id' => $mailing['id'])); + + // Check the count is correct + $this->assertEquals(2, $recipients['count'], 'Check recipient count'); + + // 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']); + } + + // 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); + } + +} From 6f3a35e0986bc21ffbf730c7d3f20f2894c1bb58 Mon Sep 17 00:00:00 2001 From: "deb.monish" Date: Fri, 2 Feb 2018 21:25:31 +0530 Subject: [PATCH 4/4] additional fixes and optimisation --- CRM/Mailing/BAO/Mailing.php | 160 +++++++++--------- ang/crmMailing/Recipients.js | 7 +- tests/phpunit/CRM/Mailing/BAO/MailingTest.php | 97 ++++++----- 3 files changed, 140 insertions(+), 124 deletions(-) diff --git a/CRM/Mailing/BAO/Mailing.php b/CRM/Mailing/BAO/Mailing.php index 08ee1f5b5a9b..cbf740667841 100644 --- a/CRM/Mailing/BAO/Mailing.php +++ b/CRM/Mailing/BAO/Mailing.php @@ -122,24 +122,28 @@ public static function getRecipients($mailingID) { $mailing = CRM_Mailing_BAO_Mailing::getTableName(); $contact = CRM_Contact_DAO_Contact::getTableName(); - $phone = CRM_Core_DAO_Phone::getTableName(); $isSMSmode = (!CRM_Utils_System::isNull($mailingObj->sms_provider_id)); $mailingGroup = new CRM_Mailing_DAO_MailingGroup(); - $recipientsGroup = $excludeSmartGroupIDs = $includeSmartGroupIDs = array(); + $recipientsGroup = $excludeSmartGroupIDs = $includeSmartGroupIDs = $priorMailingIDs = array(); $dao = CRM_Utils_SQL_Select::from('civicrm_mailing_group') - ->select('GROUP_CONCAT(entity_id SEPARATOR ",") as group_ids, group_type') - ->where('mailing_id = #mailing_id AND entity_table = "civicrm_group"') - ->groupBy('group_type') + ->select('GROUP_CONCAT(entity_id SEPARATOR ",") as group_ids, group_type, entity_table') + ->where('mailing_id = #mailing_id AND entity_table IN ("civicrm_group", "civicrm_mailing")') + ->groupBy(array('group_type', 'entity_table')) ->param('#mailing_id', $mailingID) ->execute(); while ($dao->fetch()) { - $recipientsGroup[$dao->group_type] = explode(',', $dao->group_ids); + if ($dao->entity_table == 'civicrm_mailing') { + $priorMailingIDs[$dao->group_type] = explode(',', $dao->group_ids); + } + else { + $recipientsGroup[$dao->group_type] = explode(',', $dao->group_ids); + } } // there is no need to proceed further if no mailing group is selected to include recipients, // but before return clear the mailing recipients populated earlier since as per current params no group is selected - if (empty($recipientsGroup['Include'])) { + if (empty($recipientsGroup['Include']) && empty($priorMailingIDs['Include'])) { CRM_Core_DAO::executeQuery(" DELETE FROM civicrm_mailing_recipients WHERE mailing_id = %1 ", array(1 => array($mailingID, 'Integer'))); return; } @@ -175,6 +179,8 @@ public static function getRecipients($mailingID) { (contact_id int primary key) ENGINE=HEAP" ); + // populate exclude temp-table with recipients to be excluded from the list + // on basis of selected recipients groups and/or previous mailing if (!empty($recipientsGroup['Exclude'])) { CRM_Utils_SQL_Select::from('civicrm_group_contact') ->select('DISTINCT contact_id') @@ -192,6 +198,14 @@ public static function getRecipients($mailingID) { ->execute(); } } + if (!empty($priorMailingIDs['Exclude'])) { + CRM_Utils_SQL_Select::from('civicrm_mailing_recipients') + ->select('DISTINCT contact_id') + ->where('mailing_id IN (#mailings)') + ->param('#mailings', $priorMailingIDs['Exclude']) + ->insertIgnoreInto($excludeTempTablename, array('contact_id')) + ->execute(); + } if (!empty($recipientsGroup['Base'])) { CRM_Utils_SQL_Select::from('civicrm_group_contact') @@ -202,98 +216,86 @@ public static function getRecipients($mailingID) { ->execute(); } - $tempColumn = $isSMSmode ? 'phone_id' : 'email_id'; - $email = CRM_Core_DAO_Email::getTableName(); + $entityColumn = $isSMSmode ? 'phone_id' : 'email_id'; + $entityTable = $isSMSmode ? CRM_Core_DAO_Phone::getTableName() : 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 IS NOT NULL", - "$phone.phone != ''", + "$entityTable.phone_type_id = " . CRM_Core_PseudoConstant::getKey('CRM_Core_DAO_Phone', 'phone_type_id', 'Mobile'), + "$entityTable.phone IS NOT NULL", + "$entityTable.phone != ''", + "$entityTable.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(); + $order_by = array("$entityTable.is_primary = 1"); } 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 ") + // 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, + "$entityTable.email IS NOT NULL", + "$entityTable.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. + if (!empty($recipientsGroup['Include'])) { + 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", "$email.id")) - ->replaceInto($includedTempTablename, array('contact_id', 'email_id')) + ->groupBy(array("$contact.id", "$entityTable.id")) + ->replaceInto($includedTempTablename, array('contact_id', $entityColumn)) ->param('#groups', $recipientsGroup['Include']) ->param('#mailingID', $mailingID) ->execute(); } + // Get recipients selected in prior mailings + if (!empty($priorMailingIDs['Include'])) { + CRM_Utils_SQL_Select::from('civicrm_mailing_recipients') + ->select("contact_id, $entityColumn") + ->where('mailing_id IN (#mailings)') + ->param('#mailings', $priorMailingIDs['Include']) + ->insertIgnoreInto($includedTempTablename, array('contact_id', $entityColumn)) + ->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(); - } + $query = CRM_Utils_SQL_Select::from($contact) + ->select("$contact.id as contact_id, $entityTable.id as $entityColumn") + ->join($entityTable, " 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)') + ->where($includeFilters) + ->orderBy($order_by) + ->replaceInto($includedTempTablename, array('contact_id', $entityColumn)) + ->param('#groups', $includeSmartGroupIDs) + ->param('#mailingID', $mailingID) + ->execute(); } // Construct the filtered search queries. @@ -307,7 +309,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 +318,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 +347,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/ang/crmMailing/Recipients.js b/ang/crmMailing/Recipients.js index 79f19d78779e..ceae2db8712d 100644 --- a/ang/crmMailing/Recipients.js +++ b/ang/crmMailing/Recipients.js @@ -145,8 +145,13 @@ mids.push(dv.entity_id); } } + // push non existant 0 group/mailing id in order when no recipents group or prior mailing is selected + // this will allow to resuse the below code to handle datamap if (gids.length === 0) { - return; + gids.push(0); + } + if (mids.length === 0) { + mids.push(0); } CRM.api3('Group', 'getlist', { params: { id: { IN: gids }, options: { limit: 0 } }, extra: ["is_hidden"] } ).then( diff --git a/tests/phpunit/CRM/Mailing/BAO/MailingTest.php b/tests/phpunit/CRM/Mailing/BAO/MailingTest.php index 958021bfd7d3..998869a99ae0 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); } }