From 37d82abe6c5e8116ff9de368b9f591922091019f Mon Sep 17 00:00:00 2001 From: Coleman Watts Date: Thu, 30 Apr 2020 15:14:07 -0400 Subject: [PATCH] APIv4 - Improve pseudoconstants in HAVING and Explorer --- .../Generic/Traits/CustomValueActionTrait.php | 2 +- Civi/Api4/Generic/Traits/DAOActionTrait.php | 2 +- Civi/Api4/Query/Api4SelectQuery.php | 24 +++++- Civi/Api4/Utils/FormattingUtil.php | 17 ++-- ang/api4Explorer/Explorer.html | 4 +- ang/api4Explorer/Explorer.js | 81 +++++++++++++------ css/api4-explorer.css | 4 + .../api/v4/Action/PseudoconstantTest.php | 28 +++++++ 8 files changed, 121 insertions(+), 41 deletions(-) diff --git a/Civi/Api4/Generic/Traits/CustomValueActionTrait.php b/Civi/Api4/Generic/Traits/CustomValueActionTrait.php index b35c789d01..236cb3fc4d 100644 --- a/Civi/Api4/Generic/Traits/CustomValueActionTrait.php +++ b/Civi/Api4/Generic/Traits/CustomValueActionTrait.php @@ -57,7 +57,7 @@ trait CustomValueActionTrait { $result = []; $fields = $this->entityFields(); foreach ($items as $item) { - FormattingUtil::formatWriteParams($item, $this->getEntityName(), $fields); + FormattingUtil::formatWriteParams($item, $fields); // Convert field names to custom_xx format foreach ($fields as $name => $field) { diff --git a/Civi/Api4/Generic/Traits/DAOActionTrait.php b/Civi/Api4/Generic/Traits/DAOActionTrait.php index f4ae2eda0a..45b572a252 100644 --- a/Civi/Api4/Generic/Traits/DAOActionTrait.php +++ b/Civi/Api4/Generic/Traits/DAOActionTrait.php @@ -125,7 +125,7 @@ trait DAOActionTrait { foreach ($items as $item) { $entityId = $item['id'] ?? NULL; - FormattingUtil::formatWriteParams($item, $this->getEntityName(), $this->entityFields()); + FormattingUtil::formatWriteParams($item, $this->entityFields()); $this->formatCustomParams($item, $entityId); $item['check_permissions'] = $this->getCheckPermissions(); diff --git a/Civi/Api4/Query/Api4SelectQuery.php b/Civi/Api4/Query/Api4SelectQuery.php index 7ee2985c52..63cba4e280 100644 --- a/Civi/Api4/Query/Api4SelectQuery.php +++ b/Civi/Api4/Query/Api4SelectQuery.php @@ -49,6 +49,7 @@ class Api4SelectQuery extends SelectQuery { /** * @var array + * [alias => expr][] */ protected $selectAliases = []; @@ -321,20 +322,39 @@ class Api4SelectQuery extends SelectQuery { // For WHERE clause, expr must be the name of a field. if ($type === 'WHERE') { $field = $this->getField($expr, TRUE); - FormattingUtil::formatInputValue($value, $expr, $field, $this->getEntity()); + FormattingUtil::formatInputValue($value, $expr, $field); $fieldAlias = $field['sql_name']; } // For HAVING, expr must be an item in the SELECT clause else { + // Expr references a fieldName or alias if (isset($this->selectAliases[$expr])) { $fieldAlias = $expr; + // Attempt to format if this is a real field + if (isset($this->apiFieldSpec[$expr])) { + FormattingUtil::formatInputValue($value, $expr, $this->apiFieldSpec[$expr]); + } } + // Expr references a non-field expression like a function; convert to alias elseif (in_array($expr, $this->selectAliases)) { $fieldAlias = array_search($expr, $this->selectAliases); } + // If either the having or select field contains a pseudoconstant suffix, match and perform substitution else { - throw new \API_Exception("Invalid expression in $type clause: '$expr'. Must use a value from SELECT clause."); + list($fieldName) = explode(':', $expr); + foreach ($this->selectAliases as $selectAlias => $selectExpr) { + list($selectField) = explode(':', $selectAlias); + if ($selectAlias === $selectExpr && $fieldName === $selectField && isset($this->apiFieldSpec[$fieldName])) { + FormattingUtil::formatInputValue($value, $expr, $this->apiFieldSpec[$fieldName]); + $fieldAlias = $selectAlias; + break; + } + } + } + if (!isset($fieldAlias)) { + throw new \API_Exception("Invalid expression in HAVING clause: '$expr'. Must use a value from SELECT clause."); } + $fieldAlias = '`' . $fieldAlias . '`'; } $sql_clause = \CRM_Core_DAO::createSQLFilter($fieldAlias, [$operator => $value]); diff --git a/Civi/Api4/Utils/FormattingUtil.php b/Civi/Api4/Utils/FormattingUtil.php index e8f27bb594..be91ce41e1 100644 --- a/Civi/Api4/Utils/FormattingUtil.php +++ b/Civi/Api4/Utils/FormattingUtil.php @@ -34,12 +34,11 @@ class FormattingUtil { /** * Massage values into the format the BAO expects for a write operation * - * @param $params - * @param $entity - * @param $fields + * @param array $params + * @param array $fields * @throws \API_Exception */ - public static function formatWriteParams(&$params, $entity, $fields) { + public static function formatWriteParams(&$params, $fields) { foreach ($fields as $name => $field) { if (!empty($params[$name])) { $value =& $params[$name]; @@ -47,7 +46,7 @@ class FormattingUtil { if ($value === 'null') { $value = 'Null'; } - self::formatInputValue($value, $name, $field, $entity); + self::formatInputValue($value, $name, $field); // Ensure we have an array for serialized fields if (!empty($field['serialize'] && !is_array($value))) { $value = (array) $value; @@ -81,11 +80,9 @@ class FormattingUtil { * @param $value * @param string $fieldName * @param array $fieldSpec - * @param string $entity - * Ex: 'Contact', 'Domain' * @throws \API_Exception */ - public static function formatInputValue(&$value, $fieldName, $fieldSpec, $entity) { + public static function formatInputValue(&$value, $fieldName, $fieldSpec) { // Evaluate pseudoconstant suffix $suffix = strpos($fieldName, ':'); if ($suffix) { @@ -94,11 +91,11 @@ class FormattingUtil { } elseif (is_array($value)) { foreach ($value as &$val) { - self::formatInputValue($val, $fieldName, $fieldSpec, $entity); + self::formatInputValue($val, $fieldName, $fieldSpec); } return; } - $fk = $fieldSpec['name'] == 'id' ? $entity : $fieldSpec['fk_entity'] ?? NULL; + $fk = $fieldSpec['name'] == 'id' ? $fieldSpec['entity'] : $fieldSpec['fk_entity'] ?? NULL; if ($fk === 'Domain' && $value === 'current_domain') { $value = \CRM_Core_Config::domainID(); diff --git a/ang/api4Explorer/Explorer.html b/ang/api4Explorer/Explorer.html index af771f9145..56d6e72796 100644 --- a/ang/api4Explorer/Explorer.html +++ b/ang/api4Explorer/Explorer.html @@ -87,11 +87,11 @@
{{:: name }} *
- +
- +
diff --git a/ang/api4Explorer/Explorer.js b/ang/api4Explorer/Explorer.js index 0a27ebeb13..40471bf9d8 100644 --- a/ang/api4Explorer/Explorer.js +++ b/ang/api4Explorer/Explorer.js @@ -112,19 +112,28 @@ return container; } - function getFieldList(action) { + // Returns field list formatted for select2 + function getFieldList(action, addPseudoconstant) { var fields = [], fieldInfo = _.findWhere(getEntity().actions, {name: action}).fields; + if (addPseudoconstant) { + fieldInfo = _.cloneDeep(fieldInfo); + addPseudoconstants(fieldInfo, addPseudoconstant); + } formatForSelect2(fieldInfo, fields, 'name', ['description', 'required', 'default_value']); return fields; } - function addJoins(fieldList, addWildcard) { + // Note: this function expects fieldList to be select2-formatted already + function addJoins(fieldList, addWildcard, addPseudoconstant) { var fields = _.cloneDeep(fieldList); _.each(links[$scope.entity], function(link) { var linkFields = _.cloneDeep(entityFields(link.entity)), wildCard = addWildcard ? [{id: link.alias + '.*', text: link.alias + '.*', 'description': 'All core ' + link.entity + ' fields'}] : []; if (linkFields) { + if (addPseudoconstant) { + addPseudoconstants(linkFields, addPseudoconstant); + } fields.push({ text: link.alias, description: 'Join to ' + link.entity, @@ -135,6 +144,19 @@ return fields; } + // Note: this function transforms a raw list a-la getFields; not a select2-formatted list + function addPseudoconstants(fieldList, toAdd) { + var optionFields = _.filter(fieldList, 'options'); + _.each(optionFields, function(field) { + var pos = _.findIndex(fieldList, {name: field.name}) + 1; + _.each(toAdd, function(suffix) { + var newField = _.cloneDeep(field); + newField.name += ':' + suffix; + fieldList.splice(pos, 0, newField); + }); + }); + } + $scope.help = function(title, content) { if (!content) { $scope.helpTitle = helpTitle; @@ -196,12 +218,15 @@ return info; }; + // Returns field list for write params (values, defaults) $scope.fieldList = function(param) { return function() { - var fields = _.cloneDeep($scope.action === 'getFields' ? getFieldList($scope.params.action || 'get') : $scope.fields); + var fields = _.cloneDeep(getFieldList($scope.action === 'getFields' ? ($scope.params.action || 'get') : $scope.action, ['name'])); // Disable fields that are already in use _.each($scope.params[param] || [], function(val) { - (_.findWhere(fields, {id: val[0]}) || {}).disabled = true; + var usedField = val[0].replace(':name', ''); + (_.findWhere(fields, {id: usedField}) || {}).disabled = true; + (_.findWhere(fields, {id: usedField + ':name'}) || {}).disabled = true; }); return {results: fields}; }; @@ -339,11 +364,11 @@ var actionInfo = _.findWhere(actions, {id: $scope.action}); $scope.fields = getFieldList($scope.action); if (_.contains(['get', 'update', 'delete', 'replace'], $scope.action)) { - $scope.fieldsAndJoins = addJoins($scope.fields); - var fieldsAndFunctions = _.cloneDeep($scope.fields); + $scope.fieldsAndJoins = addJoins(getFieldList($scope.action, ['name'])); + var functions = []; // SQL functions are supported if HAVING is if (actionInfo.params.having) { - fieldsAndFunctions.push({ + functions.push({ text: ts('FUNCTION'), description: ts('Calculate result of a SQL function'), children: _.transform(CRM.vars.api4.functions, function(result, fn) { @@ -355,12 +380,12 @@ }) }); } - $scope.fieldsAndJoinsAndFunctions = addJoins(fieldsAndFunctions, true); - $scope.fieldsAndJoinsAndFunctionsAndWildcards = addJoins(fieldsAndFunctions, true); + $scope.fieldsAndJoinsAndFunctions = addJoins($scope.fields.concat(functions), true); + $scope.fieldsAndJoinsAndFunctionsAndWildcards = addJoins(getFieldList($scope.action, ['name', 'label']).concat(functions), true, ['name', 'label']); } else { - $scope.fieldsAndJoins = $scope.fields; + $scope.fieldsAndJoins = getFieldList($scope.action, ['name']); $scope.fieldsAndJoinsAndFunctions = $scope.fields; - $scope.fieldsAndJoinsAndFunctionsAndWildcards = _.cloneDeep($scope.fields); + $scope.fieldsAndJoinsAndFunctionsAndWildcards = getFieldList($scope.action, ['name', 'label']); } $scope.fieldsAndJoinsAndFunctionsAndWildcards.unshift({id: '*', text: '*', 'description': 'All core ' + $scope.entity + ' fields'}); _.each(actionInfo.params, function (param, name) { @@ -424,7 +449,7 @@ $scope.havingOptions.length = 0; _.each(values, function(item) { var pieces = item.split(' AS '), - alias = _.trim(pieces[pieces.length - 1]); + alias = _.trim(pieces[pieces.length - 1]).replace(':label', ':name'); $scope.havingOptions.push({id: alias, text: alias}); }); }); @@ -972,17 +997,17 @@ $el.crmDatepicker({time: (field.input_attrs && field.input_attrs.time) || false}); } } else if (_.includes(['=', '!=', '<>', 'IN', 'NOT IN'], op) && (field.fk_entity || field.options || dataType === 'Boolean')) { - if (field.fk_entity) { - $el.crmEntityRef({entity: field.fk_entity, select:{multiple: multi}}); - } else if (field.options) { + if (field.options) { + var id = field.pseudoconstant || 'id'; $el.addClass('loading').attr('placeholder', ts('- select -')).crmSelect2({multiple: multi, data: [{id: '', text: ''}]}); loadFieldOptions(field.entity || entity).then(function(data) { - var options = []; - _.each(_.findWhere(data, {name: field.name}).options, function(val, key) { - options.push({id: key, text: val}); - }); - $el.removeClass('loading').select2({data: options, multiple: multi}); + var options = _.transform(data[field.name].options, function(options, opt) { + options.push({id: opt[id], text: opt.label, description: opt.description, color: opt.color, icon: opt.icon}); + }, []); + $el.removeClass('loading').crmSelect2({data: options, multiple: multi}); }); + } else if (field.fk_entity) { + $el.crmEntityRef({entity: field.fk_entity, select:{multiple: multi}}); } else if (dataType === 'Boolean') { $el.attr('placeholder', ts('- select -')).crmSelect2({allowClear: false, multiple: multi, placeholder: ts('- select -'), data: [ {id: 'true', text: ts('Yes')}, @@ -997,11 +1022,11 @@ function loadFieldOptions(entity) { if (!fieldOptions[entity + action]) { fieldOptions[entity + action] = crmApi4(entity, 'getFields', { - loadOptions: true, + loadOptions: ['id', 'name', 'label', 'description', 'color', 'icon'], action: action, - where: [["options", "!=", false]], - select: ["name", "options"] - }); + where: [['options', '!=', false]], + select: ['options'] + }, 'name'); } return fieldOptions[entity + action]; } @@ -1143,8 +1168,14 @@ } function getField(fieldName, entity, action) { + var suffix = fieldName.split(':')[1]; + fieldName = fieldName.split(':')[0]; var fieldNames = fieldName.split('.'); - return get(entity, fieldNames); + var field = get(entity, fieldNames); + if (field && suffix) { + field.pseudoconstant = suffix; + } + return field; function get(entity, fieldNames) { if (fieldNames.length === 1) { diff --git a/css/api4-explorer.css b/css/api4-explorer.css index 64403ba70d..18dc679278 100644 --- a/css/api4-explorer.css +++ b/css/api4-explorer.css @@ -207,6 +207,10 @@ div.select2-drop.collapsible-optgroups-enabled .select2-result-with-children.opt width: 25em; } +#bootstrap-theme.api4-explorer-page .form-control.twenty { + width: 20em; +} + /* Another weird shoreditch fix */ #bootstrap-theme .form-inline div.checkbox { margin-right: 1em; diff --git a/tests/phpunit/api/v4/Action/PseudoconstantTest.php b/tests/phpunit/api/v4/Action/PseudoconstantTest.php index 220e534997..277c47be68 100644 --- a/tests/phpunit/api/v4/Action/PseudoconstantTest.php +++ b/tests/phpunit/api/v4/Action/PseudoconstantTest.php @@ -50,6 +50,12 @@ class PseudoconstantTest extends BaseCustomValueTest { $options = array_column($options, NULL, 'name'); $this->assertEquals('Fake Type', $options['Fake_Type']['label']); + Activity::create() + ->addValue('activity_type_id:name', 'Meeting') + ->addValue('source_contact_id', $cid) + ->addValue('subject', $subject) + ->execute(); + Activity::create() ->addValue('activity_type_id:name', 'Fake_Type') ->addValue('source_contact_id', $cid) @@ -68,6 +74,28 @@ class PseudoconstantTest extends BaseCustomValueTest { $this->assertEquals('Fake Type', $act[0]['activity_type_id:label']); $this->assertEquals('Fake_Type', $act[0]['activity_type_id:name']); $this->assertTrue(is_numeric($act[0]['activity_type_id'])); + + $act = Activity::get() + ->addHaving('activity_type_id:name', '=', 'Fake_Type') + ->addHaving('subject', '=', $subject) + ->addSelect('activity_type_id:label') + ->addSelect('activity_type_id') + ->addSelect('subject') + ->execute(); + + $this->assertCount(1, $act); + $this->assertEquals('Fake Type', $act[0]['activity_type_id:label']); + $this->assertTrue(is_numeric($act[0]['activity_type_id'])); + + $act = Activity::get() + ->addHaving('activity_type_id:name', '=', 'Fake_Type') + ->addHaving('subject', '=', $subject) + ->addSelect('activity_type_id') + ->addSelect('subject') + ->execute(); + + $this->assertCount(1, $act); + $this->assertTrue(is_numeric($act[0]['activity_type_id'])); } public function testAddressOptions() { -- 2.25.1