APIv4 Export - Fix logic for exporting pseudoconstant syntax

civicrm · Dec 1, 2021 · a1e683f · a1e683f
1 parent ea21814
commit a1e683f
Show file tree

Hide file tree

Showing 2 changed files with 54 additions and 49 deletions.
diff --git a/Civi/Api4/Generic/ExportAction.php b/Civi/Api4/Generic/ExportAction.php
@@ -85,7 +85,7 @@ private function exportRecord(string $entityType, int $entityId, Result $result,
         $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,70 +141,71 @@ private function exportRecord(string $entityType, int $entityId, Result $result,
     ];
     // 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.
+   * to use pseudoconstant (field:name) syntax vs plain value.
    *
-   * 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,
+   * Generally speaking, options generated by a callback function 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.
+   * the :name property may not be reliable.
    *
    * @param array $field
    * @return bool
    */
-  private function shouldUsePseudoconstant(array $field) {
+  private function shouldUsePseudoconstant(string $entityType, array $field) {
     if (empty($field['options'])) {
       return FALSE;
     }
-    $numericKeys = array_filter(array_keys($field['options']), 'is_numeric');
-    return count($numericKeys) === count($field['options']);
+    $daoName = CoreUtil::getInfoItem($entityType, 'dao');
+    if ($daoName) {
+      return empty($daoName::getSupportedFields()[$field['name']]['pseudoconstant']['callback']);
+    }
+    return TRUE;
   }
 
   /**

diff --git a/tests/phpunit/api/v4/Entity/ManagedEntityTest.php b/tests/phpunit/api/v4/Entity/ManagedEntityTest.php
@@ -410,7 +410,7 @@ public function testManagedNavigationWeights() {
             'permission_operator' => '',
             'parent_id.name' => 'Test_Parent',
             'is_active' => TRUE,
-            'has_separator' => NULL,
+            'has_separator' => 1,
             'domain_id' => 'current_domain',
           ],
         ],
@@ -432,7 +432,7 @@ public function testManagedNavigationWeights() {
             'permission_operator' => '',
             'parent_id.name' => 'Test_Parent',
             'is_active' => TRUE,
-            'has_separator' => NULL,
+            'has_separator' => 2,
             'domain_id' => 'current_domain',
           ],
         ],
@@ -491,6 +491,10 @@ public function testManagedNavigationWeights() {
     $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']);