From 21ecd39c6929566e46ff3bded01485135db588a9 Mon Sep 17 00:00:00 2001 From: Coleman Watts Date: Wed, 1 Dec 2021 16:44:44 -0500 Subject: [PATCH] APIv4 Export - Fix logic for exporting pseudoconstant syntax --- Civi/Api4/Generic/ExportAction.php | 98 ++++++++++--------- .../api/v4/Entity/ManagedEntityTest.php | 8 +- 2 files changed, 57 insertions(+), 49 deletions(-) diff --git a/Civi/Api4/Generic/ExportAction.php b/Civi/Api4/Generic/ExportAction.php index 5c6869ffbf..bb7e14a0c7 100644 --- a/Civi/Api4/Generic/ExportAction.php +++ b/Civi/Api4/Generic/ExportAction.php @@ -85,7 +85,7 @@ class ExportAction extends AbstractAction { $pseudofields[$field['name'] . '.name'] = $field['name']; } // Use pseudoconstant syntax if appropriate - elseif ($this->shouldUsePseudoconstant($field)) { + elseif ($this->shouldUsePseudoconstant($entityType, $field)) { $select[] = $field['name'] . ':name'; $pseudofields[$field['name'] . ':name'] = $field['name']; } @@ -141,68 +141,72 @@ class ExportAction extends AbstractAction { ]; // Export entities that reference this one $daoName = CoreUtil::getInfoItem($entityType, 'dao'); - /** @var \CRM_Core_DAO $dao */ - $dao = new $daoName(); - $dao->id = $entityId; - // Collect references into arrays keyed by entity type - $references = []; - foreach ($dao->findReferences() as $reference) { - $refEntity = \CRM_Utils_Array::first($reference::fields())['entity'] ?? ''; - // Limit references by domain - if (property_exists($reference, 'domain_id')) { - if (!isset($reference->domain_id)) { - $reference->find(TRUE); - } - if (isset($reference->domain_id) && $reference->domain_id != $limitRefsByDomain) { - continue; - } - } - $references[$refEntity][] = $reference; - } - foreach ($references as $refEntity => $records) { - $refApiType = CoreUtil::getInfoItem($refEntity, 'type') ?? []; - // Reference must be a ManagedEntity - if (!in_array('ManagedEntity', $refApiType, TRUE)) { - continue; - } - $exclude = []; - // For sortable entities, order by weight and exclude weight from the export (it will be auto-managed) - if (in_array('SortableEntity', $refApiType, TRUE)) { - $exclude[] = $weightCol = CoreUtil::getInfoItem($refEntity, 'order_by'); - usort($records, function($a, $b) use ($weightCol) { - if (!isset($a->$weightCol)) { - $a->find(TRUE); + if ($daoName) { + /** @var \CRM_Core_DAO $dao */ + $dao = new $daoName(); + $dao->id = $entityId; + // Collect references into arrays keyed by entity type + $references = []; + foreach ($dao->findReferences() as $reference) { + $refEntity = \CRM_Utils_Array::first($reference::fields())['entity'] ?? ''; + // Limit references by domain + if (property_exists($reference, 'domain_id')) { + if (!isset($reference->domain_id)) { + $reference->find(TRUE); } - if (!isset($b->$weightCol)) { - $b->find(TRUE); + if (isset($reference->domain_id) && $reference->domain_id != $limitRefsByDomain) { + continue; } - return $a->$weightCol < $b->$weightCol ? -1 : 1; - }); + } + $references[$refEntity][] = $reference; } - foreach ($records as $record) { - $this->exportRecord($refEntity, $record->id, $result, $name . '_', $exclude); + foreach ($references as $refEntity => $records) { + $refApiType = CoreUtil::getInfoItem($refEntity, 'type') ?? []; + // Reference must be a ManagedEntity + if (!in_array('ManagedEntity', $refApiType, TRUE)) { + continue; + } + $exclude = []; + // For sortable entities, order by weight and exclude weight from the export (it will be auto-managed) + if (in_array('SortableEntity', $refApiType, TRUE)) { + $exclude[] = $weightCol = CoreUtil::getInfoItem($refEntity, 'order_by'); + usort($records, function ($a, $b) use ($weightCol) { + if (!isset($a->$weightCol)) { + $a->find(TRUE); + } + if (!isset($b->$weightCol)) { + $b->find(TRUE); + } + return $a->$weightCol < $b->$weightCol ? -1 : 1; + }); + } + foreach ($records as $record) { + $this->exportRecord($refEntity, $record->id, $result, $name . '_', $exclude); + } } } } /** * If a field has a pseudoconstant list, determine whether it would be better - * to use pseudoconstant (field:name) syntax. - * - * Generally speaking, options with numeric keys are the ones we need to worry about - * because auto-increment keys can vary when migrating an entity to a different database. - * - * But options with string keys tend to be stable, - * and it's better not to use the pseudoconstant syntax with these fields because - * the option list may not be populated at the time of managed entity reconciliation. + * to use pseudoconstant (field:name) syntax vs plain value. * + * @param string $entityType * @param array $field * @return bool */ - private function shouldUsePseudoconstant(array $field) { + private function shouldUsePseudoconstant(string $entityType, array $field) { if (empty($field['options'])) { return FALSE; } + $daoName = CoreUtil::getInfoItem($entityType, 'dao'); + // Options generated by a callback function tend to be stable, + // and the :name property may not be reliable. Use plain value. + if ($daoName && !empty($daoName::getSupportedFields()[$field['name']]['pseudoconstant']['callback'])) { + return FALSE; + } + // Options with numeric keys probably refer to auto-increment keys + // which vary across different databases. Use :name syntax. $numericKeys = array_filter(array_keys($field['options']), 'is_numeric'); return count($numericKeys) === count($field['options']); } diff --git a/tests/phpunit/api/v4/Entity/ManagedEntityTest.php b/tests/phpunit/api/v4/Entity/ManagedEntityTest.php index 79f21e2d87..cdb4fd6df5 100644 --- a/tests/phpunit/api/v4/Entity/ManagedEntityTest.php +++ b/tests/phpunit/api/v4/Entity/ManagedEntityTest.php @@ -410,7 +410,7 @@ class ManagedEntityTest extends UnitTestCase implements TransactionalInterface, 'permission_operator' => '', 'parent_id.name' => 'Test_Parent', 'is_active' => TRUE, - 'has_separator' => NULL, + 'has_separator' => 1, 'domain_id' => 'current_domain', ], ], @@ -432,7 +432,7 @@ class ManagedEntityTest extends UnitTestCase implements TransactionalInterface, 'permission_operator' => '', 'parent_id.name' => 'Test_Parent', 'is_active' => TRUE, - 'has_separator' => NULL, + 'has_separator' => 2, 'domain_id' => 'current_domain', ], ], @@ -491,6 +491,10 @@ class ManagedEntityTest extends UnitTestCase implements TransactionalInterface, $this->assertEquals('Navigation_Test_Parent_Navigation_Test_Child_1', $nav['export'][1]['name']); $this->assertEquals('Navigation_Test_Parent_Navigation_Test_Child_2', $nav['export'][2]['name']); $this->assertEquals('Navigation_Test_Parent_Navigation_Test_Child_3', $nav['export'][3]['name']); + // The has_separator should be using numeric key not pseudoconstant + $this->assertNull($nav['export'][0]['params']['values']['has_separator']); + $this->assertEquals(1, $nav['export'][1]['params']['values']['has_separator']); + $this->assertEquals(2, $nav['export'][2]['params']['values']['has_separator']); // Weight should not be included in export of children, leaving it to be auto-managed $this->assertArrayNotHasKey('weight', $nav['export'][1]['params']['values']); -- 2.25.1