From c0e68893f8a1ff722718ef2c04ab206dd556481d Mon Sep 17 00:00:00 2001 From: eileen Date: Wed, 3 Mar 2021 01:31:55 +1300 Subject: [PATCH] Add is empty filter to search / api This is already offered in Query https://github.com/civicrm/civicrm-core/blob/5db2212e2d408f4611439734db1a31ab32dced2f/CRM/Contact/BAO/Query.php#L3420-L3428 And in Report https://github.com/civicrm/civicrm-core/blob/c3fffe27cb8203634c7a2c047686ba3d12cc38bd/CRM/Report/Form.php#L2105-L2124 (the latter munges it in with NULL but as we often save empty strings NULL does not alwasy work for strings) --- Civi/Api4/Generic/BasicGetFieldsAction.php | 19 ++--- .../Generic/Traits/ArrayQueryActionTrait.php | 4 + Civi/Api4/Query/Api4SelectQuery.php | 33 ++++++-- Civi/Api4/Utils/CoreUtil.php | 2 + ang/api4Explorer/Explorer.js | 5 +- ext/search/Civi/Search/Admin.php | 4 +- .../crmSearchClause.component.js | 9 ++- .../ang/crmSearchAdmin/crmSearchClause.html | 2 +- .../api/v4/Action/BasicActionsTest.php | 76 ++++++++++++++----- .../phpunit/api/v4/Action/ContactGetTest.php | 62 +++++++++++++++ 10 files changed, 176 insertions(+), 40 deletions(-) diff --git a/Civi/Api4/Generic/BasicGetFieldsAction.php b/Civi/Api4/Generic/BasicGetFieldsAction.php index 49af587ffb..6999cdc20d 100644 --- a/Civi/Api4/Generic/BasicGetFieldsAction.php +++ b/Civi/Api4/Generic/BasicGetFieldsAction.php @@ -256,28 +256,29 @@ class BasicGetFieldsAction extends BasicGetAction { 'name' => 'data_type', 'data_type' => 'String', 'options' => [ - 'Integer' => ts('Integer'), + 'Array' => ts('Array'), 'Boolean' => ts('Boolean'), + 'Date' => ts('Date'), + 'Float' => ts('Float'), + 'Integer' => ts('Integer'), 'String' => ts('String'), 'Text' => ts('Text'), - 'Date' => ts('Date'), 'Timestamp' => ts('Timestamp'), - 'Array' => ts('Array'), ], ], [ 'name' => 'input_type', 'data_type' => 'String', 'options' => [ - 'Text' => ts('Text'), - 'Number' => ts('Number'), - 'Select' => ts('Select'), + 'ChainSelect' => ts('ChainSelect'), 'CheckBox' => ts('CheckBox'), - 'Radio' => ts('Radio'), 'Date' => ts('Date'), - 'File' => ts('File'), 'EntityRef' => ts('EntityRef'), - 'ChainSelect' => ts('ChainSelect'), + 'File' => ts('File'), + 'Number' => ts('Number'), + 'Radio' => ts('Radio'), + 'Select' => ts('Select'), + 'Text' => ts('Text'), ], ], [ diff --git a/Civi/Api4/Generic/Traits/ArrayQueryActionTrait.php b/Civi/Api4/Generic/Traits/ArrayQueryActionTrait.php index fd515b9a4b..88c24a78c4 100644 --- a/Civi/Api4/Generic/Traits/ArrayQueryActionTrait.php +++ b/Civi/Api4/Generic/Traits/ArrayQueryActionTrait.php @@ -130,6 +130,10 @@ trait ArrayQueryActionTrait { case 'IS NOT NULL': return is_null($value) == ($operator == 'IS NULL'); + case 'IS EMPTY': + case 'IS NOT EMPTY': + return empty($value) == ($operator == 'IS EMPTY'); + case '>': return $value > $expected; diff --git a/Civi/Api4/Query/Api4SelectQuery.php b/Civi/Api4/Query/Api4SelectQuery.php index f15db6f1d5..e588e71085 100644 --- a/Civi/Api4/Query/Api4SelectQuery.php +++ b/Civi/Api4/Query/Api4SelectQuery.php @@ -434,7 +434,7 @@ class Api4SelectQuery { if ($fieldName && $valExpr->getType() === 'SqlString') { $value = $valExpr->getExpr(); FormattingUtil::formatInputValue($value, $fieldName, $this->apiFieldSpec[$fieldName], $operator); - return \CRM_Core_DAO::createSQLFilter($fieldAlias, [$operator => $value]); + return $this->createSQLClause($fieldAlias, $operator, $value, $this->apiFieldSpec[$fieldName]); } else { $value = $valExpr->render($this->apiFieldSpec); @@ -447,6 +447,22 @@ class Api4SelectQuery { } } + $sqlClause = $this->createSQLClause($fieldAlias, $operator, $value, $field ?? NULL); + if ($sqlClause === NULL) { + throw new \API_Exception("Invalid value in $type clause for '$expr'"); + } + return $sqlClause; + } + + /** + * @param string $fieldAlias + * @param string $operator + * @param mixed $value + * @param array|null $field + * @return array|string|NULL + * @throws \Exception + */ + protected function createSQLClause($fieldAlias, $operator, $value, $field) { if ($operator === 'CONTAINS') { switch ($field['serialize'] ?? NULL) { case \CRM_Core_DAO::SERIALIZE_JSON: @@ -468,11 +484,18 @@ class Api4SelectQuery { } } - $sql_clause = \CRM_Core_DAO::createSQLFilter($fieldAlias, [$operator => $value]); - if ($sql_clause === NULL) { - throw new \API_Exception("Invalid value in $type clause for '$expr'"); + if ($operator === 'IS EMPTY' || $operator === 'IS NOT EMPTY') { + // If field is not a string or number, this will pass through and use IS NULL/IS NOT NULL + $operator = str_replace('EMPTY', 'NULL', $operator); + // For strings & numbers, create an OR grouping of empty value OR null + if (in_array($field['data_type'] ?? NULL, ['String', 'Integer', 'Float'], TRUE)) { + $emptyVal = $field['data_type'] === 'String' ? '""' : '0'; + $isEmptyClause = $operator === 'IS NULL' ? "= $emptyVal OR" : "<> $emptyVal AND"; + return "($fieldAlias $isEmptyClause $fieldAlias $operator)"; + } } - return $sql_clause; + + return \CRM_Core_DAO::createSQLFilter($fieldAlias, [$operator => $value]); } /** diff --git a/Civi/Api4/Utils/CoreUtil.php b/Civi/Api4/Utils/CoreUtil.php index 25c01f0e74..ebe7913797 100644 --- a/Civi/Api4/Utils/CoreUtil.php +++ b/Civi/Api4/Utils/CoreUtil.php @@ -77,6 +77,8 @@ class CoreUtil { public static function getOperators() { $operators = \CRM_Core_DAO::acceptedSQLOperators(); $operators[] = 'CONTAINS'; + $operators[] = 'IS EMPTY'; + $operators[] = 'IS NOT EMPTY'; return $operators; } diff --git a/ang/api4Explorer/Explorer.js b/ang/api4Explorer/Explorer.js index fb9f5c71ce..7def6e386f 100644 --- a/ang/api4Explorer/Explorer.js +++ b/ang/api4Explorer/Explorer.js @@ -1098,7 +1098,7 @@ // Add/remove value if operator allows for one this.changeClauseOperator = function(clause) { - if (_.contains(clause[1], 'NULL')) { + if (_.contains(clause[1], 'IS ')) { clause.length = 2; } else if (clause.length === 2) { clause.push(''); @@ -1138,7 +1138,8 @@ op = field.serialize || dataType === 'Array' ? 'IN' : '='; } multi = _.includes(['IN', 'NOT IN', 'BETWEEN', 'NOT BETWEEN'], op); - if (op === 'IS NULL' || op === 'IS NOT NULL') { + // IS NULL, IS EMPTY, etc. + if (_.contains(op, 'IS ')) { $el.hide(); return; } diff --git a/ext/search/Civi/Search/Admin.php b/ext/search/Civi/Search/Admin.php index 711a8378ca..34d82738bd 100644 --- a/ext/search/Civi/Search/Admin.php +++ b/ext/search/Civi/Search/Admin.php @@ -60,8 +60,8 @@ class Admin { 'NOT LIKE' => E::ts('Not Like'), 'BETWEEN' => E::ts('Is Between'), 'NOT BETWEEN' => E::ts('Not Between'), - 'IS NULL' => E::ts('Is Null'), - 'IS NOT NULL' => E::ts('Not Null'), + 'IS EMPTY' => E::ts('Is Empty'), + 'IS NOT EMPTY' => E::ts('Not Empty'), ]; } diff --git a/ext/search/ang/crmSearchAdmin/crmSearchClause.component.js b/ext/search/ang/crmSearchAdmin/crmSearchClause.component.js index de5ce60e43..b91dea8128 100644 --- a/ext/search/ang/crmSearchAdmin/crmSearchClause.component.js +++ b/ext/search/ang/crmSearchAdmin/crmSearchClause.component.js @@ -83,9 +83,14 @@ } }; + // Returns false for 'IS NULL', 'IS EMPTY', etc. true otherwise. + this.operatorTakesInput = function(operator) { + return operator.indexOf('IS ') !== 0; + }; + this.changeClauseOperator = function(clause) { - // Add/remove value if operator allows for one - if (_.contains(clause[1], 'NULL')) { + // Add/remove value depending on whether operator allows for one + if (!ctrl.operatorTakesInput(clause[1])) { clause.length = 2; } else { if (clause.length === 2) { diff --git a/ext/search/ang/crmSearchAdmin/crmSearchClause.html b/ext/search/ang/crmSearchAdmin/crmSearchClause.html index 1d5f98d96f..73d2ba64a9 100644 --- a/ext/search/ang/crmSearchAdmin/crmSearchClause.html +++ b/ext/search/ang/crmSearchAdmin/crmSearchClause.html @@ -17,7 +17,7 @@
- +
diff --git a/tests/phpunit/api/v4/Action/BasicActionsTest.php b/tests/phpunit/api/v4/Action/BasicActionsTest.php index 8df90eb877..9e54d3b6f9 100644 --- a/tests/phpunit/api/v4/Action/BasicActionsTest.php +++ b/tests/phpunit/api/v4/Action/BasicActionsTest.php @@ -27,6 +27,13 @@ use Civi\Api4\MockBasicEntity; */ class BasicActionsTest extends UnitTestCase { + private function replaceRecords(&$records) { + MockBasicEntity::delete()->addWhere('id', '>', 0)->execute(); + foreach ($records as &$record) { + $record['id'] = MockBasicEntity::create()->setValues($record)->execute()->first()['id']; + } + } + public function testCrud() { MockBasicEntity::delete()->addWhere('id', '>', 0)->execute(); @@ -89,8 +96,6 @@ class BasicActionsTest extends UnitTestCase { } public function testReplace() { - MockBasicEntity::delete()->addWhere('id', '>', 0)->execute(); - $objects = [ ['group' => 'one', 'color' => 'red'], ['group' => 'one', 'color' => 'blue'], @@ -98,9 +103,7 @@ class BasicActionsTest extends UnitTestCase { ['group' => 'two', 'color' => 'orange'], ]; - foreach ($objects as &$object) { - $object['id'] = MockBasicEntity::create()->setValues($object)->execute()->first()['id']; - } + $this->replaceRecords($objects); // Keep red, change blue, delete green, and add yellow $replacements = [ @@ -125,17 +128,13 @@ class BasicActionsTest extends UnitTestCase { } public function testBatchFrobnicate() { - MockBasicEntity::delete()->addWhere('id', '>', 0)->execute(); - $objects = [ ['group' => 'one', 'color' => 'red', 'number' => 10], ['group' => 'one', 'color' => 'blue', 'number' => 20], ['group' => 'one', 'color' => 'green', 'number' => 30], ['group' => 'two', 'color' => 'blue', 'number' => 40], ]; - foreach ($objects as &$object) { - $object['id'] = MockBasicEntity::create()->setValues($object)->execute()->first()['id']; - } + $this->replaceRecords($objects); $result = MockBasicEntity::batchFrobnicate()->addWhere('color', '=', 'blue')->execute(); $this->assertEquals(2, count($result)); @@ -235,13 +234,11 @@ class BasicActionsTest extends UnitTestCase { } public function testWildcardSelect() { - MockBasicEntity::delete()->addWhere('id', '>', 0)->execute(); - $records = [ ['group' => 'one', 'color' => 'red', 'shape' => 'round', 'size' => 'med', 'weight' => 10], ['group' => 'two', 'color' => 'blue', 'shape' => 'round', 'size' => 'med', 'weight' => 20], ]; - MockBasicEntity::save()->setRecords($records)->execute(); + $this->replaceRecords($records); foreach (MockBasicEntity::get()->addSelect('*')->execute() as $result) { ksort($result); @@ -255,14 +252,57 @@ class BasicActionsTest extends UnitTestCase { $this->assertEquals(['shape', 'size', 'weight'], array_keys($result)); } - public function testContainsOperator() { - MockBasicEntity::delete()->addWhere('id', '>', 0)->execute(); + public function testEmptyAndNullOperators() { + $records = [ + [], + ['color' => '', 'weight' => 0], + ['color' => 'yellow', 'weight' => 100000000000], + ]; + $this->replaceRecords($records); + + $result = MockBasicEntity::get() + ->addWhere('color', 'IS NULL') + ->execute()->indexBy('id'); + $this->assertCount(1, $result); + $this->assertArrayHasKey($records[0]['id'], (array) $result); + $result = MockBasicEntity::get() + ->addWhere('color', 'IS EMPTY') + ->execute()->indexBy('id'); + $this->assertCount(2, $result); + $this->assertArrayNotHasKey($records[2]['id'], (array) $result); + + $result = MockBasicEntity::get() + ->addWhere('color', 'IS NOT EMPTY') + ->execute()->indexBy('id'); + $this->assertCount(1, $result); + $this->assertArrayHasKey($records[2]['id'], (array) $result); + + $result = MockBasicEntity::get() + ->addWhere('weight', 'IS NULL') + ->execute()->indexBy('id'); + $this->assertCount(1, $result); + $this->assertArrayHasKey($records[0]['id'], (array) $result); + + $result = MockBasicEntity::get() + ->addWhere('weight', 'IS EMPTY') + ->execute()->indexBy('id'); + $this->assertCount(2, $result); + $this->assertArrayNotHasKey($records[2]['id'], (array) $result); + + $result = MockBasicEntity::get() + ->addWhere('weight', 'IS NOT EMPTY') + ->execute()->indexBy('id'); + $this->assertCount(1, $result); + $this->assertArrayHasKey($records[2]['id'], (array) $result); + } + + public function testContainsOperator() { $records = [ ['group' => 'one', 'fruit:name' => ['apple', 'pear'], 'weight' => 11], ['group' => 'two', 'fruit:name' => ['pear', 'banana'], 'weight' => 12], ]; - MockBasicEntity::save()->setRecords($records)->execute(); + $this->replaceRecords($records); $result = MockBasicEntity::get() ->addWhere('fruit:name', 'CONTAINS', 'apple') @@ -299,13 +339,11 @@ class BasicActionsTest extends UnitTestCase { } public function testPseudoconstantMatch() { - MockBasicEntity::delete()->addWhere('id', '>', 0)->execute(); - $records = [ ['group:label' => 'First', 'shape' => 'round', 'fruit:name' => 'banana'], ['group:name' => 'Second', 'shape' => 'square', 'fruit:label' => 'Pear'], ]; - MockBasicEntity::save()->setRecords($records)->execute(); + $this->replaceRecords($records); $results = MockBasicEntity::get() ->addSelect('*', 'group:label', 'group:name', 'fruit:name', 'fruit:color', 'fruit:label') diff --git a/tests/phpunit/api/v4/Action/ContactGetTest.php b/tests/phpunit/api/v4/Action/ContactGetTest.php index 965a8b88ee..8218e5189e 100644 --- a/tests/phpunit/api/v4/Action/ContactGetTest.php +++ b/tests/phpunit/api/v4/Action/ContactGetTest.php @@ -132,4 +132,66 @@ class ContactGetTest extends \api\v4\UnitTestCase { } } + public function testEmptyAndNullOperators() { + $last_name = uniqid(__FUNCTION__); + + $bob = Contact::create() + ->setValues(['first_name' => 'Bob', 'last_name' => $last_name, 'prefix_id' => 0]) + ->execute()->first(); + // Initial value is NULL, but to test the empty operator, change it to an empty string + \CRM_Core_DAO::executeQuery("UPDATE civicrm_contact SET middle_name = '' WHERE id = " . $bob['id']); + + $jan = Contact::create() + ->setValues(['first_name' => 'Jan', 'middle_name' => 'J', 'last_name' => $last_name, 'prefix_id' => 1]) + ->execute()->first(); + + $dan = Contact::create() + ->setValues(['first_name' => 'Dan', 'last_name' => $last_name, 'prefix_id' => NULL]) + ->execute()->first(); + + // Test EMPTY and NULL operators on string fields + $result = Contact::get(FALSE) + ->addWhere('last_name', '=', $last_name) + ->addWhere('middle_name', 'IS EMPTY') + ->execute()->indexBy('id'); + $this->assertCount(2, $result); + $this->assertArrayNotHasKey($jan['id'], (array) $result); + + $result = Contact::get(FALSE) + ->addWhere('last_name', '=', $last_name) + ->addWhere('middle_name', 'IS NOT NULL') + ->execute()->indexBy('id'); + $this->assertCount(2, $result); + $this->assertArrayNotHasKey($dan['id'], (array) $result); + + $result = Contact::get(FALSE) + ->addWhere('last_name', '=', $last_name) + ->addWhere('middle_name', 'IS NOT EMPTY') + ->execute()->indexBy('id'); + $this->assertCount(1, $result); + $this->assertArrayHasKey($jan['id'], (array) $result); + + // Test EMPTY and NULL operators on Integer fields + $result = Contact::get(FALSE) + ->addWhere('last_name', '=', $last_name) + ->addWhere('prefix_id', 'IS EMPTY') + ->execute()->indexBy('id'); + $this->assertCount(2, $result); + $this->assertArrayNotHasKey($jan['id'], (array) $result); + + $result = Contact::get(FALSE) + ->addWhere('last_name', '=', $last_name) + ->addWhere('prefix_id', 'IS NOT NULL') + ->execute()->indexBy('id'); + $this->assertCount(2, $result); + $this->assertArrayNotHasKey($dan['id'], (array) $result); + + $result = Contact::get(FALSE) + ->addWhere('last_name', '=', $last_name) + ->addWhere('prefix_id', 'IS NOT EMPTY') + ->execute()->indexBy('id'); + $this->assertCount(1, $result); + $this->assertArrayHasKey($jan['id'], (array) $result); + } + } -- 2.25.1