From 9f8f650e3869b1cc255c60e2b8020c511aba2693 Mon Sep 17 00:00:00 2001 From: eileen Date: Sat, 26 Sep 2020 15:15:13 +1200 Subject: [PATCH] dev/core#2046 Ensure location BAOs have create actions, deprecate add, simplify block code Per https://lab.civicrm.org/dev/core/-/issues/2046 we have a goal (long term) to consolidate create & add functions into one function - with the functionality moved from add into create & add becoming a deprecated wrapper around create. This makes that change on the IM & OpenID in line with the previous change on Phone & Email, leaving just Address & Website which are both a bit trickier. The code in Block runs on the 4 that are updated so this simplifies & confirms all 4 are calling functions that handle primary. We could switch to an apiv4 call - I just want to be a bit more sure we've handlled primary there & also that we look into Address & website --- CRM/Core/BAO/Block.php | 17 ++--------- CRM/Core/BAO/IM.php | 18 +++++++++++- CRM/Core/BAO/OpenID.php | 18 +++++++++++- tests/phpunit/CRM/Core/BAO/IMTest.php | 12 ++++---- tests/phpunit/CRM/Core/BAO/OpenIDTest.php | 36 +++++++---------------- 5 files changed, 52 insertions(+), 49 deletions(-) diff --git a/CRM/Core/BAO/Block.php b/CRM/Core/BAO/Block.php index 07167d621c..6308a63363 100644 --- a/CRM/Core/BAO/Block.php +++ b/CRM/Core/BAO/Block.php @@ -323,21 +323,8 @@ class CRM_Core_BAO_Block { } $blockFields = array_merge($value, $contactFields); - if ($name === 'Email') { - // @todo ideally all would call the api which is our main tested function, - // and towards that call the create rather than add which is preferred by the - // api. In order to be careful with change only email is swapped over here because it - // is specifically tested in testImportParserWithUpdateWithContactID - // and the primary handling is otherwise bypassed on importing an email update. - $blocks[] = CRM_Core_BAO_Email::create($blockFields); - } - elseif ($name === 'Phone') { - $blocks[] = CRM_Core_BAO_Phone::create($blockFields); - } - else { - $baoString = 'CRM_Core_BAO_' . $name; - $blocks[] = $baoString::add($blockFields); - } + $baoString = 'CRM_Core_BAO_' . $name; + $blocks[] = $baoString::create($blockFields); } return $blocks; diff --git a/CRM/Core/BAO/IM.php b/CRM/Core/BAO/IM.php index c7e11c450c..bca4ce1365 100644 --- a/CRM/Core/BAO/IM.php +++ b/CRM/Core/BAO/IM.php @@ -29,11 +29,27 @@ class CRM_Core_BAO_IM extends CRM_Core_DAO_IM { * @throws \CRM_Core_Exception * @throws \API_Exception */ - public static function add($params) { + public static function create($params) { CRM_Core_BAO_Block::handlePrimary($params, __CLASS__); return self::writeRecord($params); } + /** + * Create or update IM record. + * + * @deprecated + * + * @param array $params + * + * @return \CRM_Core_DAO|\CRM_Core_DAO_IM + * @throws \CRM_Core_Exception + * @throws \API_Exception + */ + public static function add($params) { + CRM_Core_Error::deprecatedFunctionWarning('use the v4 api'); + return self::create($params); + } + /** * Given the list of params in the params array, fetch the object * and store the values in the values array diff --git a/CRM/Core/BAO/OpenID.php b/CRM/Core/BAO/OpenID.php index 4382097e67..f200043573 100644 --- a/CRM/Core/BAO/OpenID.php +++ b/CRM/Core/BAO/OpenID.php @@ -30,11 +30,27 @@ class CRM_Core_BAO_OpenID extends CRM_Core_DAO_OpenID { * @throws \API_Exception * @throws \CRM_Core_Exception */ - public static function add($params) { + public static function create($params) { CRM_Core_BAO_Block::handlePrimary($params, __CLASS__); return self::writeRecord($params); } + /** + * Create or update OpenID record. + * + * @deprecated + * + * @param array $params + * + * @return \CRM_Core_DAO|\CRM_Core_DAO_IM + * @throws \CRM_Core_Exception + * @throws \API_Exception + */ + public static function add($params) { + CRM_Core_Error::deprecatedFunctionWarning('use the v4 api'); + return self::create($params); + } + /** * Given the list of params in the params array, fetch the object * and store the values in the values array diff --git a/tests/phpunit/CRM/Core/BAO/IMTest.php b/tests/phpunit/CRM/Core/BAO/IMTest.php index cc82bd0eb0..5847ab1b5d 100644 --- a/tests/phpunit/CRM/Core/BAO/IMTest.php +++ b/tests/phpunit/CRM/Core/BAO/IMTest.php @@ -1,5 +1,7 @@ individualCreate(); - $params = []; $params = [ 'name' => 'jane.doe', 'provider_id' => 1, @@ -25,7 +26,7 @@ class CRM_Core_BAO_IMTest extends CiviUnitTestCase { 'contact_id' => $contactId, ]; - CRM_Core_BAO_IM::add($params); + IM::create(FALSE)->setValues($params)->execute(); $imId = $this->assertDBNotNull('CRM_Core_DAO_IM', 'jane.doe', 'id', 'name', 'Database check for created IM name.' @@ -33,7 +34,6 @@ class CRM_Core_BAO_IMTest extends CiviUnitTestCase { // Now call add() to modify an existing IM - $params = []; $params = [ 'id' => $imId, 'contact_id' => $contactId, @@ -41,7 +41,7 @@ class CRM_Core_BAO_IMTest extends CiviUnitTestCase { 'name' => 'doe.jane', ]; - CRM_Core_BAO_IM::add($params); + IM::update(FALSE)->addWhere('id', '=', $imId)->setValues($params)->execute(); $isEditIM = $this->assertDBNotNull('CRM_Core_DAO_IM', $imId, 'provider_id', 'id', 'Database check on updated IM provider_name record.'); $this->assertEquals($isEditIM, 3, 'Verify IM provider_id value is 3.'); diff --git a/tests/phpunit/CRM/Core/BAO/OpenIDTest.php b/tests/phpunit/CRM/Core/BAO/OpenIDTest.php index 0e44466358..3bf68c709c 100644 --- a/tests/phpunit/CRM/Core/BAO/OpenIDTest.php +++ b/tests/phpunit/CRM/Core/BAO/OpenIDTest.php @@ -1,5 +1,7 @@ quickCleanup($tablesToTruncate); - } - - public function setUp() { - parent::setUp(); + $this->quickCleanup(['civicrm_contact', 'civicrm_openid', 'civicrm_email']); + parent::tearDown(); } /** @@ -33,15 +28,13 @@ class CRM_Core_BAO_OpenIDTest extends CiviUnitTestCase { 'is_primary' => 1, ]; - $openObject = CRM_Core_BAO_OpenID::add($params); - - $openId = $openObject->id; + $openId = OpenID::create(FALSE)->setValues($params)->execute()->first()['id']; $this->assertDBNotNull('CRM_Core_DAO_OpenID', $openIdURL, 'id', 'openid', 'Database check for created OpenID.' ); - // Now call add() to modify an existing open-id record + // Now modify an existing open-id record $params = [ 'id' => $openId, @@ -51,15 +44,12 @@ class CRM_Core_BAO_OpenIDTest extends CiviUnitTestCase { 'allowed_to_login' => 1, ]; - CRM_Core_BAO_OpenID::add($params); + OpenID::update(FALSE)->addWhere('id', '=', $openId)->setValues($params)->execute(); $allowedToLogin = $this->assertDBNotNull('CRM_Core_DAO_OpenID', $openId, 'allowed_to_login', 'id', 'Database check on updated OpenID record.' ); $this->assertEquals($allowedToLogin, 1, 'Verify allowed_to_login value is 1.'); - - $this->contactDelete($contactId); - $this->assertDBRowNotExist('CRM_Contact_DAO_Contact', $contactId); } /** @@ -79,9 +69,7 @@ class CRM_Core_BAO_OpenIDTest extends CiviUnitTestCase { 'allowed_to_login' => 1, ]; - $openObjectOne = CRM_Core_BAO_OpenID::add($params); - - $openIdOne = $openObjectOne->id; + $openIdOne = OpenID::create(FALSE)->setValues($params)->execute()->first()['id']; $this->assertDBNotNull('CRM_Core_DAO_OpenID', $openIdURLOne, 'id', 'openid', 'Database check for created OpenID.' ); @@ -94,8 +82,7 @@ class CRM_Core_BAO_OpenIDTest extends CiviUnitTestCase { 'openid' => $openIdURLTwo, ]; - $openObjectTwo = CRM_Core_BAO_OpenID::add($params); - $openIdTwo = $openObjectTwo->id; + $openIdTwo = OpenID::create(FALSE)->setValues($params)->execute()->first()['id']; $this->assertDBNotNull('CRM_Core_DAO_OpenID', $openIdURLTwo, 'id', 'openid', 'Database check for created OpenID.' @@ -105,7 +92,7 @@ class CRM_Core_BAO_OpenIDTest extends CiviUnitTestCase { $openIds = CRM_Core_BAO_OpenID::allOpenIDs($contactId); // check number of openids for the contact - $this->assertEquals(2, count($openIds), 'Checking number of returned open-ids.'); + $this->assertCount(2, $openIds, 'Checking number of returned open-ids.'); // check first openid values $this->assertEquals($openIdURLOne, $openIds[$openIdOne]['openid'], @@ -120,9 +107,6 @@ class CRM_Core_BAO_OpenIDTest extends CiviUnitTestCase { ); $this->assertEquals(0, $openIds[$openIdTwo]['is_primary'], 'Confirm is_primary field value for second openid.'); $this->assertEquals(0, $openIds[$openIdTwo]['allowed_to_login'], 'Confirm allowed_to_login field value for second openid.'); - - $this->contactDelete($contactId); - $this->assertDBRowNotExist('CRM_Contact_DAO_Contact', $contactId); } } -- 2.25.1