From ae7397a3c848bf089ad037b80267846aec2f39db Mon Sep 17 00:00:00 2001 From: eileen Date: Fri, 2 Oct 2020 11:57:07 +1300 Subject: [PATCH] dev/core#2046 Migrate BAO_Address::create towards standardisation Our standard expectation is that each BAO will have a create action that handles a single entity. For legacy reasons Address.create has special handling for multiple addresses. This extracts that handling into a separate function (legacyCreate) and updates the (very few) places currently calling create to call that. With this merged apis v3 & v4 can call Address.create - currently v3 is hard coded to call add and v4 bypasses the BAO altogher. The v4 api bypass means we are not managing is_primay integrity on v4 api calls --- CRM/Contact/Form/Inline/Address.php | 2 +- CRM/Core/BAO/Address.php | 131 ++++++++++++--------- CRM/Core/BAO/Location.php | 6 +- tests/phpunit/CRM/Core/BAO/AddressTest.php | 6 +- 4 files changed, 80 insertions(+), 65 deletions(-) diff --git a/CRM/Contact/Form/Inline/Address.php b/CRM/Contact/Form/Inline/Address.php index e33118c913..4170a36f0b 100644 --- a/CRM/Contact/Form/Inline/Address.php +++ b/CRM/Contact/Form/Inline/Address.php @@ -169,7 +169,7 @@ class CRM_Contact_Form_Inline_Address extends CRM_Contact_Form_Inline { } // save address changes - $address = CRM_Core_BAO_Address::create($params, TRUE); + $address = CRM_Core_BAO_Address::legacyCreate($params, TRUE); $this->log(); $this->ajaxResponse['addressId'] = $address[0]->id; diff --git a/CRM/Core/BAO/Address.php b/CRM/Core/BAO/Address.php index 329940adbd..fdbf08fe6b 100644 --- a/CRM/Core/BAO/Address.php +++ b/CRM/Core/BAO/Address.php @@ -29,69 +29,15 @@ class CRM_Core_BAO_Address extends CRM_Core_DAO_Address { * True if you need to fix (format) address values. * before inserting in db * - * @return array|NULL + * @return array|NULL|self * array of created address */ public static function create(&$params, $fixAddress = TRUE) { if (!isset($params['address']) || !is_array($params['address'])) { - return NULL; - } - CRM_Core_BAO_Block::sortPrimaryFirst($params['address']); - $contactId = NULL; - - $updateBlankLocInfo = CRM_Utils_Array::value('updateBlankLocInfo', $params, FALSE); - $contactId = $params['contact_id']; - //get all the addresses for this contact - $addresses = self::allAddress($contactId); - - $isPrimary = $isBilling = TRUE; - $blocks = []; - foreach ($params['address'] as $key => $value) { - if (!is_array($value)) { - continue; - } - - $addressExists = self::dataExists($value); - if (empty($value['id'])) { - if (!empty($addresses) && !empty($value['location_type_id']) && array_key_exists($value['location_type_id'], $addresses)) { - $value['id'] = $addresses[$value['location_type_id']]; - } - } - - // Note there could be cases when address info already exist ($value[id] is set) for a contact/entity - // BUT info is not present at this time, and therefore we should be really careful when deleting the block. - // $updateBlankLocInfo will help take appropriate decision. CRM-5969 - if (isset($value['id']) && !$addressExists && $updateBlankLocInfo) { - //delete the existing record - CRM_Core_BAO_Block::blockDelete('Address', ['id' => $value['id']]); - continue; - } - elseif (!$addressExists) { - continue; - } - - if ($isPrimary && !empty($value['is_primary'])) { - $isPrimary = FALSE; - } - else { - $value['is_primary'] = 0; - } - - if ($isBilling && !empty($value['is_billing'])) { - $isBilling = FALSE; - } - else { - $value['is_billing'] = 0; - } - - if (empty($value['manual_geo_code'])) { - $value['manual_geo_code'] = 0; - } - $value['contact_id'] = $contactId; - $blocks[] = self::add($value, $fixAddress); + return self::add($params, $fixAddress); } - - return $blocks; + CRM_Core_Error::deprecatedFunctionWarning('Use legacyCreate if not doing a single crud action'); + return self::legacyCreate($params, $fixAddress); } /** @@ -1366,4 +1312,73 @@ SELECT is_primary, return TRUE; } + /** + * Create multiple addresses using legacy methodology. + * + * @param array $params + * @param bool $fixAddress + * + * @return array|null + */ + public static function legacyCreate(array $params, bool $fixAddress) { + if (!isset($params['address']) || !is_array($params['address'])) { + return NULL; + } + CRM_Core_BAO_Block::sortPrimaryFirst($params['address']); + $contactId = NULL; + + $updateBlankLocInfo = CRM_Utils_Array::value('updateBlankLocInfo', $params, FALSE); + $contactId = $params['contact_id']; + //get all the addresses for this contact + $addresses = self::allAddress($contactId); + + $isPrimary = $isBilling = TRUE; + $blocks = []; + foreach ($params['address'] as $key => $value) { + if (!is_array($value)) { + continue; + } + + $addressExists = self::dataExists($value); + if (empty($value['id'])) { + if (!empty($addresses) && !empty($value['location_type_id']) && array_key_exists($value['location_type_id'], $addresses)) { + $value['id'] = $addresses[$value['location_type_id']]; + } + } + + // Note there could be cases when address info already exist ($value[id] is set) for a contact/entity + // BUT info is not present at this time, and therefore we should be really careful when deleting the block. + // $updateBlankLocInfo will help take appropriate decision. CRM-5969 + if (isset($value['id']) && !$addressExists && $updateBlankLocInfo) { + //delete the existing record + CRM_Core_BAO_Block::blockDelete('Address', ['id' => $value['id']]); + continue; + } + elseif (!$addressExists) { + continue; + } + + if ($isPrimary && !empty($value['is_primary'])) { + $isPrimary = FALSE; + } + else { + $value['is_primary'] = 0; + } + + if ($isBilling && !empty($value['is_billing'])) { + $isBilling = FALSE; + } + else { + $value['is_billing'] = 0; + } + + if (empty($value['manual_geo_code'])) { + $value['manual_geo_code'] = 0; + } + $value['contact_id'] = $contactId; + $blocks[] = self::add($value, $fixAddress); + } + return $blocks; + } + } diff --git a/CRM/Core/BAO/Location.php b/CRM/Core/BAO/Location.php index 911862b8fc..fd1545efe9 100644 --- a/CRM/Core/BAO/Location.php +++ b/CRM/Core/BAO/Location.php @@ -45,11 +45,11 @@ class CRM_Core_BAO_Location extends CRM_Core_DAO { // create location blocks. foreach (self::$blocks as $block) { - if ($block != 'address') { + if ($block !== 'address') { $location[$block] = CRM_Core_BAO_Block::create($block, $params); } - else { - $location[$block] = CRM_Core_BAO_Address::create($params, $fixAddress); + elseif (is_array($params['address'] ?? NULL)) { + $location[$block] = CRM_Core_BAO_Address::legacyCreate($params, $fixAddress); } } diff --git a/tests/phpunit/CRM/Core/BAO/AddressTest.php b/tests/phpunit/CRM/Core/BAO/AddressTest.php index 8e8ddcfe77..707df1e148 100644 --- a/tests/phpunit/CRM/Core/BAO/AddressTest.php +++ b/tests/phpunit/CRM/Core/BAO/AddressTest.php @@ -50,7 +50,7 @@ class CRM_Core_BAO_AddressTest extends CiviUnitTestCase { $fixAddress = TRUE; - CRM_Core_BAO_Address::create($params, $fixAddress); + CRM_Core_BAO_Address::legacyCreate($params, $fixAddress); $addressId = $this->assertDBNotNull('CRM_Core_DAO_Address', 'Oberoi Garden', 'id', 'street_address', 'Database check for created address.' ); @@ -76,7 +76,7 @@ class CRM_Core_BAO_AddressTest extends CiviUnitTestCase { ]; $params['contact_id'] = $contactId; - $block = CRM_Core_BAO_Address::create($params, $fixAddress); + $block = CRM_Core_BAO_Address::legacyCreate($params, $fixAddress); $this->assertDBNotNull('CRM_Core_DAO_Address', $contactId, 'id', 'contact_id', 'Database check for updated address by contactId.' @@ -253,7 +253,7 @@ class CRM_Core_BAO_AddressTest extends CiviUnitTestCase { $fixAddress = TRUE; - CRM_Core_BAO_Address::create($params, $fixAddress); + CRM_Core_BAO_Address::legacyCreate($params, $fixAddress); $addressId = $this->assertDBNotNull('CRM_Core_DAO_Address', $contactId, 'id', 'contact_id', 'Database check for created address.' -- 2.25.1