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

diff --git a/CRM/Contact/BAO/ContactType.php b/CRM/Contact/BAO/ContactType.php
index 27d594c4a4c643ddee7c13179a7a8908d296399b..685ff08f6fa693717a77310c2073a849394c0429 100644 (file)
--- 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 b6af1999903ef4303a0944809c1e244657654c45..6fd24f0289b84eacb2e6c676b87a39790ad83f7a 100644 (file)
--- a/tests/phpunit/CRM/Contact/BAO/ContactType/ContactTypeTest.php
+++ b/tests/phpunit/CRM/Contact/BAO/ContactType/ContactTypeTest.php
@@ -1,4 +1,5 @@
 <?php
+use Civi\Api4\ContactType;
 
 /**
  * Class CRM_Contact_BAO_ContactType_ContactTypeTest
@@ -8,108 +9,75 @@ class CRM_Contact_BAO_ContactType_ContactTypeTest extends CiviUnitTestCase {
 
   public function setUp() {
     parent::setUp();
-    $labelsub1 = 'sub1_individual' . substr(sha1(rand()), 0, 7);
     $params = [
-      'label' => $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);
   }