APIv4 - Improve pseudoconstants in HAVING and Explorer

civicrm · Apr 30, 2020 · fab7eb3 · fab7eb3
1 parent 99997b6
commit fab7eb3
Show file tree

Hide file tree

Showing 8 changed files with 121 additions and 41 deletions.
diff --git a/Civi/Api4/Generic/Traits/CustomValueActionTrait.php b/Civi/Api4/Generic/Traits/CustomValueActionTrait.php
@@ -57,7 +57,7 @@ protected function writeObjects($items) {
     $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
@@ -125,7 +125,7 @@ protected function writeObjects($items) {
 
     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
@@ -49,6 +49,7 @@ class Api4SelectQuery extends SelectQuery {
 
   /**
    * @var array
+   * [alias => expr][]
    */
   protected $selectAliases = [];
 
@@ -321,20 +322,39 @@ protected function composeClause(array $clause, string $type) {
     // 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
@@ -34,20 +34,19 @@ 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];
         // Hack for null values -- see comment below
         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 @@ public static function formatWriteParams(&$params, $entity, $fields) {
    * @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 @@ public static function formatInputValue(&$value, $fieldName, $fieldSpec, $entity
     }
     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
@@ -87,11 +87,11 @@ <h1 crm-page-title>
           <fieldset ng-repeat="name in ['values', 'defaults']" ng-if="::availableParams[name]" ng-mouseenter="help(name, availableParams[name])" ng-mouseleave="help()">
             <legend>{{:: name }}<span class="crm-marker" ng-if="::availableParams[name].required"> *</span></legend>
             <div class="api4-input form-inline" ng-repeat="clause in params[name]" ng-mouseenter="help('value: ' + clause[0], fieldHelp(clause[0]))" ng-mouseleave="help(name, availableParams[name])">
-              <input class="collapsible-optgroups form-control" ng-model="clause[0]" crm-ui-select="{formatResult: formatSelect2Item, formatSelection: formatSelect2Item, data: fieldList(name), allowClear: true, placeholder: 'Field'}" />
+              <input class="collapsible-optgroups form-control twenty" ng-model="clause[0]" crm-ui-select="{formatResult: formatSelect2Item, formatSelection: formatSelect2Item, data: fieldList(name), allowClear: true, placeholder: 'Field'}" />
               <input class="form-control" ng-model="clause[1]" api4-exp-value="{field: clause[0], action: action === 'getFields' ? params.action || 'get' : action}" />
             </div>
             <div class="api4-input form-inline">
-              <input class="collapsible-optgroups form-control" ng-model="controls[name]" crm-ui-select="{formatResult: formatSelect2Item, formatSelection: formatSelect2Item, data: fieldList(name), placeholder: ts('Add %1', {1: name.slice(0, -1)})}"/>
+              <input class="collapsible-optgroups form-control twenty" ng-model="controls[name]" crm-ui-select="{formatResult: formatSelect2Item, formatSelection: formatSelect2Item, data: fieldList(name), placeholder: ts('Add %1', {1: name.slice(0, -1)})}"/>
             </div>
           </fieldset>
           <fieldset ng-if="::availableParams.groupBy" ng-mouseenter="help('groupBy', availableParams.groupBy)" ng-mouseleave="help()">

diff --git a/ang/api4Explorer/Explorer.js b/ang/api4Explorer/Explorer.js
@@ -112,20 +112,29 @@
       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),
         fks = _.findWhere(links, {entity: $scope.entity}) || {};
       _.each(fks.links, 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,
@@ -136,6 +145,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;
@@ -197,12 +219,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};
       };
@@ -340,11 +365,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) {
@@ -356,12 +381,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) {
@@ -425,7 +450,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});
               });
             });
@@ -973,17 +998,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')},
@@ -998,11 +1023,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];
         }
@@ -1144,8 +1169,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
@@ -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
@@ -50,6 +50,12 @@ public function testOptionValue() {
     $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 @@ public function testOptionValue() {
     $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() {