From a4499ec59bb340e7718ff2e91f5b183ed585021c Mon Sep 17 00:00:00 2001 From: Coleman Watts Date: Wed, 3 Jun 2020 19:09:34 -0400 Subject: [PATCH] APIv4 - Support pseudoconstant suffixes in orderBy clause --- Civi/Api4/Query/Api4SelectQuery.php | 27 ++++++++++--------- ang/api4Explorer/Explorer.html | 2 +- ang/api4Explorer/Explorer.js | 4 +++ .../api/v4/Action/BasicActionsTest.php | 7 +++++ .../api/v4/Action/PseudoconstantTest.php | 27 +++++++++++++++++++ 5 files changed, 54 insertions(+), 13 deletions(-) diff --git a/Civi/Api4/Query/Api4SelectQuery.php b/Civi/Api4/Query/Api4SelectQuery.php index 5378454474..794df9580a 100644 --- a/Civi/Api4/Query/Api4SelectQuery.php +++ b/Civi/Api4/Query/Api4SelectQuery.php @@ -222,7 +222,20 @@ class Api4SelectQuery extends SelectQuery { if ($dir !== 'ASC' && $dir !== 'DESC') { throw new \API_Exception("Invalid sort direction. Cannot order by $item $dir"); } - $this->query->orderBy($this->renderExpression($item) . " $dir"); + $expr = $this->getExpression($item); + $column = $expr->render($this->apiFieldSpec); + + // Use FIELD() function to sort on pseudoconstant values + $suffix = strstr($item, ':'); + if ($suffix && $expr->getType() === 'SqlField') { + $field = $this->getField($item); + $options = FormattingUtil::getPseudoconstantList($field['entity'], $field['name'], substr($suffix, 1)); + if ($options) { + asort($options); + $column = "FIELD($column,'" . implode("','", array_keys($options)) . "')"; + } + } + $this->query->orderBy("$column $dir"); } } @@ -241,7 +254,7 @@ class Api4SelectQuery extends SelectQuery { */ protected function buildGroupBy() { foreach ($this->groupBy as $item) { - $this->query->groupBy($this->renderExpression($item)); + $this->query->groupBy($this->getExpression($item)->render($this->apiFieldSpec)); } } @@ -374,16 +387,6 @@ class Api4SelectQuery extends SelectQuery { return $sqlExpr; } - /** - * @param string $expr - * @return string - * @throws \API_Exception - */ - protected function renderExpression(string $expr) { - $sqlExpr = $this->getExpression($expr); - return $sqlExpr->render($this->apiFieldSpec); - } - /** * @inheritDoc */ diff --git a/ang/api4Explorer/Explorer.html b/ang/api4Explorer/Explorer.html index c688ea3b9a..d1379c8929 100644 --- a/ang/api4Explorer/Explorer.html +++ b/ang/api4Explorer/Explorer.html @@ -138,7 +138,7 @@
- +
diff --git a/ang/api4Explorer/Explorer.js b/ang/api4Explorer/Explorer.js index 63cb5dedd5..17de0a2958 100644 --- a/ang/api4Explorer/Explorer.js +++ b/ang/api4Explorer/Explorer.js @@ -28,6 +28,7 @@ $scope.havingOptions = []; $scope.fieldsAndJoins = []; $scope.fieldsAndJoinsAndFunctions = []; + $scope.fieldsAndJoinsAndFunctionsWithSuffixes = []; $scope.fieldsAndJoinsAndFunctionsAndWildcards = []; $scope.availableParams = {}; $scope.params = {}; @@ -357,6 +358,7 @@ $scope.action = $routeParams.api4action; $scope.fieldsAndJoins.length = 0; $scope.fieldsAndJoinsAndFunctions.length = 0; + $scope.fieldsAndJoinsAndFunctionsWithSuffixes.length = 0; $scope.fieldsAndJoinsAndFunctionsAndWildcards.length = 0; if (!actions.length) { formatForSelect2(getEntity().actions, actions, 'name', ['description', 'params']); @@ -382,10 +384,12 @@ }); } $scope.fieldsAndJoinsAndFunctions = addJoins($scope.fields.concat(functions), true); + $scope.fieldsAndJoinsAndFunctionsWithSuffixes = addJoins(getFieldList($scope.action, ['name', 'label']).concat(functions), false, ['name', 'label']); $scope.fieldsAndJoinsAndFunctionsAndWildcards = addJoins(getFieldList($scope.action, ['name', 'label']).concat(functions), true, ['name', 'label']); } else { $scope.fieldsAndJoins = getFieldList($scope.action, ['name']); $scope.fieldsAndJoinsAndFunctions = $scope.fields; + $scope.fieldsAndJoinsAndFunctionsWithSuffixes = getFieldList($scope.action, ['name', 'label']); $scope.fieldsAndJoinsAndFunctionsAndWildcards = getFieldList($scope.action, ['name', 'label']); } $scope.fieldsAndJoinsAndFunctionsAndWildcards.unshift({id: '*', text: '*', 'description': 'All core ' + $scope.entity + ' fields'}); diff --git a/tests/phpunit/api/v4/Action/BasicActionsTest.php b/tests/phpunit/api/v4/Action/BasicActionsTest.php index de06f6882e..daf2b3470e 100644 --- a/tests/phpunit/api/v4/Action/BasicActionsTest.php +++ b/tests/phpunit/api/v4/Action/BasicActionsTest.php @@ -266,6 +266,7 @@ class BasicActionsTest extends UnitTestCase { $results = MockBasicEntity::get() ->addSelect('*', 'group:label', 'group:name', 'fruit:name', 'fruit:color', 'fruit:label') + ->addOrderBy('fruit:color', "DESC") ->execute(); $this->assertEquals('round', $results[0]['shape']); @@ -277,6 +278,12 @@ class BasicActionsTest extends UnitTestCase { $this->assertEquals('banana', $results[0]['fruit:name']); $this->assertEquals('yellow', $results[0]['fruit:color']); + // Reverse order + $results = MockBasicEntity::get() + ->addOrderBy('fruit:color') + ->execute(); + $this->assertEquals('two', $results[0]['group']); + // Cannot match to a non-unique option property like :color on create try { MockBasicEntity::create()->addValue('fruit:color', 'yellow')->execute(); diff --git a/tests/phpunit/api/v4/Action/PseudoconstantTest.php b/tests/phpunit/api/v4/Action/PseudoconstantTest.php index 277c47be68..cd01509d63 100644 --- a/tests/phpunit/api/v4/Action/PseudoconstantTest.php +++ b/tests/phpunit/api/v4/Action/PseudoconstantTest.php @@ -196,6 +196,33 @@ class PseudoconstantTest extends BaseCustomValueTest { $this->assertEquals('blü', $result['myPseudoconstantTest.Color:label']); $this->assertEquals('bl_', $result['myPseudoconstantTest.Color:name']); $this->assertEquals('b', $result['myPseudoconstantTest.Color']); + + $cid1 = Contact::create() + ->setCheckPermissions(FALSE) + ->addValue('first_name', 'two') + ->addValue('myPseudoconstantTest.Technicolor:label', 'RED') + ->execute()->first()['id']; + $cid2 = Contact::create() + ->setCheckPermissions(FALSE) + ->addValue('first_name', 'two') + ->addValue('myPseudoconstantTest.Technicolor:label', 'GREEN') + ->execute()->first()['id']; + + // Test ordering by label + $result = Contact::get() + ->setCheckPermissions(FALSE) + ->addWhere('id', 'IN', [$cid1, $cid2]) + ->addSelect('id') + ->addOrderBy('myPseudoconstantTest.Technicolor:label') + ->execute()->first()['id']; + $this->assertEquals($cid2, $result); + $result = Contact::get() + ->setCheckPermissions(FALSE) + ->addWhere('id', 'IN', [$cid1, $cid2]) + ->addSelect('id') + ->addOrderBy('myPseudoconstantTest.Technicolor:label', 'DESC') + ->execute()->first()['id']; + $this->assertEquals($cid1, $result); } public function testJoinOptions() { -- 2.25.1