diff --git a/CRM/Contact/BAO/GroupContactCache.php b/CRM/Contact/BAO/GroupContactCache.php index c97afa799b44..e555d4e41f18 100644 --- a/CRM/Contact/BAO/GroupContactCache.php +++ b/CRM/Contact/BAO/GroupContactCache.php @@ -461,13 +461,17 @@ public static function load(&$group, $force = FALSE) { $customClass = NULL; if ($savedSearchID) { $ssParams = CRM_Contact_BAO_SavedSearch::getSearchParams($savedSearchID); + $groupID = CRM_Utils_Type::escape($groupID, 'Integer'); + + $excludeClause = "NOT IN ( + SELECT contact_id FROM civicrm_group_contact + WHERE civicrm_group_contact.status = 'Removed' + AND civicrm_group_contact.group_id = $groupID )"; if (!empty($ssParams['api_entity'])) { - $mainCol = 'a'; - $sql = self::getApiSQL($savedSearchID, $ssParams); + $sql = self::getApiSQL($ssParams, $excludeClause); } else { - $mainCol = 'contact_a'; // 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); @@ -478,12 +482,8 @@ public static function load(&$group, $force = FALSE) { else { $sql = self::getQueryObjectSQL($savedSearchID, $ssParams); } + $sql['from'] .= " AND contact_a.id $excludeClause"; } - $groupID = CRM_Utils_Type::escape($groupID, 'Integer'); - $sql['from'] .= " AND $mainCol.id NOT IN ( - SELECT contact_id FROM civicrm_group_contact - WHERE civicrm_group_contact.status = 'Removed' - AND civicrm_group_contact.group_id = $groupID ) "; } if (!empty($sql['select'])) { @@ -508,9 +508,7 @@ public static function load(&$group, $force = FALSE) { if (empty($contactQuery['select']) || empty($contactQuery['from'])) { continue; } - if (CRM_Core_DAO::singleValueQuery("SELECT COUNT(*) {$contactQuery['from']}") > 0) { - CRM_Core_DAO::executeQuery("INSERT IGNORE INTO $tempTable (group_id, contact_id) {$contactQuery['select']} {$contactQuery['from']}"); - } + CRM_Core_DAO::executeQuery("INSERT IGNORE INTO $tempTable (group_id, contact_id) {$contactQuery['select']} {$contactQuery['from']}"); } if ($group->children) { @@ -718,27 +716,34 @@ public static function invalidateGroupContactCache($groupID) { } /** - * @param $savedSearchID * @param array $savedSearch + * @param string $excludeClause * @return array * @throws API_Exception * @throws \Civi\API\Exception\NotImplementedException * @throws CRM_Core_Exception */ - protected static function getApiSQL($savedSearchID, array $savedSearch): array { - $apiParams = ['select' => ['id'], 'checkPermissions' => FALSE] + $savedSearch['api_params']; + protected static function getApiSQL(array $savedSearch, string $excludeClause): array { + $apiParams = $savedSearch['api_params'] + ['select' => ['id'], 'checkPermissions' => FALSE]; + list($select) = explode(' AS ', $apiParams['select'][0]); + $apiParams['select'][0] = $select . ' AS smart_group_contact_id'; $api = \Civi\API\Request::create($savedSearch['api_entity'], 'get', $apiParams); $query = new \Civi\Api4\Query\Api4SelectQuery($api); + $query->forceSelectId = FALSE; + $query->getQuery()->having('smart_group_contact_id ' . $excludeClause); $sql = $query->getSql(); return [ - 'select' => substr($sql, 0, strpos($sql, 'FROM')), - 'from' => substr($sql, strpos($sql, 'FROM')), + 'select' => substr($sql, 0, strpos($sql, "\nFROM ")), + 'from' => substr($sql, strpos($sql, "\nFROM ")), ]; } /** * Get sql from a custom search. * + * We split it up and store custom class + * so temp tables are not destroyed if they are used + * * @param int $savedSearchID * @param array $ssParams * @@ -746,22 +751,16 @@ protected static function getApiSQL($savedSearchID, array $savedSearch): array { * @throws \Exception */ protected static function getCustomSearchSQL($savedSearchID, array $ssParams): array { - // if custom search - - // we split it up and store custom class - // so temp tables are not destroyed if they are used - // hence customClass is defined above at top of function $customClass = CRM_Contact_BAO_SearchCustom::customClass($ssParams['customSearchID'], $savedSearchID); $searchSQL = $customClass->contactIDs(); $searchSQL = str_replace('ORDER BY contact_a.id ASC', '', $searchSQL); if (strpos($searchSQL, 'WHERE') === FALSE) { $searchSQL .= " WHERE ( 1 ) "; } - $sql = [ + return [ 'select' => substr($searchSQL, 0, strpos($searchSQL, 'FROM')), 'from' => substr($searchSQL, strpos($searchSQL, 'FROM')), ]; - return $sql; } /** @@ -805,11 +804,10 @@ protected static function getQueryObjectSQL($savedSearchID, array $ssParams): ar FALSE, FALSE, FALSE, TRUE ); - $sql = [ + return [ 'select' => $sqlParts['select'], 'from' => "{$sqlParts['from']} {$sqlParts['where']} {$sqlParts['having']} {$sqlParts['group_by']}", ]; - return $sql; } } diff --git a/Civi/Api4/Query/Api4SelectQuery.php b/Civi/Api4/Query/Api4SelectQuery.php index 09e95f147ec9..116974e9f039 100644 --- a/Civi/Api4/Query/Api4SelectQuery.php +++ b/Civi/Api4/Query/Api4SelectQuery.php @@ -64,6 +64,8 @@ class Api4SelectQuery extends SelectQuery { */ public $groupBy = []; + public $forceSelectId = TRUE; + /** * @param \Civi\Api4\Generic\DAOGetAction $apiGet */ @@ -76,6 +78,8 @@ public function __construct($apiGet) { $this->orderBy = $apiGet->getOrderBy(); $this->limit = $apiGet->getLimit(); $this->offset = $apiGet->getOffset(); + // Always select ID of main table unless grouping is used + $this->forceSelectId = !$this->groupBy; if ($apiGet->getDebug()) { $this->debugOutput =& $apiGet->_debugOutput; } @@ -151,8 +155,7 @@ protected function buildSelectClause() { return; } else { - // Always select ID (unless we're doing groupBy). - if (!$this->groupBy) { + if ($this->forceSelectId) { $this->select = array_merge(['id'], $this->select); } diff --git a/ang/api4Explorer/Explorer.html b/ang/api4Explorer/Explorer.html index ad35e3810317..0b4ff27b6c77 100644 --- a/ang/api4Explorer/Explorer.html +++ b/ang/api4Explorer/Explorer.html @@ -26,7 +26,7 @@

- +
diff --git a/ang/api4Explorer/Explorer.js b/ang/api4Explorer/Explorer.js index f75443dcf49b..81a07ac0fc66 100644 --- a/ang/api4Explorer/Explorer.js +++ b/ang/api4Explorer/Explorer.js @@ -703,7 +703,8 @@ $scope.saveDoc = function() { return { description: ts('Save API call as a smart group.'), - comment: ts('Allows you to create a SavedSearch containing the WHERE clause of this API call.'), + comment: ts('Create a SavedSearch using these API params to populate a smart group.') + + '\n\n' + ts('NOTE: you must select contact id as the only field.') }; }; @@ -712,6 +713,15 @@ writeCode(); $scope.save = function() { + $scope.params.limit = $scope.params.offset = 0; + if ($scope.params.chain.length) { + CRM.alert(ts('Smart groups are not compatible with API chaining.'), ts('Error'), 'error', {expires: 5000}); + return; + } + if ($scope.params.select.length !== 1 || !_.includes($scope.params.select[0], 'id')) { + CRM.alert(ts('To create a smart group, the API must select contact id and no other fields.'), ts('Error'), 'error', {expires: 5000}); + return; + } var model = { title: '', description: '', @@ -722,10 +732,11 @@ params: JSON.parse(angular.toJson($scope.params)) }; model.params.version = 4; - delete model.params.select; delete model.params.chain; delete model.params.debug; delete model.params.limit; + delete model.params.offset; + delete model.params.orderBy; delete model.params.checkPermissions; var options = CRM.utils.adjustDialogDefaults({ width: '500px', diff --git a/tests/phpunit/api/v4/Entity/SavedSearchTest.php b/tests/phpunit/api/v4/Entity/SavedSearchTest.php index 41bd7efd14a3..5c426298efcf 100644 --- a/tests/phpunit/api/v4/Entity/SavedSearchTest.php +++ b/tests/phpunit/api/v4/Entity/SavedSearchTest.php @@ -23,13 +23,14 @@ use api\v4\UnitTestCase; use Civi\Api4\Contact; +use Civi\Api4\Email; /** * @group headless */ class SavedSearchTest extends UnitTestCase { - public function testApi4SmartGroup() { + public function testContactSmartGroup() { $in = Contact::create()->setCheckPermissions(FALSE)->addValue('first_name', 'yes')->addValue('do_not_phone', TRUE)->execute()->first(); $out = Contact::create()->setCheckPermissions(FALSE)->addValue('first_name', 'no')->addValue('do_not_phone', FALSE)->execute()->first(); @@ -44,12 +45,42 @@ public function testApi4SmartGroup() { ], ], 'chain' => [ - 'group' => ['Group', 'create', ['values' => ['title' => 'Hello Test', 'saved_search_id' => '$id']], 0], + 'group' => ['Group', 'create', ['values' => ['title' => 'Contact Test', 'saved_search_id' => '$id']], 0], ], ])->first(); // Oops we don't have an api4 syntax yet for selecting contacts in a group. $ins = civicrm_api3('Contact', 'get', ['group' => $savedSearch['group']['name'], 'options' => ['limit' => 0]]); + $this->assertEquals(1, count($ins['values'])); + $this->assertArrayHasKey($in['id'], $ins['values']); + $this->assertArrayNotHasKey($out['id'], $ins['values']); + } + + public function testEmailSmartGroup() { + $in = Contact::create()->setCheckPermissions(FALSE)->addValue('first_name', 'yep')->execute()->first(); + $out = Contact::create()->setCheckPermissions(FALSE)->addValue('first_name', 'nope')->execute()->first(); + $email = uniqid() . '@' . uniqid(); + Email::create()->setCheckPermissions(FALSE)->addValue('email', $email)->addValue('contact_id', $in['id'])->execute(); + + $savedSearch = civicrm_api4('SavedSearch', 'create', [ + 'values' => [ + 'api_entity' => 'Email', + 'api_params' => [ + 'version' => 4, + 'select' => ['contact_id'], + 'where' => [ + ['email', '=', $email], + ], + ], + ], + 'chain' => [ + 'group' => ['Group', 'create', ['values' => ['title' => 'Email Test', 'saved_search_id' => '$id']], 0], + ], + ])->first(); + + // Oops we don't have an api4 syntax yet for selecting contacts in a group. + $ins = civicrm_api3('Contact', 'get', ['group' => $savedSearch['group']['name'], 'options' => ['limit' => 0]]); + $this->assertEquals(1, count($ins['values'])); $this->assertArrayHasKey($in['id'], $ins['values']); $this->assertArrayNotHasKey($out['id'], $ins['values']); }