From 173405e2eb635bcc23263b475e05489f34cda099 Mon Sep 17 00:00:00 2001 From: Coleman Watts Date: Sat, 18 Sep 2021 10:17:22 -0400 Subject: [PATCH] SearchKit - Add UI for multiple function arguments --- Civi/Api4/Query/SqlExpression.php | 8 +- Civi/Api4/Query/SqlFunction.php | 10 +-- Civi/Api4/Query/SqlFunctionCOUNT.php | 1 - Civi/Api4/Query/SqlFunctionGREATEST.php | 1 + Civi/Api4/Query/SqlFunctionLEAST.php | 1 + .../ang/crmSearchAdmin/compose.html | 23 ++--- .../crmSearchAdmin.component.js | 8 ++ .../crmSearchFunction.component.js | 87 ++++++++++++++----- .../ang/crmSearchAdmin/crmSearchFunction.html | 21 ++++- .../crmSearchAdminResultsTable.component.js | 2 + 10 files changed, 113 insertions(+), 49 deletions(-) diff --git a/Civi/Api4/Query/SqlExpression.php b/Civi/Api4/Query/SqlExpression.php index 8e3d43254d..338ae5b057 100644 --- a/Civi/Api4/Query/SqlExpression.php +++ b/Civi/Api4/Query/SqlExpression.php @@ -73,11 +73,10 @@ abstract class SqlExpression { * @param string $expression * @param bool $parseAlias * @param array $mustBe - * @param array $cantBe * @return SqlExpression * @throws \API_Exception */ - public static function convert(string $expression, $parseAlias = FALSE, $mustBe = [], $cantBe = ['SqlWild']) { + public static function convert(string $expression, $parseAlias = FALSE, $mustBe = []) { $as = $parseAlias ? strrpos($expression, ' AS ') : FALSE; $expr = $as ? substr($expression, 0, $as) : $expression; $alias = $as ? \CRM_Utils_String::munge(substr($expression, $as + 4), '_', 256) : NULL; @@ -114,11 +113,6 @@ abstract class SqlExpression { throw new \API_Exception('Unable to parse sql expression: ' . $expression); } $sqlExpression = new $className($expr, $alias); - foreach ($cantBe as $cant) { - if (is_a($sqlExpression, __NAMESPACE__ . '\\' . $cant)) { - throw new \API_Exception('Illegal sql expression.'); - } - } if ($mustBe) { foreach ($mustBe as $must) { if (is_a($sqlExpression, __NAMESPACE__ . '\\' . $must)) { diff --git a/Civi/Api4/Query/SqlFunction.php b/Civi/Api4/Query/SqlFunction.php index 521a67b3bc..94bf31b85d 100644 --- a/Civi/Api4/Query/SqlFunction.php +++ b/Civi/Api4/Query/SqlFunction.php @@ -67,7 +67,7 @@ abstract class SqlFunction extends SqlExpression { 'suffix' => [], ]; if ($param['max_expr'] && (!$param['name'] || $param['name'] === $prefix)) { - $exprs = $this->captureExpressions($arg, $param['must_be'], $param['cant_be']); + $exprs = $this->captureExpressions($arg, $param['must_be']); if (count($exprs) < $param['min_expr'] || count($exprs) > $param['max_expr']) { throw new \API_Exception('Incorrect number of arguments for SQL function ' . static::getName()); } @@ -116,17 +116,16 @@ abstract class SqlFunction extends SqlExpression { * * @param string $arg * @param array $mustBe - * @param array $cantBe * @return array * @throws \API_Exception */ - private function captureExpressions(&$arg, $mustBe, $cantBe) { + private function captureExpressions(&$arg, $mustBe) { $captured = []; $arg = ltrim($arg); while ($arg) { $item = $this->captureExpression($arg); $arg = ltrim(substr($arg, strlen($item))); - $expr = SqlExpression::convert($item, FALSE, $mustBe, $cantBe); + $expr = SqlExpression::convert($item, FALSE, $mustBe); $this->fields = array_merge($this->fields, $expr->getFields()); $captured[] = $expr; // Keep going if we have a comma indicating another expression follows @@ -253,8 +252,7 @@ abstract class SqlFunction extends SqlExpression { 'flag_before' => [], 'flag_after' => [], 'optional' => FALSE, - 'must_be' => [], - 'cant_be' => ['SqlWild'], + 'must_be' => ['SqlField', 'SqlFunction', 'SqlString', 'SqlNumber', 'SqlNull'], 'api_default' => NULL, ]; } diff --git a/Civi/Api4/Query/SqlFunctionCOUNT.php b/Civi/Api4/Query/SqlFunctionCOUNT.php index e45c9ff67d..360ce7d8af 100644 --- a/Civi/Api4/Query/SqlFunctionCOUNT.php +++ b/Civi/Api4/Query/SqlFunctionCOUNT.php @@ -26,7 +26,6 @@ class SqlFunctionCOUNT extends SqlFunction { 'flag_before' => ['DISTINCT' => ts('Distinct')], 'max_expr' => 1, 'must_be' => ['SqlField', 'SqlWild'], - 'cant_be' => [], ], ]; } diff --git a/Civi/Api4/Query/SqlFunctionGREATEST.php b/Civi/Api4/Query/SqlFunctionGREATEST.php index 3874ce4aec..6a61daad32 100644 --- a/Civi/Api4/Query/SqlFunctionGREATEST.php +++ b/Civi/Api4/Query/SqlFunctionGREATEST.php @@ -24,6 +24,7 @@ class SqlFunctionGREATEST extends SqlFunction { return [ [ 'max_expr' => 99, + 'min_expr' => 2, 'optional' => FALSE, ], ]; diff --git a/Civi/Api4/Query/SqlFunctionLEAST.php b/Civi/Api4/Query/SqlFunctionLEAST.php index bbf903819e..4b9d4e66e5 100644 --- a/Civi/Api4/Query/SqlFunctionLEAST.php +++ b/Civi/Api4/Query/SqlFunctionLEAST.php @@ -24,6 +24,7 @@ class SqlFunctionLEAST extends SqlFunction { return [ [ 'max_expr' => 99, + 'min_expr' => 2, 'optional' => FALSE, ], ]; diff --git a/ext/search_kit/ang/crmSearchAdmin/compose.html b/ext/search_kit/ang/crmSearchAdmin/compose.html index 6bcf443b91..6437a92b57 100644 --- a/ext/search_kit/ang/crmSearchAdmin/compose.html +++ b/ext/search_kit/ang/crmSearchAdmin/compose.html @@ -35,17 +35,6 @@ crm-ui-select="{placeholder: ts('Group By'), data: fieldsForGroupBy, dropdownCss: {width: '300px'}}" on-crm-ui-select="$ctrl.addParam('groupBy', selection)" > -
- - - {{:: ts('Field Transformations') }} - -
-
- -
-
-
@@ -57,3 +46,15 @@
+
+ + + {{:: ts('Field Transformations') }} + +
+ +
+ +
+
+
diff --git a/ext/search_kit/ang/crmSearchAdmin/crmSearchAdmin.component.js b/ext/search_kit/ang/crmSearchAdmin/crmSearchAdmin.component.js index ffc1ad78ec..70d08365a1 100644 --- a/ext/search_kit/ang/crmSearchAdmin/crmSearchAdmin.component.js +++ b/ext/search_kit/ang/crmSearchAdmin/crmSearchAdmin.component.js @@ -383,9 +383,17 @@ // Deletes an item from an array param this.clearParam = function(name, idx) { + if (name === 'select') { + // Function selectors use `ng-repeat` with `track by $index` so must be refreshed when splicing the array + ctrl.hideFuncitons(); + } ctrl.savedSearch.api_params[name].splice(idx, 1); }; + this.hideFuncitons = function() { + $scope.controls.showFunctions = false; + }; + function onChangeSelect(newSelect, oldSelect) { // When removing a column from SELECT, also remove from ORDER BY & HAVING _.each(_.difference(oldSelect, newSelect), function(col) { diff --git a/ext/search_kit/ang/crmSearchAdmin/crmSearchFunction.component.js b/ext/search_kit/ang/crmSearchAdmin/crmSearchFunction.component.js index 0aee848858..cebe91534a 100644 --- a/ext/search_kit/ang/crmSearchAdmin/crmSearchFunction.component.js +++ b/ext/search_kit/ang/crmSearchAdmin/crmSearchFunction.component.js @@ -21,39 +21,57 @@ string: ts('Text') }; - $scope.$watch('$ctrl.expr', function(expr) { - var fieldInfo = searchMeta.parseExpr(expr); - ctrl.path = fieldInfo.path + fieldInfo.suffix; - ctrl.field = [fieldInfo.field]; - ctrl.fn = !fieldInfo.fn ? '' : fieldInfo.fn.name; - ctrl.modifier = fieldInfo.modifier || null; + this.exprTypes = { + SqlField: {label: ts('Field'), type: 'field'}, + SqlString: {label: ts('Text'), type: 'string'}, + SqlNumber: {label: ts('Number'), type: 'number'}, + }; + + this.$onInit = function() { + var info = searchMeta.parseExpr(ctrl.expr); + ctrl.args = info.args; + ctrl.fn = info.fn; + ctrl.fnName = !info.fn ? '' : info.fn.name; initFunction(); - }); + }; + + this.addArg = function(exprType) { + exprType = exprType || ctrl.fn.params[0].must_be[0]; + ctrl.args.push({ + type: ctrl.exprTypes[exprType].type, + value: exprType === 'SqlNumber' ? 0 : '' + }); + }; function initFunction() { - ctrl.fnInfo = _.find(CRM.crmSearchAdmin.functions, {name: ctrl.fn}); - if (ctrl.fnInfo && ctrl.fnInfo.params[0] && !_.isEmpty(ctrl.fnInfo.params[0].flag_before)) { - ctrl.modifierName = _.keys(ctrl.fnInfo.params[0].flag_before)[0]; - ctrl.modifierLabel = ctrl.fnInfo.params[0].flag_before[ctrl.modifierName]; + if (!ctrl.fn) { + return; + } + if (ctrl.fn && ctrl.fn.params[0] && !_.isEmpty(ctrl.fn.params[0].flag_before)) { + ctrl.modifierName = _.keys(ctrl.fn.params[0].flag_before)[0]; + ctrl.modifierLabel = ctrl.fn.params[0].flag_before[ctrl.modifierName]; } else { ctrl.modifierName = null; ctrl.modifier = null; } + while (ctrl.args.length < ctrl.fn.params[0].min_expr) { + ctrl.addArg(); + } } // On-demand options for dropdown function selector this.getFunctions = function() { var allowedTypes = [], functions = []; - if (ctrl.expr && ctrl.field) { + if (ctrl.expr && ctrl.args[0] && ctrl.args[0].field) { if (ctrl.crmSearchAdmin.canAggregate(ctrl.expr)) { allowedTypes.push('aggregate'); } else { allowedTypes.push('comparison', 'string'); - if (_.includes(['Integer', 'Float', 'Date', 'Timestamp'], ctrl.field[0].data_type)) { + if (_.includes(['Integer', 'Float', 'Date', 'Timestamp'], ctrl.args[0].field.data_type)) { allowedTypes.push('math'); } - if (_.includes(['Date', 'Timestamp'], ctrl.field[0].data_type)) { + if (_.includes(['Date', 'Timestamp'], ctrl.args[0].field.data_type)) { allowedTypes.push('date'); } } @@ -61,11 +79,8 @@ var allowedFunctions = _.filter(CRM.crmSearchAdmin.functions, function(fn) { return fn.category === type && fn.params.length && - // For now, only support functions that take a single field - fn.params[0].min_expr === 1 && - fn.params[0].max_expr === 1 && - !_.includes(fn.params[0].cant_be, 'SqlField') && - (!fn.params[0].must_be.length || _.includes(fn.params[0].must_be, 'SqlField')); + fn.params[0].min_expr > 0 && + _.includes(fn.params[0].must_be, 'SqlField'); }); functions.push({ text: allTypes[type], @@ -76,22 +91,50 @@ return {results: functions}; }; + this.getFields = function() { + return { + results: ctrl.crmSearchAdmin.getAllFields(':label', ['Field', 'Custom', 'Extra']) + }; + }; + this.selectFunction = function() { + ctrl.fn = _.find(CRM.crmSearchAdmin.functions, {name: ctrl.fnName}); + ctrl.args.length = 1; + initFunction(); ctrl.writeExpr(); }; this.toggleModifier = function() { - ctrl.modifier = ctrl.modifier ? null : ctrl. modifierName; + ctrl.modifier = ctrl.modifier ? null : ctrl.modifierName; + ctrl.writeExpr(); + }; + + this.changeArg = function(index) { + var val = ctrl.args[index].value; + // Delete empty value + if (!val && ctrl.args.length > ctrl.fn.params[0].min_expr) { + ctrl.args.splice(index, 1); + } ctrl.writeExpr(); }; // Make a sql-friendly alias for this expression function makeAlias() { - return (ctrl.fn + '_' + ctrl.path).replace(/[.:]/g, '_'); + var args = _.pluck(_.filter(_.filter(ctrl.args, 'value'), {type: 'field'}), 'value'); + return (ctrl.fnName + '_' + args.join('_')).replace(/[.:]/g, '_'); } this.writeExpr = function() { - ctrl.expr = ctrl.fn ? (ctrl.fn + '(' + (ctrl.modifier ? ctrl.modifier + ' ' : '') + ctrl.path + ') AS ' + makeAlias()) : ctrl.path; + if (ctrl.fnName) { + var args = _.transform(ctrl.args, function(args, arg) { + if (arg.value) { + args.push(arg.type === 'string' ? JSON.stringify(arg.value) : arg.value); + } + }); + ctrl.expr = ctrl.fnName + '(' + (ctrl.modifier ? ctrl.modifier + ' ' : '') + args.join(', ') + ') AS ' + makeAlias(); + } else { + ctrl.expr = ctrl.args[0].value; + } }; } }); diff --git a/ext/search_kit/ang/crmSearchAdmin/crmSearchFunction.html b/ext/search_kit/ang/crmSearchAdmin/crmSearchFunction.html index 258433196a..5ba166840a 100644 --- a/ext/search_kit/ang/crmSearchAdmin/crmSearchFunction.html +++ b/ext/search_kit/ang/crmSearchAdmin/crmSearchFunction.html @@ -1,8 +1,25 @@
- - + + +
+ + + + + +
+
+ + +
diff --git a/ext/search_kit/ang/crmSearchAdmin/resultsTable/crmSearchAdminResultsTable.component.js b/ext/search_kit/ang/crmSearchAdmin/resultsTable/crmSearchAdminResultsTable.component.js index 36c751fc2f..f00dc3a831 100644 --- a/ext/search_kit/ang/crmSearchAdmin/resultsTable/crmSearchAdminResultsTable.component.js +++ b/ext/search_kit/ang/crmSearchAdmin/resultsTable/crmSearchAdminResultsTable.component.js @@ -108,6 +108,8 @@ if (!ui.item.sortable.dropindex && ctrl.crmSearchAdmin.groupExists) { ui.item.sortable.cancel(); } + // Function selectors use `ng-repeat` with `track by $index` so must be refreshed when rearranging the array + ctrl.crmSearchAdmin.hideFuncitons(); } }; -- 2.25.1