From 1b29c899ac95ba43e72a7b56883b1f29f4a4d6e3 Mon Sep 17 00:00:00 2001
From: Eileen McNaughton <emcnaughton@wikimedia.org>
Date: Mon, 26 Jul 2021 16:30:19 +1200
Subject: [PATCH] [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 bb19902edd..1f3d30c8a6 100644
--- a/CRM/Contact/BAO/GroupContactCache.php
+++ b/CRM/Contact/BAO/GroupContactCache.php
@@ -510,23 +510,29 @@ ORDER BY   gc.contact_id, g.children
    * @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 @@ ORDER BY   gc.contact_id, g.children
    * 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 @@ ORDER BY   gc.contact_id, g.children
   /**
    * 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 @@ ORDER BY   gc.contact_id, g.children
         ->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);
       }
     }
 
-- 
2.25.1