From 1fd2aa717ac8737191e9165a725e77305fdd7d44 Mon Sep 17 00:00:00 2001 From: Coleman Watts Date: Mon, 30 Aug 2021 11:39:02 -0400 Subject: [PATCH] SearchKit - Move field formatting from client-side to server-side Formattig field values server-side allows the view values to be reused in exports. It also opens the door for more complex formatting to happen on the server, if needed. --- Civi/Api4/Query/SqlExpression.php | 14 ++++ Civi/Api4/Query/SqlFunction.php | 14 ---- Civi/Api4/Query/SqlFunctionLOWER.php | 2 + Civi/Api4/Query/SqlFunctionUPPER.php | 2 + Civi/Api4/Query/SqlNumber.php | 2 + Civi/Api4/Query/SqlString.php | 2 + .../SearchDisplay/AbstractRunAction.php | 73 ++++++++++++++++++- .../Civi/Api4/Action/SearchDisplay/Run.php | 11 ++- .../ang/crmSearchDisplay/colType/field.html | 2 +- .../crmSearchDisplayEditable.component.js | 6 +- .../traits/searchDisplayBaseTrait.service.js | 35 +++------ .../traits/searchDisplayTasksTrait.service.js | 12 +-- .../api/v4/SearchDisplay/SearchRunTest.php | 32 ++++---- 13 files changed, 140 insertions(+), 67 deletions(-) diff --git a/Civi/Api4/Query/SqlExpression.php b/Civi/Api4/Query/SqlExpression.php index 5f80321b67..8e3d43254d 100644 --- a/Civi/Api4/Query/SqlExpression.php +++ b/Civi/Api4/Query/SqlExpression.php @@ -45,6 +45,13 @@ abstract class SqlExpression { */ public $supportsExpansion = FALSE; + /** + * Data type output by this expression + * + * @var string + */ + protected static $dataType; + /** * SqlFunction constructor. * @param string $expr @@ -166,4 +173,11 @@ abstract class SqlExpression { return substr($className, strrpos($className, '\\') + 1); } + /** + * @return string|NULL + */ + public static function getDataType():? string { + return static::$dataType; + } + } diff --git a/Civi/Api4/Query/SqlFunction.php b/Civi/Api4/Query/SqlFunction.php index c7ee3ad713..457186c2e8 100644 --- a/Civi/Api4/Query/SqlFunction.php +++ b/Civi/Api4/Query/SqlFunction.php @@ -30,13 +30,6 @@ abstract class SqlFunction extends SqlExpression { */ protected static $category; - /** - * Data type output by this function - * - * @var string - */ - protected static $dataType; - const CATEGORY_AGGREGATE = 'aggregate', CATEGORY_COMPARISON = 'comparison', CATEGORY_DATE = 'date', @@ -288,13 +281,6 @@ abstract class SqlFunction extends SqlExpression { return static::$category; } - /** - * @return string|NULL - */ - public static function getDataType():? string { - return static::$dataType; - } - /** * @return string */ diff --git a/Civi/Api4/Query/SqlFunctionLOWER.php b/Civi/Api4/Query/SqlFunctionLOWER.php index 47763c2bc4..d1a77b6b5f 100644 --- a/Civi/Api4/Query/SqlFunctionLOWER.php +++ b/Civi/Api4/Query/SqlFunctionLOWER.php @@ -16,6 +16,8 @@ namespace Civi\Api4\Query; */ class SqlFunctionLOWER extends SqlFunction { + protected static $dataType = 'String'; + protected static $category = self::CATEGORY_STRING; protected static function params(): array { diff --git a/Civi/Api4/Query/SqlFunctionUPPER.php b/Civi/Api4/Query/SqlFunctionUPPER.php index cc21cd1aaa..57e3ae3243 100644 --- a/Civi/Api4/Query/SqlFunctionUPPER.php +++ b/Civi/Api4/Query/SqlFunctionUPPER.php @@ -16,6 +16,8 @@ namespace Civi\Api4\Query; */ class SqlFunctionUPPER extends SqlFunction { + protected static $dataType = 'String'; + protected static $category = self::CATEGORY_STRING; protected static function params(): array { diff --git a/Civi/Api4/Query/SqlNumber.php b/Civi/Api4/Query/SqlNumber.php index 064121bfa9..c3331bd953 100644 --- a/Civi/Api4/Query/SqlNumber.php +++ b/Civi/Api4/Query/SqlNumber.php @@ -16,6 +16,8 @@ namespace Civi\Api4\Query; */ class SqlNumber extends SqlExpression { + protected static $dataType = 'Float'; + protected function initialize() { \CRM_Utils_Type::validate($this->expr, 'Float'); } diff --git a/Civi/Api4/Query/SqlString.php b/Civi/Api4/Query/SqlString.php index 8ea9c00137..ae2208d6f3 100644 --- a/Civi/Api4/Query/SqlString.php +++ b/Civi/Api4/Query/SqlString.php @@ -16,6 +16,8 @@ namespace Civi\Api4\Query; */ class SqlString extends SqlExpression { + protected static $dataType = 'String'; + protected function initialize() { // Remove surrounding quotes $str = substr($this->expr, 1, -1); diff --git a/ext/search_kit/Civi/Api4/Action/SearchDisplay/AbstractRunAction.php b/ext/search_kit/Civi/Api4/Action/SearchDisplay/AbstractRunAction.php index bdf3a0f2f8..d308143fb3 100644 --- a/ext/search_kit/Civi/Api4/Action/SearchDisplay/AbstractRunAction.php +++ b/ext/search_kit/Civi/Api4/Action/SearchDisplay/AbstractRunAction.php @@ -3,6 +3,7 @@ namespace Civi\Api4\Action\SearchDisplay; use Civi\API\Exception\UnauthorizedException; +use Civi\Api4\Query\SqlExpression; use Civi\Api4\SavedSearch; use Civi\Api4\SearchDisplay; use Civi\Api4\Utils\CoreUtil; @@ -91,12 +92,48 @@ abstract class AbstractRunAction extends \Civi\Api4\Generic\AbstractAction { $this->savedSearch['api_params'] += ['where' => []]; $this->savedSearch['api_params']['checkPermissions'] = empty($this->display['acl_bypass']); + $this->display['settings']['columns'] = $this->display['settings']['columns'] ?? []; $this->processResult($result); } abstract protected function processResult(\Civi\Api4\Generic\Result $result); + /** + * Transform each value returned by the API into 'raw' and 'view' properties + * @param \Civi\Api4\Generic\Result $result + * @return array + */ + protected function formatResult(\Civi\Api4\Generic\Result $result): array { + $select = []; + foreach ($this->savedSearch['api_params']['select'] as $selectExpr) { + $expr = SqlExpression::convert($selectExpr, TRUE); + $item = [ + 'fields' => [], + 'dataType' => $expr->getDataType(), + ]; + foreach ($expr->getFields() as $field) { + $item['fields'][] = $this->getField($field); + } + if (!isset($item['dataType']) && $item['fields']) { + $item['dataType'] = $item['fields'][0]['data_type']; + } + $select[$expr->getAlias()] = $item; + } + $formatted = []; + foreach ($result as $data) { + $row = []; + foreach ($data as $key => $raw) { + $row[$key] = [ + 'raw' => $raw, + 'view' => $this->formatViewValue($select[$key]['dataType'], $raw), + ]; + } + $formatted[] = $row; + } + return $formatted; + } + /** * Returns field definition for a given field or NULL if not found * @param $fieldName @@ -110,6 +147,40 @@ abstract class AbstractRunAction extends \Civi\Api4\Generic\AbstractAction { return $this->_selectQuery->getField($fieldName, FALSE); } + /** + * Format raw field value according to data type + * @param $dataType + * @param mixed $rawValue + * @return array|string + */ + protected function formatViewValue($dataType, $rawValue) { + if (is_array($rawValue)) { + return array_map(function($val) use ($dataType) { + return $this->formatViewValue($dataType, $val); + }, $rawValue); + } + + $formatted = $rawValue; + + switch ($dataType) { + case 'Boolean': + if (is_bool($rawValue)) { + $formatted = $rawValue ? ts('Yes') : ts('No'); + } + break; + + case 'Money': + $formatted = \CRM_Utils_Money::format($rawValue); + break; + + case 'Date': + case 'Timestamp': + $formatted = \CRM_Utils_Date::customFormat($rawValue); + } + + return $formatted; + } + /** * Applies supplied filters to the where clause */ @@ -248,7 +319,7 @@ abstract class AbstractRunAction extends \Civi\Api4\Generic\AbstractAction { */ protected function augmentSelectClause(&$apiParams): void { $possibleTokens = ''; - foreach ($this->display['settings']['columns'] ?? [] as $column) { + foreach ($this->display['settings']['columns'] as $column) { // Collect display values in which a token is allowed $possibleTokens .= ($column['rewrite'] ?? '') . ($column['link']['path'] ?? ''); if (!empty($column['links'])) { diff --git a/ext/search_kit/Civi/Api4/Action/SearchDisplay/Run.php b/ext/search_kit/Civi/Api4/Action/SearchDisplay/Run.php index 42017a03b6..ce4a06d7a0 100644 --- a/ext/search_kit/Civi/Api4/Action/SearchDisplay/Run.php +++ b/ext/search_kit/Civi/Api4/Action/SearchDisplay/Run.php @@ -59,10 +59,17 @@ class Run extends AbstractRunAction { $this->applyFilters(); $apiResult = civicrm_api4($entityName, 'get', $apiParams); - + // Copy over meta properties to this result $result->rowCount = $apiResult->rowCount; - $result->exchangeArray($apiResult->getArrayCopy()); $result->debug = $apiResult->debug; + + if ($this->return === 'row_count' || $this->return === 'id') { + $result->exchangeArray($apiResult->getArrayCopy()); + } + else { + $result->exchangeArray($this->formatResult($apiResult)); + } + } } diff --git a/ext/search_kit/ang/crmSearchDisplay/colType/field.html b/ext/search_kit/ang/crmSearchDisplay/colType/field.html index 04ed490ce2..daeb5eb4df 100644 --- a/ext/search_kit/ang/crmSearchDisplay/colType/field.html +++ b/ext/search_kit/ang/crmSearchDisplay/colType/field.html @@ -6,7 +6,7 @@ - {{:: $ctrl.replaceTokens(col.image.alt, row, $ctrl.settings.columns) }} + {{:: $ctrl.replaceTokens(col.image.alt, row) }} {{:: link.value }}, diff --git a/ext/search_kit/ang/crmSearchDisplay/crmSearchDisplayEditable.component.js b/ext/search_kit/ang/crmSearchDisplay/crmSearchDisplayEditable.component.js index 181c0d26d2..b6b6957562 100644 --- a/ext/search_kit/ang/crmSearchDisplay/crmSearchDisplayEditable.component.js +++ b/ext/search_kit/ang/crmSearchDisplay/crmSearchDisplayEditable.component.js @@ -19,8 +19,8 @@ this.$onInit = function() { col = this.col; - this.value = _.cloneDeep(this.row[col.editable.value]); - initialValue = _.cloneDeep(this.row[col.editable.value]); + this.value = _.cloneDeep(this.row[col.editable.value].raw); + initialValue = _.cloneDeep(this.row[col.editable.value].raw); this.field = { data_type: col.dataType, @@ -54,7 +54,7 @@ ctrl.cancel(); return; } - var values = {id: ctrl.row[col.editable.id]}; + var values = {id: ctrl.row[col.editable.id].raw}; values[col.editable.name] = ctrl.value; $('input', $element).attr('disabled', true); crmStatus({}, crmApi4(col.editable.entity, 'update', { diff --git a/ext/search_kit/ang/crmSearchDisplay/traits/searchDisplayBaseTrait.service.js b/ext/search_kit/ang/crmSearchDisplay/traits/searchDisplayBaseTrait.service.js index e43c6370a7..b2a4e9d7f6 100644 --- a/ext/search_kit/ang/crmSearchDisplay/traits/searchDisplayBaseTrait.service.js +++ b/ext/search_kit/ang/crmSearchDisplay/traits/searchDisplayBaseTrait.service.js @@ -6,15 +6,14 @@ var ts = CRM.ts('org.civicrm.search_kit'); // Replace tokens keyed to rowData. - // If rowMeta is provided, values will be formatted; if omitted, raw values will be provided. - function replaceTokens(str, rowData, rowMeta, index) { + // Pass view=true to replace with view value, otherwise raw value is used. + function replaceTokens(str, rowData, view, index) { if (!str) { return ''; } _.each(rowData, function(value, key) { if (str.indexOf('[' + key + ']') >= 0) { - var column = rowMeta && _.findWhere(rowMeta, {key: key}), - val = column ? formatRawValue(column, value) : value, + var val = view ? value.view : value.raw, replacement = angular.isArray(val) ? val[index || 0] : val; str = str.replace(new RegExp(_.escapeRegExp('[' + key + ']', 'g')), replacement); } @@ -23,7 +22,7 @@ } function getUrl(link, rowData, index) { - var url = replaceTokens(link, rowData, null, index); + var url = replaceTokens(link, rowData, false, index); if (url.slice(0, 1) !== '/' && url.slice(0, 4) !== 'http') { url = CRM.url(url); } @@ -33,14 +32,14 @@ // Returns display value for a single column in a row function formatDisplayValue(rowData, key, columns) { var column = _.findWhere(columns, {key: key}), - displayValue = column.rewrite ? replaceTokens(column.rewrite, rowData, columns) : formatRawValue(column, rowData[key]); + displayValue = column.rewrite ? replaceTokens(column.rewrite, rowData, columns) : getValue(rowData[key], 'view'); return angular.isArray(displayValue) ? displayValue.join(', ') : displayValue; } // Returns value and url for a column formatted as link(s) function formatLinks(rowData, key, columns) { var column = _.findWhere(columns, {key: key}), - value = column.image ? '' : formatRawValue(column, rowData[key]), + value = column.image ? '' : getValue(rowData[key], 'view'), values = angular.isArray(value) ? value : [value], links = []; _.each(values, function(value, index) { @@ -52,25 +51,9 @@ return links; } - // Formats raw field value according to data type - function formatRawValue(column, value) { - var type = column && column.dataType, - result = value; - if (_.isArray(value)) { - return _.map(value, function(val) { - return formatRawValue(column, val); - }); - } - if (value && (type === 'Date' || type === 'Timestamp') && /^\d{4}-\d{2}-\d{2}/.test(value)) { - result = CRM.utils.formatDate(value, null, type === 'Timestamp'); - } - else if (type === 'Boolean' && typeof value === 'boolean') { - result = value ? ts('Yes') : ts('No'); - } - else if (type === 'Money' && typeof value === 'number') { - result = CRM.formatMoney(value); - } - return result; + // Get value from column data, specify either 'raw' or 'view' + function getValue(data, ret) { + return (data || {})[ret]; } // Return a base trait shared by all search display controllers diff --git a/ext/search_kit/ang/crmSearchTasks/traits/searchDisplayTasksTrait.service.js b/ext/search_kit/ang/crmSearchTasks/traits/searchDisplayTasksTrait.service.js index 99ecc10df6..b78a189cf9 100644 --- a/ext/search_kit/ang/crmSearchTasks/traits/searchDisplayTasksTrait.service.js +++ b/ext/search_kit/ang/crmSearchTasks/traits/searchDisplayTasksTrait.service.js @@ -23,7 +23,7 @@ // Select all ctrl.allRowsSelected = true; if (ctrl.page === 1 && ctrl.results.length < ctrl.limit) { - ctrl.selectedRows = _.pluck(ctrl.results, 'id'); + ctrl.selectedRows = _.pluck(_.pluck(ctrl.results, 'id'), 'raw'); return; } // If more than one page of results, use ajax to fetch all ids @@ -37,9 +37,9 @@ // Toggle row selection selectRow: function(row) { - var index = this.selectedRows.indexOf(row.id); + var index = this.selectedRows.indexOf(row.id.raw); if (index < 0) { - this.selectedRows.push(row.id); + this.selectedRows.push(row.id.raw); this.allRowsSelected = (this.rowCount === this.selectedRows.length); } else { this.allRowsSelected = false; @@ -49,7 +49,7 @@ // @return bool isRowSelected: function(row) { - return this.allRowsSelected || _.includes(this.selectedRows, row.id); + return this.allRowsSelected || _.includes(this.selectedRows, row.id.raw); }, refreshAfterTask: function() { @@ -69,8 +69,8 @@ onPostRun: [function(results, status, editedRow) { if (editedRow && status === 'success') { // If edited row disappears (because edits cause it to not meet search criteria), deselect it - var index = this.selectedRows.indexOf(editedRow.id); - if (index > -1 && !_.findWhere(results, {id: editedRow.id})) { + var index = this.selectedRows.indexOf(editedRow.id.raw); + if (index > -1 && !_.findWhere(results, {id: editedRow.id.raw})) { this.selectedRows.splice(index, 1); } } diff --git a/ext/search_kit/tests/phpunit/api/v4/SearchDisplay/SearchRunTest.php b/ext/search_kit/tests/phpunit/api/v4/SearchDisplay/SearchRunTest.php index d833c17ff3..c22b32e6de 100644 --- a/ext/search_kit/tests/phpunit/api/v4/SearchDisplay/SearchRunTest.php +++ b/ext/search_kit/tests/phpunit/api/v4/SearchDisplay/SearchRunTest.php @@ -52,7 +52,7 @@ class SearchRunTest extends \PHPUnit\Framework\TestCase implements HeadlessInter 'api_entity' => 'Contact', 'api_params' => [ 'version' => 4, - 'select' => ['id', 'first_name', 'last_name', 'contact_sub_type:label'], + 'select' => ['id', 'first_name', 'last_name', 'contact_sub_type:label', 'is_deceased'], 'where' => [], ], ], @@ -103,15 +103,19 @@ class SearchRunTest extends \PHPUnit\Framework\TestCase implements HeadlessInter $params['filters']['first_name'] = ['One', 'Two']; $result = civicrm_api4('SearchDisplay', 'run', $params); $this->assertCount(2, $result); - $this->assertEquals('One', $result[0]['first_name']); - $this->assertEquals('Two', $result[1]['first_name']); + $this->assertEquals('One', $result[0]['first_name']['raw']); + $this->assertEquals('Two', $result[1]['first_name']['raw']); - $params['filters'] = ['id' => ['>' => $result[0]['id'], '<=' => $result[1]['id'] + 1]]; + // Raw value should be boolean, view value should be string + $this->assertEquals(FALSE, $result[0]['is_deceased']['raw']); + $this->assertEquals(ts('No'), $result[0]['is_deceased']['view']); + + $params['filters'] = ['id' => ['>' => $result[0]['id']['raw'], '<=' => $result[1]['id']['raw'] + 1]]; $params['sort'] = [['first_name', 'ASC']]; $result = civicrm_api4('SearchDisplay', 'run', $params); $this->assertCount(2, $result); - $this->assertEquals('Three', $result[0]['first_name']); - $this->assertEquals('Two', $result[1]['first_name']); + $this->assertEquals('Three', $result[0]['first_name']['raw']); + $this->assertEquals('Two', $result[1]['first_name']['raw']); $params['filters'] = ['contact_sub_type:label' => ['Tester', 'Bot']]; $result = civicrm_api4('SearchDisplay', 'run', $params); @@ -176,9 +180,9 @@ class SearchRunTest extends \PHPUnit\Framework\TestCase implements HeadlessInter $result = civicrm_api4('SearchDisplay', 'run', $params); $this->assertCount(2, $result); - $this->assertNotEmpty($result->first()['display_name']); + $this->assertNotEmpty($result->first()['display_name']['raw']); // Assert that display name was added to the search due to the link token - $this->assertNotEmpty($result->first()['sort_name']); + $this->assertNotEmpty($result->first()['sort_name']['raw']); // These items are not part of the search, but will be added via links $this->assertArrayNotHasKey('contact_type', $result->first()); @@ -195,9 +199,9 @@ class SearchRunTest extends \PHPUnit\Framework\TestCase implements HeadlessInter ], ]; $result = civicrm_api4('SearchDisplay', 'run', $params); - $this->assertEquals('Individual', $result->first()['contact_type']); - $this->assertEquals('Unit test', $result->first()['source']); - $this->assertEquals($lastName, $result->first()['last_name']); + $this->assertEquals('Individual', $result->first()['contact_type']['raw']); + $this->assertEquals('Unit test', $result->first()['source']['raw']); + $this->assertEquals($lastName, $result->first()['last_name']['raw']); } /** @@ -294,14 +298,14 @@ class SearchRunTest extends \PHPUnit\Framework\TestCase implements HeadlessInter $this->cleanupCachedPermissions(); $result = civicrm_api4('SearchDisplay', 'run', $params); $this->assertCount(1, $result); - $this->assertEquals($sampleData['Two'], $result[0]['id']); + $this->assertEquals($sampleData['Two'], $result[0]['id']['raw']); $hooks->setHook('civicrm_aclWhereClause', [$this, 'aclWhereGreaterThan']); $this->cleanupCachedPermissions(); $result = civicrm_api4('SearchDisplay', 'run', $params); $this->assertCount(2, $result); - $this->assertEquals($sampleData['Three'], $result[0]['id']); - $this->assertEquals($sampleData['Four'], $result[1]['id']); + $this->assertEquals($sampleData['Three'], $result[0]['id']['raw']); + $this->assertEquals($sampleData['Four'], $result[1]['id']['raw']); } public function testWithACLBypass() { -- 2.25.1