From d96f7984ff41c1ba9ef8b7e061ac3d82405c8af5 Mon Sep 17 00:00:00 2001 From: Eileen McNaughton Date: Mon, 26 Jul 2021 13:56:41 +1200 Subject: [PATCH 1/3] [REF] start unravelling the way we retrieve the saved search We have 3 types of saved searches - search kit - legacy core searches - legacy custom searches The only information these 3 need to load is the savedSearch details and the group ID (to add in the add & exclude). The wrangling of the params should happen in the getSql functions - which we can think about being in a listener once they have standard inputs & outputs. However, to get to that point we want to standardise those inputs & outputs. This removes only point of randomness - ie savedSearch has a consistent value & the wrangling of what is in it is pushed closer to the relevant functions --- CRM/Contact/BAO/GroupContactCache.php | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/CRM/Contact/BAO/GroupContactCache.php b/CRM/Contact/BAO/GroupContactCache.php index 740ba9b704fb..47cfbdcd2d6b 100644 --- a/CRM/Contact/BAO/GroupContactCache.php +++ b/CRM/Contact/BAO/GroupContactCache.php @@ -13,6 +13,7 @@ use Civi\Api4\Group; use Civi\Api4\Query\Api4SelectQuery; use Civi\Api4\Query\SqlExpression; +use Civi\Api4\SavedSearch; /** * @@ -796,7 +797,10 @@ private static function updateCacheFromTempTable(CRM_Utils_SQL_TempTable $groupC */ protected static function insertGroupContactsIntoTempTable(string $tempTableName, int $groupID, ?int $savedSearchID, ?string $children): void { if ($savedSearchID) { - $ssParams = CRM_Contact_BAO_SavedSearch::getSearchParams($savedSearchID); + $savedSearch = SavedSearch::get(FALSE) + ->addWhere('id', '=', $savedSearchID) + ->execute() + ->first(); $excludeClause = "NOT IN ( SELECT contact_id FROM civicrm_group_contact @@ -804,10 +808,22 @@ protected static function insertGroupContactsIntoTempTable(string $tempTableName AND civicrm_group_contact.group_id = $groupID )"; $addSelect = "$groupID AS group_id"; - if (!empty($ssParams['api_entity'])) { - $sql = self::getApiSQL($ssParams, $addSelect, $excludeClause); + if (!empty($savedSearch['api_entity'])) { + $sql = self::getApiSQL($savedSearch, $addSelect, $excludeClause); } else { + $fv = CRM_Contact_BAO_SavedSearch::getFormValues($savedSearchID); + //check if the saved search has mapping id + if ($savedSearch['mapping_id']) { + $ssParams = CRM_Core_BAO_Mapping::formattedFields($fv); + } + elseif (!empty($fv['customSearchID'])) { + $ssParams = $fv; + } + else { + $ssParams = CRM_Contact_BAO_Query::convertFormValues($fv); + } + // CRM-7021 rectify params to what proximity search expects if there is a value for prox_distance if (!empty($ssParams)) { CRM_Contact_BAO_ProximityQuery::fixInputParams($ssParams); From c3f7cb2f33ddd8d840efaf2a6f7122db1cd84756 Mon Sep 17 00:00:00 2001 From: Eileen McNaughton Date: Mon, 26 Jul 2021 14:10:46 +1200 Subject: [PATCH 2/3] [REF] Further divide savedSearchParam loading into the sql functions --- CRM/Contact/BAO/GroupContactCache.php | 51 ++++++++++++++------------- 1 file changed, 27 insertions(+), 24 deletions(-) diff --git a/CRM/Contact/BAO/GroupContactCache.php b/CRM/Contact/BAO/GroupContactCache.php index 47cfbdcd2d6b..bb19902eddc6 100644 --- a/CRM/Contact/BAO/GroupContactCache.php +++ b/CRM/Contact/BAO/GroupContactCache.php @@ -554,14 +554,19 @@ protected static function getApiSQL(array $savedSearch, string $addSelect, strin * so temp tables are not destroyed if they are used * * @param int $savedSearchID - * @param array $ssParams + * @param array $savedSearch * @param string $addSelect * @param string $excludeClause * * @return string * @throws CRM_Core_Exception */ - protected static function getCustomSearchSQL($savedSearchID, array $ssParams, string $addSelect, string $excludeClause) { + protected static function getCustomSearchSQL($savedSearchID, array $savedSearch, string $addSelect, string $excludeClause) { + $ssParams = CRM_Contact_BAO_SavedSearch::getFormValues($savedSearchID); + // CRM-7021 rectify params to what proximity search expects if there is a value for prox_distance + if (!empty($ssParams)) { + CRM_Contact_BAO_ProximityQuery::fixInputParams($ssParams); + } $searchSQL = CRM_Contact_BAO_SearchCustom::customClass($ssParams['customSearchID'], $savedSearchID)->contactIDs(); $searchSQL = str_replace('ORDER BY contact_a.id ASC', '', $searchSQL); if (strpos($searchSQL, 'WHERE') === FALSE) { @@ -577,7 +582,7 @@ protected static function getCustomSearchSQL($savedSearchID, array $ssParams, st * Get array of sql from a saved query object group. * * @param int $savedSearchID - * @param array $ssParams + * @param array $savedSearch * @param string $addSelect * @param string $excludeClause * @@ -585,7 +590,20 @@ protected static function getCustomSearchSQL($savedSearchID, array $ssParams, st * @throws \CRM_Core_Exception * @throws \CiviCRM_API3_Exception */ - protected static function getQueryObjectSQL($savedSearchID, array $ssParams, string $addSelect, string $excludeClause) { + protected static function getQueryObjectSQL($savedSearchID, array $savedSearch, string $addSelect, string $excludeClause): string { + $fv = CRM_Contact_BAO_SavedSearch::getFormValues($savedSearchID); + //check if the saved search has mapping id + if ($savedSearch['mapping_id']) { + $ssParams = CRM_Core_BAO_Mapping::formattedFields($fv); + } + else { + $ssParams = CRM_Contact_BAO_Query::convertFormValues($fv); + } + // CRM-7021 rectify params to what proximity search expects if there is a value for prox_distance + if (!empty($ssParams)) { + CRM_Contact_BAO_ProximityQuery::fixInputParams($ssParams); + } + $returnProperties = NULL; if (CRM_Core_DAO::getFieldValue('CRM_Contact_DAO_SavedSearch', $savedSearchID, 'mapping_id')) { $fv = CRM_Contact_BAO_SavedSearch::getFormValues($savedSearchID); @@ -799,6 +817,7 @@ protected static function insertGroupContactsIntoTempTable(string $tempTableName if ($savedSearchID) { $savedSearch = SavedSearch::get(FALSE) ->addWhere('id', '=', $savedSearchID) + ->addSelect('*') ->execute() ->first(); @@ -808,31 +827,15 @@ protected static function insertGroupContactsIntoTempTable(string $tempTableName AND civicrm_group_contact.group_id = $groupID )"; $addSelect = "$groupID AS group_id"; - if (!empty($savedSearch['api_entity'])) { + if ($savedSearch['api_entity']) { $sql = self::getApiSQL($savedSearch, $addSelect, $excludeClause); } else { - $fv = CRM_Contact_BAO_SavedSearch::getFormValues($savedSearchID); - //check if the saved search has mapping id - if ($savedSearch['mapping_id']) { - $ssParams = CRM_Core_BAO_Mapping::formattedFields($fv); - } - elseif (!empty($fv['customSearchID'])) { - $ssParams = $fv; - } - else { - $ssParams = CRM_Contact_BAO_Query::convertFormValues($fv); - } - - // CRM-7021 rectify params to what proximity search expects if there is a value for prox_distance - if (!empty($ssParams)) { - CRM_Contact_BAO_ProximityQuery::fixInputParams($ssParams); - } - if (isset($ssParams['customSearchID'])) { - $sql = self::getCustomSearchSQL($savedSearchID, $ssParams, $addSelect, $excludeClause); + if (!empty($savedSearch['form_values']['customSearchID'])) { + $sql = self::getCustomSearchSQL($savedSearchID, $savedSearch, $addSelect, $excludeClause); } else { - $sql = self::getQueryObjectSQL($savedSearchID, $ssParams, $addSelect, $excludeClause); + $sql = self::getQueryObjectSQL($savedSearchID, $savedSearch, $addSelect, $excludeClause); } } } From 1b29c899ac95ba43e72a7b56883b1f29f4a4d6e3 Mon Sep 17 00:00:00 2001 From: Eileen McNaughton Date: Mon, 26 Jul 2021 16:30:19 +1200 Subject: [PATCH 3/3] [Ref] simplify passed parameters I think if we want to make this some sort of hook or listener we don't really want to pass out 'add select' and 'exclude clause' when they are just group id with a bit of string. It seems cleaner to specify the sql within the function --- CRM/Contact/BAO/GroupContactCache.php | 61 +++++++++++++++------------ 1 file changed, 34 insertions(+), 27 deletions(-) diff --git a/CRM/Contact/BAO/GroupContactCache.php b/CRM/Contact/BAO/GroupContactCache.php index bb19902eddc6..1f3d30c8a6be 100644 --- a/CRM/Contact/BAO/GroupContactCache.php +++ b/CRM/Contact/BAO/GroupContactCache.php @@ -510,23 +510,29 @@ public static function getCacheInvalidDateTime(): string { * @param int $groupID - Group to invalidate */ public static function invalidateGroupContactCache($groupID): void { - CRM_Core_DAO::executeQuery("UPDATE civicrm_group + CRM_Core_DAO::executeQuery('UPDATE civicrm_group SET cache_date = NULL - WHERE id = %1 AND (saved_search_id IS NOT NULL OR children IS NOT NULL)", [ + WHERE id = %1 AND (saved_search_id IS NOT NULL OR children IS NOT NULL)', [ 1 => [$groupID, 'Positive'], ]); } /** * @param array $savedSearch - * @param string $addSelect - * @param string $excludeClause + * @param int $groupID + * * @return string * @throws API_Exception * @throws \Civi\API\Exception\NotImplementedException * @throws CRM_Core_Exception */ - protected static function getApiSQL(array $savedSearch, string $addSelect, string $excludeClause) { + protected static function getApiSQL(array $savedSearch, int $groupID): string { + $excludeClause = "NOT IN ( + SELECT contact_id FROM civicrm_group_contact + WHERE civicrm_group_contact.status = 'Removed' + AND civicrm_group_contact.group_id = $groupID )"; + $addSelect = "$groupID AS group_id"; + $apiParams = $savedSearch['api_params'] + ['select' => ['id'], 'checkPermissions' => FALSE]; $idField = SqlExpression::convert($apiParams['select'][0], TRUE)->getAlias(); // Unless there's a HAVING clause, we don't care about other columns @@ -553,15 +559,20 @@ protected static function getApiSQL(array $savedSearch, string $addSelect, strin * We split it up and store custom class * so temp tables are not destroyed if they are used * - * @param int $savedSearchID * @param array $savedSearch - * @param string $addSelect - * @param string $excludeClause + * @param int $groupID * * @return string - * @throws CRM_Core_Exception + * @throws \CRM_Core_Exception + * @throws \CiviCRM_API3_Exception */ - protected static function getCustomSearchSQL($savedSearchID, array $savedSearch, string $addSelect, string $excludeClause) { + protected static function getCustomSearchSQL(array $savedSearch, int $groupID) { + $savedSearchID = $savedSearch['id']; + $excludeClause = "NOT IN ( + SELECT contact_id FROM civicrm_group_contact + WHERE civicrm_group_contact.status = 'Removed' + AND civicrm_group_contact.group_id = $groupID )"; + $addSelect = "$groupID AS group_id"; $ssParams = CRM_Contact_BAO_SavedSearch::getFormValues($savedSearchID); // CRM-7021 rectify params to what proximity search expects if there is a value for prox_distance if (!empty($ssParams)) { @@ -581,16 +592,20 @@ protected static function getCustomSearchSQL($savedSearchID, array $savedSearch, /** * Get array of sql from a saved query object group. * - * @param int $savedSearchID * @param array $savedSearch - * @param string $addSelect - * @param string $excludeClause + * @param int $groupID * * @return string * @throws \CRM_Core_Exception * @throws \CiviCRM_API3_Exception */ - protected static function getQueryObjectSQL($savedSearchID, array $savedSearch, string $addSelect, string $excludeClause): string { + protected static function getQueryObjectSQL(array $savedSearch, int $groupID): string { + $savedSearchID = $savedSearch['id']; + $excludeClause = "NOT IN ( + SELECT contact_id FROM civicrm_group_contact + WHERE civicrm_group_contact.status = 'Removed' + AND civicrm_group_contact.group_id = $groupID )"; + $addSelect = "$groupID AS group_id"; $fv = CRM_Contact_BAO_SavedSearch::getFormValues($savedSearchID); //check if the saved search has mapping id if ($savedSearch['mapping_id']) { @@ -821,22 +836,14 @@ protected static function insertGroupContactsIntoTempTable(string $tempTableName ->execute() ->first(); - $excludeClause = "NOT IN ( - SELECT contact_id FROM civicrm_group_contact - WHERE civicrm_group_contact.status = 'Removed' - AND civicrm_group_contact.group_id = $groupID )"; - $addSelect = "$groupID AS group_id"; - if ($savedSearch['api_entity']) { - $sql = self::getApiSQL($savedSearch, $addSelect, $excludeClause); + $sql = self::getApiSQL($savedSearch, $groupID); + } + elseif (!empty($savedSearch['form_values']['customSearchID'])) { + $sql = self::getCustomSearchSQL($savedSearch, $groupID); } else { - if (!empty($savedSearch['form_values']['customSearchID'])) { - $sql = self::getCustomSearchSQL($savedSearchID, $savedSearch, $addSelect, $excludeClause); - } - else { - $sql = self::getQueryObjectSQL($savedSearchID, $savedSearch, $addSelect, $excludeClause); - } + $sql = self::getQueryObjectSQL($savedSearch, $groupID); } }