Skip to content

Commit

Permalink
additional fixes and optimisation
Browse files Browse the repository at this point in the history
  • Loading branch information
@monishdeb
monishdeb committed Feb 2, 2018
1 parent 31a5c38 commit e8a785a
Show file tree
Hide file tree
Showing 2 changed files with 103 additions and 119 deletions.
125 changes: 50 additions & 75 deletions CRM/Mailing/BAO/Mailing.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand All @@ -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);
}

Expand All @@ -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");
}
}

Expand All @@ -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();
}
Expand Down
97 changes: 53 additions & 44 deletions tests/phpunit/CRM/Mailing/BAO/MailingTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
Expand All @@ -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']));
Expand All @@ -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);
}

}

0 comments on commit e8a785a

Please sign in to comment.