SearchKit - Fix incorrect pager count when using filters
authorColeman Watts <coleman@civicrm.org>
Wed, 3 Nov 2021 18:34:08 +0000 (14:34 -0400)
committerColeman Watts <coleman@civicrm.org>
Wed, 3 Nov 2021 18:34:08 +0000 (14:34 -0400)
Fixes a bug where filters were not being applied correctly when fetching rowCount

ext/search_kit/Civi/Api4/Action/SearchDisplay/AbstractRunAction.php
ext/search_kit/Civi/Api4/Action/SearchDisplay/Download.php
ext/search_kit/Civi/Api4/Action/SearchDisplay/Run.php
ext/search_kit/Civi/Api4/Action/SearchDisplay/SavedSearchInspectorTrait.php
ext/search_kit/tests/phpunit/api/v4/SearchDisplay/SearchRunTest.php

index 4bc6287afd07708a13e975768ecb332acd55b72d..eb0a38ddaad6853f03fe7befbf7e4ee05ada9b22 100644 (file)
@@ -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);
index a234f3751b03ad792ba1e45e2fe568465ceef58c..7f8bd75a18af23b90be6a98c5831dcd1424dfb6f 100644 (file)
@@ -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
index 1b14398e0e33bc517400ee9f3331f7f416befd9f..5a42ddb614fb03ceca2225bf546ac471eff6fc7d 100644 (file)
@@ -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'])) {
index 51210eb7d6d7a4d2818a4cae4fdd0c85c7cee0c7..7b981d016a63173a1a84d0f50f3b558eec6278c9 100644 (file)
@@ -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' => [],
index b1cb6cc0bb0da3dc4a9fa6bcede0e77a83f6c7d7..a818fb0023ff69c87d06491e5e28149488fbd855 100644 (file)
@@ -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);
   }
 
   /**