From eb8dbd47ba5af22d839cc6663db46f78a997f95b Mon Sep 17 00:00:00 2001 From: eileen Date: Sat, 20 Jun 2020 12:45:16 +1200 Subject: [PATCH] Fix master regression on inactive contact types Removing the WHERE type.name = NULL here makes sense https://github.com/civicrm/civicrm-core/commit/351e8d470a95e1a610280731706c816033fb79fa#diff-ae3cfb8afd3fe1ab8f471b3a069563cdL244 but a few lines later an extra condition is added - we need the WHERE there to prevent it being added to the join clause. Note that I did something a bit yuck with the static. I want to properly re-write the function with consistent caching but after it's under testing, hence a quick fix --- CRM/Contact/BAO/ContactType.php | 15 +- .../BAO/ContactType/ContactTypeTest.php | 301 +++++++++++++----- 2 files changed, 235 insertions(+), 81 deletions(-) diff --git a/CRM/Contact/BAO/ContactType.php b/CRM/Contact/BAO/ContactType.php index 27d594c4a4..685ff08f6f 100644 --- a/CRM/Contact/BAO/ContactType.php +++ b/CRM/Contact/BAO/ContactType.php @@ -26,7 +26,7 @@ class CRM_Contact_BAO_ContactType extends CRM_Contact_DAO_ContactType { * @param array $defaults * (reference ) an assoc array to hold the flattened values. * - * @return CRM_Contact_BAO_ContactType|null + * @return CRM_Contact_DAO_ContactType|null * object on success, null otherwise */ public static function retrieve(&$params, &$defaults) { @@ -48,8 +48,7 @@ class CRM_Contact_BAO_ContactType extends CRM_Contact_DAO_ContactType { */ public static function isActive($contactType) { $contact = self::contactTypeInfo(FALSE); - $active = array_key_exists($contactType, $contact); - return $active; + return array_key_exists($contactType, $contact); } /** @@ -224,11 +223,11 @@ WHERE subtype.name IS NOT NULL AND subtype.parent_id IS NOT NULL {$ctWHERE} * Array of basic types + all subtypes. */ public static function contactTypeInfo($all = FALSE) { - static $_cache = NULL; - if ($_cache === NULL) { - $_cache = []; + 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)) { @@ -243,7 +242,7 @@ FROM civicrm_contact_type type LEFT JOIN civicrm_contact_type parent ON type.parent_id = parent.id'; if ($all === FALSE) { - $sql .= ' AND type.is_active = 1'; + $sql .= ' WHERE type.is_active = 1'; } $dao = CRM_Core_DAO::executeQuery($sql, @@ -576,7 +575,7 @@ WHERE name = %1'; * @return object|void * @throws \CRM_Core_Exception */ - public static function add(&$params) { + public static function add($params) { // label or name if (empty($params['id']) && empty($params['label'])) { diff --git a/tests/phpunit/CRM/Contact/BAO/ContactType/ContactTypeTest.php b/tests/phpunit/CRM/Contact/BAO/ContactType/ContactTypeTest.php index b6af199990..6fd24f0289 100644 --- a/tests/phpunit/CRM/Contact/BAO/ContactType/ContactTypeTest.php +++ b/tests/phpunit/CRM/Contact/BAO/ContactType/ContactTypeTest.php @@ -1,4 +1,5 @@ $labelsub1, - 'name' => $labelsub1, - // Individual - 'parent_id' => 1, + 'label' => 'sub1_individual', + 'name' => 'sub1_individual', + 'parent_id:name' => 'Individual', 'is_active' => 1, ]; - $result = CRM_Contact_BAO_ContactType::add($params); - $this->subTypesIndividual[] = $params['name']; - $labelsub2 = 'sub2_individual' . substr(sha1(rand()), 0, 7); + $this->ids['ContactType'][] = ContactType::create()->setValues($params)->execute()->first()['id']; + $params = [ - 'label' => $labelsub2, - 'name' => $labelsub2, - // Individual - 'parent_id' => 1, + 'label' => 'sub2_individual', + 'name' => 'sub2_individual', + 'parent_id:name' => 'Individual', 'is_active' => 1, ]; - $result = CRM_Contact_BAO_ContactType::add($params); - $this->subTypesIndividual[] = $params['name']; - $labelsub = 'sub_organization' . substr(sha1(rand()), 0, 7); + $this->ids['ContactType'][] = ContactType::create()->setValues($params)->execute()->first()['id']; + $params = [ - 'label' => $labelsub, - 'name' => $labelsub, - // Organization - 'parent_id' => 3, + 'label' => 'sub_organization', + 'name' => 'sub_organization', + 'parent_id:name' => 'Organization', 'is_active' => 1, ]; - $result = CRM_Contact_BAO_ContactType::add($params); - $this->subTypesOrganization[] = $params['name']; - $labelhousehold = 'sub_household' . substr(sha1(rand()), 0, 7); + $this->ids['ContactType'][] = ContactType::create()->setValues($params)->execute()->first()['id']; + $params = [ - 'label' => $labelhousehold, - 'name' => $labelhousehold, - // Household - 'parent_id' => 2, + 'label' => 'sub_household', + 'name' => 'sub_household', + 'parent_id:name' => 'Household', 'is_active' => 1, ]; - $result = CRM_Contact_BAO_ContactType::add($params); - $this->subTypesHousehold[] = $params['name']; + $this->ids['ContactType'][] = ContactType::create()->setValues($params)->execute()->first()['id']; } /** - * Test contactTypes() and subTypes() methods with valid data - * success expected + * Cleanup contact types. + * + * @throws \API_Exception + * @throws \Civi\API\Exception\UnauthorizedException */ - public function testGetMethods() { + public function tearDown() { + parent::tearDown(); + ContactType::delete()->addWhere('id', 'IN', $this->ids['ContactType'])->execute(); + } - // check all contact types - $contactTypes = ['Individual', 'Organization', 'Household']; - $result = CRM_Contact_BAO_ContactType::contactTypes('Individual'); - foreach ($contactTypes as $type) { - $this->assertEquals(in_array($type, $result), TRUE); - } + /** + * Test contactTypes() and subTypes() methods return correct contact types. + */ + public function testGetMethods() { + $result = CRM_Contact_BAO_ContactType::contactTypes(TRUE); + $this->assertEquals(array_keys($this->getExpectedContactTypes()), $result); // check for type:Individual $result = CRM_Contact_BAO_ContactType::subTypes('Individual'); - foreach ($result as $subtype) { - $subTypeName = in_array($subtype, $this->subTypesIndividual); - if (!empty($subTypeName)) { - $this->assertEquals($subTypeName, TRUE); - } - $this->assertEquals(in_array($subtype, $this->subTypesOrganization), FALSE); - $this->assertEquals(in_array($subtype, $this->subTypesHousehold), FALSE); - } + $this->assertEquals(array_keys($this->getExpectedContactSubTypes('Individual')), $result); // check for type:Organization $result = CRM_Contact_BAO_ContactType::subTypes('Organization'); - foreach ($result as $subtype) { - $this->assertEquals(in_array($subtype, $this->subTypesIndividual), FALSE); - $subTypeName = in_array($subtype, $this->subTypesOrganization); - if (!empty($subTypeName)) { - $this->assertEquals($subTypeName, TRUE); - } - $subTypeName = in_array($subTypeName, $this->subTypesHousehold); - if (empty($subTypeName)) { - $this->assertEquals($subTypeName, FALSE); - } - } + $this->assertEquals(array_keys($this->getExpectedContactSubTypes('Organization')), $result); // check for type:Household $result = CRM_Contact_BAO_ContactType::subTypes('Household'); - foreach ($result as $subtype) { - $this->assertEquals(in_array($subtype, $this->subTypesIndividual), FALSE); - $this->assertEquals(in_array($subtype, $this->subTypesOrganization), FALSE); - $this->assertEquals(in_array($subtype, $this->subTypesHousehold), TRUE); - } + $this->assertEquals(array_keys($this->getExpectedContactSubTypes('Household')), $result); - // check for all conatct types + // check for all contact types $result = CRM_Contact_BAO_ContactType::subTypes(); - foreach ($this->subTypesIndividual as $subtype) { - $this->assertEquals(in_array($subtype, $result), TRUE); - } - foreach ($this->subTypesOrganization as $subtype) { - $this->assertEquals(in_array($subtype, $result), TRUE); - } - foreach ($this->subTypesHousehold as $subtype) { - $this->assertEquals(in_array($subtype, $result), TRUE); - } + $this->assertEquals(array_keys($this->getExpectedAllSubtypes()), $result); } /** @@ -126,6 +94,194 @@ class CRM_Contact_BAO_ContactType_ContactTypeTest extends CiviUnitTestCase { $this->assertEquals(empty($result), TRUE); } + /** + * Test function for getting contact types. + * + * @throws \API_Exception + */ + public function testContactTypeInfo() { + $blahType = ['is_active' => 0, 'name' => 'blah', 'label' => 'blah blah', 'parent_id:name' => 'Individual']; + $createdType = ContactType::create()->setValues($blahType)->execute()->first(); + $activeTypes = CRM_Contact_BAO_ContactType::contactTypeInfo(); + $expected = $this->getExpectedContactTypes(); + $this->assertEquals($expected, $activeTypes); + $allTypes = CRM_Contact_BAO_ContactType::contactTypeInfo(TRUE); + $expected['blah'] = [ + 'is_active' => 0, + 'name' => 'blah', + 'label' => 'blah blah', + 'id' => $createdType['id'], + 'parent_id' => 1, + 'is_reserved' => 0, + 'parent' => 'Individual', + 'parent_label' => 'Individual', + ]; + $this->assertEquals($expected, $allTypes); + } + + /** + * Get all expected types. + * + * @return array + */ + public function getExpectedContactTypes() { + return [ + 'Individual' => + [ + 'id' => '1', + 'name' => 'Individual', + 'label' => 'Individual', + 'is_active' => '1', + 'is_reserved' => '1', + ], + 'Household' => + [ + 'id' => '2', + 'name' => 'Household', + 'label' => 'Household', + 'is_active' => '1', + 'is_reserved' => '1', + ], + 'Organization' => + [ + 'id' => '3', + 'name' => 'Organization', + 'label' => 'Organization', + 'is_active' => '1', + 'is_reserved' => '1', + ], + 'Student' => + [ + 'id' => '4', + 'name' => 'Student', + 'label' => 'Student', + 'parent_id' => '1', + 'is_active' => '1', + 'is_reserved' => '0', + 'parent' => 'Individual', + 'parent_label' => 'Individual', + ], + 'Parent' => + [ + 'id' => '5', + 'name' => 'Parent', + 'label' => 'Parent', + 'parent_id' => '1', + 'is_active' => '1', + 'is_reserved' => '0', + 'parent' => 'Individual', + 'parent_label' => 'Individual', + ], + 'Staff' => + [ + 'id' => '6', + 'name' => 'Staff', + 'label' => 'Staff', + 'parent_id' => '1', + 'is_active' => '1', + 'is_reserved' => '0', + 'parent' => 'Individual', + 'parent_label' => 'Individual', + ], + 'Team' => + [ + 'id' => '7', + 'name' => 'Team', + 'label' => 'Team', + 'parent_id' => '3', + 'is_active' => '1', + 'is_reserved' => '0', + 'parent' => 'Organization', + 'parent_label' => 'Organization', + ], + 'Sponsor' => + [ + 'id' => '8', + 'name' => 'Sponsor', + 'label' => 'Sponsor', + 'parent_id' => '3', + 'is_active' => '1', + 'is_reserved' => '0', + 'parent' => 'Organization', + 'parent_label' => 'Organization', + ], + 'sub1_individual' => + [ + 'id' => $this->ids['ContactType'][0], + 'name' => 'sub1_individual', + 'label' => 'sub1_individual', + 'parent_id' => '1', + 'is_active' => '1', + 'is_reserved' => '0', + 'parent' => 'Individual', + 'parent_label' => 'Individual', + ], + 'sub2_individual' => + [ + 'id' => $this->ids['ContactType'][1], + 'name' => 'sub2_individual', + 'label' => 'sub2_individual', + 'parent_id' => '1', + 'is_active' => '1', + 'is_reserved' => '0', + 'parent' => 'Individual', + 'parent_label' => 'Individual', + ], + 'sub_organization' => + [ + 'id' => $this->ids['ContactType'][2], + 'name' => 'sub_organization', + 'label' => 'sub_organization', + 'parent_id' => '3', + 'is_active' => '1', + 'is_reserved' => '0', + 'parent' => 'Organization', + 'parent_label' => 'Organization', + ], + 'sub_household' => + [ + 'id' => $this->ids['ContactType'][3], + 'name' => 'sub_household', + 'label' => 'sub_household', + 'parent_id' => '2', + 'is_active' => '1', + 'is_reserved' => '0', + 'parent' => 'Household', + 'parent_label' => 'Household', + ], + ]; + } + + /** + * Get subtypes for all main types. + * + * @return array + */ + public function getExpectedAllSubtypes() { + return array_merge( + $this->getExpectedContactSubTypes('Individual'), + $this->getExpectedContactSubTypes('Household'), + $this->getExpectedContactSubTypes('Organization') + ); + } + + /** + * Get the expected subtypes of the given contact type. + * + * @param string $parentType + * + * @return array + */ + public function getExpectedContactSubTypes($parentType) { + $expected = $this->getExpectedContactTypes(); + foreach ($expected as $index => $values) { + if (($values['parent_label'] ?? '') !== $parentType) { + unset($expected[$index]); + } + } + return $expected; + } + /** * Test add() methods with valid data * success expected @@ -196,7 +352,7 @@ class CRM_Contact_BAO_ContactType_ContactTypeTest extends CiviUnitTestCase { 'is_active' => 1, ]; $result = CRM_Contact_BAO_ContactType::add($params); - $this->assertEquals($result, NULL, 'In line' . __LINE__); + $this->assertEquals($result, NULL); } /** @@ -223,7 +379,6 @@ class CRM_Contact_BAO_ContactType_ContactTypeTest extends CiviUnitTestCase { * Test del() with invalid data */ public function testDelInvalid() { - $del = CRM_Contact_BAO_ContactType::del(NULL); $this->assertEquals($del, FALSE); } -- 2.25.1