From 531b6c9e4a96e5212dc1a637d6efe218ff74aedd Mon Sep 17 00:00:00 2001 From: eileen Date: Thu, 24 Sep 2020 12:49:11 +1200 Subject: [PATCH] Simplify CRM_Core_BAO_Location::createLocBlock by moving eventLocation specific handling back to the class On grepping universe we find entity is only passed into CRM_Core_BAO_Location::createLocBlock from this one place in the code - ergo we can do the handling for it on that class & not in the shared code Test cover added in preparation for this to CRM_Event_Form_ManageEvent_LocationTest --- CRM/Core/BAO/Location.php | 35 +++++++-------------- CRM/Event/Form/ManageEvent/Location.php | 14 +++++++-- tests/phpunit/CRM/Core/BAO/LocationTest.php | 20 +++++++++--- 3 files changed, 38 insertions(+), 31 deletions(-) diff --git a/CRM/Core/BAO/Location.php b/CRM/Core/BAO/Location.php index 871e593267..add13da479 100644 --- a/CRM/Core/BAO/Location.php +++ b/CRM/Core/BAO/Location.php @@ -35,11 +35,9 @@ class CRM_Core_BAO_Location extends CRM_Core_DAO { * True if you need to fix (format) address values. * before inserting in db * - * @param null $entity - * * @return array */ - public static function create(&$params, $fixAddress = TRUE, $entity = NULL) { + public static function create(&$params, $fixAddress = TRUE) { $location = []; if (!self::dataExists($params)) { return $location; @@ -48,29 +46,19 @@ class CRM_Core_BAO_Location extends CRM_Core_DAO { // create location blocks. foreach (self::$blocks as $block) { if ($block != 'address') { - $location[$block] = CRM_Core_BAO_Block::create($block, $params, $entity); + $location[$block] = CRM_Core_BAO_Block::create($block, $params); } else { - $location[$block] = CRM_Core_BAO_Address::create($params, $fixAddress, $entity); + $location[$block] = CRM_Core_BAO_Address::create($params, $fixAddress); } } - if ($entity) { - // this is a special case for adding values in location block table - - $location['id'] = self::createLocBlock($location, [ - 'entity_table' => $params['entity_table'], - 'entity_id' => $params['entity_id'], - ]); - } - else { - // when we come from a form which displays all the location elements (like the edit form or the inline block - // elements, we can skip the below check. The below check adds quite a feq queries to an already overloaded - // form - if (empty($params['updateBlankLocInfo'])) { - // make sure contact should have only one primary block, CRM-5051 - self::checkPrimaryBlocks(CRM_Utils_Array::value('contact_id', $params)); - } + // when we come from a form which displays all the location elements (like the edit form or the inline block + // elements, we can skip the below check. The below check adds quite a feq queries to an already overloaded + // form + if (empty($params['updateBlankLocInfo'])) { + // make sure contact should have only one primary block, CRM-5051 + self::checkPrimaryBlocks(CRM_Utils_Array::value('contact_id', $params)); } return $location; @@ -151,10 +139,9 @@ WHERE e.id = %1"; */ public static function addLocBlock($params) { $locBlock = new CRM_Core_DAO_LocBlock(); - $locBlock->copyValues($params); - - return $locBlock->save(); + $locBlock->save(); + return $locBlock; } /** diff --git a/CRM/Event/Form/ManageEvent/Location.php b/CRM/Event/Form/ManageEvent/Location.php index 12d6c73862..a1a79396a0 100644 --- a/CRM/Event/Form/ManageEvent/Location.php +++ b/CRM/Event/Form/ManageEvent/Location.php @@ -234,11 +234,13 @@ class CRM_Event_Form_ManageEvent_Location extends CRM_Event_Form_ManageEvent { $isUpdateToExistingLocationBlock = !empty($params['loc_event_id']) && (int) $params['loc_event_id'] === $this->locationBlock['loc_block_id']; // It should be impossible for there to be no default location type. Consider removing this handling $defaultLocationTypeID = CRM_Core_BAO_LocationType::getDefault()->id ?? 1; + $location = []; foreach ([ 'address' => $params['address'], 'phone' => $params['phone'], 'email' => $params['email'], ] as $block => $locationEntities) { + $params[$block][1]['is_primary'] = 1; foreach ($locationEntities as $index => $locationEntity) { $params[$block][$index]['location_type_id'] = $defaultLocationTypeID; @@ -247,10 +249,18 @@ class CRM_Event_Form_ManageEvent_Location extends CRM_Event_Form_ManageEvent { $params[$block][$index]['id'] = $this->locationBlock['loc_block.' . $block . $fieldKey]; } } + if ($block !== 'address') { + $location[$block] = CRM_Core_BAO_Block::create($block, $params, 'event'); + } + else { + $location[$block] = CRM_Core_BAO_Address::create($params, TRUE, 'event'); + } } - // create/update event location - $params['loc_block_id'] = CRM_Core_BAO_Location::create($params, TRUE, 'event')['id']; + $params['loc_block_id'] = CRM_Core_BAO_Location::createLocBlock($location, [ + 'entity_table' => $params['entity_table'], + 'entity_id' => $params['entity_id'], + ]); // finally update event params $params['id'] = $this->_id; diff --git a/tests/phpunit/CRM/Core/BAO/LocationTest.php b/tests/phpunit/CRM/Core/BAO/LocationTest.php index 69a003a66d..28b646d43a 100644 --- a/tests/phpunit/CRM/Core/BAO/LocationTest.php +++ b/tests/phpunit/CRM/Core/BAO/LocationTest.php @@ -245,12 +245,22 @@ class CRM_Core_BAO_LocationTest extends CiviUnitTestCase { //create location block. //with various element of location block //like address, phone, email, im. - $locBlockId = CRM_Core_BAO_Location::create($params, NULL, TRUE)['id']; + foreach (['phone', 'email', 'im', 'openid', 'address'] as $block) { + if ($block !== 'address') { + $location[$block] = CRM_Core_BAO_Block::create($block, $params, 'event'); + } + else { + $location[$block] = CRM_Core_BAO_Address::create($params, TRUE, 'event'); + } + } //update event record with location block id $eventParams = [ 'id' => $event['id'], - 'loc_block_id' => $locBlockId, + 'loc_block_id' => CRM_Core_BAO_Location::createLocBlock($location, [ + 'entity_id' => $event['id'], + 'entity_table' => 'civicrm_event', + ]), ]; CRM_Event_BAO_Event::add($eventParams); @@ -261,11 +271,11 @@ class CRM_Core_BAO_LocationTest extends CiviUnitTestCase { $event['id'], 'loc_block_id', 'id', - $locBlockId, + $eventParams['loc_block_id'], 'Checking database for the record.' ); $locElementIds = []; - $locParams = ['id' => $locBlockId]; + $locParams = ['id' => $eventParams['loc_block_id']]; CRM_Core_DAO::commonRetrieve('CRM_Core_DAO_LocBlock', $locParams, $locElementIds @@ -328,7 +338,7 @@ class CRM_Core_BAO_LocationTest extends CiviUnitTestCase { $this->assertDBCompareValues('CRM_Core_DAO_IM', $searchParams, $compareParams); // Cleanup. - CRM_Core_BAO_Location::deleteLocBlock($locBlockId); + CRM_Core_BAO_Location::deleteLocBlock($eventParams['loc_block_id']); $this->eventDelete($event['id']); $this->contactDelete($this->_contactId); } -- 2.25.1