Cleanup ContactType BAO fixme and comments; use common function

This fixes the fixme in basicTypeInfo to use getAllContactTypes instead of running its
own query, and cleans up some comments.

diff --git a/CRM/Contact/BAO/ContactType.php b/CRM/Contact/BAO/ContactType.php
index a495d7e3985d7153e6d5e4bfcf795212f646d3a9..3b7cc388701b96ca676eafe4a5930ffbdb657889 100644 (file)
--- a/CRM/Contact/BAO/ContactType.php
+++ b/CRM/Contact/BAO/ContactType.php
@@ -54,8 +54,6 @@ class CRM_Contact_BAO_ContactType extends CRM_Contact_DAO_ContactType {
   /**
    * Retrieve basic contact type information.
    *
-   * @todo - call getAllContactTypes & return filtered results.
-   *
    * @param bool $includeInactive
    *
    * @return array
@@ -65,19 +63,9 @@ class CRM_Contact_BAO_ContactType extends CRM_Contact_DAO_ContactType {
    * @throws \Civi\API\Exception\UnauthorizedException
    */
   public static function basicTypeInfo($includeInactive = FALSE) {
-    $cacheKey = 'CRM_CT_BTI_' . (int) $includeInactive;
-    $contactTypes = Civi::cache('contactTypes')->get($cacheKey, []);
-    if (!$contactTypes) {
-      $query = CRM_Utils_SQL_Select::from('civicrm_contact_type')
-        ->where('parent_id IS NULL');
-      if ($includeInactive === FALSE) {
-        $query->where('is_active = 1');
-      }
-      $dao = CRM_Core_DAO::executeQuery($query->toSQL());
-      $contactTypes = array_column($dao->fetchAll(), NULL, 'name');
-      Civi::cache('contactTypes')->set($cacheKey, $contactTypes);
-    }
-    return $contactTypes;
+    return array_filter(self::getAllContactTypes(), function($type) use ($includeInactive) {
+      return empty($type['parent']) && ($includeInactive || $type['is_active']);
+    });
   }
 
   /**
@@ -185,8 +173,7 @@ class CRM_Contact_BAO_ContactType extends CRM_Contact_DAO_ContactType {
   }
 
   /**
-   *
-   * retrieve list of all types i.e basic + subtypes.
+   * Retrieve list of all types i.e basic + subtypes.
    *
    * @param bool $all
    *
@@ -208,7 +195,6 @@ class CRM_Contact_BAO_ContactType extends CRM_Contact_DAO_ContactType {
    *
    * @return array
    *   Array of basic types + all subtypes.
-   * @throws \API_Exception
    */
   public static function contactTypeInfo($all = FALSE) {
     $contactTypes = self::getAllContactTypes();
@@ -229,7 +215,7 @@ class CRM_Contact_BAO_ContactType extends CRM_Contact_DAO_ContactType {
    * @param null $typeName
    * @param null $delimiter
    *
-   * @return array
+   * @return array|string
    *   Array of basictypes with name as 'built-in name' and 'label' as value
    * @throws \API_Exception
    */
@@ -362,7 +348,6 @@ AND   ( p.is_active = 1 OR p.id IS NULL )
    *
    * @param array|string $subType contact subType.
    * @return array|string
-   *   basicTypes.
    */
   public static function getBasicType($subType) {
     // @todo - use Cache class - ie like Civi::cache('contactTypes')
@@ -703,7 +688,6 @@ WHERE name = %1';
    * @return bool
    */
   public static function hasRelationships($contactId, $contactType) {
-    $subTypeClause = NULL;
     if (self::isaSubType($contactType)) {
       $subType = $contactType;
       $contactType = self::getBasicType($subType);
@@ -860,8 +844,10 @@ WHERE ($subtypeClause)";
   /**
    * Get all contact types, leveraging caching.
    *
-   * @return array
+   * Note, this function is used within APIv4 Entity.get, so must use a
+   * SQL query instead of calling APIv4 to avoid an infinite loop.
    *
+   * @return array
    * @throws \API_Exception
    */
   protected static function getAllContactTypes() {
@@ -877,13 +863,11 @@ WHERE ($subtypeClause)";
       foreach ($contactTypes as $id => $contactType) {
         $contactTypes[$id]['parent'] = $contactType['parent_id'] ? $name_options[$contactType['parent_id']] : NULL;
         $contactTypes[$id]['parent_label'] = $contactType['parent_id'] ? $label_options[$contactType['parent_id']] : NULL;
-        // Fix types.
+        // Cast int/bool types.
         $contactTypes[$id]['id'] = (int) $contactType['id'];
         $contactTypes[$id]['parent_id'] = $contactType['parent_id'] ? (int) $contactType['parent_id'] : NULL;
         $contactTypes[$id]['is_active'] = (bool) $contactType['is_active'];
         $contactTypes[$id]['is_reserved'] = (bool) $contactType['is_reserved'];
-        $contactTypes[$id]['description'] = $contactType['description'] ?: NULL;
-        $contactTypes[$id]['image_URL'] = $contactType['image_URL'] ?: NULL;
       }
       $cache->set($cacheKey, $contactTypes);
     }

diff --git a/tests/phpunit/CRM/Contact/BAO/ContactType/ContactTypeTest.php b/tests/phpunit/CRM/Contact/BAO/ContactType/ContactTypeTest.php
index 9adee0c3b4320f59d44d4d49ab11f423145f8839..b4f15a45890ea2550e02cdacc371fd1c3d0e0f47 100644 (file)
--- a/tests/phpunit/CRM/Contact/BAO/ContactType/ContactTypeTest.php
+++ b/tests/phpunit/CRM/Contact/BAO/ContactType/ContactTypeTest.php
@@ -121,8 +121,8 @@ class CRM_Contact_BAO_ContactType_ContactTypeTest extends CiviUnitTestCase {
       'is_reserved' => FALSE,
       'parent' => 'Individual',
       'parent_label' => 'Individual',
-      'description' => NULL,
-      'image_URL' => NULL,
+      'description' => '',
+      'image_URL' => '',
     ];
     $this->assertEquals($expected, $allTypes);
   }
@@ -141,11 +141,11 @@ class CRM_Contact_BAO_ContactType_ContactTypeTest extends CiviUnitTestCase {
           'label' => 'Individual',
           'is_active' => TRUE,
           'is_reserved' => TRUE,
-          'description' => NULL,
+          'description' => '',
           'parent_id' => NULL,
           'parent' => NULL,
           'parent_label' => NULL,
-          'image_URL' => NULL,
+          'image_URL' => '',
         ],
       'Household' =>
         [
@@ -154,11 +154,11 @@ class CRM_Contact_BAO_ContactType_ContactTypeTest extends CiviUnitTestCase {
           'label' => 'Household',
           'is_active' => TRUE,
           'is_reserved' => TRUE,
-          'description' => NULL,
+          'description' => '',
           'parent_id' => NULL,
           'parent' => NULL,
           'parent_label' => NULL,
-          'image_URL' => NULL,
+          'image_URL' => '',
         ],
       'Organization' =>
         [
@@ -167,11 +167,11 @@ class CRM_Contact_BAO_ContactType_ContactTypeTest extends CiviUnitTestCase {
           'label' => 'Organization',
           'is_active' => TRUE,
           'is_reserved' => TRUE,
-          'description' => NULL,
+          'description' => '',
           'parent_id' => NULL,
           'parent' => NULL,
           'parent_label' => NULL,
-          'image_URL' => NULL,
+          'image_URL' => '',
         ],
       'Student' =>
         [
@@ -181,10 +181,10 @@ class CRM_Contact_BAO_ContactType_ContactTypeTest extends CiviUnitTestCase {
           'parent_id' => 1,
           'is_active' => '1',
           'is_reserved' => FALSE,
-          'description' => NULL,
+          'description' => '',
           'parent' => 'Individual',
           'parent_label' => 'Individual',
-          'image_URL' => NULL,
+          'image_URL' => '',
         ],
       'Parent' =>
         [
@@ -194,10 +194,10 @@ class CRM_Contact_BAO_ContactType_ContactTypeTest extends CiviUnitTestCase {
           'parent_id' => 1,
           'is_active' => TRUE,
           'is_reserved' => FALSE,
-          'description' => NULL,
+          'description' => '',
           'parent' => 'Individual',
           'parent_label' => 'Individual',
-          'image_URL' => NULL,
+          'image_URL' => '',
         ],
       'Staff' =>
         [
@@ -207,10 +207,10 @@ class CRM_Contact_BAO_ContactType_ContactTypeTest extends CiviUnitTestCase {
           'parent_id' => 1,
           'is_active' => TRUE,
           'is_reserved' => FALSE,
-          'description' => NULL,
+          'description' => '',
           'parent' => 'Individual',
           'parent_label' => 'Individual',
-          'image_URL' => NULL,
+          'image_URL' => '',
         ],
       'Team' =>
         [
@@ -220,10 +220,10 @@ class CRM_Contact_BAO_ContactType_ContactTypeTest extends CiviUnitTestCase {
           'parent_id' => 3,
           'is_active' => TRUE,
           'is_reserved' => FALSE,
-          'description' => NULL,
+          'description' => '',
           'parent' => 'Organization',
           'parent_label' => 'Organization',
-          'image_URL' => NULL,
+          'image_URL' => '',
         ],
       'Sponsor' =>
         [
@@ -233,10 +233,10 @@ class CRM_Contact_BAO_ContactType_ContactTypeTest extends CiviUnitTestCase {
           'parent_id' => 3,
           'is_active' => TRUE,
           'is_reserved' => FALSE,
-          'description' => NULL,
+          'description' => '',
           'parent' => 'Organization',
           'parent_label' => 'Organization',
-          'image_URL' => NULL,
+          'image_URL' => '',
         ],
       'sub1_individual' =>
         [
@@ -246,10 +246,10 @@ class CRM_Contact_BAO_ContactType_ContactTypeTest extends CiviUnitTestCase {
           'parent_id' => 1,
           'is_active' => TRUE,
           'is_reserved' => FALSE,
-          'description' => NULL,
+          'description' => '',
           'parent' => 'Individual',
           'parent_label' => 'Individual',
-          'image_URL' => NULL,
+          'image_URL' => '',
         ],
       'sub2_individual' =>
         [
@@ -259,10 +259,10 @@ class CRM_Contact_BAO_ContactType_ContactTypeTest extends CiviUnitTestCase {
           'parent_id' => 1,
           'is_active' => TRUE,
           'is_reserved' => FALSE,
-          'description' => NULL,
+          'description' => '',
           'parent' => 'Individual',
           'parent_label' => 'Individual',
-          'image_URL' => NULL,
+          'image_URL' => '',
         ],
       'sub_organization' =>
         [
@@ -272,10 +272,10 @@ class CRM_Contact_BAO_ContactType_ContactTypeTest extends CiviUnitTestCase {
           'parent_id' => 3,
           'is_active' => TRUE,
           'is_reserved' => FALSE,
-          'description' => NULL,
+          'description' => '',
           'parent' => 'Organization',
           'parent_label' => 'Organization',
-          'image_URL' => NULL,
+          'image_URL' => '',
         ],
       'sub_household' =>
         [
@@ -285,10 +285,10 @@ class CRM_Contact_BAO_ContactType_ContactTypeTest extends CiviUnitTestCase {
           'parent_id' => 2,
           'is_active' => TRUE,
           'is_reserved' => FALSE,
-          'description' => NULL,
+          'description' => '',
           'parent' => 'Household',
           'parent_label' => 'Household',
-          'image_URL' => NULL,
+          'image_URL' => '',
         ],
     ];
   }