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.
civicrm · Feb 7, 2022 · 204c3c2 · 204c3c2
1 parent 6c43540
commit 204c3c2
Show file tree

Hide file tree

Showing 11 changed files with 17 additions and 6 deletions.
diff --git a/Civi/Api4/CustomField.php b/Civi/Api4/CustomField.php
@@ -16,6 +16,7 @@
  * @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
@@ -132,6 +132,11 @@ public static function getFields($checkPermissions = TRUE) {
           '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
@@ -330,6 +330,7 @@ protected function updateWeight(array &$record) {
     /** @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 @@ protected function updateWeight(array &$record) {
     $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
@@ -15,6 +15,7 @@
  *
  * @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
@@ -15,6 +15,7 @@
  *
  * @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
@@ -16,6 +16,7 @@
  * @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
@@ -15,6 +15,7 @@
  *
  * @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
@@ -15,6 +15,7 @@
  *
  * @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
@@ -16,6 +16,7 @@
  * @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
@@ -88,7 +88,7 @@ public static function parseDocBlock($comment) {
         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
@@ -423,9 +423,9 @@ public function testUpdateWeights() {
         ->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)