From 7193d9f6c711bea258e24db33969a40c639acbab Mon Sep 17 00:00:00 2001 From: Coleman Watts Date: Wed, 3 Nov 2021 14:34:08 -0400 Subject: [PATCH] SearchKit - Fix incorrect pager count when using filters Fixes a bug where filters were not being applied correctly when fetching rowCount --- .../Action/SearchDisplay/AbstractRunAction.php | 16 +++++++--------- .../Civi/Api4/Action/SearchDisplay/Download.php | 2 +- .../Civi/Api4/Action/SearchDisplay/Run.php | 4 ++-- .../SearchDisplay/SavedSearchInspectorTrait.php | 8 +++++++- .../api/v4/SearchDisplay/SearchRunTest.php | 10 ++++++++++ 5 files changed, 27 insertions(+), 13 deletions(-) diff --git a/ext/search_kit/Civi/Api4/Action/SearchDisplay/AbstractRunAction.php b/ext/search_kit/Civi/Api4/Action/SearchDisplay/AbstractRunAction.php index 4bc6287afd..eb0a38ddaa 100644 --- a/ext/search_kit/Civi/Api4/Action/SearchDisplay/AbstractRunAction.php +++ b/ext/search_kit/Civi/Api4/Action/SearchDisplay/AbstractRunAction.php @@ -97,8 +97,7 @@ abstract class AbstractRunAction extends \Civi\Api4\Generic\AbstractAction { throw new UnauthorizedException('Access denied'); } - $this->savedSearch['api_params'] += ['where' => []]; - $this->savedSearch['api_params']['checkPermissions'] = empty($this->display['acl_bypass']); + $this->_apiParams['checkPermissions'] = empty($this->display['acl_bypass']); $this->display['settings']['columns'] = $this->display['settings']['columns'] ?? []; $this->processResult($result); @@ -145,7 +144,7 @@ abstract class AbstractRunAction extends \Civi\Api4\Generic\AbstractAction { // Get value from api result unless this is a pseudo-field which gets a calculated value switch ($key) { case 'result_row_num': - return $rowIndex + 1 + ($this->savedSearch['api_params']['offset'] ?? 0); + return $rowIndex + 1 + ($this->_apiParams['offset'] ?? 0); case 'user_contact_id': return \CRM_Core_Session::getLoggedInContactID(); @@ -521,21 +520,20 @@ abstract class AbstractRunAction extends \Civi\Api4\Generic\AbstractAction { $field = $this->getField($fieldName); // If field is not found it must be an aggregated column & belongs in the HAVING clause. if (!$field) { - $this->savedSearch['api_params']['having'] = $this->savedSearch['api_params']['having'] ?? []; - $clause =& $this->savedSearch['api_params']['having']; + $clause =& $this->_apiParams['having']; } // If field belongs to an EXCLUDE join, it should be added as a join condition else { $prefix = strpos($fieldName, '.') ? explode('.', $fieldName)[0] : NULL; - foreach ($this->savedSearch['api_params']['join'] ?? [] as $idx => $join) { + foreach ($this->_apiParams['join'] as $idx => $join) { if (($join[1] ?? 'LEFT') === 'EXCLUDE' && (explode(' AS ', $join[0])[1] ?? '') === $prefix) { - $clause =& $this->savedSearch['api_params']['join'][$idx]; + $clause =& $this->_apiParams['join'][$idx]; } } } // Default: add filter to WHERE clause if (!isset($clause)) { - $clause =& $this->savedSearch['api_params']['where']; + $clause =& $this->_apiParams['where']; } $filterClauses = []; @@ -687,7 +685,7 @@ abstract class AbstractRunAction extends \Civi\Api4\Generic\AbstractAction { */ protected function getJoinFromAlias(string $alias) { $result = ''; - foreach ($this->savedSearch['api_params']['join'] ?? [] as $join) { + foreach ($this->_apiParams['join'] as $join) { $joinName = explode(' AS ', $join[0])[1]; if (strpos($alias, $joinName) === 0) { $parsed = $joinName . '.' . substr($alias, strlen($joinName) + 1); diff --git a/ext/search_kit/Civi/Api4/Action/SearchDisplay/Download.php b/ext/search_kit/Civi/Api4/Action/SearchDisplay/Download.php index a234f3751b..7f8bd75a18 100644 --- a/ext/search_kit/Civi/Api4/Action/SearchDisplay/Download.php +++ b/ext/search_kit/Civi/Api4/Action/SearchDisplay/Download.php @@ -51,7 +51,7 @@ class Download extends AbstractRunAction { */ protected function processResult(\Civi\Api4\Generic\Result $result) { $entityName = $this->savedSearch['api_entity']; - $apiParams =& $this->savedSearch['api_params']; + $apiParams =& $this->_apiParams; $settings = $this->display['settings']; // Displays are only exportable if they have actions enabled diff --git a/ext/search_kit/Civi/Api4/Action/SearchDisplay/Run.php b/ext/search_kit/Civi/Api4/Action/SearchDisplay/Run.php index 1b14398e0e..5a42ddb614 100644 --- a/ext/search_kit/Civi/Api4/Action/SearchDisplay/Run.php +++ b/ext/search_kit/Civi/Api4/Action/SearchDisplay/Run.php @@ -34,14 +34,14 @@ class Run extends AbstractRunAction { */ protected function processResult(\Civi\Api4\Generic\Result $result) { $entityName = $this->savedSearch['api_entity']; - $apiParams =& $this->savedSearch['api_params']; + $apiParams =& $this->_apiParams; $settings = $this->display['settings']; $page = $index = NULL; $key = $this->return; switch ($this->return) { case 'id': - $key = CoreUtil::getIdFieldName($this->savedSearch['api_entity']); + $key = CoreUtil::getIdFieldName($entityName); $index = [$key]; case 'row_count': if (empty($apiParams['having'])) { diff --git a/ext/search_kit/Civi/Api4/Action/SearchDisplay/SavedSearchInspectorTrait.php b/ext/search_kit/Civi/Api4/Action/SearchDisplay/SavedSearchInspectorTrait.php index 51210eb7d6..7b981d016a 100644 --- a/ext/search_kit/Civi/Api4/Action/SearchDisplay/SavedSearchInspectorTrait.php +++ b/ext/search_kit/Civi/Api4/Action/SearchDisplay/SavedSearchInspectorTrait.php @@ -22,6 +22,11 @@ trait SavedSearchInspectorTrait { */ protected $savedSearch; + /** + * @var array{select: array, where: array, having: array, orderBy: array, limit: int, offset: int, checkPermissions: bool, debug: bool} + */ + protected $_apiParams; + /** * @var \Civi\Api4\Query\Api4SelectQuery */ @@ -43,6 +48,7 @@ trait SavedSearchInspectorTrait { ->addWhere('name', '=', $this->savedSearch) ->execute()->single(); } + $this->_apiParams = $this->savedSearch['api_params'] + ['where' => [], 'join' => [], 'having' => []]; } /** @@ -88,7 +94,7 @@ trait SavedSearchInspectorTrait { protected function getSelectClause() { if (!isset($this->_selectClause)) { $this->_selectClause = []; - foreach ($this->savedSearch['api_params']['select'] ?? [] as $selectExpr) { + foreach ($this->_apiParams['select'] as $selectExpr) { $expr = SqlExpression::convert($selectExpr, TRUE); $item = [ 'fields' => [], diff --git a/ext/search_kit/tests/phpunit/api/v4/SearchDisplay/SearchRunTest.php b/ext/search_kit/tests/phpunit/api/v4/SearchDisplay/SearchRunTest.php index b1cb6cc0bb..a818fb0023 100644 --- a/ext/search_kit/tests/phpunit/api/v4/SearchDisplay/SearchRunTest.php +++ b/ext/search_kit/tests/phpunit/api/v4/SearchDisplay/SearchRunTest.php @@ -111,6 +111,8 @@ class SearchRunTest extends \PHPUnit\Framework\TestCase implements HeadlessInter $this->assertCount(2, $result); $this->assertEquals('One', $result[0]['data']['first_name']); $this->assertEquals('Two', $result[1]['data']['first_name']); + $count = civicrm_api4('SearchDisplay', 'run', ['return' => 'row_count'] + $params); + $this->assertCount(2, $count); // Raw value should be boolean, view value should be string $this->assertEquals(FALSE, $result[0]['data']['is_deceased']); @@ -122,20 +124,28 @@ class SearchRunTest extends \PHPUnit\Framework\TestCase implements HeadlessInter $this->assertCount(2, $result); $this->assertEquals('Three', $result[0]['data']['first_name']); $this->assertEquals('Two', $result[1]['data']['first_name']); + $count = civicrm_api4('SearchDisplay', 'run', ['return' => 'row_count'] + $params); + $this->assertCount(2, $count); $params['filters'] = ['last_name' => $lastName, 'contact_sub_type:label' => ['Tester', 'Bot']]; $result = civicrm_api4('SearchDisplay', 'run', $params); $this->assertCount(3, $result); + $count = civicrm_api4('SearchDisplay', 'run', ['return' => 'row_count'] + $params); + $this->assertCount(3, $count); // Comma indicates first_name OR last_name $params['filters'] = ['first_name,last_name' => $lastName, 'contact_sub_type' => ['Tester']]; $result = civicrm_api4('SearchDisplay', 'run', $params); $this->assertCount(2, $result); + $count = civicrm_api4('SearchDisplay', 'run', ['return' => 'row_count'] + $params); + $this->assertCount(2, $count); // Comma indicates first_name OR middle_name, matches "One" or "None" $params['filters'] = ['first_name,middle_name' => 'one', 'last_name' => $lastName]; $result = civicrm_api4('SearchDisplay', 'run', $params); $this->assertCount(2, $result); + $count = civicrm_api4('SearchDisplay', 'run', ['return' => 'row_count'] + $params); + $this->assertCount(2, $count); } /** -- 2.25.1