From 8497e9027483a626418415d1c713710c6e9faa21 Mon Sep 17 00:00:00 2001 From: Coleman Watts Date: Mon, 18 Oct 2021 08:45:26 -0400 Subject: [PATCH] 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. --- CRM/Contact/BAO/ContactType.php | 34 ++++-------- .../BAO/ContactType/ContactTypeTest.php | 52 +++++++++---------- 2 files changed, 35 insertions(+), 51 deletions(-) diff --git a/CRM/Contact/BAO/ContactType.php b/CRM/Contact/BAO/ContactType.php index a495d7e398..3b7cc38870 100644 --- 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 9adee0c3b4..b4f15a4589 100644 --- 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' => '', ], ]; } -- 2.25.1