APIv4 SortableEntity - Fix sorting custom fields with option groups
authorColeman Watts <coleman@civicrm.org>
Sun, 6 Feb 2022 20:01:20 +0000 (15:01 -0500)
committerColeman Watts <coleman@civicrm.org>
Mon, 7 Feb 2022 06:30:51 +0000 (01:30 -0500)
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
Civi/Api4/Entity.php
Civi/Api4/Generic/Traits/DAOActionTrait.php
Civi/Api4/MembershipType.php
Civi/Api4/Navigation.php
Civi/Api4/OptionValue.php
Civi/Api4/PriceField.php
Civi/Api4/PriceFieldValue.php
Civi/Api4/UFField.php
Civi/Api4/Utils/ReflectionUtils.php
tests/phpunit/api/v4/Action/BasicCustomFieldTest.php

index 5751e6492e8e902bd9991216b3e781f13e687065..413d268d40cfb1c1699ef9c8f54b309ee5e2d94d 100644 (file)
@@ -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
  */
index 69f5363b455866d16ee717f24b9f5929ae087313..fbe5d75f67cd5a9afd103062eb1268c00c6a7e73 100644 (file)
@@ -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);
   }
index b44c7d1fda540ff65465a372d16174d59ecc0f9d..c7661f70751fe7300acbeb0c0c5c2f9d547f33f0 100644 (file)
@@ -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
index ba4c6f80d722b75d97d84ff1b0b78b2d129ddec4..37a07191b6a59283278c5f3e2ca7e419d67ed77e 100644 (file)
@@ -15,6 +15,7 @@ namespace Civi\Api4;
  *
  * @searchable secondary
  * @orderBy weight
+ * @groupWeightsBy domain_id
  * @since 5.27
  * @package Civi\Api4
  */
index ecf14ffe4165b23698655f48572603c5945a1f21..fc1d08e56d5e7530cda9a9b784c53183d70c96c7 100644 (file)
@@ -15,6 +15,7 @@ namespace Civi\Api4;
  *
  * @searchable none
  * @orderBy weight
+ * @groupWeightsBy domain_id,parent_id
  * @since 5.19
  * @package Civi\Api4
  */
index 7ca6e314811ab725552bf3f7e94eea93d496f468..a3486c3e7c766bcd44a79a36e5c19b14db5864a8 100644 (file)
@@ -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
  */
index dcbe67026ef30efc36094b2701aa688a5fe6eb87..8b1aba7ecf237a919b225052f56d277e9a23670d 100644 (file)
@@ -15,6 +15,7 @@ namespace Civi\Api4;
  *
  * @searchable secondary
  * @orderBy weight
+ * @groupWeightsBy price_set_id
  * @since 5.27
  * @package Civi\Api4
  */
index d6e20203efdebd83930d9ca618f3572a85797983..59249021686f806049d907d969e170d01e0f517f 100644 (file)
@@ -15,6 +15,7 @@ namespace Civi\Api4;
  *
  * @searchable secondary
  * @orderBy weight
+ * @groupWeightsBy price_field_id
  * @since 5.27
  * @package Civi\Api4
  */
index 7fbd05df2f6d26685f1a8ea39c50a36d909aab16..01479ada1eb2b1e04b8a6af574ae866fede20b39 100644 (file)
@@ -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
  */
index 0263718e3aa44b82a5bd78cbcc3e373536910bfb..f09d8841d96259bc855798659e1eb123c6010248 100644 (file)
@@ -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);
         }
index ba6ea2a502443a42cdadb97c34766b164d8b8573..c070b86fe74385d3b8ee68a796d01f66bde873f6 100644 (file)
@@ -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)