From cc3044227ce18f02a7e67c0dad942273b00296ec Mon Sep 17 00:00:00 2001 From: colemanw Date: Fri, 23 Jun 2023 20:54:31 -0700 Subject: [PATCH] API/SearchKit - Improve dynamic pseudoconstant lookups and ACL adminUI table --- CRM/ACL/BAO/ACL.php | 51 ++++++++++--------- CRM/ACL/DAO/ACL.php | 11 +++- CRM/Core/BAO/CustomGroup.php | 18 ++----- CRM/Core/DAO.php | 2 +- CRM/Core/DAO/CustomGroup.php | 5 +- Civi/Api4/Generic/AbstractAction.php | 13 +++-- Civi/Api4/Utils/CoreUtil.php | 27 ++++++++++ Civi/Api4/Utils/FormattingUtil.php | 18 +++++-- .../ang/afsearchManageACLs.aff.html | 5 ++ .../managed/SavedSearch_Manage_ACLs.mgd.php | 2 +- .../SearchDisplay/AbstractRunAction.php | 15 +++++- tests/phpunit/api/v4/Custom/CoreUtilTest.php | 31 +++++++++++ xml/schema/ACL/ACL.xml | 9 ++++ xml/schema/Core/CustomGroup.xml | 2 +- 14 files changed, 151 insertions(+), 58 deletions(-) diff --git a/CRM/ACL/BAO/ACL.php b/CRM/ACL/BAO/ACL.php index 8ab4959f13..d8b84cec67 100644 --- a/CRM/ACL/BAO/ACL.php +++ b/CRM/ACL/BAO/ACL.php @@ -541,35 +541,36 @@ SELECT g.* } public static function getObjectIdOptions($context, $params): array { - if (empty($params['values']['object_table']) && empty($params['values']['object_table:label'])) { - return []; - } - if (!empty($params['values']['object_table:label'])) { - $table_name = array_flip(self::getObjectTableOptions())[$params['values']['object_table:label']]; + $tableName = $params['values']['object_table'] ?? NULL; + // Look up object_table if not known + if (!$tableName && !empty($params['values']['id'])) { + $tableName = CRM_Core_DAO::getFieldValue('CRM_ACL_DAO_ACL', $params['values']['id'], 'object_table'); } - else { - $table_name = $params['values']['object_table']; + if (!$tableName) { + return []; } - $finalOptions = []; - $entity = CoreUtil::getApiNameFromTableName($table_name); - $label = CoreUtil::getInfoItem($entity, 'label_field'); - $titlePlural = CoreUtil::getInfoItem($entity, 'title_plural'); - $finalOptions[] = [ - 'label' => ts('All %1', [1 => $titlePlural]), - 'id' => 0, - 'name' => 0, - ]; - $options = civicrm_api4($entity, 'get', [ - 'select' => [$label, 'id', 'name'], - ]); - foreach ($options as $option) { - $finalOptions[] = [ - 'label' => $option[$label], - 'id' => $option['id'], - 'name' => $option['name'] ?? $option['id'], + if (!isset(Civi::$statics[__FUNCTION__][$tableName])) { + $entity = CoreUtil::getApiNameFromTableName($tableName); + $label = CoreUtil::getInfoItem($entity, 'label_field'); + $titlePlural = CoreUtil::getInfoItem($entity, 'title_plural'); + Civi::$statics[__FUNCTION__][$tableName] = []; + Civi::$statics[__FUNCTION__][$tableName][] = [ + 'label' => ts('All %1', [1 => $titlePlural]), + 'id' => 0, + 'name' => 0, ]; + $options = civicrm_api4($entity, 'get', [ + 'select' => [$label, 'id', 'name'], + ]); + foreach ($options as $option) { + Civi::$statics[__FUNCTION__][$tableName][] = [ + 'label' => $option[$label], + 'id' => $option['id'], + 'name' => $option['name'] ?? $option['id'], + ]; + } } - return $finalOptions; + return Civi::$statics[__FUNCTION__][$tableName]; } } diff --git a/CRM/ACL/DAO/ACL.php b/CRM/ACL/DAO/ACL.php index ac13bb14c9..c370ec642b 100644 --- a/CRM/ACL/DAO/ACL.php +++ b/CRM/ACL/DAO/ACL.php @@ -6,7 +6,7 @@ * * Generated from xml/schema/CRM/ACL/ACL.xml * DO NOT EDIT. Generated by CRM_Core_CodeGen - * (GenCodeChecksum:79ed87b3613a7a626f02242218256dd1) + * (GenCodeChecksum:a77f71abc7be09b3df836ddb6e5d2990) */ /** @@ -345,6 +345,10 @@ class CRM_ACL_DAO_ACL extends CRM_Core_DAO { 'entity' => 'ACL', 'bao' => 'CRM_ACL_BAO_ACL', 'localizable' => 0, + 'html' => [ + 'type' => 'Select', + 'label' => ts("Type of Data"), + ], 'pseudoconstant' => [ 'callback' => 'CRM_ACL_BAO_ACL::getObjectTableOptions', ], @@ -366,6 +370,11 @@ class CRM_ACL_DAO_ACL extends CRM_Core_DAO { 'entity' => 'ACL', 'bao' => 'CRM_ACL_BAO_ACL', 'localizable' => 0, + 'html' => [ + 'type' => 'Select', + 'label' => ts("Which Data"), + 'controlField' => 'object_table', + ], 'pseudoconstant' => [ 'callback' => 'CRM_ACL_BAO_ACL::getObjectIdOptions', 'prefetch' => 'false', diff --git a/CRM/Core/BAO/CustomGroup.php b/CRM/Core/BAO/CustomGroup.php index 5ce0d84311..d360fa54f7 100644 --- a/CRM/Core/BAO/CustomGroup.php +++ b/CRM/Core/BAO/CustomGroup.php @@ -2059,10 +2059,10 @@ SELECT civicrm_custom_group.id as groupID, civicrm_custom_group.title as groupT * * @param $context * @param array $params - * @param array $props * @return array */ - public static function getExtendsEntityColumnValueOptions($context, $params, $props = []) { + public static function getExtendsEntityColumnValueOptions($context, $params) { + $props = $params['values'] ?? []; // Requesting this option list only makes sense if the value of 'extends' is known or can be looked up if (!empty($props['id']) || !empty($props['name']) || !empty($props['extends']) || !empty($props['extends_entity_column_id'])) { $id = $props['id'] ?? NULL; @@ -2147,10 +2147,10 @@ SELECT civicrm_custom_group.id as groupID, civicrm_custom_group.title as groupT * * @param string $context * @param array $params - * @param array $props * @return array */ - public static function getExtendsEntityColumnIdOptions($context = NULL, $params = [], $props = []) { + public static function getExtendsEntityColumnIdOptions($context = NULL, $params = []) { + $props = $params['values'] ?? []; $ogId = CRM_Core_DAO::getFieldValue('CRM_Core_DAO_OptionGroup', 'custom_data_type', 'id', 'name'); $optionValues = CRM_Core_BAO_OptionValue::getOptionValuesArray($ogId); @@ -2195,16 +2195,6 @@ SELECT civicrm_custom_group.id as groupID, civicrm_custom_group.title as groupT * @inheritDoc */ public static function buildOptions($fieldName, $context = NULL, $props = []) { - // This is necessary in order to pass $props to the callback function - if ($fieldName === 'extends_entity_column_value') { - $options = self::getExtendsEntityColumnValueOptions($context, [], $props); - return CRM_Core_PseudoConstant::formatArrayOptions($context, $options); - } - // Provides filtering by 'extends' entity - if ($fieldName === 'extends_entity_column_id') { - $options = self::getExtendsEntityColumnIdOptions($context, [], $props); - return CRM_Core_PseudoConstant::formatArrayOptions($context, $options); - } // Legacy support for APIv3 which also needs the ParticipantEventName etc pseudo-selectors if ($fieldName === 'extends' && ($props['version'] ?? NULL) == 3) { $options = CRM_Core_SelectValues::customGroupExtends(); diff --git a/CRM/Core/DAO.php b/CRM/Core/DAO.php index 6d8375861b..b8838967de 100644 --- a/CRM/Core/DAO.php +++ b/CRM/Core/DAO.php @@ -2779,7 +2779,7 @@ SELECT contact_id * @param array $values * Raw field values; whatever is known about this bao object. * - * Note: $props can contain unsanitized input and should not be passed directly to CRM_Core_PseudoConstant::get + * Note: $values can contain unsanitized input and should be handled with care by CRM_Core_PseudoConstant::get * * @return array|bool */ diff --git a/CRM/Core/DAO/CustomGroup.php b/CRM/Core/DAO/CustomGroup.php index 574988762e..0f32de497d 100644 --- a/CRM/Core/DAO/CustomGroup.php +++ b/CRM/Core/DAO/CustomGroup.php @@ -6,7 +6,7 @@ * * Generated from xml/schema/CRM/Core/CustomGroup.xml * DO NOT EDIT. Generated by CRM_Core_CodeGen - * (GenCodeChecksum:b686c4c7a9eb826d2b2aa39841a01d9d) + * (GenCodeChecksum:aa09bb4ab9224e33693159efddd64089) */ /** @@ -404,8 +404,7 @@ class CRM_Core_DAO_CustomGroup extends CRM_Core_DAO { 'controlField' => 'extends', ], 'pseudoconstant' => [ - 'optionGroupName' => 'custom_data_type', - 'optionEditPath' => 'civicrm/admin/options/custom_data_type', + 'callback' => 'CRM_Core_BAO_CustomGroup::getExtendsEntityColumnIdOptions', ], 'add' => '2.2', ], diff --git a/Civi/Api4/Generic/AbstractAction.php b/Civi/Api4/Generic/AbstractAction.php index 7a45503fca..bef7843610 100644 --- a/Civi/Api4/Generic/AbstractAction.php +++ b/Civi/Api4/Generic/AbstractAction.php @@ -511,23 +511,22 @@ abstract class AbstractAction implements \ArrayAccess { if ($field) { $optionFields[$fieldName] = [ 'val' => $record[$expr], + 'name' => $fieldName, 'expr' => $expr, 'field' => $field, 'suffix' => substr($expr, $suffix + 1), - 'depends' => $field['input_attrs']['control_field'] ?? NULL, + 'input_attrs' => $field['input_attrs'] ?? [], ]; unset($record[$expr]); } } } - // Sort option lookups by dependency, so e.g. country_id is processed first, then state_province_id, then county_id - uasort($optionFields, function ($a, $b) { - return $a['field']['name'] === $b['depends'] ? -1 : 1; - }); + // Sort lookups by `input_attrs.control_field`, so e.g. country_id is processed first, then state_province_id, then county_id + CoreUtil::topSortFields($optionFields); // Replace pseudoconstants. Note this is a reverse lookup as we are evaluating input not output. - foreach ($optionFields as $fieldName => $info) { + foreach ($optionFields as $info) { $options = FormattingUtil::getPseudoconstantList($info['field'], $info['expr'], $record, 'create'); - $record[$fieldName] = FormattingUtil::replacePseudoconstant($options, $info['val'], TRUE); + $record[$info['name']] = FormattingUtil::replacePseudoconstant($options, $info['val'], TRUE); } // The DAO works better with ints than booleans. See https://github.com/civicrm/civicrm-core/pull/23970 foreach ($record as $key => $value) { diff --git a/Civi/Api4/Utils/CoreUtil.php b/Civi/Api4/Utils/CoreUtil.php index 835d1347bc..37f1ef6125 100644 --- a/Civi/Api4/Utils/CoreUtil.php +++ b/Civi/Api4/Utils/CoreUtil.php @@ -386,4 +386,31 @@ class CoreUtil { return $fns; } + /** + * Sorts fields so that control fields are ordered before the fields they control. + * + * @param array[] $fields + * @return void + */ + public static function topSortFields(array &$fields): void { + $indexedFields = array_column($fields, NULL, 'name'); + $needsSort = []; + foreach ($indexedFields as $fieldName => $field) { + if (!empty($field['input_attrs']['control_field']) && array_key_exists($field['input_attrs']['control_field'], $indexedFields)) { + $needsSort[$fieldName] = [$field['input_attrs']['control_field']]; + } + } + // Only fire up the Topological sorter if we actually need it... + if ($needsSort) { + $fields = []; + $sorter = new \MJS\TopSort\Implementations\FixedArraySort(); + foreach ($indexedFields as $fieldName => $field) { + $sorter->add($fieldName, $needsSort[$fieldName] ?? []); + } + foreach ($sorter->sort() as $fieldName) { + $fields[] = $indexedFields[$fieldName]; + } + } + } + } diff --git a/Civi/Api4/Utils/FormattingUtil.php b/Civi/Api4/Utils/FormattingUtil.php index 522833a570..380a7afc94 100644 --- a/Civi/Api4/Utils/FormattingUtil.php +++ b/Civi/Api4/Utils/FormattingUtil.php @@ -221,6 +221,16 @@ class FormattingUtil { */ public static function formatOutputValues(&$result, $fields, $action = 'get', $selectAliases = []) { $contactTypePaths = []; + // Save an array of unprocessed values which are useful when replacing pseudocontants + $rawValues = $result; + foreach ($rawValues as $key => $value) { + // Pseudoconstants haven't been replaced yet so strip suffixes from raw values + if (strpos($key, ':') > strrpos($key, ')')) { + [$fieldName] = explode(':', $key); + $rawValues[$fieldName] = $value; + unset($rawValues[$key]); + } + } foreach ($result as $key => $value) { $fieldExpr = SqlExpression::convert($selectAliases[$key] ?? $key); $fieldName = \CRM_Utils_Array::first($fieldExpr->getFields() ?? ''); @@ -239,7 +249,7 @@ class FormattingUtil { $suffix = strrpos(($fieldName ?? ''), ':'); $fieldOptions = NULL; if (isset($value) && $suffix) { - $fieldOptions = self::getPseudoconstantList($field, $fieldName, $result, $action); + $fieldOptions = self::getPseudoconstantList($field, $fieldName, $rawValues, $action); $dataType = NULL; } // Store contact_type value before replacing pseudoconstant (e.g. transforming it to contact_type:label) @@ -270,13 +280,13 @@ class FormattingUtil { * @param array $field * @param string $fieldAlias * Field path plus pseudoconstant suffix, e.g. 'contact.employer_id.contact_sub_type:label' - * @param array $params + * @param array $values * Other values for this object * @param string $action * @return array * @throws \CRM_Core_Exception */ - public static function getPseudoconstantList(array $field, string $fieldAlias, $params = [], $action = 'get') { + public static function getPseudoconstantList(array $field, string $fieldAlias, $values = [], $action = 'get') { [$fieldPath, $valueType] = explode(':', $fieldAlias); $context = self::$pseudoConstantContexts[$valueType] ?? NULL; // For create actions, only unique identifiers can be used. @@ -288,7 +298,7 @@ class FormattingUtil { // Use BAO::buildOptions if possible if ($baoName) { $fieldName = empty($field['custom_field_id']) ? $field['name'] : 'custom_' . $field['custom_field_id']; - $options = $baoName::buildOptions($fieldName, $context, self::filterByPath($params, $fieldPath, $field['name'])); + $options = $baoName::buildOptions($fieldName, $context, self::filterByPath($values, $fieldPath, $field['name'])); } // Fallback for option lists that exist in the api but not the BAO if (!isset($options) || $options === FALSE) { diff --git a/ext/civicrm_admin_ui/ang/afsearchManageACLs.aff.html b/ext/civicrm_admin_ui/ang/afsearchManageACLs.aff.html index 4864c1fd40..7fc2077942 100644 --- a/ext/civicrm_admin_ui/ang/afsearchManageACLs.aff.html +++ b/ext/civicrm_admin_ui/ang/afsearchManageACLs.aff.html @@ -1,3 +1,8 @@
+
+

{{:: ts('ACLs allow you to control access to CiviCRM data. An ACL consists of an Operation (e.g. "View" or "Edit"), a set of data the operation can be performed on (e.g. a group of contacts), and a user Role the ACL applies to.') }}

+

{{:: ts('Depending on the chosen mode, each ACL either Allows or Denies permission. When multiple rules contradict each other (e.g. one rule allows and the second rule denies access to the same contacts) the rule with the highest priority takes precidence.') }}

+

{{:: ts('Learn more...') }}

+
diff --git a/ext/civicrm_admin_ui/managed/SavedSearch_Manage_ACLs.mgd.php b/ext/civicrm_admin_ui/managed/SavedSearch_Manage_ACLs.mgd.php index 35bff4ca20..388e7aa526 100644 --- a/ext/civicrm_admin_ui/managed/SavedSearch_Manage_ACLs.mgd.php +++ b/ext/civicrm_admin_ui/managed/SavedSearch_Manage_ACLs.mgd.php @@ -157,10 +157,10 @@ return [ 'alignment' => 'text-right', ], ], - 'actions' => TRUE, 'classes' => [ 'table', 'table-striped', + 'crm-sticky-header', ], 'addButton' => [ 'path' => 'civicrm/acl/edit?reset=1&action=add', diff --git a/ext/search_kit/Civi/Api4/Action/SearchDisplay/AbstractRunAction.php b/ext/search_kit/Civi/Api4/Action/SearchDisplay/AbstractRunAction.php index 547f730466..803678022e 100644 --- a/ext/search_kit/Civi/Api4/Action/SearchDisplay/AbstractRunAction.php +++ b/ext/search_kit/Civi/Api4/Action/SearchDisplay/AbstractRunAction.php @@ -949,12 +949,25 @@ abstract class AbstractRunAction extends \Civi\Api4\Generic\AbstractAction { $this->addSelectExpression($addition); } - // When selecting monetary fields, also select currency foreach ($apiParams['select'] as $select) { + // When selecting monetary fields, also select currency $currencyFieldName = $this->getCurrencyField($select); if ($currencyFieldName) { $this->addSelectExpression($currencyFieldName); } + // Add field dependencies needed to resolve pseudoconstants + $clause = $this->getSelectExpression($select); + if ($clause && $clause['expr']->getType() === 'SqlField' && !empty($clause['fields'])) { + $fieldAlias = array_keys($clause['fields'])[0]; + $field = $clause['fields'][$fieldAlias]; + if (!empty($field['input_attrs']['control_field']) && strpos($fieldAlias, ':')) { + $prefix = substr($fieldAlias, 0, strrpos($fieldAlias, $field['name'])); + // Don't need to add the field if a suffixed version already exists + if (!$this->getSelectExpression($prefix . $field['input_attrs']['control_field'] . ':label')) { + $this->addSelectExpression($prefix . $field['input_attrs']['control_field']); + } + } + } } } diff --git a/tests/phpunit/api/v4/Custom/CoreUtilTest.php b/tests/phpunit/api/v4/Custom/CoreUtilTest.php index 93d4abfdd7..8fe46d05ff 100644 --- a/tests/phpunit/api/v4/Custom/CoreUtilTest.php +++ b/tests/phpunit/api/v4/Custom/CoreUtilTest.php @@ -75,7 +75,38 @@ class CoreUtilTest extends CustomTestBase { ->execute()->first(); $this->assertEquals('Civi\Api4\CustomValue', CoreUtil::getApiClass('Custom_' . $multiGroup['name'])); + } + public function testTopSortFields() { + $sampleFields = [ + [ + 'name' => 'd', + 'title' => 'Fourth', + 'input_attrs' => [ + 'control_field' => 'a', + ], + ], + [ + 'name' => 'a', + 'title' => 'Third', + 'input_attrs' => [ + 'control_field' => 'c', + ], + ], + [ + 'name' => 'b', + 'title' => 'First', + ], + [ + 'name' => 'c', + 'title' => 'Second', + 'input_attrs' => [ + 'control_field' => 'b', + ], + ], + ]; + CoreUtil::topSortFields($sampleFields); + $this->assertEquals(['First', 'Second', 'Third', 'Fourth'], array_column($sampleFields, 'title')); } } diff --git a/xml/schema/ACL/ACL.xml b/xml/schema/ACL/ACL.xml index 8cb4dbbfb8..a4369a2d71 100644 --- a/xml/schema/ACL/ACL.xml +++ b/xml/schema/ACL/ACL.xml @@ -94,6 +94,10 @@ varchar 64 The table of the object controlled by this ACL entry + + + Select + CRM_ACL_BAO_ACL::getObjectTableOptions @@ -105,6 +109,11 @@ int unsigned The ID of the object controlled by this ACL entry 1.6 + + + Select + object_table + CRM_ACL_BAO_ACL::getObjectIdOptions false diff --git a/xml/schema/Core/CustomGroup.xml b/xml/schema/Core/CustomGroup.xml index c1b156b05b..f17c572e15 100644 --- a/xml/schema/Core/CustomGroup.xml +++ b/xml/schema/Core/CustomGroup.xml @@ -74,7 +74,7 @@ NULL FK to civicrm_option_value.id (for option group custom_data_type.) - custom_data_type + CRM_Core_BAO_CustomGroup::getExtendsEntityColumnIdOptions 2.2 -- 2.25.1