From 204c3c298e7ceae85a1b8f8b9091bd1042dd49b7 Mon Sep 17 00:00:00 2001 From: Coleman Watts Date: Sun, 6 Feb 2022 15:01:20 -0500 Subject: [PATCH] APIv4 SortableEntity - Fix sorting custom fields with option groups Before: APIv4 would guess which fields to use for grouping when sorting by weight. this caused a bug when sorting custom fields which also had an option_group_id, which was incorrectly guessed to be used for grouping. After: New `@groupWeightsBy` annotation removes the guesswork. --- Civi/Api4/CustomField.php | 1 + Civi/Api4/Entity.php | 5 +++++ Civi/Api4/Generic/Traits/DAOActionTrait.php | 5 ++--- Civi/Api4/MembershipType.php | 1 + Civi/Api4/Navigation.php | 1 + Civi/Api4/OptionValue.php | 1 + Civi/Api4/PriceField.php | 1 + Civi/Api4/PriceFieldValue.php | 1 + Civi/Api4/UFField.php | 1 + Civi/Api4/Utils/ReflectionUtils.php | 2 +- tests/phpunit/api/v4/Action/BasicCustomFieldTest.php | 4 ++-- 11 files changed, 17 insertions(+), 6 deletions(-) diff --git a/Civi/Api4/CustomField.php b/Civi/Api4/CustomField.php index 5751e6492e..413d268d40 100644 --- a/Civi/Api4/CustomField.php +++ b/Civi/Api4/CustomField.php @@ -16,6 +16,7 @@ namespace Civi\Api4; * @see https://docs.civicrm.org/user/en/latest/organising-your-data/creating-custom-fields/ * @searchable secondary * @orderBy weight + * @groupWeightsBy custom_group_id * @since 5.19 * @package Civi\Api4 */ diff --git a/Civi/Api4/Entity.php b/Civi/Api4/Entity.php index 69f5363b45..fbe5d75f67 100644 --- a/Civi/Api4/Entity.php +++ b/Civi/Api4/Entity.php @@ -132,6 +132,11 @@ class Entity extends Generic\AbstractEntity { 'data_type' => 'Array', 'description' => 'When joining entities in the UI, which fields should be presented by default in the ON clause', ], + [ + 'name' => 'group_weights_by', + 'data_type' => 'Array', + 'description' => 'For sortable entities, what field groupings are used to order by weight', + ], ]; }))->setCheckPermissions($checkPermissions); } diff --git a/Civi/Api4/Generic/Traits/DAOActionTrait.php b/Civi/Api4/Generic/Traits/DAOActionTrait.php index b44c7d1fda..c7661f7075 100644 --- a/Civi/Api4/Generic/Traits/DAOActionTrait.php +++ b/Civi/Api4/Generic/Traits/DAOActionTrait.php @@ -330,6 +330,7 @@ trait DAOActionTrait { /** @var \CRM_Core_DAO|string $daoName */ $daoName = CoreUtil::getInfoItem($this->getEntityName(), 'dao'); $weightField = CoreUtil::getInfoItem($this->getEntityName(), 'order_by'); + $grouping = CoreUtil::getInfoItem($this->getEntityName(), 'group_weights_by'); $idField = CoreUtil::getIdFieldName($this->getEntityName()); // If updating an existing record without changing weight, do nothing if (!isset($record[$weightField]) && !empty($record[$idField])) { @@ -339,10 +340,8 @@ trait DAOActionTrait { $newWeight = $record[$weightField] ?? NULL; $oldWeight = empty($record[$idField]) ? NULL : \CRM_Core_DAO::getFieldValue($daoName, $record[$idField], $weightField); - // FIXME: Need a more metadata-ish approach. For now here's a hardcoded list of the fields sortable entities use for grouping. - $guesses = ['option_group_id', 'price_set_id', 'price_field_id', 'premiums_id', 'uf_group_id', 'custom_group_id', 'parent_id', 'domain_id']; $filters = []; - foreach (array_intersect($guesses, array_keys($daoFields)) as $filter) { + foreach ($grouping ?? [] as $filter) { $filters[$filter] = $record[$filter] ?? (empty($record[$idField]) ? NULL : \CRM_Core_DAO::getFieldValue($daoName, $record[$idField], $filter)); } // Supply default weight for new record diff --git a/Civi/Api4/MembershipType.php b/Civi/Api4/MembershipType.php index ba4c6f80d7..37a07191b6 100644 --- a/Civi/Api4/MembershipType.php +++ b/Civi/Api4/MembershipType.php @@ -15,6 +15,7 @@ namespace Civi\Api4; * * @searchable secondary * @orderBy weight + * @groupWeightsBy domain_id * @since 5.27 * @package Civi\Api4 */ diff --git a/Civi/Api4/Navigation.php b/Civi/Api4/Navigation.php index ecf14ffe41..fc1d08e56d 100644 --- a/Civi/Api4/Navigation.php +++ b/Civi/Api4/Navigation.php @@ -15,6 +15,7 @@ namespace Civi\Api4; * * @searchable none * @orderBy weight + * @groupWeightsBy domain_id,parent_id * @since 5.19 * @package Civi\Api4 */ diff --git a/Civi/Api4/OptionValue.php b/Civi/Api4/OptionValue.php index 7ca6e31481..a3486c3e7c 100644 --- a/Civi/Api4/OptionValue.php +++ b/Civi/Api4/OptionValue.php @@ -16,6 +16,7 @@ namespace Civi\Api4; * @see \Civi\Api4\OptionGroup * @searchable secondary * @orderBy weight + * @groupWeightsBy option_group_id * @since 5.19 * @package Civi\Api4 */ diff --git a/Civi/Api4/PriceField.php b/Civi/Api4/PriceField.php index dcbe67026e..8b1aba7ecf 100644 --- a/Civi/Api4/PriceField.php +++ b/Civi/Api4/PriceField.php @@ -15,6 +15,7 @@ namespace Civi\Api4; * * @searchable secondary * @orderBy weight + * @groupWeightsBy price_set_id * @since 5.27 * @package Civi\Api4 */ diff --git a/Civi/Api4/PriceFieldValue.php b/Civi/Api4/PriceFieldValue.php index d6e20203ef..5924902168 100644 --- a/Civi/Api4/PriceFieldValue.php +++ b/Civi/Api4/PriceFieldValue.php @@ -15,6 +15,7 @@ namespace Civi\Api4; * * @searchable secondary * @orderBy weight + * @groupWeightsBy price_field_id * @since 5.27 * @package Civi\Api4 */ diff --git a/Civi/Api4/UFField.php b/Civi/Api4/UFField.php index 7fbd05df2f..01479ada1e 100644 --- a/Civi/Api4/UFField.php +++ b/Civi/Api4/UFField.php @@ -16,6 +16,7 @@ namespace Civi\Api4; * @see \Civi\Api4\UFGroup * @searchable none * @orderBy weight + * @groupWeightsBy uf_group_id * @since 5.19 * @package Civi\Api4 */ diff --git a/Civi/Api4/Utils/ReflectionUtils.php b/Civi/Api4/Utils/ReflectionUtils.php index 0263718e3a..f09d8841d9 100644 --- a/Civi/Api4/Utils/ReflectionUtils.php +++ b/Civi/Api4/Utils/ReflectionUtils.php @@ -88,7 +88,7 @@ class ReflectionUtils { elseif ($key == 'return') { $info['return'] = explode('|', $words[0]); } - elseif ($key == 'options' || $key == 'ui_join_filters') { + elseif ($key == 'options' || $key == 'ui_join_filters' || $key == 'groupWeightsBy') { $val = str_replace(', ', ',', implode(' ', $words)); $info[$key] = explode(',', $val); } diff --git a/tests/phpunit/api/v4/Action/BasicCustomFieldTest.php b/tests/phpunit/api/v4/Action/BasicCustomFieldTest.php index ba6ea2a502..c070b86fe7 100644 --- a/tests/phpunit/api/v4/Action/BasicCustomFieldTest.php +++ b/tests/phpunit/api/v4/Action/BasicCustomFieldTest.php @@ -423,9 +423,9 @@ class BasicCustomFieldTest extends BaseCustomValueTest { ->addValue('extends', 'Individual') ->execute()->first(); $sampleData = [ - ['label' => 'One'], + ['label' => 'One', 'html_type' => 'Select', 'option_values' => ['a' => 'A', 'b' => 'B']], ['label' => 'Two'], - ['label' => 'Three'], + ['label' => 'Three', 'html_type' => 'Select', 'option_values' => ['c' => 'C', 'd' => 'D']], ['label' => 'Four'], ]; CustomField::save(FALSE) -- 2.25.1