From ed4c6f821425ddf87b4e691495d7bc2a87845e6a Mon Sep 17 00:00:00 2001 From: "deb.monish" Date: Mon, 16 Oct 2017 11:04:51 +0530 Subject: [PATCH] CRM-21260: CiviMail compose UI very slow to initialize, CRM-21316: Simplify getRecipients fn --- CRM/Mailing/BAO/Mailing.php | 286 +++++++++++++++++++++++++++++++---- ang/crmMailing/Recipients.js | 3 + ang/crmMailing/services.js | 18 +-- 3 files changed, 267 insertions(+), 40 deletions(-) diff --git a/CRM/Mailing/BAO/Mailing.php b/CRM/Mailing/BAO/Mailing.php index fb5b8f13e02e..f3374e0dba2c 100644 --- a/CRM/Mailing/BAO/Mailing.php +++ b/CRM/Mailing/BAO/Mailing.php @@ -109,46 +109,227 @@ public static function getRecipientsCount($job_id, $mailing_id = NULL) { } /** - * note that $job_id is used only as a variable in the temp table construction - * and does not play a role in the queries generated. - * @param int $job_id - * (misnomer) a nonce value used to name temporary tables. - * @param int $mailing_id - * @param bool $storeRecipients - * @param bool $dedupeEmail - * @param null $mode + * This function is created after simplifying the code of self::getRecipients(..) and these are the changes made + * 1. Takes mailing ID as the only parameter, and doesn't depend on mailing Job + * 2. This function ONLY fetch and store mailing recipients, doesn't handle SMS mode + * 3. Use less JOINs in underlying queries. + * 4. Simplifies code to fetch contacts from selected mailing smart group(s). + * 5. Doesn't execute query needlessly, e.g. + * a) if SG (smart groups) are selected then ONLY queries resposible to fetch contacts from SG are executed + * b) If no 'Include' groups are selected then simply return as this means no groups are selected to fetch recipients. + * 6. Refreshes smart group contact cache at once. + * + * @param CRM_Mailing_DAO_Mailing $mailingObj * - * @return CRM_Mailing_Event_BAO_Queue|string + * @return void */ - public static function getRecipients( - $job_id, - $mailing_id = NULL, - $storeRecipients = FALSE, - $dedupeEmail = FALSE, - $mode = NULL) { + public static function getMailRecipients($mailingObj) { + $mailing = CRM_Mailing_BAO_Mailing::getTableName(); + $mailingID = $mailingObj->id; + $mailingGroup = new CRM_Mailing_DAO_MailingGroup(); + $recipientsGroup = $excludeSmartGroupIDs = $includeSmartGroupIDs = array(); + $sql = " SELECT GROUP_CONCAT(entity_id SEPARATOR \",\") as group_ids, group_type + FROM civicrm_mailing_group + WHERE mailing_id = %1 AND entity_table = 'civicrm_group' + GROUP BY group_type + "; + $dao = CRM_Core_DAO::executeQuery($sql, array(1 => array($mailingID, 'Int'))); + while ($dao->fetch()) { + $recipientsGroup[$dao->group_type] = explode(',', $dao->group_ids); + } - $mailing = CRM_Mailing_BAO_Mailing::getTableName(); - $job = CRM_Mailing_BAO_MailingJob::getTableName(); - $mg = CRM_Mailing_DAO_MailingGroup::getTableName(); - $eq = CRM_Mailing_Event_DAO_Queue::getTableName(); + // there is no need to proceed further as 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'])) { + $sql = " DELETE FROM civicrm_mailing_recipients WHERE mailing_id = %1 "; + CRM_Core_DAO::executeQuery($sql, array(1 => array($mailingID, 'Integer'))); + return; + } + + list($location_filter, $order_by) = self::getLocationFilterAndOrderBy($mailingObj->email_selection_method, $mailingObj->location_type_id); + + // get all the saved searches AND hierarchical groups + // and load them in the cache + foreach ($recipientsGroup as $groupType => $groupIDs) { + $sql = sprintf(" + SELECT * + FROM civicrm_group g + WHERE id IN (%s) AND ( saved_search_id != 0 OR saved_search_id IS NOT NULL OR children IS NOT NULL ) + ", implode(',', $groupIDs)); + + $groupDAO = CRM_Core_DAO::executeQuery($sql); + while ($groupDAO->fetch()) { + if ($groupDAO->cache_date == NULL) { + CRM_Contact_BAO_GroupContactCache::load($groupDAO); + } + if ($groupType == 'Include') { + $includeSmartGroupIDs[] = $groupDAO->id; + } + else { + $excludeSmartGroupIDs[] = $groupDAO->id; + } + } + } + + // Create a temp table for contact exclusion. + $excludeTempTablename = "excluded_recipients_mailing_temp"; + $includedTempTablename = "included_recipients_mailing_temp"; + $mailingGroup->query( + "CREATE TEMPORARY TABLE $excludeTempTablename + (contact_id int primary key) + ENGINE=HEAP" + ); + if (!empty($recipientsGroup['Exclude'])) { + $sql = sprintf(" + INSERT INTO %s (contact_id) + SELECT DISTINCT gc.contact_id + FROM civicrm_group_contact gc + WHERE gc.status = 'Added' AND gc.id IN (%s)", $excludeTempTablename, implode(',', $recipientsGroup['Exclude'])); + $mailingGroup->query($sql); + + if (count($excludeSmartGroupIDs)) { + $sql = sprintf(" + INSERT IGNORE INTO %s (contact_id) + SELECT c.contact_id + FROM civicrm_group_contact_cache c + WHERE c.group_id IN (%s) + ", $excludeTempTablename, implode(',', $excludeSmartGroupIDs)); + $mailingGroup->query($sql); + } + } + + if (!empty($recipientsGroup['Base'])) { + $sql = sprintf(" + INSERT IGNORE INTO %s (contact_id) + SELECT DISTINCT contact_id + FROM civicrm_group_contact gc + WHERE status = 'Removed' AND group_id IN (%s)", $excludeTempTablename, implode(',', $recipientsGroup['Base'])); + $mailingGroup->query($sql); + } + + // Add all the (intended) recipients of an excluded prior mailing to + // the temp table. + $excludeSubMailing = sprintf(" + INSERT IGNORE INTO %s (contact_id) + SELECT DISTINCT meq.contact_id + FROM civicrm_mailing_event_queue meq + INNER JOIN civicrm_mailing_job mj ON meq.job_id = mj.id + INNER JOIN civicrm_mailing_group mg ON mj.mailing_id = mg.entity_id AND mg.entity_table = 'civicrm_mailing' + WHERE mg.mailing_id = %d AND mg.group_type = 'Exclude'", $excludeTempTablename, $mailingID); + $mailingGroup->query($excludeSubMailing); $email = CRM_Core_DAO_Email::getTableName(); - if ($mode == 'sms') { - $phone = CRM_Core_DAO_Phone::getTableName(); + // Get all the group contacts we want to include. + $mailingGroup->query( + "CREATE TEMPORARY TABLE $includedTempTablename + (email_id int, contact_id int primary key) + ENGINE=HEAP" + ); + + // Criterias to filter recipients that need to be included + $includeFilters = array( + 'c.do_not_email = 0', + 'c.is_opt_out = 0', + 'c.is_deceased <> 1', + $location_filter, + "$email.email IS NOT NULL", + "$email.email != ''", + 'temp.contact_id IS null', + ); + // Get the group contacts, but only those which are not in the + // exclusion temp table. + $sql = sprintf("REPLACE INTO %s (email_id, contact_id) + SELECT $email.id as email_id, c.id as contact_id + FROM $email + INNER JOIN civicrm_contact c ON $email.contact_id = c.id + INNER JOIN civicrm_group_contact gc ON gc.contact_id = c.id + INNER JOIN civicrm_mailing_group mg ON gc.group_id = mg.entity_id AND mg.search_id IS NULL + LEFT JOIN %s temp ON c.id = temp.contact_id + WHERE gc.group_id IN (%s) AND gc.status = 'Added' AND %s + GROUP BY $email.id, c.id + %s", $includedTempTablename, $excludeTempTablename, implode(',', $recipientsGroup['Include']), implode(' AND ', $includeFilters), $order_by); + $mailingGroup->query($sql); + + if (count($includeSmartGroupIDs)) { + unset($includeFilters["gc.status = 'Added'"]); + $sql = sprintf("REPLACE INTO %s (email_id, contact_id) + SELECT civicrm_email.id as email_id, c.id as contact_id + FROM civicrm_contact c + INNER JOIN civicrm_email ON civicrm_email.contact_id = c.id + INNER JOIN civicrm_group_contact_cache gc ON gc.contact_id = c.id + LEFT JOIN %s temp ON temp.contact_id = c.id + WHERE gc.group_id IN (%s) AND %s %s ", $includedTempTablename, $excludeTempTablename, implode(',', $includeSmartGroupIDs), implode(' AND ', $includeFilters), $order_by); + $mailingGroup->query($sql); } - $contact = CRM_Contact_DAO_Contact::getTableName(); - $group = CRM_Contact_DAO_Group::getTableName(); - $g2contact = CRM_Contact_DAO_GroupContact::getTableName(); + // Construct the filtered search queries. + $query = " SELECT search_id, search_args, entity_id + FROM civicrm_mailing_group + WHERE search_id IS NOT NULL AND mailing_id = {$mailingID}"; + $dao = CRM_Core_DAO::executeQuery($query); + while ($dao->fetch()) { + $customSQL = CRM_Contact_BAO_SearchCustom::civiMailSQL($dao->search_id, + $dao->search_args, + $dao->entity_id + ); + $query = "REPLACE INTO {$includedTempTablename} (email_id, contact_id) {$customSQL} "; + $mailingGroup->query($query); + } - $m = new CRM_Mailing_DAO_Mailing(); - $m->id = $mailing_id; - $m->find(TRUE); + list($aclFrom, $aclWhere) = CRM_Contact_BAO_Contact_Permission::cacheClause(); + $aclWhere = $aclWhere ? "WHERE {$aclWhere}" : ''; + + // clear all the mailing recipients before populating + $sql = " DELETE FROM civicrm_mailing_recipients WHERE mailing_id = %1 "; + CRM_Core_DAO::executeQuery($sql, array(1 => array($mailingID, 'Integer'))); + + $selectClause = array('%1', 'i.contact_id', "i.email_id"); + $select = "SELECT " . implode(', ', $selectClause); + // CRM-3975 + $groupBy = $groupJoin = ''; + $orderBy = "i.contact_id, i.email_id"; + if ($mailingObj->dedupe_email) { + $orderBy = "MIN(i.contact_id), MIN(i.email_id)"; + $groupJoin = " INNER JOIN civicrm_email e ON e.id = i.email_id"; + $groupBy = " GROUP BY e.email "; + $select = CRM_Contact_BAO_Query::appendAnyValueToSelect($selectClause, 'e.email'); + } + + $sql = " INSERT INTO civicrm_mailing_recipients ( mailing_id, contact_id, email_id ) + {$select} + FROM civicrm_contact contact_a + INNER JOIN {$includedTempTablename} i ON contact_a.id = i.contact_id + {$groupJoin} + {$aclFrom} + {$aclWhere} + $groupBy + ORDER BY {$orderBy} + "; + CRM_Core_DAO::executeQuery($sql, array(1 => array($mailingID, 'Integer'))); + + // if we need to add all emails marked bulk, do it as a post filter + // on the mailing recipients table + if (CRM_Core_BAO_Email::isMultipleBulkMail()) { + self::addMultipleEmails($mailingID); + } - $email_selection_method = $m->email_selection_method; - $location_type_id = $m->location_type_id; + // Delete the temp table. + $mailingGroup->reset(); + $mailingGroup->query(" DROP TEMPORARY TABLE $excludeTempTablename "); + $mailingGroup->query(" DROP TEMPORARY TABLE $includedTempTablename "); + } + /** + * Function to retrieve location filter and order by clause later used by SQL query that is used to fetch and include mailing recipients + * + * @param string $email_selection_method + * @param int $location_type_id + * + * @return array + */ + public static function getLocationFilterAndOrderBy($email_selection_method, $location_type_id) { + $email = CRM_Core_DAO_Email::getTableName(); // Note: When determining the ORDER that results are returned, it's // the record that comes last that counts. That's because we are // INSERT'ing INTO a table with a primary id so that last record @@ -190,6 +371,49 @@ public static function getRecipients( $order_by = "ORDER BY $email.is_bulkmail"; } + return array($location_filter, $order_by); + } + + /** + * note that $job_id is used only as a variable in the temp table construction + * and does not play a role in the queries generated. + * @param int $job_id + * (misnomer) a nonce value used to name temporary tables. + * @param int $mailing_id + * @param bool $storeRecipients + * @param bool $dedupeEmail + * @param null $mode + * + * @return CRM_Mailing_Event_BAO_Queue|string + */ + public static function getRecipients( + $job_id, + $mailing_id = NULL, + $storeRecipients = FALSE, + $dedupeEmail = FALSE, + $mode = NULL) { + $mailingGroup = new CRM_Mailing_DAO_MailingGroup(); + + $mailing = CRM_Mailing_BAO_Mailing::getTableName(); + $job = CRM_Mailing_BAO_MailingJob::getTableName(); + $mg = CRM_Mailing_DAO_MailingGroup::getTableName(); + $eq = CRM_Mailing_Event_DAO_Queue::getTableName(); + + $email = CRM_Core_DAO_Email::getTableName(); + if ($mode == 'sms') { + $phone = CRM_Core_DAO_Phone::getTableName(); + } + $contact = CRM_Contact_DAO_Contact::getTableName(); + + $group = CRM_Contact_DAO_Group::getTableName(); + $g2contact = CRM_Contact_DAO_GroupContact::getTableName(); + + $m = new CRM_Mailing_DAO_Mailing(); + $m->id = $mailing_id; + $m->find(TRUE); + + list($location_filter, $order_by) = self::getLocationFilterAndOrderBy($m->email_selection_method, $m->location_type_id); + // Create a temp table for contact exclusion. $mailingGroup->query( "CREATE TEMPORARY TABLE X_$job_id @@ -1803,6 +2027,10 @@ public static function create(&$params, $ids = array()) { $job->scheduled_date = $params['scheduled_date']; $job->save(); } + elseif (!empty($params['get_recipents'])) { + //self::getRecipients(1, $mailing->id, TRUE, $mailing->dedupe_email, $mode); + self::getMailRecipients($mailing); + } return $mailing; } diff --git a/ang/crmMailing/Recipients.js b/ang/crmMailing/Recipients.js index dfb70b349314..620c01b4750a 100644 --- a/ang/crmMailing/Recipients.js +++ b/ang/crmMailing/Recipients.js @@ -145,6 +145,9 @@ mids.push(dv.entity_id); } } + if (gids.length === 0) { + return; + } CRM.api3('Group', 'getlist', { params: { id: { IN: gids } }, extra: ["is_hidden"] }).then( function(glist) { diff --git a/ang/crmMailing/services.js b/ang/crmMailing/services.js index a4de0983442b..78eeff9701b0 100644 --- a/ang/crmMailing/services.js +++ b/ang/crmMailing/services.js @@ -290,11 +290,9 @@ previewRecipients: function previewRecipients(mailing, previewLimit) { // To get list of recipients, we tentatively save the mailing and // get the resulting recipients -- then rollback any changes. - var params = angular.extend({}, mailing, mailing.recipients, { - name: 'placeholder', // for previewing recipients on new, incomplete mailing - subject: 'placeholder', // for previewing recipients on new, incomplete mailing - options: {force_rollback: 1}, - 'api.mailing_job.create': 1, // note: exact match to API default + var params = angular.extend({}, mailing.recipients, { + id: mailing.id, + get_recipents: 1, 'api.MailingRecipients.get': { mailing_id: '$value.id', options: {limit: previewLimit}, @@ -313,11 +311,9 @@ previewRecipientCount: function previewRecipientCount(mailing) { // To get list of recipients, we tentatively save the mailing and // get the resulting recipients -- then rollback any changes. - var params = angular.extend({}, mailing, mailing.recipients, { - name: 'placeholder', // for previewing recipients on new, incomplete mailing - subject: 'placeholder', // for previewing recipients on new, incomplete mailing - options: {force_rollback: 1}, - 'api.mailing_job.create': 1, // note: exact match to API default + var params = angular.extend({}, mailing.recipients, { + id: mailing.id, + get_recipents: 1, 'api.MailingRecipients.getcount': { mailing_id: '$value.id' } @@ -517,7 +513,7 @@ return crmLegacy.url('civicrm/contact/search/advanced', 'force=1&mailing_id=' + mailing.id + statType.searchFilter); case 'report': - var reportIds = CRM.crmMailing.reportIds; + var reportIds = CRM.crmMailing.reportIds; return crmLegacy.url('civicrm/report/instance/' + reportIds[statType.reportType], 'reset=1&mailing_id_value=' + mailing.id + statType.reportFilter); default: