From ccd901a1f676f1c242d0a0058e2a88f9874c1eee Mon Sep 17 00:00:00 2001 From: eileen Date: Sun, 21 Jun 2020 12:08:04 +1200 Subject: [PATCH] [REF] Cleanup function for retrieving contact types. This switches to using the apiv4 as part of trying to eliminate calls to executeQuery where the DAO is passed in like ``` = CRM_Core_DAO::executeQuery(, [], FALSE, 'CRM_Contact_DAO_ContactType' ); ``` Note that this file seems to be the only place this is done. Also note that I added tests first & the changes to the test highlight what is changed in the PR in the output - namely - stricter type casting (courtesy apiv4) - same keys present for all types --- CRM/Contact/BAO/ContactType.php | 93 +++++++------- .../BAO/ContactType/ContactTypeTest.php | 115 ++++++++++++------ 2 files changed, 126 insertions(+), 82 deletions(-) diff --git a/CRM/Contact/BAO/ContactType.php b/CRM/Contact/BAO/ContactType.php index 685ff08f6f..b356a39616 100644 --- a/CRM/Contact/BAO/ContactType.php +++ b/CRM/Contact/BAO/ContactType.php @@ -54,6 +54,8 @@ 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 @@ -111,6 +113,8 @@ class CRM_Contact_BAO_ContactType extends CRM_Contact_DAO_ContactType { /** * Retrieve all subtypes Information. * + * @todo - call getAllContactTypes & return filtered results. + * * @param array $contactType * .. * @param bool $all @@ -217,54 +221,26 @@ WHERE subtype.name IS NOT NULL AND subtype.parent_id IS NOT NULL {$ctWHERE} /** * Retrieve info array about all types i.e basic + subtypes. * + * @todo deprecate calling this with $all = TRUE in favour of getAllContactTypes + * & ideally add getActiveContactTypes & call that from this fully + * deprecated function. + * * @param bool $all * * @return array * Array of basic types + all subtypes. + * @throws \API_Exception */ public static function contactTypeInfo($all = FALSE) { - - if (!isset(Civi::$statics[__CLASS__]['contactTypeInfo'])) { - Civi::$statics[__CLASS__]['contactTypeInfo'] = []; - } - $_cache = &Civi::$statics[__CLASS__]['contactTypeInfo']; - - $argString = $all ? 'CRM_CT_CTI_1' : 'CRM_CT_CTI_0'; - if (!array_key_exists($argString, $_cache)) { - $cache = CRM_Utils_Cache::singleton(); - $_cache[$argString] = $cache->get($argString); - if (!$_cache[$argString]) { - $_cache[$argString] = []; - - $sql = ' -SELECT type.*, parent.name as parent, parent.label as parent_label -FROM civicrm_contact_type type -LEFT JOIN civicrm_contact_type parent ON type.parent_id = parent.id'; - - if ($all === FALSE) { - $sql .= ' WHERE type.is_active = 1'; - } - - $dao = CRM_Core_DAO::executeQuery($sql, - [], - FALSE, - 'CRM_Contact_DAO_ContactType' - ); - while ($dao->fetch()) { - $value = []; - CRM_Core_DAO::storeValues($dao, $value); - if (array_key_exists('parent_id', $value)) { - $value['parent'] = $dao->parent; - $value['parent_label'] = $dao->parent_label; - } - $_cache[$argString][$dao->name] = $value; + $contactTypes = self::getAllContactTypes(); + if (!$all) { + foreach ($contactTypes as $index => $value) { + if (!$value['is_active']) { + unset($contactTypes[$index]); } - - $cache->set($argString, $_cache[$argString]); } } - - return $_cache[$argString]; + return $contactTypes; } /** @@ -276,6 +252,7 @@ LEFT JOIN civicrm_contact_type parent ON type.parent_id = parent.id'; * * @return array * Array of basictypes with name as 'built-in name' and 'label' as value + * @throws \API_Exception */ public static function contactTypePairs($all = FALSE, $typeName = NULL, $delimiter = NULL) { $types = self::contactTypeInfo($all); @@ -325,6 +302,7 @@ LEFT JOIN civicrm_contact_type parent ON type.parent_id = parent.id'; $_cache = []; } + // @todo - call getAllContactTypes & return filtered results. $argString = $all ? 'CRM_CT_GSE_1' : 'CRM_CT_GSE_0'; $argString .= $isSeparator ? '_1' : '_0'; $argString .= $separator; @@ -658,7 +636,8 @@ WHERE name = %1'; /** * @param string $typeName * - * @return mixed + * @return string + * @throws \API_Exception */ public static function getLabel($typeName) { $types = self::contactTypeInfo(TRUE); @@ -804,14 +783,16 @@ WHERE extends = %1 AND ' . implode(" OR ", $subTypeClause); /** * Function that does something. - * @todo what does this function do? * * @param int $contactID - * @param $contactType + * @param string $contactType * @param array $oldSubtypeSet * @param array $newSubtypeSet * * @return bool + * @throws \CRM_Core_Exception + * + * @todo what does this function do? */ public static function deleteCustomSetForSubtypeMigration( $contactID, @@ -841,6 +822,8 @@ WHERE extends = %1 AND ' . implode(" OR ", $subTypeClause); * @param array $subtypesToPreserve * * @return bool + * + * @throws \CRM_Core_Exception */ public static function deleteCustomRowsOfSubtype($gID, $subtypes = [], $subtypesToPreserve = []) { if (!$gID or empty($subtypes)) { @@ -886,6 +869,8 @@ WHERE ($subtypeClause)"; * Entity id. * * @return null|string + * + * @throws \CRM_Core_Exception */ public static function deleteCustomRowsForEntityID($customTable, $entityID) { $customTable = CRM_Utils_Type::escape($customTable, 'String'); @@ -893,4 +878,28 @@ WHERE ($subtypeClause)"; return CRM_Core_DAO::singleValueQuery($query, [1 => [$entityID, 'Integer']]); } + /** + * Get all contact types, leveraging caching. + * + * @return array + * + * @throws \API_Exception + */ + protected static function getAllContactTypes() { + if (!Civi::cache('contactTypes')->has('all')) { + $contactTypes = (array) ContactType::get()->setCheckPermissions(FALSE) + ->setSelect(['id', 'name', 'label', 'description', 'is_active', 'is_reserved', 'image_URL', 'parent_id', 'parent_id:name', 'parent_id:label']) + ->execute()->indexBy('name'); + + foreach ($contactTypes as $id => $contactType) { + $contactTypes[$id]['parent'] = $contactType['parent_id:name']; + $contactTypes[$id]['parent_label'] = $contactType['parent_id:label']; + unset($contactTypes[$id]['parent_id:name'], $contactTypes[$id]['parent_id:label']); + } + Civi::cache('contactTypes')->set('all', $contactTypes); + } + $contactTypes = Civi::cache('contactTypes')->get('all'); + return $contactTypes; + } + } diff --git a/tests/phpunit/CRM/Contact/BAO/ContactType/ContactTypeTest.php b/tests/phpunit/CRM/Contact/BAO/ContactType/ContactTypeTest.php index 6fd24f0289..ca1aab95a6 100644 --- a/tests/phpunit/CRM/Contact/BAO/ContactType/ContactTypeTest.php +++ b/tests/phpunit/CRM/Contact/BAO/ContactType/ContactTypeTest.php @@ -42,7 +42,7 @@ class CRM_Contact_BAO_ContactType_ContactTypeTest extends CiviUnitTestCase { 'parent_id:name' => 'Household', 'is_active' => 1, ]; - $this->ids['ContactType'][] = ContactType::create()->setValues($params)->execute()->first()['id']; + $this->ids['ContactType'][] = (int) ContactType::create()->setValues($params)->execute()->first()['id']; } /** @@ -107,14 +107,16 @@ class CRM_Contact_BAO_ContactType_ContactTypeTest extends CiviUnitTestCase { $this->assertEquals($expected, $activeTypes); $allTypes = CRM_Contact_BAO_ContactType::contactTypeInfo(TRUE); $expected['blah'] = [ - 'is_active' => 0, + 'is_active' => FALSE, 'name' => 'blah', 'label' => 'blah blah', 'id' => $createdType['id'], 'parent_id' => 1, - 'is_reserved' => 0, + 'is_reserved' => FALSE, 'parent' => 'Individual', 'parent_label' => 'Individual', + 'description' => NULL, + 'image_URL' => NULL, ]; $this->assertEquals($expected, $allTypes); } @@ -131,123 +133,156 @@ class CRM_Contact_BAO_ContactType_ContactTypeTest extends CiviUnitTestCase { 'id' => '1', 'name' => 'Individual', 'label' => 'Individual', - 'is_active' => '1', - 'is_reserved' => '1', + 'is_active' => TRUE, + 'is_reserved' => TRUE, + 'description' => NULL, + 'parent_id' => NULL, + 'parent' => NULL, + 'parent_label' => NULL, + 'image_URL' => NULL, ], 'Household' => [ 'id' => '2', 'name' => 'Household', 'label' => 'Household', - 'is_active' => '1', - 'is_reserved' => '1', + 'is_active' => TRUE, + 'is_reserved' => TRUE, + 'description' => NULL, + 'parent_id' => NULL, + 'parent' => NULL, + 'parent_label' => NULL, + 'image_URL' => NULL, ], 'Organization' => [ 'id' => '3', 'name' => 'Organization', 'label' => 'Organization', - 'is_active' => '1', - 'is_reserved' => '1', + 'is_active' => TRUE, + 'is_reserved' => TRUE, + 'description' => NULL, + 'parent_id' => NULL, + 'parent' => NULL, + 'parent_label' => NULL, + 'image_URL' => NULL, ], 'Student' => [ - 'id' => '4', + 'id' => 4, 'name' => 'Student', 'label' => 'Student', - 'parent_id' => '1', + 'parent_id' => 1, 'is_active' => '1', - 'is_reserved' => '0', + 'is_reserved' => FALSE, + 'description' => NULL, 'parent' => 'Individual', 'parent_label' => 'Individual', + 'image_URL' => NULL, ], 'Parent' => [ - 'id' => '5', + 'id' => 5, 'name' => 'Parent', 'label' => 'Parent', - 'parent_id' => '1', - 'is_active' => '1', - 'is_reserved' => '0', + 'parent_id' => 1, + 'is_active' => TRUE, + 'is_reserved' => FALSE, + 'description' => NULL, 'parent' => 'Individual', 'parent_label' => 'Individual', + 'image_URL' => NULL, ], 'Staff' => [ - 'id' => '6', + 'id' => 6, 'name' => 'Staff', 'label' => 'Staff', - 'parent_id' => '1', - 'is_active' => '1', - 'is_reserved' => '0', + 'parent_id' => 1, + 'is_active' => TRUE, + 'is_reserved' => FALSE, + 'description' => NULL, 'parent' => 'Individual', 'parent_label' => 'Individual', + 'image_URL' => NULL, ], 'Team' => [ - 'id' => '7', + 'id' => 7, 'name' => 'Team', 'label' => 'Team', - 'parent_id' => '3', - 'is_active' => '1', - 'is_reserved' => '0', + 'parent_id' => 3, + 'is_active' => TRUE, + 'is_reserved' => FALSE, + 'description' => NULL, 'parent' => 'Organization', 'parent_label' => 'Organization', + 'image_URL' => NULL, ], 'Sponsor' => [ - 'id' => '8', + 'id' => 8, 'name' => 'Sponsor', 'label' => 'Sponsor', - 'parent_id' => '3', - 'is_active' => '1', - 'is_reserved' => '0', + 'parent_id' => 3, + 'is_active' => TRUE, + 'is_reserved' => FALSE, + 'description' => NULL, 'parent' => 'Organization', 'parent_label' => 'Organization', + 'image_URL' => NULL, ], 'sub1_individual' => [ 'id' => $this->ids['ContactType'][0], 'name' => 'sub1_individual', 'label' => 'sub1_individual', - 'parent_id' => '1', - 'is_active' => '1', - 'is_reserved' => '0', + 'parent_id' => 1, + 'is_active' => TRUE, + 'is_reserved' => FALSE, + 'description' => NULL, 'parent' => 'Individual', 'parent_label' => 'Individual', + 'image_URL' => NULL, ], 'sub2_individual' => [ 'id' => $this->ids['ContactType'][1], 'name' => 'sub2_individual', 'label' => 'sub2_individual', - 'parent_id' => '1', - 'is_active' => '1', - 'is_reserved' => '0', + 'parent_id' => 1, + 'is_active' => TRUE, + 'is_reserved' => FALSE, + 'description' => NULL, 'parent' => 'Individual', 'parent_label' => 'Individual', + 'image_URL' => NULL, ], 'sub_organization' => [ 'id' => $this->ids['ContactType'][2], 'name' => 'sub_organization', 'label' => 'sub_organization', - 'parent_id' => '3', - 'is_active' => '1', - 'is_reserved' => '0', + 'parent_id' => 3, + 'is_active' => TRUE, + 'is_reserved' => FALSE, + 'description' => NULL, 'parent' => 'Organization', 'parent_label' => 'Organization', + 'image_URL' => NULL, ], 'sub_household' => [ 'id' => $this->ids['ContactType'][3], 'name' => 'sub_household', 'label' => 'sub_household', - 'parent_id' => '2', - 'is_active' => '1', - 'is_reserved' => '0', + 'parent_id' => 2, + 'is_active' => TRUE, + 'is_reserved' => FALSE, + 'description' => NULL, 'parent' => 'Household', 'parent_label' => 'Household', + 'image_URL' => NULL, ], ]; } -- 2.25.1