SearchKit - Keep randomly sorted results stable across pages

SearchKit allows ORDER BY RAND() but this can cause unexpected reshuffling
when using the pager, editing-in-place or bulk-updating records.

The solution is to generate a seed on the client-side when the display initializes,
and re-use it every time results are fetched. This keeps the order stable,
only reshuffling when the browser reloads the page.

diff --git a/Civi/Api4/Query/SqlFunction.php b/Civi/Api4/Query/SqlFunction.php
index fd10f80ccabaff338cf374f723ed79d1eafe458f..b1d1c94093fe6a92649d7dc7902f7b773aeef0a3 100644 (file)
--- a/Civi/Api4/Query/SqlFunction.php
+++ b/Civi/Api4/Query/SqlFunction.php
@@ -68,7 +68,10 @@ abstract class SqlFunction extends SqlExpression {
       ];
       if ($param['max_expr'] && (!$param['name'] || $param['name'] === $prefix)) {
         $exprs = $this->captureExpressions($arg, $param['must_be'], TRUE);
-        if (count($exprs) < $param['min_expr'] || count($exprs) > $param['max_expr']) {
+        if (
+          (count($exprs) < $param['min_expr'] || count($exprs) > $param['max_expr']) &&
+          !(!$exprs && $param['optional'])
+        ) {
           throw new \API_Exception('Incorrect number of arguments for SQL function ' . static::getName());
         }
         $this->args[$idx]['expr'] = $exprs;

diff --git a/Civi/Api4/Query/SqlFunctionRAND.php b/Civi/Api4/Query/SqlFunctionRAND.php
index 7e834ea55e43b44bd4297f361999fbc02f59e2ff..8853973eb005f43e5306570056dcecb6b8ec5d74 100644 (file)
--- a/Civi/Api4/Query/SqlFunctionRAND.php
+++ b/Civi/Api4/Query/SqlFunctionRAND.php
@@ -19,7 +19,12 @@ class SqlFunctionRAND extends SqlFunction {
   protected static $category = self::CATEGORY_MATH;
 
   protected static function params(): array {
-    return [];
+    return [
+      [
+        'optional' => TRUE,
+        'must_be' => ['SqlNumber'],
+      ],
+    ];
   }
 
   /**

diff --git a/ext/search_kit/Civi/Api4/Action/SearchDisplay/AbstractRunAction.php b/ext/search_kit/Civi/Api4/Action/SearchDisplay/AbstractRunAction.php
index 88451018d24b88e35f65cfd97836add9f3ca5f26..faf18ec7865f656d9525d95312479e1ad739f154 100644 (file)
--- a/ext/search_kit/Civi/Api4/Action/SearchDisplay/AbstractRunAction.php
+++ b/ext/search_kit/Civi/Api4/Action/SearchDisplay/AbstractRunAction.php
@@ -349,6 +349,10 @@ abstract class AbstractRunAction extends \Civi\Api4\Generic\AbstractAction {
 
     $orderBy = [];
     foreach ($currentSort ?: $defaultSort as $item) {
+      // Apply seed to random sorting
+      if ($item[0] === 'RAND()' && isset($this->seed)) {
+        $item[0] = 'RAND(' . $this->seed . ')';
+      }
       $orderBy[$item[0]] = $item[1];
     }
     return $orderBy;

diff --git a/ext/search_kit/Civi/Api4/Action/SearchDisplay/Run.php b/ext/search_kit/Civi/Api4/Action/SearchDisplay/Run.php
index 3ba06edbd3fb770288e116ca47cfd7f2f666f242..4d09c7e81ebf7dd70b1928c2bac6852b4a83b97f 100644 (file)
--- a/ext/search_kit/Civi/Api4/Action/SearchDisplay/Run.php
+++ b/ext/search_kit/Civi/Api4/Action/SearchDisplay/Run.php
@@ -22,6 +22,14 @@ class Run extends AbstractRunAction {
    */
   protected $limit;
 
+  /**
+   * Integer used as a seed when ordering by RAND().
+   * This keeps the order stable enough to use a pager with random sorting.
+   *
+   * @var int
+   */
+  protected $seed;
+
   /**
    * @param \Civi\Api4\Generic\Result $result
    * @throws \API_Exception

diff --git a/ext/search_kit/ang/crmSearchDisplay/traits/searchDisplayBaseTrait.service.js b/ext/search_kit/ang/crmSearchDisplay/traits/searchDisplayBaseTrait.service.js
index 5a06852757402c2e3745c72a71b69678de00c787..89752172d5d14bca15fb985c0116e37c026e4b78 100644 (file)
--- a/ext/search_kit/ang/crmSearchDisplay/traits/searchDisplayBaseTrait.service.js
+++ b/ext/search_kit/ang/crmSearchDisplay/traits/searchDisplayBaseTrait.service.js
@@ -4,7 +4,8 @@
   // Trait provides base methods and properties common to all search display types
   angular.module('crmSearchDisplay').factory('searchDisplayBaseTrait', function(crmApi4) {
     var ts = CRM.ts('org.civicrm.search_kit'),
-      runCount = 0;
+      runCount = 0,
+      seed = Date.now();
 
     // Replace tokens keyed to rowData.
     // Pass view=true to replace with view value, otherwise raw value is used.
@@ -73,6 +74,7 @@
         var ctrl = this;
         this.limit = this.settings.limit;
         this.sort = this.settings.sort ? _.cloneDeep(this.settings.sort) : [];
+        this.seed = Date.now();
 
         this.getResults = _.debounce(function() {
           $scope.$apply(function() {
@@ -129,6 +131,7 @@
           display: this.display,
           sort: this.sort,
           limit: this.limit,
+          seed: this.seed,
           filters: _.assign({}, (this.afFieldset ? this.afFieldset.getFieldData() : {}), this.filters),
           afform: this.afFieldset ? this.afFieldset.getFormName() : null
         };

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 c9bb14a4a90eac27d2a25269aabcda0b5a60494e..20158ed149ceb4464a6e1a97c74b8a0b4234d017 100644 (file)
--- a/ext/search_kit/tests/phpunit/api/v4/SearchDisplay/SearchRunTest.php
+++ b/ext/search_kit/tests/phpunit/api/v4/SearchDisplay/SearchRunTest.php
@@ -497,4 +497,75 @@ class SearchRunTest extends \PHPUnit\Framework\TestCase implements HeadlessInter
     $this->assertStringContainsString('failed', $error);
   }
 
+  /**
+   * Test running a searchDisplay with random sorting.
+   */
+  public function testSortByRand() {
+    $lastName = uniqid(__FUNCTION__);
+    $sampleData = [
+      ['first_name' => 'One', 'last_name' => $lastName],
+      ['first_name' => 'Two', 'last_name' => $lastName],
+      ['first_name' => 'Three', 'last_name' => $lastName],
+      ['first_name' => 'Four', 'last_name' => $lastName],
+    ];
+    Contact::save(FALSE)->setRecords($sampleData)->execute();
+
+    $params = [
+      'checkPermissions' => FALSE,
+      'return' => 'page:1',
+      'savedSearch' => [
+        'api_entity' => 'Contact',
+        'api_params' => [
+          'version' => 4,
+          'select' => ['id', 'first_name', 'last_name'],
+          'where' => [['last_name', '=', $lastName]],
+        ],
+      ],
+      'display' => [
+        'type' => 'list',
+        'label' => '',
+        'settings' => [
+          'limit' => 20,
+          'pager' => TRUE,
+          'columns' => [
+            [
+              'key' => 'first_name',
+              'label' => 'First Name',
+              'dataType' => 'String',
+              'type' => 'field',
+            ],
+          ],
+          'sort' => [
+            ['RAND()', 'ASC'],
+          ],
+        ],
+      ],
+      'afform' => NULL,
+    ];
+
+    // Without seed, results are returned in unpredictable order
+    // (hard to test this, but we can at least assert we get the correct number of results back)
+    $unseeded = civicrm_api4('SearchDisplay', 'run', $params);
+    $this->assertCount(4, $unseeded);
+
+    // Seed must be an integer
+    $params['seed'] = 'hello';
+    try {
+      civicrm_api4('SearchDisplay', 'run', $params);
+      $this->fail();
+    }
+    catch (\API_Exception $e) {
+    }
+
+    // With a random seed, results should be shuffled in stable order
+    $params['seed'] = 12345678987654321;
+    $seeded = civicrm_api4('SearchDisplay', 'run', $params);
+
+    // Same seed, same order every time
+    for ($i = 0; $i <= 9; ++$i) {
+      $repeat = civicrm_api4('SearchDisplay', 'run', $params);
+      $this->assertEquals($seeded->column('id'), $repeat->column('id'));
+    }
+  }
+
 }