Fix bug inn primary handling where TRUE rather than 1 used

diff --git a/CRM/Core/BAO/Address.php b/CRM/Core/BAO/Address.php
index ea7da9440c15adeb8cf30a5c1d73ce4dfc23e5ba..771c6cc95f4e776a8da0ffa16523f17b5931adfe 100644 (file)
--- a/CRM/Core/BAO/Address.php
+++ b/CRM/Core/BAO/Address.php
@@ -131,10 +131,7 @@ class CRM_Core_BAO_Address extends CRM_Core_DAO_Address {
     $hook = empty($params['id']) ? 'create' : 'edit';
     CRM_Utils_Hook::pre($hook, 'Address', CRM_Utils_Array::value('id', $params), $params);
 
-    // if id is set & is_primary isn't we can assume no change
-    if (is_numeric(CRM_Utils_Array::value('is_primary', $params)) || empty($params['id'])) {
-      CRM_Core_BAO_Block::handlePrimary($params, get_class());
-    }
+    CRM_Core_BAO_Block::handlePrimary($params, get_class());
 
     // (prevent chaining 1 and 3) CRM-21214
     if (isset($params['master_id']) && !CRM_Utils_System::isNull($params['master_id'])) {

diff --git a/CRM/Core/BAO/Block.php b/CRM/Core/BAO/Block.php
index 4f17f5b57c49fddc8b9a37862a66d62df482e9fe..07167d621c553485b3a7a434f0d3508897601ac1 100644 (file)
--- a/CRM/Core/BAO/Block.php
+++ b/CRM/Core/BAO/Block.php
@@ -391,6 +391,10 @@ class CRM_Core_BAO_Block {
    * @throws API_Exception
    */
   public static function handlePrimary(&$params, $class) {
+    if (isset($params['id']) && CRM_Utils_System::isNull($params['is_primary'] ?? NULL)) {
+      // if id is set & is_primary isn't we can assume no change)
+      return;
+    }
     $table = CRM_Core_DAO_AllCoreTables::getTableForClass($class);
     if (!$table) {
       throw new API_Exception("Failed to locate table for class [$class]");

diff --git a/CRM/Core/BAO/Email.php b/CRM/Core/BAO/Email.php
index d92b6e94a6ea8f2e26a467d76adc4bac904b8a2e..aa0fdc956a883c5a23a74ec01aeec425b7ff2bfc 100644 (file)
--- a/CRM/Core/BAO/Email.php
+++ b/CRM/Core/BAO/Email.php
@@ -31,10 +31,7 @@ class CRM_Core_BAO_Email extends CRM_Core_DAO_Email {
    * @return object
    */
   public static function create($params) {
-    // if id is set & is_primary isn't we can assume no change
-    if (is_numeric(CRM_Utils_Array::value('is_primary', $params)) || empty($params['id'])) {
-      CRM_Core_BAO_Block::handlePrimary($params, get_class());
-    }
+    CRM_Core_BAO_Block::handlePrimary($params, get_class());
 
     $hook = empty($params['id']) ? 'create' : 'edit';
     CRM_Utils_Hook::pre($hook, 'Email', CRM_Utils_Array::value('id', $params), $params);

diff --git a/CRM/Core/BAO/IM.php b/CRM/Core/BAO/IM.php
index cdab960f6553f147df16cd296c2d536dec763594..c7e11c450cbaea76bdcdf7d88bc6f92a234c387a 100644 (file)
--- a/CRM/Core/BAO/IM.php
+++ b/CRM/Core/BAO/IM.php
@@ -30,9 +30,7 @@ class CRM_Core_BAO_IM extends CRM_Core_DAO_IM {
    * @throws \API_Exception
    */
   public static function add($params) {
-    if (empty($params['id']) || is_numeric($params['is_primary'] ?? NULL)) {
-      CRM_Core_BAO_Block::handlePrimary($params, __CLASS__);
-    }
+    CRM_Core_BAO_Block::handlePrimary($params, __CLASS__);
     return self::writeRecord($params);
   }
 

diff --git a/CRM/Core/BAO/OpenID.php b/CRM/Core/BAO/OpenID.php
index c603155a9fa51c222e925f33803d2c3045ce184b..4382097e67bcffdd1376563ef2d27d2e9c1c1cd7 100644 (file)
--- a/CRM/Core/BAO/OpenID.php
+++ b/CRM/Core/BAO/OpenID.php
@@ -31,9 +31,7 @@ class CRM_Core_BAO_OpenID extends CRM_Core_DAO_OpenID {
    * @throws \CRM_Core_Exception
    */
   public static function add($params) {
-    if (empty($params['id']) || is_numeric($params['is_primary'] ?? NULL)) {
-      CRM_Core_BAO_Block::handlePrimary($params, __CLASS__);
-    }
+    CRM_Core_BAO_Block::handlePrimary($params, __CLASS__);
     return self::writeRecord($params);
   }
 

diff --git a/CRM/Core/BAO/Phone.php b/CRM/Core/BAO/Phone.php
index dac8c9fa5fac2678d36ba7950daddc03d9dfce30..7e5a7eede55faf7e314f0e2fef87acaf6d35a221 100644 (file)
--- a/CRM/Core/BAO/Phone.php
+++ b/CRM/Core/BAO/Phone.php
@@ -26,19 +26,15 @@ class CRM_Core_BAO_Phone extends CRM_Core_DAO_Phone {
    *
    * @param array $params
    *
-   * @return object
+   * @return \CRM_Core_DAO_Phone
+   *
    * @throws API_Exception
+   * @throws \CRM_Core_Exception
    */
   public static function create($params) {
     // Ensure mysql phone function exists
     CRM_Core_DAO::checkSqlFunctionsExist();
-
-    if (is_numeric(CRM_Utils_Array::value('is_primary', $params)) ||
-      // if id is set & is_primary isn't we can assume no change
-      empty($params['id'])
-    ) {
-      CRM_Core_BAO_Block::handlePrimary($params, get_class());
-    }
+    CRM_Core_BAO_Block::handlePrimary($params, get_class());
     return self::writeRecord($params);
   }
 
@@ -69,6 +65,7 @@ class CRM_Core_BAO_Phone extends CRM_Core_DAO_Phone {
    *
    * @return array
    *   array of phone objects
+   * @throws \CRM_Core_Exception
    */
   public static function &getValues($entityBlock) {
     $getValues = CRM_Core_BAO_Block::getValues('phone', $entityBlock);

diff --git a/tests/phpunit/api/v3/PhoneTest.php b/tests/phpunit/api/v3/PhoneTest.php
index 6b13d392dcc942df285d7bfd25648098e659d5ff..a051b92656d0664d3df82127103175af1a13294c 100644 (file)
--- a/tests/phpunit/api/v3/PhoneTest.php
+++ b/tests/phpunit/api/v3/PhoneTest.php
@@ -22,6 +22,13 @@ class api_v3_PhoneTest extends CiviUnitTestCase {
   protected $_params;
   protected $_entity;
 
+  /**
+   * Should location types be checked to ensure primary addresses are correctly assigned after each test.
+   *
+   * @var bool
+   */
+  protected $isLocationTypesOnPostAssert = TRUE;
+
   public function setUp() {
     $this->_entity = 'Phone';
     parent::setUp();
@@ -35,19 +42,21 @@ class api_v3_PhoneTest extends CiviUnitTestCase {
       'contact_id' => $this->_contactID,
       'location_type_id' => $this->_locationType,
       'phone' => '(123) 456-7890',
-      'is_primary' => 1,
+      'is_primary' => TRUE,
       'phone_type_id' => 1,
     ];
   }
 
   /**
    * @param int $version
+   *
    * @dataProvider versionThreeAndFour
+   * @throws \CRM_Core_Exception
    */
   public function testCreatePhone($version) {
     $this->_apiversion = $version;
 
-    $result = $this->callAPIAndDocument('phone', 'create', $this->_params, __FUNCTION__, __FILE__);
+    $result = $this->callAPIAndDocument('Phone', 'create', $this->_params, __FUNCTION__, __FILE__);
     $this->assertEquals(1, $result['count']);
     $this->assertNotNull($result['values'][$result['id']]['id']);
 
@@ -59,7 +68,9 @@ class api_v3_PhoneTest extends CiviUnitTestCase {
    * the LocationType default
    *
    * @param int $version
+   *
    * @dataProvider versionThreeAndFour
+   * @throws \CRM_Core_Exception
    */
   public function testCreatePhoneDefaultLocation($version) {
     $this->_apiversion = $version;
@@ -164,17 +175,20 @@ class api_v3_PhoneTest extends CiviUnitTestCase {
 
   /**
    * @param int $version
+   *
    * @dataProvider versionThreeAndFour
+   * @throws \CRM_Core_Exception
    */
   public function testCreatePhonePrimaryHandlingChangeExisting($version) {
     $this->_apiversion = $version;
     $phone1 = $this->callAPISuccess('phone', 'create', $this->_params);
-    $phone2 = $this->callAPISuccess('phone', 'create', $this->_params);
+    $this->callAPISuccess('phone', 'create', $this->_params);
     $check = $this->callAPISuccess('phone', 'getcount', [
       'is_primary' => 1,
       'contact_id' => $this->_contactID,
     ]);
     $this->assertEquals(1, $check);
+    $this->callAPISuccess('Phone', 'create', ['id' => $phone1['id'], 'is_primary' => TRUE]);
   }
 
 }